Skip to content

Commit 4090723

Browse files
committed
move max_concurrent_subcalls as a general REPL property. add more tests
1 parent 4dcd1de commit 4090723

4 files changed

Lines changed: 177 additions & 8 deletions

File tree

‎rlm/core/rlm.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,8 +242,7 @@ def _spawn_completion_context(self, prompt: str | dict[str, Any]):
242242
env_kwargs["custom_sub_tools"] = self.custom_sub_tools
243243
if self.compaction and self.environment_type == "local":
244244
env_kwargs["compaction"] = True
245-
if self.environment_type == "local":
246-
env_kwargs["max_concurrent_subcalls"] = self.max_concurrent_subcalls
245+
env_kwargs["max_concurrent_subcalls"] = self.max_concurrent_subcalls
247246
environment: BaseEnv = get_environment(self.environment_type, env_kwargs)
248247

249248
if self.persistent:

‎rlm/environments/base_env.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,9 +212,12 @@ class BaseEnv(ABC):
212212
}
213213
"""
214214

215-
def __init__(self, persistent: bool = False, depth: int = 1, **kwargs):
215+
def __init__(
216+
self, persistent: bool = False, depth: int = 1, max_concurrent_subcalls: int = 4, **kwargs
217+
):
216218
self.persistent = persistent
217219
self.depth = depth
220+
self.max_concurrent_subcalls = max_concurrent_subcalls
218221
self.kwargs = kwargs
219222

220223
@abstractmethod

‎rlm/environments/local_repl.py‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,11 +139,15 @@ def __init__(
139139
max_concurrent_subcalls: int = 4,
140140
**kwargs,
141141
):
142-
super().__init__(persistent=persistent, depth=depth, **kwargs)
142+
super().__init__(
143+
persistent=persistent,
144+
depth=depth,
145+
max_concurrent_subcalls=max_concurrent_subcalls,
146+
**kwargs,
147+
)
143148

144149
self.lm_handler_address = lm_handler_address
145150
self.subcall_fn = subcall_fn # Callback for recursive RLM calls (depth > 1 support)
146-
self.max_concurrent_subcalls = max_concurrent_subcalls
147151
self.original_cwd = os.getcwd()
148152
self.temp_dir = tempfile.mkdtemp(prefix=f"repl_env_{uuid.uuid4()}_")
149153
self._lock = threading.Lock()
@@ -366,8 +370,7 @@ def _run_subcall(index: int, prompt: str) -> None:
366370

367371
with ThreadPoolExecutor(max_workers=max_workers) as executor:
368372
futures = [
369-
executor.submit(_run_subcall, i, prompt)
370-
for i, prompt in enumerate(prompts)
373+
executor.submit(_run_subcall, i, prompt) for i, prompt in enumerate(prompts)
371374
]
372375
# Wait for all futures to complete; exceptions are captured inside _run_subcall
373376
for future in as_completed(futures):

‎tests/test_rlm_query.py‎

Lines changed: 165 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,7 @@ def sometimes_fail(prompt, model):
281281

282282
def test_batched_pending_calls_ordered(self):
283283
"""Pending LLM calls should be appended in prompt order for deterministic metadata."""
284+
284285
def delayed_subcall(prompt, model):
285286
# Reverse delay so later prompts finish first
286287
delay = 0.1 if prompt == "first" else 0.01
@@ -299,12 +300,175 @@ def test_single_prompt_skips_thread_pool(self):
299300
"""Single prompt should not use ThreadPoolExecutor (no overhead)."""
300301
subcall_fn = MagicMock(return_value=_make_completion("solo"))
301302
repl = LocalREPL(subcall_fn=subcall_fn, max_concurrent_subcalls=4)
302-
result = repl.execute_code("answers = rlm_query_batched(['only'])")
303+
repl.execute_code("answers = rlm_query_batched(['only'])")
303304
assert repl.locals["answers"] == ["solo"]
304305
subcall_fn.assert_called_once_with("only", None)
305306
repl.cleanup()
306307

307308

309+
class TestMaxConcurrentSubcallsBounds:
310+
"""Tests for edge cases and boundary conditions of max_concurrent_subcalls."""
311+
312+
def test_max_concurrent_subcalls_default_value(self):
313+
"""Default max_concurrent_subcalls should be 4."""
314+
repl = LocalREPL()
315+
assert repl.max_concurrent_subcalls == 4
316+
repl.cleanup()
317+
318+
def test_max_concurrent_subcalls_custom_value(self):
319+
"""Should accept custom max_concurrent_subcalls values."""
320+
repl = LocalREPL(max_concurrent_subcalls=8)
321+
assert repl.max_concurrent_subcalls == 8
322+
repl.cleanup()
323+
324+
def test_max_concurrent_subcalls_one(self):
325+
"""max_concurrent_subcalls=1 should force sequential execution."""
326+
order = []
327+
lock = threading.Lock()
328+
329+
def sequential_subcall(prompt, model):
330+
with lock:
331+
order.append(f"start-{prompt}")
332+
time.sleep(0.05)
333+
with lock:
334+
order.append(f"end-{prompt}")
335+
return _make_completion(f"done-{prompt}")
336+
337+
repl = LocalREPL(subcall_fn=sequential_subcall, max_concurrent_subcalls=1)
338+
result = repl.execute_code("answers = rlm_query_batched(['a', 'b', 'c'])")
339+
assert result.stderr == ""
340+
answers = repl.locals["answers"]
341+
assert answers == ["done-a", "done-b", "done-c"]
342+
# With max_workers=1, tasks run one at a time so each start
343+
# should be followed by its end before the next start
344+
for i in range(0, len(order) - 1, 2):
345+
prompt_id = order[i].split("-", 1)[1]
346+
assert order[i] == f"start-{prompt_id}"
347+
assert order[i + 1] == f"end-{prompt_id}"
348+
repl.cleanup()
349+
350+
def test_max_concurrent_subcalls_larger_than_prompts(self):
351+
"""max_concurrent_subcalls larger than prompt count should work fine."""
352+
subcall_fn = MagicMock(return_value=_make_completion("ok"))
353+
repl = LocalREPL(subcall_fn=subcall_fn, max_concurrent_subcalls=100)
354+
result = repl.execute_code("answers = rlm_query_batched(['a', 'b'])")
355+
assert result.stderr == ""
356+
assert repl.locals["answers"] == ["ok", "ok"]
357+
assert subcall_fn.call_count == 2
358+
repl.cleanup()
359+
360+
def test_max_concurrent_subcalls_exact_match(self):
361+
"""max_concurrent_subcalls == prompt count should allow full parallelism."""
362+
active_count = []
363+
active_lock = threading.Lock()
364+
active = [0]
365+
366+
def tracked_subcall(prompt, model):
367+
with active_lock:
368+
active[0] += 1
369+
active_count.append(active[0])
370+
time.sleep(0.1)
371+
with active_lock:
372+
active[0] -= 1
373+
return _make_completion(f"ok {prompt}")
374+
375+
repl = LocalREPL(subcall_fn=tracked_subcall, max_concurrent_subcalls=3)
376+
result = repl.execute_code("answers = rlm_query_batched(['a', 'b', 'c'])")
377+
assert result.stderr == ""
378+
# All 3 should be able to run at once
379+
assert max(active_count) <= 3
380+
repl.cleanup()
381+
382+
def test_batched_all_failures_parallel(self):
383+
"""All subcalls failing in parallel should return all error strings."""
384+
385+
def always_fail(prompt, model):
386+
raise ValueError(f"fail-{prompt}")
387+
388+
repl = LocalREPL(subcall_fn=always_fail, max_concurrent_subcalls=4)
389+
result = repl.execute_code("answers = rlm_query_batched(['x', 'y', 'z'])")
390+
assert result.stderr == ""
391+
answers = repl.locals["answers"]
392+
assert len(answers) == 3
393+
for i, prompt in enumerate(["x", "y", "z"]):
394+
assert "Error" in answers[i]
395+
assert f"fail-{prompt}" in answers[i]
396+
repl.cleanup()
397+
398+
def test_batched_large_batch_with_low_concurrency(self):
399+
"""Many prompts with low concurrency should still complete correctly."""
400+
max_concurrent = 2
401+
active_count = []
402+
active_lock = threading.Lock()
403+
active = [0]
404+
405+
def tracked_subcall(prompt, model):
406+
with active_lock:
407+
active[0] += 1
408+
active_count.append(active[0])
409+
time.sleep(0.02)
410+
with active_lock:
411+
active[0] -= 1
412+
return _make_completion(f"r-{prompt}")
413+
414+
prompts = [f"p{i}" for i in range(10)]
415+
repl = LocalREPL(subcall_fn=tracked_subcall, max_concurrent_subcalls=max_concurrent)
416+
result = repl.execute_code(f"answers = rlm_query_batched({prompts!r})")
417+
assert result.stderr == ""
418+
answers = repl.locals["answers"]
419+
assert len(answers) == 10
420+
assert answers == [f"r-p{i}" for i in range(10)]
421+
# Concurrency cap should be respected
422+
assert max(active_count) <= max_concurrent
423+
repl.cleanup()
424+
425+
def test_batched_empty_prompts_with_parallel(self):
426+
"""Empty prompt list should return empty list regardless of concurrency setting."""
427+
subcall_fn = MagicMock()
428+
repl = LocalREPL(subcall_fn=subcall_fn, max_concurrent_subcalls=4)
429+
repl.execute_code("answers = rlm_query_batched([])")
430+
assert repl.locals["answers"] == []
431+
subcall_fn.assert_not_called()
432+
repl.cleanup()
433+
434+
def test_pending_calls_exclude_failures(self):
435+
"""Failed subcalls should not appear in pending_llm_calls."""
436+
437+
def fail_second(prompt, model):
438+
if prompt == "bad":
439+
raise RuntimeError("boom")
440+
return _make_completion(f"ok-{prompt}")
441+
442+
repl = LocalREPL(subcall_fn=fail_second, max_concurrent_subcalls=4)
443+
result = repl.execute_code("answers = rlm_query_batched(['good1', 'bad', 'good2'])")
444+
assert result.stderr == ""
445+
# Only 2 successful completions should be in rlm_calls
446+
assert len(result.rlm_calls) == 2
447+
assert result.rlm_calls[0].response == "ok-good1"
448+
assert result.rlm_calls[1].response == "ok-good2"
449+
repl.cleanup()
450+
451+
452+
class TestMaxConcurrentSubcallsOnBaseEnv:
453+
"""Tests that max_concurrent_subcalls is a property of BaseEnv, not just LocalREPL."""
454+
455+
def test_base_env_has_max_concurrent_subcalls(self):
456+
"""BaseEnv should accept and store max_concurrent_subcalls."""
457+
458+
# BaseEnv is abstract, so we test via LocalREPL which inherits from it
459+
repl = LocalREPL(max_concurrent_subcalls=16)
460+
assert repl.max_concurrent_subcalls == 16
461+
# Verify it's set via the BaseEnv.__init__ chain
462+
assert hasattr(repl, "max_concurrent_subcalls")
463+
repl.cleanup()
464+
465+
def test_base_env_default_max_concurrent_subcalls(self):
466+
"""BaseEnv default should be 4."""
467+
repl = LocalREPL()
468+
assert repl.max_concurrent_subcalls == 4
469+
repl.cleanup()
470+
471+
308472
class TestRlmQueryScaffoldRestoration:
309473
"""Test that rlm_query and rlm_query_batched are restored after overwrite."""
310474

0 commit comments

Comments
 (0)