Release v1.6.2

[neekolas]

Release v1.6.2

Add support for filtering and sorting by inserted_at time

Merge branch '11-19-add_support_for_filtering_and_sorting_by_inserted_at_time' into 11-25-release_v1.6.2

Merge branch '11-19-add_support_for_filtering_and_sorting_by_inserted_at_time' into 11-25-release_v1.6.2

Merge branch '11-25-release_v1.6.2' into release

[neekolas]

• Release v1.6.3 #505
• Release v1.6.2 #504 👈 (View in Graphite)
• main

---

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.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[claude]

Claude finished @neekolas's task —— View job

---

PR Review Complete ✅

I've completed a comprehensive review of Release v1.6.2. Here's my detailed feedback:

---

Summary of Changes

This PR introduces several new features:

1. Time-based filtering: Added insertedAfterNs and insertedBeforeNs parameters for message queries
2. Sort selection: New SortBy enum (SENT_TIME vs INSERTED_TIME) for message ordering
3. App data support: Added appData parameter for groups/conversations and updateAppData() method
4. FFI updates: New codec functions, admin tracking in FfiGroupUpdated, and content length type change

---

🔴 Critical Issues

1. Merge Conflict Left in Code (BLOCKER)

Location: library/src/main/java/org/xmtp/android/library/Conversations.kt:170-174

There's an unresolved git merge conflict marker in the production code:

<<<<<<< HEAD
    appData: String? = null,
    =======
    appData: String?,
    >>>>>>> 502528e9 (Release v1.6.2)

Impact: This will cause compilation failure.
Fix Required: Remove conflict markers and decide on the correct parameter signature (likely should be appData: String? = null to maintain backward compatibility).

2. Breaking API Change: contentLength Type

Location: library/src/main/java/xmtpv3.kt:13476

The FfiRemoteAttachment.contentLength field changed from ULong to UInt:

- var `contentLength`: kotlin.ULong,
+ var `contentLength`: kotlin.UInt,

Impact: This is a breaking change that could cause issues if:

• Files larger than ~4GB (UInt max: 4,294,967,295 bytes) are being handled
• Existing code expects ULong values

Recommendation: Verify this change is intentional and document the size limitation. If this is a fix for an upstream change, ensure consumers are aware of potential overflow issues with large files.

---

⚠️ High Priority Issues

3. Nullable appData Without Default in One Overload

Location: library/src/main/java/org/xmtp/android/library/Conversations.kt:173

One function signature has appData: String? (non-nullable type with nullable value) without a default:

suspend fun newGroupWithIdentities(
    // ... other params
    appData: String?,  // No default value
)

While newGroupCustomPermissionsWithIdentities at line 202 has appData: String? = null.

Impact: Inconsistent API - some functions require explicit null while others don't.
Recommendation: Add = null default consistently across all overloads for better API ergonomics.

4. Missing Documentation for New Features

The new filtering and sorting parameters lack KDoc documentation:

• What timezone/epoch are the insertedAfterNs/insertedBeforeNs timestamps using?
• What's the difference in behavior between SENT_TIME vs INSERTED_TIME sorting?
• Are the time filters inclusive or exclusive?
• What does appData represent and what are its size/format constraints?

Recommendation: Add KDoc comments explaining:

/**
 * Filter messages inserted after this timestamp (nanoseconds since Unix epoch).
 * Inclusive. Pass null to disable this filter.
 */
insertedAfterNs: Long? = null,

---

🟡 Medium Priority Issues

5. No Validation for Time Range Filters

Location: Group.kt, Dm.kt (multiple message query methods)

The code doesn't validate that insertedBeforeNs >= insertedAfterNs when both are provided.

Impact: Invalid time ranges could cause confusing empty results or unexpected backend errors.
Recommendation: Add validation:

require(insertedBeforeNs == null || insertedAfterNs == null || 
        insertedBeforeNs >= insertedAfterNs) {
    "insertedBeforeNs must be >= insertedAfterNs"
}

6. AppData Size Constraints Unknown

Location: Group.kt:505 - updateAppData() method

The updateAppData() accepts a raw String with no apparent size limit validation at the Kotlin layer.

Impact: Large strings could cause performance issues, network failures, or storage problems.
Recommendation: Document maximum size limits. Consider adding client-side validation if there's a known backend limit.

7. Exception Handling Could Be More Specific

Location: Group.kt:509

} catch (e: Exception) {
    throw XMTPException("Permission denied: Unable to update group app data", e)
}

This catches all exceptions and reports them as permission errors, which may not be accurate (could be network error, serialization error, etc.).

Recommendation: Catch specific exception types or improve error message:

} catch (e: Exception) {
    throw XMTPException("Failed to update group app data: ${e.message}", e)
}

---

🟢 Code Quality & Best Practices

Positive Aspects ✅

• Proper coroutine usage: All new async operations use withContext(Dispatchers.IO) correctly
• Null safety: Parameters properly marked as nullable with ?
• Consistent naming: New parameters follow existing conventions (insertedAfterNs matches createdAfterNs pattern)
• FFI integration: Checksum validation properly updated for new FFI functions
• Backward compatibility: Most changes are additive with default values (except the merge conflict issue)

Minor Suggestions

1. Consider using named parameters in complex function calls with many optional parameters to improve readability
2. The enum mapping (e.g., SortBy.SENT_TIME -> FfiSortBy.SENT_AT) is clean but appears in multiple places - could be extracted to an extension function

---

🔒 Security Considerations

8. AppData Content Type Not Enforced

The appData: String parameter accepts arbitrary string content. While flexibility is good, consider:

• Is this data user-controllable? If so, ensure consumers sanitize before display
• Should there be format validation (e.g., JSON schema)?
• Is this data encrypted at rest/in transit by the underlying libxmtp?

Recommendation: Document security considerations for appData usage in KDoc.

9. Admin Role Tracking Added

The FfiGroupUpdated now tracks admin role changes:

var `addedAdminInboxes`: List<FfiInbox>,
var `removedAdminInboxes`: List<FfiInbox>,
var `addedSuperAdminInboxes`: List<FfiInbox>,
var `removedSuperAdminInboxes`: List<FfiInbox>,

Good: This enables proper permission tracking for group administration.
Question: Are these changes reflected in the Kotlin wrapper classes (Group, etc.)? Ensure consumers can access this information.

---

📊 Performance Considerations

10. Time-Based Filtering Performance

The new insertedAfterNs/insertedBeforeNs filters are passed to FFI but:

• Good: Filtering happens at the database layer (via FFI), not in memory
• Question: Are there database indices on inserted_at timestamps? If not, these queries could be slow on large message sets

11. Sort Order Impact

Two sort options now available (SENT_AT vs INSERTED_AT):

• SENT_AT: Natural message order, but could have clock skew issues
• INSERTED_AT: Database insertion order, consistent but may differ from send order

Recommendation: Document performance characteristics in KDoc, especially if one is significantly faster.

---

🧪 Test Coverage

Current State

Existing test files (GroupTest.kt, DmTest.kt) contain references to the new parameters, suggesting tests exist.

Missing Test Coverage (Recommendations)

Based on this PR, ensure tests cover:

1. ✅ Time filtering edge cases:

   • Messages exactly at boundary timestamps
   • Empty results when no messages match time range
   • Reverse time ranges (before < after)

2. ✅ Sort order verification:

   • Verify SENT_TIME vs INSERTED_TIME produce different orderings when applicable
   • Test with messages sent out-of-order

3. ✅ AppData functionality:

   • Creating groups with appData
   • Updating appData via updateAppData()
   • Permission denied scenarios for appData updates
   • Null/empty appData handling

4. ✅ Admin tracking:

   • Verify FfiGroupUpdated properly deserializes admin changes
   • Confirm admin role changes are observable

---

📝 Summary & Recommendations

Must Fix Before Merge

1. ❌ Remove merge conflict markers in Conversations.kt:170-174
2. ⚠️ Verify intentionality of contentLength type change (ULong → UInt)

Should Fix Before Merge

3. Add default value for appData parameter consistency
4. Add KDoc documentation for new parameters
5. Add validation for time range parameters

Consider for Follow-up

6. Document appData size limits and security considerations
7. Extract repeated enum mapping logic to extension functions
8. Verify database indexing for inserted_at queries
9. Ensure admin role changes are accessible in wrapper classes

---

Final Verdict

Status: ⚠️ Needs Changes

This PR introduces useful features (time-based filtering, sort selection, app data) with generally good code quality. However, the merge conflict in production code is a blocker that must be resolved before merge. The contentLength type change also needs verification to ensure it's intentional and won't cause issues with large files.

Once the merge conflict is resolved and the type change is verified, this PR will be ready to merge. The documentation and validation improvements are highly recommended but could potentially be addressed in follow-up PRs if time-sensitive.

Effort to fix critical issues: ~5 minutes
Effort for recommended improvements: ~30-60 minutes

---

[macroscopeapp]

Release v1.6.2 and add inserted-time filtering, sort selection, app data APIs, and auth callback wiring across org.xmtp.android.library and xmtpv3.kt

Introduce inserted-time filters and DecodedMessage.SortBy, add optional appData for conversations and groups including suspend updateAppData, make Client.connectToBackend pass FfiClientMode.DEFAULT with new args and make ffiRevokeAllOtherInstallations nullable, and wire new auth and message-deletion callbacks with checksum updates in xmtpv3.kt.

📍Where to Start

Start with the FFI surface and checksum updates in xmtpv3.kt, then review insertedAfterNs/insertedBeforeNs/sortBy propagation in org.xmtp.android.library.Conversation, Dm, and Group.

---

📊 Macroscope summarized 61cccc8. 1 file reviewed, 3 issues evaluated, 3 issues filtered, 0 comments posted. View details