perf(review): overlap CI status with the model call, drop redundant PR fetch, move paging into the client#275
Merged
Conversation
…nt PR fetch, move paging into the client CI status (required-context resolution + check evaluation) depends only on the commit and base branch, not the model response, so it now runs on the review executor concurrently with the blocking AI call and is joined before the verdict — the GitHub latency overlaps the model latency instead of stacking on it (#10). The base branch ref is now carried on ReviewRequest (populated from the webhook payload for auto reviews, and from resolveMissingPrDetails' PR fetch for manual ones), so CiStatusEvaluator.resolveRequiredContexts takes it directly instead of issuing a separate getPullRequest just to read base().ref(); the prClient dependency is dropped from the evaluator (#7). Pagination for check runs and combined status moved into GitHubCheckRunClient as getAllCheckRuns/getAllCombinedStatus default methods (owning the per_page/page walk and CI_PER_PAGE/CI_MAX_PAGES), matching the other GitHub clients; the evaluator consumes the aggregated lists and keeps the readable-vs-unreadable signal (null/exception → unreadable) (#15). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🤖 ThrillhouseBot PR SummaryWhat this PR doesMoves CI status I/O (required-check resolution and check evaluation) onto a separate executor thread to run concurrently with the blocking AI model call, so GitHub API latency does not stack serially. Removes the extra getPullRequest call by passing the base branch via ReviewRequest from webhook payloads or manual triggers. Consolidates pagination for check runs and combined status into the client interface as default methods. 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 |
|---|---|---|---|
| docker-pr | check-run | ⏳ Pending | - |
| test | 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! |
|
12 tasks
devops-thiago
added a commit
that referenced
this pull request
Jun 29, 2026
…changelog cleanup (#276) ## What type of PR is this? - [x] 🚀 Performance - [x] 🔧 Refactor - [x] 📝 Documentation ## Description Diff/doc cleanup from the v0.3.0 final-review remaining items, plus a CHANGELOG correction. **PR 3 of 4** (the CI-status PR #275 is merged). Based on release/v0.3.0. **F8 — one ignore-glob pass on the diff render.** `formatFileSection` re-ran `isIgnored()` for every file inside `buildDiffStringWithStats`, a second full glob walk on the hot review path (the context loader already computed the reviewable list for the line resolver). The loader now computes `reviewableFiles` once and passes it to `buildDiffStringWithStats`, which threads the reviewable-name set into `formatFileSection` (set membership, not a fresh glob match). The base-comparison render derives its set the same way. **G1 — /add-docs uses the no-re-walk overload.** `DocGenerationService.postSuggestions` called `patchesByFile(reviewable)` on a list it had *already* filtered, re-running the glob; it now uses `patchesByReviewableFiles`. **F14 — share the soft loaders.** The best-effort fetch-and-degrade loaders (PR details, changed files, project stack, repository instructions) were copy-pasted across `AbstractPrSuggestionGenerator`, `DocGenerationService`, and `ReviewContextLoader`. They now live once in a new `SoftLoaders` helper; all three delegate to it (and the duplicated `resolveProjectStack` wrapper is gone from both call sites). **CHANGELOG correction (separate commit).** Per the net-vs-last-release policy, `[0.3.0]` should list only the delta vs v0.2.1. Bugs that were both introduced *and* fixed within the unreleased v0.3.0 cycle net out, so their entries are removed: the completed-broadcast divergence, the unreadable-CI disclosure gap, the diagram render kill-switch, the CI bot-identity match, and the decomposition's double glob walk. Kept: features, improvements, and fixes for bugs that shipped in v0.2.1 or earlier (verified against the `v0.2.1` tag — check-run-before-review, stuck session, truncated-PR `✅`, unreadable-CI approving all reproduce there). ## Related Issues N/A — v0.3.0 final-review remaining items (#8, #14) + audit gap (G1). Relates to #250. ## How Has This Been Tested? - [x] Unit tests - [ ] Integration tests - [ ] Manual testing - Existing `ReviewDiffFormatterTest`, `ReviewContextLoaderTest`, `DocGenerationServiceTest`, and the `/describe`·`/changelog` generator tests exercise the reworked paths; `SoftLoaders` is fully covered through them. - Full suite green: 1374 tests, SpotBugs 0, Spotless clean, patch fully covered (no misses, no partials on changed lines). ## 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) - [x] My changes generate no new warnings or errors ## Additional Notes **PR 3 of 4** — the CI-status I/O PR (#275) is merged; this is based on release/v0.3.0. The CHANGELOG cleanup spans entries originally added across the earlier final-review PRs; it lands here because this is the stack tip.
devops-thiago
added a commit
that referenced
this pull request
Jun 30, 2026
…R fetch, move paging into the client (#275) ## What type of PR is this? - [x] 🚀 Performance - [x] 🔧 Refactor ## Description CI-status I/O improvements from the v0.3.0 final-review remaining items. **PR 2 of 4** (the bug-fix PR #274 is already merged). Stacked on release/v0.3.0. **#10 — Overlap CI resolution with the model call.** Required-check resolution + CI evaluation depend only on the commit/base branch, not the model response, yet ran strictly *after* the blocking AI call. They now run on the shared `@ReviewExecutor` (virtual-thread) concurrently with `findingPipeline.run(...)` and are joined before the verdict — the GitHub latency overlaps the model latency instead of stacking on top of it. **#7 — Drop the redundant `getPullRequest`.** `resolveRequiredContexts` fetched the whole PR every review just to read `base().ref()`. The base branch is now carried on `ReviewRequest.baseRef` — populated from the webhook payload (auto) or the manual-trigger PR fetch in `resolveMissingPrDetails` — so the evaluator takes it directly. `CiStatusEvaluator`'s `prClient` dependency is removed. A back-compat convenience constructor on `ReviewRequest` (used by the manual entry points + tests) defaults `baseRef` to empty, so a blank ref simply gates on all checks. **#15 — Move pagination into the client.** The per_page/page walk + `CI_PER_PAGE`/`CI_MAX_PAGES` moved into `GitHubCheckRunClient` as `getAllCheckRuns`/`getAllCombinedStatus` default methods (like the other GitHub clients, which page internally). The evaluator consumes the aggregated lists and keeps the readable-vs-unreadable distinction (a `null` page body or exception → unreadable). ## Related Issues N/A — v0.3.0 final-review remaining items (#7, #10, #15). Relates to #250 and #253. ## How Has This Been Tested? - [x] Unit tests - [ ] Integration tests - [ ] Manual testing - `CiStatusEvaluatorTest`: `resolveRequiredContexts` cases migrated to the base-ref signature (no PR fetch; blank ref → gate-all); the mock now runs the client's real default paging methods so the existing per-page stubs and the multi-page/`CI_MAX_PAGES` tests still exercise pagination. - `ReviewOrchestratorTest`: CI stubs use `getAllCheckRuns`/`getAllCombinedStatus`; `request()` carries `baseRef="main"` to exercise gate-specific; blank-baseRef fallback consolidated into one test; orchestrator built with the executor. - `WebhookControllerTest`: routing tests assert the dispatched `baseRef`. - Full suite green: 1374 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) - [x] My changes generate no new warnings or errors ## Additional Notes **PR 2 of 4** — the broadcast/disclosure fix (#274) is merged; this is based on release/v0.3.0. Concurrency note: the CI task runs on the existing virtual-thread review executor and touches only stateless `@ApplicationScoped`/`@Singleton` beans + REST clients (no request-scoped state), so no request context is required on the worker thread.
devops-thiago
added a commit
that referenced
this pull request
Jun 30, 2026
…changelog cleanup (#276) ## What type of PR is this? - [x] 🚀 Performance - [x] 🔧 Refactor - [x] 📝 Documentation ## Description Diff/doc cleanup from the v0.3.0 final-review remaining items, plus a CHANGELOG correction. **PR 3 of 4** (the CI-status PR #275 is merged). Based on release/v0.3.0. **F8 — one ignore-glob pass on the diff render.** `formatFileSection` re-ran `isIgnored()` for every file inside `buildDiffStringWithStats`, a second full glob walk on the hot review path (the context loader already computed the reviewable list for the line resolver). The loader now computes `reviewableFiles` once and passes it to `buildDiffStringWithStats`, which threads the reviewable-name set into `formatFileSection` (set membership, not a fresh glob match). The base-comparison render derives its set the same way. **G1 — /add-docs uses the no-re-walk overload.** `DocGenerationService.postSuggestions` called `patchesByFile(reviewable)` on a list it had *already* filtered, re-running the glob; it now uses `patchesByReviewableFiles`. **F14 — share the soft loaders.** The best-effort fetch-and-degrade loaders (PR details, changed files, project stack, repository instructions) were copy-pasted across `AbstractPrSuggestionGenerator`, `DocGenerationService`, and `ReviewContextLoader`. They now live once in a new `SoftLoaders` helper; all three delegate to it (and the duplicated `resolveProjectStack` wrapper is gone from both call sites). **CHANGELOG correction (separate commit).** Per the net-vs-last-release policy, `[0.3.0]` should list only the delta vs v0.2.1. Bugs that were both introduced *and* fixed within the unreleased v0.3.0 cycle net out, so their entries are removed: the completed-broadcast divergence, the unreadable-CI disclosure gap, the diagram render kill-switch, the CI bot-identity match, and the decomposition's double glob walk. Kept: features, improvements, and fixes for bugs that shipped in v0.2.1 or earlier (verified against the `v0.2.1` tag — check-run-before-review, stuck session, truncated-PR `✅`, unreadable-CI approving all reproduce there). ## Related Issues N/A — v0.3.0 final-review remaining items (#8, #14) + audit gap (G1). Relates to #250. ## How Has This Been Tested? - [x] Unit tests - [ ] Integration tests - [ ] Manual testing - Existing `ReviewDiffFormatterTest`, `ReviewContextLoaderTest`, `DocGenerationServiceTest`, and the `/describe`·`/changelog` generator tests exercise the reworked paths; `SoftLoaders` is fully covered through them. - Full suite green: 1374 tests, SpotBugs 0, Spotless clean, patch fully covered (no misses, no partials on changed lines). ## 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) - [x] My changes generate no new warnings or errors ## Additional Notes **PR 3 of 4** — the CI-status I/O PR (#275) is merged; this is based on release/v0.3.0. The CHANGELOG cleanup spans entries originally added across the earlier final-review PRs; it lands here because this is the stack tip.
devops-thiago
added a commit
that referenced
this pull request
Jun 30, 2026
…R fetch, move paging into the client (#275) ## What type of PR is this? - [x] 🚀 Performance - [x] 🔧 Refactor ## Description CI-status I/O improvements from the v0.3.0 final-review remaining items. **PR 2 of 4** (the bug-fix PR #274 is already merged). Stacked on release/v0.3.0. **#10 — Overlap CI resolution with the model call.** Required-check resolution + CI evaluation depend only on the commit/base branch, not the model response, yet ran strictly *after* the blocking AI call. They now run on the shared `@ReviewExecutor` (virtual-thread) concurrently with `findingPipeline.run(...)` and are joined before the verdict — the GitHub latency overlaps the model latency instead of stacking on top of it. **#7 — Drop the redundant `getPullRequest`.** `resolveRequiredContexts` fetched the whole PR every review just to read `base().ref()`. The base branch is now carried on `ReviewRequest.baseRef` — populated from the webhook payload (auto) or the manual-trigger PR fetch in `resolveMissingPrDetails` — so the evaluator takes it directly. `CiStatusEvaluator`'s `prClient` dependency is removed. A back-compat convenience constructor on `ReviewRequest` (used by the manual entry points + tests) defaults `baseRef` to empty, so a blank ref simply gates on all checks. **#15 — Move pagination into the client.** The per_page/page walk + `CI_PER_PAGE`/`CI_MAX_PAGES` moved into `GitHubCheckRunClient` as `getAllCheckRuns`/`getAllCombinedStatus` default methods (like the other GitHub clients, which page internally). The evaluator consumes the aggregated lists and keeps the readable-vs-unreadable distinction (a `null` page body or exception → unreadable). ## Related Issues N/A — v0.3.0 final-review remaining items (#7, #10, #15). Relates to #250 and #253. ## How Has This Been Tested? - [x] Unit tests - [ ] Integration tests - [ ] Manual testing - `CiStatusEvaluatorTest`: `resolveRequiredContexts` cases migrated to the base-ref signature (no PR fetch; blank ref → gate-all); the mock now runs the client's real default paging methods so the existing per-page stubs and the multi-page/`CI_MAX_PAGES` tests still exercise pagination. - `ReviewOrchestratorTest`: CI stubs use `getAllCheckRuns`/`getAllCombinedStatus`; `request()` carries `baseRef="main"` to exercise gate-specific; blank-baseRef fallback consolidated into one test; orchestrator built with the executor. - `WebhookControllerTest`: routing tests assert the dispatched `baseRef`. - Full suite green: 1374 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) - [x] My changes generate no new warnings or errors ## Additional Notes **PR 2 of 4** — the broadcast/disclosure fix (#274) is merged; this is based on release/v0.3.0. Concurrency note: the CI task runs on the existing virtual-thread review executor and touches only stateless `@ApplicationScoped`/`@Singleton` beans + REST clients (no request-scoped state), so no request context is required on the worker thread.
devops-thiago
added a commit
that referenced
this pull request
Jun 30, 2026
…changelog cleanup (#276) ## What type of PR is this? - [x] 🚀 Performance - [x] 🔧 Refactor - [x] 📝 Documentation ## Description Diff/doc cleanup from the v0.3.0 final-review remaining items, plus a CHANGELOG correction. **PR 3 of 4** (the CI-status PR #275 is merged). Based on release/v0.3.0. **F8 — one ignore-glob pass on the diff render.** `formatFileSection` re-ran `isIgnored()` for every file inside `buildDiffStringWithStats`, a second full glob walk on the hot review path (the context loader already computed the reviewable list for the line resolver). The loader now computes `reviewableFiles` once and passes it to `buildDiffStringWithStats`, which threads the reviewable-name set into `formatFileSection` (set membership, not a fresh glob match). The base-comparison render derives its set the same way. **G1 — /add-docs uses the no-re-walk overload.** `DocGenerationService.postSuggestions` called `patchesByFile(reviewable)` on a list it had *already* filtered, re-running the glob; it now uses `patchesByReviewableFiles`. **F14 — share the soft loaders.** The best-effort fetch-and-degrade loaders (PR details, changed files, project stack, repository instructions) were copy-pasted across `AbstractPrSuggestionGenerator`, `DocGenerationService`, and `ReviewContextLoader`. They now live once in a new `SoftLoaders` helper; all three delegate to it (and the duplicated `resolveProjectStack` wrapper is gone from both call sites). **CHANGELOG correction (separate commit).** Per the net-vs-last-release policy, `[0.3.0]` should list only the delta vs v0.2.1. Bugs that were both introduced *and* fixed within the unreleased v0.3.0 cycle net out, so their entries are removed: the completed-broadcast divergence, the unreadable-CI disclosure gap, the diagram render kill-switch, the CI bot-identity match, and the decomposition's double glob walk. Kept: features, improvements, and fixes for bugs that shipped in v0.2.1 or earlier (verified against the `v0.2.1` tag — check-run-before-review, stuck session, truncated-PR `✅`, unreadable-CI approving all reproduce there). ## Related Issues N/A — v0.3.0 final-review remaining items (#8, #14) + audit gap (G1). Relates to #250. ## How Has This Been Tested? - [x] Unit tests - [ ] Integration tests - [ ] Manual testing - Existing `ReviewDiffFormatterTest`, `ReviewContextLoaderTest`, `DocGenerationServiceTest`, and the `/describe`·`/changelog` generator tests exercise the reworked paths; `SoftLoaders` is fully covered through them. - Full suite green: 1374 tests, SpotBugs 0, Spotless clean, patch fully covered (no misses, no partials on changed lines). ## 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) - [x] My changes generate no new warnings or errors ## Additional Notes **PR 3 of 4** — the CI-status I/O PR (#275) is merged; this is based on release/v0.3.0. The CHANGELOG cleanup spans entries originally added across the earlier final-review PRs; it lands here because this is the stack tip.
devops-thiago
added a commit
that referenced
this pull request
Jun 30, 2026
…R fetch, move paging into the client (#275) ## What type of PR is this? - [x] 🚀 Performance - [x] 🔧 Refactor ## Description CI-status I/O improvements from the v0.3.0 final-review remaining items. **PR 2 of 4** (the bug-fix PR #274 is already merged). Stacked on release/v0.3.0. **#10 — Overlap CI resolution with the model call.** Required-check resolution + CI evaluation depend only on the commit/base branch, not the model response, yet ran strictly *after* the blocking AI call. They now run on the shared `@ReviewExecutor` (virtual-thread) concurrently with `findingPipeline.run(...)` and are joined before the verdict — the GitHub latency overlaps the model latency instead of stacking on top of it. **#7 — Drop the redundant `getPullRequest`.** `resolveRequiredContexts` fetched the whole PR every review just to read `base().ref()`. The base branch is now carried on `ReviewRequest.baseRef` — populated from the webhook payload (auto) or the manual-trigger PR fetch in `resolveMissingPrDetails` — so the evaluator takes it directly. `CiStatusEvaluator`'s `prClient` dependency is removed. A back-compat convenience constructor on `ReviewRequest` (used by the manual entry points + tests) defaults `baseRef` to empty, so a blank ref simply gates on all checks. **#15 — Move pagination into the client.** The per_page/page walk + `CI_PER_PAGE`/`CI_MAX_PAGES` moved into `GitHubCheckRunClient` as `getAllCheckRuns`/`getAllCombinedStatus` default methods (like the other GitHub clients, which page internally). The evaluator consumes the aggregated lists and keeps the readable-vs-unreadable distinction (a `null` page body or exception → unreadable). ## Related Issues N/A — v0.3.0 final-review remaining items (#7, #10, #15). Relates to #250 and #253. ## How Has This Been Tested? - [x] Unit tests - [ ] Integration tests - [ ] Manual testing - `CiStatusEvaluatorTest`: `resolveRequiredContexts` cases migrated to the base-ref signature (no PR fetch; blank ref → gate-all); the mock now runs the client's real default paging methods so the existing per-page stubs and the multi-page/`CI_MAX_PAGES` tests still exercise pagination. - `ReviewOrchestratorTest`: CI stubs use `getAllCheckRuns`/`getAllCombinedStatus`; `request()` carries `baseRef="main"` to exercise gate-specific; blank-baseRef fallback consolidated into one test; orchestrator built with the executor. - `WebhookControllerTest`: routing tests assert the dispatched `baseRef`. - Full suite green: 1374 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) - [x] My changes generate no new warnings or errors ## Additional Notes **PR 2 of 4** — the broadcast/disclosure fix (#274) is merged; this is based on release/v0.3.0. Concurrency note: the CI task runs on the existing virtual-thread review executor and touches only stateless `@ApplicationScoped`/`@Singleton` beans + REST clients (no request-scoped state), so no request context is required on the worker thread.
devops-thiago
added a commit
that referenced
this pull request
Jun 30, 2026
…changelog cleanup (#276) ## What type of PR is this? - [x] 🚀 Performance - [x] 🔧 Refactor - [x] 📝 Documentation ## Description Diff/doc cleanup from the v0.3.0 final-review remaining items, plus a CHANGELOG correction. **PR 3 of 4** (the CI-status PR #275 is merged). Based on release/v0.3.0. **F8 — one ignore-glob pass on the diff render.** `formatFileSection` re-ran `isIgnored()` for every file inside `buildDiffStringWithStats`, a second full glob walk on the hot review path (the context loader already computed the reviewable list for the line resolver). The loader now computes `reviewableFiles` once and passes it to `buildDiffStringWithStats`, which threads the reviewable-name set into `formatFileSection` (set membership, not a fresh glob match). The base-comparison render derives its set the same way. **G1 — /add-docs uses the no-re-walk overload.** `DocGenerationService.postSuggestions` called `patchesByFile(reviewable)` on a list it had *already* filtered, re-running the glob; it now uses `patchesByReviewableFiles`. **F14 — share the soft loaders.** The best-effort fetch-and-degrade loaders (PR details, changed files, project stack, repository instructions) were copy-pasted across `AbstractPrSuggestionGenerator`, `DocGenerationService`, and `ReviewContextLoader`. They now live once in a new `SoftLoaders` helper; all three delegate to it (and the duplicated `resolveProjectStack` wrapper is gone from both call sites). **CHANGELOG correction (separate commit).** Per the net-vs-last-release policy, `[0.3.0]` should list only the delta vs v0.2.1. Bugs that were both introduced *and* fixed within the unreleased v0.3.0 cycle net out, so their entries are removed: the completed-broadcast divergence, the unreadable-CI disclosure gap, the diagram render kill-switch, the CI bot-identity match, and the decomposition's double glob walk. Kept: features, improvements, and fixes for bugs that shipped in v0.2.1 or earlier (verified against the `v0.2.1` tag — check-run-before-review, stuck session, truncated-PR `✅`, unreadable-CI approving all reproduce there). ## Related Issues N/A — v0.3.0 final-review remaining items (#8, #14) + audit gap (G1). Relates to #250. ## How Has This Been Tested? - [x] Unit tests - [ ] Integration tests - [ ] Manual testing - Existing `ReviewDiffFormatterTest`, `ReviewContextLoaderTest`, `DocGenerationServiceTest`, and the `/describe`·`/changelog` generator tests exercise the reworked paths; `SoftLoaders` is fully covered through them. - Full suite green: 1374 tests, SpotBugs 0, Spotless clean, patch fully covered (no misses, no partials on changed lines). ## 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) - [x] My changes generate no new warnings or errors ## Additional Notes **PR 3 of 4** — the CI-status I/O PR (#275) is merged; this is based on release/v0.3.0. The CHANGELOG cleanup spans entries originally added across the earlier final-review PRs; it lands here because this is the stack tip.
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
CI-status I/O improvements from the v0.3.0 final-review remaining items.
PR 2 of 4 (the bug-fix PR #274 is already merged). Stacked on
release/v0.3.0.
#10 — Overlap CI resolution with the model call. Required-check
resolution + CI evaluation depend only on the commit/base branch, not
the model response, yet ran strictly after the blocking AI call. They
now run on the shared
@ReviewExecutor(virtual-thread) concurrentlywith
findingPipeline.run(...)and are joined before the verdict — theGitHub latency overlaps the model latency instead of stacking on top of
it.
#7 — Drop the redundant
getPullRequest.resolveRequiredContextsfetched the whole PR every review just to read
base().ref(). The basebranch is now carried on
ReviewRequest.baseRef— populated from thewebhook payload (auto) or the manual-trigger PR fetch in
resolveMissingPrDetails— so the evaluator takes it directly.CiStatusEvaluator'sprClientdependency is removed. A back-compatconvenience constructor on
ReviewRequest(used by the manual entrypoints + tests) defaults
baseRefto empty, so a blank ref simply gateson all checks.
#15 — Move pagination into the client. The per_page/page walk +
CI_PER_PAGE/CI_MAX_PAGESmoved intoGitHubCheckRunClientasgetAllCheckRuns/getAllCombinedStatusdefault methods (like the otherGitHub clients, which page internally). The evaluator consumes the
aggregated lists and keeps the readable-vs-unreadable distinction (a
nullpage body or exception → unreadable).Related Issues
N/A — v0.3.0 final-review remaining items (#7, #10, #15). Relates to
#250 and #253.
How Has This Been Tested?
Unit tests
Integration tests
Manual testing
CiStatusEvaluatorTest:resolveRequiredContextscases migrated tothe base-ref signature (no PR fetch; blank ref → gate-all); the mock now
runs the client's real default paging methods so the existing per-page
stubs and the multi-page/
CI_MAX_PAGEStests still exercise pagination.ReviewOrchestratorTest: CI stubs usegetAllCheckRuns/getAllCombinedStatus;request()carriesbaseRef="main"to exercise gate-specific; blank-baseRef fallbackconsolidated into one test; orchestrator built with the executor.
WebhookControllerTest: routing tests assert the dispatchedbaseRef.Full suite green: 1374 tests, SpotBugs 0, Spotless clean, patch fully
covered.
Checklist
feature works
Additional Notes
PR 2 of 4 — the broadcast/disclosure fix (#274) is merged; this is
based on release/v0.3.0. Concurrency note: the CI task runs on the
existing virtual-thread review executor and touches only stateless
@ApplicationScoped/@Singletonbeans + REST clients (norequest-scoped state), so no request context is required on the worker
thread.