Skip rebuilding an ActionResult for a research-cache lookup

[DavertMik]

Implements plans/029-cached-research-cheap-path.md. Three lines, no behavior change.

The problem

Release 0.4.0 (#158, region states) changed Researcher.getCachedResearch from a plain hash lookup into:

return getCachedResearch(ActionResult.fromState(state).baseHash);

Keying by the region-less base hash was necessary — state.hash now forks on regions. But ActionResult.fromState synchronously reads up to three files from disk (page HTML, browser log, and the full screenshot PNG buffer), and baseHash then runs a parse5 parse of the whole page to extract headings.

So a cache lookup became one of the most expensive calls on the prompt-build path. Worse, two agents pay it twice per invocation, because they already hold the very ActionResult being rebuilt:

• captain.ts:155 builds const actionResult = ActionResult.fromState(state), then :163 rebuilds it
• navigator.ts:577 builds currentActionResult, then :578 rebuilds it

The change

An early exit in the static method, and the two call sites pass what they already hold:

static getCachedResearch(state: WebPageState): string {
  if (state instanceof ActionResult) return getCachedResearch(state.baseHash);
  return getCachedResearch(ActionResult.fromState(state).baseHash);
}

The signature is unchanged. ActionResult implements ActionResultData and ActionResultData extends WebPageState, so an ActionResult already IS a WebPageState — a WebPageState | ActionResult union would be redundant, and instanceof narrows the parameter on the cheap path.

Call sites holding only a WebPageState (researcher.ts:662, freesail-command.ts:46) keep today's behavior unchanged.

Cache-key equivalence

The cheap path skips fromState's url: state.fullUrl || state.url normalization, so the key had to be proven identical or cached research would silently re-run. computeStateHash keys off relativeUrl || url, and the constructor already applies extractStatePath, which is idempotent for path-like input.

Verified empirically — ar.baseHash === ActionResult.fromState(ar).baseHash across four URL shapes:

case	hash
path url + fullUrl	projects_abc_suites_h1_suites_h2_list
full url only	projects_abc_plans_tab_x_h1_plans
path with #hash	dash_section_h1_dash
path, no fullUrl	login_h1_login

Identical on both paths in every case.

Verification

• bun test tests/unit — 1225 pass / 0 fail
• bun test tests/integration/researcher.test.ts — 10 pass / 0 fail
• bun run format / bun run lint — exit 0
• grep -n 'getCachedResearch(state)' src/ai/captain.ts src/ai/navigator.ts — no matches

Both switched arguments were confirmed to be built from the same state as before: captain's state is not reassigned between :151 and :163; navigator's optional actionResult parameter is never supplied by its single caller (freesail-command.ts:67), so currentActionResult is always ActionResult.fromState(state) today.

Notes for the reviewer

• ActionResult.fromState is deliberately untouched. Adding an if (state instanceof ActionResult) return state; early exit there would give every caller the cheap path, but it would also skip that method's url: state.fullUrl || state.url normalization — a real behavior change, deferred by the plan.
• The plan's optional unit assertion was skipped under its own clause: no researcher unit test constructs an ActionResult, and the plan says skip rather than create one.
• Two pre-existing tsc --noEmit errors (captain.ts:35, navigator.ts:706) are unrelated to the edited lines and predate this change; the repo gates on Biome plus tests.
• No CHANGELOG.md entry — the plan scopes it out and there is no user-visible behavior change. Happy to add one.

🤖 Generated with Claude Code

https://claude.ai/code/session_01B46vQvvc47LJeCmqQxAhme