fix(controller): cap indexed options per dropdown container - #649
fix(controller): cap indexed options per dropdown container#649zhang17-24 wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f8578e2bc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| }) | ||
| return null // Skip excess options entirely (text included) | ||
| } | ||
| dropdownOptionCounts.set(container, used + 1) |
There was a problem hiding this comment.
Count only indexable dropdown options toward the cap
When viewportExpansion is finite, this increments the per-container count before handleHighlighting decides whether the option is actually in the expanded viewport and receives a highlightIndex. The extension runs PageController with viewportExpansion: 400 (packages/extension/src/agent/RemotePageController.content.ts:22-25), so a scrolled Element UI/native dropdown can have the first 20 DOM options clipped above the dropdown but still isVisible; they consume the cap, and the currently visible options later in DOM order are returned as null, leaving the agent with only the “more options” hint and no clickable options. Move the cap accounting to after the same viewport/top-element checks that make an option indexable, or only count nodes that actually get highlighted.
Useful? React with 👍 / 👎.
…visible Element UI / Avue selects commonly disable some options (cursor: not-allowed). `isInteractiveElement` treated them as non-interactive, so they never received a highlight index and the LLM could not see or select the intended option. Detect li/option elements inside dropdown/menu containers and index them even when visually disabled, keeping the complete option list in the simplified DOM.
A select/dropdown with hundreds of options (common in Element UI / Avue forms) puts every option into the simplified DOM, so the LLM request can exceed the server limit (HTTP 413). Index at most 20 options per dropdown container and fold the rest into a hint (e.g. "... 80 more option(s) not shown ..."), keeping the payload bounded while telling the LLM that more options exist.
3f8578e to
46e0667
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46e06674be
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (isDropdownOptionElement(element)) { | ||
| return true |
There was a problem hiding this comment.
Preserve disabled state for dropdown options
When a custom dropdown encodes disabled options only via class/cursor (for example Element UI's .is-disabled plus cursor:not-allowed), this early return bypasses the existing non-interactive cursor and disabled checks and gives the item a normal clickable index. Since the simplified DOM does not include class/style by default, the model cannot tell that index is not selectable, and click_element_by_index reports success even if the UI ignores the disabled click; include a disabled marker or render these as unindexed text instead.
Useful? React with 👍 / 👎.
Summary
When a dropdown (Element UI / Avue select, or a native select) has hundreds of options, every visible option is indexed into the simplified DOM. The LLM request payload then exceeds the server limit and fails with
InvokeError: HTTP 413: Payload Too Large(see #348).This PR caps the number of indexed options per dropdown container (20) and folds the excess options into a hint (e.g.
... 80 more option(s) not shown ...), keeping the payload bounded while telling the LLM that more options exist.Changes
packages/page-controller/src/dom/dom_tree/index.js: count indexed options per dropdown container inbuildDomTree; skip excess options entirely (text included) and recorddroppedOptionson the containerpackages/page-controller/src/dom/index.ts: render the folded-options hint inflatTreeToStringpackages/page-controller/src/dom/dom_tree/dropdown-cap.test.ts: new testsTest plan
npm testinpackages/page-controllerpasses (11 tests)... 80 more option(s) not shown ...(payload 2328 -> 681 chars)npm run typecheckand eslint passFixes #348