[bong-water-water-bong]

Orchestrator decision 2026-11-05, item 2's engine half: "The engine maps code 102 to the queue (wait, then retry) and only errors after the --queue-ms budget."

The decision's own item 1 and #1085's body pin the signature down: after the per-client partition fix a second client is refused at context creation with code 102 / -ENODEV — which is the same signature a dead array answers (aie2_message.c, aie2_ctx.c). So the code cannot say which it is. Only time can: a client that holds the array frees it when it exits; a dead array never does.

What was wrong before

app/npu_load_retry.h already retried a refused context — but on a 2.5 s window, sized for the evict-then-load race (engine #1010). Two seconds is right for "the last backend is still tearing down" and wrong for "another client is serving a model right now": after the 2.5 s the engine declared the driver dead and printed "reload the driver (modprobe -r amdxdna && modprobe amdxdna)", which is the one piece of advice that is wrong when the array is merely busy.

What this does

file	change
app/npu_load_retry.h	LoadRetryPolicy gains a refusal schedule beside the race window: refusal_window_s 45 s, 2 s max delay, 60 attempts. load_retry_delay() / load_retry_more() take the schedule explicitly (the old one-argument forms are kept, so every existing caller and test is untouched). open_with_retry() now classifies each failure: a refused context queues on the long schedule, anything else keeps the short one. The queued retry line says "queued: another NPU client may hold the array", and the give-up hint names both causes in the order to check them — another client first (wait for it, or stop it), the driver reload second.
app/serve.cpp	--load-queue-ms N (default 45000, 0 = refuse at once = pre-#1085 behaviour), validated, in the help, and forwarded into all three run_unified argv builders. That last part matters: serve builds that argv itself, so a flag only unified understood would have been silently unreachable from 1bit serve — which is the only path Lemonade and the appliance use.
app/unified.cpp	the flag, its validation, its help, and one LoadRetryPolicy carrying it into both NPU open sites (private route and fast lane).
docs/npu.md	new "#### A refusal is a queue, not a fault" under "When a load follows an evict": why code 102 is ambiguous, what the budget does, and that it is the load-side counterpart of --queue-ms (a request can be told it is queueing — an empty delta a second — and a load cannot).
tests/npu_load_retry_test.cpp	four new cases (7–10).

Verification, device-free (no NPU ticket, no patched module)

./build/npu_load_retry_test                               -> PASS   (all 10 cases)
1bit serve -m <NPU dir> --help                            -> the three --load-queue-ms help lines
1bit serve -m <NPU dir> --load-queue-ms -5                -> "1bit serve: --load-queue-ms takes 0 (refuse at once) or
                                                             more, not -5"        [before any device work]
1bit serve -m <NPU dir> --load-queue-ms 1234              -> reaches open_with_retry, and the fake-kernel error is NOT a
                                                             refusal, so it keeps the short window: "attempt 1 failed
                                                             after 11 ms (... not an ELF32 LE file); retrying in 150 ms"
                                                             -- the classification, observed rather than asserted
build (ONEBIT_NPU=ON -DONEBIT_NPU_RUNTIME=own -DONEBIT_NPU_RT_DIR=<npu_rt>)  -> rc 0, 0 errors, 0 warnings in the TUs
tools/copyright.py --check                                 -> rc 0

The four new test cases are the ones that make the behaviour, not the refactor, the subject:

• a refused context retries even when window_s = 0 — proof that the queue window is what acted, not the old one;
• a refusal that never clears stops at the queue cap (refusal_max_attempts), and its give-up line names both the window it spent (200 ms queue window) and the other client before the reload advice;
• --load-queue-ms 0 is exactly one attempt;
• and an ordinary failure still obeys the short policy's cap, so a genuinely broken model cannot hang a load for the queue budget. (Test 10 asserts the cap rather than the window because the injected sleep does not advance the real clock — the same reason the existing tests bound attempts.)

Ordering, and what is not here

• third_party: the amdxdna partition fix (client-scoped, measured) and the TDR recovery pair the fleet runs #1085 stays held (pi-prs): this is the engine's half, and it is useful without the patch too — a refusal from any non-locking client is now waited out instead of misdiagnosed. On a patched module it becomes the normal path for a second client.
• The end-to-end row on hardware (two clients → second queues → first finishes → second loads) belongs with third_party: the amdxdna partition fix (client-scoped, measured) and the TDR recovery pair the fleet runs #1085's rows once a patched module is installed; this PR's claim is the policy and the classification, which are device-free and therefore pinned by the unit test.
• Out of scope deliberately: 1bit forward-serve has no retry path at all (only the unified serve does), and the flag is not wired into the appliance TUI's settings.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 95c7773)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1085 - Partially compliant Compliant requirements: The --load-queue-ms flag is added and implemented in app/serve.cpp and app/unified.cpp The LoadRetryPolicy struct is extended with refusal_window_s, refusal_max_delay_s, and refusal_max_attempts The open_with_retry function is modified to distinguish refused contexts and apply different retry policies The npu_contexts_refused function correctly identifies code 102 and 104 errors The npu_contexts_refused_hint function provides the correct operator guidance The documentation in docs/npu.md is updated to explain the new behavior Non-compliant requirements: The PR description mentions that the "old commit 4 (the refused-context hint)" was dropped on rebase, but this is not directly visible in the diff Requires further human verification: The behavior of the new queue mechanism needs to be verified on actual hardware to ensure it correctly distinguishes between busy and dead arrays The performance impact of the new retry logic needs to be measured 1010 - Partially compliant Compliant requirements: The --load-queue-ms flag provides a mechanism to wait for a busy NPU array instead of immediately failing The retry logic now distinguishes between refused contexts (which are queued) and other failures (which use the short window) The error messages now provide more context about why a retry is happening Non-compliant requirements: The PR doesn't directly address the timing issue with the 2-second sleep in the appliance, which was the root cause of the race condition Requires further human verification: The fix needs to be tested in the actual appliance environment to verify it resolves the race condition

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Incorrect retry policy selection The code in open_with_retry function incorrectly applies the refusal retry policy when a context is refused, but the logic for selecting which policy to use could be improved. Specifically, the load_retry_more function is called with the wrong parameters for the refusal case, which could lead to incorrect behavior when the refusal window is exceeded. if (!load_retry_more(p, attempt, elapsed, window, max_delay, max_attempts)) { Error message clarity The error message in open_with_retry function could be improved to better distinguish between the two different failure modes. The current message includes both the attempt count and the reason, but it could be clearer about which retry policy was applied. report(std::string(what) + " failed " + std::to_string(attempt) + " attempt(s) in " + std::to_string(int(elapsed * 1000)) + " ms" + (queued ? " over a " + std::to_string(int(window * 1000)) + " ms queue window" : "") + ", last reason: " + ex.what());

[bong-water-water-bong]

VERDICT: READY at 84fc1b38 — I built and ran the test myself (it is header-only, so no lane build needed), and the one risk this design carries is the one it tests.

The test, run by me:

$ gh api …/app/npu_load_retry.h > npu_load_retry.h ; g++ -std=c++17 tests/npu_load_retry_test.cpp
$ ./a.out
npu_load_retry_test: PASS

Ten cases at this head, and the three that decide whether the design is safe are:

• 8 — a refusal that never clears runs the whole queue budget, then gives up naming both causes;
• 9 — --load-queue-ms 0 is one attempt, i.e. the operator can refuse to queue (this is what makes the default survivable);
• 10 — "the queue budget is for refusals only: an ordinary failure still obeys the SHORT policy", with p.max_attempts = 3 and p.refusal_max_attempts = 9 set apart deliberately.

Case 10 is the risk I would otherwise have written into the review: a 45 s budget applied to every load failure would make a genuinely broken model take 45 s to fail. Splitting the schedule by npu_contexts_refused() is the correct shape, and the test pins it so a later refactor cannot quietly collapse the two.

The arithmetic closes on the defaults (refusal_window_s = 45.0, refusal_max_delay_s = 2.0, refusal_max_attempts = 60, --load-queue-ms default 45000 in both app/serve.cpp and the in-process run_unified): with a 2 s cap the window is what binds at ~45 s and the 60-attempt cap is a safety net, not the limit — so the two knobs cannot disagree into a longer-than-advertised wait.

The trade this makes, stated so nobody discovers it during a demo: a genuinely dead array now answers "reload the driver" after ~45 s instead of 2.5 s, because the code cannot tell dead from busy — that is the decision's whole premise, not an oversight. What makes it acceptable is that the new hint puts the right first move in front of the operator ("another client may hold the array (wait for it, or stop it); if nothing else is serving, reload the driver"), where the old string led with modprobe -r amdxdna — advice that is actively wrong when the array is merely busy. The ordering of that sentence is the fix.

Also checked: the queue applies on both routes (the --load-queue-ms flag and its help text are in run_unified as well, with the code-102 ambiguity written into the help rather than hidden in a source comment).

Where this sits in the decision: with #1085 (driver, per-client) and lemonade #34 (product, evict-on-second), this is the engine half of item 2 — the part that turns a refusal into a wait. All three read consistently to me; the only piece still unproven anywhere in the trio is hardware evidence for #1085's partition ownership, and #34's router-level row.

[github-actions]

Persistent review updated to latest commit 95c7773