feat(verify): detect semantic planning residue - #1373
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe ChangesVerify relevance workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
Updated the branch against current main and resolved the template-parity conflict in afd700c. I also regenerated the committed openspec-verify-change skill and its parity hashes so the distributed skill matches the source template. Local validation: build passes, template parity 14/14, and the full suite passes 2253/2253 with telemetry disabled. This PR is now ready for review. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/templates/workflows/verify-change.ts`:
- Around line 119-122: Update the verify-change workflow to deterministically
discover and baseline the complete implementation-file diff before auditing
relevance, then use that file set when reviewing changed code. Apply the same
discovery instructions at src/core/templates/workflows/verify-change.ts lines
119-122 and 311-314, and regenerate skills/openspec-verify-change/SKILL.md lines
117-120 with the matching guidance.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e98dae52-6848-42ee-abf6-f261f714e77e
📒 Files selected for processing (7)
docs/commands.mddocs/editing-changes.mddocs/reviewing-changes.mddocs/workflows.mdskills/openspec-verify-change/SKILL.mdsrc/core/templates/workflows/verify-change.tstest/core/templates/skill-templates-parity.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/templates/workflows/verify-change.ts`:
- Line 193: Update the verify-change workflow rule at
src/core/templates/workflows/verify-change.ts lines 193-193 so Relevance runs
whenever the implementation baseline and file set are reliable: use tasks for
task-only changes, and tasks plus specs when both exist, without requiring every
artifact such as design.md. Apply the same graceful-degradation behavior to the
OPSX command template at src/core/templates/workflows/verify-change.ts lines
395-395.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 51fd6999-fc0a-4648-a98c-a05565f8fb8a
📒 Files selected for processing (3)
skills/openspec-verify-change/SKILL.mdsrc/core/templates/workflows/verify-change.tstest/core/templates/skill-templates-parity.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- test/core/templates/skill-templates-parity.test.ts
- skills/openspec-verify-change/SKILL.md
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
skills/openspec-verify-change/SKILL.md (3)
50-58: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInclude untracked files in the added implementation inventory.
The implementation scope lists already-versioned and staged/unstaged changes, but new implementation files can exist only as untracked files. Add untracked files to the added-file set so they are included in the relevance audit.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/openspec-verify-change/SKILL.md` around lines 50 - 58, Update the implementation scope procedure to include untracked files when building the added-file inventory, alongside committed, staged, and unstaged changes. Ensure untracked implementation files are classified and included in the saved implementation file set used by the relevance audit, while preserving the existing baseline and limitation handling.
127-142: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winScope residue findings to changed elements.
The relevance audit can examine every file in the changed-file set, but current candidates include added and modified files, not every changed element. State that each unused, unreachable, placeholder, mock, duplicated, or disconnected candidate must be compared with version-control diff hunks against the baseline before reporting planning residue.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/openspec-verify-change/SKILL.md` around lines 127 - 142, Update the “Semantic Residue Audit” guidance in “Verify Relevance” so every suspected residue candidate is confirmed against the version-control diff hunks and baseline before reporting it. Limit planning-residue findings to elements added or modified by the change, while keeping the existing checks for unused, unreachable, placeholder, mock, duplicated, or disconnected elements.
50-58: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftExpose VCS scope discovery to the verifier.
Bash(openspec:*)permits no Git inspection, and OpenSpec does not inspect project Git history or diffs. Step 4 therefore cannot establish the baseline or inventory committed, staged, unstaged, untracked, and renamed files. Add a VCS/file-inventory capability or anopenspeccommand that returns this data.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/openspec-verify-change/SKILL.md` around lines 50 - 58, Update the implementation-scope step in the verifier instructions so it uses an available VCS/file-inventory capability or an openspec command to determine the baseline and enumerate committed, staged, unstaged, untracked, added, deleted, and renamed files. Require recording the discovery method and preserve the existing fallback to state the limitation and skip the relevance audit when reliable VCS data is unavailable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@skills/openspec-verify-change/SKILL.md`:
- Around line 50-58: Update the implementation scope procedure to include
untracked files when building the added-file inventory, alongside committed,
staged, and unstaged changes. Ensure untracked implementation files are
classified and included in the saved implementation file set used by the
relevance audit, while preserving the existing baseline and limitation handling.
- Around line 127-142: Update the “Semantic Residue Audit” guidance in “Verify
Relevance” so every suspected residue candidate is confirmed against the
version-control diff hunks and baseline before reporting it. Limit
planning-residue findings to elements added or modified by the change, while
keeping the existing checks for unused, unreachable, placeholder, mock,
duplicated, or disconnected elements.
- Around line 50-58: Update the implementation-scope step in the verifier
instructions so it uses an available VCS/file-inventory capability or an
openspec command to determine the baseline and enumerate committed, staged,
unstaged, untracked, added, deleted, and renamed files. Require recording the
discovery method and preserve the existing fallback to state the limitation and
skip the relevance audit when reliable VCS data is unavailable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e6e467b1-9acc-4e39-908d-a971b609cd79
📒 Files selected for processing (5)
docs/commands.mddocs/workflows.mdskills/openspec-verify-change/SKILL.mdsrc/core/templates/workflows/verify-change.tstest/core/templates/skill-templates-parity.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- test/core/templates/skill-templates-parity.test.ts
- docs/workflows.md
- docs/commands.md
- src/core/templates/workflows/verify-change.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
skills/openspec-verify-change/SKILL.md (2)
194-194: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDefine one fallback for an unresolved baseline.
Line 53 says to ask the user when the baseline is ambiguous. Line 194 says to skip relevance when the baseline cannot be identified reliably. An ambiguous baseline is an unreliable baseline, so these instructions overlap.
Specify the sequence: prompt first and skip only when the user cannot resolve the baseline, or skip immediately.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/openspec-verify-change/SKILL.md` at line 194, Update the baseline-handling instructions in the verification workflow to define one consistent fallback: prompt the user first when the implementation diff baseline or file set is ambiguous, and skip relevance only if the user cannot resolve it. Remove the conflicting instruction that permits skipping immediately for an unreliable baseline.
167-167: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep uncertain relevance findings visible as SUGGESTION.
Step 9 emits
SUGGESTION: Possibly irrelevant codewhen purpose is uncertain. The SUGGESTION section at Lines 170-173 lists only pattern inconsistencies and minor improvements. Add uncertain relevance to that list, or state that the bullets are examples. Otherwise, the downgrade in Lines 137-139 is not explicit in the report contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/openspec-verify-change/SKILL.md` at line 167, Update the SUGGESTION section in the verification report instructions to include uncertain relevance findings, specifically the “Possibly irrelevant code” result from Step 9, or explicitly state that the listed bullets are examples. Ensure the report contract clearly preserves this finding as SUGGESTION rather than requiring a downgrade.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@skills/openspec-verify-change/SKILL.md`:
- Line 194: Update the baseline-handling instructions in the verification
workflow to define one consistent fallback: prompt the user first when the
implementation diff baseline or file set is ambiguous, and skip relevance only
if the user cannot resolve it. Remove the conflicting instruction that permits
skipping immediately for an unreliable baseline.
- Line 167: Update the SUGGESTION section in the verification report
instructions to include uncertain relevance findings, specifically the “Possibly
irrelevant code” result from Step 9, or explicitly state that the listed bullets
are examples. Ensure the report contract clearly preserves this finding as
SUGGESTION rather than requiring a downgrade.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a2d1cf6f-e9b0-4d26-aac0-912603fde2dc
📒 Files selected for processing (5)
docs/commands.mddocs/workflows.mdskills/openspec-verify-change/SKILL.mdsrc/core/templates/workflows/verify-change.tstest/core/templates/skill-templates-parity.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- docs/commands.md
- test/core/templates/skill-templates-parity.test.ts
- src/core/templates/workflows/verify-change.ts
- docs/workflows.md
1 participant
What changed
openspec-verify-changeskill and/opsx:verifycommandWhy
Issue #1073 proposes an optional semantic cleanup checkpoint after implementation and before verification/archive. This draft integrates that checkpoint into the existing optional
/opsx:verifyworkflow rather than adding another command. It keeps the workflow surface small while placing the check immediately before archive.Impact
This changes generated verification instructions and documentation only. It does not automatically remove code or block archive; it reports evidence-backed warnings and suggestions for human review.
Validation
OPENSPEC_TELEMETRY=0 ./node_modules/.bin/vitest run— 100 test files, 1,876 tests passednode build.js./node_modules/.bin/eslint src/— 0 errorsgit diff --checkReview note
This is a draft specifically to confirm that extending
/opsx:verifyis preferable to introducing a separate checkpoint command.Prepared with Codex assistance and reviewed locally for scope and false-positive boundaries.
Addresses #1073
Summary by CodeRabbit
New Features
Documentation