From d72eaa0c31e9f972fa2f7487f73d7a28d715b56b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 25 Mar 2026 16:03:41 +0000 Subject: [PATCH 1/5] Report every failed document with its ID and reason - Stop silently skipping FileExistsError (duplicates) in dataset init; report them alongside all other failures - Add _get_doc_id() helper for robust ID extraction in error messages - Each failure now shows "- : " (e.g. "ndi_document abc123 already exists" or DID validation errors) - Remove misleading "see preceding warnings" from downloadDataset error - Keep FileExistsError skip only in add_ingested_session (re-ingestion) https://claude.ai/code/session_015oe6ucKwCFznpu1PN1nAfz --- src/ndi/cloud/orchestration.py | 4 +--- src/ndi/dataset/_dataset.py | 28 ++++++++++++++++------------ 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/ndi/cloud/orchestration.py b/src/ndi/cloud/orchestration.py index 39756e7..41a6da8 100644 --- a/src/ndi/cloud/orchestration.py +++ b/src/ndi/cloud/orchestration.py @@ -132,9 +132,7 @@ def downloadDataset( parts.append(f"{db_lost} failed to add to the dataset database") raise RuntimeError( f"Downloaded {len(doc_jsons)} documents but only {db_count} " - f"were added to the dataset. {total_lost} documents lost: " - + "; ".join(parts) - + ". See preceding warnings for details on each failed document." + f"were added to the dataset. {total_lost} documents lost: " + "; ".join(parts) ) return dataset diff --git a/src/ndi/dataset/_dataset.py b/src/ndi/dataset/_dataset.py index cdc7620..7681720 100644 --- a/src/ndi/dataset/_dataset.py +++ b/src/ndi/dataset/_dataset.py @@ -169,11 +169,10 @@ def add_ingested_session(self, session: Any) -> ndi_dataset: self._session._database.add(doc) self._copy_binary_files(session, doc) except FileExistsError: - pass # Duplicates are expected and safe to skip + pass # Re-ingestion duplicates are expected except Exception as exc: - doc_id = getattr(doc, "id", "") - ingestion_failures.append((str(doc_id), str(exc))) - logger.debug("Skipping document %s during ingestion: %s", doc_id, exc) + doc_id = ndi_dataset_dir._get_doc_id(doc) + ingestion_failures.append((doc_id, str(exc))) if ingestion_failures: failure_details = "\n".join( f" - {doc_id}: {err}" for doc_id, err in ingestion_failures[:20] @@ -843,15 +842,9 @@ def __init__( for doc in documents: try: self._session._database.add(doc) - except FileExistsError: - pass # Duplicates are expected and safe to skip except Exception as exc: - doc_id = ( - getattr(doc, "id", None) or doc.get("base", {}).get("id", "") - if isinstance(doc, dict) - else "" - ) - failures.append((str(doc_id), str(exc))) + doc_id = self._get_doc_id(doc) + failures.append((doc_id, str(exc))) if failures: failure_details = "\n".join(f" - {doc_id}: {err}" for doc_id, err in failures[:20]) extra = f"\n ... and {len(failures) - 20} more" if len(failures) > 20 else "" @@ -1006,6 +999,17 @@ def dataset_erase(ndi_dataset_dir_obj: ndi_dataset_dir, areyousure: str = "no") "user did not indicate they are sure." ) + @staticmethod + def _get_doc_id(doc: Any) -> str: + """Best-effort extraction of a document ID for error reporting.""" + doc_id = getattr(doc, "id", None) + if doc_id: + return str(doc_id) + props = getattr(doc, "document_properties", doc) + if isinstance(props, dict): + return props.get("base", {}).get("id", "") + return "" + @staticmethod def _dataset_session_id_from_docs(documents: list[ndi_document]) -> str: """Extract the dataset session ID from a list of documents. From f635a7130905f000be530e8cd4959179989e52e7 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 25 Mar 2026 16:06:34 +0000 Subject: [PATCH 2/5] Include per-document failure details in downloadDataset error Store add failures on dataset instance (add_doc_failures) instead of raising from __init__, so downloadDataset() can collect them and report every failed document ID with its specific error reason in one error. Error output now looks like: Downloaded 7221 documents but only 7123 were added to the dataset. 98 document(s) lost: 98 failed to add to the dataset database: - abc123def: ndi_document abc123def already exists - 456ghi789: ... https://claude.ai/code/session_015oe6ucKwCFznpu1PN1nAfz --- src/ndi/cloud/orchestration.py | 30 ++++++++++++++++-------------- src/ndi/dataset/_dataset.py | 14 +++++--------- 2 files changed, 21 insertions(+), 23 deletions(-) diff --git a/src/ndi/cloud/orchestration.py b/src/ndi/cloud/orchestration.py index 41a6da8..f1e61be 100644 --- a/src/ndi/cloud/orchestration.py +++ b/src/ndi/cloud/orchestration.py @@ -113,27 +113,29 @@ def downloadDataset( if verbose: print(f' Files downloaded: {report["downloaded"]}, failed: {report["failed"]}') - # Check how many documents actually made it into the dataset - from ndi.query import ndi_query as _ndi_query - - db_docs = dataset.database_search(_ndi_query("").isa("base")) - db_count = len(db_docs) - db_lost = len(documents) - db_count + # Collect all failures: conversion failures + database add failures + all_failures: list[tuple[str, str]] = list(getattr(dataset, "add_doc_failures", [])) + db_lost = len(all_failures) total_lost = conversion_lost + db_lost if verbose: print("Download complete.") if total_lost > 0: - parts = [] + lines = [ + f"Downloaded {len(doc_jsons)} documents but only " + f"{len(doc_jsons) - total_lost} were added to the dataset. " + f"{total_lost} document(s) lost:" + ] if conversion_lost > 0: - parts.append(f"{conversion_lost} failed to convert from JSON to ndi_document") - if db_lost > 0: - parts.append(f"{db_lost} failed to add to the dataset database") - raise RuntimeError( - f"Downloaded {len(doc_jsons)} documents but only {db_count} " - f"were added to the dataset. {total_lost} documents lost: " + "; ".join(parts) - ) + lines.append(f"\n{conversion_lost} failed to convert from JSON to ndi_document") + if all_failures: + lines.append(f"\n{db_lost} failed to add to the dataset database:") + for doc_id, reason in all_failures[:50]: + lines.append(f"\n - {doc_id}: {reason}") + if len(all_failures) > 50: + lines.append(f"\n ... and {len(all_failures) - 50} more") + raise RuntimeError("".join(lines)) return dataset diff --git a/src/ndi/dataset/_dataset.py b/src/ndi/dataset/_dataset.py index 7681720..ccf799c 100644 --- a/src/ndi/dataset/_dataset.py +++ b/src/ndi/dataset/_dataset.py @@ -827,6 +827,10 @@ def __init__( self._path.mkdir(parents=True, exist_ok=True) + # Track documents that failed to add (list of (doc_id, reason) tuples). + # Callers (e.g. downloadDataset) can inspect this after construction. + self.add_doc_failures: list[tuple[str, str]] = [] + if documents is not None and documents: # Hidden 3rd argument: create from pre-loaded documents. # Mirrors MATLAB ndi.dataset.dir(reference, path_name, docs). @@ -838,20 +842,12 @@ def __init__( session_id=dataset_session_id, ) # Bulk-add all documents to the database - failures: list[tuple[str, str]] = [] for doc in documents: try: self._session._database.add(doc) except Exception as exc: doc_id = self._get_doc_id(doc) - failures.append((doc_id, str(exc))) - if failures: - failure_details = "\n".join(f" - {doc_id}: {err}" for doc_id, err in failures[:20]) - extra = f"\n ... and {len(failures) - 20} more" if len(failures) > 20 else "" - raise RuntimeError( - f"Failed to add {len(failures)} of {len(documents)} " - f"documents to dataset database:\n{failure_details}{extra}" - ) + self.add_doc_failures.append((doc_id, str(exc))) # Re-create session without forced ID (reads from database) self._session = ndi_session_dir(ref or "temp", self._path) elif path_or_ref is None and not ref: From 835f45a7ef8e712bb7674de35da7566e134b8d67 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 25 Mar 2026 16:21:37 +0000 Subject: [PATCH 3/5] Detect silent DID-python failures and write missingDocuments.json The 98 missing documents don't raise exceptions during database.add() - DID-python silently fails to persist them. To diagnose: - Cross-check uses raw DID get_doc_ids() instead of isa('base') query, which could miss docs whose type info wasn't stored correctly - Missing document IDs are listed in the RuntimeError message - Full JSON of all missing documents is written to missingDocuments.json in the dataset directory for inspection https://claude.ai/code/session_015oe6ucKwCFznpu1PN1nAfz --- src/ndi/cloud/orchestration.py | 78 +++++++++++++++++++++++++++++----- 1 file changed, 67 insertions(+), 11 deletions(-) diff --git a/src/ndi/cloud/orchestration.py b/src/ndi/cloud/orchestration.py index f1e61be..6677ef5 100644 --- a/src/ndi/cloud/orchestration.py +++ b/src/ndi/cloud/orchestration.py @@ -113,28 +113,84 @@ def downloadDataset( if verbose: print(f' Files downloaded: {report["downloaded"]}, failed: {report["failed"]}') - # Collect all failures: conversion failures + database add failures - all_failures: list[tuple[str, str]] = list(getattr(dataset, "add_doc_failures", [])) - db_lost = len(all_failures) - total_lost = conversion_lost + db_lost + # Collect failures: conversion + exception-tracked + silent (DID-python) + add_failures: list[tuple[str, str]] = list(getattr(dataset, "add_doc_failures", [])) + + # Cross-check using raw DID-python doc IDs (not isa('base') query, + # which might miss documents whose type info wasn't stored correctly). + db_ids = set( + dataset._session._database._driver._db.get_doc_ids( + dataset._session._database._driver._branch_id + ) + ) + + # Build a map from doc_id -> original JSON for missing-doc output + doc_json_by_id: dict[str, dict] = {} + for dj in doc_jsons: + did = dj.get("base", {}).get("id", "") if isinstance(dj, dict) else "" + if did: + doc_json_by_id[did] = dj + + # Find documents that were "added" (no exception) but aren't in the DB + tracked_ids = {f[0] for f in add_failures} + silent_failures: list[str] = [] + for doc in documents: + doc_id = ( + doc.document_properties.get("base", {}).get("id", "") + if hasattr(doc, "document_properties") + else doc.get("base", {}).get("id", "") + ) + if doc_id and doc_id not in db_ids and doc_id not in tracked_ids: + silent_failures.append(doc_id) + + total_lost = conversion_lost + len(add_failures) + len(silent_failures) if verbose: print("Download complete.") if total_lost > 0: + # Write missing documents to a JSON file for inspection + missing_docs_path = target / "missingDocuments.json" + missing_docs = [] + for doc_id in silent_failures: + if doc_id in doc_json_by_id: + missing_docs.append(doc_json_by_id[doc_id]) + else: + missing_docs.append({"base": {"id": doc_id}}) + for doc_id, reason in add_failures: + entry = dict(doc_json_by_id.get(doc_id, {"base": {"id": doc_id}})) + entry["_add_error"] = reason + missing_docs.append(entry) + if missing_docs: + import json + + missing_docs_path.write_text(json.dumps(missing_docs, indent=2, default=str)) + lines = [ f"Downloaded {len(doc_jsons)} documents but only " - f"{len(doc_jsons) - total_lost} were added to the dataset. " + f"{len(db_ids)} were added to the dataset. " f"{total_lost} document(s) lost:" ] if conversion_lost > 0: - lines.append(f"\n{conversion_lost} failed to convert from JSON to ndi_document") - if all_failures: - lines.append(f"\n{db_lost} failed to add to the dataset database:") - for doc_id, reason in all_failures[:50]: + lines.append(f"\n{conversion_lost} failed to convert from JSON" " to ndi_document") + if add_failures: + lines.append(f"\n{len(add_failures)} raised errors during" " database add:") + for doc_id, reason in add_failures[:50]: lines.append(f"\n - {doc_id}: {reason}") - if len(all_failures) > 50: - lines.append(f"\n ... and {len(all_failures) - 50} more") + if len(add_failures) > 50: + lines.append(f"\n ... and {len(add_failures) - 50} more") + if silent_failures: + lines.append( + f"\n{len(silent_failures)} were passed to" + " database.add() without error but are NOT in the" + " database (possible DID-python bug):" + ) + for doc_id in silent_failures[:50]: + lines.append(f"\n - {doc_id}") + if len(silent_failures) > 50: + lines.append(f"\n ... and {len(silent_failures) - 50} more") + if missing_docs: + lines.append(f"\nFull JSON of missing documents written to:" f"\n {missing_docs_path}") raise RuntimeError("".join(lines)) return dataset From 11cc3d52d437615c9029b0dc4e488bd8f91f9975 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 25 Mar 2026 16:34:09 +0000 Subject: [PATCH 4/5] Add *.sqlite to .gitignore Prevents test artifact databases from being tracked. https://claude.ai/code/session_015oe6ucKwCFznpu1PN1nAfz --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 7ea6ef5..be55f9f 100644 --- a/.gitignore +++ b/.gitignore @@ -49,3 +49,4 @@ site/ # Generated tutorial HTML (keep locally for review, not in repo) tutorials/*.html +*.sqlite From 2932c3c15a823ae942af47fde6ce4e0ef88eea68 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 25 Mar 2026 16:38:50 +0000 Subject: [PATCH 5/5] Normalize document_class.superclasses from bare dict to list MATLAB's jsonencode unwraps single-element cell arrays to scalars. Documents with only one superclass (e.g. just 'base') arrive from the cloud with superclasses as a bare dict instead of a one-element list. DID-python's _get_superclass_str only handles lists, so meta.superclass was stored as empty string, causing isa('base') queries to miss 98 of 7221 documents. The fix extends _normalize_depends_on (which already handles the same MATLAB issue for depends_on and file_info) to also normalize document_class.superclasses. https://claude.ai/code/session_015oe6ucKwCFznpu1PN1nAfz --- src/ndi/document.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/ndi/document.py b/src/ndi/document.py index a497020..63b51f4 100644 --- a/src/ndi/document.py +++ b/src/ndi/document.py @@ -95,15 +95,16 @@ def __init__(self, document_type: Union[str, dict, "ndi_document"] = "base", **k self._set_nested_property(key, value) def _normalize_depends_on(self): - """Ensure depends_on and files.file_info are always lists. + """Ensure depends_on, files.file_info, and superclasses are always lists. MATLAB's jsonencode converts single-element cell arrays to scalars, so documents downloaded from the cloud (or created by MATLAB) may - have ``depends_on`` or ``files.file_info`` as a bare dict instead - of a list. Normalizing here guarantees that downstream code - (including DID-python's ``doc_to_sql`` / ``_serialize_depends_on`` - and all NDI code that iterates over ``file_info``) always sees - a list. + have ``depends_on``, ``files.file_info``, or + ``document_class.superclasses`` as a bare dict instead of a list. + Normalizing here guarantees that downstream code (including + DID-python's ``doc_to_sql`` / ``_serialize_depends_on`` / + ``_get_superclass_str`` and all NDI code that iterates over these + fields) always sees a list. """ dep = self._document_properties.get("depends_on") if isinstance(dep, dict): @@ -115,6 +116,12 @@ def _normalize_depends_on(self): if isinstance(fi, dict): files["file_info"] = [fi] + dc = self._document_properties.get("document_class") + if isinstance(dc, dict): + sc = dc.get("superclasses") + if isinstance(sc, dict): + dc["superclasses"] = [sc] + @property def document_properties(self) -> dict: """The document's properties as a nested dictionary."""