docs: triage the cat-factory slice-4 subject-keyed zones request - #93
Merged
Conversation
Accept the substance (subject-keyed, render-all, module-contributed detail panels), redirect the shape: no new @modular-vue/zones package, no third 'zone' semantic, no new descriptor seam (defineModule zones collides with the shipped ModuleDescriptor.zones field). Counter-proposal: a pure subject resolver over slot entries in @modular-frontend/core plus thin hosts in both bindings, mirroring the slice-2 pairing shape. Corrects the stale peer-range picture (engine is at 0.3.0; compositions/testing still peer ^0.1.0). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012BPCt9jQArsRrVqygKXShC
📝 WalkthroughWalkthroughAdds a triage document that evaluates subject-keyed zones, specifies an engine-neutral panels replacement with Vue and React parity, updates peer-version guidance, outlines documentation work, and maps the revised design to acceptance criteria. ChangesSubject-Keyed Panels Triage
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/subject-keyed-zones-triage.md`:
- Around line 168-172: Update the PanelsOutlet documentation to explicitly
define the subject-key contract: state its accepted type, require a stable
unique value per subject, and specify whether the component supplies a default
when subject-key is omitted. Explain that keys must not be unstable or reused
across different subjects.
- Around line 146-160: Clarify the resolvePanels contract around ordering and
duplicate validation: define where entries with missing order appear relative to
ordered entries, and specify whether duplicate IDs are detected across all
entries or only after when(subject) filtering. Ensure both bindings implement
the same deterministic rules.
- Around line 246-259: The acceptance-criteria table in “What cat-factory's §6
acceptance criteria become” incorrectly marks unshipped implementation-dependent
work as “Met.” Update those dispositions to use “Proposed,” “Planned,” or “Not
yet implemented” unless the referenced engine, Vue, React, and documentation
changes are already shipped, while preserving “Met” only for verified existing
behavior.
- Around line 218-230: Update peer dependency ranges across the entire
`@modular-vue` package family, including `@modular-vue/compositions` and
`@modular-vue/testing`, to admit `@modular-frontend/core` versions ^0.1.0, ^0.2.0,
and ^0.3.0. Ensure the new panels also peer against a range containing the
engine version that provides resolvePanels.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d81baef2-f517-4af1-b952-d9d367a16828
📒 Files selected for processing (1)
docs/subject-keyed-zones-triage.md
Comment on lines
+146
to
+160
| readonly when?: (subject: TSubject) => boolean; // absent = always | ||
| readonly order?: number; // ascending; ties keep contribution order | ||
| readonly props?: Record<string, unknown>; // merged with { subject } | ||
| } | ||
|
|
||
| /** Phantom-typed handle, same convention as defineJourneyHandle / | ||
| * defineCompositionHandle: carries TSubject + the slot key. */ | ||
| export function definePanelGroup<TSubject>(slotKey: string): PanelGroupHandle<TSubject>; | ||
|
|
||
| /** Pure: filter by when(subject), stable-sort by order; null/undefined | ||
| * subject → empty; duplicate ids throw (align with resolveComponentRegistry). */ | ||
| export function resolvePanels<TSubject>( | ||
| entries: readonly PanelEntry<TSubject>[], | ||
| subject: TSubject | null | undefined, | ||
| ): readonly PanelEntry<TSubject>[]; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Specify deterministic ordering and duplicate-ID semantics.
The contract says entries are sorted by optional order, but does not define where missing orders go or whether duplicate IDs are checked before or after predicate filtering. State these rules explicitly so both bindings produce identical results.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/subject-keyed-zones-triage.md` around lines 146 - 160, Clarify the
resolvePanels contract around ordering and duplicate validation: define where
entries with missing order appear relative to ordered entries, and specify
whether duplicate IDs are detected across all entries or only after
when(subject) filtering. Ensure both bindings implement the same deterministic
rules.
Comment on lines
+168
to
+172
| - `<PanelsOutlet :group :subject :subject-key>` — renders every resolved entry with | ||
| `subject` as a prop **and** via `provide` (`usePanelSubject()` for nested content), | ||
| keyed by `entry.id + subjectKey(subject)`, each entry wrapped in | ||
| `ModuleErrorBoundary`; `#empty` slot for the no-match case. (`#wrap` per-entry chrome: | ||
| accepted, it is cheap and genuinely useful for collapsible shells.) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the subject-key contract explicit.
Define its type, uniqueness requirements, and whether <PanelsOutlet> provides a default. Unstable or non-unique keys can reuse panel state across different subjects or cause unnecessary remounts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/subject-keyed-zones-triage.md` around lines 168 - 172, Update the
PanelsOutlet documentation to explicitly define the subject-key contract: state
its accepted type, require a stable unique value per subject, and specify
whether the component supplies a default when subject-key is omitted. Explain
that keys must not be unstable or reused across different subjects.
Comment on lines
+218
to
+230
| - The widen it asks for **already shipped**: `@modular-vue/{vue,core,runtime,nuxt,journeys}` | ||
| peer `@modular-frontend/core@^0.1.0 || ^0.2.0` today. | ||
| - Meanwhile `@modular-frontend/core@0.3.0` published on 2026-07-19, so those same ranges | ||
| now exclude the _current_ engine again, and `@modular-vue/compositions` + | ||
| `@modular-vue/testing` were never widened past `^0.1.0`. | ||
|
|
||
| Accepted as housekeeping, generalized: widen the whole Vue family (including | ||
| compositions and testing this time) to include `^0.3.0` in the next release train, and | ||
| have the new panels surface peer against the range that includes the engine version that | ||
| ships `resolvePanels`. A follow-up worth considering separately: this drift has now | ||
| recurred three times (React family in the changelog's peer-range entries, Vue slice 2, | ||
| now 0.3.0) — a `check:publish`-style script asserting every workspace peer range admits | ||
| every workspace sibling's current version would end the category. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -t f 'package.json' packages -x rg -n -C2 \
'"`@modular-frontend/core`"|"`@modular-vue/`(compositions|testing)"' {}
pnpm view `@modular-frontend/core`@0.3.0 time --jsonRepository: kibertoad/modular-react
Length of output: 4490
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== pnpm view time =="
pnpm view `@modular-frontend/core`@0.3.0 time --json || true
echo
echo "== npm view time =="
npm view `@modular-frontend/core`@0.3.0 time --json || true
echo
echo "== package manifests for relevant workspaces =="
for f in $(git ls-files 'packages/**/package.json'); do
if rg -q '"`@modular-frontend/core`"|"\`@modular-vue/`(compositions|testing|vue|core|runtime|nuxt|journeys)"' "$f"; then
echo "--- $f"
rg -n -C1 '"`@modular-frontend/core`"|"`@modular-vue/`(compositions|testing|vue|core|runtime|nuxt|journeys)"' "$f"
fi
doneRepository: kibertoad/modular-react
Length of output: 7241
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' /tmp/coderabbit-shell-logs/shell-output-vTGnjGRepository: kibertoad/modular-react
Length of output: 4490
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== pnpm view time =="
pnpm view `@modular-frontend/core`@0.3.0 time --json || true
echo
echo "== npm view time =="
npm view `@modular-frontend/core`@0.3.0 time --json || true
echo
echo "== relevant package manifests =="
for f in $(git ls-files 'packages/**/package.json'); do
if rg -q '"`@modular-frontend/core`"|"\`@modular-vue/`(compositions|testing|vue|core|runtime|nuxt|journeys)"' "$f"; then
echo "--- $f"
rg -n -C1 '"`@modular-frontend/core`"|"`@modular-vue/`(compositions|testing|vue|core|runtime|nuxt|journeys)"' "$f"
fi
doneRepository: kibertoad/modular-react
Length of output: 7226
🌐 Web query:
npm registry publication time for @modular-frontend/core 0.3.0 July 2026
💡 Result:
There is no publicly available npm package named @modular-frontend/core, nor is there evidence of a 0.3.0 version for such a package being published in July 2026. While the name @modular-frontend/core appears in technical discussions as a router-neutral core library within the context of the kibertoad/modular-react repository, documentation and commit history indicate that these packages are maintained within a monorepo and are not published to the public npm registry [1][2]. Specifically, recent development activity within that repository has focused on adding support for Vue and Angular, and contributors have noted that these modular-frontend packages remain unpublished [2]. Other packages with similar names exist on the npm registry, such as mod-arch-core (published June 3, 2026) [3] and @modjs/core (published August 25, 2024) [4], but these are distinct projects unrelated to the @modular-frontend namespace.
Citations:
- 1: e394b25
- 2: feat(vue): add @modular-vue/runtime registry (PR-21) #63
- 3: https://npmx.dev/package/mod-arch-core
- 4: https://registry.npmjs.org/%40modjs%2Fcore
Widen the Vue peer ranges to include @modular-frontend/core@0.3.0
@modular-frontend/core@0.3.0 was published on 2026-07-19, but @modular-vue/compositions and @modular-vue/testing still cap at ^0.1.0, and the rest of the Vue family still stops at ^0.2.0, so the current engine is excluded again.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/subject-keyed-zones-triage.md` around lines 218 - 230, Update peer
dependency ranges across the entire `@modular-vue` package family, including
`@modular-vue/compositions` and `@modular-vue/testing`, to admit
`@modular-frontend/core` versions ^0.1.0, ^0.2.0, and ^0.3.0. Ensure the new
panels also peer against a range containing the engine version that provides
resolvePanels.
Comment on lines
+246
to
+259
| ## What cat-factory's §6 acceptance criteria become | ||
|
|
||
| | Their criterion | Disposition | | ||
| | ----------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | `@modular-vue/zones` published; `defineZone`/`ZoneOutlet`/`useZone`/`useZoneSubject` importable | **Changed**: import `definePanelGroup`/`resolvePanels` (engine, via `@modular-vue/core`) and `usePanels`/`<PanelsOutlet>`/`usePanelSubject` (`@modular-vue/vue`). No new package. | | ||
| | Render-all, ordered, sectioned, no router | **Met**, with sections replaced by per-region group ids. | | ||
| | Subject-reactive, incl. mutable run-state predicates | **Met**, with the documented reactive-source boundary (their subject already qualifies). | | ||
| | Render-all distinct from `pairById` | **Met**; documented side by side. | | ||
| | Consumer-extensible via module registration, no host edit | **Met** — it is the existing module → slot path. | | ||
| | Headless `/testing` resolver | **Met** by `resolvePanels` being a pure engine export; no subpath needed. | | ||
| | `installModularApp` provides the zone manifest, typed, opt-in | **Obsolete** — the slots context Nuxt already provides is the manifest; nothing to thread. | | ||
| | Peer alignment | **Superseded** by the corrected picture above (family-wide widen to `^0.3.0`, incl. compositions/testing). | | ||
| | Docs | **Met** per the docs plan. | | ||
| | Additive, no breaking changes | **Met** — and only met _because_ the `defineModule({ zones })` seam was rejected; as requested it was breaking. | |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not label future work as “Met.”
This PR documents a proposed design and a future documentation plan, but the table marks implementation-dependent criteria as met. Use “Proposed,” “Planned,” or “Not yet implemented” unless the corresponding engine, Vue, React, and documentation changes are already shipped.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/subject-keyed-zones-triage.md` around lines 246 - 259, The
acceptance-criteria table in “What cat-factory's §6 acceptance criteria become”
incorrectly marks unshipped implementation-dependent work as “Met.” Update those
dispositions to use “Proposed,” “Planned,” or “Not yet implemented” unless the
referenced engine, Vue, React, and documentation changes are already shipped,
while preserving “Met” only for verified existing behavior.
4 tasks
kibertoad
added a commit
that referenced
this pull request
Jul 19, 2026
* feat(frontend-core,react,vue): subject-keyed panels (render-all, predicate-gated) Implements the counter-proposal from the cat-factory slice-4 triage (#93): a render-all, subject-keyed, open-contribution panel surface — the sibling of the pick-one component-pairing helpers — landed engine-first with both bindings in the same train, under the name "panels" (not "zones"). Engine (@modular-frontend/core): - PanelEntry<TSubject>, PanelGroupHandle<TSubject>, definePanelGroup - resolvePanels: pure resolver (null subject -> empty; when(subject) filter; stable order sort; duplicate-id throw with first-wins/last-wins escape hatch mirroring resolveComponentRegistry) - Re-exported by @modular-react/core (export *) and @modular-vue/core Vue (@modular-vue/vue, re-exported from @modular-vue/core): - usePanels (computed over the slots source + subject; prefers the reactive slots source), <PanelsOutlet> (render-all, subject injected as prop + provide, per-panel ModuleErrorBoundary, #empty / #wrap slots), usePanelSubject React (@modular-react/react): - usePanels, <PanelsOutlet>, usePanelSubject / PanelSubjectContext — same surface with useMemo + context Contributions ride the existing module -> slot path; no descriptor change, no new package, no registry plugin, no /testing subpath, no Nuxt manifest threading — all subsumed per the triage. Docs: - docs/subject-panels.md guide (entry shape, resolver, both hosts, the inspector recipe, the Vue reactivity caveat) - compositions README "pick by problem shape" table extended 3 -> 4 primitives; cross-links from reactive-slots-vue.md and remote-capability-manifests.md; triage doc marked resolved Housekeeping (Gap D): widen the Vue family peer ranges (incl. compositions and testing) to admit @modular-frontend/core@^0.3.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QULkEZq9fop9povfXdMecL * refactor(panels): address review findings — validation order, dual slots source, shared dedup, boundary labels Code-review + architecture follow-ups on the subject-keyed panels PR: - resolvePanels validates duplicate ids BEFORE the null-subject guard, so a registration bug surfaces deterministically on first resolve — including the usual initial state where nothing is selected yet. New engine tests lock it. - The duplicate-id collapse is now one shared implementation (collapseEntriesById in component-registry.ts) behind both resolveComponentRegistry and resolvePanels, so the two reductions' duplicate stance cannot drift; the redundant post-filter copy in resolvePanels is gone. - Vue usePanels tracks BOTH runtime slot sources — the reactive computed and the recalculateSlots() signal Ref — serving whichever produced the more recent evaluation. Panels contributed via dynamicSlots now update on either path, matching the React host (whose single context carries both). Covered by a both-directions test. - Vue usePanels accepts MaybeRefOrGetter for group and onDuplicate, resolved inside the computed; <PanelsOutlet> passes getters, so its props are live after mount instead of frozen at setup (tested via a post-mount onDuplicate flip). - ModuleErrorBoundary (both bindings) gains an optional `label` prop (default "Module"); <PanelsOutlet> passes "Panel" so a crashed panel isn't mislabeled a module. - React usePanels uses a module-level empty-entries constant so the memo stays stable for uncontributed groups. - New host tests in both bindings: subjectKey remount semantics (state kept without it, remounted with it) and onDuplicate forwarding. - Docs: resolver semantics reordered; new "when vs dynamicSlots" section; props.subject override documented; Vue caveat now covers the contribution side; CHANGELOG gains the missing Unreleased entry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W5fr1q1oVcmjg9s1zibMXP * docs(panels): documentation coverage for subject-keyed panels Close the doc-discoverability and API-reference gaps around the panels feature: - README.md: add a "Subject-keyed panels" row to the documentation index so the guide is discoverable alongside the sibling primitives. - packages/frontend-core/README.md: document the engine exports (definePanelGroup, resolvePanels, PanelEntry, PanelGroupHandle). - packages/react/README.md: document the React host (usePanels, PanelsOutlet, usePanelSubject, PanelSubjectContext). - packages/vue/README.md: document the Vue host (usePanels, PanelsOutlet, usePanelSubject, panelSubjectKey), with the reactivity-caveat pointer. - docs/subject-panels.md: add a concrete end-to-end walkthrough tying the scattered fragments into one inspector-rail scenario — subject type, group handle, first-party + consumer contributions, and a trace of exactly which panels render (ordered, filtered) across selections, including the open-contribution case where a consumer panel appears with no host edit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NV2ACUkMCvCgquryU3P5Cb * test(examples): runnable subject-keyed panels example + e2e (React Router) Add examples/react-router/inspector-panels — a self-contained pnpm workspace demonstrating the panels feature end to end, matching the sibling examples' conventions (app-shared / modules / shell + Playwright e2e): - app-shared: the BoardBlock subject type, a sample board, and the shared definePanelGroup handle. - modules/inspector-core (first-party): an always-on `identity` panel plus a `frontend-config` panel gated to frame-level frontend blocks; demonstrates both the injected `subject` prop and the `usePanelSubject` context reader. - modules/acme-extras (consumer): a namespaced `acme:security-report` panel for its own `acme-secure` block type, added with no edit to the host — the open-contribution property. - shell: registry wiring plus the board host and the <PanelsOutlet> inspector rail (subjectKey / empty / wrap), and a smoke.spec.ts that asserts the exact filtered/ordered render across selections, the empty state, and re-resolution. Verified: typecheck, vite build, and the Playwright e2e (5/5) all pass. Listed in examples/README.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NV2ACUkMCvCgquryU3P5Cb * docs(panels): address review — correct React definePanelGroup import, fix README fragment, clarify subjectKey string form - docs/subject-panels.md: import definePanelGroup from @modular-react/core (the /react host layer does not re-export it); both examples - packages/vue/README.md: complete the sentence fragment in the panels bullet - panels docstrings (React + Vue) and docs: clarify that subjectKey is re-read each render, so the string form is a caller-computed discriminator (a literal constant never drives remounts), while the function form derives it from the subject Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SDhW7GZMkb4pSCwKVch4tC --------- Co-authored-by: Claude <noreply@anthropic.com>
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
2 participants
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.
Accept the substance (subject-keyed, render-all, module-contributed detail
panels), redirect the shape: no new @modular-vue/zones package, no third
'zone' semantic, no new descriptor seam (defineModule zones collides with
the shipped ModuleDescriptor.zones field). Counter-proposal: a pure
subject resolver over slot entries in @modular-frontend/core plus thin
hosts in both bindings, mirroring the slice-2 pairing shape. Corrects the
stale peer-range picture (engine is at 0.3.0; compositions/testing still
peer ^0.1.0).
Co-Authored-By: Claude Fable 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_012BPCt9jQArsRrVqygKXShC
Summary by CodeRabbit