From 6d33dc128f351dd203d5edafb59a2ab69e6ecea9 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 12 Sep 2026 12:33:49 -0400 Subject: [PATCH 01/16] checkpoint: the race's tightening never stands the first mark below half the price MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A raced both-yes pulled the first [taking stock] down to the very boundary the verdict landed on, and a measured planning turn was told at round three over one issue and two listings — noise the model waved past, which is what the round-twenty note then inherited. The first rung now never stands below checkpointFirstRungFloor, derived as checkpointPrice / checkpointRatio: triage may make the first look sooner, it may not make it empty. An unraced turn's ladder is byte-identical. Co-Authored-By: Claude Opus 4.8 --- .../unreleased/1004-first-rung-floor.md | 21 ++++ internal/session/checkpoint.go | 32 +++++- internal/session/checkpoint_test.go | 100 +++++++++++++++++- 3 files changed, 149 insertions(+), 4 deletions(-) create mode 100644 docs/changes/unreleased/1004-first-rung-floor.md diff --git a/docs/changes/unreleased/1004-first-rung-floor.md b/docs/changes/unreleased/1004-first-rung-floor.md new file mode 100644 index 0000000000..f03d48b748 --- /dev/null +++ b/docs/changes/unreleased/1004-first-rung-floor.md @@ -0,0 +1,21 @@ +--- +kind: fixed +title: The race can no longer pull the first taking-stock mark below half the handoff price +pr: 1004 +surface: [chat, engine] +invalidates: + - "A raced both-yes from the pre-turn route judge pulled the checkpoint's first mark down to the very step boundary the verdict landed on (`checkpointMeter.tighten` set `firstAt = rounds + 1`). Measured on a real planning turn (ded9ace743f58581, 2026-09-12): the verdict landed two rounds in, the first `[taking stock]` fired at round three over one issue and two listings — 43 KB of mostly listing noise — the model waved the note past, and the round-twenty note inherited that worthlessness. `firstAt` is now clamped at `checkpointFirstRungFloor`, derived as `checkpointPrice / checkpointRatio` (round 5 today): early enough to catch a runaway, late enough that the note's rounds, files and bytes are real. An unraced turn's ladder is byte-identical — rungs at 10, 20, 40." +--- + +The tightening itself is untouched in purpose: it is triage, the power the race +kept when the benchmark took conversion away from it — look sooner, never +oftener. Nothing in the measurement that introduced it (the mark moving to the +verdict's own boundary) spoke to how early a note is worth sending, so the floor +follows the only evidence there is, the failure above: half the price, derived +from the two constants it is a compromise between rather than written as a +number of its own. + +The manual's screen page needed no change: it describes the ordinary ladder +("after ten finished rounds, and again after twenty"), which is exactly what an +unraced turn still climbs, and it never documented the race's exception that +this bounds. diff --git a/internal/session/checkpoint.go b/internal/session/checkpoint.go index d2411b51b9..4a9de198f3 100644 --- a/internal/session/checkpoint.go +++ b/internal/session/checkpoint.go @@ -223,6 +223,30 @@ const ( // where the telling stops and the ceiling begins. checkpointNotes = checkpointMarks - 1 + // checkpointFirstRungFloor is the lowest round the race's tightening may + // stand the FIRST mark on ([checkpointMeter.tighten]). + // + // The tightening exists for TRIAGE and not for haste: a raced both-yes has + // noticed early that this message reads like work, so the first LOOK moves + // sooner — and the reading that decides anything is still the sidecar's, + // taken over the work itself. What the race may never do is stand the note + // where the note has nothing to say. Measured on a real planning turn + // (2026-09-12): the verdict landed two rounds in, the first [taking stock] + // fired at round three over one issue and two listings — 43 KB of mostly + // listing noise — and the model waved the note past, which is what taught + // it the round-twenty note could be waved past too. A telling its reader + // has learned to ignore is not a cheap telling; it is the mechanism's + // credibility spent at a rung that held no facts worth weighing. + // + // SO THE FIRST RUNG NEVER STANDS BELOW HALF THE HANDOFF PRICE. The figure + // is the price divided by the ratio rather than a number of its own, so it + // cannot drift from the two constants it is a compromise between: early + // enough that a genuinely runaway turn is still met well ahead of the full + // price, late enough that the note's own facts line — the rounds, the + // files, the bytes (inherit.go's [turnFacts]) — is about real work and not + // about a turn that has barely started. + checkpointFirstRungFloor = checkpointPrice / checkpointRatio + // THE HANDOFF BRIEF IS SENT WITH NO CEILING, like everything else on this // road. It used to carry 2000 — more room than the shaper's, because this // document is written by a model with a turn's worth of findings in front of @@ -1078,7 +1102,13 @@ func (m *checkpointMeter) tighten(verdict routeVerdict) { // The boundary this is called at is the boundary [checkpointMeter.round] is // about to count, so this fires the first mark HERE rather than one round // later: the two are read in that order at one step boundary (loop.go). - m.firstAt = m.rounds + 1 + // + // BUT NEVER BELOW THE FLOOR. A verdict that lands before the turn has run + // up facts worth weighing does not drag the mark down to where it landed; + // the mark waits at [checkpointFirstRungFloor], which says in the ladder's + // own unit why a note with nothing to weigh is worse than a note not yet + // sent. + m.firstAt = max(m.rounds+1, checkpointFirstRungFloor) } // checkpointWatchTools are the belt's two windows onto work this conversation diff --git a/internal/session/checkpoint_test.go b/internal/session/checkpoint_test.go index da2dbe3e04..344e8086a7 100644 --- a/internal/session/checkpoint_test.go +++ b/internal/session/checkpoint_test.go @@ -93,8 +93,11 @@ func TestTheMeterFiresOnlyAtTheGeometricMarks(t *testing.T) { // leans on. func TestATightenedMeterMovesOnlyTheFirstMark(t *testing.T) { meter := &checkpointMeter{} - // Two ordinary rounds, then the race's verdict lands at the third boundary. - for round := 1; round <= 2; round++ { + // THE VERDICT LANDS PAST THE FLOOR here, at a boundary where pulling the + // first rung down still means firing it at the very next step. A verdict + // that lands EARLIER meets the floor instead, which is the next test's + // business; this one pins the rungs the race is allowed to move at all. + for round := 1; round <= checkpointFirstRungFloor+1; round++ { if mark := meter.round(true); mark != 0 { t.Fatalf("round %d fired mark %d before the price", round, mark) } @@ -110,7 +113,7 @@ func TestATightenedMeterMovesOnlyTheFirstMark(t *testing.T) { t.Errorf("the race's own reading was dropped: %+v", meter.raced) } // THE LATER RUNGS ARE THE ORDINARY ONES. - for round := 4; round < checkpointMarkAt(2); round++ { + for round := meter.rounds + 1; round < checkpointMarkAt(2); round++ { if mark := meter.round(true); mark != 0 { t.Fatalf("round %d fired mark %d; the second rung stands at %d", round, mark, checkpointMarkAt(2)) @@ -150,6 +153,97 @@ func TestALateTightenKeepsTheVerdictAndMovesNothing(t *testing.T) { } } +// AND A VERDICT THAT LANDS BEFORE THE FLOOR CANNOT PULL THE FIRST RUNG BELOW IT. +// +// This is the measured failure the floor exists for: a planning turn whose race +// said yes two rounds in was told [taking stock] at round three — over one issue +// and two listings — and the model waved the note past, which is what made the +// round-twenty note just as easy to wave past. The race is TRIAGE: it may make +// the first look sooner, it may not make it empty ([checkpointFirstRungFloor]). +func TestARacedYesCannotPullTheFirstRungBelowTheFloor(t *testing.T) { + meter := &checkpointMeter{} + // The verdict lands two rounds in, exactly as the measured one did. + for round := 1; round <= 2; round++ { + if mark := meter.round(true); mark != 0 { + t.Fatalf("round %d fired mark %d before the price", round, mark) + } + } + meter.tighten(routeVerdict{Work: true, Wide: true}) + if meter.firstAt != checkpointFirstRungFloor { + t.Fatalf("the first rung stands at %d, want the floor at %d", meter.firstAt, checkpointFirstRungFloor) + } + // NOTHING FIRES BETWEEN THE VERDICT AND THE FLOOR — round three is where the + // measured turn was told, and told noise. + for round := 3; round < checkpointFirstRungFloor; round++ { + if mark := meter.round(true); mark != 0 { + t.Fatalf("round %d fired mark %d below the floor at %d", round, mark, checkpointFirstRungFloor) + } + } + if mark := meter.round(true); mark != 1 { + t.Fatalf("the floor round gave mark %d, want the first mark", mark) + } + // AND THE SECOND RUNG IS STILL THE ORDINARY ONE: sooner, never oftener. + if got := meter.markAt(2); got != checkpointMarkAt(2) { + t.Errorf("the second rung moved to %d; only the first is the race's to move", got) + } +} + +// AND A FIRST MARK FIRED AT THE FLOOR CARRIES REAL FACTS. +// +// The floor's whole reason for standing where it stands is that the note below +// it was noise, so this pins the other half of the bargain: a note sent AT the +// floor is a full one. The transcript is the shape of a turn the race was right +// about — one file opened per round, each with a real result held — and the +// note the model is handed must say all three of the figures it prices the +// roads with (inherit.go's [turnFacts.line]): the rounds, the files and the +// bytes. +func TestAFirstMarkAtTheFloorCarriesRealFacts(t *testing.T) { + transcript := []ai.Message{ + textMessage("system", "SYSTEM"), + textMessage("user", "plan the migration"), + } + for round := 1; round <= checkpointFirstRungFloor; round++ { + transcript = append(transcript, + ai.Message{Role: "assistant", ToolCalls: []ai.ToolCall{ + {Function: ai.ToolCallFunction{Name: "read", + Arguments: fmt.Sprintf(`{"path":"pkg/file%d.go"}`, round)}}}}, + textMessage("tool", strings.Repeat("x", 2_048)), + ) + } + + note := checkpointChoiceNote(readTurnFacts(transcript, turnOpenedAt(transcript), checkpointFirstRungFloor)) + if !strings.HasPrefix(note, checkpointChoiceLead+" ") { + t.Fatalf("the note does not lead with the harness's own tag: %q", note) + } + for _, want := range []string{ + fmt.Sprintf("%d rounds so far", checkpointFirstRungFloor), + fmt.Sprintf("%d files opened", checkpointFirstRungFloor), + "of results in front of you", + } { + if !strings.Contains(note, want) { + t.Errorf("the floored first mark's note is missing %q:\n%s", want, note) + } + } +} + +// AND A METER NO RACE EVER TOUCHED CLIMBS THE LADDER THE POLICY WROTE. +// +// The floor bounds the race's exception; it must not leak into the rule. A turn +// nobody said anything about stands every rung exactly where the two constants +// put it — the pin that a clamp written for the raced road did not quietly +// become a second ladder. +func TestAnUnracedMeterClimbsTheOrdinaryLadder(t *testing.T) { + meter := &checkpointMeter{} + if meter.firstAt != 0 { + t.Fatalf("an untouched meter has a first rung at %d; zero is the ordinary turn", meter.firstAt) + } + for n := 1; n <= checkpointMarks; n++ { + if got, want := meter.markAt(n), checkpointMarkAt(n); got != want { + t.Errorf("rung %d stands at %d, want the ordinary %d", n, got, want) + } + } +} + // THE PRICE IS A FLOOR AND THE LADDER IS SHORT. Both are policy rather than // arithmetic, and both are the kind of number somebody tunes without reading the // argument for it, so they are pinned to the band the design was chosen in. From b8bbdcfb2c6e9404d401ff2b74f48cbbab10bd30 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 12 Sep 2026 12:41:36 -0400 Subject: [PATCH 02/16] journal: billed arms leave their own call lines, and a turn that hopped seals one usage line per model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The measured planning chat's journal summed to $0.789 of a $1.412 bill: hedge arms and cut retries were banked to the meter through reconciled() but wrote no call line, so exactly the expensive requests left no evidence. A billed receipt now writes one, marked arm: hedge or arm: reconciled — evidence like every call line, moving no total. And the turn's usage seal named the model latched at the seal, so the mid-session hop attributed all 31 requests (2.4M input) to the second model though the first served thirteen. addUsage now keeps the turn's share per answering model — the same name the bank row carries — and appendUsage seals one line per model when there were several, the turn's one wall time riding the first by name so a replay never reads it twice. Co-Authored-By: Kimi K3 --- .../unreleased/1007-journal-billed-arms.md | 16 +++ internal/manual/chat/sessions-and-rewind.md | 12 +++ internal/session/loop.go | 13 +++ internal/session/session.go | 35 +++++++ internal/session/sessionfile.go | 97 ++++++++++++++++++- internal/session/usage_percall_test.go | 89 +++++++++++++++++ internal/session/usagejournal_test.go | 68 +++++++++++++ 7 files changed, 329 insertions(+), 1 deletion(-) create mode 100644 docs/changes/unreleased/1007-journal-billed-arms.md diff --git a/docs/changes/unreleased/1007-journal-billed-arms.md b/docs/changes/unreleased/1007-journal-billed-arms.md new file mode 100644 index 0000000000..dfe4e024f2 --- /dev/null +++ b/docs/changes/unreleased/1007-journal-billed-arms.md @@ -0,0 +1,16 @@ +--- +kind: fixed +title: billed rescue arms and cut retries journal their own call lines, and a turn that hopped models seals one usage line per model +pr: 1007 +surface: [chat, engine] +invalidates: + - "A transcript's `call` lines were written only for requests whose usage block arrived on the stream. A billed request reconciled from a late receipt — a hedge arm or a cut retry — now writes one too, marked `arm: hedge` or `arm: reconciled`; it is evidence like every call line and moves no total." + - "A turn's `usage` seal was one line naming the model latched at the seal, so a mid-turn model hop attributed the whole turn to the last model. The seal now writes one line per model that answered, each with that model's own tokens, calls and cost; the turn's wall time rides the alphabetically first line only." + - "`session.Usage` was a plain comparable struct. It now carries a `*modelShares` breakdown field — still comparable and still nil on every value but a live turn's — so `usage == (Usage{})` keeps compiling." +--- + +The measured planning session's journal summed to $0.789 of a $1.412 bill, and +its one turn seal named deepseek for 31 requests that z-ai/glm-5.3 had served +thirteen of. Both gaps were the same shape: the expensive requests left no +evidence. The meter was always right; only the journal was silent, which is why +neither fix touches a total. diff --git a/internal/manual/chat/sessions-and-rewind.md b/internal/manual/chat/sessions-and-rewind.md index 907ab9fdae..c91dac1a9b 100644 --- a/internal/manual/chat/sessions-and-rewind.md +++ b/internal/manual/chat/sessions-and-rewind.md @@ -319,6 +319,11 @@ planner, a harness run. Those are marked `aux` so a reader can tell the turn its the work done around it. A turn that spent nothing writes no line at all, because a zero is absence, not a fact worth a line. +One exception to "one line per turn": a turn that was answered by **more than one model** +— the model hopped mid-turn — writes **one line per model that answered**, each with that +model's own tokens and money. A single sum could only name one of them, and naming the one +standing at the end would hand the whole turn's bill to the wrong model. + **On a resume these lines are added up, and that sum is the session's totals.** So `/cost`, `/status` and the status line show what the whole conversation has spent across every restart, not only what has happened since you reopened it. The spend ceiling reads the same @@ -340,6 +345,13 @@ answered, the endpoint that served it when the provider names one, tokens in, ca cache write, tokens out, and the provider's own figure for the money. A request the provider reported no usage for writes no line — a row of zeros would read as a fact. +The one exception is a request whose usage block **never arrived** — a stream cut short, +or a rescue arm that ran beside a slow answer. Those are exactly the expensive requests, +and a record of only the cheap ones reads as a smaller bill than you paid. So when the +provider's own **receipt** is fetched afterwards, the request writes its line then, marked +with `arm`: `hedge` for a rescue arm, `reconciled` for any other late receipt. A receipt +names no endpoint, so the line leaves that field out rather than guess. + These lines are **evidence, never spending**. Nothing adds them up, `/cost` does not read them, and a resume ignores them entirely, because every dollar on them is already in the `usage` line that sealed the turn. They are there so a question about *shape* — what does a diff --git a/internal/session/loop.go b/internal/session/loop.go index e19dead879..2878eee4de 100644 --- a/internal/session/loop.go +++ b/internal/session/loop.go @@ -3853,6 +3853,13 @@ func (a *Agent) reconciled(receipt provider.Reconciled) { used: used, model: receipt.Model, lane: lane, ledger: true, late: true, reconciled: true, }) + // AND THE REQUEST LEAVES ITS OWN LINE, for [journalCall]'s law said another + // way: EVERY request this session makes writes one, and a request whose + // usage block never arrived wrote none — so the journal's call lines summed + // to barely half the measured session's bill, and exactly the expensive + // half was missing. The row is evidence and never spend; the money moved + // through the door above and nowhere else. + a.file.appendCall(armCall(receipt)) } // addUsage folds one response's accounting into the turn and the session, and @@ -3924,6 +3931,12 @@ func (a *Agent) addUsage(turn *Usage, response *ai.Response, model, served strin } a.bank(bankedCall{used: call, model: answered, lane: lane, ledger: true, turn: true, context: context}) + // AND THE TURN KEEPS THE SAME SHARE PER ANSWERING MODEL, under the very + // name the row above banks, so a turn that hopped seals one usage line per + // model instead of one sum under the last name standing + // ([sessionFile.appendUsage] is the reader, and the only one). + turn.addShare(answered, call) + a.file.appendCall(journalCall{ Model: strings.TrimSpace(response.Model), Endpoint: strings.TrimSpace(served), diff --git a/internal/session/session.go b/internal/session/session.go index b456b9a6ab..921b90db86 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -988,6 +988,41 @@ type Usage struct { // and OpenAI-style prompt_tokens_details.cached_tokens. CacheRead int CacheWrite int + + // byModel is a TURN's own figures kept once per model that answered, keyed + // by the response's own name with the turn's latch standing in when the + // response names nothing — exactly [Agent.addUsage]'s resolution, because + // this is accumulated beside it. A turn that hopped models mid-way seals + // one usage line per model rather than one sum attributed to whichever + // name was standing last (see [sessionFile.appendUsage]); every other Usage + // — the session total, an auxiliary call's — leaves this nil, and nil is + // what keeps their lines exactly as they were. + byModel *modelShares +} + +// modelShares is the breakdown itself, behind a pointer for one reason: Usage +// is passed by value and compared against its zero value, and a bare map field +// would make every one of those comparisons illegal. The pointer keeps Usage +// the plain value it always was, and nil says "nobody kept a breakdown" — +// which is every Usage but a turn's. +type modelShares map[string]Usage + +// addShare folds one call's figures into the per-model breakdown. It is a +// method and not arithmetic at the call site for the reason the field exists +// at all: the share has to be the SAME six figures the call banked, and a sum +// spelled twice is a sum that drifts. +func (u *Usage) addShare(model string, call Usage) { + if u.byModel == nil { + u.byModel = &modelShares{} + } + share := (*u.byModel)[model] + share.Input += call.Input + share.Output += call.Output + share.CacheRead += call.CacheRead + share.CacheWrite += call.CacheWrite + share.CostUSD += call.CostUSD + share.Calls += call.Calls + (*u.byModel)[model] = share } // CachedShare is the fraction of this session's INPUT that came off a warm diff --git a/internal/session/sessionfile.go b/internal/session/sessionfile.go index 0a22878dfc..6c429609d2 100644 --- a/internal/session/sessionfile.go +++ b/internal/session/sessionfile.go @@ -13,6 +13,7 @@ import ( "io" "os" "path/filepath" + "sort" "strconv" "strings" "sync" @@ -487,10 +488,23 @@ type journalCreated struct { // session itself" — [journalUsage] already writes its own Role the same way — // and a name invented for the default case is a name that has to be kept in step // with a registry it is not in. +// +// Arm marks the one kind of row that did not come off a live stream: a request +// whose usage block never arrived, written from the provider's own RECEIPT +// after the fact (loop.go's [Agent.reconciled]). It is "hedge" for a rescue arm +// and "reconciled" for any other late receipt, and absent on every ordinary +// row. It exists because these are exactly the expensive requests — a hedge +// fires when the first answer is slow, a receipt is fetched when a stream was +// cut — and a journal that records every cheap call and none of the dear ones +// answers a cost autopsy wrongly: the measured session's call lines summed to +// barely half its bill until the arms wrote theirs. The money is already banked +// by the reconciliation itself, so the row is evidence here and never spend, +// the same as every other line of this shape. type journalCall struct { Model string `json:"model,omitempty"` Endpoint string `json:"endpoint,omitempty"` Role string `json:"role,omitempty"` + Arm string `json:"arm,omitempty"` Input int `json:"input,omitempty"` CacheRead int `json:"cacheRead,omitempty"` CacheWrite int `json:"cacheWrite,omitempty"` @@ -498,6 +512,40 @@ type journalCall struct { CostUSD float64 `json:"costUsd,omitempty"` } +// The two words [journalCall.Arm] is ever spelled in. They are constants rather +// than literals at the call site for the reason every vocabulary in this file +// is: the row is a record somebody else's autopsy reads back, and a word +// spelled in two places is a word that drifts. +const ( + // journalArmHedge is a rescue arm's receipt: the request ran beside the + // stream it was rescuing, and its cost is waste the session chose to buy + // speed with. + journalArmHedge = "hedge" + // journalArmReconciled is any other late receipt: a stream that ended + // before its usage block — a cut, a torn ending — priced after the fact. + journalArmReconciled = "reconciled" +) + +// armCall is the line one reconciled request leaves: what the receipt carries +// and nothing more. A receipt names no endpoint — it is the provider's account +// of the call, not the router's — so the field stays empty rather than +// repeating who the stream THOUGHT was serving (the emptiness law: absent is +// "nobody said", and a guessed endpoint would read as a measured one). +func armCall(receipt provider.Reconciled) journalCall { + arm := journalArmReconciled + if receipt.Hedged { + arm = journalArmHedge + } + return journalCall{ + Model: strings.TrimSpace(receipt.Model), + Arm: arm, + Input: receipt.PromptTokens, + CacheRead: receipt.CachedTokens, + Output: receipt.CompletionTokens, + CostUSD: receipt.Cost, + } +} + // journalError is ONE CALL THAT FAILED, written down where the calls that // succeeded already are. // @@ -792,7 +840,11 @@ type journalDivision struct { Checkpoint string `json:"checkpoint,omitempty"` } -// journalUsage is one turn's accounting as the journal holds it. +// journalUsage is one turn's accounting as the journal holds it — or, when a +// turn was answered by more than one model, ONE MODEL'S SHARE of it: the seal +// then writes one line per model that answered ([sessionFile.appendUsage]), +// because a sum spelled under a single name attributes the whole turn to +// whichever model answered last. // // Duration is milliseconds and not a time.Duration because a time.Duration // marshals as bare nanoseconds, and this is a file a person reads. @@ -2834,6 +2886,49 @@ func (s *sessionFile) appendUsage(used Usage, model string, aux bool, role strin if used.Input == 0 && used.Output == 0 && used.CostUSD == 0 { return } + // A TURN MORE THAN ONE MODEL ANSWERED SEALS ONE LINE PER MODEL. The sum a + // single line carries can only name one of them, and it used to name the + // last: a turn that hopped mid-way had every request it made — on both + // sides of the hop — attributed to whichever model was standing at the + // seal, so the journal's own rollup said one model had spent money another + // had (the measured session: all 31 requests under the second model's + // name, thirteen of them served by the first). The shares were kept beside + // the sum all along (Usage.byModel, accumulated in [Agent.addUsage] from + // the response's own name); here they become the lines. + // + // THE DURATION LANDS ON ONE LINE, the first by name, because it is the + // TURN's wall time and not a figure any model earned a share of — and the + // replay adds these lines back up, so a copy on each would read the turn + // as twice as long as it ran. Money, tokens and calls divide; time does + // not, and the order is sorted so the line that carries it is a fact of + // the turn rather than of a map's mood. + if used.byModel != nil && len(*used.byModel) > 1 { + names := make([]string, 0, len(*used.byModel)) + for name := range *used.byModel { + names = append(names, name) + } + sort.Strings(names) + for i, name := range names { + share := (*used.byModel)[name] + line := journalUsage{ + Model: name, + Input: share.Input, + Output: share.Output, + CacheRead: share.CacheRead, + CacheWrite: share.CacheWrite, + CostUSD: share.CostUSD, + Calls: share.Calls, + Aux: aux, + Empty: used.EmptyReflex > 0, + Role: strings.TrimSpace(role), + } + if i == 0 { + line.DurationMS = used.Duration.Milliseconds() + } + s.writeLine(sessionEntry{Type: "usage", Usage: &line, Timestamp: stamp()}) + } + return + } s.writeLine(sessionEntry{ Type: "usage", Usage: &journalUsage{ diff --git a/internal/session/usage_percall_test.go b/internal/session/usage_percall_test.go index b3811e0bfc..9be4566a29 100644 --- a/internal/session/usage_percall_test.go +++ b/internal/session/usage_percall_test.go @@ -16,6 +16,7 @@ package session import ( "context" + "encoding/json" "go/ast" "go/parser" "go/token" @@ -161,6 +162,94 @@ func TestAHedgeLoserWritesItsReceiptAsWaste(t *testing.T) { } } +// TestAReconciledCallLeavesItsOwnLineInTheJournal is the journal half of the +// measured defect: the session's call lines summed to barely half its bill +// because every request whose usage block never arrived — the hedge arms and +// the cut retries, which is to say the expensive ones — wrote no line at all. +// The receipt now leaves one, marked with what it was and carrying what the +// receipt carried; the money moves through [Agent.bank] alone, so the meter +// still counts each receipt exactly once and the ledger grows no second row. +func TestAReconciledCallLeavesItsOwnLineInTheJournal(t *testing.T) { + ledger := filepath.Join(t.TempDir(), UsageLedgerName) + journal := filepath.Join(t.TempDir(), "session.jsonl") + agent, _ := newTestAgent(t, &scriptedCompleter{}, func(config *Config) { + config.usageLedger = ledger + config.SessionFile = journal + }) + agent.reconciled(provider.Reconciled{ + Billed: provider.Billed{Model: "rescue/model", PromptTokens: 500, CompletionTokens: 40, CachedTokens: 300, Cost: 0.42}, + Ref: "hedge-arm", Reason: "torn", Hedged: true, Found: true, + }) + agent.reconciled(provider.Reconciled{ + Billed: provider.Billed{Model: "first/model", PromptTokens: 700, CompletionTokens: 90, Cost: 0.11}, + Ref: "cut-retry", Reason: "stalled", Found: true, + }) + + calls := journalCallLines(t, journal) + if len(calls) != 2 { + t.Fatalf("the journal holds %d call lines, want one per reconciled request: %+v", len(calls), calls) + } + hedge, cut := calls[0], calls[1] + if hedge.Arm != journalArmHedge || hedge.Model != "rescue/model" || + hedge.Input != 500 || hedge.CacheRead != 300 || hedge.Output != 40 || hedge.CostUSD != 0.42 { + t.Fatalf("the rescue arm's line = %+v, want the receipt's own figures marked %q", hedge, journalArmHedge) + } + if cut.Arm != journalArmReconciled || cut.Model != "first/model" || + cut.Input != 700 || cut.Output != 90 || cut.CostUSD != 0.11 { + t.Fatalf("the cut retry's line = %+v, want the receipt's own figures marked %q", cut, journalArmReconciled) + } + // A receipt knows no endpoint — it is the provider's account of the call, + // not the router's — so the field is absent rather than guessed. + if hedge.Endpoint != "" || cut.Endpoint != "" { + t.Fatalf("a receipt invented an endpoint: %+v / %+v", hedge, cut) + } + // AND THE MONEY MOVED ONCE. The lines above are evidence: the meter holds + // the two receipts' sum and the ledger holds two rows, not four. + if got := agent.Usage().CostUSD; got != 0.53 { + t.Fatalf("the meter holds %v, want the two receipts counted once (0.53)", got) + } + if _, rows := ledgerUSD(t, ledger); rows != 2 { + t.Fatalf("the ledger holds %d rows, want the two receipts and nothing doubled", rows) + } +} + +// A receipt that could not be had writes the unbilled marker and NO call line: +// the emptiness law, because a receipt with no figures is a row of zeroes that +// would read as a fact. +func TestAMissingReceiptLeavesNoCallLine(t *testing.T) { + journal := filepath.Join(t.TempDir(), "session.jsonl") + agent, _ := newTestAgent(t, &scriptedCompleter{}, func(config *Config) { + config.SessionFile = journal + }) + agent.reconciled(provider.Reconciled{Ref: "gone", Reason: "torn"}) + if calls := journalCallLines(t, journal); len(calls) != 0 { + t.Fatalf("a missing receipt left %d call lines, want none: %+v", len(calls), calls) + } + if got := agent.Usage(); got.Calls != 0 || got.CostUSD != 0 { + t.Fatalf("a missing receipt moved the meter: %+v", got) + } +} + +// journalCallLines is every call line in one journal, in file order — the +// reader half of [sessionFile.appendCall], for tests about what was WRITTEN +// rather than what a replay chose to do with it (the replay drops these lines +// on purpose, so only the file itself can answer). +func journalCallLines(t *testing.T, path string) []journalCall { + t.Helper() + var calls []journalCall + for _, line := range readLines(t, path) { + var entry sessionEntry + if err := json.Unmarshal([]byte(line), &entry); err != nil { + continue + } + if entry.Type != "call" || entry.Call == nil { + continue + } + calls = append(calls, *entry.Call) + } + return calls +} + // TestATurnArmsCutCallReconciliation pins the session wiring rather than a // hand-built context: the provider call a real turn makes can read its sink. func TestATurnArmsCutCallReconciliation(t *testing.T) { diff --git a/internal/session/usagejournal_test.go b/internal/session/usagejournal_test.go index f05b575d29..c31a1b236c 100644 --- a/internal/session/usagejournal_test.go +++ b/internal/session/usagejournal_test.go @@ -285,3 +285,71 @@ func TestASessionsCallCountHoldsTheAuxiliaryCallsItsTurnCountDoesNot(t *testing. t.Fatalf("the auxiliary call's 0.02 is missing from the session's %.4f", used.CostUSD) } } + +// A TURN TWO MODELS ANSWERED SEALS ONE USAGE LINE PER MODEL — the rollup half +// of the measured defect. The turn below hopped mid-way, and the seal used to +// write ONE line carrying the whole turn's sum under the name standing last, +// so the journal's own record said the second model had spent money the first +// one had (the measured session: 31 requests, 2.4M input tokens, all under the +// post-hop name). The lines are the per-model shares the turn kept beside the +// sum (Usage.byModel), in name order so the one carrying the turn's wall time +// is a fact and not a coin toss — the replay adds these lines, and the turn +// ran once. +func TestATurnThatHoppedModelsSealsOneUsageLinePerModel(t *testing.T) { + path := filepath.Join(t.TempDir(), "session.jsonl") + ledger := filepath.Join(t.TempDir(), UsageLedgerName) + agent, _ := newTestAgent(t, &scriptedCompleter{}, func(config *Config) { + config.SessionFile = path + config.usageLedger = ledger + }) + + // Two answers from the model the turn started on, then one from the model + // the hop landed on — the latch the seal is handed is the LAST one, which + // is exactly the attribution the split exists to refuse. + var turn Usage + before := pricedResponse("a step", 0.25) + before.Model = "z-ai/glm-5.3" + agent.addUsage(&turn, before, "z-ai/glm-5.3", "", laneFacts{}) + before2 := pricedResponse("another step", 0.5) + before2.Model = "z-ai/glm-5.3" + before2.Usage.PromptTokens = 20 + agent.addUsage(&turn, before2, "z-ai/glm-5.3", "", laneFacts{}) + after := pricedResponse("the hop's step", 0.75) + after.Model = "deepseek/deepseek-v4.1-flash" + after.Usage.PromptTokens = 30 + agent.addUsage(&turn, after, "deepseek/deepseek-v4.1-flash", "", laneFacts{}) + + started := time.Now().Add(-time.Minute) + agent.sealTurn(turn, started, "deepseek/deepseek-v4.1-flash") + + turns := turnUsageLines(t, path) + if len(turns) != 2 { + t.Fatalf("the journal holds %d turn usage lines, want one per answering model: %+v", len(turns), turns) + } + // Name order: deepseek before z-ai. The first line carries the turn's one + // wall time; the second carries none, so a replay never reads the minute + // twice. + first, second := turns[0], turns[1] + if first.Model != "deepseek/deepseek-v4.1-flash" || first.Input != 30 || first.Output != 5 || + first.Calls != 1 || first.CostUSD != 0.75 { + t.Fatalf("the hop's line = %+v, want the last model's own figures only", first) + } + if second.Model != "z-ai/glm-5.3" || second.Input != 30 || second.Output != 10 || + second.Calls != 2 || second.CostUSD != 0.75 { + t.Fatalf("the first model's line = %+v, want its own two calls summed", second) + } + if first.DurationMS < 60000 || second.DurationMS != 0 { + t.Fatalf("the wall time is split %d + %d, want the turn's minute once and never twice", + first.DurationMS, second.DurationMS) + } + + // AND THE REPLAY ADDS THE TWO BACK INTO THE TURN'S OWN TOTALS, because the + // split is a spelling of the same money, not new money. + replayed, err := replaySessionFile(path) + if err != nil { + t.Fatalf("replay: %v", err) + } + if got := replayed.usage; got.CostUSD != 1.5 || got.Input != 60 || got.Output != 15 || got.Calls != 3 { + t.Fatalf("the split lines replay to %+v, want the turn's own sum (1.50, 60 in, 15 out, 3 calls)", got) + } +} From 61a3434d7e2b4a9a454c124d53bf8d41c400cfdd Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 12 Sep 2026 12:41:53 -0400 Subject: [PATCH 03/16] changelog: 1007 title under the one-line bound Co-Authored-By: Kimi K3 --- docs/changes/unreleased/1007-journal-billed-arms.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changes/unreleased/1007-journal-billed-arms.md b/docs/changes/unreleased/1007-journal-billed-arms.md index dfe4e024f2..4afcabce3a 100644 --- a/docs/changes/unreleased/1007-journal-billed-arms.md +++ b/docs/changes/unreleased/1007-journal-billed-arms.md @@ -1,6 +1,6 @@ --- kind: fixed -title: billed rescue arms and cut retries journal their own call lines, and a turn that hopped models seals one usage line per model +title: billed arms journal their call lines, and a hopped turn seals one usage line per model pr: 1007 surface: [chat, engine] invalidates: From f5c87977067d6ec5ca9d1747bd4a4235f615de44 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 12 Sep 2026 12:41:57 -0400 Subject: [PATCH 04/16] turn fold: slices of a repeatedly read file point at the file, and the newest stays whole MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The measured planning chat read checkpoint.go in ten overlapping slices and carried all ten, verbatim, through twenty requests: the fold bounded generic tool-result accumulation but had no notion that slices of one file are re-readable where they came from. When the foldable region now holds several reads of one path, each older slice becomes a pointer in the stub family the prompt already teaches — the read recipe, offset and limit, that brings the slice back — no copy is filed, and the newest slice is kept whole. Selection counts a kept slice as buying nothing, so a pass still runs only when it can buy the whole headroom. The trigger, target, batch-completeness and frozen-horizon laws are untouched; bash and every other tool's results keep the filed pointer they always got. Freshness of a kept slice is the held-read seam's question, not this pass's: a pointer says the range was read and how to read it back, never that the file still says it. Co-Authored-By: Kimi K3 --- docs/changes/unreleased/1005-fold-rereads.md | 15 ++ .../manual/chat/compacting-over-and-over.md | 5 + internal/manual/chat/models-and-cost.md | 6 + internal/session/turnfold.go | 157 +++++++++++- internal/session/turnfold_test.go | 232 ++++++++++++++++++ 5 files changed, 408 insertions(+), 7 deletions(-) create mode 100644 docs/changes/unreleased/1005-fold-rereads.md diff --git a/docs/changes/unreleased/1005-fold-rereads.md b/docs/changes/unreleased/1005-fold-rereads.md new file mode 100644 index 0000000000..1c9da03c7d --- /dev/null +++ b/docs/changes/unreleased/1005-fold-rereads.md @@ -0,0 +1,15 @@ +--- +kind: changed +title: The turn fold keeps the newest slice of a repeatedly read file and points the older ones at it +pr: 1005 +surface: [engine, docs] +invalidates: + - "Every folded tool result used to be replaced by a pointer to a copy filed under the session's stubs folder. When one turn reads the same file several times, the older slices now fold to a pointer naming the file with the offset and limit that read it — no copy is filed — and the newest slice of the file stays whole." +--- + +Measured on a planning conversation that read one file in ten overlapping +slices and carried all ten, verbatim, through twenty requests: the fold bound +generic tool-result accumulation but had no notion that slices of one file are +re-readable where they came from. The trigger, target, batch and horizon laws +of the pass are unchanged; bash and every other tool's results keep their +existing rules. diff --git a/internal/manual/chat/compacting-over-and-over.md b/internal/manual/chat/compacting-over-and-over.md index a4245c6559..468523a549 100644 --- a/internal/manual/chat/compacting-over-and-over.md +++ b/internal/manual/chat/compacting-over-and-over.md @@ -119,6 +119,11 @@ has not produced work stays verbatim, even when keeping it costs more tokens. Th of a result that does become a pointer remain in the session journal and the path named by the pointer. +One refinement for repetition: when the turn has read the **same file several times**, only +the newest slice stays whole. Each older slice becomes a pointer naming the file with the +`offset` and `limit` that read it, and no copy is filed — the file itself is where those +bytes came from, and `read` brings the slice back. + ## A pass that cannot reach its target The fold walks the oldest assistant work first and stops at the target, but it never folds diff --git a/internal/manual/chat/models-and-cost.md b/internal/manual/chat/models-and-cost.md index 04214221f9..1c1b83fff6 100644 --- a/internal/manual/chat/models-and-cost.md +++ b/internal/manual/chat/models-and-cost.md @@ -2219,6 +2219,12 @@ runs, the transcript gets one line such as `[folded 8 results · ~24k tokens]`. result bytes remain in `logs/stubs/`, the model can `read` the path in each stub, and the session journal keeps the original result bytes. +One shape of result needs no copy filed. When the turn has read the **same file several +times**, the older slices point at the file itself — the stub names the path with the +`offset` and `limit` that bring that slice back — and the newest slice is kept whole, +because those bytes were never anywhere else to begin with. The journal keeps them too, +as it keeps everything. + **Rung 0 — what an old result looks like in the request.** Before any of the rungs below fire, the copy of the conversation that goes to the model already carries the tool results of *earlier* turns shortened. Your transcript and the session journal keep every byte; only diff --git a/internal/session/turnfold.go b/internal/session/turnfold.go index d935691eda..2b2d90d403 100644 --- a/internal/session/turnfold.go +++ b/internal/session/turnfold.go @@ -9,6 +9,7 @@ package session // This pass bounds that accumulation without summarizing or deleting anything. import ( + "encoding/json" "fmt" "strings" "time" @@ -69,6 +70,15 @@ type turnFoldReplacement struct { // reaches its headroom target. An observation becomes consumed only after work // made from it lands. Assistant text, calls and their arguments, mutating tool // batches, and every observation not yet acted upon remain verbatim. +// +// One shape of repetition is folded WITHOUT filing a copy. A measured planning +// turn read one file in ten overlapping slices and carried all ten, verbatim, +// through twenty requests — yet a slice of a file is re-readable where it came +// from, which is the one kind of result whose bytes never needed a steward. So +// when the foldable region holds several reads of ONE path, the newest slice +// stays whole and each older slice becomes a pointer naming the file and the +// range it held (turnFoldReadRepeats); every other result keeps the filed +// pointer it always got. func (a *Agent) foldTurnOutputs(seenThrough int, consumedReads map[*ai.ToolCall]bool, hub *eventHub) { line := turnWorkingSet(a.window()) if line <= 0 { @@ -87,7 +97,8 @@ func (a *Agent) foldTurnOutputs(seenThrough int, consumedReads map[*ai.ToolCall] if total > line*bytesPerToken { limit := a.turnFoldLimitLocked(seenThrough) batches := turnFoldBatches(a.messages, a.turnFloor, limit, consumedReads) - selected = turnFoldSelection(a.messages, batches, total, turnWorkingTarget(a.window())*bytesPerToken) + keepNewest, _ := turnFoldReadRepeats(a.messages, batches) + selected = turnFoldSelection(a.messages, batches, keepNewest, total, turnWorkingTarget(a.window())*bytesPerToken) } a.mu.Unlock() if total <= line*bytesPerToken || selected == 0 { @@ -118,11 +129,12 @@ func (a *Agent) foldTurnOutputs(seenThrough int, consumedReads map[*ai.ToolCall] place := a.resultPlaceLocked() target := turnWorkingTarget(a.window()) * bytesPerToken batches := turnFoldBatches(a.messages, a.turnFloor, limit, consumedReads) + keepNewest, superseded := turnFoldReadRepeats(a.messages, batches) // A pass that cannot buy the whole headroom does not run. Every rewrite // invalidates the provider cache from that point onward; repeatedly replacing // one tiny result while protected observations hold the working set above the // line is strictly worse than retaining the original context. - selected = turnFoldSelection(a.messages, batches, total, target) + selected = turnFoldSelection(a.messages, batches, keepNewest, total, target) if selected == 0 { a.mu.Unlock() return @@ -139,11 +151,28 @@ func (a *Agent) foldTurnOutputs(seenThrough int, consumedReads map[*ai.ToolCall] batchSaved := 0 complete := true for _, index := range batch.indices { + // The newest slice of a file the turn read more than once stays + // whole: it is the reading the model is most likely working from, + // and the older slices' pointers already name how to bring any of + // their bytes back. A batch with nothing left to replace is simply + // skipped, the way a batch of already-stubbed results is. + if keepNewest[index] { + continue + } message := a.messages[index] text := messageContentText(message) - // The same pointer the stub pass and the snapshot view give, and for - // the same reason: a store ref is not one ([Agent.fullResultPointer]). - pointer := a.fullResultPointer(message, place) + pointer := "" + if slice, repeated := superseded[index]; repeated { + // An older slice of a repeatedly read file needs no copy filed: + // its bytes were never anywhere but the file itself, so the + // pointer is the read that fetches that slice again — the same + // kind of recipe the journal fallback already spells. + pointer = slice.readRecipe() + } else { + // The same pointer the stub pass and the snapshot view give, and for + // the same reason: a store ref is not one ([Agent.fullResultPointer]). + pointer = a.fullResultPointer(message, place) + } if pointer == "" { complete = false break @@ -221,11 +250,15 @@ func (a *Agent) turnFoldLimitLocked(seenThrough int) int { // reach the target, or zero when all eligible observations together cannot buy // that headroom. It deliberately overestimates savings by the small pointer // bodies; the materialization pass below checks the exact bytes before writing -// anything into the live transcript. -func turnFoldSelection(messages []ai.Message, batches []turnFoldBatch, total, target int) int { +// anything into the live transcript. A slice keepNewest holds whole buys +// nothing, and counting it would promise headroom the pass cannot deliver. +func turnFoldSelection(messages []ai.Message, batches []turnFoldBatch, keepNewest map[int]bool, total, target int) int { reclaimable := 0 for batchIndex, batch := range batches { for _, index := range batch.indices { + if keepNewest[index] { + continue + } reclaimable += messageBytes(messages[index]) } if total-reclaimable <= target { @@ -307,6 +340,116 @@ func turnFoldBatches(messages []ai.Message, start, limit int, consumedReads map[ return batches } +// readSlice is one read call's own recipe: the path it asked for and the slice +// of it, spelled in the read tool's own arguments. It is parsed from the +// call's arguments, never from the result's text — a result carries a +// continuation footer only when it was cut, while the arguments always say +// what was asked. +type readSlice struct { + path string + offset int // 1-indexed, as the read tool takes it + limit int // 0 means the call named no limit +} + +// readRecipe is the pointer an older slice folds into: the read that brings +// its bytes back. The pointer family already carries recipes and not just +// paths — the journal fallback is "grep in " — and the read +// tool's own continuation footers teach the offset spelling, so this is the +// sentence family the prompt already teaches, adapted to name the range. +func (s readSlice) readRecipe() string { + recipe := "read " + s.path + if s.offset > 1 { + recipe += fmt.Sprintf(" offset=%d", s.offset) + } + if s.limit > 0 { + recipe += fmt.Sprintf(" limit=%d", s.limit) + } + return recipe +} + +// turnFoldReadSlice answers which slice of which file the result at index +// holds: its call is found in the assistant message above it, the way +// toolNameFor finds a name, and only a genuine `read` call with a parseable +// path qualifies. Anything else — a grep, a find, an unreadable argument +// string — is not a slice and keeps the ordinary filed pointer. +func turnFoldReadSlice(messages []ai.Message, index int) (readSlice, bool) { + id := messages[index].ToolCallID + if id == "" { + return readSlice{}, false + } + for above := index - 1; above > 0; above-- { + for _, call := range messages[above].ToolCalls { + if call.ID != id { + continue + } + if call.Function.Name != "read" { + return readSlice{}, false + } + var args struct { + Path string `json:"path"` + Offset *int `json:"offset"` + Limit *int `json:"limit"` + } + if err := json.Unmarshal([]byte(call.Function.Arguments), &args); err != nil || strings.TrimSpace(args.Path) == "" { + return readSlice{}, false + } + slice := readSlice{path: args.Path, offset: 1} + if args.Offset != nil && *args.Offset > 1 { + slice.offset = *args.Offset + } + if args.Limit != nil && *args.Limit > 0 { + slice.limit = *args.Limit + } + return slice, true + } + } + return readSlice{}, false +} + +// turnFoldReadRepeats walks the foldable batches and decides, for every file +// read MORE THAN ONCE inside the region, which read is the newest slice — the +// one kept whole — and which are the older slices whose bytes the file itself +// hands back. The batches arrive in transcript order, so the last index of a +// path is its newest. A path is grouped by the exact string the calls named: +// a second spelling of the same file is a different recipe, and canonicalizing +// it is a filesystem question this pass may not ask under the session lock. +// +// FRESHNESS IS NOT THIS PASS'S QUESTION. Whether the newest slice of a file +// still says what it said — a later edit or a watch may have moved the file — +// belongs to the held-read seam at the live edge (heldreads.go), which owns +// the short-circuit of a read the world has outrun. This pass is transcript +// hygiene: the pointer it writes only ever says the range WAS read and how to +// read it back, so a stale file cannot turn it into a coverage claim, and a +// slice kept whole is the observation the model received, nothing more. +func turnFoldReadRepeats(messages []ai.Message, batches []turnFoldBatch) (keepNewest map[int]bool, superseded map[int]readSlice) { + byPath := make(map[string][]int) + slices := make(map[int]readSlice) + for _, batch := range batches { + for _, index := range batch.indices { + slice, ok := turnFoldReadSlice(messages, index) + if !ok { + continue + } + slices[index] = slice + byPath[slice.path] = append(byPath[slice.path], index) + } + } + for _, indices := range byPath { + if len(indices) < 2 { + continue + } + if keepNewest == nil { + keepNewest = make(map[int]bool) + superseded = make(map[int]readSlice) + } + keepNewest[indices[len(indices)-1]] = true + for _, index := range indices[:len(indices)-1] { + superseded[index] = slices[index] + } + } + return keepNewest, superseded +} + func turnFoldMarker(results, tokens int) string { return fmt.Sprintf("%s%d result%s · %s tokens]", foldMarkerPrefix, results, plural(results), approxTokens(tokens)) } diff --git a/internal/session/turnfold_test.go b/internal/session/turnfold_test.go index 33d36a5417..4aa7d87410 100644 --- a/internal/session/turnfold_test.go +++ b/internal/session/turnfold_test.go @@ -329,3 +329,235 @@ func TestTurnFoldKeepsUnactedResearchVerbatim(t *testing.T) { t.Fatal("research that had not produced work was folded") } } + +// appendToolRound writes one call-and-result pair into the agent's live +// transcript, the way the fixtures above spell their rounds out one at a time. +func appendToolRound(agent *Agent, id, name, args, output string) { + agent.messages = append(agent.messages, + ai.Message{Role: "assistant", ToolCalls: []ai.ToolCall{{ + ID: id, Function: ai.ToolCallFunction{Name: name, Arguments: args}, + }}}, + ai.Message{Role: "tool", ToolCallID: id, Content: []ai.ContentPart{{Type: "text", Text: output}}}, + ) +} + +// repeatedReadAgent builds a turn whose transcript holds 24 consumed, seen +// tool rounds — about three times the working-set line — ready for the fold. +func repeatedReadAgent(t *testing.T, rounds func(agent *Agent)) *Agent { + t.Helper() + agent, _ := newTestAgent(t, &scriptedCompleter{}, func(config *Config) { + config.ContextWindow = bigTestWindow + }) + agent.mu.Lock() + agent.running = true + agent.turnFloor = len(agent.messages) + rounds(agent) + agent.mu.Unlock() + return agent +} + +func foldWholeTurn(t *testing.T, agent *Agent) []ai.Message { + t.Helper() + agent.mu.Lock() + consumed := consumedTurnReads(agent.messages, agent.turnFloor, len(agent.messages)) + horizon := len(agent.messages) + agent.mu.Unlock() + agent.foldTurnOutputs(horizon, consumed, nil) + agent.mu.Lock() + messages := append([]ai.Message(nil), agent.messages...) + floor := agent.turnFloor + agent.mu.Unlock() + if len(messages) == floor { + t.Fatal("the fixture built no rounds") + } + return messages +} + +// THREE OVERLAPPING SLICES OF ONE FILE: the oldest two become pointers naming +// the path and the range each held — a recipe in the sentence family the +// prompt already teaches — while the newest slice, the one the model is still +// working from, stays byte-for-byte whole even though its batch was old enough +// to fold. Distinct reads beside them keep the filed pointer they always got. +func TestTurnFoldKeepsTheNewestSliceOfARepeatedlyReadFile(t *testing.T) { + output := turnFoldOutput() + slices := []string{ + `{"path":"checkpoint.go","offset":10,"limit":50}`, + `{"path":"checkpoint.go","offset":40,"limit":50}`, + `{"path":"checkpoint.go","offset":80,"limit":50}`, + } + agent := repeatedReadAgent(t, func(agent *Agent) { + for round, args := range slices { + appendToolRound(agent, fmt.Sprintf("slice-%d", round), "read", args, output) + } + for round := 3; round < 24; round++ { + appendToolRound(agent, fmt.Sprintf("call-%d", round), "read", + fmt.Sprintf(`{"path":"round-%d.txt"}`, round), output) + } + }) + agent.mu.Lock() + floor := agent.turnFloor + agent.mu.Unlock() + + messages := foldWholeTurn(t, agent) + + // The results of the three slices sit one after the other from the floor: + // assistant, result, assistant, result, assistant, result. + for round, want := range []string{"offset=10 limit=50", "offset=40 limit=50"} { + text := messageContentText(messages[floor+1+round*2]) + if !strings.HasPrefix(text, stubMarker) { + t.Fatalf("slice %d was not folded: %.80q", round, text) + } + if !strings.Contains(text, "full: read checkpoint.go "+want) { + t.Fatalf("slice %d pointer = %q, want the read that brings the slice back", round, text) + } + } + if got := messageContentText(messages[floor+5]); got != output { + t.Fatalf("the newest slice of checkpoint.go was rewritten: %.80q", got) + } + // A read of a file named once is none of this rule's business: round 3 was + // old enough to fold and got the ordinary filed pointer, not a recipe. + distinct := messageContentText(messages[floor+7]) + if !strings.HasPrefix(distinct, stubMarker) || strings.Contains(distinct, "full: read round-3.txt") { + t.Fatalf("a singly-read file folded into a recipe: %.80q", distinct) + } + if !holdsPrefix(messages, foldMarkerPrefix) { + t.Fatal("no fold note was written") + } + // AND THE ARITHMETIC IS THE OLD ARITHMETIC: the pass stops at the same + // headroom target with the kept-whole slice counted as buying nothing. + agent.mu.Lock() + after := turnToolBytes(agent.messages, floor) + agent.mu.Unlock() + if target := turnWorkingTarget(agent.window()) * bytesPerToken; after > target { + t.Fatalf("working set after the fold = %d bytes, want at most the %d-byte target", after, target) + } +} + +// TWENTY-FOUR DIFFERENT FILES, READ ONCE EACH: no path repeats, so the +// repeated-read rule fires nowhere and every folded result keeps the filed +// pointer the pass has always written. +func TestTurnFoldFilesReadsOfDistinctFilesAsBefore(t *testing.T) { + output := turnFoldOutput() + agent := repeatedReadAgent(t, func(agent *Agent) { + for round := range 24 { + appendToolRound(agent, fmt.Sprintf("call-%d", round), "read", + fmt.Sprintf(`{"path":"distinct-%d.txt"}`, round), output) + } + }) + + messages := foldWholeTurn(t, agent) + + stubs := 0 + for _, message := range messages { + text := messageContentText(message) + if message.Role != "tool" || !strings.HasPrefix(text, stubMarker) { + continue + } + stubs++ + if strings.Contains(text, "full: read distinct-") { + t.Fatalf("a file read once folded into a re-read recipe: %.80q", text) + } + } + if stubs == 0 { + t.Fatal("a turn over the line folded nothing") + } +} + +// A SINGLE read of a path is not a repeat: it folds under the old rule, with a +// filed copy to point at, and never a recipe naming the file itself. +func TestTurnFoldGivesASingleReadNoRecipe(t *testing.T) { + output := turnFoldOutput() + agent := repeatedReadAgent(t, func(agent *Agent) { + appendToolRound(agent, "solo", "read", `{"path":"solo.txt"}`, output) + for round := 1; round < 24; round++ { + appendToolRound(agent, fmt.Sprintf("call-%d", round), "read", `{"path":"daily.txt"}`, output) + } + }) + agent.mu.Lock() + floor := agent.turnFloor + agent.mu.Unlock() + + messages := foldWholeTurn(t, agent) + + solo := messageContentText(messages[floor+1]) + if !strings.HasPrefix(solo, stubMarker) { + t.Fatalf("the oldest result was not folded: %.80q", solo) + } + if strings.Contains(solo, "full: read solo.txt") { + t.Fatalf("a file read once points at itself: %.80q", solo) + } + if older := messageContentText(messages[floor+3]); !strings.Contains(older, "full: read daily.txt") { + t.Fatalf("an older slice of a repeated read = %.80q, want the recipe", older) + } +} + +// BASH RESULTS ARE THE OLD RULE'S ALONE: they were never eligible batches in +// this pass, and the repeated-read rule changes nothing about that — a fold +// triggered by the reads beside them leaves every one byte-for-byte. +func TestTurnFoldLeavesBashResultsToTheOlderRules(t *testing.T) { + output := turnFoldOutput() + var bashWanted []string + // Thirty rounds, every fourth a small bash result: the reads carry the + // working set over the line, the bash results only watch. + agent := repeatedReadAgent(t, func(agent *Agent) { + for round := range 30 { + call := fmt.Sprintf("call-%d", round) + if round%4 == 3 { + result := fmt.Sprintf("bash said %d", round) + bashWanted = append(bashWanted, result) + appendToolRound(agent, call, "bash", fmt.Sprintf(`{"command":"echo %d"}`, round), result) + continue + } + appendToolRound(agent, call, "read", + fmt.Sprintf(`{"path":"file-%d.txt"}`, round), output) + } + }) + + messages := foldWholeTurn(t, agent) + + var bashGot []string + stubs := 0 + for _, message := range messages { + text := messageContentText(message) + if message.Role != "tool" { + continue + } + if strings.HasPrefix(text, "bash said ") { + bashGot = append(bashGot, text) + } + if strings.HasPrefix(text, stubMarker) { + stubs++ + } + } + if !reflect.DeepEqual(bashGot, bashWanted) { + t.Fatalf("bash results were touched\n got: %v\nwant: %v", bashGot, bashWanted) + } + if stubs == 0 { + t.Fatal("a turn over the line folded nothing") + } +} + +// BELOW THE LINE, REPEATS CHANGE NOTHING. Three slices of one file are a +// repeat the new rule can see, but the working-set trigger is the only trigger +// this pass has: under it the transcript is byte-stable, fold note and all. +func TestTurnFoldLeavesRepeatedReadsBelowTheLine(t *testing.T) { + output := turnFoldOutput() + agent := repeatedReadAgent(t, func(agent *Agent) { + for round, offset := range []int{10, 40, 80} { + appendToolRound(agent, fmt.Sprintf("slice-%d", round), "read", + fmt.Sprintf(`{"path":"checkpoint.go","offset":%d,"limit":50}`, offset), output) + } + }) + agent.mu.Lock() + before := append([]ai.Message(nil), agent.messages...) + agent.mu.Unlock() + + _ = foldWholeTurn(t, agent) + + agent.mu.Lock() + after := append([]ai.Message(nil), agent.messages...) + agent.mu.Unlock() + if !reflect.DeepEqual(after, before) { + t.Fatal("repeated reads below the working-set line were folded") + } +} From 90accd477479ca2994689123f3a9ab127c9c55b6 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 12 Sep 2026 13:01:30 -0400 Subject: [PATCH 05/16] session: a read the conversation already holds answers with a pointer Held-range ledger (heldreads.go): a `read` fully covered by bytes the live transcript still carries, against an unchanged file stamp, is answered with a one-line pointer instead of a fresh fetch; a changed stamp retires the whole entry, a partial overlap fetches whole, and a stubbed or folded result drops out of coverage by itself. The read tool's description prices the round trip it costs, the system prompt teaches the pointer sentence beside the stub pointer, and the chat manual quotes the behavior. Co-Authored-By: Claude --- .../unreleased/1006-read-held-ranges.md | 15 + internal/exec/bare/tools.go | 21 +- internal/manual/chat/what-i-can-do.md | 7 + internal/session/heldreads.go | 260 ++++++++++++++++++ internal/session/heldreads_test.go | 153 +++++++++++ internal/session/loop.go | 11 +- internal/session/prefixbudget_test.go | 7 + internal/session/prompts/system.md | 1 + internal/session/session.go | 6 + 9 files changed, 479 insertions(+), 2 deletions(-) create mode 100644 docs/changes/unreleased/1006-read-held-ranges.md create mode 100644 internal/session/heldreads.go create mode 100644 internal/session/heldreads_test.go diff --git a/docs/changes/unreleased/1006-read-held-ranges.md b/docs/changes/unreleased/1006-read-held-ranges.md new file mode 100644 index 0000000000..2d7916a1ca --- /dev/null +++ b/docs/changes/unreleased/1006-read-held-ranges.md @@ -0,0 +1,15 @@ +--- +kind: fixed +title: a `read` of lines the conversation already holds answers with a pointer +pr: 1006 +surface: [engine, docs] +invalidates: + - "`read` fetched from disk on every call, no matter what was already in the transcript — measured: one planning chat issued eighteen reads of the same file as seven sliced ranges. A range fully covered by bytes the conversation still carries, with the file's stamp unmoved, now comes back as the one-line pointer `[already read] lines X–Y …`, and a partial overlap or a changed file still fetches whole." + - "`read`'s tool description priced only tokens, so a model slicing a file small had no reason to broaden it. It now states that each call is one round trip and that a range already in the conversation answers as a pointer, the system prompt's BELT_FACTS area teaches the pointer sentence beside the stub-pointer line, and the chat manual's `what-i-can-do` page quotes the pointer and the fresh-after-a-change rule." +--- + +The ledger (internal/session/heldreads.go) is per conversation and in memory +only. A span counts as held only while its transcript message still carries the +bytes verbatim — a stubbed or folded result drops out of coverage by itself — +and a changed stamp retires the file's whole entry, never merging two +generations of the file into coverage no version ever had. diff --git a/internal/exec/bare/tools.go b/internal/exec/bare/tools.go index 78d115a008..8dd9f72bfe 100644 --- a/internal/exec/bare/tools.go +++ b/internal/exec/bare/tools.go @@ -148,7 +148,26 @@ const lsSchemaJSON = `{"type":"object","properties":{"path":{"type":"string","de // every request on the belt. func readDescription(caps Caps) string { - return fmt.Sprintf("Read a file. Text, or an image (jpg, png, gif, webp, bmp) which comes back as an attachment. Text is cut at %d lines or %s, whichever comes first; page the rest with offset/limit.", caps.MaxLines, sizeWord(caps.MaxBytes)) + return fmt.Sprintf("Read a file. Text, or an image (jpg, png, gif, webp, bmp) which comes back as an attachment. Text is cut at %d lines or %s, whichever comes first; page with offset/limit. Each call is one round trip; a range already in the conversation answers as a pointer.", caps.MaxLines, sizeWord(caps.MaxBytes)) +} + +// ── what a read result's text can conclude about the file ──────────────────── + +// ReadPagingFooter reports whether a read result's text carries either of the +// two paging footers [readTool] appends. The converse is the ONLY door the +// holder of those bytes learns the file ended inside what it got: the footers +// are the tool's own sentences, built here and matched here, so the session's +// ledger reads the format at its one seam instead of guessing at strings. +func ReadPagingFooter(text string) bool { + return strings.Contains(text, "[Showing lines ") || strings.Contains(text, " more lines in file.") +} + +// ReadContentless reports whether a successful read answer carried no file +// bytes at all. The ONE such answer is the exceeds-limit line diagnostic +// [readTool] emits instead of content, and the held-ranges ledger must never +// record it as bytes the conversation holds. +func ReadContentless(text string) bool { + return strings.HasPrefix(text, "[Line ") && strings.Contains(text, " exceeds ") } func bashDescription(caps Caps) string { diff --git a/internal/manual/chat/what-i-can-do.md b/internal/manual/chat/what-i-can-do.md index 51b25d61a3..df579173bf 100644 --- a/internal/manual/chat/what-i-can-do.md +++ b/internal/manual/chat/what-i-can-do.md @@ -36,6 +36,13 @@ cut is announced so paging is possible: the end is an error: `Offset 900 is beyond end of file (120 lines total)`. A single line over the size cap is reported, not shown. +A read for a range already in the conversation comes back as a pointer instead +of fresh bytes: `[already read] lines X–Y are in this conversation above +— use those bytes.` Once any part of a file has been read here, asking for it +again is answered, not fetched — which keeps paging cheap. A file that changed +on disk since is simply read fresh, and a range some of which was never read is +fetched whole. + ## How much of a file does one read return — what is the truncation limit **The limit follows the model's context window: a tenth of it, and never more diff --git a/internal/session/heldreads.go b/internal/session/heldreads.go new file mode 100644 index 0000000000..1ba953a03a --- /dev/null +++ b/internal/session/heldreads.go @@ -0,0 +1,260 @@ +package session + +// Held reads: a `read` call whose bytes are already verbatim in this +// conversation's transcript is answered with a one-line pointer instead of a +// fresh fetch from the file system. +// +// The measured defect: one planning chat issued ten reads of the same 5,180-line +// file as seventeen small slices, each a 30–90 second model round trip, and every +// slice was then re-billed on every later request. The model slices small because +// it prices tokens, not latency, and the tool contract says nothing about either +// — nothing told it a call costs a round trip, and nothing told it it already +// held the bytes. The ledger below is the answer to the second fact; the tool's +// own description (bare/tools.go's [readDescription]) is the answer to the first. +// +// ── ONLY THE LIVE TRANSCRIPT COUNTS AS HELD ── +// +// A result whose bytes left the transcript does not count as held, however the +// ledger remembers the read. STUBBING REWRITES THE RESULT'S TEXT (stub.go's +// [stubMarker]) AND FOLDING REPLACES THE MESSAGE OUTRIGHT (turnfold.go), so the +// claim does not trust its own record: each held span carries the fingerprint of +// the message text that carried those bytes, and a span whose fingerprint no +// longer names a live tool message is dropped right there. An incremental +// ledger that trusted its record would need every one of those rewrite seams to +// notify it; checking the transcript at the claim keeps the rule in one place. +// +// ── THE STAMP IS THE FILE'S OWN CLOCK ── +// +// Coverage is claimed only while mtime+size match what the last read saw. A +// changed file invalidates its whole entry by itself, so the fresh read the +// claim then falls through to REPLACES the entry rather than accumulating two +// generations of spans. Nothing here is persisted or crosses a session boundary +// — the ledger is this agent's, in memory, and a resume rebuilds it the way it +// rebuilt the transcript. + +import ( + "encoding/json" + "fmt" + "hash/fnv" + "os" + "sort" + "sync" + "time" + + "github.com/Agent-Field/aforge-v2/internal/exec/bare" +) + +// heldLedger maps a resolved path to what this conversation already holds of +// its bytes. It is one mutex because dispatch is the only road in and the claim +// must never hold it while taking the transcript's lock. +type heldLedger struct { + mu sync.Mutex + files map[string]*heldFile +} + +// heldFile is one path's entry: the file's stamp at the last recorded read and +// the line spans that read — together or alone — returned. +type heldFile struct { + stamp heldStamp + spans []heldSpan +} + +// heldStamp is the two numbers os.Stat answers; comparable, so a mismatch is a +// whole-file invalidation with no clock-reading. +type heldStamp struct { + modified time.Time + size int64 +} + +// heldSpan is one recorded read's range: one-based first line, last line (zero +// when the read reached the end of the file), whether it reached the end, and +// the fingerprint of the message text that carried it. +type heldSpan struct { + start int + end int + eof bool + proof uint64 +} + +// heldSpec is one call's request, normalized out of its arguments. +type heldSpec struct { + path string // as the model spelled it, for the pointer answer + start int // one-based first line + end int // one-based last line; zero asks for the file to its end + abs string // resolved against the agent's workspace +} + +// heldClaim is the dispatch-door half of the ledger: ALL coverage questions are +// answered here. A fully covered range is returned as the pointer sentence; +// anything else — no entry, a changed stamp, a stubbed away result, a gap in +// the union of spans — falls through to the ordinary fetch. +func (a *Agent) heldClaim(name string, args json.RawMessage) (string, bool) { + if name != "read" { + return "", false + } + spec, ok := a.heldSpecOf(args) + if !ok { + return "", false + } + stamp, ok := heldStampOf(spec.abs) + if !ok { + // A file nothing can stat is the read tool's own error to report, and + // it is never the ledger's answer to give. + return "", false + } + a.held.mu.Lock() + file := a.held.files[spec.abs] + a.held.mu.Unlock() + if file == nil || file.stamp != stamp { + return "", false + } + // The fingerprint of every live tool message, taken under mu rather than + // the ledger's — the inverse order would see the ledger's claim and + // the transcript's rewriters praying for one another. + live := a.toolFingerprints() + var spans []heldSpan + for _, span := range file.spans { + if live[span.proof] { + spans = append(spans, span) + } + } + if !heldCovers(spans, spec) { + return "", false + } + // ONE SENTENCE, NO MACHINERY IN IT: the model has the bytes and says where. + if spec.end > 0 { + return fmt.Sprintf("[already read] %s lines %d–%d are in this conversation above — use those bytes.", spec.path, spec.start, spec.end), true + } + return fmt.Sprintf("[already read] %s from line %d to the end is in this conversation above — use those bytes.", spec.path, spec.start), true +} + +// heldNote is the record half, called with the finished result: errors and the +// tool's one successful-but-empty answer (bare.ReadContentless) never leave a +// span. A span is recorded for an explicit limit without any footer question — +// the limit names the range exactly, and the footer is what then says whether +// the file ended inside it (bare.ReadPagingFooter). A whole-file read that was +// cut by the caps is never recorded at all, because there is no honest end to +// hold. +func (a *Agent) heldNote(name string, args json.RawMessage, text string, isError bool) { + if name != "read" || isError || bare.ReadContentless(text) { + return + } + spec, ok := a.heldSpecOf(args) + if !ok { + return + } + eof := !bare.ReadPagingFooter(text) + if spec.end == 0 && !eof { + // A PAGED WHOLE-FILE READ HAS NO END TO OFFER, and a span without one + // would claim the file's tail for ever. It simply contributes nothing. + return + } + stamp, ok := heldStampOf(spec.abs) + if !ok { + return + } + span := heldSpan{start: spec.start, end: spec.end, eof: eof, proof: heldFingerprint(text)} + a.held.mu.Lock() + defer a.held.mu.Unlock() + if a.held.files == nil { + a.held.files = make(map[string]*heldFile) + } + file := a.held.files[spec.abs] + if file == nil || file.stamp != stamp { + // A changed stamp retires the entry whole: two generations of spans + // against one file would merge into coverage no version ever had. + file = &heldFile{stamp: stamp} + a.held.files[spec.abs] = file + } + // The two reads that together cover a range are MERGED AT THE CLAIM, not + // here: the spans append, and the union arithmetic that decides coverage + // runs over whatever the entry holds when the model asks. + file.spans = append(file.spans, span) +} + +// heldSpecOf normalizes one call's path, offset and limit into the range it +// asks for, resolved against the agent's workspace exactly the way the tool +// resolves it. An unparseable argument is not a ledger question at all. +func (a *Agent) heldSpecOf(args json.RawMessage) (heldSpec, bool) { + var p struct { + Path string `json:"path"` + Offset *int `json:"offset"` + Limit *int `json:"limit"` + } + if err := json.Unmarshal(args, &p); err != nil || p.Path == "" { + return heldSpec{}, false + } + spec := heldSpec{path: p.Path, start: 1, end: 0} + if p.Offset != nil && *p.Offset > 1 { + spec.start = *p.Offset + } + if p.Limit != nil && *p.Limit > 0 { + spec.end = spec.start + *p.Limit - 1 + } + spec.abs = bare.ResolvePath(p.Path, a.config.Workspace) + return spec, true +} + +// heldStampOf reads the two numbers an entry compares, or says nothing can be +// known about the file. +func heldStampOf(path string) (heldStamp, bool) { + info, err := os.Stat(path) + if err != nil { + return heldStamp{}, false + } + return heldStamp{modified: info.ModTime(), size: info.Size()}, true +} + +// heldCovers is the union arithmetic: sorted spans must run contiguously from +// the request's first line to its last, and a request for the rest of the file +// needs one reach-EOF span to close it. A gap at any point is a miss, and a +// miss is a fresh read — the claim never half-holds a range. +func heldCovers(spans []heldSpan, spec heldSpec) bool { + if len(spans) == 0 { + return false + } + sort.Slice(spans, func(i, j int) bool { return spans[i].start < spans[j].start }) + reached := spec.start // one-based next line still owed + for _, span := range spans { + if span.start > reached { + // A gap before this span: sorted by start, nothing later closes it. + return false + } + if span.eof { + // This span ran to the end of the file, and its start was already + // owed — every request it touches is answered. + return true + } + if span.end >= reached { + reached = span.end + 1 + } + if spec.end > 0 && reached > spec.end { + return true + } + } + return spec.end > 0 && reached > spec.end +} + +// toolFingerprints answers which message texts the live transcript still +// carries, computed under the transcript's own lock so a fold or a stub mid- +// claim cannot split the answer. +func (a *Agent) toolFingerprints() map[uint64]bool { + a.mu.Lock() + defer a.mu.Unlock() + out := make(map[uint64]bool, len(a.messages)) + for index := range a.messages { + if a.messages[index].Role != "tool" { + continue + } + out[heldFingerprint(messageContentText(a.messages[index]))] = true + } + return out +} + +// heldFingerprint weights one message's text with an 8-byte hash, so the claim +// can hold the proof of a 50KB read without holding the read. +func heldFingerprint(text string) uint64 { + hash := fnv.New64a() + _, _ = hash.Write([]byte(text)) + return hash.Sum64() +} diff --git a/internal/session/heldreads_test.go b/internal/session/heldreads_test.go new file mode 100644 index 0000000000..ef68668248 --- /dev/null +++ b/internal/session/heldreads_test.go @@ -0,0 +1,153 @@ +package session + +// Held-range ledger probes: a read for bytes the conversation already has is a +// pointer; a changed file, a partial overlap, a stubbed away result and a gap +// in the union are all plain fresh reads. Each probe runs the dispatch door +// itself (loop.go's [Agent.dispatchTool]), so what is asserted is what the +// model would be handed, not only the ledger's arithmetic. + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/Agent-Field/agentfield/sdk/go/ai" +) + +// readHeld runs one `read` through the whole dispatch door and records the +// result in the transcript exactly the way runTurn does, which is the only road +// the ledger's claim will consult later. +func readHeld(t *testing.T, agent *Agent, id, args string) toolResult { + t.Helper() + call := ai.ToolCall{ID: id, Function: ai.ToolCallFunction{Name: "read", Arguments: args}} + result := agent.executeTool(context.Background(), agent.newEpisode(), nil, call, "") + agent.record(ai.Message{ + Role: "tool", + ToolCallID: call.ID, + Content: []ai.ContentPart{{Type: "text", Text: result.text}}, + }) + return result +} + +func heldArgs(path string, offset, limit int) string { + args := map[string]any{"path": path} + if offset > 0 { + args["offset"] = offset + } + if limit > 0 { + args["limit"] = limit + } + out, _ := json.Marshal(args) + return string(out) +} + +// A read whose range the transcript already holds is answered with the pointer +// — and proven so by making the file unreadable between the two calls: a second +// fetch from disk would have been an error, and the pointer is not. +func TestACoveredReadComesBackAsAPointer(t *testing.T) { + agent, workspace := newTestAgent(t, &scriptedCompleter{}, nil) + path := filepath.Join(workspace, "page.md") + writeFile(t, path, "one\ntwo\nthree\nfour\nfive\n") + + first := readHeld(t, agent, "c1", heldArgs(path, 0, 0)) + if first.isError || !strings.Contains(first.text, "three") { + t.Fatalf("the plain read did not run: %.80q", first.text) + } + if err := os.Chmod(path, 0o000); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(path, 0o600) }) + + second := readHeld(t, agent, "c2", heldArgs(path, 0, 0)) + if second.isError || !strings.HasPrefix(second.text, "[already read] ") { + t.Fatalf("a held range was fetched rather than pointed at: %.80q", second.text) + } +} + +// A file whose stamp moved since the read is read fresh, and its entry is +// replaced rather than merged: the new bytes are what comes back, and the old +// span no longer covers anything. +func TestAChangedFileIsReadFresh(t *testing.T) { + agent, workspace := newTestAgent(t, &scriptedCompleter{}, nil) + path := filepath.Join(workspace, "page.md") + writeFile(t, path, "old bytes\n") + readHeld(t, agent, "c1", heldArgs(path, 0, 0)) + writeFile(t, path, "new bytes, more of them\n") + + again := readHeld(t, agent, "c2", heldArgs(path, 0, 0)) + if strings.Contains(again.text, "[already read] ") { + t.Fatalf("a changed file was pointed at: %.80q", again.text) + } + if !strings.Contains(again.text, "new bytes") { + t.Fatalf("the fresh read did not carry the new bytes: %.80q", again.text) + } +} + +// Partial overlap is never trimmed and never half-held: the second call reads +// whole from disk, and after it the union covers both ends. +func TestAPartialOverlapReadsFreshAndMerges(t *testing.T) { + agent, workspace := newTestAgent(t, &scriptedCompleter{}, nil) + path := filepath.Join(workspace, "page.md") + writeFile(t, path, "one\ntwo\nthree\nfour\nfive\nsix\nseven\neight\nnine\n") + + readHeld(t, agent, "c1", heldArgs(path, 1, 3)) + overlap := readHeld(t, agent, "c2", heldArgs(path, 3, 6)) + if strings.Contains(overlap.text, "[already read] ") { + t.Fatalf("a partial overlap was pointed at instead of read whole: %.80q", overlap.text) + } + if !strings.Contains(overlap.text, "eight") { + t.Fatalf("the overlap did not carry its full fresh range: %.80q", overlap.text) + } + whole := readHeld(t, agent, "c3", heldArgs(path, 1, 8)) + if !strings.HasPrefix(whole.text, "[already read] ") { + t.Fatalf("the merged union did not cover lines 1–8: %.80q", whole.text) + } +} + +// Two reads of one file hold their own ranges independently: a request over a +// gap between them misses, and a request inside one of them is a pointer. +func TestTwoRangesHoldIndependently(t *testing.T) { + agent, workspace := newTestAgent(t, &scriptedCompleter{}, nil) + path := filepath.Join(workspace, "page.md") + writeFile(t, path, "one\ntwo\nthree\nfour\nfive\nsix\nseven\neight\nnine\n") + + readHeld(t, agent, "c1", heldArgs(path, 1, 3)) + readHeld(t, agent, "c2", heldArgs(path, 7, 3)) + + gap := readHeld(t, agent, "c3", heldArgs(path, 1, 9)) + if strings.Contains(gap.text, "[already read] ") { + t.Fatalf("a gap between held ranges was pointed at: %.80q", gap.text) + } + inside := readHeld(t, agent, "c4", heldArgs(path, 7, 2)) + if !strings.HasPrefix(inside.text, "[already read] ") { + t.Fatalf("a request inside one held range missed: %.80q", inside.text) + } +} + +// A result stubbed out of the transcript no longer counts as held: stub.go +// rewrites the message's text to stubMarker, and the claim must then fetch. +func TestAStubbedReadStopsCountingAsHeld(t *testing.T) { + agent, workspace := newTestAgent(t, &scriptedCompleter{}, nil) + path := filepath.Join(workspace, "page.md") + writeFile(t, path, "one\ntwo\nthree\n") + readHeld(t, agent, "c1", heldArgs(path, 0, 0)) + + agent.mu.Lock() + for index := range agent.messages { + if agent.messages[index].Role == "tool" { + agent.messages[index].Content = []ai.ContentPart{{Type: "text", Text: stubMarker + " read · stubbed]"}} + } + } + agent.mu.Unlock() + + after := readHeld(t, agent, "c2", heldArgs(path, 0, 0)) + if strings.Contains(after.text, "[already read] ") { + t.Fatalf("a stubbed-out held range was still pointed at: %.80q", after.text) + } + if !strings.Contains(after.text, "three") { + t.Fatalf("the fresh read did not re-fetch the bytes: %.80q", after.text) + } +} diff --git a/internal/session/loop.go b/internal/session/loop.go index 2878eee4de..60f0676c44 100644 --- a/internal/session/loop.go +++ b/internal/session/loop.go @@ -3223,6 +3223,11 @@ func (a *Agent) dispatchTool(ctx context.Context, ep *episode, hub *eventHub, ca // citizen's rewrite is what runs — and the TOOL is the one dispatch already // found, because the name is what got us here. args := json.RawMessage(running.Function.Arguments) + // A read for bytes this conversation already has is answered from above, + // not from the disk (heldreads.go) — the gate above already judged it. + if pointer, held := a.heldClaim(call.Function.Name, args); held { + return a.finishToolResult(ep, call, toolResult{text: pointer}) + } started := time.Now() // THE CALL'S OWN ID TRAVELS WITH IT. It is what lets a tool still // running be addressed from outside the turn — the surface's key that @@ -3270,7 +3275,11 @@ func (a *Agent) dispatchTool(ctx context.Context, ep *episode, hub *eventHub, ca // observer of results would otherwise belong: runTurn writes each result // into the transcript BEFORE that seam runs, so a line added there would // be a line no model was ever sent. - return a.finishToolResult(ep, call, toolResult{text: text, isError: isError}) + result := a.finishToolResult(ep, call, toolResult{text: text, isError: isError}) + // The held-range ledger records on the FINISHED text — exactly the bytes + // the transcript will carry — and only on a result that ran (heldreads.go). + a.heldNote(call.Function.Name, args, result.text, isError) + return result } // ── TAKEN, OR NEVER HELD ── // diff --git a/internal/session/prefixbudget_test.go b/internal/session/prefixbudget_test.go index 450a2bf98f..3c792fa20e 100644 --- a/internal/session/prefixbudget_test.go +++ b/internal/session/prefixbudget_test.go @@ -388,6 +388,13 @@ const fixedPrefixBudget = 48_000 // still move. A wave that needs more than this is a wave that takes something // out first. // +// THE HELD-RANGES WAVE PAID FOR THE ROUND-TRIP PRICE IT TEACHES (2026-09-12). +// The read tool's description says a call costs one round trip and a range +// already in the conversation answers as a pointer (bare/tools.go's +// [readDescription], the ledger in heldreads.go). The sentence was priced +// into the lean arm's headroom — 87 bytes came in, 9 came back out of the +// description's redundant "the rest", and the lean prefix sits 5 under its +// budget. // THEREAFTER IT ONLY EVER RATCHETS DOWN, in the ledger discipline the full // budget above is kept under: a lane that takes bytes out lowers it in the same // commit, and nothing ever raises it again. diff --git a/internal/session/prompts/system.md b/internal/session/prompts/system.md index d578021d90..e9a956c423 100644 --- a/internal/session/prompts/system.md +++ b/internal/session/prompts/system.md @@ -152,6 +152,7 @@ deliverable it names, by its full path, and answer out of that. BELT_FACTS - NUMBERS AND FACTS COME FROM THE CONVERSATION: quote figures and claims from anything already seen here — earlier turns, earlier steps of this turn, or stubbed output you have read. An honest miss beats a fluent reconstruction. - `[output stubbed - N bytes - full output: ]` lost nothing: `read` that path when its bytes are not already here. Once read, its content remains available for the conversation; never restate an unread stub as output. +- `read` answering `[already read] …` means the bytes are in the conversation above: answer from them rather than fetching the file a second time. BEFORE RUNNING A COMMAND, CHECK THE TRANSCRIPT. If its answer is already here, use it. Re-deriving a settled fact is a defect, not diligence. diff --git a/internal/session/session.go b/internal/session/session.go index 921b90db86..f94deb07f0 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -2158,6 +2158,12 @@ type Agent struct { // arming happens inside a tool call, and a tool call must never take the // lock Interrupt has to be able to take. armMu sync.Mutex + // held is the held-range ledger (heldreads.go): which line ranges of which + // files this conversation's transcript already carries verbatim, so a read + // for exactly that is answered with a pointer and the disk is not opened. + // Like jobs it sits outside mu and holds its own lock — dispatch's claim + // takes the transcript's lock second, never first. + held heldLedger // connect is the accounts seam, nil when the feature is absent (connect.go). // It is written once at construction and read without a lock. connect connectHub From be5919c0d1c461d5c36549bf269411a47f2d0a4d Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 12 Sep 2026 13:25:04 -0400 Subject: [PATCH 06/16] lanes: the cache pin joins the lane chooser's enforced set rather than being dropped by it --- .../unreleased/1008-cache-pin-frontier.md | 14 ++++ internal/provider/lanes.go | 62 +++++++++----- internal/provider/routing_price_test.go | 81 +++++++++++++++++++ 3 files changed, 135 insertions(+), 22 deletions(-) create mode 100644 docs/changes/unreleased/1008-cache-pin-frontier.md diff --git a/docs/changes/unreleased/1008-cache-pin-frontier.md b/docs/changes/unreleased/1008-cache-pin-frontier.md new file mode 100644 index 0000000000..db56699d08 --- /dev/null +++ b/docs/changes/unreleased/1008-cache-pin-frontier.md @@ -0,0 +1,14 @@ +--- +kind: fixed +title: the cache pin is admitted into the lane chooser's enforced set rather than dropped by it +pr: 1008 +surface: [chat] +invalidates: + - "A cache pin the lane chooser's speed frontier did not name was dropped from the wire by `applyLaneChoice`, so a warm-but-second-fastest endpoint lost the call and one measured chat session rotated through five endpoints in eight calls, each hop a cold 80-110k-token prefix. The pin is now added to the enforced `only` set and still leads its order; only a role's patience refusal or a person's own lane pin removes it." +--- + +The advisory `provider.order` first-entry was honoured 29% of the time over +ten days of call log, while a strict preference's named machine served 93%. +The pin now rides inside the enforced set — and the two removals it may still +suffer are the one's measured: a role's patience refusal (2026-09-11) and a +person's own lane row. diff --git a/internal/provider/lanes.go b/internal/provider/lanes.go index d3770899a2..dc5ebc49e9 100644 --- a/internal/provider/lanes.go +++ b/internal/provider/lanes.go @@ -370,33 +370,52 @@ func (c *Client) applyLaneChoice(prefs *providerPrefs, model string, knobs callK // router's coarser model-list ceiling afterwards can only contradict that // decision and make a serving lane look as though it refused the model. demand := namedLanes(choice.Only) + // A PERSON'S OWN PIN IS READ ONCE, FOR BOTH DOORS BELOW (lanepin.go's + // [lanePinFor]). A person naming a machine wipes the cache pin off the + // demand here exactly as it wipes it off the order below: a heuristic + // about a cache is somebody's guess, and `pinned: cloudflare` is + // somebody's instruction. + person, retired := lanePinFor(model) + ownLane := person.pinned() != "" && !retired + // AND THE CACHE PIN IS ADMITTED BESIDE THE FRONTIER, NOT DROPPED BY IT. A + // demand is priced on latency times λ and tariff, and it never weighs + // cache residency — so a warm lane ranked second on speed fell off every + // forced set, the answerer became the next pin, and one measured chat + // session rotated through five machines in eight calls (GMICloud, Wafer, + // Novita, IoNet, Venice), each hop re-pricing eighty to a hundred thousand + // cold tokens at 4.7× the warm one. Admission is what makes the pin mean + // anything once a demand exists: advisory `provider.order` named the + // answering machine 29 % of the time over ten days of call log, while the + // strict preference's NAMED machine served 93 % — the pin goes IN the + // 93 % set, and it still leads the order composed over that set below. + if pinned != "" && len(demand) > 0 && !ownLane && + !namesEndpoint(choice.Ignore, pinned) && !namesEndpoint(demand, pinned) { + demand = append(demand, pinned) + } if len(demand) > 0 { no := false prefs.Only, prefs.Order, prefs.Sort = demand, nil, "" prefs.AllowFallbacks = &no prefs.MaxPrice = nil } - // AND THE AFFINITY PIN YIELDS TO A REFUSAL, which is the one thing a warm - // prefix cannot buy its way past. - // - // THE MEASURED CASE IS WHY (2026-09-11 09:33). The cache pin latches onto - // whichever machine ANSWERED (affinity.go), and `provider.order` is advisory - // once `allow_fallbacks` is true — so one request that asked for one machine - // and was answered by another made that other machine the head of every - // order for the rest of the session, and the reflex tier's whole wait - // doubled. A saving on prefill cannot pay for a wait the role has already - // said it will not sit through; a machine in `choice.Ignore` is one the role - // refused on its own declared patience (internal/lane's beyondThePatience), - // and it goes on this body's `ignore` below rather than at the front of its - // order. + // AND THE PIN YIELDS TO EXACTLY TWO THINGS: A ROLE'S REFUSAL AND A + // PERSON'S OWN PIN. The demand above can no longer remove it — the law + // is admission, and the doors left standing are the two that must still + // remove it: a refusal the role declared (measured below) and a + // person's own lane row (lanepin.go, read where the order is composed + // below). // - // AND A MACHINE OUTSIDE THE DEMAND IS THE SAME FACT STATED POSITIVELY. The - // admitted set is what the role would wait for; a machine that is not in it - // is one the frontier ruled out, and leading the order with it would either - // be ignored by the router or — under `allow_fallbacks: false` — name a - // machine the same object forbids. - if pinned != "" && (namesEndpoint(choice.Ignore, pinned) || - (len(demand) > 0 && !namesEndpoint(demand, pinned))) { + // THE REFUSAL IS THE MEASURED CASE (2026-09-11 09:33). The cache pin + // latches onto whichever machine ANSWERED (affinity.go), and `provider.order` + // is advisory wherever `allow_fallbacks` is still true — so one request that + // asked for one machine and was answered by another made that other machine + // the head of every order for the rest of the session, and the reflex tier's + // whole wait doubled — a wait the role had already declared it would not sit + // through. A saving on prefill cannot pay for that wait, and a machine in + // `choice.Ignore` is one the role refused on its own declared patience + // (internal/lane's beyondThePatience), so it goes on this body's `ignore` + // below rather than at the front of its order. + if pinned != "" && namesEndpoint(choice.Ignore, pinned) { pinned = "" } if len(choice.Order) > 0 { @@ -407,8 +426,7 @@ func (c *Client) applyLaneChoice(prefs *providerPrefs, model string, knobs callK // pin is somebody naming the machine they want. Letting the cache jump // the person would make `pinned: cloudflare, borrow when slow` mean "go // wherever the last answer came from", which is not what the row says. - person, retired := lanePinFor(model) - if person.pinned() != "" && !retired { + if ownLane { pinned = "" } if pinned != "" { diff --git a/internal/provider/routing_price_test.go b/internal/provider/routing_price_test.go index 1555e484da..1a0ea4fd20 100644 --- a/internal/provider/routing_price_test.go +++ b/internal/provider/routing_price_test.go @@ -332,6 +332,87 @@ func TestAStrictLaneDemandCarriesNoPriceCeiling(t *testing.T) { } } +// ── THE CACHE PIN BESIDE THE FRONTIER ──────────────────────────────────── + +// A warm lane the frontier did not name is admitted into the enforced set +// rather than dropped, because a demand priced on speed alone rotates the +// lineage through cold prefixes — measured at five machines in eight calls, +// 4.7× the warm bill each hop. +func TestACachePinOutsideTheFrontierIsAdmittedBesideIt(t *testing.T) { + client, _ := pricedClient(t, nil, 0.66e-6, 1.98e-6, true) + choice := lanes.Choice{Only: []string{"Fireworks"}, Order: []string{"Fireworks"}} + prefs := &providerPrefs{Sort: "latency"} + client.applyLaneChoice(prefs, "vendor/fast-model", callKnobs{laneChoice: &choice}, "Wafer") + if len(prefs.Only) != 2 || !namesEndpoint(prefs.Only, "Wafer") || + !namesEndpoint(prefs.Only, "Fireworks") { + t.Fatalf("demand = %v, want the warm lane admitted beside the frontier", prefs.Only) + } + if len(prefs.Order) == 0 || prefs.Order[0] != "Wafer" { + t.Fatalf("order = %v, want the admitted pin still leading it", prefs.Order) + } + if prefs.AllowFallbacks == nil || *prefs.AllowFallbacks { + t.Fatalf("allow_fallbacks = %v, want it off on a demand no matter who joined it", + prefs.AllowFallbacks) + } + // The frontier priced every admitted lane, including the pin, so the + // ceiling still comes off — the same answer the strict-demand test above + // pins. + if prefs.MaxPrice != nil { + t.Fatalf("the admitted pin kept a ceiling: %+v", prefs.MaxPrice) + } +} + +// A pin a role refused on its own declared patience is dropped even when a +// demand exists, which is the 2026-09-11 measured case the admission must +// not erase. +func TestACachePinBeyondThePatienceIsDropped(t *testing.T) { + client, _ := pricedClient(t, nil, 0.66e-6, 1.98e-6, true) + choice := lanes.Choice{ + Only: []string{"Fireworks"}, + Order: []string{"Fireworks"}, + Ignore: []string{"Wafer"}, + } + prefs := &providerPrefs{Sort: "latency"} + client.applyLaneChoice(prefs, "vendor/fast-model", callKnobs{laneChoice: &choice}, "Wafer") + if namesEndpoint(prefs.Only, "Wafer") || namesEndpoint(prefs.Order, "Wafer") { + t.Fatalf("the refused lane still rode: %+v", prefs) + } + if !namesEndpoint(prefs.Ignore, "Wafer") { + t.Fatalf("the refusal was not carried onto ignore: %+v", prefs.Ignore) + } +} + +// A person's own lane row clears the cache pin from the demand as well as +// from the order, because a borrowable pin is an instruction and a cache is +// a guess. +func TestAPersonsOwnPinClearsTheCachePinFromTheDemand(t *testing.T) { + client, _ := pricedClient(t, nil, 0.66e-6, 1.98e-6, true) + RepinLane(LanePin{Lane: "Novita"}) + t.Cleanup(func() { RepinLane(LanePin{}) }) + choice := lanes.Choice{Only: []string{"Fireworks"}, Order: []string{"Fireworks"}} + prefs := &providerPrefs{Sort: "latency"} + client.applyLaneChoice(prefs, "vendor/fast-model", callKnobs{laneChoice: &choice}, "Wafer") + if namesEndpoint(prefs.Only, "Wafer") || namesEndpoint(prefs.Order, "Wafer") { + t.Fatalf("the cache pin jumped the person: %+v", prefs) + } +} + +// A chooser that demanded nothing leaves the advisory path exactly as it has +// always been: the pin leads the order, no demand exists to join, and no +// fallback switch is written. +func TestNoDemandLeavesTheAdvisoryPathUntouched(t *testing.T) { + client, _ := pricedClient(t, nil, 0.66e-6, 1.98e-6, true) + choice := lanes.Choice{Order: []string{"Fireworks"}} + prefs := &providerPrefs{Sort: "latency"} + client.applyLaneChoice(prefs, "vendor/fast-model", callKnobs{laneChoice: &choice}, "Wafer") + if prefs.Only != nil || prefs.AllowFallbacks != nil { + t.Fatalf("a demand came from nowhere: %+v", prefs) + } + if len(prefs.Order) != 2 || prefs.Order[0] != "Wafer" || prefs.Order[1] != "Fireworks" { + t.Fatalf("order = %v, want the pin still leading an advisory preference", prefs.Order) + } +} + // ── WHO ANSWERED ──────────────────────────────────────────────────────────── // The slot is how a caller learns which endpoint served ITS call, rather than From 75a1fc2b105e928f542e8a6e175421670ad81132 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 12 Sep 2026 13:54:35 -0400 Subject: [PATCH 07/16] provider: the OpenRouter-takeover gate counts a refusal only when the router's own pick was refused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A narrowed demand's refusal is evidence about our choice, not the router's default: for the gate to count it arming the takeover from within the takeover's own demands. shape reaches the door from the one classifier that holds the knobs (refusalobject.go's Client.refusalObject), and the refusal door asks it (routefirst.go's Client.noteRouterRefusal, velocity.go's two refuseLane branches). The answer sides stay shape-agnostic: an answer that arrived is a verdict about the machine that spoke it, whichever spine chose it. Measured in a live chat (2026-09-12): takeover armed, every narrowed demand 404'd, each 404 re-armed the gate — four 'Retry 1/2: relaxed the endpoint filter' lines at ~80k tokens a re-send before the first rung went bare. --- .../unreleased/1009-gate-demand-refusals.md | 12 ++ internal/manual/chat/lanes.md | 11 +- internal/provider/refusalobject.go | 18 ++- internal/provider/routefirst.go | 40 +++++- internal/provider/routefirst_test.go | 117 ++++++++++++++++++ internal/provider/velocity.go | 23 ++-- 6 files changed, 200 insertions(+), 21 deletions(-) create mode 100644 docs/changes/unreleased/1009-gate-demand-refusals.md diff --git a/docs/changes/unreleased/1009-gate-demand-refusals.md b/docs/changes/unreleased/1009-gate-demand-refusals.md new file mode 100644 index 0000000000..bcb45ed2a7 --- /dev/null +++ b/docs/changes/unreleased/1009-gate-demand-refusals.md @@ -0,0 +1,12 @@ +--- +kind: fixed +title: the OpenRouter-takeover gate counts a refusal only when the router's own pick was refused +pr: 1009 +surface: [engine, docs] +invalidates: + - "The gate counted every named refusal toward the takeover, including the refusals a takeover's own narrowed demands came back with. On a live chat (2026-09-12) that looped four times — demand, 404, re-arm — each with a full ~80k-token re-send, before the ladder's first rung went bare. Only a refusal of a bare (membership-unnarrowed) attempt counts now; a narrowed demand's refusal is evidence about our choice and never enters the gate (routefirst.go's Client.noteRouterRefusal)." +--- + +The manual page for lanes said every refused or unusable answer counts toward +aforge taking over from the router. It now says only refusals the router +itself earned count, and the takeover's own demands cannot keep it alive. diff --git a/internal/manual/chat/lanes.md b/internal/manual/chat/lanes.md index c4b4f51876..921f14b1b9 100644 --- a/internal/manual/chat/lanes.md +++ b/internal/manual/chat/lanes.md @@ -63,10 +63,13 @@ answer on it is being learned like any other. back refused (a 429, a machine that cannot serve the shape) or unusable (the thread lost, tool markup, a stream that had to be cut) — twice in a short while — aforge stops lending the router the choice and picks the machine itself, from the -lanes it has been watching all along. The conversation says so once, in one -sentence, and after about half an hour of good answers the choice is the -router's again. Pinning a machine yourself in `/model` ends it there and then: -your word outranks either of them. +lanes it has been watching all along. Only a refusal the ROUTER earned counts: +once aforge is the one choosing a machine — during a takeover, or under your +pin — a refusal of that pick is about the pick, not another strike against +the router, so a takeover's own demands cannot keep it alive. The conversation +says so once, in one sentence, and after about half an hour of good answers the +choice is the router's again. Pinning a machine yourself in `/model` ends it +there and then: your word outranks either of them. **`openrouter` is `auto` without the safety.** It is the same router routing, and aforge never takes over no matter what comes back. Choose it when you would diff --git a/internal/provider/refusalobject.go b/internal/provider/refusalobject.go index 9ae6d63bdd..10488629f1 100644 --- a/internal/provider/refusalobject.go +++ b/internal/provider/refusalobject.go @@ -144,6 +144,15 @@ type laneRefusal struct { // carried from the one classification so the dispatcher and surface never // have to parse the refusal a second time. Reset string + // AskedBare says the refused attempt went out with NO membership-narrowing + // preference — no pin, no rescue's demand, no strike list — so the refusal + // is evidence about the router's DEFAULT and may count toward a takeover + // (routefirst.go's [Client.noteRouterRefusal]). One false is "the shape was + // ours", the safest reading for a direct caller of [Client.laneRefusalFor] + // — a hedge arm, which is a demand of one machine by construction — and it + // is set true by the one classifier that holds the request's knobs + // ([Client.refusalObject]). + AskedBare bool } // struck reports whether there is a lane here for the ledger to WRITE OFF — a @@ -283,7 +292,14 @@ func (c *Client) refusalObject(request *ai.Request, knobs callKnobs, err error) return laneRefusal{} } model := c.modelFor(request) - return c.laneRefusalFor(model, c.onlyLane(model, knobs), err) + refusal := c.laneRefusalFor(model, c.onlyLane(model, knobs), err) + // THE REFUSAL'S SHAPE IS READ FROM THE OBJECT THE WIRE CARRIED, for + // [laneRefusal.AskedBare]'s reason: membership-narrowing is a fact about + // the composed preference object, not about the decision that filled it, + // and re-composing it here is exact because the ONE choice the call was + // decided on is stamped on the knobs ([Client.withLaneChoice]). + refusal.AskedBare = !c.wirePreferences(model, knobs).membershipNarrowing() + return refusal } // laneRefusalFor is the same classification asked by a caller that already diff --git a/internal/provider/routefirst.go b/internal/provider/routefirst.go index 7a86c3d0d6..22a33af0ee 100644 --- a/internal/provider/routefirst.go +++ b/internal/provider/routefirst.go @@ -187,10 +187,27 @@ func (c *Client) routingGateTakenOver(model string) bool { return gateFor(model).TakenOver(laneNow()) } -// noteRouterChoice is the write side, called where an answer's lane is known. -// Refused and bad move their own axes; a healthy answer moves both. It answers -// whether THIS call armed the takeover, so the sentence a person reads is said -// once, by the call that earned it, and never by the retry that follows. +// noteRouterChoice is the write side for ANSWERS, called where an answer's lane +// is known. Refused and bad move their own axes; a healthy answer moves both. +// It answers whether THIS call armed the takeover, so the sentence a person +// reads is said once, by the call that earned it, and never by the retry that +// follows. +// +// A REFUSAL IS EVIDENCE ABOUT THE ROUTER'S DEFAULT ONLY WHEN THE ROUTER'S +// DEFAULT WAS ASKED. A demand this process narrowed, that the router then +// refused, is evidence about OUR CHOICE — and it must not hold the road. That +// is what [Client.noteRouterRefusal] splits, and the measurement that priced +// it: a live chat (2026-09-12) held four `Retry 1/2: relaxed the endpoint +// filter` lines, because the takeover had armed on earlier refusals, and every +// narrowed demand it then chose answered 404 — and counted as ANOTHER refusal, +// arming the takeover again, each cost of the loop a full ~80k-token re-send +// BEFORE the ladder's first rung could go bare. Only a bare refusal may count. +// +// THE ANSWER SIDES NEED NO SHAPE SPLIT. An answer that arrived is not a +// refusal of anything: a good one is a queue drained, and an unusable one is a +// verdict about the machine that spoke it, whichever spine chose it. So +// NoteHealthy and NoteBad apply regardless of what the request demanded; only +// the refusal axis narrows. func (c *Client) noteRouterChoice(model, served, reason string, accepted bool) (armed bool) { model = laneModel(model) if !c.carriesPreferences() || model == "" { @@ -215,6 +232,21 @@ func (c *Client) noteRouterChoice(model, served, reason string, accepted bool) ( } } +// noteRouterRefusal is the refusal door, reached from velocity.go's +// [Client.refuseLane] — the only door a refusal ever passes. It carries the +// attempt's shape because the law above splits refusals on it: only a refusal +// of a BARE attempt is evidence about the router's default, and only that may +// count toward a takeover. A refusal of a demand this process narrowed is a +// verdict about OUR CHOICE — the picker earned that, not the router — and the +// gate hears nothing, which is what keeps a takeover's own narrowed demands +// from re-arming the takeover that chose them. +func (c *Client) noteRouterRefusal(model, served string, askedBare bool) (armed bool) { + if !askedBare { + return false + } + return c.noteRouterChoice(model, served, "refused", false) +} + // armTakeover puts the gate over for one model, and forgetRouterGates empties // the map. They are for tests: a fixture about the chooser's wire object needs // the chooser to be holding the road, and no fixture may inherit another's diff --git a/internal/provider/routefirst_test.go b/internal/provider/routefirst_test.go index 77494756c9..ab792076ae 100644 --- a/internal/provider/routefirst_test.go +++ b/internal/provider/routefirst_test.go @@ -1,8 +1,12 @@ package provider import ( + "net/http" "testing" "time" + + lanes "github.com/Agent-Field/aforge-v2/internal/lane" + "github.com/Agent-Field/agentfield/sdk/go/ai" ) // ── THE GATE, MEASURED ────────────────────────────────────────────────────── @@ -146,3 +150,116 @@ func TestTheTwoModelsKeepTwoGates(t *testing.T) { t.Fatal("one model's bad afternoon took the other model's road with it") } } + +// ── THE SHAPE-AWARE REFUSAL DOOR ──────────────────────────────────────────── +// +// The measured loop (live chat, 2026-09-12): a takeover armed; the chooser's +// narrowed demand answered 404; the gate counted THAT refusal, arming the +// road again; four "Retry 1/2: relaxed the endpoint filter" lines, each paying +// a full ~80k-token re-send, before the first rung went bare. The tests below +// pin the split: only a BARE attempt's refusal is evidence about the router's +// default, and the classifier reads the attempt's shape from the wire object +// the request actually went out with. + +func TestANarrowedRefusalIsAboutOurChoiceNotTheRouter(t *testing.T) { + forgetRouterGates() + t.Cleanup(forgetRouterGates) + client := &Client{config: Config{BaseURL: "https://openrouter.ai/api/v1"}} + lanes.HeardPrefsCarried(client.config.BaseURL) + const model = "openrouter/shape-gate" + for range takeoverStrikes * 2 { + if client.noteRouterRefusal(model, "brass", false) { + t.Fatal("a refusal of our own demand claimed takeover") + } + } + if gateFor(model).TakenOver(time.Now()) { + t.Fatal("narrowed refusals took the road from the router") + } +} + +func TestBareRefusalsArmTheTakeover(t *testing.T) { + forgetRouterGates() + t.Cleanup(forgetRouterGates) + client := &Client{config: Config{BaseURL: "https://openrouter.ai/api/v1"}} + lanes.HeardPrefsCarried(client.config.BaseURL) + const model = "openrouter/shape-gate" + if client.noteRouterRefusal(model, "brass", true) { + t.Fatal("one bare refusal armed the takeover; one is a queue draining") + } + if !client.noteRouterRefusal(model, "brass", true) { + t.Fatal("the second bare refusal did not arm the takeover") + } + if !client.routingGateTakenOver(model) { + t.Fatal("two bare refusals and the gate still lends the router the road") + } +} + +func TestANarrowedRefusalCannotHoldTheRoad(t *testing.T) { + forgetRouterGates() + t.Cleanup(forgetRouterGates) + client := &Client{config: Config{BaseURL: "https://openrouter.ai/api/v1"}} + lanes.HeardPrefsCarried(client.config.BaseURL) + const model = "openrouter/shape-gate" + if client.noteRouterRefusal(model, "brass", true) { + t.Fatal("setup: one bare refusal armed") + } + if !client.noteRouterRefusal(model, "brass", true) { + t.Fatal("setup: two bare refusals did not arm") + } + gate := gateFor(model) + gate.mu.Lock() + held := gate.until + gate.mu.Unlock() + if client.noteRouterRefusal(model, "brass", false) { + t.Fatal("a narrowed refusal re-armed a held takeover") + } + gate.mu.Lock() + after := gate.until + gate.mu.Unlock() + if !after.Equal(held) { + t.Fatalf("a narrowed refusal moved the takeover's lapse from %v to %v", held, after) + } + if !client.routingGateTakenOver(model) { + t.Fatal("an armed takeover vanished under a narrowed refusal") + } +} + +func TestTheThresholdCountsOnlyBareRefusals(t *testing.T) { + forgetRouterGates() + t.Cleanup(forgetRouterGates) + client := &Client{config: Config{BaseURL: "https://openrouter.ai/api/v1"}} + lanes.HeardPrefsCarried(client.config.BaseURL) + const model = "openrouter/shape-gate" + if client.noteRouterRefusal(model, "one", true) { + t.Fatal("one bare refusal armed") + } + if client.noteRouterRefusal(model, "two", false) { + t.Fatal("a narrowed refusal counted toward the bare threshold") + } + if !client.noteRouterRefusal(model, "three", true) { + t.Fatal("the SECOND bare refusal did not finish the arming") + } + if !client.routingGateTakenOver(model) { + t.Fatal("bare, narrowed, bare did not arm at exactly two bares") + } +} + +func TestTheClassifierSpeaksTheAttemptsShape(t *testing.T) { + forgetRouterGates() + t.Cleanup(forgetRouterGates) + ledger, _ := testLedger() + client := &Client{config: Config{BaseURL: "https://openrouter.ai/api/v1"}, velocity: ledger} + lanes.HeardPrefsCarried(client.config.BaseURL) + const model = "openrouter/shape-gate" + paced := apiError(http.StatusTooManyRequests, []byte( + `{"error":{"message":"Provider returned error"}}`)) + + bare := client.refusalObject(&ai.Request{Model: model}, callKnobs{}, paced) + if !bare.AskedBare { + t.Fatal("a refusal with no membership-narrowing preference on the wire said the attempt was narrowed") + } + narrowed := client.refusalObject(&ai.Request{Model: model}, callKnobs{hedgeLane: "brass"}, paced) + if narrowed.AskedBare { + t.Fatal("a refusal of a rescue's one-machine demand said the attempt went out bare") + } +} diff --git a/internal/provider/velocity.go b/internal/provider/velocity.go index 799964a6c0..7fa08dc936 100644 --- a/internal/provider/velocity.go +++ b/internal/provider/velocity.go @@ -1257,12 +1257,11 @@ func (c *Client) refuseLane(model string, refusal laneRefusal, wait time.Duratio // remaining attempts of this very call included (retry.go). c.notePacedProvider(model, refusal.Lane, wait) c.noteLaneRefused(model, refusal.Lane, "paced") - // AND THE ROUTER'S GATE HEARS THE SAME REFUSAL ONCE (routefirst.go). - // This is the one door every refusal of a named machine passes, so it - // is where the gate's "twice lately" is kept true: a 429 here, a 404 - // below, an upstream fault beside them — one event, one strike, however - // the recovery above it goes. - if c.noteRouterChoice(model, refusal.Lane, "refused", false) { + // AND THE ROUTER'S GATE HEARS THE SAME REFUSAL ONCE, WITH THE ATTEMPT'S + // SHAPE (routefirst.go's [Client.noteRouterRefusal]): only a bare + // attempt's refusal is evidence about the router's default; a narrowed + // demand's refusal is a verdict on the pick, and the gate hears nothing. + if c.noteRouterRefusal(model, refusal.Lane, refusal.AskedBare) { parkTakeoverLine(model) } return true @@ -1284,12 +1283,12 @@ func (c *Client) refuseLane(model string, refusal laneRefusal, wait time.Duratio // count one refusal three times. The paced branch above is the one that // was silent, and the one the 429 loop lived in. c.refuseServing(model, refusal) - // AND THE GATE HEARS IT HERE, BESIDE THE STRIKE (routefirst.go) — the same - // door as the paced branch, for the same reason: one refusal of a named - // machine is one strike on the router's record, whether the machine said - // its queue was full or the pairing was impossible. An unasked machine - // (refusal.Unasked) teaches the gate nothing and never reaches this line. - if c.noteRouterChoice(model, refusal.Lane, "refused", false) { + // AND THE GATE HEARS IT HERE, BESIDE THE STRIKE, WITH THE ATTEMPT'S SHAPE + // (routefirst.go's [Client.noteRouterRefusal]): only a bare attempt's + // refusal is evidence about the router's default, exactly as in the paced + // branch. An unasked machine (refusal.Unasked) still teaches nothing and + // never reaches this line. + if c.noteRouterRefusal(model, refusal.Lane, refusal.AskedBare) { parkTakeoverLine(model) } return c.velocity.pace(model, refusal.Lane, 0) From 7d8d932d7060567589f9a73be17e7a4572f07a05 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 12 Sep 2026 14:34:39 -0400 Subject: [PATCH 08/16] tui3, session: a launch sweep names every live session on an older build rev MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The third binary-shadow incident in the repo's history ends with a visible warning instead of 'same old rev' on every screen. The sweep reads presence files (fail-open), freshness from presenceWindow (age, never pid — the file's own law), and the launch gate emits one advisory line through the same noteFacts door the landing keys use. Manual section added to starting-aforge.md. --- .../unreleased/1010-stale-build-sweep.md | 22 ++++++ internal/manual/chat/starting-aforge.md | 15 ++++ internal/session/stalebuild_test.go | 78 +++++++++++++++++++ internal/session/taskpresence.go | 59 ++++++++++++++ internal/tui3/app.go | 26 +++++++ 5 files changed, 200 insertions(+) create mode 100644 docs/changes/unreleased/1010-stale-build-sweep.md create mode 100644 internal/session/stalebuild_test.go diff --git a/docs/changes/unreleased/1010-stale-build-sweep.md b/docs/changes/unreleased/1010-stale-build-sweep.md new file mode 100644 index 0000000000..d212918c9e --- /dev/null +++ b/docs/changes/unreleased/1010-stale-build-sweep.md @@ -0,0 +1,22 @@ +--- +kind: added +title: a launch sweep names live sessions on an older build +surface: session, tui3 +pr: 1010 +--- + +Every launch now sweeps `~/.aforge/v3/projects/*/presence.json` and names any +live session holding a rev other than this binary's, with the serving pid — +the third binary-shadow incident ends with a visible warning instead of a +session that "stopped working" on the old rev. + +`internal/session` exports [SweepStaleBuilds] — a fail-open glob + freshness +filter, `presenceWindow` the only liveness rule — and `internal/tui3`'s open +path calls it once beside the landing-keys note. + +The law the sweep obeys is the presence file's own: age is the only liveness +check (the pid is reported for a person's `kill`, never consulted), which is +the same rule every other reader of those files has. + +Manual: starting-aforge.md's `/status says an older build is holding this +conversation` documents the line. diff --git a/internal/manual/chat/starting-aforge.md b/internal/manual/chat/starting-aforge.md index 0c79c24e40..a4a5740ae1 100644 --- a/internal/manual/chat/starting-aforge.md +++ b/internal/manual/chat/starting-aforge.md @@ -73,6 +73,21 @@ the life of the conversation. Starting in the "wrong" place is not a dead end, though: name the folder you actually meant with `/folder`, and the work goes there. +## /status says an older build is holding this conversation + +On every launch aforge performs one sweep: it reads every presence file under +`~/.aforge/v3/projects`, and if a live session there is running from an older +rev than this binary's own, one line in the conversation names the pids. That +is all `on an older build` means — the sweep warns. It never blocks, never +locks, never kills. + +The `/status` build line shows the serving process's build: for a chat that is +the process typed `aforge` with; for a conversation hosted by the resident +engine (`engine --daemon`) it is still whatever binary the engine launched +from. In both cases the fix is a close: close the older session or the older +engine, and its next open runs this build. No line at launch means nothing +stale is running. + ## The flags you can start it with | Flag | What it does | diff --git a/internal/session/stalebuild_test.go b/internal/session/stalebuild_test.go new file mode 100644 index 0000000000..c1d9b215f7 --- /dev/null +++ b/internal/session/stalebuild_test.go @@ -0,0 +1,78 @@ +package session + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + "time" +) + +// TestTheSweepReportsOnlyLiveOlderBuilds pins the gate's two rules: a fresh +// presence row on a different rev is reported, and three wrong kinds of row +// — same rev, stale file age, and empty build — are not. +func TestTheSweepReportsOnlyLiveOlderBuilds(t *testing.T) { + now := time.Now() + root := t.TempDir() + write := func(bucket, session string, p SessionPresence) { + dir := filepath.Join(root, bucket, session) + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + payload, err := json.Marshal(p) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "presence.json"), payload, 0o644); err != nil { + t.Fatal(err) + } + } + + write("bucketA", "sesold", SessionPresence{ + Schema: presenceSchema, SessionID: "sesold", Workspace: "/w/one", + Build: "f5e1dd98e built 2026-09-12 11:50", PID: 17376, + UpdatedAt: now, State: PresenceIdle, + }) + write("bucketB", "sesame", SessionPresence{ + Schema: presenceSchema, SessionID: "sesame", Workspace: "/w/two", + Build: "75a1fc2b1 built 2026-09-12 13:58", PID: 21896, + UpdatedAt: now, State: PresenceWorking, + }) + write("bucketC", "sestaleage", SessionPresence{ + Schema: presenceSchema, SessionID: "sestaleage", Workspace: "/w/old", + Build: "f5e1dd98e built 2026-09-12 11:50", PID: 40684, + UpdatedAt: now.Add(-presenceWindow - time.Second), State: PresenceIdle, + }) + write("bucketD", "senobuild", SessionPresence{ + Schema: presenceSchema, SessionID: "senobuild", Workspace: "/w/nob", + UpdatedAt: now, State: PresenceIdle, + }) + + rows := SweepStaleBuilds(root, "75a1fc2b1", now) + if len(rows) != 1 || rows[0].SessionID != "sesold" { + t.Fatalf("SweepStaleBuilds returned %+v — want exactly the one fresh row on the other rev", rows) + } + if rows[0].PID != 17376 || rows[0].Build != "f5e1dd98e built 2026-09-12 11:50" { + t.Fatalf("row carried the wrong facts: %+v", rows[0]) + } +} + +// TestTheSweepIgnoresUnkowableRows pins the fail-open half: an unreadable +// directory, a corrupt file and a missing rev argument each answer with no +// rows rather than a guess about who is stale. +func TestTheSweepIgnoresUnkowableRows(t *testing.T) { + if rows := SweepStaleBuilds(t.TempDir(), " ", time.Now()); len(rows) != 0 { + t.Fatalf("an empty rev argument must not sweep, got %v", rows) + } + root := t.TempDir() + bad := filepath.Join(root, "bucket", "ses") + if err := os.MkdirAll(bad, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(bad, "presence.json"), []byte("{"), 0o644); err != nil { + t.Fatal(err) + } + if rows := SweepStaleBuilds(root, "rev9", time.Now()); len(rows) != 0 { + t.Fatalf("a corrupt file sweeps nothing, got %v", rows) + } +} diff --git a/internal/session/taskpresence.go b/internal/session/taskpresence.go index 141eabb978..c30a72ddff 100644 --- a/internal/session/taskpresence.go +++ b/internal/session/taskpresence.go @@ -1410,3 +1410,62 @@ func sortPresence(rows []SessionPresence) { return rows[a].SessionID < rows[b].SessionID }) } + +// ── STALE-BUILD SWEEP: what a launch gate may warn about ──────────────────── + +// StaleBuildRow names one live session whose presence claims a build rev +// other than the process reading it now. It carries the session id, the +// workspace it was opened with, the build stamp, and the pid the owner +// wrote in — reported for a person's `kill`, NEVER for liveness (the +// header's own law: age is the only liveness rule this file has). +type StaleBuildRow struct { + SessionID string + Workspace string + Build string + PID int +} + +// SweepStaleBuilds is the launch gate's one question: which FRESH presence +// rows hold a build whose leading rev token is not `currentRev`. Files that +// are unreadable, unparsable, empty of Build, or older than [presenceWindow] +// are invisible to the sweep, by the same three rules the reader above has. +// +// The rev comparison's one rule: the first space-separated token of +// presence.Build is the rev (buildinfo stamps it " built