Files
pbakaus_impeccable/notes/adr-live-variant-mode.md
T
e587004ee4 Refactor: cleaner top-level directory structure (#138)
* refactor(content): merge content/site/ into site/content/

Phase 1 step 1 of the directory restructure. The dual content tree was
called out in CLAUDE.md as cleanup; both trees were already in sync
except for anti-patterns-catalog.js, which moves to site/data/.

- Delete content/site/skills/ and content/site/tutorials/ (duplicates of
  site/content/, which is what Astro's content collection actually reads).
- Move content/site/anti-patterns-catalog.js -> site/data/.
- Update scripts/lib/sub-pages-data.js and scripts/build.js to read from
  site/content/ and site/data/.
- Drop content/site/ from validateProse target list (site/content was
  already there).
- Rewrite the "Two content trees" section in CLAUDE.md as a single-tree
  pointer; update stale dev-server text mentioning the deleted
  server/index.js.

Tests: 186/186 pass. Skills build: clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(skill): rename source/skills/impeccable/ -> skill/

Phase 1 step 2 of the directory restructure. The path was redundantly
nested ("source/" wrapper plus "skills/impeccable/" — singular content
hidden behind the plural). Collapses to flat skill/SKILL.md +
skill/reference/ + skill/scripts/.

- Move source/skills/impeccable/ -> skill/.
- Rewrite scripts/lib/utils.js readSourceFiles(): drop the multi-skill
  iteration (CLAUDE.md commits to a single user-invocable skill); read
  skill/SKILL.md directly.
- Update scripts/build.js, scripts/generate-og-image.js, and the
  sub-pages data layer to point at skill/.
- Update tests/lib/utils.test.js: drop the "multi-skill" and "dir-name
  fallback" cases, update single-skill paths to skill/.
- Update tests/build.test.js similarly: drop "multiple skills"
  integration test, update paths.
- Update non-glob path joins in tests/framework-fixtures.test.mjs,
  tests/live-e2e/session.mjs, tests/live-e2e/agents/llm-agent.mjs,
  tools/live-loop.mjs.
- Update prose/text references in CLAUDE.md, AGENTS.md, DEVELOP.md,
  README.md, scripts/lib/sub-pages-data.js, bin/commands/skills.mjs,
  site/data/anti-patterns-catalog.js, site/pages/docs/[...slug].astro,
  docs/adr-live-variant-mode.md, docs/plans/.

Eval framework note: the separate impeccable-evals repo reads
../impeccable/source/skills/impeccable/ and needs a coordinated
rename to ../impeccable/skill/.

Tests: 186/186 pass. Skills build: clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor: rename docs/ -> notes/

Phase 1 step 3 of the directory restructure. The internal docs/ dir
(ADRs and plans) clashed with the site's /docs route. Renaming it
"notes/" makes the difference unambiguous: notes/ is project-internal
process, /docs is the user-facing route under site/pages/docs/.

No code references the dir; the rename is a clean git mv.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(site): move public/ under site/public/

Phase 2 step 4 of the directory restructure. Public assets and the
Astro publicDir now live alongside the rest of the site, so site/
is fully self-contained for static content.

- git mv public site/public.
- astro.config.mjs: add publicDir: './site/public'. Astro defaults to
  ./public at the project root, so the override is required.
- scripts/build.js: write generated _data, _headers, _redirects,
  _routes.json, and js/detect-antipatterns-browser.js into
  site/public/. Also delete the dead _REMOVED() Bun static-site
  builder (replaced by Astro at #130; the placeholder no longer earns
  its keep).
- scripts/build.js validateProse: replace the stale public/index.html
  reference (deleted at the Astro migration) with site/pages/index.astro
  in the count-validation file list, restoring homepage drift detection.
- scripts/generate-og-image.js: write OG image into site/public/.
- scripts/screenshot-antipatterns.js: read examples from + write
  screenshots to site/public/antipattern-{examples,images}/.
- scripts/lib/sub-pages-data.js: load command demos from
  site/public/js/demos/commands.
- .gitignore: rename the public/* generator-output entries to
  site/public/*.
- CLAUDE.md: refresh CSS/data-file paths (still pointing at the old
  pre-Astro public/css/ + public/js/ tree), point the changelog and
  command-add checklists at site/pages/index.astro and
  site/scripts/data.js + site/scripts/components/framework-viz.js.

Cloudflare Pages note: functions/ stays at the repo root because
CF Pages auto-discovers it there with no configuration knob to
relocate. Moving it under site/ would either break deployment or
require a build-time copy step that adds more complexity than the
cleanup is worth.

Tests: 186/186 pass. Skills + site build clean. _headers,
_redirects, _routes.json, _data/ all land in build/ correctly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(cli): consolidate bin/ + src/ + lib/ under cli/

Phase 2 step 5 of the directory restructure. The CLI surface was split
across three top-level dirs whose names were easy to mistake for each
other (especially src/ vs source/ pre-step-2). Consolidates under cli/.

- git mv bin -> cli/bin (CLI entry + skills sub-command)
- git mv src -> cli/engine (detect-antipatterns engine + browser variant)
- git mv lib -> cli/lib (download-providers helper)

Update package.json:
- bin.impeccable: cli/bin/cli.js
- main + exports: cli/engine/detect-antipatterns.mjs and the
  ./browser variant
- files: ["cli/", "LICENSE"]

Update internal references:
- cli/bin/cli.js: dynamic import points at ../engine/, package.json
  read goes one level deeper (../../package.json).
- functions/api/download/[type]/[provider]/[id].js + bundle/[provider].js:
  cli/lib/download-providers.js path.
- scripts/build.js, scripts/build-browser-detector.js,
  scripts/build-extension.js: cli/engine path constants.
- scripts/lib/sub-pages-data.js, scripts/lib/utils.js, skill/scripts/
  live-server.mjs: comment refs.
- tests/detect-antipatterns{,-browser,-fixtures}.test.{js,mjs},
  tests/windows-path-fix.test.js: import + read paths.
- AGENTS.md, CLAUDE.md: doc paths.

Verified:
- npx node cli/bin/cli.js --version, --help, detect --help all work.
- bun run build, bun run build:browser, bun run build:extension all
  clean. Browser detector lands at cli/engine/detect-antipatterns-browser.js;
  extension/detector/detect.js still emits to the same location.
- bun run test: 186/186 pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: update browser-detector paths missed in cli/ rename

Bugbot caught two runtime path leaks where the comment got renamed
to cli/engine/ but the actual code still used the old src/ segment.

- skill/scripts/live-server.mjs: detectPaths array now joins cli, engine,
  detect-antipatterns-browser.js for both the repo-relative lookup
  (4 dirs up from .claude/skills/impeccable/scripts/ to repo root) and
  the npm node_modules fallback. Without this fix, the detection
  overlay would silently not load during live-server sessions.

- scripts/build.js: the post-build copy of the browser detector into
  site/public/js/ was reading from src/. The if (fs.existsSync(...))
  guard meant the copy was silently skipping, so antipattern-examples
  pages would 404 on /js/detect-antipatterns-browser.js once the site
  was deployed.

Tests: 186/186 pass. Build clean. site/public/js/detect-antipatterns-browser.js
re-emits as expected.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: cleanup-deprecated import path missed an extra .. in cli/ rename

Bugbot caught three call sites in cli/bin/commands/skills.mjs that
import '../../skill/scripts/cleanup-deprecated.mjs'. Pre-rename, that
was correct from bin/commands/ (one parent to bin/, one to repo root).
After moving the file from bin/commands/ to cli/bin/commands/, the
path is one directory deeper, so it needs three .. segments to reach
the repo root. Without the fix, every cleanup invocation throws on
import and gets swallowed by the surrounding try/catch — silent skip.

cli/bin/cli.js's package.json read already uses '../../package.json'
(the same depth pattern), confirming three levels is correct.

Verified: dynamic import resolves and exports the expected functions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore: sweep stale path/file references missed in the restructure

Same root cause as the two bugbot finds: some references in moved or
related files weren't tracked because they didn't match a simple
sed pattern. Caught the rest by walking each moved dir's depth and
each Astro-migration deletion.

Stale path references (post-Astro migration, missed earlier):
- CLAUDE.md: legacy URL redirects "live in server/index.js" -> point
  at the actual sources (scripts/build.js generateCFConfig +
  site/public/_redirects).
- AGENTS.md: counts.js path (public/ -> site/public/), changelog file
  (public/index.html -> site/pages/index.astro), screenshots note
  (public/ -> site/), source-of-truth dirs (source/, src/ -> skill/,
  cli/).
- tests/detect-antipatterns-browser.test.mjs: comment about routes
  "in server/index.js".
- skill/reference/live.md: workflow.css example for "this repo" was
  pre-Astro (public/css/) -> site/styles/. (User-project Vite/Next
  example unchanged.)

Stale path that pointed at moved files:
- tests/skills-cli.test.js: CLI path was '..', 'bin', 'cli.js'; now
  '..', 'cli', 'bin', 'cli.js'. Test isn't wired into bun run test
  but it would have failed if invoked.

Dead files (orphaned by Astro migration, never cleaned up):
- tests/server/download-validation.test.js: imported from
  ../../server/lib/{validation,api-handlers}.js which were deleted in
  b8f09c8. Test was a silent failure waiting to happen.
- scripts/lib/render-markdown.js: 156-line module with zero consumers
  (the only caller, scripts/lib/render-page.js, was deleted in the
  Astro cleanup).
- scripts/build.js: dead commented-out generateSubPages import.

Tests: 186/186 pass. Build clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(build): remove invalid Corepack packageManager spec

Cloudflare Pages rejects the build with `Unsupported package manager
specification (bun@1.3.11)`. The packageManager field follows
Corepack's syntax which only validates npm/pnpm/yarn — `bun@X.Y.Z`
parses as a malformed Corepack directive even though Bun itself
treats it as a hint.

Pre-existing on main since d874af0 (CF Pages deploy on main also
failing); just surfaces here because the PR triggers a fresh deploy.

CF Pages auto-detects Bun anyway (the build log confirms:
"Detected the following tools from environment: bun@1.3.11,
pnpm@10.11.1, nodejs@22.16.0"). Removing the field unblocks the
deploy without changing local dev behavior.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Paul Bakaus <paulbakaus@pauls-mbp-3.lan>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-04 16:38:03 -07:00

270 lines
16 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# ADR: Live Variant Mode
**Status:** Implemented (v3.1, feature branch `feature/single-skill-consolidation`)
**Date:** 2026-04-12
**Author:** Paul Bakaus + Claude
## Context
Impeccable is a design skill for AI coding agents. It teaches AI harnesses (Claude Code, Cursor, Gemini CLI, Codex, etc.) how to produce better frontend design. The skill has 22 commands (bolder, quieter, polish, typeset, etc.) that the agent runs on source code.
The missing piece: there was no way to visually iterate on a live page. The user could ask the agent to "make this bolder," but they had to read the code diff, reload the page, and decide if they liked it. If not, they'd ask again, wait, reload, repeat. Slow and disconnected.
**Goal:** Let the user select an element directly in the browser, pick a design action, and see N real HTML+CSS variants hot-swapped in. Cycle through them visually, accept or discard, repeat. The agent generates the variants; the browser shows them.
## Decision
Build a self-contained live variant mode that ships as part of the impeccable skill (no separate npm install required). The system bridges three parties: the **browser** (where the user picks elements and cycles variants), the **server** (a localhost HTTP server that relays messages), and the **agent** (the AI that generates variants by modifying source files).
### Key architectural decisions
**1. Source modification, not DOM patching.**
Variants are written to the actual source file, not injected into the browser DOM. This means:
- Framework state (React, Vue, etc.) is preserved because the framework's own rendering pipeline handles the update via HMR.
- "Accept" is trivial: the winning variant is already in the source. Just remove the other variants.
- Variants are real code that the user can inspect in their editor, diff, and commit.
**2. SSE + fetch, not WebSocket.**
Server-Sent Events (server to browser) + fetch POST (browser to server) instead of WebSocket. This eliminates the `ws` npm dependency entirely. The server is zero-dependency pure Node.js (http, crypto, fs, net, os). This matters because the scripts ship inside the skill directory and run in the user's project without any package installation.
**3. Self-contained skill scripts.**
All live mode code lives in `skill/scripts/`:
- `live-server.mjs` — HTTP server (SSE, poll, source file reader)
- `live-poll.mjs` — CLI client for the agent poll/reply loop
- `live-wrap.mjs` — CLI helper that finds elements in source and creates variant wrappers
- `live-browser.js` — Browser script (element picker, action panel, variant cycler, global bar)
When a user installs the skill via `npx skills add pbakaus/impeccable`, they get the live mode without any additional setup. The agent runs the scripts via `node {{scripts_path}}/live-server.mjs`.
**4. HTTP long-poll for the agent, not WebSocket or stdin.**
The agent communicates with the server via HTTP long-poll (`GET /poll` blocks until a browser event arrives). This works across all AI harnesses because every harness can run a shell command and read its stdout. No harness-specific integration needed.
**5. `display: contents` variant wrapper.**
Variants are wrapped in a container with `display: contents`, which makes the wrapper invisible to CSS layout. The selected element's relationship with its parent (flex child, grid child, etc.) is preserved. The wrapper carries `data-impeccable-variants` and `data-impeccable-variant-count` attributes that the browser script uses to detect and cycle variants.
**6. No-HMR fallback.**
For dev servers that don't support HMR (like Bun's static HTML import), the browser fetches the raw source file directly from the live server's `/source` endpoint and injects the variants into the DOM. This works universally, at the cost of losing framework state on that injection.
## Architecture
```
┌─────────────────────────────────────────────────────────────────┐
│ BROWSER │
│ │
│ live-browser.js (injected via <script> tag in source HTML) │
│ ├── Element picker (mousemove highlight, click select, │
│ │ keyboard nav: arrows=siblings, shift+arrows=parent/child) │
│ ├── Action bar (floating pill: action picker, freeform input, │
│ │ variant count, go button; morphs to generating/cycling) │
│ ├── Global bar (bottom pill: Detect toggle, Pick toggle, Exit) │
│ ├── Variant cycler (MutationObserver watches for new │
│ │ [data-impeccable-variant] children in DOM) │
│ ├── SSE connection (EventSource → /events for server push) │
│ ├── fetch POST (→ /events for browser-to-server messages) │
│ └── localStorage (session state survives reloads) │
│ │
└────────────┬──────────────────────────────────┬─────────────────┘
│ SSE (server → browser) │ POST (browser → server)
│ EventSource /events │ fetch /events
▼ ▼
┌─────────────────────────────────────────────────────────────────┐
│ LIVE SERVER │
│ │
│ live-server.mjs (node, localhost:8400+, zero dependencies) │
│ ├── GET /live.js — browser script (token injected) │
│ ├── GET /detect.js — anti-pattern overlay (backwards compat)│
│ ├── GET /events — SSE stream to browser (server push) │
│ ├── POST /events — browser events (generate, accept, etc.)│
│ ├── GET /poll — agent long-poll (blocks until event) │
│ ├── POST /poll — agent reply (forwarded to browser SSE) │
│ ├── GET /source — raw file reader (no-HMR fallback) │
│ ├── GET /health — status, port, connected clients │
│ └── GET /stop — graceful shutdown │
│ │
│ State: session token, SSE client set, event queue, poll queue │
│ Server file: .impeccable/live/server.json (project root) │
│ │
└────────────┬──────────────────────────────────┬─────────────────┘
│ GET /poll (long-poll) │ POST /poll (reply)
│ blocks until browser event │ forwarded to SSE
▼ ▲
┌─────────────────────────────────────────────────────────────────┐
│ AGENT │
│ │
│ Follows skill/reference/live.md │
│ 1. Start server: node scripts_path/live-server.mjs & │
│ 2. Inject <script> into source HTML (comment-marked) │
│ 3. Poll loop: │
│ ├── generate → wrap + write variants + reply done │
│ ├── accept → present variant code + cleanup + reply done │
│ ├── discard → restore original + reply done │
│ ├── exit → cleanup script tag + stop server │
│ └── timeout → re-poll │
│ │
│ Tools used per generation: │
│ 1. node live-wrap.mjs (find element, create wrapper) │
│ 2. Edit (write all variants in single edit) │
│ 3. node live-poll.mjs --reply <id> done --file <path> │
│ │
└─────────────────────────────────────────────────────────────────┘
```
## Message flow
### Generate variants
```
User clicks element → picks "Bolder" → clicks Go
Browser POST /events: {type:"generate", id:"abc", action:"bolder", count:3, element:{...}}
Server enqueues event
Agent GET /poll returns: {type:"generate", id:"abc", action:"bolder", count:3, element:{...}}
Agent runs: node live-wrap.mjs --id abc --count 3 --classes "hero-left"
→ Finds element in source, wraps with data-impeccable-variants container
→ Original stays visible (no flash of empty content)
Agent writes all 3 variants in a single Edit tool call
→ Each variant is a <div data-impeccable-variant="N"> with full HTML replacement
→ First variant visible, others display:none
Agent POST /poll: {id:"abc", type:"done", file:"public/index.html"}
Server forwards via SSE to browser: {type:"done", file:"public/index.html"}
Browser checks: variants in DOM? (HMR)
YES → MutationObserver detected them → show cycling bar
NO → fetch /source?path=public/index.html → parse → inject → show cycling bar
```
### Accept variant
```
User clicks Accept on variant 2
Browser POST /events: {type:"accept", id:"abc", variantId:"2"}
Browser shows "Applying variant..." spinner
Agent GET /poll returns: {type:"accept", id:"abc", variantId:"2"}
Agent reads variant 2 HTML from source, presents to user
Agent removes variant wrapper, restores clean source
Agent POST /poll: {id:"abc", type:"done"}
Browser receives done via SSE → green "Variant applied" confirmation → auto-dismiss
```
### Discard
```
User clicks Discard (or presses Escape)
Browser POST /events: {type:"discard", id:"abc"}
Browser immediately: restores original element in DOM, hides bar, resets to PICKING
Agent GET /poll returns: {type:"discard", id:"abc"}
Agent removes variant wrapper from source, restores original
Agent POST /poll: {id:"abc", type:"done"}
```
## Variant wrapper format
In the source file (HTML example):
```html
<!-- impeccable-variants-start abc12345 -->
<div data-impeccable-variants="abc12345" data-impeccable-variant-count="3" style="display: contents">
<div data-impeccable-variant="original">
<!-- original element (visible until first variant arrives) -->
</div>
<div data-impeccable-variant="1">
<!-- variant 1 (visible) -->
</div>
<div data-impeccable-variant="2" style="display: none">
<!-- variant 2 -->
</div>
<div data-impeccable-variant="3" style="display: none">
<!-- variant 3 -->
</div>
</div>
<!-- impeccable-variants-end abc12345 -->
```
Comment markers enable deterministic cleanup. `display: contents` on the wrapper preserves flex/grid layout. The `data-impeccable-variant-count` attribute tells the browser how many to expect.
## Browser UI
### Global bar (always visible during live mode)
Compact floating pill at bottom center. Light, translucent, matching the brand aesthetic.
- **Impeccable** brand mark (magenta)
- **Detect** toggle (eye icon): loads anti-pattern scanner in extension mode, shows overlay count badge
- **Pick** toggle (crosshair icon): enables/disables element picker (default: on)
- **Exit** button (x): sends exit event, tears down all UI
When both Detect and Pick are active, detect overlays get `pointer-events: none` so the picker sees through them. The picker's z-index (100001) is above detect overlays (99999).
### Action bar (floating, contextual)
Appears below the selected element. Morphs between states:
- **Configure**: `[Action pill ▾] [freeform input] [×3] [Go →]`
- **Generating**: `[Action label] [● ● ○] Generating 2 of 3...`
- **Cycling**: `[←] [● ● ●] 2/3 [→] [✓ Accept] [✕]`
- **Saving**: `[spinner] Applying variant...`
- **Confirmed**: `[✓ Variant applied]` (green, auto-dismisses after 1.8s)
## Session persistence
`localStorage` stores:
- Session state (id, action, count, arrived variants, visible variant)
- Handled sessions (accepted/discarded session IDs)
This survives page reloads, browser close/reopen, HMR, and accidental refreshes. On page load, `resumeSession()` checks for an active variant wrapper in the DOM + the localStorage state and resumes the correct cycling position.
## Security
- **Session token**: `crypto.randomUUID()`, checked on all mutating endpoints and SSE connections.
- **Localhost only**: server binds to `127.0.0.1`, not `0.0.0.0`.
- **Token in server file**: `.impeccable/live/server.json` in project root. Only the user's processes can read it.
- **Token injected into `/live.js`**: the server prepends `window.__IMPECCABLE_TOKEN__` at serve time.
- **Path traversal guard**: `/source` endpoint validates the requested path is within `process.cwd()`.
- **No eval/innerHTML**: all browser UI built with `createElement` and `textContent`.
## Server resilience
- **Debounced exit**: when all SSE clients disconnect, the server waits 8 seconds before signaling exit to the agent. This avoids false exits from HMR reloads and brief network blips.
- **Stale PID detection**: on startup, the server checks if an existing PID file's process is still running. Dead processes are cleaned up automatically.
- **Browser server-lost handling**: after 5 failed SSE reconnection attempts, the browser cleans up all UI and shows a "Live server disconnected" toast.
## Performance optimizations
The generation loop was optimized from ~40s to ~15-20s:
1. **`wrap` CLI helper**: one command replaces 3-4 agent tool calls (grep + read + edit). Finds the element by ID/class/tag priority, creates the variant wrapper, returns the file path and insert line.
2. **Batch variant writes**: all variants in a single file edit instead of one per variant. Saves N-1 tool call round-trips.
3. **Page URL hint**: browser includes `location.pathname` in the generate event so the agent can map URL to source file directly.
Net effect: 4 tool calls (wrap + edit + read + reply) instead of 8+.
## Test coverage
- **26 wrap tests** (`tests/live-wrap.test.mjs`): unit tests for `buildSearchQueries`, `findElement`, `findClosingLine`, `detectCommentSyntax` + integration tests for full `wrapCli` on HTML and JSX fixtures.
- **15 server tests** (`tests/live-server.test.mjs`): integration tests that start a real server, test all endpoints, verify browser→agent event flow and agent→browser SSE delivery.
## Known limitations
- **Bun's static HTML import**: Bun's `import from "index.html"` caches at module load time. Source changes require a server restart to appear in the served HTML. The no-HMR fallback (fetch from `/source`) handles this, but it's less seamless than Vite/Next.js where HMR works natively.
- **Single generation at a time**: only one generate/cycle session can be active. This is by design (the source file can only have one variant wrapper at a time).
- **Inspection-only accept (v1)**: accepting a variant presents the code to the user and cleans up the scaffolding. Write-back (keeping the variant in place) is planned for v2.
- **No cancel during generation**: once the agent starts generating, it finishes all variants before the user can interact again.
## Future work
- **Write-back on accept**: instead of restoring the original after accept, keep the accepted variant as the new source.
- **Detect-to-fix flow**: clicking a detected anti-pattern pre-selects that element and pre-fills the action (e.g., "overused font" pre-fills "typeset").
- **Background poll for Claude Code**: use `run_in_background: true` to keep the main conversation free during the poll loop.
- **Streaming variants**: progressive reveal as the agent writes each variant (currently batched for speed).