Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
56 changes: 56 additions & 0 deletions graphify/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,62 @@ 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.

An auto-minted import stub (``external`` true, or ``type`` ``external``) is
rebuilt from the import edges of this run. Dropping one is not a lost
rationale or document node.
"""
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 _renamed_present(node_id: str) -> bool:
renamed = stem_remap.get(node_id, node_id)
renamed = doc_remap.get(renamed, renamed)
return renamed in new_ids

lost: list[dict] = []
for node in saved_nodes:
if not isinstance(node, dict) or _is_ast_tier(node):
continue
if node.get("external") is True or node.get("type") == "external":
continue
node_id = node.get("id")
if node_id in new_ids:
continue
if isinstance(node_id, str) and _renamed_present(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
78 changes: 78 additions & 0 deletions tests/test_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -1033,6 +1033,84 @@ 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_rewrites_the_same_integer_node_ids(tmp_path):
"""An unstamped integer id that is still in the graph is not a lost semantic node."""
import networkx as nx

G = nx.Graph()
G.add_nodes_from([(1, {}), (2, {}), (3, {})])
G.add_edges_from([(1, 2, {}), (1, 3, {}), (2, 3, {})])
p = tmp_path / "racecar"
assert to_json(G, {}, str(p), force=False) is True
assert to_json(G, {}, str(p), force=False) is True
assert {n["id"] for n in json.loads(p.read_text(encoding="utf-8"))["nodes"]} == {1, 2, 3}


def test_to_json_refuses_a_missing_integer_semantic_id(tmp_path):
"""A saved integer id that the new graph dropped is still refused when the total grows."""
import networkx as nx

p = tmp_path / "graph.json"
p.write_text(json.dumps({"nodes": [{"id": 4}], "links": []}), encoding="utf-8")
G = nx.Graph()
G.add_nodes_from([(1, {}), (2, {}), (3, {})])
assert to_json(G, {}, str(p), force=False) is False
assert json.loads(p.read_text(encoding="utf-8"))["nodes"] == [{"id": 4}]


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
121 changes: 116 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,117 @@ 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


@pytest.mark.parametrize("stub", [
{"id": "json", "external": True, "source_file": "", "file_type": "concept", "_origin": "semantic"},
{"id": "json", "type": "external", "source_file": "", "file_type": "concept", "_origin": "semantic"},
])
def test_check_shrink_allows_dropped_import_stub_when_total_grows(capsys, stub):
"""Removing an import drops its auto-minted stub. Added code can make the
total grow. That write must succeed without --force."""
def code(node_id):
return {
"id": node_id,
"source_file": "app.py",
"_origin": "ast",
"source_location": "L1",
"file_type": "code",
}
existing = {"nodes": [
code("app"),
code("app_load"),
stub,
], "links": []}
new = {"nodes": [
code("app"),
code("app_load"),
code("app_extra"),
code("app_more"),
], "links": []}
assert len(new["nodes"]) > len(existing["nodes"])
ok = _check_shrink(False, existing, new, rebuilt_sources={"app.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