Skip to content

vector search: add course metadata to context - #3852

Open
shanbady wants to merge 5 commits into
mainfrom
shanbady/course-metadata-context-doc
Open

vector search: add course metadata to context#3852
shanbady wants to merge 5 commits into
mainfrom
shanbady/course-metadata-context-doc

Conversation

@shanbady

Copy link
Copy Markdown
Contributor

What are the relevant tickets?

Closes https://github.com/mitodl/hq/issues/13018

Description (What does it do?)

For learning resource hybrid search, we generate a snippet that is embedded (the "context snippet"). This previously consisted of a title, description, full description, course numbers (and contentfile content if it is a learning material)

This Pr makes it so that we use the metadata document (all the info in a learning resource drawer as markdown) as the context snippet. This allows for interesting searches like finding courses that are taught by specific instructors or searching "find me biology courses that offer certificates"

Screenshots (if appropriate):

hybrid search for "math courses that offer a certificate":
Screenshot 2026-08-26 at 11 14 40 AM

How can this be tested?

  1. checkout this branch
  2. re-run the embeddings for contentfiles with the overwrite flag
    docker compose run --rm web python manage.py generate_embeddings --all --skip-contentfiles --overwrite
  3. once complete go to the vector search interface
  4. you should be able to search by instructor name, course number and other things that may be found in the resource drawer.

Additional Context

NOTE: the results you see locally will not be as good as what we get upon deploying since we use a much better sparse vector (qdrant/bm25) on our deployed environments

@shanbady shanbady added the Needs Review An open Pull Request that is ready for review label Aug 27, 2026
@shanbady
shanbady marked this pull request as ready for review August 27, 2026 21:00
@shanbady
shanbady requested a review from a team as a code owner August 27, 2026 21:00
Copilot AI balanced review requested due to automatic review settings August 27, 2026 21:00
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown

OpenAPI Changes

No changes detected

View full changelog

Unexpected changes? Ensure your branch is up-to-date with main (consider rebasing).

@shanbady shanbady changed the title Shanbady/course metadata context doc vector search: add course metadata to context Aug 27, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates vector-search embedding context to include full learning-resource metadata.

Changes:

  • Adds markdown rendering for complete metadata documents.
  • Reuses serializer context across embedding batches.
  • Expands tests for metadata, fallback, content, and truncation behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
vector_search/utils.py Builds embedding context from metadata markdown.
vector_search/utils_test.py Tests expanded embedding-context behavior.
learning_resources/serializers.py Adds full metadata markdown rendering.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread vector_search/utils.py
Comment on lines 723 to +724
stored_embedding_content = _learning_resource_embedding_context(
resource_payload
resource_payload, serializer_context
Comment thread vector_search/utils.py
@mbertrand mbertrand self-assigned this Aug 28, 2026

@mbertrand mbertrand left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to this PR, but testing the branch I hit a 500 on every hybrid search — the staleness penalty from #3834 sends midpoint=0.0 and Qdrant rejects it:

"Wrong input: Validation failed: Decay midpoint should be between 0.0 and 1.0 (exclusive), got 0."

I had to set VECTOR_SEARCH_STALENESS_PENALTY_WEIGHT=0 in my backend .env file to get around it.

Comment thread vector_search/utils.py
resource_payload = response[0].payload
stored_embedding_content = _learning_resource_embedding_context(
resource_payload
resource_payload, serializer_context

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This gate keeps the new context from ever reaching the existing catalog: the stored payload is the full doc and gets refreshed every run, so unchanged resources render identically on both sides and keep their old title/description vectors. --overwrite doesn't bypass it (it only skips the existence pre-filter). Verified locally — refreshed payloads for the 19 resources my dev Qdrant knows about, gate skipped all 19.

Storing a checksum of what was actually embedded, plus a version constant to bump on format changes (same pattern as _embed_course_metadata_as_contentfile), might fix this

@mbertrand mbertrand Aug 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PS I tried regenerating embeddings from scratch by using the --recreate-collections flag. But when I searched for instructor names or course numbers, the wrong courses turned up.

Comment thread vector_search/utils.py
)
try:
return serializer.render_markdown()
except Exception:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This catches failures on the current doc too. If a fresh doc fails to render once (e.g. transient DB error), we embed the title/description fallback but store the full doc — next run both sides render fine, compare equal, and the degraded vector is never repaired (verified locally). I'd fall back only on the stored-payload side and let a current-doc failure skip-and-log; the checksum from my other comment also fixes this.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use a test that this returns False when the payload round-trips to the same render — the docs carry real datetimes, Qdrant hands back strings, and if the serializer ever stops treating that as identity we silently re-embed the whole catalog every run. Mock client.retrieve to return json.loads(json.dumps(doc, default=str)) and assert False.

FYI the fake payload in test_should_generate_for_changed_resource now exercises the render fallback path (no delivery → KeyError).

@mbertrand mbertrand added Waiting on author and removed Needs Review An open Pull Request that is ready for review labels Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants