Fix: only clear chat on successful compact; reset compacting flag

[ericdallo]

Clear/replace chat history only when /compact completes successfully to prevent accidental data loss when the compact operation fails (e.g., due to size limits). Always reset :compacting? on chat finish, and gate on-finished-side-effect behind a success? flag. Adds unit tests.

Fixes #142

🤖 Generated with eca

Prompt + chat history

let's fix this issue: #142

Thought 28.80 s
Developing a Plan

I'm thinking we should create a clear plan that outlines fetching issue details in Phase 2. Initially, the plan can have placeholders indicating what information we'll need to collect. I want to ensure we validate the plan's feasibility and focus on exploring the repository to identify the relevant areas. If we don't know the specifics, I could suggest a general approach: read details, reproduce the issue, write tests, and implement fixes. We need to keep directory commands efficient while exploring.
Planning Phase 1 Tasks

I'm considering loading the GitHub issue content with web.run once the user says "preview changes now." I also want to make sure our plan is clear and follows instructions. We can include a step to fetch issue details from the link provided. Using the "gh issue view" command with eca_shell_command could work for this, although it might not succeed. In Phase 1, we have access to eca_shell_command for retrieving GitHub information, so I think that approach could be fine.
Planning Pull Request Processes

The user wants to create a pull request, but for now, we simply need to view an issue. Using "gh" seems right if it's available, so I plan to call eca_shell_command. First, I'll run "pwd" to get the project root directory. Then, I'll use that to execute "gh issue view 142 -R editor-code-assistant/eca" with the necessary JSON parameters. The tool requires arguments like command, working_directory, and timeout, so I want to ensure everything is set correctly.
Clarifying Tool Usage

I realize that the tool function doesn’t require a description, even though the instructions suggest it would help. The function signature lacks this, so I can provide details in bullet points instead. If "gh" isn’t installed or authenticated, it could fail, but I have a backup plan. I’ll run this in the Explore phase, where I’ll fetch the current directory, check for issue references, and grep for problems based on the issue body. If fetching fails, I could suggest opening the issue link, but web.run isn't an option in Explore.
Planning Phase 1 Execution

If "gh" fails, I’ll note that we couldn't view the issue, but I can continue by scanning the code for relevant areas. It makes sense to attempt "gh" first. In the Explore phase, I'll write 1-3 bullet points before each action. We're planning to determine the project root with "pwd," fetch issue details using "gh issue view," and search the codebase with eca_grep. If a specific file is mentioned, I might inspect it using eca_read_file. I can run "bb test" with eca_shell_command, which is read-only and shouldn’t cause any issues. Let's start by calling functions.eca_shell_command for "pwd" and proceed from there.

Understand

• Goal: Address and propose a concrete fix for GitHub issue A failed 'compact' operation should not clear the chat. #142 in the editor-code-assistant/eca repository.

Explore

• Intention:

  • Determine the project root to produce absolute paths.
  • Retrieve the full details of issue A failed 'compact' operation should not clear the chat. #142 to understand the problem, reproduction steps, and expected behavior.
  • Locate relevant code paths in the local repo that map to the issue’s described behavior.
  • If the issue is testable, attempt to run unit tests to see failures locally.

• Getting the absolute project root for all subsequent paths.
  Running 'pwd' ✅ 0.03 s

Thought 4.03 s

Running 'gh issue view 142 -R edit...' ✅ 0.69 s

Thought 7.94 s

Searching for 'compact' ✅ 0.03 s

Thought 2.86 s

Reading file commands.clj ✅ 0.01 s

Thought 2.26 s

Searching for 'on-finished-side-effec...' ✅ 0.02 s

Reading file chat.clj ✅ 0.01 s

Thought 12.10 s

Searching for 'finish-chat-prompt!' ✅ 0.01 s

Thought 13.37 s

Searching for ':compacting?' ✅ 0.02 s

Reading file chat.clj ✅ 0.00 s

Thought 11.31 s

Reading file chat_test.clj ✅ 0.00 s

Thought 7.05 s

Searching for 'ns eca.test-helper' ✅ 0.02 s

Reading file test_helper.clj ✅ 0.00 s

Thought 11.98 s

Understand

• Implement a safe “compact” flow so that a failed /compact operation does not clear the existing chat history. Specifically, only clear/replace the chat when compact completes successfully.

Explore

• Identify where “/compact” is declared and how it clears history.

• Trace where the compact flow is finalized and when side effects (history clearing) run, especially on error paths.

• Verify any state flags that track compacting flow.

• Locate compact command and its side effects to chat history.

  • Found in /home/greg/dev/eca/src/eca/features/commands.clj: handle-command! has a "compact" branch. It sets [:chats chat-id :compacting?] true, then returns {:type :send-prompt ... :on-finished-side-effect (fn ...)}, whose side effect replaces chat history with the summary and resets usage counters. This is the place that ultimately clears history.

• Find where on-finished-side-effect is invoked to see if it runs on both success and error.

  • In /home/greg/dev/eca/src/eca/features/chat.clj, finish-chat-prompt! unconditionally executes on-finished-side-effect (when present) regardless of how the run ended. finish-chat-prompt! is invoked for both normal finish and error paths. This explains the bug: on errors (e.g., size/limit failure), it still runs the compact side effect that clears history.

• Check compact tool implementation and “compacting?” flag behavior.

  • In /home/greg/dev/eca/src/eca/features/tools/chat.clj, the tool handler "eca_compact_chat" sets [:chats chat-id :compacting?] false and writes [:last-summary]. The tool is enabled only when [:compacting?] is true. If the model never calls the tool (e.g., prompt fails), :compacting? remains true indefinitely unless some other place resets it.

• Verify who calls finish-chat-prompt! and in which scenarios:

  • Normal completion: message handler with type :finish calls finish-chat-prompt! :idle (success).
  • Error/limit: :limit-reached and :on-error also call finish-chat-prompt! :idle (error), but with no distinction at the API level, so the same side effect fires.
  • User stop flow and other command flows also call finish-chat-prompt!.

Conclusion from exploration:

• Root cause: finish-chat-prompt! currently has no notion of success vs. failure and always invokes on-finished-side-effect; hence, compact’s history clearing runs even when compact fails.
• Secondary issue: If compact fails, :compacting? stays true; it should be reset to false when the compact attempt ends (successfully or not).

Decide

Chosen approach:

• Add a success flag to finish-chat-prompt! and gate on-finished-side-effect so it runs only on success. Additionally, ensure the compacting flag is reset to false whenever a compact attempt ends (success or failure).
  • Why: Centralizes completion semantics in a single place (finish-chat-prompt!), aligns with existing call sites in chat.clj, and is minimally invasive (one function plus a few call-site updates).
  • Safety: Prevents history clearing on error; avoids regressions by keeping default behavior “success” unless call sites opt-out; ensures :compacting? isn’t left dangling on failures.

Considered alternatives:

• Add checks inside compact’s on-finished-side-effect (e.g., only clear if :last-summary present).
  • Trade-off: Could still clear history if a previous :last-summary exists from an earlier run; doesn’t address dangling :compacting? on failure.
• Move all responsibility into commands.clj (e.g., wrapping side effect in a success-aware closure).
  • Trade-off: Duplicates success detection logic that’s correctly centralized in chat flow; harder to maintain.

Present Plan

Steps (if applied):

1. Update finish-chat-prompt! in /home/greg/dev/eca/src/eca/features/chat.clj:

   • Extend signature to accept an optional success? boolean (default true).
   • Always mark [:chats chat-id :compacting?] false at the end to conclude any compact attempt.
   • Invoke on-finished-side-effect only when success? is true.

2. Adjust finish-chat-prompt! call sites in /home/greg/dev/eca/src/eca/features/chat.clj:

   • When receiving assistant :finish → call with success? true.
   • On :limit-reached and :on-error → call with success? false.
   • On prompt-stop (user stop) and assert-chat-not-stopped! stop path → call with success? false.
   • Other existing call sites that represent non-LLM command completions can rely on default success behavior (no on-finished-side-effect set in those flows).

3. Add tests:

   • New unit tests in /home/greg/dev/eca/test/eca/features/chat_test.clj:
     • Verify that finish-chat-prompt! does not invoke on-finished-side-effect when success? is false and that [:compacting?] is reset to false.
     • Optional: Verify that success? true still invokes on-finished-side-effect.

4. Run unit tests locally (bb test). If any integration tests exist for compact, optionally run them if requested.

File summary:

• Would modify: /home/greg/dev/eca/src/eca/features/chat.clj
• Would create: /home/greg/dev/eca/test/eca/features/chat_test.clj

Would you like me to preview these changes now?
yes
Thought 49.52 s
Reading and processing files

I'm planning to read chat.clj once and avoid test files since it's a new file. For anchors, I need to ensure the exact text matches, including all whitespace. I've got earlier content, but to be thorough, it seems a second read is wise. The tool guidelines support this. I’ll perform a single read for accuracy, then move to Phase 3 with eca_read_file for chat.clj and follow up with eca_preview_file_change calls. Let’s call the functions!

Reading file chat.clj ✅ 0.02 s

Previewing change chat.clj +22 -22 ✅ 0.01 s
view diff

Previewing change chat.clj +1 -1 ✅ 0.01 s
view diff

Previewing change chat.clj +1 -1 ✅ 0.01 s
view diff

Previewing change chat.clj +1 -1 ✅ 0.00 s
view diff

Previewing change ❌ 0.03 s

Thought 5.11 s

Previewing change chat.clj +1 -1 ✅ 0.01 s
view diff

Previewing change chat.clj +1 -1 ✅ 0.01 s
view diff

Previewing change chat_test.clj +36 -0 ✅ 0.01 s
view diff

Here are the previews for the planned changes.

• Would modify: /home/greg/dev/eca/src/eca/features/chat.clj
  • The finish-chat-prompt! function would become multi-arity and use a success? flag; it would also always reset the compacting? flag to false and only run on-finished-side-effect on success.
  • Call sites that end with error/limit/reject/stop would pass success? false so compaction side effects won’t fire.

Preview 1/5 — update finish-chat-prompt! to gate side effects and reset compacting?:

• The preview would replace the original single-arity function with a two-arity version and add the compacting? reset plus success? gate.
• The preview would show the added arity and comments making the behavior explicit.

Preview 2/5 — assert-chat-not-stopped! calls finish with failure:

• The preview would change (finish-chat-prompt! :idle chat-ctx) to (finish-chat-prompt! :idle chat-ctx false).

Preview 3/5 — limit reached path finishes with failure:

• The preview would change the limit-reached branch to call (finish-chat-prompt! :idle chat-ctx false).

Preview 4/5 — tool-call rejection path finishes with failure:

• The preview would change the rejection branch after sending system text to call (finish-chat-prompt! :idle chat-ctx false).

Preview 5/5 — error callback finishes with failure, and prompt-stop finishes with failure:

• The preview would change the on-error callback to (finish-chat-prompt! :idle chat-ctx false).

• The preview would change prompt-stop to (finish-chat-prompt! :stopping chat-ctx false).

• Would create: /home/greg/dev/eca/test/eca/features/chat_test.clj

  • The preview would add tests ensuring:
    • When finish-chat-prompt! is called with success? false, the on-finished-side-effect does not run and compacting? resets to false.
    • When called with success? true, the on-finished-side-effect runs.

Would you like me to apply these previews as diffs now?

do it

• I added a entry in changelog under unreleased section.