Skip to content

Populate SemanticNode.FramePath and record the trace selector (Closes #1324) - #1554

Open
kblok wants to merge 5 commits into
mainfrom
fix/1324-selector-frame-path
Open

kblok wants to merge 5 commits into
mainfrom
fix/1324-selector-frame-path

Conversation

@kblok

@kblok kblok commented Nov 11, 2026

Copy link
Copy Markdown
Member

Closes #1324

Summary

  • The web engine now populates SemanticNode.FramePath: each node of a nested document carries the chain of iframe selectors enclosing it, outermost first, as upstream's toSemanticNode does. The selector of each iframe is read in the page by PageScript.FrameSelector (upstream's frameSelectorOf).
  • The recorded target descriptor now carries the structural selector as provenance (RecordedAction.Selector, RecordedTarget.Selector). Relocation never reads it and anchors leave it out, as upstream does.
  • HasValidDescriptor length-checks Selector, as upstream's readDescriptor does.

Deliberately not changed:

  • The web engine does not set Selector. Upstream's packages/web never sets a node's selector; only the mobile engine does, and the port already matches that.
  • The unknown-field drop of redactNode has no C# counterpart (a typed SemanticNode carries no undeclared key). Recorded in COMPATIBILITY.md.

Upstream parity

Upstream change: tester-army/e2e 4ce01b29 (#729, selector and framePath redaction, describeTarget selector, anchors strip selector) and the #1303 test in packages/e2e/tests/unit/agent-observation.test.ts. The other parts of #729 are outside this issue.

  • Counts: ported (redaction of selector and framePath, projectTree, describeTarget selector, anchor strip, web framePath population, frameSelectorOf as PageScript.FrameSelector); renamed: none; partial: Partial_records_the_durable_descriptors_of_each_action (the port's actions are flat, with its own summary wording); missing: none.
  • Port-only, with reasons:
    • PageScript.FrameSelector: a JS constant in PageScript, which holds the port's page scripts.
    • SelectorOfAsync: reads the frame selector only for a frame being read.
    • WebNode.FramePath and SetFramePath: the port builds WebNode DTOs before ToNode; upstream passes framePath into toSemanticNode.
    • RecordedAction.Selector and RecordedTarget.Selector: flat fields, matching the port's descriptor shape.
    • FramePathTests: .NET-only; no upstream it reads framePath off an observed web node.
  • Justified exceptions (COMPATIBILITY.md): the unknown-field drop of redactNode (no C# counterpart); web Selector not set (upstream web does not set it either).
  • Repo rules: symbol mapping and test layout checked (Partial_ prefix on the partial test); public surface: RecordedAction.md and RecordedTarget.md regenerated; no user guide documents descriptor fields.
  • Verdict: complete, with the two justified exceptions above.

Tests

  • Frame, trace, anchor, and cache groups: 369 passed.
  • Full E2E.Tests (before the review fixes): 3,901 passed; one ControlsTests file-chooser timing flake, which passed on rerun.
  • Real-model: E2E.Agent 28, TodoMvc 20, WebBenchmark 33, samples passed. WinApp: 55 passed, 2 scroll tests failing. Huge_Virtualized_List also fails on untouched origin/main (recorded as a finding). Infinite_Scroll_List passes on a clean replay cache. MobileApp: 9 failures (needs a device or Appium; not run on main).
  • dotnet format --verify-no-changes: fails for environmental reasons (CRLF checkout under core.autocrlf against the LF .editorconfig) and two pre-existing WHITESPACE errors at WebEngine.cs:1533, on lines this branch does not touch.

🤖 Generated with Claude Code

kblok and others added 4 commits November 10, 2026 22:11
The web engine stamps every node a nested document reads with the chain of
iframe selectors that encloses it, outermost first, as upstream's
toSemanticNode does (packages/web/src/observation.ts). The selector of each
iframe is read in the page by PageScript.FrameSelector (upstream's
frameSelectorOf). The main document carries no frame path.

Selector stays unset on web nodes: upstream's web engine never sets a node's
selector, only the mobile engine does, so the port matches it. The unknown
field drop of redactNode has no C# counterpart; COMPATIBILITY.md records both.

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
… parity gaps

describeTarget records the target's structural selector (upstream's
target.selector, redacted and bounded). The port now carries it as
provenance on RecordedAction and RecordedTarget. Relocation never reads it,
and anchors leave it out, as upstream's anchors.ts does.

Regenerates the RecordedAction and RecordedTarget API pages.

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
…ck selector

The frame selector is read after the owner and depth checks, so a frame that
is not read costs no round trip and a failed selector read no longer drops
the frame's owner and inset. Upstream's descriptor reader length-checks every
descriptor field, selector included, so HasValidDescriptor now does too.

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
Repo rule: a partially ported test carries the Partial_ prefix.

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 41 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

Commits

Reviewing files that changed from the base of the PR and between fbca5f1 and 62103eb.

Files selected for processing: 13

  • COMPATIBILITY.md
  • docs-site/src/content/docs/api/E2E/RecordedAction.md
  • docs-site/src/content/docs/api/E2E/RecordedTarget.md
  • src/E2E/Agent/Replay.cs
  • src/E2E/Cache/StepCache.cs
  • src/E2E/Cache/TraceRecorder.cs
  • src/E2E/Engine/PageScript.cs
  • src/E2E/Engine/WebEngine.Trace.cs
  • src/E2E/Engine/WebEngine.cs
  • tests/E2E.Tests/FramePath/FramePathTests.cs
  • tests/E2E.Tests/TraceAnchors/AnchorFixtures.cs
  • tests/E2E.Tests/TraceAnchors/DescribeDeltaTests.cs
  • tests/E2E.Tests/TraceRecorder/TraceRecorderTests.cs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

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.

Upstream Change: SemanticNode selector and framePath fields (redacted by redactNode)

1 participant