feat(go-adk): add compaction support to Go ADK runtime#1971
feat(go-adk): add compaction support to Go ADK runtime#1971yashrajshuklaaa wants to merge 3 commits into
Conversation
yashrajshuklaaa
requested review from
iplay88keys,
jmhbh and
supreme-gg-gg
as code owners
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR adds post-invocation “context compaction” support to the Go runtime by introducing a compaction package, wiring it into the A2A executor, and enabling in-memory session event replacement.
Changes:
- Added
go/adk/pkg/compactionwith configuration parsing and compaction/summarization logic. - Extended runner creation to return a compaction config and wired compaction into
KAgentExecutor.Execute. - Added
ReplaceEventsto local sessions to allow atomic in-memory event history rewrites after persisting a summary.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| go/core/internal/controller/reconciler/reconciler.go | Updates runtime feature notes to reflect compaction support. |
| go/adk/pkg/session/local_session.go | Adds an atomic ReplaceEvents method used by compaction. |
| go/adk/pkg/runner/adapter.go | Returns compaction config from CreateRunnerConfig via compaction.FromAgentConfig. |
| go/adk/pkg/compaction/compaction.go | Introduces compaction implementation and AgentConfig-to-Config translation. |
| go/adk/pkg/compaction/compaction_test.go | Adds initial unit tests for compaction helpers and nil-compactor behavior. |
| go/adk/pkg/a2a/executor.go | Instantiates a compactor and runs post-invocation compaction. |
| go/adk/cmd/main.go | Plumbs compaction config into executor and logs when enabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Implements post-invocation event history compaction for the Go ADK runtime. The upstream Go ADK v1.1.0 does not ship this so it is implemented within kagent via a new compaction package Closes kagent-dev#1783 Signed-off-by: Yashraj Shukla <shuklayashraj68@gmail.com>
yashrajshuklaaa
force-pushed
the
feat/go-adk-compaction
branch
from
ff168bb to
0b561b3
Compare
|
@iplay88keys @jmhbh @supreme-gg-gg PTAL |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
go/adk/pkg/runner/adapter.go:1
CreateRunnerConfignow has a breaking signature change by adding*compaction.Configas a new return value. If this function is used outside this package (it’s exported), this will force cascading changes for all callers. Consider a non-breaking approach such as (a) introducing a new function (e.g.,CreateRunnerConfigWithCompaction) or (b) embedding compaction config intorunner.Config/an options struct to avoid widening the return tuple.
package runner
| func buildSummaryEvent(summaryText string) *adksession.Event { | ||
| e := &adksession.Event{ | ||
| ID: uuid.NewString(), | ||
| InvocationID: "compaction_" + uuid.NewString(), | ||
| Timestamp: time.Now(), | ||
| Author: summaryEventAuthor, | ||
| } | ||
| e.LLMResponse = model.LLMResponse{ | ||
| Content: &genai.Content{ | ||
| Role: "model", | ||
| Parts: []*genai.Part{{Text: summaryText}}, | ||
| }, | ||
| } | ||
| return e | ||
| } |
| func serializeEvents(events []*adksession.Event) string { | ||
| var sb strings.Builder | ||
| for _, e := range events { | ||
| if e.Content == nil { | ||
| continue | ||
| } | ||
| role := e.Author | ||
| if role == "" { | ||
| role = e.Content.Role | ||
| } | ||
| for _, p := range e.Content.Parts { | ||
| if p == nil { | ||
| continue | ||
| } | ||
| switch { | ||
| case p.Text != "": | ||
| fmt.Fprintf(&sb, "[%s]: %s\n", role, p.Text) | ||
| case p.FunctionCall != nil: | ||
| fmt.Fprintf(&sb, "[%s] called tool %q\n", role, p.FunctionCall.Name) | ||
| case p.FunctionResponse != nil: | ||
| fmt.Fprintf(&sb, "[tool %s response]\n", p.FunctionResponse.Name) | ||
| } | ||
| } | ||
| } | ||
| return sb.String() | ||
| } |
| if len(invocations) >= c.cfg.CompactionInterval { | ||
| trigger = true | ||
| log.V(1).Info("Compaction triggered by interval", | ||
| "invocations", len(invocations), | ||
| "threshold", c.cfg.CompactionInterval) | ||
| } | ||
|
|
||
| if !trigger && c.cfg.TokenThreshold > 0 { | ||
| tokens := lastTokens | ||
| if tokens == 0 { | ||
| tokens = estimateTokens(allEvents) | ||
| } | ||
| if tokens >= c.cfg.TokenThreshold { | ||
| trigger = true | ||
| log.V(1).Info("Compaction triggered by token threshold", | ||
| "tokens", tokens, | ||
| "threshold", c.cfg.TokenThreshold) | ||
| } | ||
| } |
| keepCount := c.cfg.OverlapSize | ||
| if keepCount >= len(invocations) { | ||
| return nil | ||
| } | ||
|
|
||
| toCompact := invocations[:len(invocations)-keepCount] |
| if err := sessionSvc.AppendEvent(ctx, sess, summaryEvent); err != nil { | ||
| return fmt.Errorf("compaction: failed to persist summary event: %w", err) | ||
| } | ||
|
|
||
| var keepEvents []*adksession.Event | ||
| for _, inv := range invocations[len(invocations)-keepCount:] { | ||
| keepEvents = append(keepEvents, inv.events...) | ||
| } | ||
| // Apply EventRetentionSize cap if configured. | ||
| if c.cfg.EventRetentionSize > 0 && len(keepEvents) > c.cfg.EventRetentionSize { | ||
| keepEvents = keepEvents[len(keepEvents)-c.cfg.EventRetentionSize:] | ||
| } | ||
| replaceSessionEvents(sess, summaryEvent, keepEvents) |
| // 10b. Post-invocation compaction (no-op when not configured). | ||
| // TODO: avoid the extra GetSession call by threading the session object through Execute. | ||
| if e.compactor != nil && e.sessionService != nil { | ||
| liveSess, sessErr := e.sessionService.GetSession(ctx, e.appName, userID, sessionID) | ||
| if sessErr == nil && liveSess != nil { | ||
| if compactErr := e.compactor.MaybeCompact(ctx, liveSess, e.sessionService, 0); compactErr != nil { | ||
| e.logger.Error(compactErr, "Post-invocation compaction failed (non-fatal)") | ||
| } | ||
| } | ||
| } |
| if text == "" { | ||
| t.Fatal("expected non-empty serialized text") | ||
| } | ||
| if len(text) == 0 { | ||
| t.Fatal("serialized text should not be empty") | ||
| } |
EItanya
left a comment
EItanya
left a comment
There was a problem hiding this comment.
Thanks for taking this on. Adding Go-runtime compaction is a useful direction, and the current PR is a helpful starting point. I took a closer look against the Python ADK compaction path, and I think there is one important design gap to resolve before merging.
The main issue is that this does not yet match Python ADK compaction semantics. Python ADK does not durably delete compacted raw events. It appends a special compaction marker event with actions.compaction containing the compacted time range and compacted content, then prompt construction later uses that marker to replace covered raw events with the summary. In this PR, the Go runtime appends a normal model-looking summary event and trims only the in-memory local session. After a fresh GetSession, the persisted store still has the raw events plus the summary, and Go ADK has no marker-aware filtering step, so the next model request can see both raw history and the summary.
That means I would treat the durable-store Copilot concern as real, but I would not solve it by deleting old backend rows unless we intentionally want different semantics from Python. A closer match would be to persist some compaction marker/range and add a Go-side prompt-time filtering layer, or otherwise wrap/filter session events before Go ADK builds LLM contents.
A few related differences from the Python implementation also seem worth addressing:
- compaction_interval in Python means number of new invocations since the last compaction. The current Go code checks total grouped invocations, which can repeatedly compact the same logical history after reload or behave differently depending on in-memory truncation.
- summarizer_model appears to be parsed but not used to construct SummarizerLLM. Python builds an LlmEventSummarizer from the configured model, and otherwise initializes one from the agent model.
- token_threshold and event_retention_size are validated as a pair in Python. The Go implementation currently accepts partial token config and applies event retention to the kept events after interval compaction too, while Python uses retention for token-threshold compaction.
- Python takes care not to compact pending function calls or split function call/function response pairs. The Go implementation serializes calls/responses into text and can drop the structured pairing from the retained context.
The smaller Copilot comments about defensive copying, sentinels, and test cleanup are either already addressed or easy to handle. The extra GetSession call is a performance concern, but I think it is secondary to getting the compaction model aligned with Python first.
Suggested acceptance test: create enough persisted events to trigger compaction, run compaction, reload the session from the backing service, then verify that the next model request sees summary plus retained events rather than raw compacted history plus summary. That would catch the main semantic issue here.
Again, thank you for pushing this forward. I think the feature is valuable; it just needs the durable marker/filtering piece before it will behave like the existing Python ADK path.
|
@EItanya thanks for the detailed explanation , it really helped me understand what the correct implementation should look like So i checked the Go ADK module to see if the same primitives exist there. Went through every released version from v0.1.0 all the way to v1.4.0 , searching for EventCompaction , CompactedContent , StartTimestamp and any other naming variation I could think of. Nothing. The EventActions struct in Go ADK doesn't have a Compaction field at all and contents_processor.go has no filtering step for compaction markers either |
|
so i think the right move is to contribute to EventCompaction support upstream to google.golang.org/adk first by adding the struct to EventActions and the filtering logic in contents_processor.go to mirror what Python ADK does |
|
@EItanya opened an issue at https://github.com/google/adk-go after that we can continue with this |
Implements post-invocation event history compaction for the Go ADK runtime
The upstream Go ADK v1.1.0 does not ship this so it is implemented within kagent via a new compaction package
Closes #1783