Repository navigation
docs: new-component checklist, bounded CLAUDE.md deltas, /pre-pr command - #269
Conversation
…a /pre-pr command CONTRIBUTING-COMPONENTS.md is the machine-followable definition of complete for a new component (58 lines, checkbox/table). CLAUDE.md layers gain only what review rounds on PR #257 proved missing; net growth across the four files is ~11% after pruning now-CI-covered review rules.
WalkthroughThe PR adds a component contribution checklist and updates repository guidance for conformance gates, React compatibility, testing coverage, theming, API documentation, and story, documentation, and skill synchronization. ChangesContribution guidance
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Preview DeploymentPreview URL: https://f12303a6.bestax.pages.dev |
| # New component checklist | ||
|
|
||
| The definition of complete for adding a component to `@allxsmith/bestax-bulma` — for humans | ||
| and AI agents alike. `pnpm check:conformance` and CI enforce most of it; this file is the |
There was a problem hiding this comment.
pnpm check:conformance does not exist — 🟠 Major · Correctness
What: This checklist and three CLAUDE.md files present pnpm check:conformance as a real, CI-enforced gate, but no such npm script, CI step, or implementing file exists in the repo.
Why it matters: These files are read by humans and AI agents (Claude Action, CodeRabbit). An agent following the checklist runs pnpm check:conformance in §4 → ERR_PNPM_NO_SCRIPT → the "gate" hard-fails on a command that was never added. Worse, the docs assert CI enforces house conventions through it, so a reader believes conventions are checked when nothing checks them.
Evidence
- Root
package.jsonscripts: onlygen:catalog/gen:catalog:check— nocheck:conformance. grep -rniI conformance(excluding node_modules/.git) matches only the docs being added/edited here — no script or workflow..github/workflows/ci.ymlsteps: gen:catalog:check, build, typecheck, test, test:coverage, bundle:stats, lint, format:check, audit, build-storybook, + React 18/19 matrix. No conformance step and no story/docs "existence" check.
Referenced (all currently false):
CONTRIBUTING-COMPONENTS.md:4and:49CLAUDE.md("House conventions fail viapnpm check:conformance")bulma-ui/CLAUDE.md:31("CI'scheck:conformanceenforces")docs/CLAUDE.md:37("check:conformanceenforces the required sections")
Fix: Either land the check:conformance script + CI step in this PR, or reword every reference to describe it as proposed/not-yet-implemented (and drop the "CI enforces" / "CI only checks that a story and docs page exist" claims until the check is real).
| The definition of complete for adding a component to `@allxsmith/bestax-bulma` — for humans | ||
| and AI agents alike. `pnpm check:conformance` and CI enforce most of it; this file is the | ||
| full sequence. Conventions live in the folder `CLAUDE.md`s; templates in | ||
| `skills/bestax-custom-component/references/library-contributor.md`. |
There was a problem hiding this comment.
Broken reference: library-contributor.md — 🟡 Minor · Correctness
What: This points readers to skills/bestax-custom-component/references/library-contributor.md for templates, but that file does not exist. The references folder contains api.md, component-catalog.md, and patterns.md — the worked template/example lives in patterns.md (the canonical Dialog walkthrough).
Why it matters: An agent or contributor following this checklist chases a dead path and can't find the templates it promises.
Fix:
| `skills/bestax-custom-component/references/library-contributor.md`. | |
| `skills/bestax-custom-component/references/patterns.md`. |
There was a problem hiding this comment.
Deep review — 2 finding(s)
| No. | Severity | Area | Finding | Location |
|---|---|---|---|---|
| 1 | 🟠 Major | Correctness | pnpm check:conformance is presented as a real, CI-enforced gate but no script, CI step, or implementing file exists anywhere in the repo |
CONTRIBUTING-COMPONENTS.md:4,49, CLAUDE.md, bulma-ui/CLAUDE.md:31, docs/CLAUDE.md:37 |
| 2 | 🟡 Minor | Correctness | Points to references/library-contributor.md, which does not exist (templates are in patterns.md) |
CONTRIBUTING-COMPONENTS.md:6 |
Overall: This is a docs-only PR, so the risk is purely factual accuracy — and because these CLAUDE.md files are consumed by AI agents (Claude Action, CodeRabbit) as well as humans, a wrong instruction actively misleads. The changes are otherwise sound: every other referenced path (avatar.md, the EnhancedAddons/icons.js homepage surfaces, componentCategories.js, the theming skill references, Reveal.test.tsx with its renderToStaticMarkup SSR pattern) and the React 18/19 matrix claim all check out against the tree. The human should focus on finding number 1: decide whether to ship the check:conformance script + CI step in this PR, or reword the "CI enforces" claims until the check actually exists — an agent that runs the checklist will hit ERR_PNPM_NO_SCRIPT today.
🏄 Solid doc cleanup, dude — the map is mostly drawn right and the React 18/19 swell is real. Just do not send folks paddling out to a
check:conformancebreak that has not formed yet; patch that one call-out and it is all smooth glass.
|
@claude Full re-review of this PR at current head 607146d (you have 60 turns, diff tools, and pnpm).
Finish by updating your comment with a findings table (or "none") and an overall ship/fix-first verdict including the merge-order requirement. Generated by Claude Code |
|
Claude finished @allxsmith's task in 5m 30s —— View job Full re-review at 607146d
Findings
Everything else checks out against the current tree: Merge-order requirementConfirmed: #267 and #268 don't depend on anything from this PR, so the only safe order is #267 → #268 → this PR (any order between #267/#268). If this PR merges before either, Verdict: Fix-firstFinding #1 is new and blocking on its own — the PR doesn't deliver what its title and description promise. Findings #2/#3 are the same cross-PR ordering issues the last deep review caught; they resolve automatically once #267 and #268 merge first, but this PR must not merge ahead of them. |
Preview DeploymentPreview URL: https://3803629b.bestax.pages.dev |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CLAUDE.md (1)
35-45: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep
pnpm allaligned with the required pre-PR checks.pnpm alldoes not includepnpm check:conformance, so the documented pre-PR gate can still miss a repo-wide requirement. Add it toallor list it as a separate required step.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CLAUDE.md` around lines 35 - 45, Update the pnpm “all” pre-PR check configuration to include check:conformance, or explicitly document pnpm check:conformance as a separate required step alongside pnpm all. Keep the documented pre-PR workflow aligned so the repo-wide conformance check cannot be omitted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@CLAUDE.md`:
- Around line 35-45: Update the pnpm “all” pre-PR check configuration to include
check:conformance, or explicitly document pnpm check:conformance as a separate
required step alongside pnpm all. Keep the documented pre-PR workflow aligned so
the repo-wide conformance check cannot be omitted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6c223abc-5a23-4e7b-a199-0e5c7a0172ed
📒 Files selected for processing (1)
CLAUDE.md
| # New component checklist | ||
|
|
||
| The definition of complete for adding a component to `@allxsmith/bestax-bulma` — for humans | ||
| and AI agents alike. `pnpm check:conformance` and CI enforce most of it; this file is the |
There was a problem hiding this comment.
pnpm check:conformance does not exist — 🟠 Major · Correctness
What: This checklist (and three other files in this PR) present pnpm check:conformance as an existing, CI-enforced gate, but there is no such script anywhere in the repo — no check:conformance (or any check*) entry in the root/bulma-ui/docs package.json, no matching turbo.json task, and no CI step. scripts/ contains only gen-component-catalog.mjs.
Why it matters: These files are the machine-followable spec read by AI agents and CodeRabbit (root CLAUDE.md: "keep it accurate"). An agent or contributor who runs pnpm check:conformance gets Command "check:conformance" not found, and the repeated claim that "CI enforce most of it" / "House conventions fail via pnpm check:conformance" is simply false — CI enforces none of these house conventions.
All five references to the phantom script
| File | Line | Text |
|---|---|---|
CONTRIBUTING-COMPONENTS.md |
4 | "pnpm check:conformance and CI enforce most of it" |
CONTRIBUTING-COMPONENTS.md |
49 | "pnpm check:conformance and pnpm gen:catalog:check" |
docs/CLAUDE.md |
37 | "check:conformance enforces the required sections" |
bulma-ui/CLAUDE.md |
31 | "CI's check:conformance enforces" |
root CLAUDE.md (diff) |
+38 | "House conventions fail via pnpm check:conformance" |
Fix: Either add the check:conformance script + CI wiring in this PR, or remove/reword every reference to describe only gates that actually exist (pnpm gen:catalog:check, pnpm all, review-time checks). For this line:
| and AI agents alike. `pnpm check:conformance` and CI enforce most of it; this file is the | |
| and AI agents alike. `pnpm gen:catalog:check` and CI enforce part of it, and reviewers enforce | |
| the rest; this file is the |
| The definition of complete for adding a component to `@allxsmith/bestax-bulma` — for humans | ||
| and AI agents alike. `pnpm check:conformance` and CI enforce most of it; this file is the | ||
| full sequence. Conventions live in the folder `CLAUDE.md`s; templates in | ||
| `skills/bestax-custom-component/references/library-contributor.md`. |
There was a problem hiding this comment.
Broken template reference — library-contributor.md does not exist — 🟠 Major · Correctness
What: This points readers to skills/bestax-custom-component/references/library-contributor.md for templates, but that file does not exist. The references/ directory contains only api.md, component-catalog.md, and patterns.md.
Why it matters: The first thing this checklist tells an agent/contributor to open for templates is a dead path. The worked-example templates actually live in patterns.md (Dialog reference implementation), which SKILL.md:149 cites as "references/patterns.md for the complete [worked example]".
Fix:
| `skills/bestax-custom-component/references/library-contributor.md`. | |
| full sequence. Conventions live in the folder `CLAUDE.md`s; templates in | |
| `skills/bestax-custom-component/references/patterns.md`. |
There was a problem hiding this comment.
Deep review — 2 finding(s)
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| 1 | 🟠 Major | Correctness | pnpm check:conformance is presented as an existing CI-enforced gate, but no such script exists anywhere (5 references across 4 files) |
CONTRIBUTING-COMPONENTS.md:4,49, docs/CLAUDE.md:37, bulma-ui/CLAUDE.md:31, CLAUDE.md |
| 2 | 🟠 Major | Correctness | Template pointer to references/library-contributor.md is a dead path; the file does not exist (dir has api.md, component-catalog.md, patterns.md) |
CONTRIBUTING-COMPONENTS.md:6 |
Overall: This is a docs-only PR that adds a genuinely useful, well-structured new-component checklist, and most of its many file/path references check out (avatar.md, Reveal.test.tsx, componentCategories.js, EnhancedAddons/index.js + icons.js, the theming/skills references all exist, and the React 18/19 matrix claim matches the real react-compat CI job). The riskiest part is factual accuracy: because these files are the machine-followable spec read by AI agents and CodeRabbit, the two invented references — a check:conformance script that exists nowhere and a library-contributor.md template that was never created — will actively mislead. The human should decide whether to add check:conformance in this PR or strip every reference to it, and repoint the template link to patterns.md, before merging.
🏄 Clean little docs wave, dude — the checklist paddles out smooth and most of the breaks line up. Just two phantom reefs under the surface: a
check:conformancecommand that ain't in the water and a template file that never showed. Patch those and it's a mellow green-light ride.
|
🎉 This PR is included in version 3.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
…des (#292) PR A of #289: Bun-parity fan-out triage. Always-on (config-driven) with a fail-closed daily budget counter on tracking issue #290, label mode kept, .claude/commands/ established (triage-dedupe, triage-find-issues, triage-find-duplicate-prs, pre-pr), docs updated. Refs #289 See #269, #274, #277
|
🎉 This PR is included in version 5.4.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
Part of the AI-enrichment plan (PR 3): the bounded-prose layer. One on-demand checklist file,
minimal always-loaded CLAUDE.md deltas, and a
/pre-prcommand.Affected package(s):
CONTRIBUTING-COMPONENTS.md,.claude/commands/), CLAUDE.md layersRelated Issue(s)
Refs #263
Type of Change
What changed
CONTRIBUTING-COMPONENTS.md(58 lines, checkboxes/tables only): the definition ofcomplete for a new component — classify stock-vs-extra first, the artifact list, the docs
listing surfaces (the step every recent component PR missed), skills sync, gates
(including the React 18/19 matrix), and a pre-PR self-review list. Loaded on demand; the
loop's prompts will cite it (follow-up
ci:PR).bulma-ui/CLAUDE.md: checklist pointer; coverage techniques pointer (Reveal.test.tsx);story conventions (react-vite, autodocs, argType descriptions); no-inline-style rule
("legacy inline styles exist — don't copy them"); React 18/19 line.
docs/CLAUDE.md: avatar.md named as the exemplar (replacing "mirror a sibling page",which pointed agents at inconsistent older pages); frontmatter
title:/Overview sentenceare load-bearing for
gen:catalog; no-inline-style rule.bulma-ui/src/scss/CLAUDE.md: register every themable value (durations/offsets);Bulma tokens over literals (
cv.getVar('radius-rounded'), never9999px); scheme tokensor it's a dark-mode bug; themeable-components.md rows in the same PR.
CLAUDE.md: conformance + React-matrix lines under CI gates; checklist pointer.check:conformance(ci: add conformance gates for house conventions (listings, docs sections, SCSS, stories, inline-style) #267) now covers..claude/commands/pre-pr.md: one-shot/pre-pr— run the full gate, then self-reviewthe diff against the checklist (the parts CI can't verify).
Context budget: the four CLAUDE.md files went 12,608 → 14,123 bytes (+12%) — measured, not
estimated; every added line traces to a specific #257 correction round or a latent trap
(React matrix was documented nowhere).
Checklist
CLAUDE.mdfiles are updated (that is the PR)Test plan
pnpm run format:checkbulma-ui/CLAUDE.md(different sections)🤖 Generated with Claude Code
Summary by CodeRabbit