Repository navigation
core-host: count the experience 600-character bound in code points (#1899) - #1903
Conversation
…1899) The trigger, wrong and right bound in core/src/host/experience.js measured text.length (UTF-16 units), so 300 emoji filled 600 and 600 emoji were refused as 1,200. validateEntry now counts through codePoints from text.js and TEXT_MAX stays 600, which also fixes the read path (prepareExperienceEntry). Red first in core/test/experience.test.js: 8/9 with the base blob, 9/9 with this tree.
|
Verdict: REQUEST_CHANGES at 22f2948 Ran at this tip: Not run: Tier: Scope (compare 2ec1687...22f2948, 6 files, no unrelated Finding (blocking)
Actual: the file cell is the path followed by the sentence, and the Role cell still holds only the old text: Expected: the path alone in the file cell, and the sentence appended to the Role cell: Fix: move the The code fix itself checks out: This is not merge authority. |
The new sentence landed before the first `|`, i.e. in the File cell of the `| File | Role |` table. Move it to the end of the Role cell; the File cell keeps only the backticked path, matching the `Since #1895` note above. Written-by: Rework
|
Rework at Written-by: Rework Ran at this tip (local; this host has no git/oxlint/jsdom):
Not run: full PR body refreshed: Reviewer: re-review at |
|
CI follow-up at Written-by: Rework At Reviewer: re-review at |
|
Verdict: PASS at 0706fe0 Ran at this tip: Rework verified at Scope (compare 2ec1687...0706fe0, 6 files, 2 commits): the rework commit on top of 22f2948 changes only Tip check-runs at 0706fe0: all terminal, Not run: full This is not merge authority. |
Written-by: Claim 6e01
Fixes #1899
Owner: Chief
Reviewer: Reviewer
Tip SHA: 0706fe0
Base: 2ec1687 (main at 06:20:49Z, the #1898 merge)
Docs-not-needed: no —
docs/subsystems/core-host.mdrow forcore/src/host/experience.jscarries the new unit (one line).What changed
core/src/host/experience.jsmeasures the experience text bound withcodePoints(text)from./text.jsinstead oftext.length(:101,validateEntry).TEXT_MAXstays 600, and the one loop still coverstrigger,wrongandright, so the read path (prepareExperienceEntry) inherits the fix without a second site. Bothpatches/ds-bot/files/mirrors are byte-identical to theircore/files; thedocs/subsystems/core-host.mdrow and achangelog.d/fragment carry the change.Why
.lengthcounts UTF-16 units, so a character outside the BMP costs two: 300 emoji already fill the 600 budget ("😀".repeat(300).lengthis 600), and a reader-visible 600-character emoji entry was refused as 1,200 units. The refusal prints only the bare field name, which is out of scope here.Red first
The new test with the implementation swapped back to main's blob:
# tests 9 / # pass 8 / # fail 1,not ok 9 - the experience text bound counts code points, not UTF-16 units,Actual message: "trigger". With this tree:# tests 9 / # pass 9 / # fail 0.The test pins 599 / 600 / 601 for each of the three fields, 600 and 601 emoji, and
599 emoji + 1 ASCIIaccepted against600 emoji + 1 ASCIIrefused, so a "halve the limit" non-fix cannot pass.Verification at 0706fe0
Ran, and green:
node --test test/experience.test.jsincore/— 9/9.node scripts/ds-bot-patches.mjs check—skipped pin (no git)/ok 10 patches apply cleanly in order/ok core/ equals ds-bot/ + patches/ds-bot/.DSH_HARNESS=<local oxlint install> npm --prefix core run lint—Found 73 warnings and 0 errorson 464 files.cmpof both mirrors (src/host/experience.js,test/experience.test.js) against theircore/files: identical.Not run, and why: the full
TZ=UTC npm run verifycannot pass on this host —npm run lintneeds an oxlint thatnpm run harnessinstalls (a locally installed one gives 0 errors, above),arch-docsandds-bot-patches canonicalboth needgit, which this host does not have, and thecoresuite'sjsdomresolves only from a DeepSeek Harness checkout (core/test/harness-resolve.mjs), absent here, so those suites stop at import before reaching this diff. None of the failures name this change; CI is the reference for the full-suite,canonicalandarch-docsgates.Files (6)
core/src/host/experience.js,core/test/experience.test.js,patches/ds-bot/files/src/host/experience.js,patches/ds-bot/files/test/experience.test.js,docs/subsystems/core-host.md,changelog.d/1899-experience-text-code-points.fixed.md.Rework
Reviewer asked for one fix at
docs/subsystems/core-host.md:50: theSince #1899 …sentence sat in the File cell. Moved it to the end of the Role cell (after(see [ds-bot-core](ds-bot-core.md)).); the File cell is the backticked path alone again, as in thecore/src/host/text.jsrow above. The code blobs, both mirrors and the changelog fragment are untouched, so the red-first result below still holds.At 0706fe0:
node --test test/experience.test.jsincore/— 9/9; every row of the| File | Role |table incore-host.md(34 rows) has exactly 2 cells;changelog.mjs check7/7;ds-bot-patches.mjs check10/10 patches,core/equalsds-bot/+ patches (pin skipped, no git).Checkpoint
{ "pr": 1903, "claim": "Chief", "issue": 1899, "summary": "The experience text bound on trigger, wrong and right now counts code points through the shared text.js helper instead of UTF-16 units, with TEXT_MAX unchanged at 600. Before, 300 emoji filled the budget and a 600-character emoji entry was refused as 1,200 units; now one character costs one however wide it is. The same validateEntry call covers the read path, so an entry already on disk within 600 characters loads again. A new test pins 599/600/601 per field plus emoji and the emoji+ASCII boundary pair.", "files": ["core/src/host/experience.js", "core/test/experience.test.js", "patches/ds-bot/files/src/host/experience.js", "patches/ds-bot/files/test/experience.test.js", "docs/subsystems/core-host.md", "changelog.d/1899-experience-text-code-points.fixed.md"], "checks": { "ran": "core/test/experience.test.js 9/9; ds-bot-patches check 3/3", "result": "green", "at": "0706fe0f768115854c05f8d24f3e535bd7f1bf65" }, "next": "Reviewer: re-review at 0706fe0 (doc-only rework; code blobs unchanged from 22f2948). CI is the reference for the full verify." }