-
Notifications
You must be signed in to change notification settings - Fork 5
Add OpenAI and Anthropic support #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Replace direct Groq SDK with @ai-sdk/groq and ai packages - Implement proper proxy support using HttpsProxyAgent - Improve TypeScript types removing unnecessary 'any' casts - Maintain backward compatibility with existing API
Add @ai-sdk/openai package to support OpenAI API provider alongside existing Groq support. This completes subtask 2.1 for adding multi-provider AI support. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add full support for OpenAI and Anthropic providers alongside existing Groq - Implement provider abstraction layer with unified configuration - Add comprehensive test coverage (21 tests passing) - Update CLI to support provider selection via config commands - All tasks 2 and 3 subtasks complete and validated
- Add macOS Keychain, Linux libsecret, and Windows Credential Manager support - Implement environment variable and file-based fallback backends - Create SecretsManager with automatic backend selection - Add CLI commands for secrets management (test, set, migrate, export) - Integrate secure storage with existing config system - Maintain full backward compatibility with ~/.lazycommit file - Add comprehensive test suite for all backends - Update README with secure storage documentation Also includes: - Task Master AI project management setup - Claude Code integration and custom commands - Task 4 implementation: Secure API key storage - Task 5 created: Git worktree hook support 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
- Detect worktrees using git rev-parse --git-dir and --git-common-dir - Dynamically determine correct hooks directory for worktrees vs regular repos - Install hooks in .git/worktrees/<name>/hooks/ for worktrees - Maintain backward compatibility with regular git repositories - Update hook detection to work with variable paths Fixes #5: Hook installation now works correctly in git worktrees 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
…r repos - Detect and use custom hooks paths from core.hooksPath config - Properly handle Husky and other hook managers - Show informative messages when installing alongside Husky - Fix hook execution in repositories with custom hook configurations
- Fix hook detection to work with any hook directory (including .husky) - Change detection from checking '/hooks/prepare-commit-msg' to just '/prepare-commit-msg' - This allows the hook to run properly in non-interactive mode when installed in custom locations - Detect and inform users when installing in custom hook directories
- Add commit context retrieval with GPG verification detection - Implement hierarchical selection: verified user commits > user commits > any commits - Integrate context into both hook and interactive modes - Provide recent commit examples to AI for style consistency - Format context appropriately for AI consumption
- Set AI_SDK_LOG_WARNINGS global to false in CLI entry point - Removes presencePenalty and frequencyPenalty warnings from output - Provides cleaner user experience when running commands
- Change from appendFile to writeFile to place message at start of file - Improve hook detection to check for message file arguments - Support both path-based and argument-based hook detection - Ensure non-interactive execution when called as git hook
- Check process.stdout.isTTY and process.stdin.isTTY to determine interactivity - Show UI elements only when running in an interactive terminal - Silent operation when running as a git hook in non-interactive context - Remove incorrect argument-based detection logic
@fkalny-groupon can you share a demo video where all the things are working pretty well? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for OpenAI and Anthropic AI providers to the lazycommit tool, expanding beyond the original Groq-only implementation. The changes enable users to choose between multiple AI providers for generating commit messages.
- Adds OpenAI and Anthropic provider support with proper API key validation
- Refactors AI functionality into a unified interface supporting all three providers
- Updates configuration system to handle provider-specific settings and default models
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
tests/specs/providers.ts | New comprehensive test suite for provider configuration and validation |
tests/index.ts | Adds provider tests to test suite |
src/utils/groq.ts | Refactored to use Vercel AI SDK with updated error handling |
src/utils/config.ts | Enhanced config system with provider support and validation |
src/utils/ai.ts | New unified AI interface supporting all three providers |
src/commands/prepare-commit-msg-hook.ts | Updated to use new AI interface and provider configuration |
src/commands/lazycommit.ts | Updated to use new AI interface |
package.json | Updated dependencies and description to reflect multi-provider support |
README.md | Updated documentation with provider setup instructions |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/utils/groq.ts
Outdated
model, | ||
const result = await generateText({ | ||
model: groq(model), | ||
messages: messages as any, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type assertion as any
bypasses TypeScript's type safety. Since the messages parameter is already properly typed as Array<{ role: 'system' | 'user' | 'assistant'; content: string }>
, this cast should be unnecessary and potentially indicates a type mismatch that should be resolved properly.
messages: messages as any, | |
messages, |
Copilot uses AI. Check for mistakes.
interface NodeRequestInit extends RequestInit { | ||
agent?: HttpsProxyAgent<string>; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The NodeRequestInit
interface is duplicated across multiple provider configurations (lines 46-48, 67-69, 89-91). This code duplication should be extracted to a shared type or helper function to improve maintainability.
Copilot uses AI. Check for mistakes.
@fkalny-groupon is attempting to deploy a commit to the Kartik Labhshetwar's projects Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 86 out of 87 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
||
async getAll(service: string): Promise<Map<string, string>> { | ||
const results = new Map<string, string>(); | ||
const accounts = ['GROQ_API_KEY', 'OPENAI_API_KEY', 'ANTHROPIC_API_KEY']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardcoded API key list is duplicated across multiple backend files (keychain.ts, libsecret.ts, windows.ts, file.ts, env.ts). Consider extracting this to a shared constant to maintain consistency and reduce duplication.
Copilot uses AI. Check for mistakes.
async getAll(_service: string): Promise<Map<string, string>> { | ||
const config = await this.readConfig(); | ||
const results = new Map<string, string>(); | ||
const accounts = ['GROQ_API_KEY', 'OPENAI_API_KEY', 'ANTHROPIC_API_KEY']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same hardcoded API key list as in other backend files. This duplication makes it error-prone to add new providers since all backend files would need updating.
Copilot uses AI. Check for mistakes.
|
||
async getAll(_service: string): Promise<Map<string, string>> { | ||
const results = new Map<string, string>(); | ||
const accounts = ['GROQ_API_KEY', 'OPENAI_API_KEY', 'ANTHROPIC_API_KEY']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another instance of the duplicated API key list. Consider creating a shared constants file like src/utils/secrets/constants.ts
with export const API_KEY_ACCOUNTS = ['GROQ_API_KEY', 'OPENAI_API_KEY', 'ANTHROPIC_API_KEY'];
Copilot uses AI. Check for mistakes.
} | ||
} | ||
|
||
config[key as ConfigKeys] = parsed?.toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of parsed?.toString()
could potentially convert non-string values to strings unexpectedly. Consider proper type checking or casting based on the expected config value type for the given key.
config[key as ConfigKeys] = parsed?.toString(); | |
config[key as ConfigKeys] = typeof parsed === 'string' ? parsed : parsed?.toString(); |
Copilot uses AI. Check for mistakes.
No description provided.