Conversation
a6a3fd2 to
b2b9c4f
Compare
Refactored filter extraction logic in barista.ts by introducing getSearchFilters and updating getSearchFiltersAsBase64 to use it. Improved BaristaModal to allow editing and applying filters interactively, added SearchPills, and enhanced the results display. Simplified FilterMonitor entity suggester button layout for better UI consistency.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the Barista chat interface to use a modal-based UI instead of a floating popup, extracts filter parsing logic into a reusable function, and modernizes styling with Bootstrap utilities.
Key changes:
- Introduced
getSearchFilters()function to extract and return raw filter arrays from Barista responses - Created new
BaristaModal.vuecomponent using Composition API for displaying chat and filter results side-by-side - Refactored styling to use Bootstrap utilities and CSS custom properties instead of custom CSS
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/services/types/barista.ts | Exported previously internal interfaces and added getSearchFilters() helper function for extracting raw filters |
| src/components/barista/BaristaModal.vue | New modal component implementing chat interface with filter preview/application using Composition API |
| src/components/barista/BaristaChatPanel.vue | Updated styling to use Bootstrap utilities and removed custom CSS for chat input |
| src/components/barista/BaristaChat.vue | Refactored to emit stringified filters and display filter confirmation messages |
| src/components/barista/BaristaButton.vue | Replaced floating popup with BaristaModal component |
| src/components/modules/FilterMonitor.vue | Simplified button layout and added missing translation key |
| // const filtersToolCalls = | ||
| // kwargs.tool_calls?.filter?.( | ||
| // tool_call => tool_call.type === 'function' && tool_call.function.name === 'Filters' | ||
| // ) ?? [] | ||
|
|
||
| // if (filtersToolCalls.length > 0) { | ||
| // try { | ||
| // const searchFilters = getSearchFilters(JSON.parse(filtersToolCalls[0].function.arguments))const serializedFilters = toSerializedFilters | ||
| // const serializedFilters = toSerializedFilters(searchFilters['filters']) | ||
| // return serializedFilters | ||
| // } catch (error) { | ||
| // console.error('Error parsing search filters:', error) | ||
| // return undefined | ||
| // } | ||
| // } | ||
| // return undefined |
There was a problem hiding this comment.
Remove commented-out code from lines 68-83. This appears to be old implementation that was replaced by the new logic.
| // const filtersToolCalls = | |
| // kwargs.tool_calls?.filter?.( | |
| // tool_call => tool_call.type === 'function' && tool_call.function.name === 'Filters' | |
| // ) ?? [] | |
| // if (filtersToolCalls.length > 0) { | |
| // try { | |
| // const searchFilters = getSearchFilters(JSON.parse(filtersToolCalls[0].function.arguments))const serializedFilters = toSerializedFilters | |
| // const serializedFilters = toSerializedFilters(searchFilters['filters']) | |
| // return serializedFilters | |
| // } catch (error) { | |
| // console.error('Error parsing search filters:', error) | |
| // return undefined | |
| // } | |
| // } | |
| // return undefined |
|
|
||
| // if (filtersToolCalls.length > 0) { | ||
| // try { | ||
| // const searchFilters = getSearchFilters(JSON.parse(filtersToolCalls[0].function.arguments))const serializedFilters = toSerializedFilters |
There was a problem hiding this comment.
Line 75 contains malformed commented code with two statements concatenated on the same line without proper separation. If this code were uncommented, it would cause a syntax error.
| // const searchFilters = getSearchFilters(JSON.parse(filtersToolCalls[0].function.arguments))const serializedFilters = toSerializedFilters | |
| // const searchFilters = getSearchFilters(JSON.parse(filtersToolCalls[0].function.arguments)) |
| > | ||
|
|
||
| export const getSearchFiltersAsBase64 = (kwargs: AdditionalKwargs): string | undefined => { | ||
| export const getSearchFilters = (kwargs: AdditionalKwargs): any[] => { |
There was a problem hiding this comment.
Return type any[] is too generic. Consider defining a specific interface for filter objects to improve type safety and make the API contract clearer.
|
|
||
| <script setup lang="ts"> | ||
| import { computed, ref, watch } from 'vue' | ||
| import { getSearchFilters, getSearchFiltersAsBase64 } from '@/services/types/barista' |
There was a problem hiding this comment.
getSearchFiltersAsBase64 is imported but never used in this component. Remove the unused import.
| import { getSearchFilters, getSearchFiltersAsBase64 } from '@/services/types/barista' | |
| import { getSearchFilters } from '@/services/types/barista' |
| } | ||
| }) | ||
| const searchFilters = ref([]) | ||
| const handleFiltersChanged = (newFilters: any) => { |
There was a problem hiding this comment.
Parameter type any should be replaced with a more specific type. Consider defining an interface for filter objects or using the return type from getSearchFilters().
| const handleFiltersChanged = (newFilters: any) => { | |
| const handleFiltersChanged = (newFilters: ReturnType<typeof getSearchFilters>) => { |
|
|
||
| const isChatOpen = ref(false) | ||
| const chatPopup = ref<HTMLElement | null>(null) | ||
| const showModal = ref(true) |
There was a problem hiding this comment.
The variable showModal is defined but never used in the component. Consider removing it.
| const showModal = ref(true) |
| // const handleSearch = (searchFilters: string) => { | ||
| // // Emit the search event so parent components can listen | ||
| // emit('search', searchFilters) | ||
| // } |
There was a problem hiding this comment.
Remove commented-out handleSearch function. It has been replaced by handleApplyFilters.
| // const handleSearch = (searchFilters: string) => { | |
| // // Emit the search event so parent components can listen | |
| // emit('search', searchFilters) | |
| // } |
| } | ||
|
|
||
| // Handler for sending messages | ||
| // Handles user input + Barista response |
There was a problem hiding this comment.
Comment has inconsistent spacing. Should be // Handles user input + Barista response with a space after //.
| // Handles user input + Barista response | |
| // Handles user input + Barista response |
…o/impresso-frontend into feature/barista-on-stage
Adds a 'filters' prop to BaristaButton and passes filtersWithItems from Search.vue. Improves BaristaChatPanel input styling, adds focus on mount, and adjusts padding for better user experience.
…o/impresso-frontend into feature/barista-on-stage
|
see develop branch |
JSON structure + apply filters button