Skip to content

perf(auto-routing): classify turn 1 in parallel with the conversation read - #7511

Merged
iscekic merged 12 commits into
igor/auto-efficient-retunefrom
igor/auto-efficient-speculative
Oct 14, 2026
Merged

iscekic merged 12 commits into
igor/auto-efficient-retunefrom
igor/auto-efficient-speculative

Conversation

@iscekic

@iscekic iscekic commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator

bot: Classify turn 1 of a kilo-auto/efficient conversation in parallel with the conversation read, rather than after it. This PR is stacked on #7510. Merge #7492, #7493, #7496, #7504 and #7510 first.

Why

  • After perf(auto-routing): answer turn 2+ from the sticky model and classify in the background #7493, turn 2+ answers from the sticky model and classifies in the background. Turn 1 has no sticky model, so it still waits for the Durable Object read and then the Jev classifier, in series.
  • In production, turn-1 requests (messageCount ≤ 2) are a small share of /decide, but their p50 is several times that of later turns. The first read of a new conversation's Durable Object is the cold part.

Change

  • On a request with messageCount ≤ 2 (or unknown) that has no classification in the isolate hot cache, the classifier starts at once, in parallel with readConversationState.
  • DO miss with no sticky model: the speculative result is used like the synchronous path. The log reads classification: 'speculative'.
  • DO cache hit: the cached classification is used. The speculative call was paid, so its cost is billed once: on this response if it already resolved, otherwise through addPendingClassifierCost on the next turn.
  • DO sticky model: the sticky-immediate path from perf(auto-routing): answer turn 2+ from the sticky model and classify in the background #7493 runs, and the in-flight call becomes its background classification. No second call is made.
  • A failure is handled exactly like a synchronous classifier failure, including a billed ClassifierRunError.
  • Requests with more than 2 messages are unchanged.

Cost

  • The extra calls happen only on turn-1 cache hits, which are mostly client retries. Each one costs one classifier call and is billed to the user, so per-request cost moves by well under 1%.

Verification

  • VM: auto-routing typecheck passes, 273 tests pass, and lint is clean.
  • New tests:
    • A turn-1 miss responds after the slower of the read and the classifier, not their sum (deferred promises).
    • A cache hit bills the speculative cost once, in both settlement orders.
    • A billed speculative failure is billed once.
    • The sticky path reuses the in-flight call.
    • Requests with more than 2 messages and hot-cache hits do not speculate.
  • VM replay against the real worker and the real Jev classifier: 108 requests, 0 errors. The speculative path ran 36 times, and billed cost per scenario is unchanged. A local Durable Object read takes about a millisecond, so the latency gain shows only in production timings after deploy.

Comment thread services/auto-routing/src/decide.ts Outdated
@kilo-code-bot

kilo-code-bot Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental re-review of the turn-1 speculative classification change finds no new issues in the PR's changed code: the only edits since the previous review are test-only refactors and a formatting change to the waitUntil rejection handler, and the alarm-rescheduling code they exercise was already present on the base branch rather than introduced here.

Notes

  • decide.ts: the preparedClassification waitUntil keeps the same swallow-and-continue rejection handler as the base (now formatted across lines); the speculative promise is started in parallel with the conversation read and every path (miss, sticky-immediate, cache hit, error) still observes it.
  • classifier-usage.test.ts, decision-cache.test.ts, index.test.ts: changes are test-only — extracting idFromName/get mocks into named locals and narrowing init.body before JSON.parse, with no production behavior affected.
  • decision-cache.ts: the only PR change versus base is the getHotCachedClassification extraction; the modified scheduleAlarm/alarm() rescheduling is already present on igor/auto-efficient-retune, so it is out of scope for this PR.
Files Reviewed (6 files)
  • services/auto-routing/src/decide.ts
  • services/auto-routing/src/decision-cache.ts
  • services/auto-routing/src/classifier-usage.ts
  • services/auto-routing/src/classifier-usage.test.ts
  • services/auto-routing/src/decision-cache.test.ts
  • services/auto-routing/src/index.test.ts
Previous Review Summaries (3 snapshots, latest commit 6db9c08)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 6db9c08)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental re-review of the turn-1 speculative classification change finds no new issues: the speculative classifier starts in parallel with the conversation read, its outcome is always observed and reused across the sticky-immediate, cache-hit, and miss paths, a late cache-hit settlement still reports the paid speculative cost, and the classifier usage organization mapping now defaults a missing organization to null, matching the gateway's usage writer.

Notes

  • decide.ts: the speculative promise is wrapped so speculative.outcome is captured, handed to executionCtx.waitUntil, and awaited on the sticky-immediate and miss paths; the sticky background path reuses the in-flight call instead of issuing a second paid call, and the cache-hit path records the deferred cost once the promise settles. Each request still calls the classifier exactly once, so no double billing.
  • decision-cache.ts: extracting getHotCachedClassification is behavior-preserving relative to the previous inline hot-cache check, and the speculation guard uses the same key/TTL as the DO read.
  • classifier-usage.ts: organization_id: payload.organizationId ?? null matches llm-proxy.ts:1035 (organization_id: organizationId || null) for personal payers, and the new ZodError handling logs only the invalid field path without rejecting usage recording.
Files Reviewed (5 files)
  • services/auto-routing/src/decide.ts
  • services/auto-routing/src/decision-cache.ts
  • services/auto-routing/src/classifier-usage.ts
  • services/auto-routing/src/classifier-usage.test.ts
  • services/auto-routing/src/index.test.ts

Previous review (commit c99c445)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental re-review of the turn-1 speculative classification change confirms the previous telemetry WARNING is resolved: a speculative classifier cost is now written to classifierCostCredits and to the Analytics Engine data point on the response, a call that settles after the response is reported via deferredClassifierCostCredits at settlement, and paid speculation bypasses sampling so billing and telemetry reconcile. The speculative promise is always observed (response, deferred waitUntil, or sticky-background reuse) so no billed cost is dropped, and the classifier promise never rejects.

Files Reviewed (3 files)
  • services/auto-routing/src/decide.ts
  • services/auto-routing/src/decision-cache.ts
  • services/auto-routing/src/index.test.ts

Previous review (commit e9fc180)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The turn-1 speculative classification logic is sound — one classifier call, billed exactly once, and correctly reused for the sticky-immediate/cache paths; the only gap is that speculative cost and failures on a cache hit are charged without appearing in decision telemetry.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
services/auto-routing/src/decide.ts 706 Speculative classifier cost is added to the response but recordDecision is still called with the cache_hit outcome, so classifierCostCredits (double2) and the auto_routing_decision log report 0; a speculative failure on a cache hit is billed with no log or metric.
Files Reviewed (3 files)
  • services/auto-routing/src/decide.ts - 1 issue
  • services/auto-routing/src/decision-cache.ts - 0 issues
  • services/auto-routing/src/index.test.ts - 0 issues

Fix these issues in Kilo Cloud


Reviewed by deepseek-v4.1-flash · Input: 90K · Output: 36.8K · Cached: 1.4M

Review guidance: REVIEW.md from base branch igor/auto-efficient-retune

@iscekic
iscekic added this pull request to stack #7494 October 14, 2026 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants