From 91eb677766ab722f67d3082a6806ad8bab4ceb46 Mon Sep 17 00:00:00 2001 From: Vignesh Narayanaswamy Date: Sun, 7 Jun 2026 19:29:43 -0700 Subject: [PATCH] refactor+docs: type the hot path; record the two-subsystems decision (ADR 0006) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Type safety on the most-used SDK methods, plus the architecture finding that mapping the legacy footprint surfaced. - sdk/ledger.py: annotate add/connect/trace/upstream/downstream and _load_discovered_nodes. add()/connect() return TypedDicts (AddResult, ConnectResult) — fully typed *and* non-breaking, since result["added"] still works. mypy now type-checks these bodies (the [annotation-unchecked] notes are gone); fixed one revealed variable-reuse issue. - ADR 0006: the v0.2 "legacy" paradigm is actually the compliance-validation subsystem (rich governance Model + SR-11-7/EU-AI-Act/NIST profiles), and it's disconnected from the event-log discovery subsystem. Decision: retain it, bridge the two additively (don't delete or rush-rewrite), document both layers. mypy clean (75 files), ruff clean, 725 passed / 24 skipped, coverage 74.53%. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...006-discovery-and-validation-subsystems.md | 73 +++++++++++++++++++ docs/adr/index.md | 1 + mkdocs.yml | 1 + src/model_ledger/sdk/ledger.py | 36 ++++++--- 4 files changed, 102 insertions(+), 9 deletions(-) create mode 100644 docs/adr/0006-discovery-and-validation-subsystems.md diff --git a/docs/adr/0006-discovery-and-validation-subsystems.md b/docs/adr/0006-discovery-and-validation-subsystems.md new file mode 100644 index 0000000..5c7d4ee --- /dev/null +++ b/docs/adr/0006-discovery-and-validation-subsystems.md @@ -0,0 +1,73 @@ +--- +title: "ADR 0006 — Discovery and validation are two subsystems" +description: Recognize the event-log (discovery) and the rich governance model (validation) as complementary subsystems; bridge them additively rather than delete or rewrite either. +--- + +# ADR 0006 — Discovery and validation are two subsystems; bridge, don't delete + +**Status:** Accepted + +## Context + +The codebase contains what looks, from the export list, like competing paradigms: + +- **The event-log (v0.3+):** `ModelRef` (a thin identity) + immutable `Snapshot`s + the + `DataNode` graph. This is the **discovery / inventory / agent** subsystem — *what models + exist, how they connect, what changed*. +- **The rich governance model (v0.2):** `Model`/`ModelVersion` carrying `intended_purpose`, + `risk_rating`, `affected_populations`, stakeholders, findings, deployments; the `validate` + engine; and the SR 11‑7 / EU AI Act / NIST **compliance profiles** that check those fields. + This is the **validation** subsystem — *can a model pass an examiner's bar?* + +A surface-level read ("three paradigms in `__all__`, clean it up") suggests deleting the +v0.2 API. Mapping the dependencies shows that would be a serious mistake: the v0.2 model is +the substance behind the [Governance](../governance.md) page and the regulatory wedge — a +rich, field-level compliance model plus three non-trivial profiles, the audit-pack export, +and a meaningful share of the test suite. The thin event-log `ModelRef` carries none of those +fields, so it cannot be validated by the profiles as-is. + +The real issue is not clutter. **The two subsystems are disconnected:** a *discovered* model +(`ModelRef`) has no path to *validation*. You can inventory a model or validate one, but not +both in one flow. + +## Decision + +1. **Retain the v0.2 governance/validation subsystem.** It is not legacy dead weight; it is + the validation half of the product. Do not delete it or rewrite its profiles destructively. +2. **Bridge the two subsystems *additively*.** Make a discovered model validatable by + projecting its event-log evidence (metadata + snapshots) into the governance concepts the + profiles check — without breaking the existing rich-`Model` path. The event-log is the + spine; validation becomes a capability over it. +3. **Document both layers honestly** — discovery and validation are two intentional, + complementary subsystems, not old-vs-new. The fix for "surface clutter" is *clarity*, not + deletion. + +The bridge is a deliberate, staged effort (it touches the profile inputs and deserves its own +focused, well-tested change), not a rushed rewrite. + +## Consequences + +**Positive** + +- The compliance capability — the product's differentiator — is preserved intact. +- Once bridged, discovery feeds validation: you can ask "is this *discovered* model SR 26‑2 + ready?" in one system. +- The public surface is explained as two coherent layers rather than apologized for as clutter. + +**Negative (accepted)** + +- Two models of a "model" coexist (thin `ModelRef`, rich `Model`) until the bridge matures; + the mapping between them is an explicit layer to maintain. +- "Concept-based" validation over event-log evidence is looser than rich typed checks, so the + bridge must be designed to preserve validation meaningfulness, not just presence-of-a-field. + +## Alternatives considered + +- **Delete v0.2 and clean `__all__` (rejected):** destroys the validation subsystem and the + regulatory wedge; a shallow read of a deep system. +- **Immediately rewrite the profiles to be event-log-native (deferred):** correct direction, + but rushing it risks downgrading sophisticated validation to thin evidence-checks. Staged. +- **Leave them disconnected, document the boundary (interim):** honest but leaves the two + halves of governance unable to talk; acceptable only as a way station to the bridge. + +See [Architecture](../concepts/architecture.md) and [Governance](../governance.md). diff --git a/docs/adr/index.md b/docs/adr/index.md index 5dd40d5..2b30c4a 100644 --- a/docs/adr/index.md +++ b/docs/adr/index.md @@ -17,5 +17,6 @@ a new ADR that supersedes the old one rather than an edit. | [0003](0003-agents-first.md) | Agents are the primary interface; the SDK is tool-shaped | Accepted | | [0004](0004-framework-agnostic.md) | Framework-agnostic core; regulations are pluggable profiles | Accepted | | [0005](0005-storage-agnostic.md) | Storage-agnostic via the LedgerBackend protocol | Accepted | +| [0006](0006-discovery-and-validation-subsystems.md) | Discovery and validation are two subsystems; bridge, don't delete | Accepted | The narrative that ties these together is the [Architecture](../concepts/architecture.md) page. diff --git a/mkdocs.yml b/mkdocs.yml index 2ee2dc8..c6e446a 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -171,3 +171,4 @@ nav: - "ADR 0003 — Agents are the primary interface": adr/0003-agents-first.md - "ADR 0004 — Framework-agnostic, pluggable profiles": adr/0004-framework-agnostic.md - "ADR 0005 — Storage-agnostic backends": adr/0005-storage-agnostic.md + - "ADR 0006 — Discovery and validation are two subsystems": adr/0006-discovery-and-validation-subsystems.md diff --git a/src/model_ledger/sdk/ledger.py b/src/model_ledger/sdk/ledger.py index 6eb862a..42a03f3 100644 --- a/src/model_ledger/sdk/ledger.py +++ b/src/model_ledger/sdk/ledger.py @@ -4,13 +4,31 @@ import builtins from datetime import datetime, timezone -from typing import Any +from typing import TYPE_CHECKING, Any, TypedDict from model_ledger.backends.ledger_memory import InMemoryLedgerBackend from model_ledger.backends.ledger_protocol import LedgerBackend from model_ledger.core.exceptions import ModelNotFoundError from model_ledger.core.ledger_models import ModelRef, Snapshot, Tag +if TYPE_CHECKING: + from model_ledger.graph.models import DataNode + + +class AddResult(TypedDict): + """Result of ``Ledger.add()`` — nodes newly recorded vs. skipped as unchanged.""" + + added: int + skipped: int + + +class ConnectResult(TypedDict): + """Result of ``Ledger.connect()`` — dependency edges created vs. skipped as present.""" + + links_created: int + links_skipped: int + + # Events that are internal ledger bookkeeping or governance actions on the # composite itself. These are NOT propagated as member_changed to parent # composites — only real domain events on member models should surface there. @@ -324,7 +342,7 @@ def dependencies( # --- Graph methods (v0.4.0) --- - def add(self, nodes): + def add(self, nodes: DataNode | builtins.list[DataNode]) -> AddResult: """Register DataNodes. Each becomes a ModelRef + discovered Snapshot. Skips writing if the discovered payload is identical to the last snapshot @@ -427,7 +445,7 @@ def add(self, nodes): return {"added": added, "skipped": skipped} - def connect(self): + def connect(self) -> ConnectResult: """Match output ports to input ports. Write only new dependency links. Uses cached nodes from add() if available (avoids re-reading from backend). @@ -488,7 +506,7 @@ def connect(self): continue return {"links_created": links_created, "links_skipped": links_skipped} - def trace(self, name): + def trace(self, name: str) -> builtins.list[str]: """Topological path from sources to this node.""" self._resolve_model(name) visited = set() @@ -505,12 +523,12 @@ def _walk(n): _walk(name) return order - def upstream(self, name): + def upstream(self, name: str) -> builtins.list[str]: """All models this one depends on (transitive).""" path = self.trace(name) return [n for n in path if n != name] - def downstream(self, name): + def downstream(self, name: str) -> builtins.list[str]: """All models that depend on this one (transitive).""" self._resolve_model(name) visited = set() @@ -877,7 +895,7 @@ def composite_summary( ) return result - def _load_discovered_nodes(self): + def _load_discovered_nodes(self) -> builtins.list[DataNode]: """Rebuild DataNodes from stored discovery snapshots. Uses bulk loading if the backend supports it (1 query instead of N). @@ -895,9 +913,9 @@ def _load_discovered_nodes(self): else: # Fallback: per-model queries all_snaps = [] - for model in models: + for m in models: all_snaps.extend( - self._backend.list_snapshots(model.model_hash, event_type="discovered") + self._backend.list_snapshots(m.model_hash, event_type="discovered") ) # Group by model and take latest