Live Stream: Hard prune post-compression tool context - #5667
franksong2702 wants to merge 4 commits into
Conversation
|
Addressed Greptile’s residual-budget note in |
4224b60 to
825ad2e
Compare
🔬 Gate certification — RED ⛔ (post-compression tool-result prune is NOT idempotent — repeated compression passes re-summarize already-pruned markers — 1 SILENT)Certified head: What I ran (rebased worktree
|
| Gate | Result |
|---|---|
| Rebase onto current master | ✅ git apply clean |
| Codex (reproduce) | SHIP-WITH-FIXES — 1 SILENT (idempotence, probe-verified) |
| Full pytest suite | ✅ 12198 passed, 0 failed |
| PR compression tests | ✅ 56/56 |
Findings
✅ Core prune is safe (Codex-verified): _hard_prune_post_compression_tool_results operates ONLY on role=='tool' messages, truncating content to a token budget with a [WebUI compressed-context budget: omitted N chars] marker; uses copy.deepcopy(list(messages)) so it never mutates the live/persisted context; and it does NOT create orphaned tool_call↔tool_result pairs (Codex verified the send-path sanitization preserves paired assistant/tool IDs and this change drops no messages).
⛔ SILENT (Codex, probe-verified) — the prune is NOT idempotent (api/streaming.py:4290): re-running the prune over an already-pruned context (repeated compression passes in a long tool-heavy session) re-summarizes the already-truncated tool results. Codex's direct helper probe: after the first prune, a second prune over many pruned tool rows changed 39 rows, replacing the original markers with new "omitted 157 chars" markers → the marker text gets progressively rewritten/corrupted and token accounting drifts across compression cycles. Fix: detect an existing [WebUI compressed-context budget: marker before summarizing (line ~4298) and leave that tool content unchanged while still counting its rough tokens; add a regression asserting that pruning an already-pruned context twice returns identical contents.
Recommendation to the next agent / author
RED — gate-fail/changes-requested (1 SILENT): make _hard_prune_post_compression_tool_results() idempotent — skip re-summarizing any tool result whose content already contains the [WebUI compressed-context budget: marker (still account its rough tokens toward the budget), + a "prune twice = identical" regression. The core design is correct and safe (no orphaning, no live-mutation); the only gap is that repeated compression passes must not re-rewrite already-pruned markers. concept 4/5 (sound + safe prune, one idempotence gap). Author @franksong2702 (T1). crit=3, compression data-correctness. (Gate value: Codex proved non-idempotence with a direct double-prune probe — the suite's single-prune test can't catch a "prune the already-pruned" cycle that only happens across multiple compressions in a long tool-heavy session.)
Gate-certifier layer (warm-up → gate → release). I do not merge/tag/deploy. Core prune safe (tool-result-only, deepcopy no-mutation, no orphaned pairs — Codex verified) BUT not idempotent: re-pruning already-pruned tool rows rewrites the markers (Codex double-prune probe changed 39 rows). Fix: guard on existing [WebUI compressed-context budget: marker + count-only, + prune-twice-identical regression. Suite green 12198 + 56 compression tests (single-prune only). Backend (no Fable). Cert valid for sha:825ad2ea.
825ad2e to
8d7b539
Compare
|
Addressed the gate idempotence finding in |
🔬 Gate certification — RED ⛔ (round 2: idempotence fixed, but the already-pruned detection is a bare SUBSTRING check → a real oversized tool result containing the marker string bypasses pruning — 1 SILENT)Certified head: What I ran (rebased worktree
|
| Gate | Result |
|---|---|
| Rebase onto current master | ✅ git apply clean |
| Codex (re-verify idempotence) | SHIP-WITH-FIXES — 1 SILENT (marker-collision, verified) |
| Full pytest suite | ✅ 12199 passed, 0 failed |
| Compression tests (incl. new idempotence regression) | ✅ 57/57 |
Findings
✅ Round-1 idempotence FIXED: extracted _POST_COMPRESSION_TOOL_RESULT_MARKER = "[WebUI compressed-context budget:"; the prune now skips rows already containing the marker (still counts their rough tokens); new test_post_compression_context_hard_prune_is_idempotent() asserts pruning twice yields identical content. The core prune remains safe (tool-result-only, deepcopy no-mutation, no orphaned pairs).
⛔ SILENT (Codex, api/streaming.py:4284) — marker-collision bypasses the prune: _is_compressed_context_tool_result_summary() uses a bare substring check (MARKER in text). A genuine oversized tool result whose original content happens to contain the literal [WebUI compressed-context budget: string is treated as already-pruned → skipped → the oversized raw output stays in model-facing context. Narrow (requires the tool output to contain that WebUI-internal string), but it's a real correctness hole — a tool that echoes/quotes the marker (e.g. a grep over the codebase, or a diagnostic dumping prior context) would evade pruning. Fix (Codex): don't detect via substring — mark generated replacements with private metadata (e.g. a _webui_pruned=True flag on the message dict) OR match the exact generated summary-note shape (anchored, not in); + a collision regression test (a raw result containing the marker string must still be pruned).
Recommendation to the next agent / author
RED — gate-fail/changes-requested (1 SILENT): replace the substring already-pruned check with a robust marker — set a private flag on the message dict when the prune generates a replacement (and check that flag), or match the exact anchored summary shape — so a real tool result that merely contains the marker string isn't mistaken for already-pruned; add a collision regression. The idempotence fix itself is correct; this is a hardening of HOW "already pruned" is detected (metadata > substring). concept 4/5 (idempotence done right; the detection mechanism needs metadata not substring). Author @franksong2702 (T1). crit=3, compression data-correctness. (Gate value: detecting "did I already transform this?" by substring-matching your own output marker is fragile — real content can collide with the marker; use out-of-band metadata. Codex flagged it right after the idempotence fix introduced the substring check.)
_Gate-certifier layer (warm-up → gate → release). I do not merge/tag/deploy. Round-1 idempotence fixed (marker-skip + prune-twice-identical regression) BUT is_compressed_context_tool_result_summary is a bare substring check → a real oversized tool result containing [WebUI compressed-context budget: bypasses pruning (SILENT, streaming.py:4284). Fix: private-metadata flag or anchored-shape match, not substring; + collision regression. Core prune still safe. Suite green 12199 + 57 compression (collision not covered). Backend (no Fable). Cert valid for sha:8d7b5393.
|
Addressed the round-2 marker-collision gate in |
🔬 Gate certification — GREEN ✅ · CONVERGED (round 3 — idempotent + anchored marker detection, all findings closed)Certified head: What I ran (rebased worktree
|
| Gate | Result |
|---|---|
| Rebase onto current master | ✅ git apply clean |
| Codex (re-verify collision + idempotence + core) | SAFE TO SHIP — 0 findings |
| Full pytest suite | ✅ 12200 passed, 0 failed |
| Compression tests (incl. collision + idempotence regressions) | ✅ 58/58 |
Findings — all 3 rounds closed
✅ R2 marker-collision FIXED (anchored detection): _is_compressed_context_tool_result_summary now uses _POST_COMPRESSION_TOOL_RESULT_NOTE_RE.fullmatch(text) (regex full-match, NOT bare substring in) and handles the real trailing-note case via rpartition("\n\n") + fullmatch on the note. So a raw oversized tool result that merely CONTAINS the marker string is no longer mis-detected as already-pruned → it still gets pruned. New test_post_compression_context_raw_marker_collision_still_prunes (raw payload containing the marker + 600 lines must still prune) locks it.
✅ R1 idempotence RETAINED: a genuinely-pruned result (matching the exact note shape) is still detected → not re-summarized; test_post_compression_context_hard_prune_is_idempotent passes.
✅ Core prune still safe: tool-result-only truncation, copy.deepcopy no-mutation, no orphaned tool_call/tool_result pairs. Codex confirmed the fullmatch regex is anchored (no ReDoS), matches the real generated summary shape but not arbitrary mid-body content. Full suite green.
Recommendation to the next agent
Ready to merge — use branch gate-rebase/5667-hard-prune-anchored-marker (sha:9157897c), NOT the PR's stale head 85501acf. A properly-converged compression-context fix: prunes bloated post-compression tool-result payloads, idempotent across compression passes (anchored marker skip), and collision-safe (raw content containing the marker still prunes). Codex SAFE + full suite green + collision & idempotence regressions. concept 4/5 (3-round convergence). Author @franksong2702 (T1). crit=3, compression data-correctness.
Gate-certifier layer (warm-up → gate → release). I do not merge/tag/deploy. R2 collision fixed via NOTE_RE.fullmatch + rpartition('\n\n') note-match (not substring 'in'); r1 idempotence retained; core prune safe (tool-result-only, deepcopy, no orphaned pairs); Codex SAFE (anchored regex, no ReDoS) + full suite green (0 failed) + 58 compression tests incl collision & idempotence regressions. Backend (no Fable). 3-round convergence. Cert valid for sha:9157897c.
…marker detection) Clean rebase of franksong2702's nesquena#5667 (rebase-first). Co-authored-by: franksong2702 <franksong2702@users.noreply.github.com>
85501ac to
9157897
Compare
|
Aligned the PR branch to the gate-certified |
…marker detection) Clean rebase of franksong2702's nesquena#5667 (rebase-first). Co-authored-by: franksong2702 <franksong2702@users.noreply.github.com>
9157897 to
d88cb2e
Compare
|
Refreshed the gate-passed PR onto the latest |
|
🔬 Gate re-affirm — GREEN ✅ (round-4 = rebase-only, code byte-identical to the round-3 converged cert) Certified head: The r4 re-push is a pure rebase onto newer master (it picked up the merged #5673 docs) — I verified
Ready to merge — branch |
…marker detection) Clean rebase of franksong2702's nesquena#5667 (rebase-first). Co-authored-by: franksong2702 <franksong2702@users.noreply.github.com>
d88cb2e to
150f5eb
Compare
|
🔬 Gate re-affirm — GREEN ✅ (rebase-only re-push; code byte-identical to the round-3 converged cert) · head Verified |
150f5eb to
f73c23f
Compare
…marker detection) Clean rebase of franksong2702's nesquena#5667 (rebase-first). Co-authored-by: franksong2702 <franksong2702@users.noreply.github.com>
|
Refreshed the gate-passed PR onto latest Rebase-only safety check: Local verification on the refreshed head:
Checks have restarted on |
…marker detection) Clean rebase of franksong2702's nesquena#5667 (rebase-first). Co-authored-by: franksong2702 <franksong2702@users.noreply.github.com>
036a55f to
1b6ffcb
Compare
|
Refreshed the review-feedback fix onto latest Local verification:
Diff remains scoped to |
|
CI read-back after the refreshed head The branch is current with base ( |
…marker detection) Clean rebase of franksong2702's nesquena#5667 (rebase-first). Co-authored-by: franksong2702 <franksong2702@users.noreply.github.com>
1b6ffcb to
f1a4e5e
Compare
|
Refreshed the review-feedback fix onto latest Local verification:
Diff remains scoped to |
…marker detection) Clean rebase of franksong2702's nesquena#5667 (rebase-first). Co-authored-by: franksong2702 <franksong2702@users.noreply.github.com>
f1a4e5e to
1251221
Compare
|
Refreshed the review-feedback fix onto latest Local verification:
Diff remains scoped to |
…marker detection) Clean rebase of franksong2702's nesquena#5667 (rebase-first). Co-authored-by: franksong2702 <franksong2702@users.noreply.github.com>
|
Refreshed the review-feedback fix onto latest Local verification:
Diff remains scoped to |
|
Refreshed the review-feedback fix onto latest Local verification:
Diff remains scoped to |
|
Release-manager gate (overnight) — full suite 12,324/0 + dual advisor. This one is a genuine gate split I'm surfacing rather than deciding unilaterally: Codex: SHIP ONLY WITH FIXES. Fable: GREEN — ship it (with the same metadata flag as a non-blocking fast-follow). Fable verified the identical shape-collision but rates it LOW-MED, adversarial-only: it requires a real tool output whose final paragraph exactly fullmatches WebUI's generated note format. Crucially: full outputs always remain in the visible transcript (deepcopy-proven), the newest tool result keeps a ≥2KB snippet (budget floors at 512, no zero-out), tool_call_id linkage preserved, idempotent across repeated compressions, non-compressed turns untouched — and pre-PR every oversized payload survived uncompressed, so the PR is strictly a net improvement even at the collision edge. My recommendation: SHIP (lean Fable). The collision is narrow + adversarial, the full output is never lost (visible transcript), and it's a strict improvement over today's behavior where nothing gets pruned. The |
|
Addressed the release-manager metadata-flag gate in What changed:
Local verification:
Diff remains scoped to |
|
Refreshed the metadata-flag fix onto latest New head: Local verification:
Diff remains scoped to |
|
Refreshed the metadata-flag fix onto latest Local verification:
Diff remains scoped to |
|
Refreshed the metadata-flag fix onto latest Local verification:
Diff remains scoped to |
|
Refreshed the metadata-flag fix onto latest Local verification:
Diff remains scoped to |
|
Refreshed the metadata-flag fix onto latest Local verification:
Diff remains scoped to |
|
Refreshed the metadata-flag fix onto latest Local verification:
Diff remains scoped to |
…marker detection) Clean rebase of franksong2702's nesquena#5667 (rebase-first). Co-authored-by: franksong2702 <franksong2702@users.noreply.github.com>
|
Refreshed the metadata-flag fix onto latest Local verification:
Diff remains scoped to |
Deep-review re-gate: SAFE TO SHIP ✅ (gate-pass)Ran the full authoritative gate (Codex reproduce + 992-test compression/streaming regression suite) on live head Pairing-safety (the critical concern) verified:
Certifying gate-pass. Backend streaming-context change (no visible UI). Thanks @franksong2702. |
|
Shipped in exp-v0.52.14 — thanks @franksong2702! Deep-review re-gate verified pairing-safety + context-only + idempotence by reproduction (Codex), 65 own + 992 regression tests green, deployed to experimental. Converged from the earlier bounce. 🚀 |
Thinking Path
_prune_context_tool_results_after_compression()can still keep a very large recent tool result because the agent compressor protects the tail.context_messagesonly, so protected post-compression tool payloads cannot silently dominate the next prompt budget.Contract Routing
Task type: runtime/compression/model-context invariant.
Touched areas:
api.streaming._prune_context_tool_results_after_compression()session.context_messagesafter automatic compressiontests/test_auto_compression_card.pyRelevant public docs read:
AGENTS.mdCONTRIBUTING.mddocs/CONTRACTS.mdARCHITECTURE.mdTESTING.mddocs/rfcs/README.mddocs/rfcs/webui-run-state-consistency-contract.mdState layer mutated: model context /
context_messagesonly.Session.messages, visible transcript, Worklog/history rendering, and focused-continuation recovery metadata are intentionally not pruned.Invariant proved: after a compression event, retained tool-result payloads in
context_messagesare bounded by the compressor tail budget/default WebUI hard cap while the visible transcript remains inspectable.What Changed
tail_token_budget,threshold_tokenswhen that threshold is smaller,role: toolcontent incontext_messageswith a compact summary that states the full output remains in the visible transcript/tool log.[WebUI compressed-context budget:tool summaries are preserved on repeat prune passes while still counting toward the shared rough-token budget.context_messages, andSession.messagesstill contains the original full output.Why It Matters
Compression should preserve the UI transcript without letting raw tool dumps re-enter the next model request. This closes the budget-proof gap left after replay guards: even if a recent tool result is protected by compressor tail policy, WebUI now enforces a final model-context-only bound before the next turn can inherit that payload.
Release-note wording: After automatic context compression, WebUI now hard-prunes oversized tool-result payloads from model-facing context while keeping the full visible transcript and tool log available.
Verification
./scripts/test.sh tests/test_auto_compression_card.py::test_post_compression_context_prunes_tail_tool_results_with_active_compressor tests/test_auto_compression_card.py::test_post_compression_context_hard_prunes_protected_tail_tool_payload tests/test_auto_compression_card.py::test_post_compression_context_note_only_replacement_consumes_residual_budget tests/test_auto_compression_card.py::test_post_compression_context_hard_prune_is_idempotent tests/test_auto_compression_card.py::test_post_compression_context_raw_marker_collision_still_prunes -q-> 5 passed./scripts/test.sh tests/test_auto_compression_card.py -q-> 58 passed.venv/bin/python -m py_compile api/streaming.py tests/test_auto_compression_card.py-> passedpython3 scripts/ruff_lint.py --diff origin/master-> 2 changed Python files, 0 new findings on added/modified linesgit diff --check origin/master-> passed9157897cac630eca977339b2ea0dfcca35dbc6ec;git diff --stat 85501acf208874d5892c57d649f9681c29f3ac3c..HEADproduced no output, so this is tree-identical to the locally verified head.origin/master26a829c952e72b58d74ea00b206845c6db33b39b; new PR headd88cb2ed611c2753bcc301f312584070bcab09bapassed the focused compression tests, the full touched test file, py_compile, diff-scoped ruff, and diff-check.Risks / Follow-ups
Model Used
OpenAI GPT-5 via Codex PR-aftercare agent. Notable tool use: local git,
gh, PaperClip issue API readback, repo pytest runner, and diff-scoped ruff gate.Refs #4685.