feat(libsy): expose Python run streams#82
Conversation
4c6fc4f to
471e74c
Compare
e15dff1 to
d60553c
Compare
WalkthroughAdds comprehensive Rust/PyO3 bindings for libsy protocols, routing targets, algorithms, execution streams, and response handling, with typed Python facades, examples, and end-to-end tests. ChangesLibsy Python bindings
Estimated code review effort: 4 (Complex) | ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
crates/switchyard-py/src/libsy_bindings/protocol.rs (1)
72-87: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftDocument the Python-visible protocol wrappers.
PyWireFormatand most subsequently registered wrappers lack concise documentation, particularly around raw payload conversion and variant semantics.As per coding guidelines,
crates/**/*.rsrequires “concise comments/doc comments for public items and non-obvious helpers.”🤖 Prompt for 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. In `@crates/switchyard-py/src/libsy_bindings/protocol.rs` around lines 72 - 87, Document the Python-visible PyWireFormat wrapper and its variants with concise Rust doc comments, describing the wrapper’s raw payload conversion role and the semantics of OpenAiChat, AnthropicMessages, and OpenAiResponses. Apply the same documentation requirement to the subsequently registered protocol wrapper types referenced in this module, including public items and non-obvious conversion helpers.Source: Coding guidelines
crates/switchyard-py/src/libsy_bindings/values.rs (1)
418-628: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the non-obvious validation and stream lifecycle contracts.
crates/switchyard-py/src/libsy_bindings/values.rs#L418-L628: document single consumption, Python-source retention, and cleanup scheduling.crates/switchyard-py/src/libsy_bindings/target.rs#L166-L179: document shallow callable validation and deferred awaitability checks.crates/switchyard-py/src/libsy_bindings/run.rs#L131-L201: document terminal-step teardown, cancellation, andaclose()behavior.As per coding guidelines, add concise comments/doc comments for public items and non-obvious helpers, especially around complex validation, routing, async, lifecycle, or concurrency logic.
🤖 Prompt for 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. In `@crates/switchyard-py/src/libsy_bindings/values.rs` around lines 418 - 628, Document the non-obvious lifecycle and validation contracts with concise comments: in crates/switchyard-py/src/libsy_bindings/values.rs:418-628, explain single consumption, retained Python sources, and scheduled cleanup around PyLlmResponseStream, SourceClosingLlmStream, stream_from_python_source, and schedule_stream_source_close; in crates/switchyard-py/src/libsy_bindings/target.rs:166-179, document shallow callable validation and deferred awaitability checks; in crates/switchyard-py/src/libsy_bindings/run.rs:131-201, document terminal-step teardown, cancellation, and aclose() behavior.Source: Coding guidelines
🤖 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 `@crates/switchyard-py/src/libsy_bindings/run.rs`:
- Line 19: Update the PyLlmCall, PyStep, and PyRunStream pyclass declarations in
the run bindings to include module = "switchyard.libsy", ensuring all three
exported classes report the correct Python __module__.
In `@crates/switchyard-py/src/libsy_bindings/values.rs`:
- Around line 600-621: Update schedule_stream_source_close so the Some(future)
branch does not discard the aclose future when Handle::try_current() fails.
Schedule and await the future through an always-available executor, preserving
the existing warning behavior when awaiting it fails.
In `@switchyard_rust/libsy_protocol.pyi`:
- Around line 9-40: Update the value attributes in WireFormat, FormatId, Role,
and StopReason to read-only `@property` declarations returning str, rather than
writable annotations. Preserve FormatId.__init__ and known while ensuring all
four frozen PyO3 classes expose getter-only value properties.
In `@switchyard_rust/libsy.pyi`:
- Around line 41-63: Update the LlmCall fields context, request, decision, and
is_pending to be modeled as read-only properties in the type stub, using the
appropriate getter-only property syntax rather than plain writable annotations;
leave respond and fail unchanged.
---
Nitpick comments:
In `@crates/switchyard-py/src/libsy_bindings/protocol.rs`:
- Around line 72-87: Document the Python-visible PyWireFormat wrapper and its
variants with concise Rust doc comments, describing the wrapper’s raw payload
conversion role and the semantics of OpenAiChat, AnthropicMessages, and
OpenAiResponses. Apply the same documentation requirement to the subsequently
registered protocol wrapper types referenced in this module, including public
items and non-obvious conversion helpers.
In `@crates/switchyard-py/src/libsy_bindings/values.rs`:
- Around line 418-628: Document the non-obvious lifecycle and validation
contracts with concise comments: in
crates/switchyard-py/src/libsy_bindings/values.rs:418-628, explain single
consumption, retained Python sources, and scheduled cleanup around
PyLlmResponseStream, SourceClosingLlmStream, stream_from_python_source, and
schedule_stream_source_close; in
crates/switchyard-py/src/libsy_bindings/target.rs:166-179, document shallow
callable validation and deferred awaitability checks; in
crates/switchyard-py/src/libsy_bindings/run.rs:131-201, document terminal-step
teardown, cancellation, and aclose() behavior.
🪄 Autofix (Beta)
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: CHILL
Plan: Enterprise
Run ID: 180b0cbc-176f-4fb1-974b-aed444feab89
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!Cargo.lock
📒 Files selected for processing (18)
crates/switchyard-py/Cargo.tomlcrates/switchyard-py/src/errors.rscrates/switchyard-py/src/lib.rscrates/switchyard-py/src/libsy_bindings.rscrates/switchyard-py/src/libsy_bindings/protocol.rscrates/switchyard-py/src/libsy_bindings/run.rscrates/switchyard-py/src/libsy_bindings/target.rscrates/switchyard-py/src/libsy_bindings/values.rsexamples/libsy.pyswitchyard/__init__.pyswitchyard/libsy/__init__.pyswitchyard/libsy/algorithms.pyswitchyard/libsy/protocol.pyswitchyard_rust/libsy.pyswitchyard_rust/libsy.pyiswitchyard_rust/libsy_protocol.pyswitchyard_rust/libsy_protocol.pyitests/test_libsy_bindings.py
| use super::values::{PyContext, PyRequest, PyResponse}; | ||
| use crate::errors::py_libsy_error; | ||
|
|
||
| #[pyclass(name = "LlmCall")] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant file with line numbers
sed -n '1,240p' crates/switchyard-py/src/libsy_bindings/run.rs | cat -n
printf '\n---\n'
# Find all #[pyclass] declarations in this crate
rg -n '#\[pyclass' crates/switchyard-py/src -S
printf '\n---\n'
# Find module registration / Python module path references
rg -n 'switchyard\.libsy|module\s*=\s*"' crates/switchyard-py/src -SRepository: NVIDIA-NeMo/Switchyard
Length of output: 21055
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the surrounding binding module to understand how these classes are exposed
fd -a 'mod.rs' crates/switchyard-py/src crates/switchyard-py/src/libsy_bindings 2>/dev/null || true
fd -a 'lib.rs' crates/switchyard-py/src 2>/dev/null || true
printf '\n---\n'
# Look at the parent module file if present
for f in crates/switchyard-py/src/lib.rs crates/switchyard-py/src/libsy_bindings/mod.rs; do
if [ -f "$f" ]; then
echo "FILE: $f"
sed -n '1,220p' "$f" | cat -n
echo
fi
doneRepository: NVIDIA-NeMo/Switchyard
Length of output: 1172
Add module = "switchyard.libsy" to the exported classes in crates/switchyard-py/src/libsy_bindings/run.rs:19,107,131
PyLlmCall, PyStep, and PyRunStream still omit module, so their Python __module__ will default to builtins instead of switchyard.libsy.
🤖 Prompt for 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.
In `@crates/switchyard-py/src/libsy_bindings/run.rs` at line 19, Update the
PyLlmCall, PyStep, and PyRunStream pyclass declarations in the run bindings to
include module = "switchyard.libsy", ensuring all three exported classes report
the correct Python __module__.
| fn schedule_stream_source_close(source: Py<PyAny>) { | ||
| let result = Python::attach(|py| { | ||
| let source = source.bind(py); | ||
| if source.hasattr("aclose")? { | ||
| let awaitable = source.call_method0("aclose")?; | ||
| pyo3_async_runtimes::tokio::into_future(awaitable).map(Some) | ||
| } else if source.hasattr("close")? { | ||
| source.call_method0("close")?; | ||
| Ok(None) | ||
| } else { | ||
| Ok(None) | ||
| } | ||
| }); | ||
| match result { | ||
| Ok(Some(future)) => { | ||
| if let Ok(runtime) = tokio::runtime::Handle::try_current() { | ||
| runtime.spawn(async move { | ||
| if let Err(error) = future.await { | ||
| tracing::warn!(error = %error, "libsy response stream close failed"); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Keep aclose() scheduled even when no Tokio handle is entered
SourceClosingLlmStream::drop can run outside an entered Tokio runtime, so this branch may drop the aclose() future instead of awaiting it. Route this cleanup through an always-available executor to avoid leaking the upstream stream.
🤖 Prompt for 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.
In `@crates/switchyard-py/src/libsy_bindings/values.rs` around lines 600 - 621,
Update schedule_stream_source_close so the Some(future) branch does not discard
the aclose future when Handle::try_current() fails. Schedule and await the
future through an always-available executor, preserving the existing warning
behavior when awaiting it fails.
| class WireFormat: | ||
| OPENAI_CHAT: ClassVar[WireFormat] | ||
| ANTHROPIC_MESSAGES: ClassVar[WireFormat] | ||
| OPENAI_RESPONSES: ClassVar[WireFormat] | ||
|
|
||
| value: str | ||
|
|
||
| class FormatId: | ||
| value: str | ||
|
|
||
| def __init__(self, value: str) -> None: ... | ||
| @classmethod | ||
| def known(cls, value: WireFormat) -> FormatId: ... | ||
|
|
||
| class Role: | ||
| SYSTEM: ClassVar[Role] | ||
| DEVELOPER: ClassVar[Role] | ||
| USER: ClassVar[Role] | ||
| ASSISTANT: ClassVar[Role] | ||
| TOOL: ClassVar[Role] | ||
|
|
||
| value: str | ||
|
|
||
| class StopReason: | ||
| END_TURN: ClassVar[StopReason] | ||
| MAX_TOKENS: ClassVar[StopReason] | ||
| TOOL_USE: ClassVar[StopReason] | ||
| CONTENT_FILTER: ClassVar[StopReason] | ||
| ERROR: ClassVar[StopReason] | ||
| UNKNOWN: ClassVar[StopReason] | ||
|
|
||
| value: str |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Compare native frozen/getter declarations with writable stub attributes.
rg -n -A8 -B2 '#\[pyclass|#\[getter\]' crates/switchyard-py/src/libsy_bindings/protocol.rs
rg -n '^[[:space:]]+[a-z_][a-z_0-9]*: ' switchyard_rust/libsy_protocol.pyiRepository: NVIDIA-NeMo/Switchyard
Length of output: 25339
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
import re
rs = Path("crates/switchyard-py/src/libsy_bindings/protocol.rs").read_text()
pyi = Path("switchyard_rust/libsy_protocol.pyi").read_text()
# Pull out pyclass name + getter names from Rust
classes = ["WireFormat", "FormatId", "Role", "StopReason"]
for cls in classes:
# crude but sufficient for this read-only check
m = re.search(rf'#\[pyclass\([^\]]*name = "{cls}"[^\]]*\)\].*?impl Py{cls} \{{(.*?)\n\}}', rs, re.S)
getters = re.findall(r'#\[getter\]\s*\n\s*fn\s+([a-zA-Z_][a-zA-Z0-9_]*)\s*\(', m.group(1)) if m else []
print(cls, "Rust getters:", getters)
# Pull the pyi member declarations for the same classes
for cls in classes:
m = re.search(rf'class {cls}:\n(.*?)(?=\nclass |\Z)', pyi, re.S)
print(f"\n{cls} stub body:")
if m:
body = m.group(1)
for line in body.splitlines():
if re.match(r'\s+[A-Za-z_][A-Za-z0-9_]*:\s+', line):
print(line)
PYRepository: NVIDIA-NeMo/Switchyard
Length of output: 906
🏁 Script executed:
sed -n '120,170p' crates/switchyard-py/src/libsy_bindings/protocol.rsRepository: NVIDIA-NeMo/Switchyard
Length of output: 1114
Model these fields as read-only properties.
These are getter-only on frozen PyO3 classes, so value: str makes assignments type-check even though they fail at runtime. Apply the same @property treatment to WireFormat.value, FormatId.value, Role.value, and StopReason.value.
Representative stub correction
class WireFormat:
OPENAI_CHAT: ClassVar[WireFormat]
ANTHROPIC_MESSAGES: ClassVar[WireFormat]
OPENAI_RESPONSES: ClassVar[WireFormat]
- value: str
+ `@property`
+ def value(self) -> str: ...🤖 Prompt for 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.
In `@switchyard_rust/libsy_protocol.pyi` around lines 9 - 40, Update the value
attributes in WireFormat, FormatId, Role, and StopReason to read-only `@property`
declarations returning str, rather than writable annotations. Preserve
FormatId.__init__ and known while ensuring all four frozen PyO3 classes expose
getter-only value properties.
| class LlmCall: | ||
| context: Context | ||
| request: Request | ||
| decision: Decision | ||
| is_pending: bool | ||
|
|
||
| def respond(self, response: Response) -> None: ... | ||
| def fail(self, message: str) -> None: ... | ||
|
|
||
| class Step: | ||
| class CallLlm: | ||
| call: LlmCall | ||
|
|
||
| class Decision: | ||
| decision: _DecisionValue | ||
|
|
||
| class ReturnToAgent: | ||
| response: Response | ||
|
|
||
| class RunStream(AsyncIterator[Step]): | ||
| def __aiter__(self) -> RunStream: ... | ||
| async def __anext__(self) -> Step: ... | ||
| async def aclose(self) -> None: ... |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
# Locate relevant files
git ls-files | rg '^(switchyard_rust/libsy\.pyi|switchyard_rust/.*\.(rs|py|pyi))$'
echo '--- libsy.pyi (lines around 41-63) ---'
nl -ba switchyard_rust/libsy.pyi | sed -n '1,120p'
echo '--- search for LlmCall / RunStream / Step in Rust source ---'
rg -n "struct LlmCall|enum Step|struct RunStream|LlmCall|RunStream|Step::|CallLlm|ReturnToAgent|Decision" switchyard_rust -g '!**/target/**' -g '!**/build/**'Repository: NVIDIA-NeMo/Switchyard
Length of output: 586
🏁 Script executed:
echo 'waiting'Repository: NVIDIA-NeMo/Switchyard
Length of output: 168
🏁 Script executed:
set -euo pipefail
echo '--- switchyard_rust/libsy.pyi (around symbols) ---'
python3 - <<'PY'
from pathlib import Path
p = Path('switchyard_rust/libsy.pyi')
for i, line in enumerate(p.read_text().splitlines(), 1):
if 1 <= i <= 140:
print(f"{i:4d}: {line}")
PY
echo '--- switchyard_rust/libsy.py search hits ---'
rg -n "class LlmCall|class Step|class RunStream|def __aiter__|def __anext__|`@property`|decision|context|request|is_pending|respond|fail" switchyard_rust/libsy.py switchyard_rust/libsy_protocol.py
echo '--- switchyard_rust/libsy.py around matches ---'
python3 - <<'PY'
from pathlib import Path
for file in ['switchyard_rust/libsy.py', 'switchyard_rust/libsy_protocol.py']:
print(f"\n### {file}")
text = Path(file).read_text().splitlines()
hits = [i for i, line in enumerate(text, 1) if any(k in line for k in [
'class LlmCall','class Step','class RunStream','def __aiter__','def __anext__',
'`@property`','decision','context','request','is_pending','respond','fail'
])]
shown = set()
for h in hits:
for i in range(max(1, h-6), min(len(text), h+10)+1):
if i not in shown:
print(f"{i:4d}: {text[i-1]}")
shown.add(i)
print('---')
PYRepository: NVIDIA-NeMo/Switchyard
Length of output: 3602
🏁 Script executed:
echo 'awaiting source slices'Repository: NVIDIA-NeMo/Switchyard
Length of output: 183
🏁 Script executed:
set -euo pipefail
echo '--- switchyard_rust/libsy.py ---'
python3 - <<'PY'
from pathlib import Path
p = Path('switchyard_rust/libsy.py')
for i, line in enumerate(p.read_text().splitlines(), 1):
if i <= 220:
print(f"{i:4d}: {line}")
PY
echo '--- switchyard_rust/libsy_protocol.pyi ---'
python3 - <<'PY'
from pathlib import Path
p = Path('switchyard_rust/libsy_protocol.pyi')
for i, line in enumerate(p.read_text().splitlines(), 1):
if i <= 240:
print(f"{i:4d}: {line}")
PY
echo '--- switchyard_rust/libsy_protocol.py ---'
python3 - <<'PY'
from pathlib import Path
p = Path('switchyard_rust/libsy_protocol.py')
for i, line in enumerate(p.read_text().splitlines(), 1):
if i <= 220:
print(f"{i:4d}: {line}")
PYRepository: NVIDIA-NeMo/Switchyard
Length of output: 11544
🏁 Script executed:
set -euo pipefail
echo '--- all references to LlmCall / Step / RunStream ---'
rg -n "\bLlmCall\b|\bRunStream\b|\bStep\b" .
echo '--- libsy.pyi context around Step and RunStream ---'
python3 - <<'PY'
from pathlib import Path
p = Path('switchyard_rust/libsy.pyi')
text = p.read_text().splitlines()
for start, end in [(41, 63)]:
for i in range(start, end + 1):
print(f"{i:4d}: {text[i-1]}")
PYRepository: NVIDIA-NeMo/Switchyard
Length of output: 12441
🏁 Script executed:
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('crates/switchyard-py/src/libsy_bindings/run.rs')
text = p.read_text().splitlines()
for start, end in [(1, 240)]:
for i in range(start, min(end, len(text)) + 1):
print(f"{i:4d}: {text[i-1]}")
PYRepository: NVIDIA-NeMo/Switchyard
Length of output: 9332
🏁 Script executed:
echo 'waiting for run.rs slice'Repository: NVIDIA-NeMo/Switchyard
Length of output: 185
🏁 Script executed:
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('crates/switchyard-py/src/libsy_bindings/run.rs')
text = p.read_text().splitlines()
for i, line in enumerate(text, 1):
if i <= 240:
print(f"{i:4d}: {line}")
PYRepository: NVIDIA-NeMo/Switchyard
Length of output: 9332
🏁 Script executed:
set -euo pipefail
rg -n -C 4 '#\[pyclass\(name = "LlmCall"\)|#\[getter\]|#\[pymethods\]|fn (context|request|decision|is_pending|respond|fail)|#\[pyclass\(name = "Step"|#\[pyclass\(name = "RunStream"' crates/switchyard-py/src/libsy_bindings/run.rsRepository: NVIDIA-NeMo/Switchyard
Length of output: 2949
Model LlmCall fields as properties context, request, decision, and is_pending are getter-only at runtime, so plain annotations make them look writable in type checkers.
🤖 Prompt for 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.
In `@switchyard_rust/libsy.pyi` around lines 41 - 63, Update the LlmCall fields
context, request, decision, and is_pending to be modeled as read-only properties
in the type stub, using the appropriate getter-only property syntax rather than
plain writable annotations; leave respond and fail unchanged.
f7736f9 to
ddedbd3
Compare
Signed-off-by: nachiketb <nachiketb@nvidia.com>
a5b1a1f to
04e5eed
Compare
ddedbd3 to
efceca0
Compare
Stack
Review this PR last, after #87.
Why
Some Python hosts own their model transport and need to drive every model call themselves instead of using target clients through Algorithm.run().
What
How
Each Python RunStream owns one Rust StepStream. Model-call promises move into LlmCall and can be fulfilled once. Terminal results tear down the underlying stream promptly.
What to review
Validation