feat: expose custom rollout planes through SDK.run - #1110
Conversation
Forward an optional RolloutPlanes instance from the high-level SDK.run API to the existing RolloutConfig.planes composition point. Omitting the argument preserves current default behavior. Document the argument and add regression coverage proving both custom-instance identity and unchanged default construction. This enables per-run composition without adding a new factory or policy framework.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 1 potential issue.
2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| self_gen_no_internet: bool = False, | ||
| source_provenance: dict[str, Any] | None = None, | ||
| usage_tracking: Any = None, | ||
| planes: RolloutPlanes | None = None, |
There was a problem hiding this comment.
🟡 False-valued custom planes are ignored
False-valued custom plane bundles passed through planes are replaced by defaults. Rollout.__init__ uses truthiness although only None selects defaults.
Prompt for agents
SDK.run now accepts any RolloutPlanes implementation and documents None as the default sentinel. Rollout.__init__ in src/benchflow/rollout/__init__.py currently selects config.planes with an `or` expression, so a valid custom implementation with false truthiness is discarded. Change plane selection to distinguish None explicitly, and add a wiring test using a false-valued RolloutPlanes test double rather than only a truthy MagicMock.
Was this helpful? React with 👍 or 👎 to provide feedback.
Use None as the sole default sentinel so valid custom RolloutPlanes implementations are never discarded based on truthiness. Cover SDK wiring with a false-valued planes test double and keep the focused custom-path regression.
|
🔥 Thermo-nuclear review — final head Verdict: no blocking code-quality finding. Evidence:
Independent human approval is still required before merge. |
2 participants
Problem
RolloutConfigalready supports customRolloutPlanes, butSDK.run()did not expose that composition point. High-level SDK callers therefore could not provide per-run plane behavior without bypassing the SDK or modifying global defaults.Change
Add an optional
planesargument toSDK.run()and forward it unchanged toRolloutConfig:Omitting the argument preserves current default behavior. This reuses the existing plane contract; no factory, policy framework, or new lifecycle abstraction is introduced.
Relationship to #1107
Refs #1107. This supplies a supported integration point for consumers to apply agent-specific connector and credential policies per rollout without global monkey patches. It does not define a connector policy, disable hosted Apps/MCP, validate an effective tool inventory, or fully address #1107.
Validation
ty check src/passedFull-suite note: three host-sensitive failures were reproduced as environmental. Two credential tests observed the existing host Claude login, and one terminal assertion wrapped at the current Rich console width; those tests pass with a sterile home and wider terminal.