Skip to content

fix(agentx): preserve streaming liveness across profile cancellation - #45

Open
janbernloehr wants to merge 1 commit into
SemiAnalysisAI:masterfrom
janbernloehr:jbernloehr/fix-cancelled-stream-coverage
Open

janbernloehr wants to merge 1 commit into
SemiAnalysisAI:masterfrom
janbernloehr:jbernloehr/fix-cancelled-stream-coverage

Conversation

@janbernloehr

@janbernloehr janbernloehr commented Sep 22, 2026 •

Copy link
Copy Markdown

Summary

Prevent ProfileMetricCoverageError when a healthy, long-running streaming request is cancelled at the profiling boundary before it produces a completed metric record.

Retain actual content-arrival timestamps independently of request completion. Workers send rate-limited activity updates while a request is in flight and attach the exact last content timestamp to its credit return. The coverage gate accepts this evidence alongside its existing TTFT and inter-token-latency signals, retaining the scenario's 95% threshold.

This also covers forced phase completion when the cancellation drain times out before the worker returns its credit: activity already delivered to the Timing Manager remains available for validation.

Error and fix

Why the current gate can reject a healthy run

The gate introduced in this repository measures the latest TTFT and inter-token-latency observations in completed metric records against the configured profiling duration. PR #41 relaxed the required ratio from 98% to 95% to tolerate sparse tails, but it did not preserve activity from a request cancelled before completion.

The failure sequence is:

  1. A duration-based profiling phase reaches its send deadline while a long request is still streaming.
  2. The grace period expires and the worker is told to cancel the request.
  3. Cancelled credits do not produce the completed records used by the coverage calculation.
  4. The last completed request can predate the coverage cutoff even though the cancelled request continued producing content.
  5. Both record-based ratios fall below the threshold, and the run is rejected with a stalled-server diagnosis.

Increasing the quiet-tail allowance only moves this boundary. A sufficiently long final request can exceed any fixed allowance. Attaching activity only to the final credit return is also insufficient because the cancellation drain can time out and force completion before that return arrives.

Synthetic example

The following values illustrate the regression independently of any particular model, server, dataset, or deployment:

Observation Value
Configured profiling duration 1,800 seconds
Required coverage ratio 0.95
Latest completed-record TTFT 1,440 seconds after phase start: ratio 0.80
Latest completed-record ITL 1,620 seconds after phase start: ratio 0.90
Content from an unfinished stream 1,790 seconds after phase start
Request outcome Cancelled during shutdown; no completed metric record

Previously, this fails because neither 0.80 nor 0.90 reaches 0.95. With this change, streaming_content_ratio is approximately 0.9944, so the coverage check passes. The cancelled request still contributes no successful-request count, throughput, or latency samples.

A stream whose last content arrived at 1,709 seconds still fails. Cancellation, an open connection, and a positive in-flight count do not establish recent content activity.

Implementation

Worker observation. For a scenario that enables coverage validation, profiling workers continue using the existing SSE callback after the first meaningful response. The endpoint parser determines whether a chunk contains content. Its arrival timestamp is converted from the performance clock to wall-clock nanoseconds using a per-request clock offset. The observer records arrival time, not cancellation or credit-return time.

In-flight reporting. A new typed StreamingContent message carries the phase, concrete phase index, and content timestamp over the existing worker-to-router PUSH/PULL return channel. The first content observation is reported immediately; further reports are limited to one per second per request. These messages do not return credits, change request counts, or release concurrency slots.

Final observation. CreditContext retains the latest timestamp, and both normal and fallback credit-return paths include it in CreditReturn.last_streaming_content_ns. This preserves the exact final observation, including a chunk that arrived between periodic reports.

Phase accounting. The router forwards activity to the matching phase callback. The phase progress tracker retains the maximum timestamp, so older observations cannot move coverage backwards. Unknown or completed phases ignore activity updates. The timestamp flows through credit phase statistics into records phase statistics; aggregate statistics retain the maximum, while validation uses each concrete profiling phase's own statistics.

Validation and export. ProfileMetricDurationCoverage adds streaming_content_ratio, defaulting to zero. The Records Manager calculates it as:

clamp((last_content_timestamp - phase_start) / configured_duration, 0, 1)

Coverage passes when any of ttft_ratio, inter_token_latency_ratio, or streaming_content_ratio reaches required_ratio. Success and failure diagnostics include all three signals. The new ratio uses FiniteFloat and is bounded to [0, 1].

Behavior preserved

  • The inferencex-agentx-mvp threshold remains 0.95.
  • Cancelled requests remain excluded from successful-request accounting and latency distributions.
  • Role-only chunks, usage-only chunks, finish markers, keepalives, and [DONE] do not advance content coverage for the chat endpoint. Text and reasoning content do.
  • Warmup activity cannot satisfy a profiling phase's coverage requirement, and one profiling phase cannot rescue another.
  • First-token notifications still release prefill concurrency at most once per request, including when coverage observation continues afterward.
  • Scenarios without coverage validation retain the existing callback fast path.
  • Existing short-duration smoke-run and whole-run cancellation handling remain in place.

Scope and limitations

The coverage check establishes recent activity; it is not a continuous-availability or successful-completion guarantee. Content observed during the grace period is clamped to full coverage, consistent with the existing handling of completed-record timestamps beyond the configured window.

If a credit never returns, the gate uses the last delivered periodic observation. Because reports are limited to once per second, that observation can precede the exact final content timestamp by less than one second. No timestamp is fabricated when the worker has never reported content. A worker or communication failure that prevents all activity reports remains distinguishable from a healthy reported stream only through other diagnostics.

The additional work is endpoint parsing throughout streaming for coverage-enabled profiling requests, plus at most one activity message per second per active request. Other scenarios do not incur continuous parsing for this feature. No inference-server changes or GPU-specific behavior are required. Worker and Timing Manager processes should run the same harness revision because the typed return protocol gains a new message variant.

Validation

Deterministic reproduction of the missing-evidence failure

A prior CPU-only reproduction exercised the shipped ColumnStore and
MetricsAccumulator.profile_metric_duration_coverage at
754356e9,
using synthetic request records:

Input to the coverage calculation TTFT coverage ITL coverage Verdict at 95%
Completed records, with the cancelled straggler omitted 84.80% 92.40% Fail
Same records, plus the straggler's observed first-token timestamp 99.72% — Pass

The decisive change was retaining evidence from the unfinished request. The threshold and
configured duration stayed fixed. This establishes that discarding the straggler's observations
can cause the false positive without any inference-server failure. It validates the diagnosis and
proposed use of in-flight activity; it does not constitute an end-to-end run of this PR's patch.
The added record was an experimental input to isolate the cause, not the implementation proposed
here: this PR preserves activity separately from successful-request metrics.

Verification of this implementation

The regression tests carry streaming evidence through typed message serialization, the real
router and credit callback, phase progress statistics, the records tracker, and the coverage gate.
They establish the behaviors needed to fix the reproduced failure:

  • Recent content passes even when cancellation produces no completed metric record.
  • Already-delivered activity still passes when the credit never returns and the phase is forced
    to complete.
  • Stale or absent content still fails; cancellation alone cannot satisfy the gate.
  • Activity from another phase cannot rescue the failing phase, and completed-request counts and
    latency samples remain unchanged.

A real loopback HTTP/SSE stream also exercised AioHttpClient with the worker callback. It
produced rate-limited content reports before external task cancellation, without requiring a
completed record or credit return. This verifies that the transport supplies the evidence used by
the regression tests.

Validation covers the deterministic failure mechanism and the new activity-delivery path. The
patched harness has not been rerun against the original workload. Repository checks were also
run; the full suite's two failures reproduce on unchanged upstream.

Documentation

Updated the AgentX FAQ to explain cancelled-stream coverage, content filtering, and missing-return behavior. Updated the architecture and developer patterns to describe the activity-message path, phase scoping, callback contract, and unchanged accounting semantics.

Contribution checklist

  • Regression tests added.
  • User and developer documentation updated.
  • Applicable pre-commit hooks passed.
  • Commit signed off under the Developer Certificate of Origin.

Created with the assistance of Codex (GPT-6)


Note

Medium Risk
Changes AgentX run-validity logic and the worker/credit return protocol, but liveness evidence is scoped to coverage-only and does not alter successful-request or latency accounting.

Overview
Fixes false insufficient_profile_metric_coverage failures when a long streaming request is cancelled at the profiling boundary but was still producing content.

Coverage gate. Profiling phases can now pass the scenario’s 95% duration check via a third signal, streaming_content_ratio, alongside TTFT and inter-token latency. The ratio uses the latest parsed content timestamp (including cancelled credits with no metric record), clamped over the configured phase duration.

Worker → timing path. For scenarios with minimum_profile_metric_coverage_ratio, profiling workers keep the SSE callback active after first token, retain last_streaming_content_ns on the credit context, and emit rate-limited StreamingContent messages (≤1/s) on the existing return channel. CreditReturn carries the exact final timestamp; phase progress tracks the max and flows through records stats into RecordsManager validation. Activity messages do not return credits, release slots, or add latency/throughput samples; prefill release still happens once via FirstToken.

Docs and tests cover AgentX FAQ behavior, architecture/patterns, and edge cases (missing returns, phase isolation, non-content chunks).

Reviewed by Cursor Bugbot for commit 70af1f5. Bugbot is set up for automated code reviews on this repo. Configure here.

Signed-off-by: Jan Bernlöhr <jan@bernloehrs.de>
@github-actions

Copy link
Copy Markdown

Try out this PR

Quick install:

pip install --upgrade --force-reinstall git+https://github.com/ai-dynamo/aiperf.git@70af1f538695fb51f612710ce55a728f389a4ead

Recommended with virtual environment (using uv):

uv venv --python 3.12 && source .venv/bin/activate
uv pip install --upgrade --force-reinstall git+https://github.com/ai-dynamo/aiperf.git@70af1f538695fb51f612710ce55a728f389a4ead

Last updated for commit: 70af1f5 • Browse code

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant