fix: thread/list returning fewer than the requested amount due to filtering CXA-293 #7509

JaviSoto · 2025-12-03T00:47:14Z

This caused some conversations to not appear when they otherwise should.

Prior to this change, thread/list/list_conversations_common would:

Fetch N conversations from RolloutRecorder::list_conversations
Then it would filter those (like by the provided model_providers)
This would make it potentially return less than N items.

With this change:

list_conversations_common now continues fetching more conversations from RolloutRecorder::list_conversations until it "fills up" the requested_page_size.
Ultimately this means that clients can rely on getting eg 20 conversations if they request 20 conversations.

etraut-openai · 2025-12-09T19:48:22Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

codex-rs/app-server/src/codex_message_processor.rs

JaviSoto · 2025-12-09T21:22:52Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

codex-rs/app-server/tests/suite/v2/thread_list.rs

bolinfest · 2025-12-09T21:48:22Z

@JaviSoto can you please add more detail to the PR body as this code change is larger than I expect given the description

JaviSoto · 2025-12-10T00:17:27Z

@JaviSoto can you please add more detail to the PR body as this code change is larger than I expect given the description

@bolinfest totally fair! The diff looks a lot worse because of the test changes. It may be easier to review commit-by-commit and hiding whitespace changes. I updated the description too, let me know if that's enough detail or I can elaborate on it!

JaviSoto · 2025-12-10T00:18:38Z

@codex review

codex-rs/app-server/tests/suite/v2/thread_list.rs

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

codex-rs/app-server/src/codex_message_processor.rs

JaviSoto · 2025-12-10T02:03:21Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

codex-rs/app-server/src/codex_message_processor.rs

JaviSoto · 2025-12-10T20:41:01Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2025-12-10T20:50:27Z

codex-rs/app-server/src/codex_message_processor.rs

+            let next_cursor_value = page.next_cursor.clone();
+            next_cursor = next_cursor_value
+                .as_ref()
+                .and_then(|cursor| serde_json::to_value(cursor).ok())
+                .and_then(|value| value.as_str().map(str::to_owned));


Clear next_cursor when no further threads exist

list_conversations_common always forwards the cursor from the last page even after the requested page size has been satisfied. RolloutRecorder::list_conversations marks next_cursor as soon as a page fills (see core/src/rollout/list.rs where more_matches_available is set when items.len() == page_size), so a request that matches exactly limit conversations—e.g., three threads for a single provider when only three exist—returns a non-null next_cursor despite there being no additional results. Clients will keep paginating and receive an empty page, which contradicts the API contract that nextCursor should be None once results are exhausted.

Useful? React with 👍 / 👎.

Looked into this, and if that happens, you just request more with that cursor and then get an empty array, which is fine, and it seems that changing that behavior will just add more complexity so I think this is fine.

bolinfest

Please make sure all outstanding comments from codex review bot are addressed!

bolinfest · 2025-12-10T22:15:18Z

codex-rs/app-server/src/codex_message_processor.rs

+            let page = match RolloutRecorder::list_conversations(
+                &self.config.codex_home,
+                page_size,
+                cursor_obj.as_ref(),
+                INTERACTIVE_SESSION_SOURCES,
+                model_provider_filter.as_deref(),
+                fallback_provider.as_str(),
+            )
+            .await
+            {
+                Ok(p) => p,
+                Err(err) => {
+                    return Err(JSONRPCErrorError {
+                        code: INTERNAL_ERROR_CODE,
+                        message: format!("failed to list conversations: {err}"),
+                        data: None,
+                    });
+                }
+            };


protip

Suggested change

let page = match RolloutRecorder::list_conversations(

&self.config.codex_home,

page_size,

cursor_obj.as_ref(),

INTERACTIVE_SESSION_SOURCES,

model_provider_filter.as_deref(),

fallback_provider.as_str(),

)

.await

{

Ok(p) => p,

Err(err) => {

return Err(JSONRPCErrorError {

code: INTERNAL_ERROR_CODE,

message: format!("failed to list conversations: {err}"),

data: None,

});

}

};

let page = match RolloutRecorder::list_conversations(

&self.config.codex_home,

page_size,

cursor_obj.as_ref(),

INTERACTIVE_SESSION_SOURCES,

model_provider_filter.as_deref(),

fallback_provider.as_str(),

)

.await.map_err(|err| Err(JSONRPCErrorError {

code: INTERNAL_ERROR_CODE,

message: format!("failed to list conversations: {err}"),

data: None,

})?;

Updated, thank you!

…tering This caused some conversations to not appear when they otherwise should.

…mory

JaviSoto · 2025-12-10T22:51:50Z

Please make sure all outstanding comments from codex review bot are addressed!

This is the only outstanding one: #7509 (comment) do you agree with my response? Happy to change the behavior though.

JaviSoto force-pushed the dev/javi/thread-list-count branch from 3192da2 to fc620ac Compare December 3, 2025 00:55

JaviSoto marked this pull request as ready for review December 3, 2025 01:52

ambrosino-oai changed the title ~~fix: thread/list returning fewer than the requested amount due to filtering~~ fix: thread/list returning fewer than the requested amount due to filtering CXA-293 Dec 3, 2025

JaviSoto force-pushed the dev/javi/thread-list-count branch from fc620ac to 4253c31 Compare December 8, 2025 18:35

JaviSoto requested review from bolinfest and gabriel-openai December 8, 2025 22:06

chatgpt-codex-connector bot reviewed Dec 9, 2025

View reviewed changes

codex-rs/app-server/src/codex_message_processor.rs Outdated Show resolved Hide resolved

chatgpt-codex-connector bot reviewed Dec 9, 2025

View reviewed changes

codex-rs/app-server/tests/suite/v2/thread_list.rs Outdated Show resolved Hide resolved

JaviSoto force-pushed the dev/javi/thread-list-count branch 2 times, most recently from b2f35cc to a063271 Compare December 9, 2025 23:19

JaviSoto commented Dec 10, 2025

View reviewed changes

codex-rs/app-server/tests/suite/v2/thread_list.rs Show resolved Hide resolved

chatgpt-codex-connector bot reviewed Dec 10, 2025

View reviewed changes

codex-rs/app-server/src/codex_message_processor.rs Outdated Show resolved Hide resolved

JaviSoto force-pushed the dev/javi/thread-list-count branch 4 times, most recently from 09f9764 to 64affe5 Compare December 10, 2025 01:56

chatgpt-codex-connector bot reviewed Dec 10, 2025

View reviewed changes

codex-rs/app-server/src/codex_message_processor.rs Show resolved Hide resolved

JaviSoto force-pushed the dev/javi/thread-list-count branch from 64affe5 to 78c9ab5 Compare December 10, 2025 19:33

chatgpt-codex-connector bot reviewed Dec 10, 2025

View reviewed changes

bolinfest approved these changes Dec 10, 2025

View reviewed changes

JaviSoto added 3 commits December 10, 2025 14:51

fix: thread/list returning fewer than the requested amount due to fil…

d90d25f

…tering This caused some conversations to not appear when they otherwise should.

Refactor tests

3ad13f7

Add a max size to avoid the caller causing us to allocate too much me…

2219c15

…mory

JaviSoto added 5 commits December 10, 2025 14:51

Apply the fix further down in list_conversations_common instead

da77bc2

Small cleanup

9ff4964

Fix codex review comment

c6a75c5

Fix build

2044db8

Simplify with map_err

e65c6d5

JaviSoto force-pushed the dev/javi/thread-list-count branch from 78c9ab5 to e65c6d5 Compare December 10, 2025 22:51

JaviSoto enabled auto-merge (squash) December 10, 2025 22:52

JaviSoto merged commit e2559ab into main Dec 10, 2025
26 checks passed

JaviSoto deleted the dev/javi/thread-list-count branch December 10, 2025 23:06

github-actions bot locked and limited conversation to collaborators Dec 10, 2025

fix: thread/list returning fewer than the requested amount due to filtering CXA-293 #7509

fix: thread/list returning fewer than the requested amount due to filtering CXA-293 #7509

Conversation

JaviSoto commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

etraut-openai commented Dec 9, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

JaviSoto commented Dec 9, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

bolinfest commented Dec 9, 2025

Uh oh!

JaviSoto commented Dec 10, 2025

Uh oh!

JaviSoto commented Dec 10, 2025

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

JaviSoto commented Dec 10, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

JaviSoto commented Dec 10, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

JaviSoto Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

bolinfest left a comment

Choose a reason for hiding this comment

Uh oh!

bolinfest Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

JaviSoto Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

JaviSoto commented Dec 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JaviSoto commented Dec 3, 2025 •

edited

Loading