Skip to content
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ All notable changes to ThrillhouseBot.

### Fixed

- **The three recurring container defects are graded the same way in every pull request** (#773): a round of dogfood scoring read 65 findings over twelve pull requests against the code β€” 62 of them true β€” and found the grading, not the findings, to be the problem. The same unpinned base image drew medium on four of those pull requests and low on three, with nothing in the changes to tell them apart. A root-owned volume under a non-root `USER` drew low on the pull request where the container dies on its first write and medium on another of the same shape. The same omission drew a confident inline finding in one half of a paired-language change and a low-confidence collapsed item in the other, on evidence that was equally provable from the diff in both. A severity that moves with the review rather than with the defect teaches a reader that the field carries no information. Three classes now carry a stated grade, decided by what the defect costs and who it reaches rather than by the review's impression of it: a mutable external reference (a base image, action or chart named by a tag instead of a digest) is medium, a path the running user cannot write is high, and a container that never drops privilege is medium, each at medium confidence. The review prompt states them, and a calibration stage applies them after the finding verifier, so a downgrade there cannot re-spread the class. It regrades only a finding anchored in a Dockerfile, manifest, compose file, workflow or Terraform file that states the class in its own words, and it leaves alone any finding that also asserts something the class does not cover β€” a privileged container, a host mount or namespace, an added capability, a credential, a named CVE. Nothing is dropped, added or reworded, so the finding set is exactly the one the review produced. Medium confidence puts every anchored finding on the diff instead of in the collapsed "Things to double-check" block, and leaves the merge verdict to the rest of the review: under the default `REVIEW_BLOCKING_STRICTNESS=balanced`, which needs high confidence, no anchored finding requests changes on its own, and one the review over-graded at critical with high confidence stops doing so, which is the calibration working rather than a side effect of it. The prompt also states the mirror of the severity rule for confidence, which is the half no deterministic stage can decide: equally provable defects get equal confidence, and an unequal one must name the fact it could not check
- **Repeated timeouts on one AI call stop spending the whole retry budget** (#862): a streaming attempt waits `THRILLHOUSEBOT_REVIEW_AI_TIMEOUT_SECONDS`, 900 seconds in production, and a timeout was then retried like any other transient failure up to `THRILLHOUSEBOT_REVIEW_MAX_AI_RETRIES`, so one call could spend 75 minutes of wall clock while its review held the pull request's dispatcher slot. Production saw 20 timed-out attempts in 24 hours, all on one 503-file pull request, and the failed reviews of that day cost more than the completed ones. At most two attempts of one call may now end at the deadline: the second one fails the call instead of spending the attempts that are left, and the decision is logged at WARN with the session id, the attempt and the wait. Every other transient failure keeps the whole budget, a timeout followed by a successful attempt still succeeds, and the reasoning step-down's repeat (#839) shares the bound rather than getting a second pair of waits. The repeat keeps the full deadline, since it is there for the attempt whose first token never arrived because the provider queued the request, and the bound already brings the ceiling down from 75 minutes to 30. The setting still means one attempt's wait. The final summary call shares the loop and behaves the same way, and a review whose batches time out still discloses the files it did not read
- **A review interrupted by a restart no longer stays `in_progress` for good** (#863): a session row is written `in_progress` when the review starts and updated when it ends, so a review killed between those two writes β€” a deploy restart, a crash, a `docker kill` β€” never got the terminal one and stayed `in_progress` for the life of the database. Production had 10 such rows, the oldest from 2026-06-09 and the newest from the 2026-09-09 restart, each counting as a running review and hiding the genuinely in-flight ones among them. Startup now moves every `in_progress` row to `failed` with "Review interrupted before it finished (bot restart or crash)" as the reason, which is what tells it apart on the dashboard from a review that failed on its own. Nothing carries a review across a restart, so a row still in progress at boot belongs to a review that is over: the sweep needs no age threshold and reconciles the rows stranded before it existed, with no manual SQL. Only the status and the reason are written, so the tokens and the cost the review had already paid for stay on the row. Sweeping every row at boot is safe because the bot is a single process; running more than one replica would need the rows to carry an owner first

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,11 @@
import org.jboss.logging.Logger;

/**
* The post-AI finding chain: validate quotes, dedupe, verify against the diff, drop already-replied
* duplicates, backfill missing content anchors, and persist the response. Extracted from {@code
* ReviewOrchestrator}; the ordering is preserved verbatim β€” quote validation runs before dedupe so
* a merged finding cannot inherit a phantom quote while a verbatim sibling is discarded.
* The post-AI finding chain: validate quotes, dedupe, verify against the diff, calibrate the graded
* fields, drop already-replied duplicates, backfill missing content anchors, and persist the
* response. Extracted from {@code ReviewOrchestrator}; the ordering is preserved verbatim β€” quote
* validation runs before dedupe so a merged finding cannot inherit a phantom quote while a verbatim
* sibling is discarded.
*/
@ApplicationScoped
public class FindingPipeline {
Expand Down Expand Up @@ -1444,6 +1445,10 @@ private ReviewResponse refine(
promptInputs.previousFindings(),
located,
plan::recordVerificationCoverage);
// #773: the last word on the two graded fields, so the anchored infrastructure classes cannot
// be re-spread by the verifier's own lowering, and the grade the publisher routes on is the
// one persisted below for the next round to compare against.
aiResponse = SeverityCalibrator.calibrate(aiResponse);
aiResponse =
followUpAnalyzer.dropRepliedDuplicates(
aiResponse, ctx.priorAiResponseJsons(), ctx.inlineComments(), botIdentity);
Expand Down
Loading
Loading