Skip to content

feat: add --temperature CLI flag (default 0)#6

Open
Priyansh2116 wants to merge 2 commits into
Cyberfilo:mainfrom
Priyansh2116:feat/temperature-flag
Open

feat: add --temperature CLI flag (default 0)#6
Priyansh2116 wants to merge 2 commits into
Cyberfilo:mainfrom
Priyansh2116:feat/temperature-flag

Conversation

@Priyansh2116

Copy link
Copy Markdown

Summary

Closes #1

  • Adds a --temperature Click option (float, default 0.0, shown in --help) to the CLI
  • Stores temperature on both AnthropicClient and OpenAIClient and uses it in generate()
  • Threads it through make_client() so both the SQL-generator and selector LLM respect the flag
  • Behaviour is completely unchanged when --temperature is omitted (default 0 preserves existing determinism)

Test plan

  • promptquery --help shows --temperature FLOAT Sampling temperature passed to the LLM (0 = deterministic). [default: 0.0]
  • Existing tests pass (pytest tests/)
  • --temperature 0.7 forwards the value to the LLM API call

🤖 Generated with Claude Code

Expose the LLM sampling temperature as a `--temperature` Click option
(float, default 0.0) so callers can trade determinism for creativity.
Threads through `make_client()` into both `AnthropicClient` and
`OpenAIClient`; behaviour is unchanged when the flag is omitted.

Closes Cyberfilo#1

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Cyberfilo

Copy link
Copy Markdown
Owner

Thanks @Priyansh2116 — clean, and default=0.0 keeping determinism intact is exactly right for this project (reproducible SQL is a feature here).

Two technical things before I pull it in:

1. Range validation. type=float accepts anything, but the value goes straight to providers with different valid domains — Anthropic rejects temperature > 1, OpenAI allows up to 2. Today --temperature 5 reaches the API and fails with a provider error the user can't decode. Could you switch to type=click.FloatRange(0.0, 2.0)? That turns an opaque 400 into a clean CLI message.

2. Reasoning-model branch. In OpenAIClient.generate, the GPT-5.x / o1 / o3 path sets max_completion_tokens and never adds temperature to kwargs — so the flag is silently ignored there (correct, those models reject it). Could you drop a one-line comment so the next reader doesn't "fix" it?

Heads-up on a seam: #7 (Ollama) currently hardcodes temperature=0, so once both land we'll want to thread the flag there too for consistency — not this PR's job, just flagging it. Ticking your test-plan boxes (or pasting the --help output) would let me green-light faster. 👍

- Switch type=float to type=click.FloatRange(0.0, 2.0) so out-of-range
  values produce a clean CLI error instead of an opaque provider 400
- Add a comment in the OpenAI reasoning-model branch explaining why
  self.temperature is not forwarded there (those models reject it)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Priyansh2116

Copy link
Copy Markdown
Author

Thanks for the detailed review!

  1. Range validation — switched to type=click.FloatRange(0.0, 2.0). Clean CLI error now instead of a provider 400.
  2. Reasoning-model comment — added # self.temperature is intentionally not forwarded here. on the line after the existing comment so the next reader understands the omission is deliberate.

Both changes are in the latest commit. Happy to thread the flag into the Ollama client (#7) in a follow-up once that PR lands.

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.

Add a --temperature flag (default 0)

2 participants