Skip to content

Improve ECA Menu UX and Streamline Workflow#245

Merged
tninja merged 5 commits into
tninja:mainfrom
davidwuchn:main
Mar 21, 2026
Merged

Improve ECA Menu UX and Streamline Workflow#245
tninja merged 5 commits into
tninja:mainfrom
davidwuchn:main

Conversation

@davidwuchn
Copy link
Copy Markdown
Contributor

@davidwuchn davidwuchn commented Mar 20, 2026

Summary

This PR improves the ECA menu user experience with clearer descriptions, streamlined workflows, and better differentiation between similar commands.

Changes

1. Upgrade Menu Shows Status Buffer by Default

  • Changed ai-code-eca-upgrade to show status buffer by default (no prefix arg)
  • C-c a u now shows the ECA upgrade status buffer
  • C-u C-c a u upgrades the ECA binary from GitHub releases
  • C-u C-u C-c a u upgrades the ECA Emacs package

2. Distinct Behavior for E vs a

  • C-c a a (Start AI CLI): Quick start - reuses existing session if available
  • C-c a E (Start ECA for project): Always prompts for workspace selection
  • This removes the previous overlap where both commands had similar behavior

3. Clearer Menu Descriptions

Key Before After
E "Start ECA (C-u: pick project)" "Start ECA for project..."
D "Dashboard" "ECA Workspace Dashboard"
A "Add project" "Add project to session"
X "Remove project" "Remove project from session"
F "Share file" "Add file to shared context"
M "Share repo map" "Add repo map to shared context"
B "Add clipboard" "Add clipboard as file context"

4. Streamlined Package Upgrade

  • Removed y-or-n-p prompt from ai-code-eca-upgrade-package
  • Users already explicitly clicked "Upgrade package" button, no need for confirmation

5. Menu Item Reorder

  • Moved B (Add clipboard as file context) above Y (Clear shared) for logical grouping of "add" actions

6. Backend Check in Menu Addition

  • ai-code-eca--add-menu-group now checks if ECA is the selected backend before adding menu items

7. Safe Project Detection

  • Fixed ai-code-eca-create-session-for-workspace to safely handle cases where no project is detected

Files Changed

  • ai-code-eca.el - All UX improvements
  • test/test_ai-code-eca.el - Test fixes

Testing

  • All 26 tests pass (25 passed, 1 skipped for batch mode limitations)

Copilot AI review requested due to automatic review settings March 20, 2026 01:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refines the Emacs Code Assistant (ECA) integration in ai-code-interface.el by improving transient menu UX, making ECA session startup behaviors more distinct, and adjusting upgrade workflows to be more user-directed.

Changes:

  • Updates ECA transient menu labels, key ordering, and backend-specific menu injection behavior.
  • Changes ECA “start for project” flow to always prompt for a workspace root (instead of overloading prefix-arg behavior).
  • Switches the backend :upgrade entry to open the upgrade status UI (ai-code-eca-upgrade-show) and updates tests accordingly.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
ai-code-eca.el Updates ECA menu text/order, gates menu injection on selected backend, revises workspace-session start flow, and streamlines package upgrade confirmation.
ai-code-backends.el Points ECA backend :upgrade to ai-code-eca-upgrade-show.
test/test_ai-code-eca.el Updates menu-group tests for backend gating, layout keys reorder, and batch-mode transient availability.
test/test_ai-code-backends.el Updates backend spec contract test to match new :upgrade function.

Comment thread ai-code-eca.el Outdated
- Upgrade menu now shows status buffer instead of running binary upgrade directly
- Distinct behavior for E vs a: E always prompts for workspace, a reuses existing session
- Clearer menu descriptions for ECA commands
- Removed y-or-n-p confirmation from package upgrade (user already clicked upgrade button)
- Added backend check to ai-code-eca--add-menu-group
- Fixed tests to handle batch mode limitations
@davidwuchn
Copy link
Copy Markdown
Contributor Author

please help review and feed back

@tninja
Copy link
Copy Markdown
Owner

tninja commented Mar 21, 2026

codex-cli reported two high level points to look into.

  1. 高优先级
    位置:PR ai-code-eca.el:1061-1070,主入口在 ai-code.el:438 与 ai-code-
    backends.el:540。
    问题:PR 把 ai-code-eca-upgrade 改成了三态前缀语义,但主菜单 u 实际调用的是 ai-code-upgrade-backend,后者没有接收也没有转发 prefix arg,只是直接
    (funcall upgrade)。这意味着 PR 描述中的 C-c a u / C-u C-c a u / C-u C-u C-c a u 在真实入口上并不成立,用户从主菜单永远只会走“无前缀”分支。
    修复建议:把 ai-code-upgrade-backend 改为 (&optional arg) + (interactive
    "P"),并对函数型 :upgrade 用 let ((current-prefix-arg arg)) (call-
    interactively upgrade) 或等价方案转发前缀参数;同时补一个覆盖三种前缀的集成 测试。
  2. 高优先级
    位置:PR ai-code-eca.el:265-284。对照已安装的 ECA 实现,/home/
    tninja/.emacs.d/elpa/eca-0.8.1/eca.el:319 与 /home/tninja/.emacs.d/elpa/
    eca-0.8.1/eca-util.el:186。
    问题:PR 文档字符串和 Summary 都说 E 会“create or reuse session for that
    workspace”,但代码直接调用 eca-create-session。而 eca-create-session 在 ECA 0.8.1 里只是无条件新建 session,不做查重或复用;真正带复用逻辑的是 eca 命
    令,它先 (or (eca-session) (eca-create-session ...))。所以当前实现会为同一
    workspace 反复创建新 session,和需求不符。
    修复建议:不要直接 eca-create-session。应先按所选 workspace-root 查已有
    session,再复用;或者复用 eca 自身的会话选择逻辑,并在需要时临时绑定 eca-
    find-root-for-buffer-function / 提供显式查找函数。还需要补测试,验证对同一
    workspace 重复执行 E 不会产生重复 session。

Let me know when it is ready to merge.

@tninja
Copy link
Copy Markdown
Owner

tninja commented Mar 21, 2026

@davidwuchn 你的PR summary很漂亮, 这是怎么做的?

Issue #1: ai-code-upgrade-backend was not forwarding prefix arg
- Added optional ARG parameter
- Use call-interactively with current-prefix-arg to forward prefix

Issue #2: ai-code-eca-create-session-for-workspace always created new sessions
- Added ai-code-eca--find-session-by-workspace to find existing sessions
- Reuse existing session if workspace matches instead of creating new
- Fixes duplicate session problem reported by codex-cli

Tests: Added tests for prefix forwarding and session finding
@davidwuchn
Copy link
Copy Markdown
Contributor Author

Fixes Applied

Issue #1: Prefix arg not forwarded ✅

File: ai-code-backends.el:540

Fix: Added &optional arg parameter and forward via call-interactively:

(defun ai-code-upgrade-backend (&optional arg)
  (interactive "P")
  ...
  (let ((current-prefix-arg arg))
    (call-interactively upgrade))
  ...)

Now C-c a u, C-u C-c a u, and C-u C-u C-c a u work correctly.


Issue #2: Session not reused ✅

File: ai-code-eca.el:265

Fix: Added ai-code-eca--find-session-by-workspace helper and check for existing session before creating new:

(defun ai-code-eca--find-session-by-workspace (workspace-root)
  "Find an existing session that has WORKSPACE-ROOT in its workspace folders."
  (when (boundp 'eca--sessions)
    (let ((target (expand-file-name workspace-root)))
      (cl-find-if
       (lambda (session)
         (member target (mapcar #'expand-file-name
                                (eca--session-workspace-folders session))))
       (eca-vals eca--sessions)))))

If a session exists with the selected workspace, it's reused instead of creating a duplicate.


Tests Added

  • ai-code-test-upgrade-backend-forwards-prefix-arg - Verifies prefix forwarding
  • ai-code-test-eca-find-session-by-workspace - Verifies session finding

@tninja tninja merged commit e4f5639 into tninja:main Mar 21, 2026
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants