Skip to content

fix(kv-unified): hold a level pin at the deepest level its chain supplies until the pinned level exists (#193) - #213

Merged
nissa-seru merged 18 commits into
anima-research:mainfrom
nissa-seru:fix/193-kv-unified-level-pin-hold
Oct 25, 2026
Merged

nissa-seru merged 18 commits into
anima-research:mainfrom
nissa-seru:fix/193-kv-unified-level-pin-hold

Conversation

@nissa-seru

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

Copy link
Copy Markdown
Contributor

TL;DR: under foldingStrategy: 'kv-unified', a level pin over messages that no summary covers at that level yet made every compile throw kv-unified constraints are infeasible …, until merging produced the level on its own or the pin was released. kv-unified's canonical forest now holds such a pin at the deepest level below it that the summaries over each message supply, raw when none does, and moves it to the pinned level once that level exists. Each hold is logged when a compile first finds it and when it ends. A level pin and a lock at another level on a message in the folded middle are still refused, even before the pinned level exists.

Fixes #193

Problem

Anarchid found this while reviewing #175 (#193): with five unmerged L1s under mergeThreshold 6, a pin at level 2 made every compile throw, and twelve ticks queued no merge. A level pin (pinAtLevel, or pinRange and markDocument with level) becomes an exact constraint in kv-unified's canonical forest, and a message whose summaries don't reach that level yet has no allowed level. Nothing asks for the level: the infeasibility certificate lists it in missingLevels, and its suggestion ends "or produce the missing levels" (src/adaptive/kv-unified.ts:1952 at cd6ad3d6), but no code acts on that. So the refusal lasts until compression and merging reach the level by themselves.

In probes on main at cd6ad3d6 (0.15.0), with a mock summarizer and the default budget:

  • An L2 pin over two L1s: 13 of 13 compiles refused while the conversation stood still. Once it went on, the first compile to succeed came 4 messages later, when a sixth L1 let the ordinary merge make the L2.
  • The same pin inside one L1, and the same pin stored and reopened: the same.
  • An L3 pin over the same range: 278 of 280 compiles refused across the 124 messages before the first success.
  • An L2 pin placed while its messages were in the recent window: the first refusal came 3 messages later, once the range left the window, and 7 of the 11 compiles after the pin refused.

kv-stable never refused in any of them. At cd6ad3d6 it clamped such a pin to the lower of the pinned level and the deepest level produced over the chunk (src/adaptive/strategies/kv-stable.ts:132 there). Since #234 it renders the pin at the deepest level at or below the pinned one where the chunk's lineage has a summary that lists it (renderableLevel, src/adaptive/summary-tree.ts:270; tested in test/adaptive/overlap-tolerance.test.ts, "a pin-at-level at a level its own lineage skips holds the chunk at the next level down").

0.15.0's notes describe this refusal, and a second way to reach it that #175 added: a range pinned at a level that only a summary the load repair dropped supplied is refused as infeasible, whatever the budget.

Why hold the pin rather than refuse it or produce the level

Anarchid suggested three directions. We prototyped each on main as it stood before 0.15.0 (53bab67f, where the probes above gave the same counts) and ran the probes through them:

  • Refuse at pinAtLevel when the range doesn't have the level. Every probe's pin was refused when it was placed, since none of their levels existed yet, so no compile refused. A pin already in the store got past the check, though, and every compile refused as on main. And a level pin can't be placed until its level exists, which for recent messages means waiting for them to be compressed and merged.
  • Produce the missing levels from the conflict. The pin over two L1s and the stored pin compiled after one tick made a two-source L2. The pin inside one L1 and the L3 pin refused as on main, since there was nothing to merge. The recent-window pin did worse than main, 23 of 27 compiles refused: its messages had left the recent window but weren't in a closed chunk yet, so the merge request's range had no known ends, and it merged a run of older L1s into an L2 that didn't cover the pin (see "Out of scope").
  • Hold the pin. No compile refused in any probe, and the pinned messages rendered what kv-stable rendered on main then, at the same points. By reading the two, where the levels over a message run up from L1 without a gap, which is how merges build them, kv-stable's clamp and the hold pick the same level. On a chain with a level above the pin and none at it, kv-stable at cd6ad3d6 fixed the chunk at the pinned level and left the rest to its controller, where the hold keeps it below. Since fix(summary-tree): a chunk's summary at a level is one that lists it #234, kv-stable keeps it below too.

The design notes make a pin absolute: the solver honors it or reports infeasibility, and admission is the caller's job (docs/unified-solve-design.md, F4 and F4b). docs/memory-tools-design.md plans that admission as a dry-run compile, but it isn't built. Today pins come from library callers and from connectome-host's WebUI, which passes the operator's range to pinRange or markDocument. So the hold does the admission's work where the forest knows which levels exist. F4b now names it as the one exception to "no constraint is weakened, clamped, or selected inside the solver", and the hold is logged rather than silent.

Changes

  • The forest decides (src/adaptive/kv-unified.ts:368). levelPinHold holds a leaf's level pin when the leaf's ownership chain doesn't supply the pinned level and every other constraint on the leaf allows both the pinned level and the level the leaf would be held at: the deepest level the chain supplies below the pinned one, 0 when it supplies none. buildLeaf then narrows the pin to that level (:1000). Availability is the forest's own, the same list missingLevels is computed from, so the hold and the solver can't disagree. A pin that another constraint contradicts, such as a lock at another level, isn't held, so the pair conflicts at once, as before. Nor is a pin whose held level another constraint excludes, such as a floor above the held level and at or below the pinned one, so its conflict names the pinned level, which is the level that would clear it. A pinned level that isn't a non-negative integer is never held, as buildLeaf places no leaf at one, and another constraint whose level isn't one allows no level, since buildLeaf empties a leaf that carries it, so beside it the pin stays strict and the conflict names the pinned level. The pin is the constraint constraintsFor makes from the picker chunk's pinLevel, known by identity rather than by its source label (:350), so an exact constraint a caller makes and passes through CanonicalForestOptions.constraints stays strict whatever it's labelled. The constraint keeps the pin's own level, so derive's sameLeafInputs still matches it, and when a merge changes a leaf's chain, extend rebuilds the leaf and the hold moves with it.
  • The log (src/strategies/autobiographical.ts:15808). After each kv-unified compile that isn't a dry run, reportHeldLevelPins reads the holds off the solved forest (heldLevelPin, kv-unified.ts:383) and attributes each held message to each pin whose level its bound took. It reads a pin's span through pinPositions, as the forest's bounds do since fix(pins): a pin renders whole messages in the selects; a compile that can't hold them narrows it in memory #327 and fix(pins): every reader reads a pin over a long message whole, and the plan's over-budget refusal names the pins #330, so under adaptive resolution a pin that names one part of a message stored in parts is reported over the whole message, and a message counts once however many of its parts are held. Its lines go through the strategy's diagnostics (this.diag), so each ends with ns=<namespace>. The strategy's other text lines have named their namespace that way since fix(diagnostics): every stderr line about one namespace names it (#239) #245. When a compile first finds a pin held it warns, for example autobio: kv-unified holds pin <id> ("<name>"), pinned at L2, at L1 in 4 message(s): no summary supplies L2 over them yet. They move to L2 once one does. When a later compile finds none of the pin's messages held for it, as once its level arrives, it warns autobio: kv-unified no longer holds pin <id> ("<name>") below its L2., and when the pin is gone, autobio: pin <id> ("<name>"), held below its L2 by kv-unified, was removed. The record of holds follows the loaded branch: it starts over with each open and branch switch (:2937), so a pin still held then is logged again, and a pin that exists only on another branch isn't reported as removed. A dry run logs nothing, since a preview isn't the live state. The report runs after the solve and before the frontier is presented, so a compile refused as over budget has already logged the holds that can explain the range's size, which a test holds. By reading, a compile the solver refuses logs nothing, and its error names its cause.
  • Beside main's whole reading of pins. By our reading, fix(pins): a pin renders whole messages in the selects; a compile that can't hold them narrows it in memory #327's kv-stable rule, that a level mark holds each summary it reaches whole, and this kv-unified hold never act in the same compile, since buildPicker (src/strategies/autobiographical.ts:15638) builds one solver or the other by foldingStrategy. What they share is the pin's span, pinPositions, which both the forest's bounds and the report now read.
  • Beside feat: carry membrane's system changes in place under every strategy; firstChanged on compile results #298's system changes. A message of system changes stays raw under any pin, since resolvedPinBounds sets it raw after resolving pins (src/strategies/autobiographical.ts:3918). Its chunk carries no level, so the forest never holds it and the report never counts it: a pin over eight messages and a change between them is reported in 8 messages, and a pin over the change alone reports nothing. F4, ProtectedRange.level, the README and the fragment say so.
  • Docs: F4 and F4b in docs/unified-solve-design.md, ProtectedRange.level, PinLevelOptions.level, PickerChunk.pinLevel, both pinAtLevel docs and the README say what a level pin does before its level exists. F4 also says that a pin over a head or tail message, the recent window included, adds no restriction while the message is there, as before.
  • Tests that pinned the old behaviour: fix(compression): honor raw pins in L1 recall #116's picker-chunk test (test/l1-recall-raw-pins-frontier.test.ts) no longer tolerates the refusal. The certificate test that used an unsupplied pinLevel of 9 to make a conflict now uses a lock against a pin (test/adaptive/kv-unified-certificate-holes.test.ts). fix(autobiographical): repair summaries readers can't use on load #175's persistence test, which asserted the refusal after the load repair drops an L2, now asserts the hold, then releases the pin and compiles, and pins the range at L1 and compiles (test/strategy-persistence.test.ts:1800).
  • A fragment, changelog.d/193-kv-unified-level-pin-hold.fixed.md.

What callers notice: compiles that refused now go through. Until its level exists, a held range renders finer than it was pinned, so it can take more of the budget than the pin asked for. Where the budget can't hold the compile, it's refused as over budget (OverBudgetError) rather than as infeasible. A level pin and a lock at another level on a message in the folded middle are refused as before. The log lines are new.

What each part is for

The change should let compiles go through while a pinned level doesn't exist yet, keep refusing constraints that really conflict, and say when a pin renders below its level. Leaving it out leaves #193's refusal, and the two other directions above leave pins that still refuse.

  • levelPinHold (12 lines) decides the hold from the chain the solver itself reads, so the two can't disagree. It holds no pin whose level isn't a non-negative integer, since buildLeaf places no leaf at such a level. A clamp before the forest exists, which we also prototyped, needs a walk of its own to find which levels exist.
  • levelPins (one line, with its doc) records the constraint constraintsFor makes from a chunk's pinLevel, so the hold applies to that constraint alone. PIN_LEVEL_SOURCE names that constraint's label once, for constraintsFor and for sameLeafInputs, which reads the pin's level back from it. A constraint's source is a label any code that builds a forest with constraints can give, today only the tests, so finding the pin by its label would hold such a constraint that happened to carry it.
  • The check of the leaf's other constraints runs every other constraint on the leaf through constraintAdmits at both the pinned and the held level, since buildLeaf narrows to the held one. constraintAdmits came to main with fix(kv-unified): suggest producing missing levels only where that clears the conflicts (#256) #259, for the conflict suggestion. It's the same rule as the admitsLevel this change had added, which it now replaces. It allows no level for a constraint whose level isn't a non-negative integer, as buildLeaf empties a leaf that carries one, so such a bound can't let the hold apply to a leaf it then empties. By our reading, in a strategy compile only a lock can sit beside a level pin: a chunk carries one of pinned, pinLevel and pinMaxLevel, and head and tail chunks carry no bound. So a lock-only check would do today. The other three cases do for a constraint passed through CanonicalForestOptions.constraints, by code in the package that builds a forest with constraints, today only the tests, what the lock case does for a lock at the held level: they keep the hold from hiding that constraint's contradiction with the pin.
  • heldLevelPin (3 lines, exported) lets the report read holds through the function buildLeaf uses.
  • reportHeldLevelPins (46 lines for the function, 50 with its map and its clear on a branch switch) logs the hold, since F4 says a pin is never silently violated and a held pin renders below its level. It takes a pin's span from pinPositions rather than a walk of its own, so the report and the forest can't read a pin differently, and it counts messages rather than parts, since the pin's span covers whole messages and the line says "message(s)". It logs once when a hold starts and once when it ends. A line on every compile while held would repeat each turn and never say that the hold ended. The dry-run skip, the attribution to the binding pin and the branch clear each have a test. It scans the solved forest's leaves on every compile, since a guard that skipped the scan when no pin had a level above 0 didn't pay: with no pin held, on forests of 1,000 to 20,000 leaves the scan takes about 0.1 to 1.1 ms, under half a percent of building the forest.
  • The tests: each of the 29 removals under "Tests" fails at least one of them. The four tests on a message stored in parts share a store builder (longMessage, with partsOf and levelsOver to read the store), the shape test/whole-body-pins.test.ts uses. The report as it was before R2-1, which read a pin by its ids and counted parts, fails all four, and counting parts alone does too; reading by ids alone, still counting messages, fails three, since the count test's pin covers two messages either way. A fifth test puts a message of system changes inside a pin's span, and fails if the strategy stops keeping it raw.

Left out:

  • A log line when a hold moves between held levels: the log marks when a hold starts and ends.

The part we're least sure pays for itself is the log, the largest new code: its value depends on someone reading the host's log.

Tests

The runs below were made before the branch's older commit messages were reworded. Each commit named here has the same tree as the one the run read.

At 09dbe9b8, this PR's head, on Linux x86_64 from a fresh npm ci, with main at 1fe572ee merged in, on Node 24.22.1:

  • Typecheck and build: clean.
  • npm test: 3,169 of 3,169 pass.
  • test/kv-unified-level-pin-hold.test.ts, test/adaptive/kv-unified.test.ts, test/strategy-persistence.test.ts, test/l1-recall-raw-pins-frontier.test.ts and test/adaptive/kv-unified-certificate-holes.test.ts: 143 of 143 pass.
  • Each of 29 deliberate removals fails at least one of those tests:
    • in the forest: no hold; holding raw whatever the chain supplies; holding at a level the chain supplies above the pin; holding even where the pinned level exists; holding a pinned level that isn't a non-negative integer; finding the level pin by its label;
    • in the check of the leaf's other constraints: not weighing them before holding; weighing them only at the pinned level, or only at the held level; weighing only the first of them; weighing the pin against itself; letting a lock, a raw constraint, a cap or a floor admit a pin at any level (four removals); a cap at the pinned level, or a floor at the held level, excluding it; letting a constraint whose level isn't a non-negative integer admit levels;
    • in the report: no report; reporting from dry runs; reporting only from compiles that fit the budget; counting a held message toward every pin over it; keeping holds across a branch switch; reporting a removed pin as no longer held; reporting on every compile; keeping a hold that ended; counting parts rather than messages; reading a pin by its ids, still counting messages (three tests fail);
    • and main's line in resolvedPinBounds that keeps a message of system changes raw: the system-change test fails.

At 6eac048b, before the commits that cut the report's guard and add the fragment's grace clause, with the same main merged in, on Node 24.22.1:

  • npm test: 3,169 of 3,169 pass. A first run had one failure, the picker's 1,000-chunk stress test, at 6,039 ms against its 5 s limit while the machine's load average was about 170; the rerun passed it and everything else.
  • The test files the change touches, with test/whole-body-pins.test.ts and test/system-changes.test.ts: 243 of 243 pass.
  • Sent requests, against main 1fe572ee: over stores with no pins, with level pins at L1 and at L2 where those levels exist, with a cap, and with a long message stored in parts marked at L1, all 173 compiles send the same bytes as on main, as two runs on main do with each other. A pin at L2 over two L1s with no L2 yet is refused as infeasible on main in all three compiles and compiles at 6eac048b.
  • The report's leaf scan, timed with no pin held, on forests of 1,000, 5,000 and 20,000 leaves: about 0.1 to 1.1 ms per compile, under half a percent of building the forest it reads.

At ba1215f4, with main at 4fbe6c94 merged in, on Node 24.22.1: with the report as it was before R2-1, reading a pin by its ids and counting parts, the four tests on a message stored in parts fail, and the other 9 in test/kv-unified-level-pin-hold.test.ts pass.

At ed0cccc6's tree merged with main at 17a7f6d6, npm test passed 2,633 of 2,633 on Node 24.22.0 and on Node 22.12.0.

Before the merge, on main at cd6ad3d6 (0.15.0), on Node 24.22.0:

  • The test files the change touches, with test/kv-unified-strategy-integration.test.ts, pass 131/131. Eleven more test files, nine on kv-unified and two on kv-stable's level pins, pass 169/169.
  • With src at main, plus a stub of the one new export the tests import (heldLevelPin, returning null), 13 of the 115 tests in the touched files fail: the 11 new tests, fix(compression): honor raw pins in L1 recall #116's picker-chunk test and fix(autobiographical): repair summaries readers can't use on load #175's persistence test. The certificate test's lock-against-pin step passes there too, as a control.
  • The probes from "Problem": no compile refused in any of them. With the pin over two L1s, inside one L1 and two levels up, the pinned messages rendered at the levels kv-stable rendered them at on main then, at the two points the probe samples: L1 after the twelve rounds, and the pinned level once it existed, 4 messages later for the L2 pins and 124 for the L3 pin. The stored pin did the same as the first.

What the new tests protect:

  • test/kv-unified-level-pin-hold.test.ts runs the probe's five shapes through the strategy: over two unmerged L1s, inside one L1, two levels up, a stored pin reopened, and a pin placed in the recent window that then leaves it. Each asserts that compiles go through while the pin is held, the first four with the range rendered at the level it's held at, and all but the reopened store go on to assert that it moves to its pinned level once a merge produces it. They protect kv-unified refuses every compile while a level pin names a level no summary supplies yet #193's contract, that a level pin under kv-unified doesn't stop compiles while its level doesn't exist. Four more cover the log: one line when a hold starts and one when it ends, none from a dry run, the line for a removed pin, a hold logged before a compile is refused as over budget, only the binding pin reported where pins overlap, and a fresh start on each branch. Four more read a pin over a 6,000-word message stored in nine parts as the forest does: a pin from its fourth part to the next message is reported held in 2 messages, beside a pin at the same level over three plain messages reported in 3; a pin on its last part, still in the recent window, is reported held over the parts outside it; where L2 reaches the first part, which the pin names, before the rest (mergeThreshold 2), the hold is reported until L2 covers every part; and a message whose parts are held at different levels is reported with each of them. One more puts a message of system changes inside a pin's span: the hold is reported in the eight messages around it, and a pin over the change alone reports nothing.
  • test/adaptive/kv-unified.test.ts covers the rule in the forest itself: held at L1, or raw where no summary covers the message, moved to L2 through derive's extension when the L2 arrives, and held again when it's dropped. Locked at L0, or at the L1 the pin would be held at, the leaf conflicts and the conflict names the missing L2; locked at L2 once it exists, the two agree. A second test puts one other constraint of each kind beside a pin at a missing level, on a leaf whose chain supplies L1 and on one whose chain supplies no summary, then two together, pins at levels that aren't non-negative integers, and a caller's exact constraint that carries the pin's label. Each row checks the leaf's levels, what the conflict names and where the pin is held: a cap at the pinned level and a floor at the held level leave it held, a floor above the held level and at or below the pinned one makes the pin strict, so the conflict names the pinned level, a level of 1.5, -1 or NaN is never held, and a caller's exact constraint labelled like the pin stays strict. A third holds an L3 pin at L2 on a lineage that skips a level (L1, L2, then L4), not at the L4 above it. They protect F4b's rule that the hold relaxes nothing but the pinned level's availability.

Not verified

  • We haven't compiled a real store with a level pin under kv-unified, or run a real summarizer: the tests and probes build their stores and use a mock summarizer. How long a hold lasts depends on how soon merging reaches the level. In the probes it was 4 messages for an L2 and 124 for an L3, at the default chunk size and mergeThreshold 6.
  • How much more of the budget a held range takes on a real store.
  • connectome-host's pin-add path. The tests call pinRange and pinAtLevel directly.

Out of scope

  • connectome-host's WebUI sets levelHonored only under kv-stable (src/web/panel-data.ts:619 at connectome-host's 8a83b034, with the note at src/web/protocol.ts:900), which hasn't been true for kv-unified since fix(compression): honor raw pins in L1 recall #116. Correcting it is a change of its own in connectome-host, and either can land first.
  • enqueueMergeForRange (src/strategies/autobiographical.ts:7252) counts every unmerged summary as in range when either end of the requested range has no chunk-owned position. main's produce requests name chunk ids, and we didn't find a path that reaches it. The produce-request prototype did, which is why its recent-window probe merged the wrong L1s.
  • How a pin's level is validated. pinRange, markDocument and pinAtLevel keep a level only when it's a non-negative integer, and otherwise drop it, keeping a maxLevel that is one, else making a classic pin (normalizePinLevels). By our reading, pins read back from a store aren't checked, so where a store's pins state holds another level, as an edited or damaged one can, a negative or null level never becomes a bound the picker reads and can displace a valid overlapping pin's level, and a fractional one makes its leaf conflict without naming a level. That's as before this change.
  • Holds reach the strategy's diagnostics log only. feat(observer): tell a host's observer of each compile, compression refusal and calibration sample #335's ContextObserver hears of compiles, compression refusals and calibration, and isn't told of holds.
  • The admission docs/memory-tools-design.md plans (invariant 4) isn't built here. A dry-run compile, which that admission plans to use, logs nothing about holds.

  • Changelog fragment added: changelog.d/193-kv-unified-level-pin-hold.fixed.md.

Ramona-2416 and Esther-2584 read it internally. Effie-2289 read the first merge of main and the fixes for Anarchid's first review, Desmond-2314 read the merge of main at 17a7f6d6 and the fold into constraintAdmits, and Greta-2425 (Claude Opus 5.5) read the report's change to pinPositions and the docs and tests after it. An automated internal review read it too, and Prudence-2217 read its first audit with me.

— Hamish-2699 (Claude Opus 5.5). Oscar-1005 (Claude Opus 5.5) chose the hold over refusing at pinAtLevel or producing the level, and agreed its place in the forest. Anarchid reported the issue in #193, suggested the three directions, and reviewed this PR twice.

Description checked and posted by Lottie-2661 (Claude Opus 5.5) and Fiona-2757 (Claude Opus 5.5).

🤖 Written using Anthropic's Claude Opus 5.5 through the Rookery's tooling.

Hamish-2699 and others added 5 commits October 16, 2026 14:08
…lies until the pinned one exists

Under kv-unified, a level pin (`pinAtLevel`, or `pinRange`/`markDocument`
with `level`) became an exact constraint, and a leaf whose ownership chain
didn't supply that level yet had no allowed level, so every compile threw
"kv-unified constraints are infeasible ... produce the missing levels."
Nothing requested the level, so the refusal lasted until compression and
merging reached it on their own (Anarchid's probe on anima-research#175's review: five
unmerged L1s under mergeThreshold 6, a level-2 pin, every compile refused,
twelve ticks queued nothing), or until the pin was released. kv-stable has
always clamped the same pin to the deepest produced level
(kv-stable.ts:132). This direction was chosen over refusing at pin time
and over requesting the missing levels from the conflict, after probing
all three.

- The forest decides: where buildLeaf narrows a `pin-level` exact
  constraint, it uses pinLevelHeldAt(availableLevels, level), the pinned
  level when the chain supplies it and otherwise the deepest level the
  chain supplies below it (0 when none). Availability is the forest's own,
  the same list requestedMissingLevels is computed from, so the hold and
  the solver can't disagree. The constraint keeps the pin's own level, so
  derive's sameLeafInputs still matches it; when a merge changes a leaf's
  chain, extend rebuilds the leaf and the hold moves with it. Locks and
  every other exact constraint stay strict, so a level pin that conflicts
  with a lock is still infeasible. `PIN_LEVEL_SOURCE` names the source
  both places read.
- heldLevelPin(leaf) reports a hold. After each non-dry kv-unified pick,
  reportHeldLevelPins reads the holds off the solved forest and attributes
  each held message to the pin whose level its bound took (overlapping
  pins: the lowest level wins, so an overridden pin isn't reported). It
  logs once when a pin starts being held, naming the pin, its level and
  the level(s) held at, and once when the hold ends or the pin is removed.
  The episode map is per branch (cleared with the branch mirrors), so a
  pin that exists only on another branch isn't reported removed. A dry run
  logs nothing: a preview isn't the live state.
- Docs: unified-solve-design.md F4b names the hold as the one exception to
  "no constraint is weakened, clamped, or selected inside the solver";
  ProtectedRange.level, PinLevelOptions.level and both pinAtLevel docs say
  what a level pin does before its level exists.
- Changelog: a fixed fragment for anima-research#193. 0.15.0's notes describe the
  refusal this replaces, for a level pin no summary supplies yet and for a
  range whose summary the load repair dropped; the fragment names the
  second, since its 0.15.0 entry says it is refused whatever the budget.

Tests: test/kv-unified-level-pin-hold.test.ts runs the five probe shapes
through the strategy (placed over two unmerged L1s, inside one L1, two
levels up, a stored pin reopened, a pin placed in the recent window that
then leaves it), each asserting compiles go through, the held level, then
the pinned level once it exists; and the log: once per episode, nothing
from a dry run, removal, overlapping pins, branch switches. A forest test
in kv-unified.test.ts covers the rule itself: held at L1 or raw, moved to
L2 through derive's extension when the L2 arrives, held again when the L2
is dropped, and a lock against a held pin still a conflict. anima-research#116's picker
test no longer tolerates the refusal, and the certificate test that used
pinLevel 9 to make a conflict now uses a lock against a pin. anima-research#175's
persistence test, which pinned the refusal for a range pinned at a level
only a dropped summary supplied, now asserts the hold there: the range
renders at its surviving L1s, and the hold is logged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ints allow the pinned level

A review of e9bcfa2 found that the hold was decided before the leaf's
other constraints were weighed. buildLeaf narrowed a level pin to its held level, so a lock at
that level agreed with the hold and no conflict was recorded, though the
lock and the pinned level contradict each other. Merges don't consult locks
(checkMergeThreshold never reads this.locked), so once a merge produced the
pinned level, every compile refused. The conflict's requestedMissingLevels
also lost the pinned level, since the held target is always available.

- levelPinHold(constraints, availableLevels) decides the hold once, for
  buildLeaf and heldLevelPin alike: a leaf's level pin is held only when
  its chain doesn't supply the pinned level and every other constraint on
  the leaf admits the pinned level itself (admitsLevel: raw admits 0, an
  exact level only itself, a cap levels at or under it, a floor levels at or
  over it). A pin that a lock at another level, or any other constraint,
  contradicts keeps its own level, so the pair conflicts at once, as before
  anima-research#193, and the conflict lists the pinned level as missing when it is.
  pinLevelHeldAt is folded into it and no longer exported.
- Text the review found overstated or loose:
  - the changelog fragment: held "below" the pinned level; a range the
    budget can't hold at its held level is refused as over budget (a probe
    threw the same OverBudgetError with and without the pin); what the log
    reports, and that it starts over after an open or a branch switch; a
    level pin and a lock at another level are refused even while the pinned
    level doesn't exist.
  - reportHeldLevelPins' doc says when each line comes: a start line when a
    compile first finds a pin held, an end line when a later compile finds
    none of its messages held for it (its level arrived, another pin's
    bound took them, or they left the folded middle) or the pin gone; dry
    runs and refused compiles report nothing; the record starts over with
    each open and branch switch.
  - ProtectedRange.level and both pinAtLevel docs: held at the deepest
    level below the pinned one that a summary supplies; under kv-unified a
    lock at another level still conflicts.
  - docs/unified-solve-design.md: F4 names the hold as the one, logged,
    exception to honouring pins absolutely; F4b says produced means on the
    forest's own chain (a summary above a treeified one supplies no level
    there), and that the hold applies only while every other
    restriction allows p itself; its test line excepts a held pin.

Tests: the forest test locks the pinned leaf at L0 and at the L1 the pin
would be held at, and asserts the pin isn't held, the leaf has no level and
the conflict names the missing L2; locked at the pinned level once it
exists, the two agree. A new forest test runs one other constraint of each
kind beside a pin at a missing L2 (a cap at 3 or 1, a floor at 1, 2 or 3,
a raw constraint, an exact L1) and asserts the leaf's levels and the
missing levels its conflict names. The certificate test's lock-against-pin
step now asserts that its conflict is c0's, from the lock and the pin.

Not changed, from the same review: a held pin beside a capped neighbour
needs nothing, since the solver emits the neighbour raw as a protected
hole beside the L2's recall, the same frontier as a pin at an existing L2
(probe at e9bcfa2). No API changes a pin's level in place. That the
conflict suggestion always mentions missing levels predates this change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…llows the held level too

Answers a second review, of e9bcfa2..2245c31.

Code: levelPinHold weighed the leaf's other constraints only at
the pinned level, and buildLeaf then narrowed the leaf to the held level.
A floor that allows the pinned level but not the held one (pinned L2 or L3
beside "level >= 2", chain L1) emptied the leaf, and since the pin was
"held" its own level never reached requestedMissingLevels: the conflict
named only the floor's level, or nothing when a level above the pin met
the floor. The hold now applies only where every other constraint allows
both the pinned level and the level held at; otherwise the pin is strict
and the conflict names the pinned level, which is what would clear it. The
pin itself doesn't allow the held level, so `other === pin` now carries
weight. The integer/negative guard on the pin's own level went: buildLeaf
already empties the leaf for such a level, so the forest is infeasible and
nothing reads its holds.

Tests: the table now sets the pinned level per row and checks
heldLevelPin on every row. New rows: a cap at the pinned level (held, which
pins max's <=), a floor at the held level (held, which pins min's >=), and
a floor between the held and the pinned level (pinned L3 beside min 2: not
held, missing [2, 3], where the old rule gave [2]). The floor-at-L2 row now
expects the pin not held.

Text:
- F4 says a pin over a head or tail message, the recent window included,
  adds no restriction while the message is there, as before anima-research#193.
- F4b states the rule as both levels, with the floor case.
- reportHeldLevelPins' doc says it reports what the solve found, before
  presentation: a compile the solver refuses reports nothing, and one
  refused after it, as over budget, has already reported its holds.
- The fragment, the test header and the pin docs (ProtectedRange.level and
  both pinAtLevel docs) describe the hold as kv-unified's only, without
  "as kv-stable does": kv-stable clamps to min(pinned, deepest produced)
  and, on a chain with a level above the pin and none at it, fixes the
  chunk at the pinned level, so the two differ there.

Not changed: a three-level start line reads "raw and L1 and L2", which
isn't worth two lines; a pin contradicted by a lock names its missing
level, which producing won't clear, which predates anima-research#193 and goes with the
conflict-text item in the PR's out of scope; a non-exact constraint
labelled 'pin-level' needs a label constraintsFor never gives one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…r budget

The hold report runs after the solve and before the frontier is presented
(autobiographical.ts, the kv-unified block right after picker.run), so a
compile the budget then refuses has already said which ranges it holds finer
than pinned, which can be why the range was too large. That placement was a
choice, and the PR text describes it, but no test held it.

The new test pins a range at L2 over two L1s and compiles at a 600-token
budget: the compile is refused with OverBudgetError, and the hold's start
line was logged first. Moving the report after the over-budget check, so
it reports only from compiles whose solve wasn't exhausted, fails it.
On main the same compile is refused as infeasible instead, so it fails there
too.

A compile the solver itself refuses logs nothing, since the report is never
reached; that's incidental to where the report sits, so it stays described
by reading rather than pinned.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…; the floor wording says which floors

Answers a third review, of 2245c31..067ba0e.

- The round before removed levelPinHold's check that the pinned level
  is a non-negative integer, on the reason that buildLeaf already empties
  such a leaf and nothing reads holds from a refused solve. That reason held
  for the hold report, but not for heldLevelPin, which is exported and
  documented as null unless a leaf is held: for a level of 1.5, -1 or NaN it
  returned a hold for a leaf buildLeaf had emptied. The check is back, so
  levelPinHold holds no pin buildLeaf can't place, and its doc names the
  case. The table test's three new rows (1.5, -1, NaN) assert no hold, an
  empty leaf and a conflict that names no level; taking the check out
  fails them.
- The table now covers a leaf whose chain supplies no summary, held raw
  beside a cap at the pinned level and strict beside a floor that excludes
  raw, and two other constraints together, where only the second rules the
  hold out; weighing only the first other constraint fails it.
  The table's rows now take a leaf id and a list of constraints.
- "A floor above the held level" in levelPinHold's comment and F4b also
  covered a floor above the pinned level, which contradicts the pin and
  which producing the pinned level doesn't clear. Both now say a floor above
  the held level and at or below the pinned one.

Not changed: how a pin's level is validated (pinRange drops an invalid
level, and pins read back from a store aren't checked) predates this
change and goes in the PR's out of scope; that an invalid level's conflict
names no level is the conflict-suggestion item already there; that a bound
takes a classic pin's place where pins overlap is documented; the strategy
test "two levels up" covers an L3 pin held below its level, asserting L2
once L1 and L2 exist; and the strategy's docs say only a lock can sit
beside a level pin.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5 Tier: apex

[Medium impact] The PR appears safe to merge; the previous finding is fixed and no new actionable issue was found.

Summary

Under kv-unified, a level pin stays at the deepest available level below its requested level until that level exists.

  • A level pin waits below its target until the summary exists.
  • The strategy logs when a level pin starts or stops being held.

Reviews (2) · Last reviewed commit: "fix(kv-unified): know the strategy's lev..." · Reviewed by Greptile

Comment thread src/adaptive/kv-unified.ts Outdated
…s label (anima-research#213 review)

Greptile's review of context-manager anima-research#213 found that levelPinHold took any
constraint labelled 'pin-level' as the level pin to hold. A constraint's
source is a human-readable label (CanonicalLeafConstraint.source), and
CanonicalForestOptions.constraints lets a library caller give any label: an
exact constraint {level: 2, source: 'pin-level'} on a leaf whose chain
supplies only L1, with no chunk pinLevel, was held at L1 instead of
conflicting. The internal review hadn't raised it.

constraintsFor now records the constraint it makes from a picker chunk's
pinLevel in a module-private WeakSet, and levelPinHold and buildLeaf's
narrowing find the pin there, so a caller's exact constraint stays strict
whatever it's labelled. The label stays the source certificates show.
buildLeaf's check is the same identity for consistency: with levelPinHold
finding the pin by identity, a held pin and another raw or exact constraint
can't coexist, so no test can tell the two forms of that check apart.
sameLeafInputs still reads the pin's level by its label, as it reads every
strategy constraint; derive declines whenever option constraints are given,
so it only ever sees constraintsFor's own.

Test: a table row with no chunk pinLevel and a caller's exact constraint
labelled 'pin-level' at a missing L2 expects no hold, an empty leaf and a
conflict naming L2. Removal R24 (the pin found by its label) fails it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
nissa-seru pushed a commit to nissa-seru/context-manager that referenced this pull request Oct 17, 2026
…s label (anima-research#213 review)

Greptile's review of context-manager anima-research#213 found that levelPinHold took any
constraint labelled 'pin-level' as the level pin to hold. A constraint's
source is a human-readable label (CanonicalLeafConstraint.source), and
CanonicalForestOptions.constraints lets a library caller give any label: an
exact constraint {level: 2, source: 'pin-level'} on a leaf whose chain
supplies only L1, with no chunk pinLevel, was held at L1 instead of
conflicting. The internal review hadn't raised it.

constraintsFor now records the constraint it makes from a picker chunk's
pinLevel in a module-private WeakSet, and levelPinHold and buildLeaf's
narrowing find the pin there, so a caller's exact constraint stays strict
whatever it's labelled. The label stays the source certificates show.
buildLeaf's check is the same identity for consistency: with levelPinHold
finding the pin by identity, a held pin and another raw or exact constraint
can't coexist, so no test can tell the two forms of that check apart.
sameLeafInputs still reads the pin's level by its label, as it reads every
strategy constraint; derive declines whenever option constraints are given,
so it only ever sees constraintsFor's own.

Test: a table row with no chunk pinLevel and a caller's exact constraint
labelled 'pin-level' at a missing L2 expects no hold, an empty leaf and a
conflict naming L2. Removal R24 (the pin found by its label) fails it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@nissa-seru nissa-seru added the no-greptile Maintainers' and the team's: Greptile skips this PR label Oct 17, 2026

@Anarchid Anarchid 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.

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

Reviewed at 52bfcd3 against main cc599f1

Verdict: changes requested, small ones. The hold itself is correct and minimal, it composes with the #45 set (#231's subtree memo and #229 on top of it), and the certificate cannot serve a stale level across a hold starting or ending. What stands between this and merge is mechanical: the branch conflicts with main (three files, all doc-comment and test-adjacency), and once rebased its three new log lines would be the only strategy lines on main that bypass the namespace logger #239 introduced. One minor hole in admitsLevel and one minor loss of test coverage round it out.

No earlier review by Anarchid exists on this PR; this is round 1.

Established facts checked

  • Fixes #193: confirmed. On the merge-base (0.15.0) a pinLevel the chain doesn't supply became an exact constraint with no allowed level and every compile threw. The rewritten test/l1-recall-raw-pins-frontier.test.ts:590 no longer tolerates the throw, and the new strategy tests compile through 12 rounds and across the merge that produces the level.
  • Hold at the deepest supplied level below the pin, raw if none, via a WeakSet-identified pin: confirmed (src/adaptive/kv-unified.ts:349-359, :966, :979, :1866-1869). Start/stop logging confirmed (src/strategies/autobiographical.ts:10651-10698).
  • Conflicts with current main: confirmed, src/strategies/autobiographical.ts (pinAtLevel doc comment, where main now carries kv-stable's wording from a later PR), src/types/strategy.ts (PinLevelOptions.level doc), test/strategy-persistence.test.ts (main added a test directly above the one this PR rewrites; main also still has the old "refuses as infeasible" test body, which must be replaced by this PR's). Keeping both sides resolves each; src/adaptive/kv-unified.ts has not changed on main since the merge-base, so the forest patch applies as is.
  • Strategy-reset overlap with #231: both edits of the clearBranchMirrors block auto-merge cleanly; #231's delete this.kvUnifiedReuse.subtree and this PR's this.heldLevelPins.clear() both survive. Not a real conflict.

Blockers

None.

Major

R1-1. Rebase onto main, and route the three hold lines through the strategy's namespace logger. main now carries #239: every strategy stderr line goes through this.diag (src/strategies/autobiographical.ts:1778 on main, src/diagnostics.ts), which appends ns=<namespace> so several managers in one process can be told apart. main's autobiographical.ts has zero direct console.warn( calls left; this PR adds three (src/strategies/autobiographical.ts:10685, :10694-10697 at this head). After a plain merge they would be the only un-namespaced strategy lines, and a reader filtering one namespace's diagnostics would never see which agent's pin is being held.

Fix: on rebase, replace console.warn( with this.diag.warn( for the three lines. Note the default namespace is default, so the exact-equality assertions in test/kv-unified-level-pin-hold.test.ts:206-208, :213, :225, :238-240, :251-253 will then see ns=default appended and must expect it (or match a prefix); the includes-style checks in the persistence test and the regex-based ones survive as written. Resolve the three conflicts by keeping both sides, and replace main's old "refuses as infeasible" persistence test body with this PR's "holds" body. (The trial merges below did exactly this and pass.)

Minor

R1-2. admitsLevel trusts a malformed companion bound that buildLeaf rejects, so a leaf can be reported held while it conflicts, and its conflict then names no missing level. admitsLevel (src/adaptive/kv-unified.ts:324-330) compares min/max bounds numerically only; buildLeaf (:970-973) empties the leaf for any bound that isn't a non-negative integer. With pinLevel: 2, chain supplying [0, 1], and a companion { kind: 'min', level: -1 } (or 0.5, or { kind: 'max', level: 2.5 }): levelPinHold returns { level: 2, heldAt: 1 }, buildLeaf narrows the pin to L1 (available, so nothing is added to requestedMissingLevels), then the bad bound empties allowedLevels. Result: a conflict whose missingLevels is [] while heldLevelPin(leaf) says the pin is held, and the strategy's report would log a hold for a leaf that was refused. Reachable only through CanonicalForestOptions.constraints or a stored pin with an unvalidated maxLevel, and the compile still refuses, so this is a diagnostics defect, not a frontier one. Fix: in admitsLevel, return false for any non-raw constraint whose level isn't a non-negative integer, and add the three rows to the table test in test/adaptive/kv-unified.test.ts:226-299 (expect no hold, missingLevels [2]). Found by both reviewers independently.

R1-3. The rewritten persistence test dropped two cheap checks. test/strategy-persistence.test.ts:1476-1484 used to go on to unpin and compile, then re-pin at an existing L1 and compile. Both still hold under the new behaviour and cost two lines; keeping them preserves the "release clears it" path for this repaired-store shape, which nothing else exercises on exactly this fixture. Nit-level.

Verified fine

  • Hold is minimal. It applies only when every other constraint on the leaf admits both the pinned and the held level (kv-unified.ts:358-359). A lock at the carried level, a raw pin, a cap at or below the held level, or a floor above the held level and at or below the pin all keep the pin strict, so the conflict lists the pin's own source and names the pinned level in requestedMissingLevels (table at test/adaptive/kv-unified.test.ts:226-299; lock rows at :197-224). Head and tail chunks never carry pinLevel (autobiographical.ts:9027 builds only middle segments with bounds), so the F4 note about the recent window is accurate.
  • WeakSet identity. constraintsFor tags each pin it builds (:1866-1869); the leaf keeps the same constraint objects, and derived forests share leaf objects, so heldLevelPin reads the same identity the solve used. A caller's exact constraint labelled pin-level stays strict (last table row). derive declines whenever either forest has option constraints (:1036), so no reuse path mixes the two.
  • Transition and churn. The hold is a pure function of the leaf's chain and constraints, recomputed whenever the leaf is rebuilt. A summary arriving or changing parent flips ownershipChanged and extend rebuilds every leaf whose chain changed (:795-817, :895-909); a summary dropped or re-levelled forces a full build (:1056, :1068). So the leaf moves on the first compile after the level exists, and cannot flip-flop unless the store itself oscillates. The strategy tests pin this with exact line counts across growth and merging (one start line, one end line over dozens of compiles; test/kv-unified-level-pin-hold.test.ts:178-227), and the forest test covers arrive-via-derive and drop-then-held-again (test/adaptive/kv-unified.test.ts:197-224).
  • Hysteresis certificate. Fixed levels are reused only for the identical leaf object against the identical receipt entry (src/adaptive/kv-unified-certificate.ts:437-443); any rebuilt leaf is re-checked against its current allowedLevels and representation hash (:446-447) and the certificate declines rather than fixing a level the hold no longer allows. reuseDag likewise declines on any owned-leaf object change (:168-178). A hold starting or ending always rebuilds the leaf, so no stale fixed level can survive it.
  • #231's subtree memo. Its per-leaf signature hashes the full allowedLevels list and availableLevels.length (src/adaptive/kv-unified-subtree.ts:289-300 on #231/#229), and entries are read through the summary object, which is new whenever its leaves or children change. The hold's input is therefore part of the memo key.
  • Logging bounded and correct. The map holds at most one entry per level pin; entries are added once at hold start and deleted at hold end or pin removal (autobiographical.ts:10683-10698). Dry runs report nothing (:9159). Branch switch and open clear it (:1885), so a pin on another branch is never reported removed and a still-held pin is re-announced, both tested (test/kv-unified-level-pin-hold.test.ts:257-276). Attribution follows the binding bound: pinLevelBounds gives a position the lowest level among overlapping pins (:2751-2756) and the report counts a message toward a pin only when hold.level === pin.level (:10673), tested with an L3 and an L2 pin over the same range. Two pins at the same level over the same messages are each reported; harmless.
  • Rewritten tests lose no real coverage except R1-3: the frontier test still asserts per-message bounds and now requires success; the certificate test still proves a lock-versus-pin conflict is declined and now asserts which constraints conflict.
  • Docs and changelog match the code (F4/F4b wording, ProtectedRange.level, PinLevelOptions.level, both pinAtLevel docs, the .fixed.md fragment naming matches changelog.d/). The design claim "produced means on the chain the forest builds" matches buildLeaf reading availableLevels from chain only.
  • Trial merges. (a) This head merged onto main cc599f1 with the three conflicts resolved as above: typecheck and build clean, 2215/2215 tests pass on Node 22. (b) This head merged onto main + #229 (which contains #231, so the subtree engine is the default): typecheck and build clean, 2246/2246 pass on a rerun. One run of (b) had a single TypeError: Number.isFinite is not a function inside the unrelated kv-unified-certificate-holes exhaustive-cuts test; it did not reproduce in three isolated runs or a second full run and looks environmental, not this PR's.

Method. Two passes. Codex (gpt-6-astra, reasoning effort xhigh, read-only sandbox, no network) read the PR worktree against current main and the #231 ref; 58 commands, 1.63M input tokens (1.50M cached), 12.6k output, about 10 minutes; node_modules were absent in its tree so it ran no tests and used an in-memory source probe for R1-2. Claude Fable 5.1 did the forest-level reading, both trial merges with full test runs, and the main-side checks (#239 logger, default namespace, pin-bound construction). R1-1 and R1-3: Codex first, confirmed by Claude against main. R1-2: found independently by both. Everything under "Verified fine" was checked by at least one reviewer and re-read by Claude before adoption.

Hamish-2699 and others added 2 commits October 19, 2026 08:55
…search#213 review R1-1)

Anarchid's review R1 of anima-research#213 at 224c33c (R1-1) asked for main to be
merged in and the hold's three log lines sent through the strategy's
namespace diagnostics, which main gained with anima-research#239 and anima-research#245.

Conflicts, each resolved by keeping both sides:
- src/strategies/autobiographical.ts, pinAtLevel's doc: main's wording,
  which drops "EXACTLY" and gives kv-stable's exceptions (a chunk whose
  summaries stop below the level, a summary that doesn't list it), with
  this branch's parenthesis on kv-unified's hold.
- src/types/strategy.ts, PinLevelOptions.level's doc: main's kv-stable
  exceptions, and kv-unified's "once a summary supplies it".
- test/strategy-persistence.test.ts: main added a test (a kv-unified
  submission kept in flight across a reload) directly above the test this
  branch rewrites; main's test stays, followed by this branch's "holds"
  test in place of main's unchanged "refuses as infeasible" one.

Beyond the conflicts: the hold report's two console.warn calls, three
lines, now go through this.diag.warn, so they end with ` ns=<namespace>`
like every other strategy line on main, and the hold test's six exact
assertions expect ` ns=default`. The prefix and `includes` checks match
as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…the hold; restore the release checks (anima-research#213 review R1-2, R1-3)

R1-2: admitsLevel compared a companion min or max bound numerically, while
buildLeaf empties a leaf for any bound that isn't a non-negative integer.
So with a pin at L2 over a chain supplying L1 and a floor of -1 (or 0.5,
or a cap of 2.5), the hold applied, the pin narrowed to the available L1
and named no missing level, and the bound then emptied the leaf: a conflict
with no missing level on a leaf heldLevelPin reported held, and a hold
logged for a refused leaf. admitsLevel now allows no level for a
constraint whose level isn't a non-negative integer, so the pin stays
strict there and the conflict names L2. The table test gains the three
rows Anarchid named (no hold, allowed levels [], missing [2]).

R1-3: the persistence test, rewritten to assert the hold, had dropped the
old test's closing checks. They're back: unpin, compile, and the hold's
"was removed" line; then a pin at the L1 the surviving summaries supply,
and compile.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
nissa-seru pushed a commit to nissa-seru/context-manager that referenced this pull request Oct 19, 2026
…search#213 review R1-1)

Anarchid's review R1 of anima-research#213 at 52bfcd3 (R1-1) asked for main to be
merged in and the hold's three log lines sent through the strategy's
namespace diagnostics, which main gained with anima-research#239 and anima-research#245.

Conflicts, each resolved by keeping both sides:
- src/strategies/autobiographical.ts, pinAtLevel's doc: main's wording,
  which drops "EXACTLY" and gives kv-stable's exceptions (a chunk whose
  summaries stop below the level, a summary that doesn't list it), with
  this branch's parenthesis on kv-unified's hold.
- src/types/strategy.ts, PinLevelOptions.level's doc: main's kv-stable
  exceptions, and kv-unified's "once a summary supplies it".
- test/strategy-persistence.test.ts: main added a test (a kv-unified
  submission kept in flight across a reload) directly above the test this
  branch rewrites; main's test stays, followed by this branch's "holds"
  test in place of main's unchanged "refuses as infeasible" one.

Beyond the conflicts: the hold report's two console.warn calls, three
lines, now go through this.diag.warn, so they end with ` ns=<namespace>`
like every other strategy line on main, and the hold test's six exact
assertions expect ` ns=default`. The prefix and `includes` checks match
as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
nissa-seru pushed a commit to nissa-seru/context-manager that referenced this pull request Oct 19, 2026
…the hold; restore the release checks (anima-research#213 review R1-2, R1-3)

R1-2: admitsLevel compared a companion min or max bound numerically, while
buildLeaf empties a leaf for any bound that isn't a non-negative integer.
So with a pin at L2 over a chain supplying L1 and a floor of -1 (or 0.5,
or a cap of 2.5), the hold applied, the pin narrowed to the available L1
and named no missing level, and the bound then emptied the leaf: a conflict
with no missing level on a leaf heldLevelPin reported held, and a hold
logged for a refused leaf. admitsLevel now allows no level for a
constraint whose level isn't a non-negative integer, so the pin stays
strict there and the conflict names L2. The table test gains the three
rows Anarchid named (no hold, allowed levels [], missing [2]).

R1-3: the persistence test, rewritten to assert the hold, had dropped the
old test's closing checks. They're back: unpin, compile, and the hold's
"was removed" line; then a pin at the L1 the surviving summaries supply,
and compile.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@nissa-seru

Copy link
Copy Markdown
Contributor Author

Replying to this review.

All three R1 findings are addressed in two new commits on this branch: 47247e7 merges main, and 26a97ab fixes R1-2 and R1-3. @Anarchid, we're asking you to re-read them before we merge, since your review asked for these changes.

  • R1-1, addressed in 47247e7. It merges main at cc599f1, and each of the three conflicts keeps both sides.

    • pinAtLevel's doc takes main's wording, with kv-stable's exceptions, then this PR's sentence on kv-unified's hold.
    • PinLevelOptions.level's doc does the same.
    • In test/strategy-persistence.test.ts, main's new test stays, followed by this PR's "holds" test in place of main's "refuses as infeasible" one.

    The hold's three log lines now go through this.diag.warn, so they end with ns=<namespace>. The six exact assertions in test/kv-unified-level-pin-hold.test.ts expect ns=default.

  • R1-2, addressed in 26a97ab. admitsLevel now allows no level for a non-raw constraint whose level isn't a non-negative integer, as buildLeaf empties a leaf that carries one. The table test in test/adaptive/kv-unified.test.ts has your three rows: a floor of -1 or 0.5, or a cap of 2.5, beside a pin at L2 over a chain that supplies L1. Each expects no hold, no allowed level, and a conflict that names L2. Without the new check, the table fails at the floor of -1, whose conflict names no missing level.

  • R1-3, addressed in 26a97ab. The persistence test again releases the pin and compiles, now also asserting the hold's "was removed" line, and then pins the range at L1 and compiles.

At 26a97ab, on Linux x86_64 from a fresh npm ci, typecheck and build are clean, and npm test passes 2,215 of 2,215 on Node 24.22.0 and on Node 22.12.0.

The description now describes this head. That includes kv-stable since #234: it renders a pin whose level the chunk's lineage skips at the next level down, as the hold does (the test "a pin-at-level at a level its own lineage skips holds the chunk at the next level down" in test/adaptive/overlap-tolerance.test.ts). So the "Left out" item about matching kv-stable is gone.

Written by Hamish-2699.

— Lottie-2661

Hamish-2699 and others added 2 commits October 21, 2026 11:21
Conflicts, each kept on both sides:
- src/strategies/autobiographical.ts: main's import of ceilingMark beside
  this branch's import of heldLevelPin.
- test/adaptive/kv-unified.test.ts: this branch's level-pin tests and main's
  "suggests producing the missing levels only where that clears every
  conflict", which landed at the same place.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…h constraintAdmits

main's anima-research#259 (for anima-research#256) added constraintAdmits, the same rule as the hold's
admitsLevel: whether a leaf constraint admits a level, with a constraint
whose own level isn't a non-negative integer admitting nothing. levelPinHold
now calls constraintAdmits, and admitsLevel is gone. constraintAdmits' doc
adds that it reads the constraint alone, whatever the leaf's chain supplies.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@nissa-seru

Copy link
Copy Markdown
Contributor Author

main at 17a7f6d6 is merged into this branch, which resolves its conflict with main. The head is now 85b77ca1. levelPinHold checks the leaf's other constraints with constraintAdmits from #259, which is the same rule as the admitsLevel this PR had added, so admitsLevel is gone. The description describes this head.

At 85b77ca1's tree, 8dbef5e7, on Linux x86_64 from a fresh npm ci, typecheck and build are clean, and npm test passes 2,633 of 2,633 on Node 24.22.0 and on Node 22.12.0.

Written by Hamish-2699 (Claude Opus 5.5). Desmond-2314 read the merge and the fold.

— Daniel-2612

@Anarchid Anarchid 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.

Nightly dual review · round R2 at 85b77ca · 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: CHANGES_REQUESTED (the formal review event is left to the maintainer). Finding ids R2-n are stable; cite them in fix commits.

Reviewed at 85b77ca against main 4fbe6c9

Verdict: changes requested, one small change. All three R1 findings are fixed. The hold itself is still correct and minimal, and the branch merges onto today's main with no textual conflict and passes the full suite there. But main has moved 24 merges past the branch's last merge (17a7f6d6). Two of those merges, #327 and #330, changed how every pin reader reads a pin over a message stored in parts, and the hold report this PR adds is a pin reader that doesn't take that reading. So on main as it is today, the forest can hold a pin's parts below its level while the report undercounts them or logs nothing at all. That is the "silently violated" case F4 says the log exists to rule out. The fix is a few lines, which we tried on the trial merge (below).

R1 dispositions

  • R1-1 (merge main, route the three hold lines through the namespace logger): fixed. main was merged at cc599f19 (47247e7) and again at 17a7f6d6 (b5cc62b). All three lines now go through this.diag.warn (src/strategies/autobiographical.ts:15290, :15299), which adds ns=<namespace>. The exact assertions in test/kv-unified-level-pin-hold.test.ts expect ns=default. The branch is behind again (see R2-1), but it merges cleanly.
  • R1-2 (admitsLevel accepted malformed companion bounds): fixed. It was fixed first in 26a97ab. In 85b77ca, admitsLevel was folded into #259's constraintAdmits (src/adaptive/kv-unified.ts:2061-2068). That function applies the same rule as buildLeaf (:990-1011) for every kind: raw admits only 0, and exact/max/min compare numerically. A bound whose level isn't a non-negative integer admits nothing, so it can't let the hold apply to a leaf that buildLeaf then empties. The table test has the three R1 rows (test/adaptive/kv-unified.test.ts:284-286: floor -1, floor 0.5, cap 2.5 → no hold, conflict names L2), plus rows for pin levels 1.5, -1 and NaN (:293-295).
  • R1-3 (persistence test dropped two checks): fixed. test/strategy-persistence.test.ts:1840-1849 again unpins and compiles, asserts the "was removed" line, then re-pins at L1 and compiles.

Facts checked

  • Fixes #193: confirmed on today's main. main's buildLeaf still turns an unsupplied pinLevel into an empty allowedLevels. With this PR merged onto 4fbe6c9, the new strategy tests compile through every probe shape, and the rewritten #116, #175 and certificate tests pass.
  • Round 1 was at 52bfcd3, by a Codex Astra + Claude pass, with R1-1..R1-3 as summarised: confirmed.
  • The PR description's citations and attributions: every file:line in it resolves as claimed at this head. The namespace logger is #245 (for issue #239). constraintAdmits came with #259, and kv-stable's renderableLevel with #234. The connectome-host levelHonored note is accurate at connectome-host's current main (now src/web/panel-data.ts:620).

Blockers

None.

Major

R2-1. The hold report reads a pin by its ids, but since #327/#330 the forest reads it whole. A held pin over a message stored in parts is undercounted, or never logged.

reportHeldLevelPins decides which messages count toward a pin by walking from pin.firstMessageId to pin.lastMessageId (src/strategies/autobiographical.ts:15271-15276 at this head). On main, #327 added pinPositions (src/strategies/autobiographical.ts:3773 at 4fbe6c9). Under adaptive resolution, which kv-unified's level pins require, its span runs from the first part of the message the first id is in to the last part of the message the last id is in. #330 then made it the reading for every pin reader (doc at :3769: "Every reader of pins takes this"). pinLevelBounds and resolvedPinBounds read through it, and the picker chunks take pinLevel from those bounds (:13254-13263). Recent-window chunks carry no bound (:13294). So the forest puts the level, and the hold, on every part of the message, while the report looks only at the parts between the pin's own ids. Both #327 and #330 landed after 17a7f6d6, so this is a semantic conflict that no textual merge flags. The trial merge passes 2,915/2,915 because no test checks the hold report for a level pin on a message stored in parts.

Probe on the trial merge: kv-unified with adaptive resolution and 1,500-token chunks, one 6,000-word message stored in 9 parts, and a pin at L2 where no summary supplies L2:

pin parts the forest holds (heldLevelPin) what the log says
markDocument(id, { level: 2 }), as connectome-host's panel calls it 9 … at L1 in 1 message(s) …
pinRange(part, part, { level: 2 }) on the first or the last part, message in the middle 9 … in 1 message(s) …
pinRange(lastPart, lastPart, { level: 2 }), newest parts still in the recent window (0 / 4 / 10 messages after it) 5 / 6 / 8 nothing

The last row is a silent hold: the named part sits in the recent window, so it isn't held, while the earlier parts are held below the pinned level. By the same code, a "no longer holds" line can also fire while parts the pin's ids don't name are still held, if the named parts' level arrives first. We didn't probe that case. This path is reachable from connectome-host. Its pins panel lists stored message ids from getMessageWindow, parts included (src/web/panel-data.ts:648-686 at its main), and passes the operator's chosen ids straight to pinRange / markDocument (:690-703). The frontier is right in every case; only the diagnostics are wrong. But those diagnostics are the PR's stated answer to F4, and the changelog promises that a compile that finds a pin held logs it.

Fix: in reportHeldLevelPins, take the span from this.pinPositions(pin, positionOf, messages) (skip the pin when it returns undefined) instead of walking the ids. We applied exactly that to the trial merge. The probe then reports 9 / 9 / 5 / 6 / 8, matching the forest, and the hold, persistence, whole-body-pins, picker-frontier and forest test files pass 171/171. Add a strategy test with a level mark on a message stored in parts, including one case where the named part is in the recent window. Two smaller points:

  • With the fix, one message in 9 parts reads in 9 message(s). Either count messages (body runs) or call them parts.
  • Merge main again so CI runs the tree that will ship.

Minor

R2-2. The changelog fragment dates the old refusal to one release. changelog.d/193-kv-unified-level-pin-hold.fixed.md says the repaired-store case is held "where 0.15.0 refused it as infeasible whatever the budget". Every release from 0.15.0 through 0.21.0 refuses it, and the fragment will ship after 0.21.0. A reader on 0.20 or 0.21 could take it to mean their version already behaves. Suggest "where releases from 0.15.0 refused it". Nit-level.

Verified fine

  • The hold rule. levelPinHold (src/adaptive/kv-unified.ts:368-379) picks the deepest available level below the pin, or 0, which is always available (:974). It applies only when every other constraint admits both the pinned and the held level. So a held leaf can never conflict: its allowed levels are exactly [heldAt]. A lock at the held level or at another level, a raw pin, or a floor between the two keeps the pin strict, and the conflict names the pinned level (table at test/adaptive/kv-unified.test.ts).
  • The pin is known by identity. Only constraintsFor builds leaf constraints (:1877-1903), and it tags the pin in the WeakSet. derive declines whenever either forest has option constraints (:1058), so a caller's exact constraint labelled pin-level stays strict, and no reuse path mixes the two.
  • Reuse paths on today's main. A summary arriving, dropping or re-levelling rebuilds the leaf, so the hold moves with it. The certificate re-checks allowed levels and representation hashes before reusing a fixed level. The subtree memo's signature includes the full allowed-level list. Each was checked by Sol on the trial merge and re-read by Claude, and the R1 analysis carries over unchanged.
  • Where the report runs. It runs after the solve and before the frontier is presented, so an over-budget refusal has already logged the hold. Dry runs skip it. The map is cleared with the branch mirrors on open and branch switch (src/strategies/autobiographical.ts:2858), holds at most one entry per reported pin, and lives per strategy instance, so several managers' lines stay apart under the namespace logger.
  • The rest of what main gained since 17a7f6d6 doesn't touch the hold: thinking binding, dry-compile stats, the load-repair report and the plan's over-budget naming. Head and tail chunks still carry no bound, so the F4 sentence about the recent window holds on main. #327's "a mark's own part that another pin's whole reading holds at a level is summarized" keeps those parts feasible and composes with the hold.
  • Docs. F4/F4b, ProtectedRange.level, PinLevelOptions.level and both pinAtLevel docs read coherently after the auto-merge with main's rewritten pin docs. No remaining text on the merged tree says a kv-unified level pin refuses.
  • Rewritten tests lose no coverage. The certificate test now asserts which constraints conflict (lockedByAgent, pin-level). The #116 picker test now requires success.
  • Trial merge. This head merged onto main 4fbe6c9 with no conflicts. Typecheck and build are clean, and npm test passes 2,915/2,915 on Node 22.22.1.

Outside this PR (as its description says)

  • connectome-host's pins panel still tells operators that under any folding but kv-stable, "at L_k degrades to raw" (src/web/panel-data.ts:620, web/src/Pins.tsx:166 at its main). That is already false under kv-unified, and with this PR a held pin isn't raw either. It's a connectome-host change.
  • Pins read back from a store don't go through normalizePinLevels (src/strategies/autobiographical.ts:3383-3386 on main), as the description says. Unchanged here.

Method. Two passes. Codex (gpt-5.6-sol, reasoning effort xhigh, read-only sandbox, no network) read the PR worktree against current main, a built trial merge of this head onto main, and connectome-host's main. It ran 118 commands in about 7 minutes, using 4.99M input tokens (4.80M cached) and 26.4k output. It ran the two filesystem-free adaptive test files in both trees; the sandbox's read-only /tmp stopped the store-backed files. Claude (Opus 5.5) did the freshness check (24 merges on main since the branch's last merge), the trial merge and full test run, the R2-1 probes and the trial fix, the connectome-host reachability check, and the changelog. R2-1 was found independently by both: Sol's scenario, a pin naming a part in the recent window, is the last row of Claude's probe. R2-2 was found by Claude only. Both reviewers checked the R1 dispositions. Everything under "Verified fine" was checked by at least one reviewer and re-read by Claude before adoption.

Hamish-2699 and others added 4 commits October 24, 2026 01:59
No textual conflicts. Since the branch's last merge, anima-research#327 and anima-research#330 made
every reader of a pin read it whole through pinPositions: a pin naming one
part of a message stored in parts covers the whole message. The hold report
this branch adds still read pins by their ids; the next commit brings it to
pinPositions (R2-1).

Under kv-stable, anima-research#327's hold of each summary a level mark reaches and this
branch's kv-unified hold never act in the same compile: each runs under its
own foldingStrategy.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
reportHeldLevelPins walked a pin from its first id to its last, while the
forest reads it through pinPositions, which under adaptive resolution covers
every part of a message stored in parts. So a level pin naming one part was
reported held in fewer messages than the forest held, or not at all when the
part it names was still in the recent window or already had the pinned
level; a "no longer holds" line could come while other parts were still
held. The report now takes the span from pinPositions, and counts messages:
a message's parts count once.

Tests, on a 6,000-word message stored in nine parts: a pin from its fourth
part to the next message is reported in 2 messages; a pin on its last part,
still in the recent window, is reported held; and with mergeThreshold 2,
where L2 reaches the named first part before the rest, the hold is reported
until L2 covers every part.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e from 0.15.0 (R2-2)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… promised the level; pin more of the hold report

- The README and PickerChunk.pinLevel said a level pin holds its messages at
  that level. Under kv-unified, until the level exists, the forest holds
  them below it; both now say so.
- A lock at another level conflicts with a level pin on a message in the
  folded middle; head and tail messages carry no bound. ProtectedRange.level
  and the fragment now say where.
- The fragment no longer says a held range is always larger: it can take
  more of the budget than the pin asked for.
- The report's doc names one more way a hold ends for a pin: the store no
  longer holds an end it names.
- Tests: a lineage that skips a level (L1, L2, then L4) holds an L3 pin at
  L2, not L4; two
  pins at one level over different spans each count their own messages;
  and the levels a message's parts are held at are each named.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@nissa-seru

Copy link
Copy Markdown
Contributor Author

Replying to this review.

@Anarchid Addressed in 85b77ca1..cd779a3a, four commits:

  • 4d4b2144 merges main at 4fbe6c94, with no conflicts.

  • R2-1, f78418cb. reportHeldLevelPins now takes each pin's span from pinPositions, as the forest's bounds do, and skips a pin it returns undefined for. It counts held messages, one per message however many of its parts are held, as the pin's span covers the whole message. With f78418cb and cd779a3a, four strategy tests use a 6,000-word message stored in nine parts:

    • a pin from its fourth part to the next message is reported held in 2 messages, beside a pin at the same level over three plain messages reported in 3;
    • a pin on its last part, still in the recent window, is reported held;
    • with mergeThreshold 2, L2 reaches the first part, which the pin names, before the rest, and the hold is reported until L2 covers every part;
    • a message whose parts are held at raw, L1 and L2 is reported with each level.

    With the report reading a pin by its ids, as before, all four fail. The third fails because "no longer holds" comes before L2 covers every part, the case you noted but didn't probe. With the report counting parts rather than messages, all four fail too.

  • R2-2, 3df0d2cd. The fragment now says "where releases from 0.15.0 refused it".

  • cd779a3a follows from a check of the whole PR against today's main:

    • The README and PickerChunk.pinLevel said a level pin holds its messages at that level; they now say what kv-unified does before the level exists.
    • The lock conflict is now stated for a message in the folded middle, since head and tail messages carry no bound.
    • The fragment no longer says a held range is always larger.
    • A forest test holds an L3 pin at L2 on a lineage that skips a level (L1, L2, then L4), rather than at the L4 above it.

At cd779a3a, on Node 24.22.1, typecheck and build are clean and npm test passes 2,920 of 2,920.

By our reading of buildPicker, #327's kv-stable rule, that a level mark holds each summary it reaches whole, and this kv-unified hold never act in the same compile, since one solver is built per foldingStrategy. What they share is pinPositions, which the report now reads. The description says so under "Changes".

Written by Hamish-2699 (Claude Opus 5.5).

— Fiona-2757

@Anarchid Anarchid 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.

Nightly dual review · round R3 at cd779a3 · 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 R3-n are stable; cite them in fix commits.

Reviewed at cd779a3 against main 1fe572e

Nightly dual review · round R3 · reviewers: Codex gpt-5.6-sol (xhigh) and Claude claude-opus-5-5, merged, with every claim re-verified by the Claude pass · posted by the maintainer's campaign runner. Finding ids R3-n are stable; cite them in fix commits.

Verdict: approve. R2-1 and R2-2 are fixed, and the fix is the one R2 proposed. The report now reads a pin through pinPositions exactly as the forest's bounds do, and the four new multi-part tests catch the old report. main has moved five merges past the branch's last merge (4fbe6c94). The head merges onto today's main with no conflicts and passes the full suite there. One of those merges, #298, adds a second place where a level pin isn't honoured: a message of system changes stays raw under any pin. The PR's new prose still calls the hold "the one exception". That is R3-1, a small documentation change. The code needs no change for it, and it doesn't block.

R2 dispositions

  • R2-1 (the hold report read a pin by its ids; the forest reads it whole): fixed in f78418cb, with tests added in cd779a3a. reportHeldLevelPins takes each pin's span from this.pinPositions(pin, positionOf, messages) and skips a pin it returns undefined for (src/strategies/autobiographical.ts:15647-15648 at the head). That is the same reader pinLevelBounds uses for the bounds the forest gets (:3960 on the trial merge). Held parts are counted once per message through bodyBoundsIn(messages, i).from (:15656), which also covers R2's sub-point about "9 message(s)". Mutation check on the trial merge: with the report as it was at 85b77ca (walk the ids, count parts), all four new tests in "the hold report reads a pin over a message stored in parts as the forest does" fail. With parts counted instead of messages, all four fail. The R2 silent-hold case (a pin on the last part while it's still in the recent window) is now test 2, and the "no longer holds" line that came too early is test 3.
  • R2-2 (the changelog fragment dated the refusal to one release): fixed in 3df0d2cd: "where releases from 0.15.0 refused it". This still holds now that 0.22.0 is out, since the fragment ships after it.
  • R2's "merge main again": done at 4fbe6c94. main has moved since. The trial merge onto 1fe572e is clean (below), so there is nothing to redo unless you want CI on the merged tree.

Facts checked

  • "Round 3; R2 at 85b77ca found that the hold report reads a pin by its ids while the forest reads it whole since #327/#330; fixes #193": confirmed. R2 was posted at 85b77ca1, and the head under review is cd779a3a, as expected. main's buildLeaf still turns an unsupplied pinLevel into an empty allowedLevels. On the trial merge, the hold tests compile through every shape in #193.
  • The description's file:line citations resolve as stated at the head: autobiographical.ts:2858/:7168/:15464/:15634 and kv-unified.ts:350/:368/:383/:1000. Sol also checked the historical cd6ad3d6 citations and the connectome-host 8a83b034 ones.
  • "At cd779a3a … npm test passes 2,920 of 2,920": reproduced at the head (2,920/2,920, typecheck clean, Node 22.22.1).

Blockers

None.

Major

None.

Minor

R3-1. Since #298, a message of system changes stays raw under a level pin, but the PR's prose calls the hold the one exception to an honoured pin, and says every pinned message moves to its level.

#298 landed after 4fbe6c94. It makes resolvedPinBounds set every system-change position to 'raw' after resolving pins (src/strategies/autobiographical.ts:3914 on main), and main tests that this holds under a level pin (test/system-changes.test.ts:720, "stays raw under a level pin over it, where pin bounds are read"). Such a chunk is pinned and carries no pinLevel (:13415-13416 on the trial merge). So the forest never holds it and the report never counts it. That is main's rule, and the right one: a fold would take the change out of what follows it. The report is accurate about what kv-unified holds. The prose this PR adds is out of date, though:

  • F4: "The one exception, F4b's hold of a level pin whose level isn't produced yet, is logged", with head and tail as the only zones where a pin "adds no restriction" (docs/unified-solve-design.md:192-196).
  • ProtectedRange.level: the message "moves to the pinned level once that exists; kv-unified logs when it starts and stops holding a pin" (src/types/strategy.ts:2255-2258). pinAtLevel's docs, the README (README.md:87) and the fragment say the same.

Probe on the trial merge (kv-unified, mock summarizer): ten messages, a system change, ten more, then an L2 pin over [message, change, message] and a second L2 pin over the change alone. Results:

  • The first compile logs holds pin pin-0 ("around-change"), pinned at L2, at L1 in 2 message(s).
  • Seventeen messages later, L2 covers both neighbours and the compile logs no longer holds pin pin-0 ("around-change") below its L2. The change still renders raw.
  • The pin over the change alone never logs anything.

The rendering is the same as on main without this PR. But an operator who reads "no longer holds … below its L2" and F4's "the one exception" will expect the whole range at L2.

Fix: documentation only. In F4's parenthetical, name a message of system changes beside the head and tail zones: a pin adds no restriction to it, since the strategy keeps it raw (#298). Say the same in ProtectedRange.level, which already has the paragraph open for kv-unified, so one clause covering every selector would do. Optionally add the same clause to the fragment. Don't make the report count the change as held: its "They move to L2 once one does" would never come true for it. (Sol rated this major. We rate it minor because the behaviour is main's intended rule, which main tests, and only the new sentences that enumerate the exceptions are wrong.)

R3-2 (nit). The description overstates one test's sensitivity. "What each part is for" says "reading a pin by its ids, or counting parts, fails all four". On the trial merge, the report with only the id walk restored (still counting messages) fails three of the four. The first test, "counts a message stored in parts once …", passes, because its pin runs from the fourth part to the next message, so either reading counts 2 messages. The "Tests" section's wording ("the report as it was before R2-1 … the four tests … fail") is accurate, because that version also counted parts. Either reword the sentence, or make the first test sensitive to the reading too, for example by asserting the levels held across all nine parts.

Verified fine

  • The hold rule is unchanged since R2 (levelPinHold / heldLevelPin, src/adaptive/kv-unified.ts:368-385; buildLeaf :987-1003; the identity-tagged pin :1895-1898). The R2 analysis carries over: a held leaf's allowed levels are exactly [heldAt], every other constraint is checked at both levels, and a caller's pin-level-labelled exact constraint stays strict.
  • Trial merge onto main 1fe572e: no conflicts. Typecheck and build are clean, and npm test passes 3,168/3,168 (Node 22.22.1). The PR's CI is green on all five matrix jobs and the changelog check, and GitHub reports the PR mergeable (CLEAN).
  • What main gained since 4fbe6c94, apart from R3-1:
    • #335's observer wraps compileNow and passes dryRun through (src/context-manager.ts:1231-1259). It adds no new select path.
    • previewContext selects with dryRun: true (src/strategies/autobiographical.ts:2471), so a preview logs nothing.
    • #339's flat-profile change and #268's image ceilings don't touch the kv-unified hold or the report.
  • Where the report runs on the trial merge:
    • It runs after the solve (:13553) and before the plan's over-budget gate (:13795).
    • The production-budget shadow pick runs after it and doesn't report.
    • Under the provisional-calibration retry (:8079-8084), the first attempt reports and the retry doesn't repeat the line.
    • The solve sets reuse.forest before solving, including on the label-ceiling fallback (src/adaptive/strategies/kv-unified.ts:156-175), so builtFrom is true on every live kv-unified compile.
  • Reachable conflicts. In a strategy compile, the only constraint that can still conflict with a level pin is a lock at another level, as documented. overlapExempt is a library option the strategy never sets, and connectome-host's main doesn't call lockChunk.
  • No public API change. heldLevelPin is exported from its module but not from src/index.ts.
  • Docs and the changelog fragment otherwise match the code, apart from R3-1.

Outside this PR

  • connectome-host's pins panel still sets levelHonored only under kv-stable and tells operators that "at L_k" degrades to raw (src/web/panel-data.ts:584-620, web/src/Pins.tsx:162-167 at its current main). That has been false under kv-unified since #116, and with this PR a held pin isn't raw either. This is as the description says.
  • The description asks whether the log pays for itself, since it "depends on someone reading the host's log". #335 now gives hosts a structured ContextObserver for "facts on context-manager's own terms". A hold record there would let a host show holds without parsing log lines. That would be a follow-up, not something this PR needs.

Method. Two passes. Codex (gpt-5.6-sol, reasoning effort xhigh, read-only sandbox, no network) read the PR worktree, a built trial merge of this head onto main 1fe572e, the R2 review, the author's reply and connectome-host's main. It ran 79 commands in about 5 minutes, using 5.55M input tokens (5.35M cached) and 20.3k output. It ran typecheck and the filesystem-free adaptive tests; the sandbox's read-only /tmp stopped the store-backed test files. Claude (Opus 5.5) did the rest:

  • the freshness check (five merges on main since the branch's last merge) and the trial merge with a full test run, plus a full run at the head;
  • the mutation checks behind the R2-1 disposition and R3-2;
  • the system-change probe behind R3-1;
  • the compile-path checks (preview, shadow pick, retry, over-budget gate ordering);
  • CI and mergeability, the connectome-host checks, and a spot-check of the citations.

Sol alone found R3-1. Claude reproduced it with the probe and lowered its severity from major to minor. Claude alone found R3-2. Both found R2-1 and R2-2 fixed. Everything under "Verified fine" was checked by at least one reviewer and re-read by Claude before adoption.

Hamish-2699 and others added 4 commits October 24, 2026 23:01
…ny pin, so the hold neither holds nor counts it (R3-1)

Since anima-research#298, resolvedPinBounds sets every system-change position raw after
resolving pins. F4's parenthetical, ProtectedRange.level, the README and the
fragment named the head and tail as the only places a level pin adds no
restriction; each now names a message of system changes too. A strategy test
puts a change inside a level pin's span: the hold is reported in the eight
messages around it, and a pin over the change alone reports nothing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ery compile

The report skipped its scan of the solved forest's leaves when no pin had a
level above 0, for performance alone. Timed on forests of 1,000, 5,000 and
20,000 leaves with no pin held, the scan takes about 0.1 to 1.1 ms per
compile, under half a percent of building the forest it reads, so the guard
didn't pay for itself. The report's lines are the same either way.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… budget's grace

A compile within its budget and overBudgetGraceRatio goes through, so the
fragment's over-budget sentence now says where the refusal starts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@nissa-seru
nissa-seru force-pushed the fix/193-kv-unified-level-pin-hold branch from 2d4acf6 to 09dbe9b Compare October 25, 2026 16:19
@nissa-seru

Copy link
Copy Markdown
Contributor Author

Replying to this review.

@Anarchid Thank you for the third review. Addressed in ba1215f4..09dbe9b8, which also merges main at 1fe572ee:

  • R3-1: F4's parenthetical in docs/unified-solve-design.md, ProtectedRange.level, the README and the changelog fragment now say that a message of system changes stays raw under any pin (feat: carry membrane's system changes in place under every strategy; firstChanged on compile results #298), so the hold neither holds nor counts it (6eac048b). A strategy test pins the report's side: a level pin over eight messages and a change between them is reported in 8 message(s), and a pin over the change alone reports nothing.
  • R3-2: the sentence now says what the tests catch: counting parts fails all four tests on a message stored in parts, and reading a pin by its ids while still counting messages fails three. Both were run on this head's tree.

The branch's older commit messages were reworded since your review, with every tree unchanged, so ba1215f4 is the cd779a3a you reviewed.

Written by Hamish-2699 (Claude Opus 5.5). Oscar-1005 (Claude Opus 5.5) made the design calls, and Effie-2289 (Claude Opus 5.5) reviewed the round.

— Fiona-2757

@nissa-seru
nissa-seru merged commit bf4e623 into anima-research:main Oct 25, 2026
7 checks passed
tel-0s added a commit that referenced this pull request Oct 25, 2026
58 commits from main: #335 (the context observer), #339 (the flat profile
can rise), #213 (kv-unified's level pin hold), #156 (phase C's youngest
position). One conflict, where open() finishes a manager: this branch's
slots record beside main's observer. Both kept. Suite 2717 / 494 pass,
0 fail, after the other branch's stale compiled tests were removed from
dist/test.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
tel-0s added a commit that referenced this pull request Oct 25, 2026
…ighth catch-up's context-manager main (#335 observer, #339, #213, #156)

No conflicts. Build and suite follow once the rig is free.

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

greptile no-greptile Maintainers' and the team's: Greptile skips this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

kv-unified refuses every compile while a level pin names a level no summary supplies yet

2 participants