diff --git a/architecture/README.md b/architecture/README.md index 69b3f6a..4e0fd27 100644 --- a/architecture/README.md +++ b/architecture/README.md @@ -6,7 +6,7 @@ living prose, no frontmatter — dated by git. **Promotion rule:** when a change alters a capability's behavior, hand-edit the matching `architecture/.md` in the **same PR** that ships the code. The edit rides in the implementing diff and is reviewed with it — never applied -as a separate post-merge step. The change bundle in `planning/changes/` stays as +as a separate post-merge step. The change file in `planning/changes/` stays as the *why*; these files are the *what is true now*. ## Capabilities diff --git a/planning/.convention-version b/planning/.convention-version index 3eefcb9..227cea2 100644 --- a/planning/.convention-version +++ b/planning/.convention-version @@ -1 +1 @@ -1.0.0 +2.0.0 diff --git a/planning/README.md b/planning/README.md index 50c89c9..41321cb 100644 --- a/planning/README.md +++ b/planning/README.md @@ -15,25 +15,22 @@ it, re-run that repo's `APPLY.md` flow. **1. Choose a lane — first matching rule wins:** 1. Any of: needs design judgment · new file/module · public-API change · - cross-cutting or multi-file · non-trivial test design → **Full** - (`design.md` + `plan.md`) + cross-cutting or multi-file · non-trivial test design → **Full** (design template) 2. Purely mechanical: typo · dep bump · linter/formatter/CI tweak · - mechanical rename · single-line config → **Tiny** (no bundle, conventional + mechanical rename · single-line config → **Tiny** (no change file, conventional commit) 3. Small-but-real, none of the above: ≲30 LOC net · ≤2 files · no new file · - no public-API change · one straightforward test → **Lightweight** - (`change.md`) + no public-API change · one straightforward test → **Lightweight** (change template) -Ambiguous between two? Take the heavier. A `change.md` that outgrows its lane -splits into `design.md` + `plan.md`. +Ambiguous between two? Take the heavier. A lightweight change file that outgrows its lane is rewritten from the design template. -**2. Create the bundle** (Full / Lightweight only): -`planning/changes/YYYY-MM-DD.NN-/`, where `.NN` is a zero-padded -intra-day counter. Copy the matching template from +**2. Create the change file** (Full / Lightweight only): +`planning/changes/YYYY-MM-DD.NN-.md`, where `.NN` is a zero-padded +intra-day counter — copied from the matching template (design or change) in [`_templates/`](_templates/). **3. Ship in the implementing PR:** hand-edit the affected -`architecture/.md`, finalize the bundle's `summary:` to the +`architecture/.md`, finalize the change file's `summary:` to the realized result, and run `just check-planning` before pushing. ## Conventions @@ -46,20 +43,41 @@ realized result, and run `just check-planning` before pushing. ### Two axes, never mixed -- **`architecture/` (repo root) — the present.** One file per capability, - living prose, updated in the same PR that ships the change. The truth home. -- **`planning/changes/` — the past-and-pending.** One folder per change, +- **`architecture/` (repo root) — the present.** One file per capability, plus + a single `glossary.md` (the ubiquitous language); living prose, updated in the + same PR that ships the change. The truth home. +- **`planning/changes/` — the past-and-pending.** One file per change, kept in place after ship. A change **promotes** its conclusions into the affected `architecture/.md` by hand **in the implementing PR, alongside the code** — the edit rides in the same diff and is reviewed with it, never applied as a separate post-merge step. That hand-edit is what keeps `architecture/` -true; the bundle stays in `changes/` as the *why*. +true; the change file stays in `changes/` as the *why*. -### Change bundles +### Glossary -A change is a folder `changes/YYYY-MM-DD.NN-/`: +`architecture/glossary.md` is the project's **ubiquitous language** — one page +defining the domain terms that code, specs, and capability pages all share. Like +the capability files beside it, it is living prose with **no frontmatter**, dated +by git, and authored lazily: it appears when the first term is worth pinning down. + +Each entry is a term, a one-or-two-sentence definition of what it *is* (not what +it does), and an optional `_Avoid_:` line naming the synonyms to reject: + +```md +**Timer**: +A scheduled future delivery, identified by a timer id. +_Avoid_: job, task, alarm +``` + +Keep it a glossary, not a spec — no implementation detail. A change that +introduces or sharpens a term updates `glossary.md` in the same PR, the same way +a behavior change promotes into a capability file. + +### Change files + +A change is a file `changes/YYYY-MM-DD.NN-.md`: - `YYYY-MM-DD` — proposal date; `.NN` — zero-padded intra-day counter (`.01`, `.02`, …) that breaks same-date ties so the timeline sorts stably. @@ -68,25 +86,44 @@ A change is a folder `changes/YYYY-MM-DD.NN-/`: `summary` is written when the change is created (the intent one-liner) and **finalized at ship** to state the realized result — set in the implementing PR, alongside the code and the `architecture/` promotion. No post-merge -bookkeeping, no folder move. `date` and `slug` are never written — they are -read from the bundle's directory name. +bookkeeping, no file move. `date` and `slug` are never written — they are +read from the file name. ### Three lanes | Lane | Artifacts | Use when | |------|-----------|----------| -| **Full** | `design.md` + `plan.md` | design judgment; new file/module; public-API change; cross-cutting/multi-file; non-trivial test design | -| **Lightweight** | `change.md` | small-but-real: ≲30 LOC net, ≤2 files, no new file, no public-API change, single straightforward test | +| **Full** | one change file from the design template | design judgment; new file/module; public-API change; cross-cutting/multi-file; non-trivial test design | +| **Lightweight** | one change file from the change template | small-but-real: ≲30 LOC net, ≤2 files, no new file, no public-API change, single straightforward test | | **Tiny** | none — conventional commit | typo, dep bump, linter/formatter/CI tweak, mechanical rename, single-line config | -Heavier lane wins on ambiguity. A `change.md` that outgrows its lane splits -into `design.md` + `plan.md`. +Heavier lane wins on ambiguity. A lightweight change file that outgrows its lane is rewritten from the design template. + +### Plans are ephemeral + +The executable plan — task checklists, embedded code, commit sequences, +whatever the executor needs — is a working artifact, not history. Keep it out +of `changes/` and out of version control (git-ignored scratch, e.g. +`.superpowers/`). Once the change ships, the diff and the PR are the record +of execution; a committed plan duplicates them. `check-planning` rejects +anything in `changes/` that is not a flat change file. + +### Lean specs + +The change file is the single home of a change's rationale: + +- The PR body summarizes and links to the change file — it never restates it. +- Rejected alternatives live in `decisions/` and are referenced, not retold. +- Show a sketch when the design needs code; never the full diff-to-be. +- Delete template sections that don't apply — an empty section is ceremony. +- Most designs fit well under ~700 words; length must buy information. ### Artifacts at a glance -- **`design.md`** — the spec: the *thinking* (why, design, trade-offs, scope). -- **`plan.md`** — the plan: the *sequencing* (the executor's task checklist). -- **`change.md`** — both, condensed, for the lightweight lane. +- **design template** — the spec: the *thinking* (why, design, trade-offs, + scope); the change file it produces is the single home of rationale (see + [Lean specs](#lean-specs)). +- **change template** — the condensed spec for the lightweight lane. - **`releases/.md`** — per-release user-facing notes. - **`audits/-.md`** — findings from a code/docs/bug-hunt sweep; spawns fix changes. @@ -100,11 +137,10 @@ Templates live in [`_templates/`](_templates/). ### Frontmatter -`date` and `slug` are **derived from the directory / file name** — never +`date` and `slug` are **derived from the file name** — never repeated in frontmatter. So: -- `design.md` / `change.md`: `summary` (single line) only. -- `plan.md`: **no frontmatter** — its identity is the bundle directory. +- `changes/*.md`: `summary` (single line) only. - `decisions/*.md`: `status` (accepted|superseded), `summary`, and optional `supersedes` / `superseded_by`. - Files in `architecture/` carry **no** frontmatter — living prose, dated by git. @@ -117,7 +153,7 @@ only field the index renders. - `just index` — print the generated change/decision index (a query over the files, never committed). -- `just check-planning` — validate bundle/decision names, bundle shape, and - required frontmatter. Runs in `just lint-ci`. +- `just check-planning` — validate change/decision names, change-file shape, + and required frontmatter. Runs in `just lint-ci`. - [`deferred.md`](deferred.md) — real-but-unscheduled items. - [`architecture/`](../architecture/) — the living truth home. diff --git a/planning/_templates/change.md b/planning/_templates/change.md index d4c8962..5aa7e81 100644 --- a/planning/_templates/change.md +++ b/planning/_templates/change.md @@ -5,8 +5,8 @@ summary: One line — shown in the generated index. Written at creation; finaliz # Change: One-line capitalized title **Lane:** lightweight — ≲30 LOC net, ≤2 files, no new file, no public-API -change, a single straightforward test. If it outgrows this, split into -`design.md` + `plan.md`. +change, a single straightforward test. If it outgrows this, rewrite it from +the design template. ## Goal diff --git a/planning/_templates/design.md b/planning/_templates/design.md index d63e22d..17dbee1 100644 --- a/planning/_templates/design.md +++ b/planning/_templates/design.md @@ -4,6 +4,10 @@ summary: One line — shown in the generated index. Written at creation; finaliz # Design: One-line capitalized title + + ## Summary One paragraph. What changes, at the level a reader needs to decide if this @@ -12,37 +16,23 @@ spec is worth reading in full. ## Motivation Why now. What is broken or missing. Concrete observations / numbers, not -abstract complaints. Link to memory entries or earlier specs when relevant. - -## Non-goals - -What is deliberately out of scope and (when nontrivial) why. Each item is -a sentence; one line each. +abstract complaints. ## Design -### 1. - What changes, in enough detail that a reader who has not seen the codebase -can follow. Code samples / diagrams welcome. +can follow. Sketches and interface fragments welcome; never the full +diff-to-be. Reference rejected alternatives in `decisions/` instead of +retelling them. -### 2. - -... - -## Operations - -Out-of-repo steps (DNS, infra, external account changes). Omit if none. - -## Out of scope +## Non-goals -Already covered above under Non-goals if appropriate. Repeat-list of -explicitly-excluded follow-ups belongs here when the list is long. +What is deliberately out of scope and (when nontrivial) why. One line each. ## Testing -How we know it landed correctly. New pytest? Smoke check on live URL? -Lint pass? Be specific. +How we know it landed correctly. Be specific: the command and the expected +signal. ## Risk diff --git a/planning/_templates/glossary.md b/planning/_templates/glossary.md new file mode 100644 index 0000000..82385c3 --- /dev/null +++ b/planning/_templates/glossary.md @@ -0,0 +1,15 @@ +# Glossary + +The project's ubiquitous language — the domain terms that code, specs, and +capability pages share. Living prose, no frontmatter, dated by git. Each entry is +a term, what it *is* (not what it does), and the synonyms to avoid. No +implementation detail; this is a glossary, not a spec. + +**Term**: +A one-or-two-sentence definition of what it is. +_Avoid_: rejected-synonym, another-one + +**Another term**: +Define what it is, tightly. Group related terms under `##` subheadings when +natural clusters emerge; a flat list is fine when they don't. +_Avoid_: … diff --git a/planning/_templates/plan.md b/planning/_templates/plan.md deleted file mode 100644 index 132d720..0000000 --- a/planning/_templates/plan.md +++ /dev/null @@ -1,46 +0,0 @@ -# — implementation plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use -> superpowers:subagent-driven-development (recommended) or -> superpowers:executing-plans to implement this plan task-by-task. Steps -> use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** One sentence — what shipping this plan achieves. No design -rationale; link to the spec for that. - -**Spec:** [`design.md`](./design.md) - -**Branch:** `feat/my-change` (or `fix/`, `chore/`, etc.) - -**Commit strategy:** Per-task commits / single commit / squash on merge. -Whichever fits. - ---- - -### Task 1: - -**Files:** -- Modify: `path/to/file.py` -- Create: `path/to/new.py` - -One sentence on what this task accomplishes. No deeper reasoning — that's -in the spec. - -- [ ] **Step 1: ** - - Run / edit / verify command. Expected output. - -- [ ] **Step 2: ** - - ... - -- [ ] **Step 3: Commit** - - ```bash - git add path/to/file.py - git commit -m ": " - ``` - ---- - -### Task 2: ... diff --git a/planning/changes/2026-06-26.01-nested-gitignore/design.md b/planning/changes/2026-06-26.01-nested-gitignore.md similarity index 100% rename from planning/changes/2026-06-26.01-nested-gitignore/design.md rename to planning/changes/2026-06-26.01-nested-gitignore.md diff --git a/planning/changes/2026-06-26.01-nested-gitignore/plan.md b/planning/changes/2026-06-26.01-nested-gitignore/plan.md deleted file mode 100644 index a5b9803..0000000 --- a/planning/changes/2026-06-26.01-nested-gitignore/plan.md +++ /dev/null @@ -1,433 +0,0 @@ -# Nested `.gitignore` Support Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Make the file walk honor nested per-directory `.gitignore` files (not just the root) and expose the extra skip dirs through a repeatable `--exclude` flag. - -**Architecture:** A new `eof_fixer/discovery.py` module owns "which files to visit": a top-down DFS that carries a stack of `(anchor_dir, GitIgnoreSpec)`, evaluates each entry deepest-spec-first (first definitive `check_file(...).include` wins), prunes ignored directories, hard-skips `.git`, and skips symlinks. `main()` keeps the unchanged EOF core (`_is_binary`, `_detect_trailing`, `_fix_file`) and just consumes the new iterator. - -**Tech Stack:** Python 3.10+, `pathspec.GitIgnoreSpec` (existing dependency — no new deps), pytest. - -## Global Constraints - -- Python `>=3.10,<4`; code must run on 3.10–3.14 (no 3.11+-only syntax). -- Runtime dependencies: **`pathspec` only** — add no new dependency. -- All imports at module level, never inside function bodies. -- Type-checker suppressions use `ty: ignore`, never `# type: ignore`. -- No `print()` in source — emit via `sys.stdout.write(...)` with an explicit `\n` (see `planning/decisions/2026-06-26-no-print-in-source.md`). -- Annotate test function arguments (e.g. `tmp_path: pathlib.Path`). -- ruff line length 120; `just lint` must be clean. -- Conventional-commit messages, each ending with the trailer: - `Co-Authored-By: Claude Opus 4.8 (1M context) ` -- This is a Full-lane planning change: the implementing PR hand-edits the matching `architecture/*.md` files (Task 3). - ---- - -### Task 1: `discovery.py` — the nested-gitignore walk - -**Files:** -- Create: `eof_fixer/discovery.py` -- Test: `tests/test_discovery.py` - -**Interfaces:** -- Consumes: `pathspec.GitIgnoreSpec` (`.from_lines`, `.check_file(path).include` → `True`/`False`/`None`). For directory entries, the path passed to `check_file` MUST carry a trailing `/`. -- Produces: `iter_text_files(root: pathlib.Path, extra_excludes: Sequence[str]) -> Iterator[pathlib.Path]`, yielding paths **relative to `root`** (posix `/` separators) for files not ignored. - -- [ ] **Step 1: Write the failing tests** - -```python -# tests/test_discovery.py -import pathlib - -import pytest - -from eof_fixer.discovery import iter_text_files - - -def _walk(root: pathlib.Path, extra_excludes: list[str] | None = None) -> set[str]: - return {str(p) for p in iter_text_files(root, extra_excludes or [])} - - -def test_root_gitignore_excludes_matching_files(tmp_path: pathlib.Path) -> None: - (tmp_path / "keep.txt").write_text("x") - (tmp_path / "skip.tmp").write_text("x") - (tmp_path / ".gitignore").write_text("*.tmp\n") - result = _walk(tmp_path) - assert "keep.txt" in result - assert "skip.tmp" not in result - assert ".gitignore" in result - - -def test_extra_excludes_are_skipped(tmp_path: pathlib.Path) -> None: - (tmp_path / "keep.txt").write_text("x") - cache = tmp_path / ".cache" - cache.mkdir() - (cache / "blob").write_text("x") - result = _walk(tmp_path, [".cache", ".uv-cache"]) - assert "keep.txt" in result - assert not any(name.startswith(".cache") for name in result) - - -def test_nested_gitignore_applies_to_its_subtree(tmp_path: pathlib.Path) -> None: - sub = tmp_path / "sub" - sub.mkdir() - (sub / ".gitignore").write_text("*.log\n") - (sub / "a.log").write_text("x") - (sub / "a.txt").write_text("x") - (tmp_path / "top.log").write_text("x") # not under sub → not ignored - result = _walk(tmp_path) - assert "sub/a.txt" in result - assert "sub/a.log" not in result - assert "top.log" in result - - -def test_deeper_gitignore_overrides_shallower(tmp_path: pathlib.Path) -> None: - (tmp_path / ".gitignore").write_text("*.log\n") - sub = tmp_path / "sub" - sub.mkdir() - (sub / ".gitignore").write_text("!important.log\n") - (sub / "important.log").write_text("x") - (sub / "other.log").write_text("x") - result = _walk(tmp_path) - assert "sub/important.log" in result - assert "sub/other.log" not in result - - -def test_negation_reincludes_file(tmp_path: pathlib.Path) -> None: - (tmp_path / ".gitignore").write_text("*.log\n!keep.log\n") - (tmp_path / "a.log").write_text("x") - (tmp_path / "keep.log").write_text("x") - result = _walk(tmp_path) - assert "a.log" not in result - assert "keep.log" in result - - -def test_ignored_directory_is_pruned(tmp_path: pathlib.Path) -> None: - (tmp_path / ".gitignore").write_text("build/\n") - build = tmp_path / "build" - build.mkdir() - (build / "out.txt").write_text("x") - (build / ".gitignore").write_text("!out.txt\n") # cannot re-include under ignored dir - result = _walk(tmp_path) - assert not any(name.startswith("build") for name in result) - - -def test_git_directory_is_always_skipped(tmp_path: pathlib.Path) -> None: - git_dir = tmp_path / ".git" - git_dir.mkdir() - (git_dir / "config").write_text("x") - (tmp_path / "keep.txt").write_text("x") - result = _walk(tmp_path) - assert "keep.txt" in result - assert not any(name.startswith(".git/") for name in result) - - -def test_symlinks_are_skipped(tmp_path: pathlib.Path) -> None: - target = tmp_path / "target.txt" - target.write_text("x") - link = tmp_path / "link.txt" - try: - link.symlink_to(target) - except (OSError, NotImplementedError): # pragma: no cover - pytest.skip("symlinks not available on this platform") - result = _walk(tmp_path) - assert "target.txt" in result - assert "link.txt" not in result -``` - -- [ ] **Step 2: Run the tests to verify they fail** - -Run: `just test tests/test_discovery.py` -Expected: FAIL — `ModuleNotFoundError: No module named 'eof_fixer.discovery'`. - -- [ ] **Step 3: Implement `eof_fixer/discovery.py`** - -```python -import os -import pathlib -from collections.abc import Iterator, Sequence - -from pathspec import GitIgnoreSpec - -GITIGNORE_NAME = ".gitignore" -ALWAYS_PRUNE = ".git" - - -def _load_spec(directory: pathlib.Path) -> GitIgnoreSpec | None: - """Parse a directory's .gitignore into a spec, or None if it has none.""" - gitignore = directory / GITIGNORE_NAME - if not gitignore.is_file(): - return None - return GitIgnoreSpec.from_lines(gitignore.read_text(encoding="utf-8").splitlines()) - - -def _is_ignored(rel: str, is_dir: bool, stack: list[tuple[str, GitIgnoreSpec]]) -> bool: - """Evaluate specs deepest-first; first definitive verdict wins (git precedence).""" - suffix = "/" if is_dir else "" - for anchor, spec in reversed(stack): - subpath = rel if not anchor else rel[len(anchor) + 1 :] - result = spec.check_file(subpath + suffix) - if result.include is not None: - return result.include - return False - - -def _walk( - directory: pathlib.Path, - rel_dir: str, - stack: list[tuple[str, GitIgnoreSpec]], -) -> Iterator[pathlib.Path]: - """Depth-first walk, pushing each directory's .gitignore onto the spec stack.""" - spec = _load_spec(directory) - if spec is not None: - stack.append((rel_dir, spec)) - try: - with os.scandir(directory) as scan: - entries = sorted(scan, key=lambda entry: entry.name) - for entry in entries: - if entry.name == ALWAYS_PRUNE or entry.is_symlink(): - continue - is_dir = entry.is_dir() - rel = entry.name if not rel_dir else f"{rel_dir}/{entry.name}" - if _is_ignored(rel, is_dir, stack): - continue - if is_dir: - yield from _walk(pathlib.Path(entry.path), rel, stack) - else: - yield pathlib.Path(rel) - finally: - if spec is not None: - stack.pop() - - -def iter_text_files(root: pathlib.Path, extra_excludes: Sequence[str]) -> Iterator[pathlib.Path]: - """Yield paths (relative to root) of files not ignored by nested .gitignore files. - - `.git` is always skipped. `extra_excludes` (e.g. ['.cache', '.uv-cache']) form a - baseline gitignore spec anchored at root, applied beneath every .gitignore file. - """ - baseline = GitIgnoreSpec.from_lines(list(extra_excludes)) - stack: list[tuple[str, GitIgnoreSpec]] = [("", baseline)] - yield from _walk(root, "", stack) -``` - -- [ ] **Step 4: Run the tests to verify they pass** - -Run: `just test tests/test_discovery.py` -Expected: PASS — all 8 tests green. - -- [ ] **Step 5: Lint** - -Run: `just lint` -Expected: ruff format/check and ty all clean. - -- [ ] **Step 6: Commit** - -```bash -git add eof_fixer/discovery.py tests/test_discovery.py -git commit -m "feat: add nested .gitignore-aware file discovery - -Co-Authored-By: Claude Opus 4.8 (1M context) " -``` - ---- - -### Task 2: Wire discovery into `main()` and add `--exclude` - -**Files:** -- Modify: `eof_fixer/main.py` (replace the gitignore-loading block and the walk in `main()`; drop the `pathspec` import) -- Test: `tests/test_end_of_file_fixer.py` (existing suite must stay green; add one `--exclude` test) - -**Interfaces:** -- Consumes: `iter_text_files(root, extra_excludes)` from Task 1. -- Produces: no new public symbols. CLI gains `--exclude DIR` (repeatable, `action="append"`). Extra-exclude set is `[".cache", ".uv-cache", *args.exclude]`; `.git` is skipped by discovery and is not configurable. - -- [ ] **Step 1: Write the failing test** - -Add to `tests/test_end_of_file_fixer.py` (it already imports `main`, `Path`, `os`, `sys`, `StringIO`, `tempfile`, `shutil`; reuse the existing `_run_main_in` helper used by other tests): - -```python -def test_exclude_flag_augments_default_skips() -> None: - with tempfile.TemporaryDirectory() as temp_dir: - temp_path = Path(temp_dir) - (temp_path / "keep.txt").write_bytes(b"no newline") - vendor = temp_path / "vendor" - vendor.mkdir() - (vendor / "lib.txt").write_bytes(b"no newline") - - with _run_main_in(temp_path, ["eof-fixer", ".", "--exclude", "vendor"]) as (stdout, _stderr): - result = main() - - output = stdout.getvalue() - assert result == 1 - assert "Fixing keep.txt\n" in output - assert "Fixing vendor/lib.txt\n" not in output - # vendor file left untouched because it was excluded - assert (vendor / "lib.txt").read_bytes() == b"no newline" -``` - -- [ ] **Step 2: Run the test to verify it fails** - -Run: `just test tests/test_end_of_file_fixer.py::test_exclude_flag_augments_default_skips` -Expected: FAIL — `--exclude` is an unrecognized argument (argparse SystemExit), so `main()` never runs the assertions. - -- [ ] **Step 3: Edit `main()`** - -In `eof_fixer/main.py`, remove `import pathspec` (keep `import os`, used by the EOF core). Add the discovery import at module level: - -```python -from eof_fixer.discovery import iter_text_files -``` - -Replace the body of `main()` from the argument parser through the end with: - -```python -def main() -> int: - parser = argparse.ArgumentParser() - parser.add_argument("path", help="path to directory", type=pathlib.Path) - parser.add_argument("--check", action="store_true") - parser.add_argument( - "--exclude", - action="append", - default=None, - metavar="DIR", - help="extra file/directory name to skip, in addition to .git, .cache, .uv-cache (repeatable)", - ) - args = parser.parse_args() - - path: pathlib.Path = args.path - check: bool = args.check - - if not path.is_dir(): - parser.error(f"path is not a directory: {path}") - - extra_excludes = [".cache", ".uv-cache", *(args.exclude or [])] - open_mode = "rb" if check else "rb+" - - retv = 0 - for relative_path in iter_text_files(path, extra_excludes): - with (path / relative_path).open(open_mode) as f: - ret_for_file = _fix_file(f, check=check) - if ret_for_file: - sys.stdout.write(f"Fixing {relative_path}\n") - retv |= ret_for_file - - return retv -``` - -(The hardcoded `ignore_patterns` list, the `.gitignore` file read, the `GitIgnoreSpec` construction, and the `match_tree_files` loop are all deleted — discovery now owns them.) - -- [ ] **Step 4: Run the new test and the full suite** - -Run: `just test` -Expected: PASS — the new `--exclude` test and every existing test (including `test_end_of_file_fixer_with_gitignore`, `test_symlink_in_tree_does_not_crash`, the CRLF/CR cases, and the path/readonly/cwd tests) are green. Coverage stays at the configured threshold. - -- [ ] **Step 5: Lint** - -Run: `just lint` -Expected: clean. Confirm `import pathspec` is gone from `main.py` (it now lives only in `discovery.py`). - -- [ ] **Step 6: Commit** - -```bash -git add eof_fixer/main.py tests/test_end_of_file_fixer.py -git commit -m "feat: respect nested .gitignore and add --exclude flag - -Co-Authored-By: Claude Opus 4.8 (1M context) " -``` - ---- - -### Task 3: Promote to architecture + record the decision - -**Files:** -- Modify: `architecture/file-discovery.md` -- Modify: `architecture/cli.md` -- Create: `planning/decisions/2026-06-26-build-on-pathspec-for-nested-gitignore.md` -- Modify: `planning/changes/2026-06-26.01-nested-gitignore/design.md` (finalize `summary`) - -**Interfaces:** Documentation only — no code, no new symbols. - -- [ ] **Step 1: Update `architecture/file-discovery.md`** - -Replace the "Ignore rules" and "Path resolution" sections so they describe the nested walk. The new prose must state: -- Discovery lives in `eof_fixer/discovery.py` (`iter_text_files`); `main()` consumes it. -- The walk is a top-down DFS carrying a stack of per-directory `GitIgnoreSpec`s; an entry is classified by evaluating specs deepest-first, first definitive `check_file().include` wins (git precedence: closest file wins, negation re-includes). -- `.git` is always pruned by name; symlinks are skipped; ignored directories are pruned (git-correct — a file under an excluded parent cannot be re-included). -- The baseline excludes (`.cache`, `.uv-cache`, plus any `--exclude` values) form a root-anchored spec beneath every `.gitignore`. -- Still pure-filesystem: no `.git/info/exclude`, no global `core.excludesFile`, no git invocation; works on any directory. - -- [ ] **Step 2: Update `architecture/cli.md`** - -Under the interface description, document `--exclude DIR` (repeatable): adds extra file/directory names to skip on top of the always-skipped `.git` and the default `.cache`/`.uv-cache`. Note `.git` is not configurable. - -- [ ] **Step 3: Create the decision record** - -```markdown ---- -status: accepted -summary: Build nested .gitignore resolution on the existing pathspec dependency rather than adopt a dedicated library. ---- - -# Build nested .gitignore on pathspec, not a dedicated library - -**Decision:** Implement nested `.gitignore` resolution as a small walk over -`pathspec.GitIgnoreSpec`, rather than adding a nested-gitignore library -(e.g. `igittigitt`). - -## Context - -Honoring nested `.gitignore` needs per-directory rule collection plus correct -precedence. Dedicated libraries (`igittigitt`, `gitignorefile`, -`gitignore_parser`) offer recursive collection out of the box. `pathspec` is -already the sole runtime dependency and matches a single flat spec only. - -## Decision & rationale - -`pathspec` is the most actively maintained option (2026 releases vs. 2024/2022 -for the alternatives) and its matching engine most rigorously replicates git, -including edge cases the alternatives get wrong — `igittigitt`'s own README -documents a cross-directory negation case that fails. A dedicated library would -add a second, less-maintained pattern engine and pull in the global git ignore -by default, in exchange for saving only ~30 lines of directory-collection code -that we cover with a small, well-tested walk. Keeping one engine preserves the -tool's minimal-dependency footprint and its best-available correctness. - -## Revisit trigger - -Reopen if the per-directory walk accumulates real complexity (e.g. we need -`.git/info/exclude`, global excludes, or true "tracked-files-win" semantics), -at which point shelling out to git or adopting a maintained library may win. -``` - -- [ ] **Step 4: Finalize the bundle summary** - -In `planning/changes/2026-06-26.01-nested-gitignore/design.md`, update the frontmatter `summary:` from the intent line to the realized result, e.g.: -`summary: Nested .gitignore files are now respected tree-wide (DFS over per-directory pathspec specs) and a repeatable --exclude flag augments the default skip dirs.` - -- [ ] **Step 5: Validate planning + lint** - -Run: `just check-planning` -Expected: `planning: OK` (decision name/frontmatter valid, bundle shape valid). -Run: `just lint-ci` -Expected: clean, including `check-planning`. - -- [ ] **Step 6: Commit** - -```bash -git add architecture/file-discovery.md architecture/cli.md \ - planning/decisions/2026-06-26-build-on-pathspec-for-nested-gitignore.md \ - planning/changes/2026-06-26.01-nested-gitignore/design.md -git commit -m "docs: promote nested .gitignore to architecture + record decision - -Co-Authored-By: Claude Opus 4.8 (1M context) " -``` - ---- - -## Done criteria - -- `just test` green on 3.10–3.14; `tests/test_discovery.py` covers nested precedence, negation, directory pruning, `.git` skip, symlink skip, and extra-excludes. -- `eof-fixer` honors `.gitignore` files in subdirectories; `--exclude` augments the default skip set; `.git`/`.cache`/`.uv-cache` still skipped by default. -- No new runtime dependency (pathspec only). -- `architecture/file-discovery.md` + `architecture/cli.md` describe the new behavior; decision recorded; `just check-planning` → `planning: OK`. diff --git a/planning/changes/2026-06-26.02-deepen-fixer/design.md b/planning/changes/2026-06-26.02-deepen-fixer.md similarity index 100% rename from planning/changes/2026-06-26.02-deepen-fixer/design.md rename to planning/changes/2026-06-26.02-deepen-fixer.md diff --git a/planning/changes/2026-06-26.02-deepen-fixer/plan.md b/planning/changes/2026-06-26.02-deepen-fixer/plan.md deleted file mode 100644 index cfe8d9d..0000000 --- a/planning/changes/2026-06-26.02-deepen-fixer/plan.md +++ /dev/null @@ -1,508 +0,0 @@ -# Deepen the Fix into `fixer.py` — Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Extract the EOF-fixing capability into a deep module `eof_fixer/fixer.py` (`fix_file` + `fix_directory`) so `main()` becomes a thin CLI adapter and tests target the interface instead of the `sys.argv`/`sys.stdout`/`os.chdir` harness. - -**Architecture:** `fixer.py` owns the file-level helpers (`_is_binary`, `_detect_trailing`), a public `fix_file(file_obj, *, check) -> bool`, and `fix_directory(root, *, check=False, extra_excludes=()) -> list[pathlib.Path]` that drives `discovery.iter_text_files`. `main.py` shrinks to argparse + `is_dir` guard + render + exit. No behavior change except output timing (streaming → batch). - -**Tech Stack:** Python 3.10+, `pathspec` (via `discovery`), pytest + pytest-cov (100% enforced). - -## Global Constraints - -- Python `>=3.10,<4`; no 3.11+-only syntax. -- Runtime dependencies: `pathspec` only — add none. -- All imports at module level, never inside function bodies (including test files). -- Type-checker suppressions use `ty: ignore`, never `# type: ignore`. -- No `print()` in source — output via `sys.stdout.write(...)` with explicit `\n`. -- Annotate test function arguments (e.g. `tmp_path: pathlib.Path`). -- ruff line length 120. -- 100% test coverage is enforced by `just test`; every branch of new code must be covered. -- Conventional-commit messages ending with the trailer: - `Co-Authored-By: Claude Opus 4.8 (1M context) ` -- Full-lane planning change: the implementing work hand-edits the matching `architecture/*.md` (Task 3). - ---- - -### Task 1: Create `fixer.py` and rewire `main()` to an adapter - -**Files:** -- Create: `eof_fixer/fixer.py` -- Create: `tests/test_fixer.py` -- Modify: `eof_fixer/main.py` (remove `_is_binary`/`_detect_trailing`/`_fix_file`; `main()` calls `fix_directory`) - -**Interfaces:** -- Consumes: `eof_fixer.discovery.iter_text_files(root, extra_excludes)`. -- Produces: - - `fix_file(file_obj: IO[bytes], *, check: bool) -> bool` — True if the file was (or, under `check`, would be) fixed. - - `fix_directory(root: pathlib.Path, *, check: bool = False, extra_excludes: Sequence[str] = ()) -> list[pathlib.Path]` — relative paths fixed/would-be-fixed, in walk order. - - `DEFAULT_EXCLUDES = (".cache", ".uv-cache")`. - -- [ ] **Step 1: Write the failing tests** (`tests/test_fixer.py`) - -```python -import io -import pathlib -import stat - -import pytest - -from eof_fixer.fixer import fix_directory, fix_file - - -# ---- content layer: fix_file on BytesIO (no filesystem) ---- - -def _run(content: bytes, *, check: bool = False) -> tuple[bool, bytes]: - buffer = io.BytesIO(content) - changed = fix_file(buffer, check=check) - return changed, buffer.getvalue() - - -def test_no_trailing_newline_appends_lf() -> None: - assert _run(b"abc") == (True, b"abc\n") - - -def test_single_newline_unchanged() -> None: - assert _run(b"abc\n") == (False, b"abc\n") - - -def test_multiple_newlines_truncated_to_one() -> None: - assert _run(b"abc\n\n\n") == (True, b"abc\n") - - -def test_newlines_only_truncated_to_empty() -> None: - assert _run(b"\n\n") == (True, b"") - - -def test_empty_unchanged() -> None: - assert _run(b"") == (False, b"") - - -def test_crlf_no_trailing_appends_lf() -> None: - assert _run(b"a\r\nb") == (True, b"a\r\nb\n") - - -def test_crlf_perfect_unchanged() -> None: - assert _run(b"a\r\nb\r\n") == (False, b"a\r\nb\r\n") - - -def test_crlf_multiple_truncated_to_one() -> None: - assert _run(b"a\r\nb\r\n\r\n") == (True, b"a\r\nb\r\n") - - -def test_cr_only_multiple_truncated_to_one() -> None: - assert _run(b"a\rb\r\r") == (True, b"a\rb\r") - - -def test_bom_prefixed_is_treated_as_text() -> None: - assert _run(b"\xef\xbb\xbfabc") == (True, b"\xef\xbb\xbfabc\n") - - -def test_null_byte_in_first_1024_is_binary_skipped() -> None: - assert _run(b"a\x00b") == (False, b"a\x00b") - - -def test_null_byte_beyond_first_1024_is_treated_as_text() -> None: - content = b"a" * 1100 + b"\x00data" - changed, result = _run(content) - assert changed is True - assert result == content + b"\n" - - -def test_check_mode_returns_true_but_does_not_write() -> None: - assert _run(b"abc", check=True) == (True, b"abc") - - -# ---- orchestration layer: fix_directory on a temp tree ---- - -def _names(paths: list[pathlib.Path]) -> set[str]: - return {str(p) for p in paths} - - -def test_fixes_files_and_returns_list(tmp_path: pathlib.Path) -> None: - (tmp_path / "a.txt").write_bytes(b"no nl") - (tmp_path / "b.txt").write_bytes(b"ok\n") - fixed = fix_directory(tmp_path) - assert _names(fixed) == {"a.txt"} - assert (tmp_path / "a.txt").read_bytes() == b"no nl\n" - assert (tmp_path / "b.txt").read_bytes() == b"ok\n" - - -def test_check_mode_writes_nothing(tmp_path: pathlib.Path) -> None: - (tmp_path / "a.txt").write_bytes(b"no nl") - fixed = fix_directory(tmp_path, check=True) - assert _names(fixed) == {"a.txt"} - assert (tmp_path / "a.txt").read_bytes() == b"no nl" - - -def test_respects_nested_gitignore(tmp_path: pathlib.Path) -> None: - sub = tmp_path / "sub" - sub.mkdir() - (sub / ".gitignore").write_bytes(b"*.log\n") - (sub / "x.log").write_bytes(b"no nl") - (sub / "x.txt").write_bytes(b"no nl") - fixed = _names(fix_directory(tmp_path)) - assert "sub/x.txt" in fixed - assert "sub/x.log" not in fixed - - -def test_skips_binary(tmp_path: pathlib.Path) -> None: - (tmp_path / "bin").write_bytes(b"a\x00b") - assert fix_directory(tmp_path) == [] - assert (tmp_path / "bin").read_bytes() == b"a\x00b" - - -def test_default_excludes_cache_dirs(tmp_path: pathlib.Path) -> None: - cache = tmp_path / ".cache" - cache.mkdir() - (cache / "blob").write_bytes(b"no nl") - (tmp_path / "keep.txt").write_bytes(b"no nl") - fixed = _names(fix_directory(tmp_path)) - assert "keep.txt" in fixed - assert not any(name.startswith(".cache") for name in fixed) - - -def test_extra_excludes_augment_defaults(tmp_path: pathlib.Path) -> None: - vendor = tmp_path / "vendor" - vendor.mkdir() - (vendor / "lib.txt").write_bytes(b"no nl") - (tmp_path / "keep.txt").write_bytes(b"no nl") - fixed = _names(fix_directory(tmp_path, extra_excludes=["vendor"])) - assert "keep.txt" in fixed - assert not any(name.startswith("vendor") for name in fixed) - - -def test_symlink_does_not_crash(tmp_path: pathlib.Path) -> None: - target = tmp_path / "t.txt" - target.write_bytes(b"no nl") - link = tmp_path / "l.txt" - try: - link.symlink_to(target) - except (OSError, NotImplementedError): # pragma: no cover - pytest.skip("symlinks not available on this platform") - fixed = _names(fix_directory(tmp_path)) - assert "t.txt" in fixed - assert "l.txt" not in fixed - - -def test_runs_with_absolute_path_independent_of_cwd(tmp_path: pathlib.Path) -> None: - (tmp_path / "a.txt").write_bytes(b"no nl") - assert _names(fix_directory(tmp_path.resolve())) == {"a.txt"} - - -def test_check_mode_opens_readonly_file_without_error(tmp_path: pathlib.Path) -> None: - readonly = tmp_path / "ro.txt" - readonly.write_bytes(b"no nl") - readonly.chmod(stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH) - try: - fixed = _names(fix_directory(tmp_path, check=True)) - assert "ro.txt" in fixed - assert readonly.read_bytes() == b"no nl" - finally: - readonly.chmod(stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH) -``` - -- [ ] **Step 2: Run the new tests to verify they fail** - -Run: `just test tests/test_fixer.py` -Expected: FAIL — `ModuleNotFoundError: No module named 'eof_fixer.fixer'`. - -- [ ] **Step 3: Create `eof_fixer/fixer.py`** - -```python -import os -import pathlib -from collections.abc import Iterator, Sequence -from typing import IO - -from eof_fixer.discovery import iter_text_files - -DEFAULT_EXCLUDES = (".cache", ".uv-cache") -_BINARY_SAMPLE_SIZE = 1024 - - -def _is_binary(file_obj: IO[bytes]) -> bool: - current_pos = file_obj.tell() - file_obj.seek(0) - sample = file_obj.read(_BINARY_SAMPLE_SIZE) - file_obj.seek(current_pos) - return b"\x00" in sample - - -def _detect_trailing(file_obj: IO[bytes]) -> tuple[str, int]: - """Inspect the end of `file_obj` and return the action needed. - - Returns one of: - - ("none", 0) — file is empty, or already ends with exactly one terminator. - - ("append_lf", 0) — file lacks a trailing newline; an LF should be appended. - - ("truncate", offset) — file has excess trailing newlines; truncate to `offset` - (0 means truncate to empty). - """ - try: - file_obj.seek(-1, os.SEEK_END) - except OSError: - return ("none", 0) - - last_character = file_obj.read(1) - if last_character not in {b"\n", b"\r"}: - return ("append_lf", 0) - - while last_character in {b"\n", b"\r"}: - if file_obj.tell() == 1: - # All bytes are line terminators — truncate to empty. - return ("truncate", 0) - file_obj.seek(-2, os.SEEK_CUR) - last_character = file_obj.read(1) - - position = file_obj.tell() - remaining = file_obj.read() - for sequence in (b"\n", b"\r\n", b"\r"): - if remaining == sequence: - return ("none", 0) - if remaining.startswith(sequence): - return ("truncate", position + len(sequence)) - - return ("none", 0) # pragma: no cover - - -def fix_file(file_obj: IO[bytes], *, check: bool) -> bool: - """Normalize one open binary file to end with exactly one terminator. - - Returns True if the file was fixed (or, under `check`, would be fixed). - Binary files (null byte in the first sample) are skipped, returning False. - """ - if _is_binary(file_obj): - return False - - action, offset = _detect_trailing(file_obj) - if action == "none": - return False - - if not check: - if action == "append_lf": - # Needs this seek for windows, otherwise IOError - file_obj.seek(0, os.SEEK_END) - file_obj.write(b"\n") - else: # action == "truncate" - file_obj.seek(offset) - file_obj.truncate() - - return True - - -def fix_directory( - root: pathlib.Path, - *, - check: bool = False, - extra_excludes: Sequence[str] = (), -) -> list[pathlib.Path]: - """Fix every non-ignored text file under `root`; return the relative paths fixed. - - `.git` and `DEFAULT_EXCLUDES` (.cache/.uv-cache) are always skipped; - `extra_excludes` adds more skip names on top. Under `check`, nothing is - written but the would-be-fixed paths are still returned. - """ - excludes = [*DEFAULT_EXCLUDES, *extra_excludes] - open_mode = "rb" if check else "rb+" - fixed: list[pathlib.Path] = [] - paths: Iterator[pathlib.Path] = iter_text_files(root, excludes) - for relative_path in paths: - with (root / relative_path).open(open_mode) as file_obj: - if fix_file(file_obj, check=check): - fixed.append(relative_path) - return fixed -``` - -- [ ] **Step 4: Run the new tests to verify they pass** - -Run: `just test tests/test_fixer.py` -Expected: PASS — all content + orchestration tests green. - -- [ ] **Step 5: Rewire `eof_fixer/main.py` to a thin adapter** - -Replace the entire file with: - -```python -import argparse -import pathlib -import sys - -from eof_fixer.fixer import fix_directory - - -def main() -> int: - parser = argparse.ArgumentParser() - parser.add_argument("path", help="path to directory", type=pathlib.Path) - parser.add_argument("--check", action="store_true") - parser.add_argument( - "--exclude", - action="append", - default=None, - metavar="DIR", - help="extra file/directory name to skip, in addition to .git, .cache, .uv-cache (repeatable)", - ) - args = parser.parse_args() - - path: pathlib.Path = args.path - if not path.is_dir(): - parser.error(f"path is not a directory: {path}") - - fixed = fix_directory(path, check=args.check, extra_excludes=args.exclude or []) - for relative_path in fixed: - sys.stdout.write(f"Fixing {relative_path}\n") - return 1 if fixed else 0 -``` - -(`_is_binary`, `_detect_trailing`, `_fix_file`, and the `os`/`IO` imports are gone — they live in `fixer.py` now.) - -- [ ] **Step 6: Run the full suite** - -Run: `just test` -Expected: PASS — `tests/test_fixer.py` plus the still-present `tests/test_end_of_file_fixer.py` (which drives `main()`); the `Fixing` lines are now batch-rendered but the substring/order assertions still hold. Coverage 100%. - -- [ ] **Step 7: Lint** - -Run: `just lint` (format as you go), then confirm a clean `just lint-ci` against the committed tree. - -- [ ] **Step 8: Commit** - -```bash -git add eof_fixer/fixer.py eof_fixer/main.py tests/test_fixer.py -git commit -m "refactor: deepen fix into fixer.py; main() becomes an adapter - -Co-Authored-By: Claude Opus 4.8 (1M context) " -``` - ---- - -### Task 2: Migrate `test_end_of_file_fixer.py` to adapter-only tests - -**Files:** -- Modify: `tests/test_end_of_file_fixer.py` - -**Interfaces:** Test-only. After Task 1, the behaviors in this file are now covered at the `fixer` layer by `tests/test_fixer.py`; this task removes the redundant process-harness tests, keeping only what genuinely exercises the `main()` adapter. - -- [ ] **Step 1: Delete the now-redundant process-harness tests** - -Remove these functions from `tests/test_end_of_file_fixer.py` (each is covered by `tests/test_fixer.py` as noted): -- `test_end_of_file_fixer_command_with_check_false` → `test_fixes_files_and_returns_list` -- `test_end_of_file_fixer_command_with_check_true` → `test_check_mode_writes_nothing` -- `test_end_of_file_fixer_with_gitignore` → `test_respects_nested_gitignore` -- `test_end_of_file_fixer_skips_binary_files` → `test_skips_binary` -- `test_crlf_no_trailing_newline_appends_lf` → `test_crlf_no_trailing_appends_lf` -- `test_crlf_perfect_unchanged` → `test_crlf_perfect_unchanged` -- `test_crlf_multiple_trailing_truncated_to_one` → `test_crlf_multiple_truncated_to_one` -- `test_cr_only_multiple_trailing_truncated_to_one` → `test_cr_only_multiple_truncated_to_one` -- `test_bom_prefixed_file_is_treated_as_text` → `test_bom_prefixed_is_treated_as_text` -- `test_null_byte_beyond_first_1024_bytes_is_treated_as_text` → `test_null_byte_beyond_first_1024_is_treated_as_text` -- `test_symlink_in_tree_does_not_crash` → `test_symlink_does_not_crash` -- `test_readonly_file_in_check_mode_does_not_raise` → `test_check_mode_opens_readonly_file_without_error` -- `test_exclude_flag_augments_default_skips` → `test_extra_excludes_augment_defaults` -- `test_runs_from_unrelated_cwd_with_absolute_path` → `test_runs_with_absolute_path_independent_of_cwd` - -Keep `test_path_arg_rejects_file` and `test_path_arg_rejects_nonexistent_path` (they exercise the argparse `is_dir` guard → exit 2, which only `main()` does). - -- [ ] **Step 2: Add two adapter tests that exercise `main()`'s rendering** - -Keep the existing `_run_main_in` helper (the kept + new tests use it). Add: - -```python -def test_main_renders_fixing_lines_and_exit_code() -> None: - with tempfile.TemporaryDirectory() as temp_dir: - temp_path = Path(temp_dir) - (temp_path / "a.txt").write_bytes(b"no nl") - skipped = temp_path / "vendor" - skipped.mkdir() - (skipped / "lib.txt").write_bytes(b"no nl") - - with _run_main_in(temp_path, ["eof-fixer", ".", "--exclude", "vendor"]) as (stdout, _stderr): - result = main() - - output = stdout.getvalue() - assert result == 1 - assert "Fixing a.txt\n" in output - assert "Fixing vendor/lib.txt\n" not in output - - -def test_main_returns_zero_when_nothing_to_fix() -> None: - with tempfile.TemporaryDirectory() as temp_dir: - temp_path = Path(temp_dir) - (temp_path / "ok.txt").write_bytes(b"already fine\n") - - with _run_main_in(temp_path, ["eof-fixer", "."]) as (stdout, _stderr): - result = main() - - assert result == 0 - assert stdout.getvalue() == "" -``` - -- [ ] **Step 3: Prune now-unused imports** - -After deletion, remove any imports left unused in `tests/test_end_of_file_fixer.py` (e.g. `shutil`, `stat`, `StringIO` if no longer referenced). Let `just lint` (ruff `F401`) tell you which; remove exactly those. - -- [ ] **Step 4: Run the full suite + coverage** - -Run: `just test` -Expected: PASS, coverage 100%. The adapter tests cover `main()`'s render loop, exit-0 path, `--exclude` wiring, and both `is_dir` rejections; `fixer.py` is covered by `tests/test_fixer.py`. If coverage flags an uncovered line in `main.py`, add the minimal adapter test that reaches it (do not add `# pragma: no cover` to adapter logic). - -- [ ] **Step 5: Lint** - -Run: `just lint`, then confirm `just lint-ci` clean against the committed tree. - -- [ ] **Step 6: Commit** - -```bash -git add tests/test_end_of_file_fixer.py -git commit -m "test: move fix behavior off the process harness to the fixer interface - -Co-Authored-By: Claude Opus 4.8 (1M context) " -``` - ---- - -### Task 3: Promote to architecture + finalize the bundle summary - -**Files:** -- Modify: `architecture/eof-normalization.md` -- Modify: `architecture/cli.md` -- Modify: `planning/changes/2026-06-26.02-deepen-fixer/design.md` (finalize `summary`) - -**Interfaces:** Documentation only. - -- [ ] **Step 1: Update `architecture/eof-normalization.md`** - -Read the file first. Update it so the capability is described as living in `eof_fixer/fixer.py`: the public `fix_file(file_obj, *, check) -> bool` (one open file) and `fix_directory(root, *, check, extra_excludes) -> list[Path]` (drives `discovery.iter_text_files`, owns the `DEFAULT_EXCLUDES` = `.cache`/`.uv-cache` policy, opens `rb`/`rb+` by `check`, returns the fixed relative paths). Note `main()` no longer holds the file-level logic. Preserve the existing action-model / terminator prose. - -- [ ] **Step 2: Update `architecture/cli.md`** - -Read the file first. Update it so `main()` is described as a thin adapter over `fix_directory`: it parses args, guards `is_dir` (exit 2), then renders one `Fixing ` line per returned path and exits `1`/`0`. Note the output is now batch-rendered after the walk (not streamed). The `--exclude` and exit-code contract are unchanged. - -- [ ] **Step 3: Finalize the bundle summary** - -In `planning/changes/2026-06-26.02-deepen-fixer/design.md`, change the frontmatter `summary:` to the realized result, e.g.: -`summary: Fix behavior now lives behind eof_fixer/fixer.py (fix_file + fix_directory); main() is a thin adapter and tests target the interface instead of the argv/stdout/cwd harness.` - -- [ ] **Step 4: Validate + lint** - -Run: `just check-planning` → expect `planning: OK`. -Run: `just lint-ci` → expect clean. - -- [ ] **Step 5: Commit** - -```bash -git add architecture/eof-normalization.md architecture/cli.md \ - planning/changes/2026-06-26.02-deepen-fixer/design.md -git commit -m "docs: promote the fixer deepening to architecture - -Co-Authored-By: Claude Opus 4.8 (1M context) " -``` - ---- - -## Done criteria - -- `eof_fixer/fixer.py` exposes `fix_file` + `fix_directory`; `main.py` is a thin adapter with no file-level logic. -- Behavior unchanged except streaming → batch output; existing exit codes, `Fixing` lines, gitignore/`--exclude`/binary/symlink behavior preserved. -- Tests target the `fixer` interface (content via `BytesIO`, orchestration via `fix_directory`); only path-rejection + two render tests go through `main()`. `_run_main_in` serves just those. -- `just test` green, coverage 100%; `just lint-ci` clean; `just check-planning` → `planning: OK`. -- `architecture/eof-normalization.md` + `architecture/cli.md` describe the new structure; bundle summary finalized. diff --git a/planning/changes/2026-06-26.03-type-eof-action-seam/design.md b/planning/changes/2026-06-26.03-type-eof-action-seam.md similarity index 100% rename from planning/changes/2026-06-26.03-type-eof-action-seam/design.md rename to planning/changes/2026-06-26.03-type-eof-action-seam.md diff --git a/planning/changes/2026-06-26.03-type-eof-action-seam/plan.md b/planning/changes/2026-06-26.03-type-eof-action-seam/plan.md deleted file mode 100644 index 4aace8e..0000000 --- a/planning/changes/2026-06-26.03-type-eof-action-seam/plan.md +++ /dev/null @@ -1,208 +0,0 @@ -# Type the EOF-Action Seam — Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Replace the stringly-typed EOF-action seam in `eof_fixer/fixer.py` with a sealed union of frozen dataclasses (`Noop | AppendLf | Truncate`) consumed by an exhaustive `match`. Behavior unchanged. - -**Architecture:** `_detect_trailing` returns `_EofAction` instead of `tuple[str, int]`; `fix_file` consumes it with a wildcard-free `match`. ty enforces exhaustiveness via `fix_file`'s `bool` return type (an unhandled future variant ⇒ implicit-`None` return error) — no `assert_never`, no new pragmas, 3.10-safe. - -**Tech Stack:** Python 3.10+ (`match`, PEP 604 unions, `dataclasses`), ty, pytest (100% coverage enforced). - -## Global Constraints - -- Python `>=3.10,<4`; `target-version = py310`. No 3.11+ APIs (`typing.assert_never`/`typing.Never` are out). -- Runtime dependencies: `pathspec` only — add none. -- All imports at module level (incl. tests). `ty: ignore` not `# type: ignore`. No `print()` in source. Annotate test args. ruff `select = ["ALL"]`, line length 120, `D1` ignored (docstrings optional). -- 100% coverage enforced by `just test`. -- Behavior must not change: same `fix_file`/`fix_directory` signatures and results; the public CLI is untouched. -- Conventional-commit messages ending with: - `Co-Authored-By: Claude Opus 4.8 (1M context) ` -- Full-lane change: the implementing work updates `architecture/eof-normalization.md` (Task 2). - ---- - -### Task 1: Replace the seam with a sealed union in `fixer.py` - -**Files:** -- Modify: `eof_fixer/fixer.py` (add the dataclasses + alias; retype `_detect_trailing`; rewrite `fix_file`'s consumption) -- Modify: `tests/test_fixer.py` (add one coverage test for the `Truncate`-under-`--check` branch) - -**Interfaces:** -- Internal (new, not exported): `Noop`, `AppendLf`, `Truncate(offset: int)`, `_EofAction = Noop | AppendLf | Truncate`. -- Unchanged public: `fix_file(file_obj, *, check) -> bool`, `fix_directory(...) -> list[pathlib.Path]`. - -- [ ] **Step 1: Add the coverage test for the truncate check-mode branch** - -In `tests/test_fixer.py`, add (next to the other `_run`-based content tests; it reuses the existing `_run` helper): - -```python -def test_check_mode_truncate_does_not_write() -> None: - assert _run(b"abc\n\n\n", check=True) == (True, b"abc\n\n\n") -``` - -- [ ] **Step 2: Run it — it passes on the current code** - -Run: `just test tests/test_fixer.py::test_check_mode_truncate_does_not_write` -Expected: PASS. (This is a coverage-completeness test, not a red-green test — the current single `if not check:` already takes this path; after the refactor splits that guard per case, this test keeps the `Truncate` check-branch covered.) - -- [ ] **Step 3: Refactor `eof_fixer/fixer.py`** - -Add `import dataclasses` to the import block (keep `os`, `pathlib`, `Iterator`/`Sequence`, `IO`, and the `iter_text_files` import). Replace the `_detect_trailing` definition and the `fix_file` body as follows; leave `_is_binary`, `DEFAULT_EXCLUDES`, `_BINARY_SAMPLE_SIZE`, and `fix_directory` exactly as they are. - -Add the action types just below the constants: - -```python -@dataclasses.dataclass(frozen=True) -class Noop: - """No change: the file is empty or already ends with exactly one terminator.""" - - -@dataclasses.dataclass(frozen=True) -class AppendLf: - """The file lacks a trailing newline; append a single LF.""" - - -@dataclasses.dataclass(frozen=True) -class Truncate: - """The file has excess trailing newlines; truncate to `offset` (0 = empty).""" - - offset: int - - -_EofAction = Noop | AppendLf | Truncate -``` - -Replace `_detect_trailing` with: - -```python -def _detect_trailing(file_obj: IO[bytes]) -> _EofAction: - """Inspect the end of `file_obj` and return the action needed.""" - try: - file_obj.seek(-1, os.SEEK_END) - except OSError: - return Noop() - - last_character = file_obj.read(1) - if not last_character: - return Noop() - if last_character not in {b"\n", b"\r"}: - return AppendLf() - - while last_character in {b"\n", b"\r"}: - if file_obj.tell() == 1: - # All bytes are line terminators — truncate to empty. - return Truncate(0) - file_obj.seek(-2, os.SEEK_CUR) - last_character = file_obj.read(1) - - position = file_obj.tell() - remaining = file_obj.read() - for sequence in (b"\n", b"\r\n", b"\r"): - if remaining == sequence: - return Noop() - if remaining.startswith(sequence): - return Truncate(position + len(sequence)) - - raise AssertionError("unreachable") # pragma: no cover # noqa: EM101 -``` - -Replace `fix_file` with: - -```python -def fix_file(file_obj: IO[bytes], *, check: bool) -> bool: - """Normalize one open binary file to end with exactly one terminator. - - Returns True if the file was fixed (or, under `check`, would be fixed). - Binary files (null byte in the first sample) are skipped, returning False. - """ - if _is_binary(file_obj): - return False - - match _detect_trailing(file_obj): - case Noop(): - return False - case AppendLf(): - if not check: - # Needs this seek for windows, otherwise IOError - file_obj.seek(0, os.SEEK_END) - file_obj.write(b"\n") - return True - case Truncate(offset): - if not check: - file_obj.seek(offset) - file_obj.truncate() - return True -``` - -(No `case _`: the three cases cover `_EofAction`, and ty enforces exhaustiveness through the `-> bool` return type.) - -- [ ] **Step 4: Run the full suite + coverage** - -Run: `just test` -Expected: PASS — all existing `test_fixer.py` / `test_end_of_file_fixer.py` tests stay green (behavior is identical), plus the new truncate check-mode test. Coverage 100% (the per-case `if not check:` branches are all exercised: append-non-check, append-check, truncate-non-check, truncate-check). - -- [ ] **Step 5: Confirm the exhaustiveness guard is live (manual, not committed)** - -Temporarily add a dummy variant to the union (e.g. `class _Probe: ...` and `_EofAction = Noop | AppendLf | Truncate | _Probe`) and run `just lint-ci`; confirm ty reports `invalid-return-type` on `fix_file`. Then revert the probe. Record the observed error text in your report. (This proves the seam is exhaustively checked; do not commit the probe.) - -- [ ] **Step 6: Lint** - -Run: `just lint`, then confirm a clean `just lint-ci` against the committed tree. - -- [ ] **Step 7: Commit** - -```bash -git add eof_fixer/fixer.py tests/test_fixer.py -git commit -m "refactor: type the EOF-action seam with a sealed union - -Co-Authored-By: Claude Opus 4.8 (1M context) " -``` - ---- - -### Task 2: Promote to architecture + finalize the bundle summary - -**Files:** -- Modify: `architecture/eof-normalization.md` -- Modify: `planning/changes/2026-06-26.03-type-eof-action-seam/design.md` (finalize `summary`) - -**Interfaces:** Documentation only. - -- [ ] **Step 1: Update `architecture/eof-normalization.md`** - -Read the file first. Its "action model" section describes the three actions as -the strings `none` / `append_lf` / `truncate`. Reword it to describe the typed -sealed union: `_detect_trailing` returns `_EofAction = Noop | AppendLf | Truncate(offset)` -(a frozen-dataclass sum type, offset only on `Truncate`), and `fix_file` -consumes it with an exhaustive `match` whose completeness ty enforces via the -`bool` return type. Preserve the surrounding terminator / binary-skip prose; -only the action-model description changes. - -- [ ] **Step 2: Finalize the bundle summary** - -In `planning/changes/2026-06-26.03-type-eof-action-seam/design.md`, set the -frontmatter `summary:` to the realized result, e.g.: -`summary: _detect_trailing now returns a sealed union (Noop | AppendLf | Truncate) consumed by an exhaustive match in fix_file; the magic strings and dummy offset are gone, behavior unchanged.` - -- [ ] **Step 3: Validate + lint** - -Run: `just check-planning` → expect `planning: OK`. -Run: `just lint-ci` → expect clean. - -- [ ] **Step 4: Commit** - -```bash -git add architecture/eof-normalization.md planning/changes/2026-06-26.03-type-eof-action-seam/design.md -git commit -m "docs: promote the typed EOF-action seam to architecture - -Co-Authored-By: Claude Opus 4.8 (1M context) " -``` - ---- - -## Done criteria - -- `_detect_trailing -> _EofAction`; `fix_file` consumes via a wildcard-free `match`; the magic strings `"none"`/`"append_lf"`/`"truncate"` and the dummy offset are gone. -- Exhaustiveness is enforced by ty (verified via the Step 5 probe); no `assert_never`, no new pragmas, 3.10-safe. -- Behavior unchanged; `just test` green at 100% coverage; `just lint-ci` clean; `just check-planning` → `planning: OK`. -- `architecture/eof-normalization.md` describes the typed action model; bundle summary finalized. diff --git a/planning/index.py b/planning/index.py index a1632e1..2d70ac3 100644 --- a/planning/index.py +++ b/planning/index.py @@ -1,14 +1,12 @@ -# ruff: noqa: INP001, D212 # planning/ is not a Python package; D212/D213 conflict differs from faststream-outbox -""" -Generate the planning index from frontmatter. +# ruff: noqa: INP001 # planning/ is not a Python package (this file is vendored into consumers' planning/) +"""Generate the planning index from frontmatter. -Run via ``just index``. Globs ``planning/changes/*/`` (each bundle's -``design.md``, falling back to ``change.md``) and ``planning/decisions/*.md``, -reads their frontmatter, and prints a Markdown listing to stdout — changes -then decisions, newest-first. Never writes a file: +Run via ``just index``. Globs ``planning/changes/*.md`` and +``planning/decisions/*.md``, reads their frontmatter, and prints a Markdown +listing to stdout — changes then decisions, newest-first. Never writes a file: the listing is a query over the files, not a committed artifact. -``date`` and ``slug`` are derived from the directory / file name, not +``date`` and ``slug`` are derived from the file name, not frontmatter — the name is the single source of truth for both. """ @@ -17,12 +15,10 @@ import sys -CHANGES_DIR = pathlib.Path(__file__).parent / "changes" -DECISIONS_DIR = pathlib.Path(__file__).parent / "decisions" +ROOT = pathlib.Path(__file__).parent VALID_DECISION_STATUS = {"accepted", "superseded"} -BUNDLE_RE = re.compile(r"^(?P\d{4}-\d{2}-\d{2})\.\d{2}-(?P.+)$") +CHANGE_RE = re.compile(r"^(?P\d{4}-\d{2}-\d{2})\.\d{2}-(?P.+)$") DECISION_RE = re.compile(r"^(?P\d{4}-\d{2}-\d{2})-(?P.+)$") -ALLOWED_BUNDLE_FILES = {"design.md", "plan.md", "change.md"} SPEC_REQUIRED = ("summary",) DECISION_REQUIRED = ("status", "summary") @@ -47,7 +43,7 @@ def parse_frontmatter(text: str) -> dict[str, str]: def _named(fields: dict[str, str], name: str, pattern: re.Pattern[str]) -> dict[str, str]: - """Inject ``date``/``slug`` derived from a dir/file name into ``fields``.""" + """Inject ``date``/``slug`` derived from a file name into ``fields``.""" match = pattern.match(name) if match: fields["date"] = match.group("date") @@ -55,32 +51,29 @@ def _named(fields: dict[str, str], name: str, pattern: re.Pattern[str]) -> dict[ return fields -def load_bundles() -> list[dict[str, str]]: - """Read each bundle's summary; derive date/slug from the directory name.""" - bundles: list[dict[str, str]] = [] - if not CHANGES_DIR.is_dir(): - return bundles - for bundle in sorted(CHANGES_DIR.iterdir()): - if not bundle.is_dir(): - continue - spec = bundle / "design.md" - if not spec.exists(): - spec = bundle / "change.md" - if not spec.exists(): +def load_changes(root: pathlib.Path) -> list[dict[str, str]]: + """Read each change file's summary; derive date/slug from the file name.""" + changes_dir = root / "changes" + changes: list[dict[str, str]] = [] + if not changes_dir.is_dir(): + return changes + for path in sorted(changes_dir.glob("*.md")): + if path.name == "README.md" or path.name.startswith(("_", ".")): continue - fields = _named(parse_frontmatter(spec.read_text(encoding="utf-8")), bundle.name, BUNDLE_RE) - fields["path"] = f"changes/{bundle.name}/{spec.name}" - fields["name"] = bundle.name - bundles.append(fields) - return bundles + fields = _named(parse_frontmatter(path.read_text(encoding="utf-8")), path.stem, CHANGE_RE) + fields["path"] = f"changes/{path.name}" + fields["name"] = path.stem + changes.append(fields) + return changes -def load_decisions() -> list[dict[str, str]]: +def load_decisions(root: pathlib.Path) -> list[dict[str, str]]: """Read each decision's frontmatter; derive date/slug from the file name.""" + decisions_dir = root / "decisions" decisions: list[dict[str, str]] = [] - if not DECISIONS_DIR.is_dir(): + if not decisions_dir.is_dir(): return decisions - for path in sorted(DECISIONS_DIR.glob("*.md")): + for path in sorted(decisions_dir.glob("*.md")): if path.name == "README.md" or path.name.startswith("_"): continue fields = _named(parse_frontmatter(path.read_text(encoding="utf-8")), path.stem, DECISION_RE) @@ -90,24 +83,24 @@ def load_decisions() -> list[dict[str, str]]: return decisions -def format_row(bundle: dict[str, str]) -> str: - """Render one bundle as a Markdown list item.""" - slug = bundle.get("slug", "?") - path = bundle.get("path", "") - date = bundle.get("date", "") - summary = bundle.get("summary") or "(no summary)" +def format_row(row: dict[str, str]) -> str: + """Render one change or decision as a Markdown list item.""" + slug = row.get("slug", "?") + path = row.get("path", "") + date = row.get("date", "") + summary = row.get("summary") or "(no summary)" line = f"- **[{slug}]({path})** ({date}) — {summary}" - if bundle.get("supersedes"): - line += f" _(supersedes {bundle['supersedes']})_" - if bundle.get("superseded_by"): - line += f" _(superseded by {bundle['superseded_by']})_" + if row.get("supersedes"): + line += f" _(supersedes {row['supersedes']})_" + if row.get("superseded_by"): + line += f" _(superseded by {row['superseded_by']})_" return line -def render(bundles: list[dict[str, str]], decisions: list[dict[str, str]]) -> str: +def render(changes: list[dict[str, str]], decisions: list[dict[str, str]]) -> str: """Render the full Markdown listing: changes then decisions, newest-first.""" out = ["# Planning index", "", "_Generated by `just index` — do not edit._", "", "## Changes", ""] - change_rows = sorted(bundles, key=lambda b: b.get("name", ""), reverse=True) + change_rows = sorted(changes, key=lambda b: b.get("name", ""), reverse=True) out += [format_row(b) for b in change_rows] if change_rows else ["_None._"] out += ["", "## Decisions", ""] decision_rows = sorted(decisions, key=lambda d: d.get("name", ""), reverse=True) @@ -121,32 +114,15 @@ def _require(fields: dict[str, str], keys: tuple[str, ...], rel: str, violations violations.extend(f"{rel}: missing or empty frontmatter key '{key}'" for key in keys if not fields.get(key)) -def _check_spec_file(path: pathlib.Path, rel: str, violations: list[str]) -> None: - """Validate a design.md / change.md spec file (requires `summary`).""" +def _check_change(path: pathlib.Path, violations: list[str]) -> None: + """Validate one change file (requires `summary`).""" + rel = f"changes/{path.name}" + if CHANGE_RE.match(path.stem) is None: + violations.append(f"{rel}: file name is not 'YYYY-MM-DD.NN-slug.md'") fields = parse_frontmatter(path.read_text(encoding="utf-8")) _require(fields, SPEC_REQUIRED, rel, violations) -def _check_bundle(bundle: pathlib.Path, violations: list[str]) -> None: - """Validate one change bundle directory.""" - rel = f"changes/{bundle.name}" - if BUNDLE_RE.match(bundle.name) is None: - violations.append(f"{rel}: directory name is not 'YYYY-MM-DD.NN-slug'") - violations.extend( - f"{rel}/{child.name}: unexpected file in bundle (allowed: {', '.join(sorted(ALLOWED_BUNDLE_FILES))})" - for child in sorted(bundle.iterdir()) - if child.name not in ALLOWED_BUNDLE_FILES - ) - design = bundle / "design.md" - change = bundle / "change.md" - if not design.exists() and not change.exists(): - violations.append(f"{rel}: bundle has neither design.md nor change.md") - for spec_file in (design, change): - if spec_file.exists(): - _check_spec_file(spec_file, f"{rel}/{spec_file.name}", violations) - # plan.md carries no frontmatter — its identity comes from the bundle dir. - - def _check_decision(path: pathlib.Path, violations: list[str]) -> None: """Validate one decision file (requires `status` + `summary`).""" rel = f"decisions/{path.name}" @@ -159,25 +135,39 @@ def _check_decision(path: pathlib.Path, violations: list[str]) -> None: violations.append(f"{rel}: invalid status '{status}' (allowed: {', '.join(sorted(VALID_DECISION_STATUS))})") -def check() -> list[str]: - """Validate every bundle and decision; return the list of violation strings.""" +def check(root: pathlib.Path) -> list[str]: + """Validate every change file and decision; return the list of violation strings.""" violations: list[str] = [] - if CHANGES_DIR.is_dir(): - for bundle in sorted(CHANGES_DIR.iterdir()): - if bundle.is_dir(): - _check_bundle(bundle, violations) - if DECISIONS_DIR.is_dir(): - for path in sorted(DECISIONS_DIR.glob("*.md")): + changes_dir = root / "changes" + decisions_dir = root / "decisions" + if changes_dir.is_dir(): + for path in sorted(changes_dir.iterdir()): + if path.is_dir(): + violations.append( + f"changes/{path.name}: directory found — convention 2.0.0 uses flat change files " + f"(changes/YYYY-MM-DD.NN-slug.md; see CHANGELOG 2.0.0 for the migration)" + ) + continue + if path.name == "README.md" or path.name.startswith(("_", ".")): + continue + if path.suffix != ".md": + violations.append(f"changes/{path.name}: unexpected non-md file in changes/") + else: + _check_change(path, violations) + if decisions_dir.is_dir(): + for path in sorted(decisions_dir.glob("*.md")): if path.name == "README.md" or path.name.startswith("_"): continue _check_decision(path, violations) return violations -def main() -> int: - """Print the listing to stdout, or validate bundles with --check.""" - if "--check" in sys.argv[1:]: - violations = check() +def main(argv: list[str] | None = None, root: pathlib.Path | None = None) -> int: + """Print the listing to stdout, or validate change files and decisions with --check.""" + argv = sys.argv[1:] if argv is None else argv + root = ROOT if root is None else root + if "--check" in argv: + violations = check(root) if violations: sys.stderr.write(f"planning: {len(violations)} violation(s)\n") for violation in violations: @@ -185,7 +175,7 @@ def main() -> int: return 1 sys.stdout.write("planning: OK\n") return 0 - sys.stdout.write(render(load_bundles(), load_decisions())) + sys.stdout.write(render(load_changes(root), load_decisions(root))) return 0