Repository navigation
fix(vscode): build relationships that name the nodes the picker built - #3113
Conversation
github-actions
Bot
added
cli
5979c49 to
83089e7
Compare
| function declarations(field: any): any[] { | ||
| const catalogue = field?.items; | ||
| const offersChoice = catalogue?.oneOf?.length || catalogue?.anyOf?.length; | ||
| return [...(field?.prefixItems ?? []), ...(offersChoice ? [catalogue] : [])]; |
There was a problem hiding this comment.
The picker takes a node from the node catalogue and a relationship from the relationship catalogue separately. The relationship it adds can point to a node it did not add.
Example: a pattern lists cache then queue in its node catalogue, and gateway-connects-queue then gateway-connects-cache in its relationship catalogue. The picker adds the node cache and the relationship gateway-connects-queue, which points to queue. That node does not exist. The same happens when the first node member is only a $ref, because the picker drops that member.
Result: the canvas cannot draw that link, and the next save drops the relationship with no message to the author.
This is new in this PR: before, the picker did not read catalogues. calm generate avoids it because it takes catalogue members only from a decision that names the node and its relationships together.
There was a problem hiding this comment.
Confirmed, and the PR has changed shape because of it.
One correction: it is not new here. main produces the same dangling reference through prefixItems. Running main's own instantiateFromPattern over a node entry offering cache or queue and a relationship entry offering gateway-connects-queue or gateway-connects-cache:
nodes: gateway, cache
relationships: gateway-connects-queue
it names: gateway, queue
This PR added a second site for the same fault rather than the fault.
Two changes, in 8766545:
The picker now takes the relationship alternative whose endpoints it built, falling back to the first when none fit. It also skips an alternative it cannot build, such as a bare $ref, instead of taking it and discarding the result — that case yielded no nodes at all while the relationships were still built.
The catalogue reading is gone. PATTERN-DECISIONS.md says a catalogue is what an architecture may add and that calm generate builds no member a decision does not name, so the picker holds the same line. That removes the feature this PR was opened for, so it is now a fix and no longer part of #2859.
The defect is tracked as #3196, since it is reachable on main today.
One thing your comment points at that I have not decided here: what the picker is for. It writes to the author's file like calm generate, but calm generate asks which alternative through --option-choices and the picker does not. Nothing records which it is meant to be — #2869 does not mention patterns, #2912's description does not mention it, and it appears in no documentation. Worth your view before the picker grows any further.
The picker read the node list and the relationship list separately and took the first alternative of each, so it could build a relationship naming a node it had not built. The canvas cannot draw that link, and Apply Pattern merges it into the architecture the author already has open. It now takes the relationship alternative whose endpoints it built, falling back to the first when none fit. It also skips an alternative it cannot build, such as a bare $ref, which previously left the position empty while the relationships naming it were still built. Reading items catalogues is dropped. PATTERN-DECISIONS.md records a catalogue as what an architecture may add, and calm generate builds no member a decision does not name.
…nto feat/2859-vscode-picker-items
YoofiTT96
changed the title
Description
The VS Code pattern picker builds an architecture from a pattern. It writes that architecture into the author's file.
A pattern can offer a choice. The picker read the node list and the relationship list separately. It took the first alternative of each list. Nothing connected the two choices. The picker could therefore build a relationship that names a node it did not build.
The canvas cannot draw that link. "Apply Pattern" merges the broken relationship into the architecture that the author has open.
Fixes #3196.
What changed
The picker now takes the relationship alternative whose endpoints it built. If no alternative fits, it takes the first one. A position is therefore never empty.
The picker also skips an alternative that it cannot build, such as a bare
$ref. Before this change, that case gave no nodes. The relationships that name those nodes were still built.This PR no longer adds
itemssupportThis PR was the fifth in the #2859 stack. It added catalogue reading to the picker.
The review showed a problem. The picker chooses a catalogue node and a catalogue relationship independently. That is what makes the broken link.
PATTERN-DECISIONS.mddecides this differently. A catalogue is what an architecture may add.calm generatebuilds no member that a decision does not name. The picker now keeps the same rule and builds nothing from a catalogue. It therefore agrees with the CLI.The picker leaves the scope of #2859. The other four PRs cover
itemsfor validation, generation, the pattern diagram and the diff.The defect is older than this PR
mainmakes the same broken reference throughprefixItemsalternatives. The fix acts on the choice, not on one declaration site. It therefore covers both sites.Still open
No document records the purpose of the picker. The picker writes to the author's file, as
calm generatedoes.calm generateasks which alternative the author wants, through--option-choices. The picker does not ask.#2869 does not mention patterns. The description of #2912 does not mention the picker. No documentation mentions it. A maintainer should decide this before the picker becomes larger.
A pattern can offer no coherent combination. Such a pattern still gives a broken reference. That is a defect in the pattern, not in the picker.
Why the function moves out of the JSX
vitest.config.tssetsenvironment: 'node'. The package therefore cannot render a React component in a test.instantiateFromPatternwas private toPatternPicker.tsx. It moves topattern-instantiation.ts. This matches theBuildingBlockCreator.tsxandbuilding-block-doc.tspair beside it.Type of Change
Affected Components
cli/)calm/)calm-ai/)calm-hub/)calm-hub-ui/)calm-server/)calm-widgets/)docs/)shared/)calm-plugins/vscode/)Testing
166 tests pass. There are no lint errors. The extension builds.
Each part of the fix fails a test when it is reverted:
deployed-in,interactsandcomposed-ofare removed from the endpoints that are readA pattern that offers no choice gives the same result as before.
calm/getting-started/conference-signup.pattern.jsongives the same 164 lines. The fix acts only where a pattern offers alternatives.Checklist
🤖 Generated with Claude Code