From f9174943a2aa3b39b84e151a540aaf6d510b4e43 Mon Sep 17 00:00:00 2001 From: safishamsi Date: Sat, 4 Jul 2026 22:12:47 +0100 Subject: [PATCH] fix(detect): incremental correctness for Office sources + long paths, cache word counts (#1649, #1655, #1656) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1649: detect_incremental tracks the converted markdown sidecar, and convert_office_file early-returned whenever the sidecar existed — so a .docx/ .xlsx edited after its first conversion never updated its sidecar and was reported "unchanged" forever, freezing the graph. It now re-converts when the source is newer than the sidecar (bumping the sidecar so the hash check catches it); an unchanged source still skips the rewrite (#1226). #1655: _md5_file/save_manifest/count_words used plain open()/stat(), which the Windows file APIs reject for absolute paths over 260 chars unless prefixed with `\\?\`. Deeply-nested files never hashed, their manifest entry never stabilized, and detect_incremental re-flagged them as changed every run. A new _os_path adds the extended-length prefix on win32 for change-detection I/O (mirror of cache._normalize_path, which strips it for keys). No-op elsewhere. #1656: detect() re-parsed every PDF/docx/text file to size the corpus on each run. Word counts are now memoized in the existing content-hash stat index (keyed by size + mtime_ns), so an unchanged file is parsed once. file_hash's fastpath is guarded so a word-count-only entry (no hash) can't KeyError, and both writers augment a co-located entry in place instead of clobbering the other's field. Full suite: 2906 passed, 3 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 3 ++ graphify/cache.py | 56 +++++++++++++++++++++++++- graphify/detect.py | 67 ++++++++++++++++++++++++++------ tests/test_long_path_hashing.py | 50 ++++++++++++++++++++++++ tests/test_office_incremental.py | 67 ++++++++++++++++++++++++++++++++ tests/test_word_count_cache.py | 52 +++++++++++++++++++++++++ 6 files changed, 282 insertions(+), 13 deletions(-) create mode 100644 tests/test_long_path_hashing.py create mode 100644 tests/test_office_incremental.py create mode 100644 tests/test_word_count_cache.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 84db32d..33fa21a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ Full release notes with details on each version: [GitHub Releases](https://githu ## Unreleased +- Fix: a modified `.docx`/`.xlsx` now re-enters `--update` (#1649, thanks @Ns2384-star). `detect_incremental` tracks the converted markdown sidecar, and `convert_office_file` early-returned whenever the sidecar already existed — so an Office source edited after its first conversion never updated its sidecar and was reported "unchanged" forever, freezing the graph on a living docs corpus. The sidecar is now re-converted when the source is newer than it (which bumps the sidecar's mtime/content so the incremental hash check picks it up); an unchanged source still skips the rewrite so it never churns (#1226). +- Fix: files whose absolute path exceeds Windows' 260-char limit are now hashed (#1655, thanks @Ns2384-star). `_md5_file`/`save_manifest`/`count_words` used plain `open()`/`stat()`, which the Windows file APIs reject for long paths unless prefixed with the extended-length marker `\\?\` — so deeply-nested files (accented, deep folders) never hashed, their manifest entry never stabilized, and `detect_incremental` re-flagged them as changed on every run. Change-detection I/O now prefixes long absolute paths on win32 (mirroring the normalization `cache.py` already applied to cache keys). No-op on other platforms. +- Perf: word counts are cached against each file's stat signature (#1656, thanks @Ns2384-star). `detect()` counted words in every PDF/docx/text file to size the corpus, re-opening and re-parsing every binary on each run — minutes on a large docs corpus even when only a few files changed. Counts are now memoized in the existing content-hash stat index (keyed by size + mtime), so an unchanged file is parsed once and read from the index thereafter; incremental detection drops from O(corpus) parsing to O(changed). - Fix: a JS/TS call with no local definition and no import no longer binds to a same-named export in an unrelated package (#1659, thanks @leonaburime-ucla). When a callee had exactly one same-named definition repo-wide, the cross-file resolver emitted a `calls` edge at INFERRED/0.8 even with no import path between the two files. On a monorepo this fabricated dependencies: a 14-package repo showed `platform` and `sidecar` depending on `registry-protocol` purely because it exported generically-named symbols (`*Schema`, etc.) that unresolved calls collapsed onto. JS/TS modules have no implicit cross-module scope, so a cross-file call is real only if the caller imported it — direct JS/TS cross-file `calls` attribution is now gated on import evidence and left unresolved otherwise. Other languages keep the single-candidate resolution (C/C++ headers, Ruby autoload, same-package implicit scope legitimately call across files without an explicit import), and the `indirect_call` path (already INFERRED and callable-gated) is unchanged. As part of the fix, caller→file mapping for import-evidence now uses the raw call's `source_file` string, so a path-resolution/symlink mismatch can no longer spuriously fail evidence and mislabel a real cross-file call. ## 0.9.6 (2026-07-04) diff --git a/graphify/cache.py b/graphify/cache.py index f377889..bb3f9d5 100644 --- a/graphify/cache.py +++ b/graphify/cache.py @@ -180,6 +180,7 @@ def file_hash(path: Path, root: Path = Path(".")) -> str: st = p.stat() entry = _stat_index.get(abs_key) if (entry + and entry.get("hash") is not None # word-count-only entries carry no hash and entry.get("size") == st.st_size and entry.get("mtime_ns") == st.st_mtime_ns): return entry["hash"] @@ -199,12 +200,65 @@ def file_hash(path: Path, root: Path = Path(".")) -> str: digest = h.hexdigest() if st is not None: - _stat_index[abs_key] = {"size": st.st_size, "mtime_ns": st.st_mtime_ns, "hash": digest} + entry = _stat_index.get(abs_key) + if (entry is not None + and entry.get("size") == st.st_size + and entry.get("mtime_ns") == st.st_mtime_ns): + entry["hash"] = digest # preserve a co-located word_count + else: + _stat_index[abs_key] = {"size": st.st_size, "mtime_ns": st.st_mtime_ns, "hash": digest} _stat_index_dirty = True return digest +def cached_word_count(path: Path, root: Path, compute) -> int: + """Word count with the same (size, mtime_ns) stat-fastpath cache as + :func:`file_hash`, persisted in the shared stat index. + + ``detect()`` counts words in every PDF/docx/text file to size the corpus, + which re-opens and re-parses every binary on each run — minutes on a large + docs corpus even when only a handful of files changed (#1656). This caches + the count against the file's stat signature so an unchanged file is counted + once and read from the index thereafter. ``compute(path)`` produces the + count on a miss. A file that can't be stat'd (e.g. a Windows long path the + index normalization can't reach) simply recomputes and isn't cached — + correct, just not accelerated. + """ + global _stat_index_dirty + p = _normalize_path(Path(path)) + root = _normalize_path(Path(root)) + _ensure_stat_index(root) + abs_key = str(p.resolve()) + st: "os.stat_result | None" = None + try: + st = p.stat() + entry = _stat_index.get(abs_key) + if (entry + and entry.get("size") == st.st_size + and entry.get("mtime_ns") == st.st_mtime_ns + and "word_count" in entry): + return entry["word_count"] + except OSError: + pass + + wc = compute(Path(path)) + + if st is not None: + entry = _stat_index.get(abs_key) + if (entry + and entry.get("size") == st.st_size + and entry.get("mtime_ns") == st.st_mtime_ns): + entry["word_count"] = wc # augment the existing hash entry in place + else: + _stat_index[abs_key] = { + "size": st.st_size, "mtime_ns": st.st_mtime_ns, "word_count": wc, + } + _stat_index_dirty = True + + return wc + + def _relativize_source_files_in(payload: dict, root: Path) -> None: """Mutate ``payload`` to rewrite absolute ``source_file`` fields as forward-slash relative paths from ``root``. diff --git a/graphify/detect.py b/graphify/detect.py index 0e1c4ba..080dab8 100644 --- a/graphify/detect.py +++ b/graphify/detect.py @@ -630,11 +630,19 @@ def convert_office_file(path: Path, out_dir: Path) -> Path | None: normalized_path = unicodedata.normalize("NFC", str(path.resolve())) name_hash = hashlib.sha256(normalized_path.encode()).hexdigest()[:8] out_path = out_dir / f"{path.stem}_{name_hash}.md" - # Once the hash is stable the sidecar name is deterministic; skip re-writing - # an existing sidecar so an unchanged source never churns its mtime (which - # would still flag it as changed in detect_incremental). - if out_path.exists(): - return out_path + # Skip re-writing only when the sidecar is present AND at least as new as the + # source. detect_incremental tracks the SIDECAR (not the Office source), so a + # sidecar that is never rewritten after the source changes leaves the doc + # reported "unchanged" forever and freezes the graph (#1649). Re-converting + # when the source is newer bumps the sidecar's mtime/content, which the + # incremental hash check then correctly picks up. An unchanged source keeps + # its (newer-or-equal) sidecar untouched so it never churns (#1226). + try: + if out_path.exists() and os.stat(_os_path(out_path)).st_mtime >= os.stat(_os_path(path)).st_mtime: + return out_path + except OSError: + if out_path.exists(): + return out_path out_path.write_text( f"\n\n{text}", encoding="utf-8", @@ -651,7 +659,8 @@ def count_words(path: Path) -> int: return len(docx_to_markdown(path).split()) if ext == ".xlsx": return len(xlsx_to_markdown(path).split()) - return len(path.read_text(encoding="utf-8", errors="ignore").split()) + with open(_os_path(path), encoding="utf-8", errors="ignore") as f: + return len(f.read().split()) except Exception: return 0 @@ -1029,6 +1038,12 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace: } total_words = 0 + def _wc(path: Path) -> int: + # Cache word counts against each file's stat signature so unchanged + # PDFs/docx aren't re-parsed on every run just to size the corpus (#1656). + from graphify import cache as _cache + return _cache.cached_word_count(path, root, count_words) + skipped_sensitive: list[str] = [] ignore_patterns = _load_graphifyignore(root) ignore_cache: dict[Path, bool] = {} # shared across all _is_ignored calls in this scan @@ -1133,7 +1148,7 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace: if _is_ignored(md_path, root, ignore_patterns, _cache=ignore_cache): continue files[ftype].append(str(md_path)) - total_words += count_words(md_path) + total_words += _wc(md_path) else: skipped_sensitive.append(str(p) + " [Google Workspace export produced no readable text]") continue @@ -1144,14 +1159,14 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace: if _is_ignored(md_path, root, ignore_patterns, _cache=ignore_cache): continue files[ftype].append(str(md_path)) - total_words += count_words(md_path) + total_words += _wc(md_path) else: # Conversion failed (library not installed) - skip with note skipped_sensitive.append(str(p) + " [office conversion failed - pip install graphifyy[office]]") continue files[ftype].append(str(p)) if ftype != FileType.VIDEO: - total_words += count_words(p) + total_words += _wc(p) for ftype in files: files[ftype].sort() @@ -1185,12 +1200,40 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace: } +def _os_path(path: Path) -> str: + r"""Return an OS path string safe for open()/stat() on Windows long paths. + + On win32, paths longer than the legacy MAX_PATH (260 chars) are rejected by + the plain file APIs unless prefixed with the extended-length marker ``\\?\`` + (which also requires a fully-qualified path). Without it, _md5_file / + save_manifest / count_words silently fail to hash deeply-nested files, so + their manifest entry never stabilizes and detect_incremental re-flags them + as changed on every run (#1655). cache._normalize_path strips this prefix + for stable KEYS; this adds it for I/O. Non-win32 and already-prefixed paths + pass through unchanged. + """ + import sys + if sys.platform != "win32": + return str(path) + s = str(path) + if s.startswith("\\\\?\\"): + return s + try: + s = os.path.abspath(s) # \\?\ requires a fully-qualified path + except Exception: + return str(path) + if s.startswith("\\\\"): + # UNC share \\server\share -> \\?\UNC\server\share + return "\\\\?\\UNC\\" + s[2:] + return "\\\\?\\" + s + + def _md5_file(path: Path) -> str: """MD5 of file contents streamed in 64KB chunks — for change detection only.""" import hashlib as _hl h = _hl.md5(usedforsecurity=False) try: - with path.open("rb") as f: + with open(_os_path(path), "rb") as f: for chunk in iter(lambda: f.read(65536), b""): h.update(chunk) except OSError: @@ -1202,7 +1245,7 @@ def _stat_and_hash(path_str: str) -> tuple[str, float, str] | None: """Stat + MD5 a single file; returns None on OSError (e.g. deleted mid-run).""" try: p = Path(path_str) - return path_str, p.stat().st_mtime, _md5_file(p) + return path_str, os.stat(_os_path(p)).st_mtime, _md5_file(p) except OSError: return None @@ -1407,7 +1450,7 @@ def detect_incremental( for f in file_list: stored = manifest.get(f) try: - current_mtime = Path(f).stat().st_mtime + current_mtime = os.stat(_os_path(Path(f))).st_mtime except Exception: current_mtime = 0 diff --git a/tests/test_long_path_hashing.py b/tests/test_long_path_hashing.py new file mode 100644 index 0000000..8e4b3bd --- /dev/null +++ b/tests/test_long_path_hashing.py @@ -0,0 +1,50 @@ +r"""#1655 — files whose absolute path exceeds Windows MAX_PATH (260) must still +be hashed, or their manifest entry never stabilizes and detect_incremental +re-flags them as changed on every run. + +The plain file APIs reject long paths on win32 unless prefixed with the +extended-length marker `\\?\`. _os_path adds it (for I/O), the mirror of +cache._normalize_path which strips it (for stable keys). +""" +from __future__ import annotations + +from pathlib import Path + +from graphify import detect + + +def test_os_path_noop_on_posix(monkeypatch): + monkeypatch.setattr("sys.platform", "linux") + p = Path("/home/user/deep/file.py") + assert detect._os_path(p) == str(p) + + +def test_os_path_adds_prefix_on_win32(monkeypatch): + monkeypatch.setattr("sys.platform", "win32") + # os.path.abspath is posix here, so exercise the already-qualified branch: + # a value that abspath leaves intact still gets the prefix. + out = detect._os_path(Path("/already/abs/file.py")) + assert out.startswith("\\\\?\\") + + +def test_os_path_idempotent_on_win32(monkeypatch): + monkeypatch.setattr("sys.platform", "win32") + already = "\\\\?\\C:\\a\\file.py" + assert detect._os_path(Path(already)) == already + + +def test_hashing_still_works_and_stabilizes(tmp_path): + # End-to-end (posix): a hashed file must produce a stable, non-empty hash so + # its manifest entry doesn't churn. Guards against the _os_path indirection + # breaking normal hashing. + f = tmp_path / "deep" / "nested" / "module.py" + f.parent.mkdir(parents=True) + f.write_text("def x():\n return 1\n") + h1 = detect._md5_file(f) + h2 = detect._md5_file(f) + assert h1 and h1 == h2 + + got = detect._stat_and_hash(str(f)) + assert got is not None + assert got[0] == str(f) + assert got[2] == h1 diff --git a/tests/test_office_incremental.py b/tests/test_office_incremental.py new file mode 100644 index 0000000..19d8ec5 --- /dev/null +++ b/tests/test_office_incremental.py @@ -0,0 +1,67 @@ +"""#1649 — a modified .docx/.xlsx must re-enter --update. + +detect_incremental tracks the converted markdown SIDECAR, not the Office +source. convert_office_file used to early-return whenever the sidecar existed, +so a source edited after its first conversion never updated its sidecar and was +reported "unchanged" forever. It now re-converts when the source is newer than +the sidecar (and still skips an unchanged source so it never churns, #1226). +""" +from __future__ import annotations + +import os +from pathlib import Path + +import pytest + +from graphify import detect + +docx = pytest.importorskip("docx") + + +def _make_docx(path: Path, text: str) -> None: + d = docx.Document() + d.add_paragraph(text) + d.save(str(path)) + + +def _bump_mtime(path: Path, offset: float) -> None: + """Set path's mtime relative to now so ordering is deterministic.""" + st = path.stat() + os.utime(path, (st.st_atime, st.st_mtime + offset)) + + +def test_modified_docx_reconverts_sidecar(tmp_path: Path): + src = tmp_path / "doc.docx" + out = tmp_path / "converted" + _make_docx(src, "original alpha content") + + sidecar = detect.convert_office_file(src, out) + assert sidecar is not None + assert "original alpha content" in sidecar.read_text(encoding="utf-8") + + # Edit the source and make it newer than the sidecar. + _make_docx(src, "revised beta content") + _bump_mtime(sidecar, -10) # sidecar older than the freshly-written source + + sidecar2 = detect.convert_office_file(src, out) + assert sidecar2 == sidecar # same deterministic name + body = sidecar2.read_text(encoding="utf-8") + assert "revised beta content" in body + assert "original alpha content" not in body + + +def test_unchanged_docx_sidecar_not_rewritten(tmp_path: Path): + src = tmp_path / "doc.docx" + out = tmp_path / "converted" + _make_docx(src, "stable content") + + sidecar = detect.convert_office_file(src, out) + assert sidecar is not None + # Make the sidecar clearly newer than the (unchanged) source. + _bump_mtime(sidecar, 100) + before = sidecar.stat().st_mtime + + sidecar2 = detect.convert_office_file(src, out) + assert sidecar2 == sidecar + # Not rewritten: mtime unchanged, so detect_incremental won't see churn (#1226). + assert sidecar2.stat().st_mtime == before diff --git a/tests/test_word_count_cache.py b/tests/test_word_count_cache.py new file mode 100644 index 0000000..75bbb83 --- /dev/null +++ b/tests/test_word_count_cache.py @@ -0,0 +1,52 @@ +"""#1656 — word counts are cached against each file's stat signature so +detect() doesn't re-parse every unchanged PDF/docx on each run just to size +the corpus. +""" +from __future__ import annotations + +from pathlib import Path + +from graphify import cache + + +def test_word_count_cached_until_file_changes(tmp_path, monkeypatch): + # Isolate the stat index to this tmp root. + monkeypatch.setattr(cache, "_stat_index", {}) + monkeypatch.setattr(cache, "_stat_index_root", None) + + f = tmp_path / "doc.txt" + f.write_text("one two three four five") + + calls = {"n": 0} + def compute(p: Path) -> int: + calls["n"] += 1 + return len(p.read_text().split()) + + assert cache.cached_word_count(f, tmp_path, compute) == 5 + assert calls["n"] == 1 + # Second call, file unchanged → served from cache, compute NOT re-run. + assert cache.cached_word_count(f, tmp_path, compute) == 5 + assert calls["n"] == 1 + + # Change the file → recompute. + f.write_text("only three words now") # 4 words + assert cache.cached_word_count(f, tmp_path, compute) == 4 + assert calls["n"] == 2 + + +def test_word_count_augments_existing_hash_entry(tmp_path, monkeypatch): + # cached_word_count must not clobber a hash already stored for the file. + monkeypatch.setattr(cache, "_stat_index", {}) + monkeypatch.setattr(cache, "_stat_index_root", None) + + f = tmp_path / "m.py" + f.write_text("x = 1\n") # -> ["x", "=", "1"] == 3 tokens + h = cache.file_hash(f, tmp_path) + assert h + wc = cache.cached_word_count(f, tmp_path, lambda p: len(p.read_text().split())) + assert wc == 3 + # The hash entry survives alongside the word_count. + assert cache.file_hash(f, tmp_path) == h + key = str(cache._normalize_path(f).resolve()) + entry = cache._stat_index[key] + assert entry.get("hash") == h and entry.get("word_count") == 3