dev: local-stack — full tier-routing dev environment (real CPU engine, real token counts) - #862
dev: local-stack — full tier-routing dev environment (real CPU engine, real token counts)#862PierreLeGuen wants to merge 1 commit into
Conversation
Review —
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a local development stack (dev/local-stack) to test the two-tier context routing of cloud-api against a CPU-based llama.cpp engine. It includes Docker Compose configurations, seeding and demo scripts, and a Python-based tier shim to emulate SGLang fleets. The review feedback highlights several key improvements for the local stack's robustness: flushing the HTTP response stream immediately in the tier shim to support real-time SSE streaming, properly catching and propagating upstream HTTP errors instead of masking them as internal server errors, and implementing robust trap-based cleanups in the shell scripts to prevent temporary state files like ENGINE_DOWN and SATURATE-long from persisting upon interruption.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| while chunk := r.read(8192): | ||
| self.wfile.write(chunk) | ||
| sent += len(chunk) |
There was a problem hiding this comment.
In Python's http.server, self.wfile is buffered by default. For streaming/SSE responses, chunks written to self.wfile must be flushed immediately using self.wfile.flush(). Otherwise, the response will be buffered and sent all at once when the stream finishes, defeating the purpose of real-time streaming.
| while chunk := r.read(8192): | |
| self.wfile.write(chunk) | |
| sent += len(chunk) | |
| while chunk := r.read(8192): | |
| self.wfile.write(chunk) | |
| self.wfile.flush() | |
| sent += len(chunk) |
| import os | ||
| import sys | ||
| import threading | ||
| import urllib.request |
There was a problem hiding this comment.
Import urllib.error to handle HTTP errors from the upstream engine properly.
| import urllib.request | |
| import urllib.error | |
| import urllib.request |
| def do_POST(self): | ||
| try: | ||
| if self.path == "/v1/tokenize": | ||
| payload = json.loads(self._read_body()) | ||
| n = count_tokens(payload.get("prompt") or payload.get("content") or "") | ||
| log(f"POST /v1/tokenize -> count={n}") | ||
| self._json(200, {"count": n}) | ||
| elif self.path == "/v1/chat/completions": | ||
| self.handle_completion() | ||
| else: | ||
| self._json(404, {"error": {"message": f"no route {self.path}"}}) | ||
| except Exception as e: # keep the shim alive; surface as a 500 | ||
| log(f"ERROR {self.path}: {e}") | ||
| try: | ||
| self._json(500, {"error": {"message": str(e)}}) | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
When the upstream engine returns a non-2xx error (such as a 400 Bad Request or 422 Unprocessable Entity), urllib.request.urlopen raises a urllib.error.HTTPError. Currently, this is caught by the generic except Exception block and mapped to a 500 Internal Server Error, masking the original status code and JSON error body. Catching urllib.error.HTTPError explicitly and propagating the status code and response body back to the client will greatly improve debugging and API compatibility.
def do_POST(self):
try:
if self.path == "/v1/tokenize":
payload = json.loads(self._read_body())
n = count_tokens(payload.get("prompt") or payload.get("content") or "")
log(f"POST /v1/tokenize -> count={n}")
self._json(200, {"count": n})
elif self.path == "/v1/chat/completions":
self.handle_completion()
else:
self._json(404, {"error": {"message": f"no route {self.path}"}})
except urllib.error.HTTPError as e:
log(f"HTTPError {self.path}: {e.code}")
self.send_response(e.code)
self.send_header("Content-Type", e.headers.get("Content-Type", "application/json"))
body = e.read()
self.send_header("Content-Length", str(len(body)))
self.end_headers()
self.wfile.write(body)
except Exception as e: # keep the shim alive; surface as a 500
log(f"ERROR {self.path}: {e}")
try:
self._json(500, {"error": {"message": str(e)}})
except Exception:
pass| cleanup_all() { | ||
| [ -n "${MP_PID:-}" ] && kill "$MP_PID" 2>/dev/null || true | ||
| [ -n "${STUB_PID:-}" ] && kill "$STUB_PID" 2>/dev/null || true | ||
| [ -n "${ENGINE_PID:-}" ] && kill "$ENGINE_PID" 2>/dev/null || true | ||
| } |
There was a problem hiding this comment.
If the script fails or is interrupted after touch ENGINE_DOWN but before rm -f ENGINE_DOWN, the ENGINE_DOWN file will persist in the directory. This will cause subsequent runs of the script to fail because the engine will be considered down from the start. Adding rm -f ENGINE_DOWN to the cleanup_all trap function ensures it is always cleaned up on exit.
| cleanup_all() { | |
| [ -n "${MP_PID:-}" ] && kill "$MP_PID" 2>/dev/null || true | |
| [ -n "${STUB_PID:-}" ] && kill "$STUB_PID" 2>/dev/null || true | |
| [ -n "${ENGINE_PID:-}" ] && kill "$ENGINE_PID" 2>/dev/null || true | |
| } | |
| cleanup_all() { | |
| [ -n "${MP_PID:-}" ] && kill "$MP_PID" 2>/dev/null || true | |
| [ -n "${STUB_PID:-}" ] && kill "$STUB_PID" 2>/dev/null || true | |
| [ -n "${ENGINE_PID:-}" ] && kill "$ENGINE_PID" 2>/dev/null || true | |
| rm -f ENGINE_DOWN | |
| } |
| set -uo pipefail | ||
| cd "$(dirname "$0")" |
There was a problem hiding this comment.
If the script is interrupted or fails after touch SATURATE-long but before rm -f SATURATE-long, the saturation file will persist, leaving the long tier saturated for subsequent runs. Adding an EXIT trap to clean up SATURATE-long ensures the environment is always left in a clean state.
| set -uo pipefail | |
| cd "$(dirname "$0")" | |
| set -uo pipefail | |
| cd "$(dirname "$0")" | |
| trap 'rm -f SATURATE-long' EXIT |
370d44d to
de745f0
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f56ad99d8b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| DATABASE_NAME=platform_api | ||
| DATABASE_USERNAME=postgres | ||
| DATABASE_PASSWORD=postgres | ||
| DATABASE_MAX_CONNECTIONS=5 |
There was a problem hiding this comment.
When running the documented make api, this env file leaves DATABASE_TLS_ENABLED unset; DatabaseConfig::from_env defaults it to true, and with POSTGRES_PRIMARY_APP_ID=postgres-test the API builds a native-TLS pool for the local database. The postgres:16 service in this stack is started without any SSL configuration, so the API fails to connect/run migrations before seed or demo can work. Add DATABASE_TLS_ENABLED=false for this local stack.
Useful? React with 👍 / 👎.
| "maxOutputLength": 512, | ||
| "verifiable": true, | ||
| "isActive": true, | ||
| "providerType": "vllm", |
There was a problem hiding this comment.
For this seeded providerType: "vllm" model, omitting attestationSupported makes the repository default attestation_supported to true for non-external providers, but the local tier shims do not implement /v1/signature. In the streaming demo, cloud-api therefore tries to fetch provider signatures in the finalization path before [DONE], causing avoidable timeout delay and error logs for every local streaming completion. Seed this model with attestationSupported: false or add a signature stub.
Useful? React with 👍 / 👎.
| trap cleanup_all EXIT | ||
|
|
||
| say "building model-proxy from $MODEL_PROXY_DIR" | ||
| (cd "$MODEL_PROXY_DIR" && cargo build -q 2>&1 | tail -1 || true) |
There was a problem hiding this comment.
If this checkout already has a previous target/debug/model-proxy, a failing cargo build is hidden by || true, and the executable check below still passes against the stale binary. That makes make proxy-demo capable of validating the wrong model-proxy revision after a compile error, which undermines the control-plane demo. Let the build failure abort instead of continuing.
Useful? React with 👍 / 👎.
| --ctx-size 8192 | ||
| --parallel 2 |
There was a problem hiding this comment.
The local stack advertises an 8000-token long tier, but llama.cpp has to share/partition its --ctx-size budget across the configured parallel slots. With --ctx-size 8192 and --parallel 2, long-tier requests near the advertised 8000-token window can pass the shim's check and then be rejected by the engine (or fail under concurrent use) because there is not enough per-request context behind it. Use one slot or increase the engine context so the backing server can actually serve the shim's declared window.
Useful? React with 👍 / 👎.
Reusable local dev environment that runs the real cloud-api against a REAL
inference engine on CPU (llama.cpp + Qwen2.5-0.5B GGUF, docker) with the
GLM-5.2-style two-tier context topology:
curl -> cloud-api :13000 (mock auth, ephemeral signing keys)
|- base tier -> tier_shim :18100 (ctx 1000) --+
'- long tier -> tier_shim :18101 (ctx 8000) --+-> llama.cpp :18090
postgres :15432 (docker, isolated from the e2e test-postgres)
The tier shims play the per-tier SGLang fleets: real completions and REAL
token counts through the vLLM/SGLang-shaped /v1/tokenize, SGLang-phrased
context-window 400s (so the fall-through matcher fires as in prod),
response .model tier tags (+base/+long), and a saturation drill
(touch SATURATE-long -> 503). make up/api/seed/demo/down; demo prints an
8-check pass/fail table (small->base, oversize->long, boundary->exact
count decides, streaming, saturated-long->retryable 429).
model-proxy-demo/: drives the REAL model-proxy binary through the
long-context registration flow with fast intervals — dual same-IP probes
sharing one routed backend, the probe-cleanup ownership guard
(model-proxy#42), and the health-gated discovery stub + non-2xx circuit
breaker (cvm-compose-files#129 + model-proxy#42 interplay).
Intentionally NOT local (documented in the README): the TLS/SNI data path
through model-proxy (providers fail closed on unattested HTTPS — the TEE
trust model) and the Chutes wire client (ML-KEM + TDX verification;
covered at the pool boundary in the e2e suite).
de745f0 to
88bb2d8
Compare
|
Rebased onto |
Evrard-Nil
left a comment
Evrard-Nil
left a comment
There was a problem hiding this comment.
Thorough read of all 11 files (729 additions, zero src/ / Cargo.* changes). No production code paths are touched — everything lives under dev/local-stack/.
What this does
Adds a self-contained local dev environment for exercising the GLM-5.2-style two-tier context routing end-to-end on one machine without staging, GPUs, or a TEE:
docker-compose.yml— Postgres 16 + llama.cpp (Qwen2.5-0.5B GGUF, CPU) on isolated portstier_shim.py— Python HTTP shim that emulates a per-tier SGLang fleet: real/v1/tokenizecounts, SGLang-phrased context-400s, saturation drill via sentinel fileseed.sh— bootstraps mock admin user, two-tier model config, org/credits/API key via the admin APIdemo.sh— 8-check table: small→base, oversize→long, boundary (exact tokenize count), streaming, saturated-long→retryable 429/5xxmodel-proxy-demo/— drives the REAL model-proxy binary through the dual-probe / ownership-guard / circuit-breaker flow (model-proxy#42 already merged)
Security
- All local ports are bound to
127.0.0.1(loopback only). The llama.cpp container uses--host 0.0.0.0internally but the host-side binding is127.0.0.1:18090. env.local-stackvalues are intentional dev placeholders (local-stack-not-a-secret,local-stack-inference-key, zero S3 key) with clear comments. No real credentials..api-keyis gitignored. All generated state (cache/,run/, logs, PIDs,SATURATE-*) is gitignored.DEV=1uses ephemeral signing keys — correct for a non-TEE machine; this path is already guarded in the existing codebase to be debug-build-only.
Minor findings (non-blocking for tooling code)
-
make upcalled twice leaves orphan shims. Theuptarget spawns new shims without killing any existing ones on the same ports. The new processes will fail to bind (port already in use) and the old ones keep running silently —make upreports green because it health-checks the still-running originals.make down && make upis the correct re-run path, but a shortkill $(cat .shim-*.pid) 2>/dev/null || trueguard at the top ofupwould prevent confusion. Not blocking. -
Upstream HTTP errors masked in
tier_shim.py.urllib.request.urlopen()raisesurllib.error.HTTPErroron non-2xx. This falls through to the blanketexcept Exceptionindo_POSTand is re-served as a 500 with the Python exception string. The actual upstream status code and body are lost. For a dev tool this is tolerable (the error text mentions the upstream URL), but catchingHTTPErrorseparately and forwarding the status + body would make debugging easier. -
demo.shmissing trap forSATURATE-long. If the script is interrupted betweentouch SATURATE-long(line 53) andrm -f SATURATE-long(line 56), the sentinel file lingers and subsequent demo runs fail test 2 (oversize→long).run.shhas the same gap forENGINE_DOWN. Both could be fixed with atrap 'rm -f SATURATE-long' EXIT/trap 'rm -f ENGINE_DOWN' EXIT. The Gemini review flagged this; it appears the current HEAD didn't address it. -
ENGINE_PIDinrun.shcleanup is dead code.cleanup_all()references$ENGINE_PIDbut it is never assigned — the "fake engine" role is handled byprobe_stub.pyvia theENGINE_DOWNfile, not a separate process. The variable is harmless (:-}guard) but confusing. Worth removing. -
demo.shusesset -uobut not-e. Intentional (partial failures are accumulated into the PASS/FAIL table rather than aborting), but worth a comment so readers don't assume it's an oversight.
None of these block the PR — they're all contained within the dev tooling directory and have no impact on CI or production paths.
Verdict
Approved. The stacking dependency (#860) is already merged to main, the branch rebases cleanly, CI (security_audit) is green, no production code is touched, no real credentials, all sockets loopback-only. The tier shim correctly mirrors the SGLang phrasing that cloud-api's fall-through matcher depends on. The findings above are worth a follow-up but don't block merging dev tooling.
2 participants
Stacked on #860 (rebase onto main once it merges). Adds
dev/local-stack/: a reusable one-machine environment that runs the real cloud-api against a real inference engine on CPU with the GLM-5.2-style two-tier context topology — no staging, no GPUs, no TEE.make up && make api && make seed && make demo— the demo prints an 8-check table, all green on this machine:+base(real Qwen reply; long tier sees zero traffic)+long/v1/tokenizecount decides — shim log showscount=801keeping it on base where the heuristic alone (~1,201) would have mis-routedtouch SATURATE-long→ oversize returns retryable 429, never a context-400 (the feat: context-length tier routing (262k fleet vs 1M tier, best-effort Chutes overflow) #860 clobber fix, observable with curl)The tier shims emulate the per-tier SGLang fleets faithfully: vLLM/SGLang-shaped
/v1/tokenizebacked by the engine's real tokenizer, SGLang-phrased context-window 400s (the fall-through matcher fires exactly as in prod),.modeltier tags, saturation toggle. Engine-agnostic: swap a vLLM-CPU image into the compose file and nothing else changes.make proxy-demodrives the REAL model-proxy binary (checkout path viaMODEL_PROXY_DIR) through the long-context registration flow — dual same-IP probes sharing one routed backend, the probe-cleanup ownership guard, and the health-gated stub + non-2xx breaker. 9/9 against nearai/model-proxy#42; against unpatched main the shared backend measurably drops tototal_backends: 0for a full discovery cycle on probe unregister (the #42 repro).README documents what is intentionally NOT local: the TLS/SNI data path through model-proxy (providers fail closed on unattested HTTPS — the TEE trust model working) and the Chutes wire client (ML-KEM + TDX; covered at the pool boundary by the e2e suite in #860).
No
src/changes — tooling only.