-
Notifications
You must be signed in to change notification settings - Fork 1k
feat: enhance artifact instructions with inline guidance and XML output #422
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Schema changes: - Add `instruction` field to artifacts for inline creation guidance - Include detailed instructions for proposal, specs, design, and tasks Instruction loader enhancements: - Return `instruction` field from schema - Include `changeDir` for full path resolution - Enrich dependency info with `path` and `description` fields CLI output improvements: - New XML-style format for `openspec instructions` (better for AI parsing) - Structured tags: <artifact>, <task>, <context>, <output>, <template> - Dependencies now show full paths for easy file reading - JSON output includes all new fields
📝 WalkthroughWalkthroughAdds documentation for schema customization and workflow gaps; embeds instructional metadata into schemas; enriches artifact instruction model with Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)test/core/list.test.ts (1)
🔇 Additional comments (1)
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 |
- schema-customization.md: Guide for customizing artifact schemas - schema-workflow-gaps.md: Analysis of current workflow limitations
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/commands/artifact-workflow.ts (1)
387-389: Consider implementing success_criteria validation.Lines 387-389 include a placeholder comment "To be defined in schema validation rules" for success criteria. This suggests planned future work.
Would you like me to open an issue to track the implementation of schema validation rules and success criteria? This could include:
- Defining validation rules in artifact schemas
- Implementing runtime validation
- Providing clear success/failure feedback in the CLI
docs/schema-workflow-gaps.md (1)
131-147: Add language specifiers to code blocks for better rendering.Several code blocks lack language specifiers, which can affect syntax highlighting and rendering in some Markdown viewers.
🔎 Proposed fix to add language specifiers
-``` +```plaintext openspec/ ├── config.yaml # Project config (NEW)-``` +```plaintext 1. ./openspec/schemas/<name>/ # Project-local-``` +```plaintext 1. --schema CLI flag # Explicit override-``` +```plaintext 1. ./openspec/schemas/<name>/ # Project-local (NEW)Also applies to: 172-177, 182-187, 311-315
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
docs/schema-customization.mddocs/schema-workflow-gaps.mdschemas/spec-driven/schema.yamlschemas/tdd/schema.yamlsrc/commands/artifact-workflow.tssrc/core/artifact-graph/index.tssrc/core/artifact-graph/instruction-loader.tssrc/core/artifact-graph/types.tstest/commands/artifact-workflow.test.ts
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/*.md : Scaffold proposal using `proposal.md`, `tasks.md`, optional `design.md`, and delta specs under `openspec/changes/<id>/`
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/*.md : Scaffold proposal using `proposal.md`, `tasks.md`, optional `design.md`, and delta specs under `openspec/changes/<id>/`
Applied to files:
docs/schema-customization.mddocs/schema-workflow-gaps.mdschemas/spec-driven/schema.yamlschemas/tdd/schema.yaml
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Create `design.md` only when needed: cross-cutting changes, new external dependencies, significant data model changes, security/performance complexity, or pre-coding ambiguity
Applied to files:
docs/schema-workflow-gaps.mdschemas/spec-driven/schema.yamlschemas/tdd/schema.yaml
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/*/proposal.md : Ensure `proposal.md` includes sections: Why (1-2 sentences), What Changes (bullet list with breaking change markers), and Impact (affected specs and code)
Applied to files:
docs/schema-workflow-gaps.mdschemas/spec-driven/schema.yamlschemas/tdd/schema.yaml
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Use `## ADDED|MODIFIED|REMOVED|RENAMED Requirements` headers in spec delta files
Applied to files:
schemas/spec-driven/schema.yamlschemas/tdd/schema.yaml
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/*/tasks.md : Ensure `tasks.md` contains implementation checklist with numbered sections and checkbox items
Applied to files:
schemas/spec-driven/schema.yamlschemas/tdd/schema.yaml
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Include at least one `#### Scenario:` per requirement in spec delta files
Applied to files:
schemas/spec-driven/schema.yamlschemas/tdd/schema.yaml
📚 Learning: 2025-11-25T01:08:02.839Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:02.839Z
Learning: Use `@/openspec/AGENTS.md` to learn how to create and apply change proposals, spec format and conventions, and project structure and guidelines
Applied to files:
schemas/spec-driven/schema.yaml
📚 Learning: 2025-11-25T01:08:19.004Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Applies to openspec/changes/**/specs/**/spec.md : Use `## ADDED Requirements` for new orthogonal capabilities that can stand alone; use `## MODIFIED Requirements` for behavior changes of existing requirements
Applied to files:
schemas/tdd/schema.yaml
🧬 Code graph analysis (1)
src/core/artifact-graph/instruction-loader.ts (1)
src/core/artifact-graph/types.ts (2)
Artifact(22-22)CompletedSet(28-28)
🪛 LanguageTool
docs/schema-customization.md
[style] ~93-~93: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ... — requires manual copy and edit 2. "I want to customize just the proposal template" ...
(REP_WANT_TO_VB)
[style] ~94-~94: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...late"* — must copy entire schema 3. *"I want to see what the default schema looks like"...
(REP_WANT_TO_VB)
[style] ~95-~95: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ... like"* — must find package path 4. "I want to revert to defaults" — must delete file...
(REP_WANT_TO_VB)
docs/schema-workflow-gaps.md
[style] ~61-~61: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...2: Customizing a Schema Goal: User wants to add a "research" artifact before "propo...
(REP_WANT_TO_VB)
[style] ~305-~305: ‘first priority’ might be wordy. Consider a shorter alternative.
Context: ...openspec/schemas/to resolution order (first priority) -openspec schema copy [new-na...
(EN_WORDINESS_PREMIUM_FIRST_PRIORITY)
🪛 markdownlint-cli2 (0.18.1)
docs/schema-workflow-gaps.md
131-131: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
172-172: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
182-182: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
311-311: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (26)
schemas/spec-driven/schema.yaml (5)
9-22: LGTM! Instruction guidance aligns with established conventions.The proposal instruction block correctly documents the Why/What Changes/Impact structure and emphasizes keeping implementation details in design.md. This matches the learnings for proposal.md structure.
Based on learnings, the proposal structure and guidance are consistent with project conventions.
29-74: Comprehensive spec guidance with correct delta operations.The instruction block properly documents ADDED/MODIFIED/REMOVED/RENAMED operations and the requirement for at least one scenario per requirement. The MODIFIED workflow guidance (lines 47-54) is particularly helpful.
Based on learnings, the delta operations and scenario format align with established conventions.
82-102: Clear guidance on when and how to create design documents.The instruction appropriately emphasizes creating design.md only when needed (lines 85-89) and provides comprehensive section guidance. This aligns with the principle that not all changes require upfront design documentation.
Based on learnings, the design.md criteria match established guidelines for when design documents are beneficial.
110-133: Well-structured task breakdown guidance.The instruction provides clear guidelines for organizing tasks with numbered sections and checkboxes, matching the established convention for tasks.md structure.
Based on learnings, the tasks structure follows the expected implementation checklist format.
44-44: Error message is not "silent"—validation already catches incorrect scenario formatting and reports it.The warning about "fail silently" is misleading. When scenarios use 3 hashtags or bullets instead of 4, the validator's
countScenarios()method finds zero scenarios (which matches only^####) and produces an explicit ERROR: "must include at least one scenario." However, the error message is indirect—it doesn't explicitly state that the issue is an incorrect hashtag count. Consider improving the error message to directly report: "Scenarios must use exactly 4 hashtags (####), not 3. Found:### Scenario:format" to make the diagnosis clearer for developers.Likely an incorrect or invalid review comment.
src/core/artifact-graph/types.ts (1)
9-9: LGTM! Clean schema extension using Zod v4.The addition of the optional
instructionfield toArtifactSchemais straightforward and follows Zod v4 conventions correctly.test/commands/artifact-workflow.test.ts (3)
262-264: Test expectations correctly updated for XML-like output format.The test expectations now match the new structured output format with opening tags for artifacts and templates.
274-276: Consistent XML-style formatting in design artifact test.Test expectations properly updated to match the structured output.
287-288: Blocked artifact warning now uses structured tags.The update from plain text warning to
<warning>withstatus="missing"aligns with the new structured output approach.docs/schema-customization.md (1)
1-210: Comprehensive documentation of schema customization workflow.This documentation effectively captures the current manual process, identifies pain points, and proposes a well-thought-out CLI solution. The 2-level resolution model (user override → package built-in) is clearly explained, and the proposed
openspec schemacommands address real user needs.The implementation considerations section (lines 165-196) provides valuable technical context for future development.
src/commands/artifact-workflow.ts (4)
315-327: Clean destructuring of enhanced instruction fields.The function now accepts the new
changeDirandinstructionfields fromArtifactInstructions, aligning with the enriched data model.
329-340: Well-structured opening and warning sections.The XML-like opening tag includes helpful context attributes (id, change, schema), and the blocked warning clearly identifies missing dependencies.
350-364: Rich dependency context with full paths and descriptions.The context section now includes:
- Full file paths (line 356)
- Descriptions for each dependency (line 359)
- Status indicators (done/missing)
This provides significantly more context than the previous implementation.
373-378: Optional instruction block handles missing instructions gracefully.The conditional rendering ensures the
<instruction>block only appears when instruction content is available.schemas/tdd/schema.yaml (4)
9-43: Clear specification guidance for TDD workflow.The spec instruction appropriately focuses on WHAT to build with testable requirements and scenarios. The warning on line 21 about using exactly 4 hashtags for scenarios is consistent with the spec-driven schema.
50-94: Excellent TDD test-first guidance.The tests instruction emphasizes writing tests BEFORE implementation (line 51) and provides comprehensive coverage requirements (lines 63-68). The Given/When/Then structure (line 59) aligns well with the spec scenarios.
102-140: Implementation guidance follows TDD principles.The implementation instruction correctly describes the red-green-refactor cycle (lines 105-110) and emphasizes minimal code to pass tests. The warning "Don't over-engineer - implement only what tests require" (line 140) is excellent advice for TDD.
148-204: User-focused documentation guidance.The docs instruction appropriately emphasizes writing for users (line 158) and provides a clear structure with Overview, Getting Started, Examples, and Reference sections.
docs/schema-workflow-gaps.md (3)
1-126: Thorough analysis of current workflow gaps.The document effectively captures three key pain points:
- Schema not persisted with changes (lines 32-56)
- Manual schema customization process (lines 60-95)
- No team-sharing mechanism (lines 98-112)
Each scenario includes concrete examples showing the user experience problems.
129-188: Well-designed 3-level resolution architecture.The proposed architecture with:
- Project-local schemas (
.openspec/schemas/)- Change metadata (
change.yaml)- Project config (
config.yaml)...is a sensible progression that addresses all identified gaps. The resolution order (lines 172-176) and schema selection order (lines 182-187) are clearly defined.
273-363: Pragmatic phased implementation plan.The 4-phase approach prioritizes high-value features:
- Phase 1 (High): Change metadata - solves the "forgot --schema" problem
- Phase 2 (High): Project-local schemas - enables team sharing
- Phase 3 (Medium): Schema management CLI - improves DX
- Phase 4 (Low): Project config - adds convenience
The backwards compatibility section (lines 354-363) demonstrates careful consideration of migration concerns.
src/core/artifact-graph/index.ts (1)
39-39: No action needed. The export is currently namedDependencyInfo, and there is no evidence of a previousDependencyStatusexport in the codebase or git history. No external consumers are affected, and no breaking change has occurred.Likely an incorrect or invalid review comment.
src/core/artifact-graph/instruction-loader.ts (4)
47-58: LGTM!The interface additions are well-documented and appropriately typed. Using
string | undefinedforinstructioncorrectly reflects that it's an optional schema field.
64-75: LGTM!The enriched
DependencyInfointerface provides useful metadata (pathanddescription) that will help consumers understand dependencies better. The field names and types are appropriate.
186-201: LGTM!The function correctly populates the new
changeDirandinstructionfields, and uses the updatedgetDependencyInfohelper. The implementation aligns with the interface changes.
207-221: Defensive fallbacks are appropriate—schema validation guarantees safety.The
validateRequiresReferences()function inschema.tsvalidates at load time that all artifact IDs referenced in therequiresfield exist in the schema. This validation throwsSchemaValidationErrorif any reference is invalid, ensuringdepArtifactwill never be undefined at runtime. The optional chaining fallbacks (depArtifact?.generates ?? idanddepArtifact?.description ?? '') are sound defensive coding and will never silently mask configuration errors.
Update test to explicitly use sort='name' since the default changed from alphabetical to most-recently-modified.
Summary
instructionfield to schema artifacts for inline creation guidanceopenspec instructionsoutput with XML-style format for better AI parsingSchema Changes
Added detailed
instructionfields to artifacts in bothspec-drivenandtddschemas:proposal: Guidelines for Why/What Changes/Impact sectionsspecs: Delta operations format, requirement/scenario structuredesign: Technical decisions, architecture documentationtasks: Task breakdown from specsInstruction Loader
instructionfield from schemachangeDirfor full path resolutionDependencyInfowithpathanddescriptionCLI Output (XML Format)
New structured output for
openspec instructions:Test plan
instructionfieldsopenspec instructionsshows XML formatopenspec instructions --jsonincludes new fields🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.