Skip to content

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

Draft
thymikee wants to merge 2 commits into
mainfrom
claude/post-gesture-evidence-cleanup
Draft

refactor(daemon): consolidate post-gesture surface-evidence helpers and their rationale#1615
thymikee wants to merge 2 commits into
mainfrom
claude/post-gesture-evidence-cleanup

Conversation

@thymikee

@thymikee thymikee commented Aug 5, 2026

Copy link
Copy Markdown
Member

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

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.97 MB 1.97 MB -183 B
JS gzip 631.7 kB 631.6 kB -41 B
npm tarball 761.6 kB 761.6 kB -39 B
npm unpacked 2.67 MB 2.67 MB -183 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 26.9 ms 27.5 ms +0.6 ms
CLI --help 64.5 ms 65.2 ms +0.8 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/internal/daemon.js -183 B -41 B

@thymikee

thymikee commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

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
thymikee force-pushed the claude/post-gesture-evidence-cleanup branch from dab24b0 to 2401d28 Compare August 5, 2026 11:10
…nd 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-#1573 rationale
   (intersection rect-deltas) to a short pointer at classifyBaselineSurfaceEvidence,
   which already owns the current (post-#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.
…g 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.
@thymikee
thymikee force-pushed the claude/post-gesture-evidence-cleanup branch from 2401d28 to d20d2da Compare August 5, 2026 14:21
@thymikee

thymikee commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

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

thymikee commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

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

thymikee commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant