fix(review): disclose a summary-response cut in the posted review

[devops-thiago]

What type of PR is this?

• 🐛 Bug fix

Description

Follow-up to #511. The spend-ceiling degradation of the summary lane discloses itself in the posted review, but the length-cap truncation of the same lane was log-only — two sibling degradations of one lane, one visible, one mute.

• BudgetPlan gains a live summaryResponseCut flag (same shared-accumulator pattern as the runtime-gap lists, with a defensive-snapshot accessor); FindingPipeline records it in both branches of salvagedOrCountsOnlySummary, including the degenerate summarizeWithoutReview path.
• ReviewResult.TruncationDetail gains a matching fifth component (back-compat constructors keep the old shapes). With file-coverage gaps present the cut folds into the partial-review clause, naming both knobs (max-output-tokens / REVIEW_CONCISE_MAX_OUTPUT_TOKENS); with no file gap a dedicated "Summary shortened" banner renders instead and does not hold approval, since the findings themselves are complete. The check-run summary carries the matching brief marker on every return path, including reviews with findings.
• The check-run suffix derivation is extracted into truncationSuffixFor and the banner constant becomes a text block, keeping checkSummaryForResult under Sonar's complexity gate with no nested ternary.

Related Issues

Refs #500 (follow-up; the issue itself was closed by #511)

How Has This Been Tested?

• Unit tests

Red/green proven: with the wiring stashed out, all five new behavioral tests fail on exactly the new assertions (flag not recorded, banner absent, clause absent); with the wiring in place the full suite passes (2566 tests, 0 failures). Jacoco ∩ diff shows every changed line and branch covered. SpotBugs and Spotless are clean. The text-block conversion was verified character-identical to the previous concatenation.

Checklist

• My code follows the project's coding standards
• I have performed a self-review of my own code
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• I have updated the documentation accordingly
• My changes generate no new warnings or errors

Screenshots / Logs

Summary-only cut, posted review banner:

⚠️ Summary shortened. The model's summary response was cut at its length cap (max-output-tokens / REVIEW_CONCISE_MAX_OUTPUT_TOKENS) — the findings themselves are complete.

Combined with file gaps, one clause of the partial-review banner:

…, and the summary was shortened because the model's response was cut at its length cap (max-output-tokens / REVIEW_CONCISE_MAX_OUTPUT_TOKENS) — the findings themselves are complete

Additional Notes

A summary-only cut deliberately does not hold approval: the findings are complete, so downgrading APPROVE would punish a prose-only degradation. File-gap classes keep holding approval exactly as before.

An earlier revision of this description claimed the #511-round Sonar cleanups (retryBatch extraction, unnamed catch variable, assertThrows hoists) were part of this diff — they were not; they landed with #511's squash. Credit to the bot review for catching the drift.

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[thrillhousebot]

🤖 ThrillhouseBot PR Summary

What this PR does

Adds a live summaryResponseCut flag to BudgetPlan, records it when the summary call is truncated in FindingPipeline, carries it into ReviewResult.TruncationDetail, and renders a dedicated 'Summary shortened' banner or folds it into the partial-review clause, with matching check-run markers; also extracts retryBatch and other Sonar cleanups.

⚠️ Description vs. Implementation

The PR description does not fully match the change:

• Claimed 'with no file gap a dedicated Summary shortened banner renders instead' — in the diff the flag is only carried into TruncationDetail when plan.truncated() is true, so a summary-only cut (truncated() false) yields EMPTY and neither the banner nor the check-run marker renders.
• Claimed 'the check-run summary carries a matching brief marker in both shapes' — the findings-present branch of checkSummaryForResult still returns before appending truncationSuffix, so a review with findings loses the marker.

Control-Flow Diagram

🔀 Show diagram

flowchart TD
    A["FindingPipeline: summarize() truncated"] --> B["salvagedOrCountsOnlySummary()"]
    B --> C["plan.recordSummaryResponseCut()"]
    C --> D["VerdictBuilder.build()"]
    D --> E{"plan.truncated()?"}
    E -- "yes" --> F["TruncationDetail(..., plan.summaryWasCut())"]
    E -- "no" --> G["TruncationDetail.EMPTY"]
    F --> H{"summaryResponseCut?"}
    H -- "file gap present" --> I["coverageGapClause folds summary clause"]
    H -- "no file gap" --> J["SUMMARY_CUT_NOTICE banner"]
    G --> K["summary-only cut never disclosed"]

Loading

Changes Overview

• Files changed: 8
• Lines added: +326
• Lines removed: -28

Changed Files

File	Change	Summary
src/main/java/dev/thiagogonzaga/thrillhousebot/review/DiffBudgetPlanner.java	Modified	Adds live AtomicBoolean summaryResponseCut to BudgetPlan with snapshot accessor and convenience constructors.
src/main/java/dev/thiagogonzaga/thrillhousebot/review/FindingPipeline.java	Modified	Records plan.summaryResponseCut on both summary-truncation paths, including summarizeWithoutReview.
src/main/java/dev/thiagogonzaga/thrillhousebot/review/ReviewResult.java	Modified	Adds summaryResponseCut to TruncationDetail, SUMMARY_CUT_NOTICE, and coverage clause/brief entries.
src/main/java/dev/thiagogonzaga/thrillhousebot/review/VerdictBuilder.java	Modified	Wires summaryResponseCut into TruncationDetail and adds summary-only check-run and banner rendering.
src/test/java/dev/thiagogonzaga/thrillhousebot/review/DiffBudgetPlannerTest.java	Modified	Adds tests for live flag recording and defensive-snapshot accessor.
src/test/java/dev/thiagogonzaga/thrillhousebot/review/FindingPipelineTest.java	Modified	Asserts summary cut is recorded on plan in truncation paths and disjoint from batch cuts.
src/test/java/dev/thiagogonzaga/thrillhousebot/review/ReviewResultTest.java	Modified	Adds tests for coverage clause/brief summary-cut entries and isEmpty behavior.
src/test/java/dev/thiagogonzaga/thrillhousebot/review/VerdictBuilderTest.java	Modified	Adds tests for summary-only banner, clause folding, and findings-present check summary.

Risk Assessment

Risk	Count
🔴 Critical	0
🟠 High	1
🟡 Medium	0
🔵 Low	0

Previous Findings Status

Status	Count
✅ Resolved	0
⚠️ Still present	0
💬 Justified	0
🗂️ Superseded (targeted code left the diff)	1

Key Findings

• HIGH: Summary-only cut flag is dropped because it is only propagated when plan.truncated() is true (src/main/java/dev/thiagogonzaga/thrillhousebot/review/VerdictBuilder.java:137)

---

Automated review by ThrillhouseBot. Reply with /review to re-run.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[thrillhousebot]

ThrillhouseBot requested changes — see inline comments on the diff.

[devops-thiago]

/review

---

Generated by Claude Code