fix(callflow): disclose diagram truncation — "18 of 1223 nodes" was only in a Mermaid comment - #2518
Conversation
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR appears to revert the unreleased 0.9.35 changelog section, folding it back and re-marking 0.9.34 as unreleased. In graphify/build.py it removes the recently-added helpers _build_prune_sets and _derive_prune_root, simplifies _infer_merge_root back to a plain grandparent heuristic, and inlines the prune-set construction in merge_raw_extraction (dropping the absolute-path root derivation and zero-match handling). The changed-symbol list also spans many extractor, serve, callflow, detect, and watch modules along with their tests, so the surface area extends across graph extraction, querying, path/direction handling, and merge/prune logic rather than a single area.
No blocking issues surfaced. 4 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 3438 functions depend on the 1573 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
write_callflow_html()— 7 callers, 26 callees
Verification — 3438 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 3138 function(s) in the blast radius were not formally verified this run
· 1 more finding(s) on lines outside this diff (see the check run).
9c95c72 to
102798b
Compare
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR adds a "diagram coverage" disclosure to the section flowcharts in the callflow HTML export. It extracts the node/edge selection logic into a shared select_diagram_content helper (used by both the flowchart renderer and the new coverage note), and adds a generate_diagram_coverage function that emits a visible <p> stating how many of a section's nodes and edges are actually drawn, returning empty when nothing is omitted. New CSS styling and two tests are included, and the coverage note is wired into write_callflow_html beneath each section diagram.
No blocking issues surfaced. 4 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 394 functions depend on the 168 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
write_callflow_html()— 7 callers, 26 callees
Verification — 394 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 264 function(s) in the blast radius were not formally verified this run
· 1 more finding(s) on lines outside this diff (see the check run).
Section diagrams are capped at max_diagram_nodes/max_diagram_edges. The
counts were already computed -- omitted_nodes/omitted_edges in
generate_section_flowchart -- but emitted as a Mermaid '%%' comment,
which Mermaid does not render, so no reader ever saw them.
What the reader does see is the section intro: 'This section covers N
nodes and M internal edges; the diagram shows only representative
relationships to stay readable.' It gives the totals and never says how
much is missing, so a section reported as 1,223 nodes renders a diagram
of 18 that reads as complete.
Add a visible coverage line under each diagram ('Diagram draws 18 of
1223 nodes and 10 of 10 edges'), and extract the node/edge selection
into select_diagram_content so the note and the drawing cannot drift.
The line is omitted entirely when nothing was cut.
Verified on a 4,990-node graph: 15 sections, and every note agrees with
the renderer's own omission counts.
102798b to
d191872
Compare
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Graphify review — findings
This PR adds a visible "coverage note" beneath each section flowchart in the generated call-flow HTML report, stating how many of a section's nodes and edges the capped diagram actually draws. It introduces a new select_diagram_content helper that factors out the node/edge selection logic previously inline in generate_section_flowchart, and a new generate_diagram_coverage function (plus a CSS .diagram-coverage style) that reuses that helper to render the note, emitting nothing when the diagram is complete. The surface area covers graphify/callflow_html.py (refactored selection logic, new functions, wiring into write_callflow_html, and a stylesheet rule) and two new tests in tests/test_callflow_html.py exercising the coverage output for capped and complete diagrams.
Worth a look
- Coverage note reports edge total after cap, understating omitted edges —
graphify/callflow_html.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 404 functions depend on the 168 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
write_callflow_html()— 7 callers, 26 callees
Verification — 404 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 264 function(s) in the blast radius were not formally verified this run
· 1 more finding(s) on lines outside this diff (see the check run).
Problem
Section diagrams are capped at
max_diagram_nodes/max_diagram_edges(18 / 24 by default). The truncation counts are already computed — but emitted where no reader can see them:%%is a Mermaid comment. It is never rendered.What the reader does see is the section intro, which gives the totals and then hedges without a number:
Directly beneath that sentence sits a diagram containing 18 nodes. Nothing on the page distinguishes "this diagram is the section" from "this diagram is 1.5% of the section", so a reader who has just been told the section has 1,223 nodes reasonably reads the picture as complete. On the graph I tested, one section drew 18 of 294 and another 18 of 1,223.
Fix
Render the coverage as visible text under each diagram:
Two supporting changes:
select_diagram_contentextracts the node/edge selection thatgenerate_section_flowchartwas doing inline, so the note and the drawing are computed from one place and cannot drift. A disclosure derived independently of the thing it describes will eventually lie.Localised (zh/en) via the existing
pick_text, styled with the existing--mutedtoken.Verification
On a 4,990-node / 10,135-edge graph: 15 sections rendered, 15 coverage notes, and every note agrees exactly with the renderer's own
omitted_*counts — checked by parsing both the visible text and the Mermaid comment out of the HTML and diffing them. 0 disagreements.Notes
v8as-is (the function does not exist) and pass with the fix.v8(30 pre-existing).test_labeling.py::test_label_communities_batches_when_over_batch_sizeis flaky — it failed 3 of 4 runs on unpatchedv8when I checked.%%comment is left in place; it is harmless and useful when reading the generated source.