Program info
" mocker.patch( diff --git a/learning_resources/utils.py b/learning_resources/utils.py index bf42b48a2d..5462ee4113 100644 --- a/learning_resources/utils.py +++ b/learning_resources/utils.py @@ -27,6 +27,7 @@ LearningResourceType, semester_mapping, ) +from learning_resources.etl.constants import MARKETING_PAGE_FILE_TYPE from learning_resources.hooks import get_plugin_manager from learning_resources.models import ( ContentFile, @@ -740,53 +741,26 @@ def build_resource_summary_dict(resource): } -def _build_entry(resource, summaries_by_resource): - """Build a resource entry dict for markdown rendering.""" - entry = build_resource_summary_dict(resource) - entry["summaries"] = summaries_by_resource.get(resource.id, []) - return entry +# Bound the program marketing-page content assembled from child courses so it +# stays a reasonable embedding input. A program page previously concatenated +# every child-course content-file summary and ballooned to 23MB in prod, +# overflowing the embedding API's per-request token limit. +PROGRAM_CHILDREN_CONTENT_MAX_CHARS = 1_000_000 -def _build_entries_from_relationships(relationships, summaries_by_resource): - """Build entry dicts from prefetched relationships, including grandchildren.""" - entries = [] +def _course_ids_by_program(relationships, course_type): + """Map program id -> reachable published course ids (direct + via subprograms).""" + result = defaultdict(set) for rel in relationships: - entry = _build_entry(rel.child, summaries_by_resource) - - if ( - rel.child.resource_type == LearningResourceType.program.name - and rel.relation_type == LearningResourceRelationTypes.PROGRAM_PROGRAMS - ): - sub_entries = [ - _build_entry(sub_rel.child, summaries_by_resource) + if rel.child.resource_type == course_type: + result[rel.parent_id].add(rel.child_id) + else: + result[rel.parent_id].update( + sub_rel.child_id for sub_rel in getattr(rel.child, "program_children", []) - ] - if sub_entries: - entry["children"] = sub_entries - - entries.append(entry) - return entries - - -def _format_resource_entries(entries, heading_level=3): - """Recursively format resource entries as markdown sections.""" - sections = [] - for entry in entries: - prefix = "#" * heading_level - lines = [f"{prefix} {entry['title']}"] - if entry["description"]: - lines.append(entry["description"]) - if entry["topics"]: - lines.append(f"Topics: {', '.join(entry['topics'])}") - if entry.get("summaries"): - lines.append("\n**Content summaries:**") - lines.extend(f"- {summary}" for summary in entry["summaries"]) - sections.append("\n".join(lines)) - if entry.get("children"): - sections.extend( - _format_resource_entries(entry["children"], heading_level + 1) + if sub_rel.child.resource_type == course_type ) - return sections + return result def build_program_children_content(learning_resource): @@ -801,8 +775,10 @@ def build_program_children_content(learning_resource): def build_program_children_content_bulk(program_resources): """Build program children markdown for many program resources in bulk. - Returns a dict keyed by learning_resource id with markdown content. - Non-program resources are ignored. + For each program, concatenates the marketing-page content of its published + (or test-mode) child courses, including courses reached through child + programs. Returns a dict keyed by learning_resource id. Non-program + resources are ignored. """ programs = [ resource @@ -825,59 +801,52 @@ def build_program_children_content_bulk(program_resources): relation_type__in=program_relation_types, ) .filter(child_visibility) - .select_related("parent", "child") + .select_related("child") .prefetch_related( - "child__topics", Prefetch( "child__children", queryset=LearningResourceRelationship.objects.filter( relation_type__in=program_relation_types, ) .filter(child_visibility) - .select_related("child") - .prefetch_related("child__topics"), + .select_related("child"), to_attr="program_children", ), ) ) - child_ids = [rel.child_id for rel in relationships] - grandchild_ids = [ - gc.child_id - for rel in relationships - for gc in getattr(rel.child, "program_children", []) - ] - all_ids = set(child_ids + grandchild_ids) - set(program_ids) + course_type = LearningResourceType.course.name + course_ids_by_program = _course_ids_by_program(relationships, course_type) + all_course_ids = set().union(*course_ids_by_program.values()) - summaries_by_resource = {} - if all_ids: - summary_qs = ( + # One marketing-page content file per course (published), fetched in bulk + marketing_by_course = {} + if all_course_ids: + marketing_qs = ( ContentFile.objects.filter( - run__learning_resource_id__in=all_ids, + learning_resource_id__in=all_course_ids, + file_type=MARKETING_PAGE_FILE_TYPE, published=True, - run__published=True, ) - .exclude(summary="") - .values_list("run__learning_resource_id", "summary") + .exclude(content="") + .order_by("learning_resource_id", "id") + .values_list("learning_resource_id", "learning_resource__title", "content") ) - for resource_id, summary in summary_qs: - summaries_by_resource.setdefault(resource_id, []).append(summary) - - relationships_by_program = defaultdict(list) - for rel in relationships: - relationships_by_program[rel.parent_id].append(rel) + for lr_id, title, content in marketing_qs: + marketing_by_course.setdefault(lr_id, (title, content)) content_by_program_id = {} for program in programs: - rels = relationships_by_program.get(program.id, []) - entries = _build_entries_from_relationships(rels, summaries_by_resource) - - if not entries: + sections = [] + for course_id in sorted(course_ids_by_program.get(program.id, set())): + page = marketing_by_course.get(course_id) + if page: + title, content = page + sections.append(f"### {title}\n\n{content}") + if not sections: content_by_program_id[program.id] = "" continue - - sections = ["\n\n## Program Contents\n"] - sections.extend(_format_resource_entries(entries)) - content_by_program_id[program.id] = "\n\n".join(sections) + body = "\n\n".join(["\n\n## Program Contents\n", *sections]) + content_by_program_id[program.id] = body[:PROGRAM_CHILDREN_CONTENT_MAX_CHARS] return content_by_program_id diff --git a/learning_resources/utils_test.py b/learning_resources/utils_test.py index 596134c98d..53e74af418 100644 --- a/learning_resources/utils_test.py +++ b/learning_resources/utils_test.py @@ -19,6 +19,7 @@ CONTENT_TYPE_VIDEO, LearningResourceRelationTypes, ) +from learning_resources.etl.constants import MARKETING_PAGE_FILE_TYPE from learning_resources.etl.utils import get_content_type from learning_resources.factories import ( CourseFactory, @@ -118,6 +119,20 @@ def fixture_test_instructors_data(): return json.load(test_data)["instructors"] +def _add_course_marketing_page(course_lr, content): + """Attach a published marketing-page content file to a course.""" + from learning_resources.models import ContentFile + + return ContentFile.objects.create( + learning_resource=course_lr, + file_type=MARKETING_PAGE_FILE_TYPE, + file_extension=".md", + key=f"mktg-{course_lr.id}", + content=content, + published=True, + ) + + @pytest.fixture def program_with_visibility_children(): """Program with published, unpublished, and test_mode child courses.""" @@ -135,6 +150,8 @@ def program_with_visibility_children(): learning_resource__published=False, learning_resource__test_mode=True, ).learning_resource + for child in (published, unpublished, test_mode): + _add_course_marketing_page(child, f"Marketing copy for {child.title}.") program_lr = ProgramFactory.create(courses=[published]).learning_resource # Manually add unpublished/test_mode children since ProgramFactory # only creates PROGRAM_COURSES relationships for visible courses @@ -687,27 +704,40 @@ def test_build_program_children_content_non_program(): assert build_program_children_content(course_lr) == "" -def test_build_program_children_content_direct_courses(): - """Programs with direct course children should include them""" +def test_build_program_children_content_includes_child_course_marketing_pages(): + """A published child course's marketing-page content is included.""" course_lr = CourseFactory.create( learning_resource__title="Test Course", - learning_resource__description="A test course", ).learning_resource + _add_course_marketing_page(course_lr, "Marketing copy for the test course.") program_lr = ProgramFactory.create(courses=[course_lr]).learning_resource result = build_program_children_content(program_lr) assert "## Program Contents" in result - assert "Test Course" in result - assert "A test course" in result + assert "### Test Course" in result + assert "Marketing copy for the test course." in result + +def test_build_program_children_content_omits_courses_without_marketing_page(): + """Child courses lacking a marketing-page content file contribute nothing.""" + course_lr = CourseFactory.create( + learning_resource__title="No Marketing Course", + ).learning_resource + program_lr = ProgramFactory.create(courses=[course_lr]).learning_resource -def test_build_program_children_content_child_programs_with_courses(): - """Programs with child programs should recurse to find courses""" + assert build_program_children_content(program_lr) == "" + + +def test_build_program_children_content_recurses_child_program_courses(): + """Courses reached through a child program are included; the child program + itself is not emitted as a heading. + """ from learning_resources.models import LearningResourceRelationship grandchild_course_lr = CourseFactory.create( learning_resource__title="Grandchild Course" ).learning_resource + _add_course_marketing_page(grandchild_course_lr, "Grandchild marketing copy.") child_program_lr = ProgramFactory.create( courses=[grandchild_course_lr], learning_resource__title="Child Program" ).learning_resource @@ -719,31 +749,58 @@ def test_build_program_children_content_child_programs_with_courses(): ) result = build_program_children_content(parent_lr) - assert "Child Program" in result - assert "Grandchild Course" in result + assert "### Grandchild Course" in result + assert "Grandchild marketing copy." in result + # Child programs are traversed only to reach courses, not emitted themselves. + assert "Child Program" not in result -def test_build_program_children_content_with_summaries(): - """Child course contentfile summaries should be included""" - from learning_resources.models import ContentFile, LearningResourceRun +def test_build_program_children_content_excludes_unpublished_marketing_pages(): + """Unpublished marketing-page content files are excluded.""" + from learning_resources.models import ContentFile course_lr = CourseFactory.create( - learning_resource__title="Course With Summary" + learning_resource__title="Course With Unpublished Page" ).learning_resource - run = LearningResourceRun.objects.create( - learning_resource=course_lr, - run_id="test-run", - ) ContentFile.objects.create( - run=run, - key="transcript.txt", - summary="This is a summary of the course content.", + learning_resource=course_lr, + file_type=MARKETING_PAGE_FILE_TYPE, + file_extension=".md", + key=f"mktg-{course_lr.id}", + content="Hidden marketing copy.", + published=False, ) program_lr = ProgramFactory.create(courses=[course_lr]).learning_resource result = build_program_children_content(program_lr) - assert "This is a summary of the course content." in result - assert "Content summaries" in result + assert "Hidden marketing copy." not in result + assert result == "" + + +def test_build_program_children_content_caps_total_length(): + """Program children content is hard-capped in total size""" + course_lr = CourseFactory.create().learning_resource + _add_course_marketing_page(course_lr, "x" * 1_500_000) + program_lr = ProgramFactory.create(courses=[course_lr]).learning_resource + + result = build_program_children_content(program_lr) + assert len(result) <= 1_000_000 + + +def test_build_program_children_content_deterministic_order(): + """Child course sections appear in a stable order.""" + courses = [ + CourseFactory.create(learning_resource__title=f"Course {i}").learning_resource + for i in range(3) + ] + for i, course in enumerate(courses): + _add_course_marketing_page(course, f"body {i}") + program_lr = ProgramFactory.create(courses=courses).learning_resource + + result = build_program_children_content(program_lr) + ordered_by_id = sorted(courses, key=lambda c: c.id) + positions = [result.index(f"### {c.title}") for c in ordered_by_id] + assert positions == sorted(positions) def test_build_program_children_content_ignores_non_program_relations(): @@ -753,12 +810,14 @@ def test_build_program_children_content_ignores_non_program_relations(): course_lr = CourseFactory.create( learning_resource__title="Real Course" ).learning_resource + _add_course_marketing_page(course_lr, "Real course copy.") program_lr = ProgramFactory.create(courses=[course_lr]).learning_resource # Add a non-program relation (e.g. LEARNING_PATH_ITEMS) unrelated_lr = CourseFactory.create( learning_resource__title="Unrelated Item" ).learning_resource + _add_course_marketing_page(unrelated_lr, "Unrelated copy.") LearningResourceRelationship.objects.create( parent=program_lr, child=unrelated_lr, @@ -766,30 +825,8 @@ def test_build_program_children_content_ignores_non_program_relations(): ) result = build_program_children_content(program_lr) - assert "Real Course" in result - assert "Unrelated Item" not in result - - -def test_build_program_children_content_two_levels(): - """Program -> child program -> courses are all included (2 levels)""" - from learning_resources.models import LearningResourceRelationship - - nested_course = CourseFactory.create( - learning_resource__title="Nested Course" - ).learning_resource - mid_lr = ProgramFactory.create( - courses=[nested_course], learning_resource__title="Mid Program" - ).learning_resource - top_lr = ProgramFactory.create(courses=[]).learning_resource - LearningResourceRelationship.objects.create( - parent=top_lr, - child=mid_lr, - relation_type="PROGRAM_PROGRAMS", - ) - - result = build_program_children_content(top_lr) - assert "Mid Program" in result - assert "Nested Course" in result + assert "Real course copy." in result + assert "Unrelated copy." not in result # --- build_program_children_content_bulk tests --- @@ -818,8 +855,8 @@ def test_build_program_children_content_bulk_single_program(): """Bulk function produces same output as single-resource version.""" course_lr = CourseFactory.create( learning_resource__title="Bulk Test Course", - learning_resource__description="A description", ).learning_resource + _add_course_marketing_page(course_lr, "Bulk marketing copy.") program_lr = ProgramFactory.create(courses=[course_lr]).learning_resource single_result = build_program_children_content(program_lr) @@ -835,6 +872,8 @@ def test_build_program_children_content_bulk_multiple_programs(): course2 = CourseFactory.create( learning_resource__title="Course For Prog2" ).learning_resource + _add_course_marketing_page(course1, "Prog1 course copy.") + _add_course_marketing_page(course2, "Prog2 course copy.") prog1, prog2 = ( ProgramFactory.create(courses=[course]).learning_resource for course in [course1, course2] @@ -843,9 +882,9 @@ def test_build_program_children_content_bulk_multiple_programs(): result = build_program_children_content_bulk([prog1, prog2]) assert prog1.id in result assert prog2.id in result - assert "Course For Prog1" in result[prog1.id] - assert "Course For Prog2" in result[prog2.id] - assert "Course For Prog2" not in result[prog1.id] + assert "Prog1 course copy." in result[prog1.id] + assert "Prog2 course copy." in result[prog2.id] + assert "Prog2 course copy." not in result[prog1.id] def test_build_program_children_content_bulk_mixed_resources(): @@ -853,6 +892,7 @@ def test_build_program_children_content_bulk_mixed_resources(): course_child = CourseFactory.create( learning_resource__title="Child Course" ).learning_resource + _add_course_marketing_page(course_child, "Child course copy.") program_lr = ProgramFactory.create(courses=[course_child]).learning_resource non_program = CourseFactory.create().learning_resource @@ -868,6 +908,7 @@ def test_build_program_children_content_bulk_with_grandchildren(): grandchild = CourseFactory.create( learning_resource__title="Grandchild Course" ).learning_resource + _add_course_marketing_page(grandchild, "Grandchild copy.") child_prog = ProgramFactory.create( courses=[grandchild], learning_resource__title="Sub Program" ).learning_resource @@ -877,8 +918,8 @@ def test_build_program_children_content_bulk_with_grandchildren(): ) result = build_program_children_content_bulk([parent_lr]) - assert "Sub Program" in result[parent_lr.id] - assert "Grandchild Course" in result[parent_lr.id] + assert "### Grandchild Course" in result[parent_lr.id] + assert "Grandchild copy." in result[parent_lr.id] def test_build_program_children_content_excludes_unpublished_children( @@ -902,6 +943,8 @@ def test_build_program_children_content_excludes_unpublished_grandchildren(): is_unpublished=True, learning_resource__title="Unpublished Grandchild", ).learning_resource + _add_course_marketing_page(published_grandchild, "Published grandchild copy.") + _add_course_marketing_page(unpublished_grandchild, "Unpublished grandchild copy.") child_prog = ProgramFactory.create( courses=[published_grandchild], learning_resource__title="Sub Program" ).learning_resource @@ -921,53 +964,34 @@ def test_build_program_children_content_excludes_unpublished_grandchildren(): assert "Unpublished Grandchild" not in result[parent_lr.id] -def test_build_program_children_content_bulk_excludes_unpublished_contentfiles(): - """Unpublished content files and files on unpublished runs are excluded.""" - from learning_resources.models import ContentFile, LearningResourceRun +def test_build_program_children_content_bulk_excludes_unpublished_marketing_pages(): + """Only published marketing-page content files are included.""" + from learning_resources.models import ContentFile - course_lr = CourseFactory.create( - learning_resource__title="Course With Mixed Content" + published_course = CourseFactory.create( + learning_resource__title="Published Marketing Course" ).learning_resource + _add_course_marketing_page(published_course, "Visible marketing copy.") - published_run = LearningResourceRun.objects.create( - learning_resource=course_lr, - run_id="published-run", - published=True, - ) - unpublished_run = LearningResourceRun.objects.create( - learning_resource=course_lr, - run_id="unpublished-run", - published=False, - ) - - # Published content file on published run — should be included - ContentFile.objects.create( - run=published_run, - key="visible.txt", - summary="Visible summary", - published=True, - ) - # Unpublished content file on published run — should be excluded + unpublished_course = CourseFactory.create( + learning_resource__title="Unpublished Marketing Course" + ).learning_resource ContentFile.objects.create( - run=published_run, - key="hidden.txt", - summary="Hidden unpublished summary", + learning_resource=unpublished_course, + file_type=MARKETING_PAGE_FILE_TYPE, + file_extension=".md", + key=f"mktg-{unpublished_course.id}", + content="Hidden marketing copy.", published=False, ) - # Published content file on unpublished run — should be excluded - ContentFile.objects.create( - run=unpublished_run, - key="hidden-run.txt", - summary="Hidden run summary", - published=True, - ) - program_lr = ProgramFactory.create(courses=[course_lr]).learning_resource + program_lr = ProgramFactory.create( + courses=[published_course, unpublished_course] + ).learning_resource result = build_program_children_content_bulk([program_lr]) - assert "Visible summary" in result[program_lr.id] - assert "Hidden unpublished summary" not in result[program_lr.id] - assert "Hidden run summary" not in result[program_lr.id] + assert "Visible marketing copy." in result[program_lr.id] + assert "Hidden marketing copy." not in result[program_lr.id] @pytest.mark.parametrize( diff --git a/main/settings.py b/main/settings.py index 0635bd2183..de82fc0aec 100644 --- a/main/settings.py +++ b/main/settings.py @@ -35,7 +35,7 @@ from main.settings_pluggy import * # noqa: F403 from openapi.settings_spectacular import open_spectacular_settings -VERSION = "0.73.1" +VERSION = "0.73.2" log = logging.getLogger() @@ -140,48 +140,10 @@ "ol_hubspot", "mitol.scim.apps.ScimApp", "health_check", - "health_check.cache", - "health_check.contrib.migrations", - "health_check.contrib.celery_ping", - "health_check.contrib.redis", - "health_check.contrib.db_heartbeat", ) WEBHOOK_SECRET = get_string("WEBHOOK_SECRET", "please-change-this") -HEALTH_CHECK = { - "SUBSETS": { - # The 'startup' subset includes checks that must pass before the application can - # start. - "startup": [ - "MigrationsHealthCheck", # Ensures database migrations are applied. - "CacheBackend", # Verifies the cache backend is operational. - "RedisHealthCheck", # Confirms Redis is reachable and functional. - "DatabaseHeartBeatCheck", # Checks the database connection is alive. - ], - # The 'liveness' subset includes checks to determine if the application is - # running. - "liveness": ["DatabaseHeartBeatCheck"], # Minimal check to ensure the app is - # alive. - # The 'readiness' subset includes checks to determine if the application is - # ready to serve requests. - "readiness": [ - "CacheBackend", # Ensures the cache is ready for use. - "RedisHealthCheck", # Confirms Redis is ready for use. - "DatabaseHeartBeatCheck", # Verifies the database is ready for queries. - ], - # The 'full' subset includes all available health checks for a comprehensive - # status report. - "full": [ - "MigrationsHealthCheck", # Ensures database migrations are applied. - "CacheBackend", # Verifies the cache backend is operational. - "RedisHealthCheck", # Confirms Redis is reachable and functional. - "DatabaseHeartBeatCheck", # Checks the database connection is alive. - "CeleryPingHealthCheck", # Verifies Celery workers are responsive. - ], - } -} - if not get_bool("RUN_DATA_MIGRATIONS", default=False): MIGRATION_MODULES = {"data_fixtures": None} diff --git a/main/urls.py b/main/urls.py index dda4804b82..391ffa9ae2 100644 --- a/main/urls.py +++ b/main/urls.py @@ -17,7 +17,7 @@ from django.conf import settings from django.conf.urls.static import static from django.contrib import admin -from django.urls import include, re_path +from django.urls import include, path, re_path from django.views.generic.base import RedirectView from rest_framework.routers import DefaultRouter @@ -60,7 +60,7 @@ re_path(r"", include("webhooks.urls", namespace="webhooks")), re_path(r"", include(features_router.urls)), re_path(r"^app", RedirectView.as_view(url=settings.APP_BASE_URL)), - re_path(r"^health/", include("health_check.urls")), + path("", include("main.urls_healthcheck")), ] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT) + static(settings.STATIC_URL, document_root=settings.STATIC_ROOT) diff --git a/main/urls_healthcheck.py b/main/urls_healthcheck.py new file mode 100644 index 0000000000..e4f95e3980 --- /dev/null +++ b/main/urls_healthcheck.py @@ -0,0 +1,55 @@ +"""Healthcheck urls""" + +from django.urls import include, path +from health_check.views import HealthCheckView + +MIGRATIONS_CHECK = "health_check.contrib.migrations.backends.MigrationsHealthCheck" + +BASE_CHECKS = [ + # "default" is in-memory and always available; "redis" is the real backing cache. + ("health_check.Cache", {"alias": "redis"}), + "health_check.Database", + "health_check.contrib.redis.Redis", +] + +urlpatterns = [ + path( + "health/", + include( + [ + path( + "", + HealthCheckView.as_view( + checks=[ + *BASE_CHECKS, + MIGRATIONS_CHECK, + "health_check.contrib.celery.Ping", + ] + ), + ), + path( + "startup/", + HealthCheckView.as_view(checks=[*BASE_CHECKS, MIGRATIONS_CHECK]), + ), + path( + "liveness/", + HealthCheckView.as_view(checks=["health_check.Database"]), + ), + path( + "readiness/", + HealthCheckView.as_view(checks=[*BASE_CHECKS]), + ), + path( + "full/", + HealthCheckView.as_view( + checks=[ + *BASE_CHECKS, + MIGRATIONS_CHECK, + "health_check.contrib.celery.Ping", + ] + ), + ), + ] + ), + ), +] diff --git a/main/urls_healthcheck_test.py b/main/urls_healthcheck_test.py new file mode 100644 index 0000000000..24afbdc199 --- /dev/null +++ b/main/urls_healthcheck_test.py @@ -0,0 +1,23 @@ +"""Tests for the healthcheck urls""" + +import pytest +from django.urls import get_resolver +from health_check.views import HealthCheckView + + +@pytest.mark.parametrize( + "path", + [ + "/health/", + "/health/startup/", + "/health/liveness/", + "/health/readiness/", + "/health/full/", + ], +) +def test_healthcheck_urls_resolve(path): + """All healthcheck endpoints should resolve to HealthCheckView""" + # Resolve directly against this urlconf module rather than django.urls.resolve() + # (which walks the full project urlconf) to avoid pulling in unrelated apps. + match = get_resolver("main.urls_healthcheck").resolve(path) + assert match.func.view_class is HealthCheckView diff --git a/pyproject.toml b/pyproject.toml index bbdc431c71..7fe8ad81ac 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -32,7 +32,7 @@ dependencies = [ "django-cors-headers>=4.0.0,<5", "django-filter>=2.4.0,<3", "django-guardian>=3.0.0,<4", - "django-health-check", + "django-health-check>=3.24.0,<4", "django-imagekit>=6.0.0,<7", "django-ipware>=7.0.0,<8", "django-json-widget>=2.0.0,<3", @@ -88,7 +88,14 @@ dependencies = [ "python-dateutil>=2.8.2,<3", "python-rapidjson>=1.8,<2", "pyyaml>=6.0.0,<7", - "qdrant-client[fastembed]==1.18.0,<2", + # Minimum server version for the Query API (QueryResponse gRPC type): qdrant v1.7.0. + # qdrant-client ~=1.18.0 targets qdrant server v1.18.x; deployed server is v1.18.2 + # (ol-infrastructure/src/bridge/lib/versions.py QDRANT_VERSION). + # The prior ==1.18.0 pin blocked automatic pickup of future 1.18.x patch releases; + # switching to ~= lets uv upgrade within 1.18.x when the qdrant team publishes a + # client that resolves the gRPC QueryResponse DecodeError observed against server + # v1.18.2 with cloud_inference=True + prefer_grpc=True. + "qdrant-client[fastembed]~=1.18.0", "redis>=7.0.0,<8", "requests>=2.31.0,<3", "retry2>=0.9.5,<0.10", @@ -154,9 +161,6 @@ package = false default-groups = "all" override-dependencies = ["setuptools<80"] -[tool.uv.sources] -django-health-check = { git = "https://github.com/revsys/django-health-check", rev = "53f9bdc3a7acc8a577319987fef0bd3040eef4b4" } # pragma: allowlist secret - [tool.uv.build-backend] module-root = "" diff --git a/scripts/run-django-dev.sh b/scripts/run-django-dev.sh index 0a453be99a..c1c175f99e 100755 --- a/scripts/run-django-dev.sh +++ b/scripts/run-django-dev.sh @@ -7,7 +7,8 @@ python3 manage.py collectstatic --noinput --clear # run initial django migrations python3 manage.py migrate --noinput -granian --interface asginl --host 0.0.0.0 --port "${PORT:-8061}" --workers "${GRANIAN_WORKERS:-3}" --blocking-threads 1 --reload --reload-ignore-dirs frontends --reload-ignore-dirs staticfiles --reload-ignore-dirs .git main.asgi:application & +# --workers-kill-timeout: workers sometimes hang during graceful stop, wedging --reload. +granian --interface asginl --host 0.0.0.0 --port "${PORT:-8061}" --workers "${GRANIAN_WORKERS:-3}" --workers-kill-timeout "${GRANIAN_WORKERS_KILL_TIMEOUT:-1}" --blocking-threads 1 --reload --reload-ignore-dirs frontends --reload-ignore-dirs staticfiles --reload-ignore-dirs .git main.asgi:application & GRANIAN_PID=$! echo "Application started with PID $GRANIAN_PID" diff --git a/uv.lock b/uv.lock index 1bed38aa9e..9036a099de 100644 --- a/uv.lock +++ b/uv.lock @@ -833,10 +833,15 @@ wheels = [ [[package]] name = "django-health-check" -version = "3.20.1.dev10+g53f9bdc3a" -source = { git = "https://github.com/revsys/django-health-check?rev=53f9bdc3a7acc8a577319987fef0bd3040eef4b4#53f9bdc3a7acc8a577319987fef0bd3040eef4b4" } +version = "3.24.0" +source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "django" }, + { name = "psutil" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/98/a6/f474f443b0a7f9e7e52a25a3773a31c5c061bdb28cf311b7801fc250db9b/django_health_check-3.24.0.tar.gz", hash = "sha256:b5e01d2013a254cc5a2c7b19c62f11ea6fd2624c87a9d990e11a1b3874df78b5", size = 20807, upload-time = "2026-02-12T10:02:18.482Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/38/17/51aabb4908e007accf3c596b92a62ed7e456c581a1e45f06e0f2219dc6c0/django_health_check-3.24.0-py3-none-any.whl", hash = "sha256:bd568d7d3813980668dfbe730214aef5da7c8da73bc2aa60ec2eeca8a3788da6", size = 37964, upload-time = "2026-02-12T10:02:17.08Z" }, ] [[package]] @@ -2705,7 +2710,7 @@ requires-dist = [ { name = "django-cors-headers", specifier = ">=4.0.0,<5" }, { name = "django-filter", specifier = ">=2.4.0,<3" }, { name = "django-guardian", specifier = ">=3.0.0,<4" }, - { name = "django-health-check", git = "https://github.com/revsys/django-health-check?rev=53f9bdc3a7acc8a577319987fef0bd3040eef4b4" }, + { name = "django-health-check", specifier = ">=3.24.0,<4" }, { name = "django-imagekit", specifier = ">=6.0.0,<7" }, { name = "django-ipware", specifier = ">=7.0.0,<8" }, { name = "django-json-widget", specifier = ">=2.0.0,<3" }, @@ -2773,7 +2778,7 @@ requires-dist = [ { name = "python-dateutil", specifier = ">=2.8.2,<3" }, { name = "python-rapidjson", specifier = ">=1.8,<2" }, { name = "pyyaml", specifier = ">=6.0.0,<7" }, - { name = "qdrant-client", extras = ["fastembed"], specifier = "==1.18.0,<2" }, + { name = "qdrant-client", extras = ["fastembed"], specifier = "~=1.18.0" }, { name = "redis", specifier = ">=7.0.0,<8" }, { name = "requests", specifier = ">=2.31.0,<3" }, { name = "retry2", specifier = ">=0.9.5,<0.10" }, diff --git a/vector_search/utils.py b/vector_search/utils.py index 4a9638a678..dfb929804c 100644 --- a/vector_search/utils.py +++ b/vector_search/utils.py @@ -704,9 +704,16 @@ def _generate_content_file_points(serialized_content): 300,000 tokens per request max array size: 2048 see: https://platform.openai.com/docs/guides/rate-limits + + The 0.9 factor leaves headroom: markdown header prefixes are prepended + after the chunk-size split, so real chunks can exceed the nominal size. """ - request_chunk_size = int( - 300000 / settings.CONTENT_FILE_EMBEDDING_CHUNK_SIZE_OVERRIDE + request_chunk_size = max( + 1, + min( + 2048, + int(300000 * 0.9 / settings.CONTENT_FILE_EMBEDDING_CHUNK_SIZE_OVERRIDE), + ), ) for doc in serialized_content: diff --git a/vector_search/utils_test.py b/vector_search/utils_test.py index ac5bfbfeae..bd8da07e20 100644 --- a/vector_search/utils_test.py +++ b/vector_search/utils_test.py @@ -754,6 +754,101 @@ def test_generate_content_points_uses_standard_chunking_for_non_markdown(mocker) mock_md_chunk.assert_not_called() +def test_generate_content_points_leaves_headroom_under_token_limit(mocker): + """ + Embedding request batches must leave headroom under OpenAI's 300k + tokens-per-request limit, since markdown header prefixes are prepended + after the chunk-size split and inflate chunks past the nominal size + """ + settings.CONTENT_FILE_EMBEDDING_CHUNK_SIZE_OVERRIDE = 500 + settings.CONTENT_FILE_EMBEDDING_CHUNK_OVERLAP = 50 + + # 600 chunks * 500 tokens == exactly 300k if packed with no headroom + num_chunks = 600 + mocker.patch( + "vector_search.utils._chunk_documents", + return_value=[ + Document(page_content=f"chunk{i}", metadata={"key": "k1"}) + for i in range(num_chunks) + ], + ) + mocker.patch( + "vector_search.utils.should_generate_content_embeddings", return_value=True + ) + mocker.patch("vector_search.utils.remove_points_matching_params") + + mock_dense = mocker.MagicMock() + mock_dense.embed_documents.side_effect = lambda texts: [[0.1] for _ in texts] + mock_dense.model_short_name.return_value = "dense" + mock_sparse = mocker.MagicMock() + mock_sparse.embed_documents.side_effect = lambda texts: [[0.2] for _ in texts] + mock_sparse.model_short_name.return_value = "sparse" + mocker.patch("vector_search.utils.dense_encoder", return_value=mock_dense) + mocker.patch("vector_search.utils.sparse_encoder", return_value=mock_sparse) + + doc = { + "content": "Some plain text content", + "file_type": "page", + "file_extension": ".html", + "platform": {"code": "x"}, + "resource_readable_id": "r1", + "run_readable_id": "run1", + "key": "k1", + } + + points = list(_generate_content_file_points([doc])) + + batch_sizes = [ + len(call.args[0]) for call in mock_dense.embed_documents.call_args_list + ] + assert sum(batch_sizes) == num_chunks + assert len(points) == num_chunks + # nominal tokens per request must stay at least ~5% under the 300k limit + assert max(batch_sizes) * 500 <= 285000 + + +def test_generate_content_points_request_chunk_size_never_zero(mocker): + """ + A misconfigured (huge) chunk-size override must not make request_chunk_size 0, + which would raise ValueError in the range() batching loop + """ + settings.CONTENT_FILE_EMBEDDING_CHUNK_SIZE_OVERRIDE = 500000 + settings.CONTENT_FILE_EMBEDDING_CHUNK_OVERLAP = 50 + + mocker.patch( + "vector_search.utils._chunk_documents", + return_value=[ + Document(page_content=f"chunk{i}", metadata={"key": "k1"}) for i in range(3) + ], + ) + mocker.patch( + "vector_search.utils.should_generate_content_embeddings", return_value=True + ) + mocker.patch("vector_search.utils.remove_points_matching_params") + + mock_dense = mocker.MagicMock() + mock_dense.embed_documents.side_effect = lambda texts: [[0.1] for _ in texts] + mock_dense.model_short_name.return_value = "dense" + mock_sparse = mocker.MagicMock() + mock_sparse.embed_documents.side_effect = lambda texts: [[0.2] for _ in texts] + mock_sparse.model_short_name.return_value = "sparse" + mocker.patch("vector_search.utils.dense_encoder", return_value=mock_dense) + mocker.patch("vector_search.utils.sparse_encoder", return_value=mock_sparse) + + doc = { + "content": "Some plain text content", + "file_type": "page", + "file_extension": ".html", + "platform": {"code": "x"}, + "resource_readable_id": "r1", + "run_readable_id": "run1", + "key": "k1", + } + + points = list(_generate_content_file_points([doc])) + assert len(points) == 3 + + def test_course_metadata_indexed_with_learning_resources(mocker): # test the we embed a metadata document when embedding learning resources resources = LearningResourceFactory.create_batch(5) diff --git a/yarn.lock b/yarn.lock index 2472a444c9..c14da1cb53 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3360,13 +3360,13 @@ __metadata: languageName: node linkType: hard -"@mitodl/mitxonline-api-axios@npm:2026.6.30-1": - version: 2026.6.30-1 - resolution: "@mitodl/mitxonline-api-axios@npm:2026.6.30-1" +"@mitodl/mitxonline-api-axios@npm:2026.7.7": + version: 2026.7.7 + resolution: "@mitodl/mitxonline-api-axios@npm:2026.7.7" dependencies: "@types/node": "npm:^20.11.19" axios: "npm:^1.6.5" - checksum: 10/cb5ba8cabe66e9ace0902c96cba527052263b1599a9909d9a41a7e3cff5022c907bc2994712a9e6a055edc2307a19518fd606df91c70a8efc47457842ed5b4f8 + checksum: 10/f2aace07689d4b9b96758c6b07118be4d94cc552b48c45503e92461112f266dc876864eff76a4f4339710082e794f941fd7a5d649a3bff65b2380ad2ee3c71a7 languageName: node linkType: hard @@ -9092,7 +9092,7 @@ __metadata: resolution: "api@workspace:frontends/api" dependencies: "@faker-js/faker": "npm:^10.0.0" - "@mitodl/mitxonline-api-axios": "npm:2026.6.30-1" + "@mitodl/mitxonline-api-axios": "npm:2026.7.7" "@tanstack/react-query": "npm:^5.66.0" "@testing-library/react": "npm:^16.3.0" axios: "npm:^1.12.2" @@ -16320,7 +16320,7 @@ __metadata: "@mitodl/arithmix": "npm:^0.2.2" "@mitodl/course-search-utils": "npm:^3.5.2" "@mitodl/hacksnack": "npm:^0.1.0" - "@mitodl/mitxonline-api-axios": "npm:2026.6.30-1" + "@mitodl/mitxonline-api-axios": "npm:2026.7.7" "@mitodl/smoot-design": "npm:^6.27.0" "@mui/base": "npm:5.0.0-beta.70" "@mui/material": "npm:^6.4.5"