Repository navigation
laya-first: ship the routing table and desugar it to a collection.router policy - #1069
Conversation
…ter policy
docs/laya-first.md §4 says the table is the product surface and the policy is the mechanism, so this
adds the table and the desugaring, not a new router: config/laya-first.json (class -> model, hf_repo,
file, device, ctx) ships compiled into the engine like config/recipes.json, and `1bit laya-first`
validates it and prints the collection.router policy Lemonade already understands - one rule per class,
route_to that class's model.
Two details in the emitted policy are load-bearing, and both are asserted rather than commented:
- **min_score 0.0 on every class rule.** Laya's calibrated confidence for `code` is about 0.44, so the
fork's classifier band (default floor 0.5) would drop exactly the decisions this table exists to
route. Confidence stays in the trace; an unroutable request still falls open to default_model because
no rule matched, not because a band rejected the best class.
- **Labels on the classifier entry.** Declaring short/prose/code/long_doc is what makes a typo in a
rule's `label` a parse error in Lemonade instead of a rule that silently never matches.
Validation is strict because a table is deployed, not read: version, the four class names and nothing
else, model/hf_repo/device strings, device in {hrx,npu,hybrid,auto} (on_busy's device too), a positive
ctx, and a default_model that is one of the classes' models. A bad table is refused with the offending
value named.
tests/laya_first.sh, 19 checks, all passing: the shipped table validates; the emitted policy is
byte-identical to tests/laya_first_policy.json (so a table or desugar change is a deliberate diff); one
rule per class, every route_to a candidate, components and candidates agreeing, min_score 0.0, the
classifier's four labels and no model/prompt; a device typo and a missing class refused by name; and -
when a Lemonade checkout is reachable - the policy validates against that repo's
route_policy.schema.json, because the policy is for Lemonade and its schema is the contract. Measured
here: 0 schema errors.
Part of docs/laya-first.md §8 step 3; packaging the file into a release share/ dir is the release lane's
step, and until it lands the table is compiled in (with --laya-first FILE and
${XDG_DATA_HOME:-~/.local/share}/1bit/laya-first.json as overrides).
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at c6f877a2 — built it, its 19 checks pass, and I validated the emitted policy against the fork's schema myself. Two things to carry into the merge order.
Ran here (cmake --build --target onebit, tests/laya_first.sh build/1bit): all 19 ok, PASS. Including the ones that matter for a deployed table: the emitted policy is byte-identical to tests/laya_first_policy.json, every class rule is min_score 0.0, the classifier declares the four class labels, a device typo is refused, naming it, a missing class is refused, naming it.
min_score: 0.0 is not a guess, and I can corroborate it with a number I measured myself. On #1060's resident route service, a code prompt came back {"class":"code","confidence":0.4597368836402893,…} — i.e. 0.4597, under the fork's default 0.5 band. So the floor really would have dropped the exact decisions this table exists to route; the comment's "~0.44" and my measurement agree.
Merge coupling — this policy is only valid once lemonade #32 lands. The emitted policy carries {"type": "laya"} on its classifier, and I validated it against two schemas:
fork main (src/cpp/resources/schemas/route_policy.schema.json, no `laya` yet)
-> 1 error: ['routing','classifiers',0,'type'] 'laya' is not one of [semantic_similarity, classifier, llm, …]
fork #32 (~/wt/fork-laya @ dbe2db25b, the checkout the test itself uses)
-> 0 errors
So #32 (lemonade) must merge before or with #1069; landing the table first would leave a policy that a downstream schema check rejects. Worth putting that sentence in the body — it is the one ordering fact another lane needs.
And the cross-repo check is weaker than it reads, which is worth knowing before anyone leans on it. The same schema accepts both of my negative controls:
emitted policy with rules[0].match.min_score = 0.5 -> 0 schema errors
emitted policy with rules[0].match.label = "coding" -> 0 schema errors
The schema constrains structure, not those values. The real guarantees for the two load-bearing details come from elsewhere and are already covered: min_score: 0.0 by the byte-identical golden file plus the engine's own --check, and "a label typo is a parse error" by Lemonade's parser (that is one of #32's assertions, a label that is not a request class is a parse error). Suggest phrasing the body's cross-repo line as "the emitted policy is structurally valid against the fork's schema" so the two semantic claims are not read as schema-enforced.
Also noticed while checking: the test finds the schema at $LEMONADE_ROUTE_POLICY_SCHEMA, else ~/wt/fork-laya/…, else prints ok … skipped. That is honest (it does not pass silently), but it does mean CI will not exercise the cross-repo check at all unless the variable is set there — so the coupling above is currently only caught by whoever runs the test on a machine with a fork checkout, as I just did.
…roved it
Exercising the release-check row against a live lemond found a real defect in the desugar: with
`min_score: 0.0` a class rule matches every request. The classifier reports only the class Laya chose, so
every other label scores exactly 0.0, and `0.0 >= 0.0` satisfies a 0.0 floor - the FIRST class rule then
swallows everything and a `short` prompt routed to the `code` model.
Cost of finding it: one live run. Cost of missing it: the appliance's first request would have gone to the
wrong model for three of four classes while every self-test stayed green, because a verdict can only see
that a rule matched, not that the right one did. The tests now assert a positive floor instead of zero, and
the docs say why the number is 0.01 rather than 0.
Verified with the same live lemond after the fix: short, prose and code each PASS on the row's own verdict,
and an engine that cannot answer still FAILs ("no Laya rule matched").
|
A live run found a real defect in my desugar — fixed at With Measured against a live lemond (a stand-in engine, no device), using this desugar verbatim: So the shipped table would have sent Thanks to whoever runs the release row next — that exercise is what caught it. |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at fb30a9c0 — and one of the two changes supersedes reasoning I endorsed in my first review, so it is worth saying that plainly.
min_score: 0.0 → 0.01 is a real fix, and my earlier endorsement of 0.0 was wrong in the way that matters. I approved the zero floor on the argument that Laya's code confidence (~0.44, which I had measured myself at 0.4597) sits under the band's 0.5 default, so 0.5 would drop the decisions the table exists to route. That argument is sound and it stopped one step short: the classifier reports only the class Laya chose, so every other label scores exactly 0.0, and 0.0 >= 0.0 makes the first class rule match every request. Your live-lemond evidence — a short prompt routed to the code model — is exactly that arithmetic. A floor strictly above zero is the right shape: it cannot drop a real decision (a chosen class never scores 0) and it stops the swallow.
1bit.Laya-First → user.Laya-First is the other half of a real bug, and it is the reason #1071's row now derives the name from 1bit laya-first instead of hardcoding it. Verified on the rebuilt binary:
$ 1bit laya-first | head -4
{ "version": "1", "model_name": "user.Laya-First", "recipe": "collection.router", …
$ 1bit laya-first | grep -o '"min_score": [0-9.]*' | sort -u
"min_score": 0.01
and the 19-check suite passes at this head (bash tests/laya_first.sh build/1bit → PASS), including the byte-identical golden, the four-class rules, the refusals that name the offending value, and the schema validation.
My dependency note is unchanged and still load-bearing: the emitted policy carries {"type":"laya"}, so it validates against #32's fork schema (0 errors, checked again here) and not against the fork main schema (1 error, 'laya' is not one of …). Merge #32 first or with this.
Worth recording for the release lane: user.Laya-First is a registration name, not a cosmetic choice — the server only registers user.* definitions, which is what #1071's comment now says. A gate that hardcoded the old name would have gone red on the first good image.
…ion measured The note's §9 asked four questions; building steps 1-3 and 5 changed what the answers cost, so each now ends with the recommendation the measurements support and the measurement behind it: - the small NPU row is a provenance question, not a size one (an FLM checkpoint makes one row claim something different from every other row), and the table should carry the provenance if that is chosen; - the table's names must be registrable: Lemonade refuses definitions outside user.* (measured), so the router collection is user.*, while components keep their catalogue ids - which makes 'catalogue vs table' a question about hf_repo alone; - 'auto' has a third shape the work made obvious: alias it to user.Laya-First at install time, which is reversible configuration rather than a reserved name or a squatted one; - the 18.6 GB first pull is one line in the table, not a code decision, and reversible the same way. Plus the one constraint every one of those answers shares: a class rule needs a floor above zero, because at 0.0 the first rule matches every class (a short prompt routed to the code model, with every test green).
|
Pushed
Every answer shares one constraint: a class rule needs a floor above zero, because at |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at this head (rebase-only; the review stands).
This PR's own files are byte-identical between the heads — git diff over app/laya_first.cpp config/ tests/laya_first.sh tests/laya_first_policy.json docs/laya-first.md is empty — so nothing in my verdict changes.
The two corrections from the previous push still hold: min_score: 0.01 (a 0.0 floor matches every class, since the classifier scores only the chosen label) and user.Laya-First (the only shape the server registers). 19/19 checks passed when I rebuilt at that head, and the schema coupling stands: the emitted type: "laya" validates against lemonade #32's schema (0 errors) and not against the fork's main (1 error) — merge #32 first or with this.
… of them inside the split's file, and the moved block survives it main went bd7d451 -> cb5866e -> 05da4d8 while the five gate rows were being re-measured, and one of the twelve commits edits the file the split is built around: npu/forward/src/q4nx_forward.cpp +38/-5 (#1087 rope-scaling refusal by name, #1072 the batch phase counters zeroed BEFORE BatchPhases snapshots them) npu/forward/include/q4nx_rope.h +72 (new: q4nx_rope_declared_type / q4nx_rope_type_supported) npu/pack_import.cpp/.h +97 (#844 final-norm import from the .fconst tail envelope) app/, config/, docs/, tests/, tools/, os/ (laya-first #1069, determinism tool #1068, serve logging #1074, onnx pricing #1076, puppy vm-check #1080, release-check #1079/#1071) third_party/llama.cpp pin bump 6e42864a6 (#1075) Textually clean (`git merge-tree --write-tree` produced no CONFLICT), which is exactly the case that was NOT clean the last time main landed in that file — so the merge was checked by what the compiler and the tests say, not by "auto-merging" messages: - #1072 moved `BatchPhases bph` / `BatchPhaseScope _batch_scope` / `tp_bs` so the counters are zeroed before the snapshot is taken. Those symbols live in npu/forward/include/q4nx_lh.h in this branch (the move this PR exists for); the statement order is main's and is carried over verbatim, so the reordering lands on the header's definitions unchanged. - The moved set is still defined exactly once: `inline StepPhases g_ph` (q4nx_lh.h:116), `phprof_on`/`ph_ms` (q4nx_lh.h, one each), `inline bool g_in_batch` (:131), `struct BatchPhaseScope` (:132), `lh_dump_layer_host` — and q4nx_forward.cpp no longer defines any of them. - `BatchPhases` remains a local struct in q4nx_forward.cpp (one definition), which is where main keeps it. Build: `cmake --build build-p4 --target onebit -j4` under ~/lb/thermal-run.sh -> rc 0, 16 objects rebuilt (warm tree; the pin bump and the two touched TUs). Tests: `ctest` **87/87, 0 failed** (85 passed, 2 skipped: mtp_batch_all, gdn_batch_order — the same two that skip on main). Consequence, stated rather than implied: the five gate rows measured on A=bd7d451a are VOID and are being re-run against A built at this main; this commit carries no numeric claim of its own.
What
docs/laya-first.md§4 says the routing table is the product surface and thecollection.routerpolicy isthe mechanism. This adds both, and no new router:
config/laya-first.json— class →{model, hf_repo, file, device, ctx}(short/prose/code/long_doc),compiled into the engine the way
config/recipes.jsonandconfig/pm-experts.jsonalready are, with--laya-first FILEand${XDG_DATA_HOME:-~/.local/share}/1bit/laya-first.jsonas overrides.1bit laya-first [--check] [--policy FILE]— validates the table and prints the policy it desugars into:one rule per class,
route_tothat class's model, thelayaclassifier declared with the four labels.Two load-bearing details, both asserted
min_score: 0.01on every class rule (not0.0). Laya's calibrated confidence forcodeis ~0.44, sothe fork's classifier band (default floor 0.5) would drop exactly the decisions this table exists to route.
But a
0.0floor is the opposite bug: the classifier reports only the class Laya chose, so every otherlabel scores exactly
0.0, and0.0 >= 0.0makes the first class rule match every request — measuredagainst a live lemond, a
shortprompt went to thecodemodel with every self-test green.0.01cannotdrop a real decision (a chosen class never scores 0) and cannot swallow the rest. Confidence stays in the
trace; an unroutable request falls open to
default_modelbecause no rule matched — not because a bandrejected the best class.
Labels on the classifier entry. Declaring
short/prose/code/long_docis what makes a typo in a rule'slabela parse error in Lemonade instead of a rule that silently never matches (the classifier support isfork #32).
Validation is strict because a table is deployed, not read
version
"1"· exactly the four Laya class names ·model/hf_repo/devicestrings ·devicein{hrx, npu, hybrid, auto}(andon_busy.devicetoo) · positive integerctx·default_modelthat isone of the classes' models. A bad table is refused with the offending value named —
vulkanin a devicefield says
vulkan.Proof —
tests/laya_first.sh, 19 checks, all passByte-equality with
tests/laya_first_policy.jsonis deliberate: changing the table or the desugaring thenshows up as a reviewable diff instead of drift.
Cross-repo check I ran and kept: the emitted policy is structurally valid against the Lemonade
fork's
route_policy.schema.json— 0 errors (jsonschema, Draft 2020-12) — and the forkmainschemarejects it with exactly one error (
'laya' is not one of …), becausetype: "laya"arrives with fork #32.The schema constrains structure, not values: it accepts
min_score: 0.5andlabel: "coding"equally, soit is not what pins those two. They are pinned elsewhere —
min_score: 0.01by the byte-identical goldenfile plus this binary's own
--check, and a label typo by Lemonade's parser (one of #32's assertions).The test re-runs the schema check whenever a fork checkout or
$LEMONADE_ROUTE_POLICY_SCHEMAis presentand skips visibly otherwise, which also means CI does not exercise it unless the variable is set there.
Merge order (load-bearing): the emitted policy carries
{"type":"laya"}, which only fork #32 parses,so lemonade #32 must merge before or with this.
Neighbours on the same binary:
laya_gguf_route,laya_route_e2e,laya_route_serve,laya_checkpoint,laya_encoder,laya_bounds,laya_classify— 7/7 pass.Not here
Packaging the file to
share/1bit/laya-first.jsonin a release — that is the release lane's step; untilthen the table is compiled in and the two overrides work.
on_busyis validated but absent from theshipped table, per the note's §4: the thresholds are H0's measurement, not an invented knee.