Repository navigation
feat: step class recorded at apply, additive tracking minor, allow_ahead="expand-only" (#626) - #632
Conversation
Every generated step carries its class, expand or contract, per dialect, decided once in run_plan::step_class from the planner's op verdicts and op kinds (ADR-0054's table), with the golden table pinned by cargo test. A hand-written SQL step declares expand with `-- ferro: expand`, a new header an older ferro refuses as it refuses any unknown header. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR
Every record writer (a run, a baseline) records the step's class in a new nullable `class` column; the format table gains `minor` and `format` stays 1, so ferro 0.22 reads, inserts and re-records beside them. The first mutating verb on a lower minor adds both columns under the run lock, records minor 1 and classifies every record without a class, from the directory's files or `contract` when the migration is absent. Readers never write and read a missing class as contract. The run planner takes AllowAhead (false, "expand-only", true); a refused ahead database carries contract_steps, and expand-only names the first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR
…d the CLI (#626) `up()`, `require_applied()` and `ferro migrate up --allow-ahead` take False, True or "expand-only"; anything else is a TypeError naming the three. DatabaseAheadError carries contract_steps (NNNN_<name>:NN), and `status --steps` prints each step's class after its flags. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR
…ly" (#626) The deploying guide's recipe switches to "expand-only" with True as the unbounded form, lists which steps are expand and which contract, states the write-notion boundary with the added-label example, and says the question is answerable after the first run since the upgrade. The API pages document DatabaseAheadError.contract_steps; a test pins the quoted refusal to the text ferro raises. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR
The generator reads each placed op's verdict off its plan; an op the plan did not decide is a generator bug and now raises GenerateError::Render instead of being classified without a verdict. The schema step's class reads the parent as its declared renames leave it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR
0x054
left a comment
There was a problem hiding this comment.
Spec: PASS
- Minor · crates/ferro-migrate/src/run_plan/step_class.rs:206 at 2c416f8 · Misunderstood · Produces names
step_class(step: &GeneratedStep | &PlannedStep, verdicts) -> StepClass; shipped asstep_class(ops: impl IntoIterator<Item=&PlannedOp>, sides: Sides, constrains: bool)plus siblings (schema_step_class,index_step_class,constraint_step_class,hand_sql_class,migration_classes). No sibling ticket consumes it (#629 consumes onlyAppliedMissing/SnapshotMismatchwording), so nothing breaks; the drift is undeclared in the PR body. - Minor · crates/ferro-ddl-lowering/src/lib.rs, crates/ferro-ddl-lowering/src/policy_expr.rs, crates/ferro-migrate/src/emit.rs, crates/ferro-migrate/src/tests.rs at 2c416f8 · Extra · rustfmt-only churn in four files outside Files owned and not declared in "Outside Files owned". I confirmed the base's
crates/ferro-ddl-lowering/src/lib.rsfailsrustfmt --edition 2024 --check, soprek run --all-files(a Verify command) forces it. No behaviour change, but it should be declared, since siblings may touch these files. - Minor · crates/ferro-migrate/src/run_plan/step_class.rs:330-382 at 2c416f8 · Missing (coverage) · design choice (3) has an untested sub-branch: regenerating with a hand
--data-step(hand_modeldetection). No Rust or pytest case applies or baselines a migration made with--data-step. Any mismatch falls back tocontract, which is the safe direction. The golden table also has no--no-backfillguard-step row (ADR-0054 names "guard step → expand"; the code gives every data stepDATA_STEP_CLASS).
Design choices the ticket left open, judged against ADR-0054:
- SQLite label addition (rebuild/
AlterColumnType,fails_on_rows: Always) read asexpandwhen the only change is a label superset: accepted. The ADR says "a label addition is expand" and "a SQLite rebuild carrying only such ops is expand". It is stated in the PR body and tested (an_added_label_is_expand, both dialects). - A
not-applicablestep isexpandon that dialect: accepted. It follows the ADR's write rule ("any step that can make an old release's write fail"). It is stated and tested (03_add_constraint/02_validaterows on SQLite). - Apply-time regeneration from the two snapshots, matched by step name, with an unmatched DDL step read as hand-written (contract unless
-- ferro: expand): accepted. It is still "one computation in the core" from the planner's verdicts, and its failure mode iscontract. It is stated and tested on the main path; see the coverage Minor above for the data-step branch. AppliedMissing.expand_only, so the default text is unchanged: accepted. It is stated and tested (plain.to_string()has no "contract").
Checked: no generated file gains a class header or any new text (GeneratedStep.class is #[serde(skip)]; the generator diff adds no header token; test_generated_files_carry_no_class_header). TRACKING_FORMAT stays 1, and the only format-table change is minor INTEGER NOT NULL DEFAULT 0. Readers (read_records) probe columns and never write (test_status_on_a_lower_minor_writes_nothing). Every ticket test scenario has a matching test.
Cannot verify: that a real ferro 0.22 wheel reads, inserts and re-records beside the minor-1 tables. The tests simulate 0.22 by dropping the columns and inserting with 0.22's column list, not by running the 0.22 binary.
Quality: PASS
- Minor · crates/ferro-migrate/src/run_plan/step_class.rs:353-357 at 2c416f8 ·
generate_with(...).ok().flatten().unwrap_or_default()swallows a generator error, so every DDL step of that migration is silently recordedcontract. The class is written once and kept byrerecord, so the wrong (if safe) class sticks, and an"expand-only"refusal then says "contract" with no hint that classification failed. That is in tension with I-6 ("fail loudly over degrading silently"). Commit 9774bcb made the generator raise loudly on an unplanned op, and this drops that error again at apply time. Fix: surface the error (refuse the verb naming the migration), or at least warn on the run report. - Minor · crates/ferro-migrate/src/run_plan/step_class.rs:372-373 at 2c416f8 · this re-derives the hand data step's name (
rsplit('.')plusformat!("backfill_{}", lower)) instead of reusinggenerate::short_model_name/backfill::hand_data_step's naming. That puts a second copy of one naming rule in the code. If they drift, the classification silently falls back tocontract.
Verify: passed on 2c416f8. uv run maturin develop: built and installed. cargo test -p ferro-migrate: 409 passed. cargo test --no-default-features --features testing: 351 passed. The pytest set on sqlite+postgres: 327 passed, 14 skipped. uv run prek run --all-files: exit 0. Verify note: verified 2026-10-11T16:43:56Z on HEAD.
…or, allow_ahead="expand-only" (#626) (#632) Every step has a step class, expand or contract, decided once in the core from the planner's op verdicts and recorded in the step record at apply time. `require_applied(allow_ahead="expand-only")`, `up(...)` and `ferro migrate up --allow-ahead=expand-only` admit a database ahead only by expand steps; `DatabaseAheadError.contract_steps` names the rest. The tracking table gains an additive `minor` (format stays 1), so the upgrade rolls back to 0.22. A hand-written SQL step is expand only with the `-- ferro: expand` header. Closes #626 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR
Closes #626
What
During a rolling deploy the previous release can now boot beside a database that is ahead of it only by changes it can live with:
Say release 1 adds
bio: str | None = None. Its migration's one step isALTER TABLE "author" ADD COLUMN "bio", an expand step: release 0's inserts keep working. Release 0 starts. If release 1 instead dropped a column, that step is contract and release 0 is refused, naming the step:DatabaseAheadError.contract_steps == ["0002_drop:01"].TrueandFalsebehave as before.How it works:
crates/ferro-migrate/src/run_plan/step_class.rsreads each op's verdict and kind (ADR-0054's table). The generator asks it for every step it lays out and keeps the answer per dialect onGeneratedStep.class, which is never written to a file. A run or a baseline regenerates the migration from its two snapshots and records that class. A hand-written SQL step iscontractunless its file says-- ferro: expand, a new header that 0.22's parser refuses like any unknown header.classcolumn. A rolled-back checkout that lacks the files still answers "ahead by what?" from the records alone.formatstays 1. The format table gainsminor(0.22 never selects it), and minor 1 is the class column. Every reader and writer names its columns, so 0.22 reads, inserts and re-records beside the new column. The first mutating verb (up,down,baseline,rerecord) on a lower minor adds both columns under the run lock and records minor 1. It then classifies every record without a class: from the files when the migration is present,contractwhen it is not.statusandrequire_appliednever write, and they read a missing class ascontract.status --stepsprintsexpand/contractafter the flags.rerecordkeeps the class.Verify
Ran the ticket's Verify block on
2c416f8: passed (cargo test -p ferro-migrate 409/409; cargo test --features testing 351/351; the listed pytest files on sqlite+postgres 327 passed, 14 skipped; prek exit 0). The full suite on both backends also passed (3984 passed, 60 skipped, 2 xfailed).TDD: RED
uv run pytest --db-backends=sqlite,postgres tests/test_step_class.py -xfailed as expected (RuntimeErrorfromRunRefused.from_corenot yet takingcontract_steps, so noDatabaseAheadError); GREEN, the same file passed 26/26. The Rust golden table was written first against the missingHeaders.expand/GeneratedStep.class(compile failure), then went green (15/15).Verify block corrected: no
Notes for the reviewer
Outside Files owned (each needed for the slice):
crates/ferro-migrate/src/generate/{mod,staging,backfill}.rs:GeneratedStepgainsclass(#[serde(skip)]). The generator is the only place that knows which op lands in which step, so it asksstep_classfor each step as it lays it out. The other files only initialise the field. ferro migrate new --replace NNNN_<name>: regenerate the newest migration carrying hand-written steps (#621) #629 editsgenerate/mod.rsafter this.src/migrations_ffi.rs:plan(allow_ahead=...)takesFalse,Trueor"expand-only"and raisesTypeErrorfor anything else.src/ferro/_core.pyi: the stub'sallow_aheadtype.Decisions the ticket left open:
--data-step <Model>), it regenerates with that step, because a hand data step moves the drops into the contract. A DDL step that regeneration does not produce (for example, one written by another ferro version) is read the way a hand-written step is:contractunless its file saysexpand.AlterColumnType,fails_on_rows: Always). The same goes for a check rebuilt only to admit more labels. Both are read as the label addition they are (expand), per the ADR's "a label addition is expand". Any other type change or check rebuild stayscontract.not-applicable) isexpandthere. This follows the ADR's write rule: a step that runs nothing cannot fail a write.ENABLE,FORCE, and dropping a policy can refuse an old write, so they arecontract.DISABLEandNO FORCEareexpand.AppliedMissinggainsexpand_only: boolbesidecontract_steps, so only an expand-only refusal adds the contract sentence. The default refusal text is unchanged.The
-- ferro: expandline in the migrations guide's Headers subsection is left to #629, per the plan ruling.🤖 Generated with Claude Code
https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR