Skip to content

Comments

Add prompt fixtures and stream helpers for tests#20

Open
utmishra wants to merge 1 commit intofeat/web-searchfrom
codex/add-fixture-files-for-web-tests
Open

Add prompt fixtures and stream helpers for tests#20
utmishra wants to merge 1 commit intofeat/web-searchfrom
codex/add-fixture-files-for-web-tests

Conversation

@utmishra
Copy link
Owner

Summary

  • add prompt fixtures and streaming utilities for shared test data
  • serve deterministic stream in /api/chat when TEST_ROUTES is set
  • verify reasoning and tool calls in route and UI tests

Testing

  • pnpm test:routes
  • pnpm test:e2e (fails: Executable doesn't exist at ... please run playwright install)

https://chatgpt.com/codex/tasks/task_e_68a065926fb083209ad6fa89212fd6ae

@sonarqubecloud
Copy link

@utmishra utmishra requested a review from Copilot August 16, 2025 12:16
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds test infrastructure for chat functionality by introducing prompt fixtures and streaming utilities for deterministic testing. It replaces OpenAI API-dependent tests with deterministic test data and adds comprehensive verification of reasoning and tool calls.

  • Introduces reusable streaming utilities and prompt fixtures for consistent test data
  • Updates route tests to verify structured assistant responses including reasoning and tool calls
  • Adds end-to-end tests for UI interactions with assistant messages and reasoning details

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
web/tests/routes/chat.spec.ts Replaces OpenAI-dependent test with deterministic stream parsing and verification
web/tests/prompts/utils.ts Adds utilities for building and parsing streaming response data
web/tests/prompts/simple.ts Defines test fixtures for simple chat interactions with reasoning and tool calls
web/tests/e2e/chat.spec.ts Adds UI tests for assistant messages, reasoning details, and tool output
web/src/app/api/chat/route.ts Updates test route to use new streaming fixtures instead of generic prompt
web/playwright.config.ts Configures test environment variables for deterministic behavior

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.


expect(
chunks.some(
(c: any) =>
Copy link

Copilot AI Aug 16, 2025

Choose a reason for hiding this comment

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

Using 'any' type defeats the purpose of TypeScript's type safety. Consider using the StreamChunk type or creating a more specific type for chunk objects.

Suggested change
(c: any) =>
.filter((c: StreamChunk) => c.type === 'text')
.map((c: StreamChunk) => c.text)
.join('');
expect(text).toBe(simpleAnswerText);
expect(
chunks.some(
(c: StreamChunk) => c.type === 'reasoning' && c.text === simpleReasoning,
),
).toBeTruthy();
expect(
chunks.some(
(c: StreamChunk) =>

Copilot uses AI. Check for mistakes.

expect(
chunks.some(
(c: any) =>
Copy link

Copilot AI Aug 16, 2025

Choose a reason for hiding this comment

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

Using 'any' type defeats the purpose of TypeScript's type safety. Consider using the StreamChunk type or creating a more specific type for chunk objects.

Suggested change
(c: any) =>
.filter((c: StreamChunk) => c.type === 'text')
.map((c: StreamChunk) => c.text)
.join('');
expect(text).toBe(simpleAnswerText);
expect(
chunks.some(
(c: StreamChunk) => c.type === 'reasoning' && c.text === simpleReasoning,
),
).toBeTruthy();
expect(
chunks.some(
(c: StreamChunk) =>

Copilot uses AI. Check for mistakes.

expect(
chunks.some(
(c: any) =>
Copy link

Copilot AI Aug 16, 2025

Choose a reason for hiding this comment

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

Using 'any' type defeats the purpose of TypeScript's type safety. Consider using the StreamChunk type or creating a more specific type for chunk objects.

Suggested change
(c: any) =>
.filter((c: StreamChunk) => c.type === 'text')
.map((c: StreamChunk) => c.text)
.join('');
expect(text).toBe(simpleAnswerText);
expect(
chunks.some(
(c: StreamChunk) => c.type === 'reasoning' && c.text === simpleReasoning,
),
).toBeTruthy();
expect(
chunks.some(
(c: StreamChunk) =>

Copilot uses AI. Check for mistakes.

expect(
chunks.some(
(c: any) =>
Copy link

Copilot AI Aug 16, 2025

Choose a reason for hiding this comment

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

Using 'any' type defeats the purpose of TypeScript's type safety. Consider using the StreamChunk type or creating a more specific type for chunk objects.

Suggested change
(c: any) =>
.filter((c: StreamChunk) => c.type === 'text')
.map((c: StreamChunk) => c.text)
.join('');
expect(text).toBe(simpleAnswerText);
expect(
chunks.some(
(c: StreamChunk) => c.type === 'reasoning' && c.text === simpleReasoning,
),
).toBeTruthy();
expect(
chunks.some(
(c: StreamChunk) =>

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,16 @@
export type StreamChunk = Record<string, any>
Copy link

Copilot AI Aug 16, 2025

Choose a reason for hiding this comment

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

The StreamChunk type is too generic. Consider defining a more specific union type based on the actual chunk structures (e.g., TextChunk | ReasoningChunk | ToolChunk) to improve type safety and developer experience.

Suggested change
export type StreamChunk = Record<string, any>
export type TextChunk = { type: 'text'; text: string }
export type ReasoningChunk = { type: 'reasoning'; reasoning: string }
export type ToolChunk = { type: 'tool'; tool: string; args?: Record<string, any> }
export type StreamChunk = TextChunk | ReasoningChunk | ToolChunk

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant