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

Open
#9,689 0 comments 0 reactions 0 assignees View on GitHub
ready-for-agent
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.