Skip to content

Commit b64baad

Browse files
committed
coding-practices: polyfill chain diagram + mandatory clippy + feature-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
1 parent 80b4ec0 commit b64baad

1 file changed

Lines changed: 131 additions & 0 deletions

File tree

.claude/CODING_PRACTICES.md

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,137 @@ scalar fallback INTERNALLY; the consumer never hand-rolls.
258258
hot path → reject + cite this section. Exception: the ndarray
259259
crate itself implements backends, not a violation.
260260

261+
### How `ndarray::simd::*` resolves to backends (polyfill chain)
262+
263+
The `simd.rs` module in ndarray is the **single public surface**; it
264+
re-exports concrete types from backend files based on `cfg` target
265+
features. Consumers never reach around it. The chain:
266+
267+
```
268+
┌─────────────────────────────────────────────────────────────────┐
269+
│ ndarray::simd (src/simd.rs) ← the ONLY consumer surface │
270+
│ │
271+
│ Re-exports F32x16 / U8x64 / F16x32 / F64x8 / BF16x32 etc. from │
272+
│ the right backend, chosen by cfg(target_feature): │
273+
│ │
274+
│ ┌──────────────┐ ┌──────────────┐ ┌──────────────┐ │
275+
│ │ simd_amx.rs │ │simd_avx512.rs│ │ simd_avx2.rs │ │
276+
│ │ Intel AMX │ │ AVX-512 base │ │ AVX-2 fallbk │ │
277+
│ │ tiles + │ │ F32x16 / │ │ F32x8 / │ │
278+
│ │ VNNI + │ │ U8x64 / ... │ │ F64x4 │ │
279+
│ │ TDPBUSD / │ │ (mandatory │ │ (cfg-gated │ │
280+
│ │ TDPBF16PS │ │ floor at │ │ when build │ │
281+
│ │ via inline │ │ target-cpu= │ │ drops to │ │
282+
│ │ asm (stable) │ │ x86-64-v4) │ │ x86-64-v3) │ │
283+
│ └──────────────┘ └──────────────┘ └──────────────┘ │
284+
│ │ │ │ │
285+
│ ├─ runtime-opt ──┤ │ │
286+
│ │ (amx_available) │ │
287+
│ │ compile-time │ │
288+
│ │ cfg(avx2) │ │
289+
│ │
290+
│ ┌──────────────┐ ┌──────────────┐ ┌──────────────┐ │
291+
│ │ simd_neon.rs │ │ simd_wasm.rs │ │ (scalar) │ │
292+
│ │ aarch64 │ │ wasm32-simd │ │ last resort │ │
293+
│ │ │ │ │ │ INTERNAL to │ │
294+
│ │ │ │ │ │ each backend│ │
295+
│ └──────────────┘ └──────────────┘ └──────────────┘ │
296+
│ │
297+
│ hpc/simd_caps.rs — runtime capability struct │
298+
│ hpc/amx_matmul.rs — Intel AMX tile primitives (tile_dpbusd / │
299+
│ tile_dpbf16ps etc.) surfaced for callers │
300+
│ that want explicit matmul routing │
301+
└─────────────────────────────────────────────────────────────────┘
302+
```
303+
304+
**Mandatory consumer rule:** only ever write `use ndarray::simd::…`.
305+
The backend files are private implementation detail — they can be
306+
reshuffled at any time (new `simd_avx512fp16.rs` shipped in a point
307+
release, backends split per architecture, etc.) without breaking
308+
consumers.
309+
310+
**Explicit AMX routing** (when the caller wants to force the tile
311+
path rather than let `simd::*` infer it): the AMX sibling modules
312+
(`ndarray::simd_amx::*` and `ndarray::hpc::amx_matmul::*`) are
313+
**first-class canonical surfaces**, not backend reach. They're
314+
named at the top level because Intel AMX needs explicit OS
315+
enablement + XCR0 prctl on Linux + runtime `amx_available()`
316+
gating that's orthogonal to compile-time cfg.
317+
318+
---
319+
320+
## MANDATORY `cargo clippy` + feature-matrix discipline
321+
322+
Every PR that touches `crates/*/src/` runs this full matrix before
323+
being declared complete. `--features serve` alone is NOT enough
324+
(learned the hard way at PR #238 when `--features grpc` and
325+
`--features lab` silently broke after months of feature-drift).
326+
327+
```bash
328+
# All four compile-and-warning-clean before commit:
329+
cargo check # default
330+
cargo check --manifest-path crates/<name>/Cargo.toml --features serve
331+
cargo check --manifest-path crates/<name>/Cargo.toml --features grpc
332+
cargo check --manifest-path crates/<name>/Cargo.toml --features lab
333+
334+
# Clippy WITH -D warnings (not just --no-deps); catches redundant
335+
# closures, needless collects, manual Default impls, hidden type
336+
# complexity, etc.:
337+
cargo clippy --manifest-path crates/<name>/Cargo.toml --features lab -- -D warnings
338+
cargo clippy --manifest-path crates/<name>/Cargo.toml --features serve -- -D warnings
339+
340+
# Full test under the widest feature set:
341+
cargo test --manifest-path crates/<name>/Cargo.toml --features lab --lib
342+
343+
# Doc-tests (separate target; --lib skips them):
344+
cargo test --manifest-path crates/<name>/Cargo.toml --features lab --doc
345+
```
346+
347+
**Why `--lib` is not enough.** `cargo test` without `--lib` also runs
348+
integration tests in `tests/` and the doc-tests embedded in `///`
349+
comments. A doc comment that compiles as prose but fails as code
350+
is a latent failure; doc-tests catch it. The `--doc` run is cheap
351+
(seconds) and mandatory.
352+
353+
**Why `--features lab` is not enough.** The `lab` umbrella pulls in
354+
everything but only exercises the union. `cargo check --features grpc`
355+
ALONE still needs to work — downstream consumers that only want gRPC
356+
(not REST) compile grpc-only; if wire.rs is `serve`-gated but grpc.rs
357+
references it, the grpc-only build breaks silently.
358+
359+
**Fix pattern** (applied in PR #238 `_lab-dtos` internal feature):
360+
when two features share a dep (serde / serde_json / base64 / bytemuck
361+
used by both `serve` and `grpc`), factor into an internal feature:
362+
363+
```toml
364+
[features]
365+
_lab-dtos = ["dep:serde", "dep:serde_json", "dep:base64", "dep:bytemuck"]
366+
serve = ["_lab-dtos", "dep:axum", "dep:tokio"]
367+
grpc = ["_lab-dtos", "dep:prost", "dep:tonic", "dep:tonic-build", "dep:tokio"]
368+
lab = ["serve", "grpc", "with-engine", "with-planner"]
369+
```
370+
371+
And widen `pub mod wire` from `#[cfg(feature = "serve")]` to
372+
`#[cfg(any(feature = "serve", feature = "grpc"))]` so both transports
373+
see the shared DTOs.
374+
375+
**Reviewer trigger:** a PR whose description cites only
376+
`--features serve` test results → request re-run across the full
377+
matrix before approval. The matrix is a first-class part of the
378+
contract, not an afterthought.
379+
380+
**Rust 1.95 transition note:** `mut ref` / `ref mut` in struct
381+
pattern field shorthand are now feature-gated (were accidentally
382+
stable through 1.94). When the toolchain pin bumps, grep both
383+
`src/` trees:
384+
385+
```bash
386+
grep -rn "mut ref\b\|ref mut\b" crates/*/src/
387+
```
388+
389+
Zero hits today across `lance-graph/crates/` + `ndarray/src/`.
390+
Stay that way.
391+
261392
---
262393

263394
## The 3-Way BindSpace Mutation Scheme

0 commit comments

Comments
 (0)