Skip to content

fix(analyze): exclude contains-only AST declarations from knowledge gap detection (#4205) - #4212

Closed
nothariharan wants to merge 1 commit into
Graphify-Labs:v8from
nothariharan:fix/4205-ast-contains-only-gaps
Closed

nothariharan wants to merge 1 commit into
Graphify-Labs:v8from
nothariharan:fix/4205-ast-contains-only-gaps

Conversation

@nothariharan

Copy link
Copy Markdown
Contributor

Summary

  • Weakly-connected / Knowledge Gaps signal (suggest_questions() and GRAPH_REPORT.md) flagged plain AST declarations — TS type aliases, enum members, local consts, JSON config keys — whose only edge is the contains edge from their declaring file, because _is_file_node only recognized labels ending in ().
  • New _is_contains_only_ast_node helper in graphify/analyze.py excludes any AST-tier node (_origin == 'ast', with the legacy source_location fallback via _is_ast_tier) whose sole edge is a contains.
  • Applied to both the suggest_questions() isolated-nodes filter and the GRAPH_REPORT.md Knowledge Gaps isolated count.

Semantic-tier nodes with only a contains edge are still flagged; AST nodes whose single edge is something else (e.g. calls) are still flagged.

Fixes #4205

Test plan

  • 3 new regression tests in ests/test_report_gap_thresholds.py (contains-only AST excluded, semantic contains-only still flagged, AST node with non-contains single edge still flagged).
  • python -m pytest tests/test_report_gap_thresholds.py → 11 passed.

@github-actions

github-actions Bot commented Oct 8, 2026

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.

@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.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.

Formal verification. PR-changed functions: 1/2 verified (0 proven, 1 may-equivalent, 0 distinguished) · 1 not verified (1 vacuous).

Not verified on this run: generate (vacuous: never exercised).


Graphify review — findings

Stops reporting AST declarations like type aliases, enum members, local consts and JSON keys as knowledge gaps when their only edge is the structural contains from their declaring file. The new _is_contains_only_ast_node check removes them from both the report's Knowledge Gaps section and the isolated_nodes question in suggest_questions. Semantic nodes with a lone contains edge, and AST nodes whose single edge is any other relation, still count as gaps.

Worth a look

  • Contains-only AST check never matches on directed graphs — graphify/analyze.py:212 · 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 — 724 functions depend on the 69 functions this change touches.

Health — this change adds coupling hotspots:

  • new: _rebuild_code() — 149 callers, 56 callees
  • new: to_obsidian() — 41 callers, 14 callees
  • new: to_json() — 61 callers, 8 callees
  • new: generate() — 39 callers, 9 callees
  • new: main() — 102 callers, 3 callees
  • new: to_html() — 24 callers, 11 callees
  • new: dispatch_command() — 2 callers, 128 callees
  • new: _make_graph() — 37 callers, 6 callees
  • …and 24 more — each is listed as a finding

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

Test selection

Test selection

39 of 334 test file(s) selected (12%) via static blast radius.

  • tests/test_analyze.py — impact
  • tests/test_atomic_canvas_export.py — impact
  • tests/test_atomic_writes.py — impact
  • tests/test_build.py — impact
  • tests/test_carried_hyperedge_remap.py — impact
  • tests/test_cli_export.py — impact
  • tests/test_community_labels_skill.py — impact
  • tests/test_confidence.py — impact
  • tests/test_cross_extension_reexport_self_cycle.py — impact
  • tests/test_dedup_shrink_refuses_force_write.py — impact
  • tests/test_export.py — impact
  • tests/test_export_control_characters.py — impact
  • tests/test_export_direction.py — impact
  • tests/test_export_idempotent_writes.py — impact
  • tests/test_export_path_length.py — impact
  • tests/test_falkordb_integration.py — impact
  • tests/test_go_qualified_resolution.py — impact
  • tests/test_god_nodes_exclude_hubs.py — impact
  • tests/test_graphdb_push_indexes.py — impact
  • tests/test_hyperedge_roundtrip.py — impact
  • tests/test_hypergraph.py — impact
  • tests/test_js_import_resolution.py — impact
  • tests/test_labeling.py — impact
  • tests/test_obsidian_dangling_member.py — impact
  • tests/test_obsidian_filename_cap.py — impact
  • tests/test_obsidian_unicode_tags.py — impact
  • tests/test_obsidian_vault_migration.py — impact
  • tests/test_pipeline.py — impact
  • tests/test_python_import_resolution.py — impact
  • tests/test_reflect.py — impact
  • tests/test_report.py — impact
  • tests/test_report_gap_thresholds.py — impact, changed-test
  • tests/test_semantic_similarity.py — impact
  • tests/test_serve.py — impact
  • tests/test_serve_http.py — impact
  • tests/test_swift_builtin_noise.py — impact
  • tests/test_terraform.py — impact
  • tests/test_type_only_import_cycles.py — impact
  • tests/test_watch.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.

Formal verification

No difference found (not proven): No behavior difference found in suggest\_questions (not a proof).

The verifier ran both versions of suggest\_questions on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.

Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.

Note: An input the sampler did not try could still differ.

Could not verify: Could not verify generate.

The verifier did not have enough to check generate, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: no capturable inputs from the test suite; property tier: not verifiable: all 264 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly ValueError — names the real obstacle, not a sampling gap)

· 32 more finding(s) on lines outside this diff (see the check run).

@safishamsi

Copy link
Copy Markdown
Member

Landed in v0.9.81 via an authorship-preserving cherry-pick, so your commit is on v8 with you credited as the author. Closing as shipped — thanks @nothariharan!

@safishamsi safishamsi closed this Oct 8, 2026
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.

Weakly-connected / "Knowledge Gaps" signal doesn't filter AST-origin declarations without a () suffix — false positives dominate

2 participants