Skip to content

Commit bd6216a

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 bd6216a

13 files changed

Lines changed: 380 additions & 131 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: 80 additions & 48 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,48 @@ def resolve_extra_headers(config: dict) -> dict[str, str]:
151144
return headers
152145

153146

154-
def resolve_parallel_tool_calls(config: dict) -> bool | None:
147+
def resolve_parallel_tool_calls(config: dict) -> tuple[bool | None, bool]:
155148
"""Resolve the optional ``parallel_tool_calls:`` key.
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+
Returns ``(value, was_explicit)``:
151+
* key absent → ``(None, False)`` — "not configured". Different agent
152+
builders had different HISTORICAL defaults before this setting existed
153+
(query/chat forced sequential; the semantic linter and skill-creation
154+
agents never forced a value at all), so there's no single systemwide
155+
default to fall back to here — ``resolve_model_settings()`` lets each
156+
caller supply its own, applied only when ``was_explicit`` is ``False``.
157+
* ``true`` / ``false`` → ``(that bool, True)`` — an explicit choice that
158+
overrides every agent's own default uniformly.
159+
* explicit ``null`` → ``(None, True)`` — "don't send the setting at all"
160+
(the provider decides). This is the escape hatch for Amazon Bedrock
161+
Claude, which rejects the request when the param is sent at all —
162+
distinct from "absent" even though both currently carry a value of
163+
``None``; ``was_explicit`` is what tells them apart.
164+
* anything else (invalid) → ``(None, True)``, with a warning. Omitting
165+
is the one value known to never break any provider, so a malformed
166+
config value degrades to the safest behavior rather than to a fixed
167+
bool that could reproduce the exact failure (e.g. Bedrock's) the user
168+
may have set this key specifically to avoid.
169+
170+
This function does not depend on ``config`` being pre-merged with
171+
``DEFAULT_CONFIG`` — ``parallel_tool_calls`` is intentionally absent from
172+
``DEFAULT_CONFIG`` so ``load_config``'s merge can't mask "the user's
173+
config.yaml doesn't mention this key" as if it had been set to a value.
170174
"""
171-
default = DEFAULT_CONFIG["parallel_tool_calls"]
172-
value = config.get("parallel_tool_calls", default)
175+
if "parallel_tool_calls" not in config:
176+
return None, False
177+
value = config["parallel_tool_calls"]
173178
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
179+
return None, True
180+
if isinstance(value, bool):
181+
return value, True
182+
logger.warning(
183+
"config: 'parallel_tool_calls' must be true, false, or null, got %r "
184+
"— omitting the setting (the safest fallback; it never breaks a "
185+
"provider, unlike a fixed true/false might).",
186+
value,
187+
)
188+
return None, True
184189

185190

186191
def resolve_timeout(config: dict) -> float | None:
@@ -283,37 +288,64 @@ def get_timeout_extra_args() -> dict[str, float] | None:
283288

284289

285290
# 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
291+
# the CLI (cli._setup_llm_key) and read when building agents. ``(None, False)``
292+
# — the module's own raw default — means "not configured", matching what
293+
# ``resolve_parallel_tool_calls({})`` returns for an absent key, so an agent
294+
# built before ``_setup_llm_key`` has ever run defers to its own default via
295+
# ``resolve_model_settings()`` instead of landing on an arbitrary fixed value.
296+
_runtime_parallel_tool_calls: tuple[bool | None, bool] = (None, False)
290297

291298

292-
def set_parallel_tool_calls(value: bool | None) -> None:
293-
"""Set the process-wide agent ``parallel_tool_calls`` setting."""
299+
def set_parallel_tool_calls(value: bool | None, was_explicit: bool) -> None:
300+
"""Set the process-wide agent ``parallel_tool_calls`` setting.
301+
302+
``was_explicit`` distinguishes "the KB's config.yaml has no
303+
``parallel_tool_calls`` key" (``False``) from "the user explicitly chose
304+
this value, including an explicit ``null``" (``True``) — see
305+
:func:`resolve_parallel_tool_calls`.
306+
"""
294307
global _runtime_parallel_tool_calls
295-
_runtime_parallel_tool_calls = value
308+
_runtime_parallel_tool_calls = (value, was_explicit)
296309

297310

298-
def get_parallel_tool_calls() -> bool | None:
299-
"""Return the process-wide agent ``parallel_tool_calls`` setting (or None)."""
311+
def get_parallel_tool_calls() -> tuple[bool | None, bool]:
312+
"""Return the process-wide agent ``parallel_tool_calls`` setting as
313+
``(value, was_explicit)`` — see :func:`resolve_parallel_tool_calls`."""
300314
return _runtime_parallel_tool_calls
301315

302316

303-
def resolve_model_settings() -> dict[str, Any]:
317+
def resolve_model_settings(*, default_parallel_tool_calls: bool | None = False) -> dict[str, Any]:
304318
"""Assemble the agents-SDK ``ModelSettings`` kwargs from the process-wide LLM
305319
runtime settings (populated by ``cli._setup_llm_key``).
306320
307321
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".
322+
settings: TOOL-USING agent builders (query, chat, lint, skill-create) do
323+
``ModelSettings(**resolve_model_settings(...))`` rather than enumerating
324+
the individual getters, so a new agent-facing knob is wired in here once
325+
and can't be silently forgotten by one of them. ``None`` values are what
326+
the agents SDK treats as "unset / provider default".
327+
328+
Tool-LESS agents (e.g. the skill-eval graders) deliberately do NOT use
329+
this — ``parallel_tool_calls`` is meaningless without tools, and the
330+
agents SDK forwards an explicit ``False`` even to a tool-less request,
331+
which strict OpenAI-compatible endpoints reject. They build
332+
``ModelSettings`` directly and omit ``parallel_tool_calls`` entirely.
333+
334+
``default_parallel_tool_calls`` is applied ONLY when the KB's config.yaml
335+
doesn't configure ``parallel_tool_calls`` at all — pass your agent
336+
builder's own historical default here (``query``/``chat`` force
337+
sequential → ``False``, the module default; other builders pass their
338+
own). An explicit config value (``true``/``false``/``null``) always
339+
overrides every builder's default uniformly — that's what lets Amazon
340+
Bedrock users escape issue #175 with one setting, regardless of which
341+
agent they're running.
312342
"""
343+
value, was_explicit = get_parallel_tool_calls()
344+
parallel_tool_calls = value if was_explicit else default_parallel_tool_calls
313345
return {
314346
"extra_headers": get_extra_headers() or None,
315347
"extra_args": get_timeout_extra_args(),
316-
"parallel_tool_calls": get_parallel_tool_calls(),
348+
"parallel_tool_calls": parallel_tool_calls,
317349
}
318350

319351

openkb/skill/creator.py

Lines changed: 10 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,15 @@ 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 `parallel_tool_calls:` in config.yaml (e.g.
164+
# `null` for Amazon Bedrock, issue #175) overrides this uniformly.
165+
model_settings=ModelSettings(**resolve_model_settings(default_parallel_tool_calls=True)),
169166
)
170167

171168

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)