Skip to content

Closes #541: GRAPHIFY_NO_VIZ + GRAPHIFY_VIZ_NODE_LIMIT env vars#542

Open
saxster wants to merge 1 commit intosafishamsi:v5from
saxster:fix/541-viz-node-limit-env-var
Open

Closes #541: GRAPHIFY_NO_VIZ + GRAPHIFY_VIZ_NODE_LIMIT env vars#542
saxster wants to merge 1 commit intosafishamsi:v5from
saxster:fix/541-viz-node-limit-env-var

Conversation

@saxster
Copy link
Copy Markdown

@saxster saxster commented Apr 25, 2026

Closes #541.

Summary

Adds two env-var opt-outs to _rebuild_code in graphify/watch.py so users can skip the HTML viz step without patching the post-commit hook script:

  • GRAPHIFY_NO_VIZ=1 — unconditional skip; for CI runners and headless dev boxes that never open graph.html.
  • GRAPHIFY_VIZ_NODE_LIMIT=N — soft cap; skip when node count exceeds N.

GRAPHIFY_NO_VIZ takes priority. Both default to off, preserving current behavior. The pre-existing ValueError catch around to_html stays as a backstop for unrelated rendering failures.

Why

v0.5.0 catches ValueError from to_html so the hook no longer fails on >5000-node graphs — but it still pays the full to_html attempt before the guard rejects. On a 6000-node Django graph this was costing several seconds per commit. The env vars let the user short-circuit before the attempt.

Implementation

New helper _viz_skip_reason(node_count) reads the two env vars and returns a reason string (or None). The to_html call site checks the reason first and skips with a printed message if set.

Tests

7 new tests in tests/test_watch.py:

  • defaults off (no env vars set)
  • GRAPHIFY_NO_VIZ truthy values
  • GRAPHIFY_NO_VIZ falsy values (0, false, no, empty, whitespace)
  • GRAPHIFY_VIZ_NODE_LIMIT threshold (one below, equal, one above)
  • invalid (non-int) GRAPHIFY_VIZ_NODE_LIMIT silently treated as unset
  • GRAPHIFY_NO_VIZ priority over GRAPHIFY_VIZ_NODE_LIMIT

pytest tests/test_watch.py — 18/18 pass. Full suite — 440/447 (the 7 failures are pre-existing in v0.5.0 and unrelated to this change; verified by stashing the diff and re-running).

Test plan

  • pytest tests/test_watch.py -q
  • pytest -q (full suite — same baseline failures, no new ones)
  • Manual: GRAPHIFY_NO_VIZ=1 graphify update . shows skip message + no graph.html produced
  • Manual: GRAPHIFY_VIZ_NODE_LIMIT=10 graphify update . on a >10-node repo shows skip message

Closes safishamsi#541.

The post-commit hook calls _rebuild_code in watch.py. v0.5.0 catches
ValueError from to_html (the >5000-node guard fires inside the
visualization step itself), but on large graphs the hook still pays
the full to_html attempt before the guard rejects. Two new opt-outs
let users skip the attempt entirely:

- GRAPHIFY_NO_VIZ=1 — unconditional skip; for CI runners and headless
  dev boxes that never open graph.html.
- GRAPHIFY_VIZ_NODE_LIMIT=N — soft cap; skip when node count exceeds N.

GRAPHIFY_NO_VIZ takes priority. Both default to off, preserving current
behavior. Pre-existing ValueError catch stays as a backstop.

7 new tests cover: defaults off, NO_VIZ truthy/falsy values, NODE_LIMIT
threshold (one below, equal, one above), invalid (non-int) NODE_LIMIT
silently treated as unset, NO_VIZ priority over NODE_LIMIT.
Copilot AI review requested due to automatic review settings April 25, 2026 04:59
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds environment-variable controls to skip generating the HTML visualization during code-only rebuilds, primarily to speed up git hook runs on large graphs and CI/headless environments.

Changes:

  • Introduces _viz_skip_reason(node_count) in graphify/watch.py to decide whether to skip HTML viz based on GRAPHIFY_NO_VIZ and GRAPHIFY_VIZ_NODE_LIMIT.
  • Updates _rebuild_code() to short-circuit the to_html() call when configured, and to remove any stale graph.html when skipping.
  • Adds focused unit tests covering default behavior, env var parsing, limits, invalid values, and precedence rules.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
graphify/watch.py Adds env-var based skip logic for HTML viz generation and integrates it into _rebuild_code() with stale graph.html cleanup.
tests/test_watch.py Adds tests validating _viz_skip_reason() behavior across env var combinations and edge cases.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Qodo-Free-For-OSS
Copy link
Copy Markdown

Hi, When viz is skipped, _rebuild_code unlinks a stale graph.html without handling OSError, so file permission/locking issues can make the entire rebuild fail even though core outputs were written. The new env-var skip path increases how often this cleanup branch runs, raising the likelihood of hitting this failure mode.

Severity: action required | Category: reliability

How to fix: Make stale delete best-effort

Agent prompt to fix - you can give this to your LLM of choice:

Issue description

_rebuild_code() deletes graph.html when viz is skipped. If the file is locked or permissions prevent deletion, Path.unlink() raises and the outer except Exception causes the whole rebuild to fail.

Issue Context

The env-var early-skip path adds another route into the stale-delete branch, increasing exposure.

Fix Focus Areas

  • graphify/watch.py[141-155]

Suggested change

Wrap stale.unlink() in a try/except OSError (or Exception) that logs a warning and continues, so core rebuild products are not marked failed due to cleanup.


Found by Qodo code review. FYI, Qodo is free for open-source.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose --no-viz flag and/or GRAPHIFY_VIZ_NODE_LIMIT env var

3 participants