fix(types): require id field on tool calls in ChatMessage.from_dict

[Pouyanpi]

Description

Resolves the comment #1745 (comment) in #1745. Raise ValueError when tool call dict is missing 'id' or has empty string. Tool call IDs are required for multiturn tool use. The adapter layer synthesizes UUIDs for provider responses that omit IDs, but from_dict receives upstream data that should already have them.

[greptile-apps]

Greptile Summary

Adds strict validation in ChatMessage.from_dict to reject tool call dicts with a missing or empty id field, raising a ValueError at parse time. This complements the adapter layer (which synthesizes UUIDs for provider responses that omit IDs) by enforcing that upstream data passed through from_dict already carries valid IDs required for multiturn tool use.

Confidence Score: 5/5

Safe to merge — the change is a focused, well-tested validation addition with no P0/P1 findings.

Both changed files are straightforward: the validation logic is correct, the two new test cases directly cover the new code paths, and the existing test for the legacy flat format was correctly updated to include an id field. All remaining feedback is P2 style suggestions.

No files require special attention.

Vulnerabilities

No security concerns identified.

Important Files Changed

Filename	Overview
nemoguardrails/types.py	Adds ValueError when a tool call dict passed to from_dict has a missing or empty id field; logic is correct with two minor style notes about error message clarity and whitespace-only strings.
tests/test_types.py	Adds two new test cases (test_from_dict_raises_on_tool_call_missing_id and test_from_dict_raises_on_tool_call_empty_id) covering the new validation; existing legacy flat-format test correctly updated to include id.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["ChatMessage.from_dict(d)"] --> B{raw_tool_calls?}
    B -- No --> G[Build ChatMessage]
    B -- Yes --> C[For each tool call dict]
    C --> D{func_data present?}
    D -- Yes/Nested --> E[Parse nested format]
    D -- No/Legacy --> F[Parse flat format]
    E & F --> H["tc_id = tc.get('id')"]
    H --> I{not tc_id?}
    I -- "True: None or ''" --> J["raise ValueError"]
    I -- "False: valid string" --> K[Append ToolCall]
    K --> C
    C -- done --> G
    G --> L[return ChatMessage]
    style J fill:#ff6b6b,color:#fff
    style L fill:#51cf66,color:#fff

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: nemoguardrails/types.py
Line: 167-169

Comment:
**Same error message for missing vs. empty `id`**

`if not tc_id` evaluates to `True` for both `None` (absent key) and `""` (explicit empty string), but the error text says "missing required 'id' field" in both cases. For the empty-string case the field is present — the message is technically misleading. Consider splitting the message to help callers diagnose the problem:

```suggestion
                tc_id = tc.get("id")
                if tc_id is None:
                    raise ValueError("Tool call missing required 'id' field.")
                if tc_id == "":
                    raise ValueError("Tool call 'id' must be a non-empty string.")
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: nemoguardrails/types.py
Line: 167-169

Comment:
**Whitespace-only IDs pass validation**

`if not tc_id` catches `None` and `""` but lets through strings like `"   "` (spaces only). A whitespace-only ID would fail silently during multiturn tool use instead of raising here at parse time. Consider stripping and checking:

```suggestion
                tc_id = tc.get("id")
                if not (tc_id and tc_id.strip()):
                    raise ValueError("Tool call missing required 'id' field.")
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(types): require id field on tool cal..." | Re-trigger Greptile}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!