diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7ae468c785..6409bb3f9a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,7 +12,7 @@ jobs: # Label used to access the service container db: # Docker Hub image - image: postgres:12.22 + image: postgres:15 # Set health checks to wait until postgres has started options: >- --health-cmd pg_isready @@ -31,6 +31,20 @@ jobs: ports: - 6379:6379 + starrocks: + # All-in-one FE+BE image, for CI/local dev only — see + # learning_resources.lib.warehouse for what this backs. + image: starrocks/allin1-ubuntu:4.1.4 + options: >- + --health-cmd "bash -c 'echo > /dev/tcp/127.0.0.1/9030'" + --health-interval 10s + --health-timeout 5s + --health-retries 30 + --health-start-period 30s + ports: + - 9030:9030 + - 8030:8030 + steps: - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 with: @@ -78,6 +92,9 @@ jobs: MAILGUN_SENDER_DOMAIN: other.fake.site OPENSEARCH_INDEX: testindex INDEXING_API_USERNAME: mitodl + STARROCKS_HOST: localhost + STARROCKS_PORT: "9030" + STARROCKS_USER: root MITOL_COOKIE_DOMAIN: localhost MITOL_COOKIE_NAME: cookie_monster UNSUBSCRIBE_SECRET_KEY: fake_unsubscribe_secret diff --git a/RELEASE.rst b/RELEASE.rst index 5e2eab14f8..dfb4119220 100644 --- a/RELEASE.rst +++ b/RELEASE.rst @@ -1,6 +1,30 @@ Release Notes ============= +Version 0.77.11 +--------------- + +- fix: bump version for arithmix game (#3832) +- vector search completeness penalty (#3805) +- feat(cohort-1): StarRocks warehouse-pull ETL machinery (#3807) +- Update dependency llama-index-llms-openai to >=0.7.10,<0.8 (#3826) +- Update dependency posthog to v7 (#2765) +- Update Django to 5.2 (#3768) +- fix: bullet lists and mailto links in About section (#3802) +- Revert broken Renovate merges (#3812-#3816) (#3822) +- Update dependency tiktoken to >=0.13,<0.14 (#3816) +- Update dependency ruff to v0.16.2 (#3815) +- Update dependency llama-index-llms-openai to >=0.7.10,<0.8 (#3814) +- Update dependency litellm to v1.95.0 (#3813) +- Update dependency drf-spectacular to >=0.30,<0.31 (#3812) +- Update apache/apisix Docker tag to v3.17.0 (#2708) +- Drive React Query staleTime from the CDN TTL (#3804) +- get rid of duplicate course checks (#3803) +- feat: add a Failed filter tab to the B2B contract admin page (#3778) +- Clear redis cache on a schedule (#3789) +- adding fix for hiding offeror facets in hybrid search (#3786) +- chore(deps): mitol-django-observability 2026.3.11 -> 2026.8.19 (#3796) + Version 0.77.10 (Released August 24, 2026) --------------- diff --git a/channels/migrations/0017_channelgrouprole_index.py b/channels/migrations/0017_channelgrouprole_index.py new file mode 100644 index 0000000000..a5216f9693 --- /dev/null +++ b/channels/migrations/0017_channelgrouprole_index.py @@ -0,0 +1,17 @@ +# Generated by Django 5.2.16 on 2026-08-13 14:09 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("channels", "0016_alter_channel_featured_list"), + ] + + operations = [ + migrations.RenameIndex( + model_name="channelgrouprole", + new_name="channelgrouprole_ch_role_idx", + old_fields=("channel", "role"), + ), + ] diff --git a/channels/models.py b/channels/models.py index 7ae3121164..d44c159f9b 100644 --- a/channels/models.py +++ b/channels/models.py @@ -279,7 +279,11 @@ class ChannelGroupRole(TimestampedModel): class Meta: unique_together = (("channel", "group", "role"),) - index_together = (("channel", "role"),) + indexes = [ + models.Index( + fields=["channel", "role"], name="channelgrouprole_ch_role_idx" + ) + ] def __str__(self): return ( diff --git a/docker-compose.services.yml b/docker-compose.services.yml index ebebdf48cf..e00646fb64 100644 --- a/docker-compose.services.yml +++ b/docker-compose.services.yml @@ -39,6 +39,22 @@ services: retries: 10 ports: - "6379" + starrocks: + profiles: + - backend + # All-in-one FE+BE image, for local dev/test only — production StarRocks + # is a separately-operated cluster (see learning_resources.lib.warehouse). + image: starrocks/allin1-ubuntu:4.1.4 + ports: + - "9030:9030" # FE MySQL-protocol query port + - "8030:8030" # FE HTTP port + healthcheck: + test: ["CMD-SHELL", "bash -c 'echo > /dev/tcp/127.0.0.1/9030'"] + interval: 5s + timeout: 5s + retries: 60 + start_period: 30s + qdrant: image: qdrant/qdrant:latest ports: @@ -123,7 +139,7 @@ services: apigateway: profiles: - apisix - image: apache/apisix:3.13.0-debian # versions above this drop the local port on redirects + image: apache/apisix:3.17.0-debian@sha256:0e5377839f4ff5e322a5686ab6ce6797ba768008aca1bfc9b71149c3b326c4df # versions above this drop the local port on redirects <<: *default-extra-hosts environment: - CSRF_COOKIE_DOMAIN=${CSRF_COOKIE_DOMAIN:-.odl.local} diff --git a/fixtures/common.py b/fixtures/common.py index cc28765a80..9e1b1e0843 100644 --- a/fixtures/common.py +++ b/fixtures/common.py @@ -50,6 +50,14 @@ def warnings_as_errors(): module=".*(api_jwt|api_jws|rest_framework_jwt|astroid|bs4|celery|factory|botocore|posthog|pydantic).*", category=DeprecationWarning, ) + # django-safedelete 1.4.1 (latest) still calls LogEntry.objects.log_action(), + # deprecated in Django 5.2. Drop once upstream moves to log_actions(). + warnings.filterwarnings( + "ignore", + message=r"LogEntryManager\.log_action\(\) is deprecated.*", + module=r"safedelete\.admin", + category=DeprecationWarning, + ) yield finally: warnings.resetwarnings() diff --git a/frontends/main/Dockerfile.web b/frontends/main/Dockerfile.web index 2d38777c93..884d23e31b 100644 --- a/frontends/main/Dockerfile.web +++ b/frontends/main/Dockerfile.web @@ -20,7 +20,7 @@ # -e NEXT_PUBLIC_MITX_ONLINE_LEGACY_BASE_URL=http://mitxonline.odl.local:8065/ \ # -e NEXT_PUBLIC_MITX_ONLINE_BASE_URL=https://api.learn.mit.edu/mitxonline \ # -e NEXT_PUBLIC_MITX_ONLINE_CSRF_COOKIE_NAME=csrf_mitxonline \ -# -e NEXT_CACHE_S_MAXAGE_SECONDS=7200 \ +# -e NEXT_PUBLIC_CACHE_S_MAXAGE_SECONDS=7200 \ # -e NEXT_PUBLIC_MITOL_AXIOS_WITH_CREDENTIALS=true \ # mit-learn-nextjs:test diff --git a/frontends/main/package.json b/frontends/main/package.json index 53e8085b87..97211e5249 100644 --- a/frontends/main/package.json +++ b/frontends/main/package.json @@ -15,7 +15,7 @@ "@emotion/cache": "^11.13.1", "@emotion/styled": "^11.11.0", "@floating-ui/react": "^0.27.16", - "@mitodl/arithmix": "^0.2.4", + "@mitodl/arithmix": "^0.2.5", "@mitodl/course-search-utils": "^3.5.2", "@mitodl/hacksnack": "^0.1.2", "@mitodl/mitxonline-api-axios": "2026.8.18", diff --git a/frontends/main/src/app-pages/ChannelPage/ChannelSearch.test.tsx b/frontends/main/src/app-pages/ChannelPage/ChannelSearch.test.tsx index 796a61d6fc..0a5a56d9f2 100644 --- a/frontends/main/src/app-pages/ChannelPage/ChannelSearch.test.tsx +++ b/frontends/main/src/app-pages/ChannelPage/ChannelSearch.test.tsx @@ -7,7 +7,10 @@ import { user, } from "@/test-utils" import { setMockResponse, urls, factories, makeRequest } from "api/test-utils" -import type { LearningResourcesSearchResponse } from "api" +import type { + LearningResourcesSearchResponse, + PaginatedLearningResourceOfferorDetailList, +} from "api" import invariant from "tiny-invariant" import type { Channel } from "api/v0" import { ChannelTypeEnum } from "api/v0" @@ -16,9 +19,11 @@ import ChannelPage from "./ChannelPage" const setMockApiResponses = ({ search, channelPatch = {}, + offerors, }: { search?: Partial channelPatch?: Partial + offerors?: PaginatedLearningResourceOfferorDetailList }) => { const channel = factories.channels.channel(channelPatch) const urlParams = new URLSearchParams(channelPatch?.search_filter) @@ -58,7 +63,7 @@ const setMockApiResponses = ({ setMockResponse.get( urls.offerors.list(), - factories.learningResources.offerors({ count: 5 }), + offerors ?? factories.learningResources.offerors({ count: 5 }), ) setMockResponse.get(expect.stringContaining(urls.search.resources()), { @@ -201,6 +206,68 @@ describe("ChannelSearch", () => { expect(apiSearchParams.get("q")).toBe("python") expect(apiSearchParams.get("topic")).toBe("Economics") }) + + test("Hybrid search 'Offered By' facet only shows facets with 'display_facet' set to true", async () => { + const offerors = factories.learningResources.offerors({ count: 3 }) + offerors.results[0]!.display_facet = true + offerors.results[1]!.display_facet = false + offerors.results[2]!.display_facet = false + + const resources = factories.learningResources.resources({ + count: 3, + }).results + resources.forEach((resource, i) => { + resource.professional = true + resource.offered_by = { + code: offerors.results[i]!.code, + name: offerors.results[i]!.name, + channel_url: null, + } + }) + + const { channel } = setMockApiResponses({ + channelPatch: { channel_type: ChannelTypeEnum.Topic }, + offerors, + search: { + count: resources.length, + results: resources, + metadata: { + aggregations: { + offered_by: offerors.results.map((o, i) => ({ + key: o.code, + doc_count: 10 + i, + })), + }, + suggestions: [], + }, + }, + }) + + renderWithProviders(, { + url: `/c/${channel.channel_type}/${channel.name}?q=python&professional=true`, + }) + + const showFacetButton = await screen.findByRole("button", { + name: /Offered By/i, + }) + await user.click(showFacetButton) + + const offeror0 = await screen.findByRole("checkbox", { + name: new RegExp(`^${offerors.results[0]!.name}`), + }) + expect(offeror0).toBeVisible() + expect( + screen.queryByRole("checkbox", { + name: new RegExp(`^${offerors.results[1]!.name}`), + }), + ).not.toBeInTheDocument() + expect( + screen.queryByRole("checkbox", { + name: new RegExp(`^${offerors.results[2]!.name}`), + }), + ).not.toBeInTheDocument() + }) + test.each([ { searchFilter: "offered_by=ocw", diff --git a/frontends/main/src/app-pages/ContractAdminPage/ContractAdminPage.test.tsx b/frontends/main/src/app-pages/ContractAdminPage/ContractAdminPage.test.tsx index e24d4af220..c39bb7dad7 100644 --- a/frontends/main/src/app-pages/ContractAdminPage/ContractAdminPage.test.tsx +++ b/frontends/main/src/app-pages/ContractAdminPage/ContractAdminPage.test.tsx @@ -917,6 +917,433 @@ describe("ContractAdminPage", () => { expect(screen.getByText("pending@example.com")).toBeInTheDocument() }) + test("Failed tab filters to codes whose invite email failed", async () => { + mockedUseFeatureFlagsLoaded.mockReturnValue(true) + mockedUseFeatureFlagEnabled.mockReturnValue(true) + + const { org, contract } = makeOrgWithContract() + setMockResponse.get(managerOrgsUrl, { + count: 1, + next: null, + previous: null, + results: [org], + }) + setMockResponse.get( + managerContractDetailUrl(org.id, contract.id), + makeContractDetail(contract, { + total_codes: 2, + assigned_codes: 2, + redeemed_codes: 0, + unassigned_codes: 0, + }), + ) + + const deliveredCode = factories.contracts.contractCode({ + redemption_status: "assigned", + assigned_to: "delivered@example.com", + email_status: "delivered", + }) + const failedCode = factories.contracts.contractCode({ + redemption_status: "assigned", + assigned_to: "bounced@example.com", + email_status: "failed", + }) + + setMockResponse.get( + urls.contracts.managerContractCodes(org.id, contract.id, { + page: 1, + page_size: 25, + }), + factories.contracts.paginatedContractCodes([deliveredCode, failedCode]), + ) + setMockResponse.get( + urls.contracts.managerContractCodes(org.id, contract.id, { + page: 1, + page_size: 25, + status: "failed", + }), + factories.contracts.paginatedContractCodes([failedCode]), + ) + + renderWithProviders( + , + ) + + await screen.findByText("delivered@example.com") + + await user.click(screen.getByRole("tab", { name: "Failed" })) + + await waitFor(() => { + expect( + screen.queryByText("delivered@example.com"), + ).not.toBeInTheDocument() + }) + expect(screen.getByText("bounced@example.com")).toBeInTheDocument() + }) + + test("empty Failed tab explains that nothing failed", async () => { + mockedUseFeatureFlagsLoaded.mockReturnValue(true) + mockedUseFeatureFlagEnabled.mockReturnValue(true) + + const { org, contract } = makeOrgWithContract() + setMockResponse.get(managerOrgsUrl, { + count: 1, + next: null, + previous: null, + results: [org], + }) + setMockResponse.get( + managerContractDetailUrl(org.id, contract.id), + makeContractDetail(contract, { + total_codes: 1, + assigned_codes: 1, + redeemed_codes: 0, + unassigned_codes: 0, + }), + ) + + const deliveredCode = factories.contracts.contractCode({ + redemption_status: "assigned", + assigned_to: "delivered@example.com", + email_status: "delivered", + }) + + setMockResponse.get( + urls.contracts.managerContractCodes(org.id, contract.id, { + page: 1, + page_size: 25, + }), + factories.contracts.paginatedContractCodes([deliveredCode]), + ) + setMockResponse.get( + urls.contracts.managerContractCodes(org.id, contract.id, { + page: 1, + page_size: 25, + status: "failed", + }), + factories.contracts.paginatedContractCodes([]), + ) + + renderWithProviders( + , + ) + + await screen.findByText("delivered@example.com") + + await user.click(screen.getByRole("tab", { name: "Failed" })) + + // Filter-aware copy: a bare "No seat assignments found." would imply the + // contract has no seats at all, when in fact none of them failed. + const table = screen.getByRole("table", { name: "Seat assignments" }) + expect( + await within(table).findByRole("cell", { + name: "No failed invitations.", + }), + ).toBeInTheDocument() + // The live region mirrors the visible state, so it has to carry the same + // filter-aware copy — announcing the generic string would tell AT users the + // contract has no seat assignments at all. + expect(screen.getByRole("status")).toHaveTextContent( + "No failed invitations.", + ) + expect( + screen.queryByText("No seat assignments found."), + ).not.toBeInTheDocument() + }) + + test("empty state blames the search, not the filter, while a query is active", async () => { + mockedUseFeatureFlagsLoaded.mockReturnValue(true) + mockedUseFeatureFlagEnabled.mockReturnValue(true) + + // Drive the clock: the 300ms search debounce plus the request it triggers + // has to fit inside findBy's timeout, which is a race a loaded machine + // loses. Real timers made this pass alone and fail in a full-suite run. + jest.useFakeTimers() + try { + const timerUser = user.setup({ + advanceTimers: jest.advanceTimersByTime, + }) + + const { org, contract } = makeOrgWithContract() + setMockResponse.get(managerOrgsUrl, { + count: 1, + next: null, + previous: null, + results: [org], + }) + setMockResponse.get( + managerContractDetailUrl(org.id, contract.id), + makeContractDetail(contract, { + total_codes: 1, + assigned_codes: 1, + redeemed_codes: 0, + unassigned_codes: 0, + }), + ) + + const deliveredCode = factories.contracts.contractCode({ + redemption_status: "assigned", + assigned_to: "delivered@example.com", + email_status: "delivered", + }) + + setMockResponse.get( + urls.contracts.managerContractCodes(org.id, contract.id, { + page: 1, + page_size: 25, + }), + factories.contracts.paginatedContractCodes([deliveredCode]), + ) + setMockResponse.get( + urls.contracts.managerContractCodes(org.id, contract.id, { + page: 1, + page_size: 25, + search_term: "z", + }), + factories.contracts.paginatedContractCodes([]), + ) + + renderWithProviders( + , + ) + + await screen.findByText("delivered@example.com") + + // A single character, so the debounce collapses to exactly one request + // and no intermediate prefix needs its own mock. + await timerUser.type( + screen.getByPlaceholderText("Search by name or email..."), + "z", + ) + act(() => { + jest.advanceTimersByTime(300) + }) + + // Seats exist and none of them are excluded by a status filter here, so + // neither the per-filter copy nor "No seat assignments found." is true. + const table = screen.getByRole("table", { name: "Seat assignments" }) + expect( + await within(table).findByRole("cell", { + name: "No seat assignments match your search.", + }), + ).toBeInTheDocument() + expect( + screen.queryByText("No seat assignments found."), + ).not.toBeInTheDocument() + } finally { + jest.useRealTimers() + } + }) + + test("marks the table busy and dims stale rows while a filter change is in flight", async () => { + mockedUseFeatureFlagsLoaded.mockReturnValue(true) + mockedUseFeatureFlagEnabled.mockReturnValue(true) + + const { org, contract } = makeOrgWithContract() + setMockResponse.get(managerOrgsUrl, { + count: 1, + next: null, + previous: null, + results: [org], + }) + setMockResponse.get( + managerContractDetailUrl(org.id, contract.id), + makeContractDetail(contract, { + total_codes: 2, + assigned_codes: 2, + redeemed_codes: 0, + unassigned_codes: 0, + }), + ) + + const deliveredCode = factories.contracts.contractCode({ + redemption_status: "assigned", + assigned_to: "delivered@example.com", + email_status: "delivered", + }) + const failedCode = factories.contracts.contractCode({ + redemption_status: "assigned", + assigned_to: "bounced@example.com", + email_status: "failed", + }) + + setMockResponse.get( + urls.contracts.managerContractCodes(org.id, contract.id, { + page: 1, + page_size: 25, + }), + factories.contracts.paginatedContractCodes([deliveredCode, failedCode]), + ) + const failedCodes = + Promise.withResolvers< + ReturnType + >() + setMockResponse.get( + urls.contracts.managerContractCodes(org.id, contract.id, { + page: 1, + page_size: 25, + status: "failed", + }), + failedCodes.promise, + ) + + renderWithProviders( + , + ) + + await screen.findByText("delivered@example.com") + + await user.click(screen.getByRole("tab", { name: "Failed" })) + + // keepPreviousData keeps the All tab's rows on screen until the filtered + // response lands, so the table has to say it is updating — otherwise a + // delivered row sits under the Failed tab looking like a result. + const table = screen.getByRole("table", { name: "Seat assignments" }) + expect(table).toHaveAttribute("aria-busy", "true") + expect(screen.getByText("delivered@example.com")).toBeInTheDocument() + expect(screen.getByRole("status")).toHaveTextContent( + "Loading seat assignments", + ) + + await act(async () => { + failedCodes.resolve( + factories.contracts.paginatedContractCodes([failedCode]), + ) + }) + + await waitFor(() => { + expect(table).toHaveAttribute("aria-busy", "false") + }) + expect(screen.queryByText("delivered@example.com")).not.toBeInTheDocument() + // Switching tabs replaces the whole result set, so the new size is + // announced — the table's own region often lands back on the same + // "page 1 of 1" text it started with and would say nothing. + await screen.findByText("1 result") + }) + + // isPlaceholderData (keepPreviousData) only reflects rows carried over from + // a different query key — it's false the moment React Query has a cache + // entry for the current key, even if a mutation invalidated it and a + // refetch is running in the background. isFetching is what's true then, so + // the busy/dimmed state has to account for it too, or a revisited tab looks + // "done" while it's still silently revalidating. + test("revisiting a tab invalidated by a row mutation stays busy/dimmed until the refetch resolves", async () => { + mockedUseFeatureFlagsLoaded.mockReturnValue(true) + mockedUseFeatureFlagEnabled.mockReturnValue(true) + + const { org, contract } = makeOrgWithContract() + setMockResponse.get(managerOrgsUrl, { + count: 1, + next: null, + previous: null, + results: [org], + }) + + let contractDetailCalls = 0 + setMockResponse.get(managerContractDetailUrl(org.id, contract.id), () => { + contractDetailCalls += 1 + return contractDetailCalls === 1 + ? makeContractDetail(contract, { + total_codes: 2, + assigned_codes: 2, + redeemed_codes: 0, + unassigned_codes: 0, + }) + : makeContractDetail(contract, { + total_codes: 2, + assigned_codes: 1, + redeemed_codes: 0, + unassigned_codes: 1, + }) + }) + + const assignedCode = factories.contracts.contractCode({ + redemption_status: "assigned", + assigned_to: "pending@example.com", + }) + + // The All tab's own query key is fetched twice in this test: once on the + // initial visit, and once when it's revisited after the Pending-tab + // mutation invalidates every filter for this contract. The second fetch + // is held open so the busy/dimmed state can be asserted before resolving. + let allCodesCalls = 0 + const allCodesRefetch = + Promise.withResolvers< + ReturnType + >() + setMockResponse.get( + urls.contracts.managerContractCodes(org.id, contract.id, { + page: 1, + page_size: 25, + }), + () => { + allCodesCalls += 1 + return allCodesCalls === 1 + ? factories.contracts.paginatedContractCodes([assignedCode]) + : allCodesRefetch.promise + }, + ) + setMockResponse.get( + urls.contracts.managerContractCodes(org.id, contract.id, { + page: 1, + page_size: 25, + status: "assigned", + }), + factories.contracts.paginatedContractCodes([assignedCode]), + ) + setMockResponse.delete( + urls.contracts.managerContractCodeRevoke( + org.id, + contract.id, + assignedCode.code, + ), + assignedCode, + ) + + renderWithProviders( + , + ) + + // Load and cache the All tab. + await screen.findByText("pending@example.com") + + // Visit and cache the Pending tab too. + await user.click(screen.getByRole("tab", { name: "Pending" })) + await screen.findByText("pending@example.com") + + // Release the seat from the Pending tab — this invalidates every cached + // filter/page for the contract, including the All tab visited earlier. + await user.click(screen.getByRole("button", { name: /more actions/i })) + await user.click(screen.getByRole("menuitem", { name: "Release seat" })) + await user.click(screen.getByRole("button", { name: "Release seat" })) + + await waitFor(() => { + expect( + screen.getByRole("group", { name: "Unassigned" }), + ).toHaveTextContent("1") + }) + + // Revisit the All tab: React Query already has a (now-invalidated) cache + // entry for this exact key, so the cached row shows immediately — + // isLoading and isPlaceholderData are both false — while the second, + // deferred fetch above refetches it in the background. + await user.click(screen.getByRole("tab", { name: "All" })) + + const table = screen.getByRole("table", { name: "Seat assignments" }) + expect(table).toHaveAttribute("aria-busy", "true") + expect(screen.getByRole("status")).toHaveTextContent( + "Loading seat assignments", + ) + + await act(async () => { + allCodesRefetch.resolve(factories.contracts.paginatedContractCodes([])) + }) + + await waitFor(() => { + expect(table).toHaveAttribute("aria-busy", "false") + }) + }) + describe("status pill", () => { const setupCodeRow = ( code: ReturnType, @@ -948,10 +1375,10 @@ describe("ContractAdminPage", () => { } // "Pending"/"Redeemed" also appear as a header stat label and a filter - // tab, so an unscoped getByText for those two would be ambiguous. Scope - // to the row via its ARIA role — an accessibility-meaningful boundary - // set by this page's own markup, not a third-party implementation detail - // like a MUI-generated class name. + // tab, and "Failed" as a filter tab, so an unscoped getByText for any of + // those would be ambiguous. Scope to the row via its ARIA role — an + // accessibility-meaningful boundary set by this page's own markup, not a + // third-party implementation detail like a MUI-generated class name. const getRow = (assignedTo: string) => { const row = screen.getByText(assignedTo).closest('[role="row"]') if (!row) { @@ -1069,7 +1496,9 @@ describe("ContractAdminPage", () => { // explanation is a description, not baked into the name — MUI's // describeChild renders it as a native `title` attribute while closed, // and swaps it for a live aria-describedby while the tooltip is open. - expect(screen.getByText("Failed")).toBeInTheDocument() + expect( + within(getRow("bounced@example.com")).getByText("Failed"), + ).toBeInTheDocument() const pill = screen.getByTitle(explanation) // In the tab order, so keyboard users can reach it. (MUI only opens the // tooltip on *keyboard* focus via the CSS :focus-visible pseudo-class, diff --git a/frontends/main/src/app-pages/ContractAdminPage/ContractAdminPage.tsx b/frontends/main/src/app-pages/ContractAdminPage/ContractAdminPage.tsx index 60e7e209cc..b049ea102e 100644 --- a/frontends/main/src/app-pages/ContractAdminPage/ContractAdminPage.tsx +++ b/frontends/main/src/app-pages/ContractAdminPage/ContractAdminPage.tsx @@ -48,6 +48,7 @@ import { managerOrganizationQueries, type ManagerEnrollmentCode, } from "api/mitxonline-hooks/organizations" +import type { B2bManagerOrganizationsContractsCodesListStatusEnum } from "@mitodl/mitxonline-api-axios/v2" import type { AxiosError } from "axios" import { matchOrganizationBySlug } from "@/common/utils" import { ForbiddenError } from "@/common/errors" @@ -294,7 +295,66 @@ const ActionCell = styled.div(({ theme }) => ({ }, })) -type StatusFilter = "all" | "pending" | "redeemed" +type StatusFilter = "all" | "pending" | "redeemed" | "failed" + +/** + * Maps a tab to the API's `status` param. The UI's "Pending" is the API's + * "assigned", and `assigned` means "unredeemed" regardless of email status — + * so it is a *superset* of `failed`, and a bounced code appears under both + * tabs. That overlap is intentional: the backend can't express "assigned AND + * NOT failed", and filtering it out client-side would desync the server's + * `count` from the rendered rows (wrong page count, short pages). The row's + * status pill is what distinguishes the two. + */ +const STATUS_FILTER_PARAM: Record< + StatusFilter, + B2bManagerOrganizationsContractsCodesListStatusEnum | undefined +> = { + all: undefined, + pending: "assigned", + redeemed: "redeemed", + failed: "failed", +} + +/** + * Per-filter empty copy. A bare "No seat assignments found." on the Failed tab + * would be misleading — seats do exist, none of them failed. + */ +const EMPTY_TABLE_MESSAGE: Record = { + all: "No seat assignments found.", + pending: "No pending seat assignments.", + redeemed: "No redeemed seat assignments.", + failed: "No failed invitations.", +} + +/** + * A query that matches nothing is its own state, distinct from every entry + * above: seats exist, and some may even have the active status — they just + * don't match what was typed. Both the per-filter copy and the generic "No + * seat assignments found." misstate that, so the search case gets its own + * string. + */ +const NO_SEARCH_RESULTS_MESSAGE = "No seat assignments match your search." + +/** + * `$stale` is true whenever the rows on screen might not match the server: + * either `keepPreviousData` is carrying over the previous filter's or page's + * rows while the next request is in flight (which avoids a layout jump when + * paginating, but would otherwise leave, say, delivered rows sitting under + * the Failed tab with nothing to say they are stale), or the current + * filter's own cached rows are being silently revalidated in the background + * — e.g. a row mutation like resending an invite invalidates every tab/page/ + * search for the contract, not just the active one. Fade the rows while + * either is true. + * + * The delay means a fast response never flashes the dimming, and dropping the + * transition on the fresh state restores full opacity immediately rather than + * fading back in. + */ +const TableBody = styled.div<{ $stale: boolean }>(({ $stale }) => ({ + opacity: $stale ? 0.5 : 1, + transition: $stale ? "opacity 150ms ease 150ms" : "none", +})) const COLUMN_FLEX = { assignedTo: 2, @@ -345,7 +405,7 @@ const ContractAdminPageInternal: React.FC = ({ const [searchQuery, setSearchQuery] = useState("") const [debouncedSearchQuery, setDebouncedSearchQuery] = useState("") const [page, setPage] = useState(1) - const [searchAnnouncement, setSearchAnnouncement] = useState("") + const [resultsAnnouncement, setResultsAnnouncement] = useState("") const [rowActionResult, setRowActionResult] = useState<{ message: string severity: "success" | "error" @@ -411,6 +471,8 @@ const ContractAdminPageInternal: React.FC = ({ const { data: codes, isLoading: isLoadingCodes, + isPlaceholderData: isCodesStale, + isFetching: isCodesFetching, isError: isCodesError, error: codesError, } = useQuery({ @@ -420,33 +482,51 @@ const ContractAdminPageInternal: React.FC = ({ page, page_size: CODES_PAGE_SIZE, search_term: debouncedSearchQuery || undefined, - status: - statusFilter === "redeemed" - ? "redeemed" - : statusFilter === "pending" - ? "assigned" - : undefined, + status: STATUS_FILTER_PARAM[statusFilter], }), enabled: !!org && !!contract, placeholderData: keepPreviousData, }) - // Announce the result count after the query settles following a search change. - // Using a ref to track the last announced query so we only fire once per change, - // not on every re-render while loading. - const announcedQueryRef = useRef("") + // `isCodesStale` (isPlaceholderData) only covers rows carried over from a + // different query key via `keepPreviousData` — it goes false as soon as + // React Query has a cache entry for the current key, even if that entry was + // invalidated (e.g. by a row mutation like resending an invite, which + // invalidates every tab/page/search for the contract at once) and is being + // silently refetched in the background. `isCodesFetching` catches that case. + const isCodesRevalidating = isCodesStale || isCodesFetching + const isCodesBusy = isLoadingCodes || isCodesRevalidating + + // Announce the result count once the query settles after a change to the + // search term or the status filter — both replace the whole result set, and + // switching tabs is otherwise silent, since the table's own status region + // often lands on the same "page 1 of 1" text it started with. + // + // The ref keys on both inputs so we fire once per change rather than on every + // re-render while loading, and `isCodesBusy` holds the announcement until the + // data actually belongs to the new filter and any revalidation of it has + // resolved — otherwise a stale count could be announced immediately (from + // `keepPreviousData` on a key change, or from revisiting a cached-but- + // invalidated tab) and never corrected once the real data arrives, since the + // ref would already mark this filter/search combo as announced. + const announcedResultsRef = useRef(null) useEffect(() => { - if (isLoadingCodes || announcedQueryRef.current === debouncedSearchQuery) - return - announcedQueryRef.current = debouncedSearchQuery + if (isCodesBusy) return + const settled = `${statusFilter}:${debouncedSearchQuery}` + if (announcedResultsRef.current === settled) return + const isFirstLoad = announcedResultsRef.current === null + announcedResultsRef.current = settled + // The initial load is not a change the user made; a count there would talk + // over the page announcing itself. + if (isFirstLoad) return const count = codes?.count ?? 0 - setSearchAnnouncement("") + setResultsAnnouncement("") const id = setTimeout( - () => setSearchAnnouncement(`${count} result${count !== 1 ? "s" : ""}`), + () => setResultsAnnouncement(`${count} result${count !== 1 ? "s" : ""}`), 0, ) return () => clearTimeout(id) - }, [isLoadingCodes, debouncedSearchQuery, codes?.count]) + }, [isCodesBusy, statusFilter, debouncedSearchQuery, codes?.count]) if (isLoadingOrgs) { return ( @@ -502,6 +582,13 @@ const ContractAdminPageInternal: React.FC = ({ const pageResults = codes?.results ?? [] + // With a search term active this is a no-matches state, not a no-such-status + // state — otherwise the Failed tab would claim "No failed invitations." when + // failed ones exist but don't match the query. + const emptyTableMessage = debouncedSearchQuery + ? NO_SEARCH_RESULTS_MESSAGE + : EMPTY_TABLE_MESSAGE[statusFilter] + const totalCount = codes?.count ?? 0 const totalPages = Math.ceil(totalCount / CODES_PAGE_SIZE) @@ -701,6 +788,7 @@ const ContractAdminPageInternal: React.FC = ({ + = ({ - {searchAnnouncement} + {resultsAnnouncement} -
+ {/* Mirrors the table's visible state for AT. Deliberately a sibling + of the table rather than a child: AT may defer live-region + updates while an ancestor is aria-busy, which would swallow the + "Loading" announcement until the load it describes is already + over. Staying out also leaves the rowgroup holding only rows. + + The empty case has to use the same filter-aware copy as the cell + does — announcing the generic "no seat assignments" on the + Failed tab makes exactly the claim that copy exists to avoid. */} + + {isCodesBusy + ? "Loading seat assignments" + : pageResults.length === 0 + ? emptyTableMessage + : `Showing page ${page} of ${totalPages}`} + +
= ({
-
- - {isLoadingCodes - ? "Loading seat assignments" - : pageResults.length === 0 - ? "No seat assignments found" - : `Showing page ${page} of ${totalPages}`} - + {isLoadingCodes ? ( <> {[1, 2, 3].map((i) => ( @@ -798,7 +904,7 @@ const ContractAdminPageInternal: React.FC = ({ aria-colspan={7} style={{ flex: 1 }} > - No seat assignments found. + {emptyTableMessage} ) : ( @@ -875,14 +981,18 @@ const ContractAdminPageInternal: React.FC = ({ )) )} -
+
- + {/* Only page position. With a filter or search active there are + no rows for a reason the body already states, and a footnote + reading "No assignments" under "No failed invitations." would + contradict it. */} + {totalCount > 0 && ( + + )} {totalPages > 1 && ( { expect(raw.innerHTML).toBe(htmlString) } +test("About section keeps bullet markers on a top-level list", async () => { + const about = "
  • First
  • Second
" + const noun = faker.helpers.arrayElement(["Course", "Program"] as const) + renderWithProviders() + + const section = await screen.findByRole("region", { + name: `About this ${noun}`, + }) + const list = within(section).getByRole("list") + expect(getComputedStyle(list).listStyleType).not.toBe("none") +}) + test("About section has expected content", async () => { const about = `

${faker.lorem.paragraph()}

` const noun = faker.helpers.arrayElement(["Course", "Program"] as const) diff --git a/frontends/main/src/app-pages/ProductPages/RawHTML.tsx b/frontends/main/src/app-pages/ProductPages/RawHTML.tsx index 2257210c04..b301f1b412 100644 --- a/frontends/main/src/app-pages/ProductPages/RawHTML.tsx +++ b/frontends/main/src/app-pages/ProductPages/RawHTML.tsx @@ -12,10 +12,8 @@ const RawHTML = styled(UnstyledRawHTML)(({ theme }) => ({ marginBottom: "0", }, "& > ul": { - listStyleType: "none", marginTop: "16px", marginBottom: 0, - padding: 0, }, [theme.breakpoints.down("md")]: { ...theme.typography.body2, diff --git a/frontends/main/src/app/getQueryClient.test.tsx b/frontends/main/src/app/getQueryClient.test.tsx index aa5dc26d03..de3e1a55f9 100644 --- a/frontends/main/src/app/getQueryClient.test.tsx +++ b/frontends/main/src/app/getQueryClient.test.tsx @@ -51,6 +51,28 @@ test.each(cartesianProduct(RETRY_CASES, QUERY_CLIENTS))( }, ) +describe("browser client staleTime", () => { + const S_MAXAGE_SECONDS = 7200 + const originalEnv = process.env + + beforeEach(() => { + process.env = { ...originalEnv } + }) + afterEach(() => { + process.env = originalEnv + }) + + // Hydrated queries must not be stale on arrival: HTML served from the CDN can + // be up to s-maxage old, and a shorter staleTime refetches on hydration. + test("is the CDN TTL, in milliseconds", () => { + process.env.NEXT_PUBLIC_CACHE_S_MAXAGE_SECONDS = String(S_MAXAGE_SECONDS) + const queryClient = makeBrowserQueryClient() + expect(queryClient.getDefaultOptions().queries?.staleTime).toBe( + S_MAXAGE_SECONDS * 1000, + ) + }) +}) + test("server client retryDelay uses exponential backoff clamped to 1000ms", () => { const queryClient = getServerQueryClient() const retryDelay = queryClient.getDefaultOptions().queries?.retryDelay diff --git a/frontends/main/src/app/getQueryClient.ts b/frontends/main/src/app/getQueryClient.ts index fd9a5e7aae..15dcf3d9f0 100644 --- a/frontends/main/src/app/getQueryClient.ts +++ b/frontends/main/src/app/getQueryClient.ts @@ -5,6 +5,7 @@ import type { AxiosError } from "axios" import { cache } from "react" import { notFound } from "next/navigation" import { bootstrapApiClients } from "@/bootstrap/api" +import { getCacheSMaxageSeconds } from "@/common/config" /** Max retries after first failure */ const MAX_RETRIES = 3 @@ -148,19 +149,12 @@ const makeBrowserQueryClient = ( defaultOptions: { queries: { /** - * Public API content is server-rendered to the base page and cached by the CDN. - * Keep staleTime >= CDN TTL so hydrated queries are not immediately refetched. - * The CDN TTL is specified by the s-max-age value in the Cache-Control header - * in next.config.js. - * - * Most content is stable for ~24 hours (ETL cadence), but if staleTime is shorter - * than the CDN TTL, React Query will refetch on hydration once the cached HTML - * is older than staleTime. - * - * This can cause visible content flicker for unstable endpoints (e.g. the - * featured learning resource list, which is intentionally randomized). + * Public API content is server-rendered to the base page and cached by the + * CDN, so staleTime must be >= the CDN TTL: once the cached HTML is older + * than staleTime, React Query refetches on hydration. A long staleTime is + * fine because most content is stable for ~24 hours (ETL cadence). */ - staleTime: 30 * 60 * 1000, + staleTime: getCacheSMaxageSeconds() * 1000, /** * Throw runtime errors instead of marking query as errored. diff --git a/frontends/main/src/common/config.test.ts b/frontends/main/src/common/config.test.ts new file mode 100644 index 0000000000..d2cc5420ed --- /dev/null +++ b/frontends/main/src/common/config.test.ts @@ -0,0 +1,34 @@ +import { getCacheSMaxageSeconds } from "./config" + +describe("getCacheSMaxageSeconds", () => { + const originalEnv = process.env + + beforeEach(() => { + process.env = { ...originalEnv } + }) + afterEach(() => { + process.env = originalEnv + }) + + test("reads the configured TTL", () => { + process.env.NEXT_PUBLIC_CACHE_S_MAXAGE_SECONDS = "7200" + expect(getCacheSMaxageSeconds()).toBe(7200) + }) + + test("falls back when unset, as in local dev and the CI stack", () => { + expect(getCacheSMaxageSeconds()).toBe(1800) + }) + + // ol-infrastructure sends "" when the Pulumi config key is absent, so the + // fallback must not treat it as the number 0. + test("falls back when empty", () => { + process.env.NEXT_PUBLIC_CACHE_S_MAXAGE_SECONDS = "" + expect(getCacheSMaxageSeconds()).toBe(1800) + }) + + // Distinct from empty, which Number() also turns into 0. + test("honors an explicit 0", () => { + process.env.NEXT_PUBLIC_CACHE_S_MAXAGE_SECONDS = "0" + expect(getCacheSMaxageSeconds()).toBe(0) + }) +}) diff --git a/frontends/main/src/common/config.ts b/frontends/main/src/common/config.ts index 15a717655c..1a5f42cc70 100644 --- a/frontends/main/src/common/config.ts +++ b/frontends/main/src/common/config.ts @@ -4,3 +4,28 @@ export const getRecaptchaSiteKey = (): string | undefined => export const getStayUpdatedHubspotFormId = (): string => (env("NEXT_PUBLIC_STAY_UPDATED_HUBSPOT_FORM_ID") ?? "").trim() + +/** CDN TTL used when NEXT_PUBLIC_CACHE_S_MAXAGE_SECONDS is unset: local dev and CI. */ +const DEFAULT_S_MAXAGE_SECONDS = 1800 + +/** + * The CDN TTL for HTML page responses, in seconds. + * + * Two readers must agree on this value: proxy.ts sends it as the Cache-Control + * s-maxage, and getQueryClient.ts uses it as the browser React Query staleTime, + * so that CDN-cached HTML does not hydrate into queries that are already stale + * and refetch immediately — which shows up as flicker on unstable endpoints + * (e.g. the intentionally randomized featured learning resources). + * + * Matching them is a floor, not a guarantee: the same header sends + * stale-while-revalidate=86400, so an edge can serve HTML older than s-maxage. + * Refetching on hydration is the right outcome there. + */ +export const getCacheSMaxageSeconds = (): number => { + // Same /^\d+$/ as validateEnv.js: rejects unset and the "" ol-infrastructure + // sends when the Pulumi config key is absent, while keeping an explicit "0". + const configured = env("NEXT_PUBLIC_CACHE_S_MAXAGE_SECONDS") ?? "" + return /^\d+$/.test(configured) + ? Number(configured) + : DEFAULT_S_MAXAGE_SECONDS +} diff --git a/frontends/main/src/components/UnstyledRawHTML/UnstyledRawHTML.test.tsx b/frontends/main/src/components/UnstyledRawHTML/UnstyledRawHTML.test.tsx new file mode 100644 index 0000000000..3009b0f87e --- /dev/null +++ b/frontends/main/src/components/UnstyledRawHTML/UnstyledRawHTML.test.tsx @@ -0,0 +1,15 @@ +import React from "react" +import { render, screen } from "@testing-library/react" +import UnstyledRawHTML from "./UnstyledRawHTML" + +test("allows mailto: links to keep their href", () => { + render() + const link = screen.getByRole("link", { name: "Email us" }) + expect(link).toHaveAttribute("href", "mailto:learn@mit.edu") +}) + +test("strips disallowed URI schemes like javascript:", () => { + render() + const link = screen.getByText("Click") + expect(link).not.toHaveAttribute("href") +}) diff --git a/frontends/main/src/components/UnstyledRawHTML/UnstyledRawHTML.tsx b/frontends/main/src/components/UnstyledRawHTML/UnstyledRawHTML.tsx index db33e6c13c..cc9ba815cb 100644 --- a/frontends/main/src/components/UnstyledRawHTML/UnstyledRawHTML.tsx +++ b/frontends/main/src/components/UnstyledRawHTML/UnstyledRawHTML.tsx @@ -22,7 +22,7 @@ const SANITIZE_CONFIG: Config = { "referrerpolicy", ], ADD_URI_SAFE_ATTR: ["src"], - ALLOWED_URI_REGEXP: /^(?:(?:https):)|^(?:data:image\/)/i, + ALLOWED_URI_REGEXP: /^(?:(?:https|mailto):)|^(?:data:image\/)/i, } const UnstyledRawHTML: React.FC = memo( diff --git a/frontends/main/src/page-components/SearchDisplay/HybridSearchDisplay.tsx b/frontends/main/src/page-components/SearchDisplay/HybridSearchDisplay.tsx index 075f1b705e..be78aecb0e 100644 --- a/frontends/main/src/page-components/SearchDisplay/HybridSearchDisplay.tsx +++ b/frontends/main/src/page-components/SearchDisplay/HybridSearchDisplay.tsx @@ -1,5 +1,8 @@ import React, { useMemo } from "react" -import { learningResourceQueries } from "api/hooks/learningResources" +import { + learningResourceQueries, + useOfferorsList, +} from "api/hooks/learningResources" import type { LearningResource } from "api" import type { LearningResourcesVectorSearchResponse } from "api/v0" import getSearchParams from "./getSearchParams" @@ -95,6 +98,7 @@ const getVectorClientAggregations = ( allResults: LearningResource[], params: ReturnType, aggregationNames: string[], + displayOfferorCodes: string[], ) => { return Object.fromEntries( aggregationNames.map((name) => { @@ -104,6 +108,10 @@ const getVectorClientAggregations = ( const counts = new Map() for (const resource of resultsForFacet) { for (const value of getResourceFacetValues(resource, name)) { + // only show offerors with display_facet set, matching SearchDisplay + if (name === "offered_by" && !displayOfferorCodes.includes(value)) { + continue + } counts.set(value, (counts.get(value) ?? 0) + 1) } } @@ -132,6 +140,15 @@ const HybridSearchDisplay: React.FC = ({ typeof props.requestParams.q === "string" && props.requestParams.q.trim() !== "" + const offerorsQuery = useOfferorsList() + const displayOfferorCodes = useMemo( + () => + (offerorsQuery.data?.results ?? []) + .filter((offeror) => offeror.code && offeror.display_facet) + .map((offeror) => offeror.code), + [offerorsQuery.data?.results], + ) + const getQueryOptions = useMemo( () => (params: ReturnType) => { const hasSearchTerm = @@ -185,12 +202,13 @@ const HybridSearchDisplay: React.FC = ({ allResults, params, params.aggregations, + displayOfferorCodes, ) : vectorData.metadata.aggregations, }, } }, - [], + [displayOfferorCodes], ) return ( diff --git a/frontends/main/src/proxy.test.ts b/frontends/main/src/proxy.test.ts index 598c1b77f7..4ff257e042 100644 --- a/frontends/main/src/proxy.test.ts +++ b/frontends/main/src/proxy.test.ts @@ -122,10 +122,26 @@ describe("proxy", () => { const makeRequest = (pathname: string) => new NextRequest(new URL(pathname, "https://learn.mit.edu")) + const S_MAXAGE_SECONDS = 7200 + const originalEnv = process.env + + beforeEach(() => { + process.env = { + ...originalEnv, + NEXT_PUBLIC_CACHE_S_MAXAGE_SECONDS: String(S_MAXAGE_SECONDS), + } + }) + afterEach(() => { + process.env = originalEnv + }) + test("tags generic page routes with Cache-Control and html-pages Surrogate-Key", () => { const response = proxy(makeRequest("/about")) expect(response.headers.get("Surrogate-Key")).toBe("html-pages") - expect(response.headers.get("Cache-Control")).toContain("s-maxage=") + // s-maxage is in seconds, and comes from the configured CDN TTL. + expect(response.headers.get("Cache-Control")).toBe( + `s-maxage=${S_MAXAGE_SECONDS}, stale-if-error=86400, stale-while-revalidate=86400`, + ) }) test("appends per-item surrogate key for MITxOnline course pages", () => { @@ -133,7 +149,6 @@ describe("proxy", () => { expect(response.headers.get("Surrogate-Key")).toBe( "html-pages mitxonline:course:course-v1:MITxT+5.601x", ) - expect(response.headers.get("Cache-Control")).toContain("s-maxage=") }) test("appends per-item surrogate key for MITxOnline program pages (/programs/)", () => { diff --git a/frontends/main/src/proxy.ts b/frontends/main/src/proxy.ts index f03ca6e16e..b857cfe4b0 100644 --- a/frontends/main/src/proxy.ts +++ b/frontends/main/src/proxy.ts @@ -1,4 +1,5 @@ import { type NextRequest, NextResponse } from "next/server" +import { getCacheSMaxageSeconds } from "@/common/config" /** * Matches paths ending in a known static-asset extension (e.g. .js, .css, @@ -105,20 +106,19 @@ export function mitxonlineSurrogateKey(pathname: string): string | null { /** * Next.js proxy (formerly "middleware"): sets the Cache-Control header at - * request time so that NEXT_CACHE_S_MAXAGE_SECONDS is read from the Kubernetes - * environment rather than baked into the Docker image at build time. + * request time so the CDN TTL is read from the Kubernetes environment rather + * than baked into the Docker image at build time. * * next.config.js `headers()` runs at build time and cannot read env vars that - * vary across environments (QA vs production). Proxy runs on the Node.js - * runtime on every request, so process.env is always the live value. + * vary across environments (QA vs production). Proxy always runs on the + * Node.js runtime, on every request, so the env read is always the live value. */ export function proxy(request: NextRequest) { if (!isPageRoute(request.nextUrl.pathname)) { return NextResponse.next() } - const sMaxage = process.env.NEXT_CACHE_S_MAXAGE_SECONDS || "1800" - const cacheControl = `s-maxage=${sMaxage}, stale-if-error=86400, stale-while-revalidate=86400` + const cacheControl = `s-maxage=${getCacheSMaxageSeconds()}, stale-if-error=86400, stale-while-revalidate=86400` const response = NextResponse.next() response.headers.set("Cache-Control", cacheControl) diff --git a/frontends/main/validateEnv.js b/frontends/main/validateEnv.js index 75d8d0d710..0ff82d9ffa 100644 --- a/frontends/main/validateEnv.js +++ b/frontends/main/validateEnv.js @@ -16,9 +16,6 @@ const yup = require("yup") const schema = yup.object().shape({ // Server-only env vars MITOL_NOINDEX: yup.string().oneOf(["true", "false"]), - NEXT_CACHE_S_MAXAGE_SECONDS: yup - .string() - .matches(/^\d+$/, { excludeEmptyString: true }), // Required client/server vars — must be present in local dev and at runtime. NEXT_PUBLIC_ORIGIN: yup .string() @@ -45,6 +42,10 @@ const schema = yup.object().shape({ NEXT_PUBLIC_MITX_ONLINE_CSRF_COOKIE_NAME: yup.string().required(), // Optional client or server vars NEXT_PUBLIC_APPZI_URL: yup.string(), + // CDN TTL for HTML pages; see getCacheSMaxageSeconds in src/common/config.ts + NEXT_PUBLIC_CACHE_S_MAXAGE_SECONDS: yup + .string() + .matches(/^\d+$/, { excludeEmptyString: true }), NEXT_PUBLIC_MITOL_AXIOS_WITH_CREDENTIALS: yup .string() .oneOf(["true", "false"]), diff --git a/learning_resources/etl/loaders.py b/learning_resources/etl/loaders.py index f7632ba032..fd7481661e 100644 --- a/learning_resources/etl/loaders.py +++ b/learning_resources/etl/loaders.py @@ -62,7 +62,6 @@ bulk_resources_unpublished_actions, content_files_loaded_actions, load_course_blocklist, - load_course_duplicates, resource_delete_actions, resource_run_unpublished_actions, resource_unpublished_actions, @@ -194,9 +193,17 @@ def load_run_dependent_values( def load_instructors( - run: LearningResourceRun, instructors_data: list[dict] + run: LearningResourceRun, instructors_data: list[dict] | None ) -> list[LearningResourceInstructor]: - """Load the instructors for a resource run into the database""" + """Load the instructors for a resource run into the database. + + `None` (as opposed to `[]`) means the source didn't provide instructor + data at all; leave whatever's already on the run alone rather than + clearing it — same convention as load_topics's `topics_data`. + """ + if instructors_data is None: + return list(run.instructors.all()) + instructors = [] valid_attributes = ["first_name", "last_name"] relations = [] @@ -228,9 +235,17 @@ def load_instructors( def load_prices( - run: LearningResourceRun, prices_data: list[dict] + run: LearningResourceRun, prices_data: list[dict] | None ) -> list[LearningResourcePrice]: - """Load the prices for a resource run into the database""" + """Load the prices for a resource run into the database. + + `None` (as opposed to `[]`) means the source didn't provide price data + at all; leave whatever's already on the run alone rather than clearing + it — same convention as load_topics's `topics_data`. + """ + if prices_data is None: + return list(run.resource_prices.all()) + prices = [] for price in prices_data: lr_price, _ = LearningResourcePrice.objects.get_or_create( @@ -306,6 +321,29 @@ def load_content_tags( ) +def _resolve_run_prices( + run_data: dict, + resource_prices: list[dict] | None, + status: str | None, + learning_resource: LearningResource, +) -> list[dict] | None: + """Normalize run_data's "prices" summary field for one run. + + Returns the resource_prices list to pass to load_prices, or None if no + price data was provided by this source (leave existing values alone). + """ + if resource_prices is None: + return None + + run_data["prices"] = sorted({price["amount"] for price in resource_prices}) + if status == RunStatus.archived.value or learning_resource.certification is False: + # Archived runs or runs of resources w/out certificates should not + # have prices + run_data["prices"] = [] + return [] + return resource_prices + + def load_run( learning_resource: LearningResource, run_data: dict ) -> LearningResourceRun: @@ -323,15 +361,16 @@ def load_run( image_data = run_data.pop("image", None) status = run_data.pop("status", None) + # `None` (as opposed to an omitted key, which defaults to `[]`) is the + # sentinel for "not provided by this source, leave existing value + # alone" — same convention load_topics uses for `topics_data`. Sources + # that don't have instructor/price data (e.g. the warehouse-pull + # transforms, which lack pricing entirely) pass `None` explicitly + # rather than `[]`, so a sync doesn't wipe data another pipeline wrote. instructors_data = run_data.pop("instructors", []) - - resource_prices = run_data.get("prices", []) - run_data["prices"] = sorted({price["amount"] for price in resource_prices}) - - if status == RunStatus.archived.value or learning_resource.certification is False: - # Archived runs or runs of resources w/out certificates should not have prices - run_data["prices"] = [] - resource_prices = [] + resource_prices = _resolve_run_prices( + run_data, run_data.pop("prices", []), status, learning_resource + ) if learning_resource.test_mode: run_data["published"] = True @@ -405,10 +444,9 @@ def enqueue_content_tasks(): return learning_resource_run -def upsert_course_or_program( # noqa: C901, PLR0912 +def upsert_course_or_program( # noqa: C901 resource_data: dict, blocklist: list[str], - duplicates: list[dict], resource_type: str, *, config: CourseLoaderConfig = None, @@ -421,8 +459,6 @@ def upsert_course_or_program( # noqa: C901, PLR0912 a dict of course/program data values blocklist (list of str): list of course/program ids not to load - duplicates (list of dict): - list of duplicate course/program data resource_type (str): the type of resource to load (course or program) config (CourseLoaderConfig): @@ -448,14 +484,6 @@ def upsert_course_or_program( # noqa: C901, PLR0912 else: resource_category = LearningResourceType.program.value resource_data["resource_category"] = resource_category - deduplicated_course_id = next( - ( - record["course_id"] - for record in duplicates - if readable_id in record["duplicate_course_ids"] - ), - None, - ) platform = LearningResourcePlatform.objects.filter(code=platform_name).first() if not platform: log.exception( @@ -465,23 +493,12 @@ def upsert_course_or_program( # noqa: C901, PLR0912 ) return None, None - if deduplicated_course_id and readable_id != deduplicated_course_id: - duplicate_resource = LearningResource.objects.filter( - platform=platform, readable_id=readable_id - ).first() - if duplicate_resource: - duplicate_resource.published = False - duplicate_resource.save() - resource_unpublished_actions(duplicate_resource) - - resource_id = deduplicated_course_id or readable_id - if config and config.fetch_only: # Do not upsert the course, it should already exist. # Just find it and return it. resource = ( LearningResource.objects.filter( - readable_id=resource_id, + readable_id=readable_id, platform=platform, resource_type=resource_type, ) @@ -489,11 +506,11 @@ def upsert_course_or_program( # noqa: C901, PLR0912 .first() ) if not resource: - log.warning("No published or test_mode resource found for %s", resource_id) + log.warning("No published or test_mode resource found for %s", readable_id) return resource, False if unique_field_name != READABLE_ID_FIELD: - resource_data[READABLE_ID_FIELD] = resource_id + resource_data[READABLE_ID_FIELD] = readable_id # Some dupes may result, so we should delete all but the # most recently updated resource w/matching unique value existing_courses = LearningResource.objects.filter( @@ -521,7 +538,7 @@ def upsert_course_or_program( # noqa: C901, PLR0912 defaults=resource_data, ) else: - unique_field_value = resource_id + unique_field_value = readable_id return LearningResource.objects.select_for_update().update_or_create( **{unique_field_name: unique_field_value}, platform=platform, @@ -533,7 +550,6 @@ def upsert_course_or_program( # noqa: C901, PLR0912 def load_course( resource_data: dict, blocklist: list[str], - duplicates: list[dict], *, config=CourseLoaderConfig(), ) -> LearningResource: @@ -545,8 +561,6 @@ def load_course( a dict of course data values blocklist (list of str): list of course ids not to load - duplicates (list of dict): - list of duplicate course data config (CourseLoaderConfig): configuration on how to load this program @@ -569,7 +583,6 @@ def load_course( learning_resource, created = upsert_course_or_program( resource_data, blocklist, - duplicates, LearningResourceType.course.name, config=config, ) @@ -635,15 +648,13 @@ def load_courses( A list of course LearningResources """ blocklist = load_course_blocklist() - duplicates = load_course_duplicates(etl_source) courses_list = list(courses_data or []) courses = [ course for course in [ - load_course(course, blocklist, duplicates, config=config) - for course in courses_list + load_course(course, blocklist, config=config) for course in courses_list ] if course is not None ] @@ -682,7 +693,6 @@ class LoadedProgramCourse(NamedTuple): def load_program( program_data: dict, blocklist: list[str], - duplicates: list[dict], *, config=ProgramLoaderConfig(), ) -> ProgramLoadResult: @@ -694,8 +704,6 @@ def load_program( a dict of program data values blocklist (list of str): list of course ids not to load - duplicates (list of dict): - list of duplicate course data config (ProgramLoaderConfig): configuration on how to load this program @@ -719,7 +727,7 @@ def load_program( with transaction.atomic(): learning_resource, created = upsert_course_or_program( - program_data, [], [], LearningResourceType.program.name + program_data, [], LearningResourceType.program.name ) if not learning_resource: return ProgramLoadResult( @@ -756,9 +764,7 @@ def load_program( continue explicit_position = course_data.pop("position", None) - course_resource = load_course( - course_data, blocklist, duplicates, config=config.courses - ) + course_resource = load_course(course_data, blocklist, config=config.courses) if course_resource: loaded_courses.append( LoadedProgramCourse( @@ -890,13 +896,12 @@ def load_programs( PROGRAM_PROGRAMS or PROGRAM_COURSES based on child `display_mode`. """ blocklist = load_course_blocklist() - duplicates = load_course_duplicates(etl_source) # Pass 1: load all programs and their course children results: list[ProgramLoadResult] = [] deferred_child_programs = [] for program_data in programs_data: - result = load_program(program_data, blocklist, duplicates, config=config) + result = load_program(program_data, blocklist, config=config) results.append(result) if result.resource and result.child_programs_data: deferred_child_programs.append( diff --git a/learning_resources/etl/loaders_test.py b/learning_resources/etl/loaders_test.py index b88971abe0..4355e4ec54 100644 --- a/learning_resources/etl/loaders_test.py +++ b/learning_resources/etl/loaders_test.py @@ -49,6 +49,7 @@ load_podcast, load_podcast_episode, load_podcasts, + load_prices, load_problem_file, load_problem_files, load_program, @@ -144,14 +145,6 @@ def mock_blocklist(mocker): ) -@pytest.fixture(autouse=True) -def mock_duplicates(mocker): - """Mock the load_course_duplicates function""" - return mocker.patch( - "learning_resources.etl.loaders.load_course_duplicates", return_value=[] - ) - - @pytest.fixture def mock_get_similar_topics_qdrant(mocker): mocker.patch( @@ -295,7 +288,6 @@ def test_load_program( # noqa: PLR0913 **delivery_data, }, [], - [], ) assert Program.objects.count() == 1 @@ -364,7 +356,6 @@ def test_load_program_preserves_preset_resource_category(mock_upsert_tasks): "resource_category": LearningResourceType.course.value, }, [], - [], ) assert result.resource_category == LearningResourceType.course.value @@ -399,7 +390,6 @@ def test_load_program_defaults_resource_category(mock_upsert_tasks): "courses": [], }, [], - [], ) assert result.resource_category == LearningResourceType.program.value @@ -485,7 +475,7 @@ def test_load_program_bad_platform(mocker): "published": True, "courses": [], } - result, _, _ = load_program(props, [], [], config=ProgramLoaderConfig(prune=True)) + result, _, _ = load_program(props, [], config=ProgramLoaderConfig(prune=True)) assert result is None mock_log.assert_called_once_with( "Platform %s is null or not in database: %s", bad_platform, "abc123" @@ -611,7 +601,7 @@ def test_load_course( # noqa: PLR0913, PLR0912, PLR0915 blocklist = [learning_resource.readable_id] if blocklisted else [] - result = load_course(props, blocklist, [], config=CourseLoaderConfig(prune=True)) + result = load_course(props, blocklist, config=CourseLoaderConfig(prune=True)) assert result.professional is True if is_published and is_run_published and not blocklisted and has_upcoming_run: @@ -706,7 +696,7 @@ def test_load_course_updates_course_numbers(mock_upsert_tasks): "course": {"course_numbers": new_course_numbers}, } - load_course(props, [], [], config=CourseLoaderConfig(prune=True)) + load_course(props, [], config=CourseLoaderConfig(prune=True)) assert Course.objects.count() == 1 course.refresh_from_db() @@ -735,7 +725,7 @@ def test_load_course_bad_platform(mocker): } ], } - result = load_course(props, [], [], config=CourseLoaderConfig(prune=True)) + result = load_course(props, [], config=CourseLoaderConfig(prune=True)) assert result is None mock_log.assert_called_once_with( "Platform %s is null or not in database: %s", bad_platform, "abc123" @@ -786,7 +776,7 @@ def test_load_course_prune_preserves_checksum_on_unpublished_runs(): ], } - load_course(props, [], [], config=CourseLoaderConfig(prune=True)) + load_course(props, [], config=CourseLoaderConfig(prune=True)) retained_run.refresh_from_db() pruned_run.refresh_from_db() @@ -841,7 +831,6 @@ def test_load_program_prune_preserves_checksum_on_unpublished_runs(): "courses": [], }, [], - [], ) retained_run.refresh_from_db() @@ -853,94 +842,6 @@ def test_load_program_prune_preserves_checksum_on_unpublished_runs(): assert pruned_run.checksum == "pruned_checksum" -@pytest.mark.parametrize("course_exists", [True, False]) -@pytest.mark.parametrize("course_id_is_duplicate", [True, False]) -@pytest.mark.parametrize("duplicate_course_exists", [True, False]) -def test_load_duplicate_course( - mock_upsert_tasks, - course_exists, - course_id_is_duplicate, - duplicate_course_exists, -): - """Test that load_course loads the course""" - platform = LearningResourcePlatformFactory.create() - - course = ( - CourseFactory.create(learning_resource__runs=[], platform=platform.code) - if course_exists - else CourseFactory.build() - ) - - duplicate_course = ( - CourseFactory.create(learning_resource__runs=[], platform=platform.code) - if duplicate_course_exists - else CourseFactory.build() - ) - - if course_exists and duplicate_course_exists: - assert Course.objects.count() == 2 - elif course_exists or duplicate_course_exists: - assert Course.objects.count() == 1 - else: - assert Course.objects.count() == 0 - - duplicates = [ - { - "course_id": course.learning_resource.readable_id, - "duplicate_course_ids": [ - course.learning_resource.readable_id, - duplicate_course.learning_resource.readable_id, - ], - } - ] - - course_id = ( - duplicate_course.learning_resource.readable_id - if course_id_is_duplicate - else course.learning_resource.readable_id - ) - - props = { - "readable_id": course_id, - "platform": platform.code, - "title": "New title", - "description": "something", - "runs": [ - { - "run_id": course.learning_resource.readable_id, - "enrollment_start": "2017-01-01T00:00:00Z", - "start_date": "2017-01-20T00:00:00Z", - "end_date": "2017-06-20T00:00:00Z", - } - ], - } - - result = load_course(props, [], duplicates) - - if course_id_is_duplicate and duplicate_course_exists: - mock_upsert_tasks.deindex_learning_resource_immutable_signature.assert_called() - else: - mock_upsert_tasks.deindex_learning_resource_immutable_signature.assert_not_called() - if course.learning_resource.id: - mock_upsert_tasks.upsert_learning_resource_immutable_signature.assert_called_with( - course.learning_resource.id - ) - - assert Course.objects.count() == (2 if duplicate_course_exists else 1) - - assert isinstance(result, LearningResource) - - saved_course = LearningResource.objects.filter( - readable_id=course.learning_resource.readable_id - ).first() - - for key, value in props.items(): - assert getattr(result, key) == value, f"Property {key} should equal {value}" - assert getattr(saved_course, key) == value, ( - f"Property {key} should be updated to {value} in the database" - ) - - @pytest.mark.parametrize("unique_url", [True, False]) def test_load_course_unique_urls(unique_url): """ @@ -973,7 +874,7 @@ def test_load_course_unique_urls(unique_url): } ], } - result = load_course(props, [], []) + result = load_course(props, []) assert result.readable_id == readable_id assert result.url == unique_url assert result.published is True @@ -1016,7 +917,7 @@ def test_load_course_old_id_new_url(): } ], } - result = load_course(props, [], []) + result = load_course(props, []) assert result.readable_id == readable_id assert result.url == unique_url assert result.published is True @@ -1043,7 +944,7 @@ def test_load_course_fetch_only(mocker, course_exists): "platform": platform.code, "offered_by": {"code": OfferedBy.ocw.name}, } - result = load_course(props, [], [], config=CourseLoaderConfig(fetch_only=True)) + result = load_course(props, [], config=CourseLoaderConfig(fetch_only=True)) if course_exists: assert result == resource mock_warn.assert_not_called() @@ -1130,6 +1031,40 @@ def test_load_run(mocker, run_exists, status, certification): mock_import_task.delay.assert_not_called() +@pytest.mark.django_db(transaction=True) +def test_load_run_none_instructors_and_prices_leave_existing_values_alone(mocker): + """`None` (not an omitted key, which still defaults to `[]`) for + run_data's "instructors"/"prices" is the sentinel for "not provided by + this source, leave alone" — same convention load_topics already uses + for `topics_data`. A source that doesn't have instructor/price data + (e.g. the warehouse-pull transforms) must not wipe out values another + pipeline already populated. + """ + mocker.patch("learning_resources.tasks.import_content_files") + course = LearningResourceFactory.create( + is_course=True, runs=[], certification=True, etl_source=ETLSource.xpro.value + ) + run = LearningResourceRunFactory.create(learning_resource=course, prices=[]) + instructor = LearningResourceInstructorFactory.create(full_name="Jane Doe") + load_instructors(run, [{"full_name": instructor.full_name}]) + load_prices(run, [{"amount": Decimal("49.00"), "currency": CURRENCY_USD}]) + run.prices = [Decimal("49.00")] + run.save() + + run_data = { + "run_id": run.run_id, + "title": run.title, + "instructors": None, + "prices": None, + } + result = load_run(course, run_data) + + assert result.id == run.id + assert [i.full_name for i in result.instructors.all()] == [instructor.full_name] + assert [p.amount for p in result.resource_prices.all()] == [Decimal("49.00")] + assert result.prices == [Decimal("49.00")] + + @pytest.mark.parametrize( "etl_source", [ETLSource.mit_edx.value, ETLSource.mitxonline.value, ETLSource.xpro.value], @@ -1513,7 +1448,7 @@ def test_load_offered_bys( @pytest.mark.parametrize("prune", [True, False]) -def test_load_courses(mocker, mock_blocklist, mock_duplicates, prune): +def test_load_courses(mocker, mock_blocklist, prune): """Test that load_courses calls the expected functions""" course_to_unpublish = CourseFactory.create(etl_source=ETLSource.xpro.name) @@ -1535,16 +1470,14 @@ def test_load_courses(mocker, mock_blocklist, mock_duplicates, prune): mock_load_course.assert_any_call( course_data, mock_blocklist.return_value, - mock_duplicates.return_value, config=config, ) mock_blocklist.assert_called_once_with() - mock_duplicates.assert_called_once_with(ETLSource.xpro.name) course_to_unpublish.refresh_from_db() assert course_to_unpublish.learning_resource.published is not prune -def test_load_programs(mocker, mock_blocklist, mock_duplicates): +def test_load_programs(mocker, mock_blocklist): """Test that load_programs calls the expected functions""" program_data = [{"courses": [{"platform": "a"}, {}], "id": 5}] @@ -1560,7 +1493,6 @@ def test_load_programs(mocker, mock_blocklist, mock_duplicates): load_programs("mitx", program_data, config=ProgramLoaderConfig(prune=True)) assert mock_load_program.call_count == len(program_data) mock_blocklist.assert_called_once() - mock_duplicates.assert_called_once_with("mitx") @pytest.fixture @@ -1686,7 +1618,6 @@ def test_load_program_honors_explicit_course_position(mock_upsert_tasks): "courses": program_courses, }, [], - [], ) positions_by_readable = { @@ -3270,7 +3201,7 @@ def test_load_course_percolation( props["runs"] = [] blocklist = [learning_resource.readable_id] if blocklisted else [] - result = load_course(props, blocklist, [], config=CourseLoaderConfig(prune=True)) + result = load_course(props, blocklist, config=CourseLoaderConfig(prune=True)) mock_upsert_tasks.upsert_learning_resource_immutable_signature.assert_called_with( result.id ) @@ -3569,7 +3500,7 @@ def test_course_with_unpublished_force_ingest_is_test_mode(): } ], } - course = load_course(course_data, [], []) + course = load_course(course_data, []) assert course.require_summaries is True assert course.test_mode is True assert course.published is False diff --git a/learning_resources/etl/pipelines.py b/learning_resources/etl/pipelines.py index d686582e41..49e0013c87 100644 --- a/learning_resources/etl/pipelines.py +++ b/learning_resources/etl/pipelines.py @@ -135,7 +135,7 @@ def ocw_courses_etl( ) if data: ocw_course_data = ocw.transform_course(data) - course_resource = loaders.load_course(ocw_course_data, [], []) + course_resource = loaders.load_course(ocw_course_data, []) course_run = course_resource.runs.filter(published=True).first() if course_resource and not skip_content_files: diff --git a/learning_resources/lib/__init__.py b/learning_resources/lib/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/learning_resources/lib/warehouse.py b/learning_resources/lib/warehouse.py new file mode 100644 index 0000000000..b7cb576507 --- /dev/null +++ b/learning_resources/lib/warehouse.py @@ -0,0 +1,307 @@ +"""SQL warehouse pull infrastructure for data platform consumption. + +Query engine is not hard-wired into the ETL tasks or row-iteration logic: +the concrete backend is selected at connect time via +``settings.WAREHOUSE_BACKEND``. StarRocks is the only implemented backend +today, but any engine exposing a standard Python DB-API 2.0 cursor +(``execute`` / ``fetchmany`` / ``description`` — e.g. DuckDB) is a drop-in +addition: add a ``_connect_`` function and register it in +``_CONNECTORS`` below. ``iter_rows`` and ``BaseWarehouseETLTask`` need no +changes to support a new backend. +""" + +import logging +import re +import time +from datetime import UTC, datetime, timedelta + +import sentry_sdk +from celery import Task +from django.conf import settings +from django.core.cache import caches +from django.core.exceptions import ImproperlyConfigured + +log = logging.getLogger(__name__) + +# Only alphanumeric, underscores, and dots (schema.table notation). `\Z` +# (not `$`) is deliberate: `$` matches just before a trailing newline, which +# would let a `view_name` ending in "\n" slip through this check. +_SAFE_IDENTIFIER = re.compile(r"^[a-zA-Z0-9_.]+\Z") + +# Cache alias for sync watermarks: must outlive Redis flushes (an evicted +# watermark just makes the next incremental run fetch a wider-than-necessary +# window, which is safe — see BaseWarehouseETLTask.fetch_and_upsert's +# ``since`` contract), but should not require a dedicated model + migration +# for what is ETL bookkeeping, not domain data. +_WATERMARK_CACHE = "durable" +_WATERMARK_SQL_FORMAT = "%Y-%m-%d %H:%M:%S.%f" + +# Subtracted from the wall-clock watermark before it's stored: this consumer +# doesn't know the warehouse's own build/replication lag, so a row modified +# just before fetch_started_at could still be invisible in the queried view +# at query time (a dbt run hasn't rebuilt yet, an Iceberg commit hasn't +# landed) and would otherwise be permanently skipped by every future +# incremental pull, since its true last_modified is already below the +# watermark. This overlap trades a few harmlessly-reprocessed rows each +# incremental run (upsert is idempotent) for closing that skip window, as +# long as warehouse lag never exceeds it. +_WATERMARK_LOOKBACK = timedelta(minutes=10) + + +def _connect_starrocks(): + """Open a StarRocks DB-API connection using Django settings. + + No database is set at connect time — pymysql's `database=` becomes + MySQL's COM_INIT_DB, which StarRocks resolves as a database name (or a + `catalog.database` pair), never a bare catalog name alone. Every + view_name used with this connection must stay fully catalog-qualified + (catalog.database.table) as a result — see iter_rows's docstring. + + Raises: + ImproperlyConfigured: If STARROCKS_HOST or STARROCKS_USER is unset — + fails fast with a clear message instead of pymysql attempting + host=None/user=None. + """ + import pymysql + + if not settings.STARROCKS_HOST or not settings.STARROCKS_USER: + msg = ( + "WAREHOUSE_BACKEND is 'starrocks' but STARROCKS_HOST/" + "STARROCKS_USER are not set. Set STARROCKS_HOST/STARROCKS_PORT/" + "STARROCKS_USER/STARROCKS_PASSWORD, or switch WAREHOUSE_BACKEND " + "to a configured backend." + ) + raise ImproperlyConfigured(msg) + + return pymysql.connect( + host=settings.STARROCKS_HOST, + port=settings.STARROCKS_PORT, + user=settings.STARROCKS_USER, + password=settings.STARROCKS_PASSWORD, + cursorclass=pymysql.cursors.Cursor, + ) + + +_CONNECTORS = { + "starrocks": _connect_starrocks, +} + + +def connect_to_warehouse(): + """Open and return a DB-API connection for the configured warehouse backend. + + Returns: + A DB-API 2.0 connection (``pymysql.connections.Connection`` today). + + Raises: + ValueError: If ``settings.WAREHOUSE_BACKEND`` names an unknown backend. + Exception: propagated if the connection attempt fails. + """ + backend = settings.WAREHOUSE_BACKEND + try: + connector = _CONNECTORS[backend] + except KeyError: + msg = f"Unknown WAREHOUSE_BACKEND: {backend!r}" + raise ValueError(msg) from None + + try: + conn = connector() + except Exception: + log.exception("Failed to connect to warehouse backend %s", backend) + raise + log.info("Connected to warehouse backend %s", backend) + return conn + + +def iter_rows(conn, view_name, *, since=None, batch_size=1000): + """Iterate over rows in a warehouse view as column-keyed dicts. + + Args: + conn: An open DB-API connection. + view_name (str): Schema-qualified view name, e.g. + ``"ol_data_lake_production.ol_warehouse_production_integrations." + "integrations__learn__ocw_courses"``. A bare ``schema.table`` + name also works if the connection's default database supplies + the catalog. Must contain only alphanumeric characters, + underscores, and dots. + since (datetime | None): If given, only rows whose ``last_modified`` + column is greater than this timestamp are returned — every + ``integrations__learn__*`` view exposes ``last_modified`` per the + contract in ol-data-platform's docs/learn_marts_contract.md, so + this is safe to assume for any view passed here. ``None`` (the + default) pulls every row. + batch_size (int): Rows fetched per round-trip. + + Yields: + dict: One row, keyed by column name. + + Raises: + ValueError: If *view_name* contains characters that could allow SQL injection. + """ + if not _SAFE_IDENTIFIER.match(view_name): + msg = f"Unsafe view name: {view_name!r}" + raise ValueError(msg) + + query = f"SELECT * FROM {view_name}" # noqa: S608 + if since is not None: + # `since` is produced by our own watermark tracking, never user + # input, so a formatted literal is safe here (no parameterization + # needed). A bare quoted string, NOT a `TIMESTAMP '...'` literal — + # confirmed against a live StarRocks instance that the `TIMESTAMP` + # keyword form is a syntax error there (StarRocks implicitly casts + # a string literal compared against a DATETIME column instead). + watermark = since.strftime(_WATERMARK_SQL_FORMAT)[:-3] + query += f" WHERE last_modified > '{watermark}'" + + cur = conn.cursor() + try: + cur.execute(query) + # Read columns from the first non-empty batch rather than + # immediately after execute(): some DB-API drivers only populate + # `description` once results start arriving, not at execute() + # time (PEP 249 leaves this driver-defined). + columns = None + while True: + batch = cur.fetchmany(batch_size) + if not batch: + break + if columns is None: + columns = [d[0] for d in cur.description] + for row in batch: + yield dict(zip(columns, row)) + finally: + cur.close() + + +class BaseWarehouseETLTask(Task): + """Celery task base class for warehouse-pull ETL jobs. + + Subclasses declare ``view_name`` and implement + ``fetch_and_upsert(conn, *, since)``. Register concrete subclasses via + ``app.register_task(SubclassTask())`` — a class decorated with + ``@app.task(base=BaseWarehouseETLTask)`` does *not* work: Celery's + function-task machinery wraps the decorated object as a ``staticmethod`` + and calls it positionally, which — given a class — tries to instantiate + it as the task body instead of running its bound + ``run()``/``fetch_and_upsert()`` methods. + + Two sync modes, selected by the ``full_refresh`` kwarg passed to + ``run()``/``.delay()``/``.apply_async()``: + + - ``full_refresh=True`` (the default, and what the daily beat schedule + uses today): ``since`` is ``None`` — ``fetch_and_upsert`` pulls every + row and should prune resources no longer present in the source. This + is the self-healing baseline: it's the only mode that ever sees + deletes/unpublishes upstream. + - ``full_refresh=False``: ``since`` is this task's last recorded + watermark (or ``None`` if it has never completed an incremental run, + in which case it behaves like a full pull). ``fetch_and_upsert`` + should pass ``since`` through to ``iter_rows`` and skip pruning — + a partial pull must never be treated as the complete state of the + source. On success the watermark is advanced to "now minus + ``_WATERMARK_LOOKBACK``" (not exactly "now") — see that constant's + comment for why a bare wall-clock watermark can permanently skip + warehouse rows under build/replication lag. + + Example:: + + class SyncOCWCoursesTask(BaseWarehouseETLTask): + name = "learning_resources.tasks.SyncOCWCoursesTask" + view_name = ( + "ol_data_lake_production.ol_warehouse_production_integrations" + ".integrations__learn__ocw_courses" + ) + + def fetch_and_upsert(self, conn, *, since=None): + for row in iter_rows(conn, self.view_name, since=since): + upsert_ocw_course(row, prune=since is None) + + SyncOCWCoursesTask = app.register_task(SyncOCWCoursesTask()) + """ + + abstract = True + acks_late = True + view_name: str = "" + + def run(self, *args, full_refresh: bool = True, **kwargs): # noqa: ARG002 + """Open a warehouse connection, delegate to ``fetch_and_upsert``, log counts.""" + if not self.view_name: + msg = f"{self.__class__.__name__}.view_name must be set" + raise ValueError(msg) + + since = None if full_refresh else self._get_watermark() + # Captured before the fetch, not after: a row modified while the + # fetch is in flight must still be picked up by the *next* + # incremental run. Stamping the watermark post-fetch would let it + # fall between this window and the next, invisible until the next + # full_refresh heals it. + fetch_started_at = datetime.now(tz=UTC) + + conn = connect_to_warehouse() + start = time.monotonic() + try: + count = self.fetch_and_upsert(conn, since=since) + except Exception: + log.exception("Warehouse ETL task %s failed", self.name) + sentry_sdk.add_breadcrumb( + category="warehouse_etl", + message=f"{self.name} failed", + data={"view_name": self.view_name, "full_refresh": full_refresh}, + level="error", + ) + raise + finally: + conn.close() + + # A subclass that forgets `return count` (or returns the wrong + # type) implicitly returns None — checked before advancing the + # watermark, not just before the %d-formatted log line below, so a + # broken fetch_and_upsert fails loudly here instead of silently + # advancing the watermark and then crashing on the log call, + # which would permanently skip whatever this run should have + # picked up (the watermark update wouldn't roll back on that + # crash). + if not isinstance(count, int): + msg = ( + f"{self.__class__.__name__}.fetch_and_upsert must return " + f"int, got {type(count).__name__}" + ) + raise TypeError(msg) + + if not full_refresh: + self._set_watermark(fetch_started_at - _WATERMARK_LOOKBACK) + + elapsed = time.monotonic() - start + log.info( + "Warehouse ETL task %s finished (%s): %d rows in %.1fs", + self.name, + "full_refresh" if full_refresh else "incremental", + count, + elapsed, + ) + return count + + def fetch_and_upsert(self, conn, *, since=None) -> int: + """Pull rows from the warehouse and upsert into Django models. + + Args: + conn: An open DB-API connection (will be closed by ``run``). + since (datetime | None): Forwarded from ``run()``. ``None`` means + a full refresh — pull everything and prune. Otherwise, pull + only rows changed since this watermark and skip pruning. + + Returns: + int: Number of rows processed. + """ + raise NotImplementedError + + def _watermark_cache_key(self) -> str: + return f"warehouse_etl:last_synced_at:{self.name}" + + def _get_watermark(self): + """Return this task's last recorded incremental watermark, if any.""" + return caches[_WATERMARK_CACHE].get(self._watermark_cache_key()) + + def _set_watermark(self, value: datetime) -> None: + """Persist ``value`` as this task's incremental watermark.""" + caches[_WATERMARK_CACHE].set(self._watermark_cache_key(), value, timeout=None) diff --git a/learning_resources/lib/warehouse_factories.py b/learning_resources/lib/warehouse_factories.py new file mode 100644 index 0000000000..71ca48c05f --- /dev/null +++ b/learning_resources/lib/warehouse_factories.py @@ -0,0 +1,28 @@ +"""Synthetic warehouse-source row factories. + +Unlike learning_resources.factories (Django ORM models), these produce plain +dicts shaped like rows from an integrations__learn__* warehouse view — used +to seed a local StarRocks table so BaseWarehouseETLTask/iter_rows can be +exercised against real SQL instead of a mocked cursor (see +warehouse_integration_test.py). Each per-source PR (mitxonline, xpro, +mit_edx, ocw, program_certificates) should add its own factory here matching +that source's actual view schema, alongside its transform function. +""" + +import factory + +from main.utils import now_in_utc + + +class WarehouseTestRowFactory(factory.DictFactory): + """Generic row factory for the machinery's own scratch test table. + + Matches the `integrations__learn__test` view name convention already + used by warehouse_test.py's mock-based tests — not tied to any real + Cohort-1 source's schema, just enough columns to exercise iter_rows' + since-filtering and BaseWarehouseETLTask's fetch/watermark cycle. + """ + + id = factory.Sequence(lambda n: n) + title = factory.Faker("sentence", nb_words=4) + last_modified = factory.LazyFunction(now_in_utc) diff --git a/learning_resources/lib/warehouse_integration_test.py b/learning_resources/lib/warehouse_integration_test.py new file mode 100644 index 0000000000..5a7041dd05 --- /dev/null +++ b/learning_resources/lib/warehouse_integration_test.py @@ -0,0 +1,154 @@ +"""Integration tests for learning_resources.lib.warehouse against a real +StarRocks instance (the `starrocks/allin1-ubuntu` image — see +docker-compose.services.yml and .github/workflows/ci.yml). + +Skipped entirely unless STARROCKS_HOST is configured, so a local `pytest` +run without the `backend` compose profile up (or without CI's service +container) doesn't fail — warehouse_test.py's mock-based tests already cover +this module's logic without a live dependency; these tests exist to prove +the DB-API wiring itself works against a real StarRocks server, which a +mock can't do. +""" + +import os +from datetime import UTC, datetime, timedelta + +import pytest +from django.conf import settings + +from learning_resources.lib.warehouse import BaseWarehouseETLTask, iter_rows +from learning_resources.lib.warehouse_factories import WarehouseTestRowFactory + +pytestmark = [ + pytest.mark.skipif( + not settings.STARROCKS_HOST, + reason="STARROCKS_HOST is not configured — no live StarRocks to test against", + ), +] + +# CI runs `pytest -n logical` (see scripts/test/python_tests.sh); these +# tests share one real StarRocks table across a CREATE/INSERT/DROP +# lifecycle, so two xdist workers running this module's tests concurrently +# would race on it (one worker's teardown DROP colliding with another's +# still-running test, or their inserted rows contaminating each other's +# assertions). PYTEST_XDIST_WORKER is unset (falls back to "master") when +# not running under xdist, so this is a no-op outside CI. +_WORKER_ID = os.environ.get("PYTEST_XDIST_WORKER", "master") +_TEST_DB = f"test_integrations_{_WORKER_ID}" +_TEST_TABLE = "integrations__learn__test" +_TEST_VIEW = f"{_TEST_DB}.{_TEST_TABLE}" + + +@pytest.fixture +def starrocks_conn(): + """Open a live connection to the StarRocks test instance, with a scratch + table created before the test and dropped after. + """ + import pymysql + + conn = pymysql.connect( + host=settings.STARROCKS_HOST, + port=settings.STARROCKS_PORT, + user=settings.STARROCKS_USER, + password=settings.STARROCKS_PASSWORD, + ) + try: + with conn.cursor() as cur: + cur.execute(f"CREATE DATABASE IF NOT EXISTS {_TEST_DB}") + cur.execute(f""" + CREATE TABLE IF NOT EXISTS {_TEST_VIEW} ( + id INT, + title VARCHAR(255), + last_modified DATETIME + ) + ENGINE=OLAP + DUPLICATE KEY(id) + DISTRIBUTED BY HASH(id) + PROPERTIES ("replication_num" = "1") + """) + conn.commit() + yield conn + finally: + with conn.cursor() as cur: + cur.execute(f"DROP TABLE IF EXISTS {_TEST_VIEW}") + conn.commit() + conn.close() + + +def _insert_rows(conn, rows): + with conn.cursor() as cur: + cur.executemany( + f"INSERT INTO {_TEST_VIEW} (id, title, last_modified) VALUES (%s, %s, %s)", # noqa: S608 + [(row["id"], row["title"], row["last_modified"]) for row in rows], + ) + conn.commit() + + +class _CollectingTask(BaseWarehouseETLTask): + name = "test.CollectingTask" + view_name = _TEST_VIEW + + def fetch_and_upsert(self, conn, *, since=None) -> int: + self.collected = list(iter_rows(conn, self.view_name, since=since)) + return len(self.collected) + + +def test_iter_rows_reads_real_rows(starrocks_conn): + """iter_rows pulls actual rows back from a live StarRocks table.""" + rows = WarehouseTestRowFactory.create_batch(3) + _insert_rows(starrocks_conn, rows) + + result = sorted(iter_rows(starrocks_conn, _TEST_VIEW), key=lambda r: r["id"]) + + assert [r["id"] for r in result] == sorted(row["id"] for row in rows) + assert [r["title"] for r in result] == [ + row["title"] for row in sorted(rows, key=lambda r: r["id"]) + ] + + +def test_iter_rows_since_filters_real_rows(starrocks_conn): + """An incremental pull only returns rows newer than the watermark.""" + now = datetime.now(tz=UTC) + old_row = WarehouseTestRowFactory.create( + id=1, last_modified=now - timedelta(days=1) + ) + new_row = WarehouseTestRowFactory.create( + id=2, last_modified=now + timedelta(minutes=1) + ) + _insert_rows(starrocks_conn, [old_row, new_row]) + + result = list(iter_rows(starrocks_conn, _TEST_VIEW, since=now)) + + assert [r["id"] for r in result] == [new_row["id"]] + + +def test_base_warehouse_etl_task_runs_against_real_starrocks(mocker, starrocks_conn): + """BaseWarehouseETLTask.run() connects, fetches, and closes against a + real StarRocks connection end-to-end (connect_to_warehouse itself, not + just iter_rows, since the other tests here pass starrocks_conn directly). + + Uses a second, task-owned connection rather than the `starrocks_conn` + fixture's own connection: `run()` unconditionally closes whatever + connect_to_warehouse() returns, which would otherwise close the + fixture's connection out from under its own teardown. + """ + import pymysql + + task_conn = pymysql.connect( + host=settings.STARROCKS_HOST, + port=settings.STARROCKS_PORT, + user=settings.STARROCKS_USER, + password=settings.STARROCKS_PASSWORD, + ) + mocker.patch( + "learning_resources.lib.warehouse.connect_to_warehouse", + return_value=task_conn, + ) + rows = WarehouseTestRowFactory.create_batch(2) + _insert_rows(starrocks_conn, rows) + + task = _CollectingTask() + count = task.run() + + assert count == len(rows) + assert {r["id"] for r in task.collected} == {row["id"] for row in rows} diff --git a/learning_resources/lib/warehouse_test.py b/learning_resources/lib/warehouse_test.py new file mode 100644 index 0000000000..73b0a0961c --- /dev/null +++ b/learning_resources/lib/warehouse_test.py @@ -0,0 +1,548 @@ +"""Unit tests for learning_resources.lib.warehouse. + +Intentionally avoids loading the full Django application — patches +django.conf.settings directly so these tests run in milliseconds with no +database setup required. +""" + +from datetime import UTC, datetime, timedelta +from unittest.mock import MagicMock, patch + +import django +import pytest +from django.conf import settings as django_settings +from django.core.exceptions import ImproperlyConfigured +from freezegun import freeze_time + +# Configure Django minimally if not already configured — keeps this test +# module self-contained and fast (no database, no app registry needed). +if not django_settings.configured: + django_settings.configure( + WAREHOUSE_BACKEND="starrocks", + STARROCKS_HOST="starrocks.example.com", + STARROCKS_PORT=9030, + STARROCKS_USER="testuser", + STARROCKS_PASSWORD="secret", # noqa: S106 + ) + django.setup() + +# `caches["durable"]` (the incremental watermark store) is only touched on +# the full_refresh=False path — every test below mocks it directly rather +# than configuring a real CACHES setting, keeping this module's "no +# database, no app registry" guarantee intact. + + +from learning_resources.lib.warehouse import ( + _WATERMARK_LOOKBACK, + BaseWarehouseETLTask, + connect_to_warehouse, + iter_rows, +) + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _patch_settings(**kwargs): + """Return a patcher that overrides specific Django settings values.""" + return patch.multiple("django.conf.settings", **kwargs) + + +def _warehouse_settings(**overrides): + settings = { + "WAREHOUSE_BACKEND": "starrocks", + "STARROCKS_HOST": "starrocks.example.com", + "STARROCKS_PORT": 9030, + "STARROCKS_USER": "testuser", + "STARROCKS_PASSWORD": "secret", + } + settings.update(overrides) + return settings + + +def _make_cursor(columns, rows): + """Build a minimal mock cursor returning all rows in one batch then empty.""" + cursor = MagicMock() + cursor.description = [(col,) for col in columns] + cursor.fetchmany.side_effect = [rows, []] + return cursor + + +# --------------------------------------------------------------------------- +# connect_to_warehouse +# --------------------------------------------------------------------------- + + +@patch("pymysql.connect") +def test_connect_to_warehouse_starrocks_uses_settings(mock_connect): + """connect_to_warehouse dispatches to StarRocks and passes settings through.""" + mock_conn = MagicMock() + mock_connect.return_value = mock_conn + + with _patch_settings(**_warehouse_settings()): + result = connect_to_warehouse() + + assert result is mock_conn + call_kwargs = mock_connect.call_args.kwargs + assert call_kwargs["host"] == "starrocks.example.com" + assert call_kwargs["port"] == 9030 + assert call_kwargs["user"] == "testuser" + assert call_kwargs["password"] == "secret" # noqa: S105 + assert "database" not in call_kwargs + + +@patch("pymysql.connect", side_effect=OSError("unreachable")) +def test_connect_to_warehouse_propagates_exception(mock_connect): + """connect_to_warehouse re-raises connection errors.""" + with ( + _patch_settings(**_warehouse_settings()), + pytest.raises(OSError, match="unreachable"), + ): + connect_to_warehouse() + + +def test_connect_to_warehouse_requires_host_and_user(): + """connect_to_warehouse fails fast when STARROCKS_HOST/USER are unset, + rather than letting pymysql attempt host=None/user=None. + """ + with ( + _patch_settings(**_warehouse_settings(STARROCKS_HOST=None)), + pytest.raises(ImproperlyConfigured, match="STARROCKS_HOST"), + ): + connect_to_warehouse() + + +def test_connect_to_warehouse_rejects_unknown_backend(): + """connect_to_warehouse raises ValueError for an unconfigured backend name.""" + with ( + _patch_settings(**_warehouse_settings(WAREHOUSE_BACKEND="snowflake")), + pytest.raises(ValueError, match="Unknown WAREHOUSE_BACKEND"), + ): + connect_to_warehouse() + + +# --------------------------------------------------------------------------- +# iter_rows +# --------------------------------------------------------------------------- + + +def test_iter_rows_yields_column_keyed_dicts(): + """iter_rows converts raw tuples to column-keyed dicts.""" + conn = MagicMock() + cursor = _make_cursor(["id", "title"], [(1, "Course A"), (2, "Course B")]) + conn.cursor.return_value = cursor + + rows = list(iter_rows(conn, "catalog.schema.my_view")) + + assert rows == [ + {"id": 1, "title": "Course A"}, + {"id": 2, "title": "Course B"}, + ] + + +def test_iter_rows_respects_batch_size(): + """iter_rows passes batch_size to fetchmany.""" + conn = MagicMock() + cursor = _make_cursor(["id"], [(1,), (2,)]) + conn.cursor.return_value = cursor + + list(iter_rows(conn, "catalog.schema.my_view", batch_size=500)) + + cursor.fetchmany.assert_any_call(500) + + +def test_iter_rows_closes_cursor_on_success(): + """iter_rows always closes the cursor on normal exit.""" + conn = MagicMock() + cursor = _make_cursor(["id"], []) + conn.cursor.return_value = cursor + + list(iter_rows(conn, "catalog.schema.view")) + + cursor.close.assert_called_once() + + +def test_iter_rows_closes_cursor_on_error(): + """iter_rows closes the cursor even when execution raises.""" + conn = MagicMock() + cursor = MagicMock() + cursor.execute.side_effect = RuntimeError("query failed") + conn.cursor.return_value = cursor + + with pytest.raises(RuntimeError): + list(iter_rows(conn, "catalog.schema.view")) + + cursor.close.assert_called_once() + + +@pytest.mark.parametrize( + "bad_name", + [ + "'; DROP TABLE courses; --", + "schema.table; DELETE FROM users", + "schema.table WHERE 1=1", + "../etc/passwd", + pytest.param("schema.table\n", id="trailing-newline"), + ], +) +def test_iter_rows_rejects_unsafe_view_names(bad_name): + """iter_rows raises ValueError for names that could allow SQL injection.""" + conn = MagicMock() + with pytest.raises(ValueError, match="Unsafe view name"): + list(iter_rows(conn, bad_name)) + + +def test_iter_rows_without_since_has_no_where_clause(): + """A full-refresh pull (since=None) queries the view unfiltered.""" + conn = MagicMock() + cursor = _make_cursor(["id"], []) + conn.cursor.return_value = cursor + + list(iter_rows(conn, "catalog.schema.my_view")) + + query = cursor.execute.call_args.args[0] + assert query == "SELECT * FROM catalog.schema.my_view" + + +def test_iter_rows_with_since_filters_on_last_modified(): + """An incremental pull (since=) adds a last_modified predicate.""" + conn = MagicMock() + cursor = _make_cursor(["id"], []) + conn.cursor.return_value = cursor + since = datetime(2026, 6, 15, 12, 30, 0, tzinfo=UTC) + + list(iter_rows(conn, "catalog.schema.my_view", since=since)) + + query = cursor.execute.call_args.args[0] + assert query == ( + "SELECT * FROM catalog.schema.my_view " + "WHERE last_modified > '2026-06-15 12:30:00.000'" + ) + + +def test_iter_rows_accepts_dotted_identifiers(): + """iter_rows accepts fully-qualified catalog.schema.table names.""" + conn = MagicMock() + cursor = _make_cursor(["id"], []) + conn.cursor.return_value = cursor + + # Should not raise + list( + iter_rows( + conn, + "ol_data_lake_production.ol_warehouse_production_integrations." + "integrations__learn__ocw_courses", + ) + ) + + +def test_iter_rows_defers_description_until_first_fetch(): + """Some DB-API drivers only populate cursor.description once results + start arriving, not immediately after execute() (PEP 249 leaves this + driver-defined) — iter_rows must not read it until after the first + fetchmany call. + """ + conn = MagicMock() + cursor = MagicMock() + cursor.description = None + calls = [] + + def _fetchmany(size): + if not calls: + calls.append(1) + cursor.description = [("id",), ("title",)] + return [(1, "Course A")] + return [] + + cursor.fetchmany.side_effect = _fetchmany + conn.cursor.return_value = cursor + + rows = list(iter_rows(conn, "catalog.schema.my_view")) + + assert rows == [{"id": 1, "title": "Course A"}] + + +# --------------------------------------------------------------------------- +# BaseWarehouseETLTask +# --------------------------------------------------------------------------- + + +class _ConcreteTask(BaseWarehouseETLTask): + name = "test.ConcreteTask" + view_name = "ol_data_lake_production.ol_warehouse_production_integrations.integrations__learn__test" + + def fetch_and_upsert(self, conn, *, since=None) -> int: # noqa: ARG002 + return 42 + + +class _ErrorTask(BaseWarehouseETLTask): + name = "test.ErrorTask" + view_name = "ol_data_lake_production.ol_warehouse_production_integrations.integrations__learn__test" + + def fetch_and_upsert(self, conn, *, since=None) -> int: # noqa: ARG002 + msg = "downstream failure" + raise RuntimeError(msg) + + +@patch("learning_resources.lib.warehouse.connect_to_warehouse") +def test_base_warehouse_etl_task_run_success(mock_connect): + """run() calls fetch_and_upsert, closes the connection, and returns row count.""" + mock_conn = MagicMock() + mock_connect.return_value = mock_conn + + result = _ConcreteTask().run() + + assert result == 42 + mock_conn.close.assert_called_once() + + +@patch("learning_resources.lib.warehouse.connect_to_warehouse") +def test_base_warehouse_etl_task_closes_connection_on_error(mock_connect): + """run() closes the warehouse connection even when fetch_and_upsert raises.""" + mock_conn = MagicMock() + mock_connect.return_value = mock_conn + + with pytest.raises(RuntimeError, match="downstream failure"): + _ErrorTask().run() + + mock_conn.close.assert_called_once() + + +@patch("learning_resources.lib.warehouse.sentry_sdk") +@patch("learning_resources.lib.warehouse.connect_to_warehouse") +def test_base_warehouse_etl_task_adds_sentry_breadcrumb_on_error( + mock_connect, mock_sentry +): + """run() adds a Sentry breadcrumb at error level when fetch_and_upsert fails.""" + mock_connect.return_value = MagicMock() + + with pytest.raises(RuntimeError): + _ErrorTask().run() + + mock_sentry.add_breadcrumb.assert_called_once() + call_kwargs = mock_sentry.add_breadcrumb.call_args.kwargs + assert call_kwargs["level"] == "error" + assert call_kwargs["category"] == "warehouse_etl" + + +@patch("learning_resources.lib.warehouse.connect_to_warehouse") +def test_base_warehouse_etl_task_raises_when_view_name_empty(mock_connect): + """run() raises ValueError immediately when view_name is not set.""" + + class _NoViewTask(BaseWarehouseETLTask): + view_name = "" + + def fetch_and_upsert(self, conn) -> int: # noqa: ARG002 + return 0 + + with pytest.raises(ValueError, match="view_name must be set"): + _NoViewTask().run() + + mock_connect.assert_not_called() + + +def test_base_warehouse_etl_task_fetch_and_upsert_is_abstract(): + """fetch_and_upsert raises NotImplementedError on the base class.""" + task = BaseWarehouseETLTask() + task.view_name = "catalog.schema.view" + with pytest.raises(NotImplementedError): + task.fetch_and_upsert(conn=None) + + +@patch("learning_resources.lib.warehouse.connect_to_warehouse") +def test_base_warehouse_etl_task_rejects_non_int_return(mock_connect): + """A subclass that forgets `return count` (implicitly returning None, + or any other non-int) fails loudly, and — critically — never advances + the watermark, so the run's data isn't silently skipped by every + future incremental pull. + """ + mock_connect.return_value = MagicMock() + + class _BrokenTask(BaseWarehouseETLTask): + name = "test.BrokenTask" + view_name = ( + "ol_data_lake_production.ol_warehouse_production_integrations" + ".integrations__learn__test" + ) + + def fetch_and_upsert(self, conn, *, since=None): + pass # forgot to return count + + task = _BrokenTask() + mock_cache = MagicMock() + + with patch("learning_resources.lib.warehouse.caches") as mock_caches: + mock_caches.__getitem__.return_value = mock_cache + with pytest.raises(TypeError, match="must return int"): + task.run(full_refresh=False) + + mock_cache.set.assert_not_called() + + +def test_base_warehouse_etl_task_acks_late(): + """acks_late=True, matching the rest of the ETL task fleet (get_ocw_data, + ingest_edx_run_archive, etc.) — a worker lost mid-pull shouldn't lose + the message. + """ + assert BaseWarehouseETLTask.acks_late is True + + +# --------------------------------------------------------------------------- +# full_refresh vs. incremental +# --------------------------------------------------------------------------- + + +class _RecordingTask(BaseWarehouseETLTask): + """Records the `since` it was called with instead of hitting a real view.""" + + name = "test.RecordingTask" + view_name = "ol_data_lake_production.ol_warehouse_production_integrations.integrations__learn__test" + + def fetch_and_upsert(self, conn, *, since=None) -> int: # noqa: ARG002 + self.seen_since = since + return 7 + + +@patch("learning_resources.lib.warehouse.connect_to_warehouse") +def test_full_refresh_defaults_to_true_and_passes_since_none(mock_connect): + """Calling run() with no kwargs is a full refresh: since=None, no watermark I/O.""" + mock_connect.return_value = MagicMock() + task = _RecordingTask() + + with patch("learning_resources.lib.warehouse.caches") as mock_caches: + result = task.run() + + assert result == 7 + assert task.seen_since is None + mock_caches.__getitem__.assert_not_called() + + +@patch("learning_resources.lib.warehouse.connect_to_warehouse") +def test_incremental_reads_watermark_and_passes_it_through(mock_connect): + """full_refresh=False reads the last recorded watermark and forwards it.""" + mock_connect.return_value = MagicMock() + task = _RecordingTask() + stored_watermark = datetime(2026, 6, 1, tzinfo=UTC) + mock_cache = MagicMock() + mock_cache.get.return_value = stored_watermark + + with patch("learning_resources.lib.warehouse.caches") as mock_caches: + mock_caches.__getitem__.return_value = mock_cache + result = task.run(full_refresh=False) + + assert result == 7 + assert task.seen_since is stored_watermark + mock_cache.get.assert_called_once_with( + "warehouse_etl:last_synced_at:test.RecordingTask" + ) + + +@patch("learning_resources.lib.warehouse.connect_to_warehouse") +def test_incremental_with_no_prior_watermark_passes_since_none(mock_connect): + """An incremental run with no recorded watermark yet behaves like a full pull.""" + mock_connect.return_value = MagicMock() + task = _RecordingTask() + mock_cache = MagicMock() + mock_cache.get.return_value = None + + with patch("learning_resources.lib.warehouse.caches") as mock_caches: + mock_caches.__getitem__.return_value = mock_cache + task.run(full_refresh=False) + + assert task.seen_since is None + + +@patch("learning_resources.lib.warehouse.connect_to_warehouse") +def test_incremental_advances_watermark_on_success(mock_connect): + """A successful incremental run stores a fresh watermark, durably.""" + mock_connect.return_value = MagicMock() + task = _RecordingTask() + mock_cache = MagicMock() + mock_cache.get.return_value = None + + with patch("learning_resources.lib.warehouse.caches") as mock_caches: + mock_caches.__getitem__.return_value = mock_cache + task.run(full_refresh=False) + + mock_caches.__getitem__.assert_called_with("durable") + mock_cache.set.assert_called_once() + call_args = mock_cache.set.call_args + assert call_args.args[0] == "warehouse_etl:last_synced_at:test.RecordingTask" + assert isinstance(call_args.args[1], datetime) + assert call_args.kwargs["timeout"] is None + + +@patch("learning_resources.lib.warehouse.connect_to_warehouse") +def test_full_refresh_does_not_advance_watermark(mock_connect): + """A full-refresh run never writes to the watermark cache.""" + mock_connect.return_value = MagicMock() + task = _RecordingTask() + + with patch("learning_resources.lib.warehouse.caches") as mock_caches: + task.run(full_refresh=True) + + mock_caches.__getitem__.assert_not_called() + + +@patch("learning_resources.lib.warehouse.connect_to_warehouse") +def test_incremental_watermark_is_stamped_before_fetch_not_after(mock_connect): + """The watermark records when the fetch *started* (minus the lookback + window), not when it finished. + + Otherwise a row modified while a long-running fetch is in flight would + fall in the gap between this pull's window and the next incremental + run's `since` — invisible until the next full_refresh heals it. + """ + mock_connect.return_value = MagicMock() + fetch_started_at = datetime(2026, 6, 1, 12, 0, 0, tzinfo=UTC) + + with freeze_time(fetch_started_at) as frozen_time: + + class _SlowTask(BaseWarehouseETLTask): + name = "test.SlowTask" + view_name = "ol_data_lake_production.ol_warehouse_production_integrations.integrations__learn__test" + + def fetch_and_upsert(self, conn, *, since=None) -> int: # noqa: ARG002 + # Simulate a fetch that takes real wall-clock time. + frozen_time.tick(timedelta(minutes=10)) + return 3 + + task = _SlowTask() + mock_cache = MagicMock() + mock_cache.get.return_value = None + + with patch("learning_resources.lib.warehouse.caches") as mock_caches: + mock_caches.__getitem__.return_value = mock_cache + task.run(full_refresh=False) + + stored_watermark = mock_cache.set.call_args.args[1] + assert stored_watermark == fetch_started_at - _WATERMARK_LOOKBACK + + +@patch("learning_resources.lib.warehouse.connect_to_warehouse") +def test_incremental_does_not_advance_watermark_on_failure(mock_connect): + """A failed incremental run leaves the watermark untouched (retry re-covers the gap).""" + mock_connect.return_value = MagicMock() + + class _FailingTask(BaseWarehouseETLTask): + name = "test.FailingTask" + view_name = "ol_data_lake_production.ol_warehouse_production_integrations.integrations__learn__test" + + def fetch_and_upsert(self, conn, *, since=None) -> int: # noqa: ARG002 + msg = "boom" + raise RuntimeError(msg) + + task = _FailingTask() + mock_cache = MagicMock() + mock_cache.get.return_value = None + + with ( + patch("learning_resources.lib.warehouse.caches") as mock_caches, + patch("learning_resources.lib.warehouse.sentry_sdk"), + ): + mock_caches.__getitem__.return_value = mock_cache + with pytest.raises(RuntimeError, match="boom"): + task.run(full_refresh=False) + + mock_cache.set.assert_not_called() diff --git a/learning_resources/migrations/0086_contentfile_run_or_resource_defined.py b/learning_resources/migrations/0086_contentfile_run_or_resource_defined.py index 401824ccb2..fa40c4807f 100644 --- a/learning_resources/migrations/0086_contentfile_run_or_resource_defined.py +++ b/learning_resources/migrations/0086_contentfile_run_or_resource_defined.py @@ -15,7 +15,7 @@ class Migration(migrations.Migration): migrations.AddConstraint( model_name="contentfile", constraint=models.CheckConstraint( - check=models.Q( + condition=models.Q( models.Q( ("learning_resource__isnull", False), ("run__isnull", True) ), diff --git a/learning_resources/migrations/0103_contentfilechanges.py b/learning_resources/migrations/0103_contentfilechanges.py index a1c642221d..1c6fdb4274 100644 --- a/learning_resources/migrations/0103_contentfilechanges.py +++ b/learning_resources/migrations/0103_contentfilechanges.py @@ -43,7 +43,7 @@ class Migration(migrations.Migration): migrations.AddConstraint( model_name="contentfile", constraint=models.CheckConstraint( - check=models.Q( + condition=models.Q( models.Q( ("learning_resource__isnull", False), ("run__isnull", True) ), diff --git a/learning_resources/models.py b/learning_resources/models.py index b23ba866c3..e97e6041e8 100644 --- a/learning_resources/models.py +++ b/learning_resources/models.py @@ -1278,7 +1278,7 @@ class Meta: # add constraint so that atleast run or learning_resource is defined (not both) constraints = [ models.CheckConstraint( - check=( + condition=( models.Q(learning_resource__isnull=False, run__isnull=True) | models.Q(run__isnull=False, learning_resource__isnull=True) | models.Q( diff --git a/learning_resources/tasks.py b/learning_resources/tasks.py index 4882fbe132..1c295555e7 100644 --- a/learning_resources/tasks.py +++ b/learning_resources/tasks.py @@ -58,7 +58,7 @@ from main.celery import app from main.constants import ISOFORMAT from main.decorators import cooldown_task -from main.utils import chunks, clear_views_cache, now_in_utc +from main.utils import chunks, now_in_utc log = logging.getLogger(__name__) @@ -75,7 +75,6 @@ def update_next_start_date_and_prices(): resource_upserted_actions( resource, percolate=False, generate_embeddings=True ) - clear_views_cache() return len(resources) @@ -104,7 +103,6 @@ def get_mit_edx_data( """ courses = pipelines.mit_edx_courses_etl(api_course_datafile) programs = pipelines.mit_edx_programs_etl(api_program_datafile) - clear_views_cache() return len(courses) + len(programs) @@ -114,7 +112,6 @@ def get_mitxonline_data() -> int | None: """Execute the MITX Online ETL pipeline""" courses = pipelines.mitxonline_courses_etl() programs = pipelines.mitxonline_programs_etl() - clear_views_cache() return len(courses) + len(programs) @@ -132,7 +129,6 @@ def get_oll_data(sheets_id=None) -> int | None: """ courses = pipelines.oll_etl(sheets_id) - clear_views_cache() return len(courses) @@ -156,7 +152,6 @@ def get_xpro_data() -> int | None: """Execute the xPro ETL pipeline""" courses = pipelines.xpro_courses_etl() programs = pipelines.xpro_programs_etl() - clear_views_cache() return len(courses) + len(programs) @@ -164,7 +159,6 @@ def get_xpro_data() -> int | None: def get_mit_climate_data(): """Execute the MIT Climate ETL pipeline""" articles = pipelines.mit_climate_etl() - clear_views_cache() return len(articles) @@ -187,7 +181,6 @@ def get_content_files( log.warning("Required settings missing for %s files", etl_source) return sync_edx_course_files(etl_source, ids, keys, overwrite=overwrite) - clear_views_cache() def get_content_tasks( @@ -327,7 +320,6 @@ def get_podcast_data(): The number of results that were fetched """ results = pipelines.podcast_etl() - clear_views_cache() return len(list(results)) @@ -341,7 +333,6 @@ def get_ovs_data(): The number of results that were fetched """ results = pipelines.ovs_etl() - clear_views_cache() return len(list(results)) @@ -368,7 +359,6 @@ def get_ocw_courses( start_timestamp=utc_start_timestamp, skip_content_files=skip_content_files, ) - clear_views_cache() @app.task(bind=True, acks_late=True) @@ -400,7 +390,6 @@ def update_ocw_learning_material_resources(self): # noqa: ARG001 f"Error loading learning materials for course run {course_run.id}: {e}" ) log.exception(error) - clear_views_cache() @app.task(bind=True, acks_late=True) @@ -623,7 +612,6 @@ def get_youtube_transcripts( log.info("Updating transcripts for %i videos", videos.count()) youtube.get_youtube_transcripts(videos) - clear_views_cache() @app.task(acks_late=True) @@ -640,7 +628,6 @@ def get_ovs_transcripts(*, overwrite=False): log.info("Updating OVS transcripts for %i videos", videos.count()) ovs.get_ovs_transcripts(videos) - clear_views_cache() @app.task(acks_late=True, reject_on_worker_lost=True) @@ -648,7 +635,6 @@ def get_learning_resource_views(): """Load learning resource views from the PostHog ETL.""" pipelines.posthog_etl() - clear_views_cache() @app.task(acks_late=True) diff --git a/learning_resources/tasks_test.py b/learning_resources/tasks_test.py index ac299def5e..d7844ccd8d 100644 --- a/learning_resources/tasks_test.py +++ b/learning_resources/tasks_test.py @@ -65,34 +65,6 @@ def mock_blocklist(mocker): ) -def test_cache_is_cleared_after_task_run(mocker, mocked_celery): - """Test that the search cache is cleared out after every task run""" - mocker.patch("learning_resources.tasks.ocw_courses_etl", autospec=True) - mocker.patch("learning_resources.tasks.get_content_tasks", autospec=True) - mocker.patch("learning_resources.tasks.pipelines") - mocked_clear_views_cache = mocker.patch( - "learning_resources.tasks.clear_views_cache" - ) - tasks.get_mit_edx_data.delay() - tasks.update_next_start_date_and_prices.delay() - tasks.get_mit_edx_data.delay() - tasks.get_mitxonline_data.delay() - tasks.get_oll_data.delay() - tasks.get_xpro_data.delay() - tasks.get_podcast_data.delay() - - tasks.get_ocw_courses.delay( - url_paths=[OCW_TEST_PREFIX], - force_overwrite=False, - skip_content_files=True, - ) - - # get_youtube_data is absent on purpose: it only queues the fan-out, whose - # writes land long after it returns, so it has nothing to invalidate - tasks.get_youtube_transcripts.delay() - assert mocked_clear_views_cache.call_count == 9 - - def test_get_mit_edx_data_valid(mocker): """Verify that the get_mit_edx_data invokes the MIT edX ETL pipelines""" mock_pipelines = mocker.patch("learning_resources.tasks.pipelines") diff --git a/learning_resources/utils.py b/learning_resources/utils.py index 3823b3fd27..7a5d8f8a9f 100644 --- a/learning_resources/utils.py +++ b/learning_resources/utils.py @@ -227,24 +227,6 @@ def load_course_blocklist(): return blocklist -def load_course_duplicates(etl_source: str) -> list: - """ - Get a list of blocklisted course ids for an ETL pipeline source - Args: - etl_source (string): the ETL source for which course duplicates are needed - Returns: - list of lists of courses which are duplicates of each other - """ - duplicates_url = settings.DUPLICATE_COURSES_URL - if duplicates_url is not None: - response = requests.get(duplicates_url, timeout=settings.REQUESTS_TIMEOUT) - response.raise_for_status() - duplicates_for_all_sources = yaml.safe_load(response.text) - if etl_source in duplicates_for_all_sources: - return duplicates_for_all_sources[etl_source] - return [] - - @retry( ClientError, tries=settings.MAX_S3_GET_ITERATIONS, delay=1, backoff=2, jitter=(1, 5) ) diff --git a/learning_resources/utils_test.py b/learning_resources/utils_test.py index f8b1f3124c..1afadfdc1a 100644 --- a/learning_resources/utils_test.py +++ b/learning_resources/utils_test.py @@ -211,41 +211,6 @@ def test_load_blocklist_cached(cached_ids, settings, mocker): mock_cache.set.assert_not_called() -@pytest.mark.parametrize("url", [None, "http://test.me"]) -@pytest.mark.parametrize("etl_source", ["mitx", "other"]) -def test_load_course_duplicates(url, etl_source, settings, mocker): - """Test that a list of duplicate course id sets is returned if a URL is set""" - settings.DUPLICATE_COURSES_URL = url - file_content = """ ---- -mitx: - - duplicate_course_ids: - - MITx+1 - - MITx+2 - - MITx+3 - course_id: MITx+1 -""" - - mock_request = mocker.patch( - "requests.get", autospec=True, return_value=mocker.Mock(text=file_content) - ) - duplicates = utils.load_course_duplicates(etl_source) - if url is None: - mock_request.assert_not_called() - assert duplicates == [] - elif etl_source == "other": - mock_request.assert_called_once_with(url, timeout=settings.REQUESTS_TIMEOUT) - assert duplicates == [] - else: - mock_request.assert_called_once_with(url, timeout=settings.REQUESTS_TIMEOUT) - assert duplicates == [ - { - "duplicate_course_ids": ["MITx+1", "MITx+2", "MITx+3"], - "course_id": "MITx+1", - } - ] - - def test_safe_load_bad_json(mocker): """Test that safe_load_json returns an empty dict for invalid JSON""" mock_logger = mocker.patch("learning_resources.utils.log.exception") diff --git a/learning_resources_search/tasks.py b/learning_resources_search/tasks.py index fa9cd2a2c2..5fa43d5d73 100644 --- a/learning_resources_search/tasks.py +++ b/learning_resources_search/tasks.py @@ -106,7 +106,6 @@ def update_featured_rank(): featured_resources.values_list("position", flat=True).distinct().count(), clear_all_greater_than=True, ) - clear_views_cache() @app.task(**PARTIAL_UPDATE_TASK_SETTINGS) @@ -1036,7 +1035,10 @@ def start_recreate_index(job_id): rate_limit=settings.CELERY_SEARCH_RATE_LIMIT, ) def finish_update_index(results): # noqa: ARG001 - """Clear cached views after update index tasks complete.""" + """ + Clear cached views after update index tasks complete. + """ + log.info("update_index has finished successfully!") clear_views_cache() diff --git a/learning_resources_search/tasks_test.py b/learning_resources_search/tasks_test.py index b63575de7a..d38853451f 100644 --- a/learning_resources_search/tasks_test.py +++ b/learning_resources_search/tasks_test.py @@ -1918,21 +1918,29 @@ def test_update_featured_rank(mocker, offeror_featured_lists): ) -def test_cache_clears_after_update_featured_rank(mocker, offeror_featured_lists): - """The updated_featured_rank task should make the expected calls""" - +def test_cache_is_cleared_after_update_index(mocker, settings): + """Test that the search cache is cleared out after an update of the index""" + settings.OPENSEARCH_INDEXING_CHUNK_SIZE = 2 + settings.OPENSEARCH_DOCUMENT_INDEXING_CHUNK_SIZE = 2 mocker.patch( - "learning_resources_search.tasks.random", - return_value=0.4, + "learning_resources_search.tasks.index_learning_resources", autospec=True + ) + mocker.patch( + "learning_resources_search.tasks.get_update_courses_tasks", autospec=True ) - - mocker.patch("learning_resources_search.tasks.api.clear_featured_rank") mocked_clear_views_cache = mocker.patch( "learning_resources_search.tasks.clear_views_cache" ) - mocker.patch("learning_resources_search.tasks.api.update_document_with_partial") + mocker.patch( + "learning_resources_search.tasks.load_course_blocklist", return_value=[] + ) + sorted( + CourseFactory.create_batch(4, etl_source=ETLSource.ocw.value), + key=lambda course: course.learning_resource_id, + ) - update_featured_rank() + with pytest.raises(Ignore): + start_update_index.run(["course"], None) assert mocked_clear_views_cache.call_count == 1 @@ -1960,29 +1968,3 @@ def test_cache_is_cleared_after_reindex(mocker): ) finish_reindex_job.delay(job.id) assert mocked_clear_views_cache.call_count == 1 - - -def test_cache_is_cleared_after_update_index(mocker, settings): - """Test that the search cache is cleared out after an update of the index""" - settings.OPENSEARCH_INDEXING_CHUNK_SIZE = 2 - settings.OPENSEARCH_DOCUMENT_INDEXING_CHUNK_SIZE = 2 - mocker.patch( - "learning_resources_search.tasks.index_learning_resources", autospec=True - ) - mocker.patch( - "learning_resources_search.tasks.get_update_courses_tasks", autospec=True - ) - mocked_clear_views_cache = mocker.patch( - "learning_resources_search.tasks.clear_views_cache" - ) - mocker.patch( - "learning_resources_search.tasks.load_course_blocklist", return_value=[] - ) - sorted( - CourseFactory.create_batch(4, etl_source=ETLSource.ocw.value), - key=lambda course: course.learning_resource_id, - ) - - with pytest.raises(Ignore): - start_update_index.run(["course"], None) - assert mocked_clear_views_cache.call_count == 1 diff --git a/main/features.py b/main/features.py index e42a84d48f..4f3964540c 100644 --- a/main/features.py +++ b/main/features.py @@ -29,7 +29,7 @@ def configure(): """ if settings.POSTHOG_PROJECT_API_KEY: posthog.default_client = posthog.Client( - api_key=settings.POSTHOG_PROJECT_API_KEY, + project_api_key=settings.POSTHOG_PROJECT_API_KEY, host=settings.POSTHOG_API_HOST, debug=settings.DEBUG, on_error=None, diff --git a/main/management/commands/clear_cache.py b/main/management/commands/clear_cache.py index 194ba05b1a..0400ae34d0 100644 --- a/main/management/commands/clear_cache.py +++ b/main/management/commands/clear_cache.py @@ -6,7 +6,13 @@ class Command(BaseCommand): - """Command to clear the cache""" + """ + Clear cached view responses. + + Cached responses are otherwise dropped on a schedule by + main.tasks.clear_views_cache (CLEAR_VIEWS_CACHE_SCHEDULE_SECONDS). Run this + to publish a manual change without waiting for the next scheduled clear. + """ help = "Command to clear the cache" diff --git a/main/middleware/apisix_user.py b/main/middleware/apisix_user.py index ab81232d4a..e4dc26746a 100644 --- a/main/middleware/apisix_user.py +++ b/main/middleware/apisix_user.py @@ -12,7 +12,6 @@ from django.core.exceptions import ObjectDoesNotExist from django.db.models import Q from django.http import HttpRequest -from django.http.response import HttpResponseBase from posthog import Posthog from authentication.api import user_created_actions @@ -209,8 +208,8 @@ def get_user_from_apisix_headers( # shared with other MIT applications that use their own integer # user ids, so integer ids collide across applications. posthog.capture( - global_id, event=PostHogEvents.ACCOUNT_CREATED.value, + distinct_id=global_id, properties={ "$current_url": request.build_absolute_uri(), "global_id": global_id, @@ -251,7 +250,7 @@ class ApisixUserMiddleware(RemoteUserMiddleware): header = "HTTP_X_USERINFO" - def process_request(self, request: HttpRequest) -> HttpResponseBase | None: + def process_request(self, request: HttpRequest) -> None: """ Modify the header to contain username, pass off to RemoteUserMiddleware """ @@ -278,7 +277,7 @@ def process_request(self, request: HttpRequest) -> HttpResponseBase | None: # Already logged in as this user: skip login() so we don't cycle # the session and write last_login on every request. request.user = apisix_user - return self.get_response(request) + return None if request.user.is_authenticated and request.user != apisix_user: # The user is authenticated, but doesn't match the user we got @@ -302,4 +301,4 @@ def process_request(self, request: HttpRequest) -> HttpResponseBase | None: log.debug("Forcing user logout because no APISIX user was found") logout(request) - return self.get_response(request) + return None diff --git a/main/middleware/apisix_user_test.py b/main/middleware/apisix_user_test.py index cb83a0f907..f7d6df630c 100644 --- a/main/middleware/apisix_user_test.py +++ b/main/middleware/apisix_user_test.py @@ -69,7 +69,9 @@ def test_get_request(mocker, mock_login, settings): """Test that a valid request creates a new user.""" close_old_connections() settings.POSTHOG_PROJECT_API_KEY = "fake-key" - mock_posthog_cls = mocker.patch("main.middleware.apisix_user.Posthog") + mock_posthog_cls = mocker.patch( + "main.middleware.apisix_user.Posthog", autospec=True + ) mock_request = mocker.Mock( META={ "HTTP_X_USERINFO": b64encode(json.dumps(apisix_user_info).encode()), @@ -88,8 +90,8 @@ def test_get_request(mocker, mock_login, settings): assert user.global_id == apisix_user_info["sub"] mock_posthog_cls.assert_called_once() mock_posthog_cls.return_value.capture.assert_called_once_with( - apisix_user_info["sub"], event=PostHogEvents.ACCOUNT_CREATED.value, + distinct_id=apisix_user_info["sub"], properties=mocker.ANY, ) @@ -99,7 +101,9 @@ def test_get_request_no_posthog_key(mocker, mock_login, settings): """Test that PostHog is not called when POSTHOG_PROJECT_API_KEY is empty.""" close_old_connections() settings.POSTHOG_PROJECT_API_KEY = "" - mock_posthog_cls = mocker.patch("main.middleware.apisix_user.Posthog") + mock_posthog_cls = mocker.patch( + "main.middleware.apisix_user.Posthog", autospec=True + ) mock_request = mocker.Mock( META={ "HTTP_X_USERINFO": b64encode(json.dumps(apisix_user_info).encode()), @@ -170,7 +174,7 @@ def test_get_request_ambiguous_identity_fails_closed(mocker, mock_login): mock_get_response = mocker.Mock(return_value="response") apisix_middleware = ApisixUserMiddleware(mock_get_response) - assert apisix_middleware.process_request(mock_request) == "response" + assert apisix_middleware(mock_request) == "response" mock_login.assert_not_called() mock_get_response.assert_called_once_with(mock_request) @@ -199,9 +203,11 @@ def test_get_request_different_user_logout(mocker, client, same_user): ) mocker.patch("main.middleware.apisix_user.login") mock_logout = mocker.patch("main.middleware.apisix_user.logout") - apisix_middleware = ApisixUserMiddleware(mocker.Mock()) - apisix_middleware.process_request(mock_request) + mock_get_response = mocker.Mock(return_value="response") + apisix_middleware = ApisixUserMiddleware(mock_get_response) + assert apisix_middleware(mock_request) == "response" assert mock_logout.call_count == (0 if same_user else 1) + mock_get_response.assert_called_once_with(mock_request) @pytest.mark.django_db(transaction=True) diff --git a/main/settings.py b/main/settings.py index c6d6de4850..0e44cdf779 100644 --- a/main/settings.py +++ b/main/settings.py @@ -36,7 +36,7 @@ from main.settings_pluggy import * # noqa: F403 from openapi.settings_spectacular import open_spectacular_settings -VERSION = "0.77.10" +VERSION = "0.77.11" log = logging.getLogger() @@ -305,8 +305,6 @@ USE_I18N = True -USE_L10N = True - USE_TZ = True AUTHENTICATION_BACKENDS = ( @@ -873,6 +871,20 @@ def get_all_config_keys(): # hard limit for special cases where we need to return all results without pagination VECTOR_SEARCH_PAGE_MAX_LIMIT = get_int("VECTOR_SEARCH_PAGE_MAX_LIMIT", 200) +# Score subtracted from a completeness = 0 resource in vector search, scaled +# linearly by incompleteness. 0 disables the penalty. +# +# In *score units*, not the percent DEFAULT_SEARCH_MAX_INCOMPLETENESS_PENALTY +# uses on the OpenSearch side. The OpenSearch penalty is multiplicative, which +# works there because BM25 is unbounded and an exact match scores multiples of a +# topical one. Similarity scores are bounded and sit in a narrow band (~0.55-0.75 +# across a whole result page), so scaling them by completeness makes completeness +# the primary sort key and buries exact matches on incomplete courses. Subtracting +# a fixed budget demotes them without erasing the relevance signal. +VECTOR_SEARCH_INCOMPLETENESS_PENALTY_WEIGHT = get_float( + name="VECTOR_SEARCH_INCOMPLETENESS_PENALTY_WEIGHT", default=0.05 +) + # serve learning resource search hits from the Qdrant payload instead of # re-hydrating them from the database. Set to False to fall back to database # hydration without a deploy. @@ -936,10 +948,11 @@ def get_all_config_keys(): ), ) # OpenTelemetry configuration (consumed by mitol-django-observability). -# Tracing turns on when either OPENTELEMETRY_ENDPOINT or the -# OTEL_EXPORTER_OTLP_ENDPOINT environment variable is set -- those two and no -# others; the signal-specific OTEL_EXPORTER_OTLP_TRACES_ENDPOINT is not -# consulted by the released library. There is no flag to disable it. +# Telemetry turns on when any of OTEL_EXPORTER_OTLP_TRACES_ENDPOINT, +# OTEL_EXPORTER_OTLP_METRICS_ENDPOINT or OTEL_EXPORTER_OTLP_ENDPOINT is set in +# the environment, or the OPENTELEMETRY_ENDPOINT setting below is. Per signal +# the environment is read most-specific-first, and the setting applies only +# when the environment supplies nothing. There is no flag to disable it. OPENTELEMETRY_SERVICE_NAME = get_string("OPENTELEMETRY_SERVICE_NAME", "learn") OPENTELEMETRY_INSECURE = get_bool("OPENTELEMETRY_INSECURE", default=True) OPENTELEMETRY_ENDPOINT = get_string("OPENTELEMETRY_ENDPOINT", None) diff --git a/main/settings_celery.py b/main/settings_celery.py index 5dfe716109..c7f6331677 100644 --- a/main/settings_celery.py +++ b/main/settings_celery.py @@ -3,6 +3,7 @@ """ from celery.schedules import crontab +from django.core.exceptions import ImproperlyConfigured from redbeat import RedBeatScheduler from main.envs import get_bool, get_int, get_string @@ -168,6 +169,16 @@ "task": "main.tasks.delete_old_task_jobs", "schedule": crontab(minute=0, hour=8), # 4:00am EST }, + "clear-views-cache": { + "task": "main.tasks.clear_views_cache", + # This, not REDIS_VIEW_CACHE_DURATION, is the effective lifetime of + # a cached view response: it bounds how stale ETL and search index + # changes can look to users. Lengthen it for more cache hits and + # less rendering load, shorten it for fresher responses. + "schedule": get_int( + "CLEAR_VIEWS_CACHE_SCHEDULE_SECONDS", 60 * 60 + ), # default is every hour + }, "scrape-marketing-pages-every-1-days": { "task": "learning_resources.tasks.scrape_marketing_pages", "schedule": get_int( @@ -210,6 +221,47 @@ } ) +# Per-source cutover switch for warehouse-pull catalog ETL +# (learning_resources.tasks.Sync*Task, StarRocks-backed — see +# learning_resources.lib.warehouse.BaseWarehouseETLTask). Each stacked PR +# that adds a source registers its beat entry above and adds itself to +# _API_ETL_BEAT_ENTRIES_BY_SOURCE below; none are wired up yet. +# +# Deliberately *not* keyed on STARROCKS_HOST: during the parallel-validation +# window for each source both pipelines must run so their outputs can be +# compared. Cutover is a separate, per-source decision made once a source +# clears validation, so it gets its own setting — a comma-separated list of +# ETLSource names (e.g. "mitxonline,xpro"). +# +# Only the catalog-metadata tasks belong here. The `import_all_*_files` +# tasks stay scheduled regardless of cutover: the integrations__learn__* +# views carry course/program metadata only, not content files. +_API_ETL_BEAT_ENTRIES_BY_SOURCE: dict[str, tuple[str, ...]] = {} + +WAREHOUSE_ETL_CUTOVER_SOURCES = [ + source.strip() + for source in get_string("WAREHOUSE_ETL_CUTOVER_SOURCES", "").split(",") + if source.strip() +] + +_unknown_cutover_sources = sorted( + set(WAREHOUSE_ETL_CUTOVER_SOURCES) - set(_API_ETL_BEAT_ENTRIES_BY_SOURCE) +) +if _unknown_cutover_sources: + # Fail loud rather than silently leaving a legacy task scheduled: a + # typo here would mean both pipelines keep writing the same rows long + # after the source was believed to be cut over. + msg = ( + f"WAREHOUSE_ETL_CUTOVER_SOURCES contains unrecognized source(s): " + f"{', '.join(_unknown_cutover_sources)}. " + f"Valid values: {', '.join(sorted(_API_ETL_BEAT_ENTRIES_BY_SOURCE))}" + ) + raise ImproperlyConfigured(msg) + +for _source in WAREHOUSE_ETL_CUTOVER_SOURCES: + for _beat_entry in _API_ETL_BEAT_ENTRIES_BY_SOURCE[_source]: + CELERY_BEAT_SCHEDULE.pop(_beat_entry, None) + CELERY_TASK_SERIALIZER = "json" CELERY_RESULT_SERIALIZER = "json" CELERY_ACCEPT_CONTENT = ["json"] diff --git a/main/settings_course_etl.py b/main/settings_course_etl.py index ddd31cd9f2..cf616ff73c 100644 --- a/main/settings_course_etl.py +++ b/main/settings_course_etl.py @@ -31,7 +31,6 @@ "BLOCKLISTED_COURSES_URL", "https://raw.githubusercontent.com/mitodl/open-resource-blocklists/master/courses.txt", ) -DUPLICATE_COURSES_URL = get_string("DUPLICATE_COURSES_URL", None) # Iterator chunk size for MITx and xPRO courses @@ -152,3 +151,15 @@ "CONTENT_BASE_URL_OLL", "https://openlearninglibrary.mit.edu" ) CONTENT_BASE_URL_EDX = get_string("CONTENT_BASE_URL_EDX", "https://courses.edx.org") + +# Warehouse-pull settings for Cohort 1 catalog ETL, used to query the +# integrations schema views exposed by the OL Data Platform. WAREHOUSE_BACKEND +# selects the connector without touching ETL task code (see +# learning_resources.lib.warehouse). Backend-specific credentials are +# namespaced (e.g. STARROCKS_*) so future backends (e.g. a DuckDB-based +# local/offline mode) can sit alongside without colliding. +WAREHOUSE_BACKEND = get_string("WAREHOUSE_BACKEND", "starrocks") +STARROCKS_HOST = get_string("STARROCKS_HOST", None) +STARROCKS_PORT = get_int("STARROCKS_PORT", 9030) +STARROCKS_USER = get_string("STARROCKS_USER", None) +STARROCKS_PASSWORD = get_string("STARROCKS_PASSWORD", None) diff --git a/main/settings_test.py b/main/settings_test.py index 65a1f830a3..7bbe49047e 100644 --- a/main/settings_test.py +++ b/main/settings_test.py @@ -306,6 +306,30 @@ def test_celery_result_expires_override(self): settings_vars = self.reload_settings(module="main.settings_celery") assert settings_vars["CELERY_RESULT_EXPIRES"] == 120 + def test_warehouse_etl_cutover_sources_empty_by_default(self): + """No warehouse-pull sources are cut over by default.""" + with mock.patch.dict("os.environ", REQUIRED_SETTINGS, clear=True): + settings_vars = self.reload_settings(module="main.settings_celery") + assert settings_vars["WAREHOUSE_ETL_CUTOVER_SOURCES"] == [] + + def test_warehouse_etl_cutover_sources_rejects_unknown_source(self): + """A typo'd source name fails loudly rather than silently no-op'ing — + _API_ETL_BEAT_ENTRIES_BY_SOURCE starts empty (no source has landed a + warehouse-pull task yet), so any non-empty value is "unknown" today. + """ + with ( + mock.patch.dict( + "os.environ", + { + **REQUIRED_SETTINGS, + "WAREHOUSE_ETL_CUTOVER_SOURCES": "mitxonline", + }, + clear=True, + ), + pytest.raises(ImproperlyConfigured, match="mitxonline"), + ): + self.reload_settings(module="main.settings_celery") + def _assert_s3_storage_config( self, storages_dict, diff --git a/main/tasks.py b/main/tasks.py index 90183524fc..ddafd0f0b6 100644 --- a/main/tasks.py +++ b/main/tasks.py @@ -5,6 +5,7 @@ from django.conf import settings +from main import utils from main.celery import app from main.models import TaskBatch, TaskJob from main.utils import now_in_utc @@ -43,3 +44,13 @@ def delete_old_task_jobs(): threshold = now_in_utc() - datetime.timedelta(days=settings.TASK_JOB_RETENTION_DAYS) deleted, _ = TaskJob.objects.filter(updated_on__lt=threshold).delete() log.info("Deleted %d old task job/batch rows", deleted) + + +@app.task +def clear_views_cache(): + """ + Drop cached view responses + """ + cleared = utils.clear_views_cache() + log.info("Cleared %d cached view responses", cleared) + return cleared diff --git a/main/tasks_test.py b/main/tasks_test.py index 340d75cc8b..1911cee2b7 100644 --- a/main/tasks_test.py +++ b/main/tasks_test.py @@ -7,7 +7,7 @@ from main.factories import TaskBatchFactory, TaskJobFactory from main.models import TaskBatch, TaskJob -from main.tasks import delete_old_task_jobs, maybe_finish_task_job +from main.tasks import clear_views_cache, delete_old_task_jobs, maybe_finish_task_job from main.utils import now_in_utc pytestmark = pytest.mark.django_db @@ -53,3 +53,11 @@ def test_delete_old_task_jobs(): assert not TaskBatch.objects.filter(id=old_batch.id).exists() # cascaded assert TaskJob.objects.filter(id=recent_job.id).exists() assert TaskBatch.objects.filter(id=recent_batch.id).exists() + + +def test_clear_views_cache(mocker): + """The scheduled task delegates to the cache-clearing util""" + mock_clear = mocker.patch("main.tasks.utils.clear_views_cache", return_value=7) + + assert clear_views_cache.delay().get() == 7 + mock_clear.assert_called_once_with() diff --git a/news_events/etl/mitpe_events_test.py b/news_events/etl/mitpe_events_test.py index 48f4e75682..2206465f05 100644 --- a/news_events/etl/mitpe_events_test.py +++ b/news_events/etl/mitpe_events_test.py @@ -5,6 +5,7 @@ from pathlib import Path import pytest +from freezegun import freeze_time from news_events.etl.mitpe_events import extract, transform @@ -28,7 +29,7 @@ def test_extract(mitpe_events_json_data): assert extract() == mitpe_events_json_data -@pytest.mark.freeze_time("2020-05-21") +@freeze_time("2020-05-21") def test_transform(mitpe_events_json_data): """Assert that the transform function returns the expected data""" source_and_items = transform(extract()) diff --git a/news_events/tasks.py b/news_events/tasks.py index 92a9077768..f75fff3545 100644 --- a/news_events/tasks.py +++ b/news_events/tasks.py @@ -1,7 +1,6 @@ """Tasks for news_events""" from main.celery import app -from main.utils import clear_views_cache from news_events.etl import pipelines @@ -9,42 +8,36 @@ def get_medium_mit_news(): """Run the Medium MIT News ETL pipeline""" pipelines.medium_mit_news_etl() - clear_views_cache() @app.task(acks_late=True, reject_on_worker_lost=True) def get_ol_events(): """Run the Open Learning Events ETL pipeline""" pipelines.ol_events_etl() - clear_views_cache() @app.task def get_sloan_exec_news(): """Run the Sloan executive education news ETL pipeline""" pipelines.sloan_exec_news_etl() - clear_views_cache() @app.task def get_sloan_exec_webinars(): """Run the Sloan webinars ETL pipeline""" pipelines.sloan_webinars_etl() - clear_views_cache() @app.task(acks_late=True, reject_on_worker_lost=True) def get_mitpe_news(): """Run the MIT Professional Education news ETL pipeline""" pipelines.mitpe_news_etl() - clear_views_cache() @app.task(acks_late=True, reject_on_worker_lost=True) def get_mitpe_events(): """Run the MIT Professional Education events ETL pipeline""" pipelines.mitpe_events_etl() - clear_views_cache() @app.task(acks_late=True, reject_on_worker_lost=True) @@ -52,14 +45,12 @@ def get_website_content_news(): """Run the website content news ETL pipeline""" pipelines.articles_news_etl() - clear_views_cache() @app.task(name="news_events.tasks.get_articles_news") def get_articles_news(): """Backward-compatible alias for get_website_content_news.""" pipelines.articles_news_etl() - clear_views_cache() @app.task( @@ -90,7 +81,6 @@ def sync_website_content_to_news(self, content_id: int): sync_single_website_content_news_to_news(content) - clear_views_cache() logger.info( "Successfully synced content %s to news feed", content_id, diff --git a/news_events/tasks_test.py b/news_events/tasks_test.py index 1e28045f2b..223a5a3575 100644 --- a/news_events/tasks_test.py +++ b/news_events/tasks_test.py @@ -69,14 +69,10 @@ def test_sync_article_to_news_success(mocker, user): "news_events.etl.articles_news.sync_single_website_content_news_to_news", autospec=True, ) - mock_clear_cache = mocker.patch( - "news_events.tasks.clear_views_cache", autospec=True - ) tasks.sync_website_content_to_news(content.id) mock_sync.assert_called_once_with(content) - mock_clear_cache.assert_called_once() @pytest.mark.django_db @@ -86,14 +82,10 @@ def test_sync_article_to_news_article_not_found(mocker, caplog): "news_events.etl.articles_news.sync_single_website_content_news_to_news", autospec=True, ) - mock_clear_cache = mocker.patch( - "news_events.tasks.clear_views_cache", autospec=True - ) tasks.sync_website_content_to_news(99999) mock_sync.assert_not_called() - mock_clear_cache.assert_not_called() assert "WebsiteContent 99999 not found or not published" in caplog.text @@ -115,14 +107,10 @@ def test_sync_article_to_news_unpublished_article(mocker, user, caplog): "news_events.etl.articles_news.sync_single_website_content_news_to_news", autospec=True, ) - mock_clear_cache = mocker.patch( - "news_events.tasks.clear_views_cache", autospec=True - ) tasks.sync_website_content_to_news(content.id) mock_sync.assert_not_called() - mock_clear_cache.assert_not_called() assert f"WebsiteContent {content.id} not found or not published" in caplog.text @@ -145,12 +133,8 @@ def test_sync_article_to_news_sync_failure(mocker, user): autospec=True, side_effect=Exception("Sync failed"), ) - mock_clear_cache = mocker.patch( - "news_events.tasks.clear_views_cache", autospec=True - ) with pytest.raises(Exception, match="Sync failed"): tasks.sync_website_content_to_news(content.id) mock_sync.assert_called_once_with(content) - mock_clear_cache.assert_not_called() diff --git a/pyproject.toml b/pyproject.toml index 2ba9fd2dfc..b8be0da718 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -11,7 +11,7 @@ classifiers = [ "Programming Language :: Python :: 3.12", ] dependencies = [ - "Django==4.2.30", + "Django==5.2.16", "attrs>=25.0.0,<26", "base36>=0.1.1,<0.2", "beautifulsoup4>=4.8.2,<5", @@ -30,7 +30,7 @@ dependencies = [ "django-bitfield>=2.2.0,<3", "django-cache-memoize>=0.2.0,<0.3", "django-cors-headers>=4.0.0,<5", - "django-filter>=2.4.0,<3", + "django-filter>=26.1", "django-guardian>=3.0.0,<4", "django-health-check>=3.24.0,<4", "django-imagekit>=6.0.0,<7", @@ -57,13 +57,13 @@ dependencies = [ "langchain>=1.3.9,<1.4", "litellm==1.84.0", "llama-index>=0.14.0,<0.15", - "llama-index-llms-openai>=0.6.0,<0.7", + "llama-index-llms-openai>=0.7.10,<0.8", "lxml>=6.0.0,<7", "markdown>=3.7,<4", "markdown2>=2.4.8,<3", "mitol-django-common>=2026.4.2,<2027", "mitol-django-scim>=2026.4.2,<2027", - "mitol-django-observability>=2026.1.0,<2027", + "mitol-django-observability>=2026.8.19,<2027", "named-enum>=1.4.0,<2", "nested-lookup>=0.2.25,<0.3", "nh3>=0.3.0,<0.4", @@ -81,7 +81,7 @@ dependencies = [ "opentelemetry-instrumentation-redis>=0.52b0", "opentelemetry-instrumentation-requests>=0.52b0", "pluggy>=1.3.0,<2", - "posthog>=5.0.0,<6", + "posthog>=7.38.2,<8", "psycopg[c]>=3.2.4,<4", "pycountry>=24.6.1,<25", "pygithub>=2.0.0,<3", @@ -114,6 +114,7 @@ dependencies = [ "urllib3>=2.0.0,<3", "wrapt>=1.14.1,<2", "youtube-transcript-api>=1.0.0,<1.1", + "pypdf>=6.9.1,<7", "pypdfium2>=5.0.0,<6", "pyarrow>=24,<25", "django-zeal>=2.0.4,<3", @@ -128,6 +129,7 @@ dependencies = [ "langchain-community>=0.4.2", "langchain-text-splitters>=1.1.2", "mitol-django-keycloak>=2026.7.13,<2027", + "pymysql>=1.1.1,<2", ] [dependency-groups] @@ -144,7 +146,6 @@ dev = [ "pytest-cov>=7.0.0,<8", "pytest-django>=4.5.2,<5", "pytest-env>=1.0.0,<2", - "pytest-freezegun>=0.4.2,<0.5", "pytest-mock>=3.10.0,<4", "pytest-repeat>=0.9.4", "responses>=0.25.0,<0.26", diff --git a/renovate.json b/renovate.json index b20aea85bc..9d25ea524d 100644 --- a/renovate.json +++ b/renovate.json @@ -3,7 +3,7 @@ "extends": ["local>mitodl/.github:renovate-config"], "packageRules": [ { - "allowedVersions": "<5", + "allowedVersions": "<6", "matchPackageNames": ["django", "Django"] } ] diff --git a/uv.lock b/uv.lock index 1acf9c2102..2510d03616 100644 --- a/uv.lock +++ b/uv.lock @@ -765,16 +765,16 @@ sdist = { url = "https://files.pythonhosted.org/packages/2b/8f/77a4b8ec50c821193 [[package]] name = "django" -version = "4.2.30" +version = "5.2.16" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "asgiref" }, { name = "sqlparse" }, { name = "tzdata", marker = "sys_platform == 'win32'" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/11/b5/f1a53dc68da6429d6e0345bb848161e2381a2e9f02700148911e8582c2b3/django-4.2.30.tar.gz", hash = "sha256:4ebc7a434e3819db6cf4b399fb5b3f536310a30e8486f08b66886840be84b37c", size = 10468707, upload-time = "2026-04-07T14:05:45.57Z" } +sdist = { url = "https://files.pythonhosted.org/packages/a9/26/889449d521ae508b26de715954faecd8bcf3f740affb81b2d146a83b42a5/django-5.2.16.tar.gz", hash = "sha256:59ea02020c3136fce14bef0bbece21a10a4febef5eed1c51c22ae468efa22200", size = 10890894, upload-time = "2026-07-07T13:52:17.005Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/39/b7/a7c96f239cf91313a6589233fed55111c7063b26683b226802732c455dbc/django-4.2.30-py3-none-any.whl", hash = "sha256:4d07aaf1c62f9984842b67c2874ebbf7056a17be253860299b93ae1881faad65", size = 7997231, upload-time = "2026-04-07T14:05:38.241Z" }, + { url = "https://files.pythonhosted.org/packages/4e/13/1e5e3e4c15dcecb04281b3cb2a46a4670e1cef131068e202f6040df19224/django-5.2.16-py3-none-any.whl", hash = "sha256:04f354bf9d807a86ad1a8392fe3808d362358a8eafc322848e0e43e59b24371d", size = 8311943, upload-time = "2026-07-07T13:52:11.223Z" }, ] [[package]] @@ -837,14 +837,14 @@ wheels = [ [[package]] name = "django-filter" -version = "2.4.0" +version = "26.1" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "django" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/7b/cf/adae3e55995ea27e1dceb493e0226557d4207d8819ddb99591df5204a471/django-filter-2.4.0.tar.gz", hash = "sha256:84e9d5bb93f237e451db814ed422a3a625751cbc9968b484ecc74964a8696b06", size = 146904, upload-time = "2020-09-27T09:08:58.079Z" } +sdist = { url = "https://files.pythonhosted.org/packages/cb/3e/563965173d4cbb5fc308087e7b3d11a115b7b67273d093622480b1e31f78/django_filter-26.1.tar.gz", hash = "sha256:66ea04031b068c77c86e1ac26ced7a3f8f13ce797f5795751707e3deefc58054", size = 144299, upload-time = "2026-07-11T09:27:02.767Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/71/2b/b2fe483c3095b6222725dd05f9ad9e6ed6cb7347c154fdbd80238d36f1a8/django_filter-2.4.0-py3-none-any.whl", hash = "sha256:e00d32cebdb3d54273c48f4f878f898dced8d5dfaad009438fe61ebdf535ace1", size = 73156, upload-time = "2020-09-27T09:08:52.69Z" }, + { url = "https://files.pythonhosted.org/packages/1f/01/afffed1e3c4540fb75bf550a18b6176a9f6371b5f3e52b69a28995b6480c/django_filter-26.1-py3-none-any.whl", hash = "sha256:7d98ef2899218e6242619b532cb1b95af14e09dfcf74844aecb550ad27b59ff2", size = 94069, upload-time = "2026-07-11T09:27:01.012Z" }, ] [[package]] @@ -1033,14 +1033,14 @@ wheels = [ [[package]] name = "djangorestframework" -version = "3.17.0" +version = "3.17.2" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "django" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/b1/46/615ad5425c7b941e6fd3f382d25b1aca680022200d8e5b87bfc665c4425d/djangorestframework-3.17.0.tar.gz", hash = "sha256:456fd992a33f9e64c9d0f47e85d9787db0efb44f894c1e513315b5e74765bd4c", size = 905671, upload-time = "2026-03-18T20:33:42.861Z" } +sdist = { url = "https://files.pythonhosted.org/packages/3b/35/c96055e700fdff25da3a7b7756cfd1d4dc54f38b9bc6d6c5e19e3a0fdc20/djangorestframework-3.17.2.tar.gz", hash = "sha256:89ed713b6dc83e1539f214b7d10808ae19bb8511004beba886225da6d5c9dafa", size = 906683, upload-time = "2026-08-05T07:47:22.5Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/5f/ee/01dfe0e712d37007a4417bf7ba518a9e6830d4de009ed4bfdfebd45b99cf/djangorestframework-3.17.0-py3-none-any.whl", hash = "sha256:d84fe85f30b7ac6e8c0076ce9ff635e4eaedca5912f8d7d2926ce448c08533ba", size = 898818, upload-time = "2026-03-18T20:33:41.335Z" }, + { url = "https://files.pythonhosted.org/packages/a2/46/c14108e400b208c394325eb63fbae06c81341b6447fa1a6f9da718b17fe7/djangorestframework-3.17.2-py3-none-any.whl", hash = "sha256:cb0546a7415d5b46c04e0f4fe0a54b2109f4fdd5e83ca773c8c6183a6493d042", size = 899109, upload-time = "2026-08-05T07:47:20.853Z" }, ] [[package]] @@ -2228,59 +2228,24 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/01/a6/77fa1bbf5e42eb596b06318b3f7e6af5d0f44028046d1d598c6a595d028f/litellm-1.84.0-py3-none-any.whl", hash = "sha256:2a58d6041e6aa27d1a28dc8d8828ab500fef1a00ef74ca65e60899035010c2f2", size = 16735062, upload-time = "2026-05-14T05:45:49.927Z" }, ] -[[package]] -name = "llama-cloud" -version = "1.6.0" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "anyio" }, - { name = "distro" }, - { name = "httpx" }, - { name = "pydantic" }, - { name = "sniffio" }, - { name = "typing-extensions" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/15/bc/72256200ae875bfb352a1043a6008472829517e1bff4e3b1311639def2f8/llama_cloud-1.6.0.tar.gz", hash = "sha256:b00c75df76b59becca72f262c755a59529f0c09f0cda79e086eedefc62d59ac8", size = 2274742, upload-time = "2026-03-05T23:55:34.405Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/e2/e1/c40aff3fe70c6b457ee89f6cdaee8678fb63986b12734e7d6e4fc481d6dc/llama_cloud-1.6.0-py3-none-any.whl", hash = "sha256:3b880587ef82f23dc9f1998395b31f8a4afd77eb88f99689358a67375d16d413", size = 394869, upload-time = "2026-03-05T23:55:33.172Z" }, -] - [[package]] name = "llama-index" -version = "0.14.16" +version = "0.14.23" source = { registry = "https://pypi.org/simple" } dependencies = [ - { name = "llama-index-cli" }, { name = "llama-index-core" }, { name = "llama-index-embeddings-openai" }, - { name = "llama-index-indices-managed-llama-cloud" }, { name = "llama-index-llms-openai" }, - { name = "llama-index-readers-file" }, - { name = "llama-index-readers-llama-parse" }, { name = "nltk" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/e5/43/440cbd852b9372fd392cc81f72df75f17d6dfbe93a427c5911a3400ea168/llama_index-0.14.16.tar.gz", hash = "sha256:266c9b066f2eaee584188bbdb440ed4fd9ad41694c6c9c55c5f15e55eb9dcbc2", size = 9048, upload-time = "2026-03-10T19:20:29.96Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/50/98/12ff971b3b5f4f82153adb7f5528a276f42aafcdd6193392c43998dd32de/llama_index-0.14.16-py3-none-any.whl", hash = "sha256:cb98fece42d485f52ca847d3d16af61984fdeb7f4c0793a069357ac6eb8293ce", size = 7847, upload-time = "2026-03-10T19:20:31.151Z" }, -] - -[[package]] -name = "llama-index-cli" -version = "0.5.5" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "llama-index-core" }, - { name = "llama-index-embeddings-openai" }, - { name = "llama-index-llms-openai" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/3e/66/90747a02fa9f4e9503da40259d18105f75a02b3f3b6b722faf0502d8b40d/llama_index_cli-0.5.5.tar.gz", hash = "sha256:a2de5a22f675f60908c8cd1fd873f132cf2bfdf3462fa79ef5fbe6b95727a30b", size = 24852, upload-time = "2026-03-04T23:00:55.646Z" } +sdist = { url = "https://files.pythonhosted.org/packages/cb/5c/1f9334c3c0edd880367d0734939256f8536ca94165c6a5fdb455a7bcf180/llama_index-0.14.23.tar.gz", hash = "sha256:eac2049816a7410ff4568490cce4bdff99cda3ab99d59f52f6227dad22cda44b", size = 8566, upload-time = "2026-06-24T19:36:38.241Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/fc/ef/ca63ce9ae26de1c64fcf1876d67ee7996cecf6127f43a49c1e4a485d806c/llama_index_cli-0.5.5-py3-none-any.whl", hash = "sha256:ac041aa61c2e194266a07fea617500a063f389af7dd6ae02f8cd3f1f7644d06d", size = 28210, upload-time = "2026-03-04T23:00:54.696Z" }, + { url = "https://files.pythonhosted.org/packages/e0/92/da2a737bf712fe31c1aaec00f51745797d4570ecdf7e4f5d2b4b93b3d337/llama_index-0.14.23-py3-none-any.whl", hash = "sha256:c205de2442a7186b8e05096f0771f96fa6bdc9603fcad2e07eab1bc96dcf086a", size = 7114, upload-time = "2026-06-24T19:36:37.21Z" }, ] [[package]] name = "llama-index-core" -version = "0.14.18" +version = "0.14.23" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "aiohttp" }, @@ -2312,35 +2277,22 @@ dependencies = [ { name = "typing-inspect" }, { name = "wrapt" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/9d/ab/d864b2b9ee2eeb5f41c31f05f398cfc5cbb7f9a2eb801dfd1a811d399805/llama_index_core-0.14.18.tar.gz", hash = "sha256:5bd5153ac3f097576a42bb778977b35a9f6184b91b47919cf3bb03616ed31591", size = 11599864, upload-time = "2026-03-16T19:41:22.806Z" } +sdist = { url = "https://files.pythonhosted.org/packages/5c/ac/f885ae14317af43a026c909ea4d2083fcee2f0d014f90426b5b9aa1f9912/llama_index_core-0.14.23.tar.gz", hash = "sha256:c4baf2f2ab4f84e95090fe7941e0c87d6c514304f7bd2a749b8fa22164c1822b", size = 11588373, upload-time = "2026-06-24T19:35:55.43Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/27/33/64a32bc376390340a05f3bcea0594dc6384c77a8298569e851add75ff9bb/llama_index_core-0.14.18-py3-none-any.whl", hash = "sha256:584e5995929d129036cf8fe2d1b8f0d581e43335760f2f6083cd767a71f47f8d", size = 11945592, upload-time = "2026-03-16T19:41:19.427Z" }, + { url = "https://files.pythonhosted.org/packages/ec/d5/05d61f34c01c6578fb758d0a3ddef58d36c6ffa9a9f84a5c9a16262ad94d/llama_index_core-0.14.23-py3-none-any.whl", hash = "sha256:6a54d267826732a8507f81df40785b107f7592af20f451a39a59005147caf84c", size = 11924908, upload-time = "2026-06-24T19:35:52.833Z" }, ] [[package]] name = "llama-index-embeddings-openai" -version = "0.5.2" +version = "0.6.0" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "llama-index-core" }, { name = "openai" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/ea/a1/d238dfa453ba8ebc4f6261d6384b663f50b8dba6f4b22d8be800b305863d/llama_index_embeddings_openai-0.5.2.tar.gz", hash = "sha256:091bd0c3e9182748e8827de7d79713a219d5f5e0dc97d1bb7b271cf524520e4b", size = 7630, upload-time = "2026-03-03T11:27:38.127Z" } +sdist = { url = "https://files.pythonhosted.org/packages/06/52/eb56a4887501651fb17400f7f571c1878109ff698efbe0bbac9165a5603d/llama_index_embeddings_openai-0.6.0.tar.gz", hash = "sha256:eb3e6606be81cb89125073e23c97c0a6119dabb4827adbd14697c2029ad73f29", size = 7629, upload-time = "2026-03-12T20:21:27.234Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/1f/5e/da156f9c77443d22287eeaea341fe35fdcc25e59a9250e4cb10d4d5a066a/llama_index_embeddings_openai-0.5.2-py3-none-any.whl", hash = "sha256:37e7967de05b05f16c9b171091110bb1c6e5a0720198ea306d57cd3920cb81b7", size = 7667, upload-time = "2026-03-03T11:27:37.394Z" }, -] - -[[package]] -name = "llama-index-indices-managed-llama-cloud" -version = "0.11.0" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "llama-cloud" }, - { name = "llama-index-core" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/75/83/94407c7baf5a28602fec7f081be1b1b8c0fb19edd1b3fc7a8592d59a9c56/llama_index_indices_managed_llama_cloud-0.11.0.tar.gz", hash = "sha256:b4e685ea34e279510dd4b9d5bf1a344762beac300f1294a217366fa16f3a47bb", size = 4397, upload-time = "2026-03-17T14:38:28.708Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/47/02/5d04cd99c395d23f4ca7cfab4577e21af3c2edddf0e91906adf362804fe8/llama_index_indices_managed_llama_cloud-0.11.0-py3-none-any.whl", hash = "sha256:84b7cd4c01a4c8eb2a502588ee5e8d6f84a0d0b987cae05f2e84194361850b69", size = 3823, upload-time = "2026-03-17T14:38:27.82Z" }, + { url = "https://files.pythonhosted.org/packages/4e/d1/4bb0b80f4057903110060f617ef519197194b3ff5dd6153d850c8f5676fa/llama_index_embeddings_openai-0.6.0-py3-none-any.whl", hash = "sha256:039bb1007ad4267e25ddb89a206dfdab862bfb87d58da4271a3919e4f9df4d61", size = 7666, upload-time = "2026-03-12T20:21:28.079Z" }, ] [[package]] @@ -2358,45 +2310,15 @@ wheels = [ [[package]] name = "llama-index-llms-openai" -version = "0.6.26" +version = "0.7.10" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "llama-index-core" }, { name = "openai" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/4a/5e/a7a47d46dc2eb30953d83654112c8af6f61821ca78ef3ea22e30729aac3a/llama_index_llms_openai-0.6.26.tar.gz", hash = "sha256:3474602ecbc30c88a8b585cfd5737891d45da78251a5e067c4dbc2d3cc3d08db", size = 27262, upload-time = "2026-03-05T02:53:50.581Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/2e/8a/f46f59279c078b001374813f69987b43b7c3bd9df01981af545cf2d954d7/llama_index_llms_openai-0.6.26-py3-none-any.whl", hash = "sha256:2062ef505676d0a1c7c116c138c2f890aa7653619fc3ca697e47df7bd2ef8b3f", size = 28330, upload-time = "2026-03-05T02:53:40.421Z" }, -] - -[[package]] -name = "llama-index-readers-file" -version = "0.5.6" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "beautifulsoup4" }, - { name = "defusedxml" }, - { name = "llama-index-core" }, - { name = "pandas" }, - { name = "pypdf" }, - { name = "striprtf" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/a0/e5/dccfb495dbc40f50fcfb799db2287ac5dca4a16a3b09bae61a4ccb1788d3/llama_index_readers_file-0.5.6.tar.gz", hash = "sha256:1c08b14facc2dfe933622aaa26dc7d2a7a6023c42d3db896a2c948789edaf1ea", size = 32535, upload-time = "2025-12-24T16:04:16.421Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/fe/c3/8d28eaa962e073e6735d80847dda9fd3525cb9ff5974ae82dd20621a5a02/llama_index_readers_file-0.5.6-py3-none-any.whl", hash = "sha256:32e83f9adb4e4803e6c7cef746c44fa0949013b1cb76f06f422e9491d198dbda", size = 51832, upload-time = "2025-12-24T16:04:17.307Z" }, -] - -[[package]] -name = "llama-index-readers-llama-parse" -version = "0.6.0" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "llama-index-core" }, - { name = "llama-parse" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/e0/9e/ae83aac91f48cf79141d8751ed0f02aa4dbc62ecf397a4095455ca26f660/llama_index_readers_llama_parse-0.6.0.tar.gz", hash = "sha256:6e086b7034a001bb0204e3431cb1cee5b6ec13f930acbf9a6e82c64aed72b09e", size = 3858, upload-time = "2026-03-12T20:35:56.291Z" } +sdist = { url = "https://files.pythonhosted.org/packages/74/6e/1ef83a852296c55fe8c529b4476073eccaff150d57aceb3d6319a87dcc89/llama_index_llms_openai-0.7.10.tar.gz", hash = "sha256:aa8dabf08ea0b9740fc9dae9677f05049254be5e929794a886692eaf26973def", size = 27586, upload-time = "2026-07-21T23:16:18.087Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/72/52/1d8e1aa23ef32d0df3a7fd533ae0c1bbcd15ae24d9c43d5c66096ef47e0e/llama_index_readers_llama_parse-0.6.0-py3-none-any.whl", hash = "sha256:d06a3376bcb278a610a4dad4bdc70a5f19278370d7df756d8b5c171c3c13ff1f", size = 3200, upload-time = "2026-03-12T20:35:55.611Z" }, + { url = "https://files.pythonhosted.org/packages/8b/89/854cb7b2e5be3b3c16561e93f3edd52d8a1b6e031be1aca4519b9fdd3917/llama_index_llms_openai-0.7.10-py3-none-any.whl", hash = "sha256:b76a0c3d4aaa9aba57dc0366ca6b2346c6819bed22973e334676d7a34704e46a", size = 28673, upload-time = "2026-07-21T23:16:17.062Z" }, ] [[package]] @@ -2413,20 +2335,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/8c/28/20dc2db83adc2d9a11e042eac568f52788eb850e9381ffb1087d51f46672/llama_index_workflows-2.17.1-py3-none-any.whl", hash = "sha256:0d78fc36c2ab5430887c9f34367d59d4c22cf1e6c40ecdc3596214234c2b5010", size = 110539, upload-time = "2026-03-20T15:45:15.341Z" }, ] -[[package]] -name = "llama-parse" -version = "0.5.20" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "click" }, - { name = "llama-index-core" }, - { name = "pydantic" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/87/52/dc9ef71a43eddb8f7b7f6d887feb4e04e61f07bb99359c4aa1dd112c715b/llama_parse-0.5.20.tar.gz", hash = "sha256:649e256431d3753025b9a320bb03b76849ce4b5a1121394c803df543e6c1006f", size = 16941, upload-time = "2025-01-22T21:04:22.226Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/8c/7c/203b7ffc633b9c0823f0d0701e361e002b93bf4e493f4c494d4bd5934c0b/llama_parse-0.5.20-py3-none-any.whl", hash = "sha256:9617edb3428d3218ea01f1708f0b6105f3ffef142fedbeb8c98d50082c37e226", size = 16163, upload-time = "2025-01-22T21:04:20.751Z" }, -] - [[package]] name = "locust" version = "2.43.3" @@ -2663,7 +2571,9 @@ dependencies = [ { name = "pycountry" }, { name = "pygithub" }, { name = "pymupdf" }, + { name = "pymysql" }, { name = "pyparsing" }, + { name = "pypdf" }, { name = "pypdfium2" }, { name = "pytest-lazy-fixtures" }, { name = "python-dateutil" }, @@ -2707,7 +2617,6 @@ dev = [ { name = "pytest-cov" }, { name = "pytest-django" }, { name = "pytest-env" }, - { name = "pytest-freezegun" }, { name = "pytest-mock" }, { name = "pytest-repeat" }, { name = "pytest-xdist", extra = ["psutil"] }, @@ -2734,12 +2643,12 @@ requires-dist = [ { name = "deepmerge", specifier = ">=2.0,<3" }, { name = "dj-database-url", specifier = ">=3.0.0,<4" }, { name = "dj-static", specifier = ">=0.0.6,<0.0.7" }, - { name = "django", specifier = "==4.2.30" }, + { name = "django", specifier = "==5.2.16" }, { name = "django-anymail", extras = ["mailgun"], specifier = ">=13.0,<14" }, { name = "django-bitfield", specifier = ">=2.2.0,<3" }, { name = "django-cache-memoize", specifier = ">=0.2.0,<0.3" }, { name = "django-cors-headers", specifier = ">=4.0.0,<5" }, - { name = "django-filter", specifier = ">=2.4.0,<3" }, + { name = "django-filter", specifier = ">=26.1" }, { name = "django-guardian", specifier = ">=3.0.0,<4" }, { name = "django-health-check", specifier = ">=3.24.0,<4" }, { name = "django-imagekit", specifier = ">=6.0.0,<7" }, @@ -2772,13 +2681,13 @@ requires-dist = [ { name = "langchain-text-splitters", specifier = ">=1.1.2" }, { name = "litellm", specifier = "==1.84.0" }, { name = "llama-index", specifier = ">=0.14.0,<0.15" }, - { name = "llama-index-llms-openai", specifier = ">=0.6.0,<0.7" }, + { name = "llama-index-llms-openai", specifier = ">=0.7.10,<0.8" }, { name = "lxml", specifier = ">=6.0.0,<7" }, { name = "markdown", specifier = ">=3.7,<4" }, { name = "markdown2", specifier = ">=2.4.8,<3" }, { name = "mitol-django-common", specifier = ">=2026.4.2,<2027" }, { name = "mitol-django-keycloak", specifier = ">=2026.7.13,<2027" }, - { name = "mitol-django-observability", specifier = ">=2026.1.0,<2027" }, + { name = "mitol-django-observability", specifier = ">=2026.8.19,<2027" }, { name = "mitol-django-scim", specifier = ">=2026.4.2,<2027" }, { name = "named-enum", specifier = ">=1.4.0,<2" }, { name = "nested-lookup", specifier = ">=0.2.25,<0.3" }, @@ -2798,13 +2707,15 @@ requires-dist = [ { name = "opentelemetry-instrumentation-requests", specifier = ">=0.52b0" }, { name = "pdf2image", specifier = ">=1.17.0,<2" }, { name = "pluggy", specifier = ">=1.3.0,<2" }, - { name = "posthog", specifier = ">=5.0.0,<6" }, + { name = "posthog", specifier = ">=7.38.2,<8" }, { name = "psycopg", extras = ["c"], specifier = ">=3.2.4,<4" }, { name = "pyarrow", specifier = ">=24,<25" }, { name = "pycountry", specifier = ">=24.6.1,<25" }, { name = "pygithub", specifier = ">=2.0.0,<3" }, { name = "pymupdf", specifier = ">=1.26.6,<2" }, + { name = "pymysql", specifier = ">=1.1.1,<2" }, { name = "pyparsing", specifier = ">=3.2.1,<4" }, + { name = "pypdf", specifier = ">=6.9.1,<7" }, { name = "pypdfium2", specifier = ">=5.0.0,<6" }, { name = "pytest-lazy-fixtures", specifier = ">=1.1.1,<2" }, { name = "python-dateutil", specifier = ">=2.8.2,<3" }, @@ -2848,7 +2759,6 @@ dev = [ { name = "pytest-cov", specifier = ">=7.0.0,<8" }, { name = "pytest-django", specifier = ">=4.5.2,<5" }, { name = "pytest-env", specifier = ">=1.0.0,<2" }, - { name = "pytest-freezegun", specifier = ">=0.4.2,<0.5" }, { name = "pytest-mock", specifier = ">=3.10.0,<4" }, { name = "pytest-repeat", specifier = ">=0.9.4" }, { name = "pytest-xdist", extras = ["psutil"], specifier = ">=3.6.1,<4" }, @@ -2895,7 +2805,7 @@ wheels = [ [[package]] name = "mitol-django-observability" -version = "2026.3.11" +version = "2026.8.19" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "django" }, @@ -2907,9 +2817,9 @@ dependencies = [ { name = "pyyaml" }, { name = "structlog" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/84/3b/8c07a0a0feda332656213a8af4f055fa83fb9e0d6d483d77f12d8e63b47e/mitol_django_observability-2026.3.11.tar.gz", hash = "sha256:4539eff6b7da18e500fb0f23c0abf6bdba67eb65e35e890b08c338318494a1e6", size = 10093, upload-time = "2026-03-13T20:21:41.271Z" } +sdist = { url = "https://files.pythonhosted.org/packages/b3/60/ef5807301c6aa3a6ddbe2f48118a98d134f5fc324632f7715f270f3bb504/mitol_django_observability-2026.8.19.tar.gz", hash = "sha256:3bf3afbd16c2f0ae087ed7911f301f31e3361ed170acdc9ef29e3cee8a8d0192", size = 19173, upload-time = "2026-08-19T17:50:14.016Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/cf/76/3f730dec3ff4ff6c5ab044d7b1e027b6f3214cedb66a2532301b6eb4b285/mitol_django_observability-2026.3.11-py3-none-any.whl", hash = "sha256:7523782802c09cad3003cacecdb62cc055efdbd8a80a9ea27a12cb5aaf3f59bc", size = 16052, upload-time = "2026-03-13T20:21:40.216Z" }, + { url = "https://files.pythonhosted.org/packages/80/dd/6e6608392f587f29a950b7e6b27ef4748458dfdfcac70e7fb81907d56deb/mitol_django_observability-2026.8.19-py3-none-any.whl", hash = "sha256:442741f6b6d3ddacd85f5d32c8581c18899af2f7f299a74bc8a4116f0ae3bb8a", size = 27907, upload-time = "2026-08-19T17:50:12.945Z" }, ] [[package]] @@ -3578,27 +3488,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/20/12/38679034af332785aac8774540895e234f4d07f7545804097de4b666afd8/packaging-25.0-py3-none-any.whl", hash = "sha256:29572ef2b1f17581046b3a2227d5c611fb25ec70ca1ba8554b24b0e69331a484", size = 66469, upload-time = "2025-04-19T11:48:57.875Z" }, ] -[[package]] -name = "pandas" -version = "2.3.3" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "numpy" }, - { name = "python-dateutil" }, - { name = "pytz" }, - { name = "tzdata" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/33/01/d40b85317f86cf08d853a4f495195c73815fdf205eef3993821720274518/pandas-2.3.3.tar.gz", hash = "sha256:e05e1af93b977f7eafa636d043f9f94c7ee3ac81af99c13508215942e64c993b", size = 4495223, upload-time = "2025-09-29T23:34:51.853Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/9c/fb/231d89e8637c808b997d172b18e9d4a4bc7bf31296196c260526055d1ea0/pandas-2.3.3-cp312-cp312-macosx_10_13_x86_64.whl", hash = "sha256:6d21f6d74eb1725c2efaa71a2bfc661a0689579b58e9c0ca58a739ff0b002b53", size = 11597846, upload-time = "2025-09-29T23:19:48.856Z" }, - { url = "https://files.pythonhosted.org/packages/5c/bd/bf8064d9cfa214294356c2d6702b716d3cf3bb24be59287a6a21e24cae6b/pandas-2.3.3-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:3fd2f887589c7aa868e02632612ba39acb0b8948faf5cc58f0850e165bd46f35", size = 10729618, upload-time = "2025-09-29T23:39:08.659Z" }, - { url = "https://files.pythonhosted.org/packages/57/56/cf2dbe1a3f5271370669475ead12ce77c61726ffd19a35546e31aa8edf4e/pandas-2.3.3-cp312-cp312-manylinux_2_24_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:ecaf1e12bdc03c86ad4a7ea848d66c685cb6851d807a26aa245ca3d2017a1908", size = 11737212, upload-time = "2025-09-29T23:19:59.765Z" }, - { url = "https://files.pythonhosted.org/packages/e5/63/cd7d615331b328e287d8233ba9fdf191a9c2d11b6af0c7a59cfcec23de68/pandas-2.3.3-cp312-cp312-manylinux_2_24_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:b3d11d2fda7eb164ef27ffc14b4fcab16a80e1ce67e9f57e19ec0afaf715ba89", size = 12362693, upload-time = "2025-09-29T23:20:14.098Z" }, - { url = "https://files.pythonhosted.org/packages/a6/de/8b1895b107277d52f2b42d3a6806e69cfef0d5cf1d0ba343470b9d8e0a04/pandas-2.3.3-cp312-cp312-musllinux_1_2_aarch64.whl", hash = "sha256:a68e15f780eddf2b07d242e17a04aa187a7ee12b40b930bfdd78070556550e98", size = 12771002, upload-time = "2025-09-29T23:20:26.76Z" }, - { url = "https://files.pythonhosted.org/packages/87/21/84072af3187a677c5893b170ba2c8fbe450a6ff911234916da889b698220/pandas-2.3.3-cp312-cp312-musllinux_1_2_x86_64.whl", hash = "sha256:371a4ab48e950033bcf52b6527eccb564f52dc826c02afd9a1bc0ab731bba084", size = 13450971, upload-time = "2025-09-29T23:20:41.344Z" }, - { url = "https://files.pythonhosted.org/packages/86/41/585a168330ff063014880a80d744219dbf1dd7a1c706e75ab3425a987384/pandas-2.3.3-cp312-cp312-win_amd64.whl", hash = "sha256:a16dcec078a01eeef8ee61bf64074b4e524a2a3f4b3be9326420cabe59c4778b", size = 10992722, upload-time = "2025-09-29T23:20:54.139Z" }, -] - [[package]] name = "parso" version = "0.8.6" @@ -3708,18 +3597,17 @@ wheels = [ [[package]] name = "posthog" -version = "5.4.0" +version = "7.39.1" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "backoff" }, { name = "distro" }, - { name = "python-dateutil" }, { name = "requests" }, - { name = "six" }, + { name = "typing-extensions" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/48/20/60ae67bb9d82f00427946218d49e2e7e80fb41c15dc5019482289ec9ce8d/posthog-5.4.0.tar.gz", hash = "sha256:701669261b8d07cdde0276e5bc096b87f9e200e3b9589c5ebff14df658c5893c", size = 88076, upload-time = "2025-06-20T23:19:23.485Z" } +sdist = { url = "https://files.pythonhosted.org/packages/4f/77/3737f60571995ba07677b058bb1523b7c26f28570806b8ffaf83a66df18c/posthog-7.39.1.tar.gz", hash = "sha256:0d184596e35057457fc1094883646fd23de2d6338db8b9c3ea770643fb55d8a2", size = 428586, upload-time = "2026-08-14T13:50:24.688Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/4f/98/e480cab9a08d1c09b1c59a93dade92c1bb7544826684ff2acbfd10fcfbd4/posthog-5.4.0-py3-none-any.whl", hash = "sha256:284dfa302f64353484420b52d4ad81ff5c2c2d1d607c4e2db602ac72761831bd", size = 105364, upload-time = "2025-06-20T23:19:22.001Z" }, + { url = "https://files.pythonhosted.org/packages/88/79/ee5c01937bfb0c80415929e25aa1e8296c48e26fc9a10fe1d9f665f0f478/posthog-7.39.1-py3-none-any.whl", hash = "sha256:e76e82fe571314a0a9bc11d039fd1a1a8d210cd0f737899d41b31f61b73bf08c", size = 504259, upload-time = "2026-08-14T13:50:22.896Z" }, ] [[package]] @@ -4027,6 +3915,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/a8/b6/2a9a8556000199bbf80a5915dcd15d550d1e5288894316445c54726aaf53/pymupdf-1.27.2.2-cp310-abi3-win_amd64.whl", hash = "sha256:09bb53f9486ccb5297030cbc2dbdae845ba1c3c5126e96eb2d16c4f118de0b5b", size = 19238032, upload-time = "2026-03-20T09:45:37.941Z" }, ] +[[package]] +name = "pymysql" +version = "1.2.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/c9/bc/1c6a92f385940f727daeecf3bacaf186e03875dff57197801046c583bcf0/pymysql-1.2.0.tar.gz", hash = "sha256:6c7b17ca686988104d7426c27895b455cdeea3e9d3ceb1270f0c3704fead8c33", size = 49021, upload-time = "2026-05-19T08:26:22.302Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/c4/bd/2534e130295c8cfd4f0a2e31623baab7502278f1e97bcfe61db75656a77f/pymysql-1.2.0-py3-none-any.whl", hash = "sha256:62169ce6d5510f08e140c5e7990ee884a9764024e4a9a27b2cc11f1099322ae0", size = 45716, upload-time = "2026-05-19T08:26:20.974Z" }, +] + [[package]] name = "pynacl" version = "1.6.2" @@ -4061,11 +3958,11 @@ wheels = [ [[package]] name = "pypdf" -version = "6.9.1" +version = "6.16.1" source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/f9/fb/dc2e8cb006e80b0020ed20d8649106fe4274e82d8e756ad3e24ade19c0df/pypdf-6.9.1.tar.gz", hash = "sha256:ae052407d33d34de0c86c5c729be6d51010bf36e03035a8f23ab449bca52377d", size = 5311551, upload-time = "2026-03-17T10:46:07.876Z" } +sdist = { url = "https://files.pythonhosted.org/packages/b6/5a/df92d1c1ef8806ca28f20f978ee059894868d93de797a7e2edebe7fe1a43/pypdf-6.16.1.tar.gz", hash = "sha256:c4d1b43ddae921387321cf63936cd16a7743b91d2da92f165c149a195c972ba9", size = 7003737, upload-time = "2026-08-14T12:24:04.531Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/f9/f4/75543fa802b86e72f87e9395440fe1a89a6d149887e3e55745715c3352ac/pypdf-6.9.1-py3-none-any.whl", hash = "sha256:f35a6a022348fae47e092a908339a8f3dc993510c026bb39a96718fc7185e89f", size = 333661, upload-time = "2026-03-17T10:46:06.286Z" }, + { url = "https://files.pythonhosted.org/packages/33/a1/724b18d6757ab7253a8fecd3a430eb8d980ed26872ba16651e7b5ddfc63f/pypdf-6.16.1-py3-none-any.whl", hash = "sha256:63fec31c4092ae50b6729beedcb469055b60d20c834bde1c402df241f371f644", size = 382924, upload-time = "2026-08-14T12:24:02.854Z" }, ] [[package]] @@ -4178,19 +4075,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/27/98/822b924a4a3eb58aacba84444c7439fce32680592f394de26af9c76e2569/pytest_env-1.2.0-py3-none-any.whl", hash = "sha256:d7e5b7198f9b83c795377c09feefa45d56083834e60d04767efd64819fc9da00", size = 6251, upload-time = "2025-10-09T19:15:46.077Z" }, ] -[[package]] -name = "pytest-freezegun" -version = "0.4.2" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "freezegun" }, - { name = "pytest" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/f0/e3/c39d7c3d3afef5652f19323f3483267d7e6b0d9911c3867e10d6e2d3c9ae/pytest-freezegun-0.4.2.zip", hash = "sha256:19c82d5633751bf3ec92caa481fb5cffaac1787bd485f0df6436fd6242176949", size = 9059, upload-time = "2020-07-19T17:50:03.678Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/9e/09/0bdd7d24b9d21453ad3364ae1efbd65082045bb6081b5fd5eade91a9b644/pytest_freezegun-0.4.2-py2.py3-none-any.whl", hash = "sha256:5318a6bfb8ba4b709c8471c94d0033113877b3ee02da5bfcd917c1889cde99a7", size = 4590, upload-time = "2020-07-19T17:50:02.191Z" }, -] - [[package]] name = "pytest-lazy-fixtures" version = "1.4.0" @@ -4971,15 +4855,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/81/69/297302c5f5f59c862faa31e6cb9a4cd74721cd1e052b38e464c5b402df8b/StrEnum-0.4.15-py3-none-any.whl", hash = "sha256:a30cda4af7cc6b5bf52c8055bc4bf4b2b6b14a93b574626da33df53cf7740659", size = 8851, upload-time = "2023-06-29T22:02:56.947Z" }, ] -[[package]] -name = "striprtf" -version = "0.0.26" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/25/20/3d419008265346452d09e5dadfd5d045b64b40d8fc31af40588e6c76997a/striprtf-0.0.26.tar.gz", hash = "sha256:fdb2bba7ac440072d1c41eab50d8d74ae88f60a8b6575c6e2c7805dc462093aa", size = 6258, upload-time = "2023-07-20T14:30:36.29Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/a3/cf/0fea4f4ba3fc2772ac2419278aa9f6964124d4302117d61bc055758e000c/striprtf-0.0.26-py3-none-any.whl", hash = "sha256:8c8f9d32083cdc2e8bfb149455aa1cc5a4e0a035893bedc75db8b73becb3a1bb", size = 6914, upload-time = "2023-07-20T14:30:35.338Z" }, -] - [[package]] name = "structlog" version = "25.5.0" diff --git a/vector_search/constants.py b/vector_search/constants.py index 9d082b4b5f..a22904d4d1 100644 --- a/vector_search/constants.py +++ b/vector_search/constants.py @@ -43,6 +43,11 @@ "checksum": "checksum", } +# Payload key holding a resource's completeness score (0-1), the same value the +# OpenSearch script_score penalizes incomplete OCW courses by. Only the resources +# collection carries it; content file payloads do not. +COMPLETENESS_PAYLOAD_KEY = "completeness" + QDRANT_RESOURCE_PARAM_MAP = { "readable_id": "readable_id", "resource_type": "resource_type", @@ -96,6 +101,9 @@ "next_start_date": models.PayloadSchemaType.DATETIME, "created_on": models.PayloadSchemaType.DATETIME, "views": models.PayloadSchemaType.INTEGER, + # Not filterable or facetable -- indexed because Qdrant rejects a scoring + # formula that reads an unindexed payload key (see COMPLETENESS_PAYLOAD_KEY). + COMPLETENESS_PAYLOAD_KEY: models.PayloadSchemaType.FLOAT, } diff --git a/vector_search/utils.py b/vector_search/utils.py index b61ecc1247..d0251419e8 100644 --- a/vector_search/utils.py +++ b/vector_search/utils.py @@ -42,6 +42,7 @@ from main.utils import checksum_for_content, chunks from vector_search.constants import ( COLLECTION_PARAM_MAP, + COMPLETENESS_PAYLOAD_KEY, CONTENT_FILES_COLLECTION_NAME, COURSE_NUMBER_INDEXING_ONLY_FIELDS, QDRANT_CONTENT_FILE_INDEXES, @@ -1744,6 +1745,55 @@ def custom_score_formula(collection_name: str) -> list[models.MultExpression]: return score_expressions +def completeness_penalty_expression( + collection_name: str, +) -> models.NegExpression | None: + """ + Build the incompleteness penalty term: -weight * (1 - completeness), to be + added to the score. + + Deliberately additive rather than the multiplicative form OpenSearch uses -- + see VECTOR_SEARCH_INCOMPLETENESS_PENALTY_WEIGHT. None when the penalty is + disabled or the collection has no completeness. + """ + if collection_name != RESOURCES_COLLECTION_NAME: + return None + weight = max(settings.VECTOR_SEARCH_INCOMPLETENESS_PENALTY_WEIGHT or 0, 0) + if not weight: + return None + return models.NegExpression( + neg=models.MultExpression( + mult=[ + weight, + models.SumExpression( + sum=[1, models.NegExpression(neg=COMPLETENESS_PAYLOAD_KEY)] + ), + ] + ) + ) + + +def score_formula_query(collection_name: str) -> models.FormulaQuery | None: + """ + Build a collection's rescoring formula: the score, plus the + VECTOR_SEARCH_SCORE_BOOST boosts, minus the incompleteness penalty. None when + neither applies, so callers can skip rescoring entirely. + """ + boost_expressions = custom_score_formula(collection_name) + penalty = completeness_penalty_expression(collection_name) + if not boost_expressions and penalty is None: + return None + terms = ["$score", *boost_expressions] + if penalty is None: + return models.FormulaQuery(formula=models.SumExpression(sum=terms)) + return models.FormulaQuery( + formula=models.SumExpression(sum=[*terms, penalty]), + # Points indexed before completeness was added to the payload, and any + # resource type that does not carry it, score as fully complete. + defaults={COMPLETENESS_PAYLOAD_KEY: 1.0}, + ) + + def db_sync_to_async(func): """Offload sync DB work to the thread pool, with per-call connection cleanup.""" diff --git a/vector_search/utils_test.py b/vector_search/utils_test.py index e0b10faded..0207b5b317 100644 --- a/vector_search/utils_test.py +++ b/vector_search/utils_test.py @@ -39,6 +39,7 @@ ) from main.utils import checksum_for_content from vector_search.constants import ( + COMPLETENESS_PAYLOAD_KEY, CONTENT_FILES_COLLECTION_NAME, QDRANT_CONTENT_FILE_INDEXES, QDRANT_CONTENT_FILE_PARAM_MAP, @@ -74,6 +75,7 @@ _set_payload, async_qdrant_aggregations, check_missing_content_file_ids, + completeness_penalty_expression, compute_optimizer_settings, create_qdrant_collections, custom_score_formula, @@ -83,6 +85,7 @@ qdrant_query_conditions, remove_qdrant_records, resources_payload_selector, + score_formula_query, should_generate_content_embeddings, should_generate_resource_embeddings, update_content_file_payload, @@ -2928,6 +2931,88 @@ def test_custom_score_formula_defaults(mocker): assert isinstance(results[0].mult[2], models.GaussDecayExpression) +def test_completeness_penalty_expression(settings): + """The penalty subtracts weight * (1 - completeness) from the score.""" + settings.VECTOR_SEARCH_INCOMPLETENESS_PENALTY_WEIGHT = 0.05 + + expression = completeness_penalty_expression(RESOURCES_COLLECTION_NAME) + + assert isinstance(expression, models.NegExpression) + weight, incompleteness = expression.neg.mult + assert weight == 0.05 + # 1 - completeness + assert incompleteness.sum[0] == 1 + assert incompleteness.sum[1].neg == COMPLETENESS_PAYLOAD_KEY + + +@pytest.mark.parametrize("weight", [0, None, -1]) +def test_completeness_penalty_expression_disabled(settings, weight): + """A weight of 0, unset, or negative leaves scores alone.""" + settings.VECTOR_SEARCH_INCOMPLETENESS_PENALTY_WEIGHT = weight + + assert completeness_penalty_expression(RESOURCES_COLLECTION_NAME) is None + + +def test_completeness_penalty_expression_other_collections(settings): + """Only resource payloads carry completeness, so only they are penalized.""" + settings.VECTOR_SEARCH_INCOMPLETENESS_PENALTY_WEIGHT = 0.05 + + assert completeness_penalty_expression(CONTENT_FILES_COLLECTION_NAME) is None + + +def test_score_formula_query_combines_boosts_and_penalty(mocker, settings): + """Boosts add to the score and the penalty subtracts from it.""" + settings.VECTOR_SEARCH_INCOMPLETENESS_PENALTY_WEIGHT = 0.05 + mocker.patch( + "vector_search.utils.VECTOR_SEARCH_SCORE_BOOST", + {RESOURCES_COLLECTION_NAME: [{"boost": 0.15, "params": {"free": True}}]}, + ) + + formula_query = score_formula_query(RESOURCES_COLLECTION_NAME) + + assert formula_query.defaults == {COMPLETENESS_PAYLOAD_KEY: 1.0} + score, boost, penalty = formula_query.formula.sum + assert score == "$score" + assert isinstance(boost, models.MultExpression) + assert penalty == completeness_penalty_expression(RESOURCES_COLLECTION_NAME) + + +def test_score_formula_query_penalty_only(mocker, settings): + """With no boosts configured the formula is the score minus the penalty.""" + settings.VECTOR_SEARCH_INCOMPLETENESS_PENALTY_WEIGHT = 0.05 + mocker.patch("vector_search.utils.VECTOR_SEARCH_SCORE_BOOST", {}) + + formula_query = score_formula_query(RESOURCES_COLLECTION_NAME) + + score, penalty = formula_query.formula.sum + assert score == "$score" + assert penalty == completeness_penalty_expression(RESOURCES_COLLECTION_NAME) + + +def test_score_formula_query_boosts_only(mocker, settings): + """With the penalty disabled the formula keeps the boosts and no defaults.""" + settings.VECTOR_SEARCH_INCOMPLETENESS_PENALTY_WEIGHT = 0 + mocker.patch( + "vector_search.utils.VECTOR_SEARCH_SCORE_BOOST", + {RESOURCES_COLLECTION_NAME: [{"boost": 0.15, "params": {"free": True}}]}, + ) + + formula_query = score_formula_query(RESOURCES_COLLECTION_NAME) + + assert not formula_query.defaults + score, boost = formula_query.formula.sum + assert score == "$score" + assert isinstance(boost, models.MultExpression) + + +def test_score_formula_query_nothing_to_apply(mocker, settings): + """Nothing to boost and nothing to penalize means no rescoring stage.""" + settings.VECTOR_SEARCH_INCOMPLETENESS_PENALTY_WEIGHT = 0.05 + mocker.patch("vector_search.utils.VECTOR_SEARCH_SCORE_BOOST", {}) + + assert score_formula_query(CONTENT_FILES_COLLECTION_NAME) is None + + @pytest.mark.django_db def test_best_run_ids_for_resources_non_test_mode(): """A normal course resolves to only its best run's run_id.""" diff --git a/vector_search/views.py b/vector_search/views.py index bb8c747356..7add066862 100644 --- a/vector_search/views.py +++ b/vector_search/views.py @@ -39,11 +39,11 @@ async_qdrant_client, best_run_ids_for_resources, check_missing_content_file_ids, - custom_score_formula, db_sync_to_async, dense_encoder, qdrant_query_conditions, resources_payload_selector, + score_formula_query, sparse_encoder, ) @@ -170,18 +170,20 @@ async def _build_search_params( # noqa: PLR0913 if normalized_score is not None: search_params["score_threshold"] = normalized_score + # Boosts and the completeness penalty, or None when neither applies to + # this collection. + formula_query = score_formula_query(search_collection) + if hybrid_search: sparse_query, dense_query = await asyncio.gather( db_sync_to_async(encoder_sparse.embed)(query_string), db_sync_to_async(encoder_dense.embed_query)(query_string), ) - custom_formula_query = models.FormulaQuery( - formula=models.SumExpression( - sum=[ - "$score", - *custom_score_formula(search_collection), - ] - ) + # Each arm is rescored before fusion, so the formula shapes the + # ranks RRF sees. With nothing to apply it is an identity rescore, + # which keeps the prefetch nesting the same either way. + custom_formula_query = formula_query or models.FormulaQuery( + formula=models.SumExpression(sum=["$score"]) ) prefetch_params = [ models.Prefetch( @@ -236,6 +238,20 @@ async def _build_search_params( # noqa: PLR0913 search_params["query"] = models.OrderByQuery( order_by=self._format_order_by(order_by) ) + elif formula_query: + # Nest: dense vector prefetch → formula rescore. The score + # threshold and search params move onto the prefetch so the + # cutoff keeps applying to the raw similarity score rather than + # the rescored one, and the vector search stays tuned. + search_params["prefetch"] = models.Prefetch( + query=dense_query, + using=encoder_dense.model_short_name(), + limit=prefetch_limit, + filter=search_filter, + params=search_params["search_params"], + score_threshold=search_params.pop("score_threshold", None), + ) + search_params["query"] = formula_query else: search_params["using"] = encoder_dense.model_short_name() search_params["query"] = dense_query @@ -354,6 +370,7 @@ async def _async_vector_hits( # noqa: PLR0913 ) if prefetch_max_limit is not None: prefetch_limit = min(prefetch_limit, prefetch_max_limit) + prefetch_limit = max(prefetch_limit, offset + limit) search_params = await self._build_search_params( query_string, diff --git a/vector_search/views_test.py b/vector_search/views_test.py index 50038d1784..d25a0ffb79 100644 --- a/vector_search/views_test.py +++ b/vector_search/views_test.py @@ -16,11 +16,14 @@ ) from learning_resources_search.serializers import serialize_bulk_learning_resources from vector_search.constants import ( + COMPLETENESS_PAYLOAD_KEY, CONTENT_FILES_RETRIEVE_PAYLOAD, + RESOURCES_COLLECTION_NAME, RESOURCES_PAYLOAD_EXCLUDE, RESOURCES_RETRIEVE_PAYLOAD, ) from vector_search.encoders.utils import dense_encoder, sparse_encoder +from vector_search.utils import score_formula_query from vector_search.views import QdrantView @@ -835,7 +838,113 @@ def test_vector_search_with_score_cutoff_enforces_min_score( if hybrid_search: assert call_kwargs["score_threshold"] == settings.HYBRID_VECTOR_SEARCH_MIN_SCORE else: - assert call_kwargs["score_threshold"] == settings.DENSE_VECTOR_SEARCH_MIN_SCORE + # Dense search rescores a prefetch with the score formula, so the cutoff + # sits on the prefetch and keeps applying to the raw similarity score. + assert "score_threshold" not in call_kwargs + assert ( + call_kwargs["prefetch"].score_threshold + == settings.DENSE_VECTOR_SEARCH_MIN_SCORE + ) + + +def _completeness_penalty(formula_query): + """Pull the completeness penalty term out of a resource score formula.""" + return formula_query.formula.sum[-1] + + +@pytest.mark.parametrize("hybrid_search", [True, False]) +def test_vector_search_applies_completeness_penalty( + mocker, client, settings, hybrid_search +): + """Both search modes must rescore resources with the completeness penalty.""" + settings.VECTOR_SEARCH_INCOMPLETENESS_PENALTY_WEIGHT = 0.05 + + mock_qdrant = mocker.patch( + "qdrant_client.AsyncQdrantClient", return_value=mocker.AsyncMock() + )() + mock_result = mocker.MagicMock() + mock_result.points = [] + mock_qdrant.query_points = mocker.AsyncMock(return_value=mock_result) + mock_qdrant.scroll = mocker.AsyncMock(return_value=([], None)) + mocker.patch("vector_search.views.async_qdrant_client", return_value=mock_qdrant) + + client.get( + reverse("vector_search:v0:vector_learning_resources_search"), + data={"q": "test", "hybrid_search": hybrid_search}, + ) + + call_kwargs = mock_qdrant.query_points.mock_calls[0].kwargs + expected_penalty = _completeness_penalty( + score_formula_query(RESOURCES_COLLECTION_NAME) + ) + + if hybrid_search: + # One rescored prefetch per vector arm, fused afterwards + assert isinstance(call_kwargs["query"], models.FusionQuery) + formula_queries = [prefetch.query for prefetch in call_kwargs["prefetch"]] + else: + formula_queries = [call_kwargs["query"]] + assert call_kwargs["prefetch"].using == dense_encoder().model_short_name() + + assert formula_queries + for formula_query in formula_queries: + assert isinstance(formula_query, models.FormulaQuery) + assert formula_query.defaults == {COMPLETENESS_PAYLOAD_KEY: 1.0} + assert _completeness_penalty(formula_query) == expected_penalty + + +def test_dense_vector_search_without_formula_queries_vectors_directly( + mocker, client, settings +): + """With nothing to rescore, dense search skips the prefetch entirely.""" + settings.VECTOR_SEARCH_INCOMPLETENESS_PENALTY_WEIGHT = 0 + mocker.patch("vector_search.utils.VECTOR_SEARCH_SCORE_BOOST", {}) + + mock_qdrant = mocker.patch( + "qdrant_client.AsyncQdrantClient", return_value=mocker.AsyncMock() + )() + mock_result = mocker.MagicMock() + mock_result.points = [] + mock_qdrant.query_points = mocker.AsyncMock(return_value=mock_result) + mock_qdrant.scroll = mocker.AsyncMock(return_value=([], None)) + mocker.patch("vector_search.views.async_qdrant_client", return_value=mock_qdrant) + + client.get( + reverse("vector_search:v0:vector_learning_resources_search"), + data={"q": "test", "hybrid_search": False}, + ) + + call_kwargs = mock_qdrant.query_points.mock_calls[0].kwargs + assert "prefetch" not in call_kwargs + assert call_kwargs["using"] == dense_encoder().model_short_name() + assert call_kwargs["score_threshold"] == settings.DENSE_VECTOR_SEARCH_MIN_SCORE + + +@pytest.mark.django_db(transaction=True) +def test_content_file_search_has_no_completeness_penalty( + mocker, client, settings, content_file_viewer +): + """Content file payloads carry no completeness, so nothing is penalized.""" + settings.VECTOR_SEARCH_INCOMPLETENESS_PENALTY_WEIGHT = 0.05 + + mock_qdrant = mocker.patch( + "qdrant_client.AsyncQdrantClient", return_value=mocker.AsyncMock() + )() + mock_result = mocker.MagicMock() + mock_result.points = [] + mock_qdrant.query_points = mocker.AsyncMock(return_value=mock_result) + mock_qdrant.scroll = mocker.AsyncMock(return_value=([], None)) + mock_qdrant.count = mocker.AsyncMock(return_value=CountResult(count=0)) + mocker.patch("vector_search.views.async_qdrant_client", return_value=mock_qdrant) + + client.get( + reverse("vector_search:v0:vector_content_files_search"), + data={"q": "test", "hybrid_search": False}, + ) + + call_kwargs = mock_qdrant.query_points.mock_calls[0].kwargs + assert "prefetch" not in call_kwargs + assert call_kwargs["using"] == dense_encoder().model_short_name() @pytest.mark.parametrize("query_string", ["", "test"]) @@ -889,6 +998,39 @@ def test_build_search_params_sort_with_cutoff_score( assert search_params["query"].order_by.direction == models.Direction.ASC +def test_prefetch_limit_at_least_offset_plus_limit(mocker, settings): + """Ensure prefetch_limit is at least offset + limit even when prefetch_max_limit is smaller.""" + settings.VECTOR_HYBRID_SEARCH_PREFETCH_MAX_LIMIT = 500 + + mock_qdrant = mocker.patch( + "qdrant_client.AsyncQdrantClient", return_value=mocker.AsyncMock() + )() + mock_result = mocker.MagicMock() + mock_result.points = [] + mock_qdrant.query_points = mocker.AsyncMock(return_value=mock_result) + mock_qdrant.scroll = mocker.AsyncMock(return_value=([], None)) + mocker.patch("vector_search.views.async_qdrant_client", return_value=mock_qdrant) + + view = QdrantView() + view_spy = mocker.spy(view, "_build_search_params") + + asyncio.run( + view._async_vector_hits( # noqa: SLF001 + query_string="test", + params={}, + limit=10, + offset=600, + hybrid_search=True, + ) + ) + + assert view_spy.call_count == 1 + # offset + limit = 610, which exceeds max cap 500, so prefetch_limit must be clamped to 610 + # positional arg index 4 of bound _build_search_params is prefetch_limit (0: query, 1: collection, 2: filter, 3: limit, 4: prefetch_limit) + prefetch_limit_arg = view_spy.call_args.args[4] + assert prefetch_limit_arg == 610 + + @pytest.mark.django_db(transaction=True) def test_content_file_search_restricts_resource_query_to_best_run( mocker, client, django_user_model diff --git a/yarn.lock b/yarn.lock index b920005a74..d790bbb9e6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3557,15 +3557,15 @@ __metadata: languageName: node linkType: hard -"@mitodl/arithmix@npm:^0.2.4": - version: 0.2.4 - resolution: "@mitodl/arithmix@npm:0.2.4" +"@mitodl/arithmix@npm:^0.2.5": + version: 0.2.5 + resolution: "@mitodl/arithmix@npm:0.2.5" peerDependencies: "@emotion/react": ">=11" "@emotion/styled": ">=11" react: ">=18" react-dom: ">=18" - checksum: 10/0446f5d75db18a18d60432bf7aec508d8533797ee7d17084fc52534f29944fbcc7354dbe11c36b33b51cd5b800fe88603eaf53e5e8d370827cfb40ebab9d7e70 + checksum: 10/48ba61eb110621a75c20cad74fe4aa7a0523f23aa68de1586ef9d918c9a0113ea525e02e577bdba2b4f85fa46a272d63829d79f74da91803198b55c233d65aa9 languageName: node linkType: hard @@ -16906,7 +16906,7 @@ __metadata: "@faker-js/faker": "npm:^10.0.0" "@floating-ui/react": "npm:^0.27.16" "@happy-dom/jest-environment": "npm:^20.1.0" - "@mitodl/arithmix": "npm:^0.2.4" + "@mitodl/arithmix": "npm:^0.2.5" "@mitodl/course-search-utils": "npm:^3.5.2" "@mitodl/hacksnack": "npm:^0.1.2" "@mitodl/mitxonline-api-axios": "npm:2026.8.18"