feat(discovery): record parallel tool-call capability and exclude single-tool models from orchestrator/free - #972
Conversation
…gle-tool models from orchestrator/free - Add supports_parallel_tool_calls to DiscoveredModel (True/False/None). - Populate from provider supported_parameters listing parallel_tool_calls. - Add probe_discovered_model_tool_call_capability for live 400 evidence. - Wire field through is_general_chat_candidate, is_discovered_chat_candidate, is_routable_discovered_model, general_free_serving_candidates, agent_from_discovered, serving_tags_for_discovered, _is_general_chat_agent, and _is_general_free_agent. - Emit tool_call:multi/tool_call:single tags and reject tool_call:single agents from the general free pool. - Add tests and ADR 0039; update product-technical-gap-baseline.md. Fixes #940. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Original prompt from Automation
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
…er probe Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…l-capability' into devin/17592943-parallel-tool-call-capability # Conflicts: # tests/test_auto_discovery_server.py
|
Cross-PR integration contract: routing identity is provider-neutral |
Resolves the merge conflict between this PR's parallel-tool-call capability work and main's independent evolution since this branch forked (stale main commit c6c3a0c). - orchestrator.py: combine HEAD's supports_parallel_tool_calls computation/threading in _is_general_chat_agent with main's independently-added "structured:blocked" guard. - model_discovery.py: combine HEAD's supported_parameters consensus aggregation and _parallel_tool_call_evidence call with main's max_output_tokens/context_window aggregation and top_provider lookup in _parse_openai_compatible. - __main__.py: restructure _auto_discover_runtime_agents to layer HEAD's capability-blocked tracking (discovery:blocked:capability tag with preserve-disabled semantics, mirroring main's existing spend/structured block-tag pattern) on top of main's independently landed configured-gateway structured-chat probing, embedding routability, and limits-changed tracking. The capability_blocked signal is now computed from a stripped (supports_parallel_tool_calls reset to None) DiscoveredModel so it stays a distinct block reason rather than silently folding into spend_routable. Removed HEAD's now-superseded _should_preserve_operator_disabled_state helper in favor of main's more general block_markers-based preserve-disabled computation. - tests: merged independently-added test functions in test_auto_discovery_server.py and test_model_discovery.py (no logical overlap, both sides' tests kept). - docs: renamed docs/planning/adrs/0039-parallel-tool-call-capability.md to 0042 (main independently claimed 0039 for 0039-request-scoped-configured-endpoint-routing.md since this branch forked); updated the matching gap-baseline.md cross-reference. Verified: PYTHONPATH=. python -m pytest tests -q (3342 passed, only the 5 known pre-existing failures unrelated to this change), 100% interrogate docstring coverage on touched files, git diff --check clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
|
Merged current Why this wasn't redundant with main. Before resolving, I verified whether Conflict resolution, file by file:
Verification: Generated by Claude Code Generated by Claude Code |
| ## Research grounding | ||
|
|
||
| This decision is a capability-constrained routing safeguard, not a learned quality claim. | ||
| It reuses the repository's existing vendored routing literature: | ||
|
|
||
| - Chen, L., Zaharia, M., & Zou, J. (2023). *FrugalGPT: How to use large language models while reducing cost and improving performance*. arXiv. https://arxiv.org/abs/2305.05176 | ||
| - Ding, D., Mallick, A., Wang, C., Sim, R., Mukherjee, S., Rühle, V., Lakshmanan, L. V. S., & Awadallah, A. H. (2024). *Hybrid LLM: Cost-efficient and quality-aware query routing*. International Conference on Learning Representations. https://arxiv.org/abs/2404.14618 | ||
| - Ong, I., Almahairi, A., Wu, V., Chiang, W.-L., Wu, T., Gonzalez, J. E., Kadous, M. W., & Stoica, I. (2024). *RouteLLM: Learning to route LLMs with preference data*. arXiv. https://arxiv.org/abs/2406.18665 | ||
|
|
||
| These papers justify preserving explicit capability evidence at the routing boundary. | ||
| They do not justify inferring multi-tool support from model names or from a bare 200 | ||
| response, so this ADR keeps the field fail-closed on ambiguity. | ||
|
|
||
| The cited PDFs are already vendored in `docs/papers/`; no additional restricted paper is | ||
| copied in this run. |
…all tag loss, unbounded probe read) Two real findings from Devin's review of a1103da (the main-merge commit for this PR): 1. _refresh_discovered_tool_call_tags (contextual_orchestrator/__main__.py) conflated "a discovery marker for EITHER polarity has ever been seen" with "this specific visible tag is discovery-owned". Once discovery supplied evidence once, a later refresh would strip an operator's pre-existing tool_call:single/multi override too, even though it predated any discovery marker and belongs to the opposite polarity. Now only the visible tag paired with its own hidden discovery marker is treated as discovery-owned; an operator-authored tag of the other polarity survives evidence going from known back to unknown. 2. probe_discovered_model_tool_call_capability (model_discovery.py) read the entire provider response body with an unbounded response.read()/ exc.read(). Applied the same bounded-read-then-check pattern already used elsewhere in this module (MAX_DISCOVERY_RESPONSE_BYTES): an oversized body now returns None (ambiguous evidence) instead of buffering an unbounded amount of memory. Added regression tests for both: - test_auto_discovery_preserves_operator_tool_call_override_when_evidence_goes_stale - test_probe_discovered_model_tool_call_capability_rejects_oversized_response - test_probe_discovered_model_tool_call_capability_rejects_oversized_400_body The third finding (ADR 0042 already cites its research PDFs under docs/papers/) was informational, no action needed. Verified: targeted test files pass (194 passed), interrogate 100% on touched files, git diff --check clean; full suite rerun in progress. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
|
Addressed Devin's review of the merge commit ( Fixed — Fixed — No action — Verification: targeted test files ( Generated by Claude Code Generated by Claude Code |
|
Confirmed — the same-polarity case is real and distinct from what I just fixed. Reproduced directly: model = DiscoveredModel(..., supports_parallel_tool_calls=False) # discovery agrees: single
tags = ("discovered", "chat", "tool_call:single") # operator already set this, no discovery marker yet
tags = _refresh_discovered_tool_call_tags(tags, model)
# -> ('discovered', 'chat', 'tool_call:single', 'tool_call:single', 'discovery:tool_call:single')
# (duplicate 'tool_call:single' -- one from the operator, one just appended by discovery_tool_call_tags)
tags = _refresh_discovered_tool_call_tags(tags, replace(model, supports_parallel_tool_calls=None))
# -> ('discovered', 'chat') -- BOTH copies of 'tool_call:single' are gone, including the operator'sWhy I'm not pushing a fix for this one unilaterally. Unlike the different-polarity case, this isn't fixable by pairing markers with their own visible tag more carefully — the two Three ways to actually close this, none of which I want to pick unilaterally:
Happy to implement whichever direction you'd rather have; I don't think this should be guessed at silently given it's a safety-relevant override (excluding a model from parallel tool calls). Generated by Claude Code Generated by Claude Code |
Seonghobae pushed test_discovery_tool_call_ownership.py directly to this branch, specifying the desired same-polarity behavior as failing acceptance tests (the different-polarity case was already fixed in bd422bd; this closes the remaining gap Devin's re-review confirmed was distinct). The gap: when an operator's pre-existing tool_call:single/multi tag happens to match discovery's own value, the two are string-identical, so _refresh_discovered_tool_call_tags had no way to tell them apart once discovery's hidden marker got attached -- a later refresh would treat both occurrences as discovery-owned and strip the operator's tag along with discovery's when evidence went stale. Fix: discovery never claims ownership (by adding its hidden marker) of a visible tag that is already present without one. Since the two tags are indistinguishable strings, the only safe rule is "if it was already there unclaimed, it stays unclaimed" -- discovery's matching evidence is still effectively honored (the correct value is already showing), but the marker that would let a later refresh treat it as discovery's own (and thus remove it in the same-polarity case) is never added. Verified by exhaustive trace across every existing and new scenario (different-polarity known->known->unknown, same-polarity known-> known->unknown, fresh-discovery no-operator, discovery value flips, repeated identical discovery evidence) before implementing, then confirmed: tests/test_discovery_tool_call_ownership.py (the new acceptance tests) + test_auto_discovery_server.py + test_model_discovery.py all pass (196 passed), plus the broader chat_capability/provider_bootstrap/ discover_models_cli/provider_catalog_store/multimodal_model_group_http suites (147 passed). interrogate 100% on touched files. git diff --check clean. Full suite rerun in progress. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
|
Merged The fix: discovery never claims ownership (by attaching its hidden I traced this against every scenario before implementing — both new same-polarity tests, the different-polarity test from the previous fix, the pure-discovery lifecycle (fresh agent, evidence flipping between values, repeated identical evidence — no duplicate-tag regression), all check out. Verified: Generated by Claude Code Generated by Claude Code |
…el-tool-call-capability
…el-tool-call-capability Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
noema-review failure diagnosis + branch updateActual cause of the What I did:
This should give the required checks a fresh head to review/re-run against. Generated by Claude Code |
PR #972 (feat(discovery): record parallel tool-call capability and exclude single-tool models from orchestrator/free) independently added docs/planning/adrs/0042-parallel-tool-call-capability.md, colliding with this PR's 0042-opencode-go-provider-discovery.md -- neither number exists on main yet. Per this repo's CLAUDE.md, "a same-number collision is a rename, not a redesign," and PR #972 was updated earlier than this PR, so this PR's ADR renumbers instead. 0130 is the next free number after checking docs/planning/adrs/ on current origin/main (highest: 0126) and every other open PR's added ADR files (0127 PR #1020, 0128 PR #1012, 0129 PR #1030, plus PR #972's untouched 0042). Renamed docs/planning/adrs/0042-opencode-go-provider-discovery.md to 0130-opencode-go-provider-discovery.md, updated its front-matter id, and updated the three in-repo prose references to "ADR 0042" for this ADR (contextual_orchestrator/model_discovery.py comment, tests/test_model_discovery.py docstring, docs/kv-credentials.md, CHANGELOG.d/opencode-go-provider-discovery.md). Left the coincidental "0042" substring in docs/planning/adrs/0004-pr-review-merge-loop.md (part of an unrelated git SHA) untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
There was a problem hiding this comment.
Noema LLM review
PR adds supports_parallel_tool_calls to DiscoveredModel and propagates it through gateway metadata merge, OpenAI-compatible parsing, duplicate reconciliation, catalog persistence, bootstrap tags, and runtime routing to exclude single-tool models from general chat/free pools. Prior review threads show all reported bugs were addressed with tests. Fresh diff review found no concrete blocking regressions.
Reviewed changed lines
contextual_orchestrator/chat_capability.py:143 (RIGHT): Guardif supports_parallel_tool_calls is False: return Falseexcludes models with explicit single-tool evidence from general chat candidates while preserving existing eligibility for None evidence.contextual_orchestrator/orchestrator.py:740 (RIGHT):_is_general_chat_agentderivessupports_parallel_tool_callsfrom persisted tags and fails closed for conflictingtool_call:single/tool_call:multievidence.
Adversarial validation
contextual_orchestrator/chat_capability.py:143 (RIGHT)falsified: Addingif supports_parallel_tool_calls is False: return Falsecould regress routing for models where evidence is None, e.g., an implicit None argument now blocks a previously eligible model. — Default isNone; the early return is only triggered for explicitFalse. The existingtest_unproven_tool_call_parallelism_keeps_existing_eligibilityin tests/test_chat_capability.py asserts None and True both preserve True eligibility.contextual_orchestrator/orchestrator.py:740 (RIGHT)falsified:_is_general_chat_agentderivessupports_parallel_tool_callsfrom tags, but a malformed agent carrying bothtool_call:singleandtool_call:multicould be admitted to general chat despite contradictory evidence. — Theif "tool_call:single" in agent.tagsbranch takes precedence overtool_call:multi, setting supports_parallel_tool_calls to False. tests/test_chat_capability.py::test_conflicting_tool_call_tags_fail_closed explicitly constructs this scenario and asserts False.- Residual risk: None identified.
Findings
- No blocking findings.
- Result: APPROVE
- Head SHA:
0208f2ea182d8a0134d53eac0d7e3fa3a1cb6ada - Reviewer credential:
noema-review-github-app-refresh - Actor:
cwl-noema-review[bot]
…el-tool-call-capability-local
|
ADR number collision:
|
| PR | File (status added) |
PR created |
|---|---|---|
| #911 | docs/planning/adrs/0042-time-windowed-routing-observations.md |
2026-08-28T20:58:42Z |
| #972 (this PR) | docs/planning/adrs/0042-parallel-tool-call-capability.md |
2026-08-31T09:57:37Z |
0042 does not exist on main, so both are genuinely new claims on the same number rather than edits to an existing ADR.
Suggested resolution — a rename, not a redesign. #911 claimed the number first, so the smaller change is for this PR to renumber. The next free number is 0043 (occupied: 0001–0011, 0015, 0019–0042, 0124–0129; the mid-series gaps 0012–0014 and 0016–0018 look deliberate, so 0043 is the safe next sequential choice).
Nothing in this PR's content is in question — only the filename and any in-document self-reference to the number.
Method / limitations, so this is checkable rather than taken on faith. ADR filenames were collected via pulls/{n}/files for every open PR and intersected by four-digit prefix; only status != "removed" entries counted. No open PR exceeds 100 changed files, so single-page pagination did not truncate the scan. main itself is clean — 38 ADRs, zero duplicate numbers.
중복 판정: #972 × #1028 — 둘 다 살아 있고, 승계되지 않았으며, 태그 방출기만 정리하면 됩니다두 PR이 비테스트 소스 5개 파일을 공유하고 서로 9곳에서 충돌하기에 중복 여부를 트리로 확인했습니다. 먼저: 어느 쪽도
|
Summary
orchestrator/freeis the zero-cost, capability-blind pool used by the org's central review agents (OpenCode, Noema, Strix). A model in this pool must accept arbitrary chat requests, including requests with multiple tool calls. Issue #940 shows that NIM'smeta/llama-3.2-11b-vision-instructrejects multi-tool requests withThis model only supports single tool-calls at once!. PreviouslyDiscoveredModelcarried no tool-call parallelism signal, so there was no honest way to keep such a model out of the general free pool.This PR adds
supports_parallel_tool_calls: bool | None = NonetoDiscoveredModel, wires it through discovery-time selectors and runtimeModelAgenttags, and rejects single-tool-call agents from_is_general_free_agent.What changed
DiscoveredModelnow hassupports_parallel_tool_calls._parallel_tool_call_evidence()reads a positive signal from providersupported_parameterscontaining"parallel_tool_calls".probe_discovered_model_tool_call_capability()performs an opt-in livePOST /chat/completionsprobe withparallel_tool_calls: trueand two tool definitions. It returnsTrueon 200,Falsewhen the 400 body explicitly says single-tool-call only, andNoneon any ambiguous/network/auth error.is_general_chat_candidate(..., supports_parallel_tool_calls=...)returnsFalsewhen the evidence isFalse.is_discovered_chat_candidateandis_routable_discovered_modelnow use the field.general_free_serving_candidatesexcludes models whose evidence isFalse(via_requires_single_tool_call).agent_from_discoveredandprovider_bootstrap.serving_tags_for_discoveredemittool_call:multiortool_call:singletags.TaskOrchestrator._is_general_chat_agentderives the value from those tags and passes it tois_general_chat_candidate.TaskOrchestrator._is_general_free_agentnow requiresnot self._agent_requires_single_tool_call(agent)in addition to the existing non-text-input exclusion.docs/product-technical-gap-baseline.mdare added/updated.Follow-up
ContextualWisdomLab/.github's review sidecar still builds its own catalog and does not consumegeneral_free_serving_candidatesor preserveinput:/tool_call:tags. A separate PR to the.githubrepo is needed before the NIM vision/single-tool models are fully excluded from CIorchestrator/freeselection.Fixes #940.
Link to Devin session: https://app.devin.ai/sessions/97593a84ee7649e5afa96899f5c4c475
Open in Devin Desktop: https://app.devin.ai/desktop/session/97593a84ee7649e5afa96899f5c4c475?variant=devin