diff --git a/review-enrichment/src/analyzers/doc-comment.ts b/review-enrichment/src/analyzers/doc-comment.ts new file mode 100644 index 0000000000..62d139b7e5 --- /dev/null +++ b/review-enrichment/src/analyzers/doc-comment.ts @@ -0,0 +1,300 @@ +// Doc-comment drift analyzer (#1519). Flags JSDoc @param tags that don't match the adjacent +// function signature when the PR touches the relevant lines — stale tags for removed params +// and missing tags for added params. Pure compute; no network calls. +import type { EnrichRequest, DocCommentFinding } from "../types.js"; + +const MAX_FINDINGS = 20; +const MAX_LINE_CHARS = 2000; + +// Only scan JS/TS files — JSDoc is idiomatic there. +const JS_TS_EXT = /\.(js|ts|jsx|tsx|mjs|cjs)$/i; + +// --- JSDoc @param extraction --- + +/** Extract @param names from a JSDoc block. Handles {Type} prefix and [optional] bracket notation. */ +export function extractJsDocParams(block: string): string[] { + const names: string[] = []; + // @param {optional-type} [optional-bracket] name — all prefix forms via greedy optional groups + const re = /@param\s+(?:\{[^}]*\}\s+)?\[?(\$?[a-zA-Z_]\w*)/g; + let m: RegExpExecArray | null; + while ((m = re.exec(block)) !== null) { + const name = m[1]!; + if (name !== "this") names.push(name); // TS `this` pseudo-param is not a real parameter + } + return names; +} + +// --- Function signature parsing --- + +// Split a comma-separated param list at top-level commas, respecting nested <>, (), [], {}. +function splitTopLevel(s: string): string[] { + const parts: string[] = []; + let depth = 0; + let start = 0; + for (let i = 0; i < s.length; i++) { + const c = s[i]!; + if (c === "<" || c === "(" || c === "[" || c === "{") depth++; + else if (c === ">" || c === ")" || c === "]" || c === "}") depth--; + else if (c === "," && depth === 0) { + parts.push(s.slice(start, i).trim()); + start = i + 1; + } + } + const last = s.slice(start).trim(); + if (last) parts.push(last); + return parts.filter(Boolean); +} + +/** Extract simple parameter names from a JS/TS parameter list string (between `(` and `)`). + * Returns hasDestructured=true when any top-level param is destructured — those functions are + * skipped to avoid false positives from wrapper-name mismatches (e.g. `@param options` vs `{ a, b }`). */ +export function extractFunctionParams( + paramList: string, +): { params: string[]; hasDestructured: boolean } { + const params: string[] = []; + let hasDestructured = false; + for (const p of splitTopLevel(paramList)) { + const trimmed = p.trim(); + if (!trimmed) continue; + if (trimmed.startsWith("{") || trimmed.startsWith("[")) { + hasDestructured = true; + continue; + } + // Rest param: ...name or ...name: Type + const rest = /^\.\.\.(\$?[a-zA-Z_]\w*)/.exec(trimmed); + if (rest) { + params.push(rest[1]!); + continue; + } + // Normal: name, name?: Type, name: Type = default — extract just the identifier + const norm = /^(\$?[a-zA-Z_]\w*)/.exec(trimmed); + if (norm && norm[1] !== "this") params.push(norm[1]!); + } + return { params, hasDestructured }; +} + +// --- Detect function signatures --- + +// Patterns that identify JS/TS function definitions (not calls or control structures). +const FUNC_PATTERNS: RegExp[] = [ + // Traditional: `function name(` or `async function name(` or `export (default)? function` + /(?:^|\s)(?:export\s+(?:default\s+)?)?(?:async\s+)?function\s*\*?\s*\w+\s*[<(]/, + // Class constructor + /(?:^|\s)constructor\s*\(/, + // Class methods with TS access/abstract/static/async/override modifiers + /(?:^|\s)(?:(?:public|private|protected|static|abstract|override|async|readonly)\s+)+\w+\s*[<(]/, + // Arrow functions assigned to a binding: `const name = async? (` + /(?:^|\s)(?:export\s+)?(?:const|let|var)\s+\w+\s*=\s*(?:async\s+)?\(/, +]; + +function looksLikeFunction(line: string): boolean { + return FUNC_PATTERNS.some((re) => re.test(line)); +} + +/** Extract a function name from a signature line (best-effort; falls back to ``). */ +function extractFunctionName(line: string): string { + let m = /function\s+(\w+)/.exec(line); + if (m) return m[1]!; + if (/constructor\s*\(/.test(line)) return "constructor"; + m = + /(?:public|private|protected|static|abstract|override|async|readonly)(?:\s+(?:public|private|protected|static|abstract|override|async|readonly))*\s+(\w+)\s*[<(]/.exec( + line, + ); + if (m) return m[1]!; + m = /(?:const|let|var)\s+(\w+)\s*=/.exec(line); + if (m) return m[1]!; + return ""; +} + +/** Find the param list body (between the first balanced `(` and `)`) in concatenated signature + * lines. Returns null when parens are unbalanced — we skip rather than guess. */ +function extractParamListBody(text: string): string | null { + const open = text.indexOf("("); + if (open === -1) return null; + let depth = 0; + for (let i = open; i < text.length; i++) { + const c = text[i]!; + if (c === "(") depth++; + else if (c === ")") { + depth--; + if (depth === 0) return text.slice(open + 1, i); + } + } + return null; +} + +// --- Diff reconstruction --- + +interface ReconLine { + content: string; + isAdded: boolean; + newLine: number; +} + +/** Reconstruct the new-file view from a unified diff patch, tagging `+` lines as `isAdded`. */ +export function reconstructLines(patch: string): ReconLine[] { + const result: ReconLine[] = []; + let lineNum = 0; + for (const raw of patch.split("\n")) { + if (raw.startsWith("---") || raw.startsWith("+++")) continue; + const hunk = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(raw); + if (hunk) { + lineNum = Number(hunk[1]); + continue; + } + if (raw.startsWith("+")) { + result.push({ content: raw.slice(1), isAdded: true, newLine: lineNum++ }); + } else if (!raw.startsWith("-")) { + // context line (starts with space, or empty in some diff tools) + result.push({ content: raw.slice(1), isAdded: false, newLine: lineNum++ }); + } + } + return result; +} + +// --- Core scanning --- + +/** Scan one file's patch for doc-comment drift. + * + * Strategy: reconstruct the new-file view from hunk lines, then slide a window over + * JSDoc-to-function pairs. When any line in the window is a `+` diff line and the JSDoc + * has at least one @param, compare the tags to the signature's param list. */ +export function scanPatchForDocDrift( + path: string, + patch: string, +): DocCommentFinding[] { + const findings: DocCommentFinding[] = []; + const lines = reconstructLines(patch); + const n = lines.length; + let i = 0; + + while (i < n && findings.length < MAX_FINDINGS) { + const content = lines[i]!.content; + if (content.length > MAX_LINE_CHARS) { + i++; + continue; + } + + if (!content.trimStart().startsWith("/**")) { + i++; + continue; + } + + // Collect the JSDoc block through the line that contains `*/`. + const docStart = i; + let docEnd = i; + if (!content.includes("*/")) { + docEnd++; + while (docEnd < n && !lines[docEnd]!.content.includes("*/")) docEnd++; + } + const docBlock = lines + .slice(docStart, docEnd + 1) + .map((l) => l.content) + .join("\n"); + + // Skip blank lines between JSDoc and the function definition. + let fnIdx = docEnd + 1; + while (fnIdx < n && lines[fnIdx]!.content.trim() === "") fnIdx++; + + if (fnIdx >= n || !looksLikeFunction(lines[fnIdx]!.content)) { + i = docEnd + 1; + continue; + } + + const docParams = extractJsDocParams(docBlock); + if (docParams.length === 0) { + i = docEnd + 1; + continue; + } + + // Look across up to 8 lines to handle multi-line signatures. + const sigEnd = Math.min(fnIdx + 8, n); + const sigText = lines + .slice(fnIdx, sigEnd) + .map((l) => l.content) + .join(" "); + + const paramListBody = extractParamListBody(sigText); + if (paramListBody === null) { + // Can't parse the param list — skip rather than produce a false positive. + i = docEnd + 1; + continue; + } + + // Locate the line holding the closing paren of the param list so the change-gate window + // ends exactly at the signature and does not reach into the function body. + let sigCloseLineIdx = fnIdx; + { + let depth = 0; + let seenOpen = false; + outer: for (let li = fnIdx; li < sigEnd; li++) { + for (const ch of lines[li]!.content) { + if (ch === "(") { depth++; seenOpen = true; } + else if (ch === ")" && seenOpen) { + depth--; + if (depth === 0) { sigCloseLineIdx = li; break outer; } + } + } + } + } + + // Only flag when this PR actually touched the JSDoc-through-signature window. + // Bounded by the closing-paren line so body-only changes don't trigger drift checks. + const windowHasChange = lines + .slice(docStart, sigCloseLineIdx + 1) + .some((l) => l.isAdded); + if (!windowHasChange) { + i = docEnd + 1; + continue; + } + + const { params: fnParams, hasDestructured } = + extractFunctionParams(paramListBody); + if (hasDestructured) { + // JSDoc typically names the wrapper object (e.g. `@param options`), not the inner + // keys — a mismatch here is expected convention, not drift. + i = docEnd + 1; + continue; + } + + const fnName = extractFunctionName(lines[fnIdx]!.content); + const fnLine = lines[fnIdx]!.newLine; + const fnParamSet = new Set(fnParams); + const docParamSet = new Set(docParams); + + // Stale: @param in JSDoc for a param that no longer exists in the signature. + for (const dp of docParams) { + if (!fnParamSet.has(dp)) { + findings.push({ file: path, line: fnLine, fn: fnName, kind: "stale-param", param: dp }); + if (findings.length >= MAX_FINDINGS) return findings; + } + } + + // Missing: param in signature with no corresponding @param in the JSDoc. + for (const fp of fnParams) { + if (!docParamSet.has(fp)) { + findings.push({ file: path, line: fnLine, fn: fnName, kind: "missing-param", param: fp }); + if (findings.length >= MAX_FINDINGS) return findings; + } + } + + i = docEnd + 1; + } + + return findings; +} + +/** Analyzer entrypoint: scan every JS/TS file in the PR for doc-comment drift. */ +export async function scanDocComment( + req: EnrichRequest, +): Promise { + const findings: DocCommentFinding[] = []; + for (const file of req.files ?? []) { + if (!file.patch || !JS_TS_EXT.test(file.path)) continue; + for (const finding of scanPatchForDocDrift(file.path, file.patch)) { + findings.push(finding); + if (findings.length >= MAX_FINDINGS) return findings; + } + } + return findings; +} diff --git a/review-enrichment/src/brief.ts b/review-enrichment/src/brief.ts index ccb30f107b..ff963be319 100644 --- a/review-enrichment/src/brief.ts +++ b/review-enrichment/src/brief.ts @@ -14,13 +14,13 @@ import { scanInstallScripts } from "./analyzers/install-scripts.js"; import { scanActionPins } from "./analyzers/actions-pin.js"; import { scanEol } from "./analyzers/eol-check.js"; import { scanRedos } from "./analyzers/redos.js"; +import { scanDocComment } from "./analyzers/doc-comment.js"; import { scanCodeowners } from "./analyzers/codeowners.js"; import { scanSecretLog } from "./analyzers/secret-log.js"; import { renderBrief } from "./render.js"; type AnalyzerFn = (req: EnrichRequest, signal: AbortSignal) => Promise; -// The analyzer registry. More land behind this same shape: license (#1475), secret (#1476), static (#1477), history (#1478). const ANALYZERS: Record = { dependency: (req, signal) => scanDependencies(req, fetch, { signal }), secret: (req) => scanSecrets(req), @@ -29,6 +29,7 @@ const ANALYZERS: Record = { actionPin: (req) => scanActionPins(req), eol: (req) => scanEol(req), redos: (req) => scanRedos(req), + docComment: (req) => scanDocComment(req), codeowners: (req, signal) => scanCodeowners(req, fetch, { signal }), secretLog: (req, signal) => scanSecretLog(req, signal), }; diff --git a/review-enrichment/src/render.ts b/review-enrichment/src/render.ts index 5270f795e0..57cb8a4d60 100644 --- a/review-enrichment/src/render.ts +++ b/review-enrichment/src/render.ts @@ -129,6 +129,22 @@ export function renderBrief( } } + const docComments = findings.docComment ?? []; + if (docComments.length) { + lines.push( + "### Doc-comment drift (JSDoc @param mismatch — update the doc-comment before merging)", + ); + for (const item of docComments) { + const loc = safeCodeSpan(`${item.file}:${item.line}`); + const fn = safeCodeSpan(item.fn); + const msg = + item.kind === "stale-param" + ? `${safeCodeSpan(`@param ${item.param}`)} in JSDoc does not match any parameter of ${fn} — remove or rename` + : `${fn} has parameter ${safeCodeSpan(item.param)} with no @param in the JSDoc — add it`; + lines.push(`- ${loc} — ${msg}`); + } + } + const codeownersViolations = findings.codeowners ?? []; if (codeownersViolations.length) { const allOwners = new Set(codeownersViolations.flatMap((f) => f.owners)); diff --git a/review-enrichment/src/types.ts b/review-enrichment/src/types.ts index 0136d320af..4334cfb58d 100644 --- a/review-enrichment/src/types.ts +++ b/review-enrichment/src/types.ts @@ -93,6 +93,16 @@ export interface RedosFinding { pattern: string; } +/** A JSDoc/docstring whose @param tags don't match the adjacent function signature after the PR's changes — + * either a stale tag for a removed param, or a missing tag for an added param (#1519). */ +export interface DocCommentFinding { + file: string; + line: number; + fn: string; + kind: "stale-param" | "missing-param"; + param: string; +} + /** A changed file governed by a CODEOWNERS rule where the PR author is not listed as an owner (#1515). * The blast radius (distinct ownership domains crossed) is derived at render time from the full findings set. */ export interface CodeownersFinding { @@ -118,6 +128,7 @@ export interface BriefFindings { installScript?: InstallScriptFinding[]; eol?: EolFinding[]; redos?: RedosFinding[]; + docComment?: DocCommentFinding[]; codeowners?: CodeownersFinding[]; secretLog?: SecretLogFinding[]; } diff --git a/review-enrichment/test/enrichment.test.ts b/review-enrichment/test/enrichment.test.ts index 94c475f4d4..1695260d6f 100644 --- a/review-enrichment/test/enrichment.test.ts +++ b/review-enrichment/test/enrichment.test.ts @@ -21,6 +21,13 @@ import { scanPatchForRedos, scanRedos, } from "../dist/analyzers/redos.js"; +import { + extractJsDocParams, + extractFunctionParams, + reconstructLines, + scanPatchForDocDrift, + scanDocComment, +} from "../dist/analyzers/doc-comment.js"; import { findOwners, parseCodeowners, @@ -743,6 +750,371 @@ test("buildBrief: ReDoS analyzer runs (pure, no network)", async () => { } }); +// --- doc-comment drift tests --- + +test("extractJsDocParams: bare, typed, [bracket], [bracket=default] forms; skips `this`", () => { + const block = [ + "/**", + " * @param name bare", + " * @param {string} typed", + " * @param {Type} [optBracket] optional", + " * @param {number} [withDefault=0] with default", + " * @param this TS pseudo-param — should be skipped", + " */", + ].join("\n"); + assert.deepEqual(extractJsDocParams(block), [ + "name", + "typed", + "optBracket", + "withDefault", + ]); +}); + +test("extractJsDocParams: no @param tags returns empty array", () => { + assert.deepEqual(extractJsDocParams("/** Does something. */"), []); +}); + +test("extractFunctionParams: simple, optional, default, rest params; skips `this`", () => { + const { params, hasDestructured } = extractFunctionParams( + "a: string, b?: number, c = 0, ...rest: string[]", + ); + assert.deepEqual(params, ["a", "b", "c", "rest"]); + assert.equal(hasDestructured, false); +}); + +test("extractFunctionParams: destructured object param sets hasDestructured", () => { + const { params, hasDestructured } = extractFunctionParams( + "{ name, age }: Person, extra: string", + ); + assert.equal(hasDestructured, true); + assert.deepEqual(params, ["extra"]); +}); + +test("extractFunctionParams: destructured array param sets hasDestructured", () => { + const { hasDestructured } = extractFunctionParams("[first, ...rest]: string[]"); + assert.equal(hasDestructured, true); +}); + +test("extractFunctionParams: skips TS `this` pseudo-param", () => { + const { params } = extractFunctionParams("this: Context, x: string"); + assert.deepEqual(params, ["x"]); +}); + +test("extractFunctionParams: empty param list", () => { + const { params, hasDestructured } = extractFunctionParams(""); + assert.deepEqual(params, []); + assert.equal(hasDestructured, false); +}); + +test("reconstructLines: tags added lines, skips removed, uses hunk header for line numbers", () => { + const patch = [ + "@@ -5,3 +5,4 @@", + " context", + "+added", + "-removed", + " another", + ].join("\n"); + const lines = reconstructLines(patch); + assert.equal(lines.length, 3); + assert.deepEqual(lines[0], { content: "context", isAdded: false, newLine: 5 }); + assert.deepEqual(lines[1], { content: "added", isAdded: true, newLine: 6 }); + assert.deepEqual(lines[2], { content: "another", isAdded: false, newLine: 7 }); +}); + +test("reconstructLines: skips --- and +++ header lines", () => { + const patch = [ + "--- a/src/x.ts", + "+++ b/src/x.ts", + "@@ -1,0 +1,1 @@", + "+const x = 1;", + ].join("\n"); + const lines = reconstructLines(patch); + assert.equal(lines.length, 1); + assert.equal(lines[0]!.isAdded, true); + assert.equal(lines[0]!.newLine, 1); +}); + +test("reconstructLines: multiple hunks reset line counter", () => { + const patch = [ + "@@ -1,1 +1,1 @@", + "+first", + "@@ -10,1 +10,1 @@", + "+second", + ].join("\n"); + const lines = reconstructLines(patch); + assert.equal(lines[0]!.newLine, 1); + assert.equal(lines[1]!.newLine, 10); +}); + +test("scanPatchForDocDrift: stale-param — @param removed from function but still in JSDoc", () => { + const patch = [ + "@@ -1,7 +1,6 @@", + " /**", + " * @param name the name", + " * @param age the age", + " */", + "-function greet(name: string, age: number): string {", + "+function greet(name: string): string {", + ].join("\n"); + const findings = scanPatchForDocDrift("src/x.ts", patch); + assert.equal(findings.length, 1); + assert.equal(findings[0]!.kind, "stale-param"); + assert.equal(findings[0]!.param, "age"); + assert.equal(findings[0]!.fn, "greet"); + assert.equal(findings[0]!.file, "src/x.ts"); +}); + +test("scanPatchForDocDrift: missing-param — new param added to function with no @param in JSDoc", () => { + const patch = [ + "@@ -1,6 +1,7 @@", + " /**", + " * @param name the name", + " */", + "-function greet(name: string): string {", + "+function greet(name: string, email: string): string {", + ].join("\n"); + const findings = scanPatchForDocDrift("src/x.ts", patch); + assert.equal(findings.length, 1); + assert.equal(findings[0]!.kind, "missing-param"); + assert.equal(findings[0]!.param, "email"); +}); + +test("scanPatchForDocDrift: no change in JSDoc-function window — pre-existing drift is not flagged", () => { + // The JSDoc and function sig are both context lines; only body lines inside the function are added. + const patch = [ + "@@ -1,7 +1,8 @@", + " /**", + " * @param name the name", + " * @param stale gone", + " */", + " function greet(name: string): string {", + "+ return name;", + " }", + ].join("\n"); + const findings = scanPatchForDocDrift("src/x.ts", patch); + assert.equal(findings.length, 0); +}); + +test("scanPatchForDocDrift: JSDoc with no @param tags — nothing to drift against", () => { + const patch = [ + "@@ -1,5 +1,5 @@", + "+/**", + "+ * Does something.", + "+ */", + "+function doSomething(x: string): void {", + ].join("\n"); + assert.equal(scanPatchForDocDrift("src/x.ts", patch).length, 0); +}); + +test("scanPatchForDocDrift: destructured params — skipped to avoid wrapper-name false positives", () => { + const patch = [ + "@@ -1,5 +1,5 @@", + "+/**", + "+ * @param options the options", + "+ */", + "+function configure({ host, port }: Options): void {", + ].join("\n"); + assert.equal(scanPatchForDocDrift("src/x.ts", patch).length, 0); +}); + +test("scanPatchForDocDrift: JSDoc not followed by a function — skipped", () => { + const patch = [ + "@@ -1,5 +1,5 @@", + "+/**", + "+ * @param foo the foo", + "+ */", + "+const obj = { key: 'value' };", + ].join("\n"); + assert.equal(scanPatchForDocDrift("src/x.ts", patch).length, 0); +}); + +test("scanPatchForDocDrift: single-line JSDoc (/** ... */ on one line)", () => { + const patch = [ + "@@ -1,3 +1,3 @@", + " /** @param name the name @param stale gone */", + "-function greet(name: string, stale: number): void {", + "+function greet(name: string): void {", + ].join("\n"); + const findings = scanPatchForDocDrift("src/x.ts", patch); + assert.equal(findings.length, 1); + assert.equal(findings[0]!.kind, "stale-param"); + assert.equal(findings[0]!.param, "stale"); +}); + +test("scanPatchForDocDrift: class method with modifiers — name extracted correctly", () => { + const patch = [ + "@@ -1,7 +1,6 @@", + " /**", + " * @param id the id", + " * @param name the name", + " */", + "- public async update(id: string, name: string): Promise {", + "+ public async update(id: string): Promise {", + ].join("\n"); + const findings = scanPatchForDocDrift("src/svc.ts", patch); + assert.equal(findings.length, 1); + assert.equal(findings[0]!.fn, "update"); + assert.equal(findings[0]!.kind, "stale-param"); + assert.equal(findings[0]!.param, "name"); +}); + +test("scanPatchForDocDrift: arrow function — name extracted from const binding", () => { + const patch = [ + "@@ -1,6 +1,7 @@", + " /**", + " * @param a first", + " */", + "-const add = (a: number): number => a;", + "+const add = (a: number, b: number): number => a + b;", + ].join("\n"); + const findings = scanPatchForDocDrift("src/x.ts", patch); + assert.equal(findings.length, 1); + assert.equal(findings[0]!.fn, "add"); + assert.equal(findings[0]!.kind, "missing-param"); + assert.equal(findings[0]!.param, "b"); +}); + +test("scanPatchForDocDrift: rest param in function matches @param in JSDoc", () => { + const patch = [ + "@@ -1,5 +1,6 @@", + "+/**", + "+ * @param args the args", + "+ */", + "+function foo(...args: string[]): void {", + ].join("\n"); + // args is documented and present — no drift + assert.equal(scanPatchForDocDrift("src/x.ts", patch).length, 0); +}); + +test("scanPatchForDocDrift: unbalanced parens in signature — skipped", () => { + const patch = [ + "@@ -1,5 +1,5 @@", + "+/**", + "+ * @param x the x", + "+ */", + "+function broken(x: Map { + // Build a patch with 22 functions each with one stale param + const lines: string[] = ["@@ -1,1 +1,110 @@"]; + for (let k = 0; k < 22; k++) { + lines.push(`+/** @param stale gone */`); + lines.push(`+function fn${k}(real: string): void {}`); + } + const findings = scanPatchForDocDrift("src/x.ts", lines.join("\n")); + assert.equal(findings.length, 20); +}); + +test("scanPatchForDocDrift: change on a later line of a multi-line signature is detected", () => { + // Regression: windowHasChange must cover the full sig window, not just the first sig line. + // The JSDoc and first signature line are context; only a later param line is added. + const patch = [ + "@@ -1,6 +1,7 @@", + " /**", + " * @param a first param", + " */", + " function foo(", + "- a: string", + "+ a: string,", + "+ b: number", + " ) {}", + ].join("\n"); + const findings = scanPatchForDocDrift("src/x.ts", patch); + assert.equal(findings.length, 1); + assert.equal(findings[0]!.kind, "missing-param"); + assert.equal(findings[0]!.param, "b"); +}); + +test("scanDocComment: skips non-JS/TS files and files without patches", async () => { + const findings = await scanDocComment({ + repoFullName: "o/r", + prNumber: 1, + files: [ + { + path: "README.md", + patch: "@@ -1,0 +1,2 @@\n+/** @param foo */\n+function bar(baz: string) {}", + }, + { path: "src/a.ts" }, // no patch + { + path: "src/b.ts", + patch: [ + "@@ -1,5 +1,5 @@", + " /**", + " * @param name ok", + " * @param stale gone", + " */", + "+function f(name: string) {", + ].join("\n"), + }, + ], + }); + assert.equal(findings.length, 1); + assert.equal(findings[0]!.file, "src/b.ts"); +}); + +test("scanDocComment: caps MAX_FINDINGS across multiple files", async () => { + const files = Array.from({ length: 25 }, (_, k) => ({ + path: `src/f${k}.ts`, + patch: `@@ -1,0 +1,2 @@\n+/** @param stale gone */\n+function fn(real: string): void {}`, + })); + const findings = await scanDocComment({ repoFullName: "o/r", prNumber: 1, files }); + assert.equal(findings.length, 20); +}); + +test("renderBrief: renders stale-param and missing-param doc-comment blocks", () => { + const r = renderBrief({ + docComment: [ + { file: "src/x.ts", line: 10, fn: "greet", kind: "stale-param", param: "age" }, + { file: "src/y.ts", line: 20, fn: "configure", kind: "missing-param", param: "timeout" }, + ], + }); + assert.match(r.promptSection, /Doc-comment drift/); + assert.match(r.promptSection, /`src\/x\.ts:10`/); + assert.match(r.promptSection, /`@param age`/); + assert.match(r.promptSection, /does not match any parameter of `greet`/); + assert.match(r.promptSection, /`src\/y\.ts:20`/); + assert.match(r.promptSection, /`configure` has parameter `timeout`/); + assert.match(r.promptSection, /no @param in the JSDoc/); +}); + +test("renderBrief: no doc-comment section when docComment is empty", () => { + assert.equal(renderBrief({ docComment: [] }).promptSection, ""); +}); + +test("buildBrief: doc-comment analyzer runs (pure, no network)", async () => { + const realFetch = globalThis.fetch; + globalThis.fetch = async () => ({ ok: true, json: async () => ({}) }); + try { + const brief = await buildBrief({ + repoFullName: "o/r", + prNumber: 1, + files: [ + { + path: "src/x.ts", + patch: [ + "@@ -1,5 +1,5 @@", + " /**", + " * @param name the name", + " * @param staleParam gone", + " */", + "+function greet(name: string): string {", + ].join("\n"), + }, + ], + }); + assert.equal(brief.analyzerStatus.docComment, "ok"); + assert.equal(brief.findings.docComment!.length, 1); + assert.equal(brief.findings.docComment![0]!.kind, "stale-param"); + assert.match(brief.promptSection, /Doc-comment drift/); + } finally { + globalThis.fetch = realFetch; + } +}); + test("extractDependencyChanges: caps manifest files and patch lines", () => { const changes = extractDependencyChanges( [