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
9 changes: 5 additions & 4 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -67,23 +67,24 @@ GITTENSORY_REVIEW_ENRICHMENT=false
# 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,commitLint
# looseRange,terminology,todoMarker,magicNumber,conflictMarker,debugLeftover,commitLint
#
# Profile defaults:
# fast: dependency,lockfileDrift,secret,license,installScript,heavyDependency,actionPin,eol
# redos,provenance,secretLog,typosquat,iacMisconfig,nativeBuild,testRatio,migrationSafety
# looseRange,terminology,todoMarker,magicNumber,conflictMarker
# looseRange,terminology,todoMarker,magicNumber,conflictMarker,debugLeftover
# balanced (default): dependency,lockfileDrift,secret,license,installScript,heavyDependency
# 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,commitLint
# conflictMarker,debugLeftover,commitLint
# deep: dependency,lockfileDrift,secret,license,installScript,heavyDependency,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,commitLint
# migrationSafety,looseRange,terminology,todoMarker,magicNumber,conflictMarker,debugLeftover
# commitLint
# END GENERATED REES ANALYZERS

# Submitter-reputation spend control (internal-only): downgrades new/burst/low-rep
Expand Down
22 changes: 22 additions & 0 deletions apps/gittensory-ui/src/lib/rees-analyzers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -886,6 +886,28 @@ export const REES_ANALYZERS = [
"Structural: an exactly-seven-character marker run at column 0. The ambiguous `=======` separator is not flagged in Markdown/AsciiDoc files, where it is a legitimate section rule.",
},
},
{
name: "debugLeftover",
title: "Debug leftovers",
category: "quality",
cost: "local",
defaultEnabled: true,
profiles: ["fast", "balanced", "deep"],
requires: ["files"],
limits: {
maxFindings: 25,
maxLineChars: 2000,
},
docs: {
summary:
"Flags debugging leftovers a PR adds in non-test source — `debugger;`, bare console sinks, or `print()` calls.",
looksAt: "Added lines in changed non-test source files.",
reports: "File, line, and kind: debugger, console, or print.",
network: "Pure local analyzer. No external network call.",
notes:
"Distinct from the secrets-in-logs analyzer: this catches plain debug noise regardless of payload. String literals are stripped before matching.",
},
},
{
name: "commitLint",
title: "Conventional-commit subjects",
Expand Down
26 changes: 26 additions & 0 deletions review-enrichment/analyzer-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -999,6 +999,32 @@
"notes": "Structural: an exactly-seven-character marker run at column 0. The ambiguous `=======` separator is not flagged in Markdown/AsciiDoc files, where it is a legitimate section rule."
}
},
{
"name": "debugLeftover",
"title": "Debug leftovers",
"category": "quality",
"cost": "local",
"defaultEnabled": true,
"profiles": [
"fast",
"balanced",
"deep"
],
"requires": [
"files"
],
"limits": {
"maxFindings": 25,
"maxLineChars": 2000
},
"docs": {
"summary": "Flags debugging leftovers a PR adds in non-test source — `debugger;`, bare console sinks, or `print()` calls.",
"looksAt": "Added lines in changed non-test source files.",
"reports": "File, line, and kind: debugger, console, or print.",
"network": "Pure local analyzer. No external network call.",
"notes": "Distinct from the secrets-in-logs analyzer: this catches plain debug noise regardless of payload. String literals are stripped before matching."
}
},
{
"name": "commitLint",
"title": "Conventional-commit subjects",
Expand Down
90 changes: 90 additions & 0 deletions review-enrichment/src/analyzers/debug-leftover.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
// 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;
}
30 changes: 30 additions & 0 deletions review-enrichment/src/analyzers/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { scanMigrationSafety } from "./migration-safety.js";
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 { scanCommitLint } from "./commit-lint.js";
import { scanTerminology } from "./terminology.js";
import { scanTodoMarker } from "./todo-marker.js";
Expand Down Expand Up @@ -917,6 +918,35 @@ export const ANALYZER_DESCRIPTORS = [
},
run: (req) => scanConflictMarkers(req),
}),
descriptor({
name: "debugLeftover",
title: "Debug leftovers",
category: "quality",
cost: "local",
defaultEnabled: true,
requires: ["files"],
limits: { maxFindings: 25, maxLineChars: 2000 },
docs: {
summary:
"Flags debugging leftovers a PR adds in non-test source — `debugger;`, bare console sinks, or `print()` calls.",
looksAt: "Added lines in changed non-test source files.",
reports: "File, line, and kind: debugger, console, or print.",
network: "Pure local analyzer. No external network call.",
notes:
"Distinct from the secrets-in-logs analyzer: this catches plain debug noise regardless of payload. String literals are stripped before matching.",
},
render: (findings, helpers) => {
if (!findings.length) return [];
const lines = ["### Debug leftovers (debugger / console / print added by this PR)"];
for (const item of findings) {
lines.push(
`- ${helpers.safeCodeSpan(`${item.file}:${item.line}`)} — ${helpers.safeCodeSpan(item.kind)}`,
);
}
return lines;
},
run: (req, { signal }) => scanDebugLeftover(req, signal),
}),
descriptor({
name: "commitLint",
title: "Conventional-commit subjects",
Expand Down
1 change: 1 addition & 0 deletions review-enrichment/src/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,7 @@ export function renderBrief(
lines.push(...renderDescriptorSection("todoMarker", findings.todoMarker));
lines.push(...renderDescriptorSection("magicNumber", findings.magicNumber));
lines.push(...renderDescriptorSection("conflictMarker", findings.conflictMarker));
lines.push(...renderDescriptorSection("debugLeftover", findings.debugLeftover));
lines.push(...renderDescriptorSection("commitLint", findings.commitLint));

if (!lines.length) return { promptSection: "", systemSuffix: "" };
Expand Down
9 changes: 9 additions & 0 deletions review-enrichment/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,14 @@ export interface ConflictMarkerFinding {
marker: "<<<<<<<" | "|||||||" | "=======" | ">>>>>>>";
}

/** A debugging leftover a PR added in non-test source — `debugger;`, a bare console sink, or a `print()` call
* (#2015, part of #1499). Distinct from secret-log (sensitive payloads); reports location + kind only. */
export interface DebugLeftoverFinding {
file: string;
line: number;
kind: "debugger" | "console" | "print";
}

/** A PR commit subject that does not conform to the Conventional Commits spec (#2021, part of #1499). Reports a
* short SHA prefix, the subject, and the failing reason — never author/email. */
export interface CommitLintFinding {
Expand Down Expand Up @@ -510,6 +518,7 @@ export interface BriefFindings {
todoMarker?: TodoMarkerFinding[];
magicNumber?: MagicNumberFinding[];
conflictMarker?: ConflictMarkerFinding[];
debugLeftover?: DebugLeftoverFinding[];
commitLint?: CommitLintFinding[];
}

Expand Down
1 change: 1 addition & 0 deletions review-enrichment/test/analyzer-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ const EXPECTED_ANALYZERS = [
"todoMarker",
"magicNumber",
"conflictMarker",
"debugLeftover",
"commitLint",
];

Expand Down
95 changes: 95 additions & 0 deletions review-enrichment/test/debug-leftover.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
// Units for the debug-leftover analyzer (#2015). Own file (not enrichment.test.ts) so concurrent analyzer PRs
// don't collide. No network — pure, stateless per-line detection. Runs against the compiled dist/.
import { test } from "node:test";
import assert from "node:assert/strict";
import {
detectDebugLeftover,
scanDebugLeftover,
scanPatchForDebugLeftover,
} from "../dist/analyzers/debug-leftover.js";
import { renderBrief } from "../dist/render.js";

const patchOf = (lines: string[]) =>
`@@ -1,0 +1,${lines.length} @@\n${lines.map((l) => `+${l}`).join("\n")}`;

test("detectDebugLeftover: recognizes debugger, console sinks, and print()", () => {
assert.equal(detectDebugLeftover(" debugger;"), "debugger");
assert.equal(detectDebugLeftover("console.log('hi')"), "console");
assert.equal(detectDebugLeftover(" console.debug(state)"), "console");
assert.equal(detectDebugLeftover("print('debug')", "lib/b.py"), "print");
});

test("detectDebugLeftover: print() is Python-only and does not match method calls like document.print()", () => {
assert.equal(detectDebugLeftover("document.print()"), null);
assert.equal(detectDebugLeftover("printer.print('x')"), null);
assert.equal(detectDebugLeftover("print('debug')", "src/widget.ts"), null);
assert.equal(detectDebugLeftover("obj.print('x')", "pkg/widget.py"), null);
});

test("detectDebugLeftover: a console call inside a string literal is not flagged", () => {
assert.equal(detectDebugLeftover('const s = "console.log(\\"nope\\")"'), null);
assert.equal(detectDebugLeftover("log(`hint: console.log(here)`);"), null);
});

test("detectDebugLeftover: debugger inside a string is not flagged", () => {
assert.equal(detectDebugLeftover('const msg = "debugger;"'), null);
});

test("scanPatchForDebugLeftover: flags added lines with correct locations", () => {
const findings = scanPatchForDebugLeftover(
"src/widget.ts",
patchOf(["function f() {", " debugger;", " console.log('x');", " return g();", "}"]),
);
assert.deepEqual(findings, [
{ file: "src/widget.ts", line: 2, kind: "debugger" },
{ file: "src/widget.ts", line: 3, kind: "console" },
]);
});

test("scanPatchForDebugLeftover: only ADDED lines are scanned", () => {
const patch = [
"@@ -10,2 +10,2 @@",
" function f() {",
"- console.log('old');",
"+ print('new')",
].join("\n");
assert.deepEqual(scanPatchForDebugLeftover("pkg/widget.py", patch), [
{ file: "pkg/widget.py", line: 11, kind: "print" },
]);
});

test("scanPatchForDebugLeftover: skips test/spec files", () => {
assert.deepEqual(
scanPatchForDebugLeftover("src/widget.test.ts", patchOf(["console.log('in test')"])),
[],
);
assert.deepEqual(
scanPatchForDebugLeftover("tests/widget.spec.js", patchOf(["debugger;"])),
[],
);
});

test("scanPatchForDebugLeftover: respects the findings cap", () => {
const lines = Array.from({ length: 30 }, (_, i) => `console.log(${i});`);
assert.equal(scanPatchForDebugLeftover("src/a.ts", patchOf(lines), { maxFindings: 3 }).length, 3);
});

test("scanDebugLeftover: aggregates across files and renders in the brief", async () => {
const findings = await scanDebugLeftover({
files: [
{ path: "src/a.ts", patch: patchOf(["debugger;"]) },
{ path: "lib/b.py", patch: patchOf(["print('x')"]) },
],
});
assert.deepEqual(findings, [
{ file: "src/a.ts", line: 1, kind: "debugger" },
{ file: "lib/b.py", line: 1, kind: "print" },
]);

const { promptSection } = renderBrief({
debugLeftover: findings,
});
assert.match(promptSection, /Debug leftovers/);
assert.match(promptSection, /src\/a\.ts:1/);
assert.match(promptSection, /lib\/b\.py:1/);
});
Loading