Skip to content

fix(tune-out): entering a tune-out keeps the subconscious's window anchor at an all-except hold in force - #933

Merged
Anarchid merged 3 commits into
anima-research:fix/tune-out-resume-late-channelsfrom
nissa-seru:hilda/916-additions
Nov 6, 2026
Merged

Anarchid merged 3 commits into
anima-research:fix/tune-out-resume-late-channelsfrom
nissa-seru:hilda/916-additions

Conversation

@nissa-seru

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

Copy link
Copy Markdown
Contributor

On top of #916, entering a channel's tune-out while an older all-except hold is in force now leaves the subconscious's window anchor at the hold's start. In this pull request's new test, a hold entered at sequence 3 and then a channel's tune-out entered at 20 give the anchors [3, 20] with #916's coordinator at 9704133265, and [3, 3] at this head. main's coordinator at 48f26ce56c gives [3, 20] too.

@tel-0s, this pull request goes into #916's branch, fix/tune-out-resume-late-channels, and adds three commits to it. We're asking you to merge it there, so the anchor fix lands with #916's fix.

Problem

At 9704133265, enter() sets the subconscious's window anchor with these lines (src/tune-out/coordinator.ts:678–682):

const oldest = this.listActive().reduce(
  (min, e) => Math.min(min, e.params.startedAtSequence),
  params.startedAtSequence,
);
this.hooks.setSubconsciousAnchor(oldest);

With #916's coordinator in place of this head's, the new test, "a tune-out entered while an older all-except hold is in force leaves the subconscious's window start at the hold's start", fails at its last assertion: the anchors are [3, 20] where [3, 3] was expected. The hold's start is 3, and the second anchor is the new tune-out's start, 20.

Changes

  • enter()'s reduce (src/tune-out/coordinator.ts:676–679) starts from scope ? Math.min(params.startedAtSequence, scope.params.startedAtSequence) : params.startedAtSequence, where scope is this.getAllExcept(), which enter() already reads (:649), in 65e58aa35a. In 3be2d81a76, the comment above the reduce goes, since it only restated the reduce. 65e58aa35a's message says why the hold counts.
  • test/tune-out-resume-late-channels.test.ts gains that test, over the real ChannelRegistry and a JsStore. Its coordinatorOn takes what the coordinator's currentSequence hook answers as an optional fourth argument, whose default is the () => 9 the hook had.
  • The harness's anchor recorder, in 3edc77a5e1, satisfies TuneOutFrameworkHooks['setSubconsciousAnchor']. The harness hands the coordinator its hooks as never, so a change to that hook's shape left the anchor tests compiling and failing only at run time; now it fails the build at the recorder. The rest of the stub stays loose, as before.
  • changelog.d/tune-out-anchor-hold-in-force.fixed.md: "Hosts with a subconscious: entering a channel's tune-out while an older all-except hold is in force no longer moves the subconscious's window anchor past the hold's start."
  • No longer here: a second commit, which rewrote docs/dendrite.md's "Tune-out without a reader" paragraph around tuneOut.enabled. This pull request carried it until November 5. main has since rewritten that paragraph in f75c791a13, which removes tuneOut.enabled, so the commit is dropped and this pull request no longer touches docs/dendrite.md.

With #844

@tel-0s, by git merge-tree on November 6 at 04:31 UTC, #844 (ec41c17601) merges with #916's head with no conflicts. With this head in place of #916's, it conflicts in src/tune-out/coordinator.ts, in one block: enter()'s window anchor, where #844 calls anchorSubconscious() and this head has the reduce under Changes. That block resolves to #844's side, since its activeStarts() (src/tune-out/coordinator.ts:1079–1084 at ec41c17601) already counts the hold. Its setSubconsciousAnchor passes a SubconsciousWindowStart object (:100, :752 there) where this branch's passes a number, so after the merge this file's anchor recorder fails the build until it records start.sequence. #967 and #976, which go into #844's branch, likewise merge with #916's head with no conflicts and conflict with this head in src/tune-out/coordinator.ts. Each of the other 43 pull requests open then merges with this head as it does with #916's.

Tests

At a clean checkout of 65e58aa35a, on Linux with Node 24.22.2 and npm 11.19.1, after npm install in a fresh copy with no package-lock.json or node_modules. npm ls --depth=0 gives @animalabs/chronicle 0.5.1, @animalabs/context-manager 0.24.2, @animalabs/mcpl-core 0.4.0, @animalabs/membrane 0.8.0, @babel/parser 7.29.13, @modelcontextprotocol/client 2.4.0, @modelcontextprotocol/server 2.4.0, @types/node 22.20.5, @types/ws 8.18.2, chokidar 4.0.3, discord.js 14.28.1, tsx 4.23.15, typescript 7.0.2 and ws 8.22.1. npx tsc --noEmit, npm run build and npm run pretest each exited 0. Each test file ran alone, with the flags npm test gives node and the TAP reporter:

This repository's CI ran on this pull request at its head, 3edc77a5e1, and its 12 checks pass. Among them, Build & Test (ubuntu-latest, Node 22.12.0) runs the whole suite with npm test.

At 3be2d81a76, which only removes the comment, with the same install: npx tsc --noEmit, npm run build and npm run pretest each exited 0, and tune-out-resume-late-channels passes 4 of 4.

At 3edc77a5e1, with the same install: npx tsc --noEmit exits 0, and tune-out-resume-late-channels passes 4 of 4. With the hook's declared type swapped for an object shape like #844's, { sequence: number; epochIds: string[] }, tsc fails at test/tune-out-resume-late-channels.test.ts(54,55) with TS2345. With the same swap and the file as it was at 3be2d81a76, tsc reports nothing in the test file.

Not verified

  • Other platforms. This pull request's CI ran the whole suite on Linux only, and nothing here ran on Windows. CI's macOS legs install and start the package, and run code_execution's tests under Bun.
  • The subconscious's window. The test asserts the sequences the coordinator passes to setSubconsciousAnchor, not what the window then holds.
  • main after 48f26ce56c. No run here used a later main's coordinator. At main's 074ac60be0, enter() sets the anchor with the lines Problem quotes (src/tune-out/coordinator.ts:664–668).

  • Changelog fragment added: changelog.d/tune-out-anchor-hold-in-force.fixed.md.

Hilda-2681 made this change and ran it red first. The anchor fix first went into an earlier change of ours for #900, which Crispin-2406 read. Naomi-2422 read this pull request. Fern-2426 ran our internal review of #916. Daniel-2612 repeated the runs at a clean checkout and wrote this description. Hilda-2681 reran them at 65e58aa35a and updated the description when the second commit was dropped.

enter() set the subconscious's window anchor to the oldest start among
the tune-outs on record and its own, and left out an all-except hold in
force, which the start's resume counts and enterAllExcept's own start
stands for. Entering a channel's tune-out while an older hold was in
force moved the anchor past the hold's start, though the comment above
the reduce said never to move it forward there, and the hold's earlier
rows left the subconscious's window. main has the same gap. The reduce
now starts from the older of the new tune-out's start and the hold's,
and its comment says what it keeps.

Test: a hold entered at sequence 3, then a channel's tune-out at 20,
over the real registry and store, leave the anchor at 3 both times. The
test file's coordinator takes the sequence as an optional argument for
it. Before this change the second anchor was 20.
@nissa-seru nissa-seru self-assigned this Nov 2, 2026
@nissa-seru

Copy link
Copy Markdown
Contributor Author

/review

@anima-review

anima-review Bot commented Nov 3, 2026

Copy link
Copy Markdown

Automatic review of e2dc0af9615e.

No findings.

@nissa-seru

Copy link
Copy Markdown
Contributor Author

/review

— Hilda-2681

@anima-review

anima-review Bot commented Nov 5, 2026

Copy link
Copy Markdown

Automatic review of 65e58aa35a7e.

About these findings

The findings below came from reading this PR and relevant code; not a holistic understanding of the project's functional intent and its architecture choices. Finding precision is higher for flagging issues than identifying ideal changes/fixes. This review prioritizes recall - minor findings (in particular) may be nits.

Before actioning a finding, consider the relevant code from first principles given the functional+architectural intent. In particular, when considering adding machinery/logic, scrutinize whether the incremental value of such pays rent for the complexity it adds to the codebase - kudzu has a direct negative impact on our future ability to work with the codebase.

Consider explicitly mentioning which findings you accept/decline (by code-formatted numerical id) and why; this enables future rounds of automated review to take your notes into account.

Findings, most severe first:

  • 1. minor at src/tune-out/coordinator.ts:676: The diff rewrites the comment above enter()'s reduce as "Never move the anchor past the start of an epoch still in force, a tune-out's or the all-except hold's." That sentence states a design rule for internal code, so it isn't any of the kinds of comment the house keeps.
    • Failing case: The comment is on a private step inside TuneOutCoordinator.enter(). It isn't a license header, and it doesn't explain behaviour forced by a dependency, platform or protocol. It isn't a tool directive, and it isn't a doc comment defining a public API contract. It cites no issue or decision either. It restates the invariant the reduce already shows. The commit message (65e58aa3, lines 4–12) already carries the reasoning, so the comment can go. If editors must keep the rule, it belongs in the repository's contributor guide; the repository has no AGENTS.md.

The comment restated what the reduce does: the anchor never moves past
the start of an epoch still in force, a tune-out's or the all-except
hold's. The reason for counting the hold is in the previous commit's
message, so the code carries no comment there.
@nissa-seru

Copy link
Copy Markdown
Contributor Author

1: accepted. 3be2d81a76 removes the comment, since it only restated the reduce. 65e58aa35a's message says why the all-except hold counts.

— Hilda-2681

@nissa-seru

Copy link
Copy Markdown
Contributor Author

/review

— Hilda-2681

@anima-review

anima-review Bot commented Nov 5, 2026

Copy link
Copy Markdown

Automatic review of 3be2d81a7665.

No findings.

@Anarchid Anarchid left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nightly dual review · round R1 at 3be2d81 · reviewers: Codex gpt-5.6-sol (xhigh) and Claude claude-opus-5-5, merged and every claim re-verified by the Claude leg · posted by the maintainer's campaign runner. Verdict: APPROVE (the formal review event is left to the maintainer). Finding ids R1-n are stable; cite them in fix commits.

Reviewed at 3be2d81 against fix/tune-out-resume-late-channels 9704133

Round 1. There is no earlier review of ours on this PR, so there are no earlier findings to give dispositions for.

Verdict: APPROVE, one minor

The fix is correct and complete for the coordinator it targets. Before it, enter() was the only one of the coordinator's three anchor sites that ignored the all-except hold. resumeActiveEpochs() and enterAllExcept() already counted both kinds of epoch, so all three now agree. The bug is real in effect, not just in the recorded numbers. In context-manager 0.24.2, which this branch resolves to, WindowedPassthroughStrategy.setAnchor assigns whatever it's given (src/strategies/windowed-passthrough.ts:277-282), and the window drops every row below the anchor (:348-350). So on the base, the hold's held rows between its start and the later tune-out's start left the subconscious's view. The one thing worth acting on is in the test harness and concerns #844 rather than this branch. #844 already fixes the same bug its own way, and it changes the anchor hook's shape. When the two meet, the test file merges cleanly and then fails at runtime, because the harness erases the hook's type.

Blockers

None.

Major

None.

Minor

R1-1. The new test records the anchor hook through hooks as never, so #844's hook change breaks it at runtime, not at build.
test/tune-out-resume-late-channels.test.ts:54 records setSubconsciousAnchor: (sequence: number) => { anchors.push(sequence); }, and :59 passes the hooks as never. #844 (ec41c17601) changes the hook to (start: SubconsciousWindowStart, others?) => void (src/tune-out/coordinator.ts:752 there). It also updates every anchor recorder it can see to start.sequence: test/all-except-unit.test.ts:71 and test/tune-out-cancel-order.test.ts:80 there, which read (sequence: number) => … push(sequence) on this branch at :56 and :82. This file is new in #916, so #844 can't update it.

  • Failure scenario: git merge-tree of #844 with this head gives one conflict, in enter(), as the description says. It resolves to #844's anchorSubconscious(). This test file merges with no conflict. After the merge, anchors holds { sequence: 3, epochIds: [...] } objects. The new test's assert.deepEqual(anchors, [3, 3]) fails, and so does #916's third test with Array(3).fill(7). tsc reports neither, because of the cast. I couldn't run the merged tree: #844 needs @animalabs/chronicle ^0.6.0, which isn't on npm yet (npm install fails with ETARGET). This conclusion comes from reading the merged sources, not from running them.
  • Fix: type the one hook the test reads, and leave the rest of the stub as it is:
    setSubconsciousAnchor: ((start) => { anchors.push(start); }) satisfies TuneOutFrameworkHooks['setSubconsciousAnchor'],
    TuneOutFrameworkHooks is exported from src/tune-out/coordinator.ts:247. I checked this at this head: it compiles. Against a stand-in with #844's object shape, tsc rejects the push with TS2345, so the merge fails at build and names the line. A satisfies Partial<TuneOutFrameworkHooks> on the whole stub doesn't work: it also flags the stub's deliberate looseness (addMessage returns nothing, Row isn't StoredMessage). Also add a sentence to "With #844": the conflict resolves to #844's side, because its activeStarts() (:1079-1084 there) already includes the hold. After that, both anchor tests in this file record start.sequence.

Not counted as a finding: the reduce's seed, scope ? Math.min(params.startedAtSequence, scope.params.startedAtSequence) : params.startedAtSequence, is now the third separate copy of "the oldest start in force". #844 gathers them into activeStarts(), and this line is dropped when #844 lands, so it isn't worth refactoring here.

Verified fine

  • The fix itself. scope is read at src/tune-out/coordinator.ts:649, and nothing between there and the reduce at :676-680 can end or change the hold. readOptions is pure. clearTimers, enterTuneOut and armExpiry act on the one channel, and everything runs synchronously, so no expiry callback can run in between.
  • The other anchor sites. There are only three setSubconsciousAnchor calls in src/. resumeActiveEpochs() counts the hold while it's in force (:463-484). enterAllExcept() reduces over listActive() (:1028). The reverse ordering, a hold entered after a tune-out, is already covered by #916's third test (test/tune-out-resume-late-channels.test.ts:117-135).
  • Exits don't re-anchor, by design. A channel's expiry goes through cancel(), and the hold's goes through cancelAllExcept(). Neither moves the anchor, so ending either older epoch leaves the window start where it was until the strategy re-anchors or the next entry does.
  • A hold past its end time but not yet released still counts. getAllExcept() doesn't look at the clock, and holdFor() (:943-951) still holds traffic for such a hold, so counting it in the reduce matches what's actually held.
  • No NaN risk from older records. AllExceptParams.startedAtSequence has been written since the all-except scope was introduced (e3fbc64b), so no hold on record lacks it.
  • Red first, reproduced. I ran a fresh npm install on Node 22.22.1 / npm 10.9.4, and npx tsc --noEmit, npm run build and npm run pretest each exited 0 at this head.
    • At this head, tune-out-resume-late-channels passes 4/4.
    • With the base's coordinator.ts built in place of this head's, it passes 3 and fails 1: the new test, with [3, 20] where [3, 3] was expected.
    • With 48f26ce56c's coordinator.ts (main at the branch point), 0/4 pass.
    • The 46 test files that name TuneOutCoordinator, tune_out, tuneOut, cancelTuneOut or all-except pass 1,106 of 1,106 at this head.
  • main has the same bug. 074ac60be0 has the pre-fix reduce at src/tune-out/coordinator.ts:664-668, as the description says.
  • The description's citations hold. Line numbers at 9704133265, at this head and at 074ac60be0 all match. The textual merge results with #844 match git merge-tree. CI shows 12/12 checks green at 3be2d81a76.
  • The changelog fragment follows changelog.d/README.md for both its name (<slug>.fixed.md) and its form (bullets, opening with the audience), and it describes the ordering the fix covers.
  • The harness change keeps the other tests as they were. coordinatorOn's new fourth parameter defaults to the old () => 9, and the three existing tests don't pass it.

Method: two independent passes, merged after re-checking every claim. Codex gpt-5.6-sol at xhigh ran read-only on a detached checkout of the head: 482 s, 112 commands, about 4.11M input tokens (3.97M cached) and 19.4k output. It reported no findings. Its verified-fine list matched the Claude pass point for point, each pass reaching it independently: the three anchor sites, the timing of the scope read, startedAtSequence always being present, main having the same bug, and #844's activeStarts() subsuming the fix. I re-checked each of its citations against the committed head and the context-manager 0.24.2 source. Found only by Claude: R1-1 (the hook-shape change in #844 and the as never harness), the red/green and subset runs, and the CI and description checks. Sol couldn't run the tests (its sandbox's temp directory is read-only), and it had no network for the PR body or CI. Neither pass could build #844 merged with this head, because #844's chronicle dependency isn't published.

coordinatorOn hands the coordinator its hooks as never, so a change to
the setSubconsciousAnchor hook's shape would leave the recorder
compiling and the anchor tests failing only at run time. The recorder
now satisfies TuneOutFrameworkHooks['setSubconsciousAnchor'], so such
a change fails the build at this line instead. The rest of the stub
stays loose, as before.
@nissa-seru

Copy link
Copy Markdown
Contributor Author

Replying to this review.

R1-1: accepted, addressed in 3edc77a5e1. The anchor recorder now satisfies TuneOutFrameworkHooks['setSubconsciousAnchor'], so with #844's hook shape the build fails at that line. The description's "With #844" now says how the conflict block resolves, and that the recorder then has to record start.sequence.

— Hilda-2681

@Anarchid
Anarchid merged commit 7a77862 into anima-research:fix/tune-out-resume-late-channels Nov 6, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants