Skip to content

Require safe PostHog pagination cursors#19

Merged
nicosuave merged 1 commit into
mainfrom
fix/posthog-pagination-cursor
Jul 18, 2026
Merged

Require safe PostHog pagination cursors#19
nicosuave merged 1 commit into
mainfrom
fix/posthog-pagination-cursor

Conversation

@nicosuave

Copy link
Copy Markdown
Member

Fail before sending a full events page when its final row lacks the timestamp and UUID needed for lossless keyset pagination. Partial final pages still support generated event UUIDs. Adds regression coverage for the cursor boundary.

@nicosuave
nicosuave merged commit 1cf35ed into main Jul 18, 2026
1 check passed
@nicosuave
nicosuave deleted the fix/posthog-pagination-cursor branch July 18, 2026 16:34

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f62335668f

ℹ️ 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".

Comment thread src/importer.rs
}
}

let next_cursor = validated_event_page_cursor(count, limit, next_cursor)?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Don’t require cursors for max-events terminal pages

When IMPORT_MAX_EVENTS makes next_limit return the exact remaining cap, a response with count == limit can be the terminal page because the next loop would break before issuing another PostHog query. This validation runs before sending and treats that capped page as needing a pagination cursor, so capped smoke-test imports fail if the Nth row lacks a timestamp/UUID even though no further pagination is needed and partial final pages are otherwise allowed.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in #20. Cursor validation now runs only when the page is full and next_limit() confirms another query will be issued after send() updates the imported-event count. A full page that exactly satisfies IMPORT_MAX_EVENTS is terminal and does not require a cursor; continuing pages still fail closed without the UUID tie-breaker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant