fix(server): record streamed chat completions in Metrics

[inureyes]

Summary

The streaming branch of POST /v1/chat/completions never called Metrics::record_request, so requests_total and completion_tokens_total (and the prompt-token and generation-time totals) only moved for non-streaming chat. The WebUI chat always streams, so the Activity tiles read 0, and mlxcel_requests_total undercounted every streaming client.

• stream_chat_completion records the request once, right after slot.finish, using the result's own counts. It does not depend on stream_options.include_usage. Err records nothing, as on the non-streaming path.
• Streams the client cancelled are not counted. That covers a disconnect without X-Conversation-Id, DELETE /v1/stream, and session replacement. After an abort the worker still returns Ok with the partial count, so the route checks the cancellation flag. The flag is now cloned before it moves into the provider call. A resumable stream whose client only disconnects keeps generating and is counted.
• The mlxcel_requests_total and mlxcel_completion_tokens_total HELP lines, the two counter_source reasons (and runtime.ready-shared.json, which pins them) and docs/webui/runtime.md now state these semantics. Metric names, values and both response shapes are unchanged.

The scope is chat completions only. Streaming /v1/completions and native /completion still never record, and /v1/messages and /v1/responses streams still record even when cancelled. That is why the HELP line says "chat completion streams". Both are left for follow-ups.

Tests

New tests in server::routes::stream_route_tests::metrics_tests, a child module of stream_route_tests that reuses its helpers. stream_route_tests.rs was already 698 lines. scripted_app_with_state also returns the AppState, and scripted_app delegates to it. The cancelled and resumable cases wait for the generation task to exit before asserting, because its completion-control registration drops after the record point. Without that wait, a zero count would prove nothing.

• completed_stream_records_one_request_with_its_counts: 3 tokens, [DONE], then +1 request, +3 completion tokens, still 1 after task exit.
• client_cancelled_plain_stream_is_not_recorded: disconnect, flag flips, finish(), task exits, requests_total == 0.
• resumable_stream_is_recorded_after_a_client_disconnect: the flag stays clear, and the request counts with all 3 tokens.
• metrics_endpoint_reports_a_completed_stream: whole-line mlxcel_requests_total 1 and mlxcel_completion_tokens_total 3.
• activity_projection_counts_a_completed_stream: runtime_snapshot reports completed_requests_total 1.0 and completion_tokens_total 3.0. It is gated on the webui feature, like server::webui itself, so the --no-default-features OpenXLA compile job still builds the file.

Revert check

Arm 1 removes the record block from stream_chat_completion and runs server::routes::stream_route_tests::metrics_tests: 1 passed, 4 failed. client_cancelled_plain_stream_is_not_recorded passes here, as expected, because this arm records nothing at all.

completed_stream_records_one_request_with_its_counts
  panicked at src/server/routes/stream_route_metrics_tests.rs:83:5:
  assertion `left == right` failed
    left: 0
   right: 1
resumable_stream_is_recorded_after_a_client_disconnect
  panicked at src/server/routes/stream_route_metrics_tests.rs:165:5:
  assertion `left == right` failed
    left: 0
   right: 1
metrics_endpoint_reports_a_completed_stream
  panicked at src/server/routes/stream_route_metrics_tests.rs:196:9:
  /metrics must print "mlxcel_requests_total 1":
  (scrape body) mlxcel_requests_total 0 / mlxcel_completion_tokens_total 0
activity_projection_counts_a_completed_stream
  panicked at src/server/routes/stream_route_metrics_tests.rs:233:5:
  assertion `left == right` failed
    left: Some(0.0)
   right: Some(1.0)

Arm 2 keeps the record and drops only the !cancelled condition: 4 passed, 1 failed.

client_cancelled_plain_stream_is_not_recorded
  panicked at src/server/routes/stream_route_metrics_tests.rs:126:5:
  assertion `left == right` failed: a client-cancelled stream must not count as a completed request
    left: 1
   right: 0

With the fix restored, all 14 pass. Neither reverted state was committed. The review and security passes re-ran the arms independently and got the same split (arm 1: 1 passed, 4 failed; arm 2: 4 passed, 1 failed).

Validation

All commands use cargo test --profile test-fast --features metal,accelerate -p mlxcel --lib <path>.

• server::routes::stream_route_tests: 14 passed, 0 failed (9 existing plus 5 new).
• server::webui::runtime: 6 passed, 0 failed. This includes ready_projection_matches_whole_canonical_fixture, which pins the new counter_source strings.
• server::routes::metrics: 6 passed, 0 failed.
• cargo clippy -p mlxcel --lib --tests --features metal,accelerate -- -D warnings and cargo fmt --all -- --check are both clean.
• The full workspace suite was not run in this unit.

Real-binary verification

Run by the epic #1910 orchestrator on M1 Ultra (macOS 27.0), mlxcel-server built from this PR head (414d45dc) under --profile test-fast --features metal,accelerate, started as mlxcel-server --webui --models-dir models/mlx --api-key-file <f> --no-models-autoload --settings --props --metrics --slots against the 224-entry local store, with qwen3-0.6b-4bit loaded through POST /ui-api/v1/model-actions.

• One stream: true (with stream_options.include_usage) and one stream: false /v1/chat/completions request, max_tokens: 20, temperature: 0: both responses reported usage.completion_tokens = 9. GET /ui-api/v1/runtime?model_id=<id> then reported completed_requests_total = 2 and completion_tokens_total = 18, matching decode_tokens_total = 18. On 0ef0a1a4 the same pair gave completed_requests_total = 1 (issue body).
• /metrics?model=qwen3-0.6b-4bit printed mlxcel_requests_total 2 and mlxcel_completion_tokens_total 18. In router mode the scrape needs the model query parameter; without it the server answers 400 model name is missing from the request, which is existing behavior.
• Fresh server, one WebUI streaming chat driven with Playwright (Chromium, 1440x900) through the bundled UI: the Activity summary for the model read Total completed requests 1 requests and Total completion tokens 5 tokens.
• cargo test --profile test-fast --features metal,accelerate -p mlxcel --lib server:: on this head: 3224 passed, 0 failed, 13 ignored.

Closes #1911

[inureyes]

Implementation Review Summary

Intent

Record every finished streaming /v1/chat/completions request in Metrics once, leaving out streams the client cancelled, so the Activity tiles and mlxcel_requests_total stop undercounting.

Findings Addressed

• No CRITICAL, HIGH or MEDIUM findings, so nothing on the branch was changed. The flag the route reads is the one the SSE sender sets when a client disconnects from a non-resumable stream, and the one StreamSession::cancel sets for DELETE /v1/stream and for session replacement. The streaming and non-streaming paths share no recording code, so no request is counted twice.

Remaining Items

• "completed generation requests" is not exact for other routes. Cancelled /v1/messages and /v1/responses streams still count, and streaming /v1/completions and /completion never count. That route behavior predates this PR and is out of scope. No follow-up issue exists yet. (LOW)
• The HELP lines for mlxcel_prompt_tokens_total and mlxcel_generation_time_seconds_total still read "processed" and "total". (LOW)
• The DELETE /v1/stream exclusion is tested only indirectly, by the existing delete_cancels_the_generation_and_evicts_the_session together with the new flag test. (LOW)

Verification

• All stated requirements implemented (real-binary acceptance pending orchestrator verification)
• No placeholder/mock code remaining
• Integrated into project code flow (router mode reads the same per-model AppState)
• Project conventions followed
• Existing modules reused where applicable
• No unintended structural changes
• Tests pass: stream_route_tests 14/14, webui::runtime 6/6, routes::metrics 6/6, clippy and fmt clean

I reran the revert check on 30358e82. Without the record block: 1 passed, 4 failed. Without the !cancelled condition: 4 passed, and client_cancelled_plain_stream_is_not_recorded failed (left 1, right 0). So the negative test asserts after the route has made its record decision, not before it.

[inureyes]

Security and performance review

No CRITICAL, HIGH or MEDIUM findings, so no fix commits were pushed.

• Counter abuse: the record runs once per generation task, and stream_chat_completion is only reached from the chat handler, so nothing double counts. Suppression needs a cancellation, and the session paths that set it (DELETE /v1/stream, replacement) are keyed by (API key, conversation id), so one key cannot cancel or uncount another key's stream.
• Hot path: no per-token cost. The change adds one Arc clone per request and four relaxed atomic adds per completed stream. Nothing counts references on the token.
• Exposure: only static HELP and reason strings changed. The /metrics and /ui-api/v1/runtime shapes are unchanged, and the canonical fixture test pins the latter.
• Revert check: dropping only the !cancelled condition fails client_cancelled_plain_stream_is_not_recorded (left 1, right 0). Restored and not committed.

LOW, not fixed:

• The HELP line says "completed generation requests", but streaming /v1/completions and native /completion still never record. That gap predates this PR and is out of scope here.
• Aborted streams' prefill and partial decode no longer show in mlxcel_prompt_tokens_total or mlxcel_completion_tokens_total (issue fix(server): record streaming chat completions in Metrics so Activity stops reporting zero #1911 chose this). They still show in the decode counters. Nothing reads these counters for quotas or admission.
• With authentication disabled, every client shares one stream owner. Anyone who knows a conversation id can cancel that stream, and the cancelled stream is now also left out of the counts. This is b10621 behavior and was already documented.

Gates: stream_route_tests 14/14, webui::runtime 6/6, routes::metrics 6/6, clippy and fmt clean.

[inureyes]

PR finalization

Checked test coverage, documentation, and lint/format on top of 30358e82; pr-reviewer and pr-security-checker had already found no CRITICAL/HIGH/MEDIUM issues.

Tests

The 5 tests in server::routes::stream_route_tests::metrics_tests cover the issue's acceptance cases (completed stream, client-cancelled stream, resumable stream after disconnect, /metrics lines, the runtime projection). No new test added: the Err(_) branch and a zero-token request both fall out of the existing generic code path, and the scripted-stream harness has no way to script a generation error, so adding that capability here would be disproportionate.

Documentation

docs/webui/runtime.md, the two HELP lines in metrics.rs, and the two counter_source reasons in runtime.rs (plus the fixture pinning them) match the brief; docs/webui/ has no runtime.ko.md, so no translation file was added.

Bug found and fixed

activity_projection_counts_a_completed_stream called crate::server::webui::runtime::runtime_snapshot, but server::webui is #[cfg(feature = "webui")]. The xla-compile CI job builds with --no-default-features --features xla-diagnostics, which drops webui, so the branch failed to compile there (E0433: cannot find webui in server). Fixed in 414d45d by gating that one test the same way, matching the pattern already used throughout server/app.rs.

Validation

• cargo test --profile test-fast --features metal,accelerate -p mlxcel --lib server::routes::stream_route_tests: 14 passed, 0 failed.
• cargo test --profile test-fast --features metal,accelerate -p mlxcel --lib server::webui::runtime: 6 passed, 0 failed.
• cargo test --profile test-fast --features metal,accelerate -p mlxcel --lib server::routes::metrics: 6 passed, 0 failed.
• cargo clippy -p mlxcel --lib --tests --features metal,accelerate -- -D warnings: clean.
• cargo fmt --all -- --check: clean.

All four re-ran clean after the fix. The CI WebUI installed artifact job is failing on an unrelated real-GPU Activity performance gate (median_decode_degradation_percent: 3.19, status investigate) on the self-hosted GB10 runner; that is a real-binary performance measurement, not something this metrics-counting change plausibly causes, and is left for orchestrator verification per the brief.