Refactor command handlers into specialized functions (#346)

* fix: document magic thresholds, fix BFS perf, and add edge-case tests

Address code review findings from #345:

- Document all undocumented heuristic constants across 11 files with
  rationale comments (freshness windows, retry delays, stale detection
  thresholds, safe-band zones, viewport-sized checks, scroll indicator
  boundaries, network log scan radii, nav subtree detection thresholds)
- Fix O(n²) BFS in ui-hierarchy.ts findScopeNode — replace shift() with
  index-based queue traversal
- Remove unused variable in ios/devices.ts listAppleDevices
- Document bundletool cache thread-safety assumption in app-lifecycle.ts
- Add 7 edge-case tests to mobile-snapshot-semantics.test.ts: zero-width
  viewport, zero-height viewport, negative coordinates, exact boundary
  visibility (both visible and outside cases)
- Add 2 boundary-condition tests to output.test.ts for nav subtree
  detection thresholds (below node floor, below duplicate floor)

All 1112 tests pass.

https://claude.ai/code/session_014aYgMag5ziyhXZoXcP9a7i

* refactor: decompose large handler functions for readability

Extract handler functions from three monolithic files:
- dispatch.ts: 13 command handlers from 786-line switch statement
- request-router.ts: split executeSessionRequest into focused helpers
- session-observability.ts: split into per-command handler functions

https://claude.ai/code/session_014aYgMag5ziyhXZoXcP9a7i

* test: add malformed snapshot data coverage for buildSnapshotState/buildSnapshotVisibility

Closes the remaining acceptance gap in #345: tests for undefined nodes,
sparse node fields, filtered-vs-unfiltered comparisonSafe semantics,
raw/macos-helper backend bypass, and scroll-hint visibility detection.

https://claude.ai/code/session_014aYgMag5ziyhXZoXcP9a7i

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: thymikee

src/core/dispatch.ts

@@ -1,8 +1,15 @@
 import type { SnapshotState } from '../utils/snapshot.ts';
 import type { SessionState } from './types.ts';
 
+// How long after a navigation-sensitive action (press, click, back, open) to consider
+// the Android UI hierarchy potentially stale.  Android's UIAutomator dump is async
+// and can lag behind real transitions by up to ~2 s; 2.5 s gives a comfortable margin
+// while avoiding unnecessary retries for steady-state interactions like typing.
 const ANDROID_FRESHNESS_WINDOW_MS = 2_500;
 
+// Progressive back-off delays between freshness retry attempts.  Two retries at
+// 250 ms + 400 ms cover the vast majority of Android transition latencies without
+// adding perceptible lag to the happy path.
 export const ANDROID_FRESHNESS_RETRY_DELAYS_MS = [250, 400] as const;
 
 export type AndroidSnapshotFreshness = {
@@ -82,6 +89,9 @@ export function buildSnapshotSignatures(nodes: SnapshotState['nodes']): string[]
   );
 }
 
+// A snapshot whose node count dropped to ≤20% of the previous capture is likely a
+// stale or mid-transition dump.  The 12-node floor prevents false positives on
+// already-tiny trees where fluctuation is normal.
 export function isLikelyStaleSnapshotDrop(previousCount: number, currentCount: number): boolean {
   if (previousCount < 12) {
     return false;
@@ -97,6 +107,8 @@ export function isLikelySnapshotStuckOnPreviousRoute(
     return false;
   }
   const total = Math.max(previousSignatures.length, currentNodes.length);
+  // Trees smaller than 12 nodes are too small for meaningful route comparison —
+  // minor UI updates can produce high overlap percentages by coincidence.
   if (total < 12) {
     return false;
   }
@@ -110,6 +122,9 @@ export function isLikelySnapshotStuckOnPreviousRoute(
   }
   const additions = Math.max(0, currentSignatures.length - previousSignatures.length);
   const removals = Math.max(0, previousSignatures.length - currentSignatures.length);
+  // Consider the snapshot "stuck" when ≥90% of nodes are identical and the number of
+  // additions/removals stays within 15% (or at least 3).  These thresholds accommodate
+  // minor dynamic content (clocks, counters) while still detecting genuine route changes.
   const toleratedDelta = Math.max(3, Math.floor(total * 0.15));
   return (
     unchanged >= Math.floor(total * 0.9) &&

src/daemon/android-snapshot-freshness.ts

@@ -3,6 +3,7 @@ import fs from 'node:fs';
 import os from 'node:os';
 import path from 'node:path';
 import { handleSnapshotCommands } from '../snapshot.ts';
+import { buildSnapshotState, buildSnapshotVisibility } from '../snapshot-capture.ts';
 import { SessionStore } from '../../session-store.ts';
 import type { SessionState } from '../../types.ts';
 import { AppError } from '../../../utils/errors.ts';
@@ -1332,3 +1333,128 @@ test('wait sleep bypasses sessionless runner cleanup wrapper', async () => {
   expect(response).toBeTruthy();
   expect(response?.ok).toBe(true);
 });
+
+// ---------------------------------------------------------------------------
+// Malformed snapshot data – buildSnapshotState robustness
+// ---------------------------------------------------------------------------
+
+test('buildSnapshotState handles undefined nodes gracefully', () => {
+  const state = buildSnapshotState({ nodes: undefined, truncated: undefined }, undefined);
+  expect(state.nodes).toEqual([]);
+  expect(state.truncated).toBeUndefined();
+  expect(state.createdAt).toBeGreaterThan(0);
+});
+
+test('buildSnapshotState handles completely empty data object', () => {
+  const state = buildSnapshotState({}, undefined);
+  expect(state.nodes).toEqual([]);
+  expect(state.truncated).toBeUndefined();
+});
+
+test('buildSnapshotState handles nodes with missing fields', () => {
+  const state = buildSnapshotState(
+    {
+      nodes: [
+        { index: 0 } as any,
+        { index: 1, depth: undefined, type: undefined, label: undefined } as any,
+      ],
+      truncated: false,
+      backend: 'android',
+    },
+    undefined,
+  );
+  expect(state.nodes).toHaveLength(2);
+  // Nodes should get refs assigned even with sparse data
+  expect(state.nodes[0]?.ref).toBeTruthy();
+  expect(state.nodes[1]?.ref).toBeTruthy();
+});
+
+test('buildSnapshotState marks comparisonSafe false for filtered Android snapshots', () => {
+  const nodes = [{ index: 0, depth: 0, type: 'android.widget.TextView', label: 'A' }];
+
+  const interactiveOnly = buildSnapshotState(
+    { nodes, backend: 'android' },
+    { snapshotInteractiveOnly: true },
+  );
+  expect(interactiveOnly.comparisonSafe).toBe(false);
+
+  const compact = buildSnapshotState({ nodes, backend: 'android' }, { snapshotCompact: true });
+  expect(compact.comparisonSafe).toBe(false);
+
+  const withDepth = buildSnapshotState({ nodes, backend: 'android' }, { snapshotDepth: 2 });
+  expect(withDepth.comparisonSafe).toBe(false);
+
+  const withScope = buildSnapshotState({ nodes, backend: 'android' }, { snapshotScope: 'Header' });
+  expect(withScope.comparisonSafe).toBe(false);
+
+  const unfiltered = buildSnapshotState({ nodes, backend: 'android' }, {});
+  expect(unfiltered.comparisonSafe).toBe(true);
+});
+
+test('buildSnapshotState marks comparisonSafe false for non-Android backends', () => {
+  const nodes = [{ index: 0, depth: 0, type: 'Button', label: 'OK' }];
+  const state = buildSnapshotState({ nodes, backend: 'xctest' }, {});
+  expect(state.comparisonSafe).toBe(false);
+});
+
+// ---------------------------------------------------------------------------
+// Malformed snapshot data – buildSnapshotVisibility robustness
+// ---------------------------------------------------------------------------
+
+test('buildSnapshotVisibility returns non-partial for empty node list', () => {
+  const vis = buildSnapshotVisibility({ nodes: [], backend: 'android' });
+  expect(vis.partial).toBe(false);
+  expect(vis.visibleNodeCount).toBe(0);
+  expect(vis.totalNodeCount).toBe(0);
+  expect(vis.reasons).toEqual([]);
+});
+
+test('buildSnapshotVisibility skips semantic analysis for raw snapshots', () => {
+  const nodes = [
+    { ref: 'e1', index: 0, depth: 0, type: 'View', label: 'Root', hiddenContentBelow: true },
+  ];
+  const vis = buildSnapshotVisibility({ nodes, backend: 'android', snapshotRaw: true });
+  expect(vis.partial).toBe(false);
+  expect(vis.visibleNodeCount).toBe(1);
+  expect(vis.totalNodeCount).toBe(1);
+  expect(vis.reasons).toEqual([]);
+});
+
+test('buildSnapshotVisibility skips semantic analysis for macos-helper backend', () => {
+  const nodes = [
+    { ref: 'e1', index: 0, depth: 0, type: 'AXButton', label: 'Click Me' },
+  ];
+  const vis = buildSnapshotVisibility({ nodes, backend: 'macos-helper' });
+  expect(vis.partial).toBe(false);
+  expect(vis.reasons).toEqual([]);
+});
+
+test('buildSnapshotVisibility detects scroll-hidden-above and scroll-hidden-below', () => {
+  const nodes = [
+    {
+      ref: 'e1',
+      index: 0,
+      depth: 0,
+      type: 'ScrollView',
+      label: 'Feed',
+      hiddenContentAbove: true,
+      hiddenContentBelow: true,
+    },
+  ];
+  const vis = buildSnapshotVisibility({ nodes, backend: 'android' });
+  expect(vis.partial).toBe(true);
+  expect(vis.reasons).toContain('scroll-hidden-above');
+  expect(vis.reasons).toContain('scroll-hidden-below');
+});
+
+test('buildSnapshotVisibility handles nodes with no scroll hints as non-partial', () => {
+  const nodes = [
+    { ref: 'e1', index: 0, depth: 0, type: 'Button', label: 'OK', hittable: true },
+    { ref: 'e2', index: 1, depth: 0, type: 'Button', label: 'Cancel', hittable: true },
+  ];
+  const vis = buildSnapshotVisibility({ nodes, backend: 'xctest' });
+  expect(vis.partial).toBe(false);
+  expect(vis.visibleNodeCount).toBe(2);
+  expect(vis.totalNodeCount).toBe(2);
+  expect(vis.reasons).toEqual([]);
+});

src/daemon/handlers/__tests__/snapshot-handler.test.ts

@@ -78,6 +78,9 @@ export async function handleFindCommands(params: {
     backend?: SnapshotState['backend'];
   }> => {
     const now = Date.now();
+    // Re-use a snapshot captured within the last 750 ms to avoid redundant dumps during
+    // rapid find iterations.  Skipped when Android freshness tracking is active, because
+    // the cached tree may already be stale from a recent navigation action.
     if (lastNodes && now - lastSnapshotAt < 750 && !getActiveAndroidSnapshotFreshness(session)) {
       return { nodes: lastNodes };
     }

src/daemon/handlers/find.ts

@@ -135,6 +135,10 @@ export async function handleScrollIntoViewCommand(
 
     const distance = distanceFromSafeViewportBand(node.rect, viewportRect);
     if (distance === 0) break;
+    // If the target didn't get closer after a scroll, count it as a stall.  Two
+    // consecutive stalls (no progress in either attempt) indicate the element is
+    // likely unreachable — e.g. inside a non-scrollable container or already at
+    // the scroll boundary.
     if (distance >= lastDistance) {
       stalledCount += 1;
       if (stalledCount >= 2) {

src/daemon/handlers/interaction-scroll.ts

@@ -272,6 +272,8 @@ function findPreferredActionableDescendant(
 }
 
 function areRectsApproximatelyEqual(left: Rect, right: Rect): boolean {
+  // 0.5 px tolerance absorbs sub-pixel rounding differences that are common in
+  // accessibility tree coordinates across iOS and Android DPI scales.
   const tolerance = 0.5;
   return (
     Math.abs(left.x - right.x) <= tolerance &&
@@ -312,6 +314,10 @@ function resolveRootViewportRect(nodes: SnapshotNode[], targetRect: Rect): Rect
   return pickLargestRect(containingRects.length > 0 ? containingRects : viewportRects);
 }
 
+// An ancestor is "viewport-sized" when it covers ≥90% of the viewport area and
+// at least 80% of its own area overlaps.  This catches full-screen containers
+// (navigation bars, root views) that are technically hittable but would produce
+// imprecise taps if used as the touch target.
 function isRectViewportSized(rect: Rect, viewportRect: Rect): boolean {
   const overlapArea = intersectionArea(rect, viewportRect);
   const rectArea = rect.width * rect.height;