fix(agentx): preserve streaming liveness across profile cancellation - #45
Open
janbernloehr wants to merge 1 commit into
Open
janbernloehr wants to merge 1 commit into
janbernloehr wants to merge 1 commit into
Conversation
Signed-off-by: Jan Bernlöhr <jan@bernloehrs.de>
Try out this PRQuick install: pip install --upgrade --force-reinstall git+https://github.com/ai-dynamo/aiperf.git@70af1f538695fb51f612710ce55a728f389a4eadRecommended 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@70af1f538695fb51f612710ce55a728f389a4eadLast updated for commit: |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Prevent
ProfileMetricCoverageErrorwhen 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:
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:
Previously, this fails because neither 0.80 nor 0.90 reaches 0.95. With this change,
streaming_content_ratiois 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
StreamingContentmessage 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.
CreditContextretains the latest timestamp, and both normal and fallback credit-return paths include it inCreditReturn.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.
ProfileMetricDurationCoverageaddsstreaming_content_ratio, defaulting to zero. The Records Manager calculates it as:Coverage passes when any of
ttft_ratio,inter_token_latency_ratio, orstreaming_content_ratioreachesrequired_ratio. Success and failure diagnostics include all three signals. The new ratio usesFiniteFloatand is bounded to [0, 1].Behavior preserved
inferencex-agentx-mvpthreshold remains 0.95.[DONE]do not advance content coverage for the chat endpoint. Text and reasoning content do.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
ColumnStoreandMetricsAccumulator.profile_metric_duration_coverageat754356e9,using synthetic request records:
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:
to complete.
latency samples remain unchanged.
A real loopback HTTP/SSE stream also exercised
AioHttpClientwith the worker callback. Itproduced 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
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_coveragefailures 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, retainlast_streaming_content_nson the credit context, and emit rate-limitedStreamingContentmessages (≤1/s) on the existing return channel.CreditReturncarries the exact final timestamp; phase progress tracks the max and flows through records stats intoRecordsManagervalidation. Activity messages do not return credits, release slots, or add latency/throughput samples; prefill release still happens once viaFirstToken.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.