test(llmagent): drive context compaction against a recorded real model

[baptmont]

Last of five. Re-cut: this PR previously showed code that a later PR
corrected. It now stands on its own.

What this adds

The end-to-end test and the worked example.

Every other test in the stack uses a fake summarizer, which is right for pinning
behaviour but says nothing about whether a real model, handed a real transcript
through the real prompt-assembly path, produces something usable. This one runs
the full chain against recorded traffic: turns accumulate, the threshold trips,
a summary is written, and the next prompt carries the summary in place of the
turns it covers while the raw tail stays intact.

The assertions are load-bearing rather than structural. A compaction that
deletes its covered events without substituting anything fails here, and so does
one whose summary never reaches the prompt.

The example arms one strategy

The sliding window, with the tail-retention pair commented out and what to
delete in order to swap. Arming both is what the package documentation warns
against: the sliding window consumes the events tail retention would summarize,
so the ceiling never applies. The example previously armed both and called the
two triggers independent.

The documented command is web webui, not web. The bare form exits with
"no active sublaunchers found".

Recording note

agent/llmagent carries a broad //go:generate go test -httprecord=Test
directive, and -httprecord matches cassette file paths rather than test
names, so the documented package-wide command re-records all 20 cassettes in the
package rather than the one you meant. Filed separately as #1330. To refresh
only this cassette:

go test ./agent/llmagent/ -httprecord='TestCompactionE2E\.httprr' -run TestCompactionE2E

Testing plan

go build, go test -race -count=1 -shuffle=on, golangci-lint run and
go mod tidy -diff clean in both modules. This is the stack tip, and its tree
is byte-identical to the pre-re-cut tip, 734b2d03. Known-unrelated failure:
TestConfigureExporters in ./telemetry.

Changed since the last review

The cassette is re-recorded, which was the blocker on the thought-signature
finding you raised. That fix is in #1232 now: a signature is a handle on one
model's reasoning within one exchange, and a summary goes to a different model
on a different call, where it means nothing and only costs tokens. The traffic
shows it: the first request after a compaction went from 5,208 bytes to 1,883,
and the whole cassette from 45,757 to 37,149. The signatures the agent replays
for its own earlier turns are untouched, because those do go back to the model
that issued them.

Re-scanned for credentials after recording: no API key, no authorization or
bearer token, no quota-project header, and the surviving header set matches the
other cassettes in the directory.

The re-record command works now. compactionModel memoises per test so the
agent and the summarizer share one instance, rather than opening two recorders
that truncate one trace.

The size claim is corrected, and not the way I expected. I added the
assertion the doc promised and it failed: 782 characters before the compaction,
835 after. Not the signature, which is not text and never appeared in that
measurement. Three short turns get replaced by one model-written summary, and a
summary of two one-line turns is longer than the two lines. Compaction pays off
against real history and this fixture is not that, so the doc now says what the
test establishes. A loose bound stays, so something non-prose riding along would
still be caught.

Notes for an automated reviewer

Review this exact commit: 6b9000dd919127d8e3ba8d8bca414191699c5a05. 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]

An end-to-end test against a recorded model is the right instinct for this
feature, and the cassette is clean. But the test does not currently establish
the property it was written for.

The assertion is a golden-bytes lock. httprr keys replay on the full request
body, and the t.Fatalf on a replay miss sits at :156, ahead of every
assertion in the file. So any change to the assembled prompt fails, and they
all fail identically — a one-space edit to an instruction and a genuine
role-alternation break produce the same message at the same line. Running 13
mutations against it: 5 survive, including the wrong-covered-range mutation, both
function-call/response pairing guards, and two store violations. All 8 kills
report at :156, so none of them came from an assertion in the test. The
structural reason is worth stating plainly: with a byte-exact replay gate ahead
of the assertions, the test can only detect "the prompt changed", never "the
prompt is malformed".

The scenario also never exercises the property. Exchange 5 of the cassette is
model(text) | user(text) with zero function parts, so the
call/response-pairing invariant is untested even at record time.

Splitting this in two would give both halves real teeth: an offline test that
asserts structurally on the assembled prompt — role alternation, and every
function response having its matching call — with no cassette in the path; plus
the recorded test extended to include function traffic across a compaction
boundary.

Two things this PR inherits but is the first to compose, so they are worth
flagging here: with compaction disabled, a single planted compaction record
still replaces the whole history, because Apply at
contents_processor.go:122 never consults the config — so the "off by default"
guarantee holds only for sessions that contain no compaction record. And
summaries are never re-summarized: at 40 turns with the shipped config the
prompt carries 19 of them.

---

Additional findings

[major] session/vertexai silently drops Actions.Compaction (session/vertexai/vertexai_client.go, in createAiplatformpbEventActions / eventNeedsRawEvent / aiplatformToSessionEventActions). A compaction summary event round-tripped through the Vertex AI session service loses its Actions.Compaction with no error: createAiplatformpbEventActions returns nil for it, eventNeedsRawEvent returns false so the raw_event fallback is never taken, and the value read back is nil. Compaction on this backend is therefore write-only — the summary is stored as a contentless event and every later prompt is assembled as if no compaction had happened, so the prompt keeps growing on the backend most likely to host long sessions and the failure surfaces far from its cause. This is not a platform limit: serialising the same Actions through the existing raw_event path yields {"compaction":{"compactedContent":…,"startTimestamp":…,"endTimestamp":…}} correctly, and adk-python persists the equivalent through a _compaction custom-metadata key. Adding Compaction != nil to eventNeedsRawEvent is a one-line fix that reuses the working path; session/sessiontestsuite/service_suite.go has no Compaction case at all, so no backend is currently held to this. Raised at stack level — not introduced by this PR.

[major] Summaries accumulate without bound (internal/compactioninternal/apply.go, internal/compactioninternal/window.go). Sliding-window compaction appends one summary per window and never subsumes earlier ones, so the number of summaries in the prompt grows linearly with conversation length. Driving 40 turns with the interval and overlap that examples/compaction/main.go ships puts 19 summaries in a single prompt, and consecutive summaries share a turn, so the same content is re-sent repeatedly. The feature lowers the constant factor but leaves the prompt O(turns), which does not match the premise that compaction keeps the token count from climbing on long sessions. Subsuming a summary whose range is contained in a newer summary's range, or re-summarising accumulated summaries, would bound it; at minimum the growth characteristic deserves to be documented.

[major] Partially overlapping summaries are both materialized (internal/compactioninternal/apply.go). When two summaries overlap partially, Apply emits both, so the events in the overlap are represented twice in the assembled prompt — summaries covering e1..e3 and e2..e5 both materialize, duplicating e2 and e3. This is not an edge case under the shipped configuration: with OverlapSize > 0 — used by examples/compaction/main.go and by this test at :145 — every consecutive pair of summaries overlaps by construction, so it compounds the accumulation above. Suggest dropping any summary whose covered range intersects a later summary's range, or merging the ranges, so each event is represented exactly once.

[major] EventRetentionSize: 0 passes Validate() and swallows the in-flight user question (internal/compactioninternal/tail_retention.go; compaction.Config.Validate()). Setting TokenThreshold while leaving EventRetentionSize at its zero value validates cleanly, and the prompt that results contains the summary but not the question that triggered it — the model is asked to answer something it was never shown, with no error anywhere. The same scenario with EventRetentionSize: 2 keeps the question, isolating the zero value as the cause. Because this is the Go zero value, anyone configuring only TokenThreshold lands on it and Validate() endorses the result. Suggest rejecting EventRetentionSize == 0 when TokenThreshold > 0 (or defaulting it to a sane minimum), and unconditionally exempting the current invocation's events from tail retention.

[major] Three exported constructors are API-incompatible — NewRuntimeAPIController, NewEventarcController and NewPubSubController in server/.../controllers. Each gained a trailing variadic options parameter, which apidiff classifies as Incompatible against main. Ordinary call sites still compile, but function-value and interface-satisfaction uses do not: assigning controllers.NewRuntimeAPIController to a variable of its previous function type is now a compile error. The change arrived in #1235 rather than here (the diff between #1235 and #1236 is API-clean), so this is raised at stack level: either keep the old constructors and add NewXWithOptions variants, or record the exception explicitly in the description with a release-note entry.

[major] No CI runs on any PR in this stack (.github/workflows/go.yml:7-13). The Go workflow triggers only on pull requests targeting main or v1; every PR here targets the previous branch in the stack — this one targets baptmont/compaction-05-serving-surfaces — so the workflow never fires, and gh pr checks 1236 returns cla/google alone. lint (.) and test (.) are required on main, so they will first execute when the whole stack merges, on the combined result. That leaves the Testing Plan as an unverified self-report at review time and makes this PR's "so CI stays hermetic" argument unobservable on the PR arguing for it. Adding the stack's branch prefix to pull_request.branches (or using branches: ['**']) would verify each PR where it is reviewed.

[minor] The feature ships with no user-facing documentation. A new public session/compaction package and a runner.Config.EventsCompactionConfig field land with no prose anywhere in the repository — grep -ril compaction --include='*.md' . returns nothing, and there is no docs/ or website/ tree. The package doc also carries a dangling link: session/compaction/compaction.go:21 references [Apply], which lives in internal/compactioninternal and so cannot resolve from an exported package. Package-level prose covering the two strategies, the config fields and the ordering guarantees would make the feature learnable without reading the source; the [Apply] reference should name an exported symbol or drop the brackets.

[minor] examples/compaction/main.go:25-27 names serving surfaces the launcher does not serve. The doc comment says the launcher serves the console, the web UI, A2A and Agent Engine, but cmd/launcher/full/full.go:32-33 composes console, webui, a2a, pubsub, eventarc and api: agentengine is absent even though cmd/launcher/agentengine/ exists, and the two trigger surfaces that are composed go unmentioned — the least obvious ones, and the ones most worth an example. Given there is no other documentation for the feature, this is the artifact readers will follow. Correct the comment to match full.go, or compose agentengine if the example's claim is the intended behaviour. Introduced by #1235.

[minor] The summarizer's own prompt size pollutes the tail-retention threshold (internal/compactioninternal/tail_retention.go). Token accounting reads the newest response's promptTokenCount, but immediately after a compaction the newest response is the summarizer's, whose prompt is unrelated to the agent's: a 50000-token summarizer prompt is read as the agent's prompt size when the agent's real figure is around 100, so a 1000-token threshold fires when it should not. End to end this moves the summarizer call count in the wrong direction — TokenThreshold=0 gives 3 calls, TokenThreshold=100000 gives 5. It self-corrects once a later response lands, hence minor, but it happens on the turn immediately after every compaction. Skipping compaction-authored events when locating the newest usage record would fix it.

[baptmont]

Triaged all seven against the current code and the committed cassette. All seven stand, and two of them I can now state more strongly than the comment did.

On 3701666664: I decoded the cassette. It holds five exchanges, and exchange 5, the only post-summary prompt, is exactly [model: summary text, user: question] with zero functionCall or functionResponse parts. So recoverCompactedFunctionCalls and longestSelfContainedPrefix are provable no-ops against this recording, and the call-pairing property the test exists to demonstrate is not exercised at all.

On 3701666676: stronger than "there is no second window". The first compaction has firstNew == 0, so max(0, firstNew-overlap) is order[0] regardless of OverlapSize. The setting is inert here by construction, not just unexercised.

Two of these cannot be fixed without recording new live traffic, which needs credentials, so I will bring them back to the maintainer rather than guess. The rest, including the misleading doc comments at :44-50 and :52-54, the silent skip on a missing cassette, and the redundant go:generate, are offline fixes and are on the list.

[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]

The double-open is fixed — a counter on httprr.Open goes from 2 to 1, and the committed cassette is 8 contiguous records to EOF. The ThoughtSignature passthrough is fixed and properly pinned: deleting the strip fails TestNewSummaryEventDropsTheThoughtSignature. The re-recorded cassette is clean of credentials. One thing left, and it has a trap in it.

The new size assertion cannot catch the defect it was added for. llmagent_compaction_test.go:305 compares prompts[len-2] with prompts[len-1], which on this six-prompt recording is prompts[4] against prompts[5] — both of them after the compaction boundary. And promptTextOf counts text and function names only, so a ThoughtSignature, being a []byte field, is worth zero characters to it. Applied verbatim to the previous recording — the one carrying the 3,464-byte signature — it does not fire.

The trap is that moving it to the boundary its own comment describes makes it worse, not better. The real boundary is prompts[2] against prompts[3], and there the fixed recording measures 183 to 870, so an assertion relocated there would fail on correct behaviour. Growth across a compaction is expected here: the summary replaces several short turns with one long paragraph. Whatever this assertion should be, it is not a ratio bound on adjacent prompts, and I would rather see it dropped than left where it cannot fail.

[karolpiotrowicz]

The size assertion is gone and the reasoning you left in its place is better than the assertion was.

llmagent_compaction_test.go:295-306 now records why there is none: at the real compaction boundary this recording goes 183 characters to 870, because a summary of several short turns is longer than the turns, so an assertion placed there fails on working code — and placed anywhere else it compares two prompts on the same side of the boundary and cannot fail at all. That is exactly the trap, and removing the assertion is the right call rather than relocating it. The doc comment at the top of the test carries the same numbers now instead of the claim it used to make.

TestCompactionE2E passes here, and the load-bearing part of it — that the covered turns stop being sent and the summary reaches the prompt — is untouched.

Nothing further from me on this one.

[baptmont]

Rebase, and one fix the rebase surfaced.

TestCompactionE2E.httprr was claimed by no -httprecord directive. This file carried a comment asserting the package-level directive matched every cassette in the package — true when written, false now: main has since narrowed it to the FunctionTool, LLMAgent and ToolCallback cassettes. So this cassette could not be re-recorded by go generate, and TestHTTPRecordDirectivesPartitionCassettes failed after the rebase.

Added a directive scoped to this one cassette and rewrote the comment to say why it exists. The cassette itself is unchanged — the model traffic did not need re-recording, only the directive that claims it.

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.

[karolpiotrowicz]

Re-approving at 6b9000d to anchor the approval to the current head. The prior one named 49f31fd, and this branch does not dismiss stale reviews on push, so the badge stayed green while the SHA moved underneath it.

Nothing to re-review: this commit's own content is identical between the two revisions, with no file showing a real change. Everything that moved came from rebasing onto the PRs below it, each of which is approved at its own current head.