feat(review): add a file-by-file walkthrough table to the PR summary#179
Merged
Merged
Conversation
The first-run PR summary had a risk breakdown but no structured walkthrough, leaving reviewers to orient themselves on larger PRs from the raw diff. Add a "Changed Files" table (path, change type, one-line summary) so the summary mirrors the actual change set at a glance. The change type is taken authoritatively from the diff (FileDiff.status); the one-line description comes from the model and rides the existing review call, so there is no extra AI cost. The model emits a new `file_summaries` array (capped at 15 entries, most-impactful first); the renderer joins it to the diff's file list by path, so a file the model skips still appears (with "-" for its summary). The table is bounded to 20 rows with an "…and N more" overflow note to stay within GitHub's comment-size budget. Closes #39 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
🤖 ThrillhouseBot PR SummaryWhat this PR doesAdds a file-level walkthrough table to the PR summary comment, including change type labels and per‑file AI summaries, and updates the review pipeline to pass the changed files list. Changes Overview
Risk Assessment
No new issues found in this PR, but the review cannot be approved until the required checks are passing.
|
| Check | Type | Status | Detail |
|---|---|---|---|
| test | check-run | ⏳ Pending | - |
| Analyze (java-kotlin) | check-run | ⏳ Pending | - |
Automated review by ThrillhouseBot. Reply with /review to re-run.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #179 +/- ##
============================================
+ Coverage 99.14% 99.22% +0.07%
- Complexity 1718 1743 +25
============================================
Files 63 63
Lines 4585 4632 +47
Branches 643 647 +4
============================================
+ Hits 4546 4596 +50
+ Misses 10 8 -2
+ Partials 29 28 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Adds tests for the previously-uncovered branches in PrSummaryGenerator's changed-files walkthrough: null/blank/deleted/copied/changed change-type labels (case-insensitive), a null changedFiles list, and malformed or duplicate file_summaries (null/blank path, null summary, duplicate path keeping the first note). Brings patch coverage on the new code back to the project bar. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🤖 ThrillhouseBot PR SummaryWhat this PR doesAdds a file-by-file walkthrough table to the PR summary comment, deriving change types from the diff and per-file summaries from the AI reviewer. Changes Overview
Risk Assessment
No new issues found in this PR, but the review cannot be approved until the required checks are passing.
|
| Check | Type | Status | Detail |
|---|---|---|---|
| test | check-run | ⏳ Pending | - |
| Analyze (java-kotlin) | check-run | ⏳ Pending | - |
Automated review by ThrillhouseBot. Reply with /review to re-run.
|
devops-thiago
added a commit
that referenced
this pull request
Jun 23, 2026
Resolves conflicts from #62 (/changelog) against the commands that landed on the release branch since: /describe (#176), the file-by-file walkthrough (#179), /add-docs (#177), multi-line suggestions (#180), and the Mermaid diagram (#181). All conflicts were additive — both sides registered a new comment command. Kept both, ordering describe -> changelog -> add-docs across the enum, TriggerDetector patterns, CommentCommandService (imports, fields, constructor, switch, HELP_TEXT, handler methods), the README, and the two webhook tests. Also fixed a latent semantic break inherited from the merge: DocGenerationService (#177) constructs CreatePullRequestCommentRequest with 5 args, but multi-line suggestions (#180) extended that record to 7. The two PRs don't textually conflict, so the break slipped onto the release branch. A doc suggestion is always single-line, so start_line / start_side are passed as null. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
devops-thiago
added a commit
that referenced
this pull request
Jun 23, 2026
…179) - [x] ✨ Feature The first-run PR summary has a risk breakdown but no structured walkthrough, so reviewers have to orient themselves on larger PRs straight from the diff. This adds a **Changed Files** table to the summary comment — `path`, change type, and a one-line description per file — so the summary mirrors the actual change set at a glance. How it works: - **Change type** is taken authoritatively from the diff (`FileDiff.status`), mapped to a friendly label (Added / Removed / Renamed / Modified; unknown values pass through; a null status renders as "Changed"). - **One-line description** comes from the model via a new `file_summaries` array on the review JSON. It rides the existing review call, so there is **no extra AI cost**. The model is asked for the 15 most impactful files (skipping generated/lockfile noise). - The renderer **joins** the model's summaries to the diff's file list **by path**, so a file the model skips still appears in the table with `-` for its summary — the table always reflects the real change set. - The table is **bounded to 20 rows** with an "…and N more file(s)" overflow note, and all cells are pipe-escaped, to stay within GitHub's comment-size budget. Rendered output: ```markdown | File | Change | Summary | |------|--------|---------| | `src/A.java` | Modified | Adds null guard to parse() | | `src/B.java` | Added | New cache wrapper | ``` Implementation notes for reviewers: - `ReviewResponse.Summary` gains a `file_summaries` field (+ nested `FileSummary` record) with null-element filtering and back-compat convenience constructors, so existing callers/responses are unaffected. - `FindingVerificationService.recount` now preserves `file_summaries` when it rebuilds the summary after verification (otherwise the field would be silently dropped). - `PrSummaryGenerator.generate` takes a new `List<ChangedFile>` argument; `ReviewOrchestrator` projects the reviewed diff into it via `toChangedFiles` and threads it through `buildResult`. Closes #39 - [x] Unit tests New/updated unit tests cover: table rendering with per-file summaries, the `-` fallback when a file has no model summary, omitting the section when there are no files, row-bounding + overflow note, pipe escaping, change-type label mapping (including null/unknown), the `toChangedFiles` projection, plus `file_summaries` JSON deserialization, null-element filtering, and back-compat constructors. Full suite: **381 passing**. Spotless clean, SpotBugs (Max effort) reports 0 bugs. > Note: tests were run with the JaCoCo coverage agent disabled (`-Djacoco.skip=true -Dquarkus.jacoco.enabled=false`) because JaCoCo 0.8.15 crashes the JVM at startup on Java 25 — a pre-existing, unrelated environment issue. - [x] My code follows the project's coding standards - [x] I have performed a self-review of my own code - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] I have updated the documentation accordingly - [x] My changes generate no new warnings or errors No config flag — the walkthrough is part of the always-on summary content (like `pr_purpose`), and cost is bounded by the 15-file prompt cap and the 20-row render cap. Pairs with #54 (Mermaid control-flow diagram), which builds on the same `PrSummaryGenerator`.
devops-thiago
added a commit
that referenced
this pull request
Jun 23, 2026
Rebasing release/v0.3.0 onto main surfaced four breakages where release features predated changes that had already landed on main: - DocGenerationService: pass null start_line/start_side to CreatePullRequestCommentRequest, which gained multi-line range fields (#71) - ReviewOrchestratorTest: add the changedFiles arg to two generate() stubs for the file-by-file walkthrough (#179) - PrReviewPrompts: space out "{ path, summary }" so Qute — which now renders the full @UserMessage/@SystemMessage template (#184) — treats it as literal text instead of an expression - PrDescriptionGeneratorTest: assert marker neutralization instead of the "{|...|}" escaper wrapper that #186 removed main is the source of truth; these only adapt the release features to its APIs and behavior. compile, test-compile, all 1301 tests, and spotless:check pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
devops-thiago
added a commit
that referenced
this pull request
Jun 23, 2026
…edges Three low-severity robustness fixes surfaced by the v0.3.0 review: - PrSummaryGenerator.escapeTableCell now folds any run of line breaks into a space, so a model file-summary (or CI detail) containing a newline can no longer break the Markdown table row (#179). - DiffLineResolver.resolveSuggestionRange rejects a match that straddles a hunk boundary: the trimmed-text list is flat across hunks, so a run could span two hunks and yield a range covering lines absent from the diff (a GitHub 422). It now accepts the range only when every line in [startLine, endLine] is a real right-side line, else falls back to single-line (#71). - ChangelogEntryGenerator tolerates a decorated NONE sentinel (**NONE**, `NONE`, NONE., > NONE) so a declined draft is never posted as a literal entry, while a real entry that merely mentions the word still posts (#62). Tests added for each; full suite 1330 passing, Spotless clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
devops-thiago
added a commit
that referenced
this pull request
Jun 24, 2026
…179) - [x] ✨ Feature The first-run PR summary has a risk breakdown but no structured walkthrough, so reviewers have to orient themselves on larger PRs straight from the diff. This adds a **Changed Files** table to the summary comment — `path`, change type, and a one-line description per file — so the summary mirrors the actual change set at a glance. How it works: - **Change type** is taken authoritatively from the diff (`FileDiff.status`), mapped to a friendly label (Added / Removed / Renamed / Modified; unknown values pass through; a null status renders as "Changed"). - **One-line description** comes from the model via a new `file_summaries` array on the review JSON. It rides the existing review call, so there is **no extra AI cost**. The model is asked for the 15 most impactful files (skipping generated/lockfile noise). - The renderer **joins** the model's summaries to the diff's file list **by path**, so a file the model skips still appears in the table with `-` for its summary — the table always reflects the real change set. - The table is **bounded to 20 rows** with an "…and N more file(s)" overflow note, and all cells are pipe-escaped, to stay within GitHub's comment-size budget. Rendered output: ```markdown | File | Change | Summary | |------|--------|---------| | `src/A.java` | Modified | Adds null guard to parse() | | `src/B.java` | Added | New cache wrapper | ``` Implementation notes for reviewers: - `ReviewResponse.Summary` gains a `file_summaries` field (+ nested `FileSummary` record) with null-element filtering and back-compat convenience constructors, so existing callers/responses are unaffected. - `FindingVerificationService.recount` now preserves `file_summaries` when it rebuilds the summary after verification (otherwise the field would be silently dropped). - `PrSummaryGenerator.generate` takes a new `List<ChangedFile>` argument; `ReviewOrchestrator` projects the reviewed diff into it via `toChangedFiles` and threads it through `buildResult`. Closes #39 - [x] Unit tests New/updated unit tests cover: table rendering with per-file summaries, the `-` fallback when a file has no model summary, omitting the section when there are no files, row-bounding + overflow note, pipe escaping, change-type label mapping (including null/unknown), the `toChangedFiles` projection, plus `file_summaries` JSON deserialization, null-element filtering, and back-compat constructors. Full suite: **381 passing**. Spotless clean, SpotBugs (Max effort) reports 0 bugs. > Note: tests were run with the JaCoCo coverage agent disabled (`-Djacoco.skip=true -Dquarkus.jacoco.enabled=false`) because JaCoCo 0.8.15 crashes the JVM at startup on Java 25 — a pre-existing, unrelated environment issue. - [x] My code follows the project's coding standards - [x] I have performed a self-review of my own code - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] I have updated the documentation accordingly - [x] My changes generate no new warnings or errors No config flag — the walkthrough is part of the always-on summary content (like `pr_purpose`), and cost is bounded by the 15-file prompt cap and the 20-row render cap. Pairs with #54 (Mermaid control-flow diagram), which builds on the same `PrSummaryGenerator`.
devops-thiago
added a commit
that referenced
this pull request
Jun 24, 2026
Rebasing release/v0.3.0 onto main surfaced four breakages where release features predated changes that had already landed on main: - DocGenerationService: pass null start_line/start_side to CreatePullRequestCommentRequest, which gained multi-line range fields (#71) - ReviewOrchestratorTest: add the changedFiles arg to two generate() stubs for the file-by-file walkthrough (#179) - PrReviewPrompts: space out "{ path, summary }" so Qute — which now renders the full @UserMessage/@SystemMessage template (#184) — treats it as literal text instead of an expression - PrDescriptionGeneratorTest: assert marker neutralization instead of the "{|...|}" escaper wrapper that #186 removed main is the source of truth; these only adapt the release features to its APIs and behavior. compile, test-compile, all 1301 tests, and spotless:check pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
devops-thiago
added a commit
that referenced
this pull request
Jun 24, 2026
…edges Three low-severity robustness fixes surfaced by the v0.3.0 review: - PrSummaryGenerator.escapeTableCell now folds any run of line breaks into a space, so a model file-summary (or CI detail) containing a newline can no longer break the Markdown table row (#179). - DiffLineResolver.resolveSuggestionRange rejects a match that straddles a hunk boundary: the trimmed-text list is flat across hunks, so a run could span two hunks and yield a range covering lines absent from the diff (a GitHub 422). It now accepts the range only when every line in [startLine, endLine] is a real right-side line, else falls back to single-line (#71). - ChangelogEntryGenerator tolerates a decorated NONE sentinel (**NONE**, `NONE`, NONE., > NONE) so a declined draft is never posted as a literal entry, while a real entry that merely mentions the word still posts (#62). Tests added for each; full suite 1330 passing, Spotless clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
devops-thiago
added a commit
that referenced
this pull request
Jun 24, 2026
…179) - [x] ✨ Feature The first-run PR summary has a risk breakdown but no structured walkthrough, so reviewers have to orient themselves on larger PRs straight from the diff. This adds a **Changed Files** table to the summary comment — `path`, change type, and a one-line description per file — so the summary mirrors the actual change set at a glance. How it works: - **Change type** is taken authoritatively from the diff (`FileDiff.status`), mapped to a friendly label (Added / Removed / Renamed / Modified; unknown values pass through; a null status renders as "Changed"). - **One-line description** comes from the model via a new `file_summaries` array on the review JSON. It rides the existing review call, so there is **no extra AI cost**. The model is asked for the 15 most impactful files (skipping generated/lockfile noise). - The renderer **joins** the model's summaries to the diff's file list **by path**, so a file the model skips still appears in the table with `-` for its summary — the table always reflects the real change set. - The table is **bounded to 20 rows** with an "…and N more file(s)" overflow note, and all cells are pipe-escaped, to stay within GitHub's comment-size budget. Rendered output: ```markdown | File | Change | Summary | |------|--------|---------| | `src/A.java` | Modified | Adds null guard to parse() | | `src/B.java` | Added | New cache wrapper | ``` Implementation notes for reviewers: - `ReviewResponse.Summary` gains a `file_summaries` field (+ nested `FileSummary` record) with null-element filtering and back-compat convenience constructors, so existing callers/responses are unaffected. - `FindingVerificationService.recount` now preserves `file_summaries` when it rebuilds the summary after verification (otherwise the field would be silently dropped). - `PrSummaryGenerator.generate` takes a new `List<ChangedFile>` argument; `ReviewOrchestrator` projects the reviewed diff into it via `toChangedFiles` and threads it through `buildResult`. Closes #39 - [x] Unit tests New/updated unit tests cover: table rendering with per-file summaries, the `-` fallback when a file has no model summary, omitting the section when there are no files, row-bounding + overflow note, pipe escaping, change-type label mapping (including null/unknown), the `toChangedFiles` projection, plus `file_summaries` JSON deserialization, null-element filtering, and back-compat constructors. Full suite: **381 passing**. Spotless clean, SpotBugs (Max effort) reports 0 bugs. > Note: tests were run with the JaCoCo coverage agent disabled (`-Djacoco.skip=true -Dquarkus.jacoco.enabled=false`) because JaCoCo 0.8.15 crashes the JVM at startup on Java 25 — a pre-existing, unrelated environment issue. - [x] My code follows the project's coding standards - [x] I have performed a self-review of my own code - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] I have updated the documentation accordingly - [x] My changes generate no new warnings or errors No config flag — the walkthrough is part of the always-on summary content (like `pr_purpose`), and cost is bounded by the 15-file prompt cap and the 20-row render cap. Pairs with #54 (Mermaid control-flow diagram), which builds on the same `PrSummaryGenerator`.
devops-thiago
added a commit
that referenced
this pull request
Jun 24, 2026
Rebasing release/v0.3.0 onto main surfaced four breakages where release features predated changes that had already landed on main: - DocGenerationService: pass null start_line/start_side to CreatePullRequestCommentRequest, which gained multi-line range fields (#71) - ReviewOrchestratorTest: add the changedFiles arg to two generate() stubs for the file-by-file walkthrough (#179) - PrReviewPrompts: space out "{ path, summary }" so Qute — which now renders the full @UserMessage/@SystemMessage template (#184) — treats it as literal text instead of an expression - PrDescriptionGeneratorTest: assert marker neutralization instead of the "{|...|}" escaper wrapper that #186 removed main is the source of truth; these only adapt the release features to its APIs and behavior. compile, test-compile, all 1301 tests, and spotless:check pass.
devops-thiago
added a commit
that referenced
this pull request
Jun 24, 2026
…edges Three low-severity robustness fixes surfaced by the v0.3.0 review: - PrSummaryGenerator.escapeTableCell now folds any run of line breaks into a space, so a model file-summary (or CI detail) containing a newline can no longer break the Markdown table row (#179). - DiffLineResolver.resolveSuggestionRange rejects a match that straddles a hunk boundary: the trimmed-text list is flat across hunks, so a run could span two hunks and yield a range covering lines absent from the diff (a GitHub 422). It now accepts the range only when every line in [startLine, endLine] is a real right-side line, else falls back to single-line (#71). - ChangelogEntryGenerator tolerates a decorated NONE sentinel (**NONE**, `NONE`, NONE., > NONE) so a declined draft is never posted as a literal entry, while a real entry that merely mentions the word still posts (#62). Tests added for each; full suite 1330 passing, Spotless clean.
devops-thiago
added a commit
that referenced
this pull request
Jun 30, 2026
…estion can't be placed (+ false-truncation, null-reviewer, docs) (#278) ## What type of PR is this? - [x] 🐛 Bug fix - [x] 📝 Documentation ## Description Correctness fixes + doc gaps from the final ultra code-review of `release/v0.3.0`. **PR 1 of 2**; the other (truncated-celebration messaging + a suggestion-range edge) follows separately. Off `release/v0.3.0`. **#1 — `/add-docs` corrupted wrapped declarations (on by default).** `postDoc` anchored every doc suggestion as a single-line GitHub suggestion (`start_line=null`), but `DocGeneratorPrompts` permits a multi-line `suggestion_old` for a declaration that wraps across lines. On commit GitHub replaced only `doc.line()`, leaving the rest of the signature in place → broken/duplicated code. It now resolves the range via `DiffLineResolver.resolveSuggestionRange` (the same #71 anchoring the review path uses) and **drops** a multi-line suggestion it can't anchor to a single hunk rather than mis-anchoring it. **#2 — Base-comparison trimming forced a false "partial review".** `omittedFiles` summed the PR diff's omitted files *and* the supplementary base↔head comparison's, so a PR whose full diff was reviewed could be held from APPROVE and labelled partial when only the regression context was trimmed. Only the PR diff's omitted files gate the verdict now. *(Pre-existing — present verbatim in the `v0.2.1` tag.)* **#3 — NPE on a prior review from a deleted account.** `isFirstVisibleReview` dereferenced each prior review's author with no null guard; a review from a since-deleted account (`user: null`) threw an NPE and failed the whole review. The author is null-checked now, matching the sibling comment/summary checks. *(Pre-existing — present verbatim in `v0.2.1`.)* **Docs (#6/#7) + cleanup (#8).** CHANGELOG `[0.3.0]` Added now lists the shipped-but-undocumented features — `/add-docs` (#56), the changed-files walkthrough (#179), and the opt-in Mermaid diagram (#181). README documents `REVIEW_DIAGRAM_ENABLED` and lists `/changelog` among the paused commands. Dropped the pointless `num()` wrapper and fixed a misattributed comment. ## Related Issues N/A — surfaced by the v0.3.0 final code-review. Relates to #71, #234, #56, #181. ## How Has This Been Tested? - [x] Unit tests - [ ] Integration tests - [ ] Manual testing - `DocGenerationServiceTest.postsMultiLineSuggestionAnchoredToTheWholeDeclarationRange` (start_line..line span) and `skipsMultiLineSuggestionThatCannotBeAnchored`. - `ReviewOrchestratorTest.reviewSurvivesAPriorReviewFromADeletedAccount` (null-user review → no NPE, session completes). - Full suite green: 1377 tests, SpotBugs 0, Spotless clean, patch fully covered. ## Checklist - [x] My code follows the project's coding standards - [x] I have performed a self-review of my own code - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] I have updated the documentation accordingly (CHANGELOG + README) - [x] My changes generate no new warnings or errors ## Additional Notes #2 and #3 fix bugs that shipped in v0.2.1 (verified against the tag), so they get CHANGELOG entries; #1 fixes a v0.3.0 feature, covered by its Added entry. PR 2 of 2 will handle the truncated-clean celebration messaging and the `resolveSuggestionRange` blank-line edge.
devops-thiago
added a commit
that referenced
this pull request
Jun 30, 2026
…179) - [x] ✨ Feature The first-run PR summary has a risk breakdown but no structured walkthrough, so reviewers have to orient themselves on larger PRs straight from the diff. This adds a **Changed Files** table to the summary comment — `path`, change type, and a one-line description per file — so the summary mirrors the actual change set at a glance. How it works: - **Change type** is taken authoritatively from the diff (`FileDiff.status`), mapped to a friendly label (Added / Removed / Renamed / Modified; unknown values pass through; a null status renders as "Changed"). - **One-line description** comes from the model via a new `file_summaries` array on the review JSON. It rides the existing review call, so there is **no extra AI cost**. The model is asked for the 15 most impactful files (skipping generated/lockfile noise). - The renderer **joins** the model's summaries to the diff's file list **by path**, so a file the model skips still appears in the table with `-` for its summary — the table always reflects the real change set. - The table is **bounded to 20 rows** with an "…and N more file(s)" overflow note, and all cells are pipe-escaped, to stay within GitHub's comment-size budget. Rendered output: ```markdown | File | Change | Summary | |------|--------|---------| | `src/A.java` | Modified | Adds null guard to parse() | | `src/B.java` | Added | New cache wrapper | ``` Implementation notes for reviewers: - `ReviewResponse.Summary` gains a `file_summaries` field (+ nested `FileSummary` record) with null-element filtering and back-compat convenience constructors, so existing callers/responses are unaffected. - `FindingVerificationService.recount` now preserves `file_summaries` when it rebuilds the summary after verification (otherwise the field would be silently dropped). - `PrSummaryGenerator.generate` takes a new `List<ChangedFile>` argument; `ReviewOrchestrator` projects the reviewed diff into it via `toChangedFiles` and threads it through `buildResult`. Closes #39 - [x] Unit tests New/updated unit tests cover: table rendering with per-file summaries, the `-` fallback when a file has no model summary, omitting the section when there are no files, row-bounding + overflow note, pipe escaping, change-type label mapping (including null/unknown), the `toChangedFiles` projection, plus `file_summaries` JSON deserialization, null-element filtering, and back-compat constructors. Full suite: **381 passing**. Spotless clean, SpotBugs (Max effort) reports 0 bugs. > Note: tests were run with the JaCoCo coverage agent disabled (`-Djacoco.skip=true -Dquarkus.jacoco.enabled=false`) because JaCoCo 0.8.15 crashes the JVM at startup on Java 25 — a pre-existing, unrelated environment issue. - [x] My code follows the project's coding standards - [x] I have performed a self-review of my own code - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] I have updated the documentation accordingly - [x] My changes generate no new warnings or errors No config flag — the walkthrough is part of the always-on summary content (like `pr_purpose`), and cost is bounded by the 15-file prompt cap and the 20-row render cap. Pairs with #54 (Mermaid control-flow diagram), which builds on the same `PrSummaryGenerator`.
devops-thiago
added a commit
that referenced
this pull request
Jun 30, 2026
Rebasing release/v0.3.0 onto main surfaced four breakages where release features predated changes that had already landed on main: - DocGenerationService: pass null start_line/start_side to CreatePullRequestCommentRequest, which gained multi-line range fields (#71) - ReviewOrchestratorTest: add the changedFiles arg to two generate() stubs for the file-by-file walkthrough (#179) - PrReviewPrompts: space out "{ path, summary }" so Qute — which now renders the full @UserMessage/@SystemMessage template (#184) — treats it as literal text instead of an expression - PrDescriptionGeneratorTest: assert marker neutralization instead of the "{|...|}" escaper wrapper that #186 removed main is the source of truth; these only adapt the release features to its APIs and behavior. compile, test-compile, all 1301 tests, and spotless:check pass.
devops-thiago
added a commit
that referenced
this pull request
Jun 30, 2026
…edges Three low-severity robustness fixes surfaced by the v0.3.0 review: - PrSummaryGenerator.escapeTableCell now folds any run of line breaks into a space, so a model file-summary (or CI detail) containing a newline can no longer break the Markdown table row (#179). - DiffLineResolver.resolveSuggestionRange rejects a match that straddles a hunk boundary: the trimmed-text list is flat across hunks, so a run could span two hunks and yield a range covering lines absent from the diff (a GitHub 422). It now accepts the range only when every line in [startLine, endLine] is a real right-side line, else falls back to single-line (#71). - ChangelogEntryGenerator tolerates a decorated NONE sentinel (**NONE**, `NONE`, NONE., > NONE) so a declined draft is never posted as a literal entry, while a real entry that merely mentions the word still posts (#62). Tests added for each; full suite 1330 passing, Spotless clean.
devops-thiago
added a commit
that referenced
this pull request
Jun 30, 2026
…estion can't be placed (+ false-truncation, null-reviewer, docs) (#278) ## What type of PR is this? - [x] 🐛 Bug fix - [x] 📝 Documentation ## Description Correctness fixes + doc gaps from the final ultra code-review of `release/v0.3.0`. **PR 1 of 2**; the other (truncated-celebration messaging + a suggestion-range edge) follows separately. Off `release/v0.3.0`. **#1 — `/add-docs` corrupted wrapped declarations (on by default).** `postDoc` anchored every doc suggestion as a single-line GitHub suggestion (`start_line=null`), but `DocGeneratorPrompts` permits a multi-line `suggestion_old` for a declaration that wraps across lines. On commit GitHub replaced only `doc.line()`, leaving the rest of the signature in place → broken/duplicated code. It now resolves the range via `DiffLineResolver.resolveSuggestionRange` (the same #71 anchoring the review path uses) and **drops** a multi-line suggestion it can't anchor to a single hunk rather than mis-anchoring it. **#2 — Base-comparison trimming forced a false "partial review".** `omittedFiles` summed the PR diff's omitted files *and* the supplementary base↔head comparison's, so a PR whose full diff was reviewed could be held from APPROVE and labelled partial when only the regression context was trimmed. Only the PR diff's omitted files gate the verdict now. *(Pre-existing — present verbatim in the `v0.2.1` tag.)* **#3 — NPE on a prior review from a deleted account.** `isFirstVisibleReview` dereferenced each prior review's author with no null guard; a review from a since-deleted account (`user: null`) threw an NPE and failed the whole review. The author is null-checked now, matching the sibling comment/summary checks. *(Pre-existing — present verbatim in `v0.2.1`.)* **Docs (#6/#7) + cleanup (#8).** CHANGELOG `[0.3.0]` Added now lists the shipped-but-undocumented features — `/add-docs` (#56), the changed-files walkthrough (#179), and the opt-in Mermaid diagram (#181). README documents `REVIEW_DIAGRAM_ENABLED` and lists `/changelog` among the paused commands. Dropped the pointless `num()` wrapper and fixed a misattributed comment. ## Related Issues N/A — surfaced by the v0.3.0 final code-review. Relates to #71, #234, #56, #181. ## How Has This Been Tested? - [x] Unit tests - [ ] Integration tests - [ ] Manual testing - `DocGenerationServiceTest.postsMultiLineSuggestionAnchoredToTheWholeDeclarationRange` (start_line..line span) and `skipsMultiLineSuggestionThatCannotBeAnchored`. - `ReviewOrchestratorTest.reviewSurvivesAPriorReviewFromADeletedAccount` (null-user review → no NPE, session completes). - Full suite green: 1377 tests, SpotBugs 0, Spotless clean, patch fully covered. ## Checklist - [x] My code follows the project's coding standards - [x] I have performed a self-review of my own code - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] I have updated the documentation accordingly (CHANGELOG + README) - [x] My changes generate no new warnings or errors ## Additional Notes #2 and #3 fix bugs that shipped in v0.2.1 (verified against the tag), so they get CHANGELOG entries; #1 fixes a v0.3.0 feature, covered by its Added entry. PR 2 of 2 will handle the truncated-clean celebration messaging and the `resolveSuggestionRange` blank-line edge.
devops-thiago
added a commit
that referenced
this pull request
Jun 30, 2026
…179) - [x] ✨ Feature The first-run PR summary has a risk breakdown but no structured walkthrough, so reviewers have to orient themselves on larger PRs straight from the diff. This adds a **Changed Files** table to the summary comment — `path`, change type, and a one-line description per file — so the summary mirrors the actual change set at a glance. How it works: - **Change type** is taken authoritatively from the diff (`FileDiff.status`), mapped to a friendly label (Added / Removed / Renamed / Modified; unknown values pass through; a null status renders as "Changed"). - **One-line description** comes from the model via a new `file_summaries` array on the review JSON. It rides the existing review call, so there is **no extra AI cost**. The model is asked for the 15 most impactful files (skipping generated/lockfile noise). - The renderer **joins** the model's summaries to the diff's file list **by path**, so a file the model skips still appears in the table with `-` for its summary — the table always reflects the real change set. - The table is **bounded to 20 rows** with an "…and N more file(s)" overflow note, and all cells are pipe-escaped, to stay within GitHub's comment-size budget. Rendered output: ```markdown | File | Change | Summary | |------|--------|---------| | `src/A.java` | Modified | Adds null guard to parse() | | `src/B.java` | Added | New cache wrapper | ``` Implementation notes for reviewers: - `ReviewResponse.Summary` gains a `file_summaries` field (+ nested `FileSummary` record) with null-element filtering and back-compat convenience constructors, so existing callers/responses are unaffected. - `FindingVerificationService.recount` now preserves `file_summaries` when it rebuilds the summary after verification (otherwise the field would be silently dropped). - `PrSummaryGenerator.generate` takes a new `List<ChangedFile>` argument; `ReviewOrchestrator` projects the reviewed diff into it via `toChangedFiles` and threads it through `buildResult`. Closes #39 - [x] Unit tests New/updated unit tests cover: table rendering with per-file summaries, the `-` fallback when a file has no model summary, omitting the section when there are no files, row-bounding + overflow note, pipe escaping, change-type label mapping (including null/unknown), the `toChangedFiles` projection, plus `file_summaries` JSON deserialization, null-element filtering, and back-compat constructors. Full suite: **381 passing**. Spotless clean, SpotBugs (Max effort) reports 0 bugs. > Note: tests were run with the JaCoCo coverage agent disabled (`-Djacoco.skip=true -Dquarkus.jacoco.enabled=false`) because JaCoCo 0.8.15 crashes the JVM at startup on Java 25 — a pre-existing, unrelated environment issue. - [x] My code follows the project's coding standards - [x] I have performed a self-review of my own code - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] I have updated the documentation accordingly - [x] My changes generate no new warnings or errors No config flag — the walkthrough is part of the always-on summary content (like `pr_purpose`), and cost is bounded by the 15-file prompt cap and the 20-row render cap. Pairs with #54 (Mermaid control-flow diagram), which builds on the same `PrSummaryGenerator`.
devops-thiago
added a commit
that referenced
this pull request
Jun 30, 2026
Rebasing release/v0.3.0 onto main surfaced four breakages where release features predated changes that had already landed on main: - DocGenerationService: pass null start_line/start_side to CreatePullRequestCommentRequest, which gained multi-line range fields (#71) - ReviewOrchestratorTest: add the changedFiles arg to two generate() stubs for the file-by-file walkthrough (#179) - PrReviewPrompts: space out "{ path, summary }" so Qute — which now renders the full @UserMessage/@SystemMessage template (#184) — treats it as literal text instead of an expression - PrDescriptionGeneratorTest: assert marker neutralization instead of the "{|...|}" escaper wrapper that #186 removed main is the source of truth; these only adapt the release features to its APIs and behavior. compile, test-compile, all 1301 tests, and spotless:check pass.
devops-thiago
added a commit
that referenced
this pull request
Jun 30, 2026
…edges Three low-severity robustness fixes surfaced by the v0.3.0 review: - PrSummaryGenerator.escapeTableCell now folds any run of line breaks into a space, so a model file-summary (or CI detail) containing a newline can no longer break the Markdown table row (#179). - DiffLineResolver.resolveSuggestionRange rejects a match that straddles a hunk boundary: the trimmed-text list is flat across hunks, so a run could span two hunks and yield a range covering lines absent from the diff (a GitHub 422). It now accepts the range only when every line in [startLine, endLine] is a real right-side line, else falls back to single-line (#71). - ChangelogEntryGenerator tolerates a decorated NONE sentinel (**NONE**, `NONE`, NONE., > NONE) so a declined draft is never posted as a literal entry, while a real entry that merely mentions the word still posts (#62). Tests added for each; full suite 1330 passing, Spotless clean.
devops-thiago
added a commit
that referenced
this pull request
Jun 30, 2026
…estion can't be placed (+ false-truncation, null-reviewer, docs) (#278) ## What type of PR is this? - [x] 🐛 Bug fix - [x] 📝 Documentation ## Description Correctness fixes + doc gaps from the final ultra code-review of `release/v0.3.0`. **PR 1 of 2**; the other (truncated-celebration messaging + a suggestion-range edge) follows separately. Off `release/v0.3.0`. **#1 — `/add-docs` corrupted wrapped declarations (on by default).** `postDoc` anchored every doc suggestion as a single-line GitHub suggestion (`start_line=null`), but `DocGeneratorPrompts` permits a multi-line `suggestion_old` for a declaration that wraps across lines. On commit GitHub replaced only `doc.line()`, leaving the rest of the signature in place → broken/duplicated code. It now resolves the range via `DiffLineResolver.resolveSuggestionRange` (the same #71 anchoring the review path uses) and **drops** a multi-line suggestion it can't anchor to a single hunk rather than mis-anchoring it. **#2 — Base-comparison trimming forced a false "partial review".** `omittedFiles` summed the PR diff's omitted files *and* the supplementary base↔head comparison's, so a PR whose full diff was reviewed could be held from APPROVE and labelled partial when only the regression context was trimmed. Only the PR diff's omitted files gate the verdict now. *(Pre-existing — present verbatim in the `v0.2.1` tag.)* **#3 — NPE on a prior review from a deleted account.** `isFirstVisibleReview` dereferenced each prior review's author with no null guard; a review from a since-deleted account (`user: null`) threw an NPE and failed the whole review. The author is null-checked now, matching the sibling comment/summary checks. *(Pre-existing — present verbatim in `v0.2.1`.)* **Docs (#6/#7) + cleanup (#8).** CHANGELOG `[0.3.0]` Added now lists the shipped-but-undocumented features — `/add-docs` (#56), the changed-files walkthrough (#179), and the opt-in Mermaid diagram (#181). README documents `REVIEW_DIAGRAM_ENABLED` and lists `/changelog` among the paused commands. Dropped the pointless `num()` wrapper and fixed a misattributed comment. ## Related Issues N/A — surfaced by the v0.3.0 final code-review. Relates to #71, #234, #56, #181. ## How Has This Been Tested? - [x] Unit tests - [ ] Integration tests - [ ] Manual testing - `DocGenerationServiceTest.postsMultiLineSuggestionAnchoredToTheWholeDeclarationRange` (start_line..line span) and `skipsMultiLineSuggestionThatCannotBeAnchored`. - `ReviewOrchestratorTest.reviewSurvivesAPriorReviewFromADeletedAccount` (null-user review → no NPE, session completes). - Full suite green: 1377 tests, SpotBugs 0, Spotless clean, patch fully covered. ## Checklist - [x] My code follows the project's coding standards - [x] I have performed a self-review of my own code - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] I have updated the documentation accordingly (CHANGELOG + README) - [x] My changes generate no new warnings or errors ## Additional Notes #2 and #3 fix bugs that shipped in v0.2.1 (verified against the tag), so they get CHANGELOG entries; #1 fixes a v0.3.0 feature, covered by its Added entry. PR 2 of 2 will handle the truncated-clean celebration messaging and the `resolveSuggestionRange` blank-line edge.
devops-thiago
added a commit
that referenced
this pull request
Jun 30, 2026
…edges Three low-severity robustness fixes surfaced by the v0.3.0 review: - PrSummaryGenerator.escapeTableCell now folds any run of line breaks into a space, so a model file-summary (or CI detail) containing a newline can no longer break the Markdown table row (#179). - DiffLineResolver.resolveSuggestionRange rejects a match that straddles a hunk boundary: the trimmed-text list is flat across hunks, so a run could span two hunks and yield a range covering lines absent from the diff (a GitHub 422). It now accepts the range only when every line in [startLine, endLine] is a real right-side line, else falls back to single-line (#71). - ChangelogEntryGenerator tolerates a decorated NONE sentinel (**NONE**, `NONE`, NONE., > NONE) so a declined draft is never posted as a literal entry, while a real entry that merely mentions the word still posts (#62). Tests added for each; full suite 1330 passing, Spotless clean.
devops-thiago
added a commit
that referenced
this pull request
Jun 30, 2026
…estion can't be placed (+ false-truncation, null-reviewer, docs) (#278) ## What type of PR is this? - [x] 🐛 Bug fix - [x] 📝 Documentation ## Description Correctness fixes + doc gaps from the final ultra code-review of `release/v0.3.0`. **PR 1 of 2**; the other (truncated-celebration messaging + a suggestion-range edge) follows separately. Off `release/v0.3.0`. **#1 — `/add-docs` corrupted wrapped declarations (on by default).** `postDoc` anchored every doc suggestion as a single-line GitHub suggestion (`start_line=null`), but `DocGeneratorPrompts` permits a multi-line `suggestion_old` for a declaration that wraps across lines. On commit GitHub replaced only `doc.line()`, leaving the rest of the signature in place → broken/duplicated code. It now resolves the range via `DiffLineResolver.resolveSuggestionRange` (the same #71 anchoring the review path uses) and **drops** a multi-line suggestion it can't anchor to a single hunk rather than mis-anchoring it. **#2 — Base-comparison trimming forced a false "partial review".** `omittedFiles` summed the PR diff's omitted files *and* the supplementary base↔head comparison's, so a PR whose full diff was reviewed could be held from APPROVE and labelled partial when only the regression context was trimmed. Only the PR diff's omitted files gate the verdict now. *(Pre-existing — present verbatim in the `v0.2.1` tag.)* **#3 — NPE on a prior review from a deleted account.** `isFirstVisibleReview` dereferenced each prior review's author with no null guard; a review from a since-deleted account (`user: null`) threw an NPE and failed the whole review. The author is null-checked now, matching the sibling comment/summary checks. *(Pre-existing — present verbatim in `v0.2.1`.)* **Docs (#6/#7) + cleanup (#8).** CHANGELOG `[0.3.0]` Added now lists the shipped-but-undocumented features — `/add-docs` (#56), the changed-files walkthrough (#179), and the opt-in Mermaid diagram (#181). README documents `REVIEW_DIAGRAM_ENABLED` and lists `/changelog` among the paused commands. Dropped the pointless `num()` wrapper and fixed a misattributed comment. ## Related Issues N/A — surfaced by the v0.3.0 final code-review. Relates to #71, #234, #56, #181. ## How Has This Been Tested? - [x] Unit tests - [ ] Integration tests - [ ] Manual testing - `DocGenerationServiceTest.postsMultiLineSuggestionAnchoredToTheWholeDeclarationRange` (start_line..line span) and `skipsMultiLineSuggestionThatCannotBeAnchored`. - `ReviewOrchestratorTest.reviewSurvivesAPriorReviewFromADeletedAccount` (null-user review → no NPE, session completes). - Full suite green: 1377 tests, SpotBugs 0, Spotless clean, patch fully covered. ## Checklist - [x] My code follows the project's coding standards - [x] I have performed a self-review of my own code - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] I have updated the documentation accordingly (CHANGELOG + README) - [x] My changes generate no new warnings or errors ## Additional Notes #2 and #3 fix bugs that shipped in v0.2.1 (verified against the tag), so they get CHANGELOG entries; #1 fixes a v0.3.0 feature, covered by its Added entry. PR 2 of 2 will handle the truncated-clean celebration messaging and the `resolveSuggestionRange` blank-line edge.
9 tasks
devops-thiago
added a commit
that referenced
this pull request
Jul 1, 2026
## What type of PR is this? - [x] 📝 Documentation - [x] 📦 Dependency update ## Description Release prep for **v0.3.0**, in three parts: - **Docs — stale command lists fixed.** The v0.3.0 commands `/describe`, `/changelog`, and `/add-docs` were missing from the `webhook/` package row in `docs/ARCHITECTURE.md` and from the `CommentCommandService` class javadoc (both still listed only the five older commands). Both now match the `CommentCommand` enum and the live `/help` output. The `README.md` summary bullet now mentions the changed-files walkthrough added in v0.3.0 (#179). - **Changelog — dated and synced.** The `[0.3.0]` heading moves from `unreleased` to `2026-06-30`, and a new `### Dependencies` section records the bumps landed during the cycle. - **Version bump for tagging.** The Maven project version goes `0.3.0-SNAPSHOT` → `0.3.0` so the release can be tagged. (The frontend `package.json` keeps its own independent `1.0.0`.) No runtime behavior changes — docs, changelog, and version metadata only. ## Related Issues N/A (no code fix). This documents already-merged v0.3.0 features (`/describe` #35, `/changelog` #62, `/add-docs` #56, changed-files walkthrough #179) and records the dependency bumps #262, #263, #265, #266, #267 in the changelog. ## How Has This Been Tested? - [x] Manual testing The documented v0.3.0 command surface was dogfooded on `#256` and `devops-thiago/MongOCOM#46`; the corrected command list matches both the `CommentCommand` enum and the bot's own `/help` output. Changes are documentation/metadata only, so no automated tests apply. ## Checklist - [x] My code follows the project's coding standards - [x] 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 - [x] New and existing unit tests pass locally with my changes - [x] I have updated the documentation accordingly - [x] My changes generate no new warnings or errors ## Additional Notes - A copy-editing pass over the v0.3.0 changelog prose (against the Wikipedia "Signs of AI writing" checklist) found **no changes needed** — the entries match the established terse-technical voice, so there is no follow-up commit. - Three tracking issues surfaced during v0.3.0 dogfooding were filed **separately** (not part of this PR): #296 (`/describe` `/changelog` `/add-docs` truncation disclosure), #297 (`/summary` "already exists" wording), #298 (summary "Changes Overview" count accuracy).
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
The first-run PR summary has a risk breakdown but no structured walkthrough, so reviewers have to orient themselves on larger PRs straight from the diff. This adds a Changed Files table to the summary comment —
path, change type, and a one-line description per file — so the summary mirrors the actual change set at a glance.How it works:
FileDiff.status), mapped to a friendly label (Added / Removed / Renamed / Modified; unknown values pass through; a null status renders as "Changed").file_summariesarray on the review JSON. It rides the existing review call, so there is no extra AI cost. The model is asked for the 15 most impactful files (skipping generated/lockfile noise).-for its summary — the table always reflects the real change set.Rendered output:
Implementation notes for reviewers:
ReviewResponse.Summarygains afile_summariesfield (+ nestedFileSummaryrecord) with null-element filtering and back-compat convenience constructors, so existing callers/responses are unaffected.FindingVerificationService.recountnow preservesfile_summarieswhen it rebuilds the summary after verification (otherwise the field would be silently dropped).PrSummaryGenerator.generatetakes a newList<ChangedFile>argument;ReviewOrchestratorprojects the reviewed diff into it viatoChangedFilesand threads it throughbuildResult.Related Issues
Closes #39
How Has This Been Tested?
New/updated unit tests cover: table rendering with per-file summaries, the
-fallback when a file has no model summary, omitting the section when there are no files, row-bounding + overflow note, pipe escaping, change-type label mapping (including null/unknown), thetoChangedFilesprojection, plusfile_summariesJSON deserialization, null-element filtering, and back-compat constructors.Full suite: 381 passing. Spotless clean, SpotBugs (Max effort) reports 0 bugs.
Checklist
Additional Notes
No config flag — the walkthrough is part of the always-on summary content (like
pr_purpose), and cost is bounded by the 15-file prompt cap and the 20-row render cap. Pairs with #54 (Mermaid control-flow diagram), which builds on the samePrSummaryGenerator.