lablup / lablup/backend.ai-webui
[pr-devserver-review] Walkthrough 1/4 — review overlay: stop fields in the v3 anchor, strict resolution, volatile-query denylist, re-arm on mutation
- Dominant language
- TypeScript
- Stars
- 133
- Forks
- 81
- Avg merge
- 1d 11h
- Merged PRs (30d)
- 344
Description
## Parent
FR-3313 — Per-PR dev server review. Design: wayfinder map FR-3941 (decisions FR-3942 payload, FR-3943 headless minting). This is the bottom layer of the walkthrough stack; FR-B (guided mode) and FR-C (skill) build on it.
## What to build
The review overlay's data model and resolution learn what a **Stop** is — a pin authored by the implementing session that says what changed and what to check — without a version bump (ADR 0002, additive v3).
- **`AnchorV3` optional fields** (`react/vite-plugins/review-overlay/client/types.ts`): `ch` (what changed, ≤ 280 chars), `ck` (what to check, ≤ 280), `old` / `new` (≤ 40 each), `type` (`'added' | 'modified'`), `kind` (short element kind), `code: Array<{path: string; line: number; to?: number}>` (1–3), `sha` (40-hex head), `pr` (number), `via: Array<{click: {text?: string; tid?: string`>}}, `dlg?: 1` (picked inside `[role=dialog]`). `decodeAnchor` drops an ill-typed optional field instead of rejecting the anchor; `isStop(anchor)` = `ck` is a string. Caps enforced in one place (`anchor-guard.ts`) and shared by the encoder path the skill will use.
- **Strict resolution for stops** (`resolve.ts`): a stop accepts a text-scan candidate only when the landmark `tid` matches, and with `dlg` only candidates inside an open `[role=dialog]`; an unresolved stop stays unlocated (the measured failure: a modal stop "located" on a look-alike outside the closed modal).
- **Volatile query denylist** (`anchor.ts` capture, constant in `anchor-guard.ts`): drop `formValues` and other volatile params from `q` for every pin, reviewer pins included; the list is the codec's, one place.
- **Re-arm on DOM mutation** (`pin.ts`): while a showing view is a stop that is not located, a mutation outside the overlay host runs one debounced locate pass (today only `reposition`/`placeAll` run after the 20 × 500 ms landing ladder gives up; a modal that leaves the URL alone never re-triggers).
- **CLI**: `pnpm run review-pins parse --json` surfaces the new fields unchanged.
- Unit tests beside each module (`codec.test.ts`, `resolve.test.ts`, `anchor.test.ts`, `pin.test.ts`, `cli.test.ts`); `CONTEXT.md` gets the term **Stop** and the widened **Pin** definition.
## Acceptance criteria
- [ ] A stop encoded with every new field round-trips through `encodeAnchor`/`decodeAnchor`; an anchor with a wrong-typed `code` still decodes with `code` dropped; today's testdata blocks parse unchanged.
- [ ] A stop whose landmark is absent is **not** located by the text scan; the same anchor without `ck` still is (reviewer behaviour unchanged). A `dlg` stop is located only while a `[role=dialog]` contains the candidate.
- [ ] Capturing on `/session/start?formValues=…` yields an anchor whose `q` has no `formValues`.
- [ ] With an unlocated stop showing, inserting its element into the DOM after the ladder gave up locates it within one debounce; a located pin is not re-located on unrelated mutations.
- [ ] `bash scripts/verify.sh` passes; no new astryx token-gate findings.
JIRA Issue: FR-3949
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with react/vite-plugins/review-overlay/client/types.ts and anchor-guard.ts, then read resolve.ts, anchor.ts, pin.ts, and the named module tests. Trace encodeAnchor/decodeAnchor and the existing locate and mutation paths before checking cli.test.ts and CONTEXT.md. Done means the listed fields, strict stop resolution, query filtering, debounced re-arm, CLI output, tests, and verify.sh acceptance criteria all pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- react, typescript, vite
- Domain
- cli, devtools, frontend, testing
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100