Repository navigation
Conversation
Rename threshold to block_threshold and accept "N%" shares alongside counts. The target and block_threshold choose between whole messages and blocks; utilization only gates when a strategy fires. Offload.* returns a builder that needs .when() before use. Truncate gains a middle-out message form, skips its own previews, and defaults its block threshold to preview_tokens. Summarize handles each contiguous run separately, and overflow_protection drops the oldest messages.
Mirror the Python redesign: blockThreshold replaces threshold, "N%" shares are accepted alongside counts, the target and blockThreshold pick messages vs blocks, Offload.* returns a builder that needs .when(), truncate gains a middle-out message form and skips its own previews, and summarize handles each contiguous run separately.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
@strandly-the-agent pls review |
|
Issue: The Risks section notes a summary can still land after the assistant message that follows a summarized run, reordering content. This is a real correctness nuance that's easy to lose once this PR merges. Suggestion: File a tracking issue for the follow-up fix (and reference it here) so the stash-ref trade-off gets its own review rather than relying on the PR description to carry it forward. |
|
Issue (process): This is a breaking change ( Suggestion: Add |
|
Assessment: Comment The new rule — target + Review themes
Nice work carving out semantics that will hold up through graduation. |
strandly-the-agent
left a comment
There was a problem hiding this comment.
Changes requested — one regression in message-mode summarize; the redesign itself holds up.
- 🔴
summarize("*")now mutates run-by-run and only repairs alternation after the loop. An aux-model error on run 2+ leaves consecutiveassistantmessages persisted inagent.messages(repro: 3/18 pinned layouts fail). Onmainthe batched summary was computed before any splice, so this is new.try/finallyfixes it (inline). - 🟡
utilization=80means 80 tokens, so the likeliest typo for"80%"fires on turn 1 silently (inline). - 🟡
block_threshold=0bypasses the"*"message-mode guard that"0%"enforces, re-enabling eager drop-everything (inline).
The public .when() surface changed and there's no needs-api-review label — worth one, since this is explicitly the pre-graduation contract.
Verified / questions / reading order / appendix
✅ Verified at bebc2429
ruff check+ruff format --checkclean;mypy src/strands/_context_managerclean;pytest tests/strands/_context_manager→ 327 passed.- Repros through
Agent+ContextManagerwith a mocked model: toolUse/toolResult pairing and alternation hold fordrop("*")/truncate("*")with a pinned message at every index, inside tool loops, and forpreserve_recent0–4; message-mode truncate is idempotent across turns;overflow_protectionon 3- and 5-message histories preserves the live turn; the eager hook skips pinned messages andretrieve_contextresults; percent parsing rejects" 5 %","5","105%","-5%","abc%","", bools, nan, inf. - No
.when(threshold=call sites remain outsideteam/designs/0015-context-manager.md.
Questions (non-blocking)
- Is the absolute-token form of
utilizationneeded by anyone? Dropping it removes the int/float cliff at 1 and makes the hintstr | floathonest. OffloadConditionsis exported fromstrands.experimental.context_managerbut no public callable accepts it (.when()is kwargs-only), whileOffloadStrategyBuilder— the return type of everyOffload.*call — isn't exported. Intentional?
Reading order: base.py:28-135 (parsing) → base.py:385-471 (mode selection, dispatch) → summarize.py:59-89 → truncate.py:74-98 → presets.py.
Appendix — non-blocking (5)
presets.py:70-72: the raw-builderTypeErrorsays "call.when()with no conditions", which for a"*"target then raises theValueErroratbase.py:411. Mentionutilization/preserve_recenttoo.truncate.py:65: theblock_threshold > preview_tokenscheck only covers ints;"0.1%"of a 200k window is 200 tokens and passes. Harmless since_is_truncatedstops re-truncation, buttest_truncate_skips_preview_check_for_percent_thresholdpins the asymmetry as intended behaviour.methods/truncate.py_is_truncated: a tool result whose genuine content starts with[Truncated:is never truncated (repro'd: 20k chars left in place). Contrived.presets.py:24: "drop tool results older than 5 messages" —preserve_recentcounts matching messages, not messages.- Test gaps:
utilizationint/float boundary (1vs1.0vs2), andpreserve_recent=0vs"0%"equivalence.
Evidence (pytest log, F2 repro script + output, the Agent/ContextManager test harness, and the three raw pass reports) is in the artifact store under harness-sdk/pr/5324.
…xt window A count above the model's context window can never be reached, so the strategy would only fire on overflow.
Treats a zero window as unknown, matching how the window is resolved elsewhere.
|
@strandly-the-agent pls review |
…ditions # Conflicts: # strands-py/src/strands/_context_manager/strategies/offload/base.py # strands-py/src/strands/_context_manager/strategies/offload/summarize.py
strandly-the-agent
left a comment
There was a problem hiding this comment.
Approve — the 🔴 is fixed and verified; the two 🟡 threads are still open but non-blocking.
Status of my 10-21 findings (bebc2429 → d3b5e992):
- 🔴 summarize run failure → invalid history: fixed (552e578 guards
_summarize_contentper run;test_a_failed_run_is_left_intact_and_the_others_still_summarizeis my repro scenario and asserts alternation). The one call left outside thetry,model.count_tokens(safe), can't raise on Bedrock (falls back to the heuristic), so no residual. - 🟡
utilization=80= 80 tokens: still open — 9d8f309 caps token counts at the context window instead, which catches200001but not80. Deliberate call; leaving the thread for you to close. - 🟡
threshold=0bypasses the"*"guard: still open, unchanged (drop("*").when(threshold=0)is eager while"0%"is rejected). Non-blocking.
New in the delta — block_threshold → threshold rename, gated block strategies skip the newest message, message-mode truncate skips its own markers, raw-builder TypeError wording — all check out. The PR body still documents block_threshold and #5325's title still says blockThreshold (TS source already uses threshold). Still no needs-api-review label.
Verified / appendix
✅ Verified at d3b5e992: ruff check + ruff format --check clean; mypy src/strands/_context_manager clean; pytest tests/strands/_context_manager → 337 passed. Direct probes: utilization=200001 → ValueError at init() naming the window; drop("*").when() and the raw-builder path raise the new messages; _apply_per_block pops the newest message only when not eager.
Appendix — non-blocking (2)
base.py:443-446vs:457: withcontext_window_limit=None,_check_utilization_fitsskips the check but_utilization_reacheddivides byDEFAULT_CONTEXT_WINDOW_LIMIT, soutilization=10**9passesinit()and never fires. Probed.truncate.py_has_elision_marker: a message that carries an appended marker (the_insert_markerneighbour case) is excluded from eligibility for good, so its own content is never elided. One message, bounded — probed with a 5k-char carrier.
…arify percent thresholds
…target Target "*" now always acts on whole messages, and threshold measures each message's size. Other targets act on blocks, and threshold measures each block. A new "media" target selects image, document, video, and audio blocks outside tool results, which "*" with a threshold used to reach.
Media the heuristic token count measures as 0 tokens was never offloaded because blocks at or under an implicit 0-token threshold were skipped. Empty text blocks are still skipped, and markers leave out a size the count could not measure.
…oved A bare [Dropped] marker reads to the model as media that never arrived, so it retracts what it said about it.
…ect a zero threshold - Gated block strategies skip message 0, as message mode does. - threshold must be at least 1 token; 0 on "*" acted on every message. - A token utilization is checked against the default window when the model reports none. - Offloaded media markers name the media kind, as dropped ones do. - Clarify unset utilization, tool-name targets, the preset descriptions, and the truncate convergence error.
A token count can only be compared to a known window, so init raises when the model has no context_window_limit instead of assuming the 200,000-token default.
…has no context window A token count fires at the same conversation size whatever the window, so only the init size check needs it; without a context_window_limit the check is skipped with a warning.
|
Assessment: Comment (re-review at The warn-instead-of-raise fix is correct and well-tested. I verified the premise: State of prior review items
No new blocking issues in this commit — it's a clean, well-scoped fix. |
A whole number like utilization=80 is almost always a percent written without "%", which would fire on the first turn; the error says to write "80%".
|
Assessment: Comment (re-review at Docs-only change since Remaining non-blocking items are the still-open |
|
@strandly-the-agent pls review |
strandly-the-agent
left a comment
There was a problem hiding this comment.
Changes requested — both of my open 🟡s are now fixed; one new 🔴 on the new "media" target.
Status of previous findings (d3b5e992 → a7cebc61):
- 🟡
utilization=80= 80 tokens: fixed —_MIN_UTILIZATION_TOKENS = 1000(base.py:79) rejects it with a hint to write"80%". Probed. - 🟡
threshold=0bypassing the"*"guard: fixed —value < 1now rejected (base.py:122). Probed.
New:
- 🔴
Offload.drop("media").when()runs eagerly, so the user's image is replaced before the model's first call with[Dropped: image, removed from context after it was viewed]— the model never saw it and is told it did. Repro'd through a realAgent; inline with a suggestion.
The "*"-always-messages rule, threshold as a size filter, and the new gated-path protections all check out (attacks A–D, F–G pass). The PR body is now a good spec. Still no needs-api-review label for a new target + semantics change on the pre-graduation contract.
Verified / appendix
✅ Verified at a7cebc61: ruff check + ruff format --check clean; mypy src/strands/_context_manager clean; pytest tests/strands/_context_manager → 353 passed. Repros: drop("*").when(threshold=N) splices toolUse/toolResult pairs in both directions, never removes messages[-1], refuses a pair anchored on message 0; truncate("*").when(threshold=N) never touches message 0 and converges on the second pass; summarize("*").when(threshold=N) with two runs keeps alternation; drop("media").when(preserve_recent=1) keeps the newest image and excludes message 0; media-only messages become a single text block (never empty content); media inside toolResults is skipped. Evidence (repro-eager-media.py/.log, findings-adv3.md, adv3-bcdefg.py, pytest-a7cebc61.log) is in the artifact store under harness-sdk/pr/5324.
Appendix — non-blocking (3)
truncate.py:100-111_shorten_each: a message made of many blocks each underpreview_tokensbut overthresholdin total is never shortened, yet is re-measured (onecount_tokensper message) on every model call. Cost only.base.py:299-302: an oversizedmessages[1]paired with message 0 is permanently unremovable and re-measured each pass. Correct, just worth knowing.threshold="100%"is accepted and is a permanent no-op (resolves to the whole window). Reject or document.
- Offload.*("media") without utilization or preserve_recent raises, since the
eager hook would replace media before the model sees it
- threshold="100%" is rejected; it could never select anything
- a tool list that mixes included and excluded tools raises at construction
- truncate checks a percent threshold against preview_tokens on init
- export OffloadStrategyBuilder and note one strategy instance per agent
…truncate("*") thresholds
- the token heuristic counts media as 0, so a media block with a threshold
was never offloaded; an unmeasured media block now passes the size check
- truncate("*") with a threshold at or below preview_tokens raises, as block
targets already did; it would select messages it then cannot shorten
- count message sizes concurrently when selecting eligible messages
- when() and threshold docs match the rules
|
@strandly-the-agent pls review |
strandly-the-agent
left a comment
There was a problem hiding this comment.
Approve — the 🔴 is fixed and verified; nothing new to flag in the a7cebc61 → 51831e42 delta.
- 🔴 eager
"media"removing the image before the first model call: fixed (ee2b268, base.py:456-460). Re-ran myAgentrepro:drop("media").when()now raisesOffload.drop("media") would remove media before the model sees it; set utilization or a preserve_recent above 0;.when(threshold=100)andtruncate("media").when()raise too. The gated formdrop("media").when(preserve_recent=1)over three image turns shows the model the new image each time and drops only older ones (message 0 kept). - Also verified:
threshold="100%"rejected; percentthresholdchecked againstpreview_tokensatinit()("0.1%"of 200k → 200 tokens → clear error);["bash", "!bash"]raises instead of silently ignoring the exclusion while["bash", "ls", "!bash"]→{ls}; unmeasured media (0 tokens) always passes a block threshold;OffloadStrategyBuilderexported.
Still no needs-api-review label; this is the one thing I'd want before it leaves draft.
Verified
ruff check + ruff format --check clean; mypy src/strands/_context_manager clean; pytest tests/strands/_context_manager → 372 passed at 51831e42. Repros: repro-eager-media.py (now raises), gated_media.py (3 image turns, block keys per model call: [text,image] → [text,image],[text],[text,image] → [text,image],[text],[text,text],[text],[text,image]). Delta reviewed directly — fixes to filed findings plus small validation tightening, no fan-out.
… condition values
Human Overview
Description
ContextManager graduates in #5205, and after that the Offload
.when()conditions can only change in 2.0, so their meanings need to be right first. Onmain, settingutilizationsilently switches drop, truncate and summarize into whole-message mode. That causes three problems:thresholdpicks messages rather than blocks.utilizationdecides granularity as well as timing.The new rule is that the target chooses what a strategy acts on,
thresholdsets the minimum size to act on,utilizationonly says when it fires, andpreserve_recentonly says what's protected. Target"*"always acts on whole messages, andthresholdthen measures each message. Every other target acts on blocks, andthresholdmeasures each block. Whenthresholdisn't set, there's no size check, so every matching message or block is acted on."*"(messages)[Dropped: N messages][Dropped]threshold, keeps the head and tail withinpreview_tokensand elides the middle. With athreshold, shortens each large message in place and leaves the newest one aloneValues follow one rule: a number counts in the condition's unit, and
"N%"is a share of the whole.threshold="5%"means 5% of the context window. Decimals like0.85are not accepted, so each value has one meaning in every condition.Public API Changes
Offload.*returns an opaque builder that is not aContextStrategy. If you pass one instrategieswithout calling.when(), it raisesTypeError."media"target matches image, document, video and audio blocks outside tool results. It needsutilizationorpreserve_recent. Without either, the strategy would run on each message as it arrives and replace the media before the model's first call.["bash", "!python"]acts on bash. A list whose excludes cancel every include (["bash", "!bash"]) matches nothing and raisesValueErrorwhen it's built. Before, a mixed list logged a warning and ignored the excludes.OffloadStrategyBuilderis exported fromstrands.experimental.context_manager, since everyOffload.*call returns one.utilizationcounts conversation tokens and must be at least 1,000, so a missing%like80fails with a hint to write"80%". If it's larger than the model'scontext_window_limit, the strategy raisesValueErroron init. If the model has nocontext_window_limit, it warns and skips that check; the count still fires at the right size, since it doesn't depend on the window.Behaviour changes for existing configs
utilizationorpreserve_recent(0.85) raises aValueErrorwhose message gives the"N%"to write. A number now always counts, soutilization=1, which meant 100%, is rejected too. The presets and the"auto"and"agentic"context managers use"N%"."*"target plusutilizationnow acts on blocks.drop("*")andsummarize("*")now act on whole messages for every combination of conditions.truncate("*")withoutthresholdis now middle-out.threshold, a strategy acts on blocks of any size. Truncate no longer defaults the threshold topreview_tokens, because its preview already leaves blocks that fit untouched. It also skips blocks it has already truncated, so it doesn't re-truncate its own previews.utilizationorpreserve_recentleaves message 0 and the last message alone, as message mode already did.thresholdmust be at least 1, or a"N%"below"100%". Athresholdof 0 got past the"*"guard and acted on every message, and"100%"could never select anything.[Dropped: image, removed from context after it was viewed], not[Dropped]. On Opus 5 the bare marker read like media that never arrived, and the model took back answers it had already given correctly.[Offloaded: image, ~N tokens, removed from context after it was viewed].[Offloaded]and[Summarized: …]leave out the token count when it's 0, which is what the heuristic counter returns for media.threshold > preview_tokensfor every target, including"*", whosethresholdwould otherwise select messages it can't shorten. A token threshold is checked when the strategy is built, and a"N%"threshold on init, against the context window.overflow_protectiondrops the oldest messages instead of truncating them.Risks / Callouts
_insert_markerappends the summary to the end of that message to keep its stash block indices stable.mainalready does the same for every marker. A pinned assistant message next to the gap also absorbs the marker. I'd fix this in a follow-up so the stash-ref trade-off gets its own review.thresholdinstead of never passing it, and its[Offloaded]marker leaves out the size. Estimating media size in the fallback counter is a separate follow-up.!in the commits is deliberate.Related Issues
Prerequisite for #5205. The TypeScript side is in #5325.
Documentation PR
Docs will follow in a separate PR.
Type of Change
Breaking change
Testing
Python:
hatch fmt --formatter,hatch fmt --linter, andhatch testall pass (7983 passed).hatch run preparealso passes.Complexity:
npm run complexity. The highest score among the touched functions is 14, for_apply_per_block.Real-model runs on Bedrock Claude Haiku 4.5 and Opus 5, using a script that has the agent read two large files and then asks it to recall facts from both:
drop("*"),truncate("*", {"preview_tokens": 200})andsummarize("*")withthreshold=1000each reduced the old large messages and left the newest message alone. Both models then recalled the earlier facts correctly.drop("media").when(preserve_recent=1)across three image turns replaced the older image with the new marker. Both models then listed every color they'd seen. With the bare[Dropped]marker, Opus 5 took back its earlier answer.Agentwith a mocked model,drop("media").when()now raises, and withpreserve_recent=1the model still receives the image on the turn it was sent.I ran
hatch run prepareAdditional Details
Alternatives considered
utilizationkeep choosing granularity, and reject the conflicting combinations instead. Rejected because one condition would still mean two things, and drop's message and block forms would still behave inconsistently."*"has a threshold. This was an earlier version of this PR. Rejected because"*"would then mean blocks or messages depending on another condition, the same coupling this PR removes fromutilization.preview_tokensfor truncate. Rejected because it silently skipped media and small blocks, and truncate's preview already leaves content that fits unchanged."N%". This was an earlier version of this PR. Rejected because a number then meant a share or a count depending on its size: TypeScript can't tell1from1.0, and the two SDKs share these values, soutilizationof1meant 100% while2meant two tokens, and each condition needed its own parsing rules. Breaking experimental configs now is cheaper than graduating two spellings that can only be removed in 2.0.Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
🤖 Generated with Claude Code