Skip to content

Conversation

@uinstinct
Copy link
Contributor

@uinstinct uinstinct commented Jul 24, 2025

Description

The following error used to happen when the reranker model returned an invalid response.
Failed to rerank retrieval results TypeError: Cannot read properties of undefined (reading 'sort'). More about the RCA in the ticket.

  • add a zod validation schema for voyage ai response
  • check if scores array is valid in DocsContextProvider

resolves CON-2661

Checklist

  • [] I've read the contributing guide
  • [] The relevant docs, if any, have been updated or created
  • [] The relevant tests, if any, have been updated or created

Screen recording or screenshot

[ When applicable, please include a short screen recording or screenshot - this makes it much easier for us as contributors to review and understand your changes. See this PR as a good example. ]

Tests

[ What tests were added or updated to ensure the changes work as expected? ]


Summary by cubic

Fixed a crash when the reranker model returned an invalid scores array by adding response validation and handling invalid cases.

  • Bug Fixes
    • Added Zod schema validation for the Voyage AI reranker response.
    • Checked for a valid scores array before sorting in DocsContextProvider.

@uinstinct uinstinct requested a review from a team as a code owner July 24, 2025 13:44
@uinstinct uinstinct requested review from tingwai and removed request for a team July 24, 2025 13:44
@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Jul 24, 2025
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Jul 31, 2025
Copy link
Collaborator

@RomneyDa RomneyDa left a comment

Choose a reason for hiding this comment

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

@uinstinct see nitpick comment. Just to double check, has this been tested with voyage reranker?

data: Array<{ index: number; relevance_score: number }>;
};

VoyageRerankSuccessResponseSchema.parse(data);
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can use the zod output here, it will be typesafe!
I'd recommend safeParse so you can gracefully handle invalid values

Copy link
Contributor Author

@uinstinct uinstinct Aug 7, 2025

Choose a reason for hiding this comment

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

actually I wanted the throw the error directly if parsing failed like we do here

if (resp.status !== 200) {
throw new Error(
`VoyageReranker API error ${resp.status}: ${await resp.text()}`,
);
}

Copy link
Collaborator

@RomneyDa RomneyDa Aug 7, 2025

Choose a reason for hiding this comment

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

@uinstinct the schema.parse will throw an error if parsing fails. i.e. if the status is 200 then presumably the parse with throw an error because the response won't match the expected format, which means the status check will never be reached

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, got it.

@github-project-automation github-project-automation bot moved this from Todo to In Progress in Issues and PRs Aug 7, 2025
@uinstinct
Copy link
Contributor Author

Just to double check, has this been tested with voyage reranker?

Yes had verified on Voyage reranker

@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Aug 8, 2025
@RomneyDa
Copy link
Collaborator

RomneyDa commented Aug 8, 2025

@uinstinct sorry for the confusion,
I think the schema parser is a great idea, I just meant that the parsing error needs to be handled if it is parsed before the response status is checked

EDIT I was wrong about the order here, the main concern now is that the parse will throw an ugly zod error so should be gracefully handled

@uinstinct uinstinct requested a review from RomneyDa August 11, 2025 19:17
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Aug 11, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Aug 11, 2025
@RomneyDa RomneyDa merged commit a5d11ff into continuedev:main Aug 12, 2025
37 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Issues and PRs Aug 12, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Aug 12, 2025
@uinstinct uinstinct deleted the reranker-scores-error branch August 13, 2025 02:54
@sestinj
Copy link
Contributor

sestinj commented Aug 18, 2025

🎉 This PR is included in version 1.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@sestinj
Copy link
Contributor

sestinj commented Aug 18, 2025

🎉 This PR is included in version 1.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

lgtm This PR has been approved by a maintainer released size:M This PR changes 30-99 lines, ignoring generated files.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants