feat(python): expose libsy run stream - #392
Conversation
|
WalkthroughThe libsy Python bindings now use Rust-owned algorithms with asynchronous streaming. Python receives model calls, routing decisions, and terminal responses, then completes model calls through Changeslibsy streaming API
Estimated code review effort: 4 (Complex) | ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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
🧹 Nitpick comments (3)
tests/test_libsy_minimal_bindings.py (2)
259-267: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the
ModelCall.failguard paths.
PyModelCall::failhas two guards that no test exercises. It raisesTypeErrorwith "error must derive from BaseException" for a non-exception argument. It raisesLibsyErrorwith "model call has already been completed" for a second completion. The duplicate-completion path is covered forrespondat line 90 but not forfail.Add cases inside a
Step.CallModelbranch:with pytest.raises(TypeError, match="must derive from BaseException"): call.fail("not an exception") call.fail(RuntimeError("boom")) with pytest.raises(LibsyError, match="already been completed"): call.fail(RuntimeError("boom"))🤖 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 `@tests/test_libsy_minimal_bindings.py` around lines 259 - 267, Add coverage for both PyModelCall.fail guards in the relevant Step.CallModel test branch: assert that passing a non-BaseException raises TypeError with the expected message, then complete the call with RuntimeError and assert a second fail raises LibsyError indicating the model call is already completed.
62-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGive a clear failure when the test forgets to register a client.
Line 63 indexes
clientsoutside thetryblock. If a test omits a client for the selected target, the helper raises a bareKeyErrorwhile theCallModelis still pending. The stream then drops and the algorithm task aborts, so the reported failure does not name the missing target.Raise an explicit error instead.
♻️ Proposed refactor
case Step.CallModel(call): target = call.decision["selected_model_id"] - client = (clients or {})[target] + client = (clients or {}).get(target) + if client is None: + raise AssertionError(f"no client registered for target {target!r}") try: response = await client.call(call.request) except Exception as error: call.fail(error) else: call.respond(response)🤖 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 `@tests/test_libsy_minimal_bindings.py` around lines 62 - 69, Move the client lookup in the call-handling flow into the existing try block so missing registrations are captured before the CallModel remains pending. Convert a missing target from the clients mapping into an explicit error that identifies the selected target, then continue using call.fail(error) while preserving the existing response path for registered clients.crates/switchyard-py/src/libsy_bindings.rs (1)
164-181: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider exposing
CallModel::algorithmonModelCall.
PyModelCall::newkeeps onlycall.request.llm_requestand the decision. The upstreamCallModelalso carriesalgorithm, which exists so a host that instruments the calls it serves can attribute its own spans to the algorithm behind them. A Python host cannot read that field now.Add an
algorithmgetter if host-side span attribution is a goal for this API.♻️ Proposed addition
struct PyModelCall { inner: Option<CallModel>, + algorithm: String, request: Py<PyAny>, decision: Py<PyAny>, }/// The name of the algorithm that produced this call. #[getter] fn algorithm(&self) -> &str { &self.algorithm }Update
switchyard_rust/libsy.pyif you add the getter.🤖 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.rs` around lines 164 - 181, Expose the upstream CallModel algorithm through PyModelCall so Python hosts can attribute spans to the producing algorithm. Preserve the value in PyModelCall::new, add an algorithm getter returning its name, and update switchyard_rust/libsy.py to expose the corresponding property.
🤖 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 `@examples/libsy.py`:
- Around line 13-22: Update EchoClient to accept the selected model during
construction and have call return that stored model instead of the hardcoded
"echo" value. In the example’s routing flow, instantiate EchoClient with
decision.selected_model_id() so the final response matches the printed routing
decision, following the constructor pattern used by the test EchoClient(model).
In `@switchyard_rust/libsy.py`:
- Around line 19-20: Preserve a deprecation path for the removed public APIs: in
switchyard_rust/libsy.py at lines 19-20, document migration for LlmClient and
Algorithm.run or retain a deprecated LlmClient alias that raises
DeprecationWarning, and apply the same decision in switchyard/libsy/__init__.py
at lines 23-24 so both __all__ export surfaces remain consistent.
---
Nitpick comments:
In `@crates/switchyard-py/src/libsy_bindings.rs`:
- Around line 164-181: Expose the upstream CallModel algorithm through
PyModelCall so Python hosts can attribute spans to the producing algorithm.
Preserve the value in PyModelCall::new, add an algorithm getter returning its
name, and update switchyard_rust/libsy.py to expose the corresponding property.
In `@tests/test_libsy_minimal_bindings.py`:
- Around line 259-267: Add coverage for both PyModelCall.fail guards in the
relevant Step.CallModel test branch: assert that passing a non-BaseException
raises TypeError with the expected message, then complete the call with
RuntimeError and assert a second fail raises LibsyError indicating the model
call is already completed.
- Around line 62-69: Move the client lookup in the call-handling flow into the
existing try block so missing registrations are captured before the CallModel
remains pending. Convert a missing target from the clients mapping into an
explicit error that identifies the selected target, then continue using
call.fail(error) while preserving the existing response path for registered
clients.
🪄 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: CHILL
Plan: Enterprise
Run ID: d74a9cd8-f85e-4341-9f8e-c171cfd63f94
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!Cargo.lock
📒 Files selected for processing (6)
crates/switchyard-py/Cargo.tomlcrates/switchyard-py/src/libsy_bindings.rsexamples/libsy.pyswitchyard/libsy/__init__.pyswitchyard_rust/libsy.pytests/test_libsy_minimal_bindings.py
008ae90 to
5af7a75
Compare
433a050 to
f569a09
Compare
Signed-off-by: nachiketb <nachiketb@nvidia.com>
f569a09 to
d35d417
Compare
1 participant
What
Expose the redesigned libsy
Algorithm::run_streamcontract directly to Python. Python now consumes PyO3 complex-enumStep.CallModel,Step.Decision, andStep.Donevalues, with a typedDecisionobject and dictionary-based LLM requests and responses.Why
The previous binding retained the deleted managed
runabstraction and hid model calls behind a Python client stored on each target. The new API matches libsy after PR #373: algorithms route by model ID, and the stream consumer decides how each model call is served.How
Algorithm.runwith an asyncAlgorithm.run_streamiteratorStepas a PyO3 complex enum andModelCallas a one-shot reply handleDecisionPyO3 objectLlmTargetandLlmClientwrappersswitchyard-llm-clientandasync-traitfrom the PyO3 crateexamples/libsy.pyto handle the stream directly with one shared clientPython usage
Decision-only integrations answer intermediate judge or classifier calls, then take the final
answer call without serving it:
into_parts()returns the final decision and request after algorithm prompt rewriting. It marksthe run as deliberately abandoned rather than failed, matching the decision-only contract in
issue #310.
Intentional Python differences
The stream control flow mirrors Rust, while the boundary remains Python-native:
ModelIdvalues cross the Python boundary as stringsStep.Doneaggregates a streaming Rust response before it crosses into PythonCallModel::respond(Result<Response>)is split intorespond(mapping)andfail(exception)Algorithmis an opaque handle created by the provided factories; Python cannot implement the Rust traitrun_stream; raw request metadata is not exposedWhat to review
ModelCall.respond/ModelCall.failownershipDecisionaccess across stream steps, model calls, andinto_parts()ModelCall.into_parts()behaviorValidation
uv run pytest tests/ -v(137 passed)uv run python examples/libsy.pyuv run ruff check .uv run mypy examples/libsy.py switchyard switchyard_rustcargo test -p switchyard-pycargo clippy -p switchyard-py --all-targets -- -D warningscargo fmt --all --checkSummary by CodeRabbit
New Features
ModelCallandSteptypes for handling model interactions.Changes
run_stream.