-
-
Notifications
You must be signed in to change notification settings - Fork 4
🚀 UPDATE: v0.2.7 #21
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?
🚀 UPDATE: v0.2.7 #21
Conversation
🐛 fix: logo/branding not appearing upon prompt
|
I think good addition to this PR is adding the logo to flags such as |
|
I created a new issue here, #22. Maybe one of you @spc-28 @Abhinav-Bansal751 can take it. |
|
@warengonzaga its a good idea. |
Yes, please! |
…removed unused code
…features/ollama-provider-integration
✨ tweak: add logo when calling flags
…ation 📦 new: add ollama as model provider
WalkthroughThe update introduces modular AI model support for commit message generation, refactoring the application to use a new logo component and a model selection UI. New modules handle OpenAI and Ollama integrations. Git utilities are refactored, configuration is restructured for multiple models, and the main app logic is simplified to delegate control to new components. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Logo
participant ModelSelection
participant GitUtils
participant AIModel (OpenAI/Ollama)
User->>CLI: Run magicc CLI
CLI->>Logo: Render logo and info
Logo->>ModelSelection: Show model selection
User->>ModelSelection: Select AI model
ModelSelection->>GitUtils: Check git repo & committer
GitUtils-->>ModelSelection: Return status
ModelSelection->>AIModel: Request commit message (with git diff)
AIModel-->>ModelSelection: Return category & message
ModelSelection->>User: Show commit message prompt
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 18
🧹 Nitpick comments (8)
source/cli.js (1)
8-8: Consider conditional Logo rendering based on CLI flags.Rendering the Logo component before parsing CLI flags means it will display even for
--helpor--versioncommands, which may not provide the best user experience. Consider moving this after CLI parsing or making it conditional.-render(<Logo />, {patchConsole: false}); const cli = meow( +// Move Logo rendering here conditionally +if (!cli.flags.help && !cli.flags.version) { + render(<Logo />, {patchConsole: false}); +}source/utils/modelSelection.js (2)
13-13: The isDisabled state is never utilized.The
isDisabledstate is defined but never set totrue, so the "Model selection in progress..." message will never display.Consider setting the state when a selection is made:
const handleSelect = async item => { + setIsDisabled(true); if (item.value) {
35-37: Improve error message specificity.The error message should distinguish between "not a git repository" and "git committer not set" for better user guidance.
- } else { - console.log('This is not a git repository.'); - } + } else { + if (!gitCheck) { + console.log('This is not a git repository.'); + } else { + console.log('Git committer name and email are not set. Please configure them with git config.'); + } + }source/utils/config.json (2)
2-5: Consider extracting long prompts to separate files.The emoji and message prompts are quite lengthy and make the JSON file harder to read. Consider moving them to separate prompt files for better maintainability.
For example:
"commitConfig": { - "emoji": "You are the author of the commit message...", - "message": "You are the author of the changes..." + "emojiPromptFile": "./prompts/emoji.txt", + "messagePromptFile": "./prompts/message.txt" },
12-19: Consider extracting common configuration.Both models use the same
maxDiffSizevalue. Consider extracting common defaults to reduce duplication.{ + "defaults": { + "maxDiffSize": 4000 + }, "commitConfig": { // ... }, "models": [ { "title": "GPT-4o Mini", "provider": "openai", "model": "gpt-4o-mini", - "maxDiffSize": 4000, "apiKey": "" }, { "title": "Llama 3.1 8B", "provider": "ollama", "model": "llama3.1:8b" - "maxDiffSize": 4000 } ] }source/models/ollama.js (1)
4-4: Unused parameterflagsThe
flagsparameter is not used in this function. Consider removing it if not needed.-async function ollamaModel(model, flags, diffContent) { +async function ollamaModel(model, diffContent) {Note: This would require updating the call sites in
generateCommitMessage.js.source/models/openai.js (2)
28-28: Remove unnecessary await
getOpenAIKey()is a synchronous function and doesn't need await.- const apiKey = await getOpenAIKey(); + const apiKey = getOpenAIKey();
38-38: Fix typo in comment- // use the prmopt from the config file message and send to openai + // use the prompt from the config file message and send to openai
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
package.json(2 hunks)source/app.js(1 hunks)source/cli.js(2 hunks)source/models/ollama.js(1 hunks)source/models/openai.js(1 hunks)source/utils/commit.js(4 hunks)source/utils/config.json(1 hunks)source/utils/generateCommitMessage.js(1 hunks)source/utils/logo.js(1 hunks)source/utils/modelSelection.js(1 hunks)source/utils/openai.js(0 hunks)
💤 Files with no reviewable changes (1)
- source/utils/openai.js
🧰 Additional context used
🧬 Code Graph Analysis (6)
source/utils/logo.js (2)
source/utils/info.js (1)
info(5-13)source/utils/modelSelection.js (1)
ModelSelection(11-58)
source/app.js (1)
source/utils/logo.js (1)
Logo(8-31)
source/utils/generateCommitMessage.js (3)
source/utils/api.js (1)
config(3-3)source/utils/commit.js (6)
initGit(63-69)status(77-77)gitStatus(75-100)gitDiff(103-110)gitDiff(105-105)firstFilePath(73-73)source/models/openai.js (2)
category(31-37)message(39-45)
source/models/openai.js (1)
source/utils/api.js (4)
setOpenAIKey(28-37)deleteOPenAIKey(43-46)getOpenAIKey(39-41)config(3-3)
source/utils/commit.js (3)
source/utils/modelSelection.js (2)
rl(15-18)handleSelect(21-42)source/utils/logo.js (1)
Logo(8-31)source/utils/generateCommitMessage.js (1)
status(12-12)
source/utils/modelSelection.js (3)
source/utils/commit.js (1)
rl(12-15)source/utils/api.js (1)
config(3-3)source/utils/errors.js (1)
isCommitterSet(3-15)
🪛 Biome (1.9.4)
source/utils/generateCommitMessage.js
[error] 49-49: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ 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: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
🔇 Additional comments (3)
package.json (1)
27-28: Good improvement to cleanup scripts.Adding fallback echo messages prevents the scripts from failing when the
distdirectory doesn't exist, making the build process more robust.source/cli.js (1)
38-38: Good consistency with patchConsole setting.Adding
{patchConsole: false}to both render calls maintains consistency and prevents console output conflicts.source/utils/commit.js (1)
9-32: Good refactoring of the UI structureThe integration with the Logo component and proper cleanup of the readline interface improves the code structure.
| "ink-select-input": "^6.0.0", | ||
| "is-git-repository": "^2.0.0", | ||
| "meow": "^11.0.0", | ||
| "ollama": "^0.5.9", |
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.
💡 Verification agent
🧩 Analysis chain
Verify Ollama dependency version and security.
The new Ollama dependency supports the AI model integration. Please ensure this version is stable and free from known vulnerabilities.
🌐 Web query:
What is the latest stable version of the ollama npm package and are there any known security vulnerabilities in version 0.5.9?
💡 Result:
The latest stable version of the ollama npm package is 0.5.16, released approximately one month ago. (security.snyk.io)
Regarding version 0.5.9, there are no known security vulnerabilities associated with it. Snyk's vulnerability database indicates that version 0.5.16, which is the latest release, has no direct vulnerabilities. (security.snyk.io)
It's important to note that while version 0.5.9 does not have reported vulnerabilities, it is an older release. For the most up-to-date features and security patches, it's recommended to use the latest stable version, 0.5.16.
Citations:
- 1: https://security.snyk.io/package/npm/ollama?utm_source=openai
- 2: https://security.snyk.io/package/npm/ollama?utm_source=openai
Upgrade Ollama to the latest stable version
Ollama v0.5.9 has no known security vulnerabilities, but the latest stable release is v0.5.16 (also free of reported issues). To benefit from new features and security patches, please update:
- File:
package.json, line 50 - Change:
- "ollama": "^0.5.9",
+ "ollama": "^0.5.16",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "ollama": "^0.5.9", | |
| "ollama": "^0.5.16", |
🤖 Prompt for AI Agents
In package.json at line 50, update the Ollama dependency version from "^0.5.9"
to "^0.5.16" to use the latest stable release. This involves changing the
version string to reflect the newer version while keeping the caret to allow
compatible updates.
| <Newline /> | ||
| ================================================== | ||
| </Text> | ||
| <ModelSelection {...children} /> |
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.
🛠️ Refactor suggestion
Fix props spreading to ModelSelection.
With the corrected function signature, ensure props are properly passed to ModelSelection.
- <ModelSelection {...children} />
+ <ModelSelection {...props} />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <ModelSelection {...children} /> | |
| <ModelSelection {...props} /> |
🤖 Prompt for AI Agents
In source/utils/logo.js at line 27, the props are incorrectly spread to the
ModelSelection component. Update the code to spread the correct props object
instead of children, ensuring that all intended properties are passed properly
to ModelSelection.
| Version: <Text color="green">{info('version')}</Text> | Author:{' '} | ||
| <Text color="blue">{info('author')}</Text> |
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.
🛠️ Refactor suggestion
Add error handling for info() function calls.
The info() function can return an error (as seen in the relevant code snippet), but there's no error handling here. Consider adding fallback values.
- Version: <Text color="green">{info('version')}</Text> | Author:{' '}
- <Text color="blue">{info('author')}</Text>
+ Version: <Text color="green">{info('version') || 'Unknown'}</Text> | Author:{' '}
+ <Text color="blue">{info('author') || 'Unknown'}</Text>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Version: <Text color="green">{info('version')}</Text> | Author:{' '} | |
| <Text color="blue">{info('author')}</Text> | |
| Version: <Text color="green">{info('version') || 'Unknown'}</Text> | Author:{' '} | |
| <Text color="blue">{info('author') || 'Unknown'}</Text> |
🤖 Prompt for AI Agents
In source/utils/logo.js at lines 18 to 19, the calls to info('version') and
info('author') lack error handling, which can cause issues if info() returns an
error. Modify the code to safely call info() with try-catch blocks or
conditional checks, and provide fallback default values (e.g., 'unknown') when
an error occurs or the result is invalid, ensuring the UI remains stable.
| import {Text, Newline} from 'ink'; | ||
| import ModelSelection from './modelSelection.js'; | ||
|
|
||
| export default function Logo(children) { |
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.
Fix function parameter destructuring.
The function parameter should destructure props, not take children as a direct parameter.
-export default function Logo(children) {
+export default function Logo(props) {Or if you specifically want to pass children:
-export default function Logo(children) {
+export default function Logo({children, ...props}) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export default function Logo(children) { | |
| export default function Logo({ children, ...props }) { |
🤖 Prompt for AI Agents
In source/utils/logo.js at line 8, the function parameter currently takes
`children` directly, but it should destructure the `children` property from the
props object instead. Change the function parameter to use destructuring syntax
like `function Logo({ children })` to correctly access the children prop.
| rl.close(); | ||
|
|
||
| // console.error('Error generating commit message:', error); | ||
| exit(); |
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.
🛠️ Refactor suggestion
Remove unnecessary readline.close() calls.
Since we're removing the readline interface, these close() calls should also be removed.
} catch (error) {
- rl.close();
-
// console.error('Error generating commit message:', error);
exit();
}And:
} else {
exit();
- rl.close();
}🤖 Prompt for AI Agents
In source/utils/modelSelection.js around lines 30 to 33, remove the unnecessary
rl.close() call because the readline interface is being removed. Simply delete
the rl.close() statement to avoid calling close on a non-existent or removed
readline interface.
| if (flags.setopenai) { | ||
| setOpenAIKey(flags.setopenai); | ||
| } | ||
| if (flags.delopenai) { | ||
| deleteOPenAIKey(); | ||
| } |
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.
🛠️ Refactor suggestion
API key management should be handled separately
Setting and deleting API keys within the model function violates single responsibility principle. This should be handled at a higher level (e.g., in the CLI or UI layer).
Consider moving API key management to the appropriate layer and keeping this function focused solely on generating commit messages.
🤖 Prompt for AI Agents
In source/models/openai.js around lines 10 to 15, the code for setting and
deleting API keys is mixed within the model function, violating the single
responsibility principle. Refactor by removing the API key management logic from
this function and move it to a higher-level layer such as the CLI or UI layer.
Keep this function focused only on generating commit messages without handling
API key operations.
| const category = await openai.chat.completions.create({ | ||
| messages: [ | ||
| {role: 'system', content: config.commitConfig.emoji}, | ||
| {role: 'user', content: diffContent}, | ||
| ], | ||
| model, | ||
| }); | ||
| // use the prmopt from the config file message and send to openai | ||
| const message = await openai.chat.completions.create({ | ||
| messages: [ | ||
| {role: 'system', content: config.commitConfig.message}, | ||
| {role: 'user', content: diffContent}, | ||
| ], | ||
| model, | ||
| }); | ||
|
|
||
| return { | ||
| category: category.choices[0].message.content, | ||
| message: message.choices[0].message.content, | ||
| }; |
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.
Add error handling for API calls
The OpenAI API calls lack error handling, which could cause unhandled exceptions.
+ try {
const category = await openai.chat.completions.create({
messages: [
{role: 'system', content: config.commitConfig.emoji},
{role: 'user', content: diffContent},
],
model,
});
// use the prompt from the config file message and send to openai
const message = await openai.chat.completions.create({
messages: [
{role: 'system', content: config.commitConfig.message},
{role: 'user', content: diffContent},
],
model,
});
return {
category: category.choices[0].message.content,
message: message.choices[0].message.content,
};
+ } catch (error) {
+ console.error('OpenAI API error:', error);
+ throw new Error(`Failed to generate commit message: ${error.message}`);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const category = await openai.chat.completions.create({ | |
| messages: [ | |
| {role: 'system', content: config.commitConfig.emoji}, | |
| {role: 'user', content: diffContent}, | |
| ], | |
| model, | |
| }); | |
| // use the prmopt from the config file message and send to openai | |
| const message = await openai.chat.completions.create({ | |
| messages: [ | |
| {role: 'system', content: config.commitConfig.message}, | |
| {role: 'user', content: diffContent}, | |
| ], | |
| model, | |
| }); | |
| return { | |
| category: category.choices[0].message.content, | |
| message: message.choices[0].message.content, | |
| }; | |
| try { | |
| const category = await openai.chat.completions.create({ | |
| messages: [ | |
| {role: 'system', content: config.commitConfig.emoji}, | |
| {role: 'user', content: diffContent}, | |
| ], | |
| model, | |
| }); | |
| // use the prmopt from the config file message and send to openai | |
| const message = await openai.chat.completions.create({ | |
| messages: [ | |
| {role: 'system', content: config.commitConfig.message}, | |
| {role: 'user', content: diffContent}, | |
| ], | |
| model, | |
| }); | |
| return { | |
| category: category.choices[0].message.content, | |
| message: message.choices[0].message.content, | |
| }; | |
| } catch (error) { | |
| console.error('OpenAI API error:', error); | |
| throw new Error(`Failed to generate commit message: ${error.message}`); | |
| } |
🤖 Prompt for AI Agents
In source/models/openai.js between lines 31 and 50, the OpenAI API calls do not
have error handling, risking unhandled exceptions. Wrap the calls to
openai.chat.completions.create in try-catch blocks or a single try-catch to
catch any errors thrown during the API requests. In the catch block, handle or
log the error appropriately and ensure the function returns a meaningful
fallback or propagates the error safely.
| await execa('git', ['restore', '--staged', '.']); | ||
| } catch (error) { | ||
| console.error(error); | ||
| } |
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.
🛠️ Refactor suggestion
Improve error handling in git functions
Errors are caught and logged but not re-thrown, making it difficult for callers to handle failures properly.
} catch (error) {
- console.error(error);
+ console.error('Git operation failed:', error);
+ throw error;
}Apply this pattern to all three functions: initGit, gitStatus, and gitDiff.
Also applies to: 97-99, 107-109
🤖 Prompt for AI Agents
In source/utils/commit.js at lines 65-68, 97-99, and 107-109, the catch blocks
log errors but do not re-throw them, preventing callers from handling failures
properly. Modify each catch block in the initGit, gitStatus, and gitDiff
functions to re-throw the caught error after logging it, ensuring that error
propagation is maintained for proper handling upstream.
|
|
||
| // git status to see if there are any changes | ||
| // if there's any changes add the first file in the list of changes | ||
| let firstFilePath = ''; |
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.
🛠️ Refactor suggestion
Avoid module-level mutable state
Using a module-level variable firstFilePath can lead to race conditions and makes the code harder to test and reason about.
Consider returning the first file path from gitStatus() and passing it through the functions that need it, or encapsulating it in a class/closure.
🤖 Prompt for AI Agents
In source/utils/commit.js at line 73, the module-level mutable variable
`firstFilePath` should be removed to avoid race conditions and improve
testability. Refactor the code so that `gitStatus()` returns the first file path
directly, and pass this value explicitly to any functions that require it, or
encapsulate the state within a class or closure to maintain proper scope and
avoid shared mutable state.
| const filePaths = lines | ||
| .map(line => line.split(' ').slice(2).join(' ').trim()) | ||
| .filter(filePath => filePath !== '') | ||
| .concat( | ||
| lines | ||
| .filter(line => line.startsWith('??')) | ||
| .map(line => line.split(' ').slice(1).join(' ').trim()), | ||
| ); |
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.
💡 Verification agent
🧩 Analysis chain
Complex file path parsing logic
The file path parsing logic is complex and might not handle all git status formats correctly (e.g., renamed files, files with spaces).
Consider using a more robust git parsing library or simplifying the logic. Also, verify that this handles all git status codes correctly:
🏁 Script executed:
#!/bin/bash
# Test various git status formats
echo "Testing git status parsing with different file states..."
# Create test files with various states
touch "test file with spaces.txt"
git add "test file with spaces.txt"
git status --porcelain | head -5
# Show git status format documentation
echo -e "\nGit status format reference:"
git status --porcelain | head -5
echo "First two characters indicate: XY where X=index, Y=worktree"Length of output: 558
Refine git status parsing to handle spaces and renames
The current logic in source/utils/commit.js (lines 81–88) splits each status line on spaces and then slices/join—this:
- Retains surrounding quotes for file names with spaces (e.g.
"test file with spaces.txt") - Ignores rename (
R), copy (C) or other multi-path entries - Only special-cases
??entries, but not staged vs. unstaged changes
To fix this, replace the ad-hoc splitting with one of the following:
• Use git status --porcelain -z and split on \0 so file names (including spaces/newlines) are handled correctly.
– Each entry is <XY> <path>\0 or <XY> <src>\0<dst>\0 for renames/copies.
– You can then parse XY to include or exclude renamed files and pull the correct path.
• Or leverage a well-tested library (e.g. simple-git, nodegit) to parse porcelain output instead of manual string operations.
Making this change will ensure all status codes (A, M, D, R, C, ??, etc.) and file names with arbitrary characters are parsed reliably.
🤖 Prompt for AI Agents
In source/utils/commit.js around lines 81 to 88, the current git status parsing
uses space splitting which fails to handle file names with spaces, renames,
copies, and other status codes properly. Replace this logic by using 'git status
--porcelain -z' to get null-separated entries, then parse each entry by reading
the XY status code and the associated paths accordingly, handling single-path
and rename/copy two-path entries. Alternatively, integrate a library like
simple-git or nodegit to parse the porcelain output reliably, ensuring all file
names and status codes are correctly processed.
This pull request refactors the
Appcomponent and introduces a newLogocomponent to simplify and centralize the display of the application logo and version information. The changes primarily focus on improving the code structure and readability.Refactoring and code structure improvements:
source/app.js: Removed imports forText,Newline,BigText, andGradient, and replaced the inline logo and version information display with the newLogocomponent. [1] [2]source/utils/commit.js: Added theLogocomponent to wrap the commit message prompt and removed unnecessary whitespace. [1] [2] [3]New component introduction:
source/utils/logo.js: Created a newLogocomponent to encapsulate the display of the application logo, version, author, and help information.Minor formatting adjustments:
source/app.js: Adjusted the formatting of console log messages for better readability.Summary by CodeRabbit
New Features
Improvements
Bug Fixes