Skip to content

test: the memory-review Bot line keeps only an upper bound, so lowering REVIEW_OTHER_LINE_MAX drops context silently #1978

Closed

Description

@HugoXK

Implementation: FlyBot (ours) — PR follows within 24h. Do not pick up without a pickup-ok comment on this issue.

Written-by: FlyBot Lead
Claimed-by: FlyBot Lead
Claim-Id: dcaa85cc5e5f4953ab2cf28d0b61735e
Branch: dev/4-p284-memory-bot-line-bound
Base: 90b880e
At: 2026-11-01T04:00:32Z

Implementation: FlyBot (ours) — PR follows within 24h. Do not pick up without a pickup-ok comment.

Written-by: FlyBot Lead

Base: 90b880e

What is not pinned

core/test/memory.test.js:541 (a long Bot line cut for the review context keeps whole characters (#1918)) asserts only an upper bound on the memory-review line that #1918 rewrote:

  • :561 assert.ok(shown.endsWith('…(cut)')) — the line was cut somewhere;
  • :562 assert.ok(shown.length <= 'Writer: '.length + 800 + ' …(cut)'.length) — it is no longer than the limit.

Both stay true when the limit shrinks. REVIEW_OTHER_LINE_MAX is 800 at core/src/host/memory.js:64, and it is what a Bot's line is clipped to before it reaches the memory-review model (memory.js:1161). Lowering it hands the model a shorter line, silently, and no test goes red.

Reproduction (live main 90b880e7)

In core/:

  1. Baseline: node --import ./test/harness-resolve.mjs --test test/memory.test.js → all green.
  2. memory.js:64 800 → 700: still green — the hole. A hundred units of the Bot's line can disappear and nothing notices.
  3. 800 → 900: one failure, not ok - a long Bot line cut for the review context keeps whole characters (#1918) at memory.test.js:561 (expected: true / actual: false) — so the case guards raising the limit, not lowering it.

Measured on 42e62656, which carries the same blobs as 90b880e7 (memory.js 8ea47c4eb5, memory.test.js c4da9feb9c), so the line numbers above are the ones to use. The direction that loses data is the direction that stays green.

Expected

The Bot-line case pins what survives, the way the neighbouring user-line case already does (memory.test.js:565, lower bound keptHead.length + keptTail.length > MAX - 10 at :600):

Scope

Test-only. One case, strengthened, in core/test/memory.test.js, plus the mirror patches/ds-bot/files/test/memory.test.js, regenerated with node scripts/ds-bot-patches.mjs generate and never hand-edited. No product change: memory.js keeps REVIEW_OTHER_LINE_MAX = 800.

Acceptance

The 700 mutation's red-first output goes in the PR body (case name, actual/expected, # pass / # fail), and the tree is restored byte-identically afterwards. npm run verify green with ARCH_DOCS_BASE=90b880e7; ds-bot-patches check and canonical clean.

Duplicate check

REVIEW_OTHER_LINE_MAX → only #1918 (closed; the very case being strengthened). memory.test.js truncation → no issues. Open issues at this writing are #1967, #1971, #1976 — none about the memory review line. The same class in diagnostics.js was #1922 → fixed by #1924, both closed.

Activity

  1. added a commit that references this issue on Nov 1, 2026
    4488ae8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions