feat(llm-council): add --reasoning-effort CLI flag - #97
Open
23r2efewvcs wants to merge 11 commits into
Open
Conversation
Hardcoded max_tokens: 4000 is too small for reasoning models (Nemotron Ultra, GLM-5.2, DeepSeek-V4, Claude thinking) which need 8K-32K output tokens for their reasoning chain or they emit empty content. Introduce RUN_OPTS struct + --max-tokens flag (default 4000, preserves current behavior). Both callOpenAICompat and callAnthropic read from RUN_OPTS.max_tokens. Fixes rohitg00#88.
The HTTP request timeout is hardcoded at 120s in postJSON(). This is too short for slow upstream endpoints (NVIDIA NIM reasoning models, OpenRouter under load), causing ETIMEDOUT / 'council request timeout' failures with no way to extend. Introduce --timeout N flag (default 120000, preserves current behavior) sourced into RUN_OPTS.timeout_ms. Documented in usage(). Depends on rohitg00#89 (--max-tokens, which introduces RUN_OPTS struct). Fixes rohitg00#90.
… backoff Single transient 429 or 5xx kills the entire session. Free-tier endpoints (NVIDIA NIM, OpenRouter :free) are especially prone. Add --max-retries N flag (default 1, preserves current one-shot behavior). Both callOpenAICompat and callAnthropic now: - catch connection-level errors (ETIMEDOUT, ECONNRESET) and retry - retry on 429/5xx with exponential backoff (2s, 4s, 8s...) - fail fast after retries exhausted with clear [ERROR ...] message Depends on rohitg00#91 (--timeout, which introduces RUN_OPTS). Fixes rohitg00#92.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe council CLI now accepts configurable token limits, timeouts, retries, execution order, and reasoning effort. Provider requests apply these settings. Phase 1 and Phase 2 calls use the shared execution runner. ChangesCouncil runtime options
Estimated code review effort: 3 (Moderate) | ~25 minutes Mergeability Score: 🔵 Low · up to The PR adds a reasoning-effort option that changes model request behavior. A localized CLI validation issue can accept malformed numeric values and cause bounded configuration errors, so owner follow-up is recommended, but the PR remains mergeable. Sequence Diagram(s)sequenceDiagram
participant CLI
participant cmdRun
participant runCalls
participant ModelAPI
CLI->>cmdRun: Provide runtime flags and council query
cmdRun->>runCalls: Execute Phase 1 and Phase 2 callables
runCalls->>ModelAPI: Send configured model requests
ModelAPI-->>runCalls: Return results or retryable errors
runCalls-->>cmdRun: Return settled results
cmdRun-->>CLI: Report council output
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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}
Contributor
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 `@skills/llm-council/scripts/council.js`:
- Around line 214-218: Validate the runtime options before assigning them in the
argument-processing block: require max-tokens, timeout, and max-retries to be
integers within their supported non-negative ranges, rejecting partial or
invalid values instead of assigning NaN; ensure max-retries cannot permit
unbounded retry loops. Restrict reasoning-effort to low, medium, or high, and
preserve the existing RUN_OPTS assignments for valid inputs.
- Around line 335-347: Add documentation in skills/llm-council/SKILL.md for the
new runtime command-line flags that are already displayed in the usage() output:
--max-tokens, --timeout, --max-retries, --sequential, and --reasoning-effort.
Include descriptions matching the usage output, such as the default values for
--max-tokens (4000), --timeout (120000), --max-retries (1), the purpose of
--sequential for avoiding concurrent-request timeouts, and the
--reasoning-effort parameter (low|medium|high) for o1/o3/DeepSeek/Claude
thinking models. Ensure the documented command contract in SKILL.md aligns with
what usage() displays.
- Around line 222-229: Update runCalls so concurrent mode invokes every callable
before passing the resulting promises to Promise.allSettled, preserving the
existing settled-result shape and sequential behavior. Ensure both Phase 1 and
Phase 2 receive actual provider responses rather than function objects.
- Line 133: Update the payload construction guarded by RUN_OPTS.reasoning_effort
to use Anthropic’s adaptive thinking configuration, setting thinking.type to
adaptive and forwarding RUN_OPTS.reasoning_effort through output_config.effort.
Remove the fixed budget_tokens calculation so low, medium, and high effort
levels are preserved.
🪄 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: 31267fa2-37c8-4a9d-b3c7-f08a7b4a4fe3
📒 Files selected for processing (1)
skills/llm-council/scripts/council.js
23r2efewvcs
added a commit
to 23r2efewvcs/pro-workflow
that referenced
this pull request
Jul 31, 2026
- Fix runCalls: invoke callables before Promise.allSettled - Add parseIntSafe: validate CLI options are positive integers - Validate --reasoning-effort: only low|medium|high accepted - Fix Anthropic thinking: use adaptive type instead of fixed budget_tokens Addresses CodeRabbit comments on PR rohitg00#97.
- Add parseIntSafe: validate --max-tokens, --timeout, --max-retries are positive integers, reject NaN/negative/non-integer - Update SKILL.md with new CLI flags and runtime options table Addresses CodeRabbit comments on PR rohitg00#93.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skills/llm-council/scripts/council.js (1)
127-133: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winForward
reasoning_effortintooutput_config.effort.
payload.thinking = { type: 'adaptive' }enables adaptive thinking, but the request still omitsoutput_config.effort. Anthropic adaptive thinking expects effort as a separate top-leveloutput_config.effortvalue. Without it,--reasoning-effort low,medium, andhighsend no explicit effort guidance and may not change Claude’s reasoning depth.🔧 Proposed fix
const payload = { model, max_tokens: RUN_OPTS.max_tokens, system, messages: [{ role: 'user', content: user }], }; - if (RUN_OPTS.reasoning_effort) payload.thinking = { type: 'adaptive' }; + if (RUN_OPTS.reasoning_effort) { + payload.thinking = { type: 'adaptive' }; + payload.output_config = { effort: RUN_OPTS.reasoning_effort }; + }🤖 Prompt for 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. In `@skills/llm-council/scripts/council.js` around lines 127 - 133, Update the payload construction in the council request flow to include RUN_OPTS.reasoning_effort as output_config.effort whenever reasoning_effort is provided, while retaining payload.thinking = { type: 'adaptive' }. Ensure low, medium, and high values are forwarded explicitly without changing behavior when the option is absent.
🤖 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.
Outside diff comments:
In `@skills/llm-council/scripts/council.js`:
- Around line 127-133: Update the payload construction in the council request
flow to include RUN_OPTS.reasoning_effort as output_config.effort whenever
reasoning_effort is provided, while retaining payload.thinking = { type:
'adaptive' }. Ensure low, medium, and high values are forwarded explicitly
without changing behavior when the option is absent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 831a5320-9d00-46ee-88f7-311db331e955
📒 Files selected for processing (1)
skills/llm-council/scripts/council.js
Both callOpenAICompat and callAnthropic duplicated the same retry/backoff
loop. Extract a shared postJSONWithRetry(url, payload, headers) helper that
returns { res } or { error }, preserving max_retries, 429/5xx handling,
connection-error handling, and exponential backoff (2s, 4s, 8s...).
Addresses CodeRabbit comment on PR rohitg00#93.
Phase 1 and Phase 2 run all model calls concurrently via Promise.allSettled. On free-tier endpoints with strict concurrent-request limits (NVIDIA NIM free: 1 concurrent per API key, OpenRouter :free under load), this causes ETIMEDOUT or 429 on the 2nd/3rd call. Add --sequential flag (default: parallel, preserves current behavior). When set, calls run one at a time via a runCalls() helper that serializes execution. Slower but avoids concurrent-request rejections. Depends on rohitg00#93 (--max-retries, which introduces the retry loop). Fixes rohitg00#94.
- Fix runCalls: invoke callables before Promise.allSettled - Add parseIntSafe: validate CLI options are positive integers - Update SKILL.md with --max-tokens, --timeout, --max-retries, --sequential Addresses CodeRabbit comments on PR rohitg00#95.
- Add shell continuation backslash to multiline command in SKILL.md - Clarify phases 1 and 2 run in parallel by default; --sequential overrides - Document that --max-retries covers connection errors in help text Addresses CodeRabbit comments on PR rohitg00#95.
Modern reasoning models (OpenAI o1/o3, DeepSeek R1/V4, Claude thinking) accept a reasoning_effort parameter that controls compute budget. Without it, models either waste tokens on simple queries or truncate reasoning on complex ones. Add --reasoning-effort low|medium|high flag (default: not set, preserves current behavior). When set: - OpenAI-compat: adds reasoning.effort to payload - Anthropic: adds thinking.budget_tokens (derived from effort level) Depends on rohitg00#95 (--sequential). Fixes rohitg00#96.
- Fix runCalls: invoke callables before Promise.allSettled - Add parseIntSafe: validate CLI options are positive integers - Validate --reasoning-effort: only low|medium|high accepted - Fix Anthropic thinking: use adaptive type instead of fixed budget_tokens Addresses CodeRabbit comments on PR rohitg00#97.
- Send reasoning_effort as output_config.effort for Anthropic adaptive thinking - Document --reasoning-effort in SKILL.md command synopsis and runtime table Addresses CodeRabbit comments on PR rohitg00#97.
23r2efewvcs
force-pushed
the
feat/llm-council-reasoning-effort
branch
from
August 13, 2026 11:39
0f6792c to
45a7fe2
Compare
23r2efewvcs
added a commit
to 23r2efewvcs/pro-workflow
that referenced
this pull request
Aug 13, 2026
callOpenAICompat reads content as data.choices[0].message.content.
This fails for reasoning models which return content in non-standard
fields:
- NVIDIA NIM reasoning: message.reasoning (string)
- DeepSeek / GLM: message.reasoning_content (string)
- Some OpenRouter models: message.content as array [{type:'output',text}]
- Claude thinking via compat: message.thinking (string)
Add extractContent(data) function that tries 6 known formats in
priority order before falling back to empty string. Replaces the
single-line extraction in callOpenAICompat.
Depends on rohitg00#97 (--reasoning-effort).
Fixes rohitg00#98.
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.
Depends on #95.
Fixes #96.
What
Modern reasoning models (OpenAI o1/o3, DeepSeek R1/V4, Claude with thinking) accept a
reasoning_effortparameter that controls how much compute they spend on internal reasoning. Without it, models either waste tokens on simple queries or truncate reasoning on complex ones.Change
reasoning_effort: nulltoRUN_OPTScallOpenAICompat: addreasoning: { effort }to payload when setcallAnthropic: addthinking: { type: 'enabled', budget_tokens }when set (budget = min(max_tokens * 0.8, 16000))--reasoning-effort low|medium|highincmdRunusage()After this PR
Diff: 12 +, 6 -.
Summary by CodeRabbit