diff --git a/config.yaml.example b/config.yaml.example index eebf3bf6..ac4f6396 100644 --- a/config.yaml.example +++ b/config.yaml.example @@ -2,6 +2,16 @@ model: gpt-5.4 # LLM model (any LiteLLM-supported provider) language: en # Wiki output language pageindex_threshold: 20 # PDF pages threshold for PageIndex +# Optional: whether the LLM agents (query, chat, lint, skill) may call tools +# in parallel. Leave it UNSET (commented out) to keep OpenKB's per-agent +# defaults. Setting it applies the SAME value to every agent: +# true allow parallel tool calls +# false force sequential tool calls +# null don't send the setting at all (use the provider default) — REQUIRED +# for Amazon Bedrock Claude, which rejects the request when +# parallel_tool_calls is sent at all (any value). See #175. +# parallel_tool_calls: null + # Optional: override the entity-type vocabulary used for entity pages. # Omit this key to use the default 7 types # (person, organization, place, product, work, event, other). diff --git a/examples/configuration/README.md b/examples/configuration/README.md index 58cb7d4d..cbf4c2a6 100644 --- a/examples/configuration/README.md +++ b/examples/configuration/README.md @@ -70,6 +70,16 @@ model: gpt-5.4 # LLM model (any LiteLLM-supported provider) language: en # Wiki output language pageindex_threshold: 20 # PDF pages threshold for PageIndex +# Optional: whether the LLM agents (query, chat, lint, skill) may call tools +# in parallel. Leave it UNSET (commented out) to keep OpenKB's per-agent +# defaults. Setting it applies the SAME value to every agent: +# true allow parallel tool calls +# false force sequential tool calls +# null don't send the setting at all (use the provider default) — REQUIRED +# for Amazon Bedrock Claude, which rejects the request when +# parallel_tool_calls is sent at all (any value). See #175. +# parallel_tool_calls: null + # Optional: override the entity-type vocabulary used for entity pages. # Omit this key to use the default 7 types # (person, organization, place, product, work, event, other). @@ -95,6 +105,7 @@ pageindex_threshold: 20 # PDF pages threshold for PageIndex | `model` | `gpt-5.4` | LLM used for all compile/query/chat work. | | `language` | `en` | Language the wiki is written in. | | `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/). | +| `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). | | `entity_types` | 7 defaults | Custom vocabulary for entity pages. `other` is always kept. | | `litellm:` | – | A pass-through block for LiteLLM. See below. | @@ -177,6 +188,26 @@ LLM_API_KEY=your-key-here won't warn about a missing one. - **PageIndex Cloud** uses a separate `PAGEINDEX_API_KEY` (see [`pageindex-cloud/`](../pageindex-cloud/)). +- **Amazon Bedrock** (`model: bedrock/...`) authenticates with AWS credentials, + not `LLM_API_KEY`. Put them in `/.env` (LiteLLM/boto3 read them from the + environment); `LLM_API_KEY` isn't needed: + + ```bash + # /.env + AWS_ACCESS_KEY_ID=... + AWS_SECRET_ACCESS_KEY=... + AWS_REGION_NAME=eu-central-1 + ``` + + ```yaml + # /.openkb/config.yaml + model: bedrock/eu.anthropic.claude-sonnet-4-6 + parallel_tool_calls: null # REQUIRED for Bedrock Claude: sending + # parallel_tool_calls at all (any value) makes + # LiteLLM send a malformed tool_choice that Bedrock + # rejects (#175). null tells OpenKB to omit it. + # Write it as bare `null` — not `None` or "null". + ``` **Where keys are read from** (first match wins, existing env always respected): diff --git a/openkb/agent/linter.py b/openkb/agent/linter.py index 365f5602..407e0756 100644 --- a/openkb/agent/linter.py +++ b/openkb/agent/linter.py @@ -8,7 +8,7 @@ from agents.model_settings import ModelSettings from openkb.agent.tools import list_wiki_files, read_wiki_file -from openkb.config import get_extra_headers, get_timeout_extra_args +from openkb.config import resolve_model_settings from openkb.schema import get_agents_md MAX_TURNS = 50 @@ -82,10 +82,7 @@ def read_file(path: str) -> str: instructions=instructions, tools=[list_files, read_file], model=f"litellm/{model}", - model_settings=ModelSettings( - extra_headers=get_extra_headers() or None, - extra_args=get_timeout_extra_args(), - ), + model_settings=ModelSettings(**resolve_model_settings(default_parallel_tool_calls=None)), ) diff --git a/openkb/agent/query.py b/openkb/agent/query.py index 20414cf7..885ec235 100644 --- a/openkb/agent/query.py +++ b/openkb/agent/query.py @@ -12,7 +12,7 @@ read_wiki_image, write_kb_file, ) -from openkb.config import get_extra_headers, get_timeout_extra_args +from openkb.config import resolve_model_settings from openkb.schema import get_agents_md MAX_TURNS = 50 @@ -95,11 +95,7 @@ def get_image(image_path: str) -> ToolOutputImage | ToolOutputText: instructions=instructions, tools=[read_file, get_page_content, get_image], model=f"litellm/{model}", - model_settings=ModelSettings( - parallel_tool_calls=False, - extra_headers=get_extra_headers() or None, - extra_args=get_timeout_extra_args(), - ), + model_settings=ModelSettings(**resolve_model_settings()), ) diff --git a/openkb/cli.py b/openkb/cli.py index f39d0b44..373ca0df 100644 --- a/openkb/cli.py +++ b/openkb/cli.py @@ -56,6 +56,8 @@ def filter(self, record: logging.LogRecord) -> bool: register_kb, resolve_extra_headers, set_extra_headers, + resolve_parallel_tool_calls, + set_parallel_tool_calls, resolve_timeout, set_timeout, resolve_litellm_settings, @@ -174,6 +176,8 @@ def _setup_llm_key(kb_dir: Path | None = None) -> None: provider: str | None = None extra_headers: dict[str, str] = {} timeout: float | None = None + parallel_tool_calls: bool | None = None + parallel_tool_calls_explicit = False litellm_settings: dict = {} if kb_dir is not None: config_path = kb_dir / ".openkb" / "config.yaml" @@ -183,6 +187,7 @@ def _setup_llm_key(kb_dir: Path | None = None) -> None: provider = _extract_provider(str(model)) extra_headers = resolve_extra_headers(config) timeout = resolve_timeout(config) + parallel_tool_calls, parallel_tool_calls_explicit = resolve_parallel_tool_calls(config) litellm_settings = resolve_litellm_settings(config) # `timeout` / `extra_headers` in the block route to the per-call # stashes (replacing the legacy top-level keys); the rest are globals. @@ -194,6 +199,7 @@ def _setup_llm_key(kb_dir: Path | None = None) -> None: timeout = resolve_timeout({"timeout": litellm_settings.pop("timeout")}) set_extra_headers(extra_headers) set_timeout(timeout) + set_parallel_tool_calls(parallel_tool_calls, parallel_tool_calls_explicit) _apply_litellm_settings(litellm_settings) if not api_key: diff --git a/openkb/config.py b/openkb/config.py index a5ee010b..3291bb29 100644 --- a/openkb/config.py +++ b/openkb/config.py @@ -144,6 +144,30 @@ def resolve_extra_headers(config: dict) -> dict[str, str]: return headers +def resolve_parallel_tool_calls(config: dict) -> tuple[bool | None, bool]: + """Resolve the optional ``parallel_tool_calls:`` key to ``(value, was_explicit)``. + + Absent → ``(None, False)`` so each agent applies its own default (see + ``resolve_model_settings``). ``true``/``false`` → that bool; explicit + ``null`` → ``None`` (omit — the Amazon Bedrock #175 escape hatch); both with + ``was_explicit=True`` so they override every agent uniformly. An invalid + value degrades to omit (never breaks a provider), with a warning. + """ + if "parallel_tool_calls" not in config: + return None, False + value = config["parallel_tool_calls"] + if value is None: + return None, True + if isinstance(value, bool): + return value, True + logger.warning( + "config: 'parallel_tool_calls' must be true, false, or null, got %r " + "— omitting the setting.", + value, + ) + return None, True + + def resolve_timeout(config: dict) -> float | None: """Resolve the optional ``timeout:`` key to a finite positive number of seconds. @@ -243,6 +267,42 @@ def get_timeout_extra_args() -> dict[str, float] | None: return {"timeout": _runtime_timeout} if _runtime_timeout is not None else None +# Process-wide agent ``parallel_tool_calls`` as ``(value, was_explicit)``, set +# from config by the CLI and read when building agents. ``(None, False)`` = not +# configured, so each agent falls back to its own default (resolve_model_settings). +_runtime_parallel_tool_calls: tuple[bool | None, bool] = (None, False) + + +def set_parallel_tool_calls(value: bool | None, was_explicit: bool) -> None: + """Set the process-wide ``parallel_tool_calls`` — see :func:`resolve_parallel_tool_calls`.""" + global _runtime_parallel_tool_calls + _runtime_parallel_tool_calls = (value, was_explicit) + + +def get_parallel_tool_calls() -> tuple[bool | None, bool]: + """Return the process-wide ``parallel_tool_calls`` as ``(value, was_explicit)``.""" + return _runtime_parallel_tool_calls + + +def resolve_model_settings(*, default_parallel_tool_calls: bool | None = False) -> dict[str, Any]: + """Assemble the agents-SDK ``ModelSettings`` kwargs from the process-wide LLM + runtime settings — the single place tool-using agent builders wire them in. + + ``default_parallel_tool_calls`` (the caller's own historical default) is used + only when config didn't set ``parallel_tool_calls``; an explicit value always + wins. Tool-less agents (skill-eval graders) skip this and omit the setting — + the SDK forwards an explicit ``False`` even without tools, which strict + OpenAI-compatible endpoints reject. + """ + value, was_explicit = get_parallel_tool_calls() + parallel_tool_calls = value if was_explicit else default_parallel_tool_calls + return { + "extra_headers": get_extra_headers() or None, + "extra_args": get_timeout_extra_args(), + "parallel_tool_calls": parallel_tool_calls, + } + + def load_config(config_path: Path) -> dict[str, Any]: """Load YAML config from config_path, merged with DEFAULT_CONFIG. diff --git a/openkb/skill/creator.py b/openkb/skill/creator.py index f3f4c09e..a7748701 100644 --- a/openkb/skill/creator.py +++ b/openkb/skill/creator.py @@ -20,7 +20,7 @@ from agents import Agent, Runner, ToolOutputImage, ToolOutputText, function_tool from agents.model_settings import ModelSettings -from openkb.config import get_extra_headers, get_timeout_extra_args +from openkb.config import resolve_model_settings from openkb.prompts import load_prompt from openkb.schema import get_agents_md from openkb.skill import skill_dir @@ -154,18 +154,14 @@ def done(summary: str) -> str: done, ], model=f"litellm/{model}", - # Allow the model to issue multiple read tool calls in one turn — - # the compile's early phase is a fan-out (list dir -> read N - # summaries -> read N source page-ranges), and serialising each - # read into its own turn costs roughly 5-10 extra round-trips per - # compile. Writes serialise naturally because each - # `write_skill_file` depends on accumulated reads; the model has - # no reason to issue parallel writes to the same path. - model_settings=ModelSettings( - parallel_tool_calls=True, - extra_headers=get_extra_headers() or None, - extra_args=get_timeout_extra_args(), - ), + # Default to allowing parallel read tool calls — the compile's early + # phase is a fan-out (list dir -> read N summaries -> read N source + # page-ranges), and serialising each read into its own turn costs + # roughly 5-10 extra round-trips per compile. Writes serialise + # naturally because each `write_skill_file` depends on accumulated + # reads; the model has no reason to issue parallel writes to the + # same path. An explicit config value (e.g. `null` for Bedrock) wins. + model_settings=ModelSettings(**resolve_model_settings(default_parallel_tool_calls=True)), ) diff --git a/tests/conftest.py b/tests/conftest.py index 3d67b4b4..d9a28774 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,12 +5,14 @@ @pytest.fixture(autouse=True) def _reset_extra_headers(): - """Keep the process-wide LLM extra-headers / timeout stashes from leaking across tests.""" - from openkb.config import set_extra_headers, set_timeout + """Keep the process-wide LLM extra-headers / timeout / parallel-tool-calls + stashes from leaking across tests.""" + from openkb.config import set_extra_headers, set_parallel_tool_calls, set_timeout yield set_extra_headers({}) set_timeout(None) + set_parallel_tool_calls(None, False) @pytest.fixture diff --git a/tests/test_config.py b/tests/test_config.py index be473fcd..430582ec 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -3,16 +3,125 @@ from openkb.config import ( DEFAULT_CONFIG, get_extra_headers, + get_parallel_tool_calls, get_timeout, load_config, resolve_extra_headers, resolve_litellm_settings, + resolve_model_settings, + resolve_parallel_tool_calls, resolve_timeout, save_config, set_extra_headers, + set_parallel_tool_calls, set_timeout, ) +# --- parallel_tool_calls ------------------------------------------------------ +# +# (value, was_explicit) distinguishes "not configured" (each agent uses its own +# default) from an explicit true/false/null (overrides every agent uniformly). + + +def test_parallel_tool_calls_not_in_default_config(): + # No single default fits every agent (see module docstring above), so this + # key is intentionally absent from DEFAULT_CONFIG — load_config's merge + # must not mask "the user's config.yaml doesn't mention this key". + assert "parallel_tool_calls" not in DEFAULT_CONFIG + + +def test_resolve_parallel_tool_calls_absent_is_unset(): + assert resolve_parallel_tool_calls({}) == (None, False) + + +def test_resolve_parallel_tool_calls_explicit_bools(): + assert resolve_parallel_tool_calls({"parallel_tool_calls": True}) == (True, True) + assert resolve_parallel_tool_calls({"parallel_tool_calls": False}) == (False, True) + + +def test_resolve_parallel_tool_calls_null_means_omit(caplog): + # Explicit null = "don't send the param" (provider default). This is the + # escape hatch for Amazon Bedrock, and is silent (not an invalid value) — + # explicit and distinct from "absent" even though both currently carry a + # value of None; was_explicit is what tells them apart. + with caplog.at_level(logging.WARNING, logger="openkb.config"): + assert resolve_parallel_tool_calls({"parallel_tool_calls": None}) == (None, True) + assert caplog.text == "" + + +def test_resolve_parallel_tool_calls_rejects_non_bool(caplog): + # An invalid value (not true/false/null) degrades to the one value known + # to never break any provider — omit the setting — rather than to a fixed + # bool that could reproduce the exact failure (e.g. Amazon Bedrock) the + # user may have been trying to escape via this exact key. + with caplog.at_level(logging.WARNING, logger="openkb.config"): + assert resolve_parallel_tool_calls({"parallel_tool_calls": "true"}) == (None, True) + assert "parallel_tool_calls" in caplog.text + + +def test_parallel_tool_calls_stash_roundtrip(): + set_parallel_tool_calls(False, True) + assert get_parallel_tool_calls() == (False, True) + set_parallel_tool_calls(True, True) + assert get_parallel_tool_calls() == (True, True) + set_parallel_tool_calls(None, True) + assert get_parallel_tool_calls() == (None, True) + set_parallel_tool_calls(None, False) + assert get_parallel_tool_calls() == (None, False) + + +def test_parallel_tool_calls_stash_default_is_unset(): + # The raw stash default must mean "not configured", matching an absent key, + # so an agent built before _setup_llm_key runs defers to its own default. + set_parallel_tool_calls(None, False) + assert get_parallel_tool_calls() == resolve_parallel_tool_calls({}) + + +# --- resolve_model_settings --------------------------------------------------- + + +def test_resolve_model_settings_uses_own_default_when_unset(): + set_extra_headers({}) + set_timeout(None) + set_parallel_tool_calls(None, False) + assert resolve_model_settings() == { + "extra_headers": None, + "extra_args": None, + "parallel_tool_calls": False, # the function's own default + } + assert resolve_model_settings(default_parallel_tool_calls=None) == { + "extra_headers": None, + "extra_args": None, + "parallel_tool_calls": None, + } + assert resolve_model_settings(default_parallel_tool_calls=True) == { + "extra_headers": None, + "extra_args": None, + "parallel_tool_calls": True, + } + + +def test_resolve_model_settings_explicit_value_overrides_every_default(): + # An explicit config choice always wins over whatever default a specific + # caller would otherwise apply — the whole point of the escape hatch is + # that it works uniformly, regardless of which agent is asking. + set_extra_headers({"X-A": "1"}) + set_timeout(1200.0) + set_parallel_tool_calls(None, True) # explicit null: omit, for everyone + for default in (False, True, None): + assert resolve_model_settings(default_parallel_tool_calls=default) == { + "extra_headers": {"X-A": "1"}, + "extra_args": {"timeout": 1200.0}, + "parallel_tool_calls": None, + } + + set_parallel_tool_calls(True, True) # explicit true: allow parallel, for everyone + for default in (False, True, None): + assert ( + resolve_model_settings(default_parallel_tool_calls=default)["parallel_tool_calls"] + is True + ) + def test_default_config_keys(): assert "model" in DEFAULT_CONFIG diff --git a/tests/test_linter.py b/tests/test_linter.py index 2936f84f..32f464e2 100644 --- a/tests/test_linter.py +++ b/tests/test_linter.py @@ -42,6 +42,34 @@ def test_instructions_mention_gaps(self, tmp_path): assert "Gaps" in agent.instructions or "gaps" in agent.instructions +class TestLintAgentParallelToolCalls: + """Lint never sent parallel_tool_calls before, so its default stays "omit" + (sending any value, incl. False, breaks Bedrock #175). Explicit config wins. + """ + + def test_unset_omits_the_setting(self, tmp_path): + from openkb.config import set_parallel_tool_calls + + set_parallel_tool_calls(None, False) # not configured + agent = build_lint_agent(str(tmp_path), "bedrock/eu.anthropic.claude-sonnet-4-6") + assert agent.model_settings.parallel_tool_calls is None + + def test_explicit_null_omits(self, tmp_path): + from openkb.config import set_parallel_tool_calls + + set_parallel_tool_calls(None, True) # explicit null + agent = build_lint_agent(str(tmp_path), "gpt-4o-mini") + assert agent.model_settings.parallel_tool_calls is None + + def test_explicit_bools_flow_through(self, tmp_path): + from openkb.config import set_parallel_tool_calls + + set_parallel_tool_calls(True, True) + assert build_lint_agent(str(tmp_path), "m").model_settings.parallel_tool_calls is True + set_parallel_tool_calls(False, True) + assert build_lint_agent(str(tmp_path), "m").model_settings.parallel_tool_calls is False + + class TestRunKnowledgeLint: @pytest.mark.asyncio async def test_returns_final_output(self, tmp_path): diff --git a/tests/test_llm_config_passthrough.py b/tests/test_llm_config_passthrough.py index 7a86b73b..3aac2ac6 100644 --- a/tests/test_llm_config_passthrough.py +++ b/tests/test_llm_config_passthrough.py @@ -182,6 +182,29 @@ def test_legacy_toplevel_timeout_still_works(tmp_path, monkeypatch): assert get_timeout() == 900.0 +def test_toplevel_parallel_tool_calls_sets_stash(tmp_path, monkeypatch): + """End-to-end: a top-level `parallel_tool_calls:` in config.yaml lands in the + process-wide stash (with the explicit flag) the next time _setup_llm_key runs. + """ + from openkb.config import get_parallel_tool_calls + + _isolate_env(monkeypatch) + _write_kb_config(tmp_path, "model: gpt-4o-mini\nparallel_tool_calls: true\n") + _setup_llm_key(tmp_path) + assert get_parallel_tool_calls() == (True, True) + + +def test_toplevel_parallel_tool_calls_absent_is_unset(tmp_path, monkeypatch): + """No `parallel_tool_calls:` key → the stash reports "not configured" + ((None, False)), so each agent builder applies its own default.""" + from openkb.config import get_parallel_tool_calls + + _isolate_env(monkeypatch) + _write_kb_config(tmp_path, "model: gpt-4o-mini\n") + _setup_llm_key(tmp_path) + assert get_parallel_tool_calls() == (None, False) + + def test_litellm_block_extra_headers_win_over_legacy_toplevel(tmp_path, monkeypatch): """Symmetric with the timeout precedence test: a litellm: block extra_headers replaces the legacy top-level extra_headers. diff --git a/tests/test_query.py b/tests/test_query.py index f9e68653..14c77250 100644 --- a/tests/test_query.py +++ b/tests/test_query.py @@ -154,14 +154,53 @@ def test_extra_headers_applied_from_stash(self, tmp_path): set_extra_headers({"Editor-Version": "vscode/1.95.0"}) agent = build_query_agent(str(tmp_path), "github_copilot/gpt-5-mini") assert agent.model_settings.extra_headers == {"Editor-Version": "vscode/1.95.0"} - # Existing settings are preserved. - assert agent.model_settings.parallel_tool_calls is False def test_no_extra_headers_by_default(self, tmp_path): agent = build_query_agent(str(tmp_path), "gpt-4o-mini") assert agent.model_settings.extra_headers is None +class TestQueryAgentParallelToolCalls: + """The resolved parallel_tool_calls value (stash) reaches the model settings. + + Default is False (force sequential — unchanged historical behavior). A + config value of null resolves to None, which the agents-SDK omits from the + request — the escape hatch for Amazon Bedrock, whose Claude models reject + the request when parallel_tool_calls is sent at all (issue #175). + """ + + def test_unset_stash_still_defaults_to_false(self, tmp_path): + # No _setup_llm_key has run in this test (stash at its own raw + # "not configured" default) — build_query_agent must still land on + # the historical default, not on some other arbitrary value. + from openkb.config import set_parallel_tool_calls + + set_parallel_tool_calls(None, False) + agent = build_query_agent(str(tmp_path), "gpt-4o-mini") + assert agent.model_settings.parallel_tool_calls is False + + def test_null_stash_is_omitted(self, tmp_path): + from openkb.config import set_parallel_tool_calls + + set_parallel_tool_calls(None, True) + agent = build_query_agent(str(tmp_path), "bedrock/eu.anthropic.claude-sonnet-4-6") + assert agent.model_settings.parallel_tool_calls is None + + def test_false_stash_forces_sequential(self, tmp_path): + from openkb.config import set_parallel_tool_calls + + set_parallel_tool_calls(False, True) + agent = build_query_agent(str(tmp_path), "gpt-4o-mini") + assert agent.model_settings.parallel_tool_calls is False + + def test_true_stash_allows_parallel(self, tmp_path): + from openkb.config import set_parallel_tool_calls + + set_parallel_tool_calls(True, True) + agent = build_query_agent(str(tmp_path), "gpt-4o-mini") + assert agent.model_settings.parallel_tool_calls is True + + class TestQueryAgentTimeout: """Config-driven timeout reaches the agents-SDK model settings via extra_args. diff --git a/tests/test_skill_creator.py b/tests/test_skill_creator.py index bc44d7cf..7ac17e2b 100644 --- a/tests/test_skill_creator.py +++ b/tests/test_skill_creator.py @@ -32,19 +32,54 @@ def _make_kb(tmp_path): return tmp_path -def test_build_agent_interpolates_intent_and_name(tmp_path): - kb = _make_kb(tmp_path) - agent = build_skill_create_agent( +def _build_demo_agent(kb, model="gpt-4o-mini"): + return build_skill_create_agent( wiki_root=str(kb / "wiki"), skill_root=str(kb / "output" / "skills" / "demo"), skill_name="demo", intent="distill a tax-lookup skill", - model="gpt-4o-mini", + model=model, ) + + +def test_build_agent_interpolates_intent_and_name(tmp_path): + kb = _make_kb(tmp_path) + agent = _build_demo_agent(kb) assert "demo" in agent.instructions assert "distill a tax-lookup skill" in agent.instructions +class TestSkillCreateAgentParallelToolCalls: + """skill-create defaults to True (fan-out read phase; see creator.py), but an + explicit config value must override it — a Bedrock user's `null` has to reach + skill creation too, or `openkb skill new` stays broken on Bedrock (#175). + """ + + def test_unset_defaults_to_true(self, tmp_path): + from openkb.config import set_parallel_tool_calls + + kb = _make_kb(tmp_path) + set_parallel_tool_calls(None, False) # not configured + agent = _build_demo_agent(kb) + assert agent.model_settings.parallel_tool_calls is True + + def test_explicit_null_omits_for_bedrock(self, tmp_path): + from openkb.config import set_parallel_tool_calls + + kb = _make_kb(tmp_path) + set_parallel_tool_calls(None, True) # explicit null + agent = _build_demo_agent(kb, model="bedrock/eu.anthropic.claude-sonnet-4-6") + assert agent.model_settings.parallel_tool_calls is None + + def test_explicit_false_overrides_default(self, tmp_path): + from openkb.config import set_parallel_tool_calls + + kb = _make_kb(tmp_path) + set_parallel_tool_calls(False, True) # explicit false + agent = _build_demo_agent(kb) + assert agent.model_settings.parallel_tool_calls is False + + @pytest.mark.asyncio async def test_run_skill_create_creates_output_dir(tmp_path): kb = _make_kb(tmp_path) diff --git a/tests/test_skill_evaluator.py b/tests/test_skill_evaluator.py index dddb1471..54aa2501 100644 --- a/tests/test_skill_evaluator.py +++ b/tests/test_skill_evaluator.py @@ -412,3 +412,59 @@ async def fake_runner(*args, **kwargs): with patch("openkb.skill.evaluator.Runner.run", new=AsyncMock(side_effect=fake_runner)): with pytest.raises(RuntimeError, match="non-JSON output"): await generate_eval_set(skill_dir, model="gpt-4o-mini") + + +# -------- parallel_tool_calls on the (tool-less) eval agents ------------------ +# +# The eval agents have NO tools, so they must OMIT parallel_tool_calls in every +# config state — the SDK forwards an explicit `False` even without tools, which +# strict OpenAI endpoints reject. Regression guards against a future "finish the +# migration" that wires them through resolve_model_settings(). + + +async def _capture_eval_agent(coro_factory): + """Run an eval coroutine with Runner.run mocked, returning the Agent it built.""" + captured = {} + + async def fake_runner(agent, *args, **kwargs): + captured["agent"] = agent + # Return output shaped enough for each caller's parser not to raise. + return SimpleNamespace(final_output="NO-TRIGGER") + + with patch("openkb.skill.evaluator.Runner.run", new=AsyncMock(side_effect=fake_runner)): + try: + await coro_factory() + except Exception: + # We only care about the agent that was built, not the parse result. + pass + return captured["agent"] + + +async def _all_three_eval_agents(skill_dir): + return [ + await _capture_eval_agent( + lambda: generate_eval_set(skill_dir, model="gpt-4o-mini", count=2) + ), + await _capture_eval_agent(lambda: grade_one("desc", "q?", model="gpt-4o-mini")), + await _capture_eval_agent(lambda: grade_coverage("body", "q?", model="gpt-4o-mini")), + ] + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "stash", + [ + (None, False), # not configured + (None, True), # explicit null (Bedrock escape hatch) + (False, True), # explicit false — must NOT reach a tool-less agent as False + (True, True), # explicit true — meaningless without tools + ], +) +async def test_eval_agents_always_omit_parallel_tool_calls(tmp_path, stash): + from openkb.config import set_parallel_tool_calls + + skill_dir = _make_skill(tmp_path) + set_parallel_tool_calls(*stash) + + for agent in await _all_three_eval_agents(skill_dir): + assert agent.model_settings.parallel_tool_calls is None