|
| 1 | +// Real before/after complexity-delta analyzer (#4740, part of epic #4737's REES/deterministic-tier phase). |
| 2 | +// complexity.ts's own `complexity` analyzer explicitly disclaims being a true before/after delta: it normally |
| 3 | +// only sees diff hunks, so it can only score a NEWLY-ADDED function (whose opening line is in the diff) against |
| 4 | +// a fixed absolute threshold -- a function whose signature is unchanged but whose BODY was edited gets no score |
| 5 | +// at all, so a PR that meaningfully SIMPLIFIES a gnarly existing function gets no credit. This analyzer closes |
| 6 | +// that gap using the shared reconstructOldContent primitive (#4739): fetch the changed file's post-PR content at |
| 7 | +// headSha (the same authed GitHub contents-API fetch doc-comment-drift.ts/exhaustiveness-drift.ts already |
| 8 | +// perform for their own purposes), reverse-apply the patch to recover the pre-PR text, run complexity.ts's OWN |
| 9 | +// decision-point counting logic (`scanContentForComplexity` -- reused unchanged, not reimplemented) against BOTH |
| 10 | +// versions, match functions by name, and diff the two scores. |
| 11 | +// |
| 12 | +// Registered as a SEPARATE AnalyzerName (`complexityDelta`) rather than folded into `complexity`'s existing |
| 13 | +// entry -- see complexity.ts's header for the full reasoning. Short version: merging this network-dependent, |
| 14 | +// before/after logic into `complexity`'s single `requires`/`cost` would either (a) gate `complexity`'s existing |
| 15 | +// free, local, always-available absolute-threshold check behind `github-token`/`head-sha`, regressing it |
| 16 | +// whenever either is unavailable (scheduler.ts's skipReasonForAnalyzer skips a descriptor's `run` entirely based |
| 17 | +// on its DECLARED `requires`, before ever calling it -- this is real scheduling behavior, not just docs), or (b) |
| 18 | +// mislabel this genuinely network-costed half as `cost: "local"`, letting it dodge the `github-light` |
| 19 | +// concurrency/timeout budget and the `fast` profile's network-free guarantee. Two honestly-classified |
| 20 | +// descriptors instead of one dishonest one. |
| 21 | +// |
| 22 | +// A function whose name recurs more than once in either version (ambiguous -- same rule |
| 23 | +// scanContentForComplexity/doc-comment-drift.ts's extractFunctionParams already apply) is excluded from |
| 24 | +// matching. A function present only in the NEW version has no "before" to diff against -- that is exactly |
| 25 | +// `complexity`'s own job, not this analyzer's. A wholly-added file (reconstructOldContent's `""` return) or an |
| 26 | +// unreconstructable patch (`null`) are both "no usable before content" and degrade to zero delta findings for |
| 27 | +// that file, never a crash -- checked via plain truthiness, NEVER a strict `=== null` compare (see |
| 28 | +// reconstruct-old-content.ts's own doc comment: an empty string is falsy but `!== null`, so a strict-null check |
| 29 | +// would wrongly treat a brand-new file's "" as valid before-content). |
| 30 | +import type { EnrichRequest, ComplexityDeltaFinding } from "../types.js"; |
| 31 | +import { githubHeaders } from "../github-headers.js"; |
| 32 | +import { reconstructOldContent } from "./reconstruct-old-content.js"; |
| 33 | +import { isJsTsPath, scanContentForComplexity } from "./complexity.js"; |
| 34 | +import { DEFAULT_MAX_FINDINGS } from "./limits.js"; |
| 35 | + |
| 36 | +const GITHUB_API = "https://api.github.com"; |
| 37 | +const SLUG_RE = /^[A-Za-z0-9._-]+$/; |
| 38 | +const MAX_FILES = 20; |
| 39 | +const MAX_FINDINGS = DEFAULT_MAX_FINDINGS; |
| 40 | +const MAX_FETCH_BYTES = 1_000_000; |
| 41 | + |
| 42 | +interface ScanOptions { |
| 43 | + signal?: AbortSignal; |
| 44 | +} |
| 45 | + |
| 46 | +async function readBoundedText(resp: Response, signal?: AbortSignal): Promise<string | null> { |
| 47 | + const length = Number(resp.headers.get("content-length")); |
| 48 | + if (Number.isFinite(length) && length > MAX_FETCH_BYTES) return null; |
| 49 | + if (!resp.body) return null; |
| 50 | + |
| 51 | + const reader = resp.body.getReader(); |
| 52 | + const decoder = new TextDecoder(); |
| 53 | + let size = 0; |
| 54 | + let text = ""; |
| 55 | + try { |
| 56 | + while (true) { |
| 57 | + if (signal?.aborted) return null; |
| 58 | + const { done, value } = await reader.read(); |
| 59 | + if (done) break; |
| 60 | + size += value.byteLength; |
| 61 | + if (size > MAX_FETCH_BYTES) { |
| 62 | + await reader.cancel(); |
| 63 | + return null; |
| 64 | + } |
| 65 | + text += decoder.decode(value, { stream: true }); |
| 66 | + } |
| 67 | + text += decoder.decode(); |
| 68 | + return text; |
| 69 | + } finally { |
| 70 | + reader.releaseLock(); |
| 71 | + } |
| 72 | +} |
| 73 | + |
| 74 | +async function fetchFileAtHeadSha( |
| 75 | + owner: string, |
| 76 | + repo: string, |
| 77 | + path: string, |
| 78 | + headSha: string, |
| 79 | + token: string, |
| 80 | + fetchFn: typeof fetch, |
| 81 | + signal: AbortSignal | undefined, |
| 82 | +): Promise<string | null> { |
| 83 | + try { |
| 84 | + const encoded = path.split("/").map(encodeURIComponent).join("/"); |
| 85 | + const resp = await fetchFn( |
| 86 | + `${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/contents/${encoded}?ref=${encodeURIComponent(headSha)}`, |
| 87 | + { headers: githubHeaders(token, { raw: true }), signal }, |
| 88 | + ); |
| 89 | + if (!resp.ok) return null; |
| 90 | + return await readBoundedText(resp, signal); |
| 91 | + } catch { |
| 92 | + return null; |
| 93 | + } |
| 94 | +} |
| 95 | + |
| 96 | +/** Full-file-scan the reconstructed OLD content and the NEW (head) content of one file with |
| 97 | + * complexity.ts's shared `scanContentForComplexity`, and diff every function matched (unambiguously) by name in |
| 98 | + * both. A function with no change in its measured complexity is not reported -- only a real before/after |
| 99 | + * difference is a finding, since a "delta" of zero is nothing for the sibling aggregator (#4742) to act on. Pure. */ |
| 100 | +export function matchAndDiffFunctions( |
| 101 | + file: string, |
| 102 | + oldContent: string, |
| 103 | + newContent: string, |
| 104 | + limits: { maxFindings?: number } = {}, |
| 105 | +): ComplexityDeltaFinding[] { |
| 106 | + const maxFindings = limits.maxFindings ?? MAX_FINDINGS; |
| 107 | + if (maxFindings <= 0) return []; |
| 108 | + |
| 109 | + const oldScores = scanContentForComplexity(oldContent); |
| 110 | + const newScores = scanContentForComplexity(newContent); |
| 111 | + |
| 112 | + const findings: ComplexityDeltaFinding[] = []; |
| 113 | + for (const [name, after] of newScores) { |
| 114 | + const before = oldScores.get(name); |
| 115 | + if (!before || before.complexity === after.complexity) continue; |
| 116 | + findings.push({ |
| 117 | + file, |
| 118 | + line: after.line, |
| 119 | + name, |
| 120 | + before: before.complexity, |
| 121 | + after: after.complexity, |
| 122 | + delta: after.complexity - before.complexity, |
| 123 | + }); |
| 124 | + if (findings.length >= maxFindings) break; |
| 125 | + } |
| 126 | + return findings; |
| 127 | +} |
| 128 | + |
| 129 | +/** Analyzer entrypoint: for each changed JS/TS source file, reconstruct its pre-PR content and diff real |
| 130 | + * before/after complexity per function. Fail-safe -- never throws on a missing token/headSha, an |
| 131 | + * unreconstructable patch, or a fetch error; each degrades to zero findings for that file rather than a crash |
| 132 | + * or a guessed answer. */ |
| 133 | +export async function scanComplexityDelta( |
| 134 | + req: EnrichRequest, |
| 135 | + fetchFn: typeof fetch = fetch, |
| 136 | + options: ScanOptions = {}, |
| 137 | +): Promise<ComplexityDeltaFinding[]> { |
| 138 | + const { repoFullName, githubToken, headSha, files = [] } = req; |
| 139 | + if (!githubToken || !headSha) return []; |
| 140 | + const parts = repoFullName.split("/"); |
| 141 | + const owner = parts[0]; |
| 142 | + const repo = parts[1]; |
| 143 | + if (parts.length !== 2 || !owner || !repo || !SLUG_RE.test(owner) || !SLUG_RE.test(repo)) return []; |
| 144 | + |
| 145 | + const sources = files.filter((file) => file.patch && isJsTsPath(file.path)).slice(0, MAX_FILES); |
| 146 | + |
| 147 | + const findings: ComplexityDeltaFinding[] = []; |
| 148 | + for (const file of sources) { |
| 149 | + if (options.signal?.aborted) break; |
| 150 | + |
| 151 | + const headContent = await fetchFileAtHeadSha( |
| 152 | + owner, |
| 153 | + repo, |
| 154 | + file.path, |
| 155 | + headSha, |
| 156 | + githubToken, |
| 157 | + fetchFn, |
| 158 | + options.signal, |
| 159 | + ); |
| 160 | + if (!headContent) continue; |
| 161 | + if (options.signal?.aborted) break; // an abort during the fetch should suppress this file's findings too |
| 162 | + |
| 163 | + // `reconstructOldContent` returns EITHER `null` (patch didn't reverse-apply -- malformed/mismatched) OR `""` |
| 164 | + // (patch reverse-applied cleanly but the file is wholly new -- no old-side content at all). Both are "no |
| 165 | + // usable before content" and must be treated identically via truthiness; a strict `=== null` check would |
| 166 | + // wrongly treat the wholly-new-file "" as valid before-content. |
| 167 | + const oldContent = reconstructOldContent(headContent, file.patch!); |
| 168 | + if (!oldContent) continue; |
| 169 | + |
| 170 | + for (const finding of matchAndDiffFunctions(file.path, oldContent, headContent, { |
| 171 | + maxFindings: MAX_FINDINGS - findings.length, |
| 172 | + })) { |
| 173 | + findings.push(finding); |
| 174 | + if (findings.length >= MAX_FINDINGS) return findings; |
| 175 | + } |
| 176 | + } |
| 177 | + return findings; |
| 178 | +} |
0 commit comments