Add database schema design skill and rules documentation - #3199
Add database schema design skill and rules documentation#3199Thushani-Jayasekera wants to merge 4 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdded a database-schema design skill, API Platform schema rules, database change policies, and a Node.js CLI that generates deterministic JSON schema-review reports. ChangesDatabase Schema Review
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
||
| 3. **Every change ships its upgrade path (R0-UPGRADE-PATH).** `CREATE TABLE IF NOT EXISTS` is a no-op against a database that already exists, so a column added to a `CREATE TABLE` body reaches fresh installs only. Ship the matching per-dialect `ALTER TABLE`, nullable or defaulted so it applies while the previous release is still running. | ||
|
|
||
| 4. **No deferring a violation behind a code comment.** Never resolve a missing dialect file, a missing `ALTER` path, an absent foreign key, or a plaintext-secret column with a `-- TODO`/`FIXME` comment and merge anyway — a comment does not create a column on a customer's database. Fix it, or raise a tracked issue with an owner and a deadline in the PR description. |
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
@.agents/skills/designing-db-schemas/references/api-platform-db-schema-rules.md:
- Around line 69-73: Update the opening rule in the schema rules section to
apply to “Every entity table,” explicitly excluding pure junction/mapping tables
that follow R1-COMPOSITE-PK. Preserve the existing UUID requirement for entity
tables and the composite-key requirement for mapping tables.
- Around line 156-160: Update the R3-JSONB rule to require concrete application
query evidence using PostgreSQL JSON operators before selecting JSONB. Remove
the listed heuristics involving JSON defaults, structured column names, and
sibling-table JSONB usage, then retain the scan-compatibility rule after direct
query evidence is established.
- Around line 280-284: Update R6-ORG-INDEX in
.agents/skills/designing-db-schemas/references/api-platform-db-schema-rules.md
at lines 280-284 to exempt organization_uuid when it is already covered by the
leftmost part of a primary key or unique constraint. Apply the same exception to
the org-scoped index checklist in .agents/skills/designing-db-schemas/SKILL.md
at line 118.
- Line 163: Update the R3-JSONB-SCAN-COMPAT rule to be driver-specific: document
pgx v5.9.2 JSONBCodec behavior, including supported *string and *[]byte scan
targets, and distinguish behavior for other PostgreSQL drivers. Remove the
blanket string-scan restriction, replace unsupported pgtype.JSONB and
database/sql.Scanner examples with valid types, and avoid describing
json.RawMessage as implementing Scanner.
In @.agents/skills/designing-db-schemas/scripts/generate-schema-report.js:
- Around line 79-87: The finding normalization logic in the report-generation
mapping must suppress remediation for findings with severity LEGACY-ACCEPTED.
Update the returned fix value around severity and fix handling so
legacy-accepted findings always emit an empty or omitted fix, while preserving
supplied fixes for other severities.
- Around line 37-55: Update flag() to return null when the option has no
following token or when the next token begins with "--", so every flag rejects
missing values and option tokens. Preserve the existing defaults and validation
flow for valid --findings, --schema, and --out values.
- Around line 77-87: Update the findings normalization map to validate every
record before accessing fields: reject null or non-object entries, require rule
to be a string, and accept only supported severity values from ORDER. Do not
coerce malformed rules or unsupported severities to defaults; reject them
explicitly while preserving normalization for valid findings.
- Around line 93-100: Update the comparator used by normalised.sort to include
cmp(a.fix, b.fix) after the existing finding comparison, ensuring findings with
otherwise identical sort keys receive deterministic IDs before the assignment
logic at Lines 104-108.
In @.agents/skills/designing-db-schemas/SKILL.md:
- Around line 15-17: Add language identifiers to every reported fenced code
block: label the usage fence at .agents/skills/designing-db-schemas/SKILL.md
lines 15-17, the checklist fence at lines 96-96, and the type-reference fence at
.agents/skills/designing-db-schemas/references/api-platform-db-schema-rules.md
lines 462-462, using the appropriate Markdown language identifier for each
block.
- Around line 144-146: Update .agents/skills/designing-db-schemas/SKILL.md lines
144-146 to require per-dialect ALTER statements only for additive columns on
existing tables, while retaining guarded CREATE guidance for new tables and
indexes. Update .claude/rules/db-schema-changes.md line 15 to replace the
universal “Every change” requirement with explicit upgrade paths for new tables,
new indexes, and added columns.
In @.claude/rules/db-schema-changes.md:
- Around line 13-17: Clarify the relationship between rules R0-LEGACY-ACCEPTED
and the TODO guidance: require fixes for violations in unshipped changes, while
treating violations already present in shipped tables as LEGACY-ACCEPTED. Update
directive 4 to explicitly exempt shipped-table violations, requiring Appendix A
documentation and deferral of remediation to an approved migration plan without
adding remediation DDL.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 49b2fbed-4083-48c4-90c2-bf418f6ba1bc
📒 Files selected for processing (4)
.agents/skills/designing-db-schemas/SKILL.md.agents/skills/designing-db-schemas/references/api-platform-db-schema-rules.md.agents/skills/designing-db-schemas/scripts/generate-schema-report.js.claude/rules/db-schema-changes.md
| **R6-ORG-INDEX** — Every org-scoped table must have an index on `organization_uuid`: | ||
|
|
||
| ```sql | ||
| CREATE INDEX IF NOT EXISTS idx_<table>_org ON <table>(organization_uuid); | ||
| ``` |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
Apply one consistent exception for indexes covered by a primary key.
The rules correctly state that a leading primary-key column is already indexed, but both the checklist and R6-ORG-INDEX require a separate organization_uuid index.
.agents/skills/designing-db-schemas/references/api-platform-db-schema-rules.md#L280-L284: add “unless already covered by the leftmost part of a primary key or unique constraint.”.agents/skills/designing-db-schemas/SKILL.md#L118-L118: apply the same exception in the checklist.
📍 Affects 2 files
.agents/skills/designing-db-schemas/references/api-platform-db-schema-rules.md#L280-L284(this comment).agents/skills/designing-db-schemas/SKILL.md#L118-L118
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
@.agents/skills/designing-db-schemas/references/api-platform-db-schema-rules.md
around lines 280 - 284, Update R6-ORG-INDEX in
.agents/skills/designing-db-schemas/references/api-platform-db-schema-rules.md
at lines 280-284 to exempt organization_uuid when it is already covered by the
leftmost part of a primary key or unique constraint. Apply the same exception to
the org-scoped index checklist in .agents/skills/designing-db-schemas/SKILL.md
at line 118.
…block formatting and clarifying self-review checklist steps.
…ines in documentation.
Purpose
$subject