refactor: converge endpoint projections on shared runtime#201
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the repo’s endpoint projections (CLI, REST server, MCP, and SDK surfaces) to run off a shared build-time endpoint model + shared runtime invocation path, reducing duplicated endpoint dispatch logic and tightening cross-SDK consistency.
Changes:
- Introduces
thetadatadx::endpointas the shared endpoint invocation runtime (typed args + generated dispatch) and updates CLI/REST/MCP to use it. - Extends the generated endpoint registry with canonical REST paths and uses those paths for REST routing.
- Hardens SDK/FFI surfaces (Go header dedupe, C++ ABI fixes, Python compile coverage) and expands CI to exercise these surfaces on PRs.
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tools/server/src/state.rs | Removes DirectClient-specific accessor in favor of unified ThetaDataDx. |
| tools/server/src/router.rs | Routes are now registered via EndpointMeta.rest_path from the core registry. |
| tools/server/src/handler.rs | Replaces handwritten endpoint dispatch with shared endpoint runtime invocation. |
| tools/server/src/format.rs | Adds EndpointOutput → terminal JSON envelope conversion. |
| tools/mcp/src/main.rs | Switches MCP tool execution to use the shared endpoint runtime types and invoker. |
| tools/cli/src/main.rs | Replaces 61-endpoint match dispatch with shared runtime invocation + shared arg parsing. |
| sdks/python/src/lib.rs | Adjusts pattern matching to tolerate new struct fields (..). |
| sdks/python/Cargo.lock | Updates locked versions to align with updated core crate deps/versions. |
| sdks/go/thetadx.go | Replaces duplicated C declarations with shared ffi_bridge.h. |
| sdks/go/fpss.go | Replaces duplicated C declarations with shared ffi_bridge.h. |
| sdks/go/client.go | Replaces duplicated C declarations with shared ffi_bridge.h. |
| sdks/go/ffi_bridge.h | Adds a single shared C header for Go CGO bindings. |
| sdks/cpp/src/thetadx.cpp | Updates snapshot ABI usage and migrates Greeks + subscriptions to non-JSON ABI. |
| sdks/cpp/include/thetadx.hpp | Adds subscription types + conversion helper; updates API surface accordingly. |
| sdks/cpp/CMakeLists.txt | Fixes source filename reference for the C++ static library build. |
| crates/thetadatadx/src/registry.rs | Adds rest_path to endpoint metadata and strengthens registry invariants. |
| crates/thetadatadx/src/lib.rs | Exposes endpoint module and re-exports shared endpoint runtime types. |
| crates/thetadatadx/src/endpoint.rs | Renames MCP bridge into a general endpoint runtime; adds raw parsing helpers. |
| crates/thetadatadx/src/direct.rs | Moves large endpoint declarations to build-time generated includes. |
| crates/thetadatadx/build.rs | Generates registry, endpoint runtime dispatch, and DirectClient endpoint declarations from one model. |
| .github/workflows/python.yml | Adds PR/main smoke builds; gates wheel/sdist builds to tags/workflow_dispatch. |
| .github/workflows/ci.yml | Adds an “Extended Surfaces” job to exercise MCP/CLI/server/Python/Go/C++ on PRs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| &ticks, | ||
| ))) | ||
| .into_response(); | ||
| } |
There was a problem hiding this comment.
format=csv currently falls back to an empty CSV for list endpoints because json_to_csv requires the first response element to be an object (as_object()), but list endpoints return response: ["AAPL", ...] (scalars). If CSV is intended to work for list endpoints too, consider handling scalar arrays by emitting a single-column CSV (e.g., header value/symbol/date) instead of returning an empty body.
| } | |
| } | |
| if items.iter().all(|item| !item.is_object() && !item.is_array()) { | |
| let escape_csv = |value: &str| { | |
| if value.contains([',', '"', '\n', '\r']) { | |
| format!("\"{}\"", value.replace('"', "\"\"")) | |
| } else { | |
| value.to_owned() | |
| } | |
| }; | |
| let mut csv = String::from("value\n"); | |
| for item in &items { | |
| let field = if let Some(s) = item.as_str() { | |
| s.to_owned() | |
| } else { | |
| sonic_rs::to_string(item).unwrap_or_default() | |
| }; | |
| csv.push_str(&escape_csv(&field)); | |
| csv.push('\n'); | |
| } | |
| return ( | |
| StatusCode::OK, | |
| [("content-type", "text/csv; charset=utf-8")], | |
| csv, | |
| ) | |
| .into_response(); | |
| } |
There was a problem hiding this comment.
Fixed in b28955f. json_to_csv now supports scalar response arrays by emitting a single-column value CSV, and string fields are properly escaped. Added server tests for scalar lists, object rows, and mixed-shape rejection.
Closes #200
Summary
Verification