refactor(daemon): consolidate post-gesture surface-evidence helpers and their rationale

[thymikee]

refactor(daemon): drop the numeric-token heuristic instead of wrapping it

The previous commit replaced formatGestureNoEffectWarning's
"drop numeric-looking positionals" regex with a per-action builder table —
but kept the regex as the table's fallback. That is two mechanisms where
there was one, and the heuristic this finding exists to remove is still
in the file.

Only scroll, swipe and gesture <subtype> … reach the warning by
default (isPostGestureStabilizingAction), and across all three exactly
one positional is not part of the gesture's identity: scroll's optional
trailing amount. Echoing everything else verbatim is simpler than either
version and strictly more truthful than the original, which ate all four
coordinates of swipe <x1> <y1> <x2> <y2> and reported a contentless
bare "swipe".

scroll down 0.6 -> "scroll down"
scroll up -> "scroll up"
swipe 10 20 30 40 -> "swipe 10 20 30 40" (was: "swipe")
swipe -> "swipe"
gesture swipe left -> "gesture swipe left"
gesture fling down 100 200 -> "gesture fling down 100 200"

35 lines of table, dispatch and regex become 4 lines with one named
exception. --postGestureStabilization can still force an arbitrary
action through; echoing its positionals is the honest answer for one
warning string, and reconstructing each gesture subtype's layout would
fork a grammar that lives in GestureSemanticInput, downstream of the raw
positionals this function receives.

refactor(daemon): consolidate post-gesture surface-evidence helpers and their rationale

Six findings against interaction-outcome-policy.ts and post-gesture-stabilization.ts,
all behavior-preserving except the gesture-warning wording:

1. Extract rectsWithinTolerance and use it in areInteractionSurfaceSignaturesStable,
   classifyBaselineSurfaceEvidence, and haveIdenticalDiscriminatingSurfaces instead
   of three copies of the same four-field comparison.
2. Unbox identifiedContent's Map from { entry } to the entry itself, dropping the
   .entry indirection at every call site.
3. Document why haveIdenticalDiscriminatingSurfaces deliberately keys on key
   (not identity, the opposite of interactionSurfaceIdentity's own conclusion):
   it runs only after classifyBaselineSurfaceEvidence already accepted
   'unchanged', exists solely to gate an agent-facing no-effect claim, and a
   stricter key trades recall for precision — the safe direction for a veto.
4. Relocate decidePostGestureStabilityVerdict's stale pre-fix(daemon): judge post-gesture movement by identity, not by the intersection #1573 rationale
   (intersection rect-deltas) to a short pointer at classifyBaselineSurfaceEvidence,
   which already owns the current (post-fix(daemon): judge post-gesture movement by identity, not by the intersection #1573) evidence rule.
5. Type PostGestureStabilization.baselineBackend and CapturedSurface.backend as
   SnapshotQualityVerdict['backend'] instead of bare string, closing the silent-typo
   gap in the backend-rebase equality check.
6. Replace the numeric-regex guess in formatGestureNoEffectWarning with the known,
   fixed positional grammar for scroll/swipe; gesture keeps a narrower fallback
   since its per-subtype shape isn't uniform.

Gates: typecheck / lint / format / check:layering green; 30 unit tests in the touched files pass.

Reviewer note: the second commit reworks the first. The initial pass replaced formatGestureNoEffectWarning's numeric-token regex with a per-action builder table but kept the regex as that table's fallback — two mechanisms where there was one, with the heuristic still present. It is now one named exception (scroll's trailing amount) and no heuristic, which also fixes the real defect the table found: the old rule ate all four coordinates of swipe <x1> <y1> <x2> <y2> and emitted a contentless bare "swipe".

---

Local gate note. pnpm check:affected --run is flaky on a loaded dev host. Across six runs on three branches — plus a reproduction on unmodified origin/main by a parallel agent — failures came only from the pool android-lifecycle / android-recording / doctor / input-actions / daemon-client, always as timeouts, with a different subset each run. All pass in isolation (verified for this branch). Please treat the CI Integration Tests job on this head as authoritative for provider-integration, per AGENTS.md ("GitHub remains authoritative for reported device/toolchain lanes").

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	1.99 MB	1.99 MB	-235 B
JS gzip	635.8 kB	635.8 kB	-49 B
npm tarball	769.2 kB	769.4 kB	+117 B
npm unpacked	2.69 MB	2.69 MB	+166 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	27.8 ms	27.7 ms	-0.1 ms
CLI --help	65.5 ms	68.2 ms	+2.7 ms

Top changed chunks:

Chunk	Raw diff	Gzip diff
dist/src/internal/daemon.js	-235 B	-49 B

[thymikee]

Code review is clean on dab24b0a. I traced the production path from gesture dispatch through post-gesture stabilization to warning composition; the helper consolidation and backend type narrowing preserve semantics, and the swipe-coordinate regression test is non-vacuous against the previous numeric-token filter. Not merge-ready yet: the iOS smoke lane must complete, and because the visible changed path is the iOS post-gesture warning, please attach direct live evidence from the exact head showing a no-effect swipe warning retains its coordinates and the session remains usable for a following command.

[thymikee]

Live evidence attempt at rebased head d20d2da2, CLI + daemon built from this branch, isolated --state-dir, explicit --platform ios --udid on every command (an earlier attempt silently re-resolved onto an Android emulator mid-session — worth knowing).

I could not produce the warning, and I do not think that is a reproduction problem. Reporting the mechanism rather than presenting a near-miss as evidence.

Tried five no-effect gestures across two screens:

• Settings: drag-down at list top, horizontal drag across a vertical-only list, plus a --debug run.
• Bluesky feed on a seeded fixture sim, confirmed Already at top; no hidden content above detected, then swipe 200 250 200 650 (drag down with nothing above) — twice.

Every one reported Flung and every following capture carried no gestureNoEffect. The capture diagnostics on the Bluesky run say why:

Warning: ios snapshots are slow in this run: p95 13442ms over 2 captures
Snapshot: 167 visible nodes (171 total) (truncated)
Detected an overly complex or slow accessibility tree. Fell back to the private-ax snapshot backend.
Some deeper accessibility nodes were omitted; this tree is capped at depth 56.

Two things in that output each independently veto the claim, both by design:

1. Backend flip. Post-gesture stale-accept saturates routinely on the checkout screen; wall-clock retry budget starves under contention into stale-geometry interactions #1569's rule is that a quiet capture from a different backend can only be re-baselined against, never concluded from. A tree→private-ax fallback mid-sequence means the loop never reaches a comparable pair.
2. Truncation drift. haveIdenticalDiscriminatingSurfaces requires the discriminating sets to match exactly in both directions. A depth-56-capped, truncated 167-node tree will not match its baseline set even when nothing moved.

So the gate is working exactly as #1601 specified — silence as the safe failure mode. The consequence is the uncomfortable part: the warning appears unreachable on precisely the screen class #1600 was built for. #1600's motivating case was element-18 burning ~40 tool calls re-issuing scrolls on a Bluesky feed, and a Bluesky feed is hostile enough to capture that the corroboration can never be satisfied.

I did not isolate which of the two vetoes dominates — that needs the per-capture backend and signature sets, not just the rendered warning. Filing separately rather than expanding this PR.

What this means for this PR: nothing in it changes when the warning fires. It consolidates helpers, types baselineBackend, relocates a stale docblock, and replaces the numeric-token heuristic in the wording function. The behavioral surface it does change — swipe coordinate retention — is covered by the non-vacuous regression test you already reviewed as sound. If you want the live capture before merging, it needs a screen whose captures stay on one backend and untruncated, which is a different fixture than either I tried.

🤖 Addressed by Claude Code

[thymikee]

Re-review at d20d2da29: the rebase is patch-identical to the previously clean reviewed change, and integration with current main remains code-clean. The production route, capture-backend type narrowing, warning composition, and non-vacuous swipe-coordinate regression remain sound; no code findings. Still not ready: iOS Smoke was in progress at review time, and exact-head live evidence remains absent for a no-effect swipe warning retaining its coordinates plus a successful following command proving the session remains usable.

[thymikee]

Status update for d20d2da29: iOS Smoke has completed successfully, so code review and all CI checks are now clean. The sole readiness blocker remains the requested exact-head live evidence: a no-effect swipe warning retaining its coordinates, followed by a successful command proving the session remains usable.

[thymikee]

Thermo-nuclear code-quality review @ d20d2da29

Verdict: approve with suggestions — no structural regressions, no blockers. The consolidation does what it says: three copies of the rect comparison become one, the { entry } box is gone, the stale pre-#1573 rationale now lives at the function that owns the rule, and the second commit is the right kind of judo — the builder-table-plus-regex two-mechanism shape collapsed into one named exception with the heuristic actually deleted. The one behavior change (swipe coordinates surviving into the warning) is a strict truthfulness win and is tested.

Structural options I evaluated and rejected, recorded so they don't get re-flagged later:

• Splitting the surface-evidence model out of interaction-outcome-policy.ts. post-gesture-stabilization.ts imports only the signature/evidence half of that module, never the retry-policy half, which makes a surface-evidence.ts extraction look tempting. Rejected: the handlers consume both halves together, the shared vocabulary type hangs off SessionState, and at 463 lines the file's density is comment-mass, not concept-mass. The split would rename the dependency without deleting anything.
• Reusing areRectsApproximatelyEqual (src/utils/rect-center.ts) instead of the new rectsWithinTolerance. Rejected: tolerance 0.5 over already-rounded integers means exact equality — the 1 px band here is a different policy, and parameterizing the shared util to carry both would put a daemon-specific knob in a general helper. The local helper is right.

Findings, ranked

1. The backend union wants a name, not two paragraphs (medium). Finding 5 closes the silent-typo gap, but the confusability the new comments fight — SnapshotQualityVerdict['backend'] vs SnapshotBackend, both living in kernel/snapshot — is a naming gap, and the PR treats it with prose in two places (11 lines in types.ts, 4 on CapturedSurface) whose only job is to say what the type is NOT. export type SnapshotCaptureBackend = 'tree' | 'queries' | 'private-ax' in packages/kernel/src/snapshot.ts (with SnapshotQualityVerdict.backend: SnapshotCaptureBackend) makes both sites self-documenting; the types.ts docblock then shrinks to the load-bearing #1569 comparability warning. Fine as a follow-up.

2. Comment mass is growing faster than code is shrinking (medium). Net diff is +42 lines in a simplification PR, all of it prose. Three instances:

• The haveIdenticalDiscriminatingSurfaces addition makes one point — strict key = precision over recall, correct for a veto — three times across 20 lines. The opening sentence plus the mechanism sentence ("any volatile-state flip makes the keys mismatch … returns false") carry it; the rest is advocacy. Half the length keeps the full decision record.
• The swipe rationale now lives in three places: the describeGesturePositionals docblock, a 5-line comment on the new test, and the PR body. The test comment restates the docblock nearly verbatim; the test name already carries it.
• rectsWithinTolerance's docblock enumerates its three callers by name — a roster that drifts the first time a fourth caller lands. "The surface-stability checks in this module" says the same without the list.

The drift risk isn't hypothetical — it's already in the diff: describeGesturePositionals says "gesture <subtype> …" reaches the warning by default, but isPostGestureStabilizingAction only passes gesture swipe. Each invariant stated once, at its owner.

3. The scroll exception is the old heuristic's last vestige (low; deliberate choice acknowledged). describeGesturePositionals plus its 17-line docblock exist to suppress a single token: scroll down 0.6 → "scroll down". Echoing verbatim everywhere deletes the helper, the docblock, and the exception, and "scroll down 0.6 produced no visible change" meets the PR's own truthfulness bar; the only cost is one assertion in the existing warning test. If the amount genuinely harms the agent-facing wording, keep the exception — but then the docblock should say why the amount hurts, which is the one thing it doesn't argue.

4. In-scope nit: positionals? optionality is noise (low). markPostGestureStabilization is the only writer of session.postGestureStabilization and always sets positionals (default []), so pending.positionals ?? [] in buildAcceptedStabilizedResult guards a state that cannot exist. Make the field required and the ?? disappears — this PR is exactly the cleanup of this shape.

Merge logistics

The head predates #1613 landing on main, which rewrote this same file's viewport-root predicate (isViewportRootKind → shared isViewportRootNode). Checked the three-way merge: the hunks don't overlap, GitHub reports CLEAN, and the auto-merged result is coherent — main's side deletes the local predicate this branch still shows. All CI green on this head, Integration Tests included, consistent with the flaky-local-lanes note in the description.

[thymikee]

All four findings applied at 5731692c — three of them delete code, so I took them now rather than as follow-ups.

1. Named the union. SnapshotCaptureBackend lives in kernel/snapshot beside SnapshotBackend, and SnapshotQualityVerdict uses it. You were right that the confusability is a naming gap being treated with prose: the two docblocks whose only job was saying what the type is NOT (11 lines in types.ts, 4 on CapturedSurface) collapse to the load-bearing #1569 comparability warning. Done here rather than deferred because it is what makes finding 2 possible.

2. Comment mass — and the drift was real. Key-vs-identity now states its point once. rectsWithinTolerance no longer names its callers. The test comment restating the docblock is gone. And the drift you spotted is not hypothetical: the docblock claimed gesture <subtype> … reaches the warning by default when isPostGestureStabilizingAction only admits gesture swipe — that comment is deleted rather than corrected.

3. Deleted the scroll exception. Took the deletion. Your framing settled it: the warning names the gesture the agent issued, and scroll down 0.6 is what they issued. describeGesturePositionals, its 17-line docblock and the exception all go; positionals echo verbatim; one assertion updated. The heuristic now has no vestige at all, which is what the PR claimed two commits ago.

4. positionals required. Tightening it caught five test fixtures constructing the state without the field — the type earning its keep on contact.

Measured effect on your headline concern: net diff went from +42 to +4 lines.

Also rebased onto main post-#1613. As you predicted the three-way was clean; the local isViewportRootKind this branch still showed is gone on main's side and the hunks never overlapped.

Gates: 93 files / 733 tests on the touched suites, typecheck / lint / format / check:layering clean. check:affected red only on android-lifecycle + android-recording (~15s and ~5s timeouts), which pass 33/33 in isolation with exit 0 — the load flake, and unreachable from this change.

Still draft, still blocked on the same live capture, which #1622 is upstream of.

🤖 Addressed by Claude Code

[thymikee]

Re-reviewed exact head 5731692c: code review is clean. The named SnapshotCaptureBackend, rect-comparison consolidation, tightened key-vs-identity rationale, verbatim gesture positionals, and required positionals state are coherent; the swipe-coordinate regression is non-vacuous, all fixtures were repaired, the production warning route remains intact, and CI is green.

Readiness remains blocked by the previously requested live proof. #1622 is the behavioral prerequisite (not a compile dependency): it rebases corroboration onto a comparable fallback baseline for the hostile screen where current backend flips/truncation correctly silence the no-effect claim. Merge #1622 first, rebase #1615 onto it—both touch post-gesture-stabilization.ts near accepted-result construction—and validate the combined exact head with a no-effect coordinate swipe warning retaining all coordinates followed by a successful command. No readiness label yet.

[thymikee]

Correcting my earlier comment on this PR. I wrote that the no-effect warning "appears unreachable on precisely the screen class #1600 was built for", and named a backend flip breaking corroboration as one of two causes. That reading was wrong.

post-gesture-stability.ts corroborates against the original pre-gesture baseline rather than the loop's rebased one, and #1633 documents that as deliberate: a rebased baseline is itself a post-gesture capture, so concluding from it would prove the screen went quiet, not that the gesture did nothing. A backend flip should suppress the claim. I opened #1622 to change this and have closed it as misconceived; #1620 is corrected too.

Nothing in this PR depends on that reading — it consolidates helpers, types the capture-strategy union, relocates a docblock and deletes the numeric-token heuristic. But the comment is on this thread and was steering the live-evidence discussion, so it should not stand uncorrected.

Rebase status: this branch is now behind d919876c, where #1633 deleted post-gesture-stabilization.ts and split it into post-gesture-stability.ts, deferred-interaction-outcome.ts and gesture-no-effect.ts. That is a modify/delete against this branch, so the change needs re-deriving onto the new layout rather than merging. Checked what survived: none of the four review findings are in main — gesture-no-effect.ts still carries the /^[\d.-]+$/ filter verbatim, and interaction-outcome-policy.ts still has the three rect-comparison copies and the { entry } box. So the work stands; it just moves house.

Re-deriving now.

🤖 Addressed by Claude Code

[thymikee]

Re-derived onto d5f11f6e and squashed to one commit (9b902d4c).

#1633 subsumed none of it. It relocated post-gesture-stabilization.ts into three files and carried every one of these forward untouched: gesture-no-effect.ts still had the /^[\d.-]+$/ filter verbatim, interaction-outcome-policy.ts still had the three rect-comparison copies and the { entry } box, baselineBackend was still string, positionals still optional.

One finding deliberately narrowed in the move. #1633 made post-gesture-stability.ts generic over the surface type with backend: string | undefined. That is right for an interface that must not know about iOS capture strategies, so SnapshotCaptureBackend is applied only at the concrete site — PostGestureStabilization.baselineBackend, which is where the silent-typo gap actually lives. Applying it to the generic module would have been fidelity to my old diff at the cost of the abstraction #1633 just built.

Net is +23 rather than the +4 I reported pre-rebase: the comment trimming was against text that no longer exists, and shrinking #1633's prose is not this PR's business.

positionals being required caught six fixtures this time, one more than before — #1633 added one.

Red evidence re-established on the new layout: restoring the numeric filter fails the wording test (scroll down 1 produced no visible change). 95 files / 757 tests green; typecheck, lint, format, check:layering clean.

Gate disclosure. check:affected is red with 15 failures, all provider-integration/process-spawn at 5001–5009 ms or 11–15 s. Load average on this host is 11 / 23 / 90 with 46 vitest processes from two unrelated worktrees (priceless-goldberg-8167c6, focused-burnell-d52a8a), 23 each. Isolation on a representative subset: 46 of 47 pass, and the single non-pass is the slow-test gate — daemon-entrypoint at 5.00 s against a 2.5 s wall-clock budget — not an assertion. A wall-clock budget is precisely what a saturated host breaks. None of the failing tests can reach this change.

🤖 Addressed by Claude Code

[thymikee]

Enumerating the heuristic deletion's full output delta, since "the test pins the new wording" is not the same as "nothing else changed".

The concern worth checking is whether the old filter ever rewrote a token rather than merely dropping one — deletion would then change more outputs than the single case the test covers. It cannot: the two expressions are identical but for positionals.filter(...), and filter is subtractive. The new token sequence is always the old one plus previously-dropped tokens, same order, same action prefix. No path rewrites.

Enumerated over every shape isPostGestureStabilizingAction admits (swipe, scroll, gesture swipe), the flag-forced actions --postGestureStabilization can push through, and adversarial tokens against ^[\d.-]+$:

28 cases: 13 identical, 15 token-additions, 0 REWRITES

The 15 additions are exactly what the filter used to eat:

scroll down 1        was "scroll down"
scroll down 0.6      was "scroll down"
swipe 10 20 30 40    was "swipe"          <- the motivating regression
gesture swipe 100 200 300 400   was "gesture swipe"
pan 10 20 30 40      was "pan"
pinch 0.5            was "pinch"
type 123             was "type"
press 42 99          was "press"
x . / x - / x -- / x 1.2.3 / x 1-2 / x 0  all previously vanished

Every one is a token appearing where it previously vanished; none is a token changed. So the deletion's blast radius is fully characterised, and the flag-forced actions — which nobody had enumerated, since they only reach the warning via --postGestureStabilization — improve for the same reason swipe does.

🤖 Addressed by Claude Code

[thymikee]

Re-reviewed exact head 9b902d4c. Code review is clean: the shared rect helper and map unboxing preserve the existing comparison semantics, SnapshotCaptureBackend narrows the concrete session field correctly, required positionals matches the sole writer, and the warning regression is non-vacuous—restoring the numeric filter breaks both the scroll and coordinate-swipe assertions. The production route remains gesture dispatch → pending outcome/post-gesture marker → snapshot resolution/stability → composed warning.

Not ready yet: exact-head live evidence for the changed device-facing warning path is absent. Show a proven no-effect coordinate swipe whose next response includes the full gesture and then a successful subsequent command. Also refresh the PR body: it still describes the old file layout, two-commit sequence/test count, and a fallback the current one-commit head removes. Completed checks are green; Swift Runner Unit Compile remains queued. No code blocker or conflict found.