feat: add instruction loader for template loading and change context #414
Conversation
|
Warning Rate limit exceeded@TabishB has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 17 minutes and 2 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA new instruction-loader module is introduced to the artifact-graph core, providing utilities for loading templates, constructing change contexts from project state, generating enriched artifact instructions with dependency metadata, and formatting comprehensive change status reports. Public APIs are re-exported from the main index. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant SchemaResolver
participant FileSystem
participant ArtifactGraph
participant InstructionLoader
rect rgb(230, 245, 250)
Note over Caller,InstructionLoader: Load Change Context
Caller->>InstructionLoader: loadChangeContext(projectRoot, changeName, schemaName?)
InstructionLoader->>SchemaResolver: resolve schema
InstructionLoader->>ArtifactGraph: create graph from manifest
InstructionLoader->>FileSystem: detect completed artifacts (proposal.md)
InstructionLoader-->>Caller: ChangeContext
end
rect rgb(240, 250, 240)
Note over Caller,InstructionLoader: Generate Instructions
Caller->>InstructionLoader: generateInstructions(context, artifactId)
InstructionLoader->>FileSystem: loadTemplate(schemaName, templatePath)
InstructionLoader->>ArtifactGraph: compute dependency status
InstructionLoader->>ArtifactGraph: get unlocked artifacts
InstructionLoader-->>Caller: ArtifactInstructions
end
rect rgb(250, 240, 240)
Note over Caller,InstructionLoader: Format Change Status
Caller->>InstructionLoader: formatChangeStatus(context)
loop for each artifact
InstructionLoader->>ArtifactGraph: determine status (ready/blocked/done)
end
InstructionLoader->>ArtifactGraph: sort by build order
InstructionLoader-->>Caller: ChangeStatus
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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. |
Add the instruction-loader module that provides: - loadTemplate: Load templates from schema directories - loadChangeContext: Combine artifact graph with completion state - generateInstructions: Enrich templates with change-specific context - formatChangeStatus: Format change status as readable output This is Slice 3 of the artifact-graph system, building on the graph operations (Slice 1) and change creation utilities (Slice 2).
TabishB
force-pushed
the
feat/add-instruction-loader
branch
from
9aab4bd to
bb97257
Compare
![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
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
openspec/changes/add-instruction-loader/specs/instruction-loader/spec.mdopenspec/changes/add-instruction-loader/tasks.mdsrc/core/artifact-graph/index.tssrc/core/artifact-graph/instruction-loader.tstest/core/artifact-graph/instruction-loader.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/artifact-graph/index.ts
🧰 Additional context used
📓 Path-based instructions (3)
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/tasks.mdopenspec/changes/add-instruction-loader/specs/instruction-loader/spec.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.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/add-instruction-loader/specs/instruction-loader/spec.md
🧠 Learnings (8)
📚 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.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/**/*.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/tasks.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/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|MODIFIED|REMOVED|RENAMED Requirements` headers in spec delta files
Applied to files:
openspec/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/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/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/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 `#### Scenario: Name` format (4 hashtags) for scenario headers, not bullets or bold text
Applied to files:
openspec/changes/add-instruction-loader/specs/instruction-loader/spec.md
🧬 Code graph analysis (1)
test/core/artifact-graph/instruction-loader.test.ts (1)
src/core/artifact-graph/instruction-loader.ts (5)
loadTemplate(105-132)TemplateLoadError(11-19)loadChangeContext(142-159)generateInstructions(169-192)formatChangeStatus(228-269)
🔇 Additional comments (9)
openspec/changes/add-instruction-loader/tasks.md (1)
1-13: LGTM! Well-structured task checklist.The tasks.md file properly follows the coding guidelines with a clear implementation checklist using numbered sections and checkbox items. All tasks are concrete, verifiable, and align well with the actual implementation delivered in this PR.
test/core/artifact-graph/instruction-loader.test.ts (1)
1-264: LGTM! Excellent test coverage.The test suite is comprehensive and well-structured, covering all public APIs with 21 test cases across happy paths, error scenarios, and edge cases. The use of temporary directories with proper setup/teardown is a good practice. The tests validate:
- Template loading from schemas with proper error handling
- Change context construction with schema resolution and completion detection
- Instruction generation with metadata, dependencies, and unlock information
- Status formatting with correct artifact state transitions and build ordering
src/core/artifact-graph/instruction-loader.ts (7)
8-19: LGTM! Well-designed error class.The
TemplateLoadErrorclass properly extendsErrorand includes thetemplatePathcontext, which is valuable for debugging template loading issues. Setting thenameproperty enables better error identification in logs and error handlers.
21-95: LGTM! Clear and well-documented type definitions.The interfaces are comprehensive and well-structured:
ChangeContexteffectively combines graph state with metadataArtifactInstructionsprovides all necessary context for artifact creationArtifactStatususes a discriminated union pattern correctly withmissingDepsoptional for non-blocked states- All types have clear JSDoc documentation
97-132: LGTM! Robust template loading with proper error handling.The function correctly validates the schema and template existence before reading, and wraps all errors in the custom
TemplateLoadErrorwith appropriate context. The use of synchronous file operations is appropriate for this template-loading use case.
134-159: LGTM! Clean context loading implementation.The function effectively composes the schema resolver, graph builder, and completion detector to create a comprehensive
ChangeContext. The default schema parameter of'spec-driven'is a sensible choice for the primary use case.
161-192: LGTM! Well-composed instruction generator.The function properly validates artifact existence with a contextual error message, then composes the template loader and helper functions to build comprehensive
ArtifactInstructionsincluding all metadata, dependencies, and unlock information.
194-220: LGTM! Clean helper functions.Both helpers are well-implemented:
getDependencyStatuscorrectly maps dependencies with completion statusgetUnlockedArtifactsfinds reverse dependencies and sorts for consistencyThe O(n) iteration in
getUnlockedArtifactsis appropriate for typical artifact graph sizes.
222-269: LGTM! Correct status formatting logic.The function properly determines artifact states (done/ready/blocked) using graph methods and correctly:
- Includes
missingDepsonly for blocked artifacts- Sorts results by build order for consistency
- Computes overall completion status via
graph.isComplete()
| ## Purpose | ||
| Load templates from schema directories and enrich them with change-specific context for guiding artifact creation. | ||
|
|
||
| ## Requirements |
There was a problem hiding this comment.
Use ## ADDED Requirements header per coding guidelines.
The spec delta file should use ## ADDED Requirements instead of ## Requirements. Since this introduces new orthogonal capabilities (template loading, context loading, instruction generation, status formatting) that can stand alone, the ADDED header is appropriate.
As per coding guidelines, spec delta files must use ## ADDED|MODIFIED|REMOVED|RENAMED Requirements headers.
🔎 Proposed fix
-## Requirements
+## ADDED Requirements📝 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.
| ## Requirements | |
| ## ADDED Requirements |
🤖 Prompt for AI Agents
In openspec/changes/add-instruction-loader/specs/instruction-loader/spec.md
around line 6, the header currently reads "## Requirements" but per spec-delta
guidelines it must be one of the delta types; update the header to "## ADDED
Requirements" (exact casing and spacing) so the file uses the required spec
delta header for newly introduced capabilities.
- Move change to archive as 2025-12-28-add-instruction-loader - Create instruction-loader spec with 4 requirements
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
openspec/changes/archive/2025-12-28-add-instruction-loader/design.mdopenspec/changes/archive/2025-12-28-add-instruction-loader/proposal.mdopenspec/changes/archive/2025-12-28-add-instruction-loader/specs/instruction-loader/spec.mdopenspec/changes/archive/2025-12-28-add-instruction-loader/tasks.mdopenspec/specs/instruction-loader/spec.md
🧰 Additional context used
📓 Path-based instructions (3)
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/archive/2025-12-28-add-instruction-loader/tasks.mdopenspec/changes/archive/2025-12-28-add-instruction-loader/specs/instruction-loader/spec.md
openspec/specs/**/spec.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Use SHALL/MUST for normative requirements in spec files; avoid should/may unless intentionally non-normative
Files:
openspec/specs/instruction-loader/spec.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/archive/2025-12-28-add-instruction-loader/specs/instruction-loader/spec.md
🧠 Learnings (8)
📚 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/archive/2025-12-28-add-instruction-loader/tasks.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/**/*.md : Scaffold proposal using `proposal.md`, `tasks.md`, optional `design.md`, and delta specs under `openspec/changes/<id>/`
Applied to files:
openspec/changes/archive/2025-12-28-add-instruction-loader/tasks.mdopenspec/specs/instruction-loader/spec.mdopenspec/changes/archive/2025-12-28-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/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/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/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/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/*/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/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 `#### Scenario: Name` format (4 hashtags) for scenario headers, not bullets or bold text
Applied to files:
openspec/specs/instruction-loader/spec.md
🔇 Additional comments (2)
openspec/changes/archive/2025-12-28-add-instruction-loader/specs/instruction-loader/spec.md (1)
1-70: Spec delta structure is compliant and well-organized.The archived spec delta correctly uses
## ADDED Requirementsheader, follows 4-hashtag scenario naming conventions, and includes multiple scenarios per requirement with normative "SHALL" statements. The content aligns well with the implementation functions described in the PR summary.openspec/changes/archive/2025-12-28-add-instruction-loader/tasks.md (1)
1-13: Tasks checklist is complete and well-scoped.All 9 implementation tasks are marked complete, covering spec creation, function implementations, public API exports, comprehensive test coverage, and build verification. The checklist aligns with PR objectives.
Summary
instruction-loadermodule providing template loading and change context enrichmentloadTemplate,loadChangeContext,generateInstructions,formatChangeStatusTest plan
openspec validate add-instruction-loader🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.