Skip to content

feat(route): version the /v1/route contract and ship a Python SDK - #858

Open
steventohme wants to merge 6 commits into
mainfrom
route-sdk-contract
Open

feat(route): version the /v1/route contract and ship a Python SDK#858
steventohme wants to merge 6 commits into
mainfrom
route-sdk-contract

Conversation

@steventohme

Copy link
Copy Markdown
Collaborator

Make the route-decision endpoints a supported public surface so callers can ask "which model would you pick?" without standing up a proxy path.

  • Add schema_version: router_route_v1 to the POST /v1/route response. It was a bare {model, provider, reason} with no forward-compat handle.
  • Drop the PolicyHeaderOverridesEnabled gate on /v1/route/preview. Preview is read-only (no credentials, no mutation), so the rk_ bearer token + HMM-strategy requirement are sufficient gates. The HMM check stays because the response is HMM-shaped.
  • Document both endpoints in docs/ROUTE_DECISION_API.md as a stable contract: auth, request-shaping headers, status codes, and versioning.
  • Ship clients/python/ — a typed httpx client (sync + async) with header shaping, a schema-version guard, and status-to-typed-error mapping. It never caches or retries (decisions are per-request by construction; a 502 means the strategy is unhealthy).
  • Add a CI job for the Python client so the contract stays enforced.
  • Lay out the clients/ boundary in CLAUDE.md / AGENTS.md — other-language SDKs sit outside the Go layer model, consuming only the documented HTTP contract.

No consumer of either endpoint existed outside the router repo, so the preview gate change breaks nothing.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

Make the route-decision endpoints a supported public surface so callers can
ask "which model would you pick?" without standing up a proxy path.

- Add `schema_version: router_route_v1` to the POST /v1/route response. It
  was a bare {model, provider, reason} with no forward-compat handle, so a
  client had no way to detect a breaking shape change.
- Drop the PolicyHeaderOverridesEnabled gate on /v1/route/preview. Preview is
  read-only, exposes no credentials, and mutates nothing; the rk_ bearer token
  plus the HMM-strategy requirement are the appropriate gates. The
  HMM-strategy check stays because the response is HMM-shaped.
- Document both endpoints in docs/ROUTE_DECISION_API.md as a stable contract:
  auth, request-shaping headers, status codes, and versioning.
- Add clients/python, a typed httpx client (sync + async) with header shaping,
  a schema-version guard, and status-to-typed-error mapping. It never caches
  or retries: a decision is per-request by construction, and a 502 means the
  strategy is unhealthy rather than transiently unlucky.

clients/ sits outside the Go layer model — other-language SDKs consuming the
public HTTP contract as a black box — which CLAUDE.md/AGENTS.md now state.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 30, 2026

Copy link
Copy Markdown

Claude finished @steventohme's task —— View job


Posted an advisory comment-length review on internal/api/anthropic/route.go with two suggestions:

  • Lines 16–19 (RouteSchemaVersionV1 constant): 4-line comment restates semver convention; suggested 2-line replacement.
  • Lines 63–66 (PreviewRouteHandler godoc): 4 padded lines; suggested 3 tight lines preserving all non-obvious WHY (rk_ token, HMM shape, sidecar).

Comment thread clients/python/weave_router_client/client.py Outdated

@workweave-bot workweave-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Advisory only — comment-length nits. Won't block merge.

Comment thread internal/api/anthropic/route.go Outdated
Comment thread internal/api/anthropic/route.go Outdated
Comment thread clients/python/weave_router_client/client.py Outdated
Comment thread internal/api/anthropic/route.go
@greptile-apps

greptile-apps Bot commented Jul 30, 2026

Copy link
Copy Markdown

Security Review

The preview endpoint exposes HMM policy internals to authenticated installations that do not have PolicyHeaderOverridesEnabled when their persisted routing strategy, or the deployment default, resolves to HMM. Reproduced responses include the policy artifact hash, HMM state, class probabilities, and eligible roster identifiers. This authorization regression should be fixed before merge.

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex produced proofs for posted P1 findings, covering multiple scenarios.
  • T-Rex validated that RouteOptions terse is rejected by the live router middleware, while a boolean true is accepted.
  • T-Rex validated the Preview permission flow before and after the PR, reproducing disabled-permission traces after the PR.
  • T-Rex documented the exact client and server contract locations used in the validation.
  • T-Rex validated the after-change contract behavior, showing that the endpoint now returns HTTP 200 with policy data after the change.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (2)

  1. General comment

    P1 RouteOptions exposes invalid per-model verbosity values that the router rejects

    • Bug
      • RouteOptions(per_model_verbosity="terse") is accepted by the Python model and transmitted unchanged, but the router middleware rejects it with HTTP 400. RouteClient maps that response to InvalidRequestError.
    • Cause
      • The client declares per_model_verbosity as str | None and directly assigns it to the request header, whereas the server parses the header strictly as the literals true or false.
    • Fix
      • Change the client field to bool | None and serialize it with str(value).lower(), consistent with embed_only_user_message; update the existing client test that currently uses and expects "terse".

    T-Rex Ran code and verified through T-Rex

  2. General comment

    P1 Preview endpoint exposes HMM policy traces to installations without policy-header permission

    • Bug
      • An authenticated installation with PolicyHeaderOverridesEnabled=false can obtain a complete HMM policy preview whenever its persisted strategy is HMM or the deployment default resolves to HMM. The response includes policy artifact SHA, HMM state, class probabilities, and eligible roster IDs.
    • Cause
    • Fix
      • Restore an authorization check before strategy evaluation: retrieve the authenticated installation and return HTTP 403 permission_error unless installation != nil && installation.PolicyHeaderOverridesEnabled. Add endpoint tests for disabled permission with both persisted HMM and default HMM.

    T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "feat(route): version the /v1/route contr..." | Re-trigger Greptile

Fixed:
- per_model_verbosity was typed as a free-form string in the Python client,
  but the router's x-weave-routing-per-model-verbosity middleware only
  accepts the literal true/false and maps to *bool. Any other value
  (the test used "terse") gets a 400. Retype as bool | None, serialize with
  str(value).lower(), matching embed_only_user_message. (cursor, greptile-apps)

Applied from workweave-bot review (comment-length nits, verbatim):
- RouteSchemaVersionV1 doc: 4 lines -> 2, dropping the semver restatement.
- PreviewRouteHandler godoc: 4 padded lines -> 3 tight lines, same content.

Declined (with explanation, replied on thread):
- greptile-apps flagged dropping PolicyHeaderOverridesEnabled on
  PreviewRouteHandler as a security regression. This is the PR's intended
  change (per the PR description): preview is read-only, exposes no
  credentials, and the roster IDs it returns are already public via the
  unauthenticated GET /v1/router/hmm-roster. The rk_ bearer token + HMM-
  strategy requirement are the appropriate gates here.
@claude

claude Bot commented Jul 30, 2026

Copy link
Copy Markdown

Claude finished @steventohme's task —— View job


No comment-length issues found in the updated Go diff — the previous nits were addressed and the remaining blocks are within bounds.

View job

Comment thread clients/python/weave_router_client/client.py
PreviewGroup and PreviewDiagnostic lacked extra="allow", so additive server
fields on those nested objects were dropped rather than kept in model_extra —
breaking the additive-compatibility promise the README makes, which
RoutePreview and PreviewCandidate already honored. (cursor)

Adds a test asserting model_extra survives on all three nested preview models.
@claude

claude Bot commented Jul 30, 2026

Copy link
Copy Markdown

Claude finished @steventohme's task —— View job


No comment-length issues found in the current Go diff — the two added blocks are within bounds and explain genuine non-obvious constraints.

View job

Comment thread docs/ROUTE_DECISION_API.md Outdated
…teration 3)

The preview route group was an incomplete copy of the route group's
request-shaping chain: it omitted WithForceEffortOverride and
WithClusterVersionOverride. ForceEffort feeds policy arm hashing
(policy/arm.go), so preview could silently return a decision trace for a
different arm than /v1/route serves for identical headers — while the
documented contract and both SDKs promise the two agree. Invalid effort
values also went unvalidated on preview. (cursor)

Adds parity tests that drive both endpoints through the real registered
middleware and assert the effort override reaches router.Request.RoutingKnobs
on each, plus that an invalid value is rejected on both. Reverting the
middleware fails them.

Also corrects the docs' knob table: per-model-verbosity is a boolean
(true/false only), not a free-form value like the numeric knobs beside it.
@claude

claude Bot commented Jul 30, 2026

Copy link
Copy Markdown

Claude finished @steventohme's task —— View job


Comment-length lint

  • Run git diff origin/main...HEAD -- '*.go'
  • Analyze added comment blocks for length/verbosity
  • No issues found — exiting silently per task instructions

Comment thread internal/api/anthropic/route.go
…review (iteration 4)

Dropping PolicyHeaderOverridesEnabled from the preview handler didn't touch
WithRouterStrategyDefault, which still gates x-weave-router-strategy on that
same flag: an unauthorized value is logged and ignored, not rejected. The docs
and the Python README's example told every caller to send
x-weave-router-strategy: hmm to reach preview — that header does nothing for
an ordinary key, so preview only actually works for one where the deployment
default already resolves to HMM (true of Weave's managed deployments; not true
of a self-hosted install's cluster default). (cursor)

No code change: this is a documentation correction, not an authorization
change. Widening x-weave-router-strategy to every key is a separate, real
decision (it also gates strategy selection on /v1/messages, with billing/
quality consequences) that this PR didn't set out to make.

Fixed:
- docs/ROUTE_DECISION_API.md: preview section states the actual reachability
  condition; the headers table gains an "authorization required" column
  verified against each override middleware.
- clients/python/README.md + client.py docstrings: same correction, and the
  preview example no longer shows a strategy override that silently no-ops.
@claude

claude Bot commented Jul 30, 2026

Copy link
Copy Markdown

Claude finished @steventohme's task —— View job


Comment-length lint

  • Run git diff origin/main...HEAD -- '*.go'
  • Analyze added comment blocks for length/verbosity
  • Post advisory review with 2 suggestions

Posted an advisory comment-length review with two suggestions:

  • internal/server/server.go lines 237–241: 5-line block before previewGroup; the arm-hashing WHY fits in 3 lines.
  • internal/server/server_test.go lines 164–168: 5-line TestRegisterPreviewHonorsForceEffort godoc; the SDK-contract sentence can be dropped without losing non-obvious WHY.

View job

@workweave-bot workweave-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Advisory only — comment-length nits. Won't block merge.

Comment thread internal/server/server.go Outdated
Comment thread internal/server/server_test.go Outdated
Comment thread clients/python/weave_router_client/client.py Outdated
_parse_decision called RouteDecision.model_validate() before checking
schema_version. model/provider have no default, so a schema_version bump that
renamed or dropped either field would raise pydantic's ValidationError before
this function ever got to raise UnexpectedSchemaError — defeating the
check_schema_version=False escape hatch the README documents as letting a
caller read a newer server without a client bump. Read schema_version off the
raw payload first; only construct RouteDecision once the version check has
passed. (cursor)

Adds a test with a payload that is both a schema mismatch and missing
model/provider — reproduces the reported ValidationError-instead-of-
UnexpectedSchemaError failure and confirms the fix.

Applied from workweave-bot review (comment-length nits, verbatim):
- server.go preview-group comment: 5 lines -> 3.
- server_test.go TestRegisterPreviewHonorsForceEffort godoc: 5 lines -> 3.
@claude

claude Bot commented Jul 30, 2026

Copy link
Copy Markdown

Claude finished @steventohme's task —— View job


Comment-length lint

  • Run git diff origin/main...HEAD -- '*.go'
  • Analyze added comment blocks for length/verbosity
  • No issues found — exiting per task instructions

All added comment blocks are 3 lines or fewer and explain genuine non-obvious constraints (arm hashing, middleware parity, DB-free auth setup). Nothing to flag.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 5326e31. Configure here.

httpx.codes.REQUEST_ENTITY_TOO_LARGE,
):
raise InvalidRequestError(detail)
raise RoutingFailedError(f"router returned {response.status_code}: {detail}")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Route errors misclassified by SDK

Medium Severity

_raise_for_status funnels managed-mode /v1/route billing and auth infra failures (402 insufficient credits / spend cap, 503 billing or auth unavailable) into RoutingFailedError, which the SDK and docs describe as a non-retryable unhealthy strategy. Those responses are not Anthropic-shaped either, so _error_detail also misses their top-level message, while auth's 503 even sends Retry-After. Callers following the published contract will mis-alert and skip legitimate retries.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 5326e31. Configure here.

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