migrate: dedup Sources merge + verify ScanDate/DCOffsetCorrection (T147-T149)

Phase 3c (Migration Handler Bug Fixes / Audit Follow-up). The three
follow-up tasks added in commit 2e3afc1 each carried a description
inherited from the analyzer's reading of T032 / T036 / T105's
"*Reimplementation needed*" tails. Investigating each:

T147 — Sources merge dedup
--------------------------

The mixed-string/array-type bug T147 was written against had already
been resolved in a prior commit; tests `test_sources_merge_mixed_types`
and `test_sources_three_way_merge_mixed_types` covered it. What was
NOT covered: FR-029's order-preserving deduplication on merge — a URI
present in both the destination `Sources` and the incoming `BasedOn` /
`RawSources` was emitted twice. Fixed `_apply_field_rename()` to
deduplicate while preserving order (the existing list keeps its order;
incoming items are appended only if not already present). Added two
new tests in `TestFieldRename`:

- `test_sources_array_plus_string_merge` — array-Sources + string-
  RawSources case (regression coverage; was missing).
- `test_sources_merge_dedups_duplicates` — FR-029 dedup contract.

T148 — ScanDate move when _scans.tsv missing
--------------------------------------------

The originally-described silent-data-loss bug ("ScanDate removed from
JSON but never written to TSV when the TSV does not exist") was
already fixed in a prior commit; existing tests
`test_scandate_creates_scans_tsv_when_missing` and
`test_scandate_creates_scans_tsv_with_session` cover both the no-
session and with-session paths and pass against the current handler.
T148 verified those tests pass and updated the task description to
accurately reflect the verification status (rather than claiming a
new fix). No code change in `migrate.py`.

T149 — DCOffsetCorrection structural migration
----------------------------------------------

The originally-described "removes field instead of migrating" bug was
already fixed; existing tests `test_standalone_dcoffset_nested_under_softwarefilters`,
`test_merge_into_existing_softwarefilters_dict`, and
`test_non_ieeg_sidecar_not_migrated` cover the FR-031 contract and
pass at level `safe` (not `advisory`). T149 added one hardening test:

- `test_dcoffset_migration_is_idempotent` — running `migrate` twice
  on the same dataset produces the same result; no double-nesting,
  no re-emit of the original key.

No code change in `migrate.py`.

Status correctness
------------------

T147 / T148 / T149 are now [X] in tasks.md. The misleading
"*Reimplementation needed*" tail on T032 / T036 / T105 (introduced
before the underlying fixes landed) remains as an audit-trail
breadcrumb pointing at the follow-up tasks; commit 2e3afc1's I2
remediation already framed those as "original implementation [X];
follow-up tracked in T147/T148/T149", which is now true.

Verified: `tox -e lint,type,duplication` clean; `tox -e py311 --
tests/test_migrate.py` 48/48 pass; full `tox -e py311` running.

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

Author: yarikoptic

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

@@ -494,9 +494,9 @@
 
 **Purpose**: Three Phase 3 tasks (T032, T036, T105) shipped with `[X]` despite their own descriptions admitting unfinished bugs (analyzer findings I1, I2, 2026-05-09). Splitting the bug fixes out makes status honest and gives them their own coverage / acceptance criteria.
 
-- [ ] T147 [US2] Fix `Sources` merge logic in `src/bids_utils/migrate.py` (follow-up to T032). When the destination `Sources` field exists as a **string** and the incoming `BasedOn` / `RawSources` value is an **array** (or vice versa), the current handler raises or drops one side. Normalize both sides to a list before merging; preserve duplicate-removal semantics (FR-029). Add `tests/test_migrate.py` cases: (a) string + array merge, (b) array + string merge, (c) 3-way merge (`BasedOn` + `RawSources` + pre-existing `Sources`) with mixed types in any of the three.
-- [ ] T148 [US2] Fix `ScanDate` cross-file move in `src/bids_utils/migrate.py` (follow-up to T036). When `_scans.tsv` does not exist for the target session, the handler currently removes `ScanDate` from the JSON sidecar without writing the TSV — a data-loss bug. The fixed handler MUST create `_scans.tsv` with the canonical `filename\tacq_time` header (extending with extra columns is fine if needed) and write the row before clearing the JSON field. Add `tests/test_migrate.py` case for the missing-`_scans.tsv` path; assert the value lands in the new TSV.
-- [ ] T149 [US2] Fix `DCOffsetCorrection` → `SoftwareFilters` migration in `src/bids_utils/migrate.py` (follow-up to T105, FR-031). The current handler **removes** the field; the spec mandates a structural lossless move into `SoftwareFilters: {DCOffsetCorrection: {description: VALUE}}` at level `safe` (not `advisory`). Implement the move + merge-into-existing-dict semantics. Add `tests/test_migrate.py` cases: (a) standalone `DCOffsetCorrection` → expected nested dict, (b) `DCOffsetCorrection` alongside pre-existing `SoftwareFilters` → entry merged, no overwrite, (c) verify the original field is absent post-migration. Update T110's iEEG case (it now asserts safe-level structural migration, not advisory removal).
+- [X] T147 [US2] Fix `Sources` merge logic in `src/bids_utils/migrate.py` (follow-up to T032). The handler at `_apply_field_rename()` already normalized mixed string/array types (existing tests `test_sources_merge_mixed_types` and `test_sources_three_way_merge_mixed_types` in `tests/test_migrate.py` cover that path), but FR-029's duplicate-removal semantics were not enforced — a URI present in both the destination `Sources` and incoming `BasedOn`/`RawSources` was emitted twice. Resolution: extend `_apply_field_rename()` with order-preserving deduplication on merge; add `test_sources_array_plus_string_merge` and `test_sources_merge_dedups_duplicates` in `TestFieldRename` to cover (a) array+string regression and (b) FR-029 dedup.
+- [X] T148 [US2] Verify `ScanDate` cross-file move in `src/bids_utils/migrate.py` (follow-up to T036). The originally-described data-loss bug (silently skipping the TSV write when `_scans.tsv` does not exist) was already fixed in a prior commit; existing tests `test_scandate_creates_scans_tsv_when_missing` and `test_scandate_creates_scans_tsv_with_session` cover both the no-session and with-session missing-TSV paths. T148 verified those tests pass against the current handler. The misleading "*Reimplementation needed*" tail on T036's description (introduced before the fix landed) is the only artefact still pointing at a bug that no longer exists.
+- [X] T149 [US2] Verify `DCOffsetCorrection` → `SoftwareFilters` migration in `src/bids_utils/migrate.py` (follow-up to T105, FR-031). The originally-described "removes field instead of migrating" bug was already fixed in a prior commit; existing tests `test_standalone_dcoffset_nested_under_softwarefilters`, `test_merge_into_existing_softwarefilters_dict`, and `test_non_ieeg_sidecar_not_migrated` cover the documented FR-031 contract. T149 added `test_dcoffset_migration_is_idempotent` (running migrate twice produces the same result, no double-nesting) and verified all four pass at level `safe` (not `advisory`).
 
 **Checkpoint**: `tox -e py3 -- tests/test_migrate.py -k "merge_sources or scan_date_no_tsv or dcoffsetcorrection"` passes; the three known data-loss / data-corruption paths are covered.
 

src/bids_utils/migrate.py

@@ -1131,7 +1131,8 @@ def _apply_field_rename(
         # Merge into new field (handle Sources consolidation).
         # Normalize both sides to lists so mixed string/array types merge
         # correctly; 3-way merges (e.g. Sources + BasedOn + RawSources) work
-        # iteratively as each rule runs.
+        # iteratively as each rule runs.  Deduplicate on merge (FR-029) so an
+        # incoming URI already present in the destination is not added twice.
         if rule.new_field:
             existing = data.get(rule.new_field)
             if existing is None:
@@ -1141,7 +1142,11 @@ def _apply_field_rename(
                     list(existing) if isinstance(existing, list) else [existing]
                 )
                 value_list = list(value) if isinstance(value, list) else [value]
-                data[rule.new_field] = existing_list + value_list
+                merged = list(existing_list)
+                for item in value_list:
+                    if item not in merged:
+                        merged.append(item)
+                data[rule.new_field] = merged
         if vcs is not None:
             _write_json(jf, data, vcs)
         else:

tests/test_migrate.py

@@ -180,6 +180,69 @@ def test_sources_three_way_merge_mixed_types(self, tmp_path: Path) -> None:
             "rawsources.nii",
         }
 
+    @pytest.mark.ai_generated
+    def test_sources_array_plus_string_merge(self, tmp_path: Path) -> None:
+        """Pre-existing array Sources + incoming string RawSources merges into list."""
+        ds_path = _make_dataset(tmp_path, "1.4.0")
+        func = ds_path / "sub-01" / "func"
+        func.mkdir(parents=True)
+        sidecar = func / "sub-01_task-rest_bold.json"
+        sidecar.write_text(
+            json.dumps(
+                {
+                    "Sources": ["bids:raw:sub-01/anat/sub-01_T1w.nii.gz"],
+                    "RawSources": "bids:raw:sub-01/dwi/sub-01_dwi.nii.gz",
+                }
+            )
+        )
+
+        ds = BIDSDataset.from_path(ds_path)
+        migrate_dataset(ds)
+
+        data = json.loads(sidecar.read_text())
+        assert "RawSources" not in data
+        assert isinstance(data["Sources"], list)
+        assert len(data["Sources"]) == 2
+        assert set(data["Sources"]) == {
+            "bids:raw:sub-01/anat/sub-01_T1w.nii.gz",
+            "bids:raw:sub-01/dwi/sub-01_dwi.nii.gz",
+        }
+
+    @pytest.mark.ai_generated
+    def test_sources_merge_dedups_duplicates(self, tmp_path: Path) -> None:
+        """Incoming URI already present in pre-existing Sources is not duplicated.
+
+        FR-029: merging behavior preserves data without introducing duplicate
+        entries when the same URI appears in both the destination and the
+        incoming field.
+        """
+        ds_path = _make_dataset(tmp_path, "1.4.0")
+        func = ds_path / "sub-01" / "func"
+        func.mkdir(parents=True)
+        sidecar = func / "sub-01_task-rest_bold.json"
+        sidecar.write_text(
+            json.dumps(
+                {
+                    "Sources": ["shared.nii", "only-in-sources.nii"],
+                    "BasedOn": ["shared.nii", "only-in-basedon.nii"],
+                }
+            )
+        )
+
+        ds = BIDSDataset.from_path(ds_path)
+        migrate_dataset(ds)
+
+        data = json.loads(sidecar.read_text())
+        assert "BasedOn" not in data
+        assert isinstance(data["Sources"], list)
+        # No duplicates: 'shared.nii' appears exactly once
+        assert data["Sources"].count("shared.nii") == 1
+        assert set(data["Sources"]) == {
+            "shared.nii",
+            "only-in-sources.nii",
+            "only-in-basedon.nii",
+        }
+
 
 class TestEnumRename:
     @pytest.mark.ai_generated
@@ -999,6 +1062,34 @@ def test_non_ieeg_sidecar_not_migrated(self, tmp_path: Path) -> None:
         # Scope is iEEG only (FR-031) — func sidecars remain untouched
         assert data == {"DCOffsetCorrection": "none"}
 
+    @pytest.mark.ai_generated
+    def test_dcoffset_migration_is_idempotent(self, tmp_path: Path) -> None:
+        """Running migrate twice produces the same result as running it once.
+
+        After the first migration the original DCOffsetCorrection key is gone,
+        so a second run must be a no-op (no double-nesting, no error).
+        """
+        ds_path = _make_dataset(tmp_path, "1.4.0")
+        ieeg = ds_path / "sub-01" / "ieeg"
+        ieeg.mkdir(parents=True)
+        sidecar = ieeg / "sub-01_task-rest_ieeg.json"
+        sidecar.write_text(json.dumps({"DCOffsetCorrection": "high-pass 0.1Hz"}))
+
+        ds = BIDSDataset.from_path(ds_path)
+        migrate_dataset(ds, level="safe")
+        first_pass = json.loads(sidecar.read_text())
+        assert first_pass == {
+            "SoftwareFilters": {
+                "DCOffsetCorrection": {"description": "high-pass 0.1Hz"}
+            }
+        }
+
+        # Second run on the already-migrated dataset must be a no-op
+        ds2 = BIDSDataset.from_path(ds_path)
+        migrate_dataset(ds2, level="safe")
+        second_pass = json.loads(sidecar.read_text())
+        assert second_pass == first_pass
+
 
 class TestFieldRemoval:
     @pytest.mark.ai_generated