From 753361f476000cd89be86ba0bd59015b0864780b Mon Sep 17 00:00:00 2001 From: bohdansolovie <153934212+bohdansolovie@users.noreply.github.com> Date: Sun, 5 Jul 2026 18:14:38 +0200 Subject: [PATCH] feat(enrichment): add deep-nesting control-flow analyzer Fixes #2030 Co-authored-by: Cursor --- .env.example | 9 +- apps/gittensory-ui/src/lib/rees-analyzers.ts | 23 +++ review-enrichment/analyzer-metadata.json | 27 ++++ .../src/analyzers/deep-nesting.ts | 138 ++++++++++++++++++ review-enrichment/src/analyzers/registry.ts | 30 ++++ review-enrichment/src/render.ts | 1 + review-enrichment/src/types.ts | 10 ++ .../test/analyzer-registry.test.ts | 1 + review-enrichment/test/deep-nesting.test.ts | 112 ++++++++++++++ src/review/enrichment-analyzer-names.ts | 1 + 10 files changed, 348 insertions(+), 4 deletions(-) create mode 100644 review-enrichment/src/analyzers/deep-nesting.ts create mode 100644 review-enrichment/test/deep-nesting.test.ts diff --git a/.env.example b/.env.example index 8d35aba374..c26935a04a 100644 --- a/.env.example +++ b/.env.example @@ -68,25 +68,26 @@ 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,sizeSmell,floatingPromise,commitLint +# conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting,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,sizeSmell,floatingPromise +# debugLeftover,sizeSmell,floatingPromise,deepNesting # 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,sizeSmell,floatingPromise,commitLint +# todoMarker,magicNumber,conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting +# 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,sizeSmell,floatingPromise,commitLint +# conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting,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 c81f2f350d..4f38016f1e 100644 --- a/apps/gittensory-ui/src/lib/rees-analyzers.ts +++ b/apps/gittensory-ui/src/lib/rees-analyzers.ts @@ -1004,6 +1004,29 @@ export const REES_ANALYZERS = [ "Precision-first: bare expression statements only — assignments and non-promise callees are skipped. Structural heuristic, not a type checker.", }, }, + { + name: "deepNesting", + title: "Deep control-flow nesting", + category: "quality", + cost: "local", + defaultEnabled: true, + profiles: ["fast", "balanced", "deep"], + requires: ["files"], + limits: { + maxFindings: 25, + maxDepth: 4, + maxLineChars: 2000, + }, + docs: { + summary: + "Flags newly-added control-flow blocks whose nesting depth exceeds a threshold inside a contiguous run of added lines.", + looksAt: "Added lines in changed non-test source files within each hunk.", + reports: "File, line, measured control-flow depth, and configured threshold.", + network: "Pure local analyzer. No external network call.", + notes: + "Counts braces opened by if/for/while/switch/try/catch/else/do/with, arrow bodies, and function bodies — not object-literal braces. Resets across context lines.", + }, + }, { name: "commitLint", title: "Conventional-commit subjects", diff --git a/review-enrichment/analyzer-metadata.json b/review-enrichment/analyzer-metadata.json index 76f7797b96..d06295b18e 100644 --- a/review-enrichment/analyzer-metadata.json +++ b/review-enrichment/analyzer-metadata.json @@ -1134,6 +1134,33 @@ "notes": "Precision-first: bare expression statements only — assignments and non-promise callees are skipped. Structural heuristic, not a type checker." } }, + { + "name": "deepNesting", + "title": "Deep control-flow nesting", + "category": "quality", + "cost": "local", + "defaultEnabled": true, + "profiles": [ + "fast", + "balanced", + "deep" + ], + "requires": [ + "files" + ], + "limits": { + "maxFindings": 25, + "maxDepth": 4, + "maxLineChars": 2000 + }, + "docs": { + "summary": "Flags newly-added control-flow blocks whose nesting depth exceeds a threshold inside a contiguous run of added lines.", + "looksAt": "Added lines in changed non-test source files within each hunk.", + "reports": "File, line, measured control-flow depth, and configured threshold.", + "network": "Pure local analyzer. No external network call.", + "notes": "Counts braces opened by if/for/while/switch/try/catch/else/do/with, arrow bodies, and function bodies — not object-literal braces. Resets across context lines." + } + }, { "name": "commitLint", "title": "Conventional-commit subjects", diff --git a/review-enrichment/src/analyzers/deep-nesting.ts b/review-enrichment/src/analyzers/deep-nesting.ts new file mode 100644 index 0000000000..efebedddb5 --- /dev/null +++ b/review-enrichment/src/analyzers/deep-nesting.ts @@ -0,0 +1,138 @@ +// Deep-nesting / arrow-anti-pattern analyzer (#2030). Flags newly-added control flow whose +// control-flow brace depth exceeds a threshold inside a contiguous run of added lines — a readability +// smell distinct from cyclomatic complexity. Object-literal braces are tracked but do not increase depth. +// Pure compute over added diff lines, no network. +import type { DeepNestingFinding, EnrichRequest } from "../types.js"; +import { codeOnly } from "./secret-log.js"; +import { isTestPath } from "./test-ratio.js"; + +export const DEFAULT_MAX_DEPTH = 4; +const MAX_FINDINGS = 25; +const MAX_LINE_CHARS = 2000; + +type BraceKind = "control" | "other"; + +/** True when `{` opens control-flow scope (if/for/try/=>/function), not an object literal. Pure. */ +export function isControlFlowOpenBrace(code: string, braceIdx: number): boolean { + const before = code.slice(0, braceIdx).trimEnd(); + if (/(?:=>|\belse|\btry|\bfinally|\bdo)\s*$/i.test(before)) return true; + if (/\b(?:if|for|while|switch|catch|with)\s*\([^)]*\)\s*$/i.test(before)) return true; + if (/\b(?:async\s+)?function(?:\s+\w+)?\s*\([^)]*\)\s*$/i.test(before)) return true; + return false; +} + +/** Advance control-flow brace depth over one code fragment and return ending depth + peak. Pure. */ +export function advanceControlFlowDepth( + code: string, + depth: number, +): { depth: number; peak: number } { + let peak = depth; + const stack: BraceKind[] = []; + + for (let i = 0; i < code.length; i++) { + const ch = code[i]!; + if (ch === "{") { + const kind: BraceKind = isControlFlowOpenBrace(code, i) ? "control" : "other"; + stack.push(kind); + if (kind === "control") { + depth++; + peak = Math.max(peak, depth); + } + continue; + } + if (ch === "}") { + const kind = stack.pop(); + if (kind === "control") { + depth = Math.max(0, depth - 1); + } + } + } + + return { depth, peak }; +} + +type ScanLimits = { + maxDepth?: number; + maxFindings?: number; + signal?: AbortSignal; +}; + +type RunState = { + depth: number; + flagged: boolean; +}; + +function resetRun(state: RunState): void { + state.depth = 0; + state.flagged = false; +} + +/** Scan one file patch's added lines for deep nesting, line-cited via hunk headers. Pure. */ +export function scanPatchForDeepNesting( + path: string, + patch: string, + limits: ScanLimits = {}, +): DeepNestingFinding[] { + const configured = limits.maxDepth ?? DEFAULT_MAX_DEPTH; + const maxDepth = configured > 0 ? configured : DEFAULT_MAX_DEPTH; + const maxFindings = limits.maxFindings ?? MAX_FINDINGS; + if (maxFindings <= 0 || isTestPath(path)) return []; + const findings: DeepNestingFinding[] = []; + const run: RunState = { depth: 0, flagged: false }; + let newLine = 0; + let inHunk = false; + + const maybeFlag = (line: number, depth: number) => { + if (run.flagged || depth <= maxDepth) return; + findings.push({ file: path, line, depth, threshold: maxDepth }); + run.flagged = true; + }; + + for (const line of patch.split("\n")) { + if (limits.signal?.aborted) throw new Error("analyzer_aborted"); + const hunk = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(line); + if (hunk) { + newLine = Number(hunk[1]); + inHunk = true; + resetRun(run); + continue; + } + if (!inHunk) continue; + if (line.startsWith("+")) { + const body = line.slice(1); + if (body.length <= MAX_LINE_CHARS) { + const next = advanceControlFlowDepth(codeOnly(body), run.depth); + run.depth = next.depth; + maybeFlag(newLine, next.peak); + if (findings.length >= maxFindings) return findings; + } + newLine++; + } else { + resetRun(run); + if (!line.startsWith("-") && !line.startsWith("\\")) { + newLine++; + } + } + } + return findings; +} + +/** Analyzer entrypoint: scan every changed non-test file's added lines for deep nesting. */ +export async function scanDeepNesting( + req: EnrichRequest, + signal?: AbortSignal, +): Promise { + const findings: DeepNestingFinding[] = []; + for (const file of req.files ?? []) { + if (signal?.aborted) throw new Error("analyzer_aborted"); + if (!file.patch) continue; + for (const finding of scanPatchForDeepNesting(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/analyzers/registry.ts b/review-enrichment/src/analyzers/registry.ts index 4cac2b05e3..a7206c8069 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 { scanDeepNesting } from "./deep-nesting.js"; import { scanFloatingPromise } from "./floating-promise.js"; import { scanSizeSmell } from "./size-smell.js"; import { scanCommitLint } from "./commit-lint.js"; @@ -1076,6 +1077,35 @@ export const ANALYZER_DESCRIPTORS = [ }, run: (req, { signal }) => scanFloatingPromise(req, signal), }), + descriptor({ + name: "deepNesting", + title: "Deep control-flow nesting", + category: "quality", + cost: "local", + defaultEnabled: true, + requires: ["files"], + limits: { maxFindings: 25, maxDepth: 4, maxLineChars: 2000 }, + docs: { + summary: + "Flags newly-added control-flow blocks whose nesting depth exceeds a threshold inside a contiguous run of added lines.", + looksAt: "Added lines in changed non-test source files within each hunk.", + reports: "File, line, measured control-flow depth, and configured threshold.", + network: "Pure local analyzer. No external network call.", + notes: + "Counts braces opened by if/for/while/switch/try/catch/else/do/with, arrow bodies, and function bodies — not object-literal braces. Resets across context lines.", + }, + render: (findings, helpers) => { + if (!findings.length) return []; + const lines = ["### Deep nesting (control-flow depth added by this PR)"]; + for (const item of findings) { + lines.push( + `- ${helpers.safeCodeSpan(`${item.file}:${item.line}`)} — depth ${item.depth} (threshold ${item.threshold})`, + ); + } + return lines; + }, + run: (req, { signal }) => scanDeepNesting(req, signal), + }), descriptor({ name: "commitLint", title: "Conventional-commit subjects", diff --git a/review-enrichment/src/render.ts b/review-enrichment/src/render.ts index 4185cc7c17..571acdca44 100644 --- a/review-enrichment/src/render.ts +++ b/review-enrichment/src/render.ts @@ -484,6 +484,7 @@ export function renderBrief( lines.push(...renderDescriptorSection("debugLeftover", findings.debugLeftover)); lines.push(...renderDescriptorSection("sizeSmell", findings.sizeSmell)); lines.push(...renderDescriptorSection("floatingPromise", findings.floatingPromise)); + lines.push(...renderDescriptorSection("deepNesting", findings.deepNesting)); 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 de18137576..4a75803e29 100644 --- a/review-enrichment/src/types.ts +++ b/review-enrichment/src/types.ts @@ -494,6 +494,15 @@ export interface SizeSmellFinding { name?: string; } +/** Deep control-flow nesting newly added in the diff (#2030, part of #1499). + * Reports file, line, measured depth, and threshold — never source content. */ +export interface DeepNestingFinding { + file: string; + line: number; + depth: number; + threshold: number; +} + /** A promise-shaped call added without await/return/void or a same-line .then/.catch chain (#2023, part of #1499). * Reports location and a truncated callee name — never full expressions. */ export interface FloatingPromiseFinding { @@ -560,6 +569,7 @@ export interface BriefFindings { debugLeftover?: DebugLeftoverFinding[]; sizeSmell?: SizeSmellFinding[]; floatingPromise?: FloatingPromiseFinding[]; + deepNesting?: DeepNestingFinding[]; hardcodedUrl?: HardcodedUrlFinding[]; commitLint?: CommitLintFinding[]; } diff --git a/review-enrichment/test/analyzer-registry.test.ts b/review-enrichment/test/analyzer-registry.test.ts index 64a000c8ff..9e6e7ca626 100644 --- a/review-enrichment/test/analyzer-registry.test.ts +++ b/review-enrichment/test/analyzer-registry.test.ts @@ -50,6 +50,7 @@ const EXPECTED_ANALYZERS = [ "debugLeftover", "sizeSmell", "floatingPromise", + "deepNesting", "commitLint", ]; diff --git a/review-enrichment/test/deep-nesting.test.ts b/review-enrichment/test/deep-nesting.test.ts new file mode 100644 index 0000000000..71de1a7943 --- /dev/null +++ b/review-enrichment/test/deep-nesting.test.ts @@ -0,0 +1,112 @@ +// Units for the deep-nesting analyzer (#2030). Own file so concurrent analyzer PRs don't collide. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + advanceControlFlowDepth, + DEFAULT_MAX_DEPTH, + isControlFlowOpenBrace, + scanDeepNesting, + scanPatchForDeepNesting, +} from "../dist/analyzers/deep-nesting.js"; +import { renderBrief } from "../dist/render.js"; + +const patchOf = (lines: string[]) => + `@@ -1,0 +1,${lines.length} @@\n${lines.map((l) => `+${l}`).join("\n")}`; + +test("isControlFlowOpenBrace: distinguishes control-flow from object literals", () => { + const ifLine = "if (a) {"; + assert.equal(isControlFlowOpenBrace(ifLine, ifLine.length - 1), true); + const objLine = "const cfg = {"; + assert.equal(isControlFlowOpenBrace(objLine, objLine.length - 1), false); + const arrowLine = "items.map(x => {"; + assert.equal(isControlFlowOpenBrace(arrowLine, arrowLine.length - 1), true); +}); + +test("advanceControlFlowDepth: tracks control-flow braces, not object literals", () => { + assert.deepEqual(advanceControlFlowDepth("", 0), { depth: 0, peak: 0 }); + assert.deepEqual(advanceControlFlowDepth("if (a) {", 0), { depth: 1, peak: 1 }); + assert.deepEqual(advanceControlFlowDepth("const cfg = { a: { b: {", 0), { depth: 0, peak: 0 }); + assert.deepEqual(advanceControlFlowDepth("if (a) { foo({ x: 1 }); }", 0), { depth: 0, peak: 1 }); +}); + +test("scanPatchForDeepNesting: flags a deeply nested added block", () => { + const lines = [ + "function run() {", + " if (a) {", + " if (b) {", + " if (c) {", + " if (d) {", + " return x;", + " }", + " }", + " }", + " }", + "}", + ]; + const findings = scanPatchForDeepNesting("src/widget.ts", patchOf(lines)); + assert.equal(findings.length, 1); + assert.equal(findings[0]?.depth, DEFAULT_MAX_DEPTH + 1); + assert.equal(findings[0]?.threshold, DEFAULT_MAX_DEPTH); +}); + +test("scanPatchForDeepNesting: does not flag deeply nested object literals", () => { + const lines = [ + "export const cfg = {", + " a: {", + " b: {", + " c: {", + " d: {", + " e: 1,", + " },", + " },", + " },", + " },", + "};", + ]; + assert.deepEqual(scanPatchForDeepNesting("src/config.ts", patchOf(lines)), []); +}); + +test("scanPatchForDeepNesting: does not flag shallow nesting at the threshold", () => { + const lines = ["function run() {", " if (a) {", " if (b) {", " return x;", " }", " }", "}"]; + assert.deepEqual(scanPatchForDeepNesting("src/widget.ts", patchOf(lines)), []); +}); + +test("scanPatchForDeepNesting: respects a custom maxDepth limit", () => { + const lines = ["if (a) {", " if (b) {", " return x;", " }", "}"]; + assert.deepEqual( + scanPatchForDeepNesting("src/widget.ts", patchOf(lines), { maxDepth: 1 }), + [{ file: "src/widget.ts", line: 2, depth: 2, threshold: 1 }], + ); + assert.deepEqual(scanPatchForDeepNesting("src/widget.ts", patchOf(lines), { maxDepth: 2 }), []); +}); + +test("scanPatchForDeepNesting: resets depth across context lines", () => { + const patch = [ + "@@ -1,3 +1,4 @@", + " function outer() {", + "+ if (a) {", + " return x;", + "+ if (b) { if (c) { if (d) { if (e) { if (f) { return y; } } } } }", + ].join("\n"); + assert.equal(scanPatchForDeepNesting("src/widget.ts", patch).length, 1); +}); + +test("scanPatchForDeepNesting: skips test files and respects the cap", () => { + const deepLine = "if (a) {".repeat(DEFAULT_MAX_DEPTH + 2); + assert.deepEqual(scanPatchForDeepNesting("src/widget.test.ts", patchOf([deepLine])), []); + const patch = Array.from({ length: 30 }, (_, i) => + [`@@ -${i},0 +${i + 1},1 @@`, `+${"if (x) {".repeat(DEFAULT_MAX_DEPTH + 2)}`].join("\n"), + ).join("\n"); + assert.equal(scanPatchForDeepNesting("src/a.ts", patch, { maxFindings: 2 }).length, 2); +}); + +test("scanDeepNesting: aggregates across files and renders a public-safe brief", async () => { + const deepBlock = ["if (a) {", " if (b) {", " if (c) {", " if (d) {", " if (e) {", " }", " }", " }", " }", "}"]; + const findings = await scanDeepNesting({ + files: [{ path: "src/a.ts", patch: patchOf(deepBlock) }], + }); + assert.equal(findings.length, 1); + const { promptSection } = renderBrief({ deepNesting: findings }); + assert.match(promptSection, /Deep nesting/); + assert.match(promptSection, /src\/a\.ts:/); +}); diff --git a/src/review/enrichment-analyzer-names.ts b/src/review/enrichment-analyzer-names.ts index 1853764172..36e44da514 100644 --- a/src/review/enrichment-analyzer-names.ts +++ b/src/review/enrichment-analyzer-names.ts @@ -44,6 +44,7 @@ export const REES_ANALYZER_NAMES = [ "debugLeftover", "sizeSmell", "floatingPromise", + "deepNesting", "commitLint", ] as const;