Skip to content

Commit 1db6e14

Browse files
dmealingclaude
andcommitted
fix(verify,codegen): gate the schema snapshot (#292), agree on CHECK names (#293)
#292 -- THE SNAPSHOT WAS THE ONE FILE NOTHING CHECKED `meta migrate` diffs metadata against .metaobjects/migrations/.schema.<dialect>.json by default, so that file decides what DDL the next migration contains. `verify --db` compared the live database against the METADATA and never against it. A snapshot gone stale -- an interrupted migrate, a rollback, a bad merge -- passed clean, and the next `migrate --slug` emitted DDL that failed at apply. Reproduced end to end on a real Postgres before and after: drop a column from the snapshot, verify says `schema in sync`, migrate emits ADD COLUMN, apply gives `column "notes" of relation "contact" already exists`. An adopter had already been bitten and carried a manual workaround in its traps list. THE CHECK IS CONDITIONED ON metadata==DB, and that is the whole design. The snapshot advances at GENERATION time, so between `migrate --slug` and applying it the snapshot legitimately leads the database; there the metadata drift is non-empty and this stays silent. When metadata and the DB agree, nothing pending explains a difference. The first design keyed on the migration LEDGER instead, and the integration harness killed it: that harness applies its SQL directly, so there are no ledger rows, so every migration reads as pending and the gate would have silently never fired. Real projects apply out of band too (psql, a CI step). A gate that cannot fire is the defect being fixed, wearing a different hat. #293 -- TWO EMITTERS, TWO CONVENTIONS, NOTHING COMPARING THEM `check("chk_order_items_status")` in the generated table versus `ADD CONSTRAINT "order_items_status_chk"` in the migration. The name in the source never matched the name in the database. CODEGEN CHANGED, NOT MIGRATE. Migrate's suffix form is systematic across five constraint kinds and those names are already in live databases -- flipping migrate would emit DROP/ADD CONSTRAINT churn against production for a cosmetic fix. Codegen's prefix was two lines landing in regenerated source. Gated by a test that renders BOTH emitters from one model and asserts they agree, reading codegen's side off disk so it pins what an adopter receives. Each side was internally consistent and separately tested, which is how the split survived. @Verifiedby -- A NAME IN A COMMENT NO LONGER COUNTS AS EVIDENCE Auditing a real 19-name ledger found four claims that did not verify what they were attached to; one matched a `// via mountCrudRoutes(...)` note that was its only occurrence anywhere. Comment-only matches now emit WARN_REQUIREMENT_TEST_COMMENT_ONLY. WHOLE-LINE, not strip-to-EOL: a test titled with a URL contains `//`, and truncating there would turn a real match into a confident false error -- the failure this scan exists to avoid. Pinned by a test. `#` counts only in Python, where it is a comment. The docs now say plainly that @Verifiedby is existence evidence, not proof. The other three audited failures are semantic and no lexical rule reaches them, which is what FR-038 addresses: generate the test FROM the requirement so there is no name for an author to pick. Roadmap gains rows for FR-038 and FR-037 (whose row was missing). Full local CI green (18/18) after regenerating the advanced-modeling example, whose committed output pins a check name -- the golden lives outside the package suite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent d7617d5 commit 1db6e14

13 files changed

Lines changed: 627 additions & 11 deletions

File tree

CHANGELOG.md

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,79 @@ this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
77

88
## [Unreleased]
99

10+
### Fixed — `verify` gates the committed schema snapshot, which nothing checked (npm) — [#292](https://github.com/metaobjectsdev/metaobjects/issues/292)
11+
12+
`meta migrate` diffs metadata against `.metaobjects/migrations/.schema.<dialect>.json` by default
13+
(`--from-db` is the documented opt-out), so **that file decides what DDL the next migration
14+
contains** — and nothing in the toolchain verified it. `meta verify --db` compares the live database
15+
against the *metadata* and never against the snapshot.
16+
17+
A snapshot gone stale — an interrupted migrate, a rollback, a bad merge resolution — passed `verify`
18+
clean, and the next `migrate --slug` then emitted DDL that failed at apply. Reproduced end to end on
19+
a real Postgres: drop a column from the snapshot, `verify --db` still reports `schema in sync`,
20+
`migrate` emits `ALTER TABLE "contact" ADD COLUMN "notes" TEXT`, and applying it gives
21+
`column "notes" of relation "contact" already exists`. **The toolchain had everything it needed to
22+
know the snapshot was wrong and reported healthy.** An adopter had already been bitten and carried a
23+
manual "diff the snapshot after any migration-adjacent rollback" note in its traps list.
24+
25+
`verify --db` now compares the committed snapshot against the live database and fails when they
26+
disagree, naming the differences and how to re-derive it.
27+
28+
**The check is conditioned on metadata==DB, and that is what makes it false-positive-free.** The
29+
snapshot advances at migration-GENERATION time, so between `migrate --slug` and applying that
30+
migration it legitimately leads the database; in exactly that window the metadata↔DB drift is
31+
non-empty and this check stays silent. When metadata and the database agree there is no pending work
32+
left to explain a difference, so a snapshot that disagrees is stale.
33+
34+
Keying on the drift result rather than on the migration **ledger** is deliberate. A ledger-based
35+
"are there unapplied migrations?" test looks equivalent and is not: a project that applies its
36+
migrations out of band — `psql`, a CI step, another tool — has no ledger rows at all, so every
37+
migration reads as pending and the gate would silently never fire. That is the same class of defect
38+
as the one being fixed, and the integration harness (which applies its SQL directly) surfaced it
39+
before the design shipped. Fails open when no snapshot exists or it cannot be parsed; d1 is
40+
unaffected (its migrations stay Wrangler-native).
41+
42+
### Fixed — codegen and migrate named the same CHECK constraint two different ways (npm) — [#293](https://github.com/metaobjectsdev/metaobjects/issues/293)
43+
44+
For one `field.enum`, the generated Drizzle table emitted `check("chk_order_items_status", …)` while
45+
the migration emitted `ADD CONSTRAINT "order_items_status_chk"` — prefix versus suffix, same
46+
metadata, same version, same dialect. So the constraint name in the generated source never matched
47+
the one in the database: a `DROP CONSTRAINT` written from the generated name fails, a Postgres error
48+
quotes a name that appears nowhere in the source anyone would grep, and any reconciliation between
49+
the two (drizzle-kit introspect/push, a schema diff run as a sanity check) reports a difference that
50+
is not real.
51+
52+
**Codegen changed, not migrate**, and the direction is not arbitrary: migrate's suffix form is
53+
systematic across five constraint kinds (`_numeric_chk`, `_length_chk`, `_regex_chk`, `_cmp_chk`,
54+
`_chk`) and **those names are already in live databases**, so flipping migrate would emit DROP/ADD
55+
CONSTRAINT churn against production for a cosmetic fix. Codegen's prefix was two lines of one file,
56+
landing in regenerated source where changing it costs nothing.
57+
58+
Gated by a new test that renders both emitters from the same metadata and asserts the names match —
59+
reading codegen's side off disk rather than from an internal, so it asserts the text an adopter
60+
receives. Nothing compared the two before; each was internally consistent and separately tested,
61+
which is exactly how the divergence survived.
62+
63+
### Fixed — a `@verifiedBy` name found only in a comment now warns instead of passing (npm)
64+
65+
`checkVerifiedBy` matches a name anywhere in the test corpus as a whole word, so **a name occurring
66+
only inside a comment satisfied it.** Auditing a real 19-name ledger found four claims that did not
67+
verify what they were attached to, one of them matching a `// via mountCrudRoutes(...)` note that was
68+
its single occurrence in the entire corpus. A comment-only match now emits
69+
`WARN_REQUIREMENT_TEST_COMMENT_ONLY`, naming the file and line.
70+
71+
A **whole-line** comment test, deliberately, rather than stripping to end-of-line: a test titled with
72+
a URL contains `//`, and truncating there would turn a real match into a confident false error — the
73+
failure this scan exists to avoid. A trailing comment after code therefore still counts as code,
74+
which under-flags, matching the repo's standing bias for drift checks. `#` is treated as a comment
75+
only in Python files, where it is one.
76+
77+
`docs/features/requirements.md` now states the boundary plainly: **`@verifiedBy` is existence
78+
evidence, not proof.** The other three audited failures — a dependency-injection key, a real test of
79+
a different claim, and a test of the entry's output where the claim was about its source text — are
80+
semantic, and no lexical rule reaches them. Inverting the relationship so the test is *generated
81+
from* the requirement is specified as **FR-038**.
82+
1083
## [0.23.0] — npm `0.23.0` · PyPI `0.23.0` · NuGet `0.23.0` · Maven `7.23.0`
1184

1285
A coordinated **MINOR** across all four registries, cut as MINOR because it adds registered

docs/features/requirements.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,19 @@ entry.
149149
`@verifiedBy` names tests: `verify` checks each exists and is not skipped. It never runs
150150
them. `@trackedBy` names issues or tickets and is **not** resolved — `verify` has no network.
151151

152+
> **`@verifiedBy` is existence evidence, not proof — and the difference matters most to whoever
153+
> authored it.** The scan matches a name anywhere in the test corpus, as a whole word, in any
154+
> language; that generosity is deliberate (a "missing" verdict then means the name appears in no
155+
> test file at all, which is broken in any ecosystem) but it means the check **cannot tell whether
156+
> the named test verifies the claim.** Auditing a real 19-name ledger found four that did not: one
157+
> matched a **comment**, one a **dependency-injection key** in test setup, one a **real test of a
158+
> different claim**, and one a test of the entry's *output* where the claim was about its *source
159+
> text*. `verify` reported clean throughout. A comment-only match now warns
160+
> (`WARN_REQUIREMENT_TEST_COMMENT_ONLY`); the other three are semantic and no scan will ever reach
161+
> them. **After authoring `@verifiedBy`, open each named test and read what it asserts.** If the
162+
> claim has no test, write one rather than pointing at a name that happens to exist — a property
163+
> about source text (no forbidden identifier, no unbounded call) is testable by reading the file.
164+
152165
**Every run prints a summary**, clean or not:
153166

154167
```
Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
# FR-038 — Requirement-derived test stubs: invert `@verifiedBy` so the link cannot be faked
2+
3+
**Status:** proposed · **Date:** 2026-08-15 · **Depends on:** the `requirement.*` family (0.22.0,
4+
0.23.0) · **Supersedes nothing**`@verifiedBy` stays, with a narrowed role (§7).
5+
6+
## 1. The problem, measured
7+
8+
`@verifiedBy` asks the author to name a test that proves a requirement. `meta verify` then checks
9+
the name appears somewhere in the test corpus. **The author chooses the string, so the cheapest way
10+
to satisfy the check is to find any name that already exists** — and an agent optimising against a
11+
gate will take the cheapest satisfying move every time.
12+
13+
Auditing one real adopter ledger (55 entries, 9 carrying `@verifiedBy`, **19 names**) by opening
14+
each named test and reading its assertions found **4 of 19 did not verify the claim**:
15+
16+
| what the name matched | |
17+
|---|---|
18+
| a **comment** — its only occurrence in the entire corpus | `mountCrudRoutes` |
19+
| a **dependency-injection key** in test setup | `upsertLead: contactDeps.upsertLead,` |
20+
| a **real test of a different claim** | a test that the audit row survives a persistence *failure* |
21+
| a real test of the entry's **output**, where the claim was about its **source text** | `deriveContacts` on "the funnel names no source" |
22+
23+
`meta verify` reported **0 errors** throughout. The same agent authored the requirement statements
24+
and the evidence pointers, and nothing independent existed to disagree.
25+
26+
The scan is not defective — it is precision-over-recall on purpose, so a "missing" verdict means
27+
the name appears in **no** test file at all, which is broken in any ecosystem. It simply cannot
28+
distinguish verification from coincidence. Comment-only matches now warn
29+
(`WARN_REQUIREMENT_TEST_COMMENT_ONLY`), which reaches one of the four. **The other three are
30+
semantic and no lexical rule will ever reach them.**
31+
32+
## 2. The inversion
33+
34+
Stop asking the author to name a test. **Generate the test from the requirement.**
35+
36+
```
37+
requirement.functional leadRecord (L4, live)
38+
│ @statement / @violation
39+
40+
meta gen ──► tests/requirements/leadRecord.test.ts ← generated identity, hand-written body
41+
42+
43+
meta verify --codegen ──► the stub exists, is current, and matches the ledger
44+
```
45+
46+
The requirement's dotted path determines the test's file and name. **There is no string for the
47+
author to choose, so there is nothing to fake.** The claim and the test are the same object viewed
48+
from two sides.
49+
50+
## 3. Why this is the right shape
51+
52+
**It reuses the strongest gate instead of inventing a weak one.** A generated stub is an ordinary
53+
generated artifact, so `verify --codegen` already covers "the stub exists and is current" — the
54+
same drift gate that caught an adopter's two-release-old regen in the field. No new scan, no new
55+
diagnostic family, no new fail-open rules to reason about.
56+
57+
**It inverts the authoring gradient.** Today the cheapest satisfying move is "find a name that
58+
exists." With stubs the cheapest move is "fill in the red test already in front of you." The gate
59+
stops rewarding the shortcut.
60+
61+
**It matches this repo's own doctrine.** *Pattern-derivable from metadata = codegen, never
62+
hand-code.* A test's **identity and location** are derivable from a requirement; its **assertions**
63+
are not. That is exactly the generated-file-with-preserved-hand-edits split ADR-0034
64+
(scaffold-and-own) and the three-way merge already exist for: the stub is generated, the body is
65+
hand-written inside it and survives regeneration.
66+
67+
**It puts the claim where the work happens.** `@statement` and `@violation` are emitted as the
68+
file's doc comment, so whoever writes the assertion has the claim and its failure mode in front of
69+
them. That is better prompting than a ledger in another file, and it turns the audit from a search
70+
into a diff.
71+
72+
## 4. `@status` is already the emission switch
73+
74+
The vocabulary shipped in 0.23.0 turns out to encode exactly what each entry should emit:
75+
76+
| `@status` | emits | why |
77+
|---|---|---|
78+
| `planned` | a **skipped/todo** stub | intended, not built — a red build for something deliberately unbuilt is noise, and the existing scan already warns on skipped |
79+
| `live` | a stub that **fails until filled** | the claim says this works; an empty green test would assert the opposite |
80+
| `partial` | a stub that fails until filled, doc-commented with the `@disposition` and `@trackedBy` | a known gap still deserves the part that does work to be pinned |
81+
| `abandoned` / `superseded` | **nothing**, and an existing stub is removed | the entry's job is to record that this is gone |
82+
83+
An empty generated stub must **not** pass. A `live` stub therefore emits a failing assertion
84+
carrying the statement, not an empty body — otherwise the inversion recreates the original defect
85+
in a new place.
86+
87+
## 5. Scope
88+
89+
1. A `requirementTests()` generator in `codegen-ts`, emitting one file per `requirement.*` node at
90+
or below the link floor (L4/L5 — organisational tiers implement nothing and get no stub).
91+
2. Deterministic naming from the requirement's dotted path, and a collision rule (ADR-0044's
92+
payload-naming precedent applies: FQN-keyed with collision-scoped naming, never bare-name).
93+
3. Emission-by-status per §4, including **removal** on `abandoned`/`superseded`.
94+
4. `@statement` / `@violation` / `@implementedBy` rendered as the doc comment.
95+
5. Opt-in (§6), then the four non-TS ports, each through its own codegen (the pattern exists in all
96+
five).
97+
98+
## 6. The hazard, and the opt-in it forces
99+
100+
**An adopter with an existing ledger would get one new red test per requirement** — 245 of them on
101+
the poker estate, 55 on the other. That is hostile, and it would get the generator switched off
102+
permanently on first contact, which is the outcome to avoid above all others.
103+
104+
So: **opt-in, per generator and per entry.** A project adds `requirementTests()` deliberately; an
105+
entry can decline a stub. New requirements authored after adoption are the natural first users.
106+
`@verifiedBy` remains for the case stubs cannot serve — a suite that already exists, written before
107+
the requirement was, which is most of what an adopter has on day one.
108+
109+
## 7. What happens to `@verifiedBy`
110+
111+
It survives, narrowed and honestly documented:
112+
113+
- **Adoption path** — point at tests that already exist. Existence evidence, never proof, with the
114+
audit obligation stated in the docs (already done).
115+
- **Greenfield path** — the generated stub, where the link is structural.
116+
117+
An entry with a generated stub needs no `@verifiedBy`; the two are alternatives, not layers.
118+
119+
## 8. What this still does not solve
120+
121+
**A filled-in stub can assert something irrelevant.** The inversion removes the fakeable *name*,
122+
not the possibility of a weak assertion. What it buys is that the claim and the assertion are
123+
co-located and diffable, and that the gradient no longer rewards the shortcut.
124+
125+
The unfakeable formulation remains **`@violation`-driven mutation** — "the named test goes RED when
126+
the violation is real" — which the vocabulary already carries both halves of. It is deliberately
127+
out of scope here: `verify` is contractually forbidden from running tests (*"it never runs them"*
128+
is byte-gated in `expected-registry.json` across all five ports and restated in
129+
`spec/capability-ledger.md`), so proving belongs in a separate opt-in command or an adopter-CI
130+
recipe, specified separately. Generated stubs are the natural place for it to land later, since a
131+
stub can carry its mutation target as structured metadata rather than the hand-written
132+
`// MUTATION TARGET n:` comments adopters write today.
133+
134+
## 9. Open questions
135+
136+
- **Where do stubs live?** A dedicated `tests/requirements/` tree is greppable and easy to exclude
137+
from coverage; co-locating beside the implementation's tests is more idiomatic per ecosystem.
138+
- **Does a stub reference `@implementedBy`?** Importing the named entity would make the stub fail
139+
to compile when the model moves — a stronger link than a doc comment, but it couples the test to
140+
generated code the requirement may not otherwise touch.
141+
- **Renames.** A requirement renamed at L4 changes its stub's identity; the three-way merge cannot
142+
follow that, so the hand-written body would be orphaned. Needs a rename story before this is
143+
usable on a ledger that churns.
144+
- **Does the failing-stub default break `meta init`'s first run?** A scaffold that ships red is a
145+
bad first impression; likely the scaffolded ledger starts with `planned` entries only.

examples/advanced-modeling/src/generated/Program.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ export const programs = pgTable(
4242
},
4343
(table) => [
4444
check(
45-
"chk_programs_status",
45+
"programs_status_chk",
4646
sql`status IN ('draft', 'published', 'archived')`,
4747
),
4848
],

examples/advanced-modeling/src/generated/Purchase.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export const purchases = pgTable(
3232
},
3333
(table) => [
3434
check(
35-
"chk_purchases_status",
35+
"purchases_status_chk",
3636
sql`status IN ('pending', 'completed', 'refunded')`,
3737
),
3838
],

server/python/uv.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)