Skip to content

feat(context)!: offload conditions decide granularity, and placeholders are typed (typescript) - #5325

Draft
lizradway wants to merge 59 commits into
strands-agents:mainfrom
lizradway:lizradway/offload-conditions-ts
Draft

lizradway wants to merge 59 commits into
strands-agents:mainfrom
lizradway:lizradway/offload-conditions-ts

Conversation

@lizradway

@lizradway lizradway commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

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. On main, setting utilization silently switches drop, truncate and summarize into whole-message mode. That causes three problems:

  • threshold picks messages rather than blocks.
  • utilization decides granularity as well as timing.
  • Summarize can merge messages that aren't next to each other into one summary, which reorders the content.

The new rule is that the target chooses what a strategy acts on, blockThreshold sets the minimum size to act on, contextUtilization only says when it fires, and preserveRecent only says what's protected. The conditions are renamed from threshold and utilization so each name says what it measures. Target "*" always acts on whole messages, and blockThreshold then measures each message. Every other target acts on blocks, and blockThreshold measures each block. When blockThreshold isn't set, there's no size check, so every matching message or block is acted on.

Target "*" (messages) Any other target (blocks)
drop removes eligible messages and keeps tool-use/result pairs together: [Dropped: N messages] replaces each block with [Dropped]
truncate with no blockThreshold, keeps the head and tail within previewTokens and elides the middle. With a blockThreshold, shortens each large message in place and leaves the newest one alone previews each block
summarize summarizes each contiguous run summarizes each block

Values follow one rule: a number counts in the condition's unit, and "N%" is a share of the whole. blockThreshold: '5%' selects blocks larger than 5% of the context window. Decimals like 0.85 are not accepted, so each value has one meaning in every condition.

This PR also folds in #5657, which makes an offload placeholder a typed fact instead of text the SDK has to pattern-match. Before, a placeholder was a plain text block ending in [ref: …], and the stash, the strategies, retrieve_context and truncation all recognised it with a regex, which is how #5620 happened. A placeholder is still a TextBlock with the same text, so providers, UIs, the tracer, A2A and sessions see what they see today, but metadata.offloaded now records the offload method that left it, the preview, the stash ref, and how many whole messages it stands for. Block metadata is never sent to the model. Building placeholders and wording every marker now happen in one module, context-manager/placeholder.ts, and the SDK identifies a placeholder by its metadata, never by matching its text. Landing both together lets message-level removal share one path in drop, summarize and the emergency cut, and lets the condition and target logic move out of base.ts into conditions.ts and target.ts.

Public API Changes

// Before
Offload.truncate('toolResults').when({ threshold: 2500 })
Offload.summarize('*').when({ utilization: 0.85 })

// After
Offload.drop('toolResults')                       // no conditions: drops each tool result as it arrives, and warns once
Offload.truncate('toolResults').when({ blockThreshold: '5%' })                  // tokens, or a share of the window
Offload.summarize('*').when({ contextUtilization: '85%', preserveRecent: 4 })   // 0.85 throws: write '85%'
Offload.drop('*').when({ blockThreshold: 1000 })  // drops old messages larger than 1,000 tokens
Offload.truncate('*').when({ blockThreshold: 1000 }) // shortens each of them in place
Offload.drop('media').when({ preserveRecent: 1 }) // new target: images, documents, video, audio
Offload.drop('*')                                 // throws on init: set contextUtilization, blockThreshold, or preserveRecent
Offload.drop('media')                             // throws on init: would remove media before the model sees it
  • utilization is renamed contextUtilization, and threshold is renamed blockThreshold. A "N%" blockThreshold is still a share of the context window, so '5%' selects content larger than 5% of the window.
  • Offload.* returns an OffloadStrategy: a ContextStrategy with a .when() that returns a copy under new conditions. .when() is optional. Without conditions, a strategy acts on each matching block as it arrives, before the model reads it, and logs a warning the first time it replaces a block. The OffloadStrategyBuilder type is removed.
  • Targets "*" and "media" can't run without conditions, so a bare Offload.drop('*') throws when it's attached to an agent. With .when(), the same check runs when the strategy is built.
  • A nullish entry in strategies throws a named error instead of a TypeError from inside resolveStrategies.
  • The new "media" target matches image, document, video and audio blocks outside tool results. It needs contextUtilization or preserveRecent. Without either, the strategy would run on each message as it arrives and replace the media before the model's first call.
  • A tool list can mix included and excluded tools: it acts on the includes minus the excludes, so ['bash', '!python'] acts on bash. A list whose excludes cancel every include (['bash', '!bash']) matches nothing and throws when it's built. Before, a mixed list logged a warning and ignored the excludes.
  • A whole-number contextUtilization counts conversation tokens and must be at least 1,000, so a missing % like 80 fails with a hint to write "80%". If it's larger than the model's contextWindowLimit, init throws. If the model has no contextWindowLimit, it warns and skips that check; the count still fires at the right size, since it doesn't depend on the window.
  • .when() throws on an unknown condition name, such as a misspelled utilisation.
// A placeholder keeps its text and records what it stands for
new TextBlock('[Dropped: 3 messages] [ref: removed-abc]', {
  metadata: { offloaded: { method: 'drop', preview: '[Dropped: 3 messages]', ref: 'removed-abc', removedMessages: 3 } },
})
  • TextBlock gains an optional metadata?: TextBlockMetadata, serialized only when set. TextBlockMetadata, OffloadedBlockData (method, preview, ref?, removedMessages?) and OffloadMethod ('drop' | 'truncate' | 'summarize') are exported; isOffloaded is @internal. Older SDKs reading a new session see plain text.

Behaviour changes for existing configs

  • A decimal contextUtilization or preserveRecent (0.85) throws an error whose message gives the "N%" to write. A number now always counts, so a value of 1, which meant 100%, is rejected too. The presets and the 'auto' and 'agentic' context managers use "N%".
  • Drop or summarize with a non-"*" target plus contextUtilization now acts on blocks.
  • drop("*") and summarize("*") now act on whole messages for every combination of conditions.
  • truncate("*") without blockThreshold is now middle-out.
  • Without a blockThreshold, a strategy acts on blocks of any size. Truncate no longer defaults the threshold to previewTokens, because its preview already leaves blocks that fit untouched.
  • Block strategies skip any placeholder, so truncate doesn't re-truncate its own previews and no strategy offloads another's marker. Middle-out truncate skips only its own elision markers, so a summary or drop marker can still be elided.
  • Message-level removals (drop, summarize and the emergency cut) stash the removed messages and leave a placeholder with a ref, so retrieve_context can return them. On main they were gone.
  • retrieve_context returns tool result items as blocks, so retrieved media reaches the model as media. Text is still cut to the result budget.
  • An offload ref stays resolvable after repairAlternation merges messages, because the stash stores under the ref it emits rather than the block's position.
  • Placeholders saved by sessions before this change are plain text and are treated as ordinary content. This replaces the TS ref regex guard from fix(context): never stash or serve offload previews as originals (py + ts) #5626; the Python side of fix(context): never stash or serve offload previews as originals (py + ts) #5626 is unchanged.
  • A block strategy gated by contextUtilization or preserveRecent leaves message 0 and the last message alone, as message mode already did.
  • blockThreshold must be at least 1, or a "N%" below "100%". A blockThreshold of 0 got past the "*" guard and acted on every message, and "100%" could never select anything.
  • Empty and whitespace-only text blocks are skipped.
  • A dropped media block becomes [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.
  • An offloaded media block names its kind the same way: [Offloaded: image, ~N tokens, removed from context after it was viewed].
  • [Offloaded: …] writes its token count with commas (~2,525 tokens), as the other markers do.
  • [Offloaded] and [Summarized: …] leave out the token count when it's 0, which is what the heuristic counter returns for media.
  • Truncate checks blockThreshold > previewTokens for every target, including "*", whose blockThreshold would 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.
  • overflowProtection drops the oldest messages instead of truncating them.

Risks / Callouts

  • A summary can still land after the message that follows it. This happens when the message right after a summarized run is an assistant message. insertMarker appends the summary to the end of that message to keep its stash block indices stable. main already 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.
  • Under the heuristic token counter, media size is unknown. The heuristic counts media as 0 tokens, so a media block always passes blockThreshold instead of never passing it, and its [Offloaded] marker leaves out the size. Estimating media size in the fallback counter is a separate follow-up.
  • The unconditioned warning fires for every strategy without conditions, including a bare truncate, which only shortens blocks larger than previewTokens anyway. It fires once per strategy instance, the first time the strategy replaces a block.
  • Known gaps carried over from feat(context-ts): record offload placeholders as typed TextBlock metadata #5657. Agentic-mode removals leave no marker, and storeRemoved stores a second copy of media bytes that were already stashed per block.
  • Python parity. feat(context)!: target and block threshold decide offload granularity (python) #5324 has the conditions redesign but not typed placeholders; the Python port of metadata.offloaded is a follow-up.
  • Experimental API, but breaking. The ! in the commits is deliberate. The renames break configs on main that use threshold or utilization; each now throws an unknown-condition error naming the accepted conditions.

Related Issues

TypeScript counterpart of #5324; prerequisite for #5205. Supersedes #5657. Related: #5620, #5626. Design context: team/designs/0022 (providerData on TextBlock) and #5587 (InjectedBlock).

Documentation PR

Docs will follow in a separate PR.

Type of Change

Breaking change

Testing

  • TypeScript: npm run lint, format:check, type-check (after npm run build, which the integ project needs), and the unit suite all pass (5,962 passed, 6 skipped).
  • Complexity: npm run complexity labels it complexity/medium (max 19, collectRemovableWithPair). The highest-scoring function this PR adds is _transformBlocks, at 13.
  • Placeholders, through ContextManager and Agent with a mock model: the [BUG] ContextManager: after a session restore onto an empty stash, retrieve_context serves the truncated preview as the original #5620 seeding case over an empty stash and over a stash holding the original, history merges before a per-block offload, and retrieval of removed messages.
  • Through an Agent with a mocked model: a bare Offload.drop('toolResults') dropped both tool results, kept their stash refs, and logged the warning once. A bare Offload.drop('*') or Offload.drop('media') threw, and Offload.drop('*').when({ contextUtilization: '80%' }) ran.
  • Not run: the integ test that needs Bedrock. It was changed to use previewTokens: 50.
  • Real-model runs were done on the Python side (feat(context)!: target and block threshold decide offload granularity (python) #5324), which shares this behavior. Both Haiku 4.5 and Opus 5 recalled earlier facts correctly after each "*" strategy and after a media drop.

Additional Details

Alternatives considered
  • Let contextUtilization keep 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.
  • Act on blocks when "*" 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 from utilization.
  • Default the threshold to previewTokens for truncate. Rejected because it silently skipped media and small blocks, and truncate's preview already leaves content that fits unchanged.
  • Require .when() on every strategy. This was an earlier version of this PR. Rejected because a bare strategy like Offload.truncate('toolResults') reads as what it does, and .when({}) added nothing. The runtime warning covers the case where a strategy runs without conditions by mistake.
  • Keep decimals as aliases for "N%". This was an earlier version of this PR. Rejected because a number then meant a share or a count depending on its size: 1 and 1.0 are the same number in JavaScript, so contextUtilization of 1 meant 100% while 2 meant 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.
  • A new block type for placeholders. Rejected, following Murat's suggestion in the design thread, because it would break consumers that switch on type.
  • Placeholder data on Message.metadata. Rejected because a message can hold several placeholders, and repairAlternation moves blocks between messages when it merges neighbours, so positional indices go stale on the first merge. A field on the block travels with it.

Checklist

  • I have read the CONTRIBUTING document
  • I have reviewed and understand every line of code in this PR, including any generated by AI tools, and I can explain why it works
  • My change is focused and reasonably small; I have split unrelated work into separate PRs
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

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

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.
@github-actions github-actions Bot added typescript Pull requests that update typescript code enhancement New feature or request area-context Session or context related complexity/low Touched functions have low cognitive complexity (<=10) size/l labels Oct 21, 2026
@lizradway

Copy link
Copy Markdown
Member Author

@strandly-the-agent pls review

Comment thread strands-ts/src/context-manager/strategies/offload/truncate.ts Outdated
@github-actions

Copy link
Copy Markdown
Contributor

Assessment: Comment

Clean, well-motivated refactor. The target/blockThreshold-decides-granularity rule is a clear improvement over utilization doing double duty, the single "number = count, N% = share" rule is consistent across conditions, and the breaking changes are deliberate and well-documented. Control flow stays flat (lookup-table condition rules, small helpers) and the tests are thorough.

Review notes
  • API review: This locks the public (though @experimental) Offload / OffloadConditions contract — renaming threshold → blockThreshold, reworking granularity, requiring .when() — explicitly ahead of graduation (feat(context): graduate ContextManager out of experimental #5205), where it can then only change in 2.0. The PR body makes a strong API case, but the PR carries no api/needs-review label. Given it's a graduation prerequisite, I'd get an API bar-raiser sign-off before merge.
  • Convergence guard: One inline note on truncate.ts — the numeric blockThreshold/previewTokens guard is bypassed by percent thresholds; worth a comment or removing it since isTruncated already guarantees convergence.
  • Acknowledged ordering risk: The "a summary can land after the following assistant message" case is called out and deferred to a follow-up; the behavior is locked by tests, so that's a reasonable split.
  • Tests: Strong — full-array equality assertions and parameterized tables rather than per-field checks, including a parseCondition boundary table. No gaps or duplication spotted.

Nice work locking down the semantics before graduation — the alternatives-considered section made the design easy to follow.

@strandly-the-agent strandly-the-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested — the granularity rule itself reads well and matches the Python PR, but routing non-"*" target + utilization to the per-block path exposes one regression: _applyPerBlock has no last-message guard, so Offload.drop('toolResults').when({ utilization: '70%' }) replaces the tool result the model is about to answer with [Dropped] (verified end-to-end with a real Agent; same config on main left it intact). One 🔴, three 🟡 inline. This also changes the public Offload contract ahead of graduation and carries no api/needs-review label.

✅ Verified, questions, reading order, appendix

Verified at c92baa17

  • npx vitest run src/context-manager → 454 passed. Delta fddda555a..c92baa17 is error text + a comment only; all findings re-run at the new head.
  • Repros (uploaded to the Strandly portal): e2e-live-message.test.ts (real Agent + MockMessageModel, 5 cases incl. a main baseline), adversarial-strategies.test.ts (19 cases), truncate-convergence.test.ts.
  • Python #5324 parity: names, 'N%'/decimal aliases, percent regex, error texts, _acts_on_messages rule and builder-not-a-strategy all line up.

Questions (non-blocking)

  1. utilization: 80 now means 80 tokens and fires on the first turn; on main a value >1 effectively never fired. The (0,1]-share / integer-count overload makes the '80%'→80 slip silent. Worth rejecting 1 < utilization < 100 with a "did you mean '80%'" error?
  2. Message-level markers ([Dropped: N messages], [... N messages elided ...], [Summarized: N messages …]) carry no stash refs, so the model can't retrieve what was removed even when a stash is on. Intended for now?
  3. blockThreshold on '*' silently flips the strategy from message mode to block mode; a non-'*' target can never act on messages. Is that the contract you want to graduate? (The github-actions review above also asked for an API pass.)

Reading order: base.ts _actsOnMessages / constructor → _applyPerBlock (🔴) → truncate.ts _applyPerMessage/_selectMiddle → summarize.ts _applyPerMessage/_summarizeRun → index.ts builder → tests.

Appendix — non-blocking (5)

  • ⚪ Percent (base.ts:44) appears in the exported OffloadConditions type but isn't exported from src/index.ts, so users can't name it.
  • ⚪ preserveRecent: '0%' parses fine then drop('*') throws "set utilization or preserveRecent" (base.ts:467-471) — confusing for a value the type invites.
  • ⚪ resolveStrategies crashes with Cannot read properties of undefined (reading 'apply') on a nullish entry instead of the friendly error (presets.ts:73); TS types prevent it, JS callers don't.
  • ⚪ drop('*') re-drops its own [Dropped: N] marker on later passes and resets N to 1 — pre-existing on main, same root cause as the truncate convergence comment inline.
  • Percent blockThreshold bypassing the numeric convergence guard — already raised by the github-actions review and addressed by the comment in c92baa17; not re-raised.

Comment thread strands-ts/src/context-manager/strategies/offload/base.ts Outdated
Comment thread strands-ts/src/context-manager/strategies/offload/summarize.ts Outdated
Comment thread strands-ts/src/context-manager/strategies/offload/truncate.ts Outdated
Comment thread strands-ts/src/context-manager/strategies/offload/truncate.ts Outdated
Restores the condition name main uses, so existing .when({ threshold }) calls keep working.
…xt window

A count above the model's context window can never be reached, so the strategy would only fire on overflow.
@lizradway

Copy link
Copy Markdown
Member Author

@strandly-the-agent pls review

@strandly-the-agent strandly-the-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving at a58a30de — all three fixes hold up against the original repros, and the threshold rename matches #5324. Still needed before merge: the api/needs-review label (public Offload contract changed pre-graduation) and a PR-body refresh — it still says blockThreshold and shows threshold → blockThreshold as the API change, which is now backwards.

Previous findings:

  • 🔴 per-block + utilization touched the live message → fixed (1f9e95e): _applyPerBlock pops the last message unless the strategy runs eagerly. Re-ran the gated drop('toolResults') and drop('*') + threshold cases: older tool result dropped, live one intact.
  • 🟡 summarize run throw left partial mutation → fixed (9ea9fa1): per-run catch, logs, continues; repairAlternation still runs when anything acted.
  • 🟡 middle-out truncate re-elided its own marker → fixed (f0833f4): marker carriers are excluded from eligible. Re-ran the churn case: pass 2 returns false, later passes append a second marker and the 11 messages elided count survives.
  • 🟡 tool pair straddling the head/tail boundary → withdrawn to ⚪ per your question; replied on the thread.
✅ Verified / notes
  • npx vitest run src/context-manager → 472 passed at a58a30de (was 454; new tests cover the live-message skip, the failed run, the marker stop, and the utilization window check).
  • Round-2 repro + log uploaded to the portal (followup-fix-verification.*).
  • utilization token count > context window now throws in init() (so at initAgent, not construction) — fine, just noting it's a runtime error on agent setup rather than at .when().
  • drop('*') still re-drops its own [Dropped: N] marker on later passes — pre-existing on main, not blocking; the same carrier-exclusion trick would fix it if you want parity with truncate.
  • Python #5324 also renamed back to threshold; builder/error texts still aligned.

@lizradway

Copy link
Copy Markdown
Member Author

@strandly-the-agent can you review the api for any potential bugs or edge case? does this make sense with every way the api could be used? is this clear and documented in the comments?

@strandly-the-agent strandly-the-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API pass at a58a30de — the shape holds up; two 🟡 edges and the docs lag the rules. Answering your three questions:

  1. Bugs / edge cases: no 🔴. Two 🟡 inline: block mode never protects message 0 (message mode does), and this PR adds two new routes into block mode ('*'+threshold, utilization-gated) so e.g. drop('userText').when({ utilization: '70%' }) now drops the task prompt; and the conditions object isn't validated at runtime — a typo'd key or undefined silently turns a gated strategy into an eager / unguarded one.
  2. Does every combination make sense: yes, with the rule as stated — I ran the full {drop, truncate, summarize} × target × conditions × value-form matrix (construction + apply()). Every invalid value throws with a message that names the fix. The cells worth a second look are in the fold.
  3. Is it documented: the interface-level JSDoc states the rule, but the property docs, TruncateConfig.previewTokens, and the Offload examples don't carry it — a user hovering utilization can't see that 1 is 100% and 2 is two tokens, previewTokens doesn't say it's the head+tail budget in message mode, and there's no example of the two modes you'd reach by accident (eager {} and utilization-gated block). Suggestions inline.

Still pending from before: api/needs-review label and the PR body (blockThreshold).

Matrix cells, what I verified, appendix

Matrix — fine but worth knowing

  • truncate(target).when({ threshold: N }) throws when N <= previewTokens (good), but 'N%' isn't checked: truncate(['tool::bash']).when({ threshold: '1%' }) on a 100k window = 1,000 tokens = the default preview, so matching blocks are counted and left unchanged with no log. Either resolve-and-check in init() (window is known there, like _checkUtilizationFits) or say so on previewTokens.
  • drop('*').when({ threshold: 0 }) is accepted and is the eager "drop every block of every message" that .when({}) on '*' rejects — same request, different spelling. '0%' is rejected. Consider rejecting 0, or document "threshold: 0 matches every block".
  • Omitting utilization doesn't mean "never": with only preserveRecent the strategy runs before every model call; with neither it runs on every arriving message including the one the model is about to answer (drop('userText').when({}) drops the user's prompt). Both are by design; neither is said on OffloadConditions.
  • preserveRecent: 1.0 is the count 1 (JS has no float 1.0); 0.99 keeps all matches (ceil) while '100%' is rejected — harmless inconsistency.
  • utilization token count on a model without contextWindowLimit measures against the default window silently.
  • summarize('*') with no aux model warns and returns false forever — fine, since a real Agent always has auxModel.

Verified: npx vitest run src/context-manager 472 passed; matrix tests (api-matrix.test.ts, 16 tests; construction-matrix.test.ts, 23 cells) + outputs uploaded to the portal. Python #5324 docstrings compared: Python repeats the value forms on every when() parameter and its when() is keyword-only (typo = TypeError); TS is better on OffloadTarget and the pipeline-ordering note.

Appendix — non-blocking / pre-existing (5)

  • ⚪ OffloadTarget says arrays are "namespaced with tool::" but the prefix is optional, and mixing tool::a with !tool::b keeps only the includes with a logger.warn (base.ts:418-421). validateTarget is the natural place to make that a construction error now that everything else throws.
  • ⚪ '*' doc says "tool results + text blocks" but block mode also offloads media blocks to [Offloaded: ~N tokens].
  • ⚪ Preset doc: staleToolCleanup is "all but the newest 5 tool-result messages", not "older than 5 messages"; proactiveSummarization omits preserveRecent: 0.7. Pre-existing text.
  • ⚪ Error threshold (1000) must be greater than previewTokens (1000) to ensure truncation converges says why, not what to do — append "raise threshold or lower previewTokens".
  • ⚪ One strategy instance shared by two agents: the eager hook reads this._stash from whichever init() ran last (base.ts:499-509). Pre-existing; a "one instance per agent" line on OffloadStrategyBuilder would do.

Comment thread strands-ts/src/context-manager/strategies/offload/base.ts Outdated
Comment thread strands-ts/src/context-manager/strategies/offload/base.ts Outdated
Comment thread strands-ts/src/context-manager/strategies/offload/base.ts Outdated
Comment thread strands-ts/src/context-manager/methods/truncate.ts
Comment thread strands-ts/src/context-manager/strategies/offload/index.ts Outdated
…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.
…with a threshold

Drop removes messages over the threshold and summarize condenses them, so
truncate previews their blocks rather than eliding them, which with a threshold
behaved the same as drop.
? resolveStrategies(config.strategies)
: [
Offload.truncate('toolResults', { previewTokens: TRUNCATE_PREVIEW_TOKENS }).when({
threshold: AUTO_TRUNCATE_THRESHOLD,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

old name faild loud right, it is expected.

For target '*', it doesn't measure blocks, The name is confusing in one of its two modes

readonly name = 'offload:truncate'

private readonly _truncateConfig: TruncateConfig
private readonly _previewTokens: number

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is from AI:

truncate('*') with blockThreshold can silently do nothing. It uses two sizes. blockThreshold picks the messages, then previewTokens decides which blocks inside them get cut (_shortenEach passes this._previewTokens to _transformBlocks). I tested a 6,000-token assistant message made of ten 600-token text blocks, with blockThreshold: 1500 and previewTokens: 1000. The message was selected, but apply() returned false and the message stayed at 6,000 tokens. The PR description says it "shortens each large message in place", which doesn't hold when a message is large because it has many small blocks.

…d conditions redesign

Merges strands-agents#5657 (offload placeholders as typed TextBlock metadata) into the
conditions redesign and replaces the text-matching logic both relied on:

- truncate recognizes offloaded content with isOffloaded instead of the
  "[Truncated:" prefix, and its own elision markers by their typed
  removedMessages count instead of a regex
- every strategy builds placeholders with offloadPlaceholder; the
  "[Stashed]" suffix and string[] stash refs are gone
- drop, truncate middle-out, each summarize run and emergency truncation
  share one removal path, so messages they remove are stashed and
  retrievable; apply() restores alternation once afterwards
- targets move into OffloadTargetMatcher (target.ts) and conditions into
  ResolvedConditions (conditions.ts), leaving base.ts the strategy flow
- EmergencyTruncateStrategy is a plain ContextStrategy
Resolves the strands-agents#5626 conflicts in favor of the typed offload placeholder check, which replaces the TS ref regex guard.
@lizradway lizradway changed the title feat(context)!: target and block threshold decide offload granularity (typescript) feat(context)!: offload conditions decide granularity, and placeholders are typed (typescript) Oct 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Issue: This commit adds new public types to the core message model — OffloadedBlockData and TextBlockMetadata.offloaded on TextBlock (folded in from #5657) — but the "Public API Changes" section only documents the Offload condition changes.

Suggestion: List TextBlockMetadata.offloaded / OffloadedBlockData in the Public API Changes section (what the preview/ref/removedMessages fields mean and that they're the inspectable placeholder shape), and confirm that API-bar-raising sign-off covers this addition to the fundamental message type, since it now ships as public surface rather than an internal marker-string convention.

@github-actions

Copy link
Copy Markdown
Contributor

Assessment: Approve

Re-reviewed at b48a5bd, which folds the typed offload placeholders into the conditions redesign. This is a clean, well-executed refactor: base.ts is substantially slimmer with targets extracted into OffloadTargetMatcher (target.ts) and conditions into ResolvedConditions (conditions.ts), and the typed metadata.offloaded + isOffloaded() guard replace the previous marker-string matching throughout. My prior-round finding (stale description) is resolved — the description now uses the renamed conditions, documents optional .when(), and includes an honest Risks/Callouts section.

Review notes
  • Correctness: The shared removeMessages path (drop *, truncate middle-out, each summarize run, emergency truncate) stashes removed messages under one ref and restores alternation once in apply(); summarize's contiguous-run splitting resolves the reordering problem. carriesElisionMarker keys off typed removedMessages + formatElided, which can't collide with drop's marker.
  • Testing: stash-integration.test.ts covers the removed-message stash/retrieve roundtrip (with full-object equality assertions) and the no-stash fallback; placeholder/marker behavior is well covered.
  • Docs/governance (suggestion): New public message-model types (TextBlockMetadata.offloaded / OffloadedBlockData) aren't listed in Public API Changes — see inline comment.
  • Note: Tests/typecheck weren't runnable here (dependencies not installed); review is a static read.

Nice modularization — the single removal path and typed placeholders are a clear improvement over the previous string-matching approach.

@github-actions

Copy link
Copy Markdown
Contributor

Assessment: Approve

Re-reviewed at 6b07652, which is the chore: merge main into offload conditions merge commit (resolving the #5626 conflict in favor of the typed offload placeholder check). Verified the conflict resolution landed cleanly:

  • No conflict markers remain anywhere under src/.
  • isOffloaded is still the typed guard (metadata?.offloaded !== undefined) — the TS ref-regex guard from the conflicting side is fully gone, as intended.
  • OffloadedBlockData, TextBlockMetadata.offloaded, and the offload module structure are all intact.

The rest of the merge pulls main in (Python counterpart, docs, tooling), which is outside the TypeScript change under review. No new code issues; my prior review and the earlier doc/API-surface suggestion still stand.

…e and record the offload method

Placeholder construction and every marker string now live in context-manager/placeholder.ts.
OffloadedBlockData gains a required `method` ('drop' | 'truncate' | 'summarize'), so code and
tests identify a placeholder by its metadata instead of matching its text.
@github-actions

Copy link
Copy Markdown
Contributor

Assessment: Approve

Re-reviewed at 3cb0d77 (refactor(context-ts): move offload placeholder wording into one module and record the offload method). This is a clean, well-scoped refactor and it resolves the API-documentation item I'd flagged in earlier rounds.

What I verified
  • Centralization: All placeholder/marker wording now lives in the new context-manager/placeholder.ts, and every consumer (base, drop, summarize, truncate, methods/truncate) imports from that one module. All 8 exports are consumed and every import resolves — no dangling references left by the move.
  • Typed method recording: OffloadedBlockData now carries method/removedMessages, backed by the new public OffloadMethod union. The SDK continues to identify placeholders by typed metadata (isOffloaded), with no text/regex matching anywhere — closing out the [BUG] ContextManager: after a session restore onto an empty stash, retrieve_context serves the truncated preview as the original #5620-class fragility.
  • Tests: placeholder.test.ts asserts full objects with toStrictEqual and uses a table-driven case set covering singular/plural and unmeasured (0-token) edges. Edge handling in marker()/approxTokens()/formatOffloaded() is correct.
  • Public API: The description (Public API Changes) documents TextBlockMetadata, OffloadedBlockData (method, preview, ref?, removedMessages?), and OffloadMethod as exported, with isOffloaded @internal — this fully addresses my prior open item.

Note: I couldn't run tsc/tests (no node_modules in the review env), so this round is static analysis only; the maintainer CI typecheck/test run remains the source of truth.

Nicely done — folding message-level removal onto one path and moving the wording into a single module reads much more maintainably.

@github-actions

Copy link
Copy Markdown
Contributor

Assessment: Approve

Re-reviewed at 5bd28c3 (refactor(context-ts): collapse the drop and summarize block paths and derive isMediaBlock from the media kinds). Another clean, incremental refactor with no behavioral change and good test coverage.

What I verified
  • isMediaBlock derivation: now computed from mediaKind()/MEDIA_KINDS, so the media-block type guard and the marker labels share one source of truth — a nice DRY win.
  • formatDropped consolidation: DROPPED_MARKER + formatDroppedMedia collapsed into a single formatDropped(mediaKind?); no stale references to the old names remain, all placeholder exports are consumed, and all imports resolve.
  • Collapsed block paths: drop and summarize _replaceBlock now share the block instanceof ToolResultBlock ? block.withContent([placeholder]) : placeholder wrap. The summarize media fallback (image summarization fails / video can't be summarized → formatOffloaded) is exercised in summarize-strategy.test.ts, and the zero-token media guard in base.ts is preserved.
  • Tests: placeholder.test.ts updated to cover both formatDropped() and the media variant, keeping full-object toStrictEqual assertions.

Optional (non-blocking): the instanceof ToolResultBlock ? withContent([…]) : … wrap idiom is now repeated in drop.ts and summarize.ts — a small shared helper on BaseOffloadStrategy could finish the collapse, but it's trivial and not worth holding the PR.

Note: I couldn't run tsc/tests (no node_modules in the review env), so this round is static analysis only; CI remains the source of truth.

@lizradway
lizradway marked this pull request as draft October 29, 2026 20:55
@lizradway

Copy link
Copy Markdown
Member Author

@strandly-the-agent pls review

@strandly-the-agent strandly-the-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round 7 at 5bd28c3 — the typed-placeholder + stash/retrieval work holds up under attack (14 adversarial repros green, incl. a real-Agent eager-truncate → retrieve_context round trip), but two of the new decisions need an API call before graduation, so this is a comment, not an approval.

  1. .when() optional again reverses round 1's fix: Offload.drop('toolResults') now destroys the first tool result on arrival and only then logs its one-time warn (base.ts:279, after the replacement). Minimum fix is to warn in init(); the real question is whether eager drop of unread text should be allowed at all when 'media' throws for the identical hazard.
  2. TextBlock.metadata.offloaded puts a context-manager concept on a stable core type, and OffloadMethod / OffloadedBlockData / TextBlockMetadata are exported from the stable src/index.ts with no @experimental tag — while everything else here is experimental. At minimum tag them; also consider a custom extension point to match MessageMetadata.
  3. Python #5324 has none of this: no typed placeholders, no new stash key scheme, no retrieval rewrite, still documents tu_abc123_0 refs. Either port before #5205 or split the TS-only parts out.

Smaller: [Dropped] discards the size it computed (inline). api/needs-review label still absent. I'm not repeating JackYPCOnline's open threads.

✅ Verified, passes, appendix

Verified at 5bd28c3: npx vitest run src/context-manager src/types agent.context-manager.test.ts → 577 passed; eslint + prettier clean; type-check errors only in untouched integ files (steering, notebook). Delta reviewed: 181260c..5bd28c3 (12 commits; whole PR is now 40 files, +3112/−1198).

Passes: adversarial (completed, 14 repros) and API bar-raiser (completed) — reports + repro tests uploaded to the portal as r7-*. The correctness/tests pass timed out before writing anything; I covered its key items myself: provider leakage (five adapters have tests; Bedrock builds { text: block.text } so metadata can't leak by construction), warn placement, exports, Python parity.

Attacked and fine: stash round trip through a real Agent (original 11,489 chars returned byte-for-byte); multi-block tool results keep index order; drop-after-truncate doesn't stash the placeholder and still resolves the original; removed-<trackingId> ids retrieve dropped messages and pattern search works across them; prefix/underscore ids (tool_1 vs tool_10 vs tool) all resolve correctly; .when() copies are independent (no shared stash or warn state); toJSON/fromJSON and repairAlternation preserve metadata; '*' with no conditions throws at init(); blockThreshold: '100%' and old utilization key rejected with the expected text.

Appendix — non-blocking (5)

  • ⚪ Sessions persisted before this PR carry [ref: 1_toolu_abc_0]; the tool now answers "not found" with no format hint — fine for a feat!, but worth a line in the PR body.
  • ⚪ [ref: …] sits after the preview tail, up to previewTokens away from the [Truncated: …] header the model scans; folding it into the header would help.
  • ⚪ A tool result that contains an image is dropped as [Dropped], not as an image (mediaKind is evaluated on the ToolResultBlock).
  • ⚪ Pre-existing: a tool result reusing an earlier toolUseId overwrites that stash entry (stash.ts key ${toolUseId}_${blockIndex}), so both placeholders resolve to the later result.
  • ⚪ removedMessages counts only the current pass when a later pass removes a marker's carrier (chain is still retrievable); design doc 0015 still says threshold/utilization/skipRecent.

acted = true
}
}
if (acted) this._warnUnconditional()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The unconditioned-strategy warn fires after the damage. This is the only call to _warnUnconditional(), and it runs after message.content[blockIndex] = replacement — so for Offload.drop('toolResults') the first tool result the model was about to read is already [Dropped] when the log line appears (api-pass probe: warn calls after init: 0 | after first arrival: 1 | block dropped: true). Round 1 made .when() mandatory to close exactly this; 'media' still throws for the same hazard (base.ts:146-149).

Suggestion (minimum): warn in init() where _conditions.isEmpty and _runsEagerly are already known, before any history is touched:

  init(agent: LocalAgent, stash?: Stash): void {
    this._stash = stash
    this._checkRunnable()
    this._conditions.checkFits(agent)
    if (!this._runsEagerly) return
    this._warnUnconditional()
    agent.addHook(MessageAddedEvent, …)

Bigger question for the API pass: should eager drop of unread text (toolResults/assistantText/userText) throw like 'media' does, leaving toolResultErrors and tool arrays (where the target is the condition) as the eager-drop cases design 0015 §2.5 actually names? Python has the same post-hoc call (base.py:713-714), so whichever you pick should land in both.

Comment on lines +229 to +232
/**
* Offload method that replaced content with a placeholder.
*/
export type OffloadMethod = 'drop' | 'truncate' | 'summarize'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stable core type now carries an experimental context-manager concept, exported stable. OffloadMethod, OffloadedBlockData and TextBlockMetadata have no @experimental tag and are exported from src/index.ts:70-72 under Message types, while every other context-manager symbol is under Context Manager (experimental) or experimental/index.ts. Once this ships, the shape of offloaded is 1.x-stable even though the strategies that write it aren't. Also, sibling MessageMetadata has custom?: Record<string, JSONValue> as its extension point; TextBlockMetadata has only offloaded, so the next feature needing block metadata has no door.

Minimum: tag the three @experimental and add custom for parity. Alternative worth a look: keep TextBlockMetadata { custom? } core-agnostic and move OffloadedBlockData + isOffloaded to context-manager/placeholder.ts, exported from experimental/index.ts (cost: the guard narrows via a cast).

Suggested change
/**
* Offload method that replaced content with a placeholder.
*/
export type OffloadMethod = 'drop' | 'truncate' | 'summarize'
/**
* Offload method that replaced content with a placeholder.
*
* @experimental
*/
export type OffloadMethod = 'drop' | 'truncate' | 'summarize'

logger.debug(`trackingId=<${message.trackingId}> | dropped block from context window`)
return new TextBlock(marker)
logger.debug(`trackingId=<${message.trackingId}>, block=<${block.type}> | dropped block from context window`)
const placeholder = this._placeholder(formatDropped(mediaKind(block)), stashRef)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [Dropped] tells the model nothing about what it lost. tokens is computed for every block (base.ts:268) and passed here as _tokens, then ignored; truncate and summarize both say ~N tokens. A model deciding whether to call retrieve_context has no size to go on, and a dropped tool result that contained an image reads as plain [Dropped] because mediaKind is evaluated on the ToolResultBlock, not its content.

Suggestion: formatDropped(tokens, mediaKind) → [Dropped: ~1,300 tokens] / [Dropped: image, …], mirroring formatOffloaded.

Suggested change
const placeholder = this._placeholder(formatDropped(mediaKind(block)), stashRef)
const placeholder = this._placeholder(formatDropped(tokens, mediaKind(block)), stashRef)

(and rename the parameter from _tokens to tokens).

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-context Session or context related complexity/medium Touched functions have moderate cognitive complexity (11-25) enhancement New feature or request size/xl typescript Pull requests that update typescript code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants