From 5ca4ac48abda866116e9675b3712c119dd889752 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sat, 18 Apr 2026 12:10:33 +0800 Subject: [PATCH] fix: plan-review right-sized diff counterbalance (not minimal-diff default) /plan-ceo-review and /plan-eng-review listed "minimal diff" as an engineering preference without counterbalancing language. Reviewers picked up on that and rejected rewrites that should have been approved. The preference is now framed as "right-sized diff" with explicit permission to recommend a rewrite when the existing foundation is broken. Implementation alternatives section in CEO review gets an equal-weight clarification: don't default to minimal viable just because it is smaller. Recommend whichever best serves the user's goal; if the right answer is a rewrite, say so. Three-line tone edit per template, no voice / ETHOS / YC / promotional content change. Co-Authored-By: Claude Opus 4.7 (1M context) --- plan-ceo-review/SKILL.md | 5 +++-- plan-ceo-review/SKILL.md.tmpl | 3 ++- plan-eng-review/SKILL.md | 4 ++-- plan-eng-review/SKILL.md.tmpl | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/plan-ceo-review/SKILL.md b/plan-ceo-review/SKILL.md index c2fc9bbb..75aab7c3 100644 --- a/plan-ceo-review/SKILL.md +++ b/plan-ceo-review/SKILL.md @@ -644,7 +644,7 @@ Do NOT make any code changes. Do NOT start implementation. Your only job right n * I want code that's "engineered enough" — not under-engineered (fragile, hacky) and not over-engineered (premature abstraction, unnecessary complexity). * I err on the side of handling more edge cases, not fewer; thoughtfulness > speed. * Bias toward explicit over clever. -* Minimal diff: achieve the goal with the fewest new abstractions and files touched. +* Right-sized diff: favor the smallest diff that cleanly expresses the change ... but don't compress a necessary rewrite into a minimal patch. If the existing foundation is broken, invoke permission #9 and say "scrap it and do this instead." * Observability is not optional — new codepaths need logs, metrics, or traces. * Security is not optional — new codepaths need threat modeling. * Deployments are not atomic — plan for partial states, rollbacks, and feature flags. @@ -935,6 +935,7 @@ Rules: - At least 2 approaches required. 3 preferred for non-trivial plans. - One approach must be the "minimal viable" (fewest files, smallest diff). - One approach must be the "ideal architecture" (best long-term trajectory). +- **These two approaches have equal weight.** Don't default to "minimal viable" just because it's smaller. Recommend whichever best serves the user's goal. If the right answer is a rewrite, say so. - If only one approach exists, explain concretely why alternatives were eliminated. - Do NOT proceed to mode selection (0F) without user approval of the chosen approach. @@ -1419,7 +1420,7 @@ THE PLAN: ```bash TMPERR_PV=$(mktemp /tmp/codex-planreview-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR_PV" +codex exec "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_PV" ``` Use a 5-minute timeout (`timeout: 300000`). After the command completes, read stderr: diff --git a/plan-ceo-review/SKILL.md.tmpl b/plan-ceo-review/SKILL.md.tmpl index d128b180..93d1af0a 100644 --- a/plan-ceo-review/SKILL.md.tmpl +++ b/plan-ceo-review/SKILL.md.tmpl @@ -60,7 +60,7 @@ Do NOT make any code changes. Do NOT start implementation. Your only job right n * I want code that's "engineered enough" — not under-engineered (fragile, hacky) and not over-engineered (premature abstraction, unnecessary complexity). * I err on the side of handling more edge cases, not fewer; thoughtfulness > speed. * Bias toward explicit over clever. -* Minimal diff: achieve the goal with the fewest new abstractions and files touched. +* Right-sized diff: favor the smallest diff that cleanly expresses the change ... but don't compress a necessary rewrite into a minimal patch. If the existing foundation is broken, invoke permission #9 and say "scrap it and do this instead." * Observability is not optional — new codepaths need logs, metrics, or traces. * Security is not optional — new codepaths need threat modeling. * Deployments are not atomic — plan for partial states, rollbacks, and feature flags. @@ -242,6 +242,7 @@ Rules: - At least 2 approaches required. 3 preferred for non-trivial plans. - One approach must be the "minimal viable" (fewest files, smallest diff). - One approach must be the "ideal architecture" (best long-term trajectory). +- **These two approaches have equal weight.** Don't default to "minimal viable" just because it's smaller. Recommend whichever best serves the user's goal. If the right answer is a rewrite, say so. - If only one approach exists, explain concretely why alternatives were eliminated. - Do NOT proceed to mode selection (0F) without user approval of the chosen approach. diff --git a/plan-eng-review/SKILL.md b/plan-eng-review/SKILL.md index 1b2482e1..9fe128ef 100644 --- a/plan-eng-review/SKILL.md +++ b/plan-eng-review/SKILL.md @@ -589,7 +589,7 @@ If the user asks you to compress or the system triggers context compaction: Step * I want code that's "engineered enough" — not under-engineered (fragile, hacky) and not over-engineered (premature abstraction, unnecessary complexity). * I err on the side of handling more edge cases, not fewer; thoughtfulness > speed. * Bias toward explicit over clever. -* Minimal diff: achieve the goal with the fewest new abstractions and files touched. +* Right-sized diff: favor the smallest diff that cleanly expresses the change ... but don't compress a necessary rewrite into a minimal patch. If the existing foundation is broken, say "scrap it and do this instead." ## Cognitive Patterns — How Great Eng Managers Think @@ -1075,7 +1075,7 @@ THE PLAN: ```bash TMPERR_PV=$(mktemp /tmp/codex-planreview-XXXXXXXX) _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; } -codex exec "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached 2>"$TMPERR_PV" +codex exec "" -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_PV" ``` Use a 5-minute timeout (`timeout: 300000`). After the command completes, read stderr: diff --git a/plan-eng-review/SKILL.md.tmpl b/plan-eng-review/SKILL.md.tmpl index dab83e72..a6a8bdd4 100644 --- a/plan-eng-review/SKILL.md.tmpl +++ b/plan-eng-review/SKILL.md.tmpl @@ -45,7 +45,7 @@ If the user asks you to compress or the system triggers context compaction: Step * I want code that's "engineered enough" — not under-engineered (fragile, hacky) and not over-engineered (premature abstraction, unnecessary complexity). * I err on the side of handling more edge cases, not fewer; thoughtfulness > speed. * Bias toward explicit over clever. -* Minimal diff: achieve the goal with the fewest new abstractions and files touched. +* Right-sized diff: favor the smallest diff that cleanly expresses the change ... but don't compress a necessary rewrite into a minimal patch. If the existing foundation is broken, say "scrap it and do this instead." ## Cognitive Patterns — How Great Eng Managers Think