Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import jakarta.inject.Inject;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -146,7 +147,11 @@ ReviewResult build(
var diffStats =
DiffStats.fromFiles(overviewFiles, omitted, truncation)
.withAuthoritativeTotals(ctx.prTotals());
var omittedNames = Set.copyOf(truncation.omittedFileNames());
// Rows are dropped for every never-reviewed class β€” planned/runtime omissions AND ceiling
// skips (the detail keeps them separate only so the disclosure names the ceiling) β€” while
// clipped and response-cut files keep theirs: those were partially reviewed.
var omittedNames = new HashSet<>(truncation.omittedFileNames());
omittedNames.addAll(truncation.spendCeilingSkippedFileNames());
var changedFiles =
toChangedFiles(
overviewFiles.stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,32 @@ void budgetOmittedFilesAreDroppedFromTheWalkthroughRows() {
assertTrue(rowsCaptor.getValue().isEmpty(), rowsCaptor.getValue().toString());
}

@Test
void ceilingSkippedFilesAreDroppedFromTheWalkthroughRowsLikeOtherNotReviewedFiles() {
// #515 β€” a file skipped at the token spend ceiling was never reviewed at all: the banner says
// so and the model-facing overview lists it as not reviewed, so its walkthrough row must be
// dropped like every other not-reviewed class β€” only partially reviewed files (clipped,
// response-cut) keep their rows.
var ctx =
contextWithLineCapOmissions(
0,
List.of(
new FileDiff("reviewed.java", "modified", 1, 0, 1, ""),
new FileDiff("skipped.java", "modified", 1, 0, 1, "")));
var plan = new DiffBudgetPlanner.BudgetPlan(List.of(), List.of(), List.of(), true);
plan.recordSpendCeilingSkippedFiles(List.of("skipped.java"));
var rowsCaptor = ArgumentCaptor.forClass(List.class);

builder.build(ctx, CLEAN_RESPONSE, CI_CLEAR, plan);

verify(summaryGenerator)
.generate(anyInt(), anyInt(), anyInt(), rowsCaptor.capture(), any(), any());
assertEquals(
List.of(new PrSummaryGenerator.ChangedFile("reviewed.java", "modified")),
rowsCaptor.getValue(),
"a ceiling-skipped (never reviewed) file must not keep its walkthrough row");
}

/**
* #471 β€” the walkthrough filter is the third site that asks an immutable name set whether it
* holds a {@code FileDiff.filename()} that Jackson never validated. {@code contains(null)} throws
Expand Down
Loading