fix(detect): stop silent data loss from env-dir pruning and absolute-path sidecars (#2058, #2059)

#2058: `_is_noise_dir` treated any directory named `env`/`.env`/`*_env` as a
Python virtualenv and pruned it during the walk — before `.graphifyignore`
negation, with zero trace in any returned bucket. Real source dirs with those
names (common in UVM/ASIC verification trees) were silently lost. The venv
heuristic for those names is now gated on actual markers (`pyvenv.cfg`,
`bin`/`Scripts/activate`, `lib/python*`, `conda-meta/`); `venv`/`.venv`/`*_venv`
stay name-only. Pruned-as-noise dirs are recorded in a new `pruned_noise_dirs`
bucket for traceability, and extract.py's walk call sites pass the parent so
genuine venvs are still marker-checked and pruned.

#2059: Office and Google-Workspace sidecars were named with a hash of the
resolved ABSOLUTE source path, so the same tracked file in two clones/worktrees
produced two differently-named byte-identical sidecars — unbounded duplicates
when graphify-out/ is committed, each ingested as a distinct source doc. The
hash is now over the scan-root-relative (NFC-normalized) path, stable across
checkouts while still disambiguating same-stem files; out-of-root sources fall
back to the old absolute form. Also fixes the same bug in google_workspace's
`_sidecar_path` (which additionally never had the #1226 NFC fix).

graphify/detect.py

@@ -652,7 +652,7 @@ def xlsx_extract_structure(path: Path) -> dict:
     return {"nodes": nodes, "edges": edges}
 
 
-def convert_office_file(path: Path, out_dir: Path) -> Path | None:
+def convert_office_file(path: Path, out_dir: Path, root: "Path | None" = None) -> Path | None:
     """Convert a .docx or .xlsx to a markdown sidecar in out_dir.
 
     Returns the path of the converted .md file, or None if conversion failed
@@ -671,14 +671,30 @@ def convert_office_file(path: Path, out_dir: Path) -> Path | None:
 
     out_dir.mkdir(parents=True, exist_ok=True)
     # Use a stable name derived from the original path to avoid collisions.
-    # Normalize the resolved path to NFC before hashing: on macOS (HFS+/APFS)
-    # os.walk/rglob return filenames in NFD, while Python string literals and
-    # directly-constructed Path objects are NFC, so the same source file would
-    # otherwise hash to different sidecar names across runs — causing --update
-    # to treat every Office file as new and re-extract it (#1226).
+    # Hash the path RELATIVE to the scan root, not the absolute path: the
+    # absolute form salts the name with the checkout location, so the same
+    # tracked .xlsx in two clones/worktrees emits two differently-named,
+    # byte-identical sidecars — unbounded duplicates when graphify-out/ is
+    # committed, each ingested as a distinct source doc (#2059). The relative
+    # path still disambiguates same-stem files in different directories.
+    # Normalize to NFC before hashing: on macOS (HFS+/APFS) os.walk/rglob return
+    # filenames in NFD, while Python string literals and directly-constructed
+    # Path objects are NFC, so the same source file would otherwise hash to
+    # different sidecar names across runs — making --update treat every Office
+    # file as new and re-extract it (#1226).
     import hashlib
     import unicodedata
-    normalized_path = unicodedata.normalize("NFC", str(path.resolve()))
+    if root is None:
+        # Default layout: out_dir is <root>/<graphify-out>/converted.
+        root = out_dir.parent.parent
+    try:
+        key = path.resolve().relative_to(Path(root).resolve()).as_posix()
+    except (ValueError, OSError):
+        # Not under the scan root (custom GRAPHIFY_OUT layouts, --include
+        # sources, direct API callers): keep the previous absolute form rather
+        # than guessing, so behavior is unchanged for those cases.
+        key = str(path.resolve())
+    normalized_path = unicodedata.normalize("NFC", key)
     name_hash = hashlib.sha256(normalized_path.encode()).hexdigest()[:8]
     out_path = out_dir / f"{path.stem}_{name_hash}.md"
     # Skip re-writing only when the sidecar is present AND at least as new as the
@@ -718,7 +734,7 @@ def count_words(path: Path) -> int:
 
 # Directory names to always skip - venvs, caches, build artifacts, deps
 _SKIP_DIRS = {
-    "venv", ".venv", "env", ".env",
+    "venv", ".venv",  # "env"/".env"/"*_env" are gated on venv markers below (#2058)
     "node_modules", "__pycache__", ".git",
     "dist", "build", "target", "out",
     "site-packages", "lib64",
@@ -753,10 +769,39 @@ _SKIP_FILES = {
 _JS_SNAPSHOT_TEST_ROOTS = frozenset({"__tests__", "__test__"})
 
 
+def _has_venv_markers(d: "Path") -> bool:
+    """True only when *d* has actual virtualenv/conda structure on disk.
+
+    ``env``/``.env``/``*_env`` is a real source-directory convention (UVM/ASIC
+    verification trees, and others), so pruning it by name alone silently drops
+    legitimate source with no trace (#2058). Prune it only on real evidence: a
+    ``pyvenv.cfg``, an ``activate`` script, a ``lib/python*`` tree, or conda's
+    ``conda-meta/`` (``conda create -p ./env`` writes no pyvenv.cfg).
+    """
+    try:
+        if (d / "pyvenv.cfg").is_file():
+            return True
+        if (d / "bin" / "activate").is_file() or (d / "Scripts" / "activate").is_file():
+            return True
+        if next(d.glob("lib/python*"), None) is not None:
+            return True
+        if (d / "conda-meta").is_dir():
+            return True
+    except OSError:
+        pass
+    return False
+
+
 def _is_noise_dir(part: str, parent: "Path | None" = None) -> bool:
     """Return True if this directory name looks like a venv, cache, or dep dir."""
     if part in _SKIP_DIRS:
         return True
+    if part in ("env", ".env") or part.endswith("_env"):
+        # Ambiguous: a real venv OR a real source dir. Prune only on actual venv
+        # evidence, mirroring the "snapshots" gating (#1666/#2058).
+        if parent is None:
+            return False  # cannot verify; keep a possibly-real code dir
+        return _has_venv_markers(parent / part)
     if part == "snapshots":
         # Prune only when it looks like an actual JS/Vitest snapshot dir.
         if parent is None:
@@ -770,8 +815,9 @@ def _is_noise_dir(part: str, parent: "Path | None" = None) -> bool:
         except OSError:
             pass
         return False
-    # Catch *_venv, *_repo/site-packages patterns
-    if part.endswith("_venv") or part.endswith("_env"):
+    # Catch *_venv (unambiguous — "venv" is always a virtualenv signal). "*_env"
+    # is gated on markers above (#2058), not pruned by name.
+    if part.endswith("_venv"):
         return True
     if part.endswith(".egg-info"):
         return True
@@ -1218,6 +1264,7 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace:
     # of silently vanishing from the graph (#1922). Directory-level entries keep
     # this bounded — a pruned `data/` is one entry, not one per contained file.
     ignored: list[str] = []
+    pruned_noise: list[str] = []
     ignore_patterns = _load_graphifyignore(root, gitignore=gitignore)
     ignore_cache: dict[Path, bool] = {}  # shared across all _is_ignored calls in this scan
     # CLI --exclude patterns are anchored at the scan root and appended last
@@ -1292,6 +1339,10 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace:
                 kept_dirs: list[str] = []
                 for d in dirnames:
                     if _is_noise_dir(d, dp):
+                        # Record pruned-as-noise dirs so a wrongly-pruned real
+                        # source dir is at least traceable in the output rather
+                        # than vanishing silently (#2058).
+                        pruned_noise.append(str(dp / d) + os.sep)
                         continue
                     if _is_ignored(dp / d, root, ignore_patterns, _cache=ignore_cache):
                         ignored.append(str(dp / d) + os.sep)
@@ -1353,7 +1404,7 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace:
                     )
                     continue
                 try:
-                    md_path = convert_google_workspace_file(p, converted_dir, xlsx_to_markdown=xlsx_to_markdown)
+                    md_path = convert_google_workspace_file(p, converted_dir, xlsx_to_markdown=xlsx_to_markdown, root=root)
                 except Exception as exc:
                     skipped_sensitive.append(str(p) + f" [Google Workspace export failed: {exc}]")
                     continue
@@ -1367,7 +1418,7 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace:
                 continue
             # Office files: convert to markdown sidecar so subagents can read them
             if p.suffix.lower() in OFFICE_EXTENSIONS:
-                md_path = convert_office_file(p, converted_dir)
+                md_path = convert_office_file(p, converted_dir, root=root)
                 if md_path:
                     if _is_ignored(md_path, root, ignore_patterns, _cache=ignore_cache):
                         continue
@@ -1411,6 +1462,7 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace:
         "unclassified": sorted(unclassified),
         "walk_errors": walk_errors,
         "ignored": sorted(ignored),
+        "pruned_noise_dirs": sorted(pruned_noise),
         "graphifyignore_patterns": len(ignore_patterns),
         "scan_root": str(root.resolve()),
     }

graphify/extract.py

@@ -5199,7 +5199,7 @@ def collect_files(target: Path, *, follow_symlinks: bool = False, root: Path | N
             dp = Path(dirpath)
             dirnames[:] = [
                 d for d in dirnames
-                if not _is_noise_dir(d)
+                if not _is_noise_dir(d, dp)  # pass parent so "env"/"*_env" is marker-gated (#2058)
                 and (has_negation or not _ignored(dp / d))
             ]
             for fname in filenames:
@@ -5220,7 +5220,7 @@ def collect_files(target: Path, *, follow_symlinks: bool = False, root: Path | N
         dp = Path(dirpath)
         dirnames[:] = [
             d for d in dirnames
-            if not _is_noise_dir(d)
+            if not _is_noise_dir(d, dp)  # pass parent so "env"/"*_env" is marker-gated (#2058)
             and (not (dp / d).is_symlink() or _resolves_under_root(dp / d, containment_root))
         ]
         for fname in filenames:

graphify/google_workspace.py

@@ -121,8 +121,21 @@ def _run_gws_export(file_id: str, mime_type: str, output: Path, resource_key: st
         raise RuntimeError(f"gws export failed for {file_id}: {stderr}")
 
 
-def _sidecar_path(path: Path, out_dir: Path) -> Path:
-    name_hash = hashlib.sha256(str(path.resolve()).encode()).hexdigest()[:8]
+def _sidecar_path(path: Path, out_dir: Path, root: "Path | None" = None) -> Path:
+    # Hash the scan-root-relative, NFC-normalized path — not the absolute path.
+    # The absolute form salts the sidecar name with the checkout location, so the
+    # same shortcut in two clones/worktrees emits differently-named byte-identical
+    # sidecars, each ingested as a distinct source doc when graphify-out/ is
+    # committed (#2059; mirrors convert_office_file). NFC guards macOS NFD drift
+    # (#1226). The relative path still disambiguates same-stem files.
+    import unicodedata
+    if root is None:
+        root = out_dir.parent.parent
+    try:
+        key = path.resolve().relative_to(Path(root).resolve()).as_posix()
+    except (ValueError, OSError):
+        key = str(path.resolve())
+    name_hash = hashlib.sha256(unicodedata.normalize("NFC", key).encode()).hexdigest()[:8]
     return out_dir / f"{path.stem}_{name_hash}.md"
 
 
@@ -152,6 +165,7 @@ def convert_google_workspace_file(
     out_dir: Path,
     *,
     xlsx_to_markdown: Callable[[Path], str] | None = None,
+    root: "Path | None" = None,
 ) -> Path | None:
     """Export a Google Workspace shortcut to a Markdown sidecar.
 
@@ -164,7 +178,7 @@ def convert_google_workspace_file(
 
     shortcut = read_google_shortcut(path)
     out_dir.mkdir(parents=True, exist_ok=True)
-    out_path = _sidecar_path(path, out_dir)
+    out_path = _sidecar_path(path, out_dir, root=root)
 
     if ext == ".gdoc":
         with tempfile.NamedTemporaryFile("w+b", suffix=".md", delete=False, dir=out_dir) as tmp:

tests/test_detect.py

@@ -544,7 +544,7 @@ def test_detect_converts_google_workspace_shortcuts_when_enabled(tmp_path, monke
     shortcut = tmp_path / "notes.gdoc"
     shortcut.write_text('{"doc_id":"doc-1"}', encoding="utf-8")
 
-    def fake_convert(path, out_dir, *, xlsx_to_markdown=None):
+    def fake_convert(path, out_dir, *, xlsx_to_markdown=None, root=None):
         out_dir.mkdir(parents=True, exist_ok=True)
         out = out_dir / "notes_converted.md"
         out.write_text("# Notes\n\nA converted Google Doc.", encoding="utf-8")
@@ -1909,6 +1909,112 @@ def test_convert_office_file_does_not_rewrite_existing_sidecar(tmp_path, monkeyp
     assert second.stat().st_mtime_ns == mtime_before
 
 
+def test_convert_office_file_sidecar_name_stable_across_checkouts(tmp_path, monkeypatch):
+    """#2059: the sidecar name must depend on the scan-root-RELATIVE path, not the
+    absolute checkout location, so the same tracked file in two clones/worktrees
+    produces the same sidecar name (no unbounded duplicates when graphify-out/ is
+    committed). Also verifies the no-root fallback matches the explicit form."""
+    monkeypatch.setattr(detect_mod, "xlsx_to_markdown", lambda p: "sheet body")
+
+    def _sidecar(root):
+        src = root / "docs" / "report.xlsx"
+        out_dir = root / "graphify-out" / "converted"
+        return detect_mod.convert_office_file(src, out_dir, root=root)
+
+    checkout_a = tmp_path / "checkout-a"
+    checkout_b = tmp_path / "somewhere-else" / "checkout-b"
+    (checkout_a / "docs").mkdir(parents=True)
+    (checkout_b / "docs").mkdir(parents=True)
+    out_a = _sidecar(checkout_a)
+    out_b = _sidecar(checkout_b)
+    assert out_a is not None and out_b is not None
+    assert out_a.name == out_b.name, "sidecar name must be stable across checkouts (#2059)"
+    assert out_a.parent != out_b.parent  # sanity: genuinely different locations
+
+    # No explicit root -> the out_dir.parent.parent fallback yields the same name.
+    fallback = detect_mod.convert_office_file(
+        checkout_a / "docs" / "report.xlsx", checkout_a / "graphify-out" / "converted"
+    )
+    assert fallback is not None and fallback.name == out_a.name
+
+
+def test_convert_office_file_hash_disambiguates_same_stem(tmp_path, monkeypatch):
+    """Two same-stem Office files in different subdirs must still get distinct
+    sidecar names — the relative-path hash preserves the disambiguation purpose."""
+    monkeypatch.setattr(detect_mod, "xlsx_to_markdown", lambda p: "body")
+    root = tmp_path / "repo"
+    (root / "a").mkdir(parents=True)
+    (root / "b").mkdir(parents=True)
+    out_dir = root / "graphify-out" / "converted"
+    out_a = detect_mod.convert_office_file(root / "a" / "report.xlsx", out_dir, root=root)
+    out_b = detect_mod.convert_office_file(root / "b" / "report.xlsx", out_dir, root=root)
+    assert out_a is not None and out_b is not None
+    assert out_a.name != out_b.name, "same-stem files in different dirs must differ (#2059)"
+
+
+def test_convert_office_file_outside_root_falls_back(tmp_path, monkeypatch):
+    """A source outside the scan root (--include, custom layouts) falls back to the
+    absolute-path hash without raising, and stays deterministic."""
+    monkeypatch.setattr(detect_mod, "docx_to_markdown", lambda p: "body")
+    root = tmp_path / "repo"
+    (root / "graphify-out" / "converted").mkdir(parents=True)
+    outside = tmp_path / "elsewhere" / "doc.docx"
+    out_dir = root / "graphify-out" / "converted"
+    out1 = detect_mod.convert_office_file(outside, out_dir, root=root)
+    out2 = detect_mod.convert_office_file(outside, out_dir, root=root)
+    assert out1 is not None and out1.name == out2.name
+
+
+def test_detect_keeps_env_source_dirs(tmp_path):
+    """#2058: a real source directory named env/ or *_env/ with no virtualenv
+    markers must be indexed, not silently pruned as a false-positive venv."""
+    src_env = tmp_path / "src_env"
+    (src_env / "env").mkdir(parents=True)
+    (src_env / "env" / "ctrl_mem_env.py").write_text("def build_env():\n    return 1\n")
+    (src_env / "other_dir").mkdir()
+    (src_env / "other_dir" / "also_real.py").write_text("def x():\n    return 2\n")
+
+    all_files = [f for files in detect(tmp_path)["files"].values() for f in files]
+    assert any("ctrl_mem_env.py" in f for f in all_files), "env/ source dir wrongly pruned (#2058)"
+    assert any("also_real.py" in f for f in all_files), "*_env/ subtree wrongly pruned (#2058)"
+
+    # Nested env/ under a scan root that IS the *_env dir (issue's exact-match case).
+    nested = [f for files in detect(src_env)["files"].values() for f in files]
+    assert any("ctrl_mem_env.py" in f for f in nested), "nested env/ pruned when scanned directly (#2058)"
+
+
+def test_detect_still_prunes_real_env_venv(tmp_path):
+    """#2058: an env/ dir that IS a real virtualenv (has markers) is still pruned,
+    and the pruned dir is recorded in the traceable pruned_noise_dirs bucket."""
+    venv = tmp_path / "env"
+    (venv / "lib").mkdir(parents=True)
+    (venv / "pyvenv.cfg").write_text("home = /usr/bin\n")
+    (venv / "lib" / "sixish.py").write_text("x = 1\n")
+    (tmp_path / "main.py").write_text("def main():\n    return 1\n")
+
+    result = detect(tmp_path)
+    all_files = [f for files in result["files"].values() for f in files]
+    assert not any("sixish.py" in f for f in all_files), "real venv env/ must still be pruned"
+    assert any("main.py" in f for f in all_files)
+    assert any(f"{os.sep}env{os.sep}" in d for d in result["pruned_noise_dirs"]), (
+        "pruned venv must be traceable in pruned_noise_dirs (#2058)"
+    )
+
+
+def test_detect_prunes_venv_names_without_markers(tmp_path):
+    """#2058 must not loosen the unambiguous names: venv/.venv/*_venv are still
+    pruned by name alone (no markers needed)."""
+    for name in ("venv", ".venv", "my_venv"):
+        d = tmp_path / name
+        d.mkdir()
+        (d / "mod.py").write_text("y = 1\n")
+    (tmp_path / "app.py").write_text("def a():\n    return 1\n")
+    all_files = [f for files in detect(tmp_path)["files"].values() for f in files]
+    assert any("app.py" in f for f in all_files)
+    for name in ("venv", ".venv", "my_venv"):
+        assert not any(f"{os.sep}{name}{os.sep}" in f for f in all_files), f"{name} must stay pruned"
+
+
 def test_detect_records_unclassified_extensionless_files(tmp_path):
     # #1692: extensionless, non-shebang project files (Dockerfile, Makefile, ...)
     # were considered but left no trace. detect() now lists them under