Skip to content

Docstrings: Model, Query and relations #719

Closed

Description

@0x054

Parent

#698

What to build

The core API's docstrings render 57 Sphinx roles as literal text, drop the FerroField/ForeignKey option tables (a blank line under the section header), leave session and using out of Args: in 25 callables, carry examples that crash (ForeignKey with Annotated[int, ...]) or use test-suite models, and cite issue numbers, ADRs and codenames. The pagination methods after, before and position_of have no examples.

Every public docstring in these modules follows I-14, and the main entry points (create, get, where, save, delete, after, before, position_of, select, include) carry an example that runs against the reference prelude. Essays (Model, save, select, include, RelationProxy) shrink to reference entries and link to the guide.

Acceptance criteria

  • Every public function, method, class, property and dataclass/Pydantic field in the owned modules has a docstring in Google style as griffe parses it (no griffe warning at default level with docstring_parser="google").
  • Every summary is one line, in the infinitive ("Fetch one record by primary key."), ending with a period; class summaries are noun phrases ending with a period (docs conventions from AGENTS.md, written by the AGENTS.md ticket (AGENTS.md: rewrite I-8 for the lambda-only query API, add a docstring invariant, record the docs conventions #703)).
  • Every parameter appears in Args: with its default stated inline; ruff check --select D417 reports nothing for the owned files.
  • No Sphinx roles (:class:, :meth:, :func:, :attr:, :data:) and no RST :: blocks remain; cross-references are Markdown autorefs and resolve in the strict build.
  • Returns:, Raises:, Attributes: and Args: sections render as tables with one row per item (continuation lines indented, no blank line under a section header).
  • No ADR numbers, issue numbers, codenames, Rust internals or test paths in docstrings; explanations longer than a paragraph move to a guide or concepts page and are linked.
  • Examples sections use the lambda style named after the model, run against the shared reference prelude inside a session, and are marked to run ({test=run prelude=reference}).

Files owned

  • Modify: src/ferro/models.py, src/ferro/base.py, src/ferro/metaclass.py, src/ferro/query/__init__.py, src/ferro/query/builder.py, src/ferro/query/nodes.py, src/ferro/query/rows.py, src/ferro/query/wire.py, src/ferro/relations/__init__.py, src/ferro/relations/descriptors.py

Interfaces

Test scenarios

  • Happy path: tests/test_docstring_examples.py runs every marked example in the owned modules and they pass.
  • Happy path: the strict build succeeds and the rendered API pages for the owned objects contain no literal :class:-style roles.
  • Edge case: ruff check --select D417 over the owned modules exits 0.
  • Error path: removing one parameter from an owned docstring's Args: makes ruff check --select D417 fail.

Verify

uv run --no-sync zensical build --clean --strict
uv run --no-sync pytest tests/test_docstring_examples.py -q --no-cov -p no:cacheprovider
uv run --no-sync python -m tests._docs_checks src/ferro/models.py src/ferro/base.py src/ferro/metaclass.py src/ferro/query/__init__.py src/ferro/query/builder.py src/ferro/query/nodes.py src/ferro/query/rows.py src/ferro/query/wire.py src/ferro/relations/__init__.py src/ferro/relations/descriptors.py
uv run --no-sync ruff check --select D417 src/ferro/models.py src/ferro/base.py src/ferro/metaclass.py src/ferro/query/__init__.py src/ferro/query/builder.py src/ferro/query/nodes.py src/ferro/query/rows.py src/ferro/query/wire.py src/ferro/relations/__init__.py src/ferro/relations/descriptors.py
uv run --no-sync pytest tests/test_docs_site.py -q --no-cov -p no:cacheprovider

Blocked by

Audit findings

Rows from the docs audit this ticket covers (file, line, rule, fix)

Summary

Module Public objects Missing docstring griffe warnings (docstring) Summary form Grade
src/ferro/models.py 18 (Model + 14 methods, model_config, evict_instance, transaction) 1 (Model.model_config) 4 (**fields has no type: create, upsert, get_or_create, update_or_create) 17 infinitive C
src/ferro/base.py 26 (FerroField, ForeignKey + their __init__ and 18 attributes, FerroNullable, varchar, DbType, DbTypeToken) 18 (every attribute) 4 (Attributes: ×2 and Args: ×2 skipped: "Extraneous blank line below section title") 5 infinitive, 3 noun phrase C
src/ferro/query/builder.py 40 (Query + 22 members, ProjectedQuery + 11, Relation + 2) 5 (Query.model_cls/where_clause/order_by_clause, Relation.all, Relation.first) 0 (3 unrelated "Base class typing.Generic" resolution notices) 30 infinitive, 3 noun phrase C
src/ferro/query/rows.py 3 (Row, Row.model_config, Rows) 1 0 2 noun phrase B
src/ferro/query/nodes.py (documented: QueryProxy, Predicate, RowSelector, AggregateExpr + 3 attrs, now) 8 3 (AggregateExpr.fn/column/path) 0 6 noun phrase C
src/ferro/query/nodes.py (secondary: FieldProxy, QueryNode, RelationProxy, ValueExpr + members) 27 14 0 (1 Generic notice) 5 infinitive, 8 noun phrase C
src/ferro/__init__.py 11 (connect, PoolConfig + 3 fields, model_config, validate, create_tables, migrate, clear_registry, ensure_resolved_modelset) 5 (PoolConfig.max_connections/min_connections/settings_delivery/model_config/validate) 0 5 infinitive, 1 noun phrase C
src/ferro/_core.pyi re-exports 3 (version, reset_engine, set_default_connection) 3 in the stub (rendered from the Rust doc comment by runtime inspection) n/a 3 third-person C
Total 136 50 (46 attributes/fields, 4 callables) 8 57 infinitive / 3 third-person / 23 noun phrase

Verb form (rule 21). The project has de facto picked the infinitive ("Fetch one record…", "Add a filter
condition…"): 57 of 60 verb-led summaries. The three third-person ones are the Rust doc comments surfacing
through _core ("Returns the current version…", "Shuts down…", "Selects…"). Class and alias summaries split
between infinitive ("Provide the base class…", "Store database column metadata…", "Represent lazy collection…",
"Build and execute fluent ORM queries.") and noun phrase ("One projected record…", "Type alias for…",
"Connection pool settings…"). Pick infinitive for callables and a noun phrase for classes/aliases, and say so
in the contributor guide.

Summary line mechanics (rule 21). 29 of 85 summaries lack the final period (e.g. Model.delete,
Model.all, Model.create, Query.all/count/first/delete/exists/limit/offset, FerroField, ForeignKey,
Relation). Four summaries wrap onto a second line, so the summary griffe shows is half a sentence:
create_tables ("Manually create the missing tables for registered models on a connected"), now ("The
database clock: a recipe update() that assigns now (the"), ProjectedQuery.select, reset_engine.

src/ferro/models.py

Sev Line Rule Finding Suggested fix
M 209 rule 21 / rule 1 Model opens with "Provide the base class for all Ferro models" (an imperative for a class, no period). The body is four bold-label essays (composite uniques, composite indexes, table checks, row security) with implementation notes ("provisionally registers schema metadata in Python caches", "bulk install (#246)"). The only example defines a class and shows no result. "Base class for Ferro models." Then one paragraph, an example that saves and fetches a row inside a session, and links to the guide pages for the __ferro_*__ hooks instead of restating them.
M 366 save rule 1 / 7 A 30-line prose block mixes reference with explanation (mixin forwarding, model_copy() semantics, rolled-back inserts). Examples: row, msgs and utcnow are undefined, and four unrelated calls are stacked in one block with no sentence between them. "Note that…" is inline prose. Keep the contract (insert vs update, only=/exclude=). Move the edge cases to a Note: admonition. Split the examples into "insert" and "partial update" with a defined Chat model.
M 517, 603 delete, refresh griffe Google Returns: None on a function that returns None. using and session are undocumented, and there is no Args:. Drop Returns:. Add Args:.
M 544, 561, 585, 641, 688, 723, 749, 772, 805, 838 griffe Google session is undocumented on all, get, get_or_none, where, select, create, upsert, bulk_create, get_or_create and update_or_create. using is undocumented on all and bulk_create. select has no Args: for *selectors. Add the parameters.
M 723/749/805/838 griffe warning **fields has no annotation ("No type or annotation for parameter '**fields'"). Annotate **fields: Any, or give a type in the docstring: **fields (Any): ….
M 718 using rule 21 / 9 One-line docstring with no Args:, no return description and no example. It returns ModelConnection, which is neither exported nor documented, so the reader has nowhere to go. Document name, say what the returned object offers, and add an example: await User.using("replica").get(1). Add ModelConnection to the API page.
M 585, 838 Examples coverage get_or_none and update_or_create have no Examples (their siblings do). Add one each.
M 118 evict_instance rule 1 / I-11 Paragraph 3 is an implementation note: "Public wrapper around the FFI evict_instance (FF-D D3): resolves the route once via resolve_operation_scope…". There is no Args: (4 params), no example, and no word on when a user would call it. Replace it with when to call it (stale instance after a raw UPDATE), Args:, and an example. Move the FFI note to a comment.
M 143 transaction griffe Google The prose says "Yields a … Transaction handle", but there is no Yields: section and no Args: (using, session, immediate). The trailing paragraph uses an RST :: literal marker ("the handle is simply discarded::"), which renders as a stray colon. "useful for…" and "simply" (rule 17). claims_json is undefined. Add Args: and Yields:. Fold the second example into Examples:. Drop "simply".
L 517, 544, 603, 723, 772, 805, 838 rule 21 8 summaries have no final period. Add periods.

src/ferro/base.py

Sev Line Rule Finding Suggested fix
H 186 rule 9 The ForeignKey example crashes. author: Annotated[int, ForeignKey("posts", on_delete="CASCADE")] raises AttributeError: type object 'int' has no attribute '__ferro_identity__' at resolve_relationships, because the annotation must be the target model. author: Annotated[User, ForeignKey(related_name="posts", on_delete="CASCADE")]
H 17, 99, 176 stale fact FerroNullable ("Alembic column nullability"), FerroField.nullable ("Alembic Column.nullable when using get_metadata") and ForeignKey.nullable ("Alembic nullability") describe an Alembic-only option. It now feeds the schema IR, so every emitter honours it: n: Annotated[int | None, FerroField(nullable=False)] produced "n" integer NOT NULL under connect(auto_migrate=True). "Column nullability. 'infer' (default) follows the annotation; True/False force NULL/NOT NULL in every emitter."
H 80, 167, 131, 206 griffe Google Attributes: (both classes) and Args: (both __init__) have a blank line under the heading. griffe skips the section ("Extraneous blank line below section title"), so the site renders a literal "Attributes:" paragraph with raw :class: text (seen in site/api/fields/index.html). These are the most-read parameter lists in the library. Delete the blank line after each section title.
M 80, 167 griffe Google The lists are incomplete. FerroField Attributes: omits renamed_from. ForeignKey Attributes: omits renamed_from and relation_annotation. FerroField.__init__ Args: omits db_type and db_check. The 18 attribute objects have no docstrings of their own, and show_if_no_docstring = true lists them bare. Document parameters once, in __init__ Args:, and set merge_init_into_class = true (another auditor covers config). Alternatively, give the attributes doc comments.
M 78–115, 117–150 I-7 / rule 1 Docstring examples show Annotated[..., FerroField(...)] only. The guide's public spelling is Field(...) in both styles. FerroField and ForeignKey repeat the same parameter prose twice (class Attributes: and __init__ Args:) with different wording ("Mark the field as the table primary key" vs "Set to True when the field is the primary key"). One description per parameter. Show the Field() assignment and Annotated forms, or point to Field.
L 39 varchar griffe Google No Args: for length. "Prefer this over a raw string so IDEs…" is fine. Add Args:/Returns:.
L 78, 164 rule 21 "Store database column metadata for a model field", "Describe a forward foreign-key relationship" are imperatives for classes with no period. "Column options for a model field." / "Forward foreign-key relation to another model."

src/ferro/query/builder.py

Sev Line Rule Finding Suggested fix
H 1117, 1183, 1208 Examples coverage position_of, after and before (keyset pagination, the I-13 showcase) have no examples at all. Add one example: page 1 with order_by(lambda message: message.pinned_at).order_by(lambda message: message.id).limit(20), then .after(page[-1]).
M 556 Query rule 21 "Build and execute fluent ORM queries." The Attributes: section documents internals (where_clause: "Accumulated filter nodes", order_by_clause: "Sort definitions sent to the Rust core"), which then render a second time as bare members with no docstring. Drop the internal attributes from the public surface (rename with _), and describe what a Query is plus how to get one (Model.where/select).
M 699 select, 958 include, 1581 ProjectedQuery.order_by rule 1 / 15 Reference entries run 20–30 lines of dense prose with em-dash chains and ADR citations ("the complete-instance invariant, ADR-0007", "Include decides attached data only — joins decide membership, projection decides shape"). The explanation belongs on the queries/concepts pages. Keep the signature contract and one example per form (single field, tuple, dict alias). Link to the guide for traversal and aggregate rules.
M 895, 927 rule 9 QJTransaction and QJNote are test-fixture class names leaking into the public API page. Use the running Payment/Account example.
M 1034, 1052 rule 9 The limit and offset examples assert on private state (query._limit, query._offset). Show the result: await User.select().order_by(lambda user: user.id).limit(10).all().
M 1717, 1738 ProjectedQuery.count/exists griffe Google / rule 21 The return annotation is missing (they return a coroutine), so the rendered signature shows no type and there is no Returns:. Summaries are noun phrases ("Existence check — or raise…"). Annotate -> Coroutine[Any, Any, int] (or make them async), add Returns:, and start with "Count…" / "Return whether…".
M 1757, 1837, 1852, 1883 griffe Google ProjectedQuery.position_of, select, include and update have no Args: for row, selectors, selector, recipe/fields. Add Args: (one line each for the rejecting methods).
M 1923 Relation rule 21 / griffe "Represent lazy collection relationship queries with typing support" has no period and is vague. Relation.all and Relation.first have no docstrings. The Example splits into three >>> groups separated by blank lines, and the user.posts.all() call needs a session (see cross-module H). "Type of a reverse (BackRef) or many-to-many collection; awaiting .all() loads it." Document all/first.
L 1405, 1440, 1473 add/remove/clear griffe Google The "provide an id attribute" wording is shaky: any primary key name works, which the docstring should confirm. There is no Returns: (fine, None). The summaries have no period. "Target instances (their primary key is linked)."
L 802 order_by I-8 The text says the lambda form is "the documented style", but the example uses lambda p: p.created_at and the prose lambda u / lambda t. lambda post: post.created_at.
L 1238–1392 rule 21 10 summaries have no final period (limit, offset, all, count, first, delete, exists, add, remove, clear). Add periods.

src/ferro/query/rows.py

Sev Line Rule Finding Suggested fix
L 23 Row rule 1 / I-11 Mentions "(I-2)", "Rust hydration path", "pydantic's extra storage": implementation detail in reference. Otherwise this is the best entry in scope: it says what a Row is not, shows output, and the example is plausible. Drop the I-2 sentence. Keep the rest.
L 47 Rows rule 9 The example uses rows without defining it (it relies on the Row example above). Start with rows = await …select(...).all().

src/ferro/query/nodes.py

Sev Line Rule Finding Suggested fix
M 863 QueryProxy rule 16 / I-11 "(FF-F F-2)" in the summary. A paragraph speculates about the future: "proposed for Python in PEP 827 (draft, targeting 3.16) — adopted here when type checkers support it". Drop the codename. Move the PEP note to a code comment or the query-typing concept page.
M 816 now rule 21 The summary spans two lines and reads as a sentence fragment in the rendered summary. There is no example, though Query.update has one. "Database clock for update() recipes: assigning now stamps the column with the server's current timestamp." Add a two-line example.
M 588 AggregateExpr rule 1 / I-11 "the data the select() resolver turns into a v5 expr record field", "(#294, ADR-0009)", "post-aggregation filtering is having(), #291". having() does not exist yet, and the reference reads as if it did. The fn/column/path attributes have no docstrings. Describe what the user writes ({"total": payment.amount.sum()}) and what comes back. State that having() is not available.
M 1421, 1424 Predicate, RowSelector rule 21 / 9 Type aliases with no example of the signature they denote. RowSelector mixes in issue numbers. Show the alias's expansion (Callable[[QueryProxy[M]], QueryNode]) and one lambda.
M secondary: FieldProxy, QueryNode, RelationProxy, ValueExpr rule 22 / 4 Public docstrings cross-reference these (:class:FieldProxy, :class:QueryNode) and users call their methods (.sum(), .in_(), .like(), .merge()), but no API page has a ::: directive for them, so the references have no target. Their examples construct internals directly (FieldProxy("email") == …) instead of using a lambda. ValueExpr is five issue numbers and ".concat() is not shipped yet". The aggregate methods count/sum/avg/min/max open with "SUM(column) —" (noun phrase). merge has no Args: for patch. Add a "Column expressions" entry for FieldProxy (its operators, in_, like, aggregates, merge) with lambda examples. Keep QueryNode/RelationProxy/ValueExpr out of the reference, or document them as opaque return types in one line each.

Cross-module findings that apply everywhere in scope

Sev Where Rule Finding Suggested fix
H 57 occurrences: query/builder.py 20, models.py 15, query/nodes.py 12, base.py 10 griffe Google / rule 22 Sphinx roles (:class:, :meth:, :func:, :data:) are not parsed by mkdocstrings-python. They render as literal text, confirmed in site/api/queries/index.html ("The lambda receives a fresh :class:QueryProxy…") and site/api/fields/index.html (":func:~ferro.migrations.get_metadata"). Readers see markup noise and no links. Replace with Markdown autorefs: [QueryProxy][ferro.query.QueryProxy], or plain QueryProxy where no API page exists. Sweep with `rg ':(class
H every Model/Query example (Model.create, get, all, save, delete, refresh, Query.*, Relation) rule 9 Examples call ORM methods with no route. Run as written after connect(), they raise RuntimeError: No database route for this operation. Open a session… (implicit default routing was removed in v0.13). They pass only inside async with engines.session():, which no docstring shows or mentions. State once in the Model class docstring that the examples run inside async with engines.session():, and show that wrapper in the Model and Relation examples.
H all modules rule 9 No docstring example is executed: pyproject.toml has no --doctest-modules and no test collects docstrings. Examples use names that are never defined: row, msgs, utcnow (Model.save), claims_json (transaction), Counter, cid (Query.update), QJTransaction, QJNote (test-suite model names in Query.join / left_join). Also Transaction, a user model that shares its name with the exported ferro.Transaction handle (select, include, Row, Rows, RelationProxy). One example crashes (see base.py). Pick one running example for the API reference (for instance the User/Post pair Relation already uses), define it once, and turn on pytest --doctest-modules with an async-aware fixture, or move examples into docs/examples/ with tests. Rename the Transaction example model (Payment, Txn).
M 22 docstrings: models.py 3, query/builder.py 10, query/nodes.py 7, __init__.py 2 I-11 / rule 1 Issue numbers, ADR ids and internal codenames sit in user-facing reference text: "(#295's three rules)", "(FF-F F-2)", "(FF-D D3)", "(#287, final at #293)", "ADR-0006", "v5 expr record field", "SchemaIR", "tests/static_fixtures/bad_includes.py". Readers can't follow them, and the reasons behind them are already in the ADRs. Remove them from docstrings. Where the reason matters, say it in one plain clause ("joins on the same path render once"). Keep ADR links in code comments.
M 25 callables (list per module below) griffe Google / rule 21 Parameters missing from Args:. griffe does not warn about these (it only warns about documented names that don't exist), so this count comes from the second script. The most common gap: session is undocumented on 12 Model methods, and using on 5. Document every parameter, with its default inline ("session: Session to run in. Defaults to the ambient session.").
M 14 docstrings I-8 Lambda parameters are not the model in lowercase singular: t for Transaction (Model.select, Query.select, include, join, left_join, ProjectedQuery.order_by/all/first, Row, RowSelector, RelationProxy), p for Post and u (Query.order_by), n (left_join). lambda transaction: transaction.amount, lambda post: post.created_at, lambda note: note.account.
M 12 docstrings rule 22 None, True and False appear in prose without code font: "or return None if no row exists" (get_or_none), "Return the first matching record, or None" (Query.first, ProjectedQuery.first), "True if records exist, otherwise False" (exists), "created is True for new records" (get_or_create, update_or_create), "If True" ×6 (connect, migrate), "when True" (ForeignKey.unique), "Returns: None" (delete, refresh). Write None, True, False.
L Model.where, Query.where/all/count/delete, QueryProxy rule 9 / I-8 Examples carry # noqa: E712 and == False / == True. The lint pragma renders in the docs. Use a positive column (lambda user: user.active) or ~ negation, and drop the pragma.

Cross-cutting patterns

  1. Sphinx roles in a Google/Markdown docstring set: 57 :class:/:meth:/:func:/:data: roles in scope, all rendered as literal text on the site, with no cross-reference links.
  2. Examples are never executed and many cannot run: 0 doctests collected. 1 crashes (ForeignKey), about 10 use undefined names or test-fixture models, and every ORM example needs an engines.session() the docstrings never mention.
  3. Undocumented parameters: 25 callables have a parameter missing from Args:, led by session (12) and using (5). griffe does not catch this, so it needs its own check (or ruff D417).
  4. Internal references in user-facing reference: 22 docstrings cite issue numbers, ADR ids, codenames (FF-F F-2, FF-D D3), Rust internals (SchemaIR, FFI, "v5 expr record") or test paths.
  5. Reference entries that are explanation essays: connect (95 lines), PoolConfig (~90), Model, save, select, include, RelationProxy. Rule 1 says link to explanation pages instead.
  6. Summary-line form drifts: 57 infinitive, 3 third-person (Rust), 23 noun phrase, 29 missing the final period, 4 summaries wrapped onto two lines.
  7. I-8 lambda naming: 14 docstrings use t/p/u/n instead of the model's lowercase singular name.
  8. griffe-detected structure breaks: 4 sections silently skipped for a blank line under the title (FerroField/ForeignKey Attributes: and __init__ Args:), and 4 **fields with no type. These are the only griffe warnings, but they hide the core field-option tables.

Activity

  1. added
    documentationImprovements or additions to documentation
    ready-for-agentUnblocked and fully specified; an agent may claim it
    size:LCross-cutting or multi-worker; local waves only
    on Oct 19, 2026
  2. added theissue type on Oct 19, 2026
  3. self-assigned this
    on Oct 19, 2026
  4. 0x054 commented on Oct 19, 2026

    @0x054
    ContributorAuthor

    Status: verify-passed
    Worker: worker-719 (reviewed by reviewer-719)
    Branch: sdd/719-docstrings-core
    PR: #735
    Verify: passed 3e21b32; trial merge into feat/docs-rewrite at 853aaac passed

    Review: Spec PASS, Quality PASS. The trial merge passed Verify and the full suite (3441 passed). One Minor finding: ProjectedQuery.exists says only an aggregate-only projection raises, but any aggregate projection does. The changes outside Files owned (prelude runner, the new test, the solutions note) are accepted. The silent ForeignKey-to-JSON under postponed annotations was reproduced; it needs its own issue.

  5. 0x054 commented on Oct 19, 2026

    @0x054
    ContributorAuthor

    Merged in #735 into feat/docs-rewrite

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

documentationImprovements or additions to documentationready-for-agentUnblocked and fully specified; an agent may claim itsize:LCross-cutting or multi-worker; local waves only

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions