From d05890319e805532b29e08b13db34d95a2677287 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 8 Jun 2026 05:50:23 -0700 Subject: [PATCH] refactor(review): compress unresolved-status prose to fit parity budget After merging origin/main (v1.57.3.0), plan-devex-review exceeded the 1.05x parity ratio vs the v1.53.0.0 baseline. Rather than rebase the baseline, compressed the new prose to stay under the cap honestly: the report's unresolved-status block (~32 -> ~9 lines) and the EXIT PLAN MODE GATE's final-line check (~7 -> ~5 lines), plus the plan-devex-review review-log step. All load-bearing rules and the exact gate-checkable tokens are preserved; the static guards in gen-skill-docs.test.ts still pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- codex/SKILL.md | 55 +++++----------- devex-review/SKILL.md | 41 +++--------- plan-ceo-review/SKILL.md | 14 ++--- plan-ceo-review/sections/review-sections.md | 41 +++--------- plan-design-review/SKILL.md | 14 ++--- .../sections/review-sections.md | 41 +++--------- plan-devex-review/SKILL.md | 14 ++--- plan-devex-review/sections/review-sections.md | 62 ++++--------------- .../sections/review-sections.md.tmpl | 21 +------ plan-eng-review/SKILL.md | 14 ++--- plan-eng-review/sections/review-sections.md | 41 +++--------- scripts/resolvers/review.ts | 55 +++++----------- test/gen-skill-docs.test.ts | 4 +- 13 files changed, 107 insertions(+), 310 deletions(-) diff --git a/codex/SKILL.md b/codex/SKILL.md index 6429f01b6..4a6d6276f 100644 --- a/codex/SKILL.md +++ b/codex/SKILL.md @@ -1113,38 +1113,15 @@ empty); **VERDICT** is always present: - **VERDICT:** list reviews that are CLEAR (e.g., "CEO + ENG CLEARED — ready to implement"). If Eng Review is not CLEAR and not skipped globally, append "eng review required". -**Unresolved-decisions status (MANDATORY — the final content of every report).** -After the VERDICT line, the report MUST end with an unresolved-decisions status. This is -never omitted — it does NOT fall under the "omit when empty" rule above. It is the final -non-whitespace line(s) of the report section, written as content under the -\`## GSTACK REVIEW REPORT\` heading (a bold label — never a new \`## \` heading, -which would break the terminal-heading requirement). It is the last thing the user reads -before the approval prompt: either what is still open, or an explicit all-clear. - -Emit exactly one of: - -- **None open:** a single line containing exactly \`NO UNRESOLVED DECISIONS\`, unbolded - and with no markdown emphasis. A bolded \`**NO UNRESOLVED DECISIONS**\` does NOT count - — keep it plain so the gate's exact-line check can't be gamed by emphasis. -- **Some open:** a bold header line \`**UNRESOLVED DECISIONS:**\` followed by one bullet - per unresolved item; the last bullet is the final line of the report. Each bullet names - the decision and what breaks if it ships deferred (e.g. - "- Auth provider choice — if deferred: the login flow can't be built"). If prior - reviews carry unresolved items, add one final bullet - "+ N unresolved from prior reviews — see review rows above". - -Compute the status (this avoids double-counting the current review): -1. **Current-review items:** list every unanswered AskUserQuestion / deferred decision - from THIS review's Unresolved Decisions section, one bullet each. You have these in - context — do not reconstruct them from the log. -2. **Prior-reviews count:** the log stores only \`unresolved:N\` counts, and the current - review's row is already written before this report runs. Take the latest fresh row per - review skill (same 7-day window as the dashboard), DROP the current skill's row - entirely (do NOT backfill an older row for it), and sum \`unresolved\` across the - rest. Add the "+ N unresolved from prior reviews" bullet only when that sum is > 0. - Never sum all log rows — reruns would double-count. -3. **Sentinel rule:** emit \`NO UNRESOLVED DECISIONS\` if and only if step 1 produced - zero items AND the step 2 count is zero. +**Unresolved-decisions status (MANDATORY — never omitted; the report's final non-whitespace +line).** After VERDICT, end the report (content under the \`## GSTACK REVIEW REPORT\` +heading — a bold label, never a new \`## \` heading; exempt from the "omit when empty" +rule) with exactly one: the exact unbolded line \`NO UNRESOLVED DECISIONS\` (a bolded one +does NOT count), OR a \`**UNRESOLVED DECISIONS:**\` header + one bullet per open item +(last bullet = final line; add \`+ N unresolved from prior reviews\` only when N > 0). +This avoids double-counting: list THIS review's open items from context; for prior reviews +sum \`unresolved\` over the latest fresh row per skill (dashboard 7-day window) after you +DROP the current skill's row; emit the sentinel only when both are zero. ### Write to the plan file @@ -1186,15 +1163,13 @@ missing work — do NOT call ExitPlanMode: In-body prose that mentions "outside voice", "codex findings", or similar does NOT count — only the structured `## GSTACK REVIEW REPORT` section satisfies this check. -3. Confirm the report contains a Runs / Status / Findings table and a VERDICT - line (CODEX / CROSS-MODEL lines are absorbed if applicable). +3. Confirm the report has a Runs / Status / Findings table and a VERDICT line + (CODEX / CROSS-MODEL absorbed if applicable). 4. Confirm the report's FINAL non-whitespace line is the unresolved-decisions - status — either the exact unbolded sentinel `NO UNRESOLVED DECISIONS`, or a - bullet belonging to a `**UNRESOLVED DECISIONS:**` block that has at least one - bullet. This check is BLOCKING with no "if applicable" escape: a report with no - unresolved status, or with anything (CODEX / CROSS-MODEL / VERDICT / prose) - after it, FAILS the gate. A bolded `**NO UNRESOLVED DECISIONS**` does NOT - satisfy the sentinel — it must be the plain unbolded line. + status: the exact unbolded `NO UNRESOLVED DECISIONS`, or a bullet of a final + `**UNRESOLVED DECISIONS:**` block. BLOCKING, no "if applicable" escape — a + bolded sentinel, any trailing CODEX/CROSS-MODEL/VERDICT/prose, or a missing + status each FAILS the gate. 5. If a plan file is in context for this skill invocation: confirm `gstack-review-log` was called and `gstack-review-read` was run at least once. If no plan file is in context (e.g. `/codex consult` against a diff --git a/devex-review/SKILL.md b/devex-review/SKILL.md index 377b1adc5..ac414de60 100644 --- a/devex-review/SKILL.md +++ b/devex-review/SKILL.md @@ -1177,38 +1177,15 @@ empty); **VERDICT** is always present: - **VERDICT:** list reviews that are CLEAR (e.g., "CEO + ENG CLEARED — ready to implement"). If Eng Review is not CLEAR and not skipped globally, append "eng review required". -**Unresolved-decisions status (MANDATORY — the final content of every report).** -After the VERDICT line, the report MUST end with an unresolved-decisions status. This is -never omitted — it does NOT fall under the "omit when empty" rule above. It is the final -non-whitespace line(s) of the report section, written as content under the -\`## GSTACK REVIEW REPORT\` heading (a bold label — never a new \`## \` heading, -which would break the terminal-heading requirement). It is the last thing the user reads -before the approval prompt: either what is still open, or an explicit all-clear. - -Emit exactly one of: - -- **None open:** a single line containing exactly \`NO UNRESOLVED DECISIONS\`, unbolded - and with no markdown emphasis. A bolded \`**NO UNRESOLVED DECISIONS**\` does NOT count - — keep it plain so the gate's exact-line check can't be gamed by emphasis. -- **Some open:** a bold header line \`**UNRESOLVED DECISIONS:**\` followed by one bullet - per unresolved item; the last bullet is the final line of the report. Each bullet names - the decision and what breaks if it ships deferred (e.g. - "- Auth provider choice — if deferred: the login flow can't be built"). If prior - reviews carry unresolved items, add one final bullet - "+ N unresolved from prior reviews — see review rows above". - -Compute the status (this avoids double-counting the current review): -1. **Current-review items:** list every unanswered AskUserQuestion / deferred decision - from THIS review's Unresolved Decisions section, one bullet each. You have these in - context — do not reconstruct them from the log. -2. **Prior-reviews count:** the log stores only \`unresolved:N\` counts, and the current - review's row is already written before this report runs. Take the latest fresh row per - review skill (same 7-day window as the dashboard), DROP the current skill's row - entirely (do NOT backfill an older row for it), and sum \`unresolved\` across the - rest. Add the "+ N unresolved from prior reviews" bullet only when that sum is > 0. - Never sum all log rows — reruns would double-count. -3. **Sentinel rule:** emit \`NO UNRESOLVED DECISIONS\` if and only if step 1 produced - zero items AND the step 2 count is zero. +**Unresolved-decisions status (MANDATORY — never omitted; the report's final non-whitespace +line).** After VERDICT, end the report (content under the \`## GSTACK REVIEW REPORT\` +heading — a bold label, never a new \`## \` heading; exempt from the "omit when empty" +rule) with exactly one: the exact unbolded line \`NO UNRESOLVED DECISIONS\` (a bolded one +does NOT count), OR a \`**UNRESOLVED DECISIONS:**\` header + one bullet per open item +(last bullet = final line; add \`+ N unresolved from prior reviews\` only when N > 0). +This avoids double-counting: list THIS review's open items from context; for prior reviews +sum \`unresolved\` over the latest fresh row per skill (dashboard 7-day window) after you +DROP the current skill's row; emit the sentinel only when both are zero. ### Write to the plan file diff --git a/plan-ceo-review/SKILL.md b/plan-ceo-review/SKILL.md index f7c63dd5d..a33a874db 100644 --- a/plan-ceo-review/SKILL.md +++ b/plan-ceo-review/SKILL.md @@ -1406,15 +1406,13 @@ missing work — do NOT call ExitPlanMode: In-body prose that mentions "outside voice", "codex findings", or similar does NOT count — only the structured `## GSTACK REVIEW REPORT` section satisfies this check. -3. Confirm the report contains a Runs / Status / Findings table and a VERDICT - line (CODEX / CROSS-MODEL lines are absorbed if applicable). +3. Confirm the report has a Runs / Status / Findings table and a VERDICT line + (CODEX / CROSS-MODEL absorbed if applicable). 4. Confirm the report's FINAL non-whitespace line is the unresolved-decisions - status — either the exact unbolded sentinel `NO UNRESOLVED DECISIONS`, or a - bullet belonging to a `**UNRESOLVED DECISIONS:**` block that has at least one - bullet. This check is BLOCKING with no "if applicable" escape: a report with no - unresolved status, or with anything (CODEX / CROSS-MODEL / VERDICT / prose) - after it, FAILS the gate. A bolded `**NO UNRESOLVED DECISIONS**` does NOT - satisfy the sentinel — it must be the plain unbolded line. + status: the exact unbolded `NO UNRESOLVED DECISIONS`, or a bullet of a final + `**UNRESOLVED DECISIONS:**` block. BLOCKING, no "if applicable" escape — a + bolded sentinel, any trailing CODEX/CROSS-MODEL/VERDICT/prose, or a missing + status each FAILS the gate. 5. If a plan file is in context for this skill invocation: confirm `gstack-review-log` was called and `gstack-review-read` was run at least once. If no plan file is in context (e.g. `/codex consult` against a diff --git a/plan-ceo-review/sections/review-sections.md b/plan-ceo-review/sections/review-sections.md index afce3c990..da490be42 100644 --- a/plan-ceo-review/sections/review-sections.md +++ b/plan-ceo-review/sections/review-sections.md @@ -717,38 +717,15 @@ empty); **VERDICT** is always present: - **VERDICT:** list reviews that are CLEAR (e.g., "CEO + ENG CLEARED — ready to implement"). If Eng Review is not CLEAR and not skipped globally, append "eng review required". -**Unresolved-decisions status (MANDATORY — the final content of every report).** -After the VERDICT line, the report MUST end with an unresolved-decisions status. This is -never omitted — it does NOT fall under the "omit when empty" rule above. It is the final -non-whitespace line(s) of the report section, written as content under the -\`## GSTACK REVIEW REPORT\` heading (a bold label — never a new \`## \` heading, -which would break the terminal-heading requirement). It is the last thing the user reads -before the approval prompt: either what is still open, or an explicit all-clear. - -Emit exactly one of: - -- **None open:** a single line containing exactly \`NO UNRESOLVED DECISIONS\`, unbolded - and with no markdown emphasis. A bolded \`**NO UNRESOLVED DECISIONS**\` does NOT count - — keep it plain so the gate's exact-line check can't be gamed by emphasis. -- **Some open:** a bold header line \`**UNRESOLVED DECISIONS:**\` followed by one bullet - per unresolved item; the last bullet is the final line of the report. Each bullet names - the decision and what breaks if it ships deferred (e.g. - "- Auth provider choice — if deferred: the login flow can't be built"). If prior - reviews carry unresolved items, add one final bullet - "+ N unresolved from prior reviews — see review rows above". - -Compute the status (this avoids double-counting the current review): -1. **Current-review items:** list every unanswered AskUserQuestion / deferred decision - from THIS review's Unresolved Decisions section, one bullet each. You have these in - context — do not reconstruct them from the log. -2. **Prior-reviews count:** the log stores only \`unresolved:N\` counts, and the current - review's row is already written before this report runs. Take the latest fresh row per - review skill (same 7-day window as the dashboard), DROP the current skill's row - entirely (do NOT backfill an older row for it), and sum \`unresolved\` across the - rest. Add the "+ N unresolved from prior reviews" bullet only when that sum is > 0. - Never sum all log rows — reruns would double-count. -3. **Sentinel rule:** emit \`NO UNRESOLVED DECISIONS\` if and only if step 1 produced - zero items AND the step 2 count is zero. +**Unresolved-decisions status (MANDATORY — never omitted; the report's final non-whitespace +line).** After VERDICT, end the report (content under the \`## GSTACK REVIEW REPORT\` +heading — a bold label, never a new \`## \` heading; exempt from the "omit when empty" +rule) with exactly one: the exact unbolded line \`NO UNRESOLVED DECISIONS\` (a bolded one +does NOT count), OR a \`**UNRESOLVED DECISIONS:**\` header + one bullet per open item +(last bullet = final line; add \`+ N unresolved from prior reviews\` only when N > 0). +This avoids double-counting: list THIS review's open items from context; for prior reviews +sum \`unresolved\` over the latest fresh row per skill (dashboard 7-day window) after you +DROP the current skill's row; emit the sentinel only when both are zero. ### Write to the plan file diff --git a/plan-design-review/SKILL.md b/plan-design-review/SKILL.md index 38de6d5dc..81acc3c04 100644 --- a/plan-design-review/SKILL.md +++ b/plan-design-review/SKILL.md @@ -1427,15 +1427,13 @@ missing work — do NOT call ExitPlanMode: In-body prose that mentions "outside voice", "codex findings", or similar does NOT count — only the structured `## GSTACK REVIEW REPORT` section satisfies this check. -3. Confirm the report contains a Runs / Status / Findings table and a VERDICT - line (CODEX / CROSS-MODEL lines are absorbed if applicable). +3. Confirm the report has a Runs / Status / Findings table and a VERDICT line + (CODEX / CROSS-MODEL absorbed if applicable). 4. Confirm the report's FINAL non-whitespace line is the unresolved-decisions - status — either the exact unbolded sentinel `NO UNRESOLVED DECISIONS`, or a - bullet belonging to a `**UNRESOLVED DECISIONS:**` block that has at least one - bullet. This check is BLOCKING with no "if applicable" escape: a report with no - unresolved status, or with anything (CODEX / CROSS-MODEL / VERDICT / prose) - after it, FAILS the gate. A bolded `**NO UNRESOLVED DECISIONS**` does NOT - satisfy the sentinel — it must be the plain unbolded line. + status: the exact unbolded `NO UNRESOLVED DECISIONS`, or a bullet of a final + `**UNRESOLVED DECISIONS:**` block. BLOCKING, no "if applicable" escape — a + bolded sentinel, any trailing CODEX/CROSS-MODEL/VERDICT/prose, or a missing + status each FAILS the gate. 5. If a plan file is in context for this skill invocation: confirm `gstack-review-log` was called and `gstack-review-read` was run at least once. If no plan file is in context (e.g. `/codex consult` against a diff --git a/plan-design-review/sections/review-sections.md b/plan-design-review/sections/review-sections.md index 2b291715e..fde4b79f9 100644 --- a/plan-design-review/sections/review-sections.md +++ b/plan-design-review/sections/review-sections.md @@ -466,38 +466,15 @@ empty); **VERDICT** is always present: - **VERDICT:** list reviews that are CLEAR (e.g., "CEO + ENG CLEARED — ready to implement"). If Eng Review is not CLEAR and not skipped globally, append "eng review required". -**Unresolved-decisions status (MANDATORY — the final content of every report).** -After the VERDICT line, the report MUST end with an unresolved-decisions status. This is -never omitted — it does NOT fall under the "omit when empty" rule above. It is the final -non-whitespace line(s) of the report section, written as content under the -\`## GSTACK REVIEW REPORT\` heading (a bold label — never a new \`## \` heading, -which would break the terminal-heading requirement). It is the last thing the user reads -before the approval prompt: either what is still open, or an explicit all-clear. - -Emit exactly one of: - -- **None open:** a single line containing exactly \`NO UNRESOLVED DECISIONS\`, unbolded - and with no markdown emphasis. A bolded \`**NO UNRESOLVED DECISIONS**\` does NOT count - — keep it plain so the gate's exact-line check can't be gamed by emphasis. -- **Some open:** a bold header line \`**UNRESOLVED DECISIONS:**\` followed by one bullet - per unresolved item; the last bullet is the final line of the report. Each bullet names - the decision and what breaks if it ships deferred (e.g. - "- Auth provider choice — if deferred: the login flow can't be built"). If prior - reviews carry unresolved items, add one final bullet - "+ N unresolved from prior reviews — see review rows above". - -Compute the status (this avoids double-counting the current review): -1. **Current-review items:** list every unanswered AskUserQuestion / deferred decision - from THIS review's Unresolved Decisions section, one bullet each. You have these in - context — do not reconstruct them from the log. -2. **Prior-reviews count:** the log stores only \`unresolved:N\` counts, and the current - review's row is already written before this report runs. Take the latest fresh row per - review skill (same 7-day window as the dashboard), DROP the current skill's row - entirely (do NOT backfill an older row for it), and sum \`unresolved\` across the - rest. Add the "+ N unresolved from prior reviews" bullet only when that sum is > 0. - Never sum all log rows — reruns would double-count. -3. **Sentinel rule:** emit \`NO UNRESOLVED DECISIONS\` if and only if step 1 produced - zero items AND the step 2 count is zero. +**Unresolved-decisions status (MANDATORY — never omitted; the report's final non-whitespace +line).** After VERDICT, end the report (content under the \`## GSTACK REVIEW REPORT\` +heading — a bold label, never a new \`## \` heading; exempt from the "omit when empty" +rule) with exactly one: the exact unbolded line \`NO UNRESOLVED DECISIONS\` (a bolded one +does NOT count), OR a \`**UNRESOLVED DECISIONS:**\` header + one bullet per open item +(last bullet = final line; add \`+ N unresolved from prior reviews\` only when N > 0). +This avoids double-counting: list THIS review's open items from context; for prior reviews +sum \`unresolved\` over the latest fresh row per skill (dashboard 7-day window) after you +DROP the current skill's row; emit the sentinel only when both are zero. ### Write to the plan file diff --git a/plan-devex-review/SKILL.md b/plan-devex-review/SKILL.md index 6e2b0f28d..5a30c3f6f 100644 --- a/plan-devex-review/SKILL.md +++ b/plan-devex-review/SKILL.md @@ -1390,15 +1390,13 @@ missing work — do NOT call ExitPlanMode: In-body prose that mentions "outside voice", "codex findings", or similar does NOT count — only the structured `## GSTACK REVIEW REPORT` section satisfies this check. -3. Confirm the report contains a Runs / Status / Findings table and a VERDICT - line (CODEX / CROSS-MODEL lines are absorbed if applicable). +3. Confirm the report has a Runs / Status / Findings table and a VERDICT line + (CODEX / CROSS-MODEL absorbed if applicable). 4. Confirm the report's FINAL non-whitespace line is the unresolved-decisions - status — either the exact unbolded sentinel `NO UNRESOLVED DECISIONS`, or a - bullet belonging to a `**UNRESOLVED DECISIONS:**` block that has at least one - bullet. This check is BLOCKING with no "if applicable" escape: a report with no - unresolved status, or with anything (CODEX / CROSS-MODEL / VERDICT / prose) - after it, FAILS the gate. A bolded `**NO UNRESOLVED DECISIONS**` does NOT - satisfy the sentinel — it must be the plain unbolded line. + status: the exact unbolded `NO UNRESOLVED DECISIONS`, or a bullet of a final + `**UNRESOLVED DECISIONS:**` block. BLOCKING, no "if applicable" escape — a + bolded sentinel, any trailing CODEX/CROSS-MODEL/VERDICT/prose, or a missing + status each FAILS the gate. 5. If a plan file is in context for this skill invocation: confirm `gstack-review-log` was called and `gstack-review-read` was run at least once. If no plan file is in context (e.g. `/codex consult` against a diff --git a/plan-devex-review/sections/review-sections.md b/plan-devex-review/sections/review-sections.md index 5979acd86..9c588f201 100644 --- a/plan-devex-review/sections/review-sections.md +++ b/plan-devex-review/sections/review-sections.md @@ -578,29 +578,14 @@ If any AskUserQuestion goes unanswered, note here. Never silently default. ## Review Log -After producing the DX Scorecard above, persist the review result. Without this step -the Review Readiness Dashboard and the GSTACK REVIEW REPORT have no plan-devex-review -data to read, and the EXIT PLAN MODE GATE's "review log was called" check cannot pass. - -**PLAN MODE EXCEPTION — ALWAYS RUN:** This command writes review metadata to -`~/.gstack/` (user config directory, not project files). The skill preamble -already writes to `~/.gstack/sessions/` and `~/.gstack/analytics/` — this is -the same pattern. The review dashboard depends on this data. Skipping this -command breaks the review readiness dashboard in /ship. +Persist after the DX Scorecard — the dashboard, the GSTACK REVIEW REPORT, and the EXIT +PLAN MODE GATE's "review log was called" check depend on it. **PLAN MODE EXCEPTION — ALWAYS RUN** (writes to `~/.gstack/`, not project files): ```bash ~/.claude/skills/gstack/bin/gstack-review-log '{"skill":"plan-devex-review","timestamp":"TIMESTAMP","status":"STATUS","initial_score":N,"overall_score":N,"product_type":"PRODUCT_TYPE","tthw_current":"TTHW_CURRENT","tthw_target":"TTHW_TARGET","mode":"MODE","persona":"PERSONA","competitive_tier":"COMPETITIVE_TIER","unresolved":N,"commit":"COMMIT"}' ``` -Substitute values from the DX Scorecard and Step 0 setup: -- **TIMESTAMP**: current ISO 8601 datetime -- **STATUS**: "clean" if overall score 8+ AND 0 unresolved; otherwise "issues_open" -- **initial_score**: initial overall DX score before fixes (0-10) -- **overall_score**: final overall DX score after fixes (0-10) -- **product_type / persona / competitive_tier / mode**: from Step 0 setup -- **tthw_current / tthw_target**: time-to-hello-world now vs target -- **unresolved**: number of unresolved decisions -- **COMMIT**: output of `git rev-parse --short HEAD` +STATUS = "clean" if score 8+ AND 0 unresolved, else "issues_open"; other fields from the DX Scorecard + Step 0; COMMIT = `git rev-parse --short HEAD`. ## Review Readiness Dashboard @@ -709,38 +694,15 @@ empty); **VERDICT** is always present: - **VERDICT:** list reviews that are CLEAR (e.g., "CEO + ENG CLEARED — ready to implement"). If Eng Review is not CLEAR and not skipped globally, append "eng review required". -**Unresolved-decisions status (MANDATORY — the final content of every report).** -After the VERDICT line, the report MUST end with an unresolved-decisions status. This is -never omitted — it does NOT fall under the "omit when empty" rule above. It is the final -non-whitespace line(s) of the report section, written as content under the -\`## GSTACK REVIEW REPORT\` heading (a bold label — never a new \`## \` heading, -which would break the terminal-heading requirement). It is the last thing the user reads -before the approval prompt: either what is still open, or an explicit all-clear. - -Emit exactly one of: - -- **None open:** a single line containing exactly \`NO UNRESOLVED DECISIONS\`, unbolded - and with no markdown emphasis. A bolded \`**NO UNRESOLVED DECISIONS**\` does NOT count - — keep it plain so the gate's exact-line check can't be gamed by emphasis. -- **Some open:** a bold header line \`**UNRESOLVED DECISIONS:**\` followed by one bullet - per unresolved item; the last bullet is the final line of the report. Each bullet names - the decision and what breaks if it ships deferred (e.g. - "- Auth provider choice — if deferred: the login flow can't be built"). If prior - reviews carry unresolved items, add one final bullet - "+ N unresolved from prior reviews — see review rows above". - -Compute the status (this avoids double-counting the current review): -1. **Current-review items:** list every unanswered AskUserQuestion / deferred decision - from THIS review's Unresolved Decisions section, one bullet each. You have these in - context — do not reconstruct them from the log. -2. **Prior-reviews count:** the log stores only \`unresolved:N\` counts, and the current - review's row is already written before this report runs. Take the latest fresh row per - review skill (same 7-day window as the dashboard), DROP the current skill's row - entirely (do NOT backfill an older row for it), and sum \`unresolved\` across the - rest. Add the "+ N unresolved from prior reviews" bullet only when that sum is > 0. - Never sum all log rows — reruns would double-count. -3. **Sentinel rule:** emit \`NO UNRESOLVED DECISIONS\` if and only if step 1 produced - zero items AND the step 2 count is zero. +**Unresolved-decisions status (MANDATORY — never omitted; the report's final non-whitespace +line).** After VERDICT, end the report (content under the \`## GSTACK REVIEW REPORT\` +heading — a bold label, never a new \`## \` heading; exempt from the "omit when empty" +rule) with exactly one: the exact unbolded line \`NO UNRESOLVED DECISIONS\` (a bolded one +does NOT count), OR a \`**UNRESOLVED DECISIONS:**\` header + one bullet per open item +(last bullet = final line; add \`+ N unresolved from prior reviews\` only when N > 0). +This avoids double-counting: list THIS review's open items from context; for prior reviews +sum \`unresolved\` over the latest fresh row per skill (dashboard 7-day window) after you +DROP the current skill's row; emit the sentinel only when both are zero. ### Write to the plan file diff --git a/plan-devex-review/sections/review-sections.md.tmpl b/plan-devex-review/sections/review-sections.md.tmpl index cfb3e7968..a047d8f65 100644 --- a/plan-devex-review/sections/review-sections.md.tmpl +++ b/plan-devex-review/sections/review-sections.md.tmpl @@ -336,29 +336,14 @@ If any AskUserQuestion goes unanswered, note here. Never silently default. ## Review Log -After producing the DX Scorecard above, persist the review result. Without this step -the Review Readiness Dashboard and the GSTACK REVIEW REPORT have no plan-devex-review -data to read, and the EXIT PLAN MODE GATE's "review log was called" check cannot pass. - -**PLAN MODE EXCEPTION — ALWAYS RUN:** This command writes review metadata to -`~/.gstack/` (user config directory, not project files). The skill preamble -already writes to `~/.gstack/sessions/` and `~/.gstack/analytics/` — this is -the same pattern. The review dashboard depends on this data. Skipping this -command breaks the review readiness dashboard in /ship. +Persist after the DX Scorecard — the dashboard, the GSTACK REVIEW REPORT, and the EXIT +PLAN MODE GATE's "review log was called" check depend on it. **PLAN MODE EXCEPTION — ALWAYS RUN** (writes to `~/.gstack/`, not project files): ```bash ~/.claude/skills/gstack/bin/gstack-review-log '{"skill":"plan-devex-review","timestamp":"TIMESTAMP","status":"STATUS","initial_score":N,"overall_score":N,"product_type":"PRODUCT_TYPE","tthw_current":"TTHW_CURRENT","tthw_target":"TTHW_TARGET","mode":"MODE","persona":"PERSONA","competitive_tier":"COMPETITIVE_TIER","unresolved":N,"commit":"COMMIT"}' ``` -Substitute values from the DX Scorecard and Step 0 setup: -- **TIMESTAMP**: current ISO 8601 datetime -- **STATUS**: "clean" if overall score 8+ AND 0 unresolved; otherwise "issues_open" -- **initial_score**: initial overall DX score before fixes (0-10) -- **overall_score**: final overall DX score after fixes (0-10) -- **product_type / persona / competitive_tier / mode**: from Step 0 setup -- **tthw_current / tthw_target**: time-to-hello-world now vs target -- **unresolved**: number of unresolved decisions -- **COMMIT**: output of `git rev-parse --short HEAD` +STATUS = "clean" if score 8+ AND 0 unresolved, else "issues_open"; other fields from the DX Scorecard + Step 0; COMMIT = `git rev-parse --short HEAD`. {{REVIEW_DASHBOARD}} diff --git a/plan-eng-review/SKILL.md b/plan-eng-review/SKILL.md index aa02ef39c..711be1421 100644 --- a/plan-eng-review/SKILL.md +++ b/plan-eng-review/SKILL.md @@ -962,15 +962,13 @@ missing work — do NOT call ExitPlanMode: In-body prose that mentions "outside voice", "codex findings", or similar does NOT count — only the structured `## GSTACK REVIEW REPORT` section satisfies this check. -3. Confirm the report contains a Runs / Status / Findings table and a VERDICT - line (CODEX / CROSS-MODEL lines are absorbed if applicable). +3. Confirm the report has a Runs / Status / Findings table and a VERDICT line + (CODEX / CROSS-MODEL absorbed if applicable). 4. Confirm the report's FINAL non-whitespace line is the unresolved-decisions - status — either the exact unbolded sentinel `NO UNRESOLVED DECISIONS`, or a - bullet belonging to a `**UNRESOLVED DECISIONS:**` block that has at least one - bullet. This check is BLOCKING with no "if applicable" escape: a report with no - unresolved status, or with anything (CODEX / CROSS-MODEL / VERDICT / prose) - after it, FAILS the gate. A bolded `**NO UNRESOLVED DECISIONS**` does NOT - satisfy the sentinel — it must be the plain unbolded line. + status: the exact unbolded `NO UNRESOLVED DECISIONS`, or a bullet of a final + `**UNRESOLVED DECISIONS:**` block. BLOCKING, no "if applicable" escape — a + bolded sentinel, any trailing CODEX/CROSS-MODEL/VERDICT/prose, or a missing + status each FAILS the gate. 5. If a plan file is in context for this skill invocation: confirm `gstack-review-log` was called and `gstack-review-read` was run at least once. If no plan file is in context (e.g. `/codex consult` against a diff --git a/plan-eng-review/sections/review-sections.md b/plan-eng-review/sections/review-sections.md index d97861c3e..9fc667015 100644 --- a/plan-eng-review/sections/review-sections.md +++ b/plan-eng-review/sections/review-sections.md @@ -771,38 +771,15 @@ empty); **VERDICT** is always present: - **VERDICT:** list reviews that are CLEAR (e.g., "CEO + ENG CLEARED — ready to implement"). If Eng Review is not CLEAR and not skipped globally, append "eng review required". -**Unresolved-decisions status (MANDATORY — the final content of every report).** -After the VERDICT line, the report MUST end with an unresolved-decisions status. This is -never omitted — it does NOT fall under the "omit when empty" rule above. It is the final -non-whitespace line(s) of the report section, written as content under the -\`## GSTACK REVIEW REPORT\` heading (a bold label — never a new \`## \` heading, -which would break the terminal-heading requirement). It is the last thing the user reads -before the approval prompt: either what is still open, or an explicit all-clear. - -Emit exactly one of: - -- **None open:** a single line containing exactly \`NO UNRESOLVED DECISIONS\`, unbolded - and with no markdown emphasis. A bolded \`**NO UNRESOLVED DECISIONS**\` does NOT count - — keep it plain so the gate's exact-line check can't be gamed by emphasis. -- **Some open:** a bold header line \`**UNRESOLVED DECISIONS:**\` followed by one bullet - per unresolved item; the last bullet is the final line of the report. Each bullet names - the decision and what breaks if it ships deferred (e.g. - "- Auth provider choice — if deferred: the login flow can't be built"). If prior - reviews carry unresolved items, add one final bullet - "+ N unresolved from prior reviews — see review rows above". - -Compute the status (this avoids double-counting the current review): -1. **Current-review items:** list every unanswered AskUserQuestion / deferred decision - from THIS review's Unresolved Decisions section, one bullet each. You have these in - context — do not reconstruct them from the log. -2. **Prior-reviews count:** the log stores only \`unresolved:N\` counts, and the current - review's row is already written before this report runs. Take the latest fresh row per - review skill (same 7-day window as the dashboard), DROP the current skill's row - entirely (do NOT backfill an older row for it), and sum \`unresolved\` across the - rest. Add the "+ N unresolved from prior reviews" bullet only when that sum is > 0. - Never sum all log rows — reruns would double-count. -3. **Sentinel rule:** emit \`NO UNRESOLVED DECISIONS\` if and only if step 1 produced - zero items AND the step 2 count is zero. +**Unresolved-decisions status (MANDATORY — never omitted; the report's final non-whitespace +line).** After VERDICT, end the report (content under the \`## GSTACK REVIEW REPORT\` +heading — a bold label, never a new \`## \` heading; exempt from the "omit when empty" +rule) with exactly one: the exact unbolded line \`NO UNRESOLVED DECISIONS\` (a bolded one +does NOT count), OR a \`**UNRESOLVED DECISIONS:**\` header + one bullet per open item +(last bullet = final line; add \`+ N unresolved from prior reviews\` only when N > 0). +This avoids double-counting: list THIS review's open items from context; for prior reviews +sum \`unresolved\` over the latest fresh row per skill (dashboard 7-day window) after you +DROP the current skill's row; emit the sentinel only when both are zero. ### Write to the plan file diff --git a/scripts/resolvers/review.ts b/scripts/resolvers/review.ts index 1b590cff2..be67d43e7 100644 --- a/scripts/resolvers/review.ts +++ b/scripts/resolvers/review.ts @@ -127,38 +127,15 @@ empty); **VERDICT** is always present: - **VERDICT:** list reviews that are CLEAR (e.g., "CEO + ENG CLEARED — ready to implement"). If Eng Review is not CLEAR and not skipped globally, append "eng review required". -**Unresolved-decisions status (MANDATORY — the final content of every report).** -After the VERDICT line, the report MUST end with an unresolved-decisions status. This is -never omitted — it does NOT fall under the "omit when empty" rule above. It is the final -non-whitespace line(s) of the report section, written as content under the -\\\`## GSTACK REVIEW REPORT\\\` heading (a bold label — never a new \\\`## \\\` heading, -which would break the terminal-heading requirement). It is the last thing the user reads -before the approval prompt: either what is still open, or an explicit all-clear. - -Emit exactly one of: - -- **None open:** a single line containing exactly \\\`NO UNRESOLVED DECISIONS\\\`, unbolded - and with no markdown emphasis. A bolded \\\`**NO UNRESOLVED DECISIONS**\\\` does NOT count - — keep it plain so the gate's exact-line check can't be gamed by emphasis. -- **Some open:** a bold header line \\\`**UNRESOLVED DECISIONS:**\\\` followed by one bullet - per unresolved item; the last bullet is the final line of the report. Each bullet names - the decision and what breaks if it ships deferred (e.g. - "- Auth provider choice — if deferred: the login flow can't be built"). If prior - reviews carry unresolved items, add one final bullet - "+ N unresolved from prior reviews — see review rows above". - -Compute the status (this avoids double-counting the current review): -1. **Current-review items:** list every unanswered AskUserQuestion / deferred decision - from THIS review's Unresolved Decisions section, one bullet each. You have these in - context — do not reconstruct them from the log. -2. **Prior-reviews count:** the log stores only \\\`unresolved:N\\\` counts, and the current - review's row is already written before this report runs. Take the latest fresh row per - review skill (same 7-day window as the dashboard), DROP the current skill's row - entirely (do NOT backfill an older row for it), and sum \\\`unresolved\\\` across the - rest. Add the "+ N unresolved from prior reviews" bullet only when that sum is > 0. - Never sum all log rows — reruns would double-count. -3. **Sentinel rule:** emit \\\`NO UNRESOLVED DECISIONS\\\` if and only if step 1 produced - zero items AND the step 2 count is zero. +**Unresolved-decisions status (MANDATORY — never omitted; the report's final non-whitespace +line).** After VERDICT, end the report (content under the \\\`## GSTACK REVIEW REPORT\\\` +heading — a bold label, never a new \\\`## \\\` heading; exempt from the "omit when empty" +rule) with exactly one: the exact unbolded line \\\`NO UNRESOLVED DECISIONS\\\` (a bolded one +does NOT count), OR a \\\`**UNRESOLVED DECISIONS:**\\\` header + one bullet per open item +(last bullet = final line; add \\\`+ N unresolved from prior reviews\\\` only when N > 0). +This avoids double-counting: list THIS review's open items from context; for prior reviews +sum \\\`unresolved\\\` over the latest fresh row per skill (dashboard 7-day window) after you +DROP the current skill's row; emit the sentinel only when both are zero. ### Write to the plan file @@ -202,15 +179,13 @@ missing work — do NOT call ExitPlanMode: In-body prose that mentions "outside voice", "codex findings", or similar does NOT count — only the structured \`## GSTACK REVIEW REPORT\` section satisfies this check. -3. Confirm the report contains a Runs / Status / Findings table and a VERDICT - line (CODEX / CROSS-MODEL lines are absorbed if applicable). +3. Confirm the report has a Runs / Status / Findings table and a VERDICT line + (CODEX / CROSS-MODEL absorbed if applicable). 4. Confirm the report's FINAL non-whitespace line is the unresolved-decisions - status — either the exact unbolded sentinel \`NO UNRESOLVED DECISIONS\`, or a - bullet belonging to a \`**UNRESOLVED DECISIONS:**\` block that has at least one - bullet. This check is BLOCKING with no "if applicable" escape: a report with no - unresolved status, or with anything (CODEX / CROSS-MODEL / VERDICT / prose) - after it, FAILS the gate. A bolded \`**NO UNRESOLVED DECISIONS**\` does NOT - satisfy the sentinel — it must be the plain unbolded line. + status: the exact unbolded \`NO UNRESOLVED DECISIONS\`, or a bullet of a final + \`**UNRESOLVED DECISIONS:**\` block. BLOCKING, no "if applicable" escape — a + bolded sentinel, any trailing CODEX/CROSS-MODEL/VERDICT/prose, or a missing + status each FAILS the gate. 5. If a plan file is in context for this skill invocation: confirm \`gstack-review-log\` was called and \`gstack-review-read\` was run at least once. If no plan file is in context (e.g. \`/codex consult\` against a diff --git a/test/gen-skill-docs.test.ts b/test/gen-skill-docs.test.ts index 818058059..431209a7f 100644 --- a/test/gen-skill-docs.test.ts +++ b/test/gen-skill-docs.test.ts @@ -3269,8 +3269,8 @@ describe('GSTACK REVIEW REPORT mandatory unresolved-decisions status', () => { // The "never omit / always final" contract must be present, not just the phrase. expect(content).toContain('Unresolved-decisions status (MANDATORY'); expect(content).toMatch(/never omitted/); - // \s+ tolerates the prose line-wrap between "final" and "non-whitespace". - expect(content).toMatch(/final\s+non-whitespace line/); + // \s+ tolerates prose line-wraps within "final non-whitespace line". + expect(content).toMatch(/final\s+non-whitespace\s+line/); }); }