From f8d7d5bb1249fd516e9f86b15544cf8942d59c42 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sat, 22 Aug 2026 14:54:56 +0000 Subject: [PATCH] fix(DOC-README-ABI-LANDING-SOURCE): admit include/vllm.h as a landing source, so an ABI bump can repair the README claim it invalidates (#1690) The README's `## Use it as a library (C API)` block quotes `VLLM_ABI_VERSION` straight out of `include/vllm.h`. The header was already a `USER_USAGE_FILES` member, so bumping the ABI owed `docs/USAGE.md` -- but it was not a landing source, and the README rule refuses a claim change that no landing source accompanies. The claim could therefore be invalidated by an edit to the header and never repaired by one. That is not a hypothetical: the README sits at `VLLM_ABI_VERSION 21` against a header reading `23`, and #1655 exists because no legal change could fix it. The set's own criterion already admitted the file. The checker's comment says every member is "something the README QUOTES" -- the mission, the build entry point, the demo numbers, the two example mains. The C ABI header is that same relation, and it was the only such source missing. This is a widening, so it is bounded deliberately. It admits one file and not `include/` as a class, and it permits without demanding: an ordinary ABI change still owes only `docs/USAGE.md`. `test_the_c_abi_header_is_a_landing_source` reds before the change and passes after; `test_the_c_abi_header_permits_but_does_not_demand_readme` pins that no new README obligation appears; and the three properties already pinned -- a co-edited public projection, an ordinary page under `docs/`, and the bare README -- stay red as before. 80 tests, all green. The README's other stale figure, "46 exported functions", is stale by one and not by six: the header declares 47. An earlier review of #1655 put it at 51 or 52, which swept in the four lines of the `#define VLLM_API` visibility block and counted `vllm_*` identifiers that are typedefs and struct fields rather than exported functions. Recorded here because that count is the weaker half of the argument for deleting it, and the record should not overstate it. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code] --- .agents/issue-index.md | 1 + scripts/check-doc-checkpoint.py | 12 ++++++++++++ tests/scripts/test_doc_checkpoint.py | 18 ++++++++++++++++++ 3 files changed, 31 insertions(+) diff --git a/.agents/issue-index.md b/.agents/issue-index.md index cce6af057..4de00429f 100644 --- a/.agents/issue-index.md +++ b/.agents/issue-index.md @@ -624,3 +624,4 @@ rather than merged. `scripts/check-agent-record.py` gates both. | [#1627](https://github.com/mudler/vllm.cpp/issues/1627) | `BACKEND-TENSTORRENT-HOST-FREE-FORWARD` | **The Tenstorrent backend has no `SupportsAsyncSampledTokenReadback` override, so async scheduling resolves OFF on TT and `test_qwen3_dense_async_serving` FATALs on every cached checkpoint** (3 FATAL / 5 checkpoint-absent skip on the P150) at the anti-vacuous-pass guard `REQUIRE(loaded->async_scheduling_enabled())` (`test_qwen3_dense_async_serving.cpp:124`). Mechanism: `runner_supports_async()` derives from `vt::Backend::SupportsAsyncSampledTokenReadback()` (`runner.cpp:109-112`, default false at `backend.h:186`), overridden only by CPU and CUDA. PRE-EXISTING: zero hits under `src/vt/tenstorrent/` at base `52e328789`, and the R5 flip commits touch none of the resolution path — captured-vs-eager decode mode is orthogonal. NOT FIXED IN FLOW: enabling it needs a device-mirrored sampled-id design against the tt-metal allocator (CUDA's `async_device_mirror` equivalent) plus the #323-class guard re-proven on device — its own spec and gates; owned by this issue | bug | | [#1688](https://github.com/mudler/vllm.cpp/issues/1688) | `BACKEND-TENSTORRENT-HOST-FREE-FORWARD` | **`ReshapeAndCacheKernel` latched `VT_TT_HOST_FREE_DECODE` in a function-local `static`, so after the R5 default flip the documented opt-out `VT_TT_HOST_FREE_DECODE=0` silently did not reach that path.** Found by the fresh review of [PR #1630](https://github.com/mudler/vllm.cpp/pull/1630) at head `450a524b0`. `tenstorrent_device.h` documents `HostFreeDecodeEnabled()` as "No function-local static caching: tests toggle this env per case in one process", and the flip converted eleven call sites to a live read — `EnsureDevice2D`, `RmsNormKernel`, `PreferDeviceRope`, `CopyDeviceDeviceIfCapture`, `MemsetDeviceIfCapture`, `WarmRopeCosSin`, `WarmPagedKvShadow`, `WarmRacIdx`, `WarmPaMeta`, `WarmDecodePos`, `CaptureDecodePosAdvance` — but left `tenstorrent_ops.cpp:2130` a `static const bool`. The polarity flip is what made it bite: pre-flip the latch cached the default-OFF state so only the opt-IN could be defeated, and that was set before the process started; post-flip it caches default-ON, so the opt-out that the flip tells operators to use never arrives at RAC once any decode has run. The suite could not see it — the flip's own `support_static_graph_mode` round trip and the `setenv(...,"0")` inertness guard both prove the contract elsewhere and neither reaches `ReshapeAndCacheKernel`. FIXED IN FLOW: the `static` is dropped, matching every other converted site. NOT fixed as a test: `ReshapeAndCacheKernel` needs a real Blackhole device, so every case reaching it is behind `TenstorrentPresent()` and skips on every `rc` fleet host; the owed `thalia` case is recorded under [`tenstorrent-host-free-forward.md`](specs/tenstorrent-host-free-forward.md) `## Owed` | bug | | [#1661](https://github.com/mudler/vllm.cpp/issues/1661) | `TEST-SHELLCHECK-SKIP-GUARD` | **`test_script_stays_shellcheck_clean` ERRORs instead of skipping on hosts without the shellcheck binary: the guard probes `returncode`, but a missing binary raises `FileNotFoundError` before a returncode exists**, so the `tools suites` preflight gate is red on such hosts (`thalia`, aarch64). The test predates the window (last touched `b95543c44`); it surfaced when current main's preflight started running the module. FIXED IN FLOW by the same change: the probe gains the `except FileNotFoundError` skip arm (message unchanged), and both arms are proven — no-binary host skips via the new arm, a PATH-shim `shellcheck` exiting 3 still skips via the returncode arm, and unmodified main reds on this host | bug | +| [#1690](https://github.com/mudler/vllm.cpp/issues/1690) | `DOC-README-ABI-LANDING-SOURCE` | **`check-doc-checkpoint.py`'s `LANDING_SOURCE_FILES` omitted `include/vllm.h`, so a commit that bumps `VLLM_ABI_VERSION` could never repair the README claim it invalidated.** Found by the review of [PR #1655](https://github.com/mudler/vllm.cpp/pull/1655). The README `## Use it as a library (C API)` block quotes the ABI version out of the header; the header was in `USER_USAGE_FILES` (so an ABI change owes `docs/USAGE.md`) but not in `LANDING_SOURCE_FILES`, and the README rule refuses a claim change unaccompanied by a landing source. The claim could therefore be invalidated but not repaired by the same edit, which is how the README reached `VLLM_ABI_VERSION 21` against a header reading `23`, alongside a stale "46 exported functions" for a header declaring 47. That second half is stale by one rather than by six -- an earlier review figure of 51/52 swept in the `#define VLLM_API` visibility block and counted `vllm_*` identifiers that are typedefs and struct fields rather than exported functions -- so the case for deleting the count is that a live count of one file stored in another goes stale on any ABI addition, not that it is badly wrong. The set's own criterion already admitted it -- the checker's comment says every member is "something the README QUOTES" -- and the header was the only such source missing. FIXED IN FLOW: `include/vllm.h` is added to the set, red-before/green-after pinned by `test_the_c_abi_header_is_a_landing_source`, with `test_the_c_abi_header_permits_but_does_not_demand_readme` proving no new README obligation and the pre-existing no-class tests still green | bug | diff --git a/scripts/check-doc-checkpoint.py b/scripts/check-doc-checkpoint.py index 1858fbc71..81dce39be 100755 --- a/scripts/check-doc-checkpoint.py +++ b/scripts/check-doc-checkpoint.py @@ -223,10 +223,22 @@ # This admits exactly one document and no class. docs/BUILD.md, docs/STATUS.md # and every other page under docs/ still cannot license a README claim change, # which tests/scripts/test_doc_checkpoint.py pins directly. +# +# include/vllm.h joined the set on 2026-08-22 (#1655). It is the QUOTES relation +# again, and it was the only such source missing: the README's `## Use it as a +# library (C API)` block quotes `VLLM_ABI_VERSION` straight out of the header. +# The header was already a USER_USAGE_FILES member, so an ABI change owed +# docs/USAGE.md -- but not being a landing source meant no edit to the header +# could ever license repairing the claim the header itself invalidated. The +# README consequently sat at `21` against a header reading `23`, with no legal +# change that could fix it. Admitting the file closes that trap; it does not +# admit `include/` as a class, and it demands nothing, so an ordinary ABI change +# still owes only docs/USAGE.md. LANDING_SOURCE_FILES = frozenset( { ".agents/mission.md", "CMakeLists.txt", + "include/vllm.h", "benchmarks/demo/footprint_gb10.json", "benchmarks/demo/qwen36_27b_c1_c32.json", "benchmarks/demo/vulkan_27b_llamacpp.json", diff --git a/tests/scripts/test_doc_checkpoint.py b/tests/scripts/test_doc_checkpoint.py index 6b514bccf..a43b57741 100755 --- a/tests/scripts/test_doc_checkpoint.py +++ b/tests/scripts/test_doc_checkpoint.py @@ -427,6 +427,24 @@ def test_the_quickstart_page_permits_but_does_not_demand_readme(self): """`landing_page` never demands the README, and #1520 does not add one.""" self.assertEqual(self.errors(["docs/QUICKSTART.md"]), []) + def test_the_c_abi_header_is_a_landing_source(self): + """#1655: the README quotes `VLLM_ABI_VERSION` out of include/vllm.h. + + Every other member of the set is something the README QUOTES. This one + is too, and it was the only such source missing -- which made the ABI + claim unrepairable BY the change that invalidates it, so the README sat + two versions stale (`21` against the header's `23`) with no legal edit + that could fix it. `include/vllm.h` still owes docs/USAGE.md, which is + why that page is in the change set here. + """ + self.assertEqual( + self.errors(["README.md", "include/vllm.h", "docs/USAGE.md"]), [] + ) + + def test_the_c_abi_header_permits_but_does_not_demand_readme(self): + """Admitting it must not turn every ABI change into README churn.""" + self.assertEqual(self.errors(["include/vllm.h", "docs/USAGE.md"]), []) + def test_an_unrelated_document_never_licenses_readme_churn(self): """The property #1520 must not break, stated on a NON-projection doc.