docs: harvest review lessons from PRs reviewed 2026-09-11 to 2026-09-18

[saltas888]

Why

Weekly harvest of review lessons (/harvesting-review, scheduled run). Reviewer comments from the last week keep restating conventions that either weren't written down or weren't landing; this PR routes the durable ones into the internal-doc layer and prunes what they supersede, so the layer ends 11 lines shorter than it started (+59/−49 across 8 files).

Targets stable like the previous harvests (#10512, #10607): every edited file is internal documentation, branch-agnostic, and reaches develop through the stable→develop merge.

PRs the lessons came from

PR	Reviewer(s)	What it taught
#10624	ajtmccarty	Inline trivial shared Cypher (only a substantial multi-consumer predicate earns a constant); don't string-match generated query text in tests
#10587	ajtmccarty, saltas888	No defaulted collaborator factories reaching into settings; methods stay on the instance rather than being demoted to @staticmethod for a linter
#10606	ajtmccarty, polmichel	(report-only: non-isolated-branch special-cases, per-module component-test CI cost, USER/INTERNAL workflow tagging)
#10622	saltas888, polmichel	Models/validators raise domain errors; HTTP status mapping lives in the app-level handlers registered in server.py
#10615	petercrocker	FAQ answers stay short and never present opt-in capabilities as defaults
#10614	ogenstad	Drop the stale Python 3.9/3.10 guidance from typing.md
#10661	cubic	Five lab pages were deleted without the redirect entries docs/AGENTS.md mandates — the missing redirects-pending record is added here
#10566	polmichel	Don't store derived state — property over a second field, __post_init__ rejects impossible combinations

What changed

What was removed (each cut compensates an addition, per the harvest skill's refine, don't accrete rule):

File	Cut	Why it is not a loss
dev/guidelines/backend/python.md	The Query example in Query Pattern and the tier bullet list in Testing	Both restated the canonical pages (query-pattern.md, testing.md); now two-line pointers
dev/knowledge/backend/query-pattern.md	The Query.__init__ code listing	The attribute table right below it carries the same information
dev/guidelines/backend/testing.md	The "at some point integration tests will be merged…" note; one of three "Bad" examples in GraphQL Result Assertions	The note was a dated future plan (§5 rot); the cut example was a duplicate of the substring-match example above it — the rule text is untouched
dev/guidelines/backend/typing.md	The "(Python 3.10+)" / "(Python 3.9+)" parentheticals	The repo targets 3.14; ruff flags legacy spellings (raised by ogenstad on #10614)

What was added:

• dev/knowledge/backend/query-pattern.md — Keep Cypher readable inline now states the sharing boundary the rule lacked (which is why fix(backend): evaluate branch-agnostic retirement per batch when deleting a branch #10624 extracted fragments the reviewer had to ask to inline): a predicate of dozens of lines with 3–4 consumers earns a constant; a few-line MATCH used twice is pasted into both call sites.
• dev/guidelines/backend/testing.md — What not to test: no string-matching tests over generated query text; assert behavior in a component test and keep the shape rationale in the query's comments (ajtmccarty on fix(backend): evaluate branch-agnostic retirement per batch when deleting a branch #10624, fix landed in 5ae63da).
• dev/guidelines/backend/python.md — Don't store what you can derive (property over a mirrored field, __post_init__ rejects impossible combinations; polmichel raised this on six threads of feat: publish the repository git-state GraphQL contract [IFC-3146] #10566, fixes in ee9fbe8/ee07f8464/4cc3b915d) and Methods stay on the instance (ajtmccarty on fix: heartbeat thread and linear conflict merge for large branch diffs #10587, reverted in 2178c14; the repo suppresses PLR6301 deliberately).
• .agents/rules/backend-component-design.md — the required-collaborator sentence now also names the defaulted-factory shape (cache_factory: CacheFactory = build_default_cache) that fix: heartbeat thread and linear conflict merge for large branch diffs #10587 shipped and ajtmccarty had to flag; the rule previously only pattern-matched the | None = None form.
• dev/guidelines/backend/exceptions.md — Raise domain errors, never HTTP responses: the layering saltas888 asked about on fix(api): report one 422 error per schema write-contract violation #10622 and polmichel had to reconstruct from server.py; written down so the next reader doesn't propose an HTTPException inside a Pydantic validator.
• docs/AGENTS.md — FAQ answers stay short, and never present a capability that needs explicit deployment as the default (petercrocker's three post-merge comments on docs(faq): correct the backup and upgrade answers #10615, which landed no fix — this rule is the fix).
• docs/redirects-pending/learn-labs.yml — the record for the five learn/labs/* URLs docs(learn): point Infrahub Labs at the labs catalogue #10661 deleted; docs/AGENTS.md requires an entry in the same PR and the merged PR carried none. The catalogue lives outside this Docusaurus project, so the file notes that aggregation may need server-level handling.

Covered but still flagged (existing rules reviewers had to re-raise; no duplicate rules added):

• Narrative code comments — flagged ~12 times this week across fix(backend): evaluate branch-agnostic retirement per batch when deleting a branch #10624, fix: heartbeat thread and linear conflict merge for large branch diffs #10587, fix: index-anchored diff save and node-chunked diff calculation #10599, feat(graphql): InfrahubRepositoryBranchStatus per-branch attribute read from the graph (IFC-3127) #10606, feat(frontend): retry requests the API sheds under load #10543 despite .agents/rules/code-doc-style.md. docs: always load the comment rule and drop the full-docstring mandate #10657 (merged 09-17) already responded by making the rule always-loaded and adding the exception-contract carve-out; most of this week's violations predate that, so no further edit — worth re-checking next harvest.
• Redirects on page deletion — the rule exists twice in docs/AGENTS.md (Restructuring + Never list) and was still skipped in docs(learn): point Infrahub Labs at the labs catalogue #10661; this PR fixes the instance rather than adding prose.
• Exact-message assertions — cubic re-flagged class-only exception assertions on feat(backend): global TLS CA bundle for outbound components [INFP-586] #10487; testing.md Exception Testing and Assert exact expectations already cover it.
• Changelog-fragment bot nags — twice correctly declined by authors citing the creating-changelog-entries skill (fix to unreleased change on fix(backend): evaluate branch-agnostic retirement per batch when deleting a branch #10624; imperceptible perf change on feat(graphql): InfrahubRepositoryBranchStatus per-branch attribute read from the graph (IFC-3127) #10606). The skill's boundary is working.

Deliberately not codified (recorded so the next harvest doesn't re-litigate):

• get_one_by_id_or_default_filter not enforcing its kind (information leak past permissions, feat: publish the repository git-state GraphQL contract [IFC-3146] #10566) — tracked as bug: get_one_by_id_or_default_filter returns nodes of a kind it was not asked for #10655; the systemic fix beats a doc rule.
• TLS lessons from feat(backend): global TLS CA bundle for outbound components [INFP-586] #10487 — that PR is unmerged and carries its own dev/knowledge/backend/tls.md; harvesting them here would duplicate and race it.
• "Non-isolated branches are deprecated, add no special-cases" (feat(graphql): InfrahubRepositoryBranchStatus per-branch attribute read from the graph (IFC-3127) #10606) and "each component-test module adds CI setup cost" (feat(graphql): InfrahubRepositoryBranchStatus per-branch attribute read from the graph (IFC-3127) #10606) — single-comment signals on an unmerged PR; revisit once it lands.
• saltas888's mutate-vs-return-new-object preference (refactor(changelog): extract HFID resolution into ChangelogHfidResolver #10571) — stated as directional ("requires more refactoring"), no landed change.

How to review

Start with the two guideline files that gained rules (python.md, exceptions.md), then the cuts table above — each cut should read as a no-op against the canonical home it points to.

How to test

uv run invoke docs.lint   # passes (markdownlint: 0 errors)
uv run yamllint docs/redirects-pending/learn-labs.yml

Impact & rollout

• Internal documentation and one redirects-pending record only; no runtime code, no schema, no API change.

Checklist

• Tests added/updated — N/A, docs only
• Changelog entry — none, per the skill's boundary: agent-doc/internal-doc changes are not user-visible
• External docs updated — docs/redirects-pending/ record added (not a published page)
• Internal .md docs updated
• I have reviewed AI generated content

🤖 Generated with Claude Code

https://claude.ai/code/session_01RJWaQSqzHWDkykDErJgRkk

---

Generated by Claude Code

[cubic-dev-ai]

All reported issues were addressed across 8 files

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

Re-trigger cubic</sub>

[cubic-dev-ai]

0 issues found across 5 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 auto-approve. Documentation-only harvest that prunes stale/redundant guidance and adds review-derived rules plus a pending-redirects record; no code, runtime, or public-facing behavior changes, so it is bounded and clearly beneficial.

Re-trigger cubic</sub>