mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 14:38:59 +02:00
fix(design): run the DOM dump in the page on both engines; align doctrine with the catalog
The DOM-dump script is an arrow function, not a self-calling IIFE: Aside's `pg.evaluate($_DUMP)` receives the function and runs it in the page (the IIFE form executed in the repl sandbox, where `document` does not exist), and the fallback engine calls it with `$B js "($_DUMP)()" --out --raw`. Hygiene widens to every URL-bearing attribute (src, srcset per candidate, poster, action, formaction, data, ping, cite lose their query strings and fragments) and to data: URLs inside existing <style> nodes. The persist and scan blocks restate REPORT_DIR and RUN_ID literally instead of relying on a shell variable from an earlier block; the baseline's targetSet is defined per mode (repo-relative paths in source mode, page slugs in DOM mode) so DOM-mode deltas can match; the PR-body Detector line lists the states the probe can actually print. The DOM fixture is re-captured with the new script from outside the repo (the engine walks up from cwd for DESIGN.md, which the metadata now records). Doctrine contradictions the design specialist found: the landing-page motion rule matches the one-authored-moment reflex; the background rule names the catalog's halo/spotlight/stripe/grid slop instead of asking for gradients; the universal font rule is scoped to the display voice with the body/UI exceptions; "two typefaces max" allows the mono; the methodology's banned-font line renders BANNED_FONTS; Courier New is banned outright; the Brutalist, Retro-Futuristic, and Playful menu entries stop recommending system stacks, glow, and bounce; the coherence nudge uses the decoration vocabulary; Path A's gate names the display voice; font-loading prose points at the source the procedure verified; centered-everything is MEDIUM (an aggregate heuristic); the mockup guard reads "Never by default (unless the brief above asks for it)". The checklist's AUTO-FIX list renders the catalog's auto-fix rules; category 9 and the Hard Rules pointer count from the same partition helpers (detectorSlopEntries, judgmentTellEntries); the handoff list renders from HANDOFF_COMMANDS; a missing catalog id fails gen-skill-docs by name. gstack's own DESIGN.md gains border tokens and Decisions Log rows for its live-feed pulse and 11px mono labels. frontend-scope is case-sensitive like the bash arm. gen-skill-docs shares one emitGenerated helper for sections and lib-derived assets; renderCatalog keeps the one style with a caller. Tests: shared sliceBetween that fails on a missing end marker, the slop-gate fixture's real end marker, an isolated browse daemon for the DOM-mode E2E, the DOM hygiene test gated to CI or opt-in, docs notes for the two superseded plan sentences. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5.1
parent
b4d88a0126
commit
da6f0ff2f6
@@ -53,14 +53,14 @@ Delivery: ONE PR (user decision 2026-09-08, overriding the review's two-PR sugge
|
||||
|
||||
**2. Detector on live URLs via a rendered-DOM dump (commit 7).**
|
||||
- Mode rule: the target is a URL (including an explicit `http://localhost:3000` on a feature branch) → DOM mode, no source scan. Diff-aware mode with no URL → source mode over the changed frontend dirs. Stated in the Phase 0 prose so nobody scans source "helpfully".
|
||||
- Hook rule: `IMPECCABLE_HOOK: present` suppresses the source-mode scan only (the hook already scans edited files). DOM mode scans the dump regardless, because the hook cannot see a rendered page.
|
||||
- Hook rule (superseded before shipping, eng review decision 35): `IMPECCABLE_HOOK: present` never skips a scan; the prose tells the agent to use gstack's rows and ignore the hook's vocabulary. DOM mode scans the dump regardless, because the hook cannot see a rendered page.
|
||||
- One dump script for both engines, rendered once by the resolver as a fenced JS block, written as an IIFE expression with no single-quote characters (every `aside repl '...'` script is a single-quoted bash string, and `$B eval` wraps an expression): serialize the stylesheets whose `ownerNode` is a `<link>` (inline `<style>` nodes are already in the markup; serializing them too would double-count), catch cross-origin sheets and list them in a trailing HTML comment as unresolved, inject the text as `<style data-gstack-dom-css>` in `<head>`, return `document.documentElement.outerHTML`. Aside: the Phase 3 page script evaluates it, does `fs.writeFile(path.join(pwd, "{page}.dom.html"), html)` (the sandbox `fs` writes only under `pwd`), prints `ASIDE_DIR=`, and bash copies the file to `$REPORT_DIR/dom/`. Browse fallback: `_TMP=$(mktemp -d)`, write the same JS to `$_TMP/dom-dump.js`, run `$B eval "$_TMP/dom-dump.js" --out "$_TMP/{page}.dom.html" --raw` (`--out` accepts only temp dirs or cwd, `browse/src/path-security.ts:26`), then `cp` into `$REPORT_DIR/dom/`. `$B html` is not used: reading commands wrap stdout in untrusted-content markers (`browse/src/server.ts:1249-1255`). The row lives in design-review's own DOM-mode prose in `scripts/resolvers/design.ts`, not in the shared `{{BROWSE_FALLBACK}}` table (that table renders into ten skills). Same script, same serialization on both engines; page content may still differ by session. `{page}` reuses the slug the Phase 3 screenshots already use. In DOM mode, findings' `file:line` point into `{page}.dom.html`; the agent confirms the element in the rendered page (Rule 4), never by hunting a source line.
|
||||
- `$REPORT_DIR` in `design-review/SKILL.md.tmpl:95` changes from `$HOME/.gstack/...` to `${GSTACK_HOME:-$HOME/.gstack}/...` (as `bin/gstack-slug` already does) so the allow-list and the report dir agree when `GSTACK_HOME` is set. The Output Structure tree at tmpl:119 and the `mkdir` at `design.ts:419` keep their `~/.gstack` shorthand; neither feeds the allow-list.
|
||||
- Sequencing: in DOM mode the scan runs once, after the last Phase 3 page script has produced its dump; in source mode the scan runs in Setup. Phase 9 recomputes (DOM mode re-dumps the affected pages after reload and rescans; source mode rescans the touched files); Phase 10 reports `Detector: N → M`.
|
||||
- Scan: `gstack-design-detect.ts scan --format gstack "$REPORT_DIR/dom"`; the wrapper's target validation accepts paths under the repo root OR under `${GSTACK_HOME:-$HOME/.gstack}/projects/*/designs/` (explicit allow-list, tested with a designs path accepted and a non-designs sibling refused). Report line once: "static scan of the rendered DOM; cross-origin CSS not resolved".
|
||||
- The load-bearing assumption (impeccable's static engine reads inline `<style>` in a `.html` file) is pinned in commit 1, not commit 13: the fixture set includes `review-eval-design-slop.html` served locally, dumped with `$B eval dom-dump.js --out --raw`, and scanned. DOM mode ships in commit 7 on top of that fixture.
|
||||
|
||||
**3. Detector counts in the regression baseline (commit 7).** The `design-baseline.json` template and regression prose live in `scripts/resolvers/design.ts` (DESIGN_METHODOLOGY Phase 6, the JSON block and "Regression Output"), not in the skill template; edit there. Field: `detector: { mode: "dom"|"source", engine, total, byRule: {id: n} }`. `engine` is the version parsed from the READY path when it is the `~/.impeccable/bin/<version>/` cache, otherwise the resolved binary path; a version field in `detect --json` output wins if the fixture shows one. Previous baseline = newest `${GSTACK_HOME:-$HOME/.gstack}/projects/$SLUG/designs/design-audit-*/design-baseline.json` older than today's `$REPORT_DIR` (a second run on the same day overwrites today's baseline and compares against the most recent prior day's, by design). Diff only when `mode` matches; otherwise say "detector modes differ (dom vs source), no delta". When `engine` differs, print the delta with the caveat `engine changed X → Y; rule set may differ`. Absent field → "no detector baseline (first scan)", never `+N`. Output: ids that appeared, ids that disappeared, totals, one example row in the prose (`+ kicker-above-heading (2) - gradient-text (1) total 14 → 9`). Known limitation stated: live DOM dumps jitter (ads, timestamps, A/B), so count deltas are advisory and id appear/disappear is the signal.
|
||||
**3. Detector counts in the regression baseline (commit 7).** The `design-baseline.json` template and regression prose live in `scripts/resolvers/design.ts` (DESIGN_METHODOLOGY Phase 6, the JSON block and "Regression Output"), not in the skill template; edit there. Field: `detector: { mode: "dom"|"source", engine, total, byRule: {id: n} }`. `engine` is the version parsed from the READY path when it is the `~/.impeccable/bin/<version>/` cache, or from a sibling `VERSION` file, otherwise `sha256:<12 hex>` of the binary (superseded before shipping: never a filesystem path); a version field in `detect --json` output wins if the fixture shows one. Previous baseline = newest `${GSTACK_HOME:-$HOME/.gstack}/projects/$SLUG/designs/design-audit-*/design-baseline.json` older than today's `$REPORT_DIR` (a second run on the same day overwrites today's baseline and compares against the most recent prior day's, by design). Diff only when `mode` matches; otherwise say "detector modes differ (dom vs source), no delta". When `engine` differs, print the delta with the caveat `engine changed X → Y; rule set may differ`. Absent field → "no detector baseline (first scan)", never `+N`. Output: ids that appeared, ids that disappeared, totals, one example row in the prose (`+ kicker-above-heading (2) - gradient-text (1) total 14 → 9`). Known limitation stated: live DOM dumps jitter (ads, timestamps, A/B), so count deltas are advisory and id appear/disappear is the signal.
|
||||
|
||||
## Deferred to TODOS.md
|
||||
- `$D check` slop rubric (P3, M): add catalog LLM-only tells to `design/src/check.ts` once the catalog's LLM-tell entries have been exercised in reviews; cost per variant and vision false positives are the open questions.
|
||||
|
||||
Reference in New Issue
Block a user