D3.1 batch sweep handler + clippy -D warnings clean (117/117 tests)#238
Conversation
Phase 3 batch endpoint + 11 clippy fixes across the crate.
D3.1 — POST /v1/shader/sweep (batch mode):
Enumerates WireSweepGrid::enumerate(), validates each CodecParams
at ingress (precision-ladder + overfit guard per point), computes
kernel_signature() for each candidate, returns WireSweepResponse
with all WireSweepResult rows (stub: calibrate=None,
token_agreement=None, stub=true). SSE streaming + real per-point
calibrate/token-agreement defer to D3.1b.
Route wired in serve.rs alongside existing calibrate/probe/
token-agreement routes. Handler is ~50 LOC.
Clippy fixes (11 issues, now -D warnings clean):
- auto_style.rs: removed unused import QUALIA_DIMS (moved to test
module where it's actually needed)
- codec_bridge.rs: 3x redundant closure → tuple variant directly
- driver.rs: 3x OR patterns → range patterns (1|2|3 → 1..=3)
- wire.rs: is_aligned_64 manual impl → .is_multiple_of(64)
- wire.rs: derivable Default for WireBaseline → #[derive(Default)]
- decode_kernel.rs: doc list item overindentation → 2-space
- codec_research.rs: #[allow(clippy::type_complexity)] on existing
complex fn return type (not worth refactoring pre-existing code)
Board hygiene:
STATUS_BOARD.md D3.1 Queued → In PR
Phase state after merge:
Phase 0 ✅ complete
Phase 1 scaffold ✅ (D1.1/D1.2/D1.3)
Phase 2 scaffold ✅ (D2.1/D2.3)
Phase 3 batch ✅ D3.1 (this PR)
Remaining: D1.1b Cranelift, D2.2 real decode, D3.1b SSE, Phase 4-5
https://claude.ai/code/session_01SbYsmmbPf9YQuYbHZN52Zh
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1e5decd18
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Json(req): Json<WireSweepRequest>, | ||
| ) -> Result<Json<WireSweepResponse>, (StatusCode, Json<Value>)> { | ||
| let start = std::time::Instant::now(); | ||
| let candidates = req.grid.enumerate(); |
There was a problem hiding this comment.
Reject oversized sweep grids before enumeration
In sweep_handler, the request-controlled grid is fully materialized via req.grid.enumerate() with no cardinality limit, so a small JSON payload containing moderately sized axes can explode into an enormous Cartesian product and exhaust CPU/memory while building results. This creates a straightforward denial-of-service path for the new /v1/shader/sweep endpoint; add a hard upper bound (for example a few hundred/thousand points) and return 400 before calling enumerate().
Useful? React with 👍 / 👎.
| cardinality, | ||
| results, | ||
| elapsed_ms: start.elapsed().as_millis() as u64, | ||
| lance_fragment_path: req.log_to_lance, |
There was a problem hiding this comment.
Report Lance output path only when data is actually written
sweep_handler currently echoes req.log_to_lance into lance_fragment_path even though this D3.1 implementation does not append any rows, which makes the response claim persistence that never occurred. Clients that treat lance_fragment_path as evidence of successful logging can silently skip required retries and lose experiment results; until append is implemented, this field should remain None (or include an explicit stub marker for logging status).
Useful? React with 👍 / 👎.
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
Two fixes from Codex review on the D3.1 sweep_handler: P1 — Reject oversized sweep grids before enumerate(). A small JSON payload with moderately-sized axes can explode into an enormous Cartesian product and exhaust CPU/memory. Added MAX_GRID_CARDINALITY = 10,000 check before materialization; returns 400 with explicit error message if exceeded. P2 — Stop echoing req.log_to_lance into lance_fragment_path. The D3.1 stub does NOT actually append rows to Lance; echoing the requested path back in the response falsely claims persistence that never occurred. Clients treating lance_fragment_path as evidence of successful logging would silently skip retries and lose experiment results. Set to None until the real Lance append writer lands (D3.1b). Same anti-#219 pattern: don't claim a measurement/persistence happened when it didn't. The stub:true flag on per-point results covers measurement-honesty; this fix covers persistence-honesty. 117/117 tests pass. Clippy clean under --features serve. https://claude.ai/code/session_01SbYsmmbPf9YQuYbHZN52Zh
…ft guard)
Final Phase 3 scaffold deliverable — curl-driven lab iteration against
the shipped /v1/shader/sweep endpoint.
Files:
configs/codec/README.md — inventory + DoS-ceiling note +
anti-#219 stub:true flag explanation
configs/codec/00_pr220_baseline.yaml
- PR #220 baseline regression: 6 subspaces × 256 centroids ×
identity rotation. Expected ICC ≈ 0.195 mean when D2.2 lands
real decode-and-compare.
configs/codec/10_wider_codebook.yaml
- PR #220 fix (a): centroids ∈ {256, 512, 1024}. Cardinality 3,
three distinct kernel signatures → warm cache after one pass.
configs/codec/12_hadamard_pre_rotation.yaml
- PR #220 fix (c): Hadamard × centroids cross-product (2×2 = 4).
Hadamard stays Tier-3 F32x16 per Rule C.
scripts/codec_sweep.sh
- yq YAML → JSON conversion
- POST to ${SHADER_LAB_URL}/v1/shader/sweep (default localhost:3001)
- jq-pretty request + response
- Stub honesty check: prints results[0].stub flag
→ verifies Phase 0 returns true (machine-checkable anti-#219)
- Requires: yq (mikefarah/yq ≥ v4), curl, jq
wire.rs +1 test: sweep_request_yaml_shape_deserializes_via_serde_json
- Inline JSON fixture mirroring the canonical YAML → JSON shape
- If this test breaks, the YAML configs are stale relative to
the Rust DTOs → scripts/codec_sweep.sh would fail at runtime
- Caught a real drift during development: PascalCase "Identity"
vs the DTO's rename_all="lowercase" (YAMLs correctly use
lowercase; test fixture had the typo)
Phase state:
Phase 0 ✅ complete
Phase 1 scaffold ✅ (D1.1 / D1.2 / D1.3 shipped; D1.1b queued)
Phase 2 scaffold ✅ (D2.1 harness + D2.3 handler; D2.2 queued)
Phase 3 scaffold ✅ — D3.1 batch handler + D3.2 client driver shipped
⏳ D3.1b real Lance append writer queued
DoS-ceiling note: sweep handler rejects grids with cardinality
> 10_000 before enumeration (PR #238 P1 fix). README documents the
ceiling so config authors can budget axis lengths.
Rule D honored: adding a new codec candidate = authoring a new
YAML file in configs/codec/. Zero Rust changes. Zero rebuilds.
Rules F honored at the client boundary: YAML → JSON → HTTP ingress.
Single deserialisation at the shader-lab's handler; everything
after is in-memory Rust (WireSweepRequest → CodecParams → grid
enumerate() → per-candidate WireSweepResult).
https://claude.ai/code/session_01SbYsmmbPf9YQuYbHZN52Zh
…-matrix discipline
Two more additions per user directive — extend the existing SoA +
mandatory-simd + 3-way-merge + adhering-agent sections with:
1. ndarray::simd::* polyfill chain diagram
- Explicit ASCII diagram showing how simd.rs re-exports from
simd_amx.rs / simd_avx512.rs / simd_avx2.rs / simd_neon.rs /
simd_wasm.rs based on cfg(target_feature)
- AMX runtime-gated via amx_available() (orthogonal to compile-
time cfg because Intel AMX needs OS enablement + XCR0 prctl)
- AVX-512 baseline mandatory via target-cpu=x86-64-v4
- AVX-2 fallback cfg-gated when build drops to x86-64-v3
- Scalar is INTERNAL to each backend, never consumer-visible
- hpc/simd_caps.rs + hpc/amx_matmul.rs explicitly called out
as canonical surfaces (top-level modules, not backend reach)
- Mandatory consumer rule: `use ndarray::simd::…` only;
backend files are private implementation detail
2. Mandatory cargo clippy + feature-matrix discipline
- Full matrix: cargo check across default / serve / grpc / lab
- Clippy with -D warnings under serve AND lab (not just one)
- cargo test --lib is NOT enough — need --doc separately
- `--features lab` umbrella is NOT enough — `--features grpc`
ALONE must work for downstream consumers who don't want REST
- Fix pattern documented: internal `_lab-dtos` shared feature
for serde/serde_json/base64/bytemuck used by both serve +
grpc (the pattern applied in PR #238)
- Widen `pub mod wire` gate from `serve` to `any(serve, grpc)`
when shared DTOs exist
- Reviewer trigger: PR description citing only --features serve
tests → request full-matrix re-run
- Rust 1.95 transition guard: grep for `mut ref` / `ref mut`
struct patterns (accidentally stable through 1.94, now
feature-gated). Zero hits today; stay that way.
Both additions were learned the hard way during this session's
PR #238 rescue when `--features grpc` and `--features lab`
silently broke after months of feature drift. The discipline is
now part of the contract, not an afterthought.
Doc now 500+ lines — the author-side pattern guide + reviewer-side
rubric + feature-matrix gate in one place.
https://claude.ai/code/session_01SbYsmmbPf9YQuYbHZN52Zh
…-matrix discipline
Two more additions per user directive — extend the existing SoA +
mandatory-simd + 3-way-merge + adhering-agent sections with:
1. ndarray::simd::* polyfill chain diagram
- Explicit ASCII diagram showing how simd.rs re-exports from
simd_amx.rs / simd_avx512.rs / simd_avx2.rs / simd_neon.rs /
simd_wasm.rs based on cfg(target_feature)
- AMX runtime-gated via amx_available() (orthogonal to compile-
time cfg because Intel AMX needs OS enablement + XCR0 prctl)
- AVX-512 baseline mandatory via target-cpu=x86-64-v4
- AVX-2 fallback cfg-gated when build drops to x86-64-v3
- Scalar is INTERNAL to each backend, never consumer-visible
- hpc/simd_caps.rs + hpc/amx_matmul.rs explicitly called out
as canonical surfaces (top-level modules, not backend reach)
- Mandatory consumer rule: `use ndarray::simd::…` only;
backend files are private implementation detail
2. Mandatory cargo clippy + feature-matrix discipline
- Full matrix: cargo check across default / serve / grpc / lab
- Clippy with -D warnings under serve AND lab (not just one)
- cargo test --lib is NOT enough — need --doc separately
- `--features lab` umbrella is NOT enough — `--features grpc`
ALONE must work for downstream consumers who don't want REST
- Fix pattern documented: internal `_lab-dtos` shared feature
for serde/serde_json/base64/bytemuck used by both serve +
grpc (the pattern applied in PR #238)
- Widen `pub mod wire` gate from `serve` to `any(serve, grpc)`
when shared DTOs exist
- Reviewer trigger: PR description citing only --features serve
tests → request full-matrix re-run
- Rust 1.95 transition guard: grep for `mut ref` / `ref mut`
struct patterns (accidentally stable through 1.94, now
feature-gated). Zero hits today; stay that way.
Both additions were learned the hard way during this session's
PR #238 rescue when `--features grpc` and `--features lab`
silently broke after months of feature drift. The discipline is
now part of the contract, not an afterthought.
Doc now 500+ lines — the author-side pattern guide + reviewer-side
rubric + feature-matrix gate in one place.
https://claude.ai/code/session_01SbYsmmbPf9YQuYbHZN52Zh
Summary
Phase 3 batch sweep endpoint + 11 clippy fixes across the crate. Now passes
cargo clippy -- -D warningsclean.117/117 tests pass + clippy clean.
D3.1 —
POST /v1/shader/sweep(batch mode)Enumerates
WireSweepGrid::enumerate(), validates each candidate viaTryFrom(CodecParams)at ingress (precision-ladder + overfit guard per grid point), computeskernel_signature()for each, returnsWireSweepResponsewith allWireSweepResultrows.Currently stub:
calibrate: None, token_agreement: None, stub: trueper grid point. D3.1b adds SSE streaming + real per-point calibrate/token-agreement.Clippy fixes (11 issues → 0)
auto_style.rsQUALIA_DIMSremoved; moved to test modulecodec_bridge.rsdriver.rs1|2|3→ range1..=3wire.rs.is_multiple_of(64)replaces manual% 64 == 0wire.rsWireBaselineDefault derived via#[derive(Default)]+#[default]attributedecode_kernel.rscodec_research.rs#[allow(clippy::type_complexity)]on pre-existing complex fn returnPhase state after merge
Test Plan
cargo test --manifest-path crates/cognitive-shader-driver/Cargo.toml --features serve --lib— 117/117 passcargo clippy --manifest-path crates/cognitive-shader-driver/Cargo.toml --features serve -- -D warnings— CLEAN (0 errors, 0 warnings from this crate)STATUS_BOARD.mdD3.1 Queued → In PRhttps://claude.ai/code/session_01SbYsmmbPf9YQuYbHZN52Zh