Add a benchmark for channel import and content annotation - #15110
Merged
rtibbles merged 4 commits intoAug 4, 2026
Conversation
github-actions
Bot
added
DEV: backend
Contributor
Build Artifacts
Smoke test screenshot |
rtibblesbot
marked this pull request as ready for review
July 30, 2026 21:33
Times the content operations that the SQLAlchemy removal will touch, so later pull requests can show a measured before/after rather than an assurance. Five phases per channel against two real channels: fresh import, fresh import with the SQLite ATTACH path disabled, annotation, upgrade import and drive enumeration. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
VACUUM the destination database at the end of each content-table reset. Without it every iteration imports into a slightly more degraded database than the last, and the drift is systematic rather than random: annotation climbed monotonically from 2.81s to 3.17s over six consecutive sqlite iterations. Under the before/after protocol the branch is always captured second, so that drift lands entirely on the branch and failed a no-change self-comparison against the default 5% threshold. Stop with the schema version named when a channel resolves to no import class, rather than dying minutes later on a NoneType call. Fail the run when import_channel_from_local_db returns False. It returns rather than raises when check_and_delete_existing_channel finds nothing to do, so the phase would record a near-zero mean and --compare would read it against a real baseline as an enormous improvement and pass it. This harness exists to measure modified import code, so an import that quietly stops importing is the failure it most needs to refuse to report. Refuse a --compare path equal to --output, which would overwrite a baseline that costs hours to capture.
Fetch, describe and stage both channels before timing either of them. Studio no longer publishing a channel id, and a schema with no import class, are both knowable the moment a channel database is in hand; discovering either after the first channel has been benchmarked in full throws hours of work away, for the same reason main() already reads the baseline before the run. Log comparison warnings at WARNING level and outside the --quiet gate. Every one of them says the comparison measured something other than what was asked for, and the two that invalidate it outright — a cross-vendor comparison, and two captures taken at the same git revision — were being suppressed by a flag documented as suppressing results and the comparison table. Warn when the two captures ran under different Python versions. The report already records it, and a git checkout does not guarantee one interpreter. Reject --runs below 1, which otherwise downloads both channel databases and writes a report with no phases in it, exiting 0. Report an unmeasurable baseline against a measurable current run as an unbounded regression rather than as 0%, which would have passed it. Say in the docstring that a resumed partial download is cached unverified, and that the reset budget now carries a VACUUM on top of the cascade. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The postgres annotation phase measured 362s against under 3s for the same channel on sqlite. VACUUM was not the fix: it reclaims the dead tuples but leaves the planner's statistics describing the rows the reset deleted, so annotation planned its large UPDATEs against them. ANALYZE after the VACUUM takes that phase to 4.6s. Create KOLIBRI_HOME and its parents before handing it to kolibri.utils.conf, which only creates one level and otherwise raises "The parent of your KOLIBRI_HOME does not exist". The default's parent is ~/.cache, which is not present on every platform. Correct the docstring's reason for running from the repository root: cwd does not decide which kolibri is imported, but git_revision is read from it. Record the measured --runs 3 stability figures, and the signature of a contended capture — every phase moving by a similar amount in the same direction — which no number of runs can average away. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rtibblesbot
force-pushed
the
issue-15097-d36e60
branch
from
July 30, 2026 23:07
6b786cb to
17850ef
Compare
rtibbles
approved these changes
Aug 4, 2026
rtibbles
left a comment
rtibbles
left a comment
Member
There was a problem hiding this comment.
Seems to be working as intended, we'll run into any problems in the implementation phase, no doubt.
10 tasks
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an operator-run benchmark script timing channel import and content annotation against two published channels. Compares a capture against a baseline and exits non-zero when a phase regresses past a threshold.
Most of the file is what a trustworthy verdict costs: an untimed content-table reset that VACUUMs (and ANALYZEs on postgres) so iteration N is not measured against a database the previous iteration degraded, an exclusive lock on the benchmark home, and a refusal to report a capture whose import quietly did nothing.
References
integration_testing/scripts/viewset_serialization_benchmark.py.Reviewer guidance
content_operations_benchmark.py:252-261— the script assignsKOLIBRI_HOMErather than inheriting it, becausereset_content_tables()deletes every channel, node and local file in the home it points at. Check no path reaches that delete against a home the operator did not name.content_operations_benchmark.py:110-123— kolibri, Django and sqlalchemy imports are deferred into their callers soKOLIBRI_HOMEis set beforekolibri.utils.confsnapshots it. Is that constraint discoverable enough to survive a later module-scope import?NoLearningActivitiesChannelImport— the class comes frommin_schema_version(1on both), not the inferred version — so neither ATTACHesContentNode. Should the fixture set include a channel that does take the ATTACH path?Measurements
sqlite,
--runs 3(the default), both channels. x86_64 Linux (kernel 7.0.9), Python 3.14.6,KOLIBRI_HOMEunder~/.cacheon btrfs (not a tmpfs). Means over three samples:1ceff536…Khan Academy (English)NoLearningActivitiesChannelImportc9d7f950…Khan Academy (English - US curriculum)NoLearningActivitiesChannelImportfresh_import_no_attachseparates on SQLAlchemy statements (486 → 499, 405 → 416) but not on wall clock, per the third bullet above.drive_enumerationfalls below the 0.05 s--min-phase-sfloor: reported, not enforced.PostgreSQL,
--runs 1: exit 0, vendor recorded aspostgresql,fresh_import_no_attachskipped with the skip logged. Onc9d7f950…: fresh_import 20.50 s, upgrade_import 24.42 s, drive_enumeration 0.01 s.Verdict verification,
c9d7f950…at--runs 3: a temporarytime.sleep(0.5)inupdate_content_metadatamoved annotation +21.0% against an unpatched baseline, left every import phase within 1.7%, and gaveOVERALL VERDICT: FAIL, exit 1.AI usage
Used Claude Code to write the script and take the captures above, with the fixture and phase choices directed by me. Verified with repeated sqlite and PostgreSQL captures and with
prek.Deviations from the issue spec
c9d7f950…importing throughNoIncludedPresetsChannelImportwithContentNodeon the ATTACH path. It does not:initialize_import_managerpicks the class from the channel'smin_schema_version, which is1on both fixtures, so both import throughNoLearningActivitiesChannelImportand neither ATTACHesContentNode.fresh_import_no_attachis therefore near-flat on wall clock on both channels, and the separated SQLAlchemy statement counts are what show the phase took effect. No published channel available today changes this.@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
🟡 Waiting for feedback
Last updated: 2026-07-30 23:12 UTC