[SKILL] Add API Platform database schema design rules and reporting script#2271
[SKILL] Add API Platform database schema design rules and reporting script#2271Thushani-Jayasekera wants to merge 3 commits into
Conversation
Thushani-Jayasekera
commented
Jun 25, 2026
- Introduced a new skill for managing database schema design rules specific to the WSO2 API Platform, detailing guidelines for creating and modifying tables.
- Added a reference document outlining the rules (R1–R9) applicable to schema files.
- Implemented a script to generate structured JSON reports from schema review findings, enhancing the review process for database schema changes.
- Introduced a new skill for managing database schema design rules specific to the WSO2 API Platform, detailing guidelines for creating and modifying tables. - Added a reference document outlining the rules (R1–R9) applicable to schema files. - Implemented a script to generate structured JSON reports from schema review findings, enhancing the review process for database schema changes.
📝 WalkthroughWalkthroughAdds a database schema design skill with workflows for authoring and reviewing DDL, a rules reference covering schema conventions and engine alignment, and a CLI that converts schema review findings into a JSON report. Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/api-platform-db-schema-design-rules/references/api-platform-db-schema-rules.md:
- Around line 88-99: Scope the R3-NO-TEXT rule in the schema rules document so
it only flags bare TEXT for engines where it is disallowed, while explicitly
allowing SQLite TEXT and SQL Server NVARCHAR(MAX)/TEXT as intentional
divergences per R8. Update the wording around R3-NO-TEXT, R3-LARGE-PAYLOAD, and
R3-JSONB-Scan-Compat in the referenced rules section so the guidance is
engine-aware and no longer produces false findings for non-Postgres schemas.
Also align any duplicated checklist guidance in SKILL.md with the same
engine-specific exception.
- Around line 50-64: The identity example in the schema rules is not org-scoped
because `handle` is shown as globally unique, which conflicts with the stated
org-scoped uniqueness. Update the example in the R1-IDENTITY guidance to use a
composite uniqueness rule anchored on `organization_uuid` and `handle`, and make
the same adjustment in the duplicate template referenced by `SKILL.md`. Keep the
rest of the `handle`/`name` guidance unchanged, and ensure the example clearly
shows that the same handle can exist in different organizations.
In
@.agents/skills/api-platform-db-schema-design-rules/scripts/generate-schema-report.js:
- Around line 61-93: The report normalization in generate-schema-report.js is
deriving IDs, severities, and meta.rules inconsistently with the expected
contract. Update the mapping logic in the findings normalization block so the
generated id uses the documented rule-group identifier format, normalize
severity to the supported uppercase values before sorting/summary, and stop
inferring meta.rules from findings alone by always serializing the full expected
R1–R9 rule list in the report metadata.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 14c0ee72-78d0-4c2c-8e6b-827c981aa78f
📒 Files selected for processing (3)
.agents/skills/api-platform-db-schema-design-rules/SKILL.md.agents/skills/api-platform-db-schema-design-rules/references/api-platform-db-schema-rules.md.agents/skills/api-platform-db-schema-design-rules/scripts/generate-schema-report.js
| **R3-NO-TEXT** — Do not use the bare `TEXT` type for any column. Use either a bounded `VARCHAR(N)`, a binary type, or (Postgres only) `JSONB` when content is queried. A `TEXT` column is always a finding at MEDIUM severity. | ||
|
|
||
| **R3-LARGE-PAYLOAD** — Any payload that can grow large or is variable-length must use `BYTEA` (Postgres) / `BLOB` (SQLite) / `VARBINARY(MAX)` (SQL Server). Do **not** use wide VARCHAR for: `openapi_spec`, `model_list`, `content`, `configuration`, `properties`, `manifest`, `policy_definition`, `metadata`, `api_key_hashes`, or any future column whose value can exceed a few hundred bytes. | ||
|
|
||
| **R3-JSONB** — In PostgreSQL, use `JSONB` only when the application queries inside the JSON using Postgres JSON operators. Evidence that a column is actively queried inside: | ||
| 1. The `DEFAULT` is a JSON literal like `'{}'` | ||
| 2. The column name implies structure: `settings`, `event_data` | ||
| 3. The same column in a sibling table already uses `JSONB` | ||
|
|
||
| SQLite and SQL Server equivalents (`TEXT` / `NVARCHAR(MAX)`) are intentional type-level divergences — not findings. | ||
|
|
||
| **R3-JSONB-SCAN-COMPAT** — Do not use `JSONB` if the application layer scans it into a plain `string` variable and calls `json.Unmarshal` manually. Postgres drivers return JSONB as binary, which breaks `string` scan targets at runtime. Only use `JSONB` when the scan target implements `sql.Scanner` (e.g. `pgtype.JSONB`, `json.RawMessage`, or a custom struct). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Scope the no-TEXT rule by engine.
R8 explicitly allows SQLite TEXT and SQL Server NVARCHAR(MAX)/TEXT equivalents as intentional divergences. This blanket ban will create false findings for valid non-Postgres schemas, and the checklist in SKILL.md repeats the same contradiction.
Proposed fix
- Do not use the bare TEXT type for any column. Use either a bounded VARCHAR(N), a binary type, or (Postgres only) JSONB when content is queried.
+ In PostgreSQL, do not use bare TEXT for columns that should be bounded or binary; allow the R8 engine-specific TEXT/NVARCHAR(MAX) divergences elsewhere.🤖 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/api-platform-db-schema-design-rules/references/api-platform-db-schema-rules.md
around lines 88 - 99, Scope the R3-NO-TEXT rule in the schema rules document so
it only flags bare TEXT for engines where it is disallowed, while explicitly
allowing SQLite TEXT and SQL Server NVARCHAR(MAX)/TEXT as intentional
divergences per R8. Update the wording around R3-NO-TEXT, R3-LARGE-PAYLOAD, and
R3-JSONB-Scan-Compat in the referenced rules section so the guidance is
engine-aware and no longer produces false findings for non-Postgres schemas.
Also align any duplicated checklist guidance in SKILL.md with the same
engine-specific exception.
| WSO2 API Platform-specific database schema design, change, and review skill. Use proactively whenever: | ||
| - Adding a new table to platform-api or developer-portal schemas | ||
| - Modifying an existing table (new column, type change, constraint, index) | ||
| - Reviewing schema changes before a PR | ||
| - Writing or evaluating a migration plan | ||
| - Asking "is this table well designed?" or "what indexes does this table need?" | ||
|
|
||
| Applies platform house conventions: UUID primary keys (VARCHAR(40)) for entity tables, composite PRIMARY KEY for junction/mapping tables, handle/name/version identity triple for named resources, org-scoping, BYTEA/BLOB for large payloads, SMALLINT booleans, TIMESTAMPTZ timestamps, audit columns (created_by/at, updated_by/at), data_version for on-the-fly migrations, idempotent DDL, and standard indexing patterns. | ||
|
|
||
| Scope: applies to every schema.*.sql file in the repository. Gateway controller schemas (gateway/gateway-controller/) are included for structural rules (R1–R2, R4–R9) but R3 type validation is skipped for them — the gateway controller team owns their type choices. | ||
| allowed-tools: Bash, Read, Edit, Write, Glob |
There was a problem hiding this comment.
This is a bit too long. Shall we make it less than 1024 chars?
There was a problem hiding this comment.
There was a problem hiding this comment.
Yes, Description is just a triggering point, no need to have conventions there https://agentskills.io/skill-creation/best-practices
| ### Standard VARCHAR widths | ||
|
|
||
| ``` | ||
| VARCHAR(20) — status, lifecycle_status, kind, short enums | ||
| VARCHAR(30) — version strings (v1.0, v2.3) | ||
| VARCHAR(40) — uuid, all FK columns referencing UUIDs | ||
| VARCHAR(64) — hashes (SHA-256 hex) |
There was a problem hiding this comment.
We already have a separate file to specify the rules? Should we move them there and only keep the breadth of the flow here?
| @@ -0,0 +1,276 @@ | |||
| --- | |||
| name: api-platform-db-schema-design-rules | |||
There was a problem hiding this comment.
Lets change the name
eg: designing-db-schemas or validating.. whatever applicable
https://platform.claude.com/docs/en/agents-and-tools/agent-skills/best-practices
(I need to change the one I added for REST API too. It's name also not good)
| Write a structured findings file so findings can be consumed by other tools or tracked across reviews: | ||
|
|
||
| ```bash | ||
| node .agents/skills/api-platform-db-schema-design-rules/scripts/generate-schema-report.js \ |
There was a problem hiding this comment.
Should we use the relative path for the script from the skill folder?
Can you check if the execution breaks when we do that?
https://platform.claude.com/docs/en/agents-and-tools/agent-skills/best-practices#provide-utility-scripts
|
Some other rules
|
- Introduced a new skill for designing and reviewing database schemas specific to the WSO2 API Platform, including guidelines for table creation and modification. - Added a comprehensive reference document outlining schema design rules (R1–R10) applicable to all relevant schema files. - Implemented a script for generating structured JSON reports from schema review findings, streamlining the review process for database changes.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.agents/skills/designing-db-schemas/references/api-platform-db-schema-rules.md (1)
113-119: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winFix
handlewidth inconsistency inR3-VARCHAR-SIZES.The
R3-VARCHAR-SIZEStable listsHandle / name / display stringtogether asVARCHAR(255), butR1-IDENTITY, the entity table template, and the standard column cheat sheet all definehandleasVARCHAR(40). A 255-character URL-safe slug is excessive and contradicts the identity triple guidance. Split the row sohandleisVARCHAR(40)andname / display stringremainsVARCHAR(255).📝 Proposed fix
| UUID / foreign key | `VARCHAR(40)` | | User identity (email / sub) | `VARCHAR(200)` | -| Handle / name / display string | `VARCHAR(255)` | +| Handle (url-safe slug) | `VARCHAR(40)` | +| Name / display string | `VARCHAR(255)` | | Version string | `VARCHAR(30)` |Also update the standard column types cheat sheet language specifier:
-``` +VARCHAR(20) — status, lifecycle_status, kind, short enumsAlso applies to: 336-342, 360-388
🤖 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 113 - 119, `R3-VARCHAR-SIZES` currently groups `handle` with `name/display string` at `VARCHAR(255)`, which conflicts with the schema rules that define `handle` as `VARCHAR(40)`. Update the table in the schema rules doc to split this row so `handle` uses `VARCHAR(40)` while `name` and `display string` stay `VARCHAR(255)`, and make the same width correction wherever the standard column cheat sheet or related referenced sections describe `handle` or the identity triple (`R1-IDENTITY`, entity table template, standard column types cheat sheet)..agents/skills/designing-db-schemas/SKILL.md (1)
91-116: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd R7 (Application-Logic Safety) items to the self-review checklist.
R7 is entirely absent from the checklist, yet it contains schema-relevant constraints:
SELECT *prohibition,JSONBscan-target compatibility, immutability ofhandle/created_at/created_by, and soft-revocation column placement. Authors should verify these at DDL time to avoid application-runtime failures.🤖 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/SKILL.md around lines 91 - 116, The self-review checklist in SKILL.md is missing the R7 application-logic safety checks entirely. Add a new R7 section alongside the existing R1–R10 items in the checklist, and include the schema-related safeguards for SELECT * prohibition, JSONB scan-target compatibility, immutability of handle/created_at/created_by, and soft-revocation column placement. Keep the wording aligned with the existing checklist style so reviewers can verify these constraints at DDL review time.
♻️ Duplicate comments (1)
.agents/skills/designing-db-schemas/scripts/generate-schema-report.js (1)
65-75: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject malformed
rule/severityvalues instead of silently downgrading them.Line 71 uppercases
severitybut does not trim it, and Line 74 falls back toMEDIUMfor anything unsupported. A hand-authored value like" HIGH "or a typo will produce the wrong summary and priority without any signal. The same applies torule, which is emitted unnormalized and can generate a non-contract group key. Trim both fields, normalizerule, and fail fast on unknown values.Proposed fix
+const RULE_GROUPS = new Set(['R1','R2','R3','R4','R5','R6','R7','R8','R9','R10']); + const counters = {}; const normalised = findings.map(f => { - const rule = f.rule || 'UNKNOWN'; + const rule = String(f.rule || '').trim().toUpperCase(); + const ruleGroup = rule.split('-')[0]; + if (!RULE_GROUPS.has(ruleGroup)) { + throw new Error(`Unsupported rule group: ${f.rule}`); + } // Rule-group identifier per the report contract: R3-NO-TEXT -> r3 - const group = rule.split('-')[0].toLowerCase().replace(/[^a-z0-9]/g, '') || 'unknown'; + const group = ruleGroup.toLowerCase(); counters[group] = (counters[group] || 0) + 1; const seq = String(counters[group]).padStart(3, '0'); // Normalise severity to a supported uppercase value before sort/summary - const sev = String(f.severity || 'MEDIUM').toUpperCase(); + const sev = String(f.severity || 'MEDIUM').trim().toUpperCase(); + if (ORDER[sev] === undefined) { + throw new Error(`Unsupported severity: ${f.severity}`); + } return { id: `${group}-${seq}`, - severity: ORDER[sev] !== undefined ? sev : 'MEDIUM', + severity: sev,🤖 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/scripts/generate-schema-report.js around lines 65 - 75, The normalization in generate-schema-report.js is silently accepting malformed rule/severity values, which can produce incorrect group IDs and summaries. Update the mapping logic that builds the report entries to trim and normalize both f.rule and f.severity, using the existing group/id generation path and ORDER lookup, and fail fast instead of defaulting unsupported values to MEDIUM or emitting an unnormalized rule. Keep the validation close to the code that sets rule, sev, and id so bad inputs are rejected before summary generation.
🧹 Nitpick comments (1)
.agents/skills/designing-db-schemas/references/api-platform-db-schema-rules.md (1)
360-360: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd language specifier to the fenced code block.
The standard column types cheat sheet at line 360 triggers
MD040because it lacks a language tag. Usetextto suppress misleading syntax highlighting while satisfying the linter.🤖 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 at line 360, Add a language specifier to the fenced code block in the standard column types cheat sheet so it no longer triggers MD040; update the markdown fence in the referenced section to use text, and keep the content otherwise unchanged.
🤖 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/scripts/generate-schema-report.js:
- Around line 62-69: The finding IDs are assigned in raw input order inside
generate-schema-report.js, so the same data can produce different rN-### values
when review JSON ordering changes. Update the normalisation flow around the
findings.map logic to sort findings by stable keys first, then assign the
sequential per-group counters from that final order. Keep the rule-group
derivation and numbering logic tied to the existing normalised/report generation
path so IDs remain deterministic across runs.
In @.agents/skills/designing-db-schemas/SKILL.md:
- Around line 80-84: R9 (Idempotent DDL) is missing from the workflow rule
mappings for schema changes. Update each change-type row in the
design-db-schemas skill table so New table, New column, Type change, and New
index all include R9 alongside the existing rules. Keep the existing symbols and
wording aligned with the mapping table, and ensure the guidance explicitly
covers existence guards like IF NOT EXISTS or OBJECT_ID checks for every DDL
operation.
---
Outside diff comments:
In
@.agents/skills/designing-db-schemas/references/api-platform-db-schema-rules.md:
- Around line 113-119: `R3-VARCHAR-SIZES` currently groups `handle` with
`name/display string` at `VARCHAR(255)`, which conflicts with the schema rules
that define `handle` as `VARCHAR(40)`. Update the table in the schema rules doc
to split this row so `handle` uses `VARCHAR(40)` while `name` and `display
string` stay `VARCHAR(255)`, and make the same width correction wherever the
standard column cheat sheet or related referenced sections describe `handle` or
the identity triple (`R1-IDENTITY`, entity table template, standard column types
cheat sheet).
In @.agents/skills/designing-db-schemas/SKILL.md:
- Around line 91-116: The self-review checklist in SKILL.md is missing the R7
application-logic safety checks entirely. Add a new R7 section alongside the
existing R1–R10 items in the checklist, and include the schema-related
safeguards for SELECT * prohibition, JSONB scan-target compatibility,
immutability of handle/created_at/created_by, and soft-revocation column
placement. Keep the wording aligned with the existing checklist style so
reviewers can verify these constraints at DDL review time.
---
Duplicate comments:
In @.agents/skills/designing-db-schemas/scripts/generate-schema-report.js:
- Around line 65-75: The normalization in generate-schema-report.js is silently
accepting malformed rule/severity values, which can produce incorrect group IDs
and summaries. Update the mapping logic that builds the report entries to trim
and normalize both f.rule and f.severity, using the existing group/id generation
path and ORDER lookup, and fail fast instead of defaulting unsupported values to
MEDIUM or emitting an unnormalized rule. Keep the validation close to the code
that sets rule, sev, and id so bad inputs are rejected before summary
generation.
---
Nitpick comments:
In
@.agents/skills/designing-db-schemas/references/api-platform-db-schema-rules.md:
- Line 360: Add a language specifier to the fenced code block in the standard
column types cheat sheet so it no longer triggers MD040; update the markdown
fence in the referenced section to use text, and keep the content otherwise
unchanged.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0752ee8f-739f-43a4-9895-6a6f728578a7
📒 Files selected for processing (3)
.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
| // Assign sequential IDs per rule group and normalise | ||
| const counters = {}; | ||
| const normalised = findings.map(f => { | ||
| const rule = f.rule || 'UNKNOWN'; | ||
| // Rule-group identifier per the report contract: R3-NO-TEXT -> r3 | ||
| const group = rule.split('-')[0].toLowerCase().replace(/[^a-z0-9]/g, '') || 'unknown'; | ||
| counters[group] = (counters[group] || 0) + 1; | ||
| const seq = String(counters[group]).padStart(3, '0'); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Make finding IDs deterministic before numbering them.
Lines 62-69 assign the per-rule sequence in raw input order, but the report is sorted later. The same finding set can therefore receive different IDs when reviewers reorder the JSON, which makes cross-review tracking noisy. Sort on stable keys first, then assign the rN-### IDs from that final order.
🤖 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/scripts/generate-schema-report.js around
lines 62 - 69, The finding IDs are assigned in raw input order inside
generate-schema-report.js, so the same data can produce different rN-### values
when review JSON ordering changes. Update the normalisation flow around the
findings.map logic to sort findings by stable keys first, then assign the
sequential per-group counters from that final order. Keep the rule-group
derivation and numbering logic tied to the existing normalised/report generation
path so IDs remain deterministic across runs.
| | New table | R1 (identity), R2 (org-scoping), R3 (types), R4 (constraints), R5 (audit columns), R6 (indexes), R10 (naming) | | ||
| | New column | R3 (type), R4 (constraints), R5 (audit), R6 (index if filterable), R10 (lowercase name) | | ||
| | Type change | R3 (correct type for target engine), R8 (counterpart schemas if multi-engine) | | ||
| | New index | R6 (correct pattern — FK, status, compound, partial), R10 (lowercase index name) | | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Include R9 (Idempotent DDL) in workflow rule mappings.
R9 is absent from all change-type mappings, yet every DDL operation that creates or alters objects requires idempotency guards. New tables, new columns, type changes, and new indexes all need IF NOT EXISTS or OBJECT_ID checks. Add R9 to each row so authors do not skip existence guards.
🤖 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/SKILL.md around lines 80 - 84, R9
(Idempotent DDL) is missing from the workflow rule mappings for schema changes.
Update each change-type row in the design-db-schemas skill table so New table,
New column, Type change, and New index all include R9 alongside the existing
rules. Keep the existing symbols and wording aligned with the mapping table, and
ensure the guidance explicitly covers existence guards like IF NOT EXISTS or
OBJECT_ID checks for every DDL operation.
…ifications - Added a description for the VARCHAR(64) column to specify its use for hashes (SHA-256 hex) in the database schema reference document. - Ensured clarity in the guidelines for indexed and unique columns across all engines.