diff --git a/.ai/bids/inject-spec.md b/.ai/bids/inject-spec.md index 5f1dda43..96bee5dd 100644 --- a/.ai/bids/inject-spec.md +++ b/.ai/bids/inject-spec.md @@ -116,6 +116,29 @@ func/sub-qa_ses-20250814_task-rest_acq-p2_bold__dup-01.nii.gz 2025-08-14T > makes it possible to resume or validate a subsequent `bids-qr-sync` pass without > re-scanning the source video from scratch. +**`scans.json` sidecar:** `src/reprostim/assets/bids/scans.json` provides a default BIDS +data-dictionary sidecar (`LongName`/`Description`/`Units` per BIDS's tabular-file column +metadata schema, the same shape used for `_events.json`) documenting `filename`, `acq_time`, +and all four `reprostim_*` columns above. + +`_do_inject_scans_json(ctx)` (called by `_do_inject_all` as its first step, before any +`_scans.tsv` is touched) keeps `/scans.json` (`--dataset`/`-d`, +`BiContext.dataset_home`, default `.`) in sync with this default sample: + +- **Missing** — `scans.json` doesn't exist under `dataset_home` yet: created verbatim from + the default sample (`_load_default_scans_json()`, read via `importlib.resources` from + `assets/bids/scans.json`). +- **Present, complete** — every top-level field from the default sample is already present + (regardless of value): no-op, file is not rewritten. +- **Present, incomplete** — one or more default fields are missing: only the missing fields + are appended (`existing.update(missing)`); fields already present — default or custom + (e.g. a hand-added `operator` entry) — are left untouched, never overwritten. +- **Invalid JSON** — `scans.json` exists but fails to parse: reported as an error + (`ctx.summary.errors`/`n_errors`, `logger.error`, `out_func("ERROR: ...")`) and left + untouched — does not raise, does not block the rest of `_do_inject_all`. +- **`--dry-run`** — logs/reports what would change (create vs. which fields would be + appended) but writes nothing, consistent with Dry-Run Mode below. + ### C) QR codes file — BIDS _events-like .tsv If QR codes were parsed from the video (`--qr` mode is not `none`), the decoded QR records @@ -162,6 +185,7 @@ reprostim bids-inject [OPTIONS] PATHS... | Option | Type | Default | Description | |-------------------------------------------------|-----------------|------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | `-f / --videos PATH` | Path | required | Path to `videos.tsv` produced by `video-audit`. Video file paths in the TSV are resolved relative to this file's location. | +| `-d / --dataset PATH` | Path (dir) | `.` | Home directory of the BIDS dataset being injected into. Propagated into `BiContext.dataset_home`; `do_main` uses it to create/update `/scans.json` as its first step (see `scans.json` sidecar note below). `do_main` re-validates it exists and is a directory even when called directly, bypassing the CLI's own `click.Path(exists=True)` check — reports the error via `out_func`/`logger.error` and returns `1`, same as any other `do_main` error (no exception raised). | | `-r / --recursive` | Flag | False | When a directory is given in PATHS, recurse into subdirectories to find all `*_scans.tsv` files. | | `-b / --buffer-before DURATION` | sec or ISO 8601 | `0` | Extra video before scan onset. | | `-a / --buffer-after DURATION` | sec or ISO 8601 | `0` | Extra video after scan end. | @@ -172,7 +196,7 @@ reprostim bids-inject [OPTIONS] PATHS... | `-z / --reprostim-timezone TIMEZONE` | String | `local` | Timezone of the ReproStim capture machine, applied to naive `videos.tsv` timestamps (see Timezone Handling below). | | `-Z / --bids-timezone TIMEZONE` | String | `local` | Timezone assumed for naive BIDS `acq_time` values. When omitted, defaults to the value of `--reprostim-timezone` (see Timezone Handling below). | | `-m / --match REGEX` | String | `.*` | Regular expression matched against the `filename` field of each scan record. Only records whose `filename` matches are processed; all others are skipped. Default `.*` matches every record. Example: `func/` to restrict to functional scans only. | -| `-d / --dry-run` | Flag | False | Analyse BIDS data and resolve matches but do not call `split-video` or write any output files. Prints what would be done. | +| `-n / --dry-run` | Flag | False | Analyse BIDS data and resolve matches but do not call `split-video` or write any output files. Prints what would be done. | | `-w / --overwrite [skip\|force\|always\|error]` | Choice | `skip` | Policy for handling existing output files (see Overwrite Mode below). | | `-k / --lock [yes\|no]` | Choice | `yes` | Whether to acquire a file lock (`videos.tsv.lock`) before reading `videos.tsv`. Use `no` for dirty-read mode when the lock is held by another user (see Lock / Dirty-read Mode below). | | `-v / --verbose` | Flag | False | Increase verbosity. | @@ -278,6 +302,10 @@ reprostim bids-inject \ ## Dry-Run Mode +`--dry-run`'s short flag is `-n` (not `-d`), matching the `rsync`/`make` "no-op" convention — +`-d` is used for the `--dataset` option instead (BIDS dataset root, default `.`, home of +`scans.json`), which is used more frequently and deserves the more obvious mnemonic letter. + When `--dry-run` is set, `bids-inject` performs all analysis steps — loading `videos.tsv`, discovering `*_scans.tsv` files, resolving scan durations, matching videos, determining output paths and media suffixes — but **skips the actual `split-video` call and writes no files**. diff --git a/.ai/bids/inject-tasks.md b/.ai/bids/inject-tasks.md index 0dd745c1..55ee0587 100644 --- a/.ai/bids/inject-tasks.md +++ b/.ai/bids/inject-tasks.md @@ -8,6 +8,11 @@ Tracks implementation progress against [inject-spec.md](inject-spec.md). - [x] `PATHS` argument — one or more `_scans.tsv` files or directories - [x] `-f / --videos` — path to `videos.tsv` +- [x] `-d / --dataset` — BIDS dataset home directory, default `.`. +- [x] `do_main` re-validates `dataset_home` exists and is a directory, independent of the + CLI's own `click.Path(exists=True)` check (covers direct callers that bypass the CLI) — + reports via `out_func`/`logger.error` and returns `1`, not an exception, so it surfaces + as a normal non-zero CLI exit code - [x] `-r / --recursive` — recurse into subdirectories - [x] `-b / --buffer-before` — extra video before scan onset - [x] `-a / --buffer-after` — extra video after scan end @@ -18,7 +23,8 @@ Tracks implementation progress against [inject-spec.md](inject-spec.md). - [x] `-z / --reprostim-timezone` — timezone for `videos.tsv` timestamps - [x] `-Z / --bids-timezone` — timezone for BIDS `acq_time` values - [x] `-m / --match REGEX` — filter scan records by filename -- [x] `-d / --dry-run` +- [x] `-n / --dry-run` — short flag changed from `-d` to `-n` (rsync/make "no-op" convention); + `-d` now used for `--dataset` above - [x] `-w / --overwrite [skip|force|always|error]` — policy for existing output files - [x] `-k / --lock [yes|no]` — dirty-read mode for `videos.tsv` - [x] `-v / --verbose` @@ -139,6 +145,22 @@ Tracks implementation progress against [inject-spec.md](inject-spec.md). - [x] Handle re-runs: update existing `reprostim_*` columns in-place (don't duplicate) - [x] Skip write-back in `--dry-run` mode - [x] `reprostim_path` stored relative to `videos.tsv` location (consistent with `videos.tsv` path convention) +- [x] `src/reprostim/assets/bids/scans.json` — default BIDS data-dictionary sidecar documenting + `filename`, `acq_time`, and all four `reprostim_*` columns (`LongName`/`Description`/`Units`) + +### E) scans.json data-dictionary sync +- [x] `_load_default_scans_json()` — reads the packaged default sample via `importlib.resources` +- [x] `_do_inject_scans_json(ctx)` — called by `_do_inject_all` as its first step, before any + path in `paths` is processed +- [x] Creates `/scans.json` verbatim from the default sample when missing +- [x] No-op (no rewrite) when the existing file already has every default field +- [x] Appends only the missing default fields when the existing file has some but not all; + existing fields (default or custom, e.g. a hand-added `operator` entry) are never + overwritten +- [x] Invalid/unparseable existing `scans.json` → reported via `ctx.summary.errors`/`n_errors` + and `out_func`/`logger.error`, file left untouched, does not raise and does not abort + the rest of `_do_inject_all` +- [x] Honours `--dry-run` — logs/reports what would change, writes nothing --- @@ -304,6 +326,20 @@ Test file location: `tests/bids/test_inject.py` (mirrors `tests/audio/test_audio - [x] Re-run (columns already present) → columns updated in-place, no duplication - [x] `--dry-run` → `_scans.tsv` not modified +### scans.json data-dictionary sync tests (`_do_inject_scans_json`) +- [x] Missing `scans.json` → created verbatim from `_load_default_scans_json()` +- [x] Existing `scans.json` with every default field (plus a custom field) → byte-for-byte + untouched (no-op) +- [x] Existing `scans.json` missing some default fields → only those appended; existing + default and custom fields preserved untouched +- [x] `--dry-run`, missing file → nothing written, `[DRY-RUN] Would create ...` reported +- [x] `--dry-run`, incomplete file → nothing written, `[DRY-RUN] Would add missing field ...` + reported +- [x] Invalid JSON in existing `scans.json` → reported via `ctx.summary`/`out_func`, file left + untouched, no exception raised +- [x] `do_main` end-to-end (`dry_run=False`) → `/scans.json` created as the + pipeline's first step, before any `_scans.tsv` is processed + ### Overwrite mode tests - [x] `skip` + existing output → 0 injected, files untouched, counted as skipped diff --git a/src/reprostim/assets/bids/scans.json b/src/reprostim/assets/bids/scans.json new file mode 100644 index 00000000..4444e227 --- /dev/null +++ b/src/reprostim/assets/bids/scans.json @@ -0,0 +1,28 @@ +{ + "filename": { + "Description": "Name of the nifti file" + }, + "acq_time": { + "LongName": "Acquisition time", + "Description": "Acquisition time of the particular scan" + }, + "reprostim_path": { + "LongName": "ReproStim source video path", + "Description": "Path to the source .mkv file the injected recording was sliced from, relative to the videos.tsv location" + }, + "reprostim_offset": { + "LongName": "ReproStim buffer-segment offset", + "Description": "Offset of the buffer-segment start into the source video", + "Units": "s" + }, + "reprostim_buffer_before": { + "LongName": "ReproStim buffer before scan onset", + "Description": "Actual buffer prepended before scan onset", + "Units": "s" + }, + "reprostim_buffer_after": { + "LongName": "ReproStim buffer after scan end", + "Description": "Actual buffer appended after scan end", + "Units": "s" + } +} diff --git a/src/reprostim/bids/inject.py b/src/reprostim/bids/inject.py index d151d1a6..6064a78f 100644 --- a/src/reprostim/bids/inject.py +++ b/src/reprostim/bids/inject.py @@ -20,6 +20,7 @@ from datetime import datetime, time, timedelta, timezone, tzinfo from enum import Enum from functools import lru_cache +from importlib.resources import files from typing import Callable, List, Optional, Tuple from zoneinfo import ZoneInfo @@ -127,6 +128,11 @@ class BiSummary(BaseModel): class BiContext(BaseModel): """Context for bids-inject processing of scan records.""" + dataset_home: str = Field( + default=".", + description="Home directory of the BIDS dataset being injected into " + "(e.g. contains scans.json). Defaults to the current directory.", + ) dry_run: bool = Field( ..., description="Whether to skip actual file writes and print planned actions" ) @@ -1126,19 +1132,101 @@ def _do_inject_dir(ctx: BiContext, path: str): _do_inject_dir(ctx, entry.path) +def _load_default_scans_json() -> dict: + """Load the packaged default BIDS ``scans.json`` data-dictionary sample. + + :returns: Parsed contents of ``assets/bids/scans.json``. + :rtype: dict + """ + text = (files("reprostim") / "assets" / "bids" / "scans.json").read_text() + return json.loads(text) + + +def _do_inject_scans_json(ctx: BiContext) -> None: + """Ensure ``/scans.json`` exists and has every default field. + + Compares ``/scans.json`` against the packaged default + sample (``assets/bids/scans.json``): + + - If ``scans.json`` doesn't exist yet, it is created from the default + sample verbatim. + - If it exists and already has every top-level field from the default + sample, nothing is written. + - If it exists but is missing one or more default fields, those fields + (and only those — existing fields are left untouched) are appended + and the file is rewritten. + + Honours ``ctx.dry_run``: when set, logs/reports what would change but + writes nothing. A ``scans.json`` that fails to parse as JSON is reported + as an error (via ``ctx.summary``) and left untouched rather than + overwritten or raised as an exception. + + :param ctx: Processing context; uses ``ctx.dataset_home``, ``ctx.dry_run``, + ``ctx.out_func``, and ``ctx.summary``. + :type ctx: BiContext + """ + scans_json_path = os.path.join(ctx.dataset_home, "scans.json") + default = _load_default_scans_json() + + if not os.path.isfile(scans_json_path): + if ctx.dry_run: + msg = f"Would create {scans_json_path}" + else: + with open(scans_json_path, "w") as f: + json.dump(default, f, indent=2) + f.write("\n") + msg = f"Created {scans_json_path}" + logger.info(msg) + if ctx.out_func: + ctx.out_func(f"[DRY-RUN] {msg}" if ctx.dry_run else msg) + return + + try: + with open(scans_json_path) as f: + existing = json.load(f) + except json.JSONDecodeError as e: + err_msg = f"Failed to parse {scans_json_path}: {e}" + logger.error(err_msg) + ctx.summary.errors.append(err_msg) + ctx.summary.n_errors += 1 + if ctx.out_func: + ctx.out_func(f"ERROR: {err_msg}") + return + + missing = {k: v for k, v in default.items() if k not in existing} + if not missing: + logger.debug(f"{scans_json_path} already has all default fields") + return + + field_list = ", ".join(sorted(missing)) + if ctx.dry_run: + msg = f"Would add missing field(s) to {scans_json_path}: {field_list}" + else: + existing.update(missing) + with open(scans_json_path, "w") as f: + json.dump(existing, f, indent=2) + f.write("\n") + msg = f"Added missing field(s) to {scans_json_path}: {field_list}" + logger.info(msg) + if ctx.out_func: + ctx.out_func(f"[DRY-RUN] {msg}" if ctx.dry_run else msg) + + def _do_inject_all(ctx: BiContext, paths: List[str]): """Dispatch injection across a mixed list of file and directory paths. - For each entry in *paths*: regular files are forwarded to - :func:`_do_inject_scans`; directories are forwarded to - :func:`_do_inject_dir` (which honours ``ctx.recursive``); anything else - is logged as a warning and skipped. + First ensures ``/scans.json`` exists and has every default + field (see :func:`_do_inject_scans_json`). Then, for each entry in + *paths*: regular files are forwarded to :func:`_do_inject_scans`; + directories are forwarded to :func:`_do_inject_dir` (which honours + ``ctx.recursive``); anything else is logged as a warning and skipped. :param ctx: Processing context propagated to all subordinate calls. :type ctx: BiContext :param paths: Sequence of file or directory paths supplied by the caller. :type paths: List[str] """ + _do_inject_scans_json(ctx) # iterate over paths and depending on whether it's a file or directory, # process accordingly @@ -1371,6 +1459,7 @@ def dt_bids_to_reprostim( def do_main( paths: List[str], videos_tsv: str, + dataset_home: str, recursive: bool, match: str, buffer_before: str, @@ -1397,6 +1486,9 @@ def do_main( :param videos_tsv: Path to ``videos.tsv`` produced by ``video-audit``. Video file paths inside the TSV are resolved relative to this file's location. :type videos_tsv: str + :param dataset_home: Home directory of the BIDS dataset being injected into + (e.g. contains ``scans.json``). Defaults to the current directory. + :type dataset_home: str :param recursive: When ``True``, recurse into subdirectories when searching for ``*_scans.tsv`` files. :type recursive: bool @@ -1453,7 +1545,17 @@ def do_main( :returns: Exit code — ``0`` on success, non-zero on error. :rtype: int """ + if not os.path.isdir(dataset_home): + err_msg = ( + f"--dataset path does not exist or is not a directory: {dataset_home!r}" + ) + logger.error(err_msg) + if out_func: + out_func(f"ERROR: {err_msg}") + return 1 + ctx: BiContext = BiContext( + dataset_home=dataset_home, dry_run=dry_run, recursive=recursive, match=match, diff --git a/src/reprostim/cli/cmd_bids_inject.py b/src/reprostim/cli/cmd_bids_inject.py index 613a4aac..68775c47 100644 --- a/src/reprostim/cli/cmd_bids_inject.py +++ b/src/reprostim/cli/cmd_bids_inject.py @@ -30,6 +30,15 @@ help="Path to videos.tsv produced by video-audit. Video file paths in the TSV " "are resolved relative to this file's location.", ) +@click.option( + "-d", + "--dataset", + type=click.Path(exists=True, file_okay=False, dir_okay=True), + default=".", + show_default=True, + help="Home directory of the BIDS dataset being injected into " + "(e.g. contains scans.json). Defaults to the current directory.", +) @click.option( "-r", "--recursive", @@ -133,7 +142,7 @@ "functional scans only.", ) @click.option( - "-d", + "-n", "--dry-run", is_flag=True, default=False, @@ -174,6 +183,7 @@ def bids_inject( ctx, paths: tuple, videos: str, + dataset: str, recursive: bool, buffer_before: str, buffer_after: str, @@ -203,6 +213,7 @@ def bids_inject( logger.debug(f"Working dir : {os.getcwd()}") logger.info(f"PATHS : {paths}") logger.info(f"videos.tsv : {videos}") + logger.info(f"Dataset home : {dataset}") logger.info(f"Recursive : {recursive}") logger.info(f"Buffer before : {buffer_before}") logger.info(f"Buffer after : {buffer_after}") @@ -225,6 +236,7 @@ def bids_inject( res = do_main( paths=list(paths), videos_tsv=videos, + dataset_home=dataset, recursive=recursive, match=match, buffer_before=buffer_before, diff --git a/tests/bids/test_inject.py b/tests/bids/test_inject.py index 65f0c0a9..c8258f14 100644 --- a/tests/bids/test_inject.py +++ b/tests/bids/test_inject.py @@ -6,6 +6,7 @@ import csv import io +import json import re import shutil from datetime import datetime, time, timezone @@ -20,6 +21,7 @@ from reprostim.bids.inject import ( _REPROSTIM_COLS, DATALAD_FUSE_AVAILABLE, + BiContext, MediaSuffix, ScanMetadata, ScanRecord, @@ -28,9 +30,11 @@ _calc_media_suffix, _calc_scan_duration_sec, _calc_scan_start_end_ts, + _do_inject_scans_json, _find_bids_root, _format_bids_str, _is_scans_file, + _load_default_scans_json, _open_dataset_file, _parse_bids_float, _parse_bids_str, @@ -836,6 +840,11 @@ def _run( ret = do_main( paths=paths, videos_tsv=videos_tsv, + # videos_tsv is always written under the test's tmp_path (see + # _write_videos_tsv); anchoring dataset_home to its directory keeps + # any scans.json writes inside tmp_path instead of polluting the + # real CWD when a test calls _run with dry_run=False. + dataset_home=str(Path(videos_tsv).parent), recursive=False, match=match, buffer_before="0", @@ -855,6 +864,196 @@ def _run( return ret, output +# =========================================================================== +# _do_inject_scans_json +# =========================================================================== + + +def _bi_context(dataset_home: str, dry_run: bool = False, out_func=None) -> BiContext: + """Minimal BiContext for unit-testing _do_inject_scans_json directly.""" + return BiContext( + dataset_home=dataset_home, + dry_run=dry_run, + recursive=False, + out_func=out_func, + ) + + +def test_do_inject_scans_json_creates_when_missing(tmp_path): + """scans.json doesn't exist yet -> created verbatim from the default sample.""" + out = [] + ctx = _bi_context(str(tmp_path), out_func=out.append) + + _do_inject_scans_json(ctx) + + scans_json = tmp_path / "scans.json" + assert scans_json.is_file() + assert json.loads(scans_json.read_text()) == _load_default_scans_json() + assert any("Created" in line for line in out) + + +def test_do_inject_scans_json_noop_when_all_fields_present(tmp_path): + """scans.json already has every default field -> left byte-for-byte untouched.""" + scans_json = tmp_path / "scans.json" + default = _load_default_scans_json() + # Extra custom field alongside every default field. + content = {**default, "operator": {"Description": "Custom, non-default field"}} + original_text = json.dumps(content, indent=2) + scans_json.write_text(original_text) + + ctx = _bi_context(str(tmp_path)) + _do_inject_scans_json(ctx) + + assert scans_json.read_text() == original_text + + +def test_do_inject_scans_json_appends_missing_fields(tmp_path): + """scans.json is missing some default fields -> only those are appended.""" + scans_json = tmp_path / "scans.json" + default = _load_default_scans_json() + assert "reprostim_path" in default and "reprostim_offset" in default + existing_content = { + "filename": default["filename"], + "operator": {"Description": "Custom, non-default field"}, + } + scans_json.write_text(json.dumps(existing_content, indent=2)) + + out = [] + ctx = _bi_context(str(tmp_path), out_func=out.append) + _do_inject_scans_json(ctx) + + updated = json.loads(scans_json.read_text()) + # Existing (default and custom) fields preserved untouched. + assert updated["filename"] == default["filename"] + assert updated["operator"] == {"Description": "Custom, non-default field"} + # Missing default fields appended. + for key in default: + assert updated[key] == default[key] + assert any("Added missing field" in line for line in out) + + +def test_do_inject_scans_json_dry_run_does_not_write(tmp_path): + """--dry-run: reports what would happen but writes nothing.""" + out = [] + ctx = _bi_context(str(tmp_path), dry_run=True, out_func=out.append) + + _do_inject_scans_json(ctx) + + assert not (tmp_path / "scans.json").is_file() + assert any("[DRY-RUN]" in line and "Would create" in line for line in out) + + +def test_do_inject_scans_json_dry_run_does_not_write_missing_fields(tmp_path): + """--dry-run with an existing but incomplete scans.json: reports, doesn't write.""" + scans_json = tmp_path / "scans.json" + original_text = json.dumps({"filename": _load_default_scans_json()["filename"]}) + scans_json.write_text(original_text) + + out = [] + ctx = _bi_context(str(tmp_path), dry_run=True, out_func=out.append) + _do_inject_scans_json(ctx) + + assert scans_json.read_text() == original_text + assert any( + "[DRY-RUN]" in line and "Would add missing field" in line for line in out + ) + + +def test_do_inject_scans_json_invalid_json_reports_error_and_leaves_file(tmp_path): + """A scans.json that fails to parse is reported as an error, + not overwritten/raised.""" + scans_json = tmp_path / "scans.json" + scans_json.write_text("{not valid json") + + out = [] + ctx = _bi_context(str(tmp_path), out_func=out.append) + _do_inject_scans_json(ctx) + + assert scans_json.read_text() == "{not valid json" + assert ctx.summary.n_errors == 1 + assert len(ctx.summary.errors) == 1 + assert any("ERROR" in line for line in out) + + +def test_do_main_invokes_scans_json_step_first(tmp_path): + """do_main's real pipeline creates /scans.json as its first step.""" + scans_tsv = _copy_bids_fixture(tmp_path) + videos_tsv = _write_videos_tsv(tmp_path, _VA_V1) + ret, _output = _run([str(scans_tsv)], videos_tsv, dry_run=False, overwrite="always") + + scans_json = tmp_path / "scans.json" + assert scans_json.is_file() + assert json.loads(scans_json.read_text()) == _load_default_scans_json() + + +def test_do_main_reports_error_when_dataset_home_missing(tmp_path): + """do_main() returns 1 and reports an error when dataset_home doesn't exist. + + The CLI layer already rejects a missing --dataset path via + click.Path(exists=True), but do_main() is also callable directly + (e.g. from other code or tests), so it must not silently accept an + invalid dataset_home. Reported as a normal error result (matching the + rest of do_main's error handling) rather than an exception, so it + propagates as a non-zero CLI exit code via the usual do_main -> ctx.exit() + path instead of an uncaught traceback. + """ + videos_tsv = _write_videos_tsv(tmp_path, _VA_V1) + out = [] + ret = do_main( + paths=[_SCANS_TSV], + videos_tsv=videos_tsv, + dataset_home=str(tmp_path / "does-not-exist"), + recursive=False, + match=".*", + buffer_before="0", + buffer_after="0", + buffer_policy="flexible", + time_offset=0.0, + qr="none", + layout="nearby", + reprostim_timezone="UTC", + bids_timezone="UTC", + dry_run=True, + overwrite="skip", + lock=False, + verbose=False, + out_func=out.append, + ) + assert ret == 1 + assert any("does-not-exist" in line for line in out) + + +def test_do_main_reports_error_when_dataset_home_is_file(tmp_path): + """do_main() returns 1 and reports an error when dataset_home is a file, + not a directory.""" + videos_tsv = _write_videos_tsv(tmp_path, _VA_V1) + not_a_dir = tmp_path / "not-a-dir.txt" + not_a_dir.write_text("") + out = [] + ret = do_main( + paths=[_SCANS_TSV], + videos_tsv=videos_tsv, + dataset_home=str(not_a_dir), + recursive=False, + match=".*", + buffer_before="0", + buffer_after="0", + buffer_policy="flexible", + time_offset=0.0, + qr="none", + layout="nearby", + reprostim_timezone="UTC", + bids_timezone="UTC", + dry_run=True, + overwrite="skip", + lock=False, + verbose=False, + out_func=out.append, + ) + assert ret == 1 + assert any("not-a-dir.txt" in line for line in out) + + def test_integration_dry_run_two_matching_videos(tmp_path): """Two functional scans each matched by a distinct video → 2 injected, 1 skipped (anat).""" @@ -1713,3 +1912,64 @@ def test_cli_zero_do_main_result_exits_zero(tmp_path): [str(scans_tsv), "-f", videos_tsv], ) assert result.exit_code == 0 + + +def test_cli_dry_run_short_flag_is_n(tmp_path): + """`-n` is the --dry-run short flag; forwarded to do_main as dry_run=True. + + Regression test: `--dry-run`'s short flag was changed from `-d` to `-n` + (rsync/make "no-op" convention) to free up `-d` for a planned + `--dataset` option. + """ + scans_tsv = _copy_bids_fixture(tmp_path) + videos_tsv = _write_videos_tsv(tmp_path, _VA_V1) + + with patch("reprostim.bids.inject.do_main", return_value=0) as mock_dm: + result = CliRunner().invoke( + bids_inject, + [str(scans_tsv), "-f", videos_tsv, "-n"], + ) + assert result.exit_code == 0 + assert mock_dm.call_args.kwargs["dry_run"] is True + + +def test_cli_dataset_defaults_to_current_directory(tmp_path): + """--dataset defaults to '.' when not specified.""" + scans_tsv = _copy_bids_fixture(tmp_path) + videos_tsv = _write_videos_tsv(tmp_path, _VA_V1) + + with patch("reprostim.bids.inject.do_main", return_value=0) as mock_dm: + result = CliRunner().invoke( + bids_inject, + [str(scans_tsv), "-f", videos_tsv], + ) + assert result.exit_code == 0 + assert mock_dm.call_args.kwargs["dataset_home"] == "." + + +def test_cli_dataset_short_flag_forwarded(tmp_path): + """-d/--dataset is forwarded to do_main as dataset_home.""" + scans_tsv = _copy_bids_fixture(tmp_path) + videos_tsv = _write_videos_tsv(tmp_path, _VA_V1) + dataset_dir = tmp_path / "dataset" + dataset_dir.mkdir() + + with patch("reprostim.bids.inject.do_main", return_value=0) as mock_dm: + result = CliRunner().invoke( + bids_inject, + [str(scans_tsv), "-f", videos_tsv, "-d", str(dataset_dir)], + ) + assert result.exit_code == 0 + assert mock_dm.call_args.kwargs["dataset_home"] == str(dataset_dir) + + +def test_cli_dataset_nonexistent_dir_nonzero_exit(tmp_path): + """A --dataset path that doesn't exist is rejected by Click's Path(exists=True).""" + scans_tsv = _copy_bids_fixture(tmp_path) + videos_tsv = _write_videos_tsv(tmp_path, _VA_V1) + + result = CliRunner().invoke( + bids_inject, + [str(scans_tsv), "-f", videos_tsv, "-d", str(tmp_path / "missing")], + ) + assert result.exit_code != 0