Gokul Kolady has posted comments on this change. ( http://gerrit.cloudera.org:8080/24284 )
Change subject: IMPALA-14963: Query Profile AI Analysis Tool ...................................................................... Patch Set 5: (9 comments) http://gerrit.cloudera.org:8080/#/c/24284/4/be/src/service/query-profile-ai-analysis.cc File be/src/service/query-profile-ai-analysis.cc: http://gerrit.cloudera.org:8080/#/c/24284/4/be/src/service/query-profile-ai-analysis.cc@53 PS4, Line 53: static constexpr size_t MAX_TOOL_RESULT_CHARS = 100000; > Declare functions with static keyword instead of using anonymous namespace. Done http://gerrit.cloudera.org:8080/#/c/24284/4/be/src/service/query-profile-ai-analysis.cc@55 PS4, Line 55: static const char* SYSTEM_INSTRUCTION = > Add the "static" keyword to all these constexpr and const definitions. Done http://gerrit.cloudera.org:8080/#/c/24284/4/be/src/service/query-profile-ai-analysis.cc@94 PS4, Line 94: "function": { > Are the newline characters required? Removed newlines and now using a raw string literal http://gerrit.cloudera.org:8080/#/c/24284/4/be/src/service/query-profile-ai-analysis.cc@294 PS4, Line 294: // Truncates oversized tool output so model payloads stay bounded. > Nit: need blank line. Done http://gerrit.cloudera.org:8080/#/c/24284/4/be/src/service/query-profile-ai-analysis.cc@301 PS4, Line 301: } : : // Builds the model request payload from the current conversation state. : static Status BuildAgentPayloadJson( : const vector<ChatMessage>& conversation, string* payload_json) { : if (payload_json == nullptr) return Status("payload output pointer cannot be null"); : > Replace with JsonToString function from json-util.h. Done http://gerrit.cloudera.org:8080/#/c/24284/4/be/src/service/query-profile-ai-analysis.cc@308 PS4, Line 308: Document tools_doc; : tools_doc.Parse(PROFILE_TOOLS_JSON); : if (tools_doc.HasParseError() || !tools_doc.IsArray()) { : return Status("Failed to parse PROFILE_TOOLS_JSON for AI analysis"); : } : > Replace this function with boost::algorithm::erase_tail to do an in-place t Done http://gerrit.cloudera.org:8080/#/c/24284/4/be/src/service/query-profile-ai-analysis.cc@387 PS4, Line 387: return Status("Failed to create function context for AI analysis"); > Initializing this MemTracker with a value of -1 means it can consume unlimi Done http://gerrit.cloudera.org:8080/#/c/24284/4/be/src/service/query-profile-ai-analysis.cc@512 PS4, Line 512: Document::AllocatorType& error_alloc = tool_error_doc.GetAllocator(); : tool_error_doc.AddMember( : "error", Value(tool_status.GetDetail().c_str(), error_alloc), error_alloc); : tool_json = JsonToString(tool_error_doc); : } > Should the model be notified that it hallucinated a tool? Added logic to let the model know through conversation that it passed us a tool call without an id http://gerrit.cloudera.org:8080/#/c/24284/4/be/src/service/query-profile-ai-analysis.cc@570 PS4, Line 570: return Status("tool > This code is creating a copy of profile_tool_executor. If that is not the Done -- To view, visit http://gerrit.cloudera.org:8080/24284 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia9376b8c56df75032f51abbb76b3ac6927f9107e Gerrit-Change-Number: 24284 Gerrit-PatchSet: 5 Gerrit-Owner: Gokul Kolady <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Gokul Kolady <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Thu, 14 May 2026 20:38:54 +0000 Gerrit-HasComments: Yes
