Skip to content

fix(agent): load history entries that predate interacted_element - #5845

Open
Lesereingrape wants to merge 9 commits into
browser-use:mainfrom
Lesereingrape:fix/legacy-history-interacted-element
Open

Lesereingrape wants to merge 9 commits into
browser-use:mainfrom
Lesereingrape:fix/legacy-history-interacted-element

Conversation

@Lesereingrape

@Lesereingrape Lesereingrape commented Sep 19, 2026 •

Copy link
Copy Markdown

What

BrowserStateHistory.interacted_element is a required list[DOMInteractedElement | None] | list[None] (browser_use/browser/views.py:122), but the two branches that exist to tolerate history entries written before that field was introduced fill the missing key with None:

  • browser_use/agent/views.py:683 in AgentHistoryList.load_from_dict
  • browser_use/beta/service.py:4062 in _load_rust_history

None is the one value that field cannot hold, so the compat branch converts "legacy entry" into ValidationError: 2 validation errors for AgentHistoryList — it fails for exactly the input it was written for.

Both now insert a list. load_from_dict inserts one None per action in the entry's model_output; _load_rust_history inserts [], because that loader sets item['model_output'] = None unconditionally (beta/service.py:4059), so an entry arriving through it has no actions to align with.

Fixes #5844

Why this shape

Why a list at all. None cannot be made valid instead: widening the field type would let a genuinely missing element be confused with an un-padded action list everywhere the length is meaningful — service.py:3430 and :3481 both index into it positionally.

Why one slot per action rather than []. Raised by @cubic-dev-ai's review on this PR, and correct: the replay worker enumerates the actions and indexes element state by that same position —

  • service.py:3453 for i, action in enumerate(history_item.model_output.action):
  • service.py:3481 historical_elem = history_item.state.interacted_element[i] ← unguarded, while the sibling read at :3430 in the same function does guard (if i < len(...) else None).

So an entry with actions and an empty element list loads and then fails later with a bare IndexError during replay. That is a worse failure than the one this PR removes, so the empty list was the wrong fill even though it validates. [None] * len(actions) fixes it without inventing a convention: None is a legal item type, _update_action_indices starts with if not historical_element ... : return action (service.py:3546, i.e. "no element info → run the action as-is"), and first-party code already writes exactly this shape for "actions with no captured elements" at service.py:1751 (interacted_elements = [None]) and service.py:3318 (interacted_element=[None] * len(self.initial_actions)). tests/ci/test_ax_name_matching.py:94,205,319 pin the same representation.

What is unchanged. Readers that only consume the field stay correct either way: views.py:827 and :842 write h.state.interacted_element or [None] * len(h.model_output.action), and for an entry padded to length n that expression now returns the list it already wanted; the other readers (service.py:3238, :3797, :3800, variable_detector.py:39-41) are length- or truthiness-guarded.

Testing

tests/ci/test_history_legacy_load.py (new, 4 tests) — the two loaders, a control that pins the behaviour that already worked, and the replay-alignment case from the review.

run command result
main @ d8110c5 + the first three tests pytest tests/ci/test_history_legacy_load.py 2 failed, 1 passed
4a48943 ([] fill) + all four tests pytest tests/ci/test_history_legacy_load.py 1 failed, 3 passed
593fdc7a (per-action fill) pytest tests/ci/test_history_legacy_load.py 4 passed
4a48943 pytest tests/ci/test_history_legacy_load.py tests/ci/test_history_wait_time.py 11 passed

The two main failures are verbatim pydantic_core._pydantic_core.ValidationError: ... history.0.state.interacted_element.list[nullable[DOMInteractedElement]] Input should be a valid list [type=list_type, input_value=None, input_type=NoneType], one per loader. The middle row is the reviewer's case measured as a test: assert [] == [None] → Right contains one more item: None, i.e. the entry loads with zero slots for one action. test_load_from_dict_keeps_a_stored_interacted_element passes on every row, which is the point: a payload that already carries the key is untouched by this change.

Regression sweep over the nearest neighbours: pytest tests/ci/test_ax_name_matching.py tests/ci/test_beta_agent.py → 4 failed, 207 passed, 1 skipped (162 s). All four failures are in test_beta_agent.py (test_beta_agent_run_env_sets_packaged_agent_tools_dir, ..._preserves_explicit_agent_tools_dir, test_rust_terminal_binary_finds_default_terminal_install, test_beta_agent_adds_available_files_to_task_context) and reproduce identically with views.py reverted to 4a48943 on this machine (4 failed, 198 deselected, 3.9 s) — they concern the packaged/rust terminal and tools-dir plumbing on Windows, not history loading. test_ax_name_matching.py, which exercises the element-matching path that consumes this field, is fully green.

Static checks, run against the working tree with the repo's own config:

  • ruff check (0.12.10, the rev pinned in .pre-commit-config.yaml) on the changed files → All checks passed!
  • ruff format --check → clean. Note on how that was measured: this clone has core.autocrlf=true, so files on disk are CRLF while committed blobs are LF, and a direct check then reports a whole-file rewrite for reasons unrelated to this diff. Checking an LF-normalised copy reports 1 file already formatted.
  • pyright: browser_use/beta/service.py is in the [tool.pyright] exclude list, and neither commit widens a type — the fill is list[None] where the annotation already accepts it.

Not run: the full tests/ci suite and the browser/CDP integration tests (this reproduction needs neither), ./bin/lint.sh as a script, and anything requiring a live model provider or API key. The IndexError at service.py:3481 is cited from reading that function, not from an executed replay — driving it would need a browser session, so the test pins the invariant that makes it unreachable instead. Reported from a local source checkout, not from CI.

Overlap with existing PRs

305 open PRs (the count when this PR was first pushed) were enumerated and their patches scanned for these paths. Sixteen touch browser_use/agent/views.py, browser_use/agent/service.py history code, or browser_use/beta/service.py — including several that only construct BrowserStateHistory(..., interacted_element=[]) in tests (#5626, #5656, #5680, #5709, #5741, #5786). None of them modifies the load_from_dict fill or _load_rust_history: #5644 mentions load_from_dict in its diff but its changes are confined to browser_use/agent/service.py and tests/ci/test_agent_checkpoint.py. #5786 and #5777 touch the same file in other regions.

Additional notes

Only the missing-key branch is fixed here. A state dict that is absent entirely still fails on the required url / title / tabs fields, which is a different (and defensible) contract — load_from_dict deliberately keeps h['state'] = state inside the guard.

AI assistance: Qoder (agentic CLI) wrote the reproduction, the tests, both fills and this description, including the correction pushed in 593fdc7a. This PR was opened during an unattended contribution run, so the account owner has not read the diff yet; the human-only review attestation is therefore left unticked deliberately.

BrowserStateHistory.interacted_element is a required list, so filling it with
None in the legacy-compat branches made every entry that lacks the key fail
validation instead of loading. Fill an empty list, which is what the downstream
`or [None] * len(...)` fallbacks already treat as "no captured elements".

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 3 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread browser_use/agent/views.py Resolved
Replay reads state.interacted_element[i] for each action in
_execute_step_from_history, so an empty list loads but raises IndexError
on the first action. Fill one None per action instead, which is also what
service.py:1751 and service.py:3318 already write for "no element info".
@mintlify

mintlify Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Preview approval required

This pull request comes from a fork, so Mintlify won't build a preview until someone with write access approves commit 7d4f45f.

Approve preview

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: loading a history entry that predates state.interacted_element fails validation in the branch written to support it

1 participant