Files
fission-ai__openspec/test/utils/requirement-diff.test.ts
bsmedberg-xometry dd7cea3ffe feat(show): diff delta requirements against the main specs (#980)
* Proposed feature: openspec show --diff to see changed requirements more clearly

* Implementation of the --diff feature, which led to some spec changes during implementation.

* Update the proposal to be clearer. Thanks coderabbit

* Fix a bug identified by coderabbit with excessive trimming, and add a testcase for it.

* fix(show): harden --diff for review feedback

Keeps `openspec show <change>` without `--diff` a raw proposal
passthrough, reports when a change has no delta specs instead of
returning silently, preserves the Reason/Migration body of a REMOVED
requirement, and resolves main specs through the command's root so
`--store <id>` diffs against that store.

Text mode and JSON mode now render from one shared collection pass, the
CLI tests drive argv arrays from a mkdtemp project instead of
interpolated shell strings, `--diff` is registered for shell
completions, and the stray package-lock.json is gone.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* build(deps): declare the diff dependency and refresh the flake hash

Adds the `diff` runtime dependency that requirement-diff.ts imports,
updates pnpm-lock.yaml, and regenerates the flake's pnpmDeps hash so
`nix build` matches the new lockfile.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(show): diff nested capabilities too

collectSpecDiffs enumerated only top-level directories under the
change's specs/, so a nested capability (specs/<area>/<id>/spec.md) was
skipped: text mode printed nothing for it and its MODIFIED deltas came
back from --json with no diff. It now uses the same discoverSpecFiles()
helper ChangeParser uses, so the capability ids match the `spec` field
of the JSON deltas.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(show): report header mismatches instead of hiding them

Two cases where --diff quietly showed something misleading:

A MODIFIED requirement whose capability has no main spec was rendered as
all-additions, which reads like a new capability. It is an authoring
error archive will reject, so it now prints the raw text with a warning
naming the missing spec.

A header that differs from the main spec only in case or interior
spacing found no match at all under exact lookup, or matched under a
lowercase-only comparison that let a real mismatch through silently.
Lookup is now exact first, then the shared foldRequirementName fallback,
and a folded match prints the diff the author meant alongside a warning
that archive matches names exactly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(show): name the main spec as such in collectSpecDiffs

Comment and locals still called the main spec the "base" spec, and the
no-main-spec comment described the old all-additions behavior.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* chore(deps-dev): bump the development-dependencies group with 2 updates

Bumps the development-dependencies group with 2 updates: [smol-toml](https://github.com/squirrelchat/smol-toml) and [typescript-eslint](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/typescript-eslint).


Updates `smol-toml` from 1.7.1 to 1.8.0
- [Release notes](https://github.com/squirrelchat/smol-toml/releases)
- [Commits](https://github.com/squirrelchat/smol-toml/compare/v1.7.1...v1.8.0)

Updates `typescript-eslint` from 8.66.0 to 8.67.0
- [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases)
- [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/typescript-eslint/CHANGELOG.md)
- [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v8.67.0/packages/typescript-eslint)

---
updated-dependencies:
- dependency-name: smol-toml
  dependency-version: 1.8.0
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: development-dependencies
- dependency-name: typescript-eslint
  dependency-version: 8.67.0
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: development-dependencies
...

Signed-off-by: dependabot[bot] <support@github.com>

* chore(nix): invalidate pnpm dependency hash

* fix(nix): update pnpm dependency hash

* fix(show): harden requirement diff output

* build(nix): pin combined dependency hash

* test(show): assert proposal precedes diffs

* docs(show): clarify JSON diff diagnostics

* fix(show): retain unified diff hunk headers

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Clay Good <hi@claygood.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-08-26 20:18:03 +00:00

222 lines
7.7 KiB
TypeScript

import { describe, it, expect } from 'vitest';
import {
extractRequirementBlock,
diffRequirementBlock,
buildRenameMap,
} from '../../src/utils/requirement-diff.js';
const SAMPLE_SPEC = `# sample Specification
## Purpose
A test spec.
## Requirements
### Requirement: Load project config
The system SHALL read config.
#### Scenario: Valid config
- **WHEN** config exists
- **THEN** parse it
### Requirement: Support .yml alias
The system SHALL accept .yml.
#### Scenario: yml extension
- **WHEN** config.yml exists
- **THEN** read it
### Requirement: Enforce size limit
The system SHALL reject large configs.
#### Scenario: Over limit
- **WHEN** config > 50KB
- **THEN** reject
`;
describe('extractRequirementBlock', () => {
it('returns the full block for an exact name match', () => {
const block = extractRequirementBlock(SAMPLE_SPEC, 'Load project config');
expect(block).not.toBeNull();
expect(block!.exact).toBe(true);
expect(block!.name).toBe('Load project config');
expect(block!.raw).toContain('### Requirement: Load project config');
expect(block!.raw).toContain('The system SHALL read config.');
expect(block!.raw).toContain('#### Scenario: Valid config');
// Should NOT contain the next requirement
expect(block!.raw).not.toContain('Support .yml alias');
});
it('matches case-insensitively, and reports the match as inexact', () => {
const block = extractRequirementBlock(SAMPLE_SPEC, 'load PROJECT config');
expect(block).not.toBeNull();
expect(block!.raw).toContain('### Requirement: Load project config');
// Archive matches names exactly, so the caller has to be able to say so.
expect(block!.exact).toBe(false);
expect(block!.name).toBe('Load project config');
});
it('matches with extra outer whitespace and stays exact', () => {
const block = extractRequirementBlock(SAMPLE_SPEC, ' Load project config ');
expect(block).not.toBeNull();
expect(block!.raw).toContain('### Requirement: Load project config');
expect(block!.exact).toBe(true);
});
it('matches through interior whitespace, and reports the match as inexact', () => {
const block = extractRequirementBlock(SAMPLE_SPEC, 'Load project config');
expect(block).not.toBeNull();
expect(block!.raw).toContain('### Requirement: Load project config');
expect(block!.exact).toBe(false);
});
it('prefers the exact match over a fold match', () => {
const spec = SAMPLE_SPEC.replace(
'### Requirement: Enforce size limit',
'### Requirement: LOAD PROJECT CONFIG'
);
const block = extractRequirementBlock(spec, 'Load project config');
expect(block!.exact).toBe(true);
expect(block!.raw).toContain('The system SHALL read config.');
});
it('returns null when name does not match', () => {
const block = extractRequirementBlock(SAMPLE_SPEC, 'Nonexistent requirement');
expect(block).toBeNull();
});
it('extracts the last requirement in a file (no following header)', () => {
const block = extractRequirementBlock(SAMPLE_SPEC, 'Enforce size limit');
expect(block).not.toBeNull();
expect(block!.raw).toContain('### Requirement: Enforce size limit');
expect(block!.raw).toContain('#### Scenario: Over limit');
});
it('does not match requirement headers outside the Requirements section', () => {
const specWithPreamble = `# Spec
## Purpose
Test purpose.
### Requirement: Preamble header
This is not inside a Requirements section.
## Requirements
### Requirement: Real requirement
The system SHALL do something.
#### Scenario: Works
- **WHEN** called
- **THEN** works
`;
// The preamble header is outside ## Requirements, so it should not be found
const preambleBlock = extractRequirementBlock(specWithPreamble, 'Preamble header');
expect(preambleBlock).toBeNull();
const realBlock = extractRequirementBlock(specWithPreamble, 'Real requirement');
expect(realBlock).not.toBeNull();
expect(realBlock!.raw).toContain('The system SHALL do something.');
});
it('handles Windows-style line endings', () => {
const windowsSpec = SAMPLE_SPEC.replace(/\n/g, '\r\n');
const block = extractRequirementBlock(windowsSpec, 'Load project config');
expect(block).not.toBeNull();
expect(block!.raw).toContain('### Requirement: Load project config');
});
});
describe('diffRequirementBlock', () => {
it('produces a diff when base and delta differ', () => {
const base = '### Requirement: Foo\n\nThe system SHALL do A.\n\n#### Scenario: A\n- **WHEN** called\n- **THEN** A';
const delta = '### Requirement: Foo\n\nThe system SHALL do B.\n\n#### Scenario: A\n- **WHEN** called\n- **THEN** B';
const diff = diffRequirementBlock(base, delta, 'test');
expect(diff).toContain('-The system SHALL do A.');
expect(diff).toContain('+The system SHALL do B.');
// Keep range headers that locate each hunk, but omit synthetic file headers.
expect(diff).not.toContain('Index:');
expect(diff).not.toContain('---');
expect(diff).not.toContain('+++');
expect(diff).toContain('@@');
});
it('keeps separate range headers for distant changes', () => {
const baseLines = Array.from({ length: 20 }, (_, i) => `Line ${i + 1}`);
const deltaLines = [...baseLines];
deltaLines[0] = 'Changed first line';
deltaLines[19] = 'Changed last line';
const diff = diffRequirementBlock(baseLines.join('\n'), deltaLines.join('\n'), 'test');
expect(diff.match(/^@@/gm)).toHaveLength(2);
expect(diff).toContain('-Line 1');
expect(diff).toContain('+Changed first line');
expect(diff).toContain('-Line 20');
expect(diff).toContain('+Changed last line');
});
it('shows all additions when base is null', () => {
const delta = '### Requirement: New\n\nThe system SHALL exist.\n\n#### Scenario: Exists\n- **WHEN** checked\n- **THEN** exists';
const diff = diffRequirementBlock(null, delta, 'test');
expect(diff).toContain('+### Requirement: New');
expect(diff).toContain('+The system SHALL exist.');
expect(diff).not.toContain('No newline');
});
it('preserves leading space on context lines before first change', () => {
const base = '### Requirement: Foo\n\nLine one.\n\nLine two.\n\nLine three.';
const delta = '### Requirement: Foo\n\nLine one.\n\nLine two changed.\n\nLine three.';
const diff = diffRequirementBlock(base, delta, 'test');
const lines = diff.split('\n');
const contextLine = lines.find(l => l.endsWith('Line one.'))!;
expect(contextLine.startsWith(' ')).toBe(true);
});
it('produces empty output for identical blocks', () => {
const content = '### Requirement: Same\n\nThe system SHALL stay.\n\n#### Scenario: Same\n- **WHEN** called\n- **THEN** same';
const diff = diffRequirementBlock(content, content, 'test');
expect(diff).toBe('');
});
});
describe('buildRenameMap', () => {
it('builds map from single rename', () => {
const map = buildRenameMap([{ from: 'Old name', to: 'New name' }]);
expect(map.get('new name')).toBe('Old name');
});
it('builds map from multiple renames', () => {
const map = buildRenameMap([
{ from: 'Alpha', to: 'Bravo' },
{ from: 'Charlie', to: 'Delta' },
]);
expect(map.size).toBe(2);
expect(map.get('bravo')).toBe('Alpha');
expect(map.get('delta')).toBe('Charlie');
});
it('maps a rename chain back to the original main requirement', () => {
const map = buildRenameMap([
{ from: 'Alpha', to: 'Bravo' },
{ from: 'Bravo', to: 'Charlie' },
]);
expect(map.size).toBe(1);
expect(map.get('charlie')).toBe('Alpha');
expect(map.has('bravo')).toBe(false);
});
it('returns empty map for empty list', () => {
const map = buildRenameMap([]);
expect(map.size).toBe(0);
});
});