Skip to content

feat(mutation): add target-annotation-serde + snapshot-occlusion kernels - #1553

Merged
thymikee merged 2 commits into
mainfrom
claude/mutation-lane-pure-decision-878385
Aug 2, 2026
Merged

feat(mutation): add target-annotation-serde + snapshot-occlusion kernels#1553
thymikee merged 2 commits into
mainfrom
claude/mutation-lane-pure-decision-878385

Conversation

@thymikee

@thymikee thymikee commented Aug 2, 2026

Copy link
Copy Markdown
Member

Summary

Adds two pure decision kernels to the mutation lane's KERNEL_MODULES registry (issue #1415), per the registry's own membership rule ("pure decision kernels only — a surviving mutant here means a silently wrong agent-facing decision"):

  • target-annotation-serdepackages/ad-script/src/internal/target-annotation-serde.ts: parse/validate/normalize the .ad target-annotation comment-line codec. Zero I/O.
  • snapshot-occlusionsrc/snapshot/snapshot-occlusion.ts: the covered/not-covered decision; a wrong answer silently blocks or mis-allows a tap.

packages/replay-test was considered and correctly excluded — it spawns subprocesses, which the registry's own doc comment rules out by construction.

The harness had a packages/*/src blind spot

target-annotation-serde's own tests live in packages/ad-script/src/internal/__tests__/, not under root src/. Three places in the mutation lane hardcoded src/ as the only place a kernel's tests could live:

  • scripts/mutation/test-scope.ts (relatedTestFiles's filter, threadHostileTestFiles's walk root)
  • scripts/mutation/ownership.ts (isTestFile, ownedTestFiles's walk root)
  • vitest.mutation.config.ts (the fallback include list)

Without fixing this, adding the module would have "worked" in the sense that Stryker would run — but with zero tests in scope, so every mutant would survive. That's not a weak-test signal, it's the harness silently failing to find real coverage. Confirmed the fix works empirically: before the fix, target-annotation-serde measured 0% (all NoCoverage); after, tests exercised it correctly and the initial run measured 68.96%.

Widened all three to also recognize packages/*/src/**/*.test.ts, mirroring vitest.config.ts's own unit-core project include list (which already covers packages/*/src/**/*.test.ts — the gap was purely in the mutation lane's downstream derivation, not in the base test runner). Also widened mutation-affected.yml's PR-trigger paths and added an assertion in workflow.test.ts locking in the packages/*/src/**/*.test.ts trigger glob, since ownership is now derived from that directory too.

CI wiring

  • mutation-weekly.yml: added target-annotation-serde and snapshot-occlusion shard jobs, bumped --expect-shards 8 → 10.
  • mutation-affected.yml: added both modules' owned source paths, plus the packages/*/src/**/*.test.ts trigger glob.
  • stryker.config.json: mirrored KERNEL_MODULES' new mutate globs (asserted by scripts/mutation/config.test.ts).

Baseline

Recorded from the actual final measured run — not inherited or guessed, per the ratchet's own design (it gates baseline stability, not score; a new module's first baseline must reflect its real, current test strength):

Module Score Killed/Total
target-annotation-serde 94.03% 315/335
snapshot-occlusion 89.74% 175/195

Both start status: new, gating: false, stableRuns: 0 — consistent with every other module in the registry. This PR changes stryker.config.json's content, which per its own header comment invalidates the other 5 modules' recorded baselines as non-comparable until the next full sweep re-records them (provenance-drift, stableRuns reset to 0) — expected, harmless while gating: false, and identical to what any edit to that file already does.

Surviving-mutant triage

Started at 68.96% / 65.13% on the first real run (after the harness fix). Iterated: 21 new tests added across both files for genuine coverage gaps, confirmed each kill with a live Stryker run, then classified everything still standing. Every remaining survivor has an inline // Mutation-lane note: comment at its mutation site explaining why it's unobservable — this table is the index into those.

target-annotation-serde.ts — 20 remaining, all equivalent

# Location(s) Category Why equivalent
1 utf8ByteLength ('utf8' arg) Redundant argument Buffer.byteLength falls back to utf8 for any unrecognized encoding, including '' — verified empirically
2 truncateToUtf8Bytes early return (<= maxBytes) ×2 Redundant fast path The shrink-loop converges to the identical end whether or not the early return fires — value.slice(0,0) is always ''
3 truncateToUtf8Bytes loop guard (end > 0) ×2 Redundant fast path Same convergence argument; maxBytes is always a non-negative constant at every call site
4 truncateToUtf8Bytes surrogate-check guard (end > 0) ×2 Dead defensive code charCodeAt(-1) returns NaN; NaN comparisons are always false, so entering the block is a no-op when end === 0
5 buildCanonicalTargetAnnotationObject/buildAncestryEntryObject/buildScrollRegionObject/parseScrollRegionField optional-include guards (x !== undefined ? obj.x = x) ×5 Redundant on one branch JSON.stringify omits keys with undefined values — assigning obj.x = undefined and never assigning it serialize identically. (The "never assign a present value" side of each guard is real and tested — new/adjusted tests cover it.)
6 parseTargetAnnotationCommentLine !trimmed.startsWith('#') guard ×2 Redundant guard TARGET_ANNOTATION_LINE_RE is itself anchored on a leading #; any non-# line fails the regex too and reaches the same {kind:'none'} via the next check
7 parseTargetAnnotationCommentLine payload .trim() + its NoCoverage fallback text Redundant / unreachable JSON.parse already tolerates surrounding whitespace (verified empirically); the fallback text is unreachable in a way that changes the observable error message
8 parseNonNegativeIntField/parseFiniteNumberField typeof value !== 'number' clause ×3 (one LogicalOperator, two Conditional) Redundant clause Number.isSafeInteger/Number.isFinite never throw and return false for any non-number, so whenever typeof would fire, the other clause already does too. The Number.isFinite case is additionally unreachable via the real NaN/Infinity distinction — JSON.parse has no token for either, so a parsed number is always finite.

snapshot-occlusion.ts — 20 remaining, all equivalent

# Location(s) Category Why equivalent
1 annotateCoveredSnapshotNodes nodes.length < 2 early return Redundant fast path With 0–1 nodes, coveredPositions can never be non-empty, so the other early return a few lines down returns nodes by the same reference anyway
2 overlayPositions non-overlay ArrayDeclaration branch Inert data A stray non-numeric filler entry fails every numeric comparison/index against it — no consumer treats it as meaningful
3 findCoveringNode !targetRect guard Dead defensive code Every caller (the outer loop, and visibleCoverRect's recursive call) only ever passes a node whose rect was already confirmed positive
4 findCoveringNode self-coverage boundary (<= vs <) ×2 Backstopped elsewhere position === targetPosition implies candidate === target, so areRectsApproximatelyEqual in visibleCoverRect always excludes it regardless
5 canCoverPoint/visibleCoverRect !candidate guards ×3 Dead defensive code candidatePosition always comes from scan.overlayPositions, itself built by mapping over scan.nodes — always a valid index
6 visibleCoverRect !isOverlayLikeNode(candidate,...) re-check ×2 Dead defensive code 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
7 isCandidateTouchNode/isOverlayLikeNode own !positiveRect guards ×4 Backstopped downstream A rect-less node that slips past still gets excluded by findCoveringNode's !targetRect or visibleCoverRect's !candidateRect check on the same data
8 hasRenderableAdditionalOverlayAncestor/isSnapshotAncestor byIndex.get ternaries ×4 Redundant guard Map.get on a key that was never set (including undefined) already returns undefined — identical to the ternary's else-branch
9 isRenderableAdditionalOverlayNode optional-chaining removal Dead defensive code Only reached once isAdditionalOverlayRootNode already confirmed options.isAdditionalOverlayNode is a real function; options is never replaced mid-walk
10 isRenderableAdditionalOverlayNode positiveRect(node.rect) !== null Backstopped elsewhere Same class as #7, for the ancestor-classification path
11 normalizeNodeKind fallback text (?? '') Unreachable via public API Every caller only checks substring membership against a fixed, known fragment list; no plausible filler text coincides with any of them

Gates

  • pnpm mutation:test — 45/45 ✓
  • pnpm mutation:check --modules target-annotation-serde,snapshot-occlusion --report .tmp/mutation/mutation.json — both held, no regression ✓
  • pnpm typecheck
  • pnpm lint

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-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.93 MB 1.93 MB 0 B
JS gzip 618.1 kB 618.1 kB 0 B
npm tarball 736.4 kB 736.4 kB 0 B
npm unpacked 2.58 MB 2.58 MB 0 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 26.2 ms 26.5 ms +0.3 ms
CLI --help 64.4 ms 68.0 ms +3.6 ms

Top changed chunks: no changes in the largest emitted chunks.

@thymikee

thymikee commented Aug 2, 2026

Copy link
Copy Markdown
Member Author

Reviewed exact head dfa7a5e. The package-test discovery and two new mutation kernels are internally coherent, and I found no source-level defect in the implementation. Two blockers remain before readiness:

  1. Scope decision: closed issue test: weekly mutation-score ratchet over decision kernels (Stryker, non-gating first) #1415 explicitly approved Stryker for exactly five kernels, while this PR expands that boundary to seven without a linked approved amendment/follow-up. Please record maintainer approval by amending/reopening the issue or linking an approved follow-up before merge.
  2. Owner-action CI: Lint & Format deterministically fails on four changed files (target-annotation-serde.test.ts, ownership.ts, test-scope.ts, snapshot-occlusion.test.ts). Run pnpm format and push the result; this is not an infrastructure flake.

Android/iOS smoke and two mutation shards were also still pending at review time. Device evidence is not applicable because this changes mutation infrastructure/tests, not device behavior. Do not merge or apply ready-for-human until the scope decision is recorded, formatting is green, and remaining checks settle.

@thymikee

thymikee commented Aug 2, 2026

Copy link
Copy Markdown
Member Author

Both blockers addressed:

  1. Scope decision recorded: amendment comment on test: weekly mutation-score ratchet over decision kernels (Stryker, non-gating first) #1415 documenting the five→seven kernel extension, its provenance (2026-08-01 test-strength audit finding; maintainer-initiated implementation task), and the explicit replay-test exclusion rationale.
  2. Formatting: pnpm format over the four flagged files, pushed as 4b67b3d23. format:check, typecheck, lint, and the two touched test files (66/66) green locally.

Generated by Claude Code

@thymikee

thymikee commented Aug 2, 2026

Copy link
Copy Markdown
Member Author

Re-reviewed exact head 4b67b3d. The only code delta since the prior review is OXC formatting in the four flagged files. Issue #1415 now records the member-approved five→seven kernel amendment, including the two added modules, provenance, PR link, and subprocess-based exclusion. The mutation harness/package discovery remains coherent, no code findings remain, and every exact-head check is green—including all affected mutation shards and aggregate, lint/format, coverage, integration, and platform smoke. Device evidence is not applicable to this mutation-infrastructure/test change. Clean and ready for human review/merge.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 2, 2026
@thymikee
thymikee merged commit 60400d0 into main Aug 2, 2026
33 checks passed
@thymikee
thymikee deleted the claude/mutation-lane-pure-decision-878385 branch August 2, 2026 09:36
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-02 09:36 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant