fix DFanso#120 : Replace magic numbers with constants in LLM providers

[adeeshperera]

• Defined constants for model names, token limits, temperature settings, API endpoints, and headers in the following modules:
  • Claude
  • Groq
  • Gemini
  • ChatGPT
  • Grok
  • Ollama
• Improved code maintainability and consistency by eliminating hardcoded values.

Summary by CodeRabbit

• Chores
  • Internal code organization improvements across AI provider integrations to enhance maintainability and configuration consistency.

[coderabbitai]

Walkthrough

This PR centralizes configuration across multiple AI provider integrations (ChatGPT, Claude, Gemini, Grok, Groq, Ollama) by extracting hard-coded string and numeric literals into package-level constants. No functional behavior changes; only internal constant definitions and their usage are modified.

Changes

Cohort / File(s)	Summary
Configuration constant extraction across AI provider integrations internal/chatgpt/chatgpt.go, internal/claude/claude.go, internal/gemini/gemini.go, internal/grok/grok.go, internal/groq/groq.go, internal/ollama/ollama.go	Introduces package-level constants for model names, temperature, max tokens, API endpoints, content-type headers, and authentication headers. Replaces hard-coded literals with corresponding constants in request construction, HTTP headers, and client configuration. No changes to public APIs or control flow.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Homogeneous, repetitive refactoring pattern applied consistently across six related files. Each change follows an identical approach: define constants and replace literals. No complex logic, no public API changes, no control-flow modifications.

Possibly related PRs

• added ollama support for running local LLM's, Two new environment var… #43: Introduces Ollama support with initial defaults; this PR refactors that file by extracting hard-coded Ollama configuration (model, stream flag, content-type) into package constants.

Suggested labels

enhancement, go

Suggested reviewers

• DFanso

Poem

🐰 Constants hop where hardcodes once played,
Config strings bundled, neat and arrayed,
Six providers now share a cleaner design,
No logic shifts—just values aligned! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The PR title "Replace magic numbers with constants in LLM providers" directly and accurately describes the core objective of the changeset. All six modified files (chatgpt, claude, gemini, grok, groq, ollama) follow the exact same pattern: introducing new constants to replace hardcoded values (model names, tokens, temperatures, API endpoints, and headers). The title is concise, clear, and specific enough that a developer scanning the repository history would immediately understand the primary change. The reference to issue #120 provides appropriate context, and the terminology "magic numbers" is a widely accepted convention for referring to hardcoded literal values in code.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

internal/claude/claude.go (1)

14-22: Consider consistent naming convention for all constants.

While the constant extraction is correct, the naming is inconsistent:

• Some constants use the claude prefix: claudeModel, claudeMaxTokens, claudeAPIEndpoint, claudeAPIVersion
• Others use generic names: contentTypeJSON, xAPIKeyHeader, anthropicVersionHeader

For consistency with other provider files (grok, groq, ollama) that prefix all constants, consider renaming to:

• claudeContentType instead of contentTypeJSON
• claudeAPIKeyHeader instead of xAPIKeyHeader
• claudeVersionHeader instead of anthropicVersionHeader

This makes it clearer that these are Claude-specific constants and improves consistency across the codebase.

Apply this diff for consistent naming:

 const (
 	claudeModel        = "claude-3-5-sonnet-20241022"
 	claudeMaxTokens    = 200
 	claudeAPIEndpoint  = "https://api.anthropic.com/v1/messages"
 	claudeAPIVersion   = "2023-06-01"
-	contentTypeJSON    = "application/json"
-	anthropicVersionHeader = "anthropic-version"
-	xAPIKeyHeader      = "x-api-key"
+	claudeContentType      = "application/json"
+	claudeVersionHeader    = "anthropic-version"
+	claudeAPIKeyHeader     = "x-api-key"
 )

Then update the usages accordingly:

-	req.Header.Set("Content-Type", contentTypeJSON)
-	req.Header.Set(xAPIKeyHeader, apiKey)
-	req.Header.Set(anthropicVersionHeader, claudeAPIVersion)
+	req.Header.Set("Content-Type", claudeContentType)
+	req.Header.Set(claudeAPIKeyHeader, apiKey)
+	req.Header.Set(claudeVersionHeader, claudeAPIVersion)

internal/grok/grok.go (1)

14-20: Minor naming inconsistency in constant prefix.

Most constants correctly use the grok prefix (grokModel, grokTemperature, grokAPIEndpoint, grokContentType), but authorizationPrefix lacks it.

For consistency, consider renaming to grokAuthorizationPrefix:

 const (
 	grokModel          = "grok-3-mini-fast-beta"
 	grokTemperature    = 0
 	grokAPIEndpoint    = "https://api.x.ai/v1/chat/completions"
 	grokContentType    = "application/json"
-	authorizationPrefix = "Bearer "
+	grokAuthorizationPrefix = "Bearer "
 )

Then update line 54:

-	req.Header.Set("Authorization", fmt.Sprintf("%s%s", authorizationPrefix, apiKey))
+	req.Header.Set("Authorization", fmt.Sprintf("%s%s", grokAuthorizationPrefix, apiKey))

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 12b8d33 and 192de2d.

📒 Files selected for processing (6)

• internal/chatgpt/chatgpt.go (2 hunks)
• internal/claude/claude.go (3 hunks)
• internal/gemini/gemini.go (2 hunks)
• internal/grok/grok.go (3 hunks)
• internal/groq/groq.go (3 hunks)
• internal/ollama/ollama.go (4 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: Build Go Binary (windows-latest)
• GitHub Check: Test (1.23)

🔇 Additional comments (7)

internal/chatgpt/chatgpt.go (1)

13-15: LGTM! Clean constant extraction.

The model constant is well-named and properly applied. This improves maintainability by centralizing the model configuration.

Also applies to: 29-29

internal/gemini/gemini.go (1)

13-16: LGTM! Model and temperature constants properly extracted.

The constants follow good naming conventions and centralize configuration values effectively.

Also applies to: 33-34

internal/ollama/ollama.go (1)

14-18: LGTM! Constants properly extracted and applied.

All hardcoded values are now centralized as named constants with clear prefixes. The implementation is consistent and improves maintainability.

Also applies to: 37-37, 47-47, 60-60

internal/claude/claude.go (1)

47-48: Constants properly applied throughout the function.

The extracted constants are correctly used in the request construction, successfully eliminating the magic values.

Also applies to: 63-63, 68-70

internal/groq/groq.go (2)

39-45: LGTM! Constants are well-defined and consistently applied.

All constants follow the groq prefix convention and are properly used throughout the request construction. The temperature, token limits, system message, content type, and authorization prefix are now centralized.

Also applies to: 73-74, 76-76, 96-97

---

47-56: Clarify the purpose of httpClient initialization.

The addition of a package-level httpClient variable and init() function appears to be beyond the scope of extracting magic numbers/constants. The comment indicates it's for test overrides, but this pattern differs from other provider files that call httpClient.GetClient() directly.

Is this change necessary for this PR, or should it be separated into a different PR focused on improving testability?

If this is intentional for testing purposes, consider:

1. Adding a comment explaining why Groq needs this pattern while other providers don't
2. Potentially applying this pattern consistently across all providers in a separate PR

internal/grok/grok.go (1)

36-36: Constants properly applied throughout.

The extracted constants are correctly used in request construction, successfully eliminating hardcoded values and improving maintainability.

Also applies to: 38-38, 47-47, 53-54

[DFanso]

approved 🎊