mirror of https://github.com/garrytan/gstack.git
abafb38137
The "minimal diff" engineering preference in plan-eng-review and plan-ceo-review
is correct for bug fixes but systematically biased AI reviewers against schema
normalization: creating a new model touches more files than adding columns, so
the preference pushed recommendations toward merging concerns into existing
tables and using JSONField as a polymorphism escape hatch. Both failures share
one root — "fewer parts feels simpler" at the cost of FK integrity, DB-level
constraints, queryability, and type safety.
Template changes (both skills' preferences lists):
* Data model exception to minimal-diff — schema design normalizes first,
denormalizes for measured reasons, counts concepts not tables.
* JSONField is not an escape hatch for polymorphism — promote to explicit
columns + CheckConstraints when variants are knowable at design time.
plan-eng-review cognitive patterns (renumbered 1–18):
* Normalize first, denormalize for measured reasons (Codd/Knuth/Beck).
* Single Responsibility Principle applies to data models (Martin/Codd).
* Structure beats blobs for known polymorphism.
plan-eng-review Architecture section:
* Data model honesty check covering nullable-semantic, column clusters,
parent-field-shadowed-by-child, JSONField-hiding-schema smells.
* Full Data model review checklist subsection — 11 proactive checks
(SRP, nullable-semantic, hidden-models, parent-shadowed, JSONField,
FK deletion, snapshot-vs-live, cross-scope, derived state, field naming,
DB constraints).
Regression tests:
* 10 static grep guardrails in test/skill-validation.test.ts verify every
load-bearing bullet survives future refactors (free, <5ms, catches silent
deletion). Cognitive-patterns list contiguity (1–18) is asserted.
* New E2E case in test/skill-e2e-plan.test.ts (periodic tier) uses a
synthetic SubscriptionTier plan to verify the skill recommends a
separate model, pushes back on JSONField, and cites normalization/SRP.
Unchanged on purpose: investigate/, openclaw-investigate/ (minimal-diff
correct in bug-fix context), office-hours/ (minimal-viable as one of three
alternatives is fine), openclaw-ceo-review/ (doesn't contain the bullet).
Refs: Codd 1970 (normal forms), Knuth 1974 ("premature optimization"),
Beck ("make it work, make it right, make it fast"), Martin (SOLID/SRP).
Closes garrytan/gstack#1048
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|---|---|---|
| .. | ||
| codex-session-runner.ts | ||
| e2e-helpers.ts | ||
| eval-store.test.ts | ||
| eval-store.ts | ||
| gemini-session-runner.test.ts | ||
| gemini-session-runner.ts | ||
| llm-judge.ts | ||
| observability.test.ts | ||
| session-runner.test.ts | ||
| session-runner.ts | ||
| skill-parser.ts | ||
| touchfiles.ts | ||