mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Add item anchors to thread/items/list pagination (#48151)
## What changed
Allow `cursor` to accept an item anchor such as `{"type":"item","itemId":"item-123"}` with a non-empty `turnId`. Ascending pagination returns items after the anchor; descending pagination returns items before it. The anchor itself is excluded, and subsequent pages use the returned opaque string cursor.
Resolve anchors within the requested turn's visible history, including inherited fork history. Reject missing turn IDs and empty, unknown, or out-of-scope item IDs with invalid params (`-32602`). Preserve existing string and null cursor behavior, and update the protocol schemas, TypeScript and Python types, and API documentation.
## Testing
Add protocol round-trip and invalid-shape tests, app-server coverage for anchored pagination and errors, and thread-store coverage for growing turns, reverse pagination, page boundaries, fork visibility, and anchor scope validation.
GitOrigin-RevId: a74cea190d3ff1b44865429f8b0650b793eb688d
This commit is contained in:
@@ -131,7 +131,7 @@ mod tests {
|
||||
thread_id,
|
||||
turn_id: None,
|
||||
include_archived: true,
|
||||
cursor: None,
|
||||
position: None,
|
||||
page_size: 10,
|
||||
sort_direction: SortDirection::Asc,
|
||||
sort_key: ItemSortKey::CreatedAtOrdinal,
|
||||
|
||||
@@ -77,6 +77,7 @@ pub use types::GitInfoPatch;
|
||||
pub use types::ItemPage;
|
||||
pub use types::ItemSortKey;
|
||||
pub use types::ListItemsParams;
|
||||
pub use types::ListItemsPosition;
|
||||
pub use types::ListThreadsParams;
|
||||
pub use types::ListTimelineParams;
|
||||
pub use types::ListTurnsParams;
|
||||
|
||||
@@ -454,7 +454,7 @@ async fn migration_projects_explicit_and_implicit_legacy_completed_items() {
|
||||
thread_id,
|
||||
turn_id: None,
|
||||
include_archived: false,
|
||||
cursor: None,
|
||||
position: None,
|
||||
page_size: 10,
|
||||
sort_direction: SortDirection::Asc,
|
||||
sort_key: ItemSortKey::CreatedAtOrdinal,
|
||||
@@ -584,7 +584,7 @@ async fn migration_keeps_late_completions_in_their_original_turn() {
|
||||
thread_id,
|
||||
turn_id: None,
|
||||
include_archived: false,
|
||||
cursor: None,
|
||||
position: None,
|
||||
page_size: 10,
|
||||
sort_direction: SortDirection::Asc,
|
||||
sort_key: ItemSortKey::CreatedAtOrdinal,
|
||||
@@ -1064,7 +1064,7 @@ async fn migration_keeps_late_completions_for_surviving_turns_across_rollback()
|
||||
thread_id,
|
||||
turn_id: None,
|
||||
include_archived: false,
|
||||
cursor: None,
|
||||
position: None,
|
||||
page_size: 20,
|
||||
sort_direction: SortDirection::Asc,
|
||||
sort_key: ItemSortKey::CreatedAtOrdinal,
|
||||
|
||||
@@ -23,6 +23,7 @@ use tempfile::TempDir;
|
||||
|
||||
use super::*;
|
||||
use crate::ItemSortKey;
|
||||
use crate::ListItemsPosition;
|
||||
use crate::ListTimelineParams;
|
||||
use crate::SearchThreadOccurrencesParams;
|
||||
use crate::SortDirection;
|
||||
@@ -232,6 +233,169 @@ async fn list_items_pages_whole_thread_and_per_turn_rows() {
|
||||
assert_eq!(item_ids(&next_turn_page), vec!["item-3"]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn item_anchors_page_exclusively_while_the_turn_grows() {
|
||||
let (_home, store, thread_id) = store_with_mode(ThreadHistoryMode::Paginated).await;
|
||||
let db = history_db(&store).await;
|
||||
insert_turn(
|
||||
db,
|
||||
thread_id,
|
||||
"turn",
|
||||
/*rollout_ordinal*/ 1,
|
||||
"inProgress",
|
||||
/*error_json*/ None,
|
||||
/*first_user_item_id*/ None,
|
||||
/*final_agent_item_id*/ None,
|
||||
)
|
||||
.await;
|
||||
for (id, ordinal) in [
|
||||
("one", 2),
|
||||
("two", 3),
|
||||
("anchor", 4),
|
||||
("four", 5),
|
||||
("five", 6),
|
||||
] {
|
||||
insert_item(db, thread_id, "turn", id, ordinal).await;
|
||||
}
|
||||
insert_item(
|
||||
db,
|
||||
thread_id,
|
||||
"other-turn",
|
||||
"anchor",
|
||||
/*rollout_ordinal*/ 8,
|
||||
)
|
||||
.await;
|
||||
for (direction, expected) in [
|
||||
(SortDirection::Asc, vec!["four", "five", "new"]),
|
||||
(SortDirection::Desc, vec!["two", "one"]),
|
||||
] {
|
||||
let mut params = item_params(
|
||||
thread_id,
|
||||
Some("turn"),
|
||||
/*cursor*/ None,
|
||||
/*page_size*/ 1,
|
||||
direction,
|
||||
);
|
||||
params.position = Some(ListItemsPosition::ItemAnchor {
|
||||
item_id: "anchor".to_string(),
|
||||
});
|
||||
let first = store.list_items(params.clone()).await.expect("anchor page");
|
||||
assert_eq!(item_ids(&first), vec![expected[0]]);
|
||||
if direction == SortDirection::Asc {
|
||||
insert_item(db, thread_id, "turn", "new", /*rollout_ordinal*/ 7).await;
|
||||
}
|
||||
let mut ids = vec![first.items[0].item_id.clone()];
|
||||
params.position = first.next_cursor.map(ListItemsPosition::Cursor);
|
||||
while params.position.is_some() {
|
||||
let page = store
|
||||
.list_items(params.clone())
|
||||
.await
|
||||
.expect("continuation");
|
||||
ids.extend(page.items.into_iter().map(|item| item.item_id));
|
||||
params.position = page.next_cursor.map(ListItemsPosition::Cursor);
|
||||
}
|
||||
assert_eq!(ids, expected);
|
||||
params.position = first.backwards_cursor.map(ListItemsPosition::Cursor);
|
||||
params.sort_direction = match direction {
|
||||
SortDirection::Asc => SortDirection::Desc,
|
||||
SortDirection::Desc => SortDirection::Asc,
|
||||
};
|
||||
let backwards = store.list_items(params).await.expect("reverse page");
|
||||
assert_eq!(item_ids(&backwards), vec![expected[0]]);
|
||||
}
|
||||
for (id, direction) in [("one", SortDirection::Desc), ("new", SortDirection::Asc)] {
|
||||
let mut params = item_params(
|
||||
thread_id,
|
||||
Some("turn"),
|
||||
/*cursor*/ None,
|
||||
/*page_size*/ 2,
|
||||
direction,
|
||||
);
|
||||
params.position = Some(ListItemsPosition::ItemAnchor {
|
||||
item_id: id.to_string(),
|
||||
});
|
||||
let page = store.list_items(params).await.expect("edge anchor");
|
||||
assert!(page.items.is_empty());
|
||||
assert_eq!((page.next_cursor, page.backwards_cursor), (None, None));
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn item_anchors_require_creation_order_without_an_update_watermark() {
|
||||
let (_home, store, thread_id) = store_with_mode(ThreadHistoryMode::Paginated).await;
|
||||
insert_item(
|
||||
history_db(&store).await,
|
||||
thread_id,
|
||||
"turn",
|
||||
"anchor",
|
||||
/*rollout_ordinal*/ 1,
|
||||
)
|
||||
.await;
|
||||
for (sort_key, after_updated_at_ordinal) in [
|
||||
(ItemSortKey::UpdatedAtOrdinal, None),
|
||||
(ItemSortKey::CreatedAtOrdinal, Some(0)),
|
||||
] {
|
||||
let params = ListItemsParams {
|
||||
position: Some(ListItemsPosition::ItemAnchor {
|
||||
item_id: "anchor".to_string(),
|
||||
}),
|
||||
sort_key,
|
||||
after_updated_at_ordinal,
|
||||
..item_params(
|
||||
thread_id,
|
||||
Some("turn"),
|
||||
/*cursor*/ None,
|
||||
/*page_size*/ 1,
|
||||
SortDirection::Asc,
|
||||
)
|
||||
};
|
||||
let error = store
|
||||
.list_items(params)
|
||||
.await
|
||||
.expect_err("invalid anchor paging mode");
|
||||
assert!(matches!(error, ThreadStoreError::InvalidRequest { message }
|
||||
if message == "item anchors require creation-order paging without an update watermark"));
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn item_anchors_reject_items_outside_the_requested_scope() {
|
||||
let (_home, store, thread_id) = store_with_mode(ThreadHistoryMode::Paginated).await;
|
||||
let db = history_db(&store).await;
|
||||
insert_item(
|
||||
db,
|
||||
thread_id,
|
||||
"other-turn",
|
||||
"other-turn-item",
|
||||
/*rollout_ordinal*/ 2,
|
||||
)
|
||||
.await;
|
||||
insert_item(
|
||||
db,
|
||||
ThreadId::default(),
|
||||
"turn",
|
||||
"other-thread-item",
|
||||
/*rollout_ordinal*/ 2,
|
||||
)
|
||||
.await;
|
||||
for anchor in ["", "unknown", "other-turn-item", "other-thread-item"] {
|
||||
let mut params = item_params(
|
||||
thread_id,
|
||||
Some("turn"),
|
||||
/*cursor*/ None,
|
||||
/*page_size*/ 1,
|
||||
SortDirection::Asc,
|
||||
);
|
||||
params.position = Some(ListItemsPosition::ItemAnchor {
|
||||
item_id: anchor.to_string(),
|
||||
});
|
||||
let error = store.list_items(params).await.expect_err("invalid anchor");
|
||||
assert!(
|
||||
matches!(error, ThreadStoreError::InvalidRequest { message } if message == "cursor.itemId does not identify an item in the requested history scope")
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn timeline_interleaves_items_and_restores_page_boundary_session_state() {
|
||||
let (_home, store, thread_id) = store_with_mode(ThreadHistoryMode::Paginated).await;
|
||||
@@ -676,7 +840,7 @@ async fn list_items_filters_exclusive_update_ordinals_across_pages_and_turns() {
|
||||
assert_eq!(first_page.items, vec![item_2.clone(), item_3.clone()]);
|
||||
for params in [
|
||||
ListItemsParams {
|
||||
cursor: creation_page.next_cursor,
|
||||
position: creation_page.next_cursor.map(ListItemsPosition::Cursor),
|
||||
..updated_item_params(thread_id, /*after_updated_at_ordinal*/ 0)
|
||||
},
|
||||
item_params(
|
||||
@@ -696,7 +860,7 @@ async fn list_items_filters_exclusive_update_ordinals_across_pages_and_turns() {
|
||||
|
||||
let second_page = store
|
||||
.list_items(ListItemsParams {
|
||||
cursor: first_page.next_cursor,
|
||||
position: first_page.next_cursor.map(ListItemsPosition::Cursor),
|
||||
..updated_item_params(thread_id, /*after_updated_at_ordinal*/ 0)
|
||||
})
|
||||
.await
|
||||
@@ -723,7 +887,7 @@ async fn list_items_filters_exclusive_update_ordinals_across_pages_and_turns() {
|
||||
assert_eq!(descending_page.items, vec![item_1, item_3]);
|
||||
let descending_next_page = store
|
||||
.list_items(ListItemsParams {
|
||||
cursor: descending_page.next_cursor,
|
||||
position: descending_page.next_cursor.map(ListItemsPosition::Cursor),
|
||||
sort_direction: SortDirection::Desc,
|
||||
..updated_item_params(thread_id, /*after_updated_at_ordinal*/ 0)
|
||||
})
|
||||
@@ -992,6 +1156,29 @@ async fn lineage_reads_page_across_parent_and_child_segments() {
|
||||
vec!["root-user", "root-agent"]
|
||||
);
|
||||
|
||||
let mut anchored = item_params(
|
||||
child_id,
|
||||
Some("root-1"),
|
||||
/*cursor*/ None,
|
||||
/*page_size*/ 2,
|
||||
SortDirection::Desc,
|
||||
);
|
||||
anchored.position = Some(ListItemsPosition::ItemAnchor {
|
||||
item_id: "root-agent".to_string(),
|
||||
});
|
||||
let page = store
|
||||
.list_items(anchored.clone())
|
||||
.await
|
||||
.expect("inherited anchor");
|
||||
assert_eq!(item_ids(&page), vec!["root-user"]);
|
||||
anchored.turn_id = Some("excluded-root".to_string());
|
||||
anchored.position = Some(ListItemsPosition::ItemAnchor {
|
||||
item_id: "excluded-item".to_string(),
|
||||
});
|
||||
assert!(
|
||||
matches!(store.list_items(anchored).await, Err(ThreadStoreError::InvalidRequest { message }) if message == "cursor.itemId does not identify an item in the requested history scope")
|
||||
);
|
||||
|
||||
for sort_key in [ItemSortKey::CreatedAtOrdinal, ItemSortKey::UpdatedAtOrdinal] {
|
||||
let error = store
|
||||
.list_items(ListItemsParams {
|
||||
@@ -1498,7 +1685,7 @@ fn item_params(
|
||||
thread_id,
|
||||
turn_id: turn_id.map(str::to_owned),
|
||||
include_archived: false,
|
||||
cursor,
|
||||
position: cursor.map(ListItemsPosition::Cursor),
|
||||
page_size,
|
||||
sort_direction,
|
||||
sort_key: ItemSortKey::CreatedAtOrdinal,
|
||||
|
||||
@@ -19,6 +19,7 @@ use super::read::stored_turn_row;
|
||||
use super::thread_history_error;
|
||||
use crate::ItemSortKey;
|
||||
use crate::ListItemsParams;
|
||||
use crate::ListItemsPosition;
|
||||
use crate::SortDirection;
|
||||
use crate::StoredTurnItemsView;
|
||||
use crate::ThreadStoreError;
|
||||
@@ -208,6 +209,20 @@ pub(super) async fn page_item_rows(
|
||||
lineage: &RolloutLineage,
|
||||
params: &ListItemsParams,
|
||||
) -> ThreadStoreResult<SegmentPage<StoredThreadItemRow>> {
|
||||
let (cursor, anchor_item_id) = match params.position.as_ref() {
|
||||
Some(ListItemsPosition::Cursor(cursor)) => (Some(cursor.as_str()), None),
|
||||
Some(ListItemsPosition::ItemAnchor { item_id }) => (None, Some(item_id.as_str())),
|
||||
None => (None, None),
|
||||
};
|
||||
if anchor_item_id.is_some()
|
||||
&& (params.sort_key != ItemSortKey::CreatedAtOrdinal
|
||||
|| params.after_updated_at_ordinal.is_some())
|
||||
{
|
||||
return Err(ThreadStoreError::InvalidRequest {
|
||||
message: "item anchors require creation-order paging without an update watermark"
|
||||
.to_string(),
|
||||
});
|
||||
}
|
||||
// Update ordinals are local to a rollout. Forked lineages need a structured
|
||||
// watermark before incremental replay can safely span their segments.
|
||||
if params.after_updated_at_ordinal.is_some() && lineage.segments().len() > 1 {
|
||||
@@ -230,15 +245,55 @@ pub(super) async fn page_item_rows(
|
||||
pool,
|
||||
segment.rollout_id(),
|
||||
params,
|
||||
cursor,
|
||||
after_updated_at_ordinal,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
let cursor = parse_cursor(
|
||||
params.cursor.as_deref(),
|
||||
params.thread_id,
|
||||
CursorScope::ItemsByCreatedAtOrdinal,
|
||||
)?;
|
||||
let cursor = if let Some(anchor_item_id) = anchor_item_id {
|
||||
let turn_id = params
|
||||
.turn_id
|
||||
.as_deref()
|
||||
.filter(|id| !id.is_empty())
|
||||
.ok_or_else(|| ThreadStoreError::InvalidRequest {
|
||||
message: "turnId is required when cursor is an item anchor".to_string(),
|
||||
})?;
|
||||
if anchor_item_id.is_empty() {
|
||||
return Err(invalid_item_anchor());
|
||||
}
|
||||
let mut anchor = None;
|
||||
for segment in lineage.segments() {
|
||||
let mut query = QueryBuilder::<Sqlite>::new(
|
||||
"SELECT rollout_ordinal FROM thread_items WHERE thread_id = ",
|
||||
);
|
||||
query.push_bind(segment.rollout_id().to_string());
|
||||
query.push(" AND item_id = ").push_bind(anchor_item_id);
|
||||
push_segment_range(&mut query, segment)?;
|
||||
query.push(" AND turn_id = ").push_bind(turn_id);
|
||||
if let Some(ordinal) = query
|
||||
.build_query_scalar::<i64>()
|
||||
.fetch_optional(pool)
|
||||
.await
|
||||
.map_err(thread_history_error)?
|
||||
{
|
||||
anchor = Some(ordinal);
|
||||
break;
|
||||
}
|
||||
}
|
||||
Some(HistoryCursor {
|
||||
requested_thread_id: params.thread_id,
|
||||
rollout_ordinal: u64::try_from(anchor.ok_or_else(invalid_item_anchor)?)
|
||||
.map_err(thread_history_error)?,
|
||||
include_anchor: false,
|
||||
scope: CursorScope::ItemsByCreatedAtOrdinal,
|
||||
})
|
||||
} else {
|
||||
parse_cursor(
|
||||
cursor,
|
||||
params.thread_id,
|
||||
CursorScope::ItemsByCreatedAtOrdinal,
|
||||
)?
|
||||
};
|
||||
let mut rows = Vec::new();
|
||||
for (_, segment, segment_cursor) in
|
||||
segments_from_cursor(lineage, params.sort_direction, cursor.as_ref())?
|
||||
@@ -289,10 +344,11 @@ async fn page_updated_item_rows(
|
||||
pool: &sqlx::SqlitePool,
|
||||
rollout_id: ThreadId,
|
||||
params: &ListItemsParams,
|
||||
cursor: Option<&str>,
|
||||
after_updated_at_ordinal: u64,
|
||||
) -> ThreadStoreResult<SegmentPage<StoredThreadItemRow>> {
|
||||
let cursor = parse_cursor(
|
||||
params.cursor.as_deref(),
|
||||
cursor,
|
||||
params.thread_id,
|
||||
CursorScope::ItemsByUpdatedAtOrdinal,
|
||||
)?;
|
||||
@@ -504,3 +560,10 @@ fn page_size_too_large() -> ThreadStoreError {
|
||||
message: "page size is too large".to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
fn invalid_item_anchor() -> ThreadStoreError {
|
||||
ThreadStoreError::InvalidRequest {
|
||||
message: "cursor.itemId does not identify an item in the requested history scope"
|
||||
.to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -469,6 +469,16 @@ pub struct TurnPage {
|
||||
pub backwards_cursor: Option<String>,
|
||||
}
|
||||
|
||||
/// Starting position for listing persisted items.
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub enum ListItemsPosition {
|
||||
/// Opaque cursor returned by a previous list call.
|
||||
Cursor(String),
|
||||
/// Exclusive item anchor within the visible non-empty turn.
|
||||
/// Requires creation order without an update watermark.
|
||||
ItemAnchor { item_id: String },
|
||||
}
|
||||
|
||||
/// Parameters for listing persisted items within a thread.
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct ListItemsParams {
|
||||
@@ -478,8 +488,8 @@ pub struct ListItemsParams {
|
||||
pub turn_id: Option<String>,
|
||||
/// Whether archived threads are eligible.
|
||||
pub include_archived: bool,
|
||||
/// Opaque cursor returned by a previous list call.
|
||||
pub cursor: Option<String>,
|
||||
/// Optional starting position; omitted for the first page.
|
||||
pub position: Option<ListItemsPosition>,
|
||||
/// Maximum number of items to return.
|
||||
pub page_size: usize,
|
||||
/// Direction to sort items by the selected ordinal.
|
||||
|
||||
Reference in New Issue
Block a user