bong-water-water-bong · Nov 10, 2026 · Nov 10, 2026 · Nov 10, 2026 · Nov 10, 2026 · Nov 10, 2026
diff --git a/.github/workflows/pr-agent.yml b/.github/workflows/pr-agent.yml
@@ -38,7 +38,12 @@ jobs:
     runs-on: [self-hosted, pr-agent]
     # on the job, not the workflow: a comment that skips the job (a bot's, say) must not cancel a review
     concurrency:
-      group: pr-agent-${{ github.event.pull_request.number || github.event.issue.number }}
+      # A comment and a push are different jobs on one serialised model runner: an `/ask` must not cancel the review of
+      # the PR it is asked about, and a `synchronize` must cancel the review of the head it replaced. Grouping by event
+      # name keeps push-cancels-push (right: a newer head makes the older review moot) without letting a busy thread
+      # starve it. Measured 2026-11-10: PR #2007's review ran 19:21:26 -> cancelled 19:30:31, and `issue_comment:
+      # [created]` puts every comment on the PR into this same group with `cancel-in-progress: true`.
+      group: pr-agent-${{ github.event.pull_request.number || github.event.issue.number }}-${{ github.event_name }}
       cancel-in-progress: true
     timeout-minutes: 30
     # same-repo PRs, and comments by the owner, members or collaborators on a pull request

diff --git a/.gitignore b/.gitignore
@@ -41,3 +41,8 @@ pin-report.json
 # probe - a red that is the tree's, not the repository's (engine#1564). Ignoring them also stops run output being
 # committed by accident.
 .hrx123/logs/
+
+# npu_pack_import_test's container-genre fixtures (test_q4nx_container): written to <golden dir>/q4nx-container, and to
+# ./q4nx-container when the test runs with no golden dir -- so a run from the source root left them where `git add -A`
+# picks them up, and #1986 committed five of them (820 KB) and reddened the copyright gate. Run output, not source.
+q4nx-container/
diff --git a/CMakeLists.txt b/CMakeLists.txt
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -87,6 +87,17 @@ an RFC or issue.
   hands the script to the interpreter instead of relying on the file's exec bit; `tests/python_test_order_check.py
   --check` enforces that it follows `find_package(Python3)`. Two rows may not share a name: CMake stops at configure with
   `add_test given test NAME "…" which already exists in this directory`.
+- **A new test's registration goes in its own file.** Put the `add_test(...)` (or `add_python_test(...)`) row, its
+  comment, and its `set_tests_properties` in a new `cmake/tests-<name>.cmake` with the Apache notice, and do not touch
+  `CMakeLists.txt` for it: every `cmake/tests-*.cmake` is included by a glob near the end of `CMakeLists.txt`, after all
+  targets exist. A new file cannot conflict with another pull request, and a row appended to `CMakeLists.txt` does —
+  that file changed 37 times in one day and three open pull requests had a registration hunk that no longer applied
+  (engine [#1979](https://github.com/1bit-MONSTER/engine/issues/1979)). A row that belongs to an existing family goes in
+  that family's file (`cmake/tests-gpu-hold.cmake`). An `add_executable` for a test binary still goes in
+  `CMakeLists.txt` next to the sources it builds; only the registration moves. Existing rows stay where they are.
+  A registration file that needs a fixed position may still be `include()`d by name from `CMakeLists.txt`; the glob
+  then leaves it alone. `tests/registration_glob_test.sh` (itself registered from `cmake/tests-registration-glob.cmake`)
+  runs the glob block and holds both rules.
 - **Pins only move forward.** CI fails a pull request whose submodule pin is behind, or has
   diverged from, the pin on `main` (`tools/check_pins.py`), because that drops commits the engine
   already shipped. A rollback on purpose carries the `pin rollback` label.

diff --git a/app/forward_serve.cpp b/app/forward_serve.cpp
diff --git a/app/main.cpp b/app/main.cpp
@@ -43,6 +43,7 @@ int onebit_ort_run_main(int argc, char** argv);
 #endif
 #ifdef ONEBIT_NPU_HOST
 #include "pack_import.h"  // 1bit npu-pack: an OGA NPU pack -> a Q4NX model directory
+#include "vision_pack.h"  // 1bit npu-pack vision: a pack's vision component (engine#1788)
 #endif
 #ifdef ONEBIT_NPU
 #include "forward_serve.h"
@@ -94,6 +95,7 @@ void usage(FILE* out) {
 #endif
 #ifdef ONEBIT_NPU_HOST
                  "  npu-pack <info|import> <pack dir>  import a Ryzen AI OGA NPU pack (npu-pack --help)\n"
+                 "  npu-pack vision <pack dir>         read a pack's vision component (vision_weight.q4nx)\n"
 #endif
                  "  comfy <workflow.json>       run a ComfyUI workflow with ComfyUI.cpp (docs/comfyui.md)\n"
 #ifdef ONEBIT_LAYA
@@ -323,6 +325,18 @@ int main(int argc, char** argv) {
 #ifdef ONEBIT_NPU_HOST
     if (cmd == "npu-pack") {
         try {
+            // `npu-pack vision <pack dir>`: the vision component of a two-component pack (engine#1788), read by
+            // npu/vision_pack.h rather than the OGA importer, which knows nothing of it.
+            if (argc >= 3 && std::string(argv[2]) == "vision") {
+                if (argc != 4 || argv[3][0] == '-') {
+                    std::fprintf(stderr, "usage: 1bit npu-pack vision <pack dir>\n"
+                                         "  reads the pack's vision component (config.json's vision_model_weight, default\n"
+                                         "  vision_weight.q4nx): every tensor placed, the tower's geometry from the tensors'\n"
+                                         "  own shapes, and each config.json field the tensors overrule\n");
+                    return argc == 4 ? 0 : 2;
+                }
+                return onebit::npu::vision::print_info(argv[3]);
+            }
             return onebit::npu::pack_import::run(argc - 2, argv + 2);
         } catch (const std::exception& e) {
             std::fprintf(stderr, "1bit npu-pack: %s\n", e.what());

diff --git a/app/npu_answer_parser.h b/app/npu_answer_parser.h
@@ -142,8 +142,11 @@ inline AnswerFamily answer_family_of_template(const std::string& template_text)
 // (is_reasoning, text) pieces; finish() flushes what the holdback still held and reports the calls.
 class AnswerStream {
 public:
-    AnswerStream(AnswerFamily family, bool tools_sent)
-        : fam_(family), tags_(answer_detail::tags_of(family)), tools_sent_(tools_sent) {}
+    // `starts_in_reasoning`: the PROMPT already opened the reasoning block (prompt_opens_reasoning below), so the
+    // answer begins inside it and the first thing to look for is the closing tag.
+    AnswerStream(AnswerFamily family, bool tools_sent, bool starts_in_reasoning = false)
+        : fam_(family), tags_(answer_detail::tags_of(family)), tools_sent_(tools_sent),
+          mode_(starts_in_reasoning ? Mode::reasoning : Mode::content) {}
 
     void feed(const std::string& chunk, std::vector<std::pair<bool, std::string>>& out) {
         buf_ += chunk;
@@ -169,6 +172,9 @@ class AnswerStream {
     // The calls seen so far, in order.  A call needs tools_sent, per the issue's rule.
     const std::vector<ToolCall>& calls() const { return calls_; }
     bool saw_reasoning() const { return saw_reasoning_; }
+    // Inside a reasoning block right now (after the opening tag, before the closing one). reasoning_token_count()
+    // reads it before and after each token, so the tags themselves and held-back partial tags count with the block.
+    bool in_reasoning() const { return mode_ == Mode::reasoning; }
     bool saw_text() const { return saw_text_; }
 
     const std::string& reasoning() const { return reasoning_; }
@@ -177,8 +183,22 @@ class AnswerStream {
 private:
     enum class Mode { content, reasoning, call };
 
-    void emit(std::vector<std::pair<bool, std::string>>& out, bool is_reasoning, const std::string& text) {
-        if (text.empty()) return;
+    // Whitespace around a reasoning block is the model's layout, not part of either text: `<think>\n...\n</think>\n\n`
+    // then the answer.  Left in, the answer to "one word" comes back as "\n\nParis", and a model that thinks nothing
+    // (`<think>\n\n</think>`) sends a reasoning chunk that is two newlines.  So the whitespace that LEADS the reasoning
+    // is dropped, and so is the whitespace that leads the content when that content follows a reasoning block --
+    // the rule app/think_split.h has always applied on the in-process route ("whitespace around the block is
+    // dropped"), so the two routes return the same strings.  Content with no reasoning before it is not touched.
+    void emit(std::vector<std::pair<bool, std::string>>& out, bool is_reasoning, const std::string& raw) {
+        if (raw.empty()) return;
+        std::string text = raw;
+        bool& trim = is_reasoning ? trim_reasoning_ : trim_content_;
+        if (trim) {
+            const size_t b = text.find_first_not_of(" \t\r\n");
+            if (b == std::string::npos) return;   // all whitespace so far: nothing to send yet
+            text.erase(0, b);
+            trim = false;
+        }
         if (is_reasoning) {
             reasoning_ += text;
             saw_reasoning_ = true;
@@ -218,6 +238,8 @@ class AnswerStream {
                 }
                 if (starts(tags_.think_close)) {
                     flush_plain();
+                    // what follows a block is separated from it by layout whitespace -- unless content came first
+                    if (mode_ == Mode::reasoning && !saw_text_) trim_content_ = true;
                     mode_ = Mode::content;
                     i += std::string(tags_.think_close).size();
                     continue;
@@ -374,14 +396,32 @@ class AnswerStream {
     std::vector<ToolCall> calls_;
     bool saw_reasoning_ = false;
     bool saw_text_ = false;
+    bool trim_reasoning_ = true;   // until the reasoning's first non-whitespace character
+    bool trim_content_ = false;    // set when a reasoning block closes before any content; cleared by the first text
 };
 
+// Does the rendered prompt leave the model INSIDE a reasoning block?  Some templates end their generation prompt
+// with the opening tag itself -- Qwen3.5's ends `<|im_start|>assistant\n<think>\n` when thinking is on -- so the
+// model never writes that tag: its answer starts with the reasoning and the first tag in it is the closing one.
+// A splitter that waits to see the block opened then files the whole reasoning under `content` and drops the
+// closing tag it never saw opened, so nothing in the reply marks where the answer starts (measured on FastFlowLM's
+// Qwen3.5-2B-NPU2, engine#1786).  True when the prompt, less trailing whitespace,
+// ends with the family's opening tag.  A prompt that opens and closes the block (thinking off:
+// `<think>\n\n</think>\n\n`) does not end with it, and neither does one that never mentions it.
+inline bool prompt_opens_reasoning(const std::string& prompt, AnswerFamily family) {
+    const std::string open = answer_detail::tags_of(family).think_open;
+    size_t end = prompt.size();
+    while (end > 0 && (prompt[end - 1] == ' ' || prompt[end - 1] == '\n' || prompt[end - 1] == '\r' || prompt[end - 1] == '\t')) --end;
+    return !open.empty() && end >= open.size() && prompt.compare(end - open.size(), open.size(), open) == 0;
+}
+
 // Which of `pieces` (one generated token's text each, in order) belong to `content` -- not to
 // reasoning and not to call markup.  Callers that report a per-token value use this so reasoning and
 // call markup get no entry (issue #651 item 3).  A token that straddles a boundary ("</think>The")
 // counts as content: its text did reach the caller.
-inline std::vector<char> content_mask(const std::vector<std::string>& pieces, AnswerFamily family, bool tools_sent) {
-    AnswerStream s(family, tools_sent);
+inline std::vector<char> content_mask(const std::vector<std::string>& pieces, AnswerFamily family, bool tools_sent,
+                                      bool starts_in_reasoning = false) {
+    AnswerStream s(family, tools_sent, starts_in_reasoning);
     std::vector<char> mask;
     mask.reserve(pieces.size());
     std::vector<std::pair<bool, std::string>> out;
@@ -405,9 +445,61 @@ inline std::vector<char> content_mask(const std::vector<std::string>& pieces, An
     return mask;
 }
 
+// How many of `pieces` (one generated token's text each, in order) were reasoning: OpenAI's
+// usage.completion_tokens_details.reasoning_tokens (engine#2111). An empty `content` with finish_reason "length" reads
+// like "the model said nothing" when the whole budget went into a thinking block, and this is the number that says so.
+// A token is reasoning when none of its text reached `content` and the block was open before it, after it, or it put
+// text into the block: so the opening and closing tags, a partial tag the holdback keeps, and an unclosed block at the
+// end of the budget all count, while call markup and the whitespace after a closed block do not. A token that
+// straddles the boundary ("</think>The") is content, as content_mask() says. Same splitter, so the count, the split
+// and the logprob entries cannot disagree.
+inline int reasoning_token_count(const std::vector<std::string>& pieces, AnswerFamily family, bool tools_sent,
+                                 bool starts_in_reasoning = false) {
+    AnswerStream s(family, tools_sent, starts_in_reasoning);
+    std::vector<std::pair<bool, std::string>> out;
+    int n = 0;
+    bool last_counted = false;
+    // Tokens that produced nothing before anything was produced (leading whitespace, the first half of an opening tag
+    // the holdback keeps) belong to whatever comes next: to the block if the next token opens it, to nothing otherwise.
+    int pending = 0;
+    bool started = false;
+    for (const auto& piece : pieces) {
+        const bool before = s.in_reasoning();
+        out.clear();
+        s.feed(piece, out);
+        bool content = false, reasoning = false;
+        for (const auto& [is_reasoning, text] : out) {
+            (void)text;
+            (is_reasoning ? reasoning : content) = true;
+        }
+        last_counted = !content && (reasoning || before || s.in_reasoning());
+        if (last_counted) {
+            ++n;
+            if (!started) n += pending;
+            started = true;
+        } else if (content) {
+            started = true;
+        } else if (!started) {
+            ++pending;
+        }
+    }
+    // What the holdback still holds is flushed by finish(): if it is content, the last token's text reached `content`.
+    out.clear();
+    s.finish(out);
+    for (const auto& [is_reasoning, text] : out) {
+        (void)text;
+        if (!is_reasoning && last_counted) {
+            --n;
+            last_counted = false;
+        }
+    }
+    return n;
+}
+
 // The whole answer at once: the same splitter, so the two paths cannot disagree.
-inline ParsedAnswer parse_answer(const std::string& text, AnswerFamily family, bool tools_sent) {
-    AnswerStream s(family, tools_sent);
+inline ParsedAnswer parse_answer(const std::string& text, AnswerFamily family, bool tools_sent,
+                                 bool starts_in_reasoning = false) {
+    AnswerStream s(family, tools_sent, starts_in_reasoning);
     std::vector<std::pair<bool, std::string>> pieces;
     s.feed(text, pieces);
     s.finish(pieces);

diff --git a/app/npu_chatml.h b/app/npu_chatml.h
@@ -61,6 +61,31 @@ inline std::string chatml(const nlohmann::json& messages, bool thinking = true)
     return p;
 }
 
+// The messages a chat route renders to find where a request's system turn ends (--prefix-pin, engine#1785): the
+// request's leading system messages, then one user message whose text no question starts with.  The rendering of
+// these and the rendering of the real request agree up to the first token of the user's text and differ there, so
+// their common prefix is the part every question under this system prompt shares.
+//
+// The user message is there because a template may refuse to render without one.  Measured: the Qwen3.5 template
+// rendered alone on its system message throws, and the early pin was silently not made on that container.
+//
+// Empty array when the request does not open with a system message: there is nothing to pin early.
+template <typename Json>
+Json pin_probe_messages(const Json& messages) {
+    Json lead = Json::array();
+    if (!messages.is_array()) return lead;
+    for (const auto& m : messages) {
+        if (!m.is_object() || !m.contains("role") || !m["role"].is_string() || m["role"].template get<std::string>() != "system") break;
+        lead.push_back(m);
+    }
+    if (lead.empty()) return lead;
+    Json probe = Json::object();
+    probe["role"] = "user";
+    probe["content"] = "\xC2\xA4\xC2\xA4 pin probe \xC2\xA4\xC2\xA4";
+    lead.push_back(probe);
+    return lead;
+}
+
 // Whether the container's tokenizer can represent the prompt above.  `chatml()` is rendered for
 // every container (#832), and a tokenizer without the anchors spells `<|im_start|>` as ordinary
 // pieces, so the model is prompted with text its own template never produces (#963: on