Skip to content

fix(skill): complete induced subgraph in query fallback - #4316

Open
nothariharan wants to merge 1 commit into
Graphify-Labs:v8from
nothariharan:fix/4314-query-fallback-induced-edges
Open

nothariharan wants to merge 1 commit into
Graphify-Labs:v8from
nothariharan:fix/4314-query-fallback-induced-edges

Conversation

@nothariharan

Copy link
Copy Markdown
Contributor

Summary

Fixes #4314.

The inline NetworkX fallback for /graphify query recorded an edge only when BFS/DFS discovered an unvisited neighbour, so an edge between two already-visited nodes (two seeds, a triangle chord, a cycle arc, a cross-link) was dropped. The fallback returned a traversal tree rather than the induced subgraph over the node set it reports.

graphify query had the identical defect and already fixed it in #2323 via graphify.serve._complete_induced_edges; the inline fallback now mirrors that completion.

Reproduced on v8 with the issue's example (seeds A/B/C; edges A→B, B→C, A→C):

case before after
BFS, three seeds 0 edges 3 edges
DFS, three seeds 2 edges (missing B→C) 3 edges
BFS, single seed 2 edges (missing B→C) 3 edges

Changes

  • tools/skillgen/fragments/references/query/default.md, tools/skillgen/fragments/core/aider.md, tools/skillgen/fragments/core/devin.md: add an induced-edge completion pass after the BFS/DFS walk. The visited set controls expansion only; edges are completed over the visited set (ordered-pair keys when directed, unordered frozenset otherwise; self-loops skipped; duplicates collapsed; edges incident to unvisited nodes excluded).
  • tools/skillgen/gen.py: _is_query_induced_edges_fix_line plus registration in _SANCTIONED_MONOLITH_DIFFS (the monolith round-trip guard).
  • Regenerated all 14 graphify/skills/*/references/query.md, graphify/skill-aider.md, graphify/skill-devin.md, and every matching tools/skillgen/expected/* snapshot.
  • tests/test_skill_query_fallback_induced_edges.py: extracts the shipped fallback and executes it against a real graphify-out/graph.json; covers seeded nodes, BFS/DFS cross-links, cycles, edge direction, and parallel-edge de-duplication (24 tests across the reference fragment and the shipped aider/devin/codex artifacts).

Verification

  • python -m tools.skillgen --check → OK (134 artifacts)
  • python -m tools.skillgen --monolith-roundtrip → OK
  • python -m tools.skillgen --audit-coverage → OK
  • pytest tests/test_skill_query_fallback_induced_edges.py → 24 passed
  • Falsified against pre-fix HEAD: the seeded case emits 0 edges before, 3 after; the new fixture also asserts the completion line is present, so a revert fails rather than silently passing.
  • Swept the tree: all 35 shipped copies of the fallback now contain the completion; none remain buggy. path uses nx.shortest_path and explain is a one-hop listing, so neither needed changes.

Note: tests/test_skillgen.py::test_no_version_or_timestamp_in_output and tests/test_skill_persisted_scan_options.py fail identically on pristine v8 in an uninstalled source checkout (__version__ == "unknown" / import graphify from a temp cwd). They are unrelated to this change.

Out of scope, same defect class

graphify/benchmark.py::_query_subgraph_tokens records edges the same discovery-coupled way. It only feeds an internal token estimate for graphify benchmark, so it is not part of the /graphify query answer path; flagged for awareness rather than changed here.

Notes for maintainers

  • Target branch is v8 (repo default).
  • No API or graph-format changes. Output changes only in that previously-omitted edges between visited nodes are now emitted.

…s#4314)

The inline NetworkX fallback for `/graphify query` recorded an edge only when
BFS/DFS discovered an *unvisited* neighbour. An edge between two already-visited
nodes (two seeds, a triangle chord, a cycle arc, a cross-link) was therefore
dropped, so the fallback returned a traversal tree rather than the induced
subgraph over the node set it reports.

`graphify query` had the identical defect and fixed it in Graphify-Labs#2323 via
`graphify.serve._complete_induced_edges`; the fallback now mirrors that
completion. The visited set controls expansion only, and the edge list is
completed over the visited set: ordered-pair keys on a directed graph, unordered
frozenset keys otherwise, self-loops skipped, duplicates collapsed, and edges
incident to unvisited nodes excluded.

Updated the authoritative sources (references/query/default.md, core/aider.md,
core/devin.md), regenerated every affected artifact and expected/ snapshot, and
added a sanctioned monolith-diff predicate in tools/skillgen/gen.py.
Regression tests extract and execute the shipped fallback and cover seeded
nodes, BFS/DFS cross-links, cycles, edge direction, and parallel-edge
de-duplication.

Closes Graphify-Labs#4314
@github-actions

Copy link
Copy Markdown

Thanks for the pull request, @nothariharan. 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.

@github-actions github-actions Bot added the contributor Pull request from a returning community contributor label Oct 11, 2026

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Fixes the query fallback's subgraph output so it includes every edge between visited nodes. Previously it emitted only the BFS/DFS traversal-tree edges, which dropped seed-to-seed links, triangle chords and cross-links (#4314). After traversal, a completion pass scans edges incident to the subgraph and appends any missing ones, skipping self-loops, deduping (direction-aware on directed graphs) and keeping cost proportional to the subgraph size. The fix ships in every generated skill variant and their skillgen fragments/expected outputs, with new tests covering directed-edge direction and DFS cross-links.

Review partial — this diff was larger than one review pass covers, so later files were not reviewed; some findings may be missing.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 576 functions depend on the 505 functions this change touches.

Health — grade A; 5 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):

  • render() — 13 callers, 5 callees (high)
  • audit_coverage() — 8 callers, 6 callees (high)
  • main() — 3 callers, 11 callees (medium)
  • monolith_roundtrip() — 3 callers, 5 callees (medium)
  • test_audit_catches_a_dropped_non_allowlisted_heading() — 0 callers, 6 callees (medium)

Verification — 576 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: 576 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

368 of 368 test file(s) selected (100%) via static blast radius.

Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.

  • tests/test_affected_cli.py — full-run-safety
  • tests/test_affected_defines.py — full-run-safety
  • tests/test_affected_member_seed.py — full-run-safety
  • tests/test_agents_platform.py — full-run-safety
  • tests/test_analyze.py — full-run-safety
  • tests/test_anthropic_custom_endpoint.py — full-run-safety
  • tests/test_antigravity_install.py — full-run-safety
  • tests/test_apm_fallback_version.py — full-run-safety
  • tests/test_architecture_doc.py — full-run-safety
  • tests/test_astro_extraction.py — full-run-safety
  • tests/test_astro_import_ids.py — full-run-safety
  • tests/test_atomic_canvas_export.py — full-run-safety
  • tests/test_atomic_version_stamp.py — full-run-safety
  • tests/test_atomic_writes.py — full-run-safety
  • tests/test_backend_env_isolation.py — full-run-safety
  • tests/test_backend_extras.py — full-run-safety
  • tests/test_benchmark.py — full-run-safety
  • tests/test_benchmark_raw_graph.py — full-run-safety
  • tests/test_blade_extractor.py — full-run-safety
  • tests/test_build.py — full-run-safety
  • tests/test_build_located_semantic_identity.py — full-run-safety
  • tests/test_build_merge_dedup_scope.py — full-run-safety
  • tests/test_build_merge_hyperedges_and_prune.py — full-run-safety
  • tests/test_build_merge_shrink_guard.py — full-run-safety
  • tests/test_builtin_global_type_refs.py — full-run-safety
  • tests/test_cache.py — full-run-safety
  • tests/test_cache_stale_import_target.py — full-run-safety
  • tests/test_callflow_html.py — full-run-safety
  • tests/test_cargo_introspect.py — full-run-safety
  • tests/test_cargo_missing_manifest.py — full-run-safety
  • tests/test_carried_hyperedge_remap.py — full-run-safety
  • tests/test_case_sensitive_resolution.py — full-run-safety
  • tests/test_charmap_encoding.py — full-run-safety
  • tests/test_chunking.py — full-run-safety
  • tests/test_cjs_module_extension.py — full-run-safety
  • tests/test_claude_cli_backend.py — full-run-safety
  • tests/test_claude_md.py — full-run-safety
  • tests/test_cli_broken_pipe.py — full-run-safety
  • tests/test_cli_export.py — full-run-safety
  • tests/test_cli_help.py — full-run-safety
  • tests/test_cloud_cta.py — full-run-safety
  • tests/test_cluster.py — full-run-safety
  • tests/test_cluster_ambiguous_scale.py — full-run-safety
  • tests/test_cluster_exclude_hubs.py — full-run-safety
  • tests/test_cobol_extractor.py — full-run-safety
  • tests/test_codebuddy.py — full-run-safety
  • tests/test_community_hub_labels.py — full-run-safety
  • tests/test_community_labels_skill.py — full-run-safety
  • tests/test_confidence.py — full-run-safety
  • tests/test_corrupt_graph_json.py — full-run-safety
  • … and 318 more

non-code file(s) changed (graphify/skill-aider.md, graphify/skill-devin.md, graphify/skills/agents/references/query.md, graphify/skills/amp/references/query.md, graphify/skills/claude/references/query.md …) → running the full suite for safety (a code graph can't see config/fixture/data deps)

changed code file(s) with no mapped test (graphify/skill-aider.md, graphify/skill-devin.md, graphify/skills/agents/references/query.md, graphify/skills/amp/references/query.md, graphify/skills/claude/references/query.md …) — a coverage gap or a missing link — running the full suite rather than only the selected tests

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.

Docs that may be stale (advisory)

This branch has not been deployed

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

Labels

contributor Pull request from a returning community contributor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(query): fallback BFS/DFS traversal omits relationships between visited nodes

1 participant