Feat/router in wekai - #3
Open
oferki wants to merge 66 commits into
Open
Conversation
…d the router
The router previously had its own prefix-cache implementation and used nothing
from wekai. Not a copy — an independent reimplementation of the same idea, with
different children representation, different API shape, and eviction that wekai's
version does not have. The design had called for a vendored copy with a provenance
pin, a drift check and a golden test proving equivalence; none of that was built,
so two implementations of the same concept were free to diverge silently.
"Use the wekai module as is" turned out to be impossible: prefixTrie, trieNode,
cacheEstimator, hashMessage, estimateTokens and chunkPromptPrefixN are all
unexported in package benchmark, and only SimulateReplayCache — a batch offline
simulator — is exported. Unexported identifiers are package-scoped even inside one
module, so moving the router here would not by itself grant access. The fix is
extraction, which is only clean once both live in the same module.
github.com/weka/wekai/kvcache is now that single engine. It serves two consumers
whose needs genuinely differ, and the type absorbs both:
- benchmark, offline and single-cache: RecordAndCount walks, credits and inserts
in one call, and accumulates an aggregate ratio. A zero Config means
unbounded, which is the infinite-cache model the simulator wants, so behaviour
here is unchanged by the move.
- the router, online and one instance per backend: Query is pure so every
candidate can be asked without teaching any of them, Commit records against
exactly one, and RouterConfig() is bounded because a real vLLM node evicts and
an unbounded model drifts optimistic without limit.
benchmark keeps its unexported names as thin adapters over kvcache, so every call
site and test in that package is untouched — its existing tests were the contract
and they pass unchanged. Two details preserved deliberately: capture-file hashes
("sha256:<16 hex>") parse losslessly through HashLabel, and the hashes this package
computes are per-run trie keys that are never serialized, so changing the hash
function cannot invalidate stored results.
Two hardening fixes came out of the merge. HashContent now length-prefixes the tag,
because sha256(tag || 0x00 || content) is ambiguous when both fields are
caller-controlled: role="user\x00"+content="X" and role="user"+content="\x00X"
hashed identically, which is a craftable affinity collision that needs no attack on
SHA-256. And continuation windows carry a distinct tag, so a first window and a
continuation window of the same bytes no longer collide.
The router moves to router/ with its entrypoint at router/cmd/wllm-router — a
separate binary rather than a `wekai` subcommand, so the container holds only router
code and client-go stays out of the CLI everyone builds. Its import fence now also
asserts kvcache stays a stdlib-only leaf, so neither consumer can couple the other
to a wire format.
Verified: the whole wekai module builds and tests green, the router binary is
unchanged at 35 MB, the distroless image builds and runs non-root with a read-only
rootfs, and prefix affinity holds end to end — eight requests sharing a system
prompt all pinned to one backend.
NOTE: go.mod gains k8s.io/client-go, so anyone with a local vendor/ must re-run
`go mod vendor`. vendor/ is not committed here and is deliberately excluded from
the router's Docker context, which builds with -mod=mod instead.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Until now the only Go gate in this repo ran inside release.yml, on push to main. That is after a merge: a broken change could only be discovered once it was already on the default branch and a release was being cut from it. Nothing checked a pull request at all. Add a root Makefile as the single definition of the gates, and a ci.yml that runs `make verify` (gofmt + go vet + the full suite under -race) on pull_request and on push to main. release.yml's test step becomes `make test` — the same `go test ./... -timeout 300s` it ran before, now defined in one place so the release gate and local runs cannot drift. It deliberately does not become `make verify`: gofmt and -race are pre-merge concerns, and a formatting slip should not block a release. Notes on the pieces: - CLAUDE.md documents `task build` / `task test`, but no Taskfile exists, so those commands do not run. The Makefile provides the real entry points. Publishing stays with Dagger. - fmt-check is plain `gofmt -l`, not gofumpt: gofumpt would add a network dependency to CI and hold the repo to a standard it does not currently meet. Two pre-existing violations (benchmark/types.go, cli/command_router_replay_prepare.go) are fixed here so the gate is adoptable — gofmt -w only, no behaviour change. - The router's invariant fences (router/hack) are ordinary tests, so verify already runs them; `make fences` exists for iterating on them alone. - Fuzzing is a scheduled/dispatch job, not a per-PR gate: it is time-boxed exploration rather than a correctness check. It has already earned its keep once, catching a remote DoS where a 15-byte body looped forever with unbounded allocation. The seed corpus still runs per-PR as normal tests. - vendor/ and the router binary are now ignored. vendor/ especially: committing it flips the module to -mod=vendor, so CI would build from whatever tree was checked in rather than from go.mod, and a stale vendor/ would change what ships with no dependency change to show for it. Verified locally: make verify passes (17 packages, 0 failures), make test passes, fmt-check genuinely fails on unformatted input and passes once restored, and the fuzz target runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Ofer Kiselov Nahman <ofer.kiselovnahman@weka.io>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new Go-based wllm-router (v2) within the wekai module, including core routing primitives (backend registry, load accounting, policies), observability, Kubernetes deployment artifacts, and CI/build tooling to gate correctness before merge.
Changes:
- Adds the v2 router implementation in Go (registry/circuit/health/lease/gateway/dialect/policies) with extensive unit + invariant “fence” tests.
- Adds build, container, and Kubernetes deployment assets for running
wllm-routeras a separate distroless image. - Adds CI workflow and standardizes local/release test commands via
make test/make verify.
Reviewed changes
Copilot reviewed 65 out of 68 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| router/Makefile.mk | Router-specific Go build/test/lint/image targets. |
| router/internal/testutil/mockvllm/mockvllm.go | Programmable mock vLLM backend for router tests. |
| router/internal/registry/registry.go | Backend membership registry with snapshot publishing and drain removal. |
| router/internal/registry/backend.go | Backend model (health/provenance/capacity/load) and URL canonicalization. |
| router/internal/policy/roundrobin.go | Deterministic round-robin policy keyed by backend identity. |
| router/internal/policy/policy.go | Policy interfaces, shared tie-break logic, and basic policies. |
| router/internal/policy/cache/threshold.go | Threshold-based cache candidate filtering policy. |
| router/internal/policy/cache/threshold_test.go | Unit tests for ThresholdPolicy decision tree and invariants. |
| router/internal/policy/cache/gauge_test.go | Ensures cache-size gauges reflect commits. |
| router/internal/policy/cache/cache.go | Cache-affinity routing policy using kvcache tries. |
| router/internal/policy/cache/cache_test.go | Unit + benchmark tests for cache-affinity policy behavior. |
| router/internal/obs/obs.go | Structured logging init + request-scoped context helpers. |
| router/internal/metrics/observed_shadow.go | Test-accessible “shadow” gauge for observed cache fraction. |
| router/internal/metrics/metrics.go | Prometheus collectors + explicit registry wiring. |
| router/internal/metrics/collectors.go | Go runtime + process collectors separated from router collectors. |
| router/internal/lease/lease.go | Idempotent in-flight accounting primitive (single writer). |
| router/internal/lease/lease_test.go | Property and concurrency tests for lease invariants. |
| router/internal/jsonscan/testdata/fuzz/FuzzFieldsAgreesWithEncodingJSON/f9806f50b31bd653 | Fuzz seed corpus for JSON scanner. |
| router/internal/jsonscan/testdata/fuzz/FuzzFieldsAgreesWithEncodingJSON/f8051c0fb82f956f | Fuzz seed corpus for JSON scanner. |
| router/internal/jsonscan/testdata/fuzz/FuzzFieldsAgreesWithEncodingJSON/9010164ce57359b3 | Fuzz seed corpus for JSON scanner. |
| router/internal/jsonscan/testdata/fuzz/FuzzFieldsAgreesWithEncodingJSON/5753781e9fb25470 | Fuzz seed corpus for JSON scanner. |
| router/internal/health/health.go | Concurrent active health checker + backend gauge publishing. |
| router/internal/gateway/gateway.go | HTTP surface, admin endpoints, and middleware chain wiring. |
| router/internal/gateway/export_test.go | Exposes recover middleware for external tests. |
| router/internal/discovery/k8s/helpers_test.go | K8s test helpers for fake client object wiring. |
| router/internal/dialect/openai/openai.go | OpenAI-compatible dialect implementation (routes, unit extraction, errors, usage). |
| router/internal/dialect/openai/openai_test.go | Dialect safety/determinism tests (malformed bodies, tool-call turns). |
| router/internal/dialect/dialect.go | Dialect interface + registry and stream terminal scanning helper. |
| router/internal/config/duration.go | Human-readable JSON duration type for ConfigMap compatibility. |
| router/internal/config/config_test.go | Config parsing/validation precedence, security, and regression tests. |
| router/internal/clock/clock.go | Clock abstraction + fake clock for deterministic time-based tests. |
| router/internal/clock/clock_test.go | Tests for fake/real clock semantics. |
| router/internal/circuit/circuit.go | Sliding-window circuit breaker with half-open admission tokens. |
| router/internal/circuit/circuit_test.go | Circuit breaker regression tests (windowing, half-open, classification). |
| router/hack/manifest_test.go | Validates shipped K8s manifests and embedded config load correctly. |
| router/hack/fence_test.go | Mechanical invariant fences (imports, time usage, dead metrics, load writers). |
| router/docs/rewrite/README.md | Rewrite rationale and design doc entrypoint. |
| router/deploy/README.md | Deployment and operational guidance for wllm-router. |
| router/deploy/k8s/rbac.yaml | Namespace-scoped RBAC for discovery modes. |
| router/deploy/k8s/deployment.yaml | ConfigMap/Secret/Deployment/Service/PDB manifest bundle. |
| router/cmd/wllm-router/main.go | Router wiring: config, policy selection, discovery, health, metrics, graceful shutdown. |
| Makefile | Repo-wide Go verify/test/build targets + router build/image targets. |
| kvcache/shared_test.go | Shared kvcache invariants across router + benchmark consumers. |
| kvcache/fence_test.go | Ensures kvcache remains a stdlib-only leaf dependency. |
| go.mod | Adds Kubernetes client deps and YAML parsing for router/discovery/manifest tests. |
| Dockerfile.wllm-router | Distroless multi-stage build for router-only image. |
| cli/command_router_replay_prepare.go | Minor formatting/indentation adjustment in replay metadata. |
| benchmark/types.go | Removes trailing whitespace. |
| benchmark/cache_sim.go | Refactors benchmark cache simulation to reuse shared kvcache engine. |
| .gitignore | Ignores wllm-router binary and vendor/. |
| .github/workflows/release.yml | Uses make test for release gating. |
| .github/workflows/ci.yml | Adds PR/push CI with make verify and scheduled fuzzing. |
| .dockerignore | Reduces router image build context size. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+265
to
+276
| case "prefix-cache-candidates": | ||
| // Filters candidates to those predicted to hold the prefix, then picks | ||
| // among that filtered set rather than a single best-scoring backend; | ||
| // see ThresholdPolicy's doc comment for the full decision tree. | ||
| p := cachepolicy.NewThreshold(cachepolicy.ThresholdConfig{ | ||
| CacheThreshold: cfg.Cache.CacheThreshold, | ||
| MaxPending: cfg.Cache.BalanceAbsThreshold, | ||
| Trie: kvcache.Config{ | ||
| MaxNodes: cfg.Cache.MaxNodes, | ||
| MaxTokens: cfg.Cache.MaxTokens, | ||
| }, | ||
| }, policy.LeastOutstanding{}) |
Signed-off-by: Ofer Kiselov Nahman <ofer.kiselovnahman@weka.io>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Ofer Kiselov Nahman <ofer.kiselovnahman@weka.io>
Signed-off-by: Ofer Kiselov Nahman <ofer.kiselovnahman@weka.io>
Signed-off-by: Ofer Kiselov Nahman <ofer.kiselovnahman@weka.io>
The router read WLLM_PATH_ALLOWLIST; v1 reads FORWARD_PATH_ALLOWLIST. Since the
migration is "swap the image, keep the env", the renamed variable meant the
allowlist would simply be inactive on any deployment carried over from v1 —
every path served instead of the listed few, and nothing in the logs to say so.
Auth still applies to every path, so this was a wider served surface rather than
an open router, but the silence is the problem.
Renamed rather than aliased: nothing is deployed on the new name yet, so there
is no compatibility to preserve, and one name is better than two. This is a
deliberate break from the WLLM_ prefix every other variable uses.
Also correct the empty-allowlist documentation, which described behaviour the
code does not have. Three places (config comment, middleware comment, design
doc) claimed empty means deny-by-default, "a deliberate, breaking inversion of
v1 semantics [that] MUST be called out in release notes" per AUTH-8. The code
does no such thing: empty returns true, every path is served, and auth applies
to all of them — exactly v1. Writing that release note would have described a
breaking change that was never made.
AUTH-8 is withdrawn rather than left outstanding, because its premise was wrong.
It rested on AUTH-N2, "empty-allowlist-means-allow-all turns a config typo into
an open router". It does not: the allowlist gates reachability, auth gates
access, and the two are independent, so a mistyped allowlist widens what is
served while leaving every path behind auth. Deny-by-default would also mean an
unset variable serves nothing at all, kubelet probes included.
The two tests that were supposed to pin this did not exist, which is why doc and
code drifted silently:
- TestEmptyAllowlistServesAllPathsUnderAuth asserts empty serves all paths AND
that an uncredentialed request is still 401 — the second half is what makes
AUTH-N2's premise false rather than merely asserted.
- TestAdminNotExemptibleByAllowlist takes the adversarial case: it puts
/get_loads and /add_worker ON the allowlist and requires them to stay
authenticated, then requires an unlisted admin path to 404 even for a valid
key.
Both were mutation-tested rather than assumed to work. Making pathAllowed
deny-by-default fails the first; making listed paths bypass auth fails the
second, and that mutant registered a backend via unauthenticated /add_worker
(registry 1 -> 2 backends), so the test covers real impact and not just a status
code.
Verified: FORWARD_PATH_ALLOWLIST activates the allowlist on a running binary
(GET /get_loads -> 404) while WLLM_PATH_ALLOWLIST no longer does anything
(-> 200). make verify passes, 18 packages, 0 failures.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…w the predicted cache hit of the selected node Signed-off-by: Ofer Kiselov Nahman <ofer.kiselovnahman@weka.io>
…d the router
The router previously had its own prefix-cache implementation and used nothing
from wekai. Not a copy — an independent reimplementation of the same idea, with
different children representation, different API shape, and eviction that wekai's
version does not have. The design had called for a vendored copy with a provenance
pin, a drift check and a golden test proving equivalence; none of that was built,
so two implementations of the same concept were free to diverge silently.
"Use the wekai module as is" turned out to be impossible: prefixTrie, trieNode,
cacheEstimator, hashMessage, estimateTokens and chunkPromptPrefixN are all
unexported in package benchmark, and only SimulateReplayCache — a batch offline
simulator — is exported. Unexported identifiers are package-scoped even inside one
module, so moving the router here would not by itself grant access. The fix is
extraction, which is only clean once both live in the same module.
github.com/weka/wekai/kvcache is now that single engine. It serves two consumers
whose needs genuinely differ, and the type absorbs both:
- benchmark, offline and single-cache: RecordAndCount walks, credits and inserts
in one call, and accumulates an aggregate ratio. A zero Config means
unbounded, which is the infinite-cache model the simulator wants, so behaviour
here is unchanged by the move.
- the router, online and one instance per backend: Query is pure so every
candidate can be asked without teaching any of them, Commit records against
exactly one, and RouterConfig() is bounded because a real vLLM node evicts and
an unbounded model drifts optimistic without limit.
benchmark keeps its unexported names as thin adapters over kvcache, so every call
site and test in that package is untouched — its existing tests were the contract
and they pass unchanged. Two details preserved deliberately: capture-file hashes
("sha256:<16 hex>") parse losslessly through HashLabel, and the hashes this package
computes are per-run trie keys that are never serialized, so changing the hash
function cannot invalidate stored results.
Two hardening fixes came out of the merge. HashContent now length-prefixes the tag,
because sha256(tag || 0x00 || content) is ambiguous when both fields are
caller-controlled: role="user\x00"+content="X" and role="user"+content="\x00X"
hashed identically, which is a craftable affinity collision that needs no attack on
SHA-256. And continuation windows carry a distinct tag, so a first window and a
continuation window of the same bytes no longer collide.
The router moves to router/ with its entrypoint at router/cmd/wllm-router — a
separate binary rather than a `wekai` subcommand, so the container holds only router
code and client-go stays out of the CLI everyone builds. Its import fence now also
asserts kvcache stays a stdlib-only leaf, so neither consumer can couple the other
to a wire format.
Verified: the whole wekai module builds and tests green, the router binary is
unchanged at 35 MB, the distroless image builds and runs non-root with a read-only
rootfs, and prefix affinity holds end to end — eight requests sharing a system
prompt all pinned to one backend.
NOTE: go.mod gains k8s.io/client-go, so anyone with a local vendor/ must re-run
`go mod vendor`. vendor/ is not committed here and is deliberately excluded from
the router's Docker context, which builds with -mod=mod instead.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Until now the only Go gate in this repo ran inside release.yml, on push to main. That is after a merge: a broken change could only be discovered once it was already on the default branch and a release was being cut from it. Nothing checked a pull request at all. Add a root Makefile as the single definition of the gates, and a ci.yml that runs `make verify` (gofmt + go vet + the full suite under -race) on pull_request and on push to main. release.yml's test step becomes `make test` — the same `go test ./... -timeout 300s` it ran before, now defined in one place so the release gate and local runs cannot drift. It deliberately does not become `make verify`: gofmt and -race are pre-merge concerns, and a formatting slip should not block a release. Notes on the pieces: - CLAUDE.md documents `task build` / `task test`, but no Taskfile exists, so those commands do not run. The Makefile provides the real entry points. Publishing stays with Dagger. - fmt-check is plain `gofmt -l`, not gofumpt: gofumpt would add a network dependency to CI and hold the repo to a standard it does not currently meet. Two pre-existing violations (benchmark/types.go, cli/command_router_replay_prepare.go) are fixed here so the gate is adoptable — gofmt -w only, no behaviour change. - The router's invariant fences (router/hack) are ordinary tests, so verify already runs them; `make fences` exists for iterating on them alone. - Fuzzing is a scheduled/dispatch job, not a per-PR gate: it is time-boxed exploration rather than a correctness check. It has already earned its keep once, catching a remote DoS where a 15-byte body looped forever with unbounded allocation. The seed corpus still runs per-PR as normal tests. - vendor/ and the router binary are now ignored. vendor/ especially: committing it flips the module to -mod=vendor, so CI would build from whatever tree was checked in rather than from go.mod, and a stale vendor/ would change what ships with no dependency change to show for it. Verified locally: make verify passes (17 packages, 0 failures), make test passes, fmt-check genuinely fails on unformatted input and passes once restored, and the fuzz target runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Ofer Kiselov Nahman <ofer.kiselovnahman@weka.io>
Signed-off-by: Ofer Kiselov Nahman <ofer.kiselovnahman@weka.io>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Ofer Kiselov Nahman <ofer.kiselovnahman@weka.io>
Signed-off-by: Ofer Kiselov Nahman <ofer.kiselovnahman@weka.io>
Signed-off-by: Ofer Kiselov Nahman <ofer.kiselovnahman@weka.io>
The router read WLLM_PATH_ALLOWLIST; v1 reads FORWARD_PATH_ALLOWLIST. Since the
migration is "swap the image, keep the env", the renamed variable meant the
allowlist would simply be inactive on any deployment carried over from v1 —
every path served instead of the listed few, and nothing in the logs to say so.
Auth still applies to every path, so this was a wider served surface rather than
an open router, but the silence is the problem.
Renamed rather than aliased: nothing is deployed on the new name yet, so there
is no compatibility to preserve, and one name is better than two. This is a
deliberate break from the WLLM_ prefix every other variable uses.
Also correct the empty-allowlist documentation, which described behaviour the
code does not have. Three places (config comment, middleware comment, design
doc) claimed empty means deny-by-default, "a deliberate, breaking inversion of
v1 semantics [that] MUST be called out in release notes" per AUTH-8. The code
does no such thing: empty returns true, every path is served, and auth applies
to all of them — exactly v1. Writing that release note would have described a
breaking change that was never made.
AUTH-8 is withdrawn rather than left outstanding, because its premise was wrong.
It rested on AUTH-N2, "empty-allowlist-means-allow-all turns a config typo into
an open router". It does not: the allowlist gates reachability, auth gates
access, and the two are independent, so a mistyped allowlist widens what is
served while leaving every path behind auth. Deny-by-default would also mean an
unset variable serves nothing at all, kubelet probes included.
The two tests that were supposed to pin this did not exist, which is why doc and
code drifted silently:
- TestEmptyAllowlistServesAllPathsUnderAuth asserts empty serves all paths AND
that an uncredentialed request is still 401 — the second half is what makes
AUTH-N2's premise false rather than merely asserted.
- TestAdminNotExemptibleByAllowlist takes the adversarial case: it puts
/get_loads and /add_worker ON the allowlist and requires them to stay
authenticated, then requires an unlisted admin path to 404 even for a valid
key.
Both were mutation-tested rather than assumed to work. Making pathAllowed
deny-by-default fails the first; making listed paths bypass auth fails the
second, and that mutant registered a backend via unauthenticated /add_worker
(registry 1 -> 2 backends), so the test covers real impact and not just a status
code.
Verified: FORWARD_PATH_ALLOWLIST activates the allowlist on a running binary
(GET /get_loads -> 404) while WLLM_PATH_ALLOWLIST no longer does anything
(-> 200). make verify passes, 18 packages, 0 failures.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…w the predicted cache hit of the selected node Signed-off-by: Ofer Kiselov Nahman <ofer.kiselovnahman@weka.io>
…t/router-in-wekai
Organised by how each difference fails, because that determines what actually
bites during a cutover. Three things change without producing any error:
- Every metric was renamed, vllm_router_* -> router_*, with no overlap. A
Grafana panel does not error, it goes blank, and an alert that can never
fire looks exactly like one that is healthy. Includes a full mapping table,
separating clean renames from the ones that also changed shape —
cache_hits_total/misses_total in particular are NOT a rename, they became a
predicted fraction and an observed fraction, different question and
different units.
- Unmatched paths now 404 instead of being transparently proxied. v1 had a
catch-all, so every path reached a backend; this router serves only routes
it knows. Verified live: /tokenize, /v1/audio/transcriptions, /v1/score,
/flush_cache, /health_generate and /v1/responses/{id} all 404.
- INBOUND_API_KEY is ignored in favour of WLLM_API_KEY, and no key means no
authentication, admin endpoints included.
That last one stays as designed: no key disabling auth is what makes the router
runnable on a laptop or a trusted network, so the binary will not refuse to
start and will not demand an explicit unauthenticated flag. The doc states this
as intended rather than leaving it looking like a defect, and puts the guard
where it belongs — an assertion in the rollout that an uncredentialed
GET /get_loads returns 401, not 200.
Loud failures are covered too (policy renames, consistent_hash having no
equivalent, stricter config validation), plus the pod-spec changes: probes
invert from exec back to httpGet now that they are public by default, and the
distroless nonroot image means no shell, no kubectl exec, and no runAsUser: 0.
Every metric name and every served path asserted in the guide was checked
against the source; the two things not verified (error body shapes, log field
names) are called out as such rather than left implied.
Also fixes the build section of deploy/README.md, which still told operators to
run `make -f Makefile.go.mk build`. That file does not exist after the move into
wekai, so all five commands failed. They now point at the root Makefile.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
It was the router-local makefile from before the move into wekai, and nothing referenced it any more. It was also quietly wrong from here: every path in it assumed router/ was the module root (./cmd/wllm-router, ./internal/jsonscan, gofmt -l internal cmd hack, -f deploy/k8s/...), it pushed to ghcr.io/weka/wllm-router rather than quay.io, and its image target built the default Dockerfile instead of Dockerfile.wllm-router. Anyone who found it and ran it would have gotten failures or the wrong registry. Most of its targets already existed in the root Makefile under router-* names. Four did not, so they are ported rather than dropped, with paths corrected for the repo root: - router-run run locally against a backend at :8000 - router-deploy apply rbac.yaml and deployment.yaml - router-manifests-validate server-side dry-run of both manifests - router-image-smoke build the image, check the binary reports -version test-short and lint are deliberately not ported: the root `test` is already the non-race suite, and `verify` covers vet, gofmt and the hack/ fences. Also fixes the second stale reference in deploy/README.md, `make -f Makefile.go.mk run`, which the previous commit missed — it only fixed the build block at the top of that file. Both stale references pointed at Makefile.go.mk, a filename that has not existed in either repository. Verified: all four ported targets expand to correct paths under `make -n`, they appear in `make help`, `make router-build` produces a working binary, and `make verify` passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d the router
The router previously had its own prefix-cache implementation and used nothing
from wekai. Not a copy — an independent reimplementation of the same idea, with
different children representation, different API shape, and eviction that wekai's
version does not have. The design had called for a vendored copy with a provenance
pin, a drift check and a golden test proving equivalence; none of that was built,
so two implementations of the same concept were free to diverge silently.
"Use the wekai module as is" turned out to be impossible: prefixTrie, trieNode,
cacheEstimator, hashMessage, estimateTokens and chunkPromptPrefixN are all
unexported in package benchmark, and only SimulateReplayCache — a batch offline
simulator — is exported. Unexported identifiers are package-scoped even inside one
module, so moving the router here would not by itself grant access. The fix is
extraction, which is only clean once both live in the same module.
github.com/weka/wekai/kvcache is now that single engine. It serves two consumers
whose needs genuinely differ, and the type absorbs both:
- benchmark, offline and single-cache: RecordAndCount walks, credits and inserts
in one call, and accumulates an aggregate ratio. A zero Config means
unbounded, which is the infinite-cache model the simulator wants, so behaviour
here is unchanged by the move.
- the router, online and one instance per backend: Query is pure so every
candidate can be asked without teaching any of them, Commit records against
exactly one, and RouterConfig() is bounded because a real vLLM node evicts and
an unbounded model drifts optimistic without limit.
benchmark keeps its unexported names as thin adapters over kvcache, so every call
site and test in that package is untouched — its existing tests were the contract
and they pass unchanged. Two details preserved deliberately: capture-file hashes
("sha256:<16 hex>") parse losslessly through HashLabel, and the hashes this package
computes are per-run trie keys that are never serialized, so changing the hash
function cannot invalidate stored results.
Two hardening fixes came out of the merge. HashContent now length-prefixes the tag,
because sha256(tag || 0x00 || content) is ambiguous when both fields are
caller-controlled: role="user\x00"+content="X" and role="user"+content="\x00X"
hashed identically, which is a craftable affinity collision that needs no attack on
SHA-256. And continuation windows carry a distinct tag, so a first window and a
continuation window of the same bytes no longer collide.
The router moves to router/ with its entrypoint at router/cmd/wllm-router — a
separate binary rather than a `wekai` subcommand, so the container holds only router
code and client-go stays out of the CLI everyone builds. Its import fence now also
asserts kvcache stays a stdlib-only leaf, so neither consumer can couple the other
to a wire format.
Verified: the whole wekai module builds and tests green, the router binary is
unchanged at 35 MB, the distroless image builds and runs non-root with a read-only
rootfs, and prefix affinity holds end to end — eight requests sharing a system
prompt all pinned to one backend.
NOTE: go.mod gains k8s.io/client-go, so anyone with a local vendor/ must re-run
`go mod vendor`. vendor/ is not committed here and is deliberately excluded from
the router's Docker context, which builds with -mod=mod instead.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Ofer Kiselov Nahman <ofer.kiselovnahman@weka.io>
The replay-v3 data records every request's production-measured prompt size (usage input_tokens + cache read/creation tokens). Use that for the --limit-context skip decision instead of the len(body) > limit*4 chars heuristic, which under-estimated dense (code-heavy) prompts by up to ~35% and let real context overflows through. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rt98zb2r3b7HEyFcJ4ZTxf
anton's feedback on the first cut: it wasn't showing the tree the way kv-router-sim.html did, which he explicitly praised. This replaces the row-per-backend/column-per-block grid with a faithful port of the reference's tree rendering: a shared prefix appears ONCE, as a common ancestor box, with sessions branching below it wherever their content actually diverges — not a flat list of per-backend rows. router/internal/policy/cache/tree.go (new): merges every backend's kvcache.Trie.Chains output into ONE shared trie keyed by hash (a session served by two backends becomes a single mergeNode with both URLs in its present set — the entire mechanism behind "shared prefix, one ancestor"). Then radix-compresses it into "runs" the same way the reference collapses a long shared prefix into a single box instead of one row per block: a maximal chain that neither branches NOR changes which backends hold it becomes one row. The presence-homogeneity condition is new relative to the reference (whose "marks" live per-run by construction, so it never needed this) — ours are derived from real per-block data, so a compressed run must stop the instant two consecutive blocks disagree on which backends hold them, or the row would show a blended/wrong presence pattern. Covered by a dedicated test that would fail without that check. router/internal/viz: Snapshot's shape changes from a flat per-backend Blocks/Present list to Tree []TreeNode — a flat, PARENT-INDEXED array (Parent==-1 for a root, Children as indices), the same shape as the reference's buildView() output, built server-side so the wire payload stays small regardless of trie size. AvgCopies is still computed at the BLOCK level, before compression, so the duplication number stays accurate regardless of how the tree gets compressed or capped for display. Truncation reporting is now NodesShown/NodesTotal/Truncated (rows, not chains). page.html's renderTree/layout functions are a near-literal port of the reference's place()/px()/py() recursive centering layout and its curved-path connector drawing — the JS no longer builds the tree (that now happens in Go), it only lays out and draws the pre-flattened array. Squares are real green (the reference's own tree-view squares are actually blue; anton specifically asked for green). One correctness fix caught before it shipped: kvcache.Trie.Chains always walks its ENTIRE trie to compute an accurate total, regardless of the limit argument — only the returned slice is capped. Reusing the small display ?limit= for that fetch call would have silently under-reported NodesTotal once compression entered the picture (fewer chains fetched == fewer blocks merged == a smaller, wrong "total"). Fixed by decoupling the two: a generous fixed fetchCapPerBackend for the merge-tree input, the caller's limit applied only at the final flatten/display stage. A dedicated test (TestSnapshot_LimitTruncatesTreeRows) exercises exactly this and would have caught the bug had the fix not been made. Kept everything else from the prior commit as instructed: live ~1s polling, self-contained page (no CDNs), metrics-mux mounting at the same /router-viz and /router-viz/data paths, ?limit=/truncation labeling, avg_copies stat, per-backend health/inflight header row. Verified: go build ./..., go vet, and the full kvcache+router suite (including router/hack's fences) pass under -race — same one pre-existing, unrelated TestKvcacheDependsOnlyOnStdlib toolchain failure as every prior commit on this branch, untouched here. Tests rewritten for the new shape plus new coverage: TestSnapshot_SharedPrefixIsOneCommonAncestor (exact ancestor/children/presence assertions), TestSnapshot_ LongSharedChainCompressesToOneRow and TestSnapshot_CompressionBreaksOnPresenceChange (both directions of the compression rule), TestSnapshot_LimitTruncatesTreeRows (the fetch-cap/display-cap bug guard above), plus the existing empty/ best-effort/concurrent-Commit cases carried over. Manually verified live: rebuilt both binaries, ran wllm-router against two mock-vllm backends, sent the same shared-prefix + divergent-tail + concurrent-burst traffic as before. curl /router-viz/data confirmed: the shared 1024-byte chunk landed as ONE root node present on both backends (256 est. tokens), with 6 divergent per-session children below it each present on exactly one backend, plus an unrelated one-off prompt as its own separate root — exactly the "shared prefix = single ancestor, sessions branch below it" shape anton asked for. Truncation reporting checked honest at two different ?limit= values (44 real rows total, 8 shown by default because of the deliberately tight maxChildrenPerNode=6 per-row cap — not the 80-row overall budget — and exactly 6 shown with limit=6). /metrics on the same listener kept serving correctly throughout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rt98zb2r3b7HEyFcJ4ZTxf
anton: "make it by default show all — not make me go via query param. any such configuration should be in ui, not by query param. by default - unlimited." Removes every hardcoded cap that shipped in the last two commits: DefaultChainLimit (80), the implicit fetch cap, and maxChildrenPerNode (6) are all gone. With no user interaction /router-viz now shows the COMPLETE merged prefix tree — every row, every child, every depth. router/internal/viz.DataSource.Snapshot's signature changes from Snapshot(limit int) to Snapshot(opts SnapshotOptions), where every field (MaxRows, MaxChildren, MaxDepth) zero-values to unlimited. DataHandler still accepts ?limit=/?max_children=/?max_depth= — that's the mechanism the page's OWN UI controls use to talk to it — but omitting all three (what a fresh page load does) now reaches the DataSource as a completely zero SnapshotOptions, not a hidden non-zero default. An explicit huge value is still clamped (MaxParamValue, replacing the old MaxChainLimit) so someone hand-crafting the URL can't force a truly pathological walk, but that ceiling only ever applies to a value actually given. router/internal/policy/cache/tree.go: flattenTree takes SnapshotOptions instead of a bare limit. Fixed one ordering bug while wiring MaxDepth in: checking the depth cap AFTER a child's index had already been reserved in the parent's Children slice would leave a dangling reference to a row that was never emitted — moved the check to run once per node, before any of its children are considered, so an excluded depth never gets a half-recorded child index. fetchCapPerBackend is gone entirely — the merge-tree build now always fetches the full trie (kvcache.Trie.Chains(0)), independent of whatever display option the caller asked for, because NodesTotal must report the fleet's TRUE state regardless of what the user chose to look at. page.html: three number inputs (rows / children-per-node / depth) plus a "show all" reset button in the header — all empty by default, which is what makes the default fetch unlimited (an empty control contributes no query param at all). Control changes trigger an immediate re-poll via a single-timer scheduler (schedulePoll/runPoll) that cancels any pending scheduled poll before arming a new one — without that, a control change firing poll() directly alongside the existing setTimeout chain would silently double, then triple, the effective poll rate over time. Also added change-detection (compare the tree payload to the last render, skip the O(n) SVG rebuild when nothing changed) per the efficiency guidance, since the default view can now legitimately be large. Verified: go build ./..., go vet, and the full kvcache+router suite (including router/hack's fences) pass under -race — same one pre-existing, unrelated TestKvcacheDependsOnlyOnStdlib toolchain failure as every prior commit on this branch. New/changed tests: TestSnapshot_DefaultIsUnlimited (200 sessions, zero options, all 200 rows back), TestSnapshot_MaxChildrenLimitsPerNode and TestSnapshot_MaxDepthLimitsDepth (both new caps, and that NodesTotal stays the true total regardless), TestSnapshot_MaxRowsTruncatesTreeRows (renamed/kept from the prior commit). viz_test.go adds TestDataHandler_NoParamsMeansUnlimited (asserts the DataSource receives a literal zero-value SnapshotOptions with no query string) and TestPageHandler_HasUIControlsNotJustQueryParams (structural check that the page defines the three input elements and does NOT read location.search — the UI, not the URL, is the configuration surface). Manually verified live: rebuilt both binaries, ran wllm-router against two mock-vllm backends, sent 20 distinct sessions sharing one prefix (well past the OLD hardcoded 6-child cap) plus one unrelated prompt. curl /router-viz/data with NO query params came back nodes_shown/total 22/22, truncated:false, and the shared root's Children array had all 20 entries — confirming the default is genuinely unlimited end to end, not just at the unit-test level. Re-querying with ?max_children=3 (simulating what the UI control sends) correctly reduced it to 5/22 shown, truncated:true. Confirmed /router-viz serves the three ctl-rows/ctl-children/ctl-depth inputs, and /metrics on the same listener kept working throughout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rt98zb2r3b7HEyFcJ4ZTxf
anton's screenshot review: add a descendants-count display to each tree node box, distinct from the existing ×N backend-copies marker — the total number of REAL blocks (not compressed rows) in that node's subtree, including itself. router/internal/policy/cache/tree.go: run.computeSubtreeSize (rows only, used for ordering which branch survives a display cap) is replaced by run.computeSubtree, which fills both subtreeSize (rows) and the new subtreeBlocks (sum of RunLen across the run and its whole subtree) in one bottom-up walk. The distinction matters: a long uncontested shared prefix compresses to ONE row but should still count every block it represents toward the total, and a row further down the tree adds its own descendants on top — so subtreeBlocks is deliberately NOT the same number as a row count. router/internal/viz: TreeNode gets a SubtreeBlocks field (json:"subtree_blocks"), computed server-side during the same tree walk that already builds everything else — no extra pass, no client-side computation needed. page.html: renders it as a small "⊂N" badge just left of the backend-presence squares (existing .tsub style, same visual language as the "×N" marker at the box's far right, but a different symbol so the two are never confused), with the box width/label-truncation math extended to reserve space for it. Legend line updated to explain it. Also reconfirmed (already landed in a4bfa68, still intact): default is unlimited rows/children/depth, any caps are UI controls not query params. Verified: go build ./..., go vet, and go test ./kvcache/... ./router/... -race pass (including router/hack's fences) — same one pre-existing, unrelated TestKvcacheDependsOnlyOnStdlib toolchain failure as every prior commit on this branch. (benchmark/* currently fails vet/build due to a teammate's own concurrent in-progress edit there, per their explicit instruction not to touch that package — confirmed my own packages vet clean in isolation: `go vet ./kvcache/... ./router/...` exits 0.) New test: TestSnapshot_SubtreeBlocksCountsRealBlocksNotRows builds a known 3-row tree where the root compresses 3 blocks into one row and each child is itself a multi-block run (2 and 4 blocks) — so root.SubtreeBlocks (9) must differ from both the row count (3) and the root's own RunLen (3), proving the badge counts real underlying blocks, not tree rows; each leaf child's SubtreeBlocks is asserted to equal its own RunLen exactly. Manually verified live: rebuilt both binaries, ran wllm-router against two mock-vllm backends, sent 3 sessions sharing one prefix. curl /router-viz/data confirmed subtree_blocks=4 on the shared root (1 own + 3 one-block children) and subtree_blocks=1 on each leaf (matching its own run_len). Confirmed /router-viz's served HTML contains both the subtree_blocks field reference and the ⊂N legend text. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rt98zb2r3b7HEyFcJ4ZTxf
…ture token counts
Two fidelity gaps closed against a real-fleet comparison (goal: mock x1
run of the golden 4,800-request replay landing on the real fleet's warm
68.0% / cached 48.9%, currently 73.8/63.5 — chars-per-token and
output-kv-multiplier are the two knobs anton will sweep to calibrate,
now that they exist).
GAP 1 — --chars-per-token (default 4.0). The mock counted every content
byte at kvcache's fixed 4-bytes-per-token estimate; real vLLM's actual
tokenizer runs closer to 2.9-3.4 chars/token on dense agentic content, so
the mock was under-counting prompts by ~25-35% and its pool was
effectively too generous. router/internal/mockvllm/tokenize.go
reimplements kvcache.ChunkContent's chunking locally — NOT because the
logic differs, but because ChunkContent's per-chunk token count always
goes through kvcache.EstimateTokens, fixed at 4.0 for every OTHER
consumer of the shared package (the router's own cache prediction, the
benchmark's estimator). This engine needed its own independently
calibratable ratio without changing kvcache's default for everyone else,
while still sharing kvcache.HashContent so chain-hash semantics stay
identical. Config.blockSizeBytes changes from BlockSizeTokens*4 to
BlockSizeTokens*CharsPerToken, so a block stays N tokens under whatever
ratio the engine is calibrated to; Tokenize (and therefore trie
accounting, usage.prompt_tokens/cached_tokens, and the latency model's
token counts, which are all downstream of Tokenize's output) picks it up
for free. <=0 falls back to 4.0 in normalize() — invalid input, not a
meaningful off-switch.
GAP 2 — --output-kv-multiplier (default 1.0; 0 disables). Real vLLM
writes decode KV into the same pool as prompt KV — generated tokens
become cached blocks of the sequence — which the mock ignored entirely.
Engine.AppendOutputBlocks, called once a response is fully built (or as
far as generation got before a client disconnect), appends
ceil(outputTokens*multiplier/BlockSizeTokens) blocks to the request's own
chain via the trie's existing Commit (walk-and-extend, so any already-
matching prefix is reused rather than duplicated). Alignment with a real
follow-up turn is best-effort: each block's content is sliced from the
ACTUAL response text at that block's offset within
"assistant:"+content+"\n" — the exact byte form
chatCompletionRequest.promptBytes gives an assistant message — so at the
realistic default with a response long enough to cover every target
block, this hashes IDENTICALLY to what a real follow-up's own chunker
would produce. Two documented, deliberately-accepted limitations rather
than chased further: short replies (or a multiplier that asks for more
blocks than the literal text covers) get deterministic filler for the
remainder — no real follow-up could hit those, but they still occupy pool
capacity and are still evictable, the primary effect either way; and the
appended blocks start a FRESH continuation after the prompt's own chain
rather than merging into a partial last prompt block, so a prompt that
doesn't end exactly on a block boundary won't get byte-perfect alignment
for that one boundary-spanning block. OutputKVMultiplier<=0 is a
deliberate off-switch (unlike CharsPerToken's invalid-input fallback), so
normalize() leaves it alone — same treatment as the zero-means-instant
latency rates already in this Config.
Wired into both streaming and non-streaming paths in handlers.go/sse.go;
streaming uses the ACTUALLY generated token count/content (which can be
less than the request's max_tokens on an early client disconnect),
matching real vLLM: decode-KV is written for what was actually generated.
No existing test needed updating: DefaultConfig() now sets
OutputKVMultiplier=1.0 (the realistic default anton asked for), but every
existing test constructs Config{} literals directly rather than via
DefaultConfig(), so OutputKVMultiplier stays Go's zero value (0,
disabled) for all of them — confirmed by grep before making the change,
then confirmed again by running the full pre-existing suite unmodified.
Verified: go build ./... and go vet clean in this package's scope
(go vet ./kvcache/... ./router/... exits 0); go test
./router/internal/mockvllm/... -race passes, 27/27 (20 pre-existing +
7 new, zero pre-existing tests touched). router/hack's fences pass
(no new clockexempt or argv issues). benchmark/* currently fails to
build due to a teammate's own concurrent in-progress edit
(undefined replaySizer in replay_router_wire.go) — not touched by me,
not staged, confirmed via git status.
New tests: TestEngine_CharsPerTokenAffectsTokenAndBlockCounts (identical
body at 4.0 vs 3.2 yields proportionally more tokens AND more blocks),
TestEngine_CharsPerTokenBelowZeroFallsBackToDefault,
TestChatCompletions_CharsPerTokenAffectsUsage (same property through the
real HTTP usage.prompt_tokens field), TestEngine_OutputKVMultiplierZeroDisablesModeling,
TestEngine_OutputKVMultiplierScalesBlockCount (exact ceil() formula),
TestEngine_OutputBlocksOccupyCapacityAndAreEvictable (node count rises,
then a tiny-cap pressure test proves they're not permanently pinned),
TestEngine_OutputBlocksHitByFollowUpTurn (the achievable-alignment case:
a block-aligned prompt + real follow-up turn hits MORE than just the
original prompt).
Manually verified live: built the binary, launched with
--chars-per-token 4.0 --output-kv-multiplier 1.0, sent a real two-turn
HTTP conversation with a prompt engineered to land exactly on a block
boundary. Turn 1: usage.total_tokens=96 (32 prompt + 64 output),
cached=0. Turn 2 (embedding turn 1's exact question, exact generated
reply, plus a new question): usage.prompt_tokens_details.cached_tokens=96
— EXACTLY turn 1's total, meaning the entire first turn including its
generated output was recognized as fully cached, live, end to end.
/metrics confirmed nonzero vllm:gpu_cache_usage_perc, showing output
blocks occupying real pool capacity.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rt98zb2r3b7HEyFcJ4ZTxf
anton's addition to the subtree-blocks badge: each node also shows its DEPTH IN BLOCKS — real blocks from root through the END of this row, inclusive. A compressed run of 12 blocks advances depth by 12 in one step, not by 1 per row (the same "count real blocks, not rows" distinction the subtree badge already makes, applied to depth instead of totals). router/internal/policy/cache/tree.go: run gets a blockDepth field, filled TOP-DOWN during compressFrom itself (the opposite direction from subtreeSize/subtreeBlocks, which are bottom-up) — compressFrom now takes the parent's own blockDepth and adds this run's RunLen to get its own end-of-run depth, threading that down to each child's recursive call. router/internal/viz: TreeNode gets BlockDepth (json:"block_depth"), computed in the same walk, no extra pass. page.html: renders it as a second "d N" badge next to the existing "⊂N" subtree badge (both .tsub style, distinct prefix characters so neither reads as the other), widened the reserved badge slot to fit both. Legend updated. Verified: go vet ./kvcache/... ./router/... exits 0; go test ./router/internal/viz/... ./router/internal/policy/cache/... -race passes. New test TestSnapshot_BlockDepthAccountsForCompressedRuns reuses the existing 3-row compressed-run tree (root = 3-block run, two children of 2 and 4 blocks) and asserts root.BlockDepth=3, and each child's BlockDepth = 3 + its own RunLen (5 and 7) — proving depth advances by the full run length through a compression, not by one row at a time. Manually verified live: sent a long (~2880 byte) shared-prefix prompt with two different tails through the real router + two mock backends. curl /router-viz/data showed the shared root as a 2-block compressed row (block_depth=2, matching its own run) with two 1-block divergent children, each correctly showing block_depth=3 (parent's 2 + their own 1) — the accounting formula holding end-to-end through real traffic, not just the unit test. Sequencing note: this crossed with the earlier tokenizer/output-KV work (fb5e0b4), which had already landed by the time this request arrived — landing it now, before starting the output-KV pin-at-admission refinement anton also requested, per the "depth badge first, anton is watching" instruction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rt98zb2r3b7HEyFcJ4ZTxf
…arget's own tokenizer Supersedes the --replay-chars-per-token ratio approximation with exact token-targeted sizing: indexes the replay corpus once at startup against the first target endpoint's POST /tokenize, then binary-searches that index per block so synthesized content lands on the capture's exact token count instead of an approximate chars/token ratio. Falls back to ratio/byte sizing when the flag is off or a block has no captured Tokens.
…ction change Direction change from anton: replay content fidelity fixes belong in the mock server's accounting (owned by another agent), not client-side exact token sizing. Restores benchmark/ and cli/ to the state at cee6911 (--replay-chars-per-token, which stays as an unused-but-harmless option), undoing the 5878f20 commit's HTTP-tokenizer-oracle work.
…e-endpoint replay poster The single-endpoint fallback poster (the common router-replay path) never received limitContext or replayCharsPerToken, making both flags silent no-ops in every single-endpoint run. Originally caught and fixed inside the abandoned 5878f20; re-applied standalone after ae9042b reverted that commit wholesale. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rt98zb2r3b7HEyFcJ4ZTxf
Output-KV blocks now exert in-flight capacity pressure for a request's whole simulated duration instead of appearing only when it finishes. Engine.PinOutputBlocks computes the request's output-KV chain (content is fully deterministic once max_tokens resolves, so this needs no generation to have "happened" yet) and pins it via RecordAndPin alongside Admit's own prompt pin, released together at completion — closer to real vLLM's decode growing a running request's allocation throughout generation, and errs conservative on early disconnect. Admit's signature is unchanged (still called directly by ~10 existing tests); PinOutputBlocks is a separate, composable call made immediately alongside it in the handlers, before any simulated latency, so the same reservation covers both the non-streaming and SSE paths without needing per-path timing logic. AppendOutputBlocks (the old commit-at-completion path) is removed; sse.go's streamChat/streamCompletion no longer need a units parameter now that pinning happens upfront in the caller. Verified live: gpu_cache_usage_perc rises immediately at admission (before a slow request's simulated generation finishes), and unrelated unpinned cache gets evicted under capacity pressure from concurrent in-flight requests' output blocks alone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rt98zb2r3b7HEyFcJ4ZTxf
Each series instance's poster fired its own /v1/models discovery GET. With --limit-context now actually retiring oversized sessions without any HTTP, series churn through the queue fast enough at startup that hundreds of concurrent discovery GETs flooded the router past its request cap, shedding 503s that cascaded into per-request errors and instantly-terminated runs. One discovery per endpoint per process. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rt98zb2r3b7HEyFcJ4ZTxf
Cancelled: at this workload's output sizes (avg ~229 tokens ≈ 1 block per request), pin-at-admission's in-flight-pressure difference vs completion-time append is negligible, and the completion-time model (fb5e0b4) is already live-verified. Restoring these 5 files to their exact fb5e0b4 content (byte-for-byte) rather than using git revert, per this repo's forward-only convention — Engine.AppendOutputBlocks replaces PinOutputBlocks/outputChain, handlers.go/sse.go go back to appending output-KV blocks after generation completes instead of pinning them alongside the prompt at admission. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rt98zb2r3b7HEyFcJ4ZTxf
A --limit-context skip (or context-overflow retirement) consumed a totalEmitted slot but never completes; the --total terminator waits on totalCompleted, so once enough sessions retired, emission closed at --total emitted while completions could never reach it — the run sat drained (active=0, in_flight=0) forever. Skips now decrement totalEmitted so another request can use the slot, and exactly --total requests are genuinely attempted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rt98zb2r3b7HEyFcJ4ZTxf
…ion shed
Emulates, at the router, the per-backend concurrency level at which vLLM
itself would 429 — so a lower ceiling (e.g. 32 against a fleet running
WEKA_MAX_CONCURRENT_REQUESTS=48) can be tested without restarting any
backend. 0 disables it (unchanged default behavior).
candidates() now filters at the single site every policy's candidate set
is built from (affinity and fallback alike inherit the cap without
knowing it exists), and returns the pre-cap healthy count alongside the
capped list so the caller can tell "no healthy backend" (503, an outage)
from "every healthy backend is saturated" (429, transient — distinct
from the existing router-wide MaxConcurrentRequests 503 shed too).
Readiness deliberately keeps using the pre-cap healthy count: a fully
saturated router is still ready to receive traffic, it just sheds with
429 instead of going NotReady.
New metrics: router_saturation_rejects_total (fleet-wide) and
router_backend_cap_exceeded_total{backend} (which backend is
saturating), both wired through candidates()/inferenceHandler so the
hack/ dead-metric fence is satisfied.
Tests: an httptest scenario with 2 slow backends and cap=1 (each of two
concurrent requests lands on a different backend by construction — the
second is forced off the first once it's at cap, not by load-balance
luck — a third gets 429+Retry-After, and admission resumes once one
completes), plus two policy-level tests proving an at-cap backend is
excluded from both the cache-affinity path and the fallback
(least-outstanding) path. Verified live against the built binaries too.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rt98zb2r3b7HEyFcJ4ZTxf
The "active/POLICY" stat tile never changed once prefix-cache-aware
routing was always on, so it was pure noise. Removed it from
renderTiles(); policy_active still flows through the JSON unchanged.
In its place, a full-width banner ("router is not running a cache-aware
policy — no KV map available") renders ONLY when policy_active is false,
and now takes over from the tiles row and tree panel entirely — those
have nothing meaningful to show without a policy either (no backends, no
tree, no avg-copies), so hiding them behind the banner is clearer than a
row of zeros sitting above an empty tree. renderTree's old
!policy_active branch is dead code now that the whole panel is hidden in
that case, so it's removed; the remaining two empty-tree messages (no
backends yet / no requests yet) still apply once a policy IS active.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rt98zb2r3b7HEyFcJ4ZTxf
Mirrors the parent wllm repo's .ainav/ convention (index.md + topic dirs, terse/operational/command-first, no environment-specific values) for this repo's own operational knowledge that isn't obvious from the code alone: - router-testing/ — the centerpiece: build + launch the mock vLLM fleet + wllm-router + run a replay benchmark against it end to end, with the readiness-gate/one-arm-at-a-time/restart-between-runs gotchas that aren't discoverable from any single command's --help. calibration.md carries the 2026-08-06 DeepSeek-V2-Lite golden-replay rate-fitting facts and the uniform-speedup recipe for fast iteration. replay-notes.md documents replay-v3 file layout and three recent router-replay correctness fixes (skip emission-budget return, cached per-endpoint model discovery, phantom-error-free teardown). - viz/ — /router-viz: merged prefix tree, block-depth/subtree badges, avg_copies duplication metric, UI-controls-not-query-params, policy-inactive banner. - architecture/ — kvcache as the one trie shared by router prediction, mock ground truth, and the benchmark estimator; the two cache-aware policies and where candidate filtering actually lives; the mock engine's admission/pin/output-KV model; the router/hack fence tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rt98zb2r3b7HEyFcJ4ZTxf
Real vLLM's prefill throughput is a genuinely shared per-instance GPU resource, not an independent rate per request. The mock previously computed each request's TTFT as a pure, uncontended duration (BaseLatency + tokens/rate), so N concurrent prefills each got a private rate and throughput scaled linearly with concurrency — the closed-loop dynamics were wrong, and mock warm/cached share converged (46/48) where a real fleet under load spreads (51/35). Queueing for a shared resource, not per-token cost, is what makes deep cold turns expensive fleet-wide under concurrency. --cold-input-tps/--cached-input-tps are now the INSTANCE's aggregate rates: prefillScheduler (prefill.go) is a per-instance egalitarian processor-sharing scheduler — N concurrently-prefilling requests each drain their solo-rate-equivalent work at 1/N of wall-clock rate, so the instance's aggregate throughput stays conserved regardless of N. Event-driven per the spec: a single per-instance goroutine owns all scheduling state (no mutex needed — share memory by communicating), settling every active job's remaining work only at a join, a leave, or a completion, with exactly one "next completion" timer armed at a time via clock.Clock (not raw time.Now/Sleep, per the router/hack fence). Engine.Latency (a pure duration) is replaced by three pieces: PrefillWork (pure job-size computation), AwaitTTFT (blocks on the scheduler — genuinely contended), and DecodeDuration (unchanged per-request lump-sum decode, paired with the existing OutputTokenInterval for streaming). Decode/output stays per-request, undisturbed: vLLM's continuous batching keeps per-request decode rate ~constant until batch saturation, so that approximation still holds. Tests (prefill_test.go): a solo job completes in ~its own work; 4 identical concurrent jobs each complete in ~4x (aggregate rate conserved, ±10%); a short job joining mid-way through a long job delays it by exactly the short job's own solo size (worked out from PS's work-conservation property); AwaitTTFT's actual elapsed time is base+prefill completion for an uncontended request; plus one beyond the spec covering the cancellation/leave path (a disconnected client must stop consuming its rate share, or it leaks forever). Verified live against the built binary: a solo cold request took ~0.27s, 4 identical concurrent cold requests each took ~1.2s. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rt98zb2r3b7HEyFcJ4ZTxf
tigrawap
force-pushed
the
feat/router-in-wekai
branch
from
August 6, 2026 14:20
fb182b1 to
8764e9a
Compare
…t/router-in-wekai
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // it) — same convention router/internal/testutil/mockvllm's SSE script uses | ||
| // ("t%d"). | ||
| func syntheticTokens(n int) []string { | ||
| out := make([]string, n) |
Two staleness fixes anton flagged, both in router-testing/: calibration.md — the rate table assumed independent PER-REQUEST prefill rates; 8764e9a made --cold-input-tps/--cached-input-tps INSTANCE-AGGREGATE (processor-shared), which invalidates the old 16,174/33,297 fit. Re-fit: base 100ms, cold 64,000 tok/s, cached 133,000 tok/s (both per-instance aggregate), output 74 tok/s (per-request, unchanged — decode isn't contended). Also documents that run LENGTH, not just rate, determines whether cache-hit numbers are meaningful: --total 4800 at x10 under-thrashes (~15-20pts optimistic); --total 30000 (~6min at x10) reached warm 71.2%/cached 38.7%, matching the real fleet's warm/cached spread shape (51.3/35.6 on the 4,800-req reference). Short runs are workload-realistic only; long runs are cache-realistic. index.md — added the "Validated Standard Recipe" as a prominent worked example ahead of the step-by-step walkthrough: 4 mock instances at their own --max-concurrency 256 (backend-side default, unchanged), --max-node-concurrency 32 at the ROUTER (the cap under test lives there, not on the mock instances — spelled out explicitly since it's an easy mixup), client --concurrency 128 (= nodes x node-cap), --hot-series-concurrency 0 (off, for precise measurement), --total 30000 at x10 rates, ~6 min runtime. Documents that /router-viz is per-router-instance (second router for A/B needs its own --metrics-listen, conventionally 29001) and that a stalled-looking drain tail (in_flight trickling to 0 while giant-decode requests finish, up to ~43s each at x10) is normal completion, not a hang. The existing step-5 example is kept as an illustrative short run, now explicitly marked as such with a pointer to the validated recipe. Every flag re-verified against router/cmd/mock-vllm/main.go and cli/benchmark_options.go source, not transcribed from memory. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rt98zb2r3b7HEyFcJ4ZTxf
…t/router-in-wekai
One tree whose runs record WHICH backends hold them, replacing the reconstruct-by-querying-N-tries approach the existing cache policies use. This is the foundation for the prefix-cache-split policy; nothing uses it yet. Holder sets are a markSet over a growable []uint64, so there is no ceiling on fleet size and no bitwise code outside that one file. Runs are radix compressed, children are a sorted binary-searched slice rather than a map for the memory reason kvcache documents, and the forest is sharded 16 ways by first block hash -- lossless, since a walk always starts at the root child keyed by block 0. Sharding separates unrelated prefix families; it does NOT spread a single dominant system prompt, which is carried by the RWMutex and short critical sections instead. commit() marks the serving backend on EVERY run along the path, not only the deepest. That is explicit in the architect's notes and is the whole mechanism behind a split converging. Model isolation is structural, one root sentinel per model key, because the gateway filters candidates by DialectID and never by Model. Eviction is tail-only and TTL-based, cascading upward through dead chains; a run with any remaining child is never touched. Backend removal clears a slot everywhere before freeing it for reuse, so a new backend cannot inherit prefixes it never served. Two invariants are asserted under randomised operation sequences, because a later change could silently break either: the tail set is exactly the childless runs, and a descendant's holders are always a subset of its parent's -- the latter is what makes "deepest marked run" also mean "smallest, most specific candidate pool". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Routes by prefix affinity over the shared marked tree. Not selectable yet; wiring follows. Four tiers. cache: some run on the path is held by an available candidate, route to the least-loaded holder. split: holders exist but all are saturated or gone, so route outside the holder set past the guard AND mark the target -- the holder set grows under pressure instead of affinity being abandoned. overflow: holders are saturated and nothing clears the guard, so use idle capacity WITHOUT marking. load: nothing is marked anywhere. There is no threshold. The deepest run with an available holder wins however small a share of the request it is. The existing prefix-cache-candidates policy gates on cached/total over the whole current request, which structurally penalises exactly the long-running sessions most worth pinning -- their shared prefix is fixed while their total keeps growing -- and is the direct cause of the low predicted fractions observed in production. The overflow tier diverges from the reference simulator, which rejects here; that is why the simulator's own verdict function reports MARGINAL, since at a 20% guard every backend between 80% and 100% of its limit is idle but unusable. The architect's stated reason for the guard is to stop every backend being marked as holding every prefix -- that is about MARKING, not about refusing to serve. Separating the two keeps the guard's real job and makes premature rejection impossible. Not yet reviewed with him. This policy never rejects. Admission stays with the gateway, which already 429s exactly when no backend is under its limit. The mark/no-mark bit travels from Select to Commit on a new opaque RoutingRequest.PolicyState field; Commit defaults to marking when absent, since losing affinity silently is worse than an extra holder. RouteDecisions gains the split and overflow labels. Consumers must aggregate by label rather than enumerate members. New collectors cover splits, overflows, mean holders per block, anchor depth, pool size, tree size and eviction; avg copies is the tripwire for holder sets that only ever grow. /router-viz needs far less work than it did: the tree is natively the shape the page wants, so the per-poll cross-backend merge disappears and a run's markSet expands straight into TreeNode.Present. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds prefix-cache-split to ValidPolicies and buildPolicy, two knobs (--cache-split-guard, --cache-tail-ttl), and a TTL sweep goroutine driven off the clock abstraction so a routing decision never pays for eviction. The policy is additive and off by default; both existing cache policies are untouched. --max-node-concurrency is now MANDATORY for this policy: startup fails naming the flag. It is both the gateway's admission cap and the limit the split guard is measured against, so it has to mean one thing. Left unset, the gateway applies no cap at all while every other capacity source reads 1 (--backends carries no capacity field, max_inflight_per_backend defaults to 1, and Backend.Capacity clamps below 1 up to 1) -- so the guard would be computed against a meaningless number and nothing would say so. Inventing a second default that could disagree with the gateway's own filter would be worse. Operators enabling this policy should know that setting --max-node-concurrency turns on per-backend admission for EVERY policy, not just this one: the router will begin returning 429 all_backends_at_capacity where it previously let requests queue at the backend. Also adds policy/affinity to the core package list in the does-not-import-dialects fence, which would otherwise not have guarded it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An offline replay of agentic traffic against the real policy code, reproducing the acceptance criterion encoded in the reference simulator's renderVerdict(): a 429 before every node has reached its concurrency limit is a FAIL, and a rejection issued while idle slots existed anywhere is a premature one. Runs in about a second, no processes and no network. Three differences from the reference simulator, each deliberate. Admission is the gateway's, so the harness reproduces its candidate filter and a rejection means zero idle slots fleet-wide -- which is what makes premature rejection measurable rather than definitional. Commit is delayed by a prefill, because the router commits from ModifyResponse once response headers arrive, so every request issued inside that window still sees the pre-commit tree; the simulator commits at issue and cannot show this at all. And backends carry ground truth, tracked separately from what the tree PREDICTS, so prediction accuracy is measured rather than assumed. Measured at 8 nodes x 32 concurrency. Under capacity: 93.9% prefix hit rate against least-outstanding's 72.7%, with 1.05 mean holders per block. At 164% oversubscription: zero premature rejections, 100% peak utilisation, and holder sets that grow to ~3.4 copies because sessions genuinely cannot stay on one backend at that load. A cross-check runs the reference's faithful 3-tier ladder over the same seed. It holds a markedly better hit rate (92.3% vs 71.1%) because it refuses to serve cold, at the cost of 18,952 of 21,963 rejections landing while capacity was idle, against zero for this policy. Total work completed is within 1% either way, so this is not a throughput argument in either direction: it is client-visible 429s versus cache hit rate. That trade is reported, not asserted, because which side is preferable is a product decision. Note the workload is seeded and replays identically but run totals still move by about a percent, since every policy breaks ties with the package-level reservoir sampler LB-11 requires and that is deliberately not seeded. Every assertion is therefore a bound or a comparison between two arms of the same run, never a transcribed number. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Records the three ways this policy differs operationally from the others: --max-node-concurrency is mandatory, there are two extra knobs, and it never returns 429 itself because admission stays with the gateway. Also records the trap found while validating it: a short smoke run does NOT discriminate the policies. With a large shared system prompt the existing 0.5 threshold is satisfied anyway and both arms score identically. The regimes where they diverge are a MODEST shared prefix inside large requests (0 vs 192 cache decisions out of ~200 in one measured run) and saturation with skewed load, where the split and overflow tiers actually fire. Points at the offline fleet simulation as the place to start, since it replays the same workload in about a second before anyone boots a fleet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Ofer Kiselov Nahman <ofer.kiselovnahman@weka.io>
The generic .dockerignore (added for the router image, 2026-08-06) excluded main.go/cli/benchmark/llm from EVERY build context made from this directory — including the parent repo's wekai-src named context in create_release.sh, whose wekai CLI stage then failed with 'no Go files in /tmp/wekai-build'. BuildKit's per-Dockerfile ignore (Dockerfile.wllm-router.dockerignore) keeps the router context slim while restoring the full tree for every other consumer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rt98zb2r3b7HEyFcJ4ZTxf
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.