-
Notifications
You must be signed in to change notification settings - Fork 2.9k
openspec show --diff feature #980
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
Open
bsmedberg-xometry
wants to merge
4
commits into
Fission-AI:main
Choose a base branch
from
bsmedberg-xometry:spec-diffs
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
68f1ba1
Proposed feature: openspec show --diff to see changed requirements mo…
bsmedberg-xometry b27aa01
Implementation of the --diff feature, which led to some spec changes …
bsmedberg-xometry 7c33e0a
Update the proposal to be clearer. Thanks coderabbit
bsmedberg-xometry 3fb6795
Fix a bug identified by coderabbit with excessive trimming, and add a…
bsmedberg-xometry File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| schema: spec-driven | ||
| created: 2026-04-15 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| ## Context | ||
|
|
||
| `openspec show <change>` currently displays the raw proposal markdown (text mode) or a parsed JSON with deltas extracted from the proposal's Capabilities section. Delta spec files under `openspec/changes/<name>/specs/<cap>/spec.md` contain full requirement text including unchanged content copied from the base spec at `openspec/specs/<cap>/spec.md`. | ||
|
|
||
| Reviewers need to see *what changed* without manually diffing files. The `--diff` flag adds this capability to the existing show command. | ||
|
|
||
| The project currently has no diff dependency. chalk is already available for colorized output. | ||
|
|
||
| ## Goals / Non-Goals | ||
|
|
||
| **Goals:** | ||
| - Let users see per-requirement diffs of delta specs via `openspec show <change> --diff` | ||
| - Support both text (colorized) and JSON output modes | ||
| - Keep the implementation minimal — no changes to storage format, validation, or the archive workflow | ||
|
|
||
| **Non-Goals:** | ||
| - Changing the delta spec format to store diffs instead of full text (future work, approach 2 from proposal) | ||
| - Diffing non-spec artifacts (proposal, design, tasks) | ||
| - Providing interactive diff navigation or side-by-side views | ||
| - Git-aware diffing (this compares files on disk) | ||
|
|
||
| ## Decisions | ||
|
|
||
| ### 1. Per-requirement diffing, not whole-file | ||
|
|
||
| **Choice:** Diff individual requirement blocks, not entire spec files. | ||
|
|
||
| The delta spec format already categorizes requirements by operation (`## ADDED`, `## MODIFIED`, `## REMOVED`, `## RENAMED`). Only MODIFIED requirements need a diff — the others are self-explanatory: | ||
|
|
||
| - **ADDED** — display the full requirement text (it's all new) | ||
| - **REMOVED** — display the removal notice (Reason/Migration already present) | ||
| - **RENAMED** — display the FROM:/TO: (already present) | ||
| - **MODIFIED** — match by requirement name (`### Requirement: <name>`) against the base spec at `openspec/specs/<cap>/spec.md`, extract both blocks, and compute a unified diff of those blocks | ||
|
|
||
| **Rationale:** The existing `ChangeParser` already parses delta specs into individual requirements with operations. The `MarkdownParser` already parses base specs into requirement blocks. We match by the `### Requirement:` header text (the same matching the archive step uses). This gives focused, meaningful output without noise from unchanged requirements. | ||
|
|
||
| **Alternative rejected:** Whole-file diff of base spec vs delta spec. This works but shows context from unchanged requirements that were copied verbatim into the delta file, which is exactly the noise the user wants to eliminate. | ||
|
|
||
| ### 2. Diff library: `diff` (npm) | ||
|
|
||
| **Choice:** Use the `diff` npm package (MIT, ~40KB, zero dependencies, widely used) for the MODIFIED requirement case. | ||
|
|
||
| **Alternatives considered:** | ||
| - **Implement from scratch** — Unified diff is well-specified but subtle (context lines, hunk headers). A library avoids bugs and maintenance burden. | ||
| - **Shell out to `diff` command** — Not cross-platform (Windows lacks `diff` by default). Violates the project's cross-platform requirements. | ||
|
|
||
| The `diff` package provides `createPatch()` which generates standard unified diff output from two strings. | ||
|
|
||
| ### 3. Requirement block extraction | ||
|
|
||
| **Choice:** Extract raw markdown text for a requirement block from a spec file by: | ||
| 1. Finding the `### Requirement: <name>` header line | ||
| 2. Collecting all lines until the next `###` header at the same or higher level (or EOF) | ||
| 3. Including the header line itself in the extracted block | ||
|
|
||
| This reuses the section-parsing logic already in `MarkdownParser` but needs a function that returns the raw markdown text (not the parsed `Requirement` object) so the diff is human-readable. | ||
|
|
||
| **Matching:** Requirement names are matched case-insensitively and whitespace-insensitively, consistent with how the archive step matches requirements. When a MODIFIED requirement's name matches a RENAMED entry's TO name in the same spec, the base block is looked up using the RENAMED FROM name instead — mirroring the archive step's behavior where RENAMED is applied before MODIFIED. | ||
|
|
||
| ### 4. Integration point: `ChangeCommand.show()` | ||
|
|
||
| **Choice:** Add diff logic to `ChangeCommand.show()` in `src/commands/change.ts`. When `--diff` is set: | ||
| - In text mode: iterate delta specs, group output by capability, show each requirement with its operation and (for MODIFIED) the colorized diff | ||
| - In JSON mode: add a `diff` field to each MODIFIED delta in the output object | ||
|
|
||
| **Alternative:** A separate `openspec diff` command. Rejected because the diff is about *viewing* a change, which is what `show` does. Adding a flag is more discoverable and consistent. | ||
|
|
||
| ### 5. Diff output format | ||
|
|
||
| **Text mode:** Per capability, per requirement: | ||
| - Header: capability name and operation | ||
| - ADDED/REMOVED/RENAMED: display the requirement text as-is (prefixed with operation label) | ||
| - MODIFIED: unified diff of the requirement block, colorized with chalk (green for `+`, red for `-`, dim for headers/context) | ||
|
|
||
| **JSON mode:** `--json --diff` extends the existing `--json` output (same `{ id, title, deltaCount, deltas }` structure). For each MODIFIED delta, a `diff` string field is added containing the raw unified diff text. ADDED/REMOVED/RENAMED deltas are unchanged. This is backwards-compatible: consumers that don't look for `diff` see the same shape they always did. | ||
|
|
||
| ### 6. Flag registration | ||
|
|
||
| Add `--diff` to: | ||
| - `openspec show` (top-level, passed through as a change-only flag) | ||
| - `openspec change show` (direct) | ||
|
|
||
| Add `'diff'` to the `CHANGE_FLAG_KEYS` set in `src/commands/show.ts` so it triggers a warning when used with `--type spec`. | ||
|
|
||
| ## Risks / Trade-offs | ||
|
|
||
| - **\[New dependency\]** Adding `diff` increases the dependency footprint slightly. → The package is small, zero-dep, and MIT. Acceptable for the functionality gained. | ||
| - **\[Requirement name mismatch\]** If a MODIFIED requirement's `### Requirement:` header doesn't match the base spec (after whitespace normalization), the diff can't find the base block. → Fall back to showing the full MODIFIED text with a warning. Note: `openspec validate` does not currently check this — the mismatch is only caught at archive time by `buildUpdatedSpec()`. The `--diff` warning becomes a useful early signal of the problem. | ||
| - **\[Path display on Windows\]** Capability names derived from directory names are platform-safe already. Paths used in diff headers should use forward slashes for readability. → Normalize display paths using `.replace(/\\/g, '/')` for display only; use `path.join()` for all filesystem operations. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| ## Why | ||
|
|
||
| When proposing a change, delta spec files under `openspec/changes/<name>/specs/` duplicate large portions of existing specs in `openspec/specs/`. A MODIFIED requirement must include the entire requirement block (all scenarios), making it hard to see what actually changed versus what was copied verbatim. This friction slows review and increases the risk of errors. | ||
|
|
||
| ## What Changes | ||
|
|
||
| Add a `--diff` flag to `openspec show` (change type) that renders each delta spec as a unified diff against the corresponding base spec in `openspec/specs/`. This is the smallest viable improvement: it doesn't change the storage format or workflow, just adds a new way to view the deltas. | ||
|
|
||
| ### Approaches considered | ||
|
|
||
| Three approaches were evaluated: | ||
|
|
||
| 1. **Stop storing delta specs; edit base specs on the branch directly.** This would eliminate duplication entirely but conflicts with the spec-driven workflow where changes are proposed, reviewed, and archived as discrete artifacts before the base specs are updated. Deferred — would require rethinking the change lifecycle. | ||
|
|
||
| 2. **Store deltas as diffs instead of full specs.** The `specs/<cap>/spec.md` files inside a change would contain unified diffs (or a structured delta format) rather than full requirement text. This eliminates duplication at the source but complicates authoring (AI and humans must produce correct diffs), parsing, validation, and the archive/apply step that merges deltas into base specs. Promising for a future change, but high complexity. | ||
|
|
||
| 3. **Add `openspec show --diff` to render deltas against base specs.** (Chosen.) Leave the storage format unchanged. When displaying a change, compute the diff on the fly by comparing each delta spec file against its matching base spec. This gives reviewers the view they need with minimal code changes and zero workflow disruption. | ||
|
|
||
| ### What this change delivers | ||
|
|
||
| - A `--diff` flag on `openspec show <change>` (and `openspec change show <change>`) that outputs a human-readable unified diff per delta spec | ||
|
|
||
| **JSON mode** (`--json --diff`): The existing JSON structure (`{ id, title, deltaCount, deltas }`) is preserved. Only deltas with `operation: "MODIFIED"` include a `"diff"` field containing unified-diff text. Deltas with `operation: "ADDED"`, `"REMOVED"`, or `"RENAMED"` do not include a `"diff"` field (it is absent from the object). When a base spec exists but no matching requirement block is found for a MODIFIED delta, that delta includes a `"warning"` field (string) instead of `"diff"`, describing the mismatch. | ||
|
|
||
| **Text mode** (`--diff` without `--json`): The proposal markdown is printed first, followed by a "Specifications Changed (diffs)" section. MODIFIED deltas show colorized unified diffs (additions in green, removals in red); ADDED deltas show the full requirement text as all-additions in green; REMOVED deltas show the requirement name in red; RENAMED deltas show old and new names. When a MODIFIED delta has no base spec (new capability), the diff is rendered as all-additions. When a MODIFIED delta has a base spec but no matching requirement block, a warning is printed with the raw requirement text. | ||
|
|
||
| ## Capabilities | ||
|
|
||
| ### New Capabilities | ||
|
|
||
| None. | ||
|
|
||
| ### Modified Capabilities | ||
|
|
||
| - `cli-show`: Add `--diff` flag support for change display, computing unified diffs of delta specs against their base specs | ||
|
|
||
| ## Non-goals | ||
|
|
||
| - Changing the delta spec storage format (approach 2 above — future work) | ||
| - Changing when or how base specs are updated (approach 1 above — future work) | ||
| - Diffing non-spec artifacts (proposal, design, tasks) | ||
| - Git-aware diffing (this compares files on disk, not git history) | ||
|
|
||
| ## Impact | ||
|
|
||
| - `src/commands/show.ts` — pass `--diff` flag through to change display | ||
| - `src/commands/change.ts` — implement diff rendering in `show()` for text and JSON modes | ||
| - `src/cli/index.ts` — register `--diff` option on the show and change show commands | ||
| - New utility: diff computation between two markdown strings (can use a lightweight diff library or Node built-in) | ||
| - `src/commands/spec.ts` or `src/utils/` — helper to resolve base spec path for a given delta spec |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| ## MODIFIED Requirements | ||
|
|
||
| ### Requirement: Output format options | ||
|
|
||
| The show command SHALL support various output formats consistent with existing commands. | ||
|
|
||
| #### Scenario: JSON output | ||
|
|
||
| - **WHEN** executing `openspec show <item> --json` | ||
| - **THEN** output the item in JSON format | ||
| - **AND** include parsed metadata and structure | ||
| - **AND** maintain format consistency with existing change/spec show commands | ||
|
|
||
| #### Scenario: Flag scoping and delegation | ||
|
|
||
| - **WHEN** showing a change or a spec via the top-level command | ||
| - **THEN** accept common flags such as `--json` | ||
| - **AND** pass through type-specific flags to the corresponding implementation | ||
| - Change-only flags: `--deltas-only` (alias `--requirements-only` deprecated), `--diff` | ||
| - Spec-only flags: `--requirements`, `--no-scenarios`, `-r/--requirement` | ||
| - **AND** ignore irrelevant flags for the detected type with a warning | ||
|
|
||
| #### Scenario: Change-specific options | ||
|
|
||
| - **WHEN** showing a change with `openspec show <change-name> --deltas-only` | ||
| - **THEN** display only the deltas in JSON format | ||
| - **AND** maintain compatibility with existing change show options | ||
|
|
||
| #### Scenario: Spec-specific options | ||
|
|
||
| - **WHEN** showing a spec with `openspec show <spec-id> --requirements` | ||
| - **THEN** display only requirements in JSON format | ||
| - **AND** support other spec options (--no-scenarios, -r) | ||
| - **AND** maintain compatibility with existing spec show options | ||
|
|
||
| #### Scenario: Text mode change display | ||
|
|
||
| - **WHEN** executing `openspec show <change-name>` in text mode (no `--json`) | ||
| - **THEN** display the proposal markdown text | ||
| - **AND** if delta spec files exist under `openspec/changes/<change-name>/specs/`, display each delta spec's full content grouped by capability name | ||
|
|
||
| #### Scenario: Diff output in text mode | ||
|
|
||
| - **WHEN** executing `openspec show <change-name> --diff` in text mode (no `--json`) | ||
| - **THEN** display the proposal markdown text | ||
| - **AND** for each delta spec file under `openspec/changes/<change-name>/specs/<cap>/spec.md`, iterate parsed deltas grouped by capability | ||
| - **AND** for ADDED requirements, display the full requirement text with a green "ADDED" label | ||
| - **AND** for REMOVED requirements, display the removal notice (Reason/Migration) with a red "REMOVED" label | ||
| - **AND** for RENAMED requirements, display the FROM:/TO: with a cyan "RENAMED" label | ||
| - **AND** for MODIFIED requirements, extract the matching requirement block from the base spec at `openspec/specs/<cap>/spec.md` by `### Requirement:` header name, compute a unified diff of the base block vs the delta block, and display it colorized (green for `+` lines, red for `-` lines, dim for context lines and diff headers) | ||
| - **AND** when a MODIFIED requirement's name matches a RENAMED entry's TO name in the same spec, the system SHALL look up the base block using the RENAMED entry's FROM name instead | ||
| - **AND** if a MODIFIED requirement has no matching base requirement (and no corresponding RENAMED entry), display the full text with a warning | ||
|
|
||
| #### Scenario: Diff output in JSON mode | ||
|
|
||
| - **WHEN** executing `openspec show <change-name> --json --diff` | ||
| - **THEN** the output SHALL use the same JSON structure as `--json` alone (`{ id, title, deltaCount, deltas }`) | ||
| - **AND** for each MODIFIED delta, the delta object SHALL include an additional `diff` string field containing the unified diff of the base requirement block vs the delta requirement block | ||
| - **AND** when a MODIFIED requirement corresponds to a RENAMED entry, the base block SHALL be looked up using the RENAMED FROM name | ||
| - **AND** ADDED, REMOVED, and RENAMED deltas SHALL NOT have a `diff` field | ||
| - **AND** if a MODIFIED requirement has no matching base requirement, the delta object SHALL include a `warning` string field instead of `diff` | ||
|
|
||
| #### Scenario: Diff with no delta specs | ||
|
|
||
| - **WHEN** executing `openspec show <change-name> --diff` and the change has no delta spec files | ||
| - **THEN** print a message indicating no delta specs exist for this change | ||
| - **AND** exit with code 0 | ||
|
|
||
| #### Scenario: Diff flag on non-change item | ||
|
|
||
| - **WHEN** executing `openspec show <spec-name> --diff` | ||
| - **THEN** ignore the `--diff` flag with a warning (flag is not applicable to specs) | ||
|
|
||
| ## ADDED Requirements | ||
|
|
||
| ### Requirement: Requirement block extraction for diffing | ||
|
|
||
| The system SHALL extract raw markdown text for individual requirement blocks from spec files to support per-requirement diffing. | ||
|
|
||
| #### Scenario: Extract requirement block by name | ||
|
|
||
| - **WHEN** a requirement name is provided and a spec file contains a matching `### Requirement: <name>` header | ||
| - **THEN** the system SHALL return the raw markdown text from the `### Requirement:` header line through all content until the next `###` header at the same or higher level (or end of file) | ||
| - **AND** matching SHALL be case-insensitive and whitespace-insensitive | ||
|
|
||
| #### Scenario: Requirement name not found in base spec | ||
|
|
||
| - **WHEN** a MODIFIED delta requirement name does not match any `### Requirement:` header in the base spec | ||
| - **THEN** the system SHALL return null for the base block | ||
| - **AND** the caller SHALL display the full MODIFIED requirement text with a warning that no base was found | ||
|
|
||
| #### Scenario: Cross-platform path resolution for base specs | ||
|
|
||
| - **WHEN** resolving the base spec path for a given capability | ||
| - **THEN** the system SHALL use `path.join()` for filesystem operations to build `openspec/specs/<cap>/spec.md` | ||
| - **AND** display paths SHALL use forward slashes regardless of platform | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
The text diff scenario is specifying the wrong output.
This says
openspec show <change-name> --diffshould still print the proposal markdown first, but the intended behavior for--diffis diff-only text output. If this stays, the spec will lock in the current implementation bug.🧰 Tools
🪛 LanguageTool
[uncategorized] ~45-~45: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...-json`) - THEN display the proposal markdown text - AND for each delta spec file...
(MARKDOWN_NNP)
🤖 Prompt for AI Agents
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.
Actually, the spec was correct: I want
openspec show --diffto print the proposal and then the diffs of the changed requirements... it will display new requirements completely.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.
@bsmedberg-xometry, thanks for the clarification! I was mistaken — the spec is correct as written. The intended behavior foropenspec show <change-name> --diffin text mode is to print the proposal markdown first and then render the per-requirement diffs (with new/ADDED requirements shown in full). I'll note this for future reviews.✏️ Learnings added
🧠 Learnings used