Skip /go's fix-commit review and Final simplify for small, low-risk fixes

.github/workflows/test.yml

@@ -13,6 +13,7 @@ jobs:
       - name: Run AI tooling tests
         run: |
           python3 ai/skills/go/scripts/tests/test_run_review.py
+          python3 ai/skills/go/scripts/tests/test_review_tier.py
           ai/tests/test-skill-spec.sh
           ai/tests/test-canonical-skills.sh
           ai/bin/sync-portable-skills.sh --check

ai/README.md

@@ -52,7 +52,9 @@ When `implementation-planner` marks a plan with two or more stages `**Execution*
 
 ## Fresh review processes
 
-`go` supports Claude Code and Codex. It passes its current harness explicitly to `skills/go/scripts/run-review.py`, which starts a fresh review in the same checkout. Claude uses print mode; Codex uses `exec`. Both save the review result under the checkout's `.notes/` directory so the parent can resume at validation after a context clear. The runner requires Python 3.11 or later and the selected CLI on `PATH`; it never substitutes the other harness. The installed `review-code` skill must support `REVIEW_CODE_REVIEW_DIR`; update it from its source repository with `bin/setup` if the runner reports an older installation. Review sessions, reports, temporary checkouts, and hook files stay under each attempt in `.notes/go-reviews/`. The Codex child receives no write grants for shared review caches. After success, the parent copies the report into the shared review history, where it survives removal of the implementation worktree. A Claude review runs on the accounts that docket's `config.toml` names in `claude_config_dir` and `claude_fallback_config_dirs`, in the order docket's rotation picks them. The runner skips each account that a usage limit in docket's `limits.jsonl` still applies to. A limit applies to every config directory signed in to the login (one account in one organization) that docket recorded it with. `run-review.py accounts` prints that rotation, the limits docket recorded, and the order a review would try the accounts in now, as JSON, and `usage-all` shows it below each account's usage. Without a docket config, the review runs on the current account. The runner refuses an account that is not signed in or has no `review-code` skill. When an account hits its usage limit before the review edits any files, the runner reviews again on the next account that is signed in to a different login, and then on the current account if the list leaves it out. A run with `--since-run <run-id>` copies that earlier run's report into its own attempt and asks `review-code` for an `--append` review of the changes since that report.
+`go` supports Claude Code and Codex. It passes its current harness explicitly to `skills/go/scripts/run-review.py`, which starts a fresh review in the same checkout. Claude uses print mode; Codex uses `exec`. Both save the review result under the checkout's `.notes/` directory so the parent can resume at validation after a context clear. The runner requires Python 3.11 or later and the selected CLI on `PATH`; it never substitutes the other harness. The installed `review-code` skill must support `REVIEW_CODE_REVIEW_DIR`; update it from its source repository with `bin/setup` if the runner reports an older installation. Review sessions, reports, temporary checkouts, and hook files stay under each attempt in `.notes/go-reviews/`. The Codex child receives no write grants for shared review caches. After success, the parent copies the report into the shared review history, where it survives removal of the implementation worktree. A Claude review runs on the accounts that docket's `config.toml` names in `claude_config_dir` and `claude_fallback_config_dirs`, in the order docket's rotation picks them. The runner skips each account that a usage limit in docket's `limits.jsonl` still applies to. A limit applies to every config directory signed in to the login (one account in one organization) that docket recorded it with. `run-review.py accounts` prints that rotation, the limits docket recorded, and the order a review would try the accounts in now, as JSON, and `usage-all` shows it below each account's usage. Without a docket config, the review runs on the current account. The runner refuses an account that is not signed in or has no `review-code` skill. When an account hits its usage limit before the review edits any files, the runner reviews again on the next account that is signed in to a different login, and then on the current account if the list leaves it out. A run with `--since-run <run-id>` copies that earlier run's report into its own attempt and asks `review-code` for an `--append` review of the changes since that report. After a successful review, the runner copies the review's token-usage rows into the shared `token-usage.jsonl` with the `go` run IDs added, because the copy under `.notes/go-reviews/` is deleted with the worktree.
+
+`skills/go/scripts/review-tier.py` decides whether two late steps run. Review fix commits skips its review only for a small change to docs or tests that touches no risky path (such as a migration, a public API, auth, or infra) and makes no uncertain change (such as a rename or a deleted file). Final simplify skips its pass when the review fixes change a few lines or only docs. Prompt files, such as a `SKILL.md` or any file under `skills/`, count as code, and so does any file the script does not recognize as docs or tests. The line limits are constants at the top of the script. The Report step lists every skipped step and the commits no reviewer read.
 
 ## Model tiers
 
@@ -73,6 +75,7 @@ Run the installer and portability tests with:
 
 ```sh
 python3 ai/skills/go/scripts/tests/test_run_review.py
+python3 ai/skills/go/scripts/tests/test_review_tier.py
 ai/tests/test-skill-spec.sh
 ai/tests/test-canonical-skills.sh
 ai/bin/sync-portable-skills.sh --check

ai/skills/go/SKILL.md

@@ -47,6 +47,7 @@ active-stage: review-code
 next-action: validate-review-fixes
 review-state: .notes/go-review-state.json
 review-run: 0b6e4c2a-5d1f-4e8b-9a3c-7f2d1e6b8a40
+fix-review-tier: skip 9c1d2e3..f4a5b6c (14 lines)
 
 - implement: done
 - simplify-commit: a1b2c3d
@@ -436,6 +437,15 @@ Record `- reviews-addressed: <short HEAD sha>`.
 
 The commits from Address reviews reached the branch after `review-code` read it, and Final simplify only simplifies them. This step runs `review-code` over them once. It runs before Final simplify so that simplify also reads this step's fixes. Save `active-stage: fix-review`. A state file without `review-run` predates this step. In that case, record the `run_id` that `run-review.py status` reports as `review-run` in the same write, because that run is then the Review step's. If HEAD still equals the `review-code` sha, Address reviews committed nothing: go to the bookkeeping below.
 
+Decide whether the fix commits need a review. Run the tier script from the commit the Review step's review read, so the range covers the Review step's own fixes too. The script gives the same answer for the same range, so a resumed run makes the same decision:
+
+```bash
+REVIEW_INPUT_SHA=$(jq -r .input_sha ".notes/go-reviews/$REVIEW_RUN/state.json")
+python3 "$GO_SKILL_DIR/scripts/review-tier.py" --purpose fix-review --base "$REVIEW_INPUT_SHA"
+```
+
+It prints JSON with `tier`, `reasons`, and `stats`. Record `fix-review-tier: <tier> <short base>..<short HEAD> (<reasons, or the line count for skip>)` in the header, replacing an earlier entry, so the Report step can name the decision. `skip`, which the script returns only for small changes to docs or tests, goes to the bookkeeping below without a review. `full` runs the review as below. A small code change still gets the review, because a small change can carry a large bug.
+
 Push any unpushed commits first, and resolve a failed push before going on. `review-code` reads the PR's head from GitHub, so it does not review a commit that is only local. Save `next-action: run-review`, then run the runner as the Review step does, naming the review that step saved as `review-run`:
 
 ```bash
@@ -456,6 +466,14 @@ Then handle the result as the Review step does, from waiting for `phase: reviewe
 
 The Review, Address reviews, and Review fix commits steps commit review fixes that Quality passes never read. Save `active-stage: final-simplify`, then list the branch's own commits since the later of `simplify-commit` and a `final-simplify` entry from an earlier pass, using the `git log --first-parent --no-merges` command from Quality passes, so a later pass reads only the commits added since the last simplify pass. If `git merge-base --is-ancestor <start sha> HEAD` fails, a rebase has rewritten the branch since that pass and the fixes can no longer be told apart, so list from the branch's base as Quality passes resolves it. An empty list means the reviews changed nothing: go to the bookkeeping below.
 
+Otherwise decide whether the commits need a simplify pass. Run the tier script from the start sha the list used:
+
+```bash
+python3 "$GO_SKILL_DIR/scripts/review-tier.py" --purpose final-simplify --base <start sha>
+```
+
+Record `final-simplify-tier: <tier> <short start sha>..<short HEAD> (<reasons, or the line count for skip>)` in the header, replacing an earlier entry. `skip`, which the script returns for a few changed lines or a docs-only change, goes to the bookkeeping below.
+
 Otherwise run `simplify` over those commits, recording declined findings and held comments as Quality passes does:
 
 - Pass the list as the scope, because the fixes are usually pushed by now: `Skill("simplify", args: "Review only the changes these commits made: <shas>. They are review fixes. The rest of the branch already had a simplify pass.")`
@@ -568,7 +586,8 @@ explain-open translates each open or skipped item into plain English, weighs bot
 Then report the rest:
 
 - Commits added during the run — `git log <loop base>..HEAD --oneline` when the branch has a stage 1 commit, using the loop base that Implement's stage loop defines, otherwise `git log <simplify-commit sha>^..HEAD --oneline` using the state file's first recorded sha (everything is pushed by now, so `@{u}..HEAD` comes back empty)
-- The commits that landed after the last review read the branch, listed separately with their files: `git log --name-only --format='%h %s' <input_sha>..HEAD`, where `<input_sha>` is the value `run-review.py status` reports, or the `simplify-commit` sha when it reports none. No reviewer read those changes, so the user may want to look at them.
+- The commits that landed after the last review read the branch, listed separately with their files: `git log --name-only --format='%h %s' <input_sha>..HEAD`, where `<input_sha>` is the value `run-review.py status` reports, or the `simplify-commit` sha when it reports none. No reviewer read those changes, so the user may want to look at them. When Review fix commits skipped its review, this list holds the fix commits.
+- Each step that a tier skipped, from `fix-review-tier` and `final-simplify-tier` entries that read `skip`: the step's name, its commit range, and the reason
 - The PR URL (`gh pr view --json url -q .url`) and CI status
 - Gap entries logged this run — misses and false positives, with the `reviewhog-gaps.md` path — or that ReviewHog was skipped/timed out
 - Any drafted replies to human reviewers awaiting approval — these are never posted automatically

ai/skills/go/scripts/review-tier.py

@@ -0,0 +1,295 @@
+#!/usr/bin/env python3
+"""Decide whether go's fix-commit review or Final simplify runs over a commit range."""
+
+import argparse
+import fnmatch
+import json
+import re
+import subprocess
+import sys
+
+
+FIX_REVIEW_MAX_LINES = 100
+FINAL_SIMPLIFY_MAX_LINES = 20
+
+PROMPT_NAMES = {
+    "skill.md",
+    "agents.md",
+    "claude.md",
+    "gemini.md",
+    "copilot-instructions.md",
+    ".cursorrules",
+}
+PROMPT_SEGMENTS = {
+    "skills",
+    "agents",
+    "commands",
+    "prompts",
+    "handlers",
+    "contexts",
+    "rules",
+    "evals",
+    ".claude",
+    ".cursor",
+}
+TEST_SEGMENTS = {"test", "tests", "__tests__", "spec", "testdata"}
+TEST_NAME = re.compile(
+    r"test_.*\.py|conftest\.py|.*_test\.go|.*_spec\.rb|.*\.(test|spec)\.(js|jsx|ts|tsx|mjs|cjs)"
+)
+DOCS_EXTENSIONS = (".md", ".mdx", ".rst", ".adoc")
+DOCS_PREFIXES = ("README", "CHANGELOG", "LICENSE")
+
+API_SEGMENTS = {"api", "openapi", "serializers", "sdk"}
+WIRE_EXTENSIONS = (".proto", ".avsc", ".capnp", ".thrift", ".fbs")
+AUTH_WORDS = (
+    "auth",
+    "permission",
+    "access_control",
+    "access-control",
+    "rbac",
+    "csrf",
+    "cors",
+    "secret",
+    "credential",
+    "crypt",
+    "password",
+    "security",
+    "saml",
+    "sso",
+)
+INFRA_SEGMENTS = {
+    "charts",
+    "helm",
+    "terraform",
+    "k8s",
+    "kubernetes",
+    "argocd",
+    ".github",
+    "deploy",
+    "infra",
+}
+DEPENDENCY_NAMES = {
+    "package.json",
+    "package-lock.json",
+    "pnpm-lock.yaml",
+    "yarn.lock",
+    "pyproject.toml",
+    "poetry.lock",
+    "uv.lock",
+    "pipfile",
+    "pipfile.lock",
+    "cargo.toml",
+    "cargo.lock",
+    "go.mod",
+    "go.sum",
+    "gemfile",
+    "gemfile.lock",
+}
+DATA_DELETION = re.compile(
+    r"\bDELETE\s+FROM\b|\bDROP\s+(TABLE|COLUMN|DATABASE|INDEX)\b|\bTRUNCATE\s+TABLE\b|\bALTER\s+TABLE\b|\brm\s+-rf\b",
+    re.IGNORECASE,
+)
+STATUS_REASONS = {
+    "R": "uncertain-rename",
+    "C": "uncertain-copy",
+    "D": "uncertain-deleted-file",
+    "T": "uncertain-type-change",
+}
+SUBMODULE_MODE = "160000"
+SYMLINK_MODE = "120000"
+
+
+def git(*args):
+    return subprocess.run(
+        ["git", *args], check=True, capture_output=True
+    ).stdout.decode(errors="replace")
+
+
+def git_diff(*args):
+    # A user's external diff driver or textconv filter would change the output this script parses.
+    return git("diff", "--no-ext-diff", "--no-textconv", *args)
+
+
+def split_path(path):
+    parts = path.split("/")
+    return parts[:-1], parts[-1]
+
+
+def file_class(path):
+    # A file counts as docs or tests only when its name says so.
+    # Any file the rules do not recognize counts as code, so the review runs.
+    segments, name = split_path(path)
+    lowered = {segment.lower() for segment in segments}
+    lower_name = name.lower()
+    if lower_name in PROMPT_NAMES:
+        return "code"
+    if lowered & TEST_SEGMENTS or TEST_NAME.fullmatch(name):
+        return "test"
+    if lowered & PROMPT_SEGMENTS:
+        return "code"
+    suffix = lower_name.rpartition(".")[2] if "." in lower_name else ""
+    if lower_name.endswith(DOCS_EXTENSIONS) or (
+        name.startswith(DOCS_PREFIXES) and suffix in {"", "txt"}
+    ):
+        return "docs"
+    return "code"
+
+
+def path_risks(path):
+    segments, name = split_path(path)
+    lowered = {segment.lower() for segment in segments}
+    lower_name = name.lower()
+    lower_path = path.lower()
+    risks = []
+    if (
+        any("migration" in segment for segment in lowered)
+        or "alembic" in lowered
+        or lower_name.endswith(".sql")
+    ):
+        risks.append("risky-migration")
+    if lowered & API_SEGMENTS or lower_name.endswith(".graphql"):
+        risks.append("risky-api")
+    if lower_name.endswith(WIRE_EXTENSIONS):
+        risks.append("risky-wire-format")
+    if any(word in lower_path for word in AUTH_WORDS):
+        risks.append("risky-auth")
+    if (
+        lowered & INFRA_SEGMENTS
+        or lower_name.endswith((".tf", ".tfvars"))
+        or lower_name.startswith(("dockerfile", "docker-compose"))
+        or (lower_name.startswith("values") and lower_name.endswith((".yaml", ".yml")))
+    ):
+        risks.append("risky-infra")
+    if lower_name in DEPENDENCY_NAMES or fnmatch.fnmatch(
+        lower_name, "requirements*.txt"
+    ):
+        risks.append("risky-dependency")
+    return [f"{risk}: {path}" for risk in risks]
+
+
+def raw_entries(base, head):
+    fields = git_diff("--raw", "-z", "-M", "-C", base, head).split("\0")
+    entries = []
+    index = 0
+    while index < len(fields) and fields[index]:
+        old_mode, new_mode, _, _, status = fields[index][1:].split(" ")
+        paths = 2 if status[0] in "RC" else 1
+        entries.append(
+            {
+                "old_mode": old_mode,
+                "new_mode": new_mode,
+                "status": status[0],
+                "path": fields[index + paths],
+            }
+        )
+        index += 1 + paths
+    return entries
+
+
+def line_counts(base, head):
+    fields = git_diff("--numstat", "-z", "-M", "-C", base, head).split("\0")
+    counts = {}
+    index = 0
+    while index < len(fields) and fields[index]:
+        added, removed, path = fields[index].split("\t", 2)
+        if path:
+            index += 1
+        else:
+            path = fields[index + 2]
+            index += 3
+        counts[path] = (
+            None if added == "-" else (int(added), int(removed))
+        )
+    return counts
+
+
+def added_lines(base, head):
+    in_hunk = False
+    for line in git_diff("--unified=0", "--no-color", base, head).splitlines():
+        if line.startswith("diff --git "):
+            in_hunk = False
+        elif line.startswith("@@"):
+            in_hunk = True
+        elif in_hunk and line.startswith("+"):
+            yield line[1:]
+
+
+def uncertain_reasons(entry, counts):
+    path = entry["path"]
+    modes = {entry["old_mode"], entry["new_mode"]}
+    reasons = [STATUS_REASONS[entry["status"]]] if entry["status"] in STATUS_REASONS else []
+    if entry["status"] == "M" and entry["old_mode"] != entry["new_mode"]:
+        reasons.append("uncertain-mode-change")
+    if SUBMODULE_MODE in modes:
+        reasons.append("uncertain-submodule")
+    if SYMLINK_MODE in modes:
+        reasons.append("uncertain-symlink")
+    if counts.get(path, (0, 0)) is None:
+        reasons.append("uncertain-binary")
+    return [f"{reason}: {path}" for reason in reasons]
+
+
+def decide(purpose, base, head):
+    result = {"purpose": purpose, "base": base, "head": head}
+    try:
+        entries = raw_entries(base, head)
+        counts = line_counts(base, head)
+        deletions = purpose == "fix-review" and any(
+            DATA_DELETION.search(line) for line in added_lines(base, head)
+        )
+    except (subprocess.CalledProcessError, ValueError, IndexError) as error:
+        message = getattr(error, "stderr", None) or str(error).encode()
+        first_line = message.decode(errors="replace").strip().partition("\n")[0]
+        return result | {
+            "tier": "full",
+            "reasons": [f"uncertain-diff-failed: {first_line}"],
+            "stats": {"lines": 0, "files": 0},
+        }
+    lines = sum(sum(count) for count in counts.values() if count)
+    result["stats"] = {"lines": lines, "files": len(entries)}
+
+    uncertain = []
+    risky = []
+    shape = []
+    classes = set()
+    for entry in entries:
+        path = entry["path"]
+        uncertain += uncertain_reasons(entry, counts)
+        if purpose == "fix-review":
+            risky += path_risks(path)
+        kind = file_class(path)
+        classes.add(kind)
+        if kind == "code":
+            shape.append(f"code-file: {path}")
+        count = counts.get(path)
+        if kind == "test" and count and count[1] > count[0]:
+            shape.append(f"test-lines-removed: {path}")
+    if deletions:
+        risky.append("risky-data-deletion")
+
+    if purpose == "fix-review":
+        reasons = risky + uncertain + shape
+        limit = FIX_REVIEW_MAX_LINES
+    else:
+        reasons = uncertain
+        # A docs-only change skips Final simplify at any size.
+        limit = None if classes == {"docs"} else FINAL_SIMPLIFY_MAX_LINES
+    if limit is not None and lines > limit:
+        reasons.append(f"too-large: {lines} lines, over {limit}")
+    return result | {"tier": "full" if reasons else "skip", "reasons": reasons}
+
+
+def main():
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument(
+        "--purpose", choices=["fix-review", "final-simplify"], required=True
+    )
+    parser.add_argument("--base", required=True)
+    parser.add_argument("--head", default="HEAD")
+    args = parser.parse_args()
+    print(json.dumps(decide(args.purpose, args.base, args.head)))
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())