Skip to content

Commit 306e75d

Browse files
committed
fix(config): per-agent parallel_tool_calls defaults; fix lint & skill regressions
The earlier refactor on this branch collapsed three agent builders' distinct historical parallel_tool_calls defaults onto one shared value, changing behavior for two of them: - lint agent: never sent parallel_tool_calls before; the refactor made it send `false` by default, re-triggering the Bedrock #175 malformed-tool_choice bug for `openkb lint` (LiteLLM's Bedrock transform breaks on ANY value). - skill-create agent: kept a hardcoded `parallel_tool_calls=True` that ignored config entirely, so `openkb skill new` stayed broken on Bedrock even with the `parallel_tool_calls: null` escape hatch set. Make resolution tri-state — resolve_parallel_tool_calls now returns (value, was_explicit): - key absent -> each agent applies its OWN historical default via resolve_model_settings(default_parallel_tool_calls=...): query/chat False, lint None (omit), skill-create True. - true/false/null -> applies uniformly to every agent (the escape hatch works regardless of which agent runs). - invalid value -> omit (the one value that never breaks a provider), instead of the old fall-back to False that could silently re-break Bedrock. parallel_tool_calls is dropped from DEFAULT_CONFIG so load_config's merge can't mask "not configured"; the runtime stash carries the was_explicit flag. Tool-less eval agents (skill/evaluator.py) deliberately stay unmigrated and always omit the setting — the SDK forwards an explicit `false` even with no tools, which strict OpenAI endpoints reject. Guarded by tests. Docs: the setting now scopes to all LLM agents (query/chat/lint/skill), documents the unset-per-agent-default semantics and the bare-`null` requirement. Adds regression tests across config/query/linter/skill-creator/skill-evaluator.
1 parent bbcd63a commit 306e75d

13 files changed

Lines changed: 317 additions & 135 deletions

config.yaml.example

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,14 @@ model: gpt-5.4 # LLM model (any LiteLLM-supported provider)
22
language: en # Wiki output language
33
pageindex_threshold: 20 # PDF pages threshold for PageIndex
44

5-
# Optional: whether query/chat agents may call tools in parallel.
6-
# false (default) force sequential tool calls
7-
# true allow parallel tool calls
8-
# null don't send the setting (use the provider default) — set this
9-
# for Amazon Bedrock Claude, which rejects the request when
10-
# parallel_tool_calls is sent at all (any value). See #175.
5+
# Optional: whether the LLM agents (query, chat, lint, skill) may call tools
6+
# in parallel. Leave it UNSET (commented out) to keep OpenKB's per-agent
7+
# defaults. Setting it applies the SAME value to every agent:
8+
# true allow parallel tool calls
9+
# false force sequential tool calls
10+
# null don't send the setting at all (use the provider default) — REQUIRED
11+
# for Amazon Bedrock Claude, which rejects the request when
12+
# parallel_tool_calls is sent at all (any value). See #175.
1113
# parallel_tool_calls: null
1214

1315
# Optional: override the entity-type vocabulary used for entity pages.

examples/configuration/README.md

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,14 @@ model: gpt-5.4 # LLM model (any LiteLLM-supported provider)
7070
language: en # Wiki output language
7171
pageindex_threshold: 20 # PDF pages threshold for PageIndex
7272

73-
# Optional: whether query/chat agents may call tools in parallel.
74-
# false (default) force sequential tool calls
75-
# true allow parallel tool calls
76-
# null don't send the setting (use the provider default) — set this
77-
# for Amazon Bedrock Claude, which rejects the request when
78-
# parallel_tool_calls is sent at all (any value). See #175.
73+
# Optional: whether the LLM agents (query, chat, lint, skill) may call tools
74+
# in parallel. Leave it UNSET (commented out) to keep OpenKB's per-agent
75+
# defaults. Setting it applies the SAME value to every agent:
76+
# true allow parallel tool calls
77+
# false force sequential tool calls
78+
# null don't send the setting at all (use the provider default) — REQUIRED
79+
# for Amazon Bedrock Claude, which rejects the request when
80+
# parallel_tool_calls is sent at all (any value). See #175.
7981
# parallel_tool_calls: null
8082

8183
# Optional: override the entity-type vocabulary used for entity pages.
@@ -103,7 +105,7 @@ pageindex_threshold: 20 # PDF pages threshold for PageIndex
103105
| `model` | `gpt-5.4` | LLM used for all compile/query/chat work. |
104106
| `language` | `en` | Language the wiki is written in. |
105107
| `pageindex_threshold` | `20` | PDFs with this many pages **or more** take the long-doc (PageIndex) path; shorter ones go through the short-doc path. See [`pageindex-cloud/`](../pageindex-cloud/). |
106-
| `parallel_tool_calls` | `false` | Whether query/chat agents may call tools in parallel. `false` (default) forces sequential; `true` allows parallel; `null` omits the setting (provider default). **Amazon Bedrock needs `null`** (see below). |
108+
| `parallel_tool_calls` | unset | Whether the LLM agents (query, chat, lint, skill) may call tools in parallel. Unset keeps OpenKB's per-agent defaults; `true`/`false` force allow/sequential for every agent; `null` omits the setting (provider default). **Amazon Bedrock needs `null`** (see below). |
107109
| `entity_types` | 7 defaults | Custom vocabulary for entity pages. `other` is always kept. |
108110
| `litellm:` || A pass-through block for LiteLLM. See below. |
109111

@@ -200,10 +202,11 @@ LLM_API_KEY=your-key-here
200202
```yaml
201203
# <kb>/.openkb/config.yaml
202204
model: bedrock/eu.anthropic.claude-sonnet-4-6
203-
parallel_tool_calls: null # REQUIRED for Bedrock Claude: the default (false)
204-
# — and any explicit value — makes LiteLLM send a
205-
# malformed tool_choice that Bedrock rejects (#175).
206-
# null tells OpenKB not to send the setting at all.
205+
parallel_tool_calls: null # REQUIRED for Bedrock Claude: sending
206+
# parallel_tool_calls at all (any value) makes
207+
# LiteLLM send a malformed tool_choice that Bedrock
208+
# rejects (#175). null tells OpenKB to omit it.
209+
# Write it as bare `null` — not `None` or "null".
207210
```
208211

209212
**Where keys are read from** (first match wins, existing env always respected):

openkb/agent/linter.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ def read_file(path: str) -> str:
8282
instructions=instructions,
8383
tools=[list_files, read_file],
8484
model=f"litellm/{model}",
85-
model_settings=ModelSettings(**resolve_model_settings()),
85+
model_settings=ModelSettings(**resolve_model_settings(default_parallel_tool_calls=None)),
8686
)
8787

8888

openkb/cli.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,8 @@ def _setup_llm_key(kb_dir: Path | None = None) -> None:
176176
provider: str | None = None
177177
extra_headers: dict[str, str] = {}
178178
timeout: float | None = None
179-
parallel_tool_calls: bool | None = DEFAULT_CONFIG["parallel_tool_calls"]
179+
parallel_tool_calls: bool | None = None
180+
parallel_tool_calls_explicit = False
180181
litellm_settings: dict = {}
181182
if kb_dir is not None:
182183
config_path = kb_dir / ".openkb" / "config.yaml"
@@ -186,7 +187,7 @@ def _setup_llm_key(kb_dir: Path | None = None) -> None:
186187
provider = _extract_provider(str(model))
187188
extra_headers = resolve_extra_headers(config)
188189
timeout = resolve_timeout(config)
189-
parallel_tool_calls = resolve_parallel_tool_calls(config)
190+
parallel_tool_calls, parallel_tool_calls_explicit = resolve_parallel_tool_calls(config)
190191
litellm_settings = resolve_litellm_settings(config)
191192
# `timeout` / `extra_headers` in the block route to the per-call
192193
# stashes (replacing the legacy top-level keys); the rest are globals.
@@ -198,7 +199,7 @@ def _setup_llm_key(kb_dir: Path | None = None) -> None:
198199
timeout = resolve_timeout({"timeout": litellm_settings.pop("timeout")})
199200
set_extra_headers(extra_headers)
200201
set_timeout(timeout)
201-
set_parallel_tool_calls(parallel_tool_calls)
202+
set_parallel_tool_calls(parallel_tool_calls, parallel_tool_calls_explicit)
202203
_apply_litellm_settings(litellm_settings)
203204

204205
if not api_key:

openkb/config.py

Lines changed: 38 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,6 @@
1717
"model": "gpt-5.4",
1818
"language": "en",
1919
"pageindex_threshold": 20,
20-
# Whether query/chat agents may call tools in parallel. Default false =
21-
# force sequential tool calls (historical behavior). true = allow parallel.
22-
# null = don't send the setting at all (use the provider default) — the
23-
# escape hatch for Amazon Bedrock Claude, where sending parallel_tool_calls
24-
# (any value) makes LiteLLM emit a malformed tool_choice missing `type`, so
25-
# every query/chat fails (issue #175).
26-
"parallel_tool_calls": False,
2720
}
2821

2922
# Default entity-type vocabulary. Overridable per-KB via the optional
@@ -151,36 +144,28 @@ def resolve_extra_headers(config: dict) -> dict[str, str]:
151144
return headers
152145

153146

154-
def resolve_parallel_tool_calls(config: dict) -> bool | None:
155-
"""Resolve the optional ``parallel_tool_calls:`` key.
147+
def resolve_parallel_tool_calls(config: dict) -> tuple[bool | None, bool]:
148+
"""Resolve the optional ``parallel_tool_calls:`` key to ``(value, was_explicit)``.
156149
157-
Tri-state:
158-
* key absent → the default (``False`` — force sequential tool calls).
159-
* ``true`` / ``false`` → that bool.
160-
* explicit ``null`` → ``None``, meaning "don't send the setting" so the
161-
provider's own default applies. This is the escape hatch for Amazon
162-
Bedrock Claude, which rejects the request when the param is sent at all.
163-
164-
A non-bool, non-null value is invalid → falls back to the default with a
165-
warning. An explicit ``null`` is a valid choice and warns silently.
166-
167-
Note this relies on the config being merged with ``DEFAULT_CONFIG`` (as
168-
``load_config`` does), so an omitted key reads back as ``False`` while an
169-
explicit ``null`` reads back as ``None`` — the two are distinguishable.
150+
Absent → ``(None, False)`` so each agent applies its own default (see
151+
``resolve_model_settings``). ``true``/``false`` → that bool; explicit
152+
``null`` → ``None`` (omit — the Amazon Bedrock #175 escape hatch); both with
153+
``was_explicit=True`` so they override every agent uniformly. An invalid
154+
value degrades to omit (never breaks a provider), with a warning.
170155
"""
171-
default = DEFAULT_CONFIG["parallel_tool_calls"]
172-
value = config.get("parallel_tool_calls", default)
156+
if "parallel_tool_calls" not in config:
157+
return None, False
158+
value = config["parallel_tool_calls"]
173159
if value is None:
174-
return None
175-
if not isinstance(value, bool):
176-
logger.warning(
177-
"config: 'parallel_tool_calls' must be true, false, or null, got %r "
178-
"— using default (%r).",
179-
value,
180-
default,
181-
)
182-
return default
183-
return value
160+
return None, True
161+
if isinstance(value, bool):
162+
return value, True
163+
logger.warning(
164+
"config: 'parallel_tool_calls' must be true, false, or null, got %r "
165+
"— omitting the setting.",
166+
value,
167+
)
168+
return None, True
184169

185170

186171
def resolve_timeout(config: dict) -> float | None:
@@ -282,38 +267,39 @@ def get_timeout_extra_args() -> dict[str, float] | None:
282267
return {"timeout": _runtime_timeout} if _runtime_timeout is not None else None
283268

284269

285-
# Process-wide agent ``parallel_tool_calls`` setting, resolved from config by
286-
# the CLI (cli._setup_llm_key) and read when building query/chat agents. None =
287-
# omit the setting (provider default) — see the DEFAULT_CONFIG note on why
288-
# Bedrock needs this.
289-
_runtime_parallel_tool_calls: bool | None = None
270+
# Process-wide agent ``parallel_tool_calls`` as ``(value, was_explicit)``, set
271+
# from config by the CLI and read when building agents. ``(None, False)`` = not
272+
# configured, so each agent falls back to its own default (resolve_model_settings).
273+
_runtime_parallel_tool_calls: tuple[bool | None, bool] = (None, False)
290274

291275

292-
def set_parallel_tool_calls(value: bool | None) -> None:
293-
"""Set the process-wide agent ``parallel_tool_calls`` setting."""
276+
def set_parallel_tool_calls(value: bool | None, was_explicit: bool) -> None:
277+
"""Set the process-wide ``parallel_tool_calls`` — see :func:`resolve_parallel_tool_calls`."""
294278
global _runtime_parallel_tool_calls
295-
_runtime_parallel_tool_calls = value
279+
_runtime_parallel_tool_calls = (value, was_explicit)
296280

297281

298-
def get_parallel_tool_calls() -> bool | None:
299-
"""Return the process-wide agent ``parallel_tool_calls`` setting (or None)."""
282+
def get_parallel_tool_calls() -> tuple[bool | None, bool]:
283+
"""Return the process-wide ``parallel_tool_calls`` as ``(value, was_explicit)``."""
300284
return _runtime_parallel_tool_calls
301285

302286

303-
def resolve_model_settings() -> dict[str, Any]:
287+
def resolve_model_settings(*, default_parallel_tool_calls: bool | None = False) -> dict[str, Any]:
304288
"""Assemble the agents-SDK ``ModelSettings`` kwargs from the process-wide LLM
305-
runtime settings (populated by ``cli._setup_llm_key``).
289+
runtime settings — the single place tool-using agent builders wire them in.
306290
307-
This is the single place that maps runtime LLM config onto agent model
308-
settings: every agent builder does ``ModelSettings(**resolve_model_settings())``
309-
rather than enumerating the individual getters, so a new agent-facing knob
310-
is wired in here once and can't be silently forgotten by one builder.
311-
``None`` values are what the agents SDK treats as "unset / provider default".
291+
``default_parallel_tool_calls`` (the caller's own historical default) is used
292+
only when config didn't set ``parallel_tool_calls``; an explicit value always
293+
wins. Tool-less agents (skill-eval graders) skip this and omit the setting —
294+
the SDK forwards an explicit ``False`` even without tools, which strict
295+
OpenAI-compatible endpoints reject.
312296
"""
297+
value, was_explicit = get_parallel_tool_calls()
298+
parallel_tool_calls = value if was_explicit else default_parallel_tool_calls
313299
return {
314300
"extra_headers": get_extra_headers() or None,
315301
"extra_args": get_timeout_extra_args(),
316-
"parallel_tool_calls": get_parallel_tool_calls(),
302+
"parallel_tool_calls": parallel_tool_calls,
317303
}
318304

319305

openkb/skill/creator.py

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
from agents import Agent, Runner, ToolOutputImage, ToolOutputText, function_tool
2121
from agents.model_settings import ModelSettings
2222

23-
from openkb.config import get_extra_headers, get_timeout_extra_args
23+
from openkb.config import resolve_model_settings
2424
from openkb.prompts import load_prompt
2525
from openkb.schema import get_agents_md
2626
from openkb.skill import skill_dir
@@ -154,18 +154,14 @@ def done(summary: str) -> str:
154154
done,
155155
],
156156
model=f"litellm/{model}",
157-
# Allow the model to issue multiple read tool calls in one turn —
158-
# the compile's early phase is a fan-out (list dir -> read N
159-
# summaries -> read N source page-ranges), and serialising each
160-
# read into its own turn costs roughly 5-10 extra round-trips per
161-
# compile. Writes serialise naturally because each
162-
# `write_skill_file` depends on accumulated reads; the model has
163-
# no reason to issue parallel writes to the same path.
164-
model_settings=ModelSettings(
165-
parallel_tool_calls=True,
166-
extra_headers=get_extra_headers() or None,
167-
extra_args=get_timeout_extra_args(),
168-
),
157+
# Default to allowing parallel read tool calls — the compile's early
158+
# phase is a fan-out (list dir -> read N summaries -> read N source
159+
# page-ranges), and serialising each read into its own turn costs
160+
# roughly 5-10 extra round-trips per compile. Writes serialise
161+
# naturally because each `write_skill_file` depends on accumulated
162+
# reads; the model has no reason to issue parallel writes to the
163+
# same path. An explicit config value (e.g. `null` for Bedrock) wins.
164+
model_settings=ModelSettings(**resolve_model_settings(default_parallel_tool_calls=True)),
169165
)
170166

171167

tests/conftest.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ def _reset_extra_headers():
1212
yield
1313
set_extra_headers({})
1414
set_timeout(None)
15-
set_parallel_tool_calls(None)
15+
set_parallel_tool_calls(None, False)
1616

1717

1818
@pytest.fixture

0 commit comments

Comments
 (0)