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
8 changes: 4 additions & 4 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -69,27 +69,27 @@ GITTENSORY_REVIEW_ENRICHMENT=false
# 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
# i18n,unusedExport,exhaustiveness,flakyTest,commitLint,apiBreak
#
# 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
# debugLeftover,sizeSmell,floatingPromise,deepNesting,errorSwallow,unsafeAny,a11y,i18n,apiBreak
# 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
# errorSwallow,unsafeAny,a11y,i18n,unusedExport,exhaustiveness,flakyTest,commitLint,apiBreak
# 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
# i18n,unusedExport,exhaustiveness,flakyTest,commitLint,apiBreak
# END GENERATED REES ANALYZERS

# Submitter-reputation spend control (internal-only): downgrades new/burst/low-rep
Expand Down
24 changes: 24 additions & 0 deletions apps/gittensory-ui/src/lib/rees-analyzers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1219,6 +1219,30 @@ export const REES_ANALYZERS = [
"Structured-fields-only: reads commit.message subjects, linted independently, never cross-line state. Fail-safe on missing token/fetch error.",
},
},
{
name: "apiBreak",
title: "Breaking API change (removed/renamed export)",
category: "quality",
cost: "local",
defaultEnabled: true,
profiles: ["fast", "balanced", "deep"],
requires: ["files"],
limits: {
maxEntrypoints: 25,
maxFindings: 25,
},
docs: {
summary:
"Flags an exported symbol a PR removes or renames in a package public entrypoint — a semver-major break for downstream consumers shipped without a major version bump.",
looksAt:
"Removed (-) top-level export declarations and re-exports in changed public-entrypoint files (index/mod/main/public-api) whose name is not re-added anywhere in the same file's patch.",
reports:
"Public entrypoint file, old-file line, and the removed or renamed exported symbol name — never surrounding code.",
network: "Pure local analyzer. No external network call.",
notes:
"Conservative: only a top-level export whose exact name disappears from the file's public surface is reported; a same-name edit (signature or value change) or a non-entrypoint file is never flagged. Bounded by entrypoint and finding caps; fail-safe on absent or malformed patches.",
},
},
] as const satisfies readonly ReesAnalyzerDoc[];

export const REES_ANALYZER_NAMES = REES_ANALYZERS.map((analyzer) => analyzer.name);
26 changes: 26 additions & 0 deletions review-enrichment/analyzer-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -1374,6 +1374,32 @@
"network": "Calls the GitHub PR-commits API once, bounded to one page.",
"notes": "Structured-fields-only: reads commit.message subjects, linted independently, never cross-line state. Fail-safe on missing token/fetch error."
}
},
{
"name": "apiBreak",
"title": "Breaking API change (removed/renamed export)",
"category": "quality",
"cost": "local",
"defaultEnabled": true,
"profiles": [
"fast",
"balanced",
"deep"
],
"requires": [
"files"
],
"limits": {
"maxEntrypoints": 25,
"maxFindings": 25
},
"docs": {
"summary": "Flags an exported symbol a PR removes or renames in a package public entrypoint — a semver-major break for downstream consumers shipped without a major version bump.",
"looksAt": "Removed (-) top-level export declarations and re-exports in changed public-entrypoint files (index/mod/main/public-api) whose name is not re-added anywhere in the same file's patch.",
"reports": "Public entrypoint file, old-file line, and the removed or renamed exported symbol name — never surrounding code.",
"network": "Pure local analyzer. No external network call.",
"notes": "Conservative: only a top-level export whose exact name disappears from the file's public surface is reported; a same-name edit (signature or value change) or a non-entrypoint file is never flagged. Bounded by entrypoint and finding caps; fail-safe on absent or malformed patches."
}
}
]
}
129 changes: 129 additions & 0 deletions review-enrichment/src/analyzers/api-break.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
// Breaking-API-change analyzer (#1510, part of #1499). A no-checkout headless reviewer sees only the diff, so it
// cannot tell that a PR DROPS or RENAMES a symbol a package's public entrypoint used to export — a semver-major
// break that reaches DOWNSTREAM consumers, distinct from the in-repo caller-impact analyzer. This fills that gap
// purely from the patch: for each changed PUBLIC-ENTRYPOINT file (index/mod/main/public-api barrels) it collects
// the exported names on removed (`-`) lines and on added (`+`) lines, and reports a name present in the REMOVED
// set but absent from the ADDED set — i.e. the public surface lost it (a removal or a rename). Deliberately
// CONSERVATIVE and fail-safe: a same-name edit (a signature/value change re-adds the name) is never flagged, only
// 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";

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

// 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
// break. Declaration (`.d.ts`) and test/spec files are excluded: their exports are not a shipped public API.
const SOURCE_RE = /\.(?:ts|tsx|mts|cts|js|jsx|mjs|cjs)$/;
const SKIP_RE = /(?:\.d\.ts$|\.min\.|\.test\.|\.spec\.|__tests__\/|(?:^|\/)tests?\/)/;
const ENTRYPOINT_BASENAME = /^(?:index|mod|main|public-api|public_api|api)$/;

// A named top-level export DECLARATION at column 0 (an indented `export` inside a namespace/module block is
// intentionally not matched). `const enum` precedes the bare `const` alternative so the enum name, not `enum`, is
// captured.
const EXPORT_DECL_RE =
/^export\s+(?:async\s+)?(?:abstract\s+)?(?:function\s*\*?|class|const\s+enum|const|let|var|interface|type|enum)\s+([A-Za-z_$][\w$]*)/;
// `export * as ns from "..."` binds one namespace name; `export * from "..."` binds none and is not matched.
const EXPORT_STAR_AS_RE = /^export\s+\*\s+as\s+([A-Za-z_$][\w$]*)\s+from\b/;
// A single-line `export { a, b as c } [from "..."]` / `export type { T } from "..."` list. Aliases resolve to the
// PUBLIC (right-hand) name. A brace list spanning multiple lines is intentionally not parsed (fail-safe).
const EXPORT_NAMED_RE = /^export\s+(?:type\s+)?\{([^}]*)\}/;
const IDENT_RE = /^[A-Za-z_$][\w$]*$/;

/** True when a changed path is a package PUBLIC ENTRYPOINT (barrel/entry basename, source ext, not decl/test). Pure. */
export function isPublicEntrypoint(path: string): boolean {
if (!SOURCE_RE.test(path) || SKIP_RE.test(path)) return false;
const base = path.split("/").pop() ?? path;
const stem = base.replace(SOURCE_RE, "");
return ENTRYPOINT_BASENAME.test(stem);
}

/** Every exported symbol name a single source line declares or re-exports at the top level. Handles declarations,
* `export default`, single-line `export { … }`/`export type { … }` (aliases resolve to the PUBLIC name), and
* `export * as ns from`. A bare `export * from "…"` binds no nameable symbol and yields none. Pure. */
export function exportedNames(line: string): string[] {
const decl = EXPORT_DECL_RE.exec(line);
if (decl) return [decl[1]!];
if (/^export\s+default\b/.test(line)) return ["default"];
const starAs = EXPORT_STAR_AS_RE.exec(line);
if (starAs) return [starAs[1]!];
const named = EXPORT_NAMED_RE.exec(line);
if (named) {
const out: string[] = [];
for (const raw of named[1]!.split(",")) {
const spec = raw.trim();
if (!spec) continue;
const parts = spec.split(/\s+as\s+/);
const publicName = (parts[parts.length - 1] ?? "").trim();
if (IDENT_RE.test(publicName)) out.push(publicName);
}
return out;
}
return [];
}

interface RemovedExport {
symbol: string;
line: number;
}

/** Removed/renamed exports in one entrypoint file's patch: names exported on a removed (`-`) line whose name is NOT
* re-exported on any added (`+`) line of the same file (a same-name edit re-adds it and is not a break). The
* old-file line counter advances over removed + context lines (never added lines) so the reported line is the
* symbol's pre-PR location. Pure. */
export function removedExports(patch: string): RemovedExport[] {
const removed = new Map<string, number>();
const added = new Set<string>();
let oldLine = 0;
let inHunk = false;
for (const raw of patch.split("\n")) {
const hunk = /^@@ -(\d+)(?:,\d+)? \+\d+(?:,\d+)? @@/.exec(raw);
if (hunk) {
oldLine = Number(hunk[1]);
inHunk = true;
continue;
}
if (!inHunk) continue;
if (raw.startsWith("+")) {
if (raw.startsWith("+++")) continue;
for (const name of exportedNames(raw.slice(1))) added.add(name);
} else if (raw.startsWith("-")) {
if (raw.startsWith("---")) continue;
for (const name of exportedNames(raw.slice(1))) {
if (!removed.has(name)) removed.set(name, oldLine);
}
oldLine++;
} else if (!raw.startsWith("\\")) {
oldLine++;
}
}
const out: RemovedExport[] = [];
for (const [symbol, line] of removed) {
if (!added.has(symbol)) out.push({ symbol, line });
}
return out;
}

/** Analyzer entrypoint: flag exported symbols a PR drops or renames from a public entrypoint — a downstream
* semver-major break. Deterministic and fail-safe: returns [] when no entrypoint file changed or on absent
* patches; bounded by entrypoint and finding caps. */
export async function scanApiBreak(
req: EnrichRequest,
signal?: AbortSignal,
): Promise<ApiBreakFinding[]> {
if (signal?.aborted) return [];
const findings: ApiBreakFinding[] = [];
let entrypoints = 0;
for (const file of req.files ?? []) {
if (signal?.aborted) break;
if (!file.patch || !isPublicEntrypoint(file.path)) continue;
if (entrypoints >= MAX_ENTRYPOINTS) break;
entrypoints++;
for (const removed of removedExports(file.patch)) {
findings.push({ file: file.path, line: removed.line, symbol: removed.symbol });
if (findings.length >= MAX_FINDINGS) return findings;
}
}
return findings;
}
34 changes: 34 additions & 0 deletions review-enrichment/src/analyzers/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import { scanUndocumentedExport } from "./undocumented-export.js";
import { scanUnusedExport } from "./unused-export.js";
import { scanExhaustivenessDrift } from "./exhaustiveness-drift.js";
import { scanFlakyTest } from "./flaky-test.js";
import { scanApiBreak } from "./api-break.js";
import type {
AnalyzerDescriptor,
AnalyzerFn,
Expand Down Expand Up @@ -1375,6 +1376,39 @@ export const ANALYZER_DESCRIPTORS = [
run: (req, { signal, analysis, diagnostics }) =>
scanCommitLint(req, fetch, { signal, analysis, diagnostics }),
}),
descriptor({
name: "apiBreak",
title: "Breaking API change (removed/renamed export)",
category: "quality",
cost: "local",
defaultEnabled: true,
requires: ["files"],
limits: { maxEntrypoints: 25, maxFindings: 25 },
docs: {
summary:
"Flags an exported symbol a PR removes or renames in a package public entrypoint — a semver-major break for downstream consumers shipped without a major version bump.",
looksAt:
"Removed (-) top-level export declarations and re-exports in changed public-entrypoint files (index/mod/main/public-api) whose name is not re-added anywhere in the same file's patch.",
reports:
"Public entrypoint file, old-file line, and the removed or renamed exported symbol name — never surrounding code.",
network: "Pure local analyzer. No external network call.",
notes:
"Conservative: only a top-level export whose exact name disappears from the file's public surface is reported; a same-name edit (signature or value change) or a non-entrypoint file is never flagged. Bounded by entrypoint and finding caps; fail-safe on absent or malformed patches.",
},
render: (findings, helpers) => {
if (!findings.length) return [];
const lines = [
"### Breaking API changes (exported symbol removed or renamed in a public entrypoint)",
];
for (const item of findings) {
lines.push(
`- ${helpers.safeCodeSpan(`${item.file}:${item.line}`)} — ${helpers.safeCodeSpan(item.symbol)} removed from the public surface`,
);
}
return lines;
},
run: (req, { signal }) => scanApiBreak(req, signal),
}),
] as const satisfies readonly AnyAnalyzerDescriptor[];

export const ANALYZER_NAMES = ANALYZER_DESCRIPTORS.map(
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 @@ -498,6 +498,7 @@ export function renderBrief(
lines.push(...renderDescriptorSection("flakyTest", findings.flakyTest));
lines.push(...renderDescriptorSection("hardcodedUrl", findings.hardcodedUrl));
lines.push(...renderDescriptorSection("commitLint", findings.commitLint));
lines.push(...renderDescriptorSection("apiBreak", findings.apiBreak));

if (!lines.length) return { promptSection: "", systemSuffix: "" };

Expand Down
10 changes: 10 additions & 0 deletions review-enrichment/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,15 @@ export interface CommitLintFinding {
}

/** Structured analyzer output. Each analyzer fills its own key; more land as analyzers ship (#1477/#1478). */
/** An exported symbol a PR removes or renames in a package public entrypoint — a semver-major break for
* downstream consumers shipped without a major version bump (#1510, part of #1499). Reports file, old-file line,
* and the removed/renamed symbol name only — never code. */
export interface ApiBreakFinding {
file: string;
line: number;
symbol: string;
}

export interface BriefFindings {
dependency?: DependencyFinding[];
dependencyDiff?: DependencyDiffFinding[];
Expand Down Expand Up @@ -642,6 +651,7 @@ export interface BriefFindings {
flakyTest?: FlakyTestFinding[];
hardcodedUrl?: HardcodedUrlFinding[];
commitLint?: CommitLintFinding[];
apiBreak?: ApiBreakFinding[];
}

/** A JSDoc/TSDoc block whose `@param` tags name parameters the adjacent function no longer declares — a
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 @@ -59,6 +59,7 @@ const EXPECTED_ANALYZERS = [
"exhaustiveness",
"flakyTest",
"commitLint",
"apiBreak",
];

test("analyzer descriptors cover the runtime registry in stable order", () => {
Expand Down
Loading
Loading