Skip to content

Commit d9a88f8

Browse files
yarikopticclaude
andcommitted
speckit.analyze: remediate spec gaps and inconsistencies
Cross-artifact analysis identified 1 CRITICAL, 2 HIGH, 9 MEDIUM, and 4 LOW findings. This commit applies concrete remediations: - Add Phase 11 (tasks T083-T085) for shell completion (FR-019/020/021) which had zero coverage in tasks.md - Expand US10 (Split) from 1 to 4 acceptance scenarios - Resolve all 11 unanswered edge case questions with dispositions - Add remove_run() and split_dataset() to library API contract - Add MigrationResult dataclass to data-model.md - Make SC-003 measurable (5-second benchmark target) - Mark Phase 4 (BIDS 2.0) as PROVISIONAL pending schema stabilization - Update plan.md to reflect actual code structure (_tsv.py, test_cli_common.py) - Annotate FR-016 as specific application of FR-009 Co-Authored-By: Claude Code 2.1.92 / Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: ac75835aa773
1 parent 12e1ab3 commit d9a88f8

5 files changed

Lines changed: 83 additions & 4 deletions

File tree

.specify/specs/00-initial-design.md

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,22 +192,36 @@ A dataset needs to be split — for example, extracting only behavioral data or
192192
**Acceptance Scenarios**:
193193

194194
1. **Given** a valid dataset, **When** `bids-utils split --suffix bold --output bold-only/` is run, **Then** only BOLD-related files (and required metadata) are extracted and the result is a valid BIDS dataset.
195+
2. **Given** a valid dataset, **When** `bids-utils split --datatype anat --output anat-only/` is run, **Then** only anatomical files are extracted, `dataset_description.json` is copied, `participants.tsv` is subset to included subjects, and the result is valid.
196+
3. **Given** a valid dataset, **When** `bids-utils split --suffix bold --dry-run` is run, **Then** the tool lists files that would be extracted without creating any output.
197+
4. **Given** a dataset with inherited metadata (higher-level `.json` sidecars), **When** `bids-utils split --suffix bold --output bold-only/` is run, **Then** inherited metadata that applies to extracted files is preserved in the output (either copied or segregated to leaf level) so the resolved metadata is unchanged.
195198

196199
---
197200

198201
### Edge Cases
199202

200203
- What happens when a rename creates a filename that exceeds OS path length limits?
204+
**Resolution**: Refuse with exit code 2 and a clear error. Covered by FR-011 (refuse invalid state). No extra task needed — implement as a guard in `rename_file()`.
201205
- How does the tool handle symlinked files (common with git-annex)?
206+
**Resolution**: `_vcs.py` GitAnnex backend handles this. Locked annexed files are symlinks; `git annex` commands operate on them correctly. Covered by T017-T018.
202207
- What happens when `_scans.tsv` references files that don't exist on disk (dangling references)?
208+
**Resolution**: Warn but do not fail. Dangling references are a pre-existing dataset issue, not caused by bids-utils. Log at `-v` verbosity.
203209
- How does the tool handle partial datasets (e.g., missing `dataset_description.json`)?
210+
**Resolution**: `BIDSDataset.from_path()` raises an error if no `dataset_description.json` is found. Covered by T013-T014.
204211
- What happens when a file is locked by git-annex and content is needed for metadata operations?
212+
**Resolution**: For metadata read operations, content is needed. If locked and content unavailable, skip that file with a warning. Covered by T017-T018 (VCS tests should include locked-file scenario).
205213
- How does aggregation handle `.nwb` files that embed metadata internally?
214+
**Resolution**: Out of scope. bids-utils operates on BIDS sidecar metadata (`.json` files), not on embedded metadata within data files. NWB internal metadata is outside BIDS's inheritance model.
206215
- What happens when operating on a dataset on a read-only filesystem?
216+
**Resolution**: Operations will fail with a standard OS permission error. No special handling needed — `--dry-run` is always available for read-only inspection.
207217
- How does the tool handle datasets with both `participants.tsv` and `participants.json`?
218+
**Resolution**: `_participants.py` updates `participants.tsv` only. `participants.json` is a sidecar describing column semantics and does not need updating when rows change. Covered by T023-T024.
208219
- How does `migrate` handle a field like `IntendedFor` that uses relative paths but the referenced files don't exist (broken references)?
220+
**Resolution**: Convert the path format to BIDS URI regardless — the migration fixes the format, not the referential integrity. Log a warning about the broken reference. Covered by acceptance scenario US2.8 (ambiguous cases skipped with clear reporting).
209221
- How does `migrate` handle deprecated metadata fields that appear in inherited (higher-level) JSON sidecars vs. leaf-level ones?
222+
**Resolution**: Migrate the field wherever it appears. The inheritance chain is not changed — if `BasedOn` appears in a root-level sidecar, it is renamed to `Sources` there. Covered by T031-T038.
210223
- What happens when migrating `ScanDate` to `_scans.tsv` but no `_scans.tsv` exists yet for that subject/session?
224+
**Resolution**: Create the `_scans.tsv` with the appropriate header and populate the `acq_time` column. Explicitly covered by T036.
211225

212226
## Clarifications
213227

@@ -237,7 +251,7 @@ A dataset needs to be split — for example, extracting only behavioral data or
237251
- **FR-013**: System MUST support `-v` / `-q` verbosity controls.
238252
- **FR-014**: System MUST support `--include-sourcedata` flag for operations that can extend to `sourcedata/` and `.heudiconv/`.
239253
- **FR-015**: Sidecar discovery MUST handle all BIDS-recognized sidecar extensions (`.json`, `.bvec`, `.bval`, `.tsv` for events, etc.) based on the schema.
240-
- **FR-016**: `migrate` MUST derive all deprecation knowledge from the `bidsschematools` machine-readable schema (deprecation rules, metadata definitions, enum definitions) — not from hardcoded migration tables.
254+
- **FR-016**: `migrate` MUST derive all deprecation knowledge from the `bidsschematools` machine-readable schema (deprecation rules, metadata definitions, enum definitions) — not from hardcoded migration tables. *(Specific application of FR-009 to the migration subsystem.)*
241255
- **FR-017**: `migrate` MUST default to the current released BIDS version when no `--to` target is specified, and MUST support explicit `--to` for both 1.x and 2.0 targets.
242256
- **FR-018**: `migrate` MUST apply migrations cumulatively — migrating from 1.4 to 1.9 applies all intermediate deprecation fixes in version order.
243257
- **FR-019**: System MUST provide a `bids-utils completion [SHELL]` subcommand that outputs shell completion activation scripts. When `SHELL` argument is omitted, auto-detect from the `$SHELL` environment variable. Supported shells: Bash, Zsh, Fish (matching Click 8.0+ built-in completion support). Output goes to stdout only (no `--install` flag).
@@ -259,7 +273,7 @@ A dataset needs to be split — for example, extracting only behavioral data or
259273

260274
- **SC-001**: Every bids-examples dataset that is valid before a `rename`/`subject-rename`/`session-rename` operation is still valid after the operation completes.
261275
- **SC-002**: `--dry-run` output for every command matches the actual changes when run without `--dry-run` (verified by comparing dry-run output to actual filesystem diff).
262-
- **SC-003**: All commands complete on a 1000-subject dataset within reasonable time (no pathological performance cliffs — O(n) in number of affected files, not O(n²) in total dataset size).
276+
- **SC-003**: All commands complete on a 1000-subject dataset in O(n) time relative to affected files (not O(n²) in total dataset size). Single-entity operations (rename, remove-run) must not scan the entire dataset. Benchmark target: `rename` on a single file in a 1000-subject dataset completes in under 5 seconds.
263277
- **SC-004**: Library API is independently usable: all acceptance scenarios can be executed via Python imports without the CLI.
264278
- **SC-005**: 100% of mutating commands have both `--dry-run` and `--json` modes tested in CI.
265279
- **SC-006**: Test suite passes against at least 3 different BIDS schema versions (e.g., 1.8, 1.9, 2.0-dev).

.specify/specs/00-initial-design/contracts/library-api.md

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,38 @@ def audit_metadata(
114114
"""Report metadata inconsistencies."""
115115
```
116116

117+
### `bids_utils.run`
118+
119+
```python
120+
def remove_run(
121+
dataset: BIDSDataset,
122+
subject: str,
123+
run: str,
124+
*,
125+
suffix: str | None = None,
126+
task: str | None = None,
127+
session: str | None = None,
128+
shift: bool = True,
129+
dry_run: bool = False,
130+
force: bool = False,
131+
) -> OperationResult:
132+
"""Remove a run and optionally reindex subsequent runs."""
133+
```
134+
135+
### `bids_utils.split`
136+
137+
```python
138+
def split_dataset(
139+
dataset: BIDSDataset,
140+
target: str | Path,
141+
*,
142+
suffixes: list[str] | None = None,
143+
datatypes: list[str] | None = None,
144+
dry_run: bool = False,
145+
) -> OperationResult:
146+
"""Extract a subset of a dataset by suffix/datatype filter."""
147+
```
148+
117149
### `bids_utils.merge`
118150

119151
```python

.specify/specs/00-initial-design/data-model.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,4 +196,14 @@ class MigrationFinding:
196196
proposed_value: Any
197197
can_auto_fix: bool # False if human judgment needed
198198
reason: str | None # Why it can't be auto-fixed (if applicable)
199+
200+
@dataclass
201+
class MigrationResult:
202+
"""Result of migrate_dataset(), extends MigrationPlan with outcome."""
203+
plan: MigrationPlan
204+
success: bool
205+
dry_run: bool
206+
applied: list[MigrationFinding] # Findings that were auto-fixed
207+
skipped: list[MigrationFinding] # Findings requiring human judgment
208+
errors: list[str]
199209
```

.specify/specs/00-initial-design/plan.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ src/bids_utils/
6464
├── _types.py # Shared type definitions (PathLike, Entity, etc.)
6565
├── _vcs.py # VCS detection and operations (git mv, git annex, datalad)
6666
├── _schema.py # Schema loading and querying helpers (wraps bidsschematools)
67+
├── _tsv.py # Shared TSV read/write utilities (used by _scans.py, _participants.py)
6768
├── _scans.py # _scans.tsv read/write/update operations
6869
├── _participants.py # participants.tsv read/write/update operations
6970
├── _sidecars.py # Sidecar discovery (find all associated files for a BIDS file)
@@ -100,6 +101,8 @@ tests/
100101
├── test_run.py # Run removal tests
101102
├── test_vcs.py # VCS integration tests
102103
├── test_cli.py # CLI smoke tests
104+
├── test_cli_common.py # Tests for shared CLI options/decorators
105+
├── test_tsv.py # Tests for shared TSV utilities
103106
└── integration/
104107
└── test_bids_examples.py # Sweep tests against bids-examples
105108
```

.specify/specs/00-initial-design/tasks.md

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,8 @@
132132

133133
**Note**: Exact 2.0 transformations depend on BIDS 2.0 schema stabilization. This phase may iterate.
134134

135+
**⚠ PROVISIONAL**: Tasks T041-T045 are marked complete but their implementations are necessarily preliminary — they target the current 2.0-dev schema which is not yet finalized. These tasks will likely need re-implementation when the BIDS 2.0 schema stabilizes. Track upstream progress and re-validate.
136+
135137
---
136138

137139
## Phase 5: User Story 4 — Rename a Subject (Priority: P2)
@@ -256,7 +258,23 @@
256258

257259
---
258260

259-
## Phase 11: Polish & Cross-Cutting Concerns
261+
## Phase 11: Shell Completion (FR-019, FR-020, FR-021)
262+
263+
**Purpose**: `bids-utils completion` subcommand with BIDS-aware completions.
264+
265+
**Independent Test**: Run `bids-utils completion bash | source /dev/stdin`, verify tab-completion offers `sub-*`, `ses-*` directories and entity keys.
266+
267+
### Implementation
268+
269+
- [ ] T083 [P] Implement `src/bids_utils/cli/completion.py`: `bids-utils completion [SHELL]` click command — auto-detect shell from `$SHELL`, output activation script to stdout. Supported: Bash, Zsh, Fish (Click 8.0+ built-in).
270+
- [ ] T084 Implement BIDS-aware custom completions: filesystem-derived items (`sub-*` directories, `ses-*` directories, BIDS file paths) and entity keys from schema (`task=`, `run=`, `acq=`). Uses `_dataset.py` for dataset root resolution (FR-020: honor `--dataset` or walk up from CWD to `dataset_description.json`).
271+
- [ ] T085 Write tests for completion in `tests/test_cli.py` or `tests/test_completion.py`: `bids-utils completion --help`, shell detection, activation script output for each shell, BIDS-aware completion produces expected items
272+
273+
**Checkpoint**: `bids-utils completion` outputs working activation scripts with BIDS-aware completions.
274+
275+
---
276+
277+
## Phase 12: Polish & Cross-Cutting Concerns
260278

261279
**Purpose**: Improvements that affect multiple user stories.
262280

@@ -285,7 +303,8 @@
285303
- **Phase 8 (Remove / US7-8)**: Depends on Phase 2
286304
- **Phase 9 (Merge / US9)**: Depends on Phases 5, 6 (uses subject/session rename)
287305
- **Phase 10 (Split / US10)**: Depends on Phase 1
288-
- **Phase 11 (Polish)**: Depends on all desired phases being complete
306+
- **Phase 11 (Completion / FR-019-021)**: Depends on Phase 1 (uses `_dataset.py`, `_schema.py`)
307+
- **Phase 12 (Polish)**: Depends on all desired phases being complete
289308

290309
### Parallel Opportunities After Phase 1
291310

@@ -298,6 +317,7 @@ Phase 2 (Rename) ─→ Phase 3 (Migrate 1.x) ─→ Phase 4 (Migrate 2.0)
298317
─→ Phase 8 (Remove)
299318
Phase 7 (Metadata) can start immediately after Phase 1
300319
Phase 10 (Split) can start immediately after Phase 1
320+
Phase 11 (Completion) can start immediately after Phase 1
301321
```
302322

303323
### Within Each Phase

0 commit comments

Comments
 (0)