|
1 | 1 | --- |
2 | 2 | name: fix-schema-validation |
3 | | -description: Fix Schema Validation Errors |
| 3 | +description: Investigate and fix manual or CI schema test failures in `src/schema.test.ts` and `src/mod-schema.test.ts`, especially after upstream Cataclysm-BN data changes. |
4 | 4 | --- |
5 | 5 |
|
6 | | -This workflow guides you through resolving schema validation errors, typically caused by upstream changes in `Cataclysm-BN`. |
| 6 | +This skill is for schema test failures only. |
7 | 7 |
|
8 | | -1. **Run Tests to Identify Failure** |
9 | | - Run the schema validation tests to isolate the failure. |
| 8 | +Use it when: |
| 9 | + |
| 10 | +- `pnpm vitest run src/schema.test.ts --no-color` fails |
| 11 | +- `pnpm vitest run src/mod-schema.test.ts --no-color` fails |
| 12 | +- scheduled CI or daily/nightly builds fail during schema validation |
| 13 | +- freshly fetched fixtures introduce new JSON shapes that no longer match `src/types.ts` |
| 14 | + |
| 15 | +Do not use this skill for: |
| 16 | + |
| 17 | +- generic rendering failures with no schema assertion |
| 18 | +- routing/UI regressions unrelated to data shape validation |
| 19 | +- upstream gameplay bugs that do not break this app's schema tests |
| 20 | + |
| 21 | +## Goal |
| 22 | + |
| 23 | +Make the guide accept current Cataclysm-BN data with the smallest accurate schema change possible. |
| 24 | + |
| 25 | +That usually means: |
| 26 | + |
| 27 | +- updating `src/types.ts` |
| 28 | +- sometimes updating a local normalization/helper path when the accepted shape is materially different at runtime |
| 29 | +- verifying both core and mod schema tests when the change can affect shared types |
| 30 | + |
| 31 | +## First Principles |
| 32 | + |
| 33 | +- We are consumers of upstream JSON, not its authors. |
| 34 | +- Prefer narrow, discriminated unions over vague permissive types. |
| 35 | +- Only relax the schema as far as current upstream behavior requires. |
| 36 | +- Verify whether the new shape exists in core data, mod data, or both. |
| 37 | +- If local upstream source is stale, trust the fetched fixture and then confirm provenance via GitHub history/PRs. |
| 38 | + |
| 39 | +## Useful Sources |
| 40 | + |
| 41 | +Prefer linking to canonical project docs over duplicating their instructions here. |
| 42 | + |
| 43 | +### Local upstream Cataclysm-BN checkout |
| 44 | + |
| 45 | +Try these likely locations first: |
| 46 | + |
| 47 | +- `~/Workspace/C-BN/Cataclysm-BN` |
| 48 | +- `../Cataclysm-BN` |
| 49 | + |
| 50 | +If neither exists, ask the user for the path to their Cataclysm-BN checkout. |
| 51 | + |
| 52 | +Before trusting a local checkout, check whether it is actually current: |
| 53 | + |
| 54 | +```bash |
| 55 | +git -C ~/Workspace/C-BN/Cataclysm-BN branch -vv |
| 56 | +git -C ~/Workspace/C-BN/Cataclysm-BN log --oneline -n 20 --decorate |
| 57 | +``` |
| 58 | + |
| 59 | +Important: CI daily failures may use fresher nightly fixtures than a nearby local clone. Do not assume the local checkout contains the same change that broke the nightly build. |
| 60 | + |
| 61 | +### Fixture data |
| 62 | + |
| 63 | +- Core merged data: `_test/all.json` |
| 64 | +- Mod-scoped data: `_test/all_mods.json` |
| 65 | +- JSON exploration help: [`_test/AGENTS.md`](_test/AGENTS.md) |
| 66 | + |
| 67 | +### Upstream GitHub provenance |
| 68 | + |
| 69 | +When the failure appears to come from a recent upstream change: |
| 70 | + |
| 71 | +- inspect the upstream JSON file history |
| 72 | +- inspect recently merged Cataclysm-BN PRs |
| 73 | +- look for commit messages that mention the failing object/file/feature |
| 74 | + |
| 75 | +Useful targets: |
| 76 | + |
| 77 | +- upstream repo: `https://github.com/cataclysmbn/Cataclysm-BN` |
| 78 | +- failing workflow run in this repo |
| 79 | +- upstream PRs/commits that introduced the new JSON shape |
| 80 | + |
| 81 | +If `gh api` is awkward or blocked, `curl` against the GitHub API is acceptable. |
| 82 | + |
| 83 | +## Commands |
| 84 | + |
| 85 | +For canonical test/fixture workflow and guardrails, see [`AGENTS.md`](AGENTS.md). |
| 86 | + |
| 87 | +Schema-specific commands worth calling out here: |
| 88 | + |
| 89 | +Refresh fixtures when debugging nightly failures: |
| 90 | + |
| 91 | +```bash |
| 92 | +pnpm fetch:fixtures:nightly |
| 93 | +``` |
| 94 | + |
| 95 | +Run the core schema test: |
| 96 | + |
| 97 | +```bash |
| 98 | +pnpm vitest run src/schema.test.ts --no-color |
| 99 | +``` |
| 100 | + |
| 101 | +Run the mod schema test: |
| 102 | + |
| 103 | +```bash |
| 104 | +pnpm vitest run src/mod-schema.test.ts --no-color |
| 105 | +``` |
| 106 | + |
| 107 | +Note: do not append `--no-color` to `pnpm check`; `tsc` in this project does not accept it. |
| 108 | + |
| 109 | +## Workflow |
| 110 | + |
| 111 | +1. Identify exactly which schema test failed. |
| 112 | + |
| 113 | + Distinguish: |
| 114 | + - core schema failure in `src/schema.test.ts` |
| 115 | + - mod schema failure in `src/mod-schema.test.ts` |
| 116 | + - broader test failure where schema is only one symptom |
| 117 | + |
| 118 | + Capture: |
| 119 | + - failing object `type` |
| 120 | + - failing id/result/abstract |
| 121 | + - `__filename` from the test output |
| 122 | + - failing property path such as `/use_action/0` |
| 123 | + |
| 124 | +2. Reproduce against the right dataset. |
| 125 | + |
| 126 | + For daily/nightly CI failures, start with: |
10 | 127 |
|
11 | 128 | ```bash |
12 | | - pnpm vitest --run schema.test.ts --bail 2 |
| 129 | + pnpm fetch:fixtures:nightly |
| 130 | + pnpm vitest run src/schema.test.ts --no-color |
| 131 | + pnpm vitest run src/mod-schema.test.ts --no-color |
13 | 132 | ``` |
14 | 133 |
|
15 | | - - Look for errors like `schema matches <type> <id>`. |
16 | | - - Note the error detail, e.g., `/property/0 must NOT have more than 2 items`. |
| 134 | + If only one suite is implicated, you can stop at the failing one first. |
17 | 135 |
|
18 | | -2. **Locate Failing Data** |
19 | | - Find the specific object in the test data that causes the failure. |
20 | | - - Use a script or `jq` on `_test/all.json`. |
21 | | - - Example script pattern (create as `debug_find.cjs`): |
| 136 | +3. Locate the failing object with `jq`. |
| 137 | + |
| 138 | + Core examples: |
22 | 139 |
|
23 | 140 | ```bash |
24 | | - jq -r '.data[] | select(.type? == "item_type" and .category == "category_id" and .id="book") ' _test/all.json |
| 141 | + jq '.data[] | select(.id=="<id>" and .type=="<type>")' _test/all.json |
| 142 | + jq '.data[] | select((.__filename // "") | contains("<file-fragment>"))' _test/all.json |
| 143 | + jq '[.data[] | select(.type=="<type>") | keys[]] | unique | sort' _test/all.json |
| 144 | + ``` |
| 145 | + |
| 146 | + Mod examples: |
25 | 147 |
|
| 148 | + ```bash |
| 149 | + jq 'keys[]' -r _test/all_mods.json |
| 150 | + jq '.aftershock.data[] | select(.id=="<id>" and .type=="<type>")' _test/all_mods.json |
| 151 | + jq '[.aftershock.data[] | select(.type=="<type>") | keys[]] | unique | sort' _test/all_mods.json |
26 | 152 | ``` |
27 | 153 |
|
28 | | -3. **Check TypeScript Definition** |
29 | | - Open `src/types.ts` and locate the interface matching the failure `type`. |
30 | | - - Compare the `interface` or `type` definition with the failing data structure. |
| 154 | +4. Decide whether the break is caused by: |
| 155 | + - a genuinely new upstream variant that should be modeled in `src/types.ts` |
| 156 | + - an existing variant that the schema forgot to include |
| 157 | + - a permissive loader pattern upstream that requires a looser local type |
| 158 | + - a runtime shape difference that also requires helper/component updates |
| 159 | + - mod inheritance/override behavior that only appears in `src/mod-schema.test.ts` |
| 160 | + |
| 161 | +5. Analyze upstream Cataclysm-BN source and PR history. |
| 162 | + |
| 163 | + Check the local upstream clone first if available and current enough: |
| 164 | + |
| 165 | + ```bash |
| 166 | + rg -n '<symbol-or-json-key>' ~/Workspace/C-BN/Cataclysm-BN/data ~/Workspace/C-BN/Cataclysm-BN/src |
| 167 | + git -C ~/Workspace/C-BN/Cataclysm-BN log --oneline -- <likely-json-file> |
| 168 | + ``` |
| 169 | + |
| 170 | + If the local clone is behind nightly: |
| 171 | + - ask user to permission to update or ask him to update |
| 172 | + - inspect the commit history for that file |
| 173 | + - inspect recently merged upstream PRs related to the feature |
| 174 | + |
| 175 | + The purpose is not busywork. It tells you whether the fixture change is: |
| 176 | + - isolated to one new data shape |
| 177 | + - part of a larger upstream feature rollout |
| 178 | + - likely to have follow-on effects in mods or render tests |
| 179 | + |
| 180 | +6. Update `src/types.ts`. |
| 181 | + |
| 182 | + Most schema failures should be fixed here. |
| 183 | + |
| 184 | + Preferred patterns: |
| 185 | + - add a new discriminated union member for a new object shape |
| 186 | + - extend an existing union when upstream now permits an extra variant |
| 187 | + - keep field types precise |
| 188 | + - add brief comments only when the permissiveness would otherwise look suspicious |
| 189 | + |
| 190 | + Avoid: |
| 191 | + - replacing a specific type with `any` |
| 192 | + - widening everything to `unknown` or `Record<string, unknown>` unless absolutely necessary |
| 193 | + - adding broad catch-all strings when the code intentionally wants warnings for newly introduced upstream actions |
| 194 | + |
| 195 | +7. Update runtime handling only if the accepted shape is used by the app. |
| 196 | + |
| 197 | + Common places to inspect: |
| 198 | + - `src/data.ts` |
| 199 | + - `src/types/*.svelte` |
| 200 | + - normalization helpers such as `normalizeUseAction` |
| 201 | + |
| 202 | + Heuristic: |
| 203 | + - if the failure is only that the schema rejects a shape already handled by runtime code, stop at `src/types.ts` |
| 204 | + - if the new shape changes how data is read, normalized, indexed, or rendered, patch runtime logic too |
| 205 | + |
| 206 | +8. Decide whether UI or presentation changes are needed. |
| 207 | + |
| 208 | + Some JSON shape changes are not just schema work. They may require: |
| 209 | + - new UI for a newly introduced concept |
| 210 | + - updated labels or presentation in existing components |
| 211 | + - better rendering of newly accepted fields |
| 212 | + |
| 213 | + When that happens, consult the user before expanding scope. |
31 | 214 |
|
32 | | -4. **Verify Upstream (Optional)** |
33 | | - If you have access to `Cataclysm-BN` source code: |
34 | | - - Search for the C++ struct definition (often in `src/mapdata.h`, `src/vitamin.h`, etc.). |
35 | | - - Check the `load` function in the corresponding `.cpp` file to see how the JSON is parsed. |
36 | | - - _Crucial_: Check if the C++ parser ignores extra elements (e.g., uses `get_int(0)` and `get_int(1)` on a larger array). |
| 215 | + Common options: |
| 216 | + - fix schema only now to restore CI / validation |
| 217 | + - fix schema plus minimal runtime/UI support now |
| 218 | + - fix schema now and defer UI/presentation to a follow-up task |
37 | 219 |
|
38 | | -5. **Update Schema (`src/types.ts`)** |
39 | | - Modify the TypeScript definition to accommodate the new data format. |
40 | | - - If C++ ignores extra data, document this in a comment and make the type permissive (e.g., `(string|number)[][]` instead of `[string, number][]`). |
41 | | - - If new variants are added (e.g., object wrapper `{ ter: ... }` vs string), use Union types. |
| 220 | + Default bias for CI repair: |
| 221 | + - restore schema validation first |
| 222 | + - then ask whether UI work should happen in the same change or later |
42 | 223 |
|
43 | | -6. **Update Parsing Logic (If Needed)** |
44 | | - If the data structure change affects how data is used in the app (e.g., not just extra ignored fields, but a fundamental change in shape like Object vs Array): |
45 | | - - Check `src/types/item/spawnLocations.ts` (for Mapgen). |
46 | | - - Check relevant Svelte components. |
47 | | - - Update parsing functions (e.g., `getMapgenValueDistribution`) to handle the new shape. |
| 224 | +9. Handle mod-specific fallout explicitly. |
48 | 225 |
|
49 | | -7. **Verify Fix** |
50 | | - Re-run the tests. |
51 | | - // turbo |
| 226 | + If the failure is in `src/mod-schema.test.ts`: |
| 227 | + - inspect `_test/all_mods.json` |
| 228 | + - identify the mod id and object source chain |
| 229 | + - consider whether the same fix belongs in shared types or in mod-specific inheritance handling |
| 230 | + |
| 231 | + If a shared type change could affect mod rendering, run: |
| 232 | + |
| 233 | + ```bash |
| 234 | + pnpm gen:mod-tests |
| 235 | + pnpm vitest run src/__mod_tests__/mod.<mod_id>.test.ts --no-color |
| 236 | + ``` |
| 237 | + |
| 238 | + Or, if needed: |
52 | 239 |
|
53 | 240 | ```bash |
54 | | - pnpm fix:format && pnpm vitest --run schema.test.ts --bail 2 |
| 241 | + pnpm test:render:mods --no-color |
55 | 242 | ``` |
| 243 | + |
| 244 | + Remember: |
| 245 | + - generated files in `src/__mod_tests__/` are derived artifacts |
| 246 | + - mod schema failures and mod render failures are related but not identical |
| 247 | + |
| 248 | +10. Verify narrowly, then more broadly as needed. |
| 249 | + |
| 250 | +Minimum verification for a core schema change: |
| 251 | + |
| 252 | +```bash |
| 253 | +pnpm vitest run src/schema.test.ts --no-color |
| 254 | +pnpm check |
| 255 | +pnpm lint |
| 256 | +``` |
| 257 | + |
| 258 | +Minimum verification for a mod-related schema change: |
| 259 | + |
| 260 | +```bash |
| 261 | +pnpm vitest run src/mod-schema.test.ts --no-color |
| 262 | +pnpm check |
| 263 | +pnpm lint |
| 264 | +``` |
| 265 | + |
| 266 | +If the touched type is shared between core and mods, run both schema suites. |
| 267 | + |
| 268 | +11. Summarize provenance in the final write-up. |
| 269 | + |
| 270 | +Always record: |
| 271 | + |
| 272 | +- the failing CI run or local command |
| 273 | +- the upstream file/object that introduced the new shape |
| 274 | +- the relevant upstream Cataclysm-BN commit or PR if known |
| 275 | +- whether the scope appears isolated or broader |
| 276 | + |
| 277 | +## What Usually Matters Most |
| 278 | + |
| 279 | +- `src/types.ts`: source of truth for schema generation |
| 280 | +- `src/schema.test.ts`: validates `_test/all.json` |
| 281 | +- `src/mod-schema.test.ts`: validates `_test/all_mods.json` |
| 282 | +- `src/data.ts`: flattening/normalization/runtime use of accepted shapes |
| 283 | +- `scripts/fetch-fixtures.ts`: nightly fixture source |
| 284 | +- `scripts/gen-mod-tests.ts`: regenerate mod render tests when checking downstream mod fallout |
| 285 | +- `src/types/*.svelte`: presentation/components that may need updates after schema acceptance |
| 286 | +- [`AGENTS.md`](AGENTS.md): canonical project workflow and test matrix |
| 287 | +- [`_test/AGENTS.md`](_test/AGENTS.md): canonical `jq` exploration patterns for fixture blobs |
| 288 | + |
| 289 | +## Practical Heuristics |
| 290 | + |
| 291 | +- If the error says `must be string, but was [...]` or `must be object, but was [...]`, suspect a newly allowed union branch or array wrapper. |
| 292 | +- If only one or two new object `type` values appear in nightly data, prefer adding those explicit variants rather than broadening the whole field. |
| 293 | +- If the failing object comes from a newly added upstream JSON file, inspect that file and its introducing PR before deciding how much to widen the schema. |
| 294 | +- If core schema passes but mod schema fails, investigate mod inheritance or mod-only legacy shapes before changing shared runtime logic. |
| 295 | +- If the local Cataclysm-BN checkout lacks the failing file, it may simply be older than the nightly fixture. Confirm before drawing conclusions. |
| 296 | + |
| 297 | +## Anti-Patterns |
| 298 | + |
| 299 | +- Do not grep `_test/all.json` or `_test/all_mods.json`. |
| 300 | +- Do not “fix” schema failures by skipping tests. |
| 301 | +- Do not widen a type beyond what the fixture and upstream parser justify. |
| 302 | +- Do not stop after `src/schema.test.ts` if the changed type is shared and mods plausibly use it too. |
| 303 | +- Do not commit generated `src/__mod_tests__/` files unless the task truly requires regenerated artifacts. |
0 commit comments