the coverage followup - the bind-number gate, the orchestrator's instruments, the fixture rail - #3911
Merged
Merged
Conversation
A hand-written kn_buffer/kn_kargs literal in a race/knockout arm must land on a binding number its target kernel class declares. The class resolves from the arm's kn_tgmem constant or from kn_pipeline's pso built by pipeline_from_source in the same function; a @uniform slot legitimately takes a 4-byte buffer, so the buffer check is kind-agnostic. A mis-numbered arm dispatches, reads the wrong buffer, and race_envelope_ok fails CLOSED - the twin silently loses its crown (the mx4 race shipped exactly that way and halved a board cell). Controls: re-introducing the historic mx4 mis-bind (bxa at 2) reds; a kargs at an undeclared slot reds; the clean tree is green (36 hand-bound literals checked across 103 mapped kernels). Charter line beside the restore check; REVIEW_GPU.md's hand-binding-arm rule keeps the right-number-wrong-field half for the reviewer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LY6gNeDZkF14X81rLz3QLc
- asr_pybench_args' per-tool arms pinned (onnx carries the --out recovery file and never a device; nemo forwards --device; clip fan-out and threads/reps literals) - the gpu-pair roster extracted to asr_gpu_pair_tool (profile_common) and pinned - the tools with a GPU twin leg beside their CPU pass - the ZERO-cells refusal extracted to records_run_verdict (pure; the orchestrator tail consumes it) - a run that measures nothing carries the red verdict, testable without the environment-hostage spawn - canary_qwen_bench.py's pure seams (rep_is_degenerate, build_prompts-fresh-per-rep) get a --self-test that imports no torch/nemo, spawned from the model-free suite - the crowned lanes get their tolerance-graded whisper twin: crowns pinned ON, WORD-level transcript equality - rounding drift that flips a comma passes, a garbled crowned kernel does not (followup 60's ruled resolution; per-lane gating stands) Every new assert ran its must-red control: roster minus nemo reds, the verdict condition flipped reds, a garbled crowned transcript reds; test_gen_records_args 9/9, the crowned twin green on the live crowned box (the comma flip visible in the eyeballs, words equal). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LY6gNeDZkF14X81rLz3QLc
capture_catalog.das boots the real setup-mode server on a scratch catalog dir, downloads the SMALLEST card for real (3.1 GB, sha-verified by the server's own gate), snapshots /catalog mid-flight and at the terminal state, normalizes machine-local paths to the fixture convention, and deletes the scratch download. catalog_downloading.json and catalog_done.json are its output - the 7-card roster with genuine downloading/done states - and the fixtures README row now names the rail instead of the by-hand mid-flight curl. live-switch.spec synthesizes its second-present-row precondition on a clone (withE4bPresent, the withVisionPresent pattern) - the rail downloads one card, and specs own their preconditions. Playwright 106/106. Two rail bugs found by running it: the download route answers 202 Accepted, and the server's terminal download state is "done" - the first run logged a spurious refusal and the poll waited on "idle" forever. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LY6gNeDZkF14X81rLz3QLc
…arry real teeth Woodpecker + tdd + five checklist auditors over the branch, every accepted finding landed: - check_race_bind_numbers reached only 41 of 94 hand-bind literals: kernel annotation names carry their own _msl suffix, so the resolver's stripped token never matched a kernel key and every race BASE arm was invisible (the tdd audit proved it with a bulk-mutation census; the dasLLAMA auditor with a base-arm control). The resolver now tries both spellings - 64/94 checked, the base-arm control reds - and the licensed residue (arms whose pso/tgmem arrive as function parameters) is stated in the charter and in REVIEW_GPU.md's bounded machine-checked claim. Also: clear, never delete, the reused per-function table (the woodpecker's P1 pattern). - canary_qwen_bench.py exits 1 on zero BENCH rows (an all-degenerate run was a green harness), and the argless refusal has its assert; one live spawn proves a BENCH row (jfk 2558.4 ms, rc 0) with the timed region unchanged. - the capture rail waits for a real 5% mid-flight snapshot (a 32KB first-chunk capture pinned 0% and hollowed the progress-render assert - the tdd audit's WEAKENED verdict); fixtures re-captured, playwright 106/106, and the committed fixtures' path normalization is pinned by a must-red cell in test_model_catalog. - the crowned word-level cell earns its verdicts: non-Metal skip, tower/wdec knobs established and restored (the wdec step seat lowered for tiny, the census's same seat), and real engage witnesses - the mixed lane's tower encode plus wdec step deltas. A tensor-QK garble control showed tiny's attention does not take that route, so the cell's claim is the crowned PIPELINE; per-variant kcov witnesses are followup 60b. - gen_bench_records' failure arm routes through records_run_verdict (both arms production-reachable), asr_gpu_pair_tool carries the [arch] citation instead of prose, and the checklist repairs land across tests (skip-key property, family-tag scoping, box-decided predicate scope, lane-knob roster with whisper's knobs, the stage+mint rule's named APIs and its facade-knob residue = followup 61), benchmarks (engine-scoped tune_gate, reported-walls subtraction), dasLLAMA (attribution names the property), and the server (captured-body property form, the playwright-suite duty, transitive-require routing). Suites green on the settled tree: model-free (incl. whisper's model-gated cells under the runner env), test_model_catalog 18/18, playwright 106/106, the gate green interp+jit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LY6gNeDZkF14X81rLz3QLc
Contributor
There was a problem hiding this comment.
Pull request overview
Follow-up PR that tightens dasLLAMA review/test gates and improves server fixture regeneration to prevent silent regressions in GPU hand-binding and measurement harness logic.
Changes:
- Add/extend review gates and docs around Metal race/knockout hand-bind validation (
check_race_bind_numbers) and related GPU review policy. - Add model-free test coverage for measurement orchestrator “pure seams” and canary bench script self-test hooks.
- Add a scripted fixture capture rail for dasllama-server catalog fixtures and update Playwright tests/fixtures accordingly.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/dasllama-server/tests/live-switch.spec.js | Synthesizes a second “present” catalog row in tests now that capture rail downloads only one card. |
| utils/dasllama-server/tests/fixtures/README.md | Documents scripted capture rail for catalog fixtures. |
| utils/dasllama-server/tests/fixtures/catalog_downloading.json | Updates mid-flight catalog snapshot fixture to new capture rail output. |
| utils/dasllama-server/tests/fixtures/catalog_done.json | Updates post-download catalog snapshot fixture to new capture rail output. |
| utils/dasllama-server/tests/fixtures/capture_catalog.das | New scripted fixture regeneration rail that boots server, downloads one real card, snapshots /catalog, normalizes paths, and cleans up scratch dir. |
| utils/dasllama-server/test_model_catalog.das | Adds a test pinning fixture path normalization (no machine-local path leakage). |
| utils/dasllama-server/REVIEW.md | Tightens fixture authoring/capture rules and requires Playwright runs when fixtures/control page change. |
| modules/dasLLAMA/tests/test_whisper.das | Adds crowned word-level equality test with normalization and GPU-engage witnesses. |
| modules/dasLLAMA/tests/test_gen_records_args.das | New model-free tests for orchestrator seams: pybench args, GPU pair roster, canary self-test, and zero-cells verdict. |
| modules/dasLLAMA/tests/run.das | Registers the new test_gen_records_args.das in the suite list. |
| modules/dasLLAMA/tests/REVIEW.md | Clarifies skip-gate rules, lane-pin/loader invariants, family tagging scope, and transcript grading policy. |
| modules/dasLLAMA/tests/CLAUDE.md | Documents new test coverage and the crowned word-level gate intent. |
| modules/dasLLAMA/REVIEW.md | Broadens the “no attribution” rule to all checked-in prose/comments owned by the module. |
| modules/dasLLAMA/REVIEW.das | Adds check_race_bind_numbers and supporting helpers; wires the new gate into main(). |
| modules/dasLLAMA/REVIEW_GPU.md | Aligns hand-binding review duties with what the new machine check can/can’t see. |
| modules/dasLLAMA/performance/profile_common.das | Extracts asr_gpu_pair_tool and records_run_verdict utilities used by orchestrator/tests. |
| modules/dasLLAMA/performance/gen_bench_records.das | Uses extracted helpers for GPU-pair logic and run verdict (including zero-cells refusal). |
| modules/dasLLAMA/followup_general.md | Records followups for per-variant crowned witnesses and facade-knob image-off seam. |
| modules/dasLLAMA/benchmarks/REVIEW.md | Clarifies tune gate applicability and raw-wall reporting rule wording. |
| modules/dasLLAMA/benchmarks/asr/canary_qwen_bench.py | Adds torch-free self-test seams, argless refusal, prompt rebuild per rep, and zero-BENCH-row failure. |
| modules/dasLLAMA/ARCHITECTURE_ENGINE.md | Documents the new check_race_bind_numbers gate and its licensed residue. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ssert cleanly
- normalize_paths maps BOTH separator spellings of the scratch dir (a windows capture box
emits JSON-escaped backslashes) and panics rather than write a fixture any spelling
survived in - fail-closed beats clever matching
- the args-builder cells read flag values through arg_value ("<absent>" on a miss) - a
future builder change asserts cleanly instead of indexing out of bounds
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LY6gNeDZkF14X81rLz3QLc
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
utils/dasllama-server/tests/live-switch.spec.js:23
- withE4bPresent hard-codes the models directory string even though the catalog fixture already provides
models_dir. Deriving the row path fromd.models_diravoids duplicating the convention and prevents the helper drifting if fixtures are re-captured with a different normalized prefix.
e.path = 'C:\\Users\\user\\.dasllama\\models\\' + e.file;
modules/dasLLAMA/REVIEW.das:264
- rb_scan_classes parses class names by taking ident_prefix immediately after "class ", which mis-identifies declarations like
class private Foo : ...as class nameprivate. That can break base-class tracking and binding-number collection, causing check_race_bind_numbers to miss or mis-attribute declared bindings.
var rest = slice(line, length("class "))
if (rest |> starts_with("template ")) {
rest = slice(rest, length("template "))
}
cur = ident_prefix(rest)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The kitchen-sink followup ruled after #3909: three items, one PR.
1. The race-arm bind-number gate
check_race_bind_numbersinmodules/dasLLAMA/REVIEW.das: every hand-writtenkn_buffer/kn_kargsliteral in a race or knockout arm must land on a binding number its target kernel class declares. The class resolves from the arm'skn_tgmemconstant - with or without the kernel's own_mslsuffix (the resolver bug the round caught: annotation names carry it, so every race BASE arm was initially invisible; the fixed gate covers 64 of the 94 hand-bind literals) - or fromkn_pipeline's pso built bypipeline_from_sourcein the same function. The licensed residue - arms whose pso/tgmem arrive as function parameters - is stated in the charter and inREVIEW_GPU.md's bounded machine-checked claim. Controls: re-introducing the historic mx4 mis-bind reds, a base-arm poison reds, a kargs poison reds; the clean tree is green interp+JIT. This is the gate for the mis-bind class that silently un-crowned the mx4 tensor twin and halved a board cell - the race's own envelope check fails closed, so a mis-numbered arm just loses and nobody learns.2. The measurement orchestrator's instruments
The tdd audit of #3909 found the orchestrator side bare; the pure seams now exist and are pinned, each with a must-red control:
asr_pybench_argsper-tool arms (onnx carries the--outrecovery file and never a device; nemo forwards--device; clip fan-out, threads/reps literals)asr_gpu_pair_tool- the GPU-twin roster extracted from the inline clause,[arch]-cited to#asr-gpu-pairs(per-citer audit: no drift)records_run_verdict- the ZERO-cells refusal extracted pure, with the orchestrator's failure arm routed through it so both arms are production-reachablecanary_qwen_bench.pygains torch-free pure seams (rep_is_degenerate, per-repbuild_prompts) behind--self-test, exits 1 on a zero-BENCH-row run, and refuses argless; one live spawn proves a BENCH row (jfk 2558.4 ms, rc 0) with the timed region byte-unchanged3. The catalog fixture regeneration rail
capture_catalog.dasboots the real setup-mode server on a scratch catalog dir, downloads the smallest card for real (3.1 GB, the server's own sha gate verifying), snapshots/catalogmid-flight and at the terminal state, normalizes machine-local paths, and deletes the scratch. The committedcatalog_downloading.json/catalog_done.jsonare its output - the 7-card roster, and a genuine 5% mid-flight snapshot (the first capture's 32 KB first-chunk pinned 0% and hollowed the progress-render assert - the round's WEAKENED verdict; the rail now waits for real progress). The fixtures' path normalization is pinned by a must-red cell;live-switch.specsynthesizes its second-present-row precondition on a clone (the establishedwithVisionPresentpattern). Playwright 106/106.The round
Woodpecker (one P1: the reused per-function table is now cleared, never deleted - the documented delete-then-reuse hazard), a worktree tdd audit (its three cheap named gaps closed in-PR), and five checklist auditors. Checklist repairs land across tests (skip-key property, family-tag scoping to model suites, box-decided predicate scope, the lane-knob roster gains whisper's knobs, the stage+mint rule names its APIs with the facade-knob residue ledgered as followup 61), benchmarks (engine-scoped
tune_gate, reported-walls subtraction), dasLLAMA (attribution states the property - your own callee or data key is not attribution), and the server (captured-body property form, the playwright-suite duty, transitive-require routing).Chain green first pass: all preflight lanes, model-free suite (whisper's model-gated cells included under the runner env),
test_model_catalog18/18, playwright 106/106.🤖 Generated with Claude Code
https://claude.ai/code/session_01LY6gNeDZkF14X81rLz3QLc