feat: added support for custom templates #128
Conversation
WalkthroughAdds custom template support to the commit message generator. Users can set, retrieve, and delete custom commit message templates via a new CLI command. Templates are persisted in the store and automatically used by all LLM providers during prompt generation. Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI as Template CLI
participant Store
participant LLMProvider
participant Prompt as Prompt Builder
rect rgb(230, 245, 255)
Note over User,Store: Template Setup Phase
User->>CLI: template --set
CLI->>User: Prompt for template input
User->>CLI: Enter custom template
CLI->>Store: SaveTemplate(template)
Store->>Store: Persist to config file
CLI->>User: Confirm template saved
end
rect rgb(240, 255, 240)
Note over LLMProvider,Prompt: Message Generation Phase
User->>CLI: commit llm
CLI->>Store: GetTemplate()
Store->>CLI: Return custom template
CLI->>LLMProvider: GenerateCommitMessage()
LLMProvider->>Store: GetTemplate()
Store->>LLMProvider: Return custom template
LLMProvider->>Prompt: BuildCommitPromptWithTemplate(changes, opts, template)
Prompt->>Prompt: Apply custom template + changes
Prompt->>LLMProvider: Return templated prompt
LLMProvider->>LLMProvider: Generate message with custom template
LLMProvider->>User: Return generated commit message
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (10)
internal/groq/groq.go (2)
53-53: Critical:storeMethodsis never initialized.Same issue as in
internal/gemini/gemini.go: thestoreMethodsvariable is declared but never assigned, which will cause a nil pointer dereference panic whenGetTemplate()is called on line 66.See the comment in
internal/gemini/gemini.gofor suggested solutions. This needs to be fixed consistently across all provider files.
66-67: Major: Error from GetTemplate() is silently ignored.Same issue as in
internal/gemini/gemini.go: the error fromGetTemplate()is discarded. Unexpected errors (not "no custom template set") should be surfaced to the user.internal/chatgpt/chatgpt.go (2)
14-14: Critical:storeMethodsis never initialized.Same nil pointer dereference issue as other provider files. The variable is declared but never assigned before use on line 27.
26-28: Major: Error from GetTemplate() is silently ignored.Same error handling issue as other provider files.
internal/claude/claude.go (2)
42-42: Critical:storeMethodsis never initialized.Same nil pointer dereference issue as other provider files.
47-48: Major: Error from GetTemplate() is silently ignored.Same error handling issue as other provider files.
internal/grok/grok.go (2)
23-23: Critical:storeMethodsis never initialized.Same nil pointer dereference issue as other provider files.
29-30: Major: Error from GetTemplate() is silently ignored.Same error handling issue as other provider files.
internal/ollama/ollama.go (2)
33-33: Critical:storeMethodsis never initialized.Same nil pointer dereference issue as other provider files.
43-47: Major: Error from GetTemplate() is silently ignored.Same error handling issue as other provider files.
🧹 Nitpick comments (3)
internal/ollama/ollama.go (1)
43-43: Optional: Adjust comment style to match Go conventions.The comment starts with a capital letter but doesn't form a complete sentence. Go convention suggests either making it a complete sentence or starting with lowercase.
- // Checking for the custom template, when building the prompt + // Check for custom template when building the promptcmd/cli/createMsg.go (1)
392-395: Consider documenting the intentional fallback behavior.The error from
GetTemplate()is silently discarded, which appears to be intentional fallback behavior (using an empty string when no custom template exists). However, this could also hide legitimate configuration errors (e.g., corrupted config file).Consider either:
- Adding a comment explaining the intentional fallback, or
- Logging a debug message when template retrieval fails
- customTemplate, _ := store.GetTemplate() + // Silently fall back to empty template if none is configured + customTemplate, _ := store.GetTemplate()cmd/cli/store/store.go (1)
468-578: Consider extracting common config loading logic.The pattern of loading and unmarshaling the config file is duplicated across
SaveTemplate,GetTemplate, andDeleteTemplate(and other methods in this file). Extracting this into a helper method would improve maintainability and ensure consistent error handling.Consider adding a helper method like:
// loadConfig reads and unmarshals the config file func (s *StoreMethods) loadConfig() (*Config, string, error) { var cfg Config configPath, err := StoreUtils.GetConfigPath() if err != nil { return nil, "", err } isConfigExists := StoreUtils.CheckConfig(configPath) if !isConfigExists { return nil, configPath, fmt.Errorf("config file does not exist at %s", configPath) } data, err := os.ReadFile(configPath) if err != nil { return nil, configPath, err } if len(data) > 2 { if err := json.Unmarshal(data, &cfg); err != nil { return nil, configPath, fmt.Errorf("config file format error: %w", err) } } return &cfg, configPath, nil }Then use it in template methods:
func (s *StoreMethods) GetTemplate() (string, error) { cfg, _, err := s.loadConfig() if err != nil { return "", err } if cfg.CustomTemplate == "" { return "", errors.New("no custom template set") } return cfg.CustomTemplate, nil }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.gitignore(1 hunks)cmd/cli/createMsg.go(5 hunks)cmd/cli/store/store.go(3 hunks)cmd/cli/template.go(1 hunks)internal/chatgpt/chatgpt.go(2 hunks)internal/claude/claude.go(2 hunks)internal/gemini/gemini.go(2 hunks)internal/grok/grok.go(1 hunks)internal/groq/groq.go(3 hunks)internal/ollama/ollama.go(3 hunks)pkg/types/prompt.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (10)
internal/claude/claude.go (2)
cmd/cli/store/store.go (2)
StoreMethods(20-24)Config(65-70)pkg/types/prompt.go (1)
BuildCommitPromptWithTemplate(34-62)
internal/grok/grok.go (2)
cmd/cli/store/store.go (2)
StoreMethods(20-24)Config(65-70)pkg/types/prompt.go (1)
BuildCommitPromptWithTemplate(34-62)
internal/groq/groq.go (2)
cmd/cli/store/store.go (1)
StoreMethods(20-24)pkg/types/prompt.go (1)
BuildCommitPromptWithTemplate(34-62)
internal/ollama/ollama.go (2)
cmd/cli/store/store.go (1)
StoreMethods(20-24)pkg/types/prompt.go (1)
BuildCommitPromptWithTemplate(34-62)
cmd/cli/template.go (1)
cmd/cli/root.go (1)
Store(11-11)
internal/gemini/gemini.go (2)
cmd/cli/store/store.go (2)
StoreMethods(20-24)Config(65-70)pkg/types/prompt.go (1)
BuildCommitPromptWithTemplate(34-62)
cmd/cli/createMsg.go (5)
cmd/cli/root.go (1)
Store(11-11)pkg/types/types.go (4)
GenerationEvent(175-185)Message(80-83)LLMProvider(5-5)Config(60-63)internal/llm/provider.go (1)
Provider(24-29)pkg/types/prompt.go (1)
BuildCommitPromptWithTemplate(34-62)cmd/cli/store/store.go (3)
LLMProvider(59-62)StoreMethods(20-24)Config(65-70)
internal/chatgpt/chatgpt.go (2)
cmd/cli/store/store.go (1)
StoreMethods(20-24)pkg/types/prompt.go (1)
BuildCommitPromptWithTemplate(34-62)
cmd/cli/store/store.go (2)
pkg/types/types.go (2)
LLMProvider(5-5)Config(60-63)utils/storeUtils.go (3)
GetConfigPath(30-68)CheckConfig(9-17)CreateConfigFile(19-28)
pkg/types/prompt.go (1)
pkg/types/options.go (1)
GenerationOptions(4-10)
🔇 Additional comments (12)
.gitignore (1)
34-34: Minor: Change is orthogonal to PR objectives.The addition of
.ideais a best practice for excluding JetBrains IDE configuration directories. However, this change appears unrelated to the custom template support feature described in the PR objectives. Consider separating IDE configuration cleanup into a standalone commit or clarify if this is intentional.pkg/types/prompt.go (2)
30-32: LGTM: Clean delegation pattern.The refactoring to delegate to
BuildCommitPromptWithTemplatewith an empty template preserves backward compatibility while enabling the new template feature.
34-62: LGTM: Template integration is well-structured.The function correctly:
- Falls back to the default
CommitPromptwhen no custom template is provided- Appends "Here are the changes:" as a bridge between template and diff content
- Preserves regeneration context and style instruction handling
- Maintains consistent formatting with proper newlines
cmd/cli/template.go (4)
43-48: LGTM: Command initialization is correctly structured.The template command is properly wired to the LLM command hierarchy with appropriate flags.
50-102: LGTM: Template input handling is robust.The function correctly:
- Provides clear instructions to the user
- Reads multiline input using a scanner
- Validates the template is non-empty
- Handles scanner errors appropriately
- Provides helpful feedback after saving
129-171: LGTM: Deletion confirmation flow is well-implemented.The function properly:
- Checks if a template exists before prompting
- Requires explicit "yes" confirmation (good security practice)
- Handles input errors gracefully
- Provides clear feedback
However, note the string comparison issue on line 132 mentioned in the previous comment.
84-84: Verify that Store is properly initialized.The code uses the global
Storevariable without any nil checks. I've confirmed that:
Storeis exported but remains nil unlessStoreInit()is calledStoreInit()is defined incmd/cli/root.gobut is never called anywhere in the codebase- Multiple functions use
Storewithout validation:template.go(lines 84, 105, 130, 162) andcreateMsg.go(line 35)- This is a library package, so initialization must occur in external code
The risk is real: if
Storeis not initialized before these functions execute, a nil pointer dereference will occur. Ensure that:
- All callers invoke
StoreInit()with a properly initialized*store.StoreMethodsbefore calling any template or message generation functions- Consider adding defensive nil checks in the Store usage sites, or document the initialization requirement clearly
cmd/cli/createMsg.go (2)
149-149: LGTM: Store parameter added for template support.The addition of the
Storeparameter enables dry-run mode to fetch and display the custom template, aligning with the template-aware generation flow.
670-671: Consistent template integration in dry-run mode.The template fetching and prompt building logic correctly mirrors the actual generation flow. The silent error handling on
GetTemplate()maintains consistency withgenerateMessageWithCache(line 392), allowing graceful fallback to the default prompt when no custom template is configured.cmd/cli/store/store.go (3)
8-8: LGTM: Config struct properly extended for template support.The addition of
CustomTemplateandTemplateUpdatedAtfields withomitemptytags ensures backward compatibility with existing config files. Thetimeimport is appropriately added to support timestamp generation.Also applies to: 66-70
511-541: GetTemplate implementation is correct.The function appropriately returns an error when no custom template is set, allowing callers to fall back to default behavior. The error handling and config loading logic are consistent with other methods in the file.
580-583: LGTM: HasCustomTemplate provides clean boolean check.This convenience method appropriately handles errors silently, which is the expected behavior for a boolean "Has" check. The implementation is concise and follows Go conventions.
| } | ||
|
|
||
| cfg.CustomTemplate = template | ||
| cfg.TemplateUpdatedAt = fmt.Sprintf("%v", time.Now()) |
There was a problem hiding this comment.
Use standard timestamp format instead of default string representation.
Using fmt.Sprintf("%v", time.Now()) produces an inconsistent, non-standard timestamp format like "2025-11-09 21:05:16.123456 +0000 UTC" that's harder to parse programmatically. This could cause issues if the timestamp is later used for sorting, filtering, or display purposes.
Apply this diff to use RFC3339 format:
- cfg.TemplateUpdatedAt = fmt.Sprintf("%v", time.Now())
+ cfg.TemplateUpdatedAt = time.Now().UTC().Format(time.RFC3339)📝 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.
| cfg.TemplateUpdatedAt = fmt.Sprintf("%v", time.Now()) | |
| cfg.TemplateUpdatedAt = time.Now().UTC().Format(time.RFC3339) |
🤖 Prompt for AI Agents
In cmd/cli/store/store.go around line 501, the code sets cfg.TemplateUpdatedAt
using fmt.Sprintf("%v", time.Now()) which produces a non-standard, hard-to-parse
timestamp; replace this with a RFC3339 formatted timestamp by assigning
cfg.TemplateUpdatedAt = time.Now().Format(time.RFC3339) (or
time.Now().UTC().Format(time.RFC3339) if you want UTC) and ensure the time
package is imported if not already.
| func handleGetTemplate() { | ||
| template, err := Store.GetTemplate() | ||
| if err != nil { | ||
| if err.Error() == "no custom template set" { | ||
| fmt.Println("No custom template is currently set.") | ||
| fmt.Println() | ||
| fmt.Println("To set a custom template, run: commit llm template --set") | ||
| fmt.Println(" The default template will be used for commit message generation.") | ||
| return | ||
| } | ||
| fmt.Printf("Error retrieving template: %v\n", err) | ||
| return | ||
| } | ||
|
|
||
| fmt.Println("📋 Current Custom Template") | ||
| fmt.Println("========================================") | ||
| fmt.Println() | ||
| fmt.Println(template) | ||
| fmt.Println() | ||
| fmt.Println("========================================") | ||
| fmt.Println() | ||
| fmt.Println("💡 To update this template, run: commit llm template --set") | ||
| fmt.Println(" To delete this template, run: commit llm template --delete") | ||
| } |
There was a problem hiding this comment.
Major: Brittle error handling with string comparison.
Comparing error messages using string equality (line 107: err.Error() == "no custom template set") is fragile. If the store implementation changes the error message text, this comparison will fail and produce incorrect behavior.
Consider using sentinel errors or error types:
In cmd/cli/store/store.go, define:
var ErrNoTemplateSet = errors.New("no custom template set")Then in the store methods, return this sentinel:
func (s *StoreMethods) GetTemplate() (string, error) {
// ... load config
if config.CustomTemplate == "" {
return "", ErrNoTemplateSet
}
// ...
}In this file, use errors.Is:
if err != nil {
if errors.Is(err, store.ErrNoTemplateSet) {
fmt.Println("No custom template is currently set.")
// ...
return
}
fmt.Printf("Error retrieving template: %v\n", err)
return
}The same issue exists in handleDeleteTemplate on line 132.
🤖 Prompt for AI Agents
In cmd/cli/template.go around lines 104–127 (and similarly update
handleDeleteTemplate near line 132), replace the brittle string equality check
on err.Error() with an errors.Is check against a sentinel error exported from
the store package; add an import for the store package and the errors stdlib,
update the store implementation to define and return store.ErrNoTemplateSet when
no template exists, and then change the handlers to use if errors.Is(err,
store.ErrNoTemplateSet) { /* print the "No custom template..." messages and
return */ } else { /* print the generic error */ } so the logic relies on a
stable sentinel error instead of matching error text.
| geminiTemperature = 0.2 | ||
| ) | ||
|
|
||
| var storeMethods *store.StoreMethods |
There was a problem hiding this comment.
Critical: storeMethods is never initialized and will cause nil pointer dereference.
The package-level variable storeMethods is declared but never assigned a value. When storeMethods.GetTemplate() is called on line 25, this will cause a nil pointer dereference panic at runtime.
You need to initialize storeMethods before use. Based on the cmd/cli/root.go snippet showing Store *store.StoreMethods, the store instance should be passed to this package. Consider one of these approaches:
Option 1: Add an initialization function:
+// InitStore sets the store instance used for template retrieval.
+func InitStore(s *store.StoreMethods) {
+ storeMethods = s
+}Then call this from the CLI initialization code where the store is created.
Option 2: Pass store as a parameter:
-func GenerateCommitMessage(config *types.Config, changes string, apiKey string, opts *types.GenerationOptions) (string, error) {
+func GenerateCommitMessage(config *types.Config, changes string, apiKey string, opts *types.GenerationOptions, store *store.StoreMethods) (string, error) {This same issue exists in all provider files (chatgpt, claude, grok, groq, ollama).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In internal/gemini/gemini.go around line 19, the package-level variable
`storeMethods` is declared but never initialized which will cause a nil pointer
dereference when calling methods like GetTemplate; initialize it by adding an
exported InitStore or SetStore function (e.g., InitStore(s *store.StoreMethods)
or SetStore(s *store.StoreMethods)) in this package and call that from CLI
initialization where the Store is created (cmd/cli/root.go) to assign the
instance to the package variable, or alternatively change functions that use
storeMethods to accept a *store.StoreMethods parameter instead of using the
package var; apply the same fix to all provider files (chatgpt, claude, grok,
groq, ollama) that declare an uninitialized storeMethods.
| customTemplate, _ := storeMethods.GetTemplate() | ||
| prompt := types.BuildCommitPromptWithTemplate(changes, opts, customTemplate) |
There was a problem hiding this comment.
Major: Error from GetTemplate() is silently ignored.
The error returned by GetTemplate() is discarded with _. If the store fails to retrieve the template due to I/O errors, permission issues, or other problems, this will silently use an empty string as the template without informing the user.
Consider handling the error:
- customTemplate, _ := storeMethods.GetTemplate()
+ customTemplate, err := storeMethods.GetTemplate()
+ if err != nil && err.Error() != "no custom template set" {
+ return "", fmt.Errorf("failed to retrieve custom template: %w", err)
+ }Note: It's acceptable to ignore the "no custom template set" error since that's expected when no template is configured, and the function should fall back to the default prompt.
📝 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.
| customTemplate, _ := storeMethods.GetTemplate() | |
| prompt := types.BuildCommitPromptWithTemplate(changes, opts, customTemplate) | |
| customTemplate, err := storeMethods.GetTemplate() | |
| if err != nil && err.Error() != "no custom template set" { | |
| return "", fmt.Errorf("failed to retrieve custom template: %w", err) | |
| } | |
| prompt := types.BuildCommitPromptWithTemplate(changes, opts, customTemplate) |
🤖 Prompt for AI Agents
internal/gemini/gemini.go lines 25-26: GetTemplate() error is currently ignored
which can hide I/O/permission failures; capture the returned error, check if err
!= nil and if it is not the expected "no custom template set" sentinel (or use
an exported helper like IsNoTemplateErr), then return or propagate/log a wrapped
error so callers know retrieval failed; if the error is the expected "no
template" case, proceed to call BuildCommitPromptWithTemplate with an
empty/default template as before.
|
hey @suraj-mandal are you still working on this pr |
|
Yes.. will complete this weekend |
|
nice.. get the latest changes there are some security bumps |
2 participants
Description
Implements template management commands (--set, --get, --delete), allowing users to customize the prompt sent to LLM providers for generating commit messages.
Type of Change
Related Issue
Fixes #89
Changes Made
Added CustomTemplate field to Config struct for persistent template storage
Implemented three new methods on StoreMethods:
SaveTemplate(template string) - Saves custom template to config file
GetTemplate() - Retrieves stored custom template
DeleteTemplate() - Removes custom template from config
HasCustomTemplate() - Checks if custom template exists
Completed implementation of template command handlers:
handleSetTemplate() - Interactive multiline input for setting custom templates
handleGetTemplate() - Displays current template or notifies if none exists
handleDeleteTemplate() - Confirms and removes custom template
Added user-friendly prompts with emojis and clear instructions
Includes confirmation dialog for delete operation
Added CustomTemplate field to GenerationOptions struct
Updated BuildCommitPrompt() to check for and use custom template from options
Falls back to default prompt if no custom template is provided
Custom template is seamlessly integrated with existing style instructions and regeneration context
Template command already wired up via llmCmd.AddCommand(templateCmd) in init
Testing
Checklist
Screenshots (if applicable)
Additional Notes
For Hacktoberfest Participants
Thank you for your contribution! 🎉
Summary by CodeRabbit
New Features
--set,--get, and--deleteflags.Chores