feat(context-py): port strategy presets and agent rewire - #4282
Conversation
Port the TS context-manager presets and agent rewire (PR strands-agents#4256) to the Python SDK. The Agent constructor now uses ContextManager.from_strategy() and ContextManager.resolve_conversation_manager() instead of the old _resolve_context_manager method with SummarizingConversationManager + ContextOffloader. Key changes: - Add presets.py with resolve_preset/resolve_strategies for named presets - Add from_strategy() factory and resolve_conversation_manager() static method to ContextManager - Add overflow flag to ContextState, honored by offload and emergency truncate strategies - Protect pinned messages in offload strategies (eager hook, per-block, per-message) and preserve pinned metadata through _repair_alternation - Rewire Agent constructor to build ContextManager plugin directly - Update all tests for new architecture
|
@strandly-the-agent pls review |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
… pinned protection Cover the lines flagged by codecov: - presets.py: all preset names, unknown preset errors, mixed resolve - offload/base.py: overflow bypass on utilization gate, pinned message guards in eager hook / per-block / per-message, repair_alternation pinned metadata preservation - truncate.py: EmergencyTruncateStrategy overflow vs non-overflow paths - context_manager.py: unsupported type in from_strategy()
strandly-the-agent
left a comment
There was a problem hiding this comment.
Changes requested — the proactive_summarization preset loses its preserve_recent (0.7 → int() → 0) and summarizes the in-flight turn; mypy fails on the branch (2 errors).
Otherwise a faithful port: the overflow bypass, pinned protection and Agent rewire held up under attack (details below). One 🔴, two 🟡 inline; a few parity questions. No needs-api-review label yet, but Agent(context_manager=...) changed both its accepted types and its behaviour when conversation_manager is co-provided — worth flagging for API review.
✅ Verified at 97be1e1
ruff check+ruff format --check: clean.pytest tests(py3.13): 6594 passed, 31 skipped (1058 in_context_manager/agent, 5536 elsewhere).mypy --follow-imports=silent src/strands/_context_manager src/strands/agent/agent.py src/strands/experimental/context_manager: 2 errors —presets.py:44(float→int) andcontext_manager.py:143(list[ContextStrategy]vslist[ContextStrategy | str]). Both are in thehatch run preparegate.- Overflow path: real
Agent+ fake model, presetsauto/agentic, history 3–7 messages, 1–4 consecutive overflows — live user turn preserved every time, no orphan toolResults / role collisions,overflow=Truebypass fires as intended. - Pinned protection: eager per-block truncate, per-block, message-level drop (preserve_recent 0 and 2) — pinned message and its tool-pair partner intact;
_repair_alternationkeeps the flag. context_manager={}→ same as"auto"+NullConversationManager(matches TS). Co-providedconversation_managerwarning is tested.- Evidence (repro scripts + outputs, logs) uploaded to the artifacts bucket under
strands-agents/harness-sdk/pr/4282/. - Process note: my three specialist subagent passes timed out; I ran the remaining checks myself from their partial artifacts.
Questions (non-blocking)
- Breaking change: previously a co-provided
conversation_managerwon overcontext_manager="auto"(old docstring: "the user's conversation manager is used instead"); now it's discarded with a warning. Matches TS, but Python users who passed both silently lose their CM — should the PR body call this out as breaking? _removal_ratio: TS #4256 removed_removalRatio(message-level strategies now remove all eligible;preserveRecentis the only knob), Python keeps 0.3 inbase.py:274. Intentional divergence, or follow-up?- Emergency truncate vs pins: Python's
EmergencyTruncateStrategy._apply_per_messagegoes through the pinned-aware_get_eligible_messages; TS's ignores pins (messages.filter((_, i) => i > 0)). With every history message pinned, Python's only candidate is the live user turn, which it drops (_adv/attack3b_live_turn_deleted.out). Which behaviour is intended?
Reading order
presets.py+base.py:285-291(the coercion) → 2.context_manager.pyfrom_strategy/resolve_conversation_manager→ 3.agent.py:383-405→ 4.base.pypinned/overflow hunks → 5. tests.
Appendix — non-blocking (6)
- ⚪
context_manager.py:175stacklevel=4points one frame above the user'sAgent(...)call (verified: warning attributed to the caller of the function that constructs the Agent). Should be 3. - ⚪
ContextManagerStrategyLiteral is now defined twice (agent.py:157,context_manager.py:43);ContextManagerStrategyValue(context_manager.py:51) is an unused string constant. Define once in_context_managerand import inagent.py. - ⚪
base.py:317-318:prev.get("metadata", {}).get("custom", {})raisesAttributeErrorwhenmetadataormetadata["custom"]isNone— and since the splice already happened, the strategy aborts leaving non-alternating messages that get sent to the model. Type-violating input, so low priority; reusingpin_message._has_pinned_flag-style guarding (metadata is not None and ...) closes it. - ⚪
test_presets.pyonly asserts strategy names; asserting_preserve_recent/_utilization_thresholdper preset would have caught the 🔴. - ⚪
models/bedrock.pyhunk is a formatting-only change unrelated to this PR. - ⚪
ContextManager.resolve_conversation_managerlazily importsagent.conversation_managerfrom inside_context_manager— TS keeps this resolution inagent.ts. Layering nit; fine if deliberate.
…ecent, config types, and agent param - Support float preserve_recent (0 < value < 1 treated as ratio of matching messages) - Accept preset name strings in ContextManagerConfig.strategies - Add ContextManagerConfig to agent's context_manager type union - Fix stacklevel 4→3 in resolve_conversation_manager warning - Remove duplicate ContextManagerStrategy definition from agent.py - Remove unused ContextManagerStrategyValue constant
|
@strandly-the-agent pls review |
… when earlier strategies acted
strandly-the-agent
left a comment
There was a problem hiding this comment.
Approving — all three findings from my first pass are fixed at 43ffff0, and the fixes didn't trade one bug for another.
Still missing an API-review label; the Agent(context_manager=...) surface here mirrors TS #4256, which is itself still under human review — the two should land together.
Status of previous findings
- 🔴
presets.py:44preserve_recent 0.7 → 0 — fixed (a1b3452):_get_oldest_matchesnow treats0 < count < 1as a ratio (ceil(len * count)),_preserve_recentis a float,.when()/_build_conditions/OffloadConditionswidened. Re-ran the repro:_preserve_recent=0.7, summarizer never called, tool turn intact. Boundary check (10 msgs): 0.05→9 eligible, 0.5→5, 0.7→3, 0.99→0, 1/1.0→9, 4→6, 2.5→8 — matches TSgetOldestMatches. - 🟡
ContextManagerConfig.strategiestype — fixed (list[ContextStrategy | str]; mypy clean). - 🟡
Agent.__init__annotation — fixed (ContextManagerConfigadded, imported underTYPE_CHECKING). - ⚪ stacklevel → 3 — fixed (warning now attributed to the
Agent(...)call line). - ⚪ duplicate
ContextManagerStrategy/ unusedContextManagerStrategyValue— fixed (single definition, imported into agent.py). - Questions 1–3 (breaking-change note,
_removal_ratio, emergency-truncate vs pins) — still open, non-blocking.
✅ Verified at 43ffff0
ruff check+ruff format --check: clean.- mypy (touched modules,
--follow-imports=silent): clean (was 2 errors). pytest tests: 6598 passed, 31 skipped.- Overflow attack re-run (auto/agentic, history 3–7, 1–4 consecutive overflows): live turn preserved in all 9 scenarios, no orphan toolResults / role collisions.
- New in 43ffff0:
_run_strategiesclearscontext.overflowbeforeEmergencyTruncateStrategyonce an earlier strategy acted, so emergency truncate re-gates on real utilization instead of firing unconditionally. Behaves correctly in the overflow scenarios; note TS_runStrategiesat #4256 head doesn't do this, so it's a (sensible) divergence worth mirroring there. No test pins it down — optional. - Evidence uploaded under
strands-agents/harness-sdk/pr/4282/(*-after-fix.out,mypy-pr.txt,pytest-full.log).
|
Assessment: Comment Solid, well-tested port of the TS presets/agent-rewire to Python — Review Categories
Nice work carrying the preset-as-stable-contract framing and pinned-message protection across the strategies — the design reads cleanly. |
- Remove narrating comment in agent.py - Use _has_pinned_flag helper in _repair_alternation instead of raw dict access - Remove redundant preset validation in resolve_strategies (resolve_preset raises) - Build identity→index map to eliminate O(n²) scans in per-block, eager hook, and eligible messages - Convert preset tests from private attribute assertions to behavioral assertions
|
Assessment: Approve (pending one diff-hygiene nit) Re-reviewed at Verified fixes
Remaining (non-blocking):
Nice turnaround on the ratio bug — the preset-parity fix reads cleanly. |
Merge upstream/main into cm-presets-and-rewire-python, resolving conflicts in context_manager.py (keep both stash durability properties from upstream and from_strategy/resolve_conversation_manager from this branch) and agent.py (integrate _context_manager property with from_strategy plugin injection).
|
Assessment: Approve (one diff-hygiene nit remains) Re-checked at
Still open (non-blocking): the unrelated formatting-only churn is still bundled — No new issues from the merge — good to go once the churn is trimmed (or explicitly deferred). |
…ernalize presets, fix docstring - Remove _removal_ratio from BaseOffloadStrategy; regular strategies now remove all eligible messages (preserve_recent controls what to keep), matching TS PR strands-agents#4256 - EmergencyTruncateStrategy gets its own _apply_per_message with a module-level _EMERGENCY_REMOVAL_RATIO = 0.2 - Prefix resolve_preset/resolve_strategies with underscore (internal-only) - Rephrase resolve_conversation_manager docstring to avoid leaking NullConversationManager implementation detail
|
Assessment: Approve Re-reviewed at Verified fixes
One thing to confirm (non-blocking): the new Still open (non-blocking, third mention): the model-file formatting churn is still bundled and unrelated to this change — Great turnaround across the rounds — the preset-parity and pinned-protection design reads cleanly and is well covered. |
|
Assessment: Approve Re-reviewed at
That's exactly the clarification needed; the last-resort recovery behavior is now explicit rather than surprising. Verified locally: ruff clean, 260 context-manager tests pass. The only remaining item is the unrelated model-file formatting churn ( |
Description
Port the TS context-manager presets and agent rewire (PR #4256) to the Python SDK. The Agent constructor now uses ContextManager.from_strategy() and ContextManager.resolve_conversation_manager() instead of the old _resolve_context_manager method with SummarizingConversationManager + ContextOffloader.
Key changes:
Related Issues
Documentation PR
Type of Change
Bug fix
New feature
Breaking change
Documentation update
Other (please describe):
Testing
How have you tested the change? Verify that the changes do not break functionality or introduce new warnings.
hatch run prepareChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.