Repository navigation
fix(tree): one root per repo on merged graphs; refuse graphs with no source_file - #4541
omsenjalia wants to merge 2 commits into
Conversation
|
Thanks for the pull request, @omsenjalia. A maintainer will review it soon. Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions. A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic. |
…source_file graphify tree ignored the repo tag merge-graphs sets on every node, so the root was the (empty) common prefix of repo-relative paths and files sharing a relative path in different repos collapsed into one file node with their symbols interleaved (Graphify-Labs#4524). build_tree now groups nodes by repo and builds each repo's subtree from its own paths; an explicit --root applies inside each repo, and the zero-match guard (Graphify-Labs#2534) counts across all of them. merge-graphs itself keeps source_file on current v8. A graph where no node has one (as in Graphify-Labs#4522, whose nodes came from a non-graphify schema) used to render a blank "(empty graph)" viewer with no hint why. build_tree now raises, and the CLI exits 1 with the reason, like an unmatched --root. Fixes Graphify-Labs#4524 Refs Graphify-Labs#4522 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
49a80b2 to
6130941
Compare
There was a problem hiding this comment.
Graphify reviewed this change.
No issues found by static checks — no coupling regressions or blocking issues in the code graph. That is not the same as safe to merge: see what was not checked.
Not checked: tests were not run; no formal proof of the changed code.
Graphify review — findings
Adds per-repo roots to graphify tree for merged graphs, so src/engine.py in two repos stays two files under their own repo nodes instead of collapsing into one (#4524). The --root value is now applied inside each repo's tree, and the "matched 0 files" error fires only when no repo matches. A graph tagged with a single repo still takes the multi-repo path, so it gets an extra wrapper root labelled "1 repos". Fails loudly with a ValueError when a non-empty graph has no source_file on any node, instead of rendering a blank viewer (#4522), and the CLI surfaces this as a clean exit 1. Untagged graphs keep the old single-root build, and the --root help text now notes the per-repo behaviour.
No blocking issues surfaced. 4 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 396 functions depend on the 119 functions this change touches.
Health — grade A; 7 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):
dispatch_command()— 5 callers, 134 callees (high)main()— 110 callers, 3 callees (high)_run_cli()— 8 callers, 8 callees (high)_run_hook_guard()— 4 callers, 14 callees (high)_stale_graph_sources()— 7 callers, 6 callees (high)_run()— 4 callers, 3 callees (medium)test_poisoned_manifest_is_healed()— 0 callers, 6 callees (medium)
Verification — 396 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: 351 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
36 of 432 test file(s) selected (8%) via static blast radius.
tests/test_affected_cli.py— impacttests/test_agents_platform.py— impacttests/test_cli_export.py— impacttests/test_cloud_cta.py— impacttests/test_codebuddy.py— impacttests/test_compact_graph_json.py— impacttests/test_dedup_shrink_refuses_force_write.py— impacttests/test_devin.py— impacttests/test_explain_cli.py— impacttests/test_explain_same_file_tie.py— impacttests/test_extract_cli.py— impacttests/test_global_add_tag_inference.py— impacttests/test_god_nodes_cli.py— impacttests/test_hollow_chunks_arm_shrink_guard.py— impacttests/test_hook_guard_token_match.py— impacttests/test_hook_out_of_project_paths.py— impacttests/test_hook_strict.py— impacttests/test_html_script_breakout.py— impacttests/test_incomplete_build_guard.py— impacttests/test_install.py— impacttests/test_install_references.py— impacttests/test_lookup_exit_codes.py— impacttests/test_merge_chunks_validation.py— impacttests/test_multigraph_diagnostics.py— impacttests/test_no_dedup_flag.py— impacttests/test_opencode_plugin_refresh.py— impacttests/test_partial_cache.py— impacttests/test_path_cli.py— impacttests/test_query_cli.py— impacttests/test_query_induced_edges.py— impacttests/test_query_seeds.py— impacttests/test_skill_auto_refresh.py— impacttests/test_stale_prune.py— impacttests/test_tree_html.py— impact, changed-testtests/test_truncation_full_answer.py— impacttests/test_unverified_semantic_shrink.py— impact
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
A graph whose nodes all carry the same repo tag took the multi-repo path and gained a pointless wrapper root labelled "1 repos". Only wrap when there are two or more repos; a lone tag now labels the plain tree. Raised in review of Graphify-Labs#4541. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fixes #4524. Refs #4522.
Problem
graphify treenever read therepotag thatmerge-graphsputs on every node. The root became the empty common prefix of repo-relative paths, so the title was/. Worse,src/engine.pyin two repos collapsed into a single file node with both repos' symbols mixed together.merge-graphskeepssource_file(a test now pins that). The reported node shape (src/origin/source, wheresourceholds a repo name) isn't written anywhere in graphify. The real problem is that a graph where no node has asource_filerendered a blank(empty graph)viewer with no explanation.Change (
graphify/tree_html.py)build_treegroups file nodes byrepo. Each repo gets its own top-level root, built from that repo's paths. A graph with norepotags builds exactly as before.--rootnow applies inside each repo. The zero-match guard from Four silent failures with success exit codes: cluster-only ignores --backend, label prompt collides with theCommunity {cid}sentinel,tree --root <abs>flattens the hierarchy, built_at_commit stamped from cwd #2534 counts matches across all repos.source_filenow raisesValueError, and the CLI exits 1 with the reason, the same way an unmatched--rootdoes. A truly empty graph still returns(empty graph).src/source: in graphify tree renders an empty viewer on any merged graph — merge-graphs drops source_file from every node #4522's data,sourceis a repo name, not a path, so the fallback would guess wrong.Tests (
tests/test_tree_html.py)src/engine.pyproduce two separate file nodes with their own symbols, under per-repo roots.--rooton a merged graph (zero match raises; a repo-relative root matches in each repo), and the no-source_fileerror.merge-graphskeepssource_file, andtreeon its output draws both repo roots.ruffandpyrightare clean on the touched files. On the full suite (Windows), 18 tests fail both with and without this change, in modules unrelated to tree (test_ext_tagged_id_clash,test_skillgen_input_path_injection, memoization and getcwd tests, and others), so they come from the environment.AI disclosure (per CONTRIBUTING.md): Claude Opus 5.5 (Claude Code) materially contributed to this change.
🤖 Generated with Claude Code