Skip to content

Commit 1dedb59

Browse files
committed
fix - review comments
1 parent f5f0f75 commit 1dedb59

File tree

4 files changed

+92
-30
lines changed

4 files changed

+92
-30
lines changed

README.md

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ Looking to contribute? Check out:
3535
🎛️ **Interactive Review Flow** - Accept, regenerate with new styles, or open the message in your editor before committing
3636
📊 **File Statistics Display** - Visual preview of changed files and line counts
3737
💡 **Smart Security Scrubbing** - Automatically removes API keys, passwords, and sensitive data from diffs
38+
💾 **Intelligent Caching** - Reduces API costs by caching generated messages for similar changes
3839
🚀 **Easy to Use** - Simple CLI interface with beautiful terminal UI
3940
⚡️ **Fast** - Quick generation of commit messages
4041

@@ -53,6 +54,36 @@ You can use **Google Gemini**, **Grok**, **Claude**, **ChatGPT**, or **Ollama**
5354

5455
All scrubbing happens locally before any data leaves your machine, ensuring your secrets stay secure.
5556

57+
## 💾 Intelligent Caching
58+
59+
`commit-msg` includes a smart caching system that reduces API costs and improves performance:
60+
61+
- **Automatic Cache Management** - Similar code changes generate the same commit message without API calls
62+
- **Cost Savings** - Avoid redundant API requests for identical or similar changes
63+
- **Performance Boost** - Instant retrieval of cached messages for repeated patterns
64+
- **Cache Statistics** - Track hit rates, total savings, and cache performance
65+
- **Secure Storage** - Cache files are stored with restricted permissions (600) for security
66+
- **Automatic Cleanup** - Old cache entries are automatically removed based on age and usage
67+
68+
### Cache Management Commands
69+
70+
```bash
71+
# View cache statistics
72+
commit cache stats
73+
74+
# Clear all cached messages
75+
commit cache clear
76+
77+
# Remove old cached messages
78+
commit cache cleanup
79+
```
80+
81+
The cache intelligently identifies similar changes by analyzing:
82+
- File modifications and additions
83+
- Code structure and patterns
84+
- Commit context and style preferences
85+
- LLM provider and generation options
86+
5687
---
5788

5889
## 📦 Installation
@@ -221,6 +252,8 @@ This makes it easy to tweak the tone, iterate on suggestions, or fine-tune the f
221252
- ✅ Create conventional commit messages
222253
- 📋 Auto-copy to clipboard for immediate use
223254
- 🎨 Beautiful terminal UI with file statistics and previews
255+
- 💾 Reduce API costs with intelligent caching for similar changes
256+
- ⚡️ Get instant results for repeated code patterns
224257

225258
---
226259

@@ -256,6 +289,19 @@ Select: Change API Key
256289
Select: Delete
257290
```
258291

292+
### Cache Management
293+
294+
```bash
295+
# View cache statistics and performance
296+
commit cache stats
297+
298+
# Clear all cached messages
299+
commit cache clear
300+
301+
# Remove old cached messages (based on age and usage)
302+
commit cache cleanup
303+
```
304+
259305
---
260306

261307
## Getting API Keys

cmd/cli/store/store.go

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,10 @@ func NewStoreMethods() (*StoreMethods, error) {
4040
}, nil
4141
}
4242

43-
// Initializes Keyring instance
43+
// KeyringInit initializes a StoreMethods instance with both keyring and cache support.
44+
// This function is kept for backward compatibility with main.go.
4445
func KeyringInit() (*StoreMethods, error) {
45-
ring, err := keyring.Open(keyring.Config{
46-
ServiceName: "commit-msg",
47-
})
48-
if err != nil {
49-
return nil, fmt.Errorf("failed to open keyring: %w", err)
50-
}
51-
return &StoreMethods{ring: ring}, nil
46+
return NewStoreMethods()
5247
}
5348

5449
// LLMProvider represents a single stored LLM provider and its credential.

internal/cache/cache.go

Lines changed: 43 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -59,27 +59,35 @@ func NewCacheManager() (*CacheManager, error) {
5959

6060
// Get retrieves a cached commit message if it exists.
6161
func (cm *CacheManager) Get(provider types.LLMProvider, diff string, opts *types.GenerationOptions) (*types.CacheEntry, bool) {
62-
cm.mutex.RLock()
63-
defer cm.mutex.RUnlock()
64-
6562
key := cm.hasher.GenerateCacheKey(provider, diff, opts)
66-
entry, exists := cm.entries[key]
6763

64+
// Phase 1: Read with RLock to check existence and copy the entry
65+
cm.mutex.RLock()
66+
entry, exists := cm.entries[key]
6867
if !exists {
68+
cm.mutex.RUnlock()
69+
// Update miss statistics with write lock
70+
cm.mutex.Lock()
6971
cm.stats.TotalMisses++
7072
cm.updateHitRate()
73+
cm.mutex.Unlock()
7174
return nil, false
7275
}
7376

74-
// Update access statistics
77+
// Create a copy of the entry to avoid external mutation
78+
entryCopy := *entry
79+
cm.mutex.RUnlock()
80+
81+
// Phase 2: Update shared stats and entry with write lock
82+
cm.mutex.Lock()
83+
// Update access statistics on the original entry
7584
entry.LastAccessedAt = time.Now().Format(time.RFC3339)
7685
entry.AccessCount++
7786
cm.stats.TotalHits++
78-
79-
// Update hit rate
8087
cm.updateHitRate()
88+
cm.mutex.Unlock()
8189

82-
return entry, true
90+
return &entryCopy, true
8391
}
8492

8593
// Set stores a commit message in the cache.
@@ -133,10 +141,10 @@ func (cm *CacheManager) Clear() error {
133141

134142
// GetStats returns cache statistics.
135143
func (cm *CacheManager) GetStats() *types.CacheStats {
136-
cm.mutex.RLock()
137-
defer cm.mutex.RUnlock()
144+
cm.mutex.Lock()
145+
defer cm.mutex.Unlock()
138146

139-
// Calculate additional stats
147+
// Calculate additional stats (mutates cm.stats)
140148
cm.calculateStats()
141149

142150
// Return a copy to avoid race conditions
@@ -149,7 +157,17 @@ func (cm *CacheManager) Cleanup() error {
149157
cm.mutex.Lock()
150158
defer cm.mutex.Unlock()
151159

152-
return cm.cleanupOldEntries()
160+
// Clean up old entries in memory
161+
if err := cm.cleanupOldEntries(); err != nil {
162+
return fmt.Errorf("failed to cleanup old entries: %w", err)
163+
}
164+
165+
// Persist the cleaned cache to disk
166+
if err := cm.saveCache(); err != nil {
167+
return fmt.Errorf("failed to persist cache after cleanup: %w", err)
168+
}
169+
170+
return nil
153171
}
154172

155173
// loadCache loads the cache from disk.
@@ -176,14 +194,21 @@ func (cm *CacheManager) loadCache() error {
176194
if cacheData.Stats != nil {
177195
cm.stats = cacheData.Stats
178196
}
197+
if cm.entries == nil {
198+
cm.entries = make(map[string]*types.CacheEntry)
199+
}
200+
if cm.stats == nil {
201+
cm.stats = &types.CacheStats{}
202+
}
203+
cm.stats.TotalEntries = len(cm.entries)
179204

180205
return nil
181206
}
182207

183208
// saveCache saves the cache to disk.
184209
func (cm *CacheManager) saveCache() error {
185210
// Ensure directory exists
186-
if err := os.MkdirAll(filepath.Dir(cm.filePath), 0755); err != nil {
211+
if err := os.MkdirAll(filepath.Dir(cm.filePath), 0700); err != nil {
187212
return fmt.Errorf("failed to create cache directory: %w", err)
188213
}
189214

@@ -202,7 +227,7 @@ func (cm *CacheManager) saveCache() error {
202227
return fmt.Errorf("failed to marshal cache data: %w", err)
203228
}
204229

205-
if err := os.WriteFile(cm.filePath, data, 0644); err != nil {
230+
if err := os.WriteFile(cm.filePath, data, 0600); err != nil {
206231
return fmt.Errorf("failed to write cache file: %w", err)
207232
}
208233

@@ -233,7 +258,7 @@ func (cm *CacheManager) cleanupOldEntries() error {
233258

234259
// If we still have too many entries, remove least recently accessed
235260
if len(cm.entries)-len(keysToDelete) > cm.config.MaxEntries {
236-
cm.removeLeastAccessed(keysToDelete)
261+
keysToDelete = cm.removeLeastAccessed(keysToDelete)
237262
}
238263

239264
// Delete the selected entries
@@ -247,7 +272,7 @@ func (cm *CacheManager) cleanupOldEntries() error {
247272
}
248273

249274
// removeLeastAccessed removes the least recently accessed entries.
250-
func (cm *CacheManager) removeLeastAccessed(existingKeysToDelete []string) {
275+
func (cm *CacheManager) removeLeastAccessed(existingKeysToDelete []string) []string {
251276
type entryWithKey struct {
252277
key string
253278
entry *types.CacheEntry
@@ -282,6 +307,8 @@ func (cm *CacheManager) removeLeastAccessed(existingKeysToDelete []string) {
282307
for i := 0; i < entriesToRemove && i < len(entries); i++ {
283308
existingKeysToDelete = append(existingKeysToDelete, entries[i].key)
284309
}
310+
311+
return existingKeysToDelete
285312
}
286313

287314
// updateHitRate calculates and updates the hit rate.

internal/cache/hasher.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,6 @@ func (h *DiffHasher) shouldSkipLine(line string) bool {
7373
return true
7474
}
7575

76-
// Skip lines with file paths (but keep the actual changes)
77-
if strings.Contains(line, "/") && (strings.HasPrefix(line, "+") || strings.HasPrefix(line, "-")) {
78-
// This is a file path line, skip it
79-
return true
80-
}
81-
8276
return false
8377
}
8478

0 commit comments

Comments
 (0)