Skip to content

Commit c61459d

Browse files
yha9806claude
andcommitted
fix: improve delta parser robustness and error diagnostics (#164)
## Problem Delta validation was failing to parse requirement blocks that `openspec show --json --deltas-only` could successfully parse. The regex pattern `/^###\s+Requirement:/` was too strict about whitespace, and error messages lacked diagnostic details to help users identify parsing failures. ## Solution ### 1. Parser Enhancements (src/core/parsers/requirement-blocks.ts) - Updated all regex patterns from `/^###\s+Requirement:/` to `/^###\s*Requirement:/` to allow flexible whitespace (0 or more spaces/tabs) - Added ParseDiagnostic interface for collecting parse failure information - Enhanced parseRequirementBlocksFromSection() to collect diagnostics for near-miss header patterns (lines starting with ### but not matching full pattern) - Updated parseDeltaSpec() with optional collectDiagnostics parameter ### 2. Validator Improvements (src/core/validation/validator.ts) - Modified validateChangeDeltaSpecs() to enable diagnostic collection - Added extractSectionPreview() helper to show first 5 lines of problematic sections - Enhanced error messages to include: - Section content preview - Parse diagnostics with line numbers and problematic text - Expected format reminders - Debugging command suggestions ### 3. Test Coverage (test/core/parsers/requirement-blocks.test.ts) - Created comprehensive test suite with 13 test cases covering: - CRLF line ending handling - Mixed line endings (LF + CRLF) - Extra whitespace in headers (### Requirement:) - Missing whitespace (###Requirement:) - Tab characters in headers - Diagnostic collection verification - Backward compatibility regression tests ### 4. Documentation (openspec/AGENTS.md) - Added comprehensive troubleshooting section for delta parsing errors - Included common error patterns, debugging workflow, and enhanced diagnostics explanation ## Results - ✅ All 13 new tests passing - ✅ 278/279 existing tests passing (1 unrelated Windows-specific failure) - ✅ Backward compatible - all existing valid specs continue to parse correctly - ✅ Build successful - ✅ Cross-platform line ending support (LF, CRLF, mixed) ## OpenSpec Proposal - Change ID: improve-delta-parsing - All 31 tasks completed (100%) - Proposal validates with --strict mode Fixes #164 🤖 Generated with Claude Code Co-Authored-By: Claude <[email protected]>
1 parent d32e50f commit c61459d

File tree

8 files changed

+639
-15
lines changed

8 files changed

+639
-15
lines changed

BUG_ANALYSIS.md

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
# OpenSpec CLI Bug Analysis
2+
3+
## Environment Setup ✅
4+
5+
- **Fork创建**: `https://github.com/yha9806/OpenSpec`
6+
- **本地路径**: `I:\OpenSpec-Fix`
7+
- **Node.js版本**: v20.19.4 ✅
8+
- **pnpm版本**: 10.20.0 ✅
9+
- **测试结果**: 265/266 通过 (99.6%) ✅
10+
11+
## Bug #164: Delta Validation Parsing Failure
12+
13+
### 问题描述
14+
15+
用户报告 `openspec validate --strict` 失败并报错 "no deltas found",但 `openspec show --json --deltas-only` 可以成功解析相同文件。
16+
17+
### 根本原因分析
18+
19+
通过代码审查发现问题出在 **validator.ts**`validateChangeDeltaSpecs()` 方法 (lines 113-272):
20+
21+
#### 关键发现:
22+
23+
1. **验证逻辑缺陷** (lines 142-145):
24+
```typescript
25+
const hasEntries = plan.added.length + plan.modified.length +
26+
plan.removed.length + plan.renamed.length > 0;
27+
if (!hasEntries) {
28+
if (hasSections) emptySectionSpecs.push({ path: entryPath, sections: sectionNames });
29+
else missingHeaderSpecs.push(entryPath);
30+
}
31+
```
32+
33+
2. **Delta 解析在 requirement-blocks.ts**:
34+
- `parseDeltaSpec()` (lines 119-142): 正确解析所有 delta sections
35+
- `parseRequirementBlocksFromSection()` (lines 172-194): 使用正则 `/^###\s+Requirement:/`
36+
37+
3. **潜在问题**:
38+
- 正则表达式可能对空格敏感
39+
- 行尾符处理 (CRLF vs LF) 可能导致解析失败
40+
- `normalizeLineEndings()` (lines 112-114) 已经处理了行尾符,但可能在某些情况下失效
41+
42+
### Bug #2: 归档 Changes 无法识别
43+
44+
#### 问题描述
45+
46+
CLI 无法识别 `openspec/changes/archive/` 目录下的 changes。
47+
48+
#### 需要调查的文件:
49+
50+
- `src/core/list.ts` - 列表命令实现
51+
- `src/commands/change.ts` 或类似文件 - change 命令实现
52+
- 需要查找目录扫描逻辑
53+
54+
## 修复计划
55+
56+
### Phase 1: Delta 验证 Bug
57+
58+
1. **增强行尾符处理**:
59+
- 确保所有输入在正则匹配前都经过 `normalizeLineEndings()`
60+
- 添加调试日志记录解析失败的行
61+
62+
2. **优化正则表达式**:
63+
- 当前: `/^###\s+Requirement:/`
64+
- 考虑: `/^###\s*Requirement:\s*/` (允许0个或多个空格)
65+
66+
3. **改进错误消息**:
67+
- 当找到 section header 但没有解析到 requirements 时,输出具体哪些行被跳过
68+
- 提供示例格式
69+
70+
### Phase 2: 归档 Changes 识别
71+
72+
1. 定位目录扫描代码
73+
2. 添加对 `archive/` 子目录的支持
74+
3. 可能需要递归扫描或添加 `--include-archived` 标志
75+
76+
### Phase 3: 测试
77+
78+
1. 创建测试用例重现 Issue #164
79+
2. 验证修复对现有测试的影响
80+
3. 添加边界情况测试(CRLF, 额外空格,等)
81+
82+
## 下一步行动
83+
84+
1. 创建新分支 `fix/issue-164-delta-validation`
85+
2. 修复 delta 解析逻辑
86+
3. 运行测试套件验证
87+
4. 提交 PR 到上游
88+
89+
---
90+
91+
**Created**: 2025-11-05
92+
**Author**: yha9806
93+
**Issue Reference**: https://github.com/Fission-AI/OpenSpec/issues/164

openspec/AGENTS.md

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,49 @@ Example for RENAMED:
300300
- Exact format required: `#### Scenario: Name`
301301
- Debug with: `openspec show [change] --json --deltas-only`
302302

303+
**"Delta sections found, but no requirement entries parsed"**
304+
This error means the validator detected delta section headers (e.g., `## ADDED Requirements`) but couldn't parse any requirement blocks within them.
305+
306+
Common causes and solutions:
307+
1. **Wrong requirement header format**
308+
- ❌ Wrong: `### Some Header`, `### Feature: Name`, `###Header: Name`
309+
- ✅ Correct: `### Requirement: Feature Name`
310+
- The keyword "Requirement:" is mandatory and case-sensitive
311+
- Whitespace around `###` is flexible (0+ spaces/tabs allowed)
312+
313+
2. **Line ending issues (Windows users)**
314+
- Mixed CRLF/LF line endings can cause parsing problems in some editors
315+
- Ensure consistent line endings throughout the file
316+
- Most modern editors handle this automatically
317+
318+
3. **Hidden characters or encoding issues**
319+
- Check for invisible Unicode characters (zero-width spaces, BOM markers)
320+
- Use UTF-8 encoding without BOM
321+
- View the raw file content to inspect for hidden characters
322+
323+
4. **Debugging approach**
324+
```bash
325+
# Step 1: Check if parser sees the content differently
326+
openspec show [change] --json --deltas-only
327+
328+
# Step 2: If validation fails but show succeeds, check:
329+
# - Missing scenarios (every requirement needs ≥1 scenario)
330+
# - Missing SHALL/MUST keywords in requirement text
331+
332+
# Step 3: Check the enhanced error message details
333+
# The validator will show:
334+
# - First 5 lines of each problematic section
335+
# - Line numbers of near-miss patterns (lines starting with ### but not matching)
336+
# - Specific format expectations
337+
```
338+
339+
5. **Enhanced diagnostics**
340+
When validation fails, the error message now includes:
341+
- Preview of the first few lines from each section
342+
- Parse diagnostics showing which lines almost matched
343+
- Expected format reminders
344+
- Suggested debugging commands
345+
303346
### Validation Tips
304347

305348
```bash
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
## Why
2+
Delta validation fails to parse requirement blocks that `openspec show --json --deltas-only` successfully parses, causing false negatives. The parser's regex pattern `/^###\s+Requirement:/` is too strict about whitespace, and error messages don't provide enough detail to diagnose parsing failures.
3+
4+
## What Changes
5+
- Enhance requirement block parsing to handle edge cases (whitespace variations, line ending normalization)
6+
- Improve validation error messages to show which lines failed to parse and why
7+
- Add diagnostic logging when section headers are found but no requirements are parsed
8+
9+
## Impact
10+
- Affected specs: cli-validate
11+
- Affected code: `src/core/parsers/requirement-blocks.ts`, `src/core/validation/validator.ts`
12+
- Bug fixes: Resolves issue #164
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
## MODIFIED Requirements
2+
3+
### Requirement: Parser SHALL handle cross-platform line endings
4+
The markdown parser SHALL correctly identify sections regardless of line ending format (LF, CRLF, CR). Additionally, the parser SHALL handle variations in whitespace around requirement headers to ensure robust parsing across different text editors and platforms.
5+
6+
#### Scenario: Required sections parsed with CRLF line endings
7+
- **GIVEN** a change proposal markdown saved with CRLF line endings
8+
- **AND** the document contains `## Why` and `## What Changes`
9+
- **WHEN** running `openspec validate <change-id>`
10+
- **THEN** validation SHALL recognize the sections and NOT raise parsing errors
11+
12+
#### Scenario: Requirement headers with variable whitespace
13+
- **GIVEN** a delta spec file with `### Requirement:` (extra space) or `###Requirement:` (no space)
14+
- **WHEN** running `openspec validate <change-id>`
15+
- **THEN** the parser SHALL recognize these as valid requirement headers
16+
- **AND** validation SHALL parse the requirement blocks successfully
17+
18+
#### Scenario: Mixed line endings within file
19+
- **GIVEN** a delta spec file with inconsistent line endings (some LF, some CRLF)
20+
- **WHEN** running `openspec validate <change-id>`
21+
- **THEN** the parser SHALL normalize all line endings before processing
22+
- **AND** validation SHALL parse all requirement blocks correctly
23+
24+
### Requirement: Validation SHALL provide actionable remediation steps
25+
Validation output SHALL include specific guidance to fix each error, including expected structure, example headers, and suggested commands to verify fixes. Error messages SHALL provide detailed diagnostic information when parsing fails.
26+
27+
#### Scenario: No deltas found in change
28+
- **WHEN** validating a change with zero parsed deltas
29+
- **THEN** show error "No deltas found" with guidance:
30+
- Explain that change specs must include `## ADDED Requirements`, `## MODIFIED Requirements`, `## REMOVED Requirements`, or `## RENAMED Requirements`
31+
- Remind authors that files must live under `openspec/changes/{id}/specs/<capability>/spec.md`
32+
- Include an explicit note: "Spec delta files cannot start with titles before the operation headers"
33+
- Suggest running `openspec change show {id} --json --deltas-only` for debugging
34+
35+
#### Scenario: Delta sections found but no requirements parsed
36+
- **WHEN** validation detects delta section headers (e.g., `## ADDED Requirements`) but parses zero requirement blocks
37+
- **THEN** show error indicating which sections were found but empty
38+
- **AND** include diagnostic details:
39+
- Expected requirement header format: `### Requirement: Description`
40+
- List the first few lines after each section header (up to 5 lines) to aid debugging
41+
- Note that whitespace must be consistent and line endings normalized
42+
- Suggest checking for invisible characters or encoding issues
43+
- **AND** suggest running `openspec show {id} --json --deltas-only` to verify parser behavior
44+
45+
#### Scenario: Missing required sections
46+
- **WHEN** a required section is missing
47+
- **THEN** include expected header names and a minimal skeleton:
48+
- For Spec: `## Purpose`, `## Requirements`
49+
- For Change: `## Why`, `## What Changes`
50+
- Provide an example snippet of the missing section with placeholder prose ready to copy
51+
- Mention the quick-reference section in `openspec/AGENTS.md` as the authoritative template
52+
53+
#### Scenario: Missing requirement descriptive text
54+
- **WHEN** a requirement header lacks descriptive text before scenarios
55+
- **THEN** emit an error explaining that `### Requirement:` lines must be followed by narrative text before any `#### Scenario:` headers
56+
- Show compliant example: "### Requirement: Foo" followed by "The system SHALL ..."
57+
- Suggest adding 1-2 sentences describing the normative behavior prior to listing scenarios
58+
- Reference the pre-validation checklist in `openspec/AGENTS.md`
59+
60+
## ADDED Requirements
61+
62+
### Requirement: Parser SHALL provide diagnostic logging for parse failures
63+
When the parser encounters malformed requirement blocks, it SHALL collect diagnostic information to help authors identify the root cause without requiring deep knowledge of parser internals.
64+
65+
#### Scenario: Log lines that fail requirement header regex
66+
- **WHEN** the parser scans a delta section and encounters lines that almost match the requirement header pattern
67+
- **THEN** the parser SHALL log (at debug level or include in validation output) lines that:
68+
- Start with `###` but don't match the full pattern
69+
- Have unexpected characters or whitespace
70+
- May be missing the "Requirement:" keyword
71+
- **AND** these logs SHALL be available when validation fails to parse requirements
72+
73+
#### Scenario: Provide line numbers in error messages
74+
- **WHEN** validation fails to parse requirement blocks from a section
75+
- **THEN** error messages SHALL include:
76+
- The line number range where the section begins
77+
- Line numbers of any lines that triggered regex near-misses
78+
- A sample of the raw text (escaped/quoted) to show hidden characters
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
## 1. Parser robustness improvements
2+
- [x] 1.1 Update requirement header regex in `requirement-blocks.ts` to allow flexible whitespace: `/^###\s*Requirement:\s*/`
3+
- [x] 1.2 Add early normalization check to ensure `normalizeLineEndings()` is called before all regex operations
4+
- [x] 1.3 Add diagnostic collection: track lines that start with `###` but fail to match the requirement pattern
5+
6+
## 2. Enhanced error messaging
7+
- [x] 2.1 When `emptySectionSpecs` is not empty, include first 5 lines of section content in error message
8+
- [x] 2.2 Add helper function to format diagnostic info (line numbers, escaped text preview)
9+
- [x] 2.3 Update `VALIDATION_MESSAGES.CHANGE_NO_DELTAS` to include troubleshooting steps from spec
10+
11+
## 3. Validator diagnostic improvements
12+
- [x] 3.1 Modify `validateChangeDeltaSpecs()` to collect parse failure diagnostics from parser
13+
- [x] 3.2 When sections are found but parsing yields zero blocks, include:
14+
- Section names that were detected
15+
- First few lines of each empty section
16+
- Expected format reminder
17+
- [x] 3.3 Add debug flag support for verbose parser logging (optional enhancement)
18+
19+
## 4. Tests
20+
- [x] 4.1 Add test case: CRLF line endings in delta spec file
21+
- [x] 4.2 Add test case: Extra whitespace in requirement headers (`### Requirement:`)
22+
- [x] 4.3 Add test case: Missing space in requirement header (`###Requirement:`)
23+
- [x] 4.4 Add test case: Mixed line endings (LF and CRLF in same file)
24+
- [x] 4.5 Add test case: Verify enhanced error messages include diagnostic details
25+
- [x] 4.6 Regression test: Ensure existing valid specs still parse correctly
26+
27+
## 5. Documentation
28+
- [x] 5.1 Update validation error message templates to reference new diagnostic output
29+
- [x] 5.2 Add troubleshooting section to AGENTS.md covering common parsing issues
30+
31+
## 6. Integration validation
32+
- [x] 6.1 Run full test suite to ensure no regressions
33+
- [x] 6.2 Test with real-world spec files from the repository
34+
- [x] 6.3 Verify Issue #164 reproduction case now passes validation

src/core/parsers/requirement-blocks.ts

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ export function extractRequirementsSection(content: string): RequirementsSection
5858
let preambleLines: string[] = [];
5959

6060
// Collect preamble lines until first requirement header
61-
while (cursor < sectionBodyLines.length && !/^###\s+Requirement:/.test(sectionBodyLines[cursor])) {
61+
while (cursor < sectionBodyLines.length && !/^###\s*Requirement:/.test(sectionBodyLines[cursor])) {
6262
preambleLines.push(sectionBodyLines[cursor]);
6363
cursor++;
6464
}
@@ -76,7 +76,7 @@ export function extractRequirementsSection(content: string): RequirementsSection
7676
cursor++;
7777
// Gather lines until next requirement header or end of section
7878
const bodyLines: string[] = [headerLineCandidate];
79-
while (cursor < sectionBodyLines.length && !/^###\s+Requirement:/.test(sectionBodyLines[cursor]) && !/^##\s+/.test(sectionBodyLines[cursor])) {
79+
while (cursor < sectionBodyLines.length && !/^###\s*Requirement:/.test(sectionBodyLines[cursor]) && !/^##\s+/.test(sectionBodyLines[cursor])) {
8080
bodyLines.push(sectionBodyLines[cursor]);
8181
cursor++;
8282
}
@@ -96,6 +96,12 @@ export function extractRequirementsSection(content: string): RequirementsSection
9696
};
9797
}
9898

99+
export interface ParseDiagnostic {
100+
lineNumber: number;
101+
line: string;
102+
reason: string;
103+
}
104+
99105
export interface DeltaPlan {
100106
added: RequirementBlock[];
101107
modified: RequirementBlock[];
@@ -107,6 +113,12 @@ export interface DeltaPlan {
107113
removed: boolean;
108114
renamed: boolean;
109115
};
116+
diagnostics?: {
117+
added: ParseDiagnostic[];
118+
modified: ParseDiagnostic[];
119+
removed: ParseDiagnostic[];
120+
renamed: ParseDiagnostic[];
121+
};
110122
}
111123

112124
function normalizeLineEndings(content: string): string {
@@ -116,17 +128,26 @@ function normalizeLineEndings(content: string): string {
116128
/**
117129
* Parse a delta-formatted spec change file content into a DeltaPlan with raw blocks.
118130
*/
119-
export function parseDeltaSpec(content: string): DeltaPlan {
131+
export function parseDeltaSpec(content: string, collectDiagnostics = false): DeltaPlan {
120132
const normalized = normalizeLineEndings(content);
121133
const sections = splitTopLevelSections(normalized);
122134
const addedLookup = getSectionCaseInsensitive(sections, 'ADDED Requirements');
123135
const modifiedLookup = getSectionCaseInsensitive(sections, 'MODIFIED Requirements');
124136
const removedLookup = getSectionCaseInsensitive(sections, 'REMOVED Requirements');
125137
const renamedLookup = getSectionCaseInsensitive(sections, 'RENAMED Requirements');
126-
const added = parseRequirementBlocksFromSection(addedLookup.body);
127-
const modified = parseRequirementBlocksFromSection(modifiedLookup.body);
138+
139+
const diagnostics = collectDiagnostics ? {
140+
added: [] as ParseDiagnostic[],
141+
modified: [] as ParseDiagnostic[],
142+
removed: [] as ParseDiagnostic[],
143+
renamed: [] as ParseDiagnostic[],
144+
} : undefined;
145+
146+
const added = parseRequirementBlocksFromSection(addedLookup.body, diagnostics?.added);
147+
const modified = parseRequirementBlocksFromSection(modifiedLookup.body, diagnostics?.modified);
128148
const removedNames = parseRemovedNames(removedLookup.body);
129149
const renamedPairs = parseRenamedPairs(renamedLookup.body);
150+
130151
return {
131152
added,
132153
modified,
@@ -138,6 +159,7 @@ export function parseDeltaSpec(content: string): DeltaPlan {
138159
removed: removedLookup.found,
139160
renamed: renamedLookup.found,
140161
},
162+
diagnostics,
141163
};
142164
}
143165

@@ -169,22 +191,32 @@ function getSectionCaseInsensitive(sections: Record<string, string>, desired: st
169191
return { body: '', found: false };
170192
}
171193

172-
function parseRequirementBlocksFromSection(sectionBody: string): RequirementBlock[] {
194+
function parseRequirementBlocksFromSection(sectionBody: string, diagnostics?: ParseDiagnostic[]): RequirementBlock[] {
173195
if (!sectionBody) return [];
174196
const lines = normalizeLineEndings(sectionBody).split('\n');
175197
const blocks: RequirementBlock[] = [];
176198
let i = 0;
177199
while (i < lines.length) {
178200
// Seek next requirement header
179-
while (i < lines.length && !/^###\s+Requirement:/.test(lines[i])) i++;
201+
while (i < lines.length && !/^###\s*Requirement:/.test(lines[i])) {
202+
// Collect diagnostic for near-misses: lines starting with ### but not matching pattern
203+
if (diagnostics && lines[i].trim().startsWith('###') && !lines[i].trim().startsWith('####')) {
204+
diagnostics.push({
205+
lineNumber: i + 1,
206+
line: lines[i],
207+
reason: 'Line starts with ### but does not match requirement header pattern (### Requirement: ...)'
208+
});
209+
}
210+
i++;
211+
}
180212
if (i >= lines.length) break;
181213
const headerLine = lines[i];
182214
const m = headerLine.match(REQUIREMENT_HEADER_REGEX);
183215
if (!m) { i++; continue; }
184216
const name = normalizeRequirementName(m[1]);
185217
const buf: string[] = [headerLine];
186218
i++;
187-
while (i < lines.length && !/^###\s+Requirement:/.test(lines[i]) && !/^##\s+/.test(lines[i])) {
219+
while (i < lines.length && !/^###\s*Requirement:/.test(lines[i]) && !/^##\s+/.test(lines[i])) {
188220
buf.push(lines[i]);
189221
i++;
190222
}

0 commit comments

Comments
 (0)