feat(checkpoints): create checkpoint when user sends a message #7713
Conversation
dosubot
bot
added
size:XS
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
src/core/task/Task.ts
Outdated
| // Create a checkpoint whenever the user sends a message. | ||
| // Use allowEmpty=true to ensure a checkpoint is recorded even if there are no file changes. | ||
| if (askResponse === "messageResponse") { | ||
| void this.checkpointSave(true) |
There was a problem hiding this comment.
Missing test coverage for this new checkpoint behavior. The existing Task.spec.ts doesn't test handleWebviewAskResponse with messageResponse triggering checkpoints. Could this lead to regressions? Perhaps I should add tests to verify:
- Checkpoint is created when askResponse === 'messageResponse'
- Checkpoint uses allowEmpty=true
- Other response types don't trigger checkpoints
src/core/task/Task.ts
Outdated
| // Create a checkpoint whenever the user sends a message. | ||
| // Use allowEmpty=true to ensure a checkpoint is recorded even if there are no file changes. | ||
| if (askResponse === "messageResponse") { | ||
| void this.checkpointSave(true) |
There was a problem hiding this comment.
Is the fire-and-forget approach intentional here? If users send multiple messages quickly, could there be race conditions with concurrent checkpoint saves? The RepoPerTaskCheckpointService might handle this internally, but worth considering if saves should be queued or if the current behavior is acceptable.
src/core/task/Task.ts
Outdated
| // Create a checkpoint whenever the user sends a message. | ||
| // Use allowEmpty=true to ensure a checkpoint is recorded even if there are no file changes. | ||
| if (askResponse === "messageResponse") { | ||
| void this.checkpointSave(true) |
There was a problem hiding this comment.
I notice we're using to ensure checkpoints even without file changes. This differs from other checkpoint triggers (write_to_file, apply_diff) that only save when files change. Is creating potentially many empty commits in the shadow git repository the intended behavior? It could be useful for conversation history, but might also create noise.
src/core/task/Task.ts
Outdated
| // Create a checkpoint whenever the user sends a message. | ||
| // Use allowEmpty=true to ensure a checkpoint is recorded even if there are no file changes. | ||
| if (askResponse === "messageResponse") { | ||
| void this.checkpointSave(true) |
There was a problem hiding this comment.
Consider adding error handling or at least logging if checkpoint saving fails. While the fire-and-forget approach prevents UI blocking, silent failures might make debugging difficult:
hannesrudolph
added
the
Issue/PR - Triage
… current checkpoint updated without a chat row
dosubot
bot
added
size:S
- Propagate suppressMessage flag through event chain properly - Update ChatView to check checkpoint metadata for suppressMessage flag - Ensure checkpoint messages are created but not rendered when suppressed - Fix bug where checkpointSave(false) should have been checkpointSave(true)
dosubot
bot
added
size:M
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
|
|
||
| // Remove the 500-message limit to prevent array index shifting | ||
| // Virtuoso is designed to efficiently handle large lists through virtualization | ||
| const newVisibleMessages = modifiedMessages.filter((message) => { |
There was a problem hiding this comment.
The filtering logic for 'checkpoint_saved' messages now checks for a 'suppressMessage' flag and excludes those messages. This is a good approach; consider extracting this complex condition into a small helper function to improve readability and maintainability.
- Changed allowEmpty from true to false in checkpointSave call - Checkpoints will now only be created when there are actual file changes - This avoids creating empty commits in the shadow git repository
- Fixed test expectation to match the new function signature - saveCheckpoint now expects both allowEmpty and suppressMessage parameters
Implements automatic checkpoint creation when a user sends a message. Implementation detail: triggers checkpointSave(true) on messageResponse in Task.handleWebviewAskResponse, ensuring a checkpoint is created as soon as a user message is accepted without blocking the UI. Behavior: - Uses existing RepoPerTaskCheckpointService to persist checkpoints (shadow git). - Forces allowEmpty to ensure a commit even when no file diffs exist. - Non-blocking (fire-and-forget). Notes: - Tests for affected paths pass locally; full suite has unrelated env-dependent failures. - Respects existing enableCheckpoints flag and telemetry hooks.
Important
Automatic checkpoint creation on user message with non-blocking implementation and message suppression in chat view.
Task.handleWebviewAskResponse().RepoPerTaskCheckpointServicewithallowEmptyto ensure commits even without file diffs.suppressMessageflag to control chat visibility.checkpointSave()inindex.tsupdated to acceptsuppressMessageparameter.handleWebviewAskResponse()inTask.tstriggers checkpoint creation with suppression.ChatView.tsxfilters out suppressedcheckpoint_savedmessages from display.checkpoint.test.tsto includesuppressMessagehandling.This description was created by
for cfaa471. You can customize this summary. It will automatically update as commits are pushed.