fix(tier): tier routing reaches the wire — the tier was computed and never sent - #35
Merged
Merged
Conversation
…never sent
PRD §13.1 move 3 says "route models by step type (40–70%)" and M3's row
claims `tierCombo` does it. It did not. The runner computed a tier per step,
stored it on the run record, and then sent **one fixed combo forever**:
`onegw.Client.Chat` took no model parameter and the client was constructed
with a single combo. "Tiered routing" was a field on a run.
Worse, the names. `tierCombo` emitted planning/execution/tiny, and this
portfolio's onegw ships exactly **one** combo (`dev`) — its own README and
the live `onegw.toml` both say so. Two of the three tier names were combos
upstream does not have, so even a correct implementation could not have
routed them.
## What changed
- `ModelClient.ChatTier(ctx, tier, msgs…)` replaces the tier-less `Chat`
on the interface. A `Tierless` adapter keeps any Chat-only fake working.
- `onegw.Client.WithTiers(map[string]string)` maps tier → combo;
`ChatTier` resolves it and `Chat` stays as the default-combo path.
- The three tiers are now the three decisions the loop actually makes:
**planning**, **execution**, **synthesis**. `tierForStepName`
normalises an unknown plan tier onto execution, so an old plan naming
`tiny` still routes instead of falling off the table.
- An unmapped tier falls through to `AGENTLOOP_ONEGW_COMBO`. A one-combo
deployment is unchanged — a misconfigured tier costs the wrong model,
not the run.
- `Reply` carries both `Combo` (asked for) and `Model` (answered), because
a gateway fallback is precisely the event tiering has to be judged on;
recording only the answering leg made a fallback invisible.
Env: `AGENTLOOP_ONEGW_COMBO_{PLANNING,EXECUTION,SYNTHESIS}`.
## Verified against a stub gateway
asked: ["exec-combo", "exec-combo", "exec-combo", "synth-combo"]
Three action steps routed to the execution combo, the bound-exit answer to
the synthesis combo, and the run reported `answered_by: leg:synth-combo` —
the combo it was actually sent to.
## Tests
6 new in `internal/onegw` (tier routes to its combo, unknown tier falls back,
no tiers means the default, Reply records the combo asked for) and 2 in
`internal/loop` (the step's tier reaches the model; synthesis names its own
tier). `TestM3_PlannerDrivenRun` previously asserted `planning or tiny` —
it now asserts one of the three real tiers, which is the drift this change
exists to stop.
## What this does NOT claim
The 40–70% saving. That is §11.4's paired parity suite — the same cases run
both ways, paired per case — and it has not been run. The mechanism works and
the number is still a hypothesis; the PRD says exactly that rather than
inheriting the move's figure.
Docs: PRD §13 records the drift, its cause and the fix; USAGE's config table
gains the three variables and §9 scopes the claim. The PRD footer's nine
stacked "Last updated" stamps are now one live stamp plus a dated changelog,
because a nine-deep footer is a footer nobody reads.
linhdmn
added a commit
that referenced
this pull request
Sep 21, 2026
The only real conflict was the tier work landing where this branch had put its own `/v1/systemone` client: `internal/systemone` and #37's `internal/guardrail` are the same client built twice, and #37's is the one on main — wired into the service, with the goal screen and `screen_errors`. Keeping both would have meant two clients, two batteries and two answer parsers for one wire, so this branch is reduced to what it actually adds. Kept from this branch (the parts #37 does not have): - **The hazard, not the label.** A blocked step's record names the hazard that fired (`jailbreak 0.91`) instead of the constant `noul_battery`, and a severity-driven block records the severity row too — a single hazard row would claim a low probability caused it. `internal/loop/screen.go`. - **`RunnerConfig.Policy`** (strict by default, `AGENTLOOP_GUARDRAIL_POLICY` selects permissive). `experiments.Strict` was hard-coded in the loop and in `screenGoal`, so §17's measured strict/permissive split — a product decision the PRD names — was unreachable without a rebuild. - **The answer-shape fix (#37's client, found by driving it).** #37's `probabilityOf` read `confidence` before the hazard, so TypeSafe's real envelope — `{"type":"noul","noul":0.91,"confidence":0.88}`, which is what the API actually sends — reported jailbreak 0.88 where it is 0.91. It also did not read Laya's `probabilities["1"]`. Both are now read, both pinned by `TestBothBackendEnvelopesAgree`, and `confidence` is out of the key list: it is a calibration claim, not a verdict. - **Why the client is here at all**, in `docs/JEV-INTEGRATION.md` §2.1: policy and the wire belong to agentloop, backend selection belongs to onegw, and the "provider abstraction" that would replace all of it is a fiction — Laya is a non-autoregressive encoder and cannot satisfy a generation- provider interface. Dropped from this branch: `internal/systemone` (superseded by `internal/guardrail`), its `GuardrailClient` seam and `guardrail_unavailable` exit reason (a screen that cannot run is an observation recorded in `screen_errors`, not a new terminal state — that is #37's contract and it is the right one), and this branch's duplicate env names. Verified: `make check` green (gofmt, vet, test, golangci-lint, check-prd); stub-sidecar drive through the real binary at `AGENTLOOP_GUARDRAIL_URL` blocks/holds with the hazard row on the step.
linhdmn
added a commit
that referenced
this pull request
Sep 21, 2026
) Second merge of `origin/main` (which now carries #37, "the TypeSafe screen is wired"). The conflicts were all one thing: this branch and #37 built the same client twice — `internal/systemone` here, `internal/guardrail` there — so the resolution keeps #37's tree and this branch's four additions on top of it. Why #37's client wins: it is wired into the service (goal screen, tool-result screen, `screen_errors` on the run, `guardrail_wiring_test.go`), it treats a screen that cannot run as a recorded observation rather than a new terminal state, and it is already the API the tests on main exercise. Two clients for one wire, each with its own battery copy and answer parser, is the duplication `docs/DUPLICATION-AUDIT.md` exists to prevent. What this branch adds, i.e. what the conflict resolution preserves: 1. **The hazard, not the label.** A blocked step's record names the question that fired (`jailbreak 0.91`) instead of the constant `noul_battery`, and a severity-driven block also records the severity row — one hazard row would claim a low probability caused a block the severity decided. (`internal/loop/screen.go`, `internal/loop/screen_test.go`) 2. **`RunnerConfig.Policy`.** `experiments.Strict` was hard-coded in the loop and in `screenGoal`, so §17's measured strict/permissive split — which the PRD names as an operator decision — was unreachable without a rebuild. `AGENTLOOP_GUARDRAIL_POLICY=permissive` now selects it, and anything unrecognised stays strict. 3. **The answer-shape fix in `internal/guardrail`.** `probabilityOf` read `confidence` before the hazard, so TypeSafe's real envelope — `{"type":"noul","noul":0.91,"confidence":0.88}`, which is what the API sends — reported jailbreak 0.88 where it is 0.91; a 0.03 error in the direction that under-blocks. Laya's `probabilities["1"]` was not read either. Both envelopes are now read and pinned by `TestBothBackendEnvelopesAgree`; `confidence` is out of the key list because a calibration claim is not a verdict. 4. **`docs/JEV-INTEGRATION.md` §2.1** — why the client lives here: policy and the wire belong to agentloop, backend selection belongs to onegw, and the generation-provider abstraction that would replace it is a fiction, because Laya is a non-autoregressive encoder. Dropped: `internal/systemone`, its `GuardrailClient` seam, and its `guardrail_unavailable` exit reason. Verified after resolving: `make check` green (gofmt, vet, go test, golangci-lint, `docs/check-prd.py`).
This was referenced Sep 21, 2026
linhdmn
added a commit
that referenced
this pull request
Sep 21, 2026
…inters (#38) The index said "the three remaining stub tools, the model-driven planner and tier-combo names are the next milestone". Three of those four are now landed (#33 execution, #34 the reasoner, #35 tier routing), so the note was stale and the real next step was unnamed. Rewritten as a handoff, with each item verified against the code rather than recalled: 1. **The loop cannot read a file** — four tools, none of which returns file *content*. `query` returns graph elements with a 400-char excerpt and a rung; it is not a reader. So the reasoner locates parseConfig, learns it is in config.go:41, and cannot look at it — which is exactly why the live demo wrote `func parseConfig() {}`. `write_file`'s description already promises a "read twin" that does not exist. 2. **The run has no answer** — `goal_met` stores its synthesis in `PartialSynthesis`, a field named for the bound case, and the model's rationale sits in a step's `why`. 3. **Success criteria are prose** — `ps.Success` appears exactly once in the codebase: as prompt text in reason.go. Nothing evaluates it. 4. `web_search` is the last stub (or should be deleted). 5. The two gaps #37 recorded: reply-out unscreened, ~740ms/call unmetered. 6. Planning is still a rule table (no longer blocking — the chooser carries). Plus the working notes a new session loses time rediscovering: the worktree rule, `make check` as the gate, the shell quirks (nohup for backgrounded servers, pkill matching its own command line, ports 8080/9699 taken), and the one that matters most — every defect this repo shipped lately was a check that could not fail (#25, #29, #32, #37, and #8's checklist ticked on the author's behalf). Verify by breaking the thing, not by watching it pass.
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.
The finding
PRD §13.1 move 3 says "route models by step type (40–70%)". M3's row claims
tierCombodoes it. It did not.The runner computed a tier per step, stored it on the run record, and then sent one fixed combo forever —
onegw.Client.Chattook no model parameter, and the client was constructed with a single combo. "Tiered routing" was a field on a run.Worse, the names.
tierComboemittedplanning/execution/tiny, and this portfolio's onegw ships exactly one combo (dev). Two of the three tier names were combos upstream does not have — so even a correct implementation could not have routed them.What changed
Chat(ctx, msgs…)ChatTier(ctx, tier, msgs…)— aTierlessadapter keeps Chat-only fakes workingWithTiers(map[string]string);Chatstays as the default pathAGENTLOOP_ONEGW_COMBO; a misconfigured tier costs the wrong model, not the runReplyCombo(asked for) andModel(answered) — a fallback is the event tiering must be judged on, and recording only one side hid ittierForStepNamenormalises an unknown plan tier onto execution, so an old plan namingtinystill routes rather than falling off the table.Verified against a stub gateway
{"asked": ["exec-combo", "exec-combo", "exec-combo", "synth-combo"]}Three action steps routed to the execution combo, the bound-exit answer to the synthesis combo, and the run reported
answered_by: leg:synth-combo— the combo it was actually sent to.Tests
internal/onegw: tier routes to its combo, unknown tier falls back, no tiers means the default,Replyrecords the combo asked for.internal/loop: the step's tier reaches the model; synthesis names its own tier.TestM3_PlannerDrivenRunpreviously assertedplanning or tiny. It now asserts one of the three real tiers — that assertion is why the drift survived: it accepted a name that could never be routed.What this does NOT claim
The 40–70% saving. That is §11.4's paired parity suite — the same cases run both ways, paired per case — and it has not been run. The mechanism works; the number is still a hypothesis. The PRD says exactly that rather than inheriting the move's figure, and USAGE §9 scopes it the same way.
Docs
Last updatedstamps in the footer are now one live stamp plus a dated changelog (a nine-deep footer is a footer nobody reads).AGENTLOOP_ONEGW_COMBO_{PLANNING,EXECUTION,SYNTHESIS}; §9 replaces "tier routing is half-wired" with what is now true and what is still unproven.Checks
make checkgreen: gofmt, vet, 14/14 packages, lint 0 issues, PRD OK, selftest 12/12.