Skip to content

fix(auto-routing): address custom router review follow-ups - #8084

Merged
iscekic merged 3 commits into
mainfrom
fix/custom-router-review-followups
Oct 19, 2026
Merged

iscekic merged 3 commits into
mainfrom
fix/custom-router-review-followups

Conversation

@iscekic

@iscekic iscekic commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator

Follow-up for maintainer review remarks left open on #7784, #7791 and #7793 after they merged.

Remarks and resolutions

#7784: decision-token lifetime is not enforced (thread)
Fixed. verifyAutoDecision now rejects a token whose expiry is later than AUTO_DECISION_TTL_MS plus a small clock-skew margin; the signer and the verifier run on different hosts, so the margin is needed. Test: a token signed with a longer expiry is rejected, and a full-lifetime token is still accepted.

#7784: fallback: true with classifier fields (thread)
The contract comment was wrong. The worker sets fallback: true when no routing decision names a servable entry, and that includes the case where the classifier succeeded but the table has no servable pick. taskType/subtaskType then report that classification, and decision logs and the CLI use it. I corrected the field comment and did not add a refinement. Test: the schema accepts a fallback decision that carries a classification.

#7791: fallback can pick an entry that cannot serve the request (thread)
Fixed. DecisionRun now carries the request constraints and the capability map. Fallback picks only a servable entry that is not known to be inactive, has every enforced required modality, and does not have a context window that is provably too small. If no entry qualifies, it returns custom_router_unavailable. Tests: a default entry without image support is skipped, and a request after a classifier failure with no compatible entry returns 422.

#7791: public request bodies are buffered without a size limit (thread)
Fixed. Public routes read the body through readBoundedStream and stop at the first byte over the limit, so Content-Length is not trusted. An oversized body returns invalid_request. The limit matches the largest upstream model request size, because a decide request carries the whole conversation. Test: a body exactly at the limit is routed, and one byte more is rejected.

#7793: concurrent admissions can exceed the per-router quota (thread)
Fixed. A charged admission batch now ends with a guard statement. The guard recounts the rolling events inside the same D1 transaction and fails the batch when that batch's own events go over the limit, so the whole admission rolls back. The caller maps that rollback to the normal quota error. Test (real SQLite): two concurrent requests at the boundary admit exactly one model.

#7793: billing failure followed by a failed compensating delete leaves an unbilled result (thread)
Fixed. A graded result now stores its grader usage record in client_case_results.pending_grader_usage (new nullable column, generated migration). The column is cleared only after the usage writer acknowledges the record. A retry that finds a stored result with a pending record sends the same record again (same usage ID, same payer) before it returns the score. The existing delete on billing failure is unchanged. Test: billing fails, the delete fails, then a retry records the identical usage once, and later retries do not charge again.

Verification

On a disposable VM: tsgo typecheck for packages/auto-routing-contracts, services/auto-routing, services/auto-routing-benchmark and packages/web-shared; oxlint and oxfmt on the changed sources; full vitest suites for the three auto-routing packages (all passed). Each new regression test failed against the main sources and passed with this change.

Cap decision-token lifetime at verification, document fallback classification fields, check fallback entries against request capabilities, bound public request bodies, enforce the profile admission quota inside the admission batch, and resume unacknowledged grader usage on a retried client eval upload.
@kilo-code-bot

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

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change resolves the previously reported reliability gap: unreadable pending grader usage no longer wedges retries, and the added test covers the drop-and-regrade path.

Files Reviewed (2 files)

  • services/auto-routing-benchmark/src/client-eval.ts
  • services/auto-routing-benchmark/src/client-eval.test.ts
Previous Review Summary (commit 5dd7335)

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

Previous review (commit 5dd7335)

Status: 1 Issues Found | Recommendation: Address before merge

Executive Summary

One low-severity reliability gap in the client-eval grader-usage retry path: a stored pending usage record that cannot be parsed permanently blocks retries for that case.

Overview

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

SUGGESTION

File Line Issue
services/auto-routing-benchmark/src/client-eval.ts 216 Unguarded parse of the persisted pending grader usage can permanently wedge retries
Files Reviewed (16 files)
  • packages/auto-routing-contracts/src/custom-router.test.ts
  • packages/auto-routing-contracts/src/custom-router.ts
  • services/auto-routing-benchmark/migrations/0025_complete_triton.sql - generated
  • services/auto-routing-benchmark/migrations/meta/0025_snapshot.json - generated
  • services/auto-routing-benchmark/migrations/meta/_journal.json - generated
  • services/auto-routing-benchmark/src/client-eval.test.ts
  • services/auto-routing-benchmark/src/client-eval.ts - 1 issue
  • services/auto-routing-benchmark/src/client-profiles.ts
  • services/auto-routing-benchmark/src/db-schema.ts
  • services/auto-routing-benchmark/src/profile-quota-sql.test.ts
  • services/auto-routing-benchmark/src/profiles.test.ts
  • services/auto-routing-benchmark/src/profiles.ts
  • services/auto-routing/src/custom-router-decision.ts
  • services/auto-routing/src/custom-router.test.ts
  • services/auto-routing/src/decide.ts
  • services/auto-routing/src/public-routes.ts

Fix these issues in Kilo Cloud


Reviewed by deepseek-v4.1-flash-1017 · Input: 35.4K · Output: 5.6K · Cached: 290.8K

Review guidance: REVIEW.md from base branch main

…iew-followups

# Conflicts:
#	services/auto-routing-benchmark/migrations/meta/0025_snapshot.json
#	services/auto-routing-benchmark/migrations/meta/_journal.json
#	services/auto-routing-benchmark/src/profiles.ts
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