Skip to content

Commit 778e52a

Browse files
committed
fix: feature matrix — grpc + lab combinations were broken
Post-PR-#238 audit (user's "Careful" warning) surfaced three latent bugs that --features serve tests alone couldn't catch: 1. --features grpc ALONE failed to compile — wire.rs was `serve`-only gated but codec_research.rs (gated on any(serve, grpc)) imported wire types unconditionally. Pre-existing architectural gap from before this session; latent because no CI path exercised grpc-only. Fix: widen wire.rs + auto_detect.rs + codec_kernel_cache.rs + rotation_kernel.rs + decode_kernel.rs + token_agreement.rs gates from `serve` to `any(serve, grpc)`. Both transports share the same DTO surface; both need wire compiled. 2. --features grpc missing serde/serde_json/base64/bytemuck deps. wire.rs needs them (serde derives on DTOs; serde_json in tests; base64 for WireTensorView decode; bytemuck for lane casting). Previously only `serve` pulled them. Fix: new internal feature `_lab-dtos` that both serve + grpc pull in. Keeps the dependency sharing explicit: _lab-dtos = ["dep:serde", "dep:serde_json", "dep:base64", "dep:bytemuck"] serve = ["_lab-dtos", "dep:axum", "dep:tokio"] grpc = ["_lab-dtos", "dep:prost", "dep:tonic", "dep:tonic-build", "dep:tokio"] 3. --features lab failed with E0063 at grpc.rs:210 — WireCalibrateRequest struct-literal was missing the `params` + `tensor_view` fields I added in D0.1 (PR #227). Another latent bug: my --features serve tests didn't exercise grpc.rs. Fix: add `params: None, tensor_view: None` explicitly, with a comment noting the gRPC path uses legacy num_* fields only until the proto schema catches up (D0.3b follow-up). 4. Bonus — 3x redundant closures in grpc.rs that clippy only caught under `--features lab`: .map_err(|e| Status::invalid_argument(e)) → .map_err(Status::invalid_argument) Rust 1.95 match-ergonomics check: grep'd `mut ref` + `ref mut` in struct pattern field shorthand across cognitive-shader-driver/src. Zero hits — no Rust 1.95 breakage lurking. Verification (all passing now): cargo check (default) ✓ cargo check --features serve ✓ cargo check --features grpc ✓ cargo check --features lab ✓ cargo test --features lab --lib: 117/117 pass cargo test --features serve --lib: 117/117 pass cargo test --lib (default): 39/39 pass cargo clippy --features lab -- -D warnings: CLEAN cargo clippy --features serve -- -D warnings: CLEAN The session's 16 PRs all ran under --features serve only; this commit is the rescue that proves the other feature combinations compile clean too. Future sessions should exercise the full feature matrix before declaring any DTO change complete. https://claude.ai/code/session_01SbYsmmbPf9YQuYbHZN52Zh
1 parent a1e5dec commit 778e52a

3 files changed

Lines changed: 23 additions & 11 deletions

File tree

crates/cognitive-shader-driver/Cargo.toml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,12 @@ tonic-build = { version = "0.12", optional = true }
6868
default = []
6969
with-engine = ["dep:thinking-engine"]
7070
with-planner = ["dep:lance-graph-planner"]
71-
serve = ["dep:serde", "dep:serde_json", "dep:axum", "dep:tokio", "dep:base64", "dep:bytemuck"]
72-
grpc = ["dep:prost", "dep:tonic", "dep:tonic-build", "dep:tokio"]
71+
# Shared LAB DTOs — `wire.rs` + `auto_detect.rs` + codec kernel scaffolds
72+
# + token_agreement use these regardless of whether the transport is REST
73+
# (serve) or gRPC (grpc). Both features pull this set.
74+
_lab-dtos = ["dep:serde", "dep:serde_json", "dep:base64", "dep:bytemuck"]
75+
serve = ["_lab-dtos", "dep:axum", "dep:tokio"]
76+
grpc = ["_lab-dtos", "dep:prost", "dep:tonic", "dep:tonic-build", "dep:tokio"]
7377

7478
# `lab` — umbrella switch for the single shader-lab binary. Enables every
7579
# endpoint (REST + gRPC), the planner bridge, the thinking-engine bridge,

crates/cognitive-shader-driver/src/grpc.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ impl CognitiveShaderService for ShaderGrpcService {
185185
route_filter: if req.route_filter.is_empty() { None } else { Some(req.route_filter) },
186186
};
187187
let r = crate::codec_research::list_tensors(&wire_req)
188-
.map_err(|e| Status::invalid_argument(e))?;
188+
.map_err(Status::invalid_argument)?;
189189
Ok(Response::new(pb::TensorsResponse {
190190
total: r.total as u32,
191191
shown: r.shown as u32,
@@ -210,14 +210,19 @@ impl CognitiveShaderService for ShaderGrpcService {
210210
let wire_req = crate::wire::WireCalibrateRequest {
211211
model_path: req.model_path,
212212
tensor_name: req.tensor_name,
213+
// D0.1 extension fields — gRPC path uses legacy num_*
214+
// fields only; the richer CodecParams + TensorView path is
215+
// REST-only until the proto schema catches up (D0.3b).
216+
params: None,
217+
tensor_view: None,
213218
num_subspaces: if req.num_subspaces == 0 { 6 } else { req.num_subspaces as usize },
214219
num_centroids: if req.num_centroids == 0 { 256 } else { req.num_centroids as usize },
215220
kmeans_iterations: if req.kmeans_iterations == 0 { 20 } else { req.kmeans_iterations as usize },
216221
max_rows: if req.max_rows == 0 { None } else { Some(req.max_rows as usize) },
217222
icc_samples: if req.icc_samples == 0 { 512 } else { req.icc_samples as usize },
218223
};
219224
let r = crate::codec_research::calibrate_tensor(&wire_req)
220-
.map_err(|e| Status::invalid_argument(e))?;
225+
.map_err(Status::invalid_argument)?;
221226
Ok(Response::new(pb::CalibrateResponse {
222227
tensor_name: r.tensor_name,
223228
dims: r.dims,
@@ -248,7 +253,7 @@ impl CognitiveShaderService for ShaderGrpcService {
248253
icc_samples: if req.icc_samples == 0 { 512 } else { req.icc_samples as usize },
249254
};
250255
let r = crate::codec_research::row_count_probe(&wire_req)
251-
.map_err(|e| Status::invalid_argument(e))?;
256+
.map_err(Status::invalid_argument)?;
252257
Ok(Response::new(pb::ProbeResponse {
253258
tensor_name: r.tensor_name,
254259
n_rows: r.n_rows as u32,

crates/cognitive-shader-driver/src/lib.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,37 +112,40 @@ pub mod sigma_rosetta;
112112
// reachable through the canonical bridge.
113113

114114
// Per-op Wire DTOs (REST + protobuf). LAB-ONLY.
115-
#[cfg(feature = "serve")]
115+
// Gated on `any(serve, grpc)` because both transports share the same
116+
// DTOs; gRPC consumers (grpc.rs) and REST consumers (serve.rs) both
117+
// convert to/from the `Wire*` types in wire.rs.
118+
#[cfg(any(feature = "serve", feature = "grpc"))]
116119
pub mod wire;
117120

118121
// D0.5 — model architecture auto-detection from config.json.
119122
// CODING_PRACTICES.md gap 1 remediation. LAB-ONLY.
120-
#[cfg(feature = "serve")]
123+
#[cfg(any(feature = "serve", feature = "grpc"))]
121124
pub mod auto_detect;
122125

123126
// D1.1 — JIT kernel cache keyed by CodecParams::kernel_signature().
124127
// Structural layer; actual Cranelift IR emission defers to D1.1b. LAB-ONLY.
125-
#[cfg(feature = "serve")]
128+
#[cfg(any(feature = "serve", feature = "grpc"))]
126129
pub mod codec_kernel_cache;
127130

128131
// D1.2 — rotation primitives (Identity / Hadamard / OPQ-stub). LAB-ONLY.
129132
// Hadamard is real (in-place butterfly); OPQ is stub pending D1.1b's
130133
// ndarray::hpc::jitson_cranelift::JitEngine adapter + matrix-blob loader.
131-
#[cfg(feature = "serve")]
134+
#[cfg(any(feature = "serve", feature = "grpc"))]
132135
pub mod rotation_kernel;
133136

134137
// D1.3 — decode-kernel trait + residual composition.
135138
// Hydration/calibration path (NOT cascade inference — that uses
136139
// p64_bridge::CognitiveShader per cognitive-shader-architecture.md
137140
// line 582). LAB-ONLY.
138-
#[cfg(feature = "serve")]
141+
#[cfg(any(feature = "serve", feature = "grpc"))]
139142
pub mod decode_kernel;
140143

141144
// D2.1 — token-agreement harness scaffold (I11 cert gate infra).
142145
// Reference model loader stub + top-k comparator + stub result with
143146
// machine-checkable `stub:true` flag. D2.2 adds real safetensors decode.
144147
// LAB-ONLY.
145-
#[cfg(feature = "serve")]
148+
#[cfg(any(feature = "serve", feature = "grpc"))]
146149
pub mod token_agreement;
147150

148151
// Axum REST server. LAB-ONLY.

0 commit comments

Comments
 (0)