Skip to content

Conversation

@TamiTakamiya
Copy link
Contributor

Description

Referenced documents support on /streaming_query

The code assumes that the vector DB contains docs_url and title in metadata. They are found in the vector DB built for road-core/service.

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement

Related Tickets & Documents

  • Related Issue # n/a
  • Closes # n/a

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • Please provide detailed steps to perform tests related to this code change.
  • How were the fix/results from this change verified? Please provide relevant screenshots or results.

Copy link
Contributor

@manstis manstis left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Thanks @TamiTakamiya

IDK if @tisnik or @umago want to support customised meta-data longer-term but this certainly solves our immediate use case for Referenced Documents.

Does this also need adding to non-stream support?

@TamiTakamiya TamiTakamiya force-pushed the TamiTakamiya/referenced_docs_support branch from aef46e6 to 9c4f5b7 Compare July 3, 2025 19:49
@TamiTakamiya TamiTakamiya marked this pull request as ready for review July 3, 2025 19:52
Copy link
Contributor

@umago umago left a comment

Choose a reason for hiding this comment

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

Overall good, suggestion inline

Copy link
Contributor

@tisnik tisnik left a comment

Choose a reason for hiding this comment

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

it looks ok, thank you. Just please:

  1. rebase
  2. add the check mentioned by @umago to make sure the service won't fail with NPE

Then we can merge. Thank you in advance.

@TamiTakamiya TamiTakamiya force-pushed the TamiTakamiya/referenced_docs_support branch from 9c4f5b7 to b2385be Compare July 7, 2025 14:27
@TamiTakamiya
Copy link
Contributor Author

it looks ok, thank you. Just please:

  1. rebase
  2. add the check mentioned by @umago to make sure the service won't fail with NPE

Then we can merge. Thank you in advance.

@tisnik 1 & 2 are addressed with the latest commit. Thanks!

Copy link
Contributor

@umago umago left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the comment

Copy link
Contributor

@tisnik tisnik left a comment

Choose a reason for hiding this comment

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

nice, thank you

@tisnik tisnik merged commit 35feb6b into main Jul 7, 2025
31 checks passed
@TamiTakamiya TamiTakamiya deleted the TamiTakamiya/referenced_docs_support branch July 7, 2025 14:47
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.

5 participants