Pular para conteúdo

AUR-475: one oversized file does not discard the rest of the queue

Documento histórico do board. Pode descrever componentes removidos; para uso atual, consulte o guia do produto.

The defect, as measured

Measured inside AUR-467 and left explicitly unfixed there because it conflicted with budgeting_test.go's fixture sizing at the time: TrimToFit (internal/prompt/budgeting.go) breakd out of its loop at the first segment that did not fit the remaining token budget, instead of skipping it and trying the next, possibly smaller, one.

Observed in the user's real repository: a single large file (AGENTS.md) consumed the budget, and the fifteen .mjs files in the same commit were discarded whole -- not truncated, discarded -- because they were sorted behind it and the loop had already stopped. AUR-467 removed prose from the queue, which fixed that specific diff, but the mechanic stayed live: any large CODE file sorting first still starves every file behind it.

This is the "coverage is a lottery" defect: the review reports success having looked at an arbitrary fraction of the diff.

The fix

TrimToFit now continues past a segment that doesn't fit instead of breaking, and drops the old "truncate the first segment if the result is still empty" branch entirely. A segment either fits whole or is skipped whole; it is never truncated. This satisfies both Non-goals at once: no truncation (AC-002), and no stopping early just because the first candidate didn't fit (AC-001) -- both are the same one-line change, because the old truncate-first-and-break branch was the other half of the same defect: it silently ate the budget on a partial copy of the first file and then broke anyway.

truncateSegment (budgeting.go) is left in place, still covered by its own direct unit test in budgeting_test.go; it is simply no longer reachable from TrimToFit's production path.

Why budgeting_test.go did not need to change

TestTrimToFit and TestTrimToFit_Deterministic both use segment sets where the segment that doesn't fit is last in priority/sort order, so skip-and-continue and stop-at-first produce an identical result: there is nothing smaller behind the non-fitting segment to reach. TestTruncateSegment exercises truncateSegment directly, not through TrimToFit, and still exercises the exact same function. No assertion changed.

The coordinator expanded paths to include one AUR-467 fixture

tests/unit/AUR-467.go::Blocker2_PartialHunkNeverSilentlyComplete sized a two-hunk file so hunk0 "fits" and hunk1 does not, at a budget tight enough that hunk0's fit relied on the old truncate-first-and-break branch: at MaxTokens=1470 the coverage-reservation math left roughly 17 tokens of content budget, less than hunk0's own 35 tokens, so hunk0 did not actually fit whole either. The old code truncated it, appended the truncated copy, and the test's strings.Contains(parts.User, "hunk0") check could not tell the difference (the truncation only cuts the tail, and hunk0's marker sits at the front of the line). The fixture encoded the defective behavior, not the contract it names.

Under this card's fix a segment that does not fit whole is never admitted even partially, so at the old MaxTokens=1470 hunk0 is correctly skipped too, and the file classifies omitted (0/2) instead of partial (1/2) -- which broke the test, since it could no longer reach the partial case it exists to exercise.

What changed, and what did not. The coordinator confirmed the failure independently, expanded this card's paths to include tests/unit/AUR-467.go with a written justification in the card, and drew the line explicitly: the assertion that a partially-covered file must never report as complete is correct and stays untouched -- every t.Fatalf check in testAUR467PartialHunkNeverSilentlyComplete is byte-for-byte the same as before. The only edit is MaxTokens, from 1470 to 1510, plus a comment explaining why. At 1510 the content budget is large enough for hunk0's real 35 tokens (so it survives whole, honestly, not truncated) but still too small for hunk0 plus hunk1's 35 more (so hunk1 is correctly skipped) -- measured directly: hunk0 present, hunk1 absent, code_files_partial=1, code_files_omitted=0. The partial case is exercised for real now, instead of by an accident of the bug this card removes.

Confirmed after the fix: tests/acceptance/AUR-467.sh::TestAUR467 exits 0 (the previously-failing subtest passes again), tests/acceptance/AUR-467.sh::AC-001 still exits 0 (it never touched this subtest), and this card's own AC-001 and the sealed oci-run both still exit 0.

Selectors, each its own program

  • tests/unit/AUR-475.go::TestAUR475 -- AC-001 (later smaller files still reviewed), AC-002 (no truncation marker, no cut-mid-line content), AC-003 (hunk-level partial vs. omitted classification) against BuildPrompt directly.
  • tests/integration/AUR-475.go::IntegrationAUR475 -- the same fix at the raw TokenBudget.BuildContextSegments/TrimToFit primitive boundary: order-independence with oversized segments interleaved front/middle/back, proof that an included segment's Tokens/Content are never altered, and Meta arithmetic consistency across three budgets.
  • tests/e2e/AUR-475.sh::E2EAUR475 -- no-regression control: the real binary against tests/fixtures/repos/git-demo/repo.git still reports its fixture finding and closes --fail-on.
  • tests/acceptance/AUR-475.sh::AC-001 -- a generated program calls the real BuildPrompt against a diff shaped like the fix's target case and greps its output; no git, no python3. AC-001-MUT-001 reverts continue to break (the card's own skeptical mutation); AC-002-MUT-002 re-introduces truncation. Both must go red.