fix(core): scan fully for a recall term with an unpaired surrogate

[Totoro-qaq]

Summary

Recall's candidate source searches the serialized transcript. JSON.stringify writes a lone surrogate as a \ud800 escape, and SQLite binds a lone-surrogate term as U+FFFD. So a Recall term containing an unpaired surrogate got an empty candidate list. collectHits treats that list as authoritative, and a passage the verifier would match was silently dropped.

hasJsonEscapedCharacter now also returns true for an unpaired surrogate. It uses /\p{Cs}/u, as long-term-memory.ts does, so such a term takes the existing full-scan path. A well-formed pair such as an emoji is one code point under the u flag and keeps the candidate path.

Storage is unchanged: runRecall is the only consumer of the candidate queries, and their doc comments already leave escaped forms to the caller.

Fixes #5411

Verification

• A new test sits next to "a term the stored form escapes bypasses the candidate source". \uD800, c\uD800d and either half of a stored emoji (\uD83D, \uDE80) scan fully and return the passage; 🚀 still uses the candidate source. The test failed before the fix.
• Three mutants make it fail: disabling the guard; replacing the pattern with /[\uD800-\uDFFF]/ without the u flag, which then also matches both halves of the emoji pair; and a high-surrogate-only range /[\uD800-\uDBFF]/u, which misses the low half \uDE80.
• @maka/core test:dist 847/847, storage recall-candidate-sessions 8/8, runtime recall-ledger-corpus 2/2.
• A node:sqlite probe of instr(lower(json), ?) confirmed the miss for \uD800, c\uD800d and \uD83D, and the match for 🚀.
• Core typecheck, biome lint and format, git diff --check and the Windows test inventory all pass.

AI use

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code helped investigate, implement and test this change.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

Generated-by: Claude Code

[jackwener]

Review result

I found no correctness or scope issues at exact head 9be3999078ba41bb89fcfd3279ec17b0908b7e3f.

The defect is real on the production storage shape. A direct node:sqlite probe using the candidate query returned no match for a lone surrogate, a lone surrogate embedded in a term, or either half of a stored emoji; the complete emoji still matched. The verifier reads parsed transcript text, so accepting that empty candidate set can silently omit a passage that the verifier would otherwise return.

The repair is minimal and sits at the correct boundary: terms containing a Unicode surrogate code point now use the existing bounded full-scan path, while the Unicode-aware regular expression treats a well-formed surrogate pair as one complete code point and leaves ordinary emoji on the candidate path. I also checked low-surrogate terms and the low half of an emoji through runRecall; both bypassed the candidate source and returned the expected passage, while the complete emoji remained narrowed.

The committed regression passed and failed when I removed the new guard. A clean exact-head build then passed the full Core suite (847/847), the storage candidate suites (8/8), the runtime ledger-corpus suites (2/2), Biome, ASF headers, the Windows test inventory, and git diff --check. Current main (672d82731a638e45e3ec87022eabdcf70a5a90be) is the PR's direct parent, and the synthetic merge tree is exactly the PR tree (77a4f736d7b6d616dd6e8e8887d8ea2afca7a83a). The exact-head hosted test and label checks are both complete and successful.

---

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

[Astro-Han]

Reviewed at head 9be39990. Comment only — not an approval.

What I verified

The fix is correct and sits at the right boundary.

• packages/core/src/recall.ts:148 adds if (UNPAIRED_SURROGATE_PATTERN.test(term)) return true; to hasJsonEscapedCharacter, which feeds forceFullScan at recall.ts:452, which makes collectHits (recall.ts:786) skip listCandidateSessions entirely. So yes — the whole string is scanned, including everything after a lone surrogate; the narrowed path is not entered at all.

• /\p{Cs}/u behaves as the comment claims: it matches a lone high surrogate (U+D800) and a lone low one (U+DC00), and does not match a well-formed pair, since under the u flag \uD83D\uDE80 is one code point. Emoji terms stay on the candidate path.

• I reproduced the defect against the real runRecall using the candidateDeps double from recall.test.ts:129, pre-fix vs. with the guard added:

  term	pre-fix	post-fix
  \uD800	scannedFully=false, anchors=[], gaps="No transcript match for: …"	scannedFully=true, anchors=[m-lone]
  c\uD800d	scannedFully=false, anchors=[]	scannedFully=true, anchors=[m-lone]
  🚀	scannedFully=false, anchors=[m-emoji]	unchanged
  ship (control)	scannedFully=false, anchors=[m-emoji]	unchanged

  So the silent drop in #5411 is real and the fix resolves it without narrowing anything.

• The test's third case, ['\uD83D', 'm-emoji'] (recall.test.ts:393), is the one that isolates the second failure mode rather than JSON escaping: the stored emoji is literal UTF-8, so there is no \uXXXX escape involved — the term is simply unrepresentable when bound. A direct node:sqlite probe of instr(lower(j), ?) confirms it: \uD800, c\uD800d and \uD83D all bind as UTF-8 EF BF BD (U+FFFD) and miss, while 🚀 matches. Good coverage of both mechanisms; the comment at :392 is accurate that the in-process double is more permissive than SQLite here, which is why that row's scannedFully assertion is the one that fails pre-fix.

• Guarding the raw terms at recall.ts:452 (rather than the folded terms actually sent to the store) is sound: foldForMatch (thread-search.ts:564, NFC + toLowerCase) preserves a lone surrogate, so folding can neither introduce nor remove one. Checked directly.

• Placement matches repo convention: /\p{Cs}/u is the same idiom as LONG_TERM_MEMORY_SURROGATE_CHARS (long-term-memory.ts:73), and routing from core rather than changing the store follows the contract already documented at sqlite-runtime-store.ts:1529 ("escaped forms excepted, which the caller routes around"). I also confirmed recall is the only consumer of the candidate queries (execution-composition.ts:701), so "storage is unchanged" holds.

• biome check is clean on both changed files.

This addresses the root cause as filed in #5411 (option 1 of the two the issue suggested), not just the symptom: the predicate that decides "can a serialized-record scan vouch for this term?" is the one place the invariant is enforced, and it was missing this class.

Nits (non-blocking)

• recall.ts:141-145 — hasJsonEscapedCharacter's own doc comment still justifies only the JSON-escaping half, but the \uD83D case fails for an unrelated reason (unrepresentable when bound). The new constant's comment covers both; one extra sentence on the function would help whoever decides next time whether a case belongs here.
• recall.test.ts:402 — assert.deepEqual(anchorIds(result.passages), [anchor]) has no message, so a failure inside the loop doesn't say which term caused it. The neighbouring assertion at :397-401 does include ${JSON.stringify(term)}; adding the same here would keep loop failures self-describing.
• recall.test.ts:389-394 — only high surrogates are exercised. A lone low surrogate is a different code point; I ran \uDC00 end to end and it also scans fully, but nothing pins it. It's also the case a future [\uDC00-\uDFFF]-style rewrite of the pattern would most plausibly get wrong, so one more tuple row would be cheap insurance.
• packages/storage/src/recall-fold.ts:40 — isRecallFoldUnstable still reports a value containing a lone surrogate as fold-stable, the other half of #5411's finding. It's now unreachable from recall because core never forwards such a term, so leaving it is correct; I'd just note in that module's doc comment that "stable" is only sound because callers route surrogate terms away, so the next person touching this scan doesn't re-open the hole.

[Astro-Han]

Reviewed at head 9be39990. Comment only — not an approval.

What I verified

The fix is correct and sits at the right boundary.

• packages/core/src/recall.ts:148 adds if (UNPAIRED_SURROGATE_PATTERN.test(term)) return true; to hasJsonEscapedCharacter, which feeds forceFullScan at recall.ts:452, which makes collectHits (recall.ts:786) skip listCandidateSessions entirely. So yes — the whole string is scanned, including everything after a lone surrogate; the narrowed path is not entered at all.

• /\p{Cs}/u behaves as the comment claims: it matches a lone high surrogate (U+D800) and a lone low one (U+DC00), and does not match a well-formed pair, since under the u flag \uD83D\uDE80 is one code point. Emoji terms stay on the candidate path.

• I reproduced the defect against the real runRecall using the candidateDeps double from recall.test.ts:129, pre-fix vs. with the guard added:

  term	pre-fix	post-fix
  \uD800	scannedFully=false, anchors=[], gaps="No transcript match for: …"	scannedFully=true, anchors=[m-lone]
  c\uD800d	scannedFully=false, anchors=[]	scannedFully=true, anchors=[m-lone]
  🚀	scannedFully=false, anchors=[m-emoji]	unchanged
  ship (control)	scannedFully=false, anchors=[m-emoji]	unchanged

  So the silent drop in #5411 is real and the fix resolves it without narrowing anything.

• The test's third case, ['\uD83D', 'm-emoji'] (recall.test.ts:393), is the one that isolates the second failure mode rather than JSON escaping: the stored emoji is literal UTF-8, so there is no \uXXXX escape involved — the term is simply unrepresentable when bound. A direct node:sqlite probe of instr(lower(j), ?) confirms it: \uD800, c\uD800d and \uD83D all bind as UTF-8 EF BF BD (U+FFFD) and miss, while 🚀 matches. Good coverage of both mechanisms; the comment at :392 is accurate that the in-process double is more permissive than SQLite here, which is why that row's scannedFully assertion is the one that fails pre-fix.

• Guarding the raw terms at recall.ts:452 (rather than the folded terms actually sent to the store) is sound: foldForMatch (thread-search.ts:564, NFC + toLowerCase) preserves a lone surrogate, so folding can neither introduce nor remove one. Checked directly.

• Placement matches repo convention: /\p{Cs}/u is the same idiom as LONG_TERM_MEMORY_SURROGATE_CHARS (long-term-memory.ts:73), and routing from core rather than changing the store follows the contract already documented at sqlite-runtime-store.ts:1529 ("escaped forms excepted, which the caller routes around"). I also confirmed recall is the only consumer of the candidate queries (execution-composition.ts:701), so "storage is unchanged" holds.

• biome check is clean on both changed files.

This addresses the root cause as filed in #5411 (option 1 of the two the issue suggested), not just the symptom: the predicate that decides "can a serialized-record scan vouch for this term?" is the one place the invariant is enforced, and it was missing this class.

Nits (non-blocking)

• recall.ts:141-145 — hasJsonEscapedCharacter's own doc comment still justifies only the JSON-escaping half, but the \uD83D case fails for an unrelated reason (unrepresentable when bound). The new constant's comment covers both; one extra sentence on the function would help whoever decides next time whether a case belongs here.
• recall.test.ts:402 — assert.deepEqual(anchorIds(result.passages), [anchor]) has no message, so a failure inside the loop doesn't say which term caused it. The neighbouring assertion at :397-401 does include ${JSON.stringify(term)}; adding the same here would keep loop failures self-describing.
• recall.test.ts:389-394 — only high surrogates are exercised. A lone low surrogate is a different code point; I ran \uDC00 end to end and it also scans fully, but nothing pins it. It's also the case a future [\uDC00-\uDFFF]-style rewrite of the pattern would most plausibly get wrong, so one more tuple row would be cheap insurance.
• packages/storage/src/recall-fold.ts:40 — isRecallFoldUnstable still reports a value containing a lone surrogate as fold-stable, the other half of #5411's finding. It's now unreachable from recall because core never forwards such a term, so leaving it is correct; I'd just note in that module's doc comment that "stable" is only sound because callers route surrogate terms away, so the next person touching this scan doesn't re-open the hole.

[Astro-Han]

Heads-up: the review above was submitted twice by mistake (both are identical — an accidental double-post by the review bot). Apologies for the noise; please treat them as a single review.

[Totoro-qaq]

Thanks for the careful review. I addressed the four nits in d75d052:

• hasJsonEscapedCharacter's doc comment now gives the second reason too. A lone surrogate cannot be bound as UTF-8, so a serialized-record scan misses it even when it is stored literally as half of a pair.
• The loop's deepEqual now names the term in its message.
• I added the low half of the stored emoji (\uDE80) as a row. A high-surrogate-only pattern such as /[\uD800-\uDBFF]/u now fails on that row. I used \uDE80 rather than \uDC00 so the row still has a stored passage to return.
• recall-fold.ts's module comment now says the stable-record predicate holds only for terms the serialized record spells as typed. Terms with escaped characters or lone surrogates never reach that scan, because core scans them fully.

Claude Code-assisted.

[jackwener]

Incremental re-review

The code and tests remain correct at exact head d75d0525f7fdc7b24be888db1b0b1c7ea507f872. I found no P0–P2 issues. One minor verification-text correction remains:

• P3 — correct the u-flag mutation description in the PR body. The Verification section says that dropping the u flag makes /\p{Cs}/u match both halves of a surrogate pair. In JavaScript, /\p{Cs}/ without Unicode mode does not perform a Unicode property escape: it matches the literal text p{Cs} and matches neither lone surrogates nor an emoji. The regression still fails without u, but because the guard stops recognizing lone surrogates—not because it recognizes both halves. The new \uDE80 row instead correctly kills a high-surrogate-only guard such as /[\uD800-\uDBFF]/u. Please update that bullet so the review record describes the mutation accurately.

The new commit otherwise closes the prior non-blocking gaps without changing production behavior: it pins the low half of a stored emoji, gives each loop failure its term, and documents the Core/Storage invariant on both sides. Replacing the guard with a high-surrogate-only range made the focused regression fail specifically on \uDE80; the restored implementation passes.

Exact-head Core tests pass 847/847, and the hosted test is complete and successful. Current main is 063abf42f4f76eb8f585abaf3c299b24f2fb18f3; its synthetic merge with this head is clean (1aa05e43fc9afcfeaa48741de5d20bd3c46ece13). I installed the current-main dependency set and tested the preceding merge tree: Core 852/852, storage candidate tests 8/8, runtime ledger-corpus tests 2/2, and Biome all passed. Main then advanced by one Desktop-only commit; the Core, Storage, and Runtime subtree object IDs are identical in the latest merge tree, so those results apply unchanged. The exact-head static gates also pass.

---

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

[Totoro-qaq]

Thanks, you're right: without u, \p{Cs} is not a property escape and only matches the literal p{Cs}. The mutant I actually ran was /[\uD800-\uDFFF]/ without u, which does match both halves of the emoji pair, and I misdescribed it in the PR body. The Verification bullet is corrected. It now also lists the high-surrogate-only mutant that the \uDE80 row catches.