runtime: add the worker-side engine contract

[xiaoyu-xyz]

This PR now carries the placement revision. The head was replaced after review: Engine, Readiness, Report, Answer and EngineError moved from src/frontend to src/runtime as the review asked, the in-process worker and its queue were dropped, and the timeout semantics were unified. The diff below is against current main (7f39ac4), not against the base that was first reviewed — the earlier discussion stays in the thread for context, and this note is the line between it and the current state.

Purpose

Review of the in-process serving work asked for its placement to change: the request lifecycle belongs to the worker-side runtime, not to the frontend. docs/architecture.md gives the frontend "HTTP transport, forwarding, and response delivery" and gives admission, queues and back-pressure to the runtime, and omni-runtime already carries the admission half as SerialScheduler.

This adds the other half — the contract a model implements to be served — and the transport that serves it. It replaces the older placement rather than adding to it; the two branches that carried that placement are closed in favour of this one.

The contract, in src/runtime/src/engine.rs (140 lines)

pub trait Engine: Send + Sync + 'static {
    fn readiness(&self) -> Readiness;
    fn submit(&self, body: Vec<u8>, deadline: Instant) -> Reply;
    fn report(&self) -> Option<Report> { None }
    fn failure(&self) -> Option<String> { None }
}

Two required methods: say whether work may be sent, and accept it.

• submit returns a reply, not a result. A refusal travels the same path as an answer, so a caller has one thing to await and can still tell "there was no room" from "the runtime dropped my receiver" — two channels would make those easy to confuse.
• The types a worker must name live with the trait (Readiness, Report, Answer, EngineError), because they are what the contract is made of. Readiness is three-state: a worker that failed keeps running and reports why, so an operator reads the reason instead of guessing from an exit code.
• Queue state is published by whoever owns the queue. Report carries depth, capacity and a monotonic rejected count; both are optional so a worker that measures neither reports nothing rather than a zero it did not measure.
• report/failure have defaults, so the smallest useful worker implements two methods and nothing else.
• Nothing parses a decision envelope: bytes in, bytes out.

The transport, in src/frontend/src/engine.rs (212 lines)

The two routes, the request budget measured from the request headers, the body limit, the status mapping, and the queue depth a reply publishes. The forwarding path in src/frontend/src/lib.rs is untouched apart from exporting the module.

One timeout semantic

A request that cannot be answered inside its budget is 504, whatever part of the budget ran out — the upload, the queue, or the inference. 503 is reserved for a worker that cannot take work at all: still loading, failed, out of capacity, or gone. The forwarding path answers 504 for a backend timeout as well, so a caller does not have to know which mode it is talking to in order to read a timeout. tests/frontend/in_process.rs::timeout_and_unavailability_are_different_answers holds that line.

Not in this PR

• A queue with admission limits. Report is shaped to describe one, and tests/runtime/engine.rs exercises the contract with a worker that owns one, but the implementation stays where the runtime README puts it: SerialScheduler is admission with concurrency one, and queue-length limits are listed there as planned. This PR does not claim them.
• The lifecycle around a queue — worker-thread ownership, readiness transitions, drain on shutdown. Same reason.
• A model that implements the contract. Nothing here is a checkpoint reader or an executor; the only implementations in this PR are the two test workers. Wiring LAYA to it is a separate change.
• A runnable in-process mode in the omni-jev binary. No production engine exists yet to select.

Test Plan

cargo fmt --all --check
cargo clippy --workspace --locked --all-targets -- -D warnings
cargo test --workspace --locked

Per CONTRIBUTING, all test bodies are in the repository tests/ tree, with explicit [[test]] paths registered in the manifest.

• tests/runtime/engine.rs — the contract through its public API: an answer and a refusal both arrive on the reply; a worker without observability reports nothing and answers no work when asked how it is; a queued worker admits to capacity, reports the depth and capacity it is holding, refuses the next request without counting it, and admits again once a slot frees; a dropped reply does not disturb the queue's accounting.
• tests/frontend/in_process.rs — the transport over real sockets: readiness decides whether work is accepted and an unready worker never sees the request; bytes and content type survive; a body over the limit is refused before the worker sees it; a busy worker produces 503 and is not retried; InvalidRequest/InferenceFailed map to 400/500 with the worker's text passed through as valid JSON; the reply's depth is published as x-queue-depth; a worker that never answers is cut off by the budget at 504; and 504 and 503 do not blur.

Test Result

System1-Omni Version / Commit: 7f39ac4 (main) as base; head 1363d47.

• cargo fmt --all --check clean
• cargo clippy --workspace --locked --all-targets -- -D warnings clean
• cargo test --workspace --locked — 98 passed, 0 failed, 10 ignored, five consecutive runs without a failure. The ignored tests came with main; none are mine.
• cargo build --workspace --release --locked passes
• The existing forwarding suites are unchanged and still green

No new dependency, and one new edge: src/frontend now depends on omni-runtime, which is the point of the change. The manifests otherwise only register the two test targets, and Cargo.lock changes by a single line for that edge.

Not verified: Linux (this was macOS with Rust 1.98.1), and any model implementation, since none exists yet.

Review note

docs/architecture.md says the runtime owns "admission, queues, batch budgets, compatibility grouping, request bookkeeping, and result routing". A trait whose submit carries a deadline and whose Report describes a bounded queue is a small part of that, and it is the part a worker needs in order to be served without writing its own HTTP. If the intended home for the trait is narrower — say, admission only, with the frontend still owning any deadline arithmetic — that is a smaller change than this one and I would rather make it now than after a model depends on the shape.

[hsliuustc0106]

Reviewed commit e8eec5d0aecdfb3efca50413a0cd4bbabae05a59.

No actionable findings. All 21 workspace tests passed. Reviewed the Engine boundary, readiness/status mapping, body limit, shared upload/inference deadline, response-byte preservation and error escaping.

[Levius-Fubuki]

Reviewed e8eec5d0aecdfb3efca50413a0cd4bbabae05a59 and the full Engine/HTTP-service diff.

No actionable findings. Reviewed readiness/failure reporting, request-body and shared deadline handling, status mapping, response-byte preservation, and JSON escaping. On Linux with Rust 1.98.1, formatting, strict all-target Clippy, release build, and all 21 workspace tests passed.

The branch still conflicts with current main, so this is a COMMENT rather than merge approval. Please resolve the conflicts and rerun the checks on the rebased head; this review does not certify conflict-resolution changes that do not exist yet. The separate lifecycle implementation in #35 is reviewed separately; this PR does not itself provide a production model engine.

[xiaoyu-xyz]

@Levius-Fubuki — rebased and rerun as you asked, details below.

Rebased onto current main and rerun, as asked: head 5dd97f8, base 6d92519, mergeable is now true.

The conflict was structural. main has rewritten the README since the review — new centred header, new ## About and ## Features sections — and it replaced the exact sentence this branch had edited. The resolution keeps main's structure and carries this branch's fact into the new paragraph:

The Rust frontend forwards requests to a separately running model worker, and defines the small engine interface a model implements to serve in-process instead. The Cua-S1 4B 0.2 text adapter and Open-Jev-27B-v1.1 have native workers using shared CUDA kernels in this repository.

No source file changed in the resolution, and the diff is still this PR's five files.

Rerun on 5dd97f8 (macOS, Rust 1.98.1 — same toolchain version as your Linux run):

• cargo fmt --all --check clean
• cargo clippy --workspace --locked --all-targets -- -D warnings clean
• cargo test --workspace --locked — 40 passed, 0 failed, 9 ignored
• cargo build --workspace --release --locked passes

The counts move because main keeps adding tests, so that number is measured on the head above. The ignored tests are not mine and I have not run what they need.

On your "not a production model engine" point: agreed, and the README does not claim one — the in-repo engines are still described as the native workers that exist, and this PR only adds the interface a model would implement. The lifecycle behind that interface is #35.

One thing for whoever merges: this PR and #35 are a stack. #35 is now rebased onto this branch, so they merge in order and the second diff stays incremental rather than repeating this one. main has moved twice during review, and the README is the file that conflicts each time — if it moves again before merging, both branches need the same rebase.

[Levius-Fubuki]

lgtm @hsliuustc0106 👀

[hsliuustc0106]

Independent local review — in-process engine contract

Verdict: changes requested — placement conflicts with the merged architecture baseline.

The engineering is good — I ran it: 8/8 engine-service tests plus the 12 proxy tests pass at head 5dd97f8b, clippy clean; the deadline-carrying submit signature, three-state readiness with failure reasons, engine-measured x-queue-depth, and exact-response-text tests are all well designed.

But the design lands the request lifecycle in the wrong layer. It is written against RFC #14's original placement ("the request lifecycle in src/frontend/"); since then the repo re-baselined (#76) and implemented worker-side admission (#80 omni-runtime). docs/architecture.md now assigns the frontend "HTTP transport, forwarding, and response delivery" only, with queues/admission/back-pressure owned by the worker-side runtime — which is where this trait's shape belongs. Landing it as-is (and #35's worker-thread queue behind it) reintroduces exactly the boundary the baseline removed, and would give the crate two timeout semantics (proxy: 504; in-process budget: 503).

Proposed resolution: rehome the trait to src/runtime as the worker-side engine contract — its submit(body, deadline) -> Reply is a natural fit for what multi-request workers and the future shared runtime need — keeping only the health/status mapping and body-limit handling in the frontend. Also note the base predates the crate rename (omni-jev → frontend), so a rebase is due regardless.

Happy to re-review once the placement is decided; the trait itself is worth keeping.

Local reviewer report per the repo review skill; reflects head 5dd97f8b only.

[xiaoyu-xyz]

Superseded by #91. Closing rather than redirecting.

Your finding was about placement, and it was right: the contract belongs to the worker-side runtime, and the frontend should keep transport. #91 does exactly that — Engine, Readiness, Report, Answer and EngineError move to src/runtime/src/engine.rs, and the frontend keeps only the two routes, the body limit, the status mapping and the queue-depth header.

Two things made a fresh branch the cheaper path than redirecting this one. The base here predates 102 commits of main, and the branch still carried the in-process worker with its own queue, which is the part your finding is really about — omni-runtime already owns admission as SerialScheduler, and queue-length limits are listed in its README as planned. #91 drops that worker and does not claim the limits.

Also, one correction for the record: this branch's base does not predate a crate rename. src/frontend's package is still omni-jev on main today, there is no package named frontend in the workspace, and no commit here has renamed one. A rebase was due regardless, and #91 is based on 7f39ac4.

[xiaoyu-xyz]

Reopened with the placement revision as the head, so this thread keeps the review rather than starting a new one. 1363d47, rebased onto 7f39ac4, 9 files and +905/-1.

What changed, against the three findings:

• Placement. Engine, Readiness, Report, Answer and EngineError now live in src/runtime/src/engine.rs. The frontend keeps the two routes, the body limit, the status mapping and the queue-depth header. omni-runtime owns admission already as SerialScheduler; this adds the contract a model implements to be served by it.
• Two timeout semantics. Gone. A request that cannot be answered inside its budget is 504 whatever part ran out — upload, queue, or inference — and 503 is reserved for a worker that cannot take work at all. The forwarding path answers 504 for a backend timeout as well, so a caller does not have to know which mode it is talking to.
• The queue. Dropped from this PR. Report is shaped to describe a bounded queue and a worker that owns one is exercised in tests/runtime/engine.rs, but the implementation is not here: SerialScheduler is admission with concurrency one, and queue-length limits stay where the runtime README puts them, as planned. The worker thread, readiness transitions and drain are not here either.

The earlier discussion in this thread was about the previous placement and does not describe this head. Two things in it were also wrong about the repository and I would rather correct the record than leave them: the base never carried a crate rename — src/frontend's package is omni-jev on main today, there is no package named frontend, and no commit here renamed one — and my own claim that a test of mine fired within the first hundred submissions came from an earlier version of that test and was withdrawn at the time.

Tests are under the repository tests/ tree per CONTRIBUTING, with explicit [[test]] paths: tests/runtime/engine.rs exercises the contract through its public API, and tests/frontend/in_process.rs runs the transport over real sockets for readiness, byte preservation, the body limit, refusal without retry, error mapping, queue depth, and a worker that never answers.

On 1363d47: fmt and clippy -D warnings clean over the workspace, 98 tests passing across five consecutive runs, release build passes, and the existing forwarding suites are unchanged and green.

[Levius-Fubuki]

Rechecked 1363d47: the contract now lives in omni-runtime, the frontend keeps transport, and timeout responses use 504. Fmt, strict Clippy, release build and workspace tests passed (98 passed, 10 ignored). No actionable findings in this revision.

[xiaoyu-xyz]

A note on where the review state sits relative to the head, since the two no longer line up.

The CHANGES_REQUESTED is anchored to 5dd97f8 — the placement that review asked to change. The head is now 1363d47, which is that revision: the contract moved to src/runtime, the frontend keeps transport, and the two timeout semantics were unified. Levius-Fubuki rechecked 1363d47 and reported no actionable findings, with fmt, strict Clippy, release build and the workspace tests passing.

So nothing is outstanding from my side, and the blocking state on the PR describes a head that no longer exists. I am not asking for a status change — just recording the mapping so it is not read as "changes were requested and not addressed":

Review	Anchor	State
5dd97f8	in-process contract in src/frontend	CHANGES_REQUESTED — addressed by the placement revision
1363d47	contract in src/runtime, frontend transport only	no actionable findings

Rebase state: 1363d47 is on 7f39ac4 and mergeable is true. For the record, the base never carried a crate rename — src/frontend's package is omni-jev on main today, there is no package named frontend, and no commit here renamed one — so the only thing that had gone stale was the base.