From 8ea8fe40d6a8a2ee1b347e80b44f79174e4451cb Mon Sep 17 00:00:00 2001 From: Divyam Talwar Date: Thu, 24 Sep 2026 01:23:18 +0530 Subject: [PATCH] fix(docs): preserve semantic source bytes in anchor hashes --- src/docs/anchors.ts | 46 +++++++++++-------------- tests/shared/docs-generate.test.ts | 55 ++++++++++++++++++++++-------- tests/shared/docs-impact.test.ts | 43 +++++++++++++++++++++++ 3 files changed, 103 insertions(+), 41 deletions(-) diff --git a/src/docs/anchors.ts b/src/docs/anchors.ts index bb2618ca8..214ed50a7 100644 --- a/src/docs/anchors.ts +++ b/src/docs/anchors.ts @@ -52,37 +52,31 @@ export function readSymbolSource(node: GraphNode, repoRoot: string): string | nu } /** - * Normalize a source slice before hashing so cosmetic edits don't churn docs. + * Normalize a source slice before hashing: CRLF → LF, nothing else. * - * Strips comments, trailing whitespace, and blank lines — these change the - * bytes but not what the doc describes, so a reformat / comment edit / blank-line - * shuffle should NOT mark a doc stale. Indentation is PRESERVED (significant in - * Python). This is the main lever against false-positive rewrites. + * Comments, indentation, blank lines and trailing whitespace are all + * PRESERVED. Regex comment stripping cannot tell a comment from string + * contents — `"**\/*.ts"` … `"dist/**\/x"` looks like a block comment and + * swallowed the real code between the two literals, and whitespace inside + * multiline strings / Python indentation is semantic too. Without a real + * per-language lexer, the only safe policy is to hash the bytes. * - * It does not normalize identifiers — renaming a symbol still changes the hash - * (a genuine code edit); whether that should flag the doc is left to the gate / - * human, not hidden here. + * Tradeoff: a cosmetic edit (comment, reformat, blank line) now changes the + * hash and may trigger a doc refresh. Anchors taken under the previous + * comment/whitespace-stripping normalization will differ once for any + * symbol whose slice contained comments, blank lines or trailing + * whitespace — a one-time refresh of those docs. + * + * `language` is accepted for API compatibility and currently unused. */ -export function normalizeForHash(src: string, language?: string): string { - let s = src; - // Line comments are stripped only at line start or after whitespace — a - // bare /.*$/ would also truncate string literals ("https://x", "a#b"), - // blinding drift detection to real changes on those lines. NOTE: changing - // this normalization changes every anchor hash once (one-time full drift). - if (language === "python" || language === "ruby") { - s = s.replace(/(^|\s)#.*$/gm, "$1"); // line comments - } else { - s = s.replace(/\/\*[\s\S]*?\*\//g, ""); // block comments - s = s.replace(/(^|\s)\/\/.*$/gm, "$1"); // line comments - } - return s - .split(/\r?\n/) - .map((l) => l.replace(/\s+$/, "")) // trailing whitespace - .filter((l) => l.trim() !== "") // blank lines - .join("\n"); +export function normalizeForHash(src: string, _language?: string): string { + return src.replace(/\r\n/g, "\n"); } -/** sha256 of a source slice, after comment/whitespace normalization. */ +/** + * sha256 of a source slice after CRLF → LF normalization only (see + * `normalizeForHash`): comment and whitespace edits DO change the hash. + */ export function hashSource(src: string, language?: string): string { return createHash("sha256").update(normalizeForHash(src, language)).digest("hex"); } diff --git a/tests/shared/docs-generate.test.ts b/tests/shared/docs-generate.test.ts index 83c85b8af..e80bc714f 100644 --- a/tests/shared/docs-generate.test.ts +++ b/tests/shared/docs-generate.test.ts @@ -25,21 +25,34 @@ function mockQuery(rowsPerCall: Array> = []) { return { calls, query }; } -// ── normalizeForHash (the false-positive fix) ───────────────────────────────── +// ── normalizeForHash (CRLF → LF only; source bytes otherwise preserved) ─────── describe("normalizeForHash", () => { - it("ignores comments — a comment-only edit yields the SAME hash", () => { + it("only normalizes CRLF to LF — comments and whitespace are preserved verbatim", () => { + const src = "a();/* x */\n\n b(); // note \n# py\n"; + expect(normalizeForHash(src)).toBe(src); + expect(normalizeForHash(src, "python")).toBe(src); + expect(normalizeForHash("a();\r\n b();\r\n")).toBe("a();\n b();\n"); + }); + it("CRLF and LF versions of the same source hash identically", () => { + const lf = "function f() {\n // note\n return 1;\n}\n"; + expect(hashSource(lf.replace(/\n/g, "\r\n"))).toBe(hashSource(lf)); + expect(hashSource(lf.replace(/\n/g, "\r\n"), "python")).toBe(hashSource(lf, "python")); + }); + it("detects comment-only edits (no regex comment stripping)", () => { const a = "function f() {\n // does a thing\n return 1;\n}"; const b = "function f() {\n // does a completely different thing\n return 1;\n}"; - expect(hashSource(a)).toBe(hashSource(b)); - }); - it("ignores trailing whitespace and blank lines", () => { - const a = "function f() {\n return 1;\n}"; - const b = "function f() { \n\n return 1;\n\n}\n"; - expect(hashSource(a)).toBe(hashSource(b)); + expect(hashSource(a)).not.toBe(hashSource(b)); + expect(hashSource("return 1; // old note")).not.toBe(hashSource("return 1; // new note")); + expect(hashSource("a();/* x */\nb();")).not.toBe(hashSource("a();/* y */\nb();")); + expect(hashSource("def f():\n # old\n return 1", "python")).not.toBe( + hashSource("def f():\n # new\n return 1", "python"), + ); }); - it("strips block comments too", () => { - expect(normalizeForHash("a();/* x */\nb();")).toBe("a();\nb();"); + it("detects trailing-whitespace and blank-line edits (semantic inside multiline literals)", () => { + const a = "const s = `line1\nline2`;"; + expect(hashSource(a)).not.toBe(hashSource("const s = `line1 \nline2`;")); + expect(hashSource(a)).not.toBe(hashSource("const s = `line1\n\nline2`;")); }); it("STILL detects a real code change (different identifier / literal)", () => { expect(hashSource("return 1;")).not.toBe(hashSource("return 2;")); @@ -49,12 +62,24 @@ describe("normalizeForHash", () => { // A change after "//" inside a string MUST change the hash. expect(hashSource('const url = "https://api.example.com";')).not.toBe(hashSource('const url = "https://api.OTHER.com";')); expect(hashSource('x = "a#b"', "python")).not.toBe(hashSource('x = "a#c"', "python")); - // Real comments (start-of-line or after whitespace) are still ignored. - expect(hashSource("return 1; // old note")).toBe(hashSource("return 1; // new note")); }); - - it("uses # comments for python and preserves indentation", () => { - expect(normalizeForHash("def f():\n # comment\n return 1", "python")).toBe("def f():\n return 1"); + it("does NOT hide code between string literals that look like /* … */ (glob patterns)", () => { + // `"**/*.ts"` opens and `"dist/**/x"` closes what a regex sees as a block comment. + const base = 'const inc = "**/*.ts";\nif (ok) run(1);\nconst out = "dist/**/x";'; + const logicEdit = 'const inc = "**/*.ts";\nif (!ok) run(2);\nconst out = "dist/**/x";'; + const literalEdit = 'const inc = "**/*.tsx";\nif (ok) run(1);\nconst out = "dist/**/y";'; + expect(normalizeForHash(base)).toBe(base); + expect(hashSource(base)).not.toBe(hashSource(logicEdit)); + expect(hashSource(base)).not.toBe(hashSource(literalEdit)); + }); + it("keeps python indentation and multiline string content significant", () => { + const a = "def f():\n if x:\n return 1\n return 2"; + const dedented = "def f():\n if x:\n return 1\n return 2"; + expect(hashSource(a, "python")).not.toBe(hashSource(dedented, "python")); + const doc1 = 'def f():\n s = """\n # not a comment\n """\n return s'; + const doc2 = 'def f():\n s = """\n # changed text\n """\n return s'; + expect(normalizeForHash(doc1, "python")).toBe(doc1); + expect(hashSource(doc1, "python")).not.toBe(hashSource(doc2, "python")); }); }); diff --git a/tests/shared/docs-impact.test.ts b/tests/shared/docs-impact.test.ts index e2850b285..f39ce8716 100644 --- a/tests/shared/docs-impact.test.ts +++ b/tests/shared/docs-impact.test.ts @@ -147,6 +147,49 @@ describe("anchorStatus", () => { const a: DocAnchor = { symbol_id: n.id, content_hash: "x" }; expect(anchorStatus(a, snap([n]), dir)).toEqual({ state: "unreadable" }); }); + + // Anchor taken on `before`, file rewritten to `after`, status re-checked. + function statusAfterEdit(file: string, before: string, after: string, loc: string, language: GraphNode["language"] = "typescript") { + const n = { ...node(`${file}:foo:function`, file, loc), language }; + writeFileSync(join(dir, file), before); + const a = buildAnchor(n, dir)!; + expect(a).not.toBeNull(); + writeFileSync(join(dir, file), after); + return anchorStatus(a, snap([n]), dir).state; + } + + it("changed when logic between glob-like string literals is edited", () => { + const before = 'function foo() {\n const inc = "**/*.ts";\n if (ok) run(1);\n const out = "dist/**/x";\n}\n'; + const after = 'function foo() {\n const inc = "**/*.ts";\n if (!ok) run(2);\n const out = "dist/**/x";\n}\n'; + expect(statusAfterEdit("g.ts", before, after, "L1-L5")).toBe("changed"); + }); + + it("changed when content inside a string literal is edited", () => { + const before = 'function foo() {\n return "https://a.example/*";\n}\n'; + const after = 'function foo() {\n return "https://b.example/*";\n}\n'; + expect(statusAfterEdit("s.ts", before, after, "L1-L3")).toBe("changed"); + }); + + it("changed on a comment-only edit (bytes are hashed, comments included)", () => { + const before = "function foo() {\n // old note\n return 1;\n}\n"; + const after = "function foo() {\n // new note\n return 1;\n}\n"; + expect(statusAfterEdit("c.ts", before, after, "L1-L4")).toBe("changed"); + }); + + it("fresh when only line endings change (CRLF ↔ LF)", () => { + const lf = "function foo() {\n // note\n return 1;\n}\n"; + expect(statusAfterEdit("e.ts", lf, lf.replace(/\n/g, "\r\n"), "L1-L4")).toBe("fresh"); + expect(statusAfterEdit("e2.ts", lf.replace(/\n/g, "\r\n"), lf, "L1-L4")).toBe("fresh"); + }); + + it("changed on python indentation or multiline-string content edits", () => { + const before = "def foo():\n if x:\n return 1\n return 2\n"; + const indented = "def foo():\n if x:\n return 1\n return 2\n"; + expect(statusAfterEdit("p.py", before, indented, "L1-L4", "python")).toBe("changed"); + const s1 = 'def foo():\n s = """\n # keep\n\n """\n return s\n'; + const s2 = 'def foo():\n s = """\n # keep\n """\n return s\n'; + expect(statusAfterEdit("q.py", s1, s2, "L1-L6", "python")).toBe("changed"); + }); }); // ── computeStaleDocs (direct hash staleness) ────────────────────────────────