Use full-vocabulary top-p law in Qwen AR pipeline #104

Closed
opened 2026-10-03 09:20:30 +00:00 by hugo · 5 comments
Owner

Publication status: analysis commit dcc4c516f0e030566c67472f8f1be0baa21ab629 is durable locally; its push to origin/main awaits explicit approval after automatic review refused the default-branch push. Until published, linked repository evidence files are available locally under /Users/gb/Projects/DS4Server/docs/issue103-analysis/ and the committed test/tool paths, not yet at the remote file URLs.

Review status (updated 2026-10-03)

Confirmed defect or numerical parity difference; implementation still awaits verification. This description incorporates the independent review and the user's explicit model-specific oracle clarification. It supersedes the original issue scope and conflicting conclusions in the initial evidence report, including the incorrect default 128 GiB engagement claim. The second review is incorporated below. Original raw receipts/diagnostics remain unchanged; no production fix or new runtime/performance result is claimed. The report and AGENTS.md have not been rewritten in this issue-only update.

Model-specific parity boundary

The user explicitly clarified the oracle assignment after independent review:

  • Qwen 3.8 Flash Next: behavioral/numerical parity with the pinned MTPLX 2.12.2 revision 9882703f3105363ddc37eca9f97aa09a1d387112, including token processing, RNG/sampling, context accounting, inference and cache/session state.
  • DSv4 Flash and GLM 5.3 Flash: preserve DS4 behavior. A Qwen fix must not change their token IDs, sampler/RNG sequences, numerical inference, context accounting or cache behavior.
  • Keep changes in the Qwen family/phase/dispatch where applicable. Before changing any shared helper, trace every caller; preserve DS4-backed callers exactly and provide DS4 regression controls. A helper shared by Qwen and GLM must not silently impose MTPLX semantics on GLM. Do not globally change the DS4 sampler, shared RNG, Unicode tables, sigmoid/fast-math rules, or non-Qwen checkpoint formats for this ticket.

This is the user's model-specific clarification of the blanket DS4-oracle wording in AGENTS.md, not permission to weaken DS4 parity. Production remains native Rust/Metal with the existing Objective-C bridge; MTPLX Python is an isolated oracle only.

Broken behavior and evidence

The conditional Qwen AR pipeline has two divergences from current MTPLX: top-p normalized over only the top-k slice, and PCG64 for token 0 followed by keyed device RNG for later tokens instead of the classic request PCG64 stream. Caller: Generator → QwenExecutor → LiveSession → generate_turn → Execution::decode_ar → model.set_ar_pipeline_mode → ar::decode.

Actual engagement requires all of the following:

  • MTP disabled and pipeline enabled by the Qwen policy (MTPLX_AR_PIPELINE, default 1).
  • Eligible sampler: positive temperature, 1<top_k<4096, zero presence/frequency penalties, max_tokens>2.
  • Model::set_ar_pipeline_mode(true) returns true: every relevant PLE table must actually have a resident attachment. ngram::resident_policy automatically attempts residency at physical RAM>= 160 GiB, or when MTPLX_NGRAM_RESIDENT=1; explicit off or failed/incomplete resident attachment prevents engagement.

On the reference M5 Max 128 GiB with default SSD-PLE policy, this gate returns false and product AR already uses classic sampling. The earlier wording that default AR on this machine engages the faulty pipeline is withdrawn. A policy flag alone is not proof that the lane runs. High severity applies conditionally to the affected successfully resident configurations, including default eligible AR on >= 160 GiB machines with successful attachment and explicit resident opt-in.

The retained 64-logit primitive receipt is valid: ln([.4,.3,.3/62,...]), temperature1/k2/p.5 retains classic probabilities .5714285675/.4285714325, while local lazy sampling produces only token 0 in 128 trials; the .6/.95/k20 matrix also drops candidates. These are isolated sampling receipts, not a resident full-product reproduction. p=1 controls the shaping law, not RNG identity. No product-level resident seeded-stream comparison was performed in this analysis.

Current MTPLX contract: db6fe0fb49f60521552b36dfb34af0a508994d33 moved every token to classic _sample_from_logits and removed overlap; 5a9fe002a875f15531573b7f9960e1d85f77c2f8 then removed the lane/default/model pipeline mode. _mx_lazy_sample is off the decode path. The final native route must follow latest classic AR, not just correct the primitive's top-p.

Native implementation scope

Retire the Qwen AR pipeline completely, matching latest MTPLX classic decode and its default model forward state. Inventory and remove/update every engagement site:

  • policy.rs: server_overrides MTPLX_AR_PIPELINE default, turn_options flag routing and policy fixtures/assertions.
  • ar.rs: Options::pipeline/pipeline_eligible, pipeline_ready admission argument, lazy/keyed decode lane, Decoded::pipeline and obsolete admission/diagnostic tests.
  • execution.rs: decode_ar mode enable/restore calls, installed AR reference harness's independent MTPLX_AR_PIPELINE read (around 1188), output metrics and other test constructors asserting pipeline status.
  • model.rs: Model::set_ar_pipeline_mode and its AR-only PLE stage_disabled/prefer_lazy and model-policy lane toggling; restore the latest classic forward-state contract rather than leaving a dormant flag able to change staging.
  • binding.rs and decoder.rs: obsolete pipeline-mode assertions and tests; search the entire Qwen module for remaining AR_PIPELINE/mode engagement.

Remove AR-specific toggling, not every generic PLE or compilation field regardless of other legitimate callers. Trace remaining staging/compile consumers so classic AR, normal prefill and MTP retain their own reference behavior. Every AR token uses the existing Qwen classic sample_from_logits/PCG64 stream. Keep a lazy primitive only for an identified non-decode caller, with explicit classification. Coordinate invalid rows with #105. No global RNG/DS4 sampler changes and no DSv4/GLM staging behavior change.

Acceptance

Retain the primitive receipt as proof of the affected law; it cannot discharge actual lane engagement. Required product comparisons on unchanged artifacts/seed/settings:

  • On a >= 160 GiB resident configuration, or explicit MTPLX_NGRAM_RESIDENT=1 with successful full resident attachment and adequate resources, first prove the pre-fix lane engaged. Compare pre-fix pipeline/classic behavior and require the post-fix stream to equal classic AR and the pinned latest MTPLX contract. Record effective residency, MTP setting, sampler, emitted IDs and model/PLE forward state. This full-resident check is currently unexecuted.
  • On the 128 GiB/default SSD configuration, prove pre-fix AR already falls back to classic and post-fix classic tokens/final state remain unchanged. Do not claim the isolated lazy_sample test demonstrated a default 128 GiB product fault.
  • After removal, neither policy, explicit legacy flag, model mode nor reference harness may re-enable the keyed decode/staging route. Do not recreate an obsolete lane merely to run a toggle test. Preserve generic PLE/compiled-GDN behavior needed by actual remaining consumers.
  • Cover official .6/.95/k20, temperature1/p.95/k20, minimal .5/k2, p=0/1, ties, greedy and penalties; cold/warm turns, stop/limit capture, cancellation and retry. Same-implementation classic stream equality is required. Use the historical db6fe0fb token-for-token test as the template; it is deleted at latest HEAD.
  • DSv4 Flash/GLM 5.3 Flash sampler, RNG and inference-state regression controls stay unchanged. Record any measured throughput effect separately; no old overlap retained to weaken Qwen parity.

Full-product seeded/residency comparisons and throughput remain acceptance obligations, not completed analysis results.

Severity

High on affected resident configurations: the conditional pipeline changes sampling law and RNG continuation. Default eligible AR is affected on >= 160 GiB machines only when all PLE tables attach resident; explicit resident opt-in may also engage it. The 128 GiB reference machine with default SSD-PLE runs classic AR and is not demonstrated affected by this pipeline defect. Full-product resident impact/performance is unmeasured; the isolated primitive mismatch is confirmed.

Shared evidence and reproduction

Analysis parent: #103. Affected DS4Server revision: cea8fc6f7c7aa7aeb97ebc303594ae4088ef6983. Compared MTPLX old e652d55e2652137a4abcf1312357abbf3eb9d692 (2.10.2) with refreshed 9882703f3105363ddc37eca9f97aa09a1d387112 (2.12.2), on MLX exactly 0.32.2. Same Bare-Speed artifact revision 74559cdf34fbfc0b593de72d17e93f37fd4f9ea7; unchanged hashes, compiler/OS/hardware and effective options in environment.json. Hardware: M5 Max 128 GiB/macOS 26.6.2. Synthetic model-free cases use explicit shapes/dtypes in the diagnostic, no template/image/prompt or context setting; model case uses raw IDs, seed0 and restored identical prompt snapshots.

Durable report and instructions, native test, oracle probe. The test module is cfg(test), ignored by ordinary runs, and initially asserts/prints the affected behavior; passing this diagnostic means reproduction, not a fix. Convert only this issue's owned cases into correct-contract regressions during implementation. Shared vision test has separate case ownership listed in the report.

cargo test --all-features --bin ds4-server issue103_sampling_receipts --no-run
cargo build --bin test-supervisor
export DS4_QWEN38_ARTIFACTS='/path/to/unchanged/qwen3.8-flash-next'
export DS4_ISSUE103_ORACLE="$PWD/docs/issue103-analysis/latest-oracle.jsonl"
# Resolve the emitted test executable; do not pass cargo to the watchdog.
target/debug/test-supervisor 4096 60 120 --command /path/to/ds4_server-test-binary issue103_sampling_receipts --ignored --nocapture --test-threads=1

Immutable upstream change and latest test. This existing primitive test owns the historical law diagnostic, not current AR product routing. Current routing references: RNG/route correction, historical same-seed regression, lane/default removal, latest generation.py. Extra readbacks/synchronization and external memory samples make these correctness receipts unsuitable for speed claims.

Validation and review boundary

  • Implement production behavior in Rust with existing Metal/Objective-C bridge only; Python remains an isolated oracle.
  • Run cargo fmt --all -- --check, cargo clippy --all-targets --all-features -- -D warnings, make bundle, cargo test --all-features before every future commit, plus the owned native/oracle and product regressions above.
  • Implementation awaits the user's independent-agent review and subsequent instruction. Creating this issue or closing #103 does not authorize a fix. Review the corrected model-specific oracle, active-route applicability and state ownership first; preserve DS4-backed families.
  • No arbitrary dependencies on sibling issues: coordinate shared source changes where needed; each issue owns its concrete failure and acceptance.
Publication status: analysis commit `dcc4c516f0e030566c67472f8f1be0baa21ab629` is durable locally; its push to origin/main awaits explicit approval after automatic review refused the default-branch push. Until published, linked repository evidence files are available locally under `/Users/gb/Projects/DS4Server/docs/issue103-analysis/` and the committed test/tool paths, not yet at the remote file URLs. ## Review status (updated 2026-10-03) Confirmed defect or numerical parity difference; implementation still awaits verification. This description incorporates the [independent review](https://git.rfc1437.de/hugo/DS4Server/issues/104#issuecomment-3065) and the user's explicit model-specific oracle clarification. It supersedes the original issue scope and conflicting conclusions in the initial evidence report, including the incorrect default 128 GiB engagement claim. The [second review](https://git.rfc1437.de/hugo/DS4Server/issues/104#issuecomment-3098) is incorporated below. Original raw receipts/diagnostics remain unchanged; no production fix or new runtime/performance result is claimed. The report and AGENTS.md have not been rewritten in this issue-only update. ## Model-specific parity boundary The user explicitly clarified the oracle assignment after independent review: - Qwen 3.8 Flash Next: behavioral/numerical parity with the pinned MTPLX 2.12.2 revision `9882703f3105363ddc37eca9f97aa09a1d387112`, including token processing, RNG/sampling, context accounting, inference and cache/session state. - DSv4 Flash and GLM 5.3 Flash: preserve DS4 behavior. A Qwen fix must not change their token IDs, sampler/RNG sequences, numerical inference, context accounting or cache behavior. - Keep changes in the Qwen family/phase/dispatch where applicable. Before changing any shared helper, trace every caller; preserve DS4-backed callers exactly and provide DS4 regression controls. A helper shared by Qwen and GLM must not silently impose MTPLX semantics on GLM. Do not globally change the DS4 sampler, shared RNG, Unicode tables, sigmoid/fast-math rules, or non-Qwen checkpoint formats for this ticket. This is the user's model-specific clarification of the blanket DS4-oracle wording in AGENTS.md, not permission to weaken DS4 parity. Production remains native Rust/Metal with the existing Objective-C bridge; MTPLX Python is an isolated oracle only. ## Broken behavior and evidence The conditional Qwen AR pipeline has two divergences from current MTPLX: top-p normalized over only the top-k slice, and PCG64 for token 0 followed by keyed device RNG for later tokens instead of the classic request PCG64 stream. Caller: Generator → QwenExecutor → LiveSession → generate_turn → Execution::decode_ar → model.set_ar_pipeline_mode → ar::decode. Actual engagement requires all of the following: - MTP disabled and pipeline enabled by the Qwen policy (`MTPLX_AR_PIPELINE`, default 1). - Eligible sampler: positive temperature, 1<top_k<4096, zero presence/frequency penalties, max_tokens>2. - `Model::set_ar_pipeline_mode(true)` returns true: every relevant PLE table must actually have a resident attachment. `ngram::resident_policy` automatically attempts residency at physical RAM>= 160 GiB, or when `MTPLX_NGRAM_RESIDENT=1`; explicit off or failed/incomplete resident attachment prevents engagement. On the reference M5 Max 128 GiB with default SSD-PLE policy, this gate returns false and product AR already uses classic sampling. The earlier wording that default AR on this machine engages the faulty pipeline is withdrawn. A policy flag alone is not proof that the lane runs. High severity applies conditionally to the affected successfully resident configurations, including default eligible AR on >= 160 GiB machines with successful attachment and explicit resident opt-in. The retained 64-logit primitive receipt is valid: ln([.4,.3,.3/62,...]), temperature1/k2/p.5 retains classic probabilities .5714285675/.4285714325, while local lazy sampling produces only token 0 in 128 trials; the .6/.95/k20 matrix also drops candidates. These are isolated sampling receipts, not a resident full-product reproduction. p=1 controls the shaping law, not RNG identity. No product-level resident seeded-stream comparison was performed in this analysis. Current MTPLX contract: db6fe0fb49f60521552b36dfb34af0a508994d33 moved every token to classic _sample_from_logits and removed overlap; 5a9fe002a875f15531573b7f9960e1d85f77c2f8 then removed the lane/default/model pipeline mode. _mx_lazy_sample is off the decode path. The final native route must follow latest classic AR, not just correct the primitive's top-p. ## Native implementation scope Retire the Qwen AR pipeline completely, matching latest MTPLX classic decode and its default model forward state. Inventory and remove/update every engagement site: - `policy.rs`: server_overrides MTPLX_AR_PIPELINE default, turn_options flag routing and policy fixtures/assertions. - `ar.rs`: Options::pipeline/pipeline_eligible, pipeline_ready admission argument, lazy/keyed decode lane, Decoded::pipeline and obsolete admission/diagnostic tests. - `execution.rs`: decode_ar mode enable/restore calls, installed AR reference harness's independent MTPLX_AR_PIPELINE read (around 1188), output metrics and other test constructors asserting pipeline status. - `model.rs`: Model::set_ar_pipeline_mode and its AR-only PLE stage_disabled/prefer_lazy and model-policy lane toggling; restore the latest classic forward-state contract rather than leaving a dormant flag able to change staging. - `binding.rs` and `decoder.rs`: obsolete pipeline-mode assertions and tests; search the entire Qwen module for remaining AR_PIPELINE/mode engagement. Remove AR-specific toggling, not every generic PLE or compilation field regardless of other legitimate callers. Trace remaining staging/compile consumers so classic AR, normal prefill and MTP retain their own reference behavior. Every AR token uses the existing Qwen classic sample_from_logits/PCG64 stream. Keep a lazy primitive only for an identified non-decode caller, with explicit classification. Coordinate invalid rows with #105. No global RNG/DS4 sampler changes and no DSv4/GLM staging behavior change. ## Acceptance Retain the primitive receipt as proof of the affected law; it cannot discharge actual lane engagement. Required product comparisons on unchanged artifacts/seed/settings: - On a >= 160 GiB resident configuration, or explicit MTPLX_NGRAM_RESIDENT=1 with successful full resident attachment and adequate resources, first prove the pre-fix lane engaged. Compare pre-fix pipeline/classic behavior and require the post-fix stream to equal classic AR and the pinned latest MTPLX contract. Record effective residency, MTP setting, sampler, emitted IDs and model/PLE forward state. This full-resident check is currently unexecuted. - On the 128 GiB/default SSD configuration, prove pre-fix AR already falls back to classic and post-fix classic tokens/final state remain unchanged. Do not claim the isolated lazy_sample test demonstrated a default 128 GiB product fault. - After removal, neither policy, explicit legacy flag, model mode nor reference harness may re-enable the keyed decode/staging route. Do not recreate an obsolete lane merely to run a toggle test. Preserve generic PLE/compiled-GDN behavior needed by actual remaining consumers. - Cover official .6/.95/k20, temperature1/p.95/k20, minimal .5/k2, p=0/1, ties, greedy and penalties; cold/warm turns, stop/limit capture, cancellation and retry. Same-implementation classic stream equality is required. Use the historical db6fe0fb token-for-token test as the template; it is deleted at latest HEAD. - DSv4 Flash/GLM 5.3 Flash sampler, RNG and inference-state regression controls stay unchanged. Record any measured throughput effect separately; no old overlap retained to weaken Qwen parity. Full-product seeded/residency comparisons and throughput remain acceptance obligations, not completed analysis results. ## Severity High on affected resident configurations: the conditional pipeline changes sampling law and RNG continuation. Default eligible AR is affected on >= 160 GiB machines only when all PLE tables attach resident; explicit resident opt-in may also engage it. The 128 GiB reference machine with default SSD-PLE runs classic AR and is not demonstrated affected by this pipeline defect. Full-product resident impact/performance is unmeasured; the isolated primitive mismatch is confirmed. ## Shared evidence and reproduction Analysis parent: [#103](https://git.rfc1437.de/hugo/DS4Server/issues/103). Affected DS4Server revision: `cea8fc6f7c7aa7aeb97ebc303594ae4088ef6983`. Compared MTPLX old `e652d55e2652137a4abcf1312357abbf3eb9d692` (2.10.2) with refreshed `9882703f3105363ddc37eca9f97aa09a1d387112` (2.12.2), on MLX exactly 0.32.2. Same Bare-Speed artifact revision `74559cdf34fbfc0b593de72d17e93f37fd4f9ea7`; unchanged hashes, compiler/OS/hardware and effective options in [environment.json](https://git.rfc1437.de/hugo/DS4Server/src/commit/dcc4c516f0e030566c67472f8f1be0baa21ab629/docs/issue103-analysis/environment.json). Hardware: M5 Max 128 GiB/macOS 26.6.2. Synthetic model-free cases use explicit shapes/dtypes in the diagnostic, no template/image/prompt or context setting; model case uses raw IDs, seed0 and restored identical prompt snapshots. Durable [report and instructions](https://git.rfc1437.de/hugo/DS4Server/src/commit/dcc4c516f0e030566c67472f8f1be0baa21ab629/docs/issue103-analysis/README.md), [native test](https://git.rfc1437.de/hugo/DS4Server/src/commit/dcc4c516f0e030566c67472f8f1be0baa21ab629/src/engine/metal/qwen_mtplx/issue103_analysis.rs), [oracle probe](https://git.rfc1437.de/hugo/DS4Server/src/commit/dcc4c516f0e030566c67472f8f1be0baa21ab629/tools/issue103-reference.py). The test module is cfg(test), ignored by ordinary runs, and initially asserts/prints the affected behavior; passing this diagnostic means reproduction, not a fix. Convert only this issue's owned cases into correct-contract regressions during implementation. Shared vision test has separate case ownership listed in the report. ```sh cargo test --all-features --bin ds4-server issue103_sampling_receipts --no-run cargo build --bin test-supervisor export DS4_QWEN38_ARTIFACTS='/path/to/unchanged/qwen3.8-flash-next' export DS4_ISSUE103_ORACLE="$PWD/docs/issue103-analysis/latest-oracle.jsonl" # Resolve the emitted test executable; do not pass cargo to the watchdog. target/debug/test-supervisor 4096 60 120 --command /path/to/ds4_server-test-binary issue103_sampling_receipts --ignored --nocapture --test-threads=1 ``` Immutable upstream [change](https://github.com/youssofal/mtplx/commit/a2d0b4a493923f9c3b13178713e9f78413a1d2d7) and [latest test](https://github.com/youssofal/mtplx/blob/9882703f3105363ddc37eca9f97aa09a1d387112/tests/test_non_finite_logits_and_lazy_sampler_law.py). This existing primitive test owns the historical law diagnostic, not current AR product routing. Current routing references: [RNG/route correction](https://github.com/youssofal/mtplx/commit/db6fe0fb49f60521552b36dfb34af0a508994d33), [historical same-seed regression](https://github.com/youssofal/mtplx/blob/db6fe0fb49f60521552b36dfb34af0a508994d33/tests/test_async_decode.py), [lane/default removal](https://github.com/youssofal/mtplx/commit/5a9fe002a875f15531573b7f9960e1d85f77c2f8), [latest generation.py](https://github.com/youssofal/mtplx/blob/9882703f3105363ddc37eca9f97aa09a1d387112/mtplx/generation.py). Extra readbacks/synchronization and external memory samples make these correctness receipts unsuitable for speed claims. ## Validation and review boundary - Implement production behavior in Rust with existing Metal/Objective-C bridge only; Python remains an isolated oracle. - Run `cargo fmt --all -- --check`, `cargo clippy --all-targets --all-features -- -D warnings`, `make bundle`, `cargo test --all-features` before every future commit, plus the owned native/oracle and product regressions above. - Implementation awaits the user's independent-agent review and subsequent instruction. Creating this issue or closing #103 does not authorize a fix. Review the corrected model-specific oracle, active-route applicability and state ownership first; preserve DS4-backed families. - No arbitrary dependencies on sibling issues: coordinate shared source changes where needed; each issue owns its concrete failure and acceptance.
hugo added the bug label 2026-10-03 09:20:30 +00:00
Author
Owner

Independent review (2026-10-03)

Reviewed against DS4Server dcc4c51 and MTPLX 2.12.2 9882703f3105363ddc37eca9f97aa09a1d387112. Note: evidence commit dcc4c51 is still unpublished (main is ahead of origin/main by one commit), so the repository links in this issue currently resolve to nothing.

Defect claim: confirmed. sampling::lazy_sample (sampling.rs:196) computes the top-p cumulative mass on the softmax of the top-k slice only. policy::server_overrides defaults MTPLX_AR_PIPELINE=1 (policy.rs:56), so default AR (0.6/0.95/20) takes this lane. The .5/k2 example is arithmetically correct.

Proposed fix: diverges from latest MTPLX — needs revision. The analysis stopped at a2d0b4a and misses two later upstream commits:

  • db6fe0fb (2026-09-17) "Fix MTPLX_AR_PIPELINE sampler divergence from classic AR": the pipeline now draws every token through the classic _sample_from_logits(row, sampler, rng), so pipeline AR is byte-identical to classic AR. New test test_async_decode.py::test_ar_pipeline_matches_classic_token_for_token. _mx_lazy_sample is kept only as an off-path primitive ("no longer on the decode path").
  • 5a9fe002: removes the MTPLX_AR_PIPELINE lane and its Flash-Next default entirely.

Our ar.rs:95 has exactly the pre-db6fe0fb shape: token 0 from the PCG64 Rng, later tokens from a keyed random::split stream. Therefore:

  • "Keep the GPU/lazy route" and "do not replace RNGs" do not match the current reference contract.
  • "Do not use equality of seed streams as the oracle" is wrong within DS4Server: upstream now requires pipeline == classic AR token-for-token for the same seed. That is a same-implementation comparison and a valid acceptance test.
  • Recommended scope: follow upstream — route the AR pipeline through sample_from_logits (or drop the MTPLX_AR_PIPELINE default / lane). That fixes the sampling law and the RNG divergence together. Correcting only the top-p law inside lazy_sample would leave the second divergence in place.
  • Any measured AR throughput loss must be documented; upstream accepted it explicitly.

Severity High is reasonable (default AR configuration).

## Independent review (2026-10-03) Reviewed against DS4Server `dcc4c51` and MTPLX 2.12.2 `9882703f3105363ddc37eca9f97aa09a1d387112`. Note: evidence commit `dcc4c51` is still unpublished (`main` is ahead of `origin/main` by one commit), so the repository links in this issue currently resolve to nothing. **Defect claim: confirmed.** `sampling::lazy_sample` (sampling.rs:196) computes the top-p cumulative mass on the softmax of the top-k slice only. `policy::server_overrides` defaults `MTPLX_AR_PIPELINE=1` (policy.rs:56), so default AR (0.6/0.95/20) takes this lane. The .5/k2 example is arithmetically correct. **Proposed fix: diverges from latest MTPLX — needs revision.** The analysis stopped at a2d0b4a and misses two later upstream commits: - `db6fe0fb` (2026-09-17) "Fix MTPLX_AR_PIPELINE sampler divergence from classic AR": the pipeline now draws every token through the classic `_sample_from_logits(row, sampler, rng)`, so pipeline AR is byte-identical to classic AR. New test `test_async_decode.py::test_ar_pipeline_matches_classic_token_for_token`. `_mx_lazy_sample` is kept only as an off-path primitive ("no longer on the decode path"). - `5a9fe002`: removes the `MTPLX_AR_PIPELINE` lane and its Flash-Next default entirely. Our ar.rs:95 has exactly the pre-db6fe0fb shape: token 0 from the PCG64 `Rng`, later tokens from a keyed `random::split` stream. Therefore: - "Keep the GPU/lazy route" and "do not replace RNGs" do not match the current reference contract. - "Do not use equality of seed streams as the oracle" is wrong *within* DS4Server: upstream now requires pipeline == classic AR token-for-token for the same seed. That is a same-implementation comparison and a valid acceptance test. - Recommended scope: follow upstream — route the AR pipeline through `sample_from_logits` (or drop the `MTPLX_AR_PIPELINE` default / lane). That fixes the sampling law and the RNG divergence together. Correcting only the top-p law inside `lazy_sample` would leave the second divergence in place. - Any measured AR throughput loss must be documented; upstream accepted it explicitly. Severity High is reasonable (default AR configuration).
Author
Owner

Description corrected after independent review (2026-10-03)

Updated the issue body in response to your review. Changes:

  • Replaced the instruction to keep GPU/lazy AR with the current MTPLX classic-AR/removal contract, including db6fe0fb and 5a9fe002.
  • Added the second root cause: PCG64 token 0 followed by keyed device RNG.
  • Corrected seeded acceptance: same-implementation token-for-token comparison is required; latest no longer contains the obsolete pipeline test.
  • Restricted sampler/RNG changes to Qwen and added DSv4/GLM regression controls.

All eight issue descriptions now state the user's contract explicitly: Qwen 3.8 Flash Next follows pinned MTPLX; DSv4 Flash and GLM 5.3 Flash retain DS4 parity. Shared helpers must preserve the DS4-backed callers with exact regression controls.

For renewed verification: Verify the two later immutable commits, the final active AR caller/RNG route, the historical token-identity test, and that no DS4 sampler/RNG code changes are proposed.

This update changes tracker descriptions only, not production code, AGENTS.md or the committed raw receipts. The initial evidence report is historical where the corrected body says otherwise. Evidence commit dcc4c51 remains local/unpublished, with exact local paths and reproduction instructions in the body; no new test/performance result or implementation approval is claimed. Please record any remaining applicability/contract/acceptance gap before implementation.

## Description corrected after independent review (2026-10-03) Updated the issue body in response to [your review](https://git.rfc1437.de/hugo/DS4Server/issues/104#issuecomment-3065). Changes: - Replaced the instruction to keep GPU/lazy AR with the current MTPLX classic-AR/removal contract, including db6fe0fb and 5a9fe002. - Added the second root cause: PCG64 token 0 followed by keyed device RNG. - Corrected seeded acceptance: same-implementation token-for-token comparison is required; latest no longer contains the obsolete pipeline test. - Restricted sampler/RNG changes to Qwen and added DSv4/GLM regression controls. All eight issue descriptions now state the user's contract explicitly: Qwen 3.8 Flash Next follows pinned MTPLX; DSv4 Flash and GLM 5.3 Flash retain DS4 parity. Shared helpers must preserve the DS4-backed callers with exact regression controls. For renewed verification: Verify the two later immutable commits, the final active AR caller/RNG route, the historical token-identity test, and that no DS4 sampler/RNG code changes are proposed. This update changes tracker descriptions only, not production code, AGENTS.md or the committed raw receipts. The initial evidence report is historical where the corrected body says otherwise. Evidence commit dcc4c51 remains local/unpublished, with exact local paths and reproduction instructions in the body; no new test/performance result or implementation approval is claimed. Please record any remaining applicability/contract/acceptance gap before implementation.
Author
Owner

Re-verification of the revised description (2026-10-03) — remaining gaps

The revised body is correct about the two root causes (top-k-renormalized top-p; PCG64 token 0 followed by a keyed device stream) and about the target contract: retire the lane, as in db6fe0fb and 5a9fe002. The cited SHAs and tests check out; the historical token-for-token test exists in db6fe0fb and is gone at 9882703. MTPLX is the parity reference for Qwen 3.8 Flash Next, so the code must implement the current MTPLX state. Remaining gaps:

  1. Engagement condition is wrong in "Broken behavior" and "Severity". decode_ar only takes the lane when model.set_ar_pipeline_mode(true) returns true (execution.rs:830-833). Model::set_ar_pipeline_mode (model.rs:308) returns false unless every PLE n-gram table is resident. model.rs:281-289 only attaches the resident table when ngram::resident_policy() is true, i.e. physical RAM >= 160 GiB or MTPLX_NGRAM_RESIDENT=1 (ngram.rs:14-26). Otherwise the table streams from SSD and decode falls back to classic AR. So the lane is the default only on >=160 GiB Macs. On the 128 GiB M5 Max reference machine, default AR already runs classic AR. This mirrors upstream's own note (openai.py, qwen4_exp lanes) that the pipeline "needed a RAM-resident n-gram table, stopped engaging once the table streamed from SSD". The isolated lazy_sample receipt is valid, but it does not show product engagement on the reference machine. Please restate the trigger as ">=160 GiB RAM or MTPLX_NGRAM_RESIDENT=1, MTP disabled, sampler eligible" and adjust the severity wording ("default on >=160 GiB machines"). The fix direction (remove the lane) is unchanged and is now better justified.
  2. Complete removal scope (current MTPLX has no pipeline mode at all). 5a9fe002 removed the lane and the model pipeline mode ("no model offers the pipeline mode any more"). Removal must cover every engagement site, not only the policy default:
    • policy::server_overrides default MTPLX_AR_PIPELINE=1 (policy.rs:56) and turn_options (policy.rs:140);
    • the test/reference harness mtplx_full_installed_ar_matches_reference (execution.rs:1188), which reads MTPLX_AR_PIPELINE itself;
    • ar::Options::pipeline/pipeline_eligible, the lane body in ar::decode, Decoded::pipeline;
    • Model::set_ar_pipeline_mode and the PLE stage_disabled/prefer_lazy toggling it performs (model.rs:308-322; also asserted in binding.rs:678 and decoder.rs:535). Upstream's classic decode runs in the model's default forward state. Leaving this toggle in place would keep a non-reference PLE staging mode reachable.
  3. Acceptance addition. Add one product-level check on a >=160 GiB configuration, or with MTPLX_NGRAM_RESIDENT=1, showing the pre-fix lane engaged and the post-fix stream equal to classic AR. Add a 128 GiB/SSD control showing unchanged classic behavior.

DS4 remains the parity reference for DeepSeek and GLM. None of the above touches their sampler or RNG.

## Re-verification of the revised description (2026-10-03) — remaining gaps The revised body is correct about the two root causes (top-k-renormalized top-p; PCG64 token 0 followed by a keyed device stream) and about the target contract: retire the lane, as in `db6fe0fb` and `5a9fe002`. The cited SHAs and tests check out; the historical token-for-token test exists in `db6fe0fb` and is gone at `9882703`. MTPLX is the parity reference for Qwen 3.8 Flash Next, so the code must implement the *current* MTPLX state. Remaining gaps: 1. **Engagement condition is wrong in "Broken behavior" and "Severity".** `decode_ar` only takes the lane when `model.set_ar_pipeline_mode(true)` returns true (execution.rs:830-833). `Model::set_ar_pipeline_mode` (model.rs:308) returns false unless every PLE n-gram table is resident. `model.rs:281-289` only attaches the resident table when `ngram::resident_policy()` is true, i.e. **physical RAM >= 160 GiB or `MTPLX_NGRAM_RESIDENT=1`** (ngram.rs:14-26). Otherwise the table streams from SSD and decode falls back to classic AR. So the lane is the default only on >=160 GiB Macs. On the 128 GiB M5 Max reference machine, default AR already runs classic AR. This mirrors upstream's own note (openai.py, qwen4_exp lanes) that the pipeline "needed a RAM-resident n-gram table, stopped engaging once the table streamed from SSD". The isolated `lazy_sample` receipt is valid, but it does not show product engagement on the reference machine. Please restate the trigger as ">=160 GiB RAM or `MTPLX_NGRAM_RESIDENT=1`, MTP disabled, sampler eligible" and adjust the severity wording ("default on >=160 GiB machines"). The fix direction (remove the lane) is unchanged and is now better justified. 2. **Complete removal scope (current MTPLX has no pipeline mode at all).** `5a9fe002` removed the lane *and* the model pipeline mode ("no model offers the pipeline mode any more"). Removal must cover every engagement site, not only the policy default: - `policy::server_overrides` default `MTPLX_AR_PIPELINE=1` (policy.rs:56) and `turn_options` (policy.rs:140); - the test/reference harness `mtplx_full_installed_ar_matches_reference` (execution.rs:1188), which reads `MTPLX_AR_PIPELINE` itself; - `ar::Options::pipeline`/`pipeline_eligible`, the lane body in `ar::decode`, `Decoded::pipeline`; - `Model::set_ar_pipeline_mode` and the PLE `stage_disabled`/`prefer_lazy` toggling it performs (model.rs:308-322; also asserted in binding.rs:678 and decoder.rs:535). Upstream's classic decode runs in the model's default forward state. Leaving this toggle in place would keep a non-reference PLE staging mode reachable. 3. **Acceptance addition.** Add one product-level check on a >=160 GiB configuration, or with `MTPLX_NGRAM_RESIDENT=1`, showing the pre-fix lane engaged and the post-fix stream equal to classic AR. Add a 128 GiB/SSD control showing unchanged classic behavior. DS4 remains the parity reference for DeepSeek and GLM. None of the above touches their sampler or RNG.
Author
Owner

Description updated after second review (2026-10-03)

Addressed the remaining gaps:

  • Corrected trigger/severity: successfully resident PLE tables are required; automatic policy starts at 160 GiB or explicit MTPLX_NGRAM_RESIDENT=1. Eligibility/MTP/flag checks still apply. 128 GiB default SSD-PLE already uses classic AR, so the old default-machine impact claim is withdrawn.
  • Distinguished isolated lazy_sample receipts from unexecuted resident product tests.
  • Expanded complete retirement to policy, ar Options/Decoded/lane, execution enable/restore and harness flag read, Model::set_ar_pipeline_mode including AR-only PLE stage_disabled/prefer_lazy/policy toggles, binding/decoder assertions and remaining references. Legitimate generic PLE/compiled-GDN consumers are protected.
  • Added pre-fix resident engagement/post-fix classic token/state comparison and a 128 GiB SSD unchanged-classic control. Neither a legacy flag nor the harness may keep the removed mode reachable.

Please re-verify the exact resident gate, all removal sites, unaffected classic SSD behavior and no DSv4/GLM sampler/RNG/staging changes. This is a description correction only; no resident product test or production fix is claimed. Existing raw receipts and their publication status are unchanged.

## Description updated after second review (2026-10-03) Addressed the [remaining gaps](https://git.rfc1437.de/hugo/DS4Server/issues/104#issuecomment-3098): - Corrected trigger/severity: successfully resident PLE tables are required; automatic policy starts at 160 GiB or explicit MTPLX_NGRAM_RESIDENT=1. Eligibility/MTP/flag checks still apply. 128 GiB default SSD-PLE already uses classic AR, so the old default-machine impact claim is withdrawn. - Distinguished isolated lazy_sample receipts from unexecuted resident product tests. - Expanded complete retirement to policy, ar Options/Decoded/lane, execution enable/restore and harness flag read, Model::set_ar_pipeline_mode including AR-only PLE stage_disabled/prefer_lazy/policy toggles, binding/decoder assertions and remaining references. Legitimate generic PLE/compiled-GDN consumers are protected. - Added pre-fix resident engagement/post-fix classic token/state comparison and a 128 GiB SSD unchanged-classic control. Neither a legacy flag nor the harness may keep the removed mode reachable. Please re-verify the exact resident gate, all removal sites, unaffected classic SSD behavior and no DSv4/GLM sampler/RNG/staging changes. This is a description correction only; no resident product test or production fix is claimed. Existing raw receipts and their publication status are unchanged.
Author
Owner

Implemented and pushed in 1de78fa.

Removed the Qwen AR pipeline end to end: policy default/flag, admission, keyed lazy decode, model PLE/compile toggles, speculative tail reuse, and obsolete harness assertions. Every AR token now uses classic sample_from_logits with the request PCG64 stream; final capture forwards the last token. The historical lazy primitive is test-only. Generic PLE/GDN behavior and all DS4/GLM code are preserved.

Pre-fix full-model reproduction proved actual pipeline engagement with a fully attached resident PLE table on the 128 GiB M5 Max. The old lane diverged at token 3; default SSD correctly stayed classic. Post-fix SSD and resident runs, with the legacy flag explicitly on, exactly match both pre-fix classic baselines and pinned MTPLX 9882703: tokens, final logits, and every cache-state array. Cold/warm turns, sync/async evaluation, cancellation after three callbacks, and retry from committed state passed.

Validation: 1,120 fresh AR oracle cases; 1,080 sampler-entry cases plus penalty, tied-logit/RNG, historical-law and connected-decoder controls; 268 scoped current server-policy cases. All four repository gates passed: fmt, warnings-denied Clippy, signed bundle, and all-features tests (308 application tests plus 8 tool tests). All model artifact SHA256 values were freshly verified. Review found no remaining production pipeline admission/mode path and no DS4/GLM changes.

Complete evidence, scope review, measurements and reproduction instructions. Decode timings are recorded separately as bounded correctness-probe measurements, not steady-state throughput claims.

Implemented and pushed in [1de78fa](https://git.rfc1437.de/hugo/DS4Server/commit/1de78fa3ef16d4fbd1bc4fe519b239d081391a0e). Removed the Qwen AR pipeline end to end: policy default/flag, admission, keyed lazy decode, model PLE/compile toggles, speculative tail reuse, and obsolete harness assertions. Every AR token now uses classic sample_from_logits with the request PCG64 stream; final capture forwards the last token. The historical lazy primitive is test-only. Generic PLE/GDN behavior and all DS4/GLM code are preserved. Pre-fix full-model reproduction proved actual pipeline engagement with a fully attached resident PLE table on the 128 GiB M5 Max. The old lane diverged at token 3; default SSD correctly stayed classic. Post-fix SSD and resident runs, with the legacy flag explicitly on, exactly match both pre-fix classic baselines and pinned MTPLX 9882703: tokens, final logits, and every cache-state array. Cold/warm turns, sync/async evaluation, cancellation after three callbacks, and retry from committed state passed. Validation: 1,120 fresh AR oracle cases; 1,080 sampler-entry cases plus penalty, tied-logit/RNG, historical-law and connected-decoder controls; 268 scoped current server-policy cases. All four repository gates passed: fmt, warnings-denied Clippy, signed bundle, and all-features tests (308 application tests plus 8 tool tests). All model artifact SHA256 values were freshly verified. Review found no remaining production pipeline admission/mode path and no DS4/GLM changes. [Complete evidence, scope review, measurements and reproduction instructions](https://git.rfc1437.de/hugo/DS4Server/src/commit/1de78fa3ef16d4fbd1bc4fe519b239d081391a0e/docs/issue104-ar/README.md). Decode timings are recorded separately as bounded correctness-probe measurements, not steady-state throughput claims.
hugo closed this issue 2026-10-04 10:50:18 +00:00
Sign in to join this conversation.