Skip to content

GetUpcomingSchedule renders reduced partials, so give it reduced shapes (#635, #641, #644) - #648

Merged
jeremy merged 7 commits into
mainfrom
feat/upcoming-schedule-projection
Aug 4, 2026
Merged

jeremy merged 7 commits into
mainfrom
feat/upcoming-schedule-projection

Conversation

@jeremy

@jeremy jeremy commented Aug 4, 2026 •

Copy link
Copy Markdown
Member

Closes #635. Closes #641. Closes #644.

GetUpcomingSchedule declared the shared ScheduleEntry and a half-modelled Assignable, but BC3 renders the report through purpose-built calendar partials that write their own keys instead of rendering recordings/_recording. The published contract promised fields the endpoint does not send.

Two commits: the projection (#635 + #641), then the empty response examples (#644).

The failure, stated precisely

Any populated window — a response carrying at least one schedule entry, recurring occurrence, or assignable. Not "every live response": an empty three-array envelope decodes fine on any contract, and a call with no window is a bodiless BC3 400 before a body exists.

Red proof, run against main in a throwaway detached worktree. swift test exit 1, 4 failures and 1 pass:

Test Case '-[BasecampTests.UpcomingScheduleRedProofTests testWindowWithOneScheduleEntryDecodes]' started.
/private/tmp/.../red-main/swift/Sources/Basecamp/Services/BaseService.swift:76: error: -[BasecampTests.UpcomingScheduleRedProofTests testWindowWithOneScheduleEntryDecodes] : failed: caught error: "DecodingError.keyNotFound: Key 'type' not found in keyed decoding container. Path: scheduleEntries[0].bucket. Debug description: No value associated with key CodingKeys(stringValue: "type", intValue: nil) ("type")."
Test Case '-[BasecampTests.UpcomingScheduleRedProofTests testWindowWithOneScheduleEntryDecodes]' failed (0.027 seconds).
	 Executed 5 tests, with 4 failures (4 unexpected) in 7.523 (7.525) seconds

The other three failures are the same error at recurringScheduleEntryOccurrences[0].bucket, assignables[0].bucket, and on the fully-populated envelope. testEmptyWindowDecodes passed — that is the half that pins the claim.

bucket.type is the first key a strict decoder reaches, ahead of all six top-level members the calendar entry partial drops.

Per-SDK behaviour before this change

SDK Before
Swift Threw. ReportsService returns the typed content; BaseService decodes strictly and rethrows unwrapped.
Kotlin Did not throw, and could not. reports.kt returned JsonElement and decoded decodeFromString<JsonElement>, so no contract was enforced on this operation at all.
TypeScript Typed the members non-optional — a compile-time lie, undefined at runtime.
Go, Python, Ruby Lenient: zero value / absent key.

Commit 1 — the projection (#635, #641)

Six dedicated shapes, named after the owning report the way Draft* and MyAssignment* already are: UpcomingScheduleEntry, UpcomingAssignable, UpcomingScheduleBucket, UpcomingSchedulePerson, UpcomingAssignableParent, UpcomingAssignableCompletion.

A reduced projection is not a subset. Verified key by key against the partials:

schedules/calendar/_entry.json.jbuilder — relative to ScheduleEntry it drops created_at, updated_at, title, inherits_status, parent, description_attachments, description, bookmark_url, subscription_url, comments_url, join_url, highlighted, boosts_count, boosts_url; narrows bucket to id + name; and adds recurring, which no other schedule-entry projection emits. Every key is unconditional — there is no if in the partial — so every member is @required.

schedules/calendar/_assignable.json.jbuilder — the assignable half was under-modelled and wrong, and a ScheduleEntry-only fix would have left Swift throwing:

  • bucket omits required type — the same nested omission
  • parent omits required app_url, type, url — it is {id, title} only, and the title is todolist_or_group_title, which folds the grandparent list title in as "List: Group"
  • BC3 emits content; the SDK modelled title — the one field callers actually want was permanently absent, while a key that is always present went unmodelled
  • eight further always-present keys were unmodelled: status, visible_to_clients, completion_url, completed, repeating, comments_count, plus the partial's one conditional key, the completion block
  • type is the lowercase short recordable name (todo, card, step), not the CamelCase type other projections carry
  • completion_url is only absolute on the to-do branch: BC3 renders bucket_step_completions_path for everything else, and a _path helper emits no host

recurring is modelled, and it discriminates the two envelope arrays: BC3 selects schedule_entries with recurrence_schedule IS NULL and recurring_schedule_entry_occurrences with it NOT NULL.

All three envelope arrays are @required — reports/schedules/upcoming/index.json.jbuilder writes all three keys unconditionally, so an empty window is three empty arrays rather than a missing key.

Both window parameters are @required. Reports::Schedules::UpcomingController#index calls date_range_from_params, whose date_from_params does Date.strptime(params.require(name).to_s, …); the controller rescues ActionController::ParameterMissing and Date::Error into head :bad_request. An unbounded call has never worked. BadRequestError joins the operation's error list because a malformed-but-present date still 400s.

Go returns the reduced types through aliases (type UpcomingScheduleEntry = generated.UpcomingScheduleEntry, …) instead of converting them into full ScheduleEntry/Assignable values. Converting is what hid the mismatch: it compiles happily while zero-filling every field the endpoint never sends, so a missing created_at reads back as "" and nobody learns the response never carried it. Aliasing removes the conversion, so the spec is the only place the shape is stated.

Kotlin now returns the typed projection. The generator's findUnderlyingEntitySchema resolves one entity per response, so an object-of-arrays envelope had no single answer and fell through to JsonElement. A new opt-in branch (TYPED_ARRAY_ENVELOPE_OPERATIONS) emits a @Serializable result class when every property of the response object is an array whose items $ref a known entity; the emitter falls back to JsonElement rather than emitting something wrong if that stops holding. GetOverdueTodos also satisfies the structural test today and is deliberately not opted in — retyping is a Kotlin source break, and its renderer has not been verified.

#641 rides along, with three members rather than the two in its title: url, highlighted and status, all permitted by Schedules::Entries::BaseController#base_schedule_entry_params / Recording::StatusParam#status_param (which on create accepts drafted|active|archived|trashed). The read/write spelling split is preserved exactly: the join link is url on write and join_url on read, because recordings/_recording claims the url key for the recording's own API URL and the entry partial renders after it. Sending join_url on write is silently dropped by strong parameters.

Conformance — why this shipped

grep -rn "upcoming\|UpcomingSchedule" conformance/ exited 1. Zero coverage. The only two tests that touched the endpoint were worse than none: ruby/test/basecamp/services/reports_service_test.rb stubbed {"entries": [...]} — a top-level key BC3 has never sent — and asserted only that a Hash came back.

Added conformance/tests/upcoming_schedule.json: four cases (normal entry, recurring occurrence, assignable, empty envelope), dispatched independently by all six runners, with bodies read out of the partials. Each runner returns a flat summary of the decoded envelope, so the assertions read values back through each SDK's own model rather than off the wire body; Go and TypeScript resolve a responseBody path as a top-level key only, which is why the summary is flat rather than nested.

Also added: spec/fixtures/schedules/upcoming.json under the check-fixture-coverage manifest (validated against the generated schema, with element-level targets for all six new components), the corrected Ruby test, and new tests in Go, Swift, Kotlin, TypeScript and Python.

Two bugs found on the way, both fixed here:

  • conformance/runner/ruby/runner.rb's dig_path did current[key] || current[key.to_sym], so a present false read as a miss and fell through to nil. A responseBody assertion on a boolean field could never have seen false.
  • Go typed UpcomingScheduleEntry.starts_at/ends_at as time.Time, which cannot parse the bare date an all-day entry sends. Added to the FlexibleTime pass alongside ScheduleEntry.

Commit 2 — response examples that contradict their own schemas (#644)

Under a validator that checks each projected example against its projected schema, 7 of 10 fail on main — not the 8 the issue names:

FAIL ListRecordings 200 ListRecordings_example1: {} is not of type 'array' at []
FAIL ListRecordings 200 ListRecordings_example2: {} is not of type 'array' at []
FAIL ListRecordings 200 ListRecordings_example3: {} is not of type 'array' at []
FAIL UpdateSubscription 200 UpdateSubscription_example1: 'count' is a required property at []
FAIL UpdateSubscription 200 UpdateSubscription_example2: 'count' is a required property at []
FAIL GetTodolistOrGroup 200 GetTodolistOrGroup_example1: 'app_url' is a required property at []
FAIL GetTodolistOrGroup 200 GetTodolistOrGroup_example2: 'app_url' is a required property at []
checked=10 failures=7

Two corrections to the issue's premise, both load-bearing for #638:

  1. The GetTodolistOrGroup pair fails too. Its example value is {"result": {…}} against a schema that is a bare Todolist. So the "2 currently validated" figure counts the two that are in the failure list.
  2. The three UpdateProjectAccess examples pass, vacuously. ProjectAccessResult declares no required members, so {} satisfies it. Those are the 3 absent from the failure list.

Declaring output is not sufficient, which is the other half of the correction. The converter emits the Smithy output node verbatim, and restJson1 forces that node to be the wrapper structure. Adding an output to UpdateSubscription alone produced {"subscription": {…}} against a bare Subscription schema — trading an empty-object contradiction for a wrongly-wrapped one.

So the fix has two parts:

  • BareResponseExampleMapper mirrors BareObjectResponseMapper/BareArrayResponseMapper onto the examples. Those two rewrite only components.schemas; the examples live under paths and nobody unwrapped them. It reuses their own shouldTransform predicates so the three cannot drift on which schemas count as wrapped, and runs first (order 90) because the wrapper property name is only readable while the schema is still wrapped. It also drops an example with nothing to unwrap rather than publishing {}, so declaring input-only @examples stays legitimate and simply stops emitting a response example.
  • Faithful outputs for ListRecordings, UpdateProjectAccess and UpdateSubscription. TrashRecording is parameters-only and needs neither.

After: checked=10 failures=0.

Two consequences worth naming:

  • The jsonAdd pointers injecting Todolist.color into the GetTodolistOrGroup examples now end /value/color, not /value/result/color. jsonAdd is applied after the mappers, so a pointer still naming result rebuilds the wrapper that was just removed — as a sibling object holding only color, with no color on the body. Caught by observation, not by reasoning; the comment in examples.smithy records it.
  • spec/smithy-bare-arrays's own tests could not run. The project targets Java 11 and asked for JUnit 6, which publishes only a JVM-17 variant, so ./gradlew test failed at compileTestJava and nothing reported it — make check only ran publishToMavenLocal. Three test cases had drifted out of agreement with the mappers in the meantime (they still asserted an operation-name prefix rule and an inline-object case the mappers stopped honouring). Pinned to JUnit 5, corrected those three, and wired smithy-mapper-test into check-targets so it cannot go quiet again.

Verification

  • make check green, with REAL_EXIT=0 grepped out of a captured log and PRE_SHA == POST_SHA around the run.
  • All six SDK suites and all six conformance runners execute the four new cases. Swift and Kotlin genuinely executed, not UP-TO-DATE.
  • Operation count re-derived from behavior-model.json: 247, unchanged. This PR adds shapes and members, not operations.

Deliberately not in this PR

ScheduleEntry.join_url / .highlighted should now be @required. They were optional because GetUpcomingSchedule shared the shape and rendered the reduced partial. That reason is retired here, so both are now under-modelled rather than correctly modelled: every operation that still returns ScheduleEntry renders schedules/entries/_entry, which emits both unconditionally. Tightening them (join_url required-and-nullable, highlighted plain required) changes five other operations and forces every inline schedule-entry stub across six SDKs to carry the keys, so it belongs in its own reviewable diff. The member docs and SPEC §5 now say this outright instead of restating the retired reason.

Three other instances of the same defect class, found by the sweep #635's closing instruction asked for. Reported separately rather than absorbed — none is the same shape family.


Summary by cubic

Fixes GetUpcomingSchedule to return BC3’s reduced calendar projections and makes all SDKs decode typed results; also unwraps and corrects response examples to match their schemas. Addresses #635, #641, and #644.

  • Refactors

    • Added UpcomingScheduleEntry, UpcomingAssignable, and related types; envelope arrays are required; entries include recurring and a reduced bucket.
    • GetUpcomingSchedule now requires window_starts_on and window_ends_on.
    • Updated SDKs to use the new shapes and enforce required params: Swift (typed Upcoming*), Kotlin (UpcomingScheduleResult for array-envelope responses), Go (aliases for reduced shapes; starts_at/ends_at use types.FlexibleTime), TypeScript/Python/Ruby (method signatures updated).
    • CreateScheduleEntry accepts url, highlighted, and status (write url; read join_url).
    • Unwrapped response examples via BareResponseExampleMapper and added faithful outputs; fixed example pointers and enabled mapper tests.
    • Added a shared upcoming-schedule fixture and conformance tests across all SDKs.
    • Go: guard empty upcoming window bounds locally with clear errors; thread url/highlighted/status through the public CreateScheduleEntryRequest to the request body.
    • Ruby generator: fix YARD @param continuation to avoid trailing whitespace; added tests.
    • SPEC: updated schedule-entry write cases to 11 and documented the new create cases; fixed a related test comment.
  • Migration

    • Pass both window_starts_on and window_ends_on to reports.upcoming in all SDKs (Go now rejects empty bounds locally).
    • Read upcoming data from UpcomingScheduleEntry/UpcomingAssignable (e.g., use content for assignables; recurring exists on entries; bucket has id and name only).
    • Expect type changes: Swift arrays now [UpcomingScheduleEntry]/[UpcomingAssignable]; Kotlin returns UpcomingScheduleResult (not JsonElement); Go parses starts_at/ends_at as types.FlexibleTime and supports url/highlighted/status on create.

Written for commit bd8b524. Summary will update on new commits.

Review in cubic

jeremy added 2 commits August 3, 2026 23:56
…es (#635, #641)

`GetUpcomingSchedule` declared the shared `ScheduleEntry` and a half-modelled
`Assignable`, but BC3 renders the report through purpose-built calendar
partials — `app/views/api/schedules/calendar/_entry.json.jbuilder` and
`_assignable.json.jbuilder` — which write their own keys instead of rendering
`recordings/_recording`. The published contract promised fields the endpoint
does not send.

The proven failure is any populated window: a response carrying at least one
schedule entry, recurring occurrence or assignable. An empty envelope decodes
fine, and a call with no window is a bodiless BC3 400 before any body exists.
Against `ScheduleEntry`, Swift threw `DecodingError.keyNotFound` on
`bucket.type` — the nested omission a strict decoder reaches before any of the
six top-level members the calendar partial drops.

Six dedicated shapes replace the borrowed ones, named after their owning report
the way `Draft*` and `MyAssignment*` already are: `UpcomingScheduleEntry`,
`UpcomingAssignable`, `UpcomingScheduleBucket`, `UpcomingSchedulePerson`,
`UpcomingAssignableParent`, `UpcomingAssignableCompletion`. They are not a
subset of the full shapes — the entry partial emits `recurring`, which no other
schedule-entry projection carries, and the assignable partial emits the item
text as `content` where the retired schema declared `title`.

Also truthful now: all three envelope arrays are `@required` (the index
template writes every key unconditionally) and both window bounds are
`@required` (`params.require` + `rescue_from ActionController::ParameterMissing`
is a 400, so an unbounded call never worked).

Go returns the reduced types through aliases rather than converting them into
full `ScheduleEntry`/`Assignable` values — converting is what hid the mismatch,
since it zero-fills every field the endpoint never sends. Kotlin's reports
service returned a bare `JsonElement` and enforced no contract at all; the
generator now emits a typed result class for an object-of-arrays envelope, so
Kotlin decodes `UpcomingScheduleResult` and fails a body missing a required key.

Riding along, #641: `CreateScheduleEntry` gains `url`, `highlighted` and
`status`, all three accepted by `Schedules::Entries::BaseController` since long
before they were documented. The join link is `url` on write and `join_url` on
read — the entry's own `url` is its Basecamp API URL — so the write spelling is
deliberate.

Conformance had zero coverage of this operation, and the only two tests that
touched it used a fabricated `{"entries": [...]}` body whose top-level key BC3
has never sent. That is how six SDKs shipped the mismatch with tests passing.
Now: four six-runner cases built from the real partials, a shared
`spec/fixtures/schedules/upcoming.json` under the manifest guard, and per-SDK
tests in all six.
…644)

Eight of the ten response examples in `openapi.json` were the empty object, and
the remaining two were wrapped in a key the schema does not have. Under a
validator that checks a projected example against its projected schema, seven of
the ten fail on `main`:

    FAIL ListRecordings 200 example1: {} is not of type 'array'
    FAIL UpdateSubscription 200 example1: 'count' is a required property
    FAIL GetTodolistOrGroup 200 example1: 'app_url' is a required property

The three that pass do so vacuously — `ProjectAccessResult` declares no required
members, so `{}` satisfies it. So the "two currently validated" figure in #638
counts the `GetTodolistOrGroup` pair, and those are among the failures: their
value is `{"result": {…}}` against a schema that is a bare `Todolist`.

Two causes, one mechanical and one editorial.

The mechanical one is that `BareObjectResponseMapper` and
`BareArrayResponseMapper` rewrite only `components.schemas`. Smithy's restJson1
protocol forces an output to be a wrapper structure, those mappers unwrap the
projected schema down to the bare body BC3 sends, and the examples under `paths`
kept the wrapper nobody unwrapped. `BareResponseExampleMapper` now mirrors them:
it reuses their own `shouldTransform` predicates so the three cannot drift on
which schemas count as wrapped, and it runs first, because the wrapper property
name is only readable while the schema is still wrapped.

It also drops an example with nothing to unwrap. The converter emits a response
example for every `@examples` entry, including entries that declare only an
`input`, and fills those with `{}` — a documented 200 body no client could
receive. Declaring input-only examples stays legitimate; it just stops
publishing an empty object as the response.

The editorial one is that `ListRecordings`, `UpdateProjectAccess` and
`UpdateSubscription` had no `output` to show. All three now declare faithful
ones, so the count of validated response examples goes 2 → 10 rather than
7 → 3-by-deletion.

Two consequences worth naming:

- The `jsonAdd` pointers that inject `Todolist.color` into the
  `GetTodolistOrGroup` examples now end `/value/color`, not
  `/value/result/color`. jsonAdd is applied after the mappers, so a pointer
  still naming `result` would rebuild the wrapper that was just removed.
- `spec/smithy-bare-arrays`'s own tests could not run at all: the project
  targets Java 11 and asked for JUnit 6, which publishes only a JVM-17 variant,
  so `./gradlew test` failed at `compileTestJava` and nothing reported it. Three
  test cases had drifted out of agreement with the mappers in the meantime —
  they still asserted an operation-name prefix rule and an inline-object case
  the mappers stopped honouring. Pinned to JUnit 5, corrected those three, and
  wired `smithy-mapper-test` into `make check` so it cannot go quiet again.
@jeremy jeremy added bug Something isn't working spec Changes to the Smithy spec or OpenAPI labels Aug 4, 2026
@github-actions github-actions Bot added typescript Pull requests that update TypeScript code ruby Pull requests that update the Ruby SDK go kotlin swift conformance Conformance test suite python Pull requests that update the Python SDK labels Aug 4, 2026
@jeremy
jeremy marked this pull request as ready for review August 4, 2026 07:52
Copilot AI balanced review requested due to automatic review settings August 4, 2026 07:52

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cb438ce3ce

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread go/pkg/basecamp/reports.go
Comment thread spec/basecamp.smithy
…cription

`git diff --check` flagged five trailing-whitespace lines in
`schedules_service.rb`, all of them `#` plus the continuation padding and
nothing else. The generator folded a multi-line body/query description under its
YARD tag with `desc.gsub("\n", "\n      #   ")`, which pads every following line
including the empty ones a paragraph break is made of.

Fixed in `ruby/scripts/generate-services.rb`, not in the emitted file: patching
the output means the next `make generate` puts it straight back, and the person
who "fixes" it a second time has no way to see why it returned.

`yard_param_description` renders the first line bare — the caller has already
written `# @PARAM name [Type] ` in front of it — indents the rest, and emits a
bare `#` for a line that is empty or whitespace-only. It also rstrips, so a
Smithy doc comment carrying its own trailing space cannot launder one into a
generated file. Both call sites use it; a single-line description renders
byte-identically to the interpolation it replaces, so nothing else moved.

This is the sibling of `deprecation_doc_lines` in the types generator, which
already had exactly this shape and exactly this blank-line rule
(`generate_types_test.rb`: "blank interior line should stay a bare comment").
The services generator never hit it because no operation had carried a
multi-line body-member description until now.

Unit-tested in `ruby/test/scripts/generate_services_test.rb` — the blank-line
case, whitespace-only breaks, CRLF, and an output-neutrality assertion that a
single-line description is unchanged. A generated file cannot hold the guard for
its own generator.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR corrects GetUpcomingSchedule, which declared the shared ScheduleEntry/Assignable schemas but is actually rendered by BC3 through purpose-built, reduced calendar partials. The published contract promised fields the endpoint never sends, causing strict decoders (Swift) to throw on bucket.type and lenient SDKs to silently zero-fill. The fix introduces six dedicated reduced shapes, makes both window parameters and all three envelope arrays required, and rides along two related fixes: adding url/highlighted/status to CreateScheduleEntry (#641) and unwrapping/correcting response examples that contradicted their own schemas (#644). It fits the repo's Smithy-first architecture: the spec change cascades through the full generation pipeline into all six SDKs, backed by new conformance cases dispatched by all six runners.

Changes:

  • Adds six purpose-built shapes (UpcomingScheduleEntry, UpcomingAssignable, UpcomingScheduleBucket, UpcomingSchedulePerson, UpcomingAssignableParent, UpcomingAssignableCompletion); makes window params and envelope arrays required; retypes SDK results (Go aliases, Kotlin typed UpcomingScheduleResult via new TYPED_ARRAY_ENVELOPE_OPERATIONS, Swift [Upcoming*]).
  • Adds BareResponseExampleMapper (order 90) to unwrap/drop response examples so they match unwrapped schemas; pins JUnit 5 and wires mapper tests into check-targets.
  • Adds shared fixture + conformance suite (4 cases) across all six runners, plus per-SDK tests, and fixes a Ruby dig_path false-as-miss bug.

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.

Reviewed changes

Copilot reviewed 32 out of 65 changed files in this pull request and generated no comments.

Show a summary per file
File Description
spec/basecamp.smithy Defines six reduced shapes, makes window params required, adds BadRequestError, adds url/highlighted/status to create.
openapi.json Regenerated wire contract; examples unwrapped and non-contradictory.
go/pkg/basecamp/reports.go Type aliases + local YYYY-MM-DD validation; returns generated projection verbatim.
scripts/enhance-openapi-go-types.sh FlexibleTime for entry starts_at/ends_at; nullable dates for UpcomingAssignable.
spec/smithy-bare-arrays/.../BareResponseExampleMapper.java New mapper unwrapping/dropping response examples via sibling shouldTransform.
kotlin/generator/.../ServiceEmitter.kt, Config.kt Array-envelope typed result emission + TYPE_ALIASES/opt-in set.
*/generated/services/reports.* Regenerated upcoming(...) with required window params across SDKs.
conformance/tests/upcoming_schedule.json + runners 4 new decode cases + per-runner flat summaries; Ruby dig_path fix.
spec/fixtures/schedules/upcoming.json, manifest.yaml Shared fixture with element-level coverage targets.

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

jeremy added a commit that referenced this pull request Aug 4, 2026
Fourth review round on #642. Four findings, all upheld.

The allowlist framing was wrong in the direction that matters. I wrote that
fewer hook events are safe for an allowlist. True only if the allowlist named
both operations: one that names UpdateCard and deliberately omits GetCard used
to reject cards.update at its read, and after the collapse permits it end to
end. Both policy shapes now carry the warning, labelled, plus the observation
that they are the same hole seen twice — in each, the thing stopping the write
was the read, expressed once as an omission and once as an entry.

The class-A counting was inconsistent across all six SDKs, not the two flagged.
Python and Kotlin excluded changes their own prose called "no signal
whatsoever"; auditing every SDK against the definition moved the totals to 47
class A and 4 class B. The counting policy is now stated in the document so it
can be checked against a rule rather than an impression: one entry per distinct
change per SDK, counted where it bites; class A if any ordinary call-site shape
stays silent even when another is compile-caught; second faces annotated as
residue and counted once; raises-only-on-malformed-response is class B.

Two things fell out that were not counting problems. Ruby's #563 was missing
from the guide entirely — no mention of download_url anywhere in the chapter —
verified against source rather than prose: v0.12.0 http.get_no_retry, which
sent Accept: application/json and did not retry, became get_download calling
request_with_retry with retry_on: DOWNLOAD_RETRY_ON and accept: nil. Ruby now
has its own section. The same check confirmed Go's omission of #563 is correct,
because Go already retried at v0.12.0. Separately, the Go note claiming the
compiler catches only the pkg/generated half of Schedules().UpdateEntry was
false: UpdateScheduleEntryRequest's fields became pointers, so any pkg/basecamp
call site that set a field fails to build.

The class-B definition described only half its own membership. It said the
trigger is an absent field, but Ruby's entry fires only when the field is
populated. It now says both, and says plainly that class B is a property of a
call plus a response rather than of the call — the same method against the
other shape is not a break at all. Class A has no such dependency.

Stale counts in the chapter intros are fixed. The Go intro still said eleven
silent and two panics, which is the first thing a #go link shows, and Swift
claimed the most no-signal breaks, which stopped being true at Go ten.

Also folds in #652 (projected-example gate, stacked on #648, takes check-targets
to 43), moves #648 out of draft at cb438ce, and records that #647 is being
reworked Smithy-first because the generated UpdateCardStepRequestContent.DueOn
is *types.Date and cannot express "". The consumer-facing card shape is
unaffected by that rework. Re-derived against #648: 238 -> 247 with 14 added,
5 removed and 11 same-ID route moves survives unchanged.
jeremy added 3 commits August 4, 2026 02:36
…ot a server 400

`UpcomingSchedule` guarded both bounds by parsing them, on the assumption that
parsing rejects a missing one. It does not: `types.ParseDate("")` returns a zero
`Date` and a NIL error by design, so an empty `startDate` or `endDate` walked
straight past the guard, went out on the wire without the parameter, and came
back as exactly the BC3 400 the guard exists to convert into a local `ErrUsage`.

The emptiness check is now its own step, before the format check, for each bound
separately. The messages split with it — "window_starts_on is required" for an
absent bound, "must be in YYYY-MM-DD format" for a malformed one — because the
old combined wording described a check that was only half happening.

`TestUpcomingSchedule_RejectsMissingWindowBounds` covers both bounds in both
directions: both empty, start empty, end empty, plus the two malformed cases that
already passed. Each case fails the test if the request reaches the httptest
server, which is the property that actually distinguishes a local guard from a
server round trip. Against the un-fixed guard the three empty cases fail with
"expected a local usage error, got none" and the two malformed cases still pass —
a guard that only checks the first argument is the usual way this half-works, so
the end-empty case is the one carrying the weight.
…create request

#641 modelled `url`, `highlighted` and `status` on `CreateScheduleEntry` and five
SDKs picked them up for free, because in those five the create method IS the
generated one. Go is the sixth: its public surface is the hand-written
`CreateScheduleEntryRequest` in `pkg/basecamp/schedules.go`, and that struct never
grew the three members. `generated.CreateScheduleEntryJSONRequestBody` had them
the whole time — nothing could reach them. A Go caller wanting a video-call event
with its join link still had to create, then read, then replace, through the same
non-atomic window #641 exists to close, and `create` is the notifying write.

Nothing failed, because every gate looked at the generated layer. A five-of-six
that reads as six-of-six is how this shipped the first time.

Declaring the fields is only half of it: a member on the public struct that never
reaches the body construction is a silent no-op, the same defect wearing different
clothes. All three are threaded at the call site, and
`TestSchedulesService_CreateEntryJoinLinkHighlightAndStatus` asserts the observed
request body rather than the struct — with the three lines removed from the body
construction but the members left on the struct it fails with url/highlighted/
status all `<nil>`, which is precisely the shape of the bug.

The write spelling is `url`, NOT `join_url`. `join_url` is read-only: the
recording partial claims the `url` key for the entry's own API URL and the entry
partial renders after it, so BC3 emits the join link under a non-colliding name on
the way out. Strong parameters drop `join_url` on the way in — a 201 with no join
link — so the test asserts its ABSENCE from the body, not merely `url`'s presence.
`status` is a top-level parameter, not one of the entry's attributes: it is a
Recording column, so `wrap_parameters` leaves it outside the `schedule_entry`
envelope. All three are pointers so unset omits the key;
`TestSchedulesService_CreateEntryOmitsUnsetJoinLinkHighlightAndStatus` pins that,
and it matters most for `highlighted`, since `schedule_entries.highlighted` is
NOT NULL and an explicit null makes BC3 raise instead of applying its default.

Two conformance cases, dispatched by all six runners, so the language-level gap
cannot reopen in only one of them: one sends all three and asserts they reach the
wire while `join_url` does not, one sends none and asserts all three stay off it.
The runners needed `status` added to their write-field extraction — it is not a
`ReplaceScheduleEntry` member, so the shared map could not carry it — and the Go
runner needed a `deref` helper, since create takes plain strings where replace
takes pointers.
…e-projection

* origin/main:
  Cards: explicit due-date clears silently no-op against production — send "due_on": "" (#647)

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

@jeremy

jeremy commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

@codex review

Head has moved since your last pass (#648: cb438ce3c → 4af546518; #652: b7f86d66e → e69218c0b). These are the last two merges before the v0.13.0 freeze, so I'd like a look at the current head rather than merging on the earlier review.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: 4af546518d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

a2b44e3 took conformance/tests/schedule_entries_write.json from 9 cases
to 11, adding create-join-link and create-omits-unset, and touched no
SPEC.md. Two statements went stale and nothing gates them:
doc-constants-check syncs marked constants, not per-fixture case counts.

Appendix D still said "9 cases" and described only the replace/update/edit
triad. The §5 Conformance line enumerated the old nine case names. Both
now read 11, measured from the fixture rather than carried forward, and
both say what the create pair is doing in a file named for the merge-safe
surface: CreateScheduleEntry is a plain wire write with no read-back and
nothing to preserve, so the pair pins absent-versus-zero-value for url,
highlighted and status one operation earlier than the carve-outs. The rest
of the sweep is clean -- §18's carve-out justification and the Appendix D
category index both still hold.

Separately, in ruby/test/scripts/generate_services_test.rb the MULTILINE
constant's comment claimed it contained a blank interior line, a
trailing-space line and a CRLF pair. It contains only the blank interior
line. The trailing-space, whitespace-only and CRLF cases have their own
literals in three sibling tests, which is why MULTILINE can afford a
line-by-line assertion naming every rendered line. The tests were right;
only the comment was wrong. It now points at where the other cases live.
Copilot AI review requested due to automatic review settings August 4, 2026 18:35
@jeremy

jeremy commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

SPEC.md drift from a2b44e347, plus a test comment that misdescribes its own fixture

Both found by adversarial review. Fixed in bd8b52411.

1. SPEC.md still described a 9-case fixture

a2b44e347 took conformance/tests/schedule_entries_write.json from 9 cases to 11 — adding create-join-link and create-omits-unset — and touched no SPEC.md. Nothing gates this: doc-constants-check syncs marked constants, not per-fixture case counts.

Measured from the fixture rather than trusted (note the file is a bare array, so the case path is .[], not .cases[]):

$ jq -r 'length' conformance/tests/schedule_entries_write.json
11

$ jq -r '.[].name' conformance/tests/schedule_entries_write.json
replace-omission-clears: an unaddressed participant list, join link and highlight stay off the wire
replace-clears-carve-outs: explicit empty values reach the wire
replace-single-request: the raw path never reads before writing
update-merge: a summary-only update preserves the times, description and all-day flag
update-addresses-carve-outs: a caller-set join link and highlight reach the wire
update-clears-carve-outs: an explicitly empty join link, empty participant list and false highlight are sent
edit-clear: clearing the description keeps every other field
edit-untouched-carve-outs: a block that never assigns them leaves them off the wire
edit-touched-carve-outs: assigning the value the read already returned still sends it
create-join-link: url, highlighted and status reach the wire on create
create-omits-unset: an unset join link, highlight and status stay off the wire

SPEC.md:3332 (Appendix D) said 9 cases and described only the replace/update/edit triad. Now 11, and it names what the create pair asserts.

SPEC.md:476 (§5) enumerated the old nine case names. Now 11, and it says what the create pair is doing in a file named for the merge-safe surface — CreateScheduleEntry is a plain wire write with no read-back and nothing to preserve, so the pair pins the same absent-versus-zero-value distinction as the carve-outs, one operation earlier. They share the file because they share the fixture's entry shapes, not because create is merge-safe.

Sweep for anything else the two cases falsify: clean. SPEC.md:451/1948 (url is identity-colliding, write-side join link) and :452/1949 (highlighted was write-only until basecamp/bc3#12502) both still hold and are in fact what create-join-link asserts — its url-not-join_url requirement is the same fact one operation earlier. SPEC.md:1965 (§18) is about the composites only and is unaffected. SPEC.md:2042 (category → owning-section index) already lists §5 Schedule Entries, §18 and §10 Type Fidelity (explicit-empty vs. omitted wire semantics); the create pair falls under those and the row states no case count, so it needed no change.

2. ruby/test/scripts/generate_services_test.rb — a comment that lies about its constant

The MULTILINE comment claimed the constant held "A blank interior line, a trailing-space line, and a CRLF pair". It holds only the blank interior line:

MULTILINE = "The entry's join link.\n\nRead it back as `join_url`.\nNever as `url`."

The other three cases have their own literals in sibling tests — "first\n\nsecond \n\t\nthird" in test_no_emitted_line_carries_trailing_whitespace, "first\n \nsecond" in test_whitespace_only_line_is_treated_as_a_break, "first\r\nsecond" in test_carriage_returns_do_not_leak. The tests are correct; only the comment was wrong. It now describes what the constant actually holds and points at where the other cases live — and notes why MULTILINE is kept plain: it is the one literal whose assertion names every rendered line individually.

Verification

Exit codes captured to a log under a marker and grepped back, not read off mid-run text:

MARKER_648_DOCCONST=0        # make doc-constants-check
MARKER_648_CONFORMANCE=0     # make conformance (full, all six runners)
MARKER_648_RUBY_RAKE=0       # cd ruby && bundle exec rake test -- 1368 runs, 30421 assertions, 0 failures

Per-runner conformance totals:

Go          Passed: 180, Failed: 0, Skipped: 2, Total: 182
Kotlin      Passed: 181, Failed: 0, Skipped: 1, Total: 182
TypeScript  Results: 171 passed, 0 failed, 11 skipped
Ruby        Passed: 181, Failed: 0, Skipped: 1, Total: 182
Python      Results: 182 passed, 0 failed, 0 skipped
Swift       Passed: 181, Failed: 0, Skipped: 1, Total: 182
==> Conformance tests passed

and the two cases at issue pass in every runner that reports per-case:

$ grep -cE "PASS: (create-join-link|create-omits-unset)" verify-648.log
10

One note on a stray exit code, so it is not mistaken for a regression: running test/scripts/generate_services_test.rb alone exits 2, because SimpleCov applies the whole-suite floor (minimum_coverage line: 90, branch: 60) to a single-file run. The tests themselves are green in isolation — 9 runs, 13 assertions, 0 failures, 0 errors, 0 skips — and the full rake test exits 0. The change is comment-only in a test file either way.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bd8b52411e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread spec/basecamp.smithy
///
/// Unlike messages and documents, schedule-entry drafts are not listed by
/// GetMyDrafts.
status: String

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Declare the bad-request response for create status

This addition exposes status on CreateScheduleEntry, and the new docs here say BC3 raises ActionController::BadRequest (HTTP 400) for an unsupported status, but the operation’s errors list still omits BadRequestError, so the regenerated OpenAPI response set for create only includes 201/401/403/422/429/500. When a caller sends an invalid status, the SDK contract and generated WithResponse types do not model the 400 path; add the 400 error to CreateScheduleEntry and regenerate.

Useful? React with 👍 / 👎.

@jeremy

jeremy commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

Merging without a Codex review at head, deliberately and with a substitute rather than a waiver.

Codex last reviewed cb438ce3c; it did not pick up 4af546518 or bd8b52411, and did not respond to a review request. Copilot's check is failing across every PR in the repo today (its own infrastructure, not a gate). So both bot reviewers were unavailable for this merge.

In their place I ran an independent adversarial review targeted at exactly the un-reviewed delta. It was not a formality — it found three confirmed defects that 0-unresolved-threads and green CI had all reported as clean:

  1. This PR was missing the breaking label. It removes exported Assignable, deletes UpcomingReportOptions in TypeScript and Swift, changes Kotlin's return type, and makes Ruby/Python kwargs required. .github/release.yml derives the breaking-changes section solely from that label, so v0.13.0's notes would have filed six-SDK API removals under Bug Fixes. Label added. Worth noting the API Compatibility check could not have caught this — test.yml:859 is # Don't fail the build, just warn.
  2. SPEC.md drift from a2b44e347: conformance/tests/schedule_entries_write.json went 9 → 11 cases without SPEC.md following. :3332 still said "9 cases" and :476 listed the old nine names. Nothing gates per-fixture case counts. Fixed, with the count measured from the fixture rather than assumed.
  3. A comment in ruby/test/scripts/generate_services_test.rb describing cases its MULTILINE constant does not contain. Fixed.

Positively cleared, having actually been checked: the merge is not evil (merge-tree --write-tree reproduces the tree bit-for-bit), the projection is field-exact against the BC3 jbuilder partials (_entry 15 keys, _assignable 17 with completion correctly the only optional, _person_minimal 3), all six SDKs make both window bounds required, and the operation count stays 247 with gates going 41 → 42.

One inconsistency found but deliberately not fixed here, filed as #659: MyAssignmentAssignee and OutOfOfficePerson model the same person_minimal partial with only id required. This PR's UpcomingSchedulePerson is the correct reading; the two older shapes are under-marked.

@jeremy
jeremy merged commit e0431a7 into main Aug 4, 2026
46 of 47 checks passed
@jeremy
jeremy deleted the feat/upcoming-schedule-projection branch August 4, 2026 18:52
jeremy added a commit that referenced this pull request Aug 4, 2026
Fourth review round on #642. Four findings, all upheld.

The allowlist framing was wrong in the direction that matters. I wrote that
fewer hook events are safe for an allowlist. True only if the allowlist named
both operations: one that names UpdateCard and deliberately omits GetCard used
to reject cards.update at its read, and after the collapse permits it end to
end. Both policy shapes now carry the warning, labelled, plus the observation
that they are the same hole seen twice — in each, the thing stopping the write
was the read, expressed once as an omission and once as an entry.

The class-A counting was inconsistent across all six SDKs, not the two flagged.
Python and Kotlin excluded changes their own prose called "no signal
whatsoever"; auditing every SDK against the definition moved the totals to 47
class A and 4 class B. The counting policy is now stated in the document so it
can be checked against a rule rather than an impression: one entry per distinct
change per SDK, counted where it bites; class A if any ordinary call-site shape
stays silent even when another is compile-caught; second faces annotated as
residue and counted once; raises-only-on-malformed-response is class B.

Two things fell out that were not counting problems. Ruby's #563 was missing
from the guide entirely — no mention of download_url anywhere in the chapter —
verified against source rather than prose: v0.12.0 http.get_no_retry, which
sent Accept: application/json and did not retry, became get_download calling
request_with_retry with retry_on: DOWNLOAD_RETRY_ON and accept: nil. Ruby now
has its own section. The same check confirmed Go's omission of #563 is correct,
because Go already retried at v0.12.0. Separately, the Go note claiming the
compiler catches only the pkg/generated half of Schedules().UpdateEntry was
false: UpdateScheduleEntryRequest's fields became pointers, so any pkg/basecamp
call site that set a field fails to build.

The class-B definition described only half its own membership. It said the
trigger is an absent field, but Ruby's entry fires only when the field is
populated. It now says both, and says plainly that class B is a property of a
call plus a response rather than of the call — the same method against the
other shape is not a break at all. Class A has no such dependency.

Stale counts in the chapter intros are fixed. The Go intro still said eleven
silent and two panics, which is the first thing a #go link shows, and Swift
claimed the most no-signal breaks, which stopped being true at Go ten.

Also folds in #652 (projected-example gate, stacked on #648, takes check-targets
to 43), moves #648 out of draft at cb438ce, and records that #647 is being
reworked Smithy-first because the generated UpdateCardStepRequestContent.DueOn
is *types.Date and cannot express "". The consumer-facing card shape is
unaffected by that rework. Re-derived against #648: 238 -> 247 with 14 added,
5 removed and 11 same-ID route moves survives unchanged.
jeremy added a commit that referenced this pull request Aug 4, 2026
Rebased onto 2afc977 and re-measured rather than incremented. Eight PRs merged
since the branch was last updated, not the seven that carried the breaking
label: #647 was on the "Not in this release" list and had landed.

Counts. 55 class A and 6 class B, 61 surviving a clean build, up from 47/4/51.
Per SDK the class split is Go 12/4, Swift 10/0, TypeScript 9/0, Python 8/0,
Ruby 10/1, Kotlin 6/1, and the breaking-change column moves to 33/22/18/16/20/17.
The body parses back to those numbers rather than agreeing with them by hand.
The root README's aggregate sentence is re-derived to match, and now states both
halves numerically instead of "most" and "a few". The operation inventory is
unchanged at 238 -> 247 with the same 14 added, 5 removed and 11 same-ID route
moves, computed from openapi.json at both ends. check-targets is 43, and the
derivation is inline where the gate count was previously only projected. The
release spans 67 merged PRs, 15 labelled breaking; the gh commands that produce
both are embedded in the as-of block, with the note that a labelled PR is not
the same unit as an entry, which is why the per-SDK columns exceed 15.

#658 is class B, not class A. It does to five wrapper timestamps exactly what
#615 did to five others: QuestionReminder.RemindAt, ClientApprovalResponse's
CreatedAt and UpdatedAt, TimelineEvent.CreatedAt and WebhookDelivery.CreatedAt
compile untouched through a value-receiver call and panic on nil. #615's own
check could not see them because it keyed on the omitempty tag and these five
did not carry one. The audit is ten fields, and the entry names the near-miss
siblings that did not move, ClientApproval's pair in particular.

#664 splits. The public CreateScheduleEntryRequest fields were already string
and still are, so the wrapper half is silent: the RFC3339 ErrUsage guard is gone,
a bare date now creates an all-day entry, and a malformed value reaches bc3
instead of failing locally. That is class A. The generated
CreateScheduleEntryRequestContent went time.Time to string, which is a compile
error for pkg/generated importers. ReplaceScheduleEntryRequestContent is not a
migration from v0.12.0 at all; #632 introduced it. TypeScript and Ruby are
doc-comment only.

#647 is folded in as merged, with two corrections to what was written when it
was still a branch. It touches no schema, so the claim that it had to go
Smithy-first is withdrawn; UpdateCardStepRequestContent.DueOn was pointerized by
#560. And the v0.12.0 preservation GET was conditional, taken only when the
caller left due_on unaddressed, so the request-count table is scoped to that
path rather than presented as universal.

#648 adds no silent break anywhere. bc3's body is byte-identical before and
after, so nothing that was populated stops being so; the assignable's title was
never sent and is now spelled content. Every rename and retype is caught
statically in Go, Swift, TypeScript and Kotlin and raised immediately in Python
and Ruby, so it is one compile-or-runtime entry per SDK.

Two corrections nobody asked for. The Go class list opened "Go carries every
class-B break in the release", which stopped being true when Ruby's decode
entry moved into class B; it now claims only the panic-shaped ones. And
todos_write.json carries three errorRaised cases, not two, because #660 added a
bare-scalar kill.

#660 is a Kotlin class-B entry, which is new. Removing the client-wide isLenient
means a present, populated, wrong-typed scalar throws SerializationException
where it used to coerce to a string, and no signature moved to announce it. It
throws in the response decode, so on a write the mutation has already landed,
and it is not a BasecampException outside todolists.

#656 is Ruby class A, scoped tightly: only max_retries 0, only an ungoverned GET,
which means get_absolute and the Launchpad fetch rather than any operation
lacking a policy. Every other configuration is bit-identical.

Not in this release is now empty, and says so.
jeremy added a commit that referenced this pull request Aug 4, 2026
* MIGRATING.md: the v0.13.0 upgrade guide, silent breaks first

v0.13.0 breaks all six SDKs and 35 of those breaks are silent — no compile
error, no exception, no decoder failure. Label-generated release notes list
what merged; they cannot say what a consumer must react to or what wrong
behaviour they get if they ignore it. That had no home in this repo.

Adds MIGRATING.md at the root, linked from the root README and all six
per-SDK READMEs. Silent breaks lead the document, then one section per SDK
ordered by severity, plus an operator checklist, a "coverage: corrected and
re-scoped" section for what did not ship, and known gaps.

No CHANGELOG is reintroduced. The hand-maintained ones were deleted in #115
as superseded by auto-generated notes, and every release body since is
machine-built. CONTRIBUTING records the resulting rule: label-generated notes
say what merged, MIGRATING says what to do about it.

Corrections to the source drafts, each re-derived rather than repeated:

- TrashTodo was not a 404. bc3 draws `resources :todos, only: %i[show edit
  update destroy]`; DELETE /todos/:id returned 204 and set status to
  "archived", so every caller was archiving. It is the one #619 removal that
  takes away a working call, and it now carries its own carve-out.
- #619 removed three operations, not nine. Nine were re-pathed. Fusing the
  two sets is what made the blanket 404 reassurance look safe.
- Hook operation identity differs by SDK: Go and Ruby emit a short verb,
  the other four emit the wire operation ID, where the todolist pair kept
  its names — so an allowlist holding UpdateTodolistOrGroup passes the write
  and denies the new read.
- 238 -> 241 measured at the v0.12.0 tag and at c95d81c, not assumed.
- Kotlin binary compatibility is already disclaimed in kotlin/README.md;
  Swift has no written policy. Both are now stated rather than left unsaid.

recordings.get is documented as a known gap with a list-and-filter recipe
and its honest cost. The Go recipe compiles against this tree.

#637, #629 and #635/#641 were open at the time of writing and are recorded
under "Not in this release" rather than described as shipped.

* Fix the Go pagination advice, cut the raw-wire workaround, absorb #637/#643

Addresses both P1 review threads on #642 and folds in the two PRs that landed
since the first draft.

Pagination (P1). Cross-SDK item 1 claimed `page` was a starting offset in every
SDK and told readers to drop it to restore the old walk. For Go that was
actively harmful: `git show v0.12.0:go/pkg/basecamp/bookmarks.go` returns before
followPagination whenever page > 0, so a positive Page already meant one
request, and dropping it converts a bounded call into a full account-wide
traversal. The item is now scoped to the five SDKs where it holds — re-checked
at the tag rather than assumed, since the universal claim had already failed
once — with a Go subsection splitting the two real cases: services where the
page number was already honored (Bookmarks, Drafts, Everything*, request
unchanged) and the fourteen carrying the "not yet honored" doc, which sent no
page at all and returned page 1's rows. Gauges is in neither; it had no page.

Raw wire (P1). The Forwards().CreateReply example built a path with fmt.Sprintf
and called the raw AccountClient.Post against a route with no upstream
coverage, which is what AGENTS.md "Never Do These" 4 and 5 forbid. Removed
rather than softened, and replaced with a known-gap section stating what a
hand-built path gives up. Swept the document: the one other hit documents a real
change to the raw client's error codes, so it stays, but its fabricated path is
gone and it now says it is not a suggestion to reach for the escape hatch.

#643 landed, so basecamp.Ptr and basecamp.Deref replace the hand-rolled ptr
helper throughout, the Go section opens with the 300-pointer census and a
command that reproduces it, and ParticipantIDs *[]int64 gets its own note: nil
leaves participants alone, a pointer to an empty slice removes every one.

#637 landed and does NOT add a break to any SDK. color and comments_app_url did
not exist on Todolist at v0.12.0 in any of the six — both arrived with #628
earlier in this same release — so from the guide's baseline nothing turned from
optional to required. Counts stay 27/20/16/14/16/14. Documented where it bites:
color is required-and-nullable so explicit null decodes, comments_app_url
rejects null and absence alike.

Also: kotlin/README's append-only source-compat promise contradicted this
release repeatedly, so it now describes documented pre-1.0 breaking correctness
releases; the binary-compat disclaimer is kept and sharpened. release-github.yml
links MIGRATING.md from every release body, guarded on the file, so the link
cannot be forgotten at tag time. "Silent" is defined as source/runtime-silent
against a live server, since a suite pinning request paths does catch some.

Counts are stated as-of 51d0d86 with derivations inline, and each in-flight
change names the numbers it invalidates so the pre-tag pass is arithmetic.

* Split silent breaks into no-signal and fails-at-runtime; absorb #629 and cards

Addresses the remaining P2 and a suppressed Copilot comment on #642, re-derives
every count against main, and writes the cards due-date change.

The P2 was right, and it was a contradiction with this guide's own definition
rather than loose wording: "silent" was defined as "does not raise" and then
used to file nil-pointer panics. The section is now "Breaks your compiler will
not catch" — the property all of it actually shares — split into class A, no
signal at all, and class B, compiles then panics or raises but only when a
particular field is absent, so it passes every test where that field is
populated. Applying the definition consistently moved four entries, not the
three flagged: the three Go pointerization panics plus Ruby's
Draft#scheduled_posting_at decode, which raises NoMethodError and TypeError and
had the same defect. Two moved entries carry real no-signal residue, kept as
sub-notes rather than double-counted. Per SDK: Go 8A/3B, Swift 9A, TypeScript
5A, Python 4A, Ruby 2A/1B, Kotlin 3A — 31 + 4 = 35, unchanged in total. Body
counts verified against the table by parsing the section, not by eye.

The Swift section claimed three new optional Todolist members and named one;
the other two are required. Now singular, matching TypeScript.

Counts re-derived at 9de44b2: the inventory is 238 -> 247, not 241, since
#629 merged. Added, removed and route-moved lists are computed from openapi.json
at both ends rather than hand-edited — 14 IDs added, 5 removed, 11 same-ID moves
— and the Folders operations are flagged as drawn at /stacks, not /folders.

Cards get their own section. The half that matters most is true in production
today and is not caused by upgrading: every released SDK encodes "clear a card
due date" as omission, bc3 stopped treating omission as a clear, so that call is
a silent no-op right now. That is a reason to upgrade rather than a hazard of
it, so it sits in the operator checklist. The SDK-side change is read from
bf43715 and marked unmerged: single PUT, "due_on": "" as the clear encoding,
UpdateStepRequest.DueOn becomes *string, and the GetCard preservation read goes
away. The hook collapse is written as the inverse of the {Todolists,Update}
split because it fails the opposite way — allowlists do not start denying, but a
denylist on {Cards,Get} silently stops blocking the write it used to take down.
Removing the preservation GET also removes three named errorRaised kill cases
from cards_write.json; the class stays pinned on Todos, which still does a real
read-modify-write, so that is said rather than filed as a redundant-GET cleanup.

* Audit class A across all six SDKs; add Ruby's missing download retry

Fourth review round on #642. Four findings, all upheld.

The allowlist framing was wrong in the direction that matters. I wrote that
fewer hook events are safe for an allowlist. True only if the allowlist named
both operations: one that names UpdateCard and deliberately omits GetCard used
to reject cards.update at its read, and after the collapse permits it end to
end. Both policy shapes now carry the warning, labelled, plus the observation
that they are the same hole seen twice — in each, the thing stopping the write
was the read, expressed once as an omission and once as an entry.

The class-A counting was inconsistent across all six SDKs, not the two flagged.
Python and Kotlin excluded changes their own prose called "no signal
whatsoever"; auditing every SDK against the definition moved the totals to 47
class A and 4 class B. The counting policy is now stated in the document so it
can be checked against a rule rather than an impression: one entry per distinct
change per SDK, counted where it bites; class A if any ordinary call-site shape
stays silent even when another is compile-caught; second faces annotated as
residue and counted once; raises-only-on-malformed-response is class B.

Two things fell out that were not counting problems. Ruby's #563 was missing
from the guide entirely — no mention of download_url anywhere in the chapter —
verified against source rather than prose: v0.12.0 http.get_no_retry, which
sent Accept: application/json and did not retry, became get_download calling
request_with_retry with retry_on: DOWNLOAD_RETRY_ON and accept: nil. Ruby now
has its own section. The same check confirmed Go's omission of #563 is correct,
because Go already retried at v0.12.0. Separately, the Go note claiming the
compiler catches only the pkg/generated half of Schedules().UpdateEntry was
false: UpdateScheduleEntryRequest's fields became pointers, so any pkg/basecamp
call site that set a field fails to build.

The class-B definition described only half its own membership. It said the
trigger is an absent field, but Ruby's entry fires only when the field is
populated. It now says both, and says plainly that class B is a property of a
call plus a response rather than of the call — the same method against the
other shape is not a break at all. Class A has no such dependency.

Stale counts in the chapter intros are fixed. The Go intro still said eleven
silent and two panics, which is the first thing a #go link shows, and Swift
claimed the most no-signal breaks, which stopped being true at Go ten.

Also folds in #652 (projected-example gate, stacked on #648, takes check-targets
to 43), moves #648 out of draft at cb438ce, and records that #647 is being
reworked Smithy-first because the generated UpdateCardStepRequestContent.DueOn
is *types.Date and cannot express "". The consumer-facing card shape is
unaffected by that rework. Re-derived against #648: 238 -> 247 with 14 added,
5 removed and 11 same-ID route moves survives unchanged.

* Correct four claims in the v0.13.0 guide that do not match the source

The opening warning said the runtime failures need a payload where a field is
absent. That holds for the three Go entries; Ruby's single class-B entry has the
opposite trigger. Draft#scheduled_posting_at and MyNote#created_at/#updated_at
run through parse_datetime, which returns nil for nil and a Time otherwise, so
.start_with? and Time.parse raise only when the field is populated. A reader
following the old text builds the wrong fixture and concludes they are
unaffected. Both directions are now named, here and in the root README.

Class A was described as breaking on every response. Most of it does, but two
groups do not: the error-message and validation entries need an error status to
reach the code at all, and the field-map half needs a body of a particular
shape; downloadURL's hop-1 retry changes nothing until a network error or one of
429/502/503/504 occurs. Stated as preconditions rather than as a blanket claim.

The Go pointer example said only the field selector panics. types.Date.String
has a value receiver, so Go rewrites t.DueOn.String() to (*t.DueOn).String() and
the nil dereference panics before String is entered. The same holds for IsZero,
Before, After and Weekday on Date and for Format, Sub, Unix and Year on
time.Time. The summary bullet already said both panic; the example contradicted
it.

The Accept-header note credited only Python. Ruby dropped it on the same hop:
get_download passes accept: nil, and request_headers sets the header only when
accept is truthy. Both are named, with the observation that the other four never
sent it on that hop at v0.12.0 either.

No counts are touched.

* Re-derive every count against the final release commit

Rebased onto 2afc977 and re-measured rather than incremented. Eight PRs merged
since the branch was last updated, not the seven that carried the breaking
label: #647 was on the "Not in this release" list and had landed.

Counts. 55 class A and 6 class B, 61 surviving a clean build, up from 47/4/51.
Per SDK the class split is Go 12/4, Swift 10/0, TypeScript 9/0, Python 8/0,
Ruby 10/1, Kotlin 6/1, and the breaking-change column moves to 33/22/18/16/20/17.
The body parses back to those numbers rather than agreeing with them by hand.
The root README's aggregate sentence is re-derived to match, and now states both
halves numerically instead of "most" and "a few". The operation inventory is
unchanged at 238 -> 247 with the same 14 added, 5 removed and 11 same-ID route
moves, computed from openapi.json at both ends. check-targets is 43, and the
derivation is inline where the gate count was previously only projected. The
release spans 67 merged PRs, 15 labelled breaking; the gh commands that produce
both are embedded in the as-of block, with the note that a labelled PR is not
the same unit as an entry, which is why the per-SDK columns exceed 15.

#658 is class B, not class A. It does to five wrapper timestamps exactly what
#615 did to five others: QuestionReminder.RemindAt, ClientApprovalResponse's
CreatedAt and UpdatedAt, TimelineEvent.CreatedAt and WebhookDelivery.CreatedAt
compile untouched through a value-receiver call and panic on nil. #615's own
check could not see them because it keyed on the omitempty tag and these five
did not carry one. The audit is ten fields, and the entry names the near-miss
siblings that did not move, ClientApproval's pair in particular.

#664 splits. The public CreateScheduleEntryRequest fields were already string
and still are, so the wrapper half is silent: the RFC3339 ErrUsage guard is gone,
a bare date now creates an all-day entry, and a malformed value reaches bc3
instead of failing locally. That is class A. The generated
CreateScheduleEntryRequestContent went time.Time to string, which is a compile
error for pkg/generated importers. ReplaceScheduleEntryRequestContent is not a
migration from v0.12.0 at all; #632 introduced it. TypeScript and Ruby are
doc-comment only.

#647 is folded in as merged, with two corrections to what was written when it
was still a branch. It touches no schema, so the claim that it had to go
Smithy-first is withdrawn; UpdateCardStepRequestContent.DueOn was pointerized by
#560. And the v0.12.0 preservation GET was conditional, taken only when the
caller left due_on unaddressed, so the request-count table is scoped to that
path rather than presented as universal.

#648 adds no silent break anywhere. bc3's body is byte-identical before and
after, so nothing that was populated stops being so; the assignable's title was
never sent and is now spelled content. Every rename and retype is caught
statically in Go, Swift, TypeScript and Kotlin and raised immediately in Python
and Ruby, so it is one compile-or-runtime entry per SDK.

Two corrections nobody asked for. The Go class list opened "Go carries every
class-B break in the release", which stopped being true when Ruby's decode
entry moved into class B; it now claims only the panic-shaped ones. And
todos_write.json carries three errorRaised cases, not two, because #660 added a
bare-scalar kill.

#660 is a Kotlin class-B entry, which is new. Removing the client-wide isLenient
means a present, populated, wrong-typed scalar throws SerializationException
where it used to coerce to a string, and no signature moved to announce it. It
throws in the response decode, so on a write the mutation has already landed,
and it is not a BasecampException outside todolists.

#656 is Ruby class A, scoped tightly: only max_retries 0, only an ungoverned GET,
which means get_absolute and the Launchpad fetch rather than any operation
lacking a policy. Every other configuration is bit-identical.

Not in this release is now empty, and says so.

* State the schedule-entry clear value per field instead of universally

The Swift Behavioural bullet said an explicit "" clears any of the five
full-state fields. Only description does. "" on summary is accepted and
reads back "Untitled"; starts_at and ends_at are under
validates_presence_of in Schedule::Entry, so "" is rejected rather than
cleared; allDay is a boolean in every SDK, so "" does not typecheck at
all. The carve-out half grouped notify with the three clearable fields
even though it is a send directive with no state to clear.

* Re-derive the per-SDK README banners against the final class A/B table

The six SDK README banners still carried the counts from before the Go
reclassification and the recount that followed it, summing to 51 where
MIGRATING.md and the root README say 61. Each banner now matches its row
in the class A/B table: Go 12+4, Swift 10, TypeScript 9, Python 8, Ruby
10+1, Kotlin 6+1. Kotlin also gains the runtime clause it was missing,
since its one class B entry throws on a present field carrying a JSON
number or boolean where the model declares a string.

* Correct the merged-PR count and the two claims the reviewers caught

The release spans 55 merged pull requests, not 67. The 67 came from comparing
GitHub's Z-formatted mergedAt against a git timestamp formatted with a local
offset, using jq's string >, which is lexicographic rather than temporal; it
wrongly swept in twelve PRs merged in the hours before the v0.12.0 tag instant.
The derivation embedded in the guide taught that same broken comparison, so it
now uses %ct and fromdateiso8601 and says why. The breaking count of fifteen is
unchanged, since all fifteen merged after the tag, so the class A/B split, the
per-SDK tables and the six README banners are untouched.

The header no longer calls 2afc977 the commit the release is cut from. That
commit is the last of the release content and the baseline the counts were
measured against, but it predates this guide; the tag is cut from main after
this merges, on a tree that contains the file the release body links to.

The release-body teaser claimed the guide covers only breaks with no exception
and no decoder failure. The guide documents six breaks that do fail at runtime,
including Ruby and Kotlin raises and a Kotlin decoder failure, so the teaser now
names both the silent class and the runtime one.
jeremy added a commit that referenced this pull request Aug 6, 2026
Release prep for v0.13.0. Documentation only; no code.

Counts re-derived from origin/main by PR-merge-commit ancestry, not
incremented:

  58 -> 64 merged pull requests
  15 -> 16 labelled breaking (#678 earned it)
  238 -> 247 becomes 238 -> 249 (#679 added ArchiveProject/UnarchiveProject)
  14 added / 5 removed becomes 16 added / 5 removed; at capability level
  12 additions becomes 14. 11 route-moved is unchanged, and verified.
  Baseline `70d576bd8` -> `9a819e44d`, the last commit of release content.

The 61 = 55 + 6 split is unchanged, and reconciles across MIGRATING.md, the
root README and all six per-SDK banners (Go 12+4, TS 9, Ruby 10+1, Swift 10,
Kotlin 6+1, Python 8).

Two `247`s were deliberately NOT touched. Both are as-of facts about a
specific PR, true forever, and rewriting them would have made two correct
sentences false: #648 did leave the inventory at 247 on both sides, and #629
did take it from 241 to 247. Only #679 moved it to 249. Same distinction the
provenance-pin convention draws between a current-value claim and an as-of
one. The third `247`, "every one of the N operations in metadata.json
declares a retry block", IS a current-value claim and did move — verified
that all 249 still declare one rather than assuming it.

The derivation snippet embedded in the guide had the trap that produced a
wrong number here: `--limit 300`. `gh pr list` orders by CREATED, so a
long-open PR that merged late can fall off the end and go silently
uncounted. Raised to 1000 and documented as trap 3, alongside reading from
origin/main rather than HEAD.

SPEC §2 step 5 said Go returns an `error` on a bad `max_pages` and that
"Swift alone is not recoverable". Both false. Go panics
`"basecamp: max pages must be positive"`, and the generated client carries
no MaxPages at all, so there is no other Go path that could return one. Go
and Swift are both non-recoverable, each because the constructor taking the
cap cannot report a failure — Go's NewClient has no error return, Swift's
init is public and non-throwing. §3 step 5 already said Go panics on config
failure, so the paragraph contradicted the document around it. Every SDK's
behaviour was read out of source before this was rewritten; this was the
third factual error in that one paragraph.

Also records what #680 changed there: Python's bool exclusion, and the rule
that a guard feeding a `??` fallback must test `!= null` to match it. Codex
correctly pointed out on #680 that the spec never said whether an explicit
null counts as a supplied cap. Now it does.

MIGRATING.md gains a prose entry for the maxPages validation, stated as NOT
part of the 61: those are breaks a compiler will not catch, class A silent
and class B needing a particular server response, and this is neither — it
fails at construction, deterministically, before any request. The entry is
per-SDK because the six did not start level: Go and Ruby already rejected a
non-positive cap at v0.12.0 and move not at all, and Python's 0 and
negatives already raised, so only its type check is new.

The six PRs that landed after the guide's first draft are recorded rather
than left for a reviewer to reconcile against git log.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Breaking change to public API bug Something isn't working conformance Conformance test suite go kotlin python Pull requests that update the Python SDK ruby Pull requests that update the Ruby SDK spec Changes to the Smithy spec or OpenAPI swift typescript Pull requests that update TypeScript code

Projects

None yet

2 participants