fix(extract): normalize whitespace before truncating rationale labels (#2206)

_extract_python_rationale / _extract_js_rationale sliced the raw
docstring/comment text to 80 characters before collapsing whitespace,
so the cut could land mid-word, leave a run of literal spaces where a
newline + indentation used to be, and, when the cut landed on a ".",
produce an Obsidian export filename ending in "..md".

Both _add_rationale sites now share _shorten_rationale_label, which
normalizes whitespace first via textwrap.shorten (word-boundary safe,
adds a placeholder only when it actually truncates) and falls back to
a plain character truncation when shorten collapses to a bare
placeholder -- which it does when the first word alone is already
>= 80 chars (e.g. a comment opening with one long URL), a case that
would otherwise regress to a content-free label.
This commit is contained in:
Yyunozor
2026-07-28 09:37:39 +01:00
committed by safishamsi
parent 952b477a3f
commit b1eadad05b
2 changed files with 205 additions and 2 deletions
+28 -2
View File
@@ -7,6 +7,7 @@ import json
import os
import re
import sys
import textwrap
from collections import Counter
from dataclasses import dataclass, field
from pathlib import Path
@@ -1033,6 +1034,23 @@ _SWIFT_CONFIG = LanguageConfig(
_RATIONALE_PREFIXES = ("# NOTE:", "# IMPORTANT:", "# HACK:", "# WHY:", "# RATIONALE:", "# TODO:", "# FIXME:")
def _shorten_rationale_label(text: str, width: int = 80) -> str:
"""Collapse whitespace and truncate ``text`` to ``width`` chars for a
rationale node label, cutting on a word boundary rather than mid-word.
Shared by the Python and JS/TS rationale extractors (#2206).
``textwrap.shorten`` collapses to just the placeholder when the first
"word" alone exceeds ``width`` (e.g. a docstring/comment that opens with
an unbroken URL) -- that would emit a content-free label, so fall back to
a plain character truncation of the normalized text in that case.
"""
label = textwrap.shorten(text, width=width, placeholder="")
if label in ("", ""):
flat = " ".join(text.split())
label = flat if len(flat) <= width else flat[: width - 1] + ""
return label
def _is_autogenerated_python(source: bytes) -> bool:
"""Return True if this Python file is auto-generated and its module docstring is noise.
@@ -1091,7 +1109,11 @@ def _extract_python_rationale(path: Path, result: dict) -> None:
return None
def _add_rationale(text: str, line: int, parent_nid: str) -> None:
label = text[:80].replace("\r\n", " ").replace("\r", " ").replace("\n", " ").strip()
# Normalize whitespace before truncating, not after: slicing raw text
# first can land mid-word, leave a run of literal spaces where a
# newline + indentation used to be, or end on a "." that turns into
# an Obsidian "..md" filename once export.py appends the extension.
label = _shorten_rationale_label(text)
rid = _make_id(stem, "rationale", str(line))
if rid not in seen_ids:
seen_ids.add(rid)
@@ -1228,7 +1250,11 @@ def _extract_js_rationale(path: Path, result: dict) -> None:
seen_doc_refs: set[str] = set()
def _add_rationale(text: str, line: int) -> None:
label = text[:80].replace("\r\n", " ").replace("\r", " ").replace("\n", " ").strip()
# Normalize whitespace before truncating, not after: slicing raw text
# first can land mid-word, leave a run of literal spaces where a
# newline + indentation used to be, or end on a "." that turns into
# an Obsidian "..md" filename once export.py appends the extension.
label = _shorten_rationale_label(text)
rid = _make_id(stem, "rationale", str(line))
if rid not in seen_ids:
seen_ids.add(rid)
+177
View File
@@ -263,6 +263,106 @@ def test_decorated_method_node_id_is_class_qualified(tmp_path):
)
# ── Regression for #2206: labels must normalize whitespace before truncating ──
def test_long_docstring_label_truncates_on_word_boundary(tmp_path):
"""A docstring longer than the 80-char cap must be shortened at a word
boundary, not mid-word. Before the fix, ``text[:80]`` sliced "feeds" down
to "feed"."""
docstring = ("This routine reconciles pending settlement batches nightly "
"because upstream feeds arrive unordered and out of sequence.")
path = _write_py(tmp_path, f'''
def reconcile_batches():
"""{docstring}"""
pass
''')
result = extract_python(path)
rationale = [n for n in result["nodes"] if n.get("file_type") == "rationale"]
assert len(rationale) == 1
label = rationale[0]["label"]
assert len(label) <= 80
core = label[:-1].rstrip() if label.endswith("") else label
assert docstring.startswith(core)
if core != docstring:
# Whatever follows the retained prefix in the source must be a space
# (or end of string) -- i.e. the cut landed on a word boundary.
assert docstring[len(core):len(core) + 1] in (" ", ""), label
def test_docstring_newline_and_indentation_collapsed_to_single_space(tmp_path):
"""A multi-line docstring's line break + indentation must not survive as a
run of literal spaces inside the label (the raw slice used to keep them
because it ran before the newline-to-space normalization)."""
path = _write_py(tmp_path, '''
def sync_inventory():
"""Aggregates daily settlement counts for reconciliation runs.
Retries three times before raising to the monitoring pipeline.
"""
pass
''')
result = extract_python(path)
rationale = [n for n in result["nodes"] if n.get("file_type") == "rationale"]
label = rationale[0]["label"]
assert "\n" not in label
assert " " not in label, f"whitespace run survived in label: {label!r}"
assert "runs. Retries" in label
def test_truncated_docstring_never_ends_with_bare_period(tmp_path):
"""When the old 80-char cut happened to land on a ".", the Obsidian
exporter appended ".md" and produced a double-dot filename. A truncated
label must end on the placeholder, never on a lone trailing period."""
docstring = ("Loads the merchant configs bundle from disk once at process "
"start-up and caches. It refreshes every six hours in the background.")
path = _write_py(tmp_path, f'''
def load_merchant_config():
"""{docstring}"""
pass
''')
result = extract_python(path)
rationale = [n for n in result["nodes"] if n.get("file_type") == "rationale"]
label = rationale[0]["label"]
assert len(label) < len(docstring), "expected this docstring to be truncated"
assert not label.endswith("."), label
assert label.endswith(""), label
def test_docstring_opening_with_unbroken_long_token_keeps_content(tmp_path):
"""Adversarial case: ``textwrap.shorten`` alone collapses to just the
placeholder when the first whitespace-delimited "word" already exceeds
the width (e.g. a docstring opening with a long, unbroken URL) -- that
would regress to a content-free label, worse than the original bug. The
label must still carry real content."""
url = "https://example.com/api/v3/settlements/" + "a" * 60 + "/confirm"
docstring = f"{url} documents the retry contract for this handler."
path = _write_py(tmp_path, f'''
def call_endpoint():
"""{docstring}"""
pass
''')
result = extract_python(path)
rationale = [n for n in result["nodes"] if n.get("file_type") == "rationale"]
label = rationale[0]["label"]
assert label not in ("", ""), label
assert label.startswith("https://example.com/"), label
def test_short_docstring_label_unchanged(tmp_path):
"""Non-regression: a docstring well under 80 chars must pass through
byte-for-byte, with no placeholder and no reformatting."""
docstring = "Splits the bearer token because some clients send a stray prefix."
path = _write_py(tmp_path, f'''
def parse_token():
"""{docstring}"""
pass
''')
result = extract_python(path)
rationale = [n for n in result["nodes"] if n.get("file_type") == "rationale"]
label = rationale[0]["label"]
assert label == docstring
# ── JS/TS rationale + doc-reference extraction ────────────────────────────────
@@ -330,3 +430,80 @@ def test_js_adr_in_string_literal_not_extracted(tmp_path):
result = extract_js(path)
refs = [n for n in result["nodes"] if n.get("file_type") == "doc_ref"]
assert refs == []
# ── Regression for #2206, JS/TS site (shares the fix with the Python site) ────
def test_js_rationale_label_truncates_on_word_boundary(tmp_path):
"""Same invariant as the Python site: a long ``// WHY:`` comment must be
shortened at a word boundary, not mid-word."""
from graphify.extract import extract_js
comment_text = ("retries are capped because the upstream billing service "
"enforces a strict per-tenant rate limit that keeps dropping requests")
path = _write_ts(tmp_path, f'''
// WHY: {comment_text}
export function fetchData(): void {{}}
''')
result = extract_js(path)
rationale = [n for n in result["nodes"] if n.get("file_type") == "rationale"]
assert len(rationale) == 1
label = rationale[0]["label"]
full = f"WHY: {comment_text}"
assert len(label) <= 80
core = label[:-1].rstrip() if label.endswith("") else label
assert full.startswith(core)
if core != full:
assert full[len(core):len(core) + 1] in (" ", ""), label
def test_js_rationale_label_never_ends_with_bare_period_when_truncated(tmp_path):
"""Same invariant as the Python site: a truncated label must never end on
a lone "." (double-dot Obsidian filename)."""
from graphify.extract import extract_js
comment_text = ("retries are capped at five attempts before the circuit breaker "
"opens for the endpoint. A metrics counter records every trip.")
path = _write_ts(tmp_path, f'''
// WHY: {comment_text}
export function fetchData(): void {{}}
''')
result = extract_js(path)
rationale = [n for n in result["nodes"] if n.get("file_type") == "rationale"]
label = rationale[0]["label"]
full = f"WHY: {comment_text}"
assert len(label) < len(full), "expected this comment to be truncated"
assert not label.endswith("."), label
def test_js_rationale_comment_opening_with_unbroken_long_token_keeps_content(tmp_path):
"""Same adversarial case as the Python site: a ``// WHY:`` comment whose
content is an unbroken long URL must not collapse to a content-free
placeholder label. Unlike the Python site, the ``WHY:`` prefix always
fits on its own, so the invariant is "some real content survives",
not "the URL itself survives" (it genuinely cannot fit in 80 chars)."""
from graphify.extract import extract_js
url = "https://example.com/api/v3/settlements/" + "a" * 60 + "/confirm"
path = _write_ts(tmp_path, f'''
// WHY: {url} documents the retry contract for this handler.
export function fetchData(): void {{}}
''')
result = extract_js(path)
rationale = [n for n in result["nodes"] if n.get("file_type") == "rationale"]
label = rationale[0]["label"]
assert label not in ("", ""), label
assert label.startswith("WHY:"), label
def test_js_short_rationale_comment_unchanged(tmp_path):
"""Non-regression: a short ``// NOTE:`` comment must pass through
byte-for-byte, matching the pre-existing test above but pinned exactly."""
from graphify.extract import extract_js
path = _write_ts(tmp_path, '''
// NOTE: must run before compile() or the linker will fail
export function build(): void {}
''')
result = extract_js(path)
rationale = [n for n in result["nodes"] if n.get("file_type") == "rationale"]
assert [n["label"] for n in rationale] == [
"NOTE: must run before compile() or the linker will fail"
]