Skip to content

Commit b3a3873

Browse files
committed
refactor(enrichment): soften blame-link wording + precise category + tighter cap test (#2034)
Final review nits (all non-blocking): - Render says the file "was most recently touched by" instead of "introduced by" a specific line — accurate to the file-level path-history lookup this does. - Give the commit→PR association call its own endpointCategory ("github-commit-pulls") for precise rate attribution (loop-level MAX_LOOKUPS still bounds the total). - Cap test now asserts exactly 12 fetch calls and that files past the probe cap (f6..f9) are never queried, pinning the bound. review-enrichment: 402/402 pass.
1 parent e8dde36 commit b3a3873

3 files changed

Lines changed: 23 additions & 6 deletions

File tree

review-enrichment/src/analyzers/blame-link.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,10 @@ async function fetchGithubJson<T>(
7676
signal: AbortSignal | undefined,
7777
options: Pick<ScanOptions, "analysis" | "diagnostics">,
7878
subcall: string,
79+
endpointCategory: string,
7980
): Promise<T | null> {
8081
const fetchOptions = {
81-
endpointCategory: "github-commits",
82+
endpointCategory,
8283
headers,
8384
signal,
8485
fetchImpl: fetchFn,
@@ -110,7 +111,9 @@ export async function fetchLatestCommitSha(
110111
const url =
111112
`${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/commits` +
112113
`?path=${encodeURIComponent(path)}&per_page=1${shaQuery}`;
113-
const commits = await fetchGithubJson<CommitListItem[]>(url, headers, fetchFn, signal, options, "github-commits");
114+
const commits = await fetchGithubJson<CommitListItem[]>(
115+
url, headers, fetchFn, signal, options, "github-commits", "github-commits",
116+
);
114117
const sha = Array.isArray(commits) ? commits[0]?.sha : undefined;
115118
return typeof sha === "string" && sha ? sha : null;
116119
}
@@ -126,7 +129,9 @@ export async function fetchPrForCommit(
126129
options: Pick<ScanOptions, "analysis" | "diagnostics">,
127130
): Promise<number | null> {
128131
const url = `${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/commits/${encodeURIComponent(sha)}/pulls`;
129-
const pulls = await fetchGithubJson<AssociatedPr[]>(url, headers, fetchFn, signal, options, "github-commit-pulls");
132+
const pulls = await fetchGithubJson<AssociatedPr[]>(
133+
url, headers, fetchFn, signal, options, "github-commit-pulls", "github-commit-pulls",
134+
);
130135
const number = Array.isArray(pulls) ? pulls[0]?.number : undefined;
131136
return typeof number === "number" ? number : null;
132137
}

review-enrichment/src/analyzers/registry.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ export const ANALYZER_DESCRIPTORS = [
470470
? `commit ${helpers.safeCodeSpan(item.introducedByShaPrefix)}`
471471
: "an unknown prior change";
472472
lines.push(
473-
`- ${helpers.safeCodeSpan(item.file)} (old line ${item.line}) was most recently introduced by ${origin}`,
473+
`- ${helpers.safeCodeSpan(item.file)} (around old line ${item.line}) was most recently touched by ${origin}`,
474474
);
475475
}
476476
return lines;

review-enrichment/test/blame-link.test.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,14 +100,26 @@ test("scanBlameLink: pure-addition and added files are skipped (nothing to blame
100100
assert.deepEqual(findings, []);
101101
});
102102

103-
test("scanBlameLink: caps the number of files probed", async () => {
103+
test("scanBlameLink: caps probed files and total lookups, leaving later files untouched", async () => {
104104
const files = Array.from({ length: 10 }, (_, i) => ({
105105
path: `src/f${i}.ts`,
106106
status: "modified",
107107
patch: modifyPatch(i + 1),
108108
}));
109-
const findings = await scanBlameLink(req(files), routedFetch({ commitSha: "abcdef1234567890", prNumber: 9 }));
109+
let calls = 0;
110+
const probedPaths = new Set();
111+
const countingFetch = async (url) => {
112+
calls += 1;
113+
const path = new URL(url).searchParams.get("path");
114+
if (path) probedPaths.add(path);
115+
if (url.includes("/pulls")) return jsonResponse([{ number: 9 }]);
116+
if (url.includes("/commits?")) return jsonResponse([{ sha: "abcdef1234567890" }]);
117+
return jsonResponse([], 404);
118+
};
119+
const findings = await scanBlameLink(req(files), countingFetch);
110120
assert.equal(findings.length, 6); // MAX_FILES_PROBED
121+
assert.equal(calls, 12); // 6 files × (1 commit-list + 1 pulls) = MAX_LOOKUPS; not one more
122+
assert.deepEqual([...probedPaths].sort(), ["src/f0.ts", "src/f1.ts", "src/f2.ts", "src/f3.ts", "src/f4.ts", "src/f5.ts"]); // f6..f9 never probed
111123
});
112124

113125
test("scanBlameLink: no GitHub token → skipped (no finding, no throw)", async () => {

0 commit comments

Comments
 (0)