diff --git a/.agents/engine-matrix.md b/.agents/engine-matrix.md index d1b668b78..4883a11fd 100644 --- a/.agents/engine-matrix.md +++ b/.agents/engine-matrix.md @@ -112,7 +112,7 @@ lifecycle are unchanged. | `KV-MLA-SPEC` | Latent MLA KV specification | T2 | `vllm/v1/kv_cache_interface.py:363` | - | - | `planned: specs/mla-kv-spec.md` | `INVENTORIED` | - | | `KV-CROSS-ENCODER-SPECS` | `CrossAttentionSpec` and `EncoderOnlyAttentionSpec` KV interface specs (`ATTN-ENCODER-CROSS` covers backends only); carried from porting-inventory §2 (T2) at the v1 fold | T2 | `vllm/v1/kv_cache_interface.py:710,717` | - | - | `planned: specs/encoder-cross-kv-specs.md` | `INVENTORIED` | - | | `KV-SIZING` | GPU memory utilization and block-count overrides | T0 | `vllm/config/cache.py:68,87,168`; `tests/v1/core/test_kv_cache_utils.py:2224,2303` | fixed inputs `src/vllm/entrypoints/model_loader.cpp:117,129`; watermark `src/vllm/v1/core/kv_cache_manager.cpp:118` | watermark only `tests/vllm/v1/test_kv_cache_manager.cpp:298` | `planned: specs/kv-sizing.md` | `PARTIAL` | - | -| `KV-WARMUP-PROFILE` | Dummy runs, warmup, and startup memory profiling that derive the KV budget (`KV-SIZING` covers the sizing knobs only); carried from porting-inventory §3 (T0 there) at the v1 fold | T0 | `vllm/v1/worker/gpu/model_runner.py:504,647`; `vllm/v1/worker/gpu_worker.py:430` | - | - | `planned: specs/warmup-memory-profiling.md` | `INVENTORIED` | - | +| `KV-WARMUP-PROFILE` | Dummy runs, warmup, and startup memory profiling that derive the KV budget (`KV-SIZING` covers the sizing knobs only); carried from porting-inventory §3 (T0 there) at the v1 fold | T0 | `vllm/v1/worker/gpu_worker.py::determine_available_memory`; `vllm/v1/worker/gpu/model_runner.py::profile_run`; `vllm/v1/worker/gpu/model_runner.py::model_memory_usage` | - | - | `planned: specs/warmup-memory-profiling.md` | `INVENTORIED` | - | | `ENG-EXPERT-STREAM` | Expert streaming from disk: bank-only routed-MoE weights paged into fixed contiguous Marlin slots after logical-expert→slot remap (low-concurrency capacity mode; surpass-track — inference-time disk expert paging is ABSENT in pinned vLLM) | T2 | absent in-pin: `vllm/model_executor/offloader/uva.py:21` (CPU-blanket UVA only), `vllm/model_executor/offloader/prefetch.py:557-560` (cpu-only); design reference antirez/ds4 (`ds4_metal.m`, `ds4_cuda.cu`, `ds4_ssd.c`); local dense-stride constraint `src/vt/cuda/marlin/libtorch_stable/moe/marlin_moe_wna16/marlin_template.h:543-550` | - | - | [expert-streaming.md](specs/expert-streaming.md) | `READY` | - | | `ENG-WEIGHT-OFFLOAD` | Inference-time CPU weight offload mirror floor: UVA `cpu_offload_gb` per-parameter offload with pinned+zero-copy views and opt-in name-segment targeting (`cpu_offload_params`), plus layer-group `PrefetchOffloader`; v1-supported at the pin | T2 | `vllm/config/offload.py:23,34-44,47-76`; `vllm/model_executor/offloader/uva.py:64,80-108`; `vllm/model_executor/offloader/base.py:23-33,46-92,94-125,126-162`; install point `vllm/v1/worker/gpu_model_runner.py:939` (re-derived at the pin — the previously recorded `:445,913` are STALE and now unrelated lines); layer-wrap site `vllm/model_executor/models/utils.py:816,824`; loader re-offload `vllm/model_executor/model_loader/utils.py:160-193`; cudagraph seam `vllm/compilation/cuda_graph.py:310,324,359` + `breakable_cudagraph.py:379,387,421`; env `vllm/envs.py:278-279,1938-1943`; helpers `vllm/utils/platform_utils.py:51-57`, `vllm/utils/torch_utils.py:766-776`; upstream lineage: `offloader/base.py` header records adaptation from SGLang `srt/utils/offloader.py`; tests `tests/basic_correctness/test_cpu_offload.py:9-29`, `tests/quantization/test_cpu_offload.py:18-64` | W0a config surface: `include/vllm/config/offload.h`, `src/vllm/config/offload.cpp` (backend enum, both sub-configs, `Validate()` = 2 hard errors + 3 collected warnings, dot-anchored segment match, `int(gb*1024**3)` truncation, auto-selection order, layer grouping, JSON parse mirroring the `kv_transfer_config` precedent). UNREACHABLE: nothing constructs one yet. W0b wires it end to end: `include/vllm.h` `offload_config` (ABI v21), the C-API parse+`Validate()`+record in `src/capi/vllm_c.cpp`, `EngineParams::offload_config`, and the server `--offload-config` flag. Still UNREACHABLE by design: the config is validated and recorded, and no weight moves until W2/W5 Totality guard: `ModelFactory::supports_weight_offload` (defaults FALSE) + `RefuseUnsupportedWeightOffload` before any weight I/O + `VerifyWeightOffloadWasConsulted` after load; no model declares support yet, pinned by test. W2a decision: `include/vllm/model_executor/weight_offload_policy.h` + `src/vllm/model_executor/weight_offload_policy.cpp` (per-weight offload/not-targeted/budget-exhausted, running byte budget, `FromConfig` for the UVA arm only). Application seam CHOSEN as the LOADERS, beside `GgufKeepQuantPolicy::Route`, because a constructed `LoadedModel` has already allocated its device copy. W1 seam: `include/vllm/model_executor/weight_offloader.h` + `src/vllm/model_executor/weight_offloader.cpp` (interface, no-op default, process-global, factory), installed at `LoadedEngine::FromModelDir` and read at `ModelRegistry::Prepare` -- our analogue of `make_layers`, which this tree does not have | `tests/vllm/config/test_offload_config.cpp` 11/11 cases, 126/126 assertions; RED-first captured on a compiling stub (11/11 cases, 51/122 assertions RED, build rc=0 / 0 compile errors); mutation-proven 6/6 with compile status reported per mutation; W0b ABI round-trip `tests/capi/test_capi.cpp` "offload_config defaults to NULL and is parsed+validated (ABI v21)" 21/21 (7 refusal cases each asserted INVALID_ARGUMENT not MODEL_LOAD, plus the warning-is-not-a-refusal case), mutation-proven 2/2 (drop `Validate()` -> 3 red; warning-as-throw -> 1 red) | [weight-offload-uva.md](specs/weight-offload-uva.md) | `ACTIVE` | `CLAIM-WEIGHT-OFFLOAD-W0A` | | `ENG-HYBRID-PLACEMENT` | Hybrid device placement: per-tensor-group device assignment resolved at model build, delivering routed-MoE expert COMPUTE on the CPU backend while attention/dense/router/norms stay on GPU (surpass-track — vLLM has the CPU MoE kernels but selects them platform-wide via `current_platform.is_cpu()`, so hybrid placement is ABSENT in pin). Moves compute toward the weights, the inverse of `ENG-WEIGHT-OFFLOAD`/`ENG-EXPERT-STREAM`; composes with `BACKEND-DISTRIBUTED-TP` sharding on an orthogonal axis and refuses where they conflict. Issue [#149](https://github.com/mudler/vllm.cpp/issues/149) (CPU-MoE half only) | T2 | absent in-pin (placement): `vllm/model_executor/layers/fused_moe/oracle/mxfp4.py:533`, `oracle/fp8.py:129`, `oracle/int8.py:53`, `oracle/int_wna16.py:111`, `oracle/unquantized.py:97,202`, `oracle/w4a8_int8.py:40` (all `current_platform.is_cpu()`); kernels present but platform-gated: `vllm/model_executor/layers/fused_moe/experts/cpu_moe.py`, `cpu_fused_moe.py:398,430`, `vllm/_custom_ops.py:3790,3803`; secondary oracle llama.cpp @ `237ad9b96` (`gateable = yes`): `common/arg.cpp:2451-2478` (`-ot`/`-cmoe`/`-ncmoe`), `common/common.h:1046-1054` (`LLM_FFN_EXPS_REGEX`), `src/llama-model-loader.cpp:1158-1160`, `src/llama-model.cpp:1032`, `include/llama.h:530`, auto-fit `common/fit.h:24` + `common/fit.cpp:457,485` (TP conflict refused at `:181`) | - | - | [hybrid-placement.md](specs/hybrid-placement.md) | `READY` | - | diff --git a/.agents/issue-index.md b/.agents/issue-index.md index 4fef4a09b..c36fb5970 100644 --- a/.agents/issue-index.md +++ b/.agents/issue-index.md @@ -371,5 +371,7 @@ rather than merged. `scripts/check-agent-record.py` gates both. | [#1189](https://github.com/mudler/vllm.cpp/issues/1189) | `VT-QUANT-FP8-GROUP` | Block-wise (128x128) FP8 so `Qwen/Qwen3.8-27B-FP8` runs instead of being refused. `weight_block_size` appears nowhere in `src/` or `include/`, so `469f38395` refuses the arm by name (#1166). Six independently landable milestones; M1 lands here. M1 is `vt::QuantFp8Group`, the dynamic per-token per-group activation quant, CPU and CUDA. The numerics mirror the kernel that ACTUALLY EXECUTES on a CUDA-alike platform with a contiguous input, which is the C++ custom op at `csrc/libtorch_stable/quantization/w8a8/fp8/per_token_group_quant.cu:42-96` and NOT the Triton kernel at `fp8_utils.py:95-150`: `fp8_utils.py:635-650` calls the former and returns before the latter. The two arms differ, and the difference is measurable rather than cosmetic. The CUDA kernel divides twice, `local_absmax / max_8bit` at `:68` and `static_cast(src) / y_s` at `:85`; the Triton kernel multiplies by `(1.0 / fp8_max)` at `fp8_utils.py:145` under a comment that names the 1-ULP gap. Upstream's own test tolerates the gap with `rtol=0.15` (`test_block_fp8.py:112-114`), so a value comparison cannot tell the two apart and only a byte comparison against a spelled-out reference can. `eps` is the reduction's INITIAL value (`:47`), not a post-clamp, which is what keeps an all-zero group from dividing by zero. Scope refused here and owed to later milestones: the block-scaled GEMM (M2), `Fp8BlockWeight` and the loader (M3), `Fp8BlockLinearMethod` and the Qwen3.5 wiring (M4), the mainloop-scaled CUTLASS kernel and the column-major/TMA-aligned scale layouts (M5), merged `gate_up`/QKV (M6). M1 lands UNREACHED: no production entry point dispatches `vt::QuantFp8Group` at its merge commit, M4 owns the wiring, and `.agents/specs/vt-quant-fp8-group.md` lists it under `## Owed`. The CUDA arm compiles and its on-hardware leg is owed too, because the row took no GPU lease by design: the CPU arm is the gateable one | feature | | [#1190](https://github.com/mudler/vllm.cpp/issues/1190) | `ENV-AGNOSTIC-CAMPAIGN` | One operator's hosts, share paths and addresses are written into 227 tracked files where a `.env` placeholder belongs, so a second developer who follows the protocol documents is told to reach a box on one home network. Re-derived at `fd64c76ee` with `git grep -cIE 'dgx\.casa\|nas_share\|192\.168\.\|thor:gpu0'`: `dgx.casa` 203 files, `nas_share` 32, `192.168.` 24, `thor:gpu0` 11. The mechanism was already there and almost unused: `.env.example` is tracked, `.env` and `.agents/developer-preferences.md` are ignored, `scripts/agent-onboard.py --env-set` already records one answered value and refuses an undeclared key, and `${VLLM_SOURCE}` and `${GPU_LOCK}` already resolve in 59 and 50 files while `${DEVICE_ARCH}`, `${DEVICE_TOOLKIT_ROOT}` and `${DEVICE_COMPILER}` resolve in none. This SCOPING row lands the rule and the mechanism, not the sweep. The rule is a substitution test: replace the literal with a second developer's value and if the sentence stays true it is CONFIGURATION and becomes `${KEY}`, and if it becomes false it is PROVENANCE and stays literal. That reverses the issue's own ranking, because all 34 hits in the densest guide file `.agents/environment.md` are provenance or named-profile definition and none is configuration, so density does not predict the defect and a blind `sed` would falsify records. Landed: three new keys `GATE_CHECKOUT`, `SHARED_STORAGE_ROOT` and `GATE_DEVICE` derived from the literals that recur and map to no existing key; the create-on-first-use route in `scripts/agent-start.py`, which printed `environment: missing` as a status label and then listed next actions that never mentioned it, so the fallback in practice was a host name copied from a document; the matching obligation in `AGENTS.md`; and one worked example. The worked example `scripts/dgx-bringup.sh` found a live defect rather than a cosmetic one: it defaulted `CUTLASS_DIR` to `$HOME/cutlass_probe` while `.agents/environment.md:389` records `$HOME/cutlass-4.5.0` as mandatory on the same box, and a configure that misses CUTLASS silently drops the sm120a NVFP4 GEMM and FlashAttention-2, which that file measures as moving the SACRED `test_qwen27_paged_engine` from 235/235 to 234/235 with the source untouched, so a stale hard-coded default is a false green. Its new test also caught that `set -a; . ./.env; set +a` over an `.env.example` copy blanks a value the caller exported, so the process-environment contract is now executable. Waves `ENV-AGNOSTIC-W1-TOOLING` through `ENV-AGNOSTIC-W5-LEDGERS` own the sweep, partition all 227 files, and open their own issues. Spec [`env-agnostic.md`](specs/env-agnostic.md) | bug | | [#1193](https://github.com/mudler/vllm.cpp/issues/1193) | `SPEC-DSPARK-QWEN3-ROUTING` | A Qwen3 DSpark draft declaring `architectures=["DSparkDraftModel"]` with `model_type` `qwen3` has no route. The pin forces every DSpark draft that is not `Qwen3DSparkModel` or `Gemma4DSparkModel` onto `model_type` `deepseek_v4` (`vllm/config/speculative.py:934-944` @ `555967922`), and vLLM PR 52197 (merged 2026-08-17 at `7075ddac`) replaced that with a leading branch normalizing the pair to `Qwen3DSparkModel`. We diverge from BOTH: the forced rewrite was never ported, so nothing in `src/vllm/entrypoints/model_loader.cpp` reads a draft config's `architectures` key at all, and `SpeculativeConfig::IsDsparkDraft` (`include/vllm/config/speculative.h:120-136`) has no production caller — every reference outside its header is in `tests/vllm/config/test_speculative_dspark.cpp:132-140`, and `ResolveSpecConfig` branches on `cli.method` alone. The checkpoint is real and gateable here: `RadixArk/Qwen3.8-27B-DSpark` at revision `85ef153be924f17ce4bf62726954eeaa4a73e854` carries exactly that config shape in one 2718576122-byte shard, drafting five layers for a 64-layer Qwen3.8-27B target | bug | +| [#1198](https://github.com/mudler/vllm.cpp/issues/1198) | — | Three specs assert `model_loader.cpp` behaviour the loader no longer has, found while verifying line citations for conversion under [#1143](https://github.com/mudler/vllm.cpp/issues/1143): `safetensors-windowed-load.md:63,108` quotes a `shards.clear()` that does not exist (the shard vector is a `shared_ptr` released by the deferred-expert closure, `model_loader.cpp:1636-1643`), `gguf-dflash-draft.md:17-18` calls `LoadDflashDraft` "still typed on `std::vector`" when it takes a `SharedHeadSource`, its `A5`/`B2` rows plan around a GGUF refusal the loader says at `:906-910` is GONE, and `model-factory-registry.md:91` cites `IsDenseArch` which survives only in a comment saying the registry superseded it. Filed rather than repaired because a citation sweep can see the claim is false and not what the true statement is — that is the owning row's judgement. Owned under `## Owed` in [`citation-anchor-freshness.md`](specs/citation-anchor-freshness.md) | bug | +| [#1199](https://github.com/mudler/vllm.cpp/issues/1199) | — | `scripts/check-symbol-anchors.py --upstream-root` resolved 354 `vllm/...::Symbol` citations against the parity pin `555967922`: **343 fresh, 0 stale**, which is the measurement that decides the symbol convention over line anchors, since the same pin advance broke every line anchor [#1139](https://github.com/mudler/vllm.cpp/issues/1139) examined. The 11 that did not resolve name a FILE absent at the pin: six `.agents/model-matrix.md` rows (`olmo.py`, `olmo2.py`, `ouro.py`, `persimmon.py`, `plamo2.py`, `fuyu.py` — `registry.py:658` routes `OlmoForCausalLM` to the `transformers` fallback and `registry.py:765` lists `FuyuForCausalLM` as removed at 0.25.0, so our rows claim a mirror source vLLM deleted), two malformed `vllm/tests/kernels/...` paths, `vllm/v1/worker/gpu/worker.py` for `gpu_worker.py`, and one fixture-text false positive. Filed rather than repaired because the model-matrix half is a claim about vLLM, not a path edit. Owned under `## Owed` in [`citation-anchor-freshness.md`](specs/citation-anchor-freshness.md) | bug | | [#1213](https://github.com/mudler/vllm.cpp/issues/1213) | `ENV-LEASE-RUNTIME-STAGING` | `AGENTS.md` stated that a leased worker "has no compiler, no downloader and no Python, so it cannot produce a runtime in place", and `.agents/environment.md` carried the matching clause twice for `dgx:gpu0`. All three negatives are false. `rc describe dgx:gpu0` states that a job runs as root in an Ubuntu 24.04 container carrying `git`, `curl`, `wget`, `ssh`, `gcc`, `g++`, `make`, `cmake`, `ninja`, `pkg-config`, `python3`, `pip` and `venv`, and it instructs the reader to install anything missing; the one limit it names is the absent CUDA toolkit. Two jobs then compiled inside a lease on 2026-08-18: `claude/mudler-ubuntu-box/qwen38-gate` apt-installed `cuda-nvcc-13-0` from the `ubuntu2404/sbsa` lane and built this tree 1791/1791 to `BUILD_RC=0` (`/mnt/nas_share/rc/qwen38-gate/out-main/cfg.log` records `nvcc` 13.0.88 and `CUDA feature cutlass-fp8: ENABLED for [121a]`), and `/mnt/nas_share/rc/mtp_test/build.sh` cloned `github.com/mudler/llama.cpp` from inside a job and left a 97 MB `libggml-cuda.so` on the share. **Why it matters:** "the lease cannot produce a runtime in place" is the stated basis for treating the pinned vLLM oracle as unreachable from a lease, and that oracle is the denominator for every speed-parity number the project owes, so the premise needs re-testing rather than inheriting. This claims nothing about a model run; #1185 owns that and stays open. FIXED IN FLOW: the `AGENTS.md` paragraph and both `.agents/environment.md` clauses now say what the measurement supports, and the four real limits (no preinstalled CUDA toolkit, global installs leak until the pod restarts, CIFS `/workspace` holds no symlink so build in `/tmp` and `cp -rL`, and `-j 4` because unconstrained parallelism OOM-reboots the box) plus the host-versus-container egress distinction ride with the correction. | record | | [#1226](https://github.com/mudler/vllm.cpp/issues/1226) | `ENG-CUDAGRAPH-DEDUP` | The owed device A/B for #1162 RAN on `dgx:gpu0` 2026-08-18 (GB10, driver 580.173.02, nvcc 13.0.88, `rc` job f88d484b, gated commit `72de552c8`, whose four dedup sources are byte-identical to the merged `2a976eb9f`) and it SPLIT. CORRECTNESS PASSED and closes [#1184](https://github.com/mudler/vllm.cpp/issues/1184): 12/12 cells exit 0, zero `invalid device function` and zero `engine-fatal` where the pre-fix head `e4ce5571a` died after exactly one replay, ON replays as often as OFF (60=60, 33=33, 43=43), and `--output-token-ids` is IDENTICAL over 10/10 comparisons with the three OFF/OFF controls passing FIRST and three workloads hashing to three DIFFERENT values. THE BENEFIT IS REFUTED for the case the row was filed for: `N == M` in every `VT_CUDA_GRAPH_DEDUP=1` cell — 3 graphs to 3 execs on sizes [24 16 8], 2 to 2 on [16 8], 2 to 2 on [32 24] — with the registry count CLIMBING 1→1, 2→2, 3→3, so more than one capture reached it and the 1:1 is a measurement rather than the single-capture artefact of the first attempt. The cause is structural and was pre-registered before the run: `AppendKernelPayload` hashes (`func`, `gridDim.{x,y,z}`, `blockDim.{x,y,z}`, `sharedMemBytes`) at `src/vt/graph_dedup_runtime.h:121-128` and the memcpy payload hashes the copy extent, so the padded batch dimension is IN THE KEY, no candidate group forms and `cudaGraphExecUpdate` is never attempted. That contradicts the row's own premise, and SGLang keys the same fields (`cuda_graph_dedup_mixin.py:105-114`), so whatever folds upstream is not decode buckets either. NO throughput or memory number is recorded — clocks unpinned AND the ON arm allocated exactly as many executables as OFF. `VT_CUDA_GRAPH_DEDUP` therefore stays OFF and the default flip is NOT JUSTIFIED on this evidence. THE OPEN HYPOTHESIS this issue owns, deliberately not decided by the record that filed it: a COARSER key keeping the function addresses and topology but dropping the launch dimensions and memcpy extents would let two padded buckets form a candidate group at all, and the probe-before-fold design means a wrong grouping costs one wasted `cudaGraphExecUpdate` probe and a private executable rather than a wrong replay, so it is a cost question needing its own spec, a red-first discrimination test and a device probe-refusal rate. Also owed here: whether ANY other capture site or two models sharing the process-singleton registry can fold (only the Qwen3 dense decode driver was exercised), and a supporting `orin:gpu0` leg, BLOCKED because the Jetson 540.4.0 driver cannot run a CUDA 13 runtime (`cudaGetDeviceCount err=35`; a CUDA 12.x toolkit is the untried route). Honest gaps carried into the record: per-shape replay counts are unavailable, so B's ~30-per-shape is arithmetic over a printed TOTAL; the driver's "N captured size(s)" counts SLOTS not captures; the container's own cuBLASLt was never re-tested at CUDA 13.0. Full evidence [`benchmark-record.md`](benchmark-record.md) entry `ENG-CUDAGRAPH-DEDUP W4`; spec [`eng-cudagraph-dedup.md`](specs/eng-cudagraph-dedup.md) `## Outcome` | perf | diff --git a/.agents/porting.md b/.agents/porting.md index ec74536d7..a0ec7e099 100644 --- a/.agents/porting.md +++ b/.agents/porting.md @@ -45,6 +45,27 @@ Anything genuinely written from scratch is recorded as such in [`porting-inventory.md`](porting-inventory.md) §9. "I couldn't find it upstream" is a search result, not a conclusion — say which paths you searched. +### Name the symbol, not only the line + +A line number is a coordinate into a moving file, so it decays. Write +`` `path/to/file.cpp::SymbolName` `` whenever a citation crosses a file +boundary. Keep the line number beside it only while you are reading; a citation +that has to survive somebody else's edit carries the symbol. + +`src/vllm/entrypoints/model_loader.cpp` is the measured case +([#1143](https://github.com/mudler/vllm.cpp/issues/1143)): cited by line from +109 sites in 45 files, and one 45-line insertion near its top moved 203 of those +references at once, in files that change never opened. The same defect at +upstream scale is [#1139](https://github.com/mudler/vllm.cpp/issues/1139), where +a pin advance left three `vllm/v1/worker/**` line anchors pointing at unrelated +code and two of them had already been copied elsewhere. + +`scripts/check-symbol-anchors.py` gates the in-repo half of the convention: the +symbol a citation names must still be in the file it names. `--upstream-root +` runs the same question against the pinned oracle, which CI +cannot do because it has no checkout. Design, limits, and what the two runs +measured: [`specs/citation-anchor-freshness.md`](specs/citation-anchor-freshness.md). + ## Mirror the memory format, not just the math **A token-exactness gate cannot catch a dtype that is too WIDE.** F32 where diff --git a/.agents/specs/citation-anchor-freshness.md b/.agents/specs/citation-anchor-freshness.md new file mode 100644 index 000000000..d37e54fec --- /dev/null +++ b/.agents/specs/citation-anchor-freshness.md @@ -0,0 +1,412 @@ +# Citation anchors that do not rot + +Issues: [#1143](https://github.com/mudler/vllm.cpp/issues/1143) (in-repo scale), +[#1139](https://github.com/mudler/vllm.cpp/issues/1139) (upstream scale), +[#1198](https://github.com/mudler/vllm.cpp/issues/1198), +[#1199](https://github.com/mudler/vllm.cpp/issues/1199). +Row: none. This is a repository-wide convention plus its gate, so it has no +owning capability row and does not belong in a capability matrix. + +## Now + +`GATE-SYMBOL-ANCHORS` ships green. 93 in-repo symbol anchors are checked on +every run against a recorded floor of 85, 26 of them converted here. +`model_loader.cpp` line references in tracked, non-frozen, non-locked files fell +from 112 in 47 files to 92 in 35 on this branch; the residue is listed under +`## Owed`. + +## Scope + +One defect, two scales. + +**In-repo (#1143).** `src/vllm/entrypoints/model_loader.cpp` is cited by +absolute line number from 109 sites across 45 files. It is ~1740 lines and is +edited by almost every engine and model row, so an edit near its top silently +retargets every citation below it, in files the editing change never opens. One +45-line insertion moved 203 references at once. Nobody could then tell a +citation that change broke from one that was already wrong. + +**Upstream (#1139).** All three `vllm/v1/worker/**` line anchors on the +`KV-WARMUP-PROFILE` row point at unrelated code at the parity pin +`555967922` — a `DraftModelSpeculator.set_attn(...)` call, a `torch.zeros(...)` +argument, and a comment about `max_split_size_mb`. Two of them had already been +copied into `gguf_device_fit.h` and `expert-streaming.md`, so staleness was +propagating by copy-paste. + +Out of scope: splitting `model_loader.cpp`, and any change to +`scripts/check-agent-record.py`. The `.agents/engine-matrix.md` exclusion was +lifted after review: the fix is one CELL inside an existing row, which adds no +row and moves no counter, so it cannot collide with a concurrent session that +appends one. + +## The options, and why option 4 + +1. **Repair all 109 citations.** Fixes today, guarantees tomorrow's recurrence. + The next edit near the top of that file breaks them all again, and the repair + itself would launder pre-existing debt: rewriting a citation from the current + tree makes a wrong one and a broken one indistinguishable, which is precisely + what #1143 says nobody can do today. +2. **Repair plus a checker over line anchors.** A line number carries no claim, + so the only thing such a checker can compare against is a stored expectation + table — a record surface every pull request would have to edit, which + AGENTS.md `## Records` calls a lock and a defective gate. Reading the + expectation out of the cited file instead is the #911 tautology: that shape + reported 27/27 FRESH while five anchors pointed at unrelated code. +3. **Change the convention to symbols.** Removes the class rather than + re-baselining it, and is the largest. +4. **Scoped combination — chosen.** Adopt the symbol convention, gate it, and + convert a verified subset now, recording the rest as visible debt. + +Option 4 wins on one observation the other three miss: **the convention already +exists in this tree.** 539 `` `path::Symbol` `` citations are already written, +`.agents/model-matrix.md` uses the form for every upstream model anchor, and +they had never been checked. This change is not an invention, it is finishing +something half-adopted — which is why the gate lands green over 93 anchors +instead of needing a flag day. + +It also resolves the tautology hazard structurally rather than by care. Under +the symbol convention the two sides of the comparison come from different +places: the EXPECTATION is the symbol name, written by the citing author, living +in the CITING file; the EVIDENCE is the cited file's text. Nothing is derived +from the cited file, and nothing is stored centrally, so the surface is not a +lock — a citation's expectation rides in the file that owns the claim. + +## Design + +`scripts/check-symbol-anchors.py`. + +A citation is a single backtick span holding a path with a source extension, +`::`, and a possibly-qualified identifier. The extension requirement keeps +ordinary C++ prose out: `Qwen3_5MTPKind::kMoe` has no path in front of it. + +| Bucket | Rule | Verdict | +|---|---|---| +| in-repo | path is a tracked file, or a bare basename with our own source extension matching a tracked file | the FULL cited symbol must appear in that file with word boundaries on both sides | +| missing local | the path's directory is at least two components deep AND exists here, but the file does not | FAIL | +| upstream / unknown | anything else | counted, skipped | + +Three counts ride alongside: how many of the in-repo checks resolved an +AMBIGUOUS basename, how many FROZEN files were skipped by prefix, and how many +UNTRACKED files carry a citation this run cannot see. + +Every bucket is printed, and the three buckets must SUM to the citation count, +so a citation that stops being counted anywhere is arithmetic rather than +judgement. A run that checks ZERO in-repo citations FAILS, and so does one below +the recorded floor `MIN_IN_REPO_CHECKED` — a zero-guard over a population of +ninety is a mute switch, because one added `FROZEN_PREFIXES` entry or one +narrowed `CITATION_RE` takes the count from 91 to 1 and stays green the whole +way down. The floor carries headroom on purpose: no change has to edit it, so it +is a ratchet and not a per-PR record lock. `--min-checked` overrides it, and a +`--root` fixture tree defaults to 1. + +`--upstream-root ` additionally resolves `vllm/...` citations +against the parity pin, read with `git show :` after asserting the +checkout CONTAINS that commit. Opt-in and never a CI gate, because CI has no +oracle checkout — it is the instrument that would have found #1139 before a +reader did. + +### A citation span is not evidence + +The tautology immunity is "expectation from the CITING file, evidence from the +CITED file". Those two sides COLLAPSE when they are the same file: the +expectation is then read from the file under test, and the citation text itself +contains the symbol, so `` `a.cpp::GhostSymbol` `` written inside `a.cpp` read as +fresh over a symbol that exists nowhere else. That is #911 again, reached by a +different door, and `.agents/porting.md` now tells authors to write the full +path — which makes a file documenting its own symbols the most natural next +thing anyone writes. + +Every citation span is therefore stripped from the cited file before the search. +Stripping all of them, not only the self-citing one, also closes the cross-file +form, where `a.cpp` names `Ghost` only inside its own citation of `b.cpp`. A +citation is a claim about some file; it is never a definition or a call, so it +may not stand as evidence for one. + +### An ambiguous basename is checked, not skipped + +A basename matching more than one tracked file used to be counted and dropped, +which silently skipped five live citations. It is checked against EVERY +candidate instead, and reported only when NONE of them contains the symbol: one +candidate containing it is exactly what the citation claims, and reporting on +the ambiguity itself would be a false accusation. + +### The untracked blind spot is counted, not closed + +An untracked file is absent from `git ls-files`, so neither its citations nor +its existence as a cited PATH reaches the walk. This change found that out about +itself: the first version of the test file wrote its fixtures as real local +paths, was untracked, and ran green. CI is sound because everything is tracked +there. A local pre-commit run is not, and a green run before `git add` is not a +green run. + +Scanning the working tree instead would change what the gate's SUBJECT is, so +the skip stays. It is COUNTED and PRINTED, the same discipline `frozen_files` +already gets. + +### What it deliberately does not do + +- **Not a definition check.** "`ModelRegistry::Load`, which `model_loader.cpp` + calls" is a legitimate citation, and a definition-only rule would reject it. A + rename still reds, because a rename removes the token from the file. +- **Not a line-anchor check.** A bare line number carries no claim, so there is + nothing honest to check it against. +- **Depth two, not a root list.** `tests/` was the first cut and it was wrong: + vLLM has a `tests/` too, and twenty correct upstream anchors went red as + broken local paths on the first run. Directory existence at depth two + separates the cases without a list to maintain. The cost is that a typo in a + one-component directory falls through to the upstream bucket — a miss, never a + false accusation. +- **No network.** +- **A dot-leading path is a citation.** It was not: the grammar's first + character class excluded `.`, so a citation of anything under `.agents/` or + `.github/` matched zero times and said nothing about it. Latent while nobody + wrote one, and live the moment somebody cites a spec by symbol — which + `.agents/porting.md` now invites. + +## Tests + +`tests/scripts/test_check_symbol_anchors.py`, 21 cases. Candidate lists are +sorted, so a citation's verdict and the path a stale one names do not depend on +the iteration order of a hash set; `test_an_ambiguous_basename_is_checked_against_every_candidate` +puts the symbol in the LAST candidate so that a first-candidate-only checker +reds deterministically. The load-bearing one is +`test_verdict_depends_on_the_citing_text`: one cited file, unchanged, cited +twice with different symbols, asserting the two verdicts DIFFER. A #911-shaped +checker cannot pass it, because nothing it reads varies between the two runs. + +That case cannot see the self-citation door, because it uses two DIFFERENT +files. `test_a_citation_span_is_not_its_own_evidence` and +`test_a_citation_in_the_cited_file_is_not_evidence_either` close it from both +sides. + +`test_this_tree_meets_the_recorded_floor` asserts the checked count against a +LITERAL 85, not against the checker's own constant. Reading the constant would +make the case a tautology: lowering the floor would lower the expectation with +it. `test_the_floor_reds_when_the_population_collapses` drives `--min-checked` +over a one-citation tree so the comparison itself is executable, and +`test_the_buckets_sum_to_the_citation_count` runs a fixture tree that populates +all three buckets at once — this tree has zero missing local paths, so asserting +the sum only here would leave that bucket droppable without a red. + +`test_untracked_files_carrying_citations_are_counted` builds a real repository, +because outside one `git ls-files` fails, the checker falls back to walking the +filesystem, and the blind spot cannot exist to be measured. + +## Gates + +```sh +python3 scripts/check-symbol-anchors.py +python3 scripts/check-symbol-anchors.py --self-test +python3 tests/scripts/test_check_symbol_anchors.py +python3 scripts/check-symbol-anchors.py --upstream-root /path/to/vllm # opt-in +python3 scripts/check-symbol-anchors.py --min-checked N # override the floor +``` + +Registered in `scripts/agent-preflight.sh` (`CHECKERS` and `SUITES`) and in +`.github/workflows/ci.yml`. + +## Evidence + +Every absolute below was re-measured at the final head of this branch. The first +recorded set was not reproducible: the upstream run was recorded as 354/343 and +reproduces as 364/353, the residue as 106 in 46 to 86 in 34 and reproduces as +112 in 47 to 92 in 35, and `check-agent-record.py` was recorded as ENGINE=161 +and reports ENGINE=162. Every DELTA in that set was right and every ABSOLUTE was +not, which is the shape a number quoted from an earlier run takes. + +**Tree.** 618 citations in 2758 scanned files, 180 frozen files skipped, 0 +untracked files carrying citations; in-repo checked 93 (fresh 93, stale 0), of +which 5 resolved an ambiguous basename; upstream/unknown 525; missing local path +0; buckets sum 618 vs 618; floor 85. `rc=0`. + +**Upstream mode, at the pin.** 364 upstream anchors checked against +`5559679229bc961848b121ccdeaa8fa5d79bec98`: 353 fresh, **0 stale**, 11 naming a +file absent at the pin, which dedupe to the 10 reported lines (#1199). Not one +symbol anchor was stale across the same pin advance that broke every line anchor +#1139 examined. That is the measurement that decides the convention. + +**The perturbation control.** "0 stale" says nothing until the corpus is shown +capable of going stale. Appending `Zq` to every cited symbol name, applied and +compiling, reports `in-repo checked 93 (fresh 0, stale 93)` and +`upstream checked 364 (fresh 0, stale 353, file absent 11)`: not one of the 446 +fresh anchors survives the rename. Freshness is therefore a property this corpus +has to earn, not one the check hands it. + +**`scripts/check-agent-record.py`.** `rc=0`, ENGINE=162 MODEL=377 QUANT=82 +KERNEL=52 BACKEND=85. + +**The residue instrument, spelled so somebody else gets the number.** Population +is `git ls-tree -r --name-only ` minus four surfaces: `.agents/completed/**` +(frozen archive), `.agents/issue-index.md` (append-only, an edit is forbidden), +`.agents/benchmark-record.md` and `.agents/parity-ledger.md` (append-only +records). Counted is every occurrence of `model_loader\.cpp:[0-9]`, plus the +number of distinct files carrying one: + +```sh +excl=(':!.agents/completed' ':!.agents/issue-index.md' + ':!.agents/benchmark-record.md' ':!.agents/parity-ledger.md') +git grep -I -o -E 'model_loader\.cpp:[0-9]' -- "${excl[@]}" | wc -l +git grep -I -l -E 'model_loader\.cpp:[0-9]' -- "${excl[@]}" | wc -l +``` + +At the branch base `1f4878fdc`: 112 references in 47 files. On this branch +before merging: 92 in 35, so 20 converted. At `origin/main` `5af6e7631`: 112 in +47 again, and at this merged head 93 in 35, because `origin/main` independently +added one back in a file that already carried some. The 20 are the twenty rows +of the conversion table; the extra one is not this change's to explain away. + +**Mutation, thirteen mutations, each applied and compiling, over 21 executed +cases.** `applied` is a moved sha256 plus a non-empty `git diff --stat`; +`cases` is the LAST `Ran N tests` line, because a mutation that never applied +and one that fails to build both read as a passing test otherwise. + +| Mutation | applied | compile_rc | cases | rc | caught by | +|---|---|---|---|---|---| +| M1 every symbol declared present | yes | 0 | 21 | 1 | 7 cases | +| M2 the #911 tautology (expectation read from the CITED file) | yes | 0 | 21 | 1 | 7 cases incl. `test_verdict_depends_on_the_citing_text` | +| M3 vacuity guard removed | yes | 0 | 21 | 1 | `test_zero_checked_citations_is_a_failure` | +| M4 missing local path downgraded to a skip | yes | 0 | 21 | 1 | 3 cases | +| M5 frozen archive no longer skipped | yes | 0 | 21 | 1 | `test_the_frozen_archive_is_skipped_and_counted` | +| M6 word boundaries dropped | yes | 0 | 21 | 1 | 2 cases | +| M7 a citation span is evidence again | yes | 0 | 21 | 1 | 3 cases incl. both self-citation cases | +| M8 untracked files no longer counted | yes | 0 | 21 | 1 | `test_untracked_files_carrying_citations_are_counted` | +| M9 the floor comparison removed | yes | 0 | 21 | 1 | `test_the_floor_reds_when_the_population_collapses` | +| M10 one `FROZEN_PREFIXES` entry collapses the population | yes | 0 | 21 | 1 | 3 cases incl. `test_this_tree_meets_the_recorded_floor` | +| M11 one bucket dropped from the sum | yes | 0 | 21 | 1 | `test_the_buckets_sum_to_the_citation_count` | +| M12 only the first candidate of an ambiguous basename is consulted | yes | 0 | 21 | 1 | 4 cases, over three repeat runs | +| M13 the leading dot removed from the grammar | yes | 0 | 21 | 1 | `test_a_dot_leading_path_is_a_citation` | + +Zero invalid mutations. Every one restored byte-for-byte against the baseline +sha256. Two of the thirteen did not report honestly on the first pass, and both +are recorded because a mutation that reads green for the wrong reason is the +failure this table exists to prevent. M12's catch depended on the iteration +order of a hash set, so `by_base` is now built over a sorted walk and the case +puts the symbol in the last candidate; it was then re-run three times. M11 was +not caught at all: this tree has zero missing local +paths, so dropping that bucket left the arithmetic intact — the case now runs a +fixture tree that populates all three buckets, and the mutation reds. + +The `DISABLED_CREATION_CHECKER` stub registered in `scripts/check-pr-size.py` +fails 20 of the 21 cases, so the creation contract is rejected rather than +satisfied. + +### The gate caught this change + +`tests/scripts/test_check_symbol_anchors.py` first wrote its fixtures as +`src/vllm/a.cpp`. Untracked, it was invisible to `git ls-files` and the run was +green. The first commit made it tracked, and the checker immediately reported +twelve citations of a file that does not exist — in its own test file. That is +the failure this change exists to stop, arriving inside the pull request that +stops it, and it is why the fixtures are now `alpha/beta/...`: a directory that +exists in no tree falls into the skipped bucket in the real repository while +still resolving inside each temporary one. It also says something about the +instrument: an untracked file is not scanned, so a green run before `git add` is +not a green run. That lesson was first applied only to the +fixtures. It is now applied to the tool as well, which COUNTS and PRINTS the +untracked files carrying citations rather than passing over them in silence. + +## Converted here, and verified how + +26 anchors across 16 files. Each was checked by locating the named symbol in +`src/vllm/entrypoints/model_loader.cpp` and confirming the surrounding sentence +is still TRUE of that symbol. Most were stale before this change; the real line +is given where it differs from the cited one. + +| Site | Was | Now | Real line | +|---|---|---|---| +| `include/vllm/v1/core/sched/scheduler.h` | `model_loader.cpp:176 MakeScheduler` | `::MakeScheduler` | 812 | +| `include/vllm/v1/core/sched/scheduler.h` | `model_loader.h:206 scheduler_` | `::scheduler_` | `model_loader.h:556` | +| `src/vllm/multimodal/ltx2_video.cpp` | `:75-104` | `::SelectQueueForModel` | 148 | +| `src/vllm/multimodal/minimax_h3_video.cpp` | `:76-104` | `::SelectQueueForModel` | 148 | +| `tests/vllm/entrypoints/test_gguf_device_fit_reach.cpp` | `:100-115` | `::ResolveAutoDevice` | 100 (was correct) | +| `tests/vllm/entrypoints/test_loaded_engine_dense.cpp` | `:1081-1083,972` | `::MakeKVCacheResolved`, `::ResolveNumBlocks` | 993, 928 | +| `tests/vllm/model_executor/layers/test_fp8_block_quant.cpp` | `:1613` | `::FromModelDir` | `ModelRegistry::Load` at 1541/1706/1724 | +| `tests/vllm/model_executor/layers/test_fp8_block_quant.cpp` | `:1536` | `::RefuseUnsupportedWeightOffload` | 1629 | +| `tests/vllm/model_executor/test_gguf_device_fit.cpp` | `:1452-1453` | `::FromModelDir` | condition at 1533/1546 | +| `tests/vllm/v1/worker/test_runner.cpp` | `:1007-1023` | `::runner_` | 1135 | +| `tests/vllm/config/test_speculative_mtp_depth.cpp` | `:831` | `::ResolveMtp` | 911 | +| `tests/parity/test_qwen36_spec_decode.cpp` | `:582` | `::is_dense_model` | 1548, 1661 | +| `.agents/specs/cli-serve-bench.md` ×2 | `:800-811` | `::async_engine` | 1342 | +| `.agents/specs/vt-fp8-shared-seam.md` | `:133` | `::PrintLoadBytes`, `::LoadStatsEnabled` | 206, 188 | +| `.agents/specs/perf-chunked-prefill-budget-2026-08-13.md` ×2 | `:626-641` | `::ResolveMaxNumBatchedTokens` | 701 | +| `.agents/specs/perf-chunked-prefill-budget-2026-08-13.md` | `:704-717` | `::MakeSchedulerConfig` | 779 | +| `.agents/specs/perf-chunked-prefill-budget-2026-08-13.md` | `:1051-1058` | `::MakeScheduler` | 812, called at 1184 | +| `.agents/specs/perf-chunked-prefill-budget-2026-08-13.md` | `:711` | `::MakeSchedulerConfig` | 786 | +| `.agents/specs/gpu-mem-util-inert.md` | `:954-959` | `::ResolveNumBlocks` | 928 | +| `.agents/specs/gpu-mem-util-inert.md` | `:718-728` | `::ResolveEnablePrefixCaching` | 718 (was correct) | +| `.agents/specs/gpu-mem-util-inert.md` | `:1081-1083,972` | `::MakeKVCacheResolved`, `::ResolveNumBlocks` | 993, 928 | + +Two were already correct and are converted anyway, because a correct line +anchor is one edit away from a wrong one. + +Two more were WEAKER than what they replaced and were repaired after review. A +symbol that occurs eight times localises nothing: `::max_num_batched_tokens_` +named the value being traced rather than the hop the row describes, and is now +`::MakeScheduler`, which is the hop. `::Qwen3_5MTPKind` named the enum where the +selection is a ternary on the factory flag, and is now `::is_dense_model`, which +is the thing that decides. A conversion that reds on a rename is the minimum; a +conversion that also tells the reader where to look is the point. + +## Risks + +- **A citation can be converted to a symbol that is present but wrong.** The + gate cannot see that; only review can. Mitigated by converting nothing whose + surrounding claim could not be verified true — which is why #1198 exists + instead of eleven more conversions. +- **The `--upstream-root` mode has a known false positive** on fixture text + inside `tests/scripts/test_agent_record.py`. It is opt-in and never gates, so + the cost is one noisy line rather than a red lane. +- **A bare basename can become ambiguous** when a second file of that name + lands, silently dropping the citation into the skipped bucket. The counts are + printed, and full paths are the recommended form. + +## Owed + +- [#1143](https://github.com/mudler/vllm.cpp/issues/1143) stays OPEN and owns + the residue: 93 `model_loader.cpp:NNN` references in 35 files at this merged + head, down from 112 in 47, over the population and command recorded under + `## Evidence`. They were + left because converting one requires deciding what the author meant: a + heuristic pass that scanned a two-line context window around each of 92 such + citations found 62 naming no `model_loader.cpp` symbol at all, so there is + nothing to convert them TO without re-deriving the claim. The other 30 could + not be converted mechanically either, because the window pulls in symbols from + adjacent table rows; every conversion here was made by hand and verified + individually. +- [#1139](https://github.com/mudler/vllm.cpp/issues/1139) is CLOSED by this + change. Its last cell, the `KV-WARMUP-PROFILE` row in + `.agents/engine-matrix.md`, now carries + `vllm/v1/worker/gpu_worker.py::determine_available_memory`, + `vllm/v1/worker/gpu/model_runner.py::profile_run` and + `vllm/v1/worker/gpu/model_runner.py::model_memory_usage` in place of the three + line anchors that pointed at unrelated code at the pin. All three resolve + under `--upstream-root`; the upstream count moved 360 to 364 checked and 349 + to 353 fresh, with stale still zero. The edit is a CELL inside an existing + row: it adds no row and moves no counter, so `check-agent-record.py` reports + the same ENGINE=162 before and after. +- [#1198](https://github.com/mudler/vllm.cpp/issues/1198) - now FOUR specs + assert loader behaviour the loader no longer has, found while verifying + anchors for conversion. The fourth is + `.agents/specs/gguf-mtp-spec-decode.md`, which says in the present tense that + `LoadedEngine::FromModelDir` refuses a GGUF speculative target at + `model_loader.cpp:717-723` - the same removed refusal and the same stale range + as the `gguf-dflash-draft.md` row already on that issue. `717-723` is + `ResolveEnablePrefixCaching`. +- [#1199](https://github.com/mudler/vllm.cpp/issues/1199) - 11 upstream symbol + citations, deduping to 10 reported lines, name vLLM files absent at the pin; + six of them are `.agents/model-matrix.md` rows whose repair is a claim about + vLLM, not a path edit. +- **One-component directories still fall through to the upstream bucket.** A + typo in `tests/foo.cpp` is not reported, because vLLM has real files sitting + directly under top-level names we share and nothing in the path separates + them. This is a deliberate miss and never a false accusation; it is recorded + here rather than fixed, because narrowing it needs a rule that can tell the + two trees apart, which is what `--upstream-root` already does for the paths it + covers. + +## Stop conditions + +- Stop if a conversion cannot be verified true of the named symbol. Record it + under `## Owed` instead. +- Stop rather than storing expectations in a shared table. That surface is a + lock, and a lock is a worse defect than the staleness it would catch. diff --git a/.agents/specs/cli-serve-bench.md b/.agents/specs/cli-serve-bench.md index c02e0aea2..cf638e2a9 100644 --- a/.agents/specs/cli-serve-bench.md +++ b/.agents/specs/cli-serve-bench.md @@ -22,7 +22,7 @@ schedules a new batch before consuming the oldest result (`${VLLM_SOURCE}/vllm/v1/engine/core.py:622-669`). Our production equivalent is already implemented behind `LoadedEngine::async_engine()`: `AsyncLLM` constructs `EngineCoreProc` with the resolved queue depth -(`src/vllm/entrypoints/model_loader.cpp:800-811`), and `EngineCoreProc` selects +(`src/vllm/entrypoints/model_loader.cpp::async_engine`), and `EngineCoreProc` selects `step_with_batch_queue` at depth greater than one (`src/vllm/v1/engine/core_proc.cpp:22-37`). @@ -40,7 +40,7 @@ a binding parity result, because the frontends were mismatched. | Layer | Pinned vLLM / dependency behavior | Our anchor | |---|---|---| | CLI/client | `LLM.llm_engine.add_request` + `engine.step`, DELTA output (`tools/bench/vllm_closed_loop_metrics.py:59-102`) | closed-loop admission and metrics (`examples/bench/bench_core.h:416-589`) | -| engine dispatch | queue size and `step_fn` selection (`vllm/v1/engine/core.py:200-231`) | `LoadedEngine::async_engine` passes resolved depth (`src/vllm/entrypoints/model_loader.cpp:800-811`) | +| engine dispatch | queue size and `step_fn` selection (`vllm/v1/engine/core.py:200-231`) | `LoadedEngine::async_engine` passes resolved depth (`src/vllm/entrypoints/model_loader.cpp::async_engine`) | | batch queue | schedule-before-oldest-result (`vllm/v1/engine/core.py:622-669`) | `EngineCore::step_with_batch_queue` (`src/vllm/v1/engine/core.cpp:115-185`) | | scheduler | `AsyncScheduler` placeholder accounting (`vllm/v1/core/sched/async_scheduler.py`) | `src/vllm/v1/core/sched/async_scheduler.cpp` | | runner/sample | non-blocking sampled-token copy and device input update (`vllm/v1/worker/gpu/async_utils.py`, `gpu_model_runner.py`) | `GPUModelRunner::sample_tokens_async` and device mirror (`src/vllm/v1/worker/gpu/runner.cpp`) | diff --git a/.agents/specs/gpu-mem-util-inert.md b/.agents/specs/gpu-mem-util-inert.md index 297b0ac05..959376921 100644 --- a/.agents/specs/gpu-mem-util-inert.md +++ b/.agents/specs/gpu-mem-util-inert.md @@ -39,7 +39,7 @@ now says so. | C ABI field | `include/vllm.h:474-486` | | C ABI mapping | `src/capi/vllm_c.cpp:577-580` | | Engine field and default | `include/vllm/entrypoints/model_loader.h:87-90` | -| The place it is discarded | `src/vllm/entrypoints/model_loader.cpp:954-959` | +| The place it is discarded | `src/vllm/entrypoints/model_loader.cpp::ResolveNumBlocks` | | Upstream knob this mirrors | `vllm/config/cache.py:68` @ `555967922` | | Upstream override precedence | `vllm/config/cache.py:189` @ `555967922` | @@ -64,7 +64,7 @@ plain `double` that defaults to 0.92 cannot. `nullopt` means unset and resolves to vLLM's 0.92. This mirrors the tri-state `enable_prefix_caching` already in the same struct (`include/vllm/entrypoints/model_loader.h:120`, resolved at -`src/vllm/entrypoints/model_loader.cpp:718-728`). Nothing in the tree reads the +`src/vllm/entrypoints/model_loader.cpp::ResolveEnablePrefixCaching`). Nothing in the tree reads the field today, so the type change moves no behavior. Each surface then says what it means: @@ -95,7 +95,8 @@ that discards the value. That is the one seam every entry point reaches: the server, `examples/cli` through the C ABI, and any `include/vllm.h` client all build a `LoadedEngine`, whose constructor calls `MakeKVCacheResolved`, which calls `ResolveNumBlocks` -(`src/vllm/entrypoints/model_loader.cpp:1081-1083,972`). +(`src/vllm/entrypoints/model_loader.cpp::MakeKVCacheResolved`, +`src/vllm/entrypoints/model_loader.cpp::ResolveNumBlocks`). It fires once per engine load, not once per process. A server that loads a text engine and an embedding engine reports twice, because there are two pools and diff --git a/.agents/specs/perf-chunked-prefill-budget-2026-08-13.md b/.agents/specs/perf-chunked-prefill-budget-2026-08-13.md index 4a0490cdd..d3556d1ac 100644 --- a/.agents/specs/perf-chunked-prefill-budget-2026-08-13.md +++ b/.agents/specs/perf-chunked-prefill-budget-2026-08-13.md @@ -60,13 +60,14 @@ Unbroken chain, every hop read: |---|---| | CLI flag parsed | `src/vllm/entrypoints/openai/server_main.cpp:400` | | into `EngineParams` | `server_main.cpp:806` | -| resolved (explicit override wins) | `src/vllm/entrypoints/model_loader.cpp:626-641` | -| into `SchedulerConfig` | `model_loader.cpp:704-717` (`MakeSchedulerConfig`) | -| into the `Scheduler` | `model_loader.cpp:1051-1058` | +| resolved (explicit override wins) | `src/vllm/entrypoints/model_loader.cpp::ResolveMaxNumBatchedTokens` | +| into `SchedulerConfig` | `src/vllm/entrypoints/model_loader.cpp::MakeSchedulerConfig` | +| into the `Scheduler` | `src/vllm/entrypoints/model_loader.cpp::MakeScheduler` | | into `max_num_scheduled_tokens` | `src/vllm/v1/core/sched/scheduler.cpp:233-234` | | into the per-step budget | `scheduler.cpp:465` | -`enable_chunked_prefill` is hard-`true` for this path (`model_loader.cpp:711`). +`enable_chunked_prefill` is hard-`true` for this path +(`src/vllm/entrypoints/model_loader.cpp::MakeSchedulerConfig`). The encoder-decoder disable (`src/vllm/config/scheduler.cpp:52-57`) is not on it. `AsyncScheduler` — the production default — does not override `schedule()` (`include/vllm/v1/core/sched/async_scheduler.h:46-52`), so the budget path is the @@ -290,7 +291,7 @@ cannot move a token, but the gates are owed before merge regardless. ## Risks / decisions No vLLM-defined behavior is reopened. Our `ResolveMaxNumBatchedTokens` default -(`model_loader.cpp:626-641`: MoE 8192 at `max_num_seqs >= 32` else 4096; dense +(`src/vllm/entrypoints/model_loader.cpp::ResolveMaxNumBatchedTokens`: MoE 8192 at `max_num_seqs >= 32` else 4096; dense 2048) does NOT clearly diverge from upstream. The `2048` at `vllm/config/scheduler.py:49` is a **pydantic testing default**; `EngineArgs._set_default_args` overrides it to **8192/16384 on a >=70 GiB GPU**, diff --git a/.agents/specs/vt-fp8-shared-seam.md b/.agents/specs/vt-fp8-shared-seam.md index 622c196fd..ca6cd7ea8 100644 --- a/.agents/specs/vt-fp8-shared-seam.md +++ b/.agents/specs/vt-fp8-shared-seam.md @@ -184,8 +184,8 @@ Consequences, both plausible and neither measured here: the 35B fp8 tower's upload is missing from load accounting, and its device pages are never re-tagged, which is the shape of the GB10 weight-residency ATS penalty. The first is BOUNDED, and #974 says so: `load_stats` has exactly one non-test consumer, -`PrintLoadBytes` (`src/vllm/entrypoints/model_loader.cpp:133`), a stderr -diagnostic line behind `LoadStatsEnabled()`. Nothing allocates, sizes or +`src/vllm/entrypoints/model_loader.cpp::PrintLoadBytes`, a stderr +diagnostic line behind `src/vllm/entrypoints/model_loader.cpp::LoadStatsEnabled`. Nothing allocates, sizes or schedules on that counter, so the under-report costs the accuracy of a diagnostic, not a decision. The second consequence carries no such bound. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dd2924ed8..4b5acc689 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -290,6 +290,22 @@ jobs: tests.scripts.test_check_commit_trailers python3 tests/scripts/test_check_prompt_contract.py python3 tests/scripts/test_agent_gates.py + - name: A citation that names a symbol still finds it (#1143, #1139) + # The tree already carried 539 `path::Symbol` citations and had never + # checked one. Line anchors are what rot: one 45-line insertion near the + # top of `model_loader.cpp` moved 203 references at once, in files that + # change never opened (#1143), and a pin advance left three upstream + # line anchors on one row pointing at unrelated code (#1139). + # + # NOT the #911 shape. The expectation is the symbol name written by the + # citing author and read from the CITING file; only the evidence comes + # from the cited one. `--self-test` sweeps the fixture corpus in both + # directions. The upstream half needs an oracle checkout this runner + # does not have, so it stays opt-in behind `--upstream-root`. + run: | + python3 scripts/check-symbol-anchors.py + python3 scripts/check-symbol-anchors.py --self-test + python3 tests/scripts/test_check_symbol_anchors.py - name: NOW.md stays a short one-Read resume surface run: | python3 scripts/check-now-current.py diff --git a/docs/USAGE.md b/docs/USAGE.md index c34b6d9d0..3c789f2aa 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -213,6 +213,18 @@ welcome that the agent should relay. An explicit request can use claim action, rerun it after declaration, then run `scripts/agent-preflight.sh`. The entrypoint is non-interactive and does not mutate the checkout. +`scripts/agent-preflight.sh` now also runs `scripts/check-symbol-anchors.py`, +which reads every citation written as `` `path/to/file.cpp::SymbolName` `` and +requires that the file it names still contains that symbol. Write citations in +that form rather than as `file.cpp:412`: a line number is a coordinate into a +moving file, so an edit anywhere above it retargets the citation in files the +edit never opened. Add `--upstream-root ` to ask the same +question of the pinned oracle; that run is opt-in, because CI has no checkout to +resolve upstream paths against. Both runs print every bucket they left out -- +frozen files, untracked files, upstream paths -- and refuse a checked count +below the recorded floor, so a run that quietly stopped examining anything +cannot report as a pass. + The operator role is a coordinator, and **several may run at once**: `scripts/agent-role.py claim operator` records this worktree and is never refused, `scripts/agent-role.py show` lists the other live coordinators, and diff --git a/include/vllm/v1/core/sched/scheduler.h b/include/vllm/v1/core/sched/scheduler.h index 2a2dfafa4..3d7bd72b9 100644 --- a/include/vllm/v1/core/sched/scheduler.h +++ b/include/vllm/v1/core/sched/scheduler.h @@ -140,9 +140,9 @@ class Scheduler { // VIRTUAL destructor — REQUIRED, not cosmetic. `AsyncScheduler` derives from // this class and production/test code owns the derived object through a - // `std::unique_ptr` base pointer (src/vllm/entrypoints/ - // model_loader.cpp:176 `MakeScheduler`, include/vllm/entrypoints/ - // model_loader.h:206 `scheduler_`, tests/vllm/v1/ + // `std::unique_ptr` base pointer + // (`src/vllm/entrypoints/model_loader.cpp::MakeScheduler`, + // `include/vllm/entrypoints/model_loader.h::scheduler_`, tests/vllm/v1/ // test_async_admission_timing.cpp:167-170). Deleting a derived object through // a base pointer whose destructor is non-virtual is UNDEFINED BEHAVIOUR // ([expr.delete]/3) — the derived destructor never runs. GCC does not diff --git a/scripts/agent-preflight.sh b/scripts/agent-preflight.sh index 2d20f45c2..971da0950 100755 --- a/scripts/agent-preflight.sh +++ b/scripts/agent-preflight.sh @@ -100,6 +100,7 @@ CHECKERS=( check-oracle-pins check-now-current check-gate-commands + check-symbol-anchors ) SUITES=( @@ -148,6 +149,7 @@ SUITES=( test_gpu_lock_one_truth test_main_baseline test_agent_preflight_skip_report + test_check_symbol_anchors ) failed=() diff --git a/scripts/check-pr-size.py b/scripts/check-pr-size.py index 5af519b36..7dcef4689 100755 --- a/scripts/check-pr-size.py +++ b/scripts/check-pr-size.py @@ -317,6 +317,12 @@ # reduced one. "scripts/check-container-matrix.py": DISABLED_CREATION_CHECKER, "scripts/check-container-workflow.py": DISABLED_CREATION_CHECKER, + # GATE-SYMBOL-ANCHORS (#1143). Created here, so there is no BASE version to + # mutate. The empty stub exits 0 and prints nothing, which fails 20 of the + # 21 cases in tests/scripts/test_check_symbol_anchors.py -- including the + # clean-tree case, which asserts a checked count at or above the recorded + # floor and so cannot be satisfied by silence. + "scripts/check-symbol-anchors.py": DISABLED_CREATION_CHECKER, # 2026-08-16: the CUDA arch-gate registration guard (#960). Created in the # same PR, so there is no BASE version to mutate; its own suite loads the # checker as a module and calls into it, so the disabled stub fails at import diff --git a/scripts/check-symbol-anchors.py b/scripts/check-symbol-anchors.py new file mode 100644 index 000000000..ba8dd7485 --- /dev/null +++ b/scripts/check-symbol-anchors.py @@ -0,0 +1,579 @@ +#!/usr/bin/env python3 +"""GATE-SYMBOL-ANCHORS (#1143, #1139) — a citation that names a symbol must +still find it. + +WHY THIS EXISTS. Cross-file citations in this tree are written `file.cpp:412`. +A line number is a coordinate into a moving file, so an edit ANYWHERE above the +cited line silently retargets every citation below it, in files the editing +change never opens. #1143 measured the blast radius on one file: +`src/vllm/entrypoints/model_loader.cpp` is cited by line from 109 sites across +45 files, and a single 45-line insertion near its top moved 203 of those +references at once. #1139 is the same defect one scale up, on an upstream +anchor: three `vllm/v1/worker/**` line anchors on one roadmap row all pointed at +unrelated code once the parity pin advanced, and two of them had already been +COPIED into a header and a spec, so the staleness was propagating. + +The convention this gate enforces is the fix, not the symptom: cite the SYMBOL. +Write `` `src/vllm/entrypoints/model_loader.cpp::ResolveAutoDevice` `` instead of +`` `model_loader.cpp:100-115` ``. A symbol survives every edit that does not +rename it, `git grep` finds it, and a rename is exactly the moment a reader +WANTS the citation to break. + +WHY IT IS NOT A TAUTOLOGY. #911 burned this repo with an anchor checker that +read its expectation out of the file it was checking — it reported 27/27 FRESH +while five anchors pointed at unrelated code, because "line 504 exists" is true +of any file with 504 lines. Here the two sides come from DIFFERENT places: + + * the EXPECTATION is the symbol name, written by the citing author, and it + lives in the CITING file; + * the EVIDENCE is the cited file's text. + +No expectation is derived from the cited file, and nothing is stored in a shared +table. That also keeps the surface off the lock list AGENTS.md `## Records` +names: the expectation rides in the file that owns the claim, so N concurrent +pull requests never write one file to add N citations. + +WHAT IT DOES NOT DO, so nobody cites it for more than it delivers: + + * It does not check that the symbol is DEFINED in the cited file, only that + the file contains that identifier as a whole word. That is deliberate. + "`ModelRegistry::Load`, which `model_loader.cpp` calls" is a legitimate and + common citation, and a definition-only rule would reject it. A rename still + reds, because a rename removes the token from the file. + * It does not verify LINE citations. There is no honest way to: a bare line + number carries no claim to check against. Converting one is a per-citation + judgement about intent that no script can make, which is why #1143 refused + to have a script rewrite all 109 sites. + * By default it checks only IN-REPO citations. Upstream anchors need the + pinned oracle checkout, which CI does not have. `--upstream-root` turns that + half on for a box that does have one; see below. + * It never reads the network. + +UPSTREAM MODE (`--upstream-root `). Resolves citations +whose path starts with `vllm/` against the PINNED revision, read with +`git show :` so the checkout's working tree cannot substitute a +different revision for the pin. The pin comes from the ```parity-pin block in +`.agents/upstream-sync.md`; the checkout is asserted to CONTAIN that commit. +This mode is not a CI gate — it is the instrument that would have found #1139 +before a reader did. + +Run with `--self-test` to sweep the FIXTURES corpus below in both directions: +every `stale=True` fixture must be reported and every `stale=False` one must not. +""" + +from __future__ import annotations + +import argparse +import re +import subprocess +import sys +from dataclasses import dataclass +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] +UPSTREAM_SYNC = ROOT / ".agents/upstream-sync.md" + +# A citation is a single backtick span holding a path with a source extension, +# `::`, and a (possibly qualified) identifier. The extension requirement is what +# keeps ordinary C++ prose out: `Qwen3_5MTPKind::kMoe` has no path in front of +# it and never matches. +# +# The leading class admits a DOT. It did not, and every path beginning with one +# therefore matched zero times and said nothing about it -- so a citation of +# a spec or a workflow by symbol -- a dot-leading path -- was silently unchecked. +# That was latent while nobody wrote one, and `.agents/porting.md` now invites +# exactly that. +CITATION_RE = re.compile( + r"`([A-Za-z0-9_.][A-Za-z0-9_./+-]*\.(?:cpp|cc|h|hpp|cu|cuh|py|sh|md))" + r"::([A-Za-z_][A-Za-z0-9_]*(?:::[A-Za-z_~][A-Za-z0-9_]*)*)`" +) + +# Files whose citations are scanned. Everything else in the tree is binary, +# generated, or carries no prose. +SCAN_SUFFIXES = { + ".md", ".cpp", ".cc", ".h", ".hpp", ".cu", ".cuh", ".py", ".sh", ".yml", ".txt", +} + +# Skipped by PREFIX, and counted so the skip is visible rather than silent. +# +# `.agents/completed/` is the frozen archive: AGENTS.md `## Records` says moved +# detail keeps its provenance, and rewriting an archived citation would forge +# what a past session wrote. `.agents/issue-index.md` is append-only by rule -- +# a row there may never be edited -- so a stale anchor inside one is unrepairable +# by construction and gating it would be a gate nobody may satisfy. +FROZEN_PREFIXES = (".agents/completed/",) +FROZEN_FILES = (".agents/issue-index.md",) + +# A path claims to be OURS when its DIRECTORY exists in this tree -- not when it +# starts with a familiar root. `tests/` was the first cut and it was wrong: vLLM +# has a `tests/` too, so `tests/kernels/attention/test_cache.py::...` is an +# upstream anchor that a root-prefix rule reports as a broken local path. Twenty +# citations went red that way on the first run, every one of them upstream and +# correct. Directory existence separates the two cases without a list to +# maintain: we have `tests/vllm/` and `tests/parity/`, we do not have +# `tests/kernels/`, and a typo inside a directory we DO have still reds. +# +# The directory must be at least TWO components deep. `tests/test_config.py` and +# `tests/test_flash_attn.py` are real upstream files sitting directly under a +# top-level name we share, and nothing about the path distinguishes them from a +# local file that lost its directory. Depth is the cheap discriminator; the cost +# is that a typo in a ONE-component directory falls through to the upstream +# bucket instead of reporting, which is a miss, never a false accusation. + +# Bare-basename resolution is allowed only for our own source extensions. Every +# upstream oracle in the table is Python, so admitting a bare `.py` basename +# would let an upstream citation resolve onto an unrelated local script and go +# red for the wrong reason. +BASENAME_SUFFIXES = {".cpp", ".cc", ".h", ".hpp", ".cu", ".cuh"} + +# The recorded floor on the in-repo population. `checked == 0` alone is a mute +# switch: one added FROZEN_PREFIXES entry, or a narrowed CITATION_RE, takes the +# count from 91 to 1 and a zero-guard stays green the whole way down. This is a +# ratchet with headroom, not a per-PR record lock -- no change has to edit it, +# and it is raised only when the population has grown durably. +# +# It applies to THIS tree. A `--root` fixture holds one citation by design, so +# the floor there is `--min-checked` if given and 1 otherwise; the zero-guard +# still covers every root. +MIN_IN_REPO_CHECKED = 85 + + +@dataclass(frozen=True) +class Citation: + source: str + line: int + path: str + symbol: str + + def render(self) -> str: + return f"{self.source}:{self.line}: `{self.path}::{self.symbol}`" + + +@dataclass +class Counts: + scanned_files: int = 0 + frozen_files: int = 0 + untracked_with_citations: int = 0 + citations: int = 0 + checked: int = 0 + fresh: int = 0 + stale: int = 0 + missing_local: int = 0 + ambiguous: int = 0 + upstream: int = 0 + upstream_checked: int = 0 + upstream_fresh: int = 0 + upstream_stale: int = 0 + upstream_missing: int = 0 + + +def tracked_files(root: Path) -> list[str]: + result = subprocess.run( + ["git", "-C", str(root), "ls-files"], + stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, + text=True, + check=False, + ) + if result.returncode != 0: + return sorted( + p.relative_to(root).as_posix() + for p in root.rglob("*") + if p.is_file() and ".git/" not in p.as_posix() + ) + return sorted(result.stdout.split()) + + +def untracked_files(root: Path) -> list[str]: + """Files git can see but has not been told about.""" + + result = subprocess.run( + ["git", "-C", str(root), "ls-files", "--others", "--exclude-standard"], + stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, + text=True, + check=False, + ) + if result.returncode != 0: + return [] + return sorted(result.stdout.split()) + + +def count_untracked_citations(root: Path) -> int: + """How many UNTRACKED files carry a citation this run will not see. + + This change reported the blind spot about itself and then closed it only for + its own fixtures: the first version of the test file wrote them as real + local paths, was untracked, and ran green -- `git ls-files` does not list an + untracked file, so neither its citations nor its existence as a cited PATH + reach the walk. CI is sound because everything is tracked there. The local + pre-commit run is not, and a green run before `git add` is not a green run. + The skip is not closed here, because scanning the working tree would change + what the gate's subject IS. It is COUNTED and PRINTED, the same discipline + `frozen_files` already gets: a gate that cannot say how many things it left + out has not reported. + """ + + total = 0 + for rel in untracked_files(root): + if rel.startswith(FROZEN_PREFIXES) or rel in FROZEN_FILES: + continue + path = root / rel + if path.suffix not in SCAN_SUFFIXES or not path.is_file(): + continue + text = path.read_text(encoding="utf-8", errors="replace") + if "::" in text and CITATION_RE.search(text): + total += 1 + return total + + +def evidence(body: str) -> str: + """The cited file's text with its OWN citation spans removed. + + A citation span is a claim about some file, never a definition or a call, so + it may not stand as evidence for one. Leaving it in reopened #911 for a + self-citation: when the citing and cited files are the SAME file, the + expectation is read from the file under test and the citation text supplies + its own evidence, so `` `a.cpp::GhostSymbol` `` written inside `a.cpp` read + as fresh over a symbol that exists nowhere else. Stripping every citation + span, not only the self-citing one, also closes the cross-file form, where + `a.cpp` names `Ghost` only inside its citation of `b.cpp`. + """ + + return CITATION_RE.sub(" ", body) + + +def contains_symbol(body: str, symbol: str) -> bool: + """Whole-word search for the FULL cited symbol, qualifiers included.""" + + pattern = r"(? list[Citation]: + found: list[Citation] = [] + for rel in tracked_files(root): + if rel.startswith(FROZEN_PREFIXES) or rel in FROZEN_FILES: + counts.frozen_files += 1 + continue + path = root / rel + if path.suffix not in SCAN_SUFFIXES or not path.is_file(): + continue + counts.scanned_files += 1 + text = path.read_text(encoding="utf-8", errors="replace") + if "::" not in text: + continue + for number, line in enumerate(text.splitlines(), 1): + for match in CITATION_RE.finditer(line): + found.append(Citation(rel, number, match.group(1), match.group(2))) + counts.citations = len(found) + return found + + +def resolve_local( + cite: Citation, + tracked: set[str], + by_base: dict[str, list[str]], + root: Path, +) -> tuple[str, list[str]]: + """Return (verdict, candidates) for one citation's PATH half.""" + + if cite.path in tracked: + return "resolved", [cite.path] + base = cite.path.rsplit("/", 1)[-1] + parent = cite.path.rsplit("/", 1)[0] if "/" in cite.path else "" + if "/" in parent and (root / parent).is_dir(): + return "missing_local", [] + if Path(base).suffix not in BASENAME_SUFFIXES: + return "upstream", [] + candidates = by_base.get(base, []) + if len(candidates) == 1: + return "resolved", candidates + if len(candidates) > 1: + return "ambiguous", candidates + return "upstream", [] + + +def check_tree(root: Path) -> tuple[Counts, list[str]]: + counts = Counts() + errors: list[str] = [] + tracked = set(tracked_files(root)) + # SORTED, and over a sorted walk. `tracked` is a set, so an unsorted walk + # gives each basename's candidate list an arbitrary order -- which decides + # which path a stale ambiguous citation NAMES, and made one mutation catch + # depend on the iteration order of a hash set. + by_base: dict[str, list[str]] = {} + for rel in sorted(tracked): + by_base.setdefault(rel.rsplit("/", 1)[-1], []).append(rel) + + for cite in collect(root, counts): + verdict, candidates = resolve_local(cite, tracked, by_base, root) + if verdict == "missing_local": + counts.missing_local += 1 + errors.append( + f"{cite.render()}: names a path under this repository that does not exist" + ) + continue + if verdict == "upstream": + counts.upstream += 1 + continue + # An ambiguous basename used to be counted and dropped, so five live + # citations were skipped for naming a file that exists twice. A + # basename is checked against EVERY candidate instead: one of them + # containing the symbol is what the citation claims, and reporting only + # when NONE does keeps the ambiguity from becoming a false accusation. + if verdict == "ambiguous": + counts.ambiguous += 1 + counts.checked += 1 + hit = next( + ( + rel + for rel in candidates + if contains_symbol( + evidence((root / rel).read_text(encoding="utf-8", errors="replace")), + cite.symbol, + ) + ), + None, + ) + if hit is not None: + counts.fresh += 1 + else: + counts.stale += 1 + where = ( + candidates[0] + if len(candidates) == 1 + else f"no file named `{cite.path}` ({len(candidates)} candidates)" + ) + errors.append( + f"{cite.render()}: {where} does not contain `{cite.symbol}`" + ) + counts.untracked_with_citations = count_untracked_citations(root) + return counts, errors + + +def parity_pin(sync_path: Path) -> str: + text = sync_path.read_text(encoding="utf-8", errors="replace") + block = re.search(r"```parity-pin\n(.*?)```", text, re.S) + if block is None: + raise ValueError(f"{sync_path}: no ```parity-pin block") + pin = re.search(r"^vllm_commit\s*=\s*([0-9a-f]{7,40})\s*$", block.group(1), re.M) + if pin is None: + raise ValueError(f"{sync_path}: parity-pin block has no vllm_commit") + return pin.group(1) + + +def check_upstream(root: Path, upstream_root: Path, counts: Counts) -> list[str]: + """Verify `vllm/...::Symbol` citations against the PINNED revision.""" + + errors: list[str] = [] + pin = parity_pin(root / ".agents/upstream-sync.md") + identity = subprocess.run( + ["git", "-C", str(upstream_root), "cat-file", "-t", pin], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + check=False, + ) + if identity.returncode != 0 or identity.stdout.strip() != "commit": + raise ValueError( + f"{upstream_root}: does not contain the parity pin {pin}; " + "an unpinned checkout is not the oracle" + ) + print(f"upstream pin asserted present: {pin}") + + cache: dict[str, str | None] = {} + seen: set[tuple[str, str]] = set() + for cite in collect(root, Counts()): + if not cite.path.startswith("vllm/"): + continue + counts.upstream_checked += 1 + key = (cite.path, cite.symbol) + if cite.path not in cache: + blob = subprocess.run( + ["git", "-C", str(upstream_root), "show", f"{pin}:{cite.path}"], + stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, + text=True, + check=False, + ) + cache[cite.path] = blob.stdout if blob.returncode == 0 else None + body = cache[cite.path] + if body is None: + counts.upstream_missing += 1 + if key not in seen: + errors.append(f"{cite.render()}: absent from vLLM at {pin}") + elif contains_symbol(evidence(body), cite.symbol): + counts.upstream_fresh += 1 + else: + counts.upstream_stale += 1 + if key not in seen: + errors.append( + f"{cite.render()}: {cite.path} at {pin} does not contain `{cite.symbol}`" + ) + seen.add(key) + return errors + + +# (name, citing text, cited path, cited body, stale[, the citing file IS the +# cited file]). The last field defaults to False, which puts the citation in a +# separate `note.md`. +FIXTURES = ( + ("fresh", "`alpha/beta/a.cpp::Widget`", "alpha/beta/a.cpp", "struct Widget {};\n", False), + ( + "self-citation is not its own evidence", + "`alpha/beta/a.cpp::GhostSymbol`", + "alpha/beta/a.cpp", + "struct Widget {};\n", + True, + True, + ), + ( + "self-citation of a symbol that IS there", + "`alpha/beta/a.cpp::Widget`", + "alpha/beta/a.cpp", + "struct Widget {};\n", + False, + True, + ), + ( + "a citation in the cited file is not evidence", + "`alpha/beta/a.cpp::Ghost`", + "alpha/beta/a.cpp", + "// see `alpha/beta/b.cpp::Ghost`\n", + True, + ), + ("renamed", "`alpha/beta/a.cpp::Widget`", "alpha/beta/a.cpp", "struct Gadget {};\n", True), + ("call site counts", "`alpha/beta/a.cpp::Load`", "alpha/beta/a.cpp", " Registry::Load(x);\n", False), + ("qualified", "`alpha/beta/a.cpp::Registry::Load`", "alpha/beta/a.cpp", " Registry::Load(x);\n", False), + ( + "qualified miss", + "`alpha/beta/a.cpp::Registry::Load`", + "alpha/beta/a.cpp", + " Other::Load(x);\n", + True, + ), + ("prefix is not a word", "`alpha/beta/a.cpp::Load`", "alpha/beta/a.cpp", " LoadShards(x);\n", True), + ("suffix is not a word", "`alpha/beta/a.cpp::Shards`", "alpha/beta/a.cpp", " LoadShards(x);\n", True), +) + + +def self_test() -> int: + import tempfile + + failures: list[str] = [] + for fixture in FIXTURES: + name, citing, cited_path, cited_body, stale = fixture[:5] + self_cite = fixture[5] if len(fixture) > 5 else False + with tempfile.TemporaryDirectory() as raw: + box = Path(raw) + (box / "alpha/beta").mkdir(parents=True, exist_ok=True) + if self_cite: + (box / cited_path).write_text( + f"// see {citing} for the shape\n{cited_body}", encoding="utf-8" + ) + else: + (box / cited_path).write_text(cited_body, encoding="utf-8") + (box / "note.md").write_text( + f"see {citing} for the shape\n", encoding="utf-8" + ) + counts, errors = check_tree(box) + if counts.checked != 1: + failures.append(f"{name}: checked={counts.checked}, expected 1") + continue + reported = counts.stale == 1 + if reported != stale: + failures.append( + f"{name}: stale={reported}, expected {stale} ({errors})" + ) + print(f"self-test fixtures: {len(FIXTURES)}, failures: {len(failures)}") + for failure in failures: + print(f" {failure}") + return 1 if failures else 0 + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--root", default=str(ROOT)) + parser.add_argument( + "--upstream-root", + default=None, + help="path to a vLLM checkout containing the parity pin; enables upstream mode", + ) + parser.add_argument("--self-test", action="store_true") + parser.add_argument( + "--min-checked", + type=int, + default=None, + help="floor on the in-repo checked count; defaults to the recorded " + "MIN_IN_REPO_CHECKED for this tree and to 1 for a --root fixture", + ) + args = parser.parse_args() + + if args.self_test: + return self_test() + + root = Path(args.root).resolve() + counts, errors = check_tree(root) + + if args.min_checked is not None: + floor = args.min_checked + else: + floor = MIN_IN_REPO_CHECKED if root == ROOT else 1 + + buckets = counts.checked + counts.upstream + counts.missing_local + print( + f"symbol anchors: {counts.citations} citations in {counts.scanned_files} scanned files " + f"({counts.frozen_files} frozen files skipped, " + f"{counts.untracked_with_citations} untracked files carrying citations NOT scanned)" + ) + print( + f" in-repo checked {counts.checked} (fresh {counts.fresh}, stale {counts.stale}); " + f"upstream/unknown {counts.upstream}; " + f"of which ambiguous basename {counts.ambiguous}; " + f"missing local path {counts.missing_local}" + ) + print(f" buckets sum {buckets} vs {counts.citations} citations; floor {floor}") + + if counts.checked == 0: + errors.append( + "0 in-repo citations were checked. The tree carries symbol anchors, so a " + "zero here means the citation grammar or the file walk stopped matching, " + "not that everything is fresh." + ) + elif counts.checked < floor: + errors.append( + f"in-repo checked {counts.checked} is below the recorded floor {floor}. " + "A population that collapses is the grammar or the file walk failing, " + "not the tree losing citations -- a floor of zero over a population of " + "ninety is a mute switch." + ) + + if buckets != counts.citations: + errors.append( + f"buckets sum to {buckets} but {counts.citations} citations were found. " + "Every citation must land in exactly one of checked / upstream / " + "missing, or a citation has stopped being counted anywhere." + ) + + if args.upstream_root is not None: + errors.extend(check_upstream(root, Path(args.upstream_root).resolve(), counts)) + print( + f" upstream checked {counts.upstream_checked} " + f"(fresh {counts.upstream_fresh}, stale {counts.upstream_stale}, " + f"file absent {counts.upstream_missing})" + ) + + if errors: + print() + for error in errors: + print(f"FAIL {error}") + print(f"\n{len(errors)} stale or unresolvable symbol anchor(s)") + return 1 + print("OK every symbol anchor still finds what it names") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/src/vllm/multimodal/ltx2_video.cpp b/src/vllm/multimodal/ltx2_video.cpp index d31dc1ad6..e3020ecda 100644 --- a/src/vllm/multimodal/ltx2_video.cpp +++ b/src/vllm/multimodal/ltx2_video.cpp @@ -723,9 +723,9 @@ std::unique_ptr Ltx2VideoEngine::Load(const VideoModelParams& p // // WHICH accelerator is the PLATFORM's question, not this model file's. This // asked `TryGetBackend(kCUDA)` — the same defect work row M3a repaired in - // `SelectQueueForModel` (src/vllm/entrypoints/model_loader.cpp:75-104 — the - // full path matters, there is also a src/vllm/model_executor/model_loader/ - // DIRECTORY and the bare file name sends a reader there), where a hardcoded + // `src/vllm/entrypoints/model_loader.cpp::SelectQueueForModel` (the full path + // matters, there is also a src/vllm/model_executor/model_loader/ DIRECTORY and + // the bare file name sends a reader there), where a hardcoded // `GetBackend(kCUDA)` was the one line standing between a complete non-NVIDIA // backend and running a model. `CurrentPlatform()` walks the probe order // {kCUDA, kROCM, kXPU, kVULKAN, kMETAL, kTENSTORRENT, kCPU} diff --git a/src/vllm/multimodal/minimax_h3_video.cpp b/src/vllm/multimodal/minimax_h3_video.cpp index 65a69cda0..936fbb8a7 100644 --- a/src/vllm/multimodal/minimax_h3_video.cpp +++ b/src/vllm/multimodal/minimax_h3_video.cpp @@ -224,8 +224,8 @@ void FillNoise(std::vector& out, uint64_t seed) { // `device` is the public video ABI's selector: 0 is the CPU, 1 is "the // accelerator". WHICH accelerator is the PLATFORM's question, not this model // file's — the same question `ltx2_video.cpp:609-657` (the comment through the -// end of the capability refusal) and `SelectQueueForModel`'s auto arm -// (src/vllm/entrypoints/model_loader.cpp:76-104) ask. +// end of the capability refusal) and the auto arm of +// `src/vllm/entrypoints/model_loader.cpp::SelectQueueForModel` ask. // // This used to be `static_cast(device)`, which is not a mapping // at all. It reads the ABI selector AS AN ENUM VALUE and is correct only for as diff --git a/tests/parity/test_qwen36_spec_decode.cpp b/tests/parity/test_qwen36_spec_decode.cpp index 2bd7f6755..c20a31f39 100644 --- a/tests/parity/test_qwen36_spec_decode.cpp +++ b/tests/parity/test_qwen36_spec_decode.cpp @@ -7,7 +7,8 @@ // speculative decoding turned ON via EngineParams::speculative_config // ('{"method":"mtp","num_speculative_tokens":1}'). The 35B draft head is the MoE // MTP layer (Qwen3_5MTPKind::kMoe: 256 routed experts top-8 + shared expert), -// selected automatically by model_loader.cpp:582 for the MoE target; the GDN +// selected automatically for the MoE target by +// `src/vllm/entrypoints/model_loader.cpp::is_dense_model`; the GDN // verify/rollback path is shared bit-exactly with the 27B (I4/I5a). Asserts: // // (a) == (b) THREE-WAY IDENTITY. Greedy spec-decode is exactness-preserving, so diff --git a/tests/scripts/test_check_pr_size.py b/tests/scripts/test_check_pr_size.py index b05f92c41..1b5257573 100755 --- a/tests/scripts/test_check_pr_size.py +++ b/tests/scripts/test_check_pr_size.py @@ -493,6 +493,12 @@ def test_every_created_checker_has_closed_bootstrap_evidence(self) -> None: # 2026-08-16: the CUDA arch-gate registration guard (#960). Its suite # reaches into the checker's parser, so the disabled stub cannot load. "scripts/check-cuda-op-arch-gate.py", + # 2026-08-18: the symbol-anchor freshness gate (#1143, #1139). Created + # in the same range, so it has no BASE version to mutate. The disabled + # stub exits 0 and prints nothing, which fails 20 of its 21 cases -- + # including the clean-tree case, which asserts a checked count at or + # above the recorded floor and so cannot be satisfied by silence. + "scripts/check-symbol-anchors.py", } self.assertEqual(set(checker.CREATION_MUTATIONS), expected) for path, mutation in checker.CREATION_MUTATIONS.items(): diff --git a/tests/scripts/test_check_symbol_anchors.py b/tests/scripts/test_check_symbol_anchors.py new file mode 100644 index 000000000..1aa31baa6 --- /dev/null +++ b/tests/scripts/test_check_symbol_anchors.py @@ -0,0 +1,345 @@ +#!/usr/bin/env python3 +"""Unit and mutation checks for scripts/check-symbol-anchors.py (#1143, #1139). + +Fixture paths are `alpha/beta/...`, not `src/vllm/...`, and that is load-bearing. +This file is scanned by the checker it tests, so a fixture written as a real +local path is a real citation of a file that does not exist -- twelve of them, +which is how the first committed version reported itself red. `alpha/` exists +in no tree, so the fixtures fall into the skipped upstream bucket in the real +repository while still resolving inside each temporary one. + +The case this suite exists to pin is `test_verdict_depends_on_the_citing_text`. +#911 shipped an anchor checker that read its expectation out of the file it was +checking and reported 27/27 FRESH while five anchors pointed at unrelated code. +A checker of that shape cannot distinguish two citations that name DIFFERENT +symbols in the SAME file, because nothing it reads varies. This one must, and +that case asserts it does. +""" + +from __future__ import annotations + +import re +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +CHECKER = ROOT / "scripts/check-symbol-anchors.py" +CHECKED_RE = re.compile(r"in-repo checked (\d+) \(fresh (\d+), stale (\d+)\)") + + +def run(*args: str) -> subprocess.CompletedProcess: + return subprocess.run( + [sys.executable, str(CHECKER), *args], + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=True, + check=False, + ) + + +class Tree: + """A throwaway tree with one cited file and one citing file.""" + + def __init__(self, cited_path: str, cited_body: str, citing_body: str, + citing_path: str = "note.md"): + self.box = tempfile.TemporaryDirectory() + root = Path(self.box.name) + for rel, body in ((cited_path, cited_body), (citing_path, citing_body)): + target = root / rel + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(body, encoding="utf-8") + self.root = root + + def __enter__(self) -> Path: + return self.root + + def __exit__(self, *exc) -> None: + self.box.cleanup() + + +def init_repo(root: Path, tracked: list[str]) -> None: + """Make `root` a real repository with exactly `tracked` in the index. + + The untracked case needs one: outside a repository `git ls-files` fails and + the checker falls back to walking the filesystem, where nothing is untracked + and the blind spot cannot exist. + """ + + env = {"GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@t", + "GIT_COMMITTER_NAME": "t", "GIT_COMMITTER_EMAIL": "t@t", "PATH": "/usr/bin:/bin"} + subprocess.run(["git", "-C", str(root), "init", "-q"], check=True, env=env) + subprocess.run(["git", "-C", str(root), "add", *tracked], check=True, env=env) + subprocess.run(["git", "-C", str(root), "commit", "-qm", "seed"], check=True, env=env) + + +class SymbolAnchorTests(unittest.TestCase): + def test_self_test_sweeps_a_non_empty_corpus(self): + result = run("--self-test") + self.assertEqual(result.returncode, 0, result.stdout) + match = re.search(r"self-test fixtures: (\d+), failures: (\d+)", result.stdout) + self.assertIsNotNone(match, result.stdout) + self.assertGreater(int(match.group(1)), 0, "a corpus of zero fixtures proves nothing") + self.assertEqual(int(match.group(2)), 0, result.stdout) + + def test_this_tree_is_green_over_a_non_zero_population(self): + result = run() + self.assertEqual(result.returncode, 0, result.stdout) + match = CHECKED_RE.search(result.stdout) + self.assertIsNotNone(match, result.stdout) + self.assertGreater( + int(match.group(1)), 0, + "a green run that checked nothing is a skip wearing a pass", + ) + self.assertEqual(int(match.group(3)), 0, result.stdout) + + def test_fresh_symbol_passes(self): + with Tree("alpha/beta/a.cpp", "struct Widget {};\n", + "see `alpha/beta/a.cpp::Widget`\n") as root: + result = run("--root", str(root)) + self.assertEqual(result.returncode, 0, result.stdout) + self.assertIn("in-repo checked 1 (fresh 1, stale 0)", result.stdout) + + def test_renamed_symbol_is_reported(self): + with Tree("alpha/beta/a.cpp", "struct Gadget {};\n", + "see `alpha/beta/a.cpp::Widget`\n") as root: + result = run("--root", str(root)) + self.assertEqual(result.returncode, 1, result.stdout) + self.assertIn("does not contain `Widget`", result.stdout) + self.assertIn("note.md:1", result.stdout) + + def test_verdict_depends_on_the_citing_text(self): + """The anti-tautology case (#911). + + One cited file, unchanged, cited twice. The two verdicts must differ, + which is only possible if the expectation comes from the CITING side. + """ + + body = "struct Widget {};\n" + with Tree("alpha/beta/a.cpp", body, "`alpha/beta/a.cpp::Widget`\n") as root: + good = run("--root", str(root)) + with Tree("alpha/beta/a.cpp", body, "`alpha/beta/a.cpp::Gadget`\n") as root: + bad = run("--root", str(root)) + self.assertEqual(good.returncode, 0, good.stdout) + self.assertEqual(bad.returncode, 1, bad.stdout) + self.assertIn("fresh 1, stale 0", good.stdout) + self.assertIn("fresh 0, stale 1", bad.stdout) + + def test_a_call_site_satisfies_a_citation(self): + with Tree("alpha/beta/a.cpp", " ModelRegistry::Load(config);\n", + "`alpha/beta/a.cpp::ModelRegistry::Load`\n") as root: + result = run("--root", str(root)) + self.assertEqual(result.returncode, 0, result.stdout) + + def test_a_longer_identifier_does_not_satisfy_a_shorter_citation(self): + with Tree("alpha/beta/a.cpp", " LoadShards(dir);\n", + "`alpha/beta/a.cpp::Load`\n") as root: + result = run("--root", str(root)) + self.assertEqual(result.returncode, 1, result.stdout) + + def test_a_local_path_that_does_not_exist_is_reported(self): + with Tree("alpha/beta/a.cpp", "struct Widget {};\n", + "`alpha/beta/a.cpp::Widget` and `alpha/beta/gone.cpp::Widget`\n") as root: + result = run("--root", str(root)) + self.assertEqual(result.returncode, 1, result.stdout) + self.assertIn("names a path under this repository that does not exist", result.stdout) + + def test_an_upstream_path_is_bucketed_not_failed(self): + with Tree("alpha/beta/a.cpp", "struct Widget {};\n", + "`alpha/beta/a.cpp::Widget` and `vllm/config/model.py::ModelConfig`\n") as root: + result = run("--root", str(root)) + self.assertEqual(result.returncode, 0, result.stdout) + self.assertIn("upstream/unknown 1", result.stdout) + + def test_zero_checked_citations_is_a_failure(self): + with Tree("alpha/beta/a.cpp", "struct Widget {};\n", + "`vllm/config/model.py::ModelConfig` only\n") as root: + result = run("--root", str(root)) + self.assertEqual(result.returncode, 1, result.stdout) + self.assertIn("0 in-repo citations were checked", result.stdout) + + def test_the_frozen_archive_is_skipped_and_counted(self): + with Tree("alpha/beta/a.cpp", "struct Widget {};\n", + "`alpha/beta/a.cpp::Gadget` is stale here\n", + citing_path=".agents/completed/old/STATE-1.md") as root: + (root / "live.md").write_text("`alpha/beta/a.cpp::Widget`\n", encoding="utf-8") + result = run("--root", str(root)) + self.assertEqual(result.returncode, 0, result.stdout) + self.assertIn("1 frozen files skipped", result.stdout) + + def test_prose_without_a_path_is_not_a_citation(self): + with Tree("alpha/beta/a.cpp", "struct Widget {};\n", + "`Qwen3_5MTPKind::kMoe` and `alpha/beta/a.cpp::Widget`\n") as root: + result = run("--root", str(root)) + self.assertEqual(result.returncode, 0, result.stdout) + self.assertIn("1 citations", result.stdout) + self.assertIn("in-repo checked 1", result.stdout) + + def test_a_citation_span_is_not_its_own_evidence(self): + """A self-citation must not be satisfied by the citation text (#911 again). + + The immunity argument is "expectation from the CITING file, evidence + from the CITED file". When they are the SAME file those two sides + collapse: the checker searched the whole body, and the citation span + itself contains the symbol, so `GhostSymbol` -- which exists nowhere but + inside its own citation -- read as fresh. Zero of the tree's anchors are + self-citations today, and `.agents/porting.md` now tells authors to + write the full path, which makes a file documenting its own symbols the + most natural next thing anyone writes. + """ + + with Tree("alpha/beta/a.cpp", + "// see `alpha/beta/a.cpp::GhostSymbol` for details\n" + "struct Widget {};\n", + "`alpha/beta/a.cpp::Widget`\n") as root: + result = run("--root", str(root)) + self.assertEqual(result.returncode, 1, result.stdout) + self.assertIn("does not contain `GhostSymbol`", result.stdout) + + def test_a_citation_in_the_cited_file_is_not_evidence_either(self): + """The cross-file form of the same laundering. + + `a.cpp` names `Ghost` only inside a citation of a DIFFERENT file. That + span is someone else's claim, never a definition or a call, so it may + not satisfy a citation of `a.cpp`. + """ + + with Tree("alpha/beta/a.cpp", "// see `alpha/beta/b.cpp::Ghost`\n", + "`alpha/beta/a.cpp::Ghost`\n") as root: + result = run("--root", str(root)) + self.assertEqual(result.returncode, 1, result.stdout) + self.assertIn("does not contain `Ghost`", result.stdout) + + def test_untracked_files_carrying_citations_are_counted(self): + """The blind spot this change reported about itself, made visible. + + An untracked file is not in `git ls-files`, so its citations are never + scanned and its path never resolves. CI is safe because everything is + tracked there; a local pre-commit run is not. The skip is not closed -- + it is COUNTED, the same way `frozen_files` is. + """ + + with Tree("alpha/beta/a.cpp", "struct Widget {};\n", + "`alpha/beta/a.cpp::Widget`\n") as root: + init_repo(root, ["alpha/beta/a.cpp", "note.md"]) + (root / "scratch.md").write_text( + "`alpha/beta/a.cpp::Gadget`\n", encoding="utf-8") + result = run("--root", str(root)) + self.assertEqual(result.returncode, 0, result.stdout) + self.assertIn("1 untracked file", result.stdout) + + def test_the_floor_reds_when_the_population_collapses(self): + """A floor of zero is a mute switch. + + One added FROZEN_PREFIXES entry, or a narrowed CITATION_RE, drops the + in-repo population from 91 to 1 and a `checked == 0` guard stays green + the whole way down. + """ + + with Tree("alpha/beta/a.cpp", "struct Widget {};\n", + "`alpha/beta/a.cpp::Widget`\n") as root: + result = run("--root", str(root), "--min-checked", "80") + self.assertEqual(result.returncode, 1, result.stdout) + self.assertIn("below the recorded floor", result.stdout) + self.assertIn("80", result.stdout) + + def test_this_tree_meets_the_recorded_floor(self): + """The floor is asserted here with a LITERAL, not read from the checker. + + Reading the module's constant would make this a tautology: lowering the + floor would lower the expectation with it. + """ + + result = run() + self.assertEqual(result.returncode, 0, result.stdout) + match = CHECKED_RE.search(result.stdout) + self.assertIsNotNone(match, result.stdout) + self.assertGreaterEqual(int(match.group(1)), 85, result.stdout) + + def test_the_buckets_sum_to_the_citation_count(self): + """A floor below the real count is a mute switch unless the buckets sum. + + Every citation lands in exactly one of checked / upstream / missing, so + a citation that quietly stops being counted anywhere is arithmetic, not + judgement. + """ + + result = run() + self.assertEqual(result.returncode, 0, result.stdout) + total = int(re.search(r"symbol anchors: (\d+) citations", result.stdout).group(1)) + checked = int(CHECKED_RE.search(result.stdout).group(1)) + upstream = int(re.search(r"upstream/unknown (\d+)", result.stdout).group(1)) + missing = int(re.search(r"missing local path (\d+)", result.stdout).group(1)) + self.assertEqual(checked + upstream + missing, total, result.stdout) + self.assertIn(f"buckets sum {total} vs {total} citations", result.stdout) + + # This tree has zero missing local paths, so dropping THAT bucket from + # the sum leaves the arithmetic intact and the mute switch survives. + # One fixture tree populates all three at once. + with Tree("alpha/beta/a.cpp", "struct Widget {};\n", + "`alpha/beta/a.cpp::Widget` `alpha/beta/gone.cpp::Widget` " + "`vllm/config/model.py::ModelConfig`\n") as root: + three = run("--root", str(root)) + self.assertIn("in-repo checked 1", three.stdout) + self.assertIn("upstream/unknown 1", three.stdout) + self.assertIn("missing local path 1", three.stdout) + self.assertIn("buckets sum 3 vs 3 citations", three.stdout) + + def test_an_ambiguous_basename_is_checked_against_every_candidate(self): + """The symbol lives in the LAST candidate, not the first. + + Candidate order is sorted, so `alpha/beta/a.cpp` is consulted first and + does not carry `Widget`. A checker that stops at the first candidate + therefore reds here, deterministically. + """ + + with Tree("alpha/beta/a.cpp", "struct Other {};\n", + "`a.cpp::Widget`\n") as root: + (root / "alpha/gamma").mkdir(parents=True, exist_ok=True) + (root / "alpha/gamma/a.cpp").write_text("struct Widget {};\n", encoding="utf-8") + good = run("--root", str(root)) + with Tree("alpha/beta/a.cpp", "struct Other {};\n", + "`a.cpp::Ghost`\n") as root: + (root / "alpha/gamma").mkdir(parents=True, exist_ok=True) + (root / "alpha/gamma/a.cpp").write_text("struct Widget {};\n", encoding="utf-8") + bad = run("--root", str(root)) + self.assertEqual(good.returncode, 0, good.stdout) + self.assertIn("of which ambiguous basename 1", good.stdout) + self.assertEqual(bad.returncode, 1, bad.stdout) + self.assertIn("no file named `a.cpp`", bad.stdout) + + def test_a_dot_leading_path_is_a_citation(self): + """A dot-leading path used to match ZERO times, silently. + + Any citation of a file under `.agents/` or `.github/` was invisible to + the grammar, which is latent only until someone cites a spec by symbol. + """ + + with Tree(".alpha/beta/note.md", "the Widget seam\n", + "`.alpha/beta/note.md::Widget` and `.alpha/beta/gone.md::Widget`\n") as root: + result = run("--root", str(root)) + self.assertEqual(result.returncode, 1, result.stdout) + self.assertIn("in-repo checked 1", result.stdout) + self.assertIn("names a path under this repository that does not exist", result.stdout) + + def test_parity_pin_is_read_from_the_recorded_block(self): + sys.path.insert(0, str(ROOT / "scripts")) + try: + import importlib.util + + spec = importlib.util.spec_from_file_location("_csa", CHECKER) + module = importlib.util.module_from_spec(spec) + # `from __future__ import annotations` + @dataclass needs the module + # visible in sys.modules while its body executes. + sys.modules[spec.name] = module + spec.loader.exec_module(module) + pin = module.parity_pin(ROOT / ".agents/upstream-sync.md") + finally: + sys.path.pop(0) + self.assertRegex(pin, r"^[0-9a-f]{40}$") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/vllm/config/test_speculative_mtp_depth.cpp b/tests/vllm/config/test_speculative_mtp_depth.cpp index 336bf63c4..dac716eaa 100644 --- a/tests/vllm/config/test_speculative_mtp_depth.cpp +++ b/tests/vllm/config/test_speculative_mtp_depth.cpp @@ -73,7 +73,8 @@ TEST_CASE("the config type CARRIES a depth above 1, it never clamps it") { // The user-facing JSON seam (what --speculative-config and the C ABI parse) is // a pure shape step: it carries the user k without knowing the checkpoint's // n_predict. The loader hands exactly this value to ResolveMtp against the - // checkpoint's mtp_num_hidden_layers (model_loader.cpp:831). + // checkpoint's mtp_num_hidden_layers + // (`src/vllm/entrypoints/model_loader.cpp::ResolveMtp`). const SpeculativeConfig parsed = ParseSpeculativeConfigJson( R"({"method":"mtp","num_speculative_tokens":4})"); CHECK(parsed.method == "mtp"); diff --git a/tests/vllm/entrypoints/test_gguf_device_fit_reach.cpp b/tests/vllm/entrypoints/test_gguf_device_fit_reach.cpp index fff17d21f..31489f8f2 100644 --- a/tests/vllm/entrypoints/test_gguf_device_fit_reach.cpp +++ b/tests/vllm/entrypoints/test_gguf_device_fit_reach.cpp @@ -59,7 +59,7 @@ class HostBackend final : public vt::Backend { } // "A platform can be registered while CreateQueue still fails" is the reason // `SelectQueueForModel`'s AUTO arm wraps this call in a try/catch and falls - // back to CPU (`ResolveAutoDevice`, `model_loader.cpp:100-115`). This flag + // back to CPU (`src/vllm/entrypoints/model_loader.cpp::ResolveAutoDevice`). This flag // reproduces that box on a // host with no CUDA device, so the resolver the fit refusal reads can be // checked against the queue the load will actually run on. A flag rather than diff --git a/tests/vllm/entrypoints/test_loaded_engine_dense.cpp b/tests/vllm/entrypoints/test_loaded_engine_dense.cpp index f0542b5bc..c7925dec5 100644 --- a/tests/vllm/entrypoints/test_loaded_engine_dense.cpp +++ b/tests/vllm/entrypoints/test_loaded_engine_dense.cpp @@ -695,7 +695,8 @@ TEST_CASE("loaded_engine: an over-long prompt is REFUSED, not left waiting") { // ResolveNumBlocks is private, and a test that called it would prove the // function works rather than that anything reaches it. The chain under test is // LoadedEngine ctor -> MakeKVCacheResolved -> ResolveNumBlocks -// (model_loader.cpp:1081-1083,972). +// (`src/vllm/entrypoints/model_loader.cpp::MakeKVCacheResolved`, +// `src/vllm/entrypoints/model_loader.cpp::ResolveNumBlocks`). TEST_CASE( "loaded_engine: an EXPLICIT --gpu-memory-utilization says it did not size " diff --git a/tests/vllm/model_executor/layers/test_fp8_block_quant.cpp b/tests/vllm/model_executor/layers/test_fp8_block_quant.cpp index 7fec946a8..33df533e3 100644 --- a/tests/vllm/model_executor/layers/test_fp8_block_quant.cpp +++ b/tests/vllm/model_executor/layers/test_fp8_block_quant.cpp @@ -12,19 +12,19 @@ // from the checkpoint. The reader was sent after a tensor upstream never writes // in this mode instead of being told the fine-grained arm is absent. // -// EVERY case here enters through `ModelRegistry::Load`, the production loader -// `src/vllm/entrypoints/model_loader.cpp:1613` calls, and NOT through the -// predicate. That is deliberate and it is the reachability proof AGENTS.md -// `## Nothing lands dead` asks for: a unit test that called the predicate +// EVERY case here enters through `ModelRegistry::Load`, which the production +// loader `src/vllm/entrypoints/model_loader.cpp::FromModelDir` calls, and NOT +// through the predicate. That is deliberate and it is the reachability proof +// AGENTS.md `## Nothing lands dead` asks for: a unit test that called the predicate // directly would prove the function works and never that a load reaches it. // Deleting the call site in `ModelRegistry::Load` must red this file. // // No checkpoint, no GPU, and no model directory: the refusal fires before // `factory.load_weights`, so an EMPTY shard vector is all a load needs to reach // it. That is why the guard sits in `ModelRegistry::Load` rather than at the -// other pre-load refusal site, `RefuseUnsupportedWeightOffload` -// (`src/vllm/entrypoints/model_loader.cpp:1536`), which needs a directory on -// disk. +// other pre-load refusal site, +// `src/vllm/entrypoints/model_loader.cpp::RefuseUnsupportedWeightOffload`, +// which needs a directory on disk. #include #include diff --git a/tests/vllm/model_executor/test_gguf_device_fit.cpp b/tests/vllm/model_executor/test_gguf_device_fit.cpp index c12ea473c..7578c9dd9 100644 --- a/tests/vllm/model_executor/test_gguf_device_fit.cpp +++ b/tests/vllm/model_executor/test_gguf_device_fit.cpp @@ -56,7 +56,8 @@ std::string BuildTwoTensorGguf() { // The same file plus one tensor of the MTP / `nextn` block, which a DEFAULT load // never stages: the head is attached only under // `params.speculative_config.has_value() && method == "mtp"` -// (`src/vllm/entrypoints/model_loader.cpp:1452-1453`), and the main model reads +// (the GGUF branch of `src/vllm/entrypoints/model_loader.cpp::FromModelDir`), +// and the main model reads // `block_count - nextn_predict_layers` blocks // (`qwen3_5_gguf_weights.cpp:877-878`), so the head's blocks are outside its // range. The footprint counts it anyway, because it takes the whole tensor diff --git a/tests/vllm/v1/worker/test_runner.cpp b/tests/vllm/v1/worker/test_runner.cpp index 2a2fb5165..107f2e8a9 100644 --- a/tests/vllm/v1/worker/test_runner.cpp +++ b/tests/vllm/v1/worker/test_runner.cpp @@ -594,7 +594,8 @@ TEST_CASE("runner: MambaSpec is the allocation source of truth") { // ─── #810: THE BYTE-NEUTRALITY ARM ─────────────────────────────────────────── // // `initialize_kv_cache` serves EVERY architecture — the engine builds exactly -// one `GPUModelRunner` (model_loader.cpp:1007-1023) — so a refactor of it owes +// one `GPUModelRunner` (`src/vllm/entrypoints/model_loader.cpp::runner_`, built +// in the `LoadedEngine` member-init list) — so a refactor of it owes // a proof that it changes nothing for the models that already work. This // mirrors the BYTE-NEUTRALITY CONTRACT stated for the `per_layer_attn_specs` // seam at include/vllm/v1/kv_cache_interface.h:354-374: byte-identical