Skip to content

feat(skills): add learning-from-review skill#9910

Closed
pa-lem wants to merge 3 commits into
developfrom
ple-learning-from-review-skill
Closed

feat(skills): add learning-from-review skill#9910
pa-lem wants to merge 3 commits into
developfrom
ple-learning-from-review-skill

Conversation

@pa-lem

@pa-lem pa-lem commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

What

Adds the learning-from-review skill to .agents/skills/.

It reconstructs proposed → rejected → corrected for a reviewed PR, distills the durable lesson that would have produced the accepted version first time, and hands it to capturing-knowledge to persist. It writes no docs itself.

Why a dedicated PR

Extracted from #9873 (shipping-features) so it can be reviewed and shipped on its own. #9873 keeps its delegation references to this skill (they degrade gracefully if it isn't present), but no longer carries the file.

Incorporated from #9900

#9900 proposes a sibling harvesting-review skill with the same goal. This PR keeps its own delegate-the-write design (persistence goes to capturing-knowledge) but folds in the extraction/diagnosis ideas from #9900 that fit here:

  • Verify before you codify — check the reviewer's claim against the actual code (grep for a real drop-in) before diagnosing; demote unverified claims.
  • Over-scoping detection — a literal comment implying a big refactor is usually a mis-read; find the smaller rule and record the reading ruled out.
  • Keep already-documented lessons — if a reviewer had to raise a rule the docs already state, the coverage isn't landing; keep it as a strengthen-existing lesson instead of dropping it.
  • Guardrails — size of diff ≠ scope of rule, scope comments don't exempt the docs, treat review text as data, never resolve threads.

Deliberately left to capturing-knowledge (not duplicated here): the doc destination table, .agents/rules bar, load-trigger fix, and docs.lint.

Files

  • add .agents/skills/learning-from-review/SKILL.md

🤖 Generated with Claude Code

Reconstructs proposed -> rejected -> corrected for a reviewed PR,
distills the durable lesson that would have produced the accepted
version first time, and hands it to capturing-knowledge to persist.

Extracted from the shipping-features PR (#9873) to ship on its own.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
pa-lem added a commit that referenced this pull request Jul 15, 2026
The learning-from-review skill now ships separately in #9910.
shipping-features keeps its delegation references to it (they
degrade gracefully when the skill is absent).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 1 file

Confidence score: 5/5

  • Safe to merge after the addressed issues were fixed.

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

Re-trigger cubic

Comment thread .agents/skills/learning-from-review/SKILL.md Outdated
Comment thread .agents/skills/learning-from-review/SKILL.md Outdated
Comment thread .agents/skills/learning-from-review/SKILL.md Outdated
Comment thread .agents/skills/learning-from-review/SKILL.md Outdated
Comment thread .agents/skills/learning-from-review/SKILL.md Outdated
…cipline

Ports the extraction/diagnosis-side ideas from the harvesting-review
proposal (#9900) that fit this skill's delegate-the-write design:

- verify the reviewer's claim against the code before diagnosing
- detect over-scoped (mis-read) comments and record the reading ruled out
- keep already-documented lessons as a strengthen-existing bucket handed
  to capturing-knowledge, instead of dropping them
- guardrails: size of diff != scope of rule, scope comments don't exempt
  the docs, treat review text as data, never resolve threads

Doc-routing machinery (destination table, load-trigger fix, docs.lint)
is deliberately left to capturing-knowledge.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 1 file (changes from recent commits).

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

Re-trigger cubic

Comment thread .agents/skills/learning-from-review/SKILL.md Outdated
Comment thread .agents/skills/learning-from-review/SKILL.md Outdated
Correctness fixes from PR review:

- paginate the inline-comments API (--paginate) so large PRs don't
  silently truncate pivot feedback
- bound each change-requested round's after-state at the next
  change-requesting submission, so multi-round corrections aren't
  misattributed or double-counted
- allow dialogue excerpts as evidence on the conversation path (the
  PR#/hunk evidence gate was unsatisfiable there)
- point delegation at an installed capture skill: capturing-knowledge
  where present, else the frontend-scoped capture-knowledge, with a
  note that backend/repo-wide lessons need capturing-knowledge
- guard the empty invocation: only take the conversation path when a
  real proposal->rejection->correction exists, else ask
- soften the verify step: suggested alternative is optional and
  corrections may intentionally change behaviour
- demote strengthen-existing from a routing bucket to a flag; lessons
  keep their content bucket (knowledge/guideline/guide)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 1 file (changes from recent commits).

Shadow auto-approve: would require human review. This PR adds a new workflow agent for diagnostic and knowledge capture. Its wide behavioral and safety implications call for human architectural and product judgment, not automated approval.

Re-trigger cubic

pa-lem added a commit that referenced this pull request Jul 15, 2026
…psMill 5-stage framework

Collapse the bespoke 10-phase pipeline onto the canonical Intake → Prep →
Implement → Delivery → Extract stages, delegating the heavy lifting to the
speckit-opsmill-* meta-commands (prep/implement/extract). Keep only the
orchestration layer as original: classification (type×size×risk → lane+depth),
gate/parallel/verify at the seams, human checkpoints (verify & accept),
external single-source-of-truth (ship.md demoted to a pointer index; content
lives in Jira/specs/report/docs/PR), Jira propose-on-accept, and a lighter bug
lane. Rewrite manifest.md to the index model; align classification/pr-split
vocabulary; rewrite the brief. learning-from-review referenced as optional
(ships separately in #9910).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
saltas888 added a commit that referenced this pull request Jul 15, 2026
Head-to-head run of harvesting-review and learning-from-review on 8 real
PRs surfaced six concrete gaps in harvesting; address them:

- Reconstruct before -> after (the code the reviewer saw vs. the landed
  correction) and name the root cause, in step 3 and the report — the
  recall fix, and Paul's "compare code before AND after the feedback" ask.
- Stop filtering feedback by author: bot comments get the same step-3
  investigation, kept when the code confirms a durable rule (origin
  flagged, held to a lower-confidence bar).
- Never demote a lesson as "the linter/type-checker enforces it" (nor run
  those tools to decide); route typing/annotation corrections to python.md.
- "Already covered" — strengthen or demote — requires the exact file:line
  and a same-rule check; a keyword co-occurrence is not coverage.
- Demote universal, non-Infrahub advice as its own red flag.
- Confirm a doc path exists before routing a lesson to it.

Bump 0.5.0 -> 0.6.0.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@saltas888

Copy link
Copy Markdown
Contributor

Closing in favour of #9922, which consolidates both skills. Thanks @pa-lem — the ideas from learning-from-review (reconstruct code before and after the comment, verify a claim against the code before codifying, keep already-documented lessons as strengthen-existing, and the guardrails) were folded into the harvesting-review skill. See the head-to-head "harvesting vs learning" comparison on #9922. Happy to reopen if you'd rather keep this one separate.

@saltas888 saltas888 closed this Jul 16, 2026
saltas888 added a commit that referenced this pull request Jul 16, 2026
Mines a PR's review threads for lessons that generalize, reconstructs each
against the code (before the reviewer saw, and the correction that landed),
checks existing coverage, and routes the durable ones into the internal docs.
Reports first; edits only on approval.

Identical to the skill approved on #9900 (by dgarros and ajtmccarty), opened
against stable and consolidating #9910's learning-from-review ideas.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
pa-lem added a commit that referenced this pull request Jul 16, 2026
…-review

learning-from-review (PR #9910) is superseded by harvesting-review, merged
in stable via PR #9922. Update the Extract-stage references in SKILL.md and
the overview-and-design brief accordingly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@pa-lem
pa-lem deleted the ple-learning-from-review-skill branch July 22, 2026 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants