Fix/improve todo tool output formatting #840
Conversation
|
@rumpl @jeanlaurent, there is a case when task description is not rendering i have written a fallback for that, can u check if its fine or we should take some other approach for this case, it is visible like this for that case 
|
|
First, Thanks for working on this @benodiwal |
pkg/tools/builtin/todo.go
Outdated
|
|
||
| return &tools.ToolCallResult{ | ||
| Output: output, | ||
| Output: fmt.Sprintf("Created %d todos", len(params.Descriptions)), |
There was a problem hiding this comment.
We can't remove the current output from the create_todos tool, the LLM needs the IDs of the created todos so that it can update them at a later stage. If we don't show the IDs it will have to make an additional call to list_todos to get their IDs which just wastes tokens.
For this issue we need to only change the way we render the todo calls.
There was a problem hiding this comment.
Yeeah, ig that is the issue task description was not rendering which i mentioned above for the initial todo updates.
cc: @jeanlaurent
| icon, style := renderTodoIcon("completed") | ||
| styledLines = append(styledLines, style.Render(icon)+" "+style.Render(strings.TrimSuffix(strings.TrimSpace(line[2:]), " (Status: completed)"))) | ||
| default: | ||
| // Extract todo content, removing the ID portion |
There was a problem hiding this comment.
This is very brittle, we should find a way to test it and make sure the TUI doesn't break if we change the output of the todo tool
|
@rumpl I have implemented a simple parser to make the logic less fragile with some unit tests for the parser and integration tests for the tui as well, can u check if there are some changes required or not |
|
hey @rumpl, gentle ping to review :) |
3 participants
Closes #836