Conversation
…fig gate, local model reviewers Fixes the false-stall detection bug in the plan→review→replan convergence loop. REVIEWS.md accumulates history across cycles so raw grep inflated HIGH counts; HIGH count now comes from a per-cycle CYCLE_SUMMARY contract emitted in the review agent's return message. Key changes: - workflow.plan_review_convergence config gate (disabled by default, same pattern as workflow.code_review / workflow.nyquist_validation) - Review agent prompt defines CYCLE_SUMMARY: current_high=<N> contract with PARTIALLY RESOLVED / FULLY RESOLVED counting rules - Orchestrator aborts on absent/malformed CYCLE_SUMMARY (distinguishes both) - Warns when HIGH_COUNT > 0 but ## Current HIGH Concerns section is missing - Stall detection and --ws forwarding preserved and tested - Local model reviewers: --ollama, --lm-studio, --llama-cpp flags added to convergence workflow and review workflow; all three use OpenAI-compatible /v1/chat/completions endpoint with jq --rawfile for safe JSON encoding - review.ollama_host / review.lm_studio_host / review.llama_cpp_host config keys registered and documented (default to localhost:11434/1234/8080) - review.models.ollama / .lm_studio / .llama_cpp model-name config support - 58 tests (up from 29 in PR #2339), all passing Closes #2306 Closes #2339 Co-authored-by: Tom Boucher <trekkie@nomorestars.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 46 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds local OpenAI-compatible reviewer backends (Ollama, LM Studio, llama.cpp) to the plan-review-convergence flow, adds a config gate Changes
Sequence Diagram(s)sequenceDiagram
participant Orchestrator as Orchestrator<br/>(plan-review-convergence)
participant Config as Config<br/>Store
participant LocalServer as Local Server<br/>(Ollama/LM Studio/llama.cpp)
participant ReviewAgent as Review Agent
participant Output as REVIEWS.md
Orchestrator->>Config: Read `review.*_host` and model keys
Config-->>Orchestrator: Host URL(s) & model path(s)
Orchestrator->>LocalServer: GET /v1/models (probe)
alt server responds
LocalServer-->>Orchestrator: Models list
Orchestrator->>ReviewAgent: Spawn gsd-review with chosen backend & model
ReviewAgent->>LocalServer: POST /v1/chat/completions (jq-embedded prompt)
LocalServer-->>ReviewAgent: Completion (includes CYCLE_SUMMARY)
ReviewAgent->>Output: Write review sections + CYCLE_SUMMARY
Output-->>Orchestrator: Parsed current_high & concerns
else server unreachable
LocalServer-->>Orchestrator: error
Orchestrator->>Orchestrator: Skip reviewer / emit warning
end
Orchestrator->>Orchestrator: Loop/replan until current_high == 0 or max cycles reached
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Add workflow.plan_review_convergence, review.ollama_host, review.lm_studio_host, and review.llama_cpp_host to the SDK-side TypeScript mirror — required by the CJS↔SDK parity test (#2653). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/CONFIGURATION.md (1)
198-198: Broaden the convergence description to include local reviewers.Line 198 currently names only Codex/Gemini/Claude/OpenCode, but this PR also supports Ollama/LM Studio/llama.cpp in the same loop. Updating that sentence would keep behavior docs fully consistent.
📝 Suggested wording tweak
-| `workflow.plan_review_convergence` | boolean | `false` | Enable the `/gsd-plan-review-convergence` command. Disabled by default — the command exits with an enable instruction when this key is `false`. The command automates the manual plan→review→replan loop: it spawns external AI reviewers (Codex, Gemini, Claude, OpenCode), counts unresolved HIGH concerns via the CYCLE_SUMMARY contract, replans with `--reviews` feedback, and repeats until converged or max cycles reached. Enable with `gsd config-set workflow.plan_review_convergence true`. Added in v1.39 | +| `workflow.plan_review_convergence` | boolean | `false` | Enable the `/gsd-plan-review-convergence` command. Disabled by default — the command exits with an enable instruction when this key is `false`. The command automates the manual plan→review→replan loop: it spawns configured reviewers (Codex, Gemini, Claude, OpenCode, Ollama, LM Studio, llama.cpp), counts unresolved HIGH concerns via the CYCLE_SUMMARY contract, replans with `--reviews` feedback, and repeats until converged or max cycles reached. Enable with `gsd config-set workflow.plan_review_convergence true`. Added in v1.39 |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/CONFIGURATION.md` at line 198, The docs entry for the configuration key `workflow.plan_review_convergence` lists only external AI reviewers (Codex, Gemini, Claude, OpenCode) but omits local/embedded reviewers; update the sentence to also mention Ollama, LM Studio, and llama.cpp (or more generally "local reviewers") so the description reflects that the convergence loop spawns both external AI reviewers and local reviewers (Ollama/LM Studio/llama.cpp) when counting HIGH concerns via the CYCLE_SUMMARY contract and replanning until convergence or max cycles.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@get-shit-done/workflows/plan-review-convergence.md`:
- Around line 193-194: The HIGH_COUNT extraction can return multiple numbers;
update the extraction of HIGH_COUNT from REVIEW_AGENT_RETURN to perform a single
anchored match (start-of-line) and return only one numeric value (e.g., use a
regex anchored with ^ and a single-match option like grep -m1 or use a PCRE \K
to capture only the digits) so HIGH_COUNT is always a single integer; apply the
same anchored single-match fix to the other occurrences that parse current_high
in this file (the other extraction sites referencing
REVIEW_AGENT_RETURN/current_high).
In `@get-shit-done/workflows/review.md`:
- Line 324: The frontmatter line "reviewers:" currently hardcodes all possible
reviewers but should reflect the actual set used at runtime; update the code
that builds the workflow frontmatter to derive the reviewers list from the same
selected/available reviewer set used by invoke_reviewers (reuse the same
variable/logic instead of a static array), ensuring the generated frontmatter is
populated programmatically with that set so it matches which reviewers (e.g.,
codex, ollama, etc.) are actually enabled for the run.
---
Nitpick comments:
In `@docs/CONFIGURATION.md`:
- Line 198: The docs entry for the configuration key
`workflow.plan_review_convergence` lists only external AI reviewers (Codex,
Gemini, Claude, OpenCode) but omits local/embedded reviewers; update the
sentence to also mention Ollama, LM Studio, and llama.cpp (or more generally
"local reviewers") so the description reflects that the convergence loop spawns
both external AI reviewers and local reviewers (Ollama/LM Studio/llama.cpp) when
counting HIGH concerns via the CYCLE_SUMMARY contract and replanning until
convergence or max cycles.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 40aa6ad9-bbf0-4941-a1f6-f7c42b2ef898
📒 Files selected for processing (6)
commands/gsd/plan-review-convergence.mddocs/CONFIGURATION.mdget-shit-done/bin/lib/config-schema.cjsget-shit-done/workflows/plan-review-convergence.mdget-shit-done/workflows/review.mdtests/plan-review-convergence.test.cjs
- Anchor HIGH_COUNT extraction with head -1 to prevent multi-match when agent return message contains multiple CYCLE_SUMMARY lines (e.g. quoted back from prompt context) - Replace hardcoded reviewers list in REVIEWS.md frontmatter template with runtime-derived placeholder — the static list did not reflect which reviewers were actually invoked - Broaden workflow.plan_review_convergence docs to include local reviewers (Ollama, LM Studio, llama.cpp) alongside cloud reviewers Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The cursor-reviewer.test.cjs (and equivalent per-reviewer tests) assert that each supported reviewer appears on the reviewers: line — these are wiring tests that catch when a new reviewer is added to invocation but not to the REVIEWS.md template. Replacing the list with a placeholder broke those tests. Restore the full static list and add an inline comment clarifying that the actual committed frontmatter should be filtered to only the reviewers invoked that run — satisfying both the per-reviewer tests and the CodeRabbit correctness note. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.
Fix PR
Linked Issue
Fixes #2306
Closes #2339
What was broken
The plan→review→replan convergence loop had a false-stall detection bug: REVIEWS.md accumulates history across cycles, so a raw
grepfor HIGH count on cycle 2+ would find resolved HIGHs from prior cycles, inflating the count and triggering spurious stall warnings even when the plan was converging correctly.Additionally, the
workflow.plan_review_convergencefeature had no opt-in config gate (it ran unconditionally), and there was no support for local model reviewers (Ollama, LM Studio, llama.cpp).What this fix does
CYCLE_SUMMARY: current_high=<N>in its return message. The orchestrator parses this per-cycle value instead of grepping the accumulated REVIEWS.md file, eliminating the false-stall inflation.workflow.plan_review_convergence(disabled by default, same pattern asworkflow.code_review). Command exits with enable instructions when the key isfalse.--ollama,--lm-studio,--llama-cppflags. All three use the OpenAI-compatible/v1/chat/completionsendpoint. Host and model are configurable; host defaults tolocalhost:11434/1234/8080respectively.Root cause
The orchestrator read HIGH count by grepping the REVIEWS.md artifact file rather than receiving it from the review agent directly. Because REVIEWS.md is append-only (prior cycle findings remain as audit trail), the grep count was cumulative across cycles rather than reflecting only the current cycle's unresolved issues.
Testing
How I verified the fix
node --test tests/plan-review-convergence.test.cjs— 58/58 passnode --test tests/config-schema-sdk-parity.test.cjs— 2/2 passnode --test tests/config-schema-docs-parity.test.cjs— 1/1 pass (all new schema keys documented)Regression test added?
"workflow extracts HIGH count from CYCLE_SUMMARY contract, NOT from grepping REVIEWS.md"with inline comment explaining the false-stall mechanismPlatforms tested
Runtimes tested
Checklist
Fixes #NNN— PR will be auto-closed if missingconfirmed-buglabelnpm test)Breaking changes
None — the feature was not previously enabled by default. Users who had manually invoked
/gsd-plan-review-convergencewill now need to setworkflow.plan_review_convergence=truefirst; the command exits with clear instructions if the gate is not enabled.Additional scope: local model reviewer support
The local model flags (
--ollama,--lm-studio,--llama-cpp) and config keys (review.ollama_host,review.lm_studio_host,review.llama_cpp_host,review.models.ollama,review.models.lm_studio,review.models.llama_cpp) were added as part of the same fix cycle per the issue thread. All three backends usejq --rawfileto safely encode multi-line prompts as JSON before posting to the OpenAI-compatible endpoint.🤖 Generated with Claude Code
Summary by CodeRabbit