Repository navigation
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…h the intervals scan Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tervals Second n is playback from n - 1 to n, so an interval from 10 to 20 covers seconds 11 to 20, not 10 to 20. The old range counted one extra second for every interval that didn't start at 0. The Superset watched_video_segments dataset wraps this view in a CTE with an OR course filter, and ClickHouse can then cast end_video_position before the is_watched filter removes open intervals, failing with "Cannot convert NULL value to non-Nullable type". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bench.py renders the SQL Superset sends for each chart on the Course Dashboard engagement tabs and the Individual Learner tabs from the tutor-contrib-aspects assets, runs it against ClickHouse cold, warm and as a per-tab burst, and records duration, rows read and memory from system.query_log. compare_runs.py prints two runs side by side. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…_long `emission_time as emission_time_long` resolved to the `CAST(emission_time, 'DateTime') as emission_time` alias in the same select, so emission_time_long was stored as whole seconds. Video events within the same second then sort by verb and event id instead of by time when plays are paired with the event that ends them. A post_hook converts the column on existing installs. Rows already stored keep their truncated times. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dim_course_blocks was a view that joined the block dictionary to itself three times, for every course, on every chart load and in every engagement model. It is now a table rebuilt by a refreshable materialized view (every 5 minutes by default, ASPECTS_COURSE_STRUCTURE_REFRESH), with the same rows and columns plus: - section_course_order and subsection_course_order, which models looked up with more self-joins; - in_latest_publish, false for blocks missing from their course's latest publish (deleted). Only sections and subsections in the latest publish provide the section and subsection names, so a deleted one can no longer duplicate rows. dim_course_subsection_items, refreshed after it, holds the unit, problem and video counts per subsection that the engagement models recompute today, counting only the latest publish. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Superset video_watches dataset needs, per learner and video, the distinct seconds watched and the most views of any second. It gets them by grouping one row per watched second (about 30M for a 1,000-learner course) by every name column, which takes ~20 s and ~7 GiB per chart on 4 CPUs. fact_video_watches stores those two numbers. It sweeps each learner's interval ends (+1 where an interval starts covering seconds, -1 after its last second) instead of expanding seconds, and is refreshed right after fact_video_watch_intervals, recomputing only learner/video pairs with intervals in the lookback window. The post_hook back-fills history in learner chunks, like the intervals model. On the benchmark dataset it matches per-second counts exactly for all 49,371 pairs in the largest course. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ngagement models Correctness, checked against a generated dataset with known answers (25k learners, 14.8M events): - A section's total now covers every subsection in it. It only added up subsections where the learner had activity, so a learner who finished one subsection showed "All ... viewed" for the whole section (pages per section "all" was 7.6% high, videos 2.8x). - Problems get one row per learner and section. They got one per distinct subsection status, and the chart counts rows (problems per section "all" was 12x high, "at least one" +65%). - Units, problems and videos missing from the course's latest publish (deleted) no longer count. All three now come from one macro, engagement_by_section. Chart numbers match the known answers for pages, problems and videos at both levels. Speed: the models read small per-learner tables instead of grouping raw events on every load. fact_learner_page_visits and fact_learner_problem_attempts are insert-time materialized views with one row per learner and block, sorted by course and learner. Video engagement reads fact_video_watches. The items_per_subsection macro is replaced by dim_course_subsection_items. Output columns are unchanged. Pages and problems engagement charts are 4-9x faster for a 1,000-learner course. The CI navigation events point at subsections (as legacy courseware did); the Learning MFE sends the unit being left, which is what page engagement counts. So the pageview test now uses a small hand-built course (unit-test-seeds/engagement/), as do new section tests for problems and videos. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ed events Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The rows for subsections nobody watched came from an anti-join against the learners' rows, and a course filter on the view only reached the left side, so the right side read fact_video_watches for every course. A union and GROUP BY passes the filter to both, as the model did before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Superset filters every dashboard query by course. If a model change stops that filter from reaching the event-sized tables underneath, the results are the same but every course is read, which is the kind of regression that made dashboards time out. CI data is too small to show it in timings or rows read, so check_course_pruning.py checks the query plan instead: for every relation with a course_key column, filtered both ways Superset filters (a literal list and a subquery on dim_course_names), each read of a table sorted by course must use course_key in its primary key condition. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… in CI Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…arison SQL Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Tuple access uses tupleElement(), since sqlfmt rewrites t.1 as t .1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 10, 2026
This branch has not been deployed
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.
Builds on #177 (
fmo/video_segment_pairing_fix). Until #177 merges, this diff includes its commits; review fromb1bedd3.Fixes the engagement models so every chart number matches a known answer, and makes the slow video charts fast enough not to run the server out of memory (tutor-contrib-aspects#1335).
Results (largest course, ~1,050 learners, 14.8M events, ClickHouse 25.8 at 4 CPU / 16 GB, warm runs, course staff / admin):
The Videos tab's peak memory dropped from 13–14 GiB to about 5 GiB. Batched inserts take about the same time (2 min 47 s against #177's 3 min 12 s).
The video timings need the companion tutor-contrib-aspects PR (TODO: link). Without it the charts are still correct but stay at about 20 s.
Correctness fixes
fact_problem_engagementemitted one section row per distinct subsection status.emission_time_longwas stored as whole seconds because of an alias clash, so video events in the same second paired in the wrong order. A post_hook converts existing tables.What changed
dim_course_blocksand the newdim_course_subsection_itemsare refreshable MVs, rebuilt every 5 minutes (ASPECTS_COURSE_STRUCTURE_REFRESH).dim_course_blocksgainssection_course_order,subsection_course_orderandin_latest_publish.fact_video_watches: distinct seconds watched and most views of any second, per learner and video, refreshed right afterfact_video_watch_intervals.fact_learner_page_visitsandfact_learner_problem_attempts, filled at insert time.engagement_by_sectionmacro, and the course filter reaches every read.Testing
scripts/ci/check_course_pruning.pyfails if a course filter stops reaching an event-sized table's index.journeysdata (feat: Add ability to generate more realistic learner journey data xapi-db-load#276). Every bar matches at section and subsection level, and watched seconds are within 0.0002%. How to run it is inscripts/bench/README.md.Upgrading
dbt runupgrades the install, with no--full-refresh. It took 24 s from Video segment pairing fix #177 on the 14.8M-event dataset and 7.5 s from v8.0.0 with inserts running, with no failed inserts.Generated with Claude Opus 5.5