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
5 changes: 3 additions & 2 deletions review-enrichment/src/analyzers/a11y-regression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@
// positive tabindex values. Pure compute over added lines in .jsx/.tsx/.html/.vue files, no network.
import type { A11yFinding, EnrichRequest } from "../types.js";
import { isTestPath } from "./test-ratio.js";
import { DEFAULT_MAX_FINDINGS, DEFAULT_MAX_LINE_CHARS } from "./limits.js";

const MAX_FINDINGS = 25;
const MAX_LINE_CHARS = 2000;
const MAX_FINDINGS = DEFAULT_MAX_FINDINGS;
const MAX_LINE_CHARS = DEFAULT_MAX_LINE_CHARS;

const MARKUP_PATH_RE = /\.(?:tsx|jsx|html|vue)$/i;

Expand Down
3 changes: 2 additions & 1 deletion review-enrichment/src/analyzers/api-break.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@
// exact whole-name loss is; a non-entrypoint file is out of scope (that is the caller-impact analyzer's job).
// Deterministic, no network, no token. Reports file, old-file line, and symbol only — never surrounding code.
import type { ApiBreakFinding, EnrichRequest } from "../types.js";
import { DEFAULT_MAX_FINDINGS } from "./limits.js";

const MAX_ENTRYPOINTS = 25; // cap changed entrypoint files scanned per PR
const MAX_FINDINGS = 25; // keep the brief bounded
const MAX_FINDINGS = DEFAULT_MAX_FINDINGS;

// Files whose top-level exports form a package's PUBLIC surface: barrel/entry modules only. Restricting to these
// entrypoint basenames keeps the signal conservative — a removed export in an internal module is not a downstream
Expand Down
3 changes: 2 additions & 1 deletion review-enrichment/src/analyzers/caller-impact.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import type { AnalysisContext } from "../analysis-context.js";
import { boundedFetchJson } from "../external-fetch.js";
import { exportedNames, isPublicEntrypoint } from "./api-break.js";
import { isTestPath } from "./test-ratio.js";
import { DEFAULT_MAX_FINDINGS } from "./limits.js";

const GITHUB_API = "https://api.github.com";
const GITHUB_API_VERSION = "2022-11-28";
Expand All @@ -37,7 +38,7 @@ const MAX_SYMBOLS = 6; // removed symbols searched per PR (Code Search rate budg
const MAX_SEARCHES = 6; // bounded Code Search queries per PR
const MAX_FILE_FETCHES = 12; // bounded candidate-caller content fetches per PR
const MAX_CALLERS_PER_FINDING = 5; // caller paths listed per finding (keeps the brief bounded)
const MAX_FINDINGS = 25;
const MAX_FINDINGS = DEFAULT_MAX_FINDINGS;
const MIN_SYMBOL_LEN = 3; // skip 1-2 char names — too generic to search reliably
const MAX_FETCH_BYTES = 1_000_000;
const MAX_SEARCH_JSON_BYTES = 256 * 1024;
Expand Down
3 changes: 2 additions & 1 deletion review-enrichment/src/analyzers/commit-hygiene.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@ import type {
} from "../types.js";
import type { AnalysisContext } from "../analysis-context.js";
import { boundedFetchJson } from "../external-fetch.js";
import { DEFAULT_MAX_FINDINGS } from "./limits.js";

const GITHUB_API = "https://api.github.com";
const SLUG_RE = /^[A-Za-z0-9._-]+$/;
const MAX_COMMITS = 100;
const MAX_FINDINGS = 25;
const MAX_FINDINGS = DEFAULT_MAX_FINDINGS;
const SHA_PREFIX_LEN = 12;

// Git's own autosquash markers (`git commit --fixup`/`--squash`, `git rebase -i --autosquash`) — a documented,
Expand Down
3 changes: 2 additions & 1 deletion review-enrichment/src/analyzers/commit-lint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,12 @@ import type {
} from "../types.js";
import type { AnalysisContext } from "../analysis-context.js";
import { boundedFetchJson } from "../external-fetch.js";
import { DEFAULT_MAX_FINDINGS } from "./limits.js";

const GITHUB_API = "https://api.github.com";
const SLUG_RE = /^[A-Za-z0-9._-]+$/;
const MAX_COMMITS = 100;
const MAX_FINDINGS = 25;
const MAX_FINDINGS = DEFAULT_MAX_FINDINGS;
const SHA_PREFIX_LEN = 12;
const MAX_SUBJECT_LEN = 72; // Conventional Commits / git convention soft cap for the subject line.

Expand Down
51 changes: 33 additions & 18 deletions review-enrichment/src/analyzers/complexity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,11 @@
import type { ComplexityFinding, EnrichRequest } from "../types.js";
import { codeOnly } from "./secret-log.js";
import { isTestPath } from "./test-ratio.js";
import { DEFAULT_MAX_FINDINGS, DEFAULT_MAX_LINE_CHARS } from "./limits.js";

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

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

Expand Down Expand Up @@ -90,6 +91,34 @@ function braceDepthDelta(code: string): number {
return depth;
}

/** Given the current pending function (or none) and one added code line, return the updated pending
* state -- mutated in place and returned when tracking continues, freshly started when the line opens
* a new function, or null when neither applies. Extracted out of scanPatchForComplexity's own loop
* (rather than left as an inline if/else) to keep that loop's own control-flow nesting under this
* analyzer's sibling deep-nesting.ts threshold. Pure; deciding whether to flush (pending.depth <= 0)
* stays the caller's job so this has no dependency on flushFunction's closure. */
function advancePendingFunction(
pending: PendingFunction | null,
body: string,
commented: boolean,
code: string,
newLine: number,
): PendingFunction | null {
if (pending) {
if (!commented) pending.complexity += countDecisionPoints(code);
pending.depth += braceDepthDelta(code);
return pending;
}
const name = functionNameFromLine(body);
if (!name) return null;
return {
name,
startLine: newLine,
complexity: 1 + (commented ? 0 : countDecisionPoints(code)),
depth: braceDepthDelta(code),
};
}

type ScanLimits = {
maxComplexity?: number;
maxFindings?: number;
Expand Down Expand Up @@ -150,22 +179,8 @@ export function scanPatchForComplexity(
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();
}
}
pending = advancePendingFunction(pending, body, commented, code, newLine);
if (pending && pending.depth <= 0) flushFunction();
}
newLine++;
} else {
Expand Down
3 changes: 2 additions & 1 deletion review-enrichment/src/analyzers/conflict-marker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
// Detection is purely structural (a fixed run of seven identical characters at column 0), so there is no
// comment/string state to track. Line-cited via hunk headers, mirroring the sibling local analyzers.
import type { EnrichRequest, ConflictMarkerFinding } from "../types.js";
import { DEFAULT_MAX_FINDINGS } from "./limits.js";

const MAX_FINDINGS = 25;
const MAX_FINDINGS = DEFAULT_MAX_FINDINGS;

// Git writes each conflict marker as EXACTLY seven identical characters at the start of the line. The ours/base/
// theirs markers may carry a trailing space + label (a branch or commit); the separator is a bare seven `=`.
Expand Down
181 changes: 91 additions & 90 deletions review-enrichment/src/analyzers/debug-leftover.ts
Original file line number Diff line number Diff line change
@@ -1,90 +1,91 @@
// Debug-leftover analyzer (#2015). Flags debugging leftovers introduced in the diff — `debugger;` statements
// and bare `console.*` / `print()` calls added to non-test source files. Distinct from the secret-log analyzer
// (which only fires on sensitive-value sinks); this catches plain debug noise regardless of payload. Pure compute,
// no network. String-literal content is stripped before matching so a `"console.log('hi')"` inside a string is
// not flagged. Line-cited via hunk headers, mirroring the sibling local analyzers.
import type { DebugLeftoverFinding, EnrichRequest } from "../types.js";
import { codeOnly } from "./secret-log.js";
import { isTestPath } from "./test-ratio.js";

const MAX_FINDINGS = 25;
const MAX_LINE_CHARS = 2000;

const DEBUGGER_RE = /\bdebugger\s*;/;
const CONSOLE_RE = /\bconsole\s*\.\s*(?:log|debug|info|warn|error|trace|dir|table)\s*\(/;
const PRINT_RE = /(?<![\w.])print\s*\(/;

/** Classify one added line for a debug leftover, or null. Pure. */
export function detectDebugLeftover(
line: string,
path?: string,
): DebugLeftoverFinding["kind"] | null {
const code = codeOnly(line);
if (DEBUGGER_RE.test(code)) return "debugger";
if (CONSOLE_RE.test(code)) return "console";
// Python-only: `\bprint` after a dot would false-positive on `document.print()` / `obj.print()`.
if (path && /\.pyi?$/i.test(path) && PRINT_RE.test(code)) return "print";
return null;
}

type ScanLimits = {
maxFindings?: number;
signal?: AbortSignal;
};

/** Scan one file patch's added lines for debug leftovers, line-cited via hunk headers. Pure. */
export function scanPatchForDebugLeftover(
path: string,
patch: string,
limits: ScanLimits = {},
): DebugLeftoverFinding[] {
const maxFindings = limits.maxFindings ?? MAX_FINDINGS;
if (maxFindings <= 0 || isTestPath(path)) return [];
const findings: DebugLeftoverFinding[] = [];
let newLine = 0;
let inHunk = false;
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;
continue;
}
if (!inHunk) continue;
if (line.startsWith("+")) {
const body = line.slice(1);
if (body.length <= MAX_LINE_CHARS) {
const kind = detectDebugLeftover(body, path);
if (kind) {
findings.push({ file: path, line: newLine, kind });
if (findings.length >= maxFindings) return findings;
}
}
newLine++;
} else if (!line.startsWith("-") && !line.startsWith("\\")) {
newLine++;
}
}
return findings;
}

/** Analyzer entrypoint: scan every changed non-test file's added lines for debug leftovers. */
export async function scanDebugLeftover(
req: EnrichRequest,
signal?: AbortSignal,
): Promise<DebugLeftoverFinding[]> {
const findings: DebugLeftoverFinding[] = [];
for (const file of req.files ?? []) {
if (signal?.aborted) throw new Error("analyzer_aborted");
if (!file.patch) continue;
for (const finding of scanPatchForDebugLeftover(file.path, file.patch, {
maxFindings: MAX_FINDINGS - findings.length,
signal,
})) {
findings.push(finding);
if (findings.length >= MAX_FINDINGS) return findings;
}
}
return findings;
}
// Debug-leftover analyzer (#2015). Flags debugging leftovers introduced in the diff — `debugger;` statements
// and bare `console.*` / `print()` calls added to non-test source files. Distinct from the secret-log analyzer
// (which only fires on sensitive-value sinks); this catches plain debug noise regardless of payload. Pure compute,
// no network. String-literal content is stripped before matching so a `"console.log('hi')"` inside a string is
// not flagged. Line-cited via hunk headers, mirroring the sibling local analyzers.
import type { DebugLeftoverFinding, EnrichRequest } from "../types.js";
import { codeOnly } from "./secret-log.js";
import { isTestPath } from "./test-ratio.js";
import { DEFAULT_MAX_FINDINGS, DEFAULT_MAX_LINE_CHARS } from "./limits.js";

const MAX_FINDINGS = DEFAULT_MAX_FINDINGS;
const MAX_LINE_CHARS = DEFAULT_MAX_LINE_CHARS;

const DEBUGGER_RE = /\bdebugger\s*;/;
const CONSOLE_RE = /\bconsole\s*\.\s*(?:log|debug|info|warn|error|trace|dir|table)\s*\(/;
const PRINT_RE = /(?<![\w.])print\s*\(/;

/** Classify one added line for a debug leftover, or null. Pure. */
export function detectDebugLeftover(
line: string,
path?: string,
): DebugLeftoverFinding["kind"] | null {
const code = codeOnly(line);
if (DEBUGGER_RE.test(code)) return "debugger";
if (CONSOLE_RE.test(code)) return "console";
// Python-only: `\bprint` after a dot would false-positive on `document.print()` / `obj.print()`.
if (path && /\.pyi?$/i.test(path) && PRINT_RE.test(code)) return "print";
return null;
}

type ScanLimits = {
maxFindings?: number;
signal?: AbortSignal;
};

/** Scan one file patch's added lines for debug leftovers, line-cited via hunk headers. Pure. */
export function scanPatchForDebugLeftover(
path: string,
patch: string,
limits: ScanLimits = {},
): DebugLeftoverFinding[] {
const maxFindings = limits.maxFindings ?? MAX_FINDINGS;
if (maxFindings <= 0 || isTestPath(path)) return [];
const findings: DebugLeftoverFinding[] = [];
let newLine = 0;
let inHunk = false;
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;
continue;
}
if (!inHunk) continue;
if (line.startsWith("+")) {
const body = line.slice(1);
if (body.length <= MAX_LINE_CHARS) {
const kind = detectDebugLeftover(body, path);
if (kind) {
findings.push({ file: path, line: newLine, kind });
if (findings.length >= maxFindings) return findings;
}
}
newLine++;
} else if (!line.startsWith("-") && !line.startsWith("\\")) {
newLine++;
}
}
return findings;
}

/** Analyzer entrypoint: scan every changed non-test file's added lines for debug leftovers. */
export async function scanDebugLeftover(
req: EnrichRequest,
signal?: AbortSignal,
): Promise<DebugLeftoverFinding[]> {
const findings: DebugLeftoverFinding[] = [];
for (const file of req.files ?? []) {
if (signal?.aborted) throw new Error("analyzer_aborted");
if (!file.patch) continue;
for (const finding of scanPatchForDebugLeftover(file.path, file.patch, {
maxFindings: MAX_FINDINGS - findings.length,
signal,
})) {
findings.push(finding);
if (findings.length >= MAX_FINDINGS) return findings;
}
}
return findings;
}
5 changes: 3 additions & 2 deletions review-enrichment/src/analyzers/deep-nesting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
import type { DeepNestingFinding, EnrichRequest } from "../types.js";
import { codeOnly } from "./secret-log.js";
import { isTestPath } from "./test-ratio.js";
import { DEFAULT_MAX_FINDINGS, DEFAULT_MAX_LINE_CHARS } from "./limits.js";

export const DEFAULT_MAX_DEPTH = 4;
const MAX_FINDINGS = 25;
const MAX_LINE_CHARS = 2000;
const MAX_FINDINGS = DEFAULT_MAX_FINDINGS;
const MAX_LINE_CHARS = DEFAULT_MAX_LINE_CHARS;

type BraceKind = "control" | "other";

Expand Down
Loading