Warm the lint gate's summariser session within a review loop - #67
Merged
Conversation
The review loop's lint gate minted a fresh conversation on every round: `lint()` went through `agent…autonomous.run`, which mints `SessionId.fresh` per call, so each round re-paid the full cold prefix to emit a few hundred output tokens. Rebuilding a prefix costs roughly what 20 cache reads of it cost, so a gate that re-examines the same commands within one stage should resume instead. `lint` gains an overload taking a `Chat` (minted read-only by `Lint.summariserChat`), and `ReviewLoopState` carries that chat so the first round mints it and later rounds resume it. The agent-taking signature is unchanged and now delegates through a single-use chat. Resuming is only safe because the gate short-circuits: a round whose commands are all silent and exit zero returns `ReviewResult.empty` without a turn, so a conversation holding an earlier round's findings is never asked to judge a clean one. The summariser prompt also now states that the blocks it is shown supersede any earlier ones. Warmth is applied where the material repeats, not wherever a one-shot does. The reviewer picker, `cheapOneShot` commit messages, branch naming, `summarisePr` and stack discovery are each called once per stage or once per run over material that has changed by the next call, so they stay cold.
Review found the safety argument for reusing the lint summariser's conversation was too weak to rely on. `lint()` skips the LLM only when every command is both silent AND exits zero, but most real lint commands print on success — this project's own gate is `sbt compile`, which writes `[success] Total time: …` — so that short-circuit almost never fires and the reused conversation was consulted on every round, carrying each earlier round's findings. A repeated finding is not free: it re-enters `round.issues`, costs a coder fix turn, and lands in the returned `IgnoredIssues` as an issue the fixer "reported no fixes" for, on a round that would otherwise have been clean. So the conversation now carries forward only while the summariser has reported nothing. Any round in which it reports drops the conversation and the next round starts fresh — a conversation that has only ever said "nothing actionable" holds no finding to repeat, and the round right after a finding, where the risk is highest, always runs cold. Clean rounds are the common case, so most of the saving remains. Also pairs the gate with its conversation so neither can go missing without the other, and narrows the scaladoc and README claims to what the code actually guarantees.
Two review findings. The read-only invariant had gone from structural to conventional. On the agent-taking `lint`, `withReadOnly` was applied INSIDE the function, so no caller could bypass it; the conversation-taking overload accepted any `Chat`, and `Agent.chat()` is public, so `lint(cmds, myAgent.chat(), …)` compiled and would have run a write-enabled agent as the "read-only" lint gate. No shipped path did — the loop goes through the factory — but the scaladoc's "The LLM is invoked read-only" was no longer true by construction. The parameter is now `Lint.Summariser`, whose `private[review]` constructor makes `Lint.summariser(agent)` the only way to obtain one, mirroring `RosterEntry`. Verified both ways: substituting a bare chat is a compile error, the sanctioned path compiles. The gate-rejected half of the retirement predicate was untested — both existing tests used confidence 0.95, above the default Warning bar of 0.6, so only the `kept` disjunct ever fired. That is the non-obvious half: the gate decides what reaches the fixer, not what the conversation remembers, so a sub-threshold finding is still there to be repeated. Adds the test; deleting the disjunct now fails it and nothing else. Also narrows two claims to what the code does. The rule bounds re-reporting rather than eliminating it — a resumed conversation still holds earlier rounds' raw lint output, guarded only by the prompt — and what resuming saves is re-establishing the session, not the instruction head, which is re-sent every turn.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The review loop's lint gate minted a fresh conversation on every round.
lint()summarised throughagent…autonomous.run, which mintsSessionId.freshper call, so each round re-paid the full cold prefix(~32k prompt tokens) to produce a few hundred output tokens.
Rebuilding a prefix costs roughly what ~20 cache reads of it cost, so an
agent that re-examines the same material within one stage should resume;
one whose material changes between calls should not. The lint gate is the
first kind — same commands, same working tree, several times per stage.
What changed
lintgains an overload taking aChat— the summariser conversation —alongside the existing agent-taking signature, which is unchanged and now
delegates through a single-use chat.
Lint.summarisermints it; the parameter type'sprivate[review]constructormakes that factory the only source, so the read-only restriction it applies
cannot be bypassed by passing a bare
Chat.ReviewLoopStatecarries that conversation across rounds. It is minted onthe collecting thread, not in the
CheckedParfan-out, andConfiguredresolution stays at loop entry.
The stale-findings risk, and why the obvious defence wasn't enough
A resumed lint session can re-report an earlier round's findings from memory.
The first version of this change leaned on
lint()'s existing short-circuit:a round whose commands are all silent and exit zero returns
ReviewResult.emptywithout a turn.Review showed that defence is largely inert. The short-circuit needs the
command to be completely silent on success, and most real lint commands
print — this repo's own gate is
lint = sbt compile, which writes[success] Total time: …. So the reused conversation would in practice havebeen consulted every round, holding every earlier round's findings.
A repeated finding is not free: it re-enters
round.issues, triggers a coderfix turn, the fixer finds nothing to fix, and the loop exits through
"Fixer reported no fixes" with a phantom entry in the returned
IgnoredIssues— on a round that would otherwise have been clean.The guard is therefore in the code, not the prompt: the conversation carries
forward only while the summariser has reported nothing. Any round in which it
reports anything — gate-admitted or not, since the gate decides what reaches
the fixer, not what the conversation remembers — drops it, so the next round
starts fresh. A conversation that has only ever said "nothing actionable" holds
no finding to repeat, and the round right after a finding, where the risk is
highest, always runs cold. Clean-but-noisy rounds are the common case, so most
of the saving survives.
This bounds re-reporting rather than eliminating it. A resumed conversation
still holds every earlier round's raw lint output and could newly derive from
it a finding it previously declined to make; only the summariser prompt ("the
blocks are this run's output and supersede any earlier ones") speaks to that.
By construction the retained output is output the model already judged
non-actionable.
Warmth applied, not assumed
The other repeat one-shots were enumerated from
ManifestSession.kind = "oneShot"records and each judged by the same rule. All stay cold:ReviewerSelector.agentDriven)prepareruns once; there is no second call to resume intoFlow.defaultCommitMessage)BranchNamingStrategy)summarisePrStackDiscovery.discover)Value
Small and known to be small: the measured baseline ran 8 lint sessions
totalling $0.22, so the ceiling is ~$0.15/run, and the retirement rule gives
some of that back. This is for the cold-start hygiene and the pattern, not
the money.
Tests
conversation is re-minted each round;
rule is dropped;
the gate-rejected half of the predicate is dropped.
Each mutation-checked, and each fails only for its own mutation. Substituting
a write-enabled conversation for the summariser is a compile error.
CcNegativeCompileTestandInStageNegativeTeststill pass, so thecapture/separation-checking rejections are intact.