mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-13 06:36:26 +03:00
e587004ee4
* 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 inb8f09c8. 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 sinced874af0(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>
70 lines
8.5 KiB
Markdown
70 lines
8.5 KiB
Markdown
# 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 Bun’s test runner plus Node’s 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.
|