Move content annotation onto the Django ORM

[rtibblesbot]

Summary

Content annotation is the slowest content operation on low-end hardware, and it ran through SQLAlchemy Core against tables reflected from the database. It now issues the same UPDATE … WHERE statements through the Django ORM. set_channel_ancestors stays raw parameterized SQL, because it builds a JSON string rather than expressing a query. Annotating a channel with no nodes no longer raises.

The checksum filter's inline-literal workaround for SQLITE_MAX_VARIABLE_NUMBER moves out of sqlalchemybridge.py and into a checksumin lookup in kolibri/core/mixins.py, alongside the UUID one it mirrors.

References

Fixes #15100. Depends on #15099. Blocks #15101. Benchmark harness: #15110. Acceptance criteria with evidence: first comment below.

Reviewer guidance

• kolibri/core/content/utils/annotation.py:455 — available_children_rollup strips MPTT's ordering so lft cannot join the GROUP BY. Check every rollup subquery in the diff goes through it.
• kolibri/core/content/utils/annotation.py:367 — with no destination, these writes now run on Django's connection and join an enclosing transaction.atomic() instead of committing independently. Check no caller relies on the old independence.
• kolibri/core/content/utils/annotation.py:216 — _calculate_batch_params came out byte-identical to import_export_content.py:24. Unifying them means moving it somewhere both can import, since import_export_content already imports through to annotation.

Deviations from the issue spec

• content/upgrade.py:19 still imports SQLAlchemy, against the "no SQLAlchemy" criterion: from sqlalchemy.exc import DatabaseError backs a live handler around import_channel_from_local_db, which runs through the out-of-scope channel_import.py. Is that the boundary the issue meant?

AI usage

Used Claude Code to convert the four modules and write their tests, following a plan I directed. Verified with the content suite on SQLite and PostgreSQL 16, the content operations benchmark against both channels, and prek.

---

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

How was this generated?

• Ran pre-flight CI checks (lint, format, tests) and verified all pass
• Rebased onto the target branch and resolved any conflicts
• Reorganized commit history into clean, logical commits
• Audited the diff to ensure only issue-relevant files are changed
• Built PR body from the repository's PR template with evidence blocks

🟡 Waiting for feedback Last updated: 2026-08-08 23:59 UTC

[rtibblesbot]

Acceptance criteria from #15100, ticked here because this account cannot edit an issue body.

General

• annotation.py, importability_annotation.py, tree.py and content/upgrade.py import no SQLAlchemy — one deviation, from sqlalchemy.exc import DatabaseError in content/upgrade.py:21, in the description.
• set_channel_ancestors produces byte-identical ancestors values, node for node — test_ancestors_are_byte_identical_to_the_rendered_json reads the raw column through a cursor and compares against the string rendered in Python, including the \" escape. Written and green against the SQLAlchemy implementation first.
• Topic availability matches the current implementation for the same fixture channel — AnnotationTreeRecursion, unchanged and green on both engines.
• Coach-content flags, coach-content counts and on-device resource counts match — same class, plus test_two_channels_no_annotation_collision_* for channel scoping.
• get_channel_node_depth returns 0 for a channel with no nodes — ChannelNodeDepthTestCase.test_a_channel_with_no_nodes_is_zero_deep.
• A checksum filter accepts 15,000 checksums without exceeding the SQLite variable limit — test_checksums_are_inlined_rather_than_bound asserts the compiled statement carries no bound parameters.
• An invalid checksum yields no results rather than raising — test_an_invalid_checksum_yields_no_results.
• Marking local file availability against a file path writes to that file — test_marks_availability_in_the_destination_file.
• That same call leaves the default database untouched — test_marking_a_destination_leaves_the_default_database_untouched, seeding the same checksum in both databases so only routing distinguishes them.
• django.contrib.postgres is imported inside the PostgreSQL branch — annotation.py, coach_content_aggregate.

Testing

• A test confirms .update() with a correlated Exists emits a correlated EXISTS, before the conversions it underpins — CorrelatedExistsUpdateTestCase, committed in Probe that .update() emits a correlated EXISTS ahead of every conversion commit. It asserts the outcome against a set computed off the fixture in Python, and that the single captured statement contains EXISTS and the outer table's own id column.
• A topic whose available children are all coach content is flagged coach content, on both engines — test_all_content_nodes_available_coach_content.
• A topic with a non-coach available child is not flagged, on both engines — test_one_root_content_node_coach_content.
• Nodes named in an exclusion list are left untouched — test_all_nodes_available_exclude_all, test_all_nodes_available_exclude_root, test_all_local_files_available_exclude_all.
• An inclusion list matching nothing in a batch range updates nothing — test_include_ids_absent_from_the_channel_change_nothing. Catches returning an unconstrained queryset where the SQLAlchemy version had where(false()), which would hide the whole channel.

Testing notes from the issue

• Annotation suites on SQLite and PostgreSQL 16 — run as the whole kolibri/core/content suite plus kolibri/core/test/test_mixins.py: 1485 passed / 3 skipped on SQLite, 1476 passed / 12 skipped on PostgreSQL 16. 17 tests added, none deleted.
• ancestors compared node by node before and after converting — the byte-identity test above.
• Correlated EXISTS probe passed on both engines, so the issue's stop condition did not fire.
• Benchmark all phases before and after, on both channels — --runs 3 against the merge-base then this branch, one sitting, overall verdict PASS. Annotation is faster: −2.8% on Khan Academy (English), −5.2% on the US curriculum channel. Every other phase moved within ±2.1%.

[github-actions]

Build Artifacts

Asset type	Download link
PEX file	kolibri-0.20.0a1.dev18_g31cb6f26e.d20260808.pex
Windows Installer (EXE)	kolibri-setup-0.20.0a1.dev18+g31cb6f26e.d20260808-unsigned.exe
Debian Package	kolibri_0.20.0a1.dev18+g31cb6f26e-0ubuntu1_all.deb
Debian Package	kolibri-server_0.20.0~a1.dev18+g31cb6f26e-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.20.0a1.dev18+g31cb6f26e.d20260808.dmg
Android Package (APK)	kolibri-0.20.0a1.dev18+g31cb6f26e.d20260808-debug.apk
Raspberry Pi Image	kolibri-pi-image-0.20.0a1.dev18+g31cb6f26e.zip
TAR file	kolibri-0.20.0a1.dev18+g31cb6f26e.d20260808.tar.gz
WHL file	kolibri-0.20.0a1.dev18+g31cb6f26e.d20260808-py3-none-any.whl

Smoke test screenshot

Download screenshot

[rtibbles]

Can you include the benchmark results in the PR body please? Just saying that you did the benchmark doesn't show anything.

[rtibbles]

Oh, you added in a comment, please put in the PR body clearly too.

[rtibbles]

This is looking good - a little cleanup for readability would be helpful. The additional tests are helpful to guard against regressions, is there anything else missing?

Also, I think 11 commits might be a bit much for these changes? A shorter commit history would seem quite helpful.

[rtibbles]

It's not clear to me what this means - ideally the test class name would be self explanatory, but if not, this needs to be clearer.

[rtibblesbot]

Renamed to ExistsInUpdateEvaluatesPerRowTestCase, and the docstring now names the caller it guards:

The leaf availability passes set available=Exists(...) in an update(), which is only correct if Django evaluates that Exists once per row, matching OuterRef("id") to the row being updated. Evaluated once for the whole statement instead, every node would come out with the same availability.

[rtibbles]

I appreciate the care taken to retain the comments in their original position. It does make me wonder if most of this could be done in Django, rather than needing raw SQL. If it's not possible, that's fine - but I think we might be better off pulling the comments out of the SQL string, and just making a shorter summary comment for the _ANCESTORS_SQL.

[rtibblesbot]

Comments pulled out of the SQL, replaced with a four-line summary above the constant.

Kept as SQL: this one concatenates a JSON string out of the parent's, so an ORM version is Concat over Substr/Length/Case/Replace with a Cast on every operand to keep Concat from rejecting the mixed JSONField/text types — longer than the SQL and harder to check against the byte-identity test. The issue scoped it that way too ("Keep set_channel_ancestors as raw parameterized SQL").

[rtibblesbot]

Squashed to 4 commits: the checksum lookup, annotation.py, importability_annotation.py + content/upgrade.py, the dead-code deletion.

Two coverage gaps I know of and left:

• The per-level rollup is asserted by outcome only — AnnotationTreeRecursion, which CI runs on both engines — not by SQL shape. A subquery that lost its GROUP BY parent_id correlation surfaces there as wrong counts rather than as a shape assertion.
• Nothing drives an annotation with more than SQLITE_MAX_VARIABLE_NUMBER checksums end to end. The inlining is asserted at the lookup instead, with 15000 checksums (kolibri/core/test/test_mixins.py).

[rtibbles]

Changes make sense, and test coverage is only expanded not modified - giving confidence in the refactor. Also seems to produce a very small performance improvement, so at worst it is a wash.