From 3445521faf596e1064d6433f6ce8e063f29bebd6 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Tue, 21 Apr 2026 10:24:42 -0400 Subject: [PATCH 1/6] feat: Recurse into nested BIDS datasets beyond derivatives/ Recursive validation (`-r`) previously recognized only datasets under `derivatives//`. Nested BIDS datasets placed under `rawbids/` (new study-level container, bids-standard/bids-specification#2191) or `sourcedata/` (historically used for `sourcedata/raw/` or OpenNeuro-style `sourcedata/ds00003/`) were silently ignored, so internal bugs in those subdatasets never surfaced. Generalize the detection: - For each of `derivatives/`, `rawbids/`, `sourcedata/`, check both forms: - Immediate: the container itself contains `dataset_description.json` (single nested dataset, e.g. `rawbids/dataset_description.json`). - Subfolder: one or more immediate children contain `dataset_description.json` (e.g. `derivatives/fmriprep/`, `sourcedata/ds00003/`). - Rename the local collection to `bidsNestedDatasets` / `nestedDatasetsSummary`; keep the public `derivativesSummary` field for API stability but widen its documented semantics. - Update the text output label "Derivative:" -> "Nested dataset:". Co-Authored-By: Claude Code 2.1.116 / Claude Opus 4.7 (1M context) --- .../20260421_095017_yarikoptic_bf_recurse.md | 11 ++++ src/utils/output.ts | 4 +- src/validators/bids.ts | 60 ++++++++++++------- 3 files changed, 51 insertions(+), 24 deletions(-) create mode 100644 changelog.d/20260421_095017_yarikoptic_bf_recurse.md diff --git a/changelog.d/20260421_095017_yarikoptic_bf_recurse.md b/changelog.d/20260421_095017_yarikoptic_bf_recurse.md new file mode 100644 index 00000000..feee034c --- /dev/null +++ b/changelog.d/20260421_095017_yarikoptic_bf_recurse.md @@ -0,0 +1,11 @@ +### Changed + +- Nested BIDS datasets discovered under `derivatives/`, `rawbids/`, or + `sourcedata/` are now recursed into with `-r`, whether the container + directory itself is a BIDS dataset (e.g. `rawbids/dataset_description.json`) + or it holds one or more BIDS datasets as immediate subdirectories + (e.g. `derivatives/fmriprep/dataset_description.json`, + `sourcedata/ds00003/dataset_description.json`). The text output + section previously labeled "Derivative:" is now "Nested dataset:". + The `derivativesSummary` field on `ValidationResult` is preserved + for API stability but now covers all nested BIDS datasets. diff --git a/src/utils/output.ts b/src/utils/output.ts index 4dec0f73..fd8815e0 100644 --- a/src/utils/output.ts +++ b/src/utils/output.ts @@ -46,10 +46,10 @@ export function consoleFormat( output.push('') if (result.derivativesSummary) { for (const [key, derivResult] of Object.entries(result.derivativesSummary)) { - output.push(colors.blue(`Derivative: ${key}`)) + output.push(colors.blue(`Nested dataset: ${key}`)) if (derivResult.issues.size === 0) { - output.push(colors.green('\tThis derivative appears to be BIDS compatible.')) + output.push(colors.green('\tThis nested dataset appears to be BIDS compatible.')) } else { ;(['warning', 'error'] as Severity[]).map((severity) => { output.push(...formatIssues(derivResult.issues.filter({ severity }), options, severity)) diff --git a/src/validators/bids.ts b/src/validators/bids.ts index 86584c21..22bbce78 100644 --- a/src/validators/bids.ts +++ b/src/validators/bids.ts @@ -43,12 +43,16 @@ const perDSChecks: DSCheckFunction[] = [ * * Loads the BIDS schema, walks the file tree, and applies file-level and * dataset-level checks, accumulating any issues into the returned - * {@link ValidationResult}. Derivative datasets nested under - * `derivatives/` are detected via their own `dataset_description.json`; - * when `options.recursive` is set, BIDS-conformant derivatives are - * validated and their results attached to `derivativesSummary` on the - * returned object. Non-BIDS derivatives and the `sourcedata`, `code` - * directories are ignored. + * {@link ValidationResult}. Nested BIDS datasets are detected via their + * own `dataset_description.json` under any of `derivatives/`, + * `rawbids/`, or `sourcedata/` — either at the container's immediate + * level (e.g. `rawbids/dataset_description.json`) or one level deeper + * (e.g. `derivatives/fmriprep/dataset_description.json`). When + * `options.recursive` is set, each nested BIDS dataset is validated + * and its result is attached to `derivativesSummary` on the returned + * object (the key name is retained for API compatibility but now also + * covers non-derivative nested datasets). The `code` directory is + * always ignored, as are non-BIDS contents of the nesting containers. * * `validate` does not throw on validation failures — it records them as * issues on the result. The returned `issues` collection can be filtered @@ -126,23 +130,33 @@ export async function validate( } } - const bidsDerivatives: Promise[] = [] + // Directories that may contain nested BIDS datasets — either the directory + // itself is a BIDS dataset (e.g. `rawbids/dataset_description.json`) or its + // immediate subdirectories are (e.g. `derivatives/fmriprep/`, + // `sourcedata/ds00003/`). + const nestingContainers = ['derivatives', 'rawbids', 'sourcedata'] + const bidsNestedDatasets: Promise[] = [] const nonstdDerivatives: FileTree[] = [] fileTree.directories = fileTree.directories.filter((dir) => { - if (['sourcedata', 'code'].includes(dir.name)) { + if (dir.name === 'code') { return false } - if (dir.name !== 'derivatives') { + if (!nestingContainers.includes(dir.name)) { return true } - for (const deriv of dir.directories) { - if (deriv.get('dataset_description.json')) { - bidsDerivatives.push(subtree(deriv)) - } else { - nonstdDerivatives.push(deriv) + if (dir.get('dataset_description.json')) { + bidsNestedDatasets.push(subtree(dir)) + } else { + for (const sub of dir.directories) { + if (sub.get('dataset_description.json')) { + bidsNestedDatasets.push(subtree(sub)) + } else { + nonstdDerivatives.push(sub) + } } } - // Remove derivatives from the main fileTree + // Always remove nesting containers from the main fileTree; their own + // validation (if any) happens recursively below. return false }) @@ -194,13 +208,13 @@ export async function validate( }) }) - const derivativesSummary: Record = {} + const nestedDatasetsSummary: Record = {} if (options.recursive) { await Promise.allSettled( - bidsDerivatives.map(async (promise) => { - const deriv = await promise - derivativesSummary[deriv.name] = await validate(deriv, options) - return derivativesSummary[deriv.name] + bidsNestedDatasets.map(async (promise) => { + const nested = await promise + nestedDatasetsSummary[nested.name] = await validate(nested, options) + return nestedDatasetsSummary[nested.name] }), ) } @@ -224,8 +238,10 @@ export async function validate( summary: summary.formatOutput(), } - if (Object.keys(derivativesSummary).length) { - output['derivativesSummary'] = derivativesSummary + if (Object.keys(nestedDatasetsSummary).length) { + // Keep the field name `derivativesSummary` for API stability; it now + // holds results for all nested BIDS datasets, not just derivatives. + output['derivativesSummary'] = nestedDatasetsSummary } return output } From 29fd993e5da39b19615f49828e50dbfc5799fafe Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Tue, 21 Apr 2026 10:48:50 -0400 Subject: [PATCH 2/6] fix: Normalize invalid DatasetType for internal rule lookups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `dataset_description` setter applied the default `DatasetType` only when the field was falsy (missing / empty). A truthy but out-of-enum value (e.g. `"Raw"` — capitalization typo) was kept as-is and then used to index `rules.directories[DatasetType]`, which returned `undefined`, causing the opaque-directory set to be empty and `findDirRuleMatches` to match no rule. That cascaded into spurious `NOT_INCLUDED` errors on legitimate `sub-*/ses-*` subjects inside such datasets, on top of the legitimate `JSON_SCHEMA_VALIDATION_ERROR` on the enum violation. Treat any `DatasetType` that is not one of the spec enum values the same as a missing value — fall back to `"raw"`, per the spec's backward-compatibility default. Source the valid enum from `schema.objects.metadata.DatasetType.enum` so this stays in sync with the spec; when the schema isn't loaded (test-only construction without a schema), leave a truthy value untouched rather than guess. Do the normalization on a shallow copy of the incoming `dataset_description` so we don't mutate the memoized `dataset_description.json` object. JSON schema validation still sees the original value and reports the enum violation. Co-Authored-By: Claude Code 2.1.116 / Claude Opus 4.7 (1M context) --- .../20260421_095017_yarikoptic_bf_recurse.md | 13 ++++++++++++- src/schema/context.ts | 15 ++++++++++----- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/changelog.d/20260421_095017_yarikoptic_bf_recurse.md b/changelog.d/20260421_095017_yarikoptic_bf_recurse.md index feee034c..45193d84 100644 --- a/changelog.d/20260421_095017_yarikoptic_bf_recurse.md +++ b/changelog.d/20260421_095017_yarikoptic_bf_recurse.md @@ -8,4 +8,15 @@ `sourcedata/ds00003/dataset_description.json`). The text output section previously labeled "Derivative:" is now "Nested dataset:". The `derivativesSummary` field on `ValidationResult` is preserved - for API stability but now covers all nested BIDS datasets. + for API stability but now covers all nested BIDS datasets. See + [#390](https://github.com/bids-standard/bids-validator/pull/390). + +### Fixed + +- Normalize an invalid `DatasetType` value (not one of `raw`, `derivative`, + `study`) to the spec default for internal rule lookups so + `rules.directories[DatasetType]` resolves and downstream validation does + not cascade into spurious `NOT_INCLUDED` errors on legitimate subjects. + The original value in `dataset_description.json` is preserved so JSON + schema validation still flags the invalid enum value. See + [#390](https://github.com/bids-standard/bids-validator/pull/390). diff --git a/src/schema/context.ts b/src/schema/context.ts index a6c4dfff..c03823cd 100644 --- a/src/schema/context.ts +++ b/src/schema/context.ts @@ -76,12 +76,17 @@ export class BIDSContextDataset implements Dataset { } set dataset_description(value: Record) { - this.#dataset_description = value - if (!this.dataset_description.DatasetType) { - this.dataset_description.DatasetType = this.dataset_description.GeneratedBy - ? 'derivative' - : 'raw' + // Shallow copy: don't mutate the memoized JSON; schema validation still + // flags enum violations on the original. + const copy: Record = { ...value } + // @ts-expect-error metadata is not declared on SchemaObjects + const validTypes = this.schema?.objects?.metadata?.DatasetType?.enum as string[] | undefined + // Unknown/mis-cased DatasetType falls back to the spec default; legacy + // GeneratedBy-without-DatasetType still infers 'derivative'. + if (validTypes && !validTypes.includes(copy.DatasetType as string)) { + copy.DatasetType = copy.GeneratedBy ? 'derivative' : 'raw' } + this.#dataset_description = copy const datasetType = this.dataset_description.DatasetType as string this.opaqueDirectories = new Set( Object.values(this.schema?.rules?.directories[datasetType] ?? {}) From 861d321e6dad57a2f08bde1140fe596bd71d272f Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Thu, 23 Apr 2026 09:40:33 -0400 Subject: [PATCH 3/6] refactor: Split nested-datasets result into derivativesSummary and sourcesSummary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prior commit folded every nested BIDS dataset — whether a derivative or a "source" under `rawbids/` / `sourcedata/` — into a single `derivativesSummary` field, with a caveat in the JSDoc that the name was retained for API stability only. That is confusing for consumers and hides useful structure: downstream tools often want to act on derivatives and sources differently (e.g. gating release on derivative errors alone, or surfacing source-dataset issues separately in a UI). Split the two into distinct fields: - `derivativesSummary` — datasets found under `derivatives/` (unchanged semantics from prior main releases). - `sourcesSummary` — datasets found under `rawbids/` or `sourcedata/` (new). Both are populated only when `-r` is set, and each maps the nested dataset's relative path to its own `ValidationResult`. `detectErrors` now descends into both. Text output renders the two groups as `Derivative:` and `Source:` sections. Co-Authored-By: Claude Code 2.1.116 / Claude Opus 4.7 (1M context) --- .../20260421_095017_yarikoptic_bf_recurse.md | 9 +-- src/summary/summary.ts | 3 +- src/types/validation-result.ts | 4 ++ src/utils/output.ts | 25 ++++--- src/validators/bids.ts | 65 ++++++++++++------- 5 files changed, 71 insertions(+), 35 deletions(-) diff --git a/changelog.d/20260421_095017_yarikoptic_bf_recurse.md b/changelog.d/20260421_095017_yarikoptic_bf_recurse.md index 45193d84..428cd3f2 100644 --- a/changelog.d/20260421_095017_yarikoptic_bf_recurse.md +++ b/changelog.d/20260421_095017_yarikoptic_bf_recurse.md @@ -5,10 +5,11 @@ directory itself is a BIDS dataset (e.g. `rawbids/dataset_description.json`) or it holds one or more BIDS datasets as immediate subdirectories (e.g. `derivatives/fmriprep/dataset_description.json`, - `sourcedata/ds00003/dataset_description.json`). The text output - section previously labeled "Derivative:" is now "Nested dataset:". - The `derivativesSummary` field on `ValidationResult` is preserved - for API stability but now covers all nested BIDS datasets. See + `sourcedata/ds00003/dataset_description.json`). In the returned + `ValidationResult`, derivatives populate `derivativesSummary` (as + before) and `rawbids/`/`sourcedata/` nested datasets populate the new + `sourcesSummary` field. In text output they render as `Derivative:` + and `Source:` sections respectively. See [#390](https://github.com/bids-standard/bids-validator/pull/390). ### Fixed diff --git a/src/summary/summary.ts b/src/summary/summary.ts index 151cfda7..4232bb42 100644 --- a/src/summary/summary.ts +++ b/src/summary/summary.ts @@ -182,5 +182,6 @@ export class Summary { */ export function detectErrors(result: ValidationResult): boolean { return result.issues.get({ severity: 'error' }).length > 0 || - Object.values(result.derivativesSummary ?? {}).some((res) => detectErrors(res)) + Object.values(result.derivativesSummary ?? {}).some((res) => detectErrors(res)) || + Object.values(result.sourcesSummary ?? {}).some((res) => detectErrors(res)) } diff --git a/src/types/validation-result.ts b/src/types/validation-result.ts index 49686395..8ae5be06 100644 --- a/src/types/validation-result.ts +++ b/src/types/validation-result.ts @@ -36,5 +36,9 @@ export interface SummaryOutput { export interface ValidationResult { issues: DatasetIssues summary: SummaryOutput + /** Per-derivative validation results, keyed by their relative path. */ derivativesSummary?: Record + /** Per-source validation results (nested datasets under `rawbids/` or + * `sourcedata/`), keyed by their relative path. */ + sourcesSummary?: Record } diff --git a/src/utils/output.ts b/src/utils/output.ts index fd8815e0..61854dc5 100644 --- a/src/utils/output.ts +++ b/src/utils/output.ts @@ -44,21 +44,30 @@ export function consoleFormat( output.push('') output.push(formatSummary(result.summary)) output.push('') - if (result.derivativesSummary) { - for (const [key, derivResult] of Object.entries(result.derivativesSummary)) { - output.push(colors.blue(`Nested dataset: ${key}`)) - - if (derivResult.issues.size === 0) { - output.push(colors.green('\tThis nested dataset appears to be BIDS compatible.')) + const formatNested = ( + label: string, + nested: Record, + ) => { + for (const [key, nestedResult] of Object.entries(nested)) { + output.push(colors.blue(`${label}: ${key}`)) + + if (nestedResult.issues.size === 0) { + output.push(colors.green(`\tThis ${label.toLowerCase()} appears to be BIDS compatible.`)) } else { ;(['warning', 'error'] as Severity[]).map((severity) => { - output.push(...formatIssues(derivResult.issues.filter({ severity }), options, severity)) + output.push(...formatIssues(nestedResult.issues.filter({ severity }), options, severity)) }) } - output.push(formatSummary(derivResult.summary)) + output.push(formatSummary(nestedResult.summary)) output.push('') } } + if (result.derivativesSummary) { + formatNested('Derivative', result.derivativesSummary) + } + if (result.sourcesSummary) { + formatNested('Source', result.sourcesSummary) + } return output.join('\n') } diff --git a/src/validators/bids.ts b/src/validators/bids.ts index 22bbce78..0fb499d9 100644 --- a/src/validators/bids.ts +++ b/src/validators/bids.ts @@ -49,10 +49,11 @@ const perDSChecks: DSCheckFunction[] = [ * level (e.g. `rawbids/dataset_description.json`) or one level deeper * (e.g. `derivatives/fmriprep/dataset_description.json`). When * `options.recursive` is set, each nested BIDS dataset is validated - * and its result is attached to `derivativesSummary` on the returned - * object (the key name is retained for API compatibility but now also - * covers non-derivative nested datasets). The `code` directory is - * always ignored, as are non-BIDS contents of the nesting containers. + * and its result is attached to the returned object: + * - Derivatives under `derivatives/` go into `derivativesSummary`. + * - Sources under `rawbids/` or `sourcedata/` go into `sourcesSummary`. + * The `code` directory is always ignored, as are non-BIDS contents of + * the nesting containers. * * `validate` does not throw on validation failures — it records them as * issues on the result. The returned `issues` collection can be filtered @@ -133,23 +134,33 @@ export async function validate( // Directories that may contain nested BIDS datasets — either the directory // itself is a BIDS dataset (e.g. `rawbids/dataset_description.json`) or its // immediate subdirectories are (e.g. `derivatives/fmriprep/`, - // `sourcedata/ds00003/`). - const nestingContainers = ['derivatives', 'rawbids', 'sourcedata'] - const bidsNestedDatasets: Promise[] = [] + // `sourcedata/ds00003/`). Split into two buckets so the returned + // ValidationResult can separate derivatives from sources. + type NestedBucket = 'derivatives' | 'sources' + const nestingContainerBucket: Record = { + derivatives: 'derivatives', + rawbids: 'sources', + sourcedata: 'sources', + } + const bidsDerivatives: Promise[] = [] + const bidsSources: Promise[] = [] + // TODO: include in dataset-wide totals (file count / size). See #390 review. const nonstdDerivatives: FileTree[] = [] fileTree.directories = fileTree.directories.filter((dir) => { if (dir.name === 'code') { return false } - if (!nestingContainers.includes(dir.name)) { + const bucket = nestingContainerBucket[dir.name] + if (!bucket) { return true } + const collect = bucket === 'derivatives' ? bidsDerivatives : bidsSources if (dir.get('dataset_description.json')) { - bidsNestedDatasets.push(subtree(dir)) + collect.push(subtree(dir)) } else { for (const sub of dir.directories) { if (sub.get('dataset_description.json')) { - bidsNestedDatasets.push(subtree(sub)) + collect.push(subtree(sub)) } else { nonstdDerivatives.push(sub) } @@ -208,15 +219,24 @@ export async function validate( }) }) - const nestedDatasetsSummary: Record = {} + const derivativesSummary: Record = {} + const sourcesSummary: Record = {} if (options.recursive) { - await Promise.allSettled( - bidsNestedDatasets.map(async (promise) => { - const nested = await promise - nestedDatasetsSummary[nested.name] = await validate(nested, options) - return nestedDatasetsSummary[nested.name] - }), - ) + const validateInto = ( + trees: Promise[], + into: Record, + ) => + Promise.allSettled( + trees.map(async (promise) => { + const nested = await promise + into[nested.name] = await validate(nested, options) + return into[nested.name] + }), + ) + await Promise.all([ + validateInto(bidsDerivatives, derivativesSummary), + validateInto(bidsSources, sourcesSummary), + ]) } if (config) { @@ -238,10 +258,11 @@ export async function validate( summary: summary.formatOutput(), } - if (Object.keys(nestedDatasetsSummary).length) { - // Keep the field name `derivativesSummary` for API stability; it now - // holds results for all nested BIDS datasets, not just derivatives. - output['derivativesSummary'] = nestedDatasetsSummary + if (Object.keys(derivativesSummary).length) { + output['derivativesSummary'] = derivativesSummary + } + if (Object.keys(sourcesSummary).length) { + output['sourcesSummary'] = sourcesSummary } return output } From 7bb4fffdee6d301320a25f03abcd32fe739f0ea0 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Mon, 27 Apr 2026 15:53:08 -0400 Subject: [PATCH 4/6] refactor: Print nested-dataset sections before root, with roll-up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `consoleFormat` previously printed the root dataset's issues and summary first, followed by Derivative: and Source: sections. When many nested datasets were present, the root's status scrolled off the top of the terminal and a `tail` of the output left the user looking at the last nested dataset rather than the dataset they asked to validate. Reorder bottom-up so the root dataset comes last: - `Source: /sourcedata/...` first, then `Source: /rawbids/...` - `Derivative: /derivatives/...` - `Root dataset:` (newly labeled) — the parent's issues and Summary After the Root section, embed a `Nested datasets:` roll-up tabulating count, errors, and warnings for Sources and Derivatives, so a `tail` shows both the parent's status and an aggregate health view at a glance. Within Sources, `/sourcedata/*` keys come before `/rawbids/*` keys (alphabetical within each group). Within Derivatives, alphabetical. Co-Authored-By: Claude Code 2.1.116 / Claude Opus 4.7 (1M context) --- .../20260421_095017_yarikoptic_bf_recurse.md | 7 +- src/utils/output.ts | 94 ++++++++++++++++--- 2 files changed, 84 insertions(+), 17 deletions(-) diff --git a/changelog.d/20260421_095017_yarikoptic_bf_recurse.md b/changelog.d/20260421_095017_yarikoptic_bf_recurse.md index 428cd3f2..85e4a482 100644 --- a/changelog.d/20260421_095017_yarikoptic_bf_recurse.md +++ b/changelog.d/20260421_095017_yarikoptic_bf_recurse.md @@ -9,8 +9,11 @@ `ValidationResult`, derivatives populate `derivativesSummary` (as before) and `rawbids/`/`sourcedata/` nested datasets populate the new `sourcesSummary` field. In text output they render as `Derivative:` - and `Source:` sections respectively. See - [#390](https://github.com/bids-standard/bids-validator/pull/390). + and `Source:` sections respectively. Sections are now printed in + bottom-up order — Sources (`sourcedata/` before `rawbids/`) → + Derivatives → Root dataset — so the parent dataset's status and a + roll-up of nested errors/warnings appear at the end of the output. + See [#390](https://github.com/bids-standard/bids-validator/pull/390). ### Fixed diff --git a/src/utils/output.ts b/src/utils/output.ts index 61854dc5..d158fb5c 100644 --- a/src/utils/output.ts +++ b/src/utils/output.ts @@ -5,6 +5,7 @@ import { Table } from '@cliffy/table' import * as colors from '@std/fmt/colors' import { format as prettyBytes } from '@std/fmt/bytes' import { marked } from 'marked' +import pluralize from 'pluralize' import supportsHyperlinks from 'supports-hyperlinks' import type { SummaryOutput, ValidationResult } from '../types/validation-result.ts' import type { Issue, Severity } from '../types/issues.ts' @@ -28,27 +29,26 @@ interface LoggingOptions { * @param result - The validation result to format. * @param options - Logging options; `verbose` includes extra detail per issue. * @returns The full ANSI-coloured output string with newlines. + * + * Output order: nested datasets first (Sources before Derivatives; + * within Sources, `sourcedata/` entries before `rawbids/`), then the + * root dataset itself with a roll-up of nested-dataset issue counts. + * The root section comes last so a quick `tail` shows the + * top-level dataset's status and aggregate health at a glance. */ export function consoleFormat( result: ValidationResult, options?: LoggingOptions, ): string { - const output = [] - if (result.issues.size === 0) { - output.push(colors.green('This dataset appears to be BIDS compatible.')) - } else { - ;(['warning', 'error'] as Severity[]).map((severity) => { - output.push(...formatIssues(result.issues.filter({ severity }), options, severity)) - }) - } - output.push('') - output.push(formatSummary(result.summary)) - output.push('') + const output: string[] = [] const formatNested = ( label: string, nested: Record, + keyOrder: string[], ) => { - for (const [key, nestedResult] of Object.entries(nested)) { + for (const key of keyOrder) { + const nestedResult = nested[key] + if (!nestedResult) continue output.push(colors.blue(`${label}: ${key}`)) if (nestedResult.issues.size === 0) { @@ -62,16 +62,80 @@ export function consoleFormat( output.push('') } } + + // Sources: sourcedata first, then rawbids; alphabetical within each group. + if (result.sourcesSummary) { + const sourceKeys = Object.keys(result.sourcesSummary).sort((a, b) => { + const rank = (k: string) => + k.startsWith('/sourcedata') ? 0 : k.startsWith('/rawbids') ? 1 : 2 + return rank(a) - rank(b) || a.localeCompare(b) + }) + formatNested('Source', result.sourcesSummary, sourceKeys) + } + // Derivatives: alphabetical. if (result.derivativesSummary) { - formatNested('Derivative', result.derivativesSummary) + const derivKeys = Object.keys(result.derivativesSummary).sort() + formatNested('Derivative', result.derivativesSummary, derivKeys) } - if (result.sourcesSummary) { - formatNested('Source', result.sourcesSummary) + + // Root dataset section, last. + output.push(colors.blue('Root dataset:')) + if (result.issues.size === 0) { + output.push(colors.green('\tThis dataset appears to be BIDS compatible.')) + } else { + ;(['warning', 'error'] as Severity[]).map((severity) => { + output.push(...formatIssues(result.issues.filter({ severity }), options, severity)) + }) + } + output.push('') + output.push(formatSummary(result.summary)) + + // Roll-up of nested-dataset issue counts, embedded in the root section. + const rollup = formatNestedRollup(result) + if (rollup) { + output.push(rollup) } return output.join('\n') } +/** Tally errors/warnings across nested datasets and render a brief table. */ +function formatNestedRollup(result: ValidationResult): string { + const tally = (nested: Record | undefined) => { + if (!nested) return undefined + let errors = 0 + let warnings = 0 + for (const r of Object.values(nested)) { + errors += r.issues.get({ severity: 'error' }).length + warnings += r.issues.get({ severity: 'warning' }).length + } + return { count: Object.keys(nested).length, errors, warnings } + } + const sources = tally(result.sourcesSummary) + const derivs = tally(result.derivativesSummary) + if (!sources && !derivs) return '' + + const rows: string[][] = [] + const row = (label: string, t: { count: number; errors: number; warnings: number }) => [ + colors.magenta(label), + pluralize('dataset', t.count, true), + pluralize('error', t.errors, true), + pluralize('warning', t.warnings, true), + ] + if (sources) { + rows.push(row('Sources:', sources)) + } + if (derivs) { + rows.push(row('Derivatives:', derivs)) + } + return [ + '', + colors.magenta('Nested datasets summary:'), + new Table().body(rows).border(false).padding(2).indent(2).toString(), + '', + ].join('\n') +} + /** * Render marked tokens to ANSI strings */ From 25e551206917fa428915c8365be908007626ef24 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Sat, 2 May 2026 14:03:01 -0400 Subject: [PATCH 5/6] test: Cover sourcesSummary recursion and DatasetType normalization - detectErrors: assert errors in nested sourcesSummary are collected, mirroring the existing derivativesSummary case. - BIDSContextDataset opaqueDirectories: assert that an invalid DatasetType (e.g., "Raw"/"Derivative") is normalized via the schema's DatasetType.enum, falling back to derivative iff GeneratedBy is set. Co-Authored-By: Claude Code 2.1.126 / Claude Opus 4.7 (1M context) --- src/schema/context.test.ts | 25 ++++++++++++++++++++++++- src/summary/summary.test.ts | 22 ++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/schema/context.test.ts b/src/schema/context.test.ts index 0b104e59..45b29e26 100644 --- a/src/schema/context.test.ts +++ b/src/schema/context.test.ts @@ -41,7 +41,10 @@ Deno.test('test context LoadSidecar', async (t) => { Deno.test('BIDSContextDataset opaqueDirectories respects DatasetType', async (t) => { const schema = { - objects: { extensions: {} }, + objects: { + extensions: {}, + metadata: { DatasetType: { enum: ['raw', 'derivative'] } }, + }, rules: { directories: { raw: { @@ -101,6 +104,26 @@ Deno.test('BIDSContextDataset opaqueDirectories respects DatasetType', async (t) assertEquals(ds.opaqueDirectories.has('/c'), true) assertEquals(ds.opaqueDirectories.has('/a'), false) }) + + await t.step('invalid DatasetType normalizes to raw without GeneratedBy', () => { + const ds = new BIDSContextDataset({ + schema, + dataset_description: { DatasetType: 'Raw' }, + }) + assertEquals(ds.dataset_description.DatasetType, 'raw') + assertEquals(ds.opaqueDirectories.has('/a'), true) + assertEquals(ds.opaqueDirectories.has('/c'), false) + }) + + await t.step('invalid DatasetType normalizes to derivative with GeneratedBy', () => { + const ds = new BIDSContextDataset({ + schema, + dataset_description: { DatasetType: 'Derivative', GeneratedBy: [{ Name: 'x' }] }, + }) + assertEquals(ds.dataset_description.DatasetType, 'derivative') + assertEquals(ds.opaqueDirectories.has('/c'), true) + assertEquals(ds.opaqueDirectories.has('/a'), false) + }) }) Deno.test('test context loadSubjects', async (t) => { diff --git a/src/summary/summary.test.ts b/src/summary/summary.test.ts index 4ec651fc..6067a7b0 100644 --- a/src/summary/summary.test.ts +++ b/src/summary/summary.test.ts @@ -140,3 +140,25 @@ Deno.test('detectErrors returns false when no errors found', () => { const errors = detectErrors(mockValidationResultNoErrors) assertEquals(errors, false) }) + +const mockValidationResultBadSources: ValidationResult = { + issues: new DatasetIssues({ + issues: [], + codeMessages: new Map(), + }), + summary: json_mock_validation_result.summary, + sourcesSummary: { + '/sourcedata/mock/': { + issues: new DatasetIssues({ + issues: json_mock_validation_result.derivativesSummary['/derivatives/mock/'].issues + .issues as Issue[], + }), + summary: json_mock_validation_result.derivativesSummary['/derivatives/mock/'].summary, + }, + }, +} + +Deno.test('detectErrors collects errors from nested sources', () => { + const errors = detectErrors(mockValidationResultBadSources) + assertEquals(errors, true) +}) From 7743d10efe7f7df42d21ff80af7282e97b989372 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Wed, 3 Jun 2026 10:02:57 -0400 Subject: [PATCH 6/6] test: Cover sourcesSummary via integration on bids-examples/atlas-4S MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a recursion integration test that validates `bids-examples/atlas-4S` (which has `sourcedata/atlas-4S/dataset_description.json` — the multiple-nested shape under `sourcedata/`) and asserts that the returned `ValidationResult` carries a `sourcesSummary` keyed by a `sourcedata/` path. Closes the coverage gap codecov flagged on `src/validators/bids.ts` L264-265 (the `sourcesSummary` output assignment). The single-nested shape (container itself is a BIDS dataset, e.g. `rawbids/dataset_description.json`) remains uncovered — no fixture in bids-examples exhibits it yet. Co-Authored-By: Claude Code 2.1.161 / Claude Opus 4.7 (1M context) --- src/tests/local/derivatives.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/tests/local/derivatives.test.ts b/src/tests/local/derivatives.test.ts index 92a11343..a3d007da 100644 --- a/src/tests/local/derivatives.test.ts +++ b/src/tests/local/derivatives.test.ts @@ -11,3 +11,16 @@ Deno.test('recursive option works as expected', async (t) => { result = await validatePath(t, path, options) assert(Object.hasOwn(result.result, 'derivativesSummary')) }) + +Deno.test('recursive option populates sourcesSummary from sourcedata/', async (t) => { + // atlas-4S has sourcedata/atlas-4S/dataset_description.json — the + // multiple-nested shape under sourcedata/. + const path = 'tests/data/bids-examples/atlas-4S/' + const options = await parseOptions(['fake_dataset_arg', ...Deno.args, '-r']) + const result = await validatePath(t, path, options) + assert(Object.hasOwn(result.result, 'sourcesSummary')) + assert( + Object.keys(result.result.sourcesSummary ?? {}).some((k) => k.includes('sourcedata')), + 'expected a sourcedata/ entry in sourcesSummary', + ) +})