perf(webui): skip replay repair for sidecars already proven clean (3.8x on concurrent loads) - #7282
Open
ruizanthony wants to merge 19 commits into
Open
Conversation
…lay dedup Gate certification on 661a801 reproduced a display/context divergence: the model-context path _dedupe_replayed_context_messages still compared rows with the weak _message_replay_key (ignores reasoning/id/turn token) while the display projection had become payload-strict. Persisted rows carry the request-local _active_turn_token that _sanitize_messages_for_agent strips from the history the Agent replays, so a repeated prompt either duplicated the historical user row in model context or dropped the current exchange. Introduce _canonical_replay_digest — the strict canonical payload digest tolerant only of _active_turn_token — and use it for replay/prefix comparison and for the destructive prefix reductions on the model-context path, aligning it with the display path without weakening either. Keep conflicting-turn-token rows distinct in _deduplicate_context_messages so two durable turns sharing a visible projection are never collapsed. Adds 3 regression tests (RED on 661a801, GREEN with this fix) driving the real _settle_result_messages caller sequence.
Replay repair runs on every session LOAD, not just on save, and it dominates cold-load cost. It re-serializes every assistant message to canonical JSON and SHA-256s it, for the session AND for every compaction ancestor walked by the lineage — so a chat with 4 ancestors pays it five times per tab refresh. A py-spy profile taken under 6 concurrent loads on a live deployment put `_canonical_message_digest` as the #1 self-time frame. Because the work happens under the GIL it does not overlap between tabs: one load took 0.55s while six concurrent loads took 10.83s for the slowest (19.6x). The work is also almost always for nothing. Sampling the 60 most recent sidecars, repair changed nothing in 60 of 60 cases — 11.8s of pure waste. A file only needs repairing once; a clean file stays clean until it is rewritten. Memoize the NEGATIVE verdict only, keyed by the sha256 of the exact file bytes (already computed by the primary load path for revision tracking). A hit means "these exact bytes were proven to need no repair" and skips the pipeline. Any write changes the digest and therefore misses the cache. Measured on 6 real sidecars, same machine, warm: single load (median) 136ms -> 46ms (2.9x) single load (6 sessions) 875ms -> 302ms (2.9x) 6 concurrent loads (wall) 1054ms -> 279ms (3.8x) 6 concurrent loads (slowest) 1053ms -> 277ms (3.8x) Safety. Skipping repair is equivalent to running it because the collapse helpers never mutate their input: they build new lists and return (result, changed), returning the input untouched when changed is False. A test pins that property, so a future helper that starts mutating in place fails the suite instead of silently serving unrepaired sessions. A positive verdict is deliberately not cached: a file needing repair gets rewritten, so caching it would key on bytes that no longer exist. The cache is bounded (512 entries, LRU) and a missing digest forces the full pipeline.
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.
Problem
Replay repair runs on every session LOAD, not just on save:
Session.load()—api/models.py_load_session_from_path()—api/models.pyIt re-serializes every assistant message to canonical JSON and SHA-256s it
(
_canonical_message_digest), for the session and for every compactionancestor walked by the lineage. A chat with 4 ancestors pays it five times per
tab refresh.
It is the dominant cost of a cold load
Measured on a live deployment, per load:
A
py-spyprofile taken under 6 concurrent loads put_canonical_message_digestas the #1 self-time frame. The work runs under the GIL, so it does not
overlap between tabs:
And it is almost always for nothing
Sampling the 60 most recently written sidecars on that deployment:
That is 11.8 s of CPU spent to produce no change at all. A file only needs
repairing once; a clean file stays clean until it is rewritten.
Fix
Memoize the negative verdict only, keyed by the sha256 of the exact file
bytes — the same digest the primary load path already computes for revision
tracking, so the key is free there and ~10 ms on a 5 MB sidecar in the secondary
path (versus the ~485 ms of repair it skips).
A hit means "these exact bytes were already proven to need no repair". Any
write changes the digest and therefore misses the cache.
Results
Measured on 6 real sidecars, same machine, warm,
_load_session_from_path:The concurrent gain is larger than the single-load gain because the removed work
was serialized under the GIL: it was multiplying across tabs.
Why this is safe
Skipping repair is equivalent to running it. The collapse helpers
(
_collapse_adjacent_duplicate_partials,_collapse_duplicate_incomplete_message_ids,_collapse_adjacent_exact_assistant_replays,_collapse_duplicate_durable_empty_assistant_replays) never mutate their input:they build new lists and return
(result, changed), returning the inputuntouched when
changedisFalse. A test pins that property, so a futurehelper that starts mutating in place fails the suite rather than silently
serving unrepaired sessions.
A positive verdict is deliberately never cached. A file that needs repair
gets rewritten, so caching "needs repair" would key on bytes that no longer
exist. Only "clean" is remembered — the conservative direction: a stale miss
costs CPU, never correctness.
Bounded and fail-safe. 512 entries, LRU eviction, and a missing/
Nonedigest forces the full pipeline.
Tests
tests/test_replay_repair_clean_cache.py— 8 tests, all passing:memoized;
Existing suites exercising the modified path also pass:
test_merge_backfill_perf_optimization.py,test_offline_replay_compactor.py(65 passed together with the new file).
On the broader session/compaction/lineage/sidecar selection (30 files), failures
are identical before and after (13 failed / 350 passed in both runs,
verified by diffing sorted
FAILEDlists) — they are pre-existing on thischeckout and unrelated to this change.
Note on the base branch. The repair pipeline this optimizes
(
_repair_session_message_projections) is introduced by #7032, so this branchis stacked on it rather than on
master. Review #7032 first; the diff here islimited to
api/models.py(+89/-4) and the new test file.