Hold back generated output, and name files in commit subjects

DESIGN.md

@@ -16,6 +16,7 @@ The packages divide along one line: mechanics and judgment.
 | `repo`   | A typed layer over git, behind a one-method `Runner` interface. |
 | `plan`   | The intent of a run as data: renderable, editable, validatable. |
 | `group`  | The one judgment call, behind a `Grouper` interface. |
+| `generated` | Paths holding generated output, which a commit never carries. |
 | `style`  | Commit message conventions, applied and verified. |
 | `sweep`  | Debris detection. Reports only, never removes. |
 | `config` | `.preen.toml`, the per-repository defaults. |
@@ -32,6 +33,8 @@ matters more than avoiding a process boundary.
   absorbs unpushed commits back into the tree to be redone.
 - Groups the changes into self-contained commits, ordered so dependencies land
   first. The rules group by structure; semantic splitting needs a grouper.
+- Holds untracked generated output back from the plan, named with the pattern
+  that caught it. Overridable with `--allow-generated` or `[generated]`.
 - Shows a plan and changes nothing until it is approved. The prompt accepts
   merge, split, move, reword, drop, and reorder.
 - Stages each group precisely, including a subset of one file's hunks when an
@@ -53,8 +56,10 @@ against real repositories rather than a mock.
    hook reformatting files the run committed, and only when the caller passed
    `--allow-hook-rewrites`, which is then reported.
 2. **A plan accounts for the tree exactly once.** Every change lands in a
-   commit or a declared leftover, nothing lands twice, no commit is empty. An
-   edit that breaks this is refused and the previous plan stands.
+   commit, a declared leftover, or a declared held path, nothing lands twice,
+   no commit is empty. An edit that breaks this is refused and the previous
+   plan stands. Generated output is held rather than dropped for this reason:
+   a path preen declines to commit is still one it accounts for out loud.
 3. **Published work is not redone by accident.** A merge whose second parent is
    reachable from any remote moves the base forward instead of being flattened,
    and redoing a pushed commit requires `--pushed`.
@@ -67,7 +72,9 @@ against real repositories rather than a mock.
    index move, the working tree does not. `--hard` and `--keep` both delete
    files the undone commits added, which is a data-loss bug, not a preference.
 7. **Nothing is deleted on a guess.** The sweep reports debris and never
-   removes it.
+   removes it, and holding generated output back leaves the file untouched in
+   the working tree. preen never edits `.gitignore` either: doing so mid-run
+   would move the content hash the conservation check compares.
 
 ## Tests
 
@@ -93,6 +100,12 @@ Tag `vX.Y.Z`; the release workflow cross-builds and attaches archives. Keep
   wrong answer rewrites the wrong commit, so it stays coarse deliberately.
 - The grouper contract is JSON over stdin and stdout. No reference grouper
   ships with preen.
+- Two unrelated edits to two files in one package still share a commit. Telling
+  them apart means reading the diff for meaning, which the deterministic rules
+  will not do; the prompt's `split` and a `--grouper` cover it.
+- A held path cannot be recovered at the approval prompt. `drop` moves a path
+  out of the plan but nothing moves one back in, so the override is
+  `--allow-generated` or a `[generated] allow` entry.
 
 ## External artifacts
 

README.md

@@ -30,7 +30,9 @@ exactly. A single differing byte rolls the run back to the recovery branch it
 made before it started. Every run leaves a `preen-backup/<timestamp>` branch and
 `preen restore` puts you back. [How that works](#it-proves-it-did-not-lose-your-work).
 
-The built-in subjects say where, not why: `Add api`, `Update dependencies`. For
+The built-in subjects say what changed, not why: `Add cmd/parser.go and its
+test`, `Update dependencies`. Every one of them is built from the file list and
+nothing else, so a subject is a claim the commit can be checked against. For
 messages that explain intent, hand grouping to any program you like with
 `--grouper`, a model included, and reword anything at the approval prompt.
 
@@ -79,16 +81,20 @@ Planned commits (4):
 1. Update dependencies
      go.mod
 
-2. Add api
+2. Add api/server.go and its test
      api/server.go
      api/server_test.go
 
-3. Add store
+3. Add store/db.go
      store/db.go
 
-4. Add guide.md
+4. Update docs/guide.md
      docs/guide.md
 
+Held back as generated output (1), left uncommitted:
+     api/__pycache__/server.cpython-313.pyc  [__pycache__/: Python bytecode cache]
+   Add them to .gitignore, or rerun with --allow-generated to commit them.
+
 Apply this plan? 4 commits [y/n, or ? for edits]:
 ```
 
@@ -115,6 +121,10 @@ cannot walk you into losing a change.
   dependencies, CI, documentation, and configuration, so each commit stands on
   its own. Structure is what the rules can see; a `--grouper` program judges
   intent. Add `--gate` to verify each commit builds and passes.
+- Refuses to commit generated output. A `__pycache__` directory your `.gitignore`
+  never listed is untracked as far as git is concerned, and a tool that produces
+  commits should not help you record a bytecode cache as work. It is named in the
+  plan with the reason and left where it was. `--allow-generated` commits it.
 - Absorbs a run of unpushed commits back into the tree and redoes them clean
   with `--absorb`, no manual reset.
 - Folds dirty changes into the unpushed commits that introduced them with
@@ -140,12 +150,32 @@ It never invents changes and never touches a commit you did not ask it to.
 
 The grouping is deterministic and needs no model. preen separates dependency
 manifests, CI configuration, documentation, and configuration from source, then
-groups source by package, keeps a test file with the code it exercises, keeps
-rename pairs together, and treats anything you staged by hand as a boundary you
-drew deliberately. Dependencies are recorded first and documentation last.
-
-Because a fixed rule cannot know whether two hunks in one file are one idea or
-two, the built-in grouper never splits a file.
+groups source by package and treats anything you staged by hand as a boundary
+you drew deliberately. Dependencies are recorded first and documentation last.
+
+Inside a package it works in units rather than files. A unit is a source file
+plus the tests that exercise it, which is the one pairing a file name states
+outright: `parser_test.go` belongs with `parser.go`, and `test_runner.py` with
+`runner.py`. From there:
+
+- New code is separated from changes to code that was already there, so a new
+  file and its test do not ride along with two unrelated fixes.
+- Tests for code this run did not otherwise touch become their own commit, named
+  for what they test, and land after the code they depend on.
+- A file that moved between directories keeps both halves in one commit, so it
+  reads as a move rather than a deletion and an unrelated addition.
+
+Subjects are written from that same evidence and nothing else: the files, their
+directory, whether each is new or changed, and whether a test came with its
+source. That gives `Add cmd/parser.go and its test` and `Update token.go and
+tool.go in cmd` instead of `Update cmd`. Where a subject would run past
+`--max-subject`, it gives up detail rather than being cut off, falling back
+through the file name to the package name.
+
+Two unrelated edits to two files in the same package still share a commit.
+Telling them apart means reading the diff for meaning, which a fixed rule cannot
+do without guessing. For the same reason the built-in grouper never splits a
+file: it cannot know whether two hunks are one idea or two.
 
 When you want that judgment, hand grouping to a program:
 
@@ -165,6 +195,45 @@ than trusting it.
 Every guardrail is the same either way. The grouper chooses *what goes where*
 and nothing else.
 
+## Generated output
+
+A `__pycache__` directory your `.gitignore` never listed shows up in `git
+status` as untracked, which is the same thing a new source file looks like. A
+tool whose whole job is producing clean commits should not turn that into `Add
+__pycache__`.
+
+So preen recognizes the paths that hold generated output rather than work:
+bytecode caches, installed dependencies, coverage reports, and the files editors
+and operating systems leave lying around. An untracked path that matches one is
+held back from the plan, named with the pattern that caught it and what it
+holds, and left in your working tree exactly as it was.
+
+Held back is not dropped. The plan still accounts for the path, you see it
+before you approve anything, and the content hash preen conserves across the run
+still covers it. Nothing is deleted and nothing is written to your `.gitignore`:
+editing that file mid-run would change the very content the conservation check
+exists to prove was unchanged.
+
+Only untracked paths are ever held. A path git already tracks was committed
+deliberately at some point, and preen does not second-guess that.
+
+Override it where a project vendors one of these directories on purpose:
+
+```
+preen --allow-generated
+```
+
+```toml
+[generated]
+allow-all = false                        # commit generated output like anything else
+patterns = ["*.snap"]                    # hold these back too
+allow = ["third_party/node_modules"]     # commit these despite the patterns
+```
+
+A pattern with a slash is a path from the repository root and covers everything
+under it. A bare name ending in a slash is a directory, matched wherever it
+appears. Anything else is a glob on the file name.
+
 ## Message style
 
 preen writes a short imperative subject by default. Dictate the format with
@@ -198,6 +267,11 @@ allow-no-verify = false
 
 [protect]
 branches = ["develop", "release/*"]
+
+[generated]
+allow-all = false
+patterns = ["*.snap"]
+allow = ["third_party/node_modules"]
 ```
 
 Flags beat the config file, which beats the defaults. Every generated message is

cmd/cmd_preen.go

@@ -39,12 +39,15 @@ func runPreen(ctx context.Context, env *environment, args []string) (int, error)
 
 	engine := run.New(repository)
 	engine.Out = env.Out
+	// The grouper writes subjects, so it is told the length they have to fit.
+	builtIn := group.Heuristic{RespectStaged: true, MaxSubject: subjectBudget(opts.Style)}
+	engine.Grouper = builtIn
 	if settings.Grouper != "" {
 		// A grouper that fails or answers with something unusable falls back to
 		// the built-in rules rather than taking the run down with it.
 		engine.Grouper = group.Chain(
 			group.Command{Name: settings.Grouper, Dir: repository.Root()},
-			group.NewHeuristic(),
+			builtIn,
 		)
 	}
 	if opts.Fixup {
@@ -53,7 +56,12 @@ func runPreen(ctx context.Context, env *environment, args []string) (int, error)
 
 	built, err := engine.Plan(ctx, opts)
 	if err != nil {
-		if errors.Is(err, run.ErrNothingToDo) {
+		switch {
+		case errors.Is(err, run.ErrAllGenerated):
+			env.printf("Nothing to preen: every change is generated output.\n%v\n", err)
+			env.println("Add those paths to .gitignore, or rerun with --allow-generated.")
+			return CodeNothingToDo, nil
+		case errors.Is(err, run.ErrNothingToDo):
 			env.println("Nothing to preen: the tree is clean and no commits need redoing.")
 			return CodeNothingToDo, nil
 		}
@@ -94,7 +102,12 @@ func runFixup(ctx context.Context, env *environment, engine *run.Engine,
 	opts run.Options, settings promptSettings) (int, error) {
 	built, err := engine.PlanFixup(ctx, opts)
 	if err != nil {
-		if errors.Is(err, run.ErrNothingToDo) {
+		switch {
+		case errors.Is(err, run.ErrAllGenerated):
+			env.printf("Nothing to fold in: every change is generated output.\n%v\n", err)
+			env.println("Add those paths to .gitignore, or rerun with --allow-generated.")
+			return CodeNothingToDo, nil
+		case errors.Is(err, run.ErrNothingToDo):
 			env.println("Nothing to fold in: no dirty changes over unpushed commits.")
 			return CodeNothingToDo, nil
 		}
@@ -171,6 +184,8 @@ func parseRunFlags(env *environment, args []string) (opts run.Options, settings
 	fs.BoolVar(&opts.AllowHookRewrites, "allow-hook-rewrites", false,
 		"accept content changes a commit hook made to files the run committed")
 	fs.BoolVar(&opts.Sweep, "sweep", false, "report debug prints and other leftovers in the diff")
+	fs.BoolVar(&opts.AllowGenerated, "allow-generated", false,
+		"commit generated output instead of holding it back")
 	fs.BoolVar(&settings.DryRun, "dry-run", false, "show the plan and stop")
 	fs.BoolVar(&settings.Yes, "yes", false, "skip the approval prompt")
 	showVersion := fs.Bool("version", false, "print the version and exit")

cmd/cmd_test.go

@@ -546,3 +546,86 @@ func TestFixupCommandNothingToDo(t *testing.T) {
 		t.Errorf("output:\n%s", out)
 	}
 }
+
+// TestGeneratedOutputIsReportedNotCommitted checks the end to end behavior for
+// the defect that started this work: a bytecode cache git reports as untracked
+// must be named in the plan with the reason, and must not reach a commit.
+func TestGeneratedOutputIsReportedNotCommitted(t *testing.T) {
+	t.Parallel()
+	c := newCLI(t)
+	c.write("plugins/runner.py", "def run():\n    return 1\n")
+	c.write("plugins/__pycache__/runner.cpython-313.pyc", "\x00compiled\n")
+
+	code, out, err := c.run("y\n")
+	if err != nil {
+		t.Fatalf("run: %v", err)
+	}
+	if code != CodeOK {
+		t.Errorf("code = %d, want %d", code, CodeOK)
+	}
+	for _, want := range []string{"Held back as generated output", "Python bytecode cache", ".gitignore"} {
+		if !strings.Contains(out, want) {
+			t.Errorf("output does not mention %q:\n%s", want, out)
+		}
+	}
+	if recorded := c.git("log", "--name-only", "--format="); strings.Contains(recorded, "__pycache__") {
+		t.Errorf("the bytecode cache was committed:\n%s", recorded)
+	}
+	status := c.git("status", "--porcelain=v1", "--untracked-files=all")
+	if !strings.Contains(status, "?? plugins/__pycache__/runner.cpython-313.pyc") {
+		t.Errorf("the held path did not stay in the working tree:\n%s", status)
+	}
+}
+
+// TestGeneratedOverridesReachTheRun checks that both overrides get through the
+// command line, since a repository that vendors one of these paths has to be
+// able to commit it.
+func TestGeneratedOverridesReachTheRun(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		Name     string
+		Config   string
+		Args     []string
+		WantHeld bool
+	}{{ // Test 0: Without consent the cache is held back.
+		Name:     "default",
+		Args:     []string{"--dry-run"},
+		WantHeld: true,
+	}, { // Test 1: The flag commits it.
+		Name:     "flag",
+		Args:     []string{"--dry-run", "--allow-generated"},
+		WantHeld: false,
+	}, { // Test 2: The config file grants the same consent.
+		Name:     "config",
+		Config:   "[generated]\nallow-all = true\n",
+		Args:     []string{"--dry-run"},
+		WantHeld: false,
+	}, { // Test 3: The config exempts one path and holds the rest.
+		Name:     "config allow list",
+		Config:   "[generated]\nallow = [\"plugins/__pycache__/\"]\n",
+		Args:     []string{"--dry-run"},
+		WantHeld: false,
+	}}
+
+	for testNum, test := range tests {
+		t.Run(fmt.Sprintf("test %d %s", testNum, test.Name), func(t *testing.T) {
+			t.Parallel()
+			c := newCLI(t)
+			if test.Config != "" {
+				c.write(".preen.toml", test.Config)
+			}
+			c.write("plugins/runner.py", "def run():\n    return 1\n")
+			c.write("plugins/__pycache__/runner.cpython-313.pyc", "\x00compiled\n")
+
+			_, out, err := c.run("", test.Args...)
+			if err != nil {
+				t.Fatalf("run: %v", err)
+			}
+			held := strings.Contains(out, "Held back as generated output")
+			if held != test.WantHeld {
+				t.Errorf("held = %v, want %v, output:\n%s", held, test.WantHeld, out)
+			}
+		})
+	}
+}

cmd/execute.go

@@ -33,6 +33,13 @@ Run flags:
   --absorb        Bring unpushed commits back and redo them.
   --fixup         Fold changes into the unpushed commits that introduced them.
   --sweep         Report debug prints and other leftovers. Never removes them.
+  --allow-generated
+                  Commit generated output instead of holding it back. Untracked
+                  paths matching a never-commit pattern (__pycache__/, *.pyc,
+                  node_modules/, .DS_Store, coverage output, editor swap files
+                  and the like) are left uncommitted and reported, since they
+                  regenerate themselves. Add [generated] to .preen.toml to
+                  extend or exempt the patterns for one repository.
   --grouper PROG  Group with an external program instead of the built-in rules.
                   PROG reads JSON on stdin and writes JSON on stdout, and can
                   split one file's hunks across commits. It falls back to the

cmd/review.go

@@ -86,5 +86,6 @@ func clonePlan(src *plan.Plan) *plan.Plan {
 		dst.Commits[i].Parts = append([]plan.Part(nil), commit.Parts...)
 	}
 	dst.Leftover = append([]plan.Part(nil), src.Leftover...)
+	dst.Held = append([]plan.Held(nil), src.Held...)
 	return &dst
 }

cmd/settings.go

@@ -36,9 +36,37 @@ func applyConfig(opts *run.Options, cfg config.Config, args []string) {
 	// Protected branches always come from the repository, never from a flag, so
 	// a project can declare them once and no invocation can quietly drop them.
 	opts.Protected = cfg.Protect.Branches
+	if !flagGiven(args, "allow-generated") && cfg.Generated.AllowAll {
+		opts.AllowGenerated = true
+	}
+	opts.GeneratedPatterns = cfg.Generated.Patterns
+	opts.GeneratedAllow = cfg.Generated.Allow
 	opts.Style = mergeStyle(fileStyle, opts.Style, args)
 }
 
+// subjectBudget is the length the built-in grouper aims its subjects at: the
+// effective cap, less the room the style layer will take for a prefix.
+//
+// Working the prefix in here rather than truncating later is what keeps a
+// prefixed subject a whole phrase instead of a severed one.
+func subjectBudget(s style.Style) int {
+	budget := s.MaxSubject
+	if budget <= 0 {
+		budget = style.DefaultMaxSubject
+	}
+	if s.Prefix != "" {
+		budget -= len([]rune(s.Prefix)) + 1
+	}
+	if budget < minSubjectBudget {
+		return minSubjectBudget
+	}
+	return budget
+}
+
+// minSubjectBudget is the shortest subject budget worth aiming at. Below this a
+// subject cannot say anything, so the style layer's truncation takes over.
+const minSubjectBudget = 16
+
 // mergeStyle layers the invocation's style over the file's, keeping a file
 // value wherever the invocation did not name that flag.
 func mergeStyle(file, flags style.Style, args []string) style.Style {