diff --git a/.github/workflows/book.yml b/.github/workflows/book.yml index 3149e6c841..a8ddf7f049 100644 --- a/.github/workflows/book.yml +++ b/.github/workflows/book.yml @@ -6,11 +6,23 @@ on: paths: - "book/**" - ".github/workflows/book.yml" + # FALSIFY-BOOK-CLI-PARITY-001 asserts every `apr` subcommand has a chapter. + # Filtering on book/** alone meant it only ran when the BOOK changed, never + # when the CLI gained a command — so `apr beat-run` shipped in #1995 with no + # chapter and the gate stayed green for months, until an unrelated batch that + # happened to touch book/ woke it. A parity gate must run when EITHER side + # moves; watching only one of them is how it comes to certify nothing. + - "crates/apr-cli/src/commands_enum.rs" + - "crates/apr-cli/src/extended_commands.rs" + - "crates/apr-cli/src/tool_commands.rs" pull_request: branches: [main] paths: - "book/**" - ".github/workflows/book.yml" + - "crates/apr-cli/src/commands_enum.rs" + - "crates/apr-cli/src/extended_commands.rs" + - "crates/apr-cli/src/tool_commands.rs" permissions: contents: read @@ -78,8 +90,21 @@ jobs: if ! command -v pmat >/dev/null 2>&1; then cargo install pmat --locked --quiet || true fi - # Validate the book-completeness contract via pv (in-tree dogfood per CLAUDE.md) - pv validate contracts/apr-book-completeness-v1.yaml + # Validate the book-completeness contract via pv. + # + # This line used to call a bare `pv` and exited 127 (command not found) + # the first time the step ever ran: every earlier run died at the parity + # gate above, so nothing reached it. Note the shape of the bug — the step + # guards `pmat` with `command -v` and a comment saying it "tolerates + # absence", then invokes `pv` unguarded one line earlier, in a step whose + # entire purpose is running pv. + # + # `pv` is IN-TREE (crates/aprender-contracts-cli, [[bin]] name = "pv"), so + # build it from the tree instead of depending on an installed copy — that + # is the dogfood rule in CLAUDE.md, and it cannot drift from the contracts + # it is validating. + cargo run --quiet -p aprender-contracts-cli --bin pv -- \ + validate contracts/apr-book-completeness-v1.yaml # pmat comply check — single binary signal; tolerates absence (pmat may not be available on every runner) if command -v pmat >/dev/null 2>&1; then pmat comply check 2>&1 | tail -10 || echo "::warning::pmat comply check returned non-zero (advisory)" diff --git a/Makefile b/Makefile index 6a354f2461..c671514ed6 100644 --- a/Makefile +++ b/Makefile @@ -19,7 +19,7 @@ SHELL := /bin/bash # Multi-line recipes execute in same shell .ONESHELL: -.PHONY: all build test test-smoke test-fast test-quick test-full test-heavy lint lint-current fmt clean doc book book-build book-serve book-test tier1 tier2 tier3 tier4 coverage coverage-fast profile hooks-install hooks-verify lint-scripts bashrs-score bashrs-lint-makefile chaos-test chaos-test-full chaos-test-lite fuzz bench dev pre-push ci check run-ci run-bench audit deps-validate deny pmat-score pmat-gates quality-report semantic-search examples mutants mutants-fast property-test install-alsa test-alsa test-audio-full contract-validate contract-test contract-audit contract-regen contract-check dev-setup check-siblings +.PHONY: all build test test-smoke test-fast test-quick test-full test-heavy lint lint-current fmt clean doc book book-build book-serve book-test tier1 tier2 tier3 tier4 coverage coverage-fast profile hooks-install hooks-verify lint-scripts bashrs-score bashrs-lint-makefile chaos-test chaos-test-full chaos-test-lite fuzz bench dev pre-push ci check run-ci run-bench audit deps-validate deny pmat-score pmat-gates quality-report semantic-search examples mutants mutants-fast property-test install-alsa test-alsa test-audio-full contract-validate contract-test contract-audit contract-regen contract-check dev-setup check-siblings check-wasm32 # Default target all: tier2 @@ -985,6 +985,9 @@ publish: ## Publish crate(s) to crates.io — strips [patch], publishes, then ve echo "POST-PUBLISH VERIFICATION: PASSED"; \ fi +check-wasm32: ## Verify aprender-core still compiles for wasm32-unknown-unknown (aprender#2310) + @bash scripts/check_wasm32_core_builds.sh + check-siblings: ## Verify sibling repos exist and versions are compatible @echo "Checking sibling repositories..." @all_ok=true; \ diff --git a/README.md b/README.md index 4b8388963f..d761a2444f 100644 --- a/README.md +++ b/README.md @@ -41,9 +41,9 @@ publishing — all backed by YAML provable contracts that fail CI on drift. | Metric | Count | Source of truth | |-------:|------:|---| | Workspace crates | **78** workspace crates | `cargo metadata --no-deps` (NOT `ls crates/` — 4 are `exclude`d, 1 has no Cargo.toml) | -| Provable contracts | **1770** provable contracts | `find contracts/ -name '*.yaml'` | -| CLI commands | **103** CLI commands | `apr --help` | -| Book CLI chapters | **103** chapters | `ls book/src/cli/*.md` (parity with CLI) | +| Provable contracts | **1771** provable contracts | `find contracts/ -name '*.yaml'` | +| CLI commands | **105** CLI commands | `apr --help` | +| Book CLI chapters | **105** chapters | `ls book/src/cli/*.md` (parity with CLI) | | Book lib chapters | **69** chapters | `ls book/src/lib/*.md` (parity with `pub mod`) | These numbers are enforced by [`contracts/readme-claims-v1.yaml`](contracts/readme-claims-v1.yaml). @@ -212,7 +212,7 @@ paiml/aprender/ ├── Cargo.toml # Workspace root + `cargo install aprender` ├── crates/ │ ├── aprender-core/ # ML library (use aprender::*) -│ ├── apr-cli/ # CLI logic (103 subcommands) +│ ├── apr-cli/ # CLI logic (105 subcommands) │ ├── aprender-compute/ # SIMD/GPU compute kernels │ ├── aprender-gpu/ # CUDA PTX │ ├── aprender-serve/ # Inference server diff --git a/book/src/SUMMARY.md b/book/src/SUMMARY.md index f50665dc8b..6c393eac85 100644 --- a/book/src/SUMMARY.md +++ b/book/src/SUMMARY.md @@ -280,6 +280,7 @@ - [apr attn-viz-lint](./cli/attn-viz-lint.md) - [apr audio-inspect-lint](./cli/audio-inspect-lint.md) - [apr awq-lint](./cli/awq-lint.md) +- [apr beat-run](./cli/beat-run.md) - [apr bench](./cli/bench.md) - [apr canary](./cli/canary.md) - [apr cbtop](./cli/cbtop.md) @@ -291,6 +292,7 @@ - [apr compile](./cli/compile.md) - [apr convert](./cli/convert.md) - [apr data](./cli/data.md) +- [apr dataset](./cli/dataset.md) - [apr ddp-metrics-lint](./cli/ddp-metrics-lint.md) - [apr debug](./cli/debug.md) - [apr decrypt](./cli/decrypt.md) @@ -298,6 +300,7 @@ - [apr diff](./cli/diff.md) - [apr distill](./cli/distill.md) - [apr dry-sampling-lint](./cli/dry-sampling-lint.md) +- [apr debug embed-viz](./cli/embed-viz.md) - [apr embed-viz-lint](./cli/embed-viz-lint.md) - [apr embed](./cli/embed.md) - [apr embeddings-lint](./cli/embeddings-lint.md) @@ -321,6 +324,7 @@ - [apr imatrix-lint](./cli/imatrix-lint.md) - [apr import](./cli/import.md) - [apr inspect](./cli/inspect.md) +- [apr kernel](./cli/kernel.md) - [apr kv-timeline-lint](./cli/kv-timeline-lint.md) - [apr lint](./cli/lint.md) - [apr list](./cli/list.md) @@ -399,6 +403,7 @@ - [aprender::code](./lib/code.md) - [aprender::compute](./lib/compute.md) - [aprender::data](./lib/data.md) +- [aprender::datasets](./lib/datasets.md) - [aprender::decomposition](./lib/decomposition.md) - [aprender::demo](./lib/demo.md) - [aprender::embed](./lib/embed.md) @@ -427,6 +432,7 @@ - [aprender::nn](./lib/nn.md) - [aprender::online](./lib/online.md) - [aprender::optim](./lib/optim.md) +- [aprender::pipeline](./lib/pipeline.md) - [aprender::prelude](./lib/prelude.md) - [aprender::preprocessing](./lib/preprocessing.md) - [aprender::primitives](./lib/primitives.md) diff --git a/book/src/cli/beat-run.md b/book/src/cli/beat-run.md new file mode 100644 index 0000000000..184f790c6b --- /dev/null +++ b/book/src/cli/beat-run.md @@ -0,0 +1,75 @@ + + +# apr beat-run + +Evaluate a beat-benchmark contract against a measured value (PMAT-741) + +**Category**: Quality + +## Synopsis + +```text +apr beat-run [--measured ] [--json] +``` + +`apr beat-run` is the falsifiable runner behind the four-pillar "replace **and** +beat" mission. A beat contract pins an incumbent's baseline — the number +scikit-learn, PyTorch, Unsloth or Ollama actually produces — together with the +threshold `apr` must clear to claim a win. This command reads that contract and, +given a measurement, returns the verdict. + +Two modes: + +- **Without `--measured`** it reports the contract's pinned parameters and exits + 0. Use this to see what a beat currently claims. +- **With `--measured`** it computes the verdict and exits **non-zero on a + regression or an unjudgeable contract**, so it can gate CI directly. + +The verdict is not computed here. It comes from +`aprender_contracts::schema::Beat::evaluate`, the single source of truth, so the +CLI and the contract engine cannot drift into disagreeing about whether a beat +was won. + +## Examples + + +```bash +apr beat-run --help +``` + +Report what a beat contract pins, without judging anything: + + +```bash +apr beat-run contracts/beat-sklearn-iris-v1.yaml +``` + +Judge a measurement and gate on it — this is the form CI uses: + + +```bash +apr beat-run contracts/beat-sklearn-iris-v1.yaml --measured 0.973 +echo "exit=$?" # non-zero means REGRESSED or unjudgeable +``` + +## Exit codes + +| exit | meaning | +|-----:|---------| +| 0 | the beat was WON, or no `--measured` value was supplied | +| non-zero | REGRESSED, or the contract cannot judge the value it was given | + +An **unjudgeable** contract exits non-zero on purpose. A beat that cannot decide +is not a beat that passed: silently treating "I could not tell" as a win is the +failure mode this runner exists to prevent. + +A contract path that does not exist reports `File not found`, not a format +error — an earlier version printed `Invalid APR format:` and sent readers +looking for a model that was never involved. + +## See also + +- [`apr bench`](./bench.md) — produce the measurement this command judges +- [`apr qa`](./qa.md) — falsifiable QA gates on a model artifact +- Source: [`crates/apr-cli/src/commands/beat_run.rs`](https://github.com/paiml/aprender/blob/main/crates/apr-cli/src/commands/beat_run.rs) +- Contract: [`contracts/apr-cli-commands-v1.yaml`](https://github.com/paiml/aprender/blob/main/contracts/apr-cli-commands-v1.yaml) diff --git a/book/src/cli/dataset.md b/book/src/cli/dataset.md new file mode 100644 index 0000000000..f0c6afc36c --- /dev/null +++ b/book/src/cli/dataset.md @@ -0,0 +1,59 @@ + + +# apr dataset + +Dataset inspection tools. + +**Category**: Inspection + +## Synopsis + +```text +apr dataset audio-inspect [--format json|text] [-o FILE] [--force] +``` + +## What `audio-inspect` measures + +It decodes an uncompressed RIFF/WAVE file and reports the shape and amplitude +extrema it actually measured: + +| Field | Meaning | +|-------|---------| +| `sample_rate` | Hz, from the `fmt ` chunk — never resampled | +| `channels` | channel count, from the `fmt ` chunk — never mixed down | +| `samples` | frames per channel (torchaudio's `num_frames`) | +| `min` / `max` | amplitude extrema over every decoded sample | +| `codec` | `pcm_u8`, `pcm_s16le`, `pcm_s24le`, `pcm_s32le` or `pcm_f32le` | + +Integer PCM is normalised by the negative full-scale magnitude, the +`torchaudio.load(normalize=True)` convention. Float payloads are reported as +stored, so a float WAV that overshoots ±1 shows up as such. + +A container or codec it cannot decode — FLAC, MP3, Ogg, ADPCM, a truncated +`data` chunk, an empty stream — is **refused** with a non-zero exit and a +message naming what was found. It never estimates. + +## Example + + +```bash +apr dataset audio-inspect --help +``` + +Producing the observation `apr audio-inspect-lint` reads: + + +```bash +apr dataset audio-inspect clip.wav --format json -o audio.json +apr audio-inspect-lint --json-file audio.json --expected-sample-rate 16000 +``` + +## Full help + +Run `apr dataset audio-inspect --help` for the complete option list. + +## See also + +- Consumer: [`apr audio-inspect-lint`](./audio-inspect-lint.md) +- Source: [`crates/apr-cli/src/commands/audio_inspect.rs`](https://github.com/paiml/aprender/blob/main/crates/apr-cli/src/commands/audio_inspect.rs) +- Contract: [`contracts/apr-lint-producers-v1.yaml`](https://github.com/paiml/aprender/blob/main/contracts/apr-lint-producers-v1.yaml) diff --git a/book/src/cli/debug.md b/book/src/cli/debug.md index 8cbf8b54d6..cde6d3263f 100644 --- a/book/src/cli/debug.md +++ b/book/src/cli/debug.md @@ -9,9 +9,14 @@ Simple debugging output ("drama" mode available) ## Synopsis ```text -apr debug [OPTIONS] +apr debug [FILE] [OPTIONS] +apr debug embed-viz --model [OPTIONS] ``` +`FILE` is optional because a subcommand brings its own input. `apr debug` with +neither a file nor a subcommand refuses rather than exiting 0 having done +nothing. + ## Example @@ -25,5 +30,6 @@ Run `apr debug --help` for the complete option list. ## See also +- Subcommand: [`apr debug embed-viz`](./embed-viz.md) - Source: [`crates/apr-cli/src/commands/debug.rs`](https://github.com/paiml/aprender/blob/main/crates/apr-cli/src/commands/debug.rs) - Contract: [`contracts/apr-page-cli-debug-v1.yaml`](https://github.com/paiml/aprender/blob/main/contracts/apr-page-cli-debug-v1.yaml) diff --git a/book/src/cli/embed-viz-lint.md b/book/src/cli/embed-viz-lint.md index 9da933eb93..2e82be4bff 100644 --- a/book/src/cli/embed-viz-lint.md +++ b/book/src/cli/embed-viz-lint.md @@ -25,5 +25,6 @@ Run `apr embed-viz-lint --help` for the complete option list. ## See also +- Producer: [`apr debug embed-viz`](./embed-viz.md) - Source: [`crates/apr-cli/src/commands/embed_viz_lint.rs`](https://github.com/paiml/aprender/blob/main/crates/apr-cli/src/commands/embed_viz_lint.rs) - Contract: [`contracts/apr-page-cli-embed-viz-lint-v1.yaml`](https://github.com/paiml/aprender/blob/main/contracts/apr-page-cli-embed-viz-lint-v1.yaml) diff --git a/book/src/cli/embed-viz.md b/book/src/cli/embed-viz.md new file mode 100644 index 0000000000..7897059d13 --- /dev/null +++ b/book/src/cli/embed-viz.md @@ -0,0 +1,99 @@ + + +# apr debug embed-viz + +Project a model's token-embedding table to 2-D. + +**Category**: Inspection + +## Synopsis + +```text +apr debug embed-viz --model [--tensor NAME] [--projection pca|random] + [--seed N] [--limit N] [--tokens FILE] [-o FILE] [--force] +``` + +## What it produces + +It reads a **real** token-embedding tensor out of a GGUF, APR or SafeTensors +model — dequantising as needed via `RosettaStone::load_tensor_f32` — and writes +the `token_id,token_str,x,y` CSV that `apr embed-viz-lint` parses. One row per +**token**. + +| `--projection` | Method | +|----------------|--------| +| `pca` (default) | Exact PCA onto the top 2 principal components. Deterministic. | +| `random` | Seeded Johnson–Lindenstrauss projection. Deterministic in `--seed`, cheap at any hidden size. | +| `umap` | **Refused.** This binary implements no UMAP, and will not label a different algorithm's output `umap`. | + +`token_str` comes from the model's own GGUF vocabulary, or from `--tokens`. +When neither is available every row carries the literal `` — a +marker that claims nothing. Token text is escaped (`,` → `\x2c`, `"` → `\x22`, +`\` → `\\`, CR/LF → `\r`/`\n`) so a token containing a comma cannot silently +shift the column count the classifier counts. + +## The vocabulary axis is chosen per format + +This is the part that is easy to get wrong, and was: + +| Format | Reported shape of the embedding table | +|--------|----------------------------------------| +| GGUF | `[hidden, vocab]` — GGML `ne` order, `ne[0]` is the contiguous dimension | +| APR, SafeTensors | `[vocab, hidden]` — row-major | + +The payload is `[vocab][hidden]` in all three, so only the reported axis *order* +differs. Taking `shape[0]` as the vocabulary for every format made +`Qwen3.5-0.8B-Q4_K_M.gguf` — whose `token_embd.weight` reports `[1024, 248320]` +— emit 1024 rows for a 248320-token vocabulary, and `--projection pca` never +returned because it was handed a 248320-wide covariance problem. + +Note that `token_str` looks **correct either way**: it is resolved by row index +from the vocabulary list, so it cannot reveal this. The row count against the +real vocabulary size can. `apr embed-viz-lint --expected-vocab-size` is +therefore not a formality — run it. + +As a backstop the producer refuses outright when a model declares more tokens +than the chosen axis has rows: every token must have an embedding row, and +padding only ever goes the other way. + +## Example + + +```bash +apr debug embed-viz --help +``` + +Producing the observation `apr embed-viz-lint` reads, then checking it against +the model's real vocabulary size: + + +```bash +apr debug embed-viz --model Qwen3.5-0.8B-Q4_K_M.gguf \ + --projection random --seed 42 -o emb.csv +apr embed-viz-lint --csv-file emb.csv --expected-vocab-size 248320 +``` + +The determinism gate wants two runs at one seed: + + +```bash +apr debug embed-viz --model Qwen3.5-0.8B-Q4_K_M.gguf --seed 42 --limit 500 -o a.csv +apr debug embed-viz --model Qwen3.5-0.8B-Q4_K_M.gguf --seed 42 --limit 500 -o b.csv +apr embed-viz-lint --csv-file a.csv --csv-file-b b.csv +``` + +`--limit` caps the number of tokens projected, which is what you want on a +large vocabulary — PCA's cost is driven by the hidden size, but the CSV is one +row per token. + +## Full help + +Run `apr debug embed-viz --help` for the complete option list. + +## See also + +- Consumer: [`apr embed-viz-lint`](./embed-viz-lint.md) +- Parent command: [`apr debug`](./debug.md) +- Source: [`crates/apr-cli/src/commands/embed_viz.rs`](https://github.com/paiml/aprender/blob/main/crates/apr-cli/src/commands/embed_viz.rs) +- Layout contract: [`contracts/tensor-layout-v1.yaml`](https://github.com/paiml/aprender/blob/main/contracts/tensor-layout-v1.yaml) +- Contract: [`contracts/apr-lint-producers-v1.yaml`](https://github.com/paiml/aprender/blob/main/contracts/apr-lint-producers-v1.yaml) diff --git a/book/src/cli/kernel.md b/book/src/cli/kernel.md new file mode 100644 index 0000000000..ce0ed8276d --- /dev/null +++ b/book/src/cli/kernel.md @@ -0,0 +1,62 @@ + + +# apr kernel + +Kernel-level parity measurements. + +**Category**: Analysis + +## Synopsis + +```text +apr kernel parity [--impl tiled|flash2] [--ref naive] [--seq-len N] + [--num-heads N] [--num-kv-heads N] [--head-dim N] + [--seed N] [--json] [-o FILE] [--force] +``` + +## What `parity` measures + +`--impl tiled` runs the in-tree tiled online-softmax attention kernel +(`realizar::brick::FlashAttentionBrick`) and a naive reference — a materialised +score row with a max-subtracted softmax — over the same seeded Q/K/V, then +reports `max_abs_diff` and `cosine_sim` between the two outputs. Two +independent implementations, so the comparison can genuinely fail. + +The regime is a decode step: one query position attending over a `seq_len`-long +KV cache. The emitted body says so. + +`--impl flash2` names the pinned `hf-kernels-community:flash-attn2@` CUDA +kernel. **This binary embeds no such kernel**, so asking for it is refused with +a non-zero exit — never answered by the tiled path under flash2's name. That +matters because CRUX-L-02 pins `kernel_source` to `pkg@sha` precisely so a +provenance line cannot be borrowed. + +`--impl flash2` with `--head-dim` outside {64, 128} is refused at dispatch, and +that refusal is itself a capturable observation: the `{"error": ...}` body it +writes is what `apr attn-parity-lint --head-dim-error-file` reads. + +## Example + + +```bash +apr kernel parity --impl tiled --ref naive --seq-len 16 --num-heads 2 \ + --num-kv-heads 2 --head-dim 64 --json +``` + +Feeding the result to its lint — one body discharges both gates: + + +```bash +apr kernel parity --impl tiled --ref naive --seq-len 16 --json -o /tmp/parity.json --force +apr attn-parity-lint --parity-file /tmp/parity.json --provenance-file /tmp/parity.json +``` + +## Full help + +Run `apr kernel parity --help` for the complete option list. + +## See also + +- Consumer: [`apr attn-parity-lint`](./attn-parity-lint.md) +- Source: [`crates/apr-cli/src/commands/kernel_parity.rs`](https://github.com/paiml/aprender/blob/main/crates/apr-cli/src/commands/kernel_parity.rs) +- Contract: [`contracts/apr-lint-producers-v1.yaml`](https://github.com/paiml/aprender/blob/main/contracts/apr-lint-producers-v1.yaml) diff --git a/book/src/lib/datasets.md b/book/src/lib/datasets.md new file mode 100644 index 0000000000..c605167fa1 --- /dev/null +++ b/book/src/lib/datasets.md @@ -0,0 +1,45 @@ + + +# Module: `aprender::datasets` + +Public module of the `aprender-core` crate. + +## Source + +[`crates/aprender-core/src/datasets.rs`](https://github.com/paiml/aprender/blob/main/crates/aprender-core/src/datasets.rs) + +## Example + +```rust +use aprender::datasets::{load_iris, make_blobs}; +// See `cargo doc -p aprender-core --open` for full API reference. +``` + +## Module summary + +`aprender::datasets` provides the dataset generators and loaders that Pillar 1 +(replace **and** beat scikit-learn) measures against. It mirrors +`sklearn.datasets`: + +| function | mirrors | +|----------|---------| +| `make_blobs` | `sklearn.datasets.make_blobs` | +| `make_regression` | `sklearn.datasets.make_regression` | +| `make_classification` | `sklearn.datasets.make_classification` | +| `load_iris` | `sklearn.datasets.load_iris` | + +The embedded real data — currently Iris — is sourced once from scikit-learn and +committed to the repository, so loading it has **no runtime Python or network +dependency**. That property is what lets the beat benchmarks in +`contracts/beat-sklearn-*.yaml` compare like against like without a Python +process in the loop. + +Larger embedded sets (`load_digits`, `load_california_housing`) are not +implemented yet; they are tracked as a continuation of PMAT-720. This chapter +says so rather than implying a completeness the module does not have. + +## See also + +- [`aprender::data`](./data.md) — the columnar `DataFrame` these feed +- [`apr beat-run`](../cli/beat-run.md) — evaluates the beat contracts that + consume these datasets diff --git a/book/src/lib/pipeline.md b/book/src/lib/pipeline.md new file mode 100644 index 0000000000..3b1174a317 --- /dev/null +++ b/book/src/lib/pipeline.md @@ -0,0 +1,42 @@ + + +# Module: `aprender::pipeline` + +Public module of the `aprender-core` crate. + +## Source + +[`crates/aprender-core/src/pipeline.rs`](https://github.com/paiml/aprender/blob/main/crates/aprender-core/src/pipeline.rs) + +## Example + +```rust +use aprender::pipeline::Pipeline; +// See `cargo doc -p aprender-core --open` for full API reference. +``` + +## Module summary + +`aprender::pipeline` provides `Pipeline`, which chains transformers and ends in +a single estimator. It mirrors `sklearn.pipeline.Pipeline`: + +- **`fit`** — each transformer is fit, then applied, in sequence; the final + estimator is fit on the fully transformed data. +- **`predict` / `score`** — the same transformer chain is applied in + transform-only mode before delegating to the estimator. + +That asymmetry is the point of the type. Fitting a scaler on data that has +already been through the test-time path, or scoring against a scaler fit on the +scoring data, is the classic leakage bug; routing both through one object makes +it hard to write by accident. + +Steps use trait objects (`Box` and `Box`) so a +pipeline can be heterogeneous — for example `StandardScaler` followed by +`LogisticRegression`. + +## See also + +- [`aprender::traits`](./traits.md) — the `Transformer` and `Estimator` traits a + step must implement +- [`aprender::preprocessing`](./preprocessing.md) — the transformers most + commonly used as steps diff --git a/contracts/apr-cli-commands-v1.yaml b/contracts/apr-cli-commands-v1.yaml index 8def3ff34f..b5df63b5af 100644 --- a/contracts/apr-cli-commands-v1.yaml +++ b/contracts/apr-cli-commands-v1.yaml @@ -118,6 +118,20 @@ commands: requires_model: true side_effects: [] + # aprender#2377 finding 3: the producers `*-lint` help documents. + # See contracts/apr-lint-producers-v1.yaml. + - name: dataset + category: inspection + description: "Dataset inspection tools (audio-inspect)" + requires_model: false + side_effects: [] + + - name: kernel + category: analysis + description: "Kernel-level parity measurements (parity)" + requires_model: false + side_effects: [] + - name: trace category: inspection description: "Layer-by-layer trace analysis" diff --git a/contracts/apr-lint-producers-v1.yaml b/contracts/apr-lint-producers-v1.yaml new file mode 100644 index 0000000000..e711325d62 --- /dev/null +++ b/contracts/apr-lint-producers-v1.yaml @@ -0,0 +1,419 @@ +# APR-LINT-PRODUCERS — every *-lint consumer has a runnable producer +# +# aprender#2377 finding 3. Dogfooding 0.63.0 found three `*-lint` commands whose +# help text documented a producer the shipped binary did not have: +# +# attn-parity-lint documented `apr kernel parity …` — no `kernel` command +# audio-inspect-lint documented `apr dataset audio-inspect` — no `dataset` command +# embed-viz-lint documented `apr debug embed-viz` — no such subcommand +# +# A lint reads a captured observation; its help text is the only thing telling an +# operator how to produce that observation. Where the producer does not exist the +# lint is unreachable in practice — its gates had never run on real data and +# could not. This contract pins the property that closes it: for each pair, the +# producer's OWN output is accepted by its lint, and a corrupted body is not. +# +# The falsifiers are ROUND TRIPS, not shape assertions. A shape assertion +# ("the JSON has a sample_rate key") cannot prove producer and consumer agree; +# running the consumer on the producer's bytes can. Each round trip ships with +# its negative half, so it cannot pass vacuously. + +metadata: + id: APR-LINT-PRODUCERS + version: "1.0.0" + created: "2026-08-13" + updated: "2026-08-13" + author: PAIML Engineering + registry: true + status: partial + kind: kernel + parent_contracts: + - crux-L-02-v1 + - crux-H-13-v1 + - crux-F-18-v1 + category: "CLI — producer/consumer integrity" + competitor: none + demand_score: 5 + intake_status: implemented + description: > + Every `apr *-lint` consumer whose help text names an `apr …` producer must + have that producer in the same binary, and the producer's output must be + accepted by that lint. Producers report only what they measured: a + configuration or codec they cannot run is REFUSED with a non-zero exit and a + message naming what is missing, never a plausible-looking number and never + exit 0. + + references: + - 'aprender#2377 finding 3 — dogfood 0.63.0' + - 'contracts/crux-L-02-v1.yaml — attn-parity-lint consumer' + - 'contracts/crux-H-13-v1.yaml — audio-inspect-lint consumer' + - 'contracts/crux-F-18-v1.yaml — embed-viz-lint consumer' + - 'arXiv:2307.08691 — FlashAttention-2 (head_dim ∈ {64,128} dispatch set)' + - 'RIFF/WAVE — Multimedia Programming Interface and Data Specifications 1.0' + +equations: + audio_inspect_round_trip: + formula: | + Let W be an uncompressed RIFF/WAVE file. + obs := apr dataset audio-inspect W --format json + obs ⊨ {min, max, sample_rate, channels, samples} + Then: + apr audio-inspect-lint --json-file obs ⇒ exit 0 + and for any single-field corruption c of obs that violates an H-13 gate: + apr audio-inspect-lint --json-file c(obs) ⇒ exit ≠ 0 + Amplitude normalisation (torchaudio load(normalize=True) convention): + pcm_u8 x ↦ (x - 128) / 128 + pcm_s16 x ↦ x / 2^15 + pcm_s24 x ↦ x / 2^23 + pcm_s32 x ↦ x / 2^31 + pcm_f32 x ↦ x (reported as stored) + domain: "a RIFF/WAVE file with a `fmt ` and a `data` chunk" + codomain: "an H-13 observation body, or a non-zero exit naming the refusal" + invariants: + - "integer PCM normalises into [-1, 1]; a float payload is reported as stored" + - "a container or codec the decoder cannot read is REFUSED, never estimated" + - "a truncated `data` chunk is REFUSED — extrema over a surviving prefix answer + a question about a file that does not exist" + - "0 decoded frames is REFUSED: an empty stream has no amplitude to report" + + kernel_parity_round_trip: + formula: | + Given seeded Q ∈ R^{H×D}, K,V ∈ R^{S×H_kv×D} drawn from `--seed`: + out_tiled := FlashAttentionBrick(H, H_kv, D).forward(Q, K, V, S) + out_naive := softmax(QKᵀ / √D) · V (materialised, max-subtracted) + obs := { max_abs_diff: max|out_tiled - out_naive|, + cosine_sim: ⟨out_tiled, out_naive⟩ / (‖·‖‖·‖), + attn_impl, kernel_source, fallback } + Then: + apr attn-parity-lint --parity-file obs --provenance-file obs ⇒ exit 0 + at the SHIPPED defaults (tol_abs = 5e-3, tol_cos = 0.9999). + Provenance is never borrowed: + attn_impl == "flash2" ⟹ the pinned kernel actually ran + attn_impl != "flash2" ⟹ `fallback` is a non-empty reason + domain: "(impl ∈ {tiled, flash2}, ref = naive, seq_len, num_heads, num_kv_heads, head_dim, seed)" + codomain: "an L-02 parity+provenance body, or a non-zero exit naming the refusal" + invariants: + - "the measurement is between two INDEPENDENT implementations — online-softmax + tiling vs materialised softmax — so a regression in either makes it fail" + - "`--impl flash2` is refused: this binary embeds no hf-kernels-community + flash-attn2 kernel, so no flash2 measurement exists to report" + - "head_dim ∉ {64, 128} under `--impl flash2` errors at dispatch, and the error + body is itself the observation `--head-dim-error-file` reads" + - "the regime measured (decode step, one query position) is stated in the body" + + embed_viz_round_trip: + formula: | + Let E ∈ R^{V × d} be a model's token-embedding tensor. + Z := project(E[0..n], method) method ∈ {pca, random} + csv := "token_id,token_str,x,y" + rows (i, escape(tok_i), Z[i,0], Z[i,1]) + Then: + apr embed-viz-lint --csv-file csv --expected-vocab-size n ⇒ exit 0 + and for two runs at the same --seed: + apr embed-viz-lint --csv-file a --csv-file-b b ⇒ exit 0 (byte-identical) + domain: "(model: Path, tensor: Option, projection, seed, limit)" + codomain: "a `token_id,token_str,x,y` CSV, or a non-zero exit naming the refusal" + invariants: + - "the projection named in the report is the one that ran; `--projection umap` + is REFUSED rather than answered by pca or random under umap's name" + - "same seed ⇒ byte-identical CSV; a different seed ⇒ a different CSV" + - "token text is escaped so a token containing a comma cannot shift the column + count the F-18 classifier counts" + - "unresolvable token text is written as the literal ``, which + claims nothing, rather than a plausible-looking token" + +falsification_tests: +- id: FALSIFY-LINT-PRODUCER-001 + rule: "`apr dataset audio-inspect` output is accepted by `apr audio-inspect-lint`" + prediction: "decoding a 16 kHz stereo i16 WAV and linting the result exits 0" + test: | + set -euo pipefail + apr dataset audio-inspect clip.wav --format json -o /tmp/audio.json + apr audio-inspect-lint --json-file /tmp/audio.json \ + --expected-sample-rate 16000 --expected-channels 2 + if_fails: "the H-13 producer and consumer disagree — the lint is unreachable again" + evidence_discharged_by: + producer_path: crates/apr-cli/src/commands/audio_inspect.rs + cli_path: crates/apr-cli/src/commands/audio_inspect_lint.rs + e2e_path: crates/apr-cli/src/commands/audio_inspect_tests.rs + e2e_tests: + - round_trip_producer_output_is_accepted_by_audio_inspect_lint + - round_trip_cannot_pass_vacuously_when_the_body_is_corrupted + - json_body_carries_the_five_keys_the_h13_classifier_reads + sub_claim: | + The producer decodes a real RIFF/WAVE payload and the H-13 lint accepts + the bytes it wrote, with both optional assertions armed. The negative half + corrupts one field per H-13 gate (amplitude above full scale, non-canonical + sample rate, zero frames) and requires a refusal each time. + scope: "(wav bytes) -> observation -> lint exit status" + discharge_status: FULL_ROUND_TRIP +- id: FALSIFY-LINT-PRODUCER-002 + rule: "a codec the decoder cannot read is refused, not estimated" + prediction: "FLAC / bad magic / truncated data / 0 frames each exit non-zero" + test: | + set -euo pipefail + set +e + apr dataset audio-inspect song.flac --format json > /tmp/flac.json + EC=$? + set -e + [ "$EC" -ne 0 ] || exit 1 + if_fails: "the producer invented an observation for a file it did not decode" + evidence_discharged_by: + producer_path: crates/apr-cli/src/commands/audio_inspect.rs + e2e_path: crates/apr-cli/src/commands/audio_inspect_tests.rs + e2e_tests: + - refuses_a_flac_stream_by_name + - refuses_a_non_riff_file + - refuses_a_truncated_data_chunk_instead_of_reporting_the_prefix + - refuses_an_empty_stream_rather_than_reporting_zero_amplitude + - refuses_an_unsupported_codec_tag + - refuses_a_zero_channel_header + sub_claim: | + Six refusal paths, each asserting the refusal (not merely `is_err`) and + each requiring the message to name what was found. + scope: "(unsupported bytes) -> non-zero exit + a message naming the container" + discharge_status: FULL_ROUND_TRIP +- id: FALSIFY-LINT-PRODUCER-003 + rule: "`apr kernel parity` output is accepted by `apr attn-parity-lint`" + prediction: "tiled-vs-naive parity at the shipped 5e-3 / 0.9999 defaults exits 0" + test: | + set -euo pipefail + apr kernel parity --impl tiled --ref naive --json -o /tmp/parity.json + apr attn-parity-lint --parity-file /tmp/parity.json \ + --provenance-file /tmp/parity.json + if_fails: "the L-02 producer and consumer disagree, or the tiled kernel diverged + from the naive reference past the FlashAttention-2 bound" + evidence_discharged_by: + producer_path: crates/apr-cli/src/commands/kernel_parity.rs + cli_path: crates/apr-cli/src/commands/attn_parity_lint.rs + e2e_path: crates/apr-cli/src/commands/kernel_parity_tests.rs + e2e_tests: + - round_trip_producer_output_is_accepted_by_attn_parity_lint + - round_trip_cannot_pass_vacuously_when_the_body_is_corrupted + - tiled_and_naive_agree_far_inside_the_fa2_bound + - the_emitted_parity_metrics_are_the_ones_that_were_measured + - perturbing_the_inputs_moves_the_emitted_parity_metrics + - the_parity_metrics_are_not_vacuous + sub_claim: | + One body discharges both L-02 gates. The negative half corrupts + `max_abs_diff`, `cosine_sim`, and the provenance pair (flash2 with no + pinned sha; a blanked fallback reason) and requires a refusal each time. + + The EMITTED numbers are bound to a measurement, which is the claim this + entry previously over-stated. It credited + `the_parity_metrics_are_not_vacuous` with proving the metrics can see a + perturbation; that test operates on synthetic vectors and never calls + `run()`, so it said nothing about the shipped body. Replacing the two + measured fields in `measure_tiled` with the literals `0.0` / `1.0` left + all 16 tests of the day green, this entry included. + + Two tests now close that hole: + `the_emitted_parity_metrics_are_the_ones_that_were_measured` re-runs the + brick and the naive reference independently, across three shapes, and + requires the observation to carry exactly those numbers (modulo the 1-ULP + JSON float round trip, which it cancels rather than tolerates); + `perturbing_the_inputs_moves_the_emitted_parity_metrics` varies the seed + and requires the emitted values to move. Both are red for a fabricated + constant, including one chosen to be exactly right for the default shape. + scope: "(seeded Q/K/V) -> parity+provenance body -> lint exit status; the + emitted metrics are pinned to an independent re-measurement" + discharge_status: FULL_ROUND_TRIP +- id: FALSIFY-LINT-PRODUCER-004 + rule: "unsupported head_dim errors at dispatch and its error body feeds the lint" + prediction: "`--impl flash2 --head-dim 96 --json` exits non-zero and writes + an `error` mentioning head-dim, which `--head-dim-error-file` accepts" + test: | + set -euo pipefail + set +e + apr kernel parity --impl flash2 --ref naive --head-dim 96 --json -o /tmp/hd.json + EC=$? + set -e + [ "$EC" -ne 0 ] || exit 1 + apr attn-parity-lint --head-dim-error-file /tmp/hd.json + if_fails: "flash2 silently slow-paths an unsupported head_dim, or the refusal is + not capturable as an observation" + evidence_discharged_by: + producer_path: crates/apr-cli/src/commands/kernel_parity.rs + e2e_path: crates/apr-cli/src/commands/kernel_parity_tests.rs + e2e_tests: + - round_trip_head_dim_refusal_is_accepted_by_the_head_dim_gate + - head_dim_gate_rejects_an_error_body_that_is_not_about_head_dim + - flash2_is_refused_rather_than_answered_by_the_tiled_kernel + - flash2_at_a_supported_head_dim_still_refuses_without_the_kernel + - zero_head_dim_is_refused_with_a_head_dim_message + sub_claim: | + The head-dim refusal is emitted as `{"error": ...}` and accepted by the + head-dim gate; an unrelated error ("out of memory") is rejected by that + same gate, so the gate discriminates. `--impl flash2` is refused at every + head_dim, including the supported ones, because this binary embeds no + hf-kernels-community flash-attn2 kernel — and the refusal body carries no + `max_abs_diff`. + scope: "(impl, head_dim) -> refusal body -> head-dim gate exit status" + discharge_status: FULL_ROUND_TRIP + note: | + Exit code is apr's convention (5 = ValidationFailed for the head-dim + refusal, 12 = NotImplemented for the absent kernel), not the literal 1 the + older crux-L-02-v1 shell snippet asserts. Both are non-zero; the snippet + predates this exit-code table. +- id: FALSIFY-LINT-PRODUCER-005 + rule: "`apr debug embed-viz` CSV is accepted by `apr embed-viz-lint`" + prediction: "projecting a model's embedding table and linting the CSV exits 0 + for both pca and random, and two runs at one seed pass the determinism gate" + test: | + set -euo pipefail + apr debug embed-viz --model model.apr --projection random --seed 42 -o /tmp/a.csv + apr debug embed-viz --model model.apr --projection random --seed 42 -o /tmp/b.csv + apr embed-viz-lint --csv-file /tmp/a.csv --csv-file-b /tmp/b.csv + if_fails: "the F-18 producer and consumer disagree, or the projection is not + deterministic in --seed" + evidence_discharged_by: + producer_path: crates/apr-cli/src/commands/embed_viz.rs + cli_path: crates/apr-cli/src/commands/embed_viz_lint.rs + e2e_path: crates/apr-cli/src/commands/embed_viz_tests.rs + e2e_tests: + - round_trip_producer_csv_is_accepted_by_embed_viz_lint + - round_trip_gguf_producer_csv_is_accepted_by_embed_viz_lint + - round_trip_two_seeded_runs_pass_the_determinism_gate + - round_trip_cannot_pass_vacuously_when_the_csv_is_corrupted + - the_determinism_gate_rejects_two_different_csvs + - a_different_seed_moves_the_projection + - a_token_containing_a_comma_does_not_shift_the_column_count + sub_claim: | + The producer reads a real embedding tensor out of an APR fixture AND a + GGUF fixture, and both projections' CSVs pass schema + row-count. The + determinism gate passes on two same-seed runs AND fails on two + different-seed runs, so byte-identity is evidence rather than a tautology. + The negative half poisons a coordinate, drops a row, renames a header + column and negates a token id, requiring a refusal each time. + + The GGUF half was previously ABSENT, and this entry claimed the round trip + anyway. The only fixture was an APR table written `[VOCAB, HIDDEN]` — the + one layout where `shape[0]` really is the vocab axis — so the producer's + inversion of the GGUF axes was invisible to it while + `token_embd.weight` was, and is, the first name it looks for. On + `Qwen3.5-0.8B-Q4_K_M.gguf` the producer wrote 1024 rows for a + 248320-token vocabulary and `embed-viz-lint --expected-vocab-size 248320` + exited 5 on its own producer's output. See FALSIFY-LINT-PRODUCER-008. + scope: "(model, projection, seed, limit) -> CSV -> lint exit status, over APR + and GGUF fixtures. SafeTensors shares the row-major axis rule and is + covered by the FALSIFY-LINT-PRODUCER-008 case table, but has NO end-to-end + fixture here — that is the residual gap in this entry." + discharge_status: FULL_ROUND_TRIP +- id: FALSIFY-LINT-PRODUCER-006 + rule: "an algorithm the binary does not implement is refused, not relabelled" + prediction: "`--projection umap` exits non-zero and writes no CSV" + test: | + set -euo pipefail + set +e + apr debug embed-viz --model model.apr --projection umap -o /tmp/umap.csv + EC=$? + set -e + [ "$EC" -ne 0 ] || exit 1 + [ ! -f /tmp/umap.csv ] || exit 1 + if_fails: "a different projection shipped under UMAP's name — the exact + fabrication class aprender#2377 exists to eliminate" + evidence_discharged_by: + producer_path: crates/apr-cli/src/commands/embed_viz.rs + e2e_path: crates/apr-cli/src/commands/embed_viz_tests.rs + e2e_tests: + - umap_is_refused_rather_than_silently_substituted + - unresolved_tokens_are_marked_not_invented + - a_short_tokens_file_is_refused_rather_than_padded + sub_claim: | + UMAP is refused with `NotImplemented` and leaves no artifact behind. + Token text that cannot be resolved is written as the literal + `` rather than invented, and a `--tokens` file too short for + the projected rows is refused rather than padded. + scope: "(projection, tokens) -> non-zero exit + no artifact" + discharge_status: FULL_ROUND_TRIP +- id: FALSIFY-LINT-PRODUCER-007 + rule: "every `apr …` invocation quoted in a *-lint help string is runnable" + prediction: "resolving each backtick-quoted invocation against the clap tree succeeds" + test: | + set -euo pipefail + cargo test -p apr-cli --lib every_apr_command_quoted_in_lint_help_exists + if_fails: "a lint documents a producer the binary does not have — the defect + class aprender#2377 finding 3 reported" + evidence_discharged_by: + cli_path: crates/apr-cli/src/help_producer_truth.rs + e2e_path: crates/apr-cli/src/help_producer_truth.rs + e2e_tests: + - every_apr_command_quoted_in_lint_help_exists + - resolver_rejects_the_invocations_dogfooding_found + - resolver_accepts_real_invocations + sub_claim: | + The guard walks the real clap tree. Its own accept/reject case table is + re-run on every build, so the resolver cannot silently stop discriminating + — the three producers this contract adds moved from the reject list to the + accept list, which is how the guard proved they now exist. + scope: "(clap tree, help strings) -> unresolvable invocations" + discharge_status: FULL_ROUND_TRIP +- id: FALSIFY-LINT-PRODUCER-008 + rule: "`apr debug embed-viz` picks the vocabulary axis PER FORMAT" + prediction: "a GGUF `token_embd.weight` reported `[hidden, vocab]` yields one CSV + row per TOKEN, and the row count satisfies `--expected-vocab-size`" + test: | + set -euo pipefail + apr debug embed-viz --model model.gguf --projection random --seed 42 -o /tmp/e.csv + apr embed-viz-lint --csv-file /tmp/e.csv --expected-vocab-size "$VOCAB" + if_fails: "the producer emits one row per hidden dimension instead of per token, + and pairs each row's real token_str with coordinates projected across many + concatenated token vectors" + evidence_discharged_by: + producer_path: crates/apr-cli/src/commands/embed_viz.rs + e2e_path: crates/apr-cli/src/commands/embed_viz_tests.rs + e2e_tests: + - the_vocab_axis_is_chosen_per_format_not_assumed_to_be_axis_zero + - gguf_locate_embedding_returns_vocab_and_hidden_in_apr_order + - gguf_coordinates_come_from_the_hidden_axis + - a_vocabulary_larger_than_the_embedding_table_is_refused + - an_embedding_table_padded_above_the_token_list_is_accepted + sub_claim: | + GGUF reports GGML `ne` order, `ne[0]` contiguous, so `token_embd.weight` is + `[hidden, vocab]`; APR and SafeTensors are row-major `[vocab, hidden]`. + `contracts/tensor-layout-v1.yaml` states this, and it was re-measured + before the fix: reading the GGUF payload of `qwen2.5-coder-0.5b-instruct` + as `[vocab][hidden]` rows matches the same model's SafeTensors row at + cosine 0.999, the transposed reading at 0.014. Only the reported axis + ORDER differs between formats; the payload is `[vocab][hidden]` in all + three, so no restriding is needed once the axes are named. + + THE TRAP this entry exists to document: `token_str` is resolved from the + vocabulary by row index, so it reads correctly no matter which axis + produced the coordinates. No assertion on `token_str` can detect this. + The tests assert the ROW COUNT against the vocabulary and the COORDINATES + against a `hidden`-length slice (the fixture is rank-1, so a correct + projection is exactly linear in the token index). + + `check_vocab_axis` additionally refuses at run time when a model declares + more tokens than the chosen axis has rows. Padding only ever goes the + other way, so the one-sided check cannot fire spuriously — that direction + is pinned by `an_embedding_table_padded_above_the_token_list_is_accepted`. + scope: "(model format, reported 2-D shape) -> (vocab, hidden) -> CSV row count + and coordinate provenance" + discharge_status: FULL_ROUND_TRIP + +proof_obligations: +- type: invariant + property: "each *-lint help invocation resolves against the shipped clap tree" +- type: invariant + property: "each producer's own output is accepted by its lint (round trip)" +- type: invariant + property: "a corrupted producer body is rejected by that lint (non-vacuity)" +- type: invariant + property: "a configuration or codec the producer cannot run exits non-zero and + reports no measurement" +- type: invariant + property: "a producer's reported metric is the one it measured, not a constant" +- type: invariant + property: "the vocabulary axis of an embedding table is chosen per FORMAT" + +verification_summary: + total_obligations: 6 + proven: 0 + tested: 6 + status: tested + +pmat_work_tracking: + ticket_tag: apr-lint-producers + priority: high + auto_created: false diff --git a/contracts/apr-serve-cancellation-v1.yaml b/contracts/apr-serve-cancellation-v1.yaml index 2eef74e47c..11b3a042df 100644 --- a/contracts/apr-serve-cancellation-v1.yaml +++ b/contracts/apr-serve-cancellation-v1.yaml @@ -14,6 +14,15 @@ metadata: - 'Asserting that a cancellation flag was set proves nothing — the shipped defect is compatible with the flag being set and nobody reading it. Assert observed work (tokens produced) instead.' + - 'aprender#2375(1): the guard fired on NORMAL completion too, on the reasoning that + firing late is harmless. It is harmless only for a handler that generates inside + its own future. A streaming handler returns its SSE response while the decode loop + is still in prefill, so the guard cancelled it at the first poll and every streamed + chat reply was a well-formed event stream with zero content deltas. A completed + handler must DISARM the guard.' + - 'The falsifier for the streamed deltas called true_streaming_sse_response directly + and stayed green through all of it. A guard that never routes a request cannot see + a defect that lives in the middleware.' equations: tokens_generated_bound: @@ -62,13 +71,24 @@ equations: on client disconnect sets the flag - the inner handler runs in a separate task, so its decode loop is still alive to observe the flag after the drop + - a request that COMPLETES disarms the guard, so a response body still being + written by a background decode loop (every streaming route) is not cancelled by + its own success + - 'an abandoned STREAM is still stopped, by body-drop rather than by this guard: + hyper drops the response body, the SSE receiver drops, the generator on_token + send fails, and the loop breaks. Discharged by FALSIFY-SERVE-CANCEL-008, which + drives the real router and counts the abandonments the shared streaming sink + records: at least one (the drop reached the loop) and EXACTLY one (the loop + broke rather than kept failing to send)' - a request that COMPLETES returns a response byte-identical to the same handler invoked directly with CancelToken::never() preconditions: - the router carries the cancel_on_disconnect layer - the handler installs the extension token on its generation config postconditions: - - cancel.peek_cancelled() == true after the layer's future is dropped + - cancel.peek_cancelled() == true after the layer's future is dropped WITHOUT the + handler having returned + - cancel.peek_cancelled() == false after a request that ran to completion - the decode loop terminates with tokens_generated < cfg.max_tokens - a completed request's response body is unchanged by the layer @@ -117,13 +137,44 @@ falsification_tests: - id: FALSIFY-SERVE-CANCEL-005 name: the_real_router_hands_every_request_a_live_cancel_token prediction: 'A handler mounted under the cancel_on_disconnect layer reads a token - from the request extensions whose peek_cancelled() is true after the response future - is dropped — which CancelToken::never() can never report.' + from the request extensions that (a) is NOT cancelled after the request ran to + completion, and (b) reports cancelled once cancel() is called on it — which + CancelToken::never() can never do. (b) proves the token is live without depending + on the layer cancelling a request nobody abandoned.' test_harness: cargo test -p aprender-serve --lib the_real_router_hands_every_request_a_live_cancel_token expected_output: 'test result: ok' if_fails: 'PRE-FIX: no layer existed, so request_cancel_token fell back to CancelToken::never() and every decode loop polled a token that answers false forever. Also fails if - the layer is dropped from create_router.' + the layer is dropped from create_router, or if the guard is left ARMED on normal + completion — aprender#2375(1), which emptied every streaming response body.' +- id: FALSIFY-SERVE-CANCEL-007 + name: disarmed_guard_does_not_cancel_on_drop + prediction: 'CancelOnDrop::disarm() makes the guard drop a no-op: the token is not + cancelled. A second, still-armed guard on the same token does cancel, so the + default is unchanged and forgetting to disarm cannot silently lose the + disconnect behaviour.' + test_harness: cargo test -p aprender-serve --lib generate::cancel::tests + expected_output: 'test result: ok' + if_fails: 'Either the guard cannot be disarmed (aprender#2375(1): a completed + streaming handler cancels its own background decode loop before its first token), + or it is disarmed by default (the #2376(3) disconnect defect returns).' +- id: FALSIFY-SERVE-CANCEL-008 + name: an_abandoned_stream_is_stopped_by_the_body_drop + prediction: 'POST /v1/chat/completions {"stream":true,"max_tokens":64} through create_router + on a quantized server, with the response body DROPPED unread (max_tokens far exceeds + the 16-slot token channel, so the decode loop is provably still running), causes + MetricsCollector::streams_abandoned() to reach EXACTLY 1 — at least one, because the + body drop must reach the decode loop, and no more than one, because the loop must + then break rather than keep failing to send. A stream read to completion on the same + fixture records 0.' + test_harness: cargo test -p aprender-serve --lib an_abandoned_stream_is_stopped_by_the_body_drop + expected_output: 'test result: ok' + if_fails: 'The mechanism that replaced the guard for streaming responses does not work. + Zero abandonments means dropping the response body never reached the decode loop, so + an abandoned stream burns a core to max_tokens with nobody listening — aprender#2376(3), + reintroduced for exactly the case #2375(1) removed the guard from. More than one means + the loop kept generating after the client left (mutation-verified: making the sink + ignore the send result recorded 19 abandonments for one abandoned stream).' - id: FALSIFY-SERVE-CANCEL-006 name: a_completed_generate_request_is_unchanged_by_the_cancellation_layer prediction: 'POST /generate {"prompt":"token5","max_tokens":4} through create_router @@ -167,6 +218,24 @@ proof_obligations: Requires all three of: the token in the request extensions, the CancelOnDrop guard in the layer's future, and the handler running in its own task. Removing any one restores the defect; each removal is separately mutation-verified. +- type: invariant + property: An abandoned STREAM stops too, by body-drop rather than by the guard + formal: drop(response_body) => eventually the streaming decode loop exits, and it + does so after exactly one failed on_token send + applies_to: every streaming backend behind create_router (quantized, CUDA, + Qwen3-MoE), which all hand their decode loop the same + openai_handlers::streaming_token_sink + discharged_by: FALSIFY-SERVE-CANCEL-008 + notes: >- + aprender#2375(1) deliberately removed the guard's coverage of streaming + responses — a streaming handler COMPLETES while its decode loop is still + running, so the guard could not tell an abandoned stream from a healthy one + and cancelled both. This is the replacement mechanism, and it shipped + asserted-but-untested: the only abandonment falsifier (CANCEL-004) uses a + handler that never completes, i.e. the still-ARMED path. The three backends + each carried their own copy of the send-and-check callback; they now share + one, and it records the abandonment so the property is observable rather + than merely stated. - type: invariant property: Cancellation does not change a completed response formal: for all requests r that complete, response(router_with_layer, r) == @@ -174,8 +243,10 @@ proof_obligations: applies_to: POST /generate and every other route on create_router discharged_by: FALSIFY-SERVE-CANCEL-006 notes: >- - The guard fires on normal completion too, and the layer interposes a task. - Neither may alter a successful response. + The layer interposes a task, and the guard used to fire on normal completion + too. Neither may alter a successful response — and since aprender#2375(1), + "does not alter" includes the response BODY of a stream that is still being + produced when the handler returns. kani_harnesses: - id: KANI-SERVE-CANCEL-001 @@ -197,5 +268,7 @@ qa_gate: - Every generate handler extracts Extension and installs it on its generation config - A completed request returns the same body with and without the layer - pass_criteria: All six FALSIFY-SERVE-CANCEL falsifiers pass, and each has been + - Every streaming backend hands its decode loop openai_handlers::streaming_token_sink, + so a dropped response body stops it + pass_criteria: All seven FALSIFY-SERVE-CANCEL falsifiers pass, and each has been mutation-verified by removing the mechanism it covers and observing RED diff --git a/contracts/apr-serve-openai-compat-v1.yaml b/contracts/apr-serve-openai-compat-v1.yaml index 9cf8c12b0f..6aa65ff7f9 100644 --- a/contracts/apr-serve-openai-compat-v1.yaml +++ b/contracts/apr-serve-openai-compat-v1.yaml @@ -1,7 +1,7 @@ contract: apr-serve-openai-compat metadata: kind: pattern - version: "1.17.0" + version: "1.18.0" description: > OpenAI-compatible serve layer (/v1/chat/completions, /v1/completions, /v1/embeddings) fidelity invariants. Established by an adversarial audit @@ -410,7 +410,7 @@ falsification_tests: if_fails: 'The /v1/chat/completions handler ignores request.tools (the pre-PMAT-801 behavior — the in-tree tool-calling library had ZERO call sites outside grammar/), so a tool call in the model output is never surfaced as response tool_calls / finish_reason:"tool_calls"; OR a no-tools request is no longer byte-identical; OR arguments serializes as a nested object instead of a JSON string.' - id: FALSIFY-STREAM-TEMP-ZERO-790 name: pmat790_stream_temperature_zero_tests - prediction: 'resolve_stream_generation_config(temperature, top_p, max_tokens) maps temperature 0.0 to SamplingStrategy::Greedy with a positive (no-op 1.0) temperature, so sample_token() runs WITHOUT error and selects the argmax — directly exercising the model.generate -> sample_token -> apply_temperature chain the streaming handler runs. temperature 0 ignores top_p (stays Greedy). Positive temperatures are unchanged: greedy by default, top-p when set, and remain runnable.' + prediction: 'resolve_dense_generation_config(temperature, top_p, max_tokens) — the SHARED resolver, reached by /v1/chat/completions, /v1/completions and the /stream route alike — maps temperature 0.0 to SamplingStrategy::Greedy with a positive (no-op 1.0) temperature, so sample_token() runs WITHOUT error and selects the argmax — directly exercising the model.generate -> sample_token -> apply_temperature chain the streaming handler runs. temperature 0 ignores top_p (stays Greedy). Positive temperatures are unchanged: greedy by default, top-p when set, and remain runnable.' test_harness: 'cargo test -p aprender-serve --lib pmat790_stream_temperature_zero_tests' expected_output: "test result: ok" if_fails: 'The streaming /v1/chat/completions handler passes temperature 0.0 straight into GenerationConfig, so apply_temperature(0.0) returns Err and the handler answers HTTP 500 for every streaming chat completion with temperature 0 (the canonical OpenAI deterministic request) — the pre-PMAT-790 behavior.' @@ -540,3 +540,39 @@ falsification_tests: test_harness: 'cargo test -p aprender-serve --lib openai_compat_2375::no_shipped_string_leaks_an_internal_rust_constructor' expected_output: "test result: ok" if_fails: 'An HTTP error body instructs a client to call a Rust constructor it cannot reach, which is what #2375 finding 8 shipped.' + - id: FALSIFY-CHAT-STREAM-BODY-NONEMPTY-2375 + name: streamed_chat_body_carries_the_same_text_as_the_buffered_one + prediction: 'Through create_router (cancellation layer mounted), POST /v1/chat/completions with "stream":true on a quantized server returns text/event-stream whose concatenated choices[0].delta.content equals the non-streamed choices[0].message.content for the identical request — asserted twice, once through the tower service and once over a REAL socket served by axum::serve, so the result cannot be an artefact of oneshot. RED against the cancel_on_disconnect layer as first shipped, where the concatenation was "" because the per-request CancelOnDrop guard fired the instant the handler returned and the background decode loop observed it on its first poll.' + test_harness: 'cargo test -p aprender-serve --lib stream_and_metrics_2375::streamed_chat_body' + expected_output: "test result: ok" + if_fails: 'Every streaming chat reply is an empty but well-formed event stream: opening chunk, terminal chunk, [DONE], no content. The whitespace falsifier (sse_stream_whitespace) cannot see this because it calls true_streaming_sse_response directly and never routes a request.' + - id: FALSIFY-CHAT-STREAM-ROUTE-QUANTIZED-2375 + name: chat_completions_stream_route_serves_a_quantized_server + prediction: 'POST /v1/chat/completions/stream on a quantized-only AppState (what apr serve run model.gguf builds) answers 200 text/event-stream, terminates with data: [DONE], and its concatenated deltas equal the /v1/chat/completions body for the same request; the frame shapes of the two routes are identical. RED on 0.63.0, which answered 404 {"error":"Model registry error: No model available"} because the route resolved the dense f32 Model only.' + test_harness: 'cargo test -p aprender-serve --lib -- stream_and_metrics_2375::chat_completions_stream_route_serves_a_quantized_server stream_and_metrics_2375::stream_route_and_stream_flag_agree_on_the_frame_shape' + expected_output: "test result: ok" + if_fails: 'A route the server mounts unconditionally and prints in its own startup banner is dead on every GGUF/APR deployment, or has drifted back into a second, divergent implementation of chat completion.' + - id: FALSIFY-DENSE-TEMP-ZERO-2375 + name: temperature_zero_is_served_on_every_openai_route + prediction: '"temperature":0 — the OpenAI-canonical deterministic request — is answered 200 by /v1/chat/completions, /v1/chat/completions/stream AND /v1/completions on a dense server, and no response body contains "Temperature must be a positive". RED on the two non-stream routes, which answered 500 {"error":"Invalid shape: Temperature must be a positive finite number"} because PMAT-790 fixed one handler with a private copy of the resolution.' + test_harness: 'cargo test -p aprender-serve --lib stream_and_metrics_2375::temperature_zero_is_served_on_every_openai_route' + expected_output: "test result: ok" + if_fails: 'Deterministic decoding — what every eval harness and every reproducibility test asks for — is a server error on the dense backends.' + - id: FALSIFY-METRICS-PERCENTILES-MEASURED-2375 + name: v1_metrics_percentiles_are_measured_alongside_a_nonzero_average + prediction: 'After five completed chat requests on one AppState, GET /v1/metrics reports latency_p50_ms > 0, p50 <= p95 <= p99, and a p50 within an order of magnitude of the realizar_avg_latency_ms that GET /metrics reports for the SAME state at the same instant. Unit-level: MetricsCollector::latency_percentiles over 100 samples of 1..=100 ms returns exactly (50, 95, 99) ms — values no multiple of the 50.5 ms mean can produce — and returns None when nothing has been recorded.' + test_harness: 'cargo test -p aprender-serve --lib -- stream_and_metrics_2375::v1_metrics_percentiles_are_measured_alongside_a_nonzero_average percentiles_are_order_statistics_not_multiples_of_the_mean' + expected_output: "test result: ok" + if_fails: 'A monitor graphs a flat 0 ms p50/p95/p99 under real load (the GPU build, which is what cargo install produces), or graphs avg*1.5 and avg*2.0 as if they were the tail (the non-GPU build). Neither describes any request that happened.' + - id: FALSIFY-METRICS-MODEL-NAME-2375 + name: v1_metrics_model_name_is_derived_from_the_model_this_server_loaded + prediction: 'Two servers differing ONLY in the model file they were pointed at report two DIFFERENT model_names from GET /v1/metrics, each the file stem of its own model ("albor-370m-v1-q4_k_m" and "qwen2.5-coder-1.5b-instruct-q4k"). A server with a model but no source path reports the id /v1/models advertises ("default"); a server with no model reports "N/A" and latency_p50_ms 0.0. RED on 0.63.0, which reported "phi-2-q4_k_m" for any cached GPU model and "N/A" for everything else — and RED on any constant, which is what a single-fixture "not N/A" assertion could not do: the first version of this falsifier passed while the observed value was the constant "default" and the deriving branch ran in no test at all.' + test_harness: 'cargo test -p aprender-serve --lib -- stream_and_metrics_2375::v1_metrics_model_name_is_derived_from_the_model_this_server_loaded stream_and_metrics_2375::v1_metrics_reports_no_model_name_when_nothing_is_loaded' + expected_output: "test result: ok" + if_fails: 'Every server in a fleet reports the same wrong model name, or no name at all, to whatever is monitoring it.' + - id: FALSIFY-TEMP-DOMAIN-2375 + name: unservable_temperature_is_refused_on_every_generating_route + prediction: 'A temperature outside the servable domain (finite, >= 0) is refused with a 4xx that names the field, on all NINE routes of create_router_with_config that accept one and can generate: /v1/chat/completions, /v1/chat/completions/stream, /v1/completions, /v1/batch/completions, /api/chat, /api/generate, /generate, /stream/generate and /batch/generate (the last three validated temperature only inside their QUANTIZED backend, so on a dense server they answered 500 "Temperature must be a positive finite number" — measured, then fixed). Cases: -1 (negative) and 1e40 (finite as f64, +inf once narrowed to f32); 1e400 is refused by serde_json''s own number parser, so it is asserted only for the class it proves (client error, no sampler leak). No body contains "Temperature must be a positive". A servable 0.7 on the same route and the same server is still answered. Unit-level: resolve_dense_generation_config is TOTAL — 0, -1, -0, NaN, +inf, -inf and f32::MIN all resolve to a greedy config that sample_token runs without error, while 0.7 with top_p still resolves to nucleus sampling with the temperature unchanged.' + test_harness: 'cargo test -p aprender-serve --lib -- stream_and_metrics_2375::unservable_temperature_is_refused_on_every_generating_route stream_and_metrics_2375::temperature_domain_is_total' + expected_output: "test result: ok" + if_fails: 'Fixing temperature 0 alone leaves the rest of the domain reaching apply_temperature, which answers 500 {"error":"Invalid shape: Temperature must be a positive finite number"} — the exact body the fix set out to eliminate — on the dense backends, and serves inverted-distribution output with a 200 on the quantized ones (mutation-verified: removing the guard from ChatCompletionRequest answered temperature -1 with 200 OK and generated text). A comparison-only guard (t < 0.0) additionally lets NaN through, because every comparison with NaN is false (aprender#2391).' diff --git a/contracts/apr-stochastic-lr-v1.yaml b/contracts/apr-stochastic-lr-v1.yaml index 346585278a..844ecc56db 100644 --- a/contracts/apr-stochastic-lr-v1.yaml +++ b/contracts/apr-stochastic-lr-v1.yaml @@ -83,6 +83,22 @@ falsification_tests: cargo test -p aprender-core --lib -- test_minibatch_equals_batch 2>&1 | grep "ok" if_fails: "mini-batch gradient diverges from full-batch" +- id: FALSIFY-SGD-004 + rule: epoch shuffle arithmetic is width-portable and overflow-free + prediction: > + The Fisher-Yates partner index is computed in wrapping u64, so both SGD fit + modes run to completion under overflow checking on 64-bit and aprender-core + compiles for a 32-bit target. Refs aprender#2310. The grep is anchored on a + non-zero pass count: an unanchored `grep "ok"` matches + "test result: ok. 0 passed" and would pass on an empty filter. + test: | + cargo test -p aprender-core --lib -- classification::tests_sgd_portable_shuffle 2>&1 | grep -E "^test result: ok\. [1-9][0-9]* passed" + bash scripts/check_wasm32_core_builds.sh + if_fails: > + The MMIX LCG constants are back in a usize expression: 4 "literal out of + range for usize" errors on wasm32-unknown-unknown, and "attempt to multiply + with overflow" from epoch 3 / index 13 in any debug build on 64-bit. + proof_obligations: - type: invariant property: "default FitMode::Batch backward compatible" @@ -90,9 +106,13 @@ proof_obligations: property: "stochastic mode shuffles samples each epoch" - type: invariant property: "mini-batch(n) == full-batch for complete dataset" +- type: invariant + property: "shuffle_partner(seed, i) in [0, i] for all seed, i — the Fisher-Yates pass is a permutation" +- type: invariant + property: "shuffle_partner uses wrapping u64, never usize, so it is width-portable and cannot overflow" verification_summary: - total_obligations: 3 + total_obligations: 5 proven: 0 - tested: 0 + tested: 2 status: pending diff --git a/crates/apr-cli/src/commands/audio_inspect.rs b/crates/apr-cli/src/commands/audio_inspect.rs new file mode 100644 index 0000000000..17a8daf4eb --- /dev/null +++ b/crates/apr-cli/src/commands/audio_inspect.rs @@ -0,0 +1,339 @@ +//! `apr dataset audio-inspect` — the PRODUCER whose output `apr audio-inspect-lint` +//! reads (aprender#2377 finding 3). +//! +//! CRUX-H-13 shipped a consumer with no producer: `audio-inspect-lint`'s help +//! documented `apr dataset audio-inspect --format json` and the shipped binary +//! had neither a `dataset` command nor an `audio-inspect` one, so the lint's +//! gates had never run on real data and could not. This module decodes a real +//! RIFF/WAVE file and emits exactly the observation the H-13 classifier +//! consumes: `min`, `max`, `sample_rate`, `channels`, `samples`. +//! +//! ## What this claims, and what it does not +//! +//! It claims to have decoded uncompressed RIFF/WAVE PCM (u8 / i16 / i24 / i32) +//! and IEEE-float32 payloads and to report the *measured* amplitude extrema of +//! the decoded samples. It does NOT claim torchaudio parity beyond that: there +//! is no resampling, no channel mixdown, and no compressed-codec support. Every +//! container or codec it cannot decode is REFUSED with a non-zero exit and a +//! message naming what it found — never a plausible-looking number. +//! +//! Amplitude normalisation follows the torchaudio `load(normalize=True)` +//! convention: integer PCM is divided by the *negative* full-scale magnitude +//! (2^(bits-1)), so the range is [-1.0, 1.0 - 1ulp] and never exceeds ±1 for +//! integer input. Float payloads are reported as stored — a float WAV that +//! overshoots ±1 is a real property of that file and the lint is entitled to +//! reject it. + +use std::path::{Path, PathBuf}; + +use serde::Serialize; + +use crate::error::{refuse_overwrite, CliError, Result}; + +/// Decoded audio observation — the exact body `apr audio-inspect-lint` reads. +/// +/// Field names are load-bearing: `min`/`max`/`sample_rate`/`channels`/`samples` +/// are what `audio_inspect_classifier` looks up by key. +#[derive(Debug, Clone, PartialEq, Serialize)] +pub(crate) struct AudioObservation { + /// Source file, as given on the command line. + pub path: String, + /// Decoded codec, e.g. `pcm_s16le` or `pcm_f32le`. + pub codec: String, + /// Sample rate in Hz, straight from the `fmt ` chunk. + pub sample_rate: u32, + /// Channel count, straight from the `fmt ` chunk. + pub channels: u32, + /// Frames per channel (torchaudio's `num_frames`). + pub samples: u64, + /// `samples / sample_rate`. + pub duration_secs: f64, + /// Smallest decoded amplitude across every channel. + pub min: f64, + /// Largest decoded amplitude across every channel. + pub max: f64, + /// Bits per stored sample. + pub bits_per_sample: u16, +} + +/// Parsed `fmt ` chunk fields this decoder acts on. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct WavFmt { + format_tag: u16, + channels: u16, + sample_rate: u32, + block_align: u16, + bits_per_sample: u16, +} + +const FMT_PCM: u16 = 1; +const FMT_IEEE_FLOAT: u16 = 3; +const FMT_EXTENSIBLE: u16 = 0xFFFE; + +/// Run the producer: decode `path` and emit the observation. +pub(crate) fn run(path: &Path, json: bool, output: Option<&Path>, force: bool) -> Result<()> { + if !path.exists() { + return Err(CliError::FileNotFound(PathBuf::from(path))); + } + if let Some(out) = output { + refuse_overwrite(out, force)?; + } + + let obs = inspect(path)?; + let rendered = if json { + serde_json::to_string_pretty(&obs).map_err(|e| { + CliError::InvalidInput(format!("apr dataset audio-inspect: cannot serialize: {e}")) + })? + } else { + render_text(&obs) + }; + + match output { + Some(out) => std::fs::write(out, format!("{rendered}\n"))?, + None => println!("{rendered}"), + } + Ok(()) +} + +fn render_text(o: &AudioObservation) -> String { + format!( + "audio-inspect {}\n codec : {}\n sample_rate : {}\n channels : {}\n \ + samples : {}\n duration_secs: {:.6}\n min : {}\n max : {}", + o.path, o.codec, o.sample_rate, o.channels, o.samples, o.duration_secs, o.min, o.max + ) +} + +/// Decode `path` into an observation, or refuse with a message naming what was found. +pub(crate) fn inspect(path: &Path) -> Result { + let bytes = std::fs::read(path)?; + reject_known_non_wav(&bytes, path)?; + let fmt = parse_fmt(&bytes, path)?; + let (data, declared) = find_chunk(&bytes, b"data").ok_or_else(|| { + CliError::InvalidInput(format!( + "apr dataset audio-inspect: {} has no `data` chunk", + path.display() + )) + })?; + if declared > data.len() { + // The header promises more audio than the file holds. Reporting extrema + // over the surviving prefix would answer a question about a file that + // does not exist. + return Err(CliError::InvalidInput(format!( + "apr dataset audio-inspect: {} is truncated — its `data` chunk declares {declared} \ + bytes but only {} are present", + path.display(), + data.len() + ))); + } + let codec = codec_name(&fmt, path)?; + let frame_bytes = frame_bytes(&fmt, path)?; + let frames = frame_count(data.len(), frame_bytes, path)?; + let (min, max) = decode_extrema(data, &fmt, path)?; + + Ok(AudioObservation { + path: path.display().to_string(), + codec, + sample_rate: fmt.sample_rate, + channels: u32::from(fmt.channels), + samples: frames, + duration_secs: frames as f64 / f64::from(fmt.sample_rate), + min, + max, + bits_per_sample: fmt.bits_per_sample, + }) +} + +/// Name the container we were actually handed instead of reporting "no fmt chunk". +fn reject_known_non_wav(bytes: &[u8], path: &Path) -> Result<()> { + let named = |what: &str| { + Err(CliError::InvalidInput(format!( + "apr dataset audio-inspect: {} is {what}; this decoder reads uncompressed \ + RIFF/WAVE only (PCM u8/i16/i24/i32, IEEE float32)", + path.display() + ))) + }; + match bytes { + b if b.starts_with(b"fLaC") => named("a FLAC stream"), + b if b.starts_with(b"OggS") => named("an Ogg stream"), + b if b.starts_with(b"ID3") || b.starts_with(&[0xFF, 0xFB]) => named("an MP3 stream"), + b if b.len() >= 12 && b.starts_with(b"RIFF") && &b[8..12] == b"WAVE" => Ok(()), + b if b.starts_with(b"RIFF") => named("a RIFF file that is not WAVE"), + _ => named("not a RIFF/WAVE file (bad magic)"), + } +} + +/// Locate a top-level RIFF chunk by 4-byte id. +/// +/// Returns the payload actually present in the file AND the size the header +/// declared, so callers can tell a complete chunk from a truncated one. +fn find_chunk<'a>(bytes: &'a [u8], id: &[u8; 4]) -> Option<(&'a [u8], usize)> { + let mut pos = 12usize; // past "RIFF" "WAVE" + while pos + 8 <= bytes.len() { + let this_id = &bytes[pos..pos + 4]; + let size = u32::from_le_bytes([ + bytes[pos + 4], + bytes[pos + 5], + bytes[pos + 6], + bytes[pos + 7], + ]) as usize; + let start = pos + 8; + let end = start.checked_add(size)?.min(bytes.len()); + if this_id == id { + return Some((&bytes[start..end], size)); + } + // Chunks are word-aligned: an odd size carries one pad byte. + pos = start + size + (size & 1); + } + None +} + +fn parse_fmt(bytes: &[u8], path: &Path) -> Result { + let (raw, _declared) = find_chunk(bytes, b"fmt ").ok_or_else(|| { + CliError::InvalidInput(format!( + "apr dataset audio-inspect: {} has no `fmt ` chunk", + path.display() + )) + })?; + if raw.len() < 16 { + return Err(CliError::InvalidInput(format!( + "apr dataset audio-inspect: {} has a truncated `fmt ` chunk ({} bytes, need >= 16)", + path.display(), + raw.len() + ))); + } + let u16at = |i: usize| u16::from_le_bytes([raw[i], raw[i + 1]]); + let mut fmt = WavFmt { + format_tag: u16at(0), + channels: u16at(2), + sample_rate: u32::from_le_bytes([raw[4], raw[5], raw[6], raw[7]]), + block_align: u16at(12), + bits_per_sample: u16at(14), + }; + if fmt.format_tag == FMT_EXTENSIBLE { + // WAVE_FORMAT_EXTENSIBLE: the real tag is the first 2 bytes of the + // 16-byte SubFormat GUID at offset 24. + if raw.len() < 26 { + return Err(CliError::InvalidInput(format!( + "apr dataset audio-inspect: {} is WAVE_FORMAT_EXTENSIBLE but its `fmt ` chunk \ + is too short ({} bytes) to carry a SubFormat GUID", + path.display(), + raw.len() + ))); + } + fmt.format_tag = u16at(24); + } + validate_fmt(&fmt, path)?; + Ok(fmt) +} + +fn validate_fmt(fmt: &WavFmt, path: &Path) -> Result<()> { + let bad = |what: String| { + Err(CliError::InvalidInput(format!( + "{what} in {}", + path.display() + ))) + }; + if fmt.channels == 0 { + return bad("apr dataset audio-inspect: `fmt ` declares 0 channels".to_string()); + } + if fmt.sample_rate == 0 { + return bad("apr dataset audio-inspect: `fmt ` declares a 0 Hz sample rate".to_string()); + } + if fmt.bits_per_sample == 0 || fmt.bits_per_sample % 8 != 0 { + return bad(format!( + "apr dataset audio-inspect: unsupported bit depth {} (must be a positive multiple of 8)", + fmt.bits_per_sample + )); + } + Ok(()) +} + +fn codec_name(fmt: &WavFmt, path: &Path) -> Result { + match (fmt.format_tag, fmt.bits_per_sample) { + (FMT_PCM, 8) => Ok("pcm_u8".to_string()), + (FMT_PCM, 16) => Ok("pcm_s16le".to_string()), + (FMT_PCM, 24) => Ok("pcm_s24le".to_string()), + (FMT_PCM, 32) => Ok("pcm_s32le".to_string()), + (FMT_IEEE_FLOAT, 32) => Ok("pcm_f32le".to_string()), + (tag, bits) => Err(CliError::InvalidInput(format!( + "apr dataset audio-inspect: {} carries format tag {tag} at {bits} bits, which this \ + decoder cannot decode (supported: PCM 8/16/24/32-bit, IEEE float 32-bit)", + path.display() + ))), + } +} + +fn frame_bytes(fmt: &WavFmt, path: &Path) -> Result { + let computed = usize::from(fmt.channels) * usize::from(fmt.bits_per_sample / 8); + if fmt.block_align != 0 && usize::from(fmt.block_align) != computed { + return Err(CliError::InvalidInput(format!( + "apr dataset audio-inspect: {} declares block_align {} but {} channels x {} bits \ + needs {computed}; this decoder reads only tightly-packed frames", + path.display(), + fmt.block_align, + fmt.channels, + fmt.bits_per_sample + ))); + } + Ok(computed) +} + +fn frame_count(data_len: usize, frame_bytes: usize, path: &Path) -> Result { + if data_len % frame_bytes != 0 { + return Err(CliError::InvalidInput(format!( + "apr dataset audio-inspect: {} has a truncated final frame ({data_len} data bytes is \ + not a multiple of the {frame_bytes}-byte frame)", + path.display() + ))); + } + let frames = (data_len / frame_bytes) as u64; + if frames == 0 { + // Refusing beats inventing: min/max of an empty stream do not exist. + return Err(CliError::ValidationFailed(format!( + "apr dataset audio-inspect: {} decodes to 0 frames, so it has no amplitude to report", + path.display() + ))); + } + Ok(frames) +} + +/// Decode every sample and return the measured (min, max). +fn decode_extrema(data: &[u8], fmt: &WavFmt, path: &Path) -> Result<(f64, f64)> { + let width = usize::from(fmt.bits_per_sample / 8); + let mut min = f64::INFINITY; + let mut max = f64::NEG_INFINITY; + for raw in data.chunks_exact(width) { + let v = decode_sample(raw, fmt.format_tag, fmt.bits_per_sample); + if !v.is_finite() { + return Err(CliError::ValidationFailed(format!( + "apr dataset audio-inspect: {} contains a non-finite sample ({v})", + path.display() + ))); + } + min = min.min(v); + max = max.max(v); + } + Ok((min, max)) +} + +/// Decode one stored sample to the torchaudio-normalised amplitude domain. +fn decode_sample(raw: &[u8], format_tag: u16, bits: u16) -> f64 { + match (format_tag, bits) { + (FMT_PCM, 8) => (f64::from(raw[0]) - 128.0) / 128.0, + (FMT_PCM, 16) => f64::from(i16::from_le_bytes([raw[0], raw[1]])) / 32_768.0, + (FMT_PCM, 24) => { + // Sign-extend 24 bits into an i32 by placing them in the high bytes. + let v = i32::from_le_bytes([0, raw[0], raw[1], raw[2]]) >> 8; + f64::from(v) / 8_388_608.0 + } + (FMT_PCM, 32) => { + f64::from(i32::from_le_bytes([raw[0], raw[1], raw[2], raw[3]])) / 2_147_483_648.0 + } + // Only (FMT_IEEE_FLOAT, 32) reaches here; `codec_name` refuses the rest. + _ => f64::from(f32::from_le_bytes([raw[0], raw[1], raw[2], raw[3]])), + } +} + +#[cfg(test)] +#[path = "audio_inspect_tests.rs"] +mod tests; diff --git a/crates/apr-cli/src/commands/audio_inspect_tests.rs b/crates/apr-cli/src/commands/audio_inspect_tests.rs new file mode 100644 index 0000000000..7a2052c86f --- /dev/null +++ b/crates/apr-cli/src/commands/audio_inspect_tests.rs @@ -0,0 +1,288 @@ +//! Tests for the `apr dataset audio-inspect` producer (aprender#2377 finding 3). +//! +//! The load-bearing test here is `round_trip_*`: the producer's own output is +//! fed to `apr audio-inspect-lint` and the lint must ACCEPT it. A shape +//! assertion ("the JSON has a `sample_rate` key") cannot prove producer and +//! consumer agree; running the consumer can. + +use super::*; +use crate::commands::audio_inspect_lint; + +// ── fixtures ───────────────────────────────────────────────────────────── + +/// Build a RIFF/WAVE file in memory. +fn wav(format_tag: u16, channels: u16, rate: u32, bits: u16, payload: &[u8]) -> Vec { + let mut fmt = Vec::new(); + fmt.extend_from_slice(&format_tag.to_le_bytes()); + fmt.extend_from_slice(&channels.to_le_bytes()); + fmt.extend_from_slice(&rate.to_le_bytes()); + let block_align = channels * (bits / 8); + fmt.extend_from_slice(&(rate * u32::from(block_align)).to_le_bytes()); // byte rate + fmt.extend_from_slice(&block_align.to_le_bytes()); + fmt.extend_from_slice(&bits.to_le_bytes()); + + let mut out = Vec::new(); + out.extend_from_slice(b"RIFF"); + out.extend_from_slice(&(36 + payload.len() as u32).to_le_bytes()); + out.extend_from_slice(b"WAVE"); + out.extend_from_slice(b"fmt "); + out.extend_from_slice(&(fmt.len() as u32).to_le_bytes()); + out.extend_from_slice(&fmt); + out.extend_from_slice(b"data"); + out.extend_from_slice(&(payload.len() as u32).to_le_bytes()); + out.extend_from_slice(payload); + out +} + +fn i16_payload(samples: &[i16]) -> Vec { + samples.iter().flat_map(|s| s.to_le_bytes()).collect() +} + +/// 16 kHz stereo i16, peaks at -0.5 and +0.5 of full scale. +fn stereo_16k() -> Vec { + wav( + 1, + 2, + 16_000, + 16, + &i16_payload(&[-16384, 16384, 0, 8192, -8192, 4096]), + ) +} + +fn write(bytes: &[u8]) -> tempfile::NamedTempFile { + use std::io::Write; + let mut f = tempfile::NamedTempFile::new().expect("tempfile"); + f.write_all(bytes).expect("write"); + f.flush().expect("flush"); + f +} + +// ── ROUND TRIP: producer output must be accepted by its own lint ───────── + +#[test] +fn round_trip_producer_output_is_accepted_by_audio_inspect_lint() { + let src = write(&stereo_16k()); + let dir = tempfile::tempdir().expect("tempdir"); + let obs = dir.path().join("audio.json"); + + run(src.path(), true, Some(&obs), false).expect("producer must decode a valid 16-bit WAV"); + + // The consumer, run exactly as its help documents, on exactly what the + // producer wrote — with both optional assertions armed. + audio_inspect_lint::run(&obs, Some(16_000), Some(2), false) + .expect("audio-inspect-lint must accept the producer's own observation"); +} + +#[test] +fn round_trip_cannot_pass_vacuously_when_the_body_is_corrupted() { + let src = write(&stereo_16k()); + let dir = tempfile::tempdir().expect("tempdir"); + let obs = dir.path().join("audio.json"); + run(src.path(), true, Some(&obs), false).expect("producer"); + + let good: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&obs).expect("read")).expect("parse"); + + // Each corruption is a DIFFERENT gate of the lint; all three must reject. + for (label, mutate) in [ + ( + "amplitude above full scale", + Box::new(|v: &mut serde_json::Value| v["max"] = serde_json::json!(1.5)) + as Box, + ), + ( + "non-canonical sample rate", + Box::new(|v: &mut serde_json::Value| v["sample_rate"] = serde_json::json!(12_345)), + ), + ( + "zero frames", + Box::new(|v: &mut serde_json::Value| v["samples"] = serde_json::json!(0)), + ), + ] { + let mut bad = good.clone(); + mutate(&mut bad); + let path = dir.path().join("bad.json"); + std::fs::write(&path, serde_json::to_string(&bad).expect("ser")).expect("write"); + let err = audio_inspect_lint::run(&path, None, None, false) + .expect_err(&format!("lint must reject: {label}")); + assert!( + matches!(err, CliError::ValidationFailed(_)), + "{label}: expected a validation refusal, got {err:?}" + ); + } +} + +// ── decode correctness ─────────────────────────────────────────────────── + +#[test] +fn decodes_i16_stereo_amplitudes_and_frame_count() { + let src = write(&stereo_16k()); + let obs = inspect(src.path()).expect("decode"); + assert_eq!(obs.sample_rate, 16_000); + assert_eq!(obs.channels, 2); + assert_eq!(obs.samples, 3, "6 i16 samples over 2 channels is 3 frames"); + assert_eq!(obs.codec, "pcm_s16le"); + assert!((obs.min - -0.5).abs() < 1e-12, "min was {}", obs.min); + assert!((obs.max - 0.5).abs() < 1e-12, "max was {}", obs.max); +} + +#[test] +fn i16_full_negative_scale_normalises_to_exactly_minus_one() { + let src = write(&wav(1, 1, 8_000, 16, &i16_payload(&[i16::MIN, i16::MAX]))); + let obs = inspect(src.path()).expect("decode"); + assert!((obs.min - -1.0).abs() < 1e-12, "min was {}", obs.min); + assert!( + obs.max < 1.0, + "i16::MAX must stay below +1.0, got {}", + obs.max + ); +} + +#[test] +fn decodes_24_bit_pcm_with_sign_extension() { + // -8_388_608 (full negative scale) then +8_388_607, little-endian 3-byte. + let payload = vec![0x00, 0x00, 0x80, 0xFF, 0xFF, 0x7F]; + let src = write(&wav(1, 1, 48_000, 24, &payload)); + let obs = inspect(src.path()).expect("decode"); + assert_eq!(obs.codec, "pcm_s24le"); + assert!((obs.min - -1.0).abs() < 1e-12, "min was {}", obs.min); + assert!(obs.max > 0.999_999, "max was {}", obs.max); +} + +#[test] +fn decodes_u8_pcm_around_the_128_midpoint() { + let src = write(&wav(1, 1, 8_000, 8, &[0u8, 128, 255])); + let obs = inspect(src.path()).expect("decode"); + assert_eq!(obs.codec, "pcm_u8"); + assert!((obs.min - -1.0).abs() < 1e-12, "min was {}", obs.min); + assert!( + (obs.max - (127.0 / 128.0)).abs() < 1e-12, + "max was {}", + obs.max + ); +} + +#[test] +fn reads_the_subformat_guid_of_wave_format_extensible() { + // 0xFFFE with a 40-byte fmt whose SubFormat GUID starts with the PCM tag. + let mut fmt = Vec::new(); + fmt.extend_from_slice(&0xFFFEu16.to_le_bytes()); + fmt.extend_from_slice(&1u16.to_le_bytes()); // channels + fmt.extend_from_slice(&44_100u32.to_le_bytes()); + fmt.extend_from_slice(&88_200u32.to_le_bytes()); + fmt.extend_from_slice(&2u16.to_le_bytes()); // block align + fmt.extend_from_slice(&16u16.to_le_bytes()); // bits + fmt.extend_from_slice(&22u16.to_le_bytes()); // cbSize + fmt.extend_from_slice(&16u16.to_le_bytes()); // valid bits + fmt.extend_from_slice(&0u32.to_le_bytes()); // channel mask + fmt.extend_from_slice(&1u16.to_le_bytes()); // SubFormat: PCM + fmt.extend_from_slice(&[0u8; 14]); + + let payload = i16_payload(&[-16384, 16384]); + let mut bytes = Vec::new(); + bytes.extend_from_slice(b"RIFF"); + bytes.extend_from_slice(&(20 + fmt.len() as u32 + payload.len() as u32).to_le_bytes()); + bytes.extend_from_slice(b"WAVE"); + bytes.extend_from_slice(b"fmt "); + bytes.extend_from_slice(&(fmt.len() as u32).to_le_bytes()); + bytes.extend_from_slice(&fmt); + bytes.extend_from_slice(b"data"); + bytes.extend_from_slice(&(payload.len() as u32).to_le_bytes()); + bytes.extend_from_slice(&payload); + + let src = write(&bytes); + let obs = inspect(src.path()).expect("decode"); + assert_eq!(obs.sample_rate, 44_100); + assert_eq!(obs.codec, "pcm_s16le"); +} + +// ── honest refusals: never a plausible number for a file we cannot read ── + +#[test] +fn refuses_a_flac_stream_by_name() { + let src = write(b"fLaC\x00\x00\x00\x22and then some"); + let err = inspect(src.path()).expect_err("FLAC must be refused, not guessed at"); + assert!(err.to_string().contains("FLAC"), "got: {err}"); +} + +#[test] +fn refuses_a_non_riff_file() { + let src = write(b"this is not audio at all"); + let err = inspect(src.path()).expect_err("bad magic must be refused"); + assert!(err.to_string().contains("bad magic"), "got: {err}"); +} + +#[test] +fn refuses_a_truncated_data_chunk_instead_of_reporting_the_prefix() { + let mut bytes = stereo_16k(); + bytes.truncate(bytes.len() - 4); // header still promises the full payload + let src = write(&bytes); + let err = inspect(src.path()).expect_err("a truncated file must be refused"); + assert!(err.to_string().contains("truncated"), "got: {err}"); +} + +#[test] +fn refuses_an_empty_stream_rather_than_reporting_zero_amplitude() { + let src = write(&wav(1, 1, 16_000, 16, &[])); + let err = inspect(src.path()).expect_err("0 frames has no amplitude to report"); + assert!(err.to_string().contains("0 frames"), "got: {err}"); +} + +#[test] +fn refuses_an_unsupported_codec_tag() { + // 0x0011 = IMA ADPCM — a real WAV tag this decoder cannot decode. + let src = write(&wav(0x0011, 1, 16_000, 16, &i16_payload(&[1, 2]))); + let err = inspect(src.path()).expect_err("ADPCM must be refused"); + assert!(err.to_string().contains("format tag 17"), "got: {err}"); +} + +#[test] +fn refuses_a_zero_channel_header() { + let src = write(&wav(1, 0, 16_000, 16, &i16_payload(&[1, 2]))); + let err = inspect(src.path()).expect_err("0 channels is not decodable"); + assert!(err.to_string().contains("0 channels"), "got: {err}"); +} + +#[test] +fn refuses_a_missing_file() { + let err = run(Path::new("/no/such/audio.wav"), true, None, false) + .expect_err("a missing input must not be reported on"); + assert!(matches!(err, CliError::FileNotFound(_)), "{err:?}"); +} + +#[test] +fn refuses_to_clobber_an_existing_output_without_force() { + let src = write(&stereo_16k()); + let existing = write(b"precious"); + let err = run(src.path(), true, Some(existing.path()), false) + .expect_err("an existing output must not be overwritten silently"); + assert!(err.to_string().contains("--force"), "got: {err}"); +} + +// ── emitted shape ──────────────────────────────────────────────────────── + +#[test] +fn json_body_carries_the_five_keys_the_h13_classifier_reads() { + let src = write(&stereo_16k()); + let obs = inspect(src.path()).expect("decode"); + let v = serde_json::to_value(&obs).expect("serialize"); + for key in ["min", "max", "sample_rate", "channels", "samples"] { + assert!( + v.get(key).is_some(), + "the H-13 classifier reads `{key}` by name; it is absent from {v}" + ); + } + assert!( + !v["sample_rate"].is_string(), + "sample_rate must be a JSON number, not a rendering of one: {v}" + ); +} + +#[test] +fn text_output_names_the_measured_fields() { + let src = write(&stereo_16k()); + let obs = inspect(src.path()).expect("decode"); + let text = render_text(&obs); + assert!(text.contains("sample_rate : 16000"), "got: {text}"); + assert!(text.contains("channels : 2"), "got: {text}"); +} diff --git a/crates/apr-cli/src/commands/embed_viz.rs b/crates/apr-cli/src/commands/embed_viz.rs new file mode 100644 index 0000000000..d2eb0122b0 --- /dev/null +++ b/crates/apr-cli/src/commands/embed_viz.rs @@ -0,0 +1,426 @@ +//! `apr debug embed-viz` — the PRODUCER whose output `apr embed-viz-lint` reads +//! (aprender#2377 finding 3). +//! +//! CRUX-F-18 shipped a consumer with no producer: `embed-viz-lint`'s help +//! documented `apr debug embed-viz --seed N -o emb.csv` and `apr debug` had no +//! such subcommand, so the schema, row-count and determinism gates had never +//! run on real data and could not. +//! +//! ## What this claims +//! +//! It reads a REAL token-embedding matrix out of a GGUF / APR / SafeTensors +//! model (dequantising as needed, via `RosettaStone::load_tensor_f32`) and +//! projects it to 2-D with a named, deterministic method: +//! +//! * `--projection pca` — exact PCA onto the top 2 principal components +//! (`aprender::preprocessing::PCA`). Deterministic; cost is O(hidden²) +//! memory for the covariance eigendecomposition. +//! * `--projection random` — seeded Johnson–Lindenstrauss random projection. +//! Deterministic in `--seed`, cheap at any hidden size. +//! +//! It does **not** implement UMAP, and `--projection umap` is REFUSED with a +//! non-zero exit rather than silently substituting a different algorithm and +//! labelling the output "umap". The CSV header and the `projection` note the +//! producer prints name the method that actually ran. +//! +//! `token_str` is resolved from the model's own GGUF vocabulary, or from +//! `--tokens`. When neither is available every row carries the literal +//! `` — a marker that claims nothing — and the producer says so on +//! stderr. Token text is escaped (`\` → `\\`, `,` → `\x2c`, `"` → `\x22`, +//! CR/LF → `\r`/`\n`) so a token containing a comma cannot silently shift the +//! column count the F-18 classifier counts. + +use std::path::{Path, PathBuf}; + +use aprender::format::rosetta::{FormatType, RosettaStone}; +use aprender::format::tensors::{list_tensors, TensorListOptions}; +use aprender::text::llama_tokenizer::LlamaTokenizer; + +use crate::error::{refuse_overwrite, CliError, Result}; + +/// `--projection` is declared at the crate root (`extended_commands.rs`) because +/// `ExtendedCommands` is public and `mod commands` is not. +pub(crate) use crate::EmbedProjection as Projection; + +/// Name the projection that actually ran — this string goes in the report, so +/// it must never say `umap` for something else. +pub(crate) fn projection_label(p: Projection) -> &'static str { + match p { + Projection::Pca => "pca", + Projection::Random => "random", + Projection::Umap => "umap", + } +} + +/// Tensor names that hold token embeddings across the architectures apr reads. +pub(crate) const EMBEDDING_TENSOR_CANDIDATES: [&str; 6] = [ + "token_embd.weight", + "model.embed_tokens.weight", + "tok_embeddings.weight", + "transformer.wte.weight", + "wte.weight", + "embeddings.word_embeddings.weight", +]; + +/// Options for one `embed-viz` run. +#[derive(Debug, Clone)] +pub(crate) struct EmbedVizArgs { + pub model: PathBuf, + pub tensor: Option, + pub projection: Projection, + pub seed: u64, + pub limit: Option, + pub tokens: Option, + pub output: Option, + pub force: bool, +} + +/// Run the producer. +pub(crate) fn run(args: &EmbedVizArgs) -> Result<()> { + if args.projection == Projection::Umap { + return Err(CliError::NotImplemented( + "apr debug embed-viz: --projection umap is not implemented in this binary. \ + Refusing rather than labelling a different algorithm's output `umap`. \ + Use --projection pca (exact) or --projection random (seeded JL)." + .to_string(), + )); + } + if !args.model.exists() { + return Err(CliError::FileNotFound(args.model.clone())); + } + if let Some(out) = &args.output { + refuse_overwrite(out, args.force)?; + } + + let (name, vocab, hidden) = locate_embedding(&args.model, args.tensor.as_deref())?; + // Read the model's own vocabulary ONCE. It serves two purposes: it supplies + // `token_str` below, and its length cross-checks the axis chosen above + // against something the file states independently of the tensor shape. + let vocab_list = gguf_vocab(&args.model); + if let Some(tokenizer) = &vocab_list { + check_vocab_axis(tokenizer.vocab_size(), vocab, &name, &args.model)?; + } + let data = RosettaStone::new() + .load_tensor_f32(&args.model, &name) + .map_err(|e| { + CliError::ValidationFailed(format!( + "apr debug embed-viz: cannot read tensor `{name}` from {}: {e}", + args.model.display() + )) + })?; + if data.len() != vocab * hidden { + return Err(CliError::ValidationFailed(format!( + "apr debug embed-viz: tensor `{name}` says {vocab}x{hidden} but decoded to {} values", + data.len() + ))); + } + + let rows = args.limit.map_or(vocab, |n| n.min(vocab)); + if rows == 0 { + return Err(CliError::ValidationFailed( + "apr debug embed-viz: 0 rows selected, so there is nothing to project".to_string(), + )); + } + let coords = project(&data[..rows * hidden], rows, hidden, args)?; + let tokens = resolve_tokens(args, rows, vocab_list.as_ref())?; + let csv = render_csv(&coords, &tokens); + + match &args.output { + Some(out) => std::fs::write(out, &csv)?, + None => print!("{csv}"), + } + eprintln!( + "embed-viz: {name} vocab={vocab} hidden={hidden} -> {rows} rows, projection={}, \ + seed={}, token_str={}", + projection_label(args.projection), + args.seed, + tokens.source + ); + Ok(()) +} + +/// Refuse when the model declares more tokens than the chosen vocab axis has rows. +/// +/// An embedding table may be padded ABOVE the token list (llama.cpp rounds the +/// row count up), so `declared <= vocab` is the sound one-sided invariant. The +/// reverse — a vocabulary larger than the table that is supposed to embed it — +/// means the axes were read the wrong way round, which is exactly how the GGUF +/// defect shipped: 248320 declared tokens against a 1024-row "vocab" axis. +fn check_vocab_axis(declared: usize, vocab: usize, name: &str, model: &Path) -> Result<()> { + if declared > vocab { + return Err(CliError::ValidationFailed(format!( + "apr debug embed-viz: {} declares {declared} tokens but tensor `{name}` offers only \ + {vocab} embedding rows. Every token must have a row, so the vocabulary axis was \ + read the wrong way round for this format.", + model.display() + ))); + } + Ok(()) +} + +/// Which axis of a REPORTED 2-D shape is the vocabulary — this differs by FORMAT. +/// +/// The bug this exists to prevent: `shape[0]` was taken as the vocab axis for +/// every format. That is right for APR/SafeTensors and INVERTED for GGUF, so on +/// `Qwen3.5-0.8B-Q4_K_M.gguf` (`token_embd.weight` reported `[1024, 248320]`) +/// the producer emitted 1024 rows for a 248320-token vocabulary and paired each +/// row's real `token_str` with coordinates projected from ~242 concatenated +/// token vectors. `apr embed-viz-lint --expected-vocab-size 248320` then exited +/// 5 on the producer's own output. +/// +/// ## The rule, measured rather than assumed +/// +/// * **GGUF** reports GGML `ne` order, `ne[0]` being the CONTIGUOUS dimension: +/// `token_embd.weight` is `[hidden, vocab]`. `contracts/tensor-layout-v1.yaml` +/// states this (`gguf_shape_formula: "[hidden, vocab]"` against +/// `apr_shape_formula: "[vocab, hidden]"`). +/// * **APR** and **SafeTensors** are row-major `[vocab, hidden]`. +/// +/// The DATA is `[vocab][hidden]` with `hidden` contiguous in all three — only +/// the reported axis ORDER differs, so no restriding is needed once the axes are +/// named correctly. That was verified against the same model in both formats +/// (`qwen2.5-coder-0.5b-instruct` GGUF vs SafeTensors): reading the GGUF payload +/// as `[vocab][hidden]` rows matched the SafeTensors row at cosine **0.999**, +/// while the transposed reading matched at **0.014**. +/// +/// For a non-embedding 2-D tensor named via `--tensor` the same rule yields +/// `(out_dim, in_dim)` — the row-major interpretation — which is what the +/// row-slicing projection needs. +pub(crate) fn embedding_axes(format: FormatType, shape: &[usize]) -> (usize, usize) { + match format { + // GGML `ne` order: ne[0] is contiguous, so [hidden, vocab]. + FormatType::Gguf => (shape[1], shape[0]), + // Row-major [vocab, hidden]. + FormatType::SafeTensors | FormatType::Apr => (shape[0], shape[1]), + } +} + +/// Identify the container format, so `embedding_axes` can apply the right rule. +fn detect_format(model: &Path) -> Result { + FormatType::from_magic(model) + .or_else(|_| FormatType::from_extension(model)) + .map_err(|e| { + CliError::ValidationFailed(format!( + "apr debug embed-viz: cannot determine the format of {}: {e}", + model.display() + )) + }) +} + +/// Find the embedding tensor and its `(vocab, hidden)` extent. +/// +/// The returned pair is always in APR/row-major order regardless of the source +/// format — see `embedding_axes`. +fn locate_embedding(model: &Path, requested: Option<&str>) -> Result<(String, usize, usize)> { + let format = detect_format(model)?; + let listing = list_tensors(model, TensorListOptions::default()).map_err(|e| { + CliError::ValidationFailed(format!( + "apr debug embed-viz: cannot list tensors in {}: {e}", + model.display() + )) + })?; + let info = match requested { + Some(want) => listing + .tensors + .iter() + .find(|t| t.name == want) + .ok_or_else(|| { + CliError::ValidationFailed(format!( + "apr debug embed-viz: {} has no tensor named `{want}`", + model.display() + )) + })?, + None => listing + .tensors + .iter() + .find(|t| EMBEDDING_TENSOR_CANDIDATES.contains(&t.name.as_str())) + .ok_or_else(|| { + CliError::ValidationFailed(format!( + "apr debug embed-viz: {} has none of the known embedding tensors {:?}; \ + name one explicitly with --tensor", + model.display(), + EMBEDDING_TENSOR_CANDIDATES + )) + })?, + }; + if info.shape.len() != 2 || info.shape[0] == 0 || info.shape[1] == 0 { + return Err(CliError::ValidationFailed(format!( + "apr debug embed-viz: tensor `{}` has shape {:?}; an embedding table must be \ + 2-D [vocab, hidden]", + info.name, info.shape + ))); + } + let (vocab, hidden) = embedding_axes(format, &info.shape); + Ok((info.name.clone(), vocab, hidden)) +} + +// ── projection ─────────────────────────────────────────────────────────── + +fn project( + data: &[f32], + rows: usize, + hidden: usize, + args: &EmbedVizArgs, +) -> Result> { + let coords = match args.projection { + Projection::Pca => project_pca(data, rows, hidden)?, + Projection::Random => project_random(data, rows, hidden, args.seed), + // `run` refuses Umap before reaching here. + Projection::Umap => unreachable!("umap is refused at entry"), + }; + if let Some((i, bad)) = coords + .iter() + .enumerate() + .find(|(_, (x, y))| !x.is_finite() || !y.is_finite()) + { + return Err(CliError::ValidationFailed(format!( + "apr debug embed-viz: row {i} projected to a non-finite coordinate {bad:?}; \ + refusing to write a CSV a consumer would have to reject" + ))); + } + Ok(coords) +} + +fn project_pca(data: &[f32], rows: usize, hidden: usize) -> Result> { + use aprender::preprocessing::PCA; + use aprender::primitives::Matrix; + use aprender::traits::Transformer; + + if rows < 2 { + return Err(CliError::ValidationFailed( + "apr debug embed-viz: --projection pca needs at least 2 rows; \ + use --projection random for a single row" + .to_string(), + )); + } + let x = Matrix::from_vec(rows, hidden, data.to_vec()) + .map_err(|e| CliError::ValidationFailed(format!("apr debug embed-viz: {e}")))?; + let mut pca = PCA::new(2); + pca.fit(&x) + .map_err(|e| CliError::ValidationFailed(format!("apr debug embed-viz: PCA fit: {e}")))?; + let y = pca.transform(&x).map_err(|e| { + CliError::ValidationFailed(format!("apr debug embed-viz: PCA transform: {e}")) + })?; + Ok((0..rows) + .map(|i| (f64::from(y.get(i, 0)), f64::from(y.get(i, 1)))) + .collect()) +} + +/// Seeded Johnson–Lindenstrauss projection: X · R / sqrt(hidden), R ~ U[-1, 1). +fn project_random(data: &[f32], rows: usize, hidden: usize, seed: u64) -> Vec<(f64, f64)> { + let mut rng = super::kernel_parity::SplitMix64::new(seed ^ 0xF18_F18_F18); + let r: Vec = (0..hidden * 2).map(|_| rng.next_unit()).collect(); + let norm = 1.0 / (hidden as f64).sqrt(); + (0..rows) + .map(|i| { + let row = &data[i * hidden..(i + 1) * hidden]; + let mut x = 0.0f64; + let mut y = 0.0f64; + for (d, value) in row.iter().enumerate() { + x += f64::from(*value) * f64::from(r[d * 2]); + y += f64::from(*value) * f64::from(r[d * 2 + 1]); + } + (x * norm, y * norm) + }) + .collect() +} + +// ── token text ─────────────────────────────────────────────────────────── + +/// Resolved token strings plus a note saying where they came from. +pub(crate) struct ResolvedTokens { + pub strings: Vec, + pub source: String, +} + +const UNRESOLVED: &str = ""; + +fn resolve_tokens( + args: &EmbedVizArgs, + rows: usize, + vocab_list: Option<&LlamaTokenizer>, +) -> Result { + if let Some(path) = &args.tokens { + let text = std::fs::read_to_string(path)?; + let mut strings: Vec = text.lines().map(str::to_string).collect(); + if strings.len() < rows { + return Err(CliError::ValidationFailed(format!( + "apr debug embed-viz: --tokens {} holds {} lines but {rows} rows were projected", + path.display(), + strings.len() + ))); + } + strings.truncate(rows); + return Ok(ResolvedTokens { + strings, + source: format!("--tokens {}", path.display()), + }); + } + if let Some(strings) = vocab_list.and_then(|t| take_tokens(t, rows)) { + return Ok(ResolvedTokens { + strings, + source: "gguf tokenizer.ggml.tokens".to_string(), + }); + } + eprintln!( + "embed-viz: no token text available for {} — every token_str is `{UNRESOLVED}`. \ + Pass --tokens FILE to resolve them.", + args.model.display() + ); + Ok(ResolvedTokens { + strings: vec![UNRESOLVED.to_string(); rows], + source: UNRESOLVED.to_string(), + }) +} + +/// Read `tokenizer.ggml.tokens` out of a GGUF, when the model is one. +/// +/// Returns the tokenizer rather than a token slice so the caller pays the file +/// read ONCE and can also ask it how many tokens the model declares — the +/// cross-check in `check_vocab_axis`. +fn gguf_vocab(model: &Path) -> Option { + let bytes = std::fs::read(model).ok()?; + if !bytes.starts_with(b"GGUF") { + return None; + } + LlamaTokenizer::from_gguf_bytes(&bytes).ok() +} + +/// The first `rows` token strings, or `None` if the vocabulary cannot cover them. +fn take_tokens(tokenizer: &LlamaTokenizer, rows: usize) -> Option> { + let mut out = Vec::with_capacity(rows); + for id in 0..rows { + out.push(tokenizer.id_to_token(u32::try_from(id).ok()?)?.to_string()); + } + Some(out) +} + +/// Escape token text so it can never change the CSV column count. +pub(crate) fn escape_token(raw: &str) -> String { + let mut out = String::with_capacity(raw.len()); + for ch in raw.chars() { + match ch { + '\\' => out.push_str("\\\\"), + ',' => out.push_str("\\x2c"), + '"' => out.push_str("\\x22"), + '\n' => out.push_str("\\n"), + '\r' => out.push_str("\\r"), + c => out.push(c), + } + } + out +} + +/// Render the `token_id,token_str,x,y` CSV the F-18 classifier parses. +pub(crate) fn render_csv(coords: &[(f64, f64)], tokens: &ResolvedTokens) -> String { + let mut out = String::from("token_id,token_str,x,y\n"); + for (id, (x, y)) in coords.iter().enumerate() { + let token = tokens.strings.get(id).map_or(UNRESOLVED, String::as_str); + out.push_str(&format!("{id},{},{x:.6},{y:.6}\n", escape_token(token))); + } + out +} + +#[cfg(test)] +#[path = "embed_viz_tests.rs"] +mod tests; diff --git a/crates/apr-cli/src/commands/embed_viz_tests.rs b/crates/apr-cli/src/commands/embed_viz_tests.rs new file mode 100644 index 0000000000..25bea59470 --- /dev/null +++ b/crates/apr-cli/src/commands/embed_viz_tests.rs @@ -0,0 +1,597 @@ +//! Tests for the `apr debug embed-viz` producer (aprender#2377 finding 3). +//! +//! The load-bearing test is `round_trip_*`: the CSV the producer writes is fed +//! to `apr embed-viz-lint` and the lint must ACCEPT it — schema, row count and +//! determinism. The negative half corrupts the CSV and requires the lint to +//! reject it, so the round trip cannot pass vacuously. + +use super::*; +use crate::commands::{embed_viz_classifier, embed_viz_lint}; + +const VOCAB: usize = 24; +const HIDDEN: usize = 8; + +/// GGUF fixture extents. Deliberately UNEQUAL and different from the APR +/// fixture's, so an axis mixup cannot hide behind a coincidence. +const G_VOCAB: usize = 24; +const G_HIDDEN: usize = 6; + +/// A minimal APR v2 model carrying a real `model.embed_tokens.weight` table. +fn model_fixture() -> tempfile::NamedTempFile { + use aprender::format::v2::{AprV2Metadata, AprV2Writer}; + + let file = tempfile::NamedTempFile::with_suffix(".apr").expect("tempfile"); + let mut metadata = AprV2Metadata::new("embed-viz-fixture"); + metadata.architecture = Some("llama".to_string()); + metadata.hidden_size = Some(HIDDEN); + metadata.vocab_size = Some(VOCAB); + + let mut writer = AprV2Writer::new(metadata); + // A table with structure, so PCA has a real principal direction to find. + let data: Vec = (0..VOCAB * HIDDEN) + .map(|i| { + let row = (i / HIDDEN) as f32; + let col = (i % HIDDEN) as f32; + (row * 0.1).mul_add(col + 1.0, (col * 0.37).sin()) + }) + .collect(); + writer.add_f32_tensor("model.embed_tokens.weight", vec![VOCAB, HIDDEN], &data); + let bytes = writer.write().expect("write APR v2"); + std::fs::write(file.path(), bytes).expect("write file"); + file +} + +fn args(model: &Path, out: &Path, projection: Projection) -> EmbedVizArgs { + EmbedVizArgs { + model: model.to_path_buf(), + tensor: None, + projection, + seed: 42, + limit: None, + tokens: None, + output: Some(out.to_path_buf()), + force: false, + } +} + +// ── ROUND TRIP ─────────────────────────────────────────────────────────── + +#[test] +fn round_trip_producer_csv_is_accepted_by_embed_viz_lint() { + let model = model_fixture(); + let dir = tempfile::tempdir().expect("tempdir"); + + for projection in [Projection::Pca, Projection::Random] { + let csv = dir + .path() + .join(format!("{}.csv", projection_label(projection))); + run(&args(model.path(), &csv, projection)).expect("producer must project the table"); + + embed_viz_lint::run(&csv, Some(VOCAB), None, false).unwrap_or_else(|e| { + panic!( + "embed-viz-lint must accept the producer's own {} CSV: {e}", + projection_label(projection) + ) + }); + } +} + +/// FALSIFY-CRUX-F-18-003: two runs under the same seed must be byte-identical, +/// which is exactly what the lint's `--csv-file-b` gate checks. +#[test] +fn round_trip_two_seeded_runs_pass_the_determinism_gate() { + let model = model_fixture(); + let dir = tempfile::tempdir().expect("tempdir"); + let a = dir.path().join("a.csv"); + let b = dir.path().join("b.csv"); + + run(&args(model.path(), &a, Projection::Random)).expect("run a"); + run(&args(model.path(), &b, Projection::Random)).expect("run b"); + + embed_viz_lint::run(&a, Some(VOCAB), Some(&b), false) + .expect("two runs at seed 42 must be byte-identical"); +} + +#[test] +fn a_different_seed_moves_the_projection() { + let model = model_fixture(); + let dir = tempfile::tempdir().expect("tempdir"); + let a = dir.path().join("a.csv"); + let b = dir.path().join("b.csv"); + + run(&args(model.path(), &a, Projection::Random)).expect("run a"); + let mut other = args(model.path(), &b, Projection::Random); + other.seed = 43; + run(&other).expect("run b"); + + let (ta, tb) = ( + std::fs::read_to_string(&a).expect("a"), + std::fs::read_to_string(&b).expect("b"), + ); + assert_ne!( + ta, tb, + "if seed 42 and 43 give identical output the seed is not wired to the draw" + ); +} + +#[test] +fn round_trip_cannot_pass_vacuously_when_the_csv_is_corrupted() { + let model = model_fixture(); + let dir = tempfile::tempdir().expect("tempdir"); + let csv = dir.path().join("good.csv"); + run(&args(model.path(), &csv, Projection::Random)).expect("producer"); + let good = std::fs::read_to_string(&csv).expect("read"); + + // Replace the x coordinate of the first data row with `nan`. + let poisoned = { + let mut lines: Vec = good.lines().map(str::to_string).collect(); + let mut fields: Vec = lines[1].split(',').map(str::to_string).collect(); + fields[2] = "nan".to_string(); + lines[1] = fields.join(","); + lines.join("\n") + }; + + let cases: [(&str, String); 4] = [ + ("a non-finite coordinate", poisoned), + ( + "a dropped row", + good.lines() + .take(good.lines().count() - 1) + .collect::>() + .join("\n"), + ), + ( + "a renamed header column", + good.replacen("token_id", "id", 1), + ), + ("a negative token id", good.replacen("\n0,", "\n-1,", 1)), + ]; + + for (label, body) in cases { + let bad = dir.path().join("bad.csv"); + std::fs::write(&bad, &body).expect("write"); + let err = embed_viz_lint::run(&bad, Some(VOCAB), None, false) + .expect_err(&format!("lint must reject: {label}")); + assert!( + matches!(err, CliError::ValidationFailed(_)), + "{label}: expected a validation refusal, got {err:?}" + ); + } +} + +/// The determinism gate must be able to fail, or the byte-identity test above +/// would prove nothing. +#[test] +fn the_determinism_gate_rejects_two_different_csvs() { + let model = model_fixture(); + let dir = tempfile::tempdir().expect("tempdir"); + let a = dir.path().join("a.csv"); + let b = dir.path().join("b.csv"); + run(&args(model.path(), &a, Projection::Random)).expect("run a"); + let mut other = args(model.path(), &b, Projection::Random); + other.seed = 43; + run(&other).expect("run b"); + + let err = embed_viz_lint::run(&a, Some(VOCAB), Some(&b), false) + .expect_err("two different projections must not pass a determinism gate"); + assert!(matches!(err, CliError::ValidationFailed(_)), "{err:?}"); +} + +// ── honest refusals ────────────────────────────────────────────────────── + +#[test] +fn umap_is_refused_rather_than_silently_substituted() { + let model = model_fixture(); + let dir = tempfile::tempdir().expect("tempdir"); + let out = dir.path().join("umap.csv"); + let err = run(&args(model.path(), &out, Projection::Umap)) + .expect_err("an algorithm this binary does not implement must not be labelled as run"); + assert!(matches!(err, CliError::NotImplemented(_)), "{err:?}"); + assert!( + !out.exists(), + "a refused projection must not leave a CSV behind" + ); +} + +#[test] +fn a_missing_model_is_refused() { + let dir = tempfile::tempdir().expect("tempdir"); + let out = dir.path().join("x.csv"); + let err = run(&args( + Path::new("/no/such/model.apr"), + &out, + Projection::Pca, + )) + .expect_err("a missing model must not produce coordinates"); + assert!(matches!(err, CliError::FileNotFound(_)), "{err:?}"); +} + +#[test] +fn an_unknown_tensor_name_is_refused_and_names_what_was_asked_for() { + let model = model_fixture(); + let dir = tempfile::tempdir().expect("tempdir"); + let out = dir.path().join("x.csv"); + let mut a = args(model.path(), &out, Projection::Random); + a.tensor = Some("does.not.exist".to_string()); + let err = run(&a).expect_err("an absent tensor cannot be projected"); + assert!(err.to_string().contains("does.not.exist"), "got: {err}"); +} + +#[test] +fn a_one_dimensional_tensor_is_refused_as_an_embedding_table() { + use aprender::format::v2::{AprV2Metadata, AprV2Writer}; + let file = tempfile::NamedTempFile::with_suffix(".apr").expect("tempfile"); + let mut writer = AprV2Writer::new(AprV2Metadata::new("1d")); + writer.add_f32_tensor("model.norm.weight", vec![8], &[1.0f32; 8]); + std::fs::write(file.path(), writer.write().expect("write")).expect("write file"); + + let dir = tempfile::tempdir().expect("tempdir"); + let out = dir.path().join("x.csv"); + let mut a = args(file.path(), &out, Projection::Random); + a.tensor = Some("model.norm.weight".to_string()); + let err = run(&a).expect_err("a 1-D tensor is not an embedding table"); + assert!( + err.to_string().contains("2-D [vocab, hidden]"), + "got: {err}" + ); +} + +#[test] +fn a_zero_row_limit_is_refused() { + let model = model_fixture(); + let dir = tempfile::tempdir().expect("tempdir"); + let out = dir.path().join("x.csv"); + let mut a = args(model.path(), &out, Projection::Random); + a.limit = Some(0); + let err = run(&a).expect_err("0 rows is not a projection"); + assert!(err.to_string().contains("0 rows"), "got: {err}"); +} + +#[test] +fn refuses_to_clobber_an_existing_csv_without_force() { + let model = model_fixture(); + let dir = tempfile::tempdir().expect("tempdir"); + let out = dir.path().join("existing.csv"); + std::fs::write(&out, "precious").expect("write"); + let err = run(&args(model.path(), &out, Projection::Random)) + .expect_err("an existing CSV must not be overwritten silently"); + assert!(err.to_string().contains("--force"), "got: {err}"); +} + +#[test] +fn a_short_tokens_file_is_refused_rather_than_padded() { + let model = model_fixture(); + let dir = tempfile::tempdir().expect("tempdir"); + let tokens = dir.path().join("tokens.txt"); + std::fs::write(&tokens, "a\nb\nc\n").expect("write"); + let out = dir.path().join("x.csv"); + let mut a = args(model.path(), &out, Projection::Random); + a.tokens = Some(tokens); + let err = run(&a).expect_err("3 tokens cannot label 24 rows"); + assert!(err.to_string().contains("3 lines"), "got: {err}"); +} + +// ── limits, tokens, escaping ───────────────────────────────────────────── + +#[test] +fn limit_selects_the_first_n_rows_and_the_row_count_gate_sees_it() { + let model = model_fixture(); + let dir = tempfile::tempdir().expect("tempdir"); + let out = dir.path().join("x.csv"); + let mut a = args(model.path(), &out, Projection::Random); + a.limit = Some(5); + run(&a).expect("producer"); + + embed_viz_lint::run(&out, Some(5), None, false).expect("5 rows were requested and written"); + let err = embed_viz_lint::run(&out, Some(VOCAB), None, false) + .expect_err("the row-count gate must notice the limit"); + assert!(matches!(err, CliError::ValidationFailed(_)), "{err:?}"); +} + +#[test] +fn tokens_from_a_file_land_in_the_csv() { + let model = model_fixture(); + let dir = tempfile::tempdir().expect("tempdir"); + let tokens = dir.path().join("tokens.txt"); + let body: String = (0..VOCAB).map(|i| format!("tok{i}\n")).collect(); + std::fs::write(&tokens, body).expect("write"); + + let out = dir.path().join("x.csv"); + let mut a = args(model.path(), &out, Projection::Random); + a.tokens = Some(tokens); + run(&a).expect("producer"); + + let csv = std::fs::read_to_string(&out).expect("read"); + assert!(csv.contains(",tok0,"), "got: {csv}"); + embed_viz_lint::run(&out, Some(VOCAB), None, false).expect("lint"); +} + +/// A token containing a comma would shift the column count the F-18 classifier +/// counts, so the producer escapes it. This asserts the CLASSIFIER accepts the +/// escaped row — the property that matters — not merely that a byte changed. +#[test] +fn a_token_containing_a_comma_does_not_shift_the_column_count() { + let tokens = ResolvedTokens { + strings: vec![ + "a,b".to_string(), + "quote\"here".to_string(), + "back\\slash".to_string(), + "line\nbreak".to_string(), + ], + source: "test".to_string(), + }; + let csv = render_csv(&[(0.0, 1.0), (2.0, 3.0), (4.0, 5.0), (6.0, 7.0)], &tokens); + assert_eq!( + embed_viz_classifier::classify_schema(&csv), + embed_viz_classifier::EmbedSchemaOutcome::Ok { rows: 4 }, + "escaped token text must keep 4 columns per row:\n{csv}" + ); + assert_eq!(csv.lines().count(), 5, "header + 4 rows:\n{csv}"); +} + +#[test] +fn escape_token_is_reversible_in_the_characters_it_touches() { + assert_eq!(escape_token("a,b"), "a\\x2cb"); + assert_eq!(escape_token("a\\b"), "a\\\\b"); + assert_eq!(escape_token("a\"b"), "a\\x22b"); + assert_eq!(escape_token("a\nb"), "a\\nb"); + assert_eq!(escape_token("plain"), "plain"); +} + +#[test] +fn unresolved_tokens_are_marked_not_invented() { + let model = model_fixture(); + let dir = tempfile::tempdir().expect("tempdir"); + let out = dir.path().join("x.csv"); + run(&args(model.path(), &out, Projection::Random)).expect("producer"); + let csv = std::fs::read_to_string(&out).expect("read"); + assert!( + csv.contains(""), + "an APR fixture carries no vocabulary; token_str must say so: {csv}" + ); +} + +// ── projection maths ───────────────────────────────────────────────────── + +#[test] +fn the_projection_is_not_constant() { + let model = model_fixture(); + let dir = tempfile::tempdir().expect("tempdir"); + for projection in [Projection::Pca, Projection::Random] { + let out = dir.path().join("p.csv"); + let mut a = args(model.path(), &out, projection); + a.force = true; + run(&a).expect("producer"); + let csv = std::fs::read_to_string(&out).expect("read"); + let xs: Vec<&str> = csv + .lines() + .skip(1) + .filter_map(|l| l.split(',').nth(2)) + .collect(); + assert!( + xs.windows(2).any(|w| w[0] != w[1]), + "{} collapsed every row onto one x: {csv}", + projection_label(projection) + ); + } +} + +// ── GGUF: the first-class advertised path (aprender#2377-3 blockers 1-3) ─ +// +// `token_embd.weight` is the FIRST entry in `EMBEDDING_TENSOR_CANDIDATES`, yet +// the only fixture in this file was an APR v2 table written as +// `[VOCAB, HIDDEN]` — the single layout where `shape[0]` really is the vocab +// axis. That made the GGUF axis inversion invisible: the producer emitted 1024 +// rows for `Qwen3.5-0.8B-Q4_K_M.gguf`'s 248320-token vocabulary and +// `embed-viz-lint --expected-vocab-size 248320` exited 5 on its own producer's +// output. +// +// THE TRAP these tests are built to avoid: `token_str` is resolved from the +// vocabulary list by row index, so it reads correctly no matter which axis +// produced the coordinates. Asserting on `token_str` CANNOT detect this bug. +// What follows asserts the ROW COUNT against the real vocab size and the +// COORDINATES against a `hidden`-length slice. + +/// The value at (token `t`, dim `d`) of the GGUF fixture's embedding table. +/// +/// Rank-1 by construction — row `t` is `(t+1)` times a fixed profile — because +/// that makes the seeded JL projection EXACTLY linear in `t+1`, which is the +/// property `gguf_coordinates_come_from_the_hidden_axis` checks without having +/// to re-derive the RNG stream. +fn g_value(t: usize, d: usize) -> f32 { + (t as f32 + 1.0) * (0.25f32.mul_add(d as f32, 1.0)) +} + +/// A minimal GGUF carrying `token_embd.weight` in real GGML `ne` order. +/// +/// `ne` is `[hidden, vocab]` — the CONTIGUOUS dimension first — while the +/// payload is `[vocab][hidden]` rows, exactly as llama.cpp writes it and as +/// measured against a real model in both formats. +fn gguf_fixture(vocab: usize, hidden: usize, declared_tokens: usize) -> tempfile::NamedTempFile { + use aprender::format::gguf::{export_tensors_to_gguf, GgmlType, GgufTensor, GgufValue}; + + let mut bytes = Vec::new(); + for t in 0..vocab { + for d in 0..hidden { + bytes.extend_from_slice(&g_value(t, d).to_le_bytes()); + } + } + let tensor = GgufTensor { + name: "token_embd.weight".to_string(), + // GGML `ne` order: ne[0] is contiguous, so [hidden, vocab]. + shape: vec![hidden as u64, vocab as u64], + dtype: GgmlType::F32, + data: bytes, + }; + let tokens: Vec = (0..declared_tokens).map(|i| format!("tok{i}")).collect(); + let metadata = vec![ + ( + "general.architecture".to_string(), + GgufValue::String("llama".to_string()), + ), + ( + "tokenizer.ggml.model".to_string(), + GgufValue::String("gpt2".to_string()), + ), + ( + "tokenizer.ggml.tokens".to_string(), + GgufValue::ArrayString(tokens), + ), + ]; + + let file = tempfile::NamedTempFile::with_suffix(".gguf").expect("tempfile"); + let mut buf = Vec::new(); + export_tensors_to_gguf(&mut buf, &[tensor], &metadata).expect("write GGUF"); + std::fs::write(file.path(), buf).expect("write file"); + file +} + +/// The per-format axis rule, as a case table. GGUF is `[hidden, vocab]`; +/// APR and SafeTensors are `[vocab, hidden]`. +#[test] +fn the_vocab_axis_is_chosen_per_format_not_assumed_to_be_axis_zero() { + use aprender::format::rosetta::FormatType; + + // A real measurement: Qwen3.5-0.8B-Q4_K_M.gguf reports [1024, 248320] for a + // 248320-token vocabulary at hidden size 1024. + assert_eq!( + embedding_axes(FormatType::Gguf, &[1024, 248_320]), + (248_320, 1024), + "GGUF ne order is [hidden, vocab]" + ); + assert_eq!( + embedding_axes(FormatType::SafeTensors, &[151_936, 896]), + (151_936, 896), + "SafeTensors is row-major [vocab, hidden]" + ); + assert_eq!( + embedding_axes(FormatType::Apr, &[VOCAB, HIDDEN]), + (VOCAB, HIDDEN), + "APR is row-major [vocab, hidden]" + ); +} + +/// `locate_embedding` must hand back `(vocab, hidden)` in APR order whatever the +/// container said. This is the assertion that the projected slice is `hidden` +/// long: `run` slices `data[i * hidden .. (i + 1) * hidden]`. +#[test] +fn gguf_locate_embedding_returns_vocab_and_hidden_in_apr_order() { + let model = gguf_fixture(G_VOCAB, G_HIDDEN, G_VOCAB); + let (name, vocab, hidden) = locate_embedding(model.path(), None).expect("locate"); + assert_eq!(name, "token_embd.weight"); + assert_eq!( + (vocab, hidden), + (G_VOCAB, G_HIDDEN), + "GGUF ne [{G_HIDDEN}, {G_VOCAB}] must be read as vocab={G_VOCAB} hidden={G_HIDDEN}" + ); +} + +/// The round trip the audit found missing: producer -> lint, on GGUF. +#[test] +fn round_trip_gguf_producer_csv_is_accepted_by_embed_viz_lint() { + let model = gguf_fixture(G_VOCAB, G_HIDDEN, G_VOCAB); + let dir = tempfile::tempdir().expect("tempdir"); + + for projection in [Projection::Pca, Projection::Random] { + let csv = dir + .path() + .join(format!("g-{}.csv", projection_label(projection))); + run(&args(model.path(), &csv, projection)).expect("producer must project a GGUF table"); + + let body = std::fs::read_to_string(&csv).expect("read"); + assert_eq!( + body.lines().count() - 1, + G_VOCAB, + "one row per TOKEN, not one per hidden dim:\n{body}" + ); + + embed_viz_lint::run(&csv, Some(G_VOCAB), None, false).unwrap_or_else(|e| { + panic!( + "embed-viz-lint must accept the producer's own GGUF {} CSV: {e}", + projection_label(projection) + ) + }); + // Pinned in both directions: the row count is G_VOCAB and nothing else. + let err = embed_viz_lint::run(&csv, Some(G_HIDDEN), None, false) + .expect_err("the row-count gate must reject the hidden dim as a vocab size"); + assert!(matches!(err, CliError::ValidationFailed(_)), "{err:?}"); + } +} + +/// The coordinates must be the projection of `hidden`-long TOKEN rows. +/// +/// The fixture is rank-1 — row `t` is `(t+1)` times a fixed profile — so the +/// seeded JL projection is exactly linear in `t+1`: `x_t / x_0 == t+1`. Slicing +/// the other axis mixes several tokens into each row and destroys that ratio. +/// This is the assertion `token_str` cannot make. +#[test] +fn gguf_coordinates_come_from_the_hidden_axis() { + let model = gguf_fixture(G_VOCAB, G_HIDDEN, G_VOCAB); + let dir = tempfile::tempdir().expect("tempdir"); + let csv = dir.path().join("g.csv"); + run(&args(model.path(), &csv, Projection::Random)).expect("producer"); + + let body = std::fs::read_to_string(&csv).expect("read"); + let xs: Vec = body + .lines() + .skip(1) + .map(|l| { + l.split(',') + .nth(2) + .and_then(|f| f.parse::().ok()) + .unwrap_or_else(|| panic!("unparseable x in row `{l}`")) + }) + .collect(); + assert_eq!(xs.len(), G_VOCAB); + assert!( + xs[0].abs() > 1e-6, + "the fixture must not project token 0 onto the origin: {xs:?}" + ); + for (t, x) in xs.iter().enumerate() { + let expected = xs[0] * (t as f64 + 1.0); + assert!( + (x - expected).abs() <= 1e-4 * expected.abs().max(1.0), + "row {t}: x={x} but a hidden-axis projection of a rank-1 table must give \ + {expected} (x_0 * (t+1)); the coordinates were projected from the wrong axis" + ); + } +} + +/// A GGUF whose vocabulary is LARGER than its embedding table has been read the +/// wrong way round — every token must have a row. Padding goes the other way +/// (more rows than tokens), so this one-sided check cannot fire spuriously. +#[test] +fn a_vocabulary_larger_than_the_embedding_table_is_refused() { + let model = gguf_fixture(G_VOCAB, G_HIDDEN, G_VOCAB + 6); + let dir = tempfile::tempdir().expect("tempdir"); + let out = dir.path().join("x.csv"); + let err = run(&args(model.path(), &out, Projection::Random)) + .expect_err("30 tokens cannot be embedded by a 24-row table"); + assert!(matches!(err, CliError::ValidationFailed(_)), "{err:?}"); + assert!( + err.to_string().contains("wrong way round"), + "the refusal must name the axis as the suspect: {err}" + ); +} + +/// A table padded ABOVE the token list is normal and must still be projected. +#[test] +fn an_embedding_table_padded_above_the_token_list_is_accepted() { + let model = gguf_fixture(G_VOCAB, G_HIDDEN, G_VOCAB - 4); + let dir = tempfile::tempdir().expect("tempdir"); + let out = dir.path().join("x.csv"); + run(&args(model.path(), &out, Projection::Random)) + .expect("more rows than tokens is padding, not an inverted axis"); + embed_viz_lint::run(&out, Some(G_VOCAB), None, false).expect("lint"); +} + +#[test] +fn pca_needs_at_least_two_rows() { + let model = model_fixture(); + let dir = tempfile::tempdir().expect("tempdir"); + let out = dir.path().join("x.csv"); + let mut a = args(model.path(), &out, Projection::Pca); + a.limit = Some(1); + let err = run(&a).expect_err("PCA on one sample has no variance to decompose"); + assert!(err.to_string().contains("at least 2 rows"), "got: {err}"); +} diff --git a/crates/apr-cli/src/commands/kernel_parity.rs b/crates/apr-cli/src/commands/kernel_parity.rs new file mode 100644 index 0000000000..44b6513965 --- /dev/null +++ b/crates/apr-cli/src/commands/kernel_parity.rs @@ -0,0 +1,378 @@ +//! `apr kernel parity` — the PRODUCER whose output `apr attn-parity-lint` reads +//! (aprender#2377 finding 3). +//! +//! CRUX-L-02 shipped a consumer with no producer: `attn-parity-lint`'s help +//! documented `apr kernel parity --impl flash2 --ref naive --json` and the +//! shipped binary had no `kernel` command at all, so the parity, provenance and +//! head-dim gates had never run on real data and could not. +//! +//! ## What this measures, and what it refuses to claim +//! +//! `--impl tiled` runs the **in-tree** tiled online-softmax attention kernel +//! (`realizar::brick::FlashAttentionBrick`, the FlashAttention-2 tiling scheme +//! of Dao 2023 / Milakov & Gimelshein 2018) against a naive reference written +//! here, which materialises the score row and does a plain max-subtracted +//! softmax. Both consume the same seeded Q/K/V, so `max_abs_diff` and +//! `cosine_sim` are a real measurement of two independent implementations — it +//! can fail, and a regression in the shipped brick would make it fail. +//! +//! `--impl flash2` means the pinned `hf-kernels-community:flash-attn2@` +//! CUDA kernel. **This binary embeds no such kernel.** Asking for it is +//! REFUSED with a non-zero exit and a message saying so. It is never quietly +//! answered by the tiled path, and `attn_impl: "flash2"` is never emitted with +//! a `kernel_source` we did not load — that fabricated-provenance line is the +//! whole reason CRUX-L-02 pins the `pkg@sha` format. +//! +//! Shape note: the brick is a decode-step kernel — ONE query position attending +//! over a `seq_len`-long KV cache. The observation says so. No claim is made +//! about prefill or about causal masking across a query block. + +use std::path::Path; + +use serde::Serialize; + +use crate::error::{refuse_overwrite, CliError, Result}; + +/// `--impl` / `--ref` are declared at the crate root (`extended_commands.rs`) +/// because `ExtendedCommands` is public and `mod commands` is not. +pub(crate) use crate::{KernelImpl, KernelRef}; + +/// head_dim values the pinned flash-attn2 kernel dispatches +/// (`contracts/crux-L-02-v1.yaml` § `flash_attn2_dispatch`, arXiv:2307.08691). +pub(crate) const FLASH2_SUPPORTED_HEAD_DIMS: [usize; 2] = [64, 128]; + +/// The parity + provenance observation `apr attn-parity-lint` consumes. +/// +/// One body serves both `--parity-file` and `--provenance-file`: the numerics +/// gate reads `max_abs_diff`/`cosine_sim`, the provenance gate reads +/// `attn_impl`/`kernel_source`/`fallback`. +#[derive(Debug, Clone, PartialEq, Serialize)] +pub(crate) struct ParityObservation { + /// Implementation under test, as named on the command line. + pub kernel: String, + /// Reference implementation. + pub reference: String, + /// KV cache length attended over. + pub seq_len: usize, + /// Query head count. + pub num_heads: usize, + /// Key/value head count (GQA groups when smaller than `num_heads`). + pub num_kv_heads: usize, + /// Per-head dimension. + pub head_dim: usize, + /// Seed for the Q/K/V draw. + pub seed: u64, + /// Attention shape measured — decode step, one query position. + pub regime: String, + /// Largest absolute elementwise difference between the two outputs. + pub max_abs_diff: f64, + /// Cosine similarity of the two flattened outputs. + pub cosine_sim: f64, + /// Provenance discriminant read by `classify_provenance`. + pub attn_impl: String, + /// Pinned `pkg@sha` when — and only when — that kernel actually ran. + pub kernel_source: Option, + /// Why `attn_impl` is not `flash2`. Never empty when it is not. + pub fallback: Option, +} + +/// Dimensions requested on the command line. +#[derive(Debug, Clone, Copy)] +pub(crate) struct ParityDims { + pub seq_len: usize, + pub num_heads: usize, + pub num_kv_heads: usize, + pub head_dim: usize, + pub seed: u64, +} + +/// Run the producer. +pub(crate) fn run( + kernel: KernelImpl, + reference: KernelRef, + dims: ParityDims, + json: bool, + output: Option<&Path>, + force: bool, +) -> Result<()> { + if let Some(out) = output { + refuse_overwrite(out, force)?; + } + if let Err(err) = validate_dims(&dims) { + emit_error(&err, json, output)?; + return Err(CliError::ValidationFailed(err)); + } + if kernel == KernelImpl::Flash2 { + return refuse_flash2(&dims, json, output); + } + + let obs = measure_tiled(reference, dims)?; + let rendered = if json { + serde_json::to_string_pretty(&obs).map_err(|e| { + CliError::InvalidInput(format!("apr kernel parity: cannot serialize: {e}")) + })? + } else { + render_text(&obs) + }; + write_out(&rendered, output) +} + +fn write_out(rendered: &str, output: Option<&Path>) -> Result<()> { + match output { + Some(out) => std::fs::write(out, format!("{rendered}\n"))?, + None => println!("{rendered}"), + } + Ok(()) +} + +fn render_text(o: &ParityObservation) -> String { + format!( + "kernel parity {} vs {}\n regime : {}\n dims : seq_len={} heads={} \ + kv_heads={} head_dim={} seed={}\n max_abs_diff: {:e}\n cosine_sim : {}\n \ + attn_impl : {}\n provenance : {}", + o.kernel, + o.reference, + o.regime, + o.seq_len, + o.num_heads, + o.num_kv_heads, + o.head_dim, + o.seed, + o.max_abs_diff, + o.cosine_sim, + o.attn_impl, + o.kernel_source + .clone() + .or_else(|| o.fallback.clone()) + .unwrap_or_default() + ) +} + +/// Refuse `--impl flash2`, emitting the error JSON the head-dim gate reads. +fn refuse_flash2(dims: &ParityDims, json: bool, output: Option<&Path>) -> Result<()> { + if !FLASH2_SUPPORTED_HEAD_DIMS.contains(&dims.head_dim) { + // `{:?}` on the array would render `[64, 128]`, which reads as a closed + // INTERVAL — head_dim 96 would look supported. It is a two-element SET. + let supported = FLASH2_SUPPORTED_HEAD_DIMS + .iter() + .map(usize::to_string) + .collect::>() + .join(", "); + let msg = format!( + "unsupported-head-dim: {} — flash2 dispatches only head_dim ∈ {{{supported}}} \ + (contracts/crux-L-02-v1.yaml § flash_attn2_dispatch, arXiv:2307.08691)", + dims.head_dim + ); + emit_error(&msg, json, output)?; + return Err(CliError::ValidationFailed(msg)); + } + let msg = format!( + "flash2-kernel-unavailable: this binary embeds no \ + hf-kernels-community:flash-attn2 kernel, so no flash2 measurement exists to report. \ + Rerun with `--impl tiled` to measure the in-tree tiled kernel at head_dim {}.", + dims.head_dim + ); + emit_error(&msg, json, output)?; + Err(CliError::NotImplemented(msg)) +} + +/// Write `{"error": ...}` so a refusal is still a capturable observation. +fn emit_error(message: &str, json: bool, output: Option<&Path>) -> Result<()> { + if !json { + return Ok(()); + } + let body = serde_json::json!({ "error": message }); + let rendered = serde_json::to_string_pretty(&body).unwrap_or_default(); + write_out(&rendered, output) +} + +fn validate_dims(d: &ParityDims) -> std::result::Result<(), String> { + if d.head_dim == 0 { + return Err("unsupported-head-dim: 0 — head_dim must be positive".to_string()); + } + if d.seq_len == 0 { + return Err("apr kernel parity: --seq-len must be positive".to_string()); + } + if d.num_heads == 0 || d.num_kv_heads == 0 { + return Err( + "apr kernel parity: --num-heads and --num-kv-heads must be positive".to_string(), + ); + } + if d.num_heads % d.num_kv_heads != 0 { + return Err(format!( + "apr kernel parity: --num-heads {} is not a multiple of --num-kv-heads {} \ + (GQA needs whole groups)", + d.num_heads, d.num_kv_heads + )); + } + Ok(()) +} + +// ── the measurement ────────────────────────────────────────────────────── + +/// Deterministic SplitMix64 draw, so `--seed` really pins the inputs. +/// +/// Shared with `embed_viz`'s random projection: one seeded stream, one place to +/// audit its determinism. +pub(crate) struct SplitMix64(u64); + +impl SplitMix64 { + /// Seed the stream. + pub(crate) fn new(seed: u64) -> Self { + Self(seed) + } + + fn next_u64(&mut self) -> u64 { + self.0 = self.0.wrapping_add(0x9E37_79B9_7F4A_7C15); + let mut z = self.0; + z = (z ^ (z >> 30)).wrapping_mul(0xBF58_476D_1CE4_E5B9); + z = (z ^ (z >> 27)).wrapping_mul(0x94D0_49BB_1331_11EB); + z ^ (z >> 31) + } + /// Uniform in [-1, 1). + pub(crate) fn next_unit(&mut self) -> f32 { + // Top 24 bits scaled into [0, 1), then mapped to [-1, 1). + const SCALE: f32 = 1.0 / 16_777_216.0; + let unit = (self.next_u64() >> 40) as f32 * SCALE; + unit.mul_add(2.0, -1.0) + } +} + +fn draw(rng: &mut SplitMix64, n: usize) -> Vec { + (0..n).map(|_| rng.next_unit()).collect() +} + +/// Naive reference: materialise the score row, max-subtracted softmax, weighted V. +pub(crate) fn naive_attention( + query: &[f32], + keys: &[f32], + values: &[f32], + dims: &ParityDims, +) -> Vec { + let scale = 1.0 / (dims.head_dim as f32).sqrt(); + let group = dims.num_heads / dims.num_kv_heads; + let mut out = vec![0.0f32; dims.num_heads * dims.head_dim]; + for h in 0..dims.num_heads { + let kv = h / group; + let q = &query[h * dims.head_dim..(h + 1) * dims.head_dim]; + let mut scores = Vec::with_capacity(dims.seq_len); + for s in 0..dims.seq_len { + let base = (s * dims.num_kv_heads + kv) * dims.head_dim; + let dot: f32 = (0..dims.head_dim).map(|d| q[d] * keys[base + d]).sum(); + scores.push(dot * scale); + } + let m = scores.iter().copied().fold(f32::NEG_INFINITY, f32::max); + let mut denom = 0.0f32; + for s in &mut scores { + *s = (*s - m).exp(); + denom += *s; + } + for (s, p) in scores.iter().enumerate() { + let base = (s * dims.num_kv_heads + kv) * dims.head_dim; + for d in 0..dims.head_dim { + out[h * dims.head_dim + d] += p * values[base + d]; + } + } + for d in 0..dims.head_dim { + out[h * dims.head_dim + d] /= denom; + } + } + out +} + +/// Largest absolute elementwise difference. +pub(crate) fn max_abs_diff(a: &[f32], b: &[f32]) -> f64 { + a.iter() + .zip(b.iter()) + .map(|(x, y)| f64::from((x - y).abs())) + .fold(0.0f64, f64::max) +} + +/// Cosine similarity of two flattened outputs; `None` when either has zero norm. +pub(crate) fn cosine_sim(a: &[f32], b: &[f32]) -> Option { + let dot: f64 = a + .iter() + .zip(b.iter()) + .map(|(x, y)| f64::from(*x) * f64::from(*y)) + .sum(); + let na: f64 = a + .iter() + .map(|x| f64::from(*x) * f64::from(*x)) + .sum::() + .sqrt(); + let nb: f64 = b + .iter() + .map(|x| f64::from(*x) * f64::from(*x)) + .sum::() + .sqrt(); + if na == 0.0 || nb == 0.0 { + return None; + } + Some((dot / (na * nb)).clamp(-1.0, 1.0)) +} + +/// Draw seeded Q/K/V for `dims`. +pub(crate) fn draw_qkv(dims: &ParityDims) -> (Vec, Vec, Vec) { + let mut rng = SplitMix64(dims.seed ^ 0x5DEE_CE66_D1F5_1A3B); + let q = draw(&mut rng, dims.num_heads * dims.head_dim); + let kv_len = dims.seq_len * dims.num_kv_heads * dims.head_dim; + let k = draw(&mut rng, kv_len); + let v = draw(&mut rng, kv_len); + (q, k, v) +} + +const TILED_FALLBACK_REASON: &str = + "in-tree tiled online-softmax kernel (realizar::brick::FlashAttentionBrick); \ + this binary embeds no hf-kernels-community:flash-attn2 kernel, so no flash2 \ + provenance sha exists to pin"; + +#[cfg(feature = "inference")] +fn measure_tiled(reference: KernelRef, dims: ParityDims) -> Result { + use realizar::brick::FlashAttentionBrick; + + let (q, k, v) = draw_qkv(&dims); + let brick = FlashAttentionBrick::new(dims.num_heads, dims.num_kv_heads, dims.head_dim); + let tiled = brick.forward(&q, &k, &v, dims.seq_len).map_err(|e| { + CliError::ValidationFailed(format!( + "apr kernel parity: tiled kernel refused input: {e}" + )) + })?; + let naive = naive_attention(&q, &k, &v, &dims); + let cos = cosine_sim(&tiled, &naive).ok_or_else(|| { + CliError::ValidationFailed( + "apr kernel parity: an output vector has zero norm, so cosine similarity is undefined" + .to_string(), + ) + })?; + + Ok(ParityObservation { + kernel: "tiled".to_string(), + reference: match reference { + KernelRef::Naive => "naive".to_string(), + }, + seq_len: dims.seq_len, + num_heads: dims.num_heads, + num_kv_heads: dims.num_kv_heads, + head_dim: dims.head_dim, + seed: dims.seed, + regime: "decode-step: 1 query position over a seq_len KV cache".to_string(), + max_abs_diff: max_abs_diff(&tiled, &naive), + cosine_sim: cos, + attn_impl: "fallback".to_string(), + kernel_source: None, + fallback: Some(TILED_FALLBACK_REASON.to_string()), + }) +} + +#[cfg(not(feature = "inference"))] +fn measure_tiled(_reference: KernelRef, _dims: ParityDims) -> Result { + Err(CliError::FeatureDisabled( + "apr kernel parity --impl tiled needs the `inference` feature (it runs \ + realizar::brick::FlashAttentionBrick); rebuild with --features inference" + .to_string(), + )) +} + +#[cfg(test)] +#[path = "kernel_parity_tests.rs"] +mod tests; diff --git a/crates/apr-cli/src/commands/kernel_parity_tests.rs b/crates/apr-cli/src/commands/kernel_parity_tests.rs new file mode 100644 index 0000000000..8a84897ff3 --- /dev/null +++ b/crates/apr-cli/src/commands/kernel_parity_tests.rs @@ -0,0 +1,497 @@ +//! Tests for the `apr kernel parity` producer (aprender#2377 finding 3). +//! +//! The load-bearing test is `round_trip_*`: the producer's own output is fed to +//! `apr attn-parity-lint` and the lint must ACCEPT it — at the SHIPPED default +//! tolerances, not at tolerances chosen to make it pass. The negative half +//! corrupts the body and requires the lint to reject it, so the round trip +//! cannot pass vacuously. + +use super::*; +use crate::commands::attn_parity_lint; + +fn dims() -> ParityDims { + ParityDims { + seq_len: 32, + num_heads: 4, + num_kv_heads: 2, + head_dim: 64, + seed: 7, + } +} + +// ── ROUND TRIP ─────────────────────────────────────────────────────────── + +#[cfg(feature = "inference")] +#[test] +fn round_trip_producer_output_is_accepted_by_attn_parity_lint() { + let dir = tempfile::tempdir().expect("tempdir"); + let obs = dir.path().join("parity.json"); + + run( + KernelImpl::Tiled, + KernelRef::Naive, + dims(), + true, + Some(&obs), + false, + ) + .expect("the tiled kernel must produce a measurement"); + + // One body, both gates — parity numerics AND provenance — at the shipped + // defaults (5e-3 / 0.9999). + attn_parity_lint::run( + Some(&obs), + Some(&obs), + None, + attn_parity_lint::ATTN_PARITY_DEFAULT_MAX_ABS_DIFF, + attn_parity_lint::ATTN_PARITY_DEFAULT_MIN_COSINE_SIM, + false, + ) + .expect("attn-parity-lint must accept the producer's own observation"); +} + +#[cfg(feature = "inference")] +#[test] +fn round_trip_cannot_pass_vacuously_when_the_body_is_corrupted() { + let dir = tempfile::tempdir().expect("tempdir"); + let obs = dir.path().join("parity.json"); + run( + KernelImpl::Tiled, + KernelRef::Naive, + dims(), + true, + Some(&obs), + false, + ) + .expect("producer"); + let good: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&obs).expect("read")).expect("parse"); + + for (label, mutate) in [ + ( + "max_abs_diff past the FA2 bound", + Box::new(|v: &mut serde_json::Value| v["max_abs_diff"] = serde_json::json!(0.5)) + as Box, + ), + ( + "cosine below the floor", + Box::new(|v: &mut serde_json::Value| v["cosine_sim"] = serde_json::json!(0.9)), + ), + ( + "provenance claiming flash2 with no pinned sha", + Box::new(|v: &mut serde_json::Value| { + v["attn_impl"] = serde_json::json!("flash2"); + v["kernel_source"] = serde_json::Value::Null; + }), + ), + ( + "fallback reason blanked out", + Box::new(|v: &mut serde_json::Value| v["fallback"] = serde_json::json!("")), + ), + ] { + let mut bad = good.clone(); + mutate(&mut bad); + let path = dir.path().join("bad.json"); + std::fs::write(&path, serde_json::to_string(&bad).expect("ser")).expect("write"); + let err = attn_parity_lint::run( + Some(&path), + Some(&path), + None, + attn_parity_lint::ATTN_PARITY_DEFAULT_MAX_ABS_DIFF, + attn_parity_lint::ATTN_PARITY_DEFAULT_MIN_COSINE_SIM, + false, + ) + .expect_err(&format!("lint must reject: {label}")); + assert!( + matches!(err, CliError::ValidationFailed(_)), + "{label}: expected a validation refusal, got {err:?}" + ); + } +} + +/// The head-dim refusal is itself an observation: the error JSON it writes must +/// be accepted by `attn-parity-lint --head-dim-error-file`. +#[test] +fn round_trip_head_dim_refusal_is_accepted_by_the_head_dim_gate() { + let dir = tempfile::tempdir().expect("tempdir"); + let err_json = dir.path().join("head-dim.json"); + let mut d = dims(); + d.head_dim = 96; + + let err = run( + KernelImpl::Flash2, + KernelRef::Naive, + d, + true, + Some(&err_json), + false, + ) + .expect_err("head_dim 96 must be refused, not slow-pathed"); + assert!(matches!(err, CliError::ValidationFailed(_)), "{err:?}"); + assert!(err.exit_code_value() != 0, "a refusal must not exit 0"); + + attn_parity_lint::run( + None, + None, + Some(&err_json), + attn_parity_lint::ATTN_PARITY_DEFAULT_MAX_ABS_DIFF, + attn_parity_lint::ATTN_PARITY_DEFAULT_MIN_COSINE_SIM, + false, + ) + .expect("the head-dim gate must accept the producer's own error body"); +} + +/// The supported set must render as a SET, not `[64, 128]` — which reads as a +/// closed interval and would make head_dim 96 look supported by the very +/// message refusing it. +#[test] +fn the_head_dim_refusal_names_a_set_not_an_interval() { + let mut d = dims(); + d.head_dim = 96; + let err = run(KernelImpl::Flash2, KernelRef::Naive, d, false, None, false) + .expect_err("96 must be refused"); + let msg = err.to_string(); + assert!(msg.contains("{64, 128}"), "got: {msg}"); + assert!( + !msg.contains("[64, 128]"), + "interval notation would imply 96 is in range: {msg}" + ); +} + +#[test] +fn head_dim_gate_rejects_an_error_body_that_is_not_about_head_dim() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("other.json"); + std::fs::write(&path, r#"{"error":"out of memory"}"#).expect("write"); + let err = attn_parity_lint::run( + None, + None, + Some(&path), + attn_parity_lint::ATTN_PARITY_DEFAULT_MAX_ABS_DIFF, + attn_parity_lint::ATTN_PARITY_DEFAULT_MIN_COSINE_SIM, + false, + ) + .expect_err("an unrelated error must not discharge the head-dim gate"); + assert!(matches!(err, CliError::ValidationFailed(_)), "{err:?}"); +} + +// ── honest refusals ────────────────────────────────────────────────────── + +#[test] +fn flash2_is_refused_rather_than_answered_by_the_tiled_kernel() { + let dir = tempfile::tempdir().expect("tempdir"); + let out = dir.path().join("flash2.json"); + let err = run( + KernelImpl::Flash2, + KernelRef::Naive, + dims(), + true, + Some(&out), + false, + ) + .expect_err("a kernel this binary does not embed must not report a measurement"); + assert!( + matches!(err, CliError::NotImplemented(_)), + "expected NotImplemented, got {err:?}" + ); + + let body: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&out).expect("read")).expect("parse"); + assert!( + body.get("max_abs_diff").is_none(), + "a refusal must not carry a parity number: {body}" + ); + assert!( + body["error"] + .as_str() + .is_some_and(|s| s.contains("flash2-kernel-unavailable")), + "the refusal must name what is missing: {body}" + ); +} + +#[test] +fn flash2_at_a_supported_head_dim_still_refuses_without_the_kernel() { + for head_dim in FLASH2_SUPPORTED_HEAD_DIMS { + let mut d = dims(); + d.head_dim = head_dim; + let err = run(KernelImpl::Flash2, KernelRef::Naive, d, false, None, false) + .expect_err("head_dim being supported does not conjure the kernel"); + assert!(matches!(err, CliError::NotImplemented(_)), "{err:?}"); + } +} + +#[test] +fn zero_head_dim_is_refused_with_a_head_dim_message() { + let mut d = dims(); + d.head_dim = 0; + let err = run(KernelImpl::Tiled, KernelRef::Naive, d, false, None, false) + .expect_err("head_dim 0 is not a kernel configuration"); + assert!(err.to_string().contains("head-dim"), "got: {err}"); +} + +#[test] +fn gqa_group_mismatch_is_refused() { + let mut d = dims(); + d.num_heads = 5; + d.num_kv_heads = 2; + let err = run(KernelImpl::Tiled, KernelRef::Naive, d, false, None, false) + .expect_err("5 query heads cannot be split into 2 whole KV groups"); + assert!(err.to_string().contains("whole groups"), "got: {err}"); +} + +#[test] +fn refuses_to_clobber_an_existing_output_without_force() { + let dir = tempfile::tempdir().expect("tempdir"); + let out = dir.path().join("existing.json"); + std::fs::write(&out, "precious").expect("write"); + let err = run( + KernelImpl::Tiled, + KernelRef::Naive, + dims(), + true, + Some(&out), + false, + ) + .expect_err("an existing output must not be overwritten silently"); + assert!(err.to_string().contains("--force"), "got: {err}"); +} + +// ── the measurement itself ─────────────────────────────────────────────── + +#[cfg(feature = "inference")] +#[test] +fn tiled_and_naive_agree_far_inside_the_fa2_bound() { + use realizar::brick::FlashAttentionBrick; + let d = dims(); + let (q, k, v) = draw_qkv(&d); + let tiled = FlashAttentionBrick::new(d.num_heads, d.num_kv_heads, d.head_dim) + .forward(&q, &k, &v, d.seq_len) + .expect("tiled forward"); + let naive = naive_attention(&q, &k, &v, &d); + let mad = max_abs_diff(&tiled, &naive); + assert!( + mad < 1e-5, + "two f32 implementations of the same attention must agree to ~1e-6; got {mad:e}" + ); +} + +// ── the EMITTED numbers are the MEASURED ones ──────────────────────────── +// +// Everything above this point can pass while the producer fabricates its +// verdict. Replacing the two measured fields in `measure_tiled` with the +// literals `0.0` / `1.0` left ALL 16 tests green — including both round trips +// and `the_parity_metrics_are_not_vacuous`, which never reaches `run()` at all: +// it re-runs the brick itself on synthetic vectors and so says nothing about +// what the shipped body WROTE. Nothing connected the emitted JSON to a +// measurement. These two tests are that connection. + +/// Recompute the parity metrics for `dims` without going through `run()`. +#[cfg(feature = "inference")] +fn measure_independently(dims: &ParityDims) -> (f64, f64) { + use realizar::brick::FlashAttentionBrick; + let (q, k, v) = draw_qkv(dims); + let tiled = FlashAttentionBrick::new(dims.num_heads, dims.num_kv_heads, dims.head_dim) + .forward(&q, &k, &v, dims.seq_len) + .expect("tiled forward"); + let naive = naive_attention(&q, &k, &v, dims); + ( + max_abs_diff(&tiled, &naive), + cosine_sim(&tiled, &naive).expect("non-zero norms"), + ) +} + +/// Put a value through the same JSON round trip the observation file makes. +/// +/// `serde_json` is 1 ULP lossy on f64: `0x3e68_0000_0000_0000` comes back as +/// `0x3e68_0000_0000_0001`. Comparing a parsed observation against an in-memory +/// f64 with `==` would therefore be testing serde's float fidelity rather than +/// the producer. Sending the measured value through the same pipe cancels that +/// out and lets the comparison stay EXACT — which matters, because the gap +/// between a real cosine (0.999999999999992…) and a fabricated `1.0` is only +/// ~32 ulps, and a hand-picked epsilon could easily swallow it. +fn through_json(v: f64) -> f64 { + let s = serde_json::to_string_pretty(&serde_json::json!({ "v": v })).expect("ser"); + serde_json::from_str::(&s).expect("parse")["v"] + .as_f64() + .expect("f64") +} + +/// Read the two metrics out of a `run()`-produced observation file. +#[cfg(feature = "inference")] +fn emitted_metrics(dims: ParityDims, path: &std::path::Path) -> (f64, f64) { + run( + KernelImpl::Tiled, + KernelRef::Naive, + dims, + true, + Some(path), + true, + ) + .expect("the tiled kernel must produce a measurement"); + let body: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(path).expect("read")).expect("parse"); + ( + body["max_abs_diff"].as_f64().expect("max_abs_diff"), + body["cosine_sim"].as_f64().expect("cosine_sim"), + ) +} + +/// The numbers in the observation must be the numbers the kernels produced — +/// bit for bit, not merely "inside the tolerance the lint happens to allow". +/// +/// A producer that reports a constant passes every tolerance gate ever written, +/// which is exactly why this asserts EQUALITY TO AN INDEPENDENT MEASUREMENT +/// rather than membership of a passing range. +#[cfg(feature = "inference")] +#[test] +fn the_emitted_parity_metrics_are_the_ones_that_were_measured() { + let dir = tempfile::tempdir().expect("tempdir"); + let obs = dir.path().join("parity.json"); + + for (label, d) in [ + ("the shipped default shape", dims()), + ( + "a longer KV cache", + ParityDims { + seq_len: 128, + ..dims() + }, + ), + ( + "head_dim 128, no GQA", + ParityDims { + num_heads: 2, + num_kv_heads: 2, + head_dim: 128, + ..dims() + }, + ), + ] { + let (emitted_mad, emitted_cos) = emitted_metrics(d, &obs); + let (measured_mad, measured_cos) = measure_independently(&d); + assert_eq!( + emitted_mad, + through_json(measured_mad), + "{label}: the emitted max_abs_diff is not the measured one \ + (a fabricated constant would land here)" + ); + assert_eq!( + emitted_cos, + through_json(measured_cos), + "{label}: the emitted cosine_sim is not the measured one" + ); + // The measurement must also be a real one, not a degenerate value a + // constant could coincide with. + assert!( + measured_mad > 0.0 && measured_cos < 1.0, + "{label}: two independent f32 kernels agreed EXACTLY \ + (max_abs_diff={measured_mad}, cosine_sim={measured_cos}); this test can no \ + longer tell a measurement from a hardcoded 0.0/1.0" + ); + } +} + +/// Perturbing the inputs must MOVE the emitted numbers. +/// +/// The seed pins Q/K/V, so changing it changes what the two kernels are asked to +/// agree about, and the f32 rounding they disagree by changes with it. A body +/// that reports a constant cannot move, so this is red for any hardcoded pair — +/// including one that happened to be numerically plausible. +#[cfg(feature = "inference")] +#[test] +fn perturbing_the_inputs_moves_the_emitted_parity_metrics() { + use std::collections::BTreeSet; + + let dir = tempfile::tempdir().expect("tempdir"); + let obs = dir.path().join("parity.json"); + let mut mads: BTreeSet = BTreeSet::new(); + let mut coss: BTreeSet = BTreeSet::new(); + + for seed in [7u64, 8, 9, 1234, 20_260_813] { + let (mad, cos) = emitted_metrics(ParityDims { seed, ..dims() }, &obs); + assert!( + mad.is_finite() && cos.is_finite(), + "seed {seed}: emitted a non-finite metric ({mad}, {cos})" + ); + mads.insert(mad.to_bits()); + coss.insert(cos.to_bits()); + } + + assert!( + mads.len() > 1, + "max_abs_diff was identical across 5 different seeded inputs, so it is not \ + derived from them: {mads:?}" + ); + assert!( + coss.len() > 1, + "cosine_sim was identical across 5 different seeded inputs, so it is not \ + derived from them: {coss:?}" + ); +} + +/// The comparison must be able to FAIL — otherwise it proves nothing about the +/// kernel. Perturbing one output by 0.5 has to blow past the 5e-3 bound. +/// +/// NOTE the limit of this test, which the audit found being over-claimed: it +/// operates on synthetic vectors and never calls `run()`, so it proves only that +/// the two metric FUNCTIONS can see a perturbation. That the shipped body +/// actually reports what they returned is proved by the two tests above. +#[test] +fn the_parity_metrics_are_not_vacuous() { + let a = vec![0.25f32, -0.5, 0.75, 1.0]; + let mut b = a.clone(); + b[2] += 0.5; + assert!( + max_abs_diff(&a, &b) > 5e-3, + "max_abs_diff must see the perturbation" + ); + let cos = cosine_sim(&a, &b).expect("non-zero norms"); + assert!(cos < 0.9999, "cosine must see the perturbation, got {cos}"); + assert_eq!(max_abs_diff(&a, &a), 0.0); +} + +#[test] +fn cosine_of_a_zero_vector_is_undefined_not_one() { + assert_eq!(cosine_sim(&[0.0, 0.0], &[1.0, 1.0]), None); +} + +#[test] +fn the_same_seed_draws_the_same_inputs() { + let (q1, k1, v1) = draw_qkv(&dims()); + let (q2, k2, v2) = draw_qkv(&dims()); + assert_eq!(q1, q2); + assert_eq!(k1, k2); + assert_eq!(v1, v2); + + let mut other = dims(); + other.seed = 8; + let (q3, _, _) = draw_qkv(&other); + assert_ne!(q1, q3, "a different seed must draw different inputs"); +} + +#[test] +fn drawn_values_stay_inside_the_unit_interval() { + let (q, k, v) = draw_qkv(&dims()); + for (name, xs) in [("q", &q), ("k", &k), ("v", &v)] { + assert!( + xs.iter().all(|x| (-1.0..1.0).contains(x)), + "{name} escaped [-1, 1)" + ); + } +} + +/// A single-head, single-position case where the answer is known by hand: +/// attention over one key returns that value exactly. +#[test] +fn naive_attention_over_one_position_returns_that_value() { + let d = ParityDims { + seq_len: 1, + num_heads: 1, + num_kv_heads: 1, + head_dim: 2, + seed: 0, + }; + let out = naive_attention(&[1.0, 0.0], &[0.5, 0.5], &[3.0, -4.0], &d); + assert!((out[0] - 3.0).abs() < 1e-6, "got {out:?}"); + assert!((out[1] - -4.0).abs() < 1e-6, "got {out:?}"); +} diff --git a/crates/apr-cli/src/commands/mod.rs b/crates/apr-cli/src/commands/mod.rs index 721c38355f..be1155c1e6 100644 --- a/crates/apr-cli/src/commands/mod.rs +++ b/crates/apr-cli/src/commands/mod.rs @@ -16,6 +16,7 @@ pub(crate) mod attn_parity_classifier; pub(crate) mod attn_parity_lint; pub(crate) mod attn_viz_classifier; pub(crate) mod attn_viz_lint; +pub(crate) mod audio_inspect; pub(crate) mod audio_inspect_classifier; pub(crate) mod audio_inspect_lint; pub(crate) mod auto_quant; @@ -51,6 +52,7 @@ pub(crate) mod diagnose; pub(crate) mod dry_sampling_classifier; pub(crate) mod dry_sampling_lint; pub(crate) mod embed; +pub(crate) mod embed_viz; pub(crate) mod embed_viz_classifier; pub(crate) mod embed_viz_lint; pub(crate) mod embeddings_classifier; @@ -86,6 +88,7 @@ pub(crate) mod imatrix_lint; pub(crate) mod import; pub(crate) mod inspect; pub(crate) mod kernel_explain; +pub(crate) mod kernel_parity; pub(crate) mod kv_timeline_classifier; pub(crate) mod kv_timeline_lint; pub(crate) mod lint; diff --git a/crates/apr-cli/src/commands/publish_tests.rs b/crates/apr-cli/src/commands/publish_tests.rs index 0cd513bc2c..387677f51b 100644 --- a/crates/apr-cli/src/commands/publish_tests.rs +++ b/crates/apr-cli/src/commands/publish_tests.rs @@ -809,8 +809,16 @@ fn test_safetensors_needing_alias_no_safetensors_skips_alias() { // character 0. The model card now lives in the `readme` string field. // ========================================================================= -fn dry_run_plan_fixture(manifest: Option<&Path>) -> DryRunPlan { - let dir = std::env::temp_dir().join("apr_publish_json_fixture"); +/// Build a dry-run plan over a throwaway artifact of known size. +/// +/// `slug` MUST be unique per test. Every caller used to share one fixed path, +/// `$TMPDIR/apr_publish_json_fixture/model.safetensors`, and the harness runs +/// these tests on parallel threads — so one test's `fs::write` truncated the +/// file to 0 bytes in the window where the other was stat-ing it for +/// `size_bytes`. That surfaced as `size_bytes: 0` against the expected 16, +/// reproducible at ~1/25 full-suite runs under load. +fn dry_run_plan_fixture(slug: &str, manifest: Option<&Path>) -> DryRunPlan { + let dir = std::env::temp_dir().join(format!("apr_publish_json_fixture_{slug}")); let _ = fs::create_dir_all(&dir); let artifact = dir.join("model.safetensors"); let _ = fs::write(&artifact, b"not-a-real-model"); @@ -825,7 +833,7 @@ fn dry_run_plan_fixture(manifest: Option<&Path>) -> DryRunPlan { #[test] fn test_publish_dry_run_json_stdout_parses_as_json() { - let plan = dry_run_plan_fixture(None); + let plan = dry_run_plan_fixture("json_stdout", None); let stdout = plan.stdout(true); let parsed: serde_json::Value = serde_json::from_str(&stdout).unwrap_or_else(|e| { @@ -853,7 +861,7 @@ fn test_publish_dry_run_json_stdout_parses_as_json() { #[test] fn test_publish_dry_run_human_mode_is_still_human() { - let plan = dry_run_plan_fixture(None); + let plan = dry_run_plan_fixture("human_mode", None); let stdout = plan.stdout(false); assert!( stdout.contains("=== DRY RUN: Would publish to paiml/test-model ==="), diff --git a/crates/apr-cli/src/commands_enum.rs b/crates/apr-cli/src/commands_enum.rs index 62e0fd8b69..2655264e71 100644 --- a/crates/apr-cli/src/commands_enum.rs +++ b/crates/apr-cli/src/commands_enum.rs @@ -190,11 +190,18 @@ pub enum Commands { #[arg(long)] quality: bool, }, - /// Simple debugging output ("drama" mode available) + /// Simple debugging output ("drama" mode available), or a debug subcommand + /// + /// `apr debug model.apr` dumps the file. `apr debug embed-viz --model M` + /// projects the model's token-embedding table to 2-D — the producer + /// `apr embed-viz-lint` reads (aprender#2377 finding 3). Debug { - /// Path to .apr model file + /// Path to .apr model file (omit only when using a subcommand) #[arg(value_name = "FILE")] - file: PathBuf, + file: Option, + /// Debug subcommand, e.g. `embed-viz` + #[command(subcommand)] + action: Option, /// Theatrical "drama" mode output #[arg(long)] drama: bool, @@ -795,3 +802,45 @@ pub enum Commands { #[command(subcommand)] Mono(crate::commands::mono::MonoCommands), } + +/// Subcommands for `apr debug` (aprender#2377 finding 3). +/// +/// `embed-viz` is the PRODUCER for `apr embed-viz-lint`: CRUX-F-18 shipped the +/// lint with help pointing at `apr debug embed-viz`, which did not exist, so +/// its schema / row-count / determinism gates had never run on real data. +#[derive(Subcommand, Debug)] +pub enum DebugCommands { + /// Project a model's token-embedding table to 2-D and write the + /// `token_id,token_str,x,y` CSV `apr embed-viz-lint` reads. + /// + /// Reads the real embedding tensor (GGUF / APR / SafeTensors, dequantising + /// as needed). `--projection umap` is REFUSED with a non-zero exit rather + /// than labelling a different algorithm's output "umap". + EmbedViz { + /// Model file holding the embedding table + #[arg(long, value_name = "FILE")] + model: PathBuf, + /// Embedding tensor name (default: auto-detect the known names) + #[arg(long, value_name = "NAME")] + tensor: Option, + /// Projection method: exact `pca`, seeded `random`, or `umap` (refused) + #[arg(long, value_enum, default_value_t = EmbedProjection::Pca)] + projection: EmbedProjection, + /// Seed pinning the random projection, so a rerun is byte-identical + #[arg(long, value_name = "N", default_value_t = 0)] + seed: u64, + /// Project only the first N vocabulary rows (default: all) + #[arg(long, value_name = "N")] + limit: Option, + /// Token text, one per line, for the `token_str` column. Without it apr + /// reads the GGUF vocabulary, or writes `` + #[arg(long, value_name = "FILE")] + tokens: Option, + /// Write the CSV here instead of stdout + #[arg(short, long, value_name = "FILE")] + output: Option, + /// Overwrite an existing --output file (refused without it) + #[arg(short, long)] + force: bool, + }, +} diff --git a/crates/apr-cli/src/dispatch.rs b/crates/apr-cli/src/dispatch.rs index 9d5b09b9ce..9f744451bd 100644 --- a/crates/apr-cli/src/dispatch.rs +++ b/crates/apr-cli/src/dispatch.rs @@ -226,6 +226,54 @@ or drop `--backend`." }) } +/// Dispatch `apr debug`: either the file dump or a debug subcommand. +/// +/// aprender#2377 finding 3: `embed-viz-lint`'s help documented +/// `apr debug embed-viz` and no such subcommand existed. `file` is now optional +/// because a subcommand supplies its own input, and `apr debug` with neither +/// must REFUSE rather than dump nothing and exit 0. +fn dispatch_debug( + cli: &Cli, + file: Option<&Path>, + action: Option<&DebugCommands>, + flags: (bool, bool, bool, usize), +) -> Result<(), CliError> { + if let Some(DebugCommands::EmbedViz { + model, + tensor, + projection, + seed, + limit, + tokens, + output, + force, + }) = action + { + return commands::embed_viz::run(&commands::embed_viz::EmbedVizArgs { + model: model.clone(), + tensor: tensor.clone(), + projection: *projection, + seed: *seed, + limit: *limit, + tokens: tokens.clone(), + output: output.clone(), + force: *force, + }); + } + let file = file.ok_or_else(|| { + CliError::ValidationFailed( + "apr debug: needs a model FILE (`apr debug model.apr`) or a subcommand \ + (`apr debug embed-viz --model model.apr`)" + .to_string(), + ) + })?; + let (drama, hex, strings, limit) = flags; + let (j, verb) = (cli.json, cli.verbose); + crate::pipe::with_stdin_support(file, |p| { + debug::run(p, drama, hex, strings, limit, j, verb) + }) +} + /// Dispatch inspection commands: inspect, debug, validate, lint, explain, canary. #[allow(clippy::many_single_char_names)] fn dispatch_inspection_commands(cli: &Cli) -> Option> { @@ -248,14 +296,17 @@ fn dispatch_inspection_commands(cli: &Cli) -> Option> { // GH-685: forward cli.verbose to debug Commands::Debug { file, + action, drama, hex, strings, limit, - } => { - let (d, h, s, l, j, verb) = (*drama, *hex, *strings, *limit, cli.json, cli.verbose); - crate::pipe::with_stdin_support(file, |p| debug::run(p, d, h, s, l, j, verb)) - } + } => dispatch_debug( + cli, + file.as_deref(), + action.as_ref(), + (*drama, *hex, *strings, *limit), + ), Commands::Validate { file, diff --git a/crates/apr-cli/src/dispatch_analysis.rs b/crates/apr-cli/src/dispatch_analysis.rs index 05c6d86bf2..ff1a71301e 100644 --- a/crates/apr-cli/src/dispatch_analysis.rs +++ b/crates/apr-cli/src/dispatch_analysis.rs @@ -1,3 +1,55 @@ +/// Dispatch `apr dataset …` (aprender#2377 finding 3). +/// +/// Its own function so the producer arms do not push +/// `dispatch_analysis_commands` further past the complexity threshold it was +/// already over. +fn dispatch_dataset_command(command: &DatasetCommands, cli: &Cli) -> Result<(), CliError> { + match command { + DatasetCommands::AudioInspect { + file, + format, + output, + force, + } => commands::audio_inspect::run( + file, + format == "json" || cli.json, + output.as_deref(), + *force, + ), + } +} + +/// Dispatch `apr kernel …` (aprender#2377 finding 3). +fn dispatch_kernel_command(command: &KernelCommands, cli: &Cli) -> Result<(), CliError> { + match command { + KernelCommands::Parity { + kernel, + reference, + seq_len, + num_heads, + num_kv_heads, + head_dim, + seed, + json, + output, + force, + } => commands::kernel_parity::run( + *kernel, + *reference, + commands::kernel_parity::ParityDims { + seq_len: *seq_len, + num_heads: *num_heads, + num_kv_heads: *num_kv_heads, + head_dim: *head_dim, + seed: *seed, + }, + *json || cli.json, + output.as_deref(), + *force, + ), + } +} + /// Dispatch analysis commands (cbtop, probar, compare-hf, hex, tree, flow, oracle). /// /// Returns `None` if the command is not an analysis command, allowing the caller @@ -233,6 +285,10 @@ fn dispatch_analysis_commands(cli: &Cli) -> Option> { cli.json, ), + // aprender#2377 finding 3: the PRODUCERS the *-lint help documents. + ExtendedCommands::Dataset { command } => dispatch_dataset_command(command, cli), + ExtendedCommands::Kernel { command } => dispatch_kernel_command(command, cli), + ExtendedCommands::AudioInspectLint { json_file, expected_sample_rate, diff --git a/crates/apr-cli/src/extended_commands.rs b/crates/apr-cli/src/extended_commands.rs index fc10a556a3..a694b9b86a 100644 --- a/crates/apr-cli/src/extended_commands.rs +++ b/crates/apr-cli/src/extended_commands.rs @@ -1,3 +1,35 @@ +/// Attention implementation under test for `apr kernel parity --impl`. +/// +/// `Flash2` names the pinned `hf-kernels-community:flash-attn2@` CUDA +/// kernel. This binary embeds no such kernel, so selecting it is REFUSED — +/// never quietly answered by `Tiled` under flash2's name, which is the +/// fabricated-provenance failure CRUX-L-02 exists to prevent. +#[derive(Copy, Clone, Debug, PartialEq, Eq, ValueEnum)] +pub enum KernelImpl { + /// In-tree tiled online-softmax kernel (`realizar::brick::FlashAttentionBrick`). + Tiled, + /// Pinned hf-kernels-community flash-attn2 CUDA kernel (not embedded here). + Flash2, +} + +/// Reference implementation for `apr kernel parity --ref`. +#[derive(Copy, Clone, Debug, PartialEq, Eq, ValueEnum)] +pub enum KernelRef { + /// Materialised-score softmax attention, computed in f32 on the CPU. + Naive, +} + +/// 2-D projection for `apr debug embed-viz --projection`. +#[derive(Copy, Clone, Debug, PartialEq, Eq, ValueEnum)] +pub enum EmbedProjection { + /// Exact PCA onto the top 2 principal components (deterministic). + Pca, + /// Seeded Johnson–Lindenstrauss random projection (deterministic in --seed). + Random, + /// Not implemented in this binary — selecting it is refused, not substituted. + Umap, +} + /// Extended CLI commands (analysis, profiling, QA, benchmarks, and advanced tools). /// /// Flattened into `Commands` via `#[command(flatten)]` so all subcommands remain @@ -908,9 +940,20 @@ pub enum ExtendedCommands { value_parser = commands::threshold_arg::parse_tolerance)] loss_tolerance: f64, }, - /// Lint an externally captured audio-inspect JSON body (CRUX-H-13 — no apr producer yet) + /// Dataset inspection tools (CRUX-H-13) + Dataset { + #[command(subcommand)] + command: DatasetCommands, + }, + /// Kernel-level parity measurements (CRUX-L-02) + Kernel { + #[command(subcommand)] + command: KernelCommands, + }, + /// Lint an audio-inspect JSON body, e.g. from + /// `apr dataset audio-inspect clip.wav --format json -o audio.json` (CRUX-H-13) AudioInspectLint { - /// Path to captured JSON body + /// Path to the JSON body written by `apr dataset audio-inspect --format json` #[arg(long, value_name = "FILE")] json_file: PathBuf, /// Optional expected sample_rate (typically the `--resample-to` arg) @@ -920,15 +963,20 @@ pub enum ExtendedCommands { #[arg(long, value_name = "U32")] expected_channels: Option, }, - /// Lint captured flash-attn2 parity + provenance JSON outputs (CRUX-L-02) + /// Lint attention parity + provenance JSON, e.g. from + /// `apr kernel parity --impl tiled --ref naive --json -o parity.json` (CRUX-L-02) AttnParityLint { - /// Path to an externally captured flash2-vs-naive parity JSON body (no apr producer yet) + /// Parity JSON body (`max_abs_diff`, `cosine_sim`), as written by + /// `apr kernel parity --json` #[arg(long, value_name = "FILE")] parity_file: Option, - /// Path to an externally captured flash2 provenance JSON body (no apr producer yet) + /// Provenance JSON body (`attn_impl`, `kernel_source`, `fallback`). + /// `apr kernel parity --json` writes both gates' fields into one body, + /// so the same file may be passed here and to --parity-file #[arg(long, value_name = "FILE")] provenance_file: Option, - /// Path to captured head_dim error JSON + /// head_dim refusal JSON, as written by + /// `apr kernel parity --impl flash2 --head-dim 96 --json` (which exits non-zero) #[arg(long, value_name = "FILE")] head_dim_error_file: Option, /// Max absolute diff tolerance (default 5e-3, FlashAttention-2 bound) @@ -972,15 +1020,16 @@ pub enum ExtendedCommands { #[arg(long, value_name = "N", default_value_t = 100)] min_layers: usize, }, - /// Lint an externally captured embedding-projection CSV (CRUX-F-18 — no apr producer yet) + /// Lint an embedding-projection CSV, e.g. from + /// `apr debug embed-viz --model model.apr --seed 42 -o emb.csv` (CRUX-F-18) EmbedVizLint { - /// Path to captured embed-viz CSV (token_id,token_str,x,y) + /// Path to the `token_id,token_str,x,y` CSV written by `apr debug embed-viz` #[arg(long, value_name = "FILE")] csv_file: PathBuf, /// Expected row count == vocab_size (optional) #[arg(long, value_name = "N")] expected_vocab_size: Option, - /// Second CSV captured under the same seed for determinism check (optional) + /// Second CSV from a rerun at the same --seed, for the determinism gate (optional) #[arg(long, value_name = "FILE")] csv_file_b: Option, }, @@ -1346,6 +1395,85 @@ pub enum ExtendedCommands { }, } +/// Subcommands for `apr dataset` — dataset inspection (aprender#2377 finding 3). +/// +/// `audio-inspect` is the PRODUCER for `apr audio-inspect-lint`: CRUX-H-13 +/// shipped the lint with help pointing at a command the binary did not have, +/// so its gates had never run on real data. +#[derive(Subcommand, Debug)] +pub enum DatasetCommands { + /// Decode an uncompressed RIFF/WAVE file and report its measured shape and + /// amplitude extrema — the observation `apr audio-inspect-lint` reads. + /// + /// Supports PCM u8/i16/i24/i32 and IEEE float32. Compressed containers + /// (FLAC, MP3, Ogg) and codecs it cannot decode are REFUSED with a non-zero + /// exit; no resampling and no channel mixdown are performed, so the reported + /// `sample_rate` and `channels` are always the file's own. + AudioInspect { + /// Path to the .wav file to decode + #[arg(value_name = "FILE")] + file: PathBuf, + /// Output format: `json` for the lint-readable body, `text` for humans + #[arg(long, value_name = "FORMAT", default_value = "text", + value_parser = ["json", "text"])] + format: String, + /// Write the observation here instead of stdout + #[arg(short, long, value_name = "FILE")] + output: Option, + /// Overwrite an existing --output file (refused without it) + #[arg(short, long)] + force: bool, + }, +} + +/// Subcommands for `apr kernel` — kernel-level measurements (aprender#2377 finding 3). +/// +/// `parity` is the PRODUCER for `apr attn-parity-lint`: CRUX-L-02 shipped the +/// lint with help pointing at `apr kernel parity`, which did not exist. +#[derive(Subcommand, Debug)] +pub enum KernelCommands { + /// Measure a tiled attention kernel against a naive reference on seeded + /// Q/K/V, emitting the parity + provenance body `apr attn-parity-lint` reads. + /// + /// `--impl tiled` runs the in-tree `realizar::brick::FlashAttentionBrick` + /// online-softmax kernel. `--impl flash2` means the pinned + /// `hf-kernels-community:flash-attn2@` CUDA kernel, which this binary + /// does not embed: asking for it is REFUSED with a non-zero exit rather + /// than answered by a different kernel under a borrowed name. + Parity { + /// Attention implementation under test + #[arg(long = "impl", value_name = "IMPL", value_enum, default_value_t = KernelImpl::Tiled)] + kernel: KernelImpl, + /// Reference implementation to compare against + #[arg(long = "ref", value_name = "REF", value_enum, default_value_t = KernelRef::Naive)] + reference: KernelRef, + /// KV cache length to attend over + #[arg(long, value_name = "N", default_value_t = 128)] + seq_len: usize, + /// Number of query heads + #[arg(long, value_name = "N", default_value_t = 8)] + num_heads: usize, + /// Number of key/value heads (GQA groups when smaller than --num-heads) + #[arg(long, value_name = "N", default_value_t = 8)] + num_kv_heads: usize, + /// Per-head dimension. flash2 dispatches only 64 or 128 + #[arg(long, value_name = "N", default_value_t = 64)] + head_dim: usize, + /// Seed pinning the Q/K/V draw, so a run is reproducible + #[arg(long, value_name = "N", default_value_t = 0)] + seed: u64, + /// Emit the observation as JSON (required to capture it for the lint) + #[arg(long)] + json: bool, + /// Write the observation here instead of stdout + #[arg(short, long, value_name = "FILE")] + output: Option, + /// Overwrite an existing --output file (refused without it) + #[arg(short, long)] + force: bool, + }, +} + #[cfg(feature = "training")] /// Subcommands for `apr runs` — experiment run management (ALB-050/051) #[derive(Subcommand, Debug)] diff --git a/crates/apr-cli/src/help_producer_truth.rs b/crates/apr-cli/src/help_producer_truth.rs index e07aab4b7d..8d7d3df7c7 100644 --- a/crates/apr-cli/src/help_producer_truth.rs +++ b/crates/apr-cli/src/help_producer_truth.rs @@ -211,14 +211,18 @@ fn resolver_rejects_the_invocations_dogfooding_found() { .spawn(|| { use clap::CommandFactory; let root = Cli::command(); + // aprender#2377 finding 3 IMPLEMENTED three of the eight producers, + // so `apr dataset audio-inspect …`, `apr kernel parity …` and + // `apr debug embed-viz …` moved to the accepts-list below. What is + // left here is still missing, and `apr debug model.gguf embed-viz` + // stays: `embed-viz` is a subcommand of `debug`, not a word that + // may follow the model path. [ "apr attn-viz model.gguf", - "apr dataset audio-inspect --format json", "apr trace model.gguf --check-finite", "apr finetune --parallel ddp", "apr debug model.gguf embed-viz", "apr profile model.gguf --gpu-memory-trace", - "apr kernel parity --impl flash2", "apr quantize model.apr --imatrix calib.jsonl", ] .into_iter() @@ -256,6 +260,13 @@ fn resolver_accepts_real_invocations() { "apr quantize model.gguf --scheme q4k -o out.apr", "apr export model.apr --format gguf", "apr rm model", + // aprender#2377 finding 3: the three producers this batch added. + "apr dataset audio-inspect clip.wav --format json", + "apr dataset audio-inspect clip.wav --format json -o audio.json", + "apr kernel parity --impl tiled --ref naive --json", + "apr kernel parity --impl flash2 --ref naive --head-dim 96 --json", + "apr debug embed-viz --model model.apr --seed 42 -o emb.csv", + "apr debug model.apr --hex", ] .into_iter() .map(|t| (t, resolve(&root, t))) diff --git a/crates/apr-cli/src/lib_dispatch_coverage.rs b/crates/apr-cli/src/lib_dispatch_coverage.rs index 6cd4a5e3bb..8feed567b1 100644 --- a/crates/apr-cli/src/lib_dispatch_coverage.rs +++ b/crates/apr-cli/src/lib_dispatch_coverage.rs @@ -131,7 +131,8 @@ #[test] fn test_dispatch_model_commands_returns_none_for_debug() { let cli = make_cli(Commands::Debug { - file: PathBuf::from("model.apr"), + file: Some(PathBuf::from("model.apr")), + action: None, drama: false, hex: false, strings: false, @@ -670,7 +671,8 @@ #[test] fn test_dispatch_inspection_routes_debug() { let cli = make_cli(Commands::Debug { - file: PathBuf::from("/tmp/nonexistent_pmat540.apr"), + file: Some(PathBuf::from("/tmp/nonexistent_pmat540.apr")), + action: None, drama: false, hex: false, strings: false, diff --git a/crates/apr-cli/src/lib_extract_paths.rs b/crates/apr-cli/src/lib_extract_paths.rs index 3079ac4417..51e20478eb 100644 --- a/crates/apr-cli/src/lib_extract_paths.rs +++ b/crates/apr-cli/src/lib_extract_paths.rs @@ -371,7 +371,8 @@ #[test] fn test_execute_debug_file_not_found() { let cli = make_cli(Commands::Debug { - file: PathBuf::from("/tmp/nonexistent_model_debug_test.apr"), + file: Some(PathBuf::from("/tmp/nonexistent_model_debug_test.apr")), + action: None, drama: false, hex: false, strings: false, diff --git a/crates/apr-cli/src/lib_parse_eval.rs b/crates/apr-cli/src/lib_parse_eval.rs index c1a04425ef..5d0366cedf 100644 --- a/crates/apr-cli/src/lib_parse_eval.rs +++ b/crates/apr-cli/src/lib_parse_eval.rs @@ -325,12 +325,14 @@ match *cli.command { Commands::Debug { file, + action, drama, hex, strings, limit, } => { - assert_eq!(file, PathBuf::from("model.apr")); + assert_eq!(file, Some(PathBuf::from("model.apr"))); + assert!(action.is_none(), "no subcommand was given"); assert!(drama); assert!(hex); assert!(strings); diff --git a/crates/apr-cli/src/lib_parse_rosetta.rs b/crates/apr-cli/src/lib_parse_rosetta.rs index 48fc8ce303..e2c0b90e11 100644 --- a/crates/apr-cli/src/lib_parse_rosetta.rs +++ b/crates/apr-cli/src/lib_parse_rosetta.rs @@ -144,7 +144,8 @@ quality: false, }, Commands::Debug { - file: PathBuf::from("m.apr"), + file: Some(PathBuf::from("m.apr")), + action: None, drama: false, hex: false, strings: false, diff --git a/crates/apr-cli/tests/cli_commands.rs b/crates/apr-cli/tests/cli_commands.rs index 0bfbf458b0..a9d805fe15 100644 --- a/crates/apr-cli/tests/cli_commands.rs +++ b/crates/apr-cli/tests/cli_commands.rs @@ -46,6 +46,9 @@ fn registered_commands() -> Vec<&'static str> { "manifest", "explain", "tensors", + // aprender#2377 finding 3: the producers `*-lint` help documents. + "dataset", + "kernel", "trace", "diff", "hex", diff --git a/crates/aprender-core/src/classification/mod.rs b/crates/aprender-core/src/classification/mod.rs index 035c6bb072..4edb234b98 100644 --- a/crates/aprender-core/src/classification/mod.rs +++ b/crates/aprender-core/src/classification/mod.rs @@ -105,6 +105,31 @@ impl Default for FitMode { } } +/// Fisher-Yates partner index for position `i` in the epoch-`seed` sample shuffle. +/// +/// Contract: `contracts/apr-stochastic-lr-v1.yaml` — `stochastic_convergence` +/// ("shuffled sample order each epoch") and `minibatch_gradient` ("each sample seen +/// exactly once per epoch"). Returning a value in `[0, i]` is what makes the +/// Fisher-Yates pass a permutation. +/// The arithmetic is deliberately `u64`, not `usize`. Refs #2310. Both MMIX +/// constants exceed `u32::MAX`, so as bare `usize` literals they are a hard +/// compile error on 32-bit targets ("literal out of range for `usize`" on +/// `wasm32-unknown-unknown`), and the products overflow `u64` — `seed * MUL` from +/// `seed == 3`, `i * INC` from `i == 13` — which aborts every overflow-checked +/// build on 64-bit too. `wrapping_*` reproduces the 64-bit release-mode result +/// bit-for-bit, so no previously-trained model's epoch order shifts. +#[inline] +fn shuffle_partner(seed: usize, i: usize) -> usize { + const LCG_MULTIPLIER: u64 = 6_364_136_223_846_793_005; + const LCG_INCREMENT: u64 = 1_442_695_040_888_963_407; + let mixed = (seed as u64) + .wrapping_mul(LCG_MULTIPLIER) + .wrapping_add((i as u64).wrapping_mul(LCG_INCREMENT)); + // `i + 1` cannot overflow: `i` indexes a live Vec, so `i < usize::MAX`. + // The remainder is `< i + 1`, hence always representable as `usize`. + (mixed % (i as u64 + 1)) as usize +} + /// Logistic Regression classifier for binary classification. /// /// Uses sigmoid activation and binary cross-entropy loss with @@ -374,7 +399,7 @@ impl LogisticRegression { // Contract: stochastic_convergence — "shuffled sample order each epoch" let seed = epoch; for i in (1..n_samples).rev() { - let j = (seed * 6364136223846793005 + i * 1442695040888963407) % (i + 1); + let j = shuffle_partner(seed, i); indices.swap(i, j); } @@ -422,7 +447,7 @@ impl LogisticRegression { // Shuffle let seed = epoch; for i in (1..n_samples).rev() { - let j = (seed * 6364136223846793005 + i * 1442695040888963407) % (i + 1); + let j = shuffle_partner(seed, i); indices.swap(i, j); } @@ -755,6 +780,12 @@ mod svc_rbf_sklearn_fixture; #[path = "tests_logreg_contract.rs"] mod tests_logreg_contract; +// #2310: the SGD epoch shuffle must compile on 32-bit targets and must not +// overflow on 64-bit. Falsifiers for `shuffle_partner` and both SGD fit modes. +#[cfg(test)] +#[path = "tests_sgd_portable_shuffle.rs"] +mod tests_sgd_portable_shuffle; + // Estimator impl so LogisticRegression works with generic cross_validate / // grid_search (Pillar 1). Labels round-trip through f32; inherent API unchanged. impl crate::traits::Estimator for LogisticRegression { diff --git a/crates/aprender-core/src/classification/tests_sgd_portable_shuffle.rs b/crates/aprender-core/src/classification/tests_sgd_portable_shuffle.rs new file mode 100644 index 0000000000..bb7f54e3fb --- /dev/null +++ b/crates/aprender-core/src/classification/tests_sgd_portable_shuffle.rs @@ -0,0 +1,128 @@ +//! Falsifiers for the SGD epoch-shuffle LCG. Refs #2310. +//! +//! `fit_stochastic` / `fit_minibatch` shuffle the sample order each epoch with a +//! Fisher-Yates pass whose partner index comes from the MMIX LCG constants +//! (`6364136223846793005`, `1442695040888963407`). Those constants are 64-bit. +//! They were written as bare integer literals in a `usize` expression, which: +//! +//! * is a **hard compile error** on 32-bit targets — `error: literal out of +//! range for usize` on `wasm32-unknown-unknown` (the #2310 report), and +//! * **panics** in any overflow-checked (debug/test) build on 64-bit, because +//! `seed * 6364136223846793005` overflows `u64` from `seed == 3` onwards and +//! `i * 1442695040888963407` overflows from `i == 13` onwards. +//! +//! Both `FitMode::Stochastic` and `FitMode::MiniBatch` had zero test coverage, so +//! the 64-bit panic shipped undetected. These tests state what the code must NOT +//! do: it must not panic, must not return a non-permutation, and must not change +//! the 64-bit release-mode partner sequence that the fix preserves bit-for-bit. + +use super::{shuffle_partner, FitMode, LogisticRegression}; +use crate::primitives::Matrix; + +/// Linearly separable 2-D problem with enough rows that the Fisher-Yates loop +/// reaches `i == 13`, the first index at which `i * 1442695040888963407` +/// overflows `u64`. +fn separable_dataset() -> (Matrix, Vec) { + let mut rows = Vec::new(); + let mut labels = Vec::new(); + for k in 0..20 { + let t = k as f32; + rows.push(vec![t * 0.1 - 2.0, 0.5]); + labels.push(usize::from(k >= 10)); + } + let flat: Vec = rows.into_iter().flatten().collect(); + let x = Matrix::from_vec(20, 2, flat).expect("20x2 matrix"); + (x, labels) +} + +/// The partner index must always land inside `[0, i]`, otherwise `indices.swap` +/// would either panic or reach outside the unshuffled prefix and destroy the +/// permutation. Exercised well past the overflow thresholds (`seed >= 3`, +/// `i >= 13`). +#[test] +fn test_shuffle_partner_never_exceeds_i() { + for seed in 0..64usize { + for i in 1..256usize { + let j = shuffle_partner(seed, i); + assert!( + j <= i, + "shuffle_partner({seed}, {i}) = {j} escaped the [0, {i}] window" + ); + } + } +} + +/// Contract `apr-stochastic-lr-v1.yaml` — `minibatch_gradient` invariant +/// "Each sample seen exactly once per epoch". A Fisher-Yates pass over +/// `shuffle_partner` must yield a permutation, never a multiset with repeats. +#[test] +fn test_epoch_shuffle_is_a_permutation() { + let n_samples = 64usize; + for seed in 0..16usize { + let mut indices: Vec = (0..n_samples).collect(); + for i in (1..n_samples).rev() { + let j = shuffle_partner(seed, i); + indices.swap(i, j); + } + let mut sorted = indices.clone(); + sorted.sort_unstable(); + assert_eq!( + sorted, + (0..n_samples).collect::>(), + "epoch seed {seed} produced a non-permutation: {indices:?}" + ); + } +} + +/// Behaviour pin: the fix moves the arithmetic into explicit `u64` wrapping ops, +/// which must reproduce the pre-#2310 64-bit **release** result exactly, so no +/// already-trained model's epoch order shifts. Expected values computed from the +/// wrapping 64-bit definition `(seed*MUL + i*INC) mod (i+1)`. +#[test] +fn test_shuffle_partner_matches_64bit_wrapping_reference() { + let cases: [(usize, usize, usize); 7] = [ + (0, 1, 1), + (1, 1, 0), + (3, 2, 1), + (3, 7, 0), + (4, 12, 6), + (7, 13, 8), + (999, 255, 76), + ]; + for (seed, i, expected) in cases { + assert_eq!( + shuffle_partner(seed, i), + expected, + "shuffle_partner({seed}, {i}) drifted from the 64-bit wrapping reference" + ); + } +} + +/// #2310 regression, 64-bit half: with the default `max_iter` of 1000 the epoch +/// seed passes 3 and the Fisher-Yates index passes 13, so the pre-fix `usize` +/// multiplication aborts the process in any overflow-checked build. +#[test] +fn test_stochastic_fit_survives_overflowing_epoch_and_index() { + let (x, y) = separable_dataset(); + let mut model = LogisticRegression::new().with_fit_mode(FitMode::Stochastic); + model.fit(&x, &y).expect("stochastic fit must succeed"); + let acc = model.score(&x, &y); + assert!( + acc > 0.9, + "stochastic fit on a separable set scored {acc}, below the 0.9 floor" + ); +} + +/// #2310 regression, mini-batch path (a second, independently-compiled copy of +/// the same expression lived in `fit_minibatch`). +#[test] +fn test_minibatch_fit_survives_overflowing_epoch_and_index() { + let (x, y) = separable_dataset(); + let mut model = LogisticRegression::new().with_fit_mode(FitMode::MiniBatch(4)); + model.fit(&x, &y).expect("mini-batch fit must succeed"); + let acc = model.score(&x, &y); + assert!( + acc > 0.9, + "mini-batch fit on a separable set scored {acc}, below the 0.9 floor" + ); +} diff --git a/crates/aprender-serve/src/api/batch.rs b/crates/aprender-serve/src/api/batch.rs index dd3dfb8586..a5ab285fad 100644 --- a/crates/aprender-serve/src/api/batch.rs +++ b/crates/aprender-serve/src/api/batch.rs @@ -20,21 +20,25 @@ pub(super) struct QuantizedSampling { /// /// # Errors /// -/// 400 for a negative or NaN `temperature`, an unknown `strategy`, or a `top_p` -/// outside `(0, 1]` when nucleus sampling was requested. +/// 400 for a `temperature` outside `[0, inf)` finite (negative, NaN or infinite), +/// an unknown `strategy`, or a `top_p` outside `(0, 1]` when nucleus sampling was +/// requested. pub(super) fn resolve_quantized_sampling( strategy: &str, top_k: usize, top_p: f32, temperature: f32, ) -> Result { - // NaN is rejected alongside negatives. A negative temperature divides the logits - // by a negative number, which inverts the distribution: the model then emits its - // LEAST likely tokens and the client cannot tell that from a bad model. - if temperature.is_nan() || temperature < 0.0 { + // NaN and ±inf are rejected alongside negatives. A negative temperature divides + // the logits by a negative number, which inverts the distribution: the model then + // emits its LEAST likely tokens and the client cannot tell that from a bad model. + // `is_nan() || < 0.0` used to let `+inf` through, and `+inf` is rejected by the + // dense sampler with HTTP 500 and flattens every logit to 0.0 on the quantized + // one — so the whole non-finite class is refused here, not just NaN. + if !temperature.is_finite() || temperature < 0.0 { return Err(api_err( StatusCode::BAD_REQUEST, - format!("temperature must be >= 0, got {temperature}"), + format!("temperature must be a finite number >= 0, got {temperature}"), )); } diff --git a/crates/aprender-serve/src/api/cancel_scope.rs b/crates/aprender-serve/src/api/cancel_scope.rs index c803e184fc..fd18bbba0e 100644 --- a/crates/aprender-serve/src/api/cancel_scope.rs +++ b/crates/aprender-serve/src/api/cancel_scope.rs @@ -48,6 +48,23 @@ //! | 2 | Nothing ever sets the flag. | //! | 3 | No await point; the drop never happens mid-generation. | //! +//! # Why the guard is disarmed on completion (aprender#2375(1)) +//! +//! Step 2's guard originally fired on BOTH exits — abandonment and normal +//! completion — because firing late was assumed harmless. It is harmless only +//! for a handler that finishes its generation before returning. The streaming +//! chat backends do the opposite: they hand the decode loop to +//! `spawn_blocking` and return the SSE response at once, so this future +//! completes while the loop is still in prefill. The guard then cancelled it at +//! the very first poll, and `POST /v1/chat/completions` with `"stream":true` +//! returned a well-formed event stream containing the opening chunk, the +//! terminal chunk, and **zero content deltas** — every streamed reply empty. +//! +//! So a completed handler disarms the guard. Abandonment (drop) and a panicking +//! handler still cancel. An abandoned *stream* is still stopped, by the +//! mechanism that has always covered it: hyper drops the response body → the +//! SSE receiver drops → the generator's `on_token` send fails → the loop breaks. +//! //! # Panics are preserved //! //! A panicking handler is re-raised with [`std::panic::resume_unwind`] so hyper @@ -76,16 +93,23 @@ pub(crate) async fn cancel_on_disconnect(mut request: Request, next: Next) let token = CancelToken::new(); request.extensions_mut().insert(token.clone()); - // (2) Lives in THIS future. Dropped when the response is finished *or* when - // axum abandons the request because the client went away. - let _disconnect_guard = token.cancel_on_drop(); + // (2) Lives in THIS future. Dropped when axum abandons the request because + // the client went away — and DISARMED first when the handler returns, since + // a returned response may still be streaming from a background decode loop. + let mut disconnect_guard = token.cancel_on_drop(); // (3) The handler outlives this future's drop, so its decode loop is still // running to see the flag the guard just set. let handle = tokio::spawn(async move { next.run(request).await }); match handle.await { - Ok(response) => response, + Ok(response) => { + // The handler produced a response, so this request was not + // abandoned. Cancelling here would stop a streaming body that has + // not been written yet — see the module docs (#2375(1)). + disconnect_guard.disarm(); + response + }, Err(join_err) if join_err.is_panic() => std::panic::resume_unwind(join_err.into_panic()), Err(join_err) => ( StatusCode::INTERNAL_SERVER_ERROR, diff --git a/crates/aprender-serve/src/api/chat_completions_stream.rs b/crates/aprender-serve/src/api/chat_completions_stream.rs index 264e811057..c30dffe7ac 100644 --- a/crates/aprender-serve/src/api/chat_completions_stream.rs +++ b/crates/aprender-serve/src/api/chat_completions_stream.rs @@ -53,142 +53,32 @@ fn streaming_text_deltas( StreamedText { deltas, stopped } } -/// Resolve the `GenerationConfig` for a streaming chat completion (PMAT-790). +/// OpenAI-compatible `/v1/chat/completions/stream` endpoint (SSE). /// -/// `temperature == 0` is the canonical OpenAI request for deterministic (greedy) output, and -/// every non-streaming `/v1/chat/completions` backend honors it via the `top_k == 1` greedy -/// path. The streaming handler previously passed the raw `0.0` into `GenerationConfig`, so -/// `model.generate` -> `sample_token` -> `apply_temperature(0.0)` returned an `InvalidShape` -/// error ("Temperature must be a positive finite number") which the handler mapped to HTTP -/// 500 — so EVERY streaming chat completion with `temperature: 0` was broken. +/// aprender#2375(4): this route is mounted unconditionally and printed by the +/// server's own banner, and it answered `404 {"error":"Model registry error: No +/// model available"}` on every `apr serve run model.gguf` — the standard +/// deployment. It resolved the dense f32 [`Model`](crate::layers::Model) through +/// `AppState::get_model`, which is `None` whenever the weights are quantized, so +/// the route was dead on arrival for the whole GGUF/APR fleet while +/// `/v1/chat/completions` on the same process answered 200 with real text. /// -/// This helper forces `Greedy` for `temperature == 0` and substitutes a no-op temperature of -/// `1.0` so the sampler never sees a non-positive scale. For positive temperatures the -/// behavior is unchanged: greedy by default, or top-p when `top_p` is set. -fn resolve_stream_generation_config( - temperature: f32, - top_p: Option, - max_tokens: usize, -) -> GenerationConfig { - if temperature == 0.0 { - // Deterministic: greedy argmax, with a safe (no-op) temperature scale. - return GenerationConfig::default() - .with_max_tokens(max_tokens) - .with_temperature(1.0); - } - - let mut config = GenerationConfig::default() - .with_max_tokens(max_tokens) - .with_temperature(temperature); - if let Some(p) = top_p { - config.strategy = SamplingStrategy::TopP { p }; - } - config -} - -/// OpenAI-compatible /v1/chat/completions streaming endpoint (SSE) +/// It also carried a SECOND, separate implementation of chat completion — +/// its own prompt formatting, sampling config, id format and delta builder — +/// which is how the two paths drifted apart in the first place (this one alone +/// handled `temperature: 0`; the main one alone reached the quantized, cached, +/// CUDA and MoE backends). +/// +/// So it is now exactly what its name says: `/v1/chat/completions` with +/// `stream` forced on. Every backend, one wire format, one set of falsifiers. pub async fn openai_chat_completions_stream_handler( State(state): State, - Json(request): Json, -) -> Result>>, (StatusCode, Json)> { - let model_id = if request.model == "default" || request.model.is_empty() { - None - } else { - Some(request.model.as_str()) - }; - - let (model, tokenizer) = state.get_model(model_id).map_err(|e| { - state.metrics.record_failure(); - ( - StatusCode::NOT_FOUND, - Json(ErrorResponse { - error: e.to_string(), - }), - ) - })?; - - let prompt_text = format_chat_messages(&request.messages, Some(&request.model)); - let prompt_ids = tokenizer.encode(&prompt_text); - if prompt_ids.is_empty() { - state.metrics.record_failure(); - return Err(( - StatusCode::BAD_REQUEST, - Json(ErrorResponse { - error: "Messages cannot be empty".to_string(), - }), - )); - } - - let prompt_len = prompt_ids.len(); - let prompt: Vec = prompt_ids.iter().map(|&id| id as usize).collect(); - - // GH-665: Cap max_tokens to prevent hangs on large values - let max_tokens = request.max_tokens.unwrap_or(256).min(4096); - let config = resolve_stream_generation_config( - request.temperature.unwrap_or(0.7), - request.top_p, - max_tokens, - ); - - let request_id = format!( - "chatcmpl-{}", - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_nanos()) - .unwrap_or(0) - ); - - let generated = model.generate(&prompt, &config).map_err(|e| { - state.metrics.record_failure(); - ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(ErrorResponse { - error: e.to_string(), - }), - ) - })?; - - let token_ids: Vec = generated - .iter() - .filter_map(|&id| u32::try_from(id).ok()) - .collect(); - - let generated_ids = token_ids[prompt_len..].to_vec(); - let model_name = request.model.clone(); - let request_id_clone = request_id.clone(); - - // PMAT-758: precompute char-safe, stop-truncated deltas BEFORE streaming. The previous - // per-token `decode(&[token_id])` split multi-byte UTF-8 (emoji/CJK -> U+FFFD) and - // ignored request.stop entirely. All tokens are already generated here, so we can decode - // cumulatively and emit only complete-char, pre-stop deltas. - let StreamedText { deltas, stopped } = - streaming_text_deltas(&tokenizer, &generated_ids, request.stop.as_deref()); - // #2375(6): the terminal chunk must state why generation ended. The - // non-streaming path reports "length" at the budget; so does this one now. - let finish = FinishReason::from_generation(stopped, generated_ids.len(), max_tokens); - - let stream = async_stream::stream! { - // PMAT-753: pass ONLY the JSON payload to Event::data() — axum's Sse adds the - // `data: ` field prefix and the `\n\n` terminator itself. A manual `data: ` prefix - // would double-prefix the wire and break JSON.parse for every spec-compliant client. - let initial = ChatCompletionChunk::initial(&request_id_clone, &model_name); - let data = serde_json::to_string(&initial).unwrap_or_default(); - yield Ok(Event::default().data(data)); - - for delta in &deltas { - let chunk = ChatCompletionChunk::content(&request_id_clone, &model_name, delta); - let data = serde_json::to_string(&chunk).unwrap_or_default(); - yield Ok(Event::default().data(data)); - } - - let done = ChatCompletionChunk::done(&request_id_clone, &model_name, finish); - let data = serde_json::to_string(&done).unwrap_or_default(); - yield Ok(Event::default().data(data)); - - yield Ok(Event::default().data("[DONE]")); - }; - - Ok(Sse::new(stream)) + headers: HeaderMap, + Extension(cancel): Extension, + Json(mut request): Json, +) -> Response { + request.stream = true; + openai_chat_completions_handler(State(state), headers, Extension(cancel), Json(request)).await } #[cfg(test)] @@ -235,14 +125,15 @@ mod pmat758_streaming_delta_tests { } } -// PMAT-790: streaming /v1/chat/completions with `temperature: 0` must not 500. The handler -// builds a GenerationConfig and runs it through `model.generate` -> `sample_token` -> -// `apply_temperature`, which rejects a non-positive temperature. `temperature: 0` is the -// canonical OpenAI deterministic request and is honored by every non-streaming backend; it -// must resolve to a runnable, greedy config here too. +// PMAT-790 (+ #2375): a dense chat/completions request with `temperature: 0` must not 500. +// The handler builds a GenerationConfig and runs it through `model.generate` -> +// `sample_token` -> `apply_temperature`, which rejects a non-positive temperature. +// `temperature: 0` is the canonical OpenAI deterministic request; it must resolve to a +// runnable, greedy config on EVERY dense backend, which is why the resolver these tests +// drive is now the shared one in `realize_handlers` rather than a stream-only copy. #[cfg(test)] mod pmat790_stream_temperature_zero_tests { - use super::resolve_stream_generation_config; + use crate::api::realize_handlers::resolve_dense_generation_config as resolve_stream_generation_config; use crate::generate::{sample_token, SamplingStrategy}; use crate::tensor::Tensor; diff --git a/crates/aprender-serve/src/api/cuda_chat_backend.rs b/crates/aprender-serve/src/api/cuda_chat_backend.rs index d1ba6b843e..9ce0e390e3 100644 --- a/crates/aprender-serve/src/api/cuda_chat_backend.rs +++ b/crates/aprender-serve/src/api/cuda_chat_backend.rs @@ -142,13 +142,15 @@ async fn try_cuda_backend( let cuda_model_clone = cuda_model_lock.clone(); let prompt_ids_clone = prompt_ids.clone(); let q_config_clone = q_config.clone(); + let sink_metrics = state.metrics.clone(); tokio::task::spawn_blocking(move || { let mut cuda_model = cuda_model_clone.write().expect("operation failed"); let result = cuda_model.generate_gpu_resident_streaming( &prompt_ids_clone, &q_config_clone, - |token_id| tx.blocking_send(Ok(token_id)).is_ok(), + // Stops when the client goes away — see `streaming_token_sink`. + crate::api::openai_handlers::streaming_token_sink(tx.clone(), sink_metrics), ); if let Err(e) = result { let _ = tx.blocking_send(Err(e.to_string())); @@ -267,12 +269,14 @@ fn try_quantized_backend( let quantized_model_clone = quantized_model.clone(); let prompt_ids_clone = prompt_ids.clone(); let q_config_clone = q_config.clone(); + let sink_metrics = state.metrics.clone(); tokio::task::spawn_blocking(move || { let result = quantized_model_clone.generate_with_cache_streaming( &prompt_ids_clone, &q_config_clone, - |token_id| tx.blocking_send(Ok(token_id)).is_ok(), + // Stops when the client goes away — see `streaming_token_sink`. + crate::api::openai_handlers::streaming_token_sink(tx.clone(), sink_metrics), ); if let Err(e) = result { let _ = tx.blocking_send(Err(e.to_string())); @@ -327,17 +331,18 @@ fn convert_token_ids(ids: &[usize]) -> Result, String> { .collect() } -/// Build generation config from request parameters +/// Build generation config from request parameters. +/// +/// #2375: `temperature: 0` — the OpenAI-canonical deterministic request — +/// reached `apply_temperature` unchanged here and made this backend answer +/// HTTP 500 ("Temperature must be a positive finite number") for every dense +/// model. The resolution now lives in ONE place, shared with `/v1/completions`. fn build_gen_config(request: &ChatCompletionRequest) -> GenerationConfig { - let max_tokens = request.max_tokens.unwrap_or(256); - let temperature = request.temperature.unwrap_or(0.7); - let mut config = GenerationConfig::default() - .with_max_tokens(max_tokens) - .with_temperature(temperature); - if let Some(top_p) = request.top_p { - config.strategy = SamplingStrategy::TopP { p: top_p }; - } - config + crate::api::realize_handlers::resolve_dense_generation_config( + request.temperature.unwrap_or(0.7), + request.top_p, + request.max_tokens.unwrap_or(256), + ) } /// Registry-based model fallback (no specialized backend). @@ -800,6 +805,7 @@ fn try_qwen3_moe_backend( let quantized_clone = quantized.clone(); let input_ids_clone = input_ids.clone(); let gen_config_clone = gen_config.clone(); + let sink_metrics = state.metrics.clone(); tokio::task::spawn_blocking(move || { let result = crate::infer::qwen3_moe_generate::run_qwen3_moe_generate_streaming( @@ -807,7 +813,8 @@ fn try_qwen3_moe_backend( &quantized_clone, &input_ids_clone, &gen_config_clone, - |token_id| tx.blocking_send(Ok(token_id)).is_ok(), + // Stops when the client goes away — see `streaming_token_sink`. + crate::api::openai_handlers::streaming_token_sink(tx.clone(), sink_metrics), ); if let Err(e) = result { let _ = tx.blocking_send(Err(e.to_string())); diff --git a/crates/aprender-serve/src/api/gpu_completions_handler.rs b/crates/aprender-serve/src/api/gpu_completions_handler.rs index f52c8d8b4a..74b351be4c 100644 --- a/crates/aprender-serve/src/api/gpu_completions_handler.rs +++ b/crates/aprender-serve/src/api/gpu_completions_handler.rs @@ -141,13 +141,14 @@ fn registry_completions( let prompt_tokens = prompt_ids.len(); let prompt: Vec = prompt_ids.iter().map(|&id| id as usize).collect(); - let mut config = GenerationConfig::default() - .with_max_tokens(max_tokens) - .with_temperature(temperature) - .with_cancel(cancel.clone()); - if let Some(top_p) = request.top_p { - config.strategy = SamplingStrategy::TopP { p: top_p as f32 }; - } + // #2375: `temperature: 0` used to reach `apply_temperature` unchanged and + // answer HTTP 500 for the OpenAI-canonical deterministic request. + let config = resolve_dense_generation_config( + temperature, + request.top_p.map(|p| p as f32), + max_tokens, + ) + .with_cancel(cancel.clone()); let generated = model .generate(&prompt, &config) diff --git a/crates/aprender-serve/src/api/gpu_handlers.rs b/crates/aprender-serve/src/api/gpu_handlers.rs index eb5c0e7a8d..ae69a48162 100644 --- a/crates/aprender-serve/src/api/gpu_handlers.rs +++ b/crates/aprender-serve/src/api/gpu_handlers.rs @@ -82,8 +82,13 @@ pub struct GpuBatchRequest { /// Maximum tokens to generate per prompt #[serde(default = "default_max_tokens")] pub max_tokens: usize, - /// Temperature for sampling (0.0 = greedy) - #[serde(default)] + /// Temperature for sampling (0.0 = greedy). + /// + /// Rejected at deserialization when outside `[0, ∞)` finite (aprender#2375). + #[serde( + default, + deserialize_with = "crate::api::types::deserialize_temperature_f32_required" + )] pub temperature: f32, /// Top-k sampling (1 = greedy) #[serde(default = "default_top_k")] diff --git a/crates/aprender-serve/src/api/mod_create_demo.rs b/crates/aprender-serve/src/api/mod_create_demo.rs index b9c675fd7c..1663830a80 100644 --- a/crates/aprender-serve/src/api/mod_create_demo.rs +++ b/crates/aprender-serve/src/api/mod_create_demo.rs @@ -69,8 +69,13 @@ pub struct ChatCompletionRequest { /// Maximum tokens to generate #[serde(default)] pub max_tokens: Option, - /// Sampling temperature - #[serde(default)] + /// Sampling temperature. + /// + /// A temperature outside `[0, ∞)` finite is rejected at deserialization + /// (aprender#2375) — see `types::deserialize_temperature_f32`. `/api/chat` + /// and `/api/generate` build this struct in Rust rather than deserializing + /// it, so their own `options.temperature` carries the same guard. + #[serde(default, deserialize_with = "crate::api::types::deserialize_temperature_f32")] pub temperature: Option, /// Nucleus sampling #[serde(default)] diff --git a/crates/aprender-serve/src/api/ollama_handlers.rs b/crates/aprender-serve/src/api/ollama_handlers.rs index f45a74b4e1..8b6674b524 100644 --- a/crates/aprender-serve/src/api/ollama_handlers.rs +++ b/crates/aprender-serve/src/api/ollama_handlers.rs @@ -64,7 +64,14 @@ pub struct OllamaMessage { #[derive(Debug, Clone, Default, Deserialize)] pub struct OllamaOptions { /// Sampling temperature. - #[serde(default)] + /// + /// Rejected at deserialization when outside `[0, ∞)` finite (aprender#2375): + /// `to_chat_request` builds a `ChatCompletionRequest` in Rust, so the guard + /// on that struct's field never sees an Ollama request. + #[serde( + default, + deserialize_with = "crate::api::types::deserialize_temperature_f32" + )] pub temperature: Option, /// Nucleus sampling. #[serde(default)] diff --git a/crates/aprender-serve/src/api/openai_handlers.rs b/crates/aprender-serve/src/api/openai_handlers.rs index d85aeeb58d..7a9a4f049c 100644 --- a/crates/aprender-serve/src/api/openai_handlers.rs +++ b/crates/aprender-serve/src/api/openai_handlers.rs @@ -569,6 +569,38 @@ fn pregenerated_sse_response( Sse::new(stream).into_response() } +/// The `on_token` callback every streaming backend hands to its decode loop. +/// +/// This is the mechanism that stops an ABANDONED stream, and the reason the +/// cancellation layer is allowed to disarm its guard on completion +/// (aprender#2375(1)): a streaming handler returns its SSE response while the +/// decode loop is still running, so cancelling on completion emptied every +/// reply. What covers the abandoned case instead is this chain — +/// +/// > hyper drops the response body → the SSE receiver drops → this send fails → +/// > the callback returns `false` → `generate_with_cache_streaming` breaks. +/// +/// It is one function because all three streaming backends (quantized, CUDA, +/// Qwen3-MoE) carried their own copy of `|t| tx.blocking_send(Ok(t)).is_ok()`, +/// and a copy is exactly how one of them would end up ignoring the send result +/// and burning a core for a client that left. +/// +/// The abandonment is RECORDED, which is what makes the mechanism observable: +/// `record_stream_abandoned` fires once per abandoned stream, and a second +/// record for the same stream means the loop did not break. +pub(crate) fn streaming_token_sink( + tx: tokio::sync::mpsc::Sender>, + metrics: Arc, +) -> impl FnMut(u32) -> bool { + move |token_id| { + if tx.blocking_send(Ok(token_id)).is_ok() { + return true; + } + metrics.record_stream_abandoned(); + false + } +} + /// Build a true-streaming SSE response with keep-alive (tokens arrive via channel). /// /// Deltas are raw per-token decodes — see `decode_token`. The `clean` parameter diff --git a/crates/aprender-serve/src/api/realize_handlers.rs b/crates/aprender-serve/src/api/realize_handlers.rs index 4345f5f5df..f542812e37 100644 --- a/crates/aprender-serve/src/api/realize_handlers.rs +++ b/crates/aprender-serve/src/api/realize_handlers.rs @@ -464,8 +464,15 @@ pub struct CompletionRequest { /// Maximum tokens to generate #[serde(skip_serializing_if = "Option::is_none")] pub max_tokens: Option, - /// Temperature - #[serde(skip_serializing_if = "Option::is_none")] + /// Temperature. + /// + /// A temperature outside `[0, ∞)` finite is rejected at deserialization + /// (aprender#2375) — see `types::deserialize_temperature_f64`. + #[serde( + default, + skip_serializing_if = "Option::is_none", + deserialize_with = "crate::api::types::deserialize_temperature_f64" + )] pub temperature: Option, /// Top-p sampling #[serde(skip_serializing_if = "Option::is_none")] diff --git a/crates/aprender-serve/src/api/realize_handlers_embed_completion.rs b/crates/aprender-serve/src/api/realize_handlers_embed_completion.rs index 83f570d71f..f6f6471168 100644 --- a/crates/aprender-serve/src/api/realize_handlers_embed_completion.rs +++ b/crates/aprender-serve/src/api/realize_handlers_embed_completion.rs @@ -534,6 +534,60 @@ pub(crate) fn truncate_at_stop(text: String, stops: Option<&[String]>) -> String } } +/// Build the dense-`Model` [`GenerationConfig`] for an OpenAI request. +/// +/// `temperature: 0` is the canonical OpenAI request for deterministic output. +/// Passing that 0 straight into the config makes `model.generate` -> +/// `sample_token` -> `apply_temperature(0.0)` return +/// `InvalidShape: Temperature must be a positive finite number`, which every +/// dense backend mapped to **HTTP 500** — so `temperature: 0` was unserveable on +/// `/v1/chat/completions` and `/v1/completions` for any registry/safetensors +/// model. PMAT-790 fixed exactly one caller (the `/v1/chat/completions/stream` +/// handler) with a private copy of this logic; the other two kept 500ing, which +/// is what made a shared helper necessary. +/// +/// Deterministic requests resolve to greedy argmax with a no-op temperature of +/// `1.0` (the sampler never sees a non-positive scale). Positive temperatures +/// are unchanged: greedy by default, top-p when `top_p` is set. +/// +/// # The rest of the domain +/// +/// The first version of this resolver special-cased `temperature == 0.0` and +/// nothing else, so `-1`, `NaN`, `+inf` and a `1e40` that narrows to `+inf` +/// still reached `apply_temperature` and still produced +/// `500 {"error":"Invalid shape: Temperature must be a positive finite number"}` +/// — the exact body the fix set out to eliminate. +/// +/// Those values are now refused where the request is parsed (they are not +/// representable in a deserialized request — `types::deserialize_temperature_f32`), +/// which is the fix a client observes: 4xx naming the field. This function is +/// TOTAL as well, so that no Rust caller can construct a config the sampler +/// rejects: anything that is not a positive finite temperature resolves to the +/// same deterministic greedy config as `0`. +/// +/// `temperature.is_finite()` is load-bearing and cannot be replaced by a +/// comparison: `NaN == 0.0`, `NaN > 0.0` and `NaN < 0.0` are all false, so a +/// comparison-only guard passes NaN straight through (aprender#2391). +pub(crate) fn resolve_dense_generation_config( + temperature: f32, + top_p: Option, + max_tokens: usize, +) -> GenerationConfig { + if !temperature.is_finite() || temperature <= 0.0 { + return GenerationConfig::default() + .with_max_tokens(max_tokens) + .with_temperature(1.0); + } + + let mut config = GenerationConfig::default() + .with_max_tokens(max_tokens) + .with_temperature(temperature); + if let Some(p) = top_p { + config.strategy = SamplingStrategy::TopP { p }; + } + config +} + /// Cached model backend (includes batch path). Returns None if not available. #[cfg(feature = "gpu")] async fn try_cached_completions( diff --git a/crates/aprender-serve/src/api/router.rs b/crates/aprender-serve/src/api/router.rs index ca4bbe4783..ec5b29a569 100644 --- a/crates/aprender-serve/src/api/router.rs +++ b/crates/aprender-serve/src/api/router.rs @@ -725,44 +725,87 @@ async fn dispatch_reset_handler(State(_state): State) -> axum::respons .into_response() } +/// The name this server answers to for the model it is serving. +/// +/// aprender#2375(7): `/v1/metrics` reported `model_name` as the literal +/// `"phi-2-q4_k_m"` whenever a cached GPU model was resident and `"N/A"` +/// otherwise — neither derived from the model actually loaded, so a monitor +/// watching a fleet labelled every server with the same wrong name or with no +/// name at all. Derived here from what the loader measured, falling back to the +/// id `/v1/models` advertises, and only reporting `"N/A"` when nothing is +/// resident to name. +fn served_model_name(state: &AppState) -> String { + if let Some(stem) = state + .model_source() + .and_then(crate::api::ModelSourceInfo::path) + .and_then(|p| { + std::path::Path::new(p) + .file_stem() + .map(|s| s.to_string_lossy().into_owned()) + }) + .filter(|s| !s.is_empty()) + { + return stem; + } + if let Some(id) = state.default_model_id.clone() { + return id; + } + if state.model_loaded() { + // The id `GET /v1/models` lists in single-model mode, so a client can + // send this straight back as `"model"`. + return "default".to_string(); + } + "N/A".to_string() +} + +/// Request-latency percentiles for `/v1/metrics`, in milliseconds. +/// +/// Shared by both feature variants of [`server_metrics_handler`] so they cannot +/// disagree about the same server: the GPU build reported a hardcoded +/// `(0.0, 0.0, 0.0)` whenever no GPU dispatch metrics existed — which is every +/// CPU deployment — while the non-GPU build reported `avg`, `avg * 1.5` and +/// `avg * 2.0`, two of which are not measurements at all. +/// +/// Kernel-dispatch percentiles still win when GPU work was actually dispatched; +/// otherwise these are the collector's measured request latencies, and +/// `(0.0, 0.0, 0.0)` now means only "no request has completed yet". +fn measured_latency_percentiles(state: &AppState) -> (f64, f64, f64) { + #[cfg(feature = "gpu")] + if let Some(dispatch) = state.dispatch_metrics() { + if dispatch.gpu_dispatches() > 0 { + return ( + dispatch.gpu_latency_p50_us() / 1000.0, + dispatch.gpu_latency_p95_us() / 1000.0, + dispatch.gpu_latency_p99_us() / 1000.0, + ); + } + if dispatch.cpu_dispatches() > 0 { + return ( + dispatch.cpu_latency_p50_us() / 1000.0, + dispatch.cpu_latency_p95_us() / 1000.0, + dispatch.cpu_latency_p99_us() / 1000.0, + ); + } + } + state + .metrics + .latency_percentiles() + .map_or((0.0, 0.0, 0.0), |p| (p.p50_ms, p.p95_ms, p.p99_ms)) +} + /// Server metrics handler for TUI monitoring (PARITY-107) /// GET /v1/metrics - Returns JSON metrics for realizar-monitor #[cfg(feature = "gpu")] async fn server_metrics_handler(State(state): State) -> Json { let snapshot = state.metrics.snapshot(); - // Get latency percentiles from dispatch metrics (in microseconds, convert to ms) - let (latency_p50_ms, latency_p95_ms, latency_p99_ms, gpu_dispatches, cuda_path_active) = - if let Some(dispatch) = state.dispatch_metrics() { - // Use GPU latency if available, otherwise CPU latency - let gpu_p50 = dispatch.gpu_latency_p50_us(); - let gpu_p95 = dispatch.gpu_latency_p95_us(); - let gpu_p99 = dispatch.gpu_latency_p99_us(); - let gpu_count = dispatch.gpu_dispatches(); - - if gpu_count > 0 { - ( - gpu_p50 / 1000.0, - gpu_p95 / 1000.0, - gpu_p99 / 1000.0, - gpu_count, - true, - ) - } else { - let cpu_p50 = dispatch.cpu_latency_p50_us(); - let cpu_p95 = dispatch.cpu_latency_p95_us(); - let cpu_p99 = dispatch.cpu_latency_p99_us(); - ( - cpu_p50 / 1000.0, - cpu_p95 / 1000.0, - cpu_p99 / 1000.0, - 0, - false, - ) - } - } else { - (0.0, 0.0, 0.0, 0, false) - }; + let (latency_p50_ms, latency_p95_ms, latency_p99_ms) = measured_latency_percentiles(&state); + let (gpu_dispatches, cuda_path_active) = state + .dispatch_metrics() + .map_or((0, false), |dispatch| { + let gpu = dispatch.gpu_dispatches(); + (gpu, gpu > 0) + }); // Get GPU memory from cached model let (gpu_memory_used_bytes, gpu_memory_total_bytes): (u64, u64) = @@ -794,12 +837,7 @@ async fn server_metrics_handler(State(state): State) -> Json) -> Json) -> Json { let snapshot = state.metrics.snapshot(); + let (latency_p50_ms, latency_p95_ms, latency_p99_ms) = measured_latency_percentiles(&state); Json(ServerMetricsResponse { throughput_tok_per_sec: snapshot.tokens_per_sec, - latency_p50_ms: snapshot.avg_latency_ms, - latency_p95_ms: snapshot.avg_latency_ms * 1.5, - latency_p99_ms: snapshot.avg_latency_ms * 2.0, + latency_p50_ms, + latency_p95_ms, + latency_p99_ms, gpu_memory_used_bytes: 0, gpu_memory_total_bytes: 0, gpu_utilization_percent: 0, @@ -838,6 +881,6 @@ async fn server_metrics_handler(State(state): State) -> Json AppState { + super::native_routes_2376::quantized_state() +} + +async fn send(state: AppState, uri: &str, json: &str) -> (StatusCode, String, String) { + let response = create_router(state) + .oneshot( + Request::builder() + .method("POST") + .uri(uri) + .header("content-type", "application/json") + .body(Body::from(json.to_string())) + .expect("build request"), + ) + .await + .expect("dispatch"); + let status = response.status(); + let content_type = response + .headers() + .get("content-type") + .and_then(|v| v.to_str().ok()) + .unwrap_or_default() + .to_string(); + let bytes = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .expect("read body"); + ( + status, + content_type, + String::from_utf8_lossy(&bytes).into_owned(), + ) +} + +async fn get(state: AppState, uri: &str) -> (StatusCode, String) { + let response = create_router(state) + .oneshot( + Request::builder() + .uri(uri) + .body(Body::empty()) + .expect("build request"), + ) + .await + .expect("dispatch"); + let status = response.status(); + let bytes = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .expect("read body"); + (status, String::from_utf8_lossy(&bytes).into_owned()) +} + +/// Concatenate `choices[0].delta.content` across an SSE body — what every +/// OpenAI SDK does to reconstruct the message. +fn concat_deltas(sse_body: &str) -> String { + sse_body + .lines() + .filter_map(|line| line.strip_prefix("data: ")) + .filter(|payload| payload.trim() != "[DONE]") + .filter_map(|payload| serde_json::from_str::(payload).ok()) + .filter_map(|frame| { + frame["choices"][0]["delta"]["content"] + .as_str() + .map(str::to_string) + }) + .collect() +} + +/// The non-streamed `choices[0].message.content` for the same request. +async fn buffered_chat_content(state: AppState, request_json: &str) -> String { + let (status, _, body) = send(state, "/v1/chat/completions", request_json).await; + assert_eq!( + status, + StatusCode::OK, + "the non-streaming control must succeed, or the streamed comparison below \ + proves nothing: {body}" + ); + let json: serde_json::Value = serde_json::from_str(&body).expect("chat completion is JSON"); + json["choices"][0]["message"]["content"] + .as_str() + .expect("non-streaming content") + .to_string() +} + +// --------------------------------------------------------------------------- +// #2375(1) regression — a streamed reply must carry the reply +// --------------------------------------------------------------------------- + +const CHAT_REQUEST: &str = + r#"{"model":"default","messages":[{"role":"user","content":"token5 token6"}],"max_tokens":6"#; + +/// Streaming and non-streaming views of the same request must carry the same +/// text — through the ROUTER, with the cancellation layer mounted. +/// +/// Observed before the fix: `STREAMCAT=""` against `NONSTREAM= +/// "token5token21token9token21token34token25"`. The stream was well-formed +/// (opening chunk, terminal chunk, `[DONE]`) and completely empty of content, +/// because the per-request `CancelOnDrop` guard fired the moment the handler +/// returned the SSE response and the decode loop observed it on its first poll. +#[tokio::test] +#[cfg(feature = "gpu")] +async fn streamed_chat_body_carries_the_same_text_as_the_buffered_one() { + let expected = buffered_chat_content(quantized_state(), &format!("{CHAT_REQUEST}}}")).await; + assert!( + !expected.is_empty(), + "the fixture must generate SOME text, or an empty stream would match it" + ); + + let (status, content_type, body) = send( + quantized_state(), + "/v1/chat/completions", + &format!("{CHAT_REQUEST},\"stream\":true}}"), + ) + .await; + + assert_eq!(status, StatusCode::OK); + assert!( + content_type.starts_with("text/event-stream"), + "stream:true must be framed as SSE, got {content_type:?}" + ); + assert_eq!( + concat_deltas(&body), + expected, + "the concatenated SSE deltas must reproduce the buffered message; an empty \ + result is the cancellation guard stopping the decode loop before its first \ + token (#2375(1))" + ); +} + +/// The same property over a REAL socket, served by hyper. +/// +/// `tower::ServiceExt::oneshot` drives the router directly, so it can be told +/// that it only proves something about the tower stack. This binds a port, +/// serves the router with `axum::serve`, and reads the event stream off TCP — +/// the transport `apr serve run` uses. It fails the same way when the guard is +/// re-armed, which is what makes the cheaper test above trustworthy. +#[tokio::test(flavor = "multi_thread")] +#[cfg(feature = "gpu")] +async fn streamed_chat_body_carries_the_text_over_a_real_socket() { + let expected = buffered_chat_content(quantized_state(), &format!("{CHAT_REQUEST}}}")).await; + assert!(!expected.is_empty(), "the fixture must generate SOME text"); + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind an ephemeral port"); + let addr = listener.local_addr().expect("local addr"); + let server = tokio::spawn(async move { + axum::serve(listener, create_router(quantized_state()).into_make_service()) + .await + .ok(); + }); + + let body = reqwest::Client::new() + .post(format!("http://{addr}/v1/chat/completions")) + .header("content-type", "application/json") + .body(format!("{CHAT_REQUEST},\"stream\":true}}")) + .send() + .await + .expect("HTTP request") + .text() + .await + .expect("read the event stream"); + server.abort(); + + assert_eq!( + concat_deltas(&body), + expected, + "over a real socket the streamed deltas must reproduce the buffered message; \ + with the guard armed on completion this delivered an empty stream too" + ); +} + +/// The other half of the guard change: an ABANDONED stream must still stop. +/// +/// Disarming the guard on completion (above) removed the cancellation layer's +/// coverage of streaming responses — a streaming handler *completes* while its +/// decode loop is still running, so the layer can no longer tell an abandoned +/// stream from a healthy one. The contract +/// (`contracts/apr-serve-cancellation-v1.yaml`) states the replacement as a +/// discharged property: "an abandoned STREAM is still stopped, by body-drop +/// rather than by this guard". This is that falsifier. It was asserted and +/// tested by nothing when the guard change shipped. +/// +/// The chain, driven through the REAL router and the real quantized backend: +/// +/// 1. `POST /v1/chat/completions {"stream":true}` with a budget far larger than +/// the 16-slot token channel, so the decode loop is guaranteed to be alive +/// and blocked on a send when the client leaves; +/// 2. the response body is DROPPED without being read — what hyper does to the +/// body of an abandoned request; +/// 3. the SSE receiver drops with it, the next `on_token` send fails, and +/// `streaming_token_sink` records ONE abandonment and returns `false`; +/// 4. `generate_with_cache_streaming` breaks. +/// +/// Both assertions are load-bearing. "at least one abandonment" falsifies +/// step 3 — if dropping the body did not reach the decode loop, no send would +/// ever fail. "EXACTLY one" falsifies step 4 — a loop that keeps generating for +/// a client that left fails every subsequent send too, and records every one of +/// them. +#[tokio::test(flavor = "multi_thread")] +#[cfg(feature = "gpu")] +async fn an_abandoned_stream_is_stopped_by_the_body_drop() { + // > 16 (the channel capacity), so the loop cannot finish before the drop. + const BUDGET: usize = 64; + let request = format!( + r#"{{"model":"default","messages":[{{"role":"user","content":"token5 token6"}}],"max_tokens":{BUDGET},"stream":true}}"# + ); + + // Control FIRST: a stream that is read to the end records NO abandonment, + // so the counter is not simply firing for every stream. + let consumed_state = quantized_state(); + let (status, _, body) = send( + consumed_state.clone(), + "/v1/chat/completions", + &request.clone(), + ) + .await; + assert_eq!(status, StatusCode::OK); + assert!( + !concat_deltas(&body).is_empty(), + "the control stream must carry tokens, or 'nothing was abandoned' is trivial" + ); + assert_eq!( + consumed_state.metrics.streams_abandoned(), + 0, + "a stream the client read to the end was reported as abandoned" + ); + + // Now abandon one. + let state = quantized_state(); + let response = create_router(state.clone()) + .oneshot( + Request::builder() + .method("POST") + .uri("/v1/chat/completions") + .header("content-type", "application/json") + .body(Body::from(request)) + .expect("build request"), + ) + .await + .expect("dispatch"); + assert_eq!(response.status(), StatusCode::OK); + drop(response.into_body()); + + // Step 3: the drop must reach the decode loop. + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + while state.metrics.streams_abandoned() == 0 && std::time::Instant::now() < deadline { + tokio::time::sleep(std::time::Duration::from_millis(5)).await; + } + assert!( + state.metrics.streams_abandoned() >= 1, + "dropping the response body did not stop the decode loop: no failed token \ + send was ever observed, so the loop is still generating for a client that \ + is gone (the guard no longer covers this case — body drop is the ONLY \ + mechanism left)" + ); + + // Step 4: and it must have BROKEN the loop, not merely noticed. A loop still + // running would fail its remaining sends too; this fixture generates a token + // in well under a millisecond, so the whole remaining budget would be spent + // inside this settle window. + tokio::time::sleep(std::time::Duration::from_millis(500)).await; + assert_eq!( + state.metrics.streams_abandoned(), + 1, + "one abandoned stream must produce exactly one abandonment: more than one \ + means the decode loop kept running (and kept failing to send) after the \ + client went away, which is the {BUDGET}-token burn #2376(3) is about" + ); +} + +// --------------------------------------------------------------------------- +// #2375(4) — /v1/chat/completions/stream must serve the standard deployment +// --------------------------------------------------------------------------- + +/// The route is mounted and advertised on every server; it must answer on the +/// one deployment `apr serve run` actually produces. +#[tokio::test] +#[cfg(feature = "gpu")] +async fn chat_completions_stream_route_serves_a_quantized_server() { + let expected = buffered_chat_content(quantized_state(), &format!("{CHAT_REQUEST}}}")).await; + + let (status, content_type, body) = send( + quantized_state(), + "/v1/chat/completions/stream", + &format!("{CHAT_REQUEST}}}"), + ) + .await; + + assert_eq!( + status, + StatusCode::OK, + "0.63.0 answered 404 \"No model available\" here while /v1/chat/completions \ + on the same server returned text: {body}" + ); + assert!( + content_type.starts_with("text/event-stream"), + "the /stream route must always stream, got {content_type:?}" + ); + assert_eq!( + concat_deltas(&body), + expected, + "the dedicated stream route must deliver the same text as the endpoint it \ + is the streaming form of" + ); + assert!( + body.trim_end().ends_with("data: [DONE]"), + "an OpenAI stream terminates with the [DONE] sentinel: {body}" + ); +} + +/// The dedicated route and `"stream":true` are the same endpoint, so they must +/// produce the same wire format. They were two independent implementations. +/// +/// The comparison is only worth making on streams that carry something: two +/// empty streams have equal shapes trivially. As first written this test had no +/// such requirement and stayed GREEN under the guard-re-arm mutation — both +/// sides degraded to the same content-free frame list, so it discriminated the +/// stream-route defect only. It now requires content deltas on both sides +/// FIRST, which is what makes the equality below meaningful. +#[tokio::test] +#[cfg(feature = "gpu")] +async fn stream_route_and_stream_flag_agree_on_the_frame_shape() { + let (_, _, via_route) = send( + quantized_state(), + "/v1/chat/completions/stream", + &format!("{CHAT_REQUEST}}}"), + ) + .await; + let (_, _, via_flag) = send( + quantized_state(), + "/v1/chat/completions", + &format!("{CHAT_REQUEST},\"stream\":true}}"), + ) + .await; + + let shape = |body: &str| -> Vec { + body.lines() + .filter_map(|line| line.strip_prefix("data: ")) + .filter(|payload| payload.trim() != "[DONE]") + .filter_map(|payload| serde_json::from_str::(payload).ok()) + .map(|frame| { + format!( + "{}|{}", + frame["object"].as_str().unwrap_or("?"), + frame["choices"][0]["finish_reason"] + ) + }) + .collect() + }; + + // Both streams must actually carry the reply before their shapes are + // compared. Without this the assertion below is satisfied by two empty + // streams, which is exactly what the guard-re-arm mutation produces. + let route_text = concat_deltas(&via_route); + let flag_text = concat_deltas(&via_flag); + assert!( + !route_text.is_empty(), + "/v1/chat/completions/stream delivered no content deltas, so comparing \ + frame shapes proves nothing; frames were {:?}", + shape(&via_route) + ); + assert!( + !flag_text.is_empty(), + "/v1/chat/completions with stream:true delivered no content deltas, so \ + comparing frame shapes proves nothing; frames were {:?}", + shape(&via_flag) + ); + assert_eq!( + route_text, flag_text, + "the two forms of the same endpoint must carry the same text" + ); + + assert_eq!( + shape(&via_route), + shape(&via_flag), + "the /stream route must not be a second, divergent implementation" + ); +} + +// --------------------------------------------------------------------------- +// temperature: 0 — the canonical deterministic request must be servable +// --------------------------------------------------------------------------- + +/// `temperature: 0` on the dense backend answered +/// `500 {"error":"Invalid shape: Temperature must be a positive finite number"}` +/// on `/v1/chat/completions` and `/v1/completions`, while +/// `/v1/chat/completions/stream` served it — one handler had been fixed +/// (PMAT-790) and the other two kept its private copy of the bug. +#[tokio::test] +async fn temperature_zero_is_served_on_every_openai_route() { + let demo = || AppState::demo().expect("demo AppState"); + for (uri, json) in [ + ( + "/v1/chat/completions", + r#"{"model":"default","messages":[{"role":"user","content":"token5"}],"max_tokens":3,"temperature":0}"#, + ), + ( + "/v1/chat/completions/stream", + r#"{"model":"default","messages":[{"role":"user","content":"token5"}],"max_tokens":3,"temperature":0}"#, + ), + ( + "/v1/completions", + r#"{"model":"default","prompt":"token5","max_tokens":3,"temperature":0}"#, + ), + ] { + let (status, _, body) = send(demo(), uri, json).await; + assert_eq!( + status, + StatusCode::OK, + "temperature:0 is the OpenAI deterministic request; {uri} refused it: {body}" + ); + assert!( + !body.contains("Temperature must be a positive"), + "{uri} leaked the sampler's rejection of its own config: {body}" + ); + } +} + +/// The REST of the temperature domain: an unservable value must be refused as a +/// client error naming the field — never answered `500` with the sampler's own +/// complaint about a config the server built. +/// +/// Fixing `temperature == 0.0` alone left `-1`, `NaN`, `+inf` and any value that +/// narrows to `+inf` as an `f32` reaching `apply_temperature` and producing +/// exactly the body the fix set out to eliminate: +/// `{"error":"Invalid shape: Temperature must be a positive finite number"}`. +/// +/// The cases are chosen for what each one defeats: +/// +/// * `-1` — the plain negative. +/// * `1e40` — finite as JSON and as `f64`, `+inf` once narrowed to `f32`. A +/// guard that checks the parsed `f64` only lets this through. +/// * `1e400` — beyond `f64` entirely. Measured, not assumed: serde_json's own +/// number parser refuses this one before any of our code runs, so the refusal +/// is the generic sanitized body rather than a message naming the field. It is +/// asserted for the class it does prove — client error, no sampler leak. +/// +/// NaN has no JSON literal, so it cannot be sent over the wire at all; it is +/// covered at the resolver instead (`temperature_domain_is_total`), which is +/// where a Rust caller could still produce one. +#[tokio::test] +#[cfg(feature = "gpu")] +async fn unservable_temperature_is_refused_on_every_generating_route() { + // Every route on `create_router_with_config` that accepts a `temperature` + // and can generate — enumerated from the router's own route table, not from + // the routes that were convenient to fix. The two `/api/*` routes take it + // inside `options` and build their `ChatCompletionRequest` in Rust, so a + // guard on that struct alone would not cover them; the three native routes + // validated it only on the QUANTIZED backend, so on a dense server they + // answered `500 "Temperature must be a positive finite number"` (measured, + // then fixed, while writing this test). + let routes: [(&str, &str, &str); 9] = [ + ( + "/v1/chat/completions", + r#"{"model":"default","messages":[{"role":"user","content":"token5"}],"max_tokens":3,"temperature":"#, + "}", + ), + ( + "/v1/chat/completions/stream", + r#"{"model":"default","messages":[{"role":"user","content":"token5"}],"max_tokens":3,"temperature":"#, + "}", + ), + ( + "/v1/completions", + r#"{"model":"default","prompt":"token5","max_tokens":3,"temperature":"#, + "}", + ), + ( + "/v1/batch/completions", + r#"{"prompts":["token5"],"max_tokens":3,"temperature":"#, + "}", + ), + ( + "/api/chat", + r#"{"model":"default","messages":[{"role":"user","content":"token5"}],"options":{"num_predict":3,"temperature":"#, + "}}", + ), + ( + "/api/generate", + r#"{"model":"default","prompt":"token5","options":{"num_predict":3,"temperature":"#, + "}}", + ), + ( + "/generate", + r#"{"prompt":"token5","max_tokens":3,"temperature":"#, + "}", + ), + ( + "/stream/generate", + r#"{"prompt":"token5","max_tokens":3,"temperature":"#, + "}", + ), + ( + "/batch/generate", + r#"{"prompts":["token5"],"max_tokens":3,"temperature":"#, + "}", + ), + ]; + + for (uri, head, tail) in routes { + // `names_the_field` is false only for the value serde_json refuses before + // our guard is reached (see the doc comment). + for (unservable, names_the_field) in [("-1", true), ("1e40", true), ("1e400", false)] { + let (status, _, body) = + send(quantized_state(), uri, &format!("{head}{unservable}{tail}")).await; + + assert!( + status.is_client_error(), + "{uri} with temperature {unservable} must be refused as a client error, \ + got {status}: {body}" + ); + if names_the_field { + assert!( + body.contains("temperature"), + "{uri} refused temperature {unservable} without saying which field \ + was wrong: {body}" + ); + } + assert!( + !body.contains("Temperature must be a positive"), + "{uri} leaked the sampler's rejection of a config the SERVER built from \ + temperature {unservable}: {body}" + ); + } + + // Positive control on the SAME route and the same server: a servable + // temperature is still answered, so the rejections above are about the + // value and not about the route being broken. + let (status, _, body) = send(quantized_state(), uri, &format!("{head}0.7{tail}")).await; + assert!( + !status.is_client_error(), + "{uri} refused a servable temperature of 0.7: {status} {body}" + ); + } +} + +/// The resolver itself must be TOTAL: no `f32` may produce a config that +/// `sample_token` rejects. This is the half of the domain a client cannot reach +/// (NaN has no JSON literal) and a Rust caller can. +#[test] +fn temperature_domain_is_total() { + use crate::api::realize_handlers::resolve_dense_generation_config; + use crate::generate::{sample_token, SamplingStrategy}; + use crate::tensor::Tensor; + + let logits = Tensor::from_vec(vec![4], vec![0.1, 0.2, 0.9, 0.3]).expect("tensor"); + + for temperature in [ + 0.0_f32, + -1.0, + -0.0, + f32::NAN, + f32::INFINITY, + f32::NEG_INFINITY, + f32::MIN, + ] { + let config = resolve_dense_generation_config(temperature, Some(0.9), 16); + assert_eq!( + config.strategy, + SamplingStrategy::Greedy, + "temperature {temperature} is not a positive finite scale, so it must \ + resolve to deterministic decoding" + ); + let token = sample_token(&logits, &config, 0.5).unwrap_or_else(|e| { + panic!( + "temperature {temperature} produced a config the sampler rejects \ + (this is the HTTP 500): {e:?}" + ) + }); + assert_eq!(token, 2, "greedy must select the argmax token"); + } + + // The converse: a servable temperature is NOT rewritten to greedy, or this + // test would pass on a resolver that ignores its argument. + let config = resolve_dense_generation_config(0.7, Some(0.9), 16); + assert_eq!( + config.strategy, + SamplingStrategy::TopP { p: 0.9 }, + "a positive finite temperature with top_p must still resolve to nucleus sampling" + ); + assert!( + (config.temperature - 0.7).abs() < 1e-6, + "a servable temperature must reach the sampler unchanged, got {}", + config.temperature + ); +} + +// --------------------------------------------------------------------------- +// #2375(7) — /v1/metrics must report measurements +// --------------------------------------------------------------------------- + +/// Drive real traffic through one shared state, then compare the two metrics +/// endpoints on that same state at the same instant. 0.63.0 reported +/// `latency_p50_ms 0.0` next to `realizar_avg_latency_ms 626.79`. +#[tokio::test] +#[cfg(feature = "gpu")] +async fn v1_metrics_percentiles_are_measured_alongside_a_nonzero_average() { + let state = quantized_state(); + for _ in 0..5 { + let (status, _, body) = send( + state.clone(), + "/v1/chat/completions", + &format!("{CHAT_REQUEST}}}"), + ) + .await; + assert_eq!(status, StatusCode::OK, "traffic must succeed: {body}"); + } + + let (status, prometheus) = get(state.clone(), "/metrics").await; + assert_eq!(status, StatusCode::OK); + let avg_latency_ms: f64 = prometheus + .lines() + .find_map(|line| line.strip_prefix("realizar_avg_latency_ms ")) + .and_then(|v| v.trim().parse().ok()) + .expect("/metrics must expose realizar_avg_latency_ms"); + assert!( + avg_latency_ms > 0.0, + "the control is broken: five completed requests took no measurable time" + ); + + let (status, body) = get(state, "/v1/metrics").await; + assert_eq!(status, StatusCode::OK); + let metrics: serde_json::Value = serde_json::from_str(&body).expect("/v1/metrics is JSON"); + let p50 = metrics["latency_p50_ms"].as_f64().expect("p50 present"); + let p95 = metrics["latency_p95_ms"].as_f64().expect("p95 present"); + let p99 = metrics["latency_p99_ms"].as_f64().expect("p99 present"); + + assert!( + p50 > 0.0, + "/v1/metrics reported p50 {p50} ms while /metrics on the SAME state reported \ + an average of {avg_latency_ms} ms over the same requests" + ); + assert!( + p50 <= p95 && p95 <= p99, + "percentiles must be non-decreasing: p50={p50} p95={p95} p99={p99}" + ); + // The percentiles and the mean describe the same five requests, so they must + // be the same order of magnitude. This excludes a p50 sourced from some + // other collector, which is how the 0.0 was reported next to a 626.79. + assert!( + p50 >= avg_latency_ms / 10.0 && p50 <= avg_latency_ms * 10.0, + "p50 {p50} ms is not consistent with the mean {avg_latency_ms} ms of the same traffic" + ); +} + +/// `model_name` must name the model THIS server loaded. It was the literal +/// `"phi-2-q4_k_m"` for any cached GPU model and `"N/A"` for everything else. +/// +/// Two servers, differing only in the model they were pointed at, must report +/// two different names — each its own file stem. That is what excludes a +/// constant; "not `N/A`" does not. The first version of this falsifier asserted +/// only `!= "N/A"` and `!= "phi-2-q4_k_m"` against a fixture that set no model +/// source at all, so it passed on the value `"default"` — another constant, out +/// of the fallback arm — and the deriving branch it is named for +/// (`served_model_name`'s `file_stem`) was executed by no test in the suite. +#[tokio::test] +#[cfg(feature = "gpu")] +async fn v1_metrics_model_name_is_derived_from_the_model_this_server_loaded() { + use crate::api::ModelSourceInfo; + + async fn reported_name(dir: &std::path::Path, file_name: &str) -> String { + let path = dir.join(file_name); + // A real file, so `from_path` measures it the way `apr serve run` does + // (size and format from the bytes, not from the extension). + std::fs::write(&path, b"GGUF\0\0\0\0not-a-real-model").expect("write fixture model"); + let state = quantized_state().with_model_source(ModelSourceInfo::from_path(&path)); + + let (status, body) = get(state, "/v1/metrics").await; + assert_eq!(status, StatusCode::OK); + let metrics: serde_json::Value = serde_json::from_str(&body).expect("/v1/metrics is JSON"); + metrics["model_name"] + .as_str() + .expect("model_name present") + .to_string() + } + + let dir = tempfile::tempdir().expect("tempdir"); + let first = reported_name(dir.path(), "albor-370m-v1-q4_k_m.gguf").await; + let second = reported_name(dir.path(), "qwen2.5-coder-1.5b-instruct-q4k.gguf").await; + + assert_eq!( + first, "albor-370m-v1-q4_k_m", + "/v1/metrics must report the stem of the model file this server loaded" + ); + assert_eq!( + second, "qwen2.5-coder-1.5b-instruct-q4k", + "a second server on a different model must report ITS model, not the first one's" + ); + assert_ne!( + first, second, + "two servers serving two different models reported the same name, which no \ + derivation can do and every constant does" + ); + + // The fallback arm, pinned by name so it can never again pass as "derived": + // a resident model with no source path is reported as the id `/v1/models` + // advertises in single-model mode. + let (status, body) = get(quantized_state(), "/v1/metrics").await; + assert_eq!(status, StatusCode::OK); + let metrics: serde_json::Value = serde_json::from_str(&body).expect("/v1/metrics is JSON"); + assert_eq!( + metrics["model_name"], "default", + "with a model resident but no source path, the reported name is the id a \ + client may send back as \"model\" — not \"N/A\", and not a constant naming \ + some other model" + ); +} + +/// A server with NO model must not invent a name for one. +#[tokio::test] +async fn v1_metrics_reports_no_model_name_when_nothing_is_loaded() { + let (status, body) = get(AppState::demo_mock().expect("mock state"), "/v1/metrics").await; + assert_eq!(status, StatusCode::OK); + let metrics: serde_json::Value = serde_json::from_str(&body).expect("/v1/metrics is JSON"); + assert_eq!( + metrics["model_name"], "N/A", + "no model is loaded, so there is no name to report" + ); + assert_eq!( + metrics["latency_p50_ms"], 0.0, + "no request has completed, so there is no latency to report" + ); +} diff --git a/crates/aprender-serve/src/api/types.rs b/crates/aprender-serve/src/api/types.rs index 86ef321977..1475a33a76 100644 --- a/crates/aprender-serve/src/api/types.rs +++ b/crates/aprender-serve/src/api/types.rs @@ -58,8 +58,17 @@ pub struct GenerateRequest { /// Maximum tokens to generate #[serde(default = "default_max_tokens")] pub max_tokens: usize, - /// Sampling temperature - #[serde(default = "default_temperature")] + /// Sampling temperature. + /// + /// Rejected at deserialization when outside `[0, ∞)` finite (aprender#2375), + /// so it cannot reach a backend that does not validate it: only the + /// QUANTIZED path ran `resolve_quantized_sampling`, and on a dense server + /// `/generate`, `/stream/generate` and `/batch/generate` all answered + /// `500 "Temperature must be a positive finite number"`. + #[serde( + default = "default_temperature", + deserialize_with = "deserialize_temperature_f32_required" + )] pub temperature: f32, /// Sampling strategy: "greedy", "`top_k`", or "`top_p`" #[serde(default = "default_strategy")] @@ -134,8 +143,13 @@ pub struct BatchGenerateRequest { /// Maximum tokens to generate (shared across all prompts) #[serde(default = "default_max_tokens")] pub max_tokens: usize, - /// Sampling temperature (shared) - #[serde(default = "default_temperature")] + /// Sampling temperature (shared). + /// + /// Rejected at deserialization when outside `[0, ∞)` finite (aprender#2375). + #[serde( + default = "default_temperature", + deserialize_with = "deserialize_temperature_f32_required" + )] pub temperature: f32, /// Sampling strategy (shared) #[serde(default = "default_strategy")] @@ -281,3 +295,110 @@ impl<'de> Deserialize<'de> for ChoiceCount { } } } + +// --------------------------------------------------------------------------- +// Temperature: the servable domain, enforced where the request is parsed +// --------------------------------------------------------------------------- + +/// Is this temperature one the samplers can honour? +/// +/// The servable domain is `[0, ∞)` **finite**. `0` is the OpenAI-canonical +/// deterministic request and resolves to greedy; every positive finite value +/// scales the logits. +/// +/// Everything else is unservable, and each mode fails differently, which is why +/// none of them may reach a sampler: +/// +/// * **negative** — `logit / -t` inverts the distribution, so the dense path +/// answered HTTP 500 (`apply_temperature`) and the quantized path served the +/// model's LEAST likely tokens with a 200. +/// * **NaN** — every comparison against NaN is false, so a `t < 0.0` guard does +/// not catch it (aprender#2391 is an entire issue about exactly that). It +/// poisons every logit and the cumulative draw never fires. +/// * **±∞** — `t.is_nan() || t < 0.0` misses `+inf` as well; the dense sampler +/// rejects it (500) and the quantized one flattens every logit to 0. +/// +/// `is_finite()` is what carries the NaN and ±∞ cases: a comparison-only guard +/// such as `t < 0.0` is FALSE for NaN and lets it straight through. +#[must_use] +pub(crate) fn temperature_is_servable(temperature: f64) -> bool { + temperature.is_finite() && temperature >= 0.0 +} + +/// The client-visible refusal for an unservable temperature. +fn temperature_rejection(temperature: f64) -> E { + E::custom(format!( + "{}temperature must be a finite number >= 0 (0 means deterministic/greedy), \ + got {temperature}", + crate::api::CLIENT_VISIBLE_MARKER + )) +} + +/// `deserialize_with` for an `Option` temperature field. +/// +/// aprender#2375: `temperature: 0` was fixed by hand in the handlers and the +/// REST of the domain was left reaching `apply_temperature`, which answers +/// `500 {"error":"Invalid shape: Temperature must be a positive finite number"}` +/// — the very body that fix set out to eliminate. Refusing here makes an +/// unservable temperature unrepresentable in a deserialized request, exactly as +/// [`ChoiceCount`] does for `n > 1`, so no handler and no backend can be the one +/// that forgot. +/// +/// The `f64 -> f32` narrowing is part of the check: `1e40` is a perfectly finite +/// `f64` that becomes `+inf` as an `f32`, and that infinity is what would reach +/// the sampler. +/// +/// # Errors +/// +/// Rejects a temperature outside the servable domain — see +/// [`temperature_is_servable`]. +pub(crate) fn deserialize_temperature_f32<'de, D>(deserializer: D) -> Result, D::Error> +where + D: serde::Deserializer<'de>, +{ + let Some(raw) = Option::::deserialize(deserializer)? else { + return Ok(None); + }; + let narrowed = f64::from(raw as f32); + if !temperature_is_servable(raw) || !temperature_is_servable(narrowed) { + return Err(temperature_rejection(raw)); + } + Ok(Some(raw as f32)) +} + +/// `deserialize_with` for an `Option` temperature field. +/// +/// Same rule as [`deserialize_temperature_f32`]; the value still narrows to +/// `f32` before it reaches a sampler, so the narrowing is checked here too. +/// +/// # Errors +/// +/// Rejects a temperature outside the servable domain. +pub(crate) fn deserialize_temperature_f64<'de, D>(deserializer: D) -> Result, D::Error> +where + D: serde::Deserializer<'de>, +{ + Ok(deserialize_temperature_f32(deserializer)?.map(f64::from)) +} + +/// `deserialize_with` for a non-`Option` `f32` temperature field. +/// +/// A MISSING field never reaches here — serde's `default` attribute answers it — +/// so this sees only values the client actually sent. An explicit `null` is +/// refused rather than silently becoming a temperature the client did not +/// choose, which is what `f32::deserialize` did before this guard existed. +/// +/// # Errors +/// +/// Rejects `null` and any temperature outside the servable domain. +pub(crate) fn deserialize_temperature_f32_required<'de, D>(deserializer: D) -> Result +where + D: serde::Deserializer<'de>, +{ + deserialize_temperature_f32(deserializer)?.ok_or_else(|| { + serde::de::Error::custom(format!( + "{}temperature must be a finite number >= 0, not null", + crate::api::CLIENT_VISIBLE_MARKER + )) + }) +} diff --git a/crates/aprender-serve/src/generate/cancel.rs b/crates/aprender-serve/src/generate/cancel.rs index 6b362908e6..5ae04476a3 100644 --- a/crates/aprender-serve/src/generate/cancel.rs +++ b/crates/aprender-serve/src/generate/cancel.rs @@ -144,18 +144,38 @@ impl CancelToken { pub fn cancel_on_drop(&self) -> CancelOnDrop { CancelOnDrop { token: self.clone(), + armed: true, } } } -/// Cancels its [`CancelToken`] on drop. +/// Cancels its [`CancelToken`] on drop, unless [`CancelOnDrop::disarm`] ran first. /// -/// Firing after generation already finished is harmless — the loop is gone and the -/// flag is read by nobody — so the guard is deliberately not disarmable. That keeps -/// the disconnect path from depending on a "did we remember to disarm" branch. +/// # Why it has to be disarmable +/// +/// The original version fired unconditionally, on the reasoning that "firing after +/// generation already finished is harmless — the loop is gone and the flag is read +/// by nobody". That is true for a handler that generates *inside* its own future, +/// and false for every streaming handler: those hand the decode loop to a +/// background task and RETURN the SSE response immediately, so the middleware +/// future completes while generation is still starting. An unconditional guard +/// therefore cancelled the loop before its first token, and +/// `POST /v1/chat/completions` with `"stream":true` answered +/// `text/event-stream` carrying an opening chunk, a terminal chunk and **no +/// content deltas at all**. +/// +/// So the guard now distinguishes the two exits it always had: +/// - dropped while the future is still running (the client went away) → cancel; +/// - disarmed by a future that ran to completion → do nothing. +/// +/// A streaming request abandoned mid-body is still cancelled, by a different and +/// pre-existing mechanism: hyper drops the response body, which drops the SSE +/// receiver, which makes the generator's `on_token` send fail, which breaks the +/// decode loop. #[derive(Debug)] pub struct CancelOnDrop { token: CancelToken, + armed: bool, } impl CancelOnDrop { @@ -164,11 +184,22 @@ impl CancelOnDrop { pub fn token(&self) -> &CancelToken { &self.token } + + /// Give up the right to cancel: this guard's drop becomes a no-op. + /// + /// Call it on the path where the work this guard protects has *completed* or + /// been handed to something else that can stop it. Anything else (an early + /// return, a panic, a dropped future) leaves the guard armed. + pub fn disarm(&mut self) { + self.armed = false; + } } impl Drop for CancelOnDrop { fn drop(&mut self) { - self.token.cancel(); + if self.armed { + self.token.cancel(); + } } } @@ -270,4 +301,33 @@ mod tests { drop(guard); assert!(t.peek_cancelled()); } + + /// aprender#2375(1): a disarmed guard must NOT cancel. The streaming handlers + /// return their response while the decode loop is still starting, so a guard + /// that fired on normal completion killed the generation before its first + /// token and the SSE body carried no content deltas. + #[test] + fn disarmed_guard_does_not_cancel_on_drop() { + let t = CancelToken::new(); + { + let mut guard = t.cancel_on_drop(); + guard.disarm(); + } + assert!( + !t.peek_cancelled(), + "a disarmed guard must leave the token alive: work handed to a \ + background task outlives the future that armed the guard" + ); + } + + /// The guard is armed by default, so forgetting to disarm keeps the + /// disconnect behaviour rather than silently losing it. + #[test] + fn guard_is_armed_until_disarmed() { + let t = CancelToken::new(); + let mut guard = t.cancel_on_drop(); + drop(t.cancel_on_drop()); // a second, still-armed guard cancels + assert!(t.peek_cancelled()); + guard.disarm(); + } } diff --git a/crates/aprender-serve/src/metrics.rs b/crates/aprender-serve/src/metrics.rs index a52468ac56..0d657af42b 100644 --- a/crates/aprender-serve/src/metrics.rs +++ b/crates/aprender-serve/src/metrics.rs @@ -9,13 +9,40 @@ //! Metrics are exposed in Prometheus format for easy integration with monitoring systems. use std::{ + collections::VecDeque, sync::{ atomic::{AtomicU64, AtomicUsize, Ordering}, - Arc, + Arc, Mutex, }, time::{Duration, Instant}, }; +/// How many recent request latencies are kept for percentile reporting. +/// +/// A bounded window: memory is constant regardless of uptime, and the reported +/// percentiles describe recent behaviour rather than the whole life of the +/// process — which is what a monitor graphing p95 wants. +const LATENCY_WINDOW: usize = 1024; + +/// Measured request-latency percentiles, in milliseconds. +/// +/// aprender#2375(7): `/v1/metrics` reported `latency_p50/p95/p99 = 0.0` while +/// `/metrics` on the same process at the same instant reported +/// `avg_latency_ms 626.79` over the same 27 requests — the collector kept only +/// running totals, so there was no distribution to take a percentile of. The +/// non-GPU variant of the handler filled the gap by *deriving* p95 as +/// `avg * 1.5` and p99 as `avg * 2.0`, which are not measurements of anything. +/// These are order statistics over the real samples. +#[derive(Debug, Clone, Copy, PartialEq)] +pub struct LatencyPercentiles { + /// Median request latency (ms). + pub p50_ms: f64, + /// 95th-percentile request latency (ms). + pub p95_ms: f64, + /// 99th-percentile request latency (ms). + pub p99_ms: f64, +} + /// Central metrics collector for tracking system performance #[derive(Debug, Clone)] pub struct MetricsCollector { @@ -29,6 +56,12 @@ pub struct MetricsCollector { total_tokens: Arc, /// Total inference time in microseconds total_inference_time_us: Arc, + /// Most recent per-request latencies in microseconds (oldest first), capped + /// at [`LATENCY_WINDOW`]. This is the sample set the percentiles come from. + recent_latencies_us: Arc>>, + /// Streams whose client went away mid-generation — see + /// [`MetricsCollector::record_stream_abandoned`]. + abandoned_streams: Arc, /// Start time for rate calculations start_time: Instant, } @@ -43,18 +76,92 @@ impl MetricsCollector { failed_requests: Arc::new(AtomicUsize::new(0)), total_tokens: Arc::new(AtomicUsize::new(0)), total_inference_time_us: Arc::new(AtomicU64::new(0)), + recent_latencies_us: Arc::new(Mutex::new(VecDeque::with_capacity(LATENCY_WINDOW))), + abandoned_streams: Arc::new(AtomicUsize::new(0)), start_time: Instant::now(), } } + /// Record a streaming response whose client went away before the generation + /// finished, and which was therefore stopped early. + /// + /// aprender#2375(1)/#2376(3): the cancellation guard deliberately does NOT + /// fire for a request that completed, because a streaming handler returns + /// its SSE response while the decode loop is still running — cancelling + /// there emptied every streamed reply. What stops an ABANDONED stream + /// instead is the body drop: hyper drops the response body, the SSE receiver + /// drops with it, the next `on_token` send fails, and the loop breaks. + /// + /// That replacement mechanism was asserted in the contract and observed by + /// nothing. This counter is what makes it observable: one record per + /// abandoned stream. More than one for a single stream means the loop kept + /// running after the client left — the defect the mechanism exists to + /// prevent — and zero means the drop never reached the decode loop at all. + pub fn record_stream_abandoned(&self) { + self.abandoned_streams.fetch_add(1, Ordering::Relaxed); + } + + /// How many streams were stopped because their client went away. + #[must_use] + pub fn streams_abandoned(&self) -> usize { + self.abandoned_streams.load(Ordering::Relaxed) + } + /// Record a successful request #[allow(clippy::cast_possible_truncation)] pub fn record_success(&self, tokens: usize, duration: Duration) { self.total_requests.fetch_add(1, Ordering::Relaxed); self.successful_requests.fetch_add(1, Ordering::Relaxed); self.total_tokens.fetch_add(tokens, Ordering::Relaxed); + let elapsed_us = duration.as_micros() as u64; self.total_inference_time_us - .fetch_add(duration.as_micros() as u64, Ordering::Relaxed); + .fetch_add(elapsed_us, Ordering::Relaxed); + // aprender#2375(7): keep the SAMPLE, not just the sum. Without it there + // is no distribution, and the percentile fields could only be zeros or + // multiples of the mean. + if let Ok(mut samples) = self.recent_latencies_us.lock() { + if samples.len() == LATENCY_WINDOW { + samples.pop_front(); + } + samples.push_back(elapsed_us); + } + } + + /// Percentiles over the recent request-latency window. + /// + /// `None` when no successful request has been recorded — an honest "not + /// measured yet", which the caller must not render as a measured `0.0`. + /// + /// Nearest-rank order statistics (no interpolation): with `n` samples the + /// q-th percentile is the `ceil(q*n)`-th smallest, so `p95` of 100 samples + /// is the 95th smallest and can never be a scaled mean. + #[must_use] + #[allow( + clippy::cast_precision_loss, + clippy::cast_sign_loss, + clippy::cast_possible_truncation + )] + pub fn latency_percentiles(&self) -> Option { + let mut samples: Vec = { + let guard = self.recent_latencies_us.lock().ok()?; + if guard.is_empty() { + return None; + } + guard.iter().copied().collect() + }; + samples.sort_unstable(); + + let nth = |quantile: f64| -> f64 { + let n = samples.len(); + let rank = (quantile * n as f64).ceil().max(1.0) as usize; + let index = rank.min(n) - 1; + samples[index] as f64 / 1000.0 + }; + Some(LatencyPercentiles { + p50_ms: nth(0.50), + p95_ms: nth(0.95), + p99_ms: nth(0.99), + }) } /// Record a failed request @@ -139,7 +246,10 @@ impl MetricsCollector { realizar_error_rate {:.4}\n\ # HELP realizar_uptime_seconds Uptime in seconds\n\ # TYPE realizar_uptime_seconds counter\n\ - realizar_uptime_seconds {}\n", + realizar_uptime_seconds {}\n\ + # HELP realizar_streams_abandoned Streams stopped because the client went away\n\ + # TYPE realizar_streams_abandoned counter\n\ + realizar_streams_abandoned {}\n", snapshot.total_requests, snapshot.successful_requests, snapshot.failed_requests, @@ -149,7 +259,8 @@ impl MetricsCollector { snapshot.tokens_per_sec, snapshot.avg_latency_ms, snapshot.error_rate, - snapshot.uptime_secs + snapshot.uptime_secs, + self.streams_abandoned() ) } @@ -160,6 +271,13 @@ impl MetricsCollector { self.failed_requests.store(0, Ordering::Relaxed); self.total_tokens.store(0, Ordering::Relaxed); self.total_inference_time_us.store(0, Ordering::Relaxed); + self.abandoned_streams.store(0, Ordering::Relaxed); + // The latency window is part of "all metrics": leaving it behind would + // let a reset collector report percentiles for traffic it no longer + // counts. + if let Ok(mut samples) = self.recent_latencies_us.lock() { + samples.clear(); + } } } @@ -348,4 +466,114 @@ mod tests { approx::assert_relative_eq!(snapshot.avg_latency_ms, 0.0); approx::assert_relative_eq!(snapshot.error_rate, 0.0); } + + // ----------------------------------------------------------------------- + // aprender#2375(7): the percentiles must be order statistics of the real + // samples — not zeros, and not multiples of the mean. + // ----------------------------------------------------------------------- + + /// The falsifier. 100 requests at 1..=100 ms have mean 50.5 ms, so the + /// shipped formulas produce p50 = 50.5, p95 = 75.75 (`avg * 1.5`) and + /// p99 = 101.0 (`avg * 2.0`). The measured order statistics are 50, 95 and + /// 99 — a value `avg * k` cannot equal for both p95 and p99. + #[test] + fn percentiles_are_order_statistics_not_multiples_of_the_mean() { + let metrics = MetricsCollector::new(); + for ms in 1..=100u64 { + metrics.record_success(1, Duration::from_millis(ms)); + } + + let p = metrics + .latency_percentiles() + .expect("100 recorded requests must produce percentiles"); + + approx::assert_relative_eq!(p.p50_ms, 50.0, epsilon = 1e-6); + approx::assert_relative_eq!(p.p95_ms, 95.0, epsilon = 1e-6); + approx::assert_relative_eq!(p.p99_ms, 99.0, epsilon = 1e-6); + + // Spell the shipped fabrication out so a regression names itself. + let avg = metrics.snapshot().avg_latency_ms; + assert!( + (p.p95_ms - avg * 1.5).abs() > 1.0, + "p95 must be measured, not derived as avg*1.5 ({avg} -> {})", + avg * 1.5 + ); + assert!( + (p.p99_ms - avg * 2.0).abs() > 1.0, + "p99 must be measured, not derived as avg*2.0 ({avg} -> {})", + avg * 2.0 + ); + } + + /// Order is a property of percentiles, whatever the distribution. + #[test] + fn percentiles_are_monotonic() { + let metrics = MetricsCollector::new(); + for ms in [5u64, 900, 12, 7, 350, 8, 9, 11, 6, 10] { + metrics.record_success(1, Duration::from_millis(ms)); + } + let p = metrics.latency_percentiles().expect("samples recorded"); + assert!( + p.p50_ms <= p.p95_ms && p.p95_ms <= p.p99_ms, + "percentiles must be non-decreasing: {p:?}" + ); + assert!( + p.p99_ms >= 350.0, + "the tail must reach the slow requests, or the window is dropping them: {p:?}" + ); + } + + /// No traffic is reported as "no measurement", never as a measured 0 ms. + #[test] + fn no_samples_reports_absence_not_zero() { + let metrics = MetricsCollector::new(); + assert!( + metrics.latency_percentiles().is_none(), + "a collector with no successful request has nothing to take a percentile of" + ); + metrics.record_failure(); + assert!( + metrics.latency_percentiles().is_none(), + "a failed request carries no latency sample" + ); + } + + /// The window is bounded, and keeps the RECENT samples. + #[test] + fn window_is_bounded_and_keeps_the_newest_samples() { + let metrics = MetricsCollector::new(); + // Fill past capacity with slow requests, then push a full window of fast + // ones. The slow ones must have aged out entirely. + for _ in 0..LATENCY_WINDOW { + metrics.record_success(1, Duration::from_millis(500)); + } + for _ in 0..LATENCY_WINDOW { + metrics.record_success(1, Duration::from_millis(1)); + } + let p = metrics.latency_percentiles().expect("samples recorded"); + approx::assert_relative_eq!(p.p99_ms, 1.0, epsilon = 1e-6); + + let held = metrics + .recent_latencies_us + .lock() + .expect("window lock") + .len(); + assert_eq!( + held, LATENCY_WINDOW, + "the window must stay bounded regardless of uptime" + ); + } + + #[test] + fn reset_clears_the_latency_window() { + let metrics = MetricsCollector::new(); + metrics.record_success(1, Duration::from_millis(42)); + assert!(metrics.latency_percentiles().is_some()); + metrics.reset(); + assert!( + metrics.latency_percentiles().is_none(), + "reset must drop the samples too, or percentiles describe traffic the \ + counters no longer count" + ); + } } diff --git a/scripts/check_wasm32_core_builds.sh b/scripts/check_wasm32_core_builds.sh new file mode 100755 index 0000000000..74e40bc8b8 --- /dev/null +++ b/scripts/check_wasm32_core_builds.sh @@ -0,0 +1,159 @@ +#!/usr/bin/env bash +# check_wasm32_core_builds.sh - aprender-core must compile for a 32-bit target. +# +# THE CLASS (aprender#2310). Code written and only ever compiled on x86_64 picks +# up 64-bit assumptions silently. The SGD epoch shuffle in +# crates/aprender-core/src/classification/mod.rs spelled the MMIX LCG constants +# as bare literals in a usize expression: +# +# let j = (seed * 6364136223846793005 + i * 1442695040888963407) % (i + 1); +# +# On wasm32-unknown-unknown usize is 32 bits, so that is a hard compile error +# ("literal out of range", deny-by-default overflowing_literals) - four of them, +# and aprender-core simply does not build. Nothing in the tree ever compiled for +# a 32-bit target, so it shipped in v0.60.0 and was reported from outside. +# +# This guard runs the reporter's exact command. The getrandom_backend cfg is a +# real precondition, not decoration: getrandom 0.3 refuses +# wasm32-unknown-unknown by default, and the application (not this library) +# supplies __getrandom_v03_custom. Without the flag the build dies inside +# getrandom before it ever reaches aprender-core, which would make this check +# report a failure that has nothing to do with our code. +# +# Exit 0 = aprender-core type-checks for wasm32-unknown-unknown. +# Exit 1 = it does not, or the check could not be run (fail closed - a guard that +# cannot run must never be mistaken for a guard that passed). +# +# --self-test proves the check can still turn RED: it feeds the toolchain the +# exact #2310 literal and requires a rejection. If a future toolchain ever +# demotes overflowing_literals to a warning, or the "wasm32" target quietly +# becomes 64-bit, the self-test fails instead of this guard passing vacuously. + +set -euo pipefail + +TARGET="wasm32-unknown-unknown" +PKG="aprender-core" +# One of the two MMIX constants from #2310. Kept as data so the self-test and +# the comment above cannot drift apart. +DEFECT_LITERAL="6364136223846793005" +# The compiler's wording for the #2310 rejection. No backticks: they make the +# pattern ambiguous to shell linters and to anyone quoting this line. +DEFECT_DIAGNOSTIC="literal out of range" + +SELF_TEST_DIR="" +CHECK_LOG="" + +cleanup() { + if [ -n "${SELF_TEST_DIR}" ] && [ -d "${SELF_TEST_DIR}" ]; then + rm -rf "${SELF_TEST_DIR}" + fi + if [ -n "${CHECK_LOG}" ] && [ -f "${CHECK_LOG}" ]; then + rm -f "${CHECK_LOG}" + fi +} +trap cleanup EXIT + +ensure_target() { + if ! command -v rustup > /dev/null 2>&1; then + echo "FAIL: rustup not found; cannot prove ${TARGET} is available." >&2 + echo " Install rustup, or run the cargo command in this file by hand." >&2 + return 1 + fi + if rustup target list --installed 2> /dev/null | grep -qx "${TARGET}"; then + return 0 + fi + echo "note: ${TARGET} not installed; adding it." + if ! rustup target add "${TARGET}" > /dev/null 2>&1; then + echo "FAIL: could not install the ${TARGET} std component." >&2 + return 1 + fi + return 0 +} + +# Prove the toolchain still rejects the #2310 defect on this target. Without +# this, a green run of the main check could mean "the guard is toothless". +self_test() { + local rc + SELF_TEST_DIR="$(mktemp -d)" + + echo "pub const DEFECT: usize = ${DEFECT_LITERAL};" > "${SELF_TEST_DIR}/defect.rs" + + set +e + rustc --target "${TARGET}" --crate-type lib --emit=metadata \ + -o "${SELF_TEST_DIR}/defect.rmeta" "${SELF_TEST_DIR}/defect.rs" \ + > "${SELF_TEST_DIR}/defect.log" 2>&1 + rc=$? + set -e + + if [ "${rc}" -eq 0 ]; then + echo "SELF-TEST FAIL: rustc ACCEPTED ${DEFECT_LITERAL} as a usize on ${TARGET}." >&2 + echo " This guard can no longer detect the #2310 defect class." >&2 + return 1 + fi + if ! grep -q "${DEFECT_DIAGNOSTIC}" "${SELF_TEST_DIR}/defect.log"; then + echo "SELF-TEST FAIL: rustc rejected the defect, but not for the #2310 reason." >&2 + echo " Expected: ${DEFECT_DIAGNOSTIC}. Got:" >&2 + cat "${SELF_TEST_DIR}/defect.log" >&2 + return 1 + fi + echo "SELF-TEST PASS: ${TARGET} still rejects the #2310 literal as a usize." + return 0 +} + +main_check() { + local rc + CHECK_LOG="$(mktemp)" + + echo "Type-checking ${PKG} for ${TARGET} (aprender#2310)..." + # Append rather than overwrite: CI images bake their own RUSTFLAGS, and + # clobbering them would silently change what is being compiled. + export RUSTFLAGS="${RUSTFLAGS:-} --cfg getrandom_backend=\"custom\"" + + # Never read the status through a pipe - redirect, then read rc. + set +e + cargo check --locked -p "${PKG}" --no-default-features --target "${TARGET}" \ + > "${CHECK_LOG}" 2>&1 + rc=$? + set -e + + if [ "${rc}" -ne 0 ]; then + echo "FAIL: ${PKG} does not compile for ${TARGET} (exit ${rc})." >&2 + echo "----- errors -----" >&2 + grep -E "^error" -A 6 "${CHECK_LOG}" >&2 || cat "${CHECK_LOG}" >&2 + echo "------------------" >&2 + return 1 + fi + + echo "PASS: ${PKG} type-checks for ${TARGET}." + return 0 +} + +usage() { + echo "usage: $0 [--self-test]" + echo " (no args) self-test, then type-check ${PKG} for ${TARGET}" + echo " --self-test only prove the toolchain still rejects the #2310 literal" +} + +# cargo must run at the workspace root regardless of the caller's cwd. +REPO_ROOT="$(git rev-parse --show-toplevel 2> /dev/null || pwd)" +cd "${REPO_ROOT}" || exit 1 + +case "${1:-}" in + --self-test) + ensure_target || exit 1 + self_test || exit 1 + ;; + -h | --help) + usage + ;; + "") + ensure_target || exit 1 + self_test || exit 1 + main_check || exit 1 + ;; + *) + echo "unknown argument: ${1}" >&2 + usage >&2 + exit 2 + ;; +esac