fix(detect): incremental correctness for Office sources + long paths, cache word counts (#1649, #1655, #1656)
#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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
62b8eb1416
commit
f9174943a2
@@ -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)
|
||||
|
||||
+55
-1
@@ -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``.
|
||||
|
||||
+55
-12
@@ -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"<!-- converted from {path.name} -->\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
|
||||
|
||||
|
||||
@@ -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
|
||||
@@ -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
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user