feat(gitcommit): add remote, fetch, pull support#26
Conversation
- Add cherry_pick_skip operation - Improve error messages with conflict resolution guidance
- Remove nested output/user_msg/llm_msg wrapper objects - Remove setup handlers from tool definitions - Rename agent to tools in all handler signatures - Remove v17 compatibility code and comments
Summary of ChangesHello @jinzhongjia, 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 significantly enhances the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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 significant enhancements to the git tooling, adding support for remote management, fetch, pull, and more robust handling for merge and cherry-pick conflicts. The changes are well-structured and greatly improve the tool's capabilities. I've identified a couple of areas for improvement, mainly a regression in user feedback in one of the refactored tools and a potential bug in a regex pattern for detecting conflicts. Overall, this is a solid contribution.
| local conflicts = {} | ||
| local conflict_num = 0 | ||
|
|
||
| for conflict_block in content:gmatch("(<<<<<<<.->>>>>>>.-)\n?") do |
There was a problem hiding this comment.
The Lua pattern (<<<<<<<.->>>>>>>.-)\n? used here to find conflict blocks is incorrect. The . character in Lua patterns does not match newline characters by default. Since conflict blocks always span multiple lines, this gmatch will not find any conflicts.
To correctly capture multi-line conflict blocks, you can use a pattern that explicitly handles newlines, such as iterating with string.find or using a more complex pattern that accounts for the structure of conflict markers.
for conflict_block in content:gmatch("(<<<<<<< .-
=======
.->>>>>>> .-
?)") do
| local commits, error_msg = get_detailed_commits(from_tag, to_tag) | ||
| if not commits then | ||
| return { | ||
| status = "error", | ||
| data = { | ||
| output = error_msg, | ||
| user_msg = "✗ " .. error_msg, | ||
| llm_msg = "<aiReleaseNotes>fail: " .. error_msg .. "</aiReleaseNotes>", | ||
| }, | ||
| } | ||
| return { status = "error", data = error_msg } | ||
| end | ||
|
|
||
| if #commits == 0 then | ||
| local msg = string.format("No commits found between %s and %s", from_tag, to_tag) | ||
| return { | ||
| status = "success", | ||
| data = { | ||
| output = msg, | ||
| user_msg = "ℹ " .. msg, | ||
| llm_msg = "<aiReleaseNotes>success: " .. msg .. "</aiReleaseNotes>", | ||
| }, | ||
| } | ||
| return { status = "success", data = msg } | ||
| end | ||
|
|
||
| local prompt = prompts.create_smart_prompt(commits, style, { from = from_tag, to = to_tag }) | ||
|
|
||
| local user_msg = | ||
| string.format("📝 Generating %s release notes: %s → %s (%d commits)", style, from_tag, to_tag, #commits) | ||
|
|
||
| local llm_msg = string.format("<aiReleaseNotes>\n%s\n</aiReleaseNotes>", prompt) | ||
|
|
||
| return { | ||
| status = "success", | ||
| data = { | ||
| output = prompt, | ||
| user_msg = user_msg, | ||
| llm_msg = llm_msg, | ||
| }, | ||
| } | ||
| return { status = "success", data = prompt } | ||
| end, | ||
| } | ||
|
|
||
| AIReleaseNotes.handlers = { | ||
| setup = function(_self, _agent) | ||
| return true | ||
| end, | ||
| on_exit = function(_self, _agent) end, | ||
| on_exit = function(self, tools) end, | ||
| } | ||
|
|
||
| AIReleaseNotes.output = { | ||
| success = function(self, agent, _cmd, stdout) | ||
| local chat = agent.chat | ||
| local data = stdout[1] | ||
| local llm_msg = data and data.llm_msg or data.output | ||
| local user_msg = data and data.user_msg or data.output | ||
| return chat:add_tool_output(self, llm_msg, user_msg) | ||
| success = function(self, tools, cmd, stdout) | ||
| local chat = tools.chat | ||
| local output = stdout and #stdout > 0 and vim.iter(stdout):flatten():join("\n") or "" | ||
| local user_msg = "Release notes generated" | ||
| chat:add_tool_output(self, output, user_msg) | ||
| end, | ||
| error = function(self, agent, _cmd, stderr, stdout) | ||
| local chat = agent.chat | ||
| local data = stderr[1] or stdout[1] | ||
| local llm_msg = data and data.llm_msg or (type(data) == "string" and data or "AI release notes generation failed") | ||
| local user_msg = data and data.user_msg or "AI release notes generation failed" | ||
| return chat:add_tool_output(self, llm_msg, user_msg) | ||
| error = function(self, tools, cmd, stderr, stdout) | ||
| local chat = tools.chat | ||
| local errors = stderr and #stderr > 0 and vim.iter(stderr):flatten():join("\n") or "Unknown error" | ||
| local user_msg = "Release notes generation failed" | ||
| chat:add_tool_output(self, errors, user_msg) | ||
| end, |
There was a problem hiding this comment.
This refactoring has removed the detailed user messages for both success and error cases, replacing them with generic strings. This is a regression in user experience. For example, the user no longer sees which tags are being used for release notes generation.
To restore this valuable feedback, the cmds function can return a table containing the necessary information, and the output handlers can be updated to use it. This will make the tool's feedback much more informative.
local commits, error_msg = get_detailed_commits(from_tag, to_tag)
if not commits then
return { status = "error", data = { user_msg = "✗ " .. error_msg, output = error_msg } }
end
if #commits == 0 then
local msg = string.format("No commits found between %s and %s", from_tag, to_tag)
return { status = "success", data = { user_msg = "ℹ " .. msg, output = msg } }
end
local prompt = prompts.create_smart_prompt(commits, style, { from = from_tag, to = to_tag })
local user_msg = string.format("📝 Generating %s release notes: %s → %s (%d commits)", style, from_tag, to_tag, #commits)
return { status = "success", data = { output = prompt, user_msg = user_msg } }
end,
}
AIReleaseNotes.handlers = {
on_exit = function(self, tools) end,
}
AIReleaseNotes.output = {
success = function(self, tools, cmd, stdout)
local chat = tools.chat
local data = stdout and stdout[1]
local output, user_msg
if type(data) == "table" and data.output then
output = data.output
user_msg = data.user_msg or "Release notes generated"
else
output = stdout and #stdout > 0 and vim.iter(stdout):flatten():join("\n") or ""
user_msg = "Release notes generated"
end
chat:add_tool_output(self, output, user_msg)
end,
error = function(self, tools, cmd, stderr, stdout)
local chat = tools.chat
local data = stderr and stderr[1] or (stdout and stdout[1])
local output, user_msg
if type(data) == "table" and data.output then
output = data.output
user_msg = data.user_msg or "AI release notes generation failed"
else
output = stderr and #stderr > 0 and vim.iter(stderr):flatten():join("\n") or "Unknown error"
user_msg = "AI release notes generation failed"
end
chat:add_tool_output(self, output, user_msg)
end,
| if not ok then | ||
| local error_msg = "Git read operation failed unexpectedly: " .. tostring(result) | ||
| return { | ||
| status = "error", | ||
| data = { | ||
| output = error_msg, | ||
| user_msg = error_msg, | ||
| llm_msg = "<gitReadTool>fail: " .. error_msg .. "</gitReadTool>", | ||
| }, | ||
| } | ||
| return { status = "error", data = error_msg } | ||
| end | ||
|
|
||
| local success, output, user_msg, llm_msg = result.success, result.output, result.user_msg, result.llm_msg | ||
| local op_success, output = result.success, result.output | ||
|
|
||
| -- Ensure proper response format even if operation fails | ||
| if success then | ||
| return { status = "success", data = { output = output, user_msg = user_msg, llm_msg = llm_msg } } | ||
| if op_success then | ||
| return { status = "success", data = output or "Operation completed" } | ||
| else | ||
| -- Ensure consistent error message format | ||
| local formatted_output = { | ||
| output = output or "Git read operation failed", | ||
| user_msg = user_msg or "Git read operation failed", | ||
| llm_msg = llm_msg or "<gitReadTool>fail: Git read operation failed</gitReadTool>", | ||
| } | ||
| return { status = "error", data = formatted_output } | ||
| return { status = "error", data = output or "Git read operation failed" } | ||
| end |
There was a problem hiding this comment.
This refactoring simplifies the cmds return value to a plain string, but in doing so, it discards the rich user_msg and llm_msg that were previously generated by the GitTool functions. This leads to less informative feedback for the user in the output handlers.
To preserve the detailed messages, you could modify the GitTool functions to return a table with all necessary data, and then have the cmds function pass this structured data along. The output handlers can then be updated to parse this table and display the rich messages, similar to the suggestion for ai_release_notes.lua.
1 participant
No description provided.