Harvesting Review

4. Check existing coverage, then route (dedup)

For each investigated lesson, grep the internal-doc layer for the rule:

git grep -in "<keyword>" -- .agents/rules/ dev/guidelines/ dev/knowledge/ dev/guides/ '*AGENTS.md'

A grep hit is not coverage until you read it. Before you call a rule "already covered" — whether to report it as covered-but-flagged or to demote it as "already codified, not a lesson" — open the matched file at that line, confirm it states this rule and not an adjacent one, and cite the exact file:line. A keyword that merely co-occurs (an enum-default rule is not covered by a dependency-injection rule that happens to mention enums) is a coincidental match, not coverage.

A reviewer flagged this, so the verdicts are not a pass/fail of the docs — they are:

• Missing — the rule is written nowhere → propose the smallest addition in the most-specific home. If the only reason to write it down is a specific defect in the code today ("X is currently hand-duplicated", "Y isn't fixed yet"), it is not documentation — it will read as false the moment someone fixes it, and nothing revisits it. Phrase it as a forward-looking convention that stays true regardless of whether this exact instance ever gets fixed ("when adding a value generated this way, derive the fields rather than hand-listing them"), or drop it and suggest filing a GitHub issue instead of writing it into dev/knowledge/dev/guidelines. The same test applies when the root cause is a fragile pattern rather than a defect: if the honest fix is to stop writing the pattern, the lesson is the rule steering to the plain alternative, not a section teaching authors to survive it. A survival guide entrenches what it documents.

• Covered but ineffective — the rule is written, yet a reviewer still had to flag it. This is a finding, not a relief — there is deliberately no "covered and fine" verdict, because a documented rule a reviewer still had to raise is evidence the coverage is too weak, not proof it works. Report it prominently, never as "the layer works". Diagnose why it didn't land and propose how to make the existing doc land — do not add a duplicate rule. Pick the diagnosis:

  • too abstract / no worked example — states the principle but not the concrete case the author needed. The clearest signal: the reviewer had to describe the shape themselves (the class to build, the collaborators to inject, the method to call). If a reviewer has to draw the construction, the section is too abstract → add the ✅/❌ worked example that turns the flagged anti-pattern into the pattern.

  • not discoverable — the rule is in the right topical doc, but nothing pulled that doc into context when the author needed it. The fix is almost always two coordinated edits, not a move (the 1A+1B fix):

    • 1A — fix the load-trigger. Add or upgrade the doc's entry in the router/index (the relevant AGENTS.md list) so it says when to load the doc — the triggering task or symptom — not just what it covers. A doc absent from that list, or listed with a topic-only description ("Query patterns"), never gets loaded; most "covered but ignored" rules fail here. Keep the entry to one or two sentences and rewrite rather than append: the router is a scannable index, and an entry that swells into a paragraph stops being read. Name the trigger and topic, never the rule's mechanics — a symbol name copied into the index rots the moment it is renamed (write "workflow-name conventions", not "reference names via SomeClass.name").
    • 1B — strengthen the rule in place. Promote it out of any niche section into a prominent home and add the carve-out — but leave it in its topically-correct doc.

    Never relocate a domain rule (schema, DB, events, async-tasks…) into a general style guide because that doc is read more often: the topically-wrong home is less trustworthy, not more discoverable. The router entry is the discoverability mechanism, so fix the load-trigger, not the location.

  • mis-homed — the rule genuinely sits in the wrong topical doc, or belongs in a task's pre-submit checklist → move it to the most-specific correct home (or add the checklist line), then apply 1A so that home is actually loadable.

  • stale / contradicted — the doc no longer matches the code, so authors discount it → correct it.

  Scope is not an alibi for the docs. "Too much for this PR", "out of scope", or an existing-code carve-out are reasons not to change code now — they never exempt the documentation from being made more concrete. Before you lean on such a carve-out, check it applies: an existing-code exemption protects pre-existing code, not new code written in the old style (verify in the diff). Concluding "covered, correctly scoped, no edit" is the rationalization this step exists to prevent — reach for it only when the existing section already contains the concrete example the reviewer was forced to supply.

• Not applicable — the grep match was coincidental, or the comment was not actually a violation of the rule (a question, a one-off) → demote to Not a lesson, with the reason.

Routing rule of thumb: most-specific existing home wins; edit before create; strengthen before duplicate; fix the load-trigger before relocating; .agents/rules only for a true always/never that must fire while coding. Confirm the target file exists (ls/grep it, match sibling naming) before you route a lesson there — never invent a plausible-looking path. Also check the topic still lives in dev/ at all: changelog conventions, for one, moved out to the creating-changelog-entries skill. When an existing skill or command already owns the workflow a lesson touches (creating-changelog-entries, pre-ci, or pruning-residues from the org skills plugin — not vendored in this repo but available to agents running with it), route the edit into that skill and leave at most a pointer in dev/ — the same rule stated in two homes drifts apart.

5. Sweep for rot (prune before you add)

Every run of this skill only adds. Nothing else revisits what a previous run wrote, so the layer grows monotonically — a doc entry that was true and useful the week it landed can quietly become stale, redundant, or wrong, and stays in place forever unless a run like this one checks it. Do this sweep every time, not as an occasional separate cleanup — it is cheap (a handful of greps, not a re-read of every doc) and it is what keeps "harvested" from becoming a synonym for "bloated."

a. Mechanical staleness grep. Across the destination layer (dev/guidelines/, dev/knowledge/, dev/guides/, .agents/rules/, AGENTS.md, area AGENTS.md files), grep for the anti-patterns the docs already forbid — a rule existing but nobody enforcing it against older content is exactly the "covered but ineffective" failure mode, aimed backward instead of at this PR:

grep -rnoE '(PR #[0-9]+|#[0-9]{4,6}\b)' dev/guidelines dev/knowledge dev/guides .agents/rules $(git ls-files '*AGENTS.md')
grep -rnoE '[A-Za-z0-9_/-]+\.(py|ts|tsx):[0-9]+(-[0-9]+)?' dev/guidelines dev/knowledge dev/guides .agents/rules
grep -rniE '(known gap|currently (broken|hand-duplicated|unfixed)|not yet fixed|for now,? (this|it))' \
  dev/guidelines dev/knowledge dev/guides .agents/rules

A hit outside this run's own new edits is debt from an earlier run (or from a doc written outside this skill). For each:

• A stale citation (PR/issue number, spec file, line number) — drop the citation, keep the underlying behavior description the sentence was making. Don't touch surrounding prose beyond that.
• A defect-snapshot note ("known gap: X is currently...") — check the current code. If the defect is fixed, delete the note; it is now simply false. If still unfixed, either reframe it as a forward-looking convention (per the guardrail in §4) or drop it and flag it as a candidate GitHub issue instead of documentation.

b. Supersession check. When a lesson from this run generalizes something an earlier run wrote narrowly — the same idiom, now with a second, broader instance — edit the earlier entry in place (broaden its scope, replace its single example with the more general one) rather than leaving both. Two entries saying almost the same thing at different generality levels is worse than one that's right, because a future reader can no longer tell which one is current.

c. Fix every hit now — a punch list is not pruning. Each hit from (a) is a one-line mechanical edit: drop the citation and keep the prose, or delete a defect note once the code confirms the fix. A "found but left in place" list costs the same context as the rot it describes, and nobody comes back for it. While at the line, check that the claim the citation was attached to still holds — a dead citation often rides alongside a renamed method or a drifted line reference. Stay on the codified anti-patterns; this is not a second audit-docs run. The only unresolved entries "Pruned or consolidated" may carry are genuine calls for the user: file a GitHub issue for a real unfixed defect, or flag a section needing a fuller rewrite than a sweep should attempt inline.

6. Report

Present the findings (format below) and stop. Do not edit yet — internal-doc files shape every teammate's agent, so the blast radius is the whole team.

7. Apply (opt-in)

Ask which to apply: all / cherry-pick / none. Only then edit, following the §4 routing (edit an existing section before adding one; keep .agents/rules lean) and the §5 pruning findings.

Now apply Refine, don't accrete (top of this file) — measure the file against its size range, cut what the new rule supersedes, report the line counts. The investigation trail, the reviewer quotes, and the ticket and PR numbers belong in this report and in the commit message, never in the doc text — sweeping exactly that residue out of an artifact is what pruning-residues (org skills plugin, not vendored here) does, so run it over the final diff when the plugin is loaded. A lesson that needs three paragraphs to state has not been narrowed enough — go back to §3d. Match any example code to dev/guidelines/code-doc-style.md (no ticket/issue IDs, no naming specific callers). Never resolve review threads — reply if useful, but resolution is the human reviewer's call. After applying, run:

uv run invoke docs.lint

Report format

## Review-Lessons Report — PR #<n>

### Scope
<!-- PR, branch, how many threads read (resolved + unresolved) -->
<!-- After applying: lines added/removed per file, each file's size vs its range, what was cut. Zero
     deletions is a finding about this harvest, not a detail to omit. -->

### Existing coverage to strengthen (Covered but still flagged)

The rule already exists, yet a reviewer had to raise it — so the coverage is not landing. **This is the
highest-value output of the harvest, so it leads the report; when it is empty, open with "New rules to
add" instead.** For each:
- **Lesson** + **Source** (reviewer + quoted comment)
- **Already at**: the exact existing `file:line`, confirmed to be the same rule, not a keyword co-occurrence
- **Why it didn't land**: too abstract / not discoverable / mis-homed / missing from checklist / stale
- **Proposed edit**: how to make the *existing* coverage land — the concrete example to add, the
  load-trigger/relocation edit, or the checklist line. Not a duplicate rule.

### New rules to add (Missing)

For each:
- **Lesson**: the precise, scoped imperative (one sentence)
- **Source**: reviewer + quoted comment (and the addressing commit, if any)
- **Investigation**: the trail — (a) before → after (the code the reviewer saw vs. the correction that
  landed, citing the commit) and the claim verified in code (cite the files checked), (b) reviewer
  intent, (c) scope + the over-scoped reading ruled out
- **Root cause**: why an agent would have proposed the rejected shape — what writing the rule prevents
- **Home**: exact file (+ section) to create
- **Proposed edit**: the concrete text to add, already written in the house style (see *Refine, don't accrete* at the top of this file)

### Not Lessons (PR-local or demoted after investigation)
<!-- One-off fixes, bugs, and design calls that do NOT generalize — and candidates the investigation
     demoted (unverified claim, coincidental grep match, reviewer error, a question, or universal advice
     with no Infrahub-specific edge). Say why, briefly. Every lesson is grounded in a real comment; none
     is invented, and promoting every comment to a rule is as useless as missing the real ones. -->

### Pruned or consolidated (from the §5 sweep)
<!-- Debt from earlier runs, already fixed in this PR's diff — a hit reported without an edit is a bug
     in this run: stale citations dropped, defect notes deleted or reframed, narrow entries merged into
     the general one. For each: file:line, what was there, why it changed. Label the rare genuine user
     call (file an issue / fuller rewrite needed) "Needs a decision". Say "none found" when the sweep
     is clean. -->