xmtp_db: query to find last commit in a group from payload hash

[insipx]

Add xmtp_db::group_intent::QueryGroupIntent::find_dependant_commits to query the last commit in a group by payload hash

Introduce dependency resolution for intent payload hashes in xmtp_db, add GroupIntentError and IntentDependency, update StorageError to wrap group intent errors, and rename WASM text encode/decode exports.

📍Where to Start

Start with the find_dependant_commits implementation in group_intent.rs, then review error types in error.rs and dependency structs in types.rs.

---

📊 Macroscope summarized a139392. 7 files reviewed, 3 issues evaluated, 2 issues filtered, 1 comment posted

🗂️ Filtered Issues

xmtp_db/src/encrypted_store/group_intent.rs — 1 comment posted, 3 evaluated, 2 filtered

• line 512: Documentation/contract mismatch: The comment says "Returns a map from payload hash to a vector of dependencies (one per originator)." but the function returns Result<HashMap<PayloadHash, IntentDependency>, StorageError> and collapses multiple dependencies by erroring on d.len() > 1. This contradicts the stated behavior and creates uncertainty about the intended contract. [ Low confidence ]
• line 555: Missing payload hashes are silently dropped from the output. The function builds the map only from rows returned by the join and then transforms it with into_iter().map(...), never checking which input payload_hashes had no dependency. Therefore some inputs may not reach a defined outcome (neither a returned dependency nor an explicit error), violating the function’s stated intent to find dependencies for each payload hash. [ Already posted ]

[insipx]

This PR is part of a stack of 3 bookmarks:

1. trunk()
2. push-tkyvzsqvnkyz
3. insipx/dependency-query ← this PR
4. insipx/set-dependencies

---

Created with jj-stack

[graphite-app]

How to use the Graphite Merge Queue

Add the label mergequeue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

[claude]

Claude finished @insipx's task —— View job

---

Code Review — December 1, 2025, 15:05 UTC

Reviewed the dependency query implementation in xmtp_db. Overall, the approach is solid with good SQL optimization using a single join query. Here are my findings:

Critical Issues

1. Error handling logic issue (xmtp_db/src/encrypted_store/group_intent.rs:558-571)

The code checks d.len() > 1 and errors, then calls d.pop() which could theoretically return None. The comment says "this should be impossible" but the logic is confusing:

if d.len() > 1 {
    return Err(GroupIntentError::MoreThanOneDependency { ... });
}
let dep = d.pop().ok_or_else(|| GroupIntentError::NoDependencyFound { ... })?;

If d.len() > 1 returns early, then d.len() is either 0 or 1. If it's 0, we get NoDependencyFound, but this contradicts the SQL query which wouldn't return empty groups in the into_grouping_map() result. Consider:

• Adding a debug assertion: debug_assert!(!d.is_empty(), "grouping_map should not return empty vectors");
• Or simplifying to: let dep = d.into_iter().next().ok_or_else(...)?; after the length check

2. RetryableError semantics incorrect (xmtp_db/src/encrypted_store/group_intent/error.rs:23-30)

Both MoreThanOneDependency and NoDependencyFound return is_retryable() = true, but these are deterministic validation errors that won't resolve on retry:

• Multiple dependencies in the same epoch is a data integrity issue
• Missing dependency won't appear on retry without external state changes

These should return false to avoid infinite retry loops. If the intent is to wait for state changes, this needs explicit documentation and bounded retry logic at the call site.

Performance & Correctness

3. Missing empty input guard (xmtp_db/src/encrypted_store/group_intent.rs:525)

Already flagged by Macroscope but worth emphasizing: empty payload_hashes will generate invalid SQL. Add early return:

if payload_hashes.is_empty() {
    return Ok(HashMap::new());
}

Code Quality

4. Documentation inconsistency (xmtp_db/src/encrypted_store/group_intent.rs:512)

Comment says "Returns a map from payload hash to a vector of dependencies" but signature returns HashMap<PayloadHash, IntentDependency> (not a vector). The implementation enforces exactly one dependency per hash. Update the doc comment to match the actual contract.

5. CursorList Display without delimiter (xmtp_proto/src/types/cursor_list.rs:34-40)

Multiple cursors will be concatenated without separation: [sid(1):oid(2)][sid(3):oid(4)] becomes hard to parse. Consider:

write!(f, "{}", self.inner.iter().map(|c| c.to_string()).collect::<Vec<_>>().join(", "))

Test Coverage

Positive: Test at line 1019 covers the happy path with multiple payload hashes.

Missing coverage (per codecov):

• Empty payload_hashes input (would catch the SQL bug)
• MoreThanOneDependency error path (0% coverage in error.rs)
• NoDependencyFound error path
• Payload hashes not found in DB (silent omission mentioned by Macroscope)

Minor

6. Type naming (xmtp_db/src/encrypted_store/group_intent/types.rs:21)

PayloadHashRef<'static> aliased as PayloadHash is elegant, but the Ref suffix on the main type is slightly confusing since PayloadHash (without Ref) is actually the owned version. Consider renaming or adding doc comments to clarify.

---

Summary

• 2 critical issues (error handling, retryability)
• 1 correctness issue (empty input)
• 3 code quality improvements
• Test coverage gaps for error paths

The SQL query implementation is efficient and well-structured. Main concerns are around error handling semantics and edge case handling.

---

[codecov]

Codecov Report

❌ Patch coverage is 71.87500% with 45 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.93%. Comparing base (1e1bf34) to head (a139392).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
xmtp_proto/src/types/cursor_list.rs	0.00%	17 Missing ⚠️
xmtp_db/src/encrypted_store/group_intent/types.rs	42.85%	12 Missing ⚠️
xmtp_db/src/encrypted_store/group_intent.rs	91.37%	10 Missing ⚠️
xmtp_db/src/encrypted_store/group_intent/error.rs	0.00%	5 Missing ⚠️
xmtp_db/src/errors.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2843      +/-   ##
==========================================
- Coverage   73.95%   73.93%   -0.02%     
==========================================
  Files         387      390       +3     
  Lines       49681    49841     +160     
==========================================
+ Hits        36741    36851     +110     
- Misses      12940    12990      +50     

☔ 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.

[macroscopeapp]

Empty payload_hashes leads to eq_any([]) → IN () on SQLite, causing a runtime error. Consider returning an empty map early when payload_hashes is empty.

+         if payload_hashes.is_empty() {
+             return Ok(HashMap::new());
+         }

🚀 Reply to ask Macroscope to explain or update this suggestion.

👍 Helpful? React to give us feedback.