Add API Platform database schema design rules and reporting script#2271
Open
Thushani-Jayasekera wants to merge 1 commit into
Open
Add API Platform database schema design rules and reporting script#2271Thushani-Jayasekera wants to merge 1 commit into
Thushani-Jayasekera wants to merge 1 commit into
Conversation
Thushani-Jayasekera
commented
Jun 25, 2026
Thushani-Jayasekera
commented
Contributor
- 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.
Thushani-Jayasekera
requested review from
AnuGayan,
Arshardh,
CrowleyRajapakse,
HeshanSudarshana,
HiranyaKavishani,
Krishanx92,
PasanT9,
RakhithaRR,
Tharsanan1,
VirajSalaka,
ashera96,
chamilaadhi,
dushaniw,
hisanhunais,
malinthaprasan,
pubudu538,
renuka-fernando,
senthuran16,
tgtshanika,
tharikaGitHub and
tharindu1st
as code owners
Contributor
📝 WalkthroughAdded an API Platform database schema design rules skill, along with a detailed reference document covering schema conventions and review rules R1–R9 for the WSO2 API Platform. Included guidance for:
Also added a Node.js script that takes schema review findings and generates a structured JSON report, including normalized findings, severity summaries, rule metadata, and file output handling. WalkthroughAdds a new schema design rules skill for WSO2 API Platform, a detailed rules reference for repository schema files, and a Node.js CLI that normalizes schema findings into a JSON review report with severity summaries and output generation. Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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
Comment on lines
+50
to
+64
| **R1-IDENTITY** — Tables representing named resources (APIs, gateways, providers, applications, subscriptions, or any domain entity with a stable slug and a display name) must carry the full identity triple directly: | ||
|
|
||
| ```sql | ||
| handle VARCHAR(40) UNIQUE NOT NULL, -- url-safe slug, immutable once set | ||
| name VARCHAR(255) NOT NULL, -- human-readable display name | ||
| version VARCHAR(30) NOT NULL DEFAULT '1.0', -- semver or opaque version string | ||
| ``` | ||
|
|
||
| Identity must be denormalised onto the table itself so queries against a single table are self-contained. Do not rely on a parent record for identity fields. | ||
|
|
||
| **R1-HANDLE-NAME** — `handle` and `name` are distinct: | ||
| - `handle` is the URL-safe slug used in API paths (e.g. `/resources/{handle}`). It must be unique within its scope (always org-scoped) and treated as immutable after creation. | ||
| - `name` is the human-readable display string. It may change and **is not required to be unique** — not even within a project. | ||
|
|
||
| Conflating them (using `name` as the slug, or allowing `handle` to contain spaces) is a finding. Adding a `UNIQUE` constraint on `name` (alone or combined with `project_uuid`) for API-type entities is also a finding — enforce only `UNIQUE(organization_uuid, handle)`. |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Make the identity example org-scoped.
handle is described as unique within an organization, but this example makes it globally unique. That will reject the same slug in different orgs and conflicts with R2. Please mirror the fix in the duplicate template in SKILL.md.
Proposed fix
- handle VARCHAR(40) UNIQUE NOT NULL, -- url-safe slug, immutable once set
+ handle VARCHAR(40) NOT NULL, -- url-safe slug, immutable once set📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| **R1-IDENTITY** — Tables representing named resources (APIs, gateways, providers, applications, subscriptions, or any domain entity with a stable slug and a display name) must carry the full identity triple directly: | |
| ```sql | |
| handle VARCHAR(40) UNIQUE NOT NULL, -- url-safe slug, immutable once set | |
| name VARCHAR(255) NOT NULL, -- human-readable display name | |
| version VARCHAR(30) NOT NULL DEFAULT '1.0', -- semver or opaque version string | |
| ``` | |
| Identity must be denormalised onto the table itself so queries against a single table are self-contained. Do not rely on a parent record for identity fields. | |
| **R1-HANDLE-NAME** — `handle` and `name` are distinct: | |
| - `handle` is the URL-safe slug used in API paths (e.g. `/resources/{handle}`). It must be unique within its scope (always org-scoped) and treated as immutable after creation. | |
| - `name` is the human-readable display string. It may change and **is not required to be unique** — not even within a project. | |
| Conflating them (using `name` as the slug, or allowing `handle` to contain spaces) is a finding. Adding a `UNIQUE` constraint on `name` (alone or combined with `project_uuid`) for API-type entities is also a finding — enforce only `UNIQUE(organization_uuid, handle)`. | |
| **R1-IDENTITY** — Tables representing named resources (APIs, gateways, providers, applications, subscriptions, or any domain entity with a stable slug and a display name) must carry the full identity triple directly: | |
🤖 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 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.
Comment on lines
+88
to
+99
| **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). |
Contributor
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.
Comment on lines
+61
to
+93
| const normalised = findings.map(f => { | ||
| const ruleKey = (f.rule || 'UNKNOWN').toLowerCase().replace(/[^a-z0-9]/g, '-'); | ||
| counters[ruleKey] = (counters[ruleKey] || 0) + 1; | ||
| const seq = String(counters[ruleKey]).padStart(3, '0'); | ||
| return { | ||
| id: `${ruleKey}-${seq}`, | ||
| severity: f.severity || 'MEDIUM', | ||
| rule: f.rule || 'UNKNOWN', | ||
| table: f.table || null, | ||
| column: f.column || null, | ||
| finding: f.finding || '', | ||
| fix: f.fix || '', | ||
| }; | ||
| }); | ||
|
|
||
| // Sort: HIGH → MEDIUM → LOW | ||
| const ORDER = { HIGH: 0, MEDIUM: 1, LOW: 2 }; | ||
| normalised.sort((a, b) => (ORDER[a.severity] ?? 3) - (ORDER[b.severity] ?? 3)); | ||
|
|
||
| // Collect unique rule groups checked | ||
| const ruleGroups = [...new Set(normalised.map(f => f.rule.split('-')[0]))].sort(); | ||
|
|
||
| // Summary counts | ||
| const summary = { HIGH: 0, MEDIUM: 0, LOW: 0 }; | ||
| for (const f of normalised) summary[f.severity] = (summary[f.severity] || 0) + 1; | ||
|
|
||
| // ---------- build output ---------- | ||
| const report = { | ||
| meta: { | ||
| schema: schemaPath, | ||
| reviewedAt: new Date().toISOString(), | ||
| rules: ruleGroups.length ? ruleGroups : ['R1','R2','R3','R4','R5','R6','R7','R8','R9'], | ||
| }, |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Normalize the report contract before serializing.
id is derived from the full rule name (R3-NO-TEXT → r3-no-text-001) instead of the documented rule-group ID (r3-001), and severity is copied verbatim so lower-case or unexpected values will sort and summarize incorrectly. Also, meta.rules is inferred from findings, so clean rule groups disappear from the JSON even though the skill expects a complete R1–R9 review.
Proposed fix
- const ruleKey = (f.rule || 'UNKNOWN').toLowerCase().replace(/[^a-z0-9]/g, '-');
+ const ruleKey = (f.rule || 'UNKNOWN').split('-')[0].toLowerCase();
...
- severity: f.severity || 'MEDIUM',
+ severity: normalizeSeverity(f.severity),🤖 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/scripts/generate-schema-report.js
around lines 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.
Comment on lines
+4
to
+14
| 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 |
Contributor
There was a problem hiding this comment.
This is a bit too long. Shall we make it less than 1024 chars?
Contributor
There was a problem hiding this comment.
Contributor
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
Comment on lines
+246
to
+252
| ### 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) |
Contributor
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 | |||
Contributor
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 \ |
Contributor
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
Contributor
|
Some other rules
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.