Skip to content

refactor(models): one retired-label refusal type on every write path; allow_ahead parsed once (architecture pass on #636) - #640

Merged
0x054 merged 2 commits into
feat/pinch-migration-needsfrom
refactor/deepen-retired-label-and-allow-ahead
Oct 12, 2026
Merged

0x054 merged 2 commits into
feat/pinch-migration-needsfrom
refactor/deepen-retired-label-and-allow-ahead

Conversation

@0x054

@0x054 0x054 commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Part of #623 (deepening pass on PR #636)

Two decisions that each lived in several places now live in one.

Card 2: one retired-label refusal on every write path

Take status: Status where Status.__ferro_retired_labels__ = frozenset({"canceled"}).

Before. Order(status="canceled") raised a ValidationError whose error type was value_error (the message prefixed Value error, ...). order.status = "canceled" with validate_assignment=True did the same. With validate_assignment=False, and in update(status=...) or update(lambda o: {"status": ...}), the type was retired_enum_label. Four places built the refusal: _label_guard, _guarded_setattr, retired_label_error and refuse_retired_label_write. The docstring said every path gave retired_enum_label, and the tests only checked the message text, so nothing caught the gap.

After. refuse_retired_label(enum_cls, labels, value) is the only refusal. It raises PydanticCustomError("retired_enum_label", <the ticket's text>). The field's AfterValidator is that function with its first two arguments bound, and Pydantic reports a PydanticCustomError from a validator under its own type. refuse_retired_label_write(model_cls, column, value) covers the paths that skip validation (__setattr__ without validate_assignment, and update() in both forms): it calls the refusal and raises the ValidationError that validation would have raised. Every path now gives errors()[0]["type"] == "retired_enum_label", loc == ("status",), and the same text as before. Hydration still skips the guard (I-2).

Visible change: construction, and assignment under validate_assignment=True, now report retired_enum_label where they used to report value_error, and errors()[0]["msg"] loses the Value error, prefix. Every other path is unchanged.

Interface test: test_every_write_path_refuses_with_the_retired_enum_label_type covers 5 write paths (construction, assignment, validated assignment, keyword update(), recipe update()) × StrEnum and plain Enum × SQLite and Postgres, and checks type, exact message and loc. RED before the change: 8 failures, which were construction and validated assignment for both enum kinds on both dialects. No other test changed.

Card 6: allow_ahead spelled once

Before. Python's check_allow_ahead and Rust's allow_ahead_arg each kept their own copy of False | True | "expand-only" and of the TypeError text. Python ran first, so the Rust branch could not be reached from the public API.

After. _core._allow_ahead(value) calls allow_ahead_arg and returns the value in its Python spelling, or raises its TypeError. up() and require_applied() call it before they resolve a target, so the refusal still comes before any connection opens. check_allow_ahead is deleted. The CLI calls up() and needed no change. The Literal annotations stay as types only. The TypeError text is unchanged byte for byte: allow_ahead must be False, True or "expand-only"; got 'sometimes'.

Interface test: test_allow_ahead_takes_only_its_three_values goes through the public API only, parametrised over 'sometimes', 1 and None. It asserts the exact text and points both calls at a SQLite URL that cannot be opened, so it now also proves the error comes before any connection. With check_allow_ahead deleted and nothing put in its place, the old version of this test still passed, because the Rust check fires after connecting but before any write. The strengthened test fails there with OperationalError: unable to open database file. Nothing imported check_allow_ahead, so no test was removed.

Verify

uv run maturin develop                                   exit 0
cargo check --all-targets                                exit 0
cargo test --no-default-features --features testing      exit 0 (354 passed)
uv run pytest -v tests/test_ir_vectors_contract.py       exit 0 (14 passed)
uv run pytest --db-backends=sqlite,postgres tests/test_retired_labels.py tests/test_retired_columns.py \
  tests/test_schema_enum_annotations.py tests/test_enum_cold_hydration.py tests/test_hydration.py \
  tests/test_migrations_api.py tests/test_migrate_up.py tests/test_step_class.py \
  tests/test_docs_examples.py tests/test_docs_migrations.py   exit 0 (545 passed, 6 skipped)
uv run prek run --all-files                              exit 0

Outside Files owned

None. In src/migrations_ffi.rs the only changes are the new _allow_ahead beside allow_ahead_arg and its registration.

🤖 Generated with Claude Code

https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR

0x054 and others added 2 commits October 11, 2026 23:23
The field validator raised ValueError, so construction and assignment
under validate_assignment surfaced as value_error while attribute set
and update() surfaced as retired_enum_label. refuse_retired_label is now
the one refusal (a PydanticCustomError the AfterValidator raises as is);
refuse_retired_label_write carries it out of the paths that run no
validation. Every path yields retired_enum_label with today's text.

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

check_allow_ahead (Python) and allow_ahead_arg (Rust) held the same
three-value vocabulary and TypeError text by hand. _core._allow_ahead
exposes allow_ahead_arg's parse; up() and require_applied() call it
before resolving a target, so the refusal still arrives before any
connection opens. check_allow_ahead is gone. The interface test now
pins the exact text and that no connection is attempted.

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

@0x054 0x054 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spec: PASS

No findings against the two cards (PR body is the spec; no sub-issue, Closes rule waived by the orchestrator).

  • Card 2: test_every_write_path_refuses_with_the_retired_enum_label_type ran 20 passed (5 paths × StrEnum/plain Enum × sqlite/postgres) and asserts type, exact msg, loc. refuse_retired_label (metaclass.py:116 at dc93200) is the only place that builds the refusal: the AfterValidator takes it via partial, and __setattr__, keyword update() and recipe update() (wire.py:566) take it via refuse_retired_label_write. Old _label_guard raised ValueError, which is why construction and validated assignment reported value_error.
  • Value error, prefix: no doc or test pins it. docs/pages/guide/schema/migrations.md:250 and docs/examples/migrations_retire_label*.py check only the message body (settings.py:721 strips the prefix for an unrelated path).
  • Hydration bypass: the existing hydrated-row save tests (test_retired_labels.py:149/197/338/410) pass.
  • Card 6: grep -rn "expand-only" src/ferro turns up only types, docstrings, the CLI annotation (cli/migrate.py:326, unchanged) and _core.pyi's stub for _allow_ahead. allow_ahead_arg (migrations_ffi.rs:299) is the only parser. The new FFI fn returns PyResult with no unwrap(). I swapped _core._allow_ahead for an identity function and ran the test: all 3 cases fail with OperationalError: unable to open database file, so the test does prove the TypeError comes before connecting.

Cannot verify: none.

Quality: PASS

No Critical or Important findings. I-2, I-3, I-6 hold (the refusal names False, True or "expand-only"). Tests: one strengthened (test_allow_ahead_takes_only_its_three_values), one added, none removed, and none assert internals. No churn in untouched files. Overlap with refactor/deepen-step-class-and-moves: src/migrations_ffi.rs register() gains one line (:27). That is not _generate_migration, but adjacent registration edits could conflict textually.

Merge preview: merging origin/feat/pinch-migration-needs → "Already up to date"; cargo check --all-targets exit 0.

Verify: passed on dc93200 (maturin develop 0; cargo check 0; cargo test 0, 354 passed; ir_vectors 0, 14 passed; pytest matrix 0, 545 passed / 6 skipped; prek 0). sdd-verify note: verified 2026-10-12T03:30:05Z.

@0x054
0x054 marked this pull request as ready for review October 12, 2026 10:42
@0x054
0x054 merged commit 36448ff into feat/pinch-migration-needs Oct 12, 2026
7 checks passed
@0x054
0x054 deleted the refactor/deepen-retired-label-and-allow-ahead branch October 12, 2026 10:47
0x054 added a commit that referenced this pull request Oct 12, 2026
… 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant