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