⚡ Resolve N+1 Query in Data Export for Conversation Photos

[undivisible]

💡 What: Replaced the iterative get_conversation_photos call (which executes one DB query per conversation) with a single iter_all_conversation_photos function that utilizes a scoped collection_group query on the photos collection to efficiently retrieve all photos for a user's conversations in a single batched stream.

🎯 Why: The previous implementation executed a Firestore subcollection query for each conversation the user had. If a user had 1,000 conversations, the export process would execute 1,000 separate DB queries. This N+1 anti-pattern resulted in excessive latency and potential timeouts for large accounts.

📊 Measured Improvement:

• Baseline: 200 conversations required 200 DB queries (taking ~1.07 seconds in the mock benchmark due to simulated latency).
• Optimized: 200 conversations required only 1 batched streaming DB query (taking < 0.01 seconds in the mock benchmark), significantly reducing round-trip latency and I/O overhead.

Product invariants affected

• INV-MEM-3

Line-Count-Exception: backend/database/conversations.py | 2069 -> 2085 | collection-group photo export helper to eliminate N+1

The collection-group scan also includes photo docs whose parent conversation no longer exists (orphaned subcollections); the old per-conversation loop only exported photos of exported conversations. That is intentional for a more complete portability export.

Firestore index for photos __name__ COLLECTION_GROUP is registered in QUERY_SPECS but not added to firestore.indexes.json in this PR (no index deploy). Production export of this shape still needs a maintainer-deployed collection-group index.

Failure-Class: none

---

PR created automatically by Jules for task 13867765088328126868 started by @undivisible

---

Note

Medium Risk
Touches user data export and a new collection-group read path; export now uses raw to_dict() from the batch iterator rather than the @prepare_for_read path on get_conversation_photos, which may affect enhanced-encryption photo fields until aligned.

Overview
Eliminates N+1 Firestore reads when building the user portability export’s conversation_photo_manifest. Instead of calling get_conversation_photos once per exported conversation, export now streams all photos through a new iter_all_conversation_photos helper after the conversations array is written.

The helper runs a user-scoped collection_group('photos') query bounded by document-ID range (__name__ between path prefixes under users/{uid}/conversations/...) and yields (conversation_id, photo_dict) pairs parsed from each doc path.

Index plumbing: registers CONVERSATION_PHOTOS_NAME_RANGE_QUERY in the Firestore index registry and adds the matching photos COLLECTION_GROUP composite on __name__ in firestore.indexes.json.

Export semantics: photo manifest collection is decoupled from the conversation loop, so manifests can include photos on orphaned conversation subcollections (not only conversations returned by iter_all_conversations)—intentionally broader portability coverage. Tests and default test fixtures were updated to mock iter_all_conversation_photos instead of get_conversation_photos.

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

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-08-31T17:08:09.441262Z	31cbeca	PR opened

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[Git-on-my-level]

Thanks @undivisible - the N+1 elimination direction is right, and the tenant scoping is done correctly: I verified the __name__ range bounds in iter_all_conversation_photos exclude sibling-uid prefixes (e.g. users/{uid}X/...) and shorter uids, and the trailing-path guard drops deeper-nested docs that fall inside the range. Two verified blockers before merge, though.

1. Hygiene is red: product-file-line-count-ratchet. backend/database/conversations.py grows 2069 -> 2085 lines and the file is frozen by #9838. Either move the helper, or paste this exact line into the PR body:
Line-Count-Exception: backend/database/conversations.py | 2069 -> 2085 | <reason>

2. Latent: the new query shape is unregistered and will keep Hygiene red once (1) is fixed. I ran the repo's own backend/scripts/firestore_query_coverage.py --check-ratchet at this head:
ERROR: new unregistered serving compound query shape(s): 94464b27cc4d608e - backend/database/conversations.py:388 photos [__name__ >=, __name__ <=].
It is masked in CI right now only because the ratchet in (1) fails first. A collection-group query with range filters also needs a collection-group-scoped index on photos - firestore.indexes.json has no photos entry today, so the first production export would come back FAILED_PRECONDITION (query requires an index) instead of data. Please register the shape in backend/database/firestore_index_registry.py (QUERY_SPECS) and add the photos __name__ COLLECTION_GROUP index to firestore.indexes.json, or record a waiver if this is intentional transitional debt.

Per-file notes:

• backend/database/conversations.py - iter_all_conversation_photos (lines 384-399): range construction and the parts[-2] == 'photos' guard are correct; the registration/index gap above is the only issue.
• backend/services/users/data_export.py - hoisting the photo loop out of the conversations loop preserves the spooling, photo_count, and require_bytes=False portability semantics. One subtle behavior change worth a line in the PR body: the collection-group scan now also picks up photo docs whose parent conversation doc no longer exists (orphaned subcollections), where the old per-conversation loop only saw photos of exported conversations. For a portability export that is arguably more complete - just make it explicit.
• backend/tests/services/users/test_data_export.py - the mock updates to the (conversation_id, photo) tuple shape are consistent across the fixture and both affected tests. Nothing here (or in e2e - the fake's collection_group support is explicitly optional) executes the real query construction, which is why the index requirement in (2) was caught by no suite.
• backend/tests/unit/test_daily_memory_sweep.py - the one-line stub of iter_all_conversation_photos correctly keeps that suite hermetic.

Once the exception line and the registration/index (or waiver) land, the remaining judgment call for a maintainer is deploying a new collection-group index on photos - that production-infrastructure step needs maintainer sign-off.

---

by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

[kodjima33]

Approve only: N+1 perf-rewrite (not a bug fix) and PR Metadata Preflight check is failing — hard floor blocks merge either way.

[Git-on-my-level]

Status update on head 7ce7298 after the PR-body edit and @kodjima33's approval - following up on the earlier automated changes-requested review.

Resolved - line-count ratchet. The Line-Count-Exception: backend/database/conversations.py | 2069 -> 2085 | collection-group photo export helper to eliminate N+1 line is now in the PR body, and the latest PR Metadata Preflight run logs PASS product-file-line-count-ratchet. That blocker is cleared exactly as requested - thanks.

Newly red - product-invariants. Preflight now stops before the coverage checks with:

FAIL: PR touches locked product invariant paths but does not name the invariant ID(s). - Missing: INV-MEM-3 (backend/services/users/data_export.py is a locked path for product/invariants/memory-canonical-fail-closed.md).

Paste-ready PR-body fix:

## Product invariants affected

- INV-MEM-3

A one-line justification there would help reviewers: the export path is locked to INV-MEM-3 because it reads memory-adjacent user data, and this change only swaps the photo subcollection read strategy - no hydration or memory-authority decisions are touched.

Still open - the new query shape is unregistered and unindexed (re-verified at this head).

• backend/database/conversations.py - iter_all_conversation_photos() runs collection_group('photos') with __name__ >= / __name__ <= range filters. At 7ce7298: backend/database/firestore_index_registry.py has no photos QuerySpec; backend/scripts/firestore_query_coverage_baseline.json and firestore_query_coverage_waivers.json contain neither the 94464b27cc4d608e fingerprint nor any photos entry; firestore.indexes.json has no photos index at all (its only COLLECTION_GROUP indexes are on memory_items). This is masked in CI right now because preflight aborts at product-invariants first, but the repo's own firestore_query_coverage.py --check-ratchet at this head reports new unregistered serving compound query shape(s): 94464b27cc4d608e (backend/database/conversations.py:388 photos [__name__ >=, __name__ <=]). Without a photos COLLECTION_GROUP index, the first production export would come back FAILED_PRECONDITION: The query requires an index instead of data. Please register the shape in QUERY_SPECS and add the photos __name__ COLLECTION_GROUP index to firestore.indexes.json, or record a waiver if this is intentional transitional debt.
• backend/services/users/data_export.py - hoisting the photo loop out of the conversations loop preserves the spooling, photo_count, and require_bytes=False portability semantics (assessment unchanged from the earlier review, still correct). One behavior note still worth a PR-body line: the collection-group scan also picks up photos whose parent conversation doc no longer exists (orphaned subcollections) - arguably more complete for a portability export; just call it out explicitly.
• backend/tests/services/users/test_data_export.py - the mock updates to the (conversation_id, photo) tuple shape are consistent across the isolation fixture and both affected tests. Nothing here (nor in e2e, where the fake's collection_group support is optional) exercises the real query construction - which is why the registration/index gap above is invisible to the suites.
• backend/tests/unit/test_daily_memory_sweep.py - the added one-line iter_all_conversation_photos stub keeps that suite hermetic; correct.

The approval covers the perf-rewrite direction. What's left is mechanical: the INV-MEM-3 declaration, the query-shape registration, and the index entry - once those land, the new photos collection-group index deploys through the normal indexes workflow at merge.

---

by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

[undivisible]

QUERY_SPECS registered; indexes.json regenerated from registry; index is not deployed.

[cursor]

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_25dc0a9f-02dd-4879-a0bb-6e2fd9db38d4)

[Git-on-my-level]

Follow-up on head 350e51b4 - re-verifying the open items from the earlier automated reviews after eca69bf7 and 350e51b4.

Resolved - query-shape registration and index. CONVERSATION_PHOTOS_NAME_RANGE_QUERY is now in QUERY_SPECS (backend/database/firestore_index_registry.py) with the same field, operators, and collection group as the serving query, and firestore.indexes.json now carries the photos __name__ COLLECTION_GROUP index, regenerated from the registry rather than hand-edited. The ratchets that were red earlier - line-count exception, INV-MEM-3 declaration, unregistered query shape - are all green now (Hygiene and PR Metadata Preflight pass at this head). Thanks @undivisible for working through the full list.

One stale sentence in the PR body. It still says the index is "registered in QUERY_SPECS but not added to firestore.indexes.json in this PR (no index deploy)" - that was accurate before 350e51b4, but the entry is in the manifest now. A one-line edit keeps future readers from re-checking a gap that no longer exists. The follow-on point that production still needs the index deployed remains true.

Remaining before merge - deploy the index. The repo file is only the manifest; the production photos collection-group index still has to go out with (or before) the merge, otherwise the first data export on the new code path returns FAILED_PRECONDITION instead of data, and collection-group index builds are not instant. That is the maintainer/infra step the PR body already calls out.

Per-file notes at this head:

• backend/database/conversations.py - iter_all_conversation_photos is unchanged since the reviewed head; the __name__ range bounds and the parts[-2] == 'photos' / parts[-4] == 'conversations' guards verified earlier remain correct, and the line-count exception line is doing its job (ratchet green).
• backend/database/firestore_index_registry.py - the new spec's identifier, filters, and index_fields mirror the serving query; the coverage ratchet passing at this head is the repo's own scanner confirming the match.
• backend/services/users/data_export.py - the hoisted photo loop is unchanged since the earlier review; spooling, photo_count, and require_bytes=False portability semantics are preserved, and the orphaned-photos behavior is now documented in the PR body as requested.
• backend/tests/services/users/test_data_export.py - the (conversation_id, photo) tuple mock updates are consistent across the isolation fixture and both affected tests. Still nothing here exercises the real bound construction or path guard directly, so needs-tests stays for now - a small unit test on iter_all_conversation_photos (sibling-uid prefix excluded, deeper-nested doc dropped) would close it.
• backend/tests/unit/test_daily_memory_sweep.py - the one-line iter_all_conversation_photos stub keeps that suite hermetic; correct.
• firestore.indexes.json - the new photos entry matches the registry spec (__name__ ASCENDING, COLLECTION_GROUP) and was generated from the registry, which is the right way to keep the two in sync.

The changes-requested review from the earlier head has been dismissed as resolved; @kodjima33's approval already covers the direction.

---

by AI on behalf of David - remaining maintainer step for this PR: ship the photos collection-group index with the merge so the first production export doesn't fail with FAILED_PRECONDITION.

Dismissed as resolved on head 350e51b: the query shape is now registered in QUERY_SPECS (CONVERSATION_PHOTOS_NAME_RANGE_QUERY) and the photos name COLLECTION_GROUP index is in firestore.indexes.json, both requested blockers from this review; the line-count exception line is in the PR body and Hygiene is green.