fix(schema): make codegen idempotent so drift detection surfaces only real Riot drift#21
Merged
Merged
Conversation
… real Riot drift
The schema-drift workflow was unusable: every regen wiped 407 lines of
JSDoc, broke property names with hyphens (`ar-AE: string` is invalid TS),
and dropped item shapes whenever an array sample was empty. Three hand-
patched commits in main papered over codegen output instead of fixing
the codegen, so any rerun produced 600+ line destructive diffs.
This PR fixes the codegen so its output is reproducible:
1. **JSDoc emission**. `codegen.ts` now reads `scripts/descriptions.json`
and emits `/** ... */` blocks for interfaces and fields. The JSON is
keyed by `${game}.${interfaceName}` so identical names across games
(e.g. `BannedChampion` in both LoL and TFT) don't collide.
`scripts/extract-descriptions.mjs` is the one-shot tool that seeded
the corpus from the existing hand-edits — kept around for re-extraction.
2. **Quote non-identifier property keys**. `formatPropertyKey()` accepts
identifiers and integer literals unquoted, double-quotes everything
else. Eliminates the `ar-AE: string` invalid-TS bug. Match the double-
quote convention already in committed types.
3. **Preserve array item shape on empty samples**. `patchUnknownArrayItems`
walks the new schema and the previously-committed schema in parallel;
when the new sample produced `items: { type: 'unknown' }` (empty array)
but the prior schema captured a concrete shape, retain the prior shape.
Doesn't mask top-level field additions/removals — only restores nested
item info that this run couldn't sample.
The committed `generated/*.ts` files are the codegen's actual output
against the existing schemas, so future regens are now no-ops modulo
real Riot drift. Validated locally: regen against unchanged schemas
produces zero unexpected diffs.
Known limitation: hand-edited inline JSDoc *inside* anonymous object
types (e.g. `({ /** Title */ content: string })`) is not preserved —
those nested anonymous types have no stable name to key descriptions
against. Top-level interface and field descriptions are fully preserved.
Tests: 32 new tests (formatPropertyKey, formatJsDoc, generateInterface
with descriptions/non-identifier keys, patchUnknownArrayItems edge cases).
All 506 tests pass; build clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a new declarative catalog Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/generate-schema/codegen.ts`:
- Around line 19-35: The JSDoc for formatPropertyKey incorrectly says keys "get
wrapped in single quotes" while the implementation (and tests) wrap keys in
double quotes; update the comment to state "double quotes" (or "wrapped in
double quotes") and adjust any wording referencing single quotes to mention
double quotes, keeping references to VALID_IDENTIFIER and INTEGER_LITERAL intact
so the doc matches the actual behavior of formatPropertyKey.
🪄 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: 140e1aed-0b34-4dec-b5d8-7f79096d69ef
⛔ Files ignored due to path filters (5)
packages/whisper/src/types/generated/lol.tsis excluded by!**/generated/**packages/whisper/src/types/generated/lor.tsis excluded by!**/generated/**packages/whisper/src/types/generated/riftbound.tsis excluded by!**/generated/**packages/whisper/src/types/generated/tft.tsis excluded by!**/generated/**packages/whisper/src/types/generated/val.tsis excluded by!**/generated/**
📒 Files selected for processing (7)
scripts/descriptions.jsonscripts/extract-descriptions.mjsscripts/generate-schema/codegen.test.tsscripts/generate-schema/codegen.tsscripts/generate-schema/index.tsscripts/generate-schema/schema.test.tsscripts/generate-schema/schema.ts
1. scripts/generate-schema/codegen.ts:19-35 — JSDoc on formatPropertyKey said keys "get wrapped in single quotes" but implementation (and tests) use double quotes. Updated comment to say "double quotes" and explain the rationale (matches convention already in committed types). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Why
Per yesterday's local test of `pnpm generate-schema`, the schema-drift workflow is currently unusable:
These were three distinct codegen bugs, each previously hand-patched in main without fixing the codegen — meaning every regen reproduced the bugs and produced a 600+ line destructive diff. The actual Riot drift was buried.
Three commits in main illustrate the pattern:
What
Three targeted codegen fixes plus the supporting data:
JSDoc emission from a structured source
Quote non-identifier property keys (`formatPropertyKey()`)
Preserve array item shape on empty samples (`patchUnknownArrayItems`)
Validation
Local idempotency check: regenerated `.ts` files against unchanged schemas — diff is now scoped to inconsistencies in main itself (single-vs-double-quote drift between hand-edited interfaces, `{x}[]` vs `({x})[]` style drift, one extra blank line in some files), not codegen bugs. The committed `generated/*.ts` files in this PR are the codegen's actual output, so future regens are no-ops modulo real Riot drift.
Comparison vs. previous behavior on a clean regen against committed schemas:
Known limitation
Hand-edited inline JSDoc inside anonymous object types — e.g. `({ /** Title */ content: string })` — is not preserved. Those anonymous nested types have no stable name to key descriptions against in the JSON corpus, and adding one would require a more elaborate descriptions schema. Top-level interface descriptions and field-level descriptions are fully preserved.
Tests
Next steps after merge
🤖 Generated with Claude Code
Fixes nondeterministic codegen to enable idempotent schema generation
Addresses codegen nondeterminism so future regenerations only reflect real Riot API drift. Implements three targeted fixes and supporting data.
JSDoc emission
Property key formatting
Array item shape preservation
Validation
Key files changed