[boazdavid]

This PR stacks on #927, which stacks on the draft re-land #923. It must not merge unless the repo owner says yes to the re-land. Savings figures change. One fix (#910) also changes what the proxy forwards to the provider; that needs a human's yes.

Summary

• This PR adds the pricing fixes of fix(accountant): Credit cachesplit, report the summarize splice, mark the manual ping estimate #929 on top of the approved Fix: Combine the accountant fixes for #906-#919 on the re-land #927. Base is integ/batch1.
• It pins accountant at c1f0f7a (the merged main, with PRs 40 to 48).
• Fixes: Accountant integration: in-session cachesplit credit is $0 on the live path (pre-#768 credited it) #909 (cachesplit credit), Accountant integration: summarize books its model cost but no saving (ops_dropped) #910 (summarize), Accountant integration (#893 B3): a manual ping stores no facts, so the next request may be classified cold although the cache was read #913 (no defect found; labelled as an estimate).

What changed

Issue	Cause	Fix	Before / after (fake upstream truth)
#909	A library bug. On a short conversation the stable half the provider reads falls inside the read tolerance of the whole entry, so the B* rule dropped the split effect.	Fixed in accountant PR 48 (merged). The proxy gets a regression test with a short conversation.	cachesplit_saved_usd 0 to 0.04433; truth 0.04477
#910	rebuildCountChanged (apply/apply.go) matched a kept message to the first unused slot with the same bytes. A kept message could take the slot of a removed turn. The forwarded body held an orphan tool_result.	A sibling text now prefers the wire slot of the kept tool message next to it. A standalone message never takes another message's wire. A new check (wireOrderValid) runs at the end of the rebuild. If the order is wrong, the original body is forwarded and no result is recorded.	net_saved_usd -0.00274 to +0.00404; truth +0.00404
#913	Not a defect with the pinned library. The request after a manual ping is hot.	The Overview line, the Cache info text and the docs say the manual-ping figure is an estimate.	Test: TestTheRequestAfterAManualPingIsHotNotCold

Needs a human's yes

• Accountant integration: summarize books its model cost but no saving (ops_dropped) #910 changes what summarize forwards on transcripts with repeated sibling text. Before, the forwarded body could hold a wrong message.
• Round 1 found that the first version of the Accountant integration: summarize books its model cost but no saving (ops_dropped) #910 fix could put a tool_result before its tool_use when a standalone user text equals a sibling text. The provider would answer 400. Fixed in 883f834c, with 4 tests that failed first and a seeded property test (400 cases). wireOrderValid forwards the original body if a future matching mistake breaks the order.

How it was tested

• go test ./proxy/ ./dash/ pass. apply, keepalive, components, tooldefer pass. gofmt, go vet, go build, go mod tidy -diff are clean.
• The merge with fix(accountant): Harden stats cache key, state file and panics (#916 #917 #918 #920) #928 had one conflict: both branches add a test at the same place in proxy/accountant_ping_test.go. Both tests stay.

Known limits

• Accountant integration: the default tail breakpoint makes every request fall back (check_sum) and book no saving #906 (+11% with the default tail breakpoint) and Accountant integration: tool defer fails check_sum and conservation on every request #908 (the deferral is not credited) are not fixed here.

• Not done: storing facts for a manual ping with the rescue disabled (Accountant integration (#893 B3): a manual ping stores no facts, so the next request may be classified cold although the cache was read #913). There is no wrong phase to fix.

• The accountant pin carries more than the Accountant integration: in-session cachesplit credit is $0 on the live path (pre-#768 credited it) #909 split fix: check 13 size_range, the spend-call bad_usage checks, the int32 clamp, the TTL phase and the media fixes. This is intended.

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

[boazdavid]

Reviewer	Kind	Started by	Context	Review	Round	Head	Verdict
Reviewer 1 (glm, user of @boazdavid)	GLM rits/zai-org/glm-5-3 via Claude Code 2.1.241	offload-review skill, @boazdavid's session	fresh	1 of 2	1 of 5	382dcbcbb999	blocking

Verdict: blocking

Findings

1. apply/apply.go:2027 — The kept-wire preference can bind a standalone user text to another message's wire slot, and the forwarded body becomes invalid
   • What fails: An Anthropic request has a standalone user text, then a kept tool exchange whose sibling text is the same string. A count-changing component runs (for example summarize). The rebuild then emits the wire message that holds the tool_result BEFORE the assistant message that holds its tool_use. The provider rejects the request. The standalone text is emitted later, at the sibling's position.
   • Proof: Trace. Input: messages[0] = user, content "CALL_TOOL again" (whole-message slot s0). messages[1] = assistant tool_use b. messages[2] = user [tool_result b, text "CALL_TOOL again"] (tool slot s2, sibling slot s3). A later exchange is dropped, so the count changes and exchange b is kept. normalize builds s0.pre and s3.pre with the same bytes: {"role":"user","content":"CALL_TOOL again"} (a plain user message and a sibling text both marshal to one string content; schema.SetMessageText sets ContentStr in both cases). keptWire = {2}, from the kept tool b. Matching of out[0], the standalone user text: the loop sets matched = 0 (s0, byte-equal, not kept-wire), keeps scanning, and breaks at s3 (byte-equal, body index 2 is in keptWire). The rebuild emits orig[2], the wire message with the tool_result, at position 0. Then out[1], the assistant tool_use b, matches s1 and emits orig[1] after it. Result: tool_result for b before the tool_use that calls it. The sibling text later takes s0 and emits orig[0] at the wrong position. Before this change, the same input matched s0 first and produced the correct body. The new test covers only sibling text against sibling text (apply/ops_splice_test.go:86); it does not cover a standalone text that equals a sibling text. The trigger is the same repeated-text pattern the PR itself targets (#910: the sibling text repeats every turn), so it is reachable in real traffic.
   • Fix: Prefer a kept-wire slot only for an out message that is itself the sibling of that wire. For example: apply the preference only when the adjacent out message (before or after, because a text block can sit before or after the tool_result block) is the kept tool message that names the wire. Or match a sibling text by the wire of its adjacent tool message, not by a global byte preference. Add a test with a standalone user text equal to a sibling text.

Non-blocking

• The accountant pin (v0.1.1-0.20261105063645-c1f0f7a052db) carries more than the #909 split fix: check 13 size_range, spend-call bad_usage checks, and the int32 clamp. These change pricing and check output for malformed reports. State this in the PR if the wider pin is intended.
• dash/ui/app.js:1786 shows "· estimate" also when mp.count is 0 and the figure is $0.00. Consider hiding the suffix when there was no manual ping.
• docs/dashboard.md:311: the edited sentence is 60+ words and holds three ideas. Split it.

What I checked

• The full diff 0206b444..382dcbc (6 commits, 8 files). I read each changed line and the code around it.
• apply/apply.go rebuildCountChanged: I traced the new keptWire preference against the #910 test body (correct result), and against a standalone-text body (finding 1).
• normalize (apply.go:1742), the slot kinds, splitSlotPath, writeBackToolText, toolCallIDAt: to confirm slot order, pre byte shapes, and which body indexes a wire message can own.
• schema.SetMessageText and MessageText: a plain user message and a sibling text marshal to the same bytes.
• The new tests: traceOf, okReport, accRun.do, entryFacts, PingSavings, and the accountant API (EffectRescue, EffectSplit, PhaseHot, Saved.CacheWrite5m) all exist with matching signatures.
• The pinned accountant module in the local module cache. I diffed it against the old pin: the #909 fix is splitChangedSystem, and the manual-ping-hot rule (a billed read that covers a previous breakpoint makes the phase hot) is present in the phase logic. proxy/accountant_ping.go stores no facts for a manual ping, so the new ping test's assertions are consistent.
• dash/ui/app.js and docs/dashboard.md: the "estimate" wording matches the behavior described.
• I did not run builds or tests. Go is not installed on this machine and live tests are not approved.
• I did not read the PR description or any PR comment, per the round 1 rule.

[boazdavid]

Reviewer	Kind	Started by	Context	Review	Round	Head	Verdict
Reviewer 2 (glm, user of @boazdavid)	GLM rits/zai-org/glm-5-3 via Claude Code 2.1.241	offload-review skill, @boazdavid's session	fresh	2 of 2	1 of 5	382dcbcbb999	blocking

Verdict: blocking

Findings

1. apply/apply.go:2027 — the keptWire preference can capture a kept standalone user message, and emits a tool_result before its tool_use
   • What fails: Anthropic request, a count-changing component runs (for example summarize). The kept transcript holds a plain user message (wire i) and a later user message (wire j > i) that carries a tool_result plus a sibling text block. The plain message and the sibling text have the same bytes. The rebuild matches the plain message to the sibling slot of wire j. It then emits wire j, with its tool_result, at the position of the plain message. The assistant tool_use of wire j-1 is emitted after it. The provider rejects a tool_result that does not follow its tool_use, so the request fails with a 400. The base code matched the plain message to its own slot, so this corner is a regression of this PR.
   • Proof: a trace over the new loop.
     • normalize gives the plain user message a wholeMessage slot (apply/apply.go:1847) and the sibling text an anthropicSiblingText slot (apply/apply.go:1776). Both slots' pre values are json.Marshal of a user ChatMessage whose content is the same string: bifrost's unmarshal of string content sets ContentStr (bifrost schemas/chatcompletions.go:1200), and schema.SetMessageText sets the same field (schema/schema.go:224). So the two pre-images are byte-equal for the same text.
     • Example body, messages: [user "continue", assistant tool_use a, user [tool_result a, text "continue"], assistant tool_use d, user [tool_result d, text "x"]]. A component drops exchange d and inserts a summary, so the count changes and rebuildCountChanged runs. Tool a is kept, so keptWire = {2} (apply/apply.go:1990-2000).
     • Out message 0 is the plain user "continue". The scan at apply/apply.go:2022-2034: the slot of wire 0 byte-matches and is kept as a tentative match (line 2031); the sibling slot of wire 2 byte-matches and wire 2 is in keptWire, so it wins and the loop breaks (lines 2027-2029). The plain message is emitted as the raw bytes of wire 2.
     • The assistant tool_use a then matches wire 1. Tool a matches wire 2 by id, but wire 2 was already emitted, so it is skipped. The true sibling matches wire 0.
     • Forwarded order: [user (tool_result a + "continue"), assistant tool_use a, user "continue", summary]. The tool_result precedes its tool_use. The forwarded body is an altered request, not a fail-open one, and no size check catches it because the same wire messages are emitted, only in the wrong order.
     • The base code took the FIRST byte-equal unused slot in slot order (wire order), so it matched the plain message to wire 0 and kept the correct order.
   • Fix: do not let the keptWire preference override a match on a wholeMessage slot. A wholeMessage slot is a 1:1 body message; the #910 case has only anthropicSiblingText candidates, so the preference only needs to choose between shared slots. Alternatively, verify that the out message belongs to the wire group of the kept tool message before preferring that group. Add a test: a kept standalone user message whose text equals the sibling text of a kept exchange.

Non-blocking

• docs/dashboard.md:308 — the new sentence runs to about 60 words. CLAUDE.md asks for short sentences. Split it.
• apply/ops_splice_test.go:77 — the test file registers dropfirst in the global component registry. This follows the existing midsum pattern in the same file. No defect.
• The new tests in proxy/ were not run (live tests not approved). I verified them by reading: every helper and field they use exists (traceOf, okReport, entryFacts, PingSavings, accRun.do, envelopeOps, accountant.Result fields), and the library behavior they pin is present in the pinned version.

What I checked

• The full diff of base 0206b44..HEAD 382dcbc, and every commit in the range.
• apply/apply.go: rebuildCountChanged, normalize, splitSlotPath, writeBackToolText, toolCallIDAt, the slot kinds, and the new matching loop line by line. I traced the new #910 test through the loop by hand; it passes as written.
• The byte-equality claim behind finding 1: schema.SetMessageText, bifrost ChatMessageContent.UnmarshalJSON, and the two slot pre-image constructions.
• The accountant pin: I diffed the old and new library versions in the module cache. The new version holds the #909 fix (splitChangedSystem), the hardening checks, and the hot-phase rule "a billed read is proof the entry was alive", which the #913 test relies on. The library's go.mod is unchanged, so go.sum needs only the two updated lines.
• The two new proxy tests against the code they exercise: PingSavings (manual pings store no facts, proxy/accountant_ping.go:121), entryFacts, systemOps (the split detection in the #909 test), and the phase decision in the library.
• dash/ui/app.js and docs/dashboard.md: the "estimate" label for manual pings. Cosmetic, and it matches the #913 resolution.
• I did not run any build or test (not approved). I could not verify the go.sum hashes or the tests' pass/fail; everything above is from reading the code.

[boazdavid]

Round 1 is addressed in 883f834.

1. Blocking, both reviewers (the kept-wire preference binds a standalone user text to a sibling slot): fixed. The preference now applies only to an anthropicSiblingText slot whose wire message is named by the adjacent kept tool message (before or after the text). A wholeMessage slot is never given another message's wire. The loop also checks the order of the kept wire messages at the end (wireOrderValid): the kept messages keep their original relative order, and a tool_result does not come before its tool_use. If it fails, the rebuild logs a warning and forwards the original body (fail open). New tests in apply/ops_splice_test.go: trace A, trace B, an adversarial repeated-text case, and a seeded property test (400 cases). They failed on 382dcbc and pass now. TestWireOrderValid covers a violating order. TestSpliceKeepsSurvivorsWithIdenticalSiblingText (Accountant integration: summarize books its model cost but no saving (ops_dropped) #910) still passes.
2. Non-blocking, the accountant pin is wider than Accountant integration: in-session cachesplit credit is $0 on the live path (pre-#768 credited it) #909: stated in the PR body. It is intended.
3. Non-blocking, "· estimate" with zero manual pings: the suffix is hidden when mp.count is 0.
4. Non-blocking, long sentence in docs/dashboard.md: split into short sentences.

[boazdavid]

Reviewer	Kind	Started by	Context	Review	Round	Head	Verdict
Reviewer 2 (glm, user of @boazdavid)	GLM rits/zai-org/glm-5-3 via Claude Code 2.1.241	offload-review skill, @boazdavid's session	resumed	2 of 2	2 of 5	883f834cf75a	approve

Verdict: approve

Earlier findings

1. My round-1 finding (apply/apply.go:2027, the keptWire preference captures a standalone user message): fixed in 883f834. I checked the fix by hand, not by running tests.
   • The preference now fires only for a slot of kind anthropicSiblingText whose wire message is named by the kept tool message ADJACENT in out (apply.go:2030-2041). I traced my two round-1 scenarios through the new loop:
     • Trace B (user "continue", tool_use a, tool_result a + text "continue", dropped exchange d): the standalone user at out[0] has no adjacent tool message, so no preference fires. It matches its own wholeMessage slot. Forwarded: [user "continue", tu a, tr a, summary]. Correct.
     • Trace A (user "CALL_TOOL again", kept exchange b with the same sibling text): same shape. Correct.
   • The nextWire path (preference from the tool message AFTER the text) is safe for valid input: a standalone user message can never sit directly before a tool message in out, because a tool_result wire message must follow the assistant tool_use message in a valid body. The path serves a text block placed before the tool_result inside one wire message.
   • The new backstop wireOrderValid (apply.go:2086) rejects a rebuild whose kept messages lose their relative order or put a tool_result before its tool_use, and the caller forwards the original body. That is fail-open. I checked its rules against the internal test cases and against a correct rebuild: a correct rebuild emits strictly increasing body indices, and a dropped exchange's lone tool_result passes by design ("the splice's own business").
   • The #910 test still holds: I traced the survivor's sibling through the new loop. prevWire names its own wire, so the preference picks the survivor's sibling slot, not the removed turn's.
   • The new tests pin both of my traces (TestSpliceStandaloneTextEqualToKeptSiblingText, TestSpliceKeptPlainUserBeforeExchangeWithSameText), an adversarial repeated-text case, and a 400-case seeded property test. I traced the adversarial case by hand; the expected sequence matches what the loop produces.
2. Reviewer 1's round-1 finding 1: the same defect as mine. Fixed the same way. Agree.
3. Reviewer 1's non-blocking (the accountant pin is wider than #909): the author says it is intended and stated in the PR body. I read the PR body. It states the pin carries the size_range, bad_usage, int32 clamp, TTL phase and media fixes, and that this is intended. Accepted.
4. Reviewer 1's non-blocking ("· estimate" shown with zero manual pings): fixed in dash/ui/app.js. The suffix now shows only when mp.count is not 0.
5. My non-blocking and reviewer 1's (the long sentence in docs/dashboard.md): fixed. The sentence is split into short ones.

Findings

None.

Non-blocking

• apply/apply.go:2030 — a residual edge remains: if a component removes a sibling text but keeps its tool message, and a later standalone user message has the same bytes, the standalone can match the unused sibling slot of an already-emitted wire and be skipped, so its own wire message is not forwarded. The base code (before this PR) does the same through the first-match rule, and no current component removes a sibling alone. Note only; no change requested.
• The PR body's #910 row still describes the round-1 mechanism ("a byte match now prefers a slot that a kept tool message already names"). The final code narrows this to adjacent sibling slots plus the wireOrderValid backstop. Update the row so the description matches the code that merges.
• apply/ops_splice_test.go — in the property test, exch is assigned but never read. Dead code.

What I checked

• Last review: round 1, 382dcbc, 2026-11-05T06:50:49Z.
• New commits read: 1 (883f834 "fix(apply): Keep the kept-wire preference off standalone messages"). The full diff 382dcbcb..883f834c and the final state of every changed file around the changes.
• Read in full: 2 reviews, 1 comment, 0 inline comments (from the three saved files in my folder).
• New items since my last review: the commit 883f834 (reviewed above); the author's comment at 06:57:45Z (each point checked, see "Earlier findings"); reviewer 1's round-1 review (posted 6 seconds before mine, so unread at posting time; now read in full — same finding 1, three non-blocking notes, all addressed).
• The fix code line by line: keptWireOf, the narrowed preference in the matching loop, wireOrderValid, and the slog.Warn fail-open path. The log/slog import is present in apply.go.
• The new tests: dropExchanges and its want computation, checkWireOrder, the two trace tests, the adversarial test, the property test, and TestWireOrderValid. I traced the two trace tests, the adversarial test, and the #910 test through the matching loop by hand. I did not run them (live tests not approved).
• The PR body, to verify the author's reply that the wider accountant pin is stated there. It is.
• I did not run any build or test. Go is present on this machine but live tests are not approved, so all verification above is from reading and tracing the code.

[boazdavid]

Reviewer	Kind	Started by	Context	Review	Round	Head	Verdict
Reviewer 1 (glm, user of @boazdavid)	GLM rits/zai-org/glm-5-3 via Claude Code 2.1.241	offload-review skill, @boazdavid's session	resumed	1 of 2	2 of 5	883f834cf75a	approve

Verdict: approve

Findings

None.

Earlier findings

• My finding 1 (the kept-wire preference binds a standalone user text to a sibling slot): fixed. The preference now applies only to an anthropicSiblingText slot whose wire message is named by the adjacent kept tool message (apply/apply.go:2030-2040). A wholeMessage slot is never preferred. I traced both round-1 scenarios (trace A and trace B) through the new loop: the standalone message keeps its own slot in both. The new tests TestSpliceStandaloneTextEqualToKeptSiblingText and TestSpliceKeptPlainUserBeforeExchangeWithSameText cover exactly those traces, and I traced them through the loop by hand.
• Reviewer 2's finding 1: the same defect. Fixed by the same change. I checked it myself; I add no new proof beyond the traces above.
• My non-blocking notes 2 to 4: the "· estimate" suffix is hidden when mp.count is 0 (dash/ui/app.js:1786). The long sentence in docs/dashboard.md is split. The wider accountant pin is stated as intended in the author's reply. All addressed.

Non-blocking

• apply/apply.go:2030 — a residual edge of the preference remains: a standalone message that sits directly beside a kept tool message in out, where that wire's sibling text is absent from out, can still take the sibling slot and be dropped. I could not reach it: the input needs a user message between a tool_use and its tool_result (the provider rejects that input on arrival), or a component that deletes a sibling text while keeping its tool message (no shipped component does; summarize declines when the wire groups are stale, and its boundary walk keeps the span off wire groups when aligned). If a component like that is added later, revisit this preference. A pure deletion is also the one shape wireOrderValid cannot see.
• apply/apply.go:2092 — wireOrderValid deliberately allows a lone tool_result ("a dropped middle"). The rebuild can emit one only in the same unreachable state as above (a kept sibling text whose tool message was dropped). If that state ever becomes reachable, the backstop forwards a body the provider rejects. The base code had the same behavior, so this is not a regression.
• apply/ops_splice_test.go:226 — exch in TestSplicePropertyKeepsWireInvariant is written and never read. Dead code; remove it or use it.

What I checked

• Last review: round 1, 382dcbc, 2026-11-05T06:50:43Z.
• New commits read: 1 (883f834 "fix(apply): Keep the kept-wire preference off standalone messages").
• Read in full: 2 reviews, 1 comment, 0 inline comments (from the three files).
• New items since my last review: the author's comment ("Round 1 is addressed in 883f834", 2026-11-05T06:57:45Z) — I verified each claim in the code; the commit 883f834 — reviewed below.
• The full diff 382dcbcb..883f834c and the code around each change: rebuildCountChanged's new keptWireOf closure and preference, and the new wireOrderValid check.
• wireOrderValid itself: the strict-increase rule holds for every correct rebuild (out preserves norm order, norm order preserves body order, no shipped component reorders). The pairing rules read tool_use/tool_result blocks from the raw body; a string content yields no blocks of either type. The decline path fails open: it logs and returns the original body.
• The new tests by hand: trace A, trace B, the adversarial repeated-text case, and the property test's generator and want construction, all traced through the new matching loop. Trace A fails on 382dcbc (the standalone message takes the sibling slot, checkWireOrder catches the tool_result before its tool_use), which matches the author's claim.
• Reachability of the residual edges: components/offload/summarize_pairing.go (advanceSpanBoundary, dropOrphanedToolResults), components/offload/summarize.go (wireGroupsStale decline at line 414, emitCheckpoint's boundary walk), and components/offload/dedup.go (rewrites in place, no count change). Conclusion: with aligned wire groups the span never splits a wire message, and summarize declines when an earlier component made the groups stale, so no shipped path keeps a sibling text while dropping its tool message.
• Test hygiene: dropIDs is a package variable; no test in apply calls t.Parallel(), so the sequential tests cannot overlap. log/slog is already imported in apply.go. The dropex registration does not collide with any other name.
• I did not run builds or tests. Go is not installed on this machine and live tests are not approved.