feat(b2b_learner_records): bounded date-range filters for partitioned backfills - #72
Merged
Merged
Conversation
OpenAPI ChangesShow/hide changesUnexpected changes? Ensure your branch is up-to-date with |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Fractional exclusive upper bounds currently exclude records that fall before the requested instant.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
What changed in this PR
Adds bounded synchronization windows and course catalog filtering to the learner-records API.
Changes:
- Adds exclusive
updated_beforefilters. - Adds contract, course-run, and start-date filters.
- Updates tests and generated OpenAPI documentation.
| File | Description |
|---|---|
tests/test_learner_records.py |
Tests new filter bindings. |
src/.../routers/organizations.py |
Exposes new query parameters. |
src/.../queries.py |
Builds range and catalog predicates. |
openapi/specs/b2b_learner_records.yaml |
Publishes the new parameters. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
blarghmatey
added a commit
that referenced
this pull request
Sep 25, 2026
_cursor_value() floored every bound to whole seconds. That is safe for an inclusive lower bound (>=): it can only widen the window backward, so a record gets re-sent, never skipped. Applied to the exclusive upper bound (<) it does the opposite: updated_before=06:15:00.500 floored to 06:15:00, so a row stored at 06:15:00.250, genuinely before the requested instant, failed the predicate and was silently dropped rather than deferred to the next window. _cursor_value(round_up=True) rounds a fractional exclusive bound up to the next whole second instead, so `<` matches through that second and the same row is re-sent by whichever window starts there. Applies to updated_before and courserun_starts_before via _window_predicates(). Found by copilot-pull-request-reviewer on #72. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EtEZ67aBRDvprCjGiDBeEs
… backfills updated_since alone only supports an open-ended "everything new" sync. Add updated_before so a partner can request [since, before) windows, letting a backfill be split into non-overlapping partitions across workers or replayed one window at a time. Applies to /learners and /enrollments. Also add contract_is_active, courserun_id, and courserun_starts_after/before to /courses. mv_b2b_contract_courserun carries no record_updated_on (dim_contract is a type-1 dimension with no change-tracking column), so the catalog can't yet support a real updated_since/updated_before cursor; courserun_starts_after/before partitions it by run start date instead, which is the catalog's natural grain. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EtEZ67aBRDvprCjGiDBeEs
…nscheduled runs courserun_start_on is nullable, and SQL >= / < are never true against NULL, so a self-paced run matches neither courserun_starts_after nor courserun_starts_before. A partitioned backfill built only from those windows would never see it. Document the gap on the endpoint and in queries.courses(), and extract the [since, before) predicate pair courses() and _shared_predicates() both built independently into one _window_predicates() helper, parameterized by column. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EtEZ67aBRDvprCjGiDBeEs
_cursor_value() floored every bound to whole seconds. That is safe for an inclusive lower bound (>=): it can only widen the window backward, so a record gets re-sent, never skipped. Applied to the exclusive upper bound (<) it does the opposite: updated_before=06:15:00.500 floored to 06:15:00, so a row stored at 06:15:00.250, genuinely before the requested instant, failed the predicate and was silently dropped rather than deferred to the next window. _cursor_value(round_up=True) rounds a fractional exclusive bound up to the next whole second instead, so `<` matches through that second and the same row is re-sent by whichever window starts there. Applies to updated_before and courserun_starts_before via _window_predicates(). Found by copilot-pull-request-reviewer on #72. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EtEZ67aBRDvprCjGiDBeEs
blarghmatey
force-pushed
the
b2b-learner-records-date-range-filters
branch
from
September 25, 2026 18:33
b6544ca to
f060381
Compare
This was referenced Sep 30, 2026
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
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.
What are the relevant tickets?
N/A
Description (What does it do?)
Add
updated_before(paired withupdated_since) so partners can pull bounded[since, before)windows instead of only an open-ended sync, for partitioned backfills. Addcontract_is_active,courserun_id, andcourserun_starts_after/courserun_starts_beforefilters to/courses.Implementation details
updated_sincealone only supports "everything new since X." Addingupdated_beforelets a partner request[since, before)windows, so a backfill can be split into non-overlapping partitions across workers or replayed one window at a time. Applies to/learnersand/enrollments./coursesgets a separate kind of range filter:mv_b2b_contract_courseruncarries norecord_updated_on, becausedim_contractis a type-1 dimension with no change-tracking column upstream. So the catalog can't support a realupdated_since/updated_beforecursor yet (that would need anol-data-platformchange).courserun_starts_after/courserun_starts_beforeis a workable substitute in the meantime, partitioning the catalog by course-run start date, which is its natural grain.courserun_start_onis nullable (self-paced/unscheduled runs), and SQL>=/<are never true against NULL, so those runs match neither bound and fall outside every partition window. Documented on the endpoint and inqueries.courses(): a partitioned sync needs one unfiltered pass to pick them up._window_predicates()centralizes the[since, before)predicate pair so_shared_predicates()(record_updated_on) andcourses()(courserun_start_on) share one implementation instead of two.How can this be tested?
ruff check,ruff format --check,mypy src,bin/generate-openapi-spec --check, and the fullpytestsuite (284 tests, including 2 new ones covering these filters) all pass locally. No manual/live StarRocks testing — the test suite stubs the DB pool.🤖 Generated with Claude Code
https://claude.ai/code/session_01EtEZ67aBRDvprCjGiDBeEs