Series member ingestion diverges from MATLAB: silent handler failure, mode "ingest" vs "add", and no context for ordinary files #71

Found while writing the add-side tests for #68, comparing sqlitedb._ingest_series_members against DID-matlab's member loop (+did/+implementations/sqlitedb.m:636-691). Three divergences, listed worst first. None is exercised by any test today, which is why they survived the port.

1. A handler that produces no file fails silently

MATLAB, after dispatching the handler for a series member:

status = isfile(destPath);
if ~status
    errMsg = sprintf('customFileHandler did not produce a file at "%s"', destPath);
end
...
if ~status
    warning('DID:SQLiteDB:add_doc','Failed to cache "%s" %s referenced in document object: %s', ...);
end

Python's _ingest_series_members dispatches the handler inside a try and warns only if it raises. A handler that returns cleanly having written nothing produces no warning at all — the member is simply absent, and the caller is never told.

DID-python already gets this right for ordinary files: _ingest_location clears any stale dest_path first, then checks os.path.isfile(dest_path) afterwards and warns "custom_file_handler did not produce a file". tests/test_add_docs_file_handler.py::test_a_stale_copy_is_not_passed_off_as_freshly_retrieved covers that path. The series path has neither half.

Reproduction:

def handler(dest_path, source_path, context):
    pass                      # a plausible no-op: nothing found at the source

doc = Document("demoSeries")
doc.add_file_series("chunkdata.bin", ["ndic://d-123/f-abc"])

with warnings.catch_warnings(record=True) as caught:
    warnings.simplefilter("always")
    db.add_docs([doc], validate=False, custom_file_handler=handler)

caught                                          # []  -- MATLAB warns here
db.exist_doc(doc.id(), "chunkdata.bin_1")       # (False, None)

The missing stale-file clear is the same defect's other half: _ingest_series_members short-circuits on if os.path.exists(dest_path): continue, which is right for re-adding a document whose members are already in place, but it also means a partial file left by an earlier failed fetch is accepted as this member's bytes without anything having produced them.

2. The ingest-time mode is "ingest" in Python and 'add' in MATLAB

MATLAB sets 'mode', 'add' in the handler context, in both the file loop (:567) and the series member loop (:664), and 'open' on the read path (:1084, :1527). Python sends "open" on the read path — matching — but "ingest" at sqlitedb.py:1590.

mode is a string the caller's handler switches on, so this is a shared-contract divergence, not an internal name. A handler written against DID-matlab's documented contract falls through its case 'add' and does the wrong thing, or nothing. NDI-python#215 is about to write exactly one such handler for both languages.

Either value is defensible in isolation; they cannot both be right. MATLAB leads, so "add" unless there is a reason recorded to deviate.

3. Ordinary-file ingestion passes no context at all

_ingest_location calls the handler bare:

custom_file_handler(dest_path, location)

MATLAB dispatches it with the full context (documentId, filename, seriesName: '', uid, mode: 'add') through dispatchCustomFileHandler, the same helper Python has and uses on the series path and the open path. So a three-argument handler gets a context everywhere except here, where it silently gets called with two arguments instead.

def handler(dest_path, source_path, context=None):
    ...

db.add_docs([doc], validate=False, custom_file_handler=handler)   # context is None

The fix is to route this call through _dispatch_custom_file_handler with the same context shape, seriesName: "".

A fourth, lower-priority note

add_file_series types any non-http(s) location as location_type: "file" — matching MATLAB's localIngestLocations exactly — but the two ingest loops then key on different things. MATLAB dispatches on strcmpi(location_type,'file'), so an ndic:// member typed "file" reaches copyfile and fails; Python's _ingest_series_members dispatches on _is_remote_location, which sees the :// and hands it to the handler. Python's behaviour is the more useful of the two, but a document either language authors with an ndic:// member is ingested by only one of them. MATLAB's own test builds that document by hand and notes the same thing. Worth settling upstream rather than here.

Effect on the tests for #68

The add-side test PR pins what is not in dispute and leaves these alone:

• the silent-failure case asserts only that the document still lands and the member is absent — it does not assert that no warning is raised, which would lock in item 1;
• the context test asserts the key set and the four uncontested values, and that mode is present, but not that it equals "ingest", which would lock in item 2;
• item 3 is on the ordinary-file path and is out of that PR's scope entirely.