Repository navigation
fix(pylon): judge canary runaway by exact usage, not character estimates - #2315
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe canary now separates estimated token counts from exact usage. Exact usage above the threshold produces ChangesCanary threshold behavior
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to No actionable issue remains from this review; the change is mergeable after normal checks. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation Issue Resolution Add a confidence margin or equivalent rule for estimates when exact usage is absent. Allow a small estimate overshoot, while retaining runaway detection for estimates clearly beyond the cap. Update the event-sequence tests to cover both cases. ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
🛡️ CodeQL Analysis🚨 Found 5 issue(s) Severity Breakdown:
📋 Top Issues🔗 View full details in Security tab 🕐 Last updated: 2026-10-06 13:59:01 UTC | Commit: b106ac0 |
|
We should have 3 cases:
we shouldnt have the possible conflation on the timeout case |
|
Done in c1a9c7e. Completed with usage: runaway if exact usage is over the cap. Completed without usage: runaway if the estimate is over the cap. Timeout, stream error, or no [DONE]: the normal error, never runaway. Exact usage over the cap mid-stream still fails immediately, since it is already exact. |
The canary sets max_tokens to the runaway threshold and compared the character-based output estimate to that threshold on every SSE message. Reasoning models that run to the cap produced estimates slightly above it before the exact usage chunk arrived, so bounded responses failed the canary and demoted the model. The canary verdict now has three cases: - A stream that completes with usage fails as runaway only if the accepted exact usage is above the cap. Exact usage above the cap on a non-terminal event still fails immediately. - A stream that completes without usage fails as runaway only if the estimate is above the cap. - A timeout, read error, failure event, or stream without [DONE] fails with its normal error, never runaway. The SSE facts gain a done_sentinel flag because the shared parser also reports response.completed as complete, and the canary requires [DONE]. Closes #2314 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: jcameron <jcameron@nvidia.com>
e68beee to
a90b068
Compare
|
🎉 This PR is included in src/libraries/rust/stargate/v0.23.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
4 participants
TL;DR
Pylon canaries no longer fail as runaway generation when a reasoning model stops exactly at the canary
max_tokenscap. Exact usage decides the verdict. A character estimate is judged only when a completed stream reports no usage.Additional Details (optional for docs, build, test, refactor, ci, chore, style, and revert PRs)
Why:
send_canary_requestsendsmax_tokens = --canary-max-generation-threshold(default 237) and checkedobserved_tokens > thresholdafter every SSE message. Before the final usage chunk,observed_tokensis theOutputTokenParsercharacter estimate (ASCII / 4 plus one per non-ASCII character). Reasoning models with thinking enabled by default run to the cap on the1+1=prompt, so small estimate errors push the estimate to 238-240. The canary failed before reading the exact usage chunk (which reported exactly 237), demoting the model and causing 503s for callers while the backend was healthy.What changed in
crates/pylon-lib/src/bringup/upstream.rs:[DONE]sentinel to complete the canary.sse_message_stream.rsgains adone_sentinelfact because the shared parser also reportsresponse.completedas complete.RunawayGenerationif the accepted exact usage is above the cap, even if estimated output follows it.RunawayGenerationif the estimate is above the cap.[DONE]: the normal timeout or invalid-response error, never runaway. The one exception is exact usage above the cap on a non-terminal event, which fails immediately because the count is already exact. An estimate never fails the canary mid-stream.Also: the
--canary-max-generation-thresholdhelp text now says it is the canarymax_tokensand that exact usage overrides the estimate.Observability: no new logs, spans, or metrics. Fewer false failure results in the existing canary result metric.
For the Reviewer
Focus on the verdict logic in
crates/pylon-lib/src/bringup/upstream.rsand the event-sequence tests incrates/pylon-lib/src/bringup.rs. The three-case verdict follows review feedback. Two Codex critical reviews were run and their findings addressed.For QA (optional for docs, build, test, refactor, ci, chore, style, and revert PRs)
From
src/libraries/rust/stargate:cargo test --locked -p pylon-lib --lib: 548 passed. New event-sequence tests cover split and same-event overshoot settled by exact usage, estimated output after exact usage, regressed usage being ignored, exact usage above the cap (at completion and before the stream ends), a failure event carrying usage above the cap, no-usage streams at and above the cap, stall, EOF, or a read error after an overshoot staying a timeout or invalid response, and a completion event without[DONE]being invalid. The split-overshoot case failed before the fix withRunawayGeneration.cargo test --locked -p pylon: passed.cargo clippy --locked -p pylon-lib -p pylon --all-targets -- -D warnings: clean.cargo fmt -p pylon-lib -p pylon -- --check: clean.QA: not needed beyond CI. A live check against a reasoning model with default thinking enabled would confirm canaries stay healthy.
Issues
Closes #2314
Checklist
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Documentation