Files
pbakaus_impeccable/AGENTS.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

8.5 KiB
Raw Blame History

Repository Guidelines

Skill bootstrap: skip the post-update-cleanup step

If you load the Impeccable skill from .agents/skills/impeccable/SKILL.md (or any sibling harness directory under this repo), it begins with a <post-update-cleanup> block that runs scripts/cleanup-deprecated.mjs and then self-deletes. Do not run it here, and do not delete the block. This repo is the source of truth for the skill; cleanup-deprecated assumes a consumer project and would touch legitimate source files. The block is regenerated from skill/SKILL.md on every bun run build, so any deletion churns back. Skip it silently and proceed with the task.

Project Structure & Module Organization

source/ is the source of truth. Author skills in skill/ and keep provider output in dist/ generated, not hand-edited. Build logic lives in scripts/, with provider configs in scripts/lib/transformers/. Runtime detection code ships from src/. The website lives in public/, local API/dev serving lives in server/, and regression coverage lives in tests/ with fixtures under tests/fixtures/.

Build, Test, and Development Commands

  • bun run dev - start the local Bun server.
  • bun run build - regenerate dist/, derived site assets, and validation output.
  • bun run rebuild - clean and rebuild everything from scratch.
  • bun test tests/build.test.js - run a focused Bun test.
  • bun run test - run the full Bun + Node test suite.
  • bun run test:live-e2e - opt-in live-mode E2E against framework fixtures (~2 min; needs npx playwright install chromium once).
  • bun run build:browser / bun run build:extension - rebuild browser-specific bundles.

Run bun run build after changing anything in source/, transformer code, or user-facing counts.

Sandbox gotchas for Codex agents

Some repo workflows need to run outside the sandbox in the desktop app:

  • GitHub SSH operations that depend on the 1Password SSH agent, such as gh pr checkout, may fail in the sandbox with sign_and_send_pubkey or no 1Password approval prompt. Rerun them outside the sandbox instead of falling back to unrelated workarounds.
  • bun run build rewrites committed harness directories such as .agents/skills/. In the sandbox, Bun can hit filesystem errors while removing/recreating those trees (for example EFAULT on .agents/skills). Rerun the build outside the sandbox before treating it as a real build failure.
  • Puppeteer/headless-Chrome tests, especially node --test tests/detect-antipatterns-browser.test.mjs and the browser portion of bun run test, can hang in the sandbox while launching Chrome. Run them outside the sandbox for authoritative results.
  • The jsdom fixture suite is intentionally run with Node, not Bun: use node --test tests/detect-antipatterns-fixtures.test.mjs or the bun run test script. A direct bun test tests/detect-antipatterns-fixtures.test.mjs can time out and is not the supported signal.

Coding Style & Naming Conventions

Use ESM, semicolons, and the existing two-space indentation style in JS, HTML, and CSS. Prefer small, single-purpose modules over large abstractions. Keep filenames descriptive and lowercase with hyphens where needed; skill entrypoints stay as SKILL.md, helper scripts use .js or .mjs. In source frontmatter, use clear kebab-case names and concise descriptions. There is no dedicated formatter or linter configured here, so match surrounding code closely.

Testing Guidelines

Tests use Buns test runner plus Nodes built-in --test. Name tests *.test.js or *.test.mjs and place new fixtures near the behavior they cover, usually under tests/fixtures/. Prefer targeted test runs while iterating, then finish with bun run test. If you change generated outputs or provider transforms, verify both source parsing and at least one affected provider path in dist/.

For changes to skill/scripts/live-*.{mjs,js}, also run bun run test:live-e2e (kept out of the default suite because it does real npm install per fixture and boots framework dev servers). Scope to one fixture with IMPECCABLE_E2E_ONLY=<fixture-name> while iterating; pass IMPECCABLE_E2E_DEBUG=1 for page-DOM and dev-server-log dumps on failure. Schema and authoring guide for new fixtures live in tests/framework-fixtures/README.md.

Set IMPECCABLE_E2E_AGENT=llm to swap the deterministic fake agent for a Claude-backed one (tests/live-e2e/agents/llm-agent.mjs, default Haiku 4.5, override via IMPECCABLE_E2E_LLM_MODEL). Requires ANTHROPIC_API_KEY; tests skip cleanly when it's unset. This path hits the API — use it for verification, not CI.

Anti-pattern detection rules

cli/engine/detect-antipatterns.mjs is the source of truth for the rule engine. It feeds the CLI, the site overlay (cli/engine/detect-antipatterns-browser.js, regenerated by bun run build:browser), the Chrome extension (extension/detector/, regenerated by bun run build:extension), and the homepage DETECTION_COUNT in site/public/js/generated/counts.js (regenerated by bun run build). After any rule change run all three builds plus bun run test so nothing drifts.

TDD order is non-negotiable:

  1. Add a fixture at tests/fixtures/antipatterns/{rule-id}.html with two columns (should-flag / should-pass), each case identified by a unique heading. ≥4 flag cases and ≥5 false-positive shapes. Use explicit pixel dimensions in CSS — jsdom does no layout.
  2. Add a failing test in tests/detect-antipatterns-fixtures.test.mjs using the snippet-substring pattern (regex /"([^"]+)"/ against SHOULD_FLAG / SHOULD_PASS lists).
  3. Add the rule entry to the ANTIPATTERNS array (id, category = slop or quality, name, description, optional skillSection / skillGuideline).
  4. Implement a pure checkXxx(opts) returning [{ id, snippet }] — no DOM access inside.
  5. Add two adapters that wrap the pure check: checkElementXxxDOM(el) for the browser (getComputedStyle + getBoundingClientRect) and checkElementXxx(el, tag, window) for jsdom (parseFloat(style.width) instead of layout). Wire both adapters into both element loops in cli/engine/detect-antipatterns.mjs (browser loop ~line 1837, jsdom loop in detectHtml ~line 2058). Forgetting one is the most common mistake.
  6. Verify on a live page at http://localhost:3000/fixtures/antipatterns/{rule-id}.html and on the homepage. The two adapter paths can disagree.

Conventions: wrap the identifying heading text in straight double quotes inside snippets so the fixture test can extract it. jsdom-specific helpers resolveBackground(), resolveGradientStops(), and parseGradientColors() exist because background: shorthand isn't decomposed and computed colors aren't normalized in jsdom — use them. Reference rules to copy from: side-tab (border), low-contrast (color+gradient), icon-tile-stack (sibling relationship), flat-type-hierarchy (page-level).

Commit & Pull Request Guidelines

Recent history favors short, imperative subjects such as Fix: ..., Add ..., Improve ..., or Bump .... Keep commits focused and explain the user-facing impact when it is not obvious. PRs should summarize what changed, list validation performed, and call out regenerated artifacts like dist/ or build/. Include screenshots for visible site/ changes and mention affected providers when transform behavior changes.

Releases

Tags are per-component because the three components ship independently: skill-v (.claude-plugin/plugin.json + .claude-plugin/marketplace.json), cli-v (package.json), ext-v (extension/manifest.json). Flow: bump the relevant manifest, add a changelog entry to site/pages/index.astro (skill = bare vX.Y.Z; CLI = CLI vX.Y.Z; extension = Extension vX.Y.Z — the prefix is how scripts/release.mjs finds the right block), commit, push, then bun run release:<skill|cli|ext> (or --dry-run first). The script refuses on a dirty tree, an unpushed HEAD, a missing changelog entry, or stale build outputs; skill and extension reruns of bun run build / bun run build:extension must produce zero diff. Skill releases attach dist/universal.zip; extension releases attach dist/extension.zip. CLI ships to npm via a separate npm publish, and the extension zip uploads to the Chrome Web Store manually — both reminded at the end of the script. Fix already-shipped notes with gh release edit <tag> --notes-file <md>.

Contributor Notes

Do not edit generated provider files directly unless you are intentionally patching generated output as part of a build-system change. Prefer fixing the root source in skill/, scripts/, or cli/, then regenerate artifacts.