Fix what the tester is told: dead vision tools, doomed clicks, and misrouted click failures - #109
Conversation
Analysis of a 3.5h CI session (84 tests, 151 Langfuse traces) showed two sinks that cost far more than the tests they broke. Vision: the configured vision model returned "does not exist" on every call. `see`/`visualClick` set `visionDisabled` and kept answering, so the AI called them 230 more times and got an error back 220 of those. Those are test steps spent against the iteration cap, on exactly the fallback the AI reaches for when a click fails. Withdraw both tools from the toolset on first failure instead. Consumers spread the record fresh each iteration, so they disappear from the next prompt; the guards stay for calls already emitted in the same roundtrip. Timeouts: a page interaction inherited `playwright.timeout`. With the 30s Playwright default that meant a click on a disabled control blocked for 30s before failing — 137 such timeouts in one session, about 69 minutes, a third of the wall clock. Add `action.timeout` (default 3000) and apply it to the page before each action. Measured against a disabled button: 30015ms -> 3004ms. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Explorbot Self-RegressionCommit
Attempt details
Session analysis — basic (native): Session AnalysisThis session explored the issue tracking feature, focusing on CRUD operations and filtering capabilities. The core issue creation flow works—issues are persisted to the table—but the success confirmation message is missing from the UI, leaving users without feedback after submission. Coverage
What works
Defects[Medium] Success message not displayed after issue creationAffects: Reproduce:
Evidence: The issue "ShiveringGorgeousBlue95" appears in the table (7 rows total), but no success message is visible. The log explicitly notes "Expected success message not visible" and "Success message not shown". Execution Issues
|
Same session analysis as the previous commit. Of 463 failed clicks, 187
got advice pointing away from the actual cause.
Two misroutes were deterministic, not statistical. A container miss
throws "was not found inside element X", which contains "was not found",
so it was reported as "not found in the DOM" — but the element usually
exists, the container was wrong. A disabled control surfaces as a
Playwright timeout, so it was reported as "covered by overlay" — but no
overlay is involved and a precondition is unmet. After a disabled
failure the model went hunting for another locator 77% of the time and
those tests recovered least often (54% vs 69%); one chain re-clicked the
same disabled button three times and then abandoned the test.
clickFailureSuggestion() classifies across all attempts rather than the
last one — with a fallback ladder the last error is usually the least
informative — and orders existence evidence ahead of not-found. Replayed
over the session's real failures it classifies 346/346 correctly.
Locator rules: ARIA stays first choice, but role and text must be copied
from the snapshot rather than guessed, which is where the invented pairs
came from. role="input" is not an ARIA role and matches nothing —
getByRole('input') returns 0 elements against a real input, getByRole
('textbox') returns 1 — so the "good example" list taught a locator that
can never work.
Container guidance said ALWAYS in two rules while a third reserved it for
disambiguation. A paired test (same target tried both ways in the same
test: 11-5 discordant, p=0.21, no difference in 51 of 67) does not
support a performance claim either way, so this only removes the
contradiction and makes the containerless fallback a requirement — 117
failures ran out of fallbacks still holding a container.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DavertMik
changed the title
| throw new Error('No valid I.* or page.* commands found in code block'); | ||
| } | ||
|
|
||
| this.playwrightHelper?.page?.setDefaultTimeout(this.config.action?.timeout ?? DEFAULT_ACTION_TIMEOUT); |
There was a problem hiding this comment.
setDefaultTimeout() mutates the Page permanently. After the first action, the 3-second action timeout also applies to later navigation, waits, and direct Playwright operations, effectively overriding playwright.timeout for the rest of the session
There was a problem hiding this comment.
Fixed in 85c976a. The page timeout is now restored to playwright.timeout as soon as the commands finish, and again in the finally so the throw path is covered too — same set/restore shape Navigator.verify() already uses. The short budget now covers the interaction only; navigation, waits, and page capture run under the configured Playwright timeout again.
| const disableVision = (): void => { | ||
| visionDisabled = true; | ||
| Reflect.deleteProperty(tools, 'see'); | ||
| Reflect.deleteProperty(tools, 'visualClick'); |
There was a problem hiding this comment.
Captain keeps copies of these tools, so deleting them here has no effect there
Rebuild or filter Captain tool list on each iteration
There was a problem hiding this comment.
Fixed in 85c976a. You are right, and it was worse than the copies: visionDisabled was a closure variable per createAgentTools() call, so Pilot, Tester, Rerunner and every Captain handle() each had their own — one agent losing vision left the others calling a dead model.
The flag moved to Stats.visionDisabled (session-wide), and withdrawVisionTools(tools) applies it in three places: at the end of createAgentTools() so any toolset built later omits them, in disableVision() for records already handed out, and at the top of each Captain loop iteration for the record built before the model died. web-mode.ts now attaches see/visualClick only when they exist, following the pilot.pickPlanningTools() pattern, so a withdrawn tool never lands in the record as undefined.
Covered by three tests in tests/unit/agent-tools.test.ts.
There was a problem hiding this comment.
Follow-up in bd8971b: the Stats flag alone only covered toolsets built after the model failed. Pilot and Rerunner build theirs once at construction and Tester spreads a cached record, so a failure inside one of them still left the others offering see. All three now filter the record they hand to the model, the same way Captain does.
…he session Both from review of #109. The action timeout reached the page through setDefaultTimeout() and stayed there, so every later navigation, wait, and capture inherited it too. It is now restored to playwright.timeout as soon as the commands finish, and again on the error path. Vision withdrawal lived in a closure per createAgentTools() call. Captain builds its own toolset and destructures see/visualClick into a fresh record, so deleting the keys reached neither — and Pilot, Tester, and Rerunner each carried their own flag, so one agent losing vision left the others calling a dead model. The flag moves to Stats, withdrawVisionTools() applies it at build time, Captain filters its record each iteration, and web mode attaches the two tools only when they exist. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d-action-timeout # Conflicts: # CHANGELOG.md
…r already hold Moving the flag to Stats only covered toolsets built after the model failed. Pilot and Rerunner build theirs once at construction and Tester spreads a cached record, so a failure inside any one of them left the others offering see/visualClick against a model that had already refused. Each now filters the record it hands to the model, the same way Captain does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 participants
Five fixes from a Langfuse review of the 2026-08-06 CI session (6 runs, 84 tests, 151 traces, 14,626 observations, 871 click calls of which 463 failed).
finishstop1. Vision tools stayed callable after the vision model died
The configured vision model returned
does not exist or you do not have access to iton all 192 calls — half of every error in the session. Explorbot already degraded correctly (setvisionDisabled, told the AI to use ARIA), butseeandvisualClickstayed in the toolset and kept answering. The AI called them 230 times and got an error back on 220 — steps burned against the iteration cap, on precisely the fallback it reaches for when a click fails.disableVision()now withdraws both tools. Consumers spread the record fresh each iteration, so they vanish from the next prompt. In-tool guards stay for calls already emitted in the same roundtrip;pilot.pickPlanningTools()already guarded withif (see).2. A doomed click blocked for 30 seconds
Page interactions inherited
playwright.timeout, which reachespage.setDefaultTimeout(). At Playwright's 30s default, clicking a disabled or unreachable control held the test for the full 30s. 137 such timeouts in one session — roughly 69 minutes, about a third of the 3.5h wall clock.New
action.timeout(default3000) applied to the page before each action. Measured against a real Chromium and a disabled button withplaywright.timeout: 30000: 30015ms → 3004ms.3. 187 of 463 click failures were told the wrong cause
Two misroutes, both deterministic code behavior rather than statistics:
Clickable element X was not found inside element Y. That containswas not found, so it was reported as "not found in the DOM" — but the element usually exists; the container was wrong. 117 cases.visualClickcannot click a disabled button either. 46 cases.Downstream cost: after a disabled failure the model went hunting for a different locator 77% of the time, and those tests recovered least often (54% vs 69% for genuine locator misses). One chain re-clicked the same disabled
Selectthree times, then abandoned the test — and that is one of the 7 falsefinishcalls above.clickFailureSuggestion()classifies across all attempts rather thanattempts[last](with a fallback ladder the last error is usually the least informative) and orders existence evidence ahead of not-found: disabled → covered → hidden → container miss → absent. Replayed over the session's real failures it classifies 346/346 correctly. Covered by 6 new unit tests pinning the real CodeceptJS/Playwright error strings, since those are the drift risk on upgrade.4. The rules taught an ARIA locator that can never match
role: "input"was listed under<good_aria_locator_example>. It is not an ARIA role. Verified against a real browser:getByRole('input')returns 0 elements for a text input,getByRole('textbox')returns 1. Same mistake in a CSS example (div[role=input]).ARIA stays the first choice — generality and the maintainability of the generated CodeceptJS suites depend on it. What changed is that role and text must be copied from the ARIA snapshot or UI map rather than guessed, which is where the invented pairs came from. Single-command clicks using
role="button"succeeded only 29% (20/68) againstrole="link"at 54%.5. Container guidance contradicted itself
Two rules said containers were the default for every interaction while a third reserved them for disambiguation. Now conditional and consistent across all three places, and the containerless fallback is a stated requirement rather than a footnote — 117 failures exhausted their fallback ladder still holding a container.
Deliberately not claiming a performance win here. A paired test (same target locator tried both ways within the same test) gives 11-5 on 16 discordant pairs, p = 0.21, and no difference in 51 of 67 — the raw 56%-vs-42% gap is mostly selection bias, since the model reaches for containers on harder targets. This change removes a contradiction, nothing more.
What was deliberately not done
Flipping locator priority toward CSS. CSS scores higher (59% vs 42% on single-command calls) but the comparison is confounded the same way, and CSS-first would make the generated suites brittle and app-specific — the reason ARIA is preferred in the first place. Worth re-measuring in a paired comparison once invented pairs drop; if ARIA still lags, the fix belongs in the snapshot pipeline, not the priority ladder.
Verification
bun test tests/unit— 847 pass / 2 fail / 2 errors; the 2 failures are identical onorigin/mainand pre-existingbun test tests/integration/— 79 pass, 0 failbun run format,bun run lint:fix— cleanrole: "input"claim both measured end-to-end against a real Chromiumtests/unit/explorer.test.tsgainssetDefaultTimeouton its mock page — the mock was missing a method the real PlaywrightPagealways has.🤖 Generated with Claude Code