add ordered query combinator

[insipx]

Add ordered query combinator and modify group message querying to return ordered envelopes via queries::combinators::ordered_query::OrderedQuery with network backoff dependency resolution

Introduce ordered querying that resolves dependencies with exponential backoff, switch topic handling from raw bytes to Topic, add least-merge vector clock operations, and update group message queries to build a TopicCursor via LCC and return ordered envelopes.

📍Where to Start

Start with ordered behavior in queries by reviewing MlsApiImpl::query_group_messages in xmtp_api_d14n/src/queries/d14n/mls.rs, then follow into queries::combinators::ordered_query::OrderedQuery in xmtp_api_d14n/src/queries/combinators/ordered_query.rs and the resolver in xmtp_api_d14n/src/protocol/resolve/network_backoff.rs.

---

Macroscope summarized 43ff1b3.

[insipx]

This PR is part of a stack of 11 bookmarks:

1. trunk()
2. insipx/dependency-resolution
3. insipx/causal-sort
4. insipx/resolution
5. insipx/network-backoff
6. insipx/ordered-combinator ← this PR
7. insipx/test-resolution
8. insipx/test-ordering
9. insipx/add-published-in-epoch
10. insipx/sql-queries
11. insipx/sync-conn
12. insipx/set-dependencies

---

Created with jj-stack

[claude]

Claude finished @insipx's task —— View job

---

Code Review - November 18, 2025, 4:58 PM UTC

• Read key implementation files
• Analyze code quality and potential issues
• Check for bugs, performance, and security concerns
• Provide feedback

---

Review Summary

The ordered query combinator implementation is well-structured with good separation of concerns. Here are the key findings:

🔴 Critical Issues

1. Missing trait method implementation (Bug)

In xmtp_api_d14n/src/protocol/resolve/network_backoff.rs:103, the code calls acc.apply_least(&val.cursor), but apply_least is not part of the VectorClock trait definition in xmtp_api_d14n/src/protocol/traits/vector_clock.rs:5-23.

The trait only defines: dominates, missing, merge, merge_least, compare, and apply.

However, the implementation calls apply_least on a GlobalCursor, which has this as a direct method (not from the trait) in xmtp_proto/src/types/global_cursor.rs:39. This creates confusion and tight coupling to the concrete type rather than the trait abstraction.

Recommendation: Either add apply_least(&mut self, cursor: &Cursor) to the VectorClock trait definition, or refactor the code to use trait methods only.

⚠️ Performance Concerns

2. Unnecessary clone in ordered_query.rs:39

The TODO comment indicates awareness of a clone that might be avoidable:

.topic_cursor(self.topic_cursor.clone())

Since OrderedQuery already owns topic_cursor, consider whether the builder could take ownership via a mutable reference or restructure to avoid the clone.

3. Exponential backoff may cause long delays

In network_backoff.rs:51-69, the resolver uses exponential backoff with no apparent timeout limit beyond what ExponentialBackoff::default() provides. For missing dependencies that may never arrive, this could lead to extended blocking.

Recommendation: Document the timeout behavior or consider adding configurable timeout limits.

🟡 Code Quality

4. Error handling could be more specific

In dependency_resolution.rs:79, the error message is generic:

#[error("Resolution failed  to find all missing dependant envelopes")]

Note: Also has a double space before "to" (typo).

Consider including context about which envelopes failed to resolve or how many attempts were made.

5. Box increases allocations

In dependency_resolution.rs:76 and traits/error.rs:22,49, using Box<dyn RetryableError> for errors adds heap allocations. While necessary for trait object storage, consider whether the most common error cases could use direct enum variants instead.

✅ Positive Observations

• Clean combinator pattern following existing API design
• Good use of builder pattern with derive_builder
• Proper async trait handling for WASM/non-WASM targets
• Vector clock operations (merge_least) correctly implement LCC semantics
• Warning logs for unresolved dependencies aid debugging

📊 Test Coverage Note

Codecov shows 50.79% patch coverage with significant gaps in:

• dependency_resolution.rs (14 lines missing)
• impls.rs (9 lines missing)
• Error handling paths

Author noted tests are in #2802, which is appropriate for this PR stack structure.

---

[codecov]

Codecov Report

❌ Patch coverage is 50.79365% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.51%. Comparing base (fdb57f6) to head (ae330f1).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
..._d14n/src/protocol/traits/dependency_resolution.rs	0.00%	14 Missing ⚠️
xmtp_proto/src/api_client/impls.rs	25.00%	9 Missing ⚠️
xmtp_proto/src/traits.rs	0.00%	4 Missing ⚠️
xmtp_proto/src/traits/error.rs	0.00%	3 Missing ⚠️
xmtp_api_d14n/src/protocol/impls/vector_clock.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2782      +/-   ##
==========================================
+ Coverage   74.43%   74.51%   +0.07%     
==========================================
  Files         376      378       +2     
  Lines       48820    48879      +59     
==========================================
+ Hits        36338    36420      +82     
+ Misses      12482    12459      -23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[insipx]

this builder was a bit annoying b/c it required more Clone in trait args so i got rid of it in favor of a standalone function. Using default exponential backoff for now anyway, so only one argument

[insipx]

tests in #2802