docs(review): record the unbumped catalog-schema-version pattern

.claude/commands/mxcli-dev/review.md

@@ -61,6 +61,7 @@ proactively. Add a row after every review that surfaces something new.
 | 30 | Two commands compute the same thing from two copies of the setup (`report` re-implementing `lint`'s rule list and skipping its config), so they disagree about a project — and a SCORE carries no provenance, so neither number looks wrong | Code correctness | Extract the shared setup and route both through it. A value test cannot guard this when the copies live inside cobra `RunE` bodies: use a structural check on the source, with a positive control asserted FIRST so it cannot pass vacuously |
 | 31 | A test helper that needs a heavyweight object only to satisfy a signature (`NewLintContext(nil, nil)`, which panics) invites a nil-guard added purely to make the test compile — behaviour nothing in production needs, defended forever | Test coverage | Narrow the signature instead: if the helper does not use the parameter, drop it and let the caller apply the part it owns. A test that cannot construct an argument is usually telling you the argument does not belong |
 | 32 | A fix adds a diagnostic for a capability the model lacks while leaving in place the code that asserts the capability EXISTS — MDL042 telling the author a loop's `@caption` is dropped, while `cmd_microflows_builder_annotations.go` still ran `case *microflows.LoopedActivity: activity.Caption = ann.Caption` under the comment "LOOP / WHILE activities can carry a caption just like splits", and the describer still emitted one. Nothing read either back. The next reader trusts the code over the warning, deletes the check, and reopens the bug from the other side. The reason it survives is that it usually has TESTS — three here asserted the caption was carried, all of them against the semantic object and none against storage, so they passed throughout and failed only on the correct fix | Code correctness | `generated/metamodel` is the arbiter: a field on the semantic type it does not declare cannot survive a write, so an assignment to it is dead by construction. Grep the writer, the describer and the semantic struct and delete (or re-comment) whatever sets it. MEASURE before deleting — `exec` then `describe` on a real project, with the UNMODIFIED build, so the deletion rests on the stored document rather than on reading the codec. Invert the tests that defended it rather than deleting them, keeping any half still true (escaping coverage belongs on a type that can carry a caption), and check the inverted test fails when the assignment is put back |
+| 33 | A column added to `createTables` without bumping `CatalogSchemaVersion` — the version guard only drops tables when it CHANGES, and `CREATE TABLE IF NOT EXISTS` never adds a column, so every user with a cached catalog keeps a table the new SELECT cannot read. Measured on #1181: `activities_for` yielded 302 on `main` and **0** on the branch against the same cache, `no such column: UseRequestTimeout`, `mxcli lint` exit 1. `mxcli report` runs the same rules and never checks `QueryErrors()`, so there it would score silently | Code correctness | Bump the constant in the same commit — its doc comment says so and `62913741` is the precedent (four columns + 11→12 + a builder test). To REPRODUCE, the cache must actually be reused: build it with the old binary and run the new one with the **same spelling of `-p`**, because a relative-vs-absolute path invalidates on "MPR path changed" and hides the bug; confirm the run says "Loading cached catalog … (from cache)" before believing a green result |
 
 ---