Streaming deserialization part 3: sink

[bjester]

Summary

This change completes the streaming deserialization migration. It adds the sink, switches all callers to the new pipeline, and removes the legacy _deserialize_from_store path. Deserialization now runs as a stream, like serialization:

StoreModelSource → AppModelDeserialize → AppModelValidate → Buffer(500, by model) → DeserializeSink

New code in morango/sync/stream/deserialize.py

• DeserializeSink writes the resolved outcome of each task, one chunk at a time:
  • SAVE: The sink saves each record in its own savepoint, with pre_save and post_save muted.
  • DELETE: The sink deletes the app rows with one Django Collector for each model in the chunk.
  • PROPAGATE_DELETE and PROPAGATE_HARD_DELETE: The sink adds the records to the tracking tables with one bulk_create(ignore_conflicts=True) for each table.
  • ERROR: The sink saves the error on the Store record.
• A clean write sets dirty_bit=False and clears the error fields on a Store record. The sink does the clean writes of a chunk with one QuerySet.update.
• The sink saves each Store error one record at a time. We measured bulk_update as slower than saves of single records.
• deserialize_from_store(profile, sync_filter=None, skip_errored=False) is the entry point. It has the same shape as serialize_into_store, and it logs a count for each outcome.
• deserialize_store_model(store, fk_cache, sync_filter=None) deserializes one Store record outside of a pipeline run. It returns the validated app model, or None for a deleted record.
• The sink and deserialize_store_model use the same write helpers, delete_app_models and record_propagated_deletions.

Why the sink tests each save against two sets

The buffer holds up to one chunk of validated records before the sink writes them. FK constraints are DEFERRABLE INITIALLY DEFERRED. As a result, a save that breaks a constraint succeeds in its savepoint, but the transaction fails at commit.

Two writes of the sink can remove an FK target after a record validated against it:

1. The save of a parent fails, for example because of a unique violation, after its children passed validation.
2. A DELETE cascades to app rows that a dirty record references.

The sink records the IDs that failed and the keys of the cascaded rows. If a SAVE references one of them, the sink saves an error on its Store record and does not save the app model. Each record with a problem gets its own error, and the transaction always commits.

Callers and removal

• ReceiverDeserializeOperation and MorangoProfileController.deserialize_from_store call the new entry point. The public signature of the controller does not change.
• This change removes _deserialize_from_store, _save_deserialized_record, _validate_store_foreign_keys, _validate_missing_store_foreign_keys and _handle_deleted_store_foreign_keys. It also removes the imports that only these functions used.
• This change replaces Store._deserialize_store_model with deserialize_store_model. The new function returns the app model or None, not a tuple, and it has no defer_fks option.
• CHANGELOG.md gets a new 0.9.0 section that lists these changes. It names the replacement for each removed function, so that Kolibri can use it for its upgrade.

Behavior differences from legacy

• A record that propagates the deletion of its FK target now ends with dirty_bit=False. Legacy left it dirty.
• A dirty parent can be left out of the run by skip_errored or by the sync filter. Its children now validate against the older app row of the parent, and the sink saves them. Legacy failed them with MorangoMissingParent. This difference comes from the Part 2 transforms (Streaming deserialization part 2: transformations #390).
• Some models load a related object in clean_fields. For these models, a child in the same chunk as its dirty parent cannot load the parent yet, so it fails with MorangoDirtyParent.
• As a result, a dirty chain of depth d in one chunk needs d runs. We assume that Kolibri models do not load related objects in clean_fields, so this does not affect Kolibri.
• The test model Facility.clean_dereferences_parent now defaults to False to match Kolibri. Tests set it to True to cover the models that load related objects.
• A dirty grandchild of a record that a cascade deleted always gets a "was deleted" error and stays dirty. In legacy, the result depended on the order of the queries.
• If the save of a parent fails after its children passed validation, each child gets its own error. The cross-model path of legacy saved the children, and the transaction failed at commit.
• deserialize_store_model raises the error that the transforms resolved. This error can be MorangoDirtyParent, MorangoMissingParent or IntegrityError.
• The deserialization warnings now go to the logger morango.sync.stream.deserialize, not morango.sync.operations.

Not in scope

• A separate issue covers the record of cascaded deletions in the tracking tables. post_delete stays muted, as in legacy.
• deferred_clean_fields, ForeignKeyReference, TemporaryTable and the backend bulk helpers lose their only callers. Remove the deferred FK validation API and unused backend bulk helpers #396 tracks their removal before 0.9.0 ships.

References

• Umbrella issue: Morango memory performance improvements #279. Related issue: Reduce memory overhead of _deserialize_from_store #210, for the memory use of deserialization.
• Part 1: Streaming deserialize: add source stream and task #318. Part 2: Streaming deserialization part 2: transformations #390.
• Follow-up cleanup: Remove the deferred FK validation API and unused backend bulk helpers #396.

Reviewer guidance

Verification

• On SQLite, the full suite passes, with 644 tests passed and 2 skipped. The PostgreSQL CI job passed at c106140, before the changes from review.
• prek run --all-files passes, and makemigrations --check reports no changes.
• We turned off the sink guards and ran the tests again. All 4 DeserializeFromStoreTestCase tests that need the run to commit failed at commit with FOREIGN KEY constraint failed.

Test changes

• Before the caller switch, we replaced the comparisons in the Part 2 parity tests with literal values that we captured from legacy. The tests now run against the new pipeline. Only these expectations change:
  • The 3 rows that propagate a deletion: dirty_bit becomes False.
  • test_dirty_parent_not_in_run: The sink now saves the child and it ends clean. Legacy failed it with MorangoMissingParent.
• In DeserializationTestCases, the expected results do not change. Only the call and its keyword arguments change.
• In test_controller.py, only the logger name in test_unique_violation_logs_warning changes.
• SelfReferentialFKDeserializationTestCase::test_models_created_successfully passes only because of the new Facility default. It deserializes a full dirty chain in one run, which is the multi-run difference above.

Areas to read carefully

• DeserializeSink.consume writes in this order: errors, deletes, propagated deletions, saves, then the Store records. The deletes come before the saves, so that the save guard can see the rows that a delete in the same chunk cascades to.
• delete_app_models builds its Collector while post_delete is muted. Django looks for signal receivers when it decides to fast-delete, so the signal must be muted first.
• Each fast-deleted queryset costs one more SELECT, and only in chunks that have deletes.
• The integration tests use shared databases on disk. Do not run two test processes against them at the same time.

AI usage

I used Claude Code (Claude Opus 5.5) for this part, in a workflow of spec, plan, then implementation. I drafted the design spec and the implementation plan with Claude, and I reviewed and approved both.

A second AI review of the spec found three problems before implementation. We added the cascade case, replaced a query-count criterion that SQLite cannot meet, and documented the effect of the new Facility default.

I made the main decisions: the buffered sink, saves of single records instead of bulk_update, and a standalone function for the deserialization of one record. Claude implemented the plan one task at a time, and wrote failing tests first. Claude also captured the legacy parity values. It also turned off the guards and made sure that the new tests fail when a deferred constraint breaks.

[rtibblesbot]

🟡 Waiting for changes

Outcome: reviewed 3057b29

Last updated: 2026-10-20 01:03 UTC

[bjesterbot]

Executive Summary

This PR successfully completes the streaming deserialization migration (#279, #210). The implementation gives deserialization the same streaming pipeline architecture (StoreModelSource → AppModelDeserialize → AppModelValidate → Buffer → DeserializeSink) as serialization, eliminating unbounded memory usage during sync.

The resolution of the "sink lag" challenge with deferred foreign keys (DEFERRABLE INITIALLY DEFERRED) is particularly well designed: tracking both failed saves (failed_ids) and cascade-deleted rows (cascaded_keys) in DeserializeSink ensures that broken foreign key references are caught and recorded as individual Store errors rather than failing the transaction at commit time.

Findings Summary

• 🚨 Blocker (Must-Fix): None.
• ⚠️ Architecture & Design (Should-Fix): None.
• 💡 Tactical Suggestion (Optional):
  • morango/sync/stream/deserialize.py:542: Record error on task.errors in _record_error to keep in-memory DeserializeTask state synchronized.
  • morango/sync/stream/deserialize.py:462: Guard or assert chunk model uniformity in DeserializeSink.consume when calling delete_app_models.
• ✅ Positive Highlights:
  • Pre-save and post-save signals are muted once for the entire batch of saves.
  • Cascade tracking inspects collector.fast_deletes via values_list("pk", flat=True) to track deleted rows without suppressing Django fast-deletes.
  • Comprehensive legacy parity tests with frozen literal expectations guarantee backward compatibility.

Verification Status

• Test Suite: 644 passed, 2 skipped (pytest).
• Pre-commit / Linters: Clean (prek run --all-files passed ruff, ruff format, yaml, and file hygiene checks).
• Django Migrations: makemigrations --check reports no unapplied schema changes.

Comments on Lines Outside Diff

(None)

Generated by Antigravity using model Gemini 3.8 Flash

[rtibblesbot]

PR #392: no correctness bugs found. Chunk ordering, delete-before-save, cascade tracking and FK-failure propagation match the removed legacy path.

CI pending.

• suggestion: deferred_clean_fields now has no production caller (inline).
• nitpicks: one type annotation, rationale comments, duplicated and brittle tests (inline).

After this review: Full code review.

• Confirm the streaming deserialize_from_store path preserves sync semantics for downstream (Kolibri) callers, since the legacy Store._deserialize_store_model and defer_fks are gone.
• Check that the morango.sync.stream.deserialize logger rename in the changelog is acceptable for anyone's log config.

---

Written by rtibblesbot, an LLM-based coding agent.

[bjester]

Follow-up on the two checks in the review summary from @rtibblesbot:

• Kolibri callers: Kolibri pins morango==0.8.15, so nothing changes for it until it upgrades to 0.9.0. When it does, two call sites need updates:

  • kolibri/core/auth/sync_operations.py calls _deserialize_from_store(profile, filter=...).
  • kolibri/core/auth/test/helpers.py calls store._deserialize_store_model({}).

  The 0.9.0 changelog now names the replacement for each, and the renamed filter and skip_erroring arguments (58c3f5c).

• Logger rename: Kolibri does not configure a logger named morango.sync.operations. All matches in Kolibri are imports of the operation classes, so the rename does not affect it.

[rtibblesbot]

PR #392: 4 of 6 prior findings resolved; 2 acknowledged. No new findings. CI pending.

Prior-finding status

ACKNOWLEDGED — morango/models/core.py:546 — deferred_clean_fields has no production caller (tracked in #396)
RESOLVED — morango/sync/stream/deserialize.py:445 — counts annotation looser than its keys
ACKNOWLEDGED — morango/sync/stream/deserialize.py:521 — comments restating rationale
RESOLVED — tests/testapp/tests/sync/stream/test_deserialize.py — duplicate of test_failed_parent_save__cross_model
RESOLVED — tests/testapp/tests/sync/stream/test_deserialize.py:1683 — self-referential one-level-per-run test
RESOLVED — tests/testapp/tests/sync/stream/test_deserialize.py:1725 — "'save': 1" pins dict repr

After this review: Full code review.

• Confirm the streaming pipeline fully replaces the removed legacy deserialization in operations.py and Store, including error and cascade-delete semantics other sync code relies on.
• Check the 0.9.0 changelog entry describes the behaviour change accurately for downstream integrators.

---

Written by rtibblesbot, an LLM-based coding agent.