Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -68,32 +68,32 @@ 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,deepNesting,errorSwallow,unsafeAny,a11y
# i18n,unusedExport,exhaustiveness,flakyTest,commitLint,apiBreak,deprecatedDep,revertRecurrence
# coverageDelta,callerImpact
# conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting,errorSwallow,complexity
# unsafeAny,a11y,i18n,unusedExport,exhaustiveness,flakyTest,commitLint,apiBreak,deprecatedDep
# revertRecurrence,coverageDelta,callerImpact
#
# 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,deepNesting,errorSwallow,unsafeAny,a11y,i18n,apiBreak
# deprecatedDep
# debugLeftover,sizeSmell,floatingPromise,deepNesting,errorSwallow,complexity,unsafeAny,a11y
# i18n,apiBreak,deprecatedDep
# 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,deepNesting
# errorSwallow,unsafeAny,a11y,i18n,unusedExport,exhaustiveness,flakyTest,commitLint,apiBreak
# deprecatedDep,revertRecurrence,coverageDelta,callerImpact
# errorSwallow,complexity,unsafeAny,a11y,i18n,unusedExport,exhaustiveness,flakyTest,commitLint
# apiBreak,deprecatedDep,revertRecurrence,coverageDelta,callerImpact
# 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,deepNesting,errorSwallow,unsafeAny,a11y
# i18n,unusedExport,exhaustiveness,flakyTest,commitLint,apiBreak,deprecatedDep,revertRecurrence
# coverageDelta,callerImpact
# conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting,errorSwallow,complexity
# unsafeAny,a11y,i18n,unusedExport,exhaustiveness,flakyTest,commitLint,apiBreak,deprecatedDep
# revertRecurrence,coverageDelta,callerImpact
# END GENERATED REES ANALYZERS

# Submitter-reputation spend control (internal-only): downgrades new/burst/low-rep
Expand Down
33 changes: 29 additions & 4 deletions apps/gittensory-ui/src/lib/rees-analyzers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1041,12 +1041,37 @@ export const REES_ANALYZERS = [
},
docs: {
summary:
"Flags newly-added catch/except blocks that swallow the error — empty body, unused binding, or a bare `return null`.",
looksAt: "Added lines in changed non-test JS/TS/Python source files.",
reports: "File, line, and kind: empty-catch, unused-binding, or return-null.",
"Flags newly-added catch/except blocks (and Go if-err checks) that swallow or mishandle the error — empty body, unused binding, a bare `return null`/`nil`, or a Python bare `except:` naming no exception type.",
looksAt: "Added lines in changed non-test JS/TS/Python/Go source files.",
reports: "File, line, and kind: empty-catch, unused-binding, return-null, or bare-except.",
network: "Pure local analyzer. No external network call.",
notes:
"Multiline catch bodies are collected with brace balance. Catches that log, rethrow, or reference the binding are not flagged. Brace counting is character-level (string literals are not stripped).",
"Multiline catch/if-err bodies are collected with brace balance (Go's `if err != nil { … }`, including the if-with-initializer form, is treated the same as a JS/TS catch). Handlers that log, rethrow/panic, or reference the checked binding are not flagged. Python's bare `except:` is flagged regardless of body — it catches SystemExit/KeyboardInterrupt too. Brace counting is character-level (string literals are not stripped).",
},
},
{
name: "complexity",
title: "Approximate cyclomatic complexity",
category: "quality",
cost: "local",
defaultEnabled: true,
profiles: ["fast", "balanced", "deep"],
requires: ["files"],
limits: {
maxFindings: 25,
maxComplexity: 10,
maxLineChars: 2000,
},
docs: {
summary:
"Flags a newly-added function whose approximate cyclomatic complexity (branch/loop/logical-operator density, computed on the diff-visible lines) exceeds a threshold.",
looksAt:
"Added lines in changed non-test TS/JS source files, starting from a named function declaration or a const/let/var-assigned arrow function whose opening line is part of the diff.",
reports:
"File, line, the detected function name, the measured complexity, and the configured threshold.",
network: "Pure local analyzer. No external network call.",
notes:
"Diff-hunk approximation, not a whole-function true McCabe count: REES has no full-file content, so this counts if/for/while/case/catch/&&/||/?? token occurrences across the function's ADDED body lines only (1 + count), the same function-boundary detection size-smell.ts (#2019) uses for 'big-function'. A function whose signature line is not part of the diff is not scored. Distinct from deep-nesting (#2030), which measures brace NESTING depth, a readability smell, not decision-point density. Ternary (`? :`) is intentionally excluded — see the analyzer source header for why.",
},
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export const REES_ANALYZER_NAMES = [
"floatingPromise",
"deepNesting",
"errorSwallow",
"complexity",
"unsafeAny",
"a11y",
"i18n",
Expand Down
4 changes: 3 additions & 1 deletion review-enrichment/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
# Gittensory review-enrichment service (REES). Lean two-stage Node build; analyzers add CLI tools later (#1477).
# Gittensory review-enrichment service (REES). Lean two-stage Node build; analyzers are pure-JS diff-hunk
# heuristics with no external CLI tools (#1477 scoped this to the cheap, no-checkout path -- see
# review-enrichment/src/analyzers/complexity.ts's header for why a real linter/AST toolchain is out of scope).
# Build context = the review-enrichment/ directory (Railway "Root Directory" = review-enrichment).
FROM node:22-slim AS build
WORKDIR /app
Expand Down
35 changes: 31 additions & 4 deletions review-enrichment/analyzer-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -1180,11 +1180,38 @@
"maxLineChars": 2000
},
"docs": {
"summary": "Flags newly-added catch/except blocks that swallow the error — empty body, unused binding, or a bare `return null`.",
"looksAt": "Added lines in changed non-test JS/TS/Python source files.",
"reports": "File, line, and kind: empty-catch, unused-binding, or return-null.",
"summary": "Flags newly-added catch/except blocks (and Go if-err checks) that swallow or mishandle the error — empty body, unused binding, a bare `return null`/`nil`, or a Python bare `except:` naming no exception type.",
"looksAt": "Added lines in changed non-test JS/TS/Python/Go source files.",
"reports": "File, line, and kind: empty-catch, unused-binding, return-null, or bare-except.",
"network": "Pure local analyzer. No external network call.",
"notes": "Multiline catch bodies are collected with brace balance. Catches that log, rethrow, or reference the binding are not flagged. Brace counting is character-level (string literals are not stripped)."
"notes": "Multiline catch/if-err bodies are collected with brace balance (Go's `if err != nil { … }`, including the if-with-initializer form, is treated the same as a JS/TS catch). Handlers that log, rethrow/panic, or reference the checked binding are not flagged. Python's bare `except:` is flagged regardless of body — it catches SystemExit/KeyboardInterrupt too. Brace counting is character-level (string literals are not stripped)."
}
},
{
"name": "complexity",
"title": "Approximate cyclomatic complexity",
"category": "quality",
"cost": "local",
"defaultEnabled": true,
"profiles": [
"fast",
"balanced",
"deep"
],
"requires": [
"files"
],
"limits": {
"maxFindings": 25,
"maxComplexity": 10,
"maxLineChars": 2000
},
"docs": {
"summary": "Flags a newly-added function whose approximate cyclomatic complexity (branch/loop/logical-operator density, computed on the diff-visible lines) exceeds a threshold.",
"looksAt": "Added lines in changed non-test TS/JS source files, starting from a named function declaration or a const/let/var-assigned arrow function whose opening line is part of the diff.",
"reports": "File, line, the detected function name, the measured complexity, and the configured threshold.",
"network": "Pure local analyzer. No external network call.",
"notes": "Diff-hunk approximation, not a whole-function true McCabe count: REES has no full-file content, so this counts if/for/while/case/catch/&&/||/?? token occurrences across the function's ADDED body lines only (1 + count), the same function-boundary detection size-smell.ts (#2019) uses for 'big-function'. A function whose signature line is not part of the diff is not scored. Distinct from deep-nesting (#2030), which measures brace NESTING depth, a readability smell, not decision-point density. Ternary (`? :`) is intentionally excluded — see the analyzer source header for why."
}
},
{
Expand Down
203 changes: 203 additions & 0 deletions review-enrichment/src/analyzers/complexity.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,203 @@
// Approximate cyclomatic-complexity analyzer (#1477). REES has no full-file content -- only diff hunks -- so
// this is deliberately NOT a whole-function true McCabe count (that needs a real parser reading the ENTIRE
// function, including any part outside the diff, and a new AST-parser dependency this service does not carry).
// Instead it approximates: for each newly-added function whose OPENING line is visible in the diff (named
// `function` declarations and arrow functions assigned to const/let/var -- the same structural detection
// size-smell.ts (#2019) already uses for "big-function"), it counts branch/loop/logical-operator tokens across
// the function's ADDED body lines only and reports `1 + that count`, the standard McCabe formula computed on
// the visible slice. A function whose signature line is NOT part of the diff (only its body was edited) is not
// attributed a complexity score, the same accepted scope limit size-smell.ts already carries for "big-function".
//
// Distinct from deep-nesting.ts (#2030), which measures brace NESTING depth -- a readability smell that
// analyzer's own header explicitly disclaims as a complexity metric. This analyzer counts DECISION POINTS
// instead: a flat function (nesting depth 1) can still have high complexity from many sibling `if`/`&&` checks,
// and a deeply-nested function can have low complexity if each level has only one predicate. The two analyzers
// intentionally measure different axes of the same diff.
//
// Ternary (`? :`) is deliberately EXCLUDED from the decision-point count: distinguishing a conditional
// expression's `?` from TypeScript's optional-property/parameter marker (`foo?: T`) or optional chaining
// (`?.`) is not reliably decidable per-line by regex without a false-positive rate this precision-first
// heuristic rejects. if/for/while/case/catch/&&/||/?? are unambiguous token shapes that cover the bulk of
// realistic branching.
//
// Pure compute over added diff lines, no network, no new dependency. churn-hotspot (#1513) is not precedent for
// a broader one-time fetch here: it fetches commit METADATA that cannot exist in a diff in any form at all, so a
// fetch is its only option; complexity is partially approximable from the diff text itself, so the cheap
// in-hunk approximation -- not a full-file fetch -- is the right scope for this analyzer.
import type { ComplexityFinding, EnrichRequest } from "../types.js";
import { codeOnly } from "./secret-log.js";
import { isTestPath } from "./test-ratio.js";

export const DEFAULT_MAX_COMPLEXITY = 10;
const MAX_FINDINGS = 25;
const MAX_LINE_CHARS = 2000;

const JS_TS_PATH_RE = /\.(?:tsx?|jsx?|mts|cts|cjs|mjs)$/i;

const FUNCTION_OPEN_RE =
/\bfunction\s+(\w+)\s*\([^)]*\)\s*\{|\b(?:const|let|var)\s+(\w+)\s*=\s*(?:async\s*)?(?:function\s*)?\([^)]*\)\s*=>\s*\{/;

// Decision-point token classes, each counted as +1 branch. `if` also matches the "if" inside "else if" (correct:
// only the branch "else if" itself introduces should add 1; a bare "else" with no "if" adds 0, matching McCabe
// semantics). for/for-of/for-in/for-await, while (do-while is counted once via its trailing `while(...)`), a
// switch `case` label (never `default`, which is not an additional predicate), `catch`, and the `&&`/`||`/`??`
// short-circuit operators (each occurrence is its own branch). All patterns are flat (no group is itself
// quantified), so none can backtrack catastrophically.
const DECISION_RES: RegExp[] = [
/\bif\s*\(/g,
/\bfor\s*(?:await\s*)?\(/g,
/\bwhile\s*\(/g,
/\bcatch\s*[({]/g,
/\bcase\s+/g,
/&&/g,
/\|\|/g,
/\?\?/g,
];

function isJsTsPath(path: string): boolean {
return JS_TS_PATH_RE.test(path) && !isTestPath(path);
}

function isCommentLine(line: string): boolean {
const trimmed = line.trimStart();
return /^(?:\/\/|\/\*|\*)/.test(trimmed);
}

/** Count decision-point tokens (if/for/while/case/catch/&&/||/??) in one code fragment. Pure. */
export function countDecisionPoints(code: string): number {
let total = 0;
for (const re of DECISION_RES) {
const matches = code.match(re);
if (matches) total += matches.length;
}
return total;
}

/** The declared/assigned name when a line opens a named function declaration or an arrow function assigned to a
* const/let/var -- the same structural scope size-smell.ts's function detection uses. Pure. */
export function functionNameFromLine(line: string): string | undefined {
if (isCommentLine(line)) return undefined;
const match = FUNCTION_OPEN_RE.exec(codeOnly(line));
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 ScanLimits = {
maxComplexity?: number;
maxFindings?: number;
signal?: AbortSignal;
};

type PendingFunction = {
name: string;
startLine: number;
complexity: number;
depth: number;
};

/** Scan one file patch's added lines for a newly-added function whose approximate complexity exceeds a
* threshold, line-cited via hunk headers. Pure. */
export function scanPatchForComplexity(
path: string,
patch: string,
limits: ScanLimits = {},
): ComplexityFinding[] {
const configured = limits.maxComplexity ?? DEFAULT_MAX_COMPLEXITY;
const maxComplexity = configured > 0 ? configured : DEFAULT_MAX_COMPLEXITY;
const maxFindings = limits.maxFindings ?? MAX_FINDINGS;
if (maxFindings <= 0 || !isJsTsPath(path)) return [];

const findings: ComplexityFinding[] = [];
let newLine = 0;
let inHunk = false;
let pending: PendingFunction | null = null;

const flushFunction = () => {
if (!pending) return;
if (pending.complexity > maxComplexity) {
findings.push({
file: path,
line: pending.startLine,
name: pending.name,
complexity: pending.complexity,
threshold: maxComplexity,
});
}
pending = null;
};

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) {
flushFunction();
newLine = Number(hunk[1]);
inHunk = true;
continue;
}
if (!inHunk) continue;

if (line.startsWith("+")) {
const body = line.slice(1);
if (body.length <= MAX_LINE_CHARS) {
const commented = isCommentLine(body);
const code = codeOnly(body);
if (pending) {
if (!commented) pending.complexity += countDecisionPoints(code);
pending.depth += braceDepthDelta(code);
if (pending.depth <= 0) flushFunction();
} else {
const name = functionNameFromLine(body);
if (name) {
pending = {
name,
startLine: newLine,
complexity: 1 + (commented ? 0 : countDecisionPoints(code)),
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 every changed TS/JS file's added lines for high approximate complexity. */
export async function scanComplexity(
req: EnrichRequest,
signal?: AbortSignal,
): Promise<ComplexityFinding[]> {
const findings: ComplexityFinding[] = [];
for (const file of req.files ?? []) {
if (signal?.aborted) throw new Error("analyzer_aborted");
if (!file.patch) continue;
for (const finding of scanPatchForComplexity(file.path, file.patch, {
maxFindings: MAX_FINDINGS - findings.length,
signal,
})) {
findings.push(finding);
if (findings.length >= MAX_FINDINGS) return findings;
}
}
return findings;
}
Loading
Loading