diff --git a/CHANGELOG.md b/CHANGELOG.md index 10fdd92..3264241 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,20 @@ and the project adheres to [Semantic Versioning 2.0.0](https://semver.org/spec/v Go source — tracked or newly added — is missing its `// SPDX-License-Identifier: GPL-3.0-or-later` header, keeping the 100% coverage from regressing (ADR-0012). +- **`cache stats` subcommand.** Summarizes the local response cache: + entry count, total size, oldest/newest entry timestamps, the configured + size limit, and a per-provider/model breakdown. Read-only (ADR-0008). +- **`cache inspect ` subcommand.** Dumps a single cache entry's + metadata (provider, model, language, created-at, TTL + freshness, token + counts, on-disk size) by its cache key — the SHA-256 shown by + `--verbose` / `dry-run`, with or without the `.json` suffix. The cached + review body is shown only with `--show-content` (ADR-0008). +- **Size-bounded cache eviction.** New `cache.max_size_mb` config key: + when set (>0), each cache write that pushes the directory over the limit + evicts the oldest entries first until it fits; the just-written entry is + never evicted. Default `0` keeps the cache unlimited (`cache prune` + remains the manual stand-in). This is a fresh key with real Put-path + enforcement, not a revival of the v0.9.1-removed dead field (ADR-0008). ## [1.2.1] diff --git a/README.md b/README.md index 58dd316..af9e912 100644 --- a/README.md +++ b/README.md @@ -174,6 +174,8 @@ commitbrief list # command reference # Cache maintenance commitbrief cache clear # wipe every cached LLM response for this repo commitbrief cache prune [flags] # bounded cleanup; defaults --keep-last 500 --older-than 7d +commitbrief cache stats # entry count, size, age range, per-provider breakdown +commitbrief cache inspect # one entry's metadata (add --show-content for the body) ``` Global flags: `--json`, `--markdown`, `--output `, `--copy`, @@ -320,6 +322,7 @@ output: cache: enabled: true ttl_days: 7 + max_size_mb: 0 # 0 = unlimited; >0 evicts oldest entries past the cap ``` Review content lives in two files: @@ -400,7 +403,10 @@ removed. **When does the cache invalidate?** The cache key is a SHA-256 of `diff + system prompt + provider + model + lang + schema version`. Change any of those and you get a fresh -review. Default TTL is 7 days; configurable via `cache.ttl_days`. +review. Default TTL is 7 days; configurable via `cache.ttl_days`. Set +`cache.max_size_mb` (>0) to bound the on-disk cache: writes that push it +past the limit evict the oldest entries first (the entry just written is +never evicted). Inspect it with `cache stats` / `cache inspect `. **Can I run it in CI?** The primary target is the developer's terminal, but the CI-friendly diff --git a/internal/cache/cache.go b/internal/cache/cache.go index b0b1811..d7320f9 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -65,10 +65,11 @@ const ( ) type Cache struct { - dir string - ttl time.Duration - repoRoot string - now func() time.Time + dir string + ttl time.Duration + repoRoot string + maxSizeBytes int64 + now func() time.Time } type Options struct { @@ -82,6 +83,15 @@ type Options struct { // to DefaultTTL (7 days). TTL time.Duration + // MaxSizeBytes bounds the on-disk cache size. After each successful + // Put, if the cache directory exceeds this many bytes, the oldest + // entries (by CreatedAt, mtime fallback) are evicted oldest-first + // until the total fits — see eviction.go. Zero or negative disables + // eviction (unlimited; the manual `cache prune` stays the stand-in). + // The just-written entry is never evicted, so a single entry larger + // than the cap survives. + MaxSizeBytes int64 + // Now overrides time.Now (test injection); production callers leave it nil. Now func() time.Time } @@ -99,10 +109,11 @@ func Open(opts Options) (*Cache, error) { now = time.Now } return &Cache{ - dir: opts.Dir, - ttl: ttl, - repoRoot: opts.RepoRoot, - now: now, + dir: opts.Dir, + ttl: ttl, + repoRoot: opts.RepoRoot, + maxSizeBytes: opts.MaxSizeBytes, + now: now, }, nil } @@ -158,6 +169,16 @@ func (c *Cache) Put(key string, entry Entry) error { return fmt.Errorf("cache: entry written but %w", err) } } + + // Size-bounded eviction runs after the write so the just-stored entry + // counts toward the total and is protected from eviction. Failures + // here are best-effort cleanup — the entry is already persisted and a + // disk-pressure sweep that couldn't complete is not worth failing the + // surrounding review over, so we deliberately swallow the error. + if c.maxSizeBytes > 0 { + keep := filepath.Base(c.entryPath(key)) + _, _, _ = enforceSizeLimit(c.dir, c.maxSizeBytes, keep) + } return nil } diff --git a/internal/cache/eviction.go b/internal/cache/eviction.go new file mode 100644 index 0000000..b1310cf --- /dev/null +++ b/internal/cache/eviction.go @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package cache + +import ( + "encoding/json" + "io/fs" + "os" + "path/filepath" + "sort" + "time" +) + +// enforceSizeLimit caps the on-disk cache directory at maxBytes by +// removing entries oldest-first until the summed size fits. "Oldest" is +// each entry's CreatedAt, falling back to the file mtime when the entry +// is corrupt or pre-dates the timestamp field. It is the automatic +// counterpart to `cache prune`: cheap, no access-time tracking, and +// deliberately conservative. +// +// keep is the base filename (e.g. ".json") of an entry that must +// never be evicted — callers pass the just-written entry so a fresh Put +// can never delete its own result. Its bytes still count toward the +// total, so when a single entry alone exceeds maxBytes every other entry +// is evicted and that one survives over-budget (the alternative — wiping +// the result the user just paid for — is worse). +// +// maxBytes <= 0 is treated as "no limit" and returns immediately. A +// missing directory is not an error. Returns (entries removed, bytes +// freed, error); remove failures stop the sweep and return what was +// freed so far. +func enforceSizeLimit(dir string, maxBytes int64, keep string) (removed int, freed int64, err error) { + if maxBytes <= 0 { + return 0, 0, nil + } + + type candidate struct { + path string + size int64 + createdAt time.Time + protected bool + } + + var ( + candidates []candidate + total int64 + ) + + walkErr := filepath.WalkDir(dir, func(path string, d fs.DirEntry, werr error) error { + if werr != nil { + if os.IsNotExist(werr) && path == dir { + return filepath.SkipAll + } + return werr + } + if d.IsDir() { + return nil + } + // Only count finished entries. In-flight temp files (writeAtomic) + // and any non-entry files are ignored so a concurrent Put isn't + // double-counted or deleted mid-rename. + if filepath.Ext(path) != ".json" { + return nil + } + info, ierr := d.Info() + if ierr != nil { + return ierr + } + ts := info.ModTime() + if raw, rerr := os.ReadFile(path); rerr == nil { + var e Entry + if json.Unmarshal(raw, &e) == nil && !e.CreatedAt.IsZero() { + ts = e.CreatedAt + } + } + total += info.Size() + candidates = append(candidates, candidate{ + path: path, + size: info.Size(), + createdAt: ts, + protected: filepath.Base(path) == keep, + }) + return nil + }) + if walkErr != nil { + return 0, 0, walkErr + } + + if total <= maxBytes { + return 0, 0, nil + } + + // Oldest first; protected entry sinks to the end so it's only removed + // if it were the last candidate — which the loop below never does. + sort.Slice(candidates, func(i, j int) bool { + if candidates[i].protected != candidates[j].protected { + return !candidates[i].protected + } + return candidates[i].createdAt.Before(candidates[j].createdAt) + }) + + for _, cand := range candidates { + if total <= maxBytes { + break + } + if cand.protected { + // Reached the protected entry with the total still over + // budget: nothing more we may delete. Stop. + break + } + if rerr := os.Remove(cand.path); rerr != nil { + return removed, freed, rerr + } + removed++ + freed += cand.size + total -= cand.size + } + + return removed, freed, nil +} diff --git a/internal/cache/eviction_test.go b/internal/cache/eviction_test.go new file mode 100644 index 0000000..06c2609 --- /dev/null +++ b/internal/cache/eviction_test.go @@ -0,0 +1,214 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package cache + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// writeEntryFile drops a syntactically valid entry at dir/.json +// with the given CreatedAt and a padded Content so its on-disk size is +// roughly controllable. Returns the file's actual size. +func writeEntryFile(t *testing.T, dir, name string, createdAt time.Time, pad int) int64 { + t.Helper() + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + e := Entry{ + Version: SchemaVersion, + CreatedAt: createdAt, + TTL: 3600, + Result: Result{Content: strings.Repeat("x", pad), Format: FormatJSON}, + } + data, err := json.MarshalIndent(e, "", " ") + if err != nil { + t.Fatal(err) + } + path := filepath.Join(dir, name+".json") + if err := os.WriteFile(path, data, 0o600); err != nil { + t.Fatal(err) + } + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + return info.Size() +} + +func exists(t *testing.T, path string) bool { + t.Helper() + _, err := os.Stat(path) + if err == nil { + return true + } + if os.IsNotExist(err) { + return false + } + t.Fatalf("stat %s: %v", path, err) + return false +} + +func TestEnforceSizeLimitUnlimited(t *testing.T) { + dir := t.TempDir() + writeEntryFile(t, dir, "a", time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC), 1000) + for _, max := range []int64{0, -1} { + removed, freed, err := enforceSizeLimit(dir, max, "") + if err != nil { + t.Fatalf("max=%d: %v", max, err) + } + if removed != 0 || freed != 0 { + t.Errorf("max=%d: removed=%d freed=%d, want 0/0 (unlimited)", max, removed, freed) + } + } +} + +func TestEnforceSizeLimitNoOpUnderCap(t *testing.T) { + dir := t.TempDir() + size := writeEntryFile(t, dir, "a", time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC), 100) + removed, freed, err := enforceSizeLimit(dir, size+1000, "") + if err != nil { + t.Fatal(err) + } + if removed != 0 || freed != 0 { + t.Errorf("under cap: removed=%d freed=%d, want 0/0", removed, freed) + } +} + +func TestEnforceSizeLimitMissingDir(t *testing.T) { + removed, freed, err := enforceSizeLimit(filepath.Join(t.TempDir(), "absent"), 10, "") + if err != nil { + t.Fatalf("missing dir should not error: %v", err) + } + if removed != 0 || freed != 0 { + t.Errorf("missing dir: removed=%d freed=%d, want 0/0", removed, freed) + } +} + +func TestEnforceSizeLimitEvictsOldestFirst(t *testing.T) { + dir := t.TempDir() + base := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + // Three equally-sized entries, oldest → newest. + s := writeEntryFile(t, dir, "old", base, 500) + writeEntryFile(t, dir, "mid", base.Add(time.Hour), 500) + writeEntryFile(t, dir, "new", base.Add(2*time.Hour), 500) + + // Cap that fits ~2 entries forces exactly one eviction (the oldest). + removed, freed, err := enforceSizeLimit(dir, 2*s+s/2, "") + if err != nil { + t.Fatal(err) + } + if removed != 1 { + t.Errorf("removed = %d, want 1", removed) + } + if freed != s { + t.Errorf("freed = %d, want %d", freed, s) + } + if exists(t, filepath.Join(dir, "old.json")) { + t.Error("oldest entry should have been evicted") + } + if !exists(t, filepath.Join(dir, "mid.json")) || !exists(t, filepath.Join(dir, "new.json")) { + t.Error("newer entries should survive") + } +} + +func TestEnforceSizeLimitProtectsKeep(t *testing.T) { + dir := t.TempDir() + base := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + // "old" is both the oldest AND the protected entry — it must survive + // even though oldest-first would normally evict it first. + writeEntryFile(t, dir, "old", base, 500) + writeEntryFile(t, dir, "new", base.Add(time.Hour), 500) + + // Cap forces one eviction; protected "old" is spared, so "new" goes. + removed, _, err := enforceSizeLimit(dir, 600, "old.json") + if err != nil { + t.Fatal(err) + } + if removed != 1 { + t.Fatalf("removed = %d, want 1", removed) + } + if !exists(t, filepath.Join(dir, "old.json")) { + t.Error("protected entry must never be evicted") + } + if exists(t, filepath.Join(dir, "new.json")) { + t.Error("unprotected entry should have been evicted") + } +} + +func TestEnforceSizeLimitSingleEntryOverCapSurvivesWhenProtected(t *testing.T) { + dir := t.TempDir() + writeEntryFile(t, dir, "big", time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC), 5000) + removed, freed, err := enforceSizeLimit(dir, 100, "big.json") + if err != nil { + t.Fatal(err) + } + if removed != 0 || freed != 0 { + t.Errorf("removed=%d freed=%d, want 0/0 (lone protected entry stays over-budget)", removed, freed) + } + if !exists(t, filepath.Join(dir, "big.json")) { + t.Error("protected entry should survive even when alone over cap") + } +} + +func TestEnforceSizeLimitIgnoresNonEntryFiles(t *testing.T) { + dir := t.TempDir() + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + // A stray temp file from an in-flight write must not be counted or + // removed by the size sweep. + tmp := filepath.Join(dir, "inflight.json.tmp") + if err := os.WriteFile(tmp, make([]byte, 4000), 0o600); err != nil { + t.Fatal(err) + } + writeEntryFile(t, dir, "a", time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC), 100) + removed, _, err := enforceSizeLimit(dir, 50, "") + if err != nil { + t.Fatal(err) + } + // Only the .json entry is a candidate; the .tmp is invisible to the sweep. + if removed != 1 { + t.Errorf("removed = %d, want 1 (.tmp ignored)", removed) + } + if !exists(t, tmp) { + t.Error(".tmp file should be left untouched") + } +} + +func TestPutEvictsWhenOverMaxSize(t *testing.T) { + frozen := time.Date(2026, 5, 26, 0, 0, 0, 0, time.UTC) + now := frozen + dir := filepath.Join(t.TempDir(), "cache") + c, err := Open(Options{ + Dir: dir, + MaxSizeBytes: 1200, // fits ~1 padded entry below + Now: func() time.Time { return now }, + }) + if err != nil { + t.Fatal(err) + } + + big := Entry{Result: Result{Content: strings.Repeat("y", 800)}} + + // First Put: under cap, survives. + if err := c.Put("first", big); err != nil { + t.Fatal(err) + } + // Advance the clock so the second entry is unambiguously newer. + now = frozen.Add(time.Hour) + if err := c.Put("second", big); err != nil { + t.Fatal(err) + } + + if _, ok := c.Get("first"); ok { + t.Error("oldest entry should have been evicted on the over-cap Put") + } + if _, ok := c.Get("second"); !ok { + t.Error("just-written entry must survive eviction") + } +} diff --git a/internal/cli/cache.go b/internal/cli/cache.go index ee735d5..f0d40e3 100644 --- a/internal/cli/cache.go +++ b/internal/cli/cache.go @@ -13,11 +13,12 @@ import ( "github.com/CommitBrief/commitbrief/internal/ui" ) -// newCacheCmd is the `commitbrief cache` subtree. Currently exposes a -// single `clear` child that deletes the repo-local response cache at -// /.commitbrief/cache/. The parent exists as a namespace so -// future inspection helpers (e.g. `cache stats`, `cache inspect`) can -// slot in without re-flattening the CLI surface. +// newCacheCmd is the `commitbrief cache` subtree over the repo-local +// response cache at /.commitbrief/cache/: +// - clear — delete every cached entry +// - prune — drop old/excess entries by keep-last + age windows +// - stats — count, size, age range, per-provider/model breakdown +// - inspect — dump one entry's metadata by key func newCacheCmd() *cobra.Command { cmd := &cobra.Command{ Use: "cache", @@ -26,6 +27,8 @@ func newCacheCmd() *cobra.Command { } cmd.AddCommand(newCacheClearCmd()) cmd.AddCommand(newCachePruneCmd()) + cmd.AddCommand(newCacheStatsCmd()) + cmd.AddCommand(newCacheInspectCmd()) return cmd } diff --git a/internal/cli/cache_inspect.go b/internal/cli/cache_inspect.go new file mode 100644 index 0000000..218f58e --- /dev/null +++ b/internal/cli/cache_inspect.go @@ -0,0 +1,105 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package cli + +import ( + "encoding/json" + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" + "strings" + "time" + + "github.com/spf13/cobra" + + "github.com/CommitBrief/commitbrief/internal/cache" +) + +func newCacheInspectCmd() *cobra.Command { + var showContent bool + cmd := &cobra.Command{ + Use: "inspect ", + Short: "Show metadata for a single cache entry by key", + Long: "Dumps one cached entry's metadata (provider, model, language, timestamps, " + + "freshness, token counts, on-disk size) given its cache key. The key is the " + + "SHA-256 shown by `--verbose` / `dry-run` (the .json suffix is optional). The " + + "cached review body is omitted unless --show-content is passed.", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + app, err := resolveContext(true) + if err != nil { + return err + } + w := cmd.OutOrStdout() + + // Accept the key with or without the on-disk .json suffix. + key := strings.TrimSuffix(args[0], ".json") + cacheDir := filepath.Join(app.RepoRoot, ".commitbrief", "cache") + path := filepath.Join(cacheDir, key+".json") + + raw, err := os.ReadFile(path) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + _, _ = fmt.Fprintln(w, app.Catalog.T("cache.inspect.notfound", key, cacheDir)) + return nil + } + return fmt.Errorf("cache inspect: read %s: %w", path, err) + } + + var e cache.Entry + if err := json.Unmarshal(raw, &e); err != nil { + return fmt.Errorf("cache inspect: entry %s is corrupt: %w", key, err) + } + + // Metadata dump is debug-grade tabular output and stays English, + // consistent with the dry-run / compress tables and `--verbose`. + now := time.Now() + fresh := "fresh" + if e.ExpiredAt(now) { + fresh = "expired" + } + format := e.Result.Format + if format == "" { + format = cache.FormatJSON // empty == json, per ADR-0008 §4 + } + + _, _ = fmt.Fprintf(w, "Key: %s\n", key) + _, _ = fmt.Fprintf(w, "Provider: %s\n", orDash(e.Key.Provider)) + _, _ = fmt.Fprintf(w, "Model: %s\n", orDash(e.Key.Model)) + _, _ = fmt.Fprintf(w, "Lang: %s\n", orDash(e.Key.Lang)) + _, _ = fmt.Fprintf(w, "Created: %s\n", e.CreatedAt.UTC().Format(time.RFC3339)) + if e.TTL > 0 { + expiry := e.CreatedAt.Add(time.Duration(e.TTL) * time.Second).UTC() + _, _ = fmt.Fprintf(w, "TTL: %ds (expires %s, %s)\n", + e.TTL, expiry.Format(time.RFC3339), fresh) + } else { + _, _ = fmt.Fprintf(w, "TTL: 0 (never expires)\n") + } + _, _ = fmt.Fprintf(w, "Format: %s\n", format) + _, _ = fmt.Fprintf(w, "Size: %s\n", formatBytes(int64(len(raw)))) + _, _ = fmt.Fprintf(w, "Tokens: input=%d output=%d cached=%d\n", + e.Result.Tokens.Input, e.Result.Tokens.Output, e.Result.Tokens.Cached) + _, _ = fmt.Fprintf(w, "Diff hash: %s\n", orDash(e.Key.DiffHash)) + + if showContent { + _, _ = fmt.Fprintln(w, "\n--- content ---") + _, _ = fmt.Fprintln(w, e.Result.Content) + } + return nil + }, + } + cmd.Flags().BoolVar(&showContent, "show-content", false, + "also print the cached review body (omitted by default)") + return cmd +} + +// orDash renders an empty metadata field as a dash so the column lines +// up and a missing value is visually obvious rather than blank. +func orDash(s string) string { + if s == "" { + return "-" + } + return s +} diff --git a/internal/cli/cache_stats.go b/internal/cli/cache_stats.go new file mode 100644 index 0000000..53c85f8 --- /dev/null +++ b/internal/cli/cache_stats.go @@ -0,0 +1,176 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package cli + +import ( + "encoding/json" + "fmt" + "io/fs" + "os" + "path/filepath" + "sort" + "time" + + "github.com/spf13/cobra" + + "github.com/CommitBrief/commitbrief/internal/cache" +) + +func newCacheStatsCmd() *cobra.Command { + return &cobra.Command{ + Use: "stats", + Short: "Show cache entry count, size, age range, and per-provider breakdown", + Long: "Summarizes the repo-local response cache at /.commitbrief/cache/: " + + "total entries and bytes, the oldest/newest entry timestamps, the configured " + + "size limit (cache.max_size_mb), and a per-provider/model breakdown. Read-only — " + + "use `cache prune` / `cache clear` to reclaim space.", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + app, err := resolveContext(true) + if err != nil { + return err + } + w := cmd.OutOrStdout() + + cacheDir := filepath.Join(app.RepoRoot, ".commitbrief", "cache") + st, err := collectCacheStats(cacheDir) + if err != nil { + return fmt.Errorf("cache stats: scan %s: %w", cacheDir, err) + } + + if st.count == 0 { + _, _ = fmt.Fprintln(w, app.Catalog.T("cache.stats.empty", cacheDir)) + return nil + } + + _, _ = fmt.Fprintln(w, app.Catalog.T( + "cache.stats.summary", st.count, formatBytes(st.bytes), cacheDir)) + _, _ = fmt.Fprintln(w, app.Catalog.T( + "cache.stats.range", + st.oldest.UTC().Format(time.RFC3339), + st.newest.UTC().Format(time.RFC3339))) + + if mb := app.Config.Cache.MaxSizeMB; mb > 0 { + _, _ = fmt.Fprintln(w, app.Catalog.T( + "cache.stats.limit.bounded", formatBytes(int64(mb)*1024*1024), mb)) + } else { + _, _ = fmt.Fprintln(w, app.Catalog.T("cache.stats.limit.unlimited")) + } + + // Per-provider/model breakdown is debug-grade tabular output and + // stays English, consistent with the dry-run / compress tables. + rows := st.breakdownRows() + if len(rows) > 0 { + _, _ = fmt.Fprintln(w) + _, _ = fmt.Fprintln(w, "By provider/model:") + for _, r := range rows { + _, _ = fmt.Fprintf(w, " %-12s %-28s %4d %s\n", + r.provider, r.model, r.count, formatBytes(r.bytes)) + } + } + return nil + }, + } +} + +type cacheStatsResult struct { + count int + bytes int64 + oldest time.Time + newest time.Time + // keyed by "provider\x00model" so the two never collide for models + // that share a name across providers. + byKey map[string]*breakdownRow +} + +type breakdownRow struct { + provider string + model string + count int + bytes int64 +} + +// collectCacheStats walks the cache directory once, aggregating counts, +// byte totals, the created-at range, and a per-provider/model breakdown. +// Corrupt entries are still counted (under provider/model "?") and their +// mtime feeds the age range so the totals match what's on disk. A +// missing directory yields a zero-value cacheStats with no error. +func collectCacheStats(dir string) (cacheStatsResult, error) { + st := cacheStatsResult{byKey: map[string]*breakdownRow{}} + + walkErr := filepath.WalkDir(dir, func(path string, d fs.DirEntry, werr error) error { + if werr != nil { + if os.IsNotExist(werr) && path == dir { + return filepath.SkipAll + } + return werr + } + if d.IsDir() || filepath.Ext(path) != ".json" { + return nil + } + info, ierr := d.Info() + if ierr != nil { + return ierr + } + + provider, model := "?", "?" + ts := info.ModTime() + if raw, rerr := os.ReadFile(path); rerr == nil { + var e cache.Entry + if json.Unmarshal(raw, &e) == nil { + if e.Key.Provider != "" { + provider = e.Key.Provider + } + if e.Key.Model != "" { + model = e.Key.Model + } + if !e.CreatedAt.IsZero() { + ts = e.CreatedAt + } + } + } + + st.count++ + st.bytes += info.Size() + if st.oldest.IsZero() || ts.Before(st.oldest) { + st.oldest = ts + } + if ts.After(st.newest) { + st.newest = ts + } + + mapKey := provider + "\x00" + model + row := st.byKey[mapKey] + if row == nil { + row = &breakdownRow{provider: provider, model: model} + st.byKey[mapKey] = row + } + row.count++ + row.bytes += info.Size() + return nil + }) + if walkErr != nil { + return cacheStatsResult{}, walkErr + } + return st, nil +} + +// breakdownRows returns the per-provider/model rows sorted by entry count +// (descending), then provider, then model — a stable, deterministic order +// for the table and for tests. +func (s cacheStatsResult) breakdownRows() []breakdownRow { + rows := make([]breakdownRow, 0, len(s.byKey)) + for _, r := range s.byKey { + rows = append(rows, *r) + } + sort.Slice(rows, func(i, j int) bool { + if rows[i].count != rows[j].count { + return rows[i].count > rows[j].count + } + if rows[i].provider != rows[j].provider { + return rows[i].provider < rows[j].provider + } + return rows[i].model < rows[j].model + }) + return rows +} diff --git a/internal/cli/cache_stats_inspect_test.go b/internal/cli/cache_stats_inspect_test.go new file mode 100644 index 0000000..ba54275 --- /dev/null +++ b/internal/cli/cache_stats_inspect_test.go @@ -0,0 +1,154 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package cli + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/CommitBrief/commitbrief/internal/cache" +) + +// seedCacheEntry writes a valid cache entry at /.commitbrief/cache/.json. +func seedCacheEntry(t *testing.T, repoRoot, key, provider, model string, createdAt time.Time, content string) { + t.Helper() + dir := filepath.Join(repoRoot, ".commitbrief", "cache") + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + e := cache.Entry{ + Version: cache.SchemaVersion, + CreatedAt: createdAt, + TTL: int64((7 * 24 * time.Hour).Seconds()), + Key: cache.KeyMeta{Provider: provider, Model: model, Lang: "en", DiffHash: "sha256:deadbeef"}, + Result: cache.Result{ + Content: content, + Tokens: cache.Tokens{Input: 1000, Output: 500, Cached: 0}, + Format: cache.FormatJSON, + }, + } + data, err := json.MarshalIndent(e, "", " ") + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, key+".json"), data, 0o600); err != nil { + t.Fatal(err) + } +} + +func TestCacheStatsEmpty(t *testing.T) { + e := newCLIEnv(t) + if err := e.run("cache", "stats"); err != nil { + t.Fatalf("cache stats on empty repo: %v", err) + } + out := e.out.String() + if !strings.Contains(out, "No cached entries") { + t.Errorf("empty stats should surface empty-state message; got:\n%s", truncate(out, 400)) + } +} + +func TestCacheStatsReportsCountsAndBreakdown(t *testing.T) { + e := newCLIEnv(t) + base := time.Date(2026, 5, 20, 0, 0, 0, 0, time.UTC) + seedCacheEntry(t, e.repoRoot, "k1", "anthropic", "claude-opus-4-7", base, "aaa") + seedCacheEntry(t, e.repoRoot, "k2", "anthropic", "claude-opus-4-7", base.Add(time.Hour), "bbb") + seedCacheEntry(t, e.repoRoot, "k3", "openai", "gpt-4o", base.Add(2*time.Hour), "ccc") + + if err := e.run("cache", "stats"); err != nil { + t.Fatalf("cache stats: %v", err) + } + out := e.out.String() + for _, want := range []string{ + "Cache: 3", "By provider/model:", "anthropic", "openai", + "2026-05-20T00:00:00Z", // oldest + "Size limit: unlimited", + } { + if !strings.Contains(out, want) { + t.Errorf("stats output missing %q; got:\n%s", want, truncate(out, 800)) + } + } +} + +func TestCacheStatsShowsBoundedLimit(t *testing.T) { + e := newCLIEnv(t) + if err := e.run("config", "set", "cache.max_size_mb", "50"); err != nil { + t.Fatalf("config set max_size_mb: %v", err) + } + seedCacheEntry(t, e.repoRoot, "k1", "anthropic", "claude-opus-4-7", + time.Date(2026, 5, 20, 0, 0, 0, 0, time.UTC), "aaa") + + e.out.Reset() + if err := e.run("cache", "stats"); err != nil { + t.Fatalf("cache stats: %v", err) + } + out := e.out.String() + if !strings.Contains(out, "cache.max_size_mb=50") { + t.Errorf("stats should report the configured size limit; got:\n%s", truncate(out, 800)) + } +} + +func TestCacheInspectNotFound(t *testing.T) { + e := newCLIEnv(t) + if err := e.run("cache", "inspect", "nonexistentkey"); err != nil { + t.Fatalf("cache inspect missing key: %v", err) + } + out := e.out.String() + if !strings.Contains(out, "No cache entry with key") { + t.Errorf("inspect of missing key should surface not-found; got:\n%s", truncate(out, 400)) + } +} + +func TestCacheInspectShowsMetadata(t *testing.T) { + e := newCLIEnv(t) + seedCacheEntry(t, e.repoRoot, "abc123", "anthropic", "claude-opus-4-7", + time.Date(2026, 5, 20, 0, 0, 0, 0, time.UTC), "review body here") + + if err := e.run("cache", "inspect", "abc123"); err != nil { + t.Fatalf("cache inspect: %v", err) + } + out := e.out.String() + for _, want := range []string{ + "Provider:", "anthropic", "Model:", "claude-opus-4-7", + "Created:", "2026-05-20T00:00:00Z", "Format:", "json", + "input=1000", "Diff hash:", "sha256:deadbeef", + } { + if !strings.Contains(out, want) { + t.Errorf("inspect output missing %q; got:\n%s", want, truncate(out, 800)) + } + } + // Content is omitted unless --show-content. + if strings.Contains(out, "review body here") { + t.Errorf("inspect must not print content without --show-content; got:\n%s", truncate(out, 800)) + } +} + +func TestCacheInspectShowContent(t *testing.T) { + e := newCLIEnv(t) + seedCacheEntry(t, e.repoRoot, "abc123", "anthropic", "claude-opus-4-7", + time.Date(2026, 5, 20, 0, 0, 0, 0, time.UTC), "review body here") + + if err := e.run("cache", "inspect", "abc123", "--show-content"); err != nil { + t.Fatalf("cache inspect --show-content: %v", err) + } + out := e.out.String() + if !strings.Contains(out, "review body here") { + t.Errorf("--show-content should print the cached body; got:\n%s", truncate(out, 800)) + } +} + +func TestCacheInspectAcceptsJSONSuffix(t *testing.T) { + e := newCLIEnv(t) + seedCacheEntry(t, e.repoRoot, "abc123", "openai", "gpt-4o", + time.Date(2026, 5, 20, 0, 0, 0, 0, time.UTC), "x") + + if err := e.run("cache", "inspect", "abc123.json"); err != nil { + t.Fatalf("cache inspect with .json suffix: %v", err) + } + if !strings.Contains(e.out.String(), "gpt-4o") { + t.Errorf("inspect should accept a key with .json suffix; got:\n%s", truncate(e.out.String(), 400)) + } +} diff --git a/internal/cli/config.go b/internal/cli/config.go index 61a70c3..10e06b0 100644 --- a/internal/cli/config.go +++ b/internal/cli/config.go @@ -183,8 +183,10 @@ func configFieldGet(cfg *config.Config, path string) (string, error) { return strconv.FormatBool(cfg.Cache.Enabled), nil case "ttl_days": return strconv.Itoa(cfg.Cache.TTLDays), nil + case "max_size_mb": + return strconv.Itoa(cfg.Cache.MaxSizeMB), nil default: - return "", fmt.Errorf("config: unknown field %q in cache (allowed: enabled, ttl_days)", parts[1]) + return "", fmt.Errorf("config: unknown field %q in cache (allowed: enabled, ttl_days, max_size_mb)", parts[1]) } case "guard": @@ -301,8 +303,17 @@ func configFieldSet(cfg *config.Config, path, value string) error { return errors.New("config: cache.ttl_days cannot be negative") } cfg.Cache.TTLDays = i + case "max_size_mb": + i, err := strconv.Atoi(value) + if err != nil { + return fmt.Errorf("config: cache.max_size_mb must be an integer; got %q", value) + } + if i < 0 { + return errors.New("config: cache.max_size_mb cannot be negative") + } + cfg.Cache.MaxSizeMB = i default: - return fmt.Errorf("config: unknown field %q in cache (allowed: enabled, ttl_days)", parts[1]) + return fmt.Errorf("config: unknown field %q in cache (allowed: enabled, ttl_days, max_size_mb)", parts[1]) } return nil diff --git a/internal/cli/config_test.go b/internal/cli/config_test.go index 703989e..b71ed81 100644 --- a/internal/cli/config_test.go +++ b/internal/cli/config_test.go @@ -105,29 +105,41 @@ func TestConfigGetUnknownKey(t *testing.T) { } } -func TestConfigGetMaxSizeMBNoLongerSupported(t *testing.T) { - // UC-02 cleanup: cache.max_size_mb was dead config — defined in - // the struct but never read anywhere. It is gone in v0.9.1, so - // `config get cache.max_size_mb` must now error with the standard - // "unknown field" message rather than silently returning a number. +func TestConfigGetMaxSizeMBDefaultsToZero(t *testing.T) { + // v1.3.0: cache.max_size_mb is a real key again (size-bounded + // eviction, ADR-0008). Unlike the v0.9.1-removed dead field, this one + // is read on the Put path. The default is 0 (unlimited). e := newCLIEnv(t) - err := e.run("config", "get", "cache.max_size_mb") - if err == nil { - t.Fatalf("max_size_mb should error after removal; got success: %s", e.out.String()) + if err := e.run("config", "get", "cache.max_size_mb"); err != nil { + t.Fatalf("config get cache.max_size_mb: %v", err) + } + if got := strings.TrimSpace(e.out.String()); got != "0" { + t.Errorf("cache.max_size_mb = %q, want %q (unlimited default)", got, "0") } - if !strings.Contains(err.Error(), "max_size_mb") || !strings.Contains(err.Error(), "unknown field") { - t.Errorf("error %q should name the offending field as unknown", err.Error()) +} + +func TestConfigSetMaxSizeMBRoundTrips(t *testing.T) { + e := newCLIEnv(t) + if err := e.run("config", "set", "cache.max_size_mb", "200"); err != nil { + t.Fatalf("config set cache.max_size_mb 200: %v", err) + } + cfg := loadCfg(t, e.homeDir) + if cfg.Cache.MaxSizeMB != 200 { + t.Errorf("cache.max_size_mb = %d, want 200", cfg.Cache.MaxSizeMB) } } -func TestConfigSetMaxSizeMBNoLongerSupported(t *testing.T) { +func TestConfigSetMaxSizeMBRejectsNegative(t *testing.T) { e := newCLIEnv(t) - err := e.run("config", "set", "cache.max_size_mb", "200") + // `--` stops cobra flag parsing so the negative value reaches the + // validation branch as a positional rather than being mistaken for a + // shorthand flag. + err := e.run("config", "set", "--", "cache.max_size_mb", "-5") if err == nil { - t.Fatal("max_size_mb set should error after removal") + t.Fatal("want error for negative max_size_mb, got nil") } - if !strings.Contains(err.Error(), "max_size_mb") || !strings.Contains(err.Error(), "unknown field") { - t.Errorf("error %q should name the offending field as unknown", err.Error()) + if !strings.Contains(err.Error(), "negative") { + t.Errorf("error %q should mention it cannot be negative", err.Error()) } } diff --git a/internal/cli/review.go b/internal/cli/review.go index eb293d7..77ad5c1 100644 --- a/internal/cli/review.go +++ b/internal/cli/review.go @@ -618,10 +618,18 @@ func openCache(repoRoot string, cfg config.CacheConfig) (*cache.Cache, error) { if cfg.TTLDays > 0 { ttl = time.Duration(cfg.TTLDays) * 24 * time.Hour } + // cache.max_size_mb bounds the on-disk cache (ADR-0008 size-bounded + // eviction); <=0 means unlimited. MiB so a "50" in config matches the + // human-readable byte formatting used by cache stats / clear / prune. + var maxBytes int64 + if cfg.MaxSizeMB > 0 { + maxBytes = int64(cfg.MaxSizeMB) * 1024 * 1024 + } return cache.Open(cache.Options{ - Dir: filepath.Join(repoRoot, ".commitbrief", "cache"), - RepoRoot: repoRoot, - TTL: ttl, + Dir: filepath.Join(repoRoot, ".commitbrief", "cache"), + RepoRoot: repoRoot, + TTL: ttl, + MaxSizeBytes: maxBytes, }) } diff --git a/internal/config/config.go b/internal/config/config.go index 63b6848..c94d054 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -60,7 +60,15 @@ type OutputConfig struct { Color string `yaml:"color"` } +// CacheConfig controls the local response cache (ADR-0008). MaxSizeMB +// bounds the on-disk cache: after each write, if the cache directory +// exceeds this many mebibytes the oldest entries are evicted oldest-first +// until it fits (the just-written entry is never evicted). Zero — the +// default — disables eviction; `cache prune` stays the manual stand-in. +// A new key rather than the v0.9.1-removed `max_size_mb` revival: this +// one is actually read on the Put path. type CacheConfig struct { - Enabled bool `yaml:"enabled"` - TTLDays int `yaml:"ttl_days"` + Enabled bool `yaml:"enabled"` + TTLDays int `yaml:"ttl_days"` + MaxSizeMB int `yaml:"max_size_mb"` } diff --git a/internal/i18n/messages.en.yml b/internal/i18n/messages.en.yml index df588fb..bffc155 100644 --- a/internal/i18n/messages.en.yml +++ b/internal/i18n/messages.en.yml @@ -43,6 +43,12 @@ cache.clear.confirm: "Remove all cached entries?" cache.clear.aborted: "Aborted; cache left untouched." cache.clear.success: "Removed %d cached entr(y/ies), freed %s." cache.prune.summary: "Pruned %d entr(y/ies) (%s); %d remain." +cache.stats.empty: "No cached entries (looked in %s)." +cache.stats.summary: "Cache: %d entr(y/ies), %s at %s." +cache.stats.range: "Oldest %s · newest %s." +cache.stats.limit.unlimited: "Size limit: unlimited (set cache.max_size_mb to bound)." +cache.stats.limit.bounded: "Size limit: %s (cache.max_size_mb=%d)." +cache.inspect.notfound: "No cache entry with key %q (looked in %s)." clipboard.copied: "%d findings copied to clipboard (%s) — paste anywhere" clipboard.empty: "Nothing to copy: review found 0 findings." diff --git a/internal/i18n/messages.tr.yml b/internal/i18n/messages.tr.yml index 86fa0cc..49ed1d1 100644 --- a/internal/i18n/messages.tr.yml +++ b/internal/i18n/messages.tr.yml @@ -43,6 +43,12 @@ cache.clear.confirm: "Tüm önbellek girdileri silinsin mi?" cache.clear.aborted: "İptal edildi; önbellek değiştirilmedi." cache.clear.success: "%d önbellek girdisi silindi, %s yer açıldı." cache.prune.summary: "%d girdi temizlendi (%s); %d girdi kaldı." +cache.stats.empty: "Önbellek girdisi yok (%s konumuna bakıldı)." +cache.stats.summary: "Önbellek: %d girdi, %s, konum %s." +cache.stats.range: "En eski %s · en yeni %s." +cache.stats.limit.unlimited: "Boyut sınırı: sınırsız (sınırlamak için cache.max_size_mb ayarlayın)." +cache.stats.limit.bounded: "Boyut sınırı: %s (cache.max_size_mb=%d)." +cache.inspect.notfound: "%q anahtarlı önbellek girdisi yok (%s konumuna bakıldı)." clipboard.copied: "%d bulgu panoya kopyalandı (%s) — istediğin yere yapıştırabilirsin" clipboard.empty: "Kopyalanacak bir şey yok: review 0 bulgu çıkardı."