From 3c970c7ed9479b2fb310ff4568b60d0d286de4e1 Mon Sep 17 00:00:00 2001 From: DAVID AMID Date: Thu, 27 Aug 2026 18:27:18 +0300 Subject: [PATCH 01/19] docs(proposals): the cold sweep should adjudicate, not compact 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) Signed-off-by: DAVID AMID --- docs/proposals/sweep-adjudicator.md | 128 ++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 docs/proposals/sweep-adjudicator.md diff --git a/docs/proposals/sweep-adjudicator.md b/docs/proposals/sweep-adjudicator.md new file mode 100644 index 0000000..7c96622 --- /dev/null +++ b/docs/proposals/sweep-adjudicator.md @@ -0,0 +1,128 @@ +# The cold sweep should adjudicate, not compact + +**Status:** proposed, not implemented. This document is the specification; the implementation is +the work. + +## The problem + +`extract_llm` does one thing in two situations that want different things. + +On a **warm turn** it works the uncached tail: a cheap model writes a Starlark program that trims +one recent tool output down to what the agent needs next. Rewriting is right there — the output is +recent, the agent may still want most of it, and a smaller version of it is more useful than none +of it. + +On a **cold sweep** (`cold_cache`, prompt cache expired, whole transcript re-billing at the write +rate) it does the same thing to outputs *deep in history*. That is the wrong operation. Deep +history is either still load-bearing — in which case rewriting it corrupts content the model has +already reasoned about — or it is spent, in which case the right answer is to remove it, not to +produce a smaller version of something nobody will read. + +## The contract to use instead + +`feat/coref-compaction` reached this conclusion for the merged design and removed the `trim` +verdict (`cc1aa9f`). The evidence: + +> trim was chosen **zero times in 21 probe opportunities**, metrics were **identical without it**, +> and in production it was accepted **once against eight rejected as invented**. It was the only +> verdict that asked the model to **transport text**, which is what it is worst at. + +What remained (`internal/extract/bulk.go` on that branch) is binary: + +> `"needed_by"` — which of (a)/(b)/(c) still needs this output, or `"none"` if it is spent. +> `"quote"` — when needed_by is a/b/c, the transcript text that creates that obligation, copied +> VERBATIM. Leave empty only when needed_by is `"none"`. +> `"verdict"` — keep (still needed, **or you are unsure — this is the default**) or drop (its +> information is spent; a short descriptor of its shape will remain in its place). A verdict of +> `"drop"` **REQUIRES** needed_by `"none"`. + +Where (a)/(b)/(c) are: the current step, an unfinished user instruction, or a next step the agent +itself stated. + +The model returns a verdict and a quote. It never returns content. That removes the transporting +*operation*, not merely the transporting *strategies* — which is the strongest available form of +"never ask a model to transport text". + +## What to build + +`extract_llm_sweep`: a registered component that, on a cold sweep, adjudicates each candidate +output and either keeps it **verbatim** or drops it, leaving a short shape descriptor plus the +existing `<>` marker so `expand` still recovers the original. + +**One call per output.** Not a batch. `4ca1f13` established that the merged mode "was never bulk — +it adjudicated one output per call", so per-output adjudication is a shape that has already run. +Batching is what remains experimental, and it is what forces the failure modes this design avoids: +a shared reply that can be truncated mid-array, quote fidelity degrading with batch size (4 of 37 +quotes non-verbatim at batch 16 against 0 of 16 at batch 10), and a batch-truncation counter to +compensate. A per-output call has none of those. + +**Reuse, do not fork.** The adjudication contract text should move to a shared location rather +than being copied out of `bulk.go` — the *contract* is general, the *batching* is not. The model +client, pricing, result cache, keep-list harvesting, marker/stash machinery and the report/gate +plumbing are all shared with `extract_llm` and must stay shared. + +## Config surface + +The split that makes this expressible. `extract_llm` keeps the warm/tail path; `extract_llm_sweep` +is the cold one. Existing configs break deliberately — there is one deployment, and it is migrated +by hand. + +| `extract_llm_sweep` | `extract_llm` only | Shared by both | +|---|---|---| +| `min_tokens` (its own floor, replaces `cold_cache.min_tokens`), `min_idle_seconds`, `max_calls` | `strategy`, `rewrite`, `aggressiveness`, `max_chars`, `fire_on`, `trigger`, `llm_every_n_requests`, `llm_max_per_request`, `allow_on_caching_backend` | `model`, `marker_mode`, `context`, `context_messages`, `model_max_input_tokens`, `economic_gate` | + +Note what leaves the sweep's surface entirely: **`strategy`, `rewrite`, `aggressiveness` and +`max_chars` stop applying**, because an adjudicator selects no compaction strategy and produces no +rewritten text. `rewrite` in particular becomes moot rather than merely defaulted differently — it +governs how a *rewritten* result is validated, and nothing is rewritten. Any of these appearing +under `extract_llm_sweep` should be a config error naming the reason, not a silently ignored key. + +`per_output` and the `cold_cache` block disappear from `extract_llm`, along with the +`per_output: false with cold_cache disabled leaves the component with nothing to do` error — that +error is the seam this split removes. + +## Safety invariants + +Each is a test, and each must be verified to FAIL when its subject is reverted. + +1. **A drop that names an outstanding obligation is refused, not performed.** This is the one + verification pointing the dangerous way, and the one to write first. +2. **Unsure defaults to keep.** A missing, malformed or unparseable verdict leaves the output + verbatim. +3. **A fabricated obligation quote is counted.** It argues for *keeping*, so it is not dangerous — + but it is the signal that the model is inventing, and on this design it is the only such signal + left, since nothing else it returns is content. +4. **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. +5. **A dropped output stays recoverable.** Marker written, original stashed, `expand` resolves it. + A drop advertised as reversible that is not is a worse defect than no drop. +6. **The descriptor left in place transports nothing.** It is generated from the output's shape + (kind, size, line/record count) by *our* code, never by the model. + +## Counters + +`sweep_adjudicated`, `sweep_dropped`, `sweep_kept`, `sweep_drop_refused_obligation`, +`sweep_quote_fabricated`, `sweep_criterion_missing`. The refusal and fabrication counters are the +two that must be alertable: the first means the model tried to drop something still needed, the +second means it is inventing evidence. + +## Open questions + +- **Is the obligation quote worth its tokens per-output?** Requiring evidence halved false drops + in the merged probes (4/4 → 2/4), but that was measured at batch size, where one reply covered + many outputs. Per-output the quote is a larger share of each reply. Worth measuring before + assuming it carries over. +- **Does a spent-ness judgement need `context: full`?** Deciding "needed by nothing" plausibly + requires seeing the whole transcript, which is the expensive context mode. If so, that pairing + should be the sweep's default rather than an operator's discovery. +- **What is the economic gate's break-even for a drop?** A drop removes the entire output rather + than a fraction of it, so the saving per call is much larger than for a compaction, and the gate's + current arithmetic is calibrated on the latter. + +## Execution order + +1. The contract and its parser, in `internal/extract`, with invariants 1–4 as unit tests. No + component wiring — this slice is verifiable alone. +2. The drop path: descriptor generation, marker, stash, invariants 5–6. +3. The component and its config, plus the `extract_llm` surface removals. +4. Counters and their `/stats` golden-contract entries. From 41425ba38002705aab4d31343ca26cc378ed4c35 Mon Sep 17 00:00:00 2001 From: DAVID AMID Date: Thu, 27 Aug 2026 18:40:39 +0300 Subject: [PATCH 02/19] feat(extract): the cold-sweep adjudication contract, and the four rules 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) Signed-off-by: DAVID AMID --- internal/extract/adjudicate.go | 245 ++++++++++++++++++++++++++++ internal/extract/adjudicate_test.go | 183 +++++++++++++++++++++ 2 files changed, 428 insertions(+) create mode 100644 internal/extract/adjudicate.go create mode 100644 internal/extract/adjudicate_test.go diff --git a/internal/extract/adjudicate.go b/internal/extract/adjudicate.go new file mode 100644 index 0000000..f0b7bf6 --- /dev/null +++ b/internal/extract/adjudicate.go @@ -0,0 +1,245 @@ +package extract + +import ( + "encoding/json" + "strconv" + "strings" +) + +// COLD-SWEEP ADJUDICATION. The model returns a VERDICT, never content. +// +// This is the contract `feat/coref-compaction` arrived at (internal/extract/bulk.go), ported here +// without its batching. The contract is general; the batch is not. +// +// WHY IT IS A VERDICT AND NOT A REWRITE. `cc1aa9f` removed the `trim` verdict from that design after +// measuring it: 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, which is what it is worst +// at. What survives is binary, and it removes the transporting OPERATION rather than merely the +// transporting STRATEGIES — the strongest available form of "never ask a model to transport text". +// +// WHY THE CRITERION IS FORCED EVIDENCE AND NOT ADVICE. Arms carrying an identical criterion differed +// ONLY in whether the model had to emit which obligation still needs the output; the arm that had to +// emit it HALVED the false-drop rate (4/4 -> 2/4). Stating the criterion alone measured inert. +// Instructions a model can skim past are inert; a required output field is not. +// +// WHAT IS DELIBERATELY ABSENT, against bulk.go. Two things, and both because there is nothing here +// for them to talk about: +// +// - the READING THE EVIDENCE section. It taught the model to interpret a co-reference index's +// counters, and there is no such index on this path. Shipping the section without the counters +// would be teaching the model to read a field the prompt never carries. +// - the JUDGE THEM AGAINST EACH OTHER section. Comparative ranking is the one thing a per-output +// call structurally cannot do, so the paragraph would be a lie about the question being asked. +// +// The second absence is a KNOWN RISK, recorded here rather than hidden: the merged experiment +// measured comparative judgement as the difference between 6% and 58% live-kept, and `4ca1f13` found +// a live arm that had degraded to 1.02 verdicts per call and read that as "the per-output design +// already refuted at 6%". So the prior on per-output YIELD is negative and the safety machinery below +// is what carries this design — every failure mode it detects resolves toward keep. + +// AdjudicationItem is the one candidate output a single adjudication call is about. +type AdjudicationItem struct { + Index int // caller's message index, for the operator's logs only + ID string // tool-call id, likewise + SizeTokens int + Content string // the output itself; BuildAdjudicationPrompt bounds what it shows +} + +// Verdict is the model's whole reply. Note what is NOT in it: any field carrying output content. +// A parse that succeeds cannot produce text to splice, which is what makes the transport failure +// mode unreachable rather than merely guarded. +type Verdict struct { + Verdict string `json:"verdict"` // keep | drop + NeededBy string `json:"needed_by"` // a | b | c | none (see adjudicationContract's CRITERION) + Quote string `json:"quote,omitempty"` +} + +// adjudicationContract is deliberately blunt about consequences. +// +// The cost-honest framing is worth ~26 points of live-kept on its own, measured: an earlier prompt +// reassured the model that removals "stay recoverable on request" and produced 91% removal at 6% +// live-kept; replacing that clause with the real consequence moved haiku to 64%/32% and sonnet to +// 49%/58%. Telling a model its mistakes are cheap makes it careless. So this text states the true +// cost and NEVER mentions recoverability — even though, on this path, the drop genuinely is +// recoverable through the marker and the stash. That asymmetry is intentional: the operator gets the +// safety net, the model is not told about it. +const adjudicationContract = `You are shown ONE tool output from an agent's transcript. Decide whether the agent +still needs it. + +CRITERION. An output is SPENT only if it is needed for NONE of the following: + (a) the step the agent is on right now; + (b) any instruction the user has given that is NOT YET COMPLETE; + (c) any step the agent has EXPLICITLY STATED it will take and has not yet taken. +Only obligations WRITTEN IN THE TRANSCRIPT count -- do not invent hypothetical future needs. An +output whose information has already been captured elsewhere (a filed total, a recorded conclusion) +AND which no outstanding obligation needs in raw form is spent. + +WHAT A WRONG REMOVAL ACTUALLY COSTS. If you remove something the agent still needs, it usually does +NOT notice the gap and does not ask for the content back. It answers from worse information and gets +the task wrong. There is no safety net you should count on. A wrong removal is a silent, permanent +loss of task quality; a wrong retention costs only tokens. + +"KEEP EVERYTHING" IS A VALID AND OFTEN CORRECT ANSWER. You are judging one output in isolation, so +you cannot tell whether something else would have been the better thing to remove. If this one looks +load-bearing, keep it. Do not reach for a removal because you were asked a question. + +ANSWER THE CRITERION FIRST, THEN DECIDE: + "needed_by" -- which of (a)/(b)/(c) still needs this output, or "none" if it is spent. + "quote" -- when needed_by is a/b/c, the transcript text that creates that obligation, copied + VERBATIM. Leave empty only when needed_by is "none". + "verdict" -- keep (still needed, or you are unsure -- this is the default) or drop (its + information is spent; a short descriptor of its shape will remain in its place). + A verdict of "drop" REQUIRES needed_by "none": if any obligation still needs the + output, the verdict must be keep. + +Reply with ONLY a JSON object, no prose: +{"needed_by": "a|b|c|none", "quote": "", "verdict": "keep|drop"}` + +// adjudicationSampleChars bounds the output shown to the model. +// +// The same 4,000 the compaction prompts use, and for the same reason: it is the largest excerpt worth +// paying for on every candidate. It is a HEAD excerpt with the cut marked, because the question is +// "is this spent", which is answered from what the output IS rather than from every row in it — and +// because an unmarked cut invites the model to reason about content it was never shown. +const adjudicationSampleChars = 4000 + +// BuildAdjudicationPrompt renders the request for ONE output. goal is the conversation the caller +// chose to carry (see the component's `context` setting), so spent-ness is judged against the live +// task rather than in the abstract. +func BuildAdjudicationPrompt(goal string, item AdjudicationItem) string { + var b strings.Builder + b.WriteString(adjudicationContract) + b.WriteString("\n\nWHAT THE AGENT IS DOING NOW (judge relevance toward this):\n") + g := strings.TrimSpace(goal) + if g == "" { + g = "(no explicit goal stated)" + } + if len(g) > 4000 { + g = g[:4000] + } + b.WriteString(g) + b.WriteString("\n\n=== THE OUTPUT UNDER CONSIDERATION (") + b.WriteString(strconv.Itoa(item.SizeTokens)) + b.WriteString(" tokens)\n") + b.WriteString(clipForAdjudication(item.Content, adjudicationSampleChars)) + b.WriteString("\n") + return b.String() +} + +// clipForAdjudication bounds one output and MARKS the cut, so the model knows it is judging an +// excerpt of something larger and does not reason as though it had seen the end. +func clipForAdjudication(s string, max int) string { + if len(s) <= max { + return s + } + return s[:max] + "\n…[excerpt truncated; the output continues for " + + strconv.Itoa(len(s)-max) + " more characters]" +} + +// ParseVerdict reads the model's reply, returning the verdict and whether the reply PARSED. +// +// The two must stay distinguishable. A reply that parsed and said keep is the model DECLINING to act, +// which the contract explicitly invites; a reply that did not parse is a prompt or model failure the +// component never got an answer from. Folding both into "no drop" makes those two identical in the +// counters, and telling them apart is exactly the distinction one live arm turned on (4ca1f13). +func ParseVerdict(reply string) (Verdict, bool) { + s := stripFences(strings.TrimSpace(reply)) + i, j := strings.Index(s, "{"), strings.LastIndex(s, "}") + if i < 0 || j <= i { + return Verdict{}, false + } + var v Verdict + if err := json.Unmarshal([]byte(s[i:j+1]), &v); err != nil { + return Verdict{}, false + } + return v, true +} + +// Adjudication is what OUR code concluded, which is not the same thing as what the model said. Every +// field but Drop exists so a failure is COUNTED rather than inferred from a yield number. +type Adjudication struct { + // Parsed is false when the reply was not a usable JSON object at all. + Parsed bool + // Drop is the only field that authorises an action, and it is true only when every check below + // passed. Every other outcome leaves the output verbatim. + Drop bool + // VerdictUnusable marks a reply that parsed but carried no verdict we act on — absent, empty, or + // a value the contract does not offer (`trim`, most likely, since it used to be offered). + VerdictUnusable bool + // RefusedObligation marks a drop we would not perform: the model named an outstanding obligation + // and then dropped the output anyway. ALERTABLE — it means the model tried to remove something + // it had itself just said was still needed. + RefusedObligation bool + // QuoteFabricated marks an obligation quote that is not in the transcript. ALERTABLE — it means + // the model is inventing evidence, and on this design it is the ONLY such signal left, because + // nothing else the model returns is content. + QuoteFabricated bool + // CriterionMissing marks a reply that never answered needed_by. Tolerated, but counted: it means + // the forcing function did not run for this verdict. + CriterionMissing bool +} + +// Judge turns one reply into a decision, applying the four safety rules that make a verdict +// actionable. transcript is the agent's own text, flattened, and is what an obligation quote is +// checked against — verifying the quote is cheap and turns "did it make that up?" from a worry into +// a counter. +// +// EVERY FAILURE PATH RESOLVES TOWARD KEEP. That is not caution for its own sake: a wrong keep costs +// tokens on one turn, a wrong drop is a silent permanent loss the agent does not notice and cannot +// ask about. The two errors are not comparable, so the code does not treat them symmetrically. +func Judge(reply, transcript string) Adjudication { + v, ok := ParseVerdict(reply) + if !ok { + // UNSURE DEFAULTS TO KEEP, and an unparseable reply is the strongest form of unsure. + return Adjudication{} + } + a := Adjudication{Parsed: true} + nb := strings.ToLower(strings.TrimSpace(v.NeededBy)) + if nb == "" { + // Counted on keeps as well as drops. The name is what it says: the criterion was not + // answered. A model that never answers it is one the forcing function is not reaching at + // all, and that is the same defect whichever way the verdict fell. + a.CriterionMissing = true + } + if q := strings.TrimSpace(v.Quote); q != "" && !transcriptHasQuote(transcript, q) { + a.QuoteFabricated = true + } + switch strings.ToLower(strings.TrimSpace(v.Verdict)) { + case "drop": + // COHERENCE, AND IT POINTS THE DANGEROUS WAY. The criterion states that a drop requires + // needed_by "none". A verdict that names an outstanding obligation and drops the output + // anyway contradicts itself in the direction of silent loss, so it is REFUSED rather than + // performed. Anything that is neither empty nor "none" counts as naming one, including a + // value the contract never offered — an unrecognised criterion answer is not evidence of + // spent-ness. + if nb != "" && nb != "none" { + a.RefusedObligation = true + return a + } + a.Drop = true + case "keep": + default: + // Missing, empty, or a verdict we do not perform. `trim` is the one to expect: it was + // offered by the previous design and a model may still answer with it. Degrade to keep and + // COUNT it rather than discarding the reply — a discarded reply leaves the output unjudged, + // which looks identical to a model that said nothing. + a.VerdictUnusable = true + } + return a +} + +// transcriptHasQuote reports whether q appears in the transcript. +// +// Exact containment first, which is what the merged design checked. Then ONE whitespace-insensitive +// retry, because a model that re-wrapped a long line has copied the text faithfully and is not +// inventing — and this counter is meant to be alertable, so a routine re-wrap firing it would train +// the operator to ignore the signal that says the model is fabricating. The retry runs only on a +// miss, so the transcript is normalised at most once per fabrication-suspect quote rather than once +// per candidate. +func transcriptHasQuote(transcript, q string) bool { + if strings.Contains(transcript, q) { + return true + } + return strings.Contains(wsRe.ReplaceAllString(transcript, " "), wsRe.ReplaceAllString(q, " ")) +} diff --git a/internal/extract/adjudicate_test.go b/internal/extract/adjudicate_test.go new file mode 100644 index 0000000..a2bf525 --- /dev/null +++ b/internal/extract/adjudicate_test.go @@ -0,0 +1,183 @@ +package extract + +import ( + "strings" + "testing" +) + +// The four safety invariants of the cold-sweep adjudicator, in the order the spec ranks them by +// danger. Each is written to FAIL when its subject is reverted; the reverted-output is quoted in the +// commit message. +// +// Every test asserts a PRECONDITION first — that the reply reached the check under test at all. +// Without it a parse regression turns each of these into a vacuous pass: Judge would return the zero +// Adjudication, Drop would be false, and "the refusal worked" and "the reply was never read" would be +// indistinguishable. That is the exact failure mode these tests exist to rule out. + +const testTranscript = `user: find the flaky test and fix it +assistant: I will run the suite, then patch the failing case. +tool: 400 lines of test output +assistant: TestAuthExpiry is the flaky one. Next I will patch auth/session.go.` + +// INVARIANT 1. A drop that names an outstanding obligation is refused, not performed. This is the one +// verification pointing the dangerous way: the model has just said the output is still needed, and +// then asked for it to be removed anyway. +func TestDropNamingAnObligationIsRefused(t *testing.T) { + for _, nb := range []string{"a", "b", "c", "A", " b ", "in-progress-step"} { + reply := `{"needed_by":"` + nb + `","quote":"Next I will patch auth/session.go.","verdict":"drop"}` + a := Judge(reply, testTranscript) + // PRECONDITION: the reply parsed and the verdict reached the drop branch. If this fails the + // assertion below proves nothing — an unparsed reply is also not a drop. + if !a.Parsed { + t.Fatalf("needed_by=%q: reply did not parse, so the refusal was never exercised", nb) + } + if a.VerdictUnusable { + t.Fatalf("needed_by=%q: verdict was not read as a drop, so the refusal was never exercised", nb) + } + if a.Drop { + t.Errorf("needed_by=%q: dropped an output the model itself said is still needed", nb) + } + if !a.RefusedObligation { + t.Errorf("needed_by=%q: refusal not counted; the alertable signal would be silent", nb) + } + } +} + +// The other side of invariant 1, so it cannot be satisfied by refusing everything: needed_by "none" +// is the one answer a drop is allowed to carry, and it must go through. +func TestDropWithNoObligationIsPerformed(t *testing.T) { + a := Judge(`{"needed_by":"none","quote":"","verdict":"drop"}`, testTranscript) + if !a.Parsed { + t.Fatalf("reply did not parse") + } + if !a.Drop { + t.Fatalf("a well-formed spent verdict was not performed; the component would never act") + } + if a.RefusedObligation || a.QuoteFabricated || a.CriterionMissing || a.VerdictUnusable { + t.Errorf("clean verdict raised a failure counter: %+v", a) + } +} + +// INVARIANT 2. Unsure defaults to keep. A missing, malformed or unparseable verdict leaves the output +// verbatim. +func TestUnsureDefaultsToKeep(t *testing.T) { + cases := []struct { + name string + reply string + wantParsed bool + }{ + {"empty reply", "", false}, + {"prose, no object", "I think this output is probably spent, you can remove it.", false}, + {"truncated object", `{"needed_by":"none","verdict":"dr`, false}, + {"not json inside braces", `{needed_by: none, verdict: drop}`, false}, + {"verdict absent", `{"needed_by":"none","quote":""}`, true}, + {"verdict empty", `{"needed_by":"none","verdict":""}`, true}, + {"verdict is trim", `{"needed_by":"none","verdict":"trim"}`, true}, + {"verdict is prose", `{"needed_by":"none","verdict":"probably drop it"}`, true}, + } + for _, tc := range cases { + a := Judge(tc.reply, testTranscript) + if a.Drop { + t.Errorf("%s: dropped on an unusable verdict; unsure must default to keep", tc.name) + } + // PRECONDITION, and it is what makes this test non-vacuous in BOTH directions: a reply that + // carries a usable object must be reported as parsed, and one that does not must not. Without + // this, a Judge that returned the zero value for everything would pass every Drop assertion + // above while having stopped reading replies at all. + if a.Parsed != tc.wantParsed { + t.Errorf("%s: Parsed=%v, want %v", tc.name, a.Parsed, tc.wantParsed) + } + if tc.wantParsed && !a.VerdictUnusable { + t.Errorf("%s: an unusable verdict was not counted", tc.name) + } + } +} + +// INVARIANT 3. A fabricated obligation quote is counted. It argues for KEEPING so it is not +// dangerous — but it is the signal that the model is inventing, and on this design it is the only +// such signal left, since nothing else it returns is content. +func TestFabricatedObligationQuoteIsCounted(t *testing.T) { + a := Judge(`{"needed_by":"c","quote":"Next I will rewrite the parser in Rust.","verdict":"keep"}`, + testTranscript) + if !a.Parsed { + t.Fatalf("reply did not parse, so the quote check was never exercised") + } + if !a.QuoteFabricated { + t.Fatalf("a quote absent from the transcript was not counted as fabricated") + } + // And a real quote must NOT be counted, or the signal is noise. Both directions, because a check + // that fires on everything is as useless as one that fires on nothing. + b := Judge(`{"needed_by":"c","quote":"Next I will patch auth/session.go.","verdict":"keep"}`, + testTranscript) + if b.QuoteFabricated { + t.Errorf("a verbatim transcript quote was counted as fabricated") + } + // A re-wrapped quote is faithful copying, not invention. Counting it would train the operator to + // ignore an alertable counter. + c := Judge(`{"needed_by":"b","quote":"I will run the suite,\n then patch the failing case.","verdict":"keep"}`, + testTranscript) + if c.QuoteFabricated { + t.Errorf("a re-wrapped but faithful quote was counted as fabricated") + } +} + +// INVARIANT 4. 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. +func TestUnansweredCriterionIsToleratedAndCounted(t *testing.T) { + a := Judge(`{"verdict":"drop"}`, testTranscript) + if !a.Parsed { + t.Fatalf("reply did not parse, so the criterion check was never exercised") + } + if a.VerdictUnusable { + t.Fatalf("the verdict was not read, so this exercises invariant 2 rather than invariant 4") + } + if !a.Drop { + t.Errorf("an unanswered criterion refused the drop; yield would collapse against a model that omits it") + } + if !a.CriterionMissing { + t.Errorf("an unanswered criterion was not counted; the forcing function's absence would be invisible") + } + // Answered, so not counted — otherwise the counter says nothing. + if b := Judge(`{"needed_by":"none","verdict":"drop"}`, testTranscript); b.CriterionMissing { + t.Errorf("an answered criterion was counted as missing") + } +} + +// The contract itself must never invite the model to return content. This is the property the whole +// design rests on: no transporting verdict is offered, and no reply field can carry output text. +func TestContractOffersNoTransportingVerdict(t *testing.T) { + item := AdjudicationItem{Index: 3, ID: "toolu_1", SizeTokens: 4200, Content: "line one\nline two\n"} + p := BuildAdjudicationPrompt("fix the flaky test", item) + if !strings.Contains(p, "keep|drop") { + t.Fatalf("prompt does not carry the binary verdict contract") + } + for _, banned := range []string{"trim", "rewrite", "summarize", "shorten"} { + if strings.Contains(strings.ToLower(p), banned) { + t.Errorf("prompt invites the model to transport text: mentions %q", banned) + } + } + // It must also NOT mention recoverability. Measured: reassuring the model that removals stay + // recoverable produced 91% removal at 6% live-kept; the cost-honest framing is worth ~26 points + // of live-kept on its own. + for _, banned := range []string{"recoverab", "restore", "you can get it back"} { + if strings.Contains(strings.ToLower(p), banned) { + t.Errorf("prompt tells the model its mistakes are cheap: mentions %q", banned) + } + } + if !strings.Contains(p, "line one") { + t.Errorf("prompt does not show the output it is asking about") + } +} + +// A large output is shown as a MARKED excerpt, so the model does not reason as though it had seen the +// end of something it has not. +func TestLargeOutputExcerptIsMarked(t *testing.T) { + body := strings.Repeat("x", adjudicationSampleChars+500) + p := BuildAdjudicationPrompt("goal", AdjudicationItem{Content: body}) + if strings.Contains(p, body) { + t.Fatalf("the whole oversized body was shipped; the bound did not apply") + } + if !strings.Contains(p, "excerpt truncated") { + t.Errorf("the cut is not marked, so the model cannot tell it is judging an excerpt") + } +} From cdc33be31f9d1bbdea874913936e84eac2944c21 Mon Sep 17 00:00:00 2001 From: DAVID AMID Date: Thu, 27 Aug 2026 18:47:38 +0300 Subject: [PATCH 03/19] feat(offload): the sweep's drop path -- a shape residue that transports nothing, and a drop that stays recoverable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Signed-off-by: DAVID AMID --- components/offload/extract_sweep_drop.go | 102 +++++++++++++ components/offload/extract_sweep_drop_test.go | 140 ++++++++++++++++++ 2 files changed, 242 insertions(+) create mode 100644 components/offload/extract_sweep_drop.go create mode 100644 components/offload/extract_sweep_drop_test.go diff --git a/components/offload/extract_sweep_drop.go b/components/offload/extract_sweep_drop.go new file mode 100644 index 0000000..a6f2810 --- /dev/null +++ b/components/offload/extract_sweep_drop.go @@ -0,0 +1,102 @@ +package offload + +import ( + "encoding/json" + "fmt" + "strings" + + bschemas "github.com/maximhq/bifrost/core/schemas" + + "github.com/rossoctl/context-guru/components" + "github.com/rossoctl/context-guru/expand" + "github.com/rossoctl/context-guru/schema" +) + +// THE DROP PATH for the cold-sweep adjudicator: what a dropped tool output leaves behind, and how +// it stays recoverable. +// +// Two properties are load-bearing here, and each is a test. +// +// THE RESIDUE TRANSPORTS NOTHING. It is computed from the output's SHAPE — class, size, line and +// record counts — by this code, and it contains no byte of the output itself. That is stricter than +// the merged design's residue, which fell back to a 96-character head peek for unstructured content. +// 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 content in the request that nothing verified, and the whole point of an +// adjudicator over a compactor is that no content moves. 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 DROP IS REVERSIBLE, AND THE MODEL IS NOT TOLD SO. A full marker stashes the original and +// `expand` restores it. A drop advertised as reversible that is not would be a worse defect than no +// drop at all, so it is verified end to end rather than assumed from the fact that commitMark was +// called. 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, so the +// operator gets the safety net and the model does not get to hear about it. + +// sweepDescriptor renders the shape residue for a dropped output. +// +// It answers "what was here", never "what did it say". contentClass supplies the kind from the same +// head-sniffing regexes the economic gate is calibrated on, so the descriptor and the gate cannot +// disagree about what a candidate is. +func sweepDescriptor(content string) string { + kind := "tool output" + if name, _, ok := contentClass(content); ok { + kind = name + } + lines := strings.Count(content, "\n") + 1 + shape := fmt.Sprintf("%s, %d lines, %d tokens", kind, lines, schema.TextTokens(content)) + if n, ok := recordCount(content); ok { + shape = fmt.Sprintf("%s, %d records, %d lines, %d tokens", kind, n, lines, + schema.TextTokens(content)) + } + return "[context-guru removed a spent tool output — " + shape + "]" +} + +// recordCount counts the top-level elements of a JSON array, which is the one "how much was here" +// figure a line count cannot supply: a multi-megabyte API result is routinely a SINGLE line, and +// "1 line" is a useless thing to tell an agent about 200 records. +// +// Only a top-level array, and only when it parses. A partial or streaming payload gets the line +// count alone rather than a guess, because the descriptor's whole value is that every number in it +// is true. +func recordCount(content string) (int, bool) { + s := strings.TrimSpace(content) + if !strings.HasPrefix(s, "[") { + return 0, false + } + var rows []json.RawMessage + if err := json.Unmarshal([]byte(s), &rows); err != nil { + return 0, false + } + return len(rows), true +} + +// applySweepDrop replaces one adjudicated-spent tool output with its shape descriptor plus the +// marker, stashing the original so `expand` can restore it. It reports the store key it wrote (empty +// in the degraded marker modes) and whether the message was changed at all. +// +// Serial by contract, like extract_llm's own splice: the store write and the message mutation are +// not concurrency-safe. +// +// It goes through tryMark rather than writing the text directly, which is what makes the +// never-worse check 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 — so +// without this a drop just above the floor could grow the message it was meant to shrink. +func applySweepDrop(c *components.Ctx, rep *components.Report, mode markerMode, + msg *bschemas.ChatMessage, content string) (key string, ok bool) { + desc := sweepDescriptor(content) + hint := " [full output: call " + expand.ToolName + "]" + newText, key, eff, ok := tryMark(c, mode, content, hint, func(tok string) string { + if tok == "" { + return desc + } + return desc + "\n" + tok + }) + if !ok { + return "", false + } + commitMark(c, rep, eff, key, content) + schema.SetMessageText(msg, newText) + return key, true +} diff --git a/components/offload/extract_sweep_drop_test.go b/components/offload/extract_sweep_drop_test.go new file mode 100644 index 0000000..22d5722 --- /dev/null +++ b/components/offload/extract_sweep_drop_test.go @@ -0,0 +1,140 @@ +package offload + +import ( + "strings" + "testing" + + "github.com/rossoctl/context-guru/components" + "github.com/rossoctl/context-guru/expand" + "github.com/rossoctl/context-guru/schema" + "github.com/rossoctl/context-guru/store" +) + +// sweepFixture is a spent-looking tool output whose every word is distinctive, so invariant 6 can +// assert that NONE of them reached the descriptor. Long enough that a drop is a real reduction. +const sweepFixture = `KUBERNETES_NAMESPACE=quarantine-zebra +deployment/ingress-flamingo READY 3/3 RESTARTS 0 +deployment/ledger-armadillo READY 1/1 RESTARTS 17 +SECRET_TOKEN_kalamazoo_74119 rotated at 2026-08-14T09:31:02Z +warning: pod ledger-armadillo evicted, reason=MemoryPressure +` + `filler-line-that-is-here-only-to-clear-the-never-worse-check +` + +// INVARIANT 5. A dropped output stays recoverable: the marker is written, the original is stashed, +// and expand resolves it BYTE-FOR-BYTE. A drop advertised as reversible that is not would be a worse +// defect than no drop at all. +func TestDroppedOutputStaysRecoverable(t *testing.T) { + st := store.NewMemory(store.Options{}) + rep := &components.Report{} + c := &components.Ctx{Session: "s", Store: st} + msg := tool(sweepFixture) + + key, ok := applySweepDrop(c, rep, markerFull, &msg, sweepFixture) + // PRECONDITION: the drop actually happened. Without this the assertions below are vacuous — + // a helper that returned early and changed nothing would leave the original in place, and + // "recoverable" is trivially true of content that was never removed. + if !ok { + t.Fatal("the drop was refused, so nothing under test ran") + } + got := schema.MessageText(msg) + if got == sweepFixture { + t.Fatal("the message was not rewritten, so no recovery path was exercised") + } + if key == "" { + t.Fatal("no store key: the original was never stashed") + } + if rep.Irreversible { + t.Fatal("a full-marker drop must not be recorded as irreversible") + } + + keys := expand.ParseMarkers(got) + if len(keys) != 1 { + t.Fatalf("expected exactly one resolvable marker, got %d in %q", len(keys), got) + } + orig, resolved := expand.Resolve(st, keys[0]) + if !resolved { + t.Fatal("the marker did not resolve — the drop would be unrecoverable") + } + if orig != sweepFixture { + t.Fatalf("round-trip is not byte-for-byte:\n want %q\n got %q", sweepFixture, orig) + } + // And it must be strictly smaller, marker included, or the drop cost more than it saved. + if schema.TextTokens(got) >= schema.TextTokens(sweepFixture) { + t.Errorf("drop did not shrink the message: %d tokens from %d", + schema.TextTokens(got), schema.TextTokens(sweepFixture)) + } +} + +// A store that cannot persist must not leave an unresolvable marker behind: the drop degrades to a +// markerless one and records the deliberate lossy removal, so the pipeline keeps it rather than +// reverting it. +func TestDroppedOutputWithoutAPersistingStoreLeavesNoDanglingMarker(t *testing.T) { + rep := &components.Report{} + c := &components.Ctx{Session: "s", Store: store.Nop{}} + msg := tool(sweepFixture) + key, ok := applySweepDrop(c, rep, markerFull, &msg, sweepFixture) + if !ok { + t.Fatal("the drop was refused, so nothing under test ran") + } + got := schema.MessageText(msg) + if key != "" || strings.Contains(got, "< 0 { + b.WriteString(",") + } + b.WriteString(`{"n":777}`) + } + b.WriteString("]") + desc := sweepDescriptor(b.String()) + if !strings.Contains(desc, "200 records") { + t.Fatalf("a 200-record single-line array must report its record count, got %q", desc) + } +} From b2c0c0c3dd413a0745ef479892f33445b72ce2bb Mon Sep 17 00:00:00 2001 From: DAVID AMID Date: Thu, 27 Aug 2026 19:10:34 +0300 Subject: [PATCH 04/19] feat(offload): extract_llm_sweep -- the cold sweep adjudicates one output 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) Signed-off-by: DAVID AMID --- components/offload/extract_sweep.go | 624 +++++++++++++++++++++++ components/offload/extract_sweep_test.go | 365 +++++++++++++ 2 files changed, 989 insertions(+) create mode 100644 components/offload/extract_sweep.go create mode 100644 components/offload/extract_sweep_test.go diff --git a/components/offload/extract_sweep.go b/components/offload/extract_sweep.go new file mode 100644 index 0000000..fbf52e7 --- /dev/null +++ b/components/offload/extract_sweep.go @@ -0,0 +1,624 @@ +package offload + +import ( + "context" + "errors" + "fmt" + "log/slog" + "strings" + "sync" + "sync/atomic" + "time" + + bschemas "github.com/maximhq/bifrost/core/schemas" + + "github.com/rossoctl/context-guru/components" + "github.com/rossoctl/context-guru/expand" + "github.com/rossoctl/context-guru/internal/cheapmodel" + "github.com/rossoctl/context-guru/internal/extract" + "github.com/rossoctl/context-guru/internal/logging" + "github.com/rossoctl/context-guru/metrics" + "github.com/rossoctl/context-guru/schema" + "gopkg.in/yaml.v3" +) + +func init() { components.Register("extract_llm_sweep", newExtractSweep) } + +// ExtractSweep is the COLD-SWEEP ADJUDICATOR: on a turn whose prompt cache has expired it asks a +// cheap model, one output at a time, whether each candidate is still needed — and either keeps it +// VERBATIM or removes it, leaving a shape descriptor plus a recoverable marker. +// +// WHY IT IS A SEPARATE COMPONENT FROM extract_llm. The two situations want different operations, and +// running one operation in both is what this split ends. On a warm turn extract_llm works the +// uncached tail: the output is recent, the agent may still want most of it, and a smaller version of +// it is more useful than none of it. On a cold sweep the same code rewrote outputs DEEP IN HISTORY, +// which is the wrong operation on either branch of the only question that matters — deep history 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 rather than to produce a smaller +// version of something nobody will read. +// +// WHY THE COLD TURN IS WORTH ITS OWN COMPONENT AT ALL. On a turn whose cache has expired the provider +// re-bills the ENTIRE transcript as cache creation at 1.25x the fresh rate. MEASURED on this +// deployment over 1.4 days: those turns were 4% of requests and 31% of spend ($360 of $1,173, ~$1.64 +// each against $0.144 warm), and the shipped pipeline saved 0.015% of it. Two things are true only +// there — removing a token is worth 12.5x what it is worth warm, and touching deep history is free +// because there is no live cached prefix left to invalidate. +// +// WHAT IT NEVER DOES. It selects no compaction strategy, produces no rewritten text, and there is no +// reply field a model could return content through. `strategy`, `rewrite`, `aggressiveness` and +// `max_chars` are therefore not merely defaulted differently here, they are meaningless, and writing +// one is a config error rather than a silently ignored key (see newExtractSweep). +type ExtractSweep struct { + minTokens int + minIdleSeconds int + maxCalls int + + modelSource string + modelClient components.Model + modelName string + modelMaxInput int + + mode markerMode + ctxMode contextMode + ctxMessages int + + // gate is the shared economic gate. Its arithmetic is calibrated on COMPACTION, where a call + // removes a fraction of an output; a drop removes all of it, so the break-even for this + // component is a different number and an unmeasured one. Rather than invent a prior, the + // sweep carries its OWN ratioTracker (below) and lets the same exploration budget learn it: + // what this workload's adjudicator actually removes per candidate is the drop RATE, and the + // tracker measures exactly that. See docs/proposals/sweep-adjudicator.md, open question 3. + gate bool + pricing cheapmodel.Pricing + // ratios is deliberately NOT shared with extract_llm's. The two paths remove different + // fractions of a candidate — a partial rewrite against all-or-nothing — so pooling them would + // price each on the other's history, which is the same pooling error contentclass.go exists to + // undo one level down. + ratios ratioTracker +} + +// extractSweepConfig is the sweep's whole surface. Note what is absent and why: see the +// component doc, and the rejected-key probe in newExtractSweep. +type extractSweepConfig struct { + // MinTokens is the sweep's OWN per-output floor (0 = defaultSweepFloor). Lower than the hot + // path's, because on this turn every candidate is being re-billed at the write rate anyway. + MinTokens int `yaml:"min_tokens"` + // MinIdleSeconds demands MORE idle time than the provider TTL implies (0 = just the TTL). + // Raises the bar, never lowers it: the TTL check is the correctness condition and this is only + // extra caution. + MinIdleSeconds int `yaml:"min_idle_seconds"` + // MaxCalls caps model calls for one sweep (0 = defaultSweepMaxCalls; -1 = unlimited). + // + // It used to default to unlimited on the cold_cache block this replaces, on the reasoning that + // a sweep runs once per idle gap on a turn that is already expensive. MEASURED, that reasoning + // was wrong in the way unbounded spend paths usually are: one production request made 27 calls + // against a tenant whose llm_max_per_request was 2, spent $0.229 and added 76.6 s to a turn + // whose upstream took 33.5 s — context-guru was 2.3x slower than the model it was saving money + // on. The sweep deliberately draws on no other component's caps, so this is its ONLY brake. + MaxCalls int `yaml:"max_calls"` + + Model modelConfig `yaml:"model"` + MarkerMode string `yaml:"marker_mode"` + // Context selects how much conversation the adjudication prompt carries: goal | recent + // (default) | full. Open question 2 in the proposal: deciding "needed by NOTHING" plausibly + // requires seeing the whole transcript, which is the expensive mode. The default is NOT `full` + // because that pairing is the one this component's predecessor was measured losing money on — + // 99% of the sweep's prompt was a copy of the transcript it was compacting, sent once per + // candidate — and because nothing has yet measured whether a spent-ness judgement needs it. + Context string `yaml:"context"` + ContextMessages int `yaml:"context_messages"` + ModelMaxInput int `yaml:"model_max_input_tokens"` + EconomicGate *bool `yaml:"economic_gate"` +} + +// defaultSweepFloor is the per-output floor when none is configured. It is the cold_cache block's +// own default, carried over: on this turn every candidate re-bills at the write rate whatever we do, +// so the bar for "worth a call" is genuinely lower than on a warm turn. +const defaultSweepFloor = 1000 + +// defaultSweepMaxCalls bounds one sweep when the operator names no cap. It is llmConcurrency so a +// sweep costs ONE round of calls: the (k+1)th call cannot start until one of the first k returns, +// and at a 7.1 s median that is where a sweep starts costing more wall clock than the turn it is +// shortening. +const defaultSweepMaxCalls = llmConcurrency + +// sweepBannedKeys are the compaction knobs that have no meaning for an adjudicator, and the reason +// each one does not apply. They are refused rather than ignored: a silently accepted `rewrite: false` +// would read as "verified deletion-only is on" when nothing is being rewritten in the first place, +// and an operator migrating a cold_cache config by hand has no other way to find out. +// +// Detected on a SEPARATE probe struct rather than as fields of extractSweepConfig, because a field +// there would have to be declared to the settings form (components/all's field contract), which +// would put a knob on the page whose only behaviour is to fail. +var sweepBannedKeys = []struct { + key, why string +}{ + {"strategy", "an adjudicator selects no compaction strategy — it returns a verdict, not a program"}, + {"rewrite", "nothing is rewritten, so there is no rewrite to validate; the output is kept verbatim or removed"}, + {"aggressiveness", "there is no compaction target to teach: the only question asked is whether the output is spent"}, + {"max_chars", "no projection window exists — a dropped output leaves a shape descriptor, not a truncation"}, +} + +func newExtractSweep(raw []byte) (components.Component, error) { + // The banned keys FIRST, before components.Decode's KnownFields rejects them with a generic + // yaml message. The whole point is that the error names the reason. + if len(raw) > 0 { + var probe map[string]yaml.Node + if err := yaml.Unmarshal(raw, &probe); err == nil { + for _, b := range sweepBannedKeys { + if _, present := probe[b.key]; present { + return nil, fmt.Errorf("extract_llm_sweep: %s does not apply here: %s "+ + "(it belongs to extract_llm, the warm/tail compactor)", b.key, b.why) + } + } + } + } + cfg := extractSweepConfig{} + if err := components.Decode(raw, &cfg); err != nil { + return nil, err + } + ctxMode, err := parseContextMode(cfg.Context) + if err != nil { + return nil, fmt.Errorf("extract_llm_sweep: %w", err) + } + if cfg.MinTokens <= 0 { + cfg.MinTokens = defaultSweepFloor + } + switch { + case cfg.MaxCalls == 0: + cfg.MaxCalls = defaultSweepMaxCalls + case cfg.MaxCalls < 0: + cfg.MaxCalls = 0 // an explicit opt-out of the bound + } + gate := true + if cfg.EconomicGate != nil { + gate = *cfg.EconomicGate + } + return &ExtractSweep{ + minTokens: cfg.MinTokens, minIdleSeconds: cfg.MinIdleSeconds, maxCalls: cfg.MaxCalls, + modelSource: cfg.Model.Source, modelClient: cfg.Model.Client(), + modelName: cfg.Model.Model, modelMaxInput: cfg.ModelMaxInput, + mode: parseMarkerMode(cfg.MarkerMode), ctxMode: ctxMode, ctxMessages: cfg.ContextMessages, + gate: gate, pricing: cheapmodel.PricingFromEnv(), + }, nil +} + +func (*ExtractSweep) Name() string { return "extract_llm_sweep" } +func (*ExtractSweep) Enabled(*components.Ctx) bool { return true } + +// sweeping reports whether this turn gets the sweep: the provider's cache has certainly expired and +// any extra idle requirement is met. Everything the sweep unlocks — acting at depth, pricing at the +// write rate — is only correct when the cache really is gone. +func (e *ExtractSweep) sweeping(c *components.Ctx) bool { + if c == nil || !c.ColdCache { + return false + } + return e.minIdleSeconds <= 0 || c.IdleMs >= int64(e.minIdleSeconds)*1000 +} + +// inputLimit is the adjudication model's input budget. Same resolution order as extract_llm's: the +// operator's pin, then the static window table for a named model, then the proxied model's own +// window when the sweep runs on the incoming client. +func (e *ExtractSweep) inputLimit(c *components.Ctx) int { + if e.modelMaxInput > 0 { + return e.modelMaxInput + } + if e.modelName != "" { + if w, ok := staticWindows.Window(c.Ctx, e.modelName); ok { + return w + } + return unknownModelInputLimit + } + if e.modelSource != "config" && c.CtxWindow > 0 { + return c.CtxWindow + } + return unknownModelInputLimit +} + +// sweepUnusableSamples bounds how many unparseable replies get logged in full. Process-wide, because +// the question it answers — what is the model actually emitting? — is answered by the first few. +// +// Six rounds of the predecessor's failures were diagnosed by inferring a cause from gate counters, +// and every inference was at least partly wrong. A counter can say THAT a reply was unusable; only +// the text says WHY. +var sweepUnusableSamples atomic.Int64 + +// maxSweepUnusableSamples bounds it in count as well as length, because a systematic failure would +// otherwise flood the log with transcript content lifted out of the replies. +const maxSweepUnusableSamples = 5 + +func (e *ExtractSweep) Offload(req *bschemas.BifrostChatRequest, rep *components.Report, c *components.Ctx) ([]string, error) { + sweeping := e.sweeping(c) + if !sweeping { + // NOT a return. The frozen replays below still run, and they are the reason a sweep's + // saving survives past the turn that earned it: without them a warm turn would re-send + // every dropped output verbatim, undoing the removal AND breaking the byte-stability of + // the prefix the provider is caching. + rep.Gate("not_a_cold_sweep") + } + model := e.modelClient + if model == nil && sweeping { + var usedSource string + // ForModelSource, not For: `model.model` names the model to ADJUDICATE with even when the + // source is the incoming request. Without it, a sweep on a coding agent adjudicates with + // the agent's frontier model and the arithmetic never closes — a real cold sweep measured + // here cut the provider bill by $0.63 and spent $1.25 of opus doing it. + model, usedSource = c.Model.ForModelSource(e.modelSource, e.modelName) + if model != nil && usedSource != "" && e.modelSource != "config" && usedSource == "config" { + // A DIFFERENT credential on a DIFFERENT endpoint, so it cannot be silent: an operator + // whose config says `source: incoming` would otherwise have no way to learn that none + // of their calls went there. + rep.Gate("model_source_fell_back_to_config") + } + } + + goal := conversationContext(req, e.ctxMode, e.ctxMessages) + // The transcript, flattened, so a claimed obligation quote is VERIFIED against what the agent + // was actually told rather than trusted. This is the only remaining signal that the model is + // inventing, because nothing else it returns is content. + flat := flattenTranscript(req) + + pricing := e.pricing + if e.modelName == "" && e.modelSource != "config" && !c.SelfRates.Zero() { + // The sweep is running on the agent's own model, so the agent's rates are the real ones. + // The built-in default is haiku-class, and on a sonnet-class agent it understates every + // call by about 3x — measured, a call recorded at $0.0276 had cost about $0.083. + pricing = ratesPricing(c.SelfRates) + } else if e.modelName != "" && c.RatesFor != nil { + if rates := c.RatesFor(e.modelName); !rates.Zero() { + pricing = ratesPricing(rates) + } + } + callModel := e.modelName + if callModel == "" { + callModel = c.ModelName + } + + inputLimit := e.inputLimit(c) + promptOverhead := extractPromptOverheadTokens + schema.TextTokens(goal) + goalOverhead := promptOverheadTokens + schema.TextTokens(goal) + val := savedTokenValue(c) + ratio := e.ratios.ratio() + turnsSoFar := len(req.Input) + + type cand struct { + i int + content string + id string + gateReason string + } + var cands []cand + var keys []string + changed := 0 + + // Phase 1 (serial): replay frozen decisions at any depth, and collect the candidates that + // still need a call. + for _, i := range toolIndices(req) { + msg := &req.Input[i] + if !schema.Rewritable(*msg) { + rep.Gate("non_text_blocks") + continue + } + content := schema.MessageText(*msg) + if content == "" || expand.HasPlaceholder(content) { + rep.Gate("empty_or_marker_present") + continue + } + id := extract.ContentKey(content) + // If the agent recently EXPANDED this content, leave it verbatim — removing it again + // would just trigger another expand. + if isKeptVerbatim(c, id) { + rep.Gate("kept_verbatim_after_expand") + continue + } + // SAME-SESSION REPLAY, and it bypasses the depth gate legitimately: this session already + // sent these exact bytes on an earlier turn, so the provider's cached prefix holds the + // REMOVED form and replaying it is byte-identical. + // + // The stored value is the descriptor, which sweepDescriptor derives from the content + // alone. That is what makes the replay safe in the sense TailOnlyCold's doc requires: the + // DECISION came from a model, but the REPLACEMENT is a pure function of (content, config), + // so a replay can never emit different bytes than the turn that decided it. + if cached, hit := getResult(c, id); hit { + metrics.RecordExtractionCacheLookup(true) + if saved := schema.TextTokens(content) - schema.TextTokens(cached.Projected); saved > 0 { + metrics.RecordExtractionValue(float64(saved) * val.repeatPerToken) + } + if k, ok := applySweepDrop(c, rep, e.mode, msg, content); ok { + changed++ + if k != "" { + keys = append(keys, k) + } + rep.Gate("reapplied_same_session") + } + continue + } + if !sweeping || model == nil { + // A warm turn, or no client. Either way no NEW decision is taken; the replays above + // already ran, which is all a warm turn has to do. + if sweeping { + rep.Gate("no_model_this_request") + } + continue + } + sz := schema.TextTokens(content) + if sz < e.minTokens { + rep.Gate("below_output_floor") + metrics.RecordExtractionCacheLookup(false) + continue + } + metrics.RecordExtractionCacheLookup(false) + // The depth restriction, lifted for exactly the reason this component exists: the + // provider's entry expired, so this turn re-writes the whole transcript into a new cache + // entry whatever we do, and a message at depth is exactly as free to act on as one in the + // tail. Routed through TailOnlyCold rather than skipped, so the ONE condition that makes + // it safe is still checked here rather than assumed from `sweeping`. + if !c.TailOnlyCold(i, true) { + rep.Gate("cached_prefix") + continue + } + if !fitsModelContext(shownBodyTokens(content), promptOverhead, inputLimit) { + // Nothing to shed: the prompt is one tool output plus the contract. Leave it verbatim + // rather than spend a round-trip on a request the upstream may reject. + rep.Gate("over_model_context") + continue + } + gateReason := "gate off" + if e.gate { + seenBefore := markSeenContent(c, id) + explore := !tooSlowToExplore(metrics.ExtractionP50LatencyMs()) && + e.ratios.exploring(c.Session) + d := evaluateGate(sz, ratio, savedTokenValueAt(c, i), + callCost(pricing, sz, goalOverhead), seenBefore, turnsSoFar, explore, true) + // allowCached is unconditionally true here, and it is not an override of the + // caching-backend guard: that guard refuses candidates whose tokens are being billed + // at the cache-READ rate, and on a cold turn none are — savedTokenValueAt prices them + // at the cache-WRITE rate. The guard's own measurement (net-negative on caching + // workloads) is about warm traffic, which this component never sees. + if !d.allow { + metrics.RecordExtractionSuppressed(d.reason) + rep.Gate("economic_gate") + continue + } + metrics.RecordExtractionReason(d.reason) + gateReason = d.reason + } + cands = append(cands, cand{i: i, content: content, id: id, gateReason: gateReason}) + } + + // The sweep's own cap, drawing on no other component's allowance. The paths are switched + // independently and have opposite economics, so a shared budget would silently disable one + // depending on which fired first. + if e.maxCalls > 0 && len(cands) > e.maxCalls { + for k := e.maxCalls; k < len(cands); k++ { + rep.Gate("over_sweep_cap") + } + cands = cands[:e.maxCalls] + } + + // Phase 2 (parallel): ONE CALL PER OUTPUT. Not a batch — 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 known cost of the choice is + // that comparative judgement is unavailable; see internal/extract/adjudicate.go. + if len(cands) > 0 { + out := make([]sweepOutcome, len(cands)) + calls := make([]components.ModelCall, len(cands)) + sem := make(chan struct{}, llmConcurrency) + var wg sync.WaitGroup + for k := range cands { + wg.Add(1) + go func(k int) { + defer wg.Done() + sem <- struct{}{} + defer func() { <-sem }() + out[k], calls[k] = e.adjudicate(c, rep.Component, cands[k].content, cands[k].id, + cands[k].gateReason, goal, flat, model, pricing, callModel) + }(k) + } + wg.Wait() + // SERIAL FROM HERE, and the gates are why. components.Report is a plain struct whose + // Gates map has no lock -- a Report is copied by value across this codebase and cannot + // carry one -- so calling rep.Gate from the goroutines above is a data race, and Go's map + // implementation turns it into `fatal error: concurrent map writes` rather than a wrong + // count. Each call therefore accumulates its own gate names into its own slot and the + // serial phase raises them, which is the same discipline the ModelCall records use. + for k := range calls { + for _, g := range out[k].gates { + rep.Gate(g) + } + if calls[k].Component != "" { + rep.Calls = append(rep.Calls, calls[k]) + } + } + // Phase 3 (serial): freeze + splice. Serial because the store write and the message + // mutation are not concurrency-safe. + for k := range cands { + if !out[k].drop { + continue + } + desc := sweepDescriptor(cands[k].content) + // Freeze the decision so every later turn replays it byte-for-byte from the + // same-session path above, at any depth. Session-scoped only: unlike a compaction, + // a drop is a judgement about THIS transcript's obligations, so it must never be + // served to another session whose agent may still need the output. + putResult(c, cands[k].id, desc, "") + if key, ok := applySweepDrop(c, rep, e.mode, &req.Input[cands[k].i], cands[k].content); ok { + changed++ + if key != "" { + keys = append(keys, key) + } + } + } + } + + if changed == 0 { + rep.Skipped = true + } + return keys, nil +} + +// sweepOutcome is one adjudication's result, carried back to the SERIAL phase. +// +// The gate names travel as data rather than being raised where they are decided, because +// components.Report is copied by value across this codebase and its Gates map therefore carries no +// lock. Raising a gate from a goroutine is not a slightly-wrong counter, it is +// `fatal error: concurrent map writes` -- which is how this was found. +type sweepOutcome struct { + drop bool + gates []string +} + +func (o *sweepOutcome) gate(name string) { o.gates = append(o.gates, name) } + +// adjudicate makes ONE model call about ONE output and returns whether it may be dropped. +// +// Every failure resolves toward keep: a call error, a timeout, an unparseable reply, an unusable +// verdict, a drop that contradicts a named obligation. A wrong keep costs tokens on one turn; a +// wrong drop is a silent permanent loss the agent does not notice and cannot ask about. +func (e *ExtractSweep) adjudicate(c *components.Ctx, component string, + content, id, gateReason, goal, flat string, model components.Model, + pricing cheapmodel.Pricing, callModel string) (sweepOutcome, components.ModelCall) { + + var o sweepOutcome + + ctx, cancel := context.WithTimeout(c.Ctx, llmCallTimeout) + defer cancel() + ctx, callSink := cheapmodel.WithCallSink(ctx) + before := schema.TextTokens(content) + start := time.Now() + + reply, err := model.Complete(ctx, extract.BuildAdjudicationPrompt(goal, + extract.AdjudicationItem{ID: id, SizeTokens: before, Content: content})) + latency := float64(time.Since(start).Milliseconds()) + metrics.RecordExtractionCall(latency) + _, inTok, outTok := callSink.Totals() + cw, cr := callSink.CacheTotals() + call := components.ModelCall{ + Component: component, Model: callModel, Strategy: "adjudicate", + Cold: true, CandidateTokens: before, LatencyMs: latency, + PromptTokens: inTok, CompletionTokens: outTok, + CacheRead: cr, CacheWrite: cw, + CostUSD: pricing.Cost(inTok, outTok, cw, cr), + GateReason: gateReason, + Before: content, + } + if ctx.Err() != nil { + if errors.Is(ctx.Err(), context.DeadlineExceeded) { + atomic.AddInt64(&llmTimeouts, 1) + } else { + atomic.AddInt64(&llmErrors, 1) + } + } + if err != nil { + o.gate("sweep_call_failed") + call.Rejection = "adjudication call failed: " + err.Error() + return o, call + } + o.gate("sweep_adjudicated") + + a := extract.Judge(reply, flat) + if !a.Parsed { + // A reply that opened the object but never closed it ran out of output budget; one that + // never opened it is a format failure. The remedies are opposite — raise max_tokens versus + // fix the prompt — so they get separate counters rather than one that reads as "the prompt + // is wrong". + if strings.Contains(reply, "{") && !strings.Contains(reply, "}") { + o.gate("sweep_reply_truncated") + } else { + o.gate("sweep_unparseable") + } + if sweepUnusableSamples.Add(1) <= maxSweepUnusableSamples { + head := reply + if len(head) > 500 { + head = head[:500] + } + slog.Warn("cg.sweep.unusable_reply", "reply_len", len(reply), "head", head) + } + call.Rejection = "reply did not parse; output kept verbatim" + e.ratios.observe(0, before) + return o, call + } + if a.QuoteFabricated { + o.gate("sweep_quote_fabricated") + } + if a.CriterionMissing { + o.gate("sweep_criterion_missing") + } + if a.VerdictUnusable { + o.gate("sweep_verdict_unusable") + } + // The refusal is counted INSTEAD of a keep, not alongside it. Both leave the output verbatim, + // but "the model judged this still needed" and "the model tried to remove something it had just + // said was needed" are different events, and folding the second into the keep total is what + // would make the alertable one invisible in the ratio an operator actually looks at. + if a.RefusedObligation { + o.gate("sweep_drop_refused_obligation") + e.ratios.observe(0, before) + call.Rejection = "drop refused: the verdict named an outstanding obligation" + return o, call + } + if !a.Drop { + o.gate("sweep_kept") + // A keep is real evidence about this workload: the adjudicator looked at this output and + // judged it still needed. Ratio 0, which is what it removed. + e.ratios.observe(0, before) + call.Rejection = "adjudicated still needed; kept verbatim" + return o, call + } + after := schema.TextTokens(sweepDescriptor(content)) + if after >= before { + // The never-worse check also lives in applySweepDrop, marker included. This one is here so + // the ratio tracker is not fed a negative saving for a decision phase 3 will refuse. + o.gate("sweep_drop_would_not_shrink") + return o, call + } + o.gate("sweep_dropped") + o.drop = true + call.Accepted = true + call.SavedTokens = before - after + call.After = sweepDescriptor(content) + e.ratios.observe(before-after, before) + metrics.RecordExtractionSaving(before - after) + metrics.RecordExtractionValue(float64(before-after) * savedTokenValue(c).perToken) + if debugExtractLLM(c) { + logging.From(c.Ctx).Debug("cg.sweep.drop", "candidate_tokens", before, + "residue_tokens", after, "criterion_missing", a.CriterionMissing, + "quote_fabricated", a.QuoteFabricated) + } + return o, call +} + +// flattenTranscript renders the agent's own text as one string, for verifying an obligation quote. +// Every text block of every message, tool results included: an obligation can be created by a user +// instruction, by the agent's own stated next step, or by something a tool told it. +func flattenTranscript(req *bschemas.BifrostChatRequest) string { + var b strings.Builder + for i := range req.Input { + b.WriteString(schema.MessageText(req.Input[i])) + b.WriteByte('\n') + } + return b.String() +} + +func init() { + f := []components.Field{ + {Key: "min_tokens", Type: components.FieldInt, Default: defaultSweepFloor, Min: 1, + Hint: "The sweep's own per-output floor. Lower than extract_llm's, because on a cold turn every candidate is being re-billed at the cache-write rate whatever we do."}, + {Key: "min_idle_seconds", Type: components.FieldInt, + Hint: "Demand MORE idle time than the provider TTL implies (0 = just the TTL). Raises the bar, never lowers it — the TTL check is the correctness condition and this is extra caution."}, + {Key: "max_calls", Type: components.FieldInt, Default: defaultSweepMaxCalls, + Hint: "Cap model calls for one sweep (-1 = unlimited). The default is one concurrency round: past it the calls serialize and latency grows multiplicatively for a linear gain. Unbounded was measured at 27 calls, $0.229 and 76.6s added to a 33.5s turn."}, + {Key: "context", Type: components.FieldEnum, Default: "recent", Options: []string{"goal", "recent", "full"}, + Hint: "How much conversation the adjudication prompt carries. `full` is plausibly what a spent-ness judgement needs and is also what made the predecessor lose money (99% of the prompt was a copy of the transcript being compacted, once per candidate) — unmeasured either way, so the default stays recent."}, + {Key: "context_messages", Type: components.FieldInt, Default: defaultContextMessages, + Hint: "The N for context: recent (0 = 2). The single biggest lever on what a call COSTS."}, + {Key: "economic_gate", Type: components.FieldBool, Default: true, + Hint: "Only call the model when the expected saving exceeds the expected call cost. NOTE: the gate's arithmetic is calibrated on compaction, which removes a fraction of an output; a drop removes all of it, so the break-even here is a different and unmeasured number."}, + {Key: "model_max_input_tokens", Type: components.FieldInt, + Hint: "Pin the adjudication model's input budget, for a model id the static table cannot name (a self-hosted id, or a gateway alias). Unset = resolved per model."}, + markerModeField(), + } + f = append(f, modelFields("model")...) + components.RegisterFields("extract_llm_sweep", extractSweepConfig{}, f) +} diff --git a/components/offload/extract_sweep_test.go b/components/offload/extract_sweep_test.go new file mode 100644 index 0000000..1231201 --- /dev/null +++ b/components/offload/extract_sweep_test.go @@ -0,0 +1,365 @@ +package offload + +import ( + "context" + "strings" + "sync/atomic" + "testing" + + bschemas "github.com/maximhq/bifrost/core/schemas" + + "github.com/rossoctl/context-guru/components" + "github.com/rossoctl/context-guru/expand" + "github.com/rossoctl/context-guru/schema" + "github.com/rossoctl/context-guru/store" +) + +// verdictModel answers every adjudication with the same canned reply, and counts the calls. +type verdictModel struct { + reply string + calls int64 + prompt atomic.Value // the last prompt, for asserting what the model was shown +} + +func (m *verdictModel) Complete(_ context.Context, prompt string) (string, error) { + atomic.AddInt64(&m.calls, 1) + m.prompt.Store(prompt) + return m.reply, nil +} + +func (m *verdictModel) lastPrompt() string { + s, _ := m.prompt.Load().(string) + return s +} + +// newSweep builds the component through its registered constructor, so the config surface under +// test is the real one. economic_gate off by default here: the gate's break-even for a DROP is +// unmeasured (proposal open question 3), and a test that let it decide would be measuring the gate. +// The floor is above the filler outputs in sweepReq, so exactly ONE candidate reaches the model and +// a call count is an unambiguous assertion about it. +func newSweep(t *testing.T, model components.Model, extraYAML string) *ExtractSweep { + t.Helper() + c, err := newExtractSweep([]byte("min_tokens: 2000\neconomic_gate: false\n" + extraYAML)) + if err != nil { + t.Fatalf("newExtractSweep: %v", err) + } + e := c.(*ExtractSweep) + e.modelClient = model + return e +} + +// sweepReq puts a BIG tool output at depth (index 1), inside the cached prefix, so only a cold sweep +// can reach it. The transcript also states an obligation the refusal test quotes back. +func sweepReq() *bschemas.BifrostChatRequest { + return &bschemas.BifrostChatRequest{Input: []bschemas.ChatMessage{ + userMsg("Find the auth timeout in src/api/users.py and fix it."), + toolResultMsg(strings.Repeat("2024-01-01 GET /users/42 200 12ms src/api/users.py\n", 700)), + assistantMsg("Next I will patch the timeout in src/api/users.py."), + toolResultMsg(strings.Repeat("filler line to grow the transcript\n", 50)), + userMsg("keep going"), + }} +} + +func sweepCtx(session string, cold bool, idleMs int64, st store.Store) *components.Ctx { + return &components.Ctx{ + Session: session, Ctx: context.Background(), + Store: st, CtxWindow: 1_000_000, + // The cached boundary sits AFTER the big output, so index 1 is inside the prefix and a + // warm turn must not touch it. + CacheAware: true, MaxCachedIdx: 3, + ColdCache: cold, IdleMs: idleMs, + } +} + +// The whole point of the component: on a cold turn it removes a spent output at DEPTH, leaves a +// shape descriptor plus a recoverable marker, and recovers the original byte-for-byte. +func TestSweepDropsASpentOutputAtDepthAndKeepsItRecoverable(t *testing.T) { + model := &verdictModel{reply: `{"needed_by":"none","quote":"","verdict":"drop"}`} + e := newSweep(t, model, "") + req := sweepReq() + original := schema.MessageText(req.Input[1]) + st := store.NewMemory(store.Options{}) + rep := &components.Report{} + if _, err := e.Offload(req, rep, sweepCtx("s", true, 3_600_000, st)); err != nil { + t.Fatalf("Offload must fail open: %v", err) + } + // PRECONDITION: the component acted. Without it every assertion below is vacuous — a + // component that never ran leaves the transcript in exactly the state a "correct keep" does. + if n := atomic.LoadInt64(&model.calls); n != 1 { + t.Fatalf("expected exactly one adjudication call, got %d (gates: %v)", n, rep.Gates) + } + if rep.Gates["sweep_dropped"] != 1 { + t.Fatalf("no drop was recorded, so nothing under test ran (gates: %v)", rep.Gates) + } + got := schema.MessageText(req.Input[1]) + if got == original { + t.Fatal("the output at depth was not removed") + } + if !strings.Contains(got, "context-guru removed a spent tool output") { + t.Errorf("no shape descriptor left in place: %q", got) + } + marks := expand.ParseMarkers(got) + if len(marks) != 1 { + t.Fatalf("expected one resolvable marker, got %d in %q", len(marks), got) + } + if back, ok := expand.Resolve(st, marks[0]); !ok || back != original { + t.Fatalf("the drop is not recoverable: ok=%v byte-identical=%v", ok, back == original) + } +} + +// A keep leaves the output VERBATIM. Not "smaller" — verbatim, because there is no rewriting on this +// path and a keep that changed a byte would be the very failure the split exists to remove. +func TestSweepKeepLeavesTheOutputVerbatim(t *testing.T) { + model := &verdictModel{reply: `{"needed_by":"c","quote":"Next I will patch the timeout in src/api/users.py.","verdict":"keep"}`} + e := newSweep(t, model, "") + req := sweepReq() + original := schema.MessageText(req.Input[1]) + rep := &components.Report{} + if _, err := e.Offload(req, rep, sweepCtx("s", true, 3_600_000, store.NewMemory(store.Options{}))); err != nil { + t.Fatal(err) + } + if atomic.LoadInt64(&model.calls) != 1 { + t.Fatalf("the model was never asked, so a verbatim output proves nothing (gates: %v)", rep.Gates) + } + if rep.Gates["sweep_kept"] != 1 { + t.Fatalf("no keep was recorded (gates: %v)", rep.Gates) + } + if got := schema.MessageText(req.Input[1]); got != original { + t.Fatalf("a kept output was modified:\n want %q\n got %q", original[:80], got[:80]) + } + if !rep.Skipped { + t.Error("a sweep that changed nothing must report Skipped") + } + // And the fabrication counter must be quiet: the quote IS in the transcript. + if rep.Gates["sweep_quote_fabricated"] != 0 { + t.Errorf("a verbatim transcript quote was counted as fabricated (gates: %v)", rep.Gates) + } +} + +// The refusal, reaching all the way through the component: a drop naming an outstanding obligation +// leaves the output in place and raises the alertable counter. +func TestSweepRefusesADropThatNamesAnObligation(t *testing.T) { + model := &verdictModel{reply: `{"needed_by":"c","quote":"Next I will patch the timeout in src/api/users.py.","verdict":"drop"}`} + e := newSweep(t, model, "") + req := sweepReq() + original := schema.MessageText(req.Input[1]) + rep := &components.Report{} + if _, err := e.Offload(req, rep, sweepCtx("s", true, 3_600_000, store.NewMemory(store.Options{}))); err != nil { + t.Fatal(err) + } + if atomic.LoadInt64(&model.calls) != 1 { + t.Fatalf("the model was never asked, so the refusal was never exercised (gates: %v)", rep.Gates) + } + if rep.Gates["sweep_drop_refused_obligation"] != 1 { + t.Fatalf("the refusal was not counted (gates: %v)", rep.Gates) + } + if rep.Gates["sweep_dropped"] != 0 { + t.Fatalf("the contradictory drop was PERFORMED (gates: %v)", rep.Gates) + } + if got := schema.MessageText(req.Input[1]); got != original { + t.Fatal("the output was removed despite naming an outstanding obligation") + } +} + +// A warm turn makes no call and touches nothing, however large the candidate. This is the condition +// the whole component rests on: acting at depth is only free because the provider's entry is gone. +func TestSweepDoesNothingOnAWarmTurn(t *testing.T) { + model := &verdictModel{reply: `{"needed_by":"none","verdict":"drop"}`} + e := newSweep(t, model, "") + req := sweepReq() + original := schema.MessageText(req.Input[1]) + rep := &components.Report{} + if _, err := e.Offload(req, rep, sweepCtx("s", false, 3_600_000, store.NewMemory(store.Options{}))); err != nil { + t.Fatal(err) + } + if n := atomic.LoadInt64(&model.calls); n != 0 { + t.Fatalf("a warm turn made %d adjudication calls", n) + } + if rep.Gates["not_a_cold_sweep"] == 0 { + t.Errorf("a warm turn must say why it did nothing (gates: %v)", rep.Gates) + } + if got := schema.MessageText(req.Input[1]); got != original { + t.Fatal("a warm turn modified a message inside the cached prefix") + } +} + +// min_idle_seconds may only RAISE the bar: the TTL check is the correctness condition, this is extra +// caution on top of it. +func TestSweepMinIdleRaisesTheBar(t *testing.T) { + for _, tc := range []struct { + name string + cold bool + idleMs int64 + want bool + }{ + {"cold but only ten minutes idle", true, 600_000, false}, + {"cold and an hour idle", true, 3_600_000, true}, + {"warm, however long idle", false, 7_200_000, false}, + } { + t.Run(tc.name, func(t *testing.T) { + e := newSweep(t, &verdictModel{}, "min_idle_seconds: 1800\n") + if got := e.sweeping(sweepCtx("s", tc.cold, tc.idleMs, store.Nop{})); got != tc.want { + t.Fatalf("sweeping=%v, want %v", got, tc.want) + } + }) + } +} + +// A DROP DECIDED ON A COLD TURN MUST BE REPLAYED ON EVERY LATER TURN, warm ones included. Without +// that, 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. +func TestSweepReplaysItsDropOnTheNextWarmTurn(t *testing.T) { + model := &verdictModel{reply: `{"needed_by":"none","quote":"","verdict":"drop"}`} + e := newSweep(t, model, "") + st := store.NewMemory(store.Options{}) + + cold := sweepReq() + original := schema.MessageText(cold.Input[1]) + rep1 := &components.Report{} + if _, err := e.Offload(cold, rep1, sweepCtx("sess", true, 3_600_000, st)); err != nil { + t.Fatal(err) + } + if rep1.Gates["sweep_dropped"] != 1 { + t.Fatalf("the cold turn did not drop, so there is nothing to replay (gates: %v)", rep1.Gates) + } + coldText := schema.MessageText(cold.Input[1]) + + warm := sweepReq() // the same transcript again, on a warm turn + rep2 := &components.Report{} + if _, err := e.Offload(warm, rep2, sweepCtx("sess", false, 0, st)); err != nil { + t.Fatal(err) + } + if n := atomic.LoadInt64(&model.calls); n != 1 { + t.Fatalf("the warm turn made a model call: %d total", n) + } + if rep2.Gates["reapplied_same_session"] != 1 { + t.Fatalf("the warm turn did not replay the frozen drop (gates: %v)", rep2.Gates) + } + got := schema.MessageText(warm.Input[1]) + if got == original { + t.Fatal("the warm turn re-sent the dropped output verbatim; the saving is gone") + } + if got != coldText { + t.Fatalf("the replay is not byte-identical, so the cached prefix churns:\n cold %q\n warm %q", + coldText, got) + } +} + +// The cap is the sweep's ONLY brake, so it must bind. Unbounded was measured at 27 calls, $0.229 and +// 76.6 s added to a turn whose upstream took 33.5 s. +func TestSweepCapBinds(t *testing.T) { + model := &verdictModel{reply: `{"needed_by":"none","quote":"","verdict":"drop"}`} + e := newSweep(t, model, "max_calls: 2\n") + req := &bschemas.BifrostChatRequest{Input: []bschemas.ChatMessage{ + userMsg("do the thing"), + }} + for i := 0; i < 5; i++ { + req.Input = append(req.Input, + toolResultMsg(strings.Repeat("distinct line "+string(rune('a'+i))+"\n", 900))) + } + rep := &components.Report{} + if _, err := e.Offload(req, rep, sweepCtx("s", true, 3_600_000, store.NewMemory(store.Options{}))); err != nil { + t.Fatal(err) + } + if n := atomic.LoadInt64(&model.calls); n != 2 { + t.Fatalf("max_calls: 2 allowed %d calls (gates: %v)", n, rep.Gates) + } + if rep.Gates["over_sweep_cap"] != 3 { + t.Errorf("the three refused candidates were not counted (gates: %v)", rep.Gates) + } +} + +// EVERY CANDIDATE MUST BE ACCOUNTED FOR, and the accounting must survive concurrency. +// +// This is not a style test. The gates were originally raised from inside the per-call goroutines, +// and components.Report's Gates map carries no lock — a Report is copied by value across this +// codebase and cannot hold one — so Go's map implementation turned it into +// `fatal error: concurrent map writes` and killed the test binary rather than producing a wrong +// count. Sixteen candidates with the cap lifted puts more than llmConcurrency calls in flight, which +// is what it takes to reach it. +// +// Under `-race` the reversion is caught deterministically (a DATA RACE on components.Report.Gate). +// Without it the crash is timing-dependent, so the count assertions below are the part that holds in +// the plain suite: a lost gate is the quiet form of the same defect. +func TestSweepAccountsForEveryCandidateUnderConcurrency(t *testing.T) { + const n = 16 + model := &verdictModel{reply: `{"needed_by":"none","quote":"","verdict":"drop"}`} + e := newSweep(t, model, "max_calls: -1\n") + req := &bschemas.BifrostChatRequest{Input: []bschemas.ChatMessage{userMsg("do the thing")}} + for i := 0; i < n; i++ { + req.Input = append(req.Input, + toolResultMsg(strings.Repeat("candidate "+string(rune('a'+i))+" line\n", 900))) + } + rep := &components.Report{} + if _, err := e.Offload(req, rep, sweepCtx("s", true, 3_600_000, store.NewMemory(store.Options{}))); err != nil { + t.Fatal(err) + } + // PRECONDITION: all sixteen really went to the model, so more than llmConcurrency were in + // flight at once. With the cap binding this would be 4 and the test would prove nothing. + if got := atomic.LoadInt64(&model.calls); got != n { + t.Fatalf("expected %d concurrent adjudications, got %d (gates: %v)", n, got, rep.Gates) + } + if rep.Gates["sweep_adjudicated"] != n { + t.Errorf("sweep_adjudicated = %d, want %d — a gate was lost", rep.Gates["sweep_adjudicated"], n) + } + if rep.Gates["sweep_dropped"] != n { + t.Errorf("sweep_dropped = %d, want %d — a gate was lost", rep.Gates["sweep_dropped"], n) + } +} + +// The compaction knobs must be REFUSED with a reason, not silently ignored. An operator migrating a +// cold_cache block by hand has no other way to learn that `rewrite: false` now means nothing. +func TestSweepRejectsCompactionOnlyKeys(t *testing.T) { + for _, tc := range []struct{ key, yaml string }{ + {"strategy", "strategy: code\n"}, + {"rewrite", "rewrite: false\n"}, + {"aggressiveness", "aggressiveness: high\n"}, + {"max_chars", "max_chars: 8000\n"}, + } { + t.Run(tc.key, func(t *testing.T) { + _, err := newExtractSweep([]byte(tc.yaml)) + if err == nil { + t.Fatalf("%s was accepted; the key would silently do nothing", tc.key) + } + // The reason must be NAMED. A generic yaml "field not found" is what this test exists + // to rule out: it says the key is unknown, not why it cannot apply here. + if !strings.Contains(err.Error(), tc.key) { + t.Errorf("error does not name the key: %v", err) + } + if !strings.Contains(err.Error(), "does not apply here") { + t.Errorf("error does not say why the key cannot apply: %v", err) + } + }) + } +} + +// The prompt the component actually sends must be the adjudication contract — not a compaction +// prompt, and never one inviting the model to return content. +func TestSweepSendsTheAdjudicationContract(t *testing.T) { + model := &verdictModel{reply: `{"needed_by":"none","verdict":"drop"}`} + e := newSweep(t, model, "") + rep := &components.Report{} + if _, err := e.Offload(sweepReq(), rep, sweepCtx("s", true, 3_600_000, store.NewMemory(store.Options{}))); err != nil { + t.Fatal(err) + } + p := e2ePrompt(t, model) + for _, want := range []string{"keep|drop", `"needed_by"`, "SPENT only if"} { + if !strings.Contains(p, want) { + t.Errorf("the prompt does not carry %q", want) + } + } + for _, banned := range []string{"Starlark", "SUMMARY", "return the JSON"} { + if strings.Contains(p, banned) { + t.Errorf("the prompt is a compaction prompt: mentions %q", banned) + } + } +} + +func e2ePrompt(t *testing.T, m *verdictModel) string { + t.Helper() + p := m.lastPrompt() + if p == "" { + t.Fatal("no prompt was sent, so there is nothing to assert about it") + } + return p +} From a4b0e663ef73a26078d86345e60e3135dbe58ed5 Mon Sep 17 00:00:00 2001 From: DAVID AMID Date: Thu, 27 Aug 2026 19:17:41 +0300 Subject: [PATCH 05/19] test(sweep): pin the adjudicator's counter contract at both ends 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) Signed-off-by: DAVID AMID --- components/offload/extract_sweep_test.go | 49 ++++++++++++ proxy/sweep_counters_test.go | 98 ++++++++++++++++++++++++ 2 files changed, 147 insertions(+) create mode 100644 proxy/sweep_counters_test.go diff --git a/components/offload/extract_sweep_test.go b/components/offload/extract_sweep_test.go index 1231201..994107e 100644 --- a/components/offload/extract_sweep_test.go +++ b/components/offload/extract_sweep_test.go @@ -333,6 +333,55 @@ func TestSweepRejectsCompactionOnlyKeys(t *testing.T) { } } +// THE COUNTER CONTRACT, component end. These six names are what an operator's dashboard query and +// alert rule are written against, so a rename breaks monitoring silently rather than loudly. The +// other end is pinned in proxy/sweep_counters_test.go, which asserts the same literal strings survive +// to /stats and to the Prometheus gate series. +// +// Two of them are the ones that must be ALERTABLE, and each names a distinct misbehaviour: +// sweep_drop_refused_obligation means the model tried to remove an output it had just said was still +// needed, and sweep_quote_fabricated means it is inventing evidence — the only such signal left on +// this design, because nothing else it returns is content. +func TestSweepRaisesTheContractedCounterNames(t *testing.T) { + const obligation = "Next I will patch the timeout in src/api/users.py." + for _, tc := range []struct { + name, reply string + want []string + }{ + {"a spent output", `{"needed_by":"none","quote":"","verdict":"drop"}`, + []string{"sweep_adjudicated", "sweep_dropped"}}, + {"an output still needed", `{"needed_by":"a","quote":"` + obligation + `","verdict":"keep"}`, + []string{"sweep_adjudicated", "sweep_kept"}}, + {"a drop contradicting an obligation", `{"needed_by":"a","quote":"` + obligation + `","verdict":"drop"}`, + []string{"sweep_adjudicated", "sweep_drop_refused_obligation"}}, + {"an invented obligation", `{"needed_by":"a","quote":"rewrite the parser in Rust","verdict":"keep"}`, + []string{"sweep_quote_fabricated"}}, + {"an unanswered criterion", `{"verdict":"drop"}`, + []string{"sweep_criterion_missing"}}, + } { + t.Run(tc.name, func(t *testing.T) { + model := &verdictModel{reply: tc.reply} + e := newSweep(t, model, "") + rep := &components.Report{} + if _, err := e.Offload(sweepReq(), rep, + sweepCtx("s", true, 3_600_000, store.NewMemory(store.Options{}))); err != nil { + t.Fatal(err) + } + // PRECONDITION: an adjudication happened. A component that never called the model + // raises no counters at all, and every assertion below would then fail for the wrong + // reason — or, for a subset check, pass while proving nothing. + if rep.Gates["sweep_adjudicated"] != 1 { + t.Fatalf("no adjudication was counted, so no counter was exercised: %v", rep.Gates) + } + for _, want := range tc.want { + if rep.Gates[want] == 0 { + t.Errorf("the component did not raise %q; got %v", want, rep.Gates) + } + } + }) + } +} + // The prompt the component actually sends must be the adjudication contract — not a compaction // prompt, and never one inviting the model to return content. func TestSweepSendsTheAdjudicationContract(t *testing.T) { diff --git a/proxy/sweep_counters_test.go b/proxy/sweep_counters_test.go new file mode 100644 index 0000000..1719d9b --- /dev/null +++ b/proxy/sweep_counters_test.go @@ -0,0 +1,98 @@ +package proxy + +import ( + "encoding/json" + "net/http/httptest" + "strconv" + "strings" + "testing" + + "github.com/rossoctl/context-guru/components" + "github.com/rossoctl/context-guru/metrics" +) + +// sweepGoldenCounters is the cold-sweep adjudicator's counter contract, and it is a contract for the +// same reason statsGoldenTopLevel is one: these names are what an operator's alert rule and dashboard +// query are written against, so a rename breaks monitoring silently rather than loudly. +// +// The two marked ALERTABLE are the point of the list. The rest describe volume; those two describe +// the model misbehaving, and each names a distinct misbehaviour: +// +// - sweep_drop_refused_obligation: the model tried to remove an output it had, in the same reply, +// just said was still needed. The removal did not happen — that is the invariant — but a +// non-zero rate means the contract is not holding and the only thing standing 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. +var sweepGoldenCounters = []string{ + "sweep_adjudicated", + "sweep_criterion_missing", + "sweep_dropped", + "sweep_drop_refused_obligation", // ALERTABLE + "sweep_kept", + "sweep_quote_fabricated", // ALERTABLE +} + +// The counters must reach BOTH surfaces, because they answer different questions for different +// consumers: /stats is what the benchmark harness parses, /metrics is what an alert rule fires on. +// A counter that exists in the component and reaches neither is a log line. +func TestSweepCountersReachStatsAndMetrics(t *testing.T) { + gates := map[string]int{} + for i, name := range sweepGoldenCounters { + gates[name] = i + 1 // distinct values, so a mixed-up mapping is visible + } + agg := metrics.NewAggregator() + agg.Component(components.Report{ + Component: "extract_llm_sweep", Kind: "offload", + TokensBefore: 10_000, TokensAfter: 400, Gates: gates, + }) + h := New(nil, nil, agg, Options{}) + + // /stats: under the component's own object, so a multi-component pipeline's counters stay + // attributable rather than being summed into one pool. + w := httptest.NewRecorder() + h.stats(w, httptest.NewRequest("GET", "/stats", nil)) + var got struct { + Components map[string]struct { + Gates map[string]int64 `json:"gates"` + } `json:"components"` + } + if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil { + t.Fatalf("/stats is not the expected shape: %v\n%s", err, w.Body.String()) + } + comp, ok := got.Components["extract_llm_sweep"] + if !ok { + t.Fatalf("extract_llm_sweep is absent from /stats: %s", w.Body.String()) + } + // PRECONDITION: the gate histogram survived the rollup at all. Without it every assertion + // below would fail for one uninformative reason, and a rollup that dropped `gates` entirely + // would look identical to six renamed counters. + if len(comp.Gates) == 0 { + t.Fatal("the component's gate histogram did not reach /stats, so the counter names cannot be checked") + } + for i, name := range sweepGoldenCounters { + if comp.Gates[name] != int64(i+1) { + t.Errorf("/stats lost or renamed %q (got %d, want %d) — an operator's query breaks silently", + name, comp.Gates[name], i+1) + } + } + + // /metrics: the generic per-gate series, which is what makes the two alertable counters + // alertable without a bespoke metric each. + body := h.renderMetrics() + for i, name := range sweepGoldenCounters { + want := `cg_component_gate_declines_total{component="extract_llm_sweep",gate="` + name + `"} ` + + strconv.Itoa(i+1) + if !strings.Contains(body, want) { + t.Errorf("missing from /metrics:\n %s", want) + } + } +} + +// The other end of this contract is stated in components/offload, where +// TestSweepRaisesTheContractedCounterNames drives the real component and asserts it raises these same +// literal strings. Deliberately spelled out at BOTH ends rather than shared through one exported +// list: a rename that edited a single shared list would keep every test passing while breaking every +// deployed alert rule, which is the failure a contract test exists to prevent. From 48589aa99068210b78d982f347f40da8d84cf2df Mon Sep 17 00:00:00 2001 From: DAVID AMID Date: Thu, 27 Aug 2026 20:01:19 +0300 Subject: [PATCH 06/19] fix(extract_llm): stop raising gates from the per-call goroutines (#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) Signed-off-by: DAVID AMID --- components/offload/extract_llm.go | 23 +++++- .../offload/extract_llm_gaterace_test.go | 72 +++++++++++++++++++ 2 files changed, 93 insertions(+), 2 deletions(-) create mode 100644 components/offload/extract_llm_gaterace_test.go diff --git a/components/offload/extract_llm.go b/components/offload/extract_llm.go index 05464a5..48c99bd 100644 --- a/components/offload/extract_llm.go +++ b/components/offload/extract_llm.go @@ -1291,6 +1291,20 @@ func (e *ExtractLLM) Offload(req *bschemas.BifrostChatRequest, rep *components.R // One record per call, written to its own slot so the goroutines need no lock (a // Report is copied by value across this codebase and cannot carry one). calls := make([]components.ModelCall, len(cands)) + // AND THE SAME RULE FOR GATES, which it did not previously get (#119). + // + // The two gates raised inside runCall — deduped_inflight_extraction and reply_truncated — + // were calling rep.Gate from the goroutines. Report.Gates is a plain map with no lock, for + // exactly the reason the comment above gives, so two concurrent raises are a data race on + // a Go map. That is NOT a wrong counter and it is NOT a recoverable panic: the runtime + // aborts the process with `fatal error: concurrent map writes`, which in a proxy means the + // whole process dies rather than one component failing open. The dedup gate is the + // reachable one — singleflight releases every follower of a shared key at the same instant, + // so N identical candidates in one request give N-1 simultaneous raises. + // + // Same discipline as the records: each call appends its own gate names to its own slot, and + // the serial phase below raises them. + gateNames := make([][]string, len(cands)) sem := make(chan struct{}, llmConcurrency) var wg sync.WaitGroup runCall := func(k int) { @@ -1340,7 +1354,7 @@ func (e *ExtractLLM) Offload(req *bschemas.BifrostChatRequest, rep *components.R if !executed { // A concurrent request derived this exact result; take it and charge nothing. inflightDeduped.Add(1) - rep.Gate("deduped_inflight_extraction") + gateNames[k] = append(gateNames[k], "deduped_inflight_extraction") out[k] = outT{projected: res, summary: sum} return } @@ -1368,7 +1382,7 @@ func (e *ExtractLLM) Offload(req *bschemas.BifrostChatRequest, rep *components.R // real session: 26.8s and ~$0.08 for a reply cut off at 2048 tokens. if outTok >= int64(cheapExtractOutputTokens) { calls[k].GateReason = "reply truncated at the output cap: " + calls[k].GateReason - rep.Gate("reply_truncated") + gateNames[k] = append(gateNames[k], "reply_truncated") atomic.AddInt64(&llmTruncated, 1) } // CLASSIFY THE SILENT FAILURE — and classify it INDEPENDENTLY of whether @@ -1450,6 +1464,11 @@ func (e *ExtractLLM) Offload(req *bschemas.BifrostChatRequest, rep *components.R // reporting that zero value put a phantom `cand=0 saved=0 $0.00` row in the ledger, // inflating the call count with work that by definition did not happen. for k := range calls { + // The gates first, and unconditionally: a single-flight FOLLOWER returns before + // filling its ModelCall slot, and its gate is precisely the one that says so. + for _, g := range gateNames[k] { + rep.Gate(g) + } if calls[k].Component != "" { rep.Calls = append(rep.Calls, calls[k]) } diff --git a/components/offload/extract_llm_gaterace_test.go b/components/offload/extract_llm_gaterace_test.go new file mode 100644 index 0000000..06b72d2 --- /dev/null +++ b/components/offload/extract_llm_gaterace_test.go @@ -0,0 +1,72 @@ +package offload + +import ( + "context" + "strings" + "testing" + + bschemas "github.com/maximhq/bifrost/core/schemas" + + "github.com/rossoctl/context-guru/components" + "github.com/rossoctl/context-guru/store" +) + +// #119: extract_llm raised two gates from inside its per-call goroutines. +// +// components.Report is copied by value throughout this codebase and its Gates map therefore +// carries no lock — the file says so itself, at the declaration of the per-slot ModelCall records +// that exist for exactly this reason. Two concurrent raises are a data race on a Go map, and the +// runtime's response is not a wrong counter and not a recoverable panic: it is +// `fatal error: concurrent map writes`, which aborts the PROCESS. In a proxy that means every +// in-flight request of every session dies, rather than one component failing open — the severity +// that makes this worth its own fix rather than a note. +// +// The reachable path is the single-flight follower. extractInflight collapses identical content +// into one call and releases every waiter at the same instant, so N byte-identical candidates in +// one request produce N-1 simultaneous `deduped_inflight_extraction` raises. +// +// Under `-race` the reversion is caught deterministically. Without it the abort is +// timing-dependent, so the count assertion is what holds in the plain suite: a lost gate is the +// quiet form of the same defect, and `deduped_inflight_extraction` is the one counter that says a +// call was avoided rather than made. +func TestConcurrentCallsDoNotRaceOnTheGateHistogram(t *testing.T) { + // Byte-identical bodies, so all four share one extraction key and three become followers. + // Distinct enough from every other fixture in this package that the process-wide + // extractInflight group cannot collide with another test. + body := strings.Repeat("gaterace fixture line for issue 119, identical across candidates\n", 400) + req := &bschemas.BifrostChatRequest{Input: []bschemas.ChatMessage{ + userMsg("Summarise what these four identical outputs say."), + }} + const identical = 4 + for i := 0; i < identical; i++ { + req.Input = append(req.Input, toolResultMsg(body)) + } + + model := &silentModel{} + e := newCtxGuardComponent(t, model, "") + rep := &components.Report{} + c := &components.Ctx{ + Session: "gaterace", Ctx: context.Background(), + Store: store.NewMemory(store.Options{}), CtxWindow: 1_000_000, + // Caching off, so the tail gate lets every candidate through and all four reach the + // concurrent phase. + CacheAware: false, MaxCachedIdx: -1, + } + if _, err := e.Offload(req, rep, c); err != nil { + t.Fatalf("Offload must fail open: %v", err) + } + + // PRECONDITION: the concurrent phase ran with more than one goroutine in it. If the + // candidates never reached phase 2 — a floor, the trigger, the economic gate — then no two + // raises were ever concurrent and this test proves nothing about the race. The dedup gate + // firing is the proof, because only a follower can raise it. + got := rep.Gates["deduped_inflight_extraction"] + if got == 0 { + t.Fatalf("no single-flight follower ran, so no two gate raises were concurrent "+ + "(gates: %v) — the race was never exercised", rep.Gates) + } + if want := identical - 1; got != want { + t.Errorf("deduped_inflight_extraction = %d, want %d: a follower's gate was lost, which is "+ + "the quiet form of the same unsynchronised write (gates: %v)", got, want, rep.Gates) + } +} From 9e538f1435616bc3b7c75356328f5896cfa07f4e Mon Sep 17 00:00:00 2001 From: DAVID AMID Date: Thu, 27 Aug 2026 20:27:58 +0300 Subject: [PATCH 07/19] feat(sweep)!: adjudicate a BATCH, not one output per call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Signed-off-by: DAVID AMID --- components/component.go | 41 +++ components/offload/extract_sweep.go | 418 +++++++++++++++------- components/offload/extract_sweep_test.go | 420 ++++++++++++++++++++--- docs/proposals/sweep-adjudicator.md | 88 ++++- internal/cheapmodel/anthropic.go | 4 + internal/cheapmodel/openai.go | 4 + internal/extract/adjudicate.go | 235 ++++++++----- internal/extract/adjudicate_test.go | 247 +++++++++---- 8 files changed, 1125 insertions(+), 332 deletions(-) diff --git a/components/component.go b/components/component.go index e50081f..1494280 100644 --- a/components/component.go +++ b/components/component.go @@ -92,6 +92,28 @@ type Remodeler interface { AsModel(id string) Model } +// Budgeter is an optional interface on a Model: the same endpoint, the same credential and the same +// model id, with a larger REPLY budget. +// +// It exists because a truncated reply is the worst outcome available — full price, zero result — and +// it is indistinguishable from a model declining to act. MEASURED (`659e7a6`): the batched +// adjudication arm had 24 of 34 replies unparseable, and the cause was the client's default output +// budget, not the prompt. A verdict array over a dozen items, each carrying an obligation label and a +// verbatim quote, is simply long; a request model running adaptive thinking spends part of the budget +// before emitting any text at all. The array was cut mid-flight, the parse failed, and the caller +// changed nothing — misread as "the model declined" for three iterations. +// +// Why an interface rather than raising cheapmodel.DefaultMaxTokens: the budget a caller needs is a +// property of what it ASKS FOR, not of the endpoint. A one-tool-output compaction reply and a +// twelve-verdict array are different lengths, and raising the shared default would move both. +// +// Output tokens bill as generated, not as budgeted, so a raised ceiling costs nothing until used. +// Optional, and callers must fall back to the client as-is: a Model implementation that does not +// support it still works, it just keeps its own budget. +type Budgeter interface { + WithMaxTokens(n int) Model +} + // ModelSpec carries the LLM clients a NeedsModel component may use, resolved per // request by the host adapter. Incoming is the proxied request's own model + // credentials (nil when unavailable, e.g. the AuthBridge host); Static is a @@ -510,6 +532,25 @@ func (r *Report) Gate(name string) { r.Gates[name]++ } +// GateN records n at once, for a gate whose subject is a COUNT rather than a single candidate. +// +// It exists because a per-candidate loop cannot express "this many were OFFERED". The distinction is +// not cosmetic: a live batched-adjudication arm reported 2.80 verdicts per call and that was read as +// the batch size, when it counted what the model chose to ANSWER rather than what it was SHOWN. The +// truncation counter was firing on 43 of 162 calls at the same time, which is arithmetically +// impossible for batches of 2.8 — the resolution being that the model silently omitted labels. +// Without a way to count the offer, "the batch is starved" and "the model answered for a third of the +// batch" are the same number, and the first reading cost three iterations. +func (r *Report) GateN(name string, n int) { + if r == nil || n <= 0 { + return + } + if r.Gates == nil { + r.Gates = map[string]int{} + } + r.Gates[name] += n +} + // Saved returns non-negative tokens saved by this component. func (r Report) Saved() int { if r.TokensAfter > r.TokensBefore { diff --git a/components/offload/extract_sweep.go b/components/offload/extract_sweep.go index fbf52e7..198567c 100644 --- a/components/offload/extract_sweep.go +++ b/components/offload/extract_sweep.go @@ -44,6 +44,16 @@ func init() { components.Register("extract_llm_sweep", newExtractSweep) } // there — removing a token is worth 12.5x what it is worth warm, and touching deep history is free // because there is no live cached prefix left to invalidate. // +// ONE CALL PER BATCH, NOT PER OUTPUT, and this was got wrong once. The per-output shape is the one +// docs/results/coref-selection-experiment.md REFUTED at 6% live-kept, inside the drop-everything null +// model's error bar; the batch shape lifted that to 58% at the lowest cost per output, because +// comparative judgement beats absolute judgement. `4ca1f13` diagnosed a live arm answering 1.02 +// verdicts per call as exactly that refuted design wearing the bulk name. And `cc1aa9f` names the +// direction of the failure, which is the one a sweep cannot tolerate: 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 there. See internal/extract/adjudicate.go for the full evidence. +// // WHAT IT NEVER DOES. It selects no compaction strategy, produces no rewritten text, and there is no // reply field a model could return content through. `strategy`, `rewrite`, `aggressiveness` and // `max_chars` are therefore not merely defaulted differently here, they are meaningless, and writing @@ -87,14 +97,25 @@ type extractSweepConfig struct { // Raises the bar, never lowers it: the TTL check is the correctness condition and this is only // extra caution. MinIdleSeconds int `yaml:"min_idle_seconds"` - // MaxCalls caps model calls for one sweep (0 = defaultSweepMaxCalls; -1 = unlimited). + // MaxCalls caps BATCH calls for one sweep (0 = defaultSweepMaxCalls; -1 = unlimited). + // + // It bounds CALLS, and each call now adjudicates up to extract.MaxAdjudicationItems candidates + // together, so the two brakes are independent: the item cap is a measured quote-fidelity ceiling + // and this is a spend/latency bound. // - // It used to default to unlimited on the cold_cache block this replaces, on the reasoning that - // a sweep runs once per idle gap on a turn that is already expensive. MEASURED, that reasoning - // was wrong in the way unbounded spend paths usually are: one production request made 27 calls - // against a tenant whose llm_max_per_request was 2, spent $0.229 and added 76.6 s to a turn - // whose upstream took 33.5 s — context-guru was 2.3x slower than the model it was saving money - // on. The sweep deliberately draws on no other component's caps, so this is its ONLY brake. + // Why it still exists at all, when the measured shape made exactly one call per request: with a + // single call a transcript carrying 40 candidates would have 12 adjudicated and 28 left verbatim + // — on the one turn whose whole point is that everything is re-billing at the write rate, that is + // leaving most of the money. Nothing measured says four batches of 12 is worse than one batch of + // 12; the per-batch shape is byte-identical, and batch SIZE is the variable the experiments moved. + // The default is one concurrency round for the same reason the per-output default was: past it + // the calls serialize and latency grows multiplicatively for a linear gain. + // + // The unbounded default this replaces was measured wrong in the way unbounded spend paths usually + // are: one production request made 27 calls against a tenant whose llm_max_per_request was 2, + // spent $0.229 and added 76.6 s to a turn whose upstream took 33.5 s — context-guru was 2.3x + // slower than the model it was saving money on. The sweep draws on no other component's caps, so + // this is its only spend brake. MaxCalls int `yaml:"max_calls"` Model modelConfig `yaml:"model"` @@ -281,13 +302,7 @@ func (e *ExtractSweep) Offload(req *bschemas.BifrostChatRequest, rep *components ratio := e.ratios.ratio() turnsSoFar := len(req.Input) - type cand struct { - i int - content string - id string - gateReason string - } - var cands []cand + var cands []sweepCand var keys []string changed := 0 @@ -363,78 +378,140 @@ func (e *ExtractSweep) Offload(req *bschemas.BifrostChatRequest, rep *components rep.Gate("over_model_context") continue } - gateReason := "gate off" - if e.gate { - seenBefore := markSeenContent(c, id) - explore := !tooSlowToExplore(metrics.ExtractionP50LatencyMs()) && - e.ratios.exploring(c.Session) - d := evaluateGate(sz, ratio, savedTokenValueAt(c, i), - callCost(pricing, sz, goalOverhead), seenBefore, turnsSoFar, explore, true) - // allowCached is unconditionally true here, and it is not an override of the - // caching-backend guard: that guard refuses candidates whose tokens are being billed - // at the cache-READ rate, and on a cold turn none are — savedTokenValueAt prices them - // at the cache-WRITE rate. The guard's own measurement (net-negative on caching - // workloads) is about warm traffic, which this component never sees. - if !d.allow { - metrics.RecordExtractionSuppressed(d.reason) - rep.Gate("economic_gate") - continue + // NO PER-CANDIDATE ECONOMIC GATE HERE. It is evaluated once for the whole batch below, + // for two reasons that both point the same way. + // + // ARITHMETIC: the gate compares one candidate's expected saving against one CALL's cost, and + // a call now covers up to twelve candidates. Charging each of them a whole call priced the + // batch at ~12x its real cost and would suppress batches that pay comfortably. + // + // STARVATION, which is the worse failure. `4ca1f13` found the merged arm's real defect was an + // upstream per-candidate filter: prefix_still_referenced removed 149,681 candidates, leaving + // about one per request, so a "bulk" arm was running the per-output design refuted at 6% + // live-kept. A per-candidate gate on this path is the same mechanism — it thins the batch one + // output at a time until comparative judgement has nothing to compare — and it would return + // this component to batch-of-one silently. See sweep_batch_of_one, which counts it if any + // future filter reintroduces the shape. + cands = append(cands, sweepCand{i: i, content: content, id: id}) + } + + // THE ECONOMIC GATE, ONCE FOR THE SWEEP. Priced the way the call is actually made: the batch's + // total candidate tokens against ONE call's cost. That is the same comparison evaluateGate always + // made, just given the unit that now corresponds to a call. + gateReason := "gate off" + if e.gate && len(cands) > 0 { + var total int + seenBefore := true + for k := range cands { + total += schema.TextTokens(cands[k].content) + // Recurrence is a property of the CONTENT, so it is recorded for every candidate — a + // suppressed batch still counts as seen. The batch is treated as recurring only when + // every member is, which is the conservative direction for a spending decision. + if !markSeenContent(c, cands[k].id) { + seenBefore = false } + } + explore := !tooSlowToExplore(metrics.ExtractionP50LatencyMs()) && + e.ratios.exploring(c.Session) + // allowCached is unconditionally true, and it is not an override of the caching-backend + // guard: that guard refuses candidates whose tokens bill at the cache-READ rate, and on a + // cold turn none do — savedTokenValue prices them at the cache-WRITE rate. The guard's own + // measurement (net-negative on caching workloads) is about warm traffic, which this + // component never sees. + d := evaluateGate(total, ratio, val, callCost(pricing, total, goalOverhead), + seenBefore, turnsSoFar, explore, true) + if !d.allow { + metrics.RecordExtractionSuppressed(d.reason) + // Counted per candidate the batch would have covered, so the figure is comparable with + // the other per-candidate gates above rather than reading as a single refusal. + rep.GateN("economic_gate", len(cands)) + cands = nil + } else { metrics.RecordExtractionReason(d.reason) gateReason = d.reason } - cands = append(cands, cand{i: i, content: content, id: id, gateReason: gateReason}) } - - // The sweep's own cap, drawing on no other component's allowance. The paths are switched - // independently and have opposite economics, so a shared budget would silently disable one - // depending on which fired first. - if e.maxCalls > 0 && len(cands) > e.maxCalls { - for k := e.maxCalls; k < len(cands); k++ { - rep.Gate("over_sweep_cap") - } - cands = cands[:e.maxCalls] + for k := range cands { + cands[k].gateReason = gateReason } - // Phase 2 (parallel): ONE CALL PER OUTPUT. Not a batch — 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 known cost of the choice is - // that comparative judgement is unavailable; see internal/extract/adjudicate.go. + // Phase 2: BATCHED ADJUDICATION. Candidates are chunked into batches of at most + // extract.MaxAdjudicationItems and each batch is ONE call, so the model ranks a dozen outputs + // against each other rather than being asked "is this one expendable" twelve times over. + // + // The batches fan out, so the gate discipline from #119 applies here too: a Report's Gates map + // carries no lock, so each call accumulates its own gate names into its own slot and the serial + // phase raises them. if len(cands) > 0 { - out := make([]sweepOutcome, len(cands)) - calls := make([]components.ModelCall, len(cands)) + batches := make([][]int, 0, len(cands)/extract.MaxAdjudicationItems+1) + for lo := 0; lo < len(cands); lo += extract.MaxAdjudicationItems { + hi := lo + extract.MaxAdjudicationItems + if hi > len(cands) { + hi = len(cands) + } + idx := make([]int, 0, hi-lo) + for k := lo; k < hi; k++ { + idx = append(idx, k) + } + batches = append(batches, idx) + } + // The spend brake. NO SILENT CAPS: a truncated sweep is a bounded-coverage decision and must + // be visible, or "we judged everything" and "we judged the first twelve" read identically. + if e.maxCalls > 0 && len(batches) > e.maxCalls { + for _, b := range batches[e.maxCalls:] { + for range b { + rep.Gate("sweep_batch_truncated") + } + } + batches = batches[:e.maxCalls] + } + rep.GateN("sweep_offered", len(cands)) + + out := make([]sweepOutcome, len(batches)) + calls := make([]components.ModelCall, len(batches)) sem := make(chan struct{}, llmConcurrency) var wg sync.WaitGroup - for k := range cands { + for bi := range batches { wg.Add(1) - go func(k int) { + go func(bi int) { defer wg.Done() sem <- struct{}{} defer func() { <-sem }() - out[k], calls[k] = e.adjudicate(c, rep.Component, cands[k].content, cands[k].id, - cands[k].gateReason, goal, flat, model, pricing, callModel) - }(k) + items := make([]extract.AdjudicationItem, 0, len(batches[bi])) + for _, k := range batches[bi] { + items = append(items, extract.AdjudicationItem{ + // The LABEL is the candidate's position in cands, so the mapping back is + // ours and the model only ever handles a small integer. See + // AdjudicationItem.Label for why that is not a style choice. + Label: k, + ID: cands[k].id, + SizeTokens: schema.TextTokens(cands[k].content), + Content: cands[k].content, + }) + } + out[bi], calls[bi] = e.adjudicateBatch(c, rep.Component, items, cands, + goal, flat, model, pricing, callModel) + }(bi) } wg.Wait() - // SERIAL FROM HERE, and the gates are why. components.Report is a plain struct whose - // Gates map has no lock -- a Report is copied by value across this codebase and cannot - // carry one -- so calling rep.Gate from the goroutines above is a data race, and Go's map - // implementation turns it into `fatal error: concurrent map writes` rather than a wrong - // count. Each call therefore accumulates its own gate names into its own slot and the - // serial phase raises them, which is the same discipline the ModelCall records use. - for k := range calls { - for _, g := range out[k].gates { + + // Serial from here. + drop := map[int]bool{} + for bi := range calls { + for _, g := range out[bi].gates { rep.Gate(g) } - if calls[k].Component != "" { - rep.Calls = append(rep.Calls, calls[k]) + for _, k := range out[bi].drop { + drop[k] = true + } + if calls[bi].Component != "" { + rep.Calls = append(rep.Calls, calls[bi]) } } // Phase 3 (serial): freeze + splice. Serial because the store write and the message // mutation are not concurrency-safe. for k := range cands { - if !out[k].drop { + if !drop[k] { continue } desc := sweepDescriptor(cands[k].content) @@ -458,6 +535,16 @@ func (e *ExtractSweep) Offload(req *bschemas.BifrostChatRequest, rep *components return keys, nil } +// sweepCand is one candidate the sweep collected: where it sits, what it holds, and what the economic +// gate concluded for the batch it belongs to. A package-level type because adjudicateBatch needs to +// resolve a model-supplied LABEL back to content, and that mapping must stay on our side of the wire. +type sweepCand struct { + i int + content string + id string + gateReason string +} + // sweepOutcome is one adjudication's result, carried back to the SERIAL phase. // // The gate names travel as data rather than being raised where they are decided, because @@ -465,31 +552,63 @@ func (e *ExtractSweep) Offload(req *bschemas.BifrostChatRequest, rep *components // lock. Raising a gate from a goroutine is not a slightly-wrong counter, it is // `fatal error: concurrent map writes` -- which is how this was found. type sweepOutcome struct { - drop bool + // drop lists the candidate labels this batch authorised removing. A slice rather than a bool + // because one call now decides for many outputs. + drop []int gates []string } func (o *sweepOutcome) gate(name string) { o.gates = append(o.gates, name) } -// adjudicate makes ONE model call about ONE output and returns whether it may be dropped. +// adjudicateBatch makes ONE model call about a BATCH of outputs and returns the labels it authorised +// dropping. // -// Every failure resolves toward keep: a call error, a timeout, an unparseable reply, an unusable -// verdict, a drop that contradicts a named obligation. A wrong keep costs tokens on one turn; a -// wrong drop is a silent permanent loss the agent does not notice and cannot ask about. -func (e *ExtractSweep) adjudicate(c *components.Ctx, component string, - content, id, gateReason, goal, flat string, model components.Model, - pricing cheapmodel.Pricing, callModel string) (sweepOutcome, components.ModelCall) { +// EVERY FAILURE PATH RESOLVES TOWARD KEEP -- a call error, a timeout, an unparseable reply, an +// unusable verdict, a drop that contradicts a named obligation, a verdict for something we did not +// offer. A wrong keep costs tokens on one turn; a wrong drop is a silent permanent loss the agent +// does not notice and cannot ask about. The two errors are not comparable, so this does not treat +// them symmetrically. +func (e *ExtractSweep) adjudicateBatch(c *components.Ctx, component string, + items []extract.AdjudicationItem, cands []sweepCand, goal, flat string, + model components.Model, pricing cheapmodel.Pricing, callModel string) (sweepOutcome, components.ModelCall) { var o sweepOutcome + // A SINGLE-ITEM BATCH IS THE REFUTED DESIGN WEARING THE NEW NAME, so it is counted rather than + // silently accepted. `4ca1f13` found a live "bulk" arm answering 1.02 verdicts per call and + // diagnosed it as the per-output shape refuted at 6% live-kept; asserting one CALL was not enough + // to catch it, because one call carrying one item is exactly that. The call still proceeds -- a + // transcript can legitimately have one candidate above the floor -- but a workload where this + // fires routinely has an upstream filter starving the batch, which is the failure that cost three + // iterations. + if len(items) < 2 { + o.gate("sweep_batch_of_one") + } ctx, cancel := context.WithTimeout(c.Ctx, llmCallTimeout) defer cancel() ctx, callSink := cheapmodel.WithCallSink(ctx) - before := schema.TextTokens(content) + var before int + for _, it := range items { + before += it.SizeTokens + } start := time.Now() - reply, err := model.Complete(ctx, extract.BuildAdjudicationPrompt(goal, - extract.AdjudicationItem{ID: id, SizeTokens: before, Content: content})) + // RAISE THE REPLY BUDGET. A verdict array over a full batch, each entry carrying an obligation + // label and a verbatim quote, is long, and `659e7a6` measured what happens when the budget is the + // client's default: 24 of 34 replies cut off mid-array, which parses as nothing and is + // indistinguishable from a model declining to act. Optional interface, so a client that does not + // support it is used as-is rather than refused. + if b, ok := model.(components.Budgeter); ok { + if m := b.WithMaxTokens(extract.AdjudicationReplyTokens); m != nil { + model = m + } + } else { + // Counted, because the alternative is a silent return to the truncation regime on whatever + // client shape this is. + o.gate("sweep_reply_budget_not_raised") + } + + reply, err := model.Complete(ctx, extract.BuildAdjudicationPrompt(goal, items)) latency := float64(time.Since(start).Milliseconds()) metrics.RecordExtractionCall(latency) _, inTok, outTok := callSink.Totals() @@ -500,8 +619,7 @@ func (e *ExtractSweep) adjudicate(c *components.Ctx, component string, PromptTokens: inTok, CompletionTokens: outTok, CacheRead: cr, CacheWrite: cw, CostUSD: pricing.Cost(inTok, outTok, cw, cr), - GateReason: gateReason, - Before: content, + GateReason: cands[items[0].Label].gateReason, } if ctx.Err() != nil { if errors.Is(ctx.Err(), context.DeadlineExceeded) { @@ -515,76 +633,124 @@ func (e *ExtractSweep) adjudicate(c *components.Ctx, component string, call.Rejection = "adjudication call failed: " + err.Error() return o, call } - o.gate("sweep_adjudicated") + for range items { + o.gate("sweep_adjudicated") + } - a := extract.Judge(reply, flat) - if !a.Parsed { - // A reply that opened the object but never closed it ran out of output budget; one that - // never opened it is a format failure. The remedies are opposite — raise max_tokens versus - // fix the prompt — so they get separate counters rather than one that reads as "the prompt - // is wrong". - if strings.Contains(reply, "{") && !strings.Contains(reply, "}") { + verdicts, parsed := extract.ParseVerdicts(reply) + if !parsed { + // TRUNCATION IS NOT JUNK, and the two need opposite fixes -- raise the budget versus fix the + // prompt -- so one name for both hid a 70%-of-calls failure behind a label that reads as "the + // prompt is wrong". + if extract.ReplyWasTruncated(reply) { o.gate("sweep_reply_truncated") } else { o.gate("sweep_unparseable") } + // LOG A BOUNDED SAMPLE OF THE ACTUAL TEXT. A counter can say THAT a reply was unusable; only + // the text says WHY, and six rounds of the predecessor's failures were diagnosed by inferring + // a cause from counters with every inference at least partly wrong. if sweepUnusableSamples.Add(1) <= maxSweepUnusableSamples { head := reply if len(head) > 500 { head = head[:500] } - slog.Warn("cg.sweep.unusable_reply", "reply_len", len(reply), "head", head) + slog.Warn("cg.sweep.unusable_reply", "reply_len", len(reply), + "offered", len(items), "head", head) } - call.Rejection = "reply did not parse; output kept verbatim" + call.Rejection = "reply did not parse; every output kept verbatim" e.ratios.observe(0, before) return o, call } - if a.QuoteFabricated { - o.gate("sweep_quote_fabricated") - } - if a.CriterionMissing { - o.gate("sweep_criterion_missing") - } - if a.VerdictUnusable { - o.gate("sweep_verdict_unusable") - } - // The refusal is counted INSTEAD of a keep, not alongside it. Both leave the output verbatim, - // but "the model judged this still needed" and "the model tried to remove something it had just - // said was needed" are different events, and folding the second into the keep total is what - // would make the alertable one invisible in the ratio an operator actually looks at. - if a.RefusedObligation { - o.gate("sweep_drop_refused_obligation") + if len(verdicts) == 0 { + // A well-formed EMPTY array: the model read the batch and kept all of it. The contract + // explicitly invites that, so it must not be filed as a failure -- that conflation is what + // made "the model declined to act" and "the model was never successfully asked" the same + // number for three iterations (4ca1f13). + o.gate("sweep_kept_whole_batch") e.ratios.observe(0, before) - call.Rejection = "drop refused: the verdict named an outstanding obligation" + call.Rejection = "adjudicated: keep the whole batch" return o, call } - if !a.Drop { - o.gate("sweep_kept") - // A keep is real evidence about this workload: the adjudicator looked at this output and - // judged it still needed. Ratio 0, which is what it removed. - e.ratios.observe(0, before) - call.Rejection = "adjudicated still needed; kept verbatim" - return o, call + + offered := map[int]bool{} + for _, it := range items { + offered[it.Label] = true } - after := schema.TextTokens(sweepDescriptor(content)) - if after >= before { - // The never-worse check also lives in applySweepDrop, marker included. This one is here so - // the ratio tracker is not fed a negative saving for a decision phase 3 will refuse. - o.gate("sweep_drop_would_not_shrink") - return o, call + var removed int + seen := map[int]bool{} + for _, v := range verdicts { + if !offered[v.Label] { + // A verdict for something this batch did not offer. Never acted on: the label is how a + // decision is keyed to an output, so a wrong label is a decision about an unknown + // message and acting on it would remove the wrong content. + o.gate("sweep_verdict_unknown_label") + continue + } + if seen[v.Label] { + o.gate("sweep_verdict_duplicate_label") + continue + } + seen[v.Label] = true + content := cands[v.Label].content + a := extract.Judge(v, flat) + if a.QuoteFabricated { + o.gate("sweep_quote_fabricated") + } + if a.CriterionMissing { + o.gate("sweep_criterion_missing") + } + if a.VerdictUnusable { + o.gate("sweep_verdict_unusable") + } + // The refusal is counted INSTEAD of a keep, not alongside it. Both leave the output verbatim, + // but "the model judged this still needed" and "the model tried to remove something it had + // just said was needed" are different events, and folding the second into the keep total is + // what would make the alertable one invisible in the ratio an operator actually looks at. + if a.RefusedObligation { + o.gate("sweep_drop_refused_obligation") + continue + } + if !a.Drop { + o.gate("sweep_kept") + continue + } + sz := schema.TextTokens(content) + after := schema.TextTokens(sweepDescriptor(content)) + if after >= sz { + // The never-worse check also lives in applySweepDrop, marker included. This one is here + // so the ratio tracker is not fed a negative saving for a decision phase 3 will refuse. + o.gate("sweep_drop_would_not_shrink") + continue + } + o.gate("sweep_dropped") + o.drop = append(o.drop, v.Label) + removed += sz - after + metrics.RecordExtractionSaving(sz - after) + metrics.RecordExtractionValue(float64(sz-after) * savedTokenValue(c).perToken) + } + // An output this batch offered that no verdict mentioned is UNJUDGED, and it must not look like a + // keep: `4ca1f13` found a live arm where the model silently omitted labels and the missing answers + // were invisible, so "the batch is starved" and "the model answered for a third of it" were the + // same number. + for _, it := range items { + if !seen[it.Label] { + o.gate("sweep_verdict_missing") + } + } + // Feed the observed ratio so the gate prices FUTURE batches on what this workload actually + // achieves. Fed once per CALL over the whole batch, which is the unit the gate now prices. + e.ratios.observe(removed, before) + if removed > 0 { + call.Accepted = true + call.SavedTokens = removed + } else { + call.Rejection = "adjudicated: nothing in this batch was spent" } - o.gate("sweep_dropped") - o.drop = true - call.Accepted = true - call.SavedTokens = before - after - call.After = sweepDescriptor(content) - e.ratios.observe(before-after, before) - metrics.RecordExtractionSaving(before - after) - metrics.RecordExtractionValue(float64(before-after) * savedTokenValue(c).perToken) if debugExtractLLM(c) { - logging.From(c.Ctx).Debug("cg.sweep.drop", "candidate_tokens", before, - "residue_tokens", after, "criterion_missing", a.CriterionMissing, - "quote_fabricated", a.QuoteFabricated) + logging.From(c.Ctx).Debug("cg.sweep.batch", "offered", len(items), + "verdicts", len(verdicts), "dropped", len(o.drop), + "candidate_tokens", before, "removed_tokens", removed) } return o, call } diff --git a/components/offload/extract_sweep_test.go b/components/offload/extract_sweep_test.go index 994107e..fa19d91 100644 --- a/components/offload/extract_sweep_test.go +++ b/components/offload/extract_sweep_test.go @@ -2,7 +2,10 @@ package offload import ( "context" + "regexp" + "strconv" "strings" + "sync" "sync/atomic" "testing" @@ -10,6 +13,7 @@ import ( "github.com/rossoctl/context-guru/components" "github.com/rossoctl/context-guru/expand" + "github.com/rossoctl/context-guru/internal/extract" "github.com/rossoctl/context-guru/schema" "github.com/rossoctl/context-guru/store" ) @@ -74,7 +78,7 @@ func sweepCtx(session string, cold bool, idleMs int64, st store.Store) *componen // The whole point of the component: on a cold turn it removes a spent output at DEPTH, leaves a // shape descriptor plus a recoverable marker, and recovers the original byte-for-byte. func TestSweepDropsASpentOutputAtDepthAndKeepsItRecoverable(t *testing.T) { - model := &verdictModel{reply: `{"needed_by":"none","quote":"","verdict":"drop"}`} + model := &verdictModel{reply: `[{"i":0,"needed_by":"none","quote":"","verdict":"drop"}]`} e := newSweep(t, model, "") req := sweepReq() original := schema.MessageText(req.Input[1]) @@ -110,7 +114,7 @@ func TestSweepDropsASpentOutputAtDepthAndKeepsItRecoverable(t *testing.T) { // A keep leaves the output VERBATIM. Not "smaller" — verbatim, because there is no rewriting on this // path and a keep that changed a byte would be the very failure the split exists to remove. func TestSweepKeepLeavesTheOutputVerbatim(t *testing.T) { - model := &verdictModel{reply: `{"needed_by":"c","quote":"Next I will patch the timeout in src/api/users.py.","verdict":"keep"}`} + model := &verdictModel{reply: `[{"i":0,"needed_by":"c","quote":"Next I will patch the timeout in src/api/users.py.","verdict":"keep"}]`} e := newSweep(t, model, "") req := sweepReq() original := schema.MessageText(req.Input[1]) @@ -139,7 +143,7 @@ func TestSweepKeepLeavesTheOutputVerbatim(t *testing.T) { // The refusal, reaching all the way through the component: a drop naming an outstanding obligation // leaves the output in place and raises the alertable counter. func TestSweepRefusesADropThatNamesAnObligation(t *testing.T) { - model := &verdictModel{reply: `{"needed_by":"c","quote":"Next I will patch the timeout in src/api/users.py.","verdict":"drop"}`} + model := &verdictModel{reply: `[{"i":0,"needed_by":"c","quote":"Next I will patch the timeout in src/api/users.py.","verdict":"drop"}]`} e := newSweep(t, model, "") req := sweepReq() original := schema.MessageText(req.Input[1]) @@ -164,7 +168,7 @@ func TestSweepRefusesADropThatNamesAnObligation(t *testing.T) { // A warm turn makes no call and touches nothing, however large the candidate. This is the condition // the whole component rests on: acting at depth is only free because the provider's entry is gone. func TestSweepDoesNothingOnAWarmTurn(t *testing.T) { - model := &verdictModel{reply: `{"needed_by":"none","verdict":"drop"}`} + model := &verdictModel{reply: `[{"i":0,"needed_by":"none","verdict":"drop"}]`} e := newSweep(t, model, "") req := sweepReq() original := schema.MessageText(req.Input[1]) @@ -209,7 +213,7 @@ func TestSweepMinIdleRaisesTheBar(t *testing.T) { // that, 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. func TestSweepReplaysItsDropOnTheNextWarmTurn(t *testing.T) { - model := &verdictModel{reply: `{"needed_by":"none","quote":"","verdict":"drop"}`} + model := &verdictModel{reply: `[{"i":0,"needed_by":"none","quote":"","verdict":"drop"}]`} e := newSweep(t, model, "") st := store.NewMemory(store.Options{}) @@ -245,59 +249,229 @@ func TestSweepReplaysItsDropOnTheNextWarmTurn(t *testing.T) { } } -// The cap is the sweep's ONLY brake, so it must bind. Unbounded was measured at 27 calls, $0.229 and -// 76.6 s added to a turn whose upstream took 33.5 s. -func TestSweepCapBinds(t *testing.T) { - model := &verdictModel{reply: `{"needed_by":"none","quote":"","verdict":"drop"}`} +// THE BATCH IS ASSEMBLED AS A BATCH. This is the assertion `4ca1f13` says was missing: one call +// carrying one item is the per-output design refuted at 6% live-kept, so asserting a single call is not +// enough — the call must be shown MORE THAN ONE output. Twelve candidates must be one call of twelve. +func TestSweepOffersTheWholeBatchInOneCall(t *testing.T) { + const n = 12 + model := &labelModel{verdict: "drop", needed: "none"} + e := newSweep(t, model, "") + req := manyCandidates(n) + rep := &components.Report{} + if _, err := e.Offload(req, rep, sweepCtx("s", true, 3_600_000, store.NewMemory(store.Options{}))); err != nil { + t.Fatal(err) + } + if got := atomic.LoadInt64(&model.calls); got != 1 { + t.Fatalf("%d candidates took %d calls; the measured shape is ONE call over the batch", n, got) + } + // PRECONDITION and the point of the test: every candidate was OFFERED in that one call. + if rep.Gates["sweep_offered"] != n { + t.Fatalf("sweep_offered = %d, want %d — the batch was starved before assembly (gates: %v)", + rep.Gates["sweep_offered"], n, rep.Gates) + } + if rep.Gates["sweep_adjudicated"] != n { + t.Errorf("sweep_adjudicated = %d, want %d", rep.Gates["sweep_adjudicated"], n) + } + if rep.Gates["sweep_batch_of_one"] != 0 { + t.Errorf("a batch of %d was recorded as a batch of one (gates: %v)", n, rep.Gates) + } + // And the prompt must actually show all twelve, labelled. + p := e2ePrompt(t, model.prompt()) + for i := 0; i < n; i++ { + if !strings.Contains(p, "=== OUTPUT "+strconv.Itoa(i)) { + t.Errorf("output %d was not offered in the prompt", i) + } + } +} + +// A single-item batch is the refuted design wearing the new name, so it is COUNTED. It is legitimate +// — a transcript can have one candidate above the floor — but a workload where it fires routinely has +// an upstream filter starving the batch, which is the failure that cost three iterations. +func TestSweepCountsABatchOfOne(t *testing.T) { + model := &labelModel{verdict: "keep", needed: "a", quote: "Find the auth timeout"} + e := newSweep(t, model, "") + rep := &components.Report{} + if _, err := e.Offload(sweepReq(), rep, sweepCtx("s", true, 3_600_000, store.NewMemory(store.Options{}))); err != nil { + t.Fatal(err) + } + if rep.Gates["sweep_adjudicated"] != 1 { + t.Fatalf("the component did not adjudicate, so nothing under test ran (gates: %v)", rep.Gates) + } + if rep.Gates["sweep_batch_of_one"] != 1 { + t.Fatalf("a batch of one was not counted; a starved batch would be invisible (gates: %v)", + rep.Gates) + } +} + +// Past the item cap the sweep uses more batches, and max_calls bounds how many. Neither bound may be +// silent: a truncated sweep is a bounded-coverage decision, and "we judged everything" must not read +// the same as "we judged the first twelve". +func TestSweepBatchesPastTheItemCapAndCountsWhatItTruncated(t *testing.T) { + const n = 30 // three batches of 12/12/6 + model := &labelModel{verdict: "keep", needed: "none"} e := newSweep(t, model, "max_calls: 2\n") - req := &bschemas.BifrostChatRequest{Input: []bschemas.ChatMessage{ - userMsg("do the thing"), - }} - for i := 0; i < 5; i++ { - req.Input = append(req.Input, - toolResultMsg(strings.Repeat("distinct line "+string(rune('a'+i))+"\n", 900))) + rep := &components.Report{} + if _, err := e.Offload(manyCandidates(n), rep, + sweepCtx("s", true, 3_600_000, store.NewMemory(store.Options{}))); err != nil { + t.Fatal(err) + } + if got := atomic.LoadInt64(&model.calls); got != 2 { + t.Fatalf("max_calls: 2 allowed %d calls (gates: %v)", got, rep.Gates) + } + // Two batches of twelve were judged; the remaining six were truncated and counted. + if rep.Gates["sweep_adjudicated"] != 24 { + t.Errorf("sweep_adjudicated = %d, want 24 (two full batches)", rep.Gates["sweep_adjudicated"]) + } + if rep.Gates["sweep_batch_truncated"] != n-24 { + t.Errorf("sweep_batch_truncated = %d, want %d — bounded coverage must be visible", + rep.Gates["sweep_batch_truncated"], n-24) + } + // No batch may exceed the measured quote-fidelity ceiling. + if got := model.maxItems(); got > extract.MaxAdjudicationItems { + t.Errorf("a batch offered %d items, above the measured ceiling of %d", + got, extract.MaxAdjudicationItems) + } +} + +// A well-formed EMPTY array is the model saying "keep everything", which the contract invites. It must +// not be filed as a failure — that conflation made "the model declined to act" and "the model was +// never successfully asked" the same number for three iterations. +func TestSweepCountsADeliberateKeepAllSeparatelyFromAFailure(t *testing.T) { + keepAll := &verdictModel{reply: "[]"} + e := newSweep(t, keepAll, "") + req := sweepReq() + original := schema.MessageText(req.Input[1]) + rep := &components.Report{} + if _, err := e.Offload(req, rep, sweepCtx("s", true, 3_600_000, store.NewMemory(store.Options{}))); err != nil { + t.Fatal(err) + } + if rep.Gates["sweep_adjudicated"] == 0 { + t.Fatalf("no call was made, so nothing under test ran (gates: %v)", rep.Gates) + } + if rep.Gates["sweep_kept_whole_batch"] != 1 { + t.Fatalf("a deliberate keep-all was not counted as one (gates: %v)", rep.Gates) + } + if rep.Gates["sweep_unparseable"] != 0 || rep.Gates["sweep_reply_truncated"] != 0 { + t.Errorf("a deliberate keep-all was filed as a failure (gates: %v)", rep.Gates) + } + if schema.MessageText(req.Input[1]) != original { + t.Error("keep-all removed something") } + + // A TRUNCATED reply, by contrast, is a failure — and a different one from malformed junk. + cut := &verdictModel{reply: `[{"i":0,"needed_by":"none","quote":"partial`} + e2 := newSweep(t, cut, "") + rep2 := &components.Report{} + if _, err := e2.Offload(sweepReq(), rep2, sweepCtx("s2", true, 3_600_000, store.NewMemory(store.Options{}))); err != nil { + t.Fatal(err) + } + if rep2.Gates["sweep_reply_truncated"] != 1 { + t.Fatalf("a cut-off array was not counted as truncation (gates: %v)", rep2.Gates) + } + if rep2.Gates["sweep_unparseable"] != 0 { + t.Errorf("truncation was filed as a format failure; the two need opposite fixes (gates: %v)", + rep2.Gates) + } + if rep2.Gates["sweep_kept_whole_batch"] != 0 { + t.Errorf("a truncated reply was filed as a deliberate keep-all (gates: %v)", rep2.Gates) + } +} + +// A verdict naming a label the batch never offered must never be acted on: the label is how a decision +// is keyed to an output, so acting on a wrong one removes the wrong content. +func TestSweepIgnoresAVerdictForAnUnofferedLabel(t *testing.T) { + model := &verdictModel{reply: `[{"i":99,"needed_by":"none","quote":"","verdict":"drop"}]`} + e := newSweep(t, model, "") + req := sweepReq() + original := schema.MessageText(req.Input[1]) rep := &components.Report{} if _, err := e.Offload(req, rep, sweepCtx("s", true, 3_600_000, store.NewMemory(store.Options{}))); err != nil { t.Fatal(err) } - if n := atomic.LoadInt64(&model.calls); n != 2 { - t.Fatalf("max_calls: 2 allowed %d calls (gates: %v)", n, rep.Gates) + if rep.Gates["sweep_adjudicated"] == 0 { + t.Fatalf("no call was made, so nothing under test ran (gates: %v)", rep.Gates) + } + if rep.Gates["sweep_verdict_unknown_label"] != 1 { + t.Fatalf("a verdict for an unoffered label was not counted (gates: %v)", rep.Gates) + } + if rep.Gates["sweep_dropped"] != 0 { + t.Fatalf("a verdict for an unoffered label was ACTED ON (gates: %v)", rep.Gates) + } + if schema.MessageText(req.Input[1]) != original { + t.Fatal("content was removed on a verdict that named no offered output") + } + // The offered output got no answer, and that must not look like a keep. + if rep.Gates["sweep_verdict_missing"] != 1 { + t.Errorf("an unjudged output was not counted (gates: %v)", rep.Gates) + } +} + +// The reply budget must be RAISED for a batched reply. `659e7a6`: 24 of 34 replies were cut off at the +// client's default, which parses as nothing and is indistinguishable from a model declining to act. +func TestSweepRaisesTheReplyBudget(t *testing.T) { + model := &budgetModel{verdictModel: verdictModel{reply: "[]"}} + e := newSweep(t, model, "") + rep := &components.Report{} + if _, err := e.Offload(sweepReq(), rep, sweepCtx("s", true, 3_600_000, store.NewMemory(store.Options{}))); err != nil { + t.Fatal(err) + } + if rep.Gates["sweep_adjudicated"] == 0 { + t.Fatalf("no call was made, so the budget was never requested (gates: %v)", rep.Gates) + } + if got := model.granted(); got != extract.AdjudicationReplyTokens { + t.Fatalf("the sweep asked for a %d-token reply budget, want %d", got, + extract.AdjudicationReplyTokens) } - if rep.Gates["over_sweep_cap"] != 3 { - t.Errorf("the three refused candidates were not counted (gates: %v)", rep.Gates) + if rep.Gates["sweep_reply_budget_not_raised"] != 0 { + t.Errorf("a Budgeter client was recorded as unable to raise its budget (gates: %v)", rep.Gates) + } +} + +// A client that cannot raise its budget still works, and says so — otherwise the truncation regime +// returns silently on whatever client shape that is. +func TestSweepCountsAClientThatCannotRaiseItsBudget(t *testing.T) { + model := &verdictModel{reply: "[]"} // no WithMaxTokens + e := newSweep(t, model, "") + rep := &components.Report{} + if _, err := e.Offload(sweepReq(), rep, sweepCtx("s", true, 3_600_000, store.NewMemory(store.Options{}))); err != nil { + t.Fatal(err) + } + if rep.Gates["sweep_adjudicated"] == 0 { + t.Fatalf("no call was made (gates: %v)", rep.Gates) + } + if rep.Gates["sweep_reply_budget_not_raised"] != 1 { + t.Fatalf("a client without a budget knob was not counted (gates: %v)", rep.Gates) } } // EVERY CANDIDATE MUST BE ACCOUNTED FOR, and the accounting must survive concurrency. // -// This is not a style test. The gates were originally raised from inside the per-call goroutines, -// and components.Report's Gates map carries no lock — a Report is copied by value across this -// codebase and cannot hold one — so Go's map implementation turned it into -// `fatal error: concurrent map writes` and killed the test binary rather than producing a wrong -// count. Sixteen candidates with the cap lifted puts more than llmConcurrency calls in flight, which -// is what it takes to reach it. +// This is not a style test. The gates were originally raised from inside the per-call goroutines, and +// components.Report's Gates map carries no lock — a Report is copied by value across this codebase and +// cannot hold one — so Go's map implementation turned it into `fatal error: concurrent map writes` and +// killed the test binary rather than producing a wrong count. Batching did not remove the hazard: the +// batches still fan out, so five concurrent batch calls reach it just as five per-output calls did. // // Under `-race` the reversion is caught deterministically (a DATA RACE on components.Report.Gate). // Without it the crash is timing-dependent, so the count assertions below are the part that holds in // the plain suite: a lost gate is the quiet form of the same defect. func TestSweepAccountsForEveryCandidateUnderConcurrency(t *testing.T) { - const n = 16 - model := &verdictModel{reply: `{"needed_by":"none","quote":"","verdict":"drop"}`} + const n = 60 // five batches of twelve + model := &labelModel{verdict: "drop", needed: "none"} e := newSweep(t, model, "max_calls: -1\n") - req := &bschemas.BifrostChatRequest{Input: []bschemas.ChatMessage{userMsg("do the thing")}} - for i := 0; i < n; i++ { - req.Input = append(req.Input, - toolResultMsg(strings.Repeat("candidate "+string(rune('a'+i))+" line\n", 900))) - } rep := &components.Report{} - if _, err := e.Offload(req, rep, sweepCtx("s", true, 3_600_000, store.NewMemory(store.Options{}))); err != nil { + if _, err := e.Offload(manyCandidates(n), rep, + sweepCtx("s", true, 3_600_000, store.NewMemory(store.Options{}))); err != nil { t.Fatal(err) } - // PRECONDITION: all sixteen really went to the model, so more than llmConcurrency were in - // flight at once. With the cap binding this would be 4 and the test would prove nothing. - if got := atomic.LoadInt64(&model.calls); got != n { - t.Fatalf("expected %d concurrent adjudications, got %d (gates: %v)", n, got, rep.Gates) + // PRECONDITION: more than llmConcurrency batches really went to the model, so several were in + // flight at once. With the cap binding this would be fewer and the test would prove nothing. + if got := atomic.LoadInt64(&model.calls); got != n/extract.MaxAdjudicationItems { + t.Fatalf("expected %d concurrent batch calls, got %d (gates: %v)", + n/extract.MaxAdjudicationItems, got, rep.Gates) + } + if rep.Gates["sweep_offered"] != n { + t.Errorf("sweep_offered = %d, want %d — a gate was lost", rep.Gates["sweep_offered"], n) } if rep.Gates["sweep_adjudicated"] != n { t.Errorf("sweep_adjudicated = %d, want %d — a gate was lost", rep.Gates["sweep_adjudicated"], n) @@ -333,6 +507,95 @@ func TestSweepRejectsCompactionOnlyKeys(t *testing.T) { } } +// THE ECONOMIC GATE MUST NOT THIN THE BATCH. This is the failure `4ca1f13` traced: the merged arm's +// real defect was an upstream PER-CANDIDATE filter (prefix_still_referenced removed 149,681 +// candidates), which left about one candidate per request and silently ran the per-output design +// refuted at 6% live-kept while reporting itself as bulk. A per-candidate economic gate is the same +// mechanism, so the gate is evaluated ONCE for the batch. +// +// Run with the gate ON, which every other test in this file turns off — without this, the whole gated +// path is untested and a reversion to per-candidate gating passes the suite. +func TestSweepEconomicGateDoesNotThinTheBatch(t *testing.T) { + const n = 12 + model := &labelModel{verdict: "drop", needed: "none"} + c, err := newExtractSweep([]byte("min_tokens: 2000\n")) // gate left at its default: ON + if err != nil { + t.Fatal(err) + } + e := c.(*ExtractSweep) + e.modelClient = model + if !e.gate { + t.Fatal("this test is meaningless with the gate off") + } + rep := &components.Report{} + if _, err := e.Offload(manyCandidates(n), rep, + sweepCtx("s", true, 3_600_000, store.NewMemory(store.Options{}))); err != nil { + t.Fatal(err) + } + // PRECONDITION: the gate ALLOWED this batch. If it suppressed, the assertions below would pass + // vacuously — a batch of zero is not a thinned batch, it is no batch. + if atomic.LoadInt64(&model.calls) == 0 { + t.Fatalf("the gate suppressed a batch of %d cold-turn candidates, so thinning was never "+ + "exercised (gates: %v)", n, rep.Gates) + } + if rep.Gates["sweep_offered"] != n { + t.Fatalf("sweep_offered = %d, want %d: the gate thinned the batch one candidate at a time, "+ + "which is how a bulk arm silently becomes the refuted per-output shape (gates: %v)", + rep.Gates["sweep_offered"], n, rep.Gates) + } + if rep.Gates["sweep_batch_of_one"] != 0 { + t.Errorf("the gate reduced the batch to one (gates: %v)", rep.Gates) + } + // And it must be an all-or-nothing decision: a partial refusal is per-candidate gating by + // another name. + if g := rep.Gates["economic_gate"]; g != 0 && g != n { + t.Errorf("economic_gate refused %d of %d candidates; the decision must cover the whole batch", + g, n) + } +} + +// The other direction, so the gate is not simply inert: a batch whose total cannot pay for one call is +// refused as a whole, and every candidate it would have covered is counted so the refusal is +// comparable with the other gates rather than reading as one. +func TestSweepEconomicGateRefusesABatchThatCannotPay(t *testing.T) { + model := &labelModel{verdict: "drop", needed: "none"} + // A floor low enough to admit tiny candidates, so the batch total is far below break-even. + c, err := newExtractSweep([]byte("min_tokens: 5\n")) + if err != nil { + t.Fatal(err) + } + e := c.(*ExtractSweep) + e.modelClient = model + req := &bschemas.BifrostChatRequest{Input: []bschemas.ChatMessage{userMsg("tiny task")}} + const n = 3 + for i := 0; i < n; i++ { + req.Input = append(req.Input, toolResultMsg("small output "+strconv.Itoa(i)+"\n")) + } + // Exhaust the exploration budget first: exploration deliberately allows a bounded number of + // unprofitable calls so a pessimistic prior cannot justify itself forever, and it would otherwise + // mask the gate's arithmetic here. + for i := 0; i < 8; i++ { + e.ratios.observe(0, 4000) + } + rep := &components.Report{} + if _, err := e.Offload(req, rep, sweepCtx("s", true, 3_600_000, store.NewMemory(store.Options{}))); err != nil { + t.Fatal(err) + } + // PRECONDITION: the candidates cleared every earlier gate and reached the economic one. If they + // were stopped by the floor or the depth gate, this proves nothing about the economics. + if rep.Gates["below_output_floor"] != 0 { + t.Fatalf("candidates were stopped by the floor, not the gate (gates: %v)", rep.Gates) + } + if rep.Gates["economic_gate"] != n { + t.Fatalf("economic_gate = %d, want %d: an unprofitable batch must be refused as a whole and "+ + "counted per candidate it would have covered (gates: %v)", + rep.Gates["economic_gate"], n, rep.Gates) + } + if got := atomic.LoadInt64(&model.calls); got != 0 { + t.Errorf("a refused batch still made %d calls", got) + } +} + // THE COUNTER CONTRACT, component end. These six names are what an operator's dashboard query and // alert rule are written against, so a rename breaks monitoring silently rather than loudly. The // other end is pinned in proxy/sweep_counters_test.go, which asserts the same literal strings survive @@ -348,15 +611,15 @@ func TestSweepRaisesTheContractedCounterNames(t *testing.T) { name, reply string want []string }{ - {"a spent output", `{"needed_by":"none","quote":"","verdict":"drop"}`, + {"a spent output", `[{"i":0,"needed_by":"none","quote":"","verdict":"drop"}]`, []string{"sweep_adjudicated", "sweep_dropped"}}, - {"an output still needed", `{"needed_by":"a","quote":"` + obligation + `","verdict":"keep"}`, + {"an output still needed", `[{"i":0,"needed_by":"a","quote":"` + obligation + `","verdict":"keep"}]`, []string{"sweep_adjudicated", "sweep_kept"}}, - {"a drop contradicting an obligation", `{"needed_by":"a","quote":"` + obligation + `","verdict":"drop"}`, + {"a drop contradicting an obligation", `[{"i":0,"needed_by":"a","quote":"` + obligation + `","verdict":"drop"}]`, []string{"sweep_adjudicated", "sweep_drop_refused_obligation"}}, - {"an invented obligation", `{"needed_by":"a","quote":"rewrite the parser in Rust","verdict":"keep"}`, + {"an invented obligation", `[{"i":0,"needed_by":"a","quote":"rewrite the parser in Rust","verdict":"keep"}]`, []string{"sweep_quote_fabricated"}}, - {"an unanswered criterion", `{"verdict":"drop"}`, + {"an unanswered criterion", `[{"i":0,"verdict":"drop"}]`, []string{"sweep_criterion_missing"}}, } { t.Run(tc.name, func(t *testing.T) { @@ -385,13 +648,13 @@ func TestSweepRaisesTheContractedCounterNames(t *testing.T) { // The prompt the component actually sends must be the adjudication contract — not a compaction // prompt, and never one inviting the model to return content. func TestSweepSendsTheAdjudicationContract(t *testing.T) { - model := &verdictModel{reply: `{"needed_by":"none","verdict":"drop"}`} + model := &verdictModel{reply: `[{"i":0,"needed_by":"none","verdict":"drop"}]`} e := newSweep(t, model, "") rep := &components.Report{} if _, err := e.Offload(sweepReq(), rep, sweepCtx("s", true, 3_600_000, store.NewMemory(store.Options{}))); err != nil { t.Fatal(err) } - p := e2ePrompt(t, model) + p := e2ePrompt(t, model.lastPrompt()) for _, want := range []string{"keep|drop", `"needed_by"`, "SPENT only if"} { if !strings.Contains(p, want) { t.Errorf("the prompt does not carry %q", want) @@ -404,11 +667,76 @@ func TestSweepSendsTheAdjudicationContract(t *testing.T) { } } -func e2ePrompt(t *testing.T, m *verdictModel) string { +func e2ePrompt(t *testing.T, p string) string { t.Helper() - p := m.lastPrompt() if p == "" { t.Fatal("no prompt was sent, so there is nothing to assert about it") } return p } + +// manyCandidates builds a transcript of n tool outputs, each distinct and each above the sweep's +// floor, so batch assembly and the caps are exercised on real candidate counts. +func manyCandidates(n int) *bschemas.BifrostChatRequest { + req := &bschemas.BifrostChatRequest{Input: []bschemas.ChatMessage{userMsg("do the thing")}} + for i := 0; i < n; i++ { + req.Input = append(req.Input, + toolResultMsg(strings.Repeat("candidate "+strconv.Itoa(i)+" distinct line\n", 900))) + } + return req +} + +// labelModel answers a whole batch: it reads the labels out of the prompt and returns one verdict per +// label. Without it a batch test could only ever exercise the first candidate, which is precisely the +// blind spot that let a batch-of-one arm pass for bulk. +type labelModel struct { + verdict, needed, quote string + calls int64 + mu sync.Mutex + seenMax int + last string +} + +var labelRe = regexp.MustCompile(`=== OUTPUT (\d+) `) + +func (m *labelModel) Complete(_ context.Context, prompt string) (string, error) { + atomic.AddInt64(&m.calls, 1) + labels := labelRe.FindAllStringSubmatch(prompt, -1) + m.mu.Lock() + m.last = prompt + if len(labels) > m.seenMax { + m.seenMax = len(labels) + } + m.mu.Unlock() + parts := make([]string, 0, len(labels)) + for _, l := range labels { + parts = append(parts, `{"i":`+l[1]+`,"needed_by":"`+m.needed+ + `","quote":"`+m.quote+`","verdict":"`+m.verdict+`"}`) + } + return "[" + strings.Join(parts, ",") + "]", nil +} + +func (m *labelModel) maxItems() int { + m.mu.Lock() + defer m.mu.Unlock() + return m.seenMax +} + +func (m *labelModel) prompt() string { + m.mu.Lock() + defer m.mu.Unlock() + return m.last +} + +// budgetModel records the reply budget the caller asked for, so the raise is observable. +type budgetModel struct { + verdictModel + got atomic.Int64 +} + +func (m *budgetModel) WithMaxTokens(n int) components.Model { + m.got.Store(int64(n)) + return m +} + +func (m *budgetModel) granted() int { return int(m.got.Load()) } diff --git a/docs/proposals/sweep-adjudicator.md b/docs/proposals/sweep-adjudicator.md index 7c96622..3c34849 100644 --- a/docs/proposals/sweep-adjudicator.md +++ b/docs/proposals/sweep-adjudicator.md @@ -1,7 +1,16 @@ # The cold sweep should adjudicate, not compact -**Status:** proposed, not implemented. This document is the specification; the implementation is -the work. +**Status:** implemented as `extract_llm_sweep`. + +**CORRECTION, 2026-08-27.** The version of this document that was implemented from argued for ONE +CALL PER OUTPUT, and its argument was inverted: it cited `4ca1f13` as evidence that per-output +adjudication "is a shape that has already run", when that commit diagnoses the per-output shape as a +DEFECT — an arm that had degraded to 1.02 verdicts per call, which it names as "the per-output design +refuted at 6% live-kept, not the bulk shape that measured 58%". The secondary argument (that batching +invites truncated replies and degraded quote fidelity) was also wrong: both were already solved on +`feat/coref-compaction` before this document was written. The section below has been rewritten. The +safety invariants, the counters and the config surface were unaffected — they are per-verdict +properties and hold at any batch size. ## The problem @@ -49,12 +58,55 @@ The model returns a verdict and a quote. It never returns content. That removes output and either keeps it **verbatim** or drops it, leaving a short shape descriptor plus the existing `<>` marker so `expand` still recovers the original. -**One call per output.** Not a batch. `4ca1f13` established that the merged mode "was never bulk — -it adjudicated one output per call", so per-output adjudication is a shape that has already run. -Batching is what remains experimental, and it is what forces the failure modes this design avoids: -a shared reply that can be truncated mid-array, quote fidelity degrading with batch size (4 of 37 -quotes non-verbatim at batch 16 against 0 of 16 at batch 10), and a batch-truncation counter to -compensate. A per-output call has none of those. +**One call per BATCH.** `4ca1f13` is the commit this rests on, and it points the other way from how +it was first read. It found a live arm reporting 2,030 bulk calls and 2,074 verdicts — 1.02 verdicts +per call, so every "bulk" adjudication judged a single output — 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"*. + +The measurement behind that, from `docs/results/coref-selection-experiment.md` 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. + +`cc1aa9f` gives the direction of the failure, which is the one a SWEEP specifically 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; an adjudicator too timid to remove anything is an +expensive no-op exactly where the money is. + +The batch is capped at **12 items**, a measured ceiling rather than a round number: quote fidelity +degraded with batch size, 4 of 37 quotes non-verbatim at batch 16 against 0 of 16 at batch 10, so the +transport limit sits between them and 12 takes the conservative end (`cc1aa9f`). + +**Neither objection to batching survives.** Both were solved on `feat/coref-compaction` before this +document was written, so they never distinguished the two shapes: + +- *A shared reply truncated mid-array.* `659e7a6` traced 24 of 34 unparseable replies to a 2048-token + output budget and raised it to 16,000. A verdict array over 12 items, each carrying an obligation + label and a verbatim quote, is simply long, and a model running adaptive thinking spends part of the + budget before emitting any text. Output bills as generated rather than as budgeted, so the ceiling + costs nothing until used. Truncation is now counted separately from a format failure, because the + two need opposite fixes — raise the budget versus fix the prompt. +- *Quote fidelity decaying with batch size.* Measured, and the cap is set below the observed ceiling + (above). + +And the **transport principle never distinguished them at all**. Once `trim` is removed, no verdict +carries content in either shape: there is no reply field a model could return output text through. +That argument rules out rewriting; it says nothing about how many verdicts travel in one reply. + +**Do not thin the batch upstream.** `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. Any per-candidate gate ahead of batch assembly reproduces this: it thins the batch one +output at a time until comparative judgement has nothing to compare, and returns the component to +batch-of-one silently. That is why the economic gate is evaluated ONCE for the batch rather than per +candidate — which is also the correct arithmetic, since one call now covers up to twelve candidates and +charging each of them a whole call priced the batch at ~12x its real cost. **Reuse, do not fork.** The adjudication contract text should move to a shared location rather than being copied out of `bulk.go` — the *contract* is general, the *batching* is not. The model @@ -77,6 +129,16 @@ rewritten text. `rewrite` in particular becomes moot rather than merely defaulte governs how a *rewritten* result is validated, and nothing is rewritten. Any of these appearing under `extract_llm_sweep` should be a config error naming the reason, not a silently ignored key. +`max_calls` bounds BATCH calls, not per-output calls. The item cap above and this are independent +brakes: one is a measured quote-fidelity ceiling, the other a spend/latency bound. It defaults to one +concurrency round rather than to a single call, because with one call a transcript carrying 40 +candidates would have 12 adjudicated and 28 left verbatim — on the one turn whose whole point is that +everything is re-billing at the write rate, that is leaving most of the money. Nothing measured +compares four batches of 12 against one batch of 12; batch SIZE is the variable the experiments moved, +and the per-batch shape is identical either way. It must never truncate the CANDIDATE list instead of +the batch list: `max_calls: 4` leaving four candidates is a batch of four, which is the size at which +the model was measured unwilling to act. + `per_output` and the `cold_cache` block disappear from `extract_llm`, along with the `per_output: false with cold_cache disabled leaves the component with nothing to do` error — that error is the seam this split removes. @@ -108,10 +170,10 @@ second means it is inventing evidence. ## Open questions -- **Is the obligation quote worth its tokens per-output?** Requiring evidence halved false drops - in the merged probes (4/4 → 2/4), but that was measured at batch size, where one reply covered - many outputs. Per-output the quote is a larger share of each reply. Worth measuring before - assuming it carries over. +- **Is the obligation quote worth its tokens?** Requiring evidence halved false drops in the merged + probes (4/4 → 2/4), measured at batch size — which is the shape now shipped, so the measurement + applies directly. It is kept. (The earlier framing of this question assumed per-output calls, where + the quote would have been a much larger share of each reply; that concern is gone with the shape.) - **Does a spent-ness judgement need `context: full`?** Deciding "needed by nothing" plausibly requires seeing the whole transcript, which is the expensive context mode. If so, that pairing should be the sweep's default rather than an operator's discovery. @@ -119,7 +181,7 @@ second means it is inventing evidence. than a fraction of it, so the saving per call is much larger than for a compaction, and the gate's current arithmetic is calibrated on the latter. -## Execution order +## Execution order (as landed) 1. The contract and its parser, in `internal/extract`, with invariants 1–4 as unit tests. No component wiring — this slice is verifiable alone. diff --git a/internal/cheapmodel/anthropic.go b/internal/cheapmodel/anthropic.go index 0a2d8bf..7dea58f 100644 --- a/internal/cheapmodel/anthropic.go +++ b/internal/cheapmodel/anthropic.go @@ -35,6 +35,10 @@ type Anthropic struct { // AsModel is components.Remodeler: same endpoint, same credential, different model. func (a Anthropic) AsModel(id string) components.Model { a.Model = id; return a } +// WithMaxTokens is components.Budgeter: same call, larger reply budget. See the interface for why a +// caller asking a long question has to be able to raise this. +func (a Anthropic) WithMaxTokens(n int) components.Model { a.MaxTokens = n; return a } + func (a Anthropic) Complete(ctx context.Context, prompt string) (string, error) { return a.CompleteSystem(ctx, "", prompt) } diff --git a/internal/cheapmodel/openai.go b/internal/cheapmodel/openai.go index cc21439..d366018 100644 --- a/internal/cheapmodel/openai.go +++ b/internal/cheapmodel/openai.go @@ -24,6 +24,10 @@ type OpenAI struct { // AsModel is components.Remodeler: same endpoint, same credential, different model. func (o OpenAI) AsModel(id string) components.Model { o.Model = id; return o } +// WithMaxTokens is components.Budgeter: same call, larger reply budget. See the interface for why a +// caller asking a long question has to be able to raise this. +func (o OpenAI) WithMaxTokens(n int) components.Model { o.MaxTokens = n; return o } + func (o OpenAI) Complete(ctx context.Context, prompt string) (string, error) { return o.CompleteSystem(ctx, "", prompt) } diff --git a/internal/extract/adjudicate.go b/internal/extract/adjudicate.go index f0b7bf6..abea75d 100644 --- a/internal/extract/adjudicate.go +++ b/internal/extract/adjudicate.go @@ -6,50 +6,115 @@ import ( "strings" ) -// COLD-SWEEP ADJUDICATION. The model returns a VERDICT, never content. +// COLD-SWEEP ADJUDICATION. The model returns VERDICTS, never content. // // This is the contract `feat/coref-compaction` arrived at (internal/extract/bulk.go), ported here -// without its batching. The contract is general; the batch is not. +// with its batching, because the batch is the part that was measured good. // -// WHY IT IS A VERDICT AND NOT A REWRITE. `cc1aa9f` removed the `trim` verdict from that design after -// measuring it: 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, which is what it is worst -// at. What survives is binary, and it removes the transporting OPERATION rather than merely the -// transporting STRATEGIES — the strongest available form of "never ask a model to transport text". +// WHY IT IS A BATCH AND NOT ONE CALL PER OUTPUT. This was got wrong once already, so the evidence is +// recorded rather than summarised. docs/results/coref-selection-experiment.md measured ten arms over +// 8,105 recorded decisions: +// +// REFUTED — the per-output shape. One call shown ONE output, deciding its fate, 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. +// +// WORKS — bulk adjudication. One call shown ~15 outputs TOGETHER lifted live-kept from 6% to 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` is the commit that makes this concrete, and it is the one that was previously misread as +// evidence FOR per-output. It found a live arm reporting 1.02 verdicts per call and diagnosed that as +// a DEFECT — "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". A single call carrying a +// single item is the refuted design wearing a new name, which is why buildBatch's caller must assert +// the batch offers more than one. +// +// `cc1aa9f` adds the direction of the failure, and it is the direction 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 cold sweep exists +// because the whole transcript is re-billing at the write rate; an adjudicator too timid to remove +// anything is an expensive no-op there. +// +// WHAT THE BATCH DOES NOT COST. The two failure modes that once argued for per-output calls were both +// solved on the branch this is ported from, so they no longer distinguish the shapes: +// +// - a shared reply truncated mid-array. `659e7a6` traced 24 of 34 unparseable replies to an output +// budget of 2048 and raised it; a verdict array over 12 items each carrying an obligation label +// and a verbatim quote is simply long. Output bills as generated, not as budgeted, so the ceiling +// costs nothing until used. See ParseVerdicts for why truncation is now counted separately from a +// format failure — the two need opposite fixes. +// - quote fidelity decaying with batch size. `cc1aa9f` measured the ceiling and capped the batch +// below it: 4 of 37 quotes non-verbatim at batch 16 against 0 of 16 at batch 10. See +// MaxAdjudicationItems. +// +// And the transport principle never distinguished them at all: post-trim, NO verdict carries content +// in either shape. There is no reply field a model could return output text through. +// +// WHY IT IS A VERDICT AND NOT A REWRITE. `cc1aa9f` removed the `trim` verdict after measuring it: +// 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, which is what it is worst at. Removing it +// removes the transporting OPERATION rather than merely the transporting STRATEGIES. // // WHY THE CRITERION IS FORCED EVIDENCE AND NOT ADVICE. Arms carrying an identical criterion differed // ONLY in whether the model had to emit which obligation still needs the output; the arm that had to // emit it HALVED the false-drop rate (4/4 -> 2/4). Stating the criterion alone measured inert. // Instructions a model can skim past are inert; a required output field is not. // -// WHAT IS DELIBERATELY ABSENT, against bulk.go. Two things, and both because there is nothing here -// for them to talk about: +// WHAT IS DELIBERATELY ABSENT, against bulk.go: the READING THE EVIDENCE section. It taught the model +// to interpret a co-reference index's counters (novel / refs / ref_age / used_frac), and there is no +// such index on `main`. Shipping the section would be teaching the model to read fields the prompt +// never carries. + +// MaxAdjudicationItems caps one adjudication batch. // -// - the READING THE EVIDENCE section. It taught the model to interpret a co-reference index's -// counters, and there is no such index on this path. Shipping the section without the counters -// would be teaching the model to read a field the prompt never carries. -// - the JUDGE THEM AGAINST EACH OTHER section. Comparative ranking is the one thing a per-output -// call structurally cannot do, so the paragraph would be a lie about the question being asked. +// 12, not 15, and the number is a measured ceiling rather than a round figure. Batch size is a +// YIELD/SAFETY trade-off (docs/experiments/loca/iter019/results.md, "batch size"): at batch 3-6 the +// model dropped a genuinely-spent output only half the time, at batch 10 it cleared 100% of the +// genuinely-spent candidates. But at batch 16 the transport burden started to tell -- 4 of 37 +// required quotes came back non-verbatim, against 0 of 16 at batch 10. So the ceiling sits between 10 +// and 16 and this takes the conservative end of it. +const MaxAdjudicationItems = 12 + +// AdjudicationSampleChars bounds each output shown in the prompt. // -// The second absence is a KNOWN RISK, recorded here rather than hidden: the merged experiment -// measured comparative judgement as the difference between 6% and 58% live-kept, and `4ca1f13` found -// a live arm that had degraded to 1.02 verdicts per call and read that as "the per-output design -// already refuted at 6%". So the prior on per-output YIELD is negative and the safety machinery below -// is what carries this design — every failure mode it detects resolves toward keep. +// The whole point is comparative judgement across many outputs, so the per-output budget must stay +// small enough that a full batch plus the contract still fits the adjudication model's window. +// Exported because the caller sizes its context check against the same number. +const AdjudicationSampleChars = 4000 -// AdjudicationItem is the one candidate output a single adjudication call is about. +// AdjudicationReplyTokens is the reply budget one batched adjudication needs. +// +// 16000, from `659e7a6`, which is the commit that found this: the merged arm's replies were being cut +// off at a 2048-token default and the parse failure was misread as a model declining to act for three +// iterations. A verdict array over 12 items, each carrying an obligation label and a VERBATIM quote, +// is long -- and a request model running adaptive thinking spends part of the budget before emitting +// any text at all (a probe at max_tokens 900 returned thinking blocks and no text whatsoever). Output +// bills as generated and not as budgeted, so the ceiling costs nothing until it is used. +const AdjudicationReplyTokens = 16000 + +// AdjudicationItem is one candidate output offered for adjudication. type AdjudicationItem struct { - Index int // caller's message index, for the operator's logs only - ID string // tool-call id, likewise + // Label is the small integer the model answers with, and it is small for a measured reason. + // Asked to answer with opaque tool_use ids, the model REGULARISED them -- `toolu_01..07` for + // `toolu_probe_00..07` -- because reproducing a random identifier from thousands of tokens back + // is a copying task, not a judgement. With integer labels it was 0 bad labels in 40+ trials. + // The rule generalises: give the model short things it cannot get wrong and keep every mapping + // on our side. + Label int + ID string // tool-call id, for the operator's logs only — never shown to the model SizeTokens int Content string // the output itself; BuildAdjudicationPrompt bounds what it shows } -// Verdict is the model's whole reply. Note what is NOT in it: any field carrying output content. -// A parse that succeeds cannot produce text to splice, which is what makes the transport failure -// mode unreachable rather than merely guarded. +// Verdict is one decision. Note what is NOT in it: any field carrying output content. A parse that +// succeeds cannot produce text to splice, which is what makes the transport failure mode unreachable +// rather than merely guarded. type Verdict struct { + Label int `json:"i"` Verdict string `json:"verdict"` // keep | drop NeededBy string `json:"needed_by"` // a | b | c | none (see adjudicationContract's CRITERION) Quote string `json:"quote,omitempty"` @@ -63,9 +128,9 @@ type Verdict struct { // 49%/58%. Telling a model its mistakes are cheap makes it careless. So this text states the true // cost and NEVER mentions recoverability — even though, on this path, the drop genuinely is // recoverable through the marker and the stash. That asymmetry is intentional: the operator gets the -// safety net, the model is not told about it. -const adjudicationContract = `You are shown ONE tool output from an agent's transcript. Decide whether the agent -still needs it. +// safety net, the model is not told about it. Every softening of this text measured WORSE. +const adjudicationContract = `You are shown several tool outputs from one agent's transcript. Decide, for EACH +output, whether the agent still needs it. CRITERION. An output is SPENT only if it is needed for NONE of the following: (a) the step the agent is on right now; @@ -80,11 +145,11 @@ NOT notice the gap and does not ask for the content back. It answers from worse the task wrong. There is no safety net you should count on. A wrong removal is a silent, permanent loss of task quality; a wrong retention costs only tokens. -"KEEP EVERYTHING" IS A VALID AND OFTEN CORRECT ANSWER. You are judging one output in isolation, so -you cannot tell whether something else would have been the better thing to remove. If this one looks -load-bearing, keep it. Do not reach for a removal because you were asked a question. +JUDGE THEM AGAINST EACH OTHER. You are given several outputs precisely so you can compare. Rank them: +the ones whose information has clearly been consumed and superseded are the candidates. If they all +look load-bearing, keep them all -- "keep everything" is a valid and often correct answer. -ANSWER THE CRITERION FIRST, THEN DECIDE: +FOR EACH OUTPUT, ANSWER THE CRITERION FIRST, THEN DECIDE: "needed_by" -- which of (a)/(b)/(c) still needs this output, or "none" if it is spent. "quote" -- when needed_by is a/b/c, the transcript text that creates that obligation, copied VERBATIM. Leave empty only when needed_by is "none". @@ -93,21 +158,13 @@ ANSWER THE CRITERION FIRST, THEN DECIDE: A verdict of "drop" REQUIRES needed_by "none": if any obligation still needs the output, the verdict must be keep. -Reply with ONLY a JSON object, no prose: -{"needed_by": "a|b|c|none", "quote": "", "verdict": "keep|drop"}` +Reply with ONLY a JSON array, one object per output, no prose: +[{"i":