diff --git a/README.md b/README.md index 9552ec5..9b2c94d 100644 --- a/README.md +++ b/README.md @@ -74,6 +74,7 @@ Shareable, safe for dotfiles: ```yaml language: English num_suggestions: 10 +# max_diff_chars: 40000 # size budget for the diff sent to the model # system_message: ... # commit_message_template: "... %s" # %s is replaced by the diff # pr_title_template: "... %s" @@ -171,6 +172,7 @@ Variant that lets you edit the message before committing: - `No staged changes to commit.` — run `git add` first. - `has no model configured` — run `lazycommit config set`. - `environment variable X is not set` — your config references `$X`; export it or store the key directly. +- Large change sets: when the diff is longer than `max_diff_chars`, lazycommit sends a list of the changed files with line counts plus as many file diffs as fit; a very long file list is itself cut short with a count of the remaining files. Lock files, minified files, and binary files are listed without their diffs. Lower the value if your model still rejects the request as too large. ## License diff --git a/internal/app/ports.go b/internal/app/ports.go index b8120c9..747b697 100644 --- a/internal/app/ports.go +++ b/internal/app/ports.go @@ -28,6 +28,7 @@ type PromptSettings struct { PRTitleTemplate domain.PromptTemplate Language domain.Language SuggestionCount int + MaxDiffChars int } // ConfigRepository yields the effective settings the use cases need. diff --git a/internal/app/suggest.go b/internal/app/suggest.go index 0db4ade..be8a813 100644 --- a/internal/app/suggest.go +++ b/internal/app/suggest.go @@ -55,7 +55,7 @@ func (uc *GeneratePRTitles) Execute(ctx context.Context, target string) (Suggest } // suggestionPipeline is the shared flow: read diff, short-circuit when -// empty, build prompt, generate, parse. Commit and PR generation differ +// empty, compact to the size budget, build prompt, generate, parse. Commit and PR generation differ // only in diff source and template. type suggestionPipeline struct { gen Generator @@ -86,6 +86,12 @@ func (p suggestionPipeline) run( return SuggestionsResult{}, fmt.Errorf("loading configuration: %w", err) } + maxDiff := settings.MaxDiffChars + if maxDiff <= 0 { + maxDiff = domain.DefaultMaxDiffChars + } + diff = diff.Compact(maxDiff) + prompt := domain.NewPromptBuilder(). WithSystemMessage(settings.SystemMessage). WithTemplate(pickTemplate(settings)). diff --git a/internal/app/suggest_test.go b/internal/app/suggest_test.go index d1aaacc..a92beaf 100644 --- a/internal/app/suggest_test.go +++ b/internal/app/suggest_test.go @@ -96,6 +96,27 @@ func TestCommitSuggestionsHappyPath(t *testing.T) { } } +func TestCommitSuggestionsCompactsLargeDiff(t *testing.T) { + raw := "diff --git a/big.txt b/big.txt\n--- a/big.txt\n+++ b/big.txt\n@@ -0,0 +1,500 @@\n" + + strings.Repeat("+some added content\n", 500) + settings := testSettings(t) + settings.MaxDiffChars = 1000 + gen := &fakeGenerator{output: "feat: one"} + uc := NewGenerateCommitSuggestions(gen, + &fakeDiffSource{staged: raw}, + &fakeConfig{settings: settings}) + + if _, err := uc.Execute(context.Background()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if strings.Contains(gen.lastPrompt.User, raw) { + t.Fatal("full diff must not be sent when it exceeds the budget") + } + if !strings.Contains(gen.lastPrompt.User, "big.txt +500 -0") { + t.Fatalf("prompt misses the file summary: %q", gen.lastPrompt.User) + } +} + func TestCommitSuggestionsEmptyDiffShortCircuits(t *testing.T) { gen := &fakeGenerator{output: "unused"} uc := NewGenerateCommitSuggestions(gen, diff --git a/internal/config/config.go b/internal/config/config.go index ef64af0..dca5ec6 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -50,6 +50,7 @@ type Prompts struct { CommitMessageTemplate string `yaml:"commit_message_template,omitempty"` PRTitleTemplate string `yaml:"pr_title_template,omitempty"` NumSuggestions int `yaml:"num_suggestions,omitempty"` + MaxDiffChars int `yaml:"max_diff_chars,omitempty"` } // DefaultBackends is the effective configuration when no file exists; the @@ -198,6 +199,10 @@ func (r *Repository) PromptSettings() (app.PromptSettings, error) { if count <= 0 { count = domain.DefaultSuggestionCount } + maxDiff := p.MaxDiffChars + if maxDiff <= 0 { + maxDiff = domain.DefaultMaxDiffChars + } return app.PromptSettings{ SystemMessage: system, @@ -205,6 +210,7 @@ func (r *Repository) PromptSettings() (app.PromptSettings, error) { PRTitleTemplate: pr, Language: domain.NewLanguage(p.Language), SuggestionCount: count, + MaxDiffChars: maxDiff, }, nil } @@ -238,6 +244,9 @@ func mergePrompts(top, bottom Prompts) Prompts { if top.NumSuggestions > 0 { out.NumSuggestions = top.NumSuggestions } + if top.MaxDiffChars > 0 { + out.MaxDiffChars = top.MaxDiffChars + } return out } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index c5f4e53..a308964 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -125,6 +125,9 @@ func TestPromptSettingsDefaultsWhenNoFiles(t *testing.T) { if s.SystemMessage != domain.DefaultSystemMessage { t.Fatalf("system = %q", s.SystemMessage) } + if s.MaxDiffChars != domain.DefaultMaxDiffChars { + t.Fatalf("max diff chars = %d", s.MaxDiffChars) + } } func TestPromptSettingsLayering(t *testing.T) { @@ -134,10 +137,12 @@ func TestPromptSettingsLayering(t *testing.T) { language: Spanish system_message: global system num_suggestions: 7 +max_diff_chars: 5000 `) writeFile(t, filepath.Join(repoRoot, "lazycommit.prompts.yaml"), ` language: Korean commit_message_template: "REPO %s" +max_diff_chars: 8000 `) s, err := NewRepository(globalDir, repoRoot).PromptSettings() @@ -153,6 +158,9 @@ commit_message_template: "REPO %s" if s.SuggestionCount != 7 { t.Fatalf("global count should fall through, got %d", s.SuggestionCount) } + if s.MaxDiffChars != 8000 { + t.Fatalf("repo-local max diff chars should win, got %d", s.MaxDiffChars) + } if s.CommitTemplate.String() != "REPO %s" { t.Fatalf("repo-local template should win, got %q", s.CommitTemplate) } diff --git a/internal/domain/diff.go b/internal/domain/diff.go index 727c2b7..f53d9b5 100644 --- a/internal/domain/diff.go +++ b/internal/domain/diff.go @@ -2,13 +2,44 @@ package domain import ( "errors" + "fmt" + "path" + "strconv" "strings" + "unicode/utf8" ) // ErrEmptyDiff signals that there are no changes to describe. Callers use it // to short-circuit before any backend is invoked. var ErrEmptyDiff = errors.New("diff is empty") +// DefaultMaxDiffChars bounds the diff sent to a backend (roughly 10k +// tokens), so large change sets do not exceed model context or rate limits. +const DefaultMaxDiffChars = 40000 + +// MinDiffBudget is the smallest budget Compact honours; smaller values are +// raised to it so the summary header always fits. +const MinDiffBudget = 1000 + +const truncatedMarker = "[truncated]\n" + +// noiseFiles are generated files whose diffs carry no meaning for a commit +// message; only their names and line counts are kept when compacting. +var noiseFiles = map[string]bool{ + "package-lock.json": true, + "yarn.lock": true, + "pnpm-lock.yaml": true, + "go.sum": true, + "Cargo.lock": true, + "poetry.lock": true, + "uv.lock": true, + "composer.lock": true, + "Gemfile.lock": true, + "Pipfile.lock": true, +} + +var noiseSuffixes = []string{".min.js", ".min.css", ".js.map", ".css.map"} + // Diff is a non-empty git diff. type Diff struct { text string @@ -24,3 +55,239 @@ func NewDiff(text string) (Diff, error) { func (d Diff) String() string { return d.text } + +// Compact returns a diff of at most budget bytes; a positive budget below +// MinDiffBudget is raised to it. A diff that already fits is returned +// unchanged. Otherwise the result starts with a summary of the changed +// files (cut short with a count when the list alone would not fit), +// followed by every file diff that fits in full; leftover space goes +// to the first file that did not fit, cut at a line boundary. Lock files, +// minified files, and binary files appear only in the summary. +func (d Diff) Compact(budget int) Diff { + if budget <= 0 { + return d + } + budget = max(budget, MinDiffBudget) + if len(d.text) <= budget { + return d + } + + files := splitFileDiffs(d.text) + if len(files) == 0 { + return Diff{text: truncate(d.text, budget)} + } + + var out strings.Builder + writeSummary(&out, files, budget) + var skipped *fileDiff + for i := range files { + f := &files[i] + if f.binary || f.noise { + continue + } + if out.Len()+len(f.text) <= budget { + out.WriteString(f.text) + } else if skipped == nil { + skipped = f + } + } + if skipped != nil { + out.WriteString(truncate(skipped.text, budget-out.Len())) + } + return Diff{text: out.String()} +} + +type fileDiff struct { + text string + path string + added int + removed int + binary bool + noise bool +} + +// splitFileDiffs splits a unified git diff into per-file sections. Text +// before the first "diff --git" header is dropped. +func splitFileDiffs(text string) []fileDiff { + const header = "diff --git " + var starts []int + if strings.HasPrefix(text, header) { + starts = append(starts, 0) + } + for i := 0; ; { + j := strings.Index(text[i:], "\n"+header) + if j < 0 { + break + } + i += j + 1 + starts = append(starts, i) + } + + files := make([]fileDiff, len(starts)) + for k, start := range starts { + end := len(text) + if k+1 < len(starts) { + end = starts[k+1] + } + files[k] = newFileDiff(text[start:end]) + } + return files +} + +func newFileDiff(text string) fileDiff { + f := fileDiff{text: text} + var header, oldPath, newPath, renamePath string + inHunk := false + for line := range strings.Lines(text) { + line = strings.TrimSuffix(line, "\n") + if header == "" { + header = line + continue + } + if inHunk { + switch { + case strings.HasPrefix(line, "+"): + f.added++ + case strings.HasPrefix(line, "-"): + f.removed++ + } + continue + } + switch { + case strings.HasPrefix(line, "@@"): + inHunk = true + case strings.HasPrefix(line, "Binary files "), strings.HasPrefix(line, "GIT binary patch"): + f.binary = true + case strings.HasPrefix(line, "--- "): + oldPath = markerPath(line[len("--- "):]) + case strings.HasPrefix(line, "+++ "): + newPath = markerPath(line[len("+++ "):]) + case strings.HasPrefix(line, "rename to "): + renamePath = unquotePath(line[len("rename to "):]) + } + } + + switch { + case newPath != "": + f.path = newPath + case oldPath != "": + f.path = oldPath + case renamePath != "": + f.path = renamePath + default: + f.path = headerPath(header) + } + + base := path.Base(f.path) + f.noise = noiseFiles[base] + for _, suffix := range noiseSuffixes { + if strings.HasSuffix(base, suffix) { + f.noise = true + } + } + return f +} + +// markerPath extracts the path from the value of a "--- " or "+++ " line. +// It returns "" for /dev/null, the side of an added or deleted file. +func markerPath(value string) string { + value = strings.TrimSuffix(value, "\t") // git appends a tab to names with spaces + if value == "/dev/null" { + return "" + } + return stripSidePrefix(unquotePath(value)) +} + +// headerPath reads the path from a "diff --git" header. It is the fallback +// for sections without "---", "+++", or "rename to" lines: binary and +// mode-only changes, where both sides name the same path. +func headerPath(header string) string { + rest := strings.TrimPrefix(header, "diff --git ") + // "a/X b/X" splits exactly in the middle, even when X contains " b/". + if mid := len(rest) / 2; len(rest)%2 == 1 && rest[mid] == ' ' && + strings.HasPrefix(rest, "a/") && rest[mid+1:mid+3] == "b/" && rest[2:mid] == rest[mid+3:] { + return rest[mid+3:] + } + if i := strings.LastIndex(rest, ` "b/`); i >= 0 { + return stripSidePrefix(unquotePath(rest[i+1:])) + } + if i := strings.LastIndex(rest, " b/"); i >= 0 { + return rest[i+len(" b/"):] + } + return rest +} + +// unquotePath decodes git's C-style quoting, used for paths with special +// or non-ASCII characters. +func unquotePath(p string) string { + if strings.HasPrefix(p, `"`) { + if u, err := strconv.Unquote(p); err == nil { + return u + } + } + return p +} + +func stripSidePrefix(p string) string { + if strings.HasPrefix(p, "a/") || strings.HasPrefix(p, "b/") { + return p[2:] + } + return p +} + +// writeSummary lists every file with its line counts. When the list would +// not fit in the budget, the remaining files are replaced by a count. +func writeSummary(out *strings.Builder, files []fileDiff, budget int) { + fmt.Fprintf(out, "Diff shortened to fit the size limit. Changed files (%d):\n", len(files)) + for i, f := range files { + line := fmt.Sprintf("%s +%d -%d\n", f.path, f.added, f.removed) + switch { + case f.binary: + line = fmt.Sprintf("%s (binary, diff omitted)\n", f.path) + case f.noise: + line = fmt.Sprintf("%s +%d -%d (generated, diff omitted)\n", f.path, f.added, f.removed) + } + tail := len("\n") + if rest := len(files) - i - 1; rest > 0 { + tail += len(moreFilesLine(rest)) + } + if out.Len()+len(line)+tail > budget { + out.WriteString(moreFilesLine(len(files) - i)) + break + } + out.WriteString(line) + } + out.WriteString("\n") +} + +func moreFilesLine(n int) string { + if n == 1 { + return "... and 1 more file\n" + } + return fmt.Sprintf("... and %d more files\n", n) +} + +// truncate cuts text to at most n bytes and marks the cut; text that +// already fits is returned unchanged. It cuts at a line boundary when one +// fits, otherwise at a rune boundary. It returns "" when n leaves no room +// for content. +func truncate(text string, n int) string { + if len(text) <= n { + return text + } + limit := n - len(truncatedMarker) + if limit <= 0 { + return "" + } + if cut := strings.LastIndexByte(text[:limit], '\n'); cut >= 0 { + return text[:cut+1] + truncatedMarker + } + cut := limit - len("\n") + for cut > 0 && !utf8.RuneStart(text[cut]) { + cut-- + } + if cut == 0 { + return "" + } + return text[:cut] + "\n" + truncatedMarker +} diff --git a/internal/domain/domain_test.go b/internal/domain/domain_test.go index ac0de8c..4513b5a 100644 --- a/internal/domain/domain_test.go +++ b/internal/domain/domain_test.go @@ -2,6 +2,7 @@ package domain import ( "errors" + "strconv" "strings" "testing" ) @@ -19,6 +20,161 @@ func TestNewDiff(t *testing.T) { } } +// fileDiffText builds a minimal git diff section for path with n added lines. +func fileDiffText(path string, n int) string { + var b strings.Builder + b.WriteString("diff --git a/" + path + " b/" + path + "\n") + b.WriteString("--- a/" + path + "\n+++ b/" + path + "\n") + b.WriteString("@@ -0,0 +1," + strconv.Itoa(n) + " @@\n") + for i := 0; i < n; i++ { + b.WriteString("+line " + strconv.Itoa(i) + " of " + path + "\n") + } + return b.String() +} + +func TestCompactKeepsSmallDiff(t *testing.T) { + raw := fileDiffText("main.go", 3) + d, _ := NewDiff(raw) + if got := d.Compact(len(raw)).String(); got != raw { + t.Fatalf("diff within budget must be unchanged, got %q", got) + } + if got := d.Compact(0).String(); got != raw { + t.Fatalf("non-positive budget must keep diff unchanged, got %q", got) + } +} + +func TestCompactSkipsGeneratedAndBinaryFiles(t *testing.T) { + binary := "diff --git a/logo.png b/logo.png\nindex 1..2 100644\nBinary files a/logo.png and b/logo.png differ\n" + raw := fileDiffText("main.go", 3) + fileDiffText("go.sum", 500) + binary + fileDiffText("web/app.min.js", 200) + d, _ := NewDiff(raw) + got := d.Compact(2000).String() + + for _, want := range []string{ + "Changed files (4):", + "main.go +3 -0\n", + "go.sum +500 -0 (generated, diff omitted)", + "logo.png (binary, diff omitted)", + "web/app.min.js +200 -0 (generated, diff omitted)", + fileDiffText("main.go", 3), + } { + if !strings.Contains(got, want) { + t.Fatalf("compacted diff misses %q:\n%s", want, got) + } + } + for _, unwanted := range []string{"+line 0 of go.sum", "Binary files", "+line 0 of web/app.min.js"} { + if strings.Contains(got, unwanted) { + t.Fatalf("compacted diff should not contain %q:\n%s", unwanted, got) + } + } +} + +func TestCompactFitsBudget(t *testing.T) { + small := fileDiffText("a.go", 5) + big := fileDiffText("b.go", 1000) + raw := small + big + fileDiffText("c.go", 5) + d, _ := NewDiff(raw) + budget := 3000 + got := d.Compact(budget).String() + + if len(got) > budget { + t.Fatalf("compacted diff is %d chars, budget %d", len(got), budget) + } + if !strings.Contains(got, small) { + t.Fatal("file that fits must be kept in full") + } + if !strings.Contains(got, "+line 0 of b.go\n") || !strings.HasSuffix(got, "\n[truncated]\n") { + t.Fatalf("file over the budget must be cut and marked:\n%s", got) + } + if !strings.Contains(got, fileDiffText("c.go", 5)) { + t.Fatal("a later file that fits must be kept in full after skipping a large one") + } + if !strings.Contains(got, "b.go +1000 -0\n") { + t.Fatal("summary must list every file") + } +} + +func TestCompactRaisesTinyBudget(t *testing.T) { + raw := fileDiffText("a.go", 1000) + d, _ := NewDiff(raw) + got := d.Compact(1).String() + if len(got) > MinDiffBudget || !strings.Contains(got, "Changed files (1):") { + t.Fatalf("tiny budget must be raised to %d, got %d chars:\n%s", MinDiffBudget, len(got), got) + } +} + +func TestCompactDetectsGitBinaryPatch(t *testing.T) { + binary := "diff --git a/logo.png b/logo.png\nindex 1..2 100644\nGIT binary patch\nliteral 9000\n" + + strings.Repeat("zcmeHM\n", 1000) + d, _ := NewDiff(binary + fileDiffText("main.go", 3)) + got := d.Compact(MinDiffBudget).String() + if !strings.Contains(got, "logo.png (binary, diff omitted)") || strings.Contains(got, "zcmeHM") { + t.Fatalf("GIT binary patch must be treated as binary:\n%s", got) + } +} + +func TestCompactParsesPaths(t *testing.T) { + quoted := "diff --git \"a/d\\303\\266c/go.sum\" \"b/d\\303\\266c/go.sum\"\n" + + "--- \"a/d\\303\\266c/go.sum\"\n+++ \"b/d\\303\\266c/go.sum\"\n@@ -0,0 +1 @@\n+x\n" + spaced := "diff --git a/dir b/yarn.lock b/dir b/yarn.lock\n" + + "--- a/dir b/yarn.lock\t\n+++ b/dir b/yarn.lock\t\n@@ -1 +1 @@\n-x\n+y\n" + deleted := "diff --git a/old.go b/old.go\ndeleted file mode 100644\n" + + "--- a/old.go\n+++ /dev/null\n@@ -1 +0,0 @@\n-x\n" + binary := "diff --git a/img b/pic.png b/img b/pic.png\nindex 1..2 100644\n" + + "Binary files a/img b/pic.png and b/img b/pic.png differ\n" + renamed := "diff --git a/old b/x.go b/new b/x.go\nsimilarity index 100%\n" + + "rename from old b/x.go\nrename to new b/x.go\n" + raw := quoted + spaced + deleted + binary + renamed + fileDiffText("linker.map", 2) + + fileDiffText("app.js.map", 2) + fileDiffText("big.go", 1000) + d, _ := NewDiff(raw) + got := d.Compact(MinDiffBudget).String() + + for _, want := range []string{ + "döc/go.sum +1 -0 (generated, diff omitted)", + "dir b/yarn.lock +1 -1 (generated, diff omitted)", + "old.go +0 -1\n", + "img b/pic.png (binary, diff omitted)", + "new b/x.go +0 -0\n", + "linker.map +2 -0\n", + "app.js.map +2 -0 (generated, diff omitted)", + } { + if !strings.Contains(got, want) { + t.Fatalf("summary misses %q:\n%s", want, got) + } + } +} + +func TestCompactCutsLongFileList(t *testing.T) { + var raw strings.Builder + for i := 0; i < 500; i++ { + raw.WriteString(fileDiffText("pkg/file"+strconv.Itoa(i)+".go", 1)) + } + d, _ := NewDiff(raw.String()) + budget := 1000 + got := d.Compact(budget).String() + + if len(got) > budget { + t.Fatalf("compacted diff is %d chars, budget %d", len(got), budget) + } + if !strings.Contains(got, "more files\n") { + t.Fatalf("long file list must be cut:\n%s", got) + } +} + +func TestCompactNonGitText(t *testing.T) { + for name, raw := range map[string]string{ + "lines": strings.Repeat("+plain line\n", 200), + "single line": strings.Repeat("x", 5000), + } { + t.Run(name, func(t *testing.T) { + d, _ := NewDiff(raw) + got := d.Compact(MinDiffBudget).String() + if len(got) > MinDiffBudget || !strings.HasSuffix(got, "\n[truncated]\n") || len(got) < 100 { + t.Fatalf("text without file headers must be cut to the budget: %q", got) + } + }) + } +} + func TestNewSuggestion(t *testing.T) { cases := []struct { name string diff --git a/internal/git/git.go b/internal/git/git.go index 1c00aa6..27bd2ef 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -18,7 +18,7 @@ func New() *CLI { // StagedDiff returns `git diff --cached`; empty output means nothing staged. func (c *CLI) StagedDiff(ctx context.Context) (string, error) { - return run(ctx, "diff", "--cached") + return run(ctx, diffArgs("--cached")...) } // BranchDiff returns the merge-base diff between target and HEAD — the same @@ -27,7 +27,14 @@ func (c *CLI) BranchDiff(ctx context.Context, target string) (string, error) { if _, err := run(ctx, "rev-parse", "--verify", target); err != nil { return "", fmt.Errorf("branch %q does not exist", target) } - return run(ctx, "diff", target+"...HEAD") + return run(ctx, diffArgs(target+"...HEAD")...) +} + +// diffArgs pins the diff format that domain.Diff parses, so user settings +// such as diff.noprefix, diff.mnemonicPrefix, diff.external, or +// color.ui=always cannot change it. +func diffArgs(extra ...string) []string { + return append([]string{"diff", "--no-color", "--no-ext-diff", "--src-prefix=a/", "--dst-prefix=b/"}, extra...) } // RepoRoot returns the repository top-level directory, or "" when the diff --git a/internal/git/git_test.go b/internal/git/git_test.go index 5e3d637..db65c1b 100644 --- a/internal/git/git_test.go +++ b/internal/git/git_test.go @@ -73,6 +73,25 @@ func TestStagedDiff(t *testing.T) { } } +func TestStagedDiffIgnoresFormatConfig(t *testing.T) { + dir := initRepo(t) + gitRun(t, dir, "config", "diff.noprefix", "true") + gitRun(t, dir, "config", "diff.mnemonicPrefix", "true") + gitRun(t, dir, "config", "color.ui", "always") + if err := os.WriteFile(filepath.Join(dir, "new.txt"), []byte("x\n"), 0o644); err != nil { + t.Fatal(err) + } + gitRun(t, dir, "add", "new.txt") + + out, err := New().StagedDiff(context.Background()) + if err != nil { + t.Fatal(err) + } + if !strings.HasPrefix(out, "diff --git a/new.txt b/new.txt\n") || !strings.Contains(out, "\n+++ b/new.txt\n") { + t.Fatalf("diff format must not depend on prefix or color config: %q", out) + } +} + func TestBranchDiff(t *testing.T) { dir := initRepo(t) cli := New()