feat: nudge after todo completion#241
Conversation
- Added functionality to track todo statuses and set a nudge reminder when a todo is marked as completed. - Introduced `todoStatusById` and `todoCompletionNudgePending` properties in the session state. - Updated the `insertPruneToolContext` function to conditionally insert nudge messages based on the new todo completion state. - Refactored prompt loading to use the correct directory path.
|
@Tarquinen if you feel something is wonky, make a review and I'll fix it once I see it. Thanks |
|
Couple thoughts: I think the readme changes are a bit too much, I don't think they need to include information about internal logic such as what tool parameters.filePath is and what the prunable-tools list is. Can we simpify a bit to just what these settings are and how to use them? Since you are reusing most of the existing nudge injection logic, instead of setting a new state flag can we reuse the existing nudge counter, and when a todo changes to completed add config.tools.settings.nudgeFrequency to nudgeCounter? I think your implementation also incorrectly resets the nudge flag, it should reset when one of the prune tools is used, yours resets after a single nudge injection. The problem with this is the model isn't forced to use a tool when the nudge message is injected, it could choose to ignore it and then the nudge is lost on the next fetch. Check tool-cache.ts for how the nudge is reset during tool cache sync and tools.ts for how its reset when a tool is used. As you mentioned i'm not totally convinced this will improve performance, not to mention I haven't come up with a good way to test performance either so its all just vibes. Let me know what you think if you actually use it in a normal use case for a while. |
|
I agree with you that this feature will probably not add much to the perf, so we can actually ignore it. I am basing this on what I observed when I set the nudge so high to basically force pruning to happen only at the end of todos, and my thought was "ummm that's nearly identical to end-of-turn-pruning, so what's the point?" The test was done after I finished the code, so I just made the pr in case you thought it'd be valuable, but I personally didn't think it was after I e2e tested. If you agree, we can close this. |
|
Yea I think I agree with all of your conclusions, I'll close then. Feel free to PR any other improvements you come up with! |
2 participants
Summary
todowritecall transitions an existing todo tostatus: \"completed\".<prunable-tools>message.__dirname) for prompts.Closes #54.
Test plan
npm run typechecknpm testE2E test notes
tools.settings.nudgeFrequencyto999to avoid frequency-based nudges.extract/discardwas only performed at the end of the todo list workflow, triggered by the todo-completion nudge.