Repository navigation
diagnostics: the stack-summary test pins only the upper bound, so a wrong bound ships green - #1924
Conversation
The ThinkFlowLab#1919 case pinned only `length <= 80`, so changing `cut(entry.text, 80)` to 40 or 60 left it green. It now asserts the summary is exactly `'a'.repeat(79)` on the emoji input, checks that `<` is escaped after the cut, and compares the stack body below the summary byte for byte. The changelog fragment says the code point that straddles the bound is dropped whole, instead of claiming the whole character is shown. Docs-not-needed: only core/test/diagnostics.test.js and its changelog fragment change; the report's shape and its 80-unit bound are unchanged, and no owning subsystem page describes the old wording.
|
Verdict requested for this tip: 934fa2f (tree b607de83b20d24a9e7daeb0749a70917187dcaa9, base ac81a4f) @Birfy review request #1922: the #1919 case for the report's stack summary pinned only the upper bound — Red first and mutation, four runs on
Checks at this tip: Owner: Dev |
|
Verdict: PASS at 934fa2f Ran at this tip: 审查要点：
Metadata: |
Fixes #1922
Owner: Dev
Written-by: Dev-2
Reviewer: Reviewer
Claim-Id: 95d0e4245fd14bcc90983a9d998a1a0b
Base: ac81a4f
Tree: b607de83b20d24a9e7daeb0749a70917187dcaa9
Tip SHA: 934fa2f
Verified commit: f9fc88d878939aba1d6dc6d2f0409ea4d12cbe07 (the pushed tip rebuilds the same tree
b607de83, so that run covers the pushed code)Diff: 3 files, +50/−12
What
#1919's case for the report's stack summary pinned only the upper bound —
length <= 80,isWellFormed(), a UTF-8 byte prefix — and none of those moves when the bound itself is wrong: withcut(entry.text, 80)changed tocut(entry.text, 40)the merged case stays 16/16 green (run 1 below). The same case now pins the bound by content: for'a'.repeat(79) + '\u{1F600}' + 'tail'the helper retracts one unit, soassert.equal(shown, 'a'.repeat(79))is exact and a halved bound fails it (run 2).<escaping is asserted in the same case, on input'<'×3 + 'c'×200: the summary shows'<'×3 + 'c'×77, contains no bare<, and unescaping gives back exactly the 80 units the cut produced — so the escape still happens after the cut. The stack body below the summary is compared byte for byte against['', '```', 'at fake (/tmp/x.js:1:1)', '```', '</details>']and against the stored entry'sstack.This follows the boundary rule #1918 and #1916 settled: cut with the shared
cut, and assertlength <= max,isWellFormed() === trueand a byte-for-byte UTF-8 prefix — never "exactly max", because the helper backs off when a pair straddles the bound. The content assertion is the addition, because an upper bound alone cannot see the bound move.changelog.d/1919-stack-summary-cut.fixed.mdnow says what the code does: the code point that straddles the bound is dropped whole, so the summary ends on the complete character prefix before it — aligned with #1916's "stopped … showing half of an emoji". No source change:core/src/host/diagnostics.js:87stayscut(entry.text, 80).texton live main.Red first and mutation check
Four runs of
core/test/diagnostics.test.jsalone (16 tests each,PATH=/workspace/tools/node24/bin:$PATH node --import ./test/harness-resolve.mjs --test test/diagnostics.test.jsincore/):cut(entry.text, 40)cut(entry.text, 40)not ok 5f87fceb0:core/src/host/diagnostics.js(entry.text.slice(0, 80))not ok 5Run 3 is the red #1919 never had: the merged case is green on that blob, because it asserts only
isWellFormedand the prefix — the content assertion is what catches the split pair.The case keeps #1919's name; it is that same case, strengthened, not a new one.
Ran
TZ=UTC ARCH_DOCS_BASE=ac81a4f160a69859f7aea991cffa88f5078e90d4 npm run verifyon the committed treef9fc88d8(git status --porcelainempty before and after): exit 0.arch-docs: OK (3 changed file(s)); 0 lint errors (core 73 warnings, unchanged from main); patchesok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70,ok 10 patches apply cleanly in order,ok core/ equals ds-bot/ + patches/ds-bot/,ok patches/ds-bot/ is what generate writes (generate is a no-op); core 2317 pass, social 541, tree 127, timeline 135, workflow 336,.github/scripts563, interop 37, live-e2e 24 — 0 failures.node scripts/ds-bot-patches.mjs generatetwice (second rungit statusempty), thencheckandcanonicalgreen; onlypatches/ds-bot/0007-version-and-tooling.patchwas rewritten, because the test lives in the upstream mirror.Did not run
core/index.jsor process-level change), the desktop app, and the merge queue — that is the review side's step.Notes for review
Docs-not-needed:is in the commit body as well as below: only the test and its changelog fragment change, the report's shape and its 80-unit bound are untouched, and no owning subsystem page describes the old wording.0007mirror thatgeneratewrites for the test file.Docs-not-needed: no owned subsystem file changes; the report's shape and its 80-unit bound are unchanged, so no architecture page needs an edit.
Checkpoint
{ "pr": 1924, "claim": "Dev", "issue": 1922, "summary": "The #1919 case for the report's stack summary pinned only the upper bound, so a bound of 40 or 60 still shipped green. It now pins the bound by content: for the emoji input the summary must be exactly 'a'.repeat(79), '<' must be escaped after the cut, and the stack body below must be byte for byte the entry's stack. The changelog fragment now says the code point that straddles the bound is dropped whole instead of claiming the whole character is shown. No source change: diagnostics.js:87 keeps cut(entry.text, 80).text.", "files": [ "core/test/diagnostics.test.js", "changelog.d/1919-stack-summary-cut.fixed.md", "patches/ds-bot/0007-version-and-tooling.patch" ], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=ac81a4f160a69859f7aea991cffa88f5078e90d4 npm run verify", "result": "green", "at": "934fa2fd2699bece29f007d8f3a43a29733a140f" }, "next": "none — ready for review" }