diff --git a/.claude/guidelines/TESTING.md b/.claude/guidelines/TESTING.md new file mode 100644 index 0000000..42368b2 --- /dev/null +++ b/.claude/guidelines/TESTING.md @@ -0,0 +1,471 @@ +# Testing Guidelines + +This document outlines testing philosophy, standards, and best practices for the simple-chat project. + +## Testing Philosophy + +### Test Behavior, Not Implementation + +Focus on what the code **does**, not how it does it. Tests should verify user-facing behavior and outcomes, not internal implementation details. + +**Why:** Implementation details can change without affecting behavior. Tests tied to implementation are brittle and provide false confidence. + +### Tests as Living Documentation + +Tests should read like specifications. A well-written test suite serves as documentation that stays up-to-date with the code. + +**Why:** Clear tests help developers understand expected behavior without reading implementation code. The GIVEN-WHEN-THEN pattern supports this goal. + +### Integration Over Isolation + +Prefer testing integrated components with real dependencies (using MSW for APIs) over heavy mocking. Mock only at architectural boundaries. + +**Why:** Integration tests catch more real bugs and are less brittle. Over-mocking can hide integration issues and creates maintenance burden. + +## Testing Standards + +These are the required tools, patterns, and conventions for this project. + +### Testing Stack + +- **Test Runner:** Vitest +- **React Testing:** @testing-library/react +- **API Mocking:** MSW (Mock Service Worker) +- **User Interactions:** @testing-library/user-event +- **Timers:** Vitest fake timers (`vi.useFakeTimers()`) + +### Required Test Structure: GIVEN-WHEN-THEN + +All tests MUST use the GIVEN-WHEN-THEN pattern with explicit comments. + +**Pattern:** +```typescript +it("should do something when condition occurs", () => { + // GIVEN: Setup initial state and preconditions + const initialState = { ... }; + + // WHEN: Perform the action being tested + const result = performAction(initialState); + + // THEN: Assert the expected outcome + expect(result).toMatchObject({ ... }); +}); +``` + +**Real example:** +```typescript +it("should send message and receive response", async () => { + // GIVEN: MSW mock configured and hook initialized + server.use( + http.post("https://example.com/webhook", () => { + return HttpResponse.json({ output: "Hello from webhook!" }); + }), + ); + + const { result } = renderHook( + () => useChatWebhook({ webhookUrl: "https://example.com/webhook" }), + { wrapper }, + ); + + // WHEN: User sends a message + act(() => { + result.current.sendMessage("Hello"); + }); + + await act(async () => { + await vi.runAllTimersAsync(); + }); + + // THEN: Message is sent and response is received + expect(result.current).toMatchObject({ + messages: [ + { role: Roles.Chat, content: "Hello! How can I help you today?" }, + { role: Roles.User, content: "Hello" }, + { role: Roles.Chat, content: "Hello from webhook!" }, + ], + isLoading: false, + isError: false, + }); +}); +``` + +**Why:** This pattern makes tests self-documenting and follows the Arrange-Act-Assert principle. It clearly separates test setup, execution, and verification. + +### API Mocking Standard: MSW Only + +Always use MSW (Mock Service Worker) to mock HTTP requests. Never mock `fetch` directly. + +**✅ Standard approach:** +```typescript +import { http, HttpResponse } from "msw"; +import { server } from "@/test/mocks/server"; + +server.use( + http.post("https://example.com/api", () => { + return HttpResponse.json({ data: "test" }); + }), +); +``` + +**❌ Never do this:** +```typescript +global.fetch = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ data: "test" }), +}); +``` + +**Why:** MSW intercepts requests at the network level, providing more realistic testing, better error handling, and proper separation of concerns. + +### File Organization Standards + +**Naming conventions:** +- Component tests: `ComponentName.test.tsx` +- Hook tests: `useHookName.test.tsx` +- Utility tests: `utilityName.test.ts` + +**Test structure:** +```typescript +describe("ComponentOrHookName", () => { + // Setup + beforeEach(() => { + // Common setup for all tests + }); + + afterEach(() => { + // Cleanup + }); + + describe("feature group 1", () => { + it("should do something specific", () => { + // Test implementation + }); + }); + + describe("feature group 2", () => { + it("should handle edge case", () => { + // Test implementation + }); + }); +}); +``` + +## Best Practices + +### Assertion Strategy: One Expect Per Logical Entity + +When testing objects or responses, use a single expect statement with `toMatchObject()` instead of multiple separate expects. + +**❌ Avoid:** +```typescript +expect(response.status).toBe(200); +expect(response.body.message).toBe("Success"); +expect(response.body.data).toBeDefined(); +``` + +**✅ Prefer:** +```typescript +expect(response).toMatchObject({ + status: 200, + body: { + message: "Success", + data: expect.anything(), + }, +}); +``` + +**Why:** This makes tests more maintainable, easier to read, and reduces noise when assertions fail. + +### Test Completeness: Cover All States + +Every feature should test: +- ✅ **Initial state** - Before any interaction +- ✅ **Loading state** - During async operations (`isLoading` / pending states) +- ✅ **Success state** - Happy path with valid data +- ✅ **Error state** - Failure scenarios (network errors, invalid responses) +- ✅ **Edge cases** - Empty data, malformed data, boundary conditions + +### Selector Strategy + +**Primary approach:** Use `data-test-id` for reliable test selectors +```typescript + + +// In tests +const button = screen.getByTestId("submit-button"); +``` + +**Secondary approach:** Use role-based selectors when truly semantic +```typescript +const link = screen.getByRole("link", { name: "Home" }); +``` + +**Why:** `data-test-id` provides stable selectors that won't break with UI changes. Role-based selectors are good for accessibility validation but less reliable for component-specific tests. + +### Coverage Goals + +Aim for: +- **Critical hooks/utilities**: 90%+ coverage +- **Components with logic**: 80%+ coverage +- **UI-only components**: 70%+ coverage +- **Overall project**: 80%+ coverage + +Run coverage with: +```bash +pnpm test -- --coverage +``` + +## Technical Reference + +### MSW Setup + +MSW is configured in `src/test/mocks/`: + +- **handlers.ts**: Define default request handlers +- **server.ts**: Setup MSW server instance +- **setup.ts**: Start/stop server for all tests + +#### Adding Request Handlers + +**Global handlers** (in `handlers.ts`): +```typescript +export const handlers = [ + http.post("*/webhook", () => { + return HttpResponse.json({ output: "Default response" }); + }), +]; +``` + +**Test-specific handlers** (override in test file): +```typescript +import { server } from "@/test/mocks/server"; + +server.use( + http.post("https://example.com/webhook", () => { + return HttpResponse.json({ output: "Test-specific response" }); + }), +); +``` + +#### MSW Response Patterns + +**Success response:** +```typescript +http.post("*/api/endpoint", () => { + return HttpResponse.json({ data: "success" }); +}); +``` + +**Error response:** +```typescript +http.post("*/api/endpoint", () => { + return HttpResponse.json({ error: "Failed" }, { status: 500 }); +}); +``` + +**Network error:** +```typescript +http.post("*/api/endpoint", () => { + return HttpResponse.error(); +}); +``` + +**Malformed response:** +```typescript +http.post("*/api/endpoint", () => { + return new HttpResponse("not json", { + headers: { "Content-Type": "application/json" }, + }); +}); +``` + +### Testing Patterns + +#### Testing Hooks with React Query + +```typescript +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { renderHook, act } from "@testing-library/react"; + +describe("useMyHook", () => { + let queryClient: QueryClient; + + beforeEach(() => { + queryClient = new QueryClient({ + defaultOptions: { + queries: { retry: false }, + mutations: { retry: false }, + }, + }); + }); + + const wrapper = ({ children }: { children: ReactNode }) => ( + + {children} + + ); + + it("should fetch data", async () => { + const { result } = renderHook(() => useMyHook(), { wrapper }); + + await act(async () => { + await vi.runAllTimersAsync(); + }); + + expect(result.current.data).toMatchObject({ + // expected data + }); + }); +}); +``` + +#### Testing Components + +```typescript +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; + +describe("MyComponent", () => { + it("should handle user interaction", async () => { + render(); + + const button = screen.getByTestId("action-button"); + await userEvent.click(button); + + expect(screen.getByText("Success")).toBeInTheDocument(); + }); +}); +``` + +#### Testing Async Operations + +```typescript +it("should handle async operation", async () => { + const { result } = renderHook(() => useAsyncHook()); + + act(() => { + result.current.triggerAction(); + }); + + // Wait for async operations + await act(async () => { + await vi.runAllTimersAsync(); + }); + + // Single assertion for the entire result + expect(result.current).toMatchObject({ + isLoading: false, + data: expect.objectContaining({ + value: "expected", + }), + error: null, + }); +}); +``` + +### Technical Tips + +#### Timer Management with Fake Timers + +**Avoid waitFor with fake timers** - it causes timeouts: +```typescript +// ❌ Avoid +await waitFor(() => { + expect(result.current.data).toBeDefined(); +}); + +// ✅ Prefer +await act(async () => { + await vi.runAllTimersAsync(); +}); +expect(result.current.data).toBeDefined(); +``` + +**Advance time strategically:** +```typescript +// Advance by specific amount +await act(async () => { + await vi.advanceTimersByTimeAsync(1000); +}); + +// Run all pending timers +await act(async () => { + await vi.runAllTimersAsync(); +}); +``` + +#### Mock Console Methods When Expected + +If your code intentionally logs errors, mock console methods to avoid polluting test output: + +```typescript +const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + +// Test code that logs errors + +consoleErrorSpy.mockRestore(); +``` + +## Anti-Patterns to Avoid + +### Assertions + +**❌ Don't: Multiple expects for the same entity** +```typescript +expect(user.name).toBe("John"); +expect(user.age).toBe(30); +expect(user.email).toBe("john@example.com"); +``` + +**✅ Do: Single expect with toMatchObject** +```typescript +expect(user).toMatchObject({ + name: "John", + age: 30, + email: "john@example.com", +}); +``` + +### Mocking + +**❌ Don't: Mock fetch globally** +```typescript +global.fetch = vi.fn(); +``` + +**✅ Do: Use MSW** +```typescript +server.use(http.post("*/api", () => HttpResponse.json({}))); +``` + +**❌ Don't: Overly specific mocks** +```typescript +vi.mock("@/components/ComplexComponent", () => ({ + ComplexComponent: () =>
Mocked
, +})); +``` + +**✅ Do: Test integration when possible** +```typescript +// Test the real component with mocked dependencies (MSW, etc.) +render(); +``` + +### Implementation Details + +**❌ Don't: Test internal state** +```typescript +expect(component.state.internalCounter).toBe(5); +``` + +**✅ Do: Test behavior and output** +```typescript +expect(screen.getByText("5 items")).toBeInTheDocument(); +``` + +## Resources + +- [Vitest Documentation](https://vitest.dev/) +- [React Testing Library](https://testing-library.com/react) +- [MSW Documentation](https://mswjs.io/) +- [Testing Library Best Practices](https://kentcdodds.com/blog/common-mistakes-with-react-testing-library) diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000..f485434 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,23 @@ +{ + "permissions": { + "allow": [ + "Bash(pnpm :*)", + "Bash(git :*)", + "Bash(gh :*)", + "Bash(mkdir:*)" + ], + "deny": [ + "Bash(git push --force:*)", + "Bash(git reset --hard:*)", + "Bash(git clean:*)", + "Bash(git branch -D:*)", + "Bash(gh repo delete:*)", + "Bash(gh pr merge:*)", + "Bash(gh release delete:*)", + "Bash(pnpm publish:*)" + ], + "ask": [ + "Bash(git commit)" + ] + } +} diff --git a/.claude/tasks/01-documentation.md b/.claude/tasks/01-documentation.md new file mode 100644 index 0000000..2b95ab0 --- /dev/null +++ b/.claude/tasks/01-documentation.md @@ -0,0 +1,215 @@ +--- +title: Documentation Tasks +category: documentation +priority: low +total_tasks: 7 +estimated_time: 45min +status: pending +created: 2025-11-06 +--- + +# 📚 Documentation Tasks + +Tasks to align documentation with actual implementation. + +--- + +## Task 1: Fix /src/api Architecture Documentation + +**Status:** 🔲 Pending +**Priority:** 🟢 Low +**Time:** ~5 min +**File:** `CLAUDE.md` + +### Problem +- Documentation mentions `/src/api` directory for "API integration and data fetching" +- This directory doesn't exist +- API calls are actually handled in `/src/hooks/useChatWebhook.ts` + +### Action +Remove or update the `/src/api` reference in CLAUDE.md line 40. + +**Option A:** Remove the line +**Option B:** Update to: +```markdown +- `/src/hooks`: Custom React hooks including API integration (useChatWebhook) +``` + +### Files to modify +- `CLAUDE.md` (line 40) + +--- + +## Task 2: Fix biome.json JSX Runtime Configuration + +**Status:** 🔲 Pending +**Priority:** 🟢 Low +**Time:** ~2 min +**File:** `biome.json` + +### Problem +- `biome.json` line 16 sets `"jsxRuntime": "reactClassic"` +- Project uses React 19 with automatic JSX transformation +- Causes inconsistency with tsconfig `"jsx": "react-jsx"` + +### Action +Update `biome.json` line 16: + +```json +"jsxRuntime": "transparent" +``` + +Or remove the setting entirely to use default. + +### Files to modify +- `biome.json` (line 16) + +--- + +## Task 3: Update pnpm Version + +**Status:** 🔲 Pending +**Priority:** 🟢 Low +**Time:** ~1 min +**File:** `CLAUDE.md` + +### Problem +- Documentation states `pnpm v10.6.3` +- Actual version is `pnpm v10.14.0` + +### Action +Update CLAUDE.md line 15: + +```markdown +- **Package Manager**: pnpm v10.14.0 +``` + +### Files to modify +- `CLAUDE.md` (line 15) + +--- + +## Task 4: Document Missing Folders + +**Status:** 🔲 Pending +**Priority:** 🟢 Low +**Time:** ~5 min +**File:** `CLAUDE.md` + +### Problem +Actual project structure has folders not documented: +- `/src/domain` - Contains entities (Message, Roles enum) +- `/src/lib` - Utility functions (utils.ts with `cn()` helper) +- `/src/contexts` - Empty folder + +### Action +Add to CLAUDE.md "Architecture Notes" section: + +```markdown +- Project structure: + - `/src/components`: React components + - `/src/hooks`: Custom React hooks + - `/src/domain`: Domain entities and types + - `/src/lib`: Utility functions and helpers + - `/src/contexts`: React contexts (currently empty) +``` + +### Files to modify +- `CLAUDE.md` (lines 39-41) + +--- + +## Task 5: Update Naming Convention Documentation + +**Status:** 🔲 Pending +**Priority:** 🟢 Low +**Time:** ~3 min +**File:** `CLAUDE.md` + +### Problem +- Documentation says: "Types/Interfaces: PascalCase with prefix (IUser, TConfig)" +- Actual codebase uses: `ChatContainerProps`, `WebhookResponse` (no I/T prefix) +- Uses "Props" suffix pattern instead + +### Action +Update CLAUDE.md line 21: + +```markdown +- **Naming Conventions**: + - Components: PascalCase + - Functions/Variables: camelCase + - Types/Interfaces: PascalCase (Props suffix for component props) + - Example: `ChatContainerProps`, `WebhookResponse` +``` + +### Files to modify +- `CLAUDE.md` (line 21) + +--- + +## Task 6: Document Key Dependencies + +**Status:** 🔲 Pending +**Priority:** 🟢 Low +**Time:** ~10 min +**File:** `CLAUDE.md` + +### Problem +Important dependencies are used but not documented: +- `react-markdown` + `remark-gfm` - Markdown rendering +- `react-syntax-highlighter` - Code block syntax highlighting +- `lucide-react` - Icon library +- `class-variance-authority` - Component variants (used by shadcn) + +### Action +Add new section in CLAUDE.md after "UI Components": + +```markdown +- **Additional Libraries**: + - `react-markdown` + `remark-gfm`: Markdown rendering with GitHub flavored syntax + - `react-syntax-highlighter`: Code block highlighting in chat messages + - `lucide-react`: Icon library for UI elements + - `class-variance-authority`: Component variant utilities +``` + +### Files to modify +- `CLAUDE.md` (add after line 30) + +--- + +## Task 7: Decide on Empty /src/contexts Folder + +**Status:** 🔲 Pending +**Priority:** 🟢 Low +**Time:** ~2 min +**File:** File system + +### Problem +- `/src/contexts` folder exists but is completely empty +- Not documented in CLAUDE.md +- Unclear if it's intentional or leftover + +### Action +**Option A:** Remove the folder if not needed: +```bash +rm -rf src/contexts +``` + +**Option B:** Keep and document intent in CLAUDE.md + +**Option C:** Add a README.md inside explaining it's reserved for future use + +### Files to modify +- Either delete `src/contexts/` or document it + +--- + +## Checklist + +- [ ] Task 1: Fix /src/api documentation +- [ ] Task 2: Fix biome.json jsxRuntime +- [ ] Task 3: Update pnpm version +- [ ] Task 4: Document missing folders +- [ ] Task 5: Update naming conventions +- [ ] Task 6: Document key dependencies +- [ ] Task 7: Handle empty contexts folder diff --git a/.claude/tasks/02-critical-fixes.md b/.claude/tasks/02-critical-fixes.md new file mode 100644 index 0000000..02ade66 --- /dev/null +++ b/.claude/tasks/02-critical-fixes.md @@ -0,0 +1,282 @@ +--- +title: Critical Fixes +category: critical +priority: high +total_tasks: 5 +estimated_time: 30min +status: completed +created: 2025-11-06 +completed: 2025-11-06 +--- + +# 🔴 Critical Fixes + +Urgent issues that should be fixed immediately. + +--- + +## Task 1: Remove Debug Console.log + +**Status:** ✅ Completed +**Priority:** 🔴 Critical +**Time:** ~1 min +**File:** `src/components/chat/ChatMessage/AssistantMessage.tsx` + +### Problem +Line 34 contains a debug `console.log(style);` that was left in production code. + +### Location +```typescript +// src/components/chat/ChatMessage/AssistantMessage.tsx:34 +console.log(style); +``` + +### Action +Remove the entire line. + +### Why Critical? +- Pollutes browser console in production +- Exposes internal variable names +- Performance impact (minimal but unnecessary) + +### Files to modify +- `src/components/chat/ChatMessage/AssistantMessage.tsx` (line 34) + +--- + +## Task 2: Remove Duplicate Scroll Effect + +**Status:** ✅ Completed +**Priority:** 🔴 Critical +**Time:** ~5 min +**File:** `src/components/chat/ChatContainer.tsx` + +### Problem +Two `useEffect` hooks doing the same scroll-to-bottom action: +- Lines 34-42: Has empty dependency array (never fires) +- Lines 44-49: Actually works with proper dependencies + +The first one is redundant and confusing. + +### Location +```typescript +// Lines 34-42 - REMOVE THIS +useEffect(() => { + if (scrollAreaRef.current) { + const viewport = scrollAreaRef.current.querySelector( + "[data-radix-scroll-area-viewport]", + ); + if (viewport) { + viewport.scrollTop = viewport.scrollHeight; + } + } +}, []); // Empty deps - never runs after initial mount +``` + +### Action +Delete lines 34-42 entirely. Keep only the second useEffect (lines 44-49). + +### Why Critical? +- Code duplication +- First effect with empty deps is misleading +- Makes code harder to maintain +- Could cause bugs if someone tries to "fix" it + +### Files to modify +- `src/components/chat/ChatContainer.tsx` (lines 34-42) + +--- + +## Task 3: Add Error Handling to Fetch + +**Status:** ✅ Completed +**Priority:** 🔴 Critical +**Time:** ~10 min +**File:** `src/hooks/useChatWebhook.ts` + +### Problem +The `fetch` call has no try/catch block: +- Network errors not caught +- JSON parsing errors not handled +- No timeout handling +- Response validation missing + +### Location +```typescript +// Lines 44-56 - Current implementation +const response = await fetch(webhookUrl, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), +}); + +const data: WebhookResponse = await response.json(); +``` + +### Action +Wrap in try/catch with proper error handling: + +```typescript +try { + const response = await fetch(webhookUrl, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + }); + + if (!response.ok) { + throw new Error(`HTTP ${response.status}: ${response.statusText}`); + } + + const data: WebhookResponse = await response.json(); + + // Validate response structure + if (!data || typeof data.output !== 'string') { + throw new Error('Invalid webhook response format'); + } + + return data; +} catch (error) { + console.error('Webhook error:', error); + throw new Error( + error instanceof Error + ? error.message + : 'Failed to communicate with webhook' + ); +} +``` + +### Why Critical? +- App crashes on network errors +- Poor user experience with vague errors +- No validation of API response +- Security risk (malformed responses) + +### Files to modify +- `src/hooks/useChatWebhook.ts` (lines 44-56) + +--- + +## Task 4: Add Aria-label to Textarea + +**Status:** ✅ Completed +**Priority:** 🔴 Critical (Accessibility) +**Time:** ~2 min +**File:** `src/components/chat/ChatInput.tsx` + +### Problem +Textarea lacks `aria-label` for screen readers. Only placeholder text exists, which is not read by all assistive technologies. + +### Location +```typescript +// Current - no aria-label +