Skip to content

Favor doc_reconstruct when reconstruct_document is also mentioned#1210

Merged
dhruvkaliraman7 merged 1 commit into
mainfrom
Add-hierarchy-for-OS-doc_reconstruction
Mar 28, 2025
Merged

Favor doc_reconstruct when reconstruct_document is also mentioned#1210
dhruvkaliraman7 merged 1 commit into
mainfrom
Add-hierarchy-for-OS-doc_reconstruction

Conversation

@dhruvkaliraman7
Copy link
Copy Markdown
Contributor

No description provided.

@dhruvkaliraman7 dhruvkaliraman7 requested a review from Copilot March 5, 2025 00:34
Copy link
Copy Markdown
Contributor

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.

PR Overview

This PR modifies the OpenSearch reader initialization to favor the use of a document reconstructor over a boolean flag when both are provided.

  • Added a condition to disable the reconstruct_document flag when a doc_reconstructor is set.
  • Updated logging to inform users which configuration is being used.

Reviewed Changes

File Description
lib/sycamore/sycamore/connectors/opensearch/opensearch_reader.py Added a conditional block that logs and disables the reconstruct_document flag when a doc_reconstructor is provided to favor the latter.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

lib/sycamore/sycamore/connectors/opensearch/opensearch_reader.py:284

  • There is a naming inconsistency between 'reconstruct_document' and 'doc_reconstructor'. Consider aligning the variable names (e.g., using 'doc_reconstruct') to clearly reflect that the document reconstructor is favored when both are set.
if query_params.reconstruct_document and query_params.doc_reconstructor is not None:

@dhruvkaliraman7 dhruvkaliraman7 merged commit 5491677 into main Mar 28, 2025
@dhruvkaliraman7 dhruvkaliraman7 deleted the Add-hierarchy-for-OS-doc_reconstruction branch March 28, 2025 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants