Forward-merge release/0.7 into main - #743
Conversation
#### Overview Relay hook responses can return before the terminal scope event reaches observability subscribers. Consumers that read ATIF immediately after the hook response can therefore observe ATOF but miss the final ATIF trajectory. This change attaches a completion receipt to the exact terminal scope event. Hook handling waits for that receipt only after releasing session-manager locks. It does not drain work queued later by another session. - [x] I confirm this contribution is my own work, or I have the right to submit it under this project license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details - Add an internal `SubscriberDelivery` receipt for one queued event. - Return receipts from terminal turn, session, and subagent scope closures. - Await receipts after releasing shared session and alignment locks. - Preserve the shared FIFO dispatcher: work already ahead can delay a receipt, but later unrelated session work is not captured. - Preserve full server teardown ordering: close all sessions, globally flush subscribers, then clear plugins. - Keep streaming and public schema behavior unchanged. - Document the hook-response and full-shutdown delivery guarantees. No Fabric artifact polling or retry hotfix is required. Fabric still needs its separate Relay 0.7 and schema-v3 compatibility work. Validation: - `just test-rust` - `CARGO_INCREMENTAL=0 just test-python` — 639 passed - `XDG_CONFIG_HOME=<isolated> just test-go` - `PATH=<Node 24>:$PATH just test-node` — 353 passed - `CARGO_INCREMENTAL=0 cargo clippy --workspace --all-targets -- -D warnings` - `just docs` - `uv run pre-commit run --all-files` - Fabric Codex Relay E2E: one-shot ATOF and ATIF visibility plus two streamed turns - Platform live evaluator E2E: `AgentEvaluator -> Fabric -> Codex -> Relay`, with ATIF available immediately for metric evaluation - Platform parallel evaluator E2E: two concurrent trials produced two isolated ATIF files #### Where should the reviewer start? Start with `SubscriberDelivery` in `crates/core/src/api/runtime/subscriber_dispatcher.rs` and `pop_scope_with_subscriber_delivery` in `crates/core/src/api/scope.rs`. Then review `SessionManager::apply_events` in `crates/cli/src/sessions/mod.rs`, followed by the terminal-response and shutdown tests in `crates/cli/tests/coverage/shared/server_tests.rs`. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to NVIDIA/NeMo-Fabric#181 ## Summary by CodeRabbit - **New Features** - Hook responses now wait for related turn, subagent, or session-end events to finish processing. - Session and turn closures provide more reliable completion handling. - **Bug Fixes** - Improved event delivery ordering and error reporting. - Events blocked in one session no longer prevent other sessions from processing. - Terminal snapshots and events are flushed during server shutdown. - **Documentation** - Updated lifecycle documentation to explain hook completion and shutdown behavior. Authors: - Ajay Thorve (https://github.com/AjayThorve) Approvers: - Will Killian (https://github.com/willkill07) URL: NVIDIA#727
#### Overview Prevent a successfully completed streamed LLM call from being exported as an OpenTelemetry error when a client stops polling after the provider's terminal response but before transport EOF. - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details - Distinguish clean stream exhaustion, real stream failure, and client-drop termination internally. - Override only the synthetic client-drop error when the finalized response contains both a known terminal provider outcome and authoritative final token usage. - Preserve `ERROR` for partial drops, unknown terminal states, collector/codec failures, and upstream transport failures. - Add an integration regression using the real OpenAI Responses streaming/final response codecs, while retaining coverage that partial drops and unknown outcomes stay errors. Validation completed: - `cargo fmt --all -- --check` - `cargo clippy --workspace --all-targets -- -D warnings` - `just test-rust` - `just ci=true test-rust` (3,859 passed; CI-profile coverage report generated) - `just test-python` (639 passed) - `just test-node` (353 passed with Node 24.15.0) - `uv run pre-commit run --all-files` - Focused stream regression tests (3 integration and 1 unit test passed) - Tokenomics OTel capture with Relay built from this branch and Codex 0.146.1: 10/10 GenAI LLM spans were `STATUS_CODE_OK` with `finish_reason=stop`; no error-status spans appeared in any of the 30 Full, GenAI, and OpenInference exports. - Hosted PR matrix: 69 successful checks, 10 expected skips, 0 failures, and 0 pending checks. `just test-go` has three pre-existing failures in stale observability-config tests that still use the removed standalone OpenInference v3 field. The same three tests fail on the untouched `release/0.7` head (`5f3c210e`); all Go stream tests pass. Breaking changes: none. #### Where should the reviewer start? Start with `crates/core/src/stream.rs`, specifically the termination classification passed into `emit_end_event`, then review `dropped_stream_after_terminal_response_emits_success` in `crates/core/tests/integration/stream_tests.rs`. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to: none Authors: - Ajay Thorve (https://github.com/AjayThorve) Approvers: - Will Killian (https://github.com/willkill07) URL: NVIDIA#741
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (13)
📜 Recent review details⏰ Context from checks skipped due to timeout. (37)
🧰 Additional context used📓 Path-based instructions (31)**/*.mdx📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
Files:
{docs,examples}/**/*📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Files:
**/*📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
docs/**/*📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
**/*.{md,mdx}📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
docs/**/*.{md,mdx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{rs,py,go,js,jsx,ts,tsx,c,h,html,md,mdx,toml}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{md,mdx,rst}📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md)
Files:
docs/**/*.mdx📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
Files:
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}⚙️ CodeRabbit configuration file
Files:
**/*.rs📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Files:
**/*.{rs,py,go,js,ts}📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Files:
**/*.{rs,py,js,jsx,ts,tsx,go,c,h,cc,cpp,md,toml,yml,yaml,sh}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rs,py}📄 CodeRabbit inference engine (AGENTS.md)
Files:
crates/**/*.rs📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rs,py,js,mjs,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{toml,md,rs,py}📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Files:
crates/**/src/**/*.rs📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Files:
{crates/core,crates/adaptive}/**/*📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Files:
crates/core/**/*.rs📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
Files:
crates/core/src/{api/**/*.rs,api/runtime/**/*.rs,codec/**/*.rs,json.rs}📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Files:
crates/{core,adaptive}/**/*📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
crates/core/src/api/**/*.rs📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Files:
crates/{core,adaptive,plugin,worker,worker-proto,types}/**/*.{rs,toml}📄 CodeRabbit inference engine (.agents/skills/test-rust-core/SKILL.md)
Files:
crates/{core,adaptive,plugin,worker,worker-proto,types}/**/*📄 CodeRabbit inference engine (.agents/skills/test-rust-core/SKILL.md)
Files:
crates/{core,adaptive}/**/*.rs⚙️ CodeRabbit configuration file
Files:
**/{test,tests}/**/*📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}⚙️ CodeRabbit configuration file
Files:
crates/core/src/api/{tool,llm,shared,scope}.rs📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Files:
🧠 Learnings (3)📚 Learning: 2026-08-03T19:55:03.931ZApplied to files:
📚 Learning: 2026-07-28T20:07:29.880ZApplied to files:
📚 Learning: 2026-07-28T03:31:05.964ZApplied to files:
🔇 Additional comments (19)
WalkthroughThe change adds awaitable subscriber-delivery receipts and propagates them through scope, session, turn, agent, subagent, and tool closure. Event processing waits after releasing locks. Stream termination now distinguishes clean, failed, and dropped states. ChangesSubscriber delivery flushing
Stream termination metadata
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
size:L
willkill07
changed the title
github-actions
Bot
removed
the
Maintenance
willkill07
changed the title
2 participants
Forward-merge triggered by push to release/0.7 that creates a PR to keep main up-to-date. If this PR is unable to be immediately merged due to conflicts, it will remain open for the team to manually merge. See forward-merger docs for more info.
Summary by CodeRabbit
Bug Fixes
Documentation