feat(extract_llm)!: extract_llm_sweep — adjudicate spent tool outputs over the model's cached transcript - #118
Conversation
Specification only; no behaviour change. Committed ahead of the implementation because the reasoning is the expensive part and it is currently spread across a closed PR, a commit message on another branch, and a conversation. The argument: extract_llm does one thing in two situations that want different things. On a warm turn it trims a recent tool output, and rewriting is right there. On a cold sweep it does the same to outputs deep in history, where the operation is wrong -- that content is either still load-bearing, in which case rewriting corrupts what the model has already reasoned about, or it is spent, in which case it should be removed rather than made smaller. feat/coref-compaction reached this conclusion for the merged design and removed the `trim` verdict (cc1aa9f): chosen zero times in 21 probe opportunities, metrics identical without it, accepted once in production against eight rejected as invented, and the only verdict that asked the model to transport text. What remained is binary -- keep verbatim, or drop with a shape descriptor left in place -- with an obligation named and quoted verbatim, and unsure defaulting to keep. Applying that contract per-output removes the transporting OPERATION rather than just the transporting strategies, which is the strongest form of the rule the measurement supports. It also settles two things that were being carried as open questions: `rewrite` becomes moot on the sweep rather than differently-defaulted, since nothing is rewritten; and the extract_llm/extract_llm_sweep split stops being config hygiene, because strategy, rewrite, aggressiveness and max_chars all cease to apply to a component that selects no compaction strategy. Records the config surface field by field, six safety invariants each stated as a test that must fail when its subject is reverted, the counters and which two must be alertable, three open questions that need measurement rather than argument, and an execution order whose first slice is verifiable without any component wiring. Supersedes #117, which restricted the sweep's strategies to the non-transporting ones. That stopped the model RETURNING content but left `code` rewriting it, so it was a half-step toward this and is closed rather than merged. Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com>
…es that make a verdict actionable The cold sweep and the warm tail want different operations. On a warm turn a rewrite is right: the output is recent and a smaller version of it is more useful than none of it. On a cold sweep the same operation runs against deep history, which is either still load-bearing -- in which case rewriting corrupts content the model has already reasoned about -- or spent, in which case the answer is removal, not a smaller version of something nobody will read. This is the contract for the second case, ported from the merged design's internal/extract/bulk.go WITHOUT its batching. The contract is general; the batch is not. The verdict is binary because cc1aa9f measured the alternative. trim was chosen ZERO times in 21 probe opportunities, keep/drop scored identically to keep/drop/trim on every metric, and in production it was accepted ONCE against EIGHT rejected as invented. It was the only verdict that asked the model to transport text. Removing it removes the transporting OPERATION rather than the transporting STRATEGIES: after this there is no reply field that can carry output content at all. The criterion is a required output field rather than an instruction because arms carrying an identical criterion differed ONLY in whether the model had to name and quote the obligation, and the arm that had to emit it halved the false-drop rate, 4/4 to 2/4. Stating the criterion alone measured inert. Two sections of bulk.go's prompt are deliberately absent. READING THE EVIDENCE taught the model to interpret a co-reference index's counters, and no such index exists on this path. JUDGE THEM AGAINST EACH OTHER describes comparative ranking, which a per-output call structurally cannot do -- and that is a recorded risk, not an oversight: the merged experiment measured comparative judgement as the difference between 6% and 58% live-kept. The safety machinery is what carries this design, and every failure it detects resolves toward keep. Four guards, each verified to FAIL when its subject is reverted: * a drop that names an outstanding obligation is refused, not performed. Removing the refusal (keeping its counter) gives: --- FAIL: TestDropNamingAnObligationIsRefused (0.00s) adjudicate_test.go:38: needed_by="a": dropped an output the model itself said is still needed adjudicate_test.go:38: needed_by="b": dropped an output the model itself said is still needed adjudicate_test.go:38: needed_by="c": dropped an output the model itself said is still needed adjudicate_test.go:38: needed_by="A": dropped an output the model itself said is still needed adjudicate_test.go:38: needed_by=" b ": dropped an output the model itself said is still needed adjudicate_test.go:38: needed_by="in-progress-step": dropped an output the model itself said is still needed * unsure defaults to keep. Making an unparseable reply and an unusable verdict drop instead gives: --- FAIL: TestUnsureDefaultsToKeep (0.00s) adjudicate_test.go:81: empty reply: dropped on an unusable verdict; unsure must default to keep adjudicate_test.go:81: prose, no object: dropped on an unusable verdict; unsure must default to keep adjudicate_test.go:81: truncated object: dropped on an unusable verdict; unsure must default to keep adjudicate_test.go:81: not json inside braces: dropped on an unusable verdict; unsure must default to keep adjudicate_test.go:81: verdict absent: dropped on an unusable verdict; unsure must default to keep adjudicate_test.go:81: verdict empty: dropped on an unusable verdict; unsure must default to keep adjudicate_test.go:81: verdict is trim: dropped on an unusable verdict; unsure must default to keep adjudicate_test.go:81: verdict is prose: dropped on an unusable verdict; unsure must default to keep * a fabricated obligation quote is counted. It argues for KEEPING so it is not dangerous, but on this design it is the only remaining signal that the model is inventing, since nothing else it returns is content. Removing the containment check gives: --- FAIL: TestFabricatedObligationQuoteIsCounted (0.00s) adjudicate_test.go:106: a quote absent from the transcript was not counted as fabricated * an unanswered criterion field is tolerated and counted. Requiring it would collapse yield against a model that omits it; ignoring it would hide that the forcing function never ran. Removing the counter gives: --- FAIL: TestUnansweredCriterionIsToleratedAndCounted (0.00s) adjudicate_test.go:138: an unanswered criterion was not counted; the forcing function's absence would be invisible Every test asserts a PRECONDITION before its subject -- that the reply parsed, and that the verdict reached the branch under test. Without those, a parse regression turns each into a vacuous pass: Judge would return the zero Adjudication, Drop would be false, and "the refusal worked" would be indistinguishable from "the reply was never read". The quote check retries once whitespace-insensitively. A model that re-wrapped a long line copied faithfully and is not inventing, and this counter is meant to be alertable -- a routine re-wrap firing it would train the operator to ignore the signal that matters. No component wiring in this commit; the contract and its parser are verifiable alone. Suite: 27 packages, 0 failures. Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com>
…ts nothing, and a drop that stays recoverable Two properties carry the drop, and each is now a test. THE RESIDUE TRANSPORTS NOTHING. It is computed from the output's SHAPE -- content class, line count, record count, token count -- by this code, and contains no byte of the output. That is stricter than the merged design's residue, which fell back to a 96-character head peek for anything that was not a JSON array or object, i.e. for most real tool output. A head peek is our code copying rather than the model copying, so it is not the failure cc1aa9f was about, but it is still unverified content in the request -- and the point of an adjudicator over a compactor is that no content moves at all. It also does not help where it looks like it should: for a record set the first rows say nothing about whether the field you want is in there, which is the argument the merged design already accepted for structured content and then declined to apply to the rest. The record count is there because a line count alone cannot answer "how much was here": a multi-megabyte JSON API result is routinely a SINGLE line, which is the same shape that makes collapse skip such payloads, and "1 line" is a useless thing to tell an agent about 200 records. Only a top-level array that parses gets a count; a partial payload gets the line count rather than a guess, because the descriptor's whole value is that every number in it is true. THE DROP IS REVERSIBLE, AND THE MODEL IS NOT TOLD SO. It goes through tryMark/commitMark, so the never-worse check is MARKER-INCLUSIVE -- the descriptor is small but the marker plus its recovery hint is not free, and the pipeline's aggregate guard is per-request rather than per-message. Note the asymmetry with the prompt, which never mentions recoverability: measured, reassuring the model that removals stay recoverable produced 91% removal at 6% live-kept. The operator gets the safety net; the model does not get to hear about it. Guards, each verified to FAIL when its subject is reverted: * a dropped output stays recoverable. Removing the commitMark stash gives: --- FAIL: TestDroppedOutputStaysRecoverable (0.03s) extract_sweep_drop_test.go:56: the marker did not resolve — the drop would be unrecoverable --- FAIL: TestDroppedOutputWithoutAPersistingStoreLeavesNoDanglingMarker (0.00s) extract_sweep_drop_test.go:84: an unrecoverable drop must set Irreversible or the pipeline will revert it and omitting the marker token from the assembled text gives: --- FAIL: TestDroppedOutputStaysRecoverable (0.02s) extract_sweep_drop_test.go:52: expected exactly one resolvable marker, got 0 in "[context-guru removed a spent tool output — tool output, 7 lines, 109 tokens]" * the descriptor transports nothing. Restoring the merged design's head peek gives, on the first of four fixtures: --- FAIL: TestSweepDescriptorTransportsNothing (0.00s) extract_sweep_drop_test.go:117: descriptor transports content from the output: "KUBERNETES_NAMESPACE=quarantine-zebra" appears in "[context-guru removed a spent tool output — tool output, 7 lines, 109 tokens; began: KUBERNETES_NAMESPACE=quarantine-zebra\ndeployment/ingress-flamingo READY 3/3 RESTARTS 0\ndeplo]" extract_sweep_drop_test.go:117: descriptor transports content from the output: "deployment/ingress-flamingo" appears in ... [27 further leaks across the remaining fixtures] and removing the record count gives: --- FAIL: TestSweepDescriptorCountsRecordsOnASingleLineArray (0.00s) extract_sweep_drop_test.go:138: a 200-record single-line array must report its record count, got "[context-guru removed a spent tool output — json_blob, 1 lines, 802 tokens]" The recoverability test asserts a PRECONDITION that the drop actually happened before it asserts recovery: "recoverable" is trivially true of content that was never removed, so a helper that returned early would otherwise pass. The descriptor test asserts the shape facts are present alongside the no-leak check, because a descriptor that came back empty would pass the leak assertion while telling the agent nothing. No component wiring yet -- the splice is a package helper the sweep component will call. Suite: 27 packages, 0 failures. Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com>
…tput at a time
The component the previous two commits were building toward. On a turn whose prompt cache
has expired it asks a cheap model, ONE OUTPUT PER CALL, whether each candidate is still
needed, and either keeps it VERBATIM or removes it behind a shape descriptor and a
recoverable marker.
Not a batch, deliberately. A shared reply can be truncated mid-array, quote fidelity
degraded with batch size when measured (4 of 37 quotes non-verbatim at batch 16 against 0 of
16 at batch 10), and a batch-truncation counter has to exist to compensate. A per-output call
has none of those. The cost of the choice is that comparative judgement is unavailable, which
is recorded in internal/extract/adjudicate.go rather than glossed: the merged experiment
measured comparative ranking as the difference between 6% and 58% live-kept, so the prior on
per-output YIELD is negative and it is the safety machinery that carries this design.
Reused rather than forked, as specified: the model client, pricing, the economic gate, the
session result cache, the marker/stash machinery and the report/gate plumbing are all
extract_llm's. Only the decision is new.
The config surface is the split the proposal specifies. min_tokens, min_idle_seconds and
max_calls are the sweep's own; model, marker_mode, context, context_messages,
model_max_input_tokens and economic_gate are shared. strategy, rewrite, aggressiveness and
max_chars are REFUSED with a named reason rather than silently ignored -- an adjudicator
selects no strategy and produces no rewritten text, so a silently accepted `rewrite: false`
would read as "verified deletion-only is on" when nothing is being rewritten. Detected on a
separate probe struct, because a field on the config struct would have to be declared to the
settings form and would put a knob on the page whose only behaviour is to fail.
Three things the proposal did not cover, decided here:
* A DROP MUST BE REPLAYED ON EVERY LATER TURN. The decision is frozen in the shared
session-scoped result cache and re-applied at any depth on warm turns, because otherwise
the next warm turn re-sends the removed output verbatim -- the saving evaporates AND the
prefix the provider is caching stops being byte-stable, which costs more than the sweep
saved. This is safe in the sense TailOnlyCold's doc requires: the DECISION came from a
model, but the REPLACEMENT is sweepDescriptor(content), a pure function of the content,
so a replay cannot emit different bytes than the turn that decided it. Session-scoped
only, never global -- a drop is a judgement about THIS transcript's obligations.
* The sweep carries its OWN ratioTracker. extract_llm's is calibrated on partial rewrites
and this path is all-or-nothing, so pooling them would price each on the other's history.
The gate's break-even for a drop is still the unmeasured number the proposal names as
open question 3; rather than invent a prior, the existing exploration budget learns it.
* The refusal is counted INSTEAD of a keep, not alongside it. Both leave the output
verbatim, but folding "the model tried to remove something it had just said was needed"
into the keep total is what would make the alertable counter invisible.
A REAL DEFECT FOUND WHILE VERIFYING THE CAP TEST. The gates were first raised from inside the
per-call goroutines. components.Report is copied by value across this codebase so its Gates
map carries no lock, and Go turns that into `fatal error: concurrent map writes` rather than
a wrong count -- it killed the test binary. Each call now accumulates its own gate names into
its own slot and the serial phase raises them, the same discipline the ModelCall records use.
Guards, each verified to FAIL when its subject is reverted:
* a drop naming an outstanding obligation is refused end to end. Letting the component
perform it gives:
--- FAIL: TestSweepRefusesADropThatNamesAnObligation (0.08s)
extract_sweep_test.go:157: the contradictory drop was PERFORMED (gates: map[below_output_floor:1 sweep_adjudicated:1 sweep_drop_refused_obligation:1 sweep_dropped:1])
* the cold condition governs. Dropping the ColdCache check gives:
--- FAIL: TestSweepDoesNothingOnAWarmTurn (0.05s)
extract_sweep_test.go:179: a warm turn must say why it did nothing (gates: map[below_output_floor:1 cached_prefix:1])
--- FAIL: TestSweepMinIdleRaisesTheBar/warm,_however_long_idle (0.00s)
extract_sweep_test.go:202: sweeping=true, want false
* a drop survives into later warm turns. Not freezing the decision gives:
--- FAIL: TestSweepReplaysItsDropOnTheNextWarmTurn (0.08s)
extract_sweep_test.go:236: the warm turn did not replay the frozen drop (gates: map[not_a_cold_sweep:1])
* the cap binds. Disabling it gives:
--- FAIL: TestSweepCapBinds (0.07s)
extract_sweep_test.go:265: max_calls: 2 allowed 5 calls (gates: map[sweep_adjudicated:5 sweep_dropped:5])
* the compaction-only keys are refused with a REASON. Removing the probe leaves the generic
yaml message, which says the key is unknown rather than why it cannot apply:
--- FAIL: TestSweepRejectsCompactionOnlyKeys/strategy (0.00s)
extract_sweep_test.go:326: error does not say why the key cannot apply: yaml: unmarshal errors:
line 1: field strategy not found in type offload.extractSweepConfig
* every candidate is accounted for under concurrency. Raising the gates from the goroutines
again gives, under -race:
WARNING: DATA RACE
Read at 0x00c000b92150 by goroutine 31:
github.com/rossoctl/context-guru/components.(*Report).Gate()
components/component.go:510
github.com/rossoctl/context-guru/components/offload.(*ExtractSweep).Offload.func1()
components/offload/extract_sweep.go:418
Every test asserts a PRECONDITION that the component acted -- the call count, and the gate
for the branch under test -- before asserting the outcome. A component that never ran leaves
the transcript in exactly the state a correct keep does, so without those the drop, keep and
refusal tests would all pass on a component that had stopped working.
extract_llm keeps its per_output and cold_cache surface for now; removing it is the next
commit, so this one is additive and the suite is green either way.
Suite: 27 packages, 0 failures.
Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com>
The six counters the proposal names are what an operator's dashboard query and alert rule are
written against, so a rename breaks monitoring silently rather than loudly -- the harness and
the alert keep running and report zeros, which is the same failure mode statsGoldenTopLevel
exists to prevent for the /stats payload.
Two of the six must be ALERTABLE, and each names a distinct misbehaviour rather than a volume:
* sweep_drop_refused_obligation -- the model tried to remove an output it had, in the same
reply, just said was still needed. The removal does not happen; that is the invariant. But
a non-zero rate means the contract is not holding and the only thing between the workload
and silent context loss is our refusal.
* sweep_quote_fabricated -- the model cited transcript text that is not in the transcript. It
argues for KEEPING so it is not dangerous, but on this design it is the ONLY remaining
signal that the model is inventing, because nothing else it returns is content. If the
verdicts are to be trusted at all, this is the number that says whether they can be.
They reach an operator through the existing generic cg_component_gate_declines_total{component,
gate} series rather than through a bespoke metric each, so no new export surface is added.
Pinned at BOTH ends, with the literal strings written out twice on purpose:
* components/offload drives the real component through five outcomes -- spent, still needed,
a drop contradicting an obligation, an invented quote, an unanswered criterion -- and
asserts it raises those exact names.
* proxy asserts the same names survive to /stats under the component's own object AND to the
Prometheus gate series, with distinct per-counter values so a mixed-up mapping shows up.
A single shared list would have been less duplication and a worse test: a rename that edited it
would leave every test passing while breaking every deployed alert rule.
Verified to FAIL when its subject is reverted. Renaming the two alertable counters in the
component only:
--- FAIL: TestSweepRaisesTheContractedCounterNames/a_drop_contradicting_an_obligation (0.00s)
extract_sweep_test.go:378: the component did not raise "sweep_drop_refused_obligation"; got map[below_output_floor:1 sweep_adjudicated:1 sweep_refused:1]
--- FAIL: TestSweepRaisesTheContractedCounterNames/an_invented_obligation (0.00s)
extract_sweep_test.go:378: the component did not raise "sweep_quote_fabricated"; got map[below_output_floor:1 sweep_adjudicated:1 sweep_fabricated:1 sweep_kept:1]
and dropping the gate histogram from the /stats rollup (metrics.addGates):
--- FAIL: TestSweepCountersReachStatsAndMetrics (0.00s)
sweep_counters_test.go:73: the component's gate histogram did not reach /stats, so the counter names cannot be checked
Both tests assert a PRECONDITION first -- that an adjudication was counted, and that the gate
histogram arrived at all. Without them a component that never called the model, or a rollup
that dropped `gates` entirely, would be indistinguishable from six renamed counters.
Suite: 27 packages, 0 failures.
Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com>
|
Correction to the proposal in this PR: its justification for per-output adjudication is inverted, and the mistake is mine. The doc says
It ran accidentally, scored inside the drop-everything null model's error bar, and the commit treats it as a bug — fixed by bypassing the
Batch size is load-bearing for willingness to act, and per-output is batch-of-one — the extreme of unwilling. So the component built here is the shape the measurements refute. My secondary argument for per-output does not survive either. I claimed it avoids truncated shared replies and quote-fidelity decay; both were already addressed on Do not merge on the premise as written. The likely correct shape for the sweep is batched adjudication at ~10–12. What is unaffected, and worth keeping whichever shape wins: the contract port, the refusal of a drop that names an obligation, unsure-defaults-to-keep, fabricated-quote counting, a descriptor that transports nothing (a test caught 29 content leaks across 4 fixtures when the merged design's 96-char head peek was restored), reversibility, and the drop-replay-on-warm-turns decision that was missing from the spec. Those are orthogonal to batch size; what changes is the call site and a batch assembler. Also filed from this work: #119 — |
extract_llm raised two gates from inside its concurrent phase -- deduped_inflight_extraction and reply_truncated. components.Report is copied by value throughout this codebase and its Gates map therefore carries no lock; the file says so itself, six lines above, at the declaration of the per-slot ModelCall records that exist for exactly this reason. The gates never got the same treatment. THE SEVERITY IS WHY THIS IS ITS OWN COMMIT. An unsynchronised map write is not a wrong counter and not a recoverable panic. The runtime aborts with `fatal error: concurrent map writes`, which in a proxy kills the PROCESS -- every in-flight request of every session -- rather than letting one component fail open the way the rest of this file is careful to. Found while building extract_llm_sweep, whose first draft made the same mistake and whose cap test died on it. The reachable path is the single-flight follower. extractInflight collapses byte-identical content into one call and releases every waiter at the same instant, so N identical candidates in one request give N-1 simultaneous raises. Four identical tool outputs is enough. Fixed with the pattern already proved in the sweep component: each call appends its own gate names to its own slot, and the existing serial phase raises them. The gates are raised unconditionally there, before the `calls[k].Component != ""` guard, because a single-flight follower returns BEFORE filling its ModelCall slot -- and its gate is precisely the one that says so, so gating the raise on a filled record would drop it. The broader question -- give Report.Gate a lock once so no component can reproduce this -- stays open on #119. This commit fixes the one live occurrence in the file it was already being edited in. Verified to FAIL when reverted. Raising the gates from the goroutines again gives, under -race: WARNING: DATA RACE Read at 0x00c00015e158 by goroutine 28: github.com/rossoctl/context-guru/components.(*Report).Gate() components/component.go:507 github.com/rossoctl/context-guru/components/offload.(*ExtractLLM).Offload.func2() components/offload/extract_llm.go:1356 Previous write at 0x00c00015e158 by goroutine 27: github.com/rossoctl/context-guru/components.(*Report).Gate() components/component.go:508 github.com/rossoctl/context-guru/components/offload.(*ExtractLLM).Offload.func2() components/offload/extract_llm.go:1356 and, WITHOUT -race, the quiet form -- a lost increment -- on 3 of 20 runs: --- FAIL: TestConcurrentCallsDoNotRaceOnTheGateHistogram (0.00s) extract_llm_gaterace_test.go:69: deduped_inflight_extraction = 2, want 3: a follower's gate was lost, which is the quiet form of the same unsynchronised write (gates: map[deduped_inflight_extraction:2]) That second form is why the test asserts an exact count rather than only relying on -race, which the suite does not run by default. The test asserts a PRECONDITION that a follower actually ran -- only a follower can raise deduped_inflight_extraction -- because if the candidates never reached the concurrent phase no two raises were ever simultaneous and the test would prove nothing. Suite: 27 packages, 0 failures. Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com>
The per-output shape this component shipped with was the one the experiments REFUTED, and the
proposal doc's argument for it was inverted. Corrected here, in the code and in the doc.
WHAT THE EVIDENCE ACTUALLY SAYS. docs/results/coref-selection-experiment.md, ten arms over
8,105 recorded decisions: shown ONE output, a model scored 6% live-kept on haiku and 14% on
sonnet, both inside the drop-everything null model's error bar -- shown a single output, a
model simply drops it. Shown ~15 together it reached 58% at the LOWEST cost per output, because
the overhead amortises and, more importantly, because comparative judgement beats absolute
judgement: ranking a dozen candidates against each other is a question a model can answer, "is
this one output expendable" is not.
`4ca1f13` was cited by the proposal as evidence that per-output "is a shape that has already
run". It says the opposite. It found a live arm at 1.02 verdicts per call and filed that as the
bug: "that is the per-output design refuted at 6% live-kept, not the bulk shape that measured
58%, so iteration 014 measured something other than what it claimed". It also added the
assertion whose absence let it through -- the prompt must offer more than one output, because
"asserting a single call was not enough, since one call carrying one item is exactly the
refuted design".
`cc1aa9f` gives the direction of the failure, and it is the one a SWEEP cannot tolerate: "at
batch 3-6 the model dropped a genuinely-spent output only 2 times in 4, at batch 10 it dropped
it 4 in 4 and cleared 100% of genuinely-spent candidates. Small batches do not make it wrong,
they make it UNWILLING TO ACT, which is what a 94.6% keep rate looks like from inside." A sweep
exists because the entire transcript is re-billing at the write rate; a timid adjudicator is an
expensive no-op exactly where the money is.
NEITHER OBJECTION TO BATCHING SURVIVES. Both were already solved on feat/coref-compaction:
* truncated shared replies -- `659e7a6` traced 24 of 34 unparseable replies to a 2048-token
output budget. Fixed here by a new optional components.Budgeter interface (same shape as
Remodeler: same endpoint, same credential, larger reply budget), which the sweep uses to ask
for extract.AdjudicationReplyTokens = 16000. Not a raise of cheapmodel.DefaultMaxTokens,
because the budget a caller needs is a property of what it ASKS FOR: a one-output compaction
reply and a twelve-verdict array are different lengths. Output bills as generated rather than
as budgeted, so the ceiling costs nothing until used. A client without the knob still works
and is counted (sweep_reply_budget_not_raised) rather than silently returning to truncation.
* quote fidelity decaying with batch size -- measured, and the cap is set below the observed
ceiling: 4 of 37 quotes non-verbatim at batch 16 against 0 of 16 at batch 10, so
MaxAdjudicationItems is 12, the conservative end.
And the transport principle never distinguished the shapes at all: post-trim, no verdict carries
content either way. That argument rules out rewriting; it says nothing about how many verdicts
travel in one reply.
STARVATION, which is the failure that made this component's predecessor lie about its own shape.
`4ca1f13`'s other finding is that the arm's real defect was an upstream per-candidate filter --
prefix_still_referenced removed 149,681 candidates, leaving about one per request. There is no
coref index on main, but the ECONOMIC GATE was the same mechanism, so it now runs ONCE for the
batch rather than per candidate. That is also the correct arithmetic: it compares an expected
saving against one CALL's cost, and a call now covers up to twelve candidates, so charging each
of them a whole call priced the batch at ~12x its real cost. The old candidate-level max_calls
truncation is gone for the same reason -- max_calls: 4 leaving four candidates is a batch of
four, the size at which the model was measured unwilling to act. It now bounds BATCH calls.
Every safety guarantee is unchanged, because all of them are per-VERDICT properties: the refusal
of a drop naming an obligation, unsure-defaults-to-keep, fabricated-quote counting with the
whitespace-insensitive retry, the shape descriptor that transports nothing, reversibility, and
the drop-replay-on-warm-turns freeze. The #119 gate discipline matters more, not less: batches
still fan out, so each call accumulates its own gate names and the serial phase raises them.
New counters, all in the reviewed surface: sweep_offered (via a new Report.GateN, because a
per-candidate loop cannot express "this many were SHOWN" -- the distinction that cost three
iterations when 2.80 verdicts/call was read as the batch size), sweep_batch_truncated,
sweep_batch_of_one, sweep_kept_whole_batch, sweep_verdict_unknown_label,
sweep_verdict_duplicate_label, sweep_verdict_missing, sweep_reply_budget_not_raised.
docs/proposals/sweep-adjudicator.md carries the correction rather than the wrong reasoning.
Guards, each verified to FAIL when its subject is reverted:
* the batch is offered as a batch. Chunking at 1 instead of MaxAdjudicationItems gives:
--- FAIL: TestSweepOffersTheWholeBatchInOneCall (0.13s)
extract_sweep_test.go:265: 12 candidates took 4 calls; the measured shape is ONE call over the batch
--- FAIL: TestSweepAccountsForEveryCandidateUnderConcurrency (0.38s)
extract_sweep_test.go:470: expected 5 concurrent batch calls, got 60 (gates: map[sweep_adjudicated:60 sweep_batch_of_one:60 ...])
* the economic gate does not thin the batch. Putting it back per candidate reproduces the
starvation exactly -- twelve candidates become a batch of two:
--- FAIL: TestSweepEconomicGateDoesNotThinTheBatch (0.14s)
extract_sweep_test.go:542: sweep_offered = 2, want 12: the gate thinned the batch one candidate at a time, which is how a bulk arm silently becomes the refuted per-output shape (gates: map[economic_gate:10 sweep_adjudicated:2 sweep_dropped:2 sweep_offered:2 ...])
* the reply budget is raised. Skipping the Budgeter call gives:
--- FAIL: TestSweepRaisesTheReplyBudget (0.08s)
extract_sweep_test.go:422: the sweep asked for a 0-token reply budget, want 16000
--- FAIL: TestSweepCountsAClientThatCannotRaiseItsBudget (0.00s)
extract_sweep_test.go:443: a client without a budget knob was not counted (gates: map[... sweep_kept_whole_batch:1 ...])
* a deliberate keep-all is not a failure. Folding the empty array back in gives:
--- FAIL: TestSweepCountsADeliberateKeepAllSeparatelyFromAFailure (0.10s)
extract_sweep_test.go:352: a deliberate keep-all was not counted as one (gates: map[... sweep_verdict_missing:1])
* truncation is not a format failure. Making ReplyWasTruncated return false gives:
--- FAIL: TestTruncationIsDistinguishedFromAFormatFailure (0.00s)
adjudicate_test.go:140: a reply that opened the array and never closed it must read as truncated
--- FAIL: TestSweepCountsADeliberateKeepAllSeparatelyFromAFailure (0.09s)
extract_sweep_test.go:369: a cut-off array was not counted as truncation (gates: map[... sweep_unparseable:1])
* a verdict for an unoffered label is never acted on. Removing the check does not merely act on
the wrong content, it CRASHES -- a model-supplied label indexes the candidate slice directly:
panic: runtime error: index out of range [99] with length 1
github.com/rossoctl/context-guru/components/offload.(*ExtractSweep).adjudicateBatch(...)
components/offload/extract_sweep.go:695
* bounded coverage is visible. Dropping the truncation counter gives:
--- FAIL: TestSweepBatchesPastTheItemCapAndCountsWhatItTruncated (0.30s)
extract_sweep_test.go:326: sweep_batch_truncated = 0, want 6 — bounded coverage must be visible
* a batch of one is counted. Removing the counter gives:
--- FAIL: TestSweepCountsABatchOfOne (0.09s)
extract_sweep_test.go:301: a batch of one was not counted; a starved batch would be invisible (gates: map[... sweep_kept:1 sweep_offered:1 ...])
* the contract asks for comparative judgement. Removing the paragraph gives:
--- FAIL: TestContractAsksForComparativeJudgementAndNoTransport (0.00s)
adjudicate_test.go:246: prompt does not carry "JUDGE THEM AGAINST EACH OTHER"
The economic-gate tests run with the gate ON, which the rest of the file turns off. Without them
the whole gated path was untested and the per-candidate reversion passed the suite -- found by
running that reversion and watching nothing fail.
BREAKING: `max_calls` now bounds batch calls rather than per-output calls.
Suite: 27 packages, 0 failures. Also verified race-clean under -race.
Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com>
…wn component now
Finishes the split. extract_llm is unconditionally the warm/tail compactor: `per_output` and
the whole `cold_cache` block are gone, along with every `sweeping` branch they governed --
the cadence and pressure exemptions, the sweep's own floor, the tail-gate lift, the
request-trigger carve-out, the separate cap arm, the one-writer-then-readers call ordering,
and the AUTO file-read exception. Roughly twenty conditionals that each existed to make one
component behave as two.
BREAKING, deliberately: there is one deployment and it is migrated by hand. But a removed key
must say where it went, so `per_output` and `cold_cache` are REFUSED with an error naming the
replacement rather than falling through to KnownFields' "field not found" -- which reads as a
typo rather than a relocation. `cold_cache: {enabled: true}` silently accepted would read as
"the sweep is on" while nothing swept, and that is the most expensive available misreading of
this config: the sweep exists for the turns measured at 4% of requests and 31% of spend.
MODEL ESCALATION MOVED RATHER THAN DYING. It was already guarded on `sweeping`, so removing
the sweep would have left it unreachable -- and it protects exactly the case that matters
more now: a batch's fixed prompt cost is larger than a single output's, so a small
adjudication model's window is easier to exceed, and when fitsModelContext declines every
candidate the sweep silently does nothing on the largest, most expensive transcripts. It now
lives in extract_llm_sweep, with its pricing and call-model re-derivation intact and a
sweep_escalated_to_agent_model counter.
MIGRATED, not just compiled:
* the `housellm` preset -- the live deployment's configuration -- gains extract_llm_sweep
after extract_llm, carrying cold_cache.min_tokens 1000 as its own min_tokens. Its comment
keeps the measurements that justify the floor and now says plainly that they describe the
value of sweeping this workload, not the yield of the adjudicator that now does it.
* config/form.go loses its ONE per-component coupling. applyExtractLLMCoupling existed only
because the constructor refused `per_output: false` with the sweep off, so the form had to
translate two switches into pipeline membership; the sweep is a component now and follows
the ordinary rule. The recommended prefill serves two blocks.
* dash/ui/app.js loses XLLM_SWITCHES and the checkbox handler that flipped one switch back
on -- a checkbox meaning two different things is what the split removed.
* proxy/control_test.go, proxy/optionsfields_test.go, config/form_test.go,
components/all/xglobal_test.go, apply/sweep_variants_test.go and the docs.
TWO TESTS WERE POINTED AT THE WRONG COMPONENT AND WOULD HAVE KEPT PASSING.
TestHousellmColdSweepActuallyFires read extract_llm's preset block; left alone it would have
asserted that a component which no longer sweeps still sweeps -- the same "configured into a
no-op while looking enabled" failure it was written to catch, one level up. It now reads the
extract_llm_sweep block, answers with a verdict array instead of a Starlark program, and also
asserts sweep_dropped, because a call that adjudicates and removes nothing is the same no-op
from the operator's side. The options-fields guard asserted that a nested key is served as one
dotted field using cold_cache.min_tokens; it now uses trigger.min_request_tokens and
additionally checks that the sweep's own fields reach the page at all and that the compaction
knobs do NOT -- a field for a key the constructor rejects is a control whose only behaviour is
to fail the save.
extract_cold_test.go is reduced to what is not about the sweep component: the pricing of a
cold turn, which extract_llm still sees, and the refusal of the moved keys. Everything else it
covered -- depth, the floor, the cap, min_idle_seconds, the context mode, not draining the hot
path's budget -- is tested against extract_llm_sweep, where it is exercised through the shape
that was measured good rather than through a compaction pass pointed at deep history.
Verified to FAIL when reverted. Removing the movedToSweep probe leaves the generic yaml
message:
--- FAIL: TestKeysThatMovedToTheSweepAreRefusedByName/per_output (0.00s)
extract_cold_test.go:68: error does not name the component the key moved to: yaml: unmarshal errors:
line 1: field per_output not found in type offload.extractLLMConfig
extract_cold_test.go:71: error does not say what per_output becomes (want mention of "warm/tail pass"): yaml: unmarshal errors:
line 1: field per_output not found in type offload.extractLLMConfig
--- FAIL: TestKeysThatMovedToTheSweepAreRefusedByName/cold_cache (0.00s)
extract_cold_test.go:68: error does not name the component the key moved to: yaml: unmarshal errors:
line 1: field cold_cache not found in type offload.extractLLMConfig
which is exactly the reading this guard exists to prevent: the key looks misspelled rather
than moved.
Also adds docs/components/extract_llm_sweep.md, including the three questions the design
records rather than answers.
Suite: 27 packages, 0 failures. gofmt clean.
Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com>
… and order the calls only where the write is earnable
Found in review. The sweep launched every batch concurrently under one semaphore with no cache
coordination, so each sent the same contract and conversation context as fresh input and none
could read another's write. Three separate things were wrong, and the first one made the other
two unreachable.
THERE WAS NO BREAKPOINT AT ALL. The component called Model.Complete, which routes to
CompleteSystem(ctx, "", prompt) and thence to CompleteBlocks(ctx, nil, prompt) -- no system
field, so systemBlocks placed no cache_control mark and there was nothing for a sibling batch
to read. Not a missed optimisation: a request field that was never sent.
The prompt is now SPLIT. The contract is a system block, invariant across every request and
tenant. The goal is a second system block when there is more than one batch to share it, and
stays in the user half otherwise -- with one batch there is nothing to read the entry and a
write costs 1.25x fresh, so marking it would be a 25% loss. THE CANDIDATES ARE NEVER IN THE
PREFIX: they differ per batch, so an entry containing them could never be read, which is
strictly worse than no breakpoint. Routed through extract.AskAdjudication so completeSplit
stays unexported and the decision about which half is cacheable lives next to the contract it
splits.
THE CALLS NOW EARN THE WRITE BEFORE READING IT. cheapmodel.claimCacheWrite deliberately
withholds the breakpoint from concurrent siblings -- an entry only ever written is worse than
none -- so with every batch in flight at once, sharing could not happen even in principle. The
first batch runs alone, then the rest concurrently, the same mechanism extract_llm uses. It
could not reuse extCfg.CacheContext/serialFirst directly, because the adjudication path builds
no extract.Cfg; the mechanism is mirrored explicitly instead.
AND IT IS CONDITIONAL, WHICH IS THE PART THE MEASUREMENT DECIDED. A cache_control below the
provider's minimum cacheable prefix is silently ignored -- no error,
cache_creation_input_tokens: 0 -- and the minimum is 4,096 provider tokens on haiku-class
against 1,024 on sonnet-class. MEASURED with internal/tokens:
adjudication contract 504 o200k
contract + two-message `recent` context ~537 o200k
haiku-class floor 3,413 o200k (needs ~2,900 tokens of conversation)
sonnet-class floor 853 o200k (needs ~349, which a real context supplies)
unnameable gateway alias haiku-class, minCacheablePrefix's own default
So SHARING IS REACHABLE ON SONNET-CLASS AND PROVABLY CANNOT WORK ON HAIKU-CLASS at
`context: recent` -- and housellm pins claude-haiku-4-5. Serializing the first batch costs a
whole gateway queue round (~2-4 s p50, tail 12-16 s), so paying it for a write the provider
will refuse is strictly a loss. The component reads its own prefix size through
extract.AdjudicationPrefixTokens, asks cheapmodel.CacheablePrefix, and serializes only when the
answer is yes. On haiku it stays fully concurrent -- which is what it already did, but now for
a reason that is checked rather than by accident.
The asymmetry is recorded rather than resolved silently. `context: full` would clear haiku's
floor and is NOT adopted: that is proposal open question 2, unmeasured, and chasing a cache is
the wrong reason to change what the model is shown. Both outcomes are counted instead:
sweep_prefix_uncacheable when the floor cannot be cleared, and sweep_prefix_cache_read_ZERO when
a sibling read nothing despite the ordering -- that second failure is indistinguishable from a
working one except on the bill, which is why it needs a counter and not a comment.
max_calls DEFAULT STANDS, and now on stated arithmetic rather than an assumption I made and you
endorsed. Three extra batches duplicate ~1,600 input tokens: a fraction of a cent on haiku, and
about 4% of ONE batch's own body (twelve candidates bounded at 4,000 chars). The prefix is not
where this component's money is, by construction -- the transcript is deliberately not in it.
That is the difference from extract_llm's `context: full` sweep, where the prefix WAS the
transcript at ~138,000 tokens and duplicating it across five calls was the entire defect. The
comment on MaxCalls now says this in place of the bare extrapolation.
Guards, each verified to FAIL when its subject is reverted:
* the contract is sent as a cacheable system prefix. Reverting to Model.Complete gives:
--- FAIL: TestSweepSerializesTheFirstBatchWhenThePrefixIsCacheable (0.47s)
extract_sweep_cache_test.go:127: only 0 system block(s): the goal did not join the cacheable prefix, so siblings have nothing to read
--- FAIL: TestSweepDoesNotCacheTheGoalForASingleBatch (0.00s)
extract_sweep_cache_test.go:178: a single-batch sweep sent 0 system blocks; the goal must stay in the user half so no 1.25x write is paid for an entry nothing reads
* the candidates stay out of the prefix. Moving them into it gives:
--- FAIL: TestAdjudicationSendsTheContractAsACacheableSystemPrefix (0.00s)
adjudicate_cache_test.go:62: output 0 is inside the cacheable prefix; no sibling batch could ever read it
adjudicate_cache_test.go:66: output 0 is not in the user half, so the model was never shown it
* the first batch is serialized where the write is earnable. Removing the ordering gives:
--- FAIL: TestSweepSerializesTheFirstBatchWhenThePrefixIsCacheable (0.46s)
extract_sweep_cache_test.go:131: 3 calls had started when the first returned: the first batch did not run alone, so claimCacheWrite suppressed its breakpoint and no sibling can read a write
* and NOT serialized where it is not. Serializing unconditionally gives:
--- FAIL: TestSweepStaysConcurrentAndCountsItWhenThePrefixCannotBeCached (0.51s)
extract_sweep_cache_test.go:157: the first batch was serialized on a model that cannot cache the prefix: the queue round bought nothing (started at first return = 1)
* the uncacheable outcome is visible. Removing the counter gives:
--- FAIL: TestSweepStaysConcurrentAndCountsItWhenThePrefixCannotBeCached (0.40s)
extract_sweep_cache_test.go:153: haiku-class cannot cache this prefix and the component did not record it (gates: map[sweep_adjudicated:36 sweep_kept:36 sweep_offered:36 sweep_prefix_cache_read_ZERO:2 ...])
* a sibling that read nothing is counted. Removing that counter gives:
--- FAIL: TestSweepCountsASiblingThatReadNothingFromCache (0.39s)
extract_sweep_cache_test.go:208: sweep_prefix_cache_read_ZERO = 0, want 2: a sibling paying fresh for the shared prefix would be invisible
* a single batch pays for no cache write. Forcing cacheContext on gives:
--- FAIL: TestSweepDoesNotCacheTheGoalForASingleBatch (0.06s)
extract_sweep_cache_test.go:178: a single-batch sweep sent 2 system blocks; the goal must stay in the user half so no 1.25x write is paid for an entry nothing reads
The ordering test asserts three PRECONDITIONS before its subject -- that there were sibling
batches at all, that the prefix was judged cacheable, and that the split really produced two
system blocks -- because each of those failing would make "the first batch ran alone" true for
the wrong reason. The prefix-size test is written so that a contract which grows past a floor it
currently says it does not clear FAILS there, since that would be a real change in this
component's economics and this is where it should be noticed.
Suite: 27 packages, 0 failures. gofmt clean.
Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com>
…over its cached transcript Groundwork for the sweep redesign, landed on its own because it is verifiable on its own and because the sweep rewrite that consumes it is a separate change. A component that must decide whether a tool output is still needed cannot answer that from the output alone: need is relevance MINUS whatever has already been captured elsewhere in the transcript, and that second term lives in the later turns. Sending those turns fresh costs ~10x a cache read, and on a cheap model the required verbatim quoting degraded to 20.8% at bulk batch sizes against 0 of 59 on the request model. So the judgement wants the agent's OWN model AND the whole transcript, and only a cache read makes that affordable. Ported from a9d666f on feat/coref-compaction rather than reimplemented: * components: PrefixUsage and PrefixAsker, plus Ctx.PrefixAsk. Usage is RETURNED and not merely recorded, because a caller whose whole justification is the cache read has to be able to gate on whether the read happened -- which a metrics counter cannot support. * cheapmodel: Anthropic.CompletePrefixed, which appends the ask as a trailing user message and touches nothing else except `stream`, since every byte before the appended message is prefix. * proxy: a bounded per-session stash of the body actually forwarded, and the asker built from it. * apply: Opts.PrefixAsk through to the Ctx. THREE FACTS FROM THE ORIGINAL MEASUREMENT that are easy to lose and are now each a test: appending to a byte-identical prefix reads the whole prefix and writes nothing (19,595 read / 0 created); `tool_choice` is NOT in the cache key so forcing it to none is free AND necessary, or the prefix's tools make the model answer with a tool_use; `tools` ARE in the key, so stripping them reads a different, smaller entry. The route also rejects assistant prefill, which the appended user message satisfies by construction. THE PREFIX IS THE PREVIOUS TURN'S SENT BODY, not the incoming one. The upstream cache was populated by what context-guru emitted, i.e. the compacted form; the incoming body is uncompacted and diverges at the first thing any component removed, making everything past that point a fresh charge. Consequence, stated in the doc comment rather than left to be rediscovered: the ask sees the transcript as of the previous turn, so the newest tool output is invisible to it -- acceptable here, because tail content has had no turns in which to be superseded and would be kept anyway. Three deliberate differences from the branch this comes from: * ON BY DEFAULT, no CONTEXT_GURU_PREFIX_ASK gate. It was off there because "a feature whose benefit is a cache hit should not be on by default in a host that cannot verify the hit". We CAN verify it -- that is what returning PrefixUsage is for -- so there is nothing for the operator to opt in to. The other reason it was off, that it holds request bodies in memory, is a bound to state rather than a reason to disable: prefixask.go now documents what happens at each cap (an oversized body is not stashed; past the session or byte cap the whole stash is dropped), and the worst case is a declined sweep, never a wrong answer. * KEYED BY THE SCOPED SESSION ID. serve already receives tr.Session, which is exactly what a component reads as Ctx.Session. Keying the stash by the raw header instead would make every Ask miss while the mechanism looked switched on -- the failure mode the original's own comment warns about. * Ctx.CacheTTLMs carries the cache lifetime the cold decision already derives (5 minutes for a bare ephemeral mark, an hour for an explicit ttl, widened to the longest this prefix ever asked for). Added here because the sweep's trigger has to reason about where in the cache's LIFETIME a turn falls, not merely whether the entry is gone -- and re-deriving it in the component would be a second read of one fact, which is how the cold decision and the dashboard came to disagree once already. CompletePrefixed also documents what the cached region IS here, because it is not what the rest of that file deals with: systemBlocks places our own breakpoint and is bound by the provider's minimum cacheable prefix (4,096 tokens on haiku-class), while this places none at all -- the marks are the agent's own and the region is the transcript, which clears any floor by orders of magnitude. The model-family asymmetry that governs systemBlocks does not apply. Guards, each verified to FAIL when its subject is reverted: * tool_choice forced to none: --- FAIL: TestCompletePrefixedAppendsWithoutDisturbingThePrefix (0.00s) prefixask_test.go:127: tool_choice is not forced to none, so the model will answer with a tool_use: map[] * tools left exactly as the prefix had them: prefixask_test.go:117: tools were not preserved exactly; they are part of the cache key: [] * the ask appended as a USER message: prefixask_test.go:105: the ask was appended as role "assistant"; this route rejects assistant prefill * usage returned so the caller can gate on it: prefixask_test.go:82: CacheRead = 0, want the provider's figure — the caller gates on it --- FAIL: TestAskUsesTheStashedBodyForThatSession (0.00s) prefixask_test.go:179: CacheRead = 0; the caller cannot gate on a figure that does not arrive * a missing prefix is an ERROR, not an empty answer: --- FAIL: TestAskWithoutAStashedBodyReportsNoPrefix (0.00s) prefixask_test.go:157: a missing prefix was not reported as an error prefixask_test.go:190: session 2 got session 1's prefix (err=<nil>) * Anthropic-only: --- FAIL: TestPrefixAskerIsAnthropicOnlyAndNeedsTheIncomingClient (0.00s) prefixask_test.go:219: an OpenAI route got an asker; its cache semantics are not the measured ones * the per-body cap binds: --- FAIL: TestAnOversizedBodyIsNotStashed (0.00s) prefixask_test.go:200: a 1500001-byte body was stashed past the 1500000 cap No component consumes any of this yet, so behaviour is unchanged: the asker is built and handed to the pipeline, and nothing asks. Suite: 27 packages, 0 failures. Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com>
… drop the batching Supersedes the batching. The sweep no longer copies candidate content into any prompt: it asks the REQUEST's own model, over the transcript already in that model's prompt cache, and ships an INVENTORY of candidates rather than their content. One call for all of them. WHY. Three measurements, from a9d666f: * appending a trailing user message to a byte-identical prefix read 19,595 tokens from cache and created 0, on the live route. * verbatim quoting -- the only remaining signal that the model is inventing -- degraded to 20.8% on the cheap model at bulk batch sizes, against 0 of 59 on the request model. Since a fabricated quote is the only check left on this design, that alone settles which model is asked. * need is relevance MINUS what has already been captured elsewhere, and that second term lives in the later turns, which a prompt carrying only the candidate cannot show. REMOVED, because they existed only to bound copied content: the batch assembler, the 12-item cap, per-batch concurrency, max_calls, sweep_batch_of_one and batch-truncation counting. Also the sibling-ordering machinery from 316ead4 -- CacheablePrefix-conditional serialization, AdjudicationPrefixTokens and sweep_prefix_uncacheable -- since with one call there are no siblings to order and no floor to clear. The haiku/sonnet minimum-prefix asymmetry is moot: the cached region is now the whole transcript, which clears any minimum by orders of magnitude, and CompletePrefixed's comment records that explicitly. THE TRIGGER IS PRE-EXPIRY, NOT COLD, and that resolves a real contradiction rather than tuning one. The ask needs a WARM cache to read; the removal wants a COLD one so there is no live prefix to invalidate. Both are cheap in the window where the entry still exists but has little life left, so the sweep fires when 0 < remaining <= pre_expiry_seconds. The TTL is DERIVED, never assumed: Ctx.CacheTTLMs is the same figure apply's cold decision uses, read out of the request (5 minutes for a bare ephemeral mark, an hour for an explicit ttl, widened to the longest this prefix ever asked for). Unknown does not fire. THE WINDOW'S WIDTH IS THE ONE UNMEASURED NUMBER, and it is flagged as such in the code, the component doc and the proposal. One minute is apply.coldMargin, the only figure here with a stated purpose for clock uncertainty around cache expiry. Wider fires more often and invalidates more remaining TTL; narrower fires rarely. Nothing measures either side, so it is configurable and deliberately narrow rather than tuned. THE CACHE READ IS VERIFIED AND ALWAYS COUNTED; the miss then FALLS BACK by default. sweep_prefix_cache_read_ZERO fires whenever the read did not happen -- that part is not optional, because a silent miss looks identical to a working call except on the bill. By default the sweep then asks again with a bounded sample of each output, which is what a9d666f chose: treating "no prefix" as "no verdicts" would disable the component on every session's FIRST turn and read as a model that declined to act. `block_fallback: true` declines instead, for an operator who would rather forgo the yield than pay for it. The fallback still asks the REQUEST's model, because the measurement that chose that model is about faithful quoting rather than caching, and that reason survives the loss of the cache read. THE MODEL IS NOT A FREE CHOICE HERE, and the asymmetry with extract_llm is spelled out where a reader will hit it. extract_llm may compact with any model because its prompt CARRIES the output; this component's prompt carries an inventory and the outputs are read from the prompt cache of the model being asked. Only the request's model has that cache, so `model.source: config` is incoherent rather than merely suboptimal -- the ask would read nothing and pay fresh for the entire transcript. A `model` block is refused with an error saying exactly that, not accepted and silently corrected. KEPT, all of it: the obligation refusal, unsure-defaults-to-keep, fabricated-quote counting with the whitespace-insensitive retry, the zero-transport descriptor with its record count, reversibility, decision freezing and replay, integer labels, the unoffered-label guard, and the per-slot gate accumulation from #119. The reply budget is now cheapmodel.PrefixAskMaxTokens and matters MORE than at batch 12, since one reply carries a verdict for every candidate. Guards, each verified to FAIL when its subject is reverted: * the fallback is the default. Declining instead gives: --- FAIL: TestSweepFallsBackWhenTheCacheReadDidNotHappen (0.05s) extract_sweep_test.go:232: the default did not fall back, so the component stops working on a first turn (gates: map[below_output_floor:1 sweep_inventory_of_one:1 sweep_prefix_cache_read_ZERO:1]) * block_fallback refuses it. Ignoring the switch gives: --- FAIL: TestBlockFallbackDeclinesInsteadOfPaying (0.05s) extract_sweep_test.go:269: block_fallback did not refuse the fallback (gates: map[... sweep_fallback_used:1 ... sweep_prefix_cache_read_ZERO:1]) --- FAIL: TestSweepFallsBackWithNoAsker (0.00s) extract_sweep_test.go:311: block_fallback did not refuse the no-asker fallback (gates: map[... sweep_fallback_used:1 ...]) * the missed read is counted in BOTH modes. Removing the counter gives: --- FAIL: TestSweepFallsBackWhenTheCacheReadDidNotHappen (0.05s) extract_sweep_test.go:228: a zero cache read was not counted; the mechanism's failure would be invisible --- FAIL: TestBlockFallbackDeclinesInsteadOfPaying (0.00s) extract_sweep_test.go:266: strict mode did not count the missed read (gates: map[below_output_floor:1 sweep_fallback_blocked:1 sweep_inventory_of_one:1]) * a model block is refused with its reason. Letting it through leaves the generic yaml message: --- FAIL: TestSweepRefusesAModelBlockAndSaysWhy (0.00s) extract_sweep_test.go:329: the error does not mention "source: config", so the constraint reads as an oversight: yaml: unmarshal errors: line 1: field model not found in type offload.extractSweepConfig * the pre-expiry window governs. Reverting to the cold gate gives: --- FAIL: TestSweepFiresOnlyInThePreExpiryWindow/inside_the_window (0.00s) extract_sweep_test.go:293: sweeping = false, want true (idle=240000ms ttl=300000ms cold=false) --- FAIL: TestSweepFiresOnlyInThePreExpiryWindow/apply_already_called_it_cold (0.00s) extract_sweep_test.go:293: sweeping = true, want false (idle=240000ms ttl=300000ms cold=true) * the inventory ships no output content. Putting the body back in the ask gives: --- FAIL: TestSweepSendsTheInventoryAndNotTheOutputs (0.05s) extract_sweep_test.go:603: the whole output body was copied into the ask, defeating the mechanism extract_sweep_test.go:607: the ask is 38031 chars; it should be the contract plus one line per candidate * the unoffered-label guard prevents a crash on model input, not merely a wrong action. Removing it gives: panic: runtime error: index out of range [99] with length 1 offload.(*ExtractSweep).adjudicate(...) components/offload/extract_sweep.go:502 * a nil asker is handled rather than dereferenced: panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x18] ALSO FIXES A CORRUPTED FILE I COMMITTED IN 5d02669. config/form_test.go lost its first 60 bytes to a bad base64 round-trip, so the config package did not compile on the pushed branch even though the suite passed on the eval box -- I validated remotely and committed from a stale local copy. The header is reconstructed and every changed file's sha256 is now checked against the box before committing. docs/proposals/sweep-adjudicator.md carries its second correction and now describes the code rather than preceding it; docs/components/extract_llm_sweep.md is rewritten for the new mechanism. Suite: 27 packages, 0 failures. gofmt clean. Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com>
… coref seam Two changes, both about what the next person needs rather than about behaviour today. THE tool_use ID IS BACK IN THE INVENTORY, as a LOCATING field. Dropping it went one step past the evidence: a9d666f establishes that the ANSWER key must be an integer -- asked to answer with opaque ids the model regularised them, `toolu_01..07` for `toolu_probe_00..07`, against 0 bad labels in 40+ trials with integers -- and says nothing about whether an id may appear in the prompt. The echo risk was always about what the model RETURNS, not what it reads. The cost of absence was the one EXACT anchor there is. The id is also in the transcript the model reads from cache, so shipping it makes an inventory line and a tool result identifiable to each other; without it head-plus-size is the only matching signal, which is shaky on a transcript carrying a dozen near-identical `Read` results. The two aids now have separated roles in the code and in the contract: the label is what the model answers with, the id and the head are what it locates with, and the prompt says so outright ("ANSWER BY LABEL ... do not put it in your reply"). The unoffered-label guard's comment now records what it CANNOT catch: a label that is in range but wrong removes the wrong content and looks valid from there, and nothing downstream can detect it either. That is the failure the id exists to PREVENT rather than to detect, which is the only defence available against a plausible-but-wrong label. THE COREF SEAM IS LEFT CLEAN AND TRIPWIRED, because PR #80 rebases onto this branch and brings the index's evidence signals and index-driven candidate selection with it. * AdjudicationItem.Evidence renders into the inventory line when non-empty and is omitted entirely when not, so the index can arrive without reshaping the contract -- and the contract is the part with measurements attached. Nothing populates it on main, and the prompt deliberately says nothing about how to read one: teaching the model to interpret counters the prompt never carries would be teaching it to read a field that does not exist. * sweep_inventory_thinned, AT the candidate-gathering site rather than only in the doc. 4ca1f13's real defect was a per-candidate PRE-FILTER sitting exactly there: prefix_still_referenced removed 149,681 candidates and left about one per request, silently turning a bulk arm into the per-output shape refuted at 6% live-kept while the arm reported itself as bulk throughout. It was self-defeating twice over -- it starved the comparison, and it meant the model only ever saw what the index had ALREADY judged spent, destroying the veto the mechanism exists to provide. main has no such thinner, so `eligible` and the inventory size are equal by construction and the counter cannot fire today. That is the point: a filter added between the eligible++ and the append trips it on its first request, and the comment says where the index's verdict belongs instead (as EVIDENCE for the model to weigh, never as a gate that pre-decides). * sweep_offered is restored -- it was lost in the one-call rewrite. A per-candidate loop cannot express "this many were SHOWN", and a live arm once reported 2.80 verdicts per call read as the batch size when it counted what the model ANSWERED. Without it, "the inventory is starved" and "the model answered for a third of it" are the same number. Both docs gain the caveat the rebase makes necessary: the 58%-live-kept figure justifying batch-style adjudication was measured WITH the index supplying evidence per candidate. On main the model reasons from the transcript alone -- plausibly better, since it is not limited to exact matches and the index's documented blind spot was transformed reuse, but UNTESTED. It is now listed as the second thing a measurement should settle, after the pre-expiry window's width. Tests, both directions on each property: the id appears in the ask and the answer key is stated; evidence does NOT render empty and DOES render when populated; the tripwire reads zero on main and its arithmetic counts a thinned inventory when one is simulated. Suite: 27 packages, 0 failures. gofmt clean. Every changed file's sha256 verified against the eval box before committing. Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com>
…s are declined Three defects found by live verification against the real gateway, two of them mine. CANDIDATES ARE THE ENTIRE TRANSCRIPT (#122). The depth permission was keyed on the cache having ALREADY expired -- TailOnlyCold's `optIn && c.ColdCache` -- which the pre-expiry window makes false by construction. So candidates collapsed to the uncached tail while the prefix ask reads the previous turn's sent body, everything UP TO the cached boundary. Two disjoint sets: the model was asked to judge outputs the transcript it reads does not contain. Live, that is not subtle. One turn kept two outputs citing "Reply with the word ACK only." as the obligation for each -- a real transcript string, so no fabrication counter fired, but not a judgement about the outputs; it was the only obligation available. Another DROPPED an output having seen `begins: # ledger_b` and a token count. The trigger change is what caused it, and the objection the old comment raised against itself -- that lifting the gate on a merely-nearly-expired prefix needs a measurement of early invalidation -- does not hold: inside the window the prefix has at most the window's width left to live, so the cost is bounded by that width, not by the TTL. The window is what BUYS the permission, so keying the permission on ColdCache withdrew it exactly where it had just been paid for. Counted positively as sweep_candidate_at_depth: a refusal counter cannot distinguish "the gate refused depth" from "nothing was deep this turn". DECLINE BELOW AN INVENTORY FLOOR, default 10. The yield is a property of how many candidates the model COMPARES, and the figures are far apart: shown one output, 6% live-kept on haiku and 14% on sonnet, both inside the drop-everything null model's error bar; ~15 together, 58% at the lowest cost per output; batch 3-6 dropped a genuinely-spent output 2 times in 4; batch 10, 4 in 4. Ten is that inflection, not a round number. Below it a drop is a guess, and the errors are not symmetric -- a wrong keep costs one turn's tokens, a wrong drop costs content the agent still needs -- so declining beats asking. Counted, because a silent decline is indistinguishable from a broken component. This was not hypothetical either: sweep_inventory_of_one fired on the ORDINARY agent turn, because the tail of a normal turn is one tool result, and the component dropped on it. THE ANCHOR SHIPPED THE WRONG STRING (#123). The inventory announced `tool_use id 300c312d1492952219bfb1c4` -- extract.ContentKey, our own store key -- while the real id in that transcript was `toolu_d2`. The contract tells the model the id is "shown only so you can find the output in the conversation above", so a key that appears nowhere in the conversation is worse than no anchor: it directs the model to look something up that cannot be found. Now a separate `toolID` field lifted from ChatToolMessage.ToolCallID; `id` stays the store key. Empty when the dialect carries no id, in which case nothing is claimed. WHY NO TEST CAUGHT ANY OF IT. All 20 sweep tests ran `MaxCachedIdx: -1`, which disables the tail gate, so the disjointness could not appear -- and extract_sweep_test.go claimed the gate "is exercised separately below" when no such test existed. The anchor test hard-coded `ID: "toolu_abc123"` into an AdjudicationItem and asserted only that the prompt rendered whatever was in the field, which passes on any string including one from nowhere. The new test starts from a REQUEST and asserts provenance instead. Three tests, each verified to fail with its subject reverted: depth sweep_offered = 4, want 12 ... gates=map[cached_prefix:8 sweep_inventory_below_min:4 sweep_offered:4] -- the defect and the two fixes interacting in one line: tail-only starves the inventory, the floor then declines it floor 4 candidates: asked=true, want false ... sweep_inventory_below_min = 0, want 4 anchor the ask does not carry the wire id "toolu_wire_0", so the locating anchor names nothing the model can find in the transcript Fixture consequences, which are findings rather than churn. Every existing sweep test asserted counts against a single above-floor output -- an inventory of one, the shape the floor now declines -- so they opt in explicitly via newSweepSmall, and the comment says why a test may run there while a deployment should not. sweep_inventory_of_one is unreachable under the default now, so its test lowers the floor and the counter's job changes to "you asked for this, here is what it costs". The housellm guard gets ten candidates: the smallest transcript the shipped preset will act on. Stocking the shared fixture instead was tried and reverted -- it shifted four count-exact tests, which is how a fixture change becomes a cascade. Also corrects docs/hosted.md, wrong on five counts: it described both components calling a cheap model behind the economic gate, the sweep as firing only after expiry, adjudicating "at depth" via a batch, when the sweep asks the REQUEST's model in one call over its cached transcript, applies no economic gate, and fires before expiry. Full suite: 27 packages, 0 failures, gofmt clean. Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com>
|
All three findings from live verification are fixed in
Three tests, each revert-verified. The depth one is worth quoting because it shows the defect and both fixes interacting in a single line: Tail-only starves the inventory to 4, and the floor then declines it — which is precisely why a floor without #122 would have made the component inert rather than safe. Suite: 27 packages, 0 failures, gofmt clean, all five changed files sha256-verified local↔remote before committing. This PR still should not be marked readyThe live verification has to be re-run. Everything the last run observed was measured against tail-only candidates — the cache read, the drop end to end, the inventory sizes, the replay. The mechanism it exercised is not the mechanism now on the branch, so that evidence no longer describes this code. In particular the two things most worth knowing are now different questions: whether inventories in real traffic clear the floor of 10 (they could not before, since the tail of an ordinary turn is one output), and whether the model's judgement changes when the outputs it is judging are actually inside the transcript it reads — which on the old shape they never were. Two of the three fixes were mine to cause: I asked for the pre-expiry trigger without checking what else keyed off Also worth carrying into the re-runThe last run's report flagged that |
…#121) `cg_component_gate_declines_total{component,gate}` carried things that are not declines. A cache hit (`reapplied_same_session`), an output removed (`sweep_dropped`), an inventory offered (`sweep_offered`) all landed there, so the series ROSE AS A COMPONENT WORKED BETTER and anyone summing it to judge whether the pipeline was doing anything read the wrong sign. Split at the API rather than in the exporter. `Report.Event`/`EventN` write a second map, `Report.Events`, and the call site says which one it is. The alternative — classifying names inside promexport — puts the judgement where the author is not, and goes stale the next time a gate is added; there are 58 gate names across 31 files and no way to enumerate them statically, so a lookup table would have been wrong within a release. The axis is one question: did the component ACT, or did it turn a candidate away. Eleven names move — reapplied_same_session, reapplied_cross_session, model_source_fell_back_to_config, sweep_adjudicated, sweep_dropped, sweep_offered, sweep_candidate_at_depth, sweep_prefix_cache_read_ok, sweep_fallback_used, sweep_inventory_of_one, sweep_inventory_thinned. Everything meaning "we did not act on this candidate" stays a decline, including the ones that look like observations but resolve toward keep: sweep_kept, sweep_kept_everything, sweep_quote_fabricated, sweep_drop_refused_obligation. `sweep_inventory_below_min` stays too — declining to ask IS a decline. BOTH ENDS ARE IN THIS REPO, which is why no alias or deprecation window is needed: the exporter is proxy/promexport.go and the only consumers are the Grafana dashboards under deploy/grafana. Both are updated here. There is no external scraper to strand, which is the whole reason this could be fixed bluntly instead of carried. * components: Report.Events, Event(), EventN(), with the same nil-safety and non-positive-N behaviour as Gate/GateN. * metrics: compStat.Events, addEvents() as a SEPARATE method rather than addGates with a different target — one function writing to whichever map it is handed is how the two would come back together. Deep-copied in forSnapshot beside Gates, in the same place, for the reason recorded there: a shallow copy once handed the live map to /stats and raced the observe worker pool. * proxy: cg_component_events_total{component,event}, bounded the same way declines are. * grafana + docs/reference/routes.md: the declines panel says it carries declines only and names where the rest went. Two tests. TestGatesAndEventsAreDisjoint pins the invariant that makes the split meaningful — a name in both maps would mean the component cannot say whether the thing succeeded or was refused, and a consumer summing either series would double-count it. TestEventMatchesGateOnEdgeCases pins that Event did not quietly acquire different nil/zero behaviour from Gate. The counter-contract test now looks a name up across both histograms, deliberately: what it pins is that the component raises the contracted NAME, while which series it lands in is the exporter's contract (proxy/sweep_counters_test.go) and disjointness is the components one. Asserting all three in one fixture would make it fail for unrelated reasons. NOT fixed here, and left as it was: `cg_extract_gate_declines_total{reason}` is a different series with its own vocabulary, and nothing in it is a success. Full suite: 27 packages, 0 failures, gofmt clean. Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com>
…nted id (#111) When a marker id resolves to nothing the model reads Unavailable() either way, and both causes were collapsed into that one placeholder with no counter. They are not the same event and they need opposite responses: MALFORMED the model invented or corrupted the id. Nothing to fix; the placeholder is the correct answer. MISSING an id THIS PROXY COULD HAVE MINTED resolved to nothing. That is a context-guru defect -- a cut advertised as reversible was not, because the stash expired, the store did not persist, or the key was written under a session id nothing reads. Only the second is alertable, and nothing already in /stats could stand in for it: wasted_tokens counts tokens successfully re-served and sse_expand_after_stream counts a streaming miss, so neither can go non-zero for a broken stash. A silent no-op therefore read exactly like a session that never called expand -- which is how expand refusals ran unnoticed for three iterations, found by grepping a benchmark client's transcripts rather than from any counter in this repo. THE DESIGN QUESTION #111 RAISED, AND WHY THE ANSWER TURNED OUT TO BE CHEAP. The issue listed three ways to decide "well-formed" and worried that shape-validation couples to the hashing scheme. Two facts settle it. store.Get returns (bytes, bool) and cannot distinguish absent from expired, so classifying by store outcome -- the option that looked closest to free -- separates nothing. And making ids self-identifying with an HMAC would be exact but rewrites every marker, and marker bytes are prefix-cache-relevant: a different marker text on a later turn is a full prefix miss (inject.go). So it is shape validation, and the coupling is made safe rather than argued away. Every stash key that reaches a <<cg:HASH>> marker is a lowercase-hex sha256 prefix of length 16 (offload.hashKey, summarize's span stash) or 24 (extract.ContentKey, offload's state key). TestEveryMintedKeyIsWellFormed drives the REAL minters and asserts WellFormedID accepts what they produce, so a minter changing shape fails there instead of quietly downgrading a defect signal. That direction is the one that matters and the tests pin it explicitly: a check that is too NARROW blames us for nothing and the model for everything, which zeroes the alertable counter. Verified by dropping 24 from the accepted lengths -- four minted keys are then reported as rejected, by value. Counted at BOTH halves of reversibility, since a defect counted in one would read as half as bad: the request-path repair (expand/repair.go) and the proxy's response-side continuation loop, which is why NoteUnresolved is exported. Four tests. Two revert-verified: classification collapsed to one counter -> expected exactly one MISSING ..., got 0 expected exactly one MALFORMED ..., got 2 a minted length dropped from the check -> ContentKey("") = "e3b0c44298fc1c149afbf4c8", which WellFormedID rejects -- an unresolved lookup for it would be blamed on the model instead of counted as our defect The repair-path test asserts a precondition first -- that both ids actually reached the resolve path -- because a fixture that never matched would leave both counts at zero, which reads identically to a working split. Full suite: 27 packages, 0 failures, gofmt clean. Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com>
…max_calls Found by live verification against 7a2ef37, which is the third time this document has described code that is not there. Three factual defects, no wording changes: THE CONFIG SURFACE OMITTED A SHIPPED KEY. `min_inventory` is in extractSweepConfig, in the settings-form field contract, and is the gate that decided every real turn measured in this round -- a real Claude Code session offered 4 candidates against the floor of 10 and the component declined. A surface table that does not name it describes a component nobody is running. THE `max_calls` PARAGRAPH CONTRADICTED THE DOCUMENT'S OWN HEADER. CORRECTION 2 says the batch assembler, the item cap, per-batch concurrency and `max_calls` "are gone", and eleven lines later a paragraph still explained what `max_calls` defaults to and how the item cap composes with it. Replaced with what those batch measurements actually left behind: `min_inventory` bounds the inventory from BELOW rather than bounding calls from above, because the finding was never about call count, it was that the model's judgement is a function of how many candidates it compares. THE COUNTER LIST PREDATED THE GATES/EVENTS SPLIT. It named six counters as one flat set. #121 split them, and the split is the part an operator has to know: work performed reaches /stats under `events` and Prometheus under cg_component_events_total, while a candidate turned away stays in the declines series. Both lists here were generated from the call sites and cross-checked against them -- 9 events, 24 gates, disjoint -- so the document cannot be wrong about this in the way it was wrong about the other two. Also records `sweep_candidate_at_depth` as a regression tripwire rather than a rate, which is what #122 made it: its going to zero is how the depth permission keyed on a flag the trigger makes false would come back. NO TEST, AND NOTHING TO REVERT-VERIFY. Nothing executable changed. The accuracy claim that could rot -- the two counter lists -- was checked by extracting every Gate/GateN/Event/EventN call site from components/offload/extract_sweep.go and diffing both directions against the lists; that check reported empty on all four sets and empty overlap. components/gates_events_test.go already pins the disjointness property in the code. Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com>
Splitting Report.Gates into Gates and Events (#121) silently blinded the per-component decision line. It renders `formatGates(rep.Gates)` and nothing else, so a component whose counters are all events logged no counter information whatsoever. The component most affected is the one that only records counters when it WORKS. Observed live during #118's verification, on the best turn of the whole run: session=8c6c1850c623920a component=extract_llm_sweep verdict=acted tokens_before=34317 tokens_after=977 saved=33340 duration_ms=4683.336 changed_msgs=12 stashed=12 Twelve outputs adjudicated, twelve removed, 33,340 tokens saved, and not one counter — because all eleven names it raised are now events. That is exactly the diagnosis this line exists to provide, absent precisely when the component succeeded. `verdict=acted saved=0` versus a full gate histogram was the case it was written for; the split created a third case it could not express. Events render as their own field rather than being folded into `gates`, because the two answer opposite questions and a reader cannot otherwise tell a refusal from a success. Rendered as one `name=n name=n` STRING for the same reason gates are, and it is not a style choice: an attribute KEY is checked against the credential-name denylist, so a future event called `no_auth` would have its count replaced by «redacted». As a value it is scrubbed as content, where a short integer after `=` matches nothing. The test fixture is deliberately events-ONLY. A report carrying both would pass even with this branch removed, since the gates field would still appear — which is how the same test would have been written vacuously. It also asserts the two appear as SEPARATE fields, so a later simplification that merges them fails here. Verified by removing the branch: the line comes back as `verdict=acted tokens_before=34317 tokens_after=977 saved=33340 duration_ms=0 changed_msgs=0 stashed=0` with no counters at all — the live symptom, reproduced. The sibling defect is NOT fixed here: dash/event.go:504 and dash/schema.go:585 have no events counterpart either, so the same names are missing from the dashboard's per-request component row, which is the surface docs/hosted.md points operators to. That needs a column, a migration, an API change and UI, so it is #124 rather than a bundled change. Full suite: 27 packages, 0 failures, gofmt clean. Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com>
… refusing everything (#124, #125) Two defects live verification found, both reported rather than fixed at the time. #124 — THE DASHBOARD LOST EVERY SUCCESS. Splitting Report.Gates into Gates and Events reached /stats and Prometheus but not this store: request_components had a `gates` column and nothing else, so the eleven names that moved -- sweep_dropped, sweep_offered, sweep_candidate_at_depth, reapplied_same_session and the rest -- vanished from the per-request component row, which is the surface docs/hosted.md sends operators to. The names that moved are the ones a component raises when it WORKS, so the row was blind in proportion to how well a component was doing. Observed live: a turn that adjudicated twelve outputs and removed twelve showed an EMPTY cell, while a turn that refused everything showed a full one. Fixed end to end, because a column alone would have been half of it: an additive `events` column with a default (pre-migration rows read as '' and render "unknown", not "did nothing" -- the distinction the gates column's own comment records getting wrong once), FromTrace carrying it across, the insert and the per-request read, a SECOND json_each aggregation for the windowed totals, and the UI rendering both. A separate column and a separate query rather than a discriminator, because merging them anywhere would undo at that layer what the split did at the one above. The UI renders them in ONE cell, events first: "what happened" is the question a reader opens that cell with, and declines answer "why did nothing happen", which only matters once the first is empty. One cell rather than a new column because the column is deliberately unsortable (null in COMPONENT_SORT) and a second would have to be threaded through that array and both header rows for nothing a reader can use. gateSummary is deleted rather than left unused. #125 — THE FALLBACK REFUSED EVERYTHING, STRUCTURALLY. Two near-identical transcripts, twelve candidates each: the prefix ask dropped 12 of 12, the fallback kept 12 of 12 and cited the original read instruction as the obligation for every one. Cause: the fallback has no transcript by construction, so relevance comes from conversationGoal, which LEADS with the first user message. For a spent-ness judgement that is actively misleading -- it describes what the session set out to do, i.e. exactly what may now be finished. So the bias is structural, not tuning: a first-message goal describes the opening intent and this question needs the current one. sweepIntent keeps all three parts, ordered current-FIRST and LABELLED, with the original instruction explicitly marked as possibly already satisfied. The parts map onto the contract's own criteria rather than arriving as one blob the model has to infer structure from. The original instruction is KEPT rather than dropped: criterion (b) is an unfinished USER instruction and a standing "...and summarise them at the end" lives in that message, so removing it would trade a bias toward keeping for a bias toward dropping -- the direction that loses content. Also splits sweep_fallback_kept_everything from sweep_kept_everything. A keep-all means different things on each path and averaging them hides the more interesting one; without the split a run reads as "the component sometimes acts" when the real variable is whether the cache read happened. It is also how this fix gets checked against real traffic rather than argued about. A NOTE ON ONE OF THESE TESTS, because it is the lesson rather than the diff. The store round-trip test PASSED with `row.Events = r.Events` removed: it builds CompRow values directly, so it never exercised the Report-to-row mapping where the drop happened. That is the sixth vacuous check this component family has produced. TestFromTraceCarriesComponentEvents starts from a components.Report instead and fails with `map[]` when that line goes. Every test revert-verified, output quoted: FromTrace -> FromTrace dropped the component's events ...: map[] storage -> the events-only component row never came back with its events intent ordering -> the opening instruction precedes the current step, which is the ordering that made the fallback keep everything Full suite: 27 packages, 0 failures, gofmt clean. Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com>
|
Reviewed against current head, with live verification through a local proxy build (real Claude Code sessions, real gateway/model) and a re-analysis of the repo's own committed measurements. Blocking / correctness1. CI is red, and the fix isn't fully mechanical.
2. Live-verified: real adjudication calls failed to produce a usable verdict, 3 for 3, against the real gateway/model. This is the re-run the PR's own second comment asked for, and the result is worth attention independent of everything else here. A session with 12 live-eligible file-read candidates correctly cleared
Fail-open held throughout — no data loss, no leaked content, no crash. But on this sample, the sweep produced zero effective compactions against real traffic on 3. Real bug: the sweep's own cost ledger is structurally $0.00, always. 4. The author's own retracted premise is still open in the shipped code — this is the single most important design question left unanswered. The author's first comment concluded per-output adjudication is "the shape the measurements refute" and "the likely correct shape is batched adjudication at ~10-12" (quote fidelity: 4/37 non-verbatim at batch 16 vs 0/16 at batch 10). Current code ( Floor tuning (re-analysis of existing evidence — no live-DB access was available this session; see note below)Cold sweep floor ( Warm/tail floor (top-level Both numbers rest entirely on cost-per-call and reduction-ratio figures already in the repo, not fresh measurement — flagging that plainly rather than presenting false precision. Separately: neither Confirmed solid
Minor
|
…cap the ask Four blocking items from review, three of them defects. CI WAS RED. `mkdocs build --strict` aborted on two pages absent from the nav. components/extract_llm_sweep.md is added to the Offload list. docs/proposals/ is EXCLUDED instead, following the pattern already there for docs/superpowers/ -- "working plans/specs for the skill that authored them, not published site content". A proposal is the same category: sweep-adjudicator.md argues against an earlier version of its own design and records which measurements refuted it, which is useful in review and actively confusing on a page someone reaches looking for how to configure something. Excluding rather than adding a nav section also means the next proposal needs no nav edit, and it covers the coref proposal #80 will bring. The component doc's link to it becomes a repository path rather than a site link, which is what the strict build's remaining INFO line was pointing at -- a published 404 in waiting. THE PARSER LOST EVERY VERDICT WITH PROSE IN THE REPLY, which is why live verification saw three firings produce nothing. It took the FIRST `[` to the LAST `]` and unmarshalled the span, so any bracket anywhere in the model's reasoning made the span unparseable -- and a model asked to justify twelve verdicts writes reasoning. One of the three replies was 7,191 completion tokens at 71.2s and NOT truncated; it simply had prose around and between the JSON. Reported as `sweep_unparseable`, which reads as "the prompt is wrong". Now it tries each `[` with a streaming decoder, which reads one value and ignores what follows, and takes the first span that decodes. A decoded array is not automatically THE array, so it must carry a verdict to be believed: `[{}]` decodes into []Verdict cleanly and would give a phantom verdict for label 0 -- and label 0 is a real candidate, so acting on it would remove the wrong output. An empty array stays a legitimate keep-all, because conflating that with junk is what made "the model declined to act" and "the model was never successfully asked" one number for three iterations (4ca1f13). THE CALL'S COST LEDGER WAS STRUCTURALLY $0.00. The ModelCall record never set CostUSD at all, so the per-call figure the dashboard shows for this component read zero on every firing -- measured live at $0.00 against real cache reads of 449,304 and 449,376 tokens, while the request-level rollup had the true $0.0940 and $0.1652. A component whose entire justification is cost looked free, and two recorded totals disagreed with one of them structurally zero. Priced from the REQUEST's model, which is what this component calls by construction, and from the same rates the request-level figure uses so the two agree rather than being two independent guesses. THE ASK WAS UNCAPPED, which risked losing every verdict rather than some. The reply carries a verbatim quote per verdict against a 16,000-token budget, and live measurement puts a verdict at ~600 tokens including reasoning -- so ~26 candidates exhausts it, and truncation is all-or-nothing: the array never closes, nothing parses, every verdict is discarded. A 50-candidate transcript would have swept nothing while paying for the call. Capped at 12, largest first, and what the cap leaves unasked is counted. Twelve because two independent arguments agree on it: the budget arithmetic puts the ceiling near 26, and cc1aa9f measured quote fidelity degrading with size (4 of 37 non-verbatim at 16, 0 of 16 at 10) -- taken when content was copied into the prompt, so it corroborates rather than proves. The cap deliberately does NOT trip sweep_inventory_thinned: that alarm is for a pre-filter starving the comparison, and a ceiling firing it would make it noise on exactly the transcripts where it should be loudest. Whether the sweep should make a SECOND ask to cover the remainder is a real coverage gap needing measurement, tracked in #132 rather than guessed at. Also adds the fallback half of the keep-all split, which had no test -- only the prefix-ask equivalent was asserted, and the fallback's is the one worth watching since it resolves toward keep structurally. A BOUNDARY THE FIRST VERSION OF THE PARSER FIX GOT WRONG, caught by the existing suite rather than by me. Requiring the first element to carry a `verdict` field rejected `[{"i":1,"needed_by":"none","quote":""}]` -- a REAL verdict object whose `verdict` the model omitted -- and broke TestUnsureDefaultsToKeep. That reply must parse, so the caller can count it as an unusable verdict and default to keep: a model that answered badly and a reply that could not be read are different failures with different remedies, and they are separately counted downstream for exactly that reason. So the guard now accepts ANY populated verdict field, which still rejects `[{}]` and `[{"note":"x"}]` -- the objects carrying no information at all. Every test revert-verified, output quoted: parser -> three cases fail with "did not parse; a real reply of this shape produced zero compactions live" -- exactly the bracket-in-prose shapes, and the non-verdict arrays are then accepted, which is the other half of the fix cap -> sweep_adjudicated = 12, want 20 before the cap existed; and the over-cap remainder uncounted Full suite: 27 packages, 0 failures, gofmt clean, mkdocs --strict clean. NOT fixed here, and reported back rather than silently accepted: the reviewer's floor recommendations (cold 1000 kept, warm ~12,000 against the shipped 3000) rest on re-analysis of published figures rather than new measurement, and the warm one contradicts the preset's own committed comment -- that belongs on #120 with the author, not in this PR. Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com>
|
Thank you — this is the review that found why the mechanism produced nothing on your traffic, which no amount of unit testing here was going to surface. All four blocking items addressed in 1. CI green —
|
Independent review found one more instance of the same silent-loss class the flow-pipeline bug already was: reading `enabled` out of a map[string]any with a bare `.(bool)` assertion is wrong for a real config. YAML 1.1 accepts yes/Yes/YES/on/On/ON/y/Y as true, but decoding one of those into `any` (rather than into a typed bool field, which is what config.LoadBytes's own pre-#118 struct did) resolves to a plain string, so the assertion silently reads it as false. An account whose sweep was genuinely running under one of those spellings would have had cold_cache dropped and no extract_llm_sweep added — a valid document that quietly stopped doing its job, exactly what this file's own header comment already names as the risk config.Validate cannot catch. Re-marshals the cold_cache sub-map and decodes it through a small typed struct with KnownFields(true) instead — the same decode path config.LoadBytes itself would take. This fixes the bool-word reading by construction and replaces the hand-rolled "count the extra keys" check with the decoder's own unknown-field rejection, so an account with max_calls or min_idle_seconds set is still correctly refused, now for a clearer reason. cold_cache: (null) and cold_cache: {} both resolve to the same zero value a plain enabled: false already did, so both now drop cleanly instead of being refused. Six new tests: all eight YAML 1.1 true/false spellings, null and empty cold_cache, a non-bool enabled value (refused, not coerced), and max_calls/min_idle_seconds (still refused, via the decoder this time). Re-verified end-to-end against a fresh copy of the production database once more: all nine previously-broken accounts still recover. Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com>
…144) * fix(tenant): recover accounts whose config used a key #118 removed #118 moved extract_llm's per_output and cold_cache keys to the new extract_llm_sweep component and made config.LoadBytes refuse either one outright — "Breaking existing configs is deliberate... migrated by hand" per that component's own comment. That hand migration never happened for the accounts already running with either key set: on the very next request after the new binary shipped, buildTenantConfig started failing for them, and the proxy's fail-open guarantee took over — every one of their requests kept being forwarded, just with NO compaction applied, silently, until someone happened to read the journal rather than the dashboard (see the paired dash fix in this branch, which makes that failure visible on the page instead). This closes the gap the way it should have shipped with #118: not by loosening the refusal (it is the right call — a silently-reinterpreted cold_cache would be "the most expensive possible misreading of this config", per that same comment) but by performing the exact mechanical translation #118's own migration guidance already names, in code, so it happens once, automatically, at Open, and is provably correct before anything is written back. Nothing is deleted and nothing is guessed: per_output is dropped outright (its presence changed nothing to begin with — the sweep "now IS the warm/tail pass"), and cold_cache's settings are carried onto a new extract_llm_sweep entry, in both the components map and the pipeline list, in the position config.go's own "housellm" preset already uses. Every rewritten document is round-tripped through the same validator a user's own settings-page save already gets rejected by (Options.Validate — reused rather than a new dependency, since `tenant` importing `config` directly would cycle back through config's own tests) before it is ever written; a tenant whose config does not match the exact shape this recognizes is left untouched and logged loudly, never guessed at. Verified against a copy of the production database, through the real Open() -> config.Validate path, not a hand-rolled check: every account that used to fail to build now builds cleanly, with extract_llm_sweep present and the account's own tuning (its min_tokens, its trigger threshold) carried across untouched. Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com> * fix(dash): surface it when an account's config failed to build proxy/tenancy.go's build() fails open on purpose when an account's stored configuration cannot be built into a pipeline — it forwards the request uncompacted rather than taking the account offline over a bad config row, and marks the row's preset "invalid" so the fact is at least recorded. Until now that marker was only ever visible in the proxy's own log: a request tagged this way looked, from the dashboard, exactly like ordinary uncompacted traffic. #118 turned this from a theoretical edge case into a live incident — nine accounts silently lost all compaction for hours because their stored config used a key that release removed, and the first anyone knew was a log line, not this page. Overview now counts InvalidConfigRequests in the same window as everything else (one more query in the errgroup already parallelizing Overview's independent reads — see that function's own comment), and the UI shows an unmissable banner above the headline tiles whenever it's nonzero, naming the count and pointing at Settings. This is not folded into Diagnostics: the fact it reports is "money is being spent right now with none of the savings this page exists to show", which is exactly the class of thing this page must not let go unnoticed again. Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com> * fix(tenant): rewrite the config migration on real YAML, not regexes Independent review of the first draft found the same root cause behind every finding: a regex has no idea what it is looking at, so it broke in a different way for a trailing comma, a shared trigger block another component also owns, a comment line, or a key that sorts into a different position than expected. One of those was a real blocker: a flow-style pipeline where extract_llm was the LAST element has no trailing comma after it, so the substring replace meant to insert extract_llm_sweep was a silent no-op — and because the resulting document was still perfectly valid YAML that still built a pipeline, config.Validate could not catch it. The account's cold_cache setting was deleted, no sweep ever ran in its place, and the row's preset stopped reading "invalid" — silently defeating the dashboard banner this same PR adds to catch exactly this class of problem. Rewritten as a real decode -> structural edit -> encode round trip, the same shape config/form.go already uses for every settings-page save (yaml.NewEncoder with SetIndent(2)) — this is not a new pattern in the codebase, just the first migration to use it instead of hand-rolled text surgery. The rewrite finds extract_llm by walking the parsed components map, not by matching a shared trigger block; finds its pipeline position by comparing list elements, not literal punctuation; and never sees a comment line or a key's stored ordering at all, since the YAML parser has already resolved all of that before this code runs. Also handles cold_cache.enabled: false correctly now: dropped with nothing added, rather than refused as an unrecognized shape — the sweep it would have configured never ran, so there is nothing to migrate forward, per extract_llm.go's own migration note. And a save failure for one candidate (a locked row, a disk error) no longer stops the rest of the batch from getting their own turn. Twelve new or rewritten tests, each a direct regression for one of the shapes review found: extract_llm last in the pipeline (the blocker), a shared trigger block on another component, a multi-key trigger, a disabled cold_cache, a stray "cold_cache:" substring in a comment, an extract_llm_sweep that already exists, and the batch-isolation property under a real failure. Verified end-to-end against a fresh copy of the production database once more, through the same tenant.Open() -> config.Validate path as before: all nine previously- broken accounts still recover cleanly. Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com> * fix(tenant): decode cold_cache.enabled through a typed field, not any Independent review found one more instance of the same silent-loss class the flow-pipeline bug already was: reading `enabled` out of a map[string]any with a bare `.(bool)` assertion is wrong for a real config. YAML 1.1 accepts yes/Yes/YES/on/On/ON/y/Y as true, but decoding one of those into `any` (rather than into a typed bool field, which is what config.LoadBytes's own pre-#118 struct did) resolves to a plain string, so the assertion silently reads it as false. An account whose sweep was genuinely running under one of those spellings would have had cold_cache dropped and no extract_llm_sweep added — a valid document that quietly stopped doing its job, exactly what this file's own header comment already names as the risk config.Validate cannot catch. Re-marshals the cold_cache sub-map and decodes it through a small typed struct with KnownFields(true) instead — the same decode path config.LoadBytes itself would take. This fixes the bool-word reading by construction and replaces the hand-rolled "count the extra keys" check with the decoder's own unknown-field rejection, so an account with max_calls or min_idle_seconds set is still correctly refused, now for a clearer reason. cold_cache: (null) and cold_cache: {} both resolve to the same zero value a plain enabled: false already did, so both now drop cleanly instead of being refused. Six new tests: all eight YAML 1.1 true/false spellings, null and empty cold_cache, a non-bool enabled value (refused, not coerced), and max_calls/min_idle_seconds (still refused, via the decoder this time). Re-verified end-to-end against a fresh copy of the production database once more: all nine previously-broken accounts still recover. Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com> * fix(dash): stop spendEvents from splitting its own fixture across months TestSpendSurvivesRowEviction failed CI, deterministically, right now — not from anything in this branch, but because spendEvents built its fixture unconditionally `sessions` hours into the past, and this ran in the first few hours of a new calendar month. MonthToDateUSD always asks for the CURRENT real calendar month (time.Now(), not an injectable clock — see its own comment on why the rollup has to be real-time, not simulated), so a 10-hour-wide fixture straddling midnight on the 1st put its oldest sessions in the PREVIOUS month's tenant_spend row and only its newest in this one, and the test's month-to-date assertion only ever saw the smaller, wrong half. Confirmed by reproducing on main with this branch's own changes removed (git stash), independently twice, and by the arithmetic: at the exact times both reproductions ran, the number of sessions that had rolled into the new month matched the shortfall exactly (1 of 10, then 2 of 10, sessions × 4 turns × $0.25 each). Fixed by clamping the fixture's spacing to the room actually available since local UTC midnight on the 1st, so every session lands in the current month regardless of what day it is — exact for the ~99.9% of the month that isn't within a few hours of the boundary (spacing stays exactly one hour, unchanged), and still correct, just more tightly packed, for the sliver that is. Verified passing 5x in a row, with and without -race, at the exact moment (2026-09-01, within hours of midnight) that was failing before this. Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com> --------- Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com> Co-authored-by: Osher-Elhadad <Osher.Elhadad@ibm.com>
…by regime, and add rtk Addresses the three blocking items from review, plus the cheap ones. 1. Two candidates, and only one is a new transport. The document analysed plugin-as-interceptor (PostToolUse -> updatedToolOutput) without ever weighing plugin-as-installer-for-the-proxy (a skill writing env.ANTHROPIC_BASE_URL into the user's own settings.json), which is what #130 proposes and which gives up none of the component set. New section states the distinction, the component tables are scoped explicitly to (A), and a header note stops this page being read as the justification for #130. The headline blocker is rescoped: a plugin *manifest's* settings.json accepts only agent/subagentStatusLine -- that is not the same as "a plugin cannot set ANTHROPIC_BASE_URL", and the difference is the option that won. Title said "third transport" against the body's "fourth"; now fourth throughout. 2. cachesplit is priced by regime. -34.1%/0%->96.7% is a warm-regime figure -- one Terminal-Bench task x 3 trials (cacheinject.md:203, and :209 says treat it as one task, not a fleet average) with the A/B run back-to-back inside the TTL (dashboard.md:219). Cold interactive traffic is $0.0298 across 1,127 sessions / 11,361 requests (dashboard.md:204). Both are right; the document transferred the warm number onto a definitionally cold-regime user, called cachesplit "the best-evidenced component we have" where cacheinject.md:209 says otherwise, and claimed the figures were "not re-verified against current traffic" -- which for cachesplit is false. Corrected, the Anthropic row reads close to the vLLM row. The DAM recommendation stands but is now justified on spendgate/tenancy/limits and harness-plurality rather than on that figure. 3. Gate 0 is closed consistently. The ranked go/no-go section still called the case conditional and told the reader to run the experiment that already ran. Also: rtk is this architecture and we benchmarked it as a full arm -- -9.0% billed cost, reward-neutral, zero request-path latency (results/rtk.md:11) -- so expected value has a floor instead of resting on one session's -6,285 tokens, and the doc now claims the edge it was missing: rtk is a shell hook, so Read/Grep/Glob bypass it, while matcher ".*" does not. mask quoted as ~27.5-29.5%, single-task replay, never enforced in a benchmark arm. extract_llm 8x -> 82x underwater. Hook events ~34 -> 33. Dropped the guessed 32,768 threshold, keeping the measured (30,000, 40,000] bracket. Noted that inspect_transcript.py reports key names, not record types, so its `system` record type is a transcript event and not a request system array. Keepalive branch reference replaced with #126; extract_llm_sweep (#118) added to the offload table. inspect_transcript.py moved to deploy/harbor/, this repo's convention for analysis Python. Rebased onto main (3ebc65d). Docs-only; no code and no behaviour change. mkdocs is unaffected because docs/superpowers/ is excluded from the site build (mkdocs.yml:88-90); the three in-page anchors were validated against the file's own heading slugs and all other links are external. Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com>
…129) * docs(analysis): evaluate a Claude Code plugin as a fourth transport Asks whether the `components` core can ship as a Claude Code plugin — a thin transport beside `proxy/`, the AuthBridge plugin and `adapters/bifrost`, reusing components via configuration. Not a replacement for the proxy. One fact decides the whole evaluation. No plugin surface can read or write the outbound request: across all ~34 hook events there is no field that rewrites `messages[]`, `system[]`, `tools[]` or `cache_control`, and plugin `settings.json` accepts only `agent` and `subagentStatusLine`, so a plugin cannot even point a session at the proxy. What IS interceptable is one tool result at the moment it is produced, via `PostToolUse` → `updatedToolOutput`. That draws the line between content-scoped and envelope-scoped work, and it is the line between the two component families. Offloaders distribute. `cmdfilter`, `format`, `toon`, `extract`, `collapse`, `smartcrush`, `skeleton` and `dedup` are pure functions from one tool output to a shorter one, which is exactly what the hook hands over and accepts back. Two get better than they are in the proxy: the hook supplies `tool_input.command` and `tool_input.file_path` instead of making `cmdfilter` and `skeleton` infer both from transcript text. Cache management does not distribute, and this is the finding worth arguing with. `cachesplit` restructures the top-level `system` array; `cacheinject` reasons over `messages[]` positionally and emits request metadata; the keepalive on `feat/keepalive-strategies` must originate a request that byte-exactly reproduces a prefix and price it from `CachedTokens`, which only the provider's response carries. None of that is a tool output — it is the envelope, assembled after the last hook runs and never persisted. Verified rather than assumed: a session transcript carries `messages` and `toolUseResult` and no system prompt or tool schemas (`scripts/inspect_transcript.py`). The failure modes are asymmetric too, which is the deeper reason. A wrong offload wastes one expand round-trip, bounded and type-enforced reversible. Wrong cache work inverts: a mistimed keepalive creates at 1.25x instead of refreshing at 0.1x, a breakpoint over budget is a 400, a representation flip inside a cached prefix re-writes the suffix at 11.5x. Envelope work has no merely-no-saving fail direction. The compensating result is that the defensive half of our KV-cache layer stops being necessary rather than being ported. `state.go` names its own premise — an offloader must re-emit identical bytes "otherwise the agent (which re-sends the ORIGINAL each turn)" flips the representation. A hook rewrites the output before it enters the transcript, so the agent never holds the original. Freeze/replay, `MaxCachedIdx`, `Tracker`, `frozen_flips` and sticky ids have nothing left to defend, and `extract_llm`'s sampling nondeterminism stops disqualifying it from repair. Also `PreCompact`/`SessionEnd` replace `proxy/agentcompaction.go`'s string match against Claude Code 2.1.215 internals, which has a documented reachable false positive. What is given up: `cachesplit` (-34.1% cost, 0% -> 96.7% hit, and in every preset), plus `mask` (27.5% Terminal-Bench, 12.5% SWE-bench) and `failed_run`, both of which rewrite EARLIER messages and so cannot work at a hook that fires once at birth. On implicit prefix-cache backends (vLLM/llm-d) the cache loss is zero, because `prefixsplit` is already a no-op there. Also documented: permanence cuts both ways (an expand's restored original joins the transcript for good, so the plugin wants a more conservative pipeline than the proxy), `/stats` cost tiers are unobtainable so plugin mode cannot be benchmarked the way the proxy is, and the whole thing is gated on one unverified fact — whether `updatedToolOutput` persists into the transcript. If it does not, the proposition collapses to an expand-only MCP server. That experiment is named as gate 0 and should run before any adapter code. On DAM: land the proxy in the gateway. DAM is harness-plural and its egress already matches our gateway credential model, so a Claude-Code-only plugin covers one harness of four and none of the bring-your-own-ACP case. Ship the plugin as the Claude-Code-session layer on top, never as the DAM integration. Docs only — no code, no behavior change. Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com> Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(analysis): address review — fix CI, close Gate 0, correct the cap figure Review found one broken thing, settled the doc's central open question, and caught a wrong number. All four points addressed. CI was red: mkdocs build --strict rejected docs/analysis/claude-code-plugin.md because it was in no nav entry, and there is no docs/analysis convention to copy. Moved it to docs/superpowers/specs/2026-08-30-claude-code-plugin-transport-design.md, which exclude_docs already covers and whose stated purpose — "working plans/specs ... not published site content" — is what this doc is. That also matches the dated-spec naming of the keepalive design doc and of the local-distribution proposal in #130. The reviewer offered the alternative of a nav entry under Results:; exclusion is the better fit, because publishing a page that evaluates a plugin we have not built would read on the docs site as a product that exists. Verified with a real strict build (1.45s, no warnings) and a negative control: the same file under docs/analysis/ still aborts strict mode, so the move is the fix rather than something incidental. Gate 0 is closed, in the doc's favour. Review ran the experiment the doc asked for — a PostToolUse hook replacing a Bash output with a sentinel, captured through a raw-logging reverse proxy so the evidence is the literal outbound body rather than the transcript file — and the replacement persists and is resent verbatim on later turns. A working collapse plugin then measured -6,285 tokens on a real session, appearing as the same reduction on turn 1's cache-write and turn 2's cache-read, which is what separates a permanent reduction of resent context from a one-turn display trick. The "three things to verify" section becomes a resolved-gate section plus the risks that actually remain, and the recommendation stops being conditional. The predicted failure mode arrived on the first attempt, which is worth recording rather than smoothing over: updatedToolOutput must be the object tool_response shape ({stdout, stderr, interrupted, isImage, noOutputExpected} for Bash), and a bare string is silently ignored. That is now a hard requirement on adapters/cchook — emit the object shape, count your own rejections — instead of a general warning. The 10,000-character cap figure was wrong, and "cap" was the wrong frame. That number came from the additionalContext / systemMessage / plain-stdout cap, which does not govern this field. Measured: verbatim and uncapped to ~30,000 chars, real threshold in (30,000, 40,000] and most likely 32,768, and above it neither truncation nor rejection — the CLI's ordinary large-output handler produces a ~2,260-char <persisted-output> wrapper with a 2KB preview and a disk pointer while the local record stays intact. For an oversized hook emission that is a token-cost improvement, not a hazard. Two precision fixes. The envelope claim now records that it was verified by a stronger method than the docs (11 events exercised live, plus the installed CLI's own hookSpecificOutput validation schema: 33 events, 22 with output fields, none touching the envelope) and carries the nuance that promptCacheTtl / CLAUDE_CODE_PROMPT_CACHE_TTL is a reachable cache-TTL lever via settings or env — not via any hook or manifest field, and session-wide rather than per-breakpoint — so a flat "zero cache control" reading would overstate the gap. And the component figures quoted throughout (cachesplit's -34.1%, mask's 27.5%/12.5%, the ~7,017-token system block) are now labelled as this repo's frozen historical measurements, quoted accurately but not re-verified against current traffic by this evaluation. Finally, the recommendation leads with the scope the reviewer articulated better than the doc did: a plugin can replicate the offloader half of this repo, persistently and measurably, and categorically cannot replicate the cache-management half — not "a plugin can do what the proxy does." Docs only — no code, no behavior change. Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com> Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(analysis): separate the two plugin candidates, price cachesplit by regime, and add rtk Addresses the three blocking items from review, plus the cheap ones. 1. Two candidates, and only one is a new transport. The document analysed plugin-as-interceptor (PostToolUse -> updatedToolOutput) without ever weighing plugin-as-installer-for-the-proxy (a skill writing env.ANTHROPIC_BASE_URL into the user's own settings.json), which is what #130 proposes and which gives up none of the component set. New section states the distinction, the component tables are scoped explicitly to (A), and a header note stops this page being read as the justification for #130. The headline blocker is rescoped: a plugin *manifest's* settings.json accepts only agent/subagentStatusLine -- that is not the same as "a plugin cannot set ANTHROPIC_BASE_URL", and the difference is the option that won. Title said "third transport" against the body's "fourth"; now fourth throughout. 2. cachesplit is priced by regime. -34.1%/0%->96.7% is a warm-regime figure -- one Terminal-Bench task x 3 trials (cacheinject.md:203, and :209 says treat it as one task, not a fleet average) with the A/B run back-to-back inside the TTL (dashboard.md:219). Cold interactive traffic is $0.0298 across 1,127 sessions / 11,361 requests (dashboard.md:204). Both are right; the document transferred the warm number onto a definitionally cold-regime user, called cachesplit "the best-evidenced component we have" where cacheinject.md:209 says otherwise, and claimed the figures were "not re-verified against current traffic" -- which for cachesplit is false. Corrected, the Anthropic row reads close to the vLLM row. The DAM recommendation stands but is now justified on spendgate/tenancy/limits and harness-plurality rather than on that figure. 3. Gate 0 is closed consistently. The ranked go/no-go section still called the case conditional and told the reader to run the experiment that already ran. Also: rtk is this architecture and we benchmarked it as a full arm -- -9.0% billed cost, reward-neutral, zero request-path latency (results/rtk.md:11) -- so expected value has a floor instead of resting on one session's -6,285 tokens, and the doc now claims the edge it was missing: rtk is a shell hook, so Read/Grep/Glob bypass it, while matcher ".*" does not. mask quoted as ~27.5-29.5%, single-task replay, never enforced in a benchmark arm. extract_llm 8x -> 82x underwater. Hook events ~34 -> 33. Dropped the guessed 32,768 threshold, keeping the measured (30,000, 40,000] bracket. Noted that inspect_transcript.py reports key names, not record types, so its `system` record type is a transcript event and not a request system array. Keepalive branch reference replaced with #126; extract_llm_sweep (#118) added to the offload table. inspect_transcript.py moved to deploy/harbor/, this repo's convention for analysis Python. Rebased onto main (3ebc65d). Docs-only; no code and no behaviour change. mkdocs is unaffected because docs/superpowers/ is excluded from the site build (mkdocs.yml:88-90); the three in-page anchors were validated against the file's own heading slugs and all other links are external. Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com> --------- Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com>
Adds
extract_llm_sweep: a component that asks the request's own model, over the transcriptthat model already holds in its prompt cache, which tool outputs are spent — and removes those,
leaving a shape descriptor plus a recoverable marker. It never rewrites anything and it never copies
output content into a prompt.
It also splits the cold-sweep surface out of
extract_llm, ports the prefix-ask machinery tomain, and fixes three defects in shared machinery found along the way (#119, #121, #111).Draft until live verification is re-run — see Verification below.
Why it is a separate component, not a mode
extract_llmdoes one thing in two situations that want different operations. On a warm turn itworks the uncached tail: the output is recent, the agent may still want most of it, and a smaller
version beats none of it. Deep history is the opposite — it is either still load-bearing, in which
case rewriting corrupts content the model has already reasoned about, or it is spent, in which
case the answer is to remove it, not to produce a smaller version nobody will read.
That is not a threshold difference.
strategy,rewrite,aggressivenessandmax_charsaremeaningless for an adjudicator — it selects no compaction strategy and produces no rewritten
text — so writing one is now a config error naming its replacement rather than a silently ignored
key.
Where this started, and the three corrections
Recorded because each correction came from reading measurements already in this repo more carefully,
not from new data — and a reviewer is likely to want the reasoning as much as the diff.
It began as one call per output. The proposal cited
4ca1f13as evidence that per-outputadjudication "is a shape that has already run". That inverted what the commit says: it diagnoses
per-output as "the per-output design refuted at 6% live-kept, not the bulk shape that measured
58%", filed as a bug.
cc1aa9fadds the direction of the failure — "small batches do not makeit wrong, they make it UNWILLING TO ACT" — which a sweep specifically cannot tolerate.
Then it was batched at 12, which was better but still wrong. Batching exists to amortise copying
content into the prompt, and this design does not copy content at all.
a9d666fhad alreadyestablished the affordable shape: ask the request's model over its cached transcript, shipping an
inventory (label, size, tool-call id, first line) rather than the outputs, because "paying fresh
to send truncated copies of content the model is reading from cache would defeat the mechanism".
That makes it one call per turn rather than one per output, so batch assembly and per-batch
concurrency all went. The ask still carries a ceiling —
maxAskItems = 12— but for a differentreason than batching had: the reply holds a verbatim quote per verdict against a 16,000-token budget,
measured live at ~600 tokens per verdict, and truncation is all-or-nothing since the array never
closes and nothing parses. An uncapped ask on a 50-candidate transcript would sweep nothing while
paying for the call. What the cap leaves unasked is counted (
sweep_over_ask_cap); whether a secondask should cover the remainder is #132.
Then the trigger change silently broke the candidate set. Moving from cold-turn to a pre-expiry
window made
c.ColdCachefalse by construction — and the depth permission was keyed on it, socandidates collapsed to the uncached tail while the ask reads everything up to the cached boundary.
Two disjoint sets: the model was asked to judge outputs the transcript it reads does not contain.
Found by live verification, not review (#122).
How it works
still reads a live cache, and the prefix it invalidates has little life left. The TTL is read
from the request (
apply.cacheTTL: 5 min for a bareephemeral, 1 h explicit), surfaced asCtx.CacheTTLMsrather than re-derived; an unknown TTL does not fire.(largest first, remainder counted). The sweep accepts invalidating the prefix — that is what the
window buys, and the cost is bounded by the window's width rather than by the TTL.
tool_choice: none(free — not part of thecache key), tools left intact (they are part of the key), prefix = the previous turn's sent
body, so the newest tool output is invisible to it — acceptable, since tail content has had no
turns in which to be superseded.
needed_by(a/b/c/none) with the obligation quoted verbatim, thenkeepordrop.droprequiresneeded_by: none; unsure defaults to keep.the
<<cg:HASH>>marker, soexpandrecovers the original. The decision is frozen and replayedbyte-identically on later turns — without which the output returns verbatim next turn and the
cached prefix stops being stable.
min_inventory(default 10) rather than asking,because yield is a function of how many candidates the model compares: 1 output → 6% live-kept
on haiku and 14% on sonnet, both inside the drop-everything null model's error bar; ~15 → 58% at
the lowest cost per output; batch 3–6 dropped a genuinely-spent output 2 times in 4, batch 10 did
4 in 4. Below the floor a removal is a guess, and the errors are asymmetric — a wrong keep costs
one turn's tokens, a wrong drop costs content the agent still needs.
output. On by default (a session's first turn has no prefix), with
block_fallback: truetodecline instead. The miss is counted either way.
Breaking changes
extract_llmlosesper_outputand thecold_cacheblock, along with the "leaves the componentwith nothing to do" error that was the seam between the two behaviours. Removed keys produce a
config error naming their replacement. The
housellmpreset is migrated.cg_component_gate_declines_totalno longer carries successes; they move tocg_component_events_total{component,event}(metrics: cg_component_gate_declines_total carries events that are not declines #121). Both ends are in this repo — the exporter andthe Grafana dashboards — so there is no external scraper to strand.
Safety invariants
Each is a test, and each was verified to fail when its subject was reverted:
pointing the dangerous way;
inventing, since nothing else it returns is content;
expandresolves it;caught 29 content leaks across 4 fixtures when the merged design's 96-char head peek was
restored.
Also revert-verified: the depth permission, the inventory floor, the anchor's provenance, the
rep.Gaterace, the gates/events split, the reply parser, and the ask cap.Accounting
Savings come only from real dropped-candidate sizes, through the same per-turn, non-projected pricing
convention every other component uses. Cost is priced from the request's model — what this
component actually calls — using the same rates the request-level figure comes from, so the two
agree. That last part was a defect until
d162e10:CostUSDwas never set at all, so the per-callledger read $0.00 on every firing while the request-level rollup had the real $0.0940 and
$0.1652. A component whose whole justification is cost looked free.
Verification
Unit: 27 packages, 0 failures,
gofmtclean, sweep tests also green under-race. Every testabove was neutralised, run, and the actual failure quoted in its commit.
Live, re-run against the real gateway with a verbatim wire recorder. All seven end-to-end
behaviours hold on the current mechanism:
cache_creation = 0on both prefix asks againstcache_readof 41,274 and 41,140; the window firing at 250s with warm negative controls at 0s and15s in the same session; six drop-carrying turns all
HTTP 200with no internalrole="tool"and nomid-conversation
role="system"; model-drivenexpandrecovering 104 lines byte-for-byte; 36byte-identical replays; both fallback forks with their separate counters; and
expand_unresolved_*at zero across 32 drops.
The strongest result is a discrimination test: 12 outputs, 8 the user had finished with and 4
carrying an explicit outstanding obligation. 8/8 correct drops, 4/4 correct keeps, zero errors,
with keeps citing a verbatim in-transcript quote — and the agent then answered correctly from the
retained rows.
sweep_candidate_at_depth = 31andcached_prefixnever fired, which is #122confirmed live.
Two live findings are not resolved, and both are in the PR rather than hidden:
per-output floor rejected 11 of 15 candidates and
min_inventoryrefused the remaining 4. Thefloors compound, and a real agent turn is mostly small greps. Floor tuning is deferred to
benchmark arms by decision;
sweep_offeredandsweep_inventory_below_minwill show how often itbites.
fixed in
d162e10(first-[-to-last-]spanning the model's reasoning) and should now parse. Thethird — ask failure, then a fallback hitting its 90s deadline — is latency rather than parsing, and
one observation does not distinguish a transient failure from a fallback that is simply too slow
against that gateway. It fails safe. A wider live sample is wanted, and with the parser fixed the
remaining failure rate is the number that means something.
Five invariants can only ever be unit-verified — they need the model to misbehave on demand — and
should stay that way: the obligation refusal, a fabricated quote, an unoffered/duplicate/missing
label, unsure-defaults-to-keep on malformed replies, and
block_fallback. Note that this includesboth alertable counters,
sweep_drop_refused_obligationandsweep_quote_fabricated, whichstayed at zero across every live run — so neither has any live exercise, only revert-verified unit
coverage.
Scope limit
This component's cost rationale is specific to Anthropic prompt-caching traffic.
PrefixAskisnil by construction for any other provider, and the sweep handles nil explicitly — it declines, or
falls back to the content-carrying path. So it never crashes or misbehaves off that path; it simply
forfeits the cache read that is its entire economic justification, and the fallback pays fresh for
content the cached path reads for about a tenth of the price. Worth knowing before enabling it on a
non-caching backend:
block_fallback: trueturns that into a clean decline rather than an expensiveone.
Known unmeasured, and deliberately left so
apply.coldMargin, the only figure herewith a stated purpose for clock uncertainty around cache expiry. Wider fires more often and
invalidates more remaining TTL; narrower fires rarely. Nothing measures either side. First thing
to measure.
measured at batch size, where one reply covered many outputs.
reference-tracking hints and reasons from the transcript alone — plausibly better, since it is not
limited to exact matches and the index's documented blind spot was transformed reuse, but untested.
This is what changes when feat(coref): co-reference-aware compaction #80 rebases on top.
Prepared for #80's rebase
AdjudicationItem.Evidencerenders when populated and is omitted when not, so index signals slotinto the prompt contract without reshaping it. And
sweep_inventory_thinnedsits at thecandidate-gathering site:
4ca1f13's real defect was a per-candidate pre-filter that removed 149,681candidates and left about one per request, turning a bulk arm into the refuted shape while it
reported itself as bulk throughout. It cannot fire on
main— that is the point. A filter insertedahead of the append trips it on the first request.