-
Notifications
You must be signed in to change notification settings - Fork 971
proposal: add instruction loader and schema restructure (Slice 3) #410
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
Adds two change proposals for implementing Slice 3 of the artifact POC: 1. restructure-schema-directories - Move schemas from embedded TS objects to self-contained directories - Each schema becomes a directory with schema.yaml + templates/ - Enables co-located templates for user extensibility - 2-level resolution: user override → package built-in 2. add-instruction-loader (depends on #1) - Load templates from schema directories - Enrich templates with change context (dependencies, next steps) - Format change status for CLI output - New instruction-loader capability These proposals complete Slice 3 from docs/artifact_poc.md.
📝 WalkthroughWalkthroughAdds design, spec, and task docs for (1) an instruction-loader that loads and enriches templates from schema directories with change context, and (2) restructuring built-in schemas into per-schema directories with co-located templates and override resolution. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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 |
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: 1
🧹 Nitpick comments (2)
openspec/changes/add-instruction-loader/tasks.md (1)
25-29: Minor capitalization: "markdown" → "Markdown".Line 27 should capitalize "Markdown" as it's a proper noun for the markup language, matching technical writing conventions and improving consistency across documentation.
openspec/changes/add-instruction-loader/design.md (1)
15-18: Optional: Tighten phrasing for clarity.Line 17 could be more concise: "This proposal adds template loading and instruction enrichment on top of that structure" could be shortened to something like "This proposal adds template loading and instruction enrichment to that structure" for tighter phrasing.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
openspec/changes/add-instruction-loader/design.mdopenspec/changes/add-instruction-loader/proposal.mdopenspec/changes/add-instruction-loader/specs/instruction-loader/spec.mdopenspec/changes/add-instruction-loader/tasks.mdopenspec/changes/restructure-schema-directories/design.mdopenspec/changes/restructure-schema-directories/proposal.mdopenspec/changes/restructure-schema-directories/specs/artifact-graph/spec.mdopenspec/changes/restructure-schema-directories/tasks.md
🧰 Additional context used
📓 Path-based instructions (4)
openspec/changes/**/*.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Scaffold proposal using
proposal.md,tasks.md, optionaldesign.md, and delta specs underopenspec/changes/<id>/
Files:
openspec/changes/add-instruction-loader/design.mdopenspec/changes/restructure-schema-directories/specs/artifact-graph/spec.mdopenspec/changes/restructure-schema-directories/proposal.mdopenspec/changes/add-instruction-loader/tasks.mdopenspec/changes/add-instruction-loader/proposal.mdopenspec/changes/restructure-schema-directories/tasks.mdopenspec/changes/add-instruction-loader/specs/instruction-loader/spec.mdopenspec/changes/restructure-schema-directories/design.md
openspec/changes/**/specs/**/spec.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
openspec/changes/**/specs/**/spec.md: Use## ADDED|MODIFIED|REMOVED|RENAMED Requirementsheaders in spec delta files
Include at least one#### Scenario:per requirement in spec delta files
Use#### Scenario: Nameformat (4 hashtags) for scenario headers, not bullets or bold text
Use## ADDED Requirementsfor new orthogonal capabilities that can stand alone; use## MODIFIED Requirementsfor behavior changes of existing requirements
When using MODIFIED Requirements, paste the full requirement block including header and all scenarios
Files:
openspec/changes/restructure-schema-directories/specs/artifact-graph/spec.mdopenspec/changes/add-instruction-loader/specs/instruction-loader/spec.md
openspec/changes/*/proposal.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Ensure
proposal.mdincludes sections: Why (1-2 sentences), What Changes (bullet list with breaking change markers), and Impact (affected specs and code)
Files:
openspec/changes/restructure-schema-directories/proposal.mdopenspec/changes/add-instruction-loader/proposal.md
openspec/changes/*/tasks.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Ensure
tasks.mdcontains implementation checklist with numbered sections and checkbox items
Files:
openspec/changes/add-instruction-loader/tasks.mdopenspec/changes/restructure-schema-directories/tasks.md
🧠 Learnings (10)
📓 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>/`
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)
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
📚 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:
openspec/changes/add-instruction-loader/design.mdopenspec/changes/restructure-schema-directories/specs/artifact-graph/spec.mdopenspec/changes/restructure-schema-directories/proposal.mdopenspec/changes/add-instruction-loader/tasks.mdopenspec/changes/add-instruction-loader/proposal.mdopenspec/changes/restructure-schema-directories/tasks.mdopenspec/changes/add-instruction-loader/specs/instruction-loader/spec.mdopenspec/changes/restructure-schema-directories/design.md
📚 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:
openspec/changes/add-instruction-loader/design.mdopenspec/changes/restructure-schema-directories/tasks.mdopenspec/changes/restructure-schema-directories/design.md
📚 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:
openspec/changes/add-instruction-loader/design.mdopenspec/changes/restructure-schema-directories/specs/artifact-graph/spec.mdopenspec/changes/restructure-schema-directories/tasks.mdopenspec/changes/add-instruction-loader/specs/instruction-loader/spec.md
📚 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:
openspec/changes/restructure-schema-directories/specs/artifact-graph/spec.mdopenspec/changes/add-instruction-loader/specs/instruction-loader/spec.md
📚 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:
openspec/changes/restructure-schema-directories/specs/artifact-graph/spec.mdopenspec/changes/add-instruction-loader/specs/instruction-loader/spec.md
📚 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 : When using MODIFIED Requirements, paste the full requirement block including header and all scenarios
Applied to files:
openspec/changes/restructure-schema-directories/specs/artifact-graph/spec.mdopenspec/changes/add-instruction-loader/specs/instruction-loader/spec.md
📚 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:
openspec/changes/restructure-schema-directories/specs/artifact-graph/spec.mdopenspec/changes/add-instruction-loader/specs/instruction-loader/spec.md
📚 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:
openspec/changes/restructure-schema-directories/specs/artifact-graph/spec.md
📚 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:
openspec/changes/add-instruction-loader/tasks.mdopenspec/changes/restructure-schema-directories/tasks.mdopenspec/changes/add-instruction-loader/specs/instruction-loader/spec.md
🪛 LanguageTool
openspec/changes/add-instruction-loader/design.md
[style] ~17-~17: ‘on top of that’ might be wordy. Consider a shorter alternative.
Context: ...late loading and instruction enrichment on top of that structure. ## Goals / Non-Goals **Goa...
(EN_WORDINESS_PREMIUM_ON_TOP_OF_THAT)
openspec/changes/add-instruction-loader/tasks.md
[uncategorized] ~27-~27: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ... in instructions.ts - [ ] 4.2 Format as markdown table with status and output path - [ ]...
(MARKDOWN_NNP)
🔇 Additional comments (5)
openspec/changes/restructure-schema-directories/proposal.md (1)
1-20: Proposal structure is solid.The proposal clearly documents the restructuring with all required sections: Why, What Changes (with BREAKING marker), and Impact. The scope is well-defined.
openspec/changes/restructure-schema-directories/design.md (1)
1-128: Design is comprehensive and well-reasoned.The design doc clearly articulates goals, decisions with justifications, migration steps, and identified risks. The 2-level resolution approach and self-contained schema directories are well-justified. Packaging considerations are explicitly called out.
openspec/changes/restructure-schema-directories/tasks.md (1)
1-32: Tasks checklist is well-organized and actionable.The five sections logically sequence the work: Create directories → Update resolution → Cleanup → Package distribution → Fix template paths. Each task is clear and maps to the design decisions.
openspec/changes/add-instruction-loader/specs/instruction-loader/spec.md (1)
1-65: Spec is well-structured with clear testable scenarios.All requirements are appropriately marked ADDED, each with multiple scenarios using the #### Scenario: format. The WHEN/THEN structure is consistent and scenarios are concrete (e.g., template loading, context loading with custom schema, enrichment with dependency status).
openspec/changes/add-instruction-loader/proposal.md (1)
1-20: Proposal is complete with clear dependencies.All required sections are present: Why, What Changes, and Impact. Crucially, the dependency on restructure-schema-directories is explicitly documented at line 14, which aligns with the implementation order specified in PR objectives.
| ## MODIFIED Requirements | ||
|
|
||
| ### Requirement: Schema Loading | ||
| The system SHALL load artifact graph definitions from YAML schema files within schema directories. | ||
|
|
||
| #### Scenario: Valid schema loaded from directory | ||
| - **WHEN** a schema directory contains a valid `schema.yaml` file | ||
| - **THEN** the system returns an ArtifactGraph with all artifacts and dependencies | ||
|
|
||
| #### Scenario: Invalid schema rejected | ||
| - **WHEN** a schema YAML file is missing required fields | ||
| - **THEN** the system throws an error with a descriptive message | ||
|
|
||
| #### Scenario: Cyclic dependencies detected | ||
| - **WHEN** a schema contains cyclic artifact dependencies | ||
| - **THEN** the system throws an error listing the artifact IDs in the cycle | ||
|
|
||
| #### Scenario: Invalid dependency reference | ||
| - **WHEN** an artifact's `requires` array references a non-existent artifact ID | ||
| - **THEN** the system throws an error identifying the invalid reference | ||
|
|
||
| #### Scenario: Duplicate artifact IDs rejected | ||
| - **WHEN** a schema contains multiple artifacts with the same ID | ||
| - **THEN** the system throws an error identifying the duplicate | ||
|
|
||
| #### Scenario: Schema directory not found | ||
| - **WHEN** resolving a schema name that has no corresponding directory | ||
| - **THEN** the system throws an error listing available schemas |
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.
🧩 Analysis chain
🏁 Script executed:
fd "spec.md" openspec/changes/restructure-schema-directories/specs/artifact-graph/Repository: Fission-AI/OpenSpec
Length of output: 139
🏁 Script executed:
cat openspec/changes/restructure-schema-directories/specs/artifact-graph/spec.mdRepository: Fission-AI/OpenSpec
Length of output: 2166
🏁 Script executed:
find openspec -name "spec.md" -type f | grep artifact-graph | head -20Repository: Fission-AI/OpenSpec
Length of output: 266
🏁 Script executed:
git ls-files openspec | grep -i "artifact-graph" | grep -v "restructure-schema-directories"Repository: Fission-AI/OpenSpec
Length of output: 400
🏁 Script executed:
cat openspec/specs/artifact-graph/spec.mdRepository: Fission-AI/OpenSpec
Length of output: 4283
🏁 Script executed:
cat openspec/changes/archive/2025-12-24-add-artifact-graph-core/specs/artifact-graph/spec.mdRepository: Fission-AI/OpenSpec
Length of output: 4157
Include the full original requirement block when marking Schema Loading as MODIFIED.
Per coding guidelines, when using MODIFIED Requirements, you must paste the full prior requirement block (header and all original scenarios) before showing the modified version. The current spec shows only the final state without the original context. This allows reviewers to see what changed in the modification.
Include the original Schema Loading requirement and its scenarios, then show the modified version clearly.
🤖 Prompt for AI Agents
In openspec/changes/restructure-schema-directories/specs/artifact-graph/spec.md
around lines 1 to 28, the MODIFIED "Schema Loading" requirement only shows the
final state; per guidelines you must include the full original requirement block
(header and all original scenarios) immediately before the modified version so
reviewers can see what changed—restore/paste the complete original "Requirement:
Schema Loading" block and its scenarios verbatim, then add the modified block
(clearly separated) that contains the updated scenarios shown in the diff,
ensuring both versions are present and labeled (Original / Modified).
Update the Schema Loading requirement to include all original scenarios (modified as needed) per the MODIFIED requirement guidelines. The archiver replaces the entire requirement with the provided content.
Summary
Adds two change proposals for implementing Slice 3 of the artifact POC (
docs/artifact_poc.md):1.
restructure-schema-directories(implement first)schemas/<name>/schema.yaml+templates/2.
add-instruction-loader(depends on #1)instruction-loadercapabilityDesign decisions
resolver.ts,state.ts)Implementation order
restructure-schema-directories(18 tasks)add-instruction-loader(20 tasks)Test plan
openspec validate restructure-schema-directories --strict && openspec validate add-instruction-loader --strict🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.