[bong-water-water-bong]

docs/npu-plan.md says four plans are checked in "one per model directory on the build box", and that tools/npu_plan.py <dir> --check proves a directory's copy still matches. Both are true — and the second needs the model directory, which CI has not.

So the shipped plans had no gate at all: git grep configs/plans finds them named in two docs and read by nothing. A hand edit, or a change to the generator, could leave one refused by Plan::load() and the first evidence would be a lane loading it on the box.

They are all valid today — which is the reason to pin them

$ (each shipped plan, through the same loader the forward uses)
configs/plans/bonsai2-27b-NPU-b8.plan.json   LOADS (layers=64)
configs/plans/bonsai2-27b-NPU.plan.json      LOADS (layers=64)
configs/plans/bonsai2-27b-pb16i.plan.json    LOADS (layers=64)
configs/plans/Qwen3-0.6B.plan.json           LOADS (layers=28)

What the test does

Loads each one through Plan::load(), so the refusal vocabulary is the engine's own rather than a JSON schema written twice. A format bump is refused by name:

plan: configs/plans/Qwen3-0.6B.plan.json format is "1bit-npu-plan/2", this engine reads "1bit-npu-plan/1"

and it also asserts plan.format == kFormat, so a future format bump has to touch this test instead of silently accepting yesterday's plans.

An empty or wrong directory FAILS rather than passing over zero plans — the vacuous green this test exists to prevent.

Verified both arms, plus the real path

bumped format    → FAIL rc 1, with the sentence above
empty directory  → FAIL rc 1 ("holds at least one .json")
the real dir     → ok rc 0     (9 checks)
ctest            → Test #239 npu_plan_shipped, 100% passed, in a real configure

Registered through cmake/tests-*.cmake (#1979's glob), so no CMakeLists.txt hunk. Ratchets unmoved: ci_targets_check 9 (baseline 9, none new), tmp_path_check 44 sites none new/stale, copyright clean.

What it does not prove

That a plan still matches its model directory (that is --check, and it needs the pack), or that a plan is right — only that it is one the engine can read. Stated in the test's own header so the next reader does not over-read a green row.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1979 - Partially compliant Compliant requirements: The PR introduces a separate CMake file (cmake/tests-plan-shipped.cmake) for test registration, addressing the conflict issue The new test (npu_plan_shipped_test) is properly registered in the new CMake file The solution prevents the CMake registration from conflicting with other PRs' changes Non-compliant requirements: The PR does not implement a generator-based manifest approach for test registration The PR does not add a required check that runs git apply --check of the PR's own diff against current main Requires further human verification: Verification that the new test correctly loads all shipped plans and fails appropriately on format mismatches Confirmation that the new test registration does not conflict with other test registrations in the repository

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Test Logic Verification The test loads each shipped plan through Plan::load() to verify it can be read by the engine, but it does not verify that the plan matches its corresponding model directory. This could lead to a false sense of security if a plan is valid but no longer corresponds to the model it was generated from. for (const auto& p : plans) { const std::string name = p.filename().string(); try { const auto plan = onebit::npu::plan::Plan::load(p.string()); check(plan.layers > 0, name + ": loads through Plan::load() with " + std::to_string(plan.layers) + " layer(s)"); // The loader sets `format` to kFormat when it accepts the file; asserting it here means a future // format bump has to touch this test as well, rather than silently accepting the old plans. check(plan.format == onebit::npu::plan::kFormat, name + ": carries the format this build reads"); } catch (const std::exception& e) { check(false, name + ": REFUSED by Plan::load(): " + e.what()); } } CMake Registration Location The new CMake file cmake/tests-plan-shipped.cmake is added to handle test registration, which is a good approach to prevent conflicts. However, it's important to ensure that this file is included in the main CMakeLists.txt or another appropriate location to be properly processed during the build. # Copyright 2026 bong-water-water-bong # SPDX-License-Identifier: Apache-2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at # # http://www.apache.org/licenses/LICENSE-2.0 # # Unless required by applicable law or agreed to in writing, software # distributed under the License is distributed on an "AS IS" BASIS, # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. # # engine#799: the plans checked in under configs/plans/ were loaded by nothing. docs/npu-plan.md says # `tools/npu_plan.py <dir> --check` proves a directory's copy still matches - true, and it needs the model # directory, which CI has not. So the shipped plans had no gate: a hand edit or a generator change could leave one # refused by Plan::load() and the first evidence would be a lane loading it on the box. # # This loads each one through the same loader the forward uses. No model, no device, no pack. add_executable(npu_plan_shipped_test tests/npu_plan_shipped_test.cpp) target_include_directories(npu_plan_shipped_test PRIVATE npu) target_link_libraries(npu_plan_shipped_test PRIVATE nlohmann_json::nlohmann_json) add_test(NAME npu_plan_shipped COMMAND npu_plan_shipped_test ${CMAKE_SOURCE_DIR}/configs/plans) add_dependencies(onebit npu_plan_shipped_test)

[bong-water-water-bong]

VERDICT at 855163d: READY

A checked-in plan that is not loaded by anything is the same defect as a test that runs nowhere — the file exists, looks authoritative, and nothing would notice if it stopped parsing or named a directory that is gone. npu_plan_shipped_test closes that: the plans under configs/plans/ are loaded through the same reader the runtime uses, so a plan that drifts from the schema fails here rather than at serve time.

cmake/tests-plan-shipped.cmake puts the registration in a family file, which is the current convention (#1696's pattern) and keeps CMakeLists.txt from growing another block for one row.

The linking is also right: nlohmann_json only, no onebit_npu, so the arm tests the plan parser rather than dragging in the forward — which is what makes it runnable everywhere, including CI without a device.

[bong-water-water-bong]

VERDICT: READY at 855163d

Built it, ran it (9 arms), then broke one plan to watch the gate catch it — which is the whole question for a gate over checked-in data.

$ ./npu_plan_shipped_test
ok   the shipped plans directory holds at least one .json (configs/plans)
ok   Qwen3-0.6B.plan.json: loads through Plan::load() with 28 layer(s)
ok   Qwen3-0.6B.plan.json: carries the format this build reads
ok   bonsai2-27b-NPU-b8.plan.json: loads through Plan::load() with 64 layer(s)
…  npu_plan_shipped: ok

$ (format: "bogus-9" in Qwen3-0.6B.plan.json)
FAIL Qwen3-0.6B.plan.json: REFUSED by Plan::load(): plan: configs/plans/Qwen3-0.6B.plan.json format is "bogus-9", this engin…
FAILED

The failure names the file, the value, and what the engine reads — which is what makes this gate usable on a stale plan rather than just red.

The finding that motivates it is the one I would have missed: "git grep configs/plans finds them named in two docs and read by nothing". Four plans, documented as "one per model directory on the build box", and no gate — so a plan can drift from Plan::load() for as long as nobody
runs tools/npu_plan.py <dir> by hand, and the first symptom is a model that fails to load on a box where the plan is right but the format moved.

Two details that make it a good gate:

• the shipped plans directory holds at least one .json — the empty-set arm. A gate over a directory that has been emptied or moved would otherwise pass with zero checks, which is the failure
  mode of half the enumerations in this repo.
• both arms per plan: "loads through Plan::load()" and "carries the format this build reads" — the second is the one that catches a format bump, and it is separate because a plan can load and
  still be the wrong vintage.

One note, not a change: the arms are generated per file in the directory, so a new plan is covered automatically but a deleted one only shrinks the list — if the count matters (four, per the doc), an
arm pinning >= 4 would make a disappearance as loud as a corruption. The "at least one" arm catches the empty case, which is the important half.

Not run: tools/npu_plan.py against the plans (no device needed for Plan::load(), but the tool's path is out of scope here).

[bong-water-water-bong]

VERDICT: READY at 855163d — a gate for the one artifact in this area that CI could not reach, using the loader the forward uses.

The gap is stated as an impossibility, which is the honest form

"tools/npu_plan.py <dir> --check proves a directory's copy still matches — true, and it needs the model directory, which CI has not. So the shipped plans had no gate: a hand edit or a generator change could leave [a plan] refused by Plan::load() and the first evidence would be a lane loading it on the box."

git grep configs/plans finding them named in two docs and read by nothing is the measurement, and it is exactly the class this queue keeps finding: a documented guarantee with no instrument behind it (#2230's rates, #2229's census entries, #2197's eight prompts). The difference here is that the remedy is real, because the plan does not need the model — only the loader.

tests/npu_plan_shipped_test.cpp loads through the same path the forward does

# This loads each one through the same loader the forward uses. No model, no device, no pack.

That is the property that makes it a gate rather than a schema check: a hand-written YAML validator would pass the day Plan::load() tightened and the plan did not. No model, no device, no pack is the other half — it can run in CI, on a laptop, and in a pre-commit hook, so the check that was impossible becomes free.

The cmake/tests-plan-shipped.cmake registration also means it runs under ctest with the rest, rather than being a script someone remembers to call — which is the difference between a gate and a tool.

Two asks

1. say what a failure looks like — the loader's refusal message is the diagnostic, so quote it once in the body (or as a comment): a red row that says Plan::load(): … at a named plan is actionable; one that says "test failed" is not;
2. state the coverage boundary: does it load every file under configs/plans/, or a list? If configs/plans/ gains a fifth plan, does this gate see it — a glob does, a hardcoded list does not, and "the shipped plans had no gate" is precisely a hardcoded-list failure repeating.