Refetch documents by $sequence

[fogelito]

• Refetch using primary key ($sequence)
• Move Refetch out of withTransactions
• Refetch using find method has a limit of default 25 rows max
• Added chunking for large documents list
• Refetch using find did not preserve select queries, always returning all attributes as doing select (*)
• Fix decode issue, since find method already decoded documents

Summary by CodeRabbit

• Bug Fixes

  • Improved operator-computed document refresh by matching refreshed results using each document’s sequence value.
  • Refresh now preserves the original projection/selection so returned documents keep the expected fields.
  • Operator-driven refetch for both single and batch updates now happens after transaction commit to avoid timing-related inconsistencies.

• Tests

  • Added end-to-end coverage for operator updates with projections and for large batch sizes beyond the default limit.

[coderabbitai]

📝 Walkthrough

Walkthrough

Database now refetches operator-computed documents after commit, matches refreshed documents by $sequence, preserves selection context, and adds e2e coverage for select projections and larger batch updates.

Changes

Refetch and replay operator-computed documents

Layer / File(s)	Summary
Refetch by sequence with selections src/Database/Database.php	refetchDocuments() accepts selection context, queries by $sequence, and remaps refreshed documents back to the original input order.
Commit-time refetch wiring src/Database/Database.php	updateDocument() removes the in-transaction operator refetch and performs it after commit when operator values are present.
Batch refetch and e2e coverage src/Database/Database.php, tests/e2e/Adapter/Scopes/OperatorTests.php	updateDocuments() forwards grouped selections into refetchDocuments(), skips the extra decode path for operator results, and new e2e tests cover select projections and large batch updates.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

• utopia-php/database#618: Updates sequence population during document creation, which is directly related to the $sequence-based refetch matching in this PR.

Suggested reviewers: abnegate

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: refetching documents by $sequence.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refetch-documents-by-sequence

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

This PR changes refetchDocuments to look up documents by $sequence instead of $id, which avoids a race window where a concurrent write between the operator update and the refetch could return the wrong row. It also adds explicit chunking to respect maxQueryValues, sets an explicit Query::limit per chunk (preventing the default find limit from silently dropping rows in large batches), moves the single-document refetch outside the transaction, and threads the caller's Query::select list through to the refetch so the projected fields stay consistent.

• refetchDocuments now chunks sequences by maxQueryValues, sets a per-chunk Query::limit, and keys the result map on $sequence (which SQL adapters always include in the projection even under a custom select).
• updateDocuments operator path skips the decode() call (since find() already returns decoded docs), but the equivalent guard is absent in updateDocument where decode() is still called unconditionally after the refetch.
• Two new E2E tests cover the select-projection preservation and the large-batch (>default-limit) refetch cases.

Confidence Score: 4/5

The core refetch logic is correct and well-tested; a decode inconsistency between the two update paths warrants attention before merge.

The updateDocument single-document path unconditionally calls decode() on a document that was just replaced by the result of refetchDocuments (which already returns a decoded document from find()). The PR adds an explicit guard in updateDocuments to prevent this, with a comment explaining that double-applying decode filters corrupts data, but the identical fix is missing in updateDocument. Any non-idempotent decode filter (type coercions, date parsing, etc.) will be applied twice to the returned document for every single-document operator update.

src/Database/Database.php — specifically the updateDocument method around the post-refetch decode() call at line 6426.

Important Files Changed

Filename	Overview
src/Database/Database.php	Switches operator-refetch from $id to $sequence, adds chunking+explicit limit, preserves caller selections in updateDocuments. updateDocument unconditionally calls decode() on a refetched (already-decoded) document — the same double-decode the PR explicitly guards against in updateDocuments.
tests/e2e/Adapter/Scopes/OperatorTests.php	Adds two new E2E tests: one validating that operator refetch honours caller's Query::select projection, another confirming batches larger than find()'s default limit are fully refetched.

_{Reviews (4): Last reviewed commit: "Add tests and chunk logic" | Re-trigger Greptile}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/Database/Database.php (1)

812-842: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Add an explicit limit to the refetch query. find() falls back to a 25-row limit when no Query::limit() is present, so bulk calls to refetchDocuments() can return stale pre-operator documents for everything past the first 25. Use Query::limit(count($sequences)) here, and chunk the sequence list if needed for adapter limits.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Database/Database.php` around lines 812 - 842, The refetchDocuments()
flow currently calls find() with only a Query::equal('$sequence', $sequences)
filter, so it can silently stop at the default 25-result cap. Update the refetch
query in Database::refetchDocuments() to include
Query::limit(count($sequences)), and if the sequence list can exceed adapter
query limits, split the lookup into chunks and merge the results before
rebuilding the refetchedMap.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/Database/Database.php`:
- Around line 812-842: The refetchDocuments() flow currently calls find() with
only a Query::equal('$sequence', $sequences) filter, so it can silently stop at
the default 25-result cap. Update the refetch query in
Database::refetchDocuments() to include Query::limit(count($sequences)), and if
the sequence list can exceed adapter query limits, split the lookup into chunks
and merge the results before rebuilding the refetchedMap.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 91a5b9c5-ff4c-40bf-97eb-ffa188eb1459

📥 Commits

Reviewing files that changed from the base of the PR and between a090699 and 5803460.

📒 Files selected for processing (1)

• src/Database/Database.php

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/Database/Database.php`:
- Around line 827-833: The refetch logic in refetchDocuments() is still relying
on the default find() limit, so batches larger than 25 can leave later documents
using stale computed/operator values. Update the query passed to
Database::find() to explicitly bound it to the current batch size (or the number
of requested sequence IDs) using Query::limit(), alongside the existing
Query::equal('$sequence', $sequences) and $selections so every document in the
batch is actually refetched.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0b45f00f-1ddd-49aa-b7e4-a4c42b4548ca

📥 Commits

Reviewing files that changed from the base of the PR and between 5803460 and c67b757.

📒 Files selected for processing (1)

• src/Database/Database.php