docs: harvest review lessons from PRs merged 2026-07-24 to 2026-07-31

[saltas888]

Why

Reviewers repeatedly had to spell out the same conventions across this week's PRs, because each lesson lived in a review thread and died there. This is a harvesting-review run over PRs merged 2026-07-24 → 2026-07-31 (the window since the last such run, #10032), turning feedback that generalizes into durable internal-doc conventions so a future author (human or agent) follows the convention the first time instead of a reviewer having to raise it again.

Each candidate lesson was reconstructed against the before/after code, verified against the current codebase, and checked against what's already documented before landing — per .claude/skills/harvesting-review/.

Targets stable, not develop. Retargeted on request. Since stable is behind develop by a large margin, several lessons from this run document features that don't exist on stable at all and were dropped from this PR rather than carried over describing nonexistent code:

• dev/knowledge/frontend/diff-tree.md (the diff-tree/diff-list node-set fix, from fix: show changed nodes in diff tree when their parent is unchanged #10014) — the underlying frontend fix (build-diff-tree-items.ts) isn't on stable.
• The dev/knowledge/backend/merge-recompute.md invariant note (from fix(recompute): refresh a reader's derived values when its read peer is deleted #9845) — the whole file, and the coalesced-recompute module it describes, don't exist on stable.
• The Query.get_query_arrows() directional-Cypher lesson in database-schema.md/query-pattern.md (from the scope uniqueness constraints to specific objects #10053/scope uniqueness constraints to affected objects #10019/final uniqueness-constraint related touchups #10082 cluster) — that helper doesn't exist on stable.
• The ComputedAttribute generated-model-drift note in schema-definitions.md (from feat: separate user-facing schema (write/read) from internal model [INFP-234] #9814) — the whole "Field Visibility and the Write/Read/Internal Models" section it was attached to isn't on stable (that feature hasn't landed there either).

One example (StaleEntryCleaner/clear_expired) in dev/guidelines/backend/testing.md was genericized from its original StaleLockCleaner naming for the same reason — the rule is real and durable, but the specific class it illustrated (from #10031) isn't on stable. The pruning fixes (see below) also picked up stable's own, slightly different copy of design-system.md (an older table layout with its own separate set of stale Migrated in #NNNN citations) and cleaned that up in place rather than trying to overwrite it with develop's newer table structure.

Three rounds of follow-up after the initial push, all from user review:

1. The "no file.py:line citations" lesson had been mis-routed into .agents/rules/code-doc-style.md (a source-code comment/docstring rule, auto-injected on every turn touching .py/.ts/.tsx) by extending its paths frontmatter to also cover markdown docs. Re-homed to dev/guidelines/documentation.md instead, next to the sibling PR/spec-citation rule it belongs beside. Also dropped a "known gap" note that described a current code defect rather than a durable convention.
2. Added a "sweep for rot" step to the skill and reported 19 pre-existing citation-rot hits elsewhere in the docs — but didn't fix any of them. Called out for describing debt instead of removing it. Went back and fixed all 19 (dropped stale PR #NNNN / Migrated in #NNNN citations across 5 files, kept the underlying prose), and while verifying one file's citations against current code, found the rot wasn't just stale line numbers — templates.md cited a method, Node._apply_template(), that no longer exists (renamed to handle_object_template()); fixed that too. Also tightened the skill itself so this doesn't recur: §5's "report the rest as a punch list" escape hatch is gone — every mechanical hit is now expected to be fixed in the same PR, not deferred.
3. Retargeted to stable (this update) — required rebasing the branch and dropping/adapting the develop-only-feature lessons above rather than mechanically replaying the same diff onto a base where they don't apply.

Source PRs (audited against develop, where all twelve were merged)

Twelve PRs (one cluster of three related PRs counted once) were audited, one subagent per PR:

• perf(graphql): skip peer hydration for id-only single relationships #10062 (perf: id-only relationship hydration) — cubic: no benchmark exercised the id-only fast path, so CI couldn't have caught a regression.
• fix: show changed nodes in diff tree when their parent is unchanged #10014 (fix: diff tree dropping unchanged-parent nodes) — cubic: Jotai/mock/URL state leaking between component tests in the same file; pa-lem: the diff tree and the changed-nodes list must not share one filtered node set (bug recurred twice in the same PR, once per filter) — plus a "safe by construction" non-null-assertion trap on a re-derived Map lookup. (lesson dropped from this PR — feature not on stable, see above)
• refactor(frontend): replace Apollo Client with urql for the GraphQL transport [INFP-563] #10059 (refactor: Apollo → urql) — pa-lem: a lazily-memoized module-level client cache has no test-reset hook, so state leaks across tests in the same file.
• feat: separate user-facing schema (write/read) from internal model [INFP-234] #9814 (feat: user-facing write/read schema split) — ajtmccarty: tasks/*.py backend imports must stay function-local (asked 4 times in one PR); cubic: a generated ComputedAttribute sub-model is hand-duplicated from its canonical model with no drift guard. (the drift-guard lesson dropped from this PR — feature not on stable, see above)
• feat(api): priority-aware server-side API backpressure #9879 (feat: API backpressure) — cubic: path-exclusion matching used a bare startswith (and the same bug still lives, unfixed, in ConditionalGZipMiddleware); ajtmccarty's DI/observer and settings-at-boundary findings substantially overlap with the still-open #10065, so they were not duplicated here.
• feat(merge): selective post-merge generator & artifact regeneration (IFC-2704) #10038 (feat: selective post-merge regeneration) — cubic: 5 threads on stale file.py:line citations in dev/specs/dev/knowledge docs; a second optional filter field whose emptiness got conflated with "no filter at all"; a test whose final assertion converges via two different code paths.
• scope uniqueness constraints to specific objects #10053 / scope uniqueness constraints to affected objects #10019 / final uniqueness-constraint related touchups #10082 (uniqueness-constraint scoping cluster) — cubic: an undirected IS_RELATED match over-resolves a self-referential relationship (dropped from this PR — helper not on stable, see above); a reused component's initialize() reset its index but left a sibling cache stale.
• fix(recompute): refresh a reader's derived values when its read peer is deleted #9845 (fix: recompute on deleted read peer) — ajtmccarty: a changelog fragment can be named after a Jira ID instead of a GitHub issue number (3 already-broken links exist in CHANGELOG.md from this exact mistake); cubic: a new fallback branch in the coalesced recompute resolver only widened one of the two symmetric targets it needed. (the recompute-invariant lesson dropped from this PR — feature not on stable, see above)
• perf(computed-attribute): batch Python transform recompute #10034 (perf: batch Python-transform recompute) — ajtmccarty: add_tags() mid-run silently drops an earlier call's tags; the documented fix (in dev/knowledge/backend/async-tasks.md) isn't linked from the guide that teaches the unsafe pattern.
• merge failure recovery improvements #10031 (fix: merge failure recovery) — polmichel: a mutation method returned a value that only the test consumed (example genericized for stable, see above); a component should take a capability-scoped Protocol even outside a "should I mock?" trigger (overlaps with some updates for dev documentation #10065, not duplicated).

Also scanned with no durable lesson: #10051 (single bot thread, rebutted correctly, no fix landed).

Not duplicated: several DI/observer/config-validation findings from #9879 and #10052 (audited in this run's research but not turned into edits here) are already comprehensively covered by the still-open #10065 — landing a competing edit to the same sections would conflict rather than help.

What changed

New rules/knowledge added:

• dev/knowledge/backend/testing.md: a benchmark must exercise the exact input shape a conditional perf optimization is gated on.
• dev/guides/frontend/writing-component-tests.md: reset shared Jotai atoms / clear mocks / reset URL state between tests in the same file.
• dev/guidelines/backend/python.md: tasks/*.py imports stay function-local (documented exception to the top-of-file rule); a path-exclusion check must not use a bare startswith.
• AGENTS.md: verify a specific identifier/metric/config-default claim against the diff before writing it into a changelog fragment, PR description, or ADR.
• .agents/rules/backend-component-design.md: a component's initialize()/reset() must clear every derived cache, not just the field it obviously replaces.
• docs/docs/development/changelog.mdx + .agents/skills/creating-changelog-entries/SKILL.md: changelog fragment names must use the GitHub issue number or +, never an internal tracker ID.
• dev/guidelines/backend/testing.md: don't add a return value to a mutating method just so a test can assert on it; when a result is reachable via two different code paths, assert an intermediate signal too.
• dev/guidelines/frontend/typescript.md: a Map lookup seeded from the same array you index it with is still a type lie if asserted with !.
• dev/guidelines/documentation.md: no file.py:line citations in internal docs.

Strengthened existing coverage (rule existed but a reviewer still had to raise it):

• dev/guides/frontend/writing-unit-tests.md: added the module-level-singleton-needs-a-reset-hook case to the existing "keep tests independent" guidance.
• dev/guides/backend/creating-async-tasks.md: added a caution + cross-link to the already-documented add_tags() pitfall right where the guide teaches the pattern that reproduces it.

Pruned (from the skill's new §5 sweep):

• dev/guidelines/frontend/page-architecture.md, route-architecture.md, dev/knowledge/frontend/shared-components.md, design-system.md: dropped 17 stale PR #NNNN/Migrated in #NNNN citations total (12 found via develop's copies, 5 more found in stable's separately-structured design-system.md table), kept the underlying prose.
• dev/knowledge/backend/templates.md: dropped 4 stale file.py:line citations and fixed a renamed-method reference (_apply_template() → handle_object_template()) surfaced while verifying them.

Skill maintenance:

• .agents/skills/harvesting-review/SKILL.md: added §5 "Sweep for rot" — every run now greps the destination layer for stale citations, fixed defect-snapshots, and superseded narrow rules, and fixes them in the same PR (tightened after the first pass only reported findings without acting on them). Added a guardrail against writing defect-snapshot content as permanent docs in the first place.

Docs-only — no code, no generated files, no schema.

How to review

None of the edits touch the sections #10065 is already changing. Everything here lands in dev/guidelines/, dev/knowledge/, dev/guides/, docs/docs/, .agents/skills/, or a .agents/rules/* addendum to an existing file (no new .agents/rules/* file, so no added per-turn token cost). Given the base is stable, the diff intentionally omits any lesson tied to develop-only code — see the "Why" section for exactly which four were dropped and why.

How to test

PATH="$PWD/docs/node_modules/.bin:$PATH" uv run invoke docs.lint

0 errors (pre-existing, unrelated markdownlint warnings elsewhere, unchanged by this PR).

Impact & rollout

• Backward compatibility: docs-only, no runtime impact
• Deployment notes: safe to merge

Checklist

• Tests added/updated (n/a — docs-only)
• Changelog entry added (n/a — internal docs)
• External docs updated (n/a)
• Internal .md docs updated (internal knowledge and AI code tools knowledge)
• I have reviewed AI generated content

🤖 Generated with Claude Code

https://claude.ai/code/session_014BhxKtqsNFkVTV3BN3Qzpj

[cubic-dev-ai]

1 issue found across 17 files

Confidence score: 5/5

• In dev/knowledge/backend/merge-recompute.md, referencing a non-existent _derive_family_targets method could mislead maintainers during debugging or future edits, increasing the chance of incorrect changes in recompute target resolution; align the docs with the current _resolve_targets family-derivation flow (or add a clear pointer to where that logic now lives).

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/knowledge/backend/merge-recompute.md">

<violation number="1" location="dev/knowledge/backend/merge-recompute.md:77">
P3: The documentation references `_derive_family_targets` as a method in `core/merge/recompute_coalescing.py`, but this method does not exist in the codebase. The family-specific derivation inside `_resolve_targets` is done through `_resolve_computed_targets` and the imported functions `derive_display_label_targets` / `derive_hfid_targets`. Consider updating the reference to match the actual code — either list the real method/function names, or use a descriptive phrase like 'the family-specific derive calls within _resolve_targets' to avoid misleading readers who search for this symbol.</violation>
</file>

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

Re-trigger cubic</sub>

[cubic-dev-ai]

P3: The documentation references _derive_family_targets as a method in core/merge/recompute_coalescing.py, but this method does not exist in the codebase. The family-specific derivation inside _resolve_targets is done through _resolve_computed_targets and the imported functions derive_display_label_targets / derive_hfid_targets. Consider updating the reference to match the actual code — either list the real method/function names, or use a descriptive phrase like 'the family-specific derive calls within _resolve_targets' to avoid misleading readers who search for this symbol.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At dev/knowledge/backend/merge-recompute.md, line 77:

<comment>The documentation references `_derive_family_targets` as a method in `core/merge/recompute_coalescing.py`, but this method does not exist in the codebase. The family-specific derivation inside `_resolve_targets` is done through `_resolve_computed_targets` and the imported functions `derive_display_label_targets` / `derive_hfid_targets`. Consider updating the reference to match the actual code — either list the real method/function names, or use a descriptive phrase like 'the family-specific derive calls within _resolve_targets' to avoid misleading readers who search for this symbol.</comment>

<file context>
@@ -72,6 +72,19 @@ An empty write set dispatches nothing, which is the normal stop: an acyclic depe
 
+## Adding a new resolution branch
+
+`CoalescedRecomputeBuilder`'s target resolution (`_resolve_targets` / `_derive_family_targets` in
+`core/merge/recompute_coalescing.py`) branches on how precisely a change signature is known — an
+`UPDATED` event with a recorded field list resolves targets narrowly; an unscoped or otherwise
</file context>

Suggested change

	`CoalescedRecomputeBuilder`'s target resolution (`_resolve_targets` / `_derive_family_targets` in
	`CoalescedRecomputeBuilder`'s target resolution (`_resolve_targets` and its family-specific derive calls in

[cubic-dev-ai]

0 issues found across 4 files (changes from recent commits).

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

<sub>Shadow auto-approve: would not auto-approve. Auto-approval blocked by 1 unresolved issue from previous reviews.

Re-trigger cubic</sub>

[cubic-dev-ai]

0 issues found across 6 files (changes from recent commits).

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

<sub>Shadow auto-approve: would not auto-approve. Auto-approval blocked by 1 unresolved issue from previous reviews.

Re-trigger cubic</sub>