Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,11 @@ For both local and cloud commands, define the clap variant in the appropriate `c

Typed Rust client library for the ClickHouse Cloud API. The library owns all OpenAPI interaction and all cloud integration testing.

- `src/client.rs` — `Client` struct with one async method per OpenAPI operation.
- `src/models.rs` — request/response types matching the spec (see Request and response models below). Model structs, enums and type aliases must live here; it is the only file the drift analyzer inventories.
- `src/convert.rs` — explicit response→request conversions. The analyzer does not parse it, so nothing declared here counts as a model.
- `src/client.rs` — `Client` and shared HTTP machinery; endpoint methods live in private per-domain `src/client/*.rs` files.
- `src/models.rs` — the public model facade and shared discriminated-union macro. Request/response structs, enums, aliases, and their implementations live in private per-domain `src/models/*.rs` files and are re-exported without changing the crate-root or `models::*` paths.
- `src/convert.rs` — `MissingRequiredFields` and conversion documentation; explicit response→request conversions live in private per-domain `src/convert/*.rs` files.

The drift analyzer recursively traverses the private module trees rooted at `client.rs`, `models.rs`, and `meta.rs`. Model declarations remain literal source in that tree; declarations in conversion files do not count as models.

The API library can be updated independently of the CLI. When OpenAPI drifts, prefer updating API library on its own, add to CLI separately.

Expand All @@ -67,7 +69,7 @@ A response must never fail to deserialize because the API dropped a field, sent

- **Request types are strict.** A field the spec requires is `T`; optional or nullable fields are `Option<T>` plus `#[serde(skip_serializing_if = "Option::is_none")]`. The compiler is what enforces "strict in what we send".
- **Response types are all-`Option`.** Every field of every type reachable from a `Client` return type is `Option<T>` plus `skip_serializing_if`. A missing key *and* an explicit JSON `null` both land as `None`, natively — no attribute needed. Nothing is fabricated, so "the server sent `0`" and "the server dropped the field" stay distinguishable, and each caller resolves absence where it is used.
- **`#[serde(default)]` is banned in `models.rs`.** On a required request field it invents `""`/`0`/`false` that a get → edit → write-back caller silently persists; on an `Option` field it is dead weight. Sweeping it across every model field was the superseded policy of issues 312 and 313 — do not reintroduce it.
- **`#[serde(default)]` is banned in the model module tree.** On a required request field it invents `""`/`0`/`false` that a get → edit → write-back caller silently persists; on an `Option` field it is dead weight. Sweeping it across every model field was the superseded policy of issues 312 and 313 — do not reintroduce it.
- **Unknown fields are ignored.** Never `deny_unknown_fields`.

##### Naming and the split
Expand All @@ -82,15 +84,15 @@ Response types keep `derive(Serialize)` — `--json` and `print_human` serialize

##### Write-back conversions

Because the variants are distinct types, a caller that fetches a resource, edits it and writes it back must resolve absence explicitly — that is the point of the split, not an inconvenience of it. `src/convert.rs` owns those conversions: `TryFrom<{Name}Response> for {Name}` where a required request field can be absent, `From` where the conversion is total. A fallible one returns `MissingRequiredFields` (re-exported at the crate root; `.fields()` lists the missing **wire** names). Give each nested object its own conversion so a missing field is named at the level it is missing from, and reuse `MissingRequiredFields` rather than adding a second error type.
Because the variants are distinct types, a caller that fetches a resource, edits it and writes it back must resolve absence explicitly — that is the point of the split, not an inconvenience of it. The owning `src/convert/<domain>.rs` file contains those conversions: `TryFrom<{Name}Response> for {Name}` where a required request field can be absent, `From` where the conversion is total. A fallible one returns `MissingRequiredFields` (re-exported at the crate root; `.fields()` lists the missing **wire** names). Give each nested object its own conversion so a missing field is named at the level it is missing from, and reuse `MissingRequiredFields` rather than adding a second error type.

##### The residual, honestly

A key that is *present* with a changed type still fails. `Option<T>` absorbs absence and `null`, not a string where an array used to be — no more than `serde(default)` did. Enums absorb it through their catch-alls and `discriminated_union!` unions through the `Unknown(Value)` fallback, but a plain struct field does not. Spec conformance is the drift job's responsibility, not the runtime's.

##### How the policy is enforced

`crates/clickhouse-cloud-api/tests/spec_coverage_test.rs` pins all of it against the analyzer's `response_tree()`, which derives response reachability from `client.rs` return types so a newly wired operation is covered automatically:
`crates/clickhouse-cloud-api/tests/spec_coverage_test.rs` pins all of it against the analyzer's `response_tree()`, which derives response reachability from return types across the client module tree so a newly wired operation is covered automatically:

- `every_response_tree_field_is_option` — no non-`Option` field in the tree. Its exception list is empty; an entry needs the same bar as an analyzer exemption. A vacuity guard fails the test if the tree collapses.
- `every_response_tree_option_field_omits_none_when_serialized` — `skip_serializing_if` on every response `Option` field.
Expand All @@ -104,7 +106,7 @@ Scope enforcement to the response tree, never to "every model type": operation-u
ClickHouse Cloud OpenAPI spec: https://api.clickhouse.cloud/v1

- `.github/workflows/openapi-drift.yml` runs `scripts/check-openapi-drift.py` daily. Python owns fetching, issue rendering, and GitHub orchestration only; `python3 scripts/check-openapi-drift.py --dry-run` reproduces the rendered issue without creating one.
- `crates/clickhouse-openapi-analyzer` is the single implementation of parsing and comparison. `rust_inventory.rs` walks and parses the module trees rooted at `client.rs`, `models.rs`, and `meta.rs` with `syn`; module cfg evaluation uses the analyzer host target, excludes `test`, treats feature-gated API as enabled, and conservatively retains unknown custom cfgs. `openapi.rs` inventories the target spec and vendored snapshot; `compare.rs` maps them and emits typed findings; `config.rs` owns ClickHouse-specific policy; `report.rs` defines the stable JSON/text report; `main.rs` is the executable used by Python. Do not duplicate source parsing, exemptions, or comparison logic in tests or Python.
- `crates/clickhouse-openapi-analyzer` is the single implementation of parsing and comparison. `rust_inventory.rs` recursively walks and parses private and public modules rooted at `client.rs`, `models.rs`, and `meta.rs` (including both `<module>.rs` and `<module>/mod.rs`) with `syn`; module cfg evaluation uses the analyzer host target, excludes `test`, treats feature-gated API as enabled, and conservatively retains unknown custom cfgs. `openapi.rs` inventories the target spec and vendored snapshot; `compare.rs` maps them and emits typed findings; `config.rs` owns ClickHouse-specific policy; `report.rs` defines the stable JSON/text report; `main.rs` is the executable used by Python. Do not duplicate source parsing, exemptions, or comparison logic in tests or Python.
- The analyzer is private (`publish = false`) and a dev dependency of `clickhouse-cloud-api`. Parser/tooling dependencies such as `syn` must not enter either published crate's normal dependency graph.
- `crates/clickhouse-cloud-api/tests/spec_coverage_test.rs` analyzes the vendored snapshot; its ignored test analyzes the live spec. Both and the scheduled workflow call the same analyzer and must agree.

Expand All @@ -115,11 +117,11 @@ Work from the issue's typed findings. `spec_pointer` is an RFC 6901 location in
1. Reproduce with `python3 scripts/check-openapi-drift.py --dry-run`. The command does not update the snapshot.
2. Replace `crates/clickhouse-cloud-api/clickhouse_cloud_openapi.json` with the same live document being remediated; do not hand-edit the spec. Snapshot operation/schema findings mean this file is stale.
3. Fix the API library before considering CLI exposure. Follow the finding's pointer and Rust item:
- Missing/extra operations: add or remove the corresponding `Client` method in `client.rs`; only intentional non-OpenAPI helpers belong in `non_openapi_client_methods`.
- Missing models, fields, or extra fields: update public structs/enums/type aliases and Serde names in `models.rs`. An undefined `$ref` (`missing_schema_definition`) is an upstream-spec defect, not a model to invent locally. `models.rs` uses explicit `#[serde(rename = "...")]` wire names exclusively; `rename_all` is rejected by the analyzer parser, because wire vocabulary (Postgres GUCs, SCIM URNs, region IDs, duration literals) cannot be derived from Rust identifiers by any casing rule, and explicit literals keep the code↔spec mapping verbatim and greppable. A new schema needs one Rust type per position it is used in: `{Name}` if the finding is in request position, `{Name}Response` if in response position, both if the spec uses it in both — and the same field added to every variant. See Request and response models above.
- Missing/extra operations: add or remove the corresponding `Client` method in the owning `src/client/<domain>.rs` file; only intentional non-OpenAPI helpers belong in `non_openapi_client_methods`.
- Missing models, fields, or extra fields: update public structs/enums/type aliases and Serde names in the owning `src/models/<domain>.rs` file, then re-export a new type from the `models.rs` facade. An undefined `$ref` (`missing_schema_definition`) is an upstream-spec defect, not a model to invent locally. The model tree uses explicit `#[serde(rename = "...")]` wire names exclusively; `rename_all` is rejected by the analyzer parser, because wire vocabulary (Postgres GUCs, SCIM URNs, region IDs, duration literals) cannot be derived from Rust identifiers by any casing rule, and explicit literals keep the code↔spec mapping verbatim and greppable. A new schema needs one Rust type per position it is used in: `{Name}` if the finding is in request position, `{Name}Response` if in response position, both if the spec uses it in both — and the same field added to every variant. See Request and response models above.
- Optionality: express requiredness in the type, and pick the shape from the position. Request-position fields are `T` when the resolved spec requires them and `Option<T>` plus `skip_serializing_if` otherwise; every response-position field is `Option<T>` plus `skip_serializing_if`, whatever the spec says its requiredness is. Never add `#[serde(default)]`. A request field deliberately optional against the resolved spec needs an `optionality_exemptions` entry keyed on the **request** variant's name.
- Missing/extra enum values: update the typed enum, its Serde wire value, and its `Display` implementation. Preserve data-carrying catch-all variants.
- Beta/deprecation findings: regenerate `BETA_OPERATIONS` with `python3 scripts/regenerate-beta-lists.py` and `DEPRECATED_FIELDS` with `python3 scripts/regenerate-deprecated-fields.py`; deprecated fields also need the matching `#[cfg(feature = "deprecated-fields")]` marker in `models.rs`. The generator works from the spec, which knows nothing about split variants, so a deprecated field on a split schema needs the `{Name}Response` entry and its marker added by hand.
- Beta/deprecation findings: regenerate `BETA_OPERATIONS` with `python3 scripts/regenerate-beta-lists.py` and `DEPRECATED_FIELDS` with `python3 scripts/regenerate-deprecated-fields.py`; deprecated fields also need the matching `#[cfg(feature = "deprecated-fields")]` marker in their model domain file. The generator works from the spec, which knows nothing about split variants, so a deprecated field on a split schema needs the `{Name}Response` entry and its marker added by hand.
- Stale exemption: remove or narrow the configuration entry. Do not change comparison logic to preserve a stale exception.
- Unsupported enum constraint: prefer changing the Rust scalar to a concrete value enum. Acknowledgement is the fallback policy below, not a model-drift fix.
4. Add focused library tests for changed models/methods. A new response type wants a missing-key → `None` and an explicit-`null` → `None` case; a new split pair wants the request variant's strictness and the `TryFrom` write-back asserted. If the unsupported inventory changes, update `acknowledged_unsupported_enum_pointers`; the snapshot test derives its exact expected inventory from that configuration.
Expand Down Expand Up @@ -157,7 +159,7 @@ Enums that the CLI validates against declare `pub const VALUES: &'static [&'stat

##### Deprecated field hiding

Every spec-deprecated request or response field belongs in `meta.rs::DEPRECATED_FIELDS` and carries `#[cfg(feature = "deprecated-fields")]` on the `models.rs` field. It is therefore absent from the public model by default. Request fields that must be gated out but resolve as required are `Option<T>` with a documented optionality exemption. Entries are keyed per Rust type, so a deprecated field on a split schema needs one entry and one marker for `{Name}` and one for `{Name}Response`. Update CLI code that directly accesses or constructs an affected model so both feature configurations compile.
Every spec-deprecated request or response field belongs in `meta.rs::DEPRECATED_FIELDS` and carries `#[cfg(feature = "deprecated-fields")]` on the field in its model domain file. It is therefore absent from the public model by default. Request fields that must be gated out but resolve as required are `Option<T>` with a documented optionality exemption. Entries are keyed per Rust type, so a deprecated field on a split schema needs one entry and one marker for `{Name}` and one for `{Name}Response`. Update CLI code that directly accesses or constructs an affected model so both feature configurations compile.

##### Extending the analyzer

Expand Down
20 changes: 11 additions & 9 deletions crates/clickhouse-cloud-api/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ Typed Rust client for the [ClickHouse Cloud API](https://clickhouse.com/docs/en/

| Path | Purpose |
|------|---------|
| `src/client.rs` | `Client` struct with an async method per API endpoint |
| `src/models.rs` | Request/response types matching the OpenAPI spec |
| `src/client.rs`, `src/client/*.rs` | `Client`, shared HTTP machinery, and per-domain endpoint methods |
| `src/models.rs`, `src/models/*.rs` | Public facade/macro and private per-domain request/response models |
| `src/convert.rs`, `src/convert/*.rs` | Conversion error and per-domain response-to-request conversions |
| `src/error.rs` | Error types (`Http`, `Json`, `Api`) |
| `clickhouse_cloud_openapi.json` | Checked-in copy of the spec (used by tests) |
| `tests/spec_coverage_test.rs` | Thin snapshot/live-spec consumer of the shared drift analyzer |
Expand All @@ -28,7 +29,7 @@ Additional rules:
- **PATCH request schemas** (name contains `Patch` and ends with `Request`) are always all-optional.
- **Nullable fields** (`type: ["string", "null"]` or `oneOf` with null) are always `Option<T>`, even if required.

In `models.rs`, required non-nullable fields use bare types (`T`) and optional/nullable fields use `Option<T>`. All fields keep `#[serde(default)]` so deserialization is tolerant of partial data.
In `src/models/<domain>.rs`, request fields follow those rules: required non-nullable fields use `T`, while optional or nullable fields use `Option<T>`. Every response field uses `Option<T>` plus `skip_serializing_if`, so missing keys and explicit `null` deserialize natively to `None` and absent fields are omitted when serialized. `#[serde(default)]` is banned because it can fabricate required request values and is redundant for `Option` response fields.

### Deprecated fields

Expand All @@ -50,7 +51,7 @@ python3 scripts/regenerate-beta-lists.py
python3 scripts/check-openapi-drift.py --dry-run
```

Field optionality is maintained by hand — edit `models.rs` directly when the drift check flags a mismatch.
Field optionality is maintained by hand. Edit the owning model domain file when the drift check flags a mismatch, and add new domain types to the private module and facade re-exports in `models.rs`.

### Testing

Expand Down Expand Up @@ -88,11 +89,12 @@ All require `CLICKHOUSE_CLOUD_API_KEY`, `CLICKHOUSE_CLOUD_API_SECRET`, `CLICKHOU
Manual `Cloud Integration` dispatches accept `scope=all`, `service`, `postgres`, `organization`, or `clickpipes`. The focused scopes run only their corresponding suite; `clickpipes` runs Postgres CDC plus the fixture-gated smoke test, while `all` runs all four mandatory suites plus that optional smoke test. For full-stack validation, manually run `scope=all` against the top stack branch.

`spec_coverage_test` sends the checked-in sources and snapshot through the
private `clickhouse-openapi-analyzer` crate. That same analyzer powers the
scheduled live-spec issue, so operation, model, field, optionality, beta,
deprecation, enum, snapshot, and stale-exemption findings share one
implementation. The single ignored test runs the same report against the live
spec.
private `clickhouse-openapi-analyzer` crate. The analyzer recursively traverses
the module trees rooted at `client.rs`, `models.rs`, and `meta.rs`, including
private per-domain files. That same analyzer powers the scheduled live-spec
issue, so operation, model, field, optionality, beta, deprecation, enum,
snapshot, and stale-exemption findings share one implementation. The single
ignored test runs the same report against the live spec.

### Optionality exemptions

Expand Down
Loading