Adopt tolerant-response deserialization: sweep serde(default) and codify the policy

[sdairs]

Closes #312

Stacked on #311 — base is issue-308-clickstack-drift; retarget to main after #311 merges.

Adopts "strict in what we send, liberal in what we accept" across clickhouse-cloud-api: every model field now carries #[serde(default)], so a server-dropped response field degrades to a default instead of failing the whole response. Requests stay strict via the type system (T vs Option<T> is unchanged, so analyzer drift detection is unaffected).

What's in here (one commit per step)

• ClickStackAlertChannel → discriminated_union! dispatch on type (email/webhook), with its variants' strict fields defaulted in the same commit (defaulting a distinguishing field while the union is still untagged would misroute payloads — that ordering constraint holds throughout this PR).
• Six Builder-vs-RawSql chart-config sub-unions → discriminated_union! via a new key-absence arm: RawSql payloads carry configType: "sql" (note: the issue body said "rawSql"; the spec and existing tests say "sql"), Builder payloads lack the key entirely. Absent and present-but-non-string keys both dispatch to Builder — deliberate and tested. The absence arm takes a mandatory disqualifying-key guard (none unless "connectionId" | "sqlTemplate") so a RawSql body whose configType the server stops sending lands losslessly in Unknown instead of silently becoming an empty builder tile.
• Analyzer: rust_inventory.rs now tracks serde(default) (bare and default = "path" forms), exposed via a narrow model_fields_missing_serde_default() — no new FindingKind, no report schema_version change, no Python changes, and syn stays out of the published crate's dependency graph.
• The sweep: #[serde(default)] on every remaining strict field (~365 fields across ClickStack, SCIM, ClickPipes, Postgres, quotas, RBAC) plus 12 bare Option fields, enforced by a new every_model_field_carries_serde_default test in spec_coverage_test.rs with an empty exception list.
• Policy docs: AGENTS.md's unexplained "Retain #[serde(default)]" line replaced with the full policy and rationale (so future review-bot "serde(default) masks required field" findings can be answered by citation); crate docs gain a "Tolerant deserialization" section including the GET→write-back caveat.

Semantics decided during adversarial review

An adversarial review pass confirmed three real gaps, fixed in the final commit:

1. Known-discriminator payload errors now fall back to Unknown(value) instead of hard-failing. Previously one malformed element (e.g. a field whose type changed server-side) failed an entire list response — exactly the outage mode this PR exists to close. Unknown is now reachable via unrecognized discriminator values or undeserializable payloads, both round-tripping losslessly; the macro docs and tests pin both routes.
2. RawSql-without-configType misroute — fixed by the none unless guard described above.
3. ClickStackAlertChannel::default() round-tripped to the wrong variant (ClickStackAlertChannelEmailType's #[default] sat on Webhook because the spec gives both variants the same two-value enum). #[default] moved to Email; a new cross-union invariant test asserts every discriminated_union! enum's Default round-trips to the same variant.

Residual (documented, intentional): a plain struct field whose type changes server-side still fails the response — #[serde(default)] only covers missing keys. Explicit JSON null on Vec fields also remains an error (out of scope per the issue).

Verification

Every commit is independently green; on the final tree: cargo test --workspace (0 failures), cargo clippy --workspace --all-targets -- -D warnings, cargo fmt --all --check, cargo check --workspace --all-features, python3 -m unittest discover -s scripts/tests, and python3 scripts/check-openapi-drift.py --dry-run confirms the sweep is analyzer-neutral.

🤖 Generated with Claude Code

[sdairs]

Reviewed with focus on the idea, the policy, the standard, and the structure. Overall: approve — the policy is coherent, honestly documented (including what it does not cover), and enforced structurally via the empty-exception-list every_model_field_carries_serde_default test rather than by convention. Suggestions, in priority order:

1. The none unless guard lists are a hand-maintained invariant with doc-only enforcement. The macro docs say "If the spec ever gives the absence variant one of the guard keys, drop that key from the list" — but nothing fails if that drifts. If a Builder schema ever gains connectionId, those payloads silently start landing in Unknown. The analyzer inventory already knows every struct's wire fields; a check that each guard key is absent from the absence-variant's fields and present on some other variant's would make this structural, in the same spirit as the VALUES const check. Suggest a follow-up issue.

2. The assert_default_round_trips! union list in models_test.rs is also hand-maintained ("Add new unions with a Default impl here"). A new discriminated_union! with a Default won't fail anything if it's forgotten there. Same remedy as (1), lower priority since the failure it guards is rarer.

3. Minor perf nit: deserialize_or_raw clones the full Value on every call, including the success path. &Value implements Deserializer, so T::deserialize(&value) would avoid the clone and keep the original for the error path. Only matters for large dashboard payloads; fine to leave.

4. Debuggability note: deserialize_or_raw discards the serde error entirely, so when a payload-shape mismatch lands in Unknown, nothing records why. Lossless round-trip + Display showing the raw JSON makes this acceptable — but if support tickets ever hinge on "why is this tile Unknown," a debug!-level hook is where it would go.

5. Non-blocking observation on the policy's cost: failure moves from loud to silent — a dropped id on a list response is now "" flowing into follow-up calls rather than a deserialize error. The docs acknowledge this and the daily drift job bounds exposure to ~one day; just worth remembering that the drift job is now the policy's correctness backstop, not merely hygiene, so its cadence shouldn't be relaxed later.

(5) needs no action; (3) and (4) are take-or-leave; (1) and (2) are worth follow-up issues.

[sdairs]

I read the adversarial review above and checked its three findings against the code. It's technically accurate on all three — but I disagree with the severity ratings and the proposed architecture. My take:

[P2, lib.rs:46–48] — correct, easy fix, should block. "Bounded by the daily OpenAPI drift job" overstates the safety net: the analyzer compares spec ↔ Rust source, so a deployed server omitting a still-spec-required field is invisible to it — precisely the failure mode this PR masks. Weaken the sentence to scope the bound to spec drift and admit server-behavior drift is only covered by integration tests. Honest docs, no architecture change.

[P1, ScimUser.active] — directionally right, overstated, and the fix already exists in this PR. active is spec-required, so the default only fires on anomalous/partial representations. But false is a semantic default, not an inert one: if user.active { allow() } gets a fabricated deny (fail-closed for SCIM, but the class critique holds). The PR's own documented escape hatch — "if defaulting a specific field would be misleading, make it Option<T> with an optionality_exemptions entry" — was written but never applied. Suggest a targeted audit of semantic defaults (deny/enable flags, timestamps, quota numerics — maybe a dozen fields) rather than a blanket redesign.

[P1, models_test.rs:4102] — the test is mislabeled, not wrong. ClickPipePostPubSubSource is indeed only reachable via ClickPipePostRequest, and "dropped response fields" is the wrong frame. But the test asserts the crate's public Deserialize contract (sparse JSON deserializes; serialization unchanged), which is legitimately tested where it lives. Rename to something direction-neutral (tolerates_sparse_json) and fix the comment.

Where I part ways with the review:

• "The type system does not enforce required inputs" is strictly true and practically false. Every in-tree caller builds requests with struct literals naming all required fields (ClickPipePostRequest { name, source, ..Default::default() } — compiler-enforced). The real hole is the ..Default::default() spread plus defaults on leaf credentials — and that pre-dates this PR: main today sends {username: "", password: ""} via ClickPipeKafkaSchemaRegistryCredentials::default() (commands.rs:1170) when --schema-registry-url is passed without credentials. That's a request-construction concern that strict deserialization (the review's option a) would not fix at all.
• The serde mechanism gap the review demonstrates (from_str::<ClickPipePostRequest>("{}") now succeeds) is real but second-order: nothing in this repo deserializes request bodies, and the caveat is already documented in lib.rs. It affects hypothetical downstream consumers of the published crate, bounded by semver + docs.
• "Classify models by request/response reachability" is heavier than it looks. ClickStackSource is the only shared GET/POST model in this surface, and a send-time validator must re-derive the wire requiredness contract (which already diverges from the type contract via skip_serializing_if = "Vec::is_empty"). That's a larger diff than this PR, protecting a caller pattern that doesn't exist in-tree.

Suggested path — approve with two blocking items, not request-changes:

1. Fix the lib.rs bound sentence (blocking, trivial).
2. Apply the existing exemption policy to ScimUser.active + audit the small class of semantic defaults (blocking, small).
3. Rename the PubSub test and fix its comment (non-blocking).
4. File a follow-up issue for Default::default() spreads + leaf-credential defaults as a request-construction concern — explicitly out of scope here; it needs Option-typed credentials or a build-time validate(), owned by the ClickPipe request builders.

The review's north star (responses usable without inventing facts; incomplete requests unsendable; missing data distinguishable from real false/0) is right, and this PR achieves the first leg cleanly. The request-side gap is a pre-existing construction hole, not a regression — blocking a well-tested response-tolerance fix on a request-architecture redesign lets the perfect be the enemy of the good.