Fix 20260210 #772
Fix 20260210 #772
Conversation
…gentscope-ai#768) Parse tool input with fallback when content contains literal newlines (e.g. HTML). Validate merged input instead of raw content.
|
zhanghangwei seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Summary of ChangesHello @hangweizhang, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical issue where tool arguments returned by LLMs containing literal newlines within JSON string values (such as embedded HTML) caused parsing failures, leading to tools receiving empty or invalid inputs. The changes introduce a resilient JSON parsing utility that automatically escapes these problematic newlines, ensuring that tool calls are correctly interpreted and executed even with malformed LLM outputs. This significantly improves the reliability of tool invocation within the system. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a robust solution for handling malformed JSON from LLMs, specifically when string values contain literal newlines. The introduction of JsonParseHelper centralizes the logic for sanitizing and parsing such JSON, which is a great approach. The changes in ToolCallsAccumulator and ToolExecutor correctly leverage this new helper to provide fallback parsing. The added tests are comprehensive and cover the new functionality well. I have one suggestion to improve the maintainability of the new helper class by adding logging for parsing failures.
| public final class JsonParseHelper { | ||
|
|
||
| private JsonParseHelper() {} | ||
|
|
||
| /** | ||
| * Parse a JSON string into a map, with fallback to sanitizing literal newlines inside | ||
| * strings when the first attempt fails. Use for tool input JSON that may contain | ||
| * multi-line content (e.g. HTML). | ||
| * | ||
| * @param json JSON string (e.g. tool call arguments) | ||
| * @return parsed map, or null if parsing failed even after sanitization | ||
| */ | ||
| @SuppressWarnings("unchecked") | ||
| public static Map<String, Object> parseMapWithNewlineFallback(String json) { | ||
| if (json == null || json.isEmpty()) { | ||
| return null; | ||
| } | ||
| try { | ||
| Object parsed = JsonUtils.getJsonCodec().fromJson(json, Map.class); | ||
| return parsed != null ? (Map<String, Object>) parsed : null; | ||
| } catch (Exception e) { | ||
| String sanitized = escapeNewlinesInJsonStrings(json); | ||
| try { | ||
| Object parsed = JsonUtils.getJsonCodec().fromJson(sanitized, Map.class); | ||
| return parsed != null ? (Map<String, Object>) parsed : null; | ||
| } catch (Exception e2) { | ||
| return null; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
While the fallback logic is great for robustness, swallowing exceptions without logging can make it very difficult to debug issues where JSON parsing fails for unexpected reasons. It's a good practice to log these exceptions, at least at a debug or warn level, to provide visibility into why parsing is failing. This helps in identifying if the sanitization logic needs to be improved or if the input is fundamentally malformed.
public final class JsonParseHelper {
private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(JsonParseHelper.class);
private JsonParseHelper() {}
/**
* Parse a JSON string into a map, with fallback to sanitizing literal newlines inside
* strings when the first attempt fails. Use for tool input JSON that may contain
* multi-line content (e.g. HTML).
*
* @param json JSON string (e.g. tool call arguments)
* @return parsed map, or null if parsing failed even after sanitization
*/
@SuppressWarnings("unchecked")
public static Map<String, Object> parseMapWithNewlineFallback(String json) {
if (json == null || json.isEmpty()) {
return null;
}
try {
Object parsed = JsonUtils.getJsonCodec().fromJson(json, Map.class);
return parsed != null ? (Map<String, Object>) parsed : null;
} catch (Exception e) {
logger.debug("Failed to parse JSON, trying to sanitize newlines. Error: {}", e.getMessage());
String sanitized = escapeNewlinesInJsonStrings(json);
try {
Object parsed = JsonUtils.getJsonCodec().fromJson(sanitized, Map.class);
return parsed != null ? (Map<String, Object>) parsed : null;
} catch (Exception e2) {
logger.warn("Failed to parse JSON even after sanitizing newlines. Error: {}", e2.getMessage());
return null;
}
}
}
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
1 participant
Fixes #768
When the LLM returns tool arguments as JSON with literal newlines inside string values (e.g. HTML), the JSON is invalid, parsing fails, and the tool gets an empty input (all parameters null).
Changes:
Add JsonParseHelper: escape \n/\r inside JSON strings and parse with fallback.
ToolCallsAccumulator: parse raw content via parseMapWithNewlineFallback when args are empty.
ToolExecutor: build merged input first (including parsing content when input is empty), then validate the merged input instead of the raw content.
Tests: JsonParseHelperTest, ToolCallsAccumulatorTest (newlines in content), ToolExecutorTest (empty input + content with newlines).