feat(libsy): decide api - #207
Conversation
WalkthroughThe PR introduces decision-only request execution, adds ChangesDecision-only execution
Estimated code review effort: 5 (Critical) | ~120 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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)
crates/libsy/src/core/algorithm.rs (1)
736-757: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPrefer an error item over
unreachable!for the stream-shape mismatch.
run_stream_inneris an overridable provided method; an override that returns the other variant turns into a panic in a public entry point rather than aLibsyError. Yielding a singleErr(LibsyError::AlgorithmError { .. })stream keeps the failure mode consistent with the rest of the driver, which already decodes incompatible terminals as errors.🤖 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/libsy/src/core/algorithm.rs` around lines 736 - 757, The public stream helpers around run_stream_inner must handle an unexpected AnyStepStream variant without panicking. Replace both unreachable! branches in run_step_stream and run_decision_only_stream with streams yielding a single Err(LibsyError::AlgorithmError { .. }) item, preserving the existing successful variant handling and using the established error-stream construction pattern.crates/libsy/src/core/classifier.rs (1)
45-55: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTighten
max_classification’s doc. The current wording is redundant; spell out thatAmbiguouspreserves the variant, so a plainargmax(false)on the result still yieldsNoneunless callers opt into ignoring ambiguity.🤖 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/libsy/src/core/classifier.rs` around lines 45 - 55, Update the documentation for max_classification to remove the redundant wording and state that the Ambiguous variant is preserved. Document that calling argmax(false) on the result still returns None unless ambiguity is explicitly ignored.crates/libsy/examples/ensemble.rs (1)
386-398: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
decideon this example fails while still exploring.The exploration branch always yields
ResponseOrDecision::Response, which the decision-only stream rejects (crates/libsy/src/core/algorithm.rs:308-341). SoEnsembleOrchAlgo::decideerrors until the algorithm commits. That's a defensible design for an ensemble, but worth stating in the type/method docs so example readers aren't surprised.🤖 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/libsy/examples/ensemble.rs` around lines 386 - 398, Document the exploration behavior in the relevant EnsembleOrchAlgo decision method/type docs near the shown branch: while no committed model is available, decide returns a ResponseOrDecision::Response from the winning candidate, and the decision-only stream therefore rejects it until commitment. Clarify that this is intentional ensemble behavior without changing the implementation.
🤖 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/libsy/src/core/algorithm.rs`:
- Around line 35-40: Add `///` Rust documentation to the public
`DecisionOnlyStepStream` type alias, the `AnyStepStream` enum and each of its
variants, and the `ResponseOrDecision` enum and each of its variants. Match the
concise documentation style used by neighboring public items such as
`DecidedCall` and `DecisionOnlyStep`.
- Around line 289-290: Update the terminal-decoder comments in
crates/libsy/src/core/algorithm.rs at lines 289-290 and 322-323. In stream(),
explain that a decision-only terminal is an error because the caller requested a
served response; in decision_only_stream(), explain that a served response is an
error because the stream expects a decision handoff.
---
Nitpick comments:
In `@crates/libsy/examples/ensemble.rs`:
- Around line 386-398: Document the exploration behavior in the relevant
EnsembleOrchAlgo decision method/type docs near the shown branch: while no
committed model is available, decide returns a ResponseOrDecision::Response from
the winning candidate, and the decision-only stream therefore rejects it until
commitment. Clarify that this is intentional ensemble behavior without changing
the implementation.
In `@crates/libsy/src/core/algorithm.rs`:
- Around line 736-757: The public stream helpers around run_stream_inner must
handle an unexpected AnyStepStream variant without panicking. Replace both
unreachable! branches in run_step_stream and run_decision_only_stream with
streams yielding a single Err(LibsyError::AlgorithmError { .. }) item,
preserving the existing successful variant handling and using the established
error-stream construction pattern.
In `@crates/libsy/src/core/classifier.rs`:
- Around line 45-55: Update the documentation for max_classification to remove
the redundant wording and state that the Ambiguous variant is preserved.
Document that calling argmax(false) on the result still returns None unless
ambiguity is explicitly ignored.
🪄 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: b6341a0c-e4b6-4648-8c5f-4c615862d1be
📒 Files selected for processing (19)
crates/libsy/README.mdcrates/libsy/examples/ensemble.rscrates/libsy/src/algorithms/fall_through.rscrates/libsy/src/algorithms/llm_class.rscrates/libsy/src/algorithms/noop.rscrates/libsy/src/algorithms/passthrough.rscrates/libsy/src/algorithms/rand.rscrates/libsy/src/algorithms/stage.rscrates/libsy/src/algorithms/subagent_affinity_tests.rscrates/libsy/src/algorithms/util/affinity.rscrates/libsy/src/algorithms/util/llm_judge.rscrates/libsy/src/algorithms/util/stage.rscrates/libsy/src/algorithms/util/subagent.rscrates/libsy/src/core/algorithm.rscrates/libsy/src/core/classifier.rscrates/libsy/src/lib.rscrates/libsy/src/observability.rscrates/libsy/tests/observability.rscrates/libsy/tests/prompts.rs
| pub type DecisionOnlyStepStream = Pin<Box<dyn Stream<Item = Result<DecisionOnlyStep>> + Send>>; | ||
|
|
||
| pub enum AnyStepStream { | ||
| Step(StepStream), | ||
| DecisionOnly(DecisionOnlyStepStream), | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add /// docs to the new public items.
DecisionOnlyStepStream, AnyStepStream (and its variants), and ResponseOrDecision (and its variants) are public but undocumented, unlike the neighbouring DecidedCall/DecisionOnlyStep.
📝 Suggested docs
+/// A stream of [`DecisionOnlyStep`]s: the decision-only shape of a run.
pub type DecisionOnlyStepStream = Pin<Box<dyn Stream<Item = Result<DecisionOnlyStep>> + Send>>;
+/// Either step-stream shape a run can be driven as, selected by the entry point.
pub enum AnyStepStream {
+ /// A run that serves its final call.
Step(StepStream),
+ /// A run that hands its final call back.
DecisionOnly(DecisionOnlyStepStream),
}+/// The terminal payload of a run: the served response, or the decided call handed back.
pub enum ResponseOrDecision {
+ /// The run served its final call and this is the answer.
Response(Box<Response>),
+ /// The run stopped at its final decision; see [`DecidedCall`].
Decision((Arc<dyn Decision>, Box<Request>, Option<Box<Response>>)),
}As per coding guidelines: "Add documentation for public functions, classes, methods, API entry points, and public Rust items; ... Rust uses ///".
Also applies to: 350-353
🤖 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/libsy/src/core/algorithm.rs` around lines 35 - 40, Add `///` Rust
documentation to the public `DecisionOnlyStepStream` type alias, the
`AnyStepStream` enum and each of its variants, and the `ResponseOrDecision` enum
and each of its variants. Match the concise documentation style used by
neighboring public items such as `DecidedCall` and `DecisionOnlyStep`.
Source: Coding guidelines
| // Algorithms always run with `decision_only` off today, so the terminal payload | ||
| // is always a served response. A handoff has no path to the agent yet. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Pre-decision_only comment left behind and duplicated in both terminal decoders. The text "Algorithms always run with decision_only off today, so the terminal payload is always a served response. A handoff has no path to the agent yet" predates this PR, which adds exactly that path; it is now misleading in stream() and inverted in decision_only_stream().
crates/libsy/src/core/algorithm.rs#L289-L290: replace with why a decision-only terminal is an error on the served stream (the caller asked for a response).crates/libsy/src/core/algorithm.rs#L322-L323: replace with the mirror rationale — this stream expects the decision handoff, so a served response means the algorithm ignored the mode.
📍 Affects 1 file
crates/libsy/src/core/algorithm.rs#L289-L290(this comment)crates/libsy/src/core/algorithm.rs#L322-L323
🤖 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/libsy/src/core/algorithm.rs` around lines 289 - 290, Update the
terminal-decoder comments in crates/libsy/src/core/algorithm.rs at lines 289-290
and 322-323. In stream(), explain that a decision-only terminal is an error
because the caller requested a served response; in decision_only_stream(),
explain that a served response is an error because the stream expects a decision
handoff.
5cdcc2c to
280bf91
Compare
Signed-off-by: Greg Clark <grclark@nvidia.com> chore(libsy): wip Signed-off-by: Greg Clark <grclark@nvidia.com> feat(libsy): finish up decide api Signed-off-by: Greg Clark <grclark@nvidia.com>
Signed-off-by: Greg Clark <grclark@nvidia.com>
Signed-off-by: Greg Clark <grclark@nvidia.com>
Signed-off-by: Greg Clark <grclark@nvidia.com>
280bf91 to
456c0c3
Compare
1 participant
What
Added
decideapiStill allows offloading, but add a
Driver::final_decisionmethod whichAlgorithmsuse to signal this model call is the selected model.So
run_decision_only_streamfunctions the same asrun_steamexcept the last model call is return as aDecidedCallinstead of aCallLlmand then aReturnToAgentIn the streaming case
Driver::final_decisioncallsDriver::call_llm_target.In the decide case if simply returns a
DecidedCallcontaining theDecision,Requestand optionally theResponseif the algo happens to compute it as part of the decision (e.g. escalation router)changes
create_run_tasksignature to support both decide and stream apisAlgorithms don't change other than
create_run_tasksignature and callingfinal_decisionDriverhold adecide_onlyflag which is set when theDriveris created indecide,run,run_stream, etcWhy
Integrators want the ability to just get the decision without having libsy run the final model request.
Closes #
How tested
uv run ruff check .cleanuv run mypy switchyardcleanuv run pytest tests/greenChecklist
snake_caseof the primary class.switchyard/__init__.py.__all__if intended for downstream use.--helpupdated if customer-facing surface changed.Signed-off-by: Your Name <email>) per the DCO.Notes for reviewers
Anything reviewers should pay extra attention to — risky paths, follow-up tickets, intentional trade-offs.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation