diff --git a/apps/mobile/.kilo/WORKFLOW_LEARNINGS.md b/apps/mobile/.kilo/WORKFLOW_LEARNINGS.md index 9f1ec49fd2..99a05e37d5 100644 --- a/apps/mobile/.kilo/WORKFLOW_LEARNINGS.md +++ b/apps/mobile/.kilo/WORKFLOW_LEARNINGS.md @@ -4,4 +4,43 @@ Environment blockers and their fixes, recorded by the planner or orchestrator fo ## Planner +### Dispatching role agents from a non-kilo harness (tmux, exit codes, void rounds) + +**Symptom.** A `kilo run --agent ` dispatched from a harness whose Bash tool has a 10-minute timeout gets killed mid-review. Worse, a run that is piped (`kilo run ... | tee log`) records the exit status of `tee`, not of kilo, so a crashed agent reports `EXITCODE=0` and reads as a clean pass. + +**Cause.** Two independent traps: the harness command timeout, and `$?` after a pipeline referring to the last stage. + +**Fix.** Run the agent inside a tmux window from a small wrapper script, redirect rather than pipe, and append the exit code of the redirected command: + +```bash +cd "$WORKTREE/apps/mobile" # .kilo/agent/ must be discoverable from the cwd +kilo run "$(cat msg.txt)" --model kilo/x-ai/grok-4.5 --variant high \ + --agent mobile-plan-reviewer --file "$PLAN" > "$LOG" 2>&1 +echo "EXITCODE=$?" >> "$LOG" +``` + +Then wait event-driven with an `until grep -q EXITCODE= "$LOG"` loop that also breaks when the tmux session disappears. Keep the message positional **before** the flags: `--file` takes multiple values and swallows a trailing message as a path. + +### A kilo role agent can exit mid-run with no verdict — treat it as a void round + +**Symptom.** The agent's log ends on an ordinary progress line ("Checking how decider scores are assigned…"), the tmux window is gone, and no findings list was ever printed. With a piped exit code this is indistinguishable from a pass. + +**Cause.** Long kilo runs die on provider stream stalls, typically 10–15 minutes in. Nothing about the plan or the repository is wrong. + +**Fix.** A round that produced no explicit verdict line is **void, never a pass**. Re-dispatch a fresh agent — the review gate wants a fresh session per round anyway, so nothing is lost. Detect it by requiring the verdict text itself (`No findings.` or a numbered list), not by exit code. If several consecutive rounds die at the same point, shrink the handoff rather than retrying unchanged. + ## Orchestrator + +### Waiting on the EXITCODE marker false-triggers mid-run + +**Symptom.** An `until grep -q EXITCODE= "$LOG"` wait loop (Planner section, first entry) reports the role agent finished while it is still running: the string `EXITCODE=` already appears in the log because the agent read `WORKFLOW_LEARNINGS.md` or a handoff that documents the pattern, and the TUI echoes it into the capture. + +**Cause.** The wait pattern greps for a marker that is no longer unique to the wrapper's final append. + +**Fix.** Treat the run as done only when the tmux session is gone **or** the marker is the last line of the log (`tail -1 "$LOG" | grep -q '^EXITCODE=[0-9]'`). The plain `grep -q EXITCODE=` form is only safe if neither the handoff nor anything the agent is likely to read mentions the pattern — which this file does, so prefer the last-line check. + +### Reading Kilobot's no-findings state (post #4765) + +**Symptom.** The completion gate wants "Kilobot has reviewed the latest head", but the review no longer arrives as inline threads: with the bot skip/permit config (#4765) on main, a clean review produces a green `Kilo Code Review` check plus exactly one issue comment from `kilo-code-bot[bot]` headed `Status: No Issues Found | Recommendation: Merge`. + +**Fix.** That combination — green check on the current head, the no-issues summary comment, zero review threads (`gh api repos/.../pulls//comments` empty) — *is* the reviewed-with-no-findings state. There is nothing to reply to or resolve; the gate is met. A `BLOCKED`/`REVIEW_REQUIRED` merge state at that point only means the requested human review is pending. diff --git a/services/auto-routing/src/decision-engine.test.ts b/services/auto-routing/src/decision-engine.test.ts index ae51d84c89..a1f1581f66 100644 --- a/services/auto-routing/src/decision-engine.test.ts +++ b/services/auto-routing/src/decision-engine.test.ts @@ -316,6 +316,212 @@ describe('computeDecision', () => { }); }); + describe('benchmark noise band', () => { + // Every fixture below is taken verbatim from the live routing table + // (decider-2026-07-22T10-51-06-305Z, minAccuracy 0.95): accuracies are the + // published k/30 roundings and costs the published avgCostUsd. In + // cost_per_accuracy mode the fresh pick is candidates[0] after filtering, + // so each route array lists the named fresh pick first. + const liveClassification = ( + taskType: ClassifierOutput['taskType'], + subtaskType: ClassifierOutput['subtaskType'] + ): ClassifierOutput => ({ ...classification, taskType, subtaskType }); + + it('keeps the incumbent on a zero-passer route when it sits inside the band', () => { + // investigation/codebase_understanding has no threshold-meeting + // candidate. kimi-k2.7-code is less accurate AND ~1.16x more expensive + // per benchmark case than the fresh pick, yet it is kept: 0.8667 clears + // the 0.85 band floor, and 0.01094 * 3 is not less than 0.01272, so the + // cost condition does not fire either. + const zeroPasserTable: RoutingTable = { + ...table, + minAccuracy: 0.95, + routes: { + 'investigation/codebase_understanding': [ + { + model: 'moonshotai/kimi-k3', + accuracy: 0.9333, + avgCostUsd: 0.01094, + meetsThreshold: false, + }, + { + model: 'moonshotai/kimi-k2.7-code', + accuracy: 0.8667, + avgCostUsd: 0.01272, + meetsThreshold: false, + }, + ], + }, + }; + const decision = computeDecision( + liveClassification('investigation', 'codebase_understanding'), + zeroPasserTable, + 'moonshotai/kimi-k2.7-code' + ); + expect(decision).toMatchObject({ + model: 'moonshotai/kimi-k2.7-code', + sticky: true, + switchReason: null, + }); + }); + + it('keeps an in-band incumbent when the route’s sole passer is far more expensive', () => { + // planning_design/technical_planning has exactly one passer, + // claude-sonnet-5 at $0.0394/case. Keeping inkling (0.9333, $0.0061) + // gives up +0.033 accuracy — one graded case out of thirty — and the + // cost escape cannot help here: 0.0394 * 3 = 0.1182 is not less than + // 0.0061, so the band is the only thing preventing the switch onto a + // model 6.5x more expensive. + const solePasserTable: RoutingTable = { + ...table, + minAccuracy: 0.95, + routes: { + 'planning_design/technical_planning': [ + { + model: 'anthropic/claude-sonnet-5', + accuracy: 0.9667, + avgCostUsd: 0.0394, + meetsThreshold: true, + }, + { + model: 'thinkingmachines/inkling', + accuracy: 0.9333, + avgCostUsd: 0.0061, + meetsThreshold: false, + }, + ], + }, + }; + const decision = computeDecision( + liveClassification('planning_design', 'technical_planning'), + solePasserTable, + 'thinkingmachines/inkling' + ); + expect(decision).toMatchObject({ + model: 'thinkingmachines/inkling', + sticky: true, + switchReason: null, + }); + }); + + it('keeps a below-bar incumbent over a threshold-clearing fresh pick when the gap is 2/30 graded cases', () => { + // debugging/root_cause_analysis, the modal live case: the fresh pick + // clears the bar and costs the same to four decimal places + // ($0.00274548 vs $0.00274055), so the entire benefit of keeping + // kimi-k2.7-code is prompt-cache continuity. At n=10 distinct graded + // cases, 27/30 vs 29/30 is a difference the benchmark cannot resolve. + const modalTable: RoutingTable = { + ...table, + minAccuracy: 0.95, + routes: { + 'debugging/root_cause_analysis': [ + { + model: 'minimax/minimax-m3', + accuracy: 0.9667, + avgCostUsd: 0.00275, + meetsThreshold: true, + }, + { + model: 'moonshotai/kimi-k2.7-code', + accuracy: 0.9, + avgCostUsd: 0.00274, + meetsThreshold: false, + }, + ], + }, + }; + const decision = computeDecision( + liveClassification('debugging', 'root_cause_analysis'), + modalTable, + 'moonshotai/kimi-k2.7-code' + ); + expect(decision).toMatchObject({ + model: 'moonshotai/kimi-k2.7-code', + sticky: true, + switchReason: null, + }); + }); + + it("labels an in-band incumbent's cost-driven ejection as switchReason 'cost', not 'threshold'", () => { + // planning_design/architecture_design: the incumbent is inside the + // band, so the old code would have ejected it as 'threshold'; the band + // makes it eligible, and the ejection is then caused by the unchanged + // cost condition: 0.00660620 * 3 = 0.01981860 < 0.03943792. Telemetry + // must therefore read 'cost' — otherwise the before/after measurement + // of this change reads its own relabels backwards. + const relabelTable: RoutingTable = { + ...table, + minAccuracy: 0.95, + routes: { + 'planning_design/architecture_design': [ + { + model: 'thinkingmachines/inkling', + accuracy: 0.9, + avgCostUsd: 0.0066062, + meetsThreshold: false, + }, + { + model: 'anthropic/claude-sonnet-5', + accuracy: 0.9, + avgCostUsd: 0.03943792, + meetsThreshold: false, + }, + ], + }, + }; + const decision = computeDecision( + liveClassification('planning_design', 'architecture_design'), + relabelTable, + 'anthropic/claude-sonnet-5' + ); + expect(decision).toMatchObject({ + model: 'thinkingmachines/inkling', + sticky: false, + switchReason: 'cost', + }); + }); + + it('best_accuracy mode still ejects an in-band incumbent below the threshold', () => { + // Same route and incumbent as the sole-passer keep case above, with + // the mode flipped and the outcome inverted: the accuracy gap is + // 0.0334, below the 0.05 bestAccuracySwitchThreshold, so the gap + // condition alone would keep inkling — only best_accuracy still + // requiring meetsThreshold can eject it. + const solePasserTable: RoutingTable = { + ...table, + minAccuracy: 0.95, + routes: { + 'planning_design/technical_planning': [ + { + model: 'anthropic/claude-sonnet-5', + accuracy: 0.9667, + avgCostUsd: 0.0394, + meetsThreshold: true, + }, + { + model: 'thinkingmachines/inkling', + accuracy: 0.9333, + avgCostUsd: 0.0061, + meetsThreshold: false, + }, + ], + }, + }; + const decision = computeDecision( + liveClassification('planning_design', 'technical_planning'), + solePasserTable, + 'thinkingmachines/inkling', + new Set(), + 'best_accuracy' + ); + expect(decision).toMatchObject({ + model: 'anthropic/claude-sonnet-5', + sticky: false, + switchReason: 'threshold', + }); + }); + }); + describe('capability filters', () => { const visionTable: RoutingTable = { ...table, diff --git a/services/auto-routing/src/decision-engine.ts b/services/auto-routing/src/decision-engine.ts index 847c6d3d01..1cea4834a6 100644 --- a/services/auto-routing/src/decision-engine.ts +++ b/services/auto-routing/src/decision-engine.ts @@ -150,6 +150,23 @@ function applyCapabilityFilters( return { filtered: maxContextFallback, reason: 'ok' }; } +// A route's accuracy is graded on 10 distinct benchmark cases +// (datasets/decider-cases.ts, >=10 per taxonomy route; repetitions re-run the +// same prompts and add no independent tasks). At n=10 the one-sided 95% Wilson +// upper bound only drops below a 0.95 bar at an observed accuracy of ~0.837, so +// pass/fail at 0.95 flips on binomial noise — and each flip ejects every session +// parked on that model, paying a full prompt-cache rebuild for a difference the +// benchmark cannot resolve. Keep a cost-mode incumbent inside the band instead. +// This is a fixed band, not a per-route interval: replace it with a published +// Wilson bound once the routing table carries per-route case counts. 0.10 is +// deliberately inside the [0.0833, 0.1167) plateau where behaviour is identical +// on the current table, and stricter than the n=10 boundary (~0.113), so it never +// keeps a model a real interval would eject. Do not nudge it toward either edge: +// published accuracies are toFixed(4) roundings of k/30, so 0.083 rounds the floor +// above 26/30 and silently drops that entire tier of retained incumbents (not +// the single largest group, which sits at 27/30 and is unaffected). +const STICKY_ACCURACY_TOLERANCE = 0.1; + export function computeDecision( classification: ClassifierOutput, table: RoutingTable | null, @@ -190,9 +207,17 @@ export function computeDecision( // by a fresh pick from the eligible set, not kept. const incumbent = incumbentModel === null ? undefined : candidates.find(c => c.model === incumbentModel); + // Sticky eligibility, shared by the keep decision and the switchReason + // telemetry below so the two can never disagree. best_accuracy keeps the + // strict bar (that mode exists to buy accuracy); cost_per_accuracy keeps + // any incumbent inside the benchmark noise band. + const incumbentStickyEligible = (candidate: RankedCandidate): boolean => + mode === 'best_accuracy' + ? candidate.meetsThreshold + : candidate.accuracy >= table.minAccuracy - STICKY_ACCURACY_TOLERANCE; const stickyIncumbent = incumbent && - incumbent.meetsThreshold && + incumbentStickyEligible(incumbent) && incumbent.model !== freshPick.model && ((mode === 'cost_per_accuracy' && !(freshPick.avgCostUsd * table.switchCostFactor < incumbent.avgCostUsd)) || @@ -224,11 +249,11 @@ export function computeDecision( // 'cost': the incumbent was eligible but the mode's switch condition // (cost factor / accuracy gap) made the fresh pick worth it; // 'capability': the modality/context filters ejected it from the route; - // 'threshold': it is denied, off the route, or below the accuracy bar. + // 'threshold': it is denied, off the route, or outside the accuracy band. switchReason: !switched ? null : incumbent - ? incumbent.meetsThreshold + ? incumbentStickyEligible(incumbent) ? 'cost' : 'threshold' : routeCandidates.some(c => c.model === incumbentModel)