fix(validate): warn on ambiguous task numbering - #1523
Conversation
📝 WalkthroughWalkthroughValidation now checks resolved task files for duplicate IDs and mismatches between task IDs and numbered groups. The check reports warnings and supports direct, bulk, strict, non-strict, custom-schema, and deprecated validation flows. ChangesTask numbering validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ValidateCommand
participant Validator
participant TaskFileResolver
participant TaskFiles
ValidateCommand->>Validator: validate change with projectRoot
Validator->>TaskFileResolver: resolveTaskFilesForChange
TaskFileResolver->>TaskFiles: return tracked files or tasks.md
Validator->>TaskFiles: read task content
Validator->>Validator: append numbering warnings
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/validation/task-numbering.ts`:
- Around line 30-52: Update the task-processing loop before parseTaskLines and
duplicate tracking to return when currentGroup is undefined, so numeric-looking
task IDs in unnumbered sections are ignored. Preserve validation for numbered
groups, and add a mixed-file regression test covering a numbered group followed
by an unnumbered section with repeated numeric task text while keeping
unnumbered tasks and sections accepted.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b8c4e38f-52c2-4251-bfa8-37bc427d4082
📒 Files selected for processing (7)
src/commands/change.tssrc/commands/validate.tssrc/core/validation/task-numbering.tssrc/core/validation/validator.tssrc/utils/task-progress.tstest/cli-e2e/validate-task-numbering.test.tstest/core/task-numbering.test.ts
alfred-openspec
left a comment
There was a problem hiding this comment.
Thanks, this is close, but exact head df44f23 still creates false positives: after ## Notes, two 9.1 checkbox lines are reported as a duplicate and make --strict fail, even though tasks outside numbered groups are documented as ignored. Please fix that mixed-section case, detect duplicate IDs across all resolved task files instead of per file, and avoid applying the built-in numbered-task grammar to arbitrary custom schemas until schemas can opt in.
|
Addressed the three review points in
Validation completed with build, lint, 152 focused/related tests, and a combined tree with #1521 (30 tests). The full Windows suite still has the seven existing symlink-permission failures and no additional failures. |
alfred-openspec
left a comment
There was a problem hiding this comment.
The follow-up closes the three review gaps: checks stop outside numbered groups, duplicate IDs are tracked across resolved files with stable paths, and only the packaged spec-driven schema gets this grammar. Fresh install/build/lint, 130 focused validation/task tests, and the hosted matrix pass. Approved at 668eaae.
|
waiting on github to resolve major outage -> https://www.githubstatus.com/ |
Summary
spec-driventask ID is duplicated at full depth, including across resolved task files## N.groupMotivation
openspec validatecurrently never inspects task lists. A duplicate ID makes references such as "task 11.1" ambiguous, while a task filed under a different numbered heading is easy to overlook during long-running changes. Both cases pass even under--strict.Closes #1520.
Design
The analyzer reuses the existing task-line parser and the same task-file resolution used by progress reporting. It receives all resolved task files in stable path order and shares the first location recorded for each complete task ID, so duplicates cannot hide across files.
Numbering is interpreted only while the parser is inside a
## N.group. Entering an unnumbered level-two section clears that context before task parsing and duplicate tracking, so repeated numeric-looking checkbox text under sections such as## Notesremains accepted.The validator resolves the change's effective schema and compares its canonical directory with the packaged
spec-drivenschema before applying this grammar. Project and user overrides, including an override also namedspec-driven, remain untouched until schemas have an explicit opt-in contract.Findings are warnings, so normal validation remains non-blocking and strict validation fails through the existing warning threshold. Duplicate checks compare the complete ID, preserving distinctions such as
3.2.1and3.2.2. Group checks compare only the leading integer. Alphabetic suffixes, numbering gaps, unnumbered tasks, unnumbered sections, and files without any## N.headings remain accepted.The check runs only when validation provides a project root. Archive and existing library callers retain their previous behavior.
Testing
pnpm run buildpnpm run lintThe full suite was also attempted on Windows. Seven symlink tests fail with
EPERMbecause the current account cannot create symlinks; no other full-suite failures were observed.Compatibility
No dependencies, public APIs, schemas, or configuration formats change. Custom schemas retain their current validation behavior. Built-in
spec-driventask lists only become strict-validation failures when they contain one of the two ambiguous numbering patterns inside numbered groups.Summary by CodeRabbit
New Features
Bug Fixes
tasks.md.Tests