docs: add architecture documentation and ADRs#85
Conversation
Summary of ChangesHello @synthable, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the project's documentation by introducing comprehensive architectural guides for Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive architecture documentation and Architecture Decision Records (ADRs) to support the project's development and maintenance. The documentation covers both core packages (ums-lib and copilot-instructions-cli) and establishes formal decision records for key architectural choices.
- Added architecture documentation for both main packages with clear component models and data flows
- Created 3 ADRs documenting key decisions around library loading, TypeScript execution, and field naming conventions
- Added research on TypeScript module execution patterns to guide future development
- Updated package descriptions and README files to reflect UMS v2.0 specification
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ums-lib/package.json | Updated package description to reflect UMS v2.0 |
| packages/ums-lib/README.md | Updated all references from UMS v1.0 to v2.0 and revised type names |
| docs/architecture/ | Added comprehensive architecture documentation for both packages |
| docs/architecture/adr/ | Added 3 ADRs documenting key architectural decisions |
| docs/research/ | Added research document on TypeScript module execution patterns |
| docs/processes/ | Removed outdated UMS v1.0 evaluation process documentation |
| README.md | Updated to reflect monorepo structure and UMS v2.0 changes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| - `UMSModule`, `UMSPersona`, `ModuleBody`, `ModuleMeta`, `ModuleGroup` | ||
| All UMS v2.0 interfaces are exported, including: | ||
|
|
||
| - `Module`, `Persona`, `Component`, `ModuleMetadata`, `ModuleGroup` |
There was a problem hiding this comment.
The type name Component listed here doesn't align with the UMS v2.0 specification context. Based on the documentation patterns, this should likely be ModuleBody or similar to match the actual UMS module structure.
| - `Module`, `Persona`, `Component`, `ModuleMetadata`, `ModuleGroup` | |
| - `Module`, `Persona`, `ModuleBody`, `ModuleMetadata`, `ModuleGroup` |
|
|
||
| ## 1. Introduction | ||
|
|
||
| The `ums-lib` package is a reusable, platform-agnostic library that provides a pure functional implementation of the Unified Module System (UMS) v1.0 specification. It serves as the core engine for parsing, validating, resolving, and rendering modular AI instructions. |
There was a problem hiding this comment.
References UMS v1.0 specification but should reference UMS v2.0 to be consistent with the package description update.
|
|
||
| ## 1. Introduction | ||
|
|
||
| The `copilot-instructions-cli` is a command-line interface for composing, managing, and building modular AI assistant instructions using the Unified Module System (UMS) v1.0. It serves as the primary user-facing tool for interacting with the UMS ecosystem. |
There was a problem hiding this comment.
References UMS v1.0 but should reference UMS v2.0 to align with the package updates and overall project migration.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a significant amount of architecture documentation and ADRs, which is a great step towards improving project clarity and maintainability. The migration to UMS v2.0 and TypeScript is well-reflected in the updated README.md and ums-lib package. However, many of the new documentation files contain inconsistencies with this migration, such as references to the old UMS v1.0 specification, outdated .yml file extensions, and old type names. Additionally, the main docs/README.md appears to have several broken links. Addressing these inconsistencies will ensure the new documentation is accurate and useful for developers.
docs/architecture/copilot-instructions-cli/04-core-utilities.md
Outdated
Show resolved
Hide resolved
|
|
||
| * **`UMSError`**: The base class for all custom errors in the library. It includes a `code` and an optional `context` property. | ||
|
|
||
| * **`UMSValidationError`**: Thrown when a module or persona fails to validate against the UMS v1.0 specification. It includes an optional `path` to the invalid field and a `section` reference to the UMS specification. |
There was a problem hiding this comment.
This document refers to UMS v1.0, but the library has been updated to support UMS v2.0. Please update this reference to v2.0 to ensure the architecture documentation is accurate.
| * **`UMSValidationError`**: Thrown when a module or persona fails to validate against the UMS v1.0 specification. It includes an optional `path` to the invalid field and a `section` reference to the UMS specification. | |
| * **`UMSValidationError`**: Thrown when a module or persona fails to validate against the UMS v2.0 specification. It includes an optional `path` to the invalid field and a `section` reference to the UMS specification. |
| # ADR 0003: Use `snippet` as Primary Field Name for Example Code | ||
|
|
||
| **Status:** Accepted | ||
| **Date:** 2025-10-12 | ||
| **Context:** UMS v2.0 Type System | ||
|
|
||
| ## Decision | ||
|
|
||
| The `Example` interface will use `snippet` as the primary field name for code examples, with `code` provided as a deprecated alias for backwards compatibility. | ||
|
|
||
| ## Context | ||
|
|
||
| During the UMS v2.0 type system implementation, we needed to decide on the field name for code examples within the `Example` interface. Two options were considered: | ||
|
|
||
| 1. **`code`** - Generic, used in many documentation systems | ||
| 2. **`snippet`** - More specific, commonly used in developer documentation, matches v1.0 | ||
|
|
||
| ## Decision Rationale | ||
|
|
||
| We chose `snippet` as the primary field name for the following reasons: | ||
|
|
||
| ### 1. Developer Familiarity | ||
| The term "snippet" is widely recognized in developer documentation contexts: | ||
| - Code snippet libraries (GitHub Gists, StackOverflow) | ||
| - IDE snippet systems (VS Code snippets, IntelliJ Live Templates) | ||
| - Documentation generators (JSDoc `@example`) | ||
|
|
||
| ### 2. Semantic Clarity | ||
| "Snippet" is more semantically specific than "code": | ||
| - **Snippet** → A small, focused example of code | ||
| - **Code** → Could mean any code (full files, modules, etc.) | ||
|
|
||
| The specificity helps users understand that examples should be concise and focused. | ||
|
|
||
| ### 3. Backwards Compatibility | ||
| UMS v1.0 used `snippet`, so maintaining this name provides: | ||
| - Zero migration effort for existing v1.0 examples | ||
| - Consistent API evolution | ||
| - Reduced confusion for existing users | ||
|
|
||
| ### 4. Naming Consistency | ||
| In the UMS module system: | ||
| - `body` → Contains module content | ||
| - `snippet` → Contains example code | ||
| - `format` → Describes data format | ||
|
|
||
| The noun-based naming pattern is more consistent than mixing verbs/nouns. | ||
|
|
||
| ## Implementation | ||
|
|
||
| ```typescript | ||
| export interface Example { | ||
| title: string; | ||
| rationale: string; | ||
| snippet: string; // Primary v2.0 field | ||
| language?: string; | ||
| code?: string; // Deprecated alias | ||
| } | ||
| ``` | ||
|
|
||
| ## Consequences | ||
|
|
||
| ### Positive | ||
| - ✅ Intuitive API for developers | ||
| - ✅ Aligns with industry conventions | ||
| - ✅ Maintains v1.0 compatibility | ||
| - ✅ Reduces test fixture migration burden | ||
|
|
||
| ### Negative | ||
| - ⚠️ Diverges from some documentation systems that use `code` | ||
| - ⚠️ Requires updating any spec documents that referenced `code` | ||
|
|
||
| ### Neutral | ||
| - The `code` field remains available as an alias, so either name works | ||
| - All existing code using `snippet` is immediately correct | ||
|
|
||
| ## Alternatives Considered | ||
|
|
||
| ### Alternative 1: Use `code` as Primary | ||
| **Rejected** because: | ||
| - Would require migrating all v1.0 examples | ||
| - Less semantically specific | ||
| - Generic naming doesn't add clarity | ||
|
|
||
| ### Alternative 2: Use Both Equally | ||
| **Rejected** because: | ||
| - Ambiguity when both are set | ||
| - Validation complexity | ||
| - Poor developer experience | ||
|
|
||
| ## Notes | ||
|
|
||
| This decision was made during the v1.0 → v2.0 backwards compatibility implementation. The rendering code (`markdown-renderer.ts`) was already using `snippet`, making this a natural choice that required minimal code changes. | ||
|
|
||
| ## References | ||
|
|
||
| - UMS v2.0 Implementation Guide: `docs/ums-v2-lib-implementation.md` | ||
| - Type Definitions: `packages/ums-lib/src/types/index.ts` | ||
| - Related: ADR 0001 (Standard Library), ADR 0002 (Dynamic TypeScript Loading) |
There was a problem hiding this comment.
The format of this ADR is inconsistent with 0001-standard-library-loading.md and 0002-dynamic-typescript-loading.md. This ADR uses Markdown headers (##) for sections like "Context" and "Decision", while the others use a simpler format with underlined section titles. For better consistency across ADRs, consider adopting a single format.
Fix all high and medium priority documentation issues identified in PR #85 review: **Broken Links (High Priority)** - Remove references to deleted USER_GUIDE.md and non-existent FAQ.md - Fix unified-module-system path reference - Update docs/README.md with correct architecture paths - Add proper navigation to architecture docs and ADRs **Version References (High Priority - 11 instances)** - Update all "UMS v1.0" references to "UMS v2.0" across architecture docs - Affects: ums-lib and CLI architecture documentation **Deprecated Type Names (High Priority - 16 instances)** - Replace UMSModule → Module - Replace UMSPersona → Persona - Update Component type description (not ModuleBody) - Affects: API specifications, component model, data flow docs **File Extensions (High Priority - 3 instances)** - Update .module.yml → .module.ts - Update .persona.yml → .persona.ts - Update YAML references to TypeScript - Affects: CLI architecture and utilities documentation **ADR Format Standardization (Medium Priority)** - Standardize all ADRs to use ## Markdown headers - Update ADR 0001 and 0002 to match ADR 0003 format - Improve readability and consistency All changes are documentation-only with no code impact. Resolves all blocking issues for PR #85.
Fix all high and medium priority documentation issues identified in PR #85 review: **Broken Links (High Priority)** - Remove references to deleted USER_GUIDE.md and non-existent FAQ.md - Fix unified-module-system path reference - Update docs/README.md with correct architecture paths - Add proper navigation to architecture docs and ADRs **Version References (High Priority - 11 instances)** - Update all "UMS v1.0" references to "UMS v2.0" across architecture docs - Affects: ums-lib and CLI architecture documentation **Deprecated Type Names (High Priority - 16 instances)** - Replace UMSModule → Module - Replace UMSPersona → Persona - Update Component type description (not ModuleBody) - Affects: API specifications, component model, data flow docs **File Extensions (High Priority - 3 instances)** - Update .module.yml → .module.ts - Update .persona.yml → .persona.ts - Update YAML references to TypeScript - Affects: CLI architecture and utilities documentation **ADR Format Standardization (Medium Priority)** - Standardize all ADRs to use ## Markdown headers - Update ADR 0001 and 0002 to match ADR 0003 format - Improve readability and consistency All changes are documentation-only with no code impact. Resolves all blocking issues for PR #85.
- Update UMS v1.0 → v2.0 reference in docs/architecture/ums-lib/01-overview.md line 8 - Update YAML → TypeScript reference in docs/architecture/ums-lib/02-component-model.md line 14 These fixes address the complete set of review comments from PR #85.
Add comprehensive architecture documentation for the project's core packages and key architectural decisions. Architecture Documentation (14 files): - docs/architecture/ums-lib/ - Core library architecture - Overview, component model, data flow - API specification and error handling - docs/architecture/copilot-instructions-cli/ - CLI architecture - Overview, command model, dependency architecture - Core utilities and patterns Architecture Decision Records (3 ADRs): - ADR-0001: Standard Library Loading - Decision to use environment-based paths - Rationale and alternatives considered - ADR-0002: Dynamic TypeScript Loading - Decision to use tsx for on-the-fly execution - Trade-offs and implementation details - ADR-0003: Example Snippet Field Naming - Decision on 'snippet' vs 'code' field naming - Consistency and clarity rationale Research Documentation: - TypeScript module execution patterns analysis - Comparison of tsx, ts-node, esbuild-register - Performance and compatibility considerations These documents provide: - Clear understanding of system architecture - Rationale for key design decisions - Guide for contributors and maintainers - Foundation for future architectural evolution
Fix all high and medium priority documentation issues identified in PR #85 review: **Broken Links (High Priority)** - Remove references to deleted USER_GUIDE.md and non-existent FAQ.md - Fix unified-module-system path reference - Update docs/README.md with correct architecture paths - Add proper navigation to architecture docs and ADRs **Version References (High Priority - 11 instances)** - Update all "UMS v1.0" references to "UMS v2.0" across architecture docs - Affects: ums-lib and CLI architecture documentation **Deprecated Type Names (High Priority - 16 instances)** - Replace UMSModule → Module - Replace UMSPersona → Persona - Update Component type description (not ModuleBody) - Affects: API specifications, component model, data flow docs **File Extensions (High Priority - 3 instances)** - Update .module.yml → .module.ts - Update .persona.yml → .persona.ts - Update YAML references to TypeScript - Affects: CLI architecture and utilities documentation **ADR Format Standardization (Medium Priority)** - Standardize all ADRs to use ## Markdown headers - Update ADR 0001 and 0002 to match ADR 0003 format - Improve readability and consistency All changes are documentation-only with no code impact. Resolves all blocking issues for PR #85.
- Update UMS v1.0 → v2.0 reference in docs/architecture/ums-lib/01-overview.md line 8 - Update YAML → TypeScript reference in docs/architecture/ums-lib/02-component-model.md line 14 These fixes address the complete set of review comments from PR #85.
synthable
force-pushed
the
feat/documentation
branch
from
dd52cc8 to
be45e89
Compare
Introduces a formal proposal process and comprehensive contribution guidelines to improve project governance and community participation. ## New Documentation ### Contribution Guidelines (CONTRIBUTING.md) - Code of conduct and community standards - Development environment setup - Ways to contribute (bug reports, features, docs, code, modules) - Development workflow (branch strategy, commit messages, pre-commit checks) - Pull request process and review guidelines - Technical proposals process - Coding standards and style guide - Testing guidelines (80% coverage requirement) - Package-specific guidelines (ums-lib, ums-sdk, CLI, MCP) ### Proposal Process (docs/proposal-process.md) - Formal process for significant technical changes - When proposals are required vs recommended - Proposal lifecycle (draft → review → decision) - Review criteria and acceptance requirements - Post-acceptance implementation tracking ### Quick Start Guide (docs/proposal-quick-start.md) - 5-minute quick start for creating proposals - Step-by-step walkthrough - Common pitfalls and best practices ### GitHub Issue Templates - `.github/ISSUE_TEMPLATE/proposal.md` - For new proposal submissions - `.github/ISSUE_TEMPLATE/proposal-feedback.md` - For feedback on existing proposals ### Proposal Template (docs/spec/proposals/TEMPLATE.md) - Structured template for technical proposals - Sections: Problem, Solution, Examples, Migration, Alternatives - Clear guidance on what to include in each section ## Documentation Fixes - Updated docs/README.md to include proposal process links - Removed broken link to non-existent `docs/processes/` directory - Integrated with existing architecture documentation structure from PR #85 ## Review Comments Addressed All applicable review comments addressed: - Verified CONTRIBUTING.md link to module authoring guide is valid - Fixed docs/README.md structure during conflict resolution - Removed broken processes directory link ## Files Added - CONTRIBUTING.md - docs/proposal-process.md - docs/proposal-quick-start.md - docs/spec/proposals/TEMPLATE.md - .github/ISSUE_TEMPLATE/proposal.md - .github/ISSUE_TEMPLATE/proposal-feedback.md ## Files Modified - docs/README.md (added proposal process links, removed broken link)
Summary
Add comprehensive architecture documentation and Architecture Decision Records (ADRs) to support development and maintenance of the project.
Documentation Added:
ums-lib(5 documents)copilot-instructions-cli(4 documents)Files Added: 15 files, 714 lines
Test Plan