There are two competing answers to "what identifiers does this .h5 contain", and they disagree:
load_existing_ids (now in data/embedding/store.py) returns set(f.keys()) — flat, top level only. This is the resume key for both embedding backends and the completeness gate._collect_datasets in data/loaders/h5.py walks one level of groups. This is what load_h5 — and therefore every downstream stage — actually sees.
Consequences on a grouped .h5 (e.g. one produced by another tool, with a prot_t5/ group holding the datasets):
- Resume re-embeds everything.
load_existing_ids returns {'prot_t5'}, which never intersects the protein ids, so every sequence is treated as outstanding.
- Duplicates are then written flat, alongside the grouped originals.
load_h5 silently keeps the new flat copies and drops the originals as duplicates — h5py iterates the root alphabetically, so P12345 precedes the prot_t5 group. A stale/fresh swap with no warning.
Separately, _collect_datasets walks exactly one level, so an identifier nested two levels deep (a/b/c) is handled correctly by neither function — load_h5 omits it entirely while load_existing_ids reports 'a'.
Suggested fix. Promote _collect_datasets to a public path-taking helper (dataset_names(h5_path)) owned by h5.py, and route the resume/completeness reads through it so the gate becomes an exact predicate on what load_h5 will produce. Note this changes resume semantics — a grouped third-party .h5 would start counting as already-embedded, which is correct but is a behaviour change worth calling out. cli/annotate.py already imports the private _collect_datasets and should move to the public spelling in the same change.
Prerequisite (already shipped in fix/embed-completeness-contract): both backends now reject identifiers containing / before writing, so protspace-produced files are always flat and the two views agree for them.
Found while reviewing #430.
There are two competing answers to "what identifiers does this
.h5contain", and they disagree:load_existing_ids(now indata/embedding/store.py) returnsset(f.keys())— flat, top level only. This is the resume key for both embedding backends and the completeness gate._collect_datasetsindata/loaders/h5.pywalks one level of groups. This is whatload_h5— and therefore every downstream stage — actually sees.Consequences on a grouped
.h5(e.g. one produced by another tool, with aprot_t5/group holding the datasets):load_existing_idsreturns{'prot_t5'}, which never intersects the protein ids, so every sequence is treated as outstanding.load_h5silently keeps the new flat copies and drops the originals as duplicates — h5py iterates the root alphabetically, soP12345precedes theprot_t5group. A stale/fresh swap with no warning.Separately,
_collect_datasetswalks exactly one level, so an identifier nested two levels deep (a/b/c) is handled correctly by neither function —load_h5omits it entirely whileload_existing_idsreports'a'.Suggested fix. Promote
_collect_datasetsto a public path-taking helper (dataset_names(h5_path)) owned byh5.py, and route the resume/completeness reads through it so the gate becomes an exact predicate on whatload_h5will produce. Note this changes resume semantics — a grouped third-party.h5would start counting as already-embedded, which is correct but is a behaviour change worth calling out.cli/annotate.pyalready imports the private_collect_datasetsand should move to the public spelling in the same change.Prerequisite (already shipped in
fix/embed-completeness-contract): both backends now reject identifiers containing/before writing, so protspace-produced files are always flat and the two views agree for them.Found while reviewing #430.