Repository navigation
Aggregations guide: real data without ties, results shown, error checks that can fail #710
Copy link
Copy link
Closed
Closed
Copy link
Labels
documentationImprovements or additions to documentationImprovements or additions to documentationready-for-agentUnblocked and fully specified; an agent may claim itUnblocked and fully specified; an agent may claim itsize:SOne context window, <= 3 files; cloud-eligibleOne context window, <= 3 files; cloud-eligible
Description
Parent
What to build
The aggregations example uses placeholder data where both accounts total 30, which hides what grouped, top-N and descending examples do; a comment says one of four transactions has a price while the assert checks three; five error checks cannot fail; 24 lambdas use one-letter names. The page and its example pair get real-looking data with distinct totals, comments that match the asserts, error checks with else, and lambdas named after the model.
Pages: guide/aggregations.md
Acceptance criteria
- Every fact on the page matches the code at the time of the PR; each High finding in the audit rows below is fixed, and each Medium/Low finding is fixed or answered in the PR body with the reason it was left.
- Each page is one kind (tutorial, how-to, reference or explanation) and opens with one sentence saying what it covers, then an example on the first screen (writing-for-humans rules 1–2).
- Each page uses one running example, declared once near the top and reused; each section shows only what changed (rule 3).
- Every code block has a lead-in sentence ending in a colon and a result sentence after it, and shows its result: printed output, or the asserted value inside the embedded snippet range (rules 7–8). Snippet
end:markers sit below the assert or print that holds the result. - Every copyable block either shows the
async with engines.session():wrapper or sits under a sentence that says it runs inside one and links to Connections › Sessions; the page's first block shows the wrapper. - Every inline Python block that does real work runs under the harness (
{test=run prelude=<name>}) or moves into an example script; a block left compile-only says why in its fence marker. - Every lambda passed to a query method names its parameter after the model in lowercase singular (AGENTS.md I-8);
python -m tests._docs_checksreports no violation for the owned files. - Every error check in an owned example fails when the expected error is not raised (
try/except/else: raise AssertionError, or the harness helper). - Field-declaring examples show both declaration styles as tabs, each backed by a runnable script (AGENTS.md I-7), and the twins use the one Annotated idiom recorded in the docs conventions.
- Headings are short sentence-case noun phrases naming the feature (rule 5); how-to titles name the goal (rule 24); no minimising words (rule 17); admonitions follow rule 20.
- No ADR numbers, issue numbers, codenames, dated or release-bound notes, or contributor-only references in page text; version markers are self-contained blocks at the end of a section.
- Reference material that has a home in Reference is linked, not repeated; each topic ends with a pointer to its reference entry (rule 4).
Files owned
- Modify:
docs/pages/guide/aggregations.md,docs/examples/aggregations.py,docs/examples/aggregations_annotated.py
Interfaces
- Consumes: I-8, docs conventions (AGENTS.md ticket (AGENTS.md: rewrite I-8 for the lambda-only query API, add a docstring invariant, record the docs conventions #703))
- Consumes: harness (harness ticket (Docs harness: run inline blocks with a prelude, run docstring examples, and check lambda names and error checks #704))
- Produces: nothing other tickets rely on
Test scenarios
- Happy path: the strict site build succeeds and every snippet range on the owned pages renders without leading indentation.
- Happy path: every owned example script runs to completion under
tests/test_docs_examples.py, and every inline block marked{test=run ...}runs. - Error path: deleting the
raisean owned example's error check expects makes that example's test fail. - Edge case:
python -m tests._docs_checksover the owned files exits 0; renaming one lambda parameter totmakes it exit 1 naming the file and line.
Verify
uv run --no-sync zensical build --clean --strict
uv run --no-sync pytest tests/test_docs_examples.py -q --no-cov -p no:cacheprovider -n 8
uv run --no-sync python -m tests._docs_checks docs/pages/guide/aggregations.md docs/examples/aggregations.py docs/examples/aggregations_annotated.pyBlocked by
- Docs site config: Zensical features, Markdown extensions, API reference options and a strict build #701, AGENTS.md: rewrite I-8 for the lambda-only query API, add a docstring invariant, record the docs conventions #703, Docs harness: run inline blocks with a prelude, run docstring examples, and check lambda names and error checks #704
Audit findings
Rows from the docs audit this ticket covers (file, line, rule, fix)
docs/pages/guide/aggregations.md
- Kind (rule 1): guide, with a reference table (result types) and a little explanation (line 77, why there is no
group_by). It is well balanced. - Running example (rule 3): yes. One
Account/Transactionschema, introduced once at the top and reused in every section. Good. - Verdict: The tightest of the three: it opens with what the feature is, has one domain, and its snippets include their results. One snippet comment states a wrong fact, the seed data has ties that hide what three examples do, and the page never shows the generated SQL. Grade B.
| Sev | Line | Rule | Finding | Suggested fix |
|---|---|---|---|---|
| H | 66 (aggregations.py:100) |
facts | The snippet comment says "Only one of four transactions has a price..." but three of four do, and the assert two lines below says row.priced == 3. |
"Three of four transactions have a price; the orphan does not." |
| M | 74, 82, 122, 130 (aggregations.py:37-48) |
rule 10 | The seed gives a1 and b1 the same total (30). The grouped results [{acct:1,total:30},{acct:2,total:30}] cannot show the sum is per group. Top-N limit(1) picks between a tie, so the snippet can only assert len(top) == 1 and never shows which group won. The order_by(lambda t: t.amount.sum(), "desc") example sorts a tie, so the desc sort is invisible. |
Seed b1 with 35 (or give a1 a third row) so totals differ. Show the top-N result ([{"account_label": "a1", "total": 30}]) and a visibly descending order. |
| M | whole page | rule 13 | Grouping is derived from the record shape, but the page never shows the GROUP BY the engine writes. That SQL is the thing a reader knows. |
After the first grouped example, add SELECT account_id AS acct, SUM(amount) AS total FROM "transaction" WHERE account_id IS NOT NULL GROUP BY account_id ORDER BY acct. |
| M | 154-156 (aggregations.py:216-250) |
rule 9 | Each try/except in the errors snippet has no else, so a regression that stops raising passes silently. The reader also sees asserts instead of the messages. |
Add else: raise AssertionError(...), and show each error message as a comment or a text block. Line 139 already does this well for one case. |
| M | 117-135 | rule 1 / 5 | "pinned as three rules" uses internal test vocabulary ("pinned", also at 37 "pinned cross-backend contract"), against I-11. The three rules sit in bold run-in sentences rather than headings. | "order_by on a grouped query follows three rules:" as a numbered list, or three ### headings ("Sorting by an output name", "Sorting by an aggregate expression", "Allowed sort keys"). Say "guaranteed" instead of "pinned". |
| M | 1-162 | rule 4 / 22 | No link to api/queries.md ("Aggregates and grouped queries" section). |
Link it in the opening paragraph and in See Also. |
| L | 1, 19, 69, 85, 93, 101, 144 | rule 5 | Title Case and slogan headings: "Grouped Queries: Bare Fields Are the Keys", "\"Has no relation\" is a visible bucket", "Zero rows, zero groups", "Grouping is not partitioning", "The Loud Limits". | "Aggregations and grouped queries", "Grouped queries", "The None group for missing relations", "Grouped queries over zero rows", "Grouping and partitioning", "Unsupported aggregate forms". |
| L | snippets | I-8 | Every lambda is lambda t: (24 times). |
lambda transaction:. The string order_by("total") uses are correct here: strings are the only way to name an output field. |
| L | 77 | rule 15 | One 70-word sentence of rationale with a parenthetical aside. | Split it: "SQL already requires every selected bare column to be a group key. A separate group_by() could only repeat the projection or contradict it…" |
docs/examples/aggregations.py + aggregations_annotated.py
- Embedded by:
guide/aggregations.md(13 sections + annotated schema) - Running example: Account/Transaction ledger
- Verdict: C. Thorough coverage, with an I-8 violation throughout, a comment that contradicts its own assert, error checks that cannot fail, and placeholder data that hides the results.
| Sev | Line | Rule | Finding | Suggested fix |
|---|---|---|---|---|
| H | 54–248 (24 lambdas) | I-8 | Every lambda is lambda t: for Transaction. The page embeds all of them. |
lambda transaction: transaction.amount.sum(). Inside select(), transaction.account.label still fits on one line at 88 columns. |
| H | 99–100 | wrong fact | The comment says "Only one of four transactions has a price...", but three of four do (lines 40, 43, 46) and the assert two lines later is row.priced == 3. |
"Three of the four transactions have a price; the orphan has none." |
| H | 216–250, errors |
9 | Five try: … except TypeError as exc: assert "…" in str(exc) blocks. If the call stops raising, the except never runs and the example still passes, so the "loud limits" the page promises are untested. |
Add else: raise AssertionError("expected TypeError") as table_checks.py:38 does, or run each through pytest.raises in a test. Better for readers: show each error message as output (rule 8). |
| M | 37–38, 125, 154 | 10 | Account labels "a1"/"b1" read as placeholders, and both groups total 30. The grouped result [{"acct": 1, "total": 30}, {"acct": 2, "total": 30}] cannot show which rows went where. |
Labels "Checking"/"Savings" with distinct totals (for example 10+20=30 and 45), so every grouped answer is checkable by eye. |
| M | 157–169, top-n |
9 / 10 | With both accounts at 30, order_by("total", "desc").limit(1) picks between tied groups arbitrarily. The example only asserts len(top) == 1, so "top-N" is demonstrated with no top. |
Use distinct totals and assert the winning label. |
| M | 213–250 | 11 | errors stacks five unrelated failures in one block. |
One block per limit, or a table on the page with the message text. |
| L | 25 | I-7 | price: Decimal | None = Field(default=None): a Field() with no Ferro option, which forces an Annotated twin Annotated[Decimal | None, Field(default=None)] for nothing. |
price: Decimal | None = None in both. |
| L | 117, 178, 205 | mechanics | != None # noqa: E711 in the rendered code. |
Per-file ruff ignore (see quickstart). |
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
documentationImprovements or additions to documentationImprovements or additions to documentationready-for-agentUnblocked and fully specified; an agent may claim itUnblocked and fully specified; an agent may claim itsize:SOne context window, <= 3 files; cloud-eligibleOne context window, <= 3 files; cloud-eligible
Activity
0x054 commentedon Oct 19, 2026
Status: verify-passed
Worker: sdd worker
Branch:
sdd/710-aggregationsPR: #740
Verify: passed 3813312 (base a7cbca6 unchanged, no trial merge needed)
Re-review of the fix round: Spec PASS, Quality PASS. The two dead
#sessions-recommendedlinks now point at#sessions; the strict build is clean.docs(guide): rewrite aggregations guide and examples (#710) (#740)
0x054 commentedon Oct 19, 2026
Merged in #740 into
feat/docs-rewrite