Skip to content

fix: replace the unmaintained YAML parser - #2279

Merged
inureyes merged 3 commits into
mainfrom
fix/issue-2278-maintained-yaml-parser
Oct 12, 2026
Merged

inureyes merged 3 commits into
mainfrom
fix/issue-2278-maintained-yaml-parser

Conversation

@inureyes

@inureyes inureyes commented Oct 12, 2026 •

Copy link
Copy Markdown
Member

Replace the CLI and surgery YAML parser with deserialize-only serde-saphyr 1.3.0, removing serde_yaml and unsafe-libyaml from the lockfile and resolving RUSTSEC-2024-0449 without an advisory exception.

Preserve ordered language priorities, DuplicateLanguageCode, duplicate schema-field rejection, YAML 1.2 booleans, and ordinary merge-key handling. Recover canonical non-finite values from Serde's tagged/untagged buffering so existing operation-specific finite-value validation remains effective. Quoted canonical .nan/.inf/-.inf now follow the same handling as their unquoted forms; malformed-input diagnostic formatting comes from the new parser.

Validation: all 34 focused language-bias tests and all 28 surgery configuration tests pass, including all six shipped surgery examples. cargo deny check passes all four checks; formatting and diff checks pass. Repository contract checks also pass (123 Python tooling tests and 49 WebUI fixtures). The alpha/ratio and quoted-special regressions pass. CI Clippy, CPU-only link, and cargo-deny pass on dd36abb; the duplicate local CUDA Clippy rebuild was stopped after CI succeeded.

Closes #2278

Replace serde_yaml and unsafe-libyaml with deserialize-only serde-saphyr to clear RUSTSEC-2024-0449 without suppressing advisories. Preserve language ordering and duplicate-language diagnostics, strict schema fields, YAML 1.2 booleans, and surgery operation validation for non-finite values.

Add regressions for duplicate fields, numeric spellings, and all six shipped surgery examples. Refs #2278.
@inureyes inureyes added status:review Under review type:bug Bug fixes, error corrections, or issue resolutions priority:high High priority labels Oct 12, 2026
Cover non-finite add coefficients and interpolation ratios, including the deliberately accepted quoted canonical special values. Record the parser migration and completed validation in bilingual reports; the added test execution and Clippy gate remain pending at report preparation.

Refs #2278.
@inureyes inureyes added status:done Completed and removed status:review Under review labels Oct 12, 2026
@inureyes
inureyes merged commit fc45284 into main Oct 12, 2026
77 of 79 checks passed
@inureyes
inureyes deleted the fix/issue-2278-maintained-yaml-parser branch October 12, 2026 15:10
inureyes added a commit that referenced this pull request Oct 12, 2026
Merge the main branch prerequisite that replaces the unmaintained YAML parser so the portable A/B output directory change can pass the shared dependency gate.

Refs #2141, #2278, #2279.
inureyes added a commit that referenced this pull request Oct 12, 2026
Merge main at fc45284 so PR #2277 uses the maintained safe-Rust YAML parser and no longer inherits the RUSTSEC-2024-0449 cargo-deny failure. The split-mtp source remains unchanged.

Validation: configured version, kernel, Llama, ROCm, Python, and WebUI contracts pass; cargo deny check, formatting, and diff checks pass.

Refs #1784, #2279
inureyes added a commit that referenced this pull request Oct 23, 2026
The Activity performance gate judged five alternating pairs by a bare median against the 2% budget and also failed any mode whose baseline CV exceeded 5%. On `lablup-dgxspark21` the per-pair noise (SD 3.6 points, kurtosis 12 to 25) makes both rules fire on unchanged code. Each mode now samples until an interval decides, and a mode that cannot be decided fails as noise, not as a slowdown.

## The rule

- Per pair: `(off - on) / off * 100`, alternating order as before. Point estimate: the median. Interval: the distribution-free sign-test interval `[x(k), x(n+1-k)]` at 1% per tail (98% two-sided). It needs no variance estimate and no random bootstrap, so JavaScript and Python agree bit for bit.
- `investigate` (slowdown): the interval lies wholly above the 2% budget.
- `within-target`: it does not, and its upper bound is at most 4%.
- Otherwise add 2 pairs, from 12 up to 40; still undecided at 40 is `inconclusive`, which fails with a message naming measurement noise.
- Baseline CV and the paired range stay in the summary as diagnostics and decide nothing. The 2% budget is unchanged.
- The 4% bound is the precision a pass must reach, not a looser budget: a 4% bound gives 0.15% null inconclusive per mode, while 3% gives 2.4%, which breaks the 1% false-failure target (table below).
- All constants live in `webui/scripts/activity-performance-policy.json`. The verifier re-derives every mode from the raw samples with a Python port (`scripts/webui/activity_performance_verdict.py`) and derives the sample and preflight counts from `paired_runs`. The summarizer has no samples, so it checks each summary's internal consistency under the same policy. Both languages are tested against `webui/scripts/activity-performance-fixtures.json`.

## Resampling estimate

Noise model: all 636 complete diagnostics artifacts on this runner (2026-09-15 to 2026-10-22, 6,360 pairs), centred on a true 0% change, iid pairs drawn from that pool plus a per-run shift N(0, 0.83^2). 0.83 is the between-run SD estimated from the post-#2119 runs; one-visible and two-visible means in the same run are uncorrelated (r = -0.07). 20,000 simulated runs per row, sequential looks included:

| True degradation | investigate | inconclusive | within-target | mean pairs |
|---|---|---|---|---|
| 0% | 0.06% | 0.15% | 99.79% | 13.5 |
| 2% | 11.4% | 7.0% | 81.6% | 20.4 |
| 4% | 78.6% | 8.5% | 12.9% | 21.6 |
| 5% | 96.0% | 2.3% | 1.6% | 17.1 |
| 6% | 99.67% | 0.27% | 0.07% | 14.1 |
| 8% | 100% | 0% | 0% | 12.4 |

- False failure on a true 0% change: 0.21% per mode, about 0.4% per CI job (two modes), about 0.6% in `full` mode. With iid pairs and no run shift it is below 0.01%.
- Detection: at least 95% for true effects of 5% and above, 99.7% at 6%. At 4% it is only 79%, so the injected slowdown for acceptance is sized at about 6%.
- Rejected alternatives: a 3% upper bound (2.4% null inconclusive); passing only when the interval lies wholly below 2% (far more pairs); mean with a t-interval and Hodges-Lehmann (similar power, but the mean is hurt by the heavy tails and HL assumes a symmetry the data lacks, skew -0.7).
- Assumption: the guarantee holds while run-level shifts stay near the measured 0.8 points. A shift SD of 1.5 gives 2.4% false `investigate`; keeping such shifts out is the job of the #1949 host-quiet precheck.
- Warmup: there is no first-pair bias (pair 0 mean -0.50 vs -0.48 overall, and no order effect), so no leading pairs are discarded. The existing warmup request runs 1.6% slower than the first baseline, and that request is already excluded.

## Wall time

Measured per pair on 153 recent runs: one-visible 7.2 s median (7.7 s max), two-visible 8.4 s (9.2 s max), plus about 34 s before the first pair. In CI (visible-only-headed), the worst case at 40 pairs per mode is about 12 minutes, and the expected null run is about 4 minutes (about 2 today). `full` mode adds a hidden block, so its worst case is about 20 minutes, inside the 1800 s activity timeout. `ci.yml` timeouts are unchanged.

## Fixture provenance

The fixture copies the per-sample logs of the `webui-activity-performance-diagnostics-Linux` artifacts, none of which had expired: 35423171315 a1, 35425297407 a1/a2/a3, 35425050365 a1, 35421166203 a1, 35567777700 a1 (#1944), 35588958676 a1 (#1946) and 38265665905 a1/a2 (#2279). At their recorded five pairs, every one of them returns `continue` (no verdict), never `investigate`. Their concatenated pairs decide `within-target` at 12 pairs in both modes. The same pairs with the Activity arm 6% slower decide `investigate`, and spread-amplified or drifting (CV 12%) variants exercise `inconclusive` and CV-only cases.

## Test plan

- [x] `pnpm --dir webui run unit` and `pnpm --dir webui run lint`
- [x] `make verify-webui-helper-tests`, including the new `activity_performance_verdict_tests.py`
- [x] `python3 scripts/webui/verify_activity_performance_tests.py` and `summarize_activity_evidence_tests.py`
- [x] `make verify-versions verify-env-var-docs verify-python-tooling`
- [x] The new gate inside the real `WebUI installed artifact` job on this PR, run [39853154267](https://github.com/lablup/mlxcel/actions/runs/39853154267): one-visible within-target (median -0.15%, 98% interval [-1.83%, 1.25%]) and two-visible within-target (median -0.76%, [-2.64%, 0.47%]), both at 12 pairs, with the verifier and summarizer accepting the evidence; the verifier took 3 min 42 s.

## Acceptance runs on `lablup-dgxspark21`

- Five consecutive green `WebUI installed artifact` runs of this tree (head a7cc03f): run [39853154267](https://github.com/lablup/mlxcel/actions/runs/39853154267) attempts 1 to 5 (jobs 120084449655, 120097378392, 120110916929, 120116442394, 120121248222).
- Injected slowdown, red with `investigate` in both modes: run [39870659579](https://github.com/lablup/mlxcel/actions/runs/39870659579) on throwaway draft PR #2554 (closed, never merged). one-visible median 4.58%, 98% interval [3.89%, 5.68%]; two-visible median 8.05%, interval [7.11%, 9.62%]; 12 pairs each. The injection sleeps 400 us per decode tick for each open `/ui-api/v1/events` stream, which the Activity page holds open exactly while visible.
- The true effect was not measured beforehand on a separate harness (a local build on the shared CI host was not permitted); it is characterised only by the gate's own median and interval above, both inside the 4 to 8% target for one-visible.
- The issue's suggested injection does not work on GB10. A per-poll CPU burst of 80 spinning threads (run 39862888632) raised the runnable tasks to 46 to 87 in the visible arms, but decode stayed at 152 tok/s in both arms. A delay keyed on Activity polls (runs 39866915190 and 39868894929) also measured 0%: the instrumented run showed that no operations poll ever landed inside a decode window, because each poll was answered the moment a request finished.

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

Labels

priority:high High priority status:done Completed type:bug Bug fixes, error corrections, or issue resolutions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(deps): replace the unmaintained YAML parser blocking CI

1 participant