feat(shared): read items in pattern validation - #3060
Merged
markscott-ms merged 12 commits intoSep 17, 2026
Merged
Conversation
YoofiTT96
force-pushed
the
feat/2859-validation-items-catalog
branch
from
September 8, 2026 09:47
96d9ccd to
e5aac0f
Compare
YoofiTT96
force-pushed
the
feat/2859-validation-items-catalog
branch
2 times, most recently
from
September 11, 2026 11:10
e858e3e to
8b6ea9d
Compare
YoofiTT96
changed the title
YoofiTT96
force-pushed
the
feat/2859-validation-items-catalog
branch
from
September 11, 2026 11:18
8b6ea9d to
9624194
Compare
This was referenced Sep 11, 2026
A pattern may now declare nodes and relationships under items as well as prefixItems, so an architecture can add members rather than only choose between them. declaration-paths.ts gains the two items sites, and every rule that resolves declarations picks them up without an edit of its own, because none of them names a site. items members are not mutually exclusive. Verified with Ajv: items admits any number of matching elements, so two members can both be built, while a prefixItems entry is one position and exactly one of its alternatives is built. Two items members must therefore not share a unique-id or an interface id, and the grouping helper is renamed exclusiveGroup to say which declarations actually compete. Adds pattern-decision-must-be-declared-in-prefix-items. An architecture contains every relationship declared in prefixItems, so a decision there is always asked. An items member may be left out, so a decision declared in items can vanish and an answer never gets to decline it. pattern-prefix-items-must-declare-one-keyword becomes pattern-choice-must-declare-one-keyword, because it now covers items blocks as well as prefixItems entries. Additive: every tracked pattern in the repository produces the same findings as before, because none of them uses items yet.
Duplicate ids were blamed on the earlier declaration whenever the pair spanned prefixItems and items, because nothing ranked the two sites. That broke the contract 960fa4f set, which is that the rule names the later declaration. Ordering is a property of the site: prefixItems fills the first positions and items fills everything after. The rank joins the parsed indices that byBuildOrder already compares. Every ordering test compared two declarations at the same kind of site, which is why none of them caught it. Adds three that span both. Also records two limits the contract overstated. A decision declared as one alternative of an entry can vanish just as one declared in items can, and calm validate reports only the items case. A node declared under items must sit inside oneOf or anyOf, because items applies one schema to every position after the entries. Renames entryPath to fixedPath so it pairs with fixedIdPath, and states isAlternative against the regex rather than deriving it from two other helpers.
maxItems counts the whole array and the prefixItems entries fill it from the front, so an items member can only be built in what is left over. A pattern that pins maxItems to the number of entries can never build one, and calm generate produced an architecture that failed its own pattern on maxItems. The narrowed pattern cannot fix this. calm generate does not record a decision's options in the architecture, so extractChoicesFromArchitecture finds nothing and validation runs against the pattern as written.
An items block cannot limit how many times one member is used, so two positions may both match it and the architecture holds one unique-id twice. The pattern is correct, so calm validate cannot report it; unique-ids-must-be-unique-in-architecture reports it later. prefixItems is positional, so an architecture lists those elements first and in order. Pre-existing, but items is the first construct that makes an author think about array positions. minItems: 0 is rejected, because pattern-has-no-empty-properties reads a zero as a placeholder. Zero is the default, so the keyword can be dropped. Issue finos#2859 writes the catalog with minItems: 0.
Both halves apply to the same element. If each pins a unique-id, no element satisfies both and the position can never be filled. Verified with Ajv, and calm generate still emits the selected alternative, so it produces an architecture that fails its own pattern. The note claimed the duplicate-id rule catches the common case. Sharing an id is not the common case, and that rule reports a duplicate rather than the loss.
The path has no wildcard, so a lodash read off a named constant does the same work and keeps the marker's location beside the id's.
YoofiTT96
force-pushed
the
feat/2859-validation-items-catalog
branch
from
September 15, 2026 22:05
e57502a to
df040a7
Compare
28 tasks
YoofiTT96
marked this pull request as ready for review
September 17, 2026 10:07
YoofiTT96
requested review from
LeighFinegold,
Thels,
aidanm3341,
grahampacker-ms,
jpgough-ms,
lbulanti-ms,
rocketstack-matt and
willosborne
as code owners
September 17, 2026 10:07
markscott-ms
requested changes
Sep 17, 2026
markscott-ms
left a comment
markscott-ms
left a comment
Contributor
There was a problem hiding this comment.
General problems in this PR:
shared/src/spectral/functions/pattern/is-defined-in-oneof-or-anyof.spec.ts
● 1 [test-coverage] No test exercises a decision option legitimately referencing a node/relationship declared under items
General notes (some for later PR, recognising that this one only targets spectral validation rules)
calm-hub-ui/src/visualizer/components/reactflow/utils/patternTransformer.ts
● 32 [correctness] Pattern visualizer's getPrefixItems only reads prefixItems, never items, so items-declared nodes/relationships vanish from the rendered graph
calm-models/src/diff/pattern-diff.ts
● 50 [correctness] diffPatterns only inspects prefixItems, never items, when normalising patterns for comparison
calm-plugins/vscode/src/webview/panels/PatternPicker.tsx
● 66 [correctness] VSCode PatternPicker's instantiateFromPattern/extractValue only recurse into prefixItems, never items
A decision holds its answers in an options array, and calm generate reads options prefixItems alone. Nothing stopped an author declaring the option blocks in options items instead, where validation passed, generation ignored them, and the decision lost its answers with no message. The four rules that read decision options all select options prefixItems, so widening each to reach options items would validate a shape the generator cannot build. The new rule rejects the shape instead. itemsFitWithinMaxItems treated an empty oneOf or anyOf as a choice, because an empty array is truthy. It reported that maxItems left no room for a member the block never declared. Raised in review on finos#3060.
…og' into feat/2859-validation-items-catalog
Nothing asserted that a decision option can name a node or relationship declared in an items catalogue, so the guarantee was free to regress. The permissive half alone would pass trivially, because the rule reports only when the referenced id is a fixed prefixItems entry. The fixed-entry case is pinned beside it so both sides of the condition are covered. Raised in review on finos#3060.
The fixture pins the rule inventory, so adding a rule changes both totals and the rule list.
markscott-ms
requested changes
Sep 17, 2026
markscott-ms
approved these changes
Sep 17, 2026
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
A pattern can now declare nodes and relationships under
itemsas well asprefixItems, so an architecture may add members rather than only choose between them.calm validatereads the new sites; generation, visualisation and the pattern differ follow separately.Stacks on #3059, which exists because building this work is what exposed the
prefixItemsgaps. GitHub will not let a cross-fork PR target a branch in my fork, so the base ismainand the diff carries #3059's commits. The commits to review are the last six.Part of #2859.
The decision worth knowing
itemsmembers can coexist;prefixItemsalternatives cannot. AprefixItemsentry is one position, so one alternative wins.itemscovers every position after it, so any number of its members can be built.That is what decides the uniqueness rules: two
itemsnodes must not share aunique-idor an interface id, because both can appear. Two alternatives of one entry may.exclusiveGroup()is where this lives.Gotcha
calm generatedoes not readitemsyet. An author can declare anitemsnode, havecalm validateaccept the pattern, and get an architecture without that node, with nothing said. It closes when the generation PR lands, andPATTERN-DECISIONS.mdrecords it meanwhile.Rule changes
pattern-decision-must-be-declared-in-prefix-itemsis new. Anitemsmember may be left out, so a decision declared there can vanish and an answer never gets to decline it.pattern-items-must-fit-within-max-itemsis new.maxItemscounts the whole array and theprefixItemsentries fill it from the front, so a pattern that leaves no room declares a member nothing can ever build.pattern-prefix-items-must-declare-one-keywordbecomespattern-choice-must-declare-one-keyword, since it now coversitemsblocks too.Ordering
Duplicate ids are blamed on the later declaration. Nothing ranked the two sites, so a pair spanning
prefixItemsanditemswas blamed backwards. Ordering is a property of the site —prefixItemsfills the first positions anditemsfills everything after — and that rank now joins the parsed indicesbyBuildOrdercompares. Every existing ordering test compared two declarations at the same kind of site, which is why none caught it; three that span both are added.Tidying
decisionIsDeclaredInPrefixItemsasked whether a relationship carries options with a JSONPath query over a path that has no wildcard. It now reads that path with lodash off a named constant, beside the one that locates aunique-id. Raised in review on #3059 against the same pattern elsewhere.Type of Change
Additive. No pattern in the repository changes result, because none of them uses
itemsyet.Affected Components
cli/)calm/)calm-ai/)calm-hub/)calm-hub-ui/)calm-server/)calm-widgets/)docs/)shared/)calm-plugins/vscode/)cli/is the JUnit fixture only. It carries two counts, atestsuitestotal and aSpectral Suitetotal, and both move.Commit Message Format ✅
feat(shared): read items in pattern validationfix(shared): order items declarations after prefixItems entriesfeat(shared): reject maxItems that leaves no room for itemsrefactor(shared): read the decision marker without JSONPathTesting
161 spectral tests, 659 CLI tests including the end-to-end suite,
sharedat 107 files and 1238 tests, rootnpm testgreen. Every tracked pattern file was run against the real ruleset onmainand on this branch; the findings are identical.Rebased onto #3059 after its review round, so the ordering work here builds on the parsed-index comparator that landed there rather than on the string key it replaced.
Checklist