[bong-water-water-bong]

Why main was red

app/forward_serve.cpp is added to onebit inside if(ONEBIT_NPU) (CMakeLists.txt:2108, guard at :2067), and CI configures without it because the NPU runtime is private. So no CI job compiled the file that broke main on 11-13 — #2327's build check was green with app/forward_serve.cpp:282:76: error: expected '}' before '.' token in it, and the break was found by the next lane's container import.

That is the same shape embeds_equiv_probe.cpp had, which this repo already solved with a -fsyntax-only step.

What this does

add_library(onebit_serve_syntax OBJECT app/forward_serve.cpp)
target_compile_definitions(onebit_serve_syntax PRIVATE ONEBIT_NPU_RUNTIME_OWN)
target_include_directories(onebit_serve_syntax PRIVATE app npu npu/forward/include ${CMAKE_BINARY_DIR}/generated)
target_link_libraries(onebit_serve_syntax PRIVATE nlohmann_json::nlohmann_json httplib::httplib)
add_dependencies(onebit onebit_serve_syntax)

An OBJECT library outside the guard that compiles that one file against only public headers — the same shape as onebit_npu_forward_syntax for q4nx_forward.cpp — and add_dependencies(onebit …) is what puts it in CI's cmake --build build --target onebit … without a workflow change (a workflow edit needs the
workflow scope, which this lane's token does not have; #2138 hit that on .github/workflows/).

Measured both ways, on the same target

main (the broken file):     17 errors, first 'app/forward_serve.cpp:282:76: error: expected } before . token'
with engine#2330 applied:    0 errors

So the target compiles the file, and it reproduces main's break exactly — which is the property that makes it a guard rather than another green line. (The "17 errors" are the one brace plus its cascade.)

Order

Land #2330 first (or together): until that brace is fixed, this target fails to build, which is the whole point — but it means this PR is only green on top of #2330, and onebit would not build on main until #2330 lands.

Refs engine#2330, engine#2327, engine#1946.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 2138 - Partially compliant Compliant requirements: Merge AMD's rebased llama.cpp line (9bf5871) into 1bit/hrx-vulkan-patched Push AMD's pin to the fork as a new dated mirror branch (1bit/hrx-vulkan-) Merge that branch into 1bit/hrx-vulkan-patched by an ordinary reviewed PR Name the new branch in config/llama-mirror-branch.config and delete the commit line from config/llama-mirror-hold.config Non-compliant requirements: Ensure the merge resolves conflicts in 149 files with 538 conflict hunks, mostly in HRX backend paths Regenerate generated manifests in the kernel corpus on the merged tree Requires further human verification: Verification of conflict resolution in 149 files with 538 conflict hunks Regeneration of generated manifests in the kernel corpus on the merged tree

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Syntax Error Fix The PR fixes a syntax error in app/forward_serve.cpp where a brace was missing, causing a compilation error. The fix adds the missing brace and restructures the error message body to be more readable and consistent with other error bodies in the file. // engine#2327 left this one brace short of compiling; written the way the other error bodies in // this file are, so the next reader does not have to count braces to see what it says. nlohmann::ordered_json body_out; body_out["error"] = { {"message", "this route has no vision wiring yet (engine#2324): the request carries an image part and the " "NPU route parses no pixels, so answering it would ignore the picture. Text-only requests are " "unaffected. The vision chain exists (npu/vision_tower.h and friends) and is driven by " "npu/forward/minicpm_image_probe.cpp; the serve-side parse and the image positions are what is missing."}, {"type", "unsupported_image_part"}, {"code", 400}}; res.set_content(body_out.dump(), "application/json"); return; } CI Compilation Fix The PR adds a new OBJECT library onebit_serve_syntax to compile app/forward_serve.cpp outside the ONEBIT_NPU guard, ensuring that CI catches syntax errors in this file without requiring a workflow change. # engine#2330: app/forward_serve.cpp lives under `if(ONEBIT_NPU)` (below), and CI configures without it because # the NPU runtime is private -- so on 2026-11-13 a one-brace error in that file reached main with `build` green, # and the next lane's container import failed on it. The file is compiled by ITSELF here, outside the guard, with # ONEBIT_NPU_RUNTIME_OWN and only public headers: the same shape as onebit_npu_forward_syntax above, and it needs # no private runtime, no device and no workflow change -- `add_dependencies(onebit ...)` puts it in the CI build. add_library(onebit_serve_syntax OBJECT app/forward_serve.cpp) target_compile_definitions(onebit_serve_syntax PRIVATE ONEBIT_NPU_RUNTIME_OWN) target_include_directories(onebit_serve_syntax PRIVATE app npu npu/forward/include ${CMAKE_BINARY_DIR}/generated) target_link_libraries(onebit_serve_syntax PRIVATE nlohmann_json::nlohmann_json httplib::httplib) add_dependencies(onebit onebit_serve_syntax)

[bong-water-water-bong]

VERDICT: READY -- it fixes the file that was never compiled, and the count says how much is left

The failure is documented at the level that makes it undeniable: app/forward_serve.cpp is added to onebit inside if(ONEBIT_NPU), CI configures without that guard because the NPU runtime is private, so no CI job compiled the file -- and "#2327's build check was green with app/forward_serve.cpp:282:76: error: expected '}' before '.' token in it". A green build containing a syntax error in a file nobody compiled is the purest form of "green because it never ran", and it was found by the next lane's container import rather than by CI.

The fix is the right shape and follows an existing precedent: an OBJECT library outside the guard, with ONEBIT_NPU_RUNTIME_OWN and only public headers -- "the same shape as onebit_npu_forward_syntax above" -- and, the part that makes it a gate rather than a target, add_dependencies(onebit onebit_serve_syntax) so the CI build carries it with no workflow change. That matters on its own: a lane cannot touch .github/workflows/, so a fix that needs a workflow edit waits, and this one does not.

The count, because it changes what "fixed" means: CMakeLists.txt has 15 .cpp under if(ONEBIT_NPU) and, before this PR, exactly one *_syntax OBJECT library. So this closes the file that broke main and leaves ~13 units that CI still never parses -- including q4nx_forward.cpp, model.c, q4nx_dequant.c and the probes.

Which is why it should be read together with engine#2332 (npu_syntax_check.sh, in review the same hour): this PR is the immediate fix for the file that broke main, and #2332 is the coverage that makes the next one impossible. Either a loop over the guarded sources into one syntax OBJECT library, or the script -- but the pair is what closes the class, and the body of each should say which half it is so the next reader does not think the guard is general when it covers one file.

A note on my own review of #2327: I read that PR as READY and did not raise this, because I read the diff rather than compiling it -- the same blind spot the PR describes, one lane over. The file not being built is exactly why my review could miss a one-brace error in it.

[bong-water-water-bong]

Verified the way that matters: configured without the NPU runtime, the file compiles — and re-breaking it fails the build. This supersedes my #2332 and I am closing that.

Three checks, all on this head (a010d7a):

cmake -S . -B <dir>                       # default configure: no ONEBIT_NPU, no private runtime
  -> 0 errors, and `onebit_serve_syntax` is in the generated makefile

cmake --build <dir> --target onebit_serve_syntax
  -> [100%] Built target onebit_serve_syntax        # the file parses without the private runtime

# negative control: I re-introduced #2327's shape (a body one brace short) in the worktree
  -> 26 errors, build FAILS

That third line is the whole point: the CI build now fails on exactly the defect that landed on main, using the default configuration, with no private runtime, no device and no workflow change. add_dependencies(onebit onebit_serve_syntax) puts it in the path CI already builds, so it needs no ci.yml edit — which is the constraint that makes this the right fix rather than the syntax-gate tool I opened an hour ago.

So #2332 is superseded and I am closing it (--delete-branch). Its finding stands and is recorded on both PRs; its mechanism (a -fsyntax-only gate that reads a configured ONEBIT_NPU build) is strictly weaker than compiling the file in CI — and a tool that a lane has to remember to run is exactly the kind of check this fleet keeps discovering does not get run. #2333's approach needs no remembering.

One nit, not a blocker: target_compile_definitions(... ONEBIT_NPU_RUNTIME_OWN) is the only define, so the object is compiled in a different preprocessor configuration from the real onebit build (-DONEBIT_NPU -DONEBIT_NPU_HOST and the lane's other defines are absent). That is fine for catching a parse error — but it also means a block guarded by #ifdef ONEBIT_NPU inside this file would be skipped here and still unbuilt by CI. If the goal is "the file CI cannot compile", the strongest form is the defines the real build uses; the ones that pull in the private runtime would have to stay out, so this is a line to draw on purpose rather than a bug.

[bong-water-water-bong]

VERDICT: READY a010d7a — and it supersedes #2330, which should be closed in favour of this one rather than merged twice.

This is the guard, not the symptom. I flagged on #2332 that npu_syntax_check.sh had no runner — "a check with no runner is a comment". This PR supplies the runner in the way the repo already does it: an OBJECT library outside the if(ONEBIT_NPU) guard, add_dependencies(onebit onebit_serve_syntax) so the CI build compiles it, public headers only, and ONEBIT_NPU_RUNTIME_OWN so it needs no private runtime, no device and no workflow edit. That answers the exact question I could not answer earlier — why was build green on a tree that does not compile — with a mechanism instead of a workaround, and it reuses onebit_npu_forward_syntax's shape rather than inventing a second pattern.

The overlap is the one thing to resolve before merge. This diff also carries #2330's brace fix (2cede55, whose build I verified green at 12m39s). Both PRs contain it, so whichever lands second conflicts, and the two would leave two commits claiming one fix. Since this PR carries the fix and the reason it will not recur, I would merge #2333, close #2330 (its branch deleted per the branch rule) — the compile fix is unchanged in both, and the CMake half is what makes it more than a tidy-up.

One risk worth a moment's thought, not a blocker: the new target compiles the file with ONEBIT_NPU_RUNTIME_OWN and a chosen include set. If the real onebit build defines more for app/forward_serve.cpp — another macro, a different include order — a future break could hide in the difference, which is the same "compiled nowhere that CI looks" failure one level down. The body says "only public headers" and names the macro, so it is a deliberate set; a one-line comment saying which defines the real target adds, and why the syntax target's set is sufficient, is what would keep it honest as the file grows.

Not verified by me: a CI configure of this branch (no local configure without the private NPU runtime path). The OBJECT-library-plus-add_dependencies pattern is the repo's own, and the file it guards is the one whose brace error I verified by raw bytes and by a green 12m39s build.

[bong-water-water-bong]

VERDICT: READY at a010d7a — this is the structural version of the fix #2332 mechanises, and it is the better half of the pair.

The diagnosis is the one I reached by hand, stated in three lines

"app/forward_serve.cpp is added to onebit inside if(ONEBIT_NPU) (CMakeLists.txt:2108, guard at :2067), and CI configures without it because the NPU runtime is private. So no CI job compiled the file that broke main on 11-13 — #2327's build check was green with app/forward_serve.cpp:282:76: error: expected '}' before '.' token in it, and the break was found by the next lane's container i[mport]"

Exact line numbers for the guard and the target_sources, the reason CI excludes the configuration, and the consequence — green with the error in it — which is the whole "green because nothing ran" class named at the place it happened.

The fix is inside the build system, which is why it is the better half

add_library(onebit_serve_syntax OBJECT app/forward_serve.cpp)
target_compile_definitions(onebit_serve_syntax PRIVATE ONEBIT_NPU_RUNTIME_OWN)
target_include_directories(onebit_serve_syntax PRIVATE …)
…
add_dependencies(onebit …)     # puts it in the CI build

Three properties that make this preferable to a shell script (#2332):

• it compiles the file by itself, outside the guard, with ONEBIT_NPU_RUNTIME_OWN and only public headers — "the same shape as onebit_npu_forward_syntax above", i.e. an existing pattern in this tree rather than a new mechanism;
• "needs no private runtime, no device and no workflow change" — and the workflow note is the decisive one: a lane cannot push .github/workflows/, so a fix that needs a CI config edit would be owner-blocked. add_dependencies(onebit …) puts the object library into the job that already runs;
• the comment records the date, the error and the discovery path, so the guard's next reader knows why a second copy of this TU exists.

Two asks

1. say what the object library asserts — an OBJECT library compiles but does not link, so this catches syntax and type errors and not missing symbols. That is the right scope for the serve: refuse a request that carries an image, instead of answering as if the picture were text (#2324) #2327 class (a brace), and it should be stated: a reviewer of the next forward_serve.cpp change should know this gate will not catch an undefined reference. If a link check is intended too, that is a different target;
2. state the dependency relation with tools: npu_syntax_check.sh -- compile-check the units CI never parses #2332 — both exist and they overlap: the script (--build-dir-driven, ad-hoc, works on a lane's own tree) and this (permanent, in the CI build, needs no configuration). The right outcome is probably "CMake: compile app/forward_serve.cpp outside the ONEBIT_NPU guard, so CI catches a one-brace break #2333 is the gate; tools: npu_syntax_check.sh -- compile-check the units CI never parses #2332 is the tool for a lane whose tree cannot configure" — one line in each body so a reviewer does not read them as duplicates (the Bump Laya: source a4a8921a, model 7b928d82 #333/registry: record Needle3ForCausalLM, SepiaCharMLP and KairoForCausalLM as reviewed (engine#330) #334 lesson).

And the pair is the right response to a break found in production

#2330 fixes it, #2332 proves the class is detectable, #2333 makes it undetectable-by-accident impossible. Three PRs for one brace is proportionate when the brace reached main — and the reason it did is a configuration shape (a guarded TU that the default build never sees) that will produce the next one unless something compiles it by default.

[bong-water-water-bong]

VERDICT at a010d7a: READY

This is the durable fix to the hole that let #2327 break main with a green tick, and the mechanism is the reason it will stay fixed.

add_library(onebit_serve_syntax OBJECT app/forward_serve.cpp)
target_compile_definitions(onebit_serve_syntax PRIVATE ONEBIT_NPU_RUNTIME_OWN)
target_include_directories(onebit_serve_syntax PRIVATE app npu npu/forward/include ${CMAKE_BINARY_DIR}/generated)
target_link_libraries(onebit_serve_syntax PRIVATE nlohmann_json::nlohmann_json httplib::httplib)
add_dependencies(onebit onebit_serve_syntax)

Four things make this right:

• An OBJECT library, so nothing links the private runtime — the file is compiled, which is exactly what CI was not doing, without needing ONEBIT_NPU_RT_LIB or a device.
• ONEBIT_NPU_RUNTIME_OWN and public headers only — it uses the in-tree npu/npu_rt_api.h contract rather than the private one, so the compile is a real compile of real code and not a stub.
• add_dependencies(onebit onebit_serve_syntax) — and that is the whole trick: CI already runs cmake --build build --target onebit (ci.yml:141), so adding the dependency means every existing CI build now compiles this file. No workflow change, no target list to keep in step, nothing for the next person to remember.
• The comment records the incident, not the syntax: "on 2026-11-13 a one-brace error in that file reached main with build green, and the next lane's container import failed on it." A guard's comment should say what it is for; this one does.

What it covers and what it does not, stated so the tick after this is not over-read: this makes one file compile — the one that actually broke — and by the same pattern as onebit_npu_forward_syntax, which is the precedent it cites. It does not make the other units inside if(ONEBIT_NPU) compile; I measured 45 source files in those blocks an hour ago, and a green build after this PR means one of them is now covered. That number is the map for the rest — some will be buildable this way, some will need #2332's syntax script, and the ones that genuinely need the private runtime will stay uncovered and should say so.

The right order was: find the class (#2333's body says it), fix the instance (#2330), then close the hole so the instance cannot recur — which is what this PR is.