From 866ad9c0ee3dea84140acded8c537918709d6f40 Mon Sep 17 00:00:00 2001 From: bohdansolovie Date: Sun, 5 Jul 2026 17:15:16 +0200 Subject: [PATCH] feat(enrichment): add long-file / big-function size-smell analyzer Estimate resulting file length from hunk headers and flag added function bodies whose brace span exceeds a threshold. Pure local compute over diffs. Co-authored-by: Cursor --- .env.example | 8 +- apps/gittensory-ui/src/lib/rees-analyzers.ts | 25 +++ review-enrichment/analyzer-metadata.json | 28 +++ review-enrichment/src/analyzers/registry.ts | 33 ++++ review-enrichment/src/analyzers/size-smell.ts | 166 ++++++++++++++++++ review-enrichment/src/render.ts | 1 + review-enrichment/src/types.ts | 12 ++ .../test/analyzer-registry.test.ts | 1 + review-enrichment/test/size-smell.test.ts | 79 +++++++++ src/review/enrichment-analyzer-names.ts | 1 + 10 files changed, 350 insertions(+), 4 deletions(-) create mode 100644 review-enrichment/src/analyzers/size-smell.ts create mode 100644 review-enrichment/test/size-smell.test.ts diff --git a/.env.example b/.env.example index f31e1c8a03..dc561a2300 100644 --- a/.env.example +++ b/.env.example @@ -68,25 +68,25 @@ GITTENSORY_REVIEW_ENRICHMENT=false # commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication,churnHotspot # blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene # pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber -# conflictMarker,debugLeftover,commitLint +# conflictMarker,debugLeftover,sizeSmell,commitLint # # Profile defaults: # fast: dependency,dependencyDiff,lockfileDrift,secret,license,installScript,heavyDependency # hardcodedUrl,actionPin,eol,redos,provenance,secretLog,typosquat,iacMisconfig,nativeBuild # testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber,conflictMarker -# debugLeftover +# debugLeftover,sizeSmell # balanced (default): dependency,dependencyDiff,lockfileDrift,secret,license,installScript # heavyDependency,hardcodedUrl,actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight # typosquat,commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication # churnHotspot,blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch # commitHygiene,pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology -# todoMarker,magicNumber,conflictMarker,debugLeftover,commitLint +# todoMarker,magicNumber,conflictMarker,debugLeftover,sizeSmell,commitLint # deep: dependency,dependencyDiff,lockfileDrift,secret,license,installScript,heavyDependency # hardcodedUrl,actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight,typosquat # commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication,churnHotspot # blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene # pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber -# conflictMarker,debugLeftover,commitLint +# conflictMarker,debugLeftover,sizeSmell,commitLint # END GENERATED REES ANALYZERS # Submitter-reputation spend control (internal-only): downgrades new/burst/low-rep diff --git a/apps/gittensory-ui/src/lib/rees-analyzers.ts b/apps/gittensory-ui/src/lib/rees-analyzers.ts index f7e6f4c13f..730e7c655f 100644 --- a/apps/gittensory-ui/src/lib/rees-analyzers.ts +++ b/apps/gittensory-ui/src/lib/rees-analyzers.ts @@ -956,6 +956,31 @@ export const REES_ANALYZERS = [ "Distinct from the secrets-in-logs analyzer: this catches plain debug noise regardless of payload. String literals are stripped before matching.", }, }, + { + name: "sizeSmell", + title: "Size smells", + category: "quality", + cost: "local", + defaultEnabled: true, + profiles: ["fast", "balanced", "deep"], + requires: ["files"], + limits: { + maxFindings: 25, + maxFileLines: 400, + maxFunctionLines: 60, + maxLineChars: 2000, + }, + docs: { + summary: + "Flags maintainability size smells from patch structure: an estimated resulting file length or an added function body span that exceeds configured thresholds.", + looksAt: "Unified-diff hunk headers and added non-test source lines.", + reports: + "File, kind (long-file or big-function), measured size, threshold, and optional function name.", + network: "Pure local analyzer. No external network call.", + notes: + "File length is estimated from hunk headers (the visible patch), not a full checkout. Function detection is structural (`function` / arrow-with-brace) and counts added body lines until brace balance returns to zero.", + }, + }, { name: "commitLint", title: "Conventional-commit subjects", diff --git a/review-enrichment/analyzer-metadata.json b/review-enrichment/analyzer-metadata.json index a07d7ddeda..27e07e1ed7 100644 --- a/review-enrichment/analyzer-metadata.json +++ b/review-enrichment/analyzer-metadata.json @@ -1079,6 +1079,34 @@ "notes": "Distinct from the secrets-in-logs analyzer: this catches plain debug noise regardless of payload. String literals are stripped before matching." } }, + { + "name": "sizeSmell", + "title": "Size smells", + "category": "quality", + "cost": "local", + "defaultEnabled": true, + "profiles": [ + "fast", + "balanced", + "deep" + ], + "requires": [ + "files" + ], + "limits": { + "maxFindings": 25, + "maxFileLines": 400, + "maxFunctionLines": 60, + "maxLineChars": 2000 + }, + "docs": { + "summary": "Flags maintainability size smells from patch structure: an estimated resulting file length or an added function body span that exceeds configured thresholds.", + "looksAt": "Unified-diff hunk headers and added non-test source lines.", + "reports": "File, kind (long-file or big-function), measured size, threshold, and optional function name.", + "network": "Pure local analyzer. No external network call.", + "notes": "File length is estimated from hunk headers (the visible patch), not a full checkout. Function detection is structural (`function` / arrow-with-brace) and counts added body lines until brace balance returns to zero." + } + }, { "name": "commitLint", "title": "Conventional-commit subjects", diff --git a/review-enrichment/src/analyzers/registry.ts b/review-enrichment/src/analyzers/registry.ts index f3896e17b9..914e52f4b6 100644 --- a/review-enrichment/src/analyzers/registry.ts +++ b/review-enrichment/src/analyzers/registry.ts @@ -32,6 +32,7 @@ import { scanLooseRanges } from "./loose-range.js"; import { scanMagicNumbers } from "./magic-number.js"; import { scanConflictMarkers } from "./conflict-marker.js"; import { scanDebugLeftover } from "./debug-leftover.js"; +import { scanSizeSmell } from "./size-smell.js"; import { scanCommitLint } from "./commit-lint.js"; import { scanTerminology } from "./terminology.js"; import { scanTodoMarker } from "./todo-marker.js"; @@ -1013,6 +1014,38 @@ export const ANALYZER_DESCRIPTORS = [ }, run: (req, { signal }) => scanDebugLeftover(req, signal), }), + descriptor({ + name: "sizeSmell", + title: "Size smells", + category: "quality", + cost: "local", + defaultEnabled: true, + requires: ["files"], + limits: { maxFindings: 25, maxFileLines: 400, maxFunctionLines: 60, maxLineChars: 2000 }, + docs: { + summary: + "Flags maintainability size smells from patch structure: an estimated resulting file length or an added function body span that exceeds configured thresholds.", + looksAt: "Unified-diff hunk headers and added non-test source lines.", + reports: "File, kind (long-file or big-function), measured size, threshold, and optional function name.", + network: "Pure local analyzer. No external network call.", + notes: + "File length is estimated from hunk headers (the visible patch), not a full checkout. Function detection is structural (`function` / arrow-with-brace) and counts added body lines until brace balance returns to zero.", + }, + render: (findings, helpers) => { + if (!findings.length) return []; + const lines = ["### Size smells (long file or big function added by this PR)"]; + for (const item of findings) { + const where = item.line ? `${item.file}:${item.line}` : item.file; + const label = + item.kind === "long-file" + ? `estimated ${item.measure} lines (threshold ${item.threshold})` + : `${helpers.safeCodeSpan(item.name ?? "function")} body ${item.measure} added lines (threshold ${item.threshold})`; + lines.push(`- ${helpers.safeCodeSpan(where)} — ${helpers.safeCodeSpan(item.kind)}: ${label}`); + } + return lines; + }, + run: (req, { signal }) => scanSizeSmell(req, signal), + }), descriptor({ name: "commitLint", title: "Conventional-commit subjects", diff --git a/review-enrichment/src/analyzers/size-smell.ts b/review-enrichment/src/analyzers/size-smell.ts new file mode 100644 index 0000000000..2ffb3926cc --- /dev/null +++ b/review-enrichment/src/analyzers/size-smell.ts @@ -0,0 +1,166 @@ +// Size-smell analyzer (#2019). Flags maintainability smells from patch structure alone: a changed file +// whose estimated resulting length exceeds a threshold, or a newly-added function body whose brace span is +// too long. Pure compute over diff hunks, no network. Skips test paths. +import type { EnrichRequest, SizeSmellFinding } from "../types.js"; +import { codeOnly } from "./secret-log.js"; +import { isTestPath } from "./test-ratio.js"; + +export const DEFAULT_MAX_FILE_LINES = 400; +export const DEFAULT_MAX_FUNCTION_LINES = 60; +const MAX_FINDINGS = 25; +const MAX_LINE_CHARS = 2000; + +const HUNK_RE = /^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@/; + +const FUNCTION_BODY_OPEN_RE = + /\bfunction\s+(\w+)\s*\([^)]*\)\s*\{|\b(?:const|let|var)\s+(\w+)\s*=\s*(?:async\s*)?(?:function\s*)?\([^)]*\)\s*=>\s*\{/; + +type ScanLimits = { + maxFileLines?: number; + maxFunctionLines?: number; + maxFindings?: number; + signal?: AbortSignal; +}; + +/** Estimate the resulting file length from unified-diff hunk headers. Pure. */ +export function estimateFileLengthFromPatch(patch: string): number { + let maxEndLine = 0; + for (const line of patch.split("\n")) { + const hunk = HUNK_RE.exec(line); + if (!hunk) continue; + const newStart = Number(hunk[3]); + const newLen = hunk[4] ? Number(hunk[4]) : 1; + maxEndLine = Math.max(maxEndLine, newStart + newLen - 1); + } + return maxEndLine; +} + +function functionNameFromLine(line: string): string | undefined { + const code = codeOnly(line); + const match = FUNCTION_BODY_OPEN_RE.exec(code); + if (!match) return undefined; + return match[1] ?? match[2]; +} + +function braceDepthDelta(code: string): number { + let depth = 0; + for (const ch of code) { + if (ch === "{") depth++; + else if (ch === "}") depth--; + } + return depth; +} + +type PendingFunction = { + name: string; + startLine: number; + bodyLines: number; + depth: number; +}; +export function scanPatchForSizeSmell( + path: string, + patch: string, + limits: ScanLimits = {}, +): SizeSmellFinding[] { + const maxFileLines = limits.maxFileLines ?? DEFAULT_MAX_FILE_LINES; + const maxFunctionLines = limits.maxFunctionLines ?? DEFAULT_MAX_FUNCTION_LINES; + const maxFindings = limits.maxFindings ?? MAX_FINDINGS; + if (maxFindings <= 0 || isTestPath(path)) return []; + + const findings: SizeSmellFinding[] = []; + const fileLength = estimateFileLengthFromPatch(patch); + if (fileLength > maxFileLines) { + findings.push({ + file: path, + kind: "long-file", + measure: fileLength, + threshold: maxFileLines, + }); + if (findings.length >= maxFindings) return findings; + } + + let newLine = 0; + let inHunk = false; + let pending: PendingFunction | null = null; + + const flushFunction = () => { + if (!pending) return; + if (pending.bodyLines > maxFunctionLines) { + findings.push({ + file: path, + line: pending.startLine, + kind: "big-function", + measure: pending.bodyLines, + threshold: maxFunctionLines, + name: pending.name, + }); + } + pending = null; + }; + + for (const line of patch.split("\n")) { + if (limits.signal?.aborted) throw new Error("analyzer_aborted"); + const hunk = HUNK_RE.exec(line); + if (hunk) { + flushFunction(); + newLine = Number(hunk[3]); + inHunk = true; + continue; + } + if (!inHunk) continue; + + if (line.startsWith("+")) { + const body = line.slice(1); + if (body.length <= MAX_LINE_CHARS) { + const code = codeOnly(body); + if (pending) { + pending.bodyLines++; + pending.depth += braceDepthDelta(code); + if (pending.depth <= 0) flushFunction(); + } else { + const name = functionNameFromLine(body); + if (name) { + pending = { + name, + startLine: newLine, + bodyLines: 1, + depth: braceDepthDelta(code), + }; + if (pending.depth <= 0) flushFunction(); + } + } + } + newLine++; + } else { + flushFunction(); + if (!line.startsWith("-") && !line.startsWith("\\")) { + newLine++; + } + } + + if (findings.length >= maxFindings) return findings; + } + + flushFunction(); + return findings; +} + +/** Analyzer entrypoint: scan changed non-test files for size smells. */ +export async function scanSizeSmell( + req: EnrichRequest, + signal?: AbortSignal, +): Promise { + const findings: SizeSmellFinding[] = []; + for (const file of req.files ?? []) { + if (signal?.aborted) throw new Error("analyzer_aborted"); + if (!file.patch) continue; + for (const finding of scanPatchForSizeSmell(file.path, file.patch, { + maxFindings: MAX_FINDINGS - findings.length, + signal, + })) { + findings.push(finding); + if (findings.length >= MAX_FINDINGS) return findings; + } + } + return findings; +} diff --git a/review-enrichment/src/render.ts b/review-enrichment/src/render.ts index b2a830feaf..08486ae5e5 100644 --- a/review-enrichment/src/render.ts +++ b/review-enrichment/src/render.ts @@ -482,6 +482,7 @@ export function renderBrief( lines.push(...renderDescriptorSection("magicNumber", findings.magicNumber)); lines.push(...renderDescriptorSection("conflictMarker", findings.conflictMarker)); lines.push(...renderDescriptorSection("debugLeftover", findings.debugLeftover)); + lines.push(...renderDescriptorSection("sizeSmell", findings.sizeSmell)); lines.push(...renderDescriptorSection("hardcodedUrl", findings.hardcodedUrl)); lines.push(...renderDescriptorSection("commitLint", findings.commitLint)); diff --git a/review-enrichment/src/types.ts b/review-enrichment/src/types.ts index bccb796a75..6a7af486f2 100644 --- a/review-enrichment/src/types.ts +++ b/review-enrichment/src/types.ts @@ -483,6 +483,17 @@ export interface DebugLeftoverFinding { kind: "debugger" | "console" | "print"; } +/** Maintainability size smell from patch structure (#2019, part of #1499). + * Reports estimated file length or added function body span — never source content. */ +export interface SizeSmellFinding { + file: string; + line?: number; + kind: "long-file" | "big-function"; + measure: number; + threshold: number; + name?: string; +} + /** An absolute HTTP(S) URL or raw IP:port endpoint hardcoded in non-test, non-config source (#2027, part of #1499). * Reports location, kind, and a redacted/truncated host — never full paths or query strings. */ export interface HardcodedUrlFinding { @@ -539,6 +550,7 @@ export interface BriefFindings { magicNumber?: MagicNumberFinding[]; conflictMarker?: ConflictMarkerFinding[]; debugLeftover?: DebugLeftoverFinding[]; + sizeSmell?: SizeSmellFinding[]; hardcodedUrl?: HardcodedUrlFinding[]; commitLint?: CommitLintFinding[]; } diff --git a/review-enrichment/test/analyzer-registry.test.ts b/review-enrichment/test/analyzer-registry.test.ts index c3d68528b7..3e4d06849f 100644 --- a/review-enrichment/test/analyzer-registry.test.ts +++ b/review-enrichment/test/analyzer-registry.test.ts @@ -48,6 +48,7 @@ const EXPECTED_ANALYZERS = [ "magicNumber", "conflictMarker", "debugLeftover", + "sizeSmell", "commitLint", ]; diff --git a/review-enrichment/test/size-smell.test.ts b/review-enrichment/test/size-smell.test.ts new file mode 100644 index 0000000000..aff6ee86f5 --- /dev/null +++ b/review-enrichment/test/size-smell.test.ts @@ -0,0 +1,79 @@ +// Units for the size-smell analyzer (#2019). Own file so concurrent analyzer PRs don't collide. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + DEFAULT_MAX_FILE_LINES, + DEFAULT_MAX_FUNCTION_LINES, + estimateFileLengthFromPatch, + scanPatchForSizeSmell, + scanSizeSmell, +} from "../dist/analyzers/size-smell.js"; +import { renderBrief } from "../dist/render.js"; + +const patchOf = (lines: string[]) => + `@@ -1,0 +1,${lines.length} @@\n${lines.map((l) => `+${l}`).join("\n")}`; + +test("estimateFileLengthFromPatch: reads ending line from hunk headers", () => { + const patch = ["@@ -0,0 +1,3 @@", "+a", "+b", "+c"].join("\n"); + assert.equal(estimateFileLengthFromPatch(patch), 3); + assert.equal( + estimateFileLengthFromPatch(["@@ -10,5 +10,8 @@", "+x"].join("\n")), + 17, + ); +}); + +test("scanPatchForSizeSmell: flags a long resulting file", () => { + const patch = `@@ -0,0 +1,${DEFAULT_MAX_FILE_LINES + 1} @@\n${"+line\n".repeat(DEFAULT_MAX_FILE_LINES + 1)}`; + assert.deepEqual(scanPatchForSizeSmell("src/big.ts", patch), [ + { + file: "src/big.ts", + kind: "long-file", + measure: DEFAULT_MAX_FILE_LINES + 1, + threshold: DEFAULT_MAX_FILE_LINES, + }, + ]); +}); + +test("scanPatchForSizeSmell: flags a big function body added across lines", () => { + const header = ["function big() {"]; + const body = Array.from({ length: DEFAULT_MAX_FUNCTION_LINES }, (_, i) => ` step${i}();`); + const footer = ["}"]; + const findings = scanPatchForSizeSmell("src/widget.ts", patchOf([...header, ...body, ...footer])); + assert.equal(findings.length, 1); + assert.equal(findings[0]?.kind, "big-function"); + assert.equal(findings[0]?.name, "big"); + assert.equal(findings[0]?.measure, DEFAULT_MAX_FUNCTION_LINES + 2); +}); + +test("scanPatchForSizeSmell: clean sub-threshold file and function", () => { + const lines = ["function small() {", " return 1;", "}"]; + assert.deepEqual(scanPatchForSizeSmell("src/widget.ts", patchOf(lines)), []); +}); + +test("scanPatchForSizeSmell: skips test files and respects the cap", () => { + const longPatch = `@@ -0,0 +1,${DEFAULT_MAX_FILE_LINES + 1} @@\n${"+line\n".repeat(DEFAULT_MAX_FILE_LINES + 1)}`; + assert.deepEqual(scanPatchForSizeSmell("src/widget.test.ts", longPatch), []); +}); + +test("scanSizeSmell: respects the findings cap across files", async () => { + const longPatch = `@@ -0,0 +1,${DEFAULT_MAX_FILE_LINES + 1} @@\n${"+line\n".repeat(DEFAULT_MAX_FILE_LINES + 1)}`; + const findings = await scanSizeSmell({ + files: Array.from({ length: 30 }, (_, i) => ({ path: `src/f${i}.ts`, patch: longPatch })), + }); + assert.equal(findings.length, 25); +}); + +test("scanSizeSmell: renders a public-safe brief", async () => { + const findings = await scanSizeSmell({ + files: [{ path: "src/a.ts", patch: patchOf(["function small() {", " return 1;", "}"]) }], + }); + assert.deepEqual(findings, []); + const longPatch = `@@ -0,0 +1,${DEFAULT_MAX_FILE_LINES + 5} @@\n${"+x\n".repeat(DEFAULT_MAX_FILE_LINES + 5)}`; + const longFindings = await scanSizeSmell({ + files: [{ path: "src/b.ts", patch: longPatch }], + }); + assert.equal(longFindings[0]?.kind, "long-file"); + const { promptSection } = renderBrief({ sizeSmell: longFindings }); + assert.match(promptSection, /Size smells/); + assert.match(promptSection, /src\/b\.ts/); +}); diff --git a/src/review/enrichment-analyzer-names.ts b/src/review/enrichment-analyzer-names.ts index 021296f400..00abaa4af1 100644 --- a/src/review/enrichment-analyzer-names.ts +++ b/src/review/enrichment-analyzer-names.ts @@ -42,6 +42,7 @@ export const REES_ANALYZER_NAMES = [ "magicNumber", "conflictMarker", "debugLeftover", + "sizeSmell", "commitLint", ] as const;