feat(cuga-lite): native function calling (opt-in)#472
Conversation
…l tests - strict-xfail reproductions for issue #471: parallel tool_calls dropped (only tool_calls[0] transpiled), text preamble causes tool_calls to be ignored entirely, and NotImplementedError from bind_tools crashes the run instead of falling back (NotImplementedError is a RuntimeError) - passing code-act control proves the failures are FC-encoding-specific - deterministic scripted BaseChatModel: no API keys or network required - add architecture report: function-calling status, defect table D1-D9, and the ToolCalling / tool-invocation-policy design proposal Refs #471
…rted - NotImplementedError from BaseChatModel.bind_tools is a RuntimeError subclass, so the cap's 'except RuntimeError: raise' re-raised it and crashed call_model instead of degrading to code-act (issue #471 D9) - add _safe_bind() wrapping every bind_tools call; on NotImplementedError it logs and returns the unbound model - ungated and universally safe: turning a crash into graceful fallback cannot change any previously-successful path Refs #471
…opt-in - extract_code_from_response_tool_calls(response, multi=): multi=False keeps the legacy single-call output byte for byte; multi=True transpiles every tool call in the turn into a sequential result_i block, so parallel native tool calls execute instead of only tool_calls[0] (issue #471 D1) - identifier safety: non-identifier tool names are reported with a visible skip marker, never spliced into source (getattr/globals/eval are blocked) - gate: AgentGraphAdapter resolves cuga_lite_tool_invocation_mode (native/ hybrid) per call_model into _allow_native_tool_calls; normalize_response passes it as multi. Default/absent => code => byte-identical legacy path - Supervisor adapter never sets the flag, so it is unaffected Refs #471
- under FC opt-in, normalize_response now transpiles native tool_calls even when the model prefixes them with natural-language text; the preamble is demoted to reasoning so it is preserved, not lost (issue #471 D2) - an explicit code fence in the content still wins (code-act intent), and with FC off the method is byte-identical to before - unit tests cover FC on/off x preamble/empty/code-block/plain-answer and a never-raises guard Refs #471
- rule 7 of mcp_prompt.jinja2 becomes conditional on allow_native_tool_calls: code mode (default) renders the existing 'NO FUNCTION CALLING JSON' ban verbatim; native/hybrid permits native tool calls with a code-vs-call decision heuristic (issue #471 D4) - create_mcp_prompt gains allow_native_tool_calls (default False); prepare_node derives it from cuga_lite_tool_invocation_mode (default 'code') - add settings default + validator for cuga_lite_tool_invocation_mode - golden test asserts the default/code-mode prompt is byte-identical Refs #471
- new ToolCalling model (leaf module) + tool_calling_to_configurable():
mode code(default)/native/hybrid, native_tools/apps selection,
include_find_tools, max_bound_tools; serializes to the existing
cuga_lite_bind_tools_* + cuga_lite_tool_invocation_mode configurable keys
- CugaAgent(tool_calling=...) and invoke()/stream(tool_calling=...); the
per-invoke argument overrides the constructor default. Merged via
setdefault so explicit configurable keys are never clobbered
- default None / mode='code' serializes to {} => code-act path untouched
- promote the bind_tools cap to a per-run configurable override (D7)
- export ToolCalling from the package top level
Refs #471
- README 'Native Function Calling' section: enable via ToolCalling, options, default-off note, model-dependent behavior - runnable example docs/examples/native_function_calling/main.py - report: mark P0/P1 shipped; record the watsonx real-API finding (gpt-oss prefers code even in native mode — native path wired correctly and non-regressive; scripted tests are the rigorous before/after proof) Refs #471
… calling - ToolCalling.tool_choice (auto | required | none | provider-specific) -> cuga_lite_tool_choice; passed to bind_tools for the real-tool bind modes - _safe_bind applies tool_choice with capability detection: providers that reject it (TypeError/ValueError/NotImplementedError) fall back to a plain bind instead of crashing (issue #471 D5) - lets code-preferring models be nudged/forced toward native tool calls Refs #471
- add prompts/mcp_prompt_native.jinja2: structurally parallel to the code-act prompt (same role, apps, tools, knowledge, skills, todos, pagination and data-source rules) but reframed for native function calling — no sandbox, code, Python, print or await language; tools presented as callable functions - revert the rule-7 conditional in mcp_prompt.jinja2: the code-act prompt is now byte-identical to main and used only when tool calling is off - prepare_node selects the native template when cuga_lite_tool_invocation_mode is native/hybrid (cached loader get_native_mcp_prompt_template), else the code-act template; guarded, falls back to code-act on load failure - this is what makes real models actually emit native tool calls: with the code-only prompt they keep writing Python; verified live that watsonx gpt-oss-120b now returns native tool calls under this prompt - tests assert the two templates' framing and that code-act is unchanged Refs #471
- README: dedicated function-calling system prompt + tool_choice option - report: replace the 'prompt conditional' plan with the dedicated native template design; record that the live watsonx model now emits native tool calls under it (native_fired 2-3/run) vs 0 with the code-act prompt Refs #471
…apter state Review blockers on the function-calling branch: - the declared advanced_features.cuga_lite_tool_invocation_mode setting was never read at runtime; resolve_tool_invocation_mode() now resolves configurable > global setting > 'code' (invalid values coerce to 'code'), used by both the prompt selection and normalize_response - _allow_native_tool_calls was mutable state on the shared per-graph adapter, racing across concurrent invokes with different tool_calling modes; the mode is now derived per call — normalize_response(response, configurable) (base signature extended, default None; supervisor unaffected) — and the adapter attribute is removed. Test proves alternating modes don't leak. - add correct-vs-incorrect examples to the native prompt (tool-calling equivalents of the code-act examples: exact names, no app prefixes, no invented tools, no answering from memory, no announce-without-acting, isolated tools, per-page pagination calls) Note: gpt-oss-20b profile users keep today's legacy behavior by design — binding without native mode still uses the unchanged code prompt and legacy recovery; enable FC via settings or the SDK to get the fixed semantics. Refs #471
Adversarial branch review (7 finders + per-finding verification) surfaced
real gaps beyond the earlier blockers; fixed the confirmed ones:
- HIGH: native mode injected the code-act bundled few-shots (
find_tools exemplars) as chat-prefix turns, contradicting the FC prompt and
pushing the model back to code. prepare_node now skips bundled few-shots in
native/hybrid (explicit caller few-shots still honored).
- MED: transpiler spliced tool args as k=v kwargs — a keyword/non-identifier
param name (Gmail 'from', OData '$filter', 'page-size') made the whole
block a SyntaxError. multi=True now emits dict-splat await name(**{...});
keyword tool names are skipped, not spliced.
- MED: global cuga_lite_tool_invocation_mode='native' bound zero tools (bind
mode still defaulted to none) → model answered from memory. resolve now
couples them: native/hybrid + no bind mode -> bind 'all'. Verified live.
- MED: normalize_response gate 'python' so a text
preamble no longer suppresses native tool_calls.
- LOW: multi=False (legacy/default) now inspects only tool_calls[0] (byte-
identical to main) — a malformed leading entry yields None, not the later
valid call, restoring the default-unchanged guarantee.
- LOW: ToolCalling(mode='code') / any explicit ToolCalling now serializes the
invocation-mode key so a per-invoke opt-out overrides a global native default
(None still -> {}).
- doc: tool_choice='required' non-termination hazard documented; report and
test docstrings corrected (per-call mode derivation; D1/D2/D9 all fixed).
Tests added: keyword/non-identifier args parse; keyword tool-name skip;
malformed-leading-tool_call legacy parity; native-mode few-shot skip; prompt-
template selection wiring; explicit-code override.
Refs #471
… native prompt Two items the deep review flagged that were left out of the last pass: - fix(llm): is_tool_choice_none_tool_use_failed now inspects the exception's structured .body dict (not only str(err), which for these provider errors is just the status code). This is a pre-existing bug in errors.py — the two failing tests in test_tool_use_failed_recovery.py now pass. The FC tool_choice path does not use this function, but the Groq tool_choice=none retry ladder relies on it; string form still detected, unrelated tool_use_failed still False. - feat(cuga_lite): the native function-calling prompt's skills block was missing the STEP 0/1/2 install-first ordering (load_skill -> run every install via run_command before use). Ported from the code-act prompt, reframed for tool calls (no sandbox/code language). Covers the native + skills + shell combo. Refs #471
…ools) Review follow-up: mode='code' previously only forced the code-act prompt and legacy handling but left binding profile-driven, so a model profile that binds (gpt-oss-20b) would still bind tools under an explicit code opt-out. Now mode='code' also serializes cuga_lite_bind_tools_mode='none', overriding the profile — an explicit code opt-out is truly code-act. Verified end-to-end: resolve_bind_tools_fields on gpt-oss-20b returns 'none' with the opt-out and 'apps' without it (profile default preserved when no tool_calling is passed). Also: correct stale report §7 (13 commits not 6; per-call mode not adapter._allow_native_tool_calls; tool_choice shipped, not deferred). Refs #471
segevshlomovIBM
added
enhancement
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughCuga Lite now supports opt-in native and hybrid tool calling through typed SDK configuration, native prompts, guarded binding, multi-call response recovery, per-call overrides, documentation, examples, and automated tests. ChangesNative function-calling configuration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant CugaAgent
participant CugaLite
participant BoundModel
participant Sandbox
Caller->>CugaAgent: invoke with ToolCalling(mode="native")
CugaAgent->>CugaLite: pass configurable invocation mode
CugaLite->>BoundModel: bind selected tools
BoundModel-->>CugaLite: return native tool_calls
CugaLite->>Sandbox: transpile and execute all tool calls
Sandbox-->>CugaLite: return tool results
CugaLite-->>Caller: return final answer
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
the
readability: fair
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
src/cuga/backend/cuga_graph/nodes/cuga_lite/tests/test_tool_calling_sdk_integration.py (1)
18-66: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting shared test infrastructure.
This file duplicates nearly all test infrastructure from
test_native_tool_calls_execution.py: theNOTIFIEDglobal,notifytool,ScriptedFCModel/_ScriptedFCModel,TWO_TOOL_CALLS/TWO_CALLS, the autouse reset fixture, and the agent factory helper. Extracting these into a sharedconftest.pyortest_utils.pyin the sametests/directory would reduce maintenance burden and keep the two files focused on their distinct assertions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cuga/backend/cuga_graph/nodes/cuga_lite/tests/test_tool_calling_sdk_integration.py` around lines 18 - 66, Extract the duplicated test infrastructure shared with test_native_tool_calls_execution.py into a common conftest.py or test_utils.py within the tests directory. Move the NOTIFIED state, notify tool, _ScriptedFCModel, TWO_CALLS, reset fixture, and _agent factory there, then update both test modules to import and reuse these shared symbols while preserving their existing assertions.src/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/prepare_node.py (1)
600-610: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRuff BLE001 warning on the broad
except Exceptioncatch.Static analysis flags line 607 with BLE001 (blind exception catch). This is a deliberate graceful-degradation fallback (native template load fails → code-act prompt), and the pattern is consistent with existing
except Exceptionblocks elsewhere in this file (lines 424, 535, 593). As per coding guidelines,ruff checkmust pass before committing — verify this warning doesn't escalate to an error in the project's Ruff configuration. If it does, consider narrowing toexcept (OSError, TemplateError)or adding a# noqa: BLE001comment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/prepare_node.py` around lines 600 - 610, Resolve the Ruff BLE001 violation in the native prompt fallback around get_native_mcp_prompt_template: verify the project’s Ruff configuration, then either narrow the exception to the specific template-loading errors (such as OSError and TemplateError) or add a targeted BLE001 suppression if broad graceful fallback is required. Preserve the existing warning and code-act prompt fallback behavior.Sources: Coding guidelines, Linters/SAST tools
src/cuga/sdk.py (1)
1612-1630: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the new public
tool_callingparameters.The constructor,
invoke(), andstream()expose this API, but theirArgssections omit it. Document the supported modes and the distinction between omitted configuration and explicitToolCalling(mode="code").Also applies to: 2187-2195, 2530-2544
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cuga/sdk.py` around lines 1612 - 1630, Update the Args documentation for the constructor, invoke(), and stream() to include the public tool_calling parameter. Describe the supported ToolCalling modes and distinguish omitted/None configuration from explicitly passing ToolCalling(mode="code"), while preserving the existing parameter documentation.tests/unit/test_tool_calling_config.py (1)
44-49: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd coverage for explicit
include_find_tools=False.The tests cover only the true case. Add a case where a global/profile setting enables
find_toolsbutToolCalling(..., include_find_tools=False)must keep it disabled; update the exact expected dictionary accordingly.Also applies to: 59-67
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_tool_calling_config.py` around lines 44 - 49, Add a test alongside test_native_all_tools covering ToolCalling(mode="native", include_find_tools=False) when the global or profile configuration enables find_tools; assert the resulting configuration keeps find_tools disabled and matches the exact expected dictionary, preserving the existing true-case coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/reports/function_calling_cuga_lite.md`:
- Around line 49-55: Update the Markdown example fence surrounding the
transpiler output in function_calling_cuga_lite.md to use four backticks, so the
nested triple-backtick fence remains part of the rendered example.
- Around line 294-298: Update the implementation plan’s gating description to
document both supported opt-in paths: a per-call SDK configurable value and the
global `settings.advanced_features.cuga_lite_tool_invocation_mode` setting used
when no per-call value is supplied. Keep the existing precedence and default
behavior described by `resolve_tool_invocation_mode()` and
`native_tool_calls_enabled()`.
In `@src/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/response_utils.py`:
- Around line 38-52: Update _parse_one_tool_call to treat malformed untrusted
entries as None: only read function.name or function.arguments when function is
a dict, and require name to be a string rather than merely truthy. Preserve the
existing argument parsing and empty-dict fallback for valid entries, and add
regression coverage alongside the existing malformed-entry tests for non-dict
function values and non-string names.
In
`@src/cuga/backend/cuga_graph/nodes/cuga_lite/tests/test_tool_calling_sdk_integration.py`:
- Line 29: Replace the mutable class-level default for script in the affected
test fixture with the same safe initialization pattern used to fix
test_native_tool_calls_execution.py, such as a per-instance default factory.
Preserve the List[AIMessage] type and ensure each test instance receives its own
independent script list.
In `@src/cuga/backend/cuga_graph/nodes/cuga_lite/tool_calling.py`:
- Around line 8-9: Correct the default-mode documentation near ToolCalling to
state that ToolCalling() is not serialized as an empty object: only None is a
no-op, while explicit ToolCalling(mode="code") serializes the "code" and "none"
keys.
- Around line 100-110: Update the tool-call configuration construction around
tc.include_find_tools so cuga_lite_bind_tools_include_find_tools is always
serialized as a boolean for native/hybrid tool-selection modes, including an
explicit false value, preventing global or profile defaults from widening
selections. Add a regression test covering include_find_tools=False with an
explicit native tool selection.
- Around line 116-118: Update the serialization failure handler in the
tool-calling configuration method to return the explicit configuration that
disables native function calling and unbinds any tool-calling behavior, rather
than returning an empty dictionary. Preserve the warning log and ensure the
failure result overrides global settings or model profiles.
---
Nitpick comments:
In `@src/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/prepare_node.py`:
- Around line 600-610: Resolve the Ruff BLE001 violation in the native prompt
fallback around get_native_mcp_prompt_template: verify the project’s Ruff
configuration, then either narrow the exception to the specific template-loading
errors (such as OSError and TemplateError) or add a targeted BLE001 suppression
if broad graceful fallback is required. Preserve the existing warning and
code-act prompt fallback behavior.
In
`@src/cuga/backend/cuga_graph/nodes/cuga_lite/tests/test_tool_calling_sdk_integration.py`:
- Around line 18-66: Extract the duplicated test infrastructure shared with
test_native_tool_calls_execution.py into a common conftest.py or test_utils.py
within the tests directory. Move the NOTIFIED state, notify tool,
_ScriptedFCModel, TWO_CALLS, reset fixture, and _agent factory there, then
update both test modules to import and reuse these shared symbols while
preserving their existing assertions.
In `@src/cuga/sdk.py`:
- Around line 1612-1630: Update the Args documentation for the constructor,
invoke(), and stream() to include the public tool_calling parameter. Describe
the supported ToolCalling modes and distinguish omitted/None configuration from
explicitly passing ToolCalling(mode="code"), while preserving the existing
parameter documentation.
In `@tests/unit/test_tool_calling_config.py`:
- Around line 44-49: Add a test alongside test_native_all_tools covering
ToolCalling(mode="native", include_find_tools=False) when the global or profile
configuration enables find_tools; assert the resulting configuration keeps
find_tools disabled and matches the exact expected dictionary, preserving the
existing true-case coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a58bde54-e46d-489c-b7b0-518d414bc66c
📒 Files selected for processing (25)
README.mddocs/examples/native_function_calling/main.pydocs/reports/function_calling_cuga_lite.mdsrc/cuga/__init__.pysrc/cuga/backend/cuga_graph/nodes/cuga_agent_core/graph/graph_nodes.pysrc/cuga/backend/cuga_graph/nodes/cuga_agent_core/graph/shared_nodes.pysrc/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/graph_adapter.pysrc/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/prepare_node.pysrc/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/response_utils.pysrc/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/tests/test_normalize_response_native.pysrc/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/tests/test_response_tool_calls_transpile.pysrc/cuga/backend/cuga_graph/nodes/cuga_lite/helpers/bind_tools.pysrc/cuga/backend/cuga_graph/nodes/cuga_lite/prompt_utils.pysrc/cuga/backend/cuga_graph/nodes/cuga_lite/prompts/mcp_prompt_native.jinja2src/cuga/backend/cuga_graph/nodes/cuga_lite/tests/test_native_tool_calls_execution.pysrc/cuga/backend/cuga_graph/nodes/cuga_lite/tests/test_prepare_node_native_few_shots.pysrc/cuga/backend/cuga_graph/nodes/cuga_lite/tests/test_tool_calling_sdk_integration.pysrc/cuga/backend/cuga_graph/nodes/cuga_lite/tool_calling.pysrc/cuga/backend/llm/errors.pysrc/cuga/config.pysrc/cuga/sdk.pysrc/cuga/settings.tomltests/unit/test_bind_tools_safe_fallback.pytests/unit/test_mcp_prompt_native_tool_calls.pytests/unit/test_tool_calling_config.py
- fail-closed serialization: tool_calling_to_configurable now returns the
code/unbind config (not {}) on any error, so a serialization failure can
never leave native binding enabled via a global setting / model profile
- always pin cuga_lite_bind_tools_include_find_tools (True or False) for an
explicit native/hybrid selection, so a global/profile can't widen it
- _parse_one_tool_call guards a truthy non-dict 'function' field (would
AttributeError) and a truthy non-string 'name' (would crash isidentifier);
malformed provider entries -> None, never raise
- tests: malformed-entry skip (multi + legacy), include_find_tools pinned,
fail-closed serialization
- RUF012: scripted-model test fields use Field(default_factory=list)
- docs: fix nested markdown fence; document the global-setting opt-in path
alongside the SDK path; correct the ToolCalling default docstring
Refs #471
coderabbitai
Bot
added
the
readability: good
coderabbitai
Bot
removed
the
readability: fair
|
@coderabbitai thanks for the thorough review — all 7 comments are addressed in Major
Minor / stability
Verification: focused FC suite + |
|
I'll take a look at ヽ(•‿•)ノ ✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cuga/backend/cuga_graph/nodes/cuga_lite/tool_calling.py`:
- Line 123: Update the intentional broad exception handler at `except Exception
as e` in the tool-calling serialization flow with an inline Ruff `noqa` for
BLE001 and a brief justification describing the fail-closed behavior, while
preserving the existing exception handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 137452d3-c7a8-416a-81a4-7a03ec20e14c
📒 Files selected for processing (7)
docs/reports/function_calling_cuga_lite.mdsrc/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/response_utils.pysrc/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/tests/test_response_tool_calls_transpile.pysrc/cuga/backend/cuga_graph/nodes/cuga_lite/tests/test_native_tool_calls_execution.pysrc/cuga/backend/cuga_graph/nodes/cuga_lite/tests/test_tool_calling_sdk_integration.pysrc/cuga/backend/cuga_graph/nodes/cuga_lite/tool_calling.pytests/unit/test_tool_calling_config.py
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/unit/test_tool_calling_config.py
- src/cuga/backend/cuga_graph/nodes/cuga_lite/tests/test_native_tool_calls_execution.py
- src/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/response_utils.py
- src/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/tests/test_response_tool_calls_transpile.py
- docs/reports/function_calling_cuga_lite.md
# Conflicts: # src/cuga/config.py
haroldship
left a comment
haroldship
left a comment
There was a problem hiding this comment.
really solid work overall — the gating is careful, the default-unchanged claim holds up, and the test coverage on the new paths is thorough. requesting changes on three things, all inline:
-
schema rejection at invoke time isn't handled — providers validate tool schemas when the request is sent, not at bind, so a loose MCP schema 400s inside ainvoke_model and kills the run instead of degrading to code-act (comment on _safe_bind).
-
unparseable tool-call args become {} and the tool still executes — for all-optional-param tools that means running with defaults and reporting success (comment on response_utils.py).
-
native mode defaults to binding all tools, which routes big tool sets into the per-turn shortlister and its deliberate hard-raise. in m3/vakra some domains have several hundred tools, so this hits us immediately (comment on bind_tools.py).
all three break the "every new path degrades to code-act" promise in the PR description, and all three look like small fixes.
the rest are non-blocking: hybrid renders the pure-native prompt so it's not really hybrid (decide + document), a couple of small nits inline.
|
|
||
| def normalize_response(self, response: Any) -> Tuple[str, Optional[str]]: | ||
| def normalize_response( | ||
| self, response: Any, configurable: Optional[dict] = None |
There was a problem hiding this comment.
configurable parameter is never explained. In graph_nodes.py it just says "not used here". should explain in the docstrings what different values are allowed and what they do
| from cuga.backend.cuga_graph.nodes.cuga_lite.tool_calling import native_tool_calls_enabled | ||
|
|
||
|
|
||
| def _safe_bind(model: BaseChatModel, tools: List[StructuredTool], tool_choice: Any = None) -> BaseChatModel: |
There was a problem hiding this comment.
this guard only covers half the failure. it fires when bind_tools() itself raises — but hosted providers (OpenAI, watsonx, ...) don't validate tool schemas at bind time, binding is just local config. they validate when the chat request is actually sent, so a tool with a loose/invalid schema (common with MCP tools — missing param types, odd keywords) comes back as a 400 from inside ainvoke_model, where only the Groq tool_use_failed shape is caught — everything else re-raises and kills the run. so a tool set that works fine in code-act (schemas never leave the prompt) can crash in mode="native", which breaks the "every new path degrades to code-act" guarantee.
suggestion: when FC is opted in, catch provider bad-request errors at invoke time, log that the tool schemas were rejected, and retry once without the bound tools. note ainvoke_model only gets the bound model, so either the unbound model needs to be passed in, or the catch/retry belongs in call_model where both are in scope. (alternatively note it as a known gap in the report's deferred list — but I think it belongs in this PR since it's the ladder's biggest hole.)
| # Derived per call from configurable/settings (no adapter state: the | ||
| # adapter is shared across concurrent invokes with possibly different | ||
| # tool_calling modes). | ||
| allow_native = native_tool_calls_enabled(configurable) |
There was a problem hiding this comment.
Related gap just above in ainvoke_model: this method only recovers the Groq tool_use_failed error. Under native/hybrid, a provider rejecting the bound tool schemas (400 at invoke time) re-raises and crashes the run instead of degrading to code-act — see my comment on _safe_bind.
| try: | ||
| args = json.loads(args) | ||
| except json.JSONDecodeError: | ||
| args = {} |
There was a problem hiding this comment.
if args is a string that fails json parsing, the tool still gets called — just with no args. same for the line below: a non-dict args becomes {}. for a tool with required params that's just a TypeError the model can retry, but for a tool where all params are optional it will run with its defaults instead of the intended args, and the agent reports success. that's the same silent-wrong-execution problem this PR is fixing (D1).
I don't think returning None works either — _parse_tool_calls silently drops None entries, so the call would silently not run at all. better: on the multi/native path, skip the call and emit a visible marker like _tool_call_statements already does for non-identifier names, e.g. print("[skipped tool call with unparseable args: notify]"), so the model sees it and retries. (multi=True only — the legacy path has to stay byte-identical to main.)
| # mode was chosen, bind all tools — otherwise the native prompt tells the | ||
| # model to call tools that were never bound. The SDK ToolCalling path already | ||
| # sets a concrete bind mode, so this only fills the global-setting gap. | ||
| if mode in ("", "none", "false", "0", "off") and native_tool_calls_enabled(cfg): |
There was a problem hiding this comment.
defaulting to bind "all" puts the cap/shortlister machinery on the main SDK path, and I don't think it's ready for that. binding is resolved inside call_model, so with more than cuga_lite_bind_tools_max_count (default 128) (aside, I added that because of m3) tools every turn re-collects all tools and makes an extra LLM round-trip to shortlist them — hidden latency and cost, per turn. and the shortlister was deliberately built strict for benchmarks: hallucinated names or a missing query raise RuntimeError, which resolve_bind_tools deliberately re-raises — so the run crashes instead of degrading to code-act, which breaks the "every new path degrades to code-act" guarantee. this isn't a corner case for us: in m3/vakra there are domains with several hundred tools, so ToolCalling(mode="native") — the exact one-liner the README advertises — hits the shortlister on every turn from the start.
suggestion: when the mode came from ToolCalling (i.e., it's not a benchmark config), catch the cap RuntimeError and degrade to the unbound code-act model with a log warning. and/or reconsider the "all" default (same default in tool_calling_to_configurable, tool_calling.py:114) or at least warn on ToolCalling that large tool sets mean a per-turn shortlister call and a possible hard failure, so callers know to pass native_tools=[...]/apps=[...] or max_bound_tools=0.
| content = tool_code | ||
| elif not content: | ||
| # Legacy path (FC off, or FC on with a code block): unchanged behavior. | ||
| tool_code = extract_code_from_response_tool_calls(response, multi=allow_native) |
There was a problem hiding this comment.
nit: the elif is only reachable when allow_native is False (empty content can't contain a fence), so this is always multi=False — write that literally.
| # code-act intent wins). A non-python fence (```json / ```text) in the | ||
| # preamble must NOT suppress the tool calls. The preamble is preserved | ||
| # as reasoning, not lost (issue #471 D2). | ||
| tool_code = extract_code_from_response_tool_calls(response, multi=True) |
There was a problem hiding this comment.
when "```python" in content → the elif is skipped for non-empty content, tool_calls are ignored, and nothing is logged. If a model emits both a fence and tool_calls, that's a D1-shaped silent drop by design; add a logger.warning so it's diagnosable.
| t for t in (tools_for_prompt or []) if getattr(t, "name", None) | ||
| ] | ||
|
|
||
| # Native function calling (issue #471): when enabled (resolved above), |
There was a problem hiding this comment.
non-blocking: hybrid and native select the same template here, and mcp_prompt_native.jinja2 has all code/Python language removed — it even says output must be tool calls or plain text. so the model in hybrid mode is never told it may write code, which makes hybrid indistinguishable from native at the prompt level. the docs say hybrid means "both encodings are offered; the model picks per step". either give the native template a {% if hybrid %} section that offers the python block as an option, or re-document hybrid as "native prompt, code tolerated if the model emits it". both are fine with me, but docs and behavior should match.
2 participants
feat(cuga-lite): native function calling (opt-in)
Closes #471
Why
CUGA-lite is a code-act agent: the model calls tools by writing Python run in a sandbox. It also had a
bind_toolssubsystem, but native tool calls were mishandled on execution — only the first of several was run (the rest silently dropped while the agent reported success), a text preamble suppressed the calls entirely, and a model whosebind_toolsraisedNotImplementedErrorcrashed the run. This PR turns native function calling into a correct, opt-in capability without changing the default code-act path.Scope (what this PR does)
await tool(**{...})) so keyword / non-identifier parameter names (Gmailfrom, OData$filter,page-size) can't produce a SyntaxError.mcp_prompt_native.jinja2, structurally parallel to the code-act prompt (same role, apps, tools, knowledge/skills/todos, pagination and data-source rules — including the skill install-ordering) but with all sandbox/code/Python language removed and tools reframed as native functions. It's selected only when FC is on; the code-act prompt is untouched (byte-identical tomain).ToolCalling.CugaAgent(tools=[...], tool_calling=ToolCalling(mode="native"))and a per-invoke overrideinvoke(..., tool_calling=...). Options:mode(code|native|hybrid),native_tools/appsselection,include_find_tools,max_bound_tools, andtool_choice(auto|required|provider-specific — nudges/forces native calls for code-preferring models, with capability-detection fallback). Also readable as a global settingadvanced_features.cuga_lite_tool_invocation_mode.bind_toolsdegrades to the unbound (code-act) model instead of crashingcall_model; providers that rejecttool_choicefall back to a plain bind. Every new path istry/except-guarded and degrades to code-act.mode="code"opt-out. An explicitToolCalling(mode="code")forces the code-act prompt/handling and unbinds tools (cuga_lite_bind_tools_mode="none"), overriding a globalnativedefault and any model profile that would otherwise bind (e.g. gpt-oss-20b) — so an explicit code opt-out is truly code-act.Default is unchanged (verified)
Off by default at every layer (
settings.toml, config validator, andToolCallingdefault all"code"). With no opt-in: the code-act prompt is byte-identical tomain(0-line diff), the legacy tool-call recovery is byte-identical (single-call, empty-content-only), few-shots inject as before, and no tools are bound. Concurrency-safe: the invocation mode is derived per call fromconfigurable(not stored on the shared adapter). Dedicated "flag-off" tests lock this in.Not in this PR (deferred, additive/non-breaking)
asyncio.gather(multi-call turns run sequentially) — needs a sandbox-injected helper sinceimport asynciois restricted.StructuredTool.args_schema.invocationbreakdown (native vs code counts).on_unsupported="error"(currently only thefallback_codedegradation ladder is implemented).Testing
normalize_responseon/off × preamble/empty/code-wins/concurrency;_safe_bind+tool_choicefallback;ToolCalling→configurable + mode resolver + code opt-out unbinds the gpt-oss-20b profile; native-prompt framing, install-ordering, and template-selection wiring; few-shot skip).ruff checkclean.run_tests.sh unit_tests(CI unit job) green.gpt-oss-120b: with the native prompt the model emits native tool calls that CUGA executes (both tools run, correct answer). Also fixes a pre-existingerrors.pybody-formtool_use_faileddetection bug surfaced during review.Notes for reviewers
docs/reports/security_secret_protection_poc.{md,html}untracked files are from a separate task and are not part of this PR.docs/reports/function_calling_cuga_lite.md.Summary by CodeRabbit
ToolCalling, withnative/hybrid/codemodes and configurable tool binding andtool_choice.tool_choiceisn’t supported.