feat: Add feedback command and lessons.md integration for continuous improvement#1
feat: Add feedback command and lessons.md integration for continuous improvement#1buddhisthead wants to merge 3 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughAdds a new /speckit.feedback command and templates, cross-platform scripts to fetch PR metadata/reviews/discussions, README updates (Lessons Learned, constitution path), and inserts a Load Lessons Context step into specify/plan/implement templates to persist and load per-PR and central lesson files under memory/. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as "/speckit.feedback"
participant GH as "GitHub API"
participant FS as "Local FS (memory/)"
User->>CLI: invoke (--pr / --current / --recent / manual)
CLI->>GH: query PR metadata, review comments, discussion comments
GH-->>CLI: return pr, reviews, discussion JSON
CLI->>CLI: extract & deduplicate lessons
CLI->>FS: write `memory/feedback/pr-<n>-lessons.md` and update `memory/lessons.md`
CLI-->>User: display summary/prompts and optional commit steps
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In @.github/agents/speckit.implement.agent.md:
- Line 94: Update the C ignore pattern list in the line containing "- **C**:
`build/`, `bin/`, `obj/`, `out/`, `*.o`, `*.a`, `*.so`, `*.exe`, `Makefile`,
`config.log`, `.idea/`, `*.log`, `.env*`" by removing `Makefile` (it is
typically a tracked source file) and, if desired, separate or document
`config.log` as a generated/autoconf artifact rather than treating it the same
as source ignores; leave other generated/build artifacts (`*.o`, `*.a`,
`build/`, etc.) as-is.
In @.github/agents/speckit.plan.agent.md:
- Line 42: The text in the plan step referencing "Phase 2 planning" is
inconsistent with the defined Phases (only Phase 0 and Phase 1); update the
sentence in the step that currently reads "Stop and report: Command ends after
Phase 2 planning" to reference the correct phase (e.g., "Phase 1 planning") or
expand the Phases section to include a Phase 2 if that was intended; adjust the
phrase in the .github/agents/speckit.plan.agent.md file so the step and the
Phases section are consistent (look for the exact string "Phase 2 planning" to
locate and edit).
In @.github/agents/speckit.specify.agent.md:
- Around line 63-66: The PowerShell example is confusing because it shows
PowerShell-style flags (-Json, -Number, -ShortName) while referencing the Bash
script create-new-feature.sh; update the documentation around the
create-new-feature.sh invocation to either remove the PowerShell example or
replace it with a valid PowerShell/Windows workflow: either (A) clarify that the
PowerShell example requires running the Bash script inside WSL/Cygwin and show
the correct invocation (e.g., call bash with the script and keep POSIX flags),
or (B) provide a true PowerShell script equivalent (e.g.,
create-new-feature.ps1) and show the -Json/-Number/-ShortName usage, and
explicitly mention which environment each example targets.
- Around line 152-160: In the "Handle Validation Results" subsection (the "c.
**Handle Validation Results**" block) change the phrase "If all items pass: Mark
checklist complete and proceed to step 6" to "If all items pass: Mark checklist
complete and proceed to step 7" so the step reference points to the subsequent
"Report completion" step rather than repeating the validation step; update only
that text in the spec.
In `@scripts/bash/fetch-pr-feedback.sh`:
- Around line 155-165: The jq invocation can get empty strings for
review_comments or discussion_comments which makes --argjson fail; before
calling jq (in the block that builds the combined JSON) ensure review_comments
and discussion_comments are normalized to either valid JSON or the literal
string "null" when they are empty by checking the variables (review_comments and
discussion_comments) and replacing empty/blank values with "null" (or a valid
JSON value) so that --argjson receives proper JSON values; update the code
around the jq call that uses --argjson --argjson review_comments
"${review_comments:-null}" and --argjson --argjson discussion_comments
"${discussion_comments:-null}" to perform this validation/substitution first.
- Around line 101-133: The fetch_review_comments function can fail silently
because gh api graphql writes nothing on error; update fetch_review_comments so
that when the gh api graphql call fails (the branch that currently triggers
warn), it outputs a valid JSON token (e.g., "null" or "{}") to stdout before
returning, instead of only calling warn, so callers using jq --argjson won't
break; modify the failure path of the gh api graphql invocation (inside
fetch_review_comments) to echo a JSON placeholder and then call warn/return, and
apply the same pattern to fetch_discussion_comments to keep behavior consistent.
In `@templates/commands/feedback.md`:
- Around line 244-247: In the "## Lesson Entry Format" section, find the
lowercase occurrence of the word "markdown" in the sentence "Each lesson in the
central database uses a markdown heading..." and change it to "Markdown"
(capitalize the M) so the term is properly capitalized; update the phrase in
that paragraph near the mention of the fenced YAML metadata block.
- Around line 52-64: In templates/commands/feedback.md the template references
the wrong script directory for fetch-pr-feedback.sh (uses the
.specify/scripts/bash/ prefix); update all occurrences of the referenced script
path so they point to the actual script location
(scripts/bash/fetch-pr-feedback.sh) wherever the filename fetch-pr-feedback.sh
is used (lines showing Mode B and the Fetch PR Data step) to ensure the commands
run correctly.
🧹 Nitpick comments (3)
.github/agents/speckit.feedback.agent.md (1)
244-264: Minor: Capitalize "Markdown" as a proper noun.Per the static analysis hint, "markdown" should be capitalized to "Markdown" when referring to the formatting language.
Proposed fix
-Each lesson in the central database uses a markdown heading with a fenced YAML metadata block: +Each lesson in the central database uses a Markdown heading with a fenced YAML metadata block:.github/agents/speckit.specify.agent.md (1)
186-190: Minor: Capitalize "Markdown" as a proper noun.Per the static analysis hint, "markdown" should be "Markdown" when referring to the formatting language.
Proposed fix
- 4. **CRITICAL - Table Formatting**: Ensure markdown tables are properly formatted: + 4. **CRITICAL - Table Formatting**: Ensure Markdown tables are properly formatted:.github/agents/speckit.implement.agent.md (1)
119-124: Minor: Awkward phrasing in step 7.Line 121 reads "If you need to write tests for contracts..." which is grammatically incomplete given the context. Consider rephrasing for clarity.
Suggested fix
- **Setup first**: Initialize project structure, dependencies, configuration - - **Tests before code**: If you need to write tests for contracts, entities, and integration scenarios + - **Tests before code**: Write tests for contracts, entities, and integration scenarios before implementing them - **Core development**: Implement models, services, CLI commands, endpoints
| ## Lesson Entry Format | ||
|
|
||
| Each lesson in the central database uses a markdown heading with a fenced YAML metadata block: | ||
|
|
There was a problem hiding this comment.
Capitalize “Markdown.”
Small docs polish.
✏️ Suggested tweak
-Each lesson in the central database uses a markdown heading with a fenced YAML metadata block:
+Each lesson in the central database uses a Markdown heading with a fenced YAML metadata block:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## Lesson Entry Format | |
| Each lesson in the central database uses a markdown heading with a fenced YAML metadata block: | |
| ## Lesson Entry Format | |
| Each lesson in the central database uses a Markdown heading with a fenced YAML metadata block: | |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~246-~246: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...h lesson in the central database uses a markdown heading with a fenced YAML metadata blo...
(MARKDOWN_NNP)
🤖 Prompt for AI Agents
In `@templates/commands/feedback.md` around lines 244 - 247, In the "## Lesson
Entry Format" section, find the lowercase occurrence of the word "markdown" in
the sentence "Each lesson in the central database uses a markdown heading..."
and change it to "Markdown" (capitalize the M) so the term is properly
capitalized; update the phrase in that paragraph near the mention of the fenced
YAML metadata block.
There was a problem hiding this comment.
Pull request overview
Adds a /speckit.feedback workflow intended to extract lessons from PR reviews into a shared docs/feedback/lessons.md, and wires “load lessons context” steps into core Spec Kit command templates/agents.
Changes:
- Introduces a new
/speckit.feedbackcommand template + Copilot agent/prompt and a bash script to fetch PR feedback viagh/GraphQL. - Updates
/speckit.specify,/speckit.plan, and/speckit.implementtemplates (and Copilot agents) to load/filter lessons fromdocs/feedback/lessons.md. - Updates README and
specify initoutput to mention/speckit.feedback.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/commands/specify.md | Adds “Load Lessons Context” step for architecture/documentation lessons. |
| templates/commands/plan.md | Adds “Load Lessons Context” step for architecture/testing/performance lessons. |
| templates/commands/implement.md | Adds “Load Lessons Context” step for code-quality/security/testing lessons. |
| templates/commands/feedback.md | New feedback command template (currently has packaging/PS-variant issues and formatting issues). |
| scripts/bash/fetch-pr-feedback.sh | New bash helper to fetch PR metadata/reviews/discussion via gh + jq. |
| src/specify_cli/init.py | Adds /speckit.feedback to the “Next Steps” output. |
| README.md | Documents /speckit.feedback in the optional commands table. |
| .github/prompts/speckit.feedback.prompt.md | Adds Copilot prompt file for the new agent. |
| .github/agents/speckit.specify.agent.md | Copilot agent definition updated to include lessons-loading guidance. |
| .github/agents/speckit.plan.agent.md | Copilot agent definition updated to include lessons-loading guidance. |
| .github/agents/speckit.implement.agent.md | Copilot agent definition updated to include lessons-loading guidance. |
| .github/agents/speckit.feedback.agent.md | New Copilot agent definition for feedback workflow (also has formatting/docs issues). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
templates/commands/feedback.md
Outdated
| ### Step 2: Ensure Directory Structure | ||
|
|
||
| Check if `docs/feedback/` directory exists. If not, create it with: | ||
| - `docs/feedback/lessons.md` - Central lessons database (use template from data-model) |
There was a problem hiding this comment.
The text says “use template from data-model” / “format from data-model.md”, but there’s no data-model template to reference in this repo. Consider pointing to the “Lesson Entry Format” section below (or a concrete file path) so the agent has an unambiguous source of truth.
| - `docs/feedback/lessons.md` - Central lessons database (use template from data-model) | |
| - `docs/feedback/lessons.md` - Central lessons database (see the "Lesson Entry Format" section below for the expected structure) |
| ```markdown | ||
| ### L### | ||
|
|
||
| ```yaml | ||
| lesson_id: L### |
There was a problem hiding this comment.
The “Lesson Entry Format” example uses nested triple-backtick fences (markdown containing yaml), which will break Markdown rendering. Use a single fence (e.g., omit the outer ```markdown), or switch the outer fence to tildes (~~~) / indent the inner block.
| ### Step 2: Ensure Directory Structure | ||
|
|
||
| Check if `docs/feedback/` directory exists. If not, create it with: | ||
| - `docs/feedback/lessons.md` - Central lessons database (use template from data-model) |
There was a problem hiding this comment.
This agent doc also references “use template from data-model” / “format from data-model.md”, but there isn’t a corresponding template file in this repo. Point to a specific section/file so the instructions are actionable.
| - `docs/feedback/lessons.md` - Central lessons database (use template from data-model) | |
| - `docs/feedback/lessons.md` - Central lessons database stored as a markdown table with columns: ID, PR, Category, Lesson, Tags, Date |
| ```markdown | ||
| ### L### | ||
|
|
||
| ```yaml | ||
| lesson_id: L### |
There was a problem hiding this comment.
The “Lesson Entry Format” example nests triple-backtick fences (markdown containing yaml), which breaks Markdown rendering in most viewers. Use a single fence or different fence markers (e.g., ~~~) to avoid nesting.
buddhisthead
force-pushed
the
add-feedback-command-with-lessons-integration
branch
4 times, most recently
from
061e4b2 to
cd8892a
Compare
buddhisthead
force-pushed
the
add-feedback-command-with-lessons-integration
branch
from
cd8892a to
c652f21
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 3. **Fetch PR Data**: | ||
| - Run `scripts/bash/fetch-pr-feedback.sh <pr_number>` | ||
| - Parse the JSON output containing: |
There was a problem hiding this comment.
The command instructions reference .specify/scripts/bash/fetch-pr-feedback.sh, but this PR adds the script at scripts/bash/fetch-pr-feedback.sh and other command templates reference scripts via scripts/.... This mismatch will cause the feedback workflow to fail to locate the script; update the referenced path(s) to the actual script location (and keep it consistent across templates).
templates/commands/feedback.md
Outdated
|
|
||
| **If `--commit` flag is present**: | ||
|
|
||
| First, check if there are any uncommitted changes to .specify/memory/ lesson files: |
There was a problem hiding this comment.
There’s an extra space in the path (.specify/memory/ lesson files), which makes the example harder to copy/paste and looks like a typo. Remove the stray space so the path is consistent.
| First, check if there are any uncommitted changes to .specify/memory/ lesson files: | |
| First, check if there are any uncommitted changes to .specify/memory/lesson files: |
| 0. **Load Lessons Context** (if available): | ||
| - Check if `.specify/memory/lessons.md` exists in the repository | ||
| - If it exists, read the file and filter for lessons with categories: `architecture`, `documentation` | ||
| - Keep relevant lessons in mind when writing the specification to avoid repeating past mistakes |
There was a problem hiding this comment.
The new lessons path uses .specify/memory/lessons.md, but this template (and others) refer to the constitution via /memory/constitution.md. Mixing /memory/... and .specify/memory/... in the same command is inconsistent and may break in environments where only one of these paths is available. Consider using the same /memory/... convention here (or otherwise standardize all memory paths).
| 0. **Load Lessons Context** (if available): | ||
| - Check if `.specify/memory/lessons.md` exists in the repository | ||
| - If it exists, read the file and filter for lessons with categories: `architecture`, `testing`, `performance` | ||
| - Keep relevant lessons in mind when creating the technical plan to avoid repeating past mistakes | ||
| - You may reference applied lessons in the plan (e.g., "Per L003: ...") |
There was a problem hiding this comment.
The new lessons path uses .specify/memory/lessons.md, but this template also references the constitution via /memory/constitution.md later. Mixing /memory/... and .specify/memory/... in the same command is inconsistent and may break in environments where only one of these paths is available. Consider using the same /memory/... convention here (or otherwise standardize all memory paths).
| 0. **Load Lessons Context** (if available): | ||
| - Check if `.specify/memory/lessons.md` exists in the repository | ||
| - If it exists, read the file and filter for lessons with categories: `code-quality`, `security`, `testing` | ||
| - Keep relevant lessons in mind during implementation to avoid repeating past mistakes | ||
| - Apply lessons proactively (e.g., if L007 says "check CLI dependencies", do that in your code) |
There was a problem hiding this comment.
This template introduces .specify/memory/lessons.md as the location for lessons, while other templates use /memory/... to access shared memory artifacts (e.g., the constitution). To avoid path confusion and potential missing-file issues, standardize on one convention (preferably the existing /memory/... pattern) for lessons as well.
| ### Lessons Learned | ||
|
|
||
| Spec-kit implements a continuous improvement system that captures lessons from PR reviews and applies them to future development work. The `/speckit.feedback` command extracts insights from code review comments and stores them in two places: | ||
|
|
||
| - **`.specify/memory/lessons.md`** - Central lessons database organized by category (architecture, code-quality, testing, security, performance, documentation) | ||
| - **`.specify/memory/feedback/pr-<number>-lessons.md`** - Per-PR detailed feedback summaries that preserve the full context of each review | ||
|
|
||
| When you run `/speckit.specify`, `/speckit.plan`, or `/speckit.implement`, these commands automatically load relevant lessons from the central database for their phase and apply them proactively. This creates a compounding knowledge effect where your team's collective wisdom from past reviews continuously improves future implementations, helping avoid repeated mistakes and reinforcing best practices without manual intervention. |
There was a problem hiding this comment.
The PR description mentions lessons being stored under docs/feedback/lessons.md, but the documentation here (and templates) refer to .specify/memory/lessons.md. Please align the documentation/PR description so users don’t create lessons in the wrong location.
buddhisthead
force-pushed
the
add-feedback-command-with-lessons-integration
branch
from
c652f21 to
66b004c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| **Mode A - Specific PR**: If `--pr <number>` is present, use that PR number. | ||
|
|
||
| **Mode B - Auto-detect PR**: If no `--pr` flag but no inline text, find the open PR for the current branch (or most recently merged if no open PR exists). |
There was a problem hiding this comment.
Mode B is described as falling back to the "most recently merged" PR if no open PR exists, but fetch-pr-feedback.sh --find-current only falls back to a merged PR for the current branch and errors otherwise. Update the instructions to match the script behavior, or change the script to implement the documented global fallback.
| **Mode B - Auto-detect PR**: If no `--pr` flag but no inline text, find the open PR for the current branch (or most recently merged if no open PR exists). | |
| **Mode B - Auto-detect PR**: If no `--pr` flag but no inline text, find the open PR for the current branch. If no open PR exists for this branch, fall back to the most recently merged PR for the same branch (and error if no such PR exists). |
|
|
||
| # Get repository info | ||
| get_repo_info() { | ||
| gh repo view --json owner,name -q '"\(.owner.login)/\(.name)"' 2>/dev/null || error "Not in a GitHub repository." |
There was a problem hiding this comment.
get_repo_info wraps the interpolated owner/repo in extra quotes ("..."), which can produce a string containing quotes (e.g., "owner/repo") and break the later owner="${repo_info%/*}" / repo="${repo_info#*/}" split. Return the raw owner/repo without embedded quotes (or output JSON and parse with jq) so GraphQL parameters are correct.
| gh repo view --json owner,name -q '"\(.owner.login)/\(.name)"' 2>/dev/null || error "Not in a GitHub repository." | |
| gh repo view --json owner,name -q '.owner.login + "/" + .name' 2>/dev/null || error "Not in a GitHub repository." |
templates/commands/plan.md
Outdated
| ## Outline | ||
|
|
||
| 0. **Load Lessons Context** (if available): | ||
| - Check if `.specify/memory/lessons.md` exists in the repository |
There was a problem hiding this comment.
The new lessons path uses .specify/memory/lessons.md, but this template later references the constitution at /memory/constitution.md (line 37). This mixes two different memory-root conventions in the same command; standardize the lessons path to match the constitution path (or vice versa) to prevent agents looking in the wrong location.
| - Check if `.specify/memory/lessons.md` exists in the repository | |
| - Check if `/memory/lessons.md` exists in the repository |
| prompt: Create a technical plan for the specification | ||
| scripts: | ||
| sh: scripts/bash/fetch-pr-feedback.sh | ||
| ps: scripts/powershell/fetch-pr-feedback.ps1 |
There was a problem hiding this comment.
The frontmatter references a PowerShell script scripts/powershell/fetch-pr-feedback.ps1, but that file does not exist in the repository. Either add the PowerShell implementation or remove the ps: entry to avoid broken command execution on Windows.
| ps: scripts/powershell/fetch-pr-feedback.ps1 |
buddhisthead
force-pushed
the
add-feedback-command-with-lessons-integration
branch
from
74c2239 to
278c0ff
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@README.md`:
- Around line 278-283: Update the "Lessons Learned" section where the categories
are listed (the paragraph referencing memory/lessons.md) to include the missing
`other` category; specifically modify the bulleted description that enumerates
categories (architecture, code-quality, testing, security, performance,
documentation) to append `other` so it matches the feedback template and
supported categories.
In `@scripts/powershell/fetch-pr-feedback.ps1`:
- Around line 69-76: The Get-RepoInfo function currently pipes gh repo view into
ConvertFrom-Json before checking $LASTEXITCODE; change it to first capture the
raw output (e.g., $output = gh repo view --json owner,name), then check
$LASTEXITCODE and call Write-Error-Message/return if non-zero, and only then
call ConvertFrom-Json on the captured $output to build and return
"$($repoInfo.owner.login)/$($repoInfo.name)". Ensure you update references in
Get-RepoInfo so error handling occurs before JSON parsing.
In `@templates/commands/feedback.md`:
- Around line 91-102: The template's reference to "data-model.md" is ambiguous;
update the instructions in templates/commands/feedback.md (the bullet under file
creation) to point to the local "Lesson format" section within this document
instead of an external file. Replace the sentence "Add each lesson in the format
from data-model.md" with something like "Add each lesson using the 'Lesson
format' described in the 'Lesson format' section below in this template" so
readers know where to find the schema without needing an external file.
- Around line 55-58: The docs instruct users to run `{SCRIPT} --find-current` in
Mode B but the PowerShell helper exposes the parameter as `-FindCurrent`,
causing Windows users to fail; update the text in templates/commands/feedback.md
to mention the PowerShell-appropriate flag (e.g., reference both
`--find-current` and `-FindCurrent` or note use of `-FindCurrent` when running
the PowerShell `{SCRIPT}`) so Mode B works on Windows shells (search for the
"Get PR Number" section and the `--find-current` occurrence to locate the
wording to change).
- Around line 44-50: The doc assumes the memory/ directory exists; update the
feedback flow in templates/commands/feedback.md Step 2 to first check for and
create the memory/ directory if missing (before touching constitution.md or
creating memory/feedback/), then create memory/feedback/ and per-PR files like
memory/feedback/pr-<number>-lessons.md and memory/lessons.md as described;
ensure the creation order and presence checks are explicit so running feedback
prior to /speckit.constitution won’t fail.
🧹 Nitpick comments (1)
scripts/powershell/fetch-pr-feedback.ps1 (1)
149-195: Consider pagination to avoid truncating review data.Line 162 caps reviews at 100 and Line 168 caps comments at 50; large PRs will drop feedback. Consider adding pagination (e.g.,
pageInfo+ loop orgh api graphql --paginate) to keep extraction complete.
| ### Lessons Learned | ||
|
|
||
| Spec-kit implements a continuous improvement system that captures lessons from PR reviews and applies them to future development work. The `/speckit.feedback` command extracts insights from code review comments and stores them in two places: | ||
|
|
||
| - **`memory/lessons.md`** - Central lessons database organized by category (architecture, code-quality, testing, security, performance, documentation) | ||
| - **`memory/feedback/pr-<number>-lessons.md`** - Per-PR detailed feedback summaries that preserve the full context of each review |
There was a problem hiding this comment.
Include the “other” category in the list.
Line 282 lists categories but omits other, which is supported in the feedback template.
Suggested fix
-- **`memory/lessons.md`** - Central lessons database organized by category (architecture, code-quality, testing, security, performance, documentation)
+- **`memory/lessons.md`** - Central lessons database organized by category (architecture, code-quality, testing, security, performance, documentation, other)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### Lessons Learned | |
| Spec-kit implements a continuous improvement system that captures lessons from PR reviews and applies them to future development work. The `/speckit.feedback` command extracts insights from code review comments and stores them in two places: | |
| - **`memory/lessons.md`** - Central lessons database organized by category (architecture, code-quality, testing, security, performance, documentation) | |
| - **`memory/feedback/pr-<number>-lessons.md`** - Per-PR detailed feedback summaries that preserve the full context of each review | |
| ### Lessons Learned | |
| Spec-kit implements a continuous improvement system that captures lessons from PR reviews and applies them to future development work. The `/speckit.feedback` command extracts insights from code review comments and stores them in two places: | |
| - **`memory/lessons.md`** - Central lessons database organized by category (architecture, code-quality, testing, security, performance, documentation, other) | |
| - **`memory/feedback/pr-<number>-lessons.md`** - Per-PR detailed feedback summaries that preserve the full context of each review |
🤖 Prompt for AI Agents
In `@README.md` around lines 278 - 283, Update the "Lessons Learned" section where
the categories are listed (the paragraph referencing memory/lessons.md) to
include the missing `other` category; specifically modify the bulleted
description that enumerates categories (architecture, code-quality, testing,
security, performance, documentation) to append `other` so it matches the
feedback template and supported categories.
| # Get repository info | ||
| function Get-RepoInfo { | ||
| $repoInfo = gh repo view --json owner,name | ConvertFrom-Json | ||
| if ($LASTEXITCODE -ne 0) { | ||
| Write-Error-Message "Not in a GitHub repository." | ||
| } | ||
| return "$($repoInfo.owner.login)/$($repoInfo.name)" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n scripts/powershell/fetch-pr-feedback.ps1 | sed -n '65,80p'Repository: buddhisthead/spec-kit
Length of output: 678
Move $LASTEXITCODE check before JSON parsing.
Line 71 pipes gh repo view output directly to ConvertFrom-Json without first checking the exit code. If gh fails and outputs invalid JSON to stdout, ConvertFrom-Json throws an error before the check on line 72 can execute, preventing Write-Error-Message from running.
Suggested fix
- $repoInfo = gh repo view --json owner,name | ConvertFrom-Json
- if ($LASTEXITCODE -ne 0) {
- Write-Error-Message "Not in a GitHub repository."
- }
- return "$($repoInfo.owner.login)/$($repoInfo.name)"
+ $repoJson = gh repo view --json owner,name 2>&1
+ if ($LASTEXITCODE -ne 0) {
+ Write-Error-Message "Not in a GitHub repository."
+ }
+ $repoInfo = $repoJson | ConvertFrom-Json
+ return "$($repoInfo.owner.login)/$($repoInfo.name)"🤖 Prompt for AI Agents
In `@scripts/powershell/fetch-pr-feedback.ps1` around lines 69 - 76, The
Get-RepoInfo function currently pipes gh repo view into ConvertFrom-Json before
checking $LASTEXITCODE; change it to first capture the raw output (e.g., $output
= gh repo view --json owner,name), then check $LASTEXITCODE and call
Write-Error-Message/return if non-zero, and only then call ConvertFrom-Json on
the captured $output to build and return
"$($repoInfo.owner.login)/$($repoInfo.name)". Ensure you update references in
Get-RepoInfo so error handling occurs before JSON parsing.
| 1. **Get PR Number**: | ||
| - Mode A: Use the `--pr <number>` value | ||
| - Mode B: Run `{SCRIPT} --find-current` to get the open PR for current branch (falls back to most recently merged) | ||
|
|
There was a problem hiding this comment.
Use PowerShell-appropriate flag in Mode B.
Line 57 instructs --find-current, but the PowerShell script exposes -FindCurrent, so Windows usage will fail as written.
Suggested fix
- - Mode B: Run `{SCRIPT} --find-current` to get the open PR for current branch (falls back to most recently merged)
+ - Mode B (bash): Run `{SCRIPT} --find-current` to get the open PR for current branch (falls back to most recently merged)
+ - Mode B (PowerShell): Run `{SCRIPT} -FindCurrent` to get the open PR for current branch (falls back to most recently merged)🤖 Prompt for AI Agents
In `@templates/commands/feedback.md` around lines 55 - 58, The docs instruct users
to run `{SCRIPT} --find-current` in Mode B but the PowerShell helper exposes the
parameter as `-FindCurrent`, causing Windows users to fail; update the text in
templates/commands/feedback.md to mention the PowerShell-appropriate flag (e.g.,
reference both `--find-current` and `-FindCurrent` or note use of `-FindCurrent`
when running the PowerShell `{SCRIPT}`) so Mode B works on Windows shells
(search for the "Get PR Number" section and the `--find-current` occurrence to
locate the wording to change).
| - If file is new: Create `memory/feedback/pr-<number>-lessons.md` with YAML frontmatter: | ||
| ```yaml | ||
| --- | ||
| pr_number: <number> | ||
| pr_title: "<title>" | ||
| pr_url: "<url>" | ||
| extracted_date: <today's date YYYY-MM-DD> | ||
| lesson_count: <count> | ||
| --- | ||
| ``` | ||
| - Add each lesson in the format from data-model.md | ||
|
|
There was a problem hiding this comment.
Clarify the lesson-format reference.
Line 101 points to data-model.md, but this template doesn’t define where that file lives. Pointing to the format section below avoids confusion.
Suggested tweak
- - Add each lesson in the format from data-model.md
+ - Add each lesson in the format described in the "Lesson Entry Format" section below🤖 Prompt for AI Agents
In `@templates/commands/feedback.md` around lines 91 - 102, The template's
reference to "data-model.md" is ambiguous; update the instructions in
templates/commands/feedback.md (the bullet under file creation) to point to the
local "Lesson format" section within this document instead of an external file.
Replace the sentence "Add each lesson in the format from data-model.md" with
something like "Add each lesson using the 'Lesson format' described in the
'Lesson format' section below in this template" so readers know where to find
the schema without needing an external file.
…improvement
This PR introduces a comprehensive feedback loop system that captures lessons
from PR reviews and applies them to future development work.
## New Features
### 1. /speckit.feedback Command
- Extracts lessons learned from PR review comments and discussions
- Stores lessons in .specify/memory/lessons.md (alongside constitution.md)
- Stores PR-specific summaries in .specify/memory/feedback/pr-<number>-lessons.md
- Supports three modes:
- Auto-detect current PR: --find-current
- Specific PR: --pr <number>
- Manual lesson entry: inline text input
- Categorizes lessons: code-quality, architecture, testing, documentation,
security, performance, other
- Deduplicates lessons and tracks frequency
- Optional auto-commit with --commit flag
### 2. Lessons.md Integration Across Commands
All core speckit commands now load and apply relevant lessons:
- /speckit.specify loads architecture & documentation lessons
- /speckit.plan loads architecture, testing & performance lessons
- /speckit.implement loads code-quality, security & testing lessons
### 3. Supporting Infrastructure
- fetch-pr-feedback.sh script: Fetches PR data using GitHub CLI (bash)
- fetch-pr-feedback.ps1 script: PowerShell equivalent for Windows users
- Command templates for all AI agents (Claude, Gemini, Cursor, Copilot, etc.)
- Lessons stored in .specify/memory/ alongside project constitution
- PR-specific feedback preserved in .specify/memory/feedback/
- Uses {SCRIPT} placeholder for cross-platform compatibility
## Files Added
- scripts/bash/fetch-pr-feedback.sh (220 lines)
- scripts/powershell/fetch-pr-feedback.ps1 (277 lines)
- templates/commands/feedback.md (293 lines)
## Files Modified
- templates/commands/specify.md (added Step 0: Load Lessons - 6 lines)
- templates/commands/plan.md (added Step 0: Load Lessons - 6 lines)
- templates/commands/implement.md (added Step 0: Load Lessons - 6 lines)
- README.md (added Lessons Learned section explaining the system)
- src/specify_cli/__init__.py (added feedback to next steps output)
## Benefits
- Continuous Improvement: Lessons from PR reviews automatically inform future work
- Universal Compatibility: Works with all 15+ supported AI agents
- Cross-Platform: Both bash and PowerShell implementations
- Non-Breaking: Optional integration - only activates when lessons.md exists
- Category-Aware: Each command loads only relevant lessons to reduce noise
- Team Learning: Captures institutional knowledge from code reviews
- Compounding Knowledge: Team wisdom accumulates in memory alongside constitution
## How It Works
1. After PR review, run /speckit.feedback to extract lessons
2. Lessons are stored in .specify/memory/lessons.md with categories
3. PR-specific context preserved in .specify/memory/feedback/
4. Future runs of /speckit.specify, /speckit.plan, /speckit.implement
automatically load and apply relevant lessons
5. Teams avoid repeating past mistakes and compound knowledge over time
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
buddhisthead
force-pushed
the
add-feedback-command-with-lessons-integration
branch
from
278c0ff to
5c317de
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@scripts/powershell/fetch-pr-feedback.ps1`:
- Around line 116-132: The Find-CurrentPR logic pipes gh pr list directly into
ConvertFrom-Json which can fail silently if gh returns an error; change both gh
calls (the open and merged fallbacks) to first capture the raw output into a
variable (e.g., $rawOutput), then check $LASTEXITCODE (and handle a non-zero
exit by logging/warning and returning $null or failing), and only call
ConvertFrom-Json on $rawOutput when $LASTEXITCODE is 0; update references to
$prList, $prNumber, and the two gh pr list invocations in Find-CurrentPR
accordingly so error paths are handled before parsing.
- Around line 85-100: The Find-RecentMergedPR logic calls gh pr list and pipes
directly into ConvertFrom-Json (the invocations that populate $prList) without
checking $LASTEXITCODE; mirror the Get-RepoInfo fix by capturing the output of
gh pr list into a variable first, check $LASTEXITCODE and handle non-zero (log
an error and exit or return $null) before calling ConvertFrom-Json, and apply
this for both the --head $currentBranch call that sets $prList/$prNumber and the
fallback --limit 10 call that populates $sorted; reference the gh pr list
invocations, the $prList variable, and the $prNumber assignment when making the
change.
🧹 Nitpick comments (1)
templates/commands/feedback.md (1)
195-209: Consider handling push failures gracefully.The
--commitworkflow assumesgit pushwill succeed. If the remote is unavailable or the user lacks push permissions, the script will fail without clear recovery guidance.Suggested enhancement
git push origin <current-branch>+If push fails (e.g., network issues or permissions), the commit is still local. The user can retry with:
+bash +git push origin <current-branch> +
+
Use appropriate commit message:</details> </blockquote></details> </blockquote></details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| # First try to find a merged PR for the current branch | ||
| $prList = gh pr list --state merged --head $currentBranch --json number --limit 1 | ConvertFrom-Json | ||
| $prNumber = $null | ||
|
|
||
| if ($prList -and $prList.Count -gt 0) { | ||
| $prNumber = $prList[0].number | ||
| } | ||
|
|
||
| if (-not $prNumber) { | ||
| # Fallback: get the most recently merged PR in the repo | ||
| $prList = gh pr list --state merged --json number,mergedAt --limit 10 | ConvertFrom-Json | ||
| if ($prList -and $prList.Count -gt 0) { | ||
| $sorted = $prList | Sort-Object -Property mergedAt -Descending | ||
| $prNumber = $sorted[0].number | ||
| } | ||
| } |
There was a problem hiding this comment.
Same $LASTEXITCODE pattern issue in Find-RecentMergedPR.
Lines 86 and 95 pipe gh pr list output directly to ConvertFrom-Json without first checking the exit code. Apply the same fix pattern as suggested for Get-RepoInfo.
Suggested fix for line 86
- $prList = gh pr list --state merged --head $currentBranch --json number --limit 1 | ConvertFrom-Json
+ $prJson = gh pr list --state merged --head $currentBranch --json number --limit 1 2>&1
+ if ($LASTEXITCODE -ne 0) {
+ $prList = @()
+ } else {
+ $prList = $prJson | ConvertFrom-Json
+ }🤖 Prompt for AI Agents
In `@scripts/powershell/fetch-pr-feedback.ps1` around lines 85 - 100, The
Find-RecentMergedPR logic calls gh pr list and pipes directly into
ConvertFrom-Json (the invocations that populate $prList) without checking
$LASTEXITCODE; mirror the Get-RepoInfo fix by capturing the output of gh pr list
into a variable first, check $LASTEXITCODE and handle non-zero (log an error and
exit or return $null) before calling ConvertFrom-Json, and apply this for both
the --head $currentBranch call that sets $prList/$prNumber and the fallback
--limit 10 call that populates $sorted; reference the gh pr list invocations,
the $prList variable, and the $prNumber assignment when making the change.
| # First try to find an open PR for the current branch | ||
| $prList = gh pr list --state open --head $currentBranch --json number --limit 1 | ConvertFrom-Json | ||
| $prNumber = $null | ||
|
|
||
| if ($prList -and $prList.Count -gt 0) { | ||
| $prNumber = $prList[0].number | ||
| return $prNumber | ||
| } | ||
|
|
||
| # Fallback: try to find a merged PR for the current branch | ||
| $prList = gh pr list --state merged --head $currentBranch --json number --limit 1 | ConvertFrom-Json | ||
|
|
||
| if ($prList -and $prList.Count -gt 0) { | ||
| $prNumber = $prList[0].number | ||
| Write-Warning-Message "No open PR found, using most recently merged PR for this branch." | ||
| return $prNumber | ||
| } |
There was a problem hiding this comment.
Same $LASTEXITCODE pattern issue in Find-CurrentPR.
Lines 117 and 126 pipe gh pr list output directly to ConvertFrom-Json. Consider capturing output first, then checking exit code before parsing.
🤖 Prompt for AI Agents
In `@scripts/powershell/fetch-pr-feedback.ps1` around lines 116 - 132, The
Find-CurrentPR logic pipes gh pr list directly into ConvertFrom-Json which can
fail silently if gh returns an error; change both gh calls (the open and merged
fallbacks) to first capture the raw output into a variable (e.g., $rawOutput),
then check $LASTEXITCODE (and handle a non-zero exit by logging/warning and
returning $null or failing), and only call ConvertFrom-Json on $rawOutput when
$LASTEXITCODE is 0; update references to $prList, $prNumber, and the two gh pr
list invocations in Find-CurrentPR accordingly so error paths are handled before
parsing.
Testing Spec-Kit with Feedback CommandI've added a new InstallationInstall spec-kit from my fork using uv: uv tool install git+https://github.com/buddhisthead/spec-kit.git@v0.0.91
Initialize a New Project
Create a new project with your preferred AI agent:
specify init --ai copilot my-project
cd my-project
Replace copilot with your AI agent: claude, gemini, cursor, windsurf, etc.
Add Spec-Kit to an Existing Project
Navigate to your existing project and add spec-kit:
cd your-existing-project
specify init --ai copilot --here --force
This will merge spec-kit files into your project without destroying existing content.
Add a Second AI Agent (Optional)
If you want to use multiple AI agents in the same project:
cd my-project
specify init --ai claude --here --force
This adds Claude Code commands alongside your existing Copilot setup without destroying anything.
What's New
The /speckit.feedback command:
- Extracts lessons from PR review comments and discussions
- Stores them in .specify/memory/lessons.md (alongside your constitution)
- Automatically loads relevant lessons into /speckit.specify, /speckit.plan, and /speckit.implement
- Creates a continuous improvement loop where team knowledge compounds over time
Testing
Try running /speckit.feedback on an existing PR to see it extract lessons, or test any of the core commands to verify they work with your setup. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@README.md`:
- Line 414: Update the README.md text at the sentence referencing
memory/constitution.md so it matches the repo layout: either clarify that
"memory/constitution.md" is relative to the .specify directory, or replace the
path with the full ".specify/memory/constitution.md" string to be consistent
with the directory tree and docs/upgrade.md; adjust the sentence that currently
mentions memory/constitution.md to explicitly use
".specify/memory/constitution.md" (or add a short parenthetical note that the
path is relative to .specify) so readers aren't confused.
🧹 Nitpick comments (4)
README.md (4)
278-285: Clarify the path placeholder notation.The path format
memory/feedback/pr-<number>-lessons.mduses angle brackets that might be interpreted as literal characters rather than a placeholder. Consider adding clarifying text.📝 Suggested clarification
-- **`memory/feedback/pr-<number>-lessons.md`** - Per-PR detailed feedback summaries that preserve the full context of each review +- **`memory/feedback/pr-<number>-lessons.md`** - Per-PR detailed feedback summaries that preserve the full context of each review (where `<number>` is the PR number, e.g., `pr-42-lessons.md`)
278-285: Consider adding privacy and security guidance for the feedback extraction workflow.The Lessons Learned section describes extracting content from PR review comments but doesn't mention safeguards for sensitive information. Consider adding a note about filtering out secrets, credentials, or PII during the extraction process to prevent accidental storage of sensitive data in lesson files.
🛡️ Suggested addition
Add a note after the bulleted list:
When you run `/speckit.specify`, `/speckit.plan`, or `/speckit.implement`, these commands automatically load relevant lessons from the central database for their phase and apply them proactively. This creates a compounding knowledge effect where your team's collective wisdom from past reviews continuously improves future implementations, helping avoid repeated mistakes and reinforcing best practices without manual intervention. > **Note:** When extracting lessons from PR reviews, ensure that sensitive information such as credentials, API keys, or personally identifiable information (PII) is not included in the generated lesson files.
444-447: Consider using "GitHub issue" for clarity.The example uses the informal abbreviation "GH issue
#123". While commonly understood, using the full "GitHub issue#123" would be more explicit and consistent with professional documentation.✍️ Suggested refinement
-For GH issue `#123`, Develop BugZapper, a new fuzzy test framework". +For GitHub issue `#123`, Develop BugZapper, a new fuzzy test framework".
276-276: Consider using "before" instead of "prior to" for conciseness.The phrase "prior to using" could be simplified to "before using" for more concise documentation.
✍️ Suggested simplification
-**Must be set in the context of the agent you're working with prior to using `/speckit.plan` or follow-up commands. +**Must be set in the context of the agent you're working with before using `/speckit.plan` or follow-up commands.
| ``` | ||
|
|
||
| This step creates or updates the `.specify/memory/constitution.md` file with your project's foundational guidelines that the AI agent will reference during specification, planning, and implementation phases. | ||
| This step creates or updates the `memory/constitution.md` file with your project's foundational guidelines that the AI agent will reference during specification, planning, and implementation phases. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd . && wc -l README.mdRepository: buddhisthead/spec-kit
Length of output: 78
🏁 Script executed:
# Read the relevant sections of README.md
sed -n '410,420p' README.mdRepository: buddhisthead/spec-kit
Length of output: 797
🏁 Script executed:
# Read the directory tree section
sed -n '458,474p' README.mdRepository: buddhisthead/spec-kit
Length of output: 458
🏁 Script executed:
# Search for any references to constitution.md in the repository
rg 'constitution\.md' -B 2 -A 2Repository: buddhisthead/spec-kit
Length of output: 8908
🏁 Script executed:
# Search for .specify directory references
rg '\.specify' README.mdRepository: buddhisthead/spec-kit
Length of output: 1441
Clarify the constitution file path to match the directory structure.
Line 414 references the path as memory/constitution.md, but the directory tree shown in lines 458-474 explicitly displays .specify/memory/constitution.md. The docs/upgrade.md guide also consistently uses the full .specify/memory/constitution.md path. Either clarify the working directory context (if memory/constitution.md is relative to .specify/) or update line 414 to use the full path .specify/memory/constitution.md for consistency with the directory structure shown on the same page.
🤖 Prompt for AI Agents
In `@README.md` at line 414, Update the README.md text at the sentence referencing
memory/constitution.md so it matches the repo layout: either clarify that
"memory/constitution.md" is relative to the .specify directory, or replace the
path with the full ".specify/memory/constitution.md" string to be consistent
with the directory tree and docs/upgrade.md; adjust the sentence that currently
mentions memory/constitution.md to explicitly use
".specify/memory/constitution.md" (or add a short parenthetical note that the
path is relative to .specify) so readers aren't confused.
Overview
This PR introduces a comprehensive feedback loop system that captures lessons from PR reviews and applies them to future development work, creating a continuous improvement cycle for teams using spec-kit.
What's New
🎯 New
/speckit.feedbackCommandA powerful command that extracts and stores lessons learned from PR reviews:
--find-currentfinds the open PR for your current branch--pr <number>extracts lessons from any PR--commitflag to automatically commit lesson files🔄 Lessons Integration Across Core Commands
All main spec-kit commands now load and apply relevant lessons automatically:
/speckit.specify/speckit.plan/speckit.implementHow it works:
memory/lessons.mdexists🛠️ Supporting Infrastructure
fetch-pr-feedback.sh: Bash script that fetches PR data using GitHub CLI (gh)Files Added
New Agent Files
.github/agents/speckit.feedback.agent.md(new feedback agent).github/agents/speckit.specify.agent.md(updated with lessons).github/agents/speckit.plan.agent.md(updated with lessons).github/agents/speckit.implement.agent.md(updated with lessons).github/prompts/speckit.feedback.prompt.mdNew Scripts
scripts/bash/fetch-pr-feedback.sh(fetches PR data from GitHub)New Templates
templates/commands/feedback.md(command template for all agents)Files Modified
Command Templates
templates/commands/specify.md(added Step 0: Load Lessons Context)templates/commands/plan.md(added Step 0: Load Lessons Context)templates/commands/implement.md(added Step 0: Load Lessons Context)Documentation & Configuration
README.md(added/speckit.feedbackto optional commands table)src/specify_cli/__init__.py(added feedback command to init output)Benefits
✅ Continuous Improvement: Lessons from PR reviews automatically inform future work
✅ Universal Compatibility: Works with all 15+ supported AI agents (Claude, Copilot, Gemini, Cursor, etc.)
✅ Non-Breaking: Optional integration - only activates when
lessons.mdexists✅ Category-Aware: Each command loads only relevant lessons to reduce noise
✅ Team Learning: Captures institutional knowledge from code reviews
✅ Incremental: Can run feedback multiple times to keep lessons up to date
How It Works - End to End
During Development: Write code as usual using
/speckit.specify,/speckit.plan,/speckit.implementDuring Review: Reviewers provide feedback on the PR
After Review: Run
/speckit.feedback(or/speckit.feedback --commit) to extract lessonsFuture Work: Next time you run
/speckit.specify,/speckit.plan, or/speckit.implement,the commands automatically load relevant lessons and apply them proactively
Knowledge Compounds: Over time, your team builds a database of lessons that prevents
repeating the same mistakes
Example Workflow
Testing
Dependencies
gh(GitHub CLI) for PR data fetchingjqfor JSON parsingBreaking Changes
None. This is purely additive functionality that's backward compatible.
🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com
Summary by CodeRabbit
New Features
Workflow Integration
Documentation