-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat(architecture): add architecture.md template and auto-merge on archive #512
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
base: main
Are you sure you want to change the base?
feat(architecture): add architecture.md template and auto-merge on archive #512
Conversation
📝 WalkthroughWalkthroughAdds architecture documentation (openspec/architecture.md and template), implements extraction and merging of architectural decisions from design.md into architecture.md during archival, integrates this into the archive workflow, and updates templates, initialization prompts, and template generation to support the architecture lifecycle. Changes
Sequence DiagramsequenceDiagram
participant Archive as Archive Workflow
participant Design as design.md
participant Apply as architecture-apply
participant Arch as architecture.md
Archive->>Design: Read design.md
Design-->>Archive: Return design content
Archive->>Apply: hasArchitecturalImpact(content)?
Apply-->>Archive: yes/no
alt Architectural Impact Detected
Archive->>Apply: extractArchitecturalDecision(content, changeName, date)
Apply-->>Archive: ArchitecturalDecision
Archive->>Arch: Read architecture.md
Arch-->>Archive: Current architecture content
Archive->>Apply: appendArchitecturalDecision(currentContent, decision)
Apply-->>Archive: Updated architecture.md content
Archive->>Arch: Write updated architecture.md
Arch-->>Archive: Write confirmed
Archive->>Archive: Log architecture update
else No Architectural Impact
Archive->>Archive: Skip architecture update
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 markdownlint-cli2 (0.18.1)openspec/architecture.mdmarkdownlint-cli2 v0.18.1 (markdownlint v0.38.0) 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/archive.ts (1)
231-262: Avoid updating architecture.md before the archive succeeds.If archiving fails (existing archive or rename error), the decision is still appended, which can record a change that was never archived. Move the merge after a successful rename and read design.md from the archived location.
🛠️ Proposed fix
- // Apply architectural decisions to architecture.md if design.md has architectural impact - const archiveDate = this.getArchiveDate(); - const architectureUpdated = await applyArchitecturalDecisions( - targetPath, - changeName!, - changeDir, - archiveDate - ); - if (architectureUpdated) { - console.log('Updated architecture.md with architectural decisions.'); - } - - // Create archive directory with date prefix - const archiveName = `${archiveDate}-${changeName}`; + // Create archive directory with date prefix + const archiveDate = this.getArchiveDate(); + const archiveName = `${archiveDate}-${changeName}`; const archivePath = path.join(archiveDir, archiveName); @@ - await fs.rename(changeDir, archivePath); + await fs.rename(changeDir, archivePath); + + // Apply architectural decisions after successful archive + const architectureUpdated = await applyArchitecturalDecisions( + targetPath, + changeName!, + archivePath, + archiveDate + ); + if (architectureUpdated) { + console.log('Updated architecture.md with architectural decisions.'); + }
🤖 Fix all issues with AI agents
In `@openspec/architecture.md`:
- Around line 55-57: The table row placeholder is malformed because the header
"| Date | Decision | Rationale | Status |" expects four cells but the row
contains only the comment "<!-- Decisions are appended here during archive -->";
fix by either replacing that single-cell row with an empty four-cell row like "|
| | | |" or moving the comment outside the table (e.g., place "<!-- Decisions
are appended here during archive -->" below the table) so the Markdown table
renders correctly in openspec/architecture.md.
In `@src/core/architecture-apply.ts`:
- Around line 28-45: The pattern checking for "API" in hasArchitecturalImpact is
case-sensitive; update the architecturalPatterns array (in the
hasArchitecturalImpact function) to use a case-insensitive regex for API (e.g.,
change /\bAPI\b/ to /\bAPI\b/i or /\bapi\b/i) so occurrences like "api" or "Api"
are detected; keep the rest of the patterns unchanged.
🧹 Nitpick comments (1)
src/core/templates/architecture-template.ts (1)
62-65: Use a valid placeholder for the decisions table.The single-cell placeholder row can render as a malformed table. Consider removing it (the surrounding comments already explain auto-append behavior).
🛠️ Suggested tweak
| Date | Decision | Rationale | Status | |------|----------|-----------|--------| -| <!-- Decisions are appended here during archive --> |
Summary
Adds a global
architecture.mddocument to OpenSpec for documenting system architecture, components, data flows, and architectural decisions.Changes
New Files
Modified Files
openspec updateFeatures
Created on init/update:
architecture.mdis generated duringopenspec init(new projects) oropenspec update(existing projects)Auto-merge on archive: Architectural decisions from per-change design.md are automatically extracted and appended to the global
architecture.mddecisions tableLLM guidance: AGENTS.md now includes:
User prompts: Both init and update display helpful prompts for populating architecture.md
Template Structure
Testing
zsh-installer.test.ts(unrelated to this PR)Checklist
pnpm installpnpm run buildpnpm test(pre-existing failures only)Summary by CodeRabbit
New Features
Documentation
Workflow Updates
UX
✏️ Tip: You can customize this high-level summary in your review settings.