Cards: give the plain update name to a merge-safe composite

[jeremy]

Closes #467. Stacked on #488 → #487 → main.

BC3 builds a card's update params as { due_on: nil }.merge(card_params) (kanban/cards_controller.rb), so any update whose body omits due_on erases the card's due date. A sparse PUT — the natural thing to write, and what every generated SDK produced — silently destroyed data.

The shape

The plain name goes to the safe thing:

	update (composite)	updateVerbatim (raw)
dueOn unaddressed	GET, then resend the fetched date	BC3 clears it — the bug, honestly exposed
dueOn explicitly empty/null	clear (one PUT, no GET)	n/a
dueOn a date	set (one PUT, no GET)	set

The extra GET is paid for only when the caller left dueOn alone — the one case where the API would otherwise destroy something.

The composite deliberately does not resend everything. BC3 filters incoming assignee IDs through reachable_people, so echoing assignees back would silently unassign anyone who has since lost board access. Only the caller's own title/content/assignee_ids go out, plus due_on.

Clear is encoded by omission, not by null

The plan called for Go to send {"due_on": null} with a new SPEC exception. That doesn't work, and the recon that found it changed the design:

• SPEC.md §18 forbids it outright.
• Five of six SDKs strip nulls structurally before the wire — Python _compact, Ruby compact_params, Kotlin ?.let, TypeScript's JSON.stringify dropping undefined, Swift encodeIfPresent. One shared conformance fixture could not assert null-for-Go and absent-for-everyone-else.
• due_on: "" was the fallback, but SPEC.md §5 already records that for the Todos sibling "" is a date-format error.

Omission is correct and universally expressible: BC3 nils an omitted due date, which is exactly what a caller asking to clear wants. So no SPEC exception was needed — §18 instead gained a note that body compaction is not relaxed for composites. ""/null stays the caller-facing sentinel; omission is the wire encoding.

Naming, and why the TS type moved

Renaming the generated TS method also renames its request interface, because the TS generator derives interface names from the method name: UpdateCardRequest → UpdateVerbatimCardRequest. Rather than fight that, the composite claims the plain name — and it earns it, because the two types genuinely differ: the composite's dueOn is string | null (it can express "clear"), the verbatim one's cannot. So UpdateCardRequest keeps working for existing callers and is strictly wider than before; UpdateVerbatimCardRequest is additive.

Kotlin (UpdateCardBody), Swift (schema-derived UpdateCardRequest), Python and Ruby (kwargs) are unaffected.

Presence detection is language-native: Go *string, TS string | null, Ruby/Python nil/None with "" to clear, Kotlin nullable with "" to clear, and Swift a DueDate enum (.preserve / .clear / .on) — an optional cannot carry three states, and .preserve being the default means the safe thing is what you get by saying nothing.

Conformance — two cases, distinct dispatch identities

conformance/tests/cards_write.json, run by all five executable runners:

• UpdateCard → requestCount: 2, GET@0, PUT@1, due_on resent, assignee_ids/content absent.
• UpdateCardVerbatim → exactly one PUT, no GET, due_on absent.

The second case is the load-bearing one. Without it, later generator drift could turn both public methods into composite behaviour and nothing would notice. Swift isn't in make conformance, so CardsServiceExtensionsTests mirrors both plus the clear/set paths.

Deviation from the plan, stated plainly

The plan asked for a field-specific enhancer pass pointerizing all three UpdateCardRequestContent members. I skipped it, because it buys no wire behaviour: Go's composite goes through the map[string]any + UpdateCardWithBodyWithResponse path (sanctioned by §18 rule 1), which the typed generated body never touches. Pointerizing would also not have fixed the thing it was aimed at — AssigneeIds []int64 keeps omitempty regardless, so an empty non-nil slice would still be dropped, breaking the already-shipped "clear assignees" contract. If the intent was SPEC §10 static-typing compliance rather than behaviour, that's worth its own issue.

Tests

Go: two existing tests moved to UpdateVerbatim (they encoded verbatim semantics); six new composite tests — preservation, explicit clear, explicit date, explicit empty content, explicit empty assignees. TypeScript and Ruby unit tests likewise split across both paths. The TS one had been failing only because MSW had no GET handler — which is precisely the read-before-write being added.

make check exits 0 (verified exit code).

---

Summary by cubic

Give the plain cards.update name to a merge-safe composite across all SDKs. It preserves due_on when you omit it and keeps the raw single-PUT path as updateVerbatim (fixes #467).

• New Features

  • update now GETs the card and resends due_on only when you didn’t set it; it does not echo assignee_ids or content — only your fields plus due_on are sent.
  • The raw single-PUT method is renamed to updateVerbatim in all SDKs; the composite owns the plain update name.
  • TypeScript wires CardsService through services/cards-extensions, exports UpdateVerbatimCardRequest, and keeps UpdateCardRequest for the composite.
  • Swift adds a DueDate enum (.preserve/.clear/.on); Go’s UpdateCardRequest scalars are presence-bearing pointers; Ruby/Python/Kotlin expose update as the composite and keep update_verbatim/updateVerbatim for the raw path.
  • Conformance adds UpdateCard (composite) and UpdateCardVerbatim (raw) cases; fixtures cover preserve/clear/empty; SPEC documents the merge-safe surface and that body compaction stays strict for composites.

• Migration

  • Existing update calls now do a read-before-write. Use updateVerbatim to keep the old single-request behavior.
  • To clear the due date: TypeScript dueOn: null; Go DueOn: ptr(""); Ruby/Python due_on: ""; Kotlin dueOn = ""; Swift .clear.

^{Written for commit c21e6a8. Summary will update on new commits.}

[Copilot]

Pull request overview

This PR makes Cards.update merge-safe across all SDKs by turning it into a read-before-write composite that preserves due_on when callers omit it (preventing silent due-date data loss), while keeping the raw single-request behavior available as updateVerbatim. It also adds conformance coverage to ensure both behaviors remain distinct over time.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Changes:

• Rename the generated “raw” update method to updateVerbatim (via per-language method naming overrides) and introduce a hand-written merge-safe update composite layered on top.
• Add/update conformance + unit tests to assert the composite does GET→PUT (when due_on is unaddressed) and verbatim does single PUT.
• Document the Cards merge-safe write surface and composite rules in SPEC.md / AGENTS.md.

Reviewed changes

Copilot reviewed 29 out of 36 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
typescript/tests/services/cards.test.ts	Splits tests between updateVerbatim and merge-safe update behavior.
typescript/src/services/cards-extensions.ts	Adds TS merge-safe CardsService.update composite + request type.
typescript/src/index.ts	Re-exports composite CardsService and composite UpdateCardRequest; exports verbatim request type separately.
typescript/src/generated/services/cards.ts	Renames generated method to updateVerbatim and updates request type name.
typescript/src/client.ts	Wires the client to use the composite Cards service implementation.
typescript/scripts/generate-services.ts	Adds method-name override so UpdateCard generates as updateVerbatim.
swift/Tests/BasecampTests/CardsServiceExtensionsTests.swift	Adds Swift mirror tests for composite vs verbatim semantics.
swift/Sources/BasecampGenerator/MethodNaming.swift	Adds method-name override so generated Swift uses updateVerbatim.
swift/Sources/Basecamp/Generated/Services/CardsService.swift	Renames generated Swift method to updateVerbatim.
swift/Sources/Basecamp/CardsServiceExtensions.swift	Adds Swift merge-safe update extension + tri-state DueDate.
SPEC.md	Documents Cards merge-safe write surface + composite invariants (incl. verbatim reachability).
ruby/test/basecamp/services/cards_service_test.rb	Splits Ruby tests between verbatim and merge-safe update paths.
ruby/scripts/generate-services.rb	Adds method-name override so generated Ruby uses update_verbatim.
ruby/lib/basecamp/services/cards_extensions.rb	Adds Ruby merge-safe update via prepend extension.
ruby/lib/basecamp/generated/services/cards_service.rb	Renames generated method to update_verbatim and updates operation identity.
ruby/lib/basecamp.rb	Prepends Cards extension onto generated CardsService on load.
python/src/basecamp/services/cards.py	Adds sync/async merge-safe update services layered over generated verbatim.
python/src/basecamp/generated/services/cards.py	Renames generated methods to update_verbatim (sync + async).
python/src/basecamp/client.py	Wires sync client to the composite Cards service.
python/src/basecamp/async_client.py	Wires async client to the composite Cards service.
python/scripts/generate_services.py	Adds method-name override so generated Python uses update_verbatim.
kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/services/CardsService.kt	Adds Kotlin merge-safe update composite subclass.
kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/services/cards.kt	Makes generated CardsService extensible and renames raw method to updateVerbatim.
kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/ServiceAccessors.kt	Points the accessor to the hand-written composite Cards service.
kotlin/generator/src/main/kotlin/com/basecamp/sdk/generator/Config.kt	Declares Cards as extensible + adds method-name override for UpdateCard.
kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/Main.kt	Adds dispatch for UpdateCard composite vs UpdateCardVerbatim.
go/templates/client.tmpl	Renames Go generated wrapper methods to UpdateVerbatim* for UpdateCard.
go/pkg/generated/client.gen.go	Applies generated rename to UpdateVerbatim* methods.
go/pkg/basecamp/cards.go	Adds merge-safe Update composite + UpdateVerbatim raw method; pointerizes request fields for tri-state semantics.
go/pkg/basecamp/cards_test.go	Adjusts existing tests and adds new merge-safe update tests.
conformance/tests/cards_write.json	Adds conformance cases for composite UpdateCard and raw UpdateCardVerbatim.
conformance/runner/typescript/runner.test.ts	Dispatches new Cards update operations for TS conformance runner.
conformance/runner/ruby/runner.rb	Dispatches new Cards update operations for Ruby conformance runner.
conformance/runner/python/runner.py	Dispatches new Cards update operations for Python conformance runner.
conformance/runner/go/main.go	Dispatches new Cards update operations for Go conformance runner.
AGENTS.md	Updates architecture docs to include Cards composite alongside Todos composites.

Comments suppressed due to low confidence (1)

conformance/runner/go/main.go:569

• Same presence-vs-empty-string issue as the UpdateCard case: using getStringParam(...) != "" prevents conformance cases from expressing explicit empty strings for presence-bearing request fields. Use map key presence to decide whether to set the pointer.

		if v := getStringParam(tc.RequestBody, "title"); v != "" {
			req.Title = &v
		}
		if v := getStringParam(tc.RequestBody, "content"); v != "" {
			req.Content = &v

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[cubic-dev-ai]

6 issues found across 36 files

_{You’re at about 92% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.}

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="swift/Sources/Basecamp/CardsServiceExtensions.swift">

<violation number="1" location="swift/Sources/Basecamp/CardsServiceExtensions.swift:1">
P3: This new file has an unused `Foundation` import; removing it keeps the extension's dependencies minimal and avoids an unnecessary import warning in strict Swift builds.</violation>
</file>

<file name="kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/services/CardsService.kt">

<violation number="1" location="kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/services/CardsService.kt:59">
P2: The composite call currently emits the same `UpdateCard` operation identity as the raw endpoint, so hooks and conformance dispatch cannot distinguish `UpdateCard` (GET then PUT) from `UpdateCardVerbatim` (PUT only). Preserving the separate dispatch identity in the generated verbatim operation would keep the two paths observable as documented.</violation>
</file>

<file name="SPEC.md">

<violation number="1" location="SPEC.md:1300">
P3: Incomplete description of TypeScript null stripping: `JSON.stringify` drops `undefined` keys but preserves `null` values — it does not by itself strip null from output. Since the TypeScript composite type `dueOn?: string | null` accepts `null` as a clear signal, the implementation must pre-process null to undefined (or use a replacer) before calling `JSON.stringify`. The spec lists the serialization step alone, which is insufficient to produce the required wire omission for null inputs.</violation>
</file>

<file name="conformance/runner/typescript/runner.test.ts">

<violation number="1" location="conformance/runner/typescript/runner.test.ts:232">
P3: Field-mapping logic for `title`, `content`, and `assignee_ids` is duplicated between `UpdateCard` and `UpdateCardVerbatim`. Consider extracting a shared helper (similar to `mapTodoWireFields`) to keep mappings consistent and reduce drift risk between the two paths.</violation>

<violation number="2" location="conformance/runner/typescript/runner.test.ts:239">
P2: Unsafe `as number[]` type assertion on `body.assignee_ids`. Since `body` is `Record<string, unknown>`, this cast silently trusts the runtime type — if fixture data ever carries string IDs (e.g., `["1", "2"]`) the API receives strings where numbers are expected. Prefer reading it as a conversion: `(body.assignee_ids as number[]).map(Number)`, or validate the type before passing.</violation>
</file>

<file name="python/scripts/generate_services.py">

<violation number="1" location="python/scripts/generate_services.py:155">
P1: Override maps UpdateCard (composite path) to `update_verbatim`, which inverts the intent stated in the comment and PR description. The composite path should get the plain `update` name; the raw verbatim path should get `update_verbatim`. Update the override to `"UpdateCard": "update"` and add `"UpdateVerbatimCard": "update_verbatim"` for the other path.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: Override maps UpdateCard (composite path) to update_verbatim, which inverts the intent stated in the comment and PR description. The composite path should get the plain update name; the raw verbatim path should get update_verbatim. Update the override to "UpdateCard": "update" and add "UpdateVerbatimCard": "update_verbatim" for the other path.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At python/scripts/generate_services.py, line 155:

<comment>Override maps UpdateCard (composite path) to `update_verbatim`, which inverts the intent stated in the comment and PR description. The composite path should get the plain `update` name; the raw verbatim path should get `update_verbatim`. Update the override to `"UpdateCard": "update"` and add `"UpdateVerbatimCard": "update_verbatim"` for the other path.</comment>

<file context>
@@ -150,6 +150,9 @@
     "UpdateCardStep": "update",
+    # The plain `update` name belongs to the merge-safe composite; the raw
+    # single-PUT path keeps a name that says what it does. See #467.
+    "UpdateCard": "update_verbatim",
     "SetCardStepCompletion": "set_completion",
     "GetQuestionnaire": "get_questionnaire",
</file context>

[cubic-dev-ai]

P2: The composite call currently emits the same UpdateCard operation identity as the raw endpoint, so hooks and conformance dispatch cannot distinguish UpdateCard (GET then PUT) from UpdateCardVerbatim (PUT only). Preserving the separate dispatch identity in the generated verbatim operation would keep the two paths observable as documented.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/services/CardsService.kt, line 59:

<comment>The composite call currently emits the same `UpdateCard` operation identity as the raw endpoint, so hooks and conformance dispatch cannot distinguish `UpdateCard` (GET then PUT) from `UpdateCardVerbatim` (PUT only). Preserving the separate dispatch identity in the generated verbatim operation would keep the two paths observable as documented.</comment>

<file context>
@@ -0,0 +1,69 @@
+            dueOn.isEmpty() -> null
+            else -> dueOn
+        }
+        return updateVerbatim(
+            cardId,
+            UpdateCardBody(
</file context>

[cubic-dev-ai]

P2: Unsafe as number[] type assertion on body.assignee_ids. Since body is Record<string, unknown>, this cast silently trusts the runtime type — if fixture data ever carries string IDs (e.g., ["1", "2"]) the API receives strings where numbers are expected. Prefer reading it as a conversion: (body.assignee_ids as number[]).map(Number), or validate the type before passing.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At conformance/runner/typescript/runner.test.ts, line 239:

<comment>Unsafe `as number[]` type assertion on `body.assignee_ids`. Since `body` is `Record<string, unknown>`, this cast silently trusts the runtime type — if fixture data ever carries string IDs (e.g., `["1", "2"]`) the API receives strings where numbers are expected. Prefer reading it as a conversion: `(body.assignee_ids as number[]).map(Number)`, or validate the type before passing.</comment>

<file context>
@@ -227,6 +227,32 @@ async function executeOperation(
+            ? { dueOn: body.due_on === "" ? null : String(body.due_on) }
+            : {}),
+          ...(body.assignee_ids !== undefined
+            ? { assigneeIds: body.assignee_ids as number[] }
+            : {}),
+        });
</file context>

[cubic-dev-ai]

P3: This new file has an unused Foundation import; removing it keeps the extension's dependencies minimal and avoids an unnecessary import warning in strict Swift builds.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At swift/Sources/Basecamp/CardsServiceExtensions.swift, line 1:

<comment>This new file has an unused `Foundation` import; removing it keeps the extension's dependencies minimal and avoids an unnecessary import warning in strict Swift builds.</comment>

<file context>
@@ -0,0 +1,73 @@
+import Foundation
+
+/// A merge-safe `update` on top of the generated `CardsService` surface
</file context>

[cubic-dev-ai]

P3: Incomplete description of TypeScript null stripping: JSON.stringify drops undefined keys but preserves null values — it does not by itself strip null from output. Since the TypeScript composite type dueOn?: string | null accepts null as a clear signal, the implementation must pre-process null to undefined (or use a replacer) before calling JSON.stringify. The spec lists the serialization step alone, which is insufficient to produce the required wire omission for null inputs.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At SPEC.md, line 1300:

<comment>Incomplete description of TypeScript null stripping: `JSON.stringify` drops `undefined` keys but preserves `null` values — it does not by itself strip null from output. Since the TypeScript composite type `dueOn?: string | null` accepts `null` as a clear signal, the implementation must pre-process null to undefined (or use a replacer) before calling `JSON.stringify`. The spec lists the serialization step alone, which is insufficient to produce the required wire omission for null inputs.</comment>

<file context>
@@ -1264,9 +1290,14 @@ All wire operations are generated (rubric 1A.6). One narrow exception is sanctio
+- **Cards** `update` (merge-safe) — see §5 "Merge-Safe Write Surface (Cards)". The raw path is `updateVerbatim`.
 
-Current composites: Todos `update` (merge-safe) and `edit` (read-modify-write) — see §5 "Merge-Safe Write Surface (Todos)".
+**Body compaction is not relaxed for composites.** A composite never sends `{"field": null}` to express "clear" (§18 rule). Where the server treats an omitted key as a clear — as BC3 does for `due_on` — omission *is* the clear encoding, and it is the only one all six SDKs can express identically: five strip nulls structurally before the wire (Python `_compact`, Ruby `compact_params`, Kotlin `?.let`, TypeScript's `JSON.stringify` dropping `undefined`, Swift `encodeIfPresent`).
 
 ---
</file context>

[cubic-dev-ai]

P3: Field-mapping logic for title, content, and assignee_ids is duplicated between UpdateCard and UpdateCardVerbatim. Consider extracting a shared helper (similar to mapTodoWireFields) to keep mappings consistent and reduce drift risk between the two paths.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At conformance/runner/typescript/runner.test.ts, line 232:

<comment>Field-mapping logic for `title`, `content`, and `assignee_ids` is duplicated between `UpdateCard` and `UpdateCardVerbatim`. Consider extracting a shared helper (similar to `mapTodoWireFields`) to keep mappings consistent and reduce drift risk between the two paths.</comment>

<file context>
@@ -227,6 +227,32 @@ async function executeOperation(
 
+      case "UpdateCard":
+        // Merge-safe composite: GET then PUT, resending the fetched due_on.
+        await client.cards.update(Number(params.cardId), {
+          ...(body.title !== undefined ? { title: String(body.title) } : {}),
+          ...(body.content !== undefined ? { content: String(body.content) } : {}),
</file context>

[Copilot]

Pull request overview

Copilot reviewed 31 out of 38 changed files in this pull request and generated no new comments.

[jeremy]

Addressed the proof-matrix gap, and it turned up the runner defect you predicted.

The matrix is now complete

cards_write.json had only preservation and raw-omission. Added three cases, all five runners green:

case	asserts
update-explicit-clear	1 request, no GET, due_on absent
update-explicit-empty-content	content present and ""
update-explicit-empty-assignees	assignee_ids present and []

The Go runner really did collapse empty to absent

As you said, adding the empty-content case exposed it. The runner built the request with

if v := getStringParam(tc.RequestBody, "content"); v != "" {

which is precisely the distinction the pointer fields exist to preserve — an explicit clear was indistinguishable from an omission. Replaced with a presence-based cardUpdateRequest helper shared by both dispatch arms.

Verified the new case bites. Reintroducing the non-emptiness test:

FAIL: update-explicit-empty-content: an explicit empty content is sent, not dropped
      Expected request body field "content" on request index 0, but it was absent

Python and Kotlin now have direct service tests

Both previously relied on the shared fixture alone. python/tests/services/test_cards.py (8 tests, sync and async) and kotlin/.../CardsServiceTest.kt (5 tests) cover preservation, explicit clear, explicit date, explicit empty content/assignees, and verbatim's single PUT.

comment_count

Dropped from the write fixture's mock bodies. It is worth being precise about what it is: not purely fictional — it is absent from the Card schema (which has comments_count) but is modeled on the hand-written Go wrapper (go/pkg/basecamp/cards.go:51,105). So it is a wrapper/spec divergence. It has no place in a write-semantics fixture either way; the three source card fixtures still carry it and I've left those alone as a separate finding rather than fold an unrelated cleanup in here.

The plan now agrees with the code

You were right that leaving the closeout and the controlling plan disagreeing is not acceptable. handoff-…-cryptic-scott.md §Phase 5 is amended rather than silently diverged from: the enhancer pass is marked DESCOPED with the reason (nothing constructs the typed body, and it would not have fixed AssigneeIds anyway, which keeps omitempty regardless and would have broken the shipped clear-assignees contract), and items 2–4 are marked REVISED for the omission-not-null encoding.

make check exits 0. Rebased onto the restacked #488.

[Copilot]

Pull request overview

Copilot reviewed 31 out of 38 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 31 out of 38 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 31 out of 38 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

ruby/lib/basecamp/services/cards_extensions.rb:55

• In the due_on.nil? (preserve) branch, the fetched due_on is forwarded as-is. If the API ever returns an empty string, this will resend "" on the wire (or at least attempt to), which contradicts the comment just below about "" risking a date-format error. Consider normalizing a blank fetched value to nil so preserve never emits an empty date.

        resolved_due_on =
          if due_on.nil?
            get(card_id: card_id)["due_on"]
          elsif due_on.to_s.empty?