AI-60: Add summary_fn parameter to TemporalModel for dynamic activity summaries#1451
Open
AI-60: Add summary_fn parameter to TemporalModel for dynamic activity summaries#1451
Conversation
xumaple
force-pushed
the
maplexu/AI-60-dynamic-summaries
branch
from
eaf06d6 to
01c9f22
Compare
tconley1428
reviewed
Apr 15, 2026
| ) | ||
| if activity_config: | ||
| self._activity_config.update(activity_config) | ||
| raw = dict(activity_config) if activity_config else {} |
Contributor
There was a problem hiding this comment.
This seems quite messy. And we don't really get anything out of the subclass here since we just have summary and summary_fn anyway
Contributor
Author
There was a problem hiding this comment.
messy in terms of code cleanliness of how I'm handling this section, or this overall idea of subclassing?
Contributor
There was a problem hiding this comment.
The handling. Raw dicts, type ignores, etc.
Contributor
Author
There was a problem hiding this comment.
I decided to remove the subclass altogether, because couldn't find a way to get the code much cleaner and reduce the errors. Let me know what you think now.
xumaple
changed the title
xumaple
commented
Apr 17, 2026
| the workflow task. | ||
|
|
||
| Raises: | ||
| ValueError: If both 'summary' and 'summary_fn' are set. |
Contributor
Author
There was a problem hiding this comment.
summary doesn't make sense here. qualify with ActivityConfig::summary or equivalent
| ) | ||
| if activity_config: | ||
| self._activity_config.update(activity_config) | ||
| raw = dict(activity_config) if activity_config else {} |
Contributor
Author
There was a problem hiding this comment.
I decided to remove the subclass altogether, because couldn't find a way to get the code much cleaner and reduce the errors. Let me know what you think now.
This was
linked to
issues
Apr 17, 2026
xumaple
force-pushed
the
maplexu/AI-60-dynamic-summaries
branch
from
a1fdd16 to
5faef5f
Compare
…maries Introduce AdkActivityConfig extending ActivityConfig with a summary_fn field that accepts a callable for dynamic per-call summaries. When no summary_fn or static summary is set, falls back to reading adk_agent_name from LlmRequest labels for zero-config agent name display. Setting both summary and summary_fn raises ValueError to prevent ambiguity. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…est, label fallback test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Drop AdkActivityConfig in favor of a keyword-only summary_fn parameter on TemporalModel. Zero type: ignore comments needed. Auditor findings addressed: - Label fallback test rewritten as integration test - Exception propagation documented in summary_fn docstring - Empty string summary test added Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nd docstring Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
xumaple
force-pushed
the
maplexu/AI-60-dynamic-summaries
branch
from
5faef5f to
f96fab4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
summary_fnkeyword-only parameter toTemporalModelfor dynamic per-call activity summariessummary_fnreceives theLlmRequestand returns a summary string (orNone)summary_fnnor staticsummaryis set, falls back to readingadk_agent_namefromLlmRequest.config.labelsfor zero-config agent name displaysummary(inActivityConfig) andsummary_fnraisesValueErrorTest plan
test_summary_fn_produces_dynamic_summary— integration test verifying dynamic summary in workflow historytest_summary_fn_returning_none— summary_fn returning None produces no summarytest_summary_fn_empty_string— empty string is a valid summarytest_adk_agent_name_label_fallback— integration test verifying zero-config label fallbacktest_summary_and_summary_fn_raises— ValueError when both summary and summary_fn set🤖 Generated with Claude Code