diff --git a/cmd/agentloop/main.go b/cmd/agentloop/main.go index b2c6d37..c2273c1 100644 --- a/cmd/agentloop/main.go +++ b/cmd/agentloop/main.go @@ -72,7 +72,7 @@ func NewServer() *Server { envOr("AGENTLOOP_ONEGW_URL", "http://127.0.0.1:8080"), os.Getenv("AGENTLOOP_ONEGW_KEY"), envOr("AGENTLOOP_ONEGW_COMBO", "dev"), - ), + ).WithTiers(tiersFromEnv()), evalRunner: eval.NewRunner(func(cfg loop.RunnerConfig) (*loop.LoopRunner, *budget.Guard, tools.ToolRegistry, error) { // Same runner the service builds, minus the model: the gate and // the planner are part of what the cases exercise, so building a @@ -105,6 +105,31 @@ func envOr(key, def string) string { // // No LeanKG API key: the service is local and its REST surface is // unauthenticated by design for a single-tenant deployment (PRD §7.4). +// tiersFromEnv maps this loop's three routing tiers onto onegw combos. +// +// The gateway ships whatever combos an operator configured — in this +// portfolio, exactly one (`dev`). Naming a combo here that does not exist +// upstream is how "tiered routing" becomes an error instead of a saving, +// so an unset tier simply falls through to AGENTLOOP_ONEGW_COMBO and the +// loop still runs: +// +// AGENTLOOP_ONEGW_COMBO_PLANNING combo for plan/replan steps +// AGENTLOOP_ONEGW_COMBO_EXECUTION combo for action steps (the hot path) +// AGENTLOOP_ONEGW_COMBO_SYNTHESIS combo for the bound-exit answer +func tiersFromEnv() map[string]string { + out := map[string]string{} + for tier, key := range map[string]string{ + loop.TierPlanning: "AGENTLOOP_ONEGW_COMBO_PLANNING", + loop.TierExecution: "AGENTLOOP_ONEGW_COMBO_EXECUTION", + loop.TierSynthesis: "AGENTLOOP_ONEGW_COMBO_SYNTHESIS", + } { + if v := os.Getenv(key); v != "" { + out[tier] = v + } + } + return out +} + func leankgFromEnv() *leankg.Client { if os.Getenv("AGENTLOOP_LEANKG_OFF") != "" { return nil diff --git a/docs/PRD.md b/docs/PRD.md index 2ce9ee5..c32161b 100644 --- a/docs/PRD.md +++ b/docs/PRD.md @@ -441,6 +441,12 @@ Three of the twelve are the ones that decide whether this is a plan or a wish. * **Definition of done for M1–M6:** the milestone's acceptance passes in CI, the status column here is updated in the same commit as the work, and each acceptance becomes a named eval case — never a prose claim. +**Tier routing is wired, and the combo names were fiction (2026-09-21).** §13.1's 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 **sent one fixed combo forever** — `onegw.Client.Chat` took no model parameter and the client was constructed with a single combo. Worse, `tierCombo` emitted `planning`/`execution`/`tiny`, and this portfolio's onegw ships **one** combo (`dev`), so two of the three names were unrouteable even if they had been sent. + +Fixed: `ModelClient.ChatTier(ctx, tier, msgs…)` carries the tier to the wire, `onegw.Client.WithTiers` maps a tier to a combo, and the three tiers are now the three decisions the loop actually makes — **planning**, **execution**, **synthesis** — renamed from the old trio because a tier name that is not a combo upstream is a decorative constant. An unmapped tier falls through to `AGENTLOOP_ONEGW_COMBO`, so a one-combo deployment still runs. `Reply` now records both the combo asked for and the leg that answered, because a fallback is the event tiering has to be judged on. + +What this does **not** do: prove the 40–70% figure. That is §11.4's paired parity suite, and the acceptance has not been run. + **Task record:** this table is the status summary, not the task list. Tasks live as **GitHub issues in [`FreePeak/agentloop`](https://github.com/FreePeak/agentloop/issues)** and `todo.md` is the short index of the open ones; no `TASKS.md` is ever created. Two tracker conventions apply to this repo and are *not* yet followed: issues are now banded **P0**–**P3** (labels defined 2026-09-21; the band meanings are one line each in `todo.md`), and the open set is: **P1** [#27](https://github.com/FreePeak/agentloop/issues/27) CI + [#21](https://github.com/FreePeak/agentloop/issues/21) onegw combo reorder; **P2** [#20](https://github.com/FreePeak/agentloop/issues/20) HTMX console + [#8](https://github.com/FreePeak/agentloop/issues/8) TypeSafe checklist; **P3** [#19](https://github.com/FreePeak/agentloop/issues/19), [#15](https://github.com/FreePeak/agentloop/issues/15), [#28](https://github.com/FreePeak/agentloop/issues/28). What is still not tied together is closure: a merged PR does not close its issue, which is why [#8](https://github.com/FreePeak/agentloop/issues/8)'s implemented half still reads as open — that is the remaining half of [#28](https://github.com/FreePeak/agentloop/issues/28). **Schedule overlay** (from the book's 30-day plan, adapted — the book's week 1 "from-scratch ReAct with no framework" is subsumed by M1, and its weeks 5–8 become M4–M6): @@ -736,13 +742,14 @@ Written the way an unfriendly reviewer would write it, then answered. Every find **Read next.** §13.1 (scope → milestones), §17 (defaults), §18 (where to discount the source), §22 (this document's own weaknesses). -* Last updated: 2026-09-21 (The loop **decides its own steps**. `internal/loop/reason.go` makes one model call per step with the previous step's verbatim result, and the answer (`{tool,args,why,done}`) is what runs — so the loop observes before it reasons, which is the half it was missing. `done` exits `success`/`goal_met`, a new exit reason for the goal predicate firing rather than a bound. A resume replays the approved decision instead of re-asking (a second call can choose a different tool, so the operator would have approved one action and a different one would run). `StepRecord` now carries its `Args` and `Why`. No model client still means the deterministic rotation, unchanged.) -* Last updated: 2026-09-21 (The loop can finally **write and verify**. `internal/xdev` speaks xdev's `rpc` JSONL protocol (ready-frame version gate, event-before-response interleaving, one turn at a time, child killed when the step's budget expires) and `run_tests`/`write_file` run as one xdev turn each, in a sandboxed workspace from `AGENTLOOP_XDEV_DIR`. With no sandbox the two report *no executor configured* and `written`/`ran` stay false — "no sandbox" can never read as "the tests passed". `nextToolDefault` now gives each tool the arguments it needs to be a real call, so a write has a target instead of failing closed on a missing path. `web_search` is the last stub.) -* Last updated: 2026-09-21 (The M6 eval gate was reporting **0.5 — deploy blocked — for days** while `go test ./...` was green. Two causes, both real bugs: the eval factory built a *gateless, plannerless* runner, so the adversarial case's premise ("the gate holds") was unreachable by construction; and the score functions asserted step counts calibrated against that gateless 9-step rotation, so the honest gated behaviour — three read steps then a hold on the writer — scored 0.3. The factory now builds the service's runner (minus the model, as §11.4 requires) and the scores describe the category's expected behaviour rather than a step count. Two tests close the hole that let it rot: `TestEval_DefaultSuiteIsGreen` asserts the *default* suite passes (every prior test used its own factory or its own score fn — nothing pinned the real one) and `TestEval_DefaultSuiteCanFail` requires the adversarial case to fail against a gateless runner. Verified by breaking `Categorize` and watching the endpoint block at 0.75.) -* Last updated: 2026-09-21 (CI exists: `.github/workflows/ci.yml` runs gofmt, `go vet`, `go test`, `golangci-lint` (config pinned in `.golangci.yml`) and `docs/check-prd.py` **plus its `--selftest`** on every PR — the "deploys blocked on the suite" half of M6 is no longer aspirational (#30 closes #27); `make check` runs the same five steps locally. **Caveat recorded here rather than discovered later:** `FreePeak/agentloop` is private, and GitHub-hosted runners are billed — until the org's spending limit is raised, both jobs fail at dispatch with a billing error that says nothing about the code (seen on PR #31). `make check` is the fallback that keeps the gate honest in the meantime. Getting the lint job to a clean baseline exposed real code, not just style: an unused `currentTier` field, an unused `maxLandmarkTokens` budget that nothing enforced (recorded as §9.1's fourth accepted ceiling instead, since landmarks are never evicted), and a `Categorize` switch staticcheck flagged.) -* Last updated: 2026-09-21 (Docs synced to `b492cc2`. §13's M5 row recorded the `Categorize` fix (#25) and its stale test count; the M6 row now says out loud that the "deploys blocked on the full-suite gate" half is **not** enforced — there is no CI in this repo (#27); the §13 task-record line stopped claiming the repo has no commits/remote and now records the tracker state: priority bands P0–P3 defined and every open issue labelled (#28, half done), with issue closure still not PR-linked. `docs/USAGE.md` lost a duplicated line and its "does nothing" summary was split into what reads today vs what still does not. `docs/check-prd.py` stops false-failing on §-refs into other docs — the cause of the long-standing dual-§-ref FAIL, whose refs pointed at `docs/JEV-INTEGRATION.md`, not this PRD.) -* Last updated: 2026-09-21 (**The `query` tool is real**: it reaches LeanKG `POST /api/v1/query` through `internal/leankg`, wired from `AGENTLOOP_LEANKG_URL`, and reports the answering retrieval rung; a LeanKG outage is a recorded failed step, and the `stepError` panic on a `Success=false`/nil-error tool result is fixed. **The approval table now names the real tools**: `query`/`web_search`/`run_tests` are read-category and `write_file` holds on every call — before this all four fell to the fail-closed default, so every gated run paused on step 1 and M5's <10% interruption ceiling was unreachable.) — §4 and §5.1 FR-7, `docs/USAGE.md`, README.* -* Last updated: 2026-09-21 (TypeSafe coupling risk re-verified: onegw `systemone` Kind is **merged** into master — `9faea01`; the open blocker is PR #110 verdict-driven combo reorder, not the merge. `docs/JEV-INTEGRATION.md` §3 rewritten to reflect the split between the merged Kind and the unmerged routing.) — §14 risk row corrected; `docs/JEV-INTEGRATION.md` §3.1/§3.4/§3.5 marked done, §7 checklist itemised.* +* Last updated: 2026-09-21 (Tier routing reaches the wire. `ChatTier(ctx, tier, msgs…)` replaced the tier-less `Chat` on the model interface, `onegw.Client.WithTiers` maps tier→combo, and the three tiers are now planning/execution/synthesis instead of planning/execution/tiny — the old middle names were combos onegw does not ship. `Reply` records the combo asked for *and* the answering leg, so a fallback is visible. Verified against a stub gateway: three calls to `exec-combo`, one to `synth-combo`.) +* 2026-09-21 — (The loop **decides its own steps**. `internal/loop/reason.go` makes one model call per step with the previous step's verbatim result, and the answer (`{tool,args,why,done}`) is what runs — so the loop observes before it reasons, which is the half it was missing. `done` exits `success`/`goal_met`, a new exit reason for the goal predicate firing rather than a bound. A resume replays the approved decision instead of re-asking (a second call can choose a different tool, so the operator would have approved one action and a different one would run). `StepRecord` now carries its `Args` and `Why`. No model client still means the deterministic rotation, unchanged.) +* 2026-09-21 — (The loop can finally **write and verify**. `internal/xdev` speaks xdev's `rpc` JSONL protocol (ready-frame version gate, event-before-response interleaving, one turn at a time, child killed when the step's budget expires) and `run_tests`/`write_file` run as one xdev turn each, in a sandboxed workspace from `AGENTLOOP_XDEV_DIR`. With no sandbox the two report *no executor configured* and `written`/`ran` stay false — "no sandbox" can never read as "the tests passed". `nextToolDefault` now gives each tool the arguments it needs to be a real call, so a write has a target instead of failing closed on a missing path. `web_search` is the last stub.) +* 2026-09-21 — (The M6 eval gate was reporting **0.5 — deploy blocked — for days** while `go test ./...` was green. Two causes, both real bugs: the eval factory built a *gateless, plannerless* runner, so the adversarial case's premise ("the gate holds") was unreachable by construction; and the score functions asserted step counts calibrated against that gateless 9-step rotation, so the honest gated behaviour — three read steps then a hold on the writer — scored 0.3. The factory now builds the service's runner (minus the model, as §11.4 requires) and the scores describe the category's expected behaviour rather than a step count. Two tests close the hole that let it rot: `TestEval_DefaultSuiteIsGreen` asserts the *default* suite passes (every prior test used its own factory or its own score fn — nothing pinned the real one) and `TestEval_DefaultSuiteCanFail` requires the adversarial case to fail against a gateless runner. Verified by breaking `Categorize` and watching the endpoint block at 0.75.) +* 2026-09-21 — (CI exists: `.github/workflows/ci.yml` runs gofmt, `go vet`, `go test`, `golangci-lint` (config pinned in `.golangci.yml`) and `docs/check-prd.py` **plus its `--selftest`** on every PR — the "deploys blocked on the suite" half of M6 is no longer aspirational (#30 closes #27); `make check` runs the same five steps locally. **Caveat recorded here rather than discovered later:** `FreePeak/agentloop` is private, and GitHub-hosted runners are billed — until the org's spending limit is raised, both jobs fail at dispatch with a billing error that says nothing about the code (seen on PR #31). `make check` is the fallback that keeps the gate honest in the meantime. Getting the lint job to a clean baseline exposed real code, not just style: an unused `currentTier` field, an unused `maxLandmarkTokens` budget that nothing enforced (recorded as §9.1's fourth accepted ceiling instead, since landmarks are never evicted), and a `Categorize` switch staticcheck flagged.) +* 2026-09-21 — (Docs synced to `b492cc2`. §13's M5 row recorded the `Categorize` fix (#25) and its stale test count; the M6 row now says out loud that the "deploys blocked on the full-suite gate" half is **not** enforced — there is no CI in this repo (#27); the §13 task-record line stopped claiming the repo has no commits/remote and now records the tracker state: priority bands P0–P3 defined and every open issue labelled (#28, half done), with issue closure still not PR-linked. `docs/USAGE.md` lost a duplicated line and its "does nothing" summary was split into what reads today vs what still does not. `docs/check-prd.py` stops false-failing on §-refs into other docs — the cause of the long-standing dual-§-ref FAIL, whose refs pointed at `docs/JEV-INTEGRATION.md`, not this PRD.) +* 2026-09-21 — (**The `query` tool is real**: it reaches LeanKG `POST /api/v1/query` through `internal/leankg`, wired from `AGENTLOOP_LEANKG_URL`, and reports the answering retrieval rung; a LeanKG outage is a recorded failed step, and the `stepError` panic on a `Success=false`/nil-error tool result is fixed. **The approval table now names the real tools**: `query`/`web_search`/`run_tests` are read-category and `write_file` holds on every call — before this all four fell to the fail-closed default, so every gated run paused on step 1 and M5's <10% interruption ceiling was unreachable.) — §4 and §5.1 FR-7, `docs/USAGE.md`, README.* +* 2026-09-21 — (TypeSafe coupling risk re-verified: onegw `systemone` Kind is **merged** into master — `9faea01`; the open blocker is PR #110 verdict-driven combo reorder, not the merge. `docs/JEV-INTEGRATION.md` §3 rewritten to reflect the split between the merged Kind and the unmerged routing.) — §14 risk row corrected; `docs/JEV-INTEGRATION.md` §3.1/§3.4/§3.5 marked done, §7 checklist itemised.* +* 2026-09-21 — (System One abstraction: Laya as local Jev-compatible backend; `docs/JEV-INTEGRATION.md` rewritten; §3.1/§4.3/§14/§17/§16 updated; issues #8/#15 extended.) — cookbook deep-dive (function_calling, llm_guardrails, intent-routing) mapped to agentloop UCs. * v1.2.0 — M5 HITL (ApprovalGate, runner pause, timeout denies) + M6 (EvalRunner, 4-category suite, live HTTP API) shipped; §6 API contract reconciled to main.go routes; SSE event vocabulary narrowed to step + done; §13 milestones updated: M0–M4 closed, M5 closed on merge, M6 closed on merge, M7 conditional.* * v1.1.5 — UI design added (`docs/UI-DESIGN.md`): operator console design document from UI/UX Pro Max skill, one page per §12 surface, chart-type mapping per surface, cross-cutting UX rules, HTMX pattern table. No new PRD prose; M0 was documentation-only.* -* Last updated: 2026-09-21 (System One abstraction: Laya as local Jev-compatible backend; `docs/JEV-INTEGRATION.md` rewritten; §3.1/§4.3/§14/§17/§16 updated; issues #8/#15 extended.) — cookbook deep-dive (function_calling, llm_guardrails, intent-routing) mapped to agentloop UCs. diff --git a/docs/USAGE.md b/docs/USAGE.md index bb6649c..d8c30cb 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -107,7 +107,10 @@ Deployment facts, not compiled defaults: | `AGENTLOOP_XDEV_DIR` | a fresh temp dir | the workspace `write_file`/`run_tests` turns run in | | `AGENTLOOP_XDEV_OFF` | *(unset)* | any value disables the sandbox; those two tools then report no executor | | `AGENTLOOP_ONEGW_URL` | `http://127.0.0.1:8080` | gateway; when reachable, the model **chooses each step** | -| `AGENTLOOP_ONEGW_COMBO` | `dev` | combo used as the wire `model` for both step choice and synthesis | +| `AGENTLOOP_ONEGW_COMBO` | `dev` | default combo — the wire `model` when no tier-specific one is set | +| `AGENTLOOP_ONEGW_COMBO_PLANNING` | *(falls back to `COMBO`)* | combo for plan/replan steps | +| `AGENTLOOP_ONEGW_COMBO_EXECUTION` | *(falls back to `COMBO`)* | combo for action steps (the hot path) | +| `AGENTLOOP_ONEGW_COMBO_SYNTHESIS` | *(falls back to `COMBO`)* | combo for a bound-exit answer | Take the onegw key from `onegw.toml`'s `[auth] [[auth.keys]]`; the combo must exist there too, since the client sends whatever name you give it and onegw @@ -328,7 +331,13 @@ Stated plainly, so nobody discovers it the hard way: phases and instructions come from a rule table (`planStepCount` on the goal's word count), so the loop decides **what to do next** but not **how to break the goal up**. In practice the chooser carries the run, and the plan is a hint. -- **Tier routing is half-wired** — see §6. +- **Tier routing reaches the wire, but the 40–70% number is unproven.** The + loop sends a tier per call (`planning`/`execution`/`synthesis`) and each maps + to a combo; unmapped tiers fall through to `AGENTLOOP_ONEGW_COMBO`, so a + one-combo deployment runs unchanged. What has **not** been run is §11.4's + paired parity suite, which is the only thing that can say whether the routing + actually saves money without costing quality. Until then the saving is a + hypothesis with a working mechanism behind it. - **M7 is gated shut**, correctly: the gate is a measurement, not a milestone, and it opens only when a [PRD §10](PRD.md#10-multi-agent-stance) condition is actually met. diff --git a/internal/loop/m3_test.go b/internal/loop/m3_test.go index 33d2128..0ef3a68 100644 --- a/internal/loop/m3_test.go +++ b/internal/loop/m3_test.go @@ -123,8 +123,14 @@ func TestM3_PlannerDrivenRun(t *testing.T) { if len(result.Steps) == 0 { t.Fatal("run has no steps") } - // Tier should be set from the plan (planning or tiny default) - if result.CurrentTier != "planning" && result.CurrentTier != "tiny" { - t.Errorf("CurrentTier = %q, want planning or tiny", result.CurrentTier) + // The run reports a routing tier, and it is one of the three the loop + // actually routes on. The old assertion named `tiny`, a combo that + // does not exist in onegw and was therefore a tier nothing could route + // to — the exact drift this test now prevents. + switch result.CurrentTier { + case TierPlanning, TierExecution, TierSynthesis: + default: + t.Errorf("CurrentTier = %q, want one of %s/%s/%s", + result.CurrentTier, TierPlanning, TierExecution, TierSynthesis) } } diff --git a/internal/loop/model.go b/internal/loop/model.go index 6f73e27..9f35f42 100644 --- a/internal/loop/model.go +++ b/internal/loop/model.go @@ -9,11 +9,31 @@ import ( ) // ModelClient is the outbound model surface the loop needs — one call, -// given conversation messages. internal/onegw.Client satisfies it; tests -// use fakes. Kept deliberately this small: the loop must not grow a -// gateway's job (retries, key pools, fallback), only consume one. +// given conversation messages and the tier that should serve it. +// internal/onegw.Client satisfies it; tests use fakes. Kept deliberately +// this small: the loop must not grow a gateway's job (retries, key pools, +// fallback), only consume one. +// +// The tier parameter is not decoration. Until it existed, the loop +// computed a tier per step (M3) and never sent it, so "tiered routing" +// was a field on the run record and nothing else. type ModelClient interface { - Chat(ctx context.Context, msgs ...onegw.Message) (onegw.Reply, error) + ChatTier(ctx context.Context, tier string, msgs ...onegw.Message) (onegw.Reply, error) +} + +// Tierless adapts a client that only knows Chat, so a fake or an older +// implementation still satisfies ModelClient. Tests use it; production +// passes the real client, whose ChatTier routes. +type Tierless struct { + C interface { + Chat(ctx context.Context, msgs ...onegw.Message) (onegw.Reply, error) + } +} + +// ChatTier ignores the tier: this adapter exists precisely for clients +// that cannot route. +func (t Tierless) ChatTier(ctx context.Context, _ string, msgs ...onegw.Message) (onegw.Reply, error) { + return t.C.Chat(ctx, msgs...) } // synthesize replaces a bound-exit's deterministic partial with a real @@ -32,7 +52,9 @@ func (r *LoopRunner) synthesize(ctx context.Context, result *RunResult) error { } prompt := buildSynthesisPrompt(r.cfg.Goal, *result) - reply, err := r.model.Chat(ctx, + // Synthesis is its own step type: a run that hit a bound wants a + // different model than the one driving the loop (PRD move 3). + reply, err := r.model.ChatTier(ctx, TierSynthesis, onegw.Message{Role: "system", Content: synthesisSystemPrompt}, onegw.Message{Role: "user", Content: prompt}, ) diff --git a/internal/loop/model_test.go b/internal/loop/model_test.go index 89b7b7f..1e080b6 100644 --- a/internal/loop/model_test.go +++ b/internal/loop/model_test.go @@ -14,15 +14,17 @@ import ( // fakeModel records what it was asked and returns a canned reply. type fakeModel struct { - gotMsgs []onegw.Message - reply onegw.Reply - err error - calls int + gotMsgs []onegw.Message + gotTiers []string + reply onegw.Reply + err error + calls int } -func (f *fakeModel) Chat(_ context.Context, msgs ...onegw.Message) (onegw.Reply, error) { +func (f *fakeModel) ChatTier(_ context.Context, tier string, msgs ...onegw.Message) (onegw.Reply, error) { f.calls++ f.gotMsgs = msgs + f.gotTiers = append(f.gotTiers, tier) return f.reply, f.err } diff --git a/internal/loop/reason.go b/internal/loop/reason.go index af18544..be04dac 100644 --- a/internal/loop/reason.go +++ b/internal/loop/reason.go @@ -42,7 +42,7 @@ func (r *LoopRunner) choose(ctx context.Context, step int, result *RunResult) (S return StepChoice{}, fmt.Errorf("no tool registry") } - reply, err := r.model.Chat(ctx, + reply, err := r.model.ChatTier(ctx, r.tierForStepCombo(step), onegw.Message{Role: "system", Content: reasonSystemPrompt(reg.List())}, onegw.Message{Role: "user", Content: buildReasonPrompt(r, step, result)}, ) diff --git a/internal/loop/reason_test.go b/internal/loop/reason_test.go index 1e91ad3..3c0f431 100644 --- a/internal/loop/reason_test.go +++ b/internal/loop/reason_test.go @@ -22,9 +22,11 @@ type scriptedModel struct { errs []error calls int prompts []string + tiers []string } -func (m *scriptedModel) Chat(_ context.Context, msgs ...onegw.Message) (onegw.Reply, error) { +func (m *scriptedModel) ChatTier(_ context.Context, tier string, msgs ...onegw.Message) (onegw.Reply, error) { + m.tiers = append(m.tiers, tier) i := m.calls m.calls++ for _, msg := range msgs { @@ -351,3 +353,47 @@ func TestResumeReusesTheApprovedDecisionWithoutRecall(t *testing.T) { "(steps 1-3 can each need one call)", got) } } + +// The tier a step computes must reach the wire. It did not before: the +// loop calculated a tier per step (M3), stored it on the run record, and +// sent one fixed combo forever — so "tiered routing" was decoration. +func TestReasonerSendsTheStepsTier(t *testing.T) { + m := &scriptedModel{replies: []string{ + `{"tool":"query","args":{"query":"x"},"why":"a"}`, + `{"done":true,"why":"b"}`, + }} + r := reasonRunner(t, m, 3) + if _, err := r.Run(context.Background()); err != nil { + t.Fatalf("Run() error: %v", err) + } + if len(m.tiers) == 0 { + t.Fatal("no tier was sent to the model") + } + for i, tier := range m.tiers { + switch tier { + case TierPlanning, TierExecution, TierSynthesis: + default: + t.Errorf("call %d tier = %q, want one of the three routing tiers", i, tier) + } + } +} + +// Synthesis is its own step type, so a bound-exit answer may run on a +// different model than the loop that got stuck. +func TestSynthesisCarriesItsOwnTier(t *testing.T) { + m := &scriptedModel{} + r := reasonRunner(t, m, 1) + // max_steps=1 forces a bound exit, which synthesises. + if _, err := r.Run(context.Background()); err != nil { + t.Fatalf("Run() error: %v", err) + } + found := false + for _, tier := range m.tiers { + if tier == TierSynthesis { + found = true + } + } + if !found { + t.Errorf("tiers sent = %v, want the synthesis call to name %q", m.tiers, TierSynthesis) + } +} diff --git a/internal/loop/runner.go b/internal/loop/runner.go index 6369ade..b732244 100644 --- a/internal/loop/runner.go +++ b/internal/loop/runner.go @@ -251,21 +251,49 @@ func newRunner(cfg RunnerConfig, guard *budget.Guard, reg tools.ToolRegistry, // Kill closes the kill channel — the loop checks it at every // iteration boundary and exits state=killed within one step. +// Tier names this loop actually routes on. +// +// The PRD's §13.1 move 3 says "route models by step type (40–70%)", and +// the earlier code named three tiers — planning, execution, tiny — that +// were never sent to anything, and two of which (planning/execution) are +// not combos onegw ships. These are the three decisions the loop really +// makes, and each maps to whatever combo the operator configured: +// +// planning — the plan is being made or revised +// execution — a step's action is being chosen (the hot path) +// synthesis — a bound fired and the run needs an answer +const ( + TierPlanning = "planning" + TierExecution = "execution" + TierSynthesis = "synthesis" +) + +// tierForStep is the tier a step runs at. func (r *LoopRunner) tierForStep(step int, cfg RunnerConfig) string { if r.plan != nil && step < len(r.plan.Steps) { - return r.plan.Steps[step].Tier + if t := r.plan.Steps[step].Tier; t != "" { + return t + } } - return "planning" + return TierExecution +} + +// tierForStepCombo is what the reasoner passes to the gateway. +func (r *LoopRunner) tierForStepCombo(step int) string { + return tierForStepName(r.tierForStep(step, r.cfg)) } -func tierCombo(tier string) string { +// tierForStepName normalises a plan tier onto one of the three routing +// tiers. A plan that names something else (the old `tiny`) still routes — +// to execution — rather than silently falling off the table. +func tierForStepName(tier string) string { switch tier { - case "planning": - return "planning" - case "execution": - return "execution" + case TierPlanning: + return TierPlanning + case TierSynthesis: + return TierSynthesis default: - return "tiny" + return TierExecution } } @@ -408,7 +436,7 @@ func (r *LoopRunner) runLoop(ctx context.Context, result RunResult) (RunResult, result.State = StateActing // M3: use planner step tier for routing if r.plan != nil && step < len(r.plan.Steps) { - result.CurrentTier = tierCombo(r.plan.Steps[step].Tier) + result.CurrentTier = tierForStepName(r.plan.Steps[step].Tier) } // --- M9: choose the action --- diff --git a/internal/onegw/onegw.go b/internal/onegw/onegw.go index b8f78a5..aeb39ae 100644 --- a/internal/onegw/onegw.go +++ b/internal/onegw/onegw.go @@ -41,16 +41,30 @@ type Usage struct { // Reply is one completion. type Reply struct { Content string - Model string // the leg that actually answered, not the combo we asked for - Usage Usage + // Combo is the combo this call was routed to; Model is the leg that + // actually answered. They differ whenever onegw falls back, which is + // exactly the event tiering has to be judged on. + Combo string + Model string + Usage Usage } -// Client talks to one onegw instance with one combo bound to it. +// Client talks to one onegw instance. +// +// It holds a DEFAULT combo, not a single one: the loop routes per step +// (PRD §13.1 move 3 — "route models by step type"), so the combo has to be +// choosable per call. Sending one combo forever made tiering a decorative +// field on the run record. type Client struct { baseURL string key string combo string http *http.Client + + // combos maps a tier name to the combo that serves it. When a tier is + // absent the default combo is used, so an operator who has not set up + // several combos still gets a working loop rather than an error. + combos map[string]string } // New returns a Client. baseURL is onegw's root (e.g. @@ -65,17 +79,49 @@ func New(baseURL, key, combo string) *Client { } } -// Combo reports the combo this client routes through. +// Combo reports the default combo. func (c *Client) Combo() string { return c.combo } -// Chat sends the messages to the bound combo and returns its answer. +// WithTiers returns a copy of the client whose calls naming a tier route +// to that tier's combo. Nil or an empty map leaves the default in force. +// +// This is the whole of agentloop's tier routing: it picks the combo, onegw +// picks the leg behind it (§4.3, "agentloop sends the tier per step; onegw +// picks the leg"). +func (c *Client) WithTiers(tiers map[string]string) *Client { + out := *c + out.combos = tiers + return &out +} + +// comboFor resolves the combo for a tier, falling back to the default so a +// missing mapping degrades to "the loop still runs" rather than a failed +// step. +func (c *Client) comboFor(tier string) string { + if tier != "" && c.combos != nil { + if combo, ok := c.combos[tier]; ok && combo != "" { + return combo + } + } + return c.combo +} + +// Chat sends the messages to the default combo and returns its answer. // The caller's context bounds the call; there is no retry here, because // the loop's own wall-clock and budget bounds are the retry policy. func (c *Client) Chat(ctx context.Context, msgs ...Message) (Reply, error) { - if c.combo == "" { + return c.ChatTier(ctx, "", msgs...) +} + +// ChatTier is Chat, routed by tier. An unknown or empty tier uses the +// default combo, so a misconfigured tier costs the wrong model, not a +// failed run. +func (c *Client) ChatTier(ctx context.Context, tier string, msgs ...Message) (Reply, error) { + combo := c.comboFor(tier) + if combo == "" { return Reply{}, fmt.Errorf("onegw: no combo configured") } - body, err := json.Marshal(map[string]any{"model": c.combo, "messages": msgs}) + body, err := json.Marshal(map[string]any{"model": combo, "messages": msgs}) if err != nil { return Reply{}, fmt.Errorf("onegw: encode request: %w", err) } @@ -138,6 +184,9 @@ func (c *Client) Chat(ctx context.Context, msgs ...Message) (Reply, error) { return Reply{ Content: out.Choices[0].Message.Content, Model: out.Model, - Usage: out.Usage, + // Combo is what we ASKED for; Model is the leg that answered. + // Recording both is what makes "did tiering work?" answerable. + Combo: combo, + Usage: out.Usage, }, nil } diff --git a/internal/onegw/tier_test.go b/internal/onegw/tier_test.go new file mode 100644 index 0000000..19ee1bf --- /dev/null +++ b/internal/onegw/tier_test.go @@ -0,0 +1,125 @@ +package onegw + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" +) + +// The point of tiering: a call naming a tier goes to that tier's combo on +// the wire. Before this the combo was fixed at construction, so the loop's +// per-step tier reached nothing. +func TestChatTierRoutesToTheTierCombo(t *testing.T) { + var asked []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body struct { + Model string `json:"model"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + asked = append(asked, body.Model) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"model":"leg-that-answered","choices":[{"message":{"content":"ok"}}]}`)) + })) + defer srv.Close() + + c := New(srv.URL, "", "dev").WithTiers(map[string]string{ + "planning": "plan-combo", + "execution": "exec-combo", + }) + + if _, err := c.ChatTier(context.Background(), "execution", Message{Role: "user", Content: "a"}); err != nil { + t.Fatalf("ChatTier() error: %v", err) + } + if _, err := c.ChatTier(context.Background(), "planning", Message{Role: "user", Content: "b"}); err != nil { + t.Fatalf("ChatTier() error: %v", err) + } + // No tier named -> the default combo. + if _, err := c.Chat(context.Background(), Message{Role: "user", Content: "c"}); err != nil { + t.Fatalf("Chat() error: %v", err) + } + want := []string{"exec-combo", "plan-combo", "dev"} + if len(asked) != len(want) { + t.Fatalf("calls = %v, want %v", asked, want) + } + for i := range want { + if asked[i] != want[i] { + t.Errorf("call %d went to combo %q, want %q", i, asked[i], want[i]) + } + } +} + +// An unmapped tier must degrade to the default combo, not fail: a +// misconfigured tier should cost the wrong model, not the run. +func TestUnknownTierFallsBackToTheDefaultCombo(t *testing.T) { + var asked string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body struct { + Model string `json:"model"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + asked = body.Model + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"model":"x","choices":[{"message":{"content":"ok"}}]}`)) + })) + defer srv.Close() + + c := New(srv.URL, "", "dev").WithTiers(map[string]string{"planning": "plan-combo"}) + if _, err := c.ChatTier(context.Background(), "no-such-tier", Message{Role: "user", Content: "a"}); err != nil { + t.Fatalf("ChatTier() error: %v (an unmapped tier must not fail the call)", err) + } + if asked != "dev" { + t.Errorf("combo = %q, want the default %q", asked, "dev") + } +} + +// A client with no tiers set behaves exactly as before — one combo. +func TestNoTiersMeansTheDefaultComboOnly(t *testing.T) { + var asked []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body struct { + Model string `json:"model"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + asked = append(asked, body.Model) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"model":"x","choices":[{"message":{"content":"ok"}}]}`)) + })) + defer srv.Close() + + c := New(srv.URL, "", "harvey") + for _, tier := range []string{"planning", "execution", "synthesis", ""} { + if _, err := c.ChatTier(context.Background(), tier, Message{Role: "user", Content: "a"}); err != nil { + t.Fatalf("ChatTier(%q) error: %v", tier, err) + } + } + for i, got := range asked { + if got != "harvey" { + t.Errorf("call %d combo = %q, want harvey (no tiers configured)", i, got) + } + } +} + +// The reply records the combo ASKED FOR, separately from the leg that +// answered — observing a fallback is the only way to judge whether tiering +// is doing anything. +func TestReplyRecordsTheComboAskedFor(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"model":"deepseek-v4.1-flash","choices":[{"message":{"content":"ok"}}]}`)) + })) + defer srv.Close() + + c := New(srv.URL, "", "dev").WithTiers(map[string]string{"execution": "exec-combo"}) + got, err := c.ChatTier(context.Background(), "execution", Message{Role: "user", Content: "a"}) + if err != nil { + t.Fatalf("ChatTier() error: %v", err) + } + if got.Combo != "exec-combo" { + t.Errorf("Combo = %q, want the combo asked for", got.Combo) + } + if got.Model != "deepseek-v4.1-flash" { + t.Errorf("Model = %q, want the leg that answered", got.Model) + } +}