From c4db66afddc7d2a10477868076540b9b363317cd Mon Sep 17 00:00:00 2001 From: DragonnZhang <731557579@qq.com> Date: Sun, 26 Apr 2026 02:18:55 +0800 Subject: [PATCH] fix(desktop): bound dense assistant file reference chips --- .../assistant-file-reference-overflow.md | 55 ++++++++++ ...de-electron-desktop-implementation-plan.md | 101 ++++++++++++++++++ packages/desktop/scripts/e2e-cdp-smoke.mjs | 88 ++++++++++++++- .../src/main/acp/createE2eAcpClient.ts | 7 +- .../renderer/components/layout/ChatThread.tsx | 38 +++++-- .../components/layout/WorkspacePage.test.tsx | 64 +++++++++++ packages/desktop/src/renderer/styles.css | 20 +++- 7 files changed, 358 insertions(+), 15 deletions(-) create mode 100644 .qwen/e2e-tests/electron-desktop/assistant-file-reference-overflow.md diff --git a/.qwen/e2e-tests/electron-desktop/assistant-file-reference-overflow.md b/.qwen/e2e-tests/electron-desktop/assistant-file-reference-overflow.md new file mode 100644 index 000000000..ae2ba9092 --- /dev/null +++ b/.qwen/e2e-tests/electron-desktop/assistant-file-reference-overflow.md @@ -0,0 +1,55 @@ +# Assistant File Reference Overflow + +- Slice date: 2026-04-26 +- Executable harness: `packages/desktop/scripts/e2e-cdp-smoke.mjs` +- Command: + `cd packages/desktop && npm run e2e:cdp` +- Result: pass +- Artifact directory: + `.qwen/e2e-tests/electron-desktop/artifacts/2026-04-25T18-17-10-902Z/` + +## Scenario + +1. Launch the real Electron app with isolated HOME, runtime, user-data, and a + fake dirty Git workspace. +2. Open the fake project through the desktop directory picker path. +3. Send the first composer prompt and approve the fake command request. +4. Wait for the fake ACP assistant response with repeated dense file + references. +5. Assert the assistant message renders deduped chips, `:line:column` labels, + uncommon file references, and a compact overflow indicator. +6. Continue the existing copy, retry, changed-files, review, settings, + terminal, and final layout smoke path. + +## Assertions + +- `README.md:1` appears as one file chip even though the assistant prose + repeats it. +- `packages/desktop/src/renderer/App.tsx:12:5`, `.env.example`, + `Dockerfile`, `docs/guide.mdx`, and `src/App.vue` appear as accessible file + chips. +- The overflow indicator shows `+2 more` with the accessible label + `2 more file references`. +- File chips stay inside the assistant message and timeline, remain under the + maximum chip width, and do not create horizontal document overflow. +- The assistant action row still exposes `Copy Response`, `Retry Last Prompt`, + and `Open Changes`. +- Console errors: 0. +- Failed local network requests: 0. + +## Artifacts + +- `assistant-message-actions.json` +- `assistant-message-actions.png` +- `assistant-retry-draft.json` +- `resolved-tool-activity.json` +- `conversation-changes-summary.json` +- `completed-workspace.png` +- `electron.log` +- `summary.json` + +## Known Uncovered Risk + +This harness verifies the default 1240 px Electron window. A follow-up compact +viewport pass should assert the same dense message state near the lower +supported desktop width. diff --git a/design/qwen-code-electron-desktop-implementation-plan.md b/design/qwen-code-electron-desktop-implementation-plan.md index 74119e611..63e7058f6 100644 --- a/design/qwen-code-electron-desktop-implementation-plan.md +++ b/design/qwen-code-electron-desktop-implementation-plan.md @@ -22,6 +22,107 @@ execution order, verification, decisions, and remaining work. ## Codex Alignment Progress +### Completed Slice: Dense Assistant File Reference Overflow + +Status: completed in iteration 12. + +Goal: harden assistant prose file-reference rendering for realistic, dense +responses with repeated references, line/column suffixes, uncommon source file +extensions, and more references than can comfortably fit in the message card. + +User-visible value: assistant responses stay compact and readable in the +conversation-first workbench while still exposing useful file chips for opening +referenced files. Repeated paths do not add visual noise, and overflow is +explicit instead of silently dropping references. + +Expected files: + +- `packages/desktop/src/renderer/components/layout/ChatThread.tsx` +- `packages/desktop/src/renderer/components/layout/WorkspacePage.test.tsx` +- `packages/desktop/src/renderer/styles.css` +- `packages/desktop/src/main/acp/createE2eAcpClient.ts` +- `packages/desktop/scripts/e2e-cdp-smoke.mjs` +- `.qwen/e2e-tests/electron-desktop/assistant-file-reference-overflow.md` +- `design/qwen-code-electron-desktop-implementation-plan.md` + +Acceptance criteria: + +- Assistant prose deduplicates repeated file references while preserving the + first visible label. +- References with `:line:column` suffixes open the file path without the line + suffix and keep the visible line/column label. +- Common desktop/code references such as `.mdx`, `.mts`, `.cts`, `.vue`, + `.svelte`, `.astro`, `Dockerfile`, `Makefile`, `.env`, `.gitignore`, and + `.npmrc` can render as chips when they appear in assistant prose. +- More than six references render the first six chips plus a compact overflow + indicator with an accessible label. +- Long chips wrap/truncate within the assistant message at normal and compact + widths without horizontal page overflow or composer overlap. + +Verification: + +- Unit/component test command: + `cd packages/desktop && SHELL=/bin/bash npx vitest run src/renderer/components/layout/WorkspacePage.test.tsx` +- Build/typecheck/lint commands: + `cd packages/desktop && npm run typecheck && npm run lint && npm run build` +- Real Electron harness: + `cd packages/desktop && npm run e2e:cdp` +- Harness path: `packages/desktop/scripts/e2e-cdp-smoke.mjs` +- E2E scenario steps: launch real Electron with isolated HOME/runtime/user-data + and fake ACP, open the fake Git project, send a prompt, approve the fake + command request, wait for the dense assistant response, assert deduped chips, + line/column chips, overflow count, and contained chip geometry, then continue + the existing copy/retry/review/settings/terminal smoke path. +- E2E assertions: assistant file chips include `README.md:1`, + `packages/desktop/src/renderer/App.tsx:12:5`, `.env.example`, + `Dockerfile`, and an overflow indicator; duplicate `README.md:1` references + render once; every chip stays inside the assistant message/timeline; document + scroll width does not exceed the viewport; console errors/failed local + requests are absent. +- Diagnostic artifacts: CDP screenshots, dense assistant reference JSON, + assistant action JSON, Electron log, summary JSON under + `.qwen/e2e-tests/electron-desktop/artifacts/`. +- Required skills applied: `frontend-design` for prototype-constrained compact + chip density and overflow treatment; `electron-desktop-dev` for renderer + changes and real Electron CDP verification; `brainstorming` applied by + choosing the smallest continuation of the recorded rich-conversation backlog + from repo artifacts and `home.jpg` without pausing the autonomous loop. + +Notes and decisions: + +- The prototype shows file/change context inline with the conversation, so this + slice keeps file chips inside assistant messages rather than moving dense + references into a separate drawer. +- Overflow uses a quiet text chip so the message remains readable and does not + become a file browser. +- The fake ACP response includes deterministic dense references so the CDP + harness can verify real Electron layout and dedupe behavior. +- The first focused component test exposed a line/column stripping bug where + `path.ts:12:5` opened `path.ts:12`. The final implementation strips the + full `:line:column` suffix for open-file callbacks while preserving the + visible chip label. + +Verification results: + +- `cd packages/desktop && SHELL=/bin/bash npx vitest run src/renderer/components/layout/WorkspacePage.test.tsx` + passed with 10 tests. +- `cd packages/desktop && npm run typecheck` passed. +- `cd packages/desktop && npm run lint` passed. +- `cd packages/desktop && npm run build` passed. +- `cd packages/desktop && npm run e2e:cdp` passed after launch through real + Electron over CDP. +- Passing artifacts: + `.qwen/e2e-tests/electron-desktop/artifacts/2026-04-25T18-17-10-902Z/`. + +Next work: + +- Add a compact-viewport CDP pass or Browser bounds control for the dense + conversation state so long assistant/file chips are also asserted near the + lower supported desktop width. +- Continue rich conversation fidelity by adding clearer assistant action + feedback for clipboard/open-file failures and by keeping multiple assistant + messages dense at compact widths. + ### Completed Slice: Assistant Message Actions and File Reference Chips Status: completed in iteration 11. diff --git a/packages/desktop/scripts/e2e-cdp-smoke.mjs b/packages/desktop/scripts/e2e-cdp-smoke.mjs index 5203e0391..8bbb3f30a 100644 --- a/packages/desktop/scripts/e2e-cdp-smoke.mjs +++ b/packages/desktop/scripts/e2e-cdp-smoke.mjs @@ -927,10 +927,25 @@ async function assertAssistantMessageActions(fileName) { (button) => button.getAttribute('aria-label') || '' ) : [], + overflowText: + fileReferences?.querySelector('.message-file-reference-overflow') + ?.innerText ?? '', + overflowLabel: + fileReferences?.querySelector('.message-file-reference-overflow') + ?.getAttribute('aria-label') ?? '', + chipRects: fileReferences + ? [ + ...fileReferences.querySelectorAll( + 'button, .message-file-reference-overflow' + ) + ].map((chip) => rectFor(chip)) + : [], messageRect: rectFor(message), actionsRect: rectFor(actions), timelineRect: rectFor(timeline), - composerRect: rectFor(composer) + composerRect: rectFor(composer), + viewportWidth: window.innerWidth, + documentScrollWidth: document.documentElement.scrollWidth }; })()`); @@ -968,6 +983,45 @@ async function assertAssistantMessageActions(fileName) { ); } + for (const expectedLabel of [ + 'Open packages/desktop/src/renderer/App.tsx:12:5', + 'Open .env.example', + 'Open Dockerfile', + 'Open docs/guide.mdx', + 'Open src/App.vue', + ]) { + if (!snapshot.fileReferenceLabels.includes(expectedLabel)) { + throw new Error( + `Dense assistant file chips missing ${expectedLabel}: ${snapshot.fileReferenceLabels.join( + ', ', + )}`, + ); + } + } + + const readmeChipCount = snapshot.fileReferenceLabels.filter( + (label) => label === 'Open README.md:1', + ).length; + if (readmeChipCount !== 1) { + throw new Error( + `Repeated README.md:1 references should dedupe to one chip: ${snapshot.fileReferenceLabels.join( + ', ', + )}`, + ); + } + + if ( + snapshot.overflowText !== '+2 more' || + snapshot.overflowLabel !== '2 more file references' + ) { + throw new Error( + `Dense assistant file overflow is missing: ${JSON.stringify({ + overflowText: snapshot.overflowText, + overflowLabel: snapshot.overflowLabel, + })}`, + ); + } + for (const internalText of ['e2e-terminal-check', 'session-e2e']) { if (snapshot.messageText.includes(internalText)) { throw new Error( @@ -1005,6 +1059,38 @@ async function assertAssistantMessageActions(fileName) { if (snapshot.messageRect.bottom > snapshot.composerRect.top) { throw new Error('Assistant message overlaps the composer.'); } + + if (snapshot.documentScrollWidth > snapshot.viewportWidth + 4) { + throw new Error( + `Assistant file chips caused horizontal page overflow: ${JSON.stringify({ + documentScrollWidth: snapshot.documentScrollWidth, + viewportWidth: snapshot.viewportWidth, + })}`, + ); + } + + for (const chipRect of snapshot.chipRects) { + if (!chipRect) { + throw new Error('Assistant file chip geometry is missing.'); + } + + if (chipRect.width > 282) { + throw new Error( + `Assistant file chip is too wide: ${JSON.stringify(chipRect)}`, + ); + } + + if ( + chipRect.left < snapshot.messageRect.left || + chipRect.right > snapshot.messageRect.right + 1 || + chipRect.left < snapshot.timelineRect.left || + chipRect.right > snapshot.timelineRect.right + 1 + ) { + throw new Error( + `Assistant file chip escaped the message: ${JSON.stringify(chipRect)}`, + ); + } + } } async function assertRetryDrafted(fileName) { diff --git a/packages/desktop/src/main/acp/createE2eAcpClient.ts b/packages/desktop/src/main/acp/createE2eAcpClient.ts index 255757fec..a7196fb77 100644 --- a/packages/desktop/src/main/acp/createE2eAcpClient.ts +++ b/packages/desktop/src/main/acp/createE2eAcpClient.ts @@ -179,7 +179,12 @@ export class E2eAcpClient implements AcpSessionClient { sessionUpdate: 'agent_message_chunk', content: { type: 'text', - text: `E2E fake ACP response received: ${prompt}\n\nUpdated README.md:1 for review.`, + text: + `E2E fake ACP response received: ${prompt}\n\n` + + 'Updated README.md:1 for review. Related references: ' + + 'README.md:1, packages/desktop/src/renderer/App.tsx:12:5, ' + + '.env.example, Dockerfile, docs/guide.mdx, src/App.vue, ' + + 'Makefile, and config/settings.mts.', }, }); diff --git a/packages/desktop/src/renderer/components/layout/ChatThread.tsx b/packages/desktop/src/renderer/components/layout/ChatThread.tsx index 90214e1ec..e7a558b5f 100644 --- a/packages/desktop/src/renderer/components/layout/ChatThread.tsx +++ b/packages/desktop/src/renderer/components/layout/ChatThread.tsx @@ -340,8 +340,11 @@ function TimelineItem({ previousUserMessage: string | null; }) { if (item.type === 'message') { - const fileReferences = - item.role === 'assistant' ? extractFileReferences(item.text) : []; + const fileReferenceSet = + item.role === 'assistant' + ? extractFileReferences(item.text) + : { references: [], hiddenCount: 0 }; + const fileReferences = fileReferenceSet.references; const hasChangedFiles = Boolean(gitDiff?.files.length); return ( @@ -371,6 +374,16 @@ function TimelineItem({ ))} + {fileReferenceSet.hiddenCount > 0 ? ( +
  • + + +{fileReferenceSet.hiddenCount} more + +
  • + ) : null} ) : null} {item.role === 'assistant' ? ( @@ -472,12 +485,15 @@ function AssistantMessageActions({ ); } -const FILE_REFERENCE_PATTERN = - /(?:^|[\s([{"'`])((?:[\w@.-]+\/)*[\w@.-]+\.(?:bash|c|cc|cjs|cpp|css|go|h|hpp|html|java|js|jsx|json|kt|lock|md|mjs|py|rs|scss|sh|sql|swift|toml|ts|tsx|txt|xml|ya?ml|zsh)(?::\d+)?)(?=$|[\s)\]},.;:"'`])/giu; +const MAX_VISIBLE_FILE_REFERENCES = 6; -function extractFileReferences( - text: string, -): Array<{ label: string; path: string }> { +const FILE_REFERENCE_PATTERN = + /(?:^|[\s([{"'`])((?:[\w@.-]+\/)*(?:(?:[\w@.-]+\.(?:astro|bash|c|cc|cjs|cpp|css|cts|go|gradle|h|hpp|html|java|js|jsx|json|kt|lock|md|mdx|mjs|mts|py|rs|scss|sh|sql|svelte|swift|toml|ts|tsx|txt|vue|xml|ya?ml|zsh))|(?:Dockerfile|Makefile)(?:\.[\w.-]+)?|(?:\.(?:env(?:\.[\w.-]+)?|gitignore|npmrc)))(?::\d+(?::\d+)?)?)(?=$|[\s)\]},.;:"'`])/giu; + +function extractFileReferences(text: string): { + references: Array<{ label: string; path: string }>; + hiddenCount: number; +} { const references: Array<{ label: string; path: string }> = []; const seen = new Set(); @@ -494,12 +510,14 @@ function extractFileReferences( }); } - return references.slice(0, 6); + return { + references: references.slice(0, MAX_VISIBLE_FILE_REFERENCES), + hiddenCount: Math.max(0, references.length - MAX_VISIBLE_FILE_REFERENCES), + }; } function stripFileReferenceLine(reference: string): string { - const lineMatch = /^(.*):\d+$/u.exec(reference); - return lineMatch?.[1] ?? reference; + return reference.replace(/:\d+(?::\d+)?$/u, ''); } function ToolActivityCard({ diff --git a/packages/desktop/src/renderer/components/layout/WorkspacePage.test.tsx b/packages/desktop/src/renderer/components/layout/WorkspacePage.test.tsx index 53c3691f0..0c838d002 100644 --- a/packages/desktop/src/renderer/components/layout/WorkspacePage.test.tsx +++ b/packages/desktop/src/renderer/components/layout/WorkspacePage.test.tsx @@ -482,6 +482,70 @@ describe('WorkspacePage', () => { ).toBeTruthy(); }); + it('deduplicates and bounds dense assistant file reference chips', () => { + const onOpenFileReference = vi.fn(); + let chatState = chatReducer(createInitialChatState(), { + type: 'append_user_message', + content: 'List the touched files', + }); + chatState = chatReducer(chatState, { + type: 'server_message', + message: { + type: 'message_delta', + role: 'assistant', + text: + 'Touched README.md:1, README.md:1 again, ' + + 'packages/desktop/src/renderer/App.tsx:12:5, .env.example, ' + + 'Dockerfile, docs/guide.mdx, src/App.vue, Makefile, ' + + 'and config/settings.mts.', + }, + }); + chatState = chatReducer(chatState, { + type: 'server_message', + message: { type: 'message_complete' }, + }); + + const renderedContainer = renderWorkspace({ + chatState, + onOpenFileReference, + }); + const fileReferences = renderedContainer.querySelector( + '[data-testid="assistant-file-references"]', + ); + const labels = [...(fileReferences?.querySelectorAll('button') ?? [])].map( + (button) => button.getAttribute('aria-label'), + ); + const overflow = fileReferences?.querySelector( + '.message-file-reference-overflow', + ); + + expect(labels).toEqual([ + 'Open README.md:1', + 'Open packages/desktop/src/renderer/App.tsx:12:5', + 'Open .env.example', + 'Open Dockerfile', + 'Open docs/guide.mdx', + 'Open src/App.vue', + ]); + expect(labels.filter((label) => label === 'Open README.md:1')).toHaveLength( + 1, + ); + expect(overflow?.textContent).toBe('+2 more'); + expect(overflow?.getAttribute('aria-label')).toBe('2 more file references'); + + act(() => { + ( + fileReferences?.querySelector( + 'button[aria-label="Open packages/desktop/src/renderer/App.tsx:12:5"]', + ) as HTMLButtonElement + ).click(); + }); + + expect(onOpenFileReference).toHaveBeenCalledWith( + 'packages/desktop/src/renderer/App.tsx', + ); + }); + it('routes terminal output through an attach action', () => { const onAttachTerminalOutput = vi.fn(); const renderedContainer = renderWorkspace({ diff --git a/packages/desktop/src/renderer/styles.css b/packages/desktop/src/renderer/styles.css index 487de7660..1f87ad66e 100644 --- a/packages/desktop/src/renderer/styles.css +++ b/packages/desktop/src/renderer/styles.css @@ -732,16 +732,25 @@ summary:focus-visible { list-style: none; } +.message-file-references li { + min-width: 0; + max-width: 100%; +} + .message-file-references button, -.message-action-button { +.message-action-button, +.message-file-reference-overflow { border: 1px solid var(--line); background: rgba(238, 244, 239, 0.035); color: var(--text-soft); } -.message-file-references button { - max-width: 280px; +.message-file-references button, +.message-file-reference-overflow { + display: inline-flex; + max-width: min(280px, 100%); min-height: 24px; + align-items: center; overflow: hidden; padding: 3px 8px; border-radius: 999px; @@ -752,6 +761,10 @@ summary:focus-visible { white-space: nowrap; } +.message-file-reference-overflow { + color: var(--muted); +} + .message-file-references button:not(:disabled):hover, .message-action-button:not(:disabled):hover { border-color: rgba(85, 166, 255, 0.32); @@ -762,6 +775,7 @@ summary:focus-visible { .message-action-row { display: flex; align-items: center; + flex-wrap: wrap; gap: 6px; min-height: 28px; }