refactor(review): model summary degradation as one enum - #531
Merged
Conversation
summaryResponseCut and summarySkippedAtCeiling were two mutually exclusive booleans set on disjoint control paths, which made the meaningless both-true state representable, forced ordered else-if chains at the render sites, and duplicated the whole AtomicBoolean recorder/accessor/defensive-snapshot apparatus on BudgetPlan. Replace the pair with a single SummaryDegradation enum (NONE / RESPONSE_CUT / SKIPPED_AT_CEILING): - BudgetPlan carries one AtomicReference slot with one recorder (recordSummaryDegradation), one value accessor and one defensive snapshot accessor — canonical constructor shrinks to 8 args - TruncationDetail carries one enum component — canonical constructor shrinks to 5 args; isEmpty/hasFileGaps guards preserved verbatim - the render sites (review banner, check-run suffix, coverage-gap clause, check-run brief) switch on the enum Behavior-preserving: every rendered string stays byte-identical; the full suite is green with only mechanical test migrations.
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
🤖 ThrillhouseBot PR SummaryWhat this PR doesReplaces the mutually-exclusive summaryResponseCut/summarySkippedAtCeiling boolean pairs on BudgetPlan and TruncationDetail with a single SummaryDegradation enum (NONE/RESPONSE_CUT/SKIPPED_AT_CEILING), collapsing the two AtomicBoolean recorder/accessor/defensive-snapshot apparatuses into one AtomicReference slot. Recording sites in FindingPipeline pass the matching enum constant, and render sites in VerdictBuilder and ReviewResult switch on the enum instead of ordered else-if chains, preserving all rendered output byte-for-byte while making the both-at-once degradation state unrepresentable. Control-Flow Diagram🔀 Show diagramflowchart TD
A["Summary call finishes"]
A --> B{"Degradation lane?"}
B -- "response cut at length cap" --> C["recordSummaryDegradation(RESPONSE_CUT)"]
B -- "skipped at token spend ceiling" --> D["recordSummaryDegradation(SKIPPED_AT_CEILING)"]
B -- "complete" --> S
C --> S["BudgetPlan single slot (AtomicReference)"]
D --> S
S --> T["TruncationDetail.summaryDegradation"]
T --> U{"hasFileGaps?"}
U -- "yes" --> V["Coverage banner plus gap clauses"]
U -- "no" --> W{"Switch on degradation"}
W -- "RESPONSE_CUT" --> X["SUMMARY_CUT_NOTICE and shortened suffix"]
W -- "SKIPPED_AT_CEILING" --> Y["SUMMARY_SKIPPED_NOTICE and skipped suffix"]
W -- "NONE" --> Z["Markdown unchanged"]
Changes Overview
Changed Files
Risk Assessment
No new issues found in this PR, but the review cannot be approved until CI is confirmed green.
|
| Check | Type | Status | Detail |
|---|---|---|---|
| trivy | check-run | ⏳ Pending | - |
| changes | check-run | ⏳ Pending | - |
| format | check-run | ⏳ Pending | - |
| frontend | check-run | ⏳ Pending | - |
| actionlint | check-run | ⏳ Pending | - |
| test | check-run | ⏳ Pending | - |
| dependency-review | check-run | ⏳ Pending | - |
Automated review by ThrillhouseBot. Reply with /review to re-run.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
This was referenced Aug 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
Description
summaryResponseCutandsummarySkippedAtCeilingwere two mutually exclusive booleans set on disjoint control paths (salvagedOrCountsOnlySummaryvscountsOnlySummary). The pair made the meaningless both-true state representable, forced orderedelse ifchains at the render sites, and duplicated the entire AtomicBoolean recorder/accessor/defensive-snapshot apparatus onBudgetPlan.This replaces the pair with a single
SummaryDegradationenum (NONE/RESPONSE_CUT/SKIPPED_AT_CEILING):AtomicReference<SummaryDegradation>component with one recorder (recordSummaryDegradation), one value accessor (summaryDegradation()) and one defensive snapshot accessor, replacing both AtomicBoolean apparatuses — the canonical constructor shrinks from 9 to 8 args.isEmpty()/hasFileGaps()guards (and thetruncationDisclosuregap-only guard built on them) are preserved verbatim: a degradation still defeatsisEmpty(), still does not create a file gap.VerdictBuilder's banner selection andtruncationSuffixFor, plus the sharedReviewResult.coverageGapClauseandcoverageGapBrief.FindingPipelinepass the matching constant; a single slot makes the impossible cut-and-skipped-at-once state unrepresentable by construction.Related Issues
Fixes #527
How Has This Been Tested?
Behavior-preserving: every rendered string stays byte-identical — no rendering test's expected strings changed (the test diff touches only constructor arities, recorder calls, and flag-accessor assertions, all migrated mechanically to the enum). Full suite green: 2594 tests, 0 failures (one fewer than before only because the four per-boolean plan-apparatus tests consolidated into three enum-slot tests covering the same contracts: live recording, defensive snapshot, caller-supplied slot staying live, and no approval hold).
Gates:
spotless:applyclean;clean compile spotbugs:check spotless:check— BugInstance size is 0; jacoco ∩ diff on all changed main code (including the newSummaryDegradation.java): 0 missed instructions, 0 missed branches.Checklist
Screenshots / Logs
N/A
Additional Notes
Originally stacked on #530 (the degenerate-lane ceiling catch, whose recording site this refactor also migrates); #530 has since merged, so this targets
release/v0.6.0directly, rebased on top of it. Diff is scoped to #527 only.