Unverified 提交 485c8b8d authored 作者: wwwillchen-bot's avatar wwwillchen-bot 提交者: GitHub

fix: update chat history tests for reversed order and improve plan mode tool filtering (#2506)

## Summary - Fix chat_history.spec.ts to expect newest messages at top (reversed display order) - Update plan_mode.spec.ts to use role selector for Base UI Radio component - Skip planning-specific tools when NOT in plan mode - Simplify chatCompletionHandler.ts user message text extraction - Add AGENTS.md learning about Base UI Radio component selection in Playwright ## Test plan - [x] All lint checks pass (`npm run fmt && npm run lint:fix && npm run ts`) - [x] All 669 unit tests pass (`npm test`) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/dyad-sh/dyad/pull/2506"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a> <!-- devin-review-badge-end --> <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Fixes E2E tests to match newest-first chat history and corrects plan-mode radio selection. Also filters planning-only tools outside plan mode and simplifies user message parsing in the fake LLM handler for reliable fixture routing. - **Bug Fixes** - Align chat history E2E with newest-first order and correct selection behavior. - Use role-based selector for Base UI Radio in plan mode E2E; documented in AGENTS.md. - Skip planning-specific tools when not in plan mode. - **Refactors** - Simplify user message extraction in fake LLM chatCompletionHandler and use it for upload-image detection and “exit-plan” routing. <sup>Written for commit 0682b674e189898876f53cb3bbc9fc22a678a5f7. Summary will update on new commits.</sup> <!-- End of auto-generated description by cubic. --> Co-authored-by: 's avatarWill Chen <willchen90@gmail.com> Co-authored-by: 's avatarClaude Opus 4.5 <noreply@anthropic.com>
上级 9d11aec6
......@@ -241,6 +241,18 @@ When running GitHub Actions with `pull_request_target` on cross-repo PRs (from f
- Wrapping `ToggleGroupItem` in `TooltipTrigger` without `render` also breaks `:first-child`/`:last-child` CSS selectors for rounded corners on the group.
- For drag handles and resize rails, prefer the native `title` attribute over `Tooltip` — tooltips appear immediately on hover and interfere with drag interactions, while `title` has a built-in delay.
### Base UI Radio component selection in Playwright
When testing Base UI Radio components in Playwright E2E tests, use `getByRole('radio', { name: 'Label' })` instead of `getByLabel('Label')`. The `getByLabel` selector may not work correctly with Base UI's Radio component structure.
```ts
// Correct: use role selector
await page.getByRole("radio", { name: "React" }).click();
// May not work with Base UI Radio
await page.getByLabel("React").click();
```
### Drizzle migration conflicts during rebase
When rebasing a branch that has drizzle migrations conflicting with upstream (e.g., both have `0023_*.sql`):
......
......@@ -23,32 +23,33 @@ test("should open, navigate, and select from history menu", async ({ po }) => {
const historyMenu = po.page.locator('[data-mentions-menu="true"]');
await expect(historyMenu).toBeVisible();
// Verify menu has items (oldest at top, newest at bottom)
// Verify menu has items (newest at top, oldest at bottom - array is reversed)
const menuItems = po.page.locator('[data-mentions-menu="true"] li');
await expect(menuItems).toHaveCount(2);
await expect(menuItems.nth(0)).toContainText("First test message");
await expect(menuItems.nth(1)).toContainText("Second test message");
await expect(menuItems.nth(0)).toContainText("Second test message");
await expect(menuItems.nth(1)).toContainText("First test message");
// Verify default selection is the last (most recent) item
// Verify default selection is the last visible item (oldest message, at bottom)
// After opening, a synthetic ArrowUp is dispatched which wraps to the bottom item
const lastItem = menuItems.nth(1);
await expect(lastItem).toHaveClass(/bg-accent/, { timeout: 500 });
// Navigate up to first item
// Navigate up to first item (newest message)
await po.page.keyboard.press("ArrowUp");
const firstItem = menuItems.nth(0);
await expect(firstItem).toHaveClass(/bg-accent/);
await expect(firstItem).toContainText("First test message");
await expect(firstItem).toContainText("Second test message");
// Navigate up again to wrap to last item
// Navigate up again to wrap to last item (oldest message)
await po.page.keyboard.press("ArrowUp");
await expect(lastItem).toHaveClass(/bg-accent/);
// Select with Enter
// Select with Enter (selects oldest message)
await po.page.keyboard.press("Enter");
// Menu should close and text should be inserted
await expect(historyMenu).not.toBeVisible({ timeout: Timeout.MEDIUM });
await expect(chatInput).toContainText("Second test message", {
await expect(chatInput).toContainText("First test message", {
timeout: Timeout.MEDIUM,
});
......@@ -57,12 +58,12 @@ test("should open, navigate, and select from history menu", async ({ po }) => {
await po.page.keyboard.press("ArrowUp");
await expect(historyMenu).toBeVisible();
// Click the first item (oldest message)
// Click the first item (newest message, at top)
await menuItems.nth(0).click();
// Verify menu closed and first message was inserted
// Verify menu closed and newest message was inserted
await expect(historyMenu).not.toBeVisible({ timeout: Timeout.MEDIUM });
await expect(chatInput).toContainText("First test message", {
await expect(chatInput).toContainText("Second test message", {
timeout: Timeout.MEDIUM,
});
});
......
......@@ -66,8 +66,8 @@ test("plan mode - questionnaire flow", async ({ po }) => {
timeout: Timeout.MEDIUM,
});
// Select "React" radio option
await po.page.getByLabel("React").click();
// Select "React" radio option (using role selector for base-ui Radio component)
await po.page.getByRole("radio", { name: "React" }).click();
// Click Submit (single question → Submit button shown)
await po.page.getByRole("button", { name: /Submit/ }).click();
......
差异被折叠。
......@@ -338,6 +338,11 @@ export function buildAgentToolSet(
continue;
}
// Skip planning-specific tools when NOT in plan mode
if (!options.planModeOnly && PLANNING_SPECIFIC_TOOLS.has(tool.name)) {
continue;
}
// In read-only mode, skip tools that modify state
if (options.readOnly && tool.modifiesState) {
continue;
......
......@@ -33,45 +33,39 @@ export const createChatCompletionHandler =
.slice()
.reverse()
.find((m: any) => m.role === "user");
// Extract text content from last user message (handles both string and array content)
let userTextContent = "";
if (lastUserMessage) {
// Handle both string content and array content (AI SDK format)
let textContent = "";
if (typeof lastUserMessage.content === "string") {
textContent = lastUserMessage.content;
userTextContent = lastUserMessage.content;
} else if (Array.isArray(lastUserMessage.content)) {
const textPart = lastUserMessage.content.find(
(p: any) => p.type === "text",
);
if (textPart) {
textContent = textPart.text;
userTextContent = textPart.text;
}
}
const localAgentFixture = extractLocalAgentFixture(textContent);
const localAgentFixture = extractLocalAgentFixture(userTextContent);
console.error(
`[local-agent] Checking message: "${textContent.slice(0, 50)}", fixture: ${localAgentFixture}`,
`[local-agent] Checking message: "${userTextContent.slice(0, 50)}", fixture: ${localAgentFixture}`,
);
if (localAgentFixture) {
return handleLocalAgentFixture(req, res, localAgentFixture);
}
// Route plan acceptance message to exit-plan fixture
if (textContent.includes("I accept this plan")) {
if (userTextContent.includes("I accept this plan")) {
return handleLocalAgentFixture(req, res, "exit-plan");
}
}
let messageContent = CANNED_MESSAGE;
if (
lastMessage &&
Array.isArray(lastMessage.content) &&
lastMessage.content.some(
(part: { type: string; text: string }) =>
part.type === "text" &&
part.text.includes("[[UPLOAD_IMAGE_TO_CODEBASE]]"),
)
) {
// Check for upload image to codebase using lastUserMessage (which already handles both string and array content)
if (userTextContent.includes("[[UPLOAD_IMAGE_TO_CODEBASE]]")) {
messageContent = `Uploading image to codebase
<dyad-write path="new/image/file.png" description="Uploaded image to codebase">
DYAD_ATTACHMENT_0
......
Markdown 格式
0%
您添加了 0 到此讨论。请谨慎行事。
请先完成此评论的编辑!
注册 或者 后发表评论