refactor+docs: type the hot path; ADR-0006 (two subsystems, bridge don't delete)#19
Conversation
…(ADR 0006) 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) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91eb677766
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # --- Graph methods (v0.4.0) --- | ||
|
|
||
| def add(self, nodes): | ||
| def add(self, nodes: DataNode | builtins.list[DataNode]) -> AddResult: |
There was a problem hiding this comment.
When callers or tool-schema/framework code introspect Ledger.add with typing.get_type_hints, this annotation is evaluated against model_ledger.sdk.ledger globals, but DataNode is only imported under TYPE_CHECKING and the local import inside add() has not run. In that context the new annotation raises NameError: name 'DataNode' is not defined, which can break the agent/tool use cases this SDK supports; keep DataNode available at module scope or supply it to the introspector.
Useful? React with 👍 / 👎.
1 participant
Two things: a type-safety rigor fix, and the architecture decision that mapping the codebase surfaced.
Type the hot path
add/connect/trace/upstream/downstream/_load_discovered_nodeswere untyped, so mypy skipped their bodies. Now annotated —add()/connect()return TypedDicts (AddResult,ConnectResult), which is fully typed and non-breaking (result["added"]still works). mypy now checks these bodies; fixed one revealed variable-reuse bug.ADR 0006 — discovery and validation are two subsystems
Mapping the "legacy" footprint before cutting revealed that the v0.2
Model/profiles aren't dead weight — they're the compliance-validation subsystem behind the Governance page, disconnected from the event-log discovery subsystem. Decision: retain it, bridge the two additively, document both layers — not delete (which would destroy validation) and not rush-rewrite the profiles.mypy clean · ruff clean · 725 passed / 24 skipped · coverage 74.53%.
🤖 Generated with Claude Code