ai-code-review does not work normally with local ollama service

[xinyancici li]

Hi, all

I have found a issue when use 'ai-code-review' plugin with Ollama + Qwen2.5-Coder-7B-Instruct model (Q4_K_M), for some simple code (gerrit change), it works normally, llm service response is fit the toolsCall format, and plugin work normall too.

But for most other code,  ollama llm service did not repsonse in toolsCall format, and lead to ‘Cannot invoke "java.util.List.size()" because "toolCalls" is null’ error.

But from plugins's commit history, the plugin is tested with ollama + qwen2.5-coder, so I want to ask: how to configure ollama service or plugin to let it work normally?

My plugin config:

[plugin "ai-code-review"]
    gerritUserName = xxx
    aiModel = qwen2.5-coder:latest
    ai Domain = http://xxx.xxx.xxx.xxx:11434
    aiReviewPatchSet = true
    ai ReviewCommitMessages = true
    gptFullFileReview = true
    globalEnable = false
    enabledProjects = "algorithms"
    enabledFileExtensions = ".py, .java, .js, .jsx, .tx, .tsx, .html, .css, .cs, .cpp, .c, .cxx, .cc, .h, .hpp, .php, .rb, .swift, .kt, .r, .jl, .go, .scala, .pl, .pm, .rs, .dart, .lua, .sh, .vb, .bat"
    maxReviewFileSize = 1048576
    maxReviewLines = 5000

Ollama env parameter:

OLLAMA_HOST=XXX and OLLAMA_DEBUG=1

Other infomation:

1. I also test it using 'chatgpt-code-review-gerrit-plugin' (https://github.com/amarula/chatgpt-code-review-gerrit-plugin) (used with ollama + qwen2.5-coder), found the same issue.
2. ollama version is 0.3.12 which is close to the plugin‘s release date. and for new version like 0.5.7 has same issue.

Data infomation:

• normal request from plugin's debug info:

{ "model": "qwen2.5-coder:latest", "stream": false, "temperature": 0.2, "seed": 580874002, "messages": [ { "role": "system", "content": "Act as a PatchSet Reviewer. I will provide you with PatchSet Diffs for various files in a JSON format. Each changed file\u0027s content will be detailed in the \"content\" field of the JSON object. In this \"content\", the \"a\" items are the lines removed, the \"b\" items are the lines added, and the \"ab\" items are the unchanged lines. In your response, avoid explicitly referring to the \"a\", \"b\", and other fields from the JSON object. Instead, use more intuitive terms like \"new lines\" for additions, \"removed lines\" for deletions, and \"unchanged lines\" for the parts that haven\u0027t been altered. Also, I will supply the history of messages exchanged related to the PatchSet." }, { "role": "user", "content": "To conduct your review, follow these steps in the given order:\nBegin with examining the PatchSet Diff, focusing exclusively on the \"a\" and \"b\" items, and using the \"ab\" items solely as context to understand the changes better. Provide insights on any potential issues you foresee and suggestions for improvements if necessary, with each insight articulated as a separate reply. Concentrate exclusively on spotting and rectifying issues; avoid mentioning any positive elements. For instance, instead of saying \"this is good, but that needs improvement\", simply state \"that needs improvement\". You MUST provide your entire response as a JSON object; no other formats, such as plain text lists of suggestions, will be considered acceptable. Each reply must be formatted as an individual answer object within an array in the key `replies` of the response object, as defined in the tools function named `format_replies`. The answer object includes the string attributes `reply`, `score`, `relevance`, `repeated`, `conflicting`, with the following specifications: `reply` contains the text of the insight; `score` represents a rating (an integer from -1 to 1) attributed to the change being addressed in your reply, based on the reply content; `relevance` is a floating-point number ranging from 0 to 1, representing the importance you assign to the reply following the rules: lower values for improvement suggestions, commit messages not providing details on the testing or verification process, and missing documentation or testing of functionalities; for code issues, set `relevance` in direct relation to the issue\u0027s severity; for commit messages that do not thoroughly describe the changes, adjust `relevance` based on the significance of the unexplained aspects; `repeated` is marked true if any message in the history either contains the same core message as the `reply` or addresses the same code snippet, and is marked false otherwise; `conflicting` is marked true if either of these conditions is met: 1. issuing the reply goes against one of the Directives, or 2. any message in the history, identified by the \u0027assistant\u0027 role, is in conflict with the reply. For replies that are specific to a certain part of the code, the object must additionally include the keys `filename`, `lineNumber`, and `codeSnippet` to precisely identify the relevant code section.\nYou MUST review the commit message of the PatchSet and provide your feedback in an additional reply. The commit message is provided in the \"content\" field of \"/COMMIT_MSG\" in the same way as the file changes. Ensure that the commit message accurately and succinctly describes the changes made, and verify if it matches the nature and scope of the changes in the PatchSet. If your feedback on the commit message is negative, you are required to supply an example of commit message that meets these criteria. For instance, if your comment is \"The commit message lacks detail\", you should follow up with \"A clearer commit message would be \u0027...\u0027\".\nHere are the PatchSet Diffs:\n[{\"content\":[{\"b\":\"\\\"\\\"\\\"\\n\\nhttps://en.wikipedia.org/wiki/Bubble_sort\\n\\nWorst-case performance: O(N^2)\\n\\nIf you call bubble_sort(arr,True), you can see the process of the sort\\nDefault is simulation \u003d False\\n\\n\\\"\\\"\\\"\\n\\n\\ndef bubble_sort(arr, simulation\u003dFalse):\\n def swap(i, j):\\n arr[i], arr[j] \u003d arr[j], arr[i]\\n\\n n \u003d len(arr)\\n swapped \u003d True\\n \\n iteration \u003d 0\\n if simulation:\\n print(\\\"iteration\\\",iteration,\\\":\\\",*arr)\\n x \u003d -1\\n while swapped:\\n swapped \u003d False\\n x \u003d x + 1\\n for i in range(1, n-x):\\n if arr[i - 1] \u003e arr[i]:\\n swap(i - 1, i)\\n swapped \u003d True\\n if simulation:\\n iteration \u003d iteration + 1\\n print(\\\"iteration\\\",iteration,\\\":\\\",*arr)\\n \\n return arr\\n\"}],\"meta_b\":{\"name\":\"bubble_sort.py\",\"content_type\":\"text/x-python\"}},{\"content\":[{\"b\":\"[test] bubble sort\"}],\"meta_b\":{\"name\":\"/COMMIT_MSG\",\"content_type\":\"text/x-gerrit-commit-message\"}},{\"changeId\": \"algorithms~master~Iaae4f60f4527a1cbe74742eb452c4754c2c0bbb0\"}]\n\nHere are the message histories:\n[{\"history\":[{\"role\":\"user\",\"content\":\"@primarius-tech.com \"},{\"role\":\"assistant\",\"content\":\"The function does not include any unit tests. Consider adding a test function to verify the correctness of \\u0027bubble_sort\\u0027.\"},{\"role\":\"assistant\",\"content\":\"The function name \\u0027bubble_sort\\u0027 is not descriptive enough. Consider renaming it to something like \\u0027sort_bubble\\u0027.\"}]},{\"history\":[{\"role\":\"assistant\",\"content\":\"The function does not include any error handling for invalid input types. Consider adding checks to ensure \\u0027arr\\u0027 is a list and \\u0027simulation\\u0027 is a boolean.\"}],\"filename\":\"bubble_sort.py\",\"lineNumber\":13,\"codeSnippet\":\"def bubble_sort(arr, simulation\\u003dFalse):\"},{\"history\":[{\"role\":\"assistant\",\"content\":\"The function does not include any documentation for the parameters \\u0027arr\\u0027 and \\u0027simulation\\u0027. Adding docstrings would improve code readability.\"}],\"filename\":\"bubble_sort.py\",\"lineNumber\":13,\"codeSnippet\":\"def bubble_sort(arr, simulation\\u003dFalse):\"}]" } ], "tools": [ { "type": "function", "function": { "name": "format_replies", "description": "Provide the PatchSet review replies.", "parameters": { "type": "object", "properties": { "replies": { "type": "array", "items": { "type": "object", "properties": { "id": { "type": "integer" }, "reply": { "type": "string" }, "score": { "type": "integer" }, "relevance": { "type": "number" }, "repeated": { "type": "boolean" }, "conflicting": { "type": "boolean" }, "filename": { "type": "string" }, "lineNumber": { "type": "integer" }, "codeSnippet": { "type": "string" } }, "required": [ "reply" ] } }, "changeId": { "type": "string" } }, "required": [ "replies", "changeId" ] } } } ], "tool_choice": { "type": "function", "function": { "name": "format_replies" } } }

• request which will result in an '"java.util.List.size()" because "toolCalls" is null’ error :

{ "model": "qwen2.5-coder:latest", "stream": false, "temperature": 0.2, "seed": 1599006889, "messages": [ { "role": "system", "content": "Act as a PatchSet Reviewer. I will provide you with PatchSet Diffs for various files in a JSON format. Each changed file\u0027s content will be detailed in the \"content\" field of the JSON object. In this \"content\", the \"a\" items are the lines removed, the \"b\" items are the lines added, and the \"ab\" items are the unchanged lines. In your response, avoid explicitly referring to the \"a\", \"b\", and other fields from the JSON object. Instead, use more intuitive terms like \"new lines\" for additions, \"removed lines\" for deletions, and \"unchanged lines\" for the parts that haven\u0027t been altered. Also, I will supply the history of messages exchanged related to the PatchSet." }, { "role": "user", "content": "To conduct your review, follow these steps in the given order:\nBegin with examining the PatchSet Diff, focusing exclusively on the \"a\" and \"b\" items, and using the \"ab\" items solely as context to understand the changes better. Provide insights on any potential issues you foresee and suggestions for improvements if necessary, with each insight articulated as a separate reply. Concentrate exclusively on spotting and rectifying issues; avoid mentioning any positive elements. For instance, instead of saying \"this is good, but that needs improvement\", simply state \"that needs improvement\". You MUST provide your entire response as a JSON object; no other formats, such as plain text lists of suggestions, will be considered acceptable. Each reply must be formatted as an individual answer object within an array in the key `replies` of the response object, as defined in the tools function named `format_replies`. The answer object includes the string attributes `reply`, `score`, `relevance`, `repeated`, `conflicting`, with the following specifications: `reply` contains the text of the insight; `score` represents a rating (an integer from -1 to 1) attributed to the change being addressed in your reply, based on the reply content; `relevance` is a floating-point number ranging from 0 to 1, representing the importance you assign to the reply following the rules: lower values for improvement suggestions, commit messages not providing details on the testing or verification process, and missing documentation or testing of functionalities; for code issues, set `relevance` in direct relation to the issue\u0027s severity; for commit messages that do not thoroughly describe the changes, adjust `relevance` based on the significance of the unexplained aspects; `repeated` is marked true if any message in the history either contains the same core message as the `reply` or addresses the same code snippet, and is marked false otherwise; `conflicting` is marked true if either of these conditions is met: 1. issuing the reply goes against one of the Directives, or 2. any message in the history, identified by the \u0027assistant\u0027 role, is in conflict with the reply. For replies that are specific to a certain part of the code, the object must additionally include the keys `filename`, `lineNumber`, and `codeSnippet` to precisely identify the relevant code section.\nYou MUST review the commit message of the PatchSet and provide your feedback in an additional reply. The commit message is provided in the \"content\" field of \"/COMMIT_MSG\" in the same way as the file changes. Ensure that the commit message accurately and succinctly describes the changes made, and verify if it matches the nature and scope of the changes in the PatchSet. If your feedback on the commit message is negative, you are required to supply an example of commit message that meets these criteria. For instance, if your comment is \"The commit message lacks detail\", you should follow up with \"A clearer commit message would be \u0027...\u0027\".\nHere are the PatchSet Diffs:\n[{\"content\":[{\"b\":\"93 lines\"}],\"meta_b\":{\"name\":\"/COMMIT_MSG\",\"content_type\":\"text/x-gerrit-commit-message\"}},{\"content\":[{\"b\":\"def max_heap_sort(arr, simulation\u003dFalse):\\n \\\"\\\"\\\" Heap Sort that uses a max heap to sort an array in ascending order\\n Complexity: O(n log(n))\\n \\\"\\\"\\\"\\n iteration \u003d 0\\n if simulation:\\n print(\\\"iteration\\\",iteration,\\\":\\\",*arr)\\n \\n for i in range(len(arr) - 1, 0, -1):\\n iteration \u003d max_heapify(arr, i, simulation, iteration)\\n\\n if simulation:\\n iteration \u003d iteration + 1\\n print(\\\"iteration\\\",iteration,\\\":\\\",*arr)\\n return arr\\n\\n\\ndef max_heapify(arr, end, simulation, iteration):\\n \\\"\\\"\\\" Max heapify helper for max_heap_sort\\n \\\"\\\"\\\"\\n last_parent \u003d (end - 1) // 2\\n\\n # Iterate from last parent to first\\n for parent in range(last_parent, -1, -1):\\n current_parent \u003d parent\\n\\n # Iterate from current_parent to last_parent\\n while current_parent \u003c\u003d last_parent:\\n # Find greatest child of current_parent\\n child \u003d 2 * current_parent + 1\\n if child + 1 \u003c\u003d end and arr[child] \u003c arr[child + 1]:\\n child \u003d child + 1\\n\\n # Swap if child is greater than parent\\n if arr[child] \u003e arr[current_parent]:\\n arr[current_parent], arr[child] \u003d arr[child], arr[current_parent]\\n current_parent \u003d child\\n if simulation:\\n iteration \u003d iteration + 1\\n print(\\\"iteration\\\",iteration,\\\":\\\",*arr)\\n # If no swap occurred, no need to keep iterating\\n else:\\n break\\n arr[0], arr[end] \u003d arr[end], arr[0]\\n return iteration\\n\\ndef min_heap_sort(arr, simulation\u003dFalse):\\n \\\"\\\"\\\" Heap Sort that uses a min heap to sort an array in ascending order\\n Complexity: O(n log(n))\\n \\\"\\\"\\\"\\n iteration \u003d 0\\n if simulation:\\n print(\\\"iteration\\\",iteration,\\\":\\\",*arr)\\n \\n for i in range(0, len(arr) - 1):\\n iteration \u003d min_heapify(arr, i, simulation, iteration)\\n\\n return arr\\n\\n\\ndef min_heapify(arr, start, simulation, iteration):\\n \\\"\\\"\\\" Min heapify helper for min_heap_sort\\n \\\"\\\"\\\"\\n # Offset last_parent by the start (last_parent calculated as if start index was 0)\\n # All array accesses need to be offset by start\\n end \u003d len(arr) - 1\\n last_parent \u003d (end - start - 1) // 2\\n\\n # Iterate from last parent to first\\n for parent in range(last_parent, -1, -1):\\n current_parent \u003d parent\\n\\n # Iterate from current_parent to last_parent\\n while current_parent \u003c\u003d last_parent:\\n # Find lesser child of current_parent\\n child \u003d 2 * current_parent + 1\\n if child + 1 \u003c\u003d end - start and arr[child + start] \u003e arr[\\n child + 1 + start]:\\n child \u003d child + 1\\n \\n # Swap if child is less than parent\\n if arr[child + start] \u003c arr[current_parent + start]:\\n arr[current_parent + start], arr[child + start] \u003d \\\\\\n arr[child + start], arr[current_parent + start]\\n current_parent \u003d child\\n if simulation:\\n iteration \u003d iteration + 1\\n print(\\\"iteration\\\",iteration,\\\":\\\",*arr)\\n # If no swap occurred, no need to keep iterating\\n else:\\n break\\n return iteration\\n\"}],\"meta_b\":{\"name\":\"heap_sort_93.py\",\"content_type\":\"text/x-python\"}},{\"changeId\": \"algorithms~master~I824294edb5ccb528cc11ed4431106b8cf14df875\"}]\n\nHere are the message histories:\n[{\"history\":[{\"role\":\"assistant\",\"content\":\"The logic for finding the greatest child in \\u0027max_heapify\\u0027 is incorrect. It should compare \\u0027arr[child]\\u0027 with \\u0027arr[child + 1]\\u0027 without adding 1 to \\u0027child\\u0027. This will cause an out-of-bounds error.\"}],\"filename\":\"heap_sort_93.py\",\"lineNumber\":32,\"codeSnippet\":\"if child + 1 \\u003c\\u003d end and arr[child] \\u003c arr[child + 1]:\\n child \\u003d child + 1\"},{\"history\":[{\"role\":\"user\",\"content\":\"@primarius-tech.com \"}]},{\"history\":[{\"role\":\"assistant\",\"content\":\"The function name \\u0027max_heap_sort\\u0027 suggests it sorts in ascending order, but the implementation uses a max heap to sort in descending order. This is confusing and should be clarified.\"}],\"filename\":\"heap_sort_93.py\",\"lineNumber\":1,\"codeSnippet\":\"def max_heap_sort(arr, simulation\\u003dFalse):\"},{\"history\":[{\"role\":\"assistant\",\"content\":\"The function name \\u0027min_heap_sort\\u0027 suggests it sorts in ascending order, but the implementation uses a min heap to sort in descending order. This is confusing and should be clarified.\"}],\"filename\":\"heap_sort_93.py\",\"lineNumber\":47,\"codeSnippet\":\"def min_heap_sort(arr, simulation\\u003dFalse):\"},{\"history\":[{\"role\":\"assistant\",\"content\":\"The logic for finding the lesser child in \\u0027min_heapify\\u0027 is incorrect. It should compare \\u0027arr[child + start]\\u0027 with \\u0027arr[child + 1 + start]\\u0027 without adding 1 to \\u0027child\\u0027. This will cause an out-of-bounds error.\"}],\"filename\":\"heap_sort_93.py\",\"lineNumber\":79,\"codeSnippet\":\"if child + 1 \\u003c\\u003d end - start and arr[child + start] \\u003e arr[\\n child + 1 + start]:\\n child \\u003d child + 1\"}]" } ], "tools": [ { "type": "function", "function": { "name": "format_replies", "description": "Provide the PatchSet review replies.", "parameters": { "type": "object", "properties": { "replies": { "type": "array", "items": { "type": "object", "properties": { "id": { "type": "integer" }, "reply": { "type": "string" }, "score": { "type": "integer" }, "relevance": { "type": "number" }, "repeated": { "type": "boolean" }, "conflicting": { "type": "boolean" }, "filename": { "type": "string" }, "lineNumber": { "type": "integer" }, "codeSnippet": { "type": "string" } }, "required": [ "reply" ] } }, "changeId": { "type": "string" } }, "required": [ "replies", "changeId" ] } } } ], "tool_choice": { "type": "function", "function": { "name": "format_replies" } } }