[apex-mochen]

vLLM Version: 0.30.0 · vLLM-Omni commit: 1b97ff11881173d9926c67cb9ace9457f5e57954 (this branch is one commit on top of it)
Test Plan / Result: the exact commands and their pass counts are tabulated under "Tests" below; the base-vs-head negative control is under "Negative control".

---

Problem

With a tool-call parser configured (vllm serve --omni ... --tool-call-parser hermes), omni accepts the flag and runs the parser, but /metrics never exposes vllm:tool_call_parser_invocations_total. The same server launched through upstream vllm serve does expose it. Every parser invocation is silently dropped from the metric.

Root cause

record_tool_parser_invocation() in vllm/parser/metrics.py opens with an early return:

if _tool_call_parser_invocations is None:
    return

The counter is created only by init_parser_metrics(), and upstream's init_app_state (vllm/entrypoints/launchers/api_server/app_state.py) calls it exactly when a parser is configured:

if args.tool_call_parser is not None:
    from vllm.parser.metrics import init_parser_metrics
    init_parser_metrics(model_name=cast(str, vllm_config.model_config.served_model_name))

omni does not use that init path — it has its own omni_init_app_state in vllm_omni/entrypoints/openai/api_server.py, which has zero references to init_parser_metrics (or to tool_call_parser_invocations), while at the same time it does hand the parser to its handlers:

722   tool_parser=args.tool_call_parser   # duplex      -> OnlineRenderer
739   tool_parser=args.tool_call_parser   # duplex      -> OmniOpenAIServingChat
973   tool_parser=args.tool_call_parser   # multi-stage -> OnlineRenderer
989   tool_parser=args.tool_call_parser   # multi-stage -> OpenAIServingResponses
1013  tool_parser=args.tool_call_parser   # multi-stage -> OmniOpenAIServingChat
1127  tool_parser=args.tool_call_parser   # multi-stage -> AnthropicServingMessages

(line numbers from main; only the multi-stage ones shift once this patch is applied)

So the parser runs, calls record_tool_parser_invocation(), and the call returns immediately. This is an observability regression relative to upstream for any omni deployment that uses tool calling.

Reproduction

1. vllm serve <model> --omni --tool-call-parser hermes (any model with a chat template that supports tools).
2. Send a chat completion with a tools payload that triggers a tool call.
3. curl -s localhost:8000/metrics | grep tool_call_parser
   • upstream vllm serve: the counter is present with mode / outcome / request_type label values incrementing.
   • omni: no such metric at all (not even the zero-valued pre-created label combinations).

The same holds for the duplex and multi-stage omni paths, since all of them go through the same app-state builder.

Fix

One registration call in omni_init_app_state, mirroring upstream's guard and using the same model name the server already computed (served_model_names[0] if served_model_names else args.model):

# Upstream's init_app_state registers the parser counters whenever a
# tool-call parser is configured; until that happens
# record_tool_parser_invocation() is a silent no-op. omni wires the same
# parsers through its own init paths, so register them here too.
if getattr(args, "tool_call_parser", None) is not None:
    from vllm.parser.metrics import init_parser_metrics

    init_parser_metrics(model_name=model_name)

getattr(args, "tool_call_parser", None) rather than upstream's bare attribute access is deliberate: tests/entrypoints/openai_api/test_omni_sleep_wakeup.py builds a minimal args namespace without that attribute and exercises the pure-diffusion path, which returns before the later tool_parser=args.tool_call_parser sites. Registration is upstream's own function (idempotent by its except ValueError fallback), so re-entry and duplicate registration behave exactly as in upstream vllm serve.

Tests

Added test_parser_metrics_registered_exactly_when_a_tool_parser_is_set in tests/entrypoints/openai_api/test_api_server_guards.py, parametrized over four cases:

case	expected
no tool_call_parser attribute	not registered
tool_call_parser=None	not registered
tool_call_parser="hermes_tool_parser"	registered once, model_name="demo-model"
parser set + served_model_name=["served-alias"]	registered once, model_name="served-alias"

Executed evidence (local, vllm 0.30.0, CPU-only):

• test_api_server_guards.py — 68 passed
• test_duplex_api_server.py + test_diffusion_request_utils.py — 62 passed
• all four files including test_omni_sleep_wakeup.py — 146 passed
• ruff check and ruff format --check clean on both changed files

The reproduction was verified statically and by the regression test above; I could not start an omni server end-to-end in this environment (no GPU), so the "curl /metrics" step is the operator-facing symptom rather than a log I captured.

Negative control

vllm_omni/entrypoints/openai/api_server.py restored at file level to this PR's
base bytes (md5 7189287bf5789f4fad9a74bb7c492d1f, asserted before the run, with
init_parser_metrics absent from the file) and the new test left in place:

$ python -m pytest -q tests/entrypoints/openai_api/test_api_server_guards.py -k parser_metrics_registered
2 failed, 2 passed, 64 deselected

FAILED tests/entrypoints/openai_api/test_api_server_guards.py::test_parser_metrics_registered_exactly_when_a_tool_parser_is_set[overrides2-expected_registration2]
FAILED tests/entrypoints/openai_api/test_api_server_guards.py::test_parser_metrics_registered_exactly_when_a_tool_parser_is_set[overrides3-expected_registration3]

The two failures are exactly the parameterizations that require the registration.
The other two assert that nothing is registered (attribute absent, and
explicitly None); they pass on both sides by construction, so only the positive
cases discriminate. Re-applying the head bytes
(md5 6be274397d2af8feafeb280a71e34830) gives 4 passed.

Reviews

Patch coverage / branch review. The only new branch is the is not None guard. True is covered by cases 3 and 4, False by cases 1 and 2 (attribute absent and explicitly None). The getattr default path — attribute genuinely missing — is additionally exercised by the pre-existing test_omni_sleep_wakeup.py, which passes an args namespace lacking tool_call_parser; that test is what forced getattr over upstream's bare access. No other control flow is added or changed.

Edge cases. Normal value (a parser name) → registered with the served alias, not the raw --model; None and missing attribute → nothing registered, no AttributeError; empty served_model_names → falls back to args.model, the same expression the surrounding code already uses for the serving objects, so the metric label can never disagree with the model the server reports. Exception behaviour is inherited from init_parser_metrics unchanged.

Cross-mode / cross-backend impact. The insertion is in the shared app-state builder, so it covers duplex, multi-stage, and pure-diffusion initialisation in one place. It is inert for every deployment without --tool-call-parser (guard false → zero behaviour change). For pure-diffusion setups the counter gets registered but never incremented, which matches upstream's flag-driven semantics (upstream also registers whenever the flag is set, regardless of whether a request ever reaches the parser). No provider-, engine-, or backend-specific code is touched.

Relation to other PRs

The insertion sits in the same function as #8209, but on disjoint lines — #8209 changes the engine-client/metrics-handler region, this adds a registration immediately after model_name is computed and before the dispatch block. I have kept the two patches non-overlapping on purpose; The registration sits above both early returns in omni_init_app_state (the duplex delegation and the pure-diffusion path), so every construction path gets the counters.

The four wiring-drift PRs (#8209, #8212, #8242, #8244) originally all appended their tests to the tail of tests/entrypoints/openai_api/test_api_server_guards.py, so any two of them conflicted as soon as one merged. Each now inserts its block at a distinct point in that file, and #8244's fingerprint call no longer shares this PR's anchor. Merging all five into one tree yields 85 collected tests, all passing, with ruff clean.

AI assistance disclosure

The change was developed with AI assistance (code, tests, and the analysis above). Per the repository's disclosure policy for AI-generated contributions: I have run the tests listed above locally and reviewed the patch and its interaction with upstream's init path; the commit carries a Signed-off-by trailer matching the author identity.

Alternative approaches

• Register the counter in omni directly rather than calling
  init_parser_metrics: the metric object and the module-level registry that
  record_tool_parser_invocation() checks both live in upstream's metrics module,
  so a private second registry would fork the metric.
• Move the registration into the parser itself so any entrypoint gets it:
  that changes upstream parser semantics and is out of scope here.
• Call upstream's init_app_state to inherit the call: it also builds the
  entire generate-side surface, which omni_init_app_state deliberately replaces.

Compatibility

No new flag, no new dependency, no request-path change: one registration at init,
under upstream's own guard (args.tool_call_parser is not None). /metrics gains
the counters upstream exposes for the same configuration; a server without a
tool-call parser is unchanged.

[vllm-omni-review-bot]

This PR appears to belong to: docs/design/module/error_contracts.md, docs/design/module/profiling.md, docs/design/module/entrypoints.md, docs/design/module/observability.md.

Module owners: @alex-jw-brooks @linyueqian @NickCao

Routing: @alex-jw-brooks via module of the changed files, module named in the PR description, CODEOWNERS; @linyueqian via module of the changed files, module named in the PR description, CODEOWNERS; @NickCao via module of the changed files, module named in the PR description, CODEOWNERS

@apex-mochen, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer.

Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment.

[vllm-omni-review-bot]

Omni ReviewBot triage note

Automated triage of commit cba4f6d23649 produced:

• Priority: high. Prompt maintainer attention is suggested.

These are automated triage suggestions only — the final decision belongs to the maintainers.

[vllm-omni-review-bot]

Omni ReviewBot routing record

Assigned Direct under experiment vllm-omni-strict-5050-20260829.

[vllm-omni-review-bot]

Omni ReviewBot review

PR description

Omni's shared FastAPI app-state builder now calls upstream init_parser_metrics when --tool-call-parser is set, using the same served model name already computed for the serving objects. The frozen diff matches that claim: a nine-line registration in omni_init_app_state plus a parametrized wiring test. Previously omni still handed the parser into chat, responses, and Anthropic handlers, but record_tool_parser_invocation() stayed a no-op because the Prometheus counter was never created. With the flag set, /metrics should now expose vllm:tool_call_parser_invocations_total on the omni serve path, matching non-omni vllm serve.

Change flow

flowchart TD
  A["[EXISTING] Omni serve with --tool-call-parser"]:::existing
  B["[CHANGED] omni_init_app_state in api_server.py"]:::changed
  C["[EXISTING] vllm.parser.metrics.init_parser_metrics"]:::existing
  D["[EXISTING] /metrics vllm:tool_call_parser_invocations_total"]:::existing
  E["[NEW] test_parser_metrics_registered_exactly_when_a_tool_parser_is_set"]:::new
  A --> B
  B --> C
  C --> D
  E --> B
classDef existing fill:#e5e7eb,stroke:#6b7280,color:#111827
classDef changed fill:#fef3c7,stroke:#d97706,color:#451a03,stroke-width:2px
classDef new fill:#dcfce7,stroke:#16a34a,color:#052e16,stroke-width:2px
classDef removed fill:#fee2e2,stroke:#dc2626,color:#450a0a,stroke-width:2px

Loading

CI at a953a67a2d4e (2026-09-27T20:13:21.661711+00:00): required check(s) blocking: buildkite/vllm-omni (missing), and -5 more.

No actionable findings.

[apex-mochen]

Author self-review (@apex-mochen), as the review bot asks.

What I checked

• Scope. 2 files; the source change is one guarded registration (+9/-0)
  placed immediately after model_name is computed, plus +84 of tests.
• Root cause, read from upstream rather than inferred.
  record_tool_parser_invocation() opens with if _tool_call_parser_invocations is None: return, and the counter is created only by init_parser_metrics(), which
  upstream calls from its own app-state builder under if args.tool_call_parser is not None. omni's omni_init_app_state has zero references to either name while
  handing args.tool_call_parser to six handlers (lines listed in the body), so
  the parser runs and the counter is silently dropped.
• Branch coverage. The only new branch is the is not None guard, parametrized
  over four cases: attribute absent, explicitly None (both → nothing registered),
  a parser name, and a parser name with a served alias (→ registered once with the
  served alias). The getattr default path is additionally exercised by the
  pre-existing test_omni_sleep_wakeup.py, which passes a partial args namespace —
  that test is why getattr was used instead of upstream's bare access.
• Label consistency. model_name is the same expression the surrounding code
  already uses for the serving objects, so the metric label cannot disagree with
  the model the server reports.
• Re-entry / duplicates. Registration goes through upstream's own
  init_parser_metrics, whose except ValueError fallback makes it idempotent, so
  duplicate registration behaves exactly as in upstream vllm serve.
• Cross-mode impact. The insertion is in the shared app-state builder, so
  duplex, multi-stage and pure-diffusion init all get it in one place; it is inert
  for any deployment without --tool-call-parser (guard false → no behaviour
  change), and for pure-diffusion it registers a counter that is never incremented,
  which matches upstream's flag-driven semantics.
• Negative control, re-run just now for this review. With
  api_server.py restored at file level to this PR's base bytes (md5
  7189287b…, asserted, init_parser_metrics absent from the file) and the test
  kept, -k parser_metrics_registered gives 2 failed, 2 passed: the two cases
  that require the registration fail ([overrides2-…], [overrides3-…]), and the
  two cases asserting that nothing is registered pass on both sides by
  construction — nothing registers either way, so only the positive cases can
  discriminate. Re-applying the head bytes (md5 6be274397d2af8feafeb280a71e34830)
  gives 4 passed.
• Test runs, local, CPU/WSL/vLLM 0.30.0. guards file 68 passed; duplex +
  diffusion-utils 62 passed; all four files 146 passed; ruff check and
  ruff format --check clean on both changed files.
• Stated limit. I could not start an omni server end-to-end here (no GPU), so
  the "curl /metrics" step in the body is the operator-facing symptom, not a log I
  captured; the reproduction I actually ran is the regression test.
• Disclosure. AI assistance was used to prepare this change; I reviewed every
  line and ran every command reported above myself.

[vllm-omni-review-bot]

Omni ReviewBot review

PR description

Omni’s omni_init_app_state now calls upstream init_parser_metrics() when --tool-call-parser is set, matching upstream init_app_state. Without that registration, parser invocations still run but record_tool_parser_invocation() no-ops, so /metrics never shows vllm:tool_call_parser_invocations_total. The change is a single guarded init call (with getattr so partial args namespaces used on early-return paths do not raise), plus a parametrized regression test that asserts registration happens only when a parser name is present and uses the served model alias for the metric label.

Change flow

flowchart TD
  A["[EXISTING] args.tool_call_parser / served model name"]:::existing
  B["[CHANGED] omni_init_app_state registers parser metrics"]:::changed
  C["[EXISTING] vllm.parser.metrics.init_parser_metrics"]:::existing
  D["[EXISTING] record_tool_parser_invocation can increment"]:::existing
  E["[EXISTING] /metrics exposes tool_call_parser_invocations_total"]:::existing
  F["[NEW] test_parser_metrics_registered_exactly_when_a_tool_parser_is_set"]:::new
  A --> B
  B --> C
  C --> D
  D --> E
  F --> B
  classDef existing fill:#e5e7eb,stroke:#6b7280,color:#111827
  classDef changed fill:#fef3c7,stroke:#d97706,color:#451a03,stroke-width:2px
  classDef new fill:#dcfce7,stroke:#16a34a,color:#052e16,stroke-width:2px
  classDef removed fill:#fee2e2,stroke:#dc2626,color:#450a0a,stroke-width:2px

Loading

CI at 8ea24324a020 (2026-09-30T22:31:59.182475+00:00): required check(s) blocking: buildkite/vllm-omni (missing).

No actionable findings.

---

_{🤖 This review was generated by InferMatrix Copilot, an open-source repo-maintenance agent for PR review, CI debugging and issue triage. Try it on your own repo, and ⭐ star it if it helped!}

[apex-mochen]

Hi @NickCao, software engineering student / open-source contributor. This registers the tool-call parser metrics in omni's init path, currently missing so they never emit. Code and tests ready; would appreciate a review.

[vllm-omni-review-bot]

Omni ReviewBot: no human activity for 7 days

@apex-mochen this pull request has had no human commit, comment or review since 2026-10-03. Please confirm the current plan and next step. The author or a maintainer decides whether to change the PR state.

To keep it moving, any one of these is enough: push an update, reply to the open blocker, or post the current plan and timeline.

[apex-mochen]

Thanks for the reminder! This PR is still active and ready for further review.

The fix addresses missing tool-call parser metrics registration in vLLM-Omni’s initialization paths.

I’d appreciate a maintainer review when available. I’m happy to address any feedback or add further tests if needed.

Thanks!

[apex-mochen]

Re-verified against current main (f4108d77e) and pushed one test-only fix to the branch — cba4f6d2.

The fix is still needed. omni_init_app_state never calls init_parser_metrics() (zero occurrences in omni's tree, against four parser-wiring sites), so record_tool_parser_invocation() early-returns and /metrics never exposes vllm:tool_call_parser_invocations_total.

The new test had drifted. test_parser_metrics_registered_exactly_when_a_tool_parser_is_set monkeypatches api_server.OpenAIServingRealtime, which no longer exists on main — the class was removed in the /v1/realtime rework (#8339). On current main that raises AttributeError during setup, so all four parametrized cases error before ever reaching the assertion. cba4f6d2 drops that one mock line. The other occurrence of the same line (in test_multistage_app_state_key_snapshot) is pre-existing, already gone on main, and is left untouched.

Verification, on the branch merged into current main:

• merge is clean and adds exactly +9 (registration) and +83 (test) lines;
• tests/entrypoints/openai_api/test_api_server_guards.py: 69 passed;
• negative control — delete the registration, keep the test: exactly 2 failed / 2 passed (the two tool_call_parser cases), so the test still measures the fix and not the harness;
• ruff check + ruff format --check (0.14.10): clean;
• the registration sits before the only early return on this path (the DuplexOmni branch), so every path out of omni_init_app_state is covered.

DCO is green on cba4f6d2 and the build/pre-commit jobs have re-run. @natureofnature — this is the same omni_init_app_state you refactored in #7646; would you mind taking a look when you get a chance?

[vllm-omni-review-bot]

Omni ReviewBot review

0 actionable finding(s).

CI at cba4f6d23649 (2026-10-10T12:40:52.361080+00:00): required check(s) blocking: buildkite/vllm-omni (missing), docs/readthedocs.org:vllm-omni (pending).

Full review analysis

PR description

When omni_init_app_state is given a configured tool-call parser, it now calls upstream init_parser_metrics with the same served model name the API already computed. That creates the parser counters before duplex, pure-diffusion, and multi-stage setup return, so later parser invocations can show up on /metrics. Servers with no parser attribute, or with the parser set to None, skip registration. A new parametrized guard test checks that call.

Change flow

flowchart LR
  flag["[EXISTING] args.tool_call_parser"]:::existing
  init["[CHANGED] omni_init_app_state"]:::changed
  register["[NEW] init_parser_metrics call"]:::new
  handlers["[EXISTING] duplex and chat handlers"]:::existing
  counter["[EXISTING] parser invocation counter"]:::existing
  test["[NEW] registration guard test"]:::new
  flag --> init
  init --> register
  register --> counter
  init --> handlers
  handlers --> counter
  test --> init
  classDef existing fill:#e5e7eb,stroke:#6b7280,color:#111827
  classDef changed fill:#fef3c7,stroke:#d97706,color:#451a03,stroke-width:2px
  classDef new fill:#dcfce7,stroke:#16a34a,color:#052e16,stroke-width:2px
  classDef removed fill:#fee2e2,stroke:#dc2626,color:#450a0a,stroke-width:2px

Loading

No actionable findings.

---

_{🤖 This review was generated by InferMatrix Copilot, an open-source repo-maintenance agent for PR review, CI debugging and issue triage. Try it on your own repo, and ⭐ star it if it helped!}

[vllm-omni-review-bot]

Omni ReviewBot: no human activity for 7 days

@apex-mochen this pull request has had no human commit, comment or review since 2026-10-10. Please confirm the current plan and next step. The author or a maintainer decides whether to change the PR state.

To keep it moving, any one of these is enough: push an update, reply to the open blocker, or post the current plan and timeline.

[vllm-omni-review-bot]

Omni ReviewBot: no human activity for 14 days

@apex-mochen this pull request has had no human commit, comment or review since 2026-10-10. Please consider marking this PR as draft until work can resume. The author or a maintainer decides whether to change the PR state.

To keep it moving, any one of these is enough: push an update, reply to the open blocker, or post the current plan and timeline.

[apex-mochen]

Closing this PR to free one of the five open-PR slots for a core feature. This is a valid bugfix with a clear root cause and tests; I am not rejecting it on technical grounds. I may reopen or resubmit it if maintainers want it after the core work lands.