Skip to content

feat(migrations): retired columns and labels, step class and expand-only, migrate new --replace, foreign defaults as drift (#623) - #636

Merged
0x054 merged 10 commits into
mainfrom
feat/pinch-migration-needs
Oct 12, 2026
Merged

0x054 merged 10 commits into
mainfrom
feat/pinch-migration-needs

Conversation

@0x054

@0x054 0x054 commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

Closes #623

Pinch does rolling deploys against ferro-managed databases and asked for five things the migration doors could not do (#618–#622). #618 shipped alone as the patch release in #630. This PR carries the other four PRDs, built as five slices on feat/pinch-migration-needs, each gated by review-pr and re-verified on the integration branch after merge.

What a user sees

Retire a column before dropping it (#619, #625). Release B declares bio: str | None = Field(retired=True). The field leaves the running model: no SELECT, INSERT or UPDATE names the column, so the code works whether the column is still there or the next release has dropped it. The column stays in the schema snapshot, nullable, with every index, check and foreign key that names it. Retiring a required column generates one change, ALTER COLUMN … DROP NOT NULL. Release C deletes the declaration and gets today's destructive DROP COLUMN, plus the DROP TYPE for an orphaned enum type. Contradictions (retired=True on a non-optional field, a primary key, a live renamed_from, nullable=False, a BackRef over it) are refused at class creation with a TypeError naming the fix.

Retire an enum label before removing it (#619, #627). __ferro_retired_labels__ = frozenset({"canceled"}) keeps the member, so rows holding the label still hydrate and save, while assigning the label is refused with an error naming it as retired. The label stays in the Postgres type and in any check; nothing renders DDL.

Boot the previous release beside an expand-only database (#620, #626). 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 when it is applied. 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 column; format stays 1, so the release that upgrades ferro still rolls back to 0.22. A hand-written SQL step is expand only with the -- ferro: expand header.

Regenerate a migration after a rebase (#621, #629). Two branches both generated 0007. ferro migrate new add_x --replace 0007_add_x regenerates it against the newest snapshot below it, carrying every data step and hand-written step by name with its body intact. check and the directory loader print the --replace line for each duplicate; the run's refusals name a replaced migration and its revert recipe.

Hand-set defaults are drift (#622, #628). The live read carries each column's server default. A default ferro did not leave is a foreign default: drift prints author.slug has default '' in the database, none declared, baseline refuses it, the Postgres pass drops it under migrate_destructive, the Alembic bridge writes alter_column(server_default=None). Ferro's own defaults (a serial key's sequence, SQLite's kept literal on a NOT NULL column) are never drift.

Design record

Already on main via #630: CONTEXT.md entries for Retired column, Retired label, Step class, Expand-only, Replaced migration, Foreign default, Tracked database; ADR-0053 (a column is retired one release before it is dropped); ADR-0054 (a step's class is recorded when it is applied); amendments to ADR-0027, 0030, 0031, 0034.

Narrowed (approved in the plan)

On SQLite the reconciliation pass never rebuilds a table, so for a foreign default it raises its existing rebuild UserWarning naming the fix instead of dropping the default; drift and baseline behave as on Postgres.

Slices

Ticket PR Verify on the integration branch
#625 retired columns #631 green at c5b9e72
#626 step class, tracking minor, expand-only #632 green at 3114085
#627 retired labels #633 green at 682c30b (also fixed a cross-slice compile break between #632's test literal and #631's new field)
#629 ferro migrate new --replace #635 green at cee6a19
#628 foreign defaults are drift #634 green at c9e3f61

Plan and wave log: #623 (comment) and #623 (comment)

Exit steps

🤖 Generated with Claude Code

https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR

0x054 and others added 5 commits October 11, 2026 12:50
…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
…nd stays in the snapshot (#625) (#631)

`Field(retired=True)`, `FerroField(retired=True)` and `ForeignKey(retired=True)` take a column out of the running model while it stays in the schema snapshot, nullable, with every artifact that names it. No statement the model issues names the column: the root fetch names the registration's mapped columns instead of `*`. Retiring a required column generates the planner's `DROP NOT NULL`; deleting the declaration next release generates the destructive `DROP COLUMN` and the orphaned enum type's `DROP TYPE`. Contradictions are refused at class creation with a `TypeError` naming the fix.

Closes #625

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR
…627) (#633)

`__ferro_retired_labels__ = frozenset({"canceled"})` on a `StrEnum` keeps the member, so rows holding the label still hydrate and save, while assigning the label at construction or by attribute set is refused with an error naming it as retired in this release. The label stays in the Postgres enum type and in any `db_check` check; retiring renders no DDL and writes no migration. The guard is a per-field validator attached only to enum fields whose enum retires a label; models without retired labels gain nothing. Also carries the `retired` field a `SchemaColumn` test literal in the step-class golden table was missing after #625 and #626 merged independently.

Closes #627

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR
…migration carrying hand-written steps (#629) (#635)

`ferro migrate new <name> --replace NNNN_<name>` regenerates the newest migration against the current newest snapshot below it: a new snapshot, regenerated DDL steps, and every data step and hand-written SQL step carried across by name with its body intact, renumbered when the regenerated steps move it. The number is kept when free and the next taken when not. The directory loader admits the duplicate at the replaced number for this verb only; `check` and the loader name both duplicates in full and print one `--replace` line each; the run's snapshot-mismatch and applied-missing refusals name a replaced migration and its revert recipe. The migrations guide gains "Two branches, one number" and documents the `-- ferro: expand` header.

Closes #629

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR
…efaults and DropDefault runs on every door (#628) (#634)

`ferro migrate drift` and `ferro migrate baseline` see column defaults. The live read carries each column's server default on both dialects, and the one planner plans a `DropDefault` op for every foreign default, a live default ferro did not leave. `drift` prints `<table>.<column> has default <literal> in the database, none declared`; `baseline` refuses it and its success sentence now says only that objects ferro does not own were not compared; the Postgres pass drops it under `migrate_destructive` and reports it; the SQLite pass warns naming the migrations door (the pass never rebuilds a table); the Alembic bridge writes `alter_column(server_default=None)` with an irreversible downgrade. Ferro's own defaults are never drift: the serial sequence a Postgres column owns, and a literal on a `NOT NULL` SQLite column. A casebook case pins every door.

Closes #628

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR
@0x054

0x054 commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor Author

Review panel: PR #636 (6a06786..c9e3f61), full-epic mode for #623

Verdict: HUMAN DECISION

No Critical finding, Spec PASS and Verify green, but nine Important findings and one Important Coherence finding, most of them in the step-class machinery that allow_ahead="expand-only" trusts: the class can be wrong (edited files, unchecked backfill, a retired label beside a label addition), it can be recorded silently on failure, and the first down after upgrading from 0.22 refuses.

Spec: PASS

From review-pr's full-epic review, verbatim:

No Critical or Important finding against the epic body. I read the **Narrowed:** line first. The SQLite pass on a foreign default warns naming the hand rebuild and changes nothing, and drift / baseline report and refuse as on Postgres (foreign_default_warning / sqlite_drop_default_report in crates/ferro-ddl-lowering/src/lib.rs, tests/test_migrate_defaults.py:385). That is judged as narrowed and met. Coherence C1 is a wrong class on one path of US 49 ("a label addition is expand"). It is reported once, under Coherence, and not repeated here.

  • Minor · tests/test_migrate_replace.py (whole file) · Missing · Nothing tests --replace together with --data-step, --data-only or --no-backfill. Only --sql-step with carried steps is covered, at the Rust layer. · Epic US 72 says --replace composes with all four flags "with their usual meaning". The code passes the flags through (prepare), but no test would fail if one of them broke.

Cannot verify: whether model_validate({"bio": ...}) should also be refused. It bypasses Model.__init__, so ferro's retired-field check never runs and extra="ignore" drops the key silently (US 12/13 name "construction with the keyword"; src/ferro/models.py:301). Whether this counts as "construction" is a call for the maintainer.

Coherence

From review-pr, verbatim:

Findings

Critical

none

Important

  • F1 adversarial · crates/ferro-migrate/src/run_plan/step_class.rs:308-312 @ c9e3f61 · conf 85 · A generated DDL step's class comes from regenerating it by name, never from its file [ADR-0054]
    migration_classes matches each Ddl step to a fresh regeneration by name and never compares the file's content or checksum. ADR-0030 allows editing an unfinished step: a DROP COLUMN added by hand to a generated expand step is still recorded expand, and its -- ferro: expand header is ignored for any step whose name regenerates. · expand-only then admits a rolled-back release beside a database that dropped a column it writes; the class is permanent once recorded, on a run and on a baseline alike. · Fix: use the regenerated class only when the regenerated body for the dialect matches the file's checksum; otherwise declared_class. Pin with a test that edits a generated step to add a drop. · verified by: read migration_classes/regenerate; no comparison with step.files.
  • F2 reliability · crates/ferro-migrate/src/run_plan/step_class.rs:355-361 @ c9e3f61 · conf 82 · Regeneration failure silently becomes a permanent contract class [I-6]
    regenerate swallows every generate_with error with .ok().flatten().unwrap_or_default(); the step falls to declared_class, contract without a header, and write_classes stores it where class IS NULL, never to be corrected. No error, warning, log or report marks the step as classified by failure; plan_baseline records the same fallback. · A migration that is expand is recorded contract for good, expand-only refuses a safe deploy, and the refusal names the wrong cause. · Fix: separate "generator returned an error" from "no step of this name": refuse the verb naming the migration and the fix, or leave the class NULL and warn; at minimum report it. · verified by: no error branch from generate_with; write_classes updates only WHERE class IS NULL.
  • F3 adversarial+data-migration · crates/ferro-migrate/src/run_plan.rs:2799-2825, src/run.rs:2261-2290 @ c9e3f61 · conf 80 · The class backfill matches records to files by number and name only, before the verb's checks [ADR-0054]
    bring_up_to_minor runs as the lock is taken, before check_records/check_applied; unclassified matches a record to a directory migration by number and dir_name and the step by ordinal, never by snapshot or step checksum; write_classes writes once and never revisits. After ferro migrate new --replace 0007_add_x kept the number, a 0.22 record is stamped with the class of the new SQL, and only then does the verb refuse with SnapshotMismatch. Same for a finished step whose file was edited or renamed. · The class is meant to be what was applied; here it can be the class of different SQL, written by a verb that then applies nothing, and expand-only trusts it on every later rollback check. · Fix: classify from files only when the record's snapshot checksum and step checksum match the directory; otherwise record contract or leave it NULL; or classify after the record checks pass. · verified by: read unclassified, bring_up_to_minor, locked_with ordering, write_classes, generate.write same-name branch; the unit test covers only matching files.
  • F4 correctness+adversarial · src/ferro/metaclass.py:103-141 @ c9e3f61 · conf 85 · The retired-label guard silently does nothing for a plain Enum [ADR-0053] [I-6]
    _label_guard and _guarded_setattr refuse only when isinstance(value, str) and value in labels. After validation a field typed as a plain class Status(Enum): CANCELED = "canceled" holds the member, not a str, so Order(status=Status.CANCELED) and order.status = Status.CANCELED write the retired label with no error. declared_retired_labels accepts the declaration on any enum, and every test uses StrEnum. · The declaration is honoured for StrEnum and ignored for Enum, so this release goes on writing a label it declared retired. · Fix: compare on value.value if isinstance(value, Enum) else value, or refuse __ferro_retired_labels__ on a non-str enum with a TypeError naming the fix; add a plain-Enum test. · verified by: read the two guards and declared_retired_labels; grep of tests/test_retired_labels.py shows only StrEnum.
  • F5 standards-and-invariants · src/ferro/metaclass.py:224-226 @ c9e3f61 · conf 88 · Retired primary-key refusal names no fix [ADR-0053] [I-6]
    Field(retired=True) on a primary key raises "<Model>.<field>: a primary key cannot be retired" and stops; its four sibling refusals each name the next step, and the migrations guide says all of them do. · ADR-0053's Consequences and I-6 require every refusal to name its fix; the docs state a behaviour the code lacks. · Fix: name the next step (drop retired=True; to remove a key, change the key first, see the primary-key recipe) and pin the text as the siblings are pinned. · verified by: the refusal against the sibling helpers in the same hunk; the ADR line and the docs sentence quoted.
  • F6 reliability · src/ferro/migrations/generate.py:263-275 @ c9e3f61 · conf 80 · --replace in place: a hard kill leaves the migration hidden and check does not name it [I-6]
    When the replacement keeps the directory name, write renames NNNN_x to the hidden .NNNN_x.replaced, then calls write_migration. A kill between the two leaves the only copy, hand-written and data steps included, in a dot-directory the directory reader skips (directory.rs:640); check and status say nothing, and a later --replace on the same name runs rmtree(aside) first, deleting the surviving copy. The except BaseException rollback covers exceptions, not a kill. · A destructive in-place operation with a crash window and no detection. · Fix: have the reader or check refuse on a leftover .*.replaced / .*.partial sibling naming the recovery; never rmtree an existing aside blindly. · verified by: read generate.write, layout.write_migration, the dot skip in directory.rs; .replaced appears only in generate.py.
  • F7 data-migration · src/ferro/migrations/runner.py:1105 @ c9e3f61 · conf 88 · The first ferro migrate down after upgrading from 0.22 always refuses [ADR-0054]
    down() previews without the lock; each step's standing is the record cloned whole, class included (run_plan.rs:2233), None for 0.22 records. tracked.locked() then runs bring_up_to_minor, which writes classes and re-reads, so the locked plan's standing differs from the preview's, the comparison at line 1105 fails, and down refuses with "the database's migration records changed while the plan was shown; nothing was reverted". A retry succeeds only because the first attempt wrote the classes. · Every database upgraded from 0.22 refuses its first rollback with a message about a concurrent change that never happened; this is the path the epic exists to keep safe. · Fix: leave class out of the standing comparison (a backfill of a missing class does not change the record), or classify before the preview under the lock, or compare only the fields that set the run's edit rules. · verified by: read runner.py:1085-1110, plan_handles, run_plan.rs:2233-2263, run.rs:2141-2170/2261-2290; tests/test_migrate_down.py seeds no classless records.
  • F8 adversarial · src/ferro/query/wire.py:569-584 @ c9e3f61 · conf 80 · update() writes a retired enum label without refusing it [ADR-0053]
    _literal_set now validates the column name (a retired column is refused) but never checks the value against __ferro_retired_labels__; Model.where(...).update(status=Status.CANCELED) writes the retired label into any number of rows. The guard exists only on construction and attribute set. · update() is the one write path that sets the label on many rows at once, and the path this PR already validates for retired columns; rows written this way defeat the next release's removal backfill. · Fix: apply the model's retired-label guard in _literal_set and the recipe path, raising the same retired_enum_label error before compiling. · verified by: read _literal_set; no update( with a retired label in tests/test_retired_labels.py.
  • F9 testing · tests/test_migrate_baseline.py:191 @ c9e3f61 · conf 80 · The baseline step-class test covers only expand steps
    test_a_baseline_records_every_steps_class_and_status_shows_them builds three expand steps and asserts all three classes and status --steps lines read expand; no baseline test has a contract step, and grep contract tests/test_migrate_baseline.py finds nothing. · The assertion holds if baseline hard-codes expand or skips the planner's decision, and an adopted database ahead by a contract step would then be admitted by expand-only. · Fix: add a drop-column step or a hand SQL step without the header to the fixture and assert contract in the record and in status --steps. · verified by: read the test body; the Rust golden table pins the planner's class, nothing pins that baseline records it.

Minor

  • F10 maintainability · crates/ferro-migrate/src/run_plan/step_class.rs:123-125 @ c9e3f61 · conf 80 · RedefineIndex class rule written twice in the one module meant to decide it once (op_class and index_step_class repeat unique_class(declared_index(...).is_none_or(...))); have one helper serve both.
  • F11 api-contract · docs/pages/reference/cli.md:88 @ c9e3f61 · conf 88 · The CLI reference omits --allow-ahead[=true|false|expand-only] on up and does not say status --steps now prints the class token; only deploying.md documents the flag.
  • F12 review-pr · src/ferro/migrations/errors.py:109 @ c9e3f61 · DatabaseAheadError always appends "Pass allow_ahead=True to run beside them (a rolling deploy)", including when "expand-only" refused because of contract steps, where ADR-0054 and the guide call True the wrong form; name the real choices when contract_steps is non-empty.
  • F13 maintainability · src/ferro/migrations/runner.py:128-133 @ c9e3f61 · conf 82 · The allow_ahead vocabulary is validated in three hand-kept places (check_allow_ahead, the Rust allow_ahead_arg with its own TypeError text, the Literal annotations in the CLI and _core.pyi); the Rust branch is unreachable from the public API. Keep one parser.
  • F14 maintainability · src/introspect.rs:720 @ c9e3f61 · conf 80 · Formatter-only reflow mixed into the live-read hunks (also in the Rust test files) hides the eight real lines in about a hundred and makes blame noisy.
  • F15 maintainability · src/migrations_ffi.rs:289-298 @ c9e3f61 · conf 90 · The new allow_ahead_arg doc comment was inserted under parse_direction's doc block, so the direction doc now describes allow_ahead_arg and parse_direction has none.
  • F16 reliability · src/operations.rs:2195 @ c9e3f61 · conf 80 · The root fetch now names columns, so a mapped column dropped by hand fails with the raw driver text ("column ... does not exist") on every query of that model, naming neither drift nor ferro migrate drift [I-6].
  • F17 review-pr · tests/test_migrate_replace.py @ c9e3f61 · Missing · --replace with --data-step, --data-only or --no-backfill is untested (epic US 72).

Invariants and ADRs

Checked: AGENTS.md (I-1 items 3–11 and pins (a)–(g), I-2, I-3, I-4, I-6, I-7, I-8, I-9, I-10, I-11); CONTEXT.md (Retired column, Retired label, Step class, Expand-only, Replaced migration, Foreign default, Tracked database, Step record, Tracking table, Run, Run lock, Op verdict, Plan report); ADR-0008, 0011, 0013, 0014, 0023, 0025, 0027 (amended), 0028, 0029, 0030 (amended), 0031 (amended), 0032, 0034 (amended), 0037, 0041, 0043, 0044, 0046, 0047, 0048, 0049, 0050, 0052, 0053, 0054, and the titles of all 56 ADRs; epic Implementation Decisions and Out of Scope; the plan's Narrowed line and the wave-2 ruling; ADR-0054's table row by row against step_class.rs; I-3 grep: every new unwrap()/expect() in src/*.rs and the crates is inside #[cfg(test)]; I-9 Closes #623 present; I-10 CHANGELOG.md absent from the stat.
Violations: F1, F3, F7 and C1 [ADR-0054]; F4, F5, F8 [ADR-0053]; F2, F5, F6, F16 [I-6].
Candidate ADRs: an ADR-0054 amendment recording two rules the code applies and the table does not: a step that renders not-applicable on a dialect (a validate or add-constraint step on SQLite) is expand there, and DropRowPolicy is contract. A statement of when a step's class may be re-decided (F1, F3 and the rerecord case all hinge on it: "the class is what the applied file did, read from the records; a backfill may compute it from files only when the file is the one that was applied").
Terminology drift: the epic says --replace prints "kept, moved, renumbered" while the Moved enum spells kept/renumbered/carried; the deploying guide calls the other baseline refusals "plain MigrationRefused" where they are RunRefused carrying a kind. Neither reaches a table name or a public attribute.

Coverage

Personas: correctness, testing, maintainability, standards-and-invariants, history, adversarial (8137 executable lines; migrations and record writes), reliability (lock and refusal paths on every door), data-migration (the tracking table gains a column and existing records are classified in place), api-contract (declaration API, exported errors, CLI flags and IR envelope that Pinch consumes).
Dropped below confidence 80: 0. Cannot verify from diff: a real 0.22 binary against minor-1 tables (every persona that touched it; the tests simulate 0.22); whether a 0.22 runner ever recreates the tracking tables in a way that drops the new columns; whether a data step carried by --replace still runs against the new parent's historical models; whether a backfill DEFAULT left by a ferro older than #608 should be dropped under migrate_destructive (a product call); whether model_validate({"bio": ...}) counts as construction for the retired-field refusal; whether an edited generated step of a replaced migration is flagged before it is regenerated away; the SQLite rebuild that relaxes a retired column keeping UNIQUE, CHECK, FK and AUTOINCREMENT (the test asserts the index and a round trip); whether the minor upgrade's ALTERs are visible anywhere but log_debug.
Verify: passed on c9e3f61 (the epic's aggregate Verify, every ticket's block deduplicated, each command's exit status checked: maturin develop; cargo check --all-targets; cargo test -p ferro-schema-ir, -p ferro-migrate, --no-default-features --features testing (354); pytest tests/test_ir_vectors_contract.py (14); pytest --db-backends=sqlite,postgres over 31 files: 1608 passed, 33 skipped, 1 xfailed; prek run --all-files clean; tree unchanged).

Residual risk

  • Post-deploy checks on each database: SELECT format, minor, governed_schema FROM _ferro_migrations_format (expect format=1, minor=1, one row) and SELECT migration, step, file, class FROM _ferro_migrations WHERE class IS NULL OR class NOT IN ('expand','contract') (expect no rows once one mutating verb has run). expand-only answers only after that first post-upgrade run.
  • The minor-1 ALTER TABLE ADD COLUMN on Postgres runs in ensure_tracking_tables with no lock timeout and needs ACCESS EXCLUSIVE on both tracking tables; a long-open transaction that read them stalls the upgrade and queues readers behind it. Check pg_stat_activity for idle-in-transaction sessions before the first deploy. The upgrade is otherwise idempotent and its only trace is log_debug.
  • A record whose migration directory is absent is persisted as contract, not just read as it; write_classes never revisits a classified row, so a later checkout that has the files cannot correct it without a manual UPDATE. Adding -- ferro: expand to an applied hand step and rerecording leaves the old class.
  • On Postgres every non-sequence default is foreign, so migrate_destructive drops backfill defaults left by older ferro versions and hand-set now()/gen_random_uuid() defaults; a previous release whose INSERTs relied on one starts failing NOT NULL. DropDefault is never written to a generated file, so its expand class is moot today.
  • Rolling back to 0.22: its upsert names only its own columns, so it keeps whatever class a newer ferro wrote; records 0.22 inserts read as contract until the next new-ferro mutating verb classifies them. format stays 1 and 0.22's check_format accepts the table.
  • model_copy(update={...}) bypasses both the retired-label validator and the guarded __setattr__; no test covers it.
  • The retired-field machinery deletes entries from cls.__pydantic_fields__ and rebuilds with raise_errors=False, widening an existing dependence on Pydantic internals that a future Pydantic bump could break silently.
  • src/hydration.rs:275's precondition comment still says callers decode SELECT table.*.
  • Keyword update() now fails client-side with AttributeError for an unknown or retired name where it previously reached the database; the root fetch no longer hydrates undeclared live columns; the baseline success sentence changed. All deliberate and tested, but visible to a caller that depended on the old behaviour.

Fix round outcome (2026-10-12)

The maintainer chose a fix round for every Important finding and C1. Each landed as a fix PR into feat/pinch-migration-needs, gated by a scoped re-review by the persona that raised the finding and a merge-preview compile against the integration branch, then merged with Verify re-run there.

Fix PR Findings Re-review
#637 F6 ADDRESSED (#637 (review))
#638 F4, F5, F8 all ADDRESSED (#638 (review))
#639 F1, F2, F3, F7, F9, C1 all ADDRESSED after two rounds; round 2 narrowed F2's unclassifiable refusal to up and baseline while down, rerecord and baseline --remove proceed with the class left NULL and a warning (#639 (review), #639 (review))

This PR's head is now 6246f83. The epic's aggregate Verify (every ticket's block, deduplicated) passed there with each command's exit status checked: maturin develop; cargo check --all-targets; cargo test -p ferro-schema-ir, -p ferro-migrate, --no-default-features --features testing (354); the IR vector contract (14); pytest --db-backends=sqlite,postgres over 31 files: 1637 passed, 33 skipped, 1 xfailed; prek run --all-files clean; tree unchanged.

Revised verdict: MERGE. No Critical, no open Important, Spec PASS, Coherence resolved, Verify green. Minors F10–F17, one wording nit on the new refusal ("none of its steps was applied" reads better as "this run applied nothing"), the Candidate ADRs and the terminology drift go to close-epic's Learnings. The merge to main is the maintainer's.


Deepening pass (2026-10-12)

After the fix round, improve-pr-architecture ran over this PR and the maintainer asked for its recommended candidates to be implemented and merged here rather than as follow-ups. Two refactor PRs, each gated by review-pr (Spec PASS, Quality PASS, no Important findings) and checked to merge cleanly together:

PR What Review
#641 One step_class entry point deciding from the laid-out step, with constrains derived from the step's own ops through the staging code's own function; Carried names its carry index so the Python side maps files instead of re-matching by name. Behaviour unchanged at every former call site. #641 (review)
#640 One retired_enum_label error type on every write path (construction and validated assignment used to report value_error); allow_ahead parsed once, in the core, before any connection opens. #640 (review)

Three further candidates (a retired-column object on the model, a live-only server-default field in the IR, RunRefused payload typed by kind) go to close-epic's Learnings as follow-ups.

This PR's head is now 36448ff. The epic's aggregate Verify passed there with each exit status checked: all crates (--features testing 354), the IR vector contract (14), pytest --db-backends=sqlite,postgres over 31 files: 1661 passed, 33 skipped, 1 xfailed; prek clean; tree unchanged. Verdict stands: MERGE.

…ver deleted blindly (panel F6) (#637)

Panel finding F6 on PR #636: `ferro migrate new --replace` set the old directory aside under a hidden name before writing the new one, and a hard kill in between left the only copy where the directory reader skipped it and a later `--replace` deleted it first. Every directory read now refuses a ferro-named leftover (`interrupted_replace`, `interrupted_write`) naming the recovery, a renumbered replacement's aside carries the new directory's name so the reader can tell whether it completed, and nothing deletes an existing aside.

Part of #623

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR
0x054 and others added 2 commits October 11, 2026 22:51
… the primary-key refusal names its fix (panel F4, F5, F8) (#638)

Panel findings F4, F5 and F8 on PR #636. The retired-label guard now compares the value that would be written, so a plain `Enum` member carrying a retired label is refused like a `StrEnum` one, at construction, on attribute assignment under either `validate_assignment` setting, and in keyword and recipe `update()`; hydration still bypasses it and an unchanged label is still written back by `save()`. The retired primary-key refusal names its fix, and the migrations guide's sentence matches.

Part of #623

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR
… after upgrade succeeds (panel F1, F2, F3, F7, F9, C1) (#639)

Panel findings F1, F2, F3, F7, F9 and Coherence C1 on PR #636. A generated step's class is the regenerated one only when the regenerated rendering matches the applied file's checksum, otherwise the file's own declaration decides (F1). A migration that fails to regenerate makes `up` and `baseline` refuse with `unclassifiable`, naming the fix, before any tracking-table change; `down`, `rerecord` and `baseline --remove` proceed with the class left NULL and a warning (F2). The class backfill trusts the directory's files only when snapshot checksum, step file name and step checksum match the record, otherwise `contract` (F3). The first `down` after upgrading from 0.22 no longer refuses on the backfilled class (F7). The baseline class test pins a contract step (F9). `adds_labels_only` ignores fields that render no storage, so a label addition beside a retired label is `expand` (C1). Candidate ADR recorded in the PR: when a class may be decided from files.

Part of #623

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR
0x054 and others added 2 commits October 12, 2026 06:41
…ch carried step's move (architecture pass on #636) (#641)

Architecture pass on PR #636, cards 1 and 4; behaviour unchanged. The step class has one entry point: the generator describes each laid-out step and the run planner's module decides, deriving the `constrains` flag from the step's own ops through the same function the staging code uses; the duplicated unique-index rule is gone; the apply-time path classes data and hand steps through the same entry point. `Carried` names which carry entry a step came from, so the Python side maps files by index instead of re-matching steps by name.

Part of #623

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR
… allow_ahead parsed once (architecture pass on #636) (#640)

Architecture pass on PR #636, cards 2 and 6. One function raises the retired-label refusal as a `retired_enum_label` error on every write path, construction and validated assignment included, which used to surface as `value_error`; hydration still bypasses it. The `allow_ahead` vocabulary is parsed once, in the core, exposed over the FFI and called before any connection opens; the Python duplicate is gone, and the interface test now proves the refusal arrives before connecting.

Part of #623

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR
@0x054 0x054 changed the title feat(migrations): Pinch migration needs: retired columns and labels, step class and expand-only, migrate new --replace, foreign defaults as drift (#623) feat(migrations): retired columns and labels, step class and expand-only, migrate new --replace, foreign defaults as drift (#623) Oct 12, 2026
@0x054 0x054 changed the title feat(migrations): retired columns and labels, step class and expand-only, migrate new --replace, foreign defaults as drift (#623) feat(migrations): retired columns and labels, step class and expand-only, migrate new --replace, foreign defaults as drift (#623) Oct 12, 2026
@0x054
0x054 merged commit 183d2c3 into main Oct 12, 2026
7 checks passed
@0x054
0x054 deleted the feat/pinch-migration-needs branch October 12, 2026 13:58
0x054 added a commit that referenced this pull request Oct 12, 2026
…ver deleted blindly (panel F6) (#637)

Panel finding F6 on PR #636: `ferro migrate new --replace` set the old directory aside under a hidden name before writing the new one, and a hard kill in between left the only copy where the directory reader skipped it and a later `--replace` deleted it first. Every directory read now refuses a ferro-named leftover (`interrupted_replace`, `interrupted_write`) naming the recovery, a renumbered replacement's aside carries the new directory's name so the reader can tell whether it completed, and nothing deletes an existing aside.

Part of #623

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR
0x054 added a commit that referenced this pull request Oct 12, 2026
… the primary-key refusal names its fix (panel F4, F5, F8) (#638)

Panel findings F4, F5 and F8 on PR #636. The retired-label guard now compares the value that would be written, so a plain `Enum` member carrying a retired label is refused like a `StrEnum` one, at construction, on attribute assignment under either `validate_assignment` setting, and in keyword and recipe `update()`; hydration still bypasses it and an unchanged label is still written back by `save()`. The retired primary-key refusal names its fix, and the migrations guide's sentence matches.

Part of #623

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR
0x054 added a commit that referenced this pull request Oct 12, 2026
… after upgrade succeeds (panel F1, F2, F3, F7, F9, C1) (#639)

Panel findings F1, F2, F3, F7, F9 and Coherence C1 on PR #636. A generated step's class is the regenerated one only when the regenerated rendering matches the applied file's checksum, otherwise the file's own declaration decides (F1). A migration that fails to regenerate makes `up` and `baseline` refuse with `unclassifiable`, naming the fix, before any tracking-table change; `down`, `rerecord` and `baseline --remove` proceed with the class left NULL and a warning (F2). The class backfill trusts the directory's files only when snapshot checksum, step file name and step checksum match the record, otherwise `contract` (F3). The first `down` after upgrading from 0.22 no longer refuses on the backfilled class (F7). The baseline class test pins a contract step (F9). `adds_labels_only` ignores fields that render no storage, so a label addition beside a retired label is `expand` (C1). Candidate ADR recorded in the PR: when a class may be decided from files.

Part of #623

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR
0x054 added a commit that referenced this pull request Oct 12, 2026
…ch carried step's move (architecture pass on #636) (#641)

Architecture pass on PR #636, cards 1 and 4; behaviour unchanged. The step class has one entry point: the generator describes each laid-out step and the run planner's module decides, deriving the `constrains` flag from the step's own ops through the same function the staging code uses; the duplicated unique-index rule is gone; the apply-time path classes data and hand steps through the same entry point. `Carried` names which carry entry a step came from, so the Python side maps files by index instead of re-matching steps by name.

Part of #623

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant