Update: preserve deps viewer layout for edge modes#1321
Conversation
- Keep all edges in the HTML Graphviz layout for reduced and omitted modes, hiding unselected edges with the page background color instead of removing them before layout. - Keep text output as a filtered edge-set view and update deps_viewer documentation/tests for the split rendering behavior.
📝 WalkthroughWalkthroughThe dependency viewer now preserves full HTML graph layouts while visually de-emphasizing unselected edges for reduced and omitted modes. DOT and HTML APIs carry hidden-edge styling and visible counts, with updated CLI documentation and tests. ChangesEdge visibility rendering
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant emit_html
participant emit_dot
participant HTML
CLI->>emit_html: pass hidden edges and visible count
emit_html->>emit_dot: render full edge set with hidden styling
emit_dot-->>emit_html: generate full graph layout
emit_html->>HTML: render layout and visible-edge count
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the dependency viewer tool to preserve the full-graph layout in HTML output when using transitive reduction modes (reduced or omitted). Instead of omitting edges from the layout, unselected edges are kept in the Graphviz layout but hidden. The review feedback suggests a key improvement: using Graphviz's standard style="invis" attribute to hide edges rather than hardcoding a background color (#eef2f7). This avoids coupling the Python logic to the HTML template's CSS, prevents visual glitches on the gradient grid background, and remains robust if a dark mode is introduced. Corresponding updates to tests, CLI help text, docstrings, and the README are also recommended to maintain consistency.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/ut/py/test_deps_viewer.py (1)
422-467: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix unused loop variables and add
strict=Truetozip().Ruff flags two issues in this test:
- B007:
expected_hiddenandexpected_visible_countare unpacked but unused in the first loop (line 447). Prefix with_to suppress.- B905:
zip(captures, cases)at line 462 lacks an explicitstrict=parameter. Sincelen(captures) == len(cases)is already asserted,strict=Trueis the natural choice.♻️ Proposed fixes for B007 and B905
for mode, _expected_hidden, _expected_visible_count in cases: rc = deps_viewer.main( [ str(deps_path), "--format", "html", "--edge-mode", mode, "-o", str(tmp_path / f"{mode}.html"), ] ) assert rc == 0 assert len(captures) == len(cases) - for captured, (_mode, expected_hidden, expected_visible_count) in zip(captures, cases): + for captured, (_mode, expected_hidden, expected_visible_count) in zip(captures, cases, strict=True): assert captured["edges"] == sorted(all_edges) assert set(captured["annotations"]) == set(all_edges) assert captured["hidden_edges"] == expected_hidden assert captured["visible_edge_count"] == expected_visible_count🤖 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 `@tests/ut/py/test_deps_viewer.py` around lines 422 - 467, In test_main_html_edge_modes_keep_full_layout_and_hide_unselected_edges, rename the unused expected_hidden and expected_visible_count unpacked in the first cases loop with underscore-prefixed names, and update the later zip(captures, cases) call to pass strict=True.Source: Linters/SAST tools
simpler_setup/tools/deps_viewer.py (1)
774-812: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
hidden_edge_attrs()called redundantly for annotated hidden edges.At line 783,
hidden_attrsis computed for every edge. In the annotated path (lines 791–813), this value is unused —hidden_edge_attrs()is called again at line 799. For hidden annotated edges, this creates two identical lists. Consider reusinghidden_attrsin the annotated path instead of callinghidden_edge_attrs()again.♻️ Reuse precomputed hidden_attrs in annotated path
edge_attrs = [] if hidden: - edge_attrs.extend(hidden_edge_attrs()) + edge_attrs.extend(hidden_attrs)🤖 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 `@simpler_setup/tools/deps_viewer.py` around lines 774 - 812, Reuse the precomputed hidden_attrs value inside the annotated edge loop instead of calling hidden_edge_attrs() again. Update the hidden branch that extends edge_attrs to extend with hidden_attrs, while preserving the existing behavior for visible and unannotated edges in the edge rendering loop.
🤖 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.
Nitpick comments:
In `@simpler_setup/tools/deps_viewer.py`:
- Around line 774-812: Reuse the precomputed hidden_attrs value inside the
annotated edge loop instead of calling hidden_edge_attrs() again. Update the
hidden branch that extends edge_attrs to extend with hidden_attrs, while
preserving the existing behavior for visible and unannotated edges in the edge
rendering loop.
In `@tests/ut/py/test_deps_viewer.py`:
- Around line 422-467: In
test_main_html_edge_modes_keep_full_layout_and_hide_unselected_edges, rename the
unused expected_hidden and expected_visible_count unpacked in the first cases
loop with underscore-prefixed names, and update the later zip(captures, cases)
call to pass strict=True.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1d98ab8b-7218-409d-9b7f-4378dc98ff94
📒 Files selected for processing (3)
simpler_setup/tools/README.mdsimpler_setup/tools/deps_viewer.pytests/ut/py/test_deps_viewer.py
2 participants
Keep all edges in the HTML Graphviz layout for reduced and omitted modes, hiding unselected edges with the page background color instead of removing them before layout.
Keep text output as a filtered edge-set view and update deps_viewer documentation/tests for the split rendering behavior.