Skip to content

♻️(search) migrate to opensearch-py Q-types#130

Closed
StephanMeijer wants to merge 2 commits into
mainfrom
feat/switch-opensearch-py-dsl
Closed

♻️(search) migrate to opensearch-py Q-types#130
StephanMeijer wants to merge 2 commits into
mainfrom
feat/switch-opensearch-py-dsl

Conversation

@StephanMeijer
Copy link
Copy Markdown
Collaborator

@StephanMeijer StephanMeijer commented May 8, 2026

Important

Please merge first: #129, #127, #126, #125, #124, #123, #138

Important

Moved to #139

Summary

  • Refactor search.py to use Q objects from opensearch-py for type-safe query building
  • Add SortSpec dataclass for DSL-like sort clause construction
  • Add comprehensive test suite (84 tests total: 53 unit + 31 integration)
  • Remove all typing.Any usage in favor of proper types

Changes

File Description
src/backend/core/services/search.py Migrated to Q objects, added SortSpec
src/backend/core/views.py Deletion query migrated to Q objects
src/backend/core/tests/test_search.py 53 new unit tests
src/backend/core/tests/test_api_search.py 13 new integration tests
src/backend/core/tests/test_api_documents_delete.py 18 deletion tests

Verification

  • ✅ All 84 tests passing
  • ✅ 100% behavioral equivalence maintained
  • ✅ Access control logic preserved (security critical)
  • ✅ No changes to indexing.py, schemas, or script_fields

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the backend OpenSearch query-building layer to use opensearch-py Q objects (instead of raw dicts), adds a small sort DSL wrapper, and introduces a broad set of unit/integration tests to lock in access-control and query-shape behavior.

Changes:

  • Migrated search query construction (get_filter, get_query, get_full_text_query) to opensearchpy.Q and introduced SortSpec for sort clause generation.
  • Updated the document deletion query builder to use Q objects and serialize via .to_dict().
  • Added extensive unit + API integration test coverage for query building, access control, filtering, sorting, and deletion behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/backend/core/services/search.py Rebuilds search/filter/query/sort composition using opensearchpy.Q and SortSpec, serializing to dict at request time.
src/backend/core/views.py Updates deletion query construction to build with Q objects and serialize to dict for OpenSearch.
src/backend/core/tests/test_search.py Adds a large unit test suite for query building, access control filtering, and sort spec behavior.
src/backend/core/tests/test_api_search.py Adds integration tests for the search API endpoint (auth, filtering, sorting, access control).
src/backend/core/tests/test_api_documents_delete.py Adds deletion-query unit tests alongside existing deletion endpoint integration tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/backend/core/tests/test_search.py
Comment thread src/backend/core/tests/test_search.py Outdated
Comment thread src/backend/core/tests/test_api_documents_delete.py Outdated
Comment thread src/backend/core/views.py
@StephanMeijer StephanMeijer force-pushed the feat/switch-opensearch-py-dsl branch 3 times, most recently from 53d8977 to bc91921 Compare May 8, 2026 14:25
@StephanMeijer StephanMeijer force-pushed the feat/switch-opensearch-py-dsl branch 4 times, most recently from c52cb70 to b8227f0 Compare May 11, 2026 15:02
@StephanMeijer StephanMeijer added this to the LaSuite Find Staging milestone May 12, 2026
Signed-off-by: Stephan Meijer <me@stephanmeijer.com>
Signed-off-by: Stephan Meijer <me@stephanmeijer.com>
@StephanMeijer StephanMeijer force-pushed the feat/switch-opensearch-py-dsl branch from 147ae1c to 0aad4f5 Compare May 13, 2026 15:59
@StephanMeijer
Copy link
Copy Markdown
Collaborator Author

Important

Moved to #139

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.

2 participants