From dfa7a5e969ef8cf99f78774f447ef29645cbcdfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 2 Aug 2026 09:29:21 +0200 Subject: [PATCH 1/2] feat(mutation): add target-annotation-serde + snapshot-occlusion kernels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both are pure decision kernels the lane's own membership rule covers (target-annotation-serde: parse/validate/normalize the .ad comment-line codec, zero I/O; snapshot-occlusion: pure covered/not-covered decision where a wrong answer silently blocks or mis-allows a tap) but were excluded from KERNEL_MODULES. Fixing the harness's packages/*/src blind spot was required, not optional: test-scope.ts, ownership.ts, and vitest.mutation.config.ts all hardcoded `src/` as the only place a kernel's tests could live. target-annotation-serde's own tests live under packages/ad-script/src/internal/__tests__/, so without this fix the module would score 0% from day one — not from weak tests, but because its test file was silently invisible to the lane. Widened the same three places, plus mutation-affected.yml's path filter and isTestFile/ownedTestFiles in ownership.ts, to also recognize packages/*/src/**/*.test.ts (mirroring vitest.config.ts's own unit-core project include list). Triaged every surviving mutant from the initial run: real coverage gaps got a new/adjusted test (kill-with-test), everything else is documented equivalent with an inline comment at the mutation site explaining the invariant that makes it unobservable (redundant early-returns, JSON.stringify dropping undefined-valued keys, Number.isFinite/isSafeInteger's total-function safety, caller-enforced positiveRect/candidate invariants, etc). Baseline recorded from the actual measured run, not inherited or guessed: 94.03% (315/335) and 89.74% (175/195). --- .github/workflows/mutation-affected.yml | 10 +- .github/workflows/mutation-weekly.yml | 4 +- mutation-baselines/decision-kernels.json | 18 + .../__tests__/target-annotation-serde.test.ts | 270 +++++++++++- .../src/internal/target-annotation-serde.ts | 31 ++ scripts/mutation/modules.ts | 29 +- scripts/mutation/ownership.ts | 25 +- scripts/mutation/test-scope.ts | 36 +- scripts/mutation/workflow.test.ts | 11 +- .../__tests__/snapshot-occlusion.test.ts | 410 ++++++++++++++++++ src/snapshot/snapshot-occlusion.ts | 54 +++ stryker.config.json | 4 +- vitest.mutation.config.ts | 2 +- 13 files changed, 862 insertions(+), 42 deletions(-) diff --git a/.github/workflows/mutation-affected.yml b/.github/workflows/mutation-affected.yml index 3d80c04547..31e7650468 100644 --- a/.github/workflows/mutation-affected.yml +++ b/.github/workflows/mutation-affected.yml @@ -22,15 +22,19 @@ name: Mutation Affected on: pull_request: paths: - # Kernel sources, every src test (ownership is derived, so any test may own - # a kernel — `select` decides, not this filter), and the lane's own tooling. - # scripts/mutation/workflow.test.ts asserts this covers the registry. + # Kernel sources, every src/package test (ownership is derived, so any + # test may own a kernel — `select` decides, not this filter), and the + # lane's own tooling. scripts/mutation/workflow.test.ts asserts this + # covers the registry. - 'packages/kernel/src/errors.ts' - 'src/daemon/ref-frame.ts' - 'src/commands/interaction/runtime/settle.ts' - 'src/utils/scroll-edge-state.ts' - 'src/selectors/**' + - 'packages/ad-script/src/internal/target-annotation-serde.ts' + - 'src/snapshot/snapshot-occlusion.ts' - 'src/**/*.test.ts' + - 'packages/*/src/**/*.test.ts' - 'scripts/mutation/**' - 'scripts/lib/**' - 'stryker.config.json' diff --git a/.github/workflows/mutation-weekly.yml b/.github/workflows/mutation-weekly.yml index 1d3135157d..939044761f 100644 --- a/.github/workflows/mutation-weekly.yml +++ b/.github/workflows/mutation-weekly.yml @@ -52,6 +52,8 @@ jobs: - { name: selectors-2, module: selectors, shard: 2/4 } - { name: selectors-3, module: selectors, shard: 3/4 } - { name: selectors-4, module: selectors, shard: 4/4 } + - { name: target-annotation-serde, module: target-annotation-serde } + - { name: snapshot-occlusion, module: snapshot-occlusion } steps: - name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -109,7 +111,7 @@ jobs: # runner exports it, so the summary and the artifact carry the same numbers. - name: Ratchet the merged sweep and propose the next baseline run: | - pnpm mutation:check --report-dir .tmp/mutation/shards --expect-shards 8 --update + pnpm mutation:check --report-dir .tmp/mutation/shards --expect-shards 10 --update cp mutation-baselines/decision-kernels.json .tmp/mutation/proposed-baseline.json git checkout -- mutation-baselines/decision-kernels.json diff --git a/mutation-baselines/decision-kernels.json b/mutation-baselines/decision-kernels.json index 357333b174..402b34ce74 100644 --- a/mutation-baselines/decision-kernels.json +++ b/mutation-baselines/decision-kernels.json @@ -48,6 +48,24 @@ "strykerVersion": "9.6.1", "configHash": "sha256:806d9f2e657f", "updatedAt": "2026-07-27T14:29:40.750Z" + }, + "target-annotation-serde": { + "score": 94.03, + "killed": 315, + "survived": 20, + "total": 335, + "strykerVersion": "9.6.1", + "configHash": "sha256:8b633048f754", + "updatedAt": "2026-08-02T07:28:56.278Z" + }, + "snapshot-occlusion": { + "score": 89.74, + "killed": 175, + "survived": 20, + "total": 195, + "strykerVersion": "9.6.1", + "configHash": "sha256:8b633048f754", + "updatedAt": "2026-08-02T07:28:56.278Z" } } } diff --git a/packages/ad-script/src/internal/__tests__/target-annotation-serde.test.ts b/packages/ad-script/src/internal/__tests__/target-annotation-serde.test.ts index 5b7fc07a30..d0a0b52b14 100644 --- a/packages/ad-script/src/internal/__tests__/target-annotation-serde.test.ts +++ b/packages/ad-script/src/internal/__tests__/target-annotation-serde.test.ts @@ -103,6 +103,17 @@ test('a line that merely mentions the tag in prose is an ordinary comment', () = assert.deepEqual(parseTargetAnnotationCommentLine('# just a comment'), { kind: 'none' }); }); +test('a line that is not a comment at all is not a target annotation', () => { + assert.deepEqual(parseTargetAnnotationCommentLine('const x = 1;'), { kind: 'none' }); +}); + +test('leading and trailing whitespace around the annotation line does not break parsing', () => { + const result = parseTargetAnnotationCommentLine( + ' # agent-device:target-v1 {"role":"button","verification":"verified"} ', + ); + assert.equal(result.kind, 'v1'); +}); + // --------------------------------------------------------------------------- // Normalization: NFC, label trim/collapse, normalized-role source // --------------------------------------------------------------------------- @@ -117,6 +128,25 @@ test('normalizeLabelField treats a whitespace-only label as absent', () => { assert.equal(normalizeLabelField(' '), undefined); }); +test('an empty-string id is treated as absent, like a whitespace-only label', () => { + const parsed = parseTargetAnnotationV1Payload( + JSON.stringify({ id: '', role: 'button', verification: 'verified' }), + ); + assert.equal(parsed.id, undefined); +}); + +test('scrollRegion.label serializes and round trips like every other label field', () => { + const evidence = baseEvidence({ + id: undefined, + label: undefined, + scrollRegion: { role: 'scrollview', id: 'editor-scroll', label: 'Body' }, + }); + const json = serializeTargetAnnotationV1(evidence); + assert.ok(json.includes('"scrollRegion":{"role":"scrollview","id":"editor-scroll","label":"Body"}')); + const parsed = parseTargetAnnotationV1Payload(json); + assert.equal(parsed.scrollRegion?.label, 'Body'); +}); + test('embedded quotes and backslashes in labels round trip losslessly', () => { const evidence = baseEvidence({ label: 'Say "hi" \\ backslash', id: undefined }); const json = serializeTargetAnnotationV1(evidence); @@ -200,6 +230,38 @@ test('parser rejects more than 8 ancestry entries', () => { ); }); +test('parser accepts a payload landing exactly on the 4 KiB cap boundary', () => { + // A fixed filler length below the field cap on every other string field, + // plus one tunable ancestry-entry role (also within its own 256-byte field + // cap) sized by exact arithmetic — every character is ASCII, so 1 char is 1 + // UTF-8 byte, and the tunable field's required length is computable + // directly rather than searched for. + const FILLER = 'x'.repeat(180); + const build = (padLength: number) => { + const ancestry = Array.from({ length: TARGET_ANNOTATION_MAX_ANCESTRY }, (_unused, index) => ({ + role: index === 0 ? 'x'.repeat(padLength) : FILLER, + label: index === 0 ? undefined : FILLER, + })); + return JSON.stringify({ + id: FILLER, + role: FILLER, + label: FILLER, + ancestry, + scrollRegion: { role: FILLER, id: FILLER, label: FILLER }, + verification: 'verified', + }); + }; + const base = Buffer.byteLength(build(0), 'utf8'); + const padLength = TARGET_ANNOTATION_MAX_PAYLOAD_BYTES - base; + assert.ok( + padLength >= 0 && padLength <= TARGET_ANNOTATION_MAX_FIELD_BYTES, + `fixture no longer straddles the cap (base ${base}, needs pad ${padLength})`, + ); + const json = build(padLength); + assert.equal(Buffer.byteLength(json, 'utf8'), TARGET_ANNOTATION_MAX_PAYLOAD_BYTES); + assert.doesNotThrow(() => parseTargetAnnotationV1Payload(json)); +}); + test('truncateToUtf8Bytes never splits a surrogate pair', () => { const emoji = '\u{1F600}'; // 4 UTF-8 bytes, a surrogate pair in UTF-16 const truncated = truncateToUtf8Bytes(`ab${emoji}`, 3); @@ -210,6 +272,27 @@ test('truncateToUtf8Bytes never splits a surrogate pair', () => { assert.equal(/[\ud800-\udbff]$/.test(truncated), false); }); +test('truncateToUtf8Bytes drops a lone high surrogate at either end of its range', () => { + // Codepoint 0x10000 encodes to the lowest high surrogate (0xD800); 0x10FFFF + // (the last valid Unicode codepoint) encodes to the highest (0xDBFF). Both + // bound cases must trigger the drop, not just a value in the middle of the + // range. + for (const codepoint of [0x10000, 0x10ffff]) { + const astral = String.fromCodePoint(codepoint); + const truncated = truncateToUtf8Bytes(`a${astral}`, 4); + assert.equal(truncated, 'a', `codepoint 0x${codepoint.toString(16)} left a lone surrogate`); + } +}); + +test('truncateToUtf8Bytes keeps a fully-paired surrogate that lands exactly on the byte budget', () => { + // The budget lands the cut right after a complete surrogate pair (trimming + // only the unrelated trailing "cd"), so nothing here is a split — the + // dangling-high-surrogate guard must NOT also fire on the pair's low half. + const astral = String.fromCodePoint(0x10000); + const truncated = truncateToUtf8Bytes(`ab${astral}cd`, 6); + assert.equal(truncated, `ab${astral}`); +}); + // --------------------------------------------------------------------------- // Malformed / unbound annotations // --------------------------------------------------------------------------- @@ -219,33 +302,190 @@ test('parser rejects non-JSON payloads', () => { }); test('parser rejects a JSON array or scalar payload', () => { - assertInvalidArgs(() => parseTargetAnnotationV1Payload('[]')); - assertInvalidArgs(() => parseTargetAnnotationV1Payload('"button"')); + assertInvalidArgs(() => parseTargetAnnotationV1Payload('[]'), /must be a JSON object/); + assertInvalidArgs(() => parseTargetAnnotationV1Payload('"button"'), /must be a JSON object/); +}); + +// `typeof null === 'object'` in JS, so a bare `null` payload passes the +// typeof check above and needs its own explicit rejection — the array/scalar +// test above cannot exercise this branch. +test('parser rejects a null payload', () => { + assertInvalidArgs(() => parseTargetAnnotationV1Payload('null'), /must be a JSON object/); }); test('parser rejects a wrong-typed known field', () => { - assertInvalidArgs(() => - parseTargetAnnotationV1Payload(JSON.stringify({ role: 42, verification: 'verified' })), + assertInvalidArgs( + () => parseTargetAnnotationV1Payload(JSON.stringify({ role: 42, verification: 'verified' })), + /"role" must be a string/, + ); +}); + +test('parser rejects wrong-typed optional id and label fields, naming each in the message', () => { + assertInvalidArgs( + () => + parseTargetAnnotationV1Payload( + JSON.stringify({ id: 42, role: 'button', verification: 'verified' }), + ), + /"id" must be a string/, + ); + assertInvalidArgs( + () => + parseTargetAnnotationV1Payload( + JSON.stringify({ role: 'button', label: 42, verification: 'verified' }), + ), + /"label" must be a string/, + ); +}); + +test('parser rejects ancestry that is not an array', () => { + assertInvalidArgs( + () => + parseTargetAnnotationV1Payload( + JSON.stringify({ role: 'button', ancestry: 'toolbar', verification: 'verified' }), + ), + /"ancestry" must be an array/, + ); +}); + +test('parser rejects an ancestry entry that is not an object', () => { + assertInvalidArgs( + () => + parseTargetAnnotationV1Payload( + JSON.stringify({ role: 'button', ancestry: ['toolbar'], verification: 'verified' }), + ), + /"ancestry\[0\]" must be an object/, + ); + assertInvalidArgs( + () => + parseTargetAnnotationV1Payload( + JSON.stringify({ role: 'button', ancestry: [null], verification: 'verified' }), + ), + /"ancestry\[0\]" must be an object/, + ); +}); + +test('parser rejects a scrollRegion or rect that is not an object', () => { + assertInvalidArgs( + () => + parseTargetAnnotationV1Payload( + JSON.stringify({ role: 'button', scrollRegion: 'list', verification: 'verified' }), + ), + /"scrollRegion" must be an object/, + ); + assertInvalidArgs( + () => + parseTargetAnnotationV1Payload( + JSON.stringify({ role: 'button', rect: 'somewhere', verification: 'verified' }), + ), + /"rect" must be an object/, + ); +}); + +// `typeof null === 'object'`, so a null scrollRegion/rect needs its own +// explicit rejection — same trap as the top-level null-payload case above. +// Getting this wrong crashes on `null.role`/`null.x` (a raw TypeError) +// instead of a graceful AppError, unlike the string-typed case above. +test('parser rejects a null scrollRegion or rect with AppError, not a native crash on null property access', () => { + assertInvalidArgs( + () => + parseTargetAnnotationV1Payload( + JSON.stringify({ role: 'button', scrollRegion: null, verification: 'verified' }), + ), + /"scrollRegion" must be an object/, + ); + assertInvalidArgs( + () => + parseTargetAnnotationV1Payload( + JSON.stringify({ role: 'button', rect: null, verification: 'verified' }), + ), + /"rect" must be an object/, ); }); test('parser rejects an invalid verification value', () => { - assertInvalidArgs(() => - parseTargetAnnotationV1Payload(JSON.stringify({ role: 'button', verification: 'maybe' })), + assertInvalidArgs( + () => parseTargetAnnotationV1Payload(JSON.stringify({ role: 'button', verification: 'maybe' })), + /"verification" must be "verified" or "unverifiable"/, ); }); -test('parser rejects a negative or non-integer sibling/viewportOrder', () => { - assertInvalidArgs(() => - parseTargetAnnotationV1Payload( - JSON.stringify({ role: 'button', sibling: -1, verification: 'verified' }), - ), +test('parser rejects a negative or non-integer sibling/viewportOrder, naming each in the message', () => { + assertInvalidArgs( + () => + parseTargetAnnotationV1Payload( + JSON.stringify({ role: 'button', sibling: -1, verification: 'verified' }), + ), + /"sibling" must be a non-negative integer/, ); - assertInvalidArgs(() => - parseTargetAnnotationV1Payload( - JSON.stringify({ role: 'button', viewportOrder: 1.5, verification: 'verified' }), - ), + assertInvalidArgs( + () => + parseTargetAnnotationV1Payload( + JSON.stringify({ role: 'button', viewportOrder: 1.5, verification: 'verified' }), + ), + /"viewportOrder" must be a non-negative integer/, + ); +}); + +test('parser rejects a wrong-typed scrollRegion.id and scrollRegion.label, naming each in the message', () => { + assertInvalidArgs( + () => + parseTargetAnnotationV1Payload( + JSON.stringify({ + role: 'button', + scrollRegion: { role: 'list', id: 42 }, + verification: 'verified', + }), + ), + /"id" must be a string/, + ); + assertInvalidArgs( + () => + parseTargetAnnotationV1Payload( + JSON.stringify({ + role: 'button', + scrollRegion: { role: 'list', label: 42 }, + verification: 'verified', + }), + ), + /"scrollRegion\.label" must be a string/, + ); +}); + +test('parser rejects a wrong-typed ancestry entry label, naming it in the message', () => { + assertInvalidArgs( + () => + parseTargetAnnotationV1Payload( + JSON.stringify({ + role: 'button', + ancestry: [{ role: 'toolbar', label: 42 }], + verification: 'verified', + }), + ), + /"ancestry\[0\]\.label" must be a string/, + ); +}); + +test('parser rejects a wrong-typed rect field, naming each in the message', () => { + for (const field of ['x', 'y', 'width', 'height']) { + assertInvalidArgs( + () => + parseTargetAnnotationV1Payload( + JSON.stringify({ + role: 'button', + rect: { x: 1, y: 2, width: 3, height: 4, [field]: 'nope' }, + verification: 'verified', + }), + ), + new RegExp(`"rect\\.${field}" must be a finite number`), + ); + } +}); + +test('a whitespace-only label collapses to absent through the full parse, not just the normalizer', () => { + const parsed = parseTargetAnnotationV1Payload( + JSON.stringify({ role: 'button', label: ' ', verification: 'verified' }), ); + assert.equal(parsed.label, undefined); }); // --------------------------------------------------------------------------- diff --git a/packages/ad-script/src/internal/target-annotation-serde.ts b/packages/ad-script/src/internal/target-annotation-serde.ts index 7ed6f89042..57666ac3de 100644 --- a/packages/ad-script/src/internal/target-annotation-serde.ts +++ b/packages/ad-script/src/internal/target-annotation-serde.ts @@ -77,6 +77,10 @@ export function normalizeLabelField(value: string | undefined): string | undefin return collapsed.length > 0 ? collapsed : undefined; } +// Mutation-lane note: the `'utf8'` argument is provably redundant — +// `Buffer.byteLength` falls back to utf8 for any unrecognized encoding +// string (including `''`), so this call is byte-identical to the no-encoding +// default. export function utf8ByteLength(value: string): number { return Buffer.byteLength(value, 'utf8'); } @@ -87,6 +91,12 @@ export function utf8ByteLength(value: string): number { * surrogate pair is never split. The parser never calls this — it REJECTS * oversized fields instead (see `parseTargetAnnotationV1Payload`). */ +// Mutation-lane note: the `<= maxBytes` early return and the loop's `end > 0` +// guard are both provably redundant, not undertested — `value.slice(0, 0)` +// is always `''` (byte length 0), so the loop's own shrink-until-it-fits +// condition converges to the identical `end` with or without either guard, +// for every caller (`maxBytes` here is always a non-negative constant). +// They stay for clarity/defense-in-depth, not correctness. export function truncateToUtf8Bytes(value: string, maxBytes: number): string { if (utf8ByteLength(value) <= maxBytes) return value; let end = value.length; @@ -105,6 +115,11 @@ export function truncateToUtf8Bytes(value: string, maxBytes: number): string { // key order from the example payload). // --------------------------------------------------------------------------- +// Mutation-lane note: each `if (x !== undefined) obj.x = x` guard below is +// provably redundant on its "always assign" side — `JSON.stringify` omits any +// key whose value is `undefined`, so `obj.x = undefined` and never assigning +// `obj.x` at all serialize identically. The guard's "never assign" side is +// still real (it controls whether a *present* value gets omitted). function buildCanonicalTargetAnnotationObject( evidence: TargetAnnotationV1, ): Record { @@ -161,6 +176,12 @@ export type TargetAnnotationLineParseResult = * `target-vN` comment is an ordinary comment to a v1 reader." Any other `#` * line (including one that merely mentions the tag inside prose) is `none`. */ +// Mutation-lane note: the `!trimmed.startsWith('#')` early return is provably +// redundant — `TARGET_ANNOTATION_LINE_RE` itself is anchored on a leading +// `#`, so any non-`#` line fails the regex too and reaches the same +// `{ kind: 'none' }` via the next check. Likewise `.trim()` on the captured +// payload is redundant: `JSON.parse` already tolerates surrounding +// whitespace, so trimming first never changes the parse outcome. export function parseTargetAnnotationCommentLine(rawLine: string): TargetAnnotationLineParseResult { const trimmed = rawLine.trim(); if (!trimmed.startsWith('#')) return { kind: 'none' }; @@ -288,6 +309,10 @@ function parseAncestryEntry(entry: unknown, index: number): TargetAncestryEntry return { role, ...(label !== undefined ? { label } : {}) }; } +// Mutation-lane note: `typeof value !== 'number'` is provably redundant here +// — `Number.isSafeInteger` (like `Number.isFinite` below) never throws and +// returns `false` for any non-number input, so whenever the typeof clause is +// true the isSafeInteger clause is independently true too. function parseNonNegativeIntField(value: unknown, field: string, fallback: number): number { if (value === undefined) return fallback; if (typeof value !== 'number' || !Number.isSafeInteger(value) || value < 0) { @@ -325,6 +350,12 @@ function parseRectField(value: unknown): TargetRect | undefined { return { x, y, width, height }; } +// Mutation-lane note: `typeof value !== 'number'` is provably redundant — +// `Number.isFinite` never throws/coerces and returns `false` for any +// non-number input, so the typeof clause never fires without the isFinite +// clause also firing. It cannot be observed differently either: this is only +// ever called with a `JSON.parse`-produced value, and JSON has no NaN/ +// Infinity token, so a real (non-finite) number can never reach here. function parseFiniteNumberField(value: unknown, field: string): number { if (typeof value !== 'number' || !Number.isFinite(value)) { throw new AppError('INVALID_ARGS', `target-v1 "${field}" must be a finite number.`); diff --git a/scripts/mutation/modules.ts b/scripts/mutation/modules.ts index 0c36a6e00b..bd1db24642 100644 --- a/scripts/mutation/modules.ts +++ b/scripts/mutation/modules.ts @@ -16,7 +16,9 @@ export type ModuleId = | 'daemon-ref-frame' | 'interaction-settle' | 'scroll-edge-state' - | 'selectors'; + | 'selectors' + | 'target-annotation-serde' + | 'snapshot-occlusion'; export type KernelModule = { readonly id: ModuleId; @@ -72,6 +74,18 @@ export const KERNEL_MODULES: readonly KernelModule[] = [ // minutes in one job — past the acceptance budget and its own timeout. shards: 4, }, + { + id: 'target-annotation-serde', + label: 'Target-annotation comment-line codec (ADR 0012 decision 3)', + mutate: ['packages/ad-script/src/internal/target-annotation-serde.ts'], + owns: ['packages/ad-script/src/internal/target-annotation-serde.ts'], + }, + { + id: 'snapshot-occlusion', + label: 'Snapshot occlusion (covered/not-covered) decisions', + mutate: ['src/snapshot/snapshot-occlusion.ts'], + owns: ['src/snapshot/snapshot-occlusion.ts'], + }, ]; export const ALL_MODULE_IDS: readonly ModuleId[] = KERNEL_MODULES.map((module) => module.id); @@ -125,6 +139,19 @@ export function normalizePath(filePath: string): string { return filePath.replaceAll('\\', '/').replace(/^\.\//, ''); } +/** + * Where the mutation lane can ever find a kernel's owning test: root `src/` or + * a workspace package's `src/` — the same two roots `unit-core`'s `include` + * (`vitest.config.ts`) draws tests from. A kernel whose test lives outside + * both (e.g. `scripts/__tests__`) is unreachable by construction, not silently + * dropped. + */ +const KERNEL_TEST_FILE_RE = /^(?:src\/|packages\/[^/]+\/src\/).*\.test\.ts$/; + +export function isKernelTestFile(filePath: string): boolean { + return KERNEL_TEST_FILE_RE.test(normalizePath(filePath)); +} + /** * Which kernel module owns a repository-relative *source* path, if any. * diff --git a/scripts/mutation/ownership.ts b/scripts/mutation/ownership.ts index 1045e0d315..a4c99b1c48 100644 --- a/scripts/mutation/ownership.ts +++ b/scripts/mutation/ownership.ts @@ -21,10 +21,11 @@ import fs from 'node:fs'; import path from 'node:path'; -import { workspaceSpecifierTargets } from '../layering/package-boundaries.ts'; +import { readWorkspacePackages, workspaceSpecifierTargets } from '../layering/package-boundaries.ts'; import { walkFiles } from '../lib/walk-files.ts'; import { affectedModules, + isKernelTestFile, KERNEL_MODULES, normalizePath, type ModuleId, @@ -34,8 +35,7 @@ import { expandMutateFiles } from './test-scope.ts'; /** Test files the mutation lane can attribute to a kernel at all. */ export function isTestFile(filePath: string): boolean { - const normalized = normalizePath(filePath); - return normalized.startsWith('src/') && normalized.endsWith('.test.ts'); + return isKernelTestFile(filePath); } /** @@ -147,13 +147,24 @@ export function derivedAffectedModules( return KERNEL_MODULES.filter((module) => ids.has(module.id)).map((module) => module.id); } -/** Every test file in the repository, per module that owns it — one graph walk. */ +/** + * Every test file in the repository, per module that owns it — one graph walk + * over root `src/` plus every workspace package's `src/`, so a kernel tested + * only from inside its own package (`target-annotation-serde`) is not + * silently unownable. + */ export function ownedTestFiles(repoRoot: string): Map { const deriver = ownershipDeriver(repoRoot); const owned = new Map(KERNEL_MODULES.map((module) => [module.id, []])); - for (const file of walkFiles(path.join(repoRoot, 'src'), (file) => file.endsWith('.test.ts'))) { - const relative = normalizePath(path.relative(repoRoot, file)); - for (const id of deriver.ownersOf(relative)) owned.get(id)!.push(relative); + const testRoots = [ + path.join(repoRoot, 'src'), + ...readWorkspacePackages(repoRoot).map((pkg) => path.join(repoRoot, pkg.dir, 'src')), + ]; + for (const root of testRoots) { + for (const file of walkFiles(root, (file) => file.endsWith('.test.ts'))) { + const relative = normalizePath(path.relative(repoRoot, file)); + for (const id of deriver.ownersOf(relative)) owned.get(id)!.push(relative); + } } for (const files of owned.values()) files.sort(); return owned; diff --git a/scripts/mutation/test-scope.ts b/scripts/mutation/test-scope.ts index c70177aa77..09936363f9 100644 --- a/scripts/mutation/test-scope.ts +++ b/scripts/mutation/test-scope.ts @@ -16,9 +16,10 @@ // the in-process CLI-capture tests (`process.chdir` throws in a worker // thread) and the `node:worker_threads` PNG pipeline tests (a worker inside // a worker raises uncaught MessagePort errors that kill the runner); -// - anything outside `src/`: the unit suite also hosts the help-conformance -// gates from `scripts/__tests__`, which assert over the repo's own registries -// rather than over any decision kernel and own their CI job. +// - anything outside `src/` or a workspace package's `src/`: the unit suite +// also hosts the help-conformance gates from `scripts/__tests__`, which +// assert over the repo's own registries rather than any decision kernel +// and own their CI job. // // Nothing here weakens the ratchet: a mutant only an excluded test could kill // shows up as a survivor — visible work, never a silent pass. @@ -26,19 +27,33 @@ import fs from 'node:fs'; import path from 'node:path'; import { runCmdSync } from '../../src/utils/exec.ts'; +import { readWorkspacePackages } from '../layering/package-boundaries.ts'; import { walkFiles } from '../lib/walk-files.ts'; -import { normalizePath } from './modules.ts'; +import { isKernelTestFile, normalizePath } from './modules.ts'; /** Env var carrying the resolved scope file to `vitest.mutation.config.ts`. */ export const TEST_SCOPE_ENV = 'AGENT_DEVICE_MUTATION_TEST_FILES'; const CLI_CAPTURE_HARNESS = 'src/__tests__/cli-capture.ts'; +/** + * Every directory the mutation lane can find a test file under: root `src/` + * plus each workspace package's `src/` (`unit-core`'s own `include` list in + * `vitest.config.ts`). A kernel living in `packages/*` — `kernel-errors` + * reaches its tests only indirectly, but `target-annotation-serde` is tested + * directly from inside its package — must not lose its owning tests to a + * root-only walk. + */ +function testFileRoots(repoRoot: string): string[] { + return [ + path.join(repoRoot, 'src'), + ...readWorkspacePackages(repoRoot).map((pkg) => path.join(repoRoot, pkg.dir, 'src')), + ]; +} + /** Source modules that own a `node:worker_threads` worker. */ function workerThreadModules(repoRoot: string): string[] { - return walkFiles( - path.join(repoRoot, 'src'), - (file) => file.endsWith('.ts') && !file.endsWith('.test.ts'), - ) + return testFileRoots(repoRoot) + .flatMap((root) => walkFiles(root, (file) => file.endsWith('.ts') && !file.endsWith('.test.ts'))) .filter((file) => fs.readFileSync(file, 'utf8').includes('node:worker_threads')) .map((file) => path.basename(file, '.ts')); } @@ -51,7 +66,8 @@ function workerThreadModules(repoRoot: string): string[] { export function threadHostileTestFiles(repoRoot: string): string[] { const modules = [path.basename(CLI_CAPTURE_HARNESS, '.ts'), ...workerThreadModules(repoRoot)]; const importsHostileModule = new RegExp(`from '[^']*/(${modules.join('|')})(\\.ts)?'`); - return walkFiles(path.join(repoRoot, 'src'), (file) => file.endsWith('.test.ts')) + return testFileRoots(repoRoot) + .flatMap((root) => walkFiles(root, (file) => file.endsWith('.test.ts'))) .filter((file) => importsHostileModule.test(fs.readFileSync(file, 'utf8'))) .map((file) => normalizePath(path.relative(repoRoot, file))) .sort(); @@ -116,7 +132,7 @@ export function relatedTestFiles( ...new Set( (report.testResults ?? []) .map((result) => normalizePath(path.relative(repoRoot, result.name))) - .filter((file) => file.startsWith('src/') && !excludedSet.has(file)), + .filter((file) => isKernelTestFile(file) && !excludedSet.has(file)), ), ].sort(); } diff --git a/scripts/mutation/workflow.test.ts b/scripts/mutation/workflow.test.ts index 28322a9417..c5f59523f7 100644 --- a/scripts/mutation/workflow.test.ts +++ b/scripts/mutation/workflow.test.ts @@ -72,13 +72,18 @@ test('every kernel path a PR can touch selects the affected mutation job', () => assert.ok(selected, `no path filter selects ${owned} (module ${module.id})`); } } - // Ownership is derived, so any test in src/ can own a kernel; the filter must - // let all of them through and leave the decision to the `select` job. A - // narrower filter is exactly the omission the derivation exists to prevent. + // Ownership is derived, so any test in src/ or a workspace package's src/ + // can own a kernel; the filter must let all of them through and leave the + // decision to the `select` job. A narrower filter is exactly the omission + // the derivation exists to prevent. assert.ok( paths.includes('src/**/*.test.ts'), 'the PR lane must trigger on every src test, since test ownership is derived', ); + assert.ok( + paths.includes('packages/*/src/**/*.test.ts'), + 'the PR lane must trigger on every packages/*/src test too — target-annotation-serde is owned by one', + ); assert.match(workflow('mutation-affected.yml'), /mutation:affected --list-affected/); // The lane's own sources fail open into it too: a ratchet or baseline edit must // prove itself against real mutants, not against a stale report. diff --git a/src/snapshot/__tests__/snapshot-occlusion.test.ts b/src/snapshot/__tests__/snapshot-occlusion.test.ts index d8d54ec9a1..9fa5462759 100644 --- a/src/snapshot/__tests__/snapshot-occlusion.test.ts +++ b/src/snapshot/__tests__/snapshot-occlusion.test.ts @@ -221,3 +221,413 @@ test('cover decisions ignore annotations even through a mutation-sensitive predi assert.equal(annotated.find((n) => n.index === 10)?.interactionBlocked, 'covered'); assert.equal(annotated.find((n) => n.index === 11)?.interactionBlocked, 'covered'); }); + +test('an empty or single-node snapshot is returned unchanged, by reference', () => { + const empty: RawSnapshotNode[] = []; + assert.equal(annotateCoveredSnapshotNodes(empty), empty); + + const solo: RawSnapshotNode[] = [ + { + index: 0, + type: 'button', + role: 'button', + hittable: true, + label: 'Save', + rect: { x: 0, y: 0, width: 50, height: 20 }, + }, + ]; + assert.equal(annotateCoveredSnapshotNodes(solo), solo); +}); + +test('when nothing is covered, the exact input array is returned (no defensive copy)', () => { + // Two nodes (not one) so this actually reaches the "nothing in + // coveredPositions" early return, rather than the separate nodes.length < 2 + // early return above it. + const nodes: RawSnapshotNode[] = [ + { + index: 0, + type: 'button', + role: 'button', + hittable: true, + label: 'Save', + rect: { x: 0, y: 0, width: 50, height: 20 }, + }, + { + index: 1, + type: 'button', + role: 'button', + hittable: true, + label: 'Cancel', + rect: { x: 500, y: 500, width: 50, height: 20 }, + }, + ]; + assert.equal(annotateCoveredSnapshotNodes(nodes), nodes); +}); + +test('a node cannot be marked covered by its own descendant', () => { + // Child C renders on top of (and geometrically covers) its own parent P — an + // ordinary "content overlaps container" shape, not real occluding chrome. The + // parent/child relation must disqualify C as a cover for P regardless of + // z-order or rect overlap. + const parent: RawSnapshotNode = { + index: 0, + type: 'container', + role: 'group', + label: 'Card', + hittable: true, + rect: { x: 10, y: 10, width: 100, height: 100 }, + }; + const child: RawSnapshotNode = { + index: 1, + parentIndex: 0, + type: 'dialog', + role: 'dialog', + rect: { x: 0, y: 0, width: 200, height: 200 }, + }; + + const annotated = annotateCoveredSnapshotNodes([parent, child]); + + assert.equal(annotated[0]?.interactionBlocked, undefined); +}); + +test('a node cannot be marked covered by its own ancestor either, even when the ancestor is listed later', () => { + // The relatedness check is symmetric: it must also catch the reverse + // direction (candidate is target's ancestor), which the parent/descendant + // case above cannot exercise on its own since only later-listed nodes are + // ever considered as covers. Two levels deep (target -> intermediate -> + // grandparent) so the walk must climb past the first parent, not just check + // it directly. + const target: RawSnapshotNode = { + index: 0, + parentIndex: 1, + type: 'button', + role: 'button', + hittable: true, + label: 'Pay', + rect: { x: 10, y: 10, width: 80, height: 40 }, + }; + const intermediate: RawSnapshotNode = { + index: 1, + parentIndex: 2, + type: 'group', + role: 'group', + rect: { x: 0, y: 0, width: 300, height: 300 }, + }; + const grandparent: RawSnapshotNode = { + index: 2, + type: 'dialog', + role: 'dialog', + rect: { x: 0, y: 0, width: 400, height: 400 }, + }; + + const annotated = annotateCoveredSnapshotNodes([target, intermediate, grandparent]); + + assert.equal(annotated[0]?.interactionBlocked, undefined); +}); + +test('a later, non-overlay-classified node never covers a target, however it overlaps geometrically', () => { + // B is a plain button, not floating UI chrome (no OVERLAY_KIND_FRAGMENTS + // match, no isAdditionalOverlayNode match) — only genuine overlay-classified + // nodes may ever act as covers. B's rect is deliberately NOT + // approximately-equal to A's (a much bigger box that still contains A's + // center point) so the separate rect-equality guard cannot also explain an + // "uncovered" result — this test isolates the overlay-classification check. + const a: RawSnapshotNode = { + index: 0, + type: 'button', + role: 'button', + hittable: true, + label: 'Under', + rect: { x: 10, y: 10, width: 80, height: 40 }, + }; + const b: RawSnapshotNode = { + index: 1, + type: 'button', + role: 'button', + hittable: true, + label: 'Over', + rect: { x: 0, y: 0, width: 400, height: 400 }, + }; + + const annotated = annotateCoveredSnapshotNodes([a, b]); + + assert.equal(annotated[0]?.interactionBlocked, undefined); +}); + +test('an overlay candidate with a rect approximately equal to the target is never treated as covering it', () => { + // Same rect, both otherwise legitimate: D is genuinely overlay-classified + // and unrelated to T, so only the rect-equality guard can explain this + // staying uncovered — isolates that check from the overlay-classification + // check above. + const target: RawSnapshotNode = { + index: 0, + type: 'button', + role: 'button', + hittable: true, + label: 'Save', + rect: { x: 10, y: 10, width: 80, height: 40 }, + }; + const sameRectDialog: RawSnapshotNode = { + index: 1, + type: 'dialog', + role: 'dialog', + rect: { x: 10, y: 10, width: 80, height: 40 }, + }; + + const annotated = annotateCoveredSnapshotNodes([target, sameRectDialog]); + + assert.equal(annotated[0]?.interactionBlocked, undefined); +}); + +test('a node classified through the caller predicate is excluded as its own overlay root when a renderable ancestor is two levels up', () => { + // Root R and leaf L both match the caller predicate; L's immediate parent M + // does not. Without correctly walking past M to find R, L would wrongly + // count as an independent overlay root alongside R — this test isolates + // hasRenderableAdditionalOverlayAncestor's multi-level climb specifically + // (the single-level case is already covered by the existing + // "ignore annotations... ancestor walk" test above). + const target: RawSnapshotNode = { + index: 0, + type: 'button', + role: 'button', + hittable: true, + label: 'Save', + rect: { x: 500, y: 500, width: 80, height: 40 }, + }; + const root: RawSnapshotNode = { + index: 1, + identifier: 'overlay-root', + type: 'group', + role: 'group', + rect: { x: 0, y: 0, width: 20, height: 20 }, + }; + const middle: RawSnapshotNode = { + index: 2, + parentIndex: 1, + type: 'group', + role: 'group', + rect: { x: 0, y: 0, width: 20, height: 20 }, + }; + const leaf: RawSnapshotNode = { + index: 3, + parentIndex: 2, + identifier: 'overlay-root', + type: 'group', + role: 'group', + // Only the leaf's rect covers the target — if the leaf were wrongly kept + // as an independent overlay root (ancestor climb stopped one level too + // early at M), the target would be covered; if correctly excluded in + // favor of the root-most classification, it stays uncovered. + rect: { x: 480, y: 480, width: 200, height: 200 }, + }; + + const annotated = annotateCoveredSnapshotNodes([target, root, middle, leaf], { + isAdditionalOverlayNode: (node) => node.identifier === 'overlay-root', + }); + + assert.equal(annotated[0]?.interactionBlocked, undefined); +}); + +test('an overlay-kind node with no positive-area rect never covers anything, even a target dead center on its degenerate line', () => { + // The target's center sits exactly on x=50, the zero-width dialog's only + // x-coordinate — a generic "does the rect overlap" check could accidentally + // treat this degenerate rect as containing that single point. Positioning + // the target precisely there (rather than somewhere the rect trivially + // misses) is what makes this test isolate the width>0 requirement, not + // just "an empty rect happens not to overlap". + const target: RawSnapshotNode = { + index: 0, + type: 'button', + role: 'button', + hittable: true, + label: 'Save', + rect: { x: 30, y: 10, width: 40, height: 20 }, // center = (50, 20) + }; + const zeroWidthDialog: RawSnapshotNode = { + index: 1, + type: 'dialog', + role: 'dialog', + rect: { x: 50, y: 0, width: 0, height: 200 }, + }; + + const annotated = annotateCoveredSnapshotNodes([target, zeroWidthDialog]); + + assert.equal(annotated[0]?.interactionBlocked, undefined); +}); + +test('an overlay-kind node with zero height never covers anything, even a target dead center on its degenerate line', () => { + const target: RawSnapshotNode = { + index: 0, + type: 'button', + role: 'button', + hittable: true, + label: 'Save', + rect: { x: 10, y: 30, width: 40, height: 20 }, // center = (30, 40) + }; + const zeroHeightDialog: RawSnapshotNode = { + index: 1, + type: 'dialog', + role: 'dialog', + rect: { x: 0, y: 40, width: 200, height: 0 }, + }; + + const annotated = annotateCoveredSnapshotNodes([target, zeroHeightDialog]); + + assert.equal(annotated[0]?.interactionBlocked, undefined); +}); + +test('a node classified as viewport root never covers, even if its kind text otherwise matches overlay fragments', () => { + // type "application" + role "dialog" would match the 'dialog' overlay + // fragment, but a window/application-level node is excluded outright — it + // is the screen itself, never floating chrome on top of it. + const target: RawSnapshotNode = { + index: 0, + type: 'button', + role: 'button', + hittable: true, + label: 'Save', + rect: { x: 10, y: 10, width: 80, height: 40 }, + }; + const applicationRoot: RawSnapshotNode = { + index: 1, + type: 'application', + role: 'dialog', + rect: { x: 0, y: 0, width: 400, height: 800 }, + }; + + const annotated = annotateCoveredSnapshotNodes([target, applicationRoot]); + + assert.equal(annotated[0]?.interactionBlocked, undefined); +}); + +test('the kind fields join with a separator, so adjacent fragments never accidentally concatenate into a match', () => { + // type "tab" + role "bar" must read as "tab bar" (no match for the + // 'tabbar' overlay fragment) — never "tabbar" via an unseparated join, + // which would misclassify this as floating chrome it is not. + const target: RawSnapshotNode = { + index: 0, + type: 'button', + role: 'button', + hittable: true, + label: 'Save', + rect: { x: 10, y: 10, width: 80, height: 40 }, + }; + const coincidental: RawSnapshotNode = { + index: 1, + type: 'tab', + role: 'bar', + rect: { x: 0, y: 0, width: 400, height: 800 }, + }; + + const annotated = annotateCoveredSnapshotNodes([target, coincidental]); + + assert.equal(annotated[0]?.interactionBlocked, undefined); +}); + +test('a plain rect with no hittable/label/value/identifier is not a touch candidate and is never annotated', () => { + const inert: RawSnapshotNode = { + index: 0, + type: 'group', + role: 'group', + rect: { x: 10, y: 10, width: 80, height: 40 }, + }; + const dialog: RawSnapshotNode = { + index: 1, + type: 'dialog', + role: 'dialog', + rect: { x: 0, y: 0, width: 400, height: 800 }, + }; + + const annotated = annotateCoveredSnapshotNodes([inert, dialog]); + + assert.equal(annotated[0]?.interactionBlocked, undefined); + assert.equal(annotated[0]?.hittable, undefined); +}); + +test('a node qualifies as a touch candidate through value or identifier alone, without a label', () => { + const byValue: RawSnapshotNode = { + index: 0, + type: 'textfield', + role: 'textfield', + value: 'user@example.com', + rect: { x: 10, y: 10, width: 80, height: 40 }, + }; + const byIdentifier: RawSnapshotNode = { + index: 1, + type: 'group', + role: 'group', + identifier: 'save-button', + rect: { x: 10, y: 60, width: 80, height: 40 }, + }; + const dialog: RawSnapshotNode = { + index: 2, + type: 'dialog', + role: 'dialog', + rect: { x: 0, y: 0, width: 400, height: 800 }, + }; + + const annotated = annotateCoveredSnapshotNodes([byValue, byIdentifier, dialog]); + + assert.equal(annotated[0]?.interactionBlocked, 'covered'); + assert.equal(annotated[1]?.interactionBlocked, 'covered'); +}); + +test('a whitespace-only label, value, or identifier does not qualify a node as a touch candidate', () => { + // Each field must be independently trimmed before the emptiness check, not + // just present-and-truthy — a lone whitespace string is truthy in JS but + // carries no real content. + const whitespaceLabel: RawSnapshotNode = { + index: 0, + type: 'group', + role: 'group', + label: ' ', + rect: { x: 0, y: 0, width: 40, height: 40 }, + }; + const whitespaceValue: RawSnapshotNode = { + index: 1, + type: 'group', + role: 'group', + value: ' ', + rect: { x: 50, y: 0, width: 40, height: 40 }, + }; + const whitespaceIdentifier: RawSnapshotNode = { + index: 2, + type: 'group', + role: 'group', + identifier: ' ', + rect: { x: 100, y: 0, width: 40, height: 40 }, + }; + const dialog: RawSnapshotNode = { + index: 3, + type: 'dialog', + role: 'dialog', + rect: { x: 0, y: 0, width: 400, height: 800 }, + }; + + const annotated = annotateCoveredSnapshotNodes( + [whitespaceLabel, whitespaceValue, whitespaceIdentifier, dialog], + ); + + assert.equal(annotated[0]?.interactionBlocked, undefined); + assert.equal(annotated[1]?.interactionBlocked, undefined); + assert.equal(annotated[2]?.interactionBlocked, undefined); +}); + +test('a node qualifies as a touch candidate through a semantic role alone, without hittable/label/value/identifier', () => { + const semantic: RawSnapshotNode = { + index: 0, + type: 'checkbox', + role: 'checkbox', + rect: { x: 10, y: 10, width: 30, height: 30 }, + }; + const dialog: RawSnapshotNode = { + index: 1, + type: 'dialog', + role: 'dialog', + rect: { x: 0, y: 0, width: 400, height: 800 }, + }; + + const annotated = annotateCoveredSnapshotNodes([semantic, dialog]); + + assert.equal(annotated[0]?.interactionBlocked, 'covered'); +}); diff --git a/src/snapshot/snapshot-occlusion.ts b/src/snapshot/snapshot-occlusion.ts index 032945c0b3..b06a065aa0 100644 --- a/src/snapshot/snapshot-occlusion.ts +++ b/src/snapshot/snapshot-occlusion.ts @@ -58,6 +58,11 @@ export type SnapshotOcclusionOptions = { isAdditionalOverlayNode?: (node: RawSnapshotNode) => boolean; }; +// Mutation-lane note: the `nodes.length < 2` early return below is provably +// redundant — with 0 or 1 nodes, `coveredPositions` can never be non-empty, +// so the *other* early return a few lines down (`coveredPositions.length === +// 0`) already returns `nodes` unchanged by the same reference. This one is a +// pure allocation-avoiding fast path, not a distinct behavior. export function annotateCoveredSnapshotNodes( nodes: RawSnapshotNode[], options: SnapshotOcclusionOptions = {}, @@ -68,6 +73,11 @@ export function annotateCoveredSnapshotNodes( const scan: OcclusionScan = { nodes, byIndex, + // Mutation-lane note: replacing the `[]` (non-overlay) branch with a + // non-empty placeholder would only add extra, non-numeric entries to + // this array; every consumer treats it as a plain array of positions to + // compare/index with, so a stray non-numeric entry is inert (fails the + // comparison, indexes to `undefined`) rather than observable. overlayPositions: nodes.flatMap((node, position) => isOverlayLikeNode(node, byIndex, options) ? [position] : [], ), @@ -99,6 +109,19 @@ export function isSnapshotNodeInteractionBlocked( return node.interactionBlocked !== undefined; } +// Mutation-lane note: several guards across this call chain are provably +// redundant, not undertested: +// - `findCoveringNode`'s `!targetRect` — every caller (the outer loop below +// and `visibleCoverRect`'s recursive call) only ever passes a node whose +// rect was already confirmed positive (via `isCandidateTouchNode` or the +// `candidateRect` check just before the recursive call). +// - `canCoverPoint`'s and `visibleCoverRect`'s `!candidate` — `position` +// always comes from `scan.overlayPositions`, itself built by mapping +// over `scan.nodes`, so it is always a valid index into that same array. +// - `visibleCoverRect`'s `!isOverlayLikeNode(candidate, ...)` — a position +// only lands in `overlayPositions` because this exact predicate, over +// the exact same pristine (node, byIndex, options), already returned +// true when the array was built. function findCoveringNode( scan: OcclusionScan, targetPosition: number, @@ -119,6 +142,12 @@ function findCoveringNode( if (!targetRect) return finishFindCoveringNode(scan, targetPosition, null); const center = centerOfRect(targetRect); + // Mutation-lane note: relaxing `<=` to `<` here would only change behavior + // if `position === targetPosition` were reachable — a node covering + // itself. That case is already excluded one line below regardless: a + // self-candidate has `candidateRect === targetRect` (same node, same + // object), so `areRectsApproximatelyEqual` in `visibleCoverRect` always + // excludes it. for (const position of scan.overlayPositions) { if (position <= targetPosition) continue; const candidate = scan.nodes[position]; @@ -169,6 +198,9 @@ function visibleCoverRect( return candidateRect; } +// Mutation-lane note: the `!positiveRect` guard below is provably redundant +// — a rect-less "candidate" still flows into `findCoveringNode`, whose own +// `!targetRect` check (see the note above it) rejects it the same way. function isCandidateTouchNode(node: RawSnapshotNode): boolean { if (!positiveRect(node.rect)) return false; if (node.hittable === true) return true; @@ -176,6 +208,11 @@ function isCandidateTouchNode(node: RawSnapshotNode): boolean { return Boolean(node.label?.trim() || node.value?.trim() || node.identifier?.trim()); } +// Mutation-lane note: this function's own `!positiveRect` guard is likewise +// redundant — a rect-less node that slipped into `overlayPositions` would +// still be excluded downstream by `visibleCoverRect`'s `!candidateRect` +// check, which re-derives the same `positiveRect` over the same pristine +// node. function isOverlayLikeNode( node: RawSnapshotNode, byIndex: Map, @@ -200,6 +237,12 @@ function isAdditionalOverlayRootNode( return !hasRenderableAdditionalOverlayAncestor(node, byIndex, options); } +// Mutation-lane note: `typeof x.parentIndex === 'number' ? byIndex.get(...) : +// undefined` is provably redundant here (and in the structurally identical +// walk in `isSnapshotAncestor` below) — `Map.get` on a key that was never +// set (including `undefined`) already returns `undefined`, the exact value +// the ternary's else-branch produces, so skipping the typeof check changes +// nothing observable. function hasRenderableAdditionalOverlayAncestor( node: RawSnapshotNode, byIndex: Map, @@ -216,6 +259,11 @@ function hasRenderableAdditionalOverlayAncestor( return false; } +// Mutation-lane note: the `?.` here is provably redundant — this only runs +// once `isAdditionalOverlayRootNode` already confirmed +// `options.isAdditionalOverlayNode` is a real function (its own, unguarded +// `!== true` check would otherwise have returned early), and `options` is +// never replaced mid-walk. function isRenderableAdditionalOverlayNode( node: RawSnapshotNode, options: SnapshotOcclusionOptions, @@ -239,6 +287,12 @@ function nodeKindIncludesAny( return fragments.some((fragment) => normalized.includes(fragment)); } +// Mutation-lane note: the `?? ''` fallback's exact replacement text is not +// observable through this module's public API — every caller only checks +// substring membership against a fixed, known fragment list, and no +// plausible filler text coincides with any of them, so no test can +// distinguish `''` from another non-matching filler here without reaching +// into this private function directly. function normalizeNodeKind(node: Pick): string { return [node.type, node.role, node.subrole].map((value) => normalizeType(value ?? '')).join(' '); } diff --git a/stryker.config.json b/stryker.config.json index 8513737eed..8b6c71dd62 100644 --- a/stryker.config.json +++ b/stryker.config.json @@ -36,6 +36,8 @@ "src/utils/scroll-edge-state.ts", "src/selectors/**/*.ts", "!src/selectors/**/*.test.ts", - "!src/selectors/__tests__/**" + "!src/selectors/__tests__/**", + "packages/ad-script/src/internal/target-annotation-serde.ts", + "src/snapshot/snapshot-occlusion.ts" ] } diff --git a/vitest.mutation.config.ts b/vitest.mutation.config.ts index 9e464e91d9..9541e76a4e 100644 --- a/vitest.mutation.config.ts +++ b/vitest.mutation.config.ts @@ -37,7 +37,7 @@ export default defineConfig({ alias: workspaceAliases, }, test: { - include: scope ?? ['src/**/*.test.ts'], + include: scope ?? ['src/**/*.test.ts', 'packages/*/src/**/*.test.ts'], exclude: [...SUBPROCESS_STUB_TESTS, ...threadHostileTestFiles(repoRoot), '**/node_modules/**'], setupFiles: ['src/__tests__/hermetic-env-setup.ts', 'src/__tests__/process-memo-setup.ts'], }, From 4b67b3d23d348af7eda0dc9f66512769380fcf13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 2 Aug 2026 10:56:55 +0200 Subject: [PATCH 2/2] style: run the formatter over the four files the gate flagged --- .../internal/__tests__/target-annotation-serde.test.ts | 4 +++- scripts/mutation/ownership.ts | 5 ++++- scripts/mutation/test-scope.ts | 4 +++- src/snapshot/__tests__/snapshot-occlusion.test.ts | 9 ++++++--- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/ad-script/src/internal/__tests__/target-annotation-serde.test.ts b/packages/ad-script/src/internal/__tests__/target-annotation-serde.test.ts index d0a0b52b14..6d52f15fee 100644 --- a/packages/ad-script/src/internal/__tests__/target-annotation-serde.test.ts +++ b/packages/ad-script/src/internal/__tests__/target-annotation-serde.test.ts @@ -142,7 +142,9 @@ test('scrollRegion.label serializes and round trips like every other label field scrollRegion: { role: 'scrollview', id: 'editor-scroll', label: 'Body' }, }); const json = serializeTargetAnnotationV1(evidence); - assert.ok(json.includes('"scrollRegion":{"role":"scrollview","id":"editor-scroll","label":"Body"}')); + assert.ok( + json.includes('"scrollRegion":{"role":"scrollview","id":"editor-scroll","label":"Body"}'), + ); const parsed = parseTargetAnnotationV1Payload(json); assert.equal(parsed.scrollRegion?.label, 'Body'); }); diff --git a/scripts/mutation/ownership.ts b/scripts/mutation/ownership.ts index a4c99b1c48..ce5baba5f0 100644 --- a/scripts/mutation/ownership.ts +++ b/scripts/mutation/ownership.ts @@ -21,7 +21,10 @@ import fs from 'node:fs'; import path from 'node:path'; -import { readWorkspacePackages, workspaceSpecifierTargets } from '../layering/package-boundaries.ts'; +import { + readWorkspacePackages, + workspaceSpecifierTargets, +} from '../layering/package-boundaries.ts'; import { walkFiles } from '../lib/walk-files.ts'; import { affectedModules, diff --git a/scripts/mutation/test-scope.ts b/scripts/mutation/test-scope.ts index 09936363f9..a18e251804 100644 --- a/scripts/mutation/test-scope.ts +++ b/scripts/mutation/test-scope.ts @@ -53,7 +53,9 @@ function testFileRoots(repoRoot: string): string[] { /** Source modules that own a `node:worker_threads` worker. */ function workerThreadModules(repoRoot: string): string[] { return testFileRoots(repoRoot) - .flatMap((root) => walkFiles(root, (file) => file.endsWith('.ts') && !file.endsWith('.test.ts'))) + .flatMap((root) => + walkFiles(root, (file) => file.endsWith('.ts') && !file.endsWith('.test.ts')), + ) .filter((file) => fs.readFileSync(file, 'utf8').includes('node:worker_threads')) .map((file) => path.basename(file, '.ts')); } diff --git a/src/snapshot/__tests__/snapshot-occlusion.test.ts b/src/snapshot/__tests__/snapshot-occlusion.test.ts index 9fa5462759..86edd0d46d 100644 --- a/src/snapshot/__tests__/snapshot-occlusion.test.ts +++ b/src/snapshot/__tests__/snapshot-occlusion.test.ts @@ -604,9 +604,12 @@ test('a whitespace-only label, value, or identifier does not qualify a node as a rect: { x: 0, y: 0, width: 400, height: 800 }, }; - const annotated = annotateCoveredSnapshotNodes( - [whitespaceLabel, whitespaceValue, whitespaceIdentifier, dialog], - ); + const annotated = annotateCoveredSnapshotNodes([ + whitespaceLabel, + whitespaceValue, + whitespaceIdentifier, + dialog, + ]); assert.equal(annotated[0]?.interactionBlocked, undefined); assert.equal(annotated[1]?.interactionBlocked, undefined);