feat(export): export templates model, CRUD, and candidate resolution

.claude/hooks/check_comment_brevity.py

@@ -1,8 +1,12 @@
 #!/usr/bin/env python3
-"""PostToolUse hook: flags multi-line comment blocks in a just-written/edited file."""
+"""PreToolUse hook (Write/Edit): blocks a new multi-line comment block before it's written."""
+import ast
+import io
 import json
 import re
 import sys
+import tokenize
+from collections import Counter
 
 LINE_COMMENT_STYLES = {
     ".ts": "//", ".tsx": "//", ".js": "//", ".jsx": "//",
@@ -15,88 +19,138 @@
 }
 MAX_CONSECUTIVE = 2  # 3+ consecutive same-style comment lines triggers a flag
 MAX_BLOCK_LINES = 2  # a /* */ or docstring spanning more than this triggers a flag
-MAX_FILE_BYTES = 2 * 1024 * 1024
+DOCSTRING_OWNERS = (ast.Module, ast.ClassDef, ast.FunctionDef, ast.AsyncFunctionDef)
 
 
-def _line_at(text, offset):
-    return text.count("\n", 0, offset) + 1
+def comment_runs(numbered_lines, marker):
+    """Yield (first_lineno, text) for each run of more than MAX_CONSECUTIVE comment lines."""
+    run = []
+    for lineno, line in numbered_lines + [(None, "")]:
+        stripped = line.strip()
+        is_comment = stripped.startswith(marker) and not stripped.startswith(marker * 3)
+        if is_comment and (not run or run[-1][0] + 1 == lineno):
+            run.append((lineno, stripped))
+            continue
+        if len(run) > MAX_CONSECUTIVE:
+            yield run[0][0], "\n".join(t for _, t in run)
+        run = [(lineno, stripped)] if is_comment else []
 
 
-def find_violations(ext, text):
-    lines = text.splitlines()
-    violations = []
-
+def regex_violations(ext, text):
+    found = []
     marker = LINE_COMMENT_STYLES.get(ext)
     if marker:
-        run_start = None
-        for i, line in enumerate(lines):
-            stripped = line.strip()
-            if stripped.startswith(marker) and not stripped.startswith(marker * 3):
-                if run_start is None:
-                    run_start = i
-            else:
-                if run_start is not None and i - run_start > MAX_CONSECUTIVE:
-                    violations.append((run_start + 1, i))
-                run_start = None
-        if run_start is not None and len(lines) - run_start > MAX_CONSECUTIVE:
-            violations.append((run_start + 1, len(lines)))
+        found += comment_runs(list(enumerate(text.splitlines(), 1)), marker)
 
+    patterns = []
     if ext in BLOCK_COMMENT_EXTS:
-        for m in re.finditer(r"/\*.*?\*/", text, re.DOTALL):
-            if m.group(0).count("\n") + 1 > MAX_BLOCK_LINES:
-                violations.append((_line_at(text, m.start()), _line_at(text, m.end())))
-
+        patterns.append(r"/\*.*?\*/")
     if ext == ".py":
-        for m in re.finditer(r'("""|\'\'\')(.*?)\1', text, re.DOTALL):
+        patterns.append(r'("{3}|\'{3})(.*?)\1')
+    for pattern in patterns:
+        for m in re.finditer(pattern, text, re.DOTALL):
             if m.group(0).count("\n") + 1 > MAX_BLOCK_LINES:
-                violations.append((_line_at(text, m.start()), _line_at(text, m.end())))
+                found.append((text.count("\n", 0, m.start()) + 1, m.group(0)))
+    return found
+
+
+def python_violations(text, file_path):
+    """Use tokenize/ast so string literals aren't mistaken for comments or docstrings."""
+    tree = ast.parse(text)
+    comments = [
+        (tok.start[0], tok.string)
+        for tok in tokenize.generate_tokens(io.StringIO(text).readline)
+        if tok.type == tokenize.COMMENT and tok.line.strip().startswith("#")
+    ]
+    found = list(comment_runs(comments, "#"))
+
+    is_migration = "/migrations/versions/" in file_path
+    for node in ast.walk(tree):
+        if not isinstance(node, DOCSTRING_OWNERS) or not node.body:
+            continue
+        if isinstance(node, ast.Module) and is_migration:
+            continue  # Alembic's generated Revision ID/Revises header
+        first = node.body[0]
+        if not (isinstance(first, ast.Expr) and isinstance(first.value, ast.Constant)
+                and isinstance(first.value.value, str)):
+            continue
+        if first.end_lineno - first.lineno + 1 > MAX_BLOCK_LINES:
+            found.append((first.lineno, first.value.value.strip()))
+    return found
+
+
+def violations(ext, text, file_path):
+    if ext == ".py":
+        try:
+            return python_violations(text, file_path)
+        except (SyntaxError, tokenize.TokenError, ValueError):
+            pass
+    return regex_violations(ext, text)
 
-    return sorted(set(violations))
+
+def before_and_after(tool_name, tool_input, file_path):
+    try:
+        with open(file_path, encoding="utf-8", errors="replace") as f:
+            before = f.read()
+    except OSError:
+        before = ""
+
+    if tool_name == "Write":
+        return before, tool_input.get("content", "")
+    if tool_name == "Edit":
+        old, new = tool_input.get("old_string", ""), tool_input.get("new_string", "")
+        if not before or old not in before:
+            return "", new
+        count = -1 if tool_input.get("replace_all") else 1
+        return before, before.replace(old, new, count)
+    return None, None
 
 
 def main():
     try:
         payload = json.load(sys.stdin)
     except Exception:
-        return 0
+        return
 
+    tool_name = payload.get("tool_name")
     tool_input = payload.get("tool_input") or {}
-    file_path = tool_input.get("file_path") or (payload.get("tool_response") or {}).get("filePath")
-    if not file_path or "." not in file_path.rsplit("/", 1)[-1]:
-        return 0
+    file_path = tool_input.get("file_path", "")
+    if "." not in file_path.rsplit("/", 1)[-1]:
+        return
 
     ext = "." + file_path.rsplit(".", 1)[-1].lower()
     if ext not in LINE_COMMENT_STYLES and ext not in BLOCK_COMMENT_EXTS:
-        return 0
-
-    try:
-        import os
-        if os.path.getsize(file_path) > MAX_FILE_BYTES:
-            return 0
-        with open(file_path, "r", encoding="utf-8", errors="ignore") as f:
-            text = f.read()
-    except OSError:
-        return 0
-
-    violations = find_violations(ext, text)
-    if not violations:
-        return 0
-
-    ranges = ", ".join(f"L{a}-{b}" if a != b else f"L{a}" for a, b in violations)
-    warning = (
-        f"Comment brevity check: {file_path} has a multi-line comment block at {ranges}. "
-        "Trim it to one short line; move any longer rationale to a memory file instead."
-    )
-    print(warning, file=sys.stderr)
+        return
+
+    before, after = before_and_after(tool_name, tool_input, file_path)
+    if after is None:
+        return
+
+    existing = Counter(text for _, text in violations(ext, before, file_path))
+    introduced = []
+    for lineno, text in violations(ext, after, file_path):
+        if existing[text] > 0:
+            existing[text] -= 1
+        else:
+            introduced.append((lineno, text))
+    if not introduced:
+        return
+
+    lineno, text = introduced[0]
+    preview = text.splitlines()[0][:80]
     print(json.dumps({
-        "systemMessage": warning,
         "hookSpecificOutput": {
-            "hookEventName": "PostToolUse",
-            "additionalContext": warning,
-        },
+            "hookEventName": "PreToolUse",
+            "permissionDecision": "deny",
+            "permissionDecisionReason": (
+                f"Blocked: this {tool_name} to {file_path} introduces a multi-line comment "
+                f"at line {lineno} ({preview!r}): more than {MAX_BLOCK_LINES} lines, or more "
+                f"than {MAX_CONSECUTIVE} consecutive comment lines. Trim it to one or two "
+                "lines; longer rationale belongs in the commit message, PR description, or docs."
+            ),
+        }
     }))
-    return 0
 
 
 if __name__ == "__main__":
-    sys.exit(main())
+    main()

.claude/settings.json

@@ -56,17 +56,22 @@
             "timeout": 15
           }
         ]
-      }
-    ],
-    "PostToolUse": [
+      },
       {
         "matcher": "Write|Edit",
         "hooks": [
           {
             "type": "command",
             "command": "python3 /home/noro/repos/GrandFlow/.claude/hooks/check_comment_brevity.py",
             "timeout": 15
-          },
+          }
+        ]
+      }
+    ],
+    "PostToolUse": [
+      {
+        "matcher": "Write|Edit",
+        "hooks": [
           {
             "type": "command",
             "command": "python3 /home/noro/repos/GrandFlow/.claude/hooks/check_test_factory_usage.py",

CLAUDE.md

@@ -49,10 +49,21 @@ This is enforced, not just documented:
   blocks manually-created branches whose name doesn't match the convention.
 - `scripts/git-hooks/pre-push` refuses to push a branch with a non-conforming
   name (requires `git config core.hooksPath scripts/git-hooks` — see
-  WORKFLOW.md §7, which also covers this hook's lint-mirroring behavior).
+  WORKFLOW.md §8, which also covers this hook's lint-mirroring behavior).
 - `.github/workflows/branch-naming.yml` re-checks the branch name on every
   push and PR as a backstop.
 
 For a ticket that isn't part of an OpenSpec change, use
 `scripts/flow.py issue "<title>" "<body>"`. If such a task still needs a branch,
 ask the user before improvising a name outside the convention.
+
+## Comments — keep them short
+
+Comments (line-comment runs, `/* */` blocks, docstrings) are at most 2 lines.
+Longer rationale belongs in a commit message, PR description, or docs — not
+in the code.
+
+This is enforced, not just documented: `.claude/hooks/check_comment_brevity.py`
+(a `PreToolUse` hook on `Write`/`Edit`) denies any write that introduces a
+comment longer than that, so trim it before retrying rather than fighting the
+hook.

docs/development/WORKFLOW.md

@@ -12,6 +12,7 @@ Everything below is driven by one tool, `scripts/flow.py`:
 | `flow.py start <change> <group>` | starting a group | parent + group → In Progress, branch from `main` |
 | `flow.py pr` | opening the PR | prints the `Closes` trailer |
 | `flow.py issue "<t>" "<body>"` | one-off ticket | issue on the board, outside any change |
+| `flow.py cleanup [--yes]` | after merges pile up | deletes local branches whose remote was deleted |
 
 Sub-issues are matched to task groups by the issue number recorded in `tasks.md`,
 falling back to the `(group N)` title suffix when no number is recorded.
@@ -140,7 +141,21 @@ scripts/flow.py issue "<title>" "<body>"
 Creates the issue and puts it on the board as Todo. If it needs a branch, agree
 a name with a human first — the convention above assumes a sub-issue exists.
 
-## 7. Local lint enforcement before push
+## 7. Local branch cleanup
+
+Squash-merging means `git branch --merged` never recognizes a merged group
+branch, so they pile up locally even though the remote is clean. Run:
+
+```
+scripts/flow.py cleanup
+```
+
+It fetches with `--prune`, lists local branches whose upstream was deleted
+(skipping the current branch and any branch checked out in another worktree),
+and deletes them after a y/N confirmation (`-D`, since they're squash-merged
+rather than fast-forward-mergeable). Pass `--yes` to skip the prompt.
+
+## 8. Local lint enforcement before push
 
 `scripts/git-hooks/pre-push` mirrors each service's CI lint step
 (`black --check`, `mypy`, `flake8`) locally, scoped to whichever service(s) the
@@ -155,7 +170,7 @@ git config core.hooksPath scripts/git-hooks
 
 Bypass with `git push --no-verify` when intentionally needed.
 
-## 8. Testing the tool itself
+## 9. Testing the tool itself
 
 `scripts/test_flow.py` covers the change-name/group parsing and the `tasks.md`
 rewriting — the load-bearing, network-free parts:
@@ -166,7 +181,7 @@ python3 -m pytest scripts/test_flow.py -q
 
 `.github/workflows/tooling.yml` runs the same tests plus `black`/`flake8` over
 `scripts/` on any push or PR that touches `scripts/**`, and the pre-push hook
-(§7) mirrors it locally.
+(§8) mirrors it locally.
 
 `scripts/flow.py --dry-run <subcommand> ...` prints every mutating `gh`/`git`
 call instead of running it, which is the safe way to preview `init` or `sync`.

openspec/changes/budget-feat-313-excel-export/specs/budget-excel-export/spec.md

@@ -119,17 +119,6 @@ The export endpoint SHALL accept an optional template identifier and SHALL NOT s
 - **WHEN** a client re-sends a template identifier that has since been deleted or un-shared
 - **THEN** the request is rejected with an error identifying that the template is no longer available, rather than falling back to another template
 
-### Requirement: Template-scoped ownership
-An export template SHALL belong to exactly one organisation and SHALL carry a visibility of either private or shared with grantees. Template names SHALL be unique within an owning organisation. Every query for templates SHALL be scoped to a requesting organisation; the system SHALL NOT expose a way to read templates without an organisational scope.
-
-#### Scenario: Duplicate name within an organisation
-- **WHEN** an organisation creates a second template with the name of one it already owns
-- **THEN** the request is rejected, so which template a name refers to is never ambiguous
-
-#### Scenario: Same name across organisations
-- **WHEN** two different organisations each create a template named "Annual Report"
-- **THEN** both are accepted, and each organisation only ever sees its own
-
 ### Requirement: Renderer options applied to the generated workbook
 A template SHALL define its output as a bounded set of options over the built-in renderer — which sheets to include, whether the donor-currency estimate column is shown, column header label overrides, and whether the audit footer is shown — validated on write. The system SHALL reject an unrecognised option at write time rather than ignoring it at generation time. Options absent from a stored template SHALL fall back to the system default's value at generation time.
 

openspec/changes/budget-feat-313-excel-export/specs/budget-export-templates/spec.md

@@ -0,0 +1,25 @@
+# Spec Delta
+
+## Purpose
+
+Lets an organisation create, own, and share the export templates that `budget-excel-export`'s template selection and rendering consume — the storage, uniqueness, versioning, and access-scoping rules for those template records.
+
+## ADDED Requirements
+
+### Requirement: Template-scoped ownership
+An export template SHALL belong to exactly one organisation and SHALL carry a visibility of either private or shared with grantees. Template names SHALL be unique within an owning organisation. Every query for templates SHALL be scoped to a requesting organisation; the system SHALL NOT expose a way to read templates without an organisational scope.
+
+#### Scenario: Duplicate name within an organisation
+- **WHEN** an organisation creates a second template with the name of one it already owns
+- **THEN** the request is rejected, so which template a name refers to is never ambiguous
+
+#### Scenario: Same name across organisations
+- **WHEN** two different organisations each create a template named "Annual Report"
+- **THEN** both are accepted, and each organisation only ever sees its own
+
+### Requirement: Template versioning on edit
+An export template SHALL carry a version number that advances whenever its name, visibility, or options are updated.
+
+#### Scenario: Edit bumps version
+- **WHEN** an organisation updates one of its own templates
+- **THEN** the template's stored version increases, distinguishing the edited template from the one that produced any workbook exported before the edit