feat(perception): add action affordance markers#1064
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e84c19ff9
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function isContentEditable(value: ElementAffordanceInput['contentEditable']): boolean { | ||
| return value === true || normalize(String(value ?? '')) === 'true' || normalize(String(value ?? '')) === 'plaintext-only'; |
There was a problem hiding this comment.
Treat empty contenteditable value as editable
isContentEditable currently recognizes only true and plaintext-only, but in HTML contenteditable without a value is represented as an empty string and is also editable. In DOM mode this means elements like <div contenteditable> are misclassified as plain text and lose the # affordance marker, which weakens the new action-hinting behavior specifically on rich-text editors and similar inputs.
Useful? React with 👍 / 👎.
8e84c19 to
b68802a
Compare
Add compact affordance markers to perception output without changing canonical refs. The marker sits outside ref/backendNodeId tokens so existing ref-aware tools and parsers keep their current contract while agents get a cheaper cue for whether to type, click, or treat a target as visual.\n\nConstraint: Issue #965 requires no DOM tag injection, no XPath routing, and no ref identity/schema changes.\nRejected: Embedding markers inside refs such as [#ref_1] | would risk breaking existing ref parsers and tool arguments.\nConfidence: high\nScope-risk: narrow\nDirective: Keep future affordance changes display-only unless #828/#831 explicitly change canonical ref syntax.\nTested: npx jest tests/utils/element-affordance.test.ts tests/dom/dom-serializer.test.ts tests/tools/find.test.ts --runInBand; npm run build; npm run lint -- --quiet; npm run lint:tier\nNot-tested: Manual MCP transcript against a live browser fixture.
b68802a to
8942d5d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8942d5d89f
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (tagName === 'a' || LINK_ROLES.has(role)) { | ||
| return 'link'; |
There was a problem hiding this comment.
Respect explicit control roles before anchor fallback
classifyElementAffordance checks tagName === 'a' before CONTROL_ROLES.has(role), so elements like <a role="button"> are always labeled as link (@) instead of control ($). In pages that use ARIA-role buttons on anchors (common in SPA component libraries), this produces incorrect affordance metadata and can steer downstream action selection away from the element’s declared semantics.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3eeaaa22b7
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function isContentEditable(value: ElementAffordanceInput['contentEditable']): boolean { | ||
| return value === true || normalize(String(value ?? '')) === 'true' || normalize(String(value ?? '')) === 'plaintext-only'; | ||
| } |
There was a problem hiding this comment.
Treat empty contenteditable as editable
isContentEditable currently returns true only for boolean true, 'true', or 'plaintext-only', so elements rendered as <div contenteditable> (empty attribute value) are classified as non-editable text. In HTML, an empty contenteditable value is treated as editable, so DOM/find outputs will miss the # marker for valid text-entry targets in that common markup pattern.
Useful? React with 👍 / 👎.
Summary
element-affordanceclassifier for perception outputs.findAX/CSS results with display-only markers outside canonical refs:#text input@link$button/control%visual target[backendNodeId]as the usable identifier.Closes #965.
Scope review
This PR intentionally stays narrow and aligns with the repo direction:
Validation
npx jest tests/utils/element-affordance.test.ts tests/dom/dom-serializer.test.ts tests/tools/find.test.ts --runInBandnpm run buildnpm run lint -- --quietnpm run lint:tierManual OpenChrome validation
Not run in this PR branch. The implementation is covered by unit/serializer/tool-handler tests, and the issue keeps live MCP fixture validation as merge evidence for final acceptance.