From 3590b32252c4d7e56a10754bfa70e65efb450df1 Mon Sep 17 00:00:00 2001 From: Lex Christopherson Date: Thu, 12 Mar 2026 23:38:16 -0600 Subject: [PATCH] feat(M002/S02): Action pipeline performance Tasks: - chore(M002/S02): auto-commit after complete-slice - chore(M002/S02): auto-commit after complete-slice - chore(M002/S02/T02): auto-commit after execute-task - chore(M002/S02/T02): auto-commit after execute-task - chore(M002/S02/T01): auto-commit after execute-task - fix: expand tool outputs by default and pull main before slice merge - chore(M002/S02): auto-commit after plan-slice - docs(S02): add slice plan Branch: gsd/M002/S02 --- .gsd/DECISIONS.md | 3 + .gsd/REQUIREMENTS.md | 78 ++++++++--------- .gsd/STATE.md | 6 +- .gsd/milestones/M002/M002-ROADMAP.md | 2 +- .gsd/milestones/M002/slices/S02/S02-PLAN.md | 56 ++++++++++++ .../M002/slices/S02/tasks/T01-PLAN.md | 67 +++++++++++++++ .../M002/slices/S02/tasks/T02-PLAN.md | 52 ++++++++++++ .../extensions/browser-tools/index.ts | 2 +- .../extensions/browser-tools/settle.ts | 85 ++++++++++++++++--- .../extensions/browser-tools/state.ts | 3 +- .../browser-tools/tools/interaction.ts | 44 +++++----- .../browser-tools/tools/navigation.ts | 4 +- .../extensions/browser-tools/tools/refs.ts | 18 ++-- src/resources/extensions/gsd/git-service.ts | 3 + 14 files changed, 331 insertions(+), 92 deletions(-) create mode 100644 .gsd/milestones/M002/slices/S02/S02-PLAN.md create mode 100644 .gsd/milestones/M002/slices/S02/tasks/T01-PLAN.md create mode 100644 .gsd/milestones/M002/slices/S02/tasks/T02-PLAN.md diff --git a/.gsd/DECISIONS.md b/.gsd/DECISIONS.md index 77d7ac31f..917246d88 100644 --- a/.gsd/DECISIONS.md +++ b/.gsd/DECISIONS.md @@ -22,3 +22,6 @@ | D014 | M002/S01 | pattern | ToolDeps interface location | Defined in state.ts alongside types it references | Keeps the dependency graph simple — tool files import state.ts for ToolDeps + types. | Yes — could move to separate types.ts if state.ts grows | | D015 | M002/S01 | pattern | Factory pattern for lifecycle-dependent utils | createGetLivePagesSnapshot(ensureBrowser) instead of direct import | Avoids circular dependency between utils.ts and lifecycle.ts. Wired at orchestrator level. | No | | D016 | M002/S01 | pattern | Tool file import strategy | Tool files import state accessors and core.js functions directly — ToolDeps carries only infrastructure functions needing lifecycle wiring | Keeps ToolDeps lean. State accessors are stable imports, not runtime-wired dependencies. Avoids bloating the deps interface with every utility. | Yes — if ToolDeps grows unwieldy | +| D017 | M002/S02 | pattern | Action tool signal classification | High-signal: click, type, key_press, select_option, set_checked, navigate, click_ref, fill_ref. Low-signal: scroll, hover, drag, upload_file, hover_ref. | High-signal tools produce meaningful page changes worth capturing body text for diffs. Low-signal tools don't change page content. fill_ref is high-signal because input value changes affect form state. | Yes — if new tools need reclassification | +| D018 | M002/S02 | pattern | postActionSummary retention | Keep postActionSummary in capture.ts for summary-only tools (go_back, go_forward, reload) but remove from action tools that do before/after diff | Summary-only tools don't do diffs and don't need beforeState — postActionSummary is the right abstraction for them. Action tools need consolidated capture. | Yes — could remove entirely if summary-only tools get before/after diff | +| D019 | M002/S02 | tuning | Zero-mutation settle thresholds | 60ms detection window, 30ms shortened quiet window, totalMutationsSeen === 0 required | Conservative thresholds — 60ms is enough time for any async DOM update to start, 30ms shortened window still catches late mutations. Requiring zero total mutations (not just current poll) prevents false short-circuits. | Yes — if real-world testing shows 60ms is too short for slow SPAs | diff --git a/.gsd/REQUIREMENTS.md b/.gsd/REQUIREMENTS.md index bea130731..78eb866dc 100644 --- a/.gsd/REQUIREMENTS.md +++ b/.gsd/REQUIREMENTS.md @@ -4,39 +4,6 @@ This file is the explicit capability and coverage contract for the project. ## Active -### R017 — Consolidated state capture per action -- Class: core-capability -- Status: active -- Description: The before-state capture, after-state capture, post-action summary, and recent-error check are consolidated into fewer page.evaluate calls per action. Target: ~50-100ms savings per action. -- Why it matters: Every action tool currently runs 3-4 separate page.evaluate calls for state capture. Consolidating them reduces latency on every single browser interaction. -- Source: user -- Primary owning slice: M002/S02 -- Supporting slices: M002/S01 -- Validation: unmapped -- Notes: captureCompactPageState and postActionSummary can likely be merged into a single evaluate. - -### R018 — Conditional body text capture -- Class: core-capability -- Status: active -- Description: Body text capture (includeBodyText: true) is skipped for low-signal actions (scroll, hover, Tab key press) and enabled for high-signal actions (navigate, click, type, submit). -- Why it matters: Capturing 4000 chars of body text on every scroll or hover is wasteful. Conditional capture reduces evaluate overhead for frequent low-signal actions. -- Source: user -- Primary owning slice: M002/S02 -- Supporting slices: none -- Validation: unmapped -- Notes: Requires classifying each tool as high-signal or low-signal. - -### R019 — Faster settle on zero mutations -- Class: core-capability -- Status: active -- Description: settleAfterActionAdaptive short-circuits with a smaller quiet window when no mutation observer fires in the first 60ms. Target: ~40-80ms savings on zero-mutation actions. -- Why it matters: Many SPA interactions produce no DOM changes. The current settle logic always waits the full quiet window regardless. Short-circuiting saves time on the most common case. -- Source: user -- Primary owning slice: M002/S02 -- Supporting slices: none -- Validation: unmapped -- Notes: Track whether any mutation fired at all; if zero after 60ms, use a shorter quiet window. - ### R020 — Sharp-based screenshot resizing - Class: core-capability - Status: active @@ -116,6 +83,39 @@ This file is the explicit capability and coverage contract for the project. ## Validated +### R017 — Consolidated state capture per action +- Class: core-capability +- Status: validated +- Description: The before-state capture, after-state capture, post-action summary, and recent-error check are consolidated into fewer page.evaluate calls per action. Target: ~50-100ms savings per action. +- Why it matters: Every action tool currently runs 3-4 separate page.evaluate calls for state capture. Consolidating them reduces latency on every single browser interaction. +- Source: user +- Primary owning slice: M002/S02 +- Supporting slices: M002/S01 +- Validation: postActionSummary eliminated from action tools (grep returns 0 in interaction.ts), countOpenDialogs removed from ToolDeps, high-signal tools use single captureCompactPageState + formatCompactStateSummary pattern. Build passes. +- Notes: captureCompactPageState and postActionSummary can likely be merged into a single evaluate. + +### R018 — Conditional body text capture +- Class: core-capability +- Status: validated +- Description: Body text capture (includeBodyText: true) is skipped for low-signal actions (scroll, hover, Tab key press) and enabled for high-signal actions (navigate, click, type, submit). +- Why it matters: Capturing 4000 chars of body text on every scroll or hover is wasteful. Conditional capture reduces evaluate overhead for frequent low-signal actions. +- Source: user +- Primary owning slice: M002/S02 +- Supporting slices: none +- Validation: grep shows explicit includeBodyText: true for 5 high-signal tools and includeBodyText: false for 4 low-signal tools in interaction.ts. Classification codified in D017. Build passes. +- Notes: Requires classifying each tool as high-signal or low-signal. + +### R019 — Faster settle on zero mutations +- Class: core-capability +- Status: validated +- Description: settleAfterActionAdaptive short-circuits with a smaller quiet window when no mutation observer fires in the first 60ms. Target: ~40-80ms savings on zero-mutation actions. +- Why it matters: Many SPA interactions produce no DOM changes. The current settle logic always waits the full quiet window regardless. Short-circuiting saves time on the most common case. +- Source: user +- Primary owning slice: M002/S02 +- Supporting slices: none +- Validation: zero_mutation_shortcut settle reason in state.ts type union and settle.ts return path. Combined readSettleState() poll evaluate. 60ms/30ms thresholds codified in D019. Build passes. +- Notes: Track whether any mutation fired at all; if zero after 60ms, use a shorter quiet window. + ### R015 — Module decomposition of browser-tools - Class: quality-attribute - Status: validated @@ -338,9 +338,9 @@ This file is the explicit capability and coverage contract for the project. | R014 | anti-feature | out-of-scope | none | none | n/a | | R015 | quality-attribute | validated | M002/S01 | none | jiti load, 43 tools register, slim index, browser spot-check | | R016 | quality-attribute | validated | M002/S01 | none | window.__pi injection, zero inline redeclarations, survives navigation | -| R017 | core-capability | active | M002/S02 | M002/S01 | unmapped | -| R018 | core-capability | active | M002/S02 | none | unmapped | -| R019 | core-capability | active | M002/S02 | none | unmapped | +| R017 | core-capability | validated | M002/S02 | M002/S01 | postActionSummary eliminated, countOpenDialogs removed from ToolDeps, consolidated capture pattern | +| R018 | core-capability | validated | M002/S02 | none | explicit includeBodyText true/false per tool signal level, classification in D017 | +| R019 | core-capability | validated | M002/S02 | none | zero_mutation_shortcut settle reason, combined readSettleState poll, 60ms/30ms thresholds in D019 | | R020 | core-capability | active | M002/S03 | M002/S01 | unmapped | | R021 | core-capability | active | M002/S03 | none | unmapped | | R022 | core-capability | active | M002/S04 | M002/S01 | unmapped | @@ -353,8 +353,8 @@ This file is the explicit capability and coverage contract for the project. ## Coverage Summary -- Active requirements: 10 -- Validated requirements: 12 +- Active requirements: 7 +- Validated requirements: 15 - Deferred requirements: 3 - Out of scope: 3 -- Unmapped active requirements: 10 +- Unmapped active requirements: 7 diff --git a/.gsd/STATE.md b/.gsd/STATE.md index 4711408ef..3c6f15cc4 100644 --- a/.gsd/STATE.md +++ b/.gsd/STATE.md @@ -1,9 +1,9 @@ # GSD State **Active Milestone:** M002 — Browser Tools Performance & Intelligence -**Active Slice:** S02 — Action pipeline performance +**Active Slice:** S03 — Screenshot pipeline **Phase:** planning -**Requirements Status:** 10 active · 12 validated · 3 deferred · 3 out of scope +**Requirements Status:** 7 active · 15 validated · 3 deferred · 3 out of scope ## Milestone Registry - ✅ **M001:** Proactive Secret Management @@ -16,4 +16,4 @@ - None ## Next Action -Plan slice S02 (Action pipeline performance). +Plan slice S03 (Screenshot pipeline). diff --git a/.gsd/milestones/M002/M002-ROADMAP.md b/.gsd/milestones/M002/M002-ROADMAP.md index c96575fb5..dee536e26 100644 --- a/.gsd/milestones/M002/M002-ROADMAP.md +++ b/.gsd/milestones/M002/M002-ROADMAP.md @@ -61,7 +61,7 @@ This milestone is complete only when all are true: - [x] **S01: Module decomposition and shared evaluate utilities** `risk:high` `depends:[]` > After this: all 43 existing browser tools work identically with the new module structure; shared browser-side utilities (cssPath, simpleHash, isVisible, isEnabled, inferRole, accessibleName) are injected once via addInitScript and used by buildRefSnapshot and resolveRefTarget — verified by build success and spot-check against a real page. -- [ ] **S02: Action pipeline performance** `risk:medium` `depends:[S01]` +- [x] **S02: Action pipeline performance** `risk:medium` `depends:[S01]` > After this: captureCompactPageState and postActionSummary are consolidated into fewer evaluate calls per action; settleAfterActionAdaptive short-circuits on zero-mutation actions; low-signal actions (scroll, hover, Tab) skip body text capture — verified by build success and behavioral spot-check. - [ ] **S03: Screenshot pipeline** `risk:low` `depends:[S01]` diff --git a/.gsd/milestones/M002/slices/S02/S02-PLAN.md b/.gsd/milestones/M002/slices/S02/S02-PLAN.md new file mode 100644 index 000000000..1f69a5275 --- /dev/null +++ b/.gsd/milestones/M002/slices/S02/S02-PLAN.md @@ -0,0 +1,56 @@ +# S02: Action pipeline performance + +**Goal:** Reduce per-action evaluate overhead by consolidating state capture, short-circuiting settle on zero mutations, and skipping body text for low-signal actions. +**Demo:** Build succeeds. A browser_click action runs 3 fewer evaluate calls than before (5+N vs 8+N). Settle returns `zero_mutation_shortcut` reason when no mutations fire. Low-signal tools (scroll, hover, drag) skip body text capture. + +## Must-Haves + +- `postActionSummary` eliminated from high-signal tools — replaced by `captureCompactPageState` + `formatCompactStateSummary` +- `countOpenDialogs` removed as standalone call — dialog count comes from `captureCompactPageState`'s existing `dialog.count` field +- High-signal tools (click, type, key_press, select_option, set_checked, navigate) capture body text in afterState +- Low-signal tools (scroll, hover, drag, upload_file, hover_ref) skip body text in `captureCompactPageState` +- `settleAfterActionAdaptive` short-circuits with `zero_mutation_shortcut` settle reason when no mutations fire in the first 60ms +- `AdaptiveSettleDetails.settleReason` type includes `"zero_mutation_shortcut"` +- `readMutationCounter` + `readFocusedDescriptor` combined into single evaluate per settle poll +- Build succeeds via `npm run build` + +## Proof Level + +- This slice proves: operational + behavioral +- Real runtime required: no (build verification sufficient — behavioral improvements are structural, not observable without timing instrumentation) +- Human/UAT required: no + +## Verification + +- `npm run build` succeeds with zero errors +- `grep -c "countOpenDialogs" src/resources/extensions/browser-tools/tools/*.ts` returns 0 (no standalone dialog counting in tool files) +- `grep -c "postActionSummary" src/resources/extensions/browser-tools/tools/interaction.ts` returns 0 for high-signal tools that now use direct capture +- `grep "zero_mutation_shortcut" src/resources/extensions/browser-tools/settle.ts` finds the new settle reason +- `grep "includeBodyText" src/resources/extensions/browser-tools/tools/interaction.ts` shows explicit true/false per tool signal level + +## Tasks + +- [x] **T01: Consolidate capture pipeline and classify tool signal levels** `est:45m` + - Why: R017 + R018 — eliminate redundant evaluate calls per action by removing the `postActionSummary` + separate `captureCompactPageState` pattern in high-signal tools, folding `countOpenDialogs` into the existing `dialog.count` from captureCompactPageState, and classifying tools as high/low signal for body text capture. + - Files: `capture.ts`, `state.ts`, `utils.ts`, `index.ts`, `tools/interaction.ts`, `tools/navigation.ts`, `tools/refs.ts` + - Do: (1) Remove `postActionSummary` from ToolDeps — high-signal tools call `captureCompactPageState(includeBodyText: true)` once for afterState and derive summary via `formatCompactStateSummary`. Low-signal tools call `captureCompactPageState(includeBodyText: false)` and derive summary. (2) Remove standalone `countOpenDialogs` calls from tool files — use `afterState.dialog.count` / `beforeState.dialog.count` from the state already captured. (3) Keep `postActionSummary` function in capture.ts but remove it from ToolDeps and stop using it in action tools. Summary-only tools (go_back, go_forward, reload) can keep calling it since they don't do before/after diff. (4) Update ToolDeps interface. (5) Build verify. + - Verify: `npm run build` succeeds. `grep -c "countOpenDialogs" src/resources/extensions/browser-tools/tools/*.ts` returns 0. High-signal tools in interaction.ts have `includeBodyText: true` in afterState capture and no `postActionSummary` call. + - Done when: Build passes and high-signal tools use consolidated capture with explicit body text classification. + +- [x] **T02: Settle zero-mutation short-circuit and poll consolidation** `est:25m` + - Why: R019 — save ~50ms on zero-mutation actions by short-circuiting the settle quiet window, and reduce per-poll evaluate calls by combining readMutationCounter + readFocusedDescriptor into one evaluate. + - Files: `settle.ts`, `state.ts` + - Do: (1) Add `"zero_mutation_shortcut"` to `AdaptiveSettleDetails.settleReason` union in state.ts. (2) In `settleAfterActionAdaptive`, track whether any mutation has fired since start. After 60ms with zero mutations, switch to a 30ms quiet window instead of 100ms and return `zero_mutation_shortcut` reason. (3) Combine `readMutationCounter` + `readFocusedDescriptor` into a single `readSettleState(target, checkFocus)` evaluate that returns `{ mutationCount, focusDescriptor }`. Replace per-poll sequential evaluates with this combined call. (4) Build verify. + - Verify: `npm run build` succeeds. `grep "zero_mutation_shortcut" src/resources/extensions/browser-tools/settle.ts` finds the new reason. The combined poll evaluate is a single `target.evaluate()` call returning both mutation count and focus descriptor. + - Done when: Build passes. Settle logic has zero-mutation short-circuit and combined poll evaluate. + +## Files Likely Touched + +- `src/resources/extensions/browser-tools/capture.ts` +- `src/resources/extensions/browser-tools/settle.ts` +- `src/resources/extensions/browser-tools/state.ts` +- `src/resources/extensions/browser-tools/utils.ts` +- `src/resources/extensions/browser-tools/index.ts` +- `src/resources/extensions/browser-tools/tools/interaction.ts` +- `src/resources/extensions/browser-tools/tools/navigation.ts` +- `src/resources/extensions/browser-tools/tools/refs.ts` diff --git a/.gsd/milestones/M002/slices/S02/tasks/T01-PLAN.md b/.gsd/milestones/M002/slices/S02/tasks/T01-PLAN.md new file mode 100644 index 000000000..8b5666843 --- /dev/null +++ b/.gsd/milestones/M002/slices/S02/tasks/T01-PLAN.md @@ -0,0 +1,67 @@ +--- +estimated_steps: 5 +estimated_files: 7 +--- + +# T01: Consolidate capture pipeline and classify tool signal levels + +**Slice:** S02 — Action pipeline performance +**Milestone:** M002 + +## Description + +Eliminate redundant evaluate round-trips per action by consolidating the capture pipeline. Currently high-signal tools call `postActionSummary` (which internally calls `captureCompactPageState` without body text) and then call `captureCompactPageState` again with `includeBodyText: true` — two evaluate calls for overlapping data. Additionally, tools call `countOpenDialogs` separately even though `captureCompactPageState` already captures `dialog.count`. + +After this task: high-signal tools (click, type, key_press, select_option, set_checked, navigate) call `captureCompactPageState(includeBodyText: true)` once for afterState, derive the summary via `formatCompactStateSummary`, and read `dialog.count` from the captured state. Low-signal tools (scroll, hover, drag, upload_file) call `captureCompactPageState(includeBodyText: false)` and derive summary. Net saving: 3 evaluate round-trips per high-signal action. + +## Steps + +1. **Update ToolDeps in state.ts**: Remove `countOpenDialogs` from ToolDeps. `postActionSummary` stays in ToolDeps for now since summary-only tools (go_back, go_forward, reload) still use it — but action tools won't call it. + +2. **Refactor high-signal tools in interaction.ts**: For `browser_click`, `browser_type`, `browser_key_press`, `browser_select_option`, `browser_set_checked`: + - Remove the `postActionSummary` call + - Remove standalone `countOpenDialogs` calls — use `beforeState.dialog.count` and `afterState.dialog.count` instead + - After settle, call `captureCompactPageState(p, { ..., includeBodyText: true })` once for afterState + - Derive summary text via `deps.formatCompactStateSummary(afterState)` + - The beforeState capture already has `dialog.count` — use it directly for dialog comparison + +3. **Refactor browser_navigate in navigation.ts**: Same pattern — remove `postActionSummary`, use afterState (already captured) for summary via `formatCompactStateSummary`, use `dialog.count` from state. + +4. **Refactor ref action tools in refs.ts**: For `browser_click_ref` — remove `countOpenDialogs` calls, use state's `dialog.count`. For `browser_click_ref`, `browser_hover_ref`, `browser_fill_ref` — replace `postActionSummary` with `captureCompactPageState` + `formatCompactStateSummary`. Mark ref action tools with explicit body text classification: `browser_click_ref` and `browser_fill_ref` get `includeBodyText: true` (high-signal), `browser_hover_ref` gets `includeBodyText: false` (low-signal). + +5. **Classify low-signal tools in interaction.ts**: For `browser_scroll`, `browser_hover`, `browser_drag`, `browser_upload_file` — replace `postActionSummary` with `captureCompactPageState(includeBodyText: false)` + `formatCompactStateSummary`. This makes the signal classification explicit in code. + +## Must-Haves + +- [ ] No standalone `countOpenDialogs` calls in any tool file under `tools/` +- [ ] High-signal tools call `captureCompactPageState` with `includeBodyText: true` for afterState and derive summary via `formatCompactStateSummary` +- [ ] Low-signal tools call `captureCompactPageState` with `includeBodyText: false` and derive summary via `formatCompactStateSummary` +- [ ] `postActionSummary` remains available in ToolDeps for summary-only navigation tools (go_back, go_forward, reload) — these don't do before/after diff +- [ ] `countOpenDialogs` removed from ToolDeps interface and index.ts wiring +- [ ] `npm run build` succeeds + +## Verification + +- `npm run build` exits 0 +- `grep -c "countOpenDialogs" src/resources/extensions/browser-tools/tools/*.ts` returns 0 for every file +- `grep -c "postActionSummary" src/resources/extensions/browser-tools/tools/interaction.ts` returns 0 +- `grep "includeBodyText: false" src/resources/extensions/browser-tools/tools/interaction.ts` shows low-signal tools explicitly skipping body text +- `grep "includeBodyText: true" src/resources/extensions/browser-tools/tools/interaction.ts` shows high-signal tools explicitly including body text + +## Inputs + +- `src/resources/extensions/browser-tools/capture.ts` — `captureCompactPageState` and `postActionSummary` implementations +- `src/resources/extensions/browser-tools/state.ts` — ToolDeps interface, CompactPageState shape (includes `dialog.count`) +- `src/resources/extensions/browser-tools/utils.ts` — `formatCompactStateSummary`, `countOpenDialogs` +- `src/resources/extensions/browser-tools/tools/interaction.ts` — 10 interaction tools with current capture patterns +- `src/resources/extensions/browser-tools/tools/navigation.ts` — browser_navigate with postActionSummary + separate afterState capture +- `src/resources/extensions/browser-tools/tools/refs.ts` — click_ref/hover_ref/fill_ref with countOpenDialogs and postActionSummary +- S01 summary — module structure, ToolDeps contract, accessor patterns + +## Expected Output + +- `src/resources/extensions/browser-tools/state.ts` — ToolDeps without `countOpenDialogs` +- `src/resources/extensions/browser-tools/index.ts` — wiring without `countOpenDialogs` +- `src/resources/extensions/browser-tools/tools/interaction.ts` — all 10 tools using consolidated capture with explicit signal classification +- `src/resources/extensions/browser-tools/tools/navigation.ts` — browser_navigate using consolidated capture +- `src/resources/extensions/browser-tools/tools/refs.ts` — ref action tools using consolidated capture with signal classification diff --git a/.gsd/milestones/M002/slices/S02/tasks/T02-PLAN.md b/.gsd/milestones/M002/slices/S02/tasks/T02-PLAN.md new file mode 100644 index 000000000..7798ddc07 --- /dev/null +++ b/.gsd/milestones/M002/slices/S02/tasks/T02-PLAN.md @@ -0,0 +1,52 @@ +--- +estimated_steps: 3 +estimated_files: 2 +--- + +# T02: Settle zero-mutation short-circuit and poll consolidation + +**Slice:** S02 — Action pipeline performance +**Milestone:** M002 + +## Description + +Save ~50ms on zero-mutation actions by short-circuiting the settle quiet window, and reduce per-poll evaluate overhead by combining `readMutationCounter` and `readFocusedDescriptor` into a single evaluate call. + +Currently `settleAfterActionAdaptive` runs the full 100ms quiet window even when zero mutations have occurred. For actions like scroll, hover, or clicking static elements, this is wasted time. After 60ms with no mutation counter increment, the quiet window drops to 30ms. + +Additionally, each poll iteration runs `readMutationCounter` (1 evaluate) and optionally `readFocusedDescriptor` (1 evaluate) sequentially. Combining them into one evaluate saves 1 round-trip per poll iteration (typically 2-4 polls per settle). + +## Steps + +1. **Add settle reason to type in state.ts**: Extend `AdaptiveSettleDetails.settleReason` union to include `"zero_mutation_shortcut"`. + +2. **Create combined poll evaluate in settle.ts**: Replace separate `readMutationCounter` + `readFocusedDescriptor` calls in the poll loop with a single `readSettleState(target, checkFocus)` function that returns `{ mutationCount: number; focusDescriptor: string }` from one `target.evaluate()`. When `checkFocus` is false, return empty string for focusDescriptor. Keep the standalone `readMutationCounter` and `readFocusedDescriptor` exports for other consumers (interaction.ts imports `readFocusedDescriptor` directly for key_press before/after focus comparison). + +3. **Implement zero-mutation short-circuit in settleAfterActionAdaptive**: Track `totalMutationsSeen` (sum of all mutation increments across polls). After 60ms, if `totalMutationsSeen === 0`, switch `quietWindowMs` to 30ms. When settle completes under this condition, return `settleReason: "zero_mutation_shortcut"`. The initial `ensureMutationCounter` + first `readMutationCounter` call before the loop should also be combined into the loop's first iteration where possible (use the combined evaluate). + +## Must-Haves + +- [ ] `AdaptiveSettleDetails.settleReason` union includes `"zero_mutation_shortcut"` +- [ ] Combined poll evaluate reads mutation counter + focus descriptor in one `evaluate()` call +- [ ] Zero-mutation short-circuit: after 60ms with no mutations, quiet window reduces to 30ms +- [ ] Settle returns `"zero_mutation_shortcut"` reason when short-circuit path is taken +- [ ] Standalone `readMutationCounter` and `readFocusedDescriptor` exports preserved for external consumers +- [ ] `npm run build` succeeds + +## Verification + +- `npm run build` exits 0 +- `grep "zero_mutation_shortcut" src/resources/extensions/browser-tools/settle.ts` finds the new reason +- `grep "zero_mutation_shortcut" src/resources/extensions/browser-tools/state.ts` finds it in the type union +- The poll loop body contains a single `evaluate()` call (not two sequential ones) + +## Inputs + +- `src/resources/extensions/browser-tools/settle.ts` — current `settleAfterActionAdaptive`, `readMutationCounter`, `readFocusedDescriptor` +- `src/resources/extensions/browser-tools/state.ts` — `AdaptiveSettleDetails` interface +- S02 Research — settle timing analysis and proposed thresholds + +## Expected Output + +- `src/resources/extensions/browser-tools/settle.ts` — combined poll evaluate, zero-mutation short-circuit, new settle reason +- `src/resources/extensions/browser-tools/state.ts` — updated `AdaptiveSettleDetails.settleReason` type diff --git a/src/resources/extensions/browser-tools/index.ts b/src/resources/extensions/browser-tools/index.ts index 6d93ce5fc..f1bb2cd02 100644 --- a/src/resources/extensions/browser-tools/index.ts +++ b/src/resources/extensions/browser-tools/index.ts @@ -28,7 +28,7 @@ export default function (pi: ExtensionAPI) { truncateText: u.truncateText, verificationFromChecks: u.verificationFromChecks, verificationLine: u.verificationLine, collectAssertionState: u.collectAssertionState, formatAssertionText: u.formatAssertionText, formatDiffText: u.formatDiffText, - getUrlHash: u.getUrlHash, countOpenDialogs: u.countOpenDialogs, + getUrlHash: u.getUrlHash, captureClickTargetState: u.captureClickTargetState, readInputLikeValue: u.readInputLikeValue, firstErrorLine: u.firstErrorLine, captureAccessibilityMarkdown: u.captureAccessibilityMarkdown, resolveAccessibilityScope: u.resolveAccessibilityScope, diff --git a/src/resources/extensions/browser-tools/settle.ts b/src/resources/extensions/browser-tools/settle.ts index 7f1aca1f5..592b4f866 100644 --- a/src/resources/extensions/browser-tools/settle.ts +++ b/src/resources/extensions/browser-tools/settle.ts @@ -66,17 +66,53 @@ export async function readFocusedDescriptor(target: Page | Frame): Promise { + try { + return await target.evaluate((wantFocus: boolean) => { + const w = window as unknown as Record; + const mutationCount = typeof w.__piMutationCounter === "number" ? (w.__piMutationCounter as number) : 0; + if (!wantFocus) return { mutationCount, focusDescriptor: "" }; + const el = document.activeElement as HTMLElement | null; + if (!el || el === document.body || el === document.documentElement) { + return { mutationCount, focusDescriptor: "" }; + } + const id = el.id ? `#${el.id}` : ""; + const role = el.getAttribute("role") || ""; + const name = (el.getAttribute("aria-label") || el.getAttribute("name") || "").trim(); + return { mutationCount, focusDescriptor: `${el.tagName.toLowerCase()}${id}|${role}|${name}` }; + }, checkFocus); + } catch { + return { mutationCount: 0, focusDescriptor: "" }; + } +} + // --------------------------------------------------------------------------- // Adaptive settle // --------------------------------------------------------------------------- +/** Threshold (ms) after which zero mutations triggers a shortened quiet window. */ +const ZERO_MUTATION_THRESHOLD_MS = 60; +/** Shortened quiet window when no mutations have been observed. */ +const ZERO_MUTATION_QUIET_MS = 30; + export async function settleAfterActionAdaptive( p: Page, opts: AdaptiveSettleOptions = {}, ): Promise { const timeoutMs = Math.max(150, opts.timeoutMs ?? 500); const pollMs = Math.min(100, Math.max(20, opts.pollMs ?? 40)); - const quietWindowMs = Math.max(60, opts.quietWindowMs ?? 100); + const baseQuietWindowMs = Math.max(60, opts.quietWindowMs ?? 100); const checkFocus = opts.checkFocusStability ?? false; const startedAt = Date.now(); @@ -84,10 +120,16 @@ export async function settleAfterActionAdaptive( let sawUrlChange = false; let lastActivityAt = startedAt; let previousUrl = p.url(); + let totalMutationsSeen = 0; + let activeQuietWindowMs = baseQuietWindowMs; + // Install mutation counter + read initial state in one evaluate sequence. + // ensureMutationCounter must run first (installs the observer), then we + // read the baseline via the combined reader. await ensureMutationCounter(p).catch(() => {}); - let previousMutationCount = await readMutationCounter(p); - let previousFocus = checkFocus ? await readFocusedDescriptor(p) : ""; + const initial = await readSettleState(p, checkFocus); + let previousMutationCount = initial.mutationCount; + let previousFocus = initial.focusDescriptor; while (Date.now() - startedAt < timeoutMs) { await new Promise((resolve) => setTimeout(resolve, pollMs)); @@ -101,18 +143,18 @@ export async function settleAfterActionAdaptive( lastActivityAt = now; } - const currentMutationCount = await readMutationCounter(p); - if (currentMutationCount > previousMutationCount) { - previousMutationCount = currentMutationCount; + // Single combined evaluate for mutation count + focus descriptor. + const state = await readSettleState(p, checkFocus); + + if (state.mutationCount > previousMutationCount) { + totalMutationsSeen += state.mutationCount - previousMutationCount; + previousMutationCount = state.mutationCount; lastActivityAt = now; } - if (checkFocus) { - const currentFocus = await readFocusedDescriptor(p); - if (currentFocus !== previousFocus) { - previousFocus = currentFocus; - lastActivityAt = now; - } + if (checkFocus && state.focusDescriptor !== previousFocus) { + previousFocus = state.focusDescriptor; + lastActivityAt = now; } const pendingCritical = getPendingCriticalRequests(p); @@ -121,11 +163,26 @@ export async function settleAfterActionAdaptive( continue; } - if (now - lastActivityAt >= quietWindowMs) { + // Zero-mutation short-circuit: after ZERO_MUTATION_THRESHOLD_MS with + // no mutations observed at all, reduce the quiet window to settle faster. + if ( + totalMutationsSeen === 0 && + now - startedAt >= ZERO_MUTATION_THRESHOLD_MS && + activeQuietWindowMs !== ZERO_MUTATION_QUIET_MS + ) { + activeQuietWindowMs = ZERO_MUTATION_QUIET_MS; + } + + if (now - lastActivityAt >= activeQuietWindowMs) { + const usedShortcut = activeQuietWindowMs === ZERO_MUTATION_QUIET_MS && totalMutationsSeen === 0; return { settleMode: "adaptive", settleMs: now - startedAt, - settleReason: sawUrlChange ? "url_changed_then_quiet" : "dom_quiet", + settleReason: usedShortcut + ? "zero_mutation_shortcut" + : sawUrlChange + ? "url_changed_then_quiet" + : "dom_quiet", settlePolls: polls, }; } diff --git a/src/resources/extensions/browser-tools/state.ts b/src/resources/extensions/browser-tools/state.ts index 00e8f9bad..0f36e6385 100644 --- a/src/resources/extensions/browser-tools/state.ts +++ b/src/resources/extensions/browser-tools/state.ts @@ -163,7 +163,7 @@ export interface AdaptiveSettleOptions { export interface AdaptiveSettleDetails { settleMode: "adaptive"; settleMs: number; - settleReason: "dom_quiet" | "url_changed_then_quiet" | "timeout_fallback"; + settleReason: "dom_quiet" | "url_changed_then_quiet" | "timeout_fallback" | "zero_mutation_shortcut"; settlePolls: number; } @@ -389,7 +389,6 @@ export interface ToolDeps { formatAssertionText: (result: ReturnType) => string; formatDiffText: (diff: ReturnType) => string; getUrlHash: (url: string) => string; - countOpenDialogs: (target: Page | Frame) => Promise; captureClickTargetState: (target: Page | Frame, selector: string) => Promise; readInputLikeValue: (target: Page | Frame, selector?: string) => Promise; firstErrorLine: (err: unknown) => string; diff --git a/src/resources/extensions/browser-tools/tools/interaction.ts b/src/resources/extensions/browser-tools/tools/interaction.ts index 3bce42302..d9c594ba5 100644 --- a/src/resources/extensions/browser-tools/tools/interaction.ts +++ b/src/resources/extensions/browser-tools/tools/interaction.ts @@ -38,7 +38,6 @@ export function registerInteractionTools(pi: ExtensionAPI, deps: ToolDeps): void actionId = deps.beginTrackedAction("browser_click", params, beforeState.url).id; const beforeUrl = p.url(); const beforeHash = deps.getUrlHash(beforeUrl); - const beforeDialogCount = await deps.countOpenDialogs(target); const beforeTargetState = params.selector ? await deps.captureClickTargetState(target, params.selector) : null; @@ -85,9 +84,9 @@ export function registerInteractionTools(pi: ExtensionAPI, deps: ToolDeps): void const settle = await deps.settleAfterActionAdaptive(p); - const url = p.url(); + const afterState = await deps.captureCompactPageState(p, { selectors: params.selector ? [params.selector] : [], includeBodyText: true, target }); + const url = afterState.url; const hash = deps.getUrlHash(url); - const afterDialogCount = await deps.countOpenDialogs(target); const afterTargetState = params.selector ? await deps.captureClickTargetState(target, params.selector) : null; @@ -103,14 +102,13 @@ export function registerInteractionTools(pi: ExtensionAPI, deps: ToolDeps): void { name: "url_changed", passed: url !== beforeUrl, value: url, expected: `!= ${beforeUrl}` }, { name: "hash_changed", passed: hash !== beforeHash, value: hash, expected: `!= ${beforeHash}` }, { name: "target_state_changed", passed: targetStateChanged, value: afterTargetState, expected: beforeTargetState }, - { name: "dialog_open", passed: afterDialogCount > beforeDialogCount, value: afterDialogCount, expected: `> ${beforeDialogCount}` }, + { name: "dialog_open", passed: afterState.dialog.count > beforeState!.dialog.count, value: afterState.dialog.count, expected: `> ${beforeState!.dialog.count}` }, ], "Try a more specific selector or click a clearly interactive element." ); const clickTarget = params.selector ?? `(${params.x}, ${params.y})`; - const summary = await deps.postActionSummary(p, target); + const summary = deps.formatCompactStateSummary(afterState); const jsErrors = deps.getRecentErrors(p.url()); - const afterState = await deps.captureCompactPageState(p, { selectors: params.selector ? [params.selector] : [], includeBodyText: true, target }); const diff = diffCompactStates(beforeState!, afterState); setLastActionBeforeState(beforeState!); setLastActionAfterState(afterState); @@ -171,7 +169,8 @@ export function registerInteractionTools(pi: ExtensionAPI, deps: ToolDeps): void await target.dragAndDrop(params.sourceSelector, params.targetSelector, { timeout: 10000 }); const settle = await deps.settleAfterActionAdaptive(p); - const summary = await deps.postActionSummary(p, target); + const afterState = await deps.captureCompactPageState(p, { includeBodyText: false, target }); + const summary = deps.formatCompactStateSummary(afterState); const jsErrors = deps.getRecentErrors(p.url()); return { @@ -340,9 +339,9 @@ export function registerInteractionTools(pi: ExtensionAPI, deps: ToolDeps): void "Try clearFirst=true, use a more specific selector, or set slowly=true for key-driven inputs." ); const typeTarget = params.selector ? ` into "${params.selector}"` : ""; - const summary = await deps.postActionSummary(p, target); - const jsErrors = deps.getRecentErrors(p.url()); const afterState = await deps.captureCompactPageState(p, { selectors: params.selector ? [params.selector] : [], includeBodyText: true, target }); + const summary = deps.formatCompactStateSummary(afterState); + const jsErrors = deps.getRecentErrors(p.url()); const diff = diffCompactStates(beforeState!, afterState); setLastActionBeforeState(beforeState!); setLastActionAfterState(afterState); @@ -404,7 +403,8 @@ export function registerInteractionTools(pi: ExtensionAPI, deps: ToolDeps): void await target.locator(params.selector).first().setInputFiles(cleanFiles); const settle = await deps.settleAfterActionAdaptive(p); - const summary = await deps.postActionSummary(p, target); + const afterState = await deps.captureCompactPageState(p, { includeBodyText: false, target }); + const summary = deps.formatCompactStateSummary(afterState); const jsErrors = deps.getRecentErrors(p.url()); return { @@ -457,7 +457,8 @@ export function registerInteractionTools(pi: ExtensionAPI, deps: ToolDeps): void const maxScroll = scrollInfo.scrollHeight - scrollInfo.clientHeight; const percent = maxScroll > 0 ? Math.round((scrollInfo.scrollY / maxScroll) * 100) : 0; - const summary = await deps.postActionSummary(p, target); + const afterState = await deps.captureCompactPageState(p, { includeBodyText: false, target }); + const summary = deps.formatCompactStateSummary(afterState); const jsErrors = deps.getRecentErrors(p.url()); return { @@ -502,7 +503,8 @@ export function registerInteractionTools(pi: ExtensionAPI, deps: ToolDeps): void await target.locator(params.selector).first().hover({ timeout: 10000 }); const settle = await deps.settleAfterActionAdaptive(p); - const summary = await deps.postActionSummary(p, target); + const afterState = await deps.captureCompactPageState(p, { includeBodyText: false, target }); + const summary = deps.formatCompactStateSummary(afterState); const jsErrors = deps.getRecentErrors(p.url()); return { @@ -549,26 +551,24 @@ export function registerInteractionTools(pi: ExtensionAPI, deps: ToolDeps): void actionId = deps.beginTrackedAction("browser_key_press", params, beforeState.url).id; const beforeUrl = p.url(); const beforeFocus = await readFocusedDescriptor(target); - const beforeDialogCount = await deps.countOpenDialogs(target); await p.keyboard.press(params.key); const settle = await deps.settleAfterActionAdaptive(p, { checkFocusStability: true }); - const afterUrl = p.url(); + const afterState = await deps.captureCompactPageState(p, { includeBodyText: true, target }); + const afterUrl = afterState.url; const afterFocus = await readFocusedDescriptor(target); - const afterDialogCount = await deps.countOpenDialogs(target); const verification = deps.verificationFromChecks( [ { name: "url_changed", passed: afterUrl !== beforeUrl, value: afterUrl, expected: `!= ${beforeUrl}` }, { name: "focus_changed", passed: afterFocus !== beforeFocus, value: afterFocus, expected: `!= ${beforeFocus}` }, - { name: "dialog_open", passed: afterDialogCount > beforeDialogCount, value: afterDialogCount, expected: `> ${beforeDialogCount}` }, + { name: "dialog_open", passed: afterState.dialog.count > beforeState!.dialog.count, value: afterState.dialog.count, expected: `> ${beforeState!.dialog.count}` }, ], "If this key should trigger UI changes, confirm focus is on the intended element first." ); - const summary = await deps.postActionSummary(p, target); + const summary = deps.formatCompactStateSummary(afterState); const jsErrors = deps.getRecentErrors(p.url()); - const afterState = await deps.captureCompactPageState(p, { includeBodyText: true, target }); const diff = diffCompactStates(beforeState!, afterState); setLastActionBeforeState(beforeState!); setLastActionAfterState(afterState); @@ -660,9 +660,9 @@ export function registerInteractionTools(pi: ExtensionAPI, deps: ToolDeps): void "Confirm whether the target select uses option label or value, then retry with that exact text." ); - const summary = await deps.postActionSummary(p, target); - const jsErrors = deps.getRecentErrors(p.url()); const afterState = await deps.captureCompactPageState(p, { selectors: [params.selector], includeBodyText: true, target }); + const summary = deps.formatCompactStateSummary(afterState); + const jsErrors = deps.getRecentErrors(p.url()); const diff = diffCompactStates(beforeState!, afterState); setLastActionBeforeState(beforeState!); setLastActionAfterState(afterState); @@ -741,9 +741,9 @@ export function registerInteractionTools(pi: ExtensionAPI, deps: ToolDeps): void ); const state = params.checked ? "checked" : "unchecked"; - const summary = await deps.postActionSummary(p, target); - const jsErrors = deps.getRecentErrors(p.url()); const afterState = await deps.captureCompactPageState(p, { selectors: [params.selector], includeBodyText: true, target }); + const summary = deps.formatCompactStateSummary(afterState); + const jsErrors = deps.getRecentErrors(p.url()); const diff = diffCompactStates(beforeState!, afterState); setLastActionBeforeState(beforeState!); setLastActionAfterState(afterState); diff --git a/src/resources/extensions/browser-tools/tools/navigation.ts b/src/resources/extensions/browser-tools/tools/navigation.ts index c8119e0be..376e37692 100644 --- a/src/resources/extensions/browser-tools/tools/navigation.ts +++ b/src/resources/extensions/browser-tools/tools/navigation.ts @@ -37,9 +37,9 @@ export function registerNavigationTools(pi: ExtensionAPI, deps: ToolDeps): void const url = p.url(); const viewport = p.viewportSize(); const vpText = viewport ? `${viewport.width}x${viewport.height}` : "unknown"; - const summary = await deps.postActionSummary(p); - const jsErrors = deps.getRecentErrors(p.url()); const afterState = await deps.captureCompactPageState(p, { includeBodyText: true }); + const summary = deps.formatCompactStateSummary(afterState); + const jsErrors = deps.getRecentErrors(p.url()); const diff = diffCompactStates(beforeState, afterState); setLastActionBeforeState(beforeState); setLastActionAfterState(afterState); diff --git a/src/resources/extensions/browser-tools/tools/refs.ts b/src/resources/extensions/browser-tools/tools/refs.ts index 98f7df846..1534f8bb3 100644 --- a/src/resources/extensions/browser-tools/tools/refs.ts +++ b/src/resources/extensions/browser-tools/tools/refs.ts @@ -257,16 +257,16 @@ export function registerRefTools(pi: ExtensionAPI, deps: ToolDeps): void { }; } - const beforeUrl = p.url(); + const beforeState = await deps.captureCompactPageState(p, { includeBodyText: true, target }); + const beforeUrl = beforeState.url; const beforeHash = deps.getUrlHash(beforeUrl); - const beforeDialogCount = await deps.countOpenDialogs(target); const beforeTargetState = await deps.captureClickTargetState(target, resolved.selector); await target.locator(resolved.selector).first().click({ timeout: 8000 }); const settle = await deps.settleAfterActionAdaptive(p); - const afterUrl = p.url(); + const afterState = await deps.captureCompactPageState(p, { includeBodyText: true, target }); + const afterUrl = afterState.url; const afterHash = deps.getUrlHash(afterUrl); - const afterDialogCount = await deps.countOpenDialogs(target); const afterTargetState = await deps.captureClickTargetState(target, resolved.selector); const targetStateChanged = beforeTargetState.exists !== afterTargetState.exists || @@ -279,12 +279,12 @@ export function registerRefTools(pi: ExtensionAPI, deps: ToolDeps): void { { name: "url_changed", passed: afterUrl !== beforeUrl, value: afterUrl, expected: `!= ${beforeUrl}` }, { name: "hash_changed", passed: afterHash !== beforeHash, value: afterHash, expected: `!= ${beforeHash}` }, { name: "target_state_changed", passed: targetStateChanged, value: afterTargetState, expected: beforeTargetState }, - { name: "dialog_open", passed: afterDialogCount > beforeDialogCount, value: afterDialogCount, expected: `> ${beforeDialogCount}` }, + { name: "dialog_open", passed: afterState.dialog.count > beforeState.dialog.count, value: afterState.dialog.count, expected: `> ${beforeState.dialog.count}` }, ], "Ref may now point to an inert element. Refresh refs with browser_snapshot_refs and retry." ); - const summary = await deps.postActionSummary(p, target); + const summary = deps.formatCompactStateSummary(afterState); const jsErrors = deps.getRecentErrors(p.url()); const versionedRef = deps.formatVersionedRef(refMetadata?.version ?? refVersion, node.ref); return { @@ -377,7 +377,8 @@ export function registerRefTools(pi: ExtensionAPI, deps: ToolDeps): void { await target.locator(resolved.selector).first().hover({ timeout: 8000 }); const settle = await deps.settleAfterActionAdaptive(p); - const summary = await deps.postActionSummary(p, target); + const afterState = await deps.captureCompactPageState(p, { includeBodyText: false, target }); + const summary = deps.formatCompactStateSummary(afterState); const jsErrors = deps.getRecentErrors(p.url()); const versionedRef = deps.formatVersionedRef(refMetadata?.version ?? refVersion, node.ref); return { @@ -508,7 +509,8 @@ export function registerRefTools(pi: ExtensionAPI, deps: ToolDeps): void { "Try refreshing refs and confirm this ref still targets an input-like element." ); - const summary = await deps.postActionSummary(p, target); + const afterState = await deps.captureCompactPageState(p, { includeBodyText: true, target }); + const summary = deps.formatCompactStateSummary(afterState); const jsErrors = deps.getRecentErrors(p.url()); const versionedRef = deps.formatVersionedRef(refMetadata?.version ?? refVersion, node.ref); return { diff --git a/src/resources/extensions/gsd/git-service.ts b/src/resources/extensions/gsd/git-service.ts index 1474f52e4..e264170e3 100644 --- a/src/resources/extensions/gsd/git-service.ts +++ b/src/resources/extensions/gsd/git-service.ts @@ -487,6 +487,9 @@ export class GitServiceImpl { commitType, milestoneId, sliceId, sliceTitle, mainBranch, branch, ); + // Pull latest main before merging to avoid conflicts from remote changes + this.git(["pull", "--rebase", "origin", mainBranch], { allowFailure: true }); + // Squash merge — abort cleanly on conflict so the working tree is never // left in a half-merged state (see: merge-bug-fix). try {