v1.57.7.0 feat: GSTACK REVIEW REPORT always declares unresolved decisions#1916
Merged
Conversation
plan-devex-review carried the EXIT PLAN MODE GATE but never wrote a review-log entry, so the gate's 'review log was called' check was structurally unsatisfiable and the Review Readiness Dashboard / GSTACK REVIEW REPORT had no plan-devex-review data to read. Add a Review Log section before the dashboard read, logging the devex fields the report parser already expects (status, scores, product_type, tthw, persona, competitive_tier, unresolved, commit). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…VIEW REPORT
The report's UNRESOLVED line was optional ('omit if empty') and the EXIT
PLAN MODE GATE only checked it 'if applicable', so a plan could ship with
no statement about open decisions at all — a missed ambiguity read
identically to a clean plan. Now every report ends with a mandatory
unresolved-decisions status as its final line: either the exact unbolded
sentinel 'NO UNRESOLVED DECISIONS', or a '**UNRESOLVED DECISIONS:**' block
of bullets. The gate blocks ExitPlanMode unless that final line is present.
generatePlanFileReviewReport: current-review items are listed from context;
prior reviews contribute an aggregate count computed as latest-fresh-row-
per-skill minus the current run (no double-count, dashboard 7-day window).
generateExitPlanModeGate: check #3 is now blocking with no 'if applicable'
escape; bolded sentinel does not satisfy it.
Tests: static guard in gen-skill-docs.test.ts asserts the mandatory status
across all six report consumers and the gate across gate-bearing skills;
skill-e2e-plan.test.ts asserts the written report's final line is the
status (and fixes a stale 'four review rows' -> five-row prompt).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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) <noreply@anthropic.com>
#1909 (v1.57.3.0) added the always-loaded PR-title-version rule to ship's template and committed the regenerated ship/SKILL.md, but did not refresh the three ship golden fixtures, leaving the golden-file regression test red on main. Regenerate them from current output. The diff is purely #1909 content: the PR-title invariant line plus a previously-unresolved ${ctx.paths.binDir} placeholder that current generation correctly resolves. No feature content from this branch leaks into ship (ship does not consume the review report resolvers). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adversarial review caught that compressing the devex review-log block dropped the TIMESTAMP substitution guidance the three sibling plan-review skills carry. A literal "timestamp":"TIMESTAMP" parses as JSON but is an unparseable date, so the Review Readiness Dashboard's 7-day freshness window silently drops the plan-devex-review row (and the report's prior-review aggregation loses it). Restore the one-line instruction. Also: the plan-review-report E2E now derives its last-line check from the report slice, not the whole file, so a mis-placed report surfaces the real trailing content in the failure message. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The v1.53 anchor is four minor versions stale. v1.54-v1.57 (ship/plan carving, carve-guards, AUQ prose fallback, the cross-session decision-log preamble) plus this branch's mandatory unresolved-decisions status line pushed the three plan-review skills past the 5% ratchet even after exhaustive compression. The new baseline captures current UNION sizes (skeleton + sections/*.md, matching what parity-harness measures) so the per-skill 1.05 ratio keeps catching future bloat. The frozen v1.44.1 integrity anchor and the v1.47 size-budget baseline are untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
E2E Evals: ✅ PASS12/12 tests passed | $3.15 total cost | 12 parallel runners
12x ubicloud-standard-8 (Docker: pre-baked toolchain + deps) | wall clock ≈ slowest suite |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Restores something a token-reduction pass had quietly dropped: at plan-approval time, every GSTACK REVIEW REPORT now ends with an explicit unresolved-decisions verdict, so a plan hiding an open question no longer renders identically to a clean one.
Feature — mandatory unresolved-decisions status (
scripts/resolvers/review.ts).generatePlanFileReviewReportnow always ends the report with either the exact unbolded sentinelNO UNRESOLVED DECISIONSor a**UNRESOLVED DECISIONS:**bullet block (one bullet per open item + what breaks if deferred). Never omitted; always the final non-whitespace line. Generated into all six report consumers (plan-ceo / plan-eng / plan-design / plan-devex review,/codex,/devex-review).generateExitPlanModeGatenow blocks ExitPlanMode unless that final line is present (the old "if applicable" escape is gone). The prior-reviews unresolved count is computed without double-counting the just-run review, using the dashboard's 7-day freshness window.Fix —
plan-devex-reviewreview-log. It carried the approval gate but never calledgstack-review-log, so the gate's "review log was called" check was structurally unsatisfiable and its data was invisible to the dashboard/report. It now logs (with the correct ISO timestamp and DX fields).Tests + quality.
test/gen-skill-docs.test.tsassert the mandatory status across all six report consumers and the gate across the five gate-bearing skills, so a future compression pass can't silently drop it again.plan-review-reportE2E (test/skill-e2e-plan.test.ts) to assert the written report's final line is the unresolved status, with nothing after it.Test Coverage
New code paths (report block + blocking gate) covered by static assertions in
test/gen-skill-docs.test.tsacross all consumers + gate skills; runtime behavior covered by the extendedplan-review-reportE2E. Full free suite green (bun test, exit 0).Pre-Landing Review
Adversarial review (Claude subagent) on the diff: 1 FIXABLE finding — the devex review-log had lost its TIMESTAMP fill instruction during compression (an unparseable literal timestamp would drop the row from the 7-day dashboard window). Fixed in commit
7ffccb92. Remaining findings were a minor test-message improvement (applied) and non-issues. This plan also went through/plan-eng-review(clean, 7 findings resolved) and three/codexpasses during planning (19 findings, all incorporated).Design Review
No frontend files changed — design review skipped.
Eval Results
Prompt-template files changed, so the diff selects the plan-review eval set. The merge-blocking gate-tier evals run automatically on this PR's CI (
evals.yml,pull_request,EVALS_TIER: gate). The addedplan-review-reportassertion is periodic-tier (weekly cron), not a PR gate. Not run locally to avoid duplicating CI spend.Plan Completion
Plan items all delivered: mandatory status (resolver + gate), devex review-log fix (CP3), static + E2E tests, parity handling, golden regen. No deferred items.
TODOS
No TODO items completed in this PR.
Test plan
bun test(exit 0) on merged code🤖 Generated with Claude Code