Skip to content

Remove ChatActivity component and associated references #2648

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wwwillchen merged 2 commits into from
Feb 12, 2026
Merged

Remove ChatActivity component and associated references #2648

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b005943
Remove ChatActivity component and associated references
wwwillchen Feb 12, 2026
459d583
fix
wwwillchen Feb 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions e2e-tests/concurrent_chat.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,17 @@ test("concurrent chat", async ({ po }) => {
await po.navigation.goToAppsTab();
await po.sendPrompt("tc=chat2");
await po.snapshotMessages();
await po.chatActions.clickChatActivityButton();

// Chat #1 will be the last in the list
expect(
await po.page.getByTestId(`chat-activity-list-item-1`).textContent(),
).toContain("Chat #1");
await po.page.getByTestId(`chat-activity-list-item-1`).click();
await po.snapshotMessages({ timeout: 12_000 });
// Chat #1 tab should be visible in the chat tabs with an "in progress" indicator
// Find the tab that contains the "Chat in progress" indicator and click it
const chat1TabContainer = po.page
.locator('[aria-label="Chat in progress"]')
.locator(
"xpath=ancestor::div[contains(@class, 'flex') and contains(@class, 'h-10')]",
Comment on lines +18 to +21

@greptile-apps greptile-apps Bot Feb 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ambiguous tab selection

This locator selects the first [aria-label="Chat in progress"] on the page, then clicks the first button in its ancestor tab. If chat #2 is also still streaming at this point (timing-dependent), this can click the wrong tab and make the test flaky (snapshotting chat #2 instead of chat #1). Consider scoping the selection to chat #1’s tab (e.g., by title/text like tc=chat1 / “Chat #1”, or a stable test id on the tab) rather than relying on a global "in progress" indicator + .first().

Prompt To Fix With AI 
This is a comment left during a code review.
Path: e2e-tests/concurrent_chat.spec.ts
Line: 18:21

Comment:
**Ambiguous tab selection**

This locator selects the *first* `[aria-label="Chat in progress"]` on the page, then clicks the first `button` in its ancestor tab. If chat #2 is also still streaming at this point (timing-dependent), this can click the wrong tab and make the test flaky (snapshotting chat #2 instead of chat #1). Consider scoping the selection to chat #1’s tab (e.g., by title/text like `tc=chat1` / “Chat #1”, or a stable test id on the tab) rather than relying on a global "in progress" indicator + `.first()`.

How can I resolve this? If you propose a fix, please make it concise.

);

@github-actions github-actions Bot Feb 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 MEDIUM | test-brittleness | Found by: Correctness & Code Health, Endorsed by: UX

Brittle XPath selector coupled to Tailwind CSS classes

This XPath expression ancestor::div[contains(@class, 'flex') and contains(@class, 'h-10')] is tightly coupled to Tailwind utility classes. If the tab container's styling changes (e.g., h-10 → h-12, or classes are reorganized), this locator will silently fail to match the correct element, causing a flaky or broken test.

💡 Suggestion: Add a data-testid attribute to the chat tab container and use getByTestId() instead of XPath class matching. This would be consistent with how other test selectors in the codebase work (e.g., chat-activity-list-item-* in the old code).

await expect(chat1TabContainer).toBeVisible();

//
// Click the button inside the tab to select it
await chat1TabContainer.locator("button").first().click();
await po.snapshotMessages({ timeout: 12_000 });
});
10 changes: 0 additions & 10 deletions e2e-tests/helpers/page-objects/components/ChatActions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,16 +115,6 @@ export class ChatActions {
await this.selectChatMode("local-agent");
}

async clickChatActivityButton() {
await this.page.getByTestId("chat-activity-button").click();
}

async snapshotChatActivityList() {
await expect(
this.page.getByTestId("chat-activity-list"),
).toMatchAriaSnapshot();
}

async snapshotChatInputContainer() {
await expect(this.getChatInputContainer()).toMatchAriaSnapshot();
}
Expand Down
Loading
Loading