[HugoXK]

Fixes #1918

Owner: Dev
Written-by: Dev-1
Reviewer: Reviewer

Base: ac81a4f160a69859f7aea991cffa88f5078e90d4 (main moved from f87fceb0 to ac81a4f1 while this was being fixed — #1920 merged and rewrote 0001/0007, so the branch was rebased and the patches regenerated on the new main)
Verified commit (local): 618cfe4c5a32f1b71d6d22ad23a7ccecdc3a89c6, tree acd588b223d86e556814104a7f0be15b64b190d4 — the same tree as the pushed head below, so this is the content that was verified.
Pushed head: c6972cc7ec1969ea55c082ec0f3e223b2f89dfd7 — the push path rebuilds the commit on that base from the same tree, so the verified tree is the one above.

The correction this PR carries (from the issue body)

The original premise does not hold. This issue first claimed that core/src/host/memory.js calls the shared clip with two arguments and therefore prints (cut; undefined) into the memory-review prompt and the run log. Checked line by line at f87fceb0, that is false: memory.js imports only { codePoints, splitRef, unquote } from ./text.js (:30) and uses its own local clip (:1146) and ends (:1147), and the only importer of the shared clip anywhere in the tree is core/src/host/parts.js:22, whose three calls (:344, :355, :465) all pass a reason. So no product path reaches (cut; undefined).

What this change actually fixes, one coherent family, not added scope: (1) the exported helper's two-argument contract — a truncating two-argument call must print … (cut), not undefined (one line in text.js, unit-tested); (2) the reachable half of the same defect, the local clip at memory.js:1146 and the local ends at :1147, which bound the memory-review prompt and the run log by slicing UTF-16 units and so put half a surrogate pair in front of the model; (3) the same two helpers' tail edge in ends. All three are "a bound that does not go through the shared boundary rule", which is the rule #1916/#1917 established; the prompt and log paths the title names are fixed by (2) and (3).

Red first (source at ac81a4f1, only the new tests in the tree)

core/test/memory.test.js alone → # tests 65 / # pass 63 / # fail 2; core/test/text.test.js alone → # tests 14 / # pass 13 / # fail 1; both together → # tests 79 / # pass 76 / # fail 3:

• not ok 75 - clip says only that it cut when it is given no reason (#1918) — clip('x'.repeat(900), 800) ends with the literal … (cut; undefined); cut.includes('undefined') → true !== false
• not ok 25 - a long Bot line cut for the review context keeps whole characters (#1918) — the prompt is not well formed; prompt.isWellFormed() → false !== true
• not ok 26 - a long user line cut for the review keeps whole characters at both ends (#1918) — label aaaaaa, prompt.isWellFormed() → false !== true. The case message is 3001 units, but ends is handed the rendered line, 'User: ' + message (3007), so the 2000-unit head budget spends six units on that prefix and cuts at unit 1994 of the message — the emoji's low half for this sample. The same case keeps both ends asserted, so its tail edge (3008 - 1000 = 2008, the emoji's low half) is checked too; an earlier sample put the head cut on a plain a and reached only the tail.

The bound holds from both sides

A legal prompt is not enough: a cut that threw most of the line away would also be well formed. In the two user-line cases keptHead + keptTail is 2993 and 2994 against MAX = 3000, and the test asserts <= MAX and > MAX - 10 (never === MAX).

A temporary mutation proves the lower bound bites: with REVIEW_USER_LINE_MAX set to 1_500 (the 2000-unit head budget halved to 1000) the same test goes red —

not ok 26 - a long user line cut for the review keeps whole characters at both ends (#1918)
  error: 'short: 994 + 500'

not ok 32 - a backlog past the text limit is reviewed in passes, oldest first also goes red in that run (1 !== 2): halving the constant changes how many passes the backlog takes, which is the mutation's own effect, not a defect. The constant was restored before the commit, and the run above with it restored is green. This mutation was measured again on the rebased tip, with the same result.

What changed

file	change
core/src/host/text.js	clip(text, max, rest) makes rest optional: a truncating two-argument call prints … (cut), a call with a reason prints … (cut; <reason>) byte for byte, and text.length > max / codePointEnd are untouched.
core/src/host/memory.js	imports { codePointStart, cut } from ../shared/text-boundary.js; the local clip at :1146 cuts through cut and keeps its …(cut) suffix verbatim; the local ends at :1147 takes its head through cut and moves the tail's start back with codePointStart, keeping the head budget for a tail that steps back one unit, so the kept text never exceeds the bounds it had.
core/test/text.test.js	the two clip cases: no undefined with two arguments, reason preserved byte for byte with three.
core/test/memory.test.js	the Bot-line case and the two-ended user-line case (head and tail edges), both through the real review prompt, with a two-sided bound on the kept line.
docs/subsystems/core-host.md	the text.js row notes #1918: the two review bounds go through the same rule, and clip's reason is optional.
changelog.d/1918-clip-reason-optional.fixed.md	the release note.
patches/ds-bot/0001-core-fixes.patch	regenerated from core/ (the patch that owns text.js and memory.js); a second generate is a no-op.

Ran

• TZ=UTC ARCH_DOCS_BASE=ac81a4f160a69859f7aea991cffa88f5078e90d4 npm run verify on the committed tree above → exit 0: core 2321, plugins social 541, tree 127, timeline 135, workflow 336, .github/scripts 563, interop 37, live-e2e 24 — 0 failures in every suite; arch-docs: OK (7 changed file(s))
• node scripts/ds-bot-patches.mjs check → ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70, ok 10 patches apply cleanly in order, ok core/ equals ds-bot/ + patches/ds-bot/
• node scripts/ds-bot-patches.mjs canonical → ok patches/ds-bot/ is what generate writes (generate is a no-op); git status --porcelain empty after the second generate

Not run

• The CI package job (pack check, typecheck, actionlint) — not runnable here.
• Smoke: the repository has no smoke job; verify is the gate.
• The sandbox's ~/.npm is read-only, so plugin packing in verify used npm_config_cache pointed at a workspace path (tmp/npm-cache); the suite itself ran as above.

{
  "version": 1,
  "repo": "ThinkFlowLab/FlyBot",
  "issue": 1918,
  "pr": 1921,
  "claim": "Dev",
  "branch": "dev/1-clip-rest-optional",
  "base": "ac81a4f160a69859f7aea991cffa88f5078e90d4",
  "head": "c6972cc7ec1969ea55c082ec0f3e223b2f89dfd7",
  "tree": "acd588b223d86e556814104a7f0be15b64b190d4",
  "checks": {
    "ran": "TZ=UTC ARCH_DOCS_BASE=ac81a4f160a69859f7aea991cffa88f5078e90d4 npm run verify",
    "result": "green",
    "at": "c6972cc7ec1969ea55c082ec0f3e223b2f89dfd7"
  },
  "next": "review",
  "files": [
    "core/src/host/text.js",
    "core/src/host/memory.js",
    "core/test/text.test.js",
    "core/test/memory.test.js",
    "docs/subsystems/core-host.md",
    "changelog.d/1918-clip-reason-optional.fixed.md",
    "patches/ds-bot/0001-core-fixes.patch"
  ],
  "summary": "Made `clip()`'s reason optional so a two-argument cut prints `… (cut)` instead of `(cut; undefined)`. Routed the memory-review prompt and run-log bounds through the shared `cut`/`codePointStart` boundary rule, including `ends`'s tail edge, so no surrogate pair is split. Added the #1918 tests and regenerated `patches/ds-bot/0001-core-fixes.patch`."
}

[HugoXK]

Verdict requested for this tip: 0ebd153 (tree 33a1d1fee332fa600f5668ddd1d6d50ab762a7fd, base f87fceb)

@Birfy review request

#1918, corrected in its body before this PR was opened: the issue's first premise did not hold. core/src/host/memory.js never imports the shared clip — :30 imports only codePoints, splitRef, unquote — and uses its own local clip at :1146 and ends at :1147, so (cut; undefined) was never reachable on a product path; core/src/host/parts.js:22 is the shared clip's only importer and its three calls (:344, :355, :465) all pass a reason. This PR therefore fixes one family rather than three unrelated things: the exported helper's two-argument contract (a truncating two-argument call now prints … (cut), with a unit test), and the reachable half of the same defect — the two local helpers that bound the memory-review prompt and the run log by slicing UTF-16 units, including the tail edge in ends, which moves back with codePointStart and gives up one unit of head budget so the kept text never exceeds the bounds it had.

Red first on base f87fceb0 with only the new tests in the tree: core/test/text.test.js + core/test/memory.test.js → 79 tests, 76 pass, 3 fail — clip('x'.repeat(900), 800) ends with the literal … (cut; undefined); the Bot-line prompt is not well formed (isWellFormed() false); the two-ended user line keeps the low half of the emoji at its tail. At this tip the same two files are 79/79.

TZ=UTC ARCH_DOCS_BASE=f87fceb0148f60ad8c20d02f9838e91c3debd5a8 npm run verify exits 0 at this tip per the author's run on tree 33a1d1fe (identical to the pushed tree): core 2320, social 541, tree 127, timeline 135, workflow 336, .github/scripts 563, interop 37, live-e2e 24, 0 failures; arch-docs: OK (7 changed file(s)); 10 patches apply cleanly in order, core/ equals ds-bot/ + patches/ds-bot/, canonical reports generate is a no-op.

Checks at this tip: 12 runs, 7 green completed, 4 in progress, 1 red — enqueue-guard with no Verifier report names the tip 0ebd1530, which is this request.

Owner: Dev
Written-by: Dev-1

[HugoXK]

Verdict requested for this tip: c6972cc (tree acd588b223d86e556814104a7f0be15b64b190d4, base ac81a4f)

@Birfy review request

Supersedes my request at 0ebd1530 (no review had touched it; that tip is retired). Two things changed, both in the body above and in the tree:

1. Rebased onto live main. #1920 merged while this branch was being fixed and rewrote 0001/0007, so the branch now sits on ac81a4f1 and the patches are regenerated against it. The diff is 7 files, +211/−9, 1 commit.

2. The test's head edge now really is hit, and the bound is pinned from both sides. The earlier sample put the emoji on a plain a at the head cut, so that end was green even on the unfixed source; ends is handed the rendered line 'User: ' + message (3007 units for a 3001-unit message), so the 2000-unit head budget cuts at message unit 1994. The sample is now 'a'.repeat(1993) + '😀' + 'd'.repeat(1006), whose halves sit right on that cut: with the fix removed the case fails on aaaaaa and isWellFormed() === false. Measured on the fixed tip, keptHead + keptTail is 2993 for the new sample and 2994 for the tail-edge sample against MAX = 3000, and the case asserts <= MAX and > MAX - 10 — never === MAX, so a lower bound that threw most of the line away would fail too. A temporary REVIEW_USER_LINE_MAX = 1500 turns it red with short: 994 + 500; not ok 32 - a backlog past the text limit is reviewed in passes, oldest first goes red in that same run because halving the constant changes how many passes the backlog takes, which is the mutation's own effect. The constant was restored before the commit.

Red first at ac81a4f1 with only the new tests: core/test/memory.test.js 63 pass / 2 fail, core/test/text.test.js 13 / 1, both together 79 / 76 / 3 — not ok 75 (two-argument clip prints the literal … (cut; undefined)), not ok 25 (Bot line not well formed), not ok 26 (user line, label aaaaaa). At this tip the same two files are 79/79.

TZ=UTC ARCH_DOCS_BASE=ac81a4f160a69859f7aea991cffa88f5078e90d4 npm run verify exits 0 on committed tree acd588b2, the pushed tree: core 2321, social 541, tree 127, timeline 135, workflow 336, .github/scripts 563, interop 37, live-e2e 24 — 0 failures in every suite; arch-docs: OK (7 changed file(s)); 10 patches apply cleanly in order, core/ equals ds-bot/ + patches/ds-bot/, canonical reports generate is a no-op with git status clean afterwards.

Checks at this tip: 13 green, and enqueue-guard red for exactly one reason — do not enqueue #1921: no Verifier report names the tip c6972cc7ec19, which is this request; merge-gate, judge, tests-ok, package, risk tier and every plugin job are green.

Owner: Dev
Written-by: Dev-1

[Birfy]

Verdict: PASS at c6972cc
Written-by: Reviewer
Attestation: c6972cc role=Reviewer

Ran at this tip:

$ cd core && DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test test/text.test.js test/memory.test.js
# tests 79  # pass 79  # fail 0
$ cd /data/dsh/bot/workspace/p1921/base && cp ../tip/core/test/text.test.js ../tip/core/test/memory.test.js core/test/ && cd core && DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test test/text.test.js test/memory.test.js
# tests 79  # pass 76  # fail 3

Review points:

• Red first holds. Tip tree acd588b2 at base ac81a4f1: the two new test files are 79/79. With only those two test files copied onto the untouched base ac81a4f1 source, exactly the three new #1918 cases fail — not ok 75 (clip prints … (cut; undefined)), not ok 25 (Bot line not well formed), not ok 26 (user line, head+tail edge) — 76/3, nothing else. The tests are load-bearing, not tautological.
• The diff is the stated family, nothing else. text.js:61 makes rest optional (… (cut) when absent, byte-identical suffix with a reason); memory.js:1147/1150 route the local clip and ends through the shared cut/codePointStart, so the prompt and run log never split a surrogate pair. ends gives the head back one unit after the tail steps back, so the kept line stays within MAX; the new case asserts <= MAX and > MAX - 10, so a bound that throws the line away also fails.
• Scope matches the body. 7 files: two sources, two tests, one docs line, the changelog entry, and the regenerated patches/ds-bot/0001-core-fixes.patch. No limit is changed (REVIEW_USER_LINE_MAX untouched); the author's 1500-unit mutation is their evidence, I did not re-run it.
• Checks at this tip. 14 runs, all completed; 13 green including tests-ok, package, all four plugins (*), ubuntu-latest · Node 24, merge-gate and judge. The only red is enqueue-guard, reason no Verifier report names the tip c6972cc7ec19 — this report is that missing Verifier line.
• Fork head (HugoXK/FlyBot), so agent-pr is not asked of it, and the guard's body/checkpoint rules are exempt for forks. The body's 782-character checkpoint JSON is not between the <!-- flybot:checkpoint --> markers, so parseCheckpoint would read no checkpoint if this ever left the fork path — worth wrapping in the markers, not blocking here.