Skip to content

feat(chat): add --temperature flag to opencli chat#279

Open
zjshen14 wants to merge 1 commit into
mainfrom
fix/issue-251-chat-temperature
Open

feat(chat): add --temperature flag to opencli chat#279
zjshen14 wants to merge 1 commit into
mainfrom
fix/issue-251-chat-temperature

Conversation

@zjshen14

Copy link
Copy Markdown
Owner

Summary

  • opencli run accepted --temperature but opencli chat had no such flag, so interactive-session users had no per-invocation way to control temperature.
  • This PR adds --temperature <float> to the chat command and threads it through startChat()createAgent(), bringing chat to parity with run.
  • Complements PR fix: apply config.temperature to LLM client and expose it via opencli config #252 (which fixes the config.temperature fallback in createAgent() and adds opencli config --temperature). The two PRs touch non-overlapping lines and can be merged in any order.

Related issue

Part of #251

Test plan

  • npm run typecheck && npm run lint && npm run format:check && npm test — all pass (733 tests)
  • opencli chat --temperature 0 and opencli chat --temperature 1.5 are accepted; --temperature 3 correctly rejects with validation error
  • opencli chat with no flag continues to work unchanged (temperature is undefined, createAgent() defaults apply)

Notes for reviewers

This is intentionally narrow — only the chat command definition and startChat() signature change (4 lines added). The temperature ?? config.temperature fallback fix is handled separately in PR #252 to keep each PR focused and conflict-free.


Generated by Claude Code

`opencli run` accepted --temperature but `opencli chat` had no such flag,
forcing interactive-session users to set temperature only via config file.
This brings chat to parity with run.

Part of #251
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@zjshen14

Copy link
Copy Markdown
Owner Author

Review: feat(chat): add --temperature flag to opencli chat

What's good: This is exactly right. The change is four lines that mirror what opencli run already does — register the option, pass it down through startChat() to createAgent(). Using the same parseTemperature validator ensures consistent range checking between the two commands. CI is fully green across all three checks, mergeable_state is clean.


Notes

1. No new unit test for the CLI flag path

The 733 existing tests continue to pass, and the change is a pure threading operation (no new logic). That said, a test that stubs startChat and asserts it receives opts.temperature when --temperature is passed would lock in the wiring and catch a future refactor that drops the parameter. Not a blocker for this PR, but worth a follow-up once PR #252 also lands (at that point a single test could cover chat --temperature 0.3createAgent receives 0.3, and chat with no flag → createAgent receives undefined which then resolves to config.temperature).

2. Interaction with #230 (o1 temperature guard)

Explicitly passing --temperature 1.5 --model o1-mini via chat will fail at the OpenAI API layer (o1 models reject any temperature parameter). This is pre-existing behaviour for opencli run --temperature ... and is not made worse by this PR — both commands share the same createAgent() / openai.ts code path. PR #230 fixes it at the provider layer for both commands at once.

3. No conflicts with #252

The two PRs touch non-overlapping line ranges in src/cli/index.ts. As noted in the PR description, merge order is flexible — though once #252 lands, the end-to-end story (flag → config fallback → LLM call) will be complete.


Recommendation: Merge. The change is a clean, narrow parity feature with no risk of regression. The only open item (o1 temperature guard) is a pre-existing gap tracked under #230.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants