feat: support items-based (zero-or-more) candidate catalogs in pattern decisions - #2932
Open
YoofiTT96 wants to merge 52 commits into
Open
feat: support items-based (zero-or-more) candidate catalogs in pattern decisions#2932YoofiTT96 wants to merge 52 commits into
YoofiTT96 wants to merge 52 commits into
Conversation
Extends pattern-based decision support beyond prefixItems positional slots (pick exactly one) to also cover items.oneOf/items.anyOf open catalogs (pick zero or more), across validation, visualisation, and generate/instantiate. - shared/spectral: ids-are-unique and pattern-nodes-must-be-referenced now also scan items.oneOf/anyOf candidates (nodes, relationships, and interfaces nested in catalog nodes). - calm-hub-ui: patternTransformer extracts items catalog candidates as decision groups, and folds an options relationship's referenced ids into a single decision group regardless of whether they come from a prefixItems slot or the items catalog, creating a new group when none of the referenced ids already belong to one. Dangling id references still render nothing. - shared/generate: selectChoices narrows a selected items catalog down into prefixItems so instantiate materializes chosen catalog nodes without needing separate items handling.
…-decision-support
…n decisions Follow-up fixes and documentation on top of items-based decision support (finos#2859): - calm-hub-ui: a node that is both a decision candidate and a container child now renders inside its container rather than the choice box, and a decision box left empty by that reparenting is suppressed so no empty box is drawn. - shared/generate: an array declared only through an items catalog and generated with no selection now instantiates as [] instead of {}, fixing malformed output. - docs: add PATTERNS_OPTIONS_AND_DECISIONS.md explaining how validation, generation and visualisation each consume pattern decisions independently; linked from the shared and calm-hub-ui AGENTS.md guides. - calm-ai: document the items open-catalog construct in the pattern creation guide. Refs finos#2859
YoofiTT96
requested review from
a team,
LeighFinegold,
Thels,
aidanm3341,
grahampacker-ms,
jpgough-ms,
lbulanti-ms,
markscott-ms,
rocketstack-matt and
willosborne
as code owners
August 3, 2026 16:08
github-actions
Bot
added
shared
calm-hub-ui
YoofiTT96
marked this pull request as draft
August 3, 2026 16:16
Independent review found two validation call sites that were still prefixItems-only and had been missed when items.oneOf/anyOf support was added, contradicting the feature (finos#2859): - node-id-exists.ts (backs the connects, deployed-in/composed-of, actor/container, and decision nodes.const rules) now also resolves ids against nodes.items.oneOf/ anyOf, so a relationship or decision referencing a catalog-only node no longer raises a false "does not refer to an existing node" error. - interface-id-exists-on-node.ts now includes items-catalog nodes in its node lookup, so interface checks on catalog nodes run instead of being silently skipped. - options.ts flattenCalmItems only strips items when it is a oneOf/anyOf catalog, leaving an unrelated plain/false items schema untouched. Also document, in PATTERNS_OPTIONS_AND_DECISIONS.md, the full set of validation helpers updated and a known rendering limitation (two decisions sharing one items catalog merge into one box). Refs finos#2859
Removes docs/PATTERNS_OPTIONS_AND_DECISIONS.md and the two AGENTS.md pointers to it. An implementation-internals document is a maintenance liability: it must be updated on every change to decision handling or it silently misleads. The load-bearing context does not depend on it — the known rendering limitation is tracked in finos#2933, the validation/rendering behaviour changes are in the PR description, fixed behaviours are pinned by tests, and the allOf-merge caveat is a code comment. Refs finos#2859
…gaps Addresses nits from an independent review of finos#2932: - instantiate.ts: check const before the bare-array fallback so an array schema carrying a const materializes its value instead of being emptied to []. Latent ordering issue introduced with the earlier all-optional-array fix; not reachable by today's node/relationship arrays but fixed for safety, with a regression test. - ids-are-unique.spec.ts: add the missing relationships items.oneOf duplicate case (the code path existed but was untested). - patternTransformer.test.ts: cover an items catalog with no options relationship (extracted-but-never-folded group renders as a labelled box with no prompt). - ids-are-unique.ts: add the missing trailing newline. Refs finos#2859
YoofiTT96
marked this pull request as ready for review
August 3, 2026 21:53
markscott-ms
reviewed
Aug 6, 2026
markscott-ms
left a comment
markscott-ms
left a comment
Contributor
There was a problem hiding this comment.
Review
Adds items.oneOf/items.anyOf open-catalog support across the three surfaces that read pattern decisions independently. The core approach — narrowing a selected catalog into prefixItems inside selectChoices so instantiate() needs no new concept — is the right call and keeps the generation change small. Good comments explaining why, sensible decomposition (extractNodeDecisionGroup, extractRelationshipDecisionGroup, foldOptionsMetadataIntoDecisionGroups), and an honest write-up of the behaviour changes.
Most of my findings are gaps rather than defects in what's written; they're left inline. Two things that don't anchor to a diff line:
isPatternData in calm-hub-ui/src/visualizer/components/drawer/Drawer.tsx:21 wasn't updated
It requires properties.nodes.prefixItems to classify JSON as a pattern. A catalog-only pattern — nodes declared solely through items, a shape this PR explicitly supports and covers in patternTransformer.test.ts ("creates a decision group box for an items catalog with no options relationship") — fails that check and gets routed as an architecture. Hub-sourced patterns are saved by the calmType === 'Patterns' fallback on line 80; the file-upload path isn't. One-line fix: accept items as well as prefixItems.
isDefinedInOneOfOrAnyOf deliberately needing no change is worth a sentence in the PR body
It's the one decision-related pattern rule left untouched, so a reader will wonder. It only fires when an id is present in plain prefixItems and absent from any oneOf/anyOf, so a catalog-declared id passes rather than false-positives. Correct as-is, just non-obvious.
Verified as fine
nodeHasRelationship's$..relationship-type..*@string()already sweeps catalog relationships and decisionnodes.constarrays, so the widenedgiveninrules-pattern.tsbehaves as the PR body claims.- Ordering in
flattenCalmItems([...flattenedPrefixItems, ...selectedCatalogItems]) is correct against draft 2020-12: selected candidates land at indices >= the originalprefixItemslength, exactly where the original pattern'sitemsapplies, so the generated architecture still validates against the unmodified pattern. - The
const-before-array reordering ininstantiate.tsis right, and the dedicated test pinning that branch order is a nice touch.
Test coverage
Solid on the paths that matter, with two new branches uncovered — see the inline notes on the two test helpers. Also worth a test for the multi-decision-over-one-catalog case, whichever way it's resolved, since it pins the intended semantics.
Risk
Low. The stricter validation only affects a construct nothing in the repo uses, and is already flagged for a release note. The instantiate [] fallback is the only change touching pre-existing behaviour, and {} where an array was required was never valid output — a strict improvement.
…erns Address review findings on the items-catalog decision-support PR. - generate: guard the relationships access in flattenOptionsRelationships so a pattern whose nodes are declared entirely through an items catalog (and that carries no relationships property) no longer throws during selectChoices. - calm-hub-ui: widen isPatternData to accept nodes.items as well as nodes.prefixItems, so a catalog-only pattern dropped as a file is routed to the PatternVisualizer instead of being misclassified as an architecture. - Close test gaps: relationships items-catalog narrowing, a nodes-only catalog with no relationships property, the items.anyOf node catalog UI path, the dashed rel-decision-items edge path, and the catalog-only file classification.
When two allOf branches each declare an items open-catalog under the same array property (e.g. both define properties.nodes.items), flattenAllOf's shallow properties merge makes the later branch's catalog win and silently drops the earlier one. Add logDroppedItemsCatalogs to log this at debug level so it is discoverable under --verbose rather than only from a code comment. The detection lives at the properties-merge site where the loss actually occurs, not the top-level items else-branch (which a per-array catalog never reaches). Clarify that else-branch comment accordingly.
Behaviour-preserving cleanup of the items-catalog decision code.
- Extract getArrayKeyword(pattern, key, keyword) and reduce getPrefixItems and
getItems to thin wrappers over it, removing the duplicated allOf-walking
shape. The wrappers keep their exact return contracts ([] vs undefined, and
the items:false closed-tuple case), so every call site is unchanged.
- Extract catalogAlternatives(items) returning { groupType, alternatives } | null
and use it in both extractNodesFromPattern and extractRelationshipsFromPattern,
removing the repeated Array.isArray(oneOf/anyOf) blocks.
No behaviour change: the full calm-hub-ui suite is byte-for-byte identical
(114 files, 1371 tests) before and after, typecheck and lint clean.
Contributor
|
@YoofiTT96 let us know when this is good for re-review |
… calm-models
Two copies of the candidate-listing walk existed: calm-models ignored allOf,
shared followed it through getPatternArray and reported a path the document
didn't contain. Nothing tested or relied on the divergent behaviour, and
allOf for nodes/relationships is already unsupported, so the copies are
unified in calm-models rather than reconciled - one walk, parameterised by
resolution ('all' for listCandidates, 'operative' for the new
listSelectableCandidates), matching the pattern readChoiceBlock already set.
shared/src/pattern-candidates.ts is deleted; its two consumers
(decision-references-selectable-candidate.ts and options.ts's
assertChoicesAreSelectable) now import from @finos/calm-models/pattern.
assertChoicesAreSelectable had no test coverage before this; added 7 cases.
Added 8 new calm-models tests for listSelectableCandidates.
catalog-decisions.spec.ts's allOf regression test called the guard directly
on a raw, unflattened pattern - a call shape with no real caller, since
runGenerate always flattens first. Fixed the test to flatten first, matching
actual usage; the real generate pipeline was never broken.
…ixItems slots The rule's given already matched prefixItems slots as well as items catalogs - both shapes hit the same underlying defect, oneOf silently winning over anyOf - but the message and the rule's own description always said "an items catalog declares both...", wrong wording for half its matches. Made both shape-neutral. Added the missing prefixItems-trigger test coverage; none existed despite the given already covering it.
The authoring guide claimed a catalog node with no reference produces a warning "exactly as for prefixItems". True for a plain prefixItems entry, not for a prefixItems[i].oneOf/anyOf alternative - pattern-nodes-must-be- referenced never covered that shape, on main or after this PR. Corrected the claim and pointed at the tracked follow-up.
…ching the catalog path patternTransformer.ts hand-rolled hasOneOf/hasAnyOf for prefixItems slot alternatives in three places, right alongside calls to the shared readChoiceBlock for the items-catalog case. Same oneOf-wins-over-anyOf precedence, same shape - readChoiceBlock already answers this question. Pure refactor, no behaviour change for any valid input: verified the calm-hub-ui test suite (1419 tests) passes identically before and after, and diffed tsc --noEmit output before/after to confirm no new type errors. readChoiceBlock's null guard is marginally more defensive than the hand- rolled version for a null/undefined prefixItems entry, which the old code would have thrown on - not reachable by any legal pattern.
…st the first extractChoicesFromArchitecture indexed relationship-type.options[0], so a zero-answer anyOf decision crashed the validator (reading .description off undefined) and a two-or-more-answer decision silently validated only the first, leaving every other selection completely unchecked. Pre-existing on main, byte-identical, and unrelated to items catalogs: anyOf-typed multi-answer decisions over plain prefixItems candidates are documented on main (calm-ai/tools/pattern-creation.md, 'Providing Options with anyOf/oneOf'), already enforced by pattern-option-relationship-must-only-have-oneof-or-anyof-items, and trace to issue finos#706. The CALM meta-schema places no length restriction on options. Confirmed the real shape via the actual generate pipeline: one options[] entry per selection, not one combined entry. Fix: flatMap every options entry instead of indexing [0]. selectChoices already handles multiple CalmChoices for the same decision correctly.
…s catalogs Two related additions, both about the same problem: nothing tested a full generate-then-validate cycle against a real pattern, which is why the control-id gap and the finding-8 multi-answer bug both went unnoticed. cli.e2e.spec.ts: pins the conference-signup pattern's known, pre-existing failure (control-requirement-validation on control-id) as an explicit baseline inside the existing 'Getting Started Verification' test. Root cause: calm/getting-started/controls/permitted-connection-jdbc.config.json declares control-id 'security-003'; the shared requirement schema it's checked against demands 'security-002'. The sibling http config file - same control, different protocol - correctly uses security-002, confirming this is a copy-paste slip from when the jdbc variant was authored (both created together in commit 4b8a718, June 2025), not an intentional difference. Reproduces identically on main; the checked-in reference fixture inherits the same wrong value. Comment says to delete the block once this is actually fixed - it's a one-value data correction, not a design question. items-catalog-round-trip.e2e.spec.ts: a new, clean baseline that actually works, since no repository pattern uses items catalogs yet to point at instead. Deliberately excludes control requirements so it's unaffected by the conference-signup gap. Three cases, one shared fixture: a clean 2-select happy path, a 0-select case (the crash half of finding 8's bug), and a corrupt-the-second-selection case (the silent-validation-gap half). The corruption case is the one with real discriminating power - verified empirically against two different bugs (extractChoicesFromArchitecture's original .map(...)[0] and a simulated equivalent in the sibling flattenOptionsRelationship), confirming it fails for either and passes again once each is reverted, so its protection isn't narrowly tied to one specific code path.
|
|
The TEMPORARY comment on readArrayKeyword described the allOf fallback as if one branch won for the prefixItems/items pair. getPatternArray actually calls it once per keyword, each search independent, so the two keywords for the same property can resolve from different allOf branches. Reworded to say so. Comment only, no behaviour change.
…ent allOf branches getPatternArray resolved prefixItems and items independently, each with its own allOf fallback search. A pattern could end up with prefixItems from one branch and an items catalog from a different branch - an array no single declaration site in the document actually describes. Introduced by this PR (getPatternArray/pattern-reader.ts is new on this branch; main never resolved items through allOf at all, so the two-keyword composition wasn't reachable there). Fix: resolve one container per property (root, else the first allOf branch that declares either keyword) and read both keywords from it. No signature or caller changes. Pinned four cases in pattern-reader.spec.ts: the original repro, the reverse direction (catalog on one source, prefixItems on another), a split across two different allOf branches with an empty root, and the existing container-skip precedent (root declares the property but neither keyword).
YoofiTT96
force-pushed
the
feat/2859-items-based-decision-support
branch
from
August 25, 2026 00:00
beb0f25 to
245f5e4
Compare
…rop line citations getPatternArray's allOf table row still described the pre-fix behaviour (independent per-keyword resolution) instead of the single-branch-for-both-keywords fix, and the paragraph after it now contradicted that row by claiming no reader reaches into allOf. Corrected both. Replaced four file:line citations with function names or, where the specific expression was the point (the node-has-relationship.ts JSONPath query), a quoted snippet instead. Line numbers drift on any unrelated edit above them with nothing to catch it; three of the four citations had already gone stale.
Three cuts, no new claims: - opening two sentences duplicated pattern-creation.md's decision holder/candidate definitions near-verbatim; point at that file instead of re-deriving the concept, keep only the code-specific enforcement detail - getPatternArray was described twice two lines apart (once generically, once with allOf detail in the very next table); the first mention now just points at the table row - the allOf section stated 'unsupported' at open and close of the same subsection; kept only the clause after the second mention that adds something (the listCandidates removal didn't add allOf support), cut the restated half in front of it
pattern-diff.ts read prefixItems independently, with its own allOf fallback, and never looked at items - so a catalog candidate added or removed from a pattern produced no diff at all. calm diff --exit-code is a documented CI gate for version bumps, so a breaking catalog change could pass it silently. getPrefixItems is replaced by getCandidateItems, delegating to getPatternArray and feeding its catalog through the same expandAlternatives step slot alternatives already go through. Pinned with 4 new tests (add/remove/unchanged/instantiate). Full calm-models suite 231/231, build clean, lint 0 errors.
…odes as unchanged applyDiffStatus defaults every node to 'unchanged' and only moves it on a unique-id match against DiffResult. Catalog candidates never appeared there, so a newly added one was drawn (this PR's own transformer draws catalog candidates) but coloured as unchanged - a false statement, not an incomplete one. No source change needed here: fixing pattern-diff.ts's own blind spot (previous commit) fixes this as a direct consequence. This test proves that with the real diffPatterns output, not a hand-built DiffResult - confirmed red by reverting the previous commit and rebuilding, green after restoring it.
instantiateFromPattern only ever read properties.<nodes|relationships>. prefixItems, so a catalog-only pattern previewed as empty. It already handled prefixItems[i].oneOf/anyOf decisions correctly - only the catalog was unhandled. An items catalog is structurally the same oneOf/anyOf block a decision slot's alternatives are, so the new catalogEntry helper appends it to the same list instantiateNode/instantiateRel already unwrap, rather than adding a second code path. No test file existed for this component; added one (3 tests, red confirmed before the fix). Full vscode suite 158/158 + 1 pre-existing todo, lint 0 errors.
The comment said calm generate never fetches the schema requirement-url points at. That's not it: permitted-connection-jdbc.config.json declares control-id security-003, but the requirement schema pins security-002 - a one-value copy-paste slip (the sibling http config correctly uses security-002). The commit that added this baseline said so in its own message; only the inline comment was wrong, and it's the one a maintainer would actually follow.
…n illegal empty prefixItems An anyOf decision left unanswered (a checkbox with nothing checked) made flattenOptionsRelationship write options.prefixItems: [] into the narrowed pattern. An empty prefixItems is not a legal JSON Schema - selectChoices's own docstring already forbids it elsewhere - so the next schema compilation broke. calm generate itself didn't notice (the empty array is fine as instance data), but calm validate compiles the same narrowed pattern as a schema, and that's where it surfaced: 'options/prefixItems must NOT have fewer than 1 items'. Not a regression: on main this same input crashed a different way (options[0] read off undefined). But 'pick none from this catalog' is the feature's own advertised case, so leaving it broken undercuts the headline. Fix: a decision resolved to nothing chosen has nothing to materialize, so flattenOptionsRelationship now drops the holder relationship entirely rather than narrowing it to empty. Reproduced live against a real repository pattern (multiple-choices options-prototype.pattern.json) before writing the fix - confirmed the exact reported error, confirmed generate succeeds silently while validate is the one that breaks, confirmed the fix removes exactly that error and nothing else (a separate, pre-existing schema-compile issue in that same prototype pattern is present before and after, unrelated). Pinned with a new selectChoices test. Full shared suite 1171/1171 (1 pre-existing unrelated flaky docify test excluded), full cli suite 648/648, build and lint clean on both.
foldOptionsMetadataIntoDecisionGroups's own JSDoc said a referenced id already in a decision group gets that whole group folded into it. The code does the opposite: every decision always gets its own new group, and referenced ids are moved out of whatever group they were in, into that new one. The code is right (this is the fix behind two decisions over one catalog drawing two boxes); the comment was describing an earlier version.
logger.warn on every discarded-key detection meant new machinery in the merge path could interrupt normal output for a construct the pattern's own documentation already declares unsupported (allOf for nodes/relationships), including a false 'discarded' report on legitimate refinement - a support question with no action attached. Kept the heuristic; only the log level changes. Updated the five discarded-key tests to assert mockDebug instead of mockWarn, and the two negative assertions to check the specific discard message rather than 'not called at all', since flattenAllOf already logs unrelated debug traces that a bare not.toHaveBeenCalled() would collide with. flatten-allof.spec.ts 14/14, full shared suite 1171/1171 (1 pre-existing unrelated flaky docify test excluded), build and lint clean.
…y answer readChoiceBlock and listCandidates named the mechanism (read a block, list candidates), not the question a caller is actually asking - so the wrong one was easy to reach for silently. readChoiceBlock -> resolveOperativeChoiceBlock: this is the function that resolves a block to the one form selection would act on (oneOf wins over anyOf), matching the file's own internal 'operative' vocabulary (BlockResolution). listCandidates -> listDeclaredCandidates: pairs with the unchanged listSelectableCandidates - same shape, opposite resolution, so the two names now signal they're answering the same question two ways. Pure rename across all 13 call sites (calm-models, shared's spectral rules and options.ts, calm-hub-ui's patternTransformer.ts) plus shared/AGENTS.md's own references. No behaviour change: build clean, full calm-models/shared/calm-hub-ui/cli/vscode suites green, lint 0 errors across all five.
The Pattern Decisions section in AGENTS.md had grown to cover four readers, the allOf disagreement, enforcement, visualiser folding, and duplicated code - a lot for a package AI-assistant guide to carry, and scoped to shared even though half of it describes calm-hub-ui and calm-models behaviour too. Moved it to shared/PATTERN-DECISIONS.md: a recap of current, intended behaviour for how patterns express decisions, tying together rules spread across calm-models, shared, and calm-hub-ui. Not a design proposal - it states what the code already does and why, including two disagreements between packages that are not fixed here: - allOf's three readers, and why each made its own narrow choice instead of a real intersection merge. - options.ts vs patternTransformer.ts on a decision holder that declares both oneOf and anyOf - confirmed pre-existing against main, and confirmed that no Spectral rule's given paths reach a decision holder's own nested options block, which is the reason nothing catches it today. AGENTS.md's Pattern Decisions section is now a pointer plus the one piece of testing guidance that belongs in a package guide, not a behaviour recap. Written in Simplified Technical English per the root AGENTS.md's documentation guidance.
Member
Author
1, 2, 3, 5, 6 Addressed |
Member
Author
|
@markscott-ms PR updated with changes for re-review |
Strips the PR-review voice: changelog narration, comparisons against main, and claims that follow-up work is tracked when no issue exists. Corrects the known-disagreement section. The divergence sits inside options.ts - extractOptionsFromBlock offers the union of both choice keywords while flattenOneOfAndAnyOf resolves oneOf only, so an answer from the anyOf half is accepted and then silently discarded. The visualiser already matches generation's resolution step, and the validation rule enumerates rather than resolves, so neither is a party to it. Replaces "still duplicated" with the three concrete divergences between the two decision-holder readers. Drops the candidate-helper entry: identical code with no behavioural disagreement is not what this reference is for. Corrects the enforcement note - the keyword rule does reach a decision holder as a relationships.prefixItems entry, it just never descends into the holder's own options block.
…eaders listNodeInterfaces reads pattern schema and has no Spectral coupling, but it lived in shared's Spectral rules folder, where every other file is a rule function. It carried private isObject/readUniqueId copies only because it was reading schema in a layer that holds no schema readers. Export it from calm-models/pattern and delete candidate-helpers.ts. The type is renamed DeclaredInterface - NodeInterface collided with CalmNodeInterface, which already means a relationship endpoint. No signature or behaviour change. Both rules still compose the document path themselves, because the reader sees a candidate's schema without knowing where that schema sits.
The opening line issued an authoring directive - treat allOf as unsupported for nodes and relationships - in a maintainer-facing doc that states it describes behaviour rather than prescribing it. It was also broader than the rule it restated: pattern-creation.md scopes "unsupported" to relationships, with a reason, and no in-repo pattern uses allOf for either property. State the fact instead, and point at the author-facing guide. Also correct the table: the TEMPORARY marker sits on the private resolveArrayContainer, not on getPatternArray.
…differ The table listed three readers and their behaviour but not who calls them or what forces each to differ, which is the question it raises. Add the calling surfaces and the constraint behind each choice: the merge must hand instantiate one schema, getPatternArray must return a location and runs on the raw pattern, and the candidate readers must report a path the document actually contains. Add the observable consequence - on one pattern whose nodes live only in an allOf branch, extractOptions finds the decision while listDeclaredCandidates reports no candidates at all.
Each fix below was checked by running the code, not by reading it. The optionId collision was described as making only the first question block addressable. It depends on the path: interactively both questions are asked and both answers applied, and only the logged --option-choices replay string is lossy; loadChoicesFromInput resolves with find, so the second block is unreachable there. "Unreachable today" read as a property of the format. It is a property of this repository's patterns - option-type in the meta-schema is an unbounded array of decision objects, so a holder is designed to ask several questions. The allOf table listed assertChoicesAreSelectable as a caller exposed to the split. It runs after flattenAllOf, so it never meets an allOf; the invisibility is validation's alone. The header said the doc never picks a winner, while "Which side changes" picks one. Allow it where only one answer is coherent.
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.
Description
Lets a CALM pattern declare "zero or more of these candidates" through a JSON Schema
items.oneOf/items.anyOfopen catalog, alongside the existingprefixItemspositional slots. No meta-schema change is needed —itemsis standard JSON Schema.Closes #2859.
The reason this touches so much is that three surfaces read pattern decisions independently, and all three were
prefixItems-only: validation (shared/src/spectral), generation (shared/src/commands/generate) and visualisation (calm-hub-ui). Nothing made them agree, which is where most of the defects below came from. There is now a fixture contract intest_fixtures/decision-agreement/that bothsharedandcalm-hub-uiassert against, so the two cannot drift apart silently again.Decisions worth knowing about
A decision holder must live in
properties.relationships.prefixItems. Candidates belong in the catalog; the decision that selects them does not. A holder placed in a catalog instead makes the decision itself optional, which is incoherent, socalm validatenow rejects it — a new pattern rule, not a generate-time check.calm generatenever validates, so a user who only generates still gets silence: the decision is simply never offered, same as before. The rule is the fix; enforcement is one command away from where the mistake was made, and that's documented, not accidental.calm generatenever validates, so it carries its own guard. Choosing a candidate that selection cannot reach now throws instead of quietly producing an architecture missing what you asked for.A candidate can be drawn in one box only. When two decisions name the same candidate, the first keeps it and the second still renders with what is left. Boxing one node twice needs the nesting rework in #2933.
A decision whose every candidate shares one container loses its box, and its prompt with it. Container membership beats decision-group membership when a node is both, so a
oneOf/anyOfbetween two services deployed into the same container renders no box at all — the choice becomes invisible in the diagram, not just unstyled. Deliberate and directly tested, and the flat interim#2933's nesting rework replaces: nesting the box inside the container is the real fix, not something this PR attempts.allOffor nodes and relationships remains unsupported. Three parts of the system read it differently, all deliberately. Documented inshared/AGENTS.md; convergence is separate work.The CLI and the visualiser already disagreed on a decision holder declaring both
oneOfandanyOf, before this PR. Confirmed againstmain: the CLI offers every choice from both keywords, the visualiser draws only theoneOfhalf. Neither reader was touched here beyond the visualiser's hand-rolled check being swapped for the sharedreadChoiceBlock— same rule, same result. Not this PR's to resolve; documented inshared/AGENTS.md's "Still duplicated" table.Behaviour changes
unique-idin anitemscatalog errorsunique-idin aoneOf/anyOfslot errorsoneOfandanyOfwarnsanyOfcandidates were unreachablecalm generaterejects an unreachable answerinstantiateemits[]for an array with noprefixItems{}, invalid where an array is requiredcalm validatechecks every selected answer of a multi-answer (anyOf) decisionThe dual-keyword row above is coverage plus a message fix, not a bug fix: the rule already caught this on a
prefixItemsslot before this PR touched it, and catching it on anitemscatalog too was the intended expansion — only its wording said "items catalog" for both cases, so aprefixItemsfinding read as a false description of where the problem was.The multi-answer row above is a strictness increase, not just new coverage: an architecture with a bad second answer used to validate clean and will now fail. Worth calling out in the release note alongside the pattern-side changes, since it can surface on an architecture nobody has touched.
Nothing in this repo uses
itemscatalogs today, so nothing shipped regresses. Externally authored patterns carrying any of the newly detected faults will start failingcalm validate. Worth a release note on theshared/CLI bump.Fixed since first review
Two independent review passes surfaced defects this PR introduced after it was opened; all are fixed on this branch now. (The multi-answer validation fix is the same class of finding, but it earns its own row in the behaviour table above, not a line here — it's a strictness increase worth the extra visibility.)
nodes/relationshipsarray from two differentallOfbranches, describing an array no single declaration site actually contains.calm diff, the CALM Hub's visual pattern diff, and the VSCode pattern preview were all blind toitems-catalog candidates: a catalog addition or removal produced no diff at all, and a catalog-only pattern previewed as empty. All three now see catalog candidates the same way generation and validation already did.calm validatewith an internal schema-compilation error, because the narrowed pattern ended up with an emptyprefixItems, which is not legal JSON Schema. "Pick none from this catalog" is this feature's own advertised case, so the decision holder is now dropped instead of narrowed to empty.Deferred
Nesting decision boxes and containers stays in #2933, along with the case where two decisions share every candidate. The container-precedence behaviour here is the flat interim that #2933 supersedes.
Type of Change
Affected Components
cli/)calm/)calm-ai/)calm-hub/)calm-hub-ui/)calm-server/)calm-widgets/)docs/)shared/)calm-plugins/vscode/)Testing
Checklist