-
Notifications
You must be signed in to change notification settings - Fork 13
add write operations, example applications, and comprehensive documentation #180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ocumentation (#129) Second alpha release adding write capabilities, production-ready examples, and extensive CloudKit development documentation. ## New Public API Write Operations: - Add RecordOperation with factory methods (create, update, delete variants) - Add CloudKitService.modifyRecords() for batch operations - Add RecordManaging protocol with query/modify/delete methods - Add FilterBuilder and SortDescriptor for type-safe queries - Add CloudKitRecord/CloudKitRecordCollection protocols - Add convenience extensions for record operations Field Value Improvements: - Add bidirectional OpenAPI conversion (FieldValue ↔ Components.Schemas.FieldValue) - Add FieldValue convenience accessors (stringValue, int64Value, etc.) - Add memberwise initializer to CustomFieldValue - Refactor conversion logic into protocol-based initializers Error Handling: - Add CloudKitResponseType protocol for unified error extraction - Improve CloudKitError with operation-specific context - Add Sendable conformance throughout for Swift 6 ## Example Applications Bushel (Examples/Bushel/): - Production CLI tool syncing macOS restore images, Xcode, and Swift versions - Multi-source data fetching (ipsw.me, AppleDB, MESU, Mr. Macintosh) - Server-to-Server authentication with automatic key rotation - Metadata tracking and intelligent fetch throttling - Tutorial-quality logging and comprehensive documentation - CloudKit schema automation with cktool scripts Celestra (Examples/Celestra/): - RSS feed reader with scheduled CloudKit sync - Web authentication demonstration - Public database architecture for shared content - Clean reference implementation for common patterns MistDemo (Examples/MistDemo/): - Restored and updated authentication demonstration - AdaptiveTokenManager showing runtime auth switching - CloudKit JS web authentication flow with Hummingbird server - Swift 6 strict concurrency compliant ## Documentation CloudKit Reference Documentation (.claude/docs/): - CloudKit Web Services API reference (webservices.md) - CloudKit JS framework guide (cloudkitjs.md) - Schema language reference and quick guides - cktool and cktooljs command documentation - Public database architecture patterns - Data source API research and integration patterns DocC Articles (Sources/MistKit/Documentation.docc/): - OpenAPICodeGeneration.md - swift-openapi-generator setup - AbstractionLayerArchitecture.md - Modern Swift patterns and design - Updated Documentation.md with new article references Development Guides: - CLAUDE.md updates with logging, schema, and development workflows - Bushel implementation notes and CloudKit setup guides - Schema design workflow documentation ## Additional Changes - Update GitHub workflows for improved CI - Add .gitignore patterns for sensitive files - Improve log redaction patterns (MISTKIT_DISABLE_LOG_REDACTION support) - Add Array+Chunked utility for batch processing - Update devcontainer configuration ## Breaking Changes None - this release is additive only, maintaining full backward compatibility with v1.0.0-alpha.1. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughPR adds comprehensive CloudKit reference documentation in Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SyncCommand
participant DataSourcePipeline
participant Fetcher as Data Fetcher<br/>(IPSW/MESU/AppleDB/etc)
participant BushelCloudKitService
participant CloudKitService
participant CloudKit
User->>SyncCommand: bushel-images sync [options]
SyncCommand->>SyncCommand: Resolve CloudKit credentials
SyncCommand->>SyncCommand: Initialize SyncEngine
SyncEngine->>DataSourcePipeline: fetch(options)
par Parallel Data Fetches
DataSourcePipeline->>Fetcher: fetch() - RestoreImages
Fetcher->>Fetcher: Query remote API/HTML
Fetcher-->>DataSourcePipeline: [RestoreImageRecord]
end
DataSourcePipeline->>DataSourcePipeline: Deduplication & merge (by buildNumber)
DataSourcePipeline->>DataSourcePipeline: Resolve XcodeVersion→RestoreImage references
DataSourcePipeline-->>SyncEngine: FetchResult {images, xcode, swift}
alt dryRun disabled
SyncEngine->>BushelCloudKitService: executeBatchOperations(images)
BushelCloudKitService->>CloudKitService: modifyRecords (batch)
CloudKitService->>CloudKit: POST /records
CloudKit-->>CloudKitService: [RecordInfo]
BushelCloudKitService->>BushelCloudKitService: Partition into 200-item batches
BushelCloudKitService->>CloudKitService: modifyRecords (batch 2)
CloudKitService->>CloudKit: POST /records
BushelCloudKitService-->>SyncEngine: upload complete
SyncEngine->>BushelCloudKitService: Update DataSourceMetadata
end
SyncEngine-->>SyncCommand: SyncResult {counts}
SyncCommand->>User: Print summary & next steps
sequenceDiagram
participant User
participant UpdateCommand as UpdateCommand<br/>(Celestra)
participant CloudKitService
participant RSSFetcherService
participant RateLimiter
participant Article as Article<br/>Processing
User->>UpdateCommand: celestra update [filters]
UpdateCommand->>CloudKitService: queryFeeds(filters)
CloudKitService-->>UpdateCommand: [Feed]
loop For each Feed
UpdateCommand->>RateLimiter: waitIfNeeded(domain, minInterval)
RateLimiter-->>UpdateCommand: ready
UpdateCommand->>RSSFetcherService: fetchFeed(url, etag, lastModified)
alt 304 Not Modified
RSSFetcherService-->>UpdateCommand: wasModified=false
else Feed Updated
RSSFetcherService->>RSSFetcherService: Parse RSS via SyndiKit
RSSFetcherService-->>UpdateCommand: FeedData + new items
UpdateCommand->>UpdateCommand: Extract GUIDs from items
UpdateCommand->>CloudKitService: queryArticlesByGUIDs(guids, feedRecordName)
CloudKitService-->>UpdateCommand: [existing Article]
UpdateCommand->>Article: Dedup by GUID
Article->>Article: Partition: new vs modified
UpdateCommand->>CloudKitService: createArticles(new, batch=10)
CloudKitService-->>UpdateCommand: BatchOperationResult
UpdateCommand->>CloudKitService: updateArticles(modified, batch=10)
CloudKitService-->>UpdateCommand: BatchOperationResult
UpdateCommand->>CloudKitService: updateFeed(metadata + etag/lastModified)
end
end
UpdateCommand->>User: Print summary {success, notModified, skipped, errors}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring careful attention:
Possibly related issues
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main brightdigit/MistKit#180 +/- ##
=========================================
+ Coverage 8.32% 14.27% +5.95%
=========================================
Files 44 67 +23
Lines 7427 7179 -248
=========================================
+ Hits 618 1025 +407
+ Misses 6809 6154 -655
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Pull Request Review: v1.0.0-alpha.2SummaryThis is an excellent alpha release that successfully adds write operations, production-quality examples, and comprehensive documentation to MistKit. The code demonstrates strong software engineering practices with modern Swift patterns, proper abstraction layers, and extensive test coverage. ✅ StrengthsCode Quality & Best PracticesExcellent Architecture:
Modern Swift Patterns:
Field Value Conversion:
Error Handling:
Test CoverageComprehensive Testing:
Test Quality Highlights:
Documentation & ExamplesBushel Example (Production-Quality):
Documentation:
SecurityGood Practices:
🔍 Areas for ImprovementTest Coverage GapsMissing Write Operation Tests:
Recommendation: Add a test file Tests/MistKitTests/Service/CloudKitServiceWriteTests.swift with mock response tests similar to the existing CloudKitServiceQueryTests.swift. Potential Edge Cases1. RecordOperation Conversion Safety (Components+RecordOperation.swift:50-52) Using fatalError means this will crash at runtime if a new operation type is added but not mapped. Recommendation: Consider throwing a proper error instead, or make the enum exhaustive with @Frozen to get compile-time guarantees 2. Atomic Flag Hardcoded (CloudKitService+WriteOperations.swift:59) The atomic flag is always false, which may not be desired for all use cases. Recommendation: Consider adding an optional atomic parameter to modifyRecords() for users who need transactional guarantees 3. Array Chunking Edge Case (Array+Chunked.swift:39-42) The chunking logic looks correct, but lacks documentation about empty array behavior. Recommendation: Add a test case for empty array chunking and document the behavior Documentation SuggestionsMissing API Documentation:
Performance Considerations:
🎯 Minor IssuesCode Organization
Consistency
Logging
🔐 Security ReviewNo Security Concerns Found:
📊 Performance ConsiderationsGood Practices:
Potential Improvements:
🎓 Example QualityThe Bushel example is exceptional - it is not just demo code, it is production-ready with:
This sets a high bar for MistKit examples. ✅ Recommendations SummaryHigh Priority
Medium Priority
Low Priority
🏆 ConclusionOverall Assessment: Excellent Work! ⭐⭐⭐⭐⭐ This PR demonstrates:
The code is well-architected, maintainable, and follows modern Swift conventions. The few issues identified are minor and do not block the alpha release. Recommendation: Approve with suggested follow-up improvements in future PRs. Reviewed with focus on code quality, security, performance, and test coverage per the repository CLAUDE.md guidelines. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (14)
Examples/Bushel/XCODE_SCHEME_SETUP.md-9-16 (1)
9-16: Replace hardcoded user-specific paths with relative or placeholder paths.The documentation contains hardcoded paths (
/Users/leo/Documents/Projects/MistKit/) that won't work for other contributors. Use relative paths or placeholders instead.-3. Navigate to `/Users/leo/Documents/Projects/MistKit/Examples/Bushel/` +3. Navigate to the `Examples/Bushel/` directory in your MistKit checkout Alternatively, from Terminal: ```bash -cd /Users/leo/Documents/Projects/MistKit/Examples/Bushel +cd path/to/MistKit/Examples/Bushel open Package.swift</blockquote></details> <details> <summary>Examples/Bushel/XCODE_SCHEME_SETUP.md-64-66 (1)</summary><blockquote> `64-66`: **More hardcoded paths to update.** ```diff - **Working Directory**: - Select **Use custom working directory** - - Set to: `/Users/leo/Documents/Projects/MistKit/Examples/Bushel` + - Set to the `Examples/Bushel` directory in your MistKit checkoutExamples/Bushel/XCODE_SCHEME_SETUP.md-134-136 (1)
134-136: Additional hardcoded path.# Navigate to build products -cd /Users/leo/Documents/Projects/MistKit/Examples/Bushel/.build/arm64-apple-macosx/debug +cd .build/arm64-apple-macosx/debugExamples/Celestra/Sources/Celestra/Services/RateLimiter.swift-22-53 (1)
22-53: Usenowvariable consistently for timestamp recording.At line 52,
Date()is called to record the fetch time, but thenowvariable was already computed at line 27. This creates a small timing inconsistency between the delay calculation and the recorded timestamp.Apply this diff:
// Record this fetch time - lastFetchTimes[key] = Date() + lastFetchTimes[key] = nowAdditionally, consider explicitly propagating
Task.sleeperrors instead of silencing them withtry?at line 47, as this could mask task cancellation:if remainingDelay > 0 { let nanoseconds = UInt64(remainingDelay * 1_000_000_000) - try? await Task.sleep(nanoseconds: nanoseconds) + try await Task.sleep(nanoseconds: nanoseconds) }If you intentionally want to ignore cancellation, document this decision with a comment.
.claude/docs/sosumi-cloudkit-schema-source.md-101-101 (1)
101-101: Fix spelling error in documentation.The word "retreive" should be "retrieve".
Apply this diff:
-CloudKit doesn't preserve the comments in your schema when you upload them to CloudKit. Additionally, when you retreive the schema, the order of the elements may differ from the order when the schema was created or updated. +CloudKit doesn't preserve the comments in your schema when you upload them to CloudKit. Additionally, when you retrieve the schema, the order of the elements may differ from the order when the schema was created or updated..claude/docs/sosumi-cloudkit-schema-source.md-17-17 (1)
17-17: Fix spelling error in documentation.The word "specifed" should be "specified". While this appears to be sourced from Apple's documentation, correcting it improves clarity.
Apply this diff:
-When publishing a schema, CloudKit attempts to apply changes to the existing schema, if one exists, to convert it to the form specifed in the new schema. If the modifications required would result in potential data loss with respect to the current production schema (like removing a record type or field name that exists already), then the schema isn't valid and CloudKit makes no changes. +When publishing a schema, CloudKit attempts to apply changes to the existing schema, if one exists, to convert it to the form specified in the new schema. If the modifications required would result in potential data loss with respect to the current production schema (like removing a record type or field name that exists already), then the schema isn't valid and CloudKit makes no changes.Examples/Celestra/Sources/Celestra/Commands/ClearCommand.swift-15-27 (1)
15-27:--confirmhelp text doesn’t match behaviorThe flag help says “Skip confirmation prompt”, but there’s no interactive prompt: without
--confirmthe command just prints a warning and returns. To avoid confusing users, consider adjusting the help to reflect that--confirmis required to actually perform the deletion, e.g.:- @Flag(name: .long, help: "Skip confirmation prompt") + @Flag(name: .long, help: "Confirm deletion of all CloudKit data")The rest of the flow (printing a warning and requiring
--confirm) looks good.Examples/Bushel/Sources/BushelImages/DataSources/SwiftVersionFetcher.swift-29-30 (1)
29-30: Set locale on DateFormatter for consistent parsing.Without an explicit locale,
DateFormatteruses the device's current locale, which could cause parsing failures for English month abbreviations (e.g., "Nov") on non-English devices.let dateFormatter = DateFormatter() dateFormatter.dateFormat = "dd MMM yy" + dateFormatter.locale = Locale(identifier: "en_US_POSIX")Examples/Bushel/Sources/BushelImages/CloudKit/BushelCloudKitService.swift-117-123 (1)
117-123: Incorrect field used for error reason in logging.The error logging uses
errorRecord.recordTypefor the "reason", butrecordTypecontains the CloudKit record type (e.g., "RestoreImage"), not the error reason. IfRecordInfohas an error message field, use that instead; otherwise, consider inspecting the actual error structure.for errorRecord in errorRecords { BushelLogger.verbose( - "Error: recordName=\(errorRecord.recordName), reason=\(errorRecord.recordType)", + "Error: recordName=\(errorRecord.recordName), recordType=\(errorRecord.recordType)", subsystem: BushelLogger.cloudKit ) }Examples/Celestra/Sources/Celestra/Models/Feed.swift-29-33 (1)
29-33: Unnecessary Int64 → Int conversion.The properties are already
Int64, so converting toIntthen passing to.int64(Int)is redundant and could cause truncation on 32-bit platforms. Use the Int64 values directly.var fields: [String: FieldValue] = [ "feedURL": .string(feedURL), "title": .string(title), - "totalAttempts": .int64(Int(totalAttempts)), - "successfulAttempts": .int64(Int(successfulAttempts)), - "usageCount": .int64(Int(usageCount)), + "totalAttempts": .int64(totalAttempts), + "successfulAttempts": .int64(successfulAttempts), + "usageCount": .int64(usageCount), "isActive": .int64(isActive ? 1 : 0), - "failureCount": .int64(Int(failureCount)) + "failureCount": .int64(failureCount) ]Examples/Bushel/Sources/BushelImages/DataSources/AppleDB/AppleDBFetcher.swift-108-121 (1)
108-121: Fallback toDate()for missing release date may cause data quality issues.Using the current date as a fallback when
releaseDateis nil (line 111) could misrepresent historical entries. Consider usingniland makingreleaseDateoptional inRestoreImageRecord, or logging a warning when this fallback is used.Examples/Bushel/Sources/BushelImages/DataSources/XcodeReleasesFetcher.swift-91-94 (1)
91-94: Missing HTTP response status validation.The fetch doesn't validate the HTTP status code. A non-2xx response (e.g., 404, 500) would attempt to decode garbage data, resulting in a confusing decoding error.
func fetch() async throws -> [XcodeVersionRecord] { let url = URL(string: "https://xcodereleases.com/data.json")! - let (data, _) = try await URLSession.shared.data(from: url) + let (data, response) = try await URLSession.shared.data(from: url) + + guard let httpResponse = response as? HTTPURLResponse, + (200...299).contains(httpResponse.statusCode) else { + throw URLError(.badServerResponse) + } + let releases = try JSONDecoder().decode([XcodeRelease].self, from: data)Examples/Celestra/Sources/Celestra/Services/CloudKitService+Celestra.swift-228-237 (1)
228-237: Missing optimistic locking for article updates.Unlike
updateFeedwhich passesfeed.recordChangeTag, this batch update hardcodesrecordChangeTag: nil, skipping conflict detection. This could cause silent data loss if concurrent updates occur.return RecordOperation.update( recordType: "Article", recordName: recordName, fields: article.toFieldsDict(), - recordChangeTag: nil + recordChangeTag: article.recordChangeTag )Examples/Bushel/Sources/BushelImages/Commands/SyncCommand.swift-194-201 (1)
194-201: Incorrect terminology in error message.Line 197 mentions "API token" but the command uses Server-to-Server Keys. This could confuse users troubleshooting authentication issues.
private func printError(_ error: Error) { print("\n❌ Sync failed: \(error.localizedDescription)") print("\n💡 Troubleshooting:") - print(" • Verify your API token is valid") + print(" • Verify your Server-to-Server Key credentials are valid") print(" • Check your internet connection") print(" • Ensure the CloudKit container exists") print(" • Verify external data sources are accessible") }
🧹 Nitpick comments (60)
Examples/Bushel/.gitignore (1)
1-16: Consider expanding certificate and environment variable patterns for defensive completeness.While the root
.gitignorewill be applied to this subdirectory (via git's cascading behavior), explicitly listing all sensitive patterns in the Bushel-specific.gitignoreimproves clarity and reduces the risk of accidental commits.Given that Bushel implements server-to-server auth with key rotation, consider expanding the certificate/key patterns to match the root
.gitignore:# CloudKit Server-to-Server Private Keys *.pem +*.p8 +*.key +*.cer +*.crt +*.der +*.p12 +*.pfxSimilarly, to catch environment file variants:
# Environment variables .env +.env.development.local +.env.test.local +.env.production.local +.env.localExamples/Celestra/AI_SCHEMA_WORKFLOW.md (2)
30-42: Add language specifiers to all code blocks for proper syntax highlighting.The document contains numerous code blocks missing language identifiers (MD040). Adding these improves rendering on GitHub and web platforms. Use:
ckdbfor CloudKit schema definitionsswiftfor Swift code examplesbashfor shell commandsdifffor schema modification examplestextor no specifier for command output/errorsThis is especially important for the Swift model examples (lines 805–877, 882–942, 946–973) and schema definitions throughout, as syntax highlighting improves readability.
Example fixes:
-``` +```ckdb DEFINE SCHEMA -```-``` +```swift import Foundation import CloudKit -```-``` +```bash cktool validate-schema schema.ckdb -```Also applies to: 50-64, 232-260, 327-395, 403-441, 450-460, 474-485, 504-515, 524-530, 542-554, 561-569, 575-583, 590-602, 611-613, 621-625, 637-641, 648-653, 656-661, 663-668, 681-689, 708-714, 716-722, 724-730, 733-738, 741-746, 757-801, 805-877, 882-942, 946-973, 981-993, 997-1007, 1010-1013, 1016-1027, 1030-1040, 1044-1058
637-637: Format bare URL as a markdown link.Line 637 contains a bare URL that should be wrapped in markdown link syntax for consistency and proper rendering.
-1. Go to CloudKit Dashboard (https://icloud.developer.apple.com/dashboard) +1. Go to [CloudKit Dashboard](https://icloud.developer.apple.com/dashboard)Examples/Celestra/README.md (3)
43-87: Add blank lines around tables per Markdown best practices (MD058).Add a blank line before line 56 (Feed Record Type) and after line 65, and before line 68 (Article Record Type) and after line 79 to comply with markdown linting standards.
Apply this diff to add blank lines around the tables:
#### Feed Record Type + | Field Name | Field Type | Indexed | |------------|------------|---------| | feedURL | String | Yes (Queryable, Sortable) | @@ -63,6 +64,7 @@ In CloudKit Dashboard, create these record types in the **Public Database**: | lastAttempted | Date/Time | Yes (Queryable, Sortable) | | isActive | Int64 | Yes (Queryable) | + #### Article Record Type + | Field Name | Field Type | Indexed | |------------|------------|---------| | feedRecordName | String | Yes (Queryable, Sortable) | @@ -78,6 +80,7 @@ In CloudKit Dashboard, create these record types in the **Public Database**: | fetchedAt | Date/Time | Yes (Queryable, Sortable) | | expiresAt | Date/Time | Yes (Queryable, Sortable) | + #### 3. Generate Server-to-Server Key
123-205: Specify language for fenced code blocks (MD040).Fenced code blocks at lines 140 and 173 (example CLI output) should have a language specified. Use
textorplaintextfor output examples.Apply this diff to add language specification:
Example output: -``` +```text 🌐 Fetching RSS feed: https://example.com/feed.xml ✅ Found feed: Example Blog Articles: 25 @@ -170,7 +170,7 @@ swift run celestra update \Example output:
-+text
🔄 Starting feed update...
Filter: last attempted before 2025-01-01T00:00:00Z
Filter: minimum popularity 5--- `302-317`: **Specify language for architecture diagram code block (MD040).** The fenced code block starting at line 304 (architecture tree) should have a language specified, such as `text` or `plaintext`. Apply this diff: ```diff ## Architecture -``` +```text Celestra/ ├── Models/Examples/Celestra/Package.swift (1)
63-76:unsafeFlagsprevents this package from being used as a dependency.The use of
.unsafeFlags()means this package cannot be depended upon by other Swift packages (SPM restriction). This is acceptable since Celestra is an executable example project, not a reusable library.A few observations:
-enable-testing(line 71) is typically used for test targets, not executables- The combination of
-warn-concurrencyand-strict-concurrency=completemay be redundantConsider removing
-enable-testingif not needed:.unsafeFlags([ // Enable concurrency warnings "-warn-concurrency", // Enable actor data race checks "-enable-actor-data-race-checks", // Complete strict concurrency checking "-strict-concurrency=complete", - // Enable testing support - "-enable-testing", // Warn about functions with >100 lines "-Xfrontend", "-warn-long-function-bodies=100", // Warn about slow type checking expressions "-Xfrontend", "-warn-long-expression-type-checking=100" ])Examples/Celestra/Sources/Celestra/Services/CelestraError.swift (4)
5-18: Tighten associated error types and consider Sendable conformance
rssFetchFailed(URL, underlying: Error)andbatchOperationFailed([Error])both erase error types, which makes it harder to reason about failures and prevents straightforwardSendableconformance forCelestraError. If you plan to pass this error across concurrency boundaries, consider tightening the associated types and addingSendable:-enum CelestraError: LocalizedError { +enum CelestraError: LocalizedError, Sendable { @@ - case rssFetchFailed(URL, underlying: Error) + case rssFetchFailed(URL, underlying: any Error & Sendable) @@ - case batchOperationFailed([Error]) + case batchOperationFailed([CelestraError])You can always introduce a small adapter that maps external
Errorvalues intoCelestraErrorbefore constructing.batchOperationFailed. As per coding guidelines, this keeps errors typed, LocalizedError‑conforming, and concurrency‑safe.
32-43: RefineisRetriablesemantics for RSS and batch failures
isRetriablecurrently treats.rssFetchFailedas always retriable and.batchOperationFailedas never retriable, regardless of underlying causes. You might get more accurate behavior by inspecting the underlying error(s) (e.g., retry only for network/timeouts or 5xx/429, and aggregate retriable flags for batch failures):- case .rssFetchFailed, .networkUnavailable: - return true - case .quotaExceeded, .invalidFeedData, .batchOperationFailed, - .permissionDenied, .recordNotFound: - return false + case .rssFetchFailed(_, let underlying): + // e.g. treat only network / 5xx / 429 as retriable if the underlying error exposes that + return (underlying as? CelestraError)?.isRetriable ?? true + case .networkUnavailable: + return true + case .batchOperationFailed(let errors): + // e.g. retry if any or all underlying errors are retriable + return errors.compactMap { $0 as? CelestraError }.allSatisfy(\.isRetriable) + case .quotaExceeded, .invalidFeedData, + .permissionDenied, .recordNotFound: + return falseThis keeps retry logic closer to actual failure modes and plays better with typed errors. Based on learnings, this aligns with having robust, typed error handling.
47-83: Good LocalizedError implementation; consider richer debug context for RSS failures
errorDescription/recoverySuggestionare well-structured and user-friendly. For debugging RSS issues, you may want to capture more request/response context in.rssFetchFailed(e.g., HTTP status code, response snippet) and surface it either in the description or via additional properties used for logging:- case rssFetchFailed(URL, underlying: Error) + case rssFetchFailed(URL, statusCode: Int?, underlying: any Error & Sendable) @@ - case .rssFetchFailed(let url, let error): - return "Failed to fetch RSS feed from \(url.absoluteString): \(error.localizedDescription)" + case .rssFetchFailed(let url, let statusCode, let error): + let statusPart = statusCode.map { " (HTTP \($0))" } ?? "" + return "Failed to fetch RSS feed from \(url.absoluteString)\(statusPart): \(error.localizedDescription)"This matches the guidance to include request/response details in error types for easier diagnostics, while keeping user-facing strings concise.
87-106: Future‑proof CloudKit error classification
isCloudKitErrorRetriablecovers the knownCloudKitErrorcases, but any future cases added toCloudKitErrorwill require this switch to be updated. IfCloudKitErroris an enum that may grow, consider adding a defensive default (or@unknown defaultif applicable) for safer behavior:- private func isCloudKitErrorRetriable(_ error: CloudKitError) -> Bool { - switch error { + private func isCloudKitErrorRetriable(_ error: CloudKitError) -> Bool { + switch error { @@ - case .decodingError: - // Decoding errors are not retriable (data format issue) - return false - } + case .decodingError: + // Decoding errors are not retriable (data format issue) + return false + @unknown default: + // Be conservative for unknown cases; consider logging the raw error + return false + } }This avoids accidental “missing case” issues if
CloudKitErrorevolves independently.Examples/Celestra/Scripts/setup-cloudkit-schema.sh (3)
20-20: Simplify cktool availability check for clarity.The double-check for xcrun availability is unnecessary. Since
set -eo pipefailis already set, the script will exit if any command in a pipeline fails.-if ! command -v xcrun &> /dev/null || ! xcrun cktool --version &> /dev/null; then +if ! xcrun cktool --version &> /dev/null 2>&1; thenThis is simpler and achieves the same result: if xcrun is not in PATH or cktool is unavailable, the command fails.
59-74: Validate Team ID length as prompted.The prompt at line 67 indicates that the Team ID should be "10-character," but the validation at line 71 only checks if it is non-empty, not its length. This creates a mismatch between user expectations and validation.
Apply this diff to validate the Team ID length:
if [ -z "$CLOUDKIT_TEAM_ID" ]; then echo -e "${YELLOW}CLOUDKIT_TEAM_ID not set.${NC}" read -p "Enter your Apple Developer Team ID (10-character ID): " CLOUDKIT_TEAM_ID fi # Validate required parameters -if [ -z "$CLOUDKIT_CONTAINER_ID" ] || [ -z "$CLOUDKIT_TEAM_ID" ]; then +if [ -z "$CLOUDKIT_CONTAINER_ID" ] || [ -z "$CLOUDKIT_TEAM_ID" ] || [ ${#CLOUDKIT_TEAM_ID} -ne 10 ]; then echo -e "${RED}ERROR: Container ID and Team ID are required.${NC}" + [ ${#CLOUDKIT_TEAM_ID} -ne 10 ] && echo "Team ID must be exactly 10 characters." exit 1 fi
81-94: Quote variables in echo statements for robustness.Unquoted variables in echo statements at lines 81–83 could cause issues if values contain spaces or special characters. Similarly, SCHEMA_FILE should be quoted consistently.
Apply this diff to add proper quoting:
echo "" echo "Configuration:" -echo " Container ID: $CLOUDKIT_CONTAINER_ID" -echo " Team ID: $CLOUDKIT_TEAM_ID" -echo " Environment: $ENVIRONMENT" +echo " Container ID: ${CLOUDKIT_CONTAINER_ID}" +echo " Team ID: ${CLOUDKIT_TEAM_ID}" +echo " Environment: ${ENVIRONMENT}" echo "" # Check if schema file exists -SCHEMA_FILE="$(dirname "$0")/../schema.ckdb" -if [ ! -f "$SCHEMA_FILE" ]; then - echo -e "${RED}ERROR: Schema file not found at $SCHEMA_FILE${NC}" +SCHEMA_FILE="$(dirname "$0")/../schema.ckdb" +if [ ! -f "${SCHEMA_FILE}" ]; then + echo -e "${RED}ERROR: Schema file not found at ${SCHEMA_FILE}${NC}"Examples/Bushel/Sources/BushelImages/CloudKit/BushelCloudKitError.swift (1)
4-7: AddSendableconformance for Swift 6 concurrency safety.The error enum should conform to
Sendableper coding guidelines. However, theErrorassociated value inprivateKeyFileReadFailedis notSendable-constrained, which prevents automatic conformance.Apply this diff:
/// Errors that can occur during BushelCloudKitService operations -enum BushelCloudKitError: LocalizedError { +enum BushelCloudKitError: LocalizedError, Sendable { case privateKeyFileNotFound(path: String) - case privateKeyFileReadFailed(path: String, error: Error) + case privateKeyFileReadFailed(path: String, error: any Error & Sendable) case invalidMetadataRecord(recordName: String)If the underlying error cannot always be
Sendable, consider storing the error'slocalizedDescriptionstring instead:- case privateKeyFileReadFailed(path: String, error: Error) + case privateKeyFileReadFailed(path: String, errorDescription: String)As per coding guidelines, all types should be Sendable for Swift Concurrency safety.
Examples/Bushel/XCODE_SCHEME_SETUP.md (2)
48-61: Add language identifiers to fenced code blocks.The linter flags missing language specifiers on these code blocks. Since these are CLI argument examples, use
textorshellas the language identifier.For sync command: -``` +```text sync --container-id $(CLOUDKIT_CONTAINER_ID) --api-token $(CLOUDKIT_API_TOKEN)For export command:
-+text
export --container-id$(CLOUDKIT_CONTAINER_ID) --api-token $ (CLOUDKIT_API_TOKEN) --output ./export.jsonFor help: -``` +```text --help--- `253-256`: **Add language identifier to environment variable example.** ```diff Add logging middleware to MistKit (already configured) by setting environment variable: -``` +```bash MISTKIT_DEBUG_LOGGING=1</blockquote></details> <details> <summary>Examples/Celestra/CLOUDKIT_SCHEMA_SETUP.md (2)</summary><blockquote> `157-184`: **Add language identifier to schema code block.** ```diff -``` +```sql RECORD TYPE Feed ( "feedURL" STRING QUERYABLE SORTABLE,Using
sqlortextprovides syntax highlighting for the schema definition language.
403-406: Convert bare URL to Markdown link.For CloudKit schema issues: -- Check Apple Developer Forums: https://developer.apple.com/forums/tags/cloudkit +- Check [Apple Developer Forums](https://developer.apple.com/forums/tags/cloudkit) - Review CloudKit Dashboard logs.claude/docs/data-sources-api-research.md (2)
143-146: Consider safer error handling in documentation examples.The force-unwrap operator in the example code could mislead developers. Even in documentation, demonstrating safer patterns would be beneficial.
Apply this diff to use optional binding:
var toDate: Date { let components = DateComponents(year: year, month: month, day: day) - return Calendar.current.date(from: components)! + return Calendar.current.date(from: components) ?? Date() }Similar consideration for lines 199-201 in the
sdkVersionsJSONproperty.
191-201: Consider safer encoding in documentation examples.The force-try and force-unwrap operators could be replaced with safer alternatives to demonstrate better practices.
Apply this diff:
var sdkVersionsJSON: String { let dict: [String: String] = [ "macOS": sdks.macOS.first?.number ?? "", "iOS": sdks.iOS.first?.number ?? "", "watchOS": sdks.watchOS.first?.number ?? "", "tvOS": sdks.tvOS.first?.number ?? "", "visionOS": sdks.visionOS.first?.number ?? "" ] - let data = try! JSONEncoder().encode(dict) - return String(data: data, encoding: .utf8)! + guard let data = try? JSONEncoder().encode(dict), + let json = String(data: data, encoding: .utf8) else { + return "{}" + } + return json }Examples/Celestra/Sources/Celestra/Services/RSSFetcherService.swift (1)
162-181: Error handling looks good; consider implementing the TODO.The error handling properly distinguishes between decoding errors and general fetch failures, logging to the appropriate subsystem in each case.
The
parseUpdateIntervalmethod currently returnsnilwith a TODO comment. This means feed update preferences (RSS<ttl>and Syndication module tags) are not respected. Do you want me to help implement RSS update interval parsing, or would you like to track this as a separate issue?Examples/Bushel/Sources/BushelImages/Logger.swift (1)
1-73: Verify concurrency safety of global mutable state.The
nonisolated(unsafe) static var isVerbosepattern (line 30) is concerning despite the explanatory comment. While this pattern can work for CLI tools where the flag is set once during argument parsing before any concurrent operations, it's fragile and makes assumptions about initialization order.Recommendations:
- Safer alternative: Consider using an actor or passing verbose mode through a configuration context:
actor BushelLogger { static let shared = BushelLogger() private var isVerbose = false func setVerbose(_ value: Bool) { isVerbose = value } func shouldLogVerbose() -> Bool { isVerbose } }
Document initialization contract: If keeping the current pattern, add explicit documentation stating that
isVerboseMUST be set before any concurrent operations begin, preferably inmain()before any async contexts are created.Minor inconsistency: The code imports
Logging(swift-log) but the README documentation mentionsos.Logger. Consider aligning these for consistency.As per coding guidelines: "Use Actors for thread-safe state management"
Verify that
isVerboseis only set during synchronous CLI argument parsing inmain()before anyasyncoperations begin. If there's any possibility of concurrent writes or late initialization, refactor to use an actor or pass verbose mode through method parameters.CLAUDE.md (1)
233-234: Consider formatting bare URL as markdown link.The static analysis correctly flags the bare URL on line 233. While the content is clear, formatting it as a proper markdown link would be more consistent:
-- type order is based on the default in swiftlint: https://realm.github.io/SwiftLint/type_contents_order.html +- type order is based on the default in [swiftlint](https://realm.github.io/SwiftLint/type_contents_order.html)Examples/Bushel/Sources/BushelImages/DataSources/AppleDB/GitHubCommitter.swift (1)
4-6: Consider using Date type with custom decoding.The
dateproperty is typed asStringdespite representing an ISO 8601 timestamp. While this works for raw API responses, usingDatewith a customDecodableimplementation would eliminate repetitive parsing and enable direct date operations.If you prefer to keep the raw String for API transparency, this is acceptable as-is.
Alternatively, apply this pattern:
struct GitHubCommitter: Codable { let date: Date enum CodingKeys: String, CodingKey { case date } init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) let dateString = try container.decode(String.self, forKey: .date) let formatter = ISO8601DateFormatter() guard let parsedDate = formatter.date(from: dateString) else { throw DecodingError.dataCorruptedError( forKey: .date, in: container, debugDescription: "Date string does not match ISO 8601 format" ) } self.date = parsedDate } }Examples/Celestra/Sources/Celestra/Services/RateLimiter.swift (1)
56-67: Consider propagating Task.sleep errors.Similar to the issue in
waitIfNeeded, usingtry?at line 64 silences errors including task cancellation. Consider allowing cancellation to propagate naturally:if remainingDelay > 0 { let nanoseconds = UInt64(remainingDelay * 1_000_000_000) - try? await Task.sleep(nanoseconds: nanoseconds) + try await Task.sleep(nanoseconds: nanoseconds) }If ignoring cancellation is intentional, add a comment explaining why.
.claude/docs/SUMMARY.md (1)
1-424: Consider addressing markdown formatting issues.Static analysis flagged several formatting inconsistencies:
- Hard tabs used instead of spaces (lines 34-96)
- Bare URL on line 11 should be in angle brackets or as a markdown link
- Heading level jumps from h1 to h3 on line 17 (should increment by one level)
These are stylistic issues that don't affect readability significantly but would improve consistency with markdown conventions.
.claude/docs/sosumi-cloudkit-schema-source.md (1)
25-64: Consider adding language specifiers to code blocks.Several code blocks lack language specifiers, which would enable syntax highlighting. Consider adding appropriate identifiers (e.g.,
ebnf,cloudkit, ortext) to improve readability.Also applies to: 93-99, 118-124, 128-133, 141-145, 151-179
.claude/docs/cktooljs.md (1)
3-3: Consider wrapping bare URL in angle brackets.The URL on line 3 should be wrapped in angle brackets or converted to a markdown link for proper formatting.
Apply this diff:
-**Source**: https://developer.apple.com/documentation/cktooljs/ +**Source**: <https://developer.apple.com/documentation/cktooljs/>Examples/Bushel/Sources/BushelImages/DataSources/TheAppleWiki/IPSWParser.swift (3)
172-176: DateFormatter created on each invocation is inefficient.
DateFormatteris expensive to initialize. Creating a new instance for each date parsed during row iteration impacts performance.Consider using a static formatter:
+ private static let dateFormatter: DateFormatter = { + let formatter = DateFormatter() + formatter.dateFormat = "yyyy-MM-dd" + return formatter + }() + private func parseDate(_ str: String) -> Date? { - let formatter = DateFormatter() - formatter.dateFormat = "yyyy-MM-dd" - return formatter.date(from: str) + return Self.dateFormatter.date(from: str) }
48-58: Consider parallel page fetching with TaskGroup.Pages are fetched sequentially, but each fetch is independent. Using structured concurrency with
TaskGroupwould improve throughput when fetching multiple firmware pages.- // Fetch versions from each page - for pageTitle in versionPages { - let pageURL = try buildPageURL(for: pageTitle) - do { - let versions = try await parseIPSWPage(url: pageURL, deviceFilter: deviceFilter) - allVersions.append(contentsOf: versions) - } catch { - // Continue on page parse errors - some pages may be empty or malformed - continue - } - } + // Fetch versions from each page in parallel + try await withThrowingTaskGroup(of: [IPSWVersion].self) { group in + for pageTitle in versionPages { + let pageURL = try buildPageURL(for: pageTitle) + group.addTask { + do { + return try await self.parseIPSWPage(url: pageURL, deviceFilter: deviceFilter) + } catch { + // Continue on page parse errors - some pages may be empty or malformed + return [] + } + } + } + for try await versions in group { + allVersions.append(contentsOf: versions) + } + }As per coding guidelines, use structured concurrency with TaskGroup for parallel operations.
99-110: Deduplicate version pages to avoid redundant requests.The regex may match multiple links to the same firmware page (e.g., several references to "Firmware/Mac/12.x"), resulting in duplicate fetches.
- return versionPages + return Array(Set(versionPages)).sorted()Or use
removeDuplicates()if order matters and duplicates are adjacent.Examples/Bushel/Sources/BushelImages/DataSources/TheAppleWiki/TheAppleWikiFetcher.swift (1)
10-10: Force-unwrap is safe for hardcoded URL but consider failable initialization.The URL string is valid and hardcoded, making the force-unwrap safe in practice. However, for consistency with error handling patterns elsewhere, a guard-let with an early throw would be more defensive.
- let apiURL = URL(string: "https://theapplewiki.com/api.php?action=parse&page=Firmware/Mac&format=json")! + guard let apiURL = URL(string: "https://theapplewiki.com/api.php?action=parse&page=Firmware/Mac&format=json") else { + throw TheAppleWikiError.invalidURL("Firmware/Mac") + }Examples/Bushel/Sources/BushelImages/CloudKit/RecordManaging+Query.swift (1)
11-17: Server-side filtering is available in MistKit but not exposed through the genericquery()extension.The current implementation fetches all
DataSourceMetadatarecords and filters in-memory by record name. MistKit supports server-side filtering viaQueryFilter.equals()inCloudKitService.queryRecords(), but theRecordManagingprotocol's genericquery()method doesn't expose the filters parameter. For metadata lookups (typically small datasets), this approach is acceptable, but consider usingCloudKitServicedirectly with server-side filtering if the metadata table grows large or for consistency with filtering patterns elsewhere in the codebase.Examples/Celestra/Sources/Celestra/Models/BatchOperationResult.swift (1)
5-60: Batch aggregation logic is solid; consider future-proofing for concurrencyThe counters, success rate, and append helpers are all correct and convenient for summarizing batch CloudKit operations. If you ever need to pass
BatchOperationResultacross actors/tasks, consider:
- Introducing a typed error wrapper (e.g.,
enum BatchError: Error, Sendable) instead of storing bareError, and- Marking
BatchOperationResultasSendable.For now, if this stays confined to a single task/actor, the current design is fine.
Examples/Celestra/Sources/Celestra/Celestra.swift (1)
5-65: Env-driven CloudKit service setup looks good; consider minor ergonomics tweaksThe
Celestraentrypoint andCelestraConfig.createCloudKitService()read cleanly: env validation, key loading, and environment selection are straightforward and appropriate for a CLI. If you want to polish further, you could:
- Cache
ProcessInfo.processInfo.environmentinto a localenvconstant to avoid repeated lookups.- Optionally mirror Bushel’s pattern and allow overriding
CLOUDKIT_CONTAINER_IDvia a CLI option for easier local testing.These are purely ergonomics; current behavior is fine as-is.
Examples/Celestra/Sources/Celestra/Commands/AddFeedCommand.swift (1)
18-54: Async flow and CloudKit wiring are solid; tiny UX refinement possibleThe stepwise flow (URL validation → RSS fetch → CloudKit service creation →
Feedwrite) is clear and uses async/await correctly. One small tweak you might consider: whenresponse.feedDataisnil, theValidationError("Feed was not modified (unexpected)")message could be rephrased to something more user-facing like “Unable to read feed contents” and optionally include the HTTP status fromFetchResponseif you have it, to aid debugging.Behavior is otherwise in good shape.
.claude/docs/QUICK_REFERENCE.md (1)
5-324: Address markdownlint MD034/MD040 (bare URLs and fence languages)The content is solid; the remaining issues are just markdownlint noise:
- A few fenced code blocks (e.g. the base URL example and some other non-Swift snippets) don’t specify a language. Adding something like
text,http, orbashafter the opening ``` will clear MD040.- Bare URLs (e.g. CloudKit docs links) are flagged by MD034. If you care about a clean lint run, wrap them as
<https://…>or[label](https://…), or disable MD034 for this file.No behavior impact, purely documentation hygiene.
.claude/docs/cloudkit-public-database-architecture.md (1)
84-1981: Clean up fenced code languages and bare URLs for markdownlintThis doc is very thorough; the only actionable items are lint-related:
- Several ASCII-diagram/code fences (e.g. architecture diagrams and flow charts around the “Architecture Diagram”, “Data Flow Patterns”, and “Architecture Summary” sections) use bare ``` without a language. Adding
text(or another suitable label) will satisfy MD040 and make rendering clearer.- The external links at the end (CloudKit docs, MistKit GitHub, RSS spec) are bare URLs; wrapping them in
<…>or[label](…)will address MD034 if you want a clean lint run.No content changes needed; this is just markdown hygiene.
Examples/Bushel/Sources/BushelImages/DataSources/DataSourceFetcher.swift (1)
37-80: Shared fetch/decode helpers are good; consider DI and fewer round‑tripsThe protocol plus utilities form a nice common surface for the various fetchers. Two non-blocking improvements you might consider:
Network round‑trips (lines 49–56):
fetchDatadoes a HEAD (viaHTTPHeaderHelpers.fetchLastModified) and then a full GET. For some sources this is ideal, but for large or rate‑limited endpoints you may want an overload that:
- Either reads
Last-Modifiedfrom the GET response headers only, or- Accepts a strategy flag/closure so individual fetchers can opt out of the HEAD request.
Testability/decoding config (lines 66–80):
decodeJSONalways instantiates a freshJSONDecoderwith default strategies. If you later need custom date/key strategies or want easier testing, it’s worth:
- Allowing callers to pass an injected
JSONDecoder, or- Providing a shared, configurable decoder on
DataSourceUtilities.Current implementation is correct; these would just make the utilities more flexible and test‑friendly.
Examples/Bushel/Sources/BushelImages/Commands/StatusCommand.swift (1)
45-226: StatusCommand flow looks solid; consider reusing date/“now” for consistencyThe command logic (credential resolution, metadata query, optional error‑only filtering, and human‑readable timing) is well structured and should be easy to use.
Two small, non‑blocking tweaks you might consider:
Reuse a single
DateFormatter(lines 202–207): Constructing a newDateFormatterper call is fine for a CLI, but you could make it a static property or lazily cached to avoid repeated setup if this grows or is reused elsewhere.Single “now” per run:
printMetadatacaptureslet now = Date()per record. If you ever care about absolute consistency in the “X minutes ago” readouts, you could computenowonce inrun()and pass it in, instead of re-evaluating inside each call.Neither change is required; the current implementation is functionally correct and easy to follow.
Examples/Bushel/Sources/BushelImages/DataSources/MrMacintoshFetcher.swift (1)
10-175: HTML parsing and date handling look robust; only minor reuse tweaks possibleThe fetcher does a good job of:
- Validating the source URL,
- Extracting the page “UPDATED:” date,
- Parsing table rows into
RestoreImageRecordwith conservative guards, and- Handling multiple date formats, including inferring the year when it’s omitted.
A couple of optional improvements you might consider over time:
- Reuse
DataSourceUtilities.fetchData(from:)to centralize HTTP access and optionally pick upLast-Modifiedinformation for consistency with other data sources.- Hoist the
DateFormatterinstances out of the tight loops (e.g. static or cached properties) if this ever becomes a hot path; right now it’s fine for typical table sizes.Functionally, this fetcher looks solid.
Examples/Bushel/Sources/BushelImages/Models/RestoreImageRecord.swift (1)
82-109: Consider adding logging for deserialization failures.The
from(recordInfo:)method silently returnsnilwhen required fields are missing, which makes debugging difficult. Consider logging which field was missing before returningnil.static func from(recordInfo: RecordInfo) -> Self? { guard let version = recordInfo.fields["version"]?.stringValue, let buildNumber = recordInfo.fields["buildNumber"]?.stringValue, let releaseDate = recordInfo.fields["releaseDate"]?.dateValue, let downloadURL = recordInfo.fields["downloadURL"]?.stringValue, let fileSize = recordInfo.fields["fileSize"]?.intValue, let sha256Hash = recordInfo.fields["sha256Hash"]?.stringValue, let sha1Hash = recordInfo.fields["sha1Hash"]?.stringValue, let source = recordInfo.fields["source"]?.stringValue else { + #if DEBUG + print("RestoreImageRecord: Missing required field(s) in record \(recordInfo.recordName)") + #endif return nil }Examples/Bushel/Sources/BushelImages/CloudKit/BushelCloudKitService.swift (1)
70-72: Hardcoded query limit may truncate results.The
limit: 200is hardcoded, which could silently truncate results if more records exist. Consider either documenting this limitation, adding pagination support, or making the limit configurable./// Query all records of a given type - func queryRecords(recordType: String) async throws -> [RecordInfo] { - try await service.queryRecords(recordType: recordType, limit: 200) + func queryRecords(recordType: String, limit: Int = 200) async throws -> [RecordInfo] { + try await service.queryRecords(recordType: recordType, limit: limit) }Examples/Bushel/Sources/BushelImages/DataSources/DataSourcePipeline.swift (2)
50-71: Inconsistent error handling: errors printed then re-thrown.The pattern of catching, printing, then re-throwing adds overhead without benefit. Either let errors propagate naturally (caller handles logging) or handle them here without re-throwing.
Consider simplifying:
- do { - restoreImages = try await fetchRestoreImages(options: options) - } catch { - print("⚠️ Restore images fetch failed: \(error)") - throw error - } + restoreImages = try await fetchRestoreImages(options: options)If logging is needed, use
deferor a wrapper that logs without catching/re-throwing.
253-311: Intentional but undocumented inconsistency in error handling across sources.AppleDB errors are caught and swallowed (line 269:
// Don't throw - continue with other sources), but Mr. Macintosh and TheAppleWiki errors are thrown. If this is intentional (AppleDB is supplementary), consider documenting this in theOptionsstruct or adding a comment explaining the rationale for each source's criticality.Examples/Celestra/Sources/Celestra/Models/Feed.swift (2)
57-143: Consider using FieldValue convenience accessors for consistency.The
init(from record:)uses manual pattern matching (e.g.,if case .string(let value) = ...), whileRestoreImageRecorduses convenience accessors like.stringValue,.int64Value,.dateValue. Using the accessors would reduce boilerplate and align with the patterns in other models in this PR.Example transformation:
- if case .string(let value) = record.fields["feedURL"] { - self.feedURL = value - } else { - self.feedURL = "" - } + self.feedURL = record.fields["feedURL"]?.stringValue ?? ""
62-72: Empty string defaults for required fields may hide data corruption.Defaulting
feedURLandtitleto empty strings when fields are missing could mask data issues. Consider making these failable or throwing an error for missing required fields, similar toRestoreImageRecord.from(recordInfo:).Examples/Bushel/Sources/BushelImages/DataSources/SwiftVersionFetcher.swift (2)
40-43: Consider using structured logging instead of print.The warning on date parse failure uses
print(). For consistency with the rest of the codebase (which usesBushelLogger), consider using structured logging.guard let date = dateFormatter.date(from: dateStr) else { - print("Warning: Could not parse date: \(dateStr)") + BushelLogger.verbose("Could not parse date: \(dateStr)", subsystem: BushelLogger.dataSources) continue }
66-68: Consider extending error enum.The
FetchErrorenum currently only hasinvalidEncoding. Consider adding cases for network errors or HTML parsing failures for better error granularity, though this is optional sinceURLSessionandSwiftSoupthrow their own errors.Examples/Bushel/Sources/BushelImages/Configuration/FetchConfiguration.swift (1)
98-109: Environment loading only discovers sources defined indefaultIntervals.The loop on line 102 only iterates over keys in
defaultIntervals, so any custom source names set via environment variables (e.g.,BUSHEL_FETCH_INTERVAL_CUSTOM_SOURCE) won't be discovered. This may be intentional, but if you want to support arbitrary sources via environment variables, consider a different approach.Examples/Bushel/IMPLEMENTATION_NOTES.md (1)
429-430: Consider using an actual date instead of "Current session".The "Last Updated: Current session" is ambiguous. Consider using a specific date for better traceability.
-**Last Updated**: Current session +**Last Updated**: November 2025Examples/Bushel/Sources/BushelImages/DataSources/AppleDB/AppleDBFetcher.swift (2)
103-106: Link selection may not prioritize preferred links correctly.The
first(where:)returns the first link that is either preferred or active. If a non-preferred active link appears before a preferred link in the array, it will be selected instead of the preferred one.- guard let link = ipswSource.links?.first(where: { $0.preferred == true || $0.active == true }) else { + // Prefer preferred links over active-only links + guard let link = ipswSource.links?.first(where: { $0.preferred == true }) + ?? ipswSource.links?.first(where: { $0.active == true }) else {
127-143:FetchError.invalidURLcase is defined but never used.The
invalidURLerror case is never thrown since URLs are constructed from hardcoded strings. Consider removing it to avoid dead code, or use it if URL validation becomes dynamic.enum FetchError: LocalizedError { - case invalidURL case noDataFound case decodingFailed(Error) var errorDescription: String? { switch self { - case .invalidURL: - return "Invalid AppleDB URL" case .noDataFound:Examples/Bushel/Sources/BushelImages/Models/SwiftVersionRecord.swift (1)
78-83: DateFormatter is recreated on each call.
DateFormatterinstantiation is expensive. IfformatDateis called frequently (e.g., when listing many records), consider caching the formatter as a static property.+ private static let dateFormatter: DateFormatter = { + let formatter = DateFormatter() + formatter.dateStyle = .medium + formatter.timeStyle = .none + return formatter + }() + private static func formatDate(_ date: Date) -> String { - let formatter = DateFormatter() - formatter.dateStyle = .medium - formatter.timeStyle = .none - return formatter.string(from: date) + return dateFormatter.string(from: date) }Examples/Bushel/Sources/BushelImages/Commands/ExportCommand.swift (2)
112-143: Consider extracting duplicate filter logic into a helper.The
noBetasfilter repeats nearly identical code forrestoreImages,xcodeVersions, andswiftVersions. A small helper would reduce duplication:+ private func filterPrerelease(_ records: [RecordInfo]) -> [RecordInfo] { + records.filter { record in + if case .int64(let isPrerelease) = record.fields["isPrerelease"] { + return isPrerelease == 0 + } + return true + } + } + // Filter out betas if noBetas { - restoreImages = restoreImages.filter { record in - if case .int64(let isPrerelease) = record.fields["isPrerelease"] { - return isPrerelease == 0 - } - return true - } - - xcodeVersions = xcodeVersions.filter { record in - if case .int64(let isPrerelease) = record.fields["isPrerelease"] { - return isPrerelease == 0 - } - return true - } - - swiftVersions = swiftVersions.filter { record in - if case .int64(let isPrerelease) = record.fields["isPrerelease"] { - return isPrerelease == 0 - } - return true - } + restoreImages = filterPrerelease(restoreImages) + xcodeVersions = filterPrerelease(xcodeVersions) + swiftVersions = filterPrerelease(swiftVersions) }
207-209: Consider conformingExportErrortoLocalizedError.As per coding guidelines, typed errors should conform to
LocalizedErrorto provide user-friendly descriptions.- enum ExportError: Error { + enum ExportError: LocalizedError { case encodingFailed + + var errorDescription: String? { + switch self { + case .encodingFailed: + return "Failed to encode export data to JSON" + } + } }Examples/Bushel/Sources/BushelImages/Models/DataSourceMetadata.swift (1)
116-121: Consider caching the DateFormatter for performance.
DateFormatteris expensive to create. In display-heavy scenarios, creating it per-call can impact performance. A static cached formatter or usingISO8601DateFormattercould help.+ private static let displayDateFormatter: DateFormatter = { + let formatter = DateFormatter() + formatter.dateStyle = .medium + formatter.timeStyle = .short + return formatter + }() + private static func formatDate(_ date: Date) -> String { - let formatter = DateFormatter() - formatter.dateStyle = .medium - formatter.timeStyle = .short - return formatter.string(from: date) + return displayDateFormatter.string(from: date) }Examples/Bushel/Sources/BushelImages/Models/XcodeVersionRecord.swift (1)
132-140: File size uses decimal (SI) units rather than binary.The implementation uses 1,000,000,000 for GB (SI/decimal) rather than 1,073,741,824 for GiB (binary). This is technically correct for network/download sizes (Apple uses decimal), but consider adding a brief comment for clarity if this is intentional.
private static func formatFileSize(_ bytes: Int) -> String { + // Using SI decimal units (GB = 10^9) as used by Apple for download sizes let gb = Double(bytes) / 1_000_000_000Examples/Bushel/Sources/BushelImages/Commands/SyncCommand.swift (1)
35-43: Mutually exclusive flags lack validation.
--restore-images-only,--xcode-only, and--swift-onlyare logically mutually exclusive, but if a user passes multiple, only the first matching condition is applied silently. Consider using ArgumentParser's@OptionGroupwith a mutual exclusion group, or validate and error if multiple are set.
| <!-- | ||
| Downloaded via https://llm.codes by @steipete on November 4, 2025 at 03:26 PM | ||
| Source URL: https://developer.apple.com/icloud/ck-tool/ | ||
| Total pages processed: 1 | ||
| URLs filtered: Yes | ||
| Content de-duplicated: Yes | ||
| Availability strings filtered: Yes | ||
| Code blocks only: No | ||
| --> | ||
|
|
||
| # https://developer.apple.com/icloud/ck-tool/ | ||
|
|
||
| View in English | ||
|
|
||
| # Using cktool | ||
|
|
||
| ### Before you begin | ||
|
|
||
| - You'll need to be a current member of the Apple Developer Program or the Apple Developer Enterprise Program in order to use `cktool`. | ||
| - Understand the concepts of Authentication, as described in Automating CloudKit Development. | ||
| - Generate a CloudKit Management Token from the CloudKit Console by choosing the Settings section for your user account. Save this token, as it will not be visible again. | ||
|
|
||
| ### Installing the application | ||
|
|
||
| By default, `cktool` is distributed with Xcode starting with Xcode 13, which is available on the Mac App Store. | ||
|
|
||
| ### General usage | ||
|
|
||
| `cktool` is stateless and passes all operations to the CloudKit Management API in single operations. A full list of supported operations is available from the `help` command or the `man` pages: | ||
|
|
||
| xcrun cktool --help | ||
| OVERVIEW: CloudKit Command Line Tool | ||
|
|
||
| -h, --help Show help information. | ||
|
|
||
| SUBCOMMANDS: ... | ||
|
|
||
| ### Authenticating `cktool` to CloudKit | ||
|
|
||
| `cktool` supports both management and user tokens, and will store them securely in Mac Keychain. You can add a token using the `save-token` command, which will launch CloudKit Console for copying of the appropriate token after prompting for information from the user: | ||
|
|
||
| xcrun cktool save-token \ | ||
| --type [management | user] | ||
|
|
||
| #### Issuing Schema Management commands | ||
|
|
||
| Schema Management commands require the Management Token to be provided. Once provided, `cktool` can perform the following commands: | ||
|
|
||
| **Reset development schema.** This allows you to revert the development database to the current production definition. | ||
|
|
||
| xcrun cktool reset-schema \ | ||
| --team-id [TEAM-ID] \ | ||
| --container-id [CONTAINER] | ||
|
|
||
| **Export schema file.** This allows you to save an existing CloudKit Database schema definition to a file, which can be kept alongside the related source code: | ||
|
|
||
| xcrun cktool export-schema \ | ||
| --team-id [TEAM-ID] \ | ||
| --container-id [CONTAINER] \ | ||
| --environment [development | production] \ | ||
| [--output-file schema.ckdb] | ||
|
|
||
| **Import schema file.** This applies a file-based schema definition against the development database for testing. | ||
|
|
||
| xcrun cktool import-schema \ | ||
| --team-id [TEAM-ID] \ | ||
| --container-id [CONTAINER] \ | ||
| --environment development \ | ||
| --file schema.ckdb | ||
|
|
||
| ### Issuing data commands | ||
|
|
||
| When a user token is available, `cktool` can access public and private databases on behalf of the user. This can be used for fetching data and inserting new records prior to integration tests. Note that due to the short lifespan of the user token, frequent interactive authentication may be required. | ||
|
|
||
| Querying records can be performed with the `query-records` command. Note that queries without filters require the `___recordID` to have a `Queryable` index applied, as do fields specified in the optional `--filters` argument: | ||
|
|
||
| xcrun cktool query-records \ | ||
| --team-id [TEAMID] \ | ||
| --container-id [CONTAINER] \ | ||
| --zone-name [ZONE_NAME] \ | ||
| --database-type [public | private] \ | ||
| --environment [development | production] \ | ||
| --record-type [RECORD_TYPE] \ | ||
| [--filters [FILTER_1] [FILTER_2]] | ||
|
|
||
| Where `FILTER_1` is in the form `"[FIELD_NAME] [OPERATOR] [VALUE]"`, for example `"lastname == Appleseed"`. | ||
|
|
||
| Inserting data is accomplished with the `create-record` command: | ||
|
|
||
| xcrun cktool create-record \ | ||
| --team-id [TEAM_ID] \ | ||
| --container-id [CONTAINER] \ | ||
| --zone-name [ZONE_NAME] \ | ||
| --database-type [public | private] \ | ||
| --environment [development | production] \ | ||
| --record-type [RECORD_TYPE] \ | ||
| [--fields-json [FIELDS_JSON]] | ||
|
|
||
| Where `FIELDS_JSON` is a JSON representation of the fields to set in the record. For example: | ||
|
|
||
| '{ | ||
| "firstName": { | ||
| "type": "stringType", | ||
| "value": "Johnny" | ||
| }, | ||
| "lastName": { | ||
| "type": "stringType", | ||
| "value": "Appleseed" | ||
| } | ||
| }' | ||
|
|
||
| --- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verify licensing for verbatim Apple documentation
The header notes this file was downloaded from Apple’s cktool documentation, and the body appears to reproduce that content almost verbatim. That raises potential licensing/compliance questions about redistributing Apple’s docs in-repo.
Please confirm that this level of reuse is allowed for your distribution context. If not, consider replacing this with:
- A concise, original summary in your own words, and
- A pointer to the official cktool documentation on developer.apple.com for authoritative details.
Also, any automated style suggestions (e.g., wording like “in order to”, “prior to”) should probably be ignored here to keep the text aligned with the upstream source if you choose to retain it.
🧰 Tools
🪛 LanguageTool
[style] ~19-~19: Consider a more concise word here.
Context: ... the Apple Developer Enterprise Program in order to use cktool. - Understand the concepts...
(IN_ORDER_TO_PREMIUM)
[style] ~73-~73: ‘new records’ might be wordy. Consider a shorter alternative.
Context: ...be used for fetching data and inserting new records prior to integration tests. Note that d...
(EN_WORDINESS_PREMIUM_NEW_RECORDS)
[style] ~73-~73: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...fetching data and inserting new records prior to integration tests. Note that due to the...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
| import MistKit | ||
|
|
||
| /// Helper utilities for converting between Swift types and CloudKit FieldValue types | ||
| enum CloudKitFieldMapping { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Add Sendable conformance for Swift Concurrency safety.
This enum should conform to Sendable since it only contains static methods and no mutable state.
As per coding guidelines, all types should be Sendable for Swift Concurrency safety.
Apply this diff:
-enum CloudKitFieldMapping {
+enum CloudKitFieldMapping: Sendable {
/// Convert a String to FieldValue
static func fieldValue(from string: String) -> FieldValue {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| enum CloudKitFieldMapping { | |
| enum CloudKitFieldMapping: Sendable { |
🤖 Prompt for AI Agents
In Examples/Bushel/Sources/BushelImages/CloudKit/CloudKitFieldMapping.swift
around line 5, the enum declaration should declare conformance to Sendable;
update the enum signature to add : Sendable. Ensure there are no non-Sendable
stored properties or mutable state in the type (it currently only has static
methods), and run the build to confirm the Sendable conformance requires no
additional changes.
| /// Convert a Bool to FieldValue (using INT64 representation: 0 = false, 1 = true) | ||
| static func fieldValue(from bool: Bool) -> FieldValue { | ||
| .from(bool) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Script executed:
#!/bin/bash
# Search for FieldValue type definition
fd -t f --name '*.swift' | xargs rg -l 'FieldValue' | head -20
# Also search specifically in the same file
rg -n 'enum FieldValue|struct FieldValue|class FieldValue' --type=swift -A 5Repository: brightdigit/MistKit
<shell_result>
Repository: brightdigit/MistKit
Exit code: 0
stdout:
Examples/Bushel/README.md
Examples/MistDemo/Sources/MistDemo/MistDemo.swift
Examples/MistDemo/Sources/MistDemo/Utilities/FieldValueFormatter.swift
Examples/Bushel/Sources/BushelImages/Models/RestoreImageRecord.swift
Examples/Bushel/Sources/BushelImages/Models/XcodeVersionRecord.swift
Examples/Bushel/Sources/BushelImages/Models/DataSourceMetadata.swift
Examples/Bushel/Sources/BushelImages/Models/SwiftVersionRecord.swift
Examples/Bushel/Sources/BushelImages/CloudKit/CloudKitFieldMapping.swift
Examples/Bushel/CLOUDKIT-SETUP.md
Examples/Celestra/IMPLEMENTATION_NOTES.md
Examples/Celestra/BUSHEL_PATTERNS.md
Examples/Celestra/Sources/Celestra/Services/CloudKitService+Celestra.swift
Examples/Celestra/Sources/Celestra/Models/Feed.swift
Examples/Celestra/Sources/Celestra/Models/Article.swift
openapi.yaml
openapi-generator-config.yaml
Sources/MistKit/PublicTypes/QueryFilter.swift
Sources/MistKit/FieldValue.swift
Sources/MistKit/RecordOperation.swift
Sources/MistKit/Helpers/FilterBuilder.swift
Tests/MistKitTests/Protocols/FieldValueConvenienceTests.swift:37:internal struct FieldValueConvenienceTests {
Tests/MistKitTests/Protocols/FieldValueConvenienceTests.swift-38- @Test("stringValue extracts String from .string case")
Tests/MistKitTests/Protocols/FieldValueConvenienceTests.swift-39- internal func stringValueExtraction() {
Tests/MistKitTests/Protocols/FieldValueConvenienceTests.swift-40- let value = FieldValue.string("test")
Tests/MistKitTests/Protocols/FieldValueConvenienceTests.swift-41- #expect(value.stringValue == "test")
Tests/MistKitTests/Protocols/FieldValueConvenienceTests.swift-42- }
--
Sources/MistKit/FieldValue.swift:33:public enum FieldValue: Codable, Equatable, Sendable {
Sources/MistKit/FieldValue.swift-34- /// Conversion factor from seconds to milliseconds for CloudKit timestamps
Sources/MistKit/FieldValue.swift-35- private static let millisecondsPerSecond: Double = 1_000
Sources/MistKit/FieldValue.swift-36- case string(String)
Sources/MistKit/FieldValue.swift-37- case int64(Int)
Sources/MistKit/FieldValue.swift-38- case double(Double)
--
Tests/MistKitTests/Core/FieldValue/FieldValueTests.swift:8:internal struct FieldValueTests {
Tests/MistKitTests/Core/FieldValue/FieldValueTests.swift-9- /// Tests FieldValue string type creation and equality
Tests/MistKitTests/Core/FieldValue/FieldValueTests.swift-10- @Test("FieldValue string type creation and equality")
Tests/MistKitTests/Core/FieldValue/FieldValueTests.swift-11- internal func fieldValueString() {
Tests/MistKitTests/Core/FieldValue/FieldValueTests.swift-12- let value = FieldValue.string("test")
Tests/MistKitTests/Core/FieldValue/FieldValueTests.swift-13- #expect(value == .string("test"))
--
Tests/MistKitTests/Core/FieldValue/FieldValueConversionTests.swift:7:internal struct FieldValueConversionTests {
Tests/MistKitTests/Core/FieldValue/FieldValueConversionTests.swift-8- // MARK: - Basic Type Conversions
Tests/MistKitTests/Core/FieldValue/FieldValueConversionTests.swift-9-
Tests/MistKitTests/Core/FieldValue/FieldValueConversionTests.swift-10- @Test("Convert string FieldValue to Components.FieldValue")
Tests/MistKitTests/Core/FieldValue/FieldValueConversionTests.swift-11- func convertString() {
Tests/MistKitTests/Core/FieldValue/FieldValueConversionTests.swift-12- guard #available(macOS 11.0, iOS 14.0, tvOS 14.0, watchOS 7.0, *) else {
--
Examples/MistDemo/Sources/MistDemo/Utilities/FieldValueFormatter.swift:12:struct FieldValueFormatter {
Examples/MistDemo/Sources/MistDemo/Utilities/FieldValueFormatter.swift-13-
Examples/MistDemo/Sources/MistDemo/Utilities/FieldValueFormatter.swift-14- /// Format FieldValue fields for display
Examples/MistDemo/Sources/MistDemo/Utilities/FieldValueFormatter.swift-15- static func formatFields(_ fields: [String: FieldValue]) -> String {
Examples/MistDemo/Sources/MistDemo/Utilities/FieldValueFormatter.swift-16- if fields.isEmpty {
Examples/MistDemo/Sources/MistDemo/Utilities/FieldValueFormatter.swift-17- return "{}"
stderr:
error: Found argument '--name' which wasn't expected, or isn't valid in this context
If you tried to supply '--name' as a value rather than a flag, use '-- --name'
Usage: fd --type <filetype> [pattern] [path]...
For more information try '--help'
</shell_result>
Script executed:
#!/bin/bash
# Search for the .from method implementation
rg -nP --type=swift 'static\s+func\s+from\s*\(' -A 3 -B 1Repository: brightdigit/MistKit
<shell_result>
Repository: brightdigit/MistKit
Exit code: 0
stdout:
Sources/MistKit/Protocols/CloudKitRecord.swift-52-///
Sources/MistKit/Protocols/CloudKitRecord.swift:53:/// static func from(recordInfo: RecordInfo) -> Self? {
Sources/MistKit/Protocols/CloudKitRecord.swift-54-/// guard let name = recordInfo.fields["name"]?.stringValue else {
Sources/MistKit/Protocols/CloudKitRecord.swift-55-/// return nil
Sources/MistKit/Protocols/CloudKitRecord.swift-56-/// }
--
Sources/MistKit/Protocols/CloudKitRecord.swift-109- /// - Returns: A model instance, or `nil` if parsing fails
Sources/MistKit/Protocols/CloudKitRecord.swift:110: static func from(recordInfo: RecordInfo) -> Self?
Sources/MistKit/Protocols/CloudKitRecord.swift-111-
Sources/MistKit/Protocols/CloudKitRecord.swift-112- /// Format a CloudKit record for display output
Sources/MistKit/Protocols/CloudKitRecord.swift-113- ///
--
Tests/MistKitTests/Protocols/CloudKitRecordTests.swift-66-
Tests/MistKitTests/Protocols/CloudKitRecordTests.swift:67: static func from(recordInfo: RecordInfo) -> TestRecord? {
Tests/MistKitTests/Protocols/CloudKitRecordTests.swift-68- guard
Tests/MistKitTests/Protocols/CloudKitRecordTests.swift-69- let name = recordInfo.fields["name"]?.stringValue,
Tests/MistKitTests/Protocols/CloudKitRecordTests.swift-70- let isActive = recordInfo.fields["isActive"]?.boolValue
--
Examples/Bushel/Sources/BushelImages/Models/XcodeVersionRecord.swift-88-
Examples/Bushel/Sources/BushelImages/Models/XcodeVersionRecord.swift:89: static func from(recordInfo: RecordInfo) -> Self? {
Examples/Bushel/Sources/BushelImages/Models/XcodeVersionRecord.swift-90- guard let version = recordInfo.fields["version"]?.stringValue,
Examples/Bushel/Sources/BushelImages/Models/XcodeVersionRecord.swift-91- let buildNumber = recordInfo.fields["buildNumber"]?.stringValue,
Examples/Bushel/Sources/BushelImages/Models/XcodeVersionRecord.swift-92- let releaseDate = recordInfo.fields["releaseDate"]?.dateValue
--
Examples/Bushel/Sources/BushelImages/Models/SwiftVersionRecord.swift-50-
Examples/Bushel/Sources/BushelImages/Models/SwiftVersionRecord.swift:51: static func from(recordInfo: RecordInfo) -> Self? {
Examples/Bushel/Sources/BushelImages/Models/SwiftVersionRecord.swift-52- guard let version = recordInfo.fields["version"]?.stringValue,
Examples/Bushel/Sources/BushelImages/Models/SwiftVersionRecord.swift-53- let releaseDate = recordInfo.fields["releaseDate"]?.dateValue
Examples/Bushel/Sources/BushelImages/Models/SwiftVersionRecord.swift-54- else {
--
Examples/Bushel/Sources/BushelImages/Models/DataSourceMetadata.swift-83-
Examples/Bushel/Sources/BushelImages/Models/DataSourceMetadata.swift:84: public static func from(recordInfo: RecordInfo) -> Self? {
Examples/Bushel/Sources/BushelImages/Models/DataSourceMetadata.swift-85- guard let sourceName = recordInfo.fields["sourceName"]?.stringValue,
Examples/Bushel/Sources/BushelImages/Models/DataSourceMetadata.swift-86- let recordTypeName = recordInfo.fields["recordTypeName"]?.stringValue,
Examples/Bushel/Sources/BushelImages/Models/DataSourceMetadata.swift-87- let lastFetchedAt = recordInfo.fields["lastFetchedAt"]?.dateValue
--
Examples/Bushel/Sources/BushelImages/Models/RestoreImageRecord.swift-81-
Examples/Bushel/Sources/BushelImages/Models/RestoreImageRecord.swift:82: static func from(recordInfo: RecordInfo) -> Self? {
Examples/Bushel/Sources/BushelImages/Models/RestoreImageRecord.swift-83- guard let version = recordInfo.fields["version"]?.stringValue,
Examples/Bushel/Sources/BushelImages/Models/RestoreImageRecord.swift-84- let buildNumber = recordInfo.fields["buildNumber"]?.stringValue,
Examples/Bushel/Sources/BushelImages/Models/RestoreImageRecord.swift-85- let releaseDate = recordInfo.fields["releaseDate"]?.dateValue,
</shell_result>
Script executed:
#!/bin/bash
# Read the FieldValue.swift file to check for from method
cat -n Sources/MistKit/FieldValue.swiftRepository: brightdigit/MistKit
<shell_result>
Repository: brightdigit/MistKit
Exit code: 0
stdout:
1 //
2 // FieldValue.swift
3 // MistKit
4 //
5 // Created by Leo Dion.
6 // Copyright © 2025 BrightDigit.
7 //
8 // Permission is hereby granted, free of charge, to any person
9 // obtaining a copy of this software and associated documentation
10 // files (the “Software”), to deal in the Software without
11 // restriction, including without limitation the rights to use,
12 // copy, modify, merge, publish, distribute, sublicense, and/or
13 // sell copies of the Software, and to permit persons to whom the
14 // Software is furnished to do so, subject to the following
15 // conditions:
16 //
17 // The above copyright notice and this permission notice shall be
18 // included in all copies or substantial portions of the Software.
19 //
20 // THE SOFTWARE IS PROVIDED “AS IS”, WITHOUT WARRANTY OF ANY KIND,
21 // EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
22 // OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
23 // NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
24 // HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
25 // WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
26 // FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
27 // OTHER DEALINGS IN THE SOFTWARE.
28 //
29
30 public import Foundation
31
32 /// Represents a CloudKit field value as defined in the CloudKit Web Services API
33 public enum FieldValue: Codable, Equatable, Sendable {
34 /// Conversion factor from seconds to milliseconds for CloudKit timestamps
35 private static let millisecondsPerSecond: Double = 1_000
36 case string(String)
37 case int64(Int)
38 case double(Double)
39 case bytes(String) // Base64-encoded string
40 case date(Date) // Date/time value
41 case location(Location)
42 case reference(Reference)
43 case asset(Asset)
44 case list([FieldValue])
45
46 /// Location dictionary as defined in CloudKit Web Services
47 public struct Location: Codable, Equatable, Sendable {
48 /// The latitude coordinate
49 public let latitude: Double
50 /// The longitude coordinate
51 public let longitude: Double
52 /// The horizontal accuracy in meters
53 public let horizontalAccuracy: Double?
54 /// The vertical accuracy in meters
55 public let verticalAccuracy: Double?
56 /// The altitude in meters
57 public let altitude: Double?
58 /// The speed in meters per second
59 public let speed: Double?
60 /// The course in degrees
61 public let course: Double?
62 /// The timestamp when location was recorded
63 public let timestamp: Date?
64
65 /// Initialize a location value
66 public init(
67 latitude: Double,
68 longitude: Double,
69 horizontalAccuracy: Double? = nil,
70 verticalAccuracy: Double? = nil,
71 altitude: Double? = nil,
72 speed: Double? = nil,
73 course: Double? = nil,
74 timestamp: Date? = nil
75 ) {
76 self.latitude = latitude
77 self.longitude = longitude
78 self.horizontalAccuracy = horizontalAccuracy
79 self.verticalAccuracy = verticalAccuracy
80 self.altitude = altitude
81 self.speed = speed
82 self.course = course
83 self.timestamp = timestamp
84 }
85 }
86
87 /// Reference dictionary as defined in CloudKit Web Services
88 public struct Reference: Codable, Equatable, Sendable {
89 /// Reference action types supported by CloudKit
90 public enum Action: String, Codable, Sendable {
91 case deleteSelf = "DELETE_SELF"
92 case none = "NONE"
93 }
94
95 /// The record name being referenced
96 public let recordName: String
97 /// The action to take (DELETE_SELF, NONE, or nil)
98 public let action: Action?
99
100 /// Initialize a reference value
101 public init(recordName: String, action: Action? = nil) {
102 self.recordName = recordName
103 self.action = action
104 }
105 }
106
107 /// Asset dictionary as defined in CloudKit Web Services
108 public struct Asset: Codable, Equatable, Sendable {
109 /// The file checksum
110 public let fileChecksum: String?
111 /// The file size in bytes
112 public let size: Int64?
113 /// The reference checksum
114 public let referenceChecksum: String?
115 /// The wrapping key for encryption
116 public let wrappingKey: String?
117 /// The upload receipt
118 public let receipt: String?
119 /// The download URL
120 public let downloadURL: String?
121
122 /// Initialize an asset value
123 public init(
124 fileChecksum: String? = nil,
125 size: Int64? = nil,
126 referenceChecksum: String? = nil,
127 wrappingKey: String? = nil,
128 receipt: String? = nil,
129 downloadURL: String? = nil
130 ) {
131 self.fileChecksum = fileChecksum
132 self.size = size
133 self.referenceChecksum = referenceChecksum
134 self.wrappingKey = wrappingKey
135 self.receipt = receipt
136 self.downloadURL = downloadURL
137 }
138 }
139
140 // MARK: - Codable
141 /// Initialize field value from decoder
142 public init(from decoder: any Decoder) throws {
143 let container = try decoder.singleValueContainer()
144
145 if let value = try Self.decodeBasicTypes(from: container) {
146 self = value
147 return
148 }
149
150 if let value = try Self.decodeComplexTypes(from: container) {
151 self = value
152 return
153 }
154
155 throw DecodingError.dataCorruptedError(
156 in: container,
157 debugDescription: "Unable to decode FieldValue"
158 )
159 }
160
161 /// Decode basic field value types (string, int64, double)
162 private static func decodeBasicTypes(from container: any SingleValueDecodingContainer) throws
163 -> FieldValue?
164 {
165 if let value = try? container.decode(String.self) {
166 return .string(value)
167 }
168 if let value = try? container.decode(Int.self) {
169 return .int64(value)
170 }
171 if let value = try? container.decode(Double.self) {
172 return .double(value)
173 }
174 return nil
175 }
176
177 /// Decode complex field value types (list, location, reference, asset, date)
178 private static func decodeComplexTypes(from container: any SingleValueDecodingContainer) throws
179 -> FieldValue?
180 {
181 if let value = try? container.decode([FieldValue].self) {
182 return .list(value)
183 }
184 if let value = try? container.decode(Location.self) {
185 return .location(value)
186 }
187 if let value = try? container.decode(Reference.self) {
188 return .reference(value)
189 }
190 if let value = try? container.decode(Asset.self) {
191 return .asset(value)
192 }
193 // Try to decode as date (milliseconds since epoch)
194 if let value = try? container.decode(Double.self) {
195 return .date(Date(timeIntervalSince1970: value / Self.millisecondsPerSecond))
196 }
197 return nil
198 }
199
200 /// Encode field value to encoder
201 public func encode(to encoder: any Encoder) throws {
202 var container = encoder.singleValueContainer()
203 try encodeValue(to: &container)
204 }
205
206 // swiftlint:disable:next cyclomatic_complexity
207 private func encodeValue(to container: inout any SingleValueEncodingContainer) throws {
208 switch self {
209 case .string(let val), .bytes(let val):
210 try container.encode(val)
211 case .int64(let val):
212 try container.encode(val)
213 case .double(let val):
214 try container.encode(val)
215 case .date(let val):
216 try container.encode(val.timeIntervalSince1970 * Self.millisecondsPerSecond)
217 case .location(let val):
218 try container.encode(val)
219 case .reference(let val):
220 try container.encode(val)
221 case .asset(let val):
222 try container.encode(val)
223 case .list(let val):
224 try container.encode(val)
225 }
226 }
227 }
228
229 // MARK: - Helper Methods
230
231 extension FieldValue {
232 /// Create an int64 FieldValue from a Bool
233 ///
234 /// CloudKit represents booleans as INT64 (0/1) on the wire.
235 /// Creates a FieldValue from a Swift Bool value.
236 ///
237 /// This initializer converts Swift Bool to the appropriate int64 representation
238 /// used by CloudKit (1 for true, 0 for false).
239 ///
240 /// - Parameter booleanValue: The boolean value to convert
241 public init(booleanValue: Bool) {
242 self = .int64(booleanValue ? 1 : 0)
243 }
244 }
</shell_result>
Script executed:
#!/bin/bash
# Read the CloudKitFieldMapping.swift file to see the exact code
cat -n Examples/Bushel/Sources/BushelImages/CloudKit/CloudKitFieldMapping.swiftRepository: brightdigit/MistKit
<shell_result>
Repository: brightdigit/MistKit
Exit code: 0
stdout:
1 import Foundation
2 import MistKit
3
4 /// Helper utilities for converting between Swift types and CloudKit FieldValue types
5 enum CloudKitFieldMapping {
6 /// Convert a String to FieldValue
7 static func fieldValue(from string: String) -> FieldValue {
8 .string(string)
9 }
10
11 /// Convert an optional String to FieldValue
12 static func fieldValue(from string: String?) -> FieldValue? {
13 string.map { .string($0) }
14 }
15
16 /// Convert a Bool to FieldValue (using INT64 representation: 0 = false, 1 = true)
17 static func fieldValue(from bool: Bool) -> FieldValue {
18 .from(bool)
19 }
20
21 /// Convert an Int64 to FieldValue
22 static func fieldValue(from int64: Int64) -> FieldValue {
23 .int64(Int(int64))
24 }
25
26 /// Convert an optional Int64 to FieldValue
27 static func fieldValue(from int64: Int64?) -> FieldValue? {
28 int64.map { .int64(Int($0)) }
29 }
30
31 /// Convert a Date to FieldValue
32 static func fieldValue(from date: Date) -> FieldValue {
33 .date(date)
34 }
35
36 /// Convert a CloudKit reference (recordName) to FieldValue
37 static func referenceFieldValue(recordName: String) -> FieldValue {
38 .reference(FieldValue.Reference(recordName: recordName))
39 }
40
41 /// Convert an optional CloudKit reference to FieldValue
42 static func referenceFieldValue(recordName: String?) -> FieldValue? {
43 recordName.map { .reference(FieldValue.Reference(recordName: $0)) }
44 }
45
46 /// Extract String from FieldValue
47 static func string(from fieldValue: FieldValue) -> String? {
48 if case .string(let value) = fieldValue {
49 return value
50 }
51 return nil
52 }
53
54 /// Extract Bool from FieldValue (from INT64 representation: 0 = false, non-zero = true)
55 static func bool(from fieldValue: FieldValue) -> Bool? {
56 if case .int64(let value) = fieldValue {
57 return value != 0
58 }
59 return nil
60 }
61
62 /// Extract Int64 from FieldValue
63 static func int64(from fieldValue: FieldValue) -> Int64? {
64 if case .int64(let value) = fieldValue {
65 return Int64(value)
66 }
67 return nil
68 }
69
70 /// Extract Date from FieldValue
71 static func date(from fieldValue: FieldValue) -> Date? {
72 if case .date(let value) = fieldValue {
73 return value
74 }
75 return nil
76 }
77
78 /// Extract reference recordName from FieldValue
79 static func recordName(from fieldValue: FieldValue) -> String? {
80 if case .reference(let reference) = fieldValue {
81 return reference.recordName
82 }
83 return nil
84 }
85 }
</shell_result>
The review comment is correct: FieldValue.from(bool:) method does not exist and the code will fail to compile.
After verification, I found that:
- The
FieldValuetype is defined inSources/MistKit/FieldValue.swiftas a public enum - There is NO static
from(bool:)method onFieldValue - The correct API is
public init(booleanValue: Bool)(lines 241-243 in FieldValue.swift), which creates an.int64case with value 1 or 0
The code at line 18 in CloudKitFieldMapping.swift should be:
static func fieldValue(from bool: Bool) -> FieldValue {
FieldValue(booleanValue: bool)
}This is consistent with how other converters in the same file use case constructors or the proper initializers (e.g., .string(string), .int64(...), .date(date), .reference(...)).
🤖 Prompt for AI Agents
In Examples/Bushel/Sources/BushelImages/CloudKit/CloudKitFieldMapping.swift
around lines 16 to 19, the code calls a non-existent static method
FieldValue.from(bool:) which will not compile; replace that call with the
correct initializer by constructing the FieldValue using
FieldValue(booleanValue: bool) so the boolean is converted to the .int64
representation (0/1) as implemented in Sources/MistKit/FieldValue.swift.
| @Option(name: .long, help: "Server-to-Server Key ID (or set CLOUDKIT_KEY_ID)") | ||
| var keyID: String = "" | ||
|
|
||
| @Option(name: .long, help: "Path to private key .pem file (or set CLOUDKIT_PRIVATE_KEY_PATH)") | ||
| var keyFile: String = "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Script executed:
#!/bin/bash
# Check for environment variable naming consistency across the codebase
rg -n "CLOUDKIT_KEY_FILE|CLOUDKIT_PRIVATE_KEY_PATH" --type swift --type mdRepository: brightdigit/MistKit
<shell_result>
Repository: brightdigit/MistKit
Exit code: 0
stdout:
Examples/Celestra/Sources/Celestra/Celestra.swift:38: guard let privateKeyPath = ProcessInfo.processInfo.environment["CLOUDKIT_PRIVATE_KEY_PATH"]
Examples/Celestra/Sources/Celestra/Celestra.swift:40: throw ValidationError("CLOUDKIT_PRIVATE_KEY_PATH environment variable required")
Examples/Bushel/Sources/BushelImages/Commands/ExportCommand.swift:23: @Option(name: .long, help: "Path to private key .pem file (or set CLOUDKIT_PRIVATE_KEY_PATH)")
Examples/Bushel/Sources/BushelImages/Commands/ExportCommand.swift:55: ProcessInfo.processInfo.environment["CLOUDKIT_PRIVATE_KEY_PATH"] ?? "" :
Examples/Bushel/Sources/BushelImages/Commands/ExportCommand.swift:66: print(" export CLOUDKIT_PRIVATE_KEY_PATH=\"./private-key.pem\"")
Examples/Bushel/Sources/BushelImages/Commands/StatusCommand.swift:29: @Option(name: .long, help: "Path to private key .pem file (or set CLOUDKIT_PRIVATE_KEY_PATH)")
Examples/Bushel/Sources/BushelImages/Commands/StatusCommand.swift:57: ProcessInfo.processInfo.environment["CLOUDKIT_PRIVATE_KEY_PATH"] ?? "" :
Examples/Bushel/Sources/BushelImages/Commands/StatusCommand.swift:68: print(" export CLOUDKIT_PRIVATE_KEY_PATH=\"./private-key.pem\"")
Examples/Bushel/Sources/BushelImages/Commands/SyncCommand.swift:27: @Option(name: .long, help: "Path to private key .pem file (or set CLOUDKIT_PRIVATE_KEY_PATH)")
Examples/Bushel/Sources/BushelImages/Commands/SyncCommand.swift:84: ProcessInfo.processInfo.environment["CLOUDKIT_PRIVATE_KEY_PATH"] ?? "" :
Examples/Bushel/Sources/BushelImages/Commands/SyncCommand.swift:95: print(" export CLOUDKIT_PRIVATE_KEY_PATH=\"./private-key.pem\"")
Examples/Bushel/Sources/BushelImages/Commands/ClearCommand.swift:24: @Option(name: .long, help: "Path to private key .pem file (or set CLOUDKIT_PRIVATE_KEY_PATH)")
Examples/Bushel/Sources/BushelImages/Commands/ClearCommand.swift:47: ProcessInfo.processInfo.environment["CLOUDKIT_PRIVATE_KEY_PATH"] ?? "" :
Examples/Bushel/Sources/BushelImages/Commands/ClearCommand.swift:58: print(" export CLOUDKIT_PRIVATE_KEY_PATH=\"./private-key.pem\"")
Examples/Celestra/README.md:113:CLOUDKIT_PRIVATE_KEY_PATH=/path/to/eckey.pem
Examples/Bushel/Sources/BushelImages/Commands/ListCommand.swift:27: @Option(name: .long, help: "Path to private key .pem file (or set CLOUDKIT_PRIVATE_KEY_PATH)")
Examples/Bushel/Sources/BushelImages/Commands/ListCommand.swift:60: ProcessInfo.processInfo.environment["CLOUDKIT_PRIVATE_KEY_PATH"] ?? "" :
Examples/Bushel/Sources/BushelImages/Commands/ListCommand.swift:71: print(" export CLOUDKIT_PRIVATE_KEY_PATH=\"./private-key.pem\"")
Examples/Celestra/BUSHEL_PATTERNS.md:208:export CLOUDKIT_KEY_FILE="$HOME/.cloudkit/bushel-private-key.pem"
Examples/Bushel/README.md:161:export CLOUDKIT_KEY_FILE="./path/to/private-key.pem"
Examples/Bushel/README.md:204:export CLOUDKIT_KEY_FILE="./path/to/private-key.pem"
Examples/Bushel/README.md:418:export CLOUDKIT_KEY_FILE="$HOME/.cloudkit/bushel-private-key.pem"
Examples/Bushel/README.md:473:export CLOUDKIT_KEY_FILE="$HOME/.cloudkit/bushel-private-key.pem"
Examples/Bushel/README.md:474:ls -la "$CLOUDKIT_KEY_FILE" # Verify file exists
Examples/Bushel/IMPLEMENTATION_NOTES.md:57:| `CLOUDKIT_API_TOKEN` env var | `CLOUDKIT_KEY_ID` + `CLOUDKIT_KEY_FILE` |
Examples/Bushel/IMPLEMENTATION_NOTES.md:77:export CLOUDKIT_KEY_FILE="$HOME/.cloudkit/bushel-private-key.pem"
Examples/Bushel/IMPLEMENTATION_NOTES.md:307:export CLOUDKIT_KEY_FILE="$HOME/.cloudkit/bushel-private-key.pem"
</shell_result>
The verification reveals a significant documentation inconsistency across the Bushel examples. The review comment correctly identified an inconsistency, but the scope is broader than described:
Actual Issue:
- All Swift code in Bushel commands consistently uses
CLOUDKIT_PRIVATE_KEY_PATH - However, the documentation files (IMPLEMENTATION_NOTES.md, README.md) reference
CLOUDKIT_KEY_FILEinstead - This creates a mismatch where developers following the docs will set the wrong environment variable name
The code is internally consistent, but the documentation contradicts it. This is a documentation bug that affects user experience across the entire Bushel example, not just ListCommand.
Environment variable naming inconsistency between code and documentation.
The Swift code uses CLOUDKIT_PRIVATE_KEY_PATH, but IMPLEMENTATION_NOTES.md (lines 57, 77) and most of README.md reference CLOUDKIT_KEY_FILE. This inconsistency affects all Bushel commands and will confuse users trying to set up the environment. Align the documentation to match the actual environment variable names used in the code.
🤖 Prompt for AI Agents
In Examples/Bushel/Sources/BushelImages/Commands/ListCommand.swift around lines
24-28 there is a review noting an environment variable naming mismatch: the
Swift code uses CLOUDKIT_PRIVATE_KEY_PATH but the docs reference
CLOUDKIT_KEY_FILE; update the documentation to match the code. Search README.md
and IMPLEMENTATION_NOTES.md (noting lines 57 and 77) and replace any occurrences
of CLOUDKIT_KEY_FILE with CLOUDKIT_PRIVATE_KEY_PATH (and verify any examples or
env blocks show the exact name), then run a repo-wide search to ensure all
documentation and examples consistently use CLOUDKIT_PRIVATE_KEY_PATH so docs
align with the code.
| import Foundation | ||
|
|
||
| /// Represents a single macOS build entry from AppleDB | ||
| struct AppleDBEntry: Codable { | ||
| let osStr: String | ||
| let version: String | ||
| let build: String? // Some entries may not have a build number | ||
| let uniqueBuild: String? | ||
| let released: String // ISO date or empty string | ||
| let beta: Bool? | ||
| let rc: Bool? | ||
| let `internal`: Bool? | ||
| let deviceMap: [String] | ||
| let signed: SignedStatus | ||
| let sources: [AppleDBSource]? | ||
|
|
||
| enum CodingKeys: String, CodingKey { | ||
| case osStr, version, build, uniqueBuild, released | ||
| case beta, rc | ||
| case `internal` = "internal" | ||
| case deviceMap, signed, sources | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Add Sendable conformance for Swift 6 compliance.
The coding guidelines require all types to be Sendable for Swift Concurrency safety. This struct contains only value types and should explicitly conform to Sendable.
Apply this diff:
/// Represents a single macOS build entry from AppleDB
-struct AppleDBEntry: Codable {
+struct AppleDBEntry: Codable, Sendable {
let osStr: String
let version: StringBased on coding guidelines, ensure that SignedStatus and AppleDBSource types also conform to Sendable.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import Foundation | |
| /// Represents a single macOS build entry from AppleDB | |
| struct AppleDBEntry: Codable { | |
| let osStr: String | |
| let version: String | |
| let build: String? // Some entries may not have a build number | |
| let uniqueBuild: String? | |
| let released: String // ISO date or empty string | |
| let beta: Bool? | |
| let rc: Bool? | |
| let `internal`: Bool? | |
| let deviceMap: [String] | |
| let signed: SignedStatus | |
| let sources: [AppleDBSource]? | |
| enum CodingKeys: String, CodingKey { | |
| case osStr, version, build, uniqueBuild, released | |
| case beta, rc | |
| case `internal` = "internal" | |
| case deviceMap, signed, sources | |
| } | |
| } | |
| import Foundation | |
| /// Represents a single macOS build entry from AppleDB | |
| struct AppleDBEntry: Codable, Sendable { | |
| let osStr: String | |
| let version: String | |
| let build: String? // Some entries may not have a build number | |
| let uniqueBuild: String? | |
| let released: String // ISO date or empty string | |
| let beta: Bool? | |
| let rc: Bool? | |
| let `internal`: Bool? | |
| let deviceMap: [String] | |
| let signed: SignedStatus | |
| let sources: [AppleDBSource]? | |
| enum CodingKeys: String, CodingKey { | |
| case osStr, version, build, uniqueBuild, released | |
| case beta, rc | |
| case `internal` = "internal" | |
| case deviceMap, signed, sources | |
| } | |
| } |
🤖 Prompt for AI Agents
In Examples/Bushel/Sources/BushelImages/DataSources/AppleDB/AppleDBEntry.swift
around lines 1 to 23, the AppleDBEntry type must explicitly conform to Sendable
for Swift 6 concurrency; update the struct declaration to add : Sendable and
ensure all contained types (SignedStatus and AppleDBSource) also declare
Sendable conformance (or are conditionally Sendable) so the compiler recognizes
the entire graph as Sendable; if SignedStatus or AppleDBSource are in other
files, add Sendable conformance there as well (or mark with @unchecked Sendable
only if they contain non-Sendable reference types and you have justified
safety).
| /// Create a copy of this article with a specific recordName | ||
| /// - Parameter recordName: The CloudKit record name to set | ||
| /// - Returns: New Article instance with the recordName set | ||
| func withRecordName(_ recordName: String) -> Article { | ||
| Article( | ||
| recordName: recordName, | ||
| feed: self.feed, | ||
| title: self.title, | ||
| link: self.link, | ||
| description: self.description, | ||
| content: self.content, | ||
| author: self.author, | ||
| pubDate: self.pubDate, | ||
| guid: self.guid, | ||
| ttlDays: 30 // Use default TTL since we can't calculate from existing dates | ||
| ) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
withRecordName currently resets TTL and timestamps; preserve existing dates instead
withRecordName(_:) rebuilds the Article using the convenience init(…, ttlDays:) with a hardcoded ttlDays: 30. That has two side effects:
- It discards the original
fetchedAtandexpiresAtvalues, replacing them with “now” and “now + 30 days”. - If the instance was originally created with a custom
ttlDays(not 30), callingwithRecordNamesilently changes its retention window.
For a helper that’s conceptually “set recordName only”, this is surprising and can alter your cleanup semantics.
A safer implementation is to preserve the existing timestamps and TTL, e.g.:
- func withRecordName(_ recordName: String) -> Article {
- Article(
- recordName: recordName,
- feed: self.feed,
- title: self.title,
- link: self.link,
- description: self.description,
- content: self.content,
- author: self.author,
- pubDate: self.pubDate,
- guid: self.guid,
- ttlDays: 30 // Use default TTL since we can't calculate from existing dates
- )
- }
+ func withRecordName(_ recordName: String) -> Article {
+ Article(
+ recordName: recordName,
+ feed: feed,
+ title: title,
+ link: link,
+ description: description,
+ content: content,
+ author: author,
+ pubDate: pubDate,
+ guid: guid,
+ fetchedAt: fetchedAt,
+ expiresAt: expiresAt
+ )
+ }You can add the second initializer taking fetchedAt/expiresAt explicitly if needed. That way, withRecordName only changes the record name, not the logical lifetime of the article.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Examples/Celestra/Sources/Celestra/Models/Article.swift around lines 147–163,
withRecordName(_:) currently reconstructs the Article with a hardcoded ttlDays:
30 which resets fetchedAt/expiresAt and overwrites any custom ttl; update
withRecordName to preserve the instance's existing fetchedAt, expiresAt and
ttlDays by calling an initializer that accepts those fields (or add such an
initializer if it doesn't exist) so the method only changes recordName and
leaves timestamps/TTL unchanged.
| import MistKit | ||
|
|
||
| /// Represents an RSS feed stored in CloudKit's public database | ||
| struct Feed { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Missing Sendable conformance.
Per coding guidelines, all types should be Sendable for Swift Concurrency safety. Since all properties are value types, adding Sendable conformance should be straightforward.
-struct Feed {
+struct Feed: Sendable {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| struct Feed { | |
| struct Feed: Sendable { |
🤖 Prompt for AI Agents
In Examples/Celestra/Sources/Celestra/Models/Feed.swift around line 5, the Feed
struct lacks Sendable conformance required by our concurrency guidelines; add
Sendable to its declaration (e.g., make the struct conform to Sendable) because
its stored properties are value types, or if any property is not Sendable,
explicitly mark the struct with @unchecked Sendable and document why.
| if guids.count <= 10 { | ||
| // Create OR filter for GUIDs | ||
| let guidFilters = guids.map { guid in | ||
| QueryFilter.equals("guid", .string(guid)) | ||
| } | ||
|
|
||
| // Combine with feed filter if present | ||
| if !filters.isEmpty { | ||
| // When we have both feed and GUID filters, we need to do multiple queries | ||
| // or fetch all for feed and filter in memory | ||
| filters.append(.equals("guid", .string(guids[0]))) | ||
|
|
||
| let records = try await queryRecords( | ||
| recordType: "Article", | ||
| filters: filters, | ||
| limit: 200, | ||
| desiredKeys: ["guid", "contentHash", "___recordID"] | ||
| ) | ||
|
|
||
| // For now, fetch all articles for this feed and filter in memory | ||
| let allFeedArticles = records.map { Article(from: $0) } | ||
| let guidSet = Set(guids) | ||
| return allFeedArticles.filter { guidSet.contains($0.guid) } | ||
| } else { | ||
| // Just GUID filter - need to query each individually or use contentHash | ||
| // For simplicity, query by feedRecordName first then filter | ||
| let records = try await queryRecords( | ||
| recordType: "Article", | ||
| limit: 200, | ||
| desiredKeys: ["guid", "contentHash", "___recordID"] | ||
| ) | ||
|
|
||
| let articles = records.map { Article(from: $0) } | ||
| let guidSet = Set(guids) | ||
| return articles.filter { guidSet.contains($0.guid) } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dead code and ineffective optimization logic.
The guidFilters variable on lines 98-100 is created but never used. Additionally, the if guids.count <= 10 branch claims to optimize for small GUID sets, but:
- Line 106 only adds
guids[0]to the filter, ignoring the rest - Both branches ultimately fetch all articles and filter in memory, defeating the optimization purpose
Consider either implementing the intended OR-filter query or simplifying to always fetch-and-filter:
- if guids.count <= 10 {
- // Create OR filter for GUIDs
- let guidFilters = guids.map { guid in
- QueryFilter.equals("guid", .string(guid))
- }
-
- // Combine with feed filter if present
- if !filters.isEmpty {
- // When we have both feed and GUID filters, we need to do multiple queries
- // or fetch all for feed and filter in memory
- filters.append(.equals("guid", .string(guids[0])))
-
- let records = try await queryRecords(
- recordType: "Article",
- filters: filters,
- limit: 200,
- desiredKeys: ["guid", "contentHash", "___recordID"]
- )
-
- // For now, fetch all articles for this feed and filter in memory
- let allFeedArticles = records.map { Article(from: $0) }
- let guidSet = Set(guids)
- return allFeedArticles.filter { guidSet.contains($0.guid) }
- } else {
- // Just GUID filter - need to query each individually or use contentHash
- // For simplicity, query by feedRecordName first then filter
- let records = try await queryRecords(
- recordType: "Article",
- limit: 200,
- desiredKeys: ["guid", "contentHash", "___recordID"]
- )
-
- let articles = records.map { Article(from: $0) }
- let guidSet = Set(guids)
- return articles.filter { guidSet.contains($0.guid) }
- }
- } else {
+ // Simplify: fetch articles for feed (if specified) and filter by GUID in memory
+ {
// For large GUID sets, fetch all articles for the feed and filter in memoryCommittable suggestion skipped: line range outside the PR's diff.
| /// Delete all Feed records | ||
| func deleteAllFeeds() async throws { | ||
| let feeds = try await queryRecords( | ||
| recordType: "Feed", | ||
| limit: 200, | ||
| desiredKeys: ["___recordID"] | ||
| ) | ||
|
|
||
| guard !feeds.isEmpty else { | ||
| return | ||
| } | ||
|
|
||
| let operations = feeds.map { record in | ||
| RecordOperation.delete( | ||
| recordType: "Feed", | ||
| recordName: record.recordName, | ||
| recordChangeTag: record.recordChangeTag | ||
| ) | ||
| } | ||
|
|
||
| _ = try await modifyRecords(operations) | ||
| } | ||
|
|
||
| /// Delete all Article records | ||
| func deleteAllArticles() async throws { | ||
| let articles = try await queryRecords( | ||
| recordType: "Article", | ||
| limit: 200, | ||
| desiredKeys: ["___recordID"] | ||
| ) | ||
|
|
||
| guard !articles.isEmpty else { | ||
| return | ||
| } | ||
|
|
||
| let operations = articles.map { record in | ||
| RecordOperation.delete( | ||
| recordType: "Article", | ||
| recordName: record.recordName, | ||
| recordChangeTag: record.recordChangeTag | ||
| ) | ||
| } | ||
|
|
||
| _ = try await modifyRecords(operations) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deleteAll* functions may not delete all records.
Both functions have a hardcoded limit: 200 without pagination. If more than 200 records exist, the remaining records are silently left behind, contradicting the function names.
Consider looping until no records remain:
func deleteAllFeeds() async throws {
- let feeds = try await queryRecords(
- recordType: "Feed",
- limit: 200,
- desiredKeys: ["___recordID"]
- )
-
- guard !feeds.isEmpty else {
- return
- }
-
- let operations = feeds.map { record in
- RecordOperation.delete(
- recordType: "Feed",
- recordName: record.recordName,
- recordChangeTag: record.recordChangeTag
+ while true {
+ let feeds = try await queryRecords(
+ recordType: "Feed",
+ limit: 200,
+ desiredKeys: ["___recordID"]
)
+
+ guard !feeds.isEmpty else { return }
+
+ let operations = feeds.map { record in
+ RecordOperation.delete(
+ recordType: "Feed",
+ recordName: record.recordName,
+ recordChangeTag: record.recordChangeTag
+ )
+ }
+ _ = try await modifyRecords(operations)
}
-
- _ = try await modifyRecords(operations)
}Apply the same pattern to deleteAllArticles.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Delete all Feed records | |
| func deleteAllFeeds() async throws { | |
| let feeds = try await queryRecords( | |
| recordType: "Feed", | |
| limit: 200, | |
| desiredKeys: ["___recordID"] | |
| ) | |
| guard !feeds.isEmpty else { | |
| return | |
| } | |
| let operations = feeds.map { record in | |
| RecordOperation.delete( | |
| recordType: "Feed", | |
| recordName: record.recordName, | |
| recordChangeTag: record.recordChangeTag | |
| ) | |
| } | |
| _ = try await modifyRecords(operations) | |
| } | |
| /// Delete all Article records | |
| func deleteAllArticles() async throws { | |
| let articles = try await queryRecords( | |
| recordType: "Article", | |
| limit: 200, | |
| desiredKeys: ["___recordID"] | |
| ) | |
| guard !articles.isEmpty else { | |
| return | |
| } | |
| let operations = articles.map { record in | |
| RecordOperation.delete( | |
| recordType: "Article", | |
| recordName: record.recordName, | |
| recordChangeTag: record.recordChangeTag | |
| ) | |
| } | |
| _ = try await modifyRecords(operations) | |
| } | |
| /// Delete all Feed records | |
| func deleteAllFeeds() async throws { | |
| while true { | |
| let feeds = try await queryRecords( | |
| recordType: "Feed", | |
| limit: 200, | |
| desiredKeys: ["___recordID"] | |
| ) | |
| guard !feeds.isEmpty else { return } | |
| let operations = feeds.map { record in | |
| RecordOperation.delete( | |
| recordType: "Feed", | |
| recordName: record.recordName, | |
| recordChangeTag: record.recordChangeTag | |
| ) | |
| } | |
| _ = try await modifyRecords(operations) | |
| } | |
| } | |
| /// Delete all Article records | |
| func deleteAllArticles() async throws { | |
| while true { | |
| let articles = try await queryRecords( | |
| recordType: "Article", | |
| limit: 200, | |
| desiredKeys: ["___recordID"] | |
| ) | |
| guard !articles.isEmpty else { return } | |
| let operations = articles.map { record in | |
| RecordOperation.delete( | |
| recordType: "Article", | |
| recordName: record.recordName, | |
| recordChangeTag: record.recordChangeTag | |
| ) | |
| } | |
| _ = try await modifyRecords(operations) | |
| } | |
| } |
| struct RSSFetcherService { | ||
| private let urlSession: URLSession | ||
| private let userAgent: String | ||
|
|
||
| struct FeedData { | ||
| let title: String | ||
| let description: String? | ||
| let items: [FeedItem] | ||
| let minUpdateInterval: TimeInterval? // Parsed from <ttl> or <updatePeriod> | ||
| } | ||
|
|
||
| struct FeedItem { | ||
| let title: String | ||
| let link: String | ||
| let description: String? | ||
| let content: String? | ||
| let author: String? | ||
| let pubDate: Date? | ||
| let guid: String | ||
| } | ||
|
|
||
| struct FetchResponse { | ||
| let feedData: FeedData? // nil if 304 Not Modified | ||
| let lastModified: String? | ||
| let etag: String? | ||
| let wasModified: Bool | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Add Sendable conformance for Swift 6 compliance.
The coding guidelines require all types to be Sendable for Swift Concurrency safety. The PR objectives mention "Sendable conformance across code for Swift 6." These structs contain only value types and should explicitly conform to Sendable.
Apply these changes:
@available(macOS 13.0, *)
-struct RSSFetcherService {
+struct RSSFetcherService: Sendable {
private let urlSession: URLSession
private let userAgent: String
- struct FeedData {
+ struct FeedData: Sendable {
let title: String
let description: String?
let items: [FeedItem]
let minUpdateInterval: TimeInterval?
}
- struct FeedItem {
+ struct FeedItem: Sendable {
let title: String
let link: String
let description: String?
let content: String?
let author: String?
let pubDate: Date?
let guid: String
}
- struct FetchResponse {
+ struct FetchResponse: Sendable {
let feedData: FeedData?
let lastModified: String?
let etag: String?
let wasModified: Bool
}🤖 Prompt for AI Agents
In Examples/Celestra/Sources/Celestra/Services/RSSFetcherService.swift around
lines 7 to 33, the nested value types need explicit Sendable conformance for
Swift 6 concurrency; add ": Sendable" to the FeedData, FeedItem and
FetchResponse struct declarations, and verify their stored properties are
Sendable (or use @unchecked Sendable only if a non-Sendable field is
unavoidable). Do not change RSSFetcherService itself (it holds URLSession, a
non-Sendable reference), only add explicit Sendable conformance to the nested
value-type structs.
Second alpha release adding write capabilities, production-ready examples, and extensive CloudKit development documentation.
New Public API
Write Operations:
Field Value Improvements:
Error Handling:
Example Applications
Bushel (Examples/Bushel/):
Celestra (Examples/Celestra/):
MistDemo (Examples/MistDemo/):
Documentation
CloudKit Reference Documentation (.claude/docs/):
DocC Articles (Sources/MistKit/Documentation.docc/):
Development Guides:
Additional Changes
Breaking Changes
None - this release is additive only, maintaining full backward compatibility with v1.0.0-alpha.1.
🤖 Generated with Claude Code
Perform an AI-assisted review on
Summary by CodeRabbit
Release Notes
New Features
Documentation
Development
✏️ Tip: You can customize this high-level summary in your review settings.