Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions graphify/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,55 @@ def _is_ast_tier(item: dict) -> bool:
return isinstance(loc, str) and bool(_AST_LOC_RE.match(loc))


def _lost_saved_semantic_nodes(
Comment thread
SrijanSriv marked this conversation as resolved.
saved_nodes: list,
new_nodes: list,
deleted_sources: set[str] | None = None,
) -> list[dict]:
"""Saved semantic nodes whose ids are absent from the new graph.

A node created in this run is not in ``saved_nodes``, so a sourceless stub
cannot stand in for a missing id. ``deleted_sources`` is the files the user
removed. A code rebuild is not a deletion: do not pass the rebuilt set.
"""
deleted = {
norm
for source in (deleted_sources or ())
if (norm := _norm_source_file(source))
}
new_ids = {node.get("id") for node in new_nodes if isinstance(node, dict)}
# cluster-only and build_from_json rewrite a legacy semantic id to its
# canonical stem, then fold a bare doc id into its `_doc` twin. That id
# is the same node. A missing rationale id has no such successor.
stem_remap = _semantic_id_remap(saved_nodes, None)
rewritten = [
{**node, "id": stem_remap[node["id"]]}
if isinstance(node, dict) and node.get("id") in stem_remap
else node
for node in saved_nodes
]
doc_remap = _doc_twin_remap(rewritten)

def _still_there(node_id: str) -> bool:
if node_id in new_ids:
return True
renamed = doc_remap.get(stem_remap.get(node_id, node_id), stem_remap.get(node_id, node_id))
return renamed in new_ids

lost: list[dict] = []
for node in saved_nodes:
if not isinstance(node, dict) or _is_ast_tier(node):
continue
node_id = node.get("id")
if isinstance(node_id, str) and _still_there(node_id):
continue
source = _norm_source_file(node.get("source_file"))
if source and source in deleted:
continue
lost.append(node)
return lost


# Relations that say only "these two symbols appear together", with no claim about
# HOW. An extractor that finds a specific fact for a pair — a call, an import, an
# inheritance — routinely emits one of these for the same pair as well, so when the
Expand Down
21 changes: 20 additions & 1 deletion graphify/export.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,10 +296,14 @@ def to_json(G: nx.Graph, communities: dict[int, list[str]], output_path: str, *,
# Empty/whitespace existing file (e.g. a freshly touched path):
# no nodes to lose, so any new graph is a growth — proceed.
existing_n = 0
existing_nodes = []
else:
try:
existing_data = json.loads(raw)
existing_n = len(existing_data.get("nodes", []))
existing_nodes = existing_data.get("nodes", [])
if not isinstance(existing_nodes, list):
existing_nodes = []
existing_n = len(existing_nodes)
except Exception as exc:
# Non-empty but unparseable existing graph (corrupt or a
# mid-write): we cannot verify the new graph is not a silent
Expand All @@ -316,6 +320,21 @@ def to_json(G: nx.Graph, communities: dict[int, list[str]], output_path: str, *,
)
return False
new_n = G.number_of_nodes()
from graphify.build import _lost_saved_semantic_nodes
new_node_list = [
{"id": node_id, **{key: value for key, value in attrs.items() if key != "id"}}
for node_id, attrs in G.nodes(data=True)
]
missing = _lost_saved_semantic_nodes(existing_nodes, new_node_list, set())
if missing:
import sys as _sys
ids = ", ".join(str(node.get("id")) for node in missing)
print(
"[graphify] WARNING: Refusing to overwrite — saved semantic "
f"node(s) are missing from the new graph: {ids}.",
file=_sys.stderr,
)
return False
if new_n < existing_n:
import sys as _sys
print(
Expand Down
29 changes: 29 additions & 0 deletions graphify/watch.py
Original file line number Diff line number Diff line change
Expand Up @@ -1227,6 +1227,7 @@ def _check_shrink(
had_explicit_deletions: bool = False,
rebuilt_sources: "set[str] | None" = None,
failed_sources: "set[str] | None" = None,
deleted_sources: "set[str] | None" = None,
) -> bool:
"""Return True (ok to proceed) or False (shrink refused).

Expand All @@ -1248,6 +1249,12 @@ def _check_shrink(
``--force`` (#1116 left stale nodes write-blocked even though build dropped them).
Files in ``failed_sources`` never account for lost nodes: extraction did not
complete, so their disappearance is the silent shrink this guard protects.

A larger total is not enough when a saved semantic id is gone (#2229).
``deleted_sources`` is the files the user removed. A semantic node on one of
those files may go. A semantic node on a file that is still present may not.
The rebuilt set is not that list: an update re-extracts code and must not
treat a rationale node on that file as a deleted symbol.
"""
if force or not existing_data:
return True
Expand All @@ -1263,6 +1270,26 @@ def _check_shrink(
return True
existing_nodes = existing_data.get("nodes", [])
new_nodes = new_data.get("nodes", [])
from graphify.build import _lost_saved_semantic_nodes
missing = _lost_saved_semantic_nodes(existing_nodes, new_nodes, deleted_sources)
if missing:
if tmp is not None:
tmp.unlink(missing_ok=True)
ids = ", ".join(str(node.get("id")) for node in missing)
print(
"[graphify] WARNING: Refusing to overwrite — saved semantic "
f"node(s) are missing from the new graph: {ids}.",
file=sys.stderr,
)
if len(new_nodes) < len(existing_nodes):
print(
f"[graphify] WARNING: new graph has {len(new_nodes)} nodes but existing "
f"graph.json has {len(existing_nodes)}. Refusing to overwrite — you may be "
f"missing chunk files from a previous session. "
f"Pass --force to override.",
file=sys.stderr,
)
return False
if len(new_nodes) >= len(existing_nodes):
return True
if rebuilt_sources is not None:
Expand Down Expand Up @@ -2004,6 +2031,7 @@ def _failed(f: str) -> bool:
had_explicit_deletions=bool(deleted_paths),
rebuilt_sources=rebuilt_sources,
failed_sources=failed_sources,
deleted_sources=deleted_paths,
):
return False
from graphify.export import backup_if_protected as _backup
Expand Down Expand Up @@ -2219,6 +2247,7 @@ def _failed(f: str) -> bool:
had_explicit_deletions=bool(deleted_paths),
rebuilt_sources=rebuilt_sources,
failed_sources=failed_sources,
deleted_sources=deleted_paths,
):
return False
from graphify.exporters.html import _HTML_STALE_MARKER
Expand Down
53 changes: 53 additions & 0 deletions tests/test_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -1033,6 +1033,59 @@ def test_to_json_refuses_shrink(tmp_path):
assert to_json(_mkG(2), {}, str(p), force=True) is True # force overrides


def test_to_json_refuses_lost_semantic_id_when_total_grows(tmp_path, capsys):
"""#2229: AST growth and a new stub must not replace a saved rationale id."""
import networkx as nx

saved_nodes = [
{"id": "app_ts", "label": "app.ts", "source_file": "src/app.ts", "_origin": "ast", "source_location": "L1", "file_type": "code"},
{"id": "app_ts_main", "label": "main", "source_file": "src/app.ts", "_origin": "ast", "source_location": "L2", "file_type": "code"},
{"id": "rationale_kept", "label": "kept", "source_file": "src/app.ts", "_origin": "semantic", "source_location": None, "file_type": "rationale"},
{"id": "rationale_lost", "label": "lost", "source_file": "src/app.ts", "_origin": "semantic", "source_location": None, "file_type": "rationale"},
]
new_nodes = [
{"id": "app_ts", "label": "app.ts", "source_file": "src/app.ts", "_origin": "ast", "source_location": "L1", "file_type": "code"},
{"id": "app_ts_main", "label": "main", "source_file": "src/app.ts", "_origin": "ast", "source_location": "L2", "file_type": "code"},
{"id": "app_ts_hook", "label": "hook", "source_file": "src/app.ts", "_origin": "ast", "source_location": "L10", "file_type": "code"},
{"id": "app_ts_route", "label": "route", "source_file": "src/app.ts", "_origin": "ast", "source_location": "L20", "file_type": "code"},
{"id": "app_ts_util", "label": "util", "source_file": "src/app.ts", "_origin": "ast", "source_location": "L30", "file_type": "code"},
{"id": "rationale_kept", "label": "kept", "source_file": "src/app.ts", "_origin": "semantic", "source_location": None, "file_type": "rationale"},
{"id": "pathlib_Path", "label": "Path", "source_file": "", "source_location": None, "file_type": "concept"},
]
p = tmp_path / "graph.json"
p.write_text(json.dumps({"nodes": saved_nodes, "links": []}), encoding="utf-8")
G = nx.Graph()
for node in new_nodes:
G.add_node(node["id"], **{k: v for k, v in node.items() if k != "id"})
assert G.number_of_nodes() > len(saved_nodes)
assert to_json(G, {}, str(p), force=False) is False
written = json.loads(p.read_text(encoding="utf-8"))
assert "rationale_lost" in {n["id"] for n in written["nodes"]}
assert "rationale_lost" in capsys.readouterr().err


def test_to_json_allows_ast_growth_that_keeps_semantic_ids(tmp_path):
"""AST nodes may be added when every saved rationale id is still present."""
import networkx as nx

saved_nodes = [
{"id": "app_ts", "label": "app.ts", "source_file": "src/app.ts", "_origin": "ast", "source_location": "L1", "file_type": "code"},
{"id": "rationale_kept", "label": "kept", "source_file": "src/app.ts", "_origin": "semantic", "source_location": None, "file_type": "rationale"},
{"id": "rationale_lost", "label": "lost", "source_file": "src/app.ts", "_origin": "semantic", "source_location": None, "file_type": "rationale"},
]
new_nodes = saved_nodes + [
{"id": "app_ts_hook", "label": "hook", "source_file": "src/app.ts", "_origin": "ast", "source_location": "L10", "file_type": "code"},
]
p = tmp_path / "graph.json"
p.write_text(json.dumps({"nodes": saved_nodes, "links": []}), encoding="utf-8")
G = nx.Graph()
for node in new_nodes:
G.add_node(node["id"], **{k: v for k, v in node.items() if k != "id"})
assert to_json(G, {}, str(p), force=False) is True
written = json.loads(p.read_text(encoding="utf-8"))
assert {"rationale_kept", "rationale_lost"} <= {n["id"] for n in written["nodes"]}


def test_to_json_fails_safe_on_corrupt_existing(tmp_path):
"""A non-empty but unparseable existing graph.json (corrupt or mid-write)
must NOT be silently overwritten — we can't verify the new graph isn't a
Expand Down
89 changes: 84 additions & 5 deletions tests/test_watch.py
Original file line number Diff line number Diff line change
Expand Up @@ -1530,13 +1530,13 @@ def test_check_shrink_allows_shrink_within_rebuilt_sources(capsys):
"""#1116: a symbol removed from a re-extracted file is a legitimate shrink —
every lost node belongs to a rebuilt source, so the write proceeds (no --force)."""
existing = {"nodes": [
{"id": "a", "source_file": "m.py"},
{"id": "b", "source_file": "m.py"},
{"id": "c", "source_file": "other.py"},
{"id": "a", "source_file": "m.py", "_origin": "ast"},
{"id": "b", "source_file": "m.py", "_origin": "ast"},
{"id": "c", "source_file": "other.py", "_origin": "ast"},
], "links": []}
new = {"nodes": [
{"id": "a", "source_file": "m.py"},
{"id": "c", "source_file": "other.py"},
{"id": "a", "source_file": "m.py", "_origin": "ast"},
{"id": "c", "source_file": "other.py", "_origin": "ast"},
], "links": []}
ok = _check_shrink(False, existing, new, rebuilt_sources={"m.py"})
assert ok is True
Expand Down Expand Up @@ -1621,6 +1621,85 @@ def test_check_shrink_allows_growth():
assert ok is True


def _masked_semantic_loss():
"""#2229: AST growth plus a new sourceless stub hides one lost rationale id.

The total rises (4 → 7). The semantic count stays at 2 because pathlib_Path
fills the slot. src/app.ts is rebuilt and is not deleted, so a rebuilt-set
excuse also accepts the loss.
"""
def ast(node_id, line):
return {
"id": node_id,
"source_file": "src/app.ts",
"_origin": "ast",
"source_location": f"L{line}",
"file_type": "code",
}

def rationale(node_id):
return {
"id": node_id,
"source_file": "src/app.ts",
"_origin": "semantic",
"source_location": None,
"file_type": "rationale",
}

saved = {"nodes": [
ast("app_ts", 1),
ast("app_ts_main", 2),
rationale("rationale_kept"),
rationale("rationale_lost"),
], "links": []}
new = {"nodes": [
ast("app_ts", 1),
ast("app_ts_main", 2),
ast("app_ts_hook", 10),
ast("app_ts_route", 20),
ast("app_ts_util", 30),
rationale("rationale_kept"),
{
"id": "pathlib_Path",
"source_file": "",
"source_location": None,
"file_type": "concept",
},
], "links": []}
return saved, new


def test_check_shrink_refuses_lost_semantic_id_when_total_grows(capsys):
"""#2229: a larger total must not write away a saved rationale id."""
saved, new = _masked_semantic_loss()
assert len(new["nodes"]) > len(saved["nodes"])
ok = _check_shrink(False, saved, new, rebuilt_sources={"src/app.ts"})
assert ok is False
assert "rationale_lost" in capsys.readouterr().err


def test_check_shrink_allows_deleted_code_file_when_semantic_ids_remain(capsys):
"""A removed code file may drop its nodes. A rationale id on a file that
is still present must stay, and that pair still writes."""
existing = {"nodes": [
{"id": "gone_fn", "source_file": "gone.py", "_origin": "ast", "source_location": "L1"},
{"id": "gone_rationale", "source_file": "gone.py", "_origin": "semantic", "source_location": None, "file_type": "rationale"},
{"id": "stay_rationale", "source_file": "stay.md", "_origin": "semantic", "source_location": None, "file_type": "rationale"},
], "links": []}
new = {"nodes": [
{"id": "stay_rationale", "source_file": "stay.md", "_origin": "semantic", "source_location": None, "file_type": "rationale"},
], "links": []}
ok = _check_shrink(
False,
existing,
new,
rebuilt_sources={"gone.py"},
deleted_sources={"gone.py"},
)
assert ok is True
assert "Refusing to overwrite" not in capsys.readouterr().err


def test_check_shrink_unlinks_tmp_on_refuse(tmp_path):
"""When refusing, the temp graph file gets cleaned up so it can't leak across runs."""
tmp = tmp_path / "graph.tmp.json"
Expand Down
Loading