Port DID-matlab's file series work, the per-uid fetch lock, and fix the "uids are only unique per document" comment

Python side of VH-Lab/DID-matlab#173. The MATLAB half is merged in #183 and #184, with #185 green and pending.

Three of these are cross-language contracts, not ordinary ports: they only work if both languages agree exactly, in the same way <file>-lock and .fileCacheInfo had to agree. Those are marked ⚠ below.

1. The wrong comment (small, and it is here)

src/did/implementations/sqlitedb.py, in open_doc:

# Clear any earlier download at this path first. temppath
# persists between calls and uids are only unique per
# document, so without this a stale file would be served as
# though freshly retrieved -- and a handler that produced
# nothing would look like it had succeeded.

Uids are global, not per-document — they come from did.ido.unique_id, and this same file uses one as a global cache key via cache.full_path(str(uid)). #173 flags this comment as wrong and says it should go.

The os.remove it justifies is still worth having, but for the honest reason: a leftover from an earlier failed fetch at that path would otherwise be served as though fresh. Item 2 removes the need for it entirely.

2. ⚠ The fetch path: unique temp name, and one fetch per uid

Same code, a few lines down:

dest_path = os.path.join(temp_dir, str(uid))

One fixed scratch path per uid — the same defect DID-matlab#185 fixed. PathConstants().temppath is shared per user, so two processes fetching the same uid collide, and the os.remove above makes it worse in the concurrent case: one process can delete the file the other is mid-way through writing.

Mirror #185:

a unique temp name per fetch. MATLAB uses <uid>.<unique_id>.part in temppath. Python should use tempfile.mkstemp(dir=temp_dir, prefix=f"{uid}.", suffix=".part") — same directory, so a later os.replace stays atomic.
delete the partial when a fetch fails, so nothing is left to be mistaken for a completed download. Catch BaseException, not Exception, so KeyboardInterrupt cleans up too — that is one of the ways a partial gets created.
⚠ a per-uid fetch lock. MATLAB takes checkout_lock_file on <temppath>/<uid>-fetch-lock, with checkloops=30, throwerror=0, expiration=300. Python must use the same path or the two languages will not serialize against each other — exactly the failure the lock_file_key bridge entry already describes for the cache lock.

Note the argument choices, both deliberate and worth matching: throwerror=0 because correctness rests on the unique name and the add-file fallback, not on the lock, so a lock you cannot get must never fail a read; and expiration=300 rather than the 3600 default, because a holder that dies mid-fetch otherwise makes that uid slow to read for an hour, while expiring early costs only one redundant download.
after taking the lock, re-check the cache — the whole point is that the second caller finds the file already there instead of re-downloading.
treat losing the race at add_file as benign: if the cache already holds the uid, those are the same bytes, so use them rather than raising.

3. ⚠ File series (step 2 of #173)

src/did/implementations/sqlitedb.py has no occurrence of "series". None of this exists on the Python side yet.

⚠ The manifest is a shared binary format. docs/notes/file_series_manifest.md in DID-matlab is the spec — little-endian, DIDFSER1 magic, fixed-width NUL-padded uids, all-NUL for an absent member, optional source-name section. Both languages must read the same bytes, as with .fileCacheInfo. New MATLAB functions:

did.file.readSeriesManifest — whole manifest
did.file.writeSeriesManifest
did.file.readSeriesManifestUid — one slot, one seek, uid_width bytes; this is what makes member resolution O(1) and is the property the format was chosen for

The design decision to carry across: a series member gets no file_info entry and no files-table row. It is resolved through the manifest. Reproducing that is the whole point — per-member records would put back the ~8–11 MB of JSON the manifest exists to remove.

Resolution is: NAME_<i> parses as a member of declared series NAME → find NAME's manifest by its own uid → read slot i → the member's bytes are at <cache>/<uid> or <FileDir>/<uid>, the same two candidates in the same order as any other file.

Pieces to port:

MATLAB	what
`did.document/seriesNames`, `isFileSeries`, `seriesMemberOf`	declaration and the name-parsing rule
`did.document/seriesCount`, `seriesSourceRoot`, `seriesIngestLocations`	metadata, from memory
`did.document/addFileSeries`, `removeFileSeries`	authoring, incl. the `ingest` option for remote members
`did.document.stripSeriesIngestLocations`	member paths never reach stored JSON
`did.database/seriesHas`, `seriesMembers`	accessors; `seriesMembers` reads the manifest once and is the iterator
`did.database/cachedPathForFile`, `did.file.cachedPathForUid`	the no-SQL lookup (step 1), extended to members
`sqlitedb.do_add_doc`	copy members to `<FileDir>/<uid>`, insert no row
`sqlitedb.do_open_doc`, `check_exist_doc`	resolve `NAME_<i>` when the files query returns nothing

Two behaviours worth stating because they are easy to get subtly wrong:

seriesHas answers about the manifest, not the disk. True means the series records a member there, not that its bytes are local. Keep the two questions separate.
check_exist_doc must not fetch the manifest to answer; open_doc may. One reports local state, the other retrieves.

There is also a guard in do_add_doc that refuses a document whose series declares present members but records no ingest_locations — the shape a document has after a round trip through a database. It runs only for a document new to that database, and before any write.

4. Bridge yaml

New entries needed in did_matlab_python_bridge_file.yaml (the manifest readers/writer, cachedPathForUid), and in the document / implementations yamls for the rest. The MATLAB commits to pin matlab_last_sync_hash against are on DID-matlab main: 9b86b47, e0894f1, and #185's when it lands.

Three more DID-matlab changes landed on main after this was written (main is now d091f67), and they widen sections 2 and 3 rather than adding a separate concern. Noting them here so they're in view when the port reaches this code, not as anything separate to chase.

Section 2 — the fetch lock now has two call sites, not one. <temppath>/<uid>-fetch-lock with checkloops=30, throwerror=0, expiration=300 is unchanged, but sqlitedb/fetchSeriesMemberBytes (new in #189, 3299ab8) takes the same lock, under the member's uid, using the same unique .part name and the same benign-race handling at add_file. Whatever shape section 2's port takes, it wants to be shared between the two rather than written twice.

The reason it matters more here than for a whole-file read: a series is the case where one call resolves tens of thousands of uids, so an unlocked fixed-name scratch path is hit far more often than on the ordinary path.

Section 3 — a member whose bytes are absent can now be retrieved. #64 lists the resolution rule, which ends at "the member's bytes are at <cache>/<uid> or <FileDir>/<uid>". #188 added what happens when they are at neither:

A member has no orig_location, so the series manifest's location is handed to the handler as sourcePath, with the member's uid in the context (seriesName is populated at open time now, not only at ingest; mode is 'open'). DID composes no URL and learns no scheme.
Retrieval sits behind seriesMemberPath's mayRetrieve gate — open_doc sets it, check_exist_doc does not. That is the same "one reports local state, the other retrieves" split Port DID-matlab's file series work, the per-uid fetch lock, and fix the "uids are only unique per document" comment #64 already calls out, extended to the member.
Because sourcePath names the manifest, a handler that resolves it instead of reading context.uid fetches the manifest for every member. The result is compared against the manifest already in hand and refused with DID:SQLITEDB:FileSeries:HandlerReturnedManifest. That guard is load-bearing — #191 (717fbad) removed an earlier type check that excluded local-file manifest locations, because a lazily synced dataset has a local manifest with remote members, and excluding it made the whole mechanism inert for its main caller. Every location is offered now, remote ones first, and the byte comparison is what makes that safe.
A handler declared with only two positional inputs receives no context and so is never asked for a member; the arity rule is named once as sqlitedb.handlerTakesContext.

Section 4 — commits to pin: 3299ab8 (#189, the member fetch and its tests), 717fbad (#191), and c0fc48e (#190, the design note; docs/notes/file_series_manifest.md is still the manifest spec and now documents the retrieval contract too).

Generated by Claude Code

Section 3 now has artifacts to check against, which it did not when this was written. DID-matlab#193 is merged (main ae36c26), so every symmetry run writes:

<tempdir>/DID/symmetryTest/matlabArtifacts/file/seriesManifest/testSeriesManifestArtifacts/ — five manifests plus manifest.json
<tempdir>/DID/symmetryTest/matlabArtifacts/database/fileSeriesDocument/testFileSeriesDocumentArtifacts/ — a sqlite database with a sparse series ingested, plus manifest.json

Both manifest.json files state expected values in the file's own zero-based terms, so nothing has to be inferred from MATLAB source.

The manifests are chosen to fail a port that is only mostly right: sparse (all-NUL absent records), named (empty-name edges — first, an interior run, last; equal offsets mean empty, decreasing means corrupt, and converting to one-based before comparing is what broke DID-matlab's own reader in 5780144), allEmpty (a name section present but empty, distinct from no section), and wideUid (uid_width 40 — it is a header field, not the 33 unique_id happens to emit). The database artifact additionally pins the things a read-back test would not catch: exactly one files-table row, and ingest_locations present-but-empty in the stored document.

The reciprocal work, when Python has its half: reader tests on both sides, and a Python make_artifacts writing the same shapes under pythonArtifacts/. DID-matlab deliberately shipped no reader halves yet — its read suite asserts nSkipped == 0, so a reader parameterised over pythonArtifacts goes red the moment it lands and stays red until Python writes something. Generating first was the only order that keeps both sides green.

Generated by Claude Code

Landed in #65 (merged, commit d7dfbb4). Section-by-section:

§1 The wrong comment — gone. The os.remove it justified is redundant now that fetches use a unique scratch name (§2), and the honest reason (a leftover from a failed fetch) is covered by the partial-cleanup below.

§2 ⚠ Fetch path (sqlitedb._fetch_remote_to_cache):

Unique temp name per fetch via tempfile.mkstemp(dir=temp_dir, prefix=f"{uid}.", suffix=".part")
except BaseException: on the handler call — cleans the partial on KeyboardInterrupt too
Per-uid lock at <temppath>/<uid>-fetch-lock, check_loops=30, throw_error=False, expiration=300, exactly matching MATLAB's path so the two languages serialise against each other
Re-check the cache after taking the lock
Losing the race at add_file returns the cached copy rather than raising
Each uid gets one fetch attempt per open_doc (attempted_uids set)

§3 ⚠ File series — every listed piece ported:

Manifest binary format in did/file.py (read_series_manifest, read_series_manifest_uid O(1), write_series_manifest) matching docs/notes/file_series_manifest.md byte-for-byte
Document API: series_names, is_file_series, series_member_of, series_count, series_source_root, series_ingest_locations, add_file_series, remove_file_series, static strip_series_ingest_locations, plus file_uids and is_series_member
Database API: cached_path_for_file, series_has (manifest, not disk), series_members (the iterator), _do_cached_path_roots hook
SQLiteDB._ingest_series_members copies each member to <FileDir>/<uid> with no files-table row
SQLiteDB.open_doc / exist_doc resolve NAME_<i> through the manifest when the files query returns nothing; exist_doc never fetches, open_doc fetches via handler (DID-matlab #188/#191 guarded)
_reject_series_without_ingest_locations guard in _do_add_doc — the post-round-trip shape is refused
customFileHandler signature widened (DID-matlab #186): 3+ positional args or *args receive {documentId, filename, seriesName, uid, mode}

§4 Bridge yaml — new entries for readSeriesManifest, writeSeriesManifest, readSeriesManifestUid, cachedPathForUid, isSafeUid, and the 13 new document/database methods. Sync hashes bumped: database → ec1e733, document → 78583f6, sqlitedb → 717fbad, common/getCache → 4a0f9a5. bin/check_bridge_coverage.py gating checks all clean.

Symmetry: new tests/symmetry/{make,read}_artifacts/file/test_series_manifest.py pair mirrors tests_symmetry/+did/+symmetry/+makeArtifacts/+file/seriesManifest.m one-for-one (dense, sparse, named, allEmpty, wideUid); the full MATLAB↔Python symmetry job on the merged head is green.

One follow-up spun off from this batch: the binaryTable / fileCache lock-recovery port from DID-matlab 4a0f9a5 (resetLockState, fileCache.check, addFile roll-back-on-failure). Not part of #173 — filed separately and referenced from the out_of_sync note in the bridge YAML.

Closing.

Generated by Claude Code

Navigation Menu

Repository navigation

Port DID-matlab's file series work, the per-uid fetch lock, and fix the "uids are only unique per document" comment #64

1. The wrong comment (small, and it is here)

2. ⚠ The fetch path: unique temp name, and one fetch per uid

3. ⚠ File series (step 2 of #173)

4. Bridge yaml

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Port DID-matlab's file series work, the per-uid fetch lock, and fix the "uids are only unique per document" comment #64

Description

1. The wrong comment (small, and it is here)

2. ⚠ The fetch path: unique temp name, and one fetch per uid

3. ⚠ File series (step 2 of #173)

4. Bridge yaml

Activity

stevevanhooser commented on Sep 7, 2026

stevevanhooser commented on Sep 7, 2026

stevevanhooser commented on Sep 7, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions