I lean towards not having hard-coded integers like 2, 5, or 7 - could we think of a better way? One idea I thought of (and please feel free to propose better ways) would be to have _WINDOW_FIELDS = [FLEX_WIN_START, FLEX_WIN_END] and then _KEYSET_CURSOR_FIELDS = [last seen_project_id, last_seen_item_type, etc] and then it would allow you to do if len(filters) != len(_KEYSET_CURSOR_FIELDS), which is more readable. Feel free to use better variable names or another approach

Well, previously you added this; I just moved the code. I can think if there's better way. I am not much familiar with patterns in this codebase.

Nit: I feel like what has_last_seen represents is whether we should resume within the same flex window (correct me if I'm wrong). Could we have a more accurate name to represent this?

Sure, name can be better

Would _parse_flex_window_from_filters return incorrect result if you pass in win = _parse_flex_window_from_filters(filters)?

If not, this part seems like it could be consolidated to just be written once:

flex_window = _parse_flex_window_from_filters(filters)
if flex_window is None:
    raise ValueError("Invalid page token")
flex_window_start, flex_window_end = flex_window

nit: just spell the variables out

Page token's time window is never applied to query

High Severity

The ExportTraceItemsPageToken encodes window_start_sec/window_end_sec but these values are never used when building the next query. _export_query_meta always derives the time window from routing_decision.time_window (or original in_msg.meta), completely ignoring the incoming page token's window. Similarly, w_start/w_end in _execute are derived from the routing decision, not from the page token. When flex routing emits a 2-filter token to advance to the earlier time slice [orig_start, routed_start), the subsequent request re-queries [routed_start, orig_end) instead, causing an infinite loop.

 

^{Reviewed by Cursor Bugbot for commit b46663e. Configure here.}

Bug: The pagination token's time window (window_start_sec, window_end_sec) is ignored on subsequent requests. The query re-uses the original time window, breaking pagination logic.
_{Severity: HIGH}

Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: snuba/web/rpc/v1/endpoint_export_trace_items.py#L589-L593

Potential issue: The code generates a page token containing a specific time window
(`window_start_sec`, `window_end_sec`) for the next page of results. However, when a
subsequent request uses this token, the time window values are deserialized but never
used to constrain the new query. Instead, the query's time window is re-calculated from
the original request metadata. This causes the pagination logic to fail, as it ignores
the intended time slice from the token, leading to re-scanning the same data or skipping
data entirely. The fields `window_start_sec` and `window_end_sec` are effectively dead
data after being read from the token.

_{Did we get this right? 👍 / 👎 to inform future reviews.}