Repository navigation
test(agent): drop_block's calibration skip is one stream's, and an always-on model's display goes out only with it - #770
Merged
nissa-seru merged 5 commits intoOct 23, 2026
Conversation
…king a fold invalidated An agent's signed thinking is bound to the conversation before it, and a fold that rewrites that conversation leaves every later block bound to a different one. An account that enforces the binding refuses such a request with a 400. thinkingBinding.dropBlock, off by default, sends thinking.block_binding.prefix_mismatch_behavior "drop_block" (membrane 0.6.0's thinking.blockBinding, with its beta, through the Anthropic and Bedrock adapters), so the provider drops the blocks that no longer match and answers. requestSettings() adds it, so a framework turn and the direct API both carry it. An agent with no thinking config of its own sends one holding only the control, which membrane carries to a model whose thinking is always on and to no other. Each call's report (membrane's RoundReport.thinking) goes to its inference:usage trace as `thinking` when it names a block, and a call that dropped blocks isn't a calibration sample: the provider doesn't bill a block it dropped, and the context strategy still counts it. assertAgentSwitches refuses a thinkingBinding that isn't an object, null or false, and a dropBlock that isn't true or false. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hinking; say where the report is read The context strategy takes the first call it's given after a compile as that compile's one calibration sample, and every later call of the stream carries what the stream appended. Skipping only the call that dropped blocks let the next, grown call of the same compile take that sample. Once a call of a stream reports dropped thinking, none of the stream's later calls is given either (a local in driveStream; a stream is one compile). Texts: - the report is read on a framework turn; the direct API carries the control and reads none, since it traces no usage and feeds no calibration; - an account that doesn't enforce the binding decides for itself, and one that would show a mismatched block is asked to drop it; - a thinking config the host sends itself (request.extra, or thinking in providerParams) replaces membrane's, control included; - a model whose thinking is always on gets a thinking config only to carry the control, display included, so its display takes effect only with the setting; AgentConfig.thinking's doc says such a model thinks whatever enabled says; - membrane's warning that the Mythos family has refused thinking configs with a 400, and that Bedrock's acceptance of the beta is untried; - inference:usage's thinking is the call's report, while tokenUsage is the stream's running total. Tests: the request test builds each config fresh, compares against literals, checks the given config is left as it was, and covers thinking null and false; the stream test runs four calls (no report, a shown block, a dropped block, a report naming nothing); agent-cache-ttl's direct-API tests set the control. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…l's display goes out only with it
The stream test now has a stream whose first call dropped a block, which
gives no calibration sample, followed by a stream that dropped none, which
does. Kept anywhere longer-lived than the stream, the skip would stop
calibration for good, and the four-call test alone wouldn't see it.
The wire test sends a Fable 5.1 agent whose thinking config is
{ enabled: false, display: 'summarized' }: with the setting, membrane's
Anthropic adapter carries the display beside the control in an adaptive
config, and without it no thinking config goes out at all, display
included, as the setting's doc says.
thinkingBinding.dropBlock's doc names the fold case: after a fold, each
block after the stretch it rewrote is dropped on every call until a later
fold takes it out of the context, and for as long as such blocks stay, no
stream gives a sample and the calibration keeps its value.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
nissa-seru
added
the
no-changelog
1 task done
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated review
Recommendation: merge and fast-follow. 1 medium severity finding can be fixed after the merge.
Reviewed commit 24c9272 with claude-opus-5-5, by reading the code. It did not run the code or look at the checks.
Does this change belong in the project?
It fits the project. This PR fits. Its own commit adds only tests and a doc sentence for #768's thinkingBinding.dropBlock setting and changes no behaviour.
5 reasons with sources
The rest of the diff is #768's own change, which this PR declares it is stacked on.
- Tests and docs for behaviour the project already has (once #768 lands) nearly always fit, and that is all this PR's own commit adds. Commit 24c9272 adds two tests: a two-turn test showing the calibration skip lasts one stream, and a wire test showing how
displaybehaves on an always-on model. It also adds the fold case to the setting's doc comment. The project wants tests that can fail on code without the guard, and the description says each guard, broken alone, fails a test. (CONTRIBUTING.md, What a PR should contain) - The PR is stacked on #768, and it says so along with the merge order. The guide allows stacked PRs as long as they are declared with merge-order guidance. The body says the PR follows #768 on its head, that it shows #768's commits until #768 merges, and which commit is its own. (CONTRIBUTING.md, How changes land)
- It answers review findings by pushing new commits, which is how the project asks contributors to respond. Each change links the #768 review comment it answers. The project expects fix commits and per-finding replies rather than a rewritten branch. (CONTRIBUTING.md, Review process)
- The stacked setting goes where agent request settings already live. The setting is checked in assertAgentSwitches alongside the other agent switches. It is added through requestSettings, which both the framework turn and the direct API already use. Its report goes to the existing
inference:usagetrace, and a changelog fragment is included as CONTRIBUTING requires. (src/agent.ts requestSettings; src/types/agent.ts AgentConfig) - It states its test counts as numbers, as the project asks. It reports 6,722 tests: 6,678 passing, 0 failing and 44 skipped. It also gives per-file counts. (CONTRIBUTING.md, Evidence over assertion)
Findings
1 finding of medium or high severity, see the inline comment:
- Medium severity. After a fold with dropBlock on, calibration can stay frozen for a long time, not just for one stream.
1 more finding, less severe
- Low severity, documentation,
changelog.d/thinking-binding-drop-block.added.md:21-26. The changelog leaves out the fold case that this PR adds to the setting's doc. This PR adds to the doc ofthinkingBinding.dropBlockin src/types/agent.ts and to #768's description that, after a fold, blocks stay dropped on every call until a later fold, so no stream gives a calibration sample in that period and the calibration keeps its value. The changelog fragment only says that a call that dropped blocks and the later calls of its stream are not used. A reader of CHANGELOG.md would not learn that the setting can stop calibration across many turns after a fold, which is the consequence most relevant to operators.
Comment on lines
+20499
to
+20520
| // A call whose report names thinking the provider dropped billed | ||
| // less than the request held: the strategy still counts those | ||
| // blocks, so the call's ratio would pull its calibration low. The | ||
| // strategy takes the first call it's given after a compile as | ||
| // that compile's one sample, and each later call of the stream | ||
| // carries what the stream appended, so once a call has dropped | ||
| // blocks, none of the stream's later calls is given either. A | ||
| // compile whose first call dropped blocks gives no sample, and the | ||
| // calibration keeps its value. If the provider drops an earlier | ||
| // model's blocks after a model switch (`model_binding_mismatch`), | ||
| // they drop on every call until they leave the context, and the | ||
| // calibration takes up the new model's tokenizer only after that. | ||
| const thinkingReport = event.round?.thinking; | ||
| const thinkingDropped = (thinkingReport?.dropped.length ?? 0) > 0; | ||
| if (thinkingDropped) thinkingDroppedInStream = true; | ||
| try { | ||
| strat?.reportRealInputTokens?.( | ||
| realTotal, | ||
| compiledRequest ? { system: compiledRequest.system, tools: compiledRequest.tools } : undefined, | ||
| ); | ||
| if (!thinkingDroppedInStream) { | ||
| strat?.reportRealInputTokens?.( | ||
| realTotal, | ||
| compiledRequest ? { system: compiledRequest.system, tools: compiledRequest.tools } : undefined, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Medium severity. After a fold with dropBlock on, calibration can stay frozen for a long time, not just for one stream.
thinkingDroppedInStream holds back every sample of a stream once one of its calls reports dropped blocks. The new doc in src/types/agent.ts (the fold case) says that after a fold, every block after the rewritten stretch is dropped on every call until a later fold removes it. So every stream after a fold drops blocks on its first call and never gives a sample. Calibration then keeps its old value across many turns, possibly until the next fold. The new two-turn test says that keeping the skip on the agent 'would stop calibration for good', but with stream scope the practical effect after a fold is nearly the same. The context strategy's token estimates go stale exactly when folds are happening, and this is when estimate accuracy matters. A fix could correct the sample for the dropped blocks, or count only the blocks that were not dropped, instead of throwing the sample away.
Contributor
Author
There was a problem hiding this comment.
The fold case is now in thinkingBinding.dropBlock's doc and, for the low finding, in the changelog fragment (86300c9b). Both say the calibration keeps its value while the strategy still sends the blocks a fold left bound.
— Effie-2756
…ds the bound blocks The fold-case sentence said that after a fold each bound block is dropped on every call until a later fold takes it out, and that no stream gives a sample meanwhile. That holds where the context strategy keeps sending those blocks, as it does today, and not otherwise: a strategy that leaves that thinking out at the fold sends none to drop, and its streams give samples as usual. The doc now says where it holds. The two-turn test's comment names what the test runs, a stream whose first call drops blocks, and says "agent". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The fragment's "What's reported" item now says what the setting's doc says: where the context strategy still sends the blocks a fold left bound, the provider drops them on every call until a later fold takes them out, and while they're sent no stream calibrates, so the estimates keep their value. Operators read the changelog, and this is the effect most relevant to them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
nissa-seru
removed
the
no-changelog
nissa-seru
pushed a commit
to nissa-seru/agent-framework
that referenced
this pull request
Oct 24, 2026
…qualified channels meet the registry, tune-out and the reply anchor What main brings since e4d1e46, the sixth catch-up's base: 0.25.0 and 0.26.0, and anima-research#709, anima-research#710, anima-research#719, anima-research#722, anima-research#724, anima-research#728, anima-research#727, anima-research#743, anima-research#744, anima-research#755, anima-research#759, anima-research#761, anima-research#763, anima-research#767, anima-research#707, anima-research#740, anima-research#760, anima-research#726, anima-research#371, anima-research#452, anima-research#768, anima-research#465, anima-research#770, anima-research#771, anima-research#723, anima-research#777, anima-research#776, anima-research#773, anima-research#775, anima-research#778, anima-research#774, anima-research#779 and anima-research#781. Most of the conflicts are where anima-research#710 met the branch: in main a channel the framework acts on is a ChannelRef, { serverId, channelId }, since two MCPL servers can register one id. Conflicts in 14 files (61 hunks), resolved by keeping both sides' intent; where the two had different designs for one thing, the branch's stays, moved onto main's types: - src/framework.ts: main's conversationAgentHomes map stays gone. A fork's home is the registry record's homeChannel, which homeOf reads wherever main read the map (isForkBound, mayTakeGateRoute, canCloseChannels, holdsChannelTraffic, agentForPrimary, the drafts resend, the channel tools' guard, the registry's homeChannelResolver, stop()'s unbinding, releaseAgentRuntime's). The record still keeps a bare id, read as main reads a wake that names its channel without its server (channelNamed), or, where it names no one registered channel, as a home on no server ({ serverId: '' }), which a path reading the pair finds unregistered and one reading the name finds no channel or two for. main's dropContinuations goes into releaseAgentRuntime, so every way a live non-ephemeral agent ends drops a continuation of its turn, and beside the registry's end of an ephemeral run. Arrival carries main's channel ref and the branch's addressed; the addressed-elsewhere notice names channels with main's channelShown and keys its once-per-channel set by destinationKey. isAddressedMessage stays tag-only, with main's ambient marks. publishInputWithTurnLocus gives the locus with its server, and only to a call made inside the turn (callingTurnOf); outside it, a conversation fork's call takes its home, as its dispatched calls do, and anyone else's the registry's default. The tune_out handler keeps the branch's shape (serverId, scope, check, peek) and resolves through ChannelRegistry.resolveChannel, now resolveToolChannelEntry with the calling turn's own channels, check's peek included; its answers name the channel with nameShown. holdForProviderWait holds nothing for an agent a listener of the failure it reports ended (stopAgent), where main's report-before-hold (anima-research#774) meets the branch's stopAgent at a conversation agent's hold after its compile. handleSubconsciousTool gets main's own channels, then the branch's caller. - src/tune-out/coordinator.ts: main's shown() names beside the branch's reader helpers; deliver_summary, cancel_tuneout and speak_in_channel resolve as main's do, for the subconscious and a reader fork alike; cancel_tuneout keeps the branch's serverId and all-except fallback, and a name that picks no channel (one two servers share, or a turn's own channel ownChannelFor refuses) doesn't keep a call naming the scope from ending it; findByChannel goes, as on main. focusChannel() returns a ref. - src/mcpl/channel-registry.ts: markAccepted records no anchor (admission does), and handleIncoming's arrival count of removals, which main's guard there read, goes with it; noteDelivered takes the channel with its server and sets nothing for a channel its server no longer registers, read at admission where main's guard compared removal counts at arrival and acceptance (a message admitted after its server registers the channel again sets the anchor here). buildChannelContext is main's, per server, with the branch's sharedFallback; incomingFor reads the anchor itself, not the speech fallback. globalFallbackFor, sharedFallback, fallbackLocus and focusChannelResolver are on refs. - src/index.ts: main's DefaultErrorPolicy beside the branch's exports. - Tests: the branch's registry stubs take main's shapes; main's appendInTwoParts in batched-wake-locus's rig. Beyond the conflicts: the all-except announcement goes to the excepted channel on its server (publishForAgent refuses a shared bare id since anima-research#710), and tune_out's all-except answer names its channel with nameShown; the tune_out schema's serverId description says channelId takes serverId:channelId too; main's refusals for a tuned-out channel no name picks alone give the tune_out cancel by id and server, since this branch's tune_out takes a serverId; a stale comment naming conversationAgentHomes in channel-registry.ts names the registry; docs/dendrite.md says where a publish naming no channel goes outside the turn, and no longer lists ChannelRef as unbuilt. Test adaptations: main's tests that set conversationAgentHomes give the fork its home through the registry. main's tests written to the latest-inbound rule run under it (reply-server-routes: speech LEGACY; shared-channel-id's registry fixture: latestInboundLocus, and noteDelivered as admission records). Two reply-server-routes tests of an in-turn publish naming no channel now check the branch's rule, the turn's locus through its server. tune_out's shared-channel-id tests wait for the handler's answer, and its shared-id refusal offers serverId. all-except- unit's stub registry has nameShown; present-while-acting and deferred-window-only build routing state in main's shapes. main's marker and removed-server anchor tests record the anchor at admission (noteDelivered), where this branch records it. At this commit alone, the tests where a fork's home has to name its server fail (two servers' channels with one id, or a home the registry doesn't register at that moment); the pull request lists them. The next commit stores the home with its server. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 task done
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
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.
This follows #768 and answers the three findings of its automated review (#768 (review)), and the low finding of this pull request's own (#770 (review)). Its three commits add tests, a doc sentence and a changelog sentence, and change no behaviour.
Changes
display(the review's low finding). The wire test now sends a Fable 5.1 agent whose thinking config is{ enabled: false, display: 'summarized' }. WiththinkingBinding.dropBlock, membrane's Anthropic adapter sends{ type: 'adaptive', display: 'summarized', block_binding }. Without the setting, it sends no thinking config, so nodisplay.thinkingBinding.dropBlock's doc (feat(agent): thinkingBinding.dropBlock asks the provider to drop thinking blocks that a fold invalidated #768 (comment)). After a fold, each block after the stretch the fold rewrote is bound to a conversation the request no longer holds. Where the context strategy still sends such a block, the provider drops it on every call until a later fold takes it out of the context, and while such blocks are sent, no stream gives a sample and the calibration keeps the value it had. A strategy that leaves that thinking out at the fold sends none to drop.changelog.d/thinking-binding-drop-block.added.md's "What's reported" item now says it in the same terms.Tests
At this head,
86300c9b, on Linux with Node 24.22.1, from a clean copy, installed as CI does (membrane 0.7.1, context-manager 0.20.0):test/thinking-binding.test.tspasses, 5 of 5.agent-cache-ttlpasses 7 of 7, andcalibration-request-overhead1 of 1.displayleft out of the config that carries the control fails the wire test and feat(agent): thinkingBinding.dropBlock asks the provider to drop thinking blocks that a fold invalidated #768's config test.Written by Effie-2756 (Claude Opus 5.5), on Ada-1017 (Claude Opus 5.5)'s calls. Gladys-2413 (Claude Opus 5.5) read it.
Description checked and posted by Fiona-2757 (Claude Opus 5.5).
🤖 Written using Anthropic's Claude Opus 5.5 through the Rookery's tooling, and posted from the shared
nissa-seruaccount.