spec: address /speckit.analyze 2026-05-09 findings (I1, I2, U2, U3, G1)

Five remediations from the third /speckit.analyze pass on the
000-initial-design feature. None of these change implemented behavior;
they correct artifacts that have drifted from the canonical FR
definitions, split misleading [X] task statuses, and tighten
under-specified clauses before the Phase 2a/2b implementation phase
starts.

I1 (CRITICAL) — DCOffsetCorrection migration semantics
------------------------------------------------------

Three artifacts contradicted FR-031, which is the canonical rule:

- spec.md:44 listed `DCOffsetCorrection → deprecated` (suggests
  deletion); should be `→ SoftwareFilters` (structural lossless move
  at level `safe`). Updated.
- plan.md:308 listed it under "Field removal" advisory level; should
  be a structural-move handler at safe level. Moved into the safe-
  level handler list with explicit nested-dict semantics.
- tasks.md T110 asserted "DCOffsetCorrection removed at advisory
  level"; now asserts safe-level migration into
  `SoftwareFilters.DCOffsetCorrection.description`, including the
  merge-into-existing-dict case.

I2 (HIGH) — false [X] on three migration tasks
----------------------------------------------

T032, T036, T105 were marked complete despite their own descriptions
admitting unfinished bugs ("merge logic broken for mixed string/array
types", "data loss bug", "current code removes field instead of
migrating"). Constitution Principle I (Do No Harm) prohibits silently
shipping known data-loss bugs.

Resolution: keep the original [X] (the work that *was* done is real),
update each description to point to its bug-fix follow-up, and add a
new "Phase 3c: Migration Handler Bug Fixes (Audit Follow-up)" section
with three [ ] tasks:

- T147 — fix Sources merge for mixed string/array types (T032 fix).
- T148 — create _scans.tsv when missing in ScanDate move (T036 fix).
- T149 — implement actual SoftwareFilters move (T105 / FR-031 fix).

This makes status honest while preserving the audit trail of what
landed in commit 6fb17e3 / PR #7.

U2 (HIGH) — FR-039 "container-implied entities" wording
-------------------------------------------------------

FR-039 said cross-container `rename` rewrites "the `sub-`/`ses-` (and
any other container-implied) entity labels". "Any other container-
implied" was undefined — BIDS has no other directory-level entities
(`sample-` is filename-only, `acq-` / `task-` are not directory-
level). Narrowed to "the `sub-` and `ses-` entity labels" with an
explicit note that no other entities are inferred from the destination
path. Closes a future scope-drift hole in T132.

U3 (HIGH) — atomic-batch + VCS unlock ordering
----------------------------------------------

FR-043(c) and SC-010 promised "byte-for-byte unchanged" filesystem on
batch refusal but were silent on whether `git annex unlock` could fire
during precondition validation. The implementation hazard: a batch
that runs `unlock()` against the symlink graph (to inspect content)
before discovering a target conflict would mutate `.git/annex/objects`
on a refused batch, violating the atomicity guarantee.

FR-043(c) now mandates: precondition checks (target conflict
detection, sidecar discovery, schema validation, `_scans.tsv` row
resolution) MUST complete for every source in the batch before ANY
`vcs.unlock()` / `vcs.move()` / `os.rename()` call. On git-annex,
precondition validation runs purely against the symlink graph — no
unlock to inspect content; only after batch acceptance does the
executor begin issuing mutations.

Two implementation consequences pulled into the spec at the same time:

- (c1) The planner MUST detect intra-batch target collisions (two
  sources rewriting to the same destination — possible under cross-
  container `rename` with multiple sources, FR-039).
- (c2) When `SRC` globs overlap with discovered sidecars (e.g., a
  `*.json` glob matches a sidecar that is also discovered for another
  `SRC`'s primary), the planner MUST collapse the duplicate into a
  single rename rather than refusing or double-renaming.

SC-010(b) updated to operationalize "byte-for-byte unchanged" as a
`git status --porcelain` snapshot plus per-file
`(relpath, size, mtime, sha256)` — and to mandate that the same test
also runs on a git-annex worktree (using `bids_examples_worktree_annexed`)
to prove no `git annex unlock` fired before the batch refused.

G1 (HIGH) — foundational FR coverage tagging
--------------------------------------------

FR-001 through FR-018 had zero explicit task tags in `tasks.md` —
coverage was inferred only. The earlier spec rounds added explicit
`(FR-NNN)` tags for FR-019..FR-043, leaving the foundational FRs as a
gap a reviewer could only fill by guessing.

Bulk-tagged the canonical task per foundational FR (FR-001 → T011,
FR-002 → T005, FR-004 → T017, FR-005 → T021, FR-006 → T023, FR-009
→ T015, FR-010 → T013, FR-011 → T026, FR-012 → T062, FR-015 → T019,
FR-016 → T031, FR-017+FR-018 → T037, FR-007+FR-008+FR-013+FR-014 →
T028). FR-003 was already tagged at the Phase 1c phase header.
Coverage table from the audit is now traceable end-to-end.

Verified: no code changes; lint / type / duplication unaffected. The
new T147–T149 tasks are [ ] (open), so no behavioral claim shifts.

Co-Authored-By: Claude Code 2.1.128 / Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 4b7f5469f924

Author: yarikoptic

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

@@ -41,7 +41,7 @@ A lab maintains a BIDS dataset created under an older 1.x version (e.g., 1.4 or
 
 The BIDS specification has accumulated significant deprecations within the 1.x series that `migrate` must handle:
 
-- **Metadata field replacements**: `BasedOn` → `Sources`, `RawSources` → `Sources`, `ScanDate` → `acq_time` column in `_scans.tsv` (PET, since 1.6.0), `DCOffsetCorrection` → deprecated (iEEG, `rules/sidecars/ieeg`), `AcquisitionDuration` → `FrameAcquisitionDuration` (BOLD/ASL, `rules/sidecars/func` + `rules/checks/func`), `HardcopyDeviceSoftwareVersion` → deprecated (MRI, `rules/sidecars/mri`)
+- **Metadata field replacements**: `BasedOn` → `Sources`, `RawSources` → `Sources`, `ScanDate` → `acq_time` column in `_scans.tsv` (PET, since 1.6.0), `DCOffsetCorrection` → `SoftwareFilters` (iEEG, `rules/sidecars/ieeg`; structural lossless migration at level `safe`, see FR-031), `AcquisitionDuration` → `FrameAcquisitionDuration` (BOLD/ASL, `rules/sidecars/func` + `rules/checks/func`), `HardcopyDeviceSoftwareVersion` → deprecated (MRI, `rules/sidecars/mri`)
 - **Path format → BIDS URI migration** (since 1.8.0): `IntendedFor`, `AssociatedEmptyRoom`, `Sources` fields that use relative paths must be converted to BIDS URIs (`bids::` scheme)
 - **Value format changes**: `DatasetDOI` bare DOIs → URI format (since 1.8.0)
 - **Suffix deprecations** (since 1.5.0): `_phase` → `_part-phase_bold`, and deprecated anatomical suffixes `T2star`, `FLASH`, `PD`
@@ -348,11 +348,11 @@ Refinement of Session 2026-04-27 after initial implementation review. Two follow
 - **FR-035**: `BIDSPath.to_filename()` MUST emit entities in the order defined by the BIDS schema (`BIDSSchema.entity_order()`), not in Python dict insertion order. When a `BIDSSchema` is available (e.g., via `BIDSDataset`), any operation that mutates entity values in a filename (the new `edit-filename` command, the implicit entity-rewrite path of `rename SRC DST` when `DST` crosses subject/session containers, `subject-rename`, `session-rename`, `migrate`) MUST reorder entities before writing the new filename. This ensures `edit-filename ... --set run=99` produces `sub-01_task-rest_run-99_recording-bipolar_emg.edf` (correct order) not `sub-01_task-rest_recording-bipolar_run-99_emg.edf`.
 - **FR-036**: File iteration in rename, subject-rename, session-rename, and all operations that enumerate BIDS files MUST also match BIDS **directory-based data files** — specifically `.ds` directories (CTF MEG) and `.zarr`/`.ome.zarr` directories (microscopy/Zarr). These directories carry entity labels in their name and MUST be renamed alongside regular files. The current `not path.is_dir()` filter incorrectly skips them.
 - **FR-038**: `find_sidecars()` MUST be robust to non-BIDS-named source files (e.g., heudiconv duplicate-run output `..._bold__dup-01.nii.gz`). For such files, sidecar discovery MUST use the **full literal stem** of the source — only files sharing that exact stem (e.g., `..._bold__dup-01.json`, `..._bold__dup-01.bvec`) are treated as sidecars. The function MUST NOT attempt to canonicalize or strip non-BIDS trailing segments before stem comparison. (Closes part of bids-standard/bids-utils#14.)
-- **FR-039**: `bids-utils rename SRC [SRC ...] DST` MUST implement mv-like rename semantics over BIDS files. Single-source form (`rename SRC DST`) renames the primary file, its sidecars (using FR-038 stem rules so that non-BIDS source filenames like heudiconv `__dup-NN` are handled), updates `_scans.tsv` rows referencing the old filename, and uses `git mv` (or annex-equivalent) when the dataset is under VCS. Multi-source form (`rename SRC SRC ... DST`) is permitted when `DST` is an existing directory and is governed by the batch contract in FR-043. `rename` MUST NOT support entity-mutation flags `--set` / `--delete` / `--suffix` / `--set-suffix` — those live in `edit-filename` (FR-040); passing them MUST yield a non-zero exit and a hint pointing to `edit-filename`. When `DST` (or a multi-source destination directory) lives under a different `sub-XX/[ses-YY]/` container than the source(s), `rename` MUST detect the cross-container move and rewrite the `sub-`/`ses-` (and any other container-implied) entity labels in the destination filename(s) to match the destination path, delegating to `edit-filename`'s entity-rewrite core. The result MUST satisfy entity-order normalization (FR-035). Conflicts are refused with exit code 2 (FR-011) per the atomic-batch rules of FR-043.
+- **FR-039**: `bids-utils rename SRC [SRC ...] DST` MUST implement mv-like rename semantics over BIDS files. Single-source form (`rename SRC DST`) renames the primary file, its sidecars (using FR-038 stem rules so that non-BIDS source filenames like heudiconv `__dup-NN` are handled), updates `_scans.tsv` rows referencing the old filename, and uses `git mv` (or annex-equivalent) when the dataset is under VCS. Multi-source form (`rename SRC SRC ... DST`) is permitted when `DST` is an existing directory and is governed by the batch contract in FR-043. `rename` MUST NOT support entity-mutation flags `--set` / `--delete` / `--suffix` / `--set-suffix` — those live in `edit-filename` (FR-040); passing them MUST yield a non-zero exit and a hint pointing to `edit-filename`. When `DST` (or a multi-source destination directory) lives under a different `sub-XX/[ses-YY]/` container than the source(s), `rename` MUST detect the cross-container move and rewrite the `sub-` and `ses-` entity labels in the destination filename(s) to match the destination path, delegating to `edit-filename`'s entity-rewrite core. The cross-container rewrite is **scoped to `sub-` and `ses-`** — no other entities (e.g., `acq-`, `task-`, `sample-`) are inferred from the destination path, since BIDS does not assign them directory-level meaning. The result MUST satisfy entity-order normalization (FR-035). Conflicts are refused with exit code 2 (FR-011) per the atomic-batch rules of FR-043.
 - **FR-040**: System MUST provide a `bids-utils edit-filename SRC [SRC ...] --set KEY=VALUE [--delete KEY ...] [--set-suffix SUFFIX]` command that performs in-place edits to one or many BIDS files' filenames — both the entity portion AND the suffix — and to their sidecars / `_scans.tsv` references, **without** changing each file's directory. `edit-filename` is the canonical home for the `--set` / `--delete` / `--set-suffix` semantics formerly attached to `rename`. (The command is named `edit-filename`, not `edit-entities`, because it covers the suffix in addition to entities.) Output filenames MUST satisfy entity-order normalization (FR-035). `edit-filename` MUST be robust to non-BIDS source filenames (FR-038), preserving any unrecognized non-BIDS trailing segments (e.g., `__dup-NN`) verbatim unless the user explicitly asks for normalization (deferred to FR-037). Batch / glob input (multiple `SRC` paths spanning multiple files / subjects / sessions) MUST be supported per FR-043. Conflicts are refused with exit code 2 (FR-011).
 - **FR-041**: The following file-touching commands MUST operate correctly on non-BIDS-named source files (e.g., heudiconv `..._bold__dup-01.nii.gz`, files with stray `_test` suffixes): `rename`, `edit-filename`, `remove`, `subject-rename`, `session-rename`. Each MUST: (a) discover sidecars via FR-038 full-literal-stem matching, (b) include such files in iteration (FR-023, FR-036), (c) update `_scans.tsv` rows that reference them, and (d) preserve any unrecognized non-BIDS trailing segments verbatim except where the user explicitly opts in to normalization (FR-037). `migrate` is **out of scope** for this requirement in the initial release. Each of the five listed commands MUST have integration tests exercising at least one heudiconv-style `__dup-NN` fixture and at least one stray-non-BIDS-suffix fixture (see FR-042 / SC-009).
 - **FR-042**: The integration test suite MUST provide a generic `bids-examples`-based fixture pattern using **per-test git worktrees**: from the local `bids-examples` clone, `git worktree add` a unique throwaway worktree under `tmp_path`, run the operation under test inside it, then `git worktree remove` it on teardown (no in-place mutation of the shared clone). This SHOULD replace the existing `_copy_dataset` / `git reset --hard` pattern for new tests. Additionally, a fixture helper MUST be available to inject synthetic non-BIDS filename suffixes — heudiconv-style `__dup-NN`, plus adversarial cases such as `+mine`, `--crap`, and stray `_test` — into a deterministic (seeded) selection of files within the worktree, so the FR-041 commands can be exercised against realistic non-BIDS source filenames without requiring a real heudiconv/DICOM toolchain in CI.
-- **FR-043**: Both `rename` and `edit-filename` MUST accept **batch / glob input** — multiple `SRC` positional arguments (typically supplied via shell-glob expansion) that may span multiple files, subjects, and sessions in a single invocation. (a) For `edit-filename`, the same edit (`--set`, `--delete`, `--set-suffix`) is applied to every matched source. (b) For `rename`, multi-source input is permitted when the destination is an existing directory (mirrors Unix `mv SRC ... DST/`). (c) Every batch operation MUST be **atomic at the batch level**: every target path is computed and validated up front; if ANY target conflicts (existing file, schema violation, etc.) the entire batch is refused with exit code 2 *before any filesystem mutation occurs* — no partial completion. (d) `_scans.tsv` updates MUST be coalesced per `(subject, session)` container so a batch produces one update per scans file, not one per source. (e) The library API MUST mirror the CLI: `rename_file()` and `edit_filename()` accept a single path or a sequence of paths (`PathArg = PathLike | Sequence[PathLike]`). (f) Per Constitution Principle VIII (Observability), each per-source change MUST appear as its own `Change` entry in `OperationResult.changes`, and `--dry-run=detailed` MUST list every per-source change individually — batch input does not coalesce the user-facing change log even though it does coalesce `_scans.tsv` writes.
+- **FR-043**: Both `rename` and `edit-filename` MUST accept **batch / glob input** — multiple `SRC` positional arguments (typically supplied via shell-glob expansion) that may span multiple files, subjects, and sessions in a single invocation. (a) For `edit-filename`, the same edit (`--set`, `--delete`, `--set-suffix`) is applied to every matched source. (b) For `rename`, multi-source input is permitted when the destination is an existing directory (mirrors Unix `mv SRC ... DST/`). (c) Every batch operation MUST be **atomic at the batch level**: every target path is computed and validated up front; if ANY target conflicts (existing file, schema violation, etc.) the entire batch is refused with exit code 2 *before any filesystem mutation occurs* — no partial completion. **Mutation-before-precondition is forbidden**: precondition checks (target conflict detection, sidecar discovery, schema validation, `_scans.tsv` row resolution) MUST complete for every source in the batch before ANY `vcs.unlock()`, `vcs.move()`, or filesystem `os.rename()` call. On a git-annex worktree this means precondition validation runs purely against the symlink graph (no `unlock` to inspect content); only after the whole batch is accepted does the executor begin issuing `unlock` / `move` / `add` calls. Two implementation consequences: (c1) the planner MUST also detect intra-batch target collisions (two distinct sources rewriting to the same destination filename — possible under cross-container `rename` with multiple sources, FR-039); (c2) when `SRC` globs overlap with discovered sidecars (e.g., a `*.json` glob matches a sidecar that is also discovered for another `SRC`'s primary file), the planner MUST collapse the duplicate into a single rename rather than refusing or double-renaming. (d) `_scans.tsv` updates MUST be coalesced per `(subject, session)` container so a batch produces one update per scans file, not one per source. (e) The library API MUST mirror the CLI: `rename_file()` and `edit_filename()` accept a single path or a sequence of paths (`PathArg = PathLike | Sequence[PathLike]`). (f) Per Constitution Principle VIII (Observability), each per-source change MUST appear as its own `Change` entry in `OperationResult.changes`, and `--dry-run=detailed` MUST list every per-source change individually — batch input does not coalesce the user-facing change log even though it does coalesce `_scans.tsv` writes.
 - **FR-037** *(deferred / future work — TODO)*: System SHOULD provide a dedicated `bids-utils normalize` (or similarly named) command that performs **canonicalization / sanitization** of BIDS dataset filenames and metadata. Anticipated scope (to be designed in a follow-up clarification round):
    - Reordering entity key-value pairs to `BIDSSchema.entity_order()` (currently auto-handled per FR-035 inside `BIDSPath.to_filename()`).
    - Sanitizing entity-label values (e.g., replacing forbidden characters in label values such as `-` with `+`, per the approach used in the `nwb2bids` project; ensuring labels are alphanumeric).
@@ -384,7 +384,7 @@ Refinement of Session 2026-04-27 after initial implementation review. Two follow
 - **SC-006**: Test suite passes against at least 3 different BIDS schema versions (e.g., 1.8, 1.9, 2.0-dev).
 - **SC-007**: `migrate` eliminates all deprecation warnings when run on bids-examples datasets created under older schema versions (verified by running the BIDS validator before and after).
 - **SC-009**: For each of `rename`, `edit-filename`, `remove`, `subject-rename`, `session-rename`, the integration test suite (using the worktree fixture pattern from FR-042) MUST exercise at least one heudiconv-style `__dup-NN` source file and at least one adversarial non-BIDS-suffix source file (`+mine`, `--crap`, or stray `_test`), and each test MUST: (i) **pre-validate** the worktree post-injection and assert the validator flags the injected non-BIDS file(s) — guaranteeing the fixture genuinely produced an invalid dataset; (ii) run the operation under test; (iii) **post-validate** and assert the validator passes (or only emits pre-existing unrelated warnings unchanged from before injection), so the synthetic non-BIDS files are renamed/removed cleanly rather than silently skipped. Validator-dependent assertions skip gracefully when `bids-validator-deno` is not installed.
-- **SC-010**: Both `rename` and `edit-filename` MUST be tested under **batch / glob input** spanning multiple subjects in a single invocation: at minimum (a) a `bids-utils edit-filename sub-*/func/sub-*_task-rest_bold.nii.gz --set task=nback` over a worktree fixture with ≥2 subjects yields a still-valid dataset; (b) a deliberate batch with one conflicting target is refused with exit code 2 and the filesystem is **byte-for-byte unchanged** before and after the failed call (atomic-batch verification, FR-043).
+- **SC-010**: Both `rename` and `edit-filename` MUST be tested under **batch / glob input** spanning multiple subjects in a single invocation: at minimum (a) a `bids-utils edit-filename sub-*/func/sub-*_task-rest_bold.nii.gz --set task=nback` over a worktree fixture with ≥2 subjects yields a still-valid dataset; (b) a deliberate batch with one conflicting target is refused with exit code 2 and the filesystem is **byte-for-byte unchanged** before and after the failed call (atomic-batch verification, FR-043). The "byte-for-byte unchanged" check is operationalized as: snapshot pre = `git status --porcelain` plus `(relpath, size, mtime, sha256)` for every regular file under the worktree (excluding `.git/`); the post snapshot MUST equal the pre snapshot. The same test MUST also run on a **git-annex** worktree (using `bids_examples_worktree_annexed`) to prove that no `git annex unlock` was performed before the batch refused (FR-043(c)).
 
 ## Assumptions
 

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

@@ -304,8 +304,8 @@ tests/
    - **Cross-file moves**: `ScanDate` → `acq_time` in `_scans.tsv`
    - **Conditional field rename**: `AcquisitionDuration` → `FrameAcquisitionDuration` (only when `VolumeTiming` present) (FR-026)
    - **TSV column value**: `"89+"` string → numeric `89` in `participants.tsv` `age` column, unit-aware (FR-027)
+   - **Structural field move**: `DCOffsetCorrection` → nested entry within `SoftwareFilters` dict (iEEG, FR-031). Lossless: value is preserved under `SoftwareFilters.DCOffsetCorrection.description`, original key is removed. Merges into pre-existing `SoftwareFilters` dict if present.
 5. Implement transformation handlers — **advisory level** (opt-in via `--level=advisory`):
-   - **Field removal**: `DCOffsetCorrection` deprecated in iEEG (FR-031)
    - **Field removal**: `HardcopyDeviceSoftwareVersion` deprecated in MRI (FR-032)
    - **HIPAA age cap**: numeric values > 89 → `89` in `age` column (rule id: `age_cap_89`)
    - **Conditional field rename (ambiguous)**: `AcquisitionDuration` without `VolumeTiming` — prompt user