Skip to content

Share one landmark enumeration between recorder and page-state (predicate is shared, enumeration is not) #74

Description

@myselfsiddharth

Context

ADR-0007 made
visible_landmarks genuinely visibility-filtered in both capture sites, using the same
predicate. Review of PR #73 established that this closed only half the gap: the two sites share
the predicate but still disagree about which elements to test.

  • src/recorder/fingerprint.ts walks the whole tree, mapping implicit roles for
    FORM MAIN NAV HEADER FOOTER ASIDE, against an 8-role landmark set that includes
    complementary and region.
  • src/runner/page-state.ts checks 6 [role=] selectors, with tag fallbacks for only
    main / nav / form.

Measured on semantic markup with no redundant role attributes:

recorder  : ["banner","navigation","main","complementary","form","contentinfo","search"]
page-state: ["search","main","navigation","form"]
only in recorder: ["banner","complementary","contentinfo"]

Reproduce by calling captureFingerprint() and capturePageState() against the same page and
diffing visible_landmarks. Use markup like
<header>h</header><nav>n</nav><main>m<aside>a</aside></main><form>f</form><footer>x</footer>
— the divergence only appears without explicit role= attributes.

The in-tree fixture cannot show this. src/recorder/fixtures/grafana-gate-login.html puts
an explicit role= on every landmark (<main role="main">, <header role="banner">,
<nav role="navigation">, <form role="form">), so both paths find everything through the
[role=] branch and agree by accident. Real Grafana is unlikely to be that redundant, and #24
records against it.

Why it matters

capturePageState output becomes RepairContext.page_state. Once #27 wires a real repair
model, the model is handed a landmark list the recorder never produced — the same harm ADR-0007
set out to prevent, reached by a different mechanism. It also costs tokens and proposal quality
on a path whose measured cost feeds the PRD §9 "70% of fresh" kill line.

What to build

Extract one landmark enumeration used by both sites.

Both capture sites pass their evaluate body to the browser as a string — the recorder via
new Function, page-state via page.evaluate("...") — precisely because esbuild's
keepNames wraps named function expressions in __name(...), which does not exist in the
browser (see PR #73; guarded by tests/unit/page-state.test.ts). So the shared thing is a
JS source string, not a TS function. Any fix must preserve that, or it reintroduces a
runtime crash CI cannot see.

Decide where it lives. src/runner/ importing from src/recorder/ would be backwards — the
runner must not depend on the recorder. Options worth weighing in the PR description:

  • a small shared module (src/shared/ does not exist yet — creating it is a structural choice)
  • co-locating the snippet with the contract it serves, since visible_landmarks is a
    trajectory.schema.json field

Do not duplicate the walk into both files. Two copies is what produced this bug; a third
copy is not a fix.

Constraints

  • Preserve the string-body technique in both sites.
  • page-state gaining complementary / region / contentinfo is a behaviour change to
    repair context. Expected and desirable, but call it out — it is not a pure refactor.
  • Keep the counts (role_counts, form_count, …) DOM-wide. ADR-0007 decided they are
    structural signals, not visibility claims.
  • No new dependencies.

How to test

Add a test that asserts the two sites agree on a page built from semantic HTML with no
redundant role attributes — that is the case the current fixture cannot exercise:

npm run test          # new equivalence test
npm run ci
npm run test:canary

Then confirm the guard bites: revert one site's enumeration to its old shape and check the
equivalence test fails, rather than passing because both were changed together.

Also re-run npm run recorder -- --fixture and confirm whether the committed trajectory's
dom_digest shifts. If it does, regenerate by command and say so; do not hand-edit.

Before you open the PR

  • One enumeration, shared — not duplicated
  • String-body technique preserved in both sites; tests/unit/page-state.test.ts still green
  • Equivalence test over semantic markup without redundant role=
  • Guard proven to fail when one site is reverted
  • Behaviour change to RepairContext.page_state stated explicitly in the PR body
  • ADR-0007's enumeration caveat updated to say it is closed, and docs/gate/recorder.md /
    runner.md likewise
  • Artifacts regenerated by command if digests move
  • Branch track1/b2-shared-landmark-enumeration

Metadata

Metadata

Labels

area: recorderTouches recorderarea: runnerTouches runnerenhancementNew feature or requestsize/S<= 50 changed linestrack-1Technical gate / churn harness

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions