build_from_json now folds name->label, path->source_file, edge type->relation, and confidence_score->confidence=INFERRED before validation, so alias-carrying nodes stop entering the graph without label/source_file (invisible, unmergeable ghosts); the same folds run before dedup. _semantic_id_remap now also learns the absolute-path stem form, so a Windows absolute-derived semantic id re-keys to the canonical root-relative id. The extraction warning now breaks errors down by cause. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
334cff6172
commit
8adb261d16
+85
-2
@@ -124,6 +124,39 @@ def _normalize_hyperedge_members(he: object) -> None:
|
||||
he.pop(alias, None)
|
||||
|
||||
|
||||
def _fold_node_aliases(node: dict) -> None:
|
||||
"""Fold legacy node field aliases onto canonical keys, in place (#2194).
|
||||
|
||||
``name`` -> ``label`` and ``path`` -> ``source_file``. Uses an empty-check
|
||||
(not mere key presence) so a node carrying ``label: ""``/``None`` next to a
|
||||
real ``name`` is healed too. When the canonical field already holds a value
|
||||
it wins and the alias key is left untouched. Without this fold an alias-only
|
||||
node enters the graph with no label/source_file: it fails validation, gets
|
||||
``norm_label == ""`` (invisible to query/explain), and is excluded from every
|
||||
label-keyed merge/dedup — a permanent ghost that ``graphify update``
|
||||
re-feeds through build_from_json forever.
|
||||
"""
|
||||
if not node.get("label") and isinstance(node.get("name"), str) and node["name"]:
|
||||
node["label"] = node.pop("name")
|
||||
if not node.get("source_file") and isinstance(node.get("path"), str) and node["path"]:
|
||||
node["source_file"] = node.pop("path")
|
||||
|
||||
|
||||
def _fold_edge_aliases(edge: dict) -> None:
|
||||
"""Fold legacy edge field aliases onto canonical keys, in place (#2194).
|
||||
|
||||
``type`` -> ``relation``. A ``confidence_score`` float with no ``confidence``
|
||||
enum backfills ``confidence: "INFERRED"`` — never EXTRACTED (alias recovery
|
||||
is not provenance) and never a threshold mapping of the float. The
|
||||
``confidence_score`` key itself is NOT popped: it is a legitimate companion
|
||||
field that the edge loop sanitizes and to_json round-trips.
|
||||
"""
|
||||
if not edge.get("relation") and isinstance(edge.get("type"), str) and edge["type"]:
|
||||
edge["relation"] = edge.pop("type")
|
||||
if not edge.get("confidence") and edge.get("confidence_score") is not None:
|
||||
edge["confidence"] = "INFERRED"
|
||||
|
||||
|
||||
def _norm_source_file(p: str | None, root: str | None = None) -> str | None:
|
||||
"""Normalize path separators and relativize absolute paths.
|
||||
|
||||
@@ -395,7 +428,23 @@ def _semantic_id_remap(nodes: list, root: str | None) -> dict:
|
||||
if norm_nid == new_stem or norm_nid.startswith(new_stem + "_"):
|
||||
continue
|
||||
new_id: str | None = None
|
||||
for old_stem in _old_file_stems(rel):
|
||||
old_forms = _old_file_stems(rel)
|
||||
# #2197: on Windows, detect() can emit an ABSOLUTE source_file, and a
|
||||
# semantic fragment's id derived from that absolute path (e.g.
|
||||
# d_projects_myrepo_docs_dataflow) matches neither the canonical
|
||||
# relative stem nor the legacy short forms above — so while source_file
|
||||
# itself is healed by _norm_source_file, the id would ghost against the
|
||||
# existing graph's docs_dataflow. When the raw path was absolute and
|
||||
# relativized under root, treat the raw-absolute stem as one more
|
||||
# old-stem form — the semantic-side twin of extract.py's absolute-form
|
||||
# id registration. It is the longest form, so it goes first (greedy
|
||||
# prefix stripping, same ordering rule as _old_file_stems).
|
||||
sf_raw = str(sf).replace("\\", "/")
|
||||
if sf_raw != sf_norm and os.path.isabs(sf_raw):
|
||||
abs_stem = make_id(_file_stem(Path(sf_raw)))
|
||||
if abs_stem and abs_stem != new_stem and abs_stem not in old_forms:
|
||||
old_forms.insert(0, abs_stem)
|
||||
for old_stem in old_forms:
|
||||
if old_stem == new_stem:
|
||||
continue # already canonical for this form
|
||||
if norm_nid == old_stem:
|
||||
@@ -518,6 +567,11 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat
|
||||
file=sys.stderr,
|
||||
)
|
||||
node["source_file"] = node.pop("source")
|
||||
# Fold the remaining legacy node aliases (`name`->`label`,
|
||||
# `path`->`source_file`, #2194) before validation and before the
|
||||
# semantic-rekey / ghost-merge passes below, all of which key on
|
||||
# label/source_file and would otherwise skip the node entirely.
|
||||
_fold_node_aliases(node)
|
||||
# Default missing/None file_type to "concept" so legacy graph.json
|
||||
# entries (and stub nodes preserved by `_rebuild_code` from older
|
||||
# graphify versions that didn't always populate file_type) don't
|
||||
@@ -536,11 +590,33 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat
|
||||
for he in extraction.get("hyperedges", []) or []:
|
||||
_normalize_hyperedge_members(he)
|
||||
|
||||
# Fold legacy edge field aliases (`type`->`relation`,
|
||||
# `confidence_score`->`confidence`, #2194) BEFORE validation. The existing
|
||||
# from/to endpoint fold lives in the edge loop further down, which runs
|
||||
# after validate_extraction — too late for fields the validator requires.
|
||||
for edge in extraction.get("edges", []):
|
||||
if isinstance(edge, dict):
|
||||
_fold_edge_aliases(edge)
|
||||
|
||||
errors = validate_extraction(extraction)
|
||||
# Dangling edges (stdlib/external imports) are expected - only warn about real schema errors.
|
||||
real_errors = [e for e in errors if "does not match any node id" not in e]
|
||||
if real_errors:
|
||||
print(f"[graphify] Extraction warning ({len(real_errors)} issues): {real_errors[0]}", file=sys.stderr)
|
||||
# Break the warning down by cause (#2194): a mixed batch used to surface
|
||||
# only real_errors[0], hiding every other failure mode. Group on the
|
||||
# "missing required field 'X'" suffix and report per-cause counts plus
|
||||
# one example each, so the operator sees the full shape of the damage.
|
||||
by_cause: dict[str, list[str]] = {}
|
||||
for err in real_errors:
|
||||
m = re.search(r"missing required field '[^']*'", err)
|
||||
by_cause.setdefault(m.group(0) if m else "other schema issue", []).append(err)
|
||||
breakdown = "; ".join(
|
||||
f"{len(errs)}x {cause} (e.g. {errs[0]})" for cause, errs in by_cause.items()
|
||||
)
|
||||
print(
|
||||
f"[graphify] Extraction warning ({len(real_errors)} issues): {breakdown}",
|
||||
file=sys.stderr,
|
||||
)
|
||||
# Deterministic semantic re-key (#1504/#1509): the node-ID stem is now the
|
||||
# full repo-relative path (docs/v1/api/README.md -> docs_v1_api_readme), but
|
||||
# the semantic cache is UNVERSIONED, so a cached/LLM fragment can still carry
|
||||
@@ -972,6 +1048,13 @@ def build(
|
||||
combined["input_tokens"] += ext.get("input_tokens", 0)
|
||||
combined["output_tokens"] += ext.get("output_tokens", 0)
|
||||
if dedup and combined["nodes"]:
|
||||
# Fold legacy node field aliases before dedup (#2194): dedup runs BEFORE
|
||||
# build_from_json and keys on `label`, so a `name`/`path` alias node
|
||||
# would be invisible to it and only label-dedup one build later, after
|
||||
# build_from_json's own fold has healed the persisted graph.json.
|
||||
for n in combined["nodes"]:
|
||||
if isinstance(n, dict):
|
||||
_fold_node_aliases(n)
|
||||
combined["nodes"], combined["edges"] = deduplicate_entities(
|
||||
combined["nodes"], combined["edges"], communities={},
|
||||
dedup_llm_backend=dedup_llm_backend,
|
||||
|
||||
@@ -135,6 +135,145 @@ def test_legacy_edge_from_to_canonicalized():
|
||||
assert G.number_of_edges() == 1
|
||||
|
||||
|
||||
def test_legacy_node_name_path_aliases_folded():
|
||||
"""#2194: nodes carrying `name`/`path` instead of `label`/`source_file` must
|
||||
be canonicalized before validation, not enter the graph as label-less
|
||||
ghosts. After build the canonicalized dict also passes validation."""
|
||||
from graphify.validate import validate_extraction
|
||||
ext = {"nodes": [{"id": "n1", "name": "Foo", "path": "a/b.md", "file_type": "concept"}],
|
||||
"edges": [], "input_tokens": 0, "output_tokens": 0}
|
||||
G = build_from_json(ext)
|
||||
attrs = G.nodes["n1"]
|
||||
assert attrs["label"] == "Foo"
|
||||
assert attrs["source_file"] == "a/b.md"
|
||||
assert "name" not in attrs
|
||||
assert "path" not in attrs
|
||||
# build_from_json canonicalizes in place; the extraction dict must now be
|
||||
# schema-valid (no missing-field errors for the alias node).
|
||||
assert not [e for e in validate_extraction(ext) if "missing required field" in e]
|
||||
|
||||
|
||||
def test_legacy_edge_type_confidence_score_aliases_folded():
|
||||
"""#2194: edges carrying `type`/`confidence_score` instead of
|
||||
`relation`/`confidence` fold to canonical fields. Recovery confidence is
|
||||
INFERRED (never EXTRACTED — alias recovery is not provenance) and the
|
||||
companion confidence_score float is retained, not popped."""
|
||||
ext = {"nodes": [{"id": "n1", "label": "A", "file_type": "code", "source_file": "a.py"},
|
||||
{"id": "n2", "label": "B", "file_type": "code", "source_file": "b.py"}],
|
||||
"edges": [{"source": "n1", "target": "n2", "type": "references",
|
||||
"confidence_score": 0.9, "source_file": "a.py"}],
|
||||
"input_tokens": 0, "output_tokens": 0}
|
||||
G = build_from_json(ext)
|
||||
data = edge_data(G, "n1", "n2")
|
||||
assert data["relation"] == "references"
|
||||
assert data["confidence"] == "INFERRED"
|
||||
assert data["confidence_score"] == 0.9
|
||||
assert "type" not in data
|
||||
|
||||
|
||||
def test_node_alias_canonical_field_wins():
|
||||
"""#2194: when both the canonical field and its alias are present, the
|
||||
canonical value wins and the alias key is left untouched."""
|
||||
ext = {"nodes": [{"id": "n1", "label": "Real", "name": "Alias",
|
||||
"file_type": "code", "source_file": "a.py"}],
|
||||
"edges": [], "input_tokens": 0, "output_tokens": 0}
|
||||
G = build_from_json(ext)
|
||||
assert G.nodes["n1"]["label"] == "Real"
|
||||
assert G.nodes["n1"]["name"] == "Alias" # preserved, not consumed
|
||||
|
||||
|
||||
def test_alias_node_ghost_merges_into_ast_twin():
|
||||
"""#2194: an alias-only semantic node (name/path) must participate in the
|
||||
AST/LLM ghost merge once folded — same label and file as an AST node with a
|
||||
different id collapses into the AST node instead of surviving as a ghost."""
|
||||
ext = {"nodes": [
|
||||
{"id": "src_foo_helper", "label": "helper", "file_type": "code",
|
||||
"source_file": "src/foo.py", "_origin": "ast", "source_location": "L10"},
|
||||
{"id": "helper_ghost", "name": "helper", "path": "src/foo.py",
|
||||
"file_type": "code"},
|
||||
], "edges": [], "input_tokens": 0, "output_tokens": 0}
|
||||
G = build_from_json(ext)
|
||||
assert "src_foo_helper" in G.nodes
|
||||
assert "helper_ghost" not in G.nodes
|
||||
assert G.number_of_nodes() == 1
|
||||
|
||||
|
||||
def test_alias_node_gets_nonempty_norm_label(tmp_path):
|
||||
"""#2194: a recovered alias node must serialize with a non-empty norm_label
|
||||
so query/explain can find it."""
|
||||
from graphify.export import to_json
|
||||
ext = {"nodes": [{"id": "n1", "name": "Foo", "path": "a/b.md", "file_type": "concept"}],
|
||||
"edges": [], "input_tokens": 0, "output_tokens": 0}
|
||||
G = build_from_json(ext)
|
||||
out = tmp_path / "graph.json"
|
||||
assert to_json(G, {}, str(out))
|
||||
data = json.loads(out.read_text())
|
||||
node = next(n for n in data["nodes"] if n["id"] == "n1")
|
||||
assert node["norm_label"] == "foo"
|
||||
|
||||
|
||||
def test_extraction_warning_breakdown_by_cause(capsys):
|
||||
"""#2194: a mixed batch of schema errors must report per-cause counts, not
|
||||
just the first error."""
|
||||
ext = {"nodes": [
|
||||
{"id": "n1", "label": "A", "file_type": "code", "source_file": "a.py"},
|
||||
{"id": "n2", "label": "B", "file_type": "code", "source_file": "b.py"},
|
||||
# two nodes missing label (and carrying no name alias)
|
||||
{"id": "x1", "file_type": "code", "source_file": "x.py"},
|
||||
{"id": "x2", "file_type": "code", "source_file": "x.py"},
|
||||
], "edges": [
|
||||
# three edges missing relation (and carrying no type alias)
|
||||
{"source": "n1", "target": "n2", "confidence": "EXTRACTED", "source_file": "a.py"},
|
||||
{"source": "n2", "target": "n1", "confidence": "EXTRACTED", "source_file": "a.py"},
|
||||
{"source": "n1", "target": "x1", "confidence": "EXTRACTED", "source_file": "a.py"},
|
||||
], "input_tokens": 0, "output_tokens": 0}
|
||||
build_from_json(ext)
|
||||
err = capsys.readouterr().err
|
||||
assert "2x missing required field 'label'" in err
|
||||
assert "3x missing required field 'relation'" in err
|
||||
|
||||
|
||||
def test_absolute_derived_semantic_ids_rekeyed(tmp_path):
|
||||
"""#2197: a semantic fragment whose ids were derived from an ABSOLUTE
|
||||
source_file (Windows detect() emits them) must re-key to the canonical
|
||||
repo-relative stem instead of ghosting against the existing graph."""
|
||||
from graphify.ids import make_id
|
||||
(tmp_path / "docs").mkdir()
|
||||
abs_sf = str(tmp_path / "docs" / "DATAFLOW.md")
|
||||
abs_stem = make_id(str(tmp_path / "docs" / "DATAFLOW"))
|
||||
ext = {"nodes": [
|
||||
{"id": abs_stem, "label": "DATAFLOW.md", "file_type": "document",
|
||||
"source_file": abs_sf},
|
||||
{"id": f"{abs_stem}_pipeline", "label": "Pipeline", "file_type": "concept",
|
||||
"source_file": abs_sf},
|
||||
], "edges": [
|
||||
{"source": abs_stem, "target": f"{abs_stem}_pipeline", "relation": "describes",
|
||||
"confidence": "INFERRED", "source_file": abs_sf, "weight": 1.0},
|
||||
], "input_tokens": 0, "output_tokens": 0}
|
||||
G = build_from_json(ext, root=tmp_path)
|
||||
assert "docs_dataflow" in G.nodes
|
||||
assert "docs_dataflow_pipeline" in G.nodes
|
||||
assert abs_stem not in G.nodes
|
||||
assert G.nodes["docs_dataflow"]["source_file"] == "docs/DATAFLOW.md"
|
||||
assert G.has_edge("docs_dataflow", "docs_dataflow_pipeline")
|
||||
|
||||
|
||||
def test_absolute_derived_semantic_ids_rekeyed_backslash(tmp_path):
|
||||
"""#2197 (separator variant): the same absolute-derived-id fragment with
|
||||
backslash separators in source_file re-keys identically."""
|
||||
from graphify.ids import make_id
|
||||
(tmp_path / "docs").mkdir()
|
||||
abs_sf = str(tmp_path / "docs" / "DATAFLOW.md").replace("/", "\\")
|
||||
abs_stem = make_id(str(tmp_path / "docs" / "DATAFLOW"))
|
||||
ext = {"nodes": [
|
||||
{"id": f"{abs_stem}_pipeline", "label": "Pipeline", "file_type": "concept",
|
||||
"source_file": abs_sf},
|
||||
], "edges": [], "input_tokens": 0, "output_tokens": 0}
|
||||
G = build_from_json(ext, root=tmp_path)
|
||||
assert "docs_dataflow_pipeline" in G.nodes
|
||||
assert G.nodes["docs_dataflow_pipeline"]["source_file"] == "docs/DATAFLOW.md"
|
||||
|
||||
|
||||
def test_source_file_backslash_normalized():
|
||||
"""Windows backslash paths and POSIX paths for the same file must produce one node."""
|
||||
extraction = {
|
||||
|
||||
@@ -87,6 +87,27 @@ def test_assert_valid_passes_silently():
|
||||
assert_valid(VALID) # should not raise
|
||||
|
||||
|
||||
def test_legacy_aliases_valid_after_build_canonicalization():
|
||||
# #2194: build_from_json folds legacy aliases (name->label,
|
||||
# path->source_file, type->relation, confidence_score->confidence) in
|
||||
# place BEFORE validation, so an alias-only extraction that fails
|
||||
# validation raw is fully schema-valid after canonicalization.
|
||||
from graphify.build import build_from_json
|
||||
data = {
|
||||
"nodes": [
|
||||
{"id": "n1", "name": "Foo", "path": "a/b.md", "file_type": "concept"},
|
||||
{"id": "n2", "label": "Bar", "file_type": "code", "source_file": "bar.py"},
|
||||
],
|
||||
"edges": [
|
||||
{"source": "n1", "target": "n2", "type": "references",
|
||||
"confidence_score": 0.9, "source_file": "a/b.md"},
|
||||
],
|
||||
}
|
||||
assert any("missing required field" in e for e in validate_extraction(data))
|
||||
build_from_json(data)
|
||||
assert validate_extraction(data) == []
|
||||
|
||||
|
||||
def test_non_hashable_node_id_reported_not_raised():
|
||||
# A malformed LLM extraction can emit a list-valued id. The validator must
|
||||
# report it as an error string (its documented contract) rather than crash
|
||||
|
||||
Reference in New Issue
Block a user