Repository navigation
fix(vector-io): return the page right before the cursor when paginating with before - #6813
Conversation
…ng with before The vector store list endpoints dropped everything from the `before` cursor onwards and then kept the head of what was left, so a `before` request always returned the first page of the list instead of the page immediately preceding the cursor. Walking back from the third page therefore jumped to the first one. When the cursor is found, the page is now taken from the items closest to it, matching the OpenAI "fetch the previous page" semantics and the existing `anthropic_list_models` implementation. An unknown `before` cursor keeps its current behavior. The fix covers the router's vector store listing and the mixin's vector store, vector store file and file batch listings, with unit tests for each. Signed-off-by: Nicoló Angileri <[email protected]>
There was a problem hiding this comment.
I ran this and it fixes the paging. The PR head was behind main when I looked, so I applied the PR diff (gh pr diff) cleanly onto current main (fetched 2026-10-16) and compared that with a clean main checkout. Python 3.13.3.
Property probe on OpenAIVectorStoreMixin.openai_list_vector_stores (a small mixin subclass, no mocks of the list code). For n = 1 to 8 stores, limit = 1 to 4 and both orders (64 cases), I paged forward with after=last_id to get the pages. Then I walked backwards from the first id of the last page with before=first_id until has_more was false, and compared the pages.
- main: 24 of 64 cases do not match. Example n=3, limit=1, asc: forward pages are
[vs_1] [vs_2] [vs_3], the backward walk gives[vs_1] [vs_3]. - PR on main: 0 of 64 mismatches.
Other cursor cases (5 stores, limit 2, asc), the same on main and the PR:
- unknown
before="nope": returnsvs_1, vs_2withhas_more=True(behaves like no cursor) before="vs_1": empty,has_more=Falseafter="vs_2", before="vs_5":vs_3, vs_4
Tests: with the PR diff on main, the three files test_openai_vector_store_mixin.py, test_vector_io.py (router) and test_vector_io_stores_batches.py give 81 passed and 15 errors. All 15 errors are ModuleNotFoundError: faiss in the batches file, which includes the new test_file_batch_pagination_before_cursor, so I did not run that test. When I copy the PR's versions of the first two test files onto main, 3 fail (the router test and the two mixin tests) and 33 pass, so those tests catch the bug.
Limits: the batch file listing (openai_list_files_in_vector_store_file_batch) is read-only, I only read the same two line change there. My property probe covered the vector store listing in the mixin. The files in a store listing and the router listing were run only through the PR's own tests.
…fore-pagination Signed-off-by: Nicoló Angileri <[email protected]>
…fore-pagination Signed-off-by: Nicoló Angileri <[email protected]>
|
Hi, a gentle ping on this one when someone has a moment. @VANDRANKI tested the fix on current main and approved it, and it still merges cleanly. I'm happy to rebase or adjust if needed. Thanks! |
|
Hi @skamenan7, would you have time to review this one? |
mattf
left a comment
There was a problem hiding this comment.
@nicoloangileri thank you
What does this PR do?
beforepagination on the vector store list endpoints always returns the first page of the list, not the page right before the cursor.The listings filter out everything from the
beforecursor onwards and then slice[:limit], so they keep the head of what's left instead of the items closest to the cursor. With six vector stores andlimit=2, paging forward withaftergives[vs_1, vs_2],[vs_3, vs_4],[vs_5, vs_6], butbefore=vs_5returns[vs_1, vs_2]instead of[vs_3, vs_4]. A client walking backwards jumps straight to the start. The OpenAI docs describebefore=obj_fooas the way "to fetch the previous page of the list", andanthropic_list_modelsincore/routing_tables/models.pyalready does it that way (start_index = max(0, end_index - page_limit)).When the
beforecursor is found, the page is now[-limit:]of the remaining items.has_moreis unchanged: it was alreadylen(remaining) > limit, which is correct for the backward direction. An unknownbeforecursor keeps today's behavior and returns the head of the list.Affected listings (same pattern in each):
VectorIORouter.openai_list_vector_stores(the code path behindGET /v1/vector_stores)OpenAIVectorStoreMixin.openai_list_vector_storesOpenAIVectorStoreMixin.openai_list_files_in_vector_store(behindGET /v1/vector_stores/{id}/files)OpenAIVectorStoreMixin.openai_list_files_in_vector_store_file_batch(behindGET /v1/vector_stores/{id}/file_batches/{batch_id}/files)Not changed here:
ResponsesStore.list_response_input_itemshas the same slicing, buttest_responses_store_input_items_before_paginationasserts the current output (before="before-5", limit=2in ascending order ->[First, Second]), so it seemed better to ask before changing that behavior. I can open a follow-up if you agree it should match.Test Plan
New unit tests, one per fixed listing. All of them fail on
mainwith the head-of-list page (for exampleassert ['vs_1', 'vs_2'] == ['vs_3', 'vs_4']) and pass with this change:tests/unit/core/routers/test_vector_io.py::test_list_vector_stores_before_cursor_returns_previous_pagetests/unit/providers/utils/memory/test_openai_vector_store_mixin.py::TestBeforeCursorPagination(vector stores and vector store files)tests/unit/providers/vector_io/test_vector_io_stores_batches.py::test_file_batch_pagination_before_cursor(sqlite_vec, faiss, pgvector, qdrant)Reproduction against the router and the mixin (mocked storage only):
pre-commit run --files <changed files>passes exceptmypy, which reports the same 4 errors on untouchedmainin this environment (src/ogx_api/filters.py:73and three inopenai_responses.py). This diff adds no new ones.