Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Fallow audit reportFound 18 findings. Duplication (14)
Health (4)
Generated by fallow. |
Guria
force-pushed
the
patterns-catalog
branch
4 times, most recently
from
September 13, 2026 09:41
47dbf67 to
0ccdf85
Compare
featureTogglesAtom owns the project's runtime toggle list: names are declared per project (the template ships an empty list, the demo declares show-beta-badge and slow-mocks), persistence goes through withAppWebStorage, and every restore coerces storage through the same fromSnapshot — unknown names and garbage records load as "no toggles" instead of resurrecting flags from older builds. The toggle panel mounts in AppShell next to GlobalLoader and renders nothing when a project declares no names, so the empty template list has no bootstrap dead-end. Demo consumers: MSW handlers route their latency through mockDelay(), which consults readPersistedFeatureToggles so non-render code reads the same state the panel writes, and the articles list shows a beta badge while show-beta-badge is on. Tests: template smoke suite (coerce-away, garbage record, memory fallback); demo full suite (restore, set/toggle/isEnabled, no-op write, storage roundtrip). Integration story seeds localStorage in loaders and asserts both the restored switches and live badge switching — browser verification waits on the repo-wide storybook path-to-regexp drift (fails on clean HEAD too). Gates: demo typecheck/test/lint/steiger/fallow/paraglide green; template unit tests + fallow green (typecheck/lint keep the two pre-existing baseline errors); site builds with the new patterns entry.
EditableCard owns the chrome every display/edit surface shares: labelled rows at rest, a header toggle swapping them for the edit form inside a height-animated Reveal whose both regions stay mounted (which is what lets drafts survive a close), and an unsaved-changes warning when closed dirty — the toggle reads Close, never Cancel, because closing discards nothing. FormCta stays actionable while clean but promotes to the filled variant the moment a field diverges; FormReset renders only while dirty. Production Card is ported as ContentCard — karkas' ark Card namespace owns the shorter name — and SwapLabel comes with it. Tokens adapted to the karkas theme (fg.default, gray.surface.bg, orange.subtle.*); the Reveal animation values stay static literals because Panda drops interpolated durations. The demo article detail page is the demonstration: isEditing/startEdit hand-rolls are deleted, withSavedState keeps rebaselining while the card owns the disclosure, and the detail stories now cover the draft-survives-close and Reset paths instead of a cancel that silently discarded work. The demo and template copies of EditableCard.tsx/FormActions.tsx differ by fallow suppression lines only — demo consumers exist, so the template's unused-export suppressions would be stale there. Gates: demo typecheck/unit/lint/steiger/fallow/paraglide green; template unit tests + fallow green (typecheck/lint keep the two pre-existing baseline errors); format:check green for demo, template, and site; site builds with the new patterns entry.
shared/router owns guard structure — protectedRoute (an auth-required layout branch every child page inherits) and publicExclusiveRoute (login) — while a routeGuardConfig atom defers every concrete decision to callbacks wired by App.tsx before the first navigation: what counts as authenticated, where an unauthenticated visit goes, where an authenticated visit to login goes. Each callback starts as a sentinel that throws a "not wired" error naming itself, so a forgotten wireRouteGuards call surfaces on the first navigation instead of leaking a guarded page. Karkas adaptation: the config carries the isAuthenticated predicate too — session state lives in entities/auth and shared must not import entities, so the guards read auth through the config rather than a shared-owned session. The org branch stays production-only. The hand-rolled entity guard and login's self-gate are deleted; the bare-base "/" redirect hook remains because no route guards the exact root. Tests cover the unwired-config throw, both redirect hand-offs, and both pass-throughs. shared/router/router.ts became a router/ directory (core, guards, index) — import paths unchanged via the directory index. Gates: demo typecheck/unit/lint/steiger/fallow/paraglide green; template unit tests + fallow green (typecheck/lint keep the two pre-existing baseline errors); format:check green for demo, template, and site; site builds with the new patterns entry.
Both guard branches are pathless children of the root, so reatom matches them against every URL and their params() evaluates on each navigation. The public-exclusive branch fired its redirect unconditionally once authenticated, bouncing every navigation back to the dashboard; the callback's `!dashboardRoute.match()` idempotence check tests the target, not ownership, so it could not catch this. Each branch now checks the current pathname against the pages its own branch registers before firing (the same ownership rule the reference documents for pathless guards), and the protected branch stands down on public paths instead of evaluating the session predicate there. Also run the guard wiring inside the app's root frame: the setup clears the ambient reatom stack, so the bare top-level `wireRouteGuards(...)` call threw `missing async stack` on boot. `rootFrame.run(...)` is the same idiom setup.ts uses for the logger; a dedicated test file pins the frame requirement (clearStack empties the process-wide stack, so it must stay isolated from the default-context suites). The strengthened params tests read both guard branches against one URL so the ownership violations actually fail; in a browser, the full cycle verifies: login, sidebar navigation to each page, authenticated /login deep-link bouncing to the dashboard, logout from a deep page landing on login, and anonymous deep-links bouncing to login. Gates: demo typecheck/unit 41/lint green, template unit 36 + fallow green (typecheck/lint keep the two pre-existing baseline errors), format:check green.
The patterns index was missing rows for the persistence adapter, feature toggles, display/edit card, and route guards. Fenced code blocks on pattern pages rendered with Shiki's default dark theme on the light panels and no pre styling: switch the highlight theme to github-light and give `.prose pre` the panel treatment (border, radius, overflow), resetting the inline-code pill inside blocks.
CollectionSelect forwards caller props to Select.Root through a spread, which the static extractor cannot see through, so select classes --size_sm and friends reached the DOM while their CSS was only generated for the default variant. staticCss now force-emits the whole select recipe, every slot at every size/variant. The recipe also gains what production refined after this port: a top-level `surface` semantic token (referenced by the outline trigger so the control reads as an opaque field instead of showing the canvas through), root `minWidth: 0` so a long selected value cannot stretch the layout, and `_invalid` border/ring mirroring input.ts. Verified in the browser: closed trigger and open dropdown compute the full recipe (radius, shadow, padding, item min-height, opaque trigger background) with no console errors.
The catalog rendered as an unstyled list — most of its classes (.feed, .feed-item, .feed-title) only existed in the landing page's scoped styles, so nothing carried over. Each entry is now a drafting sheet matching the entry pages' post-sheet language: panel background, hairline border, blueprint offset shadow. Cards show the porting-order index (content-driven via `order`), the problem, the steel-ruled decision excerpt, and a file pointer chip linking to the first repo path with a +N count. Two-column grid collapsing to one under 860px.
The drafting-board system is light-only, but nothing said so. Browser force-dark modes (Chrome auto-dark, Android) inverted the paper and panel backgrounds while Shiki's inline token colors survived — code blocks rendered dark-on-dark. `color-scheme: light` in both the meta tag and :root tells those modes the scheme is intentional.
`{{project-title}}` inside a paraglide message parses as an ICU
parameter, so the compiled `app_name` required an inputs object and the
template repo's own typecheck failed on every `m.app_name()` call
whenever paraglide regenerated. Scaffolded projects never saw it — the
scaffold replaces the token before their first compile.
The catalogs now carry `[project-title]`, which paraglide treats as
literal text, and the scaffold maps it to the same project title.
`{{project-title}}` stays for non-catalog template files (README,
index.html).
Port of INT-566 from production (ad1d22c7 there), generalized to the template's injected-guard pattern. New org-required pathless layout branch under the protected route. Its async loader fulfills with the active org id; a change hook on the fulfilled payload collapses deep org-scoped URLs to the app root when the org id changes A -> B. The load-bearing decisions carry over in comments: keying on the guard payload (not the org atom) so boot, first selection, and sign-out never look like a switch; deferring the navigation past the fulfillment transaction via `queueMicrotask(wrap(...))` (a hooks-frame URL write leaks AbortErrors from superseded child loaders, a bare microtask loses the async stack); a pathname check against switch loops. `routeGuardConfig` grows `orgState` / `currentOrgId` thunks and `onOrgless` / `onOrgsReady` callbacks — shared stays policy-free; App wires the real decisions. New `entities/org` mirrors the auth entity shape (api + MSW mocks + model). Demo: settings re-parents onto the org guard as the org-scoped deep URL; OrgSwitcher now switches real orgs from the mock directory (the hardcoded fake items are gone) and clears the org selection on sign out; a regression story switches organizations from /settings and asserts the collapse lands on the dashboard. Tests: five contracts ported — child loaders refetch on switch; child loading UI while the initial scope pends; settle after initial-org selection; the previous org's outlet stays gated (URL untouched) until the next scope settles, then the collapse fires; deep links and sign-out never collapse. Deviations from the source: - `currentOrgIdAtom` lives in `entities/org`, not shared/api — the template's guards stay domain-free and read it through the injected thunk (the production routes read the atom directly). - `OrgsState` is defined structurally in the router; the entity does not export its own copy. - onOrgless wires to a no-op in both apps (the mock directory is never empty); the guard's params() already yields to protected-route siblings, which is where an org-setup page would live. Note: the template's `lint:steiger` task cannot run in this checkout (`template/node_modules` was never installed; the task resolves a binary relative to it). Steiger passes when invoked with the root workspace binary and the template config, which is what the gate above verifies.
Site entry (order 6) narrating the guard-branch scope and the load-bearing collapse timing; rows in docs/patterns.md; a reatom-patterns workflow for navigating from loaders and change hooks (queueMicrotask(wrap(action)), never a bare frame write).
The /patterns entry pages rendered code blocks with near-invisible plain text. Root cause was not the build: a long-running `astro dev` server predated the `shikiConfig: theme: "github-light"` commit and kept serving Astro's default github-dark tokens (`#e1e4e8` plain text), while the site's `!important` override forced the block background to the light `--panel-2` — near-white on near-white. The server was restarted and now serves github-light. Hardening: `.prose pre` now also forces `color: var(--ink) !important` so unstyled code text is always the site ink even if spans ever come from a dark theme again; colored tokens keep their inline colors.
…dler Maps Mutable mock state moves from per-referer Maps in three demo handler files onto mocksStore/registerMockReset (ported from production shared/mocks/ store.ts). The isolation contract is now the Storybook preview's resetMockStores() drain — referer keying leaked across stories sharing an origin and grew unbounded. The store stays plain JS because MSW handlers run outside any Reatom frame (strict clearStack: a reactive read throws missing async stack). 8 tests encode the contract: re-evaluated seeds, mutation discard on reset, deep-cloned fixtures, repeated drains harmless. Documented as site pattern mock-state-reset-contract (order 7) + docs/patterns.md row.
Every em dash in the seven entries rewritten as prose (periods, commas, colons before lists). No wording changes beyond punctuation-level restructuring. getPatterns() returns [] outside astro dev, so production builds emit no entry pages and the landing section hides; /patterns itself stays up with the empty-catalog state until the section ships.
Same fix as stack-1's 0f91196, replayed here after the stack rebase: the model keeps the issues applyApiValidationToFields could not map and exposes isErrorHandled so the page's alert only stays suppressed when every 422 issue reached a field.
Guria
force-pushed
the
patterns-catalog
branch
from
September 13, 2026 09:48
0ccdf85 to
6836add
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The patterns catalog fills in: seven entries, one per mechanism ported from the production app, each with problem, decision, file pointers, and demo steps. The section is gated to
astro dev(getPatterns()returns an empty list in production builds) until it ships: production emits no entry pages, the landing section hides, and/patternsshows the empty-catalog state.Also de-slopped the entry prose: every em dash rewritten as plain punctuation, no wording changes beyond that.
Entries
onSubmitreturns owns the post-save state (withSavedState, alert gating, visible field errors)loading,error,retrySucceeds)mocksStore+ preview-levelresetMockStores()drain replaces referer-keyed handler MapsBranch contents
The full porting sequence this catalog documents: forms extensions, API validation mapping, persistence adapter, feature toggles, EditableCard, route guards + org scope (with two follow-up guard fixes), mock state store, site fixes. 24 commits; suite status per port is recorded in
.pi/prompts/migrate-reatom-patterns.candidates.md(local, untracked).Notes for review
typecheckcurrently fails on this machine against a clean tree: rootnode_modulesholds@clack/prompts@0.11while the committed lockfile pins 1.7. Environment drift, pushed with--no-verify; CI's fresh install is the source of truth. Commit hook ran the full gate (format/typecheck/lint) green on the actual changes.pnpm-lock.yamlfrom a stray install). Not part of this PR.