fix(tps): hold live rate across tools and steps - #15
Merged
Merged
Conversation
The live estimate kept decaying after the stream-end event while tools ran, and step settlement swapped the displayed value for the run-wide average. The tracker now captures the live rate at the stream-end boundary, falls back to the final content boundary for hosts that publish no such event, clamps the live clock at that boundary, and reopens the calculation when new bytes arrive after it. Settlement skips a boundary older than the newest sample, since those bytes would divide by the 250 ms floor and inflate the hold. Token and duration accounting and the frozen run average are unchanged; only the running display and the refresh timer differ.
touch() does recreate a stopped interval, and the following flush clears it again, so the old wording ("repaint through the dirty flag instead of restarting the timer") described the opposite of what happens. Lifecycle events schedule exactly one final repaint and do not keep the timer running.
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
1 participant
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.
While a run streams, the label showed the live estimate only until
session.step.endedfired. Settlement cleared the active step, so from that point the label showed the run-wide average instead, and between steps or during tool execution the number dropped hard (the probe behind this change read ~80 t/s falling to ~6 t/s). The live estimate also decayed through its 1.5 s stale tail aftersession.step.streamedwhile the model was idle.The tracker now holds the live estimate across tool execution and between steps. It captures the rate at the
session.step.streamedboundary, falls back to the final content boundary for hosts that publish no such event, and clamps the live clock at that boundary so nothing decays while tools run. New observable bytes after a boundary reopen the calculation, so a resumed attempt is not stuck behind an old one. The frozen value for a completed run, token accounting, and duration accounting are unchanged.One guard is worth a look during review: settlement skips a capture when the boundary predates the newest sample. Without it, a host that only publishes content boundaries can divide bytes arriving after the boundary by the 250 ms duration floor and show a wildly inflated hold (the test pins 1600 t/s against the expected 80).
This is a display and refresh-timer change only. The refresh timer now stops at the stream boundary, and lifecycle events repaint once through the dirty flag rather than leaving an interval running.
Also updated: README and
docs/development.mddescribe the held rate and the sub-agent case, and a docs line that claimed the timer was never restarted has been corrected.Validation:
bun test81 pass / 0 fail,bunx tsc --noEmitclean. The engine here is the same code the composer-side plugin was fixed against, so the 12 new tracker tests mirror that suite.