diff --git a/.claude/skills/ssr-form-migration/SKILL.md b/.claude/skills/ssr-form-migration/SKILL.md new file mode 100644 index 00000000..2588b9fe --- /dev/null +++ b/.claude/skills/ssr-form-migration/SKILL.md @@ -0,0 +1,122 @@ +--- +name: ssr-form-migration +description: Migrate an SSR form or wizard modal to the whole-form HTMX swap doctrine, top-rooted render functions, the data-driven session-backed wizard engine, and (where it helps) Selmer templates. Use when simplifying a server-rendered form/wizard modal in this codebase, removing EDN-snapshot round-trips, deleting *-no-cursor* duplicate render fns, collapsing per-interaction routes, or replacing the multi-step wizard protocol machinery. +--- + +# SSR Form & Wizard Migration + +A repeatable method for making a server-rendered form/wizard modal **simpler** without +changing user-facing behavior. Distilled from the first proven migration — the +`transaction/edit.clj` modal, which already runs on the whole-form `hx-select` swap +approach with **zero out-of-band swaps**. Every migration *reads this skill first* and +*extends it last* (the Growth contract below). If migration N+1 is not easier than N, +the skill-update step was skipped — treat that as a bug. + +The four patterns every migration moves code toward live in `reference/`: + +- `reference/swap-doctrine.md` — the four-rule HTMX swap priority order + the focus + invariant + Alpine-survives-swap hardening + target-selector strategy. +- `reference/render-functions.md` — one render fn per component, taking explicit data + **or a top-rooted cursor**; no faked cursor positions, no `*-no-cursor*` twins. +- `reference/form-vs-wizard.md` — single-step → plain form; multi-step → data-driven + engine with **per-step state in the Ring session** (the Django `formtools` model). +- `reference/selmer-conventions.md` — plain-HTML attributes via Selmer, the + Hiccup↔Selmer interop bridge, include/block patterns. + +Growing cookbooks (append every migration): +`component-cookbook.md`, `gotchas.md`, `test-recipes.md`, `scorecard.md`. + +--- + +## The per-migration playbook + +Run this loop for each modal. The phase notes in the migration plan list only what is +*specific* to a modal; this loop is the constant. + +1. **Read the skill.** Skim `reference/` and note which `component-cookbook.md` + entries and `gotchas.md` you can reuse. Start from the cookbook, not a blank file. + +2. **Classify** (`reference/form-vs-wizard.md`). + - Single logical step (even with a `?mode=` toggle or add/remove rows) → **plain + form**: no server-side wizard state, no snapshot, no protocol. + - Genuinely multiple steps the user advances through → **wizard**: the data-driven + engine + per-step session storage. + - When in doubt, it's a form. + +3. **Baseline the scorecard** (`scorecard.md`, heuristics in §6 of the plan). Record + before-numbers with cheap tools: + ```bash + F=src/clj/auto_ap/ssr/.clj + wc -l $F # LOC (heuristic 4) + grep -c 'defn.*-no-cursor' $F # *-no-cursor* twins (heuristic 1) + grep -cE 'with-cursor|MapCursor\.' $F # faked cursor re-roots (heuristic 1) + grep -c 'hx-swap-oob' $F # OOB swaps (heuristic 7) + grep -cE '"hx-[a-z]' $F # mixed string hx- attrs (heuristic 8) + # route count: count this modal's entries in src/cljc/auto_ap/routes/*.cljc + ``` + +4. **Characterize behavior (test-first).** Write or confirm a Playwright spec that + captures *current* behavior before you touch anything — focus/caret survival across + swaps, each field round-trip, validation errors, and the real save. This spec is the + parity contract; it must stay green through every commit. See `test-recipes.md`. + +5. **Consolidate render functions** (`reference/render-functions.md`). Make each render + fn take explicit data or a **top-rooted cursor**. Delete `*-no-cursor*` duplicates + and any `with-cursor`/`MapCursor` rebind that fakes a deep starting position + (heuristics 1, 2). Using a cursor is fine; faking where it *starts* is not. + +6. **Templatize in Selmer** (`reference/selmer-conventions.md`) where the component is + interactive/attribute-heavy. Reuse cookbook bits; add new ones back (heuristics 5, 8). + Static markup may stay Hiccup — Selmer scope is hybrid (Open decision 2). + +7. **Wire HTMX per the swap doctrine** (`reference/swap-doctrine.md`). Keep the focus + invariant intact. No OOB except a genuinely disjoint region, documented (heuristic 7). + +8. **Collapse routes** to 2 (`GET` open, `POST` submit) — `+1` for an add-row endpoint, + `+1` for the single `*-form-changed` whole-form re-render endpoint (heuristic 6). + +9. **Verify.** Modal e2e green + full e2e suite at-or-above baseline; assert DB + mutations by querying Datomic, not markup; REPL-check the pure render/data fns. + Re-measure the scorecard — **no metric may regress for the touched modal** without a + written exception in `gotchas.md`. + +10. **Commit** one reversible feature commit. The message includes the scorecard delta + and the reused/new cookbook entries. + +11. **Feed the skill** (the Growth contract). *Not optional.* + +--- + +## Growth contract — the last task of every migration + +- Converted a component? → add its before/after to `component-cookbook.md`. +- Hit a surprise? → one entry in `gotchas.md`. +- Found a test pattern? → `test-recipes.md`. +- Playbook step missing or wrong? → fix this `SKILL.md`. +- Measured the scorecard? → append the row to `scorecard.md`. + +**Success signal:** each migration reuses more cookbook entries and starts from a better +scorecard baseline than the previous one. + +--- + +## Non-negotiables + +- **Focus invariant:** the input the user is typing in is *never* inside the region its + own request swaps. Violating this drops the caret. (Proven by the + `transaction-edit-swap.spec.ts` caret tests.) +- **No new OOB swaps.** If tempted to OOB something inside the same feature, restructure + the DOM so the dependent element shares an ancestor with the trigger and use an + ordinary swap (e.g. totals in a sibling ``). +- **Behavior parity is proven by tests, not by reading.** The full e2e suite stays green + after every migration. +- **Don't game the heuristics.** They're directional evidence paired with the e2e parity + gate; review the trend, not single numbers. + +## Project conventions that bite (see `gotchas.md`) + +- Edit Clojure with the clojure-mcp tools (`clojure_edit`, `clojure_edit_replace_sexp`), + not the raw file editor. `clj-paren-repair` then `lein cljfmt fix` when a file won't + compile. +- Run tests via the `clojure-eval` skill / `clj-nrepl-eval -p PORT`, not `lein test`. +- Temp files go in `./tmp/`. diff --git a/.claude/skills/ssr-form-migration/reference/component-cookbook.md b/.claude/skills/ssr-form-migration/reference/component-cookbook.md new file mode 100644 index 00000000..36549f26 --- /dev/null +++ b/.claude/skills/ssr-form-migration/reference/component-cookbook.md @@ -0,0 +1,103 @@ +# Component cookbook + +GROWS every migration. Each entry: what it is, the swap rule it uses, and the canonical +snippet. Reuse these before writing anything new; the success signal is *more reuse each +migration*. + +Seeded from `transaction/edit.clj` (Hiccup form — Selmer versions land in Phase 2). + +--- + +## typeahead (account / vendor) — Alpine + tippy, survives swaps + +Used for account and vendor selection. Click-to-select (not a live text caret), so a +whole-form swap on change is safe. Null-guard `tippy?`/`$refs.input?`. + +```clojure +(defn account-typeahead* [{:keys [name value client-id x-model]}] + [:div.flex.flex-col + (com/typeahead {:name name + :placeholder "Search..." + :url (hu/url (bidi/path-for ssr-routes/only-routes :account-search) + (cond-> {:purpose "transaction"} client-id (assoc :client-id client-id))) + :id name + :x-model x-model ; binds selected value into the row's Alpine scope + :value value + :content-fn (fn [v] (:account/name (d-accounts/clientize ... v client-id)))})]) +``` +Reuse note: `:x-model` lets the *parent* row read the selected id (e.g. `accountId`) to +gate a targeted location swap. See account-row. + +## account-row — cursor render fn + per-row targeted location swap + whole-form remove + +The canonical "row in a repeated grid" pattern. One render fn, top-rooted cursor. +- account typeahead binds `accountId` into row Alpine scope; +- **location cell** swaps *only itself* (`#account-location-`) on `changed` + (swap-doctrine Rule 2); +- **amount cell** swaps *only* `#account-totals` (Rule 4, sibling tbody); +- **remove** swaps the whole form (Rule 3). + +```clojure +(defn transaction-account-row* [{:keys [value client-id amount-mode index]}] + (com/data-grid-row + (-> {:class "account-row" :id (str "account-row-" index) + :x-data (hx/json {:show ... :accountId (fc/field-value (:transaction-account/account value))}) + :data-key "show" :x-ref "p"} + hx/alpine-mount-then-appear) + (fc/with-field :db/id (com/hidden {:name (fc/field-name) :value (fc/field-value)})) + (fc/with-field :transaction-account/account + (com/data-grid-cell {} (com/validated-field {:errors (fc/field-errors)} + (account-typeahead* {:value (fc/field-value) :client-id client-id + :name (fc/field-name) :x-model "accountId"})))) + (fc/with-field :transaction-account/location + (com/data-grid-cell {:id (str "account-location-" index)} ...Rule 2 targeted swap...)) + (fc/with-field :transaction-account/amount + (com/data-grid-cell {} ...Rule 4 totals swap...)) + (com/data-grid-cell {:class "align-top"} ...Rule 3 whole-form remove...))) +``` +TODO Phase 2: drop the `transaction-account-row-no-cursor*` twin; this is the only kept form. + +## totals in a sibling `` — Rule 4 instead of OOB + +Running totals live in their own ``, a sibling of the +input-bearing rows, so an amount edit refreshes them with a plain targeted swap and never +replaces the amount input (caret survives). + +```clojure +(com/data-grid + {:footer-tbody + [:tbody {:id "account-totals"} + (com/data-grid-row {:class "account-total-row"} ... (account-total* request) ...) + (com/data-grid-row {:class "account-balance-row"} ... (account-balance* request) ...)]} + ...input rows...) +``` + +## money-input / text-input amount field — Rule 4 targeted totals swap + +```clojure +(com/money-input + {:name (fc/field-name) :id (str "account-amount-" index) :class "w-16 account-amount-field" + :hx-post (bidi/path-for ssr-routes/only-routes ::route/edit-form-changed) + :hx-target "#account-totals" :hx-select "#account-totals" :hx-swap "outerHTML" + :hx-trigger "keyup changed delay:300ms" :hx-include "closest form"}) +``` +`%` mode swaps to `com/text-input {:type "number" :step "0.01"}` with the same swap attrs. + +## memo field — Rule 1, no request + +```clojure +(com/text-input {:value (fc/field-value) :name (fc/field-name) :id "edit-memo" + :placeholder "Optional note"}) ; no hx-* — rides along to save +``` + +## mode toggle ($/% radio, simple/advanced link) — Rule 3, whole-form swap + +```clojure +(com/radio-card {:options [{:value "$" :content "$"} {:value "%" :content "%"}] + :value amount-mode :name "step-params[amount-mode]" + :hx-post (bidi/path-for ssr-routes/only-routes ::route/toggle-amount-mode) + :hx-target "#wizard-form" :hx-select "#wizard-form" :hx-swap "outerHTML" + :hx-include "closest form"}) +``` +TODO Phase 2: the simple/advanced toggle becomes a `?mode=` re-render (plain form), not a +dedicated route. diff --git a/.claude/skills/ssr-form-migration/reference/form-vs-wizard.md b/.claude/skills/ssr-form-migration/reference/form-vs-wizard.md new file mode 100644 index 00000000..b4228144 --- /dev/null +++ b/.claude/skills/ssr-form-migration/reference/form-vs-wizard.md @@ -0,0 +1,115 @@ +# Forms vs. wizards (and the data-driven wizard engine) + +## Classify first + +| Signal | Classification | +|--------|----------------| +| One logical step — even with a `?mode=` toggle, $/% radio, or add/remove rows | **plain form** | +| The user genuinely advances through ordered steps, each validated before the next | **wizard** | +| In doubt | **form** | + +Most "wizards" in this codebase are single-step forms wearing wizard costumes: they +implement the multi-step protocol (`mm/ModalWizardStep` + friends), serialize an EDN +snapshot into hidden fields, and register 10–20 stacked-middleware routes — all for one +step. That is pure overhead to delete. + +## The machinery being replaced + +`transaction/edit.clj` today still carries the old shape, useful as the "before": + +```clojure +(defrecord LinksStep [linear-wizard] + mm/ModalWizardStep + (step-name [_] "Transaction Actions") + (step-key [_] :links) + (edit-path [_ _] []) + (step-schema [_] (mm/form-schema linear-wizard)) + (render-step [this {{:keys [snapshot step-params]} :multi-form-state :as request}] ...)) +``` + +…plus the snapshot round-trip: the whole accumulating form state is serialized to hidden +fields (custom EDN readers), then rebuilt every request by merging the posted pieces back +into the snapshot (`:multi-form-state :snapshot` is read ~75× in `edit.clj`). The +serialization needs custom readers, the merge is error-prone, and the payload grows each +step. + +--- + +## Single-step → plain form + +Two routes: `GET` (render) and `POST` (validate + save). State is plain form fields + an +entity id. No snapshot, no server state, no protocol. + +```clojure +{::route/edit (fn [req] (html-response (render-edit-form {:entity (get-entity req)}))) + ::route/edit-submit (fn [req] (validate-and-save req))} +``` + +A `?mode=` toggle is just the `GET` re-rendering with a different query param — still a +plain form. An add-row interaction is one extra `POST` that appends a fresh row and +re-renders (the `+1` route). + +--- + +## Genuinely multi-step → data-driven engine with session-stored step state + +> **Inspiration — Django `formtools` `WizardView`.** Django's wizard does *not* round-trip +> a serialized blob of the whole form through the page. Each step's validated data is +> written to a **storage backend (the user session by default)** under that step's key, +> and the steps are combined only at the very end via `get_all_cleaned_data()`. We adopt +> the same model: **replace the EDN snapshot + piecewise merging with per-step form state +> stored in the Ring session.** A step writes its own data under its own key; nothing is +> merged into a snapshot and nothing about other steps rides through the form. +> Refs: `formtools.wizard.views.WizardView`, `SessionStorage`, `get_all_cleaned_data()` +> (https://django-formtools.readthedocs.io/en/latest/wizard.html). + +A wizard is **data**: + +```clojure +(def vendor-wizard-config + {:steps [{:key :info :schema info-schema :fields [...] :render render-info-step + :next (fn [data] :terms)} + {:key :terms :schema terms-schema :fields [...] :render render-terms-step + :next (fn [data] :done)}] + :init-fn (fn [req] {...}) + :submit-route "/admin/vendor/wizard/submit" + :done-fn (fn [all-data req] (save! all-data) (html-response "Saved"))}) +``` + +with a tiny engine (no protocols) whose state lives **in the session**, keyed by a wizard +instance id, each step's data under its own step key — the formtools `SessionStorage` +model. No snapshot, no custom EDN readers, no merge-into-snapshot: + +```clojure +;; Storage backed by the Ring session. Path: [:wizards :step-data ] +(defn create-wizard! [session config] + (let [id (str (java.util.UUID/randomUUID))] + [id (assoc-in session [:wizards id] + {:current-step (-> config :steps first :key) :step-data {}})])) + +(defn put-step [session id k data] (assoc-in session [:wizards id :step-data k] data)) ; replace, not merge +(defn set-step [session id k] (assoc-in session [:wizards id :current-step] k)) +(defn get-all [session id] (->> (get-in session [:wizards id :step-data]) vals (apply merge))) +(defn forget [session id] (update session :wizards dissoc id)) +``` + +The render emits only a **reference token** (`wizard-id`, `current-step`) in the form — +never the form's state. The submit handler validates the posted step, `put-step`s it, +computes `:next`, and either advances (`set-step`) or finishes (`get-all` + `:done-fn` + +`forget`). Every fn returns the updated session for the handler to thread into the Ring +response (`(assoc resp :session session')`). + +**Two routes per wizard:** open (`partial open-wizard config`) and submit +(`partial handle-step-submit config`). State is namespaced by `wizard-id` inside the +session, so multiple in-flight wizards (and browser tabs) don't collide, and it is +discarded on completion (`forget`). + +### Storage lifetime (Open decision 1) + +State lives in the Ring session, scoped to true multi-step wizards (plain forms hold +none). Lifetime follows the session; `forget` on completion prevents session bloat. For +long-lived wizards, confirm the session backend (in-memory vs. durable) is acceptable or +pick a durable store. **This engine is built in Phase 6** (Transaction Rule) — until then +this file describes the target; validate `components/wizard_state.clj` + +`components/wizard2.clj` against it when they land, and update this doc from the real +implementation. diff --git a/.claude/skills/ssr-form-migration/reference/gotchas.md b/.claude/skills/ssr-form-migration/reference/gotchas.md new file mode 100644 index 00000000..56846ae2 --- /dev/null +++ b/.claude/skills/ssr-form-migration/reference/gotchas.md @@ -0,0 +1,55 @@ +# Gotchas + +GROWS every migration. One entry per surprise. Also the home for any **written exception** +to the scorecard ratchet (a metric that regressed for a documented reason). + +--- + +## Stale `$refs` / `tippy` after a swap + +A whole-form swap can run an Alpine event handler *before* the component re-initialises, +so a handler that dereferences `$refs.input.__x_tippy` or calls `tippy.show()` throws. +**Always null-guard:** `$refs.input?.__x_tippy?.hide()`, `tippy?.show()`. The +`transaction-edit-swap.spec.ts` `trackErrors()` helper fails the test on any `pageerror` +or `console.error`, which is exactly how a stale-ref throw surfaces. + +## Let the server value win — don't preserve Alpine state across a server-driven change + +When a server change should update a component (e.g. choosing a vendor sets its default +account), rebuild that section fresh on the swap so the server-provided value lands +without keying tricks. The bug this prevents: "changing the vendor a *second* time doesn't +update the account" because preserved Alpine state shadowed the new server value. If you +*must* preserve a component, key it by value so a change forces re-init: +`(assoc attrs :key (str id "--" current-value))`. + +## Focus dies if the typed input is inside its own swapped region + +The single most important invariant. Amount field → swap a sibling tbody, not the row. +Memo → swap nothing. If a caret test (`sameNode`) fails, the input is in its own swap +region — re-target to a sibling/ancestor that excludes it. + +## Faked cursors breed duplicate render fns + +A `with-cursor`/`MapCursor` re-root to fake a deep start forces a `*-no-cursor*` twin. +Removing the fake lets you delete the twin. Don't "fix" a faked cursor in place — top-root +it and collapse to one render fn. (See `render-functions.md`.) + +## Edit Clojure with clojure-mcp tools, not the file editor + +`clojure_edit` / `clojure_edit_replace_sexp`. If a file won't compile: `clj-paren-repair` +the file, then retry; if still broken, `lein cljfmt check`. Run tests via `clojure-eval` / +`clj-nrepl-eval -p PORT`, never `lein test` (slow, last resort). + +## Solr/typeahead in tests + +Account/vendor search is backed by Solr, unavailable in tests. To drive a typeahead in +e2e: type under the 3-char threshold, then inject a result into Alpine state +(`Alpine.$data(el).elements = [{value, label}]`) and click it — the real click handler, +`tippy.hide()`, Alpine reactivity, and the HTMX swap all run as in production. Entity ids +come from `GET /test-info`. + +--- + +## Scorecard exceptions (ratchet violations with a reason) + +_None yet._ Append here if a migration must let a metric regress for a documented reason. diff --git a/.claude/skills/ssr-form-migration/reference/render-functions.md b/.claude/skills/ssr-form-migration/reference/render-functions.md new file mode 100644 index 00000000..ad261809 --- /dev/null +++ b/.claude/skills/ssr-form-migration/reference/render-functions.md @@ -0,0 +1,85 @@ +# Render functions: explicit data, or a top-rooted cursor + +**One function, data in, markup out.** The data can arrive as a plain map *or* via a +cursor — as long as the cursor was rooted at the top of the form and walked down to here, +never faked to start at this depth. The rule is about *where the cursor starts*, not +whether you use one. + +## GOOD — explicit data, pure, testable without setup + +```clojure +(defn account-row [{:keys [account index client-id amount-mode]}] + (com/data-grid-row + (com/hidden {:name (str "accounts[" index "][db/id]") + :value (or (:db/id account) "")}) + (com/data-grid-cell + (account-typeahead* {:value (:transaction-account/account account) + :name (str "accounts[" index "][account]") + :client-id client-id})) + ...)) +``` + +## ALSO FINE — a cursor that started at the form root and was advanced naturally + +```clojure +;; The top-level render walks the cursor; the row fn receives the dereferenced row +;; (or the advanced cursor). No rebinding of *current*/*prefix* to fake depth. +(defn account-rows [accounts-cursor] + (for [row-cursor (fc/each accounts-cursor)] ; advanced from the root, not faked + (account-row {:account @row-cursor :index (fc/index row-cursor) ...}))) +``` + +`transaction/edit.clj`'s `transaction-account-row*` is the cursor form done right: the +caller (`account-grid-body*`) holds a top-rooted cursor via `fc/cursor-map` and hands each +row cursor to one render fn. + +--- + +## The SMELL this migration removes + +### 1. Faking the cursor's starting position + +A "form cursor" is fine. The pain is **rebinding the dynamic root deeper in the tree** so +a deeply nested render fn can run against a fragment. Real example from +`transaction/edit.clj`'s `simple-mode-fields*` (the thing to delete): + +```clojure +;; SMELL: re-roots the cursor to a synthetic MapCursor pointed at accounts[0] so a +;; fragment can render "deep". Fragile, and the source of the *-no-cursor* twin below. +(fc/with-field :transaction/accounts + (fc/with-cursor (let [cur fc/*current*] + (if (sequential? @cur) + (nth cur 0 nil) + (auto_ap.cursor.MapCursor. {} (cursor/state cur) + (conj (cursor/path cur) 0)))) + ...)) +``` + +Target: the cursor begins at the top level of what the form consumes and walks down +naturally. Because the **whole form is re-rendered each time** (swap doctrine), there is +no longer any reason to fake a deep starting position. + +### 2. The `*-no-cursor*` twin + +Faking the deep cursor forces a *second copy of the same markup* — one that reads the +faked cursor and one that takes plain params for the cases where the fake can't be set up. +`transaction/edit.clj` has exactly this pair: + +```clojure +(defn transaction-account-row* [{:keys [value index client-id ...]}] ...) ; cursor form +(defn transaction-account-row-no-cursor* [{:keys [account index client-id ...]}] ...) ; duplicate markup +``` + +**Fix:** keep one render fn. If a caller already holds a top-rooted cursor, advance it and +hand the row data (or the advanced cursor) to that one fn. Delete the `*-no-cursor*` copy. +Heuristic 1 targets `grep -c 'defn.*-no-cursor'` → 0 and faked-cursor re-roots → 0. + +## Scorecard hooks (heuristics 1, 2) + +```bash +grep -c 'defn.*-no-cursor' $F # → 0 +grep -cE 'with-cursor|MapCursor\.' $F # faked re-roots → 0 (top-rooted cursors are fine) +``` + +Top-rooted cursors do **not** count against heuristic 1 — only *re-roots that fake depth* +and the `*-no-cursor*` twins do. diff --git a/.claude/skills/ssr-form-migration/reference/scorecard.md b/.claude/skills/ssr-form-migration/reference/scorecard.md new file mode 100644 index 00000000..d9f1f036 --- /dev/null +++ b/.claude/skills/ssr-form-migration/reference/scorecard.md @@ -0,0 +1,46 @@ +# Quality scorecard (the ratchet) + +Cheap to measure (`grep -c`, `wc -l`, `clj-kondo`), recorded **before/after each +migration** in the commit message and in the results table below. **No metric may regress +for the touched modal** without a written exception in `gotchas.md`. These are directional +evidence, not targets to game — always paired with the e2e parity gate. + +## Heuristics + +| # | Heuristic | Measure | Target | +|---|-----------|---------|--------| +| 1 | Faked cursor positions (not cursors themselves) | `grep -cE 'with-cursor\|MapCursor\.'` re-roots + `grep -c 'defn.*-no-cursor'` | → 0 (top-rooted cursors are fine) | +| 2 | Implicit state merges (snapshot/cursor) | count merge sites | → 0 (forms); explicit `put-step` only (wizards) | +| 3 | Branching complexity | `clj-kondo`, or count `cond`/`condp`/`case`/nested `if` + max depth | net ↓ | +| 4 | Lines of code | `wc -l` on the modal's file(s) | net ↓ | +| 5 | Reuse / cross-form similarity | cookbook components reused; duplicated-block count | reuse ↑, dup ↓ | +| 6 | Route count | count routes for the modal | → 2 (+1 for add-row) | +| 7 | OOB swaps | `grep -c hx-swap-oob` | → 0 unless a justified disjoint-region case is documented | +| 8 | Attribute consistency | mixed `:x-`/`"x-"` encodings in migrated template | → 0 | + +## How to measure (copy/paste) + +```bash +F=src/clj/auto_ap/ssr/.clj +echo "LOC $(wc -l < $F)" +echo "no-cursor twins $(grep -c 'defn.*-no-cursor' $F)" +echo "faked-cursor roots $(grep -cE 'with-cursor|MapCursor\.' $F)" +echo "snapshot merges $(grep -c ':multi-form-state :snapshot' $F)" +echo "branch forms $(grep -cE '\(cond |\(condp |\(case |\(when-not ' $F)" +echo "hx-swap-oob $(grep -c 'hx-swap-oob' $F)" +echo "mixed string hx- $(grep -cE '\"hx-[a-z]' $F)" +# route count: count this modal's entries in src/cljc/auto_ap/routes/*.cljc +``` + +## Results + +Each migration appends one row (after-numbers), referencing the before in the diff. + +| Phase | Modal | LOC | Routes | no-cursor twins | faked roots | snapshot merges | OOB | mixed hx- | cookbook reused / added | +|-------|-------|-----|--------|-----------------|-------------|-----------------|-----|-----------|-------------------------| +| 1 (baseline) | Transaction Edit `transaction/edit.clj` | 1608 | ~12 | 1 | 2 | ~75 | 0 | 8 | — / seeded 7 entries | + +> Phase 1 is distillation only — no app code changed. The Transaction Edit row is the +> **before** baseline that Phase 2 must beat (target: routes → ~3, no-cursor → 0, faked +> roots → 0, snapshot merges → 0, LOC ↓, mixed hx- → 0). The `0` OOB is already achieved +> by the merged reference and must not regress. diff --git a/.claude/skills/ssr-form-migration/reference/selmer-conventions.md b/.claude/skills/ssr-form-migration/reference/selmer-conventions.md new file mode 100644 index 00000000..ac4837b9 --- /dev/null +++ b/.claude/skills/ssr-form-migration/reference/selmer-conventions.md @@ -0,0 +1,68 @@ +# Selmer template conventions + +> **Status: STUB — validated in Phase 2.** This file describes the target. The Selmer +> dependency, render helper, and interop bridge are added in Phase 2 (Transaction Edit); +> rewrite this file from the *real, verified* example once that lands, and record each +> converted component in `component-cookbook.md`. + +## Why Selmer for interactive components + +In Hiccup the same Alpine/HTMX attribute is sometimes a keyword and sometimes a string in +the same file — there's no rule a reader (or an LLM) can rely on: + +```clojure +;; All of these appear in one component today: +:x-ref "input" "x-ref" "hidden" +:x-model "value.value" "x-model" "search" +"@keydown.down.prevent.stop" "tippy.show();" ; handlers MUST be strings +:x-init "..." ; structural attrs are keywords +``` + +In a Selmer template the same markup is unambiguous plain HTML: + +```html +{# templates/components/typeahead.html #} +
+ + + + ... +
+``` + +Note the `tippy?.` null-guard carried over from the swap doctrine — Selmer doesn't change +the Alpine-survives-swap requirement. + +## Render helper + interop bridge (the Phase 2 foundation) + +```clojure +(defn render [tpl ctx] (selmer/render-file tpl ctx)) +(defn hiccup->html [h] (hiccup/html h)) ; embed hiccup inside selmer via {{ frag|safe }} +;; selmer fragment inside hiccup: [:div (hiccup/raw (render "..." ctx))] +``` + +The bridge must work **both ways** during the strangler transition: a Hiccup component +renders inside a Selmer template (pass `(hiccup->html h)` into the context, render with +`|safe`), and a Selmer fragment renders inside a Hiccup tree (`(hiccup/raw (render ...))`). +Prove both in Phase 2 before broad use. + +## Composition + +Selmer composes via `{% include %}` and `{% block %}`. Prefer small per-component +templates that the cookbook references by path. Keep `|safe` to values the server fully +controls (rendered Hiccup, JSON for `x-data`), never raw user input. + +## Scope (Open decision 2) + +Hybrid: convert interactive/attribute-heavy components first; static markup may stay +Hiccup. Revisit a fuller sweep in Phase 11. + +## Attribute-consistency scorecard (heuristic 8) + +```bash +grep -cE '"x-[a-z]|"hx-[a-z]|"@' # → 0 mixed encodings in Selmer +``` +A migrated Selmer template has no mixed `:x-`/`"x-"` encodings because everything is plain +HTML. diff --git a/.claude/skills/ssr-form-migration/reference/swap-doctrine.md b/.claude/skills/ssr-form-migration/reference/swap-doctrine.md new file mode 100644 index 00000000..c250dd93 --- /dev/null +++ b/.claude/skills/ssr-form-migration/reference/swap-doctrine.md @@ -0,0 +1,149 @@ +# Whole-form HTMX swap doctrine + +Every interactive control picks a swap strategy in this **priority order** (prefer the +earliest rule that works). Worked examples are the real `transaction/edit.clj` swaps. + +## Rule 1 — No request when the field affects nothing else + +Its value rides along in the form and is read on submit. No `hx-*` at all. + +```clojure +;; transaction/edit.clj — the memo field. Editing it issues NO request; the value +;; just rides along until save. The e2e proves zero POSTs fire while typing. +(com/text-input {:value (fc/field-value) + :name (fc/field-name) + :id "edit-memo" + :placeholder "Optional note"}) +``` + +## Rule 2 — Targeted swap of a single isolated cell when the effect is purely local + +Give the cell a stable id, keep it **out of the typed input's subtree**, and post the +whole form but `hx-select` back only that cell. + +```clojure +;; transaction/edit.clj — selecting an account only changes that row's valid Location +;; options, so the change swaps just this cell. Nothing else re-renders. +[:div {:id (str "account-location-" index)} ; stable, per-row id + (com/validated-field + {:x-hx-val:account-id "accountId" + :x-dispatch:changed "accountId" ; Alpine fires `changed` when account changes + :hx-trigger "changed" + :hx-post (bidi/path-for ssr-routes/only-routes ::route/edit-form-changed) + :hx-target (str "#account-location-" index) + :hx-select (str "#account-location-" index) + :hx-swap "outerHTML" + :hx-include "closest form"} ; whole form posts; only this cell swaps back + (location-select* {...}))] +``` + +## Rule 3 — Whole-form swap when the change touches interdependent state + +Vendor change, add/remove row, mode toggle, $/% radio. The form's hidden state rides +along, so one swap keeps everything consistent — **no out-of-band swaps**. + +```clojure +;; transaction/edit.clj — vendor change rebuilds the whole manual-coding section +;; (vendor default account, terms, etc. are interdependent). +[:div {:hx-trigger "change" + :hx-post (bidi/path-for ssr-routes/only-routes ::route/edit-vendor-changed) + :hx-target "#wizard-form" + :hx-select "#wizard-form" + :hx-swap "outerHTML" + :hx-sync "this:replace" + :hx-include "closest form"} + ...] +``` + +The active tab/action round-trips through the form (it's a hidden field bound to Alpine +`activeForm`), so it **survives** the whole-form swap — that's why a whole-form swap is +safe here even though the user is "on" a tab. + +## Rule 4 — OOB only for genuinely disjoint DOM regions + +A global flash/toast, a nav badge, a modal at the document root. **If tempted to OOB +something inside the same feature, restructure instead**: give the dependent element a +common ancestor with the trigger and use an ordinary swap. + +Worked example — running **totals live in their own sibling ``** so an amount edit +swaps the totals without ever replacing the amount input: + +```clojure +;; The totals tbody is a sibling of the input-bearing rows. +(com/data-grid + {:footer-tbody [:tbody {:id "account-totals"} ...totals rows...]} + ...account rows with inputs...) + +;; The amount input posts the whole form but hx-selects ONLY #account-totals. +(com/money-input + {:name (fc/field-name) + :id (str "account-amount-" index) + :class "w-16 account-amount-field" + :hx-post (bidi/path-for ssr-routes/only-routes ::route/edit-form-changed) + :hx-target "#account-totals" ; a SIBLING of this input's row... + :hx-select "#account-totals" ; ...so the input is never in the swapped region + :hx-swap "outerHTML" + :hx-trigger "keyup changed delay:300ms" + :hx-include "closest form"}) +``` + +`grep -c hx-swap-oob` on a migrated modal must be `0` unless a justified disjoint-region +case is documented here and in `gotchas.md`. + +--- + +## The focus invariant (must always hold) + +> The input the user is typing in is never inside the region its own request swaps. + +This is *the* reason the doctrine works. The amount field swaps a sibling tbody; the memo +field swaps nothing; the account typeahead's change swaps the whole form but the typeahead +isn't an active text caret at that moment (it's a click-to-select). The +`transaction-edit-swap.spec.ts` `sameNode` assertions exist to catch any violation. + +## Alpine components must survive swaps + +When a whole-form swap replaces a region containing Alpine/tippy components, they get +re-initialised from the server-provided values. Two hardening moves: + +1. **Null-guard every reference** that depends on Alpine/tippy being initialised: + ```clojure + "@keydown.down.prevent.stop" "tippy?.show()" + "@keydown.enter.prevent.stop" "$refs.input?.__x_tippy?.hide(); ..." + ``` + (`$refs.input?` / `tippy?` — the `?` matters; a swap can run a handler before re-init.) + +2. **Let the server value win.** Because the section is rebuilt fresh on each swap, the + server-driven value (e.g. a vendor's default account) lands without keying tricks — + no preserved stale Alpine state to fight. The "changing the vendor a *second* time + still updates it" e2e is the regression guard for this. + + If you *do* preserve a component across a morph/replace, key it by its server value so + a server-driven change forces re-init: `(assoc attrs :key (str id "--" current-value))`. + +Use `hx/alpine-mount-then-appear` for rows that should mount-then-transition-in (it sets +`x-data {data-key false}`, `x-init $nextTick(() => key=true)`, `x-show key`). + +--- + +## Selector strategy for targeted swaps (a consideration, not a mandate) + +Rules 2 and 4 need a stable `hx-target`/`hx-select`. Per-element unique ids +(`#account-location-0`) work and are what transaction-edit uses today. They get noisy in +deeply repeated/nested structures. When you hit that (Phase 5 / the wizards), consider: + +- **Semantic markup + data-attributes** — mark rows/cells with their identity and target + by attribute, no per-element ids: + ```html + + … + + + ``` +- **A `form-path -> selector` function**, derived the same way a cursor path is, so the + server and the markup agree on the target by construction. A render fn at form-path + `[:accounts 0 :location]` computes its own stable selector from that path. + +**Decision status:** still per-element ids. The first modal to hit nested repeated swaps +(Invoice Bulk Edit, Phase 5) settles the convention and records it here + in +`component-cookbook.md` for the wizards to reuse. diff --git a/.claude/skills/ssr-form-migration/reference/test-recipes.md b/.claude/skills/ssr-form-migration/reference/test-recipes.md new file mode 100644 index 00000000..2b590f33 --- /dev/null +++ b/.claude/skills/ssr-form-migration/reference/test-recipes.md @@ -0,0 +1,89 @@ +# Test recipes + +GROWS every migration. How to characterize and verify a modal. Consistent with the +project `testing-conventions` skill: test user-observable behavior, assert DB state +directly, don't test the means. + +## The three test layers + +1. **Characterization e2e first (Playwright).** Before changing a modal, write/confirm a + spec capturing *current* behavior — focus/caret survival across swaps, each field + round-trip, validation errors, the real save. This is the parity contract; keep it + green through every commit. +2. **Pure-function checks via REPL.** Once render/data-prep fns are pure, exercise them + with `clojure-eval` / `clj-nrepl-eval -p `. Assert on returned data; for markup + use string matches (`(re-find #"accounts\[0\]\[account\]" (str html))`) — this style + survives the Selmer switch. Avoid brittle structural assertions. +3. **DB-state assertions for mutations.** If a submit writes Datomic, verify by querying + the DB, not by asserting on markup. + +## Running e2e + +```bash +npx playwright test # full suite +npx playwright test e2e/transaction-edit-swap.spec.ts # one spec +``` +- Config: `playwright.config.ts`, `baseURL http://localhost:3333`, `webServer: + lein run -m auto-ap.test-server`, `reuseExistingServer: !CI`. +- **The server must be from the worktree you're testing.** `reuseExistingServer` will + silently reuse *any* server on `:3333` — including another worktree's. Confirm with + `ls -la /proc/$(lsof -ti :3333)/cwd` (or restart on a clean port) before trusting a run. +- The test-server port is hardcoded (`test_server.clj` `run-jetty {:port 3333}`); to run a + second server from another worktree, change that or parameterise it. + +## Driving a typeahead in e2e (Solr unavailable in tests) + +```js +await typeahead.locator('a[x-ref="input"]').click(); // open tippy dropdown +const search = page.locator('[data-tippy-root] input[x-model="search"]').first(); +await search.fill('te'); // under 3-char Solr threshold +await typeahead.evaluate((el, id) => { // inject a clickable result + window.Alpine.$data(el).elements = [{ value: id, label: 'Test Account' }]; +}, accountId); +await page.locator('[data-tippy-root] a', { hasText: 'Test Account' }).first().click(); +``` +Entity ids come from `GET /test-info` (`{accounts:{test-account, vendor, vendor2, ...}}`). + +## Proving the focus invariant (caret survival) — the key swap test + +```js +// before the debounced swap lands, capture the live focused node... +await page.evaluate(() => { window.__focused = document.activeElement; }); +await swap; // waitForResponse on the *-form-changed POST +const ok = await page.evaluate(() => { + const a = document.activeElement; + return { sameNode: a === window.__focused, value: a?.value, caret: a?.selectionStart }; +}); +// ...assert the SAME node survived with value + caret intact. +``` +`trackErrors(page)` (collect `pageerror` + `console.error`, assert `[]`) catches a swap +that throws on a stale `$refs`/`tippy` — pair it with every swap test. + +## Asserting "no request" (Rule 1 fields) + +```js +let posts = 0; +page.on('request', r => { if (r.url().includes('edit-form-changed') && r.method()==='POST') posts++; }); +// ...type in the memo... +expect(posts).toBe(0); // memo affects nothing → issues no request +``` + +--- + +## E2E baseline (the regression gate — never drop below this) + +The full suite must stay green after every migration. Specs touching the migrated modals: + +| Spec | Tests | Role | +|------|-------|------| +| `e2e/transaction-edit-swap.spec.ts` | 8 | **Phase 2 parity contract** — whole-form `hx-select` swaps, caret survival, no-request memo, vendor re-select | +| `e2e/transaction-edit.spec.ts` | 15 | transaction edit behavior | +| `e2e/bulk-code-transactions.spec.ts` | 18 | Phase 3 (bulk code) | +| `e2e/transaction-import.spec.ts` | 4 | import | +| `e2e/transaction-navigation.spec.ts` | 13 | navigation | + +**Pass/fail baseline: TO BE CAPTURED at the first Phase 2 e2e run** against a test server +booted from *this* worktree (`integreat-execute-refactor`). At distillation time `:3333` +was occupied by the `integreat-render-whole-form` worktree (morph version), so a run then +would not reflect the merged hx-select reference. Record the green count here once +captured, and never drop below it.