fix(flue-review): elide oversized diff sections before staging for the model - #2393
Conversation
|
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-playground | 3ca1027 | Aug 10 2026, 06:57 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-do | 3ca1027 | Aug 10 2026, 06:57 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-cache | 3ca1027 | Aug 10 2026, 06:59 AM |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the right fix for the model-review outage: staging an elided diff preserves the review agent's workflow (the SKILL.md already tells the agent to read full files from the checkout) while preventing generated files from blowing up the context window. The size-based heuristic is simple, general, and fits the worker-only architecture.
I read the full diff, infra/flue-review/.flue/lib/diff-budget.ts, review.ts, the new tests, tsconfig.json, and package.json. Most of the change is clean and well-tested. However, I found one latent logic bug in elide(): it can return early without marking a section as reduced, which the total-budget loop assumes has happened. That can spin forever on a section whose format lacks a recognizable +++/@@ header. Since this PR is itself a fix for a production review failure, leaving a path to a Worker CPU-timeout hang is worth fixing before merge. A small refactoring and regression test cover it.
The package is private, so no changeset is required, and the i18n/RTL/SQL conventions in AGENTS.md don't apply to this internal worker. ES2023 target is configured, so Array.prototype.toSorted() typechecks correctly.
| export function elideLargeDiffSections(diff: string, budget: DiffBudget = {}): string { | ||
| const perFileBytes = budget.perFileBytes ?? DEFAULT_PER_FILE_BYTES; | ||
| const totalBytes = budget.totalBytes ?? DEFAULT_TOTAL_BYTES; | ||
| if (diff.length <= Math.min(perFileBytes, totalBytes)) return diff; | ||
|
|
||
| const sections = splitSections(diff); | ||
| for (const section of sections) { | ||
| if (section.text.length > perFileBytes) elide(section); | ||
| } | ||
| // Still over the total budget: elide the largest remaining sections until | ||
| // under it (or nothing left to elide). | ||
| let total = sections.reduce((n, s) => n + s.text.length, 0); | ||
| while (total > totalBytes) { | ||
| const next = sections | ||
| .filter((s) => !s.elided) | ||
| .toSorted((a, b) => b.text.length - a.text.length)[0]; | ||
| if (!next) break; | ||
| total -= next.text.length; | ||
| elide(next); | ||
| total += next.text.length; | ||
| } | ||
| return sections.map((s) => s.text).join(""); | ||
| } | ||
|
|
||
| function splitSections(diff: string): Section[] { | ||
| const starts: number[] = []; | ||
| const re = /^diff --git /gm; | ||
| for (let m = re.exec(diff); m; m = re.exec(diff)) starts.push(m.index); | ||
| if (starts.length === 0) return [{ text: diff, elided: false }]; | ||
| const sections: Section[] = []; | ||
| if (starts[0] !== 0) sections.push({ text: diff.slice(0, starts[0]), elided: true }); | ||
| for (let i = 0; i < starts.length; i++) { | ||
| const end = i + 1 < starts.length ? starts[i + 1] : diff.length; | ||
| sections.push({ text: diff.slice(starts[i], end), elided: false }); | ||
| } | ||
| return sections; | ||
| } | ||
|
|
||
| function elide(section: Section): void { | ||
| const lines = section.text.split("\n"); | ||
| // Keep the file header: everything up to and including the `+++` line (or | ||
| // the whole header for binary/rename-only sections with no hunks). | ||
| let headerEnd = lines.findIndex((line) => line.startsWith("+++ ")); | ||
| if (headerEnd === -1) headerEnd = lines.findIndex((line) => line.startsWith("@@ ")) - 1; | ||
| if (headerEnd < 0) return; | ||
| const body = lines.length - (headerEnd + 1); | ||
| section.text = [ | ||
| ...lines.slice(0, headerEnd + 1), | ||
| `(diff content elided: ${body} lines over the size budget -- read this file from the checkout instead)`, | ||
| "", | ||
| ].join("\n"); | ||
| section.elided = true; | ||
| } |
There was a problem hiding this comment.
[needs fixing] elide() can return without setting section.elided, and the loops assume every processed section is marked reduced. If a diff section lacks a +++/@@ header and is large enough to be selected by the total-budget while loop, it is selected again on the next iteration because section.elided never flips to true, so total never shrinks and the Worker spins until its execution limit. Malformed or unusual formats (e.g. a headerless file section, a very large mode-only diff, or a binary patch shaped differently than expected) are enough to trigger this.
The per-file loop also calls elide() on the preface section even though it was already flagged elided: true in splitSections, which is a hint that elided is being overloaded.
Fix by (1) guarding the per-file loop so it only tries to reduce sections that aren't already marked, and (2) making elide() always mark the section and fall back to replacing the whole section when no header boundary is found.
| export function elideLargeDiffSections(diff: string, budget: DiffBudget = {}): string { | |
| const perFileBytes = budget.perFileBytes ?? DEFAULT_PER_FILE_BYTES; | |
| const totalBytes = budget.totalBytes ?? DEFAULT_TOTAL_BYTES; | |
| if (diff.length <= Math.min(perFileBytes, totalBytes)) return diff; | |
| const sections = splitSections(diff); | |
| for (const section of sections) { | |
| if (section.text.length > perFileBytes) elide(section); | |
| } | |
| // Still over the total budget: elide the largest remaining sections until | |
| // under it (or nothing left to elide). | |
| let total = sections.reduce((n, s) => n + s.text.length, 0); | |
| while (total > totalBytes) { | |
| const next = sections | |
| .filter((s) => !s.elided) | |
| .toSorted((a, b) => b.text.length - a.text.length)[0]; | |
| if (!next) break; | |
| total -= next.text.length; | |
| elide(next); | |
| total += next.text.length; | |
| } | |
| return sections.map((s) => s.text).join(""); | |
| } | |
| function splitSections(diff: string): Section[] { | |
| const starts: number[] = []; | |
| const re = /^diff --git /gm; | |
| for (let m = re.exec(diff); m; m = re.exec(diff)) starts.push(m.index); | |
| if (starts.length === 0) return [{ text: diff, elided: false }]; | |
| const sections: Section[] = []; | |
| if (starts[0] !== 0) sections.push({ text: diff.slice(0, starts[0]), elided: true }); | |
| for (let i = 0; i < starts.length; i++) { | |
| const end = i + 1 < starts.length ? starts[i + 1] : diff.length; | |
| sections.push({ text: diff.slice(starts[i], end), elided: false }); | |
| } | |
| return sections; | |
| } | |
| function elide(section: Section): void { | |
| const lines = section.text.split("\n"); | |
| // Keep the file header: everything up to and including the `+++` line (or | |
| // the whole header for binary/rename-only sections with no hunks). | |
| let headerEnd = lines.findIndex((line) => line.startsWith("+++ ")); | |
| if (headerEnd === -1) headerEnd = lines.findIndex((line) => line.startsWith("@@ ")) - 1; | |
| if (headerEnd < 0) return; | |
| const body = lines.length - (headerEnd + 1); | |
| section.text = [ | |
| ...lines.slice(0, headerEnd + 1), | |
| `(diff content elided: ${body} lines over the size budget -- read this file from the checkout instead)`, | |
| "", | |
| ].join("\n"); | |
| section.elided = true; | |
| } | |
| for (const section of sections) { | |
| if (!section.elided && section.text.length > perFileBytes) elide(section); | |
| } |
| export function elideLargeDiffSections(diff: string, budget: DiffBudget = {}): string { | |
| const perFileBytes = budget.perFileBytes ?? DEFAULT_PER_FILE_BYTES; | |
| const totalBytes = budget.totalBytes ?? DEFAULT_TOTAL_BYTES; | |
| if (diff.length <= Math.min(perFileBytes, totalBytes)) return diff; | |
| const sections = splitSections(diff); | |
| for (const section of sections) { | |
| if (section.text.length > perFileBytes) elide(section); | |
| } | |
| // Still over the total budget: elide the largest remaining sections until | |
| // under it (or nothing left to elide). | |
| let total = sections.reduce((n, s) => n + s.text.length, 0); | |
| while (total > totalBytes) { | |
| const next = sections | |
| .filter((s) => !s.elided) | |
| .toSorted((a, b) => b.text.length - a.text.length)[0]; | |
| if (!next) break; | |
| total -= next.text.length; | |
| elide(next); | |
| total += next.text.length; | |
| } | |
| return sections.map((s) => s.text).join(""); | |
| } | |
| function splitSections(diff: string): Section[] { | |
| const starts: number[] = []; | |
| const re = /^diff --git /gm; | |
| for (let m = re.exec(diff); m; m = re.exec(diff)) starts.push(m.index); | |
| if (starts.length === 0) return [{ text: diff, elided: false }]; | |
| const sections: Section[] = []; | |
| if (starts[0] !== 0) sections.push({ text: diff.slice(0, starts[0]), elided: true }); | |
| for (let i = 0; i < starts.length; i++) { | |
| const end = i + 1 < starts.length ? starts[i + 1] : diff.length; | |
| sections.push({ text: diff.slice(starts[i], end), elided: false }); | |
| } | |
| return sections; | |
| } | |
| function elide(section: Section): void { | |
| const lines = section.text.split("\n"); | |
| // Keep the file header: everything up to and including the `+++` line (or | |
| // the whole header for binary/rename-only sections with no hunks). | |
| let headerEnd = lines.findIndex((line) => line.startsWith("+++ ")); | |
| if (headerEnd === -1) headerEnd = lines.findIndex((line) => line.startsWith("@@ ")) - 1; | |
| if (headerEnd < 0) return; | |
| const body = lines.length - (headerEnd + 1); | |
| section.text = [ | |
| ...lines.slice(0, headerEnd + 1), | |
| `(diff content elided: ${body} lines over the size budget -- read this file from the checkout instead)`, | |
| "", | |
| ].join("\n"); | |
| section.elided = true; | |
| } | |
| function elide(section: Section): void { | |
| section.elided = true; | |
| const lines = section.text.split("\n"); | |
| // Keep the file header: everything up to and including the `+++` line (or | |
| // the whole header for binary/rename-only sections with no hunks). | |
| let headerEnd = lines.findIndex((line) => line.startsWith("+++ ")); | |
| if (headerEnd === -1) headerEnd = lines.findIndex((line) => line.startsWith("@@ ")) - 1; | |
| if (headerEnd < 0) { | |
| section.text = `(diff content elided: ${lines.length} lines over the size budget -- read this file from the checkout instead)\n\n`; | |
| return; | |
| } | |
| const body = lines.length - (headerEnd + 1); | |
| section.text = [ | |
| ...lines.slice(0, headerEnd + 1), | |
| `(diff content elided: ${body} lines over the size budget -- read this file from the checkout instead)`, | |
| "", | |
| ].join("\n"); | |
| } |
Please also add a regression test in infra/flue-review/test/diff-budget.test.ts that passes a large headerless section and asserts the budget loop terminates and replaces the section instead of returning the original content.
github-actions
Bot
added
review/awaiting-author
There was a problem hiding this comment.
Pull request overview
Addresses Flue review failures caused by oversized staged unified diffs (e.g., large generated type files) by eliding large per-file diff sections and enforcing an overall diff-size budget before the model reads the diff from the workspace.
Changes:
- Add
elideLargeDiffSections()utility to shrink oversized per-file diff sections to headers + an elision notice, and optionally elide additional sections until under a total-size budget. - Apply the diff elision step when staging the unified diff into the review workspace.
- Add a vitest suite covering small-diff passthrough, per-file elision, total-budget elision, and header-only sections.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| infra/flue-review/.flue/lib/diff-budget.ts | Introduces diff section splitting + size-budget elision logic. |
| infra/flue-review/.flue/workflows/review.ts | Routes the fetched unified diff through the new elision utility before writing to DIFF_PATH. |
| infra/flue-review/test/diff-budget.test.ts | Adds tests validating elision behavior and invariants. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| function elide(section: Section): void { | ||
| const lines = section.text.split("\n"); | ||
| // Keep the file header: everything up to and including the `+++` line (or | ||
| // the whole header for binary/rename-only sections with no hunks). | ||
| let headerEnd = lines.findIndex((line) => line.startsWith("+++ ")); | ||
| if (headerEnd === -1) headerEnd = lines.findIndex((line) => line.startsWith("@@ ")) - 1; | ||
| if (headerEnd < 0) return; | ||
| const body = lines.length - (headerEnd + 1); | ||
| section.text = [ | ||
| ...lines.slice(0, headerEnd + 1), | ||
| `(diff content elided: ${body} lines over the size budget -- read this file from the checkout instead)`, | ||
| "", | ||
| ].join("\n"); | ||
| section.elided = true; | ||
| } |
| it("leaves a header-only section (no hunks) alone", () => { | ||
| const rename = [ | ||
| "diff --git a/old.ts b/new.ts", | ||
| "similarity index 100%", | ||
| "rename from old.ts", | ||
| "rename to new.ts", | ||
| "", | ||
| ].join("\n"); | ||
| const filler = fileSection("big.ts", 2_000); | ||
| const out = elideLargeDiffSections(rename + filler, { perFileBytes: 1_000, totalBytes: 1_500 }); | ||
| expect(out).toContain("rename from old.ts"); | ||
| }); | ||
| }); |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
github-actions
Bot
added
review/needs-rereview
There was a problem hiding this comment.
This still looks like the right fix for the model-review outage: putting the staged diff through a size budget before writing it to the workspace protects the agent’s first read without breaking the review workflow, because the agent SKILL.md already tells the reviewer to read full files from the checkout.
I re-read the full diff, infra/flue-review/.flue/lib/diff-budget.ts, infra/flue-review/.flue/workflows/review.ts, and the new tests. The prior review’s latent-infinite-loop concern has been addressed: elide() now marks the section as elided before attempting to reduce it, and the new "skips an unreducible headerless section instead of looping on it" test exercises the exact path. The total-budget loop can therefore always make progress and will terminate.
The rest of the implementation (per-file threshold, total-budget pass, header-preserving reduction, ES2023 toSorted() usage) is unchanged in spirit and matches the PR description. The package is private, so no changeset is required, and the AGENTS.md i18n/RTL/SQL/etc. conventions do not apply to this internal worker.
I found no remaining correctness, security, or convention issues. LGTM.
github-actions
Bot
added
review/approved
What does this PR do?
Fixes the review failure on #2392 (
model_reviewFlueError): the review agent's first step is a whole-file read of the staged unified diff, so a PR whose diff is dominated by large generated files (#2392 stages ~29k lines ofworker-configuration.d.ts) lands the entire diff in the model context and kills the model call.The staged diff now passes through
elideLargeDiffSectionsbefore being written to the workspace: any per-file section over 48 KiB is reduced to its header plus a note telling the agent to read that file from the checkout, and if the diff still exceeds a 384 KiB total budget the largest remaining sections are elided until it fits. Small diffs pass through byte-identical.Already deployed; this PR's own review runs on the fixed worker.
Closes #
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain.Changeset and i18n are n/a: private
infra/flue-reviewworker only. Typecheck run for the package (tsc --noEmitwith generated worker types); tests are the flue-review vitest suite (54 passing, four new for the elision behavior).AI-generated code disclosure
Screenshots / test output
Try this PR
Open a fresh playground →
A full working EmDash site, deployed from this branch. Each visit gets its own session-scoped sandbox: no login needed and no shared state. Try the admin, edit content, hit the public site.
Tracks
fix/flue-review-diff-budget. Updated automatically when the playground redeploys.