From b7c709e13d22f567c14d1aaa51b3c0d00b94fb54 Mon Sep 17 00:00:00 2001 From: abhay-codes07 Date: Fri, 14 Aug 2026 14:24:01 +0100 Subject: [PATCH] fix(export): budget export filenames against the destination path, not just NAME_MAX (#2655) Adds paths.stem_filename_budget(output_dir, *, reserve, limit=200) and threads it through the Obsidian and wiki exporters so a filename stem is budgeted against the whole Windows MAX_PATH window (drive + dirs + name + NUL), not just the per-component 200-char NAME_MAX cap. On POSIX the helper returns the limit unchanged, so existing vaults stay byte-identical; on Windows a long output directory no longer pushes the total path over MAX_PATH and aborts the export mid-write. Co-Authored-By: Claude Opus 4.8 (1M context) --- graphify/export.py | 52 ++++++-- graphify/paths.py | 49 ++++++++ graphify/wiki.py | 24 +++- tests/test_export_path_length.py | 207 +++++++++++++++++++++++++++++++ 4 files changed, 318 insertions(+), 14 deletions(-) create mode 100644 tests/test_export_path_length.py diff --git a/graphify/export.py b/graphify/export.py index 03159ada..e80924ff 100644 --- a/graphify/export.py +++ b/graphify/export.py @@ -16,6 +16,7 @@ from networkx.readwrite import json_graph from graphify.security import sanitize_label from graphify.analyze import _node_community_map from graphify.build import edge_data +from graphify.paths import stem_filename_budget from graphify.exporters.graphdb import push_to_falkordb, push_to_neo4j # noqa: E402,F401 @@ -479,7 +480,7 @@ def _cap_filename(s: str, limit: int = 200) -> str: return f"{truncated}_{digest}" -def _obsidian_safe_stem(label: str) -> str: +def _obsidian_safe_stem(label: str, limit: int = 200) -> str: """Filename stem for an Obsidian note / canvas card from a node label. Strips filesystem-unsafe characters, a trailing ``.md``-family extension @@ -507,7 +508,15 @@ def _obsidian_safe_stem(label: str) -> str: # emit a "@.md"-style filename. (#1409) if not re.search(r"\w", cleaned, flags=re.UNICODE): return "unnamed" - return _cap_filename(cleaned) + return _cap_filename(cleaned, limit) + + +# Room _dedup_node_filenames / the community loop need for a collision suffix +# ("_1" … "_999") appended AFTER the stem was capped. +_DEDUP_SUFFIX_RESERVE = 4 + +# Prefix the community overview notes carry ("_COMMUNITY_Backend.md"). +_COMMUNITY_PREFIX = "_COMMUNITY_" def _dedup_node_filenames(G: nx.Graph, safe_name) -> dict[str, str]: @@ -576,9 +585,16 @@ def to_obsidian( node_community = _node_community_map(communities) + # Cap stems against THIS vault's path, not just NAME_MAX: on Windows the + # 200-byte default plus an ordinary vault directory overruns MAX_PATH and + # every note write raises FileNotFoundError (#2655). No-op on POSIX. + _stem_limit = stem_filename_budget(out, reserve=_DEDUP_SUFFIX_RESERVE) + # Map node_id → safe filename so wikilinks stay consistent. # Deduplicate: if two nodes produce the same filename, append a numeric suffix. - node_filename = _dedup_node_filenames(G, _obsidian_safe_stem) + node_filename = _dedup_node_filenames( + G, lambda label: _obsidian_safe_stem(label, _stem_limit) + ) # Helper: compute dominant confidence for a node across all its edges def _dominant_confidence(node_id: str) -> str: @@ -692,8 +708,13 @@ def to_obsidian( # this path had no dedup at all, so even same-case duplicate labels collided. community_filename: dict = {} used_community: set[str] = set() + # The community stem carries the "_COMMUNITY_" prefix on top of the dedup + # suffix, so it gets that much less of the MAX_PATH window (#2655). + _community_stem_limit = stem_filename_budget( + out, reserve=_DEDUP_SUFFIX_RESERVE + len(_COMMUNITY_PREFIX) + ) for cid in communities: - base = f"_COMMUNITY_{_obsidian_safe_stem(_community_name(cid))}" + base = f"{_COMMUNITY_PREFIX}{_obsidian_safe_stem(_community_name(cid), _community_stem_limit)}" candidate = base n = 1 while candidate.lower() in used_community: @@ -768,7 +789,10 @@ def to_obsidian( if cross: lines.append("## Connections to other communities") for other_cid, edge_count in sorted(cross.items(), key=lambda x: -x[1]): - other_fname = community_filename.get(other_cid) or f"_COMMUNITY_{_obsidian_safe_stem(_community_name(other_cid))}" + other_fname = community_filename.get(other_cid) or ( + f"{_COMMUNITY_PREFIX}" + f"{_obsidian_safe_stem(_community_name(other_cid), _community_stem_limit)}" + ) lines.append(f"- {edge_count} edge{'s' if edge_count != 1 else ''} to [[{other_fname}]]") lines.append("") @@ -866,9 +890,18 @@ def to_canvas( # Obsidian canvas color codes (cycle through for communities) CANVAS_COLORS = ["1", "2", "3", "4", "5", "6"] # red, orange, yellow, green, cyan, purple - # Build node_filenames if not provided (same dedup logic as to_obsidian) + # Build node_filenames if not provided (same dedup logic as to_obsidian). + # The CLI calls to_canvas without passing the map, so it must derive the + # SAME stem budget to keep card links pointing at the notes to_obsidian + # wrote — hence budgeting against the canvas's own directory, which is the + # vault directory (#2655). + _stem_limit = stem_filename_budget( + Path(output_path).parent, reserve=_DEDUP_SUFFIX_RESERVE + ) if node_filenames is None: - node_filenames = _dedup_node_filenames(G, _obsidian_safe_stem) + node_filenames = _dedup_node_filenames( + G, lambda label: _obsidian_safe_stem(label, _stem_limit) + ) # Fallback: with no community data (e.g. --no-cluster builds or a missing # analysis sidecar) the grid below produces nothing and the canvas is written @@ -982,7 +1015,10 @@ def to_canvas( row = m_idx // inner_cols nx_x = gx + 20 + col * (180 + 20) nx_y = gy + 80 + row * (60 + 20) - fname = node_filenames.get(node_id, _obsidian_safe_stem(G.nodes[node_id].get("label", node_id))) + fname = node_filenames.get( + node_id, + _obsidian_safe_stem(G.nodes[node_id].get("label", node_id), _stem_limit), + ) canvas_nodes.append({ "id": f"n_{node_id}", "type": "file", diff --git a/graphify/paths.py b/graphify/paths.py index 8a3cdc21..ba15b32c 100644 --- a/graphify/paths.py +++ b/graphify/paths.py @@ -346,6 +346,55 @@ def is_absolute_any_platform(p: "str | Path | None") -> bool: return PurePosixPath(s).is_absolute() or PureWindowsPath(s).is_absolute() +# Legacy Windows path ceiling. Unless long-path support is enabled *and* every +# consumer opts in, the ENTIRE path — drive, directories, filename, and the +# terminating NUL — must fit in MAX_PATH (260) characters, so the usable budget +# is 259. POSIX has no equivalent whole-path ceiling in practice; its limit is +# per-component (NAME_MAX, conventionally 255 bytes). +_WINDOWS_MAX_PATH = 260 + +# Floor for the stem budget below. A directory deep enough to push the budget +# under this cannot host readable filenames anyway; keep enough room for +# _cap_filename's "_" + 8-char digest so a truncated stem stays collision-safe +# and deterministic rather than degenerating into a bare prefix. +_MIN_STEM_BUDGET = 16 + + +def stem_filename_budget(output_dir: "str | Path", *, reserve: int = 0, limit: int = 200) -> int: + """Largest filename stem an exporter may write directly into ``output_dir``. + + Exporters cap note/article filenames so they stay under the filesystem's + per-component limit (conventionally NAME_MAX=255 bytes, hence the 200 + default). That is the right question on POSIX and the wrong one on Windows, + where the constraint is on the WHOLE path, not the component: a 200-char + stem under a perfectly ordinary vault directory such as + ``C:\\Users\\me\\projects\\svc\\graphify-out\\obsidian`` exceeds MAX_PATH and + the write dies with ``FileNotFoundError``, aborting the export mid-vault. + + Returns ``limit`` unchanged on POSIX, so existing output is byte-for-byte + stable there. On Windows it returns the smaller of ``limit`` and whatever + still fits inside MAX_PATH once ``output_dir``, the separator, ``reserve`` + (room for caller-added prefixes/collision suffixes) and the ``.md`` + extension are accounted for. + + The budget is a CHARACTER count, but callers that cap UTF-8 BYTES may pass + it straight through: a string's UTF-8 length is never below its character + length, so a byte-capped stem always satisfies the character ceiling too. + """ + if os.name != "nt": + return limit + try: + base = os.path.abspath(str(output_dir)) + except (OSError, ValueError): + return limit + # An extended-length path ("\\?\C:\...", "\\?\UNC\...") opts out of MAX_PATH + # entirely, so nothing needs shrinking. + if base.startswith("\\\\?\\"): + return limit + budget = (_WINDOWS_MAX_PATH - 1) - len(base) - len(os.sep) - reserve - len(".md") + return max(_MIN_STEM_BUDGET, min(limit, budget)) + + def nfc(s: str) -> str: """NFC-normalize a path string. diff --git a/graphify/wiki.py b/graphify/wiki.py index d9032109..61d9b268 100644 --- a/graphify/wiki.py +++ b/graphify/wiki.py @@ -7,7 +7,11 @@ import re import networkx as nx from graphify.build import edge_data +from graphify.paths import stem_filename_budget +# Room _unique_slug needs for the collision suffix ("_2" … "_999") it appends +# after _safe_filename has already capped the slug. +_SLUG_SUFFIX_RESERVE = 4 # Characters a slug may not contain, because the article's LINK and its ON-DISK # NAME have to be the same string (#2597). Anything left here must be legal, @@ -24,15 +28,17 @@ from graphify.build import edge_data _UNSAFE_SLUG_CHARS = re.compile(r'[<>:"/\\|?*#%\x00-\x1f\x7f]') -def _safe_filename(name: str) -> str: +def _safe_filename(name: str, limit: int = 200) -> str: """Make a label safe for use as a filename across platforms AND as a markdown link destination. Substitutes characters that Windows reserves in filenames (< > : " / \\ | ? *) plus the ones that would make the emitted link stop matching the file on disk, and strips trailing dots/spaces, also reserved. - Falls back to 'unnamed' for empty results and caps length at 200 - chars to stay well under common filesystem limits. + Falls back to 'unnamed' for empty results and caps length at ``limit`` + chars (default 200) to stay well under common filesystem limits; ``to_wiki`` + lowers ``limit`` when the wiki directory leaves less than that inside + Windows' MAX_PATH window (#2655). Parentheses are DROPPED rather than substituted: every callable node is labelled ``foo()``, and substituting would leave a trailing ``foo__`` on @@ -45,7 +51,7 @@ def _safe_filename(name: str) -> str: s = s.replace("(", "").replace(")", "") s = _UNSAFE_SLUG_CHARS.sub('_', s) s = s.strip('. ') - return s[:200] if s else 'unnamed' + return s[:limit] if s else 'unnamed' def _md_link(label: str, resolver: dict[str, str]) -> str: @@ -322,6 +328,12 @@ def to_wiki( count = 0 used_slugs: set[str] = set() + # Articles are capped against THIS wiki directory, not just NAME_MAX: on + # Windows a 200-char slug under an ordinary graphify-out/wiki/ overruns + # MAX_PATH and write_text raises FileNotFoundError partway through the + # export (#2655). No-op on POSIX. + _slug_limit = stem_filename_budget(out, reserve=_SLUG_SUFFIX_RESERVE) + def _unique_slug(base: str) -> str: # Fold case in the collision check: two labels differing only by case # (e.g. "Parser" vs "parser") resolve to one path on case-insensitive @@ -349,7 +361,7 @@ def to_wiki( community_slugs: dict[int, str] = {} for cid in communities: label = labels.get(cid, f"Community {cid}") - slug = _unique_slug(_safe_filename(label)) + slug = _unique_slug(_safe_filename(label, _slug_limit)) community_slugs[cid] = slug resolver.setdefault(label, slug) @@ -357,7 +369,7 @@ def to_wiki( for node_data in god_nodes_data: nid = node_data.get("id") if nid and nid in G: - slug = _unique_slug(_safe_filename(node_data['label'])) + slug = _unique_slug(_safe_filename(node_data['label'], _slug_limit)) god_articles.append((nid, slug)) resolver.setdefault(node_data['label'], slug) diff --git a/tests/test_export_path_length.py b/tests/test_export_path_length.py new file mode 100644 index 00000000..841160b1 --- /dev/null +++ b/tests/test_export_path_length.py @@ -0,0 +1,207 @@ +"""Regression tests for issue #2655: export filename caps must respect the +DESTINATION PATH length, not only the per-component NAME_MAX. + +#1094 capped export stems at 200 bytes so they stay under the conventional +255-byte NAME_MAX. That is the correct constraint on POSIX and the wrong one on +Windows, where the limit applies to the WHOLE path (MAX_PATH = 260 chars +including the terminating NUL). A 200-byte stem under an ordinary vault +directory therefore overruns MAX_PATH, and `graphify export obsidian` / +`export wiki` die mid-write with FileNotFoundError, leaving a half-written +vault behind. + +The budget math is exercised on every platform by faking `os.name`, and the +exporters' wiring is exercised by forcing a small budget, so this suite has +real teeth on the Linux CI runners as well as on Windows. +""" +import json +import os +import re + +import networkx as nx +import pytest + +from graphify import export as export_mod +from graphify import wiki as wiki_mod +from graphify.export import _obsidian_safe_stem, to_canvas, to_obsidian +from graphify.paths import _MIN_STEM_BUDGET, _WINDOWS_MAX_PATH, stem_filename_budget +from graphify.wiki import _safe_filename, to_wiki + + +def _graph(labels: list[str]) -> tuple[nx.Graph, dict[int, list[str]]]: + G = nx.Graph() + ids = [] + for i, lab in enumerate(labels): + nid = f"n{i}" + G.add_node(nid, label=lab, file_type="code", source_file="x.py", community=0) + ids.append(nid) + for a, b in zip(ids, ids[1:]): + G.add_edge(a, b, relation="calls", confidence="EXTRACTED") + return G, {0: ids} + + +def _fake_windows(monkeypatch): + """Make stem_filename_budget take its Windows branch on any host. + + abspath becomes identity so a literal ``C:\\...`` string is not prefixed + with the POSIX cwd when the test runs on Linux. + """ + monkeypatch.setattr(os, "name", "nt") + monkeypatch.setattr(os.path, "abspath", lambda p: str(p)) + + +# --------------------------------------------------------------------------- +# stem_filename_budget: the budget math +# --------------------------------------------------------------------------- + +def test_budget_is_untouched_on_posix(monkeypatch): + monkeypatch.setattr(os, "name", "posix") + # Even an absurdly deep directory must not change POSIX behaviour: the + # constraint there is per-component, and existing vaults must stay stable. + assert stem_filename_budget("/" + "d/" * 200, reserve=4) == 200 + + +def test_budget_shrinks_so_the_whole_path_fits_max_path(monkeypatch): + _fake_windows(monkeypatch) + vault = r"C:\Users\dev\projects\payments-api\graphify-out\obsidian" + budget = stem_filename_budget(vault, reserve=4) + + assert budget < 200, "an ordinary vault path must shrink the 200-byte default" + # The longest name this budget can produce still has to fit in MAX_PATH. + longest = len(vault) + len(os.sep) + budget + len("_999") + len(".md") + assert longest < _WINDOWS_MAX_PATH + + +def test_budget_accounts_for_the_caller_reserve(monkeypatch): + _fake_windows(monkeypatch) + vault = r"C:\Users\dev\projects\payments-api\graphify-out\obsidian" + assert stem_filename_budget(vault, reserve=4) - stem_filename_budget(vault, reserve=15) == 11 + + +def test_budget_never_exceeds_the_requested_limit(monkeypatch): + _fake_windows(monkeypatch) + # A very short root leaves plenty of room; the NAME_MAX-derived limit still wins. + assert stem_filename_budget("C:\\", reserve=0) == 200 + + +def test_budget_floors_instead_of_going_negative(monkeypatch): + _fake_windows(monkeypatch) + deep = "C:\\" + "\\".join("dir%03d" % i for i in range(40)) + assert len(deep) > _WINDOWS_MAX_PATH + # A negative budget would make _cap_filename slice with a negative index and + # silently emit a garbage stem, so the floor matters. + assert stem_filename_budget(deep, reserve=4) == _MIN_STEM_BUDGET + + +def test_budget_ignores_extended_length_paths(monkeypatch): + _fake_windows(monkeypatch) + # "\\?\" opts the path out of MAX_PATH entirely - nothing to shrink. + assert stem_filename_budget(r"\\?\C:\very\deep" + "\\x" * 100, reserve=4) == 200 + + +# --------------------------------------------------------------------------- +# The stem helpers honour an explicit limit +# --------------------------------------------------------------------------- + +def test_obsidian_stem_honours_an_explicit_limit(): + stem = _obsidian_safe_stem("a" * 300, 60) + assert len(stem.encode("utf-8")) <= 60 + + +def test_obsidian_stem_stays_collision_safe_at_a_small_limit(): + prefix = "z" * 250 + a = _obsidian_safe_stem(prefix + "_ALPHA", 40) + b = _obsidian_safe_stem(prefix + "_BETA", 40) + assert a != b, "truncation dropped the only distinguishing bytes" + assert len(a.encode("utf-8")) <= 40 and len(b.encode("utf-8")) <= 40 + + +def test_wiki_safe_filename_honours_an_explicit_limit(): + assert len(_safe_filename("w" * 300, 60)) <= 60 + + +# --------------------------------------------------------------------------- +# The exporters actually thread the budget through (runs on every platform) +# --------------------------------------------------------------------------- + +def test_obsidian_respects_a_small_budget_and_links_still_resolve(tmp_path, monkeypatch): + monkeypatch.setattr(export_mod, "stem_filename_budget", lambda out, **kw: 40 - kw.get("reserve", 0)) + G, comms = _graph(["a" * 300, "b" * 300, "neighbor"]) + to_obsidian(G, comms, str(tmp_path)) + + written = list(tmp_path.glob("*.md")) + assert len(written) == 4, [p.name for p in written] # 3 nodes + 1 community + for p in written: + assert len(p.stem) <= 40, p.name + + stems = {p.stem for p in written} + for p in written: + for target in re.findall(r"\[\[([^\]|]+)", p.read_text(encoding="utf-8")): + assert target in stems, f"dangling wikilink {target!r} in {p.name}" + + +def test_canvas_card_refs_match_the_notes_under_a_small_budget(tmp_path, monkeypatch): + monkeypatch.setattr(export_mod, "stem_filename_budget", lambda out, **kw: 40 - kw.get("reserve", 0)) + G, comms = _graph(["a" * 300, "b" * 300]) + to_obsidian(G, comms, str(tmp_path)) + # The CLI calls to_canvas without the node_filenames map, so the canvas has + # to re-derive the same budget or every card points at a missing note. + to_canvas(G, comms, str(tmp_path / "graph.canvas")) + + data = json.loads((tmp_path / "graph.canvas").read_text(encoding="utf-8")) + refs = [n["file"] for n in data["nodes"] if n.get("type") == "file"] + assert refs + for ref in refs: + assert (tmp_path / ref).exists(), f"canvas card points at missing note: {ref}" + + +def test_wiki_respects_a_small_budget(tmp_path, monkeypatch): + monkeypatch.setattr(wiki_mod, "stem_filename_budget", lambda out, **kw: 40 - kw.get("reserve", 0)) + G, comms = _graph(["a" * 300, "b" * 300]) + out = tmp_path / "wiki" + to_wiki(G, comms, str(out), community_labels={0: "L" * 300}) + + written = list(out.glob("*.md")) + assert written + for p in written: + assert len(p.stem) <= 40, p.name + + +# --------------------------------------------------------------------------- +# End-to-end on the platform that actually has the ceiling +# --------------------------------------------------------------------------- + +_WINDOWS_ONLY = pytest.mark.skipif( + os.name != "nt", reason="MAX_PATH is a Windows constraint" +) + + +@_WINDOWS_ONLY +def test_obsidian_writes_paths_inside_max_path(tmp_path): + G, comms = _graph(["a" * 300, "short"]) + to_obsidian(G, comms, str(tmp_path)) + written = list(tmp_path.glob("*.md")) + assert written + for p in written: + assert len(str(p)) < _WINDOWS_MAX_PATH, f"{len(str(p))} chars: {p}" + + +@_WINDOWS_ONLY +def test_wiki_writes_paths_inside_max_path(tmp_path): + G, comms = _graph(["a" * 300, "short"]) + out = tmp_path / "wiki" + to_wiki(G, comms, str(out), community_labels={0: "C" * 300}) + written = list(out.glob("*.md")) + assert written + for p in written: + assert len(str(p)) < _WINDOWS_MAX_PATH, f"{len(str(p))} chars: {p}" + + +@_WINDOWS_ONLY +def test_canvas_card_targets_exist_inside_max_path(tmp_path): + G, comms = _graph(["a" * 300, "short"]) + to_obsidian(G, comms, str(tmp_path)) + to_canvas(G, comms, str(tmp_path / "graph.canvas")) + data = json.loads((tmp_path / "graph.canvas").read_text(encoding="utf-8")) + for node in data["nodes"]: + if node.get("type") == "file": + assert (tmp_path / node["file"]).exists(), node["file"]