Add HED validation checks

[bendichter]

Closes #718.

This adds HED validation to the inspector using the validator that ndx-hed provides. There are two checks, both at BEST_PRACTICE_VIOLATION:

• check_hed_lab_metadata_exists (on NWBFile) fires when a file contains HED annotations but no HedLabMetaData declaring the version of the HED schema they use. It finds the annotated columns by neurodata type name rather than by class, so it works even when ndx-hed is not installed, in which case PyNWB builds those classes from the specification cached in the file.
• check_hed_annotations_valid (on DynamicTable) runs the validator and turns each error it reports into its own InspectorMessage, naming the column, the row, and the HED error code. An error that repeats down a column, such as the same misspelled tag on every row, is collapsed into a single message with the number of rows it affects.

A report on a file with a few deliberate mistakes looks like this:

0.0  Importance.BEST_PRACTICE_VIOLATION: check_hed_annotations_valid - 'DynamicTable' object with name 'trials'
       Message: HED validation error in column 'reaction_time' (TAG_INVALID): 'InvalidValueTag' in InvalidValueTag/# is not a valid base HED tag.

0.1  Importance.BEST_PRACTICE_VIOLATION: check_hed_annotations_valid - 'EventsTable' object with name 'test_events'
       Message: HED validation error in column 'HED', row 1 (TAG_INVALID): 'InvalidEventTag' in InvalidEventTag is not a valid base HED tag.

Per-Column Validation Rather Than validate_file

The natural entry point is HedNWBValidator.validate_file(nwbfile), but it calls to_dataframe() on every DynamicTable in the file, including tables that carry no HED at all. For the inspector that is not affordable: a file with a single HED column and a large Units table would have the whole ragged spike_times column read into memory, and the inspector is often run against files streamed over a network.

So the check is registered on DynamicTable, skips tables with no HED columns, and calls validate_table, which touches only the annotated columns. The trade-off is that the assembled (BIDS-style) validation is not performed, so errors that only appear when the columns of a row are combined, and timeline validation of Onset/Offset pairing, are not caught. Every individual HED string in the file is still validated, including the categorical annotations in a MeaningsTable, which is itself a DynamicTable with a HedTags column and so is validated in its own right (and attributed to itself, which also avoids reporting the same error twice).

I would be glad to be talked out of this if there is a cheaper way to get the assembled validation, for example an entry point that takes one table and does not materialize the columns it does not need.

Packaging and CI

ndx-hed is a new optional dependency, installed with pip install nwbinspector[hed]. It is pinned to >0.2.0 because 0.2.0 is the last release that predates the ndx-hed support for pynwb 4 and it imports ndx-events, which does not coexist with the EventsTable in pynwb 4. The import in the check module is guarded by try/except ImportError, so both checks always register and simply produce nothing when the package is missing, which keeps the check registry and the configuration files the same in every environment. I confirmed that path by blocking the import: the checks stay registered, report nothing, and raise no errors.

Because there is not yet an ndx-hed release with pynwb 4 support, I did not add hed to the extras that CI installs, and the new tests will skip there until that release lands. Locally I ran them against ndx-hed from main and the full suite passes. Once the release is out, adding hed to the pip install ".[dandi,zarr]" line in testing.yml is all that is needed.

Testing

tests/unit_tests/test_hed.py covers both checks: the passing and failing cases, the file with annotations but no schema version, the row that is reported, the collapsing of repeated errors, and a HedValueVector template. The module is skipped with importorskip when ndx-hed is absent. I also checked the whole path on a file written to disk and read back, and the docs build clean with both checks appearing in the auto-generated listing by importance.

@rly @oruebel @VisLab, I would appreciate your read on two things in particular: whether BEST_PRACTICE_VIOLATION is the right level for an invalid HED tag (I kept it off CRITICAL so that it does not gate DANDI publication on a brand new check), and whether giving up the assembled validation is an acceptable trade for the memory behavior.

🤖 Generated with Claude Code

[bendichter]

I opened hed-standard/ndx-hed#149 upstream, which is relevant to the trade-off described above, and it comes with a measurement that corrects part of what I wrote.

I benchmarked a file with a 300-unit Units table (20,000 spikes each) and a 50,000-row HED-annotated events table that also carries a 64-wide float column:

operation	time	peak memory
to_dataframe(units), a table with no HED	0.04 s	48 MB
to_dataframe(events), the annotated table	0.46 s	42 MB
validate_table(events), what this check does	91 s	0.6 MB
validate_file(nwbfile), the whole file	120 s	97 MB

The memory argument I gave for not using validate_file is weaker than I made it sound. Materializing the unrelated Units table cost 48 MB here, not the gigabytes I was worried about, though it does scale with the table: a Units table of 1,000 units at 100,000 spikes would be closer to 800 MB.

The dominant cost is neither entry point. Validation is per row and linear, about 0.48 ms per row in memory and 1.8 ms per row reading from a file, and it is completely insensitive to how many distinct annotations a column contains. A 4,000-row column of four distinct tags takes 1.9 s, while those four strings validated once each take 2 ms. validate_vector builds and validates a fresh HedString for every row even when thousands of rows repeat the same annotation, which is the normal shape of event annotation. That is what hed-standard/ndx-hed#149 addresses, and it takes the 4,000-row case from 1.12 s to 0.0011 s with output that is identical rather than merely equivalent.

Nothing in this PR needs to change today, but it does affect where this should end up. If ndx-hed both skips tables that carry no HED and stops revalidating repeated annotations, validate_file becomes affordable for the inspector and switching to it is worth reconsidering, since it would buy the assembled and timeline validation that this check currently gives up. What would remain are two things that are worth solving separately: a whole-file entry point attributes every message to root rather than to the table, and the two paths number rows differently, with the assembled path counting the header as row 1 and validate_table being 0-based.

Two smaller notes. The per-row cost applies to this check as it stands, so a file with a 50,000-row annotated column will take about 90 seconds here. I can do the same deduplication on the inspector side, validating the distinct values of a column and mapping each issue back to the rows that hold that value, so that the check does not have to wait on an ndx-hed release. That also feeds the message collapsing this PR already does. Say the word and I will add it. Separately, the ndx-hed changelog shows the next release is 1.0.0 rather than 0.3.0, which is why the new extra pins ndx-hed>0.2.0 instead of naming a version.

@VisLab, the upstream change is small and the equivalence checks are in the description there, so that is probably the better place to push back on it.

[VisLab]

@bendichter: The performance tests were particularly interesting. I have not examined the code yet in detail, but I would like to come to an overall understanding of the strategy before I dive in. There seem to be a lot of checks missing between simple HED tag validation and assembly. Some of these checks might be included at low cost using a different implementation. I think the timeline validation (which looks for relationships among rows could be skipped completely for now as you have done), but there may be some additional checks that would be useful.

1. Just creating a HedLabMetadata object makes sure it is valid or it throws an exception.
2. The check_hed_lab_metadata_exists based on existence of HED is good. I agree that ndx-hed extension inclusion isn't good especially if the extension is folded into the core.
3. HedTags columns can appear in regular DynamicTable or in a DynamicTable that is a MeaningsTable. In the latter case, it contains HED annotations for categorical columns. Checking each HED annotation for validation in either case is necessary, but if the column in the base DynamicTable contains a value that doesn't have a mapping in the MeaningsTable is that detected by the inspector as an error. (It is a HED error.) For HED, the reverse is not an error (e.g., the MeaningsTable can contain annotations for values that are not in the column it is annotating.).
4. In my first read, I didn't see anything about HedValueVector, which is a VectorData that has a single HED annotation that contains a # leaf. This annotation applies to the entire column. Depending on the Tag/# attributes (whether it has value classes and/or unit classes specified), it checks whether the actual column values actually comply with those class rules.
5. The HED validators ALWAYS short-circuit if ANY errors are found in the sidecar equivalent. For NWB this is the MeaningsTable + the HedValueVector. These are usually the source of the repeated errors.
6. The conversion to DataFrame is mainly used for "efficient" assembly across columns. It is possible that we might be able to come up with a different vectorized approach to improve efficiency, I'm not sure.
7. The test to make sure that a MeaningsTable does not contain any HedValueVector columns wasn't mentioned. This is in the HedNWBValidator.validate_file now and should be carried over.

I think other structural checks are built in to the creation of the classes themselves. Could you briefly clarify how the HedValueVector is being checked and what kinds of checks are being applied to the actual column values? Could you also clarify the short circuit strategy that is being employed? I will start reviewing the code, but your clarifications would be helpful.

[bendichter]

Thanks @VisLab — answers point by point, with the code updated where you found gaps (c4eaeec and 534df56).

1 (HedLabMetaData validates on construction): Agreed, and the check leans on that: it retrieves the HedLabMetaData from the file (constructed, and therefore schema-validated, at read time), and any HedFileError/ValueError raised while building the validator or validating the table is caught and reported as its own inspector message rather than crashing the inspection.

3 (data value with no mapping in the MeaningsTable): Added as check_meanings_table_includes_all_values. Since the completeness rule is stated by MeaningsTable itself, I implemented it as a general table check rather than a HED one, so it fires even when ndx-hed is not installed (and its tests run in CI today, unlike the HED test module, which skips until the next ndx-hed release). It compares the distinct values of the annotated column against the MeaningsTable's value column and reports the missing ones. Per your direction note: None/""/"n/a" are treated as missing data needing no entry, and unused entries in the MeaningsTable remain legal.

4 (HedValueVector): It is covered. validate_table dispatches HedValueVector columns to validate_value_vector, which first validates the template itself (placeholders allowed) and then substitutes each row's value for # and validates the expanded string, so the value-class/unit-class rules attached to the Tag/# are enforced against the actual column values. There is a test exercising this (test_check_hed_annotations_valid_of_a_value_vector).

5 (short-circuiting): Within a HedValueVector column, validate_value_vector short-circuits: if the template has errors, the per-row substitution pass is skipped. There is no sidecar-level short-circuit in the per-column path because there is no assembly step for a bad "sidecar equivalent" to poison — a MeaningsTable's annotations are validated in their own right and attributed to the MeaningsTable itself. The repeated-error noise that short-circuiting normally prevents is handled on the inspector side instead: identical errors down a column (same column, code, and tag) are collapsed into one message with the affected row count.

7 (no HedValueVector in a MeaningsTable): Good catch — that rule lives only in validate_file, so it was not carried over. Now added as its own check, check_hed_value_vector_not_in_meanings_table, implemented directly in the inspector (it reports a message rather than raising, and works even when ndx-hed is not installed).

On 2 and 6: agreed, and the assembly/vectorization discussion probably belongs with hed-standard/ndx-hed#149 — if that lands together with skipping HED-free tables, switching this to assembled validation is worth revisiting.

(The same push also fixes the link that was failing the external-link CI check: the HED resources site moved to https://www.hedtags.org/hed-resources/.)

[codecov-commenter]

Codecov Report

❌ Patch coverage is 46.08696% with 62 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.95%. Comparing base (9be6250) to head (534df56).

Files with missing lines	Patch %	Lines
src/nwbinspector/checks/_hed.py	31.81%	60 Missing ⚠️
src/nwbinspector/checks/_tables.py	92.30%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #719      +/-   ##
==========================================
+ Coverage   79.71%   80.95%   +1.23%     
==========================================
  Files          48       49       +1     
  Lines        1923     2037     +114     
==========================================
+ Hits         1533     1649     +116     
+ Misses        390      388       -2     

Files with missing lines	Coverage Δ
src/nwbinspector/checks/__init__.py	100.00% <100.00%> (ø)
src/nwbinspector/checks/_tables.py	96.68% <92.30%> (-0.76%)	⬇️
src/nwbinspector/checks/_hed.py	31.81% <31.81%> (ø)

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[VisLab]

This clarifies things a lot and we're making progress, but I would like to push back on the short-circuit strategy and suggest a possible modification.

On the short-circuit strategy: If I am understanding the inspector strategy correctly, it continues to evaluate rows of a DynamicTable after fatal errors and just collapses the row numbers (this is with respect to HED). I see the following problems with that.

1. It creates a LOT of extra overhead (with respect to HED validation) for things where there is a single point of failure (in a MeaningsTable or the annotation in the metadata of a HedValueVector).
2. Having a zillion row numbers associated with the error obscures the place where the user has to fix the problem.
3. It misses many checks that are currently done in the "Sidecar validation" such as checking the use of templates ({}) and definitions.

I propose the alternative (much lower overhead approach):

The current get_bids_tabular in bids2nwb converts a DynamicTable to a DataFrame and then uses that DataFrame to extract the sidecar equivalent dictionary. In hindsight that may not have been a brilliant strategy as far as validation efficiency in NWB goes. The sidecar dictionary could easily be extracted directly from the DynamicTable without the overhead of converting anything to a DataFrame. This sidecar could then be completely validated and the error messages converted to point to exactly what has to be corrected without referring to the contents of the rows of the DynamicTable proper. (In the hedtools we have ready changed some of the error messages to say "in the BIDS sidecar or NWB meanings table" but the whole error message and error context thing is going to need some work downstream so it doesn't refer to BIDS when its NWB.)

The bottom line is -- if any of these table-level HED annotations cause an error, there is no point in continuing HED validation at all because a) each error is likely to trigger errors on many if not all rows of the table and b) the errors it triggers are likely to be wrong anyway because validation assumes some basic semantics are being followed and have already been checked.

Summary of proposed steps.

1. I write a get_json_hed_dict function which gets the HED JSON dictionary directly from the DynamicTable (i.e., revise lines 185-251 of get_bids_tabular data and expose as a separate function with no Pandas.)
2. The Inspector call this function, creates a Sidecar, validates it, and massages the error messages as needed (or I could do a wrapper for the errors once I have a format for what the Inspector wants.)

If there are any errors (not just warnings), the Inspector should not continue with validation of HED in the table.

This is a very low cost -- the MeaningsTable generally have only a few rows and the HedValueVector only has a single annotation. Many additional things are checked besides basic tag validity.

Note: the proposed validation includes the validation of the UnitClass in the 'HedValueVector but not its ValueClass validation, which needs to be apply to the individual row values. However, many Tag/# tags do not have ValueClass attributes. If the tag doesn't have a ValueClass attribute there is no need to call it. (We may need to expose some of the internal functions in hedtools when apply that validation. As I read it, the above proposal does that validation automatically. I would like to not do that in the first short circuit phase.

The hedtools validation and how it integrates into the inspector:

1. Validate the sidecar (proposed here) and short circuit if ANY errors.
2. If table has a HedTags column in the main DynamicTable (not a MeaningsTable), validate the annotations in that column individually. There can be at most one such column. Each of these errors is a distinct error, but potential errors grow with table size, so some damping of reporting may be necessary. The key here is that if we did the sidecar short circuit, each of these errors is highly likely to be a distinct error. Short-circuit here too.
3. Now if we want to do the ValueClass validation on the HedValueVector columns, we could. The hedtools folds this in to the assembled annotation check. However, this could be relatively low cost if in NWB by short circuiting if no ValueClass. Also the underlying checks in HedTools are based on library of regular expressions and checks which might possibly be integrated into some of NWB's type checking. I think this is downstream.
4. Then assembly (not on the first pass, but I think it should be an option in the inspector) -- maybe two level. The assembly could be redone to not use Pandas, but I would need to understand more of the DynamicTable internals to propose an efficient way to do that.