From 6e7a8ba203ea70d70573cf34d7618c262d7c6af8 Mon Sep 17 00:00:00 2001 From: Shevchik Igor Date: Sun, 16 Aug 2026 03:15:34 +0000 Subject: [PATCH 1/2] test(CommandPalette): pin the CRLF conjunction and the malformed-region guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two real gaps left by a mutation pass over every constant and branch in `src/runtime/utils/search.ts` — 62 mutations, 55 killed, 5 provably equivalent, these two survivors. **The CRLF carve-out was pinned by its constants but not by its conjunction** (#410). `previous === CARRIAGE_RETURN && current === LINE_FEED` could widen to `||` with the whole suite green, because the only CR/LF fixture is a real pair and on that input both operators agree. Widened, a mark swallows the character after a lone CR, or the LF after any character: 'a\rXb' → 'a\rXb' instead of 'a\rXb' 'aX\nb' → 'aX\nb' instead of 'aX\nb' Mixed line endings in indexed content are ordinary, so this is reachable rather than theoretical. Both fixtures sit entirely below the fast-path floor, which is what routes them into the carve-out instead of to the segmenter. **The single-character skip's length check had no fixture** (#411). `region.length === 2` is unreachable from a well-typed caller — every region is a real 2-tuple — so dropping it changed nothing observable. It is not dead code though: `CommandPaletteGroup.postFilter` takes whatever the application hands it, which is the same reason the integer filter beside it exists. A three-element array whose first two bounds match is malformed, not degenerate, and must not be skipped as one. The fixture casts through `unknown` deliberately, since that is how a loosely-typed caller reaches it. Each mutation is now killed by exactly one test and nothing else, and the pre-existing pair test still pins `CARRIAGE_RETURN` — verified, so the new case adds coverage rather than moving it. Closes #410. Closes #411. Tests only; `src/` untouched. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LWWrBHgfqGSbeU3V6UuMF8 --- test/utils/search.spec.ts | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/utils/search.spec.ts b/test/utils/search.spec.ts index 209c6a0c..001d40cf 100644 --- a/test/utils/search.spec.ts +++ b/test/utils/search.spec.ts @@ -419,6 +419,24 @@ describe('highlight', () => { expect(highlight({ label: value, matches: [{ key: 'label', value, indices: [[2, 2]] }] }, 'a', 'label')).toBe(value) }) + it('does not treat a malformed region as degenerate just because its first two bounds match', () => { + // The skip is guarded on `region.length === 2` as well as on the bounds + // being equal, and that half had no fixture: every region reaching it from + // a well-typed caller is a real 2-tuple, so dropping the length check + // changed nothing observable (#411). + // + // `RangeTuple` is what the type says; `CommandPaletteGroup.postFilter` + // takes whatever the application hands it, which is the same reason the + // integer filter below exists. A longer array is not a one-character + // region and must not be skipped as one — the cast is how a loosely-typed + // caller reaches this, deliberately. + const value = 'alpha beta' + const malformed = [[2, 2, 999]] as unknown as [number, number][] + + expect(highlight({ label: value, matches: [{ key: 'label', value, indices: malformed }] }, 'a', 'label')) + .toBe('alpha beta') + }) + it.each(ASTRAL)('wraps %s wholly, wherever the region boundary falls', (astral) => { const value = `${astral.repeat(3)}tail` @@ -542,6 +560,24 @@ describe('highlight', () => { expect(result).toBe('a\r\nb') }) + it('keeps a lone CR or LF apart — the carve-out is the pair, not either half', () => { + // The case above cannot tell `previous === CR && current === LF` from + // `previous === CR || current === LF`: on a real pair both are true. So + // the conjunction went unpinned while its two constants were guarded, and + // widening it to `||` made a mark swallow the character after a lone CR, + // or the LF after any character (#410). + // + // Both fixtures sit entirely below the fast-path floor, which is what + // routes them into the carve-out rather than to the segmenter. + const afterLoneCR = 'a\rXb' + const beforeLoneLF = 'aX\nb' + + expect(highlight({ label: afterLoneCR, matches: [{ key: 'label', value: afterLoneCR, indices: [[0, 1]] }] }, 'x', 'label')) + .toBe('a\rXb') + expect(highlight({ label: beforeLoneLF, matches: [{ key: 'label', value: beforeLoneLF, indices: [[0, 1]] }] }, 'x', 'label')) + .toBe('aX\nb') + }) + it('leaves a bare run of emoji modifiers whole — UAX #29 makes it one cluster', () => { // Not a realistic label, but it pins the behaviour that made the earlier // fixture wrong: 20 bare U+1F3FF are one character, not twenty, so the From 8ed159e82c929492fd99a12ea8e3c75233230142 Mon Sep 17 00:00:00 2001 From: Shevchik Igor Date: Sun, 16 Aug 2026 03:54:28 +0000 Subject: [PATCH 2/2] test(CommandPalette): cover both shapes of CR/LF over-joining, and correct a claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three reviews of the first revision, three things to fix. **One fixture pair was not enough.** `||` fires when either side matches, but `(CR|LF) && (CR|LF)` fires when both sides are line breaks in any order — an equally plausible typo, and it survived a lone CR beside a letter and a lone LF after one. It needs a reversed or doubled pair. `a\n\rb`, `a\r\rb` and `a\n\nb` are added; the segmenter breaks all three, verified against a real `Intl.Segmenter`, and the mutant joins all three. **The test invited an assumption it does not earn.** Every fixture in it is a pair the carve-out correctly declines to join, so it is blind to *under*-joining — a deleted carve-out, or no snapping at all, sails through untouched. That belongs to the pre-existing pair test twelve lines above, and the comment now says so rather than leaving the family resemblance to imply redundancy. Renamed to describe what it pins: CR then LF in that order, never either alone. **The malformed-region comment claimed a parity that does not hold.** It said the length check exists for the same reason as the integer filter. It does not. The filter removes a region whose `NaN` would otherwise reach the cursor and repeat the whole value; the length check sanitizes nothing, because nothing downstream reads past `region[1]` — verified, only indices 0 and 1 are read anywhere in the file. It decides one thing: whether a malformed region gets the one-character skip. It should not, since a longer array is out of contract rather than degenerate, and skipping it drops a highlight the caller asked for. The same review found the branch is reachable with no cast at all: `postFilter` is typed `(term, items: T[]) => T[]` and `CommandPaletteItem` carries a `[key: string]: any`, so `items[i].matches` is `any` inside that callback, type-checked against the real types under `--strict`. The `unknown` here is an artefact of calling `highlight()` directly, not of the scenario. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LWWrBHgfqGSbeU3V6UuMF8 --- test/utils/search.spec.ts | 71 ++++++++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 23 deletions(-) diff --git a/test/utils/search.spec.ts b/test/utils/search.spec.ts index 001d40cf..7f6e3d05 100644 --- a/test/utils/search.spec.ts +++ b/test/utils/search.spec.ts @@ -421,15 +421,24 @@ describe('highlight', () => { it('does not treat a malformed region as degenerate just because its first two bounds match', () => { // The skip is guarded on `region.length === 2` as well as on the bounds - // being equal, and that half had no fixture: every region reaching it from - // a well-typed caller is a real 2-tuple, so dropping the length check - // changed nothing observable (#411). + // being equal, and that half had no fixture: every region the suite builds + // is a real 2-tuple, so dropping the length check changed nothing (#411). // - // `RangeTuple` is what the type says; `CommandPaletteGroup.postFilter` - // takes whatever the application hands it, which is the same reason the - // integer filter below exists. A longer array is not a one-character - // region and must not be skipped as one — the cast is how a loosely-typed - // caller reaches this, deliberately. + // Reachable without any cast from application code: `postFilter` is typed + // `(term, items: T[]) => T[]`, and `CommandPaletteItem` carries a + // `[key: string]: any` index signature, so `items[i].matches` is `any` + // inside that callback. `unknown` appears here only because this test + // calls `highlight()` directly, against its stricter signature. + // + // Worth being exact about what the guard is and is not. It does not + // sanitize: nothing downstream reads past `region[1]`, so a three-element + // region slices identically either way. It decides one thing only — + // whether a malformed region gets the one-character skip. It should not, + // because a longer array is out of contract rather than degenerate, and + // silently treating it as a single character drops a highlight the caller + // asked for. That is a smaller claim than the integer filter above, which + // removes a region whose `NaN` would otherwise reach the cursor and repeat + // the whole value. const value = 'alpha beta' const malformed = [[2, 2, 999]] as unknown as [number, number][] @@ -560,22 +569,38 @@ describe('highlight', () => { expect(result).toBe('a\r\nb') }) - it('keeps a lone CR or LF apart — the carve-out is the pair, not either half', () => { - // The case above cannot tell `previous === CR && current === LF` from - // `previous === CR || current === LF`: on a real pair both are true. So - // the conjunction went unpinned while its two constants were guarded, and - // widening it to `||` made a mark swallow the character after a lone CR, - // or the LF after any character (#410). + it('joins CR and LF in that order only, never either one alone', () => { + // Every pair here is one the segmenter breaks, so a correct + // implementation snaps none of them. That is the point: the case above + // holds the only pair the carve-out *does* join, and on a real CR+LF + // `previous === CR && current === LF` and `previous === CR || current + // === LF` agree. So the conjunction went unpinned while both its + // constants were guarded (#410). + // + // Two shapes of over-joining, and the second is why one fixture is not + // enough: `||` fires when either side matches, `(CR|LF) && (CR|LF)` fires + // when both sides are line breaks in any order. The first is caught by a + // lone CR or LF beside a letter; the second survives that and needs a + // reversed or doubled pair. + // + // Under-joining — failing to keep a real CR+LF together — is what the + // case above pins. These add nothing there, deliberately; do not read + // them as covering it. // - // Both fixtures sit entirely below the fast-path floor, which is what - // routes them into the carve-out rather than to the segmenter. - const afterLoneCR = 'a\rXb' - const beforeLoneLF = 'aX\nb' - - expect(highlight({ label: afterLoneCR, matches: [{ key: 'label', value: afterLoneCR, indices: [[0, 1]] }] }, 'x', 'label')) - .toBe('a\rXb') - expect(highlight({ label: beforeLoneLF, matches: [{ key: 'label', value: beforeLoneLF, indices: [[0, 1]] }] }, 'x', 'label')) - .toBe('aX\nb') + // Every fixture sits entirely below the fast-path floor, which is what + // routes it into the carve-out rather than to the segmenter. + const pairs: [string, string][] = [ + ['a\rXb', 'a\rXb'], + ['aX\nb', 'aX\nb'], + ['a\n\rb', 'a\n\rb'], + ['a\r\rb', 'a\r\rb'], + ['a\n\nb', 'a\n\nb'] + ] + + for (const [value, expected] of pairs) { + expect(highlight({ label: value, matches: [{ key: 'label', value, indices: [[0, 1]] }] }, 'x', 'label')) + .toBe(expected) + } }) it('leaves a bare run of emoji modifiers whole — UAX #29 makes it one cluster', () => {