From c865a3c9b0feea421b391b28340de153ec3b9269 Mon Sep 17 00:00:00 2001 From: safishamsi Date: Tue, 30 Jun 2026 12:57:33 +0100 Subject: [PATCH] fix(reflect): layout-ordered source resolution for overlay staleness (#1558) Refines the staleness file resolution (00e00a0) by folding in the two genuine merits of @TPAteeq's parallel fix (#1558), which independently and correctly diagnosed the same root-mismatch bug: - Layout-ordered candidates: try the layout-appropriate root FIRST (the graphify-out parent for the standard layout, graph.json's own dir for a flat layout) before the other. The prior order tried the grandparent first unconditionally, which in a flat layout (graph.json at the project root) could fingerprint a same-named file one directory up. Existence checking is kept on top, so a defeated name heuristic or a stale .graphify_root marker still falls through to the real file. - Adds @TPAteeq's .graphify_root-marker-driven regression test, plus a flat-layout test that pins the ordering (editing the real file flips stale; editing the same-named decoy one dir up does not). Co-Authored-By: tpateeq Co-Authored-By: Claude Opus 4.8 (1M context) --- graphify/reflect.py | 36 ++++++++++++++++++++----------- tests/test_reflect.py | 50 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 12 deletions(-) diff --git a/graphify/reflect.py b/graphify/reflect.py index 61a122d4..4e04e058 100644 --- a/graphify/reflect.py +++ b/graphify/reflect.py @@ -35,6 +35,7 @@ from pathlib import Path from typing import Any from graphify.ingest import OUTCOMES +from graphify.paths import GRAPHIFY_OUT_NAME _UNCATEGORIZED = "Uncategorized" @@ -669,12 +670,17 @@ def _resolve_source_path(src: str, graph_path: Path) -> Path | None: ``source_file`` is stored relative to the PROJECT root, but graph.json may live in ``/graphify-out/`` (so its own dir is not the root) or directly - at the root (``extract --out .``). Rather than guess the root from a directory - name (brittle: a ``GRAPHIFY_OUT`` override changes it), try the likely roots in - order and return the first where the file actually exists. The same candidate - search runs at write and read time, so the writer and reader resolve to the - same file. Order: the committed ``.graphify_root`` marker (#686), then the - graphify-out-parent, then graph.json's own dir, then the cwd. + at the root (``extract --out .``). Resolve the root in the most-likely order + and return the first candidate where the file actually exists, so a defeated + heuristic or a stale marker can never strand the file (every node would then + look "changed"). The same search runs at write and read time, so the writer + and reader resolve to the same file. + + Order: the committed ``.graphify_root`` marker (#686/#1423 — authoritative for + an absolute/elsewhere ``GRAPHIFY_OUT`` override); then the layout-appropriate + root *first* — graph.json's parent's parent for the ``graphify-out`` layout, + or graph.json's own dir for a flat layout — which avoids matching a same-named + file one directory up; then the other of the two; then the cwd. """ if not src: return None @@ -682,14 +688,20 @@ def _resolve_source_path(src: str, graph_path: Path) -> Path | None: if p.is_absolute(): return p if p.is_file() else None gp = Path(graph_path) + out_dir = gp.parent candidates: list[Path] = [] - marker = gp.parent / ".graphify_root" try: - if marker.is_file(): - candidates.append(Path(marker.read_text(encoding="utf-8").strip())) - except OSError: - pass - candidates += [gp.parent.parent, gp.parent, Path(".")] + recorded = (out_dir / ".graphify_root").read_text(encoding="utf-8").strip() + if recorded: + candidates.append(Path(recorded)) + except (OSError, ValueError): + pass # unreadable/non-UTF-8 marker -> fall through (best-effort) + # Layout-appropriate root first (precision), then the other (robustness). + if out_dir.name == GRAPHIFY_OUT_NAME: + candidates += [out_dir.parent, out_dir] + else: + candidates += [out_dir, out_dir.parent] + candidates.append(Path(".")) seen: set[str] = set() for base in candidates: key = str(base) diff --git a/tests/test_reflect.py b/tests/test_reflect.py index 61e0dd28..c24cacef 100644 --- a/tests/test_reflect.py +++ b/tests/test_reflect.py @@ -863,6 +863,56 @@ def test_relative_source_file_not_spuriously_stale_in_graphify_out_layout(tmp_pa assert load_learning_overlay(out / "graph.json")["auth_login"]["stale"] is True +def test_relative_source_file_resolved_via_graphify_root_marker(tmp_path): + """When a committed .graphify_root marker records the project root (e.g. a + GRAPHIFY_OUT override pointing the output dir elsewhere), the fingerprint + resolves source_file against that root, not graph.json's own dir.""" + proj = tmp_path / "project" + proj.mkdir() + (proj / "auth.py").write_text("def login(): pass\n", encoding="utf-8") + out = tmp_path / "elsewhere-out" # output dir NOT under the project + _overlay_graph(out, [ + {"id": "auth_login", "label": "login()", "source_file": "auth.py", "community": 0}, + ]) + (out / ".graphify_root").write_text(str(proj), encoding="utf-8") # the marker + mem = out / "memory" + _write_raw_doc(mem, "a.md", "2026-05-01", outcome="useful", nodes=["login()"]) + _write_raw_doc(mem, "b.md", "2026-05-10", outcome="useful", nodes=["login()"]) + reflect(mem, out / "reflections" / "LESSONS.md", + graph_path=out / "graph.json", now=_NOW) + assert load_learning_overlay(out / "graph.json")["auth_login"]["stale"] is False + + +def test_flat_layout_does_not_match_same_named_file_one_dir_up(tmp_path): + """In a flat layout (graph.json at the project root), the resolver must use the + graph's own dir, not its parent — otherwise a same-named file one level up + would be fingerprinted instead, producing a wrong staleness verdict.""" + proj = tmp_path / "proj" + proj.mkdir() + (proj / "util.py").write_text("REAL = 1\n", encoding="utf-8") + # A decoy same-named file in the parent dir (tmp_path / util.py). + (tmp_path / "util.py").write_text("DECOY = 2\n", encoding="utf-8") + # Flat layout: graph.json sits directly in proj/ (not a graphify-out subdir). + proj.joinpath("graph.json").write_text(json.dumps({ + "nodes": [{"id": "util", "label": "util.py", "source_file": "util.py", + "source_location": "L1", "community": 0}], + "links": [], + }), encoding="utf-8") + mem = proj / "memory" + _write_raw_doc(mem, "a.md", "2026-05-01", outcome="useful", nodes=["util.py"]) + _write_raw_doc(mem, "b.md", "2026-05-10", outcome="useful", nodes=["util.py"]) + reflect(mem, proj / "reflections" / "LESSONS.md", + graph_path=proj / "graph.json", now=_NOW) + # Not stale on a clean build... + assert load_learning_overlay(proj / "graph.json")["util"]["stale"] is False + # ...and editing the REAL file (proj/util.py) flips it, while editing the + # decoy (parent) does not — proving the resolver bound to the right file. + (tmp_path / "util.py").write_text("DECOY = 999\n", encoding="utf-8") + assert load_learning_overlay(proj / "graph.json")["util"]["stale"] is False + (proj / "util.py").write_text("REAL = 999\n", encoding="utf-8") + assert load_learning_overlay(proj / "graph.json")["util"]["stale"] is True + + def test_provenance_capped_to_five_most_recent(tmp_path): """A node cited by >5 useful results keeps exactly the 5 most-recent in provenance (recent-first)."""