add feature: config summarisation, context compression, caching#1198
add feature: config summarisation, context compression, caching#1198Insomniac2904 wants to merge 4 commits intokagent-dev:mainfrom
Conversation
Signed-off-by: insomniac2904 <adarshkumar7478@gmail.com>
|
this only adds the configuration options to the resources - are the features actually implemented in the ADK library too? |
Signed-off-by: insomniac2904 <adarshkumar7478@gmail.com>
Signed-off-by: Adarsh Kumar <109868197+Insomniac2904@users.noreply.github.com>
Hey @peterj you were right. I could not find the context config for golang, only for python. I have revert back the original changes and made the changes to types.py and _a2a.py files. Also i am not sure for the imports that needs to me made for |
|
This seems like a very important feature to have, otherwise long sessions with agents might be impossible (?). Any idea what is needed to move this forward? Can I help in any way? |
Signed-off-by: Adarsh Kumar <109868197+Insomniac2904@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Exposes ADK context management options (compaction/summarization and caching) through the Agent configuration/CRD to help prevent context window overflows for tool-heavy agents (issue #1173).
Changes:
- Adds Python config models for context compression/caching and a helper to build ADK config objects.
- Extends the Python A2A app bootstrap to accept context-related configs for ADK
App. - Updates Agent CRD schema to include
.spec.declarative.context(cache + compression).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| python/packages/kagent-adk/src/kagent/adk/types.py | Adds context config types + helper; adds context to AgentConfig. |
| python/packages/kagent-adk/src/kagent/adk/_a2a.py | Adds context_configs plumbing into app construction (currently broken/incomplete). |
| helm/kagent-crds/templates/kagent.dev_agents.yaml | Adds CRD schema for .spec.declarative.context. |
| go/config/crd/bases/kagent.dev_agents.yaml | Adds CRD schema for .spec.declarative.context. |
| go/api/v1alpha2/zz_generated.deepcopy.go | Minor generated import formatting change. |
Comments suppressed due to low confidence (1)
python/packages/kagent-adk/src/kagent/adk/_a2a.py:109
- The
App(...)created increate_runner()does not includeevents_compaction_config/context_cache_config, so context compaction/caching will not be enabled even ifcontext_configsis provided. Pass the configs into thisApp(...)(or reuse the configuredAppinstance).
def create_runner() -> Runner:
root_agent = self.root_agent_factory()
adk_app = App(name=self.app_name, root_agent=root_agent, plugins=self.plugins)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.agent_card = agent_card | ||
| self._lifespan = lifespan | ||
| self.plugins = plugins if plugins is not None else [] | ||
| self.context_configs = context_configs or {"compaction": None, "cache": None} |
There was a problem hiding this comment.
The default context_configs dict uses keys compaction/cache, but build_adk_context_configs(...) currently produces events_compaction_config/context_cache_config. Standardize the keys across the codepath so callers can pass the output through without needing an ad-hoc remap.
| context: | ||
| properties: | ||
| cache: | ||
| properties: | ||
| cacheIntervals: | ||
| type: integer | ||
| enabled: | ||
| type: boolean | ||
| minTokens: | ||
| type: integer | ||
| ttlSeconds: | ||
| type: integer | ||
| required: | ||
| - enabled | ||
| type: object | ||
| compression: | ||
| properties: | ||
| compactionInterval: | ||
| type: integer | ||
| enabled: | ||
| type: boolean | ||
| overlapSize: | ||
| type: integer | ||
| summarizer: | ||
| properties: | ||
| model: | ||
| description: Model to use for summarization (required | ||
| if type is llm) | ||
| type: string | ||
| type: | ||
| default: llm | ||
| enum: | ||
| - llm | ||
| - text | ||
| type: string | ||
| type: object | ||
| required: | ||
| - enabled | ||
| type: object | ||
| type: object |
There was a problem hiding this comment.
This CRD schema adds .spec.declarative.context, but the Go API type DeclarativeAgentSpec (go/api/v1alpha2/agent_types.go) does not define a matching field. With controller-runtime typed clients, this config is likely dropped during decode and never reaches the translator/runtime. Add the field to the Go types and regenerate CRDs to keep schema + controller behavior consistent.
| context: | |
| properties: | |
| cache: | |
| properties: | |
| cacheIntervals: | |
| type: integer | |
| enabled: | |
| type: boolean | |
| minTokens: | |
| type: integer | |
| ttlSeconds: | |
| type: integer | |
| required: | |
| - enabled | |
| type: object | |
| compression: | |
| properties: | |
| compactionInterval: | |
| type: integer | |
| enabled: | |
| type: boolean | |
| overlapSize: | |
| type: integer | |
| summarizer: | |
| properties: | |
| model: | |
| description: Model to use for summarization (required | |
| if type is llm) | |
| type: string | |
| type: | |
| default: llm | |
| enum: | |
| - llm | |
| - text | |
| type: string | |
| type: object | |
| required: | |
| - enabled | |
| type: object | |
| type: object |
| context: | ||
| properties: |
There was a problem hiding this comment.
The PR description/example includes windowCompression, but the schema here only adds cache and compression. Either add the missing windowCompression config to the CRD (and implement it) or update the PR description to reflect the supported fields.
| def build_adk_context_configs(ctx: Optional[ContextConfig], default_llm: Any) -> Dict[str, Any]: | ||
| configs = { | ||
| "events_compaction_config": None, | ||
| "context_cache_config": None | ||
| } |
There was a problem hiding this comment.
build_adk_context_configs uses Dict in the return type but Dict is not imported in this module. Without from __future__ import annotations, this will raise a NameError at import time. Use dict[str, Any] (Py3.11+) or import Dict from typing.
| if ctx.compression.summarizer and ctx.compression.summarizer.type == "llm": | ||
| # Use the provided model for summarization, or fall back to the main agent model | ||
| summerizer_model_name = ctx.compression.summarizer.model | ||
| summarizer_llm = GeminiLLM(model=summerizer_model_name) if summerizer_model_name else default_llm | ||
| summarizer = LlmEventSummarizer(llm=summarizer_llm) |
There was a problem hiding this comment.
When a summarizer model override is provided, the code always instantiates GeminiLLM(model=...). This will break for non-Gemini model names (e.g. the PR description’s gpt-4o-mini) and also ignores the main agent model provider. Consider either (1) always using default_llm and disallowing provider changes, or (2) extending ContextSummarizerSettings to include a provider/type and constructing the correct ADK LLM wrapper accordingly.
| adk_app = App( | ||
| name=self.app_name, | ||
| root_agent=self.root_agent, | ||
| plugins=self.plugins, | ||
| events_compaction_config=self.context_configs.get("compaction"), |
There was a problem hiding this comment.
build() constructs an App(...) with root_agent=self.root_agent, but KAgentApp never defines self.root_agent. This will raise AttributeError the first time build() is called.
| cache_intervals=ctx.cache.cache_intervals or 5, | ||
| ) | ||
|
|
||
| return configs |
There was a problem hiding this comment.
There should be a blank line separation between the end of build_adk_context_configs and the next top-level class Bedrock definition; ruff/pycodestyle typically flags this (E305).
| return configs | |
| return configs |
| context: | ||
| properties: | ||
| cache: | ||
| properties: | ||
| cacheIntervals: | ||
| type: integer | ||
| enabled: | ||
| type: boolean | ||
| minTokens: | ||
| type: integer | ||
| ttlSeconds: | ||
| type: integer | ||
| required: | ||
| - enabled | ||
| type: object | ||
| compression: | ||
| properties: | ||
| compactionInterval: | ||
| type: integer | ||
| enabled: | ||
| type: boolean | ||
| overlapSize: | ||
| type: integer | ||
| summarizer: | ||
| properties: | ||
| model: | ||
| description: Model to use for summarization (required | ||
| if type is llm) | ||
| type: string | ||
| type: | ||
| default: llm | ||
| enum: | ||
| - llm | ||
| - text | ||
| type: string | ||
| type: object | ||
| required: | ||
| - enabled | ||
| type: object | ||
| type: object |
There was a problem hiding this comment.
This CRD schema adds .spec.declarative.context, but the Go API type DeclarativeAgentSpec (go/api/v1alpha2/agent_types.go) does not define a matching field. With controller-runtime typed clients, this config is likely dropped during decode and never reaches the translator/runtime. Add the field to the Go types and regenerate CRDs to keep schema + controller behavior consistent.
| context: | |
| properties: | |
| cache: | |
| properties: | |
| cacheIntervals: | |
| type: integer | |
| enabled: | |
| type: boolean | |
| minTokens: | |
| type: integer | |
| ttlSeconds: | |
| type: integer | |
| required: | |
| - enabled | |
| type: object | |
| compression: | |
| properties: | |
| compactionInterval: | |
| type: integer | |
| enabled: | |
| type: boolean | |
| overlapSize: | |
| type: integer | |
| summarizer: | |
| properties: | |
| model: | |
| description: Model to use for summarization (required | |
| if type is llm) | |
| type: string | |
| type: | |
| default: llm | |
| enum: | |
| - llm | |
| - text | |
| type: string | |
| type: object | |
| required: | |
| - enabled | |
| type: object | |
| type: object |
| class ContextCompressionSettings(BaseModel): | ||
| enabled: bool | ||
| compaction_interval: Optional[int] = Field(None, alias="compactionInterval") | ||
| overlap_size: Optional[int] = Field(None, alias="overlapSize") | ||
| summarizer: Optional[ContextSummarizerSettings] = None |
There was a problem hiding this comment.
PEP 8 / ruff: top-level class definitions should be separated by two blank lines. In this new block, the Context*Settings classes are adjacent without the expected spacing, which will typically trigger E302/E305.
| execute_code: bool | None = None | ||
| context: Optional[ContextConfig] = None |
There was a problem hiding this comment.
context is added to AgentConfig, but to_agent() does not use it and there’s no wiring here to pass context settings into the ADK App/Runner. As-is, setting .context in config/CRD won’t change runtime behavior.
|
Sorry for the AI slop here. I need to go over this a bit more in detail before it can be merge ready. I'd appreciate some feedback on the approach, though. If I fix all the issues, would you accept it? |
|
Closing this in favor of #1298 |
|
any updates on that matter? can not really work like this currently |
|
Working on this in #1340 now |
adds feature for #1173.
example use case: