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
13 changes: 7 additions & 6 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ GITTENSORY_REVIEW_ENRICHMENT=false
# churnHotspot,blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch
# commitHygiene,pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology
# todoMarker,magicNumber,conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting
# errorSwallow,complexity,unsafeAny,a11y,i18n,unusedExport,exhaustiveness,flakyTest,commitLint
# apiBreak,deprecatedDep,revertRecurrence,coverageDelta,callerImpact
# errorSwallow,complexity,complexityDelta,unsafeAny,a11y,i18n,unusedExport,exhaustiveness
# flakyTest,commitLint,apiBreak,deprecatedDep,revertRecurrence,coverageDelta,callerImpact
#
# Profile defaults:
# fast: dependency,dependencyDiff,lockfileDrift,secret,license,installScript,heavyDependency
Expand All @@ -84,16 +84,17 @@ GITTENSORY_REVIEW_ENRICHMENT=false
# duplicationDelta,churnHotspot,blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport
# staleBranch,commitHygiene,pendingReviewRequests,testRatio,migrationSafety,looseRange
# terminology,todoMarker,magicNumber,conflictMarker,debugLeftover,sizeSmell,floatingPromise
# deepNesting,errorSwallow,complexity,unsafeAny,a11y,i18n,unusedExport,exhaustiveness,flakyTest
# commitLint,apiBreak,deprecatedDep,revertRecurrence,coverageDelta,callerImpact
# deepNesting,errorSwallow,complexity,complexityDelta,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,duplicationDelta
# churnHotspot,blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch
# commitHygiene,pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology
# todoMarker,magicNumber,conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting
# errorSwallow,complexity,unsafeAny,a11y,i18n,unusedExport,exhaustiveness,flakyTest,commitLint
# apiBreak,deprecatedDep,revertRecurrence,coverageDelta,callerImpact
# errorSwallow,complexity,complexityDelta,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
25 changes: 25 additions & 0 deletions apps/gittensory-ui/src/lib/rees-analyzers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1102,6 +1102,31 @@ export const REES_ANALYZERS = [
"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.",
},
},
{
name: "complexityDelta",
title: "Complexity delta (before/after)",
category: "quality",
cost: "github-light",
defaultEnabled: true,
profiles: ["balanced", "deep"],
requires: ["files", "github-token", "head-sha"],
limits: {
maxFiles: 20,
maxFindings: 25,
},
docs: {
summary:
"Flags a function whose approximate cyclomatic complexity changed between the pre-PR and head versions of a file -- not just newly-added functions.",
looksAt:
"Changed TS/JS source files, reconstructing the pre-PR file at headSha via the shared before-content primitive and re-running complexity's own decision-point counting on both versions.",
reports:
"File, the function's current line, name, and its before/after/delta approximate complexity.",
network:
"Calls the GitHub API for changed file contents at headSha. Requires headSha and token forwarding for private repos.",
notes:
"Complements complexity (new-function absolute threshold): a function whose signature is unchanged but whose body got simpler shows a negative (improving) delta -- the case the absolute-threshold analyzer alone cannot see. A wholly-added file or an unreconstructable patch degrades to zero findings for that file rather than guessing. A function name that recurs more than once in either version is excluded from matching (ambiguous).",
},
},
{
name: "unsafeAny",
title: "Unsafe any (TS)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export const REES_ANALYZER_NAMES = [
"deepNesting",
"errorSwallow",
"complexity",
"complexityDelta",
"unsafeAny",
"a11y",
"i18n",
Expand Down
27 changes: 27 additions & 0 deletions review-enrichment/analyzer-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -1244,6 +1244,33 @@
"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."
}
},
{
"name": "complexityDelta",
"title": "Complexity delta (before/after)",
"category": "quality",
"cost": "github-light",
"defaultEnabled": true,
"profiles": [
"balanced",
"deep"
],
"requires": [
"files",
"github-token",
"head-sha"
],
"limits": {
"maxFiles": 20,
"maxFindings": 25
},
"docs": {
"summary": "Flags a function whose approximate cyclomatic complexity changed between the pre-PR and head versions of a file -- not just newly-added functions.",
"looksAt": "Changed TS/JS source files, reconstructing the pre-PR file at headSha via the shared before-content primitive and re-running complexity's own decision-point counting on both versions.",
"reports": "File, the function's current line, name, and its before/after/delta approximate complexity.",
"network": "Calls the GitHub API for changed file contents at headSha. Requires headSha and token forwarding for private repos.",
"notes": "Complements complexity (new-function absolute threshold): a function whose signature is unchanged but whose body got simpler shows a negative (improving) delta -- the case the absolute-threshold analyzer alone cannot see. A wholly-added file or an unreconstructable patch degrades to zero findings for that file rather than guessing. A function name that recurs more than once in either version is excluded from matching (ambiguous)."
}
},
{
"name": "unsafeAny",
"title": "Unsafe any (TS)",
Expand Down
178 changes: 178 additions & 0 deletions review-enrichment/src/analyzers/complexity-delta.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
// Real before/after complexity-delta analyzer (#4740, part of epic #4737's REES/deterministic-tier phase).
// complexity.ts's own `complexity` analyzer explicitly disclaims being a true before/after delta: it normally
// only sees diff hunks, so it can only score a NEWLY-ADDED function (whose opening line is in the diff) against
// a fixed absolute threshold -- a function whose signature is unchanged but whose BODY was edited gets no score
// at all, so a PR that meaningfully SIMPLIFIES a gnarly existing function gets no credit. This analyzer closes
// that gap using the shared reconstructOldContent primitive (#4739): fetch the changed file's post-PR content at
// headSha (the same authed GitHub contents-API fetch doc-comment-drift.ts/exhaustiveness-drift.ts already
// perform for their own purposes), reverse-apply the patch to recover the pre-PR text, run complexity.ts's OWN
// decision-point counting logic (`scanContentForComplexity` -- reused unchanged, not reimplemented) against BOTH
// versions, match functions by name, and diff the two scores.
//
// Registered as a SEPARATE AnalyzerName (`complexityDelta`) rather than folded into `complexity`'s existing
// entry -- see complexity.ts's header for the full reasoning. Short version: merging this network-dependent,
// before/after logic into `complexity`'s single `requires`/`cost` would either (a) gate `complexity`'s existing
// free, local, always-available absolute-threshold check behind `github-token`/`head-sha`, regressing it
// whenever either is unavailable (scheduler.ts's skipReasonForAnalyzer skips a descriptor's `run` entirely based
// on its DECLARED `requires`, before ever calling it -- this is real scheduling behavior, not just docs), or (b)
// mislabel this genuinely network-costed half as `cost: "local"`, letting it dodge the `github-light`
// concurrency/timeout budget and the `fast` profile's network-free guarantee. Two honestly-classified
// descriptors instead of one dishonest one.
//
// A function whose name recurs more than once in either version (ambiguous -- same rule
// scanContentForComplexity/doc-comment-drift.ts's extractFunctionParams already apply) is excluded from
// matching. A function present only in the NEW version has no "before" to diff against -- that is exactly
// `complexity`'s own job, not this analyzer's. A wholly-added file (reconstructOldContent's `""` return) or an
// unreconstructable patch (`null`) are both "no usable before content" and degrade to zero delta findings for
// that file, never a crash -- checked via plain truthiness, NEVER a strict `=== null` compare (see
// reconstruct-old-content.ts's own doc comment: an empty string is falsy but `!== null`, so a strict-null check
// would wrongly treat a brand-new file's "" as valid before-content).
import type { EnrichRequest, ComplexityDeltaFinding } from "../types.js";
import { githubHeaders } from "../github-headers.js";
import { reconstructOldContent } from "./reconstruct-old-content.js";
import { isJsTsPath, scanContentForComplexity } from "./complexity.js";
import { DEFAULT_MAX_FINDINGS } from "./limits.js";

const GITHUB_API = "https://api.github.com";
const SLUG_RE = /^[A-Za-z0-9._-]+$/;
const MAX_FILES = 20;
const MAX_FINDINGS = DEFAULT_MAX_FINDINGS;
const MAX_FETCH_BYTES = 1_000_000;

interface ScanOptions {
signal?: AbortSignal;
}

async function readBoundedText(resp: Response, signal?: AbortSignal): Promise<string | null> {
const length = Number(resp.headers.get("content-length"));
if (Number.isFinite(length) && length > MAX_FETCH_BYTES) return null;
if (!resp.body) return null;

const reader = resp.body.getReader();
const decoder = new TextDecoder();
let size = 0;
let text = "";
try {
while (true) {
if (signal?.aborted) return null;
const { done, value } = await reader.read();
if (done) break;
size += value.byteLength;
if (size > MAX_FETCH_BYTES) {
await reader.cancel();
return null;
}
text += decoder.decode(value, { stream: true });
}
text += decoder.decode();
return text;
} finally {
reader.releaseLock();
}
}

async function fetchFileAtHeadSha(
owner: string,
repo: string,
path: string,
headSha: string,
token: string,
fetchFn: typeof fetch,
signal: AbortSignal | undefined,
): Promise<string | null> {
try {
const encoded = path.split("/").map(encodeURIComponent).join("/");
const resp = await fetchFn(
`${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/contents/${encoded}?ref=${encodeURIComponent(headSha)}`,
{ headers: githubHeaders(token, { raw: true }), signal },
);
if (!resp.ok) return null;
return await readBoundedText(resp, signal);
} catch {
return null;
}
}

/** Full-file-scan the reconstructed OLD content and the NEW (head) content of one file with
* complexity.ts's shared `scanContentForComplexity`, and diff every function matched (unambiguously) by name in
* both. A function with no change in its measured complexity is not reported -- only a real before/after
* difference is a finding, since a "delta" of zero is nothing for the sibling aggregator (#4742) to act on. Pure. */
export function matchAndDiffFunctions(
file: string,
oldContent: string,
newContent: string,
limits: { maxFindings?: number } = {},
): ComplexityDeltaFinding[] {
const maxFindings = limits.maxFindings ?? MAX_FINDINGS;
if (maxFindings <= 0) return [];

const oldScores = scanContentForComplexity(oldContent);
const newScores = scanContentForComplexity(newContent);

const findings: ComplexityDeltaFinding[] = [];
for (const [name, after] of newScores) {
const before = oldScores.get(name);
if (!before || before.complexity === after.complexity) continue;
findings.push({
file,
line: after.line,
name,
before: before.complexity,
after: after.complexity,
delta: after.complexity - before.complexity,
});
if (findings.length >= maxFindings) break;
}
return findings;
}

/** Analyzer entrypoint: for each changed JS/TS source file, reconstruct its pre-PR content and diff real
* before/after complexity per function. Fail-safe -- never throws on a missing token/headSha, an
* unreconstructable patch, or a fetch error; each degrades to zero findings for that file rather than a crash
* or a guessed answer. */
export async function scanComplexityDelta(
req: EnrichRequest,
fetchFn: typeof fetch = fetch,
options: ScanOptions = {},
): Promise<ComplexityDeltaFinding[]> {
const { repoFullName, githubToken, headSha, files = [] } = req;
if (!githubToken || !headSha) return [];
const parts = repoFullName.split("/");
const owner = parts[0];
const repo = parts[1];
if (parts.length !== 2 || !owner || !repo || !SLUG_RE.test(owner) || !SLUG_RE.test(repo)) return [];

const sources = files.filter((file) => file.patch && isJsTsPath(file.path)).slice(0, MAX_FILES);

const findings: ComplexityDeltaFinding[] = [];
for (const file of sources) {
if (options.signal?.aborted) break;

const headContent = await fetchFileAtHeadSha(
owner,
repo,
file.path,
headSha,
githubToken,
fetchFn,
options.signal,
);
if (!headContent) continue;
if (options.signal?.aborted) break; // an abort during the fetch should suppress this file's findings too

// `reconstructOldContent` returns EITHER `null` (patch didn't reverse-apply -- malformed/mismatched) OR `""`
// (patch reverse-applied cleanly but the file is wholly new -- no old-side content at all). Both are "no
// usable before content" and must be treated identically via truthiness; a strict `=== null` check would
// wrongly treat the wholly-new-file "" as valid before-content.
const oldContent = reconstructOldContent(headContent, file.patch!);
if (!oldContent) continue;

for (const finding of matchAndDiffFunctions(file.path, oldContent, headContent, {
maxFindings: MAX_FINDINGS - findings.length,
})) {
findings.push(finding);
if (findings.length >= MAX_FINDINGS) return findings;
}
}
return findings;
}
Loading
Loading