feat(compaction): compact mid-invocation once the prompt crosses a threshold

[baptmont]

Third of five. Re-cut: this PR previously showed code that later PRs
corrected, and its base moved from compaction-03-telemetry (now closed) to
compaction-02-sliding-window.

What this adds

Tail retention: the strategy that actually bounds prompt size.

The sliding window in #1232 replaces each group of invocations with one summary,
and summaries are never re-summarized, so it reduces prompt size by a constant
factor rather than bounding it. Tail retention seeds each new window with the
previous summary, so history stays one rolling summary plus a raw tail however
long the conversation runs.

It runs inside an invocation, before a model call, once the prompt passes
TokenThreshold. Running mid-turn is what lets it catch a single long
tool-calling turn that inflates the prompt on its own, which a post-invocation
strategy cannot see until the turn is over.

Two things worth reviewing closely

The live turn's question is held back. Summarizing it means summarizing the
instruction currently being carried out, and EventRetentionSize cannot protect
it, because it counts events and a turn is not a fixed number of them: one tool
round costs two. Only that one event is held back, so a long tool loop can still
compact its own traffic.

A blocked head does not stall it. One unanswered call among parallel
siblings used to stop the strategy for the rest of the session, silently, since
"no window" and "nothing to do yet" are both nil. The scan now resumes wherever
the set of open obligations changed, not only where it grew.

Enable this or the sliding window, not both

They share a candidate rule. Tail retention summarizes the events no compaction
already covers, and the sliding window covers everything it reaches, so with
both enabled tail retention never finds enough uncovered events to fire and the
ceiling never applies. Measured over 160 turns with a 520-character summary:
tail retention alone holds the prompt flat at about 550 characters, the two
together grow it to 41,000.

This is documented rather than rejected in Validate. adk-python starves its
own token-threshold strategy the same way, so it is the shared design, and
AGENTS.md makes adk-python the source of truth for behaviour.

Testing plan

go build, go test -race -count=1 -shuffle=on, golangci-lint run and
go mod tidy -diff clean in both modules, verified at this slice specifically.
Known-unrelated failure: TestConfigureExporters in ./telemetry.

Changed since the last review

Your first finding reproduces, and I was wrong to say otherwise. I reported
that I could not make the rolling summary fail end to end and asked for your
scenario. My probes never built it: the live-head exclusion only lands inside
the record's range when the held-back question has events after it, and a plain
text turn puts the question last. Add a prior compaction and a tool-calling turn
and it happens exactly as described. Fixed by coversAllOf on #1232, and
TestSelectTailRetentionWindowSeedsWhileHoldingBackTheLiveHead is the combined
hold-back-plus-seed case that was missing.

The race guard gets a real before-state. KnownEventIDs captures identities
before the model call and RangeRacedSince compares against those, rather than
against a live handle that already contains whatever arrived during it.

The progress gate took three attempts to get right, which is worth knowing.
Closing it only in the Finish callback reached neither failure that actually
happens, so a failing summarizer still cost 29 summarizer calls across a
30-round tool loop; it is 1 now. Then treating a discard as a failure was worse
than the bug it replaced, because a competing compaction landing is benign and
nothing can reopen a gate closed that way. Then closing it on an empty window
took the storm to 0 rather than 1, because an empty window is "not yet" on a
turn that keeps appending. Five subtests pin which outcomes close it.

The gate is keyed per agent, by name, branch and isolation scope. Branch
alone gave a loop agent's worker and critic one gate, which is the case the
scoping exists for: only a parallel agent gets a distinct branch.

Tail-retention summaries pass through SanitizeSummary. They still do not
run the plugin pipeline, and that now looks correct rather than pending:
adk-python runs no plugin hook on either compaction path, so this matches it and
it is the sliding-window path that goes further.

Still open, and deliberately: window selection is linear in session length now
rather than quadratic, but the mid-turn path still walks the session before each
model call.

Notes for an automated reviewer

Review this exact commit: 982b3e5c63fde3b80facb7bcc2777a16b18e38b4. Check out the SHA, not a branch
name. Several branches in this repository have similar names and hold older
versions of this same code: baptmont/compaction-03-telemetry (ff5b84bb)
and baptmont/compaction-07-review-fixes (888ee5c1) belong to closed PRs
from an earlier shape of the stack, and the pre-re-cut tips are still reachable.
Two agents auditing this stack have already spent a full run on the wrong tree
and reported bugs that no longer exist. If internal/compactioninternal has no
coverIndex and no RepairAfterAppend, you are on the wrong commit.

Deliberate decisions, so they are not re-reported as defects. Each is
documented at the code:

• HasUsableSummary is only a nil check. A record whose content holds no prose
  still covers its range and deletes those events. The stricter check was
  written and reverted: adk-python checks only is None, this stack tracks it,
  and nothing here can produce such a record.
• Tail-retention summaries do not run the plugin pipeline. adk-python runs no
  plugin hook on either compaction path, so this matches it.
• The repair after the append is one round. A straggler landing during the
  repair would need another, and each round strictly shrinks what the record
  covers on both axes -- the timestamp range and the stream position. It shrinks
  only the range if the positional guard is taken from the surviving record, and
  that was a real defect, fixed in round 6 by foldCorrections.
• session/database orders ties by id, which is stable but arbitrary. True
  insertion order needs a sequence column, which Vertex AI would not carry.
• The sliding window and tail retention do not compose; enabling both is
  documented as leaving the prompt unbounded, and adk-python behaves the same.

Known flakes, both unrelated to this stack and both passing on re-run:
TestDelegation_04_ChatToTwoSingleTurnParallel (issue #1313) and an occasional
golangci-lint runner error reporting no export data for github.com/google/uuid.

Verification: build, go test -race -count=1 -shuffle=on,
golangci-lint run and go mod tidy -diff are clean at this commit in both
modules, and CI runs on this stack now.

Decisions taken since round 3, so they are not re-reported

Each is documented at the code. Raise them if you disagree, but they are choices
rather than oversights:

• Parallel fan-out costs 8x on tail retention. Eight children each pass the
  gate and seven summaries are discarded. The output and the prompt are correct;
  it is model spend. Making siblings share a gate walks back the per-agent
  scoping that fixed the loop-agent case, so it stands.
• session/database breaks a timestamp tie on id DESC. Two random UUIDs,
  which is not conversational order. adk-python orders identically, and after
  the repair fix nothing correctness-critical depends on tie order.
• The sliding-window path runs the plugin pipeline and tail retention does
  not. adk-python runs no plugin hook on either, so the first is a deliberate
  divergence in the safe direction and the second matches the reference. The gap
  is documented on the exported surface, at compaction.Config.TokenThreshold.
• Two records over an identical range are resolved by stream position. This
  cannot tell a re-summarization from two independent summaries that happen to
  span one instant, because the record stores holes rather than membership.
  Preferring the later one keeps the documented property that a re-summarized
  range never appears twice, at the cost of the rarer case. Holes are unioned
  across records that share a range and the same summary text, which is what a
  correction is, so a correction cannot lose its straggler to an ordering tie.
• TestNoEventIsDroppedWithoutHavingBeenSummarized is breadth, not a
  substitute for the targeted tests. It found a real unsoundness during
  development, and it does not currently generate the shapes that reproduce the
  tie-safe cut, the hole union or the content check. Its comment says so.
• The two raw_event Vertex recordings pin a whole session.Event JSON dump
  and are brittle to any new field on that struct. Out of scope here: not
  compaction code.

Round 6 — self-review before re-requesting review

Four independent reviewers were run over the stack before asking for another
round. Every fix below is mutation-tested: the mutation that reproduces the
original defect is named in the test's doc comment.

Two defects that deleted conversation.

• copyAny returned any payload that was not map[string]any/[]any
  untouched. A tool payload here is not decoded JSON -- it is whatever a Go
  handler returned, stored as-is -- so loadmemorytool, which stores
  []memory.Entry holding a *genai.Content, handed a summarizer live pointers
  into stored history. Now a depth-bounded reflective deep copy.
• The repair pass covered an event it never saw. A correction shrinks coverage
  on the timestamp axis and moves forward on the stream axis; the positional
  guard used the survivor's position, so an event appended between a record and
  its correction was outside the original's reach and inside the survivor's.
  Holes were already folded across the correction group and position was not;
  foldCorrections now folds both, taking the group's earliest position.

Four more. newCoverIndex indexed contentless records that prompt assembly
skips, leaving events permanently raw and permanently uncompactable at once;
tail retention drew its window from the whole session while the threshold that
triggered it was scope-filtered, so a sub-agent could spend its one compaction on
a sibling branch; token usage was dropped on the error path, so the most
expensive failure mode reported zero spend; and the corrective write ran on the
caller's context, so a cancellation between the two appends left a record
claiming a range nothing had summarized.

Four tests that could not fail were found and strengthened, including the
alias guard itself, which compared only top-level genai.Part pointers and so
was blind to a fresh Blob holding the original's byte slice.

Known to remain. When the only uncovered candidates ahead of the retained
tail sit inside the previous range -- what a repair manufactures -- the rolling
seed can produce a range strictly narrower than the record it replaces, so
neither subsumes the other and both summaries materialize in one prompt. It is
self-limiting and costs a wasted compaction rather than data. Left deliberately:
both candidate fixes carry hole-inheritance subtleties whose failure mode is
deleting conversation, so it wants its own change and its own review.

[karolpiotrowicz]

Mid-invocation tail retention is the right idea and the token gate is a
reasonable trigger, but the default configuration currently removes the user's
own question, and two failure paths go silent.

1. EventRetentionSize: 0 is the default and it swallows the live question.
resolveCompactionConfig fills only Summarizer, so the minimal config that
turns the feature on — &compaction.Config{TokenThreshold: N} — leaves retention
at zero. Validate() accepts it (while rejecting the mirror image at
compaction.go:117), and selectTailRetentionWindow(events, 0) then compacts
the in-flight user turn away. Either default it to a non-zero value or make
Validate() reject the pair.

2. A summarizer error wrapping context.Canceled disappears. The processor
yields the error into the flow (compaction_processor.go:55) and the scheduler
unwraps it to NodeCancelled (scheduler.go:786-789), so the turn ends with no
answer and no error. The identical error surfaces correctly from the
post-invocation pass, which is a useful contrast for the fix.

3. Tail retention breaks every ModeSingleTurn delegation. The processor
appends to ctx.Session(), which for a delegated sub-agent is
*llmagent.wrappedSession (llm_agent_wrapper.go:109) — rejected by every
session backend. It surfaces as a tool error, so the caller sees zero compaction
errors and a confusing failure instead.

One design point worth a second look: there is no cooldown, no
progress requirement and no per-invocation cap, so the processor re-fires ahead
of nearly every model call in a tool loop. Measured 1.46×–1.85× the model calls
in that shape — and at retention=2 the token signal is pinned across all 12
calls, so compaction fires 11 times while being structurally unable to reduce
anything. A cooldown plus a "did the last pass actually shrink the prompt" gate
would bound it.

Smaller: an event stamped exactly at the previous compaction's EndTimestamp is
skipped by tail_retention.go:160 and then deleted by the range, so it is lost;
and the same-timestamp back-off at :174-181 runs before the
longestSelfContainedPrefix trim at :184, so the trim can still split a group
and orphan a function response.

---

Additional findings

[minor] Compaction summaries carry no IsolationScope, so compaction is a paid-for no-op for scoped agents. stamp (internal/compactioninternal/compactor.go:121-133) sets only ID, InvocationID and Timestamp, while internal/llminternal/contents_processor.go:106 filters on an exact IsolationScope match before Apply runs, so the summary never reaches the prompt: a scoped agent's prompt still contains all four raw events, whereas a control summary carrying scope1 collapses them as intended. There is no event loss — Apply runs on the already-filtered list (contents_processor.go:120), which no longer contains the summary, so it drops nothing — the cost is that the summarizer runs, is billed and writes an event while the prompt never shrinks, for scoped agents. This also predates this change; the sliding-window path uses the same unfiltered collection and the same stamp. Populating Branch and IsolationScope on the summary event from the invocation context (in stamp or in CompactionRequestProcessor) would fix it.

[minor] Tail retention is silently inert on RunLive, on a rationale the live path contradicts. The comment at runner/runner.go:555-559 explains the omission by saying a live session "streams over a persistent connection instead of re-sending assembled history each turn", but the live path does assemble the full history through the same request-processor chain (internal/llminternal/base_flow.go:301) and send it with SendHistory (:380), re-sending on every resumption inside the reconnect loop at :340 — so history size does matter there. Meanwhile Runner.New accepts TokenThreshold for a live-only runner with no error, no warning and nothing in the public docs, so a user who configures the safety net gets neither compaction nor a signal. Opting the live path out may well be right; the stated reason is the part to correct, since it is the justification a future maintainer will rely on. Either wire the runtime into RunLive or reject/warn on a tail-retention config for a live-only runner, and document on Config.TokenThreshold which serving surfaces run tail retention. (Growth on a real live session was not demonstrated — that needs a live model connection — so this rests on reading the live path.)

[minor] The public config surface omits the facts users need, and the Config doc contradicts Validate(). The exported TokenThreshold doc (session/compaction/compaction.go:66-69) states only when compaction fires; the three material facts — the count comes from the previous response, the fallback is len(text)/4 over text parts only, and the comparison is against the total prompt — live in unexported internals (internal/compactioninternal/tail_retention.go:81-90, :106-118), and the repository carries no markdown guidance for choosing a value. Separately, compaction.go:47-48 says enabling neither strategy "disables compaction entirely" while Validate() at :120-122 rejects exactly that config and runner.New fails hard on &compaction.Config{}. This is documentation with teeth: the retention-zero config and the accepted-but-inert config both read as legal from the field docs. State the units and the lagging-signal caveat on the exported field, give a rule of thumb relative to the model's context window, note that a tool round costs two events (so EventRetentionSize: 2 retains half an exchange), and reword the Config doc to something like "a non-nil Config must enable at least one strategy; use a nil Config to disable compaction".

[minor] The compaction runtime survives onto the context handed to a user-supplied Summarizer, with no depth guard. internal/compactioninternal/compactor.go:102 passes the invocation context to cfg.Summarizer.SummarizeEvents unchanged. The default path is safe — LLMSummarizer calls s.model.GenerateContent directly (session/compaction/llm_summarizer.go:122), bypassing Flow, so CompactionRequestProcessor is never re-entered — but Summarizer is a documented public extension point (session/compaction/compaction.go:126-136), and a summarizer that runs an agent or flow on the given context would re-enter compaction on the same session. Nothing bounds the recursion, since every level sees the same over-threshold session. Strip the runtime before calling the summarizer (ctx = compactionctx.ToContext(ctx, nil)), or carry a depth guard on the runtime.

[karolpiotrowicz]

[blocker] retentionSize > 0 means the zero value retains nothing, and Validate() accepts it: it rejects the mirror mistake (EventRetentionSize > 0 && TokenThreshold == 0) but blesses &compaction.Config{TokenThreshold: 1000}, and resolveCompactionConfig (runner/runner.go:149) fills in only Summarizer — yet zero is exactly what a user writes when they only care about the token threshold. The whole history then becomes the window, including the user message the runner appended moments earlier:

turn-2 prompt: [model] S1
               [user] Continue processing previous requests as instructed...

The live question is gone and the model is asked to continue without it. The same run with EventRetentionSize: 1 keeps WHAT-IS-MY-LIVE-QUESTION in the prompt, isolating the zero value as the cause. The same defect appears at any EventRetentionSize below the current turn's event count — a tool round costs two events, so EventRetentionSize: 2 fails on the first tool call.

Reject TokenThreshold > 0 && EventRetentionSize == 0 in Validate(), symmetric with the existing check, or default it in resolveCompactionConfig; independently, selectTailRetentionWindow should never summarize events from the current invocation (ev.InvocationID == ctx.InvocationID()), which also covers the EventRetentionSize: 2 variant.

[baptmont]

Fixed. Validate now rejects a token threshold with a zero retention size, and the field doc says why.

Confirmed against the reference while checking: adk-python does the same thing, if event_retention_size == 0: events_to_compact = candidate_events, so the window extends to the newest event and the turn in progress is summarized out of its own prompt. Shared rather than Go-only, and with no legitimate use in either, so Go rejects it rather than waiting. Recorded in the cross-language notes so the Python side can pick it up.

[baptmont]

Thank you for this. It is the most useful review on the stack, and it caught something I had got wrong rather than merely missed: I had described the hardening added in #1232 as if it were global, when it only ever applied to the post-invocation sliding-window path. This PR's mid-turn path had unfixed twins of the same defects. I have posted a correction on #1232.

Pushed now, with a regression test for each, every one confirmed to fail against the previous behaviour:

• 3701645682 (blocker) appending through a session wrapper
• 3701645681 (blocker) the scheduler swallowing a cancelled-summarizer error
• 3701645712 (major) no re-read or compare before recording a summary
• 3701645734 (minor) no ctx.Err() gates
• 3701645719 (minor) window not trimmed to one branch and scope

internal/llminternal/compaction_processor_test.go is new. The file had no tests at all, which is how three of the five survived, so 3701645724 is half closed too.

3701645697 was already fixed by 8e8b1ddf on #1231, after you wrote it.

Still open and triaged, not forgotten: the two trigger blockers (3701645675 no cooldown or per-invocation cap, 3701645677 EventRetentionSize: 0 validating), the tie at the previous-compaction boundary (3701645693), the partial-overlap ranges (3701645704), the stale token signal (3701645708, 3701645686), the mid-turn abort after side effects (3701645710), the silent decline (3701645716), and the remaining minors. Several of those are behaviour decisions rather than mechanical fixes, so I would rather agree the semantics with you than guess: in particular what "cannot compact" should do, and whether a per-invocation cap or a cooldown is the right brake.

One thing you did not raise that the triage turned up: the tail path also has no equivalent of skipBlockedHead, so a permanently blocked window head silently stops tail compaction for the rest of the session.

[baptmont]

All nine agreed decisions are implemented and pushed. Summary of what changed and where, plus the things I found along the way that were not in the plan.

Landed

Decision	Where	Note
Additive WithOptions siblings	#1235	signatures restored byte for byte
apidiff in CI	#1297, off main	standalone, independent of this stack
gen_ai.system on the span	#1233	semconv values, divergence recorded
Both strategies gated per invocation	#1234	plus a wiring bug the test exposed
EventRetentionSize: 0 rejected	#1234	shared with adk-python
Stale token signal, silent decline	#1234
Prompt-still-fits fallback	#1234	a failed optimisation no longer kills a turn
IsFinalResponse, transcript budget	#1232
Window-cap divergence recorded	notes doc

The #1236 cassette was also re-recorded with a fourth tool-calling turn, so pairing across a summary is exercised for the first time.

Four things worth flagging

The TestConfigureExporters failure I have been reporting for days is already fixed on main. It reproduces at this stack's base and passes on current main, so it is stale rather than open. No issue filed. It will disappear when the stack is rebased onto current main, which is worth doing at some point regardless: main has moved on, including #1252 changing Event JSON encoding. I checked that one specifically, and EventActions.MarshalJSON embeds an alias precisely so new fields are not dropped, so Actions.Compaction will serialize correctly after the merge.

The A2A cleanup hang is filed as #1298, with the evidence that it hits both remoteagent and remoteagent/v2 at 31 to 32 seconds against a 2 second norm, which points at one shared timeout rather than two test problems.

Three of the seven open #1234 items turned out to be shared with adk-python, not Go defects: the zero retention size, the tie at the compaction boundary, and the absent cooldown. That changed what I did with them: the first is rejected in Go because it has no legitimate use anywhere, the other two are recorded in the cross-language notes rather than diverged on unilaterally.

Two of my own tests were vacuous and I only caught them by mutating the source. The both-strategies test passed whether or not the gate existed, because the configuration never made the two strategies want the same turn. And an assertion I added last session checked for the absence of gen_ai.compaction.interval when the real key is compaction_interval, so it could never have failed. Both are fixed and now fail against the behaviour they are meant to catch. I mention it because it is the failure mode this review has repeatedly punished, and self-review did not catch either one; mutation did.

Still open

Deliberately not done, with reasoning in the replies: the partially overlapping ranges from the two producers, which decision 4 largely prevents and which is better judged after it lands; the coverage model, left timestamp-based to match the reference; and recovered-call ordering, which interacts with the stream ordering changed on #1232.

Every branch was verified independently: build, -race -shuffle=on, lint and tidy, in both modules.

[baptmont]

Ready for another look. Before asking, I went back over every inline comment on #1231 to #1236 and checked the code rather than trusting my own earlier replies. Two things came out of that.

50 of your comments never got an inline answer. On #1232 I replied to all 19 in a commit message, which from your side looks identical to being ignored. Same for most of #1231. Every one of those now has a reply on the thread itself.

One thing I reported as fixed was not. The claim that compaction "keeps prompts small" is wrong, because the sliding window never re-summarizes a summary. I said I had corrected the wording. I had corrected it in one place, and the original had already been copied into three more, one of which my own search missed because it wraps across two lines. Fixed in all four now.

Thread state

Threads are resolved only where the fix is real and I can point at it. Anything partly done or not done is left open on purpose, including four on #1231 that were marked resolved earlier but should not have been.

PR	Resolved	Left open
#1231	16	4
#1232	13	6
#1233	8	7
#1234	8	13
#1235	3	5
#1236	4	3

So: 52 done, 38 still open. The open ones split into things that need a decision from you and things that just need doing, and each thread says which.

The larger open questions, if you would rather answer them here than thread by thread: whether coverage should be keyed on event identity instead of timestamps, which closes two defects at once but changes the stored shape and diverges from the Python version; whether summaries should go through the plugin pipeline, which the Python version effectively does and this does not; and whether the mid-turn strategy needs a cooldown, which neither implementation has today.

State of the code

Every branch passes build, race tests with shuffling, lint and tidy in both modules. I dispatched the Go workflow manually to confirm that, because these pull requests target the integration branch and so get no automated checks beyond the CLA. That gap is worth fixing separately.

Also opened #1297, which adds an API compatibility check, since nothing in CI would have caught the signature break you found here.

[karolpiotrowicz]

Reviewed at 7d4bdde. The doc-only commits pushed since then do not change any line cited below.

This revision adds four mechanisms: the progress gate, the session unwrap, the race check and the deliberate degradation. Three of them have defects the suite cannot see, and two make the feature worse than not having it.

1. AllowAt implements the opposite of its own doc for a grown prompt. The comment two lines above says "A prompt that has actually shrunk, or grown past where it was, is allowed through"; the code is last == 0 || int64(tokens) < last , which admits only a shrink. Once a compaction happens at N tokens, tail retention is refused for the rest of the invocation however large the prompt gets: AllowAt(5100) and AllowAt(50000) after RecordAt(5000) are both false. In a tool loop this is not theoretical — I measured a single invocation reaching 45,056 tokens against a 2,000 threshold, after compaction had visibly shrunk it twice. The same loop with the gate removed peaks at 10,012 for four extra summarizer calls. One value cannot express both "the last compaction did not help" and "it helped and the turn grew again".

Worth knowing before fixing: because promptTokenCount still returns the pre-compaction usage reading on the call immediately after a compaction, the next AllowAt sees tokens == last and declines anyway, so changing < to <= buys exactly one extra firing.

2. The rolling seed backdates a coverage range across the retained tail. Candidates are taken by stream position after the previous compaction record, which is appended after the raw tail it deliberately did not cover; the seed then starts the new range back at the previous range's start. Covered and summarized diverge. Six ordinary turns through the real runner at TokenThreshold: 1000, EventRetentionSize: 2 silently dropped four stored events from every later prompt — they were handed to no summarizer and appear in no summary. This is the coverage-model issue written up on #1231, and it is the sharpest instance of it, because it needs no ties, no concurrency and no unusual configuration.

3. The live turn is summarized out of its own prompt at every retention size Validate() accepts. The new branch closes EventRetentionSize == 0, but the defect is a count-vs-turn mismatch, not a zero: a tool round costs two events. At retention 1 and 2, four of six second-turn prompts lost the user's question; at retention 3, two of six. Excluding the current invocation's events is what actually holds.

4. RecordAt fires before the summarizer runs and is never rolled back. Here , so one transient failure disarms compaction for the whole invocation with nothing stored: a summarizer that 503s once and would then succeed produced one attempt, zero stored events, and a prompt that reached 15,063 against a 1,000 threshold. Combined with 1 and with the degraded error path this is self-concealing — the gate stops retrying and degrade stops reporting.

A few others. RangeRaced builds its "already known" set from the live session handle at check time, and parallelagent sub-agents share that handle, so a sibling's append during the model call reads as pre-existing; its compaction-event branch cannot fire at all, since a concurrently appended record is stamped after everything it covers. selectTailRetentionWindow has no skipBlockedHead fallback, so one unanswered tool call stalls tail retention for the rest of the session — I had 38 compactable events behind one. promptTokenCount scans the whole session with no branch or isolation-scope filter, so a sub-agent whose own prompt is a couple of tokens saw 200,000. The delta estimator counts only part.Text, which is exactly what a tool loop does not grow by — 400,000 characters of function response moved the estimate by nothing. Mid-turn summaries skip RunOnEventCallback while the post-invocation pass runs it. UnwrapSession is an unbounded loop with no cycle guard on the per-model-call path. And Validate() now rejects a TokenThreshold-only config that the previous revision accepted, which apidiff cannot see — defaulting would be the non-breaking choice.

Two doc fixes while you are in there: TailRetention's doc block runs into ProgressGate's with no blank line, so go doc TailRetention prints nothing; and compactionFailure's rationale describes the error being yielded into the flow's error channel, but its only caller is the log.Printf in degrade.

[karolpiotrowicz]

Duplicate of the review above — posted three times by mistake. Please read the first copy; this one has no additional content.

[karolpiotrowicz]

RangeRaced is properly fixed — 8-way parallel fan-out with a barrier now stores 1 record and discards 7, against 8 stored and 28 overlapping pairs before. The gate re-fire is fixed too: the same 30-round tool loop that made 29 summarizer calls now makes 1. Four things left, one of which I would want a doc line for before this ships.

Tail-retention summaries still never reach the plugin pipeline. SanitizeSummary now has a live second call site at compaction_processor.go:128, which closes half of it, but the plugins themselves do not run on this path. With a redaction plugin registered: 1 summary stored, plugin invoked 0 times, the planted secret reaching the session unredacted. The sliding-window path calls RunOnEventCallback and this one does not. The in-tree comment is honest about the gap, but nothing on the exported surface tells someone configuring tail retention alongside a redaction plugin that the plugin will not see these events, and the failure is silent.

The retry fix went one step too far. compactionctx.go:127 gates on !st.failed, :144 sets it on a failure or a decline, and :155 clears only the token marker, so the first failure latches compaction off for the rest of the invocation. Same test on both revisions, one invocation with a 40-round tool loop and a summarizer failing once: before, 2 calls and 1 record stored; now, 1 call and 0 records. Nothing is lost and the post-invocation pass still runs, so this is a worse trade rather than a defect — but it swaps "retry forever" for "never retry" when the useful answer is two or three attempts per invocation. Clearing failed in Recovered is not the fix, since Recovered only fires below the threshold and the prompt does not drop after a failure.

The gate is keyed per agent now (:94), which fixes the loop-agent case. The side effect is that 8 parallel children each pass the check and each call the summarizer, and the race guard then discards 7 of the 8 results. The stored outcome is right and the prompt is right — it is 8× the model spend inside one turn.

Small one: the doc comments at compactionctx.go:245 sit under const runtimeCtxKey and describe AllowAt, with a [Runtime.Recovered] link that does not resolve.

[baptmont]

Correcting something I told you on three PRs: the two strategies do compose. I said they did not, and that was an over-generalisation from a single configuration.

What I originally measured was CompactionInterval: 2 with EventRetentionSize: 10, which is what the shipped example used. Tail retention never fires there, the prompt grows, and that part was real. I then wrote it up as a property of the design rather than of those numbers, without sweeping the parameters. Driving the real runner over 60 turns:

CompactionInterval	EventRetentionSize	summarizer calls	final prompt
2	10	30	13,935 chars, growing
3	10	20	9,147 chars, growing
10	10	45	543 chars, bounded
20	2	41	495 chars, bounded
off	10	41	543 chars, bounded

At interval 2 and 3 the call count is exactly the sliding-window count, so tail retention contributes nothing. At interval 10 and up it exceeds it and the prompt is bounded: both strategies running, correctly.

The real rule is that tail retention fires only when more events accumulate between sliding-window compactions than EventRetentionSize holds back. A short interval keeps that number small, so a large retention size starves it.

Nothing in the code changed as a result, which is the one piece of luck here: I deliberately did not add a Validate rejection when I believed they did not compose, on the grounds that refusing a well-formed config would break existing setups. So the mistake never reached behaviour. What changed is documentation: the package doc now carries the table above instead of "enable one, not both", the Config and the five duplicated field docs are corrected, and the example says the interval is why tail retention would not fire there rather than claiming the combination is invalid.

There is also a divergence worth recording, which I have not chased down. adk-python's selector returns a usable window at interval 2 with retention 2, and Go's does not: Go excludes events covered by any later record, where Python uses a simple cutoff after the latest record's end, so Go starves across a wider band. My two measurements were not like-for-like (Python's selector called directly, Go end to end through the runner), so I would want to redo that before calling it exact.

[karolpiotrowicz]

The bounded-retry change is the right shape and I verified it through Runner.Run — details on #1232, where the gate lives. trimToTimestampBoundary also checks out now: tieSafeCut and the balance check sit at the same index, so a returned prefix always ends where the open-obligation set was empty, and the two other routes I chased (the backward walk in tail retention, and skipBlockedHead) cannot reintroduce an unbalanced window.

One small thing here. compactionctx_test.go:182 reads:

// The worker is untouched by any of it.
if !worker.AllowAt(2500) {
	_ = worker
}

There is no t.Error in the branch, so it cannot fail whichever way AllowAt answers, and the cross-agent isolation the comment describes is unpinned. _ = worker also keeps the branch non-empty, so staticcheck stays quiet about it.

The property is worth pinning for real, because per-agent scoping of the failure counter is exactly what stops one sub-agent's failures suppressing another's compaction. Note the worker already recorded a compaction earlier in the test, so AllowAt returns false for it on its own account — asserting through worker cannot express the property. A third gate that never recorded would.

Same scope note as on #1232: I have not finished reviewing the rest of the stack this round, so this is not a complete pass.

[karolpiotrowicz]

The replacement is right. compactionctx_test.go:185-188 asserts through a third gate that has neither compacted nor failed, which is the only way to express the property — and the comment says why the worker could not carry it. Keying every gate to one scope now fails the test, which is the check that matters.

Verified at 9f5cb80: build, the full suite and golangci-lint clean.

[karolpiotrowicz]

Nothing in this PR's own diff needs changing. Build, the full suite, go vet, gofmt and golangci-lint are all clean at the current head (9f5cb80), and the bystander gate you replaced the no-op assertion with does express the property — keying every gate to one scope fails it, which is the check that matters.

One thing to carry, and it is not yours to fix here. tail_retention.go:389 is the third consumer of coversAfter, and I have left a finding on #1232 about that function reading superseded compaction records. The effect here is that a straggler rescued by RepairAfterAppend is skipped by tail-retention window selection and can never be summarized. Tail retention is the strategy an application turns on when it needs a bound, so this path is where the symptom actually costs something, even though both newCoverIndex and coversAfter live in #1232. Worth re-checking this window once the fix lands upstream.

Also worth a look before this lands, and I could not settle it either way: whether LatestCompactionEvent can select a sibling branch's record and strand a branch's own earlier summary. tail_retention.go:463-468 drops the seed on a branch mismatch, which is right, but I did not establish what happens to the earlier same-branch summary in that case. You will know in a minute whether that is reachable.

The mid-turn repair you added while fixing the round-6 findings is the more valuable half of that change, and the plugin-pipeline gap you documented at compaction_processor.go:124-127 matches Kotlin — neither SlidingWindowEventCompactor.kt nor TokenThresholdEventCompactor.kt runs a plugin hook, and CompactionRequestProcessor.kt has none at all. Both your comments justify it against adk-python, which is not this stack's bar, so the conclusion is right for a reason the comments do not give.

[baptmont]

Nothing here was on your list; this is a self-review round before I ask you to
look again. Three things landed in this slice.

Tail retention drew its window from the wrong conversation.
promptTokenCount filters by the turn's branch and isolation scope, and its
comment gives the reason — a sub-agent whose own prompt is a couple of tokens
read its parent's 200,000. The candidate list did not apply that filter, so the
threshold was evaluated against one conversation and the window chosen from
another. A sub-agent could spend its one allowed compaction summarizing a
sibling branch, its own prompt would not shrink by a token, and the progress
gate would then stand down for the rest of an over-threshold turn — the strategy
that bounds growth, switched off by having compacted the wrong thing. Same
filter on both now.

The corrective write no longer dies with the caller. Storing a summary and
correcting it is two appends, and in between the stored record claims a range
wider than what it summarized. Both repair paths ran on the caller's context, so
a cancellation in that gap — a user hanging up mid-turn — left the claim
standing permanently. RepairContext detaches from cancellation and bounds the
write, following auth/providers.go.

A panicking summarizer could take the process down. summarizeTraced
re-panics deliberately so the failure stays visible on the span, and nothing
recovered it on the post-invocation path, which runs from a defer after the
answer has already shipped. The recover went into compactAfterInvocation
rather than its callers: there are two of them, one per entry point, and they
are copies, so fixing the one I found first would have missed the other.

One I have not fixed, deliberately. When the only uncovered candidates ahead
of the retained tail sit inside the previous range — which is what a repair
manufactures — the rolling seed produces a range strictly narrower than the
record it replaces. Neither subsumes the other, both materialize, and the prompt
after compacting is larger than before it, with the invocation's one allowed
compaction spent. It is self-limiting and costs a wasted call rather than data.
Both fixes I can see carry hole-inheritance requirements whose failure mode is
deleting conversation, so I would rather it were its own change with its own
review than a late edit in a round that already touches a lot. Happy to be told
otherwise.

Review 0e65c8ae.

[karolpiotrowicz]

The three you found here all check out, and the placement decisions are the ones I would want. RepairContext is the right idiom — context.WithTimeout(context.WithoutCancel(ctx), repairTimeout) keeps the caller's values, drops its cancellation, and bounds the write — and both call sites take it with defer cancel(), so the two-phase window really is closed on both paths rather than the one you happened to find first. Same for putting the panic recovery in compactAfterInvocation rather than in its two copied callers.

The candidate list not applying promptTokenCount's branch and scope filter is the one I would have most regretted missing. A sub-agent spending its one allowed compaction on a sibling branch, not shrinking its own prompt by a token, and then standing the progress gate down for the rest of an over-threshold turn is the bound switching itself off, and it would have looked like tail retention simply not working.

Deferring the narrower-seed case is the right call. It costs a wasted model call rather than data, it is self-limiting, and both available fixes carry hole-inheritance requirements whose failure mode is deleting conversation — which is not what you want as a late edit in a round that already rewrites the copy path and the cover index. Worth a tracked follow-up so it does not get lost.

Build, the full suite over 157 packages, go vet, gofmt and -race are all clean at 0e65c8ae. golangci-lint is not: it reports the same two reflect.Ptr findings in compactor_test.go that #1232 carries, since the file arrives through the base. Fixing it there fixes it here.

I have left one finding on #1232 that reaches this PR too — the subsumed-record guard in newCoverIndex is not pinned by any test, and tail retention is the consumer where it matters most.

[karolpiotrowicz]

Approving at 0e65c8a. Nothing in this PR's own diff needed changing, and the three defects you found here yourself are the ones that mattered: the candidate list not applying promptTokenCount's branch and scope filter would have looked like tail retention simply not working, rather than like the bound switching itself off.

Carrying forward from #1232 rather than holding this: the subsumption guard in newCoverIndex is not pinned by any test, and tail retention is the consumer where that matters most. The narrower-seed case you deferred is the right call to defer.

[baptmont]

Rebased on the #1232 fixes at 3b722e34; nothing in this PR's own diff changed.

Both follow-ups you carried forward are addressed there: reflect.Ptr is now reflect.Pointer, which clears this PR too since the file arrives through the base, and the subsumption guard in newCoverIndex is pinned by a test that fails under your mutation.

One correction, detailed on #1232: the case you described for the guard — a record subsumed by a strictly wider range with different content — is not reachable, because subsumption requires the subsuming record to sit later in the stream, so it always has both a later positional guard and a superset range. The reachable case is an identical range with different summary text, where subsumption is decided by position without comparing holes while foldCorrections merges holes only across records that share their content. Tail retention is indeed the consumer where it matters, since it is the path that asks selection this question before every model call.

[baptmont]

Rebase, plus the other half of one change from #1232.

ProgressGate.AllowAt() no longer takes a prompt size. The Gate implementation ignored the argument entirely, so the interface advertised a size-aware decision that no implementation made. The size question is already answered before the gate is consulted: the caller only reaches it having found the prompt over the threshold, and calls Recovered as soon as it drops back under. Behaviour is unchanged — this removes a parameter, not a rule.

Recording it because I got this wrong first: I read the gate as an unconditional one-compaction-per-invocation latch and was about to change it. It is not. Recovered() re-arms it whenever the prompt goes back under the threshold, so the real rule is "compact once, then not again until the last one demonstrably worked", which is defensible. Only the dead parameter was wrong.

Shared context for this round. The whole stack was rebased onto current main (3f69c505), absorbing 55 commits. Four conflicts, all from #1252 "give Event a consistent JSON encoding":

• session/session.go — main's new JSON tags, MarshalJSON and nilOrRef merged with our Compaction field and the EventCompaction/EventRef types. One hunk ended mid-function and truncated nilOrRef; gofmt caught it.
• internal/telemetry/logger.go — the only semantic one. Main changed variantToGenAISystem to return *attribute.KeyValue; ours returned *log.KeyValue via a GenAISystemAttr helper we had extracted. Resolved by keeping main's signature and implementing it through our helper, so there is one source of truth rather than two.
• Imports only in session/database/service_test.go and session/vertexai/service_test.go.

Every slice was rebuilt from the rebased tree and verified to build, lint and test on its own. The full suite is clean.