Skip to content

Report what the app said and did, not just how the page moved - #129

Merged
DavertMik merged 4 commits into
mainfrom
feat/page-diff-messages-and-requests
Aug 24, 2026
Merged

Report what the app said and did, not just how the page moved#129
DavertMik merged 4 commits into
mainfrom
feat/page-diff-messages-and-requests

Conversation

@DavertMik

Copy link
Copy Markdown
Contributor

A click that fires a request the server rejects looks like a success today. pageDiff describes DOM and accessibility-tree movement only, so the toast carrying the server's error is either buried in raw markup — collapseHtmlParts replaces every part once the diff exceeds 8000 chars or 8 parts, which a real app re-render does routinely — or stripped outright by conversation compaction. ariaChanges cannot carry it at all: nodeToEntry drops every non-interactive role, so an alert never reaches the model.

Meanwhile the failed request reaches Pilot only as a session-wide network errors: POST /api/… → 400 in <state>, with no page context and no link to the action that caused it. A bare 400 reads as a missing value. In trace ecad7a69 that is exactly what happened: Pilot twice told the tester to fill in a Title that had been filled five minutes earlier, and the run spent 21 clicks and 8 minutes on a Save the server had already refused.

What changes

Three new fields on PageDiff, all scoped to the action that produced them:

  • messages — text the app put on the page in response. Live-region content first (role=alert|alertdialog|status|log, aria-live, <output>), then any other text that appeared. The second tier is what catches notifications built with no ARIA at all, which is the common case. Built from htmlDiff.added, which already computed it and threw it away.
  • requests — same-origin xhr/fetch made during the action, with status. captureMainDocumentResponse already watched the action's exact window for the main document status; it now records these too, so there is no second listener and no interaction with requestStore.clear().
  • consoleErrors — what the page logged while the action ran. grabBrowserLogs() drains its store, so this was already per-action; nothing surfaced it.

A 4xx/5xx now leads the tool's suggestion and points at the message the user was shown, instead of leaving the model to read the click as successful and try again.

Pilot reads the same evidence through formatActions, which analyzeProgress already sends — so it arrives attributed to the action, and narrowed to failure signal: rejected requests only, the first two messages, one console error. The review that misfired would have read:

[SUCCESS] ACTION click: Save
   requests: POST /api/zephyr/plans → 400
   messages: Operation against a key holding the wrong kind of value
   console: Ember Data Request POST /api/zephyr/plans returned a 400

Bug found on the way

Browser console messages were never recorded. grabBrowserLogs() returns Playwright ConsoleMessage objects whose type and text are methods, and captureBrowserLogs filtered with ['info','error','warning','warn'].includes(log.type) — comparing a function against a list of strings, always false. Every entry was discarded, which is why console errors read none for a whole session while the page was logging its own failure. Both fields are normalized at capture; tests/unit/action-browser-logs.test.ts locks it down.

Deliberately out of scope

  • htmlDiff runs only when the URL is unchanged, so a flash message shown after a redirect is not picked up — context re-injection already covers that case.
  • Requests that never get a response (connection refused, aborted) fire requestfailed, not response, so they carry no status and do not appear.
  • Cross-origin calls are filtered out, matching XhrCapture, to keep third-party analytics out of every tool result.

Testing

bun test tests/unit (1037 pass) and bun test tests/integration (81 pass), plus format:check and lint, all verified against this commit in a clean worktree. New coverage in page-diff-evidence, pilot-action-evidence and action-browser-logs.

This changes what the models see on every action, so it is worth a regression label before merging — I have not applied one.

🤖 Generated with Claude Code

DavertMik and others added 2 commits August 21, 2026 03:20
A click that fired a request the server rejected looked like a success. The
page diff described DOM and accessibility-tree movement only: a toast carrying
the server's error was either buried in raw markup that gets collapsed on a
re-render, or dropped by conversation compaction, and the rejected request
reached the Pilot much later as a bare session-wide status code.

Every action now carries three more pieces of evidence in its page diff:

- messages: text the app put on the page in response. Live-region content
  first (role=alert/status/log, aria-live, output), then any other text that
  appeared, which is what catches notifications built with no ARIA at all.
- requests: same-origin xhr/fetch made during the action, with status. The
  listener that already watched the action's window for the main document
  status now records these too.
- consoleErrors: what the page logged while the action ran.

A 4xx/5xx now leads the tool's suggestion and points at the message the user
was shown, instead of leaving the model to read the click as successful.

The Pilot reads the same evidence through recent_actions, attributed to the
action that caused it and narrowed to failure signal: rejected requests only,
first two messages, one console error.

Fixes browser logs never being recorded: grabBrowserLogs returns Playwright
ConsoleMessage objects whose type and text are methods, so the level filter
compared a function against a list of strings and discarded every entry. Both
are normalized at capture, which is why console errors always read as none.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Every tool reads the diff off ActionResult.fromState(currentState), not off
the instance the capture produced. networkRequests survives that only through
the object spread, so a later change to fromState that lists fields explicitly
would silently empty pageDiff.requests with nothing failing.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Comment thread src/utils/html-diff.ts Outdated
const appearedText = added.filter((line) => line.startsWith(TEXT_LINE_PREFIX)).map((line) => line.slice(TEXT_LINE_PREFIX.length));
const messages: string[] = [];

for (const candidate of [...collectLiveRegionTexts(originalMap, modifiedMap), ...appearedText]) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This puts any newly appeared page text into messages, not just actual app notifications. After normal navigation, headings and content from the new page may be reported to Pilot as feedback from the action. I’d only include generic appeared text when the URL hasn’t changed and handle live-region messages separately. We should also add a test for navigation between pages

Comment thread src/action.ts Outdated

const call: NetworkCall = { method: request.method(), path: url.pathname, status };
if (this.networkRequests.some((r) => r.method === call.method && r.path === call.path && r.status === call.status)) return;
if (status < 400 && this.networkRequests.length >= MAX_NETWORK_CALLS) return;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The limit currently applies only to successful requests, while failed requests can keep growing without a cap. Since this data is added to the model context, it would be safer to limit the total list while giving failed requests priority

@DavertMik DavertMik added the regression Run the LLM regression suite (one run per label add) label Aug 22, 2026
DavertMik and others added 2 commits August 23, 2026 01:51
Review of #129 asked why any text that appeared counted as a message, and why
the cap on recorded requests held back only the successful ones. Both hold, and
the first one runs deeper than the messages.

Text that appeared is only a reply to the action while the page stayed the same.
Across a navigation it is the content of the page that opened, so only what the
live regions announced carries over — the flash message of a redirect, which the
old rule dropped entirely because no diff was computed across URLs at all.

Elements are no longer compared across two pages either. A navigating action used
to report one ARIA diff whose halves belonged to different pages: the elements of
the page left behind listed as removed beside the elements of the page arrived at,
the chrome common to both cancelled out, and nothing saying which side was which —
so elements that no longer exist read as available. Such an action now reports the
move, what the app announced in transit, and its requests; the page arrived at is
described in full by the context that follows it. The tool result says so, rather
than letting an empty element diff read as nothing having happened, the Pilot gets
the move attributed to the action that caused it, and prima's envelope names the
page it left for instead of printing "no change".

The request list is capped as a whole, with a rejected call evicting a successful
one. Failures used to grow without a limit into the model's context.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
…es-and-requests

# Conflicts:
#	CHANGELOG.md
@DavertMik
DavertMik merged commit cd60c39 into main Aug 24, 2026
2 checks passed
@DavertMik
DavertMik deleted the feat/page-diff-messages-and-requests branch August 24, 2026 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

regression Run the LLM regression suite (one run per label add)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants