fix(extract): warn when files skip extraction for a missing optional dep (#1745)
When the [sql] extra is absent, .sql files are counted as code and scanned but extract_sql returns an error result and zero nodes — and the graph builds "successfully" with the entire SQL corpus missing. Neither existing warning catches it: #1666's zero-node warning skips results carrying an "error", and #1689 only covers files with NO extractor at all (.sql HAS a dispatch entry). extract() now scans per-file results for a "not installed" error, groups the affected files by extension, and prints a warning naming the extra that restores the language (pip install "graphifyy[sql]"), via a small _EXTRA_FOR_EXTENSION map (sql, terraform, dm). The map is only consulted after an extractor actually reports the dependency missing, so it can't mislabel a language that has a working fallback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
committed by
safishamsi
co-authored by
Claude Opus 4.8
parent
29a6954f91
commit
8b7ffc5b15
@@ -4,6 +4,8 @@ Full release notes with details on each version: [GitHub Releases](https://githu
|
||||
|
||||
## 0.9.12 (unreleased)
|
||||
|
||||
- Fix: files whose extractor bailed out for a missing optional dependency no longer vanish without a trace (#1745, thanks @rithyKabir). `.sql` files (and other extra-gated languages) have a dispatch entry, so the #1689 no-extractor warning can't fire, and `extract_sql` returns an error result when `tree-sitter-sql` is absent, so the #1666 zero-node warning skips it too — the graph built "successfully" while an entire SQL corpus contributed nothing. `extract()` now surfaces these grouped by extension, naming the extra that restores the language (e.g. `pip install "graphifyy[sql]"`).
|
||||
|
||||
- Fix: `build_from_json` is deterministic across process runs again (#1753, thanks @erasmust-dotcom). The ghost-node merge iterated `set(G.nodes())`, so which node survived a `(basename, label)` collision depended on CPython's per-process string-hash seed — rebuilding the same extraction JSON in a fresh process could silently pick a different canonical id (breaking the cluster→relabel workflow with a `KeyError` on an id that vanished). The Pass 1/Pass 2 loops now iterate in sorted order. Additionally, two non-AST (semantic) nodes sharing a key but from *different* files are now treated as distinct concepts and both survive (mirroring the AST/AST ambiguity guard #1257) instead of one arbitrarily merging away; a genuine same-file duplicate still collapses.
|
||||
|
||||
- Fix: a Java field/parameter/return-type reference to a class whose simple name is shared by two modules no longer dangles on a sourceless phantom node (#1744, thanks @aviciot). Both same-named classes already survive as distinct path-scoped nodes, but the cross-module `references` edge was left pointing at a bare no-source stub because `_resolve_java_type_references` re-pointed `implements`/`inherits`/`imports` but not `references` — so a query about the referenced class could miss it. The Java resolver now disambiguates `references` by the importing file's `import` statement (falling back to same-package), mirroring the C# resolver, and drops the orphaned phantom.
|
||||
|
||||
@@ -3742,6 +3742,20 @@ _DISPATCH: dict[str, Any] = {
|
||||
}
|
||||
|
||||
|
||||
# Extensions whose extractor depends on an optional-dependency extra
|
||||
# (pyproject [project.optional-dependencies]) and hard-fails without it,
|
||||
# rather than falling back like Pascal does. Used by the #1745 warning in
|
||||
# extract() to tell the user which extra restores the language.
|
||||
_EXTRA_FOR_EXTENSION = {
|
||||
".sql": "sql",
|
||||
".tf": "terraform",
|
||||
".tfvars": "terraform",
|
||||
".hcl": "terraform",
|
||||
".dm": "dm",
|
||||
".dme": "dm",
|
||||
}
|
||||
|
||||
|
||||
# Extensionless executables (CLI entry points like `devctl` or `manage`) carry
|
||||
# their language in the shebang, not the suffix. detect.classify_file already
|
||||
# routes them to the CODE path via _shebang_interpreter; _get_extractor must
|
||||
@@ -4201,6 +4215,35 @@ def extract(
|
||||
file=sys.stderr, flush=True,
|
||||
)
|
||||
|
||||
# #1745: an extractor IS wired up for these files but bailed out because its
|
||||
# dependency is missing (e.g. .sql needs tree-sitter-sql from the [sql]
|
||||
# extra). Neither warning above fires — #1666 skips results that carry an
|
||||
# error, #1689 only covers files with no extractor — so the graph builds
|
||||
# "successfully" while every such file silently contributes nothing.
|
||||
# Surface them grouped by extension, naming the extra that provides the
|
||||
# dependency when there is one.
|
||||
_missing_dep_count: dict[str, int] = {}
|
||||
_missing_dep_error: dict[str, str] = {}
|
||||
for i, _p in enumerate(paths):
|
||||
_err = (per_file[i] or {}).get("error") or ""
|
||||
if "not installed" in _err:
|
||||
_ext = _p.suffix.lower()
|
||||
_missing_dep_count[_ext] = _missing_dep_count.get(_ext, 0) + 1
|
||||
_missing_dep_error.setdefault(_ext, _err)
|
||||
for _ext, _n in sorted(_missing_dep_count.items(), key=lambda kv: (-kv[1], kv[0])):
|
||||
_extra = _EXTRA_FOR_EXTENSION.get(_ext)
|
||||
if _extra:
|
||||
_reason = _missing_dep_error[_ext].split(". ")[0]
|
||||
_hint = f' Install it with: pip install "graphifyy[{_extra}]"'
|
||||
else:
|
||||
_reason = _missing_dep_error[_ext]
|
||||
_hint = ""
|
||||
print(
|
||||
f" warning: {_n} {_ext} file(s) contributed nothing to the graph "
|
||||
f"because a dependency is missing: {_reason}.{_hint} (#1745)",
|
||||
file=sys.stderr, flush=True,
|
||||
)
|
||||
|
||||
all_nodes: list[dict] = []
|
||||
all_edges: list[dict] = []
|
||||
all_raw_calls: list[dict] = []
|
||||
|
||||
@@ -1,7 +1,11 @@
|
||||
import json
|
||||
import os
|
||||
import sys
|
||||
from collections import Counter
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from graphify.extract import extract_python, extract, collect_files, _make_id, extract_bash, extract_json, _DISPATCH
|
||||
|
||||
FIXTURES = Path(__file__).parent / "fixtures"
|
||||
@@ -1828,6 +1832,36 @@ def test_extract_no_warning_when_all_code_has_extractors(tmp_path, capsys):
|
||||
assert "no AST extractor" not in err
|
||||
|
||||
|
||||
def test_extract_warns_when_sql_extra_missing(tmp_path, capsys, monkeypatch):
|
||||
# #1745: .sql HAS a dispatch entry, so the #1689 warning can't fire, and
|
||||
# extract_sql returns an "error" result when tree-sitter-sql is absent, so
|
||||
# the #1666 warning skips it too. The files must not vanish silently:
|
||||
# extract() surfaces them with the [sql] extra named.
|
||||
monkeypatch.setitem(sys.modules, "tree_sitter_sql", None) # import -> ImportError
|
||||
s1 = tmp_path / "schema.sql"; s1.write_text("CREATE TABLE users (id INT);\n")
|
||||
s2 = tmp_path / "views.sql"; s2.write_text("CREATE VIEW v AS SELECT * FROM users;\n")
|
||||
py = tmp_path / "main.py"; py.write_text("def main():\n return 1\n")
|
||||
|
||||
result = extract([s1, s2, py], cache_root=tmp_path)
|
||||
err = capsys.readouterr().err
|
||||
|
||||
assert "2 .sql file(s)" in err
|
||||
assert "tree_sitter_sql not installed" in err
|
||||
assert 'graphifyy[sql]' in err
|
||||
assert "#1745" in err
|
||||
# the Python file still extracts normally
|
||||
labels = [n.get("label") for n in result["nodes"]]
|
||||
assert any(str(l).startswith("main") for l in labels)
|
||||
|
||||
|
||||
def test_extract_no_missing_dep_warning_when_sql_installed(tmp_path, capsys):
|
||||
pytest.importorskip("tree_sitter_sql")
|
||||
s = tmp_path / "schema.sql"; s.write_text("CREATE TABLE users (id INT);\n")
|
||||
extract([s], cache_root=tmp_path)
|
||||
err = capsys.readouterr().err
|
||||
assert "#1745" not in err
|
||||
|
||||
|
||||
def test_extract_progress_final_line_uses_consistent_denominator(tmp_path, capsys):
|
||||
# #1693: intermediate progress lines count against uncached_work; the final
|
||||
# "100%" line must NOT switch to total_files (which includes cached hits and
|
||||
|
||||
Reference in New Issue
Block a user