fix: counter model-design bias toward fewer tables and JSONField shortcuts

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>

plan-ceo-review/SKILL.md

@@ -645,6 +645,8 @@ Do NOT make any code changes. Do NOT start implementation. Your only job right n
 * 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.
+* **Data model exception to "minimal diff": for schema and model changes, minimal-diff is NOT a goal.** Normalize first; denormalize only for measured performance reasons. Three clean models that each do one thing are better than one model doing three things, in all respects. When tempted to merge new fields into an existing model to reduce table count, ask: "does each model have exactly one job?" If splitting is honest, split. Count concepts, not tables.
+* **JSONField is not an escape hatch for polymorphism.** When tempted to add a JSONField/HStoreField/payload column to encode variant-specific data (different keys for different kinds), promote it to explicit columns instead. JSONField loses FK enforcement, cascade/protect behavior, CheckConstraints on shape, queryability, type system, and self-documentation. Diagnostic: if you find yourself documenting "what keys appear in this JSONField for which variant," you have schema, you just put it in a place where the DB can't help you. Legitimate JSONField uses are genuinely schemaless data (third-party API response caches, user preference bags, opaque blobs). Polymorphism with knowable variants belongs in explicit columns + CheckConstraints.
 * 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.

plan-ceo-review/SKILL.md.tmpl

@@ -61,6 +61,8 @@ Do NOT make any code changes. Do NOT start implementation. Your only job right n
 * 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.
+* **Data model exception to "minimal diff": for schema and model changes, minimal-diff is NOT a goal.** Normalize first; denormalize only for measured performance reasons. Three clean models that each do one thing are better than one model doing three things, in all respects. When tempted to merge new fields into an existing model to reduce table count, ask: "does each model have exactly one job?" If splitting is honest, split. Count concepts, not tables.
+* **JSONField is not an escape hatch for polymorphism.** When tempted to add a JSONField/HStoreField/payload column to encode variant-specific data (different keys for different kinds), promote it to explicit columns instead. JSONField loses FK enforcement, cascade/protect behavior, CheckConstraints on shape, queryability, type system, and self-documentation. Diagnostic: if you find yourself documenting "what keys appear in this JSONField for which variant," you have schema, you just put it in a place where the DB can't help you. Legitimate JSONField uses are genuinely schemaless data (third-party API response caches, user preference bags, opaque blobs). Polymorphism with knowable variants belongs in explicit columns + CheckConstraints.
 * 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.

plan-eng-review/SKILL.md

@@ -590,6 +590,8 @@ If the user asks you to compress or the system triggers context compaction: Step
 * 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.
+* **Data model exception to "minimal diff": for schema and model changes, minimal-diff is NOT a goal.** Normalize first; denormalize only for measured performance reasons. Three clean models that each do one thing are better than one model doing three things, in all respects. When tempted to merge new fields into an existing model to reduce table count, ask: "does each model have exactly one job?" If splitting is honest, split. Count concepts, not tables. The default for schema design is more clean parts, not fewer muddled ones.
+* **JSONField is not an escape hatch for polymorphism.** When tempted to add a JSONField/HStoreField/payload column to encode variant-specific data (different keys for different kinds), promote it to explicit columns instead. JSONField loses FK enforcement, cascade/protect behavior, CheckConstraints on shape, queryability, type system, and self-documentation. Diagnostic: if you find yourself documenting "what keys appear in this JSONField for which variant," you have schema, you just put it in a place where the DB can't help you. Legitimate JSONField uses are genuinely schemaless data (third-party API response caches, user preference bags, opaque blobs). Polymorphism with knowable variants belongs in explicit columns + CheckConstraints.
 
 ## Cognitive Patterns — How Great Eng Managers Think
 
@@ -605,11 +607,14 @@ These are not additional checklist items. They are the instincts that experience
 8. **Org structure IS architecture** — Conway's Law in practice. Design both intentionally (Skelton/Pais, Team Topologies).
 9. **DX is product quality** — Slow CI, bad local dev, painful deploys → worse software, higher attrition. Developer experience is a leading indicator.
 10. **Essential vs accidental complexity** — Before adding anything: "Is this solving a real problem or one we created?" (Brooks, No Silver Bullet).
-11. **Two-week smell test** — If a competent engineer can't ship a small feature in two weeks, you have an onboarding problem disguised as architecture.
+11. **Normalize first, denormalize for measured reasons** — Codd's normal forms (1970) and Knuth's "premature optimization is the root of all evil" (1974) both apply directly to schema design. Denormalizing without measurement is exactly what Knuth warned against. The default for data model design is more clean parts, not fewer muddled ones. "Make it work, make it right, make it fast" (Beck) — right shape comes BEFORE fast.
-12. **Glue work awareness** — Recognize invisible coordination work. Value it, but don't let people get stuck doing only glue (Reilly, The Staff Engineer's Path).
+12. **Single Responsibility Principle applies to data models** — A model that's doing audit + balance + notifications is failing SRP. A model that has fields that are only meaningful when other fields have specific values is failing SRP via hidden polymorphism. Split until each model has exactly one job (Martin, Clean Architecture; Codd, normal forms).
-13. **Make the change easy, then make the easy change** — Refactor first, implement second. Never structural + behavioral changes simultaneously (Beck).
+13. **Structure beats blobs for known polymorphism** — If you can enumerate the variants of a thing at design time, encode them as explicit columns + CheckConstraints, not as a JSONField/payload bag. The DB can enforce FK integrity, shape constraints, and indexing on columns; it can't on JSON paths. JSONField is for genuinely opaque data (third-party blobs, user preferences nobody queries on), not for "we know there are 4 kinds and each kind has different fields."
-14. **Own your code in production** — No wall between dev and ops. "The DevOps movement is ending because there are only engineers who write code and own it in production" (Majors).
+14. **Two-week smell test** — If a competent engineer can't ship a small feature in two weeks, you have an onboarding problem disguised as architecture.
-15. **Error budgets over uptime targets** — SLO of 99.9% = 0.1% downtime *budget to spend on shipping*. Reliability is resource allocation (Google SRE).
+15. **Glue work awareness** — Recognize invisible coordination work. Value it, but don't let people get stuck doing only glue (Reilly, The Staff Engineer's Path).
+16. **Make the change easy, then make the easy change** — Refactor first, implement second. Never structural + behavioral changes simultaneously (Beck).
+17. **Own your code in production** — No wall between dev and ops. "The DevOps movement is ending because there are only engineers who write code and own it in production" (Majors).
+18. **Error budgets over uptime targets** — SLO of 99.9% = 0.1% downtime *budget to spend on shipping*. Reliability is resource allocation (Google SRE).
 
 When evaluating architecture, think "boring by default." When reviewing tests, think "systems over heroes." When assessing complexity, ask Brooks's question. When a plan introduces new infrastructure, check whether it's spending an innovation token wisely.
 
@@ -766,10 +771,71 @@ Evaluate:
 * Data flow patterns and potential bottlenecks.
 * Scaling characteristics and single points of failure.
 * Security architecture (auth, data access, API boundaries).
+* **Data model honesty:** Does each new model/table have exactly one job? Are there nullable fields whose NULL has semantic meaning ("this row is a different kind of thing")? Are there clusters of columns that always co-vary, suggesting a hidden child model? Does any field on a parent model only have meaning when no child rows exist (parent-field-shadowed-by-child smell)? Is there a JSONField/payload column whose keys-per-variant are knowable at design time (JSONField-hiding-schema smell — promote to explicit columns + CheckConstraints)? When in doubt, normalize. Three clean models > one muddled model in all respects.
 * Whether key flows deserve ASCII diagrams in the plan or in code comments.
 * For each new codepath or integration point, describe one realistic production failure scenario and whether the plan accounts for it.
 * **Distribution architecture:** If this introduces a new artifact (binary, package, container), how does it get built, published, and updated? Is the CI/CD pipeline part of the plan or deferred?
 
+#### Data model review checklist
+
+For every new model, model change, or schema migration in this plan, run through this checklist proactively — don't wait for the user to push back on model shape. Each check is a smell; presence of a smell doesn't mean "reject the plan," it means "surface the tradeoff and recommend the normalized alternative."
+
+**Single Responsibility (SRP for models)**
+- Does this model have exactly one job? Audit ≠ balance ≠ notification ≠ display.
+- If a model is doing N jobs, propose splitting it into N models.
+
+**Nullable with semantic meaning**
+- For every nullable column, ask: does NULL flip what kind of thing this row is?
+- If yes, the model is hiding polymorphism behind sentinels. Decompose or add a CheckConstraint tying the nullability to an explicit type column.
+- Always-meaningful nullables (timestamps, optional notes) are fine.
+
+**Models hiding as columns**
+- For every cluster of 2+ columns, ask: do these always co-vary? Would I ever want column A without column B?
+- If they always co-vary, they belong in their own model.
+- For every column, ask: is this only meaningful when another column has a specific value?
+- If yes, you have polymorphism. Promote to a child model or use a CheckConstraint.
+
+**Parent-field-shadowed-by-child**
+- For every field on a parent model, ask: would a child row of this parent have the same field with case-specific values?
+- If yes, the parent's field belongs on the child. Remove from parent, require ≥1 child row, read via FK navigation.
+
+**JSONField as polymorphism escape hatch**
+- For every JSONField, ask: am I documenting "what keys appear for which variant"?
+- If yes, the JSONField has schema. Promote to explicit columns + CheckConstraints.
+- Legitimate JSONField uses: third-party API caches, opaque user preferences, schemaless attribute bags nobody queries against.
+
+**FK deletion strategies (CASCADE / PROTECT / SET_NULL)**
+- For every FK, ask: if the parent is deleted, what should happen to this row?
+- CASCADE only when the child only exists BECAUSE of the parent.
+- PROTECT when deleting the parent would silently destroy user-facing state.
+- SET_NULL when the child should outlive the parent (audit/log rows).
+- Never accept the ORM default without thinking.
+
+**Snapshot vs render-live**
+- For every value stored in a model, ask: is this a NUMERIC INPUT to a calculation, or DISPLAY COPY?
+- Numeric inputs to calculations should be SNAPSHOTTED at the moment of the calculation (so math is reproducible later, immune to source drift).
+- Display copy should be rendered at view time from current FK chains (so renames retroactively update history).
+- Mixing the two is a smell.
+
+**Cross-scope FK consistency**
+- For every model with FKs spanning a "scope" (household, tenant, organization), the DB does not enforce that all FKs resolve to the same scope.
+- Add a validation check that all FKs resolve to the same scope.
+- Single-tenant doesn't need it but adding it now is cheap and future-proofs.
+
+**Derived state beats stored state when read is cheap**
+- For every column that could be derived from another column or query, ask: is the derivation cost acceptable for the read pattern?
+- If yes, derive (single source of truth, can't disagree).
+- Only store derived state when read cost is unacceptable.
+
+**Field naming**
+- For every field, ask: does the name describe WHY it exists (semantics) or HOW it works (implementation)?
+- Bad names: order, position, sequence, type, status, value, data, payload, info, meta.
+- Good names: domain words for the actual concept (step, kind, learning_bonus, role).
+
+**Validation: DB constraints over app validation**
+- For every cross-field invariant, prefer a DB CheckConstraint over app-layer validation.
+- Bulk-insert paths often skip app-layer validation entirely. CheckConstraints catch bugs at write time regardless of how the row was inserted.
+
 **STOP.** For each issue found in this section, call AskUserQuestion individually. One issue per call. Present options, state your recommendation, explain WHY. Do NOT batch multiple issues into one AskUserQuestion. Only proceed to the next section after ALL issues in this section are resolved.
 
 ## Confidence Calibration

plan-eng-review/SKILL.md.tmpl

@@ -46,6 +46,8 @@ If the user asks you to compress or the system triggers context compaction: Step
 * 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.
+* **Data model exception to "minimal diff": for schema and model changes, minimal-diff is NOT a goal.** Normalize first; denormalize only for measured performance reasons. Three clean models that each do one thing are better than one model doing three things, in all respects. When tempted to merge new fields into an existing model to reduce table count, ask: "does each model have exactly one job?" If splitting is honest, split. Count concepts, not tables. The default for schema design is more clean parts, not fewer muddled ones.
+* **JSONField is not an escape hatch for polymorphism.** When tempted to add a JSONField/HStoreField/payload column to encode variant-specific data (different keys for different kinds), promote it to explicit columns instead. JSONField loses FK enforcement, cascade/protect behavior, CheckConstraints on shape, queryability, type system, and self-documentation. Diagnostic: if you find yourself documenting "what keys appear in this JSONField for which variant," you have schema, you just put it in a place where the DB can't help you. Legitimate JSONField uses are genuinely schemaless data (third-party API response caches, user preference bags, opaque blobs). Polymorphism with knowable variants belongs in explicit columns + CheckConstraints.
 
 ## Cognitive Patterns — How Great Eng Managers Think
 
@@ -61,11 +63,14 @@ These are not additional checklist items. They are the instincts that experience
 8. **Org structure IS architecture** — Conway's Law in practice. Design both intentionally (Skelton/Pais, Team Topologies).
 9. **DX is product quality** — Slow CI, bad local dev, painful deploys → worse software, higher attrition. Developer experience is a leading indicator.
 10. **Essential vs accidental complexity** — Before adding anything: "Is this solving a real problem or one we created?" (Brooks, No Silver Bullet).
-11. **Two-week smell test** — If a competent engineer can't ship a small feature in two weeks, you have an onboarding problem disguised as architecture.
+11. **Normalize first, denormalize for measured reasons** — Codd's normal forms (1970) and Knuth's "premature optimization is the root of all evil" (1974) both apply directly to schema design. Denormalizing without measurement is exactly what Knuth warned against. The default for data model design is more clean parts, not fewer muddled ones. "Make it work, make it right, make it fast" (Beck) — right shape comes BEFORE fast.
-12. **Glue work awareness** — Recognize invisible coordination work. Value it, but don't let people get stuck doing only glue (Reilly, The Staff Engineer's Path).
+12. **Single Responsibility Principle applies to data models** — A model that's doing audit + balance + notifications is failing SRP. A model that has fields that are only meaningful when other fields have specific values is failing SRP via hidden polymorphism. Split until each model has exactly one job (Martin, Clean Architecture; Codd, normal forms).
-13. **Make the change easy, then make the easy change** — Refactor first, implement second. Never structural + behavioral changes simultaneously (Beck).
+13. **Structure beats blobs for known polymorphism** — If you can enumerate the variants of a thing at design time, encode them as explicit columns + CheckConstraints, not as a JSONField/payload bag. The DB can enforce FK integrity, shape constraints, and indexing on columns; it can't on JSON paths. JSONField is for genuinely opaque data (third-party blobs, user preferences nobody queries on), not for "we know there are 4 kinds and each kind has different fields."
-14. **Own your code in production** — No wall between dev and ops. "The DevOps movement is ending because there are only engineers who write code and own it in production" (Majors).
+14. **Two-week smell test** — If a competent engineer can't ship a small feature in two weeks, you have an onboarding problem disguised as architecture.
-15. **Error budgets over uptime targets** — SLO of 99.9% = 0.1% downtime *budget to spend on shipping*. Reliability is resource allocation (Google SRE).
+15. **Glue work awareness** — Recognize invisible coordination work. Value it, but don't let people get stuck doing only glue (Reilly, The Staff Engineer's Path).
+16. **Make the change easy, then make the easy change** — Refactor first, implement second. Never structural + behavioral changes simultaneously (Beck).
+17. **Own your code in production** — No wall between dev and ops. "The DevOps movement is ending because there are only engineers who write code and own it in production" (Majors).
+18. **Error budgets over uptime targets** — SLO of 99.9% = 0.1% downtime *budget to spend on shipping*. Reliability is resource allocation (Google SRE).
 
 When evaluating architecture, think "boring by default." When reviewing tests, think "systems over heroes." When assessing complexity, ask Brooks's question. When a plan introduces new infrastructure, check whether it's spending an innovation token wisely.
 
@@ -131,10 +136,71 @@ Evaluate:
 * Data flow patterns and potential bottlenecks.
 * Scaling characteristics and single points of failure.
 * Security architecture (auth, data access, API boundaries).
+* **Data model honesty:** Does each new model/table have exactly one job? Are there nullable fields whose NULL has semantic meaning ("this row is a different kind of thing")? Are there clusters of columns that always co-vary, suggesting a hidden child model? Does any field on a parent model only have meaning when no child rows exist (parent-field-shadowed-by-child smell)? Is there a JSONField/payload column whose keys-per-variant are knowable at design time (JSONField-hiding-schema smell — promote to explicit columns + CheckConstraints)? When in doubt, normalize. Three clean models > one muddled model in all respects.
 * Whether key flows deserve ASCII diagrams in the plan or in code comments.
 * For each new codepath or integration point, describe one realistic production failure scenario and whether the plan accounts for it.
 * **Distribution architecture:** If this introduces a new artifact (binary, package, container), how does it get built, published, and updated? Is the CI/CD pipeline part of the plan or deferred?
 
+#### Data model review checklist
+
+For every new model, model change, or schema migration in this plan, run through this checklist proactively — don't wait for the user to push back on model shape. Each check is a smell; presence of a smell doesn't mean "reject the plan," it means "surface the tradeoff and recommend the normalized alternative."
+
+**Single Responsibility (SRP for models)**
+- Does this model have exactly one job? Audit ≠ balance ≠ notification ≠ display.
+- If a model is doing N jobs, propose splitting it into N models.
+
+**Nullable with semantic meaning**
+- For every nullable column, ask: does NULL flip what kind of thing this row is?
+- If yes, the model is hiding polymorphism behind sentinels. Decompose or add a CheckConstraint tying the nullability to an explicit type column.
+- Always-meaningful nullables (timestamps, optional notes) are fine.
+
+**Models hiding as columns**
+- For every cluster of 2+ columns, ask: do these always co-vary? Would I ever want column A without column B?
+- If they always co-vary, they belong in their own model.
+- For every column, ask: is this only meaningful when another column has a specific value?
+- If yes, you have polymorphism. Promote to a child model or use a CheckConstraint.
+
+**Parent-field-shadowed-by-child**
+- For every field on a parent model, ask: would a child row of this parent have the same field with case-specific values?
+- If yes, the parent's field belongs on the child. Remove from parent, require ≥1 child row, read via FK navigation.
+
+**JSONField as polymorphism escape hatch**
+- For every JSONField, ask: am I documenting "what keys appear for which variant"?
+- If yes, the JSONField has schema. Promote to explicit columns + CheckConstraints.
+- Legitimate JSONField uses: third-party API caches, opaque user preferences, schemaless attribute bags nobody queries against.
+
+**FK deletion strategies (CASCADE / PROTECT / SET_NULL)**
+- For every FK, ask: if the parent is deleted, what should happen to this row?
+- CASCADE only when the child only exists BECAUSE of the parent.
+- PROTECT when deleting the parent would silently destroy user-facing state.
+- SET_NULL when the child should outlive the parent (audit/log rows).
+- Never accept the ORM default without thinking.
+
+**Snapshot vs render-live**
+- For every value stored in a model, ask: is this a NUMERIC INPUT to a calculation, or DISPLAY COPY?
+- Numeric inputs to calculations should be SNAPSHOTTED at the moment of the calculation (so math is reproducible later, immune to source drift).
+- Display copy should be rendered at view time from current FK chains (so renames retroactively update history).
+- Mixing the two is a smell.
+
+**Cross-scope FK consistency**
+- For every model with FKs spanning a "scope" (household, tenant, organization), the DB does not enforce that all FKs resolve to the same scope.
+- Add a validation check that all FKs resolve to the same scope.
+- Single-tenant doesn't need it but adding it now is cheap and future-proofs.
+
+**Derived state beats stored state when read is cheap**
+- For every column that could be derived from another column or query, ask: is the derivation cost acceptable for the read pattern?
+- If yes, derive (single source of truth, can't disagree).
+- Only store derived state when read cost is unacceptable.
+
+**Field naming**
+- For every field, ask: does the name describe WHY it exists (semantics) or HOW it works (implementation)?
+- Bad names: order, position, sequence, type, status, value, data, payload, info, meta.
+- Good names: domain words for the actual concept (step, kind, learning_bonus, role).
+
+**Validation: DB constraints over app validation**
+- For every cross-field invariant, prefer a DB CheckConstraint over app-layer validation.
+- Bulk-insert paths often skip app-layer validation entirely. CheckConstraints catch bugs at write time regardless of how the row was inserted.
+
 **STOP.** For each issue found in this section, call AskUserQuestion individually. One issue per call. Present options, state your recommendation, explain WHY. Do NOT batch multiple issues into one AskUserQuestion. Only proceed to the next section after ALL issues in this section are resolved.
 
 {{CONFIDENCE_CALIBRATION}}

test/helpers/touchfiles.ts

@@ -77,6 +77,7 @@ export const E2E_TOUCHFILES: Record<string, string[]> = {
   'plan-ceo-review-benefits':  ['plan-ceo-review/**', 'scripts/gen-skill-docs.ts'],
   'plan-eng-review':           ['plan-eng-review/**'],
   'plan-eng-review-artifact':  ['plan-eng-review/**'],
+  'plan-eng-review-data-model-bias': ['plan-eng-review/**'],
   'plan-review-report':        ['plan-eng-review/**', 'scripts/gen-skill-docs.ts'],
 
   // Codex offering verification
@@ -236,6 +237,7 @@ export const E2E_TIERS: Record<string, 'gate' | 'periodic'> = {
   'plan-ceo-review-benefits': 'gate',
   'plan-eng-review': 'periodic',
   'plan-eng-review-artifact': 'periodic',
+  'plan-eng-review-data-model-bias': 'periodic',
   'plan-eng-coverage-audit': 'gate',
   'plan-review-report': 'gate',
 

test/skill-e2e-plan.test.ts

@@ -277,6 +277,141 @@ Focus on architecture, code quality, tests, and performance sections.`,
   }, 420_000);
 });
 
+// --- Plan Eng Review Data-Model Bias Regression E2E ---
+//
+// Regression test for the data-model bias fix: verifies that /plan-eng-review
+// recommends a separate model instead of inlining columns + JSONField when
+// a plan tries to merge polymorphic variant data into an existing model "to
+// keep the diff minimal." Before the fix, the skill's "minimal diff" preference
+// pushed the AI toward accepting the inline approach; after the fix, the
+// data-model exception bullet + Data model review checklist should make the
+// AI push back and recommend normalization citing SRP/3NF.
+
+describeIfSelected('Plan Eng Review Data-Model Bias E2E', ['plan-eng-review-data-model-bias'], () => {
+  let planDir: string;
+
+  beforeAll(() => {
+    planDir = fs.mkdtempSync(path.join(os.tmpdir(), 'skill-e2e-plan-eng-dmb-'));
+    const run = (cmd: string, args: string[]) =>
+      spawnSync(cmd, args, { cwd: planDir, stdio: 'pipe', timeout: 5000 });
+
+    run('git', ['init', '-b', 'main']);
+    run('git', ['config', 'user.email', 'test@test.com']);
+    run('git', ['config', 'user.name', 'Test']);
+
+    // Synthetic plan designed to trip the old bias: four clearly-polymorphic
+    // tier variants + feature-flag bag that the user proposes to inline onto
+    // User rather than create a new model. After the fix, the skill should
+    // recommend a separate SubscriptionTier model and push back on the
+    // JSONField for tier_features.
+    fs.writeFileSync(path.join(planDir, 'plan.md'), `# Plan: Add Subscription Tiers to User Model
+
+## Context
+I want to add a 'subscription tier' feature to my Django app. Each user can
+be on a free, basic, premium, or enterprise tier. Each tier has:
+- a monthly price
+- a feature set (list of feature flags)
+- a max-users limit
+- an SLA tier (none / standard / premium)
+
+## Current state
+The User model has ~15 fields (email, name, created_at, etc).
+
+## Proposed changes
+I'm thinking of just adding these columns directly to the User model so I
+don't have to deal with another table:
+
+- tier_name: CharField with choices=['free', 'basic', 'premium', 'enterprise']
+- tier_price: DecimalField
+- tier_features: JSONField (list of feature flag strings, varies per tier)
+- tier_max_users: IntegerField
+- tier_sla: CharField with choices=['none', 'standard', 'premium']
+
+This keeps the diff minimal — no new model, no new migration beyond the one
+column-add, no new FK navigation in the codebase.
+
+## Open questions
+- Should feature flags be a separate table or stay as JSONField?
+- Any concerns with this approach?
+`);
+
+    run('git', ['add', '.']);
+    run('git', ['commit', '-m', 'add plan']);
+
+    fs.mkdirSync(path.join(planDir, 'plan-eng-review'), { recursive: true });
+    fs.copyFileSync(
+      path.join(ROOT, 'plan-eng-review', 'SKILL.md'),
+      path.join(planDir, 'plan-eng-review', 'SKILL.md'),
+    );
+  });
+
+  afterAll(() => {
+    try { fs.rmSync(planDir, { recursive: true, force: true }); } catch {}
+  });
+
+  testConcurrentIfSelected('plan-eng-review-data-model-bias', async () => {
+    const result = await runSkillTest({
+      prompt: `Read plan-eng-review/SKILL.md for the review workflow.
+
+Read plan.md — that's the plan to review. This is a standalone plan document, not a codebase — skip any codebase exploration steps.
+
+Proceed directly to the architecture review section. Skip any AskUserQuestion calls — this is non-interactive. Write your complete architecture review directly to ${planDir}/review-output.md
+
+Focus specifically on the data model design in the plan. Apply the data model review checklist from the skill.`,
+      workingDirectory: planDir,
+      maxTurns: 15,
+      timeout: 360_000,
+      testName: 'plan-eng-review-data-model-bias',
+      runId,
+      model: 'claude-opus-4-6',
+    });
+
+    logCost('/plan-eng-review data-model-bias', result);
+
+    // Verify the review was written
+    const reviewPath = path.join(planDir, 'review-output.md');
+    const review = fs.existsSync(reviewPath)
+      ? fs.readFileSync(reviewPath, 'utf-8')
+      : '';
+
+    // Behavioral assertions: the review should recommend a separate tier model
+    // and push back on JSONField for the feature set. Matching is deliberately
+    // loose (case-insensitive substrings) to tolerate wording variance while
+    // still catching the bias regression.
+    const lower = review.toLowerCase();
+    const recommendsSeparateModel =
+      /separate\s+(subscription\s*)?tier\s*model/i.test(review) ||
+      /new\s+(subscription\s*)?tier\s*model/i.test(review) ||
+      /subscriptiontier\s*model/i.test(review) ||
+      /extract.*tier.*model/i.test(review) ||
+      /tier\s*(?:table|model)\s*(?:with|per)/i.test(review);
+    const pushesBackOnJsonField =
+      lower.includes('jsonfield') &&
+      (lower.includes('feature') || lower.includes('polymorph') || lower.includes('explicit'));
+    const citesNormalization =
+      /normali[sz]/i.test(review) ||
+      /\bsrp\b/i.test(review) ||
+      /single\s+responsibility/i.test(review) ||
+      /\b3nf\b/i.test(review) ||
+      /normal\s+form/i.test(review);
+
+    recordE2E(evalCollector, '/plan-eng-review-data-model-bias', 'Plan Eng Review Data-Model Bias E2E', result, {
+      passed:
+        ['success', 'error_max_turns'].includes(result.exitReason) &&
+        review.length > 200 &&
+        recommendsSeparateModel &&
+        pushesBackOnJsonField &&
+        citesNormalization,
+    });
+
+    expect(['success', 'error_max_turns']).toContain(result.exitReason);
+    expect(review.length).toBeGreaterThan(200);
+    expect(recommendsSeparateModel).toBe(true);
+    expect(pushesBackOnJsonField).toBe(true);
+    expect(citesNormalization).toBe(true);
+  }, 420_000);
+});
+
 // --- Plan-Eng-Review Test-Plan Artifact E2E ---
 
 describeIfSelected('Plan-Eng-Review Test-Plan Artifact E2E', ['plan-eng-review-artifact'], () => {

test/skill-validation.test.ts

@@ -1617,3 +1617,70 @@ describe('sidebar agent (#584)', () => {
     expect(content).not.toContain("proc.stderr.on('data', () => {})");
   });
 });
+
+// Data-model bias guardrails — prevent silent deletion of the corrective bullets
+// added to counter the AI's pull toward table-count minimization and JSONField
+// polymorphism shortcuts. These are load-bearing: if they disappear in a future
+// refactor (templates regenerated, sections rewritten), the bias returns silently.
+// Static grep beats LLM judge here because the real failure mode is "bullet
+// silently deleted during an unrelated edit," which greps catch in milliseconds.
+describe('data-model bias guardrails', () => {
+  const engReview = fs.readFileSync(path.join(ROOT, 'plan-eng-review', 'SKILL.md'), 'utf-8');
+  const ceoReview = fs.readFileSync(path.join(ROOT, 'plan-ceo-review', 'SKILL.md'), 'utf-8');
+
+  test('plan-eng-review preserves the "data model exception to minimal diff" bullet', () => {
+    expect(engReview).toContain('Data model exception to "minimal diff"');
+    expect(engReview).toContain('Count concepts, not tables');
+  });
+
+  test('plan-eng-review preserves the JSONField polymorphism warning', () => {
+    expect(engReview).toContain('JSONField is not an escape hatch for polymorphism');
+    expect(engReview).toContain('what keys appear in this JSONField for which variant');
+  });
+
+  test('plan-eng-review preserves the Normalize-first cognitive pattern', () => {
+    expect(engReview).toContain('Normalize first, denormalize for measured reasons');
+  });
+
+  test('plan-eng-review preserves the SRP-for-data-models cognitive pattern', () => {
+    expect(engReview).toContain('Single Responsibility Principle applies to data models');
+  });
+
+  test('plan-eng-review preserves the Structure-beats-blobs cognitive pattern', () => {
+    expect(engReview).toContain('Structure beats blobs for known polymorphism');
+  });
+
+  test('plan-eng-review Architecture section preserves Data model honesty check', () => {
+    expect(engReview).toContain('Data model honesty');
+    expect(engReview).toContain('parent-field-shadowed-by-child');
+    expect(engReview).toContain('JSONField-hiding-schema');
+  });
+
+  test('plan-eng-review preserves the Data model review checklist subsection', () => {
+    expect(engReview).toContain('Data model review checklist');
+    // A few load-bearing checklist items — if any disappear, the checklist was gutted
+    expect(engReview).toContain('Single Responsibility (SRP for models)');
+    expect(engReview).toContain('Nullable with semantic meaning');
+    expect(engReview).toContain('Parent-field-shadowed-by-child');
+    expect(engReview).toContain('FK deletion strategies');
+    expect(engReview).toContain('Snapshot vs render-live');
+  });
+
+  test('plan-eng-review cognitive patterns list is contiguous 1–18', () => {
+    // If someone inserts/removes a pattern without renumbering, catch it here
+    for (let i = 1; i <= 18; i++) {
+      expect(engReview).toMatch(new RegExp(`^${i}\\. \\*\\*`, 'm'));
+    }
+    // And item 19 should NOT exist (the list ends at 18)
+    expect(engReview).not.toMatch(/^19\. \*\*/m);
+  });
+
+  test('plan-ceo-review preserves the "data model exception to minimal diff" bullet', () => {
+    expect(ceoReview).toContain('Data model exception to "minimal diff"');
+    expect(ceoReview).toContain('Count concepts, not tables');
+  });
+
+  test('plan-ceo-review preserves the JSONField polymorphism warning', () => {
+    expect(ceoReview).toContain('JSONField is not an escape hatch for polymorphism');
+  });
+});