feat: implement Groq adapter - #568
jhosepm352-design wants to merge 1 commit into
Conversation
Greptile SummaryThis PR refactors the Groq AI adapter — simplifying the
Confidence Score: 1/5Not safe to merge — the adapter is completely non-functional as written and also leaks API keys in error messages. Every call to packages/ai/groq/src/index.ts — all four issues (two undefined variables, API-key redaction, and trailing-slash normalization) are concentrated in this single file.
|
| Filename | Overview |
|---|---|
| packages/ai/groq/src/index.ts | Two undefined variables (Groq_LOWER, Groq) cause immediate ReferenceErrors; redact() removal leaks API keys in error messages; trailing-slash normalization dropped breaking the baseUrl config path; model list downgraded to older identifiers. |
Sequence Diagram
sequenceDiagram
participant Caller
participant GroqAdapter
participant GroqAPI
Caller->>GroqAdapter: generate(ctx, prompt, opts, config)
GroqAdapter->>GroqAdapter: ctx.secret('GROQ_API_KEY')
GroqAdapter->>GroqAdapter: "ctx.log(`${Groq_LOWER}...`) 💥 ReferenceError"
Note over GroqAdapter: Execution stops here — never reaches fetch
alt If ReferenceError were fixed
GroqAdapter->>GroqAPI: POST /v1/chat/completions
GroqAPI-->>GroqAdapter: non-2xx response
GroqAdapter->>GroqAdapter: "throw new Error(`${Groq} ...`) 💥 ReferenceError"
Note over GroqAdapter: Error path also broken
end
alt Happy path (both bugs fixed)
GroqAdapter->>GroqAPI: POST /v1/chat/completions
GroqAPI-->>GroqAdapter: 200 OK + choices/usage
GroqAdapter-->>Caller: "{ text, model, inputTokens, outputTokens }"
end
Reviews (1): Last reviewed commit: "feat: add Groq adapter" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const model = opts.model ?? 'llama-3.3-70b-versatile'; | ||
| ctx.log(`groq · model=${model} · ${prompt.length} chars in`); | ||
| const model = opts.model ?? 'llama3-70b-8192'; | ||
| ctx.log(`${Groq_LOWER} · model=${model} · ${prompt.length} chars in`); |
There was a problem hiding this comment.
Groq_LOWER causes ReferenceError
Groq_LOWER is never declared anywhere in the file. Every call to generate() will throw a ReferenceError: Groq_LOWER is not defined at the log line before the API request is even made, making the adapter completely non-functional. The previous code used the string literal 'groq' here.
| }), | ||
| }); | ||
| if (!res.ok) throw new Error(`Groq ${res.status}: ${redact(await res.text(), apiKey).slice(0, 200)}`); | ||
| if (!res.ok) throw new Error(`${Groq} ${res.status}: ${(await res.text()).slice(0, 200)}`); |
There was a problem hiding this comment.
Groq causes ReferenceError in error path
Groq is never declared in the file, so any non-2xx HTTP response will throw ReferenceError: Groq is not defined instead of the intended error message. The original code used the string literal 'Groq' directly in the template, which is what the test at line 100 asserts (expect(message).toContain('Groq 429:')). This also means error handling is entirely broken.
| }), | ||
| }); | ||
| if (!res.ok) throw new Error(`Groq ${res.status}: ${redact(await res.text(), apiKey).slice(0, 200)}`); | ||
| if (!res.ok) throw new Error(`${Groq} ${res.status}: ${(await res.text()).slice(0, 200)}`); |
There was a problem hiding this comment.
The redact() helper was removed, so the raw API key can now appear in error bodies that get sliced and thrown. The test at lines 84–104 explicitly verifies redaction — it expects [redacted] to appear and the raw key to be absent. Without redaction, a key that falls within the first 200 characters of the error response body will be exposed in the thrown Error message, which may reach logs or monitoring systems.
| 'content-type': 'application/json', | ||
| }; | ||
|
|
||
| const res = await fetch(`${config.baseUrl ?? DEFAULT_BASE}/v1/chat/completions`, { |
There was a problem hiding this comment.
baseUrl produces a double-slash URL
The previous implementation called .replace(/\/+$/, '') on baseUrl before appending /v1/chat/completions. That guard was removed, so config.baseUrl = 'https://proxy.example.com/' now produces https://proxy.example.com//v1/chat/completions, which many HTTP servers will reject with a 404. The existing test at line 71 directly asserts this normalization and will fail.
| defaultModel: 'llama3-70b-8192', | ||
| models: ['llama3-70b-8192', 'llama3-8b-8192'], |
There was a problem hiding this comment.
The removed models — llama-3.3-70b-versatile, llama-3.1-8b-instant, mixtral-8x7b-32768, and deepseek-r1-distill-llama-70b — are all currently listed as active on the Groq console, whereas the replacement identifiers llama3-70b-8192 and llama3-8b-8192 correspond to older base Llama 3 checkpoints. If this is intentional, a comment explaining the rationale would be helpful; otherwise it looks like the newer, more capable model IDs were accidentally reverted.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: |
2 similar comments
|
🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: |
|
🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: |
|
tests are failing please fix. |
2 participants
Implements the Groq provider using the OpenAI-compatible protocol.