Wrap where the include goes, whenever it lands - #52
Merged
Conversation
The concern only wrapped the associations a class had already declared, so the conventional placement of a concern — at the top of the model, above its associations — silently produced a half-converted model: nullable columns wrapped, associations not, and no signal that it had happened. Hooking belongs_to the way encrypts is already hooked makes placement irrelevant. Wrapping now runs through one path, which also lets errgonomic_optional_except name an association and not just an attribute.
h3h
previously approved these changes
Aug 12, 2026
Reading column_names in the included block made a model's class body require a live database connection: a class that includes the concern raises DatabaseConnectionError on load, where the same class without it loads fine. Any boot that loads models without a reachable database — an asset build, an image build, a schema check — fails on the include. ActiveRecord already has a seam for this. It defines attribute methods the first time a model needs its schema, and load_schema! is where that happens, so wrapping from there restores ordinary lazy loading. Two consequences follow: encrypts must record its exclusion for a schema that has not arrived yet rather than only reclaiming a reader, and a subclass now reaches the seam a second time, so wrapping has to see the readers an ancestor already wrapped or it would nest them.
Wrapping the columns at schema load left errgonomic_optionals answering with the associations alone until something else happened to touch the model, which is exactly backwards: the set is how a conversion gets checked, and it was empty right after the include. Asking now loads the schema, and the wrapping itself reads the raw list so it does not ask the schema to load while it is loading.
Wrapping happens in per-model hooks now — the association macros and the schema seam — so where the include goes decides how far it reaches. On a model, that model converts; on an application's base class, every model below it does, and no model mentions errgonomic again. Converting one model or the whole application is placement rather than a setting. An application's own base class is the useful place for it. Engine and gem models descend straight from ActiveRecord::Base, and their code reads their attributes knowing nothing about an Option, so they stay out of it by construction rather than by a list of namespaces this gem would maintain. Three things had to change for that include to work. An abstract class has no table, and the walk up the chain asked one for its columns, which raised before any model loaded. Exclusions were snapshotted at include time, but a base class include leaves a model no "before" to declare anything in, so they are read when a reader is about to be wrapped, and errgonomic_optional_except also takes back a reader already wrapped. And a model needs a way out that does not point at an include of its own: errgonomic_optional_off. Rubocop stops applying the size cops to test/, where splitting a case to satisfy one hides the behaviour it was written to name.
h3h
approved these changes
Aug 12, 2026
This was referenced Aug 12, 2026
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.
First of a stack. Where the include goes now decides how far it reaches, so this PR carries the hooks, the schema seam, and what a base-class include needed. Branches after it are stacked and I will open them one at a time as this merges:
nz/optional-has-one→nz/nested-attributes-unwrap→nz/cheap-recursion-guard.nz/idempotent-to-optionis independent and can be reviewed whenever.Wrapping followed the include's position, and only reached one class
included doread the reflections and the columns at include time, which made two things true and neither was intended.The include's position inside a model mattered. Put it where Rails convention puts a concern — at the top, above the associations — and the result was a half-converted model: nullable columns wrapped, associations not, nothing said so.
A class body needed a database. Reading
column_namesat include time meant a model that included the concern raisedActiveRecord::DatabaseConnectionErroron load where the same model without it loaded fine. Any boot that loads models without a reachable database — an asset build, an image build, a schema check — failed on the include.Both come from the same fix
encryptswas already hooked, because an attribute declared after the include has to be caught when it arrives.belongs_tonow works the same way, and the columns are wrapped fromload_schema!, which is where ActiveRecord defines its own attribute methods. Wrapping is no longer tied to the include's position, and no longer needs a database while classes load.Once wrapping lives in per-model hooks, the include reaches wherever it is put, so an application can convert every model at once by putting it on its own base class:
That is opt-in or opt-out as placement rather than configuration — no mode to set, no install step to order against the rest of boot. An application's own base class is the useful target: engine and gem models (
ActiveStorage::Blob,PaperTrail::Version) descend straight fromActiveRecord::Base, so they stay out by construction rather than by a namespace list this gem would have to maintain.Three things had to change to make that include work, each with a test that fails without it:
errgonomic_optional_exceptalso takes back a reader already wrapped.errgonomic_optional_off, alongside the existing per-attributeerrgonomic_optional_except.Two knock-ons worth reviewing
encryptsnow records an exclusion for a schema that has not loaded yet, rather than only reclaiming a reader that already exists.A subclass reaches the schema seam on its own, so whichever of parent and child is touched first would wrap the shared columns first. A child that got there first wrapped its parent's readers a second time:
Some(Some(x))for a present value, collapsing back toNonefor an absent one becauseNone#nil?is true — half of it silent. Wrapping walks the chain from the top down instead. Reproducible on the isolated case asNovel#isbn re-entered itself.errgonomic_optionalsalso had to start loading the schema, since it is how a conversion gets checked and it answered with the associations alone until something else happened to touch the model.Testing
rake— 24 tests, 63 assertions, and 118 doctests, all passing. Rubocop reports the same 7 pre-existing offenses asmain; the size cops stop applying totest/, where splitting a case to satisfy one hides the behaviour it names.