Repository navigation
fix(leads): add id tiebreaker to score-ordered lead paging - #83
Merged
Merged
Conversation
query_leads_page ordered only by score, so with tied scores PostgreSQL could return overlapping/skipping LIMIT/OFFSET pages; the legacy workbook migration (#71) could then duplicate some leads, drop others, and still mark itself complete. Add the unique lead id as a secondary sort key on both backends (query_leads_page, get_leads/query_leads, get_job_leads), and add SQLite and PostgreSQL regression tests over 1001 tied scores. Signed-off-by: debpalash <debash67@outlook.in> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
query_leads_pageordered only byscore DESCwithLIMIT/OFFSET. When scores tie, PostgreSQL does not guarantee a stable order between queries, so consecutive pages can overlap and skip rows. The legacy-lead migration added in #71 (migrate_workbook_to_v2, which reads 500-row pages) relies on this paging. It could duplicate some leads, drop others, and still mark the workbook as migrated, because the count check only comparesmigrated >= total.SQLite hides the bug because tied rows come back in rowid order.
Fix
The unique lead
idis now a secondary sort key, in the same direction as the primary key, everywhere leads are paged by a sort column:PgLeadStore.query_leads_pageandLeadDB.query_leads_page:score DESC, id DESCPgLeadStore.get_leadsandLeadDB.get_leads:<col> <dir>, id <dir>. These back the offset-paged/leadsAPI.LeadDB.get_job_leads:score DESC, id DESC. This query hasLIMITbut no offset, so the change only makes results deterministic.Callers of
query_leads_page(workbook_query_leadsand migration, andaudiences/refresh.py) all get the fix through the store.Index note (no migration added)
PostgreSQL already has
ix_leads_ws_score (workspace_id, score). The planner can still use it for the workspace filter and score order, then do an incremental sort onidwithin ties. I did not add an index. If profiling of large workspaces shows the sort matters, a follow-up migration could replace it with(workspace_id, score, id); a backward scan of that index servesscore DESC, id DESCexactly. SQLite'sidx_leads_scorebehaves the same way.Tests
tests/test_lead_store_sqlite.py: 1001 leads with the same score, paged 100 at a time throughquery_leads_pageandget_leads. Each id appears exactly once.tests/test_workbook_migration_paging.py: the migration over 1001 tied-score leads (500-row pages). Every lead is migrated exactly once.tests/test_lead_paging_pg.py(new,postgresmarker, gated onTEST_DATABASE_URL, added to the "PostgreSQL integrity and tenancy" CI job): the same two checks againstPgLeadStoreunder FORCE RLS.main'sstore.py, the direct paging test fails on local Postgres: 997 distinct ids out of 1001. With the fix it passes.Local results:
uv run pytest -qgives 2206 passed, 139 skipped. The PG CI job's files against local Postgres give 20 passed.Refs #71
🤖 Generated with Claude Code