Gateway P2.5 — final whole-branch review (`feat/gateway-p2.5-gpuserver-facade`)

Branch HEAD 7f534c6, base 408477d. Reviewed against the engine's shipped client
(ahu-ocr-tidyup/backend/src/lib/gpu-job-client.ts) and the gpu-server it targets
(ahu-ocr-tidyup/gpu-server/app/main.py + schemas/responses.py).

Verdict: NEEDS FIX WAVE

The design is sound and the local-coordination path is faithful and well-tested.
But the branch is not mergeable as-is because the façade is broken under
coordination: redis — the HA/production mode. One must-fix (Redis persistence
of submit_path) makes every OCR job fail before the gpu-server is contacted.
Fix that + one more Redis field + one config default, then ship.

Must-fix list

Job.SubmitPath is not persisted by the Redis store — façade non-functional under coordination: redis. (Critical)
Job.ContentType is not persisted by the Redis store — multipart POST /jobs uploads corrupted under Redis. (Critical; pre-existing P2 gap that P2.5 now depends on)
max_body_bytes default (32 MB) < gpu-server limit (50 MB) — valid large docs get 413 through the gateway that the gpu-server would accept. (Important; config-only fix)

Recommend folding the error-detail fidelity fix (below) into the same wave since it is the same class of change.

Compatibility matrix (engine ↔ façade) — the 6 crux points

#	Point	Local coord.	Redis coord.
1	Submit: all 12 per-op paths accept the engine body/Content-Type, no `?model=` required, return `202 {job_id}`	MATCH	MATCH (submit itself works; failure is downstream)
2	Forward: adapter POSTs verbatim body to `endpoint+SubmitPath`, reads `{job_id}`, polls `/jobs/{gpuId}`	MATCH	MISMATCH — `SubmitPath` lost in Redis → `NO_SUBMIT_PATH`, gpu-server never called
3	Poll shape: top-level `job_id/status/stage/progress/result/error/queue_position/eta_ms`; unknown→404; completed→`result`; failed→`error`	MATCH (shape) / see fidelity gap for `error` content	same
4	Idempotency: key required (400 if absent — cannot fire for the engine, which always derives a sha256 key); dedup→same `job_id`, ONE gpu-server submit	MATCH	MATCH
5	JOB_UNKNOWN resubmit: engine resubmits same key on 404 → dedup returns existing job, or re-creates if truly lost	MATCH	MATCH
6	Remaining mismatch that breaks the flip	large-doc 413 (point 3 below), error-detail content	Redis SubmitPath/ContentType (breaks flip entirely)

Points 1, 4, 5 verified end-to-end by internal/server/facade_test.go (all 12 paths
through the real mux against a stub gpu-server; idempotent-resubmit → one submit;
404 JOB_UNKNOWN; missing-key 400). Point 3 shape verified by the completed/failed
tests. All tests use jobs.NewLocal(), so the Redis path (points 2/6) is entirely
untested — which is how the serialization gap slipped through.

Cross-cutting findings

Critical

C1 — SubmitPath not persisted in Redis store.
internal/jobs/redisstore.go fieldsFromJob (~L513) and jobFromHash (~L547) omit
submit_path. Under coordination: redis, a façade job is written to a Redis hash
on Create and re-hydrated by ClaimNext→jobFromHash before the worker runs it.
The re-hydrated job has SubmitPath == "", so GPUServerJobAdapter.submit
(adapter_gpuserver.go:131) returns NO_SUBMIT_PATH immediately — every OCR job
fails and the gpu-server is never contacted. The LocalStore survives only because
clone() copies the whole struct. The example config ships coordination: local but
the HA pair (and CONVENTIONS) runs redis; this is the production configuration.
Fix: add "submit_path": j.SubmitPath to fieldsFromJob and SubmitPath: m["submit_path"] to jobFromHash.

C2 — ContentType not persisted in Redis store.
Same two functions omit content_type. Pre-existing P2 gap (also degrades azure-di
multipart under Redis), but P2.5's generic POST /jobs path relies on it: the engine
uploads multipart/form-data; boundary=… + file/document_type. Under Redis the
re-hydrated job has ContentType == "", the adapter defaults to application/json,
and the forwarded multipart bytes are mis-framed → gpu-server 422. Per-op JSON paths
survive by luck (default already application/json).
Fix: persist/restore content_type alongside C1.

Important

I1 — max_body_bytes default rejects large docs the gpu-server accepts.
Gateway default MaxBodyBytes = 32<<20 (~32 MB, config.go:77); gpu-server
MAX_FILE_SIZE = 50 MB (main.py:27). A 32–50 MB PDF that works today via
GPU_SERVER_URL → gpu-server gets a 413 BODY_TOO_LARGE through the façade
(facade.go:117), surfacing to the engine as GpuJobSubmitFailedError. This breaks
"flip with no other change" for large documents. deploy/gateway.example.yaml does
not set max_body_bytes.
Fix: set max_body_bytes: 52428800 (or higher, to cover multipart overhead) in the example/deploy config, and document it in the flip checklist.

Minor

M1 — Enable-order footgun (not reachable via main.go).
If EnableJobs were called before EnableGPUServerFacade (with a gpu-server
upstream present), jobRoutesMounted would already be set, so POST /jobs +
GET /jobs/{id} keep the generic P2 handlers while the 11 per-op paths use the
façade — a mixed state where per-op jobs poll through the wrong (nested error_code)
shape. main.go calls strictly either/or so this cannot happen in production; the
jobRoutesMounted/facadeMounted guards prevent a mux panic. Worth a one-line
assertion or comment guarding the invariant for future callers.

M2 — Mixed config silently drops the generic submit API.
When a config has both a gpuserver-job upstream and generic ocr-http/classify
job upstreams, main.go mounts only the façade; there is no POST /jobs?model=
route for the generic upstreams (their jobs can only be processed, not submitted via
HTTP). Not a real deployment shape, but undocumented.

Regression + dormancy check

Dormancy: with no gpuserver-job upstream, EnableGPUServerFacade returns
before mounting anything (server.go:215); main.go falls through to EnableJobs.
TestFacadeDormant proves per-op paths 404 and generic POST /jobs?model= still
works. /v1 and /synthesis/v1 proxy paths are untouched. Byte-identical to P2
when dormant — confirmed.
POST /jobs collision: handled by the jobRoutesMounted guard + either/or
dispatch in main.go. No double-registration (no mux panic) and /jobs is never
left unmounted. Confirmed.
Inherited invariants: on-prem enforcement present in both the façade submit
(facade.go:124) and the manager (manager.go:271); slot release on every path
(inherited via process/releaseOnce); panic-recover in process; audit
body-cap + doc_hash + per-op model label (verified by TestFacadeAllPaths
audit assertions); drain→503 with Retry-After (TestFacadeDrainReturns503);
manager started exactly once via the s.jobMgr guard in startManager. All hold.

Fidelity-gap call (failed-poll `error` content)

Call: fix in THIS wave (bundle with the required Redis fix), not defer.

On a failed job the façade returns error = job.ErrorCode (e.g. "GPUSERVER_FAILED",
facade.go:210), not the gpu-server's original error detail string. The adapter
does carry it (AdapterError.Detail = p.Error, adapter_gpuserver.go:224) but
Manager.finishWithPayload persists only the code, so the engine surfaces
"… failed: GPUSERVER_FAILED" to users instead of the real reason.

In isolation this is defensible backlog — the flip still functions (failed jobs
return a non-empty error, the engine's failed-branch works, no hang), and the plan
explicitly marked it acceptable. But since C1/C2 already force a fix wave touching
exactly the store-serialization + finishWithPayload plumbing this fix needs
(add Job.ErrorDetail, persist in both stores, return it as error when present,
falling back to ErrorCode), the marginal cost is near zero and it closes the last
real contract-fidelity gap the OCR integration review flagged. Do it now.

Suite

go build ./... — clean
go vet ./... — clean
go test -race ./... — all packages PASS (config, jobs, pool, registry, proxy,
server, audit, idem). No race reports. Note: façade coverage is Local-store only;
add a Redis-store round-trip test for submit_path/content_type as part of the fix.

One-paragraph assessment

P2.5 is a well-architected, faithful façade over the P2 machinery: all 12 submit
paths route and forward verbatim, idempotency/dedup gives exactly one gpu-server
submit per doc, the poll speaks the gpu-server's top-level shape with a 404
JOB_UNKNOWN the engine keys off, and dormancy is byte-identical to P2 — all
confirmed by a genuinely end-to-end test suite that builds/vets/races clean. It is
not mergeable yet for one reason: the new Job.SubmitPath (and the inherited
ContentType) are never written to the Redis store, so under coordination: redis
— the HA/production mode the deploy targets — every OCR job fails with
NO_SUBMIT_PATH before the gpu-server is ever called, and multipart uploads are
mis-framed. That is a two-line-per-function serialization fix plus a
Redis-round-trip test; bundle the max_body_bytes default bump and the failed-error
detail fidelity fix into the same short wave, and the branch is ready.

Gateway P2.5 — final whole-branch review (feat/gateway-p2.5-gpuserver-facade)