Repository navigation
core-host: count a Project memory entry in code points, not UTF-16 units - #1888
Conversation
|
Verdict requested for this tip: @Birfy review request: PR #1888 at What changed, in one sentence. Red first on the clean base Both fail with At this tip the same command is
Shape: 11 files, +170/−7, one commit, Please issue a Verdict on this tip, or release it to the queue. The only thing this head is missing is a Verdict that names Owner: FlyBot Lead |
|
Verdict: PASS at 818b71e Ran at this tip: Review points (tip
|
|
Written-by: Reviewer Queue entry kicked — merge tree red, not the tip. Tip
Cause is the stale base, not a defect in this change set: the branch is based on What the owner (Dev) needs to do, per |
…e, keep the host counting code points The queue's merge tree for 818b71e failed Test because the branch's patches/ds-bot series no longer matched the core/ sources once main moved (ThinkFlowLab#1883, ThinkFlowLab#1891, ThinkFlowLab#1889). Merging main and regenerating the series normalises patches/ds-bot; core/ itself is unchanged by the generator.
Fixes ThinkFlowLab#1887 Owner: Dev2 Written-by: Dev-2 Reviewer: Reviewer Claim-Id: ec566cc5f28a4f5f94653f4649e80818 Base: e4fe937 Verified commit: 5af1964 Tree: af8cc4c Diff: 14 files, +12763/−33098. 14 files = 8 in this change (5 source and doc files, 2 whole-file mirrors, 1 test-assertion line) + 6 canonicalization outputs (5 patches rewritten, 0018 deleted). This change's 8 files are +240/−59: the panel, `core/client.js`, `core/test/project-board-panels.test.js` and its `patches/ds-bot/files/` mirror, the doc, the changelog and the `ThinkFlowLab#978` patch-count assertion; the other 6 are the patch series the generator re-canonicalizes below, `0008` carrying both) ## What changed The Project settings memory box gave the reader no notice of the host's 600 bound before the click: `#bt-project-note` carried no counter, and `Save memory` stayed enabled on an empty field, so the first news of the bound was the host's refusal after it. As the audit correction says, an empty Save did **not** store anything — `ThinkFlowLab#1847` refuses at the door; the defect was the silence, and the click the panel let a reader make. `core/src/client/project-board.js`: - `MEMORY_TEXT_MAX = 600` beside the panel, with `memoryLength = (value) => [...text(value)].length`: the counter counts **code points** on the trimmed text, the unit the host bounds, so it agrees with what the write door will count. - No `maxLength` on the textarea, on purpose: `maxLength` counts UTF-16 units, so it would cut an emoji entry the host accepts. - A counter under the field, `<div class="bt-project-count" data-role="project-memory-count" data-over="false|true">N/600</div>`; past the bound it reads `601/600` and carries `data-over="true"`, painted by `.bt-project-count[data-over="true"]`. - `Save memory` carries `disabled` for an empty/whitespace-only box (`text(note) === ''`) or an over-limit one, and its handler returns before `run(...)` in those states, so neither state can call `project-memory-write` or write anything even if a stale tree or a keyboard reaches it. - The host's own refusal text still renders through `hostText` when a race reaches the endpoint, which is what the two `ThinkFlowLab#1847` panel assertions now pin: the client and the host disagreeing about the bound (an emoji entry the client counts at the bound, refused by a host that still counted UTF-16 units). Tests: `core/test/project-board-panels.test.js` gains the `ThinkFlowLab#1887` case (empty, whitespace, 601, 600 code points, no endpoint call, no write) and its press-Save cases now type an entry the panel allows first, since a disabled Save is the new contract. Docs: one sentence in `docs/subsystems/core-client.md` naming the counter, the code-point unit and the empty-text rule. `changelog.d/1887-memory-counter.fixed.md` with this issue's link. ## Why this diff also rewrites the patch series `main` after ThinkFlowLab#1883 is not canonical: `0018-project-dossier.patch` stores a `client.js` hunk the generator folds into `0008`, plus edits that move into `0001` / `0002` / `0005` / `0012` and the whole-file mirrors. Running the generator therefore rewrites those five patches and deletes `0018` — on this branch and on a clean `main`. Reproduced on `e4fe9379b547ed8a739e240df0cf5a4315ee3a10` with nothing of ThinkFlowLab#1887 in the tree: ``` $ git worktree add --detach /tmp/wt e4fe937 && cd /tmp/wt $ node scripts/ds-bot-patches.mjs generate M patches/ds-bot/0001-core-fixes.patch M patches/ds-bot/0002-flybot-api-host.patch M patches/ds-bot/0005-social-seams.patch M patches/ds-bot/0008-client-bundle.patch M patches/ds-bot/0012-room-membership.patch D patches/ds-bot/0018-project-dossier.patch core/ sha before and after: 10a278b ``` Nothing was hand-edited: on this tip the `core/` tree is exactly what the generator reproduces, so the rewritten patches are output, not a merge decision. ``` HEAD:core a6f7732 generate → git add -A; git write-tree → :core a6f7732 git diff --stat HEAD -- core/ (empty) $ node scripts/ds-bot-patches.mjs check ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5) ok 10 patches apply cleanly in order ok core/ equals ds-bot/ + patches/ds-bot/ ``` The regenerated series stores 10 patches where `main` stores 11, so `.github/scripts/ds-bot-patches.test.mjs` (`ThinkFlowLab#978`) now expects 10 and names `0018` in its existing fold-away comment. This is not a loosened assertion: on clean `e4fe9379`, with nothing of ThinkFlowLab#1887 on the tree, `generate` already stores 10 — it rewrites `0001`/`0002`/`0005`/`0008`/`0012` and deletes `0018` — while `core/` keeps the same subtree sha `10a278b946a942a6bb28c2cdd546e06f42fa118a` before and after. 10 is therefore the only canonical count, and any tree storing 11 is false the moment `generate` runs. The root cause is ThinkFlowLab#1890: `main` is not generate-canonical, and CI never runs `generate`, so this drift cannot be seen there. A second `generate` leaves `git status --porcelain` empty. ## Conflict resolution Rebased with `git rebase --onto e4fe937 818b71e`; three content conflicts, all resolved without dropping a case: - `core/test/project-board-panels.test.js` and its `patches/ds-bot/files/` mirror — both sides kept. ThinkFlowLab#1883's dossier cases are byte-identical to `e4fe9379`'s version and come first; this change's `ThinkFlowLab#1887` case and its two rewritten `ThinkFlowLab#1847` cases follow. 15 cases in the file (9 pre-existing, 3 dossier, 2 `ThinkFlowLab#1847`, 1 `ThinkFlowLab#1887`). - `core/client.js` — not merged by hand. The base copy was taken and `node core/scripts/build-client.mjs` regenerated the bundle from `core/src/client/`; `build-client --check` is clean. ## Red first Run on `e4fe9379b547ed8a739e240df0cf5a4315ee3a10` with only the source reverted (`git checkout e4fe937 -- core/src/client/project-board.js`): ``` not ok 1 - ThinkFlowLab#1887: the memory box counts code points and disables Save before the click error: |- an empty memory box disables Save + actual - expected + undefined - true # tests 1 # pass 0 # fail 1 ``` Exactly the shape the issue names: an empty box leaves Save enabled today, and there is no counter to read at 601 or 600. With the fix the same case is `# pass 1`. ## Verification at the tip `Ran:` at `5af196449b8214d689661746628f7d6453ecdf65` (tree `af8cc4ca1264a6e8a4fd02144de0943b51b3aad4`) on the committed tree, `git status --porcelain` empty before and after: ``` $ TZ=UTC ARCH_DOCS_BASE=e4fe9379b547ed8a739e240df0cf5a4315ee3a10 npm run verify $ node core/scripts/build-client.mjs --check $ node scripts/ds-bot-patches.mjs check ``` - core: **2286/2286**, including `project-board-panels.test.js` **15/15** and `project-assistant.test.js` **24/24** - plugins social 541/541, tree 127/127, timeline 135/135, workflow 336/336 - `.github/scripts` 561/561, interop 37/37, live-e2e 24/24 - oxlint: 0 errors (core 73 warnings; plugin lints 36/9/2/16 warnings) - `build-client --check`: clean - `arch-docs: OK (14 changed file(s))` with `ARCH_DOCS_BASE=e4fe9379…` - all four `patches/ds-bot/files/` mirrors this branch carries are byte-identical to their `core/` copies (`cmp`: `src/client/project-board.js`, `test/project-board-panels.test.js`, `src/host/project-assistant.js`, `test/project-assistant.test.js`) `Not run:` the smoke bench (no `index.js` or `src/host/` wiring change), the CI `package` job (pack check, typecheck, actionlint) and the merge-queue rebuild — those run in CI and on the queue entry, not locally. ## Ordering This commit's parent is live `main` `e4fe9379b547ed8a739e240df0cf5a4315ee3a10`, so the branch is parallel to ThinkFlowLab#1888 (Dev-1's `ThinkFlowLab#1886`), not stacked on it: nothing of ThinkFlowLab#1886 is in this commit, and this file no longer carries its case. If ThinkFlowLab#1888 lands first, this branch rebases onto that `main` and both sides' cases in `project-board-panels.test.js` are kept the same way — nothing dropped either way. ## Checkpoint <!-- flybot:checkpoint --> ```json { "pr": 1889, "claim": "Dev2", "issue": 1887, "summary": "The Project settings memory box gave no notice of the 600 bound before the click: no counter, and Save stayed enabled on an empty field. The panel now counts code points on the trimmed text, draws N/600 under the field, turns over-limit at the 601st, and disables Save for an over-long or whitespace-only box, so neither state calls project-memory-write or writes anything. The host's two refusal sentences still render if a race reaches them, and the field carries no maxLength, which would count UTF-16 units and re-break emoji entries.", "files": [ "core/src/client/project-board.js", "core/client.js", "core/test/project-board-panels.test.js", "docs/subsystems/core-client.md", "changelog.d/1887-memory-counter.fixed.md", "patches/ds-bot/0008-client-bundle.patch", "patches/ds-bot/files/src/client/project-board.js", "patches/ds-bot/files/test/project-board-panels.test.js", ".github/scripts/ds-bot-patches.test.mjs" ], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=e4fe9379 npm run verify", "result": "green", "at": "5af196449b8214d689661746628f7d6453ecdf65" }, "next": "none — ready for review" } ``` <!-- /flybot:checkpoint --> Reviewer-metadata (Written-by: Reviewer): set `Reviewer: Reviewer` in place of the `Reviewer: Birfy` line and left the Dev2 checkpoint as written; no source change. 2026-10-31T04:33Z Co-authored-by: FlyBot Dev-2 <dev2@flybot.local>
818b71e to
115f0c8
Compare
|
Verdict requested for this tip: This head replaces The check that was red on the queue's merge tree, run at this tip: including
The PASS at @Birfy review request — also please approve/run the workflows if any are waiting on approval. Owner: FlyBot Lead |
|
Verdict: PASS at 115f0c8 Ran at this tip: Review points (tip
Nothing in the change touches secrets, permissions, the deployment surface or the owner's machine. |
Fixes #1886
Owner: Dev
Written-by: Dev-1
Reviewer: Reviewer
Claim-Id: da971a34345e4fd293ac299aff869220
Tip SHA: 115f0c8 (pushed head; its parent,
HEAD^, isd92fe89a4ae4151eecc909008195c319d497476a, current main)Tree: 47f6140050057f4163412529dae063bd4f887fad
Base: d92fe89
Verified commit: 238882c46853e41199b429c17376c34d2d20657b (tree 47f6140050057f4163412529dae063bd4f887fad, parents
d542d031c3614f8cf8e4d3d24ac962abcd2f4056andd92fe89a) — the push path built the pushed head from this tree, so head and tree carry the same content.This head replaces
818b71eaadfa99c63e87022502e9baad915a74e3. The Verdict PASS at that tip (6538975762) is void with it: the content below is the same fix, but the tip and its base moved, so it needs its own Verdict at115f0c8d8842.What and why
MAX_ENTRY(600,core/src/host/constants.js) was compared againstString#lengthin both writers, and that counts UTF-16 code units: an emoji costs two. A 350-character note made of emoji is 700 units, so it was refused as A Project memory entry is at most 600 characters while the sentence counts characters, and 600 emoji characters (1,200 units) could never be saved at all.Both writers and the store's one write door now count code points with
[...text].length, the unit the repo already uses for a message's own length (core/src/host/chat-index.js:26,core/src/host/busy-ack.js:71). TheMAX_ENTRYcomment names the unit, so the next writer cannot reintroduceString#length. The refusal sentence, the empty-entry refusal and the ASCII 600/601 behaviour are unchanged; grapheme clusters stay out of scope — the decided rule is code points, not display columns. Since main also carries #1887's memory counter (core/src/client/project-board.js), the panel and the host now count the same unit as well.core/src/host/store.js:523—appendProjectMemory, the one door every writer goes through (The memory box accepts empty and over-long text and reports Memory saved., while the assistant's own writer refuses both #1847), counts code points.core/src/host/project-assistant.js:193— the assistant writer counts the same unit.core/src/host/constants.js:24-29— theMAX_ENTRYcomment (line 29) states the unit and why.docs/subsystems/core-host.md:53— the Project Assistant row records the core-host: count a Project memory entry in code points, not UTF-16 units #1886 unit.Why the tip moved: a stale base on the merge tree
The queue built a merge tree for
818b71eaon base1adfbe9615a8and itsTestjob failed at 04:27:16Z inpackage:.github/scripts/ds-bot-patches.test.mjs:192—src/host/store.js hunk 1 (@@ -719,6 +719,10 @@)context mismatch. The storedpatches/ds-botseries was generated against575accc6, so once main moved past #1883/#1891/#1889 the series no longer describedcore/on the merged tree. The tip's own checks were green; the merge tree was not.So this head merges current main into the branch and regenerates the series, as the review recipe asks:
git merge d92fe89a— one conflict, inpatches/ds-bot/0002-flybot-api-host.patch; resolved by taking main's version ofpatches/ds-bot/, per the recipe.core/src/host/store.js,core/test/project-board-panels.test.jsanddocs/subsystems/core-host.mdauto-merged; I read the merged hunks, and no conflict markers remain anywhere in the tree.node scripts/ds-bot-patches.mjs generate— rewrotepatches/ds-bot/from the mergedcore/. On top of main's series the generator changed exactly one file,0002-flybot-api-host.patch(+8/−3), which holds this branch's own store/assistant hunks; every other patch it wrote back is byte-identical to main's.0007-version-and-tooling.patchis byte-identical to main's (the generator's "wrote" log lists it, so I checked withgit diffrather than counting log lines).0018-project-dossier.patchis already gone from main (Show each Project Bot's dossier on the project board #1883), so this branch deletes nothing.#978count assertion is already10on main with its explanation, from Warn about the Project memory bound before Save #1889; this branch does not edit it and does not relax anything. The test passes on its own merits below.core/is untouched by all of this: the branch's own change is unchanged, and generating patches does not rewrite sources.git rev-parse HEAD:coreand the tree of the workingcore/after generate are bothfbdc54a647f67efd8c16834d450926b3be82ea55.Red first
Clean base
575accc63bc42b3becd2003601126d2409b417fbwith only the two test files added,TZ=UTC node --import ./test/harness-resolve.mjs --test test/project-board-panels.test.js test/project-assistant.test.js:Both fail with
A Project memory entry is at most 600 characters/ codebot/invalid, and the stacks name the two old lines:store.js:521andproject-assistant.js:191. The tests use real emoji, not ASCII filler: 350 × 😀 is stored, 600 code points stored, 601 refused, ASCII 600 still stored and 601 still refused, and the assistant writer and the store accept and refuse the same entry.After the fix, on the verified tree (
core/test/project-board-panels.test.js:383,core/test/project-assistant.test.js:117), the same two files are# tests 41 / # pass 41 / # fail 0— 41 because main's #1887 counter tests live in the panels file now.What I ran
node --test .github/scripts/ds-bot-patches.test.mjson the verified tree — the check the queue actually failed:ok 9 - \#978: ...andok 10 - \#978: the git-free applier and git apply produce the same tree on every stored patch. This is the evidence for the patch series, notcheck(a series can passcheckand still fail the merge-tree test).TZ=UTC ARCH_DOCS_BASE=d92fe89a4ae4151eecc909008195c319d497476a npm run verifyon the clean committed tree (tree47f61400, the pushed tip's tree) — exit 0: core 2288/2288, social 541/541, tree 127/127, timeline 135/135, workflow 336/336,.github/scripts561/561, interop 37/37, live-e2e 24/24; core lint 73 warnings / 0 errors; plugin lint 36/9/2/16 warnings, 0 errors;arch-docs: OK (11 changed file(s))againstARCH_DOCS_BASE=d92fe89a;build-client --checkgreen inside the chain.node scripts/ds-bot-patches.mjs check— supporting evidence, three lines ok:ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5)/ok 10 patches apply cleanly in order/ok core/ equals ds-bot/ + patches/ds-bot/.cmpon fourpatches/ds-bot/files/mirrors againstcore/— byte-identical:src/host/project-assistant.js,test/project-assistant.test.js,test/project-board-panels.test.js,src/client/project-board.js.git status --porcelain— empty at the verified tree.What I did not run
core/dev/bench.sh smoke/core/dev/smoke.mjs): not run — it needs adeepseek-harnesssource checkout that this machine does not have. The host change is two length checks and a comment, so the unit suites above cover it.Diff
11 files, +170 / −7 (
git diff --stat d92fe89a 238882c4):core/src/host/constants.js,core/src/host/store.js,core/src/host/project-assistant.js,core/test/project-board-panels.test.js,core/test/project-assistant.test.js, their threepatches/ds-bot/files/mirrors,patches/ds-bot/0002-flybot-api-host.patch,docs/subsystems/core-host.md, andchangelog.d/1886-memory-code-points.fixed.md.core/client.jsis untouched — no client source changed on this branch, so no rebuild was needed andbuild-client --checkstays green.{ "pr": 1888, "claim": "Dev", "issue": 1886, "summary": "The Project memory bound counted UTF-16 code units, so a 350-character emoji note was refused as over 600 characters. Both writers and the store now count code points with [...text].length. This head also merges current main and regenerates patches/ds-bot, because the stored series was generated on an older base and failed the queue's merge-tree test; core/ itself is unchanged by that.", "files": ["core/src/host/store.js", "core/src/host/project-assistant.js", "core/src/host/constants.js", "core/test/project-board-panels.test.js", "core/test/project-assistant.test.js", "docs/subsystems/core-host.md", "changelog.d/1886-memory-code-points.fixed.md"], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=d92fe89a npm run verify; node --test .github/scripts/ds-bot-patches.test.mjs; node scripts/ds-bot-patches.mjs check", "result": "green", "at": "115f0c8d884231545a1684907e567d6cba711391" }, "next": "none — ready for review at this tip" }