Skip to content

feat(review): enforce an aggregate token budget across the AI review prompt #3941

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JSONbored merged 1 commit into from
Jul 7, 2026
Merged

feat(review): enforce an aggregate token budget across the AI review prompt #3941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
88 changes: 71 additions & 17 deletions src/services/ai-review.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,40 @@ export function parseModelReview(text: string): ModelReview | null {
}
}

// Aggregate ceiling across ALL optional context sections combined (#3900). Each section below already
// enforces its OWN per-section cap (FILE_CONTENT_BUDGET, MAX_CONTEXT_CHARS, MAX_PROMPT_CHARS,
// MAX_ENRICHMENT_PROMPT_SECTION_CHARS...), but nothing previously bounded the COMBINED total: with every
// convergence feature enabled on one repo, the worst-case assembled prompt exceeds 200,000 characters before
// the system prompt is even added, degrading signal-to-noise on exactly the large/complex PRs that most need
// focused attention. The diff + description are NOT counted against this ceiling -- they are the primary
// review target and are always included in full (capped at 120,000 + 2,000 chars regardless). 200,000 sits
// comfortably above diff+description+grounding's own worst case (~182k) so grounding is effectively never
// trimmed, while still meaningfully bounding the "every feature enabled" case.
const AGGREGATE_CONTEXT_BUDGET_CHARS = 200_000;

/**
* Priority-ordered cutoff (#3900): walk the optional sections highest-priority-first, including each while it
* still fits under the remaining budget, and stop entirely (dropping this section AND every lower-priority
* one after it) the moment one would not fit. A simple, predictable priority cutoff -- not a bin-packing
* optimization that could skip a large section to squeeze in a smaller, lower-priority one instead.
*/
function selectContextSectionsWithinBudget(
sections: ReadonlyArray<{ key: string; text: string | null | undefined }>,
usedChars: number,
budgetChars: number,
): Set<string> {
const included = new Set<string>();
let running = usedChars;
for (const section of sections) {
if (!section.text) continue;
const addedChars = section.text.length + 2; // +2 for the blank-line separator `lines.push("", text)` adds
if (running + addedChars > budgetChars) break;
included.add(section.key);
running += addedChars;
}
return included;
}

function buildUserPrompt(input: GittensoryAiReviewInput): string {
const lines = [
`Repository: ${input.repoFullName}`,
Expand All @@ -687,33 +721,50 @@ function buildUserPrompt(input: GittensoryAiReviewInput): string {
// review — self-host reviewers are configured with at least as much room). (#extensive-reviews)
input.diff.slice(0, 120000),
];
// Convergence (grounding): append the FINISHED CI status + FULL file content when the caller supplied them
// (flag GITTENSORY_REVIEW_GROUNDING on). Absent/empty (the default) → the prompt is byte-identical to today.
// Convergence (grounding): the FINISHED CI status + FULL file content when the caller supplied them (flag
// GITTENSORY_REVIEW_GROUNDING on). Absent/empty (the default) → the prompt is byte-identical to today.
const groundingSection = input.grounding?.promptSection;
if (groundingSection) lines.push("", groundingSection);
// Convergence (RAG retrieval): append the retrieved RELEVANT EXISTING CODE / DOCS block when the caller
// supplied one (flag GITTENSORY_REVIEW_RAG on AND an index exists). Absent/empty (the default) → byte-identical.
// Convergence (RAG retrieval): the retrieved RELEVANT EXISTING CODE / DOCS block when the caller supplied
// one (flag GITTENSORY_REVIEW_RAG on AND an index exists). Absent/empty (the default) → byte-identical.
const ragSection = input.ragContext;
if (ragSection) lines.push("", ragSection);
// Deterministic impact map (#2186): append the "IMPACT MAP" block when the caller supplied one (BOTH
// Deterministic impact map (#2186): the "IMPACT MAP" block when the caller supplied one (BOTH
// GITTENSORY_REVIEW_IMPACT_MAP AND the per-repo review.impact_map opt-in on, AND the computation found at
// least one affected module). Absent/empty (the default) → the prompt is byte-identical to today.
const impactMapSection = input.impactMapContext;
if (impactMapSection) lines.push("", impactMapSection);
// Repo quality-culture profile (#2995): append the ADDITIVE "REPO QUALITY-CULTURE PROFILE" reference block
// when the caller supplied one (flag GITTENSORY_REVIEW_CULTURE_PROFILE + review.culture_profile both on).
// Absent/empty (the default) → the prompt is byte-identical. Reference-only grounding, never a gate input.
const cultureProfileSection = input.cultureProfileContext;
if (cultureProfileSection) lines.push("", cultureProfileSection);
// Review-enrichment brief (#1472): append the external REES analysis block when the caller supplied one (flag
// Review-enrichment brief (#1472): the external REES analysis block when the caller supplied one (flag
// GITTENSORY_REVIEW_ENRICHMENT on AND REES_URL set). Absent/empty (the default) → the prompt is byte-identical.
const enrichmentSection = input.enrichment?.promptSection;
if (enrichmentSection) lines.push("", enrichmentSection);
// Test-evidence classifier (#2558): ground the reviewer's test-adequacy judgment in the engine's own
// Repo quality-culture profile (#2995): the ADDITIVE "REPO QUALITY-CULTURE PROFILE" reference block when
// the caller supplied one (flag GITTENSORY_REVIEW_CULTURE_PROFILE + review.culture_profile both on).
// Absent/empty (the default) → the prompt is byte-identical. Reference-only grounding, never a gate input.
const cultureProfileSection = input.cultureProfileContext;
// Test-evidence classifier (#2558): grounds the reviewer's test-adequacy judgment in the engine's own
// deterministic classification instead of eyeballing the diff. Absent/no changed code files without test
// evidence ⇒ the prompt is byte-identical.
const testEvidenceSection = buildTestEvidencePromptSection(input.changedFiles ?? []);
if (testEvidenceSection) lines.push("", testEvidenceSection);

// Priority order (highest first): grounding (CI truth + full-file content) > RAG (codebase context) >
// impact map (deterministic blast-radius) > enrichment (external analyzer brief) > culture profile (soft
// house-style reference) > test-evidence flag (smallest, narrowest signal).
const included = selectContextSectionsWithinBudget(
[
{ key: "grounding", text: groundingSection },
{ key: "rag", text: ragSection },
{ key: "impactMap", text: impactMapSection },
{ key: "enrichment", text: enrichmentSection },
{ key: "cultureProfile", text: cultureProfileSection },
{ key: "testEvidence", text: testEvidenceSection },
],
lines.join("\n").length,
AGGREGATE_CONTEXT_BUDGET_CHARS,
);

if (groundingSection && included.has("grounding")) lines.push("", groundingSection);
if (ragSection && included.has("rag")) lines.push("", ragSection);
if (impactMapSection && included.has("impactMap")) lines.push("", impactMapSection);
if (cultureProfileSection && included.has("cultureProfile")) lines.push("", cultureProfileSection);
if (enrichmentSection && included.has("enrichment")) lines.push("", enrichmentSection);
if (testEvidenceSection && included.has("testEvidence")) lines.push("", testEvidenceSection);
return lines.join("\n");
}

Expand Down Expand Up @@ -2178,4 +2229,7 @@ export const __aiReviewInternals = {
runWorkersOpinion,
coerceAiUsage,
aggregateActualUsage,
buildUserPrompt,
selectContextSectionsWithinBudget,
AGGREGATE_CONTEXT_BUDGET_CHARS,
};
120 changes: 120 additions & 0 deletions test/unit/ai-review.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ const {
runWorkersOpinion,
coerceAiUsage,
aggregateActualUsage,
buildUserPrompt,
selectContextSectionsWithinBudget,
AGGREGATE_CONTEXT_BUDGET_CHARS,
} = __aiReviewInternals;

type InlineFinding = {
Expand Down Expand Up @@ -3451,6 +3454,123 @@ describe("buildTestEvidencePromptSection (#2558)", () => {
});
});

describe("selectContextSectionsWithinBudget (#3900)", () => {
it("includes every present section when the total comfortably fits the budget", () => {
const included = selectContextSectionsWithinBudget(
[
{ key: "a", text: "x".repeat(100) },
{ key: "b", text: "y".repeat(100) },
{ key: "c", text: undefined },
],
0,
1000,
);
expect(included).toEqual(new Set(["a", "b"]));
});

it("stops at the first section that would overflow and drops every lower-priority section after it, even one that would individually fit", () => {
const included = selectContextSectionsWithinBudget(
[
{ key: "first", text: "a".repeat(500) },
{ key: "second", text: "b".repeat(600) }, // 500+600=1100 > 1000 -- overflows here
{ key: "third", text: "c".repeat(10) }, // would individually fit (500+10=510 <= 1000), but must NOT be
// included: a hard priority cutoff, not a bin-packing optimization that skips a large blocked section
// to squeeze in a smaller lower-priority one.
],
0,
1000,
);
expect(included).toEqual(new Set(["first"]));
});

it("skips an absent (undefined) section without consuming budget or affecting later decisions", () => {
const included = selectContextSectionsWithinBudget(
[
{ key: "present-1", text: "a".repeat(400) },
{ key: "absent", text: undefined },
{ key: "present-2", text: "b".repeat(400) },
],
0,
1000,
);
expect(included).toEqual(new Set(["present-1", "present-2"]));
});

it("includes a section landing exactly on the budget boundary, excludes one that overflows by a single character", () => {
const exact = selectContextSectionsWithinBudget([{ key: "a", text: "x".repeat(8) }], 0, 10); // 0+8+2=10 <= 10
expect(exact).toEqual(new Set(["a"]));
const over = selectContextSectionsWithinBudget([{ key: "a", text: "x".repeat(9) }], 0, 10); // 0+9+2=11 > 10
expect(over).toEqual(new Set());
});

it("accounts for chars already used (e.g. the diff/description) before evaluating the first section", () => {
const included = selectContextSectionsWithinBudget([{ key: "a", text: "x".repeat(100) }], 950, 1000); // 950+100+2 > 1000
expect(included).toEqual(new Set());
});
});

describe("buildUserPrompt aggregate context budget (#3900)", () => {
const budgetBaseInput: GittensoryAiReviewInput = {
repoFullName: "owner/repo",
prNumber: 1,
title: "PR",
diff: "diff content",
mode: "advisory",
};

it("includes every optional section when everything is enabled but comfortably under budget", () => {
const user = buildUserPrompt({
...budgetBaseInput,
grounding: { promptSection: "GROUNDING-SECTION" },
ragContext: "RAG-SECTION",
impactMapContext: "IMPACT-MAP-SECTION",
enrichment: { promptSection: "ENRICHMENT-SECTION" },
cultureProfileContext: "CULTURE-PROFILE-SECTION",
changedFiles: [{ path: "src/a.ts" }],
});
expect(user).toContain("GROUNDING-SECTION");
expect(user).toContain("RAG-SECTION");
expect(user).toContain("IMPACT-MAP-SECTION");
expect(user).toContain("ENRICHMENT-SECTION");
expect(user).toContain("CULTURE-PROFILE-SECTION");
expect(user).toContain("zero test-path evidence");
});

it("drops the lowest-priority sections first when every section enabled together would exceed the aggregate budget", () => {
// Sized so grounding+RAG survive (highest priority) but impact-map/enrichment/culture-profile/test-evidence
// -- everything below RAG in priority order -- get cut once the running total would overflow.
const grounding = "G".repeat(150_000);
const rag = "R".repeat(40_000);
const impactMap = "I".repeat(20_000);
const user = buildUserPrompt({
...budgetBaseInput,
grounding: { promptSection: grounding },
ragContext: rag,
impactMapContext: impactMap,
enrichment: { promptSection: "ENRICHMENT-SECTION" },
cultureProfileContext: "CULTURE-PROFILE-SECTION",
changedFiles: [{ path: "src/a.ts" }],
});
expect(user).toContain(grounding);
expect(user).toContain(rag);
expect(user).not.toContain(impactMap);
expect(user).not.toContain("ENRICHMENT-SECTION");
expect(user).not.toContain("CULTURE-PROFILE-SECTION");
expect(user).not.toContain("zero test-path evidence");
expect(user.length).toBeLessThanOrEqual(AGGREGATE_CONTEXT_BUDGET_CHARS);
});

it("never trims grounding even with the diff at its own maximum size and grounding at its OWN real-world maximum (review-grounding.ts's 60k FILE_CONTENT_BUDGET)", () => {
const grounding = "G".repeat(60_000);
const user = buildUserPrompt({
...budgetBaseInput,
diff: "d".repeat(120_000),
grounding: { promptSection: grounding },
});
expect(user).toContain(grounding);
});
});

describe("REVIEW_SYSTEM_PROMPT performance-regression instruction (#2559)", () => {
it("instructs the model to treat a genuine algorithmic/performance regression as a blocker category", async () => {
const run = vi.fn(
Expand Down