Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions grpc_servicer/smg_grpc_servicer/vllm/servicer.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,10 +323,21 @@ async def Generate(
finally:
self._mm_inflight.release()
# A PD prefill leg answers with the identity so decode is
# served without pixels or references.
# served without pixels or references. The identity is an
# optimisation with a fallback (decode reprocesses), so a
# shape it cannot read must not fail a served request.
if kv_transfer_params is not None:
if media_identity_supported():
media_identity = build_media_identity(prompt)
try:
media_identity = build_media_identity(prompt)
except Exception as e: # noqa: BLE001 - any failure falls back
logger.warning(
"Request %s: media identity not built (%s); the decode leg "
"will reprocess the media",
request_id,
e,
)
Comment on lines +334 to +339

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Nit: This warning logs only str(e), which drops the exception type and traceback. For a bare KeyError — one of the failure modes this PR anticipates (prompt["prompt_token_ids"]) — the log reads media identity not built ('prompt_token_ids'), with no hint of what went wrong or where. Since this catch exists precisely for unanticipated shapes, the traceback is the diagnostic you'll want. This file already does that for the analogous best-effort fallback at _notify_kv_rejected (line 502-507): keep the message as-is and add exc_info=True (the test's getMessage() assertions are unaffected).

media_identity = None
else:
logger.warning(
"Request %s: the installed smg-grpc-proto has no media_identity; "
Expand Down
24 changes: 24 additions & 0 deletions grpc_servicer/tests/test_vllm_pd_media_identity.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,3 +154,27 @@ async def test_decode_leg_from_identity_never_touches_the_processor(monkeypatch)
decode = await _complete(servicer, _request(kv=True, identity=prefill.media_identity))
assert processor.calls == [], "the decode leg carried identity, not references"
assert not decode.HasField("media_identity")


@pytest.mark.asyncio
async def test_an_identity_that_cannot_be_built_does_not_fail_the_request(monkeypatch, caplog):
# The identity is an optimisation with a fallback (decode reprocesses the
# media), so a shape it did not anticipate must not turn a served request
# into an error.
from smg_grpc_servicer.vllm import servicer as servicer_module

def explode(prompt):
raise RuntimeError("stack expects each tensor to be equal size")

monkeypatch.setattr(servicer_module, "build_media_identity", explode)
processor = _Processor()
servicer = _servicer(monkeypatch, processor)
with caplog.at_level("WARNING", logger="smg_grpc_servicer.vllm.servicer"):
complete = await _complete(servicer, _request(refs=True, kv=True))
assert processor.calls == ["req-pd"]
assert list(complete.output_ids) == [42]
assert not complete.HasField("media_identity")
assert any(
"media identity not built" in r.getMessage() and "equal size" in r.getMessage()
for r in caplog.records
)
Loading