fix: harden SDK CI and preserve MCP wildcard fields#204
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens CI coverage across the Rust core and SDK surfaces, updates the Python packaging flow to build/validate abi3 wheels and sdists during PRs, and adjusts MCP tick serialization to preserve wildcard option contract identifiers for bulk responses.
Changes:
- MCP: emit
expiration,strike, and human-readablerightfor wildcard/bulk option tick rows while omitting them for single-contract rows. - Python SDK/CI: switch to abi3 (py39+) packaging with PR-time wheel + sdist build/install/import smoke checks; align metadata by sourcing version dynamically and adding missing Cargo package metadata.
- Go/C++/CI/docs: tighten platform-specific build/link settings and update CI workflows (protoc installation, timeouts, concurrency, stricter release behavior).
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/mcp/src/main.rs | Adds helpers + tests to include wildcard option contract identifiers in serialized tick rows. |
| sdks/README.md | Documents the SDK validation matrix and updates build commands (Python/C++). |
| sdks/python/README.md | Documents abi3 wheels and pins the recommended maturin version range. |
| sdks/python/pyproject.toml | Switches Python version to dynamic (maturin/Cargo-sourced) and pins maturin in build-system. |
| sdks/python/Cargo.toml | Adds missing Cargo package metadata and switches PyO3 to abi3-py39. |
| sdks/go/thetadx.go | Uses platform-specific CGO LDFLAGS for Linux/macOS/Windows. |
| sdks/go/README.md | Clarifies Go SDK platform support and Windows artifact expectations. |
| sdks/cpp/README.md | Updates CMake build instructions and example run path. |
| sdks/cpp/include/thetadx.h | Makes 64-byte alignment portable across MSVC vs GCC/Clang. |
| sdks/cpp/CMakeLists.txt | Uses an imported target for platform-correct FFI linking (incl. Windows import libs). |
| .github/workflows/python.yml | Adds PR-time wheel/sdist validation across OSes + abi3 smoke checks on multiple Python versions. |
| .github/workflows/docs.yml | Updates GitHub Actions versions for docs build/deploy. |
| .github/workflows/ci.yml | Adds concurrency/timeouts, replaces setup-protoc with a composite action, expands SDK binding checks, and tightens publish behavior. |
| .github/actions/install-protoc/action.yml | New composite action to install protoc on Linux/macOS/Windows runners. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - uses: Swatinem/rust-cache@v2 | ||
| - uses: ./.github/actions/install-protoc | ||
| - shell: bash | ||
| run: python -m pip install --upgrade pip maturin |
There was a problem hiding this comment.
The workflow installs the latest maturin (pip install --upgrade ... maturin), but the repo pins maturin>=1.9.4,<2.0 in sdks/python/pyproject.toml. Installing an unbounded version makes CI non-reproducible and can break unexpectedly when maturin 2.x releases; consider installing with the same version range here (and in the other jobs in this workflow).
| run: python -m pip install --upgrade pip maturin | |
| run: python -m pip install --upgrade pip "maturin>=1.9.4,<2.0" |
| - uses: ./.github/actions/install-protoc | ||
| - shell: bash | ||
| run: python -m pip install --upgrade pip maturin | ||
| - shell: bash | ||
| run: python -m maturin build --release -m sdks/python/Cargo.toml -o dist/ --interpreter python |
There was a problem hiding this comment.
Same issue as the build job: this installs unpinned maturin, which can drift from the maturin>=1.9.4,<2.0 constraint in sdks/python/pyproject.toml. Pin the installed maturin version range here to keep the ABI3 smoke checks reproducible.
| - uses: Swatinem/rust-cache@v2 | ||
| - uses: ./.github/actions/install-protoc | ||
| - shell: bash | ||
| run: python -m pip install --upgrade pip maturin | ||
| - shell: bash | ||
| run: python -m maturin sdist -m sdks/python/Cargo.toml -o dist/ |
There was a problem hiding this comment.
The sdist job installs the latest maturin, but sdks/python/pyproject.toml constrains maturin>=1.9.4,<2.0. Pinning the installed version range here helps avoid future CI breakages when new maturin majors are released.
| # From source (requires Rust toolchain) | ||
| # From source (requires Rust toolchain and maturin 1.9.4+) | ||
| cd sdks/python | ||
| pip install maturin |
There was a problem hiding this comment.
This section says building from source requires maturin 1.9.4+, but the command shown is pip install maturin (unbounded). To avoid users accidentally picking up an incompatible maturin major (e.g., 2.x), align the command with the pinned range used elsewhere (e.g., maturin>=1.9.4,<2.0).
| pip install maturin | |
| pip install "maturin>=1.9.4,<2.0" |
| shell: bash | ||
| run: | | ||
| set -euo pipefail | ||
| brew update |
There was a problem hiding this comment.
Running brew update during CI tends to add significant time and is a common source of transient failures on GitHub-hosted macOS runners. Consider removing brew update (or replacing it with a less disruptive approach) and just installing protobuf to make protoc setup faster and more reliable.
| brew update |
Summary
Details
arduino/setup-protocaction with a local composite protoc installerexpiration,strike, and human-readablerightfields in MCP output for wildcard option tick rows while omitting them for single-contract rowsValidation
cargo fmt --allcargo test -p thetadatadx --lockedcargo clippy -p thetadatadx --locked -- -D warningscargo test --manifest-path tools/mcp/Cargo.toml --lockedcargo clippy --manifest-path tools/mcp/Cargo.toml --locked -- -D warningscargo test --manifest-path tools/server/Cargo.toml --lockedcargo clippy --manifest-path tools/server/Cargo.toml --locked -- -D warningscargo test --manifest-path tools/cli/Cargo.toml --lockedcargo clippy --manifest-path tools/cli/Cargo.toml --locked -- -D warningscargo check --manifest-path sdks/python/Cargo.toml --lockedmaturinmaturincargo build --release -p thetadatadx-ffi --lockedgo test ./...insdks/gocmake -S sdks/cpp -B build/cpp && cmake --build build/cpp --config Release --target thetadatadx_cppNotes