From 28d0afba4ba0a766399bf7818aeb7861301c2f16 Mon Sep 17 00:00:00 2001 From: mhenrixon Date: Mon, 14 Sep 2026 00:56:18 +0200 Subject: [PATCH 1/2] chore: seed the lode and enable the pre-PR gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A fork of the code's own facts, so the next session does not have to re-derive them and the gate has something to enforce. lode/ now holds the baseline (summary, terminology, practices, lode-map, the plans pointer) and one folder per subsystem: form-api, inference, theming, client-validation, live-validation, packaging, testing and docs-site. Every claim was checked against the code, and every line range and count was computed with a script rather than read off the screen — the leaf/role/example counts, the def-to-end ranges, the 198 examples, the 19 (+2 reactive) theme roles. lode/workflow.md is the profile the shared /lode: workflow commands read. Its ten headings are filled from the code and config as they are, from CLAUDE.md and .claude/rules, and from the seven local commands this change retires — their conflict rules, CI runbook and constraint tables move there rather than being lost with the files. Two review rules, in lode/review/. From cubic (1 learning) and from the merged PR review threads (2 accepted findings across 22 merged PRs): three raw findings, merged to two, none dropped — the cubic learning and PR #24 are the same rule about docs/app/assets/stylesheets/tailwind.sources.css, so they share one entry. PR #23's "one ### subsection per release heading" is the other. Neither has a test; both say so. Retired the seven local commands the plugin supersedes: lfg, tdd, plan, github-review-pr, github-review-failures, github-review-comments, finish-prs. architect, security and review-pr are repo-specific and stay. CLAUDE.md's command table, its model-tier list, .claude/README.md and SKILL_TEMPLATE.md now name what actually exists. The plugin is enabled in .claude/settings.json, lode/tmp/ is gitignored, and CLAUDE.md and AGENTS.md both carry a Memory section pointing at lode-map.md. Ten doc-versus-code disagreements were found and deliberately not fixed here — a seeding change describes the tree it ships in. The sharpest: `bundle exec rake` is named as the pre-commit gate in two places and always fails, because RuboCop::RakeTask passes no paths and walks into docs/, which needs a gem from a different bundle. Claude-Session: https://claude.ai/code/session_01Hga3idYBp7fWdj7GmmfofF --- .claude/README.md | 15 +- .claude/SKILL_TEMPLATE.md | 2 +- .claude/commands/finish-prs.md | 141 ---------- .claude/commands/github-review-comments.md | 237 ----------------- .claude/commands/github-review-failures.md | 178 ------------- .claude/commands/github-review-pr.md | 163 ------------ .claude/commands/lfg.md | 284 --------------------- .claude/commands/plan.md | 92 ------- .claude/commands/tdd.md | 126 --------- .claude/settings.json | 13 + .gitignore | 3 + AGENTS.md | 7 + CLAUDE.md | 41 ++- lode/client-validation/summary.md | 139 ++++++++++ lode/docs-site/summary.md | 102 ++++++++ lode/form-api/summary.md | 166 ++++++++++++ lode/inference/summary.md | 112 ++++++++ lode/live-validation/summary.md | 111 ++++++++ lode/lode-map.md | 44 ++++ lode/packaging/summary.md | 142 +++++++++++ lode/plans/README.md | 17 ++ lode/practices.md | 87 +++++++ lode/review/changelog.md | 9 + lode/review/docs-site.md | 8 + lode/summary.md | 39 +++ lode/terminology.md | 58 +++++ lode/testing/summary.md | 89 +++++++ lode/theming/summary.md | 137 ++++++++++ lode/workflow.md | 187 ++++++++++++++ 29 files changed, 1510 insertions(+), 1239 deletions(-) delete mode 100644 .claude/commands/finish-prs.md delete mode 100644 .claude/commands/github-review-comments.md delete mode 100644 .claude/commands/github-review-failures.md delete mode 100644 .claude/commands/github-review-pr.md delete mode 100644 .claude/commands/lfg.md delete mode 100644 .claude/commands/plan.md delete mode 100644 .claude/commands/tdd.md create mode 100644 .claude/settings.json create mode 100644 lode/client-validation/summary.md create mode 100644 lode/docs-site/summary.md create mode 100644 lode/form-api/summary.md create mode 100644 lode/inference/summary.md create mode 100644 lode/live-validation/summary.md create mode 100644 lode/lode-map.md create mode 100644 lode/packaging/summary.md create mode 100644 lode/plans/README.md create mode 100644 lode/practices.md create mode 100644 lode/review/changelog.md create mode 100644 lode/review/docs-site.md create mode 100644 lode/summary.md create mode 100644 lode/terminology.md create mode 100644 lode/testing/summary.md create mode 100644 lode/theming/summary.md create mode 100644 lode/workflow.md diff --git a/.claude/README.md b/.claude/README.md index d0fd781..eafebe8 100644 --- a/.claude/README.md +++ b/.claude/README.md @@ -5,7 +5,10 @@ the whole team (and every autonomous session) shares the same conventions. ``` .claude/ -├── commands/ Slash commands (/lfg, /tdd, /plan, /security, …) — one markdown file each +├── commands/ This repo's own slash commands (/architect, /security, /review-pr) — one markdown +│ file each. The workflow commands (/lode:lfg, /lode:tdd, /lode:plan, /lode:review-pr, +│ /lode:finish-prs, /lode:debug-flaky) come from the lode@zoolutions plugin and read +│ lode/workflow.md for everything repo-specific. ├── rules/ Standing rules auto-loaded into context (coding-style, testing, git-workflow, agents) ├── README.md This file — how to author a command └── SKILL_TEMPLATE.md Copy-paste starting point for a new command @@ -14,7 +17,7 @@ the whole team (and every autonomous session) shares the same conventions. ## Anatomy of a command A command is a markdown file under `commands/` with a YAML frontmatter block -followed by the prompt body. `.claude/commands/tdd.md` is a good reference. +followed by the prompt body. `.claude/commands/security.md` is a good reference. ```markdown --- @@ -47,15 +50,15 @@ latest model in that tier, so a command never goes stale on an outdated pin. | Tier | Use for | Commands here | |------|---------|---------------| | `haiku` | Mechanical / config work, diff pattern-scanning | *(none yet)* | -| `sonnet` | Prescriptive, pattern-following passes with a tight prompt | `/github-review-comments`, `/github-review-failures` | -| `opus` | Orchestration, security, review synthesis, and reasoning-heavy specialists | `/lfg`, `/architect`, `/security`, `/review-pr`, `/github-review-pr`, `/tdd` | -| `fable` | Read-only planning that hands execution to cheaper models | `/plan` | +| `sonnet` | Prescriptive, pattern-following passes with a tight prompt | — | +| `opus` | Orchestration, security, review synthesis, and reasoning-heavy specialists | `/architect`, `/security`, `/review-pr` | +| `fable` | Read-only planning that hands execution to cheaper models | — | Rules of thumb: - **Always use the alias**, never `claude-opus-4-8` or another full model ID — aliases track the latest model per tier and never rot. -- **`fable` is pinned only on `/plan`.** For a plain interactive session, pick it +- **`fable` is pinned on no local command.** For a plain interactive session, pick it per-session with `/model` when you want the most capable model for architecture or the hardest debugging. - **Subagents don't inherit the tier for free.** When a command (or you) spawns a diff --git a/.claude/SKILL_TEMPLATE.md b/.claude/SKILL_TEMPLATE.md index a5ae1c3..696a598 100644 --- a/.claude/SKILL_TEMPLATE.md +++ b/.claude/SKILL_TEMPLATE.md @@ -10,7 +10,7 @@ work, `sonnet` for prescriptive pattern-following passes, `opus` for orchestration, security, review synthesis, and reasoning-heavy specialists. Always use the tier alias, never a full model ID — aliases track the latest model in the tier. Pin `fable` only on read-only planning commands that hand execution -to cheaper models (see `/plan`); otherwise choose it per-session with `/model`. +to cheaper models; otherwise choose it per-session with `/model`. ```markdown --- diff --git a/.claude/commands/finish-prs.md b/.claude/commands/finish-prs.md deleted file mode 100644 index 66f8099..0000000 --- a/.claude/commands/finish-prs.md +++ /dev/null @@ -1,141 +0,0 @@ ---- -description: "Drive a set of open PRs to merge-ready, one at a time, in a given order. Auto-resolves the recurring CHANGELOG [Unreleased] conflict, runs /github-review-pr (CI failures then review comments) on each, then waits for the user to merge before rebasing and advancing to the next. Use to clear a stack of stacked/parallel PRs without manual rebase churn." -model: opus -argument-hint: "ordered PR list (e.g. '292 288 289 293 294 295'); optional 'automerge' to enable gh auto-merge; empty = auto-discover your open PRs" -allowed-tools: Bash(gh pr list:*), Bash(gh pr view:*), Bash(gh pr checks:*), Bash(gh pr diff:*), Bash(gh pr comment:*), Bash(gh pr merge:*), Bash(gh api:*), Bash(gh run view:*), Bash(git:*), Bash(bundle:*), Bash(bundle exec:*), Bash(cd:*), Read, Write, Edit, Glob, Grep, Agent, Skill, TaskCreate, TaskUpdate, TaskGet, TaskList, ScheduleWakeup ---- - -# Finish PRs (ordered merge-ready loop): $ARGUMENTS - -You are driving a set of open pull requests to **merge-ready** state, one at a time, in a defined order, minimizing the manual rebase/CI back-and-forth that stacked or parallel PRs create. - -The one thing that makes a batch of PRs churn on this repo is **recurring and mechanical**, so this command resolves it automatically instead of surfacing it to the user each time: - -1. **CHANGELOG `[Unreleased]` conflicts** — every PR appends an entry under the same section, so each merge re-conflicts the rest. The resolution is always a *union at a known anchor* (`### Added` / `### Fixed` / `### Changed` / `### Removed` / `### Breaking Changes`). - -**This command does NOT merge PRs itself** unless the user passed `automerge`. Branch protection requires review approval, and the user typically wants to eyeball each merge. Default behavior: make each PR merge-ready, then pause and let the user merge; when a merge lands, rebase the remaining PRs and continue. - ---- - -## Phase 0: Parse the PR list and order - -`$ARGUMENTS` may be: - -- A space/comma-separated ordered list of PR numbers: `292 288 289 293 294 295` (also accepts `#292`, `PR292`). -- The word `automerge` anywhere in the args → enable `gh pr merge --auto --squash` on each PR once it is green + approved (still respects branch protection; GitHub merges when gates pass). Strip it out before parsing numbers. -- Empty → auto-discover: `gh pr list --author=@me --state=open --limit 100 --json number,title,headRefName,createdAt` and order **oldest-first** (`createdAt` ascending). The explicit `--limit` matters — `gh pr list` defaults to 30, so without it the discovery silently drops older PRs once the queue grows past 30. Oldest-first is the safe default: the earliest PR is usually the base others were cut from, so merging it first minimizes downstream rebases. Show the discovered order and proceed. - -**Order matters.** Each merge invalidates the others' merge base. Processing in a fixed order means you rebase each remaining PR exactly once per upstream merge, not repeatedly. If the user gave an explicit order, honor it exactly — they may know a dependency the metadata doesn't show. - -Create a task list (TaskCreate) with one task per PR, in order, so progress is visible. Mark the current PR `in_progress`. - -Confirm the plan in one line: `Finishing N PRs in order: #a → #b → #c. Mode: .` - ---- - -## Phase 1: Locate each PR's working tree - -For each PR you need a checkout of its branch to rebase and push. Prefer, in order: - -1. An existing worktree already on that branch: `git worktree list` — match the branch. (Parallel-agent runs leave worktrees under `.claude/worktrees/agent-*`.) -2. If none, create one: `git worktree add .claude/worktrees/finish- ` (fetch the branch first: `git fetch origin `). - -Never rebase a branch that is currently checked out in the **main working directory** — operate in a worktree so the user's main checkout is undisturbed. Never touch `main` directly. - ---- - -## Phase 2: Per-PR loop - -Process PRs strictly in order. For the current PR: - -### 2a. Sync onto latest main (rebase, auto-resolving the known CHANGELOG conflict) - -```bash -git fetch origin main --quiet -cd -git rebase origin/main -``` - -If the rebase stops on a conflict: - -- **`CHANGELOG.md`** — resolve as a union. The conflict is diff3-shaped at the top of an `### ` list: `main`'s entries on the HEAD side, this branch's new entry on the other. Keep **both**, in a sensible order (feature/fix entries before the pre-existing docs-site entry; this branch's own entry adjacent to the others in its category). Practically, for the common "both sides insert at the same anchor" case, strip the markers keeping both blocks: - - ```bash - perl -0pi -e 's/^<<<<<<< HEAD\n//mg; s/^\|\|\|\|\|\|\| [^\n]*\n=======\n//mg; s/^>>>>>>> [^\n]*\n//mg;' CHANGELOG.md - ``` - - Then **read the result** and verify: no conflict markers remain (`grep -n '^<<<<<<<\|^=======\|^>>>>>>>\|^|||||||' CHANGELOG.md`), this PR's `Refs #` entry is present exactly once, ordering reads cleanly, and no unrelated entry was dropped or duplicated. The perl is a fast path, not a substitute for reading — if the conflict is not the simple same-anchor shape, resolve it by hand. - -- **Any other conflicted file** — this command's auto-resolution covers only the one known-mechanical file. For anything else, STOP the rebase (`git rebase --abort`), report the conflicted file(s) to the user, and ask how to proceed. Do not guess at semantic conflicts. - -`git add` the resolved files and `git rebase --continue` (set `GIT_EDITOR=true` to accept the message). Repeat until the rebase completes. - -### 2b. Push the rebased branch - -```bash -git push --force-with-lease origin -``` - -`--force-with-lease` (never bare `--force`) so a concurrent push from the user aborts the overwrite instead of clobbering it. - -### 2c. Run the full review pass - -Invoke `/github-review-pr ` (via the Skill tool). It runs **CI failures first, then review comments** — do not re-implement its logic. It will: - -- Fix any red CI checks (lint, specs, build) and push. -- Address every unresolved review thread (CodeRabbit or human): implement valid fixes, push back with reasoning on wrong ones, resolve threads. - -Wait for it to finish. If it reports a persistent failure it could not fix (or a review thread it could not resolve without a decision), surface that to the user for this PR and move it to a `needs-user` state — do not block the whole queue on one stuck PR; note it and continue to the next PR, then return. - -### 2d. Verify merge-ready - -```bash -gh pr view --json mergeable,mergeStateStatus,reviewDecision --jq '{mergeable,mergeStateStatus,reviewDecision}' -gh pr checks -``` - -Merge-ready means: `mergeable=MERGEABLE`, no failing checks (green or pending-green), and `reviewDecision` is `APPROVED` or empty (not `CHANGES_REQUESTED`). A `BLOCKED` mergeStateStatus with everything else green usually means "awaiting required approval" — that is expected and fine; it is the user's/reviewer's gate, not a defect. - -### 2e. Hand off for merge - -- **`automerge` mode:** `gh pr merge --auto --squash` (GitHub merges when gates pass). Then go to Phase 3 to wait for the merge to land before advancing. -- **Default (pause) mode:** report this PR as ✅ merge-ready with its URL and a one-line "what's in it," and tell the user it's ready to merge. Then **wait** (Phase 3). - -Mark the PR's task `completed` (merge-ready) — or `needs-user` via a metadata note if it got stuck in 2c. - ---- - -## Phase 3: Wait for the merge, then advance - -The loop is **gated on the target PR merging**, because each merge is what invalidates the next PR's base. - -- **automerge mode:** poll `gh pr view --json state --jq .state` until `MERGED`. Use `ScheduleWakeup` with a delay matched to CI duration (this repo's checks run ~1–3 min; poll ~180s, staying inside the prompt-cache window) rather than a busy sleep. When merged, advance. -- **default mode:** the user merges manually and will tell you (or you are re-invoked). On the next turn, re-check `gh pr view --json state`. If `MERGED`, advance to the next PR in the list and repeat Phase 2 (its rebase now picks up the just-merged changes). If not yet merged, report current status and stop — do not spin. - -When you advance, **always re-fetch and rebase the next PR onto the new main** (Phase 2a) before doing anything else — the merge that just landed is exactly the change it needs to absorb. - -If the user merges a PR **out of the planned order**, adapt: drop it from the remaining list and rebase whatever is now next. - ---- - -## Phase 4: Final report - -When the queue is drained (all merged, or all merge-ready-and-handed-off, or blocked-on-user): - -| PR | Result | Note | -|----|--------|------| -| #a | ✅ merged / ✅ merge-ready / ⏳ awaiting-merge / ⚠️ needs-user | one line | - -Then: what the user must do next (merge the ready ones, decide on any `needs-user` items), and whether the recurring CHANGELOG conflict is worth fixing at the source. - ---- - -## Important notes - -- **Never bare `git push --force`** — always `--force-with-lease`. -- **Never rebase the branch checked out in the main working directory** — use a worktree. -- **Never auto-resolve a conflict outside the one known-mechanical file** (`CHANGELOG.md`). Stop and ask. -- **Never merge in default mode** — the user merges; you make ready and wait. -- **Don't re-implement `/github-review-pr`, `/github-review-failures`, or `/github-review-comments`** — invoke them. -- **One stuck PR must not block the rest** — mark it `needs-user`, continue the queue, return to it in the final report. -- **Read every auto-resolved CHANGELOG** before pushing — the perl fast-path is not a substitute for verifying the entry survived and reads correctly. diff --git a/.claude/commands/github-review-comments.md b/.claude/commands/github-review-comments.md deleted file mode 100644 index 090b81f..0000000 --- a/.claude/commands/github-review-comments.md +++ /dev/null @@ -1,237 +0,0 @@ ---- -description: "Use when a PR has unresolved review comments that need responses -- evaluates each comment, implements valid fixes, pushes back on incorrect suggestions, and resolves all threads." -model: sonnet -argument-hint: "PR number (e.g., 123 or #123)" -allowed-tools: Bash(gh pr view:*), Bash(gh pr diff:*), Bash(gh pr comment:*), Bash(gh api:*), Bash(git log:*), Bash(git blame:*), Bash(git push:*), Bash(git commit:*), Bash(git add:*), Bash(bundle exec:*), Read, Write, Edit, Glob, Grep, Agent ---- - -# Review GitHub PR Comments: $ARGUMENTS - -You are reviewing and responding to all unresolved review comments on a GitHub pull request. Apply technical rigour -- evaluate each comment against the actual codebase before accepting or rejecting it. - -## Phase 0: Determine the PR Number - -The user may provide a PR number as `$ARGUMENTS`. Parse it flexibly: - -- `PR123`, `PR 123`, `pr123` -> PR 123 -- `123` -> PR 123 -- `#123` -> PR 123 -- Empty/blank -> auto-detect from current branch - -**If no PR number is provided**, detect it automatically: - -```bash -gh pr list --author=@me --head="$(git branch --show-current)" --state=open --json number,title -``` - -If exactly one open PR exists for the current branch, use it. If none or multiple, ask the user. - -Once you have the PR number, confirm it: - -```bash -gh pr view --json title,state,url -``` - ---- - -## Phase 1: Fetch All Unresolved Review Comments - -Retrieve all review comments and identify unresolved ones: - -```bash -# Get all review comments (not resolved) -gh api "repos/mhenrixon/phlex-forms/pulls//comments" --paginate - -# Get all review threads to check resolution status -gh api graphql -f query=' - query($owner: String!, $repo: String!, $pr: Int!) { - repository(owner: $owner, name: $repo) { - pullRequest(number: $pr) { - reviewThreads(first: 100) { - nodes { - id - isResolved - path - line - comments(first: 20) { - nodes { - id - databaseId - body - author { login } - createdAt - } - } - } - } - } - } - } -' -f owner=mhenrixon -f repo=phlex-forms -F pr= -``` - -For each unresolved thread, extract: -- Thread ID (for resolving) -- Comment body (the review feedback) -- File path and line number (if inline) -- Author (to understand context) - -Filter to only **unresolved** threads. Skip bot comments (CodeRabbit, dependabot), resolved threads, and PR description comments. - -If there are no unresolved review comments, report that and stop. - ---- - -## Phase 2: Read and Categorise Each Comment - -For each unresolved comment, read the full body and categorise it: - -| Category | Action | -|----------|--------| -| Valid fix needed | Implement the fix | -| Valid test gap | Add the missing test | -| Valid style/consistency issue | Fix it | -| Incorrect suggestion | Push back with technical reasoning | -| Suggestion conflicts with architecture | Push back, reference existing patterns | -| Over-engineering / YAGNI | Push back, explain why it's unnecessary | -| Unclear | Ask for clarification (do NOT implement) | - -**Before categorising**, always: -1. Read the actual file and line being commented on -2. Check if the suggestion is technically correct for THIS codebase -3. Check if it would break existing functionality (a consuming docs site) -4. Check if existing patterns/conventions contradict the suggestion -5. Check CLAUDE.md rules -- project conventions override reviewer preferences - ---- - -## Phase 3: Implement Accepted Fixes - -For all comments you've decided to accept: - -1. **Make the code changes** -- edit the relevant files -2. **Run affected tests** to verify nothing breaks: - ```bash - bundle exec rspec - ``` -3. **Run validators**: - ```bash - bundle exec rubocop - ``` -4. **Commit** all fixes together with a clear message: - ```bash - git commit -m "$(cat <<'EOF' - fix: address PR review feedback - - - Description of fix 1 - - Description of fix 2 - EOF - )" - ``` -5. **Push** to the remote branch: - ```bash - git push - ``` - ---- - -## Phase 4: Reply to Every Comment - -For **each** unresolved thread, reply: - -### For accepted fixes: - -Reply with what was fixed and the commit SHA: - -```bash -gh api "repos/mhenrixon/phlex-forms/pulls//comments//replies" \ - --method POST \ - -f 'body=Fixed in . .' -``` - -### For rejected suggestions: - -Reply with technical reasoning: - -```bash -gh api "repos/mhenrixon/phlex-forms/pulls//comments//replies" \ - --method POST \ - -f 'body=' -``` - -### Resolving threads (via GraphQL): - -After replying, resolve the thread: - -```bash -gh api graphql -f query=' - mutation($threadId: ID!) { - resolveReviewThread(input: {threadId: $threadId}) { - thread { isResolved } - } - } -' -f threadId= -``` - -### For general PR comments (not inline review threads): - -Reply directly: - -```bash -gh pr comment --body "" -``` - ---- - -## Phase 5: Verify Completion - -After processing all comments, verify no unresolved threads remain: - -```bash -gh api graphql -f query=' - query($owner: String!, $repo: String!, $pr: Int!) { - repository(owner: $owner, name: $repo) { - pullRequest(number: $pr) { - reviewThreads(first: 100) { - totalCount - nodes { isResolved } - } - } - } - } -' -f owner=mhenrixon -f repo=phlex-forms -F pr= -``` - -Report the final tally: how many comments were accepted/fixed, how many were pushed back on, and confirm all threads are resolved. - ---- - -## Response Style - -When replying to comments: - -- **No performative agreement** -- never say "Great point!" or "You're absolutely right!" -- **No gratitude** -- never say "Thanks for catching that" -- **Be direct** -- state the fix or the reasoning, nothing more -- **Reference commits** -- always include the short SHA when a fix was made -- **Be specific** -- when pushing back, reference actual code, not abstract principles - -When pushing back: - -- Use technical reasoning grounded in the actual codebase -- Reference existing patterns if the suggestion contradicts them -- Reference CLAUDE.md rules when applicable -- Explain what would break or what edge case the reviewer missed -- If the suggestion is valid in principle but wrong for this context, say so - ---- - -## Important Notes - -- Always read the actual code before evaluating a comment -- reviewers sometimes misread diffs -- If a comment reveals a genuine bug you missed, fix it without defensiveness -- If multiple comments suggest the same change, implement it once and reference the fix in all replies -- Bot reviewers (CodeRabbit, etc.) sometimes suggest changes that conflict with project conventions -- verify against CLAUDE.md -- If a new round of review comments appears after your push (from re-review), report that to the user rather than entering an infinite loop - -Now begin by determining the PR number from `$ARGUMENTS` or the current branch. diff --git a/.claude/commands/github-review-failures.md b/.claude/commands/github-review-failures.md deleted file mode 100644 index 9737a9a..0000000 --- a/.claude/commands/github-review-failures.md +++ /dev/null @@ -1,178 +0,0 @@ ---- -description: "Use when CI checks are failing on a PR — fetches failure logs, diagnoses root causes, implements fixes, and pushes until CI is green." -model: sonnet -argument-hint: "PR number (e.g., 41 or #41)" -allowed-tools: Bash(gh pr view:*), Bash(gh pr checks:*), Bash(gh pr diff:*), Bash(gh api:*), Bash(gh run view:*), Bash(git log:*), Bash(git diff:*), Bash(git push:*), Bash(git commit:*), Bash(git add:*), Bash(bundle exec:*), Read, Write, Edit, Glob, Grep, Agent ---- - -# Fix GitHub CI Failures: $ARGUMENTS - -You are diagnosing and fixing CI failures on a GitHub pull request. Work systematically: identify failures, read logs, diagnose root causes, fix locally, verify, push. - -## Phase 0: Determine the PR Number - -The user may provide a PR number as `$ARGUMENTS`. Parse it flexibly: - -- `PR41`, `PR 41`, `pr41` -> PR 41 -- `41` -> PR 41 -- `#41` -> PR 41 -- Empty/blank -> auto-detect from current branch - -**If no PR number is provided**, detect it automatically: - -```bash -gh pr list --author=@me --head="$(git branch --show-current)" --state=open --json number,title -``` - -If exactly one open PR exists for the current branch, use it. If none or multiple, ask the user. - -Once you have the PR number, confirm it: - -```bash -gh pr view --json title,state,url,mergeable -``` - -**Pre-flight: merge conflicts (detection only).** If `mergeable` is `CONFLICTING`, STOP — do not diagnose CI on a conflicted branch (the merge itself may fix or cause the failures). Report the conflict and hand off to `/github-review-pr`, whose Phase A0 owns the resolution runbook — this command's toolset deliberately does not include the merge machinery. If `mergeable` is `UNKNOWN`, note it and proceed: the orchestrator resolves the ambiguity; a standalone run shouldn't block on GitHub's recompute. - ---- - -## Phase 1: Identify Failing Checks - -```bash -gh pr checks -``` - -Categorise each failing check: - -| Check Type | Examples | How to Get Logs | -|------------|----------|----------------| -| Lint (rubocop) + gem build | `Lint` | `gh run view --job= --log-failed` | -| Unit + component specs | `Ruby 3.x` | `gh run view --job= --log-failed` | -| Docs-site deploy (if triggered) | `Deploy docs` | `gh run view --job= --log-failed` | - -Extract the run ID and job IDs from the check URLs. The URL format is: -`https://github.com/mhenrixon/phlex-forms/actions/runs//job/` - -If all checks pass or are pending, report that and stop. - ---- - -## Phase 2: Fetch Failure Logs - -For each failing check, get the logs: - -```bash -# Get the failed job logs (condensed output) -gh run view --job= --log-failed -``` - -If `--log-failed` output is too large or unclear, try: - -```bash -# Full log for a specific job -gh run view --job= --log 2>&1 | tail -100 -``` - ---- - -## Phase 3: Diagnose Each Failure - -For each failure, determine the root cause: - -### Lint Failures - -Look for: -- RuboCop offenses: file path, line number, cop name, message - -**Key**: RuboCop failures can often be auto-fixed with `bundle exec rubocop -A `. - -### Spec Failures - -Look for: -- Test name and file path -- Error class and message -- Relevant backtrace lines (ignore framework noise) -- Whether it's a test environment issue vs actual code bug - -**Key patterns**: -- `NameError: uninitialized constant` -> missing require or renamed class -- `NoMethodError: undefined method` -> API change, missing method -- Component spec `expected HTML to include X` -> the render output changed, or a class the CSS relies on was dropped -- `expected: X, got: Y` -> logic bug or test needs updating - -### Build / Deploy Failures - -Look for: -- Gem build errors: missing files in gemspec, syntax errors -- Bundle install failures: dependency conflicts -- Deploy workflow: Docker build context, Kamal image/service name mismatch, missing `docs` environment secret - ---- - -## Phase 4: Fix Locally - -For each diagnosed failure: - -1. **Read the relevant file** to understand context before fixing -2. **Make the fix** -- edit the file -3. **Verify locally** before committing: - -```bash -# For rubocop failures -bundle exec rubocop - -# For spec failures -bundle exec rspec - -# For full validation -bundle exec rake -``` - -### Fix Priority Order - -1. **Lint/style fixes** first (fast, deterministic) -2. **Spec failures** second (may require understanding the code change) -3. **Build/deploy issues** third (usually gemspec, dependency, or a Kamal name mismatch) - ---- - -## Phase 5: Commit and Push - -```bash -git add -git commit -m "$(cat <<'EOF' -fix(ci): - -- Fix 1 description -- Fix 2 description -EOF -)" -git push -``` - ---- - -## Phase 6: Verify - -After pushing, check if CI has been re-triggered: - -```bash -gh pr checks -``` - -If there are still pending checks, report which checks are running and what was fixed. Do NOT poll in a loop -- report the status and let the user know. - -If you can identify that certain failures will persist for environmental reasons (e.g., a deploy job that needs a `docs` environment secret not present on a fork PR), flag that explicitly. - ---- - -## Important Notes - -- **Read before fixing** -- always read the actual failing code before attempting a fix -- **Fix the root cause** -- don't add `# rubocop:disable` to bypass lint; fix the actual issue (a targeted `# rubocop:disable` is acceptable only when RuboCop is demonstrably wrong) -- **Don't fix unrelated failures** -- if a spec was already failing on main, note it but don't fix it in this PR -- **Deploy vs. test failures** -- the `Deploy docs` workflow only fires on a release/dispatch; a normal PR usually runs lint + specs. A deploy failure is often a Kamal image/service name or missing-secret issue, not a code bug (see the README deploy section). -- **Flaky tests** -- if a test passes locally but fails in CI, note it as potentially flaky rather than adding workarounds. -- **Don't retry CI blindly** -- diagnose first, fix, then push. Each push triggers a full CI run. - -Now begin by determining the PR number and fetching the failing checks. diff --git a/.claude/commands/github-review-pr.md b/.claude/commands/github-review-pr.md deleted file mode 100644 index 4d6d65a..0000000 --- a/.claude/commands/github-review-pr.md +++ /dev/null @@ -1,163 +0,0 @@ ---- -description: "Use when a PR needs full review — resolves merge conflicts with the base first, then fixes CI failures, then addresses unresolved review comments. Conflicts first so CI diagnoses the post-merge reality; failures before comments because comment fixes trigger new CI runs that obscure the original failures." -model: opus -argument-hint: "PR number (e.g., 156 or #156)" -allowed-tools: Bash(gh pr list:*), Bash(gh pr view:*), Bash(gh pr checks:*), Bash(gh pr checkout:*), Bash(gh pr diff:*), Bash(gh pr comment:*), Bash(gh api:*), Bash(gh run view:*), Bash(git log:*), Bash(git blame:*), Bash(git diff:*), Bash(git status:*), Bash(git switch:*), Bash(git fetch:*), Bash(git merge:*), Bash(git merge-tree:*), Bash(git rev-parse:*), Bash(git push:*), Bash(git commit:*), Bash(git add:*), Bash(bundle exec:*), Bash(bundle install:*), Bash(bun install:*), Bash(cd:*), Read, Write, Edit, Glob, Grep, Agent ---- - -# Review GitHub PR (full pass): $ARGUMENTS - -You are running a full review pass on a pull request. The pass has three phases that MUST run in this order: - -1. **Phase A0: merge conflicts** — bring the branch up to date with its base and resolve any conflicts before anything else. -2. **Phase A: CI failures** — fix anything red before touching review comments. -3. **Phase B: review comments** — only after Phase A leaves CI green (or pending green after a push). - -## Why this order matters - -**Conflicts before failures**: CI results only matter for the code that will actually merge. On a conflicted (or stale) branch you'd diagnose failures against a base that no longer exists — and the conflict resolution itself changes code, invalidating the run you just fixed. Resolving conflicts first means Phase A reads CI for the post-merge reality, and you spend exactly one extra CI cycle instead of two. - -**Failures before comments**: if you fix review comments first, every commit pushes a new CI run. By the time the review-comment fixes finish, the original failure logs are buried under new pipeline runs. Symptoms: - -- The failing spec log you needed to read is now from a stale run; the latest run is still in progress on top of your unrelated comment fixes. -- A review-comment fix accidentally repairs the CI failure as a side effect, and you lose the chance to verify the failure was real. -- A review-comment fix accidentally INTRODUCES a CI failure, and you can't tell whether the new failure was pre-existing or your fault. - -Conflicts-first, then failures-first eliminates this confusion. CI is either green or red on a known commit against the current base; the review-comment fixes layer cleanly on top. - -## Phase 0: Determine the PR Number - -The user may provide a PR number as `$ARGUMENTS`. Parse it flexibly: - -- `PR156`, `PR 156`, `pr156` → PR 156 -- `156` → PR 156 -- `#156` → PR 156 -- Empty/blank → auto-detect from current branch - -**If no PR number is provided**, detect it automatically: - -```bash -gh pr list --author=@me --head="$(git branch --show-current)" --state=open --json number,title -``` - -If exactly one open PR exists for the current branch, use it. If none or multiple, ask the user. - -Once you have the PR number, confirm it: - -```bash -gh pr view --json title,state,url -``` - ---- - -## Phase A0: Merge conflicts - -Check whether the branch merges cleanly into its base: - -```bash -gh pr view --json mergeable,mergeStateStatus,baseRefName -``` - -| `mergeable` | Action | -|-------------|--------| -| `MERGEABLE` | Skip to Phase A. | -| `UNKNOWN` | GitHub is recomputing (common right after pushes, and it can stay UNKNOWN for minutes). Don't poll it — verify **locally**, against the PR's actual head (NOT `HEAD`, which may be some other checked-out branch): `git fetch origin ` and `git fetch origin pull//head`, verify both refs resolve (`git rev-parse --verify origin/^{commit}` and `git rev-parse --verify FETCH_HEAD^{commit}` — a bad ref also exits 1 from merge-tree, so exit code alone can't be trusted), then `git merge-tree --write-tree --name-only origin/ FETCH_HEAD`. Clean exit → no conflicts, skip to Phase A. Exit 1 **with conflict output** → resolve below (the `--name-only` file list is your work list). | -| `CONFLICTING` | Resolve, below. | - -### Resolution procedure - -1. Check out the PR's branch (`gh pr checkout `) with a clean tree (`git status`). Stash nothing — if the tree is dirty, stop and ask the user. -2. `git fetch origin ` then **`git merge origin/`** — MERGE, never rebase. The branch is shared (it has a PR); a rebase would require a force-push, which is forbidden on shared branches. -3. Resolve every conflicted file **semantically** — read both sides and produce the version that preserves BOTH changes' intent. Never blanket `--ours`/`--theirs` a source file. Repo-specific rules: - - **`CHANGELOG.md` (Unreleased)**: union — keep BOTH sides' entries (main's landed bullets and this branch's), most recent first, without duplicating the Keep-a-Changelog `### Added`/`### Fixed`/`### Changed` subheads. Losing either side is a real regression reviewers rarely catch. - - **`lib/phlex_forms/version.rb`**: releases land DIRECTLY on `main` via `rake release[X.Y.Z]` (the Rakefile aborts unless the current branch IS `main` — no release PRs exist), so an ordinary feature branch never edits this file — a conflict here means the BRANCH bumped it on purpose. Keep the branch's bump in that case; if the intent isn't obvious from the branch's own commits, stop and ask. Only take the base's version when the branch's edit was clearly accidental. - - **`docs/Gemfile.lock`** (the docs app's committed lockfile — the gem root's `Gemfile.lock` is gitignored and can never conflict): NEVER hand-merge lockfile hunks. The common conflict is the `phlex-forms (X.Y.Z)` path-gem pin, which `rake release` bumps directly on `main` — take the base's file (it carries the released pin). If the BRANCH deliberately changed docs dependencies, re-apply that intent by editing `docs/Gemfile` and running `cd docs && bundle install` — but note the Rakefile documents that a full re-resolve can fail on this lockfile's broad PLATFORMS list ("Could not find gems … valid for all resolution platforms"); if it fails, stop and ask rather than hand-editing the lock. - - **`docs/bun.lock`**: same principle — take the base's file; if the branch changed `docs/package.json`, re-run `cd docs && bun install` so the lock re-resolves on top. Never hand-merge. - - **Append-only registries** (`docs/app/models/doc.rb`): both sides usually appended — keep both `page` lines, in base order first. (`docs/config/routes.rb` is static docs-kit boilerplate here — pages register only in `doc.rb` and are served through the one `docs/:doc` route, so routes.rb shouldn't conflict per-page.) - - There are no tracked generated artifacts in this repo (the docs CSS builds under `docs/app/assets/builds/` are gitignored), so nothing needs a regenerate-instead-of-merge rule beyond the lockfiles above. -4. Run the verification gates BEFORE pushing the merge — scoped to what the conflict touched, at minimum: - ```bash - bundle exec rubocop lib spec # the CI Lint job's exact invocation - bundle exec rspec - # docs/ files involved (the docs app has its own .rubocop.yml — - # pass the changed files explicitly): - cd docs && bundle exec rubocop - ``` -5. Commit the merge (keep git's standard merge-commit message; add a body line naming any non-obvious resolution choice) and `git push` — a merge commit never needs force. - -### Phase A0 exit criteria - -- The PR reports `MERGEABLE` (or the local `git merge-tree` check is clean), AND the merge commit (if one was needed) is pushed. -- If the merge produced changes, CI is now re-running — that's expected; Phase A reads the fresh run. -- If a conflict cannot be resolved with confidence (both sides rewrote the same logic and the correct combination isn't decidable from the code), **stop and ask the user** — a guessed resolution that compiles is worse than a question. - ---- - -## Phase A: Run `/github-review-failures` - -Invoke the existing `/github-review-failures` slash command with the same `$ARGUMENTS` value. Its purpose: fix every failing CI check, push, leave the branch in a state where CI is either green or running-pending-toward-green. - -Follow that command's full process — phases 1–6 of the failures runbook. The slash command is at `.claude/commands/github-review-failures.md`. Its workflow: - -1. Identify failing checks via `gh pr checks `. -2. Fetch failure logs. -3. Diagnose root cause for each. -4. Fix locally — lint first (fast, deterministic), then specs, then build issues. -5. Verify locally before commit (`bundle exec rspec `, `bundle exec rubocop`). -6. Commit + push + report which checks are now running. - -### Phase A exit criteria - -Before moving to Phase B, one of these must be true: - -- All CI checks are green on the latest pushed commit. OR -- All CI checks are pending (running) on the latest pushed commit, AND no checks failed in the most recent completed run on this commit. OR -- A persistent CI failure exists that is **not caused by changes on this branch** (e.g., a flaky test on `main`, a deploy job that fails for environmental reasons). Report this explicitly and proceed to Phase B with the caveat noted. - -If failures persist on this branch's changes, **do NOT proceed to Phase B**. Report what's still failing, what's been tried, and ask the user how to proceed. - ---- - -## Phase B: Run `/github-review-comments` - -Once Phase A's exit criteria are met, invoke `/github-review-comments` with the same `$ARGUMENTS`. Its purpose: address every unresolved review thread on the PR, push fixes, reply with commit SHAs, and resolve the threads. - -The slash command is at `.claude/commands/github-review-comments.md`. Its workflow: - -1. Fetch all unresolved review threads via the GitHub GraphQL API. -2. Read and categorise each comment (valid fix / invalid suggestion / unclear). -3. Implement accepted fixes; verify locally (specs, validators, rubocop). -4. Commit all fixes together with a clear message; push. -5. Reply to every thread with the commit SHA (for accepted fixes) or technical reasoning (for rejections). -6. Resolve each thread via the GraphQL `resolveReviewThread` mutation. -7. Verify no unresolved threads remain. - -### Phase B exit criteria - -- All unresolved review threads have been replied to and resolved (or the user has explicitly approved leaving a specific thread open). -- The branch has been pushed with all accepted fixes. - ---- - -## Phase C: Final report - -Before reporting, re-check mergeability once more (`gh pr view --json mergeable`, or the local `git merge-tree` check if UNKNOWN) — the base can move underneath a long pass. If a NEW conflict appeared, loop back to Phase A0. - -After all phases complete, report: - -1. **Phase A0 summary**: whether the branch was conflicted, which files conflicted, how each was resolved (and the merge commit SHA) — or "clean merge, no action". -2. **Phase A summary**: which CI failures were diagnosed and fixed. Note the commit SHAs for the fixes. -3. **Phase B summary**: which review comments were accepted (with commit SHAs), which were pushed back on (with reasoning), and the final unresolved-thread count (should be 0). -4. **End state**: final mergeability + CI status on the latest commit. -5. **Outstanding work**: anything that still needs attention — e.g., CI was pending at the end of Phase B and the user should verify the latest run after the comment fixes. - ---- - -## Important Notes - -- **Do not interleave the phases.** Don't fix a CI failure, then a review comment, then another CI failure. The whole point of this command is the strict ordering. -- **A new CI failure emerging during Phase B** (e.g., a comment fix breaks a spec) means looping back to Phase A — fix the new failure before continuing comment work. Likewise, **a new conflict appearing mid-pass** (the base moved) means looping back to Phase A0. These loop-backs are the only allowed reverse directions. -- **If the PR is already merged**, there is nothing to review — report that and stop. (A stale `$ARGUMENTS` or a just-merged PR shows up as `state: MERGED` in Phase 0's confirm step.) -- **If the PR merges cleanly, has no failures AND no unresolved comments**, report "PR is clean" and stop. -- **If `$ARGUMENTS` is the same as the current open PR**, the two child slash commands will see the same PR. They share state through the git branch and the GitHub API, not through any in-process variable. -- **Don't re-implement the child slash commands' logic**. Invoke them and let them do their work. This command is the orchestrator. diff --git a/.claude/commands/lfg.md b/.claude/commands/lfg.md deleted file mode 100644 index c24d58d..0000000 --- a/.claude/commands/lfg.md +++ /dev/null @@ -1,284 +0,0 @@ ---- -description: "Executes full autonomous engineering workflow with verification. Use when implementing complete features, tackling GitHub issues, or running end-to-end development cycles." -model: opus -argument-hint: "GitHub issue number/URL or feature description" -allowed-tools: Bash(gh issue view:*), Bash(gh search:*), Bash(gh issue list:*), Bash(gh issue close:*), Bash(gh pr create:*), Bash(gh pr view:*), Bash(bundle exec:*), Bash(bun:*), Bash(git:*), Read, Write, Edit, Glob, Grep, Agent ---- - -# LFG - Full Autonomous Workflow - -Execute a complete engineering workflow with verification at each phase. - -## Phase 0: Branch Setup - -**BEFORE any other work, prepare the git branch:** - -1. Check the current branch: `git branch --show-current` -2. If NOT on `main`, switch: `git checkout main` -3. Pull latest: `git pull origin main` -4. Create feature branch: `git checkout -b issue-{number}-{brief-description}` (or `feature/{description}` if no issue number) - ---- - -## Phase 1: Understand - -### Step 1: Gather Requirements - -If `$ARGUMENTS` is a GitHub issue number or URL: - -```bash -gh issue view --json title,body,labels,assignees,comments -``` - -If `$ARGUMENTS` is a description, use it directly. - -### Step 2: Define Acceptance Criteria - -**MANDATORY:** Write explicit acceptance criteria: - -- **GIVEN** [context/setup] -- **WHEN** [action taken] -- **THEN** [expected outcome] - -You MUST NOT proceed until you can articulate these clearly. - -### Step 3: Comprehension Gate - -Before proceeding, you must: - -1. State the problem/feature in one sentence -2. Explain WHY this is needed (the user-facing payoff — a model-bound form that infers the right control and renders under any theme, written once) -3. List what changes from a host app's perspective (the `field` / builder / config API delta) -4. Identify edge cases not explicitly mentioned -5. Explain the flow: `PhlexForms.configure` → `Builder#field` asks `PhlexForms::Inference` for the control type → `PhlexForms::Theme` maps the role to a `Forms::*` leaf → the leaf delegates markup to the daisyui gem (or a `Forms::Plain::*` fallback) → the host gets model-bound HTML. Which link changes? - -If you cannot complete ALL five items, investigate further. - -### Step 4: Create Task List - -Create a TaskCreate todo list with specific implementation steps. - ---- - -## Phase 2: Explore - -1. Find related files (Glob/Grep or Explore agent) -2. Read existing patterns in similar leaf components (`lib/forms/*.rb`) -3. Understand integration points across the layers -4. Check existing test coverage in `spec/forms/**` (integration) and `spec/phlex_forms/**` (unit) -5. Review the builder surface (`lib/phlex_forms/builder.rb`) — the `field` verb + PascalCase escape hatches — and the form entry points (`lib/forms/form.rb`, `lib/forms/base.rb`) -6. Review the leaf components in `lib/forms/` (Input, Select, Textarea, Checkbox, Toggle, Radio, FileInput, …); each delegates markup to the daisyui gem via `PhlexForms::DelegatedField` -7. Review type inference (`lib/phlex_forms/inference.rb`) — the precedence order a new control-type rule must slot into -8. Review theming (`lib/phlex_forms/theme.rb` + `lib/forms/plain/`) — a new leaf role must be mapped in BOTH `Theme.daisy` and `Theme.plain` -9. Review the config surface (`lib/phlex_forms/configuration.rb`) — any globally-tunable behavior (theme, `infer_from_model`, `field_variants`, `icon_renderer`) goes here -10. Review the engine (`lib/phlex_forms/engine.rb`) — optional Rails wiring: importmap, i18n load path, the `Forms::Live` param type; and the live/reactive surface (`lib/forms/live.rb`, `app/javascript/phlex_forms/controllers/validations/`) - ---- - -## Phase 3: Plan - -1. List files to modify with specific changes -2. List new files to create with purpose -3. Identify the config default vs. per-call override (globally-tunable behavior goes on `Configuration`; per-form/per-class options go through the builder) -4. Plan test coverage across layers (TDD: tests FIRST) — config default, inference precedence, leaf render, theme parity -5. Update the task list -6. Consider backwards compatibility (hosts that never configure, and existing form classes, must keep working verbatim) -7. If the change adds a leaf, plan the theme mapping in BOTH `Theme.daisy` and `Theme.plain`; if it's live/reactive, plan the soft phlex-reactive guard and any `Engine` initializer - ---- - -## Phase 4: Implement (TDD) - -### The deviation log (keep it from the first edit) - -The plan is the map; the codebase is the territory. The moment reality forces a choice the plan or issue didn't settle, log it in `implementation-notes.md` at the repo root — one line, at the moment it happens, not reconstructed later: - -- **Deviations** — the plan said X, you did Y, because Z -- **Discoveries** — facts about the codebase the plan didn't know -- **Judgment calls** — choices the user might have made differently (defaults, naming, scope cuts) - -Pick the conservative option and keep going. The log is how the user audits your judgment afterwards. Never commit the file: its contents move into the PR body, then the file is deleted. - -For each logical unit: - -### 4.1: Write Failing Test First - -```bash -bundle exec rspec -``` - -### 4.2: Implement Minimum Code - -Write the MINIMUM code to make the test pass. Follow project patterns: - -| Never Do | Always Do | -|----------|-----------| -| Hand-write raw daisyUI markup in a leaf | Delegate to the daisyui gem via `PhlexForms::DelegatedField` | -| Hardcode a globally-tunable value in a component | Read it from `PhlexForms.config` | -| Add a daisy leaf and forget the plain one | Map the role in BOTH `Theme.daisy` and `Theme.plain` | -| Add an inference rule ahead of an explicit option | Respect precedence: caller-passed `as:`/options always win | -| Hard-require phlex-reactive | Guard `Forms::Live` behind `defined?(Phlex::Reactive)` | -| Break the leaf initializer signature | Keep `(*modifiers, name:, id:, value:, error:, required:, ...)` — it's the theme seam | -| Fabricate a model to test against | Build a real ActiveModel double with `build_model` (column types/enums/validators drive inference) | - -### 4.3: Refactor - -Once green, refactor while keeping tests passing. - -### 4.4: Validate - -```bash -bundle exec rubocop lib spec -``` - -### 4.5: Repeat - -Move to the next unit. Mark task items complete. - ---- - -## Phase 5: Deep Root Cause Analysis (Bug Fixes Only) - -**If this is a bug fix, investigate before implementing.** - -### Trace the lifecycle - -For the failing behavior: -- Did it originate in inference (wrong control type), in a leaf's render (wrong markup/variants), in the theme map (missing role, daisy/plain drift), or in config (a default nothing reads)? -- Is it a server-render bug or a client-enhancement bug (does it reproduce with the Stimulus validation / `Forms::Live` disabled)? -- What ASSUMPTIONS does the code make at the failure point? Which was violated, and WHY? - -### Use git history - -```bash -git log --oneline -20 -git blame -``` - -### Map all callers - -Use Grep to find every call site. Does the bug happen only for a certain model -shape (a boolean column, an enum, a `belongs_to`)? Only under the plain theme? -Only when a host overrides a default (`infer_from_model = false`, custom -`field_variants`)? - -### Five Whys - -Keep asking WHY until you reach the real fix point. - -### Fix-location principle - -The best fix is usually NOT where the error surfaced: -- Wrong control inferred → the precedence order in `PhlexForms::Inference`, not a special case in the leaf -- Plain theme renders wrong → the `Theme.plain` mapping or the `Forms::Plain::*` leaf, not a branch in the daisy leaf -- Missing `*-error` class → `DelegatedField#daisy_modifiers`, not an inline class hack -- Variant not applied → the `field_variants` stacking order (global → form → call-site), not a per-call override - -### Unacceptable superficial fixes — DO NOT DO THESE - -- `rescue nil` / bare `rescue` to silence an error you don't understand -- `&.` to paper over a nil without finding why it's nil -- `return if x.nil?` to silently skip -- swallowing errors instead of logging + fixing the cause - -**These HIDE bugs. Find the EARLIEST point you could prevent the error and fix there.** - ---- - -## Phase 6: Verify - -**ALL of these must pass before committing:** - -```bash -bundle exec rubocop lib spec -bundle exec rspec -``` - -### Solution verification - -- "If I were the requester, is this fully resolved?" -- "Did I fix the ROOT CAUSE, not the symptom?" -- "Do the tests prove it?" -- "Does every existing host app and form class still work verbatim (backwards compatible)?" -- "If the change needs setup, is it documented in the README (and wired into the `PhlexForms::Engine` initializer if it's a live/reactive concern)?" - ---- - -## Phase 7: Commit & PR - -### Commit - -```bash -git add -git commit -m "$(cat <<'EOF' -feat(scope): brief description - -## Summary -[What changed and why] - -## Test Coverage -- spec 1: validates X -- spec 2: validates the config-driven default - -## Verification -- [x] bundle exec rubocop lib spec passes -- [x] bundle exec rspec passes -EOF -)" -``` - -### Push & PR - -```bash -git push -u origin $(git branch --show-current) - -gh pr create --title "feat(scope): brief description" --body-file /tmp/pr-body.md -``` - -Write the PR body to a temp file (`--body-file`) to avoid shell-interpolation of -backticks/tables. The body is copied verbatim — if you would not type a -backslash in a GitHub comment, do not type one in the heredoc. - -**The PR body MUST contain a GitHub closing keyword for every issue it -resolves** — `Closes #12`, `Fixes #13`, `Resolves #9` (one per issue; only these -keywords auto-close, and `Refs #12` does NOT). Put them in the Summary so the -issues close automatically when the PR merges to the default branch. This works -with squash-merge; a keyword only in a commit body can be lost when commits are -squashed, so the PR body is the reliable place. For a multi-issue PR, list every -one: `Closes #9, closes #10, closes #11` (repeat the keyword — `Closes #9, #10` -only closes #9). - -The PR body MUST also end with a `## Deviations & judgment calls` section copied -from `implementation-notes.md` (then delete the file). If the plan held -completely, write "None — the plan held." This section is read FIRST in review — -it is the audit trail for every decision the plan didn't make. - -If a PR ever merges without the keyword (issues stay open), close them manually -with `gh issue close --reason completed --comment "Fixed in # (merged)."`. - ---- - -## Phase 8: Comprehension Close-Out - -The tests prove the CODE is right; this phase keeps the USER's mental model right. After the PR is up, end your final message with: - -1. **The decisions, not the diff** — the 3–5 non-obvious choices in this change someone must understand to maintain it. Lead with anything from the deviation log; the user has never seen those. -2. **Three merge-gate questions** the user should be able to answer before merging. If any answer isn't obvious to them, offer a walkthrough — an unanswerable question is comprehension debt, and merging anyway is how it compounds. - ---- - -## Verification Checklist - -- [ ] All acceptance criteria met -- [ ] Tests written BEFORE implementation -- [ ] `bundle exec rubocop lib spec` passes -- [ ] `bundle exec rspec` passes -- [ ] Backwards compatible — existing host apps and form classes unchanged -- [ ] New leaf mapped in BOTH `Theme.daisy` and `Theme.plain`; live/reactive features guard the soft phlex-reactive dependency -- [ ] Required setup documented in the README (and wired into the `PhlexForms::Engine` initializer if it's a live/reactive concern) -- [ ] PR created with summary + test plan -- [ ] PR body has a closing keyword (`Closes #N` / `Fixes #N`) for EVERY resolved issue — one per issue, not `Refs` -- [ ] PR body ends with `## Deviations & judgment calls` (from implementation-notes.md, since deleted) -- [ ] Comprehension close-out delivered (decisions + three merge-gate questions) - -Now, execute this workflow for the provided issue or feature. diff --git a/.claude/commands/plan.md b/.claude/commands/plan.md deleted file mode 100644 index 62fd9f3..0000000 --- a/.claude/commands/plan.md +++ /dev/null @@ -1,92 +0,0 @@ ---- -description: "Investigates the codebase, designs a solution, and produces a durable plan artifact — a GitHub issue or a plan markdown under docs/plans/. Read-only: never edits application code. Use before /lfg for anything non-trivial." -model: fable -argument-hint: "issue | md | " -allowed-tools: Bash(gh issue create:*), Bash(gh issue list:*), Bash(gh issue view:*), Bash(gh search:*), Bash(gh label list:*), Bash(git log:*), Bash(git diff:*), Bash(git branch:*), Bash(date:*), Read, Grep, Glob, Write, Agent, AskUserQuestion ---- - -# Plan — design expensive, execute cheap - -You are the planning specialist. This command runs on the most capable model deliberately: the thinking happens here, the execution happens later on cheaper models (`/lfg` on Opus, layer specialists on Sonnet). That split only works if the plan is **self-contained** — an executor with none of this session's context must be able to implement it without guessing. - -## Output mode from $ARGUMENTS - -| $ARGUMENTS starts with | Artifact | -|------------------------|----------| -| `issue` | GitHub issue (default — feeds directly into `/lfg `) | -| `md` or `file` | Markdown file at `docs/plans/YYYY-MM-DD-.md` (date from `date +%F`) | -| anything else | GitHub issue | - -## Hard constraints - -- **Read-only for source code.** Never edit application code, never commit, never create branches. The only file you may Write is a new plan markdown under `docs/plans/`. -- **Never reproduce secrets** (keys, tokens, credentials) in the plan, even redacted ones you encounter while reading config. -- **Dedupe before creating an issue**: `gh issue list --search ""` — if an existing issue covers this, extend it in your summary instead of duplicating. - -## Phase 1 — Investigate - -Protect this session's context: delegate mechanical exploration to cheaper subagents and keep Fable for judgment. - -1. Fan out Explore agents (`model: haiku`) for file discovery and naming-convention sweeps; use `model: sonnet` agents when a subsystem needs to be read and summarized. Launch independent explorations in parallel. -2. Read the load-bearing files yourself — the ones the design decision actually hinges on. Don't design from subagent summaries alone. -3. Check the architecture layers in `CLAUDE.md` and read the matching source files — past decisions and gotchas live there. -4. Check `git log` for recent related work; the design should extend it, not fight it. - -## Phase 2 — Surface the unknowns (blindspot pass + interview) - -Investigation tells you what the codebase says; this phase finds what the REQUEST doesn't say. Run it BEFORE designing — a wrong assumption caught here costs one question; caught in review it costs a rewrite. - -1. **Blindspot pass.** Write down the unknowns you are carrying into the design: - - decisions the request leaves open (defaults, naming, public API/config surface, rollout & upgrade story) - - edge cases the codebase makes possible that the request never mentions - - anything with no precedent in this repo — flag it explicitly as unknown-unknown territory -2. **Interview the user** with AskUserQuestion, one question at a time, prioritized by blast radius: architecture-changing answers first, then public API / config surface, then UX. Rules: - - Skip anything the codebase, CLAUDE.md, or an existing issue already answers. - - 2–5 questions is the sweet spot; zero is fine when the request is genuinely unambiguous — say so rather than inventing questions. - - Every question offers concrete options with a recommended default, never an open-ended essay prompt. -3. **Record the answers** in the plan's Decision section as `Settled in interview:` bullets — constraints the executor must not re-litigate. - -## Phase 3 — Design - -- Develop 2-3 candidate approaches with real tradeoffs. Pick one and say why; record why the others lost. -- The chosen design must respect project invariants: model introspection always behind `respond_to?` guards (degrade gracefully for POROs — no hard ActiveRecord dependency); `daisyui` and `phlex-reactive` are **soft** dependencies (require-rescue-`LoadError` + Zeitwerk `ignore`; the gem must render the Plain theme without them installed); leaf components resolve their classes through `PhlexForms::Theme` and need daisy + Plain parity; literal Tailwind/daisy class strings only (no interpolation); explicit `as:`/`choices:`/caller kwargs always win over inference; TDD (specs named before implementation steps); never `raw`/`html_safe` on user- or model-supplied data. -- Decide the test strategy per the testing rules: unit specs for `PhlexForms::` config/inference/theme, component specs asserting rendered HTML for both the daisy and Plain themes, integration specs for the `Forms::` builder end-to-end. - -## Phase 4 — Emit the plan artifact - -Use this structure for the issue body or markdown file. Every section is load-bearing — an executor uses Context to avoid re-discovery, Steps to act, Gates to verify, Boundaries to stop. - -```markdown -# - -## Problem / Goal -<What's wrong or missing, who it affects, what done looks like.> - -## Context (read these first) -<Bullet list: `path/to/file.rb` — why it matters to this change. Include the `Forms::` builder/component layer, the `PhlexForms::` configuration/inference/theme layer, and the theme registry as relevant. Self-contained: no references to "as discussed" or this session.> - -## Decision -<Chosen approach and rationale. Then: alternatives considered and why each was rejected. End with `Settled in interview:` bullets for every constraint the user confirmed in the interview phase — the executor must not re-litigate these.> - -## Implementation steps -<Ordered, small, each mapped to the appropriate architecture layer. Specs come before the code they cover. Name exact files to create or change.> - -## Verification gates -<Exact commands + expected outcome:> -- `bundle exec rspec <paths>` — all green -- `bundle exec rubocop` — no offenses - -## Out of scope -<Explicit boundaries — the adjacent things an eager executor must NOT do.> - -## Execution -Execute with `/lfg <issue-number>` (or `/lfg docs/plans/<file>.md`). -``` - -For GitHub issues: create with `gh issue create --title "..." --body-file <tmpfile>`. Write the body to a temp file first; do not use inline heredoc with `gh pr create --body` (code fences get mangled by shell interpolation). - -For markdown files: Write to `docs/plans/YYYY-MM-DD-<slug>.md`. Leave it uncommitted — committing is the user's call. - -## Phase 5 — Handoff - -Report back: link to the issue (or file path), the chosen approach in 2-3 sentences, and the exact execute command. Stop there — do not start implementing. diff --git a/.claude/commands/tdd.md b/.claude/commands/tdd.md deleted file mode 100644 index e1d7143..0000000 --- a/.claude/commands/tdd.md +++ /dev/null @@ -1,126 +0,0 @@ ---- -description: "Use when implementing any feature or fixing any bug — enforces RED-GREEN-REFACTOR: write failing test first, implement minimum code to pass, then refactor." -model: sonnet ---- - -# TDD Command - -Enforce test-driven development with RED → GREEN → REFACTOR. - -## The TDD Cycle - -```text -RED: Write a failing test (it MUST fail first) -GREEN: Write MINIMAL code to pass (nothing more) -REFACTOR: Improve code while keeping tests green -REPEAT: Next scenario -``` - -## When to Use - -- Implementing a new `Forms::` leaf component or extending an existing one -- Adding a config knob to `PhlexForms::Configuration` -- Changing type inference (`PhlexForms::Inference`) -- Adding or remapping a theme role (`PhlexForms::Theme`) -- Fixing a bug (write the reproducing test FIRST) - -## Workflow - -### Step 1: Write Failing Tests (RED) - -Pick the cheapest layer that proves the behavior. Unit specs live in -`spec/phlex_forms/**` (pure logic); integration specs live in `spec/forms/**` -and render a real form through the `render_form` / `FormContext` kit helper. -Model doubles come from the `build_model` ActiveModel helper -(`spec/support/model_helpers.rb`). - -```ruby -# Config (pure, no Rails): a new configuration knob + its default -RSpec.describe PhlexForms::Configuration do - it "defaults infer_from_model to true" do - expect(described_class.new.infer_from_model).to be(true) - end -end - -# Inference (pure precedence logic — the highest-value unit target): -RSpec.describe PhlexForms::Inference do - it "maps a boolean column to a toggle" do - model = build_model(:user, notify: false) - result = described_class.resolve(model:, name: :notify) - expect(result.as).to eq(:toggle) - end -end - -# Theme: role resolution + `.with` overrides -RSpec.describe PhlexForms::Theme do - it "resolves :plain to the unstyled input leaf" do - expect(described_class.resolve(:plain)[:input]).to eq(Forms::Plain::Input) - end -end - -# Integration: the rendered markup for a model-bound field -RSpec.describe Forms::Field do - it "renders type=email for an email attribute" do - model = build_model(:user, email: "a@b.c") - html = render_form(model) { |f| f.field :email } - expect(html).to include('type="email"') - end -end -``` - -### Step 2: Run — Verify FAIL - -```bash -bundle exec rspec <spec_file> -# FAIL — confirms the test runs, tests the right thing, and the code doesn't already exist -``` - -### Step 3: Implement Minimal Code (GREEN) - -### Step 4: Run — Verify PASS - -```bash -bundle exec rspec <spec_file> -# N examples, 0 failures -``` - -### Step 5: Refactor - -Improve while staying green: extract methods, improve names, reduce duplication. - -### Step 6: Run Full Suite + Lint - -```bash -bundle exec rspec -bundle exec rubocop lib spec -``` - -## Coverage Expectations - -| Code | Minimum | -|------|---------| -| All code | 80% | -| `PhlexForms::Inference` (every precedence branch + the `infer_from_model` kill-switch) | ~100% | -| `PhlexForms::Configuration` (every knob + its default + override) | 100% | -| `PhlexForms::Theme` (role resolution, `.with` overrides, daisy/plain parity) | 100% | - -## Best Practices - -**DO:** test FIRST; verify RED; minimal GREEN; refactor green; assert on -component **semantics** (an inferred control type, a `required` flag, an -error-variant class, an inferred choices list) via a real `render_form`; build -model doubles with `build_model` so column types, enums, and validators drive -inference exactly as they would in a host app. - -**DON'T:** implement before testing; assert brittle full-HTML snapshots; test -implementation details; hardcode a value a leaf should read from config; assert -against a plain object when the behavior depends on a model shape (use -`build_model` with the right attributes/validations). - -## Checklist - -- [ ] Tests written BEFORE implementation; RED verified -- [ ] Minimal GREEN; refactored green -- [ ] Coverage meets the bar (~100% on inference, config, theme) -- [ ] Edge cases covered (POROs falling through to the name map, `infer_from_model` off, plain-theme render) -- [ ] `bundle exec rubocop lib spec` passes diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000..90b8653 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,13 @@ +{ + "extraKnownMarketplaces": { + "zoolutions": { + "source": { + "source": "github", + "repo": "zoolutions/claude-plugins" + } + } + }, + "enabledPlugins": { + "lode@zoolutions": true + } +} diff --git a/.gitignore b/.gitignore index caf66eb..ab856fa 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,6 @@ Gemfile.lock # rspec failure tracking .rspec_status *.gem + +# lode scratch space (evidence, gate reports) — never committed +/lode/tmp/ diff --git a/AGENTS.md b/AGENTS.md index 8ea8159..acbaa31 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,6 +12,13 @@ with a Plain (unstyled) theme fallback, and offers optional server-truth live validation over phlex-reactive. The gem also **dogfoods a docs site** under `docs/` (built on docs-kit). +## Memory + +Durable project memory lives in `lode/` (index: `lode/lode-map.md`). Read it +before exploring the code. `lode/review/` holds accepted review findings as +rules about the system; `/lode:gate` enforces them before any push and +`/lode:learn` adds to them. + ## The two things you'll be asked to do ### A. Change the gem (a component, inference rule, theme, config, the live layer) diff --git a/CLAUDE.md b/CLAUDE.md index d3824e9..accdc77 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -7,6 +7,15 @@ type and the `required` flag from the model. DaisyUI-styled by default (via the theme fallback, plus optional server-truth live validation over [phlex-reactive](https://github.com/zoolutions/phlex-reactive). +## Memory + +Durable project memory lives in `lode/` (index: `lode/lode-map.md`). Read it +before exploring the code. `lode/review/` holds accepted review findings as +rules about the system; `/lode:gate` enforces them before any push and +`/lode:learn` adds to them. `lode/workflow.md` is the repo profile the shared +`/lode:` workflow commands read. The lode describes the system as it is; +`CHANGELOG.md` records what changed. + ## Tech Stack - **Ruby**: >= 3.4 (aligns with the optional phlex-reactive live integration) @@ -50,18 +59,24 @@ The docs site under `docs/` has its own bundle (Ruby 4.0.5): `cd docs && bin/dev ## Slash Commands +The `/lode:` commands come from the `lode@zoolutions` plugin and read +`lode/workflow.md` for everything repo-specific. The three unprefixed ones are +local to this repo (`.claude/commands/`). + | Command | Purpose | |---------|---------| -| `/plan` | Fable-powered planning → GitHub issue or `docs/plans/` markdown (read-only; execute with `/lfg`) | -| `/lfg` | Full autonomous workflow: branch → understand → explore → plan → TDD → verify → PR | -| `/tdd` | Enforce RED → GREEN → REFACTOR | +| `/lode:plan` | Read-only planning → GitHub issue or `docs/plans/` markdown (execute with `/lode:lfg`) | +| `/lode:lfg` | Full autonomous workflow: branch → understand → explore → plan → TDD → verify → PR | +| `/lode:tdd` | Enforce RED → GREEN → REFACTOR | | `/architect` | Coordinate a change across the builder → components → inference → theme → live layers | | `/security` | Security audit (HTML escaping, model-bound params, the live action whitelist, CSRF) | | `/review-pr` | Review a PR for pattern compliance | -| `/github-review-pr` | Full PR pass: fix CI failures, then resolve review comments (in that order) | -| `/github-review-failures` | Fix failing CI checks until green | -| `/github-review-comments` | Process unresolved PR review comments | -| `/finish-prs` | Drive a stack of open PRs to merge-ready one at a time | +| `/lode:review-pr` | Full PR pass: resolve conflicts, fix CI failures, then process review comments | +| `/lode:finish-prs` | Drive a stack of open PRs to merge-ready one at a time | +| `/lode:debug-flaky` | Root-cause an intermittent spec — evidence → repro → stress-proofed fix | +| `/lode:gate` | Pre-PR gate: fresh-context review against the rules and `lode/review/`; the push hook requires it | +| `/lode:learn` | Write accepted review findings into `lode/review/` | +| `/lode:sync` | Keep `lode/` true to the code after a change | ## Architecture @@ -93,9 +108,12 @@ Commands and agents pin a model **tier** via frontmatter aliases, not a full model ID — aliases track the latest model in each tier, so pins never go stale: - `haiku` — mechanical/config work, diff pattern-scans -- `sonnet` — layer specialists / pattern-following implementation (the default for `/tdd`, the review-comment/failure runbooks) -- `opus` — orchestration, security, production/PR review (`/lfg`, `/architect`, `/security`, `/review-pr`, `/github-review-pr`) -- `fable` — pinned only on `/plan` (read-only planning that hands execution to cheaper models); otherwise choose it per-session with `/model` for architecture and the hardest debugging +- `sonnet` — layer specialists / pattern-following implementation +- `opus` — orchestration, security, PR review (`/architect`, `/security`, `/review-pr`) +- `fable` — read-only planning that hands execution to cheaper models; choose it per-session with `/model` for architecture and the hardest debugging + +The `/lode:` commands pin their own tiers in the plugin; these three apply to the +local commands under `.claude/commands/`. When spawning subagents for mechanical work (file finding, pattern scans), pass a cheaper model explicitly (`model: haiku`) rather than letting them inherit the @@ -118,7 +136,8 @@ session model. See `.claude/rules/agents.md`. ## More Documentation -- `.claude/commands/` — slash command definitions +- `lode/` — durable project memory (start at `lode/lode-map.md`) +- `.claude/commands/` — the repo's own slash commands (`/architect`, `/security`, `/review-pr`) - `.claude/rules/` — coding style, git workflow, testing, agents - `README.md` — the full field API / inference / theming / live-validation guide - `docs/` — the published documentation site (docs-kit) diff --git a/lode/client-validation/summary.md b/lode/client-validation/summary.md new file mode 100644 index 0000000..4961fa9 --- /dev/null +++ b/lode/client-validation/summary.md @@ -0,0 +1,139 @@ +# Client-side validation: mirroring the model's validators + +Turned on with `Form(model:, validate: true)` (or `form_options validate: true`). +It is a **mirror**, not a replacement: the server's validators stay +authoritative, and the client only shortens the loop for the cases a browser can +reproduce. + +## Server half + +`Forms::Validations::Introspector` (`lib/forms/validations/introspector.rb`, +228 lines) reads a model class's validators and returns a `data:` hash per +attribute. `Introspector.for(model_or_class)` (`introspector.rb:51-58`) returns +`Introspector::Null` — whose `data_attributes_for` returns `{}` — for `nil` or +for anything that does not respond to `validators_on`, so callers never guard. +`Form#validations_introspector` (`form.rb:54-61`) memoises the real one when +`@validate` is set and the Null otherwise. + +`SUPPORTED` maps **8** short validator class names to controller suffixes: +`PresenceValidator`, `LengthValidator`, `FormatValidator`, +`NumericalityValidator`, `InclusionValidator`, `ExclusionValidator`, +`ConfirmationValidator`, `AcceptanceValidator`. Matching on the **short** name +(`validator.class.name.split("::").last`) picks up both +`ActiveModel::Validations::*` and the parallel `ActiveRecord::Validations::*` +subclasses without listing both namespaces. A validator whose `:if`, `:unless` +or `:on` option is `present?` is rejected outright (`applicable_validators`, +`introspector.rb:110-114`) — a `.present?` test, where `Inference` and +`Forms::Field` use `options.key?` (`../inference/summary.md`). + +### The attribute encoding + +`data_attributes_for` (`introspector.rb:74-94`) returns +`{ controller: "validations--presence validations--length", … }` plus one key +per rule value. The value keys are built by `data_key` +(`introspector.rb:105-108`) as `:"validations__<suffix>_<key>_value"`: Phlex +rewrites `_` to `-` in a `data:` key, so `__` is how a literal `-` reaches the +HTML and `data-validations--length-maximum-value` comes out the other side. + +`CONTROLLER_PREFIX = "validations"` is the single source of that identifier. It +matches the shipped controller path +(`app/javascript/phlex_forms/controllers/validations/<suffix>_controller.js`), +so `lazyLoadControllersFrom("phlex_forms/controllers")` resolves +`validations--length` to the right file. `Form#apply_validation_coordinator` +derives the form-level identifier from the same constant — +`"#{CONTROLLER_PREFIX}--form"` (`form.rb:211-224`) — so field and form +identifiers cannot drift. + +Per-validator value builders: `presence` → `required: "true"`; `length` → +`maximum`/`minimum`/`is`, with `in:`/`within:` expanded to `minimum`+`maximum`; +`format` → the regex translated for JS (`\A`/`\z`/`\Z` → `^`/`$`, a +`(?-mix:…)` wrapper stripped) plus `i`/`m` flags; `numericality` → the 9 +`NUMERICALITY_KEYS`; `inclusion`/`exclusion` → the list as JSON; +`confirmation` → `match: "<attr>_confirmation"`; `acceptance` → the accepted +values as JSON, defaulting to `%w[1 true]`. `allow_blank` rides along on +`length`, `format`, `numericality`, `inclusion` and `exclusion`; `allow_nil` +only on `length` and `numericality`. + +### Inline rules without a model + +`Forms::Validations::ManualRules` (`lib/forms/validations/manual_rules.rb`, 77 +lines) takes `{ length: {...}, presence: true, … }`, builds a throwaway +`ActiveModel` class whose validators reproduce the rules, and delegates to the +Introspector — so the per-validator → data-attribute conversion exists in one +place. Reached through `f.field(:x, validate: { length: { maximum: 60 } })`. + +That ad-hoc class always names its attribute `:value` +(`manual_rules.rb:34, 47`), which is invisible for every rule except +`confirmation:` — it emits `match: "value_confirmation"` whatever the real field +is called, so inline confirmation rules only work on a field actually named +`value`. + +### The per-field merge seam + +`Forms::Field#apply_validations` (`field.rb:225-232`) is the seam that carries +both the Stimulus data and (under `live`) the blur trigger. `field` routes +through it (`builder.rb:81`), and so do six escape hatches — `Input`, `Select`, +`Textarea`, `Checkbox`, `Toggle`, `FileInput`. The other four — `Radio`, +`Hidden`/`hidden_field`, `Label`, `Control` — call the leaf directly and get no +client validation and no live trigger. + +`validate: false` opts a field out, `validate: true` uses the form-level +introspector, a Hash means inline rules, and anything else yields `{}` +(`validation_data_for`, `field.rb:250-256`). Merging is key-aware: +`TOKEN_JOINED_DATA_KEYS` (`:controller`, `:action`) are **joined** with a space +rather than replaced (`merge_data`, `field.rb:266-276`), which is what lets a +validation controller, a live trigger and a caller-supplied controller coexist +on one element. + +## Form-level wiring + +`apply_validation_coordinator` (`form.rb:211-224`) adds three things to the +`<form>`: the `validations--form` controller, the `submit->validations--form#onSubmit` +action (without it the controller connects but never intercepts), and +`novalidate` — the Stimulus layer owns error display, so the native browser UI +is turned off. Both the controller list and the action list are appended to +whatever the caller passed. + +## Client half + +10 files under `app/javascript/phlex_forms/controllers/validations/`: the 8 +field validators, the shared `base_controller.js`, and `form_controller.js`. + +`FieldValidatorController` (`base_controller.js`, 166 lines) attaches to the +input itself, listens for `blur` and for the synthetic +`invalidate:validations` event, and asks the subclass's `check(value)`. +Details that matter: + +- Errors are stored per validator on the element + (`element.__formsValidationErrors[identifier]`), so one validator reporting + valid cannot erase another's message. The element shows the first message in + sorted-identifier order, not all of them — the order is sorted purely so the + displayed text does not flicker between renders. +- The error slot is the Stimulus `error` target when the page pre-rendered one; + otherwise the controller reuses, or creates, a + `<p data-validations--error="<id>">` adjacent to the input. The attribute is + set with `setAttribute` because `dataset` rejects keys containing `--`. +- The error class is chosen per tag — `textarea-error`, `select-error`, else + `input-error` — rather than adding all three. + +`form_controller.js` (43 lines) intercepts `submit`, dispatches +`invalidate:validations` to every **non-disabled** `input`/`textarea`/`select` +whose `data-controller` names a `validations--*` other than the coordinator, +collects errors into `event.detail.errors`, and on any error cancels the submit +and focuses plus scrolls to the first one. The coordinator knows nothing about +which validators exist — a new validator is a new field controller and nothing +here. + +Messages come from `app/javascript/phlex_forms/i18n.js` (40 lines), which reads +the locale off `<html lang>`, falls back to a `<meta name="locale">` then `en`, +and lets a host override any string via `window.PhlexForms.messages`. The +bundled table (`messages.js`, 100 lines) ships **en, fr and af** — a different +set from the gem's Ruby locales (`en`, `sv`, `de` under `config/locales/`), so +a Swedish or German host gets translated submit buttons and English client-side +validation messages. + +## Related + +- `../live-validation/summary.md` — the server-truth alternative, which merges + through the same `apply_validations` seam +- `../inference/summary.md` — the other reader of the same validator objects diff --git a/lode/docs-site/summary.md b/lode/docs-site/summary.md new file mode 100644 index 0000000..a205cf5 --- /dev/null +++ b/lode/docs-site/summary.md @@ -0,0 +1,102 @@ +# The docs site (`docs/`) + +A self-contained [docs-kit](https://github.com/zoolutions/docs-kit) Rails app +that documents the gem and **dogfoods it**: `docs/Gemfile:14` path-depends on +`..` and fully requires it, so pages render live examples of the real +components (the interactive tag field on `/docs/tag-fields`). It has its own +bundle, its own Ruby (`docs/.ruby-version`: 4.0.5, against the gem's floor of +3.4), its own `.rubocop.yml`, and its own committed `Gemfile.lock` and +`bun.lock`. Deployed to <https://phlex-forms.zoolutions.llc>. + +## Page → behaviour map + +Pages are registered in `docs/app/models/doc.rb` — one `page "Title", group:` +line each, 16 lines, 16 page classes under `docs/app/views/docs/pages/`. A page +with no registry line is **not routed and not in the nav**; a registry line +whose view class does not resolve is silently skipped everywhere. `rails g +docs_kit:page "Title" --group=…` writes both halves. + +| When you change… | Update | +|---|---| +| the `field` verb, its options, an `as:` kind | `field_api.rb`, `quick_start.rb` | +| `PhlexForms::Inference` (precedence, a new mapping) | `inference.rb` | +| `Forms::Base`, `form_options`, the `live` macro's class side | `form_classes.rb` | +| a theme role, a Plain twin, `Theme#with` | `theming.rb` | +| `ClassMerge`, `field_variants`, a daisy modifier | `variants.rb` | +| `row`/`group` | `layout.rb` | +| the tag field, `live_tags` | `tag_fields.rb` | +| `Forms::Live` behaviour | `live_validation.rb` | +| the Introspector, a Stimulus validator | `client_validation.rb` | +| `fields_for`, `collection_check_boxes`, `collection_select`, `checkbox_group` | `nested_collections.rb` | +| an escape hatch (`Input`, `Control`, …) | `escape_hatches.rb` | +| `PhlexForms::Configuration` | `configuration.rb` | +| either cop | `rubocop_cops.rb` | +| installation, engine wiring, importmap pins | `installation.rb`, `overview.rb` | + +The README (461 lines) covers the same surface and drifts independently — a +user-facing change updates **both**. + +## Authoring contract + +`docs/AGENTS.md` carries the docs-kit block, and `docs/.claude/skills/ +write-docs-page/SKILL.md` is the Claude Code skill for it. The rules that bite: + +- `DocsUI::Section` owns structure and the "On this page" TOC. A Markdown `##` + is never page structure — only a sub-heading inside a Section. +- Prose is `md <<~'MD'` — **single-quoted** heredoc, so `#{…}` stays literal + (Phlex escapes author text; never `html_safe`). +- Positional primary argument, keyword modifiers: `Section("Title", + description:)`, `Code(source, filename:)`. +- The page must read with JavaScript off; the one `docs-nav` controller only + enhances. +- `DocsKit.configuration.themes` (`config/initializers/docs_kit.rb:22`) and the + `@plugin "daisyui" { themes: … }` list in + `app/assets/stylesheets/application.tailwind.css` must name the same nine + themes (`dark light synthwave retro cyberpunk dracula night nord sunset`). + Adding one means adding both. +- No inline `rubocop:disable` to force layout. + +## The CSS build and its generated file + +`bin/build-css` resolves `daisyui`, `docs-kit` and `phlex-forms` with +`bundle show` and writes their `**/*.rb` globs into +`app/assets/stylesheets/tailwind.sources.css`, which +`application.tailwind.css` imports, then runs `bunx @tailwindcss/cli`. It +fails fast if any of the three cannot be resolved — a silently missing +`@source` ships an unstyled site. `phlex-forms` is in that list because the +docs render real gem components whose literal class strings Tailwind must see. + +`tailwind.sources.css` is **committed output**. Every build path regenerates it, +but the committed paths still have to be real — see `../review/docs-site.md`. +`bun run build:css` / `watch:css` wrap the script; `lib/tasks/build_css.rake` +hooks `css:build` onto `assets:precompile` for Docker and production. + +## Running and checking it + +From inside `docs/` (never the repo root — separate bundle): + +``` +bin/dev # execs `bin/rails server` — it does NOT read Procfile.dev, + # so run `bun run watch:css` yourself in a second shell +bin/rubocop # its own config, inheriting rubocop-rails-omakase + docs-kit +bin/ci # ActiveSupport::ContinuousIntegration: setup, rubocop, + # bundler-audit, bin/importmap audit, brakeman +``` + +`docs/config/ci.rb` defines those five steps. There is **no RSpec suite in +`docs/`** and no GitHub workflow that runs `bin/ci` — the only automated docs +job is `deploy-docs.yml` on a release. + +## Deploy + +`deploy-docs.yml` (release published, or manual dispatch) calls docs-kit's +reusable `deploy.yml@main` with `image: zoolutions/phlex-forms` and +`service: phlex-forms`. `docs/config/deploy.yml` is the dash config: +`minimum_version: 4.0.7`, Cloudflare Tunnel plus dash-proxy, health check on +`/up`, status pages served from `public/`. Because the deploy is +release-triggered, the site is only ever as new as the last gem release. + +## Related + +- `../review/docs-site.md` — the accepted review rule about the generated CSS +- `../packaging/summary.md` — the release that triggers the deploy diff --git a/lode/form-api/summary.md b/lode/form-api/summary.md new file mode 100644 index 0000000..64871bc --- /dev/null +++ b/lode/form-api/summary.md @@ -0,0 +1,166 @@ +# Form API: the builder, the form, the field + +## The three hosts of one API + +`PhlexForms::Builder` (`lib/phlex_forms/builder.rb`, 235 lines) is the whole +caller-facing surface. It is a mixin, and its hosts supply five things: +`render`, `model`, `scope`, `errors`, and `field_object(name, error_name:)`. + +| Host | File | What it adds | +|---|---|---| +| `Forms::Form` | `lib/forms/form.rb` (277) | the `<form>` element, scope/url/method derivation, CSRF + `_method` + `enctype`, `fields_for`, the Rails-parity collection helpers | +| `Forms::Base` | `lib/forms/base.rb` (100) | declarative subclasses — `form_options`, `#fields`, the `live` macro | +| `Forms::FieldsForBuilder` | `lib/forms/fields_for_builder.rb` (71) | a nested scope; renders and themes through `@parent_form` | + +`Forms::Base < Forms::Form`, so a declarative form is a form. `FieldsForBuilder` +is not — it only includes the mixin and delegates `render`, `theme` and +`default_field_variants` to the parent (`fields_for_builder.rb:21-36`). + +`Form` also carries three Rails-`FormBuilder` compatibility aliases so call +sites that introspect the bound record keep working: `object` → `model`, +`object_name` → `scope` (`form.rb:25-26`) and `[]` → `field_object` +(`form.rb:75`), which is what makes `f[:email].hidden` read. + +## `field` — what one call does + +`Builder#field` (`builder.rb:67-94`), in order: + +1. prepends `default_field_variants` to the positional modifiers; +2. asks `PhlexForms::Inference.resolve` for the control kind, the (possibly + rewritten) attribute name, a label, choices, attributes, `multiple` and + `required` — see `../inference/summary.md`; +3. builds a `Forms::Field` for the **inferred** name while passing the + **original** name as `error_name:`, so a `belongs_to` rendered as + `country_id` still shows the error Rails put on `country`; +4. resolves `required` — an explicit `required:` wins, else the field's own + presence validators or the inferred association requirement; +5. resolves the label — `label: false` omits it, else `label:`, else the + inferred association label, else `Field#field_label`; +6. merges attributes **inference first, caller last** (`options = + inferred.attributes.merge(options)`), then layers validator-derived + attributes underneath via `Field#apply_validations`; +7. for `multiple` (a `has_many_attached`), sets `multiple: true` and an + array `name` unless the caller already passed them; +8. materializes `choices` — inference hands back a lambda for association + choices, called only when the caller passed none (`materialize_choices`, + `builder.rb:226-228`); +9. computes the checkbox-group ARIA pair (below); +10. renders `fo.control(…)` and yields the inner input to it. + +### `render_field_input` — the `as:` dispatch + +`builder.rb:196-216` switches on `as || resolve_input_type(name, modifiers)`. +Nine kinds have their own branch: `:select`, `:textarea`, `:toggle`, +`:checkbox`, `:checkbox_group`, `:file`, `:hidden`, `:rich_textarea`, `:tags`. +**Everything else is treated as an input type** and rendered through the +`:input` role with `type:` set to it (`:datetime` is rewritten to +`:"datetime-local"`). So `as: :radio` does not reach the `:radio` role — it +renders `<input type="radio">` through the input leaf. + +Two kinds deliberately drop `required:` before rendering: `:checkbox_group` +(one array name shared by many boxes, so the browser cannot satisfy it) and +`:tags` (the value lives in a hidden field). Both are commented at the branch. + +### Checkbox-group ARIA (`group_aria`, `builder.rb:101-115`) + +A checkbox group renders `div[role="group"]`, which a plain `<label for>` +cannot name. For `as: :checkbox_group` **only**, the builder stamps +`#{field_id}_label` / `#{field_id}_hint` ids on the Control's own visible label +and hint and passes `aria: { labelledby:, describedby: }` through to the group +div — reusing the chrome sighted users see rather than inventing a naming API. +Every other kind gets `[{}, {}]`, and so does a checkbox group with neither a +label nor a hint. + +## Escape hatches + +`Input`, `Select`, `Textarea`, `Checkbox`, `Radio`, `Toggle`, `FileInput`, +`Hidden`, `Label`, `Control` (`builder.rb:137-191`), plus `hidden_field` — a +Rails-`FormBuilder`-compatible alias of `Hidden` for straight migration of +`form.hidden_field(:token, value: x)` call sites. They skip `Inference` +entirely; `Input` resolves its type from `resolve_input_type`, which consults +only the 16 positional `INPUT_TYPE_MODIFIERS` and the 26-key +`INPUT_TYPE_INFERENCE` attribute-name map, falling back to `:text`. + +`row(columns:)` and `group(legend:)` are layout helpers rendering the `:row` and +`:group` roles. + +## `Forms::Field` — the per-field context + +`lib/forms/field.rb` (322 lines) knows one field's name, scope, model and error +set, and builds leaves with `name`/`id`/`value`/`error` wired in +(`field_attributes`, `field.rb:303-305`). Facts that bite: + +- **`input` reroutes hidden.** `Field#input` returns `#hidden` when + `type.to_s == "hidden"` (`field.rb:32-40`), so both `f.Input(:t, :hidden)` and + `f.Input(:t, type: :hidden)` land on the bare leaf. daisyUI's + `.input { display: inline-flex }` overrides WebKit's non-`!important` + `input[type=hidden] { display: none }`, which makes a styled hidden field a + visible, focusable tab stop in Safari. +- **`radio` drops the model value.** `field_attributes` carries + `value: field_value`; `Field#radio` removes it before merging the radio's own + positional value, and stamps `id: "#{field_id}_#{value}"` + (`field.rb:80-91`) — otherwise every radio in a group renders the model's + current value. +- **`field_value` is Ransack-safe.** It rescues `NoMethodError` but re-raises + unless the receiver is the model and the missing name is this field's + (`field.rb:206-214`), so a `method_missing`-backed getter still dispatches and + a genuine typo inside the getter is not swallowed. +- **`invalid?` checks both names** — `@name` and `@error_name` + (`field.rb:186-190`); `field_error_message` reads the first full message from + either (`field.rb:307-311`). +- **`field_id` flattens a nested scope** — `@scope.tr('[', '_').delete(']')`, + so `user[addresses_attributes][0]` becomes `user_addresses_attributes_0` + (`field.rb:196-202`). +- **`checkbox_group` resolves items itself** (`field.rb:138-153`): `value:` is a + Symbol or Proc; the per-item text is `item_label:` then `label:`, and when + neither is given the first of `PhlexForms::Inference::LABEL_METHODS` + (`name`, `title`, `label`, `to_s`) the item responds to. `item_label:` exists + so `f.field(:tags, as: :checkbox_group, label: "Tags")` can put a heading on + the Control and still customise item text. The leaf gets the resolved + `{ value:, label:, checked:, id: }` array as `options:` — never the raw + collection, and never the `value:`/`item_label:` resolvers, which are + consumed here. +- **`theme` is the form's**, falling back to `Theme.resolve(nil)` when the host + does not respond to `theme` (`field.rb:236-238`). + +## The `<form>` element + +`Form#initialize` (`form.rb:28-50`) derives: + +- **scope** — `model_name.param_key`, else a String/Symbol as given, else + `class.name.underscore.tr("/", "_")`. `scope: false` opts out entirely and + emits bare field names (reactive row editors, `<template>`-cloned rows). +- **url** — `derive_url` (`form.rb:258-265`); for `model: [parent, child]` the + record is the last element and the parents contribute `/parents/:id` + segments. +- **method** — a caller's `method:` wins; otherwise `:patch` when the record is + persisted, else `:post`. The element itself always uses `get`/`post` and a + `_method` hidden field carries the rest (`form_method`, `method_field`). +- **enctype** — `multipart/form-data` on every non-GET form + (`form_attributes`, `form.rb:193-206`), so a file input can never silently + fail to upload. +- **CSRF** — only when `Phlex::Rails::Helpers::FormAuthenticityToken` is + available and the method is not GET. + +`live: true` on an inline form raises `ArgumentError` (`form.rb:30-35`): the +live endpoint rebuilds the component from its class, and a block cannot be +serialized. The message names the bound record's class, or the literal +`YourModel` when no model was passed. + +### Nested attributes and collections + +`fields_for` (`form.rb:100-114`) nests under `scope[assoc_attributes]`, or under +the raw name with `nested_attributes: false` for a JSONB/hash column. It indexes +only a **genuine** collection: `collection?` is `Enumerable && !Hash` +(`form.rb:171-173`) — a Hash responds to `each_with_index` but is one nested +record, and iterating it would emit `scope[assoc][0][field]`. + +`collection_check_boxes` emits a leading empty-array hidden field then yields a +`CollectionCheckBoxBuilder` per item; `collection_select` maps the collection to +choice pairs and prepends `prompt:` as a blank-valued option. + +## Related + +- `../inference/summary.md` — what `field` asks the model +- `../theming/summary.md` — how a role becomes markup +- `../client-validation/summary.md`, `../live-validation/summary.md` diff --git a/lode/inference/summary.md b/lode/inference/summary.md new file mode 100644 index 0000000..b4628ae --- /dev/null +++ b/lode/inference/summary.md @@ -0,0 +1,112 @@ +# Inference: what the model is asked, and in what order + +`PhlexForms::Inference` (`lib/phlex_forms/inference.rb`, 238 lines) is a +`module_function` module with no state. It is pure introspection: every model +touch on the eager path is `respond_to?`-gated, and the two methods that walk a +model (`structural`, `from_column`) rescue `StandardError` to `nil` +(`inference.rb:81-88`, `130-142`), as does `validator_attributes` +(`191-209`). A PORO, a Struct or an untyped ActiveModel therefore falls through +to the attribute-name map rather than raising. ActiveRecord is never required. + +The one unguarded touch is `association_choices` (`inference.rb:171-177`), which +calls `klass.all` and `record.public_send(label_method)`. It runs from the +lambda `association` returns, so `Builder#materialize_choices` calls it long +after `structural`'s rescue has returned — an exception there reaches the +caller. + +## The Result + +`Result = Data.define(:as, :name, :label, :choices, :attributes, :multiple, +:required)` with a `.blank(as:, name:)` constructor (`inference.rb:23-27`). +`name` is the attribute the input renders under, which is not always the name +the caller passed. + +## Precedence — first hit wins + +`base_result` (`inference.rb:61-71`): + +| # | Source | Wins because | +|---|---|---| +| 1 | explicit `as:` | the caller always wins | +| 2 | a positional modifier in `Builder::INPUT_TYPE_MODIFIERS` (16 symbols) | `f.field :price, :number` | +| 3 | `choices:` present → `:select` | passing choices is asking for a select | +| 4 | model structure — rich text, attachment, enum, `belongs_to` | ground truth about the column's meaning | +| 5 | column type via `COLUMN_TYPE_MAP` (9 entries) | ground truth about its shape | +| 6 | `Builder::INPUT_TYPE_INFERENCE` attribute-name map (26 keys) | `email`, `phone`, `birthday` … | +| 7 | `:text` | | + +Steps 4 and 5 **and** the validator attributes are skipped entirely when +`PhlexForms.config.infer_from_model` is false: `base_result` returns +`name_map_result` straight away (`inference.rb:68`) and `resolve` returns before +merging validator attributes (`inference.rb:53`). Steps 1-3 and the name map +still apply — the knob turns off *model* inference, not all of it. + +## Step 4 in detail (`structural`, `inference.rb:81-88`) + +Tried in this order, first non-nil wins: + +1. **rich text** — a `rich_text_<name>` association exists (ActionText's + `has_rich_text`) → `:rich_textarea`. +2. **attachment** — `reflect_on_attachment(name)` → `:file`, with + `multiple: true` when the macro is `:has_many_attached`. +3. **enum** — `defined_enums` has the name → `:select` over + `[key.humanize, key]` pairs. +4. **association** — `reflect_on_association` for the name **or** the name minus + a `_id` suffix, matching only a non-polymorphic `belongs_to` + (`inference.rb:118-128`). The result rewrites `name` to + `reflection.foreign_key`, takes its label from `human_attribute_name`, and + marks `required` from the association's own presence validators. Its + `choices` is a **lambda** — `-> { association_choices(reflection.klass) }` — + so `klass.all` is only loaded when the caller passed no `choices:`; + `Builder#field` calls it through `materialize_choices`. The option text comes + from the first of `LABEL_METHODS` (`name`, `title`, `label`, `to_s`) the + **first** record responds to — it is memoised with `||=` and reused for every + record — and each option's value is `record.id`. + +`column_type` (`inference.rb:144-153`) prefers `type_for_attribute` and falls +back to `attribute_types`, so a plain `ActiveModel::Attributes` class works +without ActiveRecord. A `:number` from an integer column carries `step: 1`; a +decimal or float carries `(10 ** -scale).to_f` when the type reports a scale and +the String `"any"` when it reports none (`step_for`, `inference.rb:155-162`). + +`COLUMN_TYPE_MAP` covers non-string columns only — a `:string` column has no +entry, so it falls through to the name map, which is what disambiguates +`email`/`password`/`phone` from a plain text field. + +## Validator-derived attributes (`resolve`, `inference.rb:51-59`) + +Merged **orthogonally** and **underneath** whatever `base_result` produced: +`result.with(attributes: validator_attrs.merge(result.attributes))`. Two +validators contribute: + +- `LengthValidator#options[:maximum]` → `maxlength`, but only for the 7 + `TEXT_LIKE` kinds (`text email password tel url search textarea`). +- `NumericalityValidator` → `min`/`max`, but only when the kind is `:number`. + Inclusive bounds map directly; an exclusive bound maps to ±1 **only** with + `only_integer`, and is otherwise skipped — `greater_than: 0.5` has no correct + HTML equivalent (`numericality_attributes`, `inference.rb:211-225`). + +Any validator carrying `:if`, `:unless` or `:on` is skipped (`conditional?`, +`inference.rb:234-236`), and the same predicate gates `presence_validated?` +(`inference.rb:179-186`): those need server context the browser does not have. + +## Where the same logic lives three times + +"Conditional validator" and "has a presence validator" are written out in three +places, never shared: + +| Where | Methods | Test | +|---|---|---| +| `PhlexForms::Inference` | `presence_validated?` (`inference.rb:179-186`), `conditional?` (`234-236`) | `options.key?` | +| `Forms::Field` | `required?` (`field.rb:176-184`), `conditional?` (`278-280`) | `options.key?` | +| `Forms::Validations::Introspector` | `applicable_validators` (`introspector.rb:110-114`), `conditional?` (`116-119`) | `options[…].present?` | + +The Introspector's `.present?` test is the odd one: a validator written +`validates :x, presence: true, if: nil` is skipped by inference and by +`required?` but still mirrored to the client. A change to what "conditional" +means has to touch all three. + +## Related + +- `../form-api/summary.md` — how `field` consumes a `Result` +- `../client-validation/summary.md` — the other reader of the same validators diff --git a/lode/live-validation/summary.md b/lode/live-validation/summary.md new file mode 100644 index 0000000..8544b42 --- /dev/null +++ b/lode/live-validation/summary.md @@ -0,0 +1,111 @@ +# Live validation: the form as a reactive component + +`Forms::Live` (`lib/forms/live.rb`, 223 lines) makes a whole form one +phlex-reactive component. Blur on any input, plus a debounced form-wide `input` +event, POSTs every field to one `:validate` action that assigns a whitelisted +slice to the model, runs the **real** ActiveModel validators — uniqueness, +`:if`/`:unless`, `confirmation`, `:on` contexts all work — and replies with a +focus-preserving morph. **Nothing is ever persisted.** + +## Only a `Forms::Base` subclass can be live + +`Forms::Base.live(model:, scope: nil, debounce: 300)` (`base.rb:55-66`) raises +`PhlexForms::FeatureUnavailable` when phlex-reactive is absent, naming the form +class (or the literal `this form` for an anonymous class), then +`include Forms::Live` and calls `setup_live`. An inline +`Form(model:, live: …)` raises `ArgumentError` instead (`form.rb:30-35`): the +endpoint rebuilds the component from its class, and a caller's block cannot be +serialized. `reactive_available?` (`base.rb:69-71`) is factored out purely so a +spec can drive the guard. + +`setup_live` (`live.rb:45-58`) declares `reactive_state :model_gid, :touched`, +the `:validate` action with its param schema, and — guarded by `respond_to?` so +older phlex-reactive still loads — `skip_verify_authorized :validate`. +phlex-reactive >= 0.11 defaults `verify_authorized` on and raises +`AuthorizationNotVerified` for an action that never authorizes; `:validate` is a +deliberate no-persist read-only pass, so it opts out rather than pretending. + +## Identity is state, not a reactive record + +The signed token carries `model_gid` and `touched` (`live.rb:130-138`): + +- `model_gid` is the model's GlobalID **only when persisted** + (`signed_model_gid?`, `live.rb:204-206`) — `reactive_record` cannot round-trip + an unsaved draft, so a new record rebuilds as `live_model_class.new` + (`locate_live_model`, `live.rb:198-202`). +- `touched` is the list of field names the user has left. `field_object` + (`live.rb:185-192`) passes `errors: nil` for an untouched field, so nothing + flashes red before the user has finished typing. A form re-rendered after a + failed classic submit arrives with errors already on the model, so the + constructor unions in `@errors.attribute_names` (`live.rb:137`) and those + surface without a touch. + +`#id` (`live.rb:141-143`) is the Streamable contract: `@options[:id]` or +`"<edit|new>_<scope>"`. + +## The `:validate` action + +`validate(_touch: nil, **posted)` (`live.rb:149-154`) adds `_touch` to the +touched set, assigns, validates, and replies `morph`. The wire key is `_touch` +— underscored so it can never collide with a model attribute. The posted +attributes arrive under the form's scope, declared with the custom +`:form_attributes` param type, which passes a Hash through and `DROP`s anything +else (`register_param_type!`, `live.rb:36-42`). That type is registered from an +engine initializer because phlex-reactive freezes its registry after boot +(`engine.rb:38-40`); the spec suite registers it in `spec_helper.rb:22`. + +### The assignment whitelist + +`assign_live_attributes` (`live.rb:210-221`) writes only through public setters +the model responds to, and only for permitted keys. The default permit set is +derived (`derived_live_attributes`, `live.rb:111-124`): every +`attribute_names` entry, every validated attribute, and the +`<attr>_confirmation` twin of any `ConfirmationValidator`. `live_permit` replaces +that set and a declared `live_tags` name is unioned in, but `live_deny` is +subtracted **last** (`live_permitted_attributes`, `live.rb:98-103`), so +`live_deny :tags` does remove a lifted tag field from the permit set. +Class-level readers fall back up the superclass chain through `inherited_live`, +so a subclassed live form inherits its parent's configuration. + +## Live tag fields + +phlex-reactive's tag controller reads **one** `data-reactive-tags-field` per +reactive root, so a live form can lift **at most one** tag field onto its root; +a second `live_tags` call raises `ArgumentError` telling you to render it as a +standalone (non-live) `field :x, as: :tags` (`live.rb:84-94`). + +`Forms::Live#form_attributes` (`live.rb:160-181`) mixes onto the `<form>`: the +reactive root, the debounced `on(:validate, event: "input")` trigger, and — when +a tag field is declared — `reactive_tags(name: field_name(tag))` and +`reactive_filter(input: "#<query id>")`, deriving both through the same +`field_name`/`field_id` path the widget uses so the selectors match. + +`Forms::Live::Field` (`lib/forms/live/field.rb`, 46 lines) is the per-field +half. It merges the blur trigger into the input's `data:` through +`apply_validations` — the same seam the client-side mirror uses, and therefore +reaching only the builder methods that call it +(`../client-validation/summary.md`) — and it overrides `tag_field` so that +**only** the declared `live_tags` field renders the rootless variant +(`live/field.rb:26-37`); any other tag field falls through to the self-rooted, +non-live widget. Call-site `suggestions:` win over the declaration's unless they +are empty or nil (`blank_suggestions?`). + +Rendering rootless is what makes live tags work at all: with no nested reactive +root, the hidden tags input's nearest root ancestor is the `<form>`, so the +form owns the field and `:validate` collects it. + +## Soft-dependency surface + +`lib/phlex_forms.rb:96-103` `ignore`s six paths when `Phlex::Reactive` is +undefined — `forms/live.rb`, `forms/live/`, `forms/tag_field.rb`, +`forms/plain/tag_field.rb`, `forms/rootless_tag_field.rb`, +`forms/plain/rootless_tag_field.rb` — because each includes or inherits from a +class that includes `Phlex::Reactive::Component` / `ClientBindings` at class +level and so cannot even load. `Theme` correspondingly omits the `tag_field` +roles (`../theming/summary.md`). + +## Related + +- `../client-validation/summary.md` — the Stimulus mirror, and the shared + `apply_validations` merge seam +- `../packaging/summary.md` — the engine initializers diff --git a/lode/lode-map.md b/lode/lode-map.md new file mode 100644 index 0000000..de53804 --- /dev/null +++ b/lode/lode-map.md @@ -0,0 +1,44 @@ +# Lode map + +The repository's durable memory. Read this first; each file below is one topic +and states the system as it is. + +## Baseline + +| File | Purpose | +|---|---| +| [`summary.md`](summary.md) | what phlex-forms is, and the three invariants every change is measured against | +| [`terminology.md`](terminology.md) | the vocabulary — builder, role, leaf, Plain twin, binding contract, touched, rootless | +| [`practices.md`](practices.md) | patterns not already in `.claude/rules/`: issue-citing comments, adding a role, styling seams, the `apply_validations` seam | +| [`workflow.md`](workflow.md) | the profile the shared `/lode:` workflow skills read — commands, branches, layers, shapes, constraints, docs, CI, flakes, conflicts, verification | +| [`plans/README.md`](plans/README.md) | where a plan artifact goes | + +## Subsystems + +| File | Purpose | +|---|---| +| [`form-api/summary.md`](form-api/summary.md) | `PhlexForms::Builder` and its three hosts, what one `field` call does, the `as:` dispatch, `Forms::Field`, the `<form>` element, nested attributes | +| [`inference/summary.md`](inference/summary.md) | the precedence chain, what the model is asked and in what order, validator-derived attributes, the three copies of "conditional validator" | +| [`theming/summary.md`](theming/summary.md) | roles → leaf classes, the 19 (+2 reactive) roles, the binding contract, Plain twins, `DelegatedField`, `ClassMerge`, the literal-class-string rule, icons | +| [`client-validation/summary.md`](client-validation/summary.md) | `validate: true` — the Introspector, the `data-validations--*` encoding, inline `ManualRules`, the per-field merge seam, the ten Stimulus controllers | +| [`live-validation/summary.md`](live-validation/summary.md) | `live model:` — the `:validate` action, signed `model_gid`/`touched` state, the assignment whitelist, live tag fields, the phlex-reactive soft-dependency surface | +| [`packaging/summary.md`](packaging/summary.md) | boot order, the two Zeitwerk roots, the engine's four initializers, the gemspec, the two RuboCop cops, `bin/release` → `rake release`, the three workflows | +| [`testing/summary.md`](testing/summary.md) | the three spec layers, the four global helpers, the one global reset, what is covered and what is not | +| [`docs-site/summary.md`](docs-site/summary.md) | the docs-kit app under `docs/` — page registry, the behaviour → page map, the authoring contract, the CSS build, the deploy | + +## Review rules + +Accepted review findings, written as rules about the system. `/lode:gate` +enforces them before a push; `/lode:learn` adds to them. + +| File | Covers | +|---|---| +| [`review/docs-site.md`](review/docs-site.md) | the generated `tailwind.sources.css` and its `@source` paths | +| [`review/changelog.md`](review/changelog.md) | one `###` subsection per category per release heading | + +## Not in the lode + +`lode/tmp/` is gitignored scratch space for a run's evidence. The standing +rules live in `.claude/rules/` (coding style, testing, git workflow, agents) and +`CLAUDE.md`; this directory describes the system, not the process of changing +it — except `workflow.md`, which is the bridge. diff --git a/lode/packaging/summary.md b/lode/packaging/summary.md new file mode 100644 index 0000000..6fb75b1 --- /dev/null +++ b/lode/packaging/summary.md @@ -0,0 +1,142 @@ +# Packaging: boot, autoloading, the engine, the cops, the release + +## Boot order (`lib/phlex_forms.rb`, 110 lines) + +1. `date` and three ActiveSupport core-ext requires (`object/blank`, + `enumerable`, `string`), then `phlex`, `glyphs`, `zeitwerk` + (`phlex_forms.rb:3-9`). +2. `require "daisy_ui"` and `require "phlex/reactive"`, each in a + `begin/rescue LoadError` with a comment saying what is lost + (`phlex_forms.rb:11-26`). +3. `require_relative "phlex_forms/version"` — before Zeitwerk, and ignored by + it. +4. `module PhlexForms` with `Error < StandardError`, + `FeatureUnavailable < Error`, and the `configuration` / `config` / + `configure` / `reset_configuration!` singletons. +5. `module Forms; extend Phlex::Kit; end` — this is what makes every + `Forms::Foo` callable as a bare `Foo(...)` in a host that `include Forms`. +6. Zeitwerk, below. +7. `require_relative "phlex_forms/engine" if defined?(Rails::Engine)`. + +`lib/phlex-forms.rb` is a 4-line alias so `require "phlex-forms"` works; RuboCop +`Naming/FileName` excludes it. + +## Two sibling autoload roots + +``` +lib/phlex_forms -> PhlexForms:: (config, inference, theme, class_merge, …) +lib/forms -> Forms:: (the component kit) +``` + +They are **siblings, not nested** (`phlex_forms.rb:82-83`), which is why the +loader is driven manually instead of `Zeitwerk::Loader.for_gem` — the component +root has to map onto a top-level `Forms` while the internals stay under +`PhlexForms`. The inflector teaches it `phlex_forms`/`phlex-forms` → +`PhlexForms`. + +Four paths are ignored unconditionally (`phlex_forms.rb:86-91`): +`phlex_forms/version.rb` (already required), `lib/rubocop` and +`phlex_forms/rubocop.rb` (they open the `RuboCop::` namespace and load on +demand from a host's `.rubocop.yml`), and `phlex_forms/engine.rb` (required +conditionally at the bottom, so Zeitwerk must not eager-load it). + +Six more are ignored when `Phlex::Reactive` is undefined — see +`../live-validation/summary.md`. + +## The engine + +`PhlexForms::Engine` (`lib/phlex_forms/engine.rb`, 42 lines) is assets-and- +locales only: no `isolate_namespace`, no routes, models or helpers. Four +initializers: + +| Initializer | Does | +|---|---| +| `phlex_forms.assets` | appends `app/javascript` to `config.assets.paths` if the app has an assets config | +| `phlex_forms.importmap` (`before: "importmap"`) | appends `config/importmap.rb` to `importmap.paths` and `app/javascript` to `cache_sweepers`, each `respond_to?`-guarded | +| `phlex_forms.i18n` | `unshift`s the gem's `config/locales/*.yml` onto `I18n.load_path`, so a host app's own keys win | +| `phlex_forms.live_param_type` | `Forms::Live.register_param_type!` when phlex-reactive is present — the registry freezes after boot | + +`config/importmap.rb` pins the bundled controllers with `pin_all_from` under +`phlex_forms/controllers`, plus `phlex_forms/i18n` and `phlex_forms/messages` +by hand. Ruby locale files ship for `en`, `sv` and `de`; the JavaScript message +table bundles `en`, `fr` and `af` instead +(`../client-validation/summary.md`). + +## The gemspec + +`phlex-forms.gemspec`: Ruby `>= 3.4`, four runtime dependencies — +`activesupport (>= 7.0, < 9)`, `glyphs (>= 0.2.0, < 1)`, +`phlex (~> 2.0, >= 2.0.0)`, `zeitwerk (~> 2.6)`. Both `daisyui` and +`phlex-reactive` are deliberately absent; `glyphs` is present but is **not** the +default icon renderer (`../theming/summary.md`). + +`s.files` uses `git ls-files` with a `Dir` glob fallback for a checkout without +`.git`, and both branches must ship the same four prefixes — `exe/`, `lib/`, +`app/`, `config/` — plus `CHANGELOG.md`, `LICENSE.txt`, `README.md`. `app/` +carries the Stimulus controllers and `config/` the importmap and locales, so +dropping either publishes a gem whose engine wires up nothing. + +The gem's own `Gemfile.lock` is **gitignored** — correct for a library, and the +reason `rake release` never stages it. + +## The RuboCop cops + +Opt-in for host apps: `require: phlex_forms/rubocop` plus `inherit_gem: +phlex-forms: config/rubocop.yml`, which enables both cops scoped to +`app/components/**/*.rb` and `app/views/**/*.rb`. + +- `PhlexForms/RawForm` (51 lines, autocorrecting) rewrites `form_with` and a raw + `form(...)` element to `Form`. It deliberately skips a bare `form` with no + arguments and no block — that is a variable or method reference + (`form.label(...)`), not a Phlex element. +- `PhlexForms/LegacyFormMethod` (103 lines, message only) flags 12 Rails + `*_field` methods and 11 other legacy names on a receiver named `form`, `f`, + `af` or anything matching `/_form\z/`, suggesting `form.field(...)` first and + the PascalCase escape hatch second. + +## Release + +`bin/release` (111 lines) → `rake release[X.Y.Z]` (`Rakefile:37-190`). + +`bin/release` computes the next version from `lib/phlex_forms/version.rb` (the +source of truth, not the newest tag), shows the commits since the last tag, and +confirms. Arguments: `patch` (default) / `minor` / `major` / an explicit +`X.Y.Z` or `vX.Y.Z`, plus `list`, `--dry-run`/`-n`, `--force`/`-f`, `--help`. + +`rake release` aborts unless the branch is `main` and the tree is clean, then: +bumps the version file; string-edits **only** the `phlex-forms (X.Y.Z)` pin in +`docs/Gemfile.lock` (a full `bundle install` there can fail on that lockfile's +broad `PLATFORMS` list, which would abort a release); `gem build --strict`; +commits `chore: bump version to X.Y.Z`; pushes `main`; and creates the GitHub +Release. Every step is idempotent — an unchanged version, an already-pushed +`main` or an existing release is skipped, not repeated. + +Two extra modes: `rake release[X.Y.Z,force]` deletes the existing release and +tag first so a re-cut points at the current `main`, and `rake release[pre]` +re-releases the version already in the file as a GitHub prerelease. Publishing +is entirely CI's job — never `gem push` by hand. + +## CI + +Three workflows, all in `.github/workflows/`: + +- `main.yml` — on push to `main` and on every PR. `Lint` (`bundle exec rubocop + lib spec`, Ruby 4.0) and `Specs (Ruby 3.4 | 4.0)` (`bundle exec rspec`), + `fail-fast: false`. +- `release.yml` — on a published release. `test` (rspec on 3.4) → `build` + (verifies the tag matches `PhlexForms::VERSION`, `gem build --strict`, fails + if the packed gem contains any `.git*`, `*.gemspec`, `spec/` or `test/`, + emits sha256/sha512) → `publish-rubygems` (trusted publishing via + `rubygems/configure-rubygems-credentials`, a Sigstore bundle, skipped with a + warning if the version is already on RubyGems) → `upload-release-assets`. +- `deploy-docs.yml` — on a published release or manual dispatch; calls + `zoolutions/docs-kit/.github/workflows/deploy.yml@main` with + `image: zoolutions/phlex-forms` and `service: phlex-forms`. + +There is **no** workflow for the `docs/` app itself: `docs/bin/ci` (setup, +rubocop, bundler-audit, `bin/importmap audit`, brakeman) is run by hand. + +## Related + +- `../testing/summary.md` — what the CI jobs run +- `../docs-site/summary.md` — the app `deploy-docs.yml` ships diff --git a/lode/plans/README.md b/lode/plans/README.md new file mode 100644 index 0000000..167c643 --- /dev/null +++ b/lode/plans/README.md @@ -0,0 +1,17 @@ +# Plans + +Where a plan artifact goes in this repository. + +**Default: a GitHub issue** on `zoolutions/phlex-forms`. Implementation then runs +against the issue number, and the PR closes it with a `Closes #N` keyword in the +body (see `../workflow.md`, Branches and PRs). + +**A file instead**, when the plan is too long or too provisional for an issue: +`docs/plans/YYYY-MM-DD-<slug>.md`. That directory does not exist yet — create it +on first use. It sits inside the docs-kit Rails app but outside everything the +app loads: `docs/.rubocop.yml` lints only `app/`, `config/`, `Rakefile` and +`config.ru`, pages are routed from `docs/app/models/doc.rb` alone, and the +Docker build copies but never reads it. Leave a plan file uncommitted unless the +user asks for it in the branch. + +Nothing here moves existing plans; there are none. diff --git a/lode/practices.md b/lode/practices.md new file mode 100644 index 0000000..0a11fa2 --- /dev/null +++ b/lode/practices.md @@ -0,0 +1,87 @@ +# Practices + +Patterns this codebase follows that `.claude/rules/` does not already state. +The rules own the basics — file size, `respond_to?` guards, literal class +strings, soft dependencies, delegation to the `daisyui` gem, TDD, conventional +commits, the release path. Read `.claude/rules/coding-style.md`, +`.claude/rules/testing.md` and `.claude/rules/git-workflow.md` first; this file +is the rest. + +## A comment that explains a non-obvious choice cites its issue + +Fifteen comments across ten files carry the issue that forced the behaviour — +`(issue #9)`, `(issue #13)`, `(#168)` for an upstream phlex-reactive issue, +`zazu#2934` for a bug found in a consuming app. They are load-bearing: without +`(issue #13)` the `field_attributes.except(:value)` in `Field#radio` reads like +a defensive habit instead of the fix for every radio rendering the model's +current value. A comment that says *why* with no issue is fine; a comment that +restates the next line is not. + +## Add a role, not a class + +A new control kind is four coordinated edits, in this order: + +1. the leaf under `lib/forms/`, honouring the binding contract + (`*modifiers, name:, id:, value:, error:, disabled:, required:`); +2. a `Forms::Plain::` twin — a **subclass** that overrides only rendering, so + the binding logic exists once (`lode/theming/summary.md`); +3. both maps in `PhlexForms::Theme` (`Theme.daisy` **and** `Theme.plain`), or + `reactive_roles` if it needs phlex-reactive; +4. a `Forms::Field` builder method that assembles it from `field_attributes`, + and a `render_field_input` branch if `field` should reach it by `as:`. + +Skipping (2) or (3) is the failure mode: the daisy path renders and the plain +path raises `KeyError` at request time, where no spec looks unless a theme-parity +example exists. + +## Styling seams beat a rewritten template + +When a Plain twin would have to copy a complex `view_template` to restyle it, +extract the class strings into small reader methods on the daisy leaf and let +the twin override those instead. `Forms::TagField` (seven seams) and +`Forms::CheckboxGroup` (four) are the precedent; the markup then cannot drift +between themes because there is only one copy of it. + +## Options flow through `apply_validations`, and `data:` merges by key + +`Forms::Field#apply_validations` is where a field's `data:` accumulates — the +Stimulus validation attributes, the live blur trigger, and whatever the caller +passed. `merge_data` joins `:controller` and `:action` with a space +(`TOKEN_JOINED_DATA_KEYS`) and replaces everything else. Anything that needs to +add a Stimulus controller to a field goes through this seam; writing +`data: { controller: … }` directly silently drops the others. + +## A missing soft dependency raises a sentence, not a `NameError` + +`PhlexForms::FeatureUnavailable` messages name the gem, the Gemfile line to add, +and the working alternative — `Theme.daisy` points at the plain theme, +`Base.live` points at `validate: true`. A guard that returns `nil` or lets a +`NameError` escape is the wrong shape: the caller asked for the feature +explicitly. + +## Derived values are `Data`, not hashes + +`PhlexForms::Inference::Result` is a `Data.define` with a `.blank` constructor +and `.with` for the incremental refinements each inference step makes. A new +multi-valued return from an internal module follows it rather than returning a +hash whose keys are documented in a comment. + +## Endless defs for one-line readers + +`def field_name(name) = @scope ? "#{@scope}[#{name}]" : name.to_s`, +`def group_classes = VARIANT_CLASSES.fetch(…)`. Used consistently for the +styling seams and the Rails-parity accessors; a method with a guard clause or a +rescue is written out. + +## A user-facing change lands with its changelog entry and its docs page + +One PR carries the code, a `CHANGELOG.md` entry under the right existing +`###` subsection (`lode/review/changelog.md`), and the `docs/` page that +describes the behaviour (`lode/docs-site/summary.md` has the map). The README +covers the same surface as the docs site and drifts independently — check it +too. + +## Related + +- `lode/workflow.md` — the commands, branches, CI and conflict rules +- `lode/review/` — accepted review findings as rules diff --git a/lode/review/changelog.md b/lode/review/changelog.md new file mode 100644 index 0000000..e47e561 --- /dev/null +++ b/lode/review/changelog.md @@ -0,0 +1,9 @@ +Accepted review findings about `CHANGELOG.md`. + +### A release heading carries at most one `###` subsection per Keep-a-Changelog category +- **Holds because:** the file is Keep a Changelog, so a reader scans `## [version]` → `### Added` / `### Changed` / `### Fixed` and expects that subsection to be the whole list for that category in that release. A PR that opens its own `### Fixed` at the top of `## [Unreleased]` when one already exists further down splits the list in two, and the second half is read as belonging to an earlier release or missed entirely. A new entry is appended to the existing subsection, not given a new heading. +- **Where:** `CHANGELOG.md` +- **Safe direction:** find the existing subsection under the target release heading and add the bullet there; only create a `###` heading when that category has none. The same rule resolves a merge conflict in this file — union the bullets under one heading, never keep both sides' subheadings (`lode/workflow.md`, Conflicts). +- **Proven by:** no test. `ruby -e` over the file, grouping `###` lines by the preceding `##`, shows the duplicates. +- **Current state:** `## [Unreleased]` is the only release heading in the file — every version through 0.3.1 shipped without one — and it carries two `### Fixed` (lines 10 and 71) and two `### Changed` (59 and 137). The next changelog edit merges rather than adding a third. +- **Origin:** PR #23 diff --git a/lode/review/docs-site.md b/lode/review/docs-site.md new file mode 100644 index 0000000..d0ccd2d --- /dev/null +++ b/lode/review/docs-site.md @@ -0,0 +1,8 @@ +Accepted review findings about the docs app under `docs/` and its generated assets. + +### `tailwind.sources.css` is committed output whose `@source` globs must be paths that exist outside the machine that generated them +- **Holds because:** `bin/build-css` writes one `@source "<path>/**/*.rb";` line per gem from `bundle show daisyui|docs-kit|phlex-forms`, so the file records whatever the generating environment resolved — a real gem dir and the repo checkout normally, but an ephemeral worktree or sandbox path when the build ran in one. `application.tailwind.css` `@import`s the generated file, and `phlex-forms` is in the list because the docs render live gem components whose literal class strings (`badge`, `menu`, `input`) Tailwind must scan. Every build path does regenerate it first — `bun run build:css` directly, and Docker through `lib/tasks/build_css.rake`, which enhances `assets:precompile` — so a stale path is not a build failure. It is a landmine: a Tailwind run that consumes the committed file without regenerating silently drops those classes and ships an unstyled site, the exact failure `bin/build-css`'s fail-fast guard exists to prevent, and nothing in CI checks it. +- **Where:** `docs/bin/build-css`, `docs/app/assets/stylesheets/tailwind.sources.css`, `docs/app/assets/stylesheets/application.tailwind.css`, `docs/lib/tasks/build_css.rake`, `docs/Dockerfile` (`bin/rails assets:precompile`) +- **Safe direction:** regenerate in a real checkout and commit that, or leave the file untouched in the diff. Never hand-edit a path into it, and never commit a `tailwind.sources.css` produced inside a temporary worktree, a sandbox, or another repo's scratch directory — read the diff before staging it. +- **Proven by:** no test. The current committed file points at `/Users/mhenrixon/.gem/…` and `/Users/mhenrixon/Code/mhenrixon/phlex-forms`, which is the correct shape; verification is reading the three `@source` lines. +- **Origin:** cubic learning bc7ba61f; PR #24 diff --git a/lode/summary.md b/lode/summary.md new file mode 100644 index 0000000..b8e6df1 --- /dev/null +++ b/lode/summary.md @@ -0,0 +1,39 @@ +# phlex-forms + +A model-bound form builder for [Phlex](https://www.phlex.fun), published as the +`phlex-forms` gem. One verb carries the library: `f.field :email` renders a +label, an input and an error-or-hint in a single call, with the control type, +the `required` flag, the choices and the label all read off the bound model +(`PhlexForms::Inference`). Components live under `Forms::` (a `Phlex::Kit`, so a +host that `include Forms` calls `Form(model:) { … }` bare); the machinery lives +under `PhlexForms::`. The same form class renders DaisyUI markup or bare +semantic HTML by swapping a theme, and a `Forms::Base` subclass can opt into +server-truth live validation over phlex-reactive or a client-side Stimulus +mirror of the model's own validators. + +Three invariants govern every change: + +1. **Everything degrades.** `daisyui` and `phlex-reactive` are soft dependencies + (`require`-rescue-`LoadError` plus Zeitwerk `ignore`). The introspection + paths — `PhlexForms::Inference`, `Forms::Field#required?`, + `Forms::Field#field_value` — sit behind `respond_to?` guards and rescue to a + safe default, so the gem boots and renders (Plain theme, attribute-name + inference) for a PORO in a host that has neither gem installed. The + deliberate exceptions are the Rails-parity helpers a caller opts into: + `Form#time_zone_select` and `Form#collection_check_boxes` call + `@model&.public_send(name)` directly, and `Inference.association_choices` + calls `klass.all` from a lambda that runs outside `structural`'s rescue. +2. **A role, not a class.** A leaf component is reached through + `PhlexForms::Theme[role]`, never by name, and every role is mapped in both + `Theme.daisy` and `Theme.plain`. The leaf initializer signature + (`*modifiers, name:, id:, value:, error:, required:, …`) is the seam that + makes the two interchangeable. +3. **The caller always wins.** Inference is a default: an explicit `as:`, a + positional type modifier, `choices:`, or any passed-through option beats + anything read from the model, and validator-derived attributes merge + underneath the caller's options. + +The gem ships four runtime dependencies (`activesupport`, `glyphs`, `phlex`, +`zeitwerk`), a Rails engine that is loaded only when `Rails::Engine` is defined, +two RuboCop cops that host apps opt into, and a docs-kit site under `docs/` that +deploys from the same GitHub Release that publishes the gem. diff --git a/lode/terminology.md b/lode/terminology.md new file mode 100644 index 0000000..923143c --- /dev/null +++ b/lode/terminology.md @@ -0,0 +1,58 @@ +# Terminology + +- **builder** — the object yielded by `Form(model:) { |f| … }`, and `self` + inside a `Forms::Base#fields`. The API is the `PhlexForms::Builder` mixin, + included by `Forms::Form` and `Forms::FieldsForBuilder`. +- **the `field` verb** — `Builder#field(name, *modifiers, **options)`: the + primary API. Renders a **control** wrapping label + input + error/hint. +- **escape hatch** — the ten PascalCase builder methods (`Input`, `Select`, + `Textarea`, `Checkbox`, `Radio`, `Toggle`, `FileInput`, `Hidden`, `Label`, + `Control`, `builder.rb:137-191`), plus `hidden_field` (a Rails-`FormBuilder` + alias of `Hidden`) and `Form#submit`. Same model binding, no control wrapper, + no inference beyond the attribute-name map. +- **leaf component** — a `Forms::*` Phlex component that renders one element + (`Forms::Input`, `Forms::Select`, …). Reached through a theme role, never by + name. +- **role** — a theme key (`:input`, `:select`, `:control`, `:hidden`, …). + `PhlexForms::Theme#[]` maps a role to a leaf class and raises `KeyError` + listing the known roles for an unknown one. +- **Plain twin** — the `Forms::Plain::*` subclass of a daisy leaf that + overrides only rendering (a `view_template`, or the styling seams), accepts + positional variants and ignores them, ships zero styling classes, and signals + invalidity with `aria-invalid` instead of a colour class. +- **binding contract** — the leaf initializer keywords a theme swap must + preserve: `*modifiers, name:, id:, value:, error:, disabled:, required:`. + `:hidden` is the deliberate exception — `name:`/`id:`/`value:` only. +- **delegation** — a daisy leaf hands its markup to the `daisyui` gem + (`render DaisyUI::Input.new(*daisy_modifiers, **binding_attributes)`) instead + of writing daisyUI classes itself; the helpers are in + `PhlexForms::DelegatedField`. +- **modifier** — a positional Symbol on a component call (`:primary`, `:lg`). + Most name a daisyUI variant; the 16 in `Builder::INPUT_TYPE_MODIFIERS` name an + input type instead and are stripped before the leaf sees them. +- **field variants** — modifiers prepended to every `field`'s inner input; + stacked global (`PhlexForms.config.field_variants`) → form + (`field_variants:` / `form_options`) → call site, last wins. +- **inference** — `PhlexForms::Inference.resolve`, returning a `Result` Data + object (`as`, `name`, `label`, `choices`, `attributes`, `multiple`, + `required`). Gated by `PhlexForms.config.infer_from_model`. +- **name rewrite** — inference resolving `:country` to the foreign key + `:country_id` for a `belongs_to`; the field renders under the new name while + errors still read from the association name (`error_name:`). +- **client-side validation** (`validate: true`) — `Forms::Validations:: + Introspector` translating the model's own validators into `data-validations--*` + attributes that the eight bundled Stimulus field controllers re-check in the + browser. Server validation stays authoritative. +- **live validation** (`live model: …`) — the whole form as one phlex-reactive + component; blur and a debounced form-wide input POST to a `:validate` action + that assigns a whitelist, runs the real validators, and morphs the reply back. + Nothing is persisted. +- **touched** — the reactive state list of field names the user has left; an + untouched field is rendered with no errors so nothing flashes early. +- **rootless tag field** — the tag widget rendered without its own reactive + root, so the enclosing live `<form>` owns its hidden field. Lifted onto the + form root by `live_tags`. +- **the docs site** — the docs-kit Rails app under `docs/`, with its own + bundle, Ruby version, RuboCop config and lockfile; it path-depends on the gem. +- **lode** — this directory: the repo's durable memory. `lode/review/` holds + accepted review findings as rules; `/lode:gate` enforces them before a push. diff --git a/lode/testing/summary.md b/lode/testing/summary.md new file mode 100644 index 0000000..441542a --- /dev/null +++ b/lode/testing/summary.md @@ -0,0 +1,89 @@ +# Testing: layers, helpers, and what is and is not covered + +RSpec, 18 spec files, **198 examples** (`grep -rhE '^\s*it ["(]' spec +--include='*_spec.rb' | wc -l`). `.rspec` loads `spec_helper` for every file and +prints documentation format; the suite runs in random order with a seeded +`srand`. + +## The three layers + +| Layer | Path | Boots | For | +|---|---|---|---| +| Unit | `spec/phlex_forms/**` | nothing | `Inference` precedence, `ClassMerge` families | +| Integration | `spec/forms/**` | a Phlex kit context, no Rails request | that a rendered form emits the expected markup for a bound model | +| Cops | `spec/rubocop/cops_spec.rb` | RuboCop against source snippets | the two cops flag and autocorrect | + +## Helpers (`spec/support/`, all four included globally) + +- `ModelHelpers#build_model(name, validations: -> { … }, **attributes)` — an + anonymous `ActiveModel::Model` + `ActiveModel::Attributes` class with a real + `name` singleton (ActiveModel::Naming needs one) and the given attributes. + Validations are applied with `class_exec`, not `class_eval`, so an arity-0 + lambda is not rejected for the implicit class argument. No database. +- `PhlexHelpers#render_form(model, **form_args) { |f| … }` — renders a real + `Forms::Form` through a `Phlex::HTML` context that `include Forms`, exactly as + a host app would. `#kit(&)` is the lower-level version for bare kit helpers. +- `ComponentHelpers#render_component(component, &)` — named so it does not + shadow Phlex's own instance-level `render` inside component contexts. +- `HTMLHelpers#html(string)` — whitespace normalisation so a spec can compare + against a readable heredoc. + +## Global setup (`spec/spec_helper.rb`, 49 lines) + +- Loads `i18n`, `active_model`, `phlex-forms`, `super_diff/rspec`. +- Pushes the gem's own `config/locales/*.yml` onto `I18n.load_path` and sets + `available_locales` to `en sv de` — the engine does this in a host app, so + without it `Forms::Submit`'s `I18n.t("cmd.create_model")` would not resolve. +- When `Phlex::Reactive` is defined, installs a `MessageVerifier` (token signing + happens on every live render) and calls `Forms::Live.register_param_type!`. +- `config.after { PhlexForms.reset_configuration! }` — the one global reset. A + spec that sets a theme, an icon renderer or `infer_from_model` does not have + to clean up; a spec that mutates anything **else** global does. + +## Conventions the suite follows + +- Assert on semantics — `name="user[email]"`, the selected option, the error + text, the error variant class — never a full-HTML snapshot. +- Live specs are guarded by `if defined?(Phlex::Reactive)` and stub the reply, + since no endpoint is booted. The one behaviour a spec cannot drive — + `verify_authorized` — is asserted at class level + (`skip_verify_authorized?(:validate)`). +- Both themes get exercised where a Plain twin exists. + +## Coverage, honestly + +Well covered: `Inference` precedence (17 examples), `ClassMerge` (13), +`Forms::Form` (28) and the leaf components (28 in `components_spec.rb`), +`checkbox_group` (26), the tag field across three files (14 + 5 + 4) and its +Plain twin (4), `Introspector` (11), `Live` (11), `rich_textarea` (8), `Theme` +(8), `Base` (5), `file_input` (5), `layout` (4), the cops (6). + +Thin or absent: + +- **`PhlexForms::Configuration` has no spec file of its own.** Its knobs + (`theme`, `infer_from_model`, `field_variants`, `icon_renderer`) are exercised + only indirectly, through the form and theme specs. +- **The Stimulus controllers have no JS tests** — there is no JS test runner in + the repo. Everything under `app/javascript/` is verified only by reading. +- `spec/forms/time_zone_select_spec.rb` has a single example. +- No spec renders the gem inside a real Rails app, so `PhlexForms::Engine`'s + four initializers are uncovered. + +## CI + +`.github/workflows/main.yml`: `Lint` runs `bundle exec rubocop lib spec` on +Ruby 4.0; `Specs (Ruby 3.4)` and `Specs (Ruby 4.0)` run `bundle exec rspec`, +with `fail-fast: false` so both Ruby cells report. Nothing in the suite touches +the network or a database, so two checkouts can run it concurrently. + +The local pre-commit gate is `bundle exec rspec && bundle exec rubocop lib spec`. +`bundle exec rake` (the default task, `spec` then `rubocop`) cannot be used: +`RuboCop::RakeTask` passes no paths, so RuboCop walks into `docs/`, reads +`docs/.rubocop.yml` and aborts with `cannot load such file -- +docs_kit/rubocop` — the docs app's RuboCop plugin lives in its own bundle. + +## Related + +- `../workflow.md` — the commands, and the shapes every change is checked + against +- `../packaging/summary.md` — the release workflow's own test job diff --git a/lode/theming/summary.md b/lode/theming/summary.md new file mode 100644 index 0000000..739936e --- /dev/null +++ b/lode/theming/summary.md @@ -0,0 +1,137 @@ +# Theming: roles, leaves, and the two themes + +## A theme is a frozen role → class map + +`PhlexForms::Theme` (`lib/phlex_forms/theme.rb`, 99 lines) wraps one Hash. +`#[]` `fetch`es and raises `KeyError` listing the known roles +(`theme.rb:27-31`); `#with(**overrides)` returns a new theme, so a host can +replace a single leaf: `PhlexForms::Theme.resolve(:plain).with(input: MyInput)`. + +`Theme.resolve(value)` (`theme.rb:38-47`) accepts a `Theme`, `:daisy`, +`:plain`, or `nil` meaning `PhlexForms.config.theme`; anything else raises +`ArgumentError`. `Configuration#theme` defaults to `Theme.daisy` when `DaisyUI` +is defined and `Theme.plain` otherwise (`configuration.rb:57-59`), and +`Theme.daisy` raises `PhlexForms::FeatureUnavailable` if the gem is absent +(`theme.rb:50-54`) — so asking for daisy explicitly in a host without the gem +fails loudly instead of rendering unstyled. + +Selection points, most local last: `PhlexForms.configure { |c| c.theme = … }` → +`form_options theme: :plain` → `Form(model:, theme: :plain)`. The form resolves +once in its initializer (`form.rb:40`) and `Forms::Field` reads it back through +the form (`field.rb:236-238`). + +## The roles + +Both themes map the same **19** roles literally: `input`, `select`, +`choices_select`, `textarea`, `rich_textarea`, `checkbox`, `toggle`, `radio`, +`checkbox_group`, `file`, `wrapped_input`, `hidden`, `control`, `label`, +`field_error`, `field_hint`, `submit`, `row`, `group`. Two more — +`tag_field` and `rootless_tag_field` — are added by `reactive_roles` +(`theme.rb:92-96`) **only when `Phlex::Reactive` is defined**, so a host without +phlex-reactive gets the theme's own `KeyError` rather than a `NameError` on an +unloaded class. That makes 21 roles in a reactive host, 19 without. + +`Forms::` also holds components no role points at — `EmailField`, +`PasswordField`, `Range`, `TimeZoneSelect`, `CollectionCheckBox`, +`CollectionLabel` — reached by name from the kit or from `Form`'s Rails-parity +helpers, and therefore not theme-swappable. + +Four roles are deliberately shared or re-pointed rather than twinned one-to-one: + +| Role | daisy | plain | Why | +|---|---|---|---| +| `hidden` | `Forms::Hidden` | `Forms::Hidden` | a hidden field has no styling, no variants and no error state | +| `choices_select` | `Forms::ChoicesSelect` | `Forms::Plain::Select` | no choices.js in the plain theme; it swallows `searchable:` | +| `rich_textarea` | `Forms::RichTextarea` | `Forms::Plain::Textarea` | rich text needs an editor integration the plain theme does not ship | +| `toggle` | `Forms::Toggle` | `Forms::Plain::Checkbox` | a toggle is a styled checkbox | + +## The binding contract + +A leaf's initializer keywords are the seam: `*modifiers, name:, id:, value:, +error:, disabled:, required:` plus per-leaf extras. All 17 `Forms::Plain::*` +classes are **subclasses** of their daisy leaf, so the binding logic is written +once and cannot drift. They override rendering in one of two ways: + +- **14 override `view_template`** — `Plain::Checkbox`, `Control`, `FieldError`, + `FieldHint`, `FileInput`, `Group`, `Input`, `Label`, `Radio`, `Row`, `Select` + (which also swallows `searchable:` in its own `initialize`), `Submit`, + `Textarea`, `WrappedInput`. `Forms::Plain::Checkbox` still emits the hidden + unchecked-value input, because that is binding, not styling. +- **3 override only styling seams and reuse the inherited template** — + `Plain::TagField`, `Plain::RootlessTagField`, `Plain::CheckboxGroup`. Their + markup can never drift between themes because there is only one copy of it: + + - `Forms::TagField` exposes `root_classes`, `list_classes`, `menu_classes`, + `option_classes`, `chip_classes`, `remove_classes`, `input_classes` + (`tag_field.rb:132-138`). `Forms::Plain::TagField` overrides all seven: + `root_classes` becomes the bare hook `"tag-field"`, `input_classes` becomes + the caller's own `class:`, and the other five become `nil`. + `Forms::RootlessTagField` reuses `#body` verbatim and only drops the + reactive-root `<div>`, and `Forms::Plain::RootlessTagField` repeats the same + seven overrides (Ruby has no multiple inheritance to share them). + - `Forms::CheckboxGroup` exposes `render_checkbox`, `group_classes`, + `item_classes`, `item_label_classes` (`checkbox_group.rb:82-102`); the Plain + twin overrides those four, replacing the DaisyUI-delegated checkbox with a + bare `<input type="checkbox">`. + +## Delegation (`PhlexForms::DelegatedField`) + +`lib/phlex_forms/delegated_field.rb` (71 lines) is included by the daisy leaves +that wrap a `daisyui` gem component. A leaf sets `@modifiers`, `@error`, +`@disabled`, `@required`, `@full_width`, `@attributes`, then calls: + +- `normalize_modifiers` — drops `IGNORED_MODIFIERS` (`:bordered`, a daisyUI v4 + no-op in v5) so v4-era call sites keep working. +- `daisy_modifiers` — appends `:error` when the field is invalid **and** the + caller did not already pass the `:error` modifier itself + (`delegated_field.rb:23-27`). A different colour modifier (`:primary`) does + not suppress it — the check is `@modifiers.include?(:error)`, not "any + colour" — so an invalid `:primary` field renders `[:primary, :error]`. +- `binding_attributes(**extra)` — `name`/`id`/`class` plus the caller's + passthrough attributes minus `:error`, `:value`, `:class`, with `disabled` + and `required` added only when true, then `compact`ed. +- `unstyled_attributes(**extra)` — the Plain variant: caller classes verbatim, + no width class, and `aria-invalid: true` instead of a colour modifier. + +`width_class` (`delegated_field.rb:48-52`) returns the caller's `class:` +untouched unless `@full_width`; with it, it merges `"w-full"` and the caller's +`class:` through `ClassMerge` rather than joining them, so a caller's `w-36` +**replaces** the default instead of leaving stylesheet source order to pick. + +## `PhlexForms::ClassMerge` + +`lib/phlex_forms/class_merge.rb` (66 lines). Three families are mutually +exclusive and resolved last-token-wins: `SIZE` (`-xs|sm|md|lg|xl` suffix), +`COLOR` (`-primary|secondary|accent|neutral|info|success|warning|error` +suffix), and `WIDTH` (anchored `\Aw-`, so `min-w-*`/`max-w-*` compose freely). +A family key is namespaced by the token's prefix — `input-sm` and `select-sm` +are different families — and `WIDTH` is tested first so a `w-*` token can never +be mis-bucketed (`family_key`, `class_merge.rb:56-64`). Anything the three +patterns do not match passes through in order. There is deliberately no +`tailwind_merge` dependency: these are the only conflicts form fields produce. + +## The class-string rule + +Class strings must be **literal and greppable**: a host's Tailwind scanner reads +the gem's `.rb` files, so a name built with `#{…}` is invisible to it and the +style never ships. Variant maps are therefore literal tables — +`Forms::Row::COLUMN_CLASSES`, `Forms::CheckboxGroup::VARIANT_CLASSES` — and a +daisy leaf hands its size/colour modifiers to the `daisyui` gem rather than +composing a class name. `Forms::ChoicesSelect` is the exception that proves it: +choices.js replaces the element, so its size and colour ride as +`data-choices-size-value` / `data-choices-color-value` for the controller to +apply, never as classes (`choices_select.rb:63-82`). + +## Icons + +`Configuration#icon_renderer` defaults to a lambda calling +`PhlexForms::InlineIcons.render`, a one-entry map (`chevron-down`) returning a +raw SVG String; an unknown name returns `""` rather than raising. `glyphs` is a +hard dependency but **not** the default renderer — it resolves SVGs from the +host's rails_icons asset tree, which a minimal host has not set up, so it is +opt-in via `PhlexForms::Configuration.glyphs_renderer` (`configuration.rb:28-35`). + +## Related + +- `../form-api/summary.md` — who asks for a role +- `../packaging/summary.md` — why the reactive-gated leaves are not autoloaded diff --git a/lode/workflow.md b/lode/workflow.md new file mode 100644 index 0000000..9f72f4e --- /dev/null +++ b/lode/workflow.md @@ -0,0 +1,187 @@ +# Workflow profile + +Everything the shared workflow skills (`/lode:lfg`, `/lode:review-pr`, +`/lode:finish-prs`, `/lode:debug-flaky`, `/lode:tdd`, `/lode:plan`) need to know +about phlex-forms that is not already in `CLAUDE.md`, `.claude/rules/` or the +rest of `lode/`. + +## Commands + +| Purpose | Command | Notes | +|---|---|---| +| fast loop (one file) | `bundle exec rspec spec/<path>_spec.rb` | no network, no database, no services; a unit file runs in well under a second | +| full suite | `bundle exec rspec` | 198 examples, all in-process. Safe to run in two worktrees at once — nothing shared, no ports, no fixture dirs | +| lint | `bundle exec rubocop lib spec` | the CI Lint job's exact invocation, and the only one that works. `bundle exec rubocop -A lib spec` to autocorrect. The two paths are deliberate: `Rakefile`, `bin/` and `*.gemspec` are not linted | +| both, as CI sees them | `bundle exec rspec && bundle exec rubocop lib spec` | **not** `bundle exec rake`. The default task is `spec` then `rubocop`, and `RuboCop::RakeTask` passes no paths, so RuboCop descends into `docs/`, loads `docs/.rubocop.yml` and dies on `cannot load such file -- docs_kit/rubocop` — that gem is in the docs bundle, not the root one. `bundle exec rake` therefore always fails at the lint step, whatever the code says. `CLAUDE.md` and `.claude/rules/git-workflow.md` both still name it as the pre-commit gate | +| one CI cell locally | n/a | the matrix is only Ruby 3.4 and 4.0 running the same two commands; `.tool-versions` pins 4.0.5 | +| docs lint / checks | `cd docs && bin/rubocop`, `cd docs && bin/ci` | separate bundle — `cd docs && bundle install` first. `bin/ci` is `ActiveSupport::ContinuousIntegration` over `docs/config/ci.rb`: setup, rubocop, bundler-audit, `bin/importmap audit`, brakeman. `bundler-audit` and `importmap audit` hit the network | +| docs CSS build | `cd docs && bun run build:css` | wraps `bin/build-css`; needs `bun` and `bundle show` to resolve daisyui, docs-kit and phlex-forms | +| run the app | `cd docs && bin/dev` | execs `bin/rails server` only. It does **not** read `Procfile.dev`, so start `bun run watch:css` in a second shell or the CSS goes stale | +| release | `bin/release [patch\|minor\|major\|X.Y.Z]` (`list`, `--dry-run`, `--force`) | only from `main`, only with a clean tree; wraps `rake release[X.Y.Z]` | + +## Branches and PRs + +- Default branch: `main`. Remote: `zoolutions/phlex-forms` (public). The `.claude/commands/*` still name `mhenrixon/phlex-forms` — stale, the repo moved. +- Work branches root off fresh `origin/main`. History shows `fix/*`, `chore/*`, + `feature/*` and `issue-<n>-<slug>`; `.claude/rules/git-workflow.md` lists + `feature/`, `fix/`, `refactor/`, `ci/`, `chore/`. +- Commits: conventional, with a scope where it helps + (`fix(file_input): …`, `chore(deploy): …`); the body says why. The scope list + in `.claude/rules/git-workflow.md` (`shell`, `sidebar`, `page`, `registry`, …) + is inherited from a docs-site repo and does not match this one — use the layer + or component name instead. +- PR body sections, in order: Summary, Test plan, Deviations & judgment calls, + Gate. The Summary must carry a closing keyword per resolved issue — + `Closes #9, closes #10` — because squash-merge can drop a keyword that lives + only in a commit body. +- Merge policy: squash on `main` after green and approval (every merge commit on + `main` carries a `(#N)` suffix). Releases are the one exception: `rake release` + commits `chore: bump version to X.Y.Z` directly on `main`. +- Never rebase a branch someone else could have pulled; merge `main` forward + into it instead. `/lode:finish-prs` is the one place a rebase plus + `--force-with-lease` is used, in a worktree, on a branch only its author has. +- Attribution: no `Co-Authored-By: Claude`, no "Generated with" line. + +## Layers + +| Layer | Files | Edit rule | +|---|---|---| +| Builder API | `lib/phlex_forms/builder.rb`, `lib/forms/form.rb`, `lib/forms/base.rb`, `lib/forms/fields_for_builder.rb` | owned here; the `field` signature and the ten PascalCase escape hatches are public API — additive only | +| Per-field context | `lib/forms/field.rb`, `lib/forms/live/field.rb` | owned here; every leaf is built from `field_attributes`, never by hand | +| Leaf components | `lib/forms/*.rb` and `lib/forms/plain/*.rb` | owned here; a daisy leaf needs a Plain subclass and both `Theme` maps (`lode/theming/summary.md`) | +| Inference | `lib/phlex_forms/inference.rb` | owned here; a new rule slots into the precedence chain, behind `respond_to?` | +| Theme / merge | `lib/phlex_forms/theme.rb`, `class_merge.rb`, `delegated_field.rb` | owned here | +| Config + boot | `lib/phlex_forms/configuration.rb`, `lib/phlex_forms.rb`, `lib/phlex_forms/engine.rb` | owned here; a new knob needs a default that preserves today's behaviour | +| Stimulus controllers | `app/javascript/phlex_forms/**` | owned here, untested — no JS runner in the repo. Identifiers derive from `Introspector::CONTROLLER_PREFIX`; the file path must match | +| Cops | `lib/rubocop/phlex_forms/**`, `lib/phlex_forms/rubocop.rb`, `config/rubocop.yml` | owned here; opt-in for host apps, Zeitwerk-ignored | +| Docs site | `docs/**` | owned here, separate bundle and RuboCop. Generated: `docs/app/assets/stylesheets/tailwind.sources.css` — edit `bin/build-css`, regenerate; never hand-edit | +| Lockfiles | `docs/Gemfile.lock`, `docs/bun.lock` | committed; regenerate, never hand-merge. The gem's own `Gemfile.lock` is gitignored | + +## Shapes + +Every change is checked against these before it is called done. + +- **A PORO / Struct model** with no `validators_on`, `type_for_attribute`, + `defined_enums` or `reflect_on_association` — must degrade to the + attribute-name map, never `NoMethodError`. +- **An ActiveModel form object** (`build_model` in the specs) — attributes and + validators, no columns, no associations. +- **A `belongs_to` rendered under its foreign key** — the input is + `country_id`, the errors live on `country`. +- **A `Hash`-backed nested scope** (a JSONB column) versus a genuine collection + — `fields_for` must not index the Hash. +- **Both themes.** Any leaf change is rendered under `:daisy` and `:plain`. +- **Both soft dependencies absent.** The gem must boot and render with neither + `daisyui` nor `phlex-reactive` — that is what the Zeitwerk `ignore` list and + `Theme.reactive_roles` protect. +- **A caller who passed the option explicitly** — `as:`, `choices:`, + `required:`, `class:`, `multiple:`, a positional modifier: the caller wins + over anything inferred. +- **`PhlexForms.config.infer_from_model = false`** — model inference off, name + map still on. +- **Ruby 3.4 and Ruby 4.0**, the two CI cells. +- **A field with no model** (`Form(scope: false)`, a bare kit helper) — + `field_value` returns nil rather than raising. + +## Constraints + +Reviewer suggestions that are wrong in this repository. + +| Suggestion | Why it is wrong here | +|---|---| +| "Build the class name from the variant" (`"input-#{size}"`) | a host's Tailwind scanner reads the gem's `.rb` files; an interpolated name is invisible and the style never ships | +| "Add `tailwind_merge` for class conflicts" | `PhlexForms::ClassMerge` covers the only three families form fields produce, and the gemspec's four runtime deps are deliberate | +| "Make `daisyui` / `phlex-reactive` a real dependency" | both are soft on purpose; `Theme.plain` and `FeatureUnavailable` are the designed fallbacks | +| "Reference `Forms::Input` directly here" | leaves resolve through `theme[role]` so a theme swap works; a direct reference pins the daisy leaf | +| "Rescue and return nil so it can't blow up" | the introspection paths already rescue to a safe default; a new bare rescue hides the bug instead | +| "Use `glyphs` as the default icon renderer" | rails_icons resolves SVGs from the host's asset tree, which a minimal host has not set up — the inline SVG default is why the gem renders anywhere | +| "Give the checkbox group a `required` attribute" | one array name shared by many boxes; the browser cannot satisfy it. Validate server-side | +| "Run `cd docs && bundle install` to fix the lockfile pin" | `docs/Gemfile.lock`'s broad `PLATFORMS` list makes a full re-resolve fail on most machines; `rake release` string-edits only the `phlex-forms` pin for exactly this reason | +| "Hand-edit `tailwind.sources.css`" | it is generated output; see `lode/review/docs-site.md` | + +## Docs + +- User-facing docs live in `docs/app/views/docs/pages/` (16 pages, registered in + `docs/app/models/doc.rb`). The behaviour → page map is in + `lode/docs-site/summary.md`; the authoring contract is `docs/AGENTS.md` and + `docs/.claude/skills/write-docs-page/SKILL.md`. +- `README.md` (461 lines) documents the same surface and drifts independently — + a user-facing change updates it too. +- Changelog: `CHANGELOG.md`, Keep a Changelog. Entries go under the existing + `### Added` / `### Changed` / `### Fixed` subsection of the target release + heading — never a second subsection of the same name + (`lode/review/changelog.md`). +- A change to the `field` API, a theme role, an inference rule, a config knob or + either cop updates its docs page **and** the changelog in the same PR. +- Files that pin a version and drift after a release: `docs/Gemfile.lock` pins + `phlex-forms (X.Y.Z)` twice. `rake release` bumps it; if it ever drifts, + string-edit those two lines rather than re-resolving the bundle. + +## CI + +- Workflows: `.github/workflows/main.yml` (push to `main` + every PR: `Lint` on + Ruby 4.0, `Specs (Ruby 3.4)` and `Specs (Ruby 4.0)`, `fail-fast: false`); + `release.yml` (on a published release: test → build → publish-rubygems → + upload-release-assets); `deploy-docs.yml` (on a published release or manual + dispatch, calls docs-kit's reusable `deploy.yml@main`). +- Matrix: Ruby only, `3.4` and `4.0`. A cell differs from a local run only by + Ruby version — `bundler-cache: true`, nothing else installed. +- Fetch a failure: `gh pr checks <PR>`, then + `gh run view <RUN_ID> --job=<JOB_ID> --log-failed`. +- "Green" means all three `main.yml` jobs. `Deploy docs` only runs on a release, + so it is never a PR gate. +- Known not-this-branch failures: none recurring. `Deploy docs` can fail for + environmental reasons (a missing `docs` environment secret, a registry push) + without anything being wrong with the branch. +- Shared or rate-limited services the checks hit: none. The gem suite is fully + in-process, so PRs can run concurrently. + +## Flake sources + +- **None observed in the gem suite.** No network, no database, no clock, no + filesystem writes, no sleeps — 198 in-process examples. +- The two real non-determinism surfaces, if one ever appears: + - **Random order plus global state.** `config.order = :random` with a seeded + `srand`, and the only automatic cleanup is + `config.after { PhlexForms.reset_configuration! }`. A spec that mutates + anything else global — `I18n.locale`, a `Theme` constant, a stubbed + `Phlex::Reactive.verifier` — leaks in seed order. Reproduce with the failing + run's `--seed`. + - **`defined?(Phlex::Reactive)` guards.** Whole files of live and tag-field + specs are skipped when phlex-reactive is not in the bundle, so a "passing" + run can have exercised less than another. Check the example count before + trusting a green run. +- `docs/bin/ci` does hit the network (`bundler-audit`, `bin/importmap audit`); a + failure there can be an advisory-database update rather than a code change. + +## Conflicts + +| File | Rule | +|---|---| +| `CHANGELOG.md` | union under **one** heading per category: keep both sides' bullets, drop the duplicate `### Added`/`### Fixed`/`### Changed` subhead. Losing either side's entry is a real regression (`lode/review/changelog.md`) | +| `docs/Gemfile.lock` | never hand-merge. Take the base's file — it carries the released `phlex-forms` pin. If the branch genuinely changed docs dependencies, edit `docs/Gemfile` and re-run `cd docs && bundle install`; if that fails on the `PLATFORMS` list, stop and ask | +| `docs/bun.lock` | take the base's file; if the branch changed `docs/package.json`, re-run `cd docs && bun install` | +| `lib/phlex_forms/version.rb` | releases land directly on `main`, so a feature branch normally never touches it. A conflict here means the branch bumped it deliberately — keep the branch's bump, or ask if the intent is not in its commits | +| `docs/app/models/doc.rb` | append-only registry: keep both `page` lines, base order first | +| `docs/app/assets/stylesheets/tailwind.sources.css` | generated. Take either side, then regenerate with `cd docs && bin/build-css` and verify the three `@source` paths are real | +| `lib/phlex_forms/theme.rb` | both sides usually added a role: keep both entries in **both** the `daisy` and `plain` maps, or the plain path raises `KeyError` at request time | +| the gem's `Gemfile.lock` | cannot conflict — it is gitignored | + +## Verification + +- The manual check a user of this change would do: `cd docs && bin/dev`, open the + page under `docs/app/views/docs/pages/` that documents the changed behaviour, + and read the rendered markup — the docs site renders the real components + against the real gem through `path: ".."`. For a theme change, look at the + page under both a light and a dark daisyUI theme via the switcher. +- For a change with no docs page, the check is a spec that renders through + `render_form` and asserts on the emitted attribute, plus reading the HTML the + spec produced. +- Stress iterations for a flake proof: 50 runs of the one file with a fresh + seed each (`bundle exec rspec <file>` in a loop) — the suite is fast enough + that a real order-dependence shows well inside that. +- Where evidence goes: `lode/tmp/` (gitignored, never committed) unless the PR + needs an auditable trail. `implementation-notes.md` at the repo root is the + deviation log during a run; its contents move into the PR body's + "Deviations & judgment calls" section and the file is then deleted, never + committed. From 06dae0e5da26b2b325b2310ad4616f497b5a744b Mon Sep 17 00:00:00 2001 From: mhenrixon <mikael@mhenrixon.com> Date: Mon, 14 Sep 2026 01:15:37 +0200 Subject: [PATCH 2/2] =?UTF-8?q?docs(lode):=20gate=20findings=20=E2=80=94?= =?UTF-8?q?=20the=20real=20example=20count,=20and=20the=20routes=20note?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rspec --dry-run reports 202 examples; the grep-based count of 198 missed the five examples one each loop builds in components_spec.rb, so that file's row and the three suite totals are corrected. The retired github-review-pr command's note that docs/config/routes.rb never conflicts per page is restored to the Conflicts table. Claude-Session: https://claude.ai/code/session_01Hga3idYBp7fWdj7GmmfofF --- lode/testing/summary.md | 2 +- lode/workflow.md | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lode/testing/summary.md b/lode/testing/summary.md index 441542a..2c34800 100644 --- a/lode/testing/summary.md +++ b/lode/testing/summary.md @@ -1,6 +1,6 @@ # Testing: layers, helpers, and what is and is not covered -RSpec, 18 spec files, **198 examples** (`grep -rhE '^\s*it ["(]' spec +RSpec, 18 spec files, **202 examples** (`bundle exec rspec --dry-run`; a line-based `grep` for `it` says 198 because `spec/forms/components_spec.rb` builds five examples from one `each` loop) (`grep -rhE '^\s*it ["(]' spec --include='*_spec.rb' | wc -l`). `.rspec` loads `spec_helper` for every file and prints documentation format; the suite runs in random order with a seeded `srand`. diff --git a/lode/workflow.md b/lode/workflow.md index 9f72f4e..285132c 100644 --- a/lode/workflow.md +++ b/lode/workflow.md @@ -10,7 +10,7 @@ rest of `lode/`. | Purpose | Command | Notes | |---|---|---| | fast loop (one file) | `bundle exec rspec spec/<path>_spec.rb` | no network, no database, no services; a unit file runs in well under a second | -| full suite | `bundle exec rspec` | 198 examples, all in-process. Safe to run in two worktrees at once — nothing shared, no ports, no fixture dirs | +| full suite | `bundle exec rspec` | 202 examples, all in-process. Safe to run in two worktrees at once — nothing shared, no ports, no fixture dirs | | lint | `bundle exec rubocop lib spec` | the CI Lint job's exact invocation, and the only one that works. `bundle exec rubocop -A lib spec` to autocorrect. The two paths are deliberate: `Rakefile`, `bin/` and `*.gemspec` are not linted | | both, as CI sees them | `bundle exec rspec && bundle exec rubocop lib spec` | **not** `bundle exec rake`. The default task is `spec` then `rubocop`, and `RuboCop::RakeTask` passes no paths, so RuboCop descends into `docs/`, loads `docs/.rubocop.yml` and dies on `cannot load such file -- docs_kit/rubocop` — that gem is in the docs bundle, not the root one. `bundle exec rake` therefore always fails at the lint step, whatever the code says. `CLAUDE.md` and `.claude/rules/git-workflow.md` both still name it as the pre-commit gate | | one CI cell locally | n/a | the matrix is only Ruby 3.4 and 4.0 running the same two commands; `.tool-versions` pins 4.0.5 | @@ -139,7 +139,7 @@ Reviewer suggestions that are wrong in this repository. ## Flake sources - **None observed in the gem suite.** No network, no database, no clock, no - filesystem writes, no sleeps — 198 in-process examples. + filesystem writes, no sleeps — 202 in-process examples. - The two real non-determinism surfaces, if one ever appears: - **Random order plus global state.** `config.order = :random` with a seeded `srand`, and the only automatic cleanup is @@ -162,7 +162,7 @@ Reviewer suggestions that are wrong in this repository. | `docs/Gemfile.lock` | never hand-merge. Take the base's file — it carries the released `phlex-forms` pin. If the branch genuinely changed docs dependencies, edit `docs/Gemfile` and re-run `cd docs && bundle install`; if that fails on the `PLATFORMS` list, stop and ask | | `docs/bun.lock` | take the base's file; if the branch changed `docs/package.json`, re-run `cd docs && bun install` | | `lib/phlex_forms/version.rb` | releases land directly on `main`, so a feature branch normally never touches it. A conflict here means the branch bumped it deliberately — keep the branch's bump, or ask if the intent is not in its commits | -| `docs/app/models/doc.rb` | append-only registry: keep both `page` lines, base order first | +| `docs/app/models/doc.rb` | append-only registry: keep both `page` lines, base order first. `docs/config/routes.rb` is docs-kit boilerplate serving every page through one `docs/:doc` route, so it does not conflict per page | | `docs/app/assets/stylesheets/tailwind.sources.css` | generated. Take either side, then regenerate with `cd docs && bin/build-css` and verify the three `@source` paths are real | | `lib/phlex_forms/theme.rb` | both sides usually added a role: keep both entries in **both** the `daisy` and `plain` maps, or the plain path raises `KeyError` at request time | | the gem's `Gemfile.lock` | cannot conflict — it is gitignored |