bench: add quality gate for Qwen 3.6 27B AR on R9700 and Strix - #636
bench: add quality gate for Qwen 3.6 27B AR on R9700 and Strix#636Graffioh wants to merge 4 commits into
Conversation
264b8b9 to
243397a
Compare
243397a to
79ac7ab
Compare
There was a problem hiding this comment.
12 issues found across 43 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="bench/correctness/losslessness.py">
<violation number="1" location="bench/correctness/losslessness.py:71">
P2: When a token artifact contains JSON booleans, `_load_tokens` accepts them as integers, so `true` compares equal to token ID `1` and can produce a false `PASS`. Reject booleans explicitly when validating token elements.</violation>
</file>
<file name="server/scripts/profile.py">
<violation number="1" location="server/scripts/profile.py:857">
P2: When `--losslessness-policy fail` is supplied without `--check-lossless`, `main()` skips the policy and exits 0, so a misconfigured losslessness gate silently passes. Reject non-`report` policies without `--check-lossless`, or make the policy enable the check.</violation>
</file>
<file name="bench/performance/from_profile.py">
<violation number="1" location="bench/performance/from_profile.py:15">
P3: from_profile.py copies sha256() and git_commit() verbatim from from_generation.py. Duplicating these makes a future change (e.g. hash algorithm or git timeout) need to be applied in two places. Import them from bench.performance.from_generation instead.</violation>
</file>
<file name="bench/release/qwen38_strix.py">
<violation number="1" location="bench/release/qwen38_strix.py:157">
P2: When `qualify.py` receives `--server-url` with a port different from the profile, this stage launches on `profile.server_port` but waits on `LUCE_SERVER_URL`, so R0 times out or validates another listener. Use one URL/port source for the built-in runner or reject the override.</violation>
<violation number="2" location="bench/release/qwen38_strix.py:212">
P2: Changing a workload's `temperature` in `models.yaml` has no effect: `_generation` always sends `--temperature 0`, and R5 later rejects the report against `Workload.temperature`. Pass the workload's configured temperature into `_generation`, keeping an explicit zero only for stages without a workload setting.</violation>
</file>
<file name="bench/quality/run_lm_eval.py">
<violation number="1" location="bench/quality/run_lm_eval.py:94">
P3: When lm-eval is unavailable, this import fails before `build_model()` can emit its installation guidance, so the CLI produces a raw traceback. Wrap both lm-eval imports in the same actionable `RuntimeError` handling or centralize dependency loading.</violation>
</file>
<file name=".audit/pr636-prod-value.tsv">
<violation number="1" location=".audit/pr636-prod-value.tsv:17">
P2: This record cannot stand as an audit of when events happened: the timestamps are not a genuine chronological log. The rows at 15:12:00.470476Z/01/02 share identical microseconds and sit exactly 1.000000s apart, and the 17:13:01-06 rows are each exactly 1s apart — pattern-generated values, not per-event clock readings. The recorded order is also self-contradictory: this `validation` row at 15:12:02 claims a "final scoped" run over "the publish set changed after the full model-backed qualification", yet the `scope` row (15:16:03, which removes profiles from the publish set) and the `design`/`implementation` rows (17:13:01-02, which define the final recipe) are timestamped after it. A final validation cannot precede the changes that produce the publish set it validates. Since this file's stated purpose is production-gate evidence, record genuine per-event timestamps (or an explicit logical sequence) rather than a synthesized one-second-spaced timeline.</violation>
</file>
<file name="server/scripts/quality_ab_simple.py">
<violation number="1" location="server/scripts/quality_ab_simple.py:73">
P2: For these short prompts, fixed `--max-ctx 4096` makes every request process substantial unused KV capacity and slows the quality gate. Derive the context limit from the rendered prompt or otherwise auto-fit it instead of hard-coding 4096.</violation>
<violation number="2" location="server/scripts/quality_ab_simple.py:152">
P2: For the Qwen3.6 target, the control runs sample at the model-card temperature instead of decoding greedily, so they can differ randomly and fail the new verdict. Send `temperature: 0.0` in this deterministic A/B request.</violation>
<violation number="3" location="server/scripts/quality_ab_simple.py:215">
P2: When `DFLASH_SERVER_BIN` points to a non-executable regular file, preflight accepts it and the run crashes in `subprocess.Popen`. Require execute permission when validating the server binary.
(Based on your team's feedback about executable binary overrides.)</violation>
</file>
<file name="server/tests/test_tokenizer.py">
<violation number="1" location="server/tests/test_tokenizer.py:315">
P2: The default model points to server/models/qwen3-0.6b-ci.gguf, which nothing in the repository builds or downloads, so the documented reusable command `server/tests/test_tokenizer.py --require-hf` fails with "Model file not found" on a clean checkout. Add the CI step that provisions this tokenizer-only GGUF (or fall back to a bundled Qwen3-0.6B GGUF path) and document how the artifact is produced, so the gate is reproducible outside the staging build.</violation>
</file>
<file name="bench/profiles.py">
<violation number="1" location="bench/profiles.py:123">
P2: `_workloads()` validates `concurrency`/`maximum_tokens`/`repetitions` with the strict `_positive_int()` helper but coerces `warmups` via `int()` and `temperature` via `float()`. YAML values like `warmups: 2.9` (truncated to 2) or `temperature: true` (accepted as 1.0) pass silently; the boolean case would randomize generation and defeat the `require_identical: true` guarantee on the paged c4 smoke, while a non-numeric value raises an unhandled `TypeError` instead of `ProfileError`. Validate both with strict type checks (e.g., a non-negative-int helper for `warmups` and a real-number check for `temperature`) consistent with the sibling fields.</violation>
</file>
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
Re-trigger cubic
79ac7ab to
869f1c9
Compare
There was a problem hiding this comment.
1 existing issue remains and no new issues found across 32 files
Requires human review: Auto-approval blocked because this review re-detected 6 unresolved issues already reported by Cubic.
Re-trigger cubic
a9970d3 to
419e72e
Compare
419e72e to
96f5c87
Compare
|
@cubic-dev-ai review this PR |
@Graffioh I have started the AI code review. It will take a few minutes to complete. |
96f5c87 to
9f67122
Compare
There was a problem hiding this comment.
Review completed against the latest diff
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
9f67122 to
9794f0e
Compare
9794f0e to
925e204
Compare
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 18 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
bd24735 to
ad1e25f
Compare
ad1e25f to
e67ea59
Compare
Purpose
This PR gives Qwen 3.6 27B Q4_K_M target-only AR a defined quality process on the two AMD devices we use today: R9700 and Strix Halo.
It covers paged AR at concurrency 1 and 4. It does not add speculation, Qwen 3.8, DS4, logits observation, or a Cartesian feature matrix. Those paths can reuse this structure with their own profiles and baselines.
Two levels of checks
workflow_dispatchlucebox3The production workflow does not approve a release or promote a baseline. A person reviews the report and retained evidence, then records the decision separately.
Production qualification
The manual run uses one exact profile from model artifact to performance baseline:
Missing weights, hardware drift, malformed evidence, server failure, nondeterministic output, and baseline identity mismatches fail closed.
Feature-aware baseline identity
Feature selection is part of the profile and baseline identity, not a free-form runtime toggle. Each profile declares a normalized
feature_setand orderedserver_arguments, and records the complete performance-relevant server configuration in reports and evidence.--speclais rejected for an AR profile; a SpecLA profile must explicitly declare it.This prevents a
--speclarun from being compared with or promoted over the no-SpecLA AR baseline, and gives future serving features the same separation.Structure
bench/models.yamlowns the shared Qwen 3.6 recipe and the R9700 and Strix hardware bindings.bench/profiles.pyparses and validates the profile before a GPU process starts.bench/release/qwen36_amd.pyadapts the profile to the Qwen server and implements the R0-R6 stages.bench/release/qualify.pyowns process lifecycle, timeouts, cleanup, evidence validation, and the R7 report.bench/performance/owns request timing, report construction, baseline comparison, immutable baseline history, and the history viewer.bench/baselines/contains the reviewed hardware- and feature-specific baselines and their schema..github/workflows/ci.ymlruns bounded PR checks..github/workflows/production-quality.ymlruns the manual two-GPU qualification.bench/release/README.mdexplains the operator workflow.bench/baselines/README.mdexplains reviewed baseline promotion and performance history.Scope
The checked-in baselines qualify the Qwen 3.6 artifact available on
main. Qwen 3.8 speculative work in #625 needs separate profile identities and calibration after it lands.This PR keeps the serving API generation-only. It does not add scoring endpoints, forced-token requests, observer hooks, or logprob internals.
Verification
gfx1201) and Strix Halo (gfx1151).419e72eb.origin/main.