Skip to content

Comments

Conversational search#160

Draft
korgon wants to merge 39 commits intodevelopfrom
conversational-search
Draft

Conversational search#160
korgon wants to merge 39 commits intodevelopfrom
conversational-search

Conversation

@korgon
Copy link

@korgon korgon commented Jan 7, 2026

  • under active development - DO NOT MERGE

Copilot AI review requested due to automatic review settings January 7, 2026 15:50
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 introduces conversational search functionality through a new Chat feature. The implementation includes a complete chat interface with support for image uploads, product attachments, and AI-powered responses.

Key Changes:

  • Added ChatController, ChatStore, and ChatSessionStore for managing chat state and interactions
  • Integrated chat API client with support for various request types (general, product query, image search, etc.)
  • Created Chat UI components with message display, attachments, and feedback mechanisms

Reviewed changes

Copilot reviewed 42 out of 43 changed files in this pull request and generated 14 comments.

Show a summary per file
File Description
packages/snap-store-mobx/src/types.ts Added ChatStoreConfig types and integrated into StoreConfigs union
packages/snap-store-mobx/src/Chat/ChatStore.ts Implements main chat store with session management and storage
packages/snap-store-mobx/src/Chat/Stores/ChatSessionStore.ts Manages individual chat sessions with message history and attachments
packages/snap-store-mobx/src/Chat/Stores/ChatAttachmentStore.ts Handles image and product attachments with state management
packages/snap-controller/src/Chat/ChatController.ts Controller implementation for chat interactions and API requests
packages/snap-preact/src/Templates/SnapTemplates.tsx Integrates chat targets and configuration into template system
packages/snap-preact/components/src/components/Organisms/Chat/Chat.tsx Main chat UI component with message display and input
packages/snap-client/src/Client/apis/Chat.ts API client for chat endpoints including message sending and image upload
packages/snap-client/src/Client/transforms/chatResponse.ts Transforms chat API responses to standardized format
.github/workflows/test.yml Temporarily disabled tests (commented out)
Comments suppressed due to low confidence (1)

packages/snap-controller/src/Chat/ChatController.ts:1

  • Corrected spelling of 'transfrorm respose' to 'transform response'.
/*

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

treePath: globalTreePath,
};

let props = mergeProps('facets', globalTheme, defaultProps, properties);
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The first argument should be 'chat' instead of 'facets' to correctly identify the component type for props merging.

Suggested change
let props = mergeProps('facets', globalTheme, defaultProps, properties);
let props = mergeProps('chat', globalTheme, defaultProps, properties);

Copilot uses AI. Check for mistakes.

if (sessionId) {
// @ts-ignore - add sessionId to response context
responseJSON.context = response.context || { sessionId };
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

Attempting to assign to response.context but should be responseJSON.context. The current code creates the context object correctly but the comment suggests confusion about the source.

Suggested change
responseJSON.context = response.context || { sessionId };
responseJSON.context = responseJSON.context || { sessionId };

Copilot uses AI. Check for mistakes.

async postUploadImage(requestParameters: UploadImageRequestModel): Promise<UploadImageResponseModel> {
const headerParameters: HTTPHeaders = {
'X-Widget-Id': 'test-ss-demo',
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

Hard-coded widget ID should be passed as a parameter instead. This makes the upload functionality non-functional for other widgets and creates a security risk by exposing test credentials.

Copilot uses AI. Check for mistakes.
Comment on lines +56 to +59
# - name: Tests
# env:
# NODE_OPTIONS: "--max-old-space-size=4096"
# run: npm run test
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

Tests have been commented out in CI. This should be re-enabled before merging to ensure code quality and prevent regressions.

Suggested change
# - name: Tests
# env:
# NODE_OPTIONS: "--max-old-space-size=4096"
# run: npm run test
- name: Tests
env:
NODE_OPTIONS: "--max-old-space-size=4096"
run: npm run test

Copilot uses AI. Check for mistakes.
};

export class ChatAttachmentProduct extends ChatAttachment {
public type: 'product' | never = 'product';
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The union with 'never' is redundant. The type should simply be 'product' as a literal type.

Copilot uses AI. Check for mistakes.
) : (
<></>
);
} else if (type == 'product') {
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

Use strict equality (===) instead of loose equality (==) for type comparisons.

Copilot uses AI. Check for mistakes.
const attachments: string[] = [];
if (request.data.requestType === 'imageSearch') {
const imageId = request.data.attachedImageId;
const attachedImage = this.attachments.attached.find((item) => item.type == 'image' && item.imageId == imageId);
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

Use strict equality (===) instead of loose equality (==) for type and ID comparisons.

Copilot uses AI. Check for mistakes.

if (request.data.requestType === 'productQuery') {
const productId = request.data.productId;
const attachedImage = this.attachments.attached.find((item) => item.type == 'product' && item.productId == productId);
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

Use strict equality (===) instead of loose equality (==) for type and ID comparisons.

Copilot uses AI. Check for mistakes.
}

export interface MessageProps {
chatItem: any;
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

Replace any type with the proper ChatMessage type that's imported in the file for better type safety.

Copilot uses AI. Check for mistakes.
public storage: StorageStore;
public chats: ChatSessionStore[] = [];
public currentChatId: string;
public quickViewResult: any = {};
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

Replace any type with Product | null or similar appropriate type for better type safety.

Suggested change
public quickViewResult: any = {};
public quickViewResult: ChatAttachmentProduct | null = null;

Copilot uses AI. Check for mistakes.
@korgon korgon marked this pull request as draft January 7, 2026 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants