fix: wait for Relay ATIF finalization - #187
Conversation
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
WalkthroughRelay artifact utilities now detect and await finalized local ATIF files. Claude and Codex integrate this flow, return explicit timeout failures, and mark failed runtimes unusable. Tests cover delayed artifacts, invalid files, timeouts, isolation, and configurable end-to-end ATIF behavior. ChangesRelay ATIF artifact handling
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Adapter
participant NeMoRelay
participant relay_artifacts
participant LocalATIF
Adapter->>relay_artifacts: Snapshot ATIF files
Adapter->>NeMoRelay: Execute invocation
NeMoRelay-->>Adapter: Successful response
Adapter->>relay_artifacts: Await finalized ATIF
relay_artifacts->>LocalATIF: Poll new or modified JSON files
LocalATIF-->>relay_artifacts: Finalized artifact
relay_artifacts-->>Adapter: Artifact path
Adapter-->>Adapter: Return Relay output with artifact
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@adapters/claude/src/nemo_fabric_adapters/claude/adapter.py`:
- Around line 917-919: Return only the finalized ATIF for the current
invocation: in
adapters/claude/src/nemo_fabric_adapters/claude/adapter.py:917-919 and
adapters/codex/src/nemo_fabric_adapters/codex/adapter.py:1098-1100, retain the
path returned by wait_for_finalized_atif() and pass artifact data to
_relay_output() that excludes pre-existing ATIF files. In
tests/adapters/test_claude_adapter.py:808-864 and
tests/adapters/test_codex_adapter.py:591-632, pre-create a stale ATIF and assert
the relay output contains only the newly finalized ATIF.
In `@tests/adapters/test_adapters_common_relay_artifacts.py`:
- Around line 95-114: Add a test alongside
test_wait_for_finalized_atif_accepts_modified_existing_path that replaces the
existing ATIF file via Path.replace() using a separately written temporary file,
then calls relay_artifacts.wait_for_finalized_atif and asserts the replaced path
is returned. Ensure the setup snapshots the original file before replacement so
the test verifies detection of an atomic inode replacement.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 175d2556-bcde-4146-a592-29603fc131d3
📒 Files selected for processing (7)
adapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pytests/adapters/test_adapters_common_relay_artifacts.pytests/adapters/test_claude_adapter.pytests/adapters/test_codex_adapter.pytests/e2e/test_claude.py
📜 Review details
⏰ Context from checks skipped due to timeout. (17)
- GitHub Check: Test (Python 3.11, linux-amd64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.14, linux-arm64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.13, linux-amd64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.12, linux-arm64)
- GitHub Check: Test (Python 3.14, macos-arm64)
- GitHub Check: Test (Python 3.11, linux-arm64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.12, linux-amd64)
- GitHub Check: Test (Python 3.13, linux-arm64)
- GitHub Check: Test (Python 3.14, linux-amd64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Pre-commit
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{rs,py,pyi,json,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Determine and update every affected public surface, including the CLI, PyO3 bindings, Python SDK, type stubs, schemas, and adapter contract, so they remain in parity.
Files:
tests/adapters/test_adapters_common_relay_artifacts.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pytests/e2e/test_claude.pytests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
**/*
📄 CodeRabbit inference engine (.agents/skills/karpathy-guidelines/SKILL.md)
**/*: Before implementing, explicitly state assumptions, surface ambiguity and tradeoffs, present multiple interpretations when relevant, and ask for clarification rather than silently deciding or proceeding when requirements are unclear.
Prefer the minimum code needed to solve the requested problem: avoid speculative features, unnecessary abstractions, unrequested flexibility, and handling of impossible scenarios; simplify overcomplicated solutions.
When editing existing code, make surgical changes only: do not modify unrelated code, comments, formatting, or pre-existing dead code; match the existing style, and remove only unused imports, variables, or functions introduced by your changes.
Define verifiable success criteria for each task, such as writing regression tests for bugs and invalid-input tests for validation, then verify the implementation against those criteria. For multi-step work, state a brief plan with a verification check for each step.
**/*: Always spellNVIDIAin all caps; do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun, because the name begins with an “en” sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company; use trademark symbols with product names only when required by the document type or legal guidance.
Verify official capitalization, spacing, hyphenation, and spelling for NVIDIA and third-party product names; do not rewrite official product names for grammar or title-case rules.
Precede NVIDIA product names withNVIDIAon first mention when natural and accurate, and link the first mention when the destination helps the reader.
On first use, include the company name and full model qualifier when it helps identify the model; preserve official capitalization and punctuation, and use shorter family names only after establishing the full name.
For learning-oriented and developer content, do not force trademark symbols unless explicitly required; for press, ...
Files:
tests/adapters/test_adapters_common_relay_artifacts.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pytests/e2e/test_claude.pytests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.Use
snake_casefor functions and variables; usePascalCasefor Rust types and Python classes.
Files:
tests/adapters/test_adapters_common_relay_artifacts.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pytests/e2e/test_claude.pytests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Python code or a Python-facing adapter changes, run
just test-python.In Python SDK, adapters, examples, and tests, follow the existing style, use type annotations for public APIs, and keep native binding declarations synchronized with their Rust implementations.
Files:
tests/adapters/test_adapters_common_relay_artifacts.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pytests/e2e/test_claude.pytests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
**/*.{rs,py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*.{rs,py,pyi}: If public configuration types change, confirm schema snapshot tests injust test-rustpass and review generated schema diffs.
For schema or public contract changes, run both language suites and review changes underschemas/and generated API references.
Files:
tests/adapters/test_adapters_common_relay_artifacts.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pytests/e2e/test_claude.pytests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
tests/adapters/**/*.py
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
tests/adapters/**/*.py: If an adapter or integration changes, run its focused tests.
For adapter behavior changes, run focused adapter tests undertests/adapters, then runjust test-python.
Files:
tests/adapters/test_adapters_common_relay_artifacts.pytests/adapters/test_codex_adapter.pytests/adapters/test_claude_adapter.py
**/*.{py,pyi,rs}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
For Python SDK or PyO3 binding changes, use
python-tests, run focused pytest tests first, thenjust test-python; rebuild withjust build-pythonwhen native code or packaging changes.Public contract changes must keep native Python binding declarations synchronized with their Rust implementations.
Files:
tests/adapters/test_adapters_common_relay_artifacts.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pytests/e2e/test_claude.pytests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
**/*.{rs,py,toml}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
When editing version helpers, verify every
nemo-fabric-*workspace package through Cargo metadata and reject a static version inpython/pyproject.toml.
Files:
tests/adapters/test_adapters_common_relay_artifacts.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pytests/e2e/test_claude.pytests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
**/*.{toml,rs,py}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
Avoid blind repository-wide replacement of version-like strings; distinguish package-version references from examples and unrelated dependency versions.
Files:
tests/adapters/test_adapters_common_relay_artifacts.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pytests/e2e/test_claude.pytests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
**/*.{md,mdx,yml,py,rs,sh}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
Keep documentation aligned with current NeMo Fabric behavior, repository layout, entry points, commands, package names, APIs, bindings, and support claims.
Files:
tests/adapters/test_adapters_common_relay_artifacts.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pytests/e2e/test_claude.pytests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
tests/**/*.{rs,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the corresponding Rust crate or the relevant area under
tests/.
Files:
tests/adapters/test_adapters_common_relay_artifacts.pytests/e2e/test_claude.pytests/adapters/test_codex_adapter.pytests/adapters/test_claude_adapter.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Use pytest to run Python tests.
Do not add@pytest.mark.asyncioto tests; async tests are automatically detected by the async runner.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, useunittest.mock.MagicMockorAsyncMock, using thespecargument when necessary, rather than defining a new class.
Prefix mocked class names withmock, notfake.
Prefer pytest fixtures over helper methods.
If a fixture is needed in multiple test files, define it once inconftest.pyrather than repeating it.
Define fixtures using@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and a<fixture_name>_fixturefunction; specifyscopeonly when it is notfunction.
Preferpytest.mark.parametrizeover separate tests for different input types.
Use@pytest.mark.usefixtureswhen a fixture is needed but its returned value is unused or it returns no value.
Avoid defensive programming in tests; access expected values directly so missing data raises a clear failure, such as usingresults["data"]instead ofresults.get("data").
When adapter installation metadata changes, packaging metadata tests must directly assert that the root project depends unconditionally on the exact-versionnemo-fabric-runtimedistribution.
Packaging metadata tests must verify that each root harness extra delegates to the matching version of the leaf adapter'sharnessextra.
Packaging metadata tests must verify that bare leaf dependencies remain adapter-owned and that the rootadapter-testsdependency group installs each leaf through itsharnessextra.
Packaging metadata tests must verify that every leaf providesfull; only adapters importing NeMo Relay Python APIs providerelay, while adapters using an external Relay executable havefullequal toharness.
Files:
tests/adapters/test_adapters_common_relay_artifacts.pytests/e2e/test_claude.pytests/adapters/test_codex_adapter.pytests/adapters/test_claude_adapter.py
{tests/**,python/tests/**}
⚙️ CodeRabbit configuration file
{tests/**,python/tests/**}: Tests should cover the behavior promised by the changed API surface, including error paths, lifecycle cleanup, and SDK/native parity where relevant.
Files:
tests/adapters/test_adapters_common_relay_artifacts.pytests/e2e/test_claude.pytests/adapters/test_codex_adapter.pytests/adapters/test_claude_adapter.py
adapters/*/src/**/*.py
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
adapters/*/src/**/*.py: Use the existing Fabricpythonorprocessrunner and normalized request/result contracts; do not add a runner or one-off abstraction for a single adapter.
Treat normalizedconfig,capability_plan,telemetry_plan, andruntime_contextas authoritative; reserveharness.settingsfor adapter-wide behavior.
Apply configuration precedence in this order: normalized config; Fabric-resolved plans and runtime context; harness-specific settings; descriptor and adapter defaults. Reject conflicting duplicate declarations and unsupported behavior with actionable errors.
Validate dependency versions, hooks, and credentials before harness invocation, and never expose credential values in output, errors, events, logs, or fixtures.
Forward only required system variables, selected credential variables, telemetry variables, and documented harness-specific environment; never forward or log unrelated environment values.
Maintain one local adapter host per Fabric runtime across orderedstart→invoke*→stop; emit exactly one JSON lifecycle response per request on stdout and diagnostics on stderr.
Return harness-level invoke failures as successful lifecycle responses containingresponse: null,failed: true, and structurederrorfields (code,message,retryable, and optionalmetadata).
Do not emit Relay streaming records on adapter stdout; return exactly one terminal lifecycle response while SDK-owned NDJSON streaming occurs out of band.
Scope workspace, generated configuration, state, sessions, and artifacts to the resolved runtime context, and isolate stateful adapters by Fabric runtime ID.
Keep stdout stable by emittingresponseand adapter-specific extensions such aserror, harness events, usage, and session IDs; do not duplicate top-level fields Fabric supplies when buildingRunResult.
Claim capabilities only when they are implemented and tested end to end: enforce blocked tools on every tool path, support only valid MCP...
Files:
adapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
{adapters/**,examples/**}
⚙️ CodeRabbit configuration file
{adapters/**,examples/**}: Review adapter and example changes for command correctness, config/schema consistency, artifact handling, and compatibility with the public NeMo Fabric contracts.
Files:
adapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
🧠 Learnings (1)
📚 Learning: 2026-07-09T22:28:51.689Z
Learnt from: AjayThorve
Repo: NVIDIA/NeMo-Fabric PR: 43
File: adapters/claude-sdk/src/nemo_fabric_adapters/claude_sdk/adapter.py:164-168
Timestamp: 2026-07-09T22:28:51.689Z
Learning: In the NeMo-Fabric adapters, treat path values used in Fabric adapter configuration (including logic like `_resolve_path` in adapter.py) as config-root-relative. Do not apply `Path.expanduser()` (or otherwise apply `~`/home or shell-style expansion), because it will make the resolved paths normalize inconsistently across adapters. Also, do not rely on or add any resolution behavior that uses `harness.settings.cwd` as an override point for these adapter paths—`harness.settings.cwd` is explicitly unsupported in this adapter context.
Applied to files:
adapters/codex/src/nemo_fabric_adapters/codex/adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
🪛 ast-grep (0.45.0)
tests/adapters/test_adapters_common_relay_artifacts.py
[info] 76-76: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"schema_version": "ATIF-v1.7", "steps": []})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 102-102: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"schema_version": "ATIF-v1.7", "steps": [{"step_id": 1}]})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 143-143: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"schema_version": "ATIF-v1.7", "steps": []})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
tests/adapters/test_codex_adapter.py
[info] 611-611: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"schema_version": "ATIF-v1.7", "steps": []})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
tests/adapters/test_claude_adapter.py
[info] 825-825: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"schema_version": "ATIF-v1.7", "steps": []})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Ruff (0.16.1)
tests/adapters/test_adapters_common_relay_artifacts.py
[warning] 73-73: Missing return type annotation for private function finish_candidate
Add return type annotation: None
(ANN202)
[warning] 140-140: Missing return type annotation for private function write_atif
(ANN202)
tests/adapters/test_codex_adapter.py
[warning] 606-606: Missing return type annotation for private function finish_turn
(ANN202)
[warning] 609-609: Missing return type annotation for private function write_atif
Add return type annotation: None
(ANN202)
[warning] 636-636: Unused function argument: mock_codex
(ARG001)
adapters/codex/src/nemo_fabric_adapters/codex/adapter.py
[warning] 35-35: Use from nemo_fabric_adapters.common import relay_artifacts in lieu of alias
Replace with from nemo_fabric_adapters.common import relay_artifacts
(PLR0402)
tests/adapters/test_claude_adapter.py
[warning] 823-823: Missing return type annotation for private function write_atif
Add return type annotation: None
(ANN202)
🔇 Additional comments (7)
adapters/common/src/nemo_fabric_adapters/common/relay_artifacts.py (1)
21-109: LGTM!tests/adapters/test_adapters_common_relay_artifacts.py (1)
15-93: LGTM!Also applies to: 117-169
adapters/claude/src/nemo_fabric_adapters/claude/adapter.py (1)
35-35: LGTM!Also applies to: 727-745, 890-890
adapters/codex/src/nemo_fabric_adapters/codex/adapter.py (1)
926-944: LGTM!Also applies to: 1075-1078, 1116-1118
tests/adapters/test_claude_adapter.py (1)
755-805: LGTM!Also applies to: 873-926
tests/adapters/test_codex_adapter.py (1)
139-179: LGTM!Also applies to: 635-679
tests/e2e/test_claude.py (1)
71-139: LGTM!Also applies to: 182-204
|
/merge |
2 participants
Overview
Forward-port the Relay ATIF finalization fix from #181 to main.
Relay can report an agent turn complete before its asynchronous ATIF subscriber
has finished writing the trajectory. Codex and Claude previously collected
Relay artifacts immediately after that terminal result, so a successful
invocation could include ATOF while missing its ATIF.
This PR adds a shared finalization barrier for Codex and Claude. It does not
change package versions, dependency constraints, manifests, lockfiles, or the
observability schema.
Details
modification time) for runtime-scoped ATIF files before each invocation.
candidates and accept one when it contains a complete JSON object.
without reading unchanged trajectories from earlier turns.
mark the persistent runtime unavailable, and bypass later directory
rescanning so a late ATIF cannot be attributed to another response.
runtime-owned Relay gateway.
boundaries already finalize Relay output before collection.
No public API or configuration contract changes, and there are no breaking
changes.
Validation
lint, Cargo/uv lock freshness, both attribution checks, and dependency
license diff.
Codex 10/10 and Claude 10/10, with 20 isolated runtime-scoped ATIF
directories and zero contract failures.
5/5, with ten isolated directories, valid ATIF-v1.7 trajectories, and zero
contract failures.
Rust tests were not rerun because this PR changes only Python adapter behavior
and tests. Documentation is unchanged because public configuration and API
contracts are unchanged.
Where should the reviewer start?
Start with
adapters/common/src/nemo_fabric_adapters/common/relay_artifacts.py, then the
calls from CodexRuntime.invoke() and ClaudeRuntime.invoke(). The focused tests
cover delayed and partial writes, new and changed paths, remote-only storage,
timeout behavior, late-file exclusion, unusable runtimes, and parallel
runtime-directory isolation.
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Relates to fix: wait for Relay ATIF finalization #181 and the NeMo Platform ATIF release blocker.
I confirm this contribution is my own work, or I have the right to submit it under this project's license.
I searched existing issues and open pull requests, and this does not duplicate existing work.
Summary by CodeRabbit
New Features
Bug Fixes
Tests