jaseci-labs/jac

refactor(byllm): reduce bloat in parallel tool-calling PR #5590

Aperta

#5711 aperta il 27 apr 2026

 (2 commenti) (0 reazioni) (1 assegnatario)Jac (378 fork)auto 404
deslopgood first issue

Metriche repository

Star
 (555 stelle)
Metriche merge PR
 (Merge medio 1g 12h) (358 PR mergiate in 30 g)

Descrizione

PR #5590 introduced parallel tool calling for byllm. The core dispatch engine is well-designed, but the implementation has several areas of unnecessary complexity that should be cleaned up:

1. Duplicated dispatch logic in invoke vs streaming paths

The finish-tool split → batch → dispatch_batch → reorder pattern is repeated nearly identically in both BaseLLM.invoke and BaseLLM._invoke_streaming (~90 lines duplicated). Should be extracted into a shared helper.

2. Over-engineered LLM prompt injection (~50 lines in make_model_params)

The "intelligent scheduling" system mutates every tool description with [PARALLEL-SAFE]/[SEQUENTIAL] tags AND injects multi-line scheduling rules into the system prompt. LLMs already understand how to batch independent tool calls — this prompt stuffing burns tokens on every request for marginal benefit. Consider removing or making it opt-in only (currently opt-out via parallel_hint=False).

3. Thread-in-thread timeout in _call_sync

_call_sync spawns a threading.Thread for timeout handling, but it's already running inside a ThreadPoolExecutor worker. concurrent.futures supports future.result(timeout=...) natively — the manual thread + result_box/error_box pattern adds unnecessary complexity.

4. asyncio.run() per async tool call

Each async tool creates a fresh event loop via asyncio.run() inside the thread pool. Multiple concurrent async tools could share a loop instead.

5. Timing metrics should be a separate concern

llm_timing and tool_timing StreamEvents were added inline in the streaming loop. These are useful but orthogonal to parallel dispatch — should be split into a separate PR or at minimum not interleaved with the dispatch refactor.

6. Brittle source-grep tests

Several of the 777 lines of tests are string searches on source code (e.g., verifying dispatch_batch appears in basellm source). These test code existence rather than behavior and will break on any refactor.

Suggested action

~30-40% of the added lines could be cut or deferred without losing the parallel dispatch feature. The core parallel.jac module and config plumbing are solid — the cleanup is mostly in the integration points.

Ref: #5590

Guida contributor