docs(guidelines): internal-doc conventions from six harvest runs - #10421
Open
saltas888 wants to merge 3 commits into
Open
docs(guidelines): internal-doc conventions from six harvest runs#10421saltas888 wants to merge 3 commits into
saltas888 wants to merge 3 commits into
Conversation
Ports the remaining content of #10030 onto stable. That PR's branch has develop merged into it, so retargeting it would have turned a 35-file docs change into a 157-commit develop-to-stable merge; this carries the same net patch on a branch cut from stable instead. The harvesting-review skill, the internal-doc house style, the pre-ci CI-parity step, and the citation-rot pruning sweep already reached stable via #10270 and #10259. What was still missing is the conventions themselves: - dev/guidelines/backend/exceptions.md and asgi-middleware.md, split out of python.md so each loads only on its trigger - backend rules: Sequence[T] over T | Sequence[T], concrete-only n.kind, changelog secret masking, InfrahubBatch concurrency, retry_db_transaction scope, query limit/offset and auto-pagination - testing rules: verify wiring by parsing source, assert intermediate signals on converging paths, poll instead of sleep, parametrize with dataclasses - frontend rules: reset shared atoms and URL state between tests, assert the value not the text beside it, no owned state mirrored into a Jotai atom - process: PR ceremony scales with the change, migration-layer fixes over lazy repair in save(), Settings fields reach the hand-maintained compose anchor - development/grafana/AGENTS.md for dashboard conventions query-pattern.md's five near-identical walkthroughs collapse into one example plus an accessor table, paying for the additions. Docs only. Every symbol the new rules name resolves on stable, and all relative links and section anchors check out.
Contributor
There was a problem hiding this comment.
1 issue found across 35 files
Confidence score: 5/5
dev/guidelines/backend/checklist.mdmakes an overly broad claim that every new setting must be reachable from every Compose entry point, which could mislead contributors about documented development-onlyINFRAHUB_*settings; narrow the wording to explicitly exclude that category.
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/checklist.md">
<violation number="1" location="dev/guidelines/backend/checklist.md:64">
P3: The opening claim that a new setting 'must be reachable from every Compose entry point' is over-broad: the codebase has a documented category of development-only `INFRAHUB_*` settings (such as the dev-only experimental dark-theme flag) that is intentionally excluded from the root generated `docker-compose.yml` and exposed only through `development/docker-compose.yml`. Under that rule a contributor adding a dev-only setting would be instructed to add it to the production-facing root compose. Qualify the statement (e.g. 'every Compose entry point unless the setting is intentionally dev-only') so the guidance matches the existing dev-only exposure policy.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
Each was checked against stable's code before changing:
- grafana: rabbitmq_instance_monitoring.json really does define a plain
${datasource}, so the blanket "never a generic $datasource" was wrong;
the rule is now to check the dashboard's own templating block
- python.md: lead with list[str]/tuple[str] rather than Sequence[str]. A
bare str satisfies Sequence[str], so that annotation left open the very
character-iteration bug the section is about
- creating-async-tasks.md: Step 5 and Step 6 demonstrated the anti-pattern
the new caution warns about. Step 6 now passes the branch tag at
submission, and Step 5 says which tags belong where. Step 6 also called
workflow.submit(), which does not exist — corrected to submit_workflow()
- AGENTS.md: invoke lint also runs yamllint repo-wide; only its ruff checks
are directory-scoped
- git-workflow.md: dropped the older, vaguer lint/tooling bullet the new
one supersedes rather than leaving both
- creating-migrations.md: per-item try/collect only works for an
ArbitraryMigration running auto-commit statements. GraphMigration and
SchemaMigration wrap execute in start_transaction(), where a failed
statement aborts the transaction
- docs/AGENTS.md: the site defaults to light mode and supports a dark theme;
it does not render on one
- testing.md: the cited wiring test walks flow source for service.database
access, not decorators off a module tree
- testing.md: the good example's loop body was a literal no-op against an
unused cache, so the assertion passed trivially. Rewritten against the
real MemoryCache API (list_keys/delete/set) with a seeded store
- mutations.md: mutate_upsert reaches only the undecorated mutate_create
and _call_mutate_update, so it is the single retry point today; nesting
is the hazard to avoid, not the current state
- creating-changelog-entries: dependency bumps are `changed`, which
changelog.mdx already states; housekeeping covers build/tooling/CI
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 10 files (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
- creating-changelog-entries is a project-agnostic skill, and it already
states further down that dependency-bump classification is
project-specific. Dropped the unqualified "a dependency bump is
`changed`" claim I had added, which contradicted it; the Infrahub answer
stays in changelog.mdx, which the skill already points to
- grafana: "most define both" was wrong. All six non-rabbitmq dashboards
define ${datasource_prometheus}; only the three with Loki panels also
define ${datasource_loki}, and loki_monitoring.json is not one of them
- creating-async-tasks: Step 6 submits a branch parameter, so the Step 2
flow it targets now declares one
Contributor
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. PR updates internal developer documentation, agent skill/rule files, and dev guidelines across .agents/ and dev/ — no production code, config, API, or schema changes.
Re-trigger cubic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
This PR is the same net patch as #10030, on a branch cut from
stable.