test: add scroll-edge-state suite, close mutation gap (#1455)

[thymikee]

Summary

• src/utils/scroll-edge-state.ts (326 LOC of real decision logic) had no dedicated test file anywhere in the repo and scored 28.75% mutation (92 killed / 228 survived of 320 mutants, per mutation-baselines/decision-kernels.json) — the worst of the five tracked decision kernels.
• Adds a behavioral test suite covering every exported function's decisions — captureScrollEdgeState, runScrollEdgePasses, formatScrollEdgeMessage — plus the private container-selection, hidden-edge, and scope logic they exercise.

Test architecture (updated per review, twice)

Split by durable question into six sibling files plus a shared fixtures module (matching the existing daemon-runtime-port*.test.ts sibling-suite convention), each well under the 500-LOC "extract before adding" tripwire:

File	LOC	Tests	Covers
scroll-edge-state-fixtures.ts	85	—	shared builders: windowRoot, scrollNode, capture, scopeFor, scrollSnapshot, captureThrows
scroll-edge-state-container-existence.test.ts	51	4	no scrollable node / zero-area rects → null container
scroll-edge-state-hidden-edge.test.ts	70	6	hasHiddenContentAtEdge decisions, both edges
scroll-edge-state-target-resolution.test.ts	196	9	target.nodeIndex/target.point resolution, containsPoint boundary
scroll-edge-state-broad-selection.test.ts	62	3	no-target selection among multiple scrollables
scroll-edge-state-scope.test.ts	~200	19	buildScrollContainerScope/isUsefulScope/isUniqueScopeValue decisions
scroll-edge-state-pass-orchestration.test.ts	311	18	formatScrollEdgeMessage, retry/error wrapping, runScrollEdgePasses

(The first iteration was a single 1,323-line file; the second split it three ways but left one 562-line file with 20 inline node literals despite the shared fixtures module — both flagged in review. This iteration adds a scrollNode() builder and splits container-selection further into four single-question files.)

Production changes (review-authorized/exposed)

Three changes, each responding to a specific review finding rather than discretionary cleanup:

1. Removed dead signature state. ScrollEdgeState.signature/buildScrollStateSignature/collectSubtreeNodes/hasAncestor/roundSignatureNumber had zero consumers anywhere in the repo (grepped exhaustively — both call sites only ever read .canScroll/.scope). The original suite pinned its exact private string format ("mutation-score gaming" per review) instead of testing an observable decision. Deleted outright rather than left for a future PR to rediscover.
2. Tightened analyzeScrollEdgeState's parameter type. It accepted ... | undefined and normalized with ?? [], but the real contract — both production call sites, and captureScrollEdgeState's own captureNodes return type — always resolves to a real array. The undefined branch was only reachable via a type-unsound test cast (undefined as unknown as SnapshotNode[]). Dropped the | undefined and the cast-driven test.
3. Simplified buildScrollContainerScope. It mapped a missing identifier/label to a placeholder '', which then always failed isUsefulScope's length check — so the placeholder's exact content never mattered functionally, but review correctly found that a synthetic mutated placeholder ('' → some Stryker-chosen string) usually isn't observably equivalent once node.value gets planted into the mix, and the honest fix is to stop synthesizing a fallback at all: [identifier, label].filter(isString).map(trim).find(...) instead of .map(toStringOrEmpty).find(...). Behaviorally identical for every real input, and removes the mutation surface entirely rather than chasing it with a test coupled to Stryker's specific replacement text.

scroll-edge-state.ts: 326 → 289 LOC.

Mutation score

Scoped run: pnpm mutation:run --modules scroll-edge-state

	Score	Killed/Total	Surviving
Before (fresh baseline run, matches mutation-baselines/decision-kernels.json)	28.75%	92/320	228
After	95.14%	274/288	14

This is local evidence, not CI-verified: the mutation-affected CI lane only gates once mutation-baselines/decision-kernels.json's gating flag flips to true (currently false, 0/2 stable weekly runs), so CI does not select this module for this PR. The baseline file itself was not modified — see below.

Remaining 14 survivors — mechanically traced, corrected after a real counter-example

Each survivor was checked against the exact mutated span from the Stryker JSON report. Review previously caught one of these justifications as wrong (the ''-fallback mutant, addressed above by removing the fallback rather than re-justifying it) — the rest:

1. ensureSnapshotNodes's ref assignment ternary (line 156, 6 mutants) — ref is never read anywhere in scroll-edge-state.ts or its dependency chain; it exists only to satisfy the SnapshotNode type contract.
2. selectScrollContainer's scrollables.length === 0 early return (line 168, 2 mutants) — findNearestScrollableAncestor applies the identical scrollable+usable-rect predicate independently, so an empty scrollables cascades every fallback to null regardless.
3. x === undefined ? undefined : byIndex.get(x) guards (lines 207, 215, 3 mutants) — every real SnapshotNode.index/parentIndex is an always-defined number, so byIndex.get(undefined) already naturally returns undefined.
4. Redundant .trim() before .toLowerCase().includes(normalized) (line 250, 1 mutant) — .trim() only strips leading/trailing whitespace from the haystack; it cannot change whether .includes() finds an inner substring. Mathematically equivalent for every input.
5. matches[0]?.index === target.index (line 253, 2 mutants) — the container's own identifier/label is always a substring of itself after normalization, so the target node is always a member of matches; matches.length === 1 already implies matches[0] === target by construction.

Verification

• pnpm test:unit — the 6 new suites (59 tests) all pass. Full unit lane: unrelated failures each run are the repo's known CPU-contention timing flake (different file set each run — confirmed by re-running the failing files in isolation, where all pass).
• pnpm check:tooling — green (lint, typecheck, layering, depgraph, fallow, mcp-metadata, build, bundle-owner-files).
• pnpm format — ran; no diffs beyond the touched files.

Baseline file

mutation-baselines/decision-kernels.json was not modified. Per docs/agents/testing.md's mutation-ratchet section, recording a new baseline (pnpm mutation:baseline) is a separate, reviewed commit, not something this PR does.

Fixes #1455

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	1.87 MB	1.87 MB	-648 B
JS gzip	602.8 kB	602.6 kB	-170 B
npm tarball	720.5 kB	720.4 kB	-86 B
npm unpacked	2.52 MB	2.52 MB	-149 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	28.1 ms	27.6 ms	-0.5 ms
CLI --help	57.7 ms	57.9 ms	+0.2 ms

Top changed chunks:

Chunk	Raw diff	Gzip diff
dist/src/recording-export-quality.js	-648 B	-170 B

[thymikee]

P2 — The new 1,184-line test file violates the repository test-file context-safety rules and mixes formatter checks, fixtures, container selection, signatures, and pass-loop behavior. Please split it into focused sibling suites with named shared fixtures while preserving this coverage.

[thymikee]

Two blockers at exact head a0e8fbbf:

1. P2 — split the test architecture before merge. src/utils/scroll-edge-state.test.ts is now 1,323 LOC, with large repeated inline snapshot fixtures. AGENTS explicitly says tests are not exempt, >1,000 LOC is architecture debt, test files must mirror source concepts, and shared fixtures belong in a sibling module. Split the suite by durable behavior (container selection, pass orchestration, scope/signature) and extract named snapshot fixtures/builders.

2. P2 — replace mutation-score gaming with observable behavior. The signature block pins private serialization details and uses impossible typed inputs (undefined as unknown as number, 'garbage' as unknown as number). ScrollEdgeState.signature has no production reader outside this module; real scroll routes consume canScroll and scope. This contradicts test: scroll-edge-state has zero direct tests and a 28.75% mutation score — write the missing suite #1455’s requirement to test edge/direction/progress decisions rather than implementation details and docs/agents/testing.md’s behavioral-test rule. Remove dead/private representation tests (and simplify dead production state if warranted), then preserve the score with tests of observable scroll decisions. Also update the PR evidence: CI selected no mutation module because this baseline is gating:false, so the claimed 95.31% remains local evidence rather than a CI-verified result.

[thymikee]

Split the suite into three behavior-focused sibling files (container selection, scope naming, pass orchestration) plus a shared scroll-edge-state-fixtures.ts module, matching the existing daemon-runtime-port*.test.ts sibling-suite convention. Every file is now well under the 1,000-LOC tripwire (largest is 562 LOC).

🤖 Addressed by Claude Code

[thymikee]

Both blockers addressed:

1. Test split — done, see reply above.
2. Signature test-gaming — removed the dead signature/buildScrollStateSignature/collectSubtreeNodes/hasAncestor/roundSignatureNumber production code entirely (verified zero consumers anywhere in the repo, only .canScroll/.scope are ever read), so there's no private representation left to test against. Score after (local, scoped pnpm mutation:run --modules scroll-edge-state): 94.81% (274/289, 15 survivors, all mechanically traced against the exact Stryker-mutated span — not guessed — and documented in the PR description). PR description now explicitly notes CI didn't select this module (baseline gating:false), so this is local evidence pending a CI-verified run.

🤖 Addressed by Claude Code

[thymikee]

The original blockers are improved, but exact head 444ab669 still needs tightening before merge:

• P2 — test topology remains over budget. scroll-edge-state-container-selection.test.ts is 562 LOC, past AGENTS’ “past 500, extract before adding behavior” tripwire, with 20 inline nodes fixtures despite the shared fixture module. Split durable questions (hidden-edge, target resolution, broad selection) and/or add named builders so the suite is locally readable rather than another aggregation file.
• P2 — remove the unreachable undefined case. The test forces capture(undefined as unknown as SnapshotNode[]), but the public captureNodes contract requires an array and both production call sites normalize missing nodes with ?? []. This tests a private branch TypeScript/production routes cannot reach. Delete the cast/test and tighten analyzeScrollEdgeState to accept the real contract.
• P2 — one claimed equivalent mutant is not equivalent. For the missing identifier/label fallback mutant ('' → "Stryker was here!"), a ScrollView with no identifier/label and value: "Stryker was here!" makes the mutant return that scope while real code returns undefined, because uniqueness scans node.value. Kill that mutant or remove the redundant fallback branch, and correct the PR’s survivor justification/evidence.

CI is otherwise green; iOS smoke has now passed. No ready-for-human label until these test-quality gaps are resolved.

[thymikee]

All three addressed at 9c802b7c2:

1. Test topology over budget — split container-selection.test.ts (562 LOC) into four single-question files (container-existence, hidden-edge, target-resolution, broad-selection), largest now 196 LOC. Added a scrollNode() builder to the fixtures module and reused it across the suite to cut the inline-literal repetition.
2. Unreachable undefined case — you're right, deleted that test and its cast. Tightened analyzeScrollEdgeState's parameter type to drop | undefined (both real call sites, and captureScrollEdgeState's own captureNodes return type, always resolve to a real array).
3. Equivalence claim was wrong — confirmed your counter-example (I'd missed that isUniqueScopeValue scans node.value, not just identifier/label). Rather than write a test coupled to Stryker's specific replacement string, removed the redundant fallback branch itself: buildScrollContainerScope now filters to real strings instead of mapping a missing field to a placeholder '' (behaviorally identical — the placeholder always failed isUsefulScope's length check — but the mutation surface is gone entirely).

Updated score (local, scoped pnpm mutation:run --modules scroll-edge-state): 95.14% (274/288, 14 survivors, all re-verified against this commit's line numbers). PR description updated with the new file table, production-change rationale per finding, and corrected survivor list.

🤖 Addressed by Claude Code

[thymikee]

Exact-head review is clean. The production simplifications remove unreleased, unconsumed state and preserve real call-site behavior; the focused suites exercise exported decisions without private casts; the mutation result is internally consistent at 274/288 (95.14%) with the remaining survivors plausibly justified; and the baseline is untouched. Exact-head CI is fully green.

One optional tightening: the scoped-vs-unscoped “messages differ” test in scroll-edge-state-pass-orchestration.test.ts is redundant after the preceding exact-message assertions and can be removed. The 311-line orchestration suite is slightly above the 300-line target but below the 500-line extraction threshold; the focused multi-file split is a reasonable resolution of the prior oversized monolith. No blocking findings.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-29 15:35 UTC