Skip to content

fix(consolidation): cursor-paginate fact reads to remove the 10k results ceiling (RES-946) - #333

Open
jhkchan wants to merge 1 commit into
mainfrom
feature/res-946-consolidation-cursor-pagination
Open

fix(consolidation): cursor-paginate fact reads to remove the 10k results ceiling (RES-946)#333
jhkchan wants to merge 1 commit into
mainfrom
feature/res-946-consolidation-cursor-pagination

Conversation

@jhkchan

@jhkchan jhkchan commented Jul 17, 2026

Copy link
Copy Markdown
Member

Problem (RES-946)

Space consolidation could not run on channels above ~10k facts. iter_all_fact_ids and iter_unclustered_facts used offset pagination (fetch_objects(filters=…, offset=…)), which Weaviate caps at QUERY_MAXIMUM_RESULTS (default 10000). Past that, full_reconsolidate failed with query maximum results exceeded and the Space wiki never built.

Live evidence (RLP POC): channel rlp-ingest at 12,169 factsFull reconsolidation error … query maximum results exceededcreated=0 updated=0 facts=0 errors=1.

Fix

Switch both reads to Weaviate v4 cursor pagination via collection.iterator() with client-side channel_id / tier / cluster_id predicates — the same idiom already used by iter_atomic_fact_ids_and_text and snapshot_all_facts_for_reembed in this file. Cursor iteration walks by UUID: no offset ceiling, no 10MB-per-page risk (the client batches internally).

  • Public signatures + yield shapes unchanged → the two consolidation.py callers and existing mocked tests are untouched.
  • Docstrings updated to drop the now-false 10k-cap / "cursor incompatible with filters" notes.

Scaling caveat (documented in-code)

iterator() can't filter server-side, so this scans the whole MemoryFact collection per channel — fine at Atlas's documented scale (~5k–50k facts). A very large multi-channel install wants channel-scoped keyset pagination, which needs an indexed unique monotonic property (a schema migration) — deliberately out of scope here.

Tests

  • New tests/stores/test_weaviate_consolidation_cursor.py: >10k rows across multiple channels/tiers flow through; only matching rows are yielded (with vectors for unclustered); fetch_objects (offset) is never called.
  • Existing tests/services/test_consolidation_streaming.py still passes (interface preserved).
  • 5 passed locally.

Part of epic RES-943 (RLP full-corpus scale + no-cloud gaps).

🤖 Generated with Claude Code

https://claude.ai/code/session_01PMMM6KQXmzAEA42UxpyMUm

…Y_MAXIMUM_RESULTS ceiling (RES-946)

`iter_all_fact_ids` and `iter_unclustered_facts` used offset pagination
(`fetch_objects(filters=..., offset=...)`), which Weaviate caps at
`QUERY_MAXIMUM_RESULTS` (default 10000). On channels with >10k atomic facts,
`full_reconsolidate` failed with `query maximum results exceeded` and the Space
wiki could never consolidate (observed live at 12,169 facts:
`created=0 updated=0 facts=0 errors=1`).

Switch both to Weaviate v4 cursor pagination via `collection.iterator()` with
client-side channel/tier/cluster predicates — the same idiom already used by
`iter_atomic_fact_ids_and_text` and `snapshot_all_facts_for_reembed`. Cursor
iteration walks by UUID with no offset ceiling and no 10MB-per-page risk.
Public signatures and yield shapes are unchanged, so the two consolidation.py
callers and existing mocked tests are untouched.

Scaling caveat (documented in-code): `iterator()` cannot filter server-side, so
this scans the whole MemoryFact collection per channel — fine at Atlas's
documented scale (~5k-50k facts). Channel-scoped keyset pagination would need an
indexed unique monotonic property (a schema migration), deliberately out of scope.

Adds tests/stores/test_weaviate_consolidation_cursor.py: >10k rows across
multiple channels/tiers flow through, only matching rows are yielded (with
vectors for unclustered), and offset `fetch_objects` is never called.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PMMM6KQXmzAEA42UxpyMUm
@CLAassistant

CLAassistant commented Jul 17, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@jhkchan

jhkchan commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

Independent adversarial review — APPROVE (nits only).

No P0/P1 code defect. Verified: filter parity is exact (channel_id/tier/cluster_id=="__none__" client-side mirrors the old server-side Filter), tuple shape/type parity (str(uuid), cluster_id or ""), include_vector=True honored and normalized by _obj_to_fact, both walks run under asyncio.to_thread, no drop/duplicate (UUID-cursor), no other-caller regression. Correct fix for the 10k ceiling.

Should-fix (follow-up, not blocking):

  • [P1 test blind spot] test_..._filters_channel_tier_cluster_with_vectors is vacuous — the fake iterator ignores include_vector and always attaches .vector, so the test passes even if the store stops requesting vectors. Mutation-proved (flipping include_vectorFalse kept all 3 tests green). Capture the kwargs and assert include_vector is True, and only attach .vector when requested.
  • [P2 efficiency] iter_all_fact_ids no longer passes return_properties=["cluster_id"], so iterator() materializes all properties per row just to read uuid+cluster_id. Add return_properties=["cluster_id","channel_id","tier"].

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.

2 participants