arch: phase-4 Architecture Drift Report (diff two graph snapshots) - #12
Conversation
Killer feature #1: where phase-2 diffs *findings* (a violation only shows once it crosses a rule threshold), drift diffs the *structure* between two runs (baseline = main's graph, PR = current) and reports what moved — before it trips a rule. - arch/drift.py: snapshot(g) -> compact, committable {component metrics + namespace dependency surface (incl. external framework edges) + cycle set}. diff(base, cur) -> risk-tagged items: new/resolved cycles (type/ns/asm), new/removed dependencies, Ce coupling jumps, instability shifts. gate(d, level) for a PR ratchet. A new dependency into a sensitive_targets layer (SQL/WPF) is High. - arch/drift_cli.py: --save-snapshot, then --baseline (snapshot OR raw graph.json) + --graph -> drift.json + a PR-friendly drift.md grouped by 🔴/🟠/🔵. --gate-level fails (exit 2) on drift at/above a risk; report-only otherwise. - arch/rules.json: `drift` config block (thresholds + sensitive_targets). - docs: phase-4 section marked engine-ready with a worked example. - tests: +10 (identical=empty, new cycle=high, new SQL dep=high, plain dep=medium, coupling jump, resolved cycle=info, gate high-only, raw-graph baseline, CLI snapshot->gate) -> 41/41, -O safe. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds architecture drift snapshots, diffs, gating, and a CLI for report generation. Updates rules, tests, and docs to cover cycle, dependency, coupling, and instability comparisons. ChangesArchitecture Drift Report
Sequence Diagram(s)sequenceDiagram
participant Main as arch.drift_cli.main
participant Loader as JSON loader
participant Drift as arch.drift.diff
participant Gate as arch.drift.gate
participant FS as file system
participant Stdout as stdout
Main->>Loader: read baseline, current, rules, snapshot files
Main->>Drift: compare snapshots
opt --gate-level set
Main->>Gate: evaluate blocking risk floor
end
Main->>FS: write drift.json and drift.md
Main->>Stdout: print drift counts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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: 17fd6ed9af
ℹ️ 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".
| edges.add((a, b)) | ||
| cycles = [] | ||
| for comp in g.type_cycles(): | ||
| cycles.append({"level": "type", "members": sorted(g.name(m) for m in comp)}) |
There was a problem hiding this comment.
When a solution already has a type cycle between common short names (for example Service/Repository in one namespace), adding another cycle with the same short names in a different namespace produces the same members list here. _cycle_key then collapses the existing and new cycles, so the new high-risk cycle is not reported or gated. Include a fully qualified name or id in the snapshot key instead of only g.name(m).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Real bug, @chatgpt-codex-connector — short-name keys would collapse Sts.A.Service↔Repository and Sts.B.Service↔Repository into one, masking the new high-risk cycle from both the report and the gate.
Fixed in 6053d4d: snapshot() now keys type-cycle members by fully-qualified name (namespace.Name) via _fqn — identity, not just display (the report reads cleaner too: Sts.Broker.Services.OrderSync, Sts.Domain.Orders.OrderRepository). Regression test_drift_cycle_key_uses_full_identity adds an identically-named cycle in a second namespace and asserts it's detected as exactly one new cycle.
Generated by Claude Code
There was a problem hiding this comment.
To use Codex here, create an environment for this repo.
| if isinstance(data, dict) and data.get("schema") == SCHEMA: | ||
| return data |
There was a problem hiding this comment.
When a baseline snapshot was saved with one level and the CLI is later run with rules selecting another level, this returns the saved dict unchanged even though it carries level. The diff then compares incompatible namespace and assembly components/edges and emits bogus new/removed items, which can block a gate for an unchanged graph. Check the saved level against key and fail clearly on mismatches.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed, @chatgpt-codex-connector — a namespace snapshot diffed at assembly level compares incompatible component/edge keys and emits bogus new/removed items that could block a clean gate.
Fixed in 6053d4d: as_snapshot() now validates the saved level against the requested one and raises ValueError on mismatch; drift_cli converts that to a clean exit 2 with a message ("re-create the snapshot at level X"). Test test_drift_level_mismatch_rejected covers both the raise and the same-level pass-through.
Generated by Claude Code
There was a problem hiding this comment.
To use Codex here, create an environment for this repo.
…hot (Codex) Two Codex P2 correctness bugs in the drift snapshot: - _cycle_key used short type names, so two distinct cycles sharing short names (Service/Repository in different namespaces) collapsed to one key and a newly introduced cycle was masked from the report/gate. Snapshot now keys type-cycle members by fully-qualified name (namespace.Name) via _fqn — identity, not just display (the report reads cleaner too). - as_snapshot returned a saved snapshot unchanged even when it was built at a different `level` than the diff requested, so a namespace snapshot diffed at assembly level emitted bogus new/removed items and could block a clean gate. It now validates saved level == requested level and raises; drift_cli converts that to a clean exit 2. Tests: +2 (cross-namespace same-short-name cycles stay distinct; level mismatch rejected) -> 43/43, -O safe. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@arch/drift_cli.py`:
- Around line 111-113: The gate-check result in the drift CLI assigns both
passed and blocking, but only passed is used later, so the unused blocking
binding triggers the Ruff warning. Update the logic around D.gate in
drift_cli.py to discard the second return value and keep only the passed flag,
preserving the existing downstream behavior in _drift_md and the exit code path.
In `@docs/own-net-auditor.md`:
- Around line 170-172: Clarify the baseline snapshot workflow in the
own-net-auditor docs: the current wording around the snapshot/baseline is
ambiguous because it suggests the baseline “lives in the repo” while phase 2
treats baselines as user-generated artifacts kept out of git. Update the text
around the snapshot description to explicitly state where the baseline is
stored, when it is created, and that generated snapshots should not be committed
accidentally, using the snapshot/baseline terminology already present in this
section.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b6062a43-51ad-420c-9dfc-bb9acbf60f11
📒 Files selected for processing (5)
arch/drift.pyarch/drift_cli.pyarch/rules.jsonarch/tests/test_arch.pydocs/own-net-auditor.md
… (CodeRabbit) Two minor nitpicks: - drift_cli: `passed, blocking = D.gate(...)` left `blocking` unused (report sections come from d["items"]) -> Ruff RUF059. Discard it: `passed, _ = ...`. - docs/drift.py: wording said the baseline snapshot is "committable / lives in the repo", contradicting phase 2 where baselines are user artifacts kept out of git. Reword: snapshot is a stand artifact, .gitignored, fed to CI like the phase-2 baseline; don't commit generated snapshots. No behavior change; 43/43, -O safe. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa
2 participants
Что это
Killer feature №1. Фаза 2 диффит находки — видит нарушение, только когда оно перешло порог правила. Drift — дополнение: диффит саму структуру между двумя прогонами (baseline = граф
main, current = граф PR) и репортит, что сдвинулось, ещё до того как это станет находкой. То самое «PR провёл канализацию через гостиную».Чисто Python поверх
component_metrics()(фаза 3) + механики baseline (фаза 2). Контрактgraph.jsonне меняется — со стороны .NET ничего нового не нужно.Движок
arch/drift.pysnapshot(g)— компактный, коммитимый: метрики на компонент + namespace-поверхность зависимостей (включая рёбра во внешние фреймворки — там и живёт «new SQL dependency») + множество циклов.diff(base, cur)→ risk-tagged items: новые/убранные циклы (type/ns/asm), новые/убранные зависимости, скачкиCe, сдвиги нестабильности.gate(d, level)— PR-ратчет.arch/drift_cli.py—--save-snapshot, затем--baseline(снапшот или сыройgraph.json) +--graph→drift.json+ PR-friendlydrift.md(группировка 🔴/🟠/🔵).--gate-level→ exit 2 на дрейфе ≥ уровня; иначе report-only.arch/rules.json— блокdrift(пороги +sensitive_targets: новая зависимость в SQL/WPF = High).Проверено на синтетическом стенде
Взял baseline-граф (150 типов), применил PR-style регрессии (новый SQL-leak из Domain, новый cross-module цикл, скачок coupling) →
drift.md:Идентичные графы → ноль items (precision). Resolved-цикл → info, гейт не блочит.
Тесты
arch/tests/test_arch.py41/41 (bare +-O): +10 на drift — identical=empty, new cycle=high, new SQL dep=high, plain dep=medium, coupling jump, resolved cycle=info, gate high-only, raw-graph baseline, CLI snapshot→gate с кодами выхода.🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit
New Features
drift.jsonand a PR-friendlydrift.md, with optional drift gating and distinct exit codes.Bug Fixes
Documentation
Tests