Skip to content
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ All notable changes to ThrillhouseBot.
- **An empty server-sent event in a streamed review does not reach the Vert.x uncaught-exception handler, and a test now holds that through the real client** (#237): this is the older report of the throw #555 fixed in 0.6.0, where a payload-less event (a keep-alive comment, a bare `data:`, a stray blank line) was handed to Jackson on the event loop and logged as `Uncaught exception received by Vert.x` while the review went on to complete. The report was parked waiting on an upstream fix; there is none. `OpenAiRestApi.OpenAiRestApiReaderInterceptor` in quarkus-langchain4j 1.13.1 is the 1.11.2 code line for line and still passes the empty payload straight to the mapper, so the dependency bumps since did not change the behaviour and the in-app reader interceptor from #555 remains the fix. What #555 proved around one dispatched event is now proved on a real stream: the `OpenAiRestApi` client reads from a loopback provider that sends all three filler shapes between two real chunks, both chunks arrive, and nothing reaches the Vert.x handler. With the interceptor reduced to a pass-through the same stream reports four uncaught mapping failures while still delivering both chunks, the exact shape the report described
- **The on-request commands no longer render a whole-PR diff that nothing reads** (#474): since #457 `/describe`, `/changelog`, `/improve` and `/generate-tests` send the model per-batch renders planned from the file list, but `loadInputs` still built the `max-diff-lines`-capped render of the whole pull request on every call, hundreds of kilobytes on a large one, and read it once to ask whether it was blank. It cannot be blank whenever the file list is not, so a pull request whose every changed file was ignored or a pure rename passed the check, had its title, body and instructions loaded, and only then stopped on an empty batch plan. The reviewable-file list is the signal now: an empty one ends the command before anything else is fetched, and the posted outcome is unchanged in every case. The `diff` field is gone from the shared `Inputs` record, where its meaning had come to depend on the command that built it (empty was the abort signal on four commands and the normal value on `/add-docs`)
- **A length stop with no content is repeated once with reasoning disabled instead of failing the review** (#839): a streamed review call that ends with `finish_reason=length` and an empty content body spent its whole output allowance on reasoning and never began the answer. Three production reviews in a row failed that way at `AI_REASONING_EFFORT=max`, each billed for 65536 output tokens of nothing, and the maintainer got "could not be completed". The retry loop treated every length stop as deterministic and gave up after one attempt, which is right for an answer that outgrew the cap and wrong for one that never started. The two shapes are now told apart by the content length. A stop with content keeps the no-retry salvage path. A stop with none is repeated once with `reasoning_effort=none` for that call only, the step-down is logged at WARN with the input and output token counts, and the review's scope note says it ran with reasoning disabled. The repeat goes straight to reasoning off rather than one tier down: on the provider where this was measured the tiers barely change the reasoning length, so a tier down burns the cap again. The configured effort is not changed and the next review starts at it; a repeat that also stops at the cap fails as before. The final summary call shares the loop and gets the same treatment; the blocking verifier and reply calls do not stream through it and keep their fail-open handling
- **A coverage artifact that uploads the HTML report beside `jacoco.xml` is read, and one that is refused says so** (#813): the archive walk charged every file entry to the 512-entry cap, not only the `.xml` entries that can carry a report, so uploading `target/site/jacoco/` (one `.html` per class plus the stylesheet, images and script) crossed the cap on a project of a few hundred classes and the whole artifact was refused, with a DEBUG line as the only trace. Only `.xml` entries count toward the cap now; every other entry is still drained in full against the 128 MB aggregate budget from #789, so the zip-bomb bound is unchanged. A refusal is logged at WARN with the counts, and the review summary's scope note says the configured coverage artifact was not read and why. Latent until `thrillhousebot.review.patch-coverage.enabled` is turned on
- **The `:latest` gate ignores pre-release and floating tags, and records its decision** (#733): `promote` found the highest existing release with `sort -V`, which ranks `v0.6.8-rc1` and `nightly` above `v0.6.8`, so a repository carrying either would have read a legitimate release as older than the highest one, and `publish-docs` and `bump-version` would have been skipped with nothing in the run to say so, which is the silent miss #717 set out to end. Only `vMAJOR.MINOR.PATCH` releases take part in the comparison now, and the outcome, promoted or held back and which version it lost to, is written to the run summary and a notice annotation either way. A refused docs dispatch still fails the job. Latent here, since the repository has no such tags
- **A truncation's concise flag can no longer be re-marked apart from its message** (#600): `AiResponseTruncatedException#implicatingConciseModel()` flipped the flag and kept the message, which is how a summary-lane truncation once told the operator to raise a knob its own flag said did not apply (#581). #595 moved both the remedy and the flag into `ModelLane#truncation`, and the method has had no production caller since. It is removed, so the two are set together at construction or not at all
- **The truncation javadocs describe what the lanes carry** (#600): `TruncatedResponseSalvager#salvage` and `AiResponseTruncatedException` still said the blocking lanes buffer no body, which #592 made false when it put `Result#content()` on the truncation for exactly those lanes. A reader was being told salvage is impossible where it now runs. Both say that every lane carries the text it had before the cut and that a null body is a call that produced none
Expand Down
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -751,6 +751,13 @@ Nothing about coverage is ever inferred from the diff, and a line's *absence* fr
list is explicitly not evidence that a test covers it. Files the ignore list already
excluded are never reported as under-tested.

Uploading the whole `target/site/jacoco/` tree, with the HTML report beside `jacoco.xml`,
is fine: only `.xml` entries count toward the archive's 512-entry cap. An artifact the
bot found but refused to read β€” more `.xml` entries than that, or one that inflates past
128 MB β€” is named in the review summary's scope note the way an ignore glob that matched
nothing is. An artifact that was never uploaded, has expired, or holds no JaCoCo XML is
still the quiet case above: nothing was refused, so nothing is disclosed.

The file is read from the repository's default branch on each review and cached for
five minutes. YAML anchors, aliases and merge keys are resolved; a document that is
oversized, nested absurdly deep, or built on a runaway alias expansion is refused whole.
Expand Down
Loading
Loading