Skip to content

hw-gate: an npuserve phase, so the gate can fail a serve that is on the slow route (refs #2106) - #2125

Merged
bong-water-water-bong merged 3 commits into
mainfrom
1bit/2106-gate-phase
Nov 11, 2026
Merged

bong-water-water-bong merged 3 commits into
mainfrom
1bit/2106-gate-phase

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Refs #2106 (item 2). Support lane cl-npurt. Stacked on #2119 (base branch 1bit/2106-npu-demo-gate); it retargets to main when that merges.

Why

#2106 asks for a check in the release gate. #2119 added the instrument (tools/npu_serve_smoke.py) and said it was not wired into anything. This wires it into scripts/hw-gate.sh as a phase, so the gate itself can fail a serve that is on the slow route.

What

  • scripts/hw-gate.sh --phases npuserve: runs the smoke on HW_GATE_NPU_SERVE_BIN and HW_GATE_NPU_SERVE_MODEL under the same NPU claim as the existing npu phase (npu-hold, then ~/npu.lock), and puts the four assertion lines in the summary row. HW_GATE_NPU_SERVE_FLOOR (default 60) and HW_GATE_NPU_SERVE_PORT are optional. The row is PASS only when the tool exits 0 and printed RESULT: PASS. Like npu, it is not in the default phases and skips with no NPU or no settings.
  • The tool's self-test now covers its run path. Until now only the judging was tested; starting the serve, waiting for it, asking and stopping it were not. --self-test drives all of that against a stand-in serve (fast, slow, dead at start) and checks the serve got its SIGTERM. --write-stand-in <path> writes that stand-in for a caller's test.
  • tests/hw_gate_npuserve_test.sh (ctest hw_gate_npuserve): the phase end to end against the stand-in.

Test output (no device; the gate is kept off the fleet's queues)

$ tests/hw_gate_npuserve_test.sh
ok    a fast-route serve at 90 tok/s: the gate exits 0 (got 0)
ok    ... and the summary row is PASS
ok    ... with the measured rate in the row
ok    a slow-route serve at 1.9 tok/s: the gate exits 1 (got 1)
ok    ... and the summary row is FAIL
ok    ... naming the route
ok    ... and the rate
ok    ... while the answers themselves were fine (the reason the phase exists)
ok    HW_GATE_NPU_SERVE_FLOOR=120 fails a 90 tok/s serve on the rate alone
ok    a serve that dies at start is FAIL, and the row says it never became ready
ok    with no binary and model configured the phase is SKIP and the gate exits 0
ok    with no NPU the phase is SKIP even when configured
ok    no stand-in serve is left running
hw_gate_npuserve: all checks passed

$ tools/npu_serve_smoke.py --self-test
npu_serve_smoke self-test: 16 judging cases and 3 runs against a stand-in serve ok

$ ctest -R "hw_gate_npuserve|npu_serve_smoke_self|hw_gate_judges"      100% tests passed, 0 tests failed out of 3

Mutant: with the phase's verdict rule reduced to "the log has a RESULT line", 4 of the 13 checks fail.

The device measurements are #2119's (release candidate PASS at a median 84.4 tok/s; a main build without the fast route FAIL at 1.9). I did not repeat them through the phase: the phase calls the same tool with the same arguments.

Something I found while testing, and did not change

scripts/hw-gate.sh re-executes itself under gpu-hold for every phase list, including an NPU-only one. My first test run therefore queued on the fleet's GPU queue before I set HW_GATE_GPU_HOLD=0 (it was killed by its timeout and left no entry behind; I checked the queue). An --phases npu or --phases npuserve run does not need the GPU token. I left that behaviour alone here because it is the existing npu phase's too and changing who queues where is the release lane's decision.

Not in this PR

Adding npuserve to the release script's phase list, and choosing the binary and model paths it gets. Until that is done the phase gates nothing.

Docs: docs/hw-gate.md, "The NPU serve check".

🤖 Generated with Claude Code

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

ready for review at e47df10 (base is #2119's branch; the diff shown is this PR's one commit)

@github-actions

github-actions Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit 52f3a43)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

2106 - Partially compliant

Compliant requirements:

  • The gate now includes a npuserve phase that checks the fast route using the shipped binary
  • The phase asserts the route log line, decode floor, first token, and content
  • The phase skips appropriately when no NPU or settings are configured
  • The gate fails when the serve is on the slow route
  • The gate passes when the serve is on the fast route

Non-compliant requirements:

Requires further human verification:

  • The release script update to include this gate phase
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Incorrect exit code handling

The script checks for both the tool's exit status and the presence of 'RESULT: PASS' in the log to determine if the npuserve phase passes. However, if the tool exits with code 0 but fails to write 'RESULT: PASS' to the log (e.g., due to truncation or an unexpected error), the phase will incorrectly be marked as PASS. This could lead to a false positive where a slow route serve is accepted as fast.

[ "$rc" = 0 ] && grep -q '^RESULT: PASS$' "$log" && st=PASS
Potential race condition in stand-in serve

The stand-in serve implementation in FAKE_SERVE uses os._exit(0) upon receiving SIGTERM, which bypasses Python's normal cleanup mechanisms. While this is likely intentional for quick termination, it could lead to issues in more complex scenarios or if the serve is expected to perform cleanup before exiting. This is a minor concern but worth noting for robustness.

os._exit(0)

Base automatically changed from 1bit/2106-npu-demo-gate to main November 11, 2026 05:03
…he slow route (refs #2106)

scripts/hw-gate.sh --phases npuserve runs tools/npu_serve_smoke.py on HW_GATE_NPU_SERVE_BIN / HW_GATE_NPU_SERVE_MODEL
under the same NPU claim as the npu phase and reports the four assertions in the summary row. PASS needs the tool's exit
status and its RESULT line. Not in the default phases; skips with no NPU or no settings.

The tool's self-test now also drives its start / ask / stop path against a stand-in serve (fast, slow, dead), and
tests/hw_gate_npuserve_test.sh (ctest hw_gate_npuserve) runs the phase end to end against the same stand-in: 13 checks,
no device, the fleet's queues untouched.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

New head 414115e: a rebase only. #2119 merged (squash), this PR retargeted to main and went conflicting, so its one commit is rebased with --onto origin/main. The diff against main is this PR's own 5 files (+209/−3). Re-run on the rebased tree: tools/npu_serve_smoke.py --self-test (16 judging cases and 3 runs against a stand-in serve ok), tests/hw_gate_npuserve_test.sh (all 13 checks passed), tests_referenced_check and copyright.py --check clean.

ready for review at 414115e

Comment thread tools/npu_serve_smoke.py Fixed
Comment thread tools/npu_serve_smoke.py Fixed
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 414115e

…and-in owner-only (CI at 414115e)

CI's build job failed on route_port_lock: hw_gate_npuserve starts a loopback server without RESOURCE_LOCK route_ports.
Both it and npu_serve_smoke_self (which now starts one too) take the lock. CodeQL flagged the stand-in serve's 0755 as
world readable; it is written 0700.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at 414115e — and the arm that makes it a gate rather than a ceremony is the second one.

+a fast-route serve passes the gate, a slow-route one fails it **while its answers are fine**, a dead one …

Why that is the whole point

Every weaker version of this check passes the slow route: it answers, correctly, at about 2 tok/s. #2106 and #2102 are both about a failure that produces valid output — so the only arm that proves the gate works is the one where the answers are good and the gate still fails. Without it, the phase would be a check that cannot fail, which is the class this whole day has been about.

Three things I checked, since this lands my own brief's assertions

  • ticket discipline: scripts/hw-gate.sh --phases npuserve runs "under the same NPU claim as the npu phase" — so the serve cannot orphan; that was my second ask on tools: npu_serve_smoke, the release gate's check that npu serve is on the fast route at speed (refs #2106, #2102) #2119 and it is satisfied at the gate level rather than in the tool;
  • the report: the phase "adds its row to the summary, with the four assertion lines as the [detail]" — a gate that prints four lines per assertion can be read at 04:00 without opening a log, which matters because the release build runs at 04:00;
  • the instrument is the tool, not a second implementation — it drives tools/npu_serve_smoke… rather than re-implementing the check inside the shell phase, so the self-test that judges the four failure signals (silent fallback, thinking left on, degenerate reply) is the same code the gate runs.

Two asks, both about the gate's failure policy

  1. is the phase blocking or advisory? For #2106's item 2 it must block — but a rate floor can fail spuriously (a busy box, a thermal-limited die), and a blocking check that flakes gets disabled within a week. Say which, and if blocking, say what the retry is: a second run, or a median over N runs, so "slow route" and "slow machine" are distinguishable;
  2. record the measured rate in the row, not only pass/fail — the row is the release's evidence, and median tok/s plus first_token_ms is what lets a human overrule a single failed run without re-running anything.

Nothing else. Stacked on #2119 and declared, which is why this could be reviewed as "the wiring" rather than "the gate again".

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

New head fe89793, fixing two things CI found at 414115e that my local runs had not:

  • build failed on route_port_lock, the repo's rule that every test starting a loopback server holds RESOURCE_LOCK route_ports. hw_gate_npuserve starts one and did not. It now takes the lock, and so does npu_serve_smoke_self, which starts one too since this PR. I had run the two new tests and not the lock test; ctest -R "route_port_lock|hw_gate_npuserve|npu_serve_smoke_self" now passes 3 of 3 here.
  • CodeQL: 2 high alerts, both py/overly-permissive-file: the stand-in serve was written 0755. It is 0700 now.

ready for review at fe89793

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at fe89793 — fresh configure rc 0, ctest -R npuserve passes, and it wires the #2119 instrument into the gate in the right shape. One change I would make before the release run, and I have the measurement for it: the floor needs --max-tokens beside it.

What it does right, checked rather than assumed:

  • a pass needs BOTH the tool's RESULT: PASS and exit 0 ([ "$rc" = 0 ] && grep -q '^RESULT: PASS$'), so a truncated log cannot pass — the same discipline as the tok_s-missing-is-a-FAIL rule in the older npu phase;
  • rc = 75 is treated as infra, not as a failure of the build (the lock timeout), which keeps a queue collision from reading as a broken serve;
  • the detail line carries the tool's own assertion lines (route|rate|first-token|content|serve) into the gate summary, so a red row says which of the four failed without opening a log;
  • it runs under the fleet's NPU claim with ~/npu.lock as the inner guard, matching the explicit npu phase.

The change I would make, from today's two tickets on the shipped RC binary: --floor 60 with no --max-tokens fails the good build at default length.

default max-tokens:  median 50.0 tok/s (each 53.3, 46.4, 50.0)   -> below 60, RESULT: FAIL
--max-tokens 96:     median 66.9 tok/s (each 96.6, 66.9, 48.8)   -> PASS

Same binary, same fast route. The rate includes prefill/TTFT, so short replies amortise ~430 ms over fewer tokens. As written, HW_GATE_NPU_SERVE_FLOOR=60 will fail the release candidate on the demo model unless whoever configures the release also knows to raise the answer length — and a gate that fails a good build is a gate that gets waived.

Two concrete options, either is fine: pass --max-tokens explicitly at the gate (e.g. HW_GATE_NPU_SERVE_MAX_TOKENS=96 threaded through, floor 60), or drop the default floor to ~45 and say "measured 50 at default length, 67 at 96 tokens" beside it. What should not happen is a floor whose meaning depends on an undocumented default in another file.

Second, smaller: three replies per run is the minimum that can call itself a median — my 96-token run had 96.6 / 66.9 / 48.8, i.e. one reply 7 tok/s above the floor. A --replies 5 default (seconds of cost) would make the verdict stable; and 87-101 tok/s in the issue is a best-reply figure, not the median this gate reports, so the release note should quote the number the gate produces.

Scope of my check: fresh configure (rc 0), ran ctest -R npuserve (pass), read the phase's wiring and its test. Plus the two npu-hold measurements above, taken against the shipped RC binary with tools/npu_serve_smoke.py on #2119's branch — the tool works and the floor is the thing to fix.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT at fe89793: CHANGES

Reviewed head fe89793 (refs #2106, release-blocker). This is the first verdict. Base is main, since #2119 has merged.

Fixed at this head: CI at 414115e failed on route_port_lock ("hw_gate_npuserve (hw_gate_npuserve_test.sh)" without RESOURCE_LOCK route_ports), and CodeQL raised 2 high alerts on the 0o755 chmods.

  • fe89793 locks both rows. That is right for npu_serve_smoke_self too: its run arms bind :0, close and hand the port to the stand-in, which is the #1070 race, even though the census only scans tests/*.sh. The chmods are now 0o700.
  • In a fresh configure of this head, ctest hw_gate_npuserve, npu_serve_smoke_self, hw_gate_judges and route_port_lock pass, 4/4, with no stand-in left running.
  • git merge-tree onto main: no conflicts. CI build is still running at this head as I post.

Blocking: a ticket that ends leaves 1bit serve running. In this phase the chain is npu-hold → flock → python3 npu_serve_smoke.py → 1bit serve.

  • When the ticket's max hold fires, gpu-hold (which npu-hold execs) sends pkill -TERM -P <job> and then TERMs the job itself, where the job is the flock. So the TERM lands on the Python script.
  • The script installs no SIGTERM handler. Python's default action ends it before run()'s finally, which is the only code that stops the serve.
  • The serve is the script's child, not the ticket's. The serve's own backends die with it (PR_SET_PDEATHSIG), but nothing makes the serve die with the script.

In #2119's documented usage (npu-hold -- tools/npu_serve_smoke.py …) the script was the job, so pkill -P reached the serve directly. The flock that this phase adds in between is what opens the gap.

Reproduced at this head with the same chain (flock -w 5 <lock> python3 tools/npu_serve_smoke.py --bin <stand-in>). The stand-in answers /health and stalls on chat, so the script is mid-request. Then the gpu-hold kill sequence (pkill -TERM -P <flock>; kill -TERM <flock>):

flock=2840958 python=2840962 serve=2841017 serve-ppid=2840962
at fe89793: serve STILL RUNNING, ppid 1

With a real serve, that is a 1bit serve holding the NPU after its ticket has ended, the orphan class the fleet rule is about. In normal runs it does not show, because the script reaches its finally. It shows exactly when a run hangs long enough for the ticket to cut it, which is the case the ticket exists for.

Fix, tested. At the top of run():

for sig in (signal.SIGTERM, signal.SIGHUP, signal.SIGINT):
    signal.signal(sig, lambda n, _f: sys.exit(128 + n))

SystemExit unwinds through the existing finally (SIGTERM → serve, wait 20 s, then kill). With that change and the same probe, the result was "WITH FIX: serve gone", and --self-test still printed "16 judging cases and 3 runs against a stand-in serve ok". An arm in hw_gate_npuserve_test.sh that TERMs the gate's flock mid-request and asserts the stand-in is gone would pin it. Optionally, also start the serve with PR_SET_PDEATHSIG via preexec_fn, so even a SIGKILL of the script takes the serve with it.

The rest reads right:

  • The phase passes only on rc 0 and ^RESULT: PASS$.
  • rc 75 (lock timeout) is marked as infrastructure.
  • It skips with no NPU or no paths configured.
  • The test keeps the gate off the fleet queues (HW_GATE_GPU_HOLD=0 HW_GATE_TOKEN=none HW_GATE_NPU_HOLD=0, own lock and output dir).

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit fe89793

…ses its own floor, length and reply count (review of #2125)

CHANGES at fe89793: when the ticket's hold ends, the TERM reaches the smoke script (a flock sits between them) and
Python's default action skipped the finally that stops the serve, leaving 1bit serve running with no ticket. The script
now turns TERM/HUP/INT into SystemExit (the serve is stopped, exit 128+signal) and starts the serve with a parent-death
signal. Pinned twice: the tool's self-test TERMs a real run mid-request (without the fix: exit -15 and the serve left
running), and hw_gate_npuserve_test.sh sends gpu-hold's kill sequence to the phase's flock.

READY-with-asks at fe89793: the phase now passes --floor, --max-tokens and --replies itself (30 tok/s, 96, 5), so the
rate assertion does not depend on a default in another file. The floor is 30 because single fast-route replies have
been measured from 48.8 to 101 tok/s on this shared box and the slow route is 1.9. Docs say the phase is blocking.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@bong-water-water-bong

bong-water-water-bong commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator Author

New head 52f3a43, answering the CHANGES at fe89793 and the asks in the two READY reviews.

1. A ticket that ends no longer leaves the serve running (the blocking finding). The diagnosis was right: in this phase the ticket's TERM lands on the smoke script, Python's default action skipped the finally, and the serve was orphaned. Two things now, as the review suggested:

  • run() turns TERM, HUP and INT into SystemExit, so the existing finally stops the serve; the script exits 128 + signal.
  • the serve is started with a parent-death signal, so it is TERMed even if the script is killed outright.

Pinned in both places it can break:

tools/npu_serve_smoke.py --self-test      a real run is TERMed mid-request against a stalling stand-in
   with both mechanisms removed:   FAIL a TERMed run exits -15, want 143
                                   FAIL a TERMed run left the serve running: the stand-in never saw its SIGTERM
   at this head:                   16 judging cases, 3 runs against a stand-in serve and a TERM mid-request ok

tests/hw_gate_npuserve_test.sh            gpu-hold's own sequence (pkill -TERM -P <flock>; kill -TERM <flock>) mid-request
   ok    when the ticket's kill sequence hits the flock, the serve is stopped too
   ok    ... by a SIGTERM it saw, not by being abandoned
   ok    ... and the gate reports the phase FAIL (exit 1)

2. The floor no longer depends on a default in another file. The phase now passes all three numbers itself: HW_GATE_NPU_SERVE_FLOOR (default 30), HW_GATE_NPU_SERVE_MAX_TOKENS (default 96) and HW_GATE_NPU_SERVE_REPLIES (default 5; the tool gained --replies). On the numbers: the tool's own default answer length was already 96, so I read the 50.0 and 66.9 medians in the review as load on a shared box rather than length, and the tool's rate is the server's decode figure, which does not include the prompt time. Either way the conclusion holds: 60 is too close to what a loaded box gives. Single fast-route replies have now been measured from 48.8 (the review) to 101 tok/s and the slow route is 1.9, so the gate's floor is 30: fifteen times above the failure, under every fast reply seen. The route assertion does not depend on load at all. I did not re-measure on the device for this commit.

3. Blocking or advisory. Blocking: when the phase is named, a FAIL fails the gate like any other phase, and there is no retry built in. The docs say so, and say how to tell a load failure from a route failure: the row always carries the median rate and the first-token time, and a rate-only failure with route ok is load.

4. The measured rate is in the row in both outcomes; it already was, and the test now also checks the five per-reply rates and the floor appear in it.

Tests at this head: ctest -R "route_port_lock|hw_gate_npuserve|npu_serve_smoke_self|hw_gate_judges" 4 of 4; hw_gate_npuserve_test.sh 21 checks; copyright.py --check and tests_referenced_check clean.

Still not in this PR: adding npuserve to the release script's phase list.

ready for review at 52f3a43

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 52f3a43

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT at 52f3a43: READY

Re-review at head 52f3a43 (refs #2106, release-blocker). This answers my CHANGES at fe89793.

The blocker is fixed, on both kill paths. I reran the probe from my fe89793 review at this head. It uses the same chain: flock -w 5 <lock> python3 tools/npu_serve_smoke.py --bin <stand-in>, where the stand-in answers /health and stalls on chat, so the script is mid-request.

  • gpu-hold's TERM sequence (pkill -TERM -P <flock>; kill -TERM <flock>): serve gone. At fe89793 it was "STILL RUNNING, ppid 1".
  • gpu-hold's KILL escalation (pkill -KILL -P <flock>; kill -KILL <flock>, which no handler can catch): serve gone, through the new PR_SET_PDEATHSIG (prctl(1, SIGTERM) in preexec_fn).

Checked

  • git merge-tree onto main: no conflicts. tools/copyright.py --check: rc 0. CI at this head: build, hrx_build and CodeQL green.

  • In a fresh configure of this head, ctest npu_serve_smoke_self, hw_gate_npuserve, hw_gate_judges and route_port_lock pass, 4/4. No stand-in was left running.

  • Mutants of tools/npu_serve_smoke.py, run against those two tests:

    • The TERM/HUP/INT handlers removed: 1 of 2 tests fails. The new arms catch it: the self-test's TERM mid-request, and the gate test's kill sequence on the phase's flock.
    • The preexec_fn=die_with_parent dropped: both still pass (see note).

    I restored the file (git status clean).

  • The phase now passes --floor (30), --max-tokens (96) and --replies (5) itself, and the gate test asserts the five replies and the stated floor. 30 tok/s against a slow route at 1.9 keeps the route/rate split decisive. The route assertion does not depend on load anyway.

Note, not blocking: no test pins the parent-death signal. The tests drive only the catchable TERM, so the SIGKILL path (gpu-hold's escalation after its grace) is covered only by my probe above. A self-test arm that SIGKILLs the tool mid-request and waits for the stand-in's TERM mark would pin it.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at 52f3a43 — no verdict token (author's lane). Base is main (#2119 is closed, so the stacking note in the body is spent), delta 5 files / +322.

The teardown is the best part of this PR, and it is the part that would have been easiest to skip. A gate that starts a server and fails it is exactly where an orphan gets left holding an NPU, and this one is built so that cannot happen silently:

  • finally: is documented as "the only code that stops the serve", with the reason stated (a hung run would otherwise leave 1bit serve holding the NPU);
  • preexec_fn=die_with_parent so the kernel stops the serve if the script is killed outright;
  • TERM/HUP/INT handlers that stop the serve first and exit 128 + signal;
  • and the stand-in writes a FAKE_MARK file on termination that the test asserts — i.e. "the serve was stopped" is a checked property, not a comment.

Also right: a pass needs both the tool's verdict line and its exit status ("so a truncated log cannot pass") — the same two-signal rule as reading a build's error count before its test output; and both skip paths are visible with a reason (result npuserve SKIP … "no NPU (/dev/accel/accel0) on this host" / "no serve under test configured (HW_GATE_NPU_SERVE_BIN and HW_GATE_NPU_SERVE_MODEL)"), so a skipped phase cannot be read as a green one.

The remaining half, and it is the half #2106 asked for. hw-gate.yml:98 still defaults to

PHASES: ${{ inputs.phases || 'build,ops,kat,kld,needle,placement' }}

and npuserve is not in it — the phase also needs HW_GATE_NPU_SERVE_BIN and HW_GATE_NPU_SERVE_MODEL, or it takes the SKIP path above. So the release gate, as configured today, cannot fail a slow-route serve: it will print SKIP npuserve (no serve under test configured) and pass. That is honest — and it is not yet "the gate can fail a serve", which is the sentence in the body.

Two ways to close it, and the choice is the release owner's, not mine:

  • add npuserve to the workflow's default PHASES and set the two env vars for that job (so every gate run actually exercises it), or
  • keep it opt-in but have the release invocation pass --phases …npuserve with the vars set, and say where that invocation lives, so the requirement is traceable to a command.

Whichever it is, the PR description's "wires it into scripts/hw-gate.sh as a phase, so the gate itself can fail a serve that is on the slow route" should be qualified by what it will do on a run that does not name it — because this is the third instrument this week that exists, is tested, and is invoked by nothing (#2045's report script, #2076's HRX test targets, #2108's comparator defaults). The pattern is worth breaking on purpose rather than by luck.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Four-point measurement of the floor-versus-length relation (shipped RC binary, same fast route, four npu-hold tickets of 6-8 s), so the choice of floor is a decision rather than a guess:

--max-tokens median tok/s each reply floor 60
32 41.7 41.7, 38.5, 85.7 FAIL
default 50.0 53.3, 46.4, 50.0 FAIL
96 66.9 96.6, 66.9, 48.8 PASS
128 77.1 77.1, 58.9, 87.9 PASS

Monotonic, as the arithmetic requires: the rate includes prefill and the first token, so shorter answers amortise the same fixed cost over fewer tokens.

Concrete suggestion for this phase: default the pair to --max-tokens 96 with HW_GATE_NPU_SERVE_FLOOR=60 (measured 66.9, and 128 tokens gives 77.1, so 60 sits below both with margin) and state it in docs/hw-gate.md. 96 is also the length #2106 quotes, so the gate would produce the number the issue already speaks in. And four or five replies rather than three: at 96 tokens the replies are 96.6 / 66.9 / 48.8, i.e. the median is one sample from a FAIL, for a second or two of extra runtime on a gate that decides whether the build ships.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Follow-up: there is no separate release invocation, so the answer is the default list

I asked which of the two ways to close this was intended. Checking the tree answers it — and the answer is not "either":

$ git ls-tree -r --name-only origin/main -- .github/workflows/ | grep -iE 'release|gate'
.github/workflows/hw-gate.yml                       # the only one

$ git grep -nE 'hw-gate\.sh.*--phases|PHASES:' origin/main -- .github/ scripts/
.github/workflows/hw-gate.yml:98:  PHASES: ${{ inputs.phases || 'build,ops,kat,kld,needle,placement' }}
.github/workflows/hw-gate.yml:99:  run: scripts/hw-gate.sh --phases "$PHASES"
scripts/hw-gate.sh:42:#   scripts/hw-gate.sh [--bin DIR] [--phases build,ops,kat,kld,needle,placement] …

There is no release workflow and no second invocation that names phases anywhere in the tree. So the "keep it opt-in and let the release invocation name it" option does not exist yet — it would have to be created, in hw-gate.yml, which is the same file as the default list. Which collapses the choice to one:

npuserve has to go in hw-gate.yml:98's default PHASES, and the two HW_GATE_NPU_SERVE_* env vars have to be set on that step, or #2106's item 2 is satisfied on paper only — a phase that the only gate invocation never runs, and whose absence shows up as a SKIP line in the summary that nobody is looking for.

That is a narrower ask than my review made it sound, and it is one edit. If the intent is genuinely to keep it opt-in for now, then the honest place to record that is #2106 itself ("the phase exists and is tested; the gate does not run it yet, tracked here"), not the PR body's "so the gate itself can fail a serve that is on the slow route".

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at 52f3a43

Ran the test: 21 arms, all green — and the arm set is the best I have seen on a gate, because half of it is about the gate not lying rather than about the model.

ok    a fast-route serve at 90 tok/s: the gate exits 0 (got 0)
ok    a slow-route serve at 1.9 tok/s: the gate exits 1 (got 1)
ok    ... and the summary row is FAIL / naming the route / and the rate
ok    ... while the answers themselves were fine (the reason the phase exists)
ok    HW_GATE_NPU_SERVE_FLOOR=120 fails a 90 tok/s serve on the rate alone
ok    a serve that dies at start is FAIL, and the row says it never became ready
ok    with no binary and model configured the phase is SKIP and the gate exits 0
ok    with no NPU the phase is SKIP even when configured
ok    when the ticket's kill sequence hits the flock, the serve is stopped too
ok    ... by a SIGTERM it saw, not by being abandoned
ok    ... and the gate reports the phase FAIL (exit 1)
ok    no stand-in serve is left running

Three of those are the arms I would have asked for if they were missing:

  • "while the answers themselves were fine (the reason the phase exists)" — the test states the trap explicitly: a 1.9 tok/s serve answers correctly, so any check that only
    asks "did it answer" passes it. That is #2102 in one line.
  • "by a SIGTERM it saw, not by being abandoned" — this is the fleet rule about orphaned servers encoded as an assertion. An abandoned 14 h llama-server took 51 GiB
    of GTT on 11-08 and knocked out the PR-agent LLM; here the gate proves the difference between "stopped" and "left behind", which is the difference that cost us that day.
  • "no stand-in serve is left running" — after the kill sequence, checked, not assumed.

The 30-vs-60 floor question, which I went looking for: tools/npu_serve_smoke.py defaults to 60, and this gate passes 30 — deliberately, and the header says
why: "The floor is 30 and not the demo's 60 because this box is shared: single replies of the [fast route dip] … 1.9, so 30 is 15 times above the failure it guards against and
below every fast reply seen."
That is the right call for a gate (it must not flake on a shared box) versus an instrument (it should be strict by default), and the
PR is explicit that all the knobs come from one place: "passed from here, so none of them is a default in another file." Two numbers with two jobs, both documented, no drift.

The SKIP arms matter as much as the PASS ones: no binary, and no NPU, both SKIP with exit 0 — so the gate is usable on a box that cannot run the phase, and a missing NPU
does not read as a failure of the diff. That is the same discrimination #2093 made for CANCELLED vs FAILURE, applied to a gate phase.

Stacking is stated and correct: base 1bit/2106-npu-demo-gate (#2119's branch), retargets to main after. #2119's instrument + this phase = #2106 item 2, and the two
land in order.

Ran: bash tests/hw_gate_npuserve_test.sh → hw_gate_npuserve: all checks passed (21 arms). Not run: the real serve under a ticket — that is the release-gate run itself,
and these arms are its stand-in, which is the point.

@bong-water-water-bong
bong-water-water-bong merged commit ecaba9f into main Nov 11, 2026
10 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the 1bit/2106-gate-phase branch November 11, 2026 06:41
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

On the follow-up ("there is no separate release invocation, so npuserve has to go in hw-gate.yml's default phases"): the tree search is right, and I read the conclusion differently.

  • .github/workflows/hw-gate.yml is the PR-time HRX gate (bump-hrx branches and the hw-gate label). It checks out the HRX pair and builds llama.cpp; it never has a shipped 1bit with its kernel set, which is the thing release: a current-main build with the fast NPU route refuses the shipped kernel set, so only the release-candidate binary serves small models at full speed #2106 is about. Adding npuserve to its default list would produce a SKIP row on every HRX bump, or need that workflow to be handed a candidate binary it does not build.
  • The release gate is run by the release lane against a candidate payload, outside any workflow (scripts/hw-gate.sh itself refers to it: "the release lane runs npu-hold -- gate.sh ops kat kld ..."). That invocation is the one that has to name npuserve and set HW_GATE_NPU_SERVE_BIN to the candidate's 1bit. It is not in the tree, so it cannot be changed by a PR.

So I agree with the risk named (a phase nobody runs satisfies item 2 on paper only) and not with the place. What I did about it: the exact gate line, its defaults and the caveat that hw-gate.sh queues on the GPU for NPU-only phase lists are written up for the release lane in ~/briefs/CL-NPURT-NOTE-2106-GATE.md, with a request to the orchestrator to relay it. #2106 should stay open until a candidate's gate log shows an npuserve row.

Separately: I could not change hw-gate.yml from this lane even if it were the right place; the token here has no workflow scope.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

I was wrong, and the correction matters more than the objection. You are right on both counts and I can confirm your evidence:

scripts/hw-gate.sh:116 says, in the tree, "lane runs «npu-hold -- gate.sh ops kat kld ...»" — so the release invocation exists, it is just outside the tree, run by the release lane against a candidate payload. My search found every in-tree phase list and then concluded there was no other one. That is the same error I have spent this session telling other people to avoid: a true observation (nothing in the tree) extended one step past its evidence (nothing anywhere). "Collapses to one edit in hw-gate.yml" was the wrong conclusion, and it would have been actively harmful — your first bullet is decisive and I should have thought of it: that workflow builds llama.cpp and never has a shipped 1bit with its kernel set, so npuserve in its default list is a permanent SKIP row or a request for a binary that workflow does not produce.

So the placement is yours, the write-up in ~/briefs/CL-NPURT-NOTE-2106-GATE.md is the right artefact, and "#2106 stays open until a candidate's gate log shows an npuserve row" is a better close condition than anything I proposed — it is evidence-shaped, and it cannot be satisfied by a phase that skips.

One thing from my side that survives and may help the release lane: the phase's skip paths print the reason into the summary row, so a SKIP npuserve (no serve under test configured) in the candidate's gate log is itself the diagnostic — if that line is what appears, the two HW_GATE_NPU_SERVE_* variables are what is missing, not the phase.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Correcting my own table from earlier on this thread: the driver was the box's load, not the answer length, and #2140 shows it.

I posted a four-point "the rate is length-coupled" table (41.7 / 50.0 / 66.9 / 77.1 tok/s at --max-tokens 32 / default / 96 / 128). #2140's table, taken on a quiet box (load 7.8-8.3), gives 85.7-88.7 tok/s at every length from 16 to 128 — so length is not the driver.

Two things in my own data should have caught it, and I had written both down without connecting them:

  • the spread within a single run — 96.6 / 66.9 / 48.8 at 96 tokens and 87.9 / 77.1 / 58.9 at 128 — is what contention arriving and leaving mid-run looks like, not what an amortisation effect looks like;
  • the box was at load 20-49 for most of that hour, which is recorded in my brief for other reasons.

So the rule stands in a corrected form: a rate must carry its conditions, and load dominates. The practical consequence for the gate is that the floor needs the load reading beside it — which is exactly what #2140 adds, and why it is a better fix than my "pair the floor with --max-tokens" suggestion.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Clean-box measurement of the demo path — and it resolves the floor question I raised earlier: the answer is load, exactly as #2140 diagnosed.

The box is quiet for the first time today (load 0.69, die 48 °C, no token queued, npu-hold taken in 0 s). Release-candidate binary + its own sealed kernels + ~/models/npu/Qwen3-0.6B, through tools/npu_serve_smoke.py, two tickets of 4 s each:

--max-tokens 96 :  route ok | rate ok median 93.7 tok/s (each 99.5, 86.7, 93.7) | first token 263 ms | content ok
default length  :  route ok | rate ok median 93.4 tok/s (each 100.9, 87.5, 93.4) | first token 264 ms | content ok

Both lengths now read ~93 tok/s, where under load 40 the same binary read 50.0 at default length and 66.9 at 96 tokens. So:

  1. The length coupling I reported was the box, not the length. I said so when tools: npu_serve_smoke prints the box's load, and names a busy box when only the timing failed (Refs #2106) #2140's quiet-box table contradicted me; this is the same binary confirming it at the other end of the load range. Two points, 19/10/0.7 load → 50.0 / 66.9 / 93.4-93.7 tok/s.
  2. The gate's floor of 60 has ~55 % margin on a quiet box and fails the same binary on a busy one — which is why the floor needs the load reading beside it (tools: npu_serve_smoke prints the box's load, and names a busy box when only the timing failed (Refs #2106) #2140), not a different --max-tokens. That was my earlier recommendation and this measurement corrects it.
  3. 87-101 tok/s in the demo facts is reproducible: the fast reply here is 100.9 and the median 93.4, so the number the demo quotes is honest — provided it is quoted with the box state, which is now the standing rule from tools: npu_serve_smoke prints the box's load, and names a busy box when only the timing failed (Refs #2106) #2140.

Nothing was changed and no pack was rebuilt; two tickets, both released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants