feat(calm-suite): Add comprehensive support for multi-doc and complex governance relationships#2740
Conversation
eddie-knight
requested review from
a team,
gjs-opsflo,
jpgough-ms,
markscott-ms,
opsflowanoop and
rocketstack-matt
as code owners
… sidecar The CalmStudio editor gains the gemara/governance and multi-document feature set: - bind Gemara controls/guidance to CALM via grc.store-backed decorators (YAML/JSON) - connected AIGF<->CCC Governance view, one-click Apply governance, CALM 1.2 attest - link-based C4 cross-document navigation (drill on double-click + breadcrumb), plus a "Create new linked document" button that scaffolds a child doc from a node - decorators persisted as a standalone *.decorators.json sidecar (split from the architecture document), document-scoped decorator panel, split JSON viewer, .zip export - diagram layout + editable document title persisted in CALM metadata - AIGF Multi-Agent Reference Architecture template as a C4 document series Signed-off-by: Eddie Knight <knight@linux.com>
Bring CALMGuard onto the canonical nested relationship-type form for cross-tool interop with CalmStudio, updating the extractor/normalizer/types, examples, and tests. Signed-off-by: Eddie Knight <knight@linux.com>
…file) Add a Makefile to launch the suite apps, build @finos/calm-models before CALMGuard in CI, and update the root lockfile for the new Studio/Guard dependencies. Signed-off-by: Eddie Knight <knight@linux.com>
eddie-knight
force-pushed
the
feat/calmstudio-gemara-controls
branch
from
6337eef to
7b168f4
Compare
initcron
left a comment
initcron
left a comment
There was a problem hiding this comment.
Substantive PR — the canonical-CALM 1.2 discipline is genuinely impressive. attestControlOnNode writing controls['ai-governance'] with positive AND negative tests (attest.test.ts:39,41 — the negative assertion that air-prev-002 is NOT a framework-prefixed key is exactly the right defense), details.detailed-architecture honoring the canonical core.json:56-67 shape, and the Export-only discipline from #2691 explicitly preserved with a contributor warning at io/export.ts:92-96. The reference Multi-Agent Reference Architecture series validates cleanly against calm validate. Big build of well-tested work.
One blocker, two maintainer questions, and a few non-blocking notes.
🔴 Blocker
B-1: Cmd+Z keyboard undo from canvas does NOT restore governance
There are two divergent undo wirings:
calm-suite/calm-studio/apps/studio/src/lib/canvas/CalmCanvas.svelte:471-479 (the @svelte-put/shortcut Cmd+Z handler bound to the canvas):
```svelte
function handleUndo() {
if (readonly) return;
const snapshot = undo();
if (snapshot) {
nodes = snapshot.nodes;
edges = snapshot.edges;
applyFromCanvas(nodes, edges); // ← rebuilds model from flow projection
}
}
```
calm-suite/calm-studio/apps/studio/src/routes/+page.svelte:957-963 (the menu-bar / Tauri handler):
```svelte
undo: () => {
const snapshot = undo();
if (snapshot) {
applyFromJson(snapshot.model); // ← restores decorators/controls
nodes = snapshot.nodes;
edges = snapshot.edges;
}
},
```
applyFromCanvas rebuilds via flowToCalm(nodes, edges) and merges with the prior model. But decorators are explicitly NOT in flow projection data — see DecoratorBadge.svelte:7-9 ("the canvas node/edge data does not carry decorators"). So a keyboard Cmd+Z fires after a governance attestation will silently fail to restore decorators / node-level controls.
Commit 866a4472 ("make governance changes undoable") + history.test.ts:144-167 capture and exercise the model snapshot — but the test calls applyFromJson(snap!.model) directly. It doesn't drive the CalmCanvas.handleUndo keyboard path that most users will hit.
Action: Either (a) make CalmCanvas.handleUndo/handleRedo apply snapshot.model too (mirror +page.svelte's wiring), then add a test that drives the keyboard handler against a governance mutation; or (b) confirm the canvas Cmd+Z is unreachable in practice (e.g. suppressed when GovernanceView has focus) and document why. Until one is true, the PR body's claim "governance changes are undoable" holds for the menu path but breaks for the keyboard path.
🟡 Important
I-1: metadata.name and metadata.c4-level are not namespaced
calm-suite/calm-studio/apps/studio/src/lib/io/documentName.ts:20 uses NAME_KEY = 'name'; calm-suite/calm-studio/apps/studio/src/lib/c4/c4Documents.svelte.ts:52 uses 'c4-level'. Compare to metadata.calmstudio-layout which IS correctly calmstudio--prefixed.
CALM 1.2 metadata has additionalProperties: true so this is spec-valid today, but if CALM 1.3 adopts a top-level document name or a C4-level concept, the unprefixed keys risk collision. Recommend either:
- (a) rename to
calmstudio-nameandcalmstudio-c4-levelfor consistency withcalmstudio-layout, or - (b) open spec issues / ADRs proposing
nameandc4-levelas canonical CALM 1.3 vocabulary.
Don't silently squat on bare names — that's the kind of decision that becomes load-bearing once docs are in the wild.
Questions for the maintainer (not blocking, but worth a deliberate call)
Q-1: Should the three themes be split?
The PR body itself organises the work into three numbered themes (Gemara↔CALM governance, link-based C4 navigation, doc UX+persistence). The 57 commits cleanly bucket into those three (~22 / ~16 / ~12 / ~7 misc). Theme 3 is the cleanest to extract — different files almost entirely. If a regression surfaces in any one theme post-merge, the others can't be reverted independently.
Not asking @eddie-knight to redo the work — just calling it out so the reviewer can decide whether to land as one or ask for a split.
Q-2: GovernancePanel→GovernanceView feature reduction
The deleted GovernancePanel.svelte (-924) surfaced AIGF risks with severity badges (OP/SEC/RC), an architecture-level governance score (ffd3871 retired), and per-mitigation Apply buttons. The new GovernanceView.svelte (+835) reorganises around a guidance↔controls join — a richer model — but a user who consumed "show me applicable risks for this AI node" no longer has that view. The catalogue data still exists (@calmstudio/calm-core/src/aigf/catalogue.ts — aigfRisks, 23 entries) but nothing renders it.
If this was always scaffolding, the rewrite is clean. If risk visibility was a documented user feature, this is a regression. Worth an explicit "yes I accept losing the risks UI" sign-off.
Q-3: Stack depth — #2740 ← #2692 ← #2691 ← #2683, all open
If any of the three base PRs is rejected or substantively changed in review, this PR rebases through the cascade. I confirmed gh pr diff 2740 correctly filters to this PR's diff only (16,375 lines == matches numstat), so we're reviewing the right surface — but landing this requires the full stack to converge. Worth deciding the merge order explicitly before this is approved.
🔵 Nits (non-blocking)
- All 57 commits carry a
Claude-Session: https://claude.ai/code/session_...trailer. Not Co-Authored-By, doesn't trip EasyCLA, but the URL 404s for everyone else and adds noise togit log. Worth scrubbing in a future workflow. hub.ts:11-15honestly documents that there's no signature/provenance verification on fetched catalogs. Trust model OK for first cut, but worth a SECURITY.md note or follow-up issue tracking cosign/oras as a future server-side proxy task.c4-levelmetadata key has a reader inc4Documents.svelte.ts:49-54but no interactive writer (onlygenerate.mjs:227,245for the bundled series). Interactive multi-doc series won't get the key written. Cosmetic — falls back to depth-derived badge. Either add a writer or remove the read.+page.svelteis 3089+ lines. Not for this PR, but future work should extract handlers into+page.actions.tsor per-feature modules.- No svelte-check baseline N→N count in the PR body. "No new errors over branch baseline" is unverifiable externally. Concrete numbers next time would close that gap.
Missing test coverage
Tests for the new work are mostly thoughtful — but three gaps stand out:
CalmCanvas.handleUndokeyboard path with governance state (see B-1). Existinghistory.test.tstests the snapshot mechanism, not the wired keyboard handler.@architecturelegacy fallback —loadGovernance.ts:36-39andscope.ts:13both honor it on read, but no integration test boots a doc with@architecturedecorators and verifies governance still loads from the fallback path.- Cross-tool round-trip of
metadata.calmstudio-layoutandmetadata.name— the body claims "ignored by non-Studio consumers" but the only verification is specadditionalProperties: true. A round-trip e2e throughcli validate+cli docify+ the CALMGuard normalizer would close this.
Will re-review once B-1 + I-1 land. Thanks for the careful work on the canonical-CALM discipline — it's the standout strength of this PR.
2 participants
Description
Demo:
https://drive.google.com/file/d/1nrKWN0fidQC8MHq31GWxF8_aHDFnrr8_/view
Builds the FINOS governance + C4 story into CalmStudio as first-class, canonical CALM 1.2, and makes documents persist what users actually care about (name, layout). Three themes:
1. Gemara ↔ CALM governance (the headline)
controls— domain-keyed (ai-governance), framework ids inconfig— never framework-prefixed keys.composed-of/deployed-in), not the non-canonical@architecturesentinel (kept only as a legacy fallback). This is the seam CALMGuard reads.2. Link-based C4 navigation
node.details.detailed-architecturelinks across documents — no level tabs; one file = one diagram.3. Document UX + persistence
metadata.nameand the diagram layout (node positions) inmetadata.calmstudio-layout— both restored on load (auto-layout fallback for un-positioned nodes). Templates spawn in named.Canonical-CALM guarantee: the relationship shape stays nested, controls stay domain-keyed, and all presentation data (name, layout) lives in
metadata(ignored by non-Studio consumers). Verified bycalm-spec-expert(canonical CALM 1.2 ✓) andpedantic-qa-guardian(SHIP) reviews across the governance, C4, and persistence work.Type of Change
Affected Components
Testing
I have tested my changes locally
I have added/updated unit tests
All existing tests pass
CalmStudio app: 498 vitest tests green; calm-core: 124 green.
svelte-checkat the established baseline (no new errors over the branch baseline).Production
npm run buildgreen; reference CALM document series passesnode cli/dist/index.js validate("All documents valid").Node 22 (CI baseline) throughout.
Checklist
type(calm-suite): …, DCO-signed)🤖 Generated with Claude Code