From 5e329adc28fb1797e85dab59acc28e152dee2062 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Fri, 17 Jul 2026 01:28:21 -0500 Subject: [PATCH] =?UTF-8?q?fix(kernel):=20CRLF=20docstring=20parity=20?= =?UTF-8?q?=E2=80=94=20JS=20multiline=20^=20anchors=20after=20\r,=20regex?= =?UTF-8?q?=20crate's=20(=3Fm)^=20is=20\n-only=20(#1329)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On CRLF checkouts (every Windows autocrlf clone) the JS reference's block-continuation strip /^\s*\*\s?/gm finds a line start after the \r and its greedy \s* consumes the \n, leaving a bare \r in the docstring; the kernel's (?m)^ pass matched after \n only and kept \r\n. Caught by the O2 Windows VM leg (6 kernel-tsjs-parity failures), reproduced on macOS by CRLF-converting the fixtures. js_multiline_strip now replicates the JS anchor set (\n, \r, U+2028, U+2029) for all five line-marker passes; CRLF variants of every torture fixture are pinned in kernel-tsjs-parity, derived in-memory so nothing can normalize them away. Co-authored-by: Claude Fable 5 --- __tests__/kernel-tsjs-parity.test.ts | 19 ++++++ codegraph-kernel/src/docstring.rs | 77 ++++++++++++++++++++--- docs/design/rust-kernel-migration-plan.md | 7 +++ 3 files changed, 93 insertions(+), 10 deletions(-) diff --git a/__tests__/kernel-tsjs-parity.test.ts b/__tests__/kernel-tsjs-parity.test.ts index cdf0289..c16d41f 100644 --- a/__tests__/kernel-tsjs-parity.test.ts +++ b/__tests__/kernel-tsjs-parity.test.ts @@ -125,6 +125,25 @@ describe.skipIf(!kernelBuilt)('kernel TS/JS extraction parity', () => { assertParity(rel, fs.readFileSync(file, 'utf8'), 'typescript'); }); + // Every torture fixture again with CRLF line endings — the shape every + // Windows autocrlf checkout has. Derived in memory (not a checked-in CRLF + // file) so no platform or editor can silently normalize it away. Pins the + // JS-multiline-^ semantics in the kernel's docstring cleaning: JS `^`/m + // anchors after \r too, so the block-continuation `\s*` eats the `\n` and + // the cleaned docstring keeps a bare `\r` (caught on the Windows VM leg of + // the O2 gate; diverged in the kernel until docstring.rs mirrored it). + it.each([ + ['torture.tsx', 'tsx'], + ['torture.js', 'javascript'], + ['Torture.java', 'java'], + ['torture.py', 'python'], + ['torture.go', 'go'], + ] as const)('torture fixture CRLF parity: %s', (name, lang) => { + const file = path.join(FIXTURE_DIR, name); + const crlf = fs.readFileSync(file, 'utf8').replace(/(? { // tree-sitter error RECOVERY differs between UTF-8 (native) and UTF-16 // (web-tree-sitter) parsing — same grammar, same core version — so the diff --git a/codegraph-kernel/src/docstring.rs b/codegraph-kernel/src/docstring.rs index f4861a9..de9a55a 100644 --- a/codegraph-kernel/src/docstring.rs +++ b/codegraph-kernel/src/docstring.rs @@ -52,14 +52,55 @@ fn cleaners() -> &'static Cleaners { paren_star_close: Regex::new(r"\*\)$").unwrap(), brace_open: Regex::new(r"^\{").unwrap(), brace_close: Regex::new(r"\}$").unwrap(), - slashes: Regex::new(r"(?m)^//[/!]?\s?").unwrap(), - dashes: Regex::new(r"(?m)^--\s?").unwrap(), - hash: Regex::new(r"(?m)^#\s?").unwrap(), - percent: Regex::new(r"(?m)^%+\s?").unwrap(), - star_cont: Regex::new(r"(?m)^\s*\*\s?").unwrap(), + slashes: Regex::new(r"\A//[/!]?\s?").unwrap(), + dashes: Regex::new(r"\A--\s?").unwrap(), + hash: Regex::new(r"\A#\s?").unwrap(), + percent: Regex::new(r"\A%+\s?").unwrap(), + star_cont: Regex::new(r"\A\s*\*\s?").unwrap(), }) } +/// JS multiline `^` anchors after \n, \r, U+2028, U+2029; the regex crate's +/// `(?m)^` anchors after `\n` only. On CRLF content the JS engine finds a line +/// start after the `\r`, so a greedy leading `\s*` (the block-continuation +/// rule) consumes the `\n` and leaves the bare `\r` in the docstring — +/// byte-parity on CRLF checkouts (every Windows autocrlf clone) depends on +/// reproducing exactly that. +fn is_js_line_terminator(ch: char) -> bool { + matches!(ch, '\n' | '\r' | '\u{2028}' | '\u{2029}') +} + +/// JS-semantics `str.replace(/^/gm, "")`: try the \A-anchored `pat` at +/// position 0 and after every JS line terminator, left to right, resuming +/// after each match's end — a faithful /g replace. (Remaining known +/// divergence: JS `\s` includes U+FEFF, Rust's does not; an embedded BOM +/// inside a comment is accepted as unreachable.) +fn js_multiline_strip(s: &str, pat: &Regex) -> String { + let mut out = String::with_capacity(s.len()); + let mut last = 0usize; + let mut pos = 0usize; + while pos <= s.len() { + let at_line_start = pos == 0 + || s[..pos].chars().next_back().is_some_and(is_js_line_terminator); + if at_line_start { + if let Some(m) = pat.find(&s[pos..]) { + if !m.is_empty() { + out.push_str(&s[last..pos]); + last = pos + m.end(); + pos = last; + continue; + } + } + } + match s[pos..].chars().next() { + Some(c) => pos += c.len_utf8(), + None => break, + } + } + out.push_str(&s[last..]); + out +} + /// cleanCommentMarkers — strip comment syntax, keep the prose. pub fn clean_comment_markers(comment: &str) -> String { let c = cleaners(); @@ -77,11 +118,11 @@ pub fn clean_comment_markers(comment: &str) -> String { s = c.brace_open.replace(&s, "").into_owned(); s = c.brace_close.replace(&s, "").into_owned(); } - s = c.slashes.replace_all(&s, "").into_owned(); - s = c.dashes.replace_all(&s, "").into_owned(); - s = c.hash.replace_all(&s, "").into_owned(); - s = c.percent.replace_all(&s, "").into_owned(); - s = c.star_cont.replace_all(&s, "").into_owned(); + s = js_multiline_strip(&s, &c.slashes); + s = js_multiline_strip(&s, &c.dashes); + s = js_multiline_strip(&s, &c.hash); + s = js_multiline_strip(&s, &c.percent); + s = js_multiline_strip(&s, &c.star_cont); s.trim().to_string() } @@ -137,4 +178,20 @@ mod tests { "Adds things.\n@param a first" ); } + + /// CRLF parity with the JS reference: multiline `^` matches after `\r`, + /// so the block-continuation `\s*` eats the `\n` and the bare `\r` + /// survives in the cleaned docstring (pinned against the wasm extractor + /// on a CRLF checkout — the Windows autocrlf shape). + #[test] + fn crlf_matches_js_reference() { + assert_eq!( + clean_comment_markers("/**\r\n * Class docs.\r\n * Multi-line.\r\n */"), + "Class docs.\rMulti-line." + ); + assert_eq!( + clean_comment_markers("// a\r\n// b"), + "a\r\nb" + ); + } } diff --git a/docs/design/rust-kernel-migration-plan.md b/docs/design/rust-kernel-migration-plan.md index ed7d378..038ea0a 100644 --- a/docs/design/rust-kernel-migration-plan.md +++ b/docs/design/rust-kernel-migration-plan.md @@ -134,6 +134,13 @@ init twice (kernel arm vs `CODEGRAPH_KERNEL=0`), `dump-graph.mjs` each, `cmp`. refs carry NO denormalized filePath/language (the store fills them). The strict full-object parity compare exists because a loose one masked precisely this. - Grammar bumps: crate + vendored wasm move TOGETHER or kernel-grammar-parity fails. +- **JS multiline `^` anchors after `\r` (and U+2028/U+2029); the regex crate's + `(?m)^` is `\n`-only** — on CRLF checkouts (Windows autocrlf) the JS reference's + greedy `\s*` eats the `\n` of a CRLF pair and the cleaned docstring keeps a bare + `\r`. Caught by the O2 Windows leg (6 parity failures), fixed via + `js_multiline_strip` in `docstring.rs`; CRLF variants of every torture fixture + are pinned in `kernel-tsjs-parity` (derived in-memory — normalization-proof). + Any future walker regex with `(?m)` needs the same scrutiny. - Perf claims: measure before believing — the plan's own §1/§6 expectations were corrected twice (many-core parse-loop wall = store-writer, §4d; cg1212 parse = C-bound, §4f).