refactor(landmarks): one shared enumeration for both capture sites (#74)

[myselfsiddharth]

Closes #74.

What changed

ADR-0007 made visible_landmarks genuinely visibility-filtered in both capture sites using
the same predicate. That closed half the gap — the two sites still disagreed about which
elements to test:

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

Both now run one enumeration from src/shared/landmarks.ts — role vocabulary,
implicit-role map, visibility predicate, and tree walk — and neither owns a copy. The
browser-side role list is generated from the exported TypeScript constants with
JSON.stringify, so the TS copy and the copy that actually runs cannot drift.

The recorder still does its own DOM-wide counting pass for role_counts, but through the
shared paragentWalk / paragentRoleOf, so role resolution exists once. Counts stay
DOM-wide per ADR-0007.

⚠️ This is a behaviour change to RepairContext.page_state, not a pure refactor

capturePageState now reports:

• complementary, contentinfo, region — its old 6-selector list could not produce
  them at all, at any markup.
• banner / navigation / form / main from semantic tags without a role= attribute.
• DOM order instead of its own fixed role-array order.

Expected and desirable — it is the point of the issue — but it means anything reading
page_state.visible_landmarks sees a longer, differently-ordered list. Today the only reader
is StubRepairModelClient, which ignores it; when #27 wires a real model this is the list it
gets, and it is now the list the recorder actually produced.

The recorder is unchanged. Same 8-role set, same implicit map, same predicate, same DOM
order.

Where it lives, and why

The issue asked for this to be weighed rather than assumed.

Option	Verdict
src/runner/ imports src/recorder/	Inverts the pipeline dependency — the runner must not depend on the recorder.
src/recorder/ imports src/runner/	No better, just backwards in the other direction.
Co-locate with the contract, in contracts/	contracts/ is four language-neutral JSON Schemas and is outside tsconfig.json's include, so a .ts file there would ship untypechecked. It would also make contracts/ a mixed JSON/TS directory to save one import.
src/shared/ (chosen)	A leaf that imports nothing from src/. Both capture sites depend on it; it depends on neither.

The real risk with a package called shared is that it becomes a utility drawer and later a
dependency cycle. Constrained explicitly in src/shared/index.ts and in
docs/architecture.md: something belongs here only if it runs inside
the browser and two packages must run the identical copy. Anything else goes in the package
that owns it.

The string-body technique is preserved

Both sites still hand their evaluate body to the browser as text — the recorder via
new Function, page-state via page.evaluate("...") — because esbuild's keepNames wraps
named function expressions in __name(...), which does not exist in the browser (PR #73). So
the shared unit is a JS source string, not a TS function. A shared function would have
reintroduced a ReferenceError that CI cannot see, since capturePageState's only caller is
the repair loop that the gate:matrix exit-2 guard keeps unreached.

tests/unit/page-state.test.ts is unchanged and still green.

The guard, and proof it bites

tests/unit/landmarks.test.ts (8 tests) asserts the two sites agree on semantic markup with
no redundant role= — the case the in-tree fixture cannot exercise, because
grafana-gate-login.html puts an explicit role= on every landmark and both paths therefore
agree by accident. Expected lists are named in full, so two sites both returning [] cannot
pass. One test also fails if a second copy of the predicate appears anywhere under src/.

Reverted page-state to its old enumeration and re-ran:

 ❯ tests/unit/landmarks.test.ts (8 tests | 6 failed)
   × agrees on semantic markup with no redundant role attributes
       expected [ 'main', 'navigation', 'form' ] to deeply equal [ Array(6) ]
       - "banner", - "complementary", - "contentinfo"
   × agrees when the same landmarks carry redundant role attributes
   × agrees on which semantic landmarks are hidden
   × agrees on roles reachable only through an explicit role attribute
       expected [ 'search', 'main' ] to deeply equal [ 'main', 'search', 'region' ]
   × orders both lists by DOM position
   × keeps the predicate in exactly one place under src/
       + "runner/page-state.ts"
   ✓ agrees on the in-tree recorder fixture        <- still passes, as #74 predicted
   ✓ derives the browser-side role vocabulary from the exported constants

The fixture-based test passing under the revert is the issue's own claim, reproduced.

Artifacts

npm run recorder -- --fixture re-records with an identical dom_digest and identical
visible_landmarks; the only diff was recorded_at / started_offset_ms / duration_ms
wall-clock noise, so that churn was reverted rather than committed. npm run compile on
contracts/examples/trajectory.example.json regenerates the bundle byte-identical. No artifact
changes in this PR.

Docs

• ADR-0007 — the enumeration caveat now records that Share one landmark enumeration between recorder and page-state (predicate is shared, enumeration is not) #74 closed it, states the
  page_state behaviour change, and adds an open question about <search> / named <section>
  (both are in the role set but have no implicit tag mapping; adding them would change what the
  recorder writes, which is out of scope here).
• docs/gate/recorder.md, docs/gate/runner.md, docs/gate/compiler.md — the
  "predicate shared, enumeration not" wording replaced with what is now true.
• docs/architecture.md — src/shared/ added to the package table with a section on why
  it exists and why it must stay a leaf, plus a sixth invariant covering the string-body
  technique and the one-copy rule.

Tests

npm run ci            # secret-scan, contracts, lint, typecheck, 53 unit, 1 integration — pass
npm run test:canary   # 6 pass

🤖 Generated with Claude Code

[OM152002]

Approve. Verified by running it, not by reading it.

Confirmed

• The divergence is gone. On the markup that used to split them, both sites now return ["banner","navigation","main","complementary","region","search","form","contentinfo"] — identical, order included. page-state previously missed complementary, contentinfo, region.
• Visibility agrees: a page whose only banner is visibility:hidden plus a display:none aside gives ["main"] from both.
• Independent grep for checkVisibility under src/ finds exactly one implementation.
• Both sites still pass a string to the browser, so the #73 __name crash cannot return.
• Full suite green; no conflict with #77; architecture.md gained the src/shared/ row.

Injecting the role vocabulary via JSON.stringify from the exported constants is the right call — the TS copy and the browser copy now cannot drift. keeps the predicate in exactly one place under src/ is a real structural guard.

Non-blocking

1. Branch is BEHIND main and protection is strict: true — needs a main merge before it can land.
2. The recorder now walks the DOM twice (counts pass + landmark pass) where it was one pass. Irrelevant at recording speed, but it is a change.
3. Shared blind spot, not a divergence: <search> and <section aria-label> map to search/region in ARIA but have no implicit-tag entry, so neither site reports them — verified, both return ["main"]. #74 stays closed since they agree, but it under-reports on semantic markup and #24 records against real Grafana next. Already an ADR-0007 open question; worth keeping visible.
4. LANDMARK_ENUMERATION_JS declares consts — injecting it twice into one scope would be a redeclaration error. One docstring line would prevent that.