Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR sets up Playwright end-to-end testing infrastructure for the webview application with comprehensive test coverage for key user workflows.
- Adds Playwright configuration and test dependencies
- Implements E2E tests for ingestion flow, chat functionality, and project management
- Updates UI components to support better testability and user experience
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| webview/playwright.config.ts | New Playwright configuration with browser setup and local dev server integration |
| webview/package.json | Adds Playwright and Node.js type dependencies |
| webview/e2e/test-utils.ts | Helper utilities for TRPC response mocking and type definitions |
| webview/e2e/ingestion.spec.ts | E2E tests for repository ingestion workflow and file navigation |
| webview/e2e/chat-drawer.spec.ts | E2E tests for chat drawer messaging with streaming and persistence |
| webview/e2e/add-project.spec.ts | E2E tests for project creation with slug validation and duplicate prevention |
| webview/apps/webapp/src/components/workspace/ChatDrawer.tsx | Removes unused navigation functionality from chat drawer |
| webview/apps/webapp/src/app/workspace/page.tsx | Adds duplicate slug validation and error handling to project creation |
| webview/.gitignore | Adds Playwright-specific ignore patterns |
Files not reviewed (1)
- webview/pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
… setting test skips since a lot of this is time based but running on 4 different web uis is throwing issues.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- webview/pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
webview/package.json:1
- Empty dependencies object should be removed to keep the package.json clean. Since there are no runtime dependencies, this line can be deleted entirely.
{
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- webview/pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- webview/pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
webview/apps/webapp/src/app/workspace/page.tsx:1
- The sync button is being uncommented but lacks proper loading/error state handling. Consider adding error handling for the triggerReingest operation and ensuring the isBusy state properly reflects the async operation status.
/* eslint-disable @next/next/no-img-element */
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| // Types and helpers | ||
|
|
||
| async function loadEnvVarFromFile( |
There was a problem hiding this comment.
ummmm don't think this is right
| const configuredDir = | ||
| (await loadEnvVarFromFile(envLocalPath, "ANALYSIS_DB_DIR")) || ""; | ||
| const dbDir = configuredDir || repoRoot; | ||
| const dbPath = path.join(dbDir, `${slug}.db`); |
There was a problem hiding this comment.
can't we call the analysis API instead? so it lists files and defs which we can verify
| if (cardCount > 0) { | ||
| filesWithDefinitionCards++; | ||
| // Heuristic: a definition summary exists if we can read any non-empty text under a definition heading | ||
| const headingText = (await cardHeadings.first().textContent())?.trim(); |
There was a problem hiding this comment.
is this correct??
we should verify the summary title instead "Summary"
No description provided.