[1b] Add typed where() with field-method conditions by davegaeddert · Pull Request #84 · dropseed/plain

davegaeddert · 2026-07-23T15:48:59Z

What

A typed query API alongside filter():

User.query.where(
    User.email.equals("foo@example.com"),
    ~User.role.equals("admin"),
    User.age.gte(18),
)

Each argument is a Q built from a real field, and positional conditions are ANDed. Because every condition comes from the field itself, a type checker rejects a misspelled field or a wrong value type at the call site — unlike string keyword lookups. QuerySet.where(*conditions: Q) takes no kwargs at all, which is what makes that guarantee total.

Conditions live on `Field`

All of them, including the string-only ones:

Every field: equals, not_equal, gt, gte, lt, lte, is_null, is_in
String-valued fields: also contains, icontains, startswith, endswith

The string ones are on Field too, restricted by their self annotation:

def contains(self: Field[str] | Field[str | None], value: str) -> Q: ...

That restriction is doing real work. Since #83, models annotate fields with the base Field[T] — a field's declared type is Field[str], never TextField[str] — so anything living only on a subclass is invisible to the checker. Putting the methods on Field is what makes them reachable; the self type is what keeps Field[int].startswith(...) an error. It also means every string-valued field gets them regardless of base class: GenericIPAddressField is a DefaultableField and RandomStringField is a ColumnField, and neither inherits TextField.

There is deliberately no runtime counterpart to that restriction. Contains, IContains, StartsWith and EndsWith are registered on Field itself (@Field.register_lookup in lookups.py), so IntegerField.get_lookup("contains") returns a real lookup and nothing at runtime tells a string field from an int one. The type level is the guard — the same type-first model the encrypted fields use — and the test says so rather than pretending otherwise. _build_q does reject a lookup a field hasn't registered at all.

is_in refuses a bare str/bytes at runtime. Iterable[str] is satisfied by str, so name.is_in("abc") type-checks and would iterate characters; str is a Sequence[str], so excluding it statically isn't expressible either. The error names the list spelling.

The method names are listed once, as CONDITION_METHODS / STRING_CONDITION_METHODS in fields/base.py next to the methods. Traversal and the tests import from there.

Foreign-key traversal

Accessing a field through a relation builds the joined lookup:

Post.query.where(Post.author.email.equals("a@example.com"))  # Q(author__email="...")
Post.query.where(Post.author.profile.city.equals("Topeka"))  # chains

The mechanism is one method — Field.with_lookup_prefix(prefix) returns a detached copy of the field (via the existing Field.__copy__) whose name carries the relation path. Traversal hands back the related model's own field, so its own condition methods build the right keys. Nothing re-implements or rewrites a field's surface, which means the traversed surface equals the direct surface by construction rather than by a gate that has to be kept in sync — including an encrypted field's refusals, whose message names the full store__api_key path for free.

Every relation encountered along the way is another hop, many-to-many included — WidgetTag.widget.tags.name.equals("metal") builds Q(widget__tags__name="metal"). One rule serves the first hop and every later one: the descriptor builds a RelatedFieldRef once (a cached_property, since the target may still be a string at construction) and delegates to it. That hop is runtime-only at the type level for now: tags is declared ManyToManyManager[Tag], and claiming class access to an M2M yields type[Tag] would be false for Widget.tags itself, which is not a traversal entry point.

Conditions on the relation itself

A relation is not a field, so it carries no conditions. Match on the key it points at:

Post.query.where(Post.author.id.equals(author.id))
Post.query.where(Post.author.id.is_in([a.id for a in authors]))
Post.query.where(Post.author.id.is_null())        # nullable relation

This is the typed spelling of filter(author=author) and compiles to the same author__id= lookup — asserted directly in a test, not just claimed. Post.author.equals(author) raises an AttributeError naming this spelling (an AttributeError, not a TypeError, so hasattr and getattr(..., default) keep behaving).

It can't be smoothed over: to the type checker Post.author is type[Author], which is precisely what makes Post.author.email.equals(...) type-check, and there is nowhere on type[Author] to hang a condition method. A runtime method the checker rejects at every call site would be worse than none.

Class-level FK access returns the descriptor

ForwardForeignKeyDescriptor.__get__(instance=None) returns self, the ordinary descriptor convention. Traversal is served by __getattr__, which only fires for names the descriptor doesn't define.

That works because the descriptor's own attributes are now _-prefixed — _field, _is_cached, _get_queryset, _get_prefetch_queryset — and Meta skips _-prefixed attributes when it collects fields (meta.py), so a field name can never collide with a descriptor attribute. Shadowing is closed at the source rather than routed around: previously a related model with a column named field or get_queryset silently resolved to the descriptor's attribute and built wrong SQL with no error.

_get_prefetch_queryset is the duck-typed prefetch protocol, so the two manager classes and the single call site in query.py are renamed with it. Nothing outside plain-postgres references any of these; the managers keep their public get_queryset, and RelatedObjectDoesNotExist stays public on purpose (it is user-facing and documented).

Returning the descriptor also means no inspect.getattr_static workarounds in the prefetch path or __reduce__, no object allocated per class-level read, and no function-local import re-executed on every read: 39 ns per access, against 257 ns for the proxy this replaces.

RelatedFieldRef survives for hops beyond the first, where there is a relation path to accumulate and no field to hand back yet.

Encrypted fields: one runtime guard, eleven static declarations

Encrypted fields store non-deterministic ciphertext, so a value comparison can never match. is_null() is the one condition that stays open — it reads the column's NULL-ness, not its contents.

The runtime block is a single _build_q override on EncryptedField, refusing any suffix but isnull. Every condition funnels through _build_q, so this covers conditions that don't exist yet, and the message names the method the caller actually wrote.

The static block is eleven bodiless Never-typed declarations under if TYPE_CHECKING:. Narrowing an inherited parameter to Never is a Liskov violation — that is what makes it a compile-time block — so each carries # ty: ignore[invalid-method-override].

`EncryptedField[T]` is the annotation

api_key: EncryptedField[str] = types.EncryptedTextField(max_length=200)

not Field[str]. The annotation is what the checker reads, and this is the Field[T] subclass that declares the blocks; a plain Field[str] hides them and lets Model.api_key.equals("x") type-check its way to a runtime TypeError. It types the synthesized constructor exactly as Field[T] does. Exported from plain.postgres alongside Field.

Because a class-level suppression could otherwise hide a block that stopped blocking, test_every_blocked_method_is_rejected_statically pins the surface from outside: every blocked method gets a call site carrying # ty: ignore[invalid-argument-type] inside a pytest.raises(TypeError). ty reports an unused suppression as an error, so widening any parameter away from Never fails the type check — and the same statements assert the runtime raise, pinning both sides together. test_is_null_survives_the_block_statically carries no marker, so a Never creeping onto is_null() also breaks the build.

Behavior change: `get_or_create()` on an encrypted field now raises

get_or_create(api_key="k") used to "work" — and quietly created a new row on every call, because the lookup could never match existing ciphertext. It now raises, and the message says to move the value into defaults=:

Integration.query.get_or_create(name="acme", defaults={"api_key": "k"})

Same for update_or_create() and for an expression right-hand side like filter(api_key=F("name")). This is the deliberate half of the block: silent duplication is what it exists to prevent. An encrypted value can be written, just not looked up.

What still matches: None, and — on a string-valued encrypted field — the empty string, which is stored as plaintext '' (that is what makes default="" expressible as a column DEFAULT). So filter(token=""), exclude(token="") and token.equals("") all keep working, and the typed path and the kwarg path allow exactly the same set. equals/not_equal are narrowed to those values statically rather than blocked outright; the other nine conditions stay Never.

The kwarg path is blocked the same way: get_lookups() owns the complete registry and registers _EncryptedExact for "exact", which raises on a non-None right-hand value instead of silently returning zero rows, while None passes through so the exact-None → isnull rewrite still works. filter(api_key=None) still means IS NULL; filter(api_key="x") raises. It reads the field name from lhs.output_field.name and keeps its own sentence, because filter() is supported here — just not against a value.

None operands on ordering conditions

age.gte(None) used to get past the checker and die much later as ValueError("Cannot use None as a query value"), with no field in the message. It now raises a TypeError at the call site naming the field and the method. equals(None) (the ORM's exact-None rewrite) and is_null() stay legal.

This one cannot be fixed statically. On a nullable field T includes None, and None can't be subtracted from a TypeVar. Probed against ty 0.0.80 and pyright 1.1.414: self: Field[X | None], value: X does select the intended overload (confirmed by discriminating on distinct return types), but both solve X as int | None, so gte(None) type-checks either way. A constrained TypeVar doesn't help. The runtime refusal is the guard, and the test records why there is no static pin next to it.

Also in this branch

The shadowing fixtures add ShadowTarget/ShadowSource to the examples test app, and the string-condition fixture adds StringConditionsExample (the repo's only GenericIPAddressField model, so it is what the end-to-end pattern tests run against). That moves the app's leaf migration, so tests/internal/test_migrations_reset.py — which had pinned the leaf, baseline name, migration numbers and file counts by hand — now derives all of them from the real history, including the models the leaf creates. Any future branch that adds a migration to examples is unaffected.

That part is independent of where().

Tests / checks

./scripts/fix — clean
./scripts/type-check . — clean, whole repo (ty 0.0.80). The # ty: ignore[...] markers in the typed-where and encrypted-field tests are load-bearing regression detectors, not noise. The overload choices above were cross-checked against pyright 1.1.414 as well.
./scripts/test — all 27 package suites plus the example project green: 2613 passed, 1 skipped. plain-postgres: 983 passed, 1 skipped. plain: 697 passed.

Diff against master: 24 files, +1826 / −113.

Two annotation notes that outlive this PR. Never annotate a field Field[Any]: Any satisfies the model-valued __get__ overload, so class access types as type[Any] and the whole condition surface disappears without an error — use the concrete shape, or Field[object] when the column really does hold arbitrary JSON (plain-cache's value changes here for exactly that reason). And since class access now types as the related model, Model.fk.RelatedObjectDoesNotExist is a type error: catch Related.DoesNotExist (the exception subclasses it) or AttributeError.

First slice of the typed query API. Field classes are now generic over their value type T; stubs return the parameterized descriptor instead of the primitive. Combined with Field's overloaded __get__, this gives: class User(postgres.Model): email = types.EmailField() note = types.TextField(allow_null=True, required=False) User.email # EmailField[str] — typed reference, .equals()/.contains()/... user.email # str — value type preserved User.note # TextField[str | None] user.note # str | None — nullability preserved Adds equals/not_equal/gt/gte/lt/lte/is_null on Field[T] and contains/icontains/startswith/endswith on TextField, each returning Q. New QuerySet.where(*conditions: Q) accepts them positionally with no **kwargs, so a type checker can reject field typos and value-type mismatches at the call site. Coexists with filter()/exclude(). Model field declarations across the monorepo drop their primitive annotation (`name: str = types.TextField()` → `name = types.TextField()`). The descriptor protocol handles both class- and instance-access typing.

Override equals/not_equal/gt/gte/lt/lte on EncryptedFieldMixin with a `Never`-typed parameter so a type checker rejects the call at the use site, and raise TypeError at runtime as a safety net. is_null remains the only meaningful comparison since ciphertext is non-deterministic. This makes the design's "method surface = capability" promise actually load-bearing: misuse is a type error, not a runtime no-op or a silently empty filter.

Address review findings on the previous commit: 1. The typed `.equals()` block doesn't help users on the legacy kwarg path. `filter(api_key='x')` still resolved via the allowed `exact` lookup and silently returned zero rows (ciphertext is non- deterministic). Wrap the exact lookup class in `get_lookup()` so non-None right-hand values raise TypeError at lookup construction. None still passes through, preserving the exact-None → isnull rewrite. 2. Strengthen `is_null` test to inspect the Q's children instead of just asserting isinstance — a regression in `Field.is_null` would have slipped past the old check. 3. Add `assert self.name is not None` in the error-message helper, so a call on an unbound field fails loudly instead of rendering "None" in the error. 4. Change override return type from `Q` to `Never` to match the actually-unreachable return; `Never` is assignable to `Q` so call sites like `where(field.equals(...))` still type-check at the use site, with the `Never` parameter error as the surfaced diagnostic. 5. Parametrize the ordering-comparison test so each method (gt/gte/lt/ lte) reports independently.

Extend the typed read API across forward foreign keys: Child.parent.name.equals("foo") → Q(parent__name="foo") Order.user.profile.city.equals(...) → Q(user__profile__city=...) Runtime: new RelatedFieldRef / PrefixedFieldRef helpers proxy class-level attribute access through to the related model's fields, accumulating a lookup-path prefix as it goes. ForwardForeignKeyDescriptor gets a __getattr__ that yields the initial RelatedFieldRef so chaining starts from `Child.parent`. The SQL builder's existing names_to_path / join machinery handles the rest — we just produce correctly-prefixed Q. Typing: ForeignKeyField stub now returns a _ForeignKeyDescriptor[T, V] with overloaded __get__ — class access yields `type[T]` (so the related model's typed field surface is visible) and instance access yields V (T or T | None). __set__ is overloaded to accept V | int so bare PK assignment still type-checks. Migration: drops `: ModelType = types.ForeignKeyField(...)` annotations across the monorepo. The new stub provides the descriptor type and the overloads handle both class- and instance-side typing. Scope: forward FK only. Reverse FK (ReverseForeignKey) and M2M traversal will follow the same pattern in a separate commit.

1. Encrypted-field bypass: PrefixedFieldRef now calls _reject_if_blocked in equals/not_equal/gt/gte/lt/lte and the string lookups. If the wrapped field is an EncryptedFieldMixin instance, raise TypeError with the same "use .is_null() instead" hint the direct-access path gives. The SQL-layer block (_exact_for_encrypted) still catches it as a second line of defense, but the typed-API now fails at the call site for clearer stack traces. 2. Multi-hop coverage: added tests exercising the RelatedFieldRef → RelatedFieldRef → PrefixedFieldRef recursion via Grandchild.mid_parent.grandparent.name, both as Q construction and end-to-end query. 3. Bool slip-through: documented the Python `bool <: int` quirk in the _ForeignKeyDescriptor stub comment so future maintainers know the runtime check in ForwardForeignKeyDescriptor.__set__ is the only guard. (No clean way to exclude bool from `int` in Python's type system.) 4. Descriptor attribute shadowing: documented in the RelatedFieldRef docstring with a pinned test asserting the current behavior (where a field named `field` on the related model would be shadowed by the descriptor's own `.field`). The architectural fix — returning a fresh proxy from __get__(instance=None) — is bigger and deferred.

Field.__set__ was already a data descriptor at runtime, but its parameter was typed `value: Any` — so ty silently allowed wrong-type assignment like `row.name = 123` on a TextField. Narrow to `value: T` so the type checker enforces the declared field type at the call site. Plain's runtime is more permissive (`to_python` converts strings → ints etc.), but encouraging explicit conversion at the boundary catches a real bug class and makes the new typed-field declarations actually pull their weight. Tests added: a TYPE_CHECKING-only block with deliberately wrong assignments and `# ty: ignore[invalid-assignment]` markers. If the type-check ever loosens, ty flags the markers as unused suppressions — making regressions visible.

Master's "Type fields as parameterized descriptors" overlaps heavily with typed-where's foundation. Resolution keeps typed-where's typed-query surface (.equals/.contains/...; where(); FK traversal via _ForeignKeyDescriptor.__get__ -> type[T]) and adopts master's annotation conventions where they're additive: - JSONField / EncryptedJSONField: LHS-annotate explicitly (stubs return Any). - String-arg ForeignKeyField: take master's split overloads — bare T with required LHS annotation — and apply across affected models. - TextField.contains/icontains/startswith/endswith: kept (typed-where needs them; master had removed them). Reordered tests/app/examples/models/relationships.py so WidgetTag.widget uses a class-arg FK (Widget defined first, M2M through="WidgetTag") preserving type-level FK traversal in test_typed_where_fk after master's LHS-annotation convention.

Class-level access to a forward FK (Child.parent) now returns a fresh RelatedFieldRef traversal proxy from __get__ instead of the descriptor itself. Attribute lookup on the proxy reads the related model with inspect.getattr_static, so a related field whose name collides with a public descriptor attribute (field, is_cached, get_queryset, get_prefetch_queryset) resolves to the field rather than silently returning the descriptor attribute and building wrong SQL. Prefetch machinery, the only code that needs the descriptor via class-level access, now reaches it with inspect.getattr_static in get_prefetcher to bypass the proxy.

is_in builds a Q(field__in=[...]) membership condition on every field, typed as an iterable of the field's value type so a wrong element type is rejected at the call site. Negation composes with ~. FK traversal builds the prefixed path, and encrypted fields block it with a Never-typed parameter plus a runtime TypeError, matching the other comparisons.

Add a Querying subsection covering where(), the condition methods on every field and on text fields, negation and combination, foreign-key traversal, and the encrypted-field restriction to is_null().

…lution PrefixedFieldRef now delegates each condition to the wrapped field's own method and rewrites the resulting Q's leaf keys onto the relation path, instead of hand-mirroring every condition method. This makes the traversed surface exactly the field's own surface: a method the field doesn't define (contains on a non-text field) raises AttributeError through traversal, and encrypted-field blocking is enforced automatically by the field's own method bodies — no separate reject hook. RelatedFieldRef resolves names through the related model's metadata (get_forward_field) rather than attribute lookup, which is shadowing-immune by construction. Both refs now require a resolved model class; the dead string-model branch is gone. Field._build_q builds its Q via the positional-tuple constructor rather than poking children directly.

…pted text surface Master's ruff 0.16 defaults and ty 0.0.80 landed after this branch: convert the shadowing migration to tuples, bind the AttributeError probe, and suppress the Liskov diagnostics the Never-typed blocks exist to cause. Master reparented EncryptedTextField onto TextField, so it now inherits contains/icontains/startswith/endswith - block those the same way, and keep the non-None exact rejection inside master's get_lookups() registry. Master's migrations-reset tests pinned the examples leaf by name; derive the leaf, the next number, and the leaf's models from the real history so adding a migration to examples doesn't break them.

The pending-changes assertion read "Create model " with no name, which passes no matter what the refusal lists. Read the leaf migration's CreateModel operations instead, so deleting it has a named consequence and no branch has to re-pin the model by hand.

The ty: ignore[invalid-method-override] on EncryptedTextField and EncryptedJSONField is class-wide, so it would also hide a block that stopped blocking. Each blocked method now has a direct call site carrying ty: ignore[invalid-argument-type] - ty reports an unused suppression as an error, so widening any parameter away from Never fails type-check - and the same calls assert the runtime TypeError, pinning both sides together. is_null() carries no marker, so a Never creeping onto it breaks the build. Note on each class-level ignore that it exists for the deliberate Never narrowing and that any other override on the class needs a hand check.

#83's annotated model style and typed-construction stubs win for model declarations; typed where()'s condition methods, FK traversal proxies, and encrypted-field blocks win for the query surface. - example/app/tasks/models.py, tests/app/examples/models/relationships.py: take master's `Field[T]` annotations and drop the `query = QuerySet()` redeclarations. - relationships.py keeps this branch's declaration order (WidgetTag after Widget, class-argument FK) — only a class-argument FK gives the stub a resolvable `T`, which is what makes `WidgetTag.widget.name.equals(...)` type-check as well as run. - shadowing.py's ShadowTarget/ShadowSource fixtures converted to the annotated style. - types.pyi keeps both: `_ForeignKeyDescriptor[T, V](_Field[V])` from master for `Field[T]`-annotated construction, and this branch's `__get__(None) -> type[T]` for class-level traversal.

#83 annotates every model field with the *base* `Field[T]`. A declared type is what the checker reads, so that annotation erases the concrete descriptor the stub returns — and with it everything typed where() had hung off the concrete class: TextField's pattern conditions, the FK descriptor's `type[T]` class access, and the encrypted fields' `Never` blocks. Put each back where the annotation can still see it. - `Field.__get__` gains two model-valued class-access overloads, so a field whose value type is a model (an FK, nullable or not) yields `type[T]` and the related model's field surface stays reachable for traversal. This is what makes `Model(fk=obj)` and `Model.fk.name.equals(...)` both type-check through the same `Field[Related]` annotation. `_ForeignKeyDescriptor` no longer overrides `__get__` at all — the base covers it, and an override there was both invisible behind the annotation and a Liskov error. - `Field` declares `contains`/`icontains`/`startswith`/`endswith` under TYPE_CHECKING with a `self: Field[str] | Field[str | None]` annotation. The implementations stay on TextField, so the runtime surface (and the traversal surface that mirrors it) is unchanged, while `Field[int].startswith(...)` is still a type error. - `EncryptedFieldMixin` becomes `EncryptedField[T]`, a real `Field[T]` subclass exported from `plain.postgres`, and encrypted model fields are annotated with it. It's the only way to keep the blocks visible: they can't live on `Field[T]` without blocking every field. The pattern-condition blocks move up from EncryptedTextField, so EncryptedJSONField carries them too. `assert_type(Model.name, ...)` now asserts `Field[str]` rather than `TextField[str]` — under #83 that is the declared type — and a new type-check-only case pins the string-only restriction.

`Field` declares contains/icontains/startswith/endswith under TYPE_CHECKING, restricted by their `self` annotation to `Field[str]` / `Field[str | None]`, so they survive the `Field[T]` annotation models write. But the implementations lived on `TextField`, and two string-valued fields aren't TextFields: `GenericIPAddressField` (a DefaultableField) and `RandomStringField` (a ColumnField). So `Model.ip.contains("10.")` type-checked and raised AttributeError. Extract the four methods into `StringConditionsMixin` and mix it into all three — TextField, GenericIPAddressField, RandomStringField — so the runtime surface matches the declaration exactly. Both fields already register the underlying lookups, so the conditions execute; the end-to-end tests filter an `inet` column and a generated token. Non-string fields still get neither the declaration nor the methods, which is what keeps `test_traversed_surface_matches_direct_field_surface` and the `_CONDITION_METHODS` traversal gate honest. `EncryptedField` still precedes the mixin in EncryptedTextField's MRO, so the `Never` blocks still win. New `StringConditionsExample` fixture (migration 0020, a CreateModel so the reset test's leaf anchor stays non-empty) carries a GenericIPAddressField and a RandomStringField, and the public tests cover the Q shape, the executed query, the static types, and the absence on non-string fields.

pullapprove5 · 2026-09-19T02:45:10Z

PASS: 1 review scope passed

	Scope	Progress
✅	code	1/1

View in PullApprove

Next steps:

@davegaeddert: Merge or close

davegaeddert · 2026-09-19T02:45:12Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fc66c73082

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…r pickling Three P2 findings, each reproduced first. **Conditions on the relation itself** (related_typed.py). `Child.parent.equals(obj)` and `Child.parent.is_null()` raised `AttributeError: parent.equals is not a traversable field or relation` — RelatedFieldRef treated the condition name as a field on the related model. The methods deliberately aren't added: to the type checker `Child.parent` is `type[Parent]` (Field.__get__'s model-valued overloads), which is what makes `Child.parent.name.equals(...)` type-check, and a runtime method the checker rejects is worse than none. Instead the proxy now raises a TypeError naming the spelling that works — `Child.parent.id.equals(...)`, which compiles to the same `parent__id=` lookup `filter(parent=obj)` produces. The field lookup still runs first, so a related model with a real column named `equals` keeps traversing to it. RelatedFieldRef carries the FK's target field name so the message names the actual key. **README typed-query example** annotated `email: str` / `age: int` on `types.*` assignments and redeclared `query`, all pre-#83 — 3 ty errors as written (`str has no attribute equals`, `int has no attribute gte`, invalid `query` override). Now `Field[str]` / `Field[int | None]` with the `Field` import, and every snippet in the section verified through a scratch probe. **Descriptor pickling** (related_descriptors.py). `__reduce__` reconstructed with `getattr`, which runs `__get__` and returns a RelatedFieldRef — a pickled ForwardForeignKeyDescriptor came back as a traversal proxy with no `.field`. Reconstruct with `inspect.getattr_static`, the same way the prefetch path already reaches for the descriptor. Tests: relation-key conditions (Q shape, end-to-end equals / is_in / is_null, and an assertion that the typed spelling returns exactly what `filter(parent=)` returns), the helpful error across every condition name, the surviving AttributeError for a genuine typo, condition-named related columns still traversing, and pickle round-trips for a required and a nullable relation. Documented the spelling in the README and the postgres rule.

davegaeddert · 2026-09-19T03:07:17Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f37b02c3ce

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Four reviewers converged on the same shape: conditions belong on `Field`, and everything else should route through them rather than re-implement them. **Traversal hands back the field itself.** `PrefixedFieldRef` wrapped a field, delegated each condition by name through a hand-typed `_CONDITION_METHODS` gate, then rewrote the resulting Q's keys with `_prefix_q` -- three copies of one idea, and `_prefix_q` mutated `Q.children` in place, which forbids ever caching a Q. All of it is replaced by `Field.with_lookup_prefix(prefix)`: a detached copy (via the existing `Field.__copy__`) whose `name` carries the relation path, so the field's own condition methods build the right keys. The traversed surface is now identical to direct access by construction instead of by a gate that had to be kept in sync -- including an encrypted field's refusals, whose message names the full `store__api_key` path for free. **String conditions move onto Field**, keeping the `self: Field[str] | Field[str | None]` restriction the TYPE_CHECKING declarations already carried, so every string-valued field has them regardless of base class and `StringConditionsMixin` disappears. Note the runtime guard that was proposed alongside this can't exist: `Contains`/`IContains`/`StartsWith`/`EndsWith` are registered on `Field` itself, so nothing at runtime distinguishes a string field from an int one. The `self` restriction is the guard, and the test says so. `_build_q` does now reject a lookup a field hasn't registered at all. **Encrypted fields get one runtime guard instead of eleven.** The eleven `Never`-typed signatures stay as bodiless declarations under `if TYPE_CHECKING:` -- they are the static block -- and `EncryptedField._build_q` implements it once, refusing anything but `isnull`. That closes the loop for conditions that don't exist yet. `_build_q` takes the method name so messages still name `.gt()`. `_EncryptedExact` reads `lhs.output_field.name` instead of a `getattr` chain with a `"<encrypted>"` sentinel; it keeps its own sentence because `filter(field=None)` *is* supported and "does not support .filter()" would be wrong. **Ordering conditions reject a None operand** at the call site. On a nullable field `T` includes None, so `age.gte(None)` got past the checker and died much later as `ValueError("Cannot use None as a query value")`. This can't be fixed statically -- probed against ty 0.0.80 and pyright 1.1.414, `self: Field[X | None], value: X` selects the intended overload but solves `X` as `int | None`, and None can't be subtracted from a TypeVar -- so it raises here instead, with the field and method in the message. `equals(None)` and `is_null()` stay legal. Also: the condition-method names live once as `CONDITION_METHODS` / `STRING_CONDITION_METHODS` next to the methods (there were four copies); the two model-valued `__get__` overloads collapse into one `self: Field[M] | Field[M | None]` (verified on both checkers); `LOOKUP_SEP` replaces literal `"__"`; the `assert self.name is not None` guards that could never fire become truthiness asserts; and `RelatedFieldRef`'s "relation, not a field" error is an AttributeError so `hasattr` and `getattr(..., default)` keep working.

Returning a `RelatedFieldRef` proxy from `__get__` only ever existed to dodge name shadowing: a related model with a column called `field` or `get_queryset` would hit the descriptor's own attribute instead of traversing. The proxy paid for that by breaking the descriptor convention everywhere else -- prefetching and `__reduce__` both had to reach past `__get__` with `inspect.getattr_static`, and every class-level read allocated a proxy and re-executed a function-local import (257 ns, vs 42 ns for returning `self`). Close the hole at the source instead. The descriptor's attributes are now `_`-prefixed (`_field`, `_is_cached`, `_get_queryset`, `_get_prefetch_queryset`), and `Meta` skips `_`-prefixed attributes when it collects fields (meta.py), so a field name can never collide with a descriptor attribute -- shadowing is closed rather than routed around. Traversal moves onto the descriptor as `__getattr__`, which only fires for names the descriptor doesn't define: a leaf field comes back from `with_lookup_prefix`, a further foreign key hands off to `RelatedFieldRef`, which is now needed only for hops beyond the first. `_get_prefetch_queryset` is the duck-typed prefetch protocol, so the two manager classes and the one call site in query.py are renamed with it. Nothing outside plain-postgres references any of these; the managers keep their public `get_queryset`. `RelatedObjectDoesNotExist` stays public on purpose -- it is user-facing and documented. Both `inspect.getattr_static` patches revert, and the pickling test goes with them: the round-trip is covered by the convention it no longer breaks. The shadowing cases stay as a pin on the rename -- re-publishing any of those four names would fail them. Also in this commit: `QuerySet.where()` calls `self.filter(*conditions)` instead of reaching into `_filter_or_exclude`.

Eight verified findings, each reproduced first. **Every relation is a traversal hop, not just foreign keys.** `WidgetTag.widget.tags` fell through to `with_lookup_prefix` and came back as an M2M field copy renamed `"widget__tags"`, so `.tags.name` resolved to that *string* (`'str' object has no attribute 'equals'`) and `.tags.equals(t.id)` silently built `Q(widget__tags=1)`. Any `RelatedField` is now a hop -- `widget__tags__name` is as valid a lookup path as `widget__author__name` -- so traversal continues through it and the relation itself gets the same "use .id.<method>()" advice. Static typing of that hop is runtime-only for now and the tests say why: `tags` is declared `ManyToManyManager[Tag]`, and typing class access as `type[Tag]` would be false for `Widget.tags` itself. **One traversal rule instead of two.** The descriptor's `__getattr__` duplicated `RelatedFieldRef.__getattr__` and re-ran four function-local imports per attribute read. It now builds a `RelatedFieldRef` once per descriptor (`cached_property`, like `RelatedObjectDoesNotExist`, because the remote model may still be a string at construction) and delegates. The M2M fix above therefore lives in one place. **A traversal that runs before the registry resolves says so.** The descriptor dereferenced `remote_field.model._model_meta` with no guard, so a module-level traversal raised `'str' has no attribute '_model_meta'` -- which `hasattr` then swallowed, turning a timing problem into a missing attribute. Delegation routes it through `RelatedFieldRef.__init__`'s assertion, whose message now names the unresolved string and says to move the call inside a function. **`is_in` refuses a bare str/bytes.** `Iterable[str]` is satisfied by `str`, so `name.is_in("abc")` type-checked and iterated characters. Not expressible statically (`str` is a `Sequence[str]`), so `_build_q` refuses it next to the None guard and names the list spelling. **Encrypted equality raises through `get_or_create()` too, deliberately.** `get_or_create(api_key="k")` previously "worked" by creating a new row every call, because the lookup could never match existing ciphertext. It now raises -- that silent duplication is exactly what the block exists to prevent -- and the message drops the blanket "use .is_null()" for advice that fits: equality on an encrypted column can never match, `is_null()` is the only condition, and `get_or_create`/`update_or_create` callers should move the value into `defaults=`. Covered for the kwarg, `get_or_create` and `F()` paths, and documented in the README as a break. **Docs matched to the code.** The relation-condition error is an `AttributeError`, not a `TypeError`, in the README and the package-level rule (regenerated with `plain agent install`; mirrors byte-identical). Stale comments fixed: the encrypted JSON case describes `EncryptedField[dict | None]` rather than a mixin and gains the static pins the other eleven have (spelled directly, since a marker can't bind through `getattr`), and the string-condition block no longer claims a non-text field lacks the method at runtime -- it doesn't, which is the whole reason the guard is type-first. **Public tests test the public surface.** The `CONDITION_METHODS` sweeps, `with_lookup_prefix` and `_model_meta` cases move to `tests/internal/test_typed_where_internals.py` per tests-layout; the public files keep the documented spellings.

…ed refs Ten findings; each reproduced before fixing. **The empty string is matchable, and the two query paths now agree.** `_encrypt("")` returns `""` -- the empty string is stored as plaintext, which is exactly what makes `default=""` expressible as a column DEFAULT and what the README's own `required=False, default=""` idiom relies on. Blocking it regressed `exclude(token="")` with a message that was false for that value. Both paths now ask the field which values it stores deterministically: `matches_deterministically` returns None for every encrypted field, and text fields add `""`. That closed a second gap -- `field.equals(None)` raised while `filter(field=None)` worked, and the error even advertised `=None`. The static side had to move with it, so `equals`/`not_equal` are narrowed (overloaded on the `self` type, since models annotate `EncryptedField[T]` and that annotation is all the checker sees) instead of blocked outright. The other nine stay `Never`. **Traversal edges.** An unresolved string FK target made `ForeignObjectRel.model` raise TypeError straight out of `__getattr__`, so `hasattr` *raised* instead of returning False; it now raises `UnresolvedRelationError` (an AttributeError subclass) carrying the "move this inside a function" message. The ref is cached by hand rather than by `cached_property`, because an AttributeError raised inside a descriptor's `__get__` sends Python back to `__getattr__` and the message was being replaced by a bare "no attribute '_typed_ref'". The "use the key" advice spelled the path with lookup separators past the first hop (`widget__tags.id`); it is built from the attribute spelling now (`widget.tags.id`). Reverse relations reported "not a traversable field or relation", which is false -- `filter(author__posts__title=...)` works -- so they get their own message naming the `filter()` path and why the typed API can't express them. **Traversed fields are properly detached.** The copy kept `model`, `column` and `cached_col`, so `str()` claimed `examples.DeleteParent.parent__name` -- a column that doesn't exist. It now drops the attachment state and says what it is: `parent__name (lookup reference)`. Pickling round-trips as a plain field, which the existing `__reduce__` already handled once `model` was gone. **`Field[Any]` silently loses the typed surface** -- `Any` satisfies the model-valued `__get__` overload, so class access types as `type[Any]`. That is a checker limitation, not something to outsmart; plain-cache's `value` is annotated `Field[object]` instead (the column holds arbitrary JSON, so `dict[str, Any]` would be false), verified by probe, and the rule now says not to reach for `Field[Any]`. **Docs.** The rule and README claimed `Widget.tags.name.equals(...)` works from a class-level M2M; it doesn't and couldn't be typed. They now say traversal starts from a forward FK, an M2M is traversable only as a later hop, and to use `filter(tags__name=...)` otherwise. `RelatedObjectDoesNotExist` is documented with its typed spelling (`except Related.DoesNotExist`), since class access now types as the related model. And "Text fields add contains…" became every string-valued field, which is what shipped.

davegaeddert added 12 commits May 23, 2026 13:10

Merge remote-tracking branch 'origin/master' into typed-where

9cf1090

Document typed where() conditions

3da7f53

Add a Querying subsection covering where(), the condition methods on every field and on text fields, negation and combination, foreign-key traversal, and the encrypted-field restriction to is_null().

davegaeddert marked this pull request as ready for review July 23, 2026 16:15

davegaeddert changed the title ~~Add typed where() with field-method conditions~~ [merge 2] Add typed where() with field-method conditions Jul 23, 2026

davegaeddert changed the title ~~[merge 2] Add typed where() with field-method conditions~~ [1b] Add typed where() with field-method conditions Jul 23, 2026

davegaeddert marked this pull request as draft August 31, 2026 02:42

davegaeddert added 10 commits September 18, 2026 14:32

Merge origin/master into typed-where

a289af1

Merge origin/master into typed-where

4d3cb34

Merge remote-tracking branch 'origin/master' into typed-where

d6ec8de

Converge the reset test on the master-bound version

6e0bbd1

davegaeddert mentioned this pull request Sep 19, 2026

Add value_type= to core columns; HashedPassword replaces PasswordField #131

Draft

davegaeddert marked this pull request as ready for review September 19, 2026 02:45

chatgpt-codex-connector Bot reviewed Sep 19, 2026

View reviewed changes

Comment thread plain-postgres/plain/postgres/fields/related_typed.py

Comment thread plain-postgres/plain/postgres/README.md Outdated

Comment thread plain-postgres/plain/postgres/fields/related_descriptors.py Outdated

chatgpt-codex-connector Bot reviewed Sep 19, 2026

View reviewed changes

Comment thread plain-postgres/plain/postgres/fields/base.py Outdated

davegaeddert added 4 commits September 18, 2026 23:23

davegaeddert merged commit f37ab10 into master Sep 19, 2026
9 checks passed

davegaeddert deleted the typed-where branch September 19, 2026 17:39

This was referenced Sep 19, 2026

Add a ty typing corpus and stub-vs-runtime conformance for plain-postgres #132

Merged

Add typed select() for column selection #89

Merged

Navigation Menu

[1b] Add typed where() with field-method conditions - #84

[1b] Add typed where() with field-method conditions#84
davegaeddert merged 27 commits into
masterfrom
typed-where

davegaeddert commented Jul 23, 2026 •

edited

Loading

Uh oh!

pullapprove5 Bot commented Sep 19, 2026

Uh oh!

davegaeddert commented Sep 19, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

davegaeddert commented Sep 19, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

davegaeddert commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Conditions live on Field

Foreign-key traversal

Conditions on the relation itself

Class-level FK access returns the descriptor

Encrypted fields: one runtime guard, eleven static declarations

EncryptedField[T] is the annotation

Behavior change: get_or_create() on an encrypted field now raises

None operands on ordering conditions

Also in this branch

Tests / checks

Uh oh!

pullapprove5 Bot commented Sep 19, 2026

Uh oh!

davegaeddert commented Sep 19, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

davegaeddert commented Sep 19, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

davegaeddert commented Jul 23, 2026 •

edited

Loading

Conditions live on `Field`

`EncryptedField[T]` is the annotation

Behavior change: `get_or_create()` on an encrypted field now raises