feat(swift-example-app): document count aggregation view (DOC-10/11/12)#3926
Conversation
Add a read-only "Count Documents" view that exercises document COUNT
aggregation (total / where-filtered / group_by) over the already-exported
FFI `dash_sdk_document_count`.
- SwiftDashSDK: thin `documentCount(...) -> DocumentCountResult` wrapper
in PlatformQueryExtensions (call -> marshal -> return; parses the
{"counts":{hexKey:u64}} result, total under the empty-string key).
- SwiftExampleApp: CountDocumentsView reached via Settings -> State
Transitions -> Document -> Count Documents; accessible navigationLink
contract/type pickers + where/group_by JSON fields + result display.
- TEST_PLAN: split the former DOC-08 (count/sum/avg, was no-UI) into
DOC-10 (count total), DOC-11 (count filtered), DOC-12 (count grouped),
now drivable, plus DOC-13 (sum) / DOC-14 (average) which stay blocked
because the FFI sum/average entry points return NotImplemented (blocked
upstream on grovedb PR 670).
No Rust/xcframework change: dash_sdk_document_count was already exported.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
✅ Review complete (commit f133c2a) |
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds document count aggregation to the Swift SDK: a new ChangesDocument Count Aggregation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Swift-only PR that adds a thin documentCount FFI wrapper and a read-only CountDocumentsView. Marshaling, ownership, and the limit==0 guard match the existing documentList/documentGet precedent and the FFI contract. No consensus, Rust, signing, or cryptography touched. Two minor nitpicks verified: a UX wart in the grouped-result rendering and a theoretical u64 precision gap through JSONSerialization.
💬 2 nitpick(s)
| Section("Total") { | ||
| HStack { | ||
| Text("Count") | ||
| Spacer() | ||
| Text(result.total.map(String.init) ?? "—") | ||
| .fontWeight(.bold) | ||
| .foregroundColor(.primary) | ||
| .accessibilityIdentifier("countDocuments.totalCount") | ||
| } | ||
| } |
There was a problem hiding this comment.
💬 Nitpick: "Total" section shows "—" when results are grouped
When group_by is set, the FFI omits the empty-string aggregate entry, so result.total is nil and the Section("Total") row renders Count — above the per-group rows. The dash above grouped data reads as a failed-to-compute total. Hide the Total section when result.isGrouped, or surface a derived sum across groups. total: UInt64? is correctly typed — purely a presentation issue.
source: ['claude']
| var counts: [String: UInt64] = [:] | ||
| for (key, value) in countsObject { | ||
| if let num = value as? NSNumber { | ||
| counts[key] = num.uint64Value | ||
| } | ||
| } |
There was a problem hiding this comment.
💬 Nitpick: u64 counts above Int64.max lose precision through JSONSerialization → uint64Value
The Rust side serializes counts as raw JSON integers from BTreeMap<String, u64>. Foundation's JSONSerialization only guarantees fidelity for integers in the Int64 range — values in (Int64.max, UInt64.max] are typically returned as a Double-backed NSNumber, and num.uint64Value then silently truncates/rounds. Document counts will not realistically reach 2^63, so this is theoretical rather than a real bug. If you want full round-trip fidelity, decode with JSONDecoder into a Codable struct keyed [String: UInt64], which preserves UInt64 precision.
source: ['claude', 'codex']
Issue being fixed or feature implemented
DOC-08in the iOS test plan lumped document count / sum / average aggregation into one row marked 🔌 SDK-only, no UI — nothing in SwiftExampleApp drove it, so it could never be QA'd through the app. Document COUNT is in fact fully implemented at the FFI layer (dash_sdk_document_count, proof-verified); only SUM/AVERAGE are still upstream-blocked. This adds the missing app surface for COUNT and splits the catalog row so each capability is tracked independently.What was done?
PlatformQueryExtensions.swift): added a thin read-only bridgedocumentCount(dataContractId:documentType:whereJSON:orderByJSON:groupByJSON:limit:) -> DocumentCountResultover the already-exported FFIdash_sdk_document_count(call → marshal → return; parses the{"counts":{hexKey:u64}}result; total is the empty-string key, grouped results are per-hex-key). Follows the existingdocumentList/documentGetpattern — no business logic.CountDocumentsView.swift, new;TransitionCategoryView.swift): a "Count Documents" read view reachable via Settings → State Transitions → Document → Count Documents. Accessible (navigationLink) contract + document-type pickers, optionalwhere/group_byJSON fields, a Run button, and total / per-group / error result sections.DOC-08(now an ➖ umbrella pointer) into:DOC-10count — total,DOC-11count — filtered (where),DOC-12count — grouped (group_by) → all 🧪, now drivable.DOC-13sum,DOC-14average → 🚫, because the FFIdash_sdk_document_sum/_averageentry points returnNotImplemented(blocked upstream on grovedb PR 670). Updated the §6 Document index, Appendix A read-query row, and the completeness lists accordingly.No Rust / xcframework change —
dash_sdk_document_countwas already implemented and exported in the header; this is Swift-only.How Has This Been Tested?
Built for the iOS Simulator (iPhone 16,
iphonesimulator) —** BUILD SUCCEEDED **— and driven end-to-end on testnet against a purpose-built countable contract (G72GKU5X…, doc typeitemwithdocumentsCountable: true+ abyCategorycountable index, seeded with category a×3 / b×2 / c×1):where category == "a"): result 3 ✅where category in ["a","b","c"]+group_by ["category"]): per-group {a:3, b:2, c:1} ✅ (group_by requires anIn/rangewhereper the platform; the FFI operator token is lowercasein)All three recorded
pass, andDOC-13/DOC-14recordedblocked(grovedb PR 670), to the QA contract; they render on the live QA dashboard.Breaking Changes
None.
Checklist:
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation