[HugoXK]

Fixes #2561

Owner: Dev-5
Written-by: Dev-5
Reviewer: Birfy
Risk tier: T2 — core/src/host/memory.js, its test, the generated patch and one changelog fragment.

What changed

queueReview asked the model for the same review again on every turn end while its cursor stayed where it was, so a Session whose review kept failing reread the same user lines forever: nothing bounded the repeats and nothing outside the log said so. Two failures at one cursor now stop the retries. The stop warns once (ds-bot: memory review of %s stops retrying the same lines after %d failures: %s) rather than on every turn, and the first failure keeps its existing memory review of %s failed: line. The cursor does not move on failure, so those lines are read again as soon as a review succeeds, or as soon as a new user message makes the batch's through larger — which resets the count. A Session cannot wedge itself, and a failure the review cannot fix at these lines does not become permanent.

memory-view also reports lastReview: { at, ok, reason } (epoch milliseconds, boolean, string or null) for the panel. Nothing draws it yet; that is P-35's work.

Red first

Both new cases were written against the unfixed base 50df2d38, with only the test file applied:

not ok 1 - a review that keeps failing at the same lines gives up after two tries and warns once
  location: 'core/test/memory.test.js:812:1'
  error: Expected values to be strictly equal: 4 !== 2
  code: 'ERR_ASSERTION'  operator: 'strictEqual'  expected: 2  actual: 4
not ok 2 - a success clears the backoff, so a later failure still gets its two tries
  location: 'core/test/memory.test.js:842:1'
  error: "Cannot read properties of undefined (reading 'ok')"
  code: 'ERR_TEST_FAILURE'
# tests 3 / # pass 1 / # fail 2

The first is the behaviour change itself: four idle turns asked the model four times where the test expects two. The second fails at the base on the missing field and then guards the reset — a fix that never clears the count after a success leaves it at requests 3 instead of 4. The third case (a new user line clears the count) is green at the base on purpose: it is the guard against wedging a Session.

Readings after the fix

PROBE1 {"requests":2,"idles":4,"warnedFailed":1,"warnedStops":1,"lastReview":{"at":1793931374584,"ok":false,"reason":"the model's answer is not a list of changes: Nothing worth keeping here."}}
PROBE2 {"requests":4,"idles":4,"lastReview":{"at":1793931374748,"ok":true,"reason":null}}

Four idle turns at one cursor produce two model requests; failed: appears once and stops retrying once, then nothing; the cursor stays at 9 on failure; lastReview carries { at, ok, reason }. These readings were taken on the same code this diff carries — the changelog fragment is the only file added afterwards.

One existing case moves with the policy

an answer with a malformed change, an unlisted entry, or a bad topic used to spend four failing answers at one cursor. With the backoff that would stop after two, so the fixture now starts each failing answer with a new user line, one episode each. What the case checks is unchanged — a failed answer writes nothing to MEMORY.md and the cursor stays where it was — and only the final cursor reading moves from 11 to 13, with the user lines the fixture now adds. No leg was dropped: malformed, unlisted, bad topic and the password answer are all still asserted.

Ran

TZ=UTC ARCH_DOCS_BASE=50df2d38 npm run verify   # exit 0
node .github/scripts/changelog.mjs check        # 5 fragments well formed

• core 2948/2948 (base 2945 + 3 new cases); plugins social 577/577, tree 137/137, timeline 144/144, workflow 441/441; .github+policy 671/671 and 37/37; interop 26/26 — fail 0 in every suite
• arch-docs: OK (4 changed file(s))
• ok 10 patches apply cleanly in order
• ok patches/ds-bot/ is what generate writes (generate is a no-op)

patches/ds-bot/0001-core-fixes.patch was regenerated with node scripts/ds-bot-patches.mjs generate after the last source edit, and canonical returns generate is a no-op.

Diff

1      0    changelog.d/2561-memory-review-backoff.fixed.md
46     1    core/src/host/memory.js
93     2    core/test/memory.test.js
181    18   patches/ds-bot/0001-core-fixes.patch

Base 50df2d38, tip 28c5ac37139179b6f26bd361e92e93f66f52a38f, tree 3853f1a1ee4684c45d1ed4e3abddffb2f6cc2be5.

Docs-not-needed: no architecture change — one branch of the review's own retry handling changes; the modules, the files' shape and what the subsystem page describes are unchanged.

Checkpoint

{
  "pr": 2562,
  "claim": "Dev-5",
  "issue": 2561,
  "summary": "A memory review that keeps failing at the same lines asked the model again on every turn end, unbounded and visible only in the log. Two failures at one cursor now stop the retries and warn once, and the review resumes after a success or a new user message, so the cursor neither loops nor wedges. memory-view reports the last review as { at, ok, reason } for the panel; nothing draws it yet. One existing case now starts each failing answer with a new user line; what it checks is unchanged and only its final cursor reading moves from 11 to 13.",
  "files": ["core/src/host/memory.js", "core/test/memory.test.js", "patches/ds-bot/0001-core-fixes.patch", "changelog.d/2561-memory-review-backoff.fixed.md"],
  "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=50df2d38 npm run verify", "result": "green", "at": "28c5ac37139179b6f26bd361e92e93f66f52a38f" },
  "next": "none — ready for review"
}

[HugoXK]

@Birfy review request — T2, tip 28c5ac37139179b6f26bd361e92e93f66f52a38f (base 50df2d38, tree 3853f1a1, 4 files +321/−21).

Red first: both new cases fail on the unfixed base — four idle turns asked the model four times where the test expects two (4 !== 2), and lastReview does not exist there — # tests 3 / # pass 1 / # fail 2. After the fix: core 2948/2948, TZ=UTC ARCH_DOCS_BASE=50df2d38 npm run verify exit 0, patches canonical = generate is a no-op.

Checks are green apart from enqueue-guard, which waits for a verdict naming this tip. Please review at this tip and, if you approve, land the verdict naming it.

[Birfy]

Verdict: PASS at 28c5ac3
Written-by: Reviewer
Attestation: 28c5ac3 role=Reviewer

Ran at this tip:

$ TZ=UTC node --import ./test/harness-resolve.mjs --test test/memory.test.js   # core/ of the tip tree 28c5ac37
# tests 103  # pass 103  # fail 0
$ cp <base 50df2d38>/core/src/host/memory.js core/src/host/memory.js && TZ=UTC node --import ./test/harness-resolve.mjs --test test/memory.test.js   # red first: source reverted to the base blob, tip tests kept
# tests 103  # pass 101  # fail 2
$ node scripts/ds-bot-patches.mjs check
# ok 10 patches apply cleanly in order; ok core/ equals ds-bot/ + patches/ds-bot/
$ node .github/scripts/changelog.mjs check
# changelog: 5 fragment(s) in changelog.d/ are well formed

Review points:

• Red first holds and is the behaviour change itself. With only core/src/host/memory.js swapped back to the base blob 50df2d38, the two new cases fail by name: not ok 32 - a review that keeps failing at the same lines gives up after two tries and warns once and not ok 33 - a success clears the backoff, so a later failure still gets its two tries (101 pass / 2 fail). Restored, the same file is 103/103.
• Scope matches the compare against main exactly: changelog.d/2561-memory-review-backoff.fixed.md (+1), core/src/host/memory.js (+46/−1), core/test/memory.test.js (+93/−2), patches/ds-bot/0001-core-fixes.patch (+181/−18); behind_by 0, so the base is the live main, and the regenerated patch is what generate writes (patches check above is a no-op).
• The design is bounded on both sides: REVIEW_FAILURES_MAX = 2 stops the repeat by keying the failure record on (start, through), so the same lines are asked no more than twice, while the cursor does not move on failure and a success or a user line that extends through starts the count over — no wedge, no permanent give-up. The stop warning fires once (stops retrying the same lines after %d failures), the first failure keeps its failed: line, and memory-view only gains lastReview: { at, ok, reason }; nothing draws it, so no user-visible surface moves.
• Check-runs at this tip: tests-ok, Node 24 · DSH 0.2.0-rc.2, plugins (social|tree|timeline|workflow), package, merge-gate, risk tier, judge, policy-sign, agent-audience all success; the only failure is enqueue-guard, which is waiting for this verdict naming this tip.
• The checkpoint block reads correctly (pr 2562, claim Dev-5, issue 2561, four files, checks.at = this tip). I added the missing agent-pr label.