Skip to content

fix(ai): wire configured max_tokens and temperature onto requests#207

Open
ramnnn2006 wants to merge 1 commit into
optiqor:mainfrom
ramnnn2006:fix/ai-analyzer-request-tokens-temp
Open

fix(ai): wire configured max_tokens and temperature onto requests#207
ramnnn2006 wants to merge 1 commit into
optiqor:mainfrom
ramnnn2006:fix/ai-analyzer-request-tokens-temp

Conversation

@ramnnn2006

Copy link
Copy Markdown
Contributor

What

DefaultAnalyzer.Analyze built the CompletionRequest without MaxTokens or Temperature, so the documented request-level fields never carried through the only real caller. anyone relying on per-request values had them silently dropped.

Why

Fixes #120

How

added MaxTokens and Temperature to AnalyzerConfig and DefaultAnalyzer, set them on the request in Analyze, and buildAnalyzer in doctor.go passes the configured values through. zero stays zero so providers keep their own fallback intact.

Testing

  • go build ./... passes

  • go test ./... passes

  • go vet ./... passes

  • golangci-lint run ./... passes

  • Tested locally with:

  • N/A - pure docs/refactor

Checklist

  • PR title follows Conventional Commits (feat(scope): subject)
  • All commits are DCO-signed (git commit -s)
  • No unrelated changes pulled in
  • Documentation updated where user-visible behavior change
  • Added/updated tests for new code paths
  • If a new doctor rule, paired with a chaos scenario in `s

@ramnnn2006 ramnnn2006 requested a review from btwshivam as a code owner June 8, 2026 19:19
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

🚀 First PR — welcome aboard!

A few things to expect:

  1. CI: every PR runs build + race tests + lint + (eventually) the kernel matrix. If something fails, the log will tell you exactly which gate.
  2. DCO: every commit needs Signed-off-by:git commit -s adds it automatically.
  3. Conventional Commits: PR titles like feat(doctor): add new rule or fix(bpf): handle X. We squash-merge by default.
  4. Review: a maintainer will review within 72 hours. Suggestions are conversations, not orders — push back if something doesn't fit your context.

If you get stuck, reply here or jump to Discussions. We want this PR to land.

@github-actions github-actions Bot added level:critical Touches BPF, security, or release surfaces (auto-applied) testing Tests and test coverage area/integrations External integrations (sinks, exports, CI) and removed level:critical Touches BPF, security, or release surfaces (auto-applied) labels Jun 8, 2026
DefaultAnalyzer.Analyze built the CompletionRequest without MaxTokens or
Temperature, so the documented request-level fields never carried through
the only real caller. It happened to work because each provider falls back
to its own default, but anyone relying on the per-request values had them
silently dropped.

Thread the configured values through AnalyzerConfig onto the request so the
request-level fields are honored, with consistent precedence: configured
values ride the request, and a zero value leaves the provider default intact.

Fixes optiqor#120

Signed-off-by: ramnnn2006 <phrkvvp@gmail.com>
@ramnnn2006 ramnnn2006 force-pushed the fix/ai-analyzer-request-tokens-temp branch from 4fea552 to 532e8ce Compare June 8, 2026 19:23
@github-actions github-actions Bot added the level:critical Touches BPF, security, or release surfaces (auto-applied) label Jun 8, 2026
@ramnnn2006

Copy link
Copy Markdown
Contributor Author

my badd, just looked into it and fixed the gofmt formatting issue as it wasn't pushed to the fork repo while creating this pr.
could you check the ci again now?

@ramnnn2006

ramnnn2006 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

@btwshivam hi ,
if the changes look fine, could you also add the type:bug , the code quality ( clean/exceptional) and gssoc:approved labels? ty

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

Labels

area/integrations External integrations (sinks, exports, CI) level:critical Touches BPF, security, or release surfaces (auto-applied) testing Tests and test coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(ai): analyzer drops configured max_tokens and temperature

1 participant