Skip to content

Commit 9f6ff01

Browse files
Merge pull request #50 from VH-Lab/claude/did-matlab-issue-55-i3pfhq
Remove a document completely when its last branch drops it
2 parents 633979e + 8a6f646 commit 9f6ff01

5 files changed

Lines changed: 574 additions & 106 deletions

File tree

‎PORTING_INSTRUCTIONS.md‎

Lines changed: 39 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,8 @@ document entry in `bridge.yaml`. `binaryTable`'s write path, `binaryTable.
411411
compare` and `fileCache` were ported 2026-08-29 and are no longer listed — see
412412
"The file cache" below. `database.exist_doc` and `add_docs`'
413413
`customFileHandler` were ported 2026-08-30 and are no longer listed — see
414-
"exist_doc and ingestion at add time" below.)
414+
"exist_doc and ingestion at add time" below. The three issue #55 rows were
415+
ported 2026-08-30 and are no longer listed — see below.)
415416

416417
| MATLAB feature | Bridge file | Priority |
417418
|---|---|---|
@@ -422,9 +423,6 @@ compare` and `fileCache` were ported 2026-08-29 and are no longer listed — see
422423
| `database.display_branches` | bridge.yaml | Low |
423424
| `database.close_doc` | bridge.yaml | Low |
424425
| `query.searchcellarray2searchstructure` handles only numbers and strings; MATLAB also handles cells, structs and logicals | bridge.yaml | Low |
425-
| Deleting ingested copies from the file cache on removal — MATLAB's `do_remove_doc` deletes each `files.cached_location` from disk before dropping the rows (issue #55) | bridge_implementations.yaml | Medium |
426-
| Retiring removed document ids — MATLAB's `deleted_docs` table and the `DID:SQLITEDB:DELETED_DOC` refusal in `do_add_doc` (issue #55) | bridge_implementations.yaml | Medium |
427-
| Reclaiming documents orphaned by a branch deletion — MATLAB's `do_delete_branch` runs the same reclamation as `do_remove_doc`; Python's drops the branch rows and stops (issue #55) | bridge_implementations.yaml | Medium |
428426

429427
### Issue #55: what landed in MATLAB, and what is left here
430428

@@ -439,35 +437,43 @@ row they reference. Python is the language that enforces that order
439437
(`PRAGMA foreign_keys = ON`), which is why the ordering bug surfaced here first
440438
as a document with an attached file refusing to be removed.
441439

442-
The other two are rows in the table above. Both are genuinely missing, not
443-
merely unreachable:
444-
445-
- **Cached files.** Both ingestion paths are implemented here now — remote in
446-
DID-python#42, local in DID-python#45 — so any location marked `ingest` is
447-
copied to `FileDir/<uid>` and recorded in `cached_location`, exactly as in
448-
MATLAB. `d7ac853` pointed both languages at one shared cache directory. A
449-
removal from Python therefore leaves a file behind that the same removal from
450-
MATLAB deletes, in a directory both of them read. This is the more pressing
451-
of the two rows: it now applies to ordinary documents, not just remote ones.
452-
- **Retired ids.** MATLAB records the id in a `deleted_docs` table and raises
453-
`DID:SQLITEDB:DELETED_DOC` on a re-add. Nothing stops the same re-add here.
454-
455-
Neither breaks reading a database across languages. `deleted_docs` is created
456-
on demand, is not one of MATLAB's mandatory tables, and MATLAB does not add it
457-
to a Python-written database merely by opening one.
458-
459-
A third row was added when MATLAB `a0bb373` finished the issue. `do_remove_doc`
460-
is not the only way a document comes to be referenced by no branch — deleting a
461-
branch is the other, and MATLAB now runs the same reclamation there.
462-
`_do_delete_branch` here still drops the `branch_docs` and `branches` rows and
463-
stops, so a branch deleted from Python leaves behind documents no branch
464-
references, with their field data and their ingested files.
465-
466-
Whoever ports it should carry the guard, not just the deletes: the reclamation
467-
fires only when **no** `branch_docs` row for the document survives anywhere, so
468-
a document that any other branch still holds is left alone. That is what makes
469-
it safe under DID's branch model, where deleting documents from one branch does
470-
not delete them from others.
440+
The other two were ported on 2026-08-30 (DID-python#49), along with a third
441+
that MATLAB `a0bb373` added when it finished the issue:
442+
443+
- **Ingested files.** `_reclaim_unreferenced_doc` collects the document's
444+
`files.cached_location` values and unlinks them. This became pressing once
445+
DID-python#45 closed local ingestion: any location marked `ingest` is copied
446+
to `FileDir/<uid>` here too, and `d7ac853` pointed both languages at one
447+
shared cache directory, so a removal used to leave a file behind that the
448+
same removal from MATLAB deleted.
449+
- **Retired ids.** A `deleted_docs` table, and a refusal in `_do_add_doc`.
450+
MATLAB raises `DID:SQLITEDB:DELETED_DOC`; Python raises `ValueError`, this
451+
class's convention for the same kind of refusal.
452+
- **Branch deletion.** `do_remove_doc` is not the only way a document comes to
453+
be referenced by no branch — deleting a branch is the other, and
454+
`_do_delete_branch` now runs the same reclamation through the same helper.
455+
456+
Two things are worth keeping in mind about that code:
457+
458+
**The guard, not just the deletes.** Reclamation fires only when **no**
459+
`branch_docs` row for the document survives anywhere, so a document any other
460+
branch still holds is left alone. That is what makes it safe under DID's branch
461+
model, where deleting documents from one branch does not delete them from
462+
others.
463+
464+
**One deliberate ordering difference.** MATLAB deletes the files from disk
465+
before the rows; Python collects the paths, deletes the rows, commits, and
466+
unlinks after. MATLAB runs no transaction, so the order costs it nothing; here,
467+
unlinking first would mean a later SQL failure rolled the rows back with the
468+
files already gone. The three SQL deletes stay in MATLAB's order.
469+
470+
Porting the retirement was a deliberate **behavior change** for Python, and one
471+
existing test asserted the old behavior (`test_remove_and_readd_doc`, now
472+
`test_removed_doc_id_cannot_be_readded`). Note that Python never had the
473+
stale-`doc_data` symptom the issue describes — it has reclaimed those rows
474+
since #39, so a re-added id got fresh ones. The retirement is here so ids are
475+
not re-used under the branch model, and so both languages agree about whether
476+
the operation is legal on a shared database file.
471477

472478
### Enumerated dependency lists (`name_1`, `name_2`, ...) — ported 2026-08-28
473479

‎src/did/did_matlab_python_bridge_implementations.yaml‎

Lines changed: 64 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -143,17 +143,25 @@ classes:
143143
144144
- name: do_add_doc
145145
python_name: _do_add_doc
146-
out_of_sync: true
147-
out_of_sync_reason: >
148-
Ingestion is fully in sync as of DID-python#45 (see sync_notes). What
149-
reopened this entry is unrelated to ingestion: MATLAB edb1a6b (issue
150-
#55) refuses a document whose id was previously removed from every
151-
branch, raising DID:SQLITEDB:DELETED_DOC after consulting the
152-
deleted_docs table. Python has neither the table nor the check, so
153-
re-adding a removed id still succeeds here. See the do_remove_doc
154-
entry, which is where that id is retired.
146+
out_of_sync: false
155147
sync_notes: >
156-
CLOSED 2026-08-30 (DID-python#45). Python's _do_add_doc now mirrors
148+
CLOSED 2026-08-30 (DID-python#49). _do_add_doc now refuses a document
149+
whose id was removed from every branch, as MATLAB edb1a6b does
150+
(issue #55). MATLAB raises DID:SQLITEDB:DELETED_DOC; Python raises
151+
ValueError, which is this class's convention for the same kind of
152+
refusal -- compare the OnDuplicate='error' path, where MATLAB's
153+
DID:SQLITEDB:DUPLICATE_DOC is likewise a ValueError here. The check
154+
runs after the branch-existence check, so a bad branch is still
155+
reported as a bad branch.
156+
157+
Note for anyone reading this as a bug history: unlike MATLAB, Python
158+
never had the stale-doc_data symptom the issue describes, because it
159+
has reclaimed the doc_data rows since #39 -- a re-added id got fresh
160+
rows. The retirement is here so that ids are not re-used under DID's
161+
branch model, and so that both languages agree about whether the
162+
operation is legal on a shared database file.
163+
164+
CLOSED 2026-08-30 (DID-python#45). Python's _do_add_doc mirrors
157165
MATLAB's ingestion loop in full. Three separate gaps were closed in
158166
sequence, recorded here because the entry was out_of_sync for each:
159167
@@ -236,31 +244,33 @@ classes:
236244
237245
- name: do_remove_doc
238246
python_name: _do_remove_doc
239-
out_of_sync: true
240-
out_of_sync_reason: >
241-
The reclamation itself is in sync (see decision_log). What remains is
242-
two things MATLAB edb1a6b added on top of it, neither of which exists
243-
here:
244-
245-
- Cached files are not deleted from disk. When the last branch
246-
reference goes, MATLAB reads files.cached_location for the document
247-
and deletes each ingested copy before dropping the rows. Python
248-
drops the rows and leaves the copies behind. Since DID-python#45
249-
closed local ingestion this applies to ordinary documents, not just
250-
remote ones: any location marked `ingest` is now copied to
251-
FileDir/<uid> here too, and the two languages share one cache
252-
directory (MATLAB d7ac853). A removal from Python therefore leaves
253-
a file behind that the same removal from MATLAB deletes, in a
254-
directory both of them read.
255-
- Removed document ids are not retired. MATLAB records the id in a
256-
new deleted_docs table and refuses it on the way back in; Python
257-
has neither the table nor the check, so a removed id can be added
258-
again. See the do_add_doc entry.
259-
260-
Neither breaks reading across languages. deleted_docs is created on
261-
demand and is not one of MATLAB's mandatory tables, so a
262-
Python-written database opens in MATLAB unchanged, and MATLAB does
263-
not write the table into one merely by reading it.
247+
out_of_sync: false
248+
sync_notes: >
249+
CLOSED 2026-08-30 (DID-python#49). Both of the things MATLAB edb1a6b
250+
added on top of the reclamation are now here too:
251+
252+
- Ingested copies are deleted from disk. _reclaim_unreferenced_doc
253+
collects files.cached_location for the document and unlinks each
254+
one. This mattered once DID-python#45 closed local ingestion: any
255+
location marked `ingest` is copied to FileDir/<uid> here too, and
256+
the two languages share one cache directory (MATLAB d7ac853), so a
257+
removal used to leave a file behind that the same removal from
258+
MATLAB deleted, in a directory both of them read.
259+
- Removed document ids are retired, in the same deleted_docs table.
260+
See the do_add_doc entry for the refusal on the way back in.
261+
262+
One deliberate difference in ordering. MATLAB deletes the files from
263+
disk BEFORE the rows; Python collects the paths, deletes the rows,
264+
commits, and unlinks after. MATLAB runs no transaction, so the order
265+
costs it nothing; here, unlinking first would mean a later SQL
266+
failure rolled the rows back with the files already gone. Unlinking
267+
after the commit can at worst orphan a file, which is recoverable.
268+
The three SQL deletes remain in MATLAB's order.
269+
270+
Reading across languages was never at risk and still is not:
271+
deleted_docs is created on demand in both, is not one of MATLAB's
272+
mandatory tables, and neither language writes it into a database
273+
merely by reading one.
264274
input_arguments:
265275
- name: document_id
266276
type_matlab: "char"
@@ -291,27 +301,27 @@ classes:
291301
292302
- name: do_delete_branch
293303
python_name: _do_delete_branch
294-
out_of_sync: true
295-
out_of_sync_reason: >
296-
Recorded 2026-08-30. Deleting a branch is the second way a document
297-
comes to be referenced by no branch at all, and MATLAB a0bb373
298-
(issue #55) now gives it the same clean-up its do_remove_doc does:
299-
it notes which documents the branch holds, drops the branch_docs
300-
rows, and hands every document with no surviving reference to
301-
reclaim_unreferenced_doc - deleting its cached files from disk and
302-
its doc_data, files and docs rows, and retiring its id.
303-
304-
Python's _do_delete_branch deletes the branch_docs and branches rows
305-
and stops, which is what MATLAB did before a0bb373. So a database
306-
that has had a branch deleted from Python keeps documents no branch
307-
references, with their field data and their ingested files, where
308-
the same operation in MATLAB would have removed them.
309-
310-
The guard is worth stating because it is what makes the reclamation
304+
out_of_sync: false
305+
sync_notes: >
306+
CLOSED 2026-08-30 (DID-python#49), matching MATLAB a0bb373. Deleting
307+
a branch is the second way a document comes to be referenced by no
308+
branch at all, so it owes the same clean-up do_remove_doc does.
309+
_do_delete_branch now notes which documents the branch holds before
310+
dropping its rows, then hands every document with no surviving
311+
reference to _reclaim_unreferenced_doc - the same helper, so the
312+
files, the rows and the id retirement all follow automatically.
313+
314+
The guard is the same test in both languages, and is what makes this
311315
safe under DID's branch model: it fires only when NO branch_docs row
312316
for the document survives anywhere, so a document any other branch
313-
still holds is untouched. Porting this means reusing that same test,
314-
not deleting whatever the branch happened to hold.
317+
still holds is untouched. Deleting documents from one branch does
318+
not delete them from other branches.
319+
320+
Still divergent, but unrelated to issue #55 and unchanged by it:
321+
MATLAB's database.delete_branch validates the branch, refuses a
322+
frozen one and refuses one with sub-branches, and resets
323+
current_branch_id afterwards. Python's does none of that - see the
324+
delete_branch entry in bridge.yaml.
315325
input_arguments:
316326
- name: branch_id
317327
type_matlab: "char"

0 commit comments

Comments
 (0)