Repository navigation
feat: V1 orchestration pipeline, workflows, and README - #7
Conversation
Wire the full review pipeline: orchestrate.ts composes the I/O clients (github, openrouter, context) with the pure review modules (diff, phases, prompt, selection, comment mapping) into a 15-step pipeline. Skip paths after PR identification post body-only reviews (§5 amendment). createPromptedGenerateFindings implements the V1 default — one prompted combined-phase call with per-run nonce-wrapped delimiters. main.ts replaces the scaffold stub with the full wiring: client construction, orchestrate call, output setting, and cost summary write. self_review.yml dogfoods the action on its own PRs and @umm review comments. release.yml builds and pushes the GHCR image, digest-flips action.yml, force-moves semver + floating major tags, and creates a GitHub Release. README documents all 12 inputs, 4 outputs, usage example with both triggers, and the review pipeline. 207 tests (41 new for orchestrate); 3 mutation spot checks verified. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HBJw5bhDigxu8HM2sPPZVf
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The diff token budget check uses half of
contextBudgetTokenswhilereadChangedFilesis given the full budget, which can lead to total context exceeding the configured budget; consider passing an explicit remaining-budget into the context readers so the diff + files stay within a single consistent cap. - The orchestration pipeline currently resolves phases but only ever uses
phases[0], silently ignoring any additional phases; if multi-phase operation is not yet supported, it may be clearer to enforcecombinedexplicitly or throw when more than one phase is configured.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The diff token budget check uses half of `contextBudgetTokens` while `readChangedFiles` is given the full budget, which can lead to total context exceeding the configured budget; consider passing an explicit remaining-budget into the context readers so the diff + files stay within a single consistent cap.
- The orchestration pipeline currently resolves phases but only ever uses `phases[0]`, silently ignoring any additional phases; if multi-phase operation is not yet supported, it may be clearer to enforce `combined` explicitly or throw when more than one phase is configured.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…ency - Subtract diffTokens from the file budget in orchestrate.ts so total prompt context respects the stated contextBudgetTokens cap (was passing the full budget to readChangedFiles, allowing ~1.5x overshoot) - release.yml: downgrade GITHUB_TOKEN to contents:read on the release job — only the app token does writes (git push, gh release) - release.yml: add concurrency block to serialize releases and prevent floating major-tag races on simultaneous tag pushes - self_review.yml: remove unused pull-requests:write on GITHUB_TOKEN — the app token handles review posting Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HBJw5bhDigxu8HM2sPPZVf
|
Warning Review limit reached
Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe action now implements end-to-end PR review orchestration, including diff handling, context loading, model generation, review submission, outputs, documentation, self-review automation, and digest-pinned release automation. ChangesReview action delivery
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 53-57: Update the actions/create-github-app-token step with id
app-token to explicitly set permission-contents to write, restricting the
generated token to repository content write access while preserving the existing
app-id and private-key inputs.
In @.github/workflows/self_review.yml:
- Around line 34-38: Add permission-contents: read and permission-pull-requests:
write to the app-token step using actions/create-github-app-token, limiting the
generated GitHub App token to the required permissions while preserving the
existing app-id and private-key inputs.
In `@src/__tests__/orchestrate.test.ts`:
- Around line 81-90: Replace the loosely typed recording fields in
RecordingStubs with the actual argument shapes of the recorded calls, then
update all affected assertions in the test to use the inferred values without as
casts. In the phase assertions, replace phases[0]! with destructuring into
firstPhase and an explicit definedness guard before accessing it. Remove every
as assertion and non-null assertion in orchestrate.test.ts while preserving the
existing test behavior.
In `@src/orchestrate.ts`:
- Around line 140-167: Update the diff-size check in the Step 6 annotation flow
to report the effective half-budget limit in the skip message instead of the
full context budget. In the Step 9 context-reading flow, pass the remaining
budget after the annotated diff—config.contextBudgetTokens minus diffTokens—to
contextReader.readChangedFiles, while preserving the existing changedPaths and
conventions handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9414d9b6-a9dc-486f-96c5-9044cff6ee09
📒 Files selected for processing (7)
.github/workflows/release.yml.github/workflows/self_review.ymlAGENTS.mdREADME.mdsrc/__tests__/orchestrate.test.tssrc/main.tssrc/orchestrate.ts
- Extract IIFE in orchestrate step 13 into named buildReviewPayload
function (callback-decomposition trigger: 3 named intermediates)
- Rename reviewBody → reviewPayload (value is { body, comments,
fallbackBody }, not just a body string)
- Add JSDoc to orchestrate and createPromptedGenerateFindings exports
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HBJw5bhDigxu8HM2sPPZVf
Fix assertion quality (exact match on hardcoded skip reason, deterministic cost summary values over expect.any(String), annotation-specific format check). Replace banned `!` non-null assertions with guards. Add 4 tests: skip review identity fields, budget-minus-diff wiring, remainingTokens passthrough, and changedPaths extraction from diff. 207 -> 211 tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HBJw5bhDigxu8HM2sPPZVf
Restore the collectRawInputs JSDoc that documents the SDK boundary semantics (getBooleanInput YAML 1.2 enforcement, runner-materialized defaults). Add model input to self_review.yml from vars.OPENROUTER_MODEL so the repo variable takes effect instead of falling through to the action.yml default. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HBJw5bhDigxu8HM2sPPZVf
![umm-actually[bot]](https://avatars.githubusercontent.com/in/4233716?s=60&v=4){kind=small}
…eparate cap The description claimed the budget covers "conventions + file contents + diff", but conventions are truncated independently at an 8k-token cap (CONVENTIONS_TOKEN_CAP in prompt.ts) and are not counted against the context_budget_tokens budget. A user relying on this description would underestimate their actual prompt size by up to 8k tokens. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HBJw5bhDigxu8HM2sPPZVf
buildReviewBody now accepts optional heading/description params so the 422 fallback body says "Findings — inline comments were unavailable" instead of "Findings beyond the diff" (which mischaracterizes why findings are in the body when GitHub rejects inline comments). self_review.yml now requests umm-actually[bot] as a reviewer before running the action so the bot appears in the Reviewers sidebar rather than under "+1 more reviewer." Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HBJw5bhDigxu8HM2sPPZVf
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/__tests__/orchestrate.test.ts`:
- Line 303: Update the RecordingStubs recording-array properties to use the
corresponding method parameter types, such as
Parameters<ContextReader["readChangedFiles"]>[0], instead of Record<string,
unknown>[] or unknown[]. Then remove the remaining as assertions at the
reviewCall and related call sites around lines 376–377, 406, and 417–419,
preserving the existing recorded-call behavior without introducing any, as, or
non-null assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 63807df8-969b-4b20-80e0-8d3936413013
📒 Files selected for processing (8)
.github/workflows/release.yml.github/workflows/self_review.ymlREADME.mdaction.ymlsrc/__tests__/orchestrate.test.tssrc/main.tssrc/orchestrate.tssrc/review/comment-mapping.ts
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/release.yml
- src/main.ts
- src/orchestrate.ts
|
Responding to the Sourcery review body findings:
🔍 ship-check · pr-monitor · claude-opus-4-6 |
Workflow permissions: - release.yml: restore contents: write, add permission-contents: write to app-token, add explicit persist-credentials: true on push checkout - self_review.yml: restore pull-requests: write, add permission-contents and permission-pull-requests to app-token for least-privilege scoping Review body: - buildReviewBody now shows "N finding(s) posted as inline comments" when all findings are inlined (was attribution-only, looked empty) - Skip message references effective half-budget limit instead of full Test types: - Recording arrays typed with actual param shapes, eliminating all 19 lint warnings (as casts, ! assertions) — 0 warnings across the suite - first() helper replaces unsafe array access patterns Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HBJw5bhDigxu8HM2sPPZVf
Budget test now asserts the exact computed value (contextBudgetTokens - sampleDiffTokens) instead of loose range checks. changedPaths now includes the old path for renames (file.from when both paths are real files) so findRelatedFiles catches callers that still reference the pre-rename path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HBJw5bhDigxu8HM2sPPZVf
AGENTS.md test conventions: when fixtures and stubs produce deterministic results, assert the entire return value or call params — fragments are the cheap option. orchestrate.test.ts: all happy path, skip path, zero findings, cap, and fallback body assertions now use toEqual on the full result object and full submitReview params (body, comments, fallbackBody) instead of individual field checks or toContain fragments. Expected values are precomputed from the same deterministic fixtures using the actual pipeline functions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HBJw5bhDigxu8HM2sPPZVf
Surfaces what's working, what's being built (review dedup, doc-staleness, branded checks, vault-cortex swap), and what's on the roadmap (V1.5 verification tool, V2 agentic exploration). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HBJw5bhDigxu8HM2sPPZVf
Public OSS README should only contain items relevant to users of the action, not internal project details. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HBJw5bhDigxu8HM2sPPZVf
| - name: Flip action.yml image to digest-pinned GHCR reference | ||
| env: | ||
| DIGEST: ${{ needs.build-and-push.outputs.digest }} | ||
| IMAGE: ghcr.io/${{ github.repository }} | ||
| run: | | ||
| sed -i "s|image: Dockerfile|image: docker://${IMAGE}@${DIGEST}|" action.yml | ||
| git config user.name "umm-actually[bot]" | ||
| git config user.email "umm-actually[bot]@users.noreply.github.com" | ||
| git add action.yml | ||
| git commit -m "chore(release): pin image to ${DIGEST}" |
There was a problem hiding this comment.
[high/correctness] Release workflow fails on second release due to empty commit (confidence: high)
The step "Flip action.yml image to digest-pinned GHCR reference" uses sed to replace "image: Dockerfile" with a pinned image reference. After the first release, action.yml no longer contains that string, so sed makes no change, resulting in no diff. The subsequent git commit command fails with "nothing to commit", breaking the job. Subsequent releases will fail.
Failure scenario: After the first release publishes and pins the image, any further tag push triggers the release workflow, causing the flip step to fail because of the empty commit, preventing the release from completing.
Use sed -i "s|^image: .*|image: docker://${IMAGE}@${DIGEST}|" action.yml to always replace the image line, or check for changes before committing (e.g., git diff --quiet && exit 0).| if: >- | ||
| github.event_name == 'pull_request' || | ||
| ( | ||
| github.event_name == 'issue_comment' && | ||
| github.event.issue.pull_request && | ||
| contains(github.event.comment.body, '@umm review') | ||
| ) |
There was a problem hiding this comment.
[medium/correctness] Self-review workflow may loop if the bot's own review comment contains '@umm review' (confidence: medium)
The workflow triggers on issue_comment events where the comment contains @umm review. Since the bot itself posts a review comment that could inadvertently include that string (either from the LLM output or from the bot's own markdown), the workflow would re-trigger, potentially causing an infinite loop and wasteful API calls.
Failure scenario: The LLM generates a finding that includes the string "@umm review" in a code suggestion or description; the bot posts the review comment, which triggers the workflow again, leading to repeated runs until the action is cancelled or GitHub rate limits are hit.
Add a condition like github.actor != 'umm-actually[bot]' to prevent the bot from triggering itself.| default: combined | ||
| context_budget_tokens: | ||
| description: Approximate token budget for prompt context (conventions + file contents + diff) | ||
| description: Approximate token budget for prompt context (file contents + diff — conventions have a separate cap) |
There was a problem hiding this comment.
[medium/correctness] action.yml claims conventions have a separate token cap, but no such cap exists (confidence: high)
The input description for context_budget_tokens says conventions have a separate cap, implying they are not counted in this budget. However, the implementation reads the conventions file without any token limit and does not apply a separate cap, potentially allowing an arbitrarily large conventions file to blow up the prompt context beyond model limits.
Failure scenario: A repository has a very large conventions file; the action includes the entire file in the prompt, possibly exceeding the model's context window and causing the LLM call to fail or produce truncated analysis, with no budget safeguard.
Update the description to clarify that conventions are included without a strict cap, or implement a token budget cap for conventions.| 4. Builds a structured prompt with randomized delimiter nonces (prompt injection defense) and sends it to OpenRouter | ||
| 5. Validates the response against a strict Zod schema, retrying with a fallback model if the primary fails | ||
| 6. Filters findings by severity threshold, deduplicates overlapping findings, and caps if configured | ||
| 7. Maps findings to inline PR review comments anchored to diff lines, with a snap-to-nearest-hunk fallback |
There was a problem hiding this comment.
[medium/correctness] README claims deduplication of overlapping findings, but no deduplication is performed (confidence: high)
The "How it works" section states that findings are deduplicated after filtering, but the code only uses selectFindings which filters by severity threshold and caps count; no deduplication logic exists. This misleads users about the review quality.
Failure scenario: A user expects the bot to avoid posting duplicate findings for the same issue; they may receive redundant findings, degrading the review's usefulness and violating the documentation promise.
Remove the deduplication claim or implement deduplication logic.
…to the retry ladder (#52) * fix: add per-attempt request timeout so a hung provider call fails into the retry ladder A stalled OpenRouter request previously hung the job until the runner's 6-hour kill: chat.send had no timeout, and the retry/fallback ladder only engages once an attempt fails. Observed live on vault-onboarding PR #7, where a deepseek-v4-flash call sat 17+ minutes with fallback_model unset. - New request_timeout_seconds input (default 600) threaded from action.yml through config into the OpenRouter client - chat.send now passes the SDK's RequestOptions.timeoutMs, so a timed-out attempt aborts the HTTP call and classifies as a status-less retryable api_error — same-model retry, then fallback model Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: cap request_timeout_seconds at the 2^31-1 ms timer limit A positive-integer check alone accepts values whose milliseconds overflow the timer cap (or reach Infinity), which timer implementations clamp to ~1 ms — every request would time out instantly. Bound the input at 2,147,483 seconds so seconds x 1000 always stays a valid delay. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: bound the generation cost lookup with the same request timeout The cost lookup ran with no timeout, so a stalled generations endpoint could hang the job after the review itself was already accepted — the same incident class the per-attempt timeout eliminates. The lookup is best-effort, so a timeout degrades to a null cost with a warning. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: wire request_timeout_seconds from a repo variable in self-review UMM_REQUEST_TIMEOUT_SECONDS tunes the per-attempt timeout from repo settings, matching the OPENROUTER_MODEL pattern. Uses an explicit || '600' fallback because a bare unset var passes an empty string, which would override the action default and fail validation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: treat an empty request_timeout_seconds as the 600 default Workflows wiring a bare unset repo variable pass an empty string, which would otherwise override the action.yml default and fail validation. Empty now means "not provided" — same semantics as max_findings and fallback_model — so bare-var wiring works without an || fallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * style: drop redundant comment from self-review timeout wiring Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 participant
Summary
src/orchestrate.ts— 15-step pipeline composing the I/O clients and pure review modules: event resolution → diff fetch/parse/annotate → context reads (budgeted) → generate findings (via theGenerateFindingsseam) → select → map to inline comments → post one consolidated review. Skip paths after PR identification post body-only reviews stating the reason (§5 amendment).createPromptedGenerateFindingsimplements the V1 default — one prompted combined-phase call with per-run nonce-wrapped delimiters.src/main.ts— replaces the scaffoldsetFailedstub with full client wiring, orchestrate call, output setting, and cost summary write.github/workflows/self_review.yml— dogfood workflow: reviews own PRs onpull_requestand@umm reviewcomment trigger.github/workflows/release.yml— GHCR image build + digest-flip + semver & floating major tag force-move + GitHub ReleaseREADME.md— inputs/outputs tables, usage example with both triggersTest plan
createPromptedGenerateFindingsnonce/model passthrough)not_a_prguard → test fails, (2) remove skip review post → test fails, (3) fix nonce → test failsself_review.ymlfires on this PR (secrets already configured)🤖 Generated with Claude Code
https://claude.ai/code/session_01HBJw5bhDigxu8HM2sPPZVf
Summary by CodeRabbit