Add documented request options to query/modify/changes (#383 #384 #385)

[leogdion]

Epic #409.

Adds the documented-but-missing request-body options across three record endpoints (openapi.yaml + regenerated MistKitOpenAPI) and surfaces them through CloudKitService:

• records/query (records/query: add zoneWide and numbersAsStrings request options #383): zoneWide, numbersAsStrings → queryRecords(_:…)
• records/modify (records/modify: add zoneID, desiredKeys, numbersAsStrings request options #384): zoneID, desiredKeys, numbersAsStrings → modifyRecords(_:…)
• records/changes (records/changes: add desiredKeys and desiredRecordTypes request options #385): desiredKeys, desiredRecordTypes → fetchRecordChanges / fetchAllRecordChanges

All new parameters are optional (default nil) → existing call sites unchanged; nil omits the field and preserves CloudKit's default behavior.

Added CloudKitServiceTests.RequestOptions asserting each option serializes into the request body via the recording MockTransport. To stay under the 225-line file_length warning, split the deprecated queryRecords overloads into CloudKitService+Operations+Deprecated.swift and the create/update/delete wrappers into CloudKitService+RecordWriteConvenience.swift (pure moves).

Out of scope (flagged in #385): live-response confirmation of RecordResponse pluginFields/zoneID and AssetUploadResponse wrappingKey.

Closes #383, closes #384, closes #385.

Verification

swift build, full swift test (541 pass), swift-format + swiftlint --strict clean. Request-body serialization unit-tested; live round-trip of zoneWide/numbersAsStrings deferred.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f6de94f9-426f-4962-be15-0d4c2e715e31

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/openapi-request-options

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.93%. Comparing base (7791b09) to head (3258ea0).
⚠️ Report is 2 commits behind head on v1.0.0-beta.3.

Additional details and impacted files

@@                Coverage Diff                @@
##           v1.0.0-beta.3     #412      +/-   ##
=================================================
+ Coverage          73.85%   74.93%   +1.08%     
=================================================
  Files                156      158       +2     
  Lines               3698     3727      +29     
=================================================
+ Hits                2731     2793      +62     
+ Misses               967      934      -33     

Flag	Coverage Δ
mistdemo-spm-macos	11.37% <0.00%> (+0.18%)	⬆️
mistdemo-swift-6.2-jammy	11.37% <0.00%> (+0.18%)	⬆️
mistdemo-swift-6.2-noble	11.48% <0.00%> (+0.28%)	⬆️
mistdemo-swift-6.3-jammy	11.37% <0.00%> (+0.18%)	⬆️
mistdemo-swift-6.3-noble	11.37% <0.00%> (+0.18%)	⬆️
spm	72.95% <100.00%> (+1.37%)	⬆️
swift-6.1-jammy	72.75% <100.00%> (+1.16%)	⬆️
swift-6.1-noble	73.08% <100.00%> (+1.42%)	⬆️
swift-6.2-jammy	72.73% <100.00%> (+1.13%)	⬆️
swift-6.2-noble	72.81% <100.00%> (+0.97%)	⬆️
swift-6.3-jammy	73.10% <100.00%> (+1.53%)	⬆️
swift-6.3-noble	72.75% <100.00%> (+1.21%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[claude]

Code Review

Summary: Adds the documented-but-missing request-body options across three endpoints (query, modify, changes). All new params are optional with nil defaults — existing call sites are unchanged.

---

Strengths

• Backward-compatible: every new parameter defaults to nil, which preserves CloudKit's default behavior and means zero churn at call sites.
• OpenAPI spec updated first, code generated from it — consistent with the project's OpenAPI-driven workflow.
• New RequestOptions test suite verifies request-body serialization via MockTransport, which is exactly the right level to test "does the parameter reach the wire" without live network calls.
• Splitting deprecated queryRecords overloads into CloudKitService+Operations+Deprecated.swift and convenience write methods into CloudKitService+RecordWriteConvenience.swift is a clean file-length fix that keeps related things together.

---

Observations / Potential Issues

zoneID on modifyRecords vs existing zone handling

modifyRecords already routes through Database (which encodes the database path). Adding an explicit zoneID parameter lets callers target a specific zone within that database — useful, but the interaction with the default zone handling should be documented. If a caller passes a zoneID, is the request's zone replaced or supplemented? The OpenAPI spec doc just says "Optional target zone" — confirming this against the CloudKit Web Services reference would close that gap.

desiredKeys on modifyRecords

The doc says "limit the fields returned in the modify response." This could be misread as filtering which fields are written. Suggest sharpening the wording to: "Limits the record fields returned in the modify response (does not affect which fields are saved)."

numbersAsStrings is Bool? not Bool

Using Bool? (so nil = omit from request, preserving CloudKit default) is correct for optional request fields. Worth noting in doc comments that nil (not false) is the "use server default" value, since callers who set false explicitly would serialize "numbersAsStrings": false unnecessarily.

Test: try? on the service call

_ = try? await service.queryRecords(…, zoneWide: true, …)

The try? silences any error. If the MockTransport were mis-configured and failed before recording the body, the #require in sentBody would catch it — so this is safe. But using try await + ignoring the return value (or _ = try await) would make the intent clearer: the call is expected to succeed.

Missing coverage: nil omits the key

The tests verify that true values serialize correctly. A complementary test checking that omitting the parameter (or passing nil) results in the key being absent from the request body would give stronger confidence that the nil → omit behavior works. This is "nice to have," not a blocker.

---

File organization

The CloudKitService+RecordWriteConvenience.swift move is pure relocation — createRecord, updateRecord, deleteRecord are unchanged. The CLAUDE.md table in the "CloudKitService Operations" section will need updating to add this new file and describe its contents. (The table currently lists CloudKitService+WriteOperations.swift for create/update/delete.)

---

Overall: Clean feature addition, well within the codebase's conventions. The zoneID interaction and desiredKeys doc wording are the only items worth a follow-up. LGTM with those addressed.

[claude]

Review

Reviewed the diff (Sources/MistKit/CloudKitService/{Operations,Operations+Deprecated,RecordWriteConvenience,SyncOperations,WriteOperations}.swift, Sources/MistKitOpenAPI/Types.swift, openapi.yaml, new RequestOptions test suite).

Overall

Clean, well-scoped addition. All new parameters (zoneWide, numbersAsStrings, zoneID, desiredKeys, desiredRecordTypes) are optional with nil defaults and inserted before the trailing non-defaulted database: label, so existing labeled call sites keep compiling unchanged — matches the PR description's non-breaking claim. Parameter ordering in each signature matches its doc-comment ordering. The Operations+Deprecated.swift / RecordWriteConvenience.swift splits are pure moves (verified byte-for-byte against the pre-move code) and bring every touched file comfortably under the 225-line file_length threshold.

The generated Sources/MistKitOpenAPI/Types.swift changes look consistent with the openapi.yaml additions (new struct properties + init params + CodingKeys cases; these jsonPayload structs rely on synthesized Codable, so no hand-written encode(to:)/init(from:) needed updating). YAML indentation/description style matches the surrounding spec.

Minor observations (non-blocking)

1. queryRecords always sends a hardcoded zoneID: .init(zoneName: "_defaultZone"), even when the caller passes zoneWide: true (CloudKitService+Operations.swift:85). Per CloudKit JS docs, zoneWide: true searches all zones and zoneID is presumably ignored in that case, but the request is now sending two fields with arguably contradictory intent. Worth a one-line comment (or a test) confirming/pinning down CloudKit's actual precedence, so a future reader doesn't have to rediscover it.
2. CloudKitService+SyncOperations.swift is now 222 lines — 3 lines under the 225-line lint ceiling this PR was explicitly working around elsewhere. The next small addition to that file will likely force another split; might be worth doing preemptively while this PR is already touching the file.
3. Test coverage only exercises the "option set" path. The three new RequestOptions tests assert the fields serialize when provided, but there's no assertion that they're omitted from the JSON body when left nil (the asymmetric nil-omission behavior called out elsewhere in the codebase's FieldValue docs). A quick #expect(body["zoneWide"] == nil)-style negative case would close that gap cheaply.
4. Live round-trip verification against real CloudKit responses is explicitly deferred per the PR description — reasonable given this is request-body-only plumbing, just flagging it's still open per records/changes: add desiredKeys and desiredRecordTypes request options #385.

Security / correctness

No injection or unsafe-serialization concerns — all new values flow through the generated Codable types via typed properties, nothing string-interpolated into URLs/headers. Didn't spot any bugs in the plumbing itself.

Note: I wasn't able to execute swift build/swift test/swiftlint in this review sandbox, so I'm relying on the PR description's stated verification (541 tests passing, swift-format/swiftlint clean) rather than re-confirming it myself.

[claude]

Review: Add documented request options to query/modify/changes (#383 #384 #385)

Reviewed the full diff (openapi.yaml, generated MistKitOpenAPI/Types.swift, CloudKitService extensions, and the MistDemo CLI wiring + tests). Overall this is clean, well-scoped work — solid summary below.

Code quality / best practices

• The CloudKitService+Operations+Deprecated.swift and CloudKitService+RecordWriteConvenience.swift splits are verified pure moves (diffed the removed vs. added blocks byte-for-byte) — good call keeping this PR's diff free of incidental churn while satisfying the 225-line file_length lint. New file sizes (104/159/131/135 lines) are comfortably under the limit.
• All new parameters are optional and default to nil, so existing call sites are untouched — matches the "asymmetric request tagging" philosophy documented in CLAUDE.md (only send what the caller explicitly opts into).
• zoneID.map { Components.Schemas.ZoneID(from: $0) } in modifyRecords follows the exact pattern already used in CloudKitService+ZoneOperations.swift/CloudKitService+AssetOperations.swift — consistent.
• New/moved files carry the correct license header and explicit internal import modifiers per the project's import convention.
• QueryCommand.swift's switch from the deprecated filters: [QueryFilter]?/sortBy: nil call to building a Query(recordType:filters:sortBy:) value with non-optional [] defaults is required by Query's existing initializer signature and is done correctly.

Potential bugs / edge cases

• MistDemoConfiguration.commaSeparatedList(forKey:) (Configuration/MistDemoConfiguration.swift:154-159): if the key is present but the value is an empty string (e.g. --fields "" or CLOUDKIT_FIELDS=), "".split(separator: ",") yields [], not nil. That means desiredKeys/desiredRecordTypes would serialize as an empty array in the request rather than being omitted — CloudKit's desiredKeys: [] semantics ("fetch no fields") differ from "fetch all fields" (omitted). Minor/edge-case, but worth a guard (nil if the trimmed result is empty) if accidental empty flags are a realistic input.
• optionalBool(forKey:) (MistDemoConfiguration.swift:150-152) does two lookups (string(forKey:) then bool(forKey:)) to distinguish "absent" from "explicit false" — correct, but re-resolves the same key twice through ConfigReader. Not a correctness issue, just a minor inefficiency worth a comment explaining why it's not one call (the "why" here — distinguishing nil vs explicit false — is already documented in the doc comment, so this is a non-issue, just flagging for visibility).

Performance

• No concerns — all new parameters are pass-through plumbing (no new network calls, no added iteration/allocation on hot paths). The auto-chunking/pagination loops (fetchAllRecordChanges) correctly thread desiredKeys/desiredRecordTypes through every page request without behavior change to the pagination logic itself.

Security

• No concerns. No new external input parsing beyond existing CLI/env/config patterns; no secrets or credentials touched.

Test coverage

• Good: CloudKitServiceTests.RequestOptions.swift asserts the three request bodies actually serialize zoneWide/numbersAsStrings/zoneID/desiredKeys/desiredRecordTypes via the recording MockTransport — this is the right layer to test (wire format), not just that the Swift call compiles.
• The new MistDemo config tests (FetchChangesConfigTests, ModifyConfigTests, QueryConfigTests+RequestOptions) verify default-nil and explicit-value construction, consistent with the existing test style in that suite.
• Gap (pre-existing pattern, not introduced by this PR): none of the MistDemo config tests exercise the actual .parse(from:) → ConfigReader path (real CLI args/env vars) — they all construct config structs directly. That means optionalBool/commaSeparatedList themselves (and the empty-string edge case above) aren't covered by a test that goes through real argument parsing. Worth a follow-up if/when CLI-level integration tests are added for this module, but not a blocker given the existing test conventions.
• Live round-trip verification of zoneWide/numbersAsStrings/desiredRecordTypes behavior against a real CloudKit container is explicitly called out as deferred in the PR description — reasonable given pluginFields/zoneID/wrappingKey are already flagged out-of-scope in records/changes: add desiredKeys and desiredRecordTypes request options #385.

Nice, tightly-scoped PR. Nothing above blocks merging — the empty-string commaSeparatedList edge case is the only thing I'd consider addressing before merge, and only if empty --fields "" is a realistic user input for this CLI.

[claude]

Review: #412 — Request options for query/modify/changes

Reviewed the diff against v1.0.0-beta.3. Overall this is a clean, well-scoped, additive change that follows the project's established pattern (optional params defaulted to nil, existing call sites unaffected). A few notes below; nothing blocking.

Nice catches

• QueryCommand.swift: switching to the Query-based queryRecords(_:...) overload also wires up desiredKeys: config.fields — the --fields flag previously parsed a value that was never passed to the service call. This silently fixes a pre-existing bug as a side effect of the refactor. Worth calling out explicitly in the PR description/changelog since it's a behavior change beyond the three stated issues (records/query: add zoneWide and numbersAsStrings request options #383/records/modify: add zoneID, desiredKeys, numbersAsStrings request options #384/records/changes: add desiredKeys and desiredRecordTypes request options #385).
• Splitting the deprecated queryRecords overloads and the single-record write convenience methods (createRecord/updateRecord/deleteRecord) into their own files is a clean pure-move to stay under the 225-line file_length SwiftLint threshold — diff confirms no logic changed, just relocated.
• New parameters are consistently inserted before the trailing database: parameter across all three call sites (queryRecords, modifyRecords, fetchRecordChanges/fetchAllRecordChanges), so no existing named-argument call site breaks.

Minor issues

1. MistDemoConfiguration.commaSeparatedList(forKey:) (Configuration/MistDemoConfiguration.swift):

   public func commaSeparatedList(forKey key: String) -> [String]? {
     string(forKey: key)?
       .split(separator: ",")
       .map { String($0).trimmingCharacters(in: .whitespaces) }
   }

   .split(separator:) drops empty subsequences by default, so an explicitly-empty config value (--fields "" or an env var set to "") returns [] rather than nil. That gets forwarded as desiredKeys: [] in the request instead of being omitted — CloudKit may treat an empty desiredKeys array differently from an absent one (e.g., "no fields" vs. "default fields"). Consider mapping [] -> nil if that distinction matters, or add a test for the empty-string case to document the intended behavior either way.

2. MistDemoConfiguration.optionalBool(forKey:):

   public func optionalBool(forKey key: String) -> Bool? {
     string(forKey: key) != nil ? bool(forKey: key) : nil
   }

   This does two separate provider lookups (string then bool) for one logical read. Functionally fine for a demo CLI, but if bool(forKey:default:)'s underlying parser can't parse whatever string is present, it silently falls back to default: false rather than surfacing a parse error — a typo like --zone-wide=oops would quietly resolve to false instead of true. Low risk given this is example/demo code, but worth a comment if that's intentional.

3. Codecov flags CloudKitService+RecordWriteConvenience.swift at 32% patch coverage (21 missing lines). Since this file is a pure move of createRecord/updateRecord/deleteRecord out of CloudKitService+WriteOperations.swift, and the new CloudKitServiceTests.RecordWriteConvenience.swift does exercise all three methods (success + empty-response-throws for create/update, a completion check for delete), this is likely a coverage-attribution artifact of the file rename rather than a real regression — worth a quick look at the Codecov report to confirm the "missing" lines are doc comments/braces and not actual untested branches.

Security / correctness

No concerns — all new fields (zoneWide, numbersAsStrings, zoneID, desiredKeys, desiredRecordTypes) are optional pass-through values serialized by the generated OpenAPI client; no new parsing of untrusted input beyond the existing CLI/config surface, and no manual edits to the generated MistKitOpenAPI sources (openapi.yaml + Types.swift diffs match).

Test coverage

Good: request-body serialization is directly asserted via MockTransport for all three endpoints (CloudKitServiceTests.RequestOptions.swift), and MistDemo config defaulting/carrying is tested for each new flag. Per the PR description, live round-trip confirmation of zoneWide/numbersAsStrings behavior against a real CloudKit response is explicitly deferred — reasonable given the existing "documented but unverified" pattern already used elsewhere in this repo (e.g., pluginFields/wrappingKey).

[claude]

Review: PR #412 — Add documented request options to query/modify/changes

This PR threads three previously-undocumented-in-code CloudKit request-body options (zoneWide, numbersAsStrings on query; zoneID, desiredKeys, numbersAsStrings on modify; desiredKeys, desiredRecordTypes on changes) through openapi.yaml → generated Types.swift → CloudKitService → the MistDemo CLI, plus two pure-move refactors splitting deprecated/convenience methods into new files. The openapi.yaml/Types.swift wiring and the two file-split refactors are clean and behavior-preserving. The issues below are mostly API-completeness gaps where the new options don't fully reach all the places a caller would expect them, plus one pre-existing bug re-exposed by a touched line.

Findings (most severe first):

1. Sources/MistKit/CloudKitService/CloudKitService+RecordWriteConvenience.swift:62,103,129 — createRecord/updateRecord/deleteRecord each call modifyRecords([operation], database: database) without forwarding the new zoneID/desiredKeys/numbersAsStrings params that modifyRecords gained in this same PR (CloudKitService+WriteOperations.swift:78-84). Callers of these convenience wrappers have no way to target a non-default zone or request numbersAsStrings on a single-record write, even though the batch primitive they wrap now supports it. (Confirmed)

2. Examples/MistDemo/Sources/MistDemoKit/Commands/QueryCommand.swift:83 — sortBy: [] is hardcoded when building the Query; config.sort (parsed from --sort field:order, stored in QueryConfig.sort) is never read anywhere in execute(). The --sort CLI flag is silently a no-op. This is exactly the line this PR touched (sortBy: nil → sortBy: []), so it re-exposes a pre-existing gap rather than introducing a new one, but it's on a line this PR modified.

3. Sources/MistKit/CloudKitService/CloudKitService+Operations.swift:85 — queryRecords still hardcodes zoneID: .init(zoneName: "_defaultZone") in the request body, unconditionally, right alongside the new zoneWide: Bool? param (line 90). The two are bolted on independently rather than reconciled — zoneWide: true is sent together with an explicit single-zone zoneID, and queryRecords never gained its own zoneID parameter (unlike modifyRecords, which did), so there's no way to actually target a non-default zone via this method regardless of zoneWide. The new test only asserts zoneWide/numbersAsStrings serialize — it doesn't check what zoneID accompanies them. Live round-trip verification of this option was explicitly deferred per the PR description, so the actual CloudKit-side behavior here is unconfirmed — worth confirming before relying on zoneWide.

4. Sources/MistKit/CloudKitService/CloudKitService+Operations+Deprecated.swift:84-102,142-158 — the two deprecated queryRecords(recordType:...) overloads have no zoneWide/numbersAsStrings params and no way to forward them to the modern overload, so callers still on the deprecated string-based API (one of which this PR's own new test exercises) can't reach the new options at all. Minor since deprecated APIs are conventionally frozen, but worth a note.

5. Examples/MistDemo/Sources/MistDemoKit/Commands/ModifyCommand.swift:106 — config.zone.map { ZoneID(zoneName: $0, ownerName: nil) } doesn't guard against an empty string. --zone "" would produce .some("") and send zoneID: {zoneName: ""} to CloudKit instead of omitting zoneID entirely.

6. Examples/MistDemo/Sources/MistDemoKit/Configuration/MistDemoConfiguration.swift:150-152 — optionalBool(forKey:) does string(forKey: key) != nil ? bool(forKey: key) : nil, walking the full provider hierarchy (CLI → env → .env → defaults) twice per call. Cost is negligible in practice (runs once at CLI startup), but it's inconsistent with the neighboring int(forKey:default:) pattern in the same file, which does a single lookup.

7. Examples/MistDemo/Sources/MistDemoKit/Configuration/MistDemoConfiguration.swift:154-166 — the new commaSeparatedList(forKey:) and the pre-existing filterStrings(forKey:) right below it are near-identical (split/trim/map), differing only in separator (, vs |) and nil-vs-empty-array default. Could share one private helper parameterized by separator.

8. Examples/MistDemo/Sources/MistDemoKit/Commands/QueryCommand.swift:74-75 — the NOTE comment ("Zone, offset, and continuation marker support require enhancements to CloudKitService.queryRecords... Feature: Add pagination support with continuation markers for query results #145, Feature: Add custom CloudKit zone support for queries #146") is now stale: this PR's own diff adds zoneWide to that exact method, and continuationMarker/offset are already threaded through QueryConfig but not consumed here either (ties to Fix "method_lines" issue in Sources/MistKit/MKDatabase.swift #2/Fix "method_complexity" issue in Sources/MistKit/MKDatabase.swift #3).

No blocking correctness issues in the core request-serialization/openapi wiring — the two file-split refactors are byte-for-byte behavior-preserving, and openapi.yaml ↔ generated Types.swift stay consistent in both directions. The main theme above is that several new options don't fully propagate to sibling call sites (convenience wrappers, deprecated overloads) that a caller might reasonably expect to also gain them.

[claude]

Review: Add documented request options to query/modify/changes (#383 #384 #385)

Solid, well-scoped PR: zoneWide/numbersAsStrings on query, zoneID/desiredKeys/numbersAsStrings on modify, and desiredKeys/desiredRecordTypes on changes are all threaded through as trailing optional parameters with correct openapi.yaml → generated-type → CloudKitService plumbing, backed by request-body serialization tests. The two "pure move" file splits (+Operations+Deprecated.swift, +RecordWriteConvenience.swift) are verified byte-for-byte identical to the code they extracted. desiredRecordTypes checks out against cloudkitjs.md's RecordZoneChangesOptions (cited in #385), so it's not a fabricated field.

A few things worth a look before merge:

1. Sources/MistKit/CloudKitService/CloudKitService+Operations.swift:85 — queryRecords always hardcodes zoneID: .init(zoneName: "_defaultZone") regardless of the new zoneWide flag. Nothing gates the zoneID assignment on zoneWide, so a caller passing zoneWide: true still gets an explicit default-zone zoneID in the request body alongside it — the new option may be a silent no-op if CloudKit's server prioritizes an explicit zoneID. Notably, QueryRequestOptionsPhase.swift explicitly documents that it only exercises zoneWide (asserts the request succeeds) without asserting its actual effect, so this wouldn't be caught by CI either way. Worth confirming CloudKit's real precedence and/or omitting zoneID when zoneWide is true.

2. Examples/MistDemo/Sources/MistDemoKit/Configuration/MistDemoConfiguration.swift:150 — optionalBool(forKey:) detects flag presence via string(forKey:) != nil before calling bool(forKey:). Swift Configuration's CommandLineArgumentsProvider docs describe a bare --flag as "a Boolean flag, treated as true" — i.e., likely stored as a native .bool ConfigContent, not a .string. If string(forKey:) doesn't coerce across types, a bare --zone-wide/--numbers-as-strings flag could silently fail the presence check and never reach bool(forKey:). None of the new tests exercise this through the real CLI-parsing path (they construct QueryConfig/ModifyConfig directly), so this gap wouldn't surface in CI. Worth double-checking against the actual swift-configuration behavior, or switching to a direct bool(forKey:)-without-default check if one exists.

3. Examples/MistDemo/Sources/MistDemoKit/Configuration/MistDemoConfiguration.swift:154-159 — commaSeparatedList(forKey:) returns [], not nil, for an empty-string value, since "".split(separator: ",") drops empty subsequences by default. A user passing --fields "" (or CLOUDKIT_FIELDS="") intending "no restriction" instead gets desiredKeys: [], which CloudKit treats as "return no fields" — the opposite of the likely intent. (This mirrors a pre-existing equivalent gap in QueryConfig+Parsing.swift's inline --fields handling, so it's not strictly new, but the new helper reproduces it for desiredKeys/desiredRecordTypes too.)

4. Sources/MistKit/CloudKitService/CloudKitService+RecordWriteConvenience.swift — createRecord/updateRecord/deleteRecord don't forward the new zoneID/desiredKeys/numbersAsStrings options when calling modifyRecords([operation], database:). This looks like an intentional scope boundary given the "pure move" framing, but it does mean a caller wanting e.g. a non-default zoneID on a single createRecord currently has no wrapper for it and must drop to modifyRecords directly — worth a one-line doc comment if that's deliberate.

5. Minor duplication: commaSeparatedList(forKey:) (new) and the pre-existing inline comma-split-and-trim logic in QueryConfig+Parsing.swift:101-106 implement the same rule for the same --fields key without being consolidated — the PR added a reusable helper but left the older call site on its own inline copy, so the two can drift (e.g. a fix to empty-string handling per point 3 would need to land in both places).

6. Convention nit: Examples/MistDemo/Sources/MistDemoKit/Integration/Phases/ChangesRequestOptionsPhase.swift declares private static func exercise (type method) after internal func run (instance method), which is out of order under SwiftLint's default type_contents_order (type methods before instance methods) — CLAUDE.md pins this repo to swiftlint's default ordering.

Nothing structural — these are all worth a look but don't block the core feature addition, which is clean and well-tested for the request-serialization surface it targets.

🤖 Generated with Claude Code