fix(cpp): add null-safety checks for JSON string fields in LLM response parsing

[andrewssobral]

Summary

• Add .is_string() / .is_null() guards before all .get<std::string>() calls on LLM response JSON fields
• Fixes crash (json.exception.type_error.302: type must be string, but is null) when smaller LLM models (e.g. qwen3.5:9b) return null for fields like "tool" or "content" instead of omitting them
• json.contains() returns true for null values, so the existing checks were insufficient

Files changed

• cpp/src/json_utils.cpp — parseLlmResponse(): guard tool and answer fields
• cpp/src/agent.cpp — callLlm(): guard content field in both streaming fallback and non-streaming paths
• cpp/src/clean_console.cpp — display methods: guard tool, command, error, output, status fields

Test plan

• Verified qwen3.5:9b (previously crashing) now completes successfully
• Verified qwen3:8b and qwen3.5:cloud still work
• All 278 unit tests pass (./tests_mock)

[kovtcharov]

@claude please review this PR.

[github-actions]

Claude finished @kovtcharov's task —— View job

---

Review Complete

• Read changed files and understand the fix
• Analyze null-safety pattern implementation
• Check for edge cases and completeness
• Provide review feedback

---

Summary

🟢 LGTM - This PR correctly addresses the crash caused by json.contains() returning true for null values.

Code Review

Changes look good. The fix properly guards all .get<std::string>() calls with type checks before extraction:

File	Pattern	Assessment
agent.cpp:308-310, 328-330	.is_string() before content extraction	✅ Correct
json_utils.cpp:253, 281	.is_string() before tool extraction	✅ Correct
json_utils.cpp:278	!.is_null() before answer with jsonToString()	✅ Correct - jsonToString handles non-string types
clean_console.cpp:87,130,139,148,183	.is_string() guards on display fields	✅ Correct

Minor Observations

1. Consistency in json_utils.cpp:250-251: The first answer check at line 250 uses jsonToString() without a null guard, while line 278 adds !j["answer"].is_null(). This is fine because jsonToString() handles non-string types by calling .dump(), but for consistency you could add the null check to both paths. Not a blocker.

2. clean_console.cpp:166: The MCP content format check (item["text"].is_string()) was already correct - good that it matches the pattern.

Verification

The fix correctly identifies that json.contains("key") returns true when the key exists with a null value. Adding .is_string() or !.is_null() checks ensures the code doesn't attempt get<std::string>() on null JSON values.

---

[kovtcharov]

@claude can you explain why the following tests are failing? https://github.com/amd/gaia/actions/runs/24406513448/job/71422231020?pr=780

[github-actions]

Claude finished @kovtcharov's task —— View job

---

Unable to fetch CI logs

• Attempt to fetch CI job log (failed - insufficient permissions)
• Analyze what context is available
• Provide response

---

I'm not able to retrieve the logs for run 24406513448 / job 71422231020 from this environment — the required commands (gh run view --log-failed, gh api, curl against the GitHub API) are not in my allow‑list, so I can't see the actual failure output. I don't want to guess at the root cause without the log.

A few options to move forward:

1. Paste the failing test names / relevant log excerpt here and I'll dig into the specific failures. The most useful pieces are:
   • Which workflow/job (e.g. build_cpp.yml, test_unit.yml, lint.yml, etc.)
   • The first failed test name(s) and the assertion / error message
   • Whether it's a compile error vs. a runtime test failure
2. Re-run the failed jobs from the Actions UI — if the failure is flaky (common on the self-hosted runners for test_lemonade_server, test_chat_agent, etc.) it may pass on retry.
3. Grant me gh run view / gh api access via --allowedTools so I can pull the log directly on the next invocation.

What I can say about this PR's likely impact

Looking at the changes in this PR (null-safety guards only, no signature changes, no control-flow changes beyond "skip extraction when field is null"), the risk of it breaking tests is low and narrow:

• cpp/src/json_utils.cpp — parseLlmResponse(): if a C++ unit test in cpp/tests/test_json_utils.cpp was asserting that parsed.toolName gets populated when "tool": null is in the payload (unlikely, but possible), it would now be empty. Worth checking that file.
• cpp/src/agent.cpp — callLlm(): same consideration for cpp/tests/test_agent.cpp if any test injects a response with "content": null.
• cpp/src/clean_console.cpp — cpp/tests/test_clean_console.cpp display tests may have expected something to be printed for null fields; now those branches are skipped.

If the failures are in any of those three test files, that's almost certainly where to look first. If the failures are in Python tests (test_unit.yml, test_*_agent.yml), they are unrelated to this PR and likely pre-existing flakes or an environment issue.

Happy to do a targeted analysis as soon as you can share the failing test names or the relevant log snippet.
| Branch: cpp-null-safety-fix

[kovtcharov]

@claude is the failing c++ test a result of this PR?