refactor: redesign Explorer API to a minimal, recovery-ambient interface#101
Open
DavertMik wants to merge 4 commits into
Open
refactor: redesign Explorer API to a minimal, recovery-ambient interface#101DavertMik wants to merge 4 commits into
DavertMik wants to merge 4 commits into
Conversation
Planner and Pilot ran at a 9% prompt cache rate while the default-model agents sat at 75%. The gap was three missing properties: a stable byte-identical prefix, an unchanging tool list, and append-only history. - Pilot: system, verdict and reset prompts now open with their static bodies and close with the per-test task block. Extract buildTaskContext() for the dynamic part; drop the request type from the verdict opener since the user message already carries it. - Pilot: always send the same tool set and gate it with toolChoice 'auto'/'none' instead of flipping the tool list between calls, which invalidated the whole conversation's cache. - Pilot: stop rewriting history via cleanupTag. Pilot conversations are light by design; the trimmed KBs cost more re-tokenized than they save. - Planner: order messages system -> static rules -> static output format -> page research -> approach + URL context, so re-plans of the same page reuse the research prefix. - Route the click disambiguator off the agentic model: ~350-token one-shot prompts can never cache and don't need the expensive model. Prompts are reordered, not rewritten. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Explorer's public surface shrinks from ~42 members to 13. The noise had three sources: four inconsistent error policies, a service-locator getter per collaborator, and the playwrightHelper leak that forced the recovery machinery to be public. - withPage(fn) is the single recovery-wrapped page seam; the page getter gives a raw null-safe peek for background consumers; playwrightHelper is private - one recover(error?) replaces runWithBrowserRecovery, recoverFromBrowserError, restartBrowser, ensurePageAvailable, and handleExecutionError; the recover-then-restart escalation is internal - action() returns a recovery-aware Action, fixing bare createAction() silently bypassing recovery - visit(url) returns the observed ActionResult; beginTest(test) returns a TestRun handle so a startTest/stopTest mismatch is unrepresentable - agents receive services via AgentDeps (TaskAgent owns the shared fields) instead of Explorer getters; ExplorBot now owns StateManager, Reporter, RequestStore, and PlaywrightRecorder - element annotation and eidx helpers move to utils/web-annotate.ts and utils/web-eidx.ts as page-taking functions - other-tabs info becomes observed state on StateManager Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Frame context is acting-level state, so exitIframe() lives on Action (which already holds the helper) instead of Explorer's interface. Failed attempts now restore the main-frame context ambiently for every action, generalizing the guard the form tool had; the exitIframe tool runs I.switchTo() through the action seam, so the step is recorded for generated tests. Explorer is down to 12 public members. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Explorbot Self-RegressionCommit
Attempt details
Session analysis — basic (native): Session AnalysisIssue tracking functionality was exercised across creation, labeling, filtering, and search flows. All five tests passed with core functionality verified, though success toast visibility was not reliably confirmed. Coverage
What works
Execution Issues
|
DenysKuchma
requested changes
Jul 20, 2026
| await this.reporter.reportTestStart(test); | ||
| await this.closeOtherTabs(); | ||
| this.stateManager.otherTabs = []; | ||
| if (!this.page && !(await this.recoverOrRestart())) { |
Collaborator
There was a problem hiding this comment.
This no-op stop leaves the already started test lifecycle unfinished because test.start() and reportTestStart() were called above. Defer them until recovery succeeds or finalize the failed test
| this.observedTestPages.clear(); | ||
| } | ||
|
|
||
| async playwrightLocatorCount(locatorFn: (page: any) => any): Promise<number> { |
Collaborator
There was a problem hiding this comment.
Remove from docs playwrightLocatorCount
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Explorerhad grown into a god-module: ~42 public members mixing six concerns, four inconsistent error policies for the same failure, seven service-locator getters, and a publicplaywrightHelperthat let 18 call sites reach raw Playwright internals — which in turn forced the whole recovery machinery to be public. This PR reshapes it into a deep module with a 13-member interface.The new interface
Key changes
recover(error?). This also fixes a real bug:Actions created via barecreateAction()silently bypassed recovery whileexecuteActiongot it.playwrightHelperis private.withPage(fn)replaces all 18 external raw-page reaches and the 5 caller-siderunWithBrowserRecoverywraps; thepagegetter covers the three background peek-only consumers (axe scans, screencast) where triggering recovery would race the main flow.AgentDepsobject (src/ai/agent.ts);TaskAgentowns the shared fields and getters.ExplorBotnow truly ownsStateManager,Reporter,RequestStore, andPlaywrightRecordervia lazy accessors and injects them downward.beginTest(test)returnsTestRun { started, stop(meta) }, making the oldstartTest(a)/stopTest(b)mismatch unrepresentable; page watchers are fully internal.web-utils.utils/web-annotate.ts(annotatePageElements) andutils/web-eidx.ts(eidxInContainer,eidxByLocator) are page-taking functions called throughwithPage.stateManager.otherTabsreplaces thehasOtherTabs/getOtherTabsInfo/clearOtherTabsInfotrio.User-visible behavior
browsertool: therestartaction merged intorecover(recovery escalates to restart automatically).Verification
biome checkclean; scopedtscshows no new errors beyond pre-existing onesNot run:
tests/regression(real-AI harness) — recommend one live session before merging.Follow-up (out of scope)
Actionstill exposes its ownplaywrightHelper(single read atnavigator.ts:357) — same leak one layer down, worth a companion cleanup.🤖 Generated with Claude Code