Optimize get sequences

[fogelito]

Summary by CodeRabbit

• Bug Fixes
  • Improved document creation and upsert performance by avoiding unnecessary sequence lookups.
  • Ensured existing documents retain their original sequences during mixed create-and-update operations.
  • Corrected sequence handling for documents across multiple tenants.
  • Upsert callbacks now consistently report sequences matching those stored with the documents.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 99a10274-02c1-47db-bfc8-5e3530dea794

📥 Commits

Reviewing files that changed from the base of the PR and between 20ed701 and 76e7801.

📒 Files selected for processing (4)

• src/Database/Adapter/SQL.php
• src/Database/Database.php
• tests/e2e/Adapter/Scopes/DocumentTests.php
• tests/e2e/Adapter/Scopes/GeneralTests.php

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

Changes

Sequence retrieval and tenant binding

Layer / File(s)	Summary
Deduplicate shared-table tenant bindings src/Database/Adapter/SQL.php	getSequences binds one tenant placeholder per distinct tenant and passes the distinct tenant count to getTenantQuery.
Gate batch sequence and refetch operations src/Database/Database.php	Create and upsert paths fetch sequences only when callbacks or relationships require them. Upserts preserve sequences from existing rows and gate operator refetching on callback usage.
Validate created and upserted sequences tests/e2e/Adapter/Scopes/DocumentTests.php, tests/e2e/Adapter/Scopes/GeneralTests.php	End-to-end tests verify emitted sequences, stored sequences, mixed-batch preservation, and cross-tenant sequence values.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🔵 Low · up to 76e78

The PR reduces redundant sequence lookups and preserves authorization, but a mixed-tenant batch containing the same document ID could associate a sequence with the wrong tenant. This bounded data-isolation risk is mergeable with explicit owner awareness and follow-up to preserve compound tenant/document identity and add coverage.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant Database
  participant SQLAdapter
  participant Storage
  Caller->>Database: create or upsert documents
  Database->>Storage: write document batch
  alt callback or relationships require documents
    Database->>SQLAdapter: getSequences()
    SQLAdapter-->>Database: return sequences
  else existing upsert rows
    Database->>Database: preserve sequences from existing documents
  end
  Database-->>Caller: emit documents through onNext

Loading

Suggested reviewers: abnegate

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately identifies the main change: optimizing sequence retrieval and binding. It is concise and related to the implementation and tests.
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.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch optimize-get-sequences

---

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 reduces redundant sequence lookups during batch creates and upserts while preserving callback-visible sequences and existing-row sequence values.

• Deduplicates tenant bindings in SQL sequence queries.
• Avoids sequence retrieval when no callback or relationship processing needs the result.
• Reuses sequences already obtained for existing upserted documents.
• Avoids operator-result refetches when no callback consumes the result.
• Adds mixed-batch and cross-tenant sequence coverage.

Confidence Score: 5/5

The PR appears safe to merge with no actionable correctness or security defects identified.

The optimized paths preserve tenant placeholder alignment, adapter batch ordering, callback-visible sequences, relationship processing, cache invalidation, and batch event behavior.

Important Files Changed

Filename	Overview
src/Database/Adapter/SQL.php	Deduplicates tenant placeholders and keeps the generated tenant predicate aligned with the bound values.
src/Database/Database.php	Optimizes batch result shaping while retaining sequence retrieval and operator refetches on paths where documents are consumed.
tests/e2e/Adapter/Scopes/DocumentTests.php	Adds callback sequence assertions and coverage for interleaved existing and newly inserted upsert rows.
tests/e2e/Adapter/Scopes/GeneralTests.php	Extends tenant-per-document coverage to verify callback sequences across distinct tenants.

_{Reviews (1): Last reviewed commit: "Run" | Re-trigger Greptile}