Repository navigation
fix(docs): forward top_n to rerank in gen-qa-openai notebook - #628
Open
RaphaelFakhri wants to merge 1 commit into
Open
RaphaelFakhri wants to merge 1 commit into
RaphaelFakhri wants to merge 1 commit into
Conversation
rerank() accepted a top_n argument but always sent top_n=10 to pc.inference.rerank, so cascading_retrieval() and rag() returned 10 papers no matter which top_n the caller passed. Pass the argument through.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
In
docs/gen-qa-openai.ipynb,rerank(query, results, top_n=5)ignores itstop_nargument and always sendstop_n=10topc.inference.rerank. As a result:cascading_retrieval(..., top_n=5)in the "Cascading Retrieval" section returns 10 papers instead of 5.rag(query)asks fortop_n=5, but the prompt sent togpt-4o-minicontains 10 papers, which doubles the context size and token cost and contradicts the notebook text ("returning the top_n results").Solution
Pass the
top_nargument through topc.inference.rerank. This is a one-line change.Type of change
Test plan
The repository has no unit test harness for notebooks, so the cell that defines
query_pinecone,dedupe,rerankandcascading_retrievalwas executed against stubpcand index objects that record the arguments sent topc.inference.rerank:top_n=3andtop_n=5both sendtop_n=10and return 10 results (0 passed, 2 failed).top_n=3returns 3 results andtop_n=5returns 5 results (2 passed, 0 failed).ruff check,ruff format --check,codespelland the four notebook checks from the lint workflow (check-secrets,check-structure,check-timeless,check-pinning) pass on the changed notebook.Note
Low Risk
Documentation-only notebook fix with no production code or security impact.
Overview
Fixes a bug in the gen-qa-openai notebook where
rerank()accepted atop_nargument but always calledpc.inference.rerankwith a hardcodedtop_n=10.The call now passes through the function’s
top_nparameter, socascading_retrievalandraghonor values liketop_n=5instead of returning ten papers and inflating the RAG prompt sent togpt-4o-mini.Reviewed by Cursor Bugbot for commit 6d03772. Bugbot is set up for automated code reviews on this repo. Configure here.