From 216c7eaa943d43567174ab6cf8bdc2aaf420ff7e Mon Sep 17 00:00:00 2001 From: Louise Lau Date: Thu, 2 Jul 2026 09:53:00 +0800 Subject: [PATCH] =?UTF-8?q?fix(codegraph):=20strip=20comments=20before=20p?= =?UTF-8?q?arameterize=20alignment=20=E2=80=94=20found=20by=20the=20live?= =?UTF-8?q?=20v8=20dry-run?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ran the extract-helper (v8) selection live against QodeX itself (1,686 functions → 25 near-dupe clusters). Verdict: blocked — 0 qualifying clusters. Hand-inspection confirmed every decline was CORRECT (dedupeStr vs dedupe differ on empty-string handling; the isPrivateOrLocal pair is structurally divergent; walkSource is genuinely a 4-way divergent family; the path-helpers cluster mixes two arg-shapes) — the conservative gate is doing its job. But the run also exposed a real calibration bug: 100%-similar pairs were declining as "structurally divergent" ONLY because their COMMENTS tokenized differently. Comments are semantically irrelevant to alignment. - helper-extract.ts: stripComments() (string-literal-aware — a // inside a string survives) applied before token alignment in proposeParameterizedHelper. - Regression test: comment-only differences now align (params = the actual varying literal); a // inside a string is not eaten. - ADOPTION.md: the live dry-run story — "a block worth reading": receipt, per-decline inspection, and the refine-the-guardrail loop. --- docs/ADOPTION.md | 28 ++++++++++++++++++++++++++++ src/codegraph/helper-extract.ts | 26 ++++++++++++++++++++++++-- test/helper-extract.test.ts | 21 +++++++++++++++++++++ 3 files changed, 73 insertions(+), 2 deletions(-) diff --git a/docs/ADOPTION.md b/docs/ADOPTION.md index 7f7cfcd..c15243b 100644 --- a/docs/ADOPTION.md +++ b/docs/ADOPTION.md @@ -74,6 +74,34 @@ Proposed shared helper `extractedHelper(kind, inclusive)` — the bodies differ (not covered — different structure: the ZodNumber variants, whose value is 0 vs BigInt(0)) ``` +### Live `extract-helper` (v8) dry-run on QodeX itself — a block worth reading + +We ran the v8 scope's selection live against this repo (1,686 functions → 25 near-dupe clusters) +and the verdict was: + +``` +VERIFIED-PR: blocked — dry-run: 0 candidate(s) +``` + +Zero qualifying clusters — and hand-inspection confirmed **every decline was correct**: + +- `dedupeStr` vs `dedupe` (99% similar) — *behaviorally different*: one filters out empty + strings, the other keeps them. Merging them mechanically would change behavior. +- the two `isPrivateOrLocal` copies (100% similar) — one merges two `if`s the other keeps + separate; structurally divergent even though equivalent, so mechanical parameterization + can't prove itself. +- the famous 4-copy `walkSource` family — genuinely divergent (different result shapes and + filters); it needs human-judgment consolidation, not a 3am mechanical pass. +- the 5-copy path-helpers cluster — actually *two* families (1-arg and 2-arg) mixed; v8's + full-coverage rule refuses to half-consolidate a cluster. + +The dry-run also caught a real calibration bug — 100%-similar pairs were being declined just +because their *comments* tokenized differently — which is now fixed (comments are stripped +before alignment). That's the loop working end to end: run live → read the receipt → refine the +guardrail → the remaining blocks are all *correct* blocks. On a codebase with textbook +copy-paste-then-tweak-a-literal families (like zod's `positive`/`negative` above), v8 fires; +on a codebase without them, it proves that fact instead of forcing a refactor. + **And one of them became a real upstream PR:** the axios finding was reviewed by hand, refactored into `lib/helpers/setFormDataHeaders.js` (keeping the more defensive of the two variants), verified against axios's own suite (lint clean; all 89 form-data tests passing), and submitted — diff --git a/src/codegraph/helper-extract.ts b/src/codegraph/helper-extract.ts index 4da85fb..8f351d5 100644 --- a/src/codegraph/helper-extract.ts +++ b/src/codegraph/helper-extract.ts @@ -196,6 +196,26 @@ export interface ParamProposal { interface RawTok { text: string; start: number } +/** Remove comments (but NOT string contents) so comment-only differences don't block alignment. PURE. */ +function stripComments(body: string): string { + // Walk once, honoring string/template literals so a "//" inside a string survives. + let out = ''; + let i = 0; + while (i < body.length) { + const ch = body[i]!; + if (ch === '"' || ch === "'" || ch === '`') { + const quote = ch; out += ch; i++; + while (i < body.length && body[i] !== quote) { if (body[i] === '\\') { out += body[i]! + (body[i + 1] ?? ''); i += 2; continue; } out += body[i]!; i++; } + out += body[i] ?? ''; i++; + continue; + } + if (ch === '/' && body[i + 1] === '/') { while (i < body.length && body[i] !== '\n') i++; continue; } + if (ch === '/' && body[i + 1] === '*') { i += 2; while (i < body.length && !(body[i] === '*' && body[i + 1] === '/')) i++; i += 2; continue; } + out += ch; i++; + } + return out; +} + /** Code tokens WITH literals kept and source offsets (for substitution in the sketch). PURE. */ function rawTokens(body: string): RawTok[] { const re = /"(?:[^"\\]|\\.)*"|'(?:[^'\\]|\\.)*'|`(?:[^`\\]|\\.)*`|\d+(?:\.\d+)?|[A-Za-z_$][\w$]*|[^\sA-Za-z0-9_$]/g; @@ -217,8 +237,10 @@ export function proposeParameterizedHelper(members: FunctionUnit[], helperName = const decline = (reason: string): ParamProposal => ({ ok: false, reason, helperName, params: [], sketch: '', calls: [] }); if (members.length < 2) return decline('need at least two functions'); - // Neutralize each function's own name so it never reads as a varying token. - let bodies = members.map(m => m.body.replace(new RegExp(`\\b${m.name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}\\b`, 'g'), helperName)); + // Neutralize each function's own name, and strip comments BEFORE aligning: two bodies that + // differ only in comments are the same code (dogfooding caught 100%-similar pairs declining + // as "structurally divergent" purely because their comments tokenized differently). + let bodies = members.map(m => stripComments(m.body).replace(new RegExp(`\\b${m.name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}\\b`, 'g'), helperName)); let toks = bodies.map(b => rawTokens(b)); let dropped: string[] | undefined; if (toks.some(t => t.length !== toks[0]!.length)) { diff --git a/test/helper-extract.test.ts b/test/helper-extract.test.ts index 3566a11..ec4e2ac 100644 --- a/test/helper-extract.test.ts +++ b/test/helper-extract.test.ts @@ -155,6 +155,27 @@ describe('proposeParameterizedHelper (v2 — parameterize the near-dupes)', () = expect(proposeParameterizedHelper(identical).reason).toMatch(/consolidate-dupes/); }); + it('comment-only differences do NOT block alignment (live dry-run regression)', () => { + // Dogfooding v8 on QodeX: 100%-similar pairs declined as "structurally divergent" only + // because their comments tokenized differently. Comments are semantically irrelevant. + const commented = [ + { name: 'a', file: 'f.ts', startLine: 1, endLine: 4, body: 'a(x) {\n // clamp to the range\n return Math.min(x, 10);\n}' }, + { name: 'b', file: 'g.ts', startLine: 1, endLine: 4, body: 'b(x) {\n /* different words here */\n return Math.min(x, 20);\n}' }, + ]; + const pr = proposeParameterizedHelper(commented); + expect(pr.ok).toBe(true); // aligns despite different comments + expect(pr.params).toHaveLength(1); // only the 10/20 literal varies + expect(pr.params[0]!.values).toEqual(['10', '20']); + // …and a // inside a STRING literal is not treated as a comment. + const url = [ + { name: 'u1', file: 'f.ts', startLine: 1, endLine: 3, body: 'u1() { return fetch("https://a.example/x"); }' }, + { name: 'u2', file: 'g.ts', startLine: 1, endLine: 3, body: 'u2() { return fetch("https://b.example/x"); }' }, + ]; + const pu = proposeParameterizedHelper(url); + expect(pu.ok).toBe(true); + expect(pu.params[0]!.values).toEqual(['"https://a.example/x"', '"https://b.example/x"']); + }); + it('mixed structural variants: parameterizes the LARGEST aligned subset and reports the rest as dropped', () => { // The real zod shape: a Number family plus a BigInt variant whose body has extra tokens. const bigint = { name: 'positiveBig', file: 'types.ts', startLine: 20, endLine: 27,