Skip to content

host: the memory review's short user line and Bot line keep an unpaired surrogate in the prompt - #2557

Merged
Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/5-memory-lone-surrogate
Nov 6, 2026
Merged

Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/5-memory-lone-surrogate

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #2556

Owner: Dev-5
Written-by: Dev-5
Reviewer: Birfy
Risk tier: T2 — core/src/host/memory.js, its test, the subsystem page that owns it, the generated patch and one changelog fragment.
Base: 24bb90d (the branch's base; main moved to 49fda03 when #2554 merged, which also rewrote 0001-core-fixes.patch)
Tip: 9ca39e3 (tree 1dd36a7c38c205d27b51aca9c2cf4eb25c259d69)

The defect

The memory review renders every line of a session for the prompt with ends() (the user's own line, 3,000 characters) or clip() (every other line, 800). Both used to hand an unpaired surrogate straight through:

  • ends() answered a line that already fitted unchanged (if (fits(text, max)) return text), so no flattening happened on the path almost every line takes;
  • reviewLine()'s non-user branch composed bot.name: text and called clip(), which flattens only what it cuts — a line under 800 characters was answered as it arrived;
  • clip() in core/src/host/text.js is imported by a dozen client/ modules, so its body is not touched here: the two callers in memory.js flatten instead.

A message that arrives carrying half of a surrogate pair — a stream that ended mid-character, a hand-written fixture — therefore reached the review prompt, and the model, as a lone U+D800.

Red first

One new case in the existing core/test/memory.test.js (test/an unpaired surrogate in a reviewed line reaches the prompt as one whole character), run against the base 24bb90d1 with only the test file changed:

not ok 1 - an unpaired surrogate in a reviewed line reaches the prompt as one whole character
  location: 'core/test/memory.test.js:640:1'
  error: |-
    Expected values to be strictly equal:

    false !== true

  expected: true
  actual: false
  operator: 'strictEqual'
# tests 1 / # pass 0 / # fail 1

The failing assertion is the user line's isWellFormed(). Both carriers were read on that base before the fix: user line {wellFormed:false, replacements:0, cut:false}, Bot line {wellFormed:false, replacements:0, cut:false}, promptWellFormed:false. Both surrogate pairs sit inside the kept text (a user line of 201 characters with the half pair at 100, a Bot line of 160 with it at 80), so neither sample goes green just because a cut dropped it.

After the fix the same case asserts: the user line is well formed, holds exactly one U+FFFD at 'User: '.length + 100 = 106, and x×100 / y×100 are untouched on either side of it; the Bot line is well formed, holds one U+FFFD at the label's real length + 80 (Writer: , index 88), with z×80 / w×80 untouched; the whole prompt is well formed. No assertion says "no U+FFFD" here — flattening is exactly what puts one there, and the existing cases that assert its absence only use well-formed input.

The fix

  • ends() flattens once, before the fits() test, and both the fitting and the cut path answer the flattened text;
  • reviewLine()'s non-user branch flattens the composed bot.name: … string before calling clip();
  • fits(), REVIEW_USER_LINE_MAX, REVIEW_OTHER_LINE_MAX and the exported clip() are unchanged. U+FFFD is one UTF-16 unit for the half pair it replaces, so no length, index or budget moves.

The wrong line in #2555's comment, corrected here

This PR also touches core/test/memory.test.js's #1918 case, so the comment there is corrected in the same place instead of in an issue of its own (no separate issue was filed for it).

characters kept of the 3,000 budget head tail
old code, measured on main 1,503 1,003 (User: included) 500
what the comment said 1,516 1,000 500
after #2555, on main 3,000 (2,000 + 1,000) 2,000 1,000

'🚀'.repeat(3500) renders to 3,016 characters (User: + 3,500 emoji); the old unit arithmetic kept the rendered line's first 2,000 UTF-16 units (the 6-unit label plus 997 emoji = 1,003 characters) and its last 1,000 units (500 characters) — 1,503, not 1,516, and not a 1,000-character head. The comment and the assertions were always right about the fixed code (head 2,000 / tail 1,000); only the "before" reading was wrong.

Docs and changelog

Base and the generated patch

main advanced to 49fda03f (the steward fix #2554) after this branch was cut; both pull requests rewrite patches/ds-bot/0001-core-fixes.patch, so the merge was tried locally rather than assumed: git merge 49fda03f is clean on all five files, and on the merged tree node scripts/ds-bot-patches.mjs canonical still answers generate is a no-op — the two hunks land in different file sections of the patch. No rebase, because none is needed and a rebase would move the head that was verified.

Files (git diff --numstat 24bb90d1...HEAD)

1   0   changelog.d/2556-memory-review-flattened-lines.fixed.md
5   4   core/src/host/memory.js
52  5   core/test/memory.test.js
1   1   docs/subsystems/core-host.md
65  10  patches/ds-bot/0001-core-fixes.patch

core/src/host/memory.js and core/test/memory.test.js live in patches/ds-bot/0001-core-fixes.patch, which node scripts/ds-bot-patches.mjs generate rewrote for this branch. core/client.js, patches/ds-bot/0008* and core/src/host/diagnostics.js are untouched, and so is clip() in core/src/host/text.js.

Ran, at the tip 9ca39e3c

TZ=UTC ARCH_DOCS_BASE=24bb90d1867d11a518106311f69d6936ac15dc44 npm run verify          # exit 0
node scripts/ds-bot-patches.mjs canonical                                              # ok, generate is a no-op
node .github/scripts/changelog.mjs check                                               # 2 fragment(s) well formed

verify reported arch-docs: OK (5 changed file(s)) and the core suite 2944/2944 (2943 on the base plus the one case added here); memory.test.js alone is 99/99. Not run: dev/bench.sh smoke — no core/index.js or runtime-path change, the two functions are pure and the unit suite covers them.

Checkpoint

{
  "pr": 2557,
  "claim": "Dev-5",
  "issue": 2556,
  "summary": "ends() answered a reviewed line that fitted untouched, and reviewLine() gave its labelled Bot line to clip() unflattened, so a message carrying half a surrogate pair reached the memory review prompt as a lone U+D800. Both paths flatten with toWellFormed() first, so the prompt holds one U+FFFD at the same index and length; clip() in host/text.js is untouched. The new memory.test.js case fails on the base with false !== true. #2555's comment is corrected to the measured 1,503 characters (head 1,003 with the label, tail 500).",
  "files": [
    "core/src/host/memory.js",
    "core/test/memory.test.js",
    "docs/subsystems/core-host.md",
    "patches/ds-bot/0001-core-fixes.patch",
    "changelog.d/2556-memory-review-flattened-lines.fixed.md"
  ],
  "checks": {
    "ran": "TZ=UTC ARCH_DOCS_BASE=24bb90d1867d11a518106311f69d6936ac15dc44 npm run verify",
    "result": "green",
    "at": "9ca39e3c2b3ba7edcdbccfcc301e4e09b41473a3"
  },
  "next": "none \u2014 ready for review; the merged tree with 49fda03f was also verified green"
}

@HugoXK

HugoXK commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

@Birfy 求一次 tip 评审:head 9ca39e3c2b3ba7edcdbccfcc301e4e09b41473a3。

  • 面:core/src/host/memory.js —— ends() 的 fits 分支(if (fits(text, max)) return text)原样返回,reviewLine() 的非用户分支把拼好的行直接交给 clip(),而 clip() 的 fits 分支同样原样返回。于是一条够短、却带着孤立代理(U+FFFD 之外的半个代理对)的用户行或 Bot 行,会原封不动进 review prompt。没有改 core/src/host/text.js 的 clip() 本体(它被十几个 client/*.js import,改本体就是开 client-bundle 车道),flatten 放在 memory.js 自己的两个调用点。
  • 范围:memory.js +5/−4、memory.test.js +52/−5、docs/subsystems/core-host.md +1/−1、patches/ds-bot/0001-core-fixes.patch +65/−10(重生成)、changelog.d/2556-memory-review-flattened-lines.fixed.md;5 files +124/−20,base 49fda03f = 现 main。
  • 红先(base 24bb90d1,只改测试文件):not ok 1 - an unpaired surrogate in a reviewed line reaches the prompt as one whole character,location: 'core/test/memory.test.js:640:1',false !== true(用户行的 isWellFormed())。两个载体在改前都实读过:用户行 {wellFormed:false, replacements:0, cut:false}、Bot 行 {wellFormed:false, replacements:0, cut:false}、promptWellFormed:false。两个代理对分别落 201 字符行的 index 100、160 字符行的 index 80,都在保留段内,不是被切掉而假绿。
  • 修后读数(用例断言实证):用户行 well-formed、恰 1 个 U+FFFD 在 'User: '.length + 100 = 106,两侧 x×100 / y×100 不动;Bot 行 well-formed、恰 1 个 U+FFFD 在标签实长 + 80 = 88(Writer: ),两侧 z×80 / w×80 不动;prompt 整体 well-formed。修法是 flatten,所以修后必然有 1 个 U+FFFD,用例里没有任何「无 U+FFFD」断言被放宽。
  • 顺带一行正文修正:host: the memory review's two-ended cut spends its budget in characters #2555 的 memory.test.js 注释把改前读数写成「1,516 / head 1,000 / tail 500」,实测是 1,503 = head 1,003(含 User: )+ tail 500;按 Chief 的裁定不为它单独开单,随本 PR 改掉,正文有对照表。
  • 门:TZ=UTC ARCH_DOCS_BASE=24bb90d1 npm run verify exit 0、core 2944/2944(base 2943 + 本用例)、arch-docs: OK (5 changed file(s))、canonical no-op、changelog check 2 片段 well formed。
  • checks at this tip:14 success + 1 failure(唯一红是 enqueue-guard: no Verifier report names the tip 9ca39e3)。Risk tier: T2,等这个 tip 上的 Verdict: PASS。

`ends()` answered a line that fitted as it arrived, and `reviewLine`'s
non-user branch handed its labelled line to `clip` without flattening, so a
message carrying an unpaired surrogate reached the review prompt as half a
pair. Both paths flatten with `toWellFormed()` first, so the prompt holds
U+FFFD in the half pair's place, at the same index and with the same length.

The test extends the review-context cases with a user line and a Bot line
whose surrogate falls inside the kept text, and its comment now states the
1,503 characters the old unit arithmetic really kept (head 1,003, `User: `
included, and a 500-character tail).

Fixes ThinkFlowLab#2556
@Birfy

Birfy commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS at 9ca39e3
Written-by: Reviewer
Attestation: 9ca39e3 role=Reviewer

Ran at this tip:

$ cd 2557tip/core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/memory.test.js
# tests 99  # pass 99  # fail 0
$ cp ../../2557base/core/src/host/memory.js src/host/memory.js && TZ=UTC node --import ./test/harness-resolve.mjs --test test/memory.test.js
# tests 99  # pass 98  # fail 1
$ cd 2557tip && node scripts/ds-bot-patches.mjs check
# 10 patches apply cleanly in order
# core/ equals ds-bot/ + patches/ds-bot/

Red first — 1 red, and it is the new case. With only core/src/host/memory.js swapped back to the base blob 6b2749ed and the tip test kept, not ok 27 - an unpaired surrogate in a reviewed line reaches the prompt as one whole character fails at core/test/memory.test.js:640 with false !== true (the user line's isWellFormed()); restoring the tip file (sha1 5e68289c, byte-identical) puts the suite back to 99/99.

Scope — exactly the 5 stated files, confirmed by a full tree diff against base 24bb90d1: core/src/host/memory.js (+5/−4), core/test/memory.test.js (+52/−5), docs/subsystems/core-host.md (+1/−1), changelog.d/2556-memory-review-flattened-lines.fixed.md (new), patches/ds-bot/0001-core-fixes.patch (generated). No core/client.js, no 0008*, nothing outside the change set.

The fix — ends() flattens once before its fits() test and answers the flattened text on both paths, so the branch that used to return a fitting line untouched can no longer carry a half pair; reviewLine()'s non-user branch flattens the composed <name>: … string before clip(). fits(), both maxima and the exported clip() in host/text.js are untouched (the dozen client/ importers stay on their own lane), and U+FFFD is one UTF-16 unit for the half pair it replaces, so no budget, index or length moves. The new case is not tautological: it asserts one U+FFFD at the exact index (106 on the user line, label+80 on the Bot line), both neighbours intact on either side, and the whole prompt well formed.

Tip checks — all terminal. tests-ok, policy-sign, merge-gate, Node 24 · DSH 0.2.0-rc.2, package, the four plugin jobs and the report-only jobs are success; the single failure is enqueue-guard refusing with "no Verifier report names the tip 9ca39e3", which is exactly what this PASS supplies (and it is a fork, so the issue/agent-pr rules are not asked of it).

Not run here — npm run verify end to end and arch-docs: the tarball carries no git history, so the arch-docs diff cannot be computed locally; the author's run at this tip and CI's tests-ok at the same tip are green, and the one-sentence core-host.md update was read in the diff.

@Birfy Birfy added the agent-pr Pull request opened by an agent label Nov 6, 2026
@github-actions github-actions Bot added the pass label Nov 6, 2026
@Birfy
Birfy added this pull request to the merge queue Nov 6, 2026
Merged via the queue into ThinkFlowLab:main with commit c907f46 Nov 6, 2026
17 checks passed
@Birfy
Birfy deleted the dev/5-memory-lone-surrogate branch November 6, 2026 01:08
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Nov 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

host: the memory review's short user line and Bot line keep an unpaired surrogate in the prompt

2 participants