perf(tui): single-pass token accounting on per-turn pressure paths - #5665
Closed
AdityaVG13 wants to merge 2 commits into
Closed
perf(tui): single-pass token accounting on per-turn pressure paths#5665AdityaVG13 wants to merge 2 commits into
AdityaVG13 wants to merge 2 commits into
Conversation
The per-turn metadata build and the compaction decision each re-walked the full transcript more than once per step. Make those walks single-pass with byte-identical behavior, and fix one cache-invalidation bug found along the way. compaction_decision_with_billed: the same pure estimator ran twice per step once pressure existed, once in the pressure gate and once in the prune projection. Compute it at most once and thread it to both consumers. Guard order (gate, prune projection, too-few-messages, retained-floor) is untouched and decisions are identical for all inputs. When provider-billed input alone proves pressure, the estimate walk is skipped entirely without bypassing the guards that keep a compaction loop from re-firing on every step. active_input_tokens_with_current_text: the per-turn metadata builder deep-cloned the entire message history just to append the composer text before estimating. Estimate the history in place and add the one-message delta arithmetically instead. The estimator inflates the summed byte count by a factor of 1.5 rounded up as a whole, so the delta is floor(own * 3 / 2) plus one exactly when the running byte sum is even and the composer text contributes an odd count. Proven against a clone-and-estimate reference over 80,000 parity combinations and pinned per case by a test that also covers replayed reasoning, the arm where a naive helper undercounts. Op::SyncSession: session restore assigned the restored message list directly to the session field, bypassing the messages_revision bump the token-estimate cache keys on, so a restored conversation could be priced from a stale estimate. Bump the revision and pin the behavior with a test. Verification: cargo fmt clean; compaction:: and engine preview, pressure, and restore tests 19 passed via remote rch lane; cargo check --all-targets clean with no warnings.
The streaming render ran the math transform over the full accumulated content on every chunk, before the incremental markdown cache that itself only renders deltas, so streamed turns paid an O(n) copy and scan per chunk with O(n^2) cumulative cost. render_latex_in_text now returns Cow<str>. One byte scan for the three opening delimiters ($, \(, \[) decides between borrowing the input untouched, the overwhelming case for streamed prose, and running the transform. Output is byte-identical either way: the full transform still re-verifies delimiters precisely, so the fast scan cannot create false negatives. Tests: no_math_is_borrowed_without_copy pins the borrowed path and that the \( form is not missed by the fast scan; test_inline_dollar pins the owned path on math input. Verification: cargo fmt clean; latex_render:: 18 passed via remote rch lane; cargo check --all-targets clean with no warnings.
Owner
|
Thank you, Aditya. I verified both patches in this PR are already on Landed commits: I’m closing this duplicate PR because the contribution is already shipped on |
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.
perf(tui): single-pass token accounting on per-turn pressure paths
The per-turn metadata build and the compaction decision each re-walked the full transcript more than once per step, and the streaming render re-processed the whole accumulated message on every chunk. This series makes those walks single-pass with byte-identical behavior, pins the equivalence with tests, and fixes one cache-invalidation bug found along the way.
<turn_meta>build per user or steer messageOne estimate per compaction decision
compaction_decision_with_billedcalled the same pure estimator twice per step once pressure existed: once in the pressure gate and once in the prune projection. It is now computed at most once and threaded to both consumers. Guard order (gate, prune projection, too-few-messages, retained-floor) is untouched and decisions are identical for all inputs. When provider-billed input alone proves pressure, the estimate walk is skipped entirely without bypassing the guards that keep a compaction loop from re-firing on every step.No full-transcript clone in the per-turn metadata build
The builder deep-cloned the entire message history just to append the composer text before estimating. It now estimates the history in place and adds the one-message delta arithmetically. The subtle part: the estimator inflates the summed byte count by a factor of 1.5 rounded up as a whole, so the delta is
floor(own * 3 / 2)plus one exactly when the running byte sum is even and the composer text contributes an odd count. This is proven against a clone-and-estimate reference over 80,000 parity combinations and pinned per case by a test that also covers replayed reasoning, the arm where a naive helper undercounts.Session restore invalidates the estimate cache
Session sync assigned the restored message list directly to the session field, bypassing the revision bump the token-estimate cache keys on, so a restored conversation could be priced from a stale estimate. The bump is restored and a test pins that the cache recomputes after a restore.
Zero-copy LaTeX pre-pass
The streaming render ran the math transform over the full accumulated content on every chunk, before the incremental markdown cache that itself only renders deltas. The transform now returns a
Cow<str>: one byte scan for the three opening delimiters decides between borrowing the input untouched, the overwhelming case for streamed prose, and running the transform. Output is byte-identical either way. A test pins the borrowed path and that the\(form is not missed by the fast scan.Testing
cargo fmtclean.compaction::andlatex_render::: 62 passed. Engine preview, pressure, and restore tests: 14 passed.cargo check --all-targets: clean, no warnings.