docs: harvest review lessons from PRs reviewed 2026-07-31 to 2026-08-06

[saltas888]

Why

A /harvesting-review run over the PRs reviewed 2026-07-31 → 2026-08-06, the window since the previous run (#10098, which covered 2026-07-24 → 2026-07-31). Reviewers kept spending their time on the same things; each lesson lived in a thread and died there.

Stacked on #10098 → #10030 → develop. This run depends on both: it strengthens a rule #10098 added and follows the size discipline #10030 introduced.

The headline is that three of the eight lessons were already documented and a reviewer still had to raise them. Those are strengthened where they already live — no duplicate rules.

Source PRs

Fifteen merged PRs read, resolved and unresolved threads both, across develop and release-1.11. Dependabot bumps and branch-sync merges skipped.

PR	What review surfaced
#10055	ajtmccarty ×3: a component building its collaborator mid-run; wiring errors raised at runtime instead of construction. cubic: a cascade source accepted with no output capturer; both sides of the contract erased to Any
#10058	ajtmccarty: inject SchemaManager instead of reaching for registry; a predicate living on the model
#10095	ajtmccarty: parametrize over dict keys with an in-test lookup; task logic that should live in a module, which would also retire its deferred imports; a closed value set typed as str
#10106	gmazoyer ×2: a Query subclass keeping its own limit/offset; get_as_list_of_type existing and being missed
#10096	gmazoyer: spec vocabulary in a test name that means nothing without the spec
#10108	ajtmccarty: an absolute performance figure in an ADR. cubic: an ADR claim the code contradicts
#10111	saltas888: URL state mirrored into a Jotai atom. cubic: a toast assertion that passes with the wrong branch name
#10128	saltas888: is a useMemo still needed under the React Compiler? cubic: an isFetching flag the provider never reads, making its test inert
#10129	cubic ×3: "main" → is_default behavior changes with no test exercising a non-main default
#10114	ajtmccarty ×3: doc claims about merged-branch behavior the code contradicts
#10090, #10104	bot findings retracted after a human corrected them
#10123, #10125, #10136	scanned, no durable lesson

#10079 and #10002 also merged in this window but were already harvested into #10030.

Strengthened — the rule existed and was missed anyway

.agents/rules/backend-component-design.md — one reviewer made the same point four times across two PRs, against a rule that is auto-injected into every agent turn. It only forbade constructing a sub-component inside a parent's __init__, so building one part-way through a run slipped past it, and it said nothing about when bad wiring must fail. Now: the graph is built at the entry point in one pass, and a mismatch raises there rather than degrading to a runtime fallback. Paid for by merging two overlapping paragraphs on constructor injection — the file grows by 4 lines for two new rules.

dev/guidelines/backend/testing.md — said "use dataclasses for parametrized cases" but never named the hybrid that got written. Now names it. Paid for by compressing the four-bullet "when to use" list that said one thing.

dev/guidelines/frontend/page-architecture.md — the no-mirroring rule was written for forms and useState, so URL state copied into a Jotai atom wasn't covered. Generalized to any state with an owner in the table above it.

Added

• dev/knowledge/backend/query-pattern.md — pass limit/offset to the base Query constructor. Verified in core/query/__init__.py: execute() branches on self.limit or self.offset, so a subclass holding private copies leaves both None and gets query_with_size_limit()'s chunked re-execution on top of its own paging. Plus: read a collect() column with get_as_list_of_type.
• .agents/rules/code-doc-style.md — spec vocabulary is as unreadable as a spec ID in a test name or docstring.
• dev/guidelines/documentation.md — measurements go in as relative comparisons; the absolute figure depends on the environment it was taken in.
• dev/guides/frontend/writing-component-tests.md — a test must fail when the behavior breaks: assert the value rather than the static text beside it, and check the component actually reads the flag the setup varies.
• dev/guidelines/backend/python.md — the tasks/*.py function-local import exception (added in docs: harvest review lessons from PRs merged 2026-07-24 to 2026-07-31 #10098) covers a thin wrapper, not logic hosted in tasks/. A task body needing a dozen deferred imports is saying the logic belongs elsewhere.

Pruned

• dev/guidelines/frontend/typescript.md taught useMemo/useCallback dependency arrays while dev/knowledge/frontend/react.md says not to use them at all under the React Compiler. A reviewer had to ask which was right — the contradiction is the finding.
• dev/knowledge/backend/schema-definitions.md pointed at an issue for "planned improvements" that closed on 2026-07-20.
• dev/knowledge/backend/query-pattern.md was 520 lines against a 200–400 range, and this run adds to it, so its five near-identical result-dataclass walkthroughs collapse into one example plus an accessor table — which is where the get_as_list_of_type lesson naturally lands. The file ends at 459.

Sweep otherwise clean: no defect-snapshot notes survive, and the remaining #NNNN grep hits are git-workflow.md's issue-syntax examples and documentation.md's own rule text.

Context budget

+56 / −92 lines — net −36. Every file inside its range except query-pattern.md, which is 59 over after dropping 61 lines; splitting its ## Internals block is the next candidate and wants its own change. writing-component-tests.md is at 398 of 400, so the next lesson landing there compresses first.

File	Lines	Range
dev/knowledge/backend/query-pattern.md	459 (was 520)	200–400
dev/guides/frontend/writing-component-tests.md	398	200–400
dev/guidelines/backend/testing.md	403	100–400
dev/guidelines/backend/python.md	396	100–400
.agents/rules/backend-component-design.md	98	—

Not lessons

#10106's keyset-pagination pair — ajtmccarty rejected it with reasoning and nothing landed. #10090's "SDK lacks IPAddress" and #10104's "missing author filter" — both bots retracted after being corrected. #10055's artifact-hard-coded failure fallback — real, but confined to that module. #10114's doc corrections and #10108's contradicted ADR claim — already covered by the verify-claims-against-the-diff rule root AGENTS.md gained last run. A "TIL", a screenshot request, and a jotai-naming aside — no rule.

How to test

Docs only. markdown-lint, validate-dev-guideline-links, and validate-documentation-style cover these paths in CI; uv run invoke docs.lint globs docs/docs/**, so it doesn't reach dev/ or .agents/. Relative links and code-fence balance checked locally.

🤖 Generated with Claude Code

[cubic-dev-ai]

2 issues found across 10 files

Confidence score: 4/5

• In dev/guidelines/backend/testing.md, the new statement about keeping case data inside @parametrize conflicts with the existing module-level *_TEST_CASES pattern, which can cause inconsistent tests and reviewer churn as teams follow different styles — align the guidance to one canonical pattern and update the example to match.
• In dev/guidelines/backend/python.md, recommending a top-level import in tasks can interact badly with tasks/__init__.py eager submodule imports, potentially reloading much of the backend on each invoke and slowing local tooling — narrow the recommendation to lazy/local imports (or document the task-package exception explicitly).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="dev/guidelines/backend/testing.md">

<violation number="1" location="dev/guidelines/backend/testing.md:246">
P3: The new guidance "the case data lives in the `parametrize` decorator" contradicts the documented pattern just above it, which places case data in a module-level constant (`MY_FUNCTION_TEST_CASES`) and has the decorator reference that list. Taken literally it would push developers to inline case data into the decorator, violating guideline #4 (define test cases as module-level constants). Rephrase to say the decorator passes the full case object (so the pytest id identifies it), while the data stays in the module-level list.</violation>
</file>

<file name="dev/guidelines/backend/python.md">

<violation number="1" location="dev/guidelines/backend/python.md:84">
P3: This recommendation can undo the exception it extends: `tasks/__init__.py` eagerly imports every task submodule, so a top-level import of the new module in a task would reload the full backend on every `invoke` — the exact eager-load the exception avoids. The new module should keep heavy imports deferred (or the task should import it lazily), otherwise the 'deferred imports mostly disappear' advice is misleading when followed literally. Consider clarifying where the single import lives so the guidance stays consistent with the stated rationale.</violation>
</file>

<sub>Shadow auto-approve: would not auto-approve because issues were found.

Re-trigger cubic</sub>

[cubic-dev-ai]

P3: The new guidance "the case data lives in the parametrize decorator" contradicts the documented pattern just above it, which places case data in a module-level constant (MY_FUNCTION_TEST_CASES) and has the decorator reference that list. Taken literally it would push developers to inline case data into the decorator, violating guideline #4 (define test cases as module-level constants). Rephrase to say the decorator passes the full case object (so the pytest id identifies it), while the data stays in the module-level list.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At dev/guidelines/backend/testing.md, line 246:

<comment>The new guidance "the case data lives in the `parametrize` decorator" contradicts the documented pattern just above it, which places case data in a module-level constant (`MY_FUNCTION_TEST_CASES`) and has the decorator reference that list. Taken literally it would push developers to inline case data into the decorator, violating guideline #4 (define test cases as module-level constants). Rephrase to say the decorator passes the full case object (so the pytest id identifies it), while the data stays in the module-level list.</comment>

<file context>
@@ -240,12 +240,10 @@ SCHEMA_VALIDATION_TEST_CASES: list[SchemaValidationTestCase] = [
-- Test cases share a common structure
-- You want readable test IDs in pytest output
-- The test logic is the same but data varies
+Whichever you pick, the case data lives in the `parametrize` decorator. Don't parametrize over the keys of a module-level dict and look the values up inside the test — the reader has to hold two places in their head to see what a case actually asserts, and the pytest id no longer tells them.
 
 For simpler cases with only 2-3 scenarios, standard `@pytest.mark.parametrize` with tuples may be sufficient:
</file context>

Suggested change

	Whichever you pick, the case data lives in the `parametrize` decorator. Don't parametrize over the keys of a module-level dict and look the values up inside the test — the reader has to hold two places in their head to see what a case actually asserts, and the pytest id no longer tells them.
	Whichever you pick, the parametrize decorator passes each case object whole — the pytest id is the case's `name` — rather than parametrizing over the keys of a module-level dict and looking the values up inside the test. The latter makes the reader hold two places in their head to see what a case actually asserts, and the pytest id no longer tells them.

[cubic-dev-ai]

P3: This recommendation can undo the exception it extends: tasks/__init__.py eagerly imports every task submodule, so a top-level import of the new module in a task would reload the full backend on every invoke — the exact eager-load the exception avoids. The new module should keep heavy imports deferred (or the task should import it lazily), otherwise the 'deferred imports mostly disappear' advice is misleading when followed literally. Consider clarifying where the single import lives so the guidance stays consistent with the stated rationale.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At dev/guidelines/backend/python.md, line 84:

<comment>This recommendation can undo the exception it extends: `tasks/__init__.py` eagerly imports every task submodule, so a top-level import of the new module in a task would reload the full backend on every `invoke` — the exact eager-load the exception avoids. The new module should keep heavy imports deferred (or the task should import it lazily), otherwise the 'deferred imports mostly disappear' advice is misleading when followed literally. Consider clarifying where the single import lives so the guidance stays consistent with the stated rationale.</comment>

<file context>
@@ -81,6 +81,10 @@ deliberate, documented exception: `pyproject.toml`'s `"tasks/**.py"` per-file-ig
 imports (stdlib, `invoke`, sibling `.shared`/`.utils` modules) at the top; defer the rest into the
 function that needs them.
 
+The exception covers a thin task wrapper, not logic that happens to live in `tasks/`. A task body
+needing a dozen deferred imports is telling you the logic belongs in a module of its own, which the
+task then imports once — put it there and the deferred imports mostly disappear with it.
</file context>

[saltas888]

Cherry-picked into #10030 as 376403e (review follow-ups in ae7986f). Closing in favor of #10030 so the harvest series lands as one PR.