diff --git a/CHANGELOG.md b/CHANGELOG.md index 351e7cabd..132a673ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,28 @@ # Changelog +## [1.61.0.0] - 2026-07-16 + +## **`/plan-eng-review` and `/plan-ceo-review` stop pushing you toward fewer tables and JSONField shortcuts — without swinging to the opposite extreme.** + +Both skills' "right-sized diff" preference was quietly biased against schema normalization: adding a new model touches more files than adding a column, so reviewers kept steering people toward merging new fields into existing tables and reaching for JSONField as a polymorphism shortcut. This release adds an explicit data-model exception — normalize first, denormalize when you have a measured reason — plus a full "Data model review checklist" in `/plan-eng-review`'s Architecture Review (SRP, nullable-semantic smells, hidden polymorphism, FK deletion strategy, and seven more). Early feedback flagged the fix as too absolute in the other direction, so this release also adds explicit counterexamples: legitimate JSONField uses (caching a third-party webhook payload verbatim, an open-ended preference bag), denormalization backed by a real measurement (a profiled hot path, a load test), and the case where keeping a single trivial field inline is genuinely the right, minimal call. + +### What this means for you + +Ask `/plan-eng-review` to review a schema change and it will push back on merging polymorphic data into an existing model or reaching for JSONField when the variants are already known — but it will also recognize when JSONField is the correct choice (a verbatim third-party payload cache), when denormalizing is justified (you've measured the bottleneck), and when a single new field genuinely doesn't need its own table. + +### Itemized changes + +#### Changed +- `/plan-eng-review` and `/plan-ceo-review`: added a "data model exception to right-sized diff" — normalize first, denormalize for a measured reason, don't split reflexively when a field has no independent query pattern or consumer. +- `/plan-eng-review` and `/plan-ceo-review`: added explicit JSONField guidance — not an escape hatch for known, stable polymorphism, but legitimately fine for third-party payload caches, open-ended preference bags, and schemas still being discovered. +- `/plan-eng-review`: Architecture Review gained a "Data model honesty" check and a full Data Model Review Checklist (11 proactive checks) that runs on every model touched by a plan, instead of waiting for the user to push back on model shape. +- `/plan-eng-review`: three new cognitive patterns (normalize-first, SRP-for-data-models, structure-beats-blobs-for-known-polymorphism). + +#### For contributors +- New periodic-tier E2E evals exercising all three review-feedback categories end-to-end: a plan that should trigger pushback (polymorphic data inlined with JSONField), one that shouldn't (a legitimate third-party payload cache), one testing measured denormalization is accepted, and one testing a minimal single-field addition isn't over-split. +- New free unit tests for the negation-aware text matcher these evals rely on (`test/helpers/e2e-helpers.test.ts`), covering sentence-boundary scanning, curly-apostrophe contractions, contrastive phrasing ("rather than"/"instead of"), and markdown-bullet clause boundaries — all real gaps found across four rounds of specialist and adversarial review during this branch's own `/ship`. +- Extracted shared E2E fixture setup (`setupPlanEngReviewFixture`) and a hermetic, absolute-path-pinned prompt builder (`planEngReviewDataModelPrompt`) so these new tests can't accidentally grade against a machine's globally-installed skill instead of the branch under test. + ## [1.60.1.0] - 2026-07-09 ## **The /autoplan dual-voice eval is back on the board, catching real regressions.** diff --git a/TODOS.md b/TODOS.md index 29721e6e5..706ba0c23 100644 --- a/TODOS.md +++ b/TODOS.md @@ -735,6 +735,19 @@ scope of that PR; deliberately deferred to keep PTY-import small. ## Testing +## P2: /plan-ceo-review data-model-bias behavioral E2E coverage + +**What:** The v1.61.0.0 schema-consolidation-bias fix (garrytan/gstack#1048) added the same two data-model preference bullets ("right-sized diff" exception + "JSONField not an escape hatch") to both `/plan-eng-review` and `/plan-ceo-review`, but the four new periodic-tier E2E evals (`plan-eng-review-data-model-{bias,legitimate-json,measured-denorm,minimal-change}`) only drive `/plan-eng-review`. `/plan-ceo-review`'s copy of the same bullets has zero behavioral test coverage — only the static grep guardrail in `test/skill-validation.test.ts` confirms the text still exists. + +**Why:** `/plan-ceo-review` runs a materially different workflow (SCOPE EXPANSION / SELECTIVE EXPANSION / HOLD SCOPE modes, its own `sections/review-sections.md`) — identical prose is no guarantee the model actually applies the bias fix the same way it does in `/plan-eng-review`. `touchfiles.ts` doesn't map any test to a change in these bullets for plan-ceo-review, so a regression there would go unnoticed by diff-based test selection entirely. Flagged by the ship-workflow coverage audit during this branch's own `/ship` run. + +**Context:** Mirror the four `plan-eng-review-data-model-*` blocks in `test/skill-e2e-plan.test.ts` for plan-ceo-review — same synthetic plans, same `setupPlanEngReviewFixture`-style hermetic fixture (would need a `plan-ceo-review` variant), same `matchesUnnegated`-guarded regex assertions. plan-ceo-review's HOLD SCOPE mode is the simplest one that still exercises the Architecture Review, per the pattern already used in `test/skill-e2e-plan-ceo-review-section-loading.test.ts`. + +**Priority:** P2. +**Effort:** M (human: ~half day / CC: ~1-2h — mostly copy-adapt of the existing plan-eng-review blocks). + +--- + ## P2: Per-finding AskUserQuestion count assertion for /plan-ceo-review **What:** PTY E2E test that drives /plan-ceo-review through Step 0 with a stable fixture diff containing N known findings, asserts that exactly N distinct AskUserQuestions fire (one per finding) before plan_ready. diff --git a/VERSION b/VERSION index c4190e004..82b3fa4c6 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.60.1.0 +1.61.0.0 diff --git a/package.json b/package.json index 846438a60..9ffb1caf7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.60.1.0", + "version": "1.61.0.0", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module",