Skip to content

Commit 905dbab

Browse files
docs: define executor streaming boundaries (vllm-project#246)
## Summary - define one target streaming pipeline and assign transport, normalization, ingestion, relay, and orchestration to separate owners - add mandatory executor-streaming change rules to `AGENTS.md`, including bounded-memory channels and measurement-gated worker placement - add a read-only `executor-architecture-review` skill that checks proposed changes against the repository documentation - connect the architecture to RFC vllm-project#241 and the focused follow-ups in vllm-project#243, vllm-project#244, and vllm-project#245 without changing runtime behavior ## Test Plan - `quick_validate.py skills/executor-architecture-review` - `pre-commit run --files AGENTS.md ARCHITECTURE.md skills/executor-architecture-review/SKILL.md` - `cargo fmt --all -- --check` - `git diff --check` - `cargo test --workspace` was attempted, but the machine ran out of disk space while compiling; this PR changes no Rust code Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
1 parent 474d154 commit 905dbab

3 files changed

Lines changed: 150 additions & 9 deletions

File tree

AGENTS.md

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ This repository is Rust-first under the `vllm-project` GitHub organization.
3030
├── crates/agentic-praxis/ # Praxis integration
3131
├── python/agentic_api/ # Python distribution, diagnostics, and launcher
3232
├── tests/python/ # Python package and CLI tests
33+
├── skills/ # Repository-specific agent review workflows
3334
├── pyproject.toml # Python wheel build metadata
3435
├── Cargo.toml # Workspace manifest and shared dependencies/lints
3536
└── docs/ # Documentation (MkDocs)
@@ -102,6 +103,32 @@ lifecycle, module-by-module walkthrough, and contribution guide.
102103
- Respect this dependency direction: handlers call core APIs; executor coordinates `events`, `tool`, and `storage`;
103104
those modules share contracts through `types`. Do not introduce transport concerns into core types or business logic.
104105

106+
#### Executor streaming change rules
107+
108+
Read [the target streaming pipeline](ARCHITECTURE.md#target-streaming-pipeline-and-ownership-boundaries) before changing
109+
streaming code in `events/` or `executor/`. RFC [#241](https://github.com/vllm-project/agentic-api/issues/241) is split
110+
into synchronous ingestion [#243](https://github.com/vllm-project/agentic-api/issues/243), client delivery
111+
[#244](https://github.com/vllm-project/agentic-api/issues/244), and execution-placement measurement
112+
[#245](https://github.com/vllm-project/agentic-api/issues/245). New work must converge on those boundaries:
113+
114+
- Assign every responsibility to one stage: `inference.rs` owns HTTP/SSE framing, `events/` owns normalization, the
115+
synchronous ingestion state machine owns semantic-event validation and output-item assembly, the stream relay owns
116+
ordered client delivery, and `engine.rs` owns inference rounds, the tool loop, and persistence.
117+
- Extend the single ingestion path. Do not add a second parser, lifecycle validator, delta folder, finalizer,
118+
tool-call translator, or client-emission path. Different validation policies must share the same state transitions.
119+
- Keep transport free of semantic-event and output-item decisions. Keep ingestion free of client-delivery concerns.
120+
Keep the relay free of response-assembly state. Keep the engine free of SSE parsing.
121+
- Do not introduce unbounded channels. A channel needs an entry capacity plus either a byte budget or a maximum item
122+
size, defined full/disconnect behavior, cancellation propagation, and task join/error handling.
123+
- Run synchronous ingestion inline by default. Add worker or `spawn_blocking` placement only when representative
124+
measurements show a benefit and include queue occupancy, tail latency, memory, and cancellation behavior.
125+
- Preserve compile-time extension points: typed output-item state, exhaustive matches, and one declared path for a new
126+
semantic event or output-item kind. Test invalid lifecycle order, identifier/index mismatch, duplicate completion,
127+
terminal finalization, slow consumers, and disconnects when the changed boundary can encounter them.
128+
129+
Use [`skills/executor-architecture-review/SKILL.md`](skills/executor-architecture-review/SKILL.md) for a focused,
130+
read-only boundary review while planning or reviewing these changes.
131+
105132
## Rust Best Practices
106133

107134
- Do not use loose/untyped JSON signatures (`serde_json::Value` and similar) at a public API boundary. Request,

ARCHITECTURE.md

Lines changed: 74 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -283,11 +283,12 @@ executor so the accumulator doesn't do inline JSON parsing.
283283
directions, and (if it carries structured data) an `EventPayload` variant.
284284
2. `events/normalize.rs` — extend `extract_payload` and add an `extract_*` helper if
285285
the payload needs real parsing (otherwise it can fall through to `Raw`).
286-
3. `executor/accumulator.rs`'s `process_event` — add a match arm to fold the new event
287-
into accumulator state, the same way every existing streamed field does.
288-
4. Only if the event is gateway-synthesized (a built-in tool's lifecycle event):
289-
`executor/gateway.rs`'s `synthetic_event` call sites, and if it's a function-call
290-
shaped event needing translation, `executor/function_sse.rs`.
286+
3. Extend the single ingestion transition dispatcher. Today that is
287+
`executor/accumulator.rs`'s `process_event`; [#243](https://github.com/vllm-project/agentic-api/issues/243) will
288+
consolidate the stable entry point. Do not create a caller-specific validator or folding path.
289+
4. If the event is gateway-synthesized (a built-in tool's lifecycle event), construct a typed `EventFrame` in
290+
`executor/gateway.rs` and feed it through the same ingestion/relay boundaries. Function-call shape translation
291+
remains an ingestion concern, currently implemented by `executor/function_sse.rs`.
291292

292293
### `executor/` — the loop, and the server's only door into storage
293294

@@ -350,14 +351,71 @@ call inference, run the tool loop, persist. `agentic-server` never reaches past
350351
- **`error.rs`**`ExecutorError`, with the mapping methods (`http_status()`,
351352
`error_type()`, `into_response_body()`, ...) handlers use to render errors.
352353

354+
#### Target streaming pipeline and ownership boundaries
355+
356+
The streaming executor is converging on one linear pipeline under RFC
357+
[#241](https://github.com/vllm-project/agentic-api/issues/241). The current implementation still has overlapping
358+
validation, accumulation, translation, and emission responsibilities in `upstream.rs`, `accumulator.rs`,
359+
`function_sse.rs`, and `gateway_accumulator.rs`; that overlap is migration state, not an extension pattern. New work
360+
must move toward the following ownership model:
361+
362+
```text
363+
upstream HTTP bytes
364+
365+
366+
inference transport ──raw SSE data line──▶ event normalization
367+
│ EventFrame
368+
369+
synchronous ingestion state machine
370+
│ validated semantic events/items
371+
372+
stream relay ──▶ client
373+
374+
engine.rs surrounds the per-round path: inference rounds → tool loop → persistence
375+
```
376+
377+
| Stage | Owns | Must not own |
378+
| --- | --- | --- |
379+
| Inference transport (`inference.rs`) | HTTP request/response I/O, byte-chunk handling, SSE framing, timeouts, and `[DONE]` detection | Typed semantic-event validation, output-item lifecycle, translation, or client ordering |
380+
| Event normalization (`events/`) | Converting one raw SSE data line into one typed `EventFrame` | Cross-event lifecycle state, response assembly, or delivery |
381+
| Synchronous ingestion ([#243](https://github.com/vllm-project/agentic-api/issues/243)) | One entry point for normalization policy, semantic-event lifecycle validation, typed output-item slots, delta folding, tool-call shape translation, and finalization | Async task placement, client backpressure, cross-round sequencing, or persistence |
382+
| Stream relay ([#244](https://github.com/vllm-project/agentic-api/issues/244)) | Cross-round sequence numbers, public `output_index` rebasing, lifecycle suppression, deferred-event ordering, bounded client delivery, and disconnect propagation | Re-parsing SSE data, reconstructing output items, or deciding the tool loop |
383+
| Orchestrator (`engine.rs`) | Turn and inference-round control, tool-loop decisions, terminal-response policy, and persistence | SSE framing/parsing or a second semantic-event state machine |
384+
385+
The boundary contract is **one owner and one path per concern**:
386+
387+
- Every streamed upstream response enters the same synchronous ingestion state machine. Rejecting and compatibility
388+
validation policies may choose different outcomes, but they must exercise the same typed transitions rather than
389+
maintaining separate validators.
390+
- An output item's lifecycle is scoped to one inference round and keyed by validated `output_index`; item ID and kind
391+
must agree on every subsequent semantic event. Completed slots remain distinguishable from never-seen slots so
392+
index reuse and duplicate completion can be detected. Finalization consumes the round's ingest state.
393+
- Each supported output-item kind has typed in-flight state and participates in exhaustive transition/finalization
394+
matches. Adding a kind extends those declared matches and their tests instead of adding a side path.
395+
- Downstream stages consume the typed result of the preceding stage. They do not parse the raw line again, infer a
396+
second lifecycle from the wire object, or reconstruct response state already owned upstream in the pipeline.
397+
398+
Concurrency is a deployment choice around this synchronous semantic core, not part of the core itself. Introduce a
399+
channel only at a real task-ownership boundary. Every channel needs a bounded entry count and either a byte budget or
400+
a maximum item size that gives a known memory ceiling. Define what happens when it is full, when the receiver
401+
disconnects, and when either task is cancelled or fails; carry cancellation through the whole producer/consumer path
402+
and join spawned tasks. Instrument entry and byte occupancy when tuning a capacity.
403+
404+
Run ingestion inline unless representative measurements show that worker placement improves the complete request
405+
path. Benchmark [#245](https://github.com/vllm-project/agentic-api/issues/245) owns that decision and must compare
406+
equivalent semantics, realistic event sizes and pacing, concurrent requests, slow consumers, tail latency, CPU,
407+
memory, thread count, and queue occupancy. `spawn_blocking` and a capacity such as 16 entries are hypotheses, not
408+
architectural defaults.
409+
353410
#### `accumulator.rs``ResponseAccumulator`: a stability contract, not just a file
354411

355412
`ResponseAccumulator` is the SSE state machine that turns a stream of `EventFrame`s
356413
into a `ResponsePayload`. Its public surface is intentionally small
357414
(`new`, `from_json`, `from_stream`, `from_sse_lines`, `mark_incomplete`, `finalize`) and
358-
**should not grow**. Don't add a new public method and call it from another method on
359-
the struct — that's a surface API change. Extend behavior through the existing
360-
pattern instead:
415+
**should not grow outside the approved [#243 consolidation](https://github.com/vllm-project/agentic-api/issues/243)**.
416+
Until that consolidation lands, don't add a new public method and call it from another method on the struct — that's
417+
another surface API change. Extend current behavior through the existing pattern while preserving the target
418+
single-ingestion boundary above:
361419

362420
- Each output item arrives via `response.output_item.added` and is **parked** as an
363421
`InFlightEntry` in `self.in_flight: IndexMap<String, InFlightEntry>`, keyed by item
@@ -392,6 +450,11 @@ deduplicates `response.created`/`response.in_progress` so they fire once per res
392450
rather than once per round. `gateway.rs` and `upstream.rs` both feed frames through it
393451
via `process_event`/`synthetic_event`/`emit_sse_frame`.
394452

453+
This is the current precursor to the stream-relay boundary in
454+
[#244](https://github.com/vllm-project/agentic-api/issues/244). New delivery,
455+
buffering, and backpressure behavior belongs in that relay consolidation rather than
456+
in the response accumulator or inference transport.
457+
395458
#### `function_sse.rs``FunctionSseTranslator`
396459

397460
vLLM only ever emits `function_call` SSE events, regardless of which tool type the
@@ -662,9 +725,11 @@ router, reusing the same core logic in-process.
662725
| Task | Where |
663726
|---|---|
664727
| Add a new HTTP or WebSocket route | `agentic-server/src/handler/{http,websocket}/`, wire it in `app.rs`'s `build_router_with_auth` |
665-
| Support a new upstream SSE event | `events/types.rs` → `events/normalize.rs` → `executor/accumulator.rs` (+ `gateway.rs`/`function_sse.rs` if it's gateway-synthesized) |
728+
| Support a new upstream SSE event | `events/types.rs` → `events/normalize.rs` → the single ingestion dispatcher tracked by [#243](https://github.com/vllm-project/agentic-api/issues/243); do not add a caller-specific path |
666729
| Add a new tool type | `tool/handler.rs` impl(s) → `tool/normalize.rs` → `tool/registry.rs` → `tool/executors.rs` if it needs lazy connection setup |
667730
| Change gateway-round concurrency or lifecycle ordering | `executor/gateway.rs` (`GatewayScheduler`/event plans) + `executor/engine.rs` (round decision/ordered streaming) + `tool/ownership.rs` (typed binding and same-tool safety) |
731+
| Change client streaming order, buffering, or backpressure | The stream-relay boundary tracked by [#244](https://github.com/vllm-project/agentic-api/issues/244); do not add it to `inference.rs` or the response accumulator |
732+
| Move streaming ingestion to a worker | Benchmark the equivalent inline and worker paths under [#245](https://github.com/vllm-project/agentic-api/issues/245) before changing executor placement |
668733
| Change continuation history visibility | `storage/types/item.rs::into_input_items` → `types/io/output.rs::to_input_item` (preservation) → `types/io/input.rs::model_input` (upstream visibility) |
669734
| Add a CRUD operation beyond persist/rehydrate | `executor/modes/conversation.rs` or `modes/response.rs`, backed by `storage/conversation.rs` / `storage/response.rs` |
670735
| Change how output items are assembled from a stream | `executor/accumulator.rs` — respect the `TryFrom`/`ApplyDone` pattern, don't add new public methods |
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
---
2+
name: executor-architecture-review
3+
description: Use when planning or reviewing agentic-api changes that affect executor streaming, upstream SSE handling, output-item lifecycle, stream delivery, backpressure, or worker placement.
4+
---
5+
6+
# Executor Architecture Review
7+
8+
Perform a read-only review that protects the executor's streaming ownership boundaries. Do not edit files, create or
9+
update issues, post comments, commit, or push.
10+
11+
Use `skills/pr-review/SKILL.md` instead for a broad pull-request review that does not involve these boundaries.
12+
13+
## Authoritative inputs
14+
15+
1. Read `AGENTS.md` and `TERMINOLOGY.md` completely.
16+
2. Read `ARCHITECTURE.md`, especially "Target streaming pipeline and ownership boundaries" and the affected module
17+
sections. Treat it as the source of truth; this skill does not replace it.
18+
3. Resolve the exact task, diff, or pull request and inspect the complete changed path with relevant surrounding code.
19+
20+
## Review workflow
21+
22+
1. Map each changed responsibility to exactly one owner: inference transport, event normalization, synchronous
23+
ingestion, stream relay, or engine orchestration. Flag logic that has no owner or appears in multiple stages.
24+
2. Trace one representative streaming event from upstream bytes through client delivery. Check that each stage
25+
consumes the previous stage's typed result instead of re-parsing or reconstructing its state.
26+
3. Check output-item lifecycle invariants: validated `output_index`, stable item ID and kind, typed active state,
27+
detectable index reuse/duplicate completion, exhaustive kind handling, and consuming terminal finalization.
28+
4. Check concurrency boundaries. Every channel needs entry and memory bounds, full/disconnect behavior, cancellation
29+
propagation, and join/error handling. Treat `spawn_blocking`, worker placement, and proposed capacities as claims
30+
requiring representative measurements, not defaults.
31+
5. Match tests to the changed owner. Consider malformed and out-of-order semantic events, ID/index mismatches,
32+
duplicate completion, incomplete terminal state, tiny capacities, oversized items, slow consumers, cancellation,
33+
and task failure when applicable. Do not demand unrelated cases.
34+
35+
## Findings
36+
37+
Report only actionable correctness, regression, performance, or missing-test issues. Verify each finding against the
38+
actual path and use this form:
39+
40+
```text
41+
[P1] path/to/file.rs:123 — short title
42+
Boundary: <owner and violated responsibility>
43+
Evidence: <specific path or scenario>
44+
Impact: <observable failure>
45+
Fix: <concise direction that restores one owner and one path>
46+
```
47+
48+
Order findings from `P0` to `P3`. If none remain, say so explicitly. End with a short ownership map, tests or
49+
benchmarks inspected or run, and any environment limitations.

0 commit comments

Comments
 (0)