Scoped field updates and context initialization issue

[bjester]

Summary

• Scopes model saves to TransferSession and SyncSession to only the fields that should have changed
• Adds regression test and fixes context initialization issue that could result in is_push of None when TransferSession.push is False

TODO

• Have tests been written for the new code?

Reviewer guidance

The tests passed with the scoped changes, and the regression test covers the context bug. Is that sufficient?

The context bug is more important in the context of Kolibri's sync-extras-plugin, since it was passing in a hardcoded value to is_push

Kolibri's morango integration tests pass with these updates.

Issues addressed

Related to this investigation:

• Background queuing causes int vs None comparison exception kolibri-sync-extras-plugin#14
• Correct behavior for background initialization, only useful for pulls kolibri-sync-extras-plugin#15

AI Usage

AI found the issue in the SessionContext.

[rtibblesbot]

🔵 Review posted

Last updated: 2026-07-08 23:54 UTC

[rtibblesbot]

PR #346 scopes sync-stage save() calls to update_fields and fixes the SessionContext.is_push coercion bug. The context fix and its regression test are clean, but one update_fields list drops fields the same code path assigns.

• blocking — validate_and_create_buffer_data no longer persists bytes_sent/bytes_received locally (inline).
• nitpick — a few update_fields lists include fields the branch doesn't always mutate (operations.py:1112 server_fsic on the client branch; operations.py:1504 records_total on push; core.py:336 both stage fields). Harmless since in-memory equals persisted, but if the goal is strict scoping they could be narrowed. Not worth changing on their own.
• suggestion — no test pins the "only these fields are written" contract; a future edit that mutates a field but forgets to extend update_fields would silently regress with no failure. The is_push fix is well-tested; consider a targeted test that mutates a field out-of-band and asserts a scoped save leaves it intact.

CI: the plain unit-test matrix was pending and the app-token check is failing (unrelated permissions step, not this diff) — hence COMMENT-level CI, but the byte-accounting bug drives REQUEST_CHANGES.

---

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Ran a phased review pipeline over the pull request diff:

• Classified the diff to select review passes (core, frontend, backend) and whether manual QA was required
• Core review pass checked correctness, design, architecture, testing, completeness, and DRY/SRP/Rule-of-Three principles
• Specialized frontend/backend review passes applied framework-specific lenses where those files changed
• For UI changes: manual QA and an accessibility audit against a live dev server, when available
• Checked CI status and linked issue acceptance criteria
• Synthesized one review from those passes and chose the verdict from the findings, CI status, and QA evidence

[rtibbles]

Changes make sense and align with the PR description.