fix(cli): include flows in architecture diffs by ASVLCII · Pull Request #3046 · finos/architecture-as-code

ASVLCII · 2026-09-01T09:53:49Z

Description

Fixes #2998. Include flows in architecture diffs, JSON output, summaries and --exit-code. Match flows by unique-id; report missing IDs under invalidItems so they cannot silently pass the change gate.

Addresses the outstanding review: close the metadata JSDoc, remove the duplicate spread, preserve invalid nodes and relationships when composing flow results, use fixture-based tests, and update the CLI documentation. Pattern diffs retain optional flow fields.

AI-assisted: Codex helped restore, review and verify this patch.

Type of Change

Bug fix
Documentation update
Test additions or updates

Affected Components

CLI (cli/)
CALM Lab (calm-lab/)
CALM Models (calm-models/)
Shared (shared/)

Commit Message Format

Commits use Conventional Commits. Existing commits and authorship are preserved; upstream updates are merged without rewriting history.

Testing

I have tested my changes locally
I have added/updated unit tests
All existing tests pass

Verified on GitHub-hosted Linux runners with Node 26.3.1 and Java 21, not the Windows host:

Focused model/shared/packed-CLI/lab suites: 191 passed.
Full TypeScript workspace run: 8,030 passed, two TODOs, no failed assertions. The coverage command exits non-zero for the four baseline shortfalls below.
Full TypeScript build passed after building the existing MCP dependency before the GitHub Action workspace. Build and full-suite results.
Maven reactor clean install and CALM Hub verify passed; the latter reports 3,828 passed with no failures or skips. Java job. The overall run is red because its initial focused-test harness applied package-wide coverage thresholds; that harness was corrected in the later runs.
Full workspace lint passed with zero errors and 11 warnings; CALM Lab typecheck and upstream-relative diff checks passed. Final comparison and lint run.

Coverage limitations: @calmstudio/calm-core, @calmstudio/extensions, @calmstudio/mcp and @calmstudio/studio fall below their configured coverage thresholds. All four failures reproduce with identical percentages on unmodified upstream 8bac0ba7; no thresholds were changed. The full-test checkbox remains unchecked because the coverage command is not green. The verification-only workflow is not part of this PR.

Checklist

My commits follow the conventional commit format
I have updated documentation if necessary
I have added tests for my changes
My changes follow the project's coding standards

Signed-off-by: ASVLCII <308706627+ASVLCII@users.noreply.github.com>

linux-foundation-easycla · 2026-09-01T09:53:57Z

The committers listed above are authorized under a signed CLA.

✅ login: ASVLCII / name: ASVLCII (d26b2cc)

ASVLCII · 2026-09-01T16:31:02Z

All checks and EasyCLA are green now. Feel free to merge when you're ready.

aamanrebello · 2026-09-03T10:40:28Z

I am happy with these changes, I would also want the opinion of Matt Bain on this though.

ASVLCII · 2026-09-05T10:31:22Z

Sounds good, happy to wait for Matt's review. The fixture move I mentioned is still outstanding on my side.

rocketstack-matt

Design and tests look right — flows are matched by unique-id and correctly wired into hasChanges/summary/JSON output. Two things before merge: this branch conflicts with main (a parallel PR added MetadataDiffResult to the same ArchitectureDiffResult union), and the CLI README's diff section doesn't mention flows yet (it still documents/shows only Nodes/Relationships in the --format summary output).

rocketstack-matt · 2026-09-06T09:33:10Z

    controlItemsModified: { [controlId: string]: ControlItemDiffResult };
 }

+export interface FlowDiffResult {


Nit: this is the only composed result interface here without a doc comment (AdrDiffResult/ControlDiffResult above both have one).

rocketstack-matt · 2026-09-26T14:50:04Z

@ASVLCII are you able to resolve the conflicts and add the doc comment?

ASVLCII · 2026-09-27T07:01:23Z

Resolved the conflict with current main, including both flow and metadata diff results. Added the FlowDiffResult doc comment, moved flow cases into diff-test-architectures.json, and updated the CLI summary docs to include flows. The focused diff suites pass locally with 89 tests, and the scoped diff check is clean. The merge brings in whitespace warnings from unrelated upstream files, which I left unchanged. CI is running on the update.

rocketstack-matt

The build is broken by an unclosed doc comment in diff-types.ts, which is why every CI job is red. Everything else is small.

rocketstack-matt · 2026-09-27T16:11:42Z

+    flowsSame: CalmFlowSchema[];
+}
+
+/**


This /** never closes, so MetadataDiffResult is swallowed by the comment and every CI job fails on TS2552: Cannot find name 'MetadataDiffResult'. Restoring the original comment also completes the doc-comment nit above.

Suggested change

/**

/**

* Represents the result of diffing two CALM Metadata Schema objects.

*/

rocketstack-matt · 2026-09-27T16:11:42Z

+        ...flowsDiff,
+        ...flowsDiff,


flowsDiff is spread twice. Harmless at runtime, but it looks like a merge artefact.

Suggested change

...flowsDiff,

...flowsDiff,

...flowsDiff,

rocketstack-matt · 2026-09-27T16:11:42Z

    };
 }

+export function diffFlows(flowsA: CalmFlowSchema[], flowsB: CalmFlowSchema[]): FlowDiffResult {


Flows without a unique-id all collapse onto one Map key, so a removed id-less flow can be reported as unchanged: two id-less flows on side A and one on side B gives flowsSame: 1, flowsRemoved: 0, and --exit-code passes. diffNodesAndRelationships guards the same case by surfacing id-less items under invalidItems so they count as a change. Happy for this to be a follow-up issue if you'd rather keep this PR tight.

(non-blocking nit) The other exported sub-diffs all carry a JSDoc; this one doesn't.

Suggested change

export function diffFlows(flowsA: CalmFlowSchema[], flowsB: CalmFlowSchema[]): FlowDiffResult {

/**

* Core diff function for CALM flow arrays. Flows are matched by `unique-id` and classified as added, removed, modified or unchanged.

*/

export function diffFlows(flowsA: CalmFlowSchema[], flowsB: CalmFlowSchema[]): FlowDiffResult {

rocketstack-matt · 2026-09-27T16:11:42Z

+      {
+        "unique-id": "read-flow",
+        "name": "Read flow",
+        "description": "Writes a record.",


(non-blocking nit) read-flow still says "Writes a record."

Suggested change

"description": "Writes a record.",

"description": "Reads a record.",

rocketstack-matt · 2026-10-02T14:25:42Z

Closing due to inactivity - please feel free to reopen if you get the chance to look back into this @ASVLCII

ASVLCII · 2026-10-05T12:12:50Z

I missed your September 27 review, @rocketstack-matt. Sorry about that. The unclosed doc comment is a real build break, and I also missed the duplicate flowsDiff spread and the id-less flow case you flagged. I'll work through those and the remaining doc nits, then rerun the checks before reopening. The earlier local result doesn't cover the review findings.

aamanrebello · 2026-10-09T05:07:20Z

@ASVLCII we are having some requests to get this feature up and running, any chance we could step up the timeline?

ASVLCII · 2026-10-09T11:03:16Z

Yep @aamanrebello, I owe you the fixes and let this sit too long. I'm prioritizing the outstanding review now: the broken doc comment, duplicate flowsDiff spread, and handling for flows without IDs. I'll rerun the build and regression tests before reopening, then post the results here. Sorry for the delay.

aamanrebello · 2026-10-27T15:06:04Z

@ASVLCII a small reminder

ASVLCII · 2026-10-29T07:51:05Z

@aamanrebello @rocketstack-matt, sorry for the delay. The review fixes are pushed in 868fdbc, and the branch is current with main. The broken JSDoc, duplicate spread and missing flow IDs are addressed.

Verification results are in the PR description, including the four coverage shortfalls reproduced unchanged on upstream. The separate verification workflow is not in the patch.

GitHub reports viewerCanReopen: false for my account, so I couldn't reopen this PR. Could one of you reopen it? The updated branch is ASVLCII:fix/diff-flows.

fix(cli): include flows in architecture diffs

d26b2cc

Signed-off-by: ASVLCII <308706627+ASVLCII@users.noreply.github.com>

ASVLCII requested review from a team, LeighFinegold, Thels, aidanm3341, grahampacker-ms, jpgough-ms, lbulanti-ms, markscott-ms, rocketstack-matt and willosborne as code owners September 1, 2026 09:53

github-actions Bot added the shared label Sep 1, 2026

aamanrebello reviewed Sep 2, 2026

View reviewed changes

Comment thread shared/src/commands/diff/diff-core.ts Resolved

aamanrebello reviewed Sep 2, 2026

View reviewed changes

Comment thread calm-models/src/diff/diff.spec.ts Resolved

rocketstack-matt reviewed Sep 6, 2026

View reviewed changes

fix(cli): include flows in architecture diffs

593acc5

github-actions Bot added the cli Affects `cli` code label Sep 27, 2026

rocketstack-matt requested changes Sep 27, 2026

View reviewed changes

rocketstack-matt mentioned this pull request Oct 2, 2026

calm diff summary and --exit-code ignore ADR, control and metadata changes #3210

Closed

rocketstack-matt closed this Oct 2, 2026

fix(cli): address flow diff review and merge upstream

fd0a803

fix(cli): merge current upstream for flow diff

868fdbc

aamanrebello reopened this Nov 6, 2026

Merge branch 'main' into fix/diff-flows

262deb7

-/**
+/**
+ * Represents the result of diffing two CALM Metadata Schema objects.
+ */

	"description": "Writes a record.",
	"description": "Reads a record.",

Conversation

ASVLCII commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of Change

Affected Components

Commit Message Format

Testing

Checklist

Uh oh!

linux-foundation-easycla Bot commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ASVLCII commented Sep 1, 2026

Uh oh!

Uh oh!

Uh oh!

aamanrebello commented Sep 3, 2026

Uh oh!

ASVLCII commented Sep 5, 2026

Uh oh!

rocketstack-matt left a comment

Choose a reason for hiding this comment

Uh oh!

rocketstack-matt Sep 6, 2026

Choose a reason for hiding this comment

Uh oh!

rocketstack-matt commented Sep 26, 2026

Uh oh!

ASVLCII commented Sep 27, 2026

Uh oh!

rocketstack-matt left a comment

Choose a reason for hiding this comment

Uh oh!

rocketstack-matt Sep 27, 2026

Choose a reason for hiding this comment

Uh oh!

rocketstack-matt Sep 27, 2026

Choose a reason for hiding this comment

Uh oh!

rocketstack-matt Sep 27, 2026

Choose a reason for hiding this comment

Uh oh!

rocketstack-matt Sep 27, 2026

Choose a reason for hiding this comment

Uh oh!

rocketstack-matt commented Oct 2, 2026

Uh oh!

ASVLCII commented Oct 5, 2026

Uh oh!

aamanrebello commented Oct 9, 2026

Uh oh!

ASVLCII commented Oct 9, 2026

Uh oh!

aamanrebello commented Oct 27, 2026

Uh oh!

ASVLCII commented Oct 29, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ASVLCII commented Sep 1, 2026 •

edited

Loading

linux-foundation-easycla Bot commented Sep 1, 2026 •

edited

Loading