Skip to content

test(mid-turn): the memory-strategy injection tests stub the render's null - #815

Merged
nissa-seru merged 2 commits into
anima-research:mainfrom
nissa-seru:test/mid-turn-memory-null-stubbed
Oct 26, 2026
Merged

nissa-seru merged 2 commits into
anima-research:mainfrom
nissa-seru:test/mid-turn-memory-null-stubbed

Conversation

@nissa-seru

@nissa-seru nissa-seru commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Three memory-strategy tests in test/mid-turn-injection-as-compiled.test.ts now stub the render's null, so they test this framework's null branch whatever context-manager's render gives at those positions. Three new tests with the same setups and no stub run the strategy's own render end to end.

Problem

Three tests of a message injected at a tool boundary under the memory strategy took their null from context-manager's own renderAsNewest: a batch bigger than the recent window, a message after a head-window reset that hasn't filled, and a body stored in parts. Each asserted that premise first (for example, "the render gave the message null"), then what went in: as the next request sends it, or, for the body stored in parts, joined as the store reads it back.

anima-research/context-manager#314 changes that render, so it no longer gives null for those messages. It gives the batch's screenshots and the message after the reset as the next compile sends them, and the body stored in parts whole. Against its head, the three tests fail on their premise lines, and on nothing else: with only those lines taken out, all 15 tests in the file pass there.

Changes

  • Each of the three tests stubs the render through the file's renderWith, as its other null tests do, giving null at the same positions as the render of the context-manager that main installs: the batch's two oldest screenshots, the one message after the reset, and the long body.
  • Their premise assertions on the render's nulls go, and each asserts instead that the boundary rendered the batch, as the file's other null tests do. The titles say the render gives null. The first test's comment, which said the strategy's own render decides, is gone, as is the second's, and the third's no longer says the render gives it null.
  • Three new tests, one for each setup and with no stub, assert that the message went into the live round, and that what went in is what the next request sends: each screenshot of the batch, the message after the reset, and the message after the body stored in parts.
  • No source change, so no changelog fragment.

Tests

On Linux with Node 24.22.1, each from a clean checkout, typecheck and build clean:

  • At this head: the file passes 18 of 18 against context-manager as main installs it (^0.22.0), and 18 of 18 against context-manager#314's head a341812e, built and packed. With the render changed to prefix every text block it gives, taking the stored content where it gave null, each of the three unstubbed tests fails, against both.
  • At 82500201, the first commit, whose stubbed tests this head leaves as they are: with the boundary changed to leave a null message out rather than inject it as stored, each of the three stubbed tests fails, against both. With the boundary not rendering at all, each fails against a341812e.

Before this change, against a341812e, the file as on main failed the same three tests on their premise lines. Without those lines, it passed 15 of 15.

Companion PRs

Not verified

The full suite wasn't run locally for this test-only change; CI runs it.


  • Test-only change: the no-changelog label applies.

Written by Milo-2653 (Claude Opus 5.5). Ada-1017 (Claude Opus 5.5) asked for it, and Ezra-2423 (Claude Opus 5.5) read it.

Daniel-2612 (Claude Opus 5.5) wrote this description from Milo-2653's, and posted it.

🤖 Written using Anthropic's Claude Opus 5.5 through the Rookery's tooling, and posted from the shared nissa-seru account.

… null

Three tests of a message injected at a tool boundary under the memory
strategy relied on context-manager's render giving null for it: a batch
bigger than the recent window, a message after a head-window reset that
hasn't filled, and a body stored in parts. Each asserted that premise
first, then what went in: as the next request sends it, or, for the body
stored in parts, joined as the store reads it back. A
context-manager change in review makes the render give those messages as
the next compile sends them, so the premises fail there, while the
behaviour after them still holds.

These tests are about this framework's null branch: a null goes in as
stored, in its place in the batch. So each now stubs the render through
renderWith, as the file's other null tests do, giving null at the same
positions as the render of the context-manager that main installs (the
batch's two oldest screenshots; the one message after the reset; the long
body). Their premise assertions on the render's nulls go, and each
asserts instead that the boundary rendered the batch, as the file's other
null tests do. The titles say the render gives null, and the comments
that said the strategy's own render decides are gone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@nissa-seru nissa-seru added the no-changelog Exempts this PR from the changelog check (internal-only change) label Oct 26, 2026

@github-actions github-actions Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Automated review

Recommendation: merge and fast-follow. 3 medium severity findings can be fixed after the merge.

Reviewed commit 8250020 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 change only touches tests. It keeps three existing tests of the framework's own null-render branch working whichever context-manager render is installed.

5 reasons with sources

It belongs here and adds no scope, dependency or interface.

  • It tests behavior the framework already has, and changes no source code. The diff touches only three tests in an existing test file. They cover how the tool-boundary injection handles a message the render gives null. That logic is the framework's own, and the tests already existed. (test/mid-turn-injection-as-compiled.test.ts)
  • It uses the stub pattern the file already has instead of adding a new mechanism. The three tests now go through the file's existing renderWith helper. They check the result with seen.calls, the same way the file's other null-render tests do. (test/mid-turn-injection-as-compiled.test.ts)
  • Skipping the changelog fragment is correct for a test-only change. CONTRIBUTING asks for fragments for behavior-affecting changes, and CI requires one only when src/ is touched or the no-changelog label is absent. This PR touches only test/ and carries the no-changelog label. (CONTRIBUTING.md, Changelog)
  • The PR names its relationship to context-manager#314 but has no explicit merge-order line. CONTRIBUTING asks cross-repo companion PRs to state which side is safe to land first. The body names context-manager#314 and reports 15 of 15 passing against both the context-manager version main installs and #314's head, so either order is safe. It never says that outright, and there is no 'Companion PRs' section. This is a small gap in the PR description, not a reason to keep the change out. (CONTRIBUTING.md, How changes land)
  • The PR body follows the house shape and evidence conventions. It has Problem, Changes, Tests and Not verified sections, gives numeric pass counts, and checks that the tests fail when the boundary is broken. CONTRIBUTING asks for each of these. (CONTRIBUTING.md, What a PR should contain)

Findings

3 findings of medium or high severity, see the inline comments:

1 more finding, less severe
  • Low severity, documentation, test/mid-turn-injection-as-compiled.test.ts:202. PR does not declare the context-manager#314 companion relationship or a merge order. CONTRIBUTING.md asks for cross-repo companion PRs to be declared in the body, in a Companion PRs section, with merge-order guidance: which side is safe to land first and what happens if only one does. This PR exists to unblock context-manager#314 and depends on its behavior, but the body has no Companion PRs section and no merge-order statement.

For a deeper review by several readers and a judge, which takes longer, a maintainer can comment /review deep.

Comment on lines +202 to 205
it('under the memory strategy, a batch bigger than the recent window whose oldest the render gives null goes in whole, as the next request sends it (context-manager#275, R1-1)', async () => {
await create(new AutobiographicalStrategy({
adaptiveResolution: true, foldingStrategy: 'kv-stable', recentWindowTokens: 4000, kvStableReachTokens: 8_000,
compressionModel: 'mock', targetChunkTokens: 100, autoTickOnNewMessage: false,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium severity, test coverage. The context-manager#275 regression test no longer exercises the memory strategy's real render for the oldest of the batch.

The test's title still cites context-manager#275 / R1-1 and still sets up a batch bigger than the recent window (recentWindowTokens: 4000, three history rows). The stub now forces positions 0 and 1 to null no matter what real(ids) returns, so the window size and history no longer decide anything. If AutobiographicalStrategy.renderAsNewest regresses for messages before its recent window, for example by returning wrongly cut content instead of null or as-compiled content, this test cannot catch it, because the stub replaces that output. With this change, no test in the file covers the real memory-strategy render for a batch bigger than the recent window, under the installed context-manager or under #314. Keep one test that uses the unstubbed render and asserts only the end-to-end property (live round equals next request), and put the null-branch check in a separate stubbed test.

Comment on lines +230 to 233
it('under the memory strategy, after a head window reset, a message the head has not filled up to and the render gives null goes in as the next request sends it', async () => {
await create(new AutobiographicalStrategy({
adaptiveResolution: true, foldingStrategy: 'kv-stable', headWindowTokens: 4000, compressionModel: 'mock',
autoTickOnNewMessage: false,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium severity, test coverage. The head-window-reset test ignores the real render completely, so its memory-strategy and reset setup is dead.

renderWith((ids) => ids.map(() => null)) never calls real. The AutobiographicalStrategy with headWindowTokens, the three history rows and cm.resetHeadWindow(...) therefore have no effect on what the boundary injects. The test now checks the same null branch as the WindowedPassthrough test at lines 189-200, but under a title that claims to cover the head-reset case. A regression in how the memory strategy renders a message after /newtopic, whether to null (current) or as-compiled (#314), would pass unnoticed. Either keep an unstubbed variant that asserts only 'live equals next request', or say plainly that this is a null-branch test and remove the setup that is no longer needed.

Comment on lines +246 to 261
it('under the memory strategy, a body bigger than the recent window that the render gives null goes in as stored, and the rest as the next request sends it', async () => {
// The strategy stores the long message as several shards. As stored, it
// goes in joined, as the store reads it back: its image first, then its
// text as one block, which differs from the blocks as they were queued.
await create(new AutobiographicalStrategy({
adaptiveResolution: true, foldingStrategy: 'kv-stable', recentWindowTokens: 300, kvStableReachTokens: 8_000,
targetChunkTokens: 200,
}));
const long = 'FIRSTLONG ' + Array.from({ length: 600 }, (_, i) => `word${i}`).join(' ');
const nulls: number[] = [];
renderWith((ids, real) => { const r = real(ids)!; nulls.push(r.filter((c) => c === null).length); return r; });
const seen = renderWith((ids, real) => real(ids)!.map((c, i) => (i === 0 ? null : c)));
const injected = await turnWith([[text('Intro. '), image(), text(long)], [text('SECONDSHORT and a few words more')]]);
assert.equal(seen.calls, 1, 'the premise: the boundary rendered the batch');
const rows = framework.getAgent('assistant')!.getContextManager().getAllMessages();
const row = rows.find((m) => JSON.stringify(m.content).includes('FIRSTLONG'))!;
assert.ok(rows.filter((m) => m.bodyGroupId !== undefined && m.bodyGroupId === row.bodyGroupId).length > 1, 'the premise: it is stored in parts');
assert.equal(nulls[0], 1, 'the premise: the render gave it null');
assert.deepEqual(JSON.parse(holding(injected, 'FIRSTLONG')), [image(), text(`Intro. ${long}`)], 'it goes in whole, as the store reads it back');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium severity, test coverage. The sharded-body test no longer checks what the real render gives a body stored in parts.

The stub forces index 0 (the sharded FIRSTLONG body) to null. The test therefore only checks that the boundary's as-stored fallback joins shards as getAllMessages reads them back. It no longer checks anything about how the memory strategy renders a sharded body at a boundary, and it never compares FIRSTLONG in the live round with the next request. Under context-manager#314 the real render gives this body as the next compile sends it, and no test here pins that the live round and next request agree for a sharded body. The test can't fail on a regression in the real render path for sharded bodies.

…each stubbed test

Stubbing the render's null in the three memory-strategy tests left the
file with no test that runs the strategy's own render at a tool boundary
and checks that what goes into the live round is what the next request
sends. Beside each stubbed test is now one with the same setup and no
stub, asserting only that:
- a batch bigger than the recent window: each screenshot;
- a message after a head-window reset the head hasn't filled up to;
- the message after a body bigger than the recent window.

For the body itself the two still differ: it goes in whole, joined as the
store reads it back, where the next compile sends it in more than one
piece, an exception the boundary already names. So that test checks the
message after it.

These pass with context-manager as main installs it and with a render
that gives these messages as the next compile sends them, and each fails
when the render's output is changed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@nissa-seru
nissa-seru merged commit e27609b into anima-research:main Oct 26, 2026
10 checks passed
nissa-seru pushed a commit to nissa-seru/agent-framework that referenced this pull request Oct 26, 2026
main brought anima-research#826, anima-research#819, anima-research#824, anima-research#821, anima-research#825 (ignore and include read
gitignore's forms, and refuse any other), anima-research#815, anima-research#820 and anima-research#818 (answers that
write say where the change stands on disk). Where they met this change, in
src/modules/workspace/index.ts:

- the imports from observe.ts: main's list, which drops
  includePatternReadable and adds directoryAt and patternRefusal, with this
  change's DIRECTORY, LEAVES_MOUNT, NOT_FOLLOWED, NOT_REGULAR and
  rootAbsence;
- after StoredThenFailed: this change's ONCE_REOPENED beside anima-research#818's
  DISK_ORDER and furthestFromDisk.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tel-0s added a commit that referenced this pull request Oct 27, 2026
268 commits from main, 28 pull requests: #353 (every agent on the store
re-initialised after a rollback, suppression, switch, undo or redo),
#817, #835, #818, #820, #815, #825, #821, #824 and the rest, with main's
manager-release machinery (an ephemeral run's auxiliary membrane, a
tracked manager released with its agent) and writeToFork (a fork's
channel message held behind a live surgery). Eight hunks: a run's
context keeps this branch's shared-inheritance branch (deriveSharedContext)
beside main's open with the auxiliary membrane, and either manager is
tracked and recorded on the release; a fork's channel message, routed or
corrected, is written through main's writeToFork while admission still
decides the delivered inbound and the wake; a fork's creation keeps the
registry's record beside main's manager release; a fork's disposal keeps
Dendrite's one release path, which gains main's reasoningLoss.forget and
releaseManager (and the name-only branch its forget). .gitignore and the
diagnostics test take main's words. #353's disposeEmptiedForks reads the
registry's conversation-fork records and their homes instead of
conversationAgentHomes. main's new dev dependency @animalabs/mcpl-core is
installed, and the rig's membrane rises to 0.7.2 (connectome-host main
asks for it).

Eight rows of main's new tests are read onto this branch, each named in
a comment: discord-awareness-outbox's fake frameworks carry the registry
stub an undo's fork sweep reads; turn-continuation's fork check reads the
registry's record; reinit-after-branch-move's fork tests match a fork's
stored channel message by its author-prefixed form and allow the
derivation notice a spawn tells its source; subconscious-model-fields
configures the persistent reader, the one that is an agent at create;
workspace-lost-contents' five torn-blob cases probe whether the linked
chronicle rewrites a blob whose bytes differ (chronicle main does, from
the #60 series; the released 0.4.0 keeps the file) and assert each
behaviour. Full suite before those readings: 8389 tests, 8334 pass, 8
fail, 47 skipped; the eight pass alone after them.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Exempts this PR from the changelog check (internal-only change)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant