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 diff --git a/src/ndi/cloud/orchestration.py b/src/ndi/cloud/orchestration.py index 39756e7..6677ef5 100644 --- a/src/ndi/cloud/orchestration.py +++ b/src/ndi/cloud/orchestration.py @@ -113,29 +113,85 @@ 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 + # 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) - db_docs = dataset.database_search(_ndi_query("").isa("base")) - db_count = len(db_docs) - db_lost = len(documents) - db_count - total_lost = conversion_lost + db_lost + total_lost = conversion_lost + len(add_failures) + len(silent_failures) if verbose: print("Download complete.") if total_lost > 0: - parts = [] + # 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(db_ids)} 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) - + ". See preceding warnings for details on each failed document." - ) + 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(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 diff --git a/src/ndi/dataset/_dataset.py b/src/ndi/dataset/_dataset.py index cdc7620..ccf799c 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] @@ -828,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). @@ -839,26 +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 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))) - 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}" - ) + doc_id = self._get_doc_id(doc) + 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: @@ -1006,6 +995,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. 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."""