MEME hit-likelihood estimate: verified JS evaluator, no ONNX runtime in the bundle - #150
Merged
Conversation
- static/prescreen/meme_power_gate.onnx (82 KB) — the stage-1 gate model, generated from axomeme/dm3-prescreen (clientside/export_gate_onnx.py). - static/prescreen/ort/ort-wasm-simd-threaded.wasm (13 MB) — the single default ORT-web WASM runtime, vendored so it loads locally (CSP-safe, offline), served on demand only when a MEME gate runs. - src/lib/services/prescreen/gate.js — the runtime-agnostic gate module. - src/lib/services/prescreen/*.fixtures.json — parity fixtures for the tests. - onnxruntime-web dep + onnxruntime-node dev dep (tests).
Adds the client-side AxoMEME stage-1 gate as a GREEN/YELLOW/RED advisory badge that renders above the runtime estimate when MEME is selected (issue #147). - PreScreenGate.svelte: the badge. Client-side only (browser/onMount guard — onnxruntime-web can't run under Cloudflare SSR); MEME-only; non-blocking. Uses the CPU-only ORT build (onnxruntime-web/wasm) so we ship one 13 MB WASM, not the 26 MB WebGPU/jsep variant. Fails silent (advisory is non-essential). - gateAdvisory.js: testable wrapper — loadGateSession (lazy ORT + vendored wasmPaths), computeAdvisory (MEME-only + branch-length-tree gate, returns null otherwise). Injectable ORT for tests. - MethodSelector.svelte: render <PreScreenGate> above <AnalysisTimingEstimate>, fed the in-memory alignment (.canonicalFasta) + tree (.usertree || .nj). - vite.config.ts: exclude onnxruntime-web from optimizeDeps (it ships its own vendored WASM). - prescreen-gate.test.js: 16 unit tests scoring through the REAL onnx via onnxruntime-node (shares kernels with -web) — feature parity, tier boundaries, MEME-only + no-tree degradation, shallow-tree-gates-RED end to end. - 18-prescreen-gate.spec.js: e2e — badge shows for MEME (verified RED on the CD2-slim demo), absent for FEL. Component rendering is covered here because the repo's vitest toolchain doesn't mount Svelte components. Verified: 268 unit tests + 2 e2e pass; badge renders RED above the timing estimate in-browser.
Reworks the AxoMEME stage-1 gate into a "Before you run" outlook row and takes the ONNX runtime out of the browser bundle entirely. Behaviour - Merges the estimate into one RunOutlook panel with the runtime estimate, instead of two competing coloured boxes stacked above Run. - A discouraging estimate now routes to a better-suited method, or says plainly that no method will help, rather than only grading the data. - Replaces the bare probability with the three inputs it was computed from; the score and its limits sit behind a disclosure. - Distinguishes not-applicable / loading / ok / cannot-assess / error, so a broken estimator no longer renders as a healthy one. - Adds an out-of-distribution guard, and discloses when branch lengths come from an inferred neighbour-joining tree rather than the user's own. Payload - Drops onnxruntime-web and onnxruntime-node. The 150-tree ensemble is walked by treeEnsemble.js over a 37 KB coefficient table, so selecting a non-MEME method downloads 0 bytes of estimator payload, measured in both dev and a production build; MEME downloads ~59 KB. Previously every method pulled 13.5 MB. - Removing the native binding also unbreaks the unit suite on x64 Node, which could not load onnxruntime-node at all. Verification - scripts/prescreen/onnx_to_ensemble.py regenerates the coefficient table from the .onnx and reproduces the committed table exactly. - scripts/prescreen/verify_parity.py scores 20k vectors through both onnxruntime and the shipped JS evaluator, and fails on any level disagreement or hash mismatch. Wired up as .github/workflows/model-parity.yml. - The .onnx hash pin is now compared against the file on disk rather than pattern-matched, so a regenerate against the wrong model is caught. Known limitation: the model is not monotone in its inputs, so adding sequences can lower the estimate. Tracked in #151.
stevenweaver
changed the title
Resolves the package.json conflict by keeping both sides: main's fast/slow e2e split and this branch's model-parity scripts. The conflict was why CI never ran on #150. GitHub builds pull_request workflow runs from the refs/pull/N/merge commit, and a conflicting PR has no such ref, so no run is triggered at all — silently, with nothing shown in the Actions tab. Also picks up main's e2e coverage tooling: 18-meme-hit-likelihood.spec.js now imports the coverage fixture like every other spec, so the estimator is included in the coverage report rather than being the one gap in it, and both describe blocks get a 180s timeout. Under 4 workers sharing one dev server the setup alone can exceed the global 60s cap, which made the suite flaky-and-retried instead of either passing or failing honestly.
The first assertion that mounts MemeHitLikelihood is also what makes the dev server transform it and its four-module import graph for the first time. That cold cost lands on one assertion; 30s cleared it locally but failed on a cold CI runner and only passed on retry, which reads as an unstable feature rather than a one-off compile.
The previous commit blamed a cold module-graph transform and raised a timeout. That was wrong: the failure snapshot shows the app back on the Data tab with "Upload or select a file to get started" and no method dropdown at all, so no wait length could have helped. Two things combine. goToAnalyzeTab returns false instead of throwing when the tab is not clickable yet, so a missed click is silent. And the marker loadDemoFile waits on, sequence-info, is the unconditional root element of dataReaderResults, so it appears when that component mounts rather than when the file becomes the active selection. Between those, clicking Analyze can land back on Data, and the spec only discovers it much later as a missing estimator row. Retry the navigation with expect().toPass() and assert the dropdown as the real precondition. Three consecutive local runs are clean; it previously flaked about every other run.
This is the actual cause of the estimator spec's flake, and it was not a timeout or a missed click. MemeHitLikelihood was the only file in the codebase using the deep-subpath form (lucide-svelte/icons/signal, /help-circle, /alert-triangle, /loader-2). Every other component imports from the lucide-svelte barrel, which is already in the eager graph. Because this component is dynamically imported, those four subpaths were node_modules dependencies the dev server never saw at startup, so first touching them mid-session triggered a Vite dependency re-optimization and a full page reload. That dropped the selected file and returned the app to the Data tab, which is why the failure snapshot showed "Upload or select a file to get started" and no method dropdown, and why no amount of waiting helped. Reproduced deterministically by clearing node_modules/.vite, which is the state CI is always in after npm ci. Two cold-cache runs are now clean, and faster (38.7s vs 51s) because the reload is gone. Production output is unaffected: the estimator is still a separate chunk, and the e2e byte budget still holds. This also fixes a real dev-mode papercut — the same reload hit anyone running npm run dev the first time they selected MEME.
The model was trained on one label: whether a MEME run reported at least one selected site. It has never scored BUSTED, aBSREL or FUBAR, so it cannot support claims about them — and five of the six guidance strings made exactly those claims. "BUSTED can detect selection here", "FUBAR holds up better than MEME here", and "a different method will not help" are all predictions about methods this estimate has no evidence on, carrying the authority of a number that only describes MEME. The reasoning behind them was defensible phylogenetics, which is the problem: it was the author's judgement presented as the model's finding, and a reader cannot tell the two apart. Guidance now stays on what the estimate can speak to — whether MEME has enough substitutions to fit its site-level tests, and what about the DATA would change that. "Collect more divergent sequences" is a claim about this alignment relative to MEME's needs. "Run BUSTED instead" is a claim about BUSTED. Removes the switch-method routing and its plumbing (the dispatch chain through RunOutlook to MethodSelector.switchMethod) rather than leaving it computed and unused. Adds a regression guard at both the unit and e2e level asserting no other method is ever named in user-facing text.
Two guards for the two ways this branch nearly embarrassed itself. Scope. The cross-method claims were caught by a person reading the copy; nothing in the suite noticed. Adds a guard that walks every state the estimate can reach — non-MEME, no alignment, topology-only tree, out of distribution, scored at each level, each tree-source caveat, and the error path — and asserts no other analysis method is named in any field that can reach a screen. State enumeration rather than a source grep, so a new string in a new file is still covered as long as some state renders it. Verified to fail: reintroducing "BUSTED would detect it gene-wide" turns it red. Note the two scope tests are complementary, not redundant. The state walk does not reach the generic 'unlikely' branch, because no realistic alignment lands there; the per-level test constructs feature vectors directly and does. Staging. Datamonkey_2_0, video-frames, benchmark-results, hyphy-wasm-artifact and src/benchmark/test-alignments are local scratch at the repo root, so a reflexive `git add -A` sweeps 265 files and ~270 MB into a commit. That happened twice here and was caught both times before pushing. Ignoring them makes the safe command the easy one. The loose scripts/*.sh and e2e/video-frames.spec.js are left tracked-able on purpose: they look like real figure tooling, not artifacts.
The estimate now reads XGBoost's own save_model() output directly. There is no converter and no intermediate format between the file the ML team exports and the bytes the browser parses. Why the pipeline changed Previously: sklearn -> skl2onnx -> .onnx -> onnx_to_ensemble.py -> ensemble.json -> treeEnsemble.js. Every hop could drift, and twice did: skl2onnx emits a different graph shape per estimator family, and HistGradientBoosting would not export at all. Three model families arrived in three days, two of which broke the converter. XGBoost's JSON is a stable, documented, first-party format, so the whole chain collapses to export-then-parse. xgbEnsemble.js is ~40 lines and is the entire runtime. Why the thresholds changed The old cuts were derived against meme_validation.tsv, which turns out to be the file train_gate_xgb.py fits on — so those rates were measured on the model's own training rows. Re-measured against the 5,982 production jobs the model has never seen, the 0.30 cut gives a 28.6% hit rate, which does not support the word "unlikely". UNLIKELY_MAX moves to 0.10, where the rate is 5.4% [2.0, 11.4] on 1.9% of submissions. LIKELY_MIN is unchanged. Out-of-sample band rates, n=5,982: 5.4% / 51.9% / 92.6%. Copy states each band's own observed rate and nothing else. The overall base rate was removed: it is a claim about the corpus rather than about the alignment on screen, and repeating it in all three bands added nothing. Verification - verify_parity.py now proves the shipped JS walker reproduces XGBoost's own scoring of meme_gate.json, and self-tests: it was confirmed to fail on a perturbed leaf, on a flipped comparison operator, and on broken base_score parsing. - New out-of-sample calibration test binds the copy's claims to the held-out population; confirmed to fail when the band cuts widen. - The scope guard now reaches all three bands; confirmed to fail when a cross-method claim is injected into the uncertain copy. - meme_gate.json is committed byte-identical to the exported artifact, and the production chunk was verified to contain those exact bytes. - Estimator CSS moved out of the component so a non-MEME method fetches zero estimator bytes in a production build, not just in dev. 369 unit tests, 5/5 e2e, parity gate green.
The `test` script is already `npm run test:unit -- --run`, so the job's extra `--run` reached vitest twice and it exited with "Expected a single value for option --run, received [true, true]" before running a single test. The job has failed on every push since it was added, and the message reads like a vitest bug rather than a mistake in the invocation.
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
1 participant
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.
Reworks the AxoMEME stage-1 gate (#147) into a "Before you run" outlook row, and removes the ONNX runtime from the browser bundle entirely.
Why
The original wiring shipped an advisory badge above the Run button. Reviewing it turned up twelve problems, the load-bearing ones being that it cost every user 13.5 MB whether or not they used MEME, presented an unsourced probability to researchers who publish results, and told people their analysis was unlikely to work without telling them what to do instead.
Behaviour
RunOutlookpanel holds both the runtime estimate and the MEME estimate, instead of two near-identical coloured boxes stacked above Run.not-applicable,loading,ok,cannot-assessanderrorare now distinguishable, so a broken estimator no longer renders identically to a healthy one.Payload
onnxruntime-webandonnxruntime-nodeare both dropped. The 150-tree ensemble is walked directly bytreeEnsemble.jsover a 37 KB coefficient table.Measured with a
page.on('response')byte counter against both the dev server and a production build. Removing the native binding also unbreaks the unit suite on x64 Node, which previously could not loadonnxruntime-nodeat all.Verification
Replacing a runtime with 40 lines of JS is only safe if the coefficients provably match the model the ML and QC teams validated, so that is now enforced rather than assumed:
scripts/prescreen/onnx_to_ensemble.pyregenerates the coefficient table from the.onnx. It reproduces the committed table exactly — zero difference across all 2,204 nodes.scripts/prescreen/verify_parity.pyscores 20k deterministic vectors plus adversarial rows through both onnxruntime and the shipped JS evaluator, and fails on any level disagreement or hash mismatch. Currently: max |Δ| 1.5e-07, zero level disagreements..github/workflows/model-parity.ymlruns both on any change undersrc/lib/services/prescreen/orscripts/prescreen/..onnxhash pin is now compared against the file on disk instead of pattern-matched, so regenerating against the wrong model is caught.Both failure modes were tested by deliberately tampering — a corrupted hash and a shifted leaf weight each produce a non-zero exit.
Retraining is now
npm run regen:model && npm run verify:model-parity.Known limitation — please read before merging
The model is not monotone in its inputs: adding sequences, codons or tree depth can lower the estimate, occasionally across a bucket boundary. At 200 codons and branch length 0.01, going from 28 to 29 sequences drops it from likely to uncertain. Tracked in #151, with a monotone retrain already prepared by the ML team.
This PR discloses the behaviour to the user rather than hiding it, but a reviewer should decide whether that is sufficient to ship or whether it waits on the retrain.
Review notes
frac_p_definedis retained in the feature vector but uses 0 of the model's 2,205 split nodes; it is a post-hoc feature the training never selected. The monotone retrain drops it, which is a[1,4]→[1,3]contract change.src/lib/services/prescreen/meme_hit_likelihood.onnxis committed but never fetched at runtime — it exists so the coefficients can be regenerated and re-verified.