fix(explore): guide planning with focused discovery questions - #1017
fix(explore): guide planning with focused discovery questions#1017DanRioDev wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThis PR adds planning guidance to explore mode. The guidance uses repository evidence, dependency-ordered questions, focused prompts, grounded recommendations, conversational decision tracking, and separate write authorization. Tests, parity hashes, and a patch changeset are updated. ChangesExplore planning guidance
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The PR is merge-ready after normal checks and review; no actionable merge-blocking risk remains. A non-blocking follow-up is to explicitly include openspec/AGENTS.md in the planning scan so project-specific guidance is less likely to be missed. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The changes address issue [ Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (2 skipped: 2 unsupported.)
✨ 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: 1
🧹 Nitpick comments (1)
src/core/templates/workflows/explore.ts (1)
17-113: Extract shared explore-policy text to a single source to prevent drift.The same long guidance is maintained twice (
instructionsand slash-commandcontent). This is likely to drift and forces repetitive hash-baseline churn for text-only edits.Also applies to: 450-553
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/templates/workflows/explore.ts` around lines 17 - 113, The file duplicates the long "explore" guidance in two places (the instructions block and the slash-command `content`) which will drift; extract the shared text into a single exported constant (e.g., EXPLORE_POLICY_TEXT or explorePolicyText) and replace the inline literals used by `instructions` and the slash-command `content` with references to that constant; update any imports/usages in the same module (and the other occurrence around lines 450-553) so both places read the single source, and add a short unit or snapshot check if the project uses text-baseline tests to prevent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/core/templates/workflows/explore.ts`:
- Around line 38-44: Add "@/openspec/AGENTS.md" to the mandatory pre-question
context scan used by the explore workflow: update the list/constant that defines
required contexts (e.g., MANDATORY_PRE_QUESTION_CONTEXTS or
PRE_QUESTION_CHECKLIST) in the explore template in
src/core/templates/workflows/explore.ts so that any request mentioning
planning/proposals/spec/change/plan triggers loading/opening openspec/AGENTS.md
before prompting the user; ensure the path is included in the same scanning
order as other repo-authoritative files and adjust any helper that builds the
pre-question context bundle (e.g., collectPreQuestionContexts) to include it.
---
Nitpick comments:
In `@src/core/templates/workflows/explore.ts`:
- Around line 17-113: The file duplicates the long "explore" guidance in two
places (the instructions block and the slash-command `content`) which will
drift; extract the shared text into a single exported constant (e.g.,
EXPLORE_POLICY_TEXT or explorePolicyText) and replace the inline literals used
by `instructions` and the slash-command `content` with references to that
constant; update any imports/usages in the same module (and the other occurrence
around lines 450-553) so both places read the single source, and add a short
unit or snapshot check if the project uses text-baseline tests to prevent
regressions.
🪄 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
Run ID: 9058fa01-5adc-4502-933b-60fbc57181f3
📒 Files selected for processing (3)
.changeset/curious-explore-questions.mdsrc/core/templates/workflows/explore.tstest/core/templates/skill-templates-parity.test.ts
| Before asking the user, check whether the answer is available from: | ||
|
|
||
| - Existing OpenSpec changes, specs, proposals, designs, or tasks | ||
| - Source code, tests, docs, configs, package metadata, or recent conventions | ||
| - Existing architecture, naming, integration, UX, or API patterns | ||
|
|
||
| Do not ask repository-answerable questions. Explore first, then state what you found. |
There was a problem hiding this comment.
Include openspec/AGENTS.md in the mandatory pre-question context scan.
The new “repository-answerable” checklist omits openspec/AGENTS.md, which is the authoritative source for proposal/spec conventions during planning conversations.
Suggested patch
Before asking the user, check whether the answer is available from:
+- openspec/AGENTS.md for proposal/spec conventions and project guidance
- Existing OpenSpec changes, specs, proposals, designs, or tasks
- Source code, tests, docs, configs, package metadata, or recent conventions
- Existing architecture, naming, integration, UX, or API patternsBased on learnings: Always open @/openspec/AGENTS.md when the request mentions planning/proposals/spec/change/plan so exploration follows authoritative project conventions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/templates/workflows/explore.ts` around lines 38 - 44, Add
"@/openspec/AGENTS.md" to the mandatory pre-question context scan used by the
explore workflow: update the list/constant that defines required contexts (e.g.,
MANDATORY_PRE_QUESTION_CONTEXTS or PRE_QUESTION_CHECKLIST) in the explore
template in src/core/templates/workflows/explore.ts so that any request
mentioning planning/proposals/spec/change/plan triggers loading/opening
openspec/AGENTS.md before prompting the user; ensure the path is included in the
same scanning order as other repo-authoritative files and adjust any helper that
builds the pre-question context bundle (e.g., collectPreQuestionContexts) to
include it.
There was a problem hiding this comment.
Pull request overview
Updates the Explore workflow templates to guide users through more dependency-aware discovery when planning changes, while keeping Explore as a non-implementation mode.
Changes:
- Added “Relentless Interview Mode” guidance (decision-tree order, recommended defaults, one-question-at-a-time format, and “explore repo first” direction).
- Updated example dialogues and optional summary structure to reflect the new questioning approach.
- Refreshed template parity hashes and added a patch changeset for the package release.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/core/templates/workflows/explore.ts |
Updates Explore skill + /opsx:explore command prompt content to drive structured discovery and repo-first questioning. |
test/core/templates/skill-templates-parity.test.ts |
Updates expected hashes for modified Explore templates and generated content. |
.changeset/curious-explore-questions.md |
Adds a patch changeset describing the Explore guidance improvements. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| - Follow a script | ||
| - Ask the same questions every time | ||
| - Use a fixed questionnaire, but do maintain a live question tree for this specific problem |
|
|
||
| Why this matters: this determines whether we need CRDTs, | ||
| locking, or simple realtime events. | ||
|
|
|
|
||
| - Follow a script | ||
| - Ask the same questions every time | ||
| - Use a fixed questionnaire, but do maintain a live question tree for this specific problem | ||
| - Produce a specific artifact |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
clay-good
changed the title
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
Reviewed at 2d7af72. The planning guidance is focused and evidence-first without forcing an interview or weakening Explore and write-confirmation boundaries. Build and all 48 focused template and parity tests pass.
Status
Ready for human review. Local verification passes. Linux, macOS, and Windows CI, Security, lint/type check, release tracking, and CodeRabbit have passed. No PR approval or merge has been submitted.
What was wrong
Explore could leave planning as an unguided conversation instead of helping the user resolve decisions. Issue #1064 describes this friction.
How it was fixed
One shared planning section now guides both the skill and slash command to inspect repository evidence before asking factual questions, resolve blocking decisions before dependent details, and ask one focused question at a time. Recommendations are conditional on evidence; user intent is never invented.
The guidance keeps proposed defaults separate from accepted decisions and lets the user pause, pivot, or defer. Freeform exploration, explicit write confirmation, store selection, project context, and artifact capture behavior remain intact.
The hardening preserves the original contributor commits, resolves conflicts against current main, updates the distributed Explore skill and three parity hashes, and retains the patch changeset.
Replication / proof
initemits identical planning guidance to the Claude skill, Codex skill, and Claude command. Removing the section and runningupdate --forcerestores all three exactly.Notes
The first Windows run timed out after 10 seconds in the unchanged store test
produces byte-identical store.yaml without --remote. A rerun passed on the exact same commit; no test limits or source files were changed.Tests verify instruction contracts and generated content, not empirical model compliance. The original exhaustive interview and mandatory recommendation format were narrowed to avoid endless questioning and ambiguous consent.
The initial full run had six environment-related failures. Isolating global tool directories, using a standalone Node binary outside the npx cache, and clearing inherited shell-completion settings resolved them without repository changes. The completion failures were independently reproduced on unchanged main.
Thanks @DanRioDev for the original contribution and direction.
Closes #1064