finos / finos/architecture-as-code

Pattern diff reports a node as unchanged when it stops being guaranteed

Open
#3,111 0 comments 0 reactions 1 assignee Claimed by @YoofiTT96 View on GitHub
Dominant language
TypeScript
Stars
399
Forks
138
Avg merge
3d 21h
Merged PRs (30d)
42

Description

## Summary

A pattern can declare a node in three ways, and they promise different things to anyone building an architecture from it. A bare `prefixItems` entry is always built. An alternative inside a `oneOf` or `anyOf` is built only if chosen. `calm diff` cannot tell these apart, so moving a node between them is reported as no change.

## Reproduction

Version 1 guarantees `cache`. Version 2 makes it one of two alternatives, so an architecture may get `queue` instead:

```
v1 nodes.prefixItems = [ webapp, cache ]
v2 nodes.prefixItems = [ webapp, { oneOf: [ cache, queue ] } ]
```

Verified against `main`:

```
bare -> alternative : added=queue removed=- same=webapp,cache
alternative -> bare : added=- removed=queue same=webapp,cache
```

`cache` is reported unchanged in both directions. Only `queue` appearing or disappearing is noticed.

## Why it matters

`hasChanges` is what `calm diff --exit-code` returns, and it counts only the add, remove, modify and rename buckets. A node that merely changed obligation lands in `nodesSame`, so a CI gate on pattern changes passes.

The second direction is the damaging one. Making a previously optional node guaranteed invalidates every existing architecture that declined it, and the report says nothing changed.

## Cause

`expandAlternatives` in `calm-models/src/diff/pattern-diff.ts` flattens a `oneOf`/`anyOf` entry into its alternatives and discards the wrapper. By the time the comparison runs, a bare entry and an alternative are the same shape.

## Scope

| Affected | Verified |
|---|---|
| Nodes, `oneOf` | yes |
| Nodes, `anyOf` | yes |
| Relationships, `oneOf` | yes — `r1` reported as `edgesSame` |

## Note on naming

A fix should not use "mandatory" and "optional" as the two states. Once `items` catalogues are read (#2859), there are three levels of obligation, not two, and an alternative and a catalogue member are both optional in different senses. A pair meaning "became less guaranteed" and "became more guaranteed" describes all three.

## Not in scope

Reading `items` catalogues in the differ. That is #2859, and it is a separate defect — the catalogue is not read at all, so a member appearing is invisible. This issue is about obligation, and it reproduces on `main` with no `items` involved.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.