Upload a series manifest from DID's file cache after ingest (#306)

[stevevanhooser]

Closes #306. Fixes the real bug the round-trip test PR #305 caught on TEST_USER_1 last night: uploadDataset silently sends no bytes for a series manifest, so every follow-on download 404s on the manifest uid, both reconstruction paths land on ingest_locations empty, and DID's MembersNotLocatable guard refuses the doc.

Root cause

file_uploads_for_document walks files.file_info[].locations[].location and requires os.path.exists(candidate) on the recorded path.

did.document.Document.add_file_series writes the manifest to a tempfile.NamedTemporaryFile(prefix="did_manifest_", suffix=".manifest", delete=False), and add_file defaults delete_original=True on a file location. So database_add copies the manifest into <FileDir>/<uid> and removes the tempfile. By the time the upload's file pass runs, the recorded location is gone; the bytes are still on disk keyed by uid, but the upload never sees them and the manifest never reaches the cloud.

Reproduced locally:

=== AFTER database_add ===
  location: /tmp/did_manifest_kgwlwr83.manifest    # <-- gone
  exists: False

=== check DID FileDir ===
  cached file: 41269720e0a8f771_...  189 bytes    # <-- bytes still on disk under the uid

The fix

Teach the upload path to fall back to did.file.cached_path_for_uid(uid, additional_roots=[...]) when the recorded location no longer resolves. Same helper the read path uses for on-demand file resolution; a pure function of the uid and the roots.

• src/ndi/cloud/upload.py: file_uploads_for_document(doc, *, additional_roots=None) and uploadFilesForDatasetDocuments(..., additional_roots=None) gain the parameter. _cached_path_for_uid isolates the DID call so a did.file import failure never breaks the ordinary path.
• src/ndi/cloud/orchestration.py: adds _did_file_store_roots(dataset) which pulls dataset._session._database.binary_path (the DID FileDir). uploadDataset passes it in during the sync_files=True file pass.
• src/ndi/cloud/sync/operations.py: _upload_binaries gains a dataset kwarg; all three call sites (uploadNew, mirrorToRemote, twoWaySync) pass their dataset through. _did_file_store_roots is reused.

The recorded location is still preferred when it resolves. Only files whose source path has moved (ingested manifests, or any user file the caller deleted post-add_file) reach the fallback. Ordinary docs whose source paths are still on disk upload from those paths, unchanged.

Tests

New file tests/test_cloud_upload_uses_did_cache.py, eight offline tests:

• TestTheFallbackFindsAnIngestedManifest -- pure file_uploads_for_document shape:
  • fallback finds (uid, cached_path) when additional_roots contains <uid>;
  • without roots the fallback does not fire (backwards compatible);
  • a live recorded location is preferred over the cache;
  • no uid, or neither location resolves, yields nothing;
  • MATLAB-shaped bare-dict locations still works.
• TestEndToEndAgainstARealDatabase:
  • build a demoNDISeries doc via add_file_series, database_add it, confirm the recorded location is gone, confirm the manifest bytes are in DID's FileDir, and confirm file_uploads_for_document(doc, additional_roots=_did_file_store_roots(dataset)) yields (manifest_uid, cached_path).
  • _did_file_store_roots(None) / object() returns [] -- reads defensively so a stand-in dataset does not stop an upload.

The live-cloud end-to-end coverage remains PR #305's tests/test_cloud_file_series_round_trip.py; with this fix, it stops failing on the download side (the manifest bytes will actually be on the cloud now).

Verification

pytest tests/test_cloud_upload_uses_did_cache.py \
       tests/test_cloud_upload_orchestration.py \
       tests/test_sync_binaries_and_deltas.py \
       tests/test_sync_binaries_gate_the_index.py \
       tests/test_cloud_series_reconstruction.py \
       tests/test_cloud_series_reconstruction_remote.py \
       tests/test_cloud_download_ordering.py

73 passed. Bridge checks (139 tests) green with the local NDI-matlab checkout. black --check and ruff check clean.

References

• Live-cloud round-trip test: a downloaded series' members must be readable (SyncFiles=true and =false) #304 (the round-trip test)
• Live-cloud file-series round-trip test (#304) #305 (the round-trip test's PR that caught this)
• Actions run 34924762494 (TEST_USER_1, where the failure surfaced)
• Rebuild series ingest_locations on SyncFiles=false download (companion to VH-Lab/NDI-matlab#988) #302 / PR Rebuild series ingest_locations on SyncFiles=false download (#302) #303 (the SyncFiles=false ingest_locations rebuild that already handled the download side; this closes the upload half)

🤖 Generated with Claude Code

https://claude.ai/code/session_01AdGZ8fRPeY9JSpUHgXm9ao

---

Generated by Claude Code

[stevevanhooser]

Closing this without merging. The fix is at the wrong layer and only half-solves #306.

Two problems with this approach:

1. Wrong source of truth. file_uploads_for_document reads files.file_info[].locations[].location from the document dict — the authoring record. That path is the source the caller passed to add_file, and DID's ingest (by default delete_original=True) removes it. The additional_roots fallback added here papers over that at the upload layer, but the real fix is to ask DID's storage where the bytes live — which is exactly what NDI-matlab's list_binary_files.m does via database_existbinarydoc.

2. Series members are missed. A file series is manifest + N members. Members have no file_info entry — they're enumerated from series_info[k].count and named NAME_1, NAME_2, …. This PR only recovers the manifest. NDI-matlab iterates series_info(s).count and calls database_existbinarydoc(doc, "NAME_i") for each. Without that enumeration the upload still misses every series member.

Reshaping from scratch on a new branch to mirror MATLAB's pattern: use dataset.database_existbinarydoc(doc_id, filename) as the source of truth, iterate document.current_file_list() for ordinary files (yields the manifest name for a series), and iterate series_info[k].count for the members. The replacement PR will close #306.

---

Generated by Claude Code