Syncrule resurrection: _update_syncgraph_in_db re-adds syncrule documents under ids DID has retired #67

The name

Syncrule resurrection. A syncrule document is deleted and then brought back under the same id — an id DID has, as of today, permanently retired.

Shared with NDI-matlab, where this code came from: VH-Lab/NDI-matlab#898. MATLAB is the source of truth, so the fix should be decided there and ported here. This issue exists so the Python side is tracked and does not get "fixed" independently into a divergence.

What happens

ndi.session.session_base._update_syncgraph_in_db removes the stored syncgraph and syncrule documents, then re-adds documents built from the same in-memory rule objects. Those carry the rules' original ids. Since DID-matlab#55 and its DID-python port, an id removed from its last branch goes into deleted_docs and is refused on any later add:

ValueError: Cannot add document '...' - a document with this id was previously
removed from every branch. Document ids are never re-used; generate a new id
for a new document.

Why the syncgraph is fine and the rules are not

As of the current branch this function rebuilds the syncgraph before generating documents, so the syncgraph document gets a fresh id:

rebuilt = ndi_time_syncgraph(self)
for rule in self._syncgraph.rules:
    rebuilt.add_rule(rule)
self._syncgraph = rebuilt

That mirrors NDI-matlab's newsyncgraph = ndi.time.syncgraph(...). The rules are not covered: add_rule appends the object it is handed without cloning, and syncrule.new_document() uses rule.id, so every surviving rule is re-added under the id its deleted document had.

Reproducing it

session.syncgraph_addrule(ndi_time_syncrule_filematch())  # update 1: nothing to delete
session.syncgraph_addrule(another_rule)                   # update 2: rule 1 deleted, then re-added

syncgraph_rmrule on the only rule does not trigger it — the removed rule is not re-added. That is why the existing test_syncgraph_addrule / test_syncgraph_rmrule pair passes and this stays hidden: neither test performs a second add with a surviving rule.

A regression test should be part of the fix, and it needs to be that two-add shape.

Why this was not fixed alongside the batch/rebuild change

The commit that added the rebuild and batched the add ("Rebuild and batch-add the syncgraph, as NDI-matlab does") was fixing two other defects DID's changes exposed in this same function — one-at-a-time adds breaking the syncrule_id_1 dependency now that VH-Lab/DID-python#41 validates enumerated values, and reuse of the syncgraph's id.

Resurrection was found while doing that and deliberately left alone. Python inherits this code's shape from MATLAB, and patching it here first would convert a bug the two languages share into a divergence they do not.

What to decide

Options are laid out in VH-Lab/NDI-matlab#898 and should be chosen there:

1. Give rules fresh ids on rebuild — cheap, but rule ids then churn on unrelated edits.
2. Delete only rules that are actually gone, leaving survivors untouched — preserves ids, closer to what the function means, needs a correct notion of "same rule" (add_rule already has an equality check).
3. Reconsider whether syncrule documents should be owned per-syncgraph at all.

Option 2 looks closest to correct.

Related

• Syncrule resurrection: update_syncgraph_in_db re-adds syncrule documents under ids DID has retired VH-Lab/NDI-matlab#898 — the MATLAB twin; decide there.
• Documents and document data should be permanently removed when they are removed from all branches VH-Lab/DID-matlab#55 and its DID-python port — the id retirement that armed this.
• Enumerated dependency values are never validated (shared with DID-matlab) VH-Lab/DID-python#41 — enumerated dependency validation, which exposed the sibling defect in this function.