Refit OpenAPI drift checks around shared syn analyzer#297
Conversation
Code review findingsReviewed the shared-analyzer refit. Verified locally: analyzer tests + entrypoint parity, 1. 2. Nested-object property enums are attributed to the top-level property — 3. Daily Only #1 has a behavioral consequence (a double-count during actual drift), and it's a small localized fix. Nice single-sourcing overall — the executable-vs-library parity test plus Python-only-consumes-JSON contract match what AGENTS.md promises. |
|
Review findings for head
Verification completed successfully:
The existing Cursor finding about the live test including snapshot drift appears intentional: the live consumer is expected to surface stale snapshot operation/schema findings. |
There was a problem hiding this comment.
Pull request overview
This PR refactors the ClickHouse Cloud OpenAPI drift detection to rely on a single, shared Rust analyzer crate (syn-based) and makes both the snapshot test (spec_coverage_test.rs) and the daily live-spec GitHub-issue workflow (check-openapi-drift.py) consume the same versioned DriftReport.
Changes:
- Adds a private workspace crate (
clickhouse-openapi-analyzer) that inventories Rust + OpenAPI and emits a deterministic, serializable drift report. - Replaces the previous duplicated Rust/Python parsing/comparison logic with thin consumers (Rust test + Python workflow/renderer).
- Updates CI/workflows and documentation to build/test the analyzer and validate issue rendering.
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| scripts/tests/test_check_openapi_drift.py | Adds unit tests for the Python drift script’s report grouping, rendering, and error handling. |
| scripts/resolve-field-requirements.py | Updates comments to clarify the analyzer config is the canonical policy source for partial required schemas. |
| scripts/regenerate-deprecated-fields.py | Updates script guidance to reference analyzer-driven drift reporting. |
| scripts/regenerate-beta-lists.py | Updates script guidance to reference analyzer-driven drift reporting. |
| scripts/check-openapi-drift.py | Reworks the drift workflow script to fetch the live spec, invoke the Rust analyzer, and render a GitHub issue from the shared report. |
| crates/clickhouse-openapi-analyzer/tests/entrypoint_test.rs | Ensures the analyzer binary and library produce identical reports for the vendored snapshot. |
| crates/clickhouse-openapi-analyzer/src/rust_inventory.rs | Implements syn-based Rust source inventory (client methods, model structs/enums/aliases, serde/cfg metadata). |
| crates/clickhouse-openapi-analyzer/src/report.rs | Defines the versioned DriftReport schema and stable sorting/deduping/rendering behavior. |
| crates/clickhouse-openapi-analyzer/src/openapi.rs | Implements OpenAPI inventory (operations, schemas/properties, refs, beta/deprecation markers, enum constraints with stable JSON pointers). |
| crates/clickhouse-openapi-analyzer/src/main.rs | Adds the openapi-drift-analyzer CLI entrypoint used by Python automation. |
| crates/clickhouse-openapi-analyzer/src/lib.rs | Exposes the analyzer library API (analyze) and error types for reuse by tests/automation. |
| crates/clickhouse-openapi-analyzer/src/config.rs | Centralizes ClickHouse-specific comparison policy/exemptions and acknowledged unsupported enum pointers. |
| crates/clickhouse-openapi-analyzer/src/compare.rs | Implements the spec↔code drift comparisons and report generation, including stale-exemption detection. |
| crates/clickhouse-openapi-analyzer/Cargo.toml | Declares the private analyzer crate + dependencies (syn/clap/serde/etc). |
| crates/clickhouse-cloud-api/tests/spec_coverage_test.rs | Replaces bespoke parsing assertions with a thin analyzer invocation for snapshot and live-spec checks. |
| crates/clickhouse-cloud-api/src/meta.rs | Updates docs to reflect drift is now reported by the shared analyzer. |
| crates/clickhouse-cloud-api/README.md | Documents the new analyzer crate, workflow, and commands for drift checks/regeneration. |
| crates/clickhouse-cloud-api/Cargo.toml | Adds the analyzer as a dev-dependency for snapshot/live drift tests. |
| Cargo.toml | Adds the analyzer crate to the workspace members list. |
| Cargo.lock | Locks analyzer dependencies (syn/quote bumps, new crate entries). |
| AGENTS.md | Updates internal developer guidance to describe the analyzer-driven drift workflow and config/exemptions. |
| .github/workflows/test-cloud-api.yml | Expands CI to build/test/clippy the analyzer and run Python drift-renderer tests when relevant paths change. |
| .github/workflows/openapi-drift.yml | Installs Rust toolchain so the scheduled drift workflow can run the analyzer via Cargo. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ 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 7b68dc1. Configure here.
| }; | ||
| } | ||
| } | ||
| EnumContext::Unknown |
There was a problem hiding this comment.
Path item parameter enums unmapped
Medium Severity
The enum_context function doesn't correctly identify enums for path-item level parameters. Its current logic expects parameters to be nested under an HTTP method, but path-item parameters are directly under the path. This causes these enums to be classified as EnumContext::Unknown, preventing them from being properly mapped to client method arguments.
Reviewed by Cursor Bugbot for commit 7b68dc1. Configure here.


Closes #295.
Follow-up model/API remediation for the 11 acknowledged scalar-backed enum constraints is tracked in #296.
Problem
The snapshot coverage test and daily Python workflow independently parsed Rust source and implemented the same code-vs-spec comparisons. That duplicated syntax handling, Serde interpretation, exemptions, and drift semantics in two fragile line/regex-based implementations.
What changed
clickhouse-openapi-analyzerworkspace crate.client.rs,models.rs, andmeta.rsstructurally withsyn.DriftReportwith stable OpenAPI JSON pointers and Rust item locations.spec_coverage_test.rsandcheck-openapi-drift.pyto thin consumers of the same report.Impact
There are no CLI behavior changes or public runtime API changes.
synremains isolated to the private analyzer/dev dependency path. The vendored snapshot produces zero actionable findings and 11 visible acknowledged diagnostics.Verification
cargo build --workspacecargo test --workspacecargo clippy --workspace --all-targets -- -D warningspython3 -m unittest discover -s scripts/tests -p 'test_*.py'