Repository navigation
Add invocation-scoped code interpreter bindings - #10137
isaacbmiller wants to merge 1 commit into
Conversation
Greptile SummaryThe change introduces invocation-scoped interpreter binding and restores caller-owned interpreter configuration after RLM execution. Sequential reuse was exercised and did not retain a prior invocation's tools: each call observed its own tool set and the original backend tools were restored. However, custom interpreters with an unrelated Confidence Score: 2/5Not safe to merge until caller-owned interpreter binding is made compatible with unrelated backend APIs and concurrent ownership is made exclusive or rejected. Two blocking failures remain: one prevents valid custom interpreter implementations from running, and one permits request-scoped tool isolation to break during overlapping calls. Files Needing Attention: dspy/primitives/code_interpreter.py, dspy/predict/rlm.py
|
ee474b3 to
b9cb2d5
Compare
b9cb2d5 to
d903e10
Compare
755b1d3 to
dafb766
Compare
797afdc to
4b1daea
Compare
4b1daea to
7c81bdc
Compare
41e56c7 to
326da97
Compare
326da97 to
0b0cdfa
Compare
0b0cdfa to
3aa3c16
Compare
3aa3c16 to
2adf180
Compare
2adf180 to
22ada11
Compare
| if callable(bind := getattr(interpreter, "bind", None)): | ||
| bind(tools=tools, output_fields=output_fields) |
There was a problem hiding this comment.
Unrelated bind methods break caller-owned interpreters
_bind_interpreter() treats any callable attribute named bind as DSPy's optional binding API and unconditionally passes tools= and output_fields=. A caller-owned object can satisfy CodeInterpreter while using an unrelated bind(connection) method; RLM setup then raises TypeError before execution. Invoke an explicitly compatible binding interface, or use the protocol-owned state-update fallback when that signature is unsupported.
Artifacts
Focused reproduction source for a protocol-valid interpreter with bind(connection)
- The authored runtime script defines a structurally valid caller-owned interpreter with an unrelated one-argument bind method and invokes validation plus binding; it isolates the failing path.
Runtime output showing bind(connection) rejects RLM keyword arguments
- Running the reproduction with the repository virtual environment accepted the interpreter as CodeInterpreter, then captured the unexpected-tools-keyword TypeError and unchanged tools; the claim is verified.
22ada11 to
f1605ea
Compare
| if callable(bind := getattr(interpreter, "bind", None)): | ||
| bind(tools=tools, output_fields=output_fields) |
There was a problem hiding this comment.
Unrelated bind methods are invoked as DSPy bindings
A caller-owned object can satisfy CodeInterpreter while exposing an unrelated bind(connection) method. _bind_interpreter() treats any callable attribute named bind as DSPy's binding API and calls it with tools= and output_fields=, raising TypeError before RLM execution. Restrict this path to an explicit compatible binding interface, or fall back to updating the protocol-owned interpreter state when that signature is unsupported.
Artifacts
Focused caller-owned lifecycle pytest source
- Authored pytest source creates protocol-conforming interpreters and executes the exact incompatible-bind, sequential-reuse, and concurrent-overlap paths; it is the runnable proof harness.
Focused caller-owned lifecycle validation output
- Captured output of `.venv/bin/python -m pytest -q trex-artifacts/caller-owned-lifecycle-validation.py` from `/home/user/repo`, showing all three focused assertions passed and confirming the observed current behaviors.
| original = (dict(interpreter.tools), getattr(interpreter, "output_fields", None)) | ||
| try: | ||
| self._inject_execution_context(interpreter, execution_tools) | ||
| yield interpreter | ||
| finally: | ||
| _bind_interpreter(interpreter, *original) |
There was a problem hiding this comment.
Caller-owned interpreter bindings race across concurrent RLM calls
Two overlapping RLM calls can mutate the same caller-owned interpreter without exclusive ownership of its bindings. Each call independently snapshots tools, injects invocation-specific tools, executes, and restores its snapshot. Concurrent execution causes one invocation to observe the other invocation's llm_query closure and can leave invocation tools registered after both calls complete. This breaks invocation-scoped tool isolation and can expose a tool from one request to another. Serialize the complete snapshot/bind/execute/restore lifecycle per caller-owned interpreter, or explicitly reject overlapping use.
Artifacts
Focused caller-owned lifecycle pytest source
- Authored pytest source creates protocol-conforming interpreters and executes the exact incompatible-bind, sequential-reuse, and concurrent-overlap paths; it is the runnable proof harness.
Focused caller-owned lifecycle validation output
- Captured output of `.venv/bin/python -m pytest -q trex-artifacts/caller-owned-lifecycle-validation.py` from `/home/user/repo`, showing all three focused assertions passed and confirming the observed current behaviors.
f1605ea to
923fec4
Compare
923fec4 to
5958810
Compare
| if callable(bind := getattr(interpreter, "bind", None)): | ||
| bind(tools=tools, output_fields=output_fields) |
There was a problem hiding this comment.
Unrelated bind methods are invoked as DSPy bindings
CodeInterpreter does not define bind, so a caller-owned interpreter can validly satisfy the protocol while using bind for an unrelated API. _bind_interpreter() treats any callable attribute with that name as DSPy's optional binding interface and calls it with tools= and output_fields=. A structurally compliant interpreter exposing bind(connection) therefore raises TypeError before RLM execution. Use an explicit, documented execution-binding capability, or fall back to updating the protocol-owned state when that capability is unavailable.
Artifacts
Focused RLM caller-owned bind collision reproducer source
- Authored minimal Python fixture that satisfies CodeInterpreter structurally while defining an unrelated bind(connection) method, then invokes RLM. It isolates the incompatible binding call.
Baseline run for structurally compliant caller-owned interpreter
- Executed baseline command from /home/user/repo shows the fixture passes CodeInterpreter validation and its bind(connection) API succeeds, establishing the pre-RLM condition.
RLM setup failure with unrelated caller-owned bind method
- Executed current-code RLM command exits 1 and traces _bind_interpreter line 184 calling bind with tools=, which raises the reproduced TypeError.
Existing interpreter bind test selection result
- Executed existing bind-focused pytest selection; both selected tests were skipped due to unavailable interpreter fixtures, so the focused runtime reproducer is the direct proof.
| original = (dict(interpreter.tools), getattr(interpreter, "output_fields", None)) | ||
| try: | ||
| self._inject_execution_context(interpreter, execution_tools) | ||
| yield interpreter | ||
| finally: | ||
| _bind_interpreter(interpreter, *original) |
There was a problem hiding this comment.
Caller-owned interpreter bindings race across concurrent RLM calls
The caller-owned path snapshots mutable bindings, installs invocation tools, yields for execution, and restores its snapshot without exclusive ownership of the interpreter. Two overlapping calls can therefore observe each other's tools, restore over each other's active bindings, and leave invocation tools behind after both calls finish. The deterministic concurrent run showed one invocation receiving the other invocation's tool, the other losing its own tool after restoration, and a stale invocation tool remaining afterward. Serialize the complete snapshot/bind/execute/restore lifecycle per caller-owned interpreter, or reject overlapping use.
Artifacts
Deterministic shared-interpreter race reproducer source
- Authored focused Python reproducer creates two overlapping RLM caller-owned interpreter scopes and forces the leak-and-restore interleaving, proving the concurrency invariant being tested.
Before capture of legacy snapshot update restore race
- Executed parent-behavior model shows invocation A seeing B's tool, B losing its tool after A restores, and the final caller binding retaining A's tool, proving the original race.
After capture of current RLM binding race
- Executed current `RLM._interpreter_context()` path produces the same leaked tools and incorrect final binding as the before capture, proving the issue remains.
Focused upstream test collection attempt
- Executed focused pytest command could not collect because optional `gepa` is absent from the project virtual environment, documenting the upstream-suite blocker.
069eab1 to
8dfbc14
Compare
| if callable(bind := getattr(interpreter, "bind", None)): | ||
| bind(tools=tools, output_fields=output_fields) |
There was a problem hiding this comment.
Unrelated bind methods break caller-owned interpreters
CodeInterpreter does not require a bind method, so a caller-owned interpreter may validly use bind(connection) for its own API. _bind_interpreter() calls every callable attribute with that name as though it accepted DSPy's tools= and output_fields= keywords. RLM setup then raises TypeError before the interpreter can start or execute. Restrict this to an explicit compatible binding capability, or fall back to updating the protocol-owned interpreter state when the binding signature is unsupported.
Artifacts
- Authored Python reproduction defining the no-bind baseline and structurally valid interpreter with unrelated bind(connection), then invoking caller-owned RLM setup.
- Executed no-bind control showing the structural CodeInterpreter passes RLM setup, starts, executes once, and returns answer ok.
- Executed structurally valid bind(connection) variant showing RLM setup raises TypeError for the unsupported tools keyword before start or execution.
Current binding source excerpt
- Executed numbered source capture showing CodeInterpreter requirements, unconditional bind invocation, and its caller-owned RLM setup path.
| _validate_interpreter(interpreter) | ||
| self._inject_execution_context(interpreter, execution_tools) | ||
| yield interpreter | ||
| original = (dict(interpreter.tools), getattr(interpreter, "output_fields", None)) | ||
| try: | ||
| self._inject_execution_context(interpreter, execution_tools) | ||
| yield interpreter | ||
| finally: | ||
| _bind_interpreter(interpreter, *original) |
There was a problem hiding this comment.
Caller-owned interpreter bindings race across concurrent RLM calls
Overlapping calls using the same caller-owned interpreter independently snapshot, bind, execute, and restore mutable tool bindings. One invocation can therefore observe another invocation's tools, lose its own binding while still active, and leave stale invocation tools after cleanup. This breaks request-scoped capability isolation and can expose tenant-specific or privileged tools across requests. Serialize the full lifecycle per caller-owned interpreter, or explicitly reject concurrent use.
Artifacts
Deterministic caller-owned interpreter lifecycle reproduction source
- Authored script loads the checked-out RLM lifecycle and coordinates sequential and overlapping caller-owned interpreter contexts, ending with a deterministic race assertion.
Sequential caller-owned interpreter lifecycle output
- Executed sequential control run restores the interpreter to only its baseline tool, establishing the non-overlapping comparison.
Overlapping caller-owned interpreter lifecycle output
- Executed overlapping run shows invocation A leaking into B, invocation B being lost while active, and A leaking after cleanup, reproducing the defect.
Focused RLM pytest collection attempt
- Executed focused pytest command could not collect because the environment lacks `litellm`, so package-level regression collection was blocked.
Worktree integrity check after validation
- Executed diff and status check confirms no tracked repository source modifications were made during validation.
- Captured repository path, commit identifier, and Python runtime used for the reproduction.
| def _bind_interpreter(interpreter, tools, output_fields=None) -> None: | ||
| if callable(bind := getattr(interpreter, "bind", None)): | ||
| bind(tools=tools, output_fields=output_fields) |
There was a problem hiding this comment.
Unrelated bind methods break caller-owned interpreters
CodeInterpreter does not define bind, but _bind_interpreter() treats any callable attribute with that name as DSPy's binding API and invokes it with tools= and output_fields=. A structurally valid caller-owned interpreter may use bind(connection) for an unrelated API; RLM setup then raises TypeError before execute() runs. Use an explicit compatible binding capability, validate the binding signature, or fall back to updating the protocol-owned state.
Artifacts
Focused reproduction source with an unrelated bind(connection) method
- Defines a structurally valid caller-owned interpreter with unrelated bind(connection) and invokes RLM, showing the exact setup path that fails before execution.
RLM setup failure with unrelated bind(connection)
- Runs the focused RLM reproduction and captures the TypeError plus execute_called=False, proving the unguarded bind call blocks execution.
Matched control source with DSPy-compatible keyword bind method
- Defines the same minimal interpreter shape with bind(*, tools, output_fields=None) so the expected RLM binding contract can be exercised.
Matched RLM control succeeds with keyword-compatible bind
- Runs the matched control and captures result_answer=setup completed plus execute_called=True, showing RLM proceeds when bind supports the assumed keywords.
Focused existing RLM interpreter lifecycle tests passing
- Runs the current RLM interpreter lifecycle regression suite, which passed 6 tests after the reproduction runs.
| if interpreter is not None: | ||
| _validate_interpreter(interpreter) | ||
| self._inject_execution_context(interpreter, execution_tools) | ||
| yield interpreter | ||
| original = (dict(interpreter.tools), getattr(interpreter, "output_fields", None)) | ||
| try: | ||
| self._inject_execution_context(interpreter, execution_tools) | ||
| yield interpreter | ||
| finally: | ||
| _bind_interpreter(interpreter, *original) | ||
| return |
There was a problem hiding this comment.
Shared interpreter calls leak invocation tools
Caller-owned interpreters are snapshotted, rebound, executed, and restored without exclusive ownership. Overlapping calls can therefore execute with another invocation's tools and leave invocation-scoped tools installed after both calls finish. The reproduced overlap made invocation A receive B's tool result and retained A's owner, llm_query, and llm_query_batched bindings afterward. Serialize the complete lifecycle per interpreter or reject concurrent use.
Artifacts
Deterministic shared-interpreter RLM concurrency reproduction source
- Executable source runs real RLM forward and aforward lifecycle paths against a barrier-controlled caller-owned interpreter; it proves the race is reproducible.
Captured deterministic reproduction source with checksum
- Command-captured SHA-256 and full source of the executed reproduction; it makes the supplied test artifact traceable.
Sequential caller-owned interpreter control output
- Sequential executions return A then B and restore only the original base tool state; it establishes the non-overlapping baseline.
Overlapping caller-owned interpreter reproduction output
- Barrier-controlled overlapping forward and aforward calls return B's tool result to A and leave A's stale context installed; it reproduces the defect.
Current RLM interpreter lifecycle lines
- Captured current source lines 507-539, 702-737, and 791-826 showing unsynchronized context mutation and both public call paths; it identifies the affected code.
Focused RLM regression suite output
- Executed `tests/predict/test_rlm.py` with 106 passing and 39 skipped tests; existing focused coverage remains green despite the concurrency defect.
8dfbc14 to
becd946
Compare
becd946 to
557c31d
Compare
|
|
||
| def _bind_interpreter(interpreter, tools, output_fields=None) -> None: | ||
| if callable(bind := getattr(interpreter, "bind", None)): | ||
| bind(tools=tools, output_fields=output_fields) |
There was a problem hiding this comment.
Incidental bind method breaks RLM
CodeInterpreter does not define a bind method, but _bind_interpreter() invokes any callable attribute with that name using DSPy's tools= and output_fields= keywords. A caller-owned interpreter can otherwise conform to the protocol while using bind(connection) for an unrelated API; RLM setup then raises TypeError before execute() runs. Restrict this path to an explicit compatible binding capability, validate the supported signature, or use the protocol-owned mutable-state fallback when it is unavailable.
Artifacts
Focused RLM caller-owned interpreter binding reproducer source
- The authored executable defines protocol-conforming control and unrelated-bind caller-owned interpreters and invokes the RLM setup path; it is the exact script used for the comparison.
Control interpreter reaches execute during RLM caller-owned setup
- Running the focused script with a conforming interpreter that has no bind method returned `reached-execute` with `execute_calls=1`; ordinary caller-owned setup reaches execution.
Unrelated bind method raises before RLM executes code
- Running the same script with an unrelated `bind(connection)` method raised the unexpected `tools` keyword TypeError with `execute_calls=0`; current RLM setup is incompatible with this conforming caller-owned interpreter.
Current RLM regression suite passes
- The current RLM test suite completed with 106 passed and 39 skipped; existing coverage does not exercise an unrelated callable bind method.
Co-authored-by: Isaac Miller <isaac@cmpnd.ai> Amp-Thread-ID: https://ampcode.com/threads/T-019fcce1-bc8b-753e-886f-9b77c0ccb376
557c31d to
2b6e023
Compare
| def _bind_interpreter(interpreter, tools, output_fields=None) -> None: | ||
| if callable(bind := getattr(interpreter, "bind", None)): | ||
| bind(tools=tools, output_fields=output_fields) |
There was a problem hiding this comment.
Unrelated bind methods are invoked as RLM bindings
CodeInterpreter does not define bind, so a caller-owned interpreter may validly use bind(connection) for another backend operation. _bind_interpreter() treats every callable attribute with that name as DSPy's binding API and invokes it with tools= and output_fields=. A protocol-compliant interpreter with an unrelated bind(connection) therefore raises TypeError before it executes any RLM code. Use an explicit compatible binding capability or fall back to updating the protocol-owned interpreter state when that capability is unavailable.
Artifacts
Reproduction source for caller-owned interpreter with unrelated bind method
- This exact authored script defines a structurally valid caller-owned interpreter with `bind(connection)` and drives the real RLM forward path, showing the failing condition.
RLM output before safe binding behavior
- The executed command output shows protocol validation succeeds but RLM setup raises the unexpected `tools` keyword TypeError before interpreter execution, confirming the defect.
In-process safe-binding control source
- This exact authored control script replaces only the test process binding helper with the protocol fallback and drives the same RLM flow, demonstrating the expected usable behavior without altering repository source.
RLM output with in-process safe binding control
- The executed control output shows successful RLM completion, one interpreter execution, restored caller tools, and no caller-owned shutdown, demonstrating that safe binding resolves the failure.
| original = (dict(interpreter.tools), getattr(interpreter, "output_fields", None)) | ||
| try: | ||
| self._inject_execution_context(interpreter, execution_tools) | ||
| yield interpreter | ||
| finally: | ||
| _bind_interpreter(interpreter, *original) |
There was a problem hiding this comment.
Concurrent caller-owned RLM calls race on tool bindings
Caller-owned interpreters are snapshotted, rebound, executed, and restored without exclusive ownership of their mutable bindings. Overlapping RLM calls can therefore execute with another invocation's tools, lose their own active tools during restoration, and retain stale invocation tools afterward. Serialize the complete snapshot/bind/execute/restore lifecycle per interpreter, or explicitly reject concurrent use.
Artifacts
Deterministic RLM caller-owned interpreter lifecycle reproduction source
- Authored executable source that creates sequential and forced-overlap caller-owned interpreter scenarios, showing the lifecycle behavior under test.
Runtime output for caller-owned interpreter sequential and concurrent lifecycle reproduction
- Captured command output from running the authored reproduction with the project virtual environment, showing sequential success and the concurrent binding leak.
Focused RLM interpreter lifecycle test output
- Captured command output from the existing focused interpreter-lifecycle pytest class, confirming its lifecycle coverage passes while it does not exercise overlapping ownership.
Summary
PythonInterpreter.bind(tools=..., output_fields=...)to replace host bindings without resetting interpreter process or namespace statebindhook existsSUBMIT(output)when typed output fields are removedStack
Depends directly on #10136. Exact stacked base:
e3a4f4d5af850a92337743fbb9f988af8a43ff8c.Size
Production diff: exactly +30/-24 across four files. Tests: +37/-2. Documentation: +2/-2.
Deliberate scope
bindis runtime configuration, not a capability or transaction framework. It does not add concurrent binding, freeze callable metadata, audit runner globals, or delete stale guest wrapper symbols. Removed wrappers retain no host dispatch authority. The optional hook is intentionally duck-typed; compatibility for an unrelated third-party method also namedbindis not included.Validation
SUBMIT, caller-owned sync/async restoration, factory shutdown, and Flex paths coveredLMRequestsnapshot remain intact on the stacked basegit range-diffis exactly equivalent to the previously approved bind patchgit diff --checkare cleanIndependent adversarial review
557c31d6e51cca0f235e65ba823a7566d2b53026has sole parent/basee3a4f4d5af850a92337743fbb9f988af8a43ff8cLMRequestsnapshot is retained object-identically.Unknown tool; no removed host capability remains reachable. This is an accepted residual, not restack drift.Current CI
At the latest complete review snapshot: 20 checks passed, 1 expected skip, 0 failed, with Greptile still pending; GitHub reported mergeable. #10136's unrelated workflow-lint download failure must be rerun before the stack can merge.