fix: relax ECA layout test to not assume parent-menu key ownership#362
Merged
Conversation
The test ai-code-test-eca-menu-group-appears-in-layout incorrectly asserted that key "A" maps to ai-code-eca-add-workspace-folder in both parent transient layouts. PR #361 added ("A" "Derive Architecture Guardrails" ai-code-derive-architecture-guardrails) to ai-code--menu-other-tools, creating a collision. Changes: - Add canonical check of all ECA bindings against ai-code-eca-menu - Remove "A" from parent-layout command assertions (known collision) - Add weak (should (transient-get-suffix prefix "A")) to document the collision without dictating what "A" maps to - Keep "F" special-case handling unchanged Agent-Logs-Url: https://github.com/tninja/ai-code-interface.el/sessions/1882eb4f-2064-491d-9972-b79610269ee4 Co-authored-by: tninja <714625+tninja@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix failing test in ECA transient menu layout
fix: relax ECA layout test to not assume parent-menu key ownership
May 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ai-code-test-eca-menu-group-appears-in-layoutasserted that"A"maps toai-code-eca-add-workspace-folderin both parent transient layouts, but that key is already claimed byai-code-derive-architecture-guardrailsinai-code--menu-other-tools, causing the assertion to fail.Changes
E W D A X F M B Y) are now checked againstai-code-eca-menu, which is the authoritative source of truth for the complete ECA command set."A"from parent-layout command assertions — replaced with a weaker(should (transient-get-suffix prefix "A"))that only documents the collision point without dictating which command wins."F"special-case preserved — the existing split behavior (ai-code--infix-toggle-auto-follow-upinai-code-menu-default,ai-code-eca-share-file-contextinai-code-menu-2-columnsandai-code-eca-menu) is kept as-is.Original prompt
The unit test job is failing in repository
tninja/ai-code-interface.elfor PR #361.Please create a pull request that fixes the failing test in the ECA transient menu layout tests.
Context:
.github/workflows/unit-tests.ymlai-code-test-eca-menu-group-appears-in-layout"A"to map toai-code-eca-add-workspace-folderai-code-derive-architecture-guardrails2dc0081fd6de843cc123be16d4bfb9e238878d02Relevant files:
test/test_ai-code-eca.elai-code-eca.el.github/workflows/unit-tests.ymlWhat to change:
ai-code-eca-menuas the canonical source of truth for the complete ECA command set."F"key if needed.Success criteria:
Please open a PR with the fix.
The following is the prior conversation context from the user's chat exploration (may be truncated):
User: Please find a solution for this failing job. Use the logs, job definition, and any referenced files where the failure occurred. Keep your response focused on the solution and include code suggestions when appropriate.
Assistant: The failure is a test/code mismatch in the ECA transient menu layout, not a workflow problem.
Root cause
The failing test is
ai-code-test-eca-menu-group-appears-in-layout.The assertion failure shows:
"A"→ai-code-eca-add-workspace-folder"A"→ai-code-derive-architecture-guardrailsFrom the job log:
ai-code-test-eca-menu-group-appears-in-layout(eq ai-code-derive-architecture-guardrails ai-code-eca-add-workspace-folder)So the test assumes the ECA group is directly visible at top-level in both layouts, but in the actual loaded transient layout,
"A"is already occupied by another command.Relevant code
The ECA menu is injected here:
ai-code-eca.el#L134-L157It appends this group to both menu layouts:
The test is too strict because it looks up single-key suffixes on the full parent transient:
test/test_ai-code-eca.el#L87-L110That breaks as soon as the main menu already uses one of those keys.
Solution
Update the test so it verifies the dedicated ECA submenu layout, and only checks parent layouts for keys that are intentionally exposed there and not shadowed.
Recommended test change
Replace the broad top-level assertions with:
ai-code-eca-menu"F"if that is intentionalExample patch for
test/test_ai-code-eca.el: