viz: architecture / drift / runtime dashboard (phases 3–5) - #14
Merged
Conversation
A standalone, self-contained HTML view over the new engines, separate from
viz/build_dashboard.py so neither refactors the other. Computes straight from the
stand artifacts via the engine modules:
- graph.json -> coupling map (Instability vs Abstractness on the main sequence, or
Ce/Ca bars when no is_abstract flag) + architecture findings by rule + a per-namespace
Ca/Ce/I/A/D table.
- drift.json -> drift risk buckets + a risk-tagged "what moved" table.
- findings.json + runtime.json -> runtime correlation donut (confirmed / static-only /
blind spot) + a confirmed-and-unpredicted-leaks table with confidence.
Every section is optional and renders a muted empty-state when its artifact is absent,
so it smokes in CI (no stand data) and lights up on the stand. XSS-safe (esc() before any
innerHTML, <>& escaped in the embedded JSON), Plotly vendored for a fully offline file.
Verified headless: 4 charts mount, drift/runtime tables populate, zero console errors;
empty-state renders cleanly. Wired into the CI dashboard smoke.
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 (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughCI now runs both dashboard smoke builds. A new arch dashboard generator reads stand artifacts and emits an HTML dashboard with empty-state handling when inputs are missing. ChangesArch dashboard generation and smoke coverage
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@viz/build_arch_dashboard.py`:
- Around line 128-136: The dashboard copy contains ambiguous Unicode characters
flagged by Ruff RUF001, specifically the “Phases 3–5” text, the multiplication
sign in “static × heap retention,” and the en dash in “main sequence”/“3–5”.
Update the HTML text in the template section that renders the coupling,
archrules, driftbar, and rtdonut cards to use ASCII equivalents, or
suppress/configure RUF001 if the typography is intentional. Keep the existing
meaning intact while making the strings Ruff-compliant.
- Around line 245-250: The _plotly_tag helper currently falls back to the CDN
when viz/plotly.min.js is missing, which bypasses the offline self-contained
requirement. Update _plotly_tag to require the vendored Plotly file: if
os.path.exists(vendored) is false, fail immediately with a clear error instead
of returning the remote script tag. Keep the existing vendored-file path in
_plotly_tag as the only source for the script content.
- Around line 36-55: The issue is that `_safe_json()` and `collect()` treat
malformed or schema-invalid artifacts as absent, hiding stale upstream data as
empty state. Update `collect()` and the `Graph(...)` initialization path so a
present `graph.json` with invalid JSON or schema `ValueError` is surfaced as an
error instead of returning `None`/`g = None`. Keep `_safe_json()` or its callers
distinguishing “file missing” from “file present but invalid,” and preserve the
upstream validation failure from `Graph` rather than swallowing it.
- Around line 77-80: The runtime correlation in the build_arch_dashboard flow
only runs when findings.json is a dict, so list-shaped findings are being
ignored and the runtime section stays empty. Update the guard around the
_safe_json/load path in the build_arch_dashboard logic so it accepts either a
dict with a findings key or a top-level list, then pass the normalized findings
collection into rt.correlate while keeping the runtime payload handling in
place. Use the existing symbols _safe_json, rt.correlate, and rt.load_config to
locate and adjust the branch.
🪄 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: 54883048-629c-4390-aa85-feb3d083d757
📒 Files selected for processing (2)
.github/workflows/ci.ymlviz/build_arch_dashboard.py
![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: 9840ef7293
ℹ️ 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".
| if os.path.exists(vendored): | ||
| with open(vendored, encoding="utf-8") as fh: | ||
| return "<script>" + fh.read() + "</script>" | ||
| return '<script src="https://cdn.plot.ly/plotly-2.35.2.min.js" charset="utf-8"></script>' |
There was a problem hiding this comment.
When this script is run from a fresh checkout with real graph/drift/runtime artifacts and then opened on an offline stand, this fallback emits only a CDN <script> because viz/plotly.min.js is gitignored and not tracked. The first Plotly.newPlot(...) call then throws Plotly is not defined, so none of the populated dashboard renders despite being advertised as self-contained; either vendor/inline Plotly for this dashboard or fail with instructions before writing a non-offline file.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Right diagnosis, @chatgpt-codex-connector — viz/plotly.min.js is gitignored, so on a fresh checkout the fallback emits a CDN-only file and a populated dashboard throws Plotly is not defined offline, contradicting the "self-contained" wording.
Fixed the wording (the actual defect) in 7b48a67 rather than failing the build: _plotly_tag now warns loudly on fallback and the docstrings no longer claim unconditional offline (Plotly inlined only when vendored, else CDN). I kept the fallback for parity with the already-merged build_dashboard.py and because fail-loud would break the CI smoke (vendor blob isn't in CI). If we want a true offline guarantee, that's a follow-up to vendor Plotly for both dashboards — flagged for @PhysShell.
Generated by Claude Code
There was a problem hiding this comment.
To use Codex here, create an environment for this repo.
… claim/unicode (Codex+CodeRabbit)
- _safe_json now distinguishes a MISSING artifact (-> None, empty-state) from a present-
but-malformed one (-> raise); the graph block lets Graph's schema ValueError propagate
instead of swallowing it, so a stale/corrupt graph.json fails loudly rather than
rendering as "not collected yet".
- runtime correlation accepts a list-shaped findings.json (top-level list) as well as the
usual {"findings": [...]} dict, so the section isn't silently empty.
- _plotly_tag keeps the CDN fallback (parity with build_dashboard.py + the gitignored
4.5MB vendor blob; fail-loud would break the CI smoke) but now WARNS on fallback, and the
docstrings no longer overclaim "self-contained/offline" — Plotly is inlined only when
vendored, else CDN.
- Ruff RUF001: replace ambiguous unicode in HTML copy with entities/ASCII
(3–5, static × heap, null-cell '-').
Re-verified headless: 4 charts, drift/runtime tables populate, zero console errors;
malformed/stale graph now raises; empty-state and list-findings paths both build.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
viz/build_arch_dashboard.py (1)
69-86: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate present artifact shapes before rendering empty states.
Line 69 and Line 81 still let some present-but-stale artifacts behave like “missing”: e.g.
drift.jsonas{}leavesD.driftnull, and invalidruntime.json/findings.jsonshapes can suppress the runtime section. Only missing files should become empty states.Proposed fix
drift = _safe_json(drift_path) - if isinstance(drift, dict) and "items" in drift: + if drift is not None: + if not isinstance(drift, dict) or not isinstance(drift.get("items"), list): + raise ValueError(f"{drift_path} must be an object with list-valued 'items'") buckets = collections.Counter(i.get("risk") for i in drift["items"]) data["drift"] = { "counts": [buckets.get(r, 0) for r in RISK_ORDER], @@ static = _safe_json(findings_path) dump = _safe_json(runtime_path) - if isinstance(dump, dict): + if dump is not None: + if not isinstance(dump, dict): + raise ValueError(f"{runtime_path} must be a JSON object") # findings.json is normally {"findings": [...]} but accept a top-level list too - static_findings = (static.get("findings", []) if isinstance(static, dict) - else static if isinstance(static, list) else None) + if isinstance(static, dict): + if not isinstance(static.get("findings"), list): + raise ValueError(f"{findings_path} must contain list-valued 'findings'") + static_findings = static["findings"] + else: + static_findings = static if isinstance(static, list) else None🤖 Prompt for 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. In `@viz/build_arch_dashboard.py` around lines 69 - 86, The drift/runtime handling in build_arch_dashboard.py is treating present-but-invalid artifacts like missing data, which can hide sections that should still render. Update the `_safe_json` consumers around `drift`, `static`, and `dump` so only absent files map to empty states, while malformed-but-present JSON still produces a rendered artifact state (or validation path) instead of leaving `data["drift"]` null or skipping `rt.correlate`; use the existing `drift`, `static_findings`, and `dump` flow to distinguish file absence from invalid shape before rendering.
🤖 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 `@viz/build_arch_dashboard.py`:
- Around line 260-262: The Plotly CDN fallback script tags in the dashboard
builders are missing subresource integrity and CORS protection, so update the
fallback returned by the arch dashboard generator and the corresponding fallback
in the main dashboard builder to include both an integrity hash for
plotly-2.35.2.min.js and crossorigin="anonymous". Use the existing CDN fallback
logic in build_arch_dashboard and build_dashboard to locate the two return
values and keep the offline-vendored path unchanged.
---
Outside diff comments:
In `@viz/build_arch_dashboard.py`:
- Around line 69-86: The drift/runtime handling in build_arch_dashboard.py is
treating present-but-invalid artifacts like missing data, which can hide
sections that should still render. Update the `_safe_json` consumers around
`drift`, `static`, and `dump` so only absent files map to empty states, while
malformed-but-present JSON still produces a rendered artifact state (or
validation path) instead of leaving `data["drift"]` null or skipping
`rt.correlate`; use the existing `drift`, `static_findings`, and `dump` flow to
distinguish file absence from invalid shape before rendering.
🪄 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: 9fd09ab6-a644-4501-b9db-767bb18dba6d
📒 Files selected for processing (1)
viz/build_arch_dashboard.py
…CodeRabbit)
- build_arch_dashboard: a present-but-malformed drift.json (e.g. {}) or a
findings.json with a non-list "findings" / a non-object runtime.json now raise
instead of silently rendering as a missing-artifact empty-state; only an absent
file maps to empty-state. Consistent with the _safe_json fix.
- Both dashboards: SRI-pin the Plotly CDN fallback with
integrity="sha384-cCVCZkAj…" + crossorigin="anonymous" so the remote bundle
can't be swapped silently. The hash is of plotly-2.35.2.min.js, verified
byte-identical to the vendored viz/plotly.min.js.
Verified: bad drift/findings shapes raise; the SRI fallback tag is well-formed;
both dashboards still build (CI smoke); demo stays headless-clean (4 charts, 0
console errors).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Что это
Дашборд для новых движков (фазы 3–5) — отдельный от
viz/build_dashboard.py(тот про 72k сырых находок), чтобы не перелопачивать существующий. Считает прямо из стенд-артефактов через модули движков:graph.jsonis_abstract) + arch-находки по правилам + таблица Ca/Ce/I/A/D по namespacedrift.jsonfindings.json+runtime.jsonКлючевое
esc()перед любым innerHTML,<>&экранированы в встроенном JSON (те же паттерны, что CodeRabbit одобрил в основном дашборде).Запуск
Проверено на синтетическом стенде (тот же граф с подсаженными регрессиями + runtime-дамп): 4 High дрейфа, confirmed-leak DocumentsViewModel, blind-spot OrphanGridAdapter — всё рисуется.
🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit