Add bidirectional enum-value drift detection (#275)

[sdairs]

Important

Reviewer guidance: review the model fixes, skip the drift-checker methodology

This PR is still intended to merge because it contains valid ClickHouse Cloud model corrections described below. However, the Rust/Python drift-checker implementation introduced here is temporary and will be completely replaced by #297 with one shared syn-based analyzer.

Please review in this PR: the enum/model corrections, regenerated values, compatibility impact, and regression coverage.

Please do not spend time reviewing in depth: the line-oriented Rust parsing, Python regex parsing, spec-to-Rust mapping architecture, or duplicated comparison logic in spec_coverage_test.rs and check-openapi-drift.py. Review that methodology in #297 instead.

Merge order: merge #290 first, then rebase/retarget #297 onto main.

Closes #275.

Stacked on #288 — this branch is based on issue-287-openapi-drift and reuses its refreshed spec snapshot. Merge #288 first; this PR's diff will then shrink to its own commit.

Problem

Neither scripts/check-openapi-drift.py nor spec_coverage_test.rs compared the values of spec enum: [...] arrays against Rust enum variants, so a value added to or removed from a spec enum was invisible to both. Real incident (#273): the spec removed "snapshot" from the PubSub seekType enum, the stale Snapshot variant kept serializing a value the API rejects, and the drift check reported 0.

Changes reviewers should review

The new checks surfaced real model drift, fixed in this PR:

• 29 missing values: europe-west2 in 8 region enums, GCP_PSC_SERVICE_ATTACHMENT in both reverse-private-endpoint type enums, 6 new threshold types in the 3 ClickStack alert enums, and duration in ClickStackNumberFormatOutput. These were added with their Display arms.
• 66 extra PgSize variants: instance sizes the spec no longer lists were removed by regenerating the enum from the spec.
• Breaking library impact: the removed PgSize variants are no longer public, and the Default moves from the removed c6gd.medium to c6gd.large. The CLI is unaffected because parse_pg_size uses Serde strings.
• Regression coverage demonstrates both directions of enum-value drift, including the Resolve OpenAPI drift: slow query patterns, Postgres metrics, RPE update (#224) #273 seekType/snapshot failure mode.

Temporary drift implementation — superseded by #297

This PR adds bidirectional enum-value coverage to both the Rust snapshot test and Python daily workflow:

• spec → code: catches spec values with no matching Rust variant.
• code → spec: catches Rust variants that serialize values no longer accepted by the spec.

Those protections and characterization cases are useful, but the implementation extends the existing duplicated parsing approach. #297 replaces the methodology with one private Rust analyzer that:

• parses Rust structurally with syn;
• owns mapping, exemptions, and all comparison semantics once;
• emits one typed DriftReport consumed by both the snapshot test and daily workflow; and
• inventories nested, parameter, composition, and unsupported enum constraints explicitly.

Historical implementation details in this PR (not the design to review)

• Inline property enums resolve through the struct field's declared type, stripped of Option/Vec/Box; named enum schemas resolve through pascalize_identifier.
• Wire values come from #[serde(rename = "...")], falling back to the variant identifier.
• Untagged catch-alls are excluded by attribute rather than name.
• Non-untagged data-carrying enums, numeric enums, and items-level enums are excluded.
• EXTRA_ENUM_VALUE_EXEMPTIONS mirrors the existing extra-field exemption mechanism.

These mechanics are replaced, not evolved, by #297.

Verification

• cargo test --workspace
• cargo clippy --workspace --all-targets with zero warnings
• cargo build --features deprecated-fields
• Drift-script dry run against the snapshot: zero findings, matching the Rust tests
• Regression drills for added values, removed values, non-enum mappings, and stale exemptions

🤖 Generated with Claude Code

[iskakaushik]

LGTM

[iskakaushik]

LGTM

[macroscopeapp]

Macroscope has since reviewed this pull request. An earlier review was skipped by a cost limit; a review has now completed, so that notice no longer applies.

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit be4123e. Configure here.}

[rndD]

LGTM

[macroscopeapp]

🟡 Medium src/openapi.rs:169

required_non_nullable is computed only from the property's inline shape, so a required property that uses a $ref to a nullable schema is incorrectly recorded as non-nullable. This causes compare_fields to report a false optionality mismatch when the Rust side correctly models the property as Option<T>. is_nullable would need to resolve $ref and composition keywords like anyOf to detect nullability through references.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @crates/clickhouse-openapi-analyzer/src/openapi.rs around line 169:

`required_non_nullable` is computed only from the property's inline shape, so a required property that uses a `$ref` to a nullable schema is incorrectly recorded as non-nullable. This causes `compare_fields` to report a false optionality mismatch when the Rust side correctly models the property as `Option<T>`. `is_nullable` would need to resolve `$ref` and composition keywords like `anyOf` to detect nullability through references.

[macroscopeapp]

🟡 Medium src/openapi.rs:80

When an OpenAPI document declares an enum-constrained parameter at the path-item level (/paths/{path}/parameters/...), the analyzer reports an UnsupportedEnumConstraint instead of comparing the enum against the client argument. The issue is in collect_enums: it recurses into path-item parameters but enum_context only classifies pointers shaped like /paths/{path}/{method}/parameters/..., so the path-item-level pointer falls into EnumContext::Unknown. To fix this, extend the pointer classification in enum_context (or its caller) to recognize parameters directly under a path item and map it to the same context as operation-level parameters.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @crates/clickhouse-openapi-analyzer/src/openapi.rs around line 80:

When an OpenAPI document declares an enum-constrained parameter at the path-item level (`/paths/{path}/parameters/...`), the analyzer reports an `UnsupportedEnumConstraint` instead of comparing the enum against the client argument. The issue is in `collect_enums`: it recurses into path-item `parameters` but `enum_context` only classifies pointers shaped like `/paths/{path}/{method}/parameters/...`, so the path-item-level pointer falls into `EnumContext::Unknown`. To fix this, extend the pointer classification in `enum_context` (or its caller) to recognize `parameters` directly under a path item and map it to the same context as operation-level parameters.

[macroscopeapp]

🟡 Medium src/openapi.rs:84

collect_refs traverses the entire OpenAPI document, including non-schema locations such as example, default, and vendor extensions. A literal $ref appearing inside one of those data payloads (e.g. "#/components/schemas/Fake") is incorrectly inventoried as a schema reference, producing a false MissingSchemaDefinition drift finding. Consider restricting traversal to the same genuine JSON Schema positions that collect_enums walks, so non-schema content is never inspected.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @crates/clickhouse-openapi-analyzer/src/openapi.rs around line 84:

`collect_refs` traverses the entire OpenAPI document, including non-schema locations such as `example`, `default`, and vendor extensions. A literal `$ref` appearing inside one of those data payloads (e.g. `"#/components/schemas/Fake"`) is incorrectly inventoried as a schema reference, producing a false `MissingSchemaDefinition` drift finding. Consider restricting traversal to the same genuine JSON Schema positions that `collect_enums` walks, so non-schema content is never inspected.

[macroscopeapp]

🟡 Medium src/openapi.rs:436

$ref request bodies are silently skipped during schema collection, so enum constraints inside referenced request-body schemas are never found. When an operation's requestBody is a $ref to components/requestBodies, collect_content_owner_schemas only checks for an inline content key, finds none, and returns — and component request bodies are not traversed anywhere else. Consider resolving $ref values in collect_content_owner_schemas (or before calling it) so referenced request-body schemas are traversed.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @crates/clickhouse-openapi-analyzer/src/openapi.rs around line 436:

`$ref` request bodies are silently skipped during schema collection, so enum constraints inside referenced request-body schemas are never found. When an operation's `requestBody` is a `$ref` to `components/requestBodies`, `collect_content_owner_schemas` only checks for an inline `content` key, finds none, and returns — and component request bodies are not traversed anywhere else. Consider resolving `$ref` values in `collect_content_owner_schemas` (or before calling it) so referenced request-body schemas are traversed.

[macroscopeapp]

🟡 Medium src/compare.rs:452

map_enum reports a named schema as unsupported and skips enum comparison when the Rust model is a public type alias (e.g. pub type Choice = ChoiceValue). It looks up the pascalized schema name directly in rust.enums, but Choice is an alias, not an enum, so it produces "Rust type Choice is not an enum" even though ChoiceValue is a valid value enum. Properties and parameters resolve aliases via terminal_type, but the NamedSchema branch skips that resolution. Consider resolving the type name through terminal_type before the rust.enums lookup.

     let (type_name, rust_item) = match &constraint.context {
         EnumContext::NamedSchema { schema } => {
             let name = pascalize(schema);
             if !rust.model_types.contains(&name) {
                 return EnumMapping::Unmapped;
             }
-            (Some(name.clone()), format!(models.rs::{name}"))
+            (
+                rust.terminal_type(&TypeNode::Path(name.clone())),
+                format!("models.rs::{name}"),
+            )
         }

Also found in 1 other location(s)

crates/clickhouse-openapi-analyzer/src/rust_inventory.rs:249

collect_models records type aliases, but optionality checks call field.rust_type.is_option() without resolving those aliases. For example, pub type MaybeName = Option<String>; pub struct Model { pub name: MaybeName } is inventoried as a non-optional field, so the drift analyzer emits a false optionality mismatch (or misses the inverse mismatch) for valid alias-backed optional fields.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @crates/clickhouse-openapi-analyzer/src/compare.rs around lines 452-459:

`map_enum` reports a named schema as unsupported and skips enum comparison when the Rust model is a public type alias (e.g. `pub type Choice = ChoiceValue`). It looks up the pascalized schema name directly in `rust.enums`, but `Choice` is an alias, not an enum, so it produces "Rust type Choice is not an enum" even though `ChoiceValue` is a valid value enum. Properties and parameters resolve aliases via `terminal_type`, but the `NamedSchema` branch skips that resolution. Consider resolving the type name through `terminal_type` before the `rust.enums` lookup.

Also found in 1 other location(s):
- crates/clickhouse-openapi-analyzer/src/rust_inventory.rs:249 -- `collect_models` records type aliases, but optionality checks call `field.rust_type.is_option()` without resolving those aliases. For example, `pub type MaybeName = Option<String>; pub struct Model { pub name: MaybeName }` is inventoried as a non-optional field, so the drift analyzer emits a false optionality mismatch (or misses the inverse mismatch) for valid alias-backed optional fields.