Repository navigation
ogar-from-ruff: lift Rails enum variant keys in declared order - #373
Merged
Merged
Conversation
lift_enum ignored AttrDecl options and returned Static(empty) although the ruby walker keeps the variant hash. Keys now land in the key slot in source order; the label is the key (no label is declared in Rails source; matches ogar-from-schema); the Rails integer stays out of the label slot because the emitter publishes it as variantValue. A duplicate key is refused (None). Empty options (array form, constant, no argument) stay key-less and are documented as 'not recovered'. Corrected the stale doc comment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XAqKMMWgnUU9g7Ldyn6sbk
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
|
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
lift_enum(crates/ogar-from-ruff/src/lib.rs) ignoredAttrDecl::options, so every Rails enum came out asEnumSource::Static(empty).Its doc comment blamed the walker for dropping the variant hash, but that is wrong. The
ruff_ruby_spowalker keeps every key, its integer, and the source order. Forenum :status, { shipped: 3, cancelled: 7 }the hash reachesoptionsthroughas_hash_options(ruff_ruby_spo/src/walk.rs:449-453). The loss happened in this lift.Change
lift_enumnow puts the variant keys intoEnumSource::Static, in declared order.ogar-from-schema; no "no label" convention exists.ogar-emitterpublishes that slot asvariantValue, and the integer is a source-local physical id. Its home is the source's versioned code map in DR, not the class's meaning. A test pins this.lift_enumreturnsNone, following the crate'sOption-returninglift_*convention. Rails itself rejects duplicate enum keys, so such a source is invalid input.The change is in
crates/ogar-from-ruff/src/lib.rsonly: about 35 non-test lines, mostly doc text.ogar_vocabis unchanged.Tests
Six new tests. Before the fix, four were red: keeps_variant_keys_in_declared_order, reorder_changes_order_not_key_identity, label_is_the_key_never_the_rails_integer, refuses_duplicate_key. The other two are guards: ordinary attributes are unchanged, and an enum-less class has no enums.
Disable run (after commit). I replaced the assignment with
Static(Vec::new())and asserted the patch applied. Three tests went red: keeps_variant_keys_in_declared_order, label_is_the_key_never_the_rails_integer and reorder_changes_order_not_key_identity. refuses_duplicate_key stayed green, because the duplicate check runs before that line.Validation
All runs used
CARGO_PROFILE_DEV_DEBUG=0 CARGO_PROFILE_TEST_DEBUG=0 CARGO_INCREMENTAL=0.cargo test -p ogar-from-ruff: 89 passed, 0 failed. I re-ran this independently.ogar-vocabandogar-from-rails(which wraps the same lift) pass.-D warningsand fmt are clean.ogar-doc-ir,ogar-from-docv1andogar-locopass.ogar-action-handlerhas 3 tests inogit_schema_compositionthat also fail on a cleanmainworktree (panic inogit_checkout.rs:23). The likely cause is the missing OGIT checkout. They are not from this change.Open (not in this PR)
Static, now documented as "variant list not recovered". An explicit marker needs either anEnumSourcevariant or a walker change (ruff PR-R1); that is a vocabulary decision.enum status: {...}produces no declaration at all in the walker. That is a ruff fix (PR-R1).Part of the OGAR × DR × DT64 semantic-continuity plan (producer fidelity, PR-O1).
Generated by Claude Code