[cryptotavares]

Summary: Skip automatic observation collection for read-only, discovery, and raw CDP tools. Keep observations for mutating tools and eligible batches, and reset the diff baseline after accessibility snapshots.

Stack context: mm CLI v2, Phase 1, P1-05. Base: #60 (cryptotavares/mmcli-p1-04-remove-knowledge-store); followed by #62. Review #60 first.

Problem

The daemon collects observations after read-only and discovery calls, although their responses omit those observations. That work also refreshes refs and advances the diff baseline. The knowledge store that consumed it is gone after #60.

Raw cdp calls use the mutating category. A small result, such as Runtime.evaluate 1+1, therefore triggers and returns an additional observation. On mobile, observation collection queries app state and requests test IDs and an accessibility tree.

Solution

Keep the policy at the shared HTTP executor. Tool handlers and platform drivers retain their existing responsibilities.

flowchart TD
    A[Queued tool finishes] --> B{Category and batch policy}
    B -->|readonly, discovery, cdp, or batch none| C[Skip observation collection]
    B -->|mutating or eligible batch| D{Active session?}
    D -->|yes| E[Collect observation through platform driver]
    D -->|no| C
    E --> F[Apply response inclusion policy]
    F -->|included| G[Compact against last observation]
    F -->|omitted| H[Return result and timing]
    C --> H
    G --> H

    classDef new fill:#fff3cd,stroke:#d39e00
    class B,C,F new

Loading

Yellow boxes are new.

shouldIncludeObservationsInResponse calls shouldCollectObservations first. A shared batch-policy helper supplies the default, so collection and response inclusion use the same rule.

The registry gives cdp its own public category. CDP method execution stays in the platform driver. A state-changing CDP call needs a subsequent mm describe-screen to refresh refs. Per-call observe opt-in remains a P1-06 follow-up.

accessibility_snapshot now clears the diff baseline, like describe_screen, launch, and cleanup. The next collected mutation observation is full and compacted. Other read calls leave the baseline unchanged.

What changes

Condition or boundary	Before	After
readonly or discovery call	Collects an observation but omits it from the response	Skips automatic collection; observationMs is zero
Raw cdp call	Uses mutating; collects and returns an observation when available	Uses cdp; skips automatic collection and returns no observations
Mutating call with an active session	Collects and returns an observation when available	Same collection, compaction, and non-fatal failure handling
run_steps policy	all by default; none skips collection; failures filters the response	Same batch-level policy; failures returns observations only for a tool error or failed summary
Ref refresh	Automatic observations refresh the session refMap	Skipped observations do not refresh refs; tree-producing discovery handlers still update refs themselves
accessibility_snapshot followed by a mutation	Retains an older diff baseline	Clears the baseline; the next mutation returns a full compact observation

Decisions for reviewers

Decision	Reason
Stop collection at the executor boundary	Removes redundant browser and mobile work without changing each read-tool handler.
Add cdp to public ToolCategory	Makes the default explicit. CDP methods can change state, so callers must request fresh discovery afterward.
Keep collection separate from response inclusion	A successful batch with failures still collects an observation and advances the baseline, but omits it from the response.
Clear the baseline after accessibility snapshots	The caller receives a tree directly; the next mutation must not diff against an older observation.

Review guide

Read the files in this order:

#	File	What to check
1	src/tools/registry.ts	Public cdp category, tool assignment, and unchanged safe default for unknown tool names.
2	src/server/create-server.ts	Collection predicate, shared batch policy, response inclusion, and accessibility baseline reset.
3	src/server/create-server.test.ts	Per-category timing, skipped collection, batch failures, and baseline behavior.
4	src/tools/registry.test.ts	cdp classification and registry/category coverage.
5	README.md, SKILL.md	Observation rules, CDP ref-refresh instructions, and baseline reset documentation.

Testing

• Fresh check at remote head 74b9819d26e4e1ff63b5a9be675a4b3cefd94e68: CI=true yarn vitest run src/server/create-server.test.ts src/tools/registry.test.ts src/tools/batch.test.ts src/tools/cdp.test.ts src/tools/discovery-tools.test.ts --coverage.enabled=false --maxWorkers=2 --minWorkers=1 passes: 5 files, 181 tests.
• yarn lint passes. Production type checking passes with yarn tsc --project tsconfig.build.json --noEmit.
• yarn tsc --project tsconfig.test.json --noEmit fails at the unchanged remote head with nine diagnostics. They affect the new timing-table callback and existing mobile-driver, network-mock, and screenshot tests. Vitest reports no type errors, but its result does not establish a clean explicit test-project type check.
• The Proof and Unit tests sections below retain the original historical evidence. These fresh checks do not repeat live-device validation.

Breaking changes

• readonly, discovery and cdp tools no longer collect observations. As a result:
  • their responses no longer include observations (cdp used to include them);
  • they no longer refresh the session refMap through an observation (discovery tools still set it themselves);
  • they no longer move the diff baseline for the next mutating call.
• A cdp call that changes state (for example Runtime.evaluate with a .click()) must now be followed by describe-screen to get fresh refs.
• The public ToolCategory type gains 'cdp'. Exhaustive switches on it need a new case.

Consumer impact

Consumer	Change needed	Done on branch
Extension test/e2e/playwright/llm-workflow/	None. No code or test reads observations.	n/a
Mobile tests/llm-workflow/	None. Same check.	n/a

Proof

Tested with a yalc build of this branch in the Extension (e2e build, headless) and in Mobile (dev client on iOS sim and Android emulator), on wallet home after unlock. Before = #60 tip (0.12.0+f2caa574), after = 0.12.0+ce3fd9f0. Ext was rechecked on the final commit (0.12.0+8a7091bf). Bench: scripts/bench.mjs, 5 runs (Android 3), p50.

Check	Before	After
cdp Runtime.evaluate 1+1 bytes, ext / iOS / Android	7408 / 59228 / 1054409 (smoke)	267 / 240 / 244
get-state totalMs, ext / iOS / Android	695 / 5736 / 6345	9 / 652 / 129
cdp totalMs, ext / iOS / Android	963 / 3334 / 6882	2 / 6 / 497
list-testids totalMs, ext / iOS / Android	1340 / 8901 / 13665	632 / 3835 / 11639
observationMs for get-state, get-text, hermes-targets, list-testids, describe-screen, screenshot, accessibility-snapshot, cdp	53–8001	0 (no observations key), all platforms
click --testid unlock-submit observations, ext / iOS / Android	yes	yes (71 / 4638 / 2355 ms)

Notes from the proof run

• Host load was 7.7–18.4 during the runs, so mobile toolMs varies a lot from run to run.
• Android discovery toolMs went up in the after run (list-testids p50 5975 → 11639 ms). This change does not touch discovery tools. The likely cause is variation in the ~1 MB Android a11y snapshot (the same area as the existing list-testids timeout). Totals still fell because observation time is now 0.
• On Android, some mutating calls (wait-for, dismiss-keyboard) still hit the 8 s observation timeout and return no observations. This already happened before the change (P1-04 build: iOS click-unlock 8003 ms).

Transcripts (ext, final commit)

$ yarn mm cdp Runtime.evaluate '{"expression":"1+1","returnByValue":true}'
{
  "method": "Runtime.evaluate",
  "result": {
    "result": {
      "type": "number",
      "value": 2,
      "description": "2"
    }
  },
  "meta": { "timing": { "queueMs": 0, "toolMs": 2, "observationMs": 0, "totalMs": 3 } }
}

click-unlock     obs=yes observationMs=16
describe-screen  obs=no  observationMs=0
get-state        obs=no  observationMs=0
list-testids     obs=no  observationMs=0
cdp-eval         obs=no  observationMs=0  bytes=266
screenshot       obs=no  observationMs=0

Unit tests

Test Files  46 passed (46)
     Tests  1406 passed (1406)
Type Errors no errors
All files | 96.92 | 93.37 | 94.85 | 97.1
yarn lint → exit 0

New or changed: per-category tests for shouldCollectObservations and shouldIncludeObservationsInResponse; endpoint tests for observationMs (0 for readonly, discovery and cdp; > 0 for mutating); read tools don't move the diff baseline; accessibility_snapshot resets it; the cdp category in the registry.