refactor: decouple chat.vue #2974
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `dashboard/src/components/chat/InputArea.vue:98-104` </location>
<code_context>
+ }
+}
+
+async function startRecording() {
+ const stream = await navigator.mediaDevices.getUserMedia({ audio: true });
+ mediaRecorder = new MediaRecorder(stream);
+ mediaRecorder.ondataavailable = (event) => {
+ audioChunks.push(event.data);
+ };
+ mediaRecorder.start();
+ isRecording.value = true;
+}
</code_context>
<issue_to_address>
**issue:** startRecording does not handle permission denial or errors from getUserMedia.
Add error handling to notify the user and maintain consistent UI state if getUserMedia fails or access is denied.
</issue_to_address>
### Comment 2
<location> `dashboard/src/components/chat/InputArea.vue:137-146` </location>
<code_context>
+ stagedImagesUrl.value.splice(index, 1);
+}
+
+function handlePaste(event: ClipboardEvent) {
+ const items = event.clipboardData?.items;
+ if (!items) return;
+ for (let i = 0; i < items.length; i++) {
+ if (items[i].type.indexOf('image') !== -1) {
+ const file = items[i].getAsFile();
+ if (file) processAndUploadImage(file);
+ }
+ }
+}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** handlePaste does not prevent default paste behavior.
To avoid duplicate images or unexpected behavior, call event.preventDefault() when processing pasted images.
```suggestion
function handlePaste(event: ClipboardEvent) {
event.preventDefault();
const items = event.clipboardData?.items;
if (!items) return;
for (let i = 0; i < items.length; i++) {
if (items[i].type.indexOf('image') !== -1) {
const file = items[i].getAsFile();
if (file) processAndUploadImage(file);
}
}
}
```
</issue_to_address>
### Comment 3
<location> `dashboard/src/components/chat/SidebarPanel.vue:109-118` </location>
<code_context>
+
+ onUnmounted(() => dispose());
+
+ function formatDate(ts) {
+ // Sidebar 简化显示：当天仅显示时/分，否则显示月/日 时:分
+ if (!ts) return '';
+ const date = new Date(ts * 1000);
+ const now = new Date();
+ const todayStart = new Date(now.getFullYear(), now.getMonth(), now.getDate()).getTime();
+ if (date.getTime() < todayStart) {
+ return new Intl.DateTimeFormat(undefined, { month: '2-digit', day: '2-digit', hour: '2-digit', minute: '2-digit' }).format(date);
+ }
+ return new Intl.DateTimeFormat(undefined, { hour: '2-digit', minute: '2-digit' }).format(date);
+ }
+
</code_context>
<issue_to_address>
**suggestion:** formatDate uses undefined locale for Intl.DateTimeFormat.
Defaulting to the browser locale may cause inconsistent date formats. Please use the application's configured locale for consistency.
Suggested implementation:
```
// 假设 appLocale 是应用配置的 locale，可以通过 props、inject、或 composable 获取
// 例如：const appLocale = inject('appLocale') || 'en-US';
const appLocale = 'en-US'; // 请根据实际情况替换为应用的 locale
function formatDate(ts) {
// Sidebar 简化显示：当天仅显示时/分，否则显示月/日 时:分
if (!ts) return '';
const date = new Date(ts * 1000);
const now = new Date();
const todayStart = new Date(now.getFullYear(), now.getMonth(), now.getDate()).getTime();
if (date.getTime() < todayStart) {
return new Intl.DateTimeFormat(appLocale, { month: '2-digit', day: '2-digit', hour: '2-digit', minute: '2-digit' }).format(date);
}
return new Intl.DateTimeFormat(appLocale, { hour: '2-digit', minute: '2-digit' }).format(date);
}
```
- If your application already provides a locale (e.g., via Vue's i18n plugin, a global store, or context), replace `const appLocale = 'en-US'` with the correct way to access it.
- If `formatDate` is used outside the setup script, ensure `appLocale` is accessible in that scope.
</issue_to_address>
### Comment 4
<location> `dashboard/src/services/chat.api.ts:1` </location>
<code_context>
<script>
import { router } from '@/router';
-import axios from 'axios';
import { ref } from 'vue';
import { useCustomizerStore } from '@/stores/customizer';
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring repeated axios logic into a shared helper to simplify each API function.
Here’s a small refactoring that pulls out all the common bits into one helper and shrinks each endpoint down to just its unique bits:
```ts
// api.ts
import axios, { Method, AxiosRequestConfig } from 'axios';
const api = axios.create({ baseURL: '/api/chat' });
interface WrappedResponse<T> {
data: T;
}
async function apiRequest<T>(
method: Method,
url: string,
config: AxiosRequestConfig = {}
): Promise<T> {
const res = await api.request<WrappedResponse<T>>({ method, url, ...config });
return res.data.data;
}
export default apiRequest;
```
Then in your chat client file:
```ts
// chatApi.ts
import apiRequest from './api';
// Conversations
export const listConversations = () =>
apiRequest<Conversation[]>('get', '/conversations');
export const getConversation = (conversation_id: string) =>
apiRequest<Conversation>('get', '/get_conversation', {
params: { conversation_id },
});
export const newConversation = () =>
apiRequest<Conversation>('get', '/new_conversation');
export const deleteConversation = (conversation_id: string) =>
apiRequest<void>('get', '/delete_conversation', {
params: { conversation_id },
});
export const renameConversation = (conversation_id: string, title: string) =>
apiRequest<void>('post', '/rename_conversation', {
data: { conversation_id, title },
});
// Media
const withForm = (url: string, file: File | Blob) => {
const form = new FormData();
form.append('file', file);
return apiRequest<FileMeta>('post', url, {
data: form,
headers: { 'Content-Type': 'multipart/form-data' },
});
};
export const postImage = (file: File) => withForm('/post_image', file);
export const postFile = (file: File | Blob) => withForm('/post_file', file);
export const getFile = (filename: string) =>
apiRequest<Blob>('get', '/get_file', {
params: { filename },
responseType: 'blob',
});
// keep your fetch-based streaming call as-is
export async function sendMessageStream(payload: { /* ... */ }) {
/* unchanged */
}
```
Benefits:
- One place to configure baseURL, error handling, and `.data?.data` extraction.
- Each exported function is now 1–2 lines of its unique logic.
- Adding headers, responseType, params, form data is all still possible via the `config` parameter.
</issue_to_address>
### Comment 5
<location> `dashboard/src/composables/chat/useChatStream.ts:11` </location>
<code_context>
+};
+
+export function useChatStream(getMediaUrl: (filename: string) => Promise<string>) {
+ async function runStream(body: ReadableStream<Uint8Array> | null, handlers: ChatStreamHandlers = {}) {
+ if (!body) return;
+ const reader = body.getReader();
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring runStream by extracting JSON parsing and type dispatch into helper functions for improved clarity and maintainability.
```markdown
You can flatten `runStream` by pulling out the JSON-parsing and type-dispatch logic into small helpers. For example:
```ts
type StreamChunk = {
type: string
data: any
cid?: string
streaming?: boolean
}
async function parseChunk(line: string): Promise<StreamChunk | null> {
try {
const raw = line.replace(/^data:\s*/, '')
const obj = JSON.parse(raw)
if (obj && typeof obj === 'object' && 'type' in obj) return obj
} catch {
console.warn('failed to parse', line)
}
return null
}
function makeDispatch(
getMediaUrl: (f: string) => Promise<string>,
handlers: ChatStreamHandlers
) {
let inStreaming = false
const map: Record<string, (c: StreamChunk) => Promise<void>> = {
error: async c => handlers.onError?.(c.data),
image: async c => {
const url = await getMediaUrl(String(c.data).replace('[IMAGE]', ''))
handlers.onImage?.(url)
},
record: async c => {
const url = await getMediaUrl(String(c.data).replace('[RECORD]', ''))
handlers.onAudio?.(url)
},
plain: async c => {
if (!inStreaming) {
handlers.onTextStart?.(c.data)
inStreaming = true
} else {
handlers.onTextAppend?.(c.data)
}
},
update_title: async c => handlers.onUpdateTitle?.(c.cid!, c.data),
}
return async (chunk: StreamChunk) => {
await (map[chunk.type] ?? (async () => {}))(chunk)
if ((chunk.type === 'break' && chunk.streaming) || !chunk.streaming) {
inStreaming = false
}
}
}
```
Then your `runStream` becomes:
```ts
async function runStream(
body: ReadableStream<Uint8Array> | null,
handlers: ChatStreamHandlers = {}
) {
if (!body) return
const reader = body.getReader()
const decoder = new TextDecoder()
const dispatch = makeDispatch(getMediaUrl, handlers)
while (true) {
try {
const { done, value } = await reader.read()
if (done) break
const chunk = decoder.decode(value, { stream: true })
for (const line of chunk.split('\n\n')) {
const trimmed = line.trim()
if (!trimmed) continue
const data = await parseChunk(trimmed)
if (data) await dispatch(data)
}
} catch (err) {
handlers.onError?.(err)
break
}
}
}
```
This splits responsibilities into:
1. `parseChunk` – JSON parsing & validation
2. `makeDispatch` – a flat map of type-handlers
3. `runStream` – streaming loop only
which should be much easier to read and test.
</issue_to_address>
### Comment 6
<location> `dashboard/src/composables/chat/useSidebarState.ts:6` </location>
<code_context>
+const LOCAL_KEY = 'sidebarCollapsed';
+
+export function useSidebarState() {
+ const sidebarCollapsed = ref(true);
+ const sidebarHovered = ref(false);
+ const sidebarHoverExpanded = ref(false);
</code_context>
<issue_to_address>
**issue (complexity):** Consider consolidating multiple sidebar state refs into a single reactive object and using local variables for timers.
```suggestion
// Instead of 5 separate refs (`sidebarCollapsed`, `sidebarHovered`,
// `sidebarHoverExpanded`, `sidebarHoverTimer`, `sidebarHoverDelay`),
// you can collapse them into one reactive object + a local timer variable.
import { reactive, toRefs, watch, onBeforeUnmount } from 'vue'
const LOCAL_KEY = 'sidebarCollapsed'
const HOVER_DELAY = 100
export function useSidebarState() {
// 1. single state object
const state = reactive({
collapsed: JSON.parse(localStorage.getItem(LOCAL_KEY) ?? 'true'),
hovered: false,
hoverExpanded: false,
})
// 2. local timer var (no need for a ref)
let hoverTimer: number
// 3. persist whenever `collapsed` changes
watch(() => state.collapsed, v =>
localStorage.setItem(LOCAL_KEY, JSON.stringify(v))
)
function toggleSidebar() {
if (state.hoverExpanded) {
state.hoverExpanded = false
} else {
state.collapsed = !state.collapsed
}
}
function handleSidebarMouseEnter() {
if (!state.collapsed) return
state.hovered = true
hoverTimer = window.setTimeout(() => {
if (state.hovered) {
state.hoverExpanded = true
state.collapsed = false
}
}, HOVER_DELAY)
}
function handleSidebarMouseLeave() {
state.hovered = false
clearTimeout(hoverTimer)
if (state.hoverExpanded) {
state.collapsed = true
state.hoverExpanded = false
}
}
onBeforeUnmount(() => clearTimeout(hoverTimer))
// expose individual properties if you still need them
return {
...toRefs(state),
toggleSidebar,
handleSidebarMouseEnter,
handleSidebarMouseLeave,
}
}
```
Benefits:
1. State is grouped into one `reactive` object.
2. `hoverTimer` and `HOVER_DELAY` are simple locals (no extra refs).
3. `collapsed` persistence is handled declaratively with a `watch`.
4. Functionality remains identical.
</issue_to_address>
### Comment 7
<location> `dashboard/src/composables/chat/useChatRouteSync.ts:4-31` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 8
<location> `dashboard/src/composables/chat/useChatRouteSync.ts:33-58` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 9
<location> `dashboard/src/composables/chat/useChatStream.ts:11-75` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 10
<location> `dashboard/src/composables/chat/useChatStream.ts:41` </location>
<code_context>
const type = chunk_json.type;
</code_context>
<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/use-object-destructuring))
```suggestion
const {type} = chunk_json;
```
<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.
From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>
### Comment 11
<location> `dashboard/src/composables/chat/useMediaCache.ts:4-10` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 12
<location> `dashboard/src/composables/chat/useMediaCache.ts:12-17` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 13
<location> `dashboard/src/composables/chat/useSidebarState.ts:16-18` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 14
<location> `dashboard/src/composables/chat/useSidebarState.ts:20-27` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 15
<location> `dashboard/src/composables/chat/useSidebarState.ts:29-38` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 16
<location> `dashboard/src/composables/chat/useSidebarState.ts:40-50` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 17
<location> `dashboard/src/composables/chat/useSidebarState.ts:52-54` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+98
to
+104
| async function startRecording() { | ||
| const stream = await navigator.mediaDevices.getUserMedia({ audio: true }); | ||
| mediaRecorder = new MediaRecorder(stream); | ||
| mediaRecorder.ondataavailable = (event) => { | ||
| audioChunks.push(event.data); | ||
| }; | ||
| mediaRecorder.start(); |
There was a problem hiding this comment.
issue: startRecording does not handle permission denial or errors from getUserMedia.
Add error handling to notify the user and maintain consistent UI state if getUserMedia fails or access is denied.
Comment on lines
+137
to
+146
| function handlePaste(event: ClipboardEvent) { | ||
| const items = event.clipboardData?.items; | ||
| if (!items) return; | ||
| for (let i = 0; i < items.length; i++) { | ||
| if (items[i].type.indexOf('image') !== -1) { | ||
| const file = items[i].getAsFile(); | ||
| if (file) processAndUploadImage(file); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
suggestion (bug_risk): handlePaste does not prevent default paste behavior.
To avoid duplicate images or unexpected behavior, call event.preventDefault() when processing pasted images.
Suggested change
| function handlePaste(event: ClipboardEvent) { | |
| const items = event.clipboardData?.items; | |
| if (!items) return; | |
| for (let i = 0; i < items.length; i++) { | |
| if (items[i].type.indexOf('image') !== -1) { | |
| const file = items[i].getAsFile(); | |
| if (file) processAndUploadImage(file); | |
| } | |
| } | |
| } | |
| function handlePaste(event: ClipboardEvent) { | |
| event.preventDefault(); | |
| const items = event.clipboardData?.items; | |
| if (!items) return; | |
| for (let i = 0; i < items.length; i++) { | |
| if (items[i].type.indexOf('image') !== -1) { | |
| const file = items[i].getAsFile(); | |
| if (file) processAndUploadImage(file); | |
| } | |
| } | |
| } |
Comment on lines
+109
to
+118
| function formatDate(ts) { | ||
| // Sidebar 简化显示：当天仅显示时/分，否则显示月/日 时:分 | ||
| if (!ts) return ''; | ||
| const date = new Date(ts * 1000); | ||
| const now = new Date(); | ||
| const todayStart = new Date(now.getFullYear(), now.getMonth(), now.getDate()).getTime(); | ||
| if (date.getTime() < todayStart) { | ||
| return new Intl.DateTimeFormat(undefined, { month: '2-digit', day: '2-digit', hour: '2-digit', minute: '2-digit' }).format(date); | ||
| } | ||
| return new Intl.DateTimeFormat(undefined, { hour: '2-digit', minute: '2-digit' }).format(date); |
There was a problem hiding this comment.
suggestion: formatDate uses undefined locale for Intl.DateTimeFormat.
Defaulting to the browser locale may cause inconsistent date formats. Please use the application's configured locale for consistency.
Suggested implementation:
// 假设 appLocale 是应用配置的 locale，可以通过 props、inject、或 composable 获取
// 例如：const appLocale = inject('appLocale') || 'en-US';
const appLocale = 'en-US'; // 请根据实际情况替换为应用的 locale
function formatDate(ts) {
// Sidebar 简化显示：当天仅显示时/分，否则显示月/日 时:分
if (!ts) return '';
const date = new Date(ts * 1000);
const now = new Date();
const todayStart = new Date(now.getFullYear(), now.getMonth(), now.getDate()).getTime();
if (date.getTime() < todayStart) {
return new Intl.DateTimeFormat(appLocale, { month: '2-digit', day: '2-digit', hour: '2-digit', minute: '2-digit' }).format(date);
}
return new Intl.DateTimeFormat(appLocale, { hour: '2-digit', minute: '2-digit' }).format(date);
}
- If your application already provides a locale (e.g., via Vue's i18n plugin, a global store, or context), replace
const appLocale = 'en-US'with the correct way to access it. - If
formatDateis used outside the setup script, ensureappLocaleis accessible in that scope.
| @@ -0,0 +1,89 @@ | |||
| import axios from 'axios'; | |||
There was a problem hiding this comment.
issue (complexity): Consider refactoring repeated axios logic into a shared helper to simplify each API function.
Here’s a small refactoring that pulls out all the common bits into one helper and shrinks each endpoint down to just its unique bits:
// api.ts
import axios, { Method, AxiosRequestConfig } from 'axios';
const api = axios.create({ baseURL: '/api/chat' });
interface WrappedResponse<T> {
data: T;
}
async function apiRequest<T>(
method: Method,
url: string,
config: AxiosRequestConfig = {}
): Promise<T> {
const res = await api.request<WrappedResponse<T>>({ method, url, ...config });
return res.data.data;
}
export default apiRequest;Then in your chat client file:
// chatApi.ts
import apiRequest from './api';
// Conversations
export const listConversations = () =>
apiRequest<Conversation[]>('get', '/conversations');
export const getConversation = (conversation_id: string) =>
apiRequest<Conversation>('get', '/get_conversation', {
params: { conversation_id },
});
export const newConversation = () =>
apiRequest<Conversation>('get', '/new_conversation');
export const deleteConversation = (conversation_id: string) =>
apiRequest<void>('get', '/delete_conversation', {
params: { conversation_id },
});
export const renameConversation = (conversation_id: string, title: string) =>
apiRequest<void>('post', '/rename_conversation', {
data: { conversation_id, title },
});
// Media
const withForm = (url: string, file: File | Blob) => {
const form = new FormData();
form.append('file', file);
return apiRequest<FileMeta>('post', url, {
data: form,
headers: { 'Content-Type': 'multipart/form-data' },
});
};
export const postImage = (file: File) => withForm('/post_image', file);
export const postFile = (file: File | Blob) => withForm('/post_file', file);
export const getFile = (filename: string) =>
apiRequest<Blob>('get', '/get_file', {
params: { filename },
responseType: 'blob',
});
// keep your fetch-based streaming call as-is
export async function sendMessageStream(payload: { /* ... */ }) {
/* unchanged */
}Benefits:
- One place to configure baseURL, error handling, and
.data?.dataextraction. - Each exported function is now 1–2 lines of its unique logic.
- Adding headers, responseType, params, form data is all still possible via the
configparameter.
| }; | ||
|
|
||
| export function useChatStream(getMediaUrl: (filename: string) => Promise<string>) { | ||
| async function runStream(body: ReadableStream<Uint8Array> | null, handlers: ChatStreamHandlers = {}) { |
There was a problem hiding this comment.
issue (complexity): Consider refactoring runStream by extracting JSON parsing and type dispatch into helper functions for improved clarity and maintainability.
You can flatten `runStream` by pulling out the JSON-parsing and type-dispatch logic into small helpers. For example:
```ts
type StreamChunk = {
type: string
data: any
cid?: string
streaming?: boolean
}
async function parseChunk(line: string): Promise<StreamChunk | null> {
try {
const raw = line.replace(/^data:\s*/, '')
const obj = JSON.parse(raw)
if (obj && typeof obj === 'object' && 'type' in obj) return obj
} catch {
console.warn('failed to parse', line)
}
return null
}
function makeDispatch(
getMediaUrl: (f: string) => Promise<string>,
handlers: ChatStreamHandlers
) {
let inStreaming = false
const map: Record<string, (c: StreamChunk) => Promise<void>> = {
error: async c => handlers.onError?.(c.data),
image: async c => {
const url = await getMediaUrl(String(c.data).replace('[IMAGE]', ''))
handlers.onImage?.(url)
},
record: async c => {
const url = await getMediaUrl(String(c.data).replace('[RECORD]', ''))
handlers.onAudio?.(url)
},
plain: async c => {
if (!inStreaming) {
handlers.onTextStart?.(c.data)
inStreaming = true
} else {
handlers.onTextAppend?.(c.data)
}
},
update_title: async c => handlers.onUpdateTitle?.(c.cid!, c.data),
}
return async (chunk: StreamChunk) => {
await (map[chunk.type] ?? (async () => {}))(chunk)
if ((chunk.type === 'break' && chunk.streaming) || !chunk.streaming) {
inStreaming = false
}
}
}Then your runStream becomes:
async function runStream(
body: ReadableStream<Uint8Array> | null,
handlers: ChatStreamHandlers = {}
) {
if (!body) return
const reader = body.getReader()
const decoder = new TextDecoder()
const dispatch = makeDispatch(getMediaUrl, handlers)
while (true) {
try {
const { done, value } = await reader.read()
if (done) break
const chunk = decoder.decode(value, { stream: true })
for (const line of chunk.split('\n\n')) {
const trimmed = line.trim()
if (!trimmed) continue
const data = await parseChunk(trimmed)
if (data) await dispatch(data)
}
} catch (err) {
handlers.onError?.(err)
break
}
}
}This splits responsibilities into:
parseChunk– JSON parsing & validationmakeDispatch– a flat map of type-handlersrunStream– streaming loop only
which should be much easier to read and test.
| const LOCAL_KEY = 'sidebarCollapsed'; | ||
|
|
||
| export function useSidebarState() { | ||
| const sidebarCollapsed = ref(true); |
There was a problem hiding this comment.
issue (complexity): Consider consolidating multiple sidebar state refs into a single reactive object and using local variables for timers.
Suggested change
| const sidebarCollapsed = ref(true); | |
| // Instead of 5 separate refs (`sidebarCollapsed`, `sidebarHovered`, | |
| // `sidebarHoverExpanded`, `sidebarHoverTimer`, `sidebarHoverDelay`), | |
| // you can collapse them into one reactive object + a local timer variable. | |
| import { reactive, toRefs, watch, onBeforeUnmount } from 'vue' | |
| const LOCAL_KEY = 'sidebarCollapsed' | |
| const HOVER_DELAY = 100 | |
| export function useSidebarState() { | |
| // 1. single state object | |
| const state = reactive({ | |
| collapsed: JSON.parse(localStorage.getItem(LOCAL_KEY) ?? 'true'), | |
| hovered: false, | |
| hoverExpanded: false, | |
| }) | |
| // 2. local timer var (no need for a ref) | |
| let hoverTimer: number | |
| // 3. persist whenever `collapsed` changes | |
| watch(() => state.collapsed, v => | |
| localStorage.setItem(LOCAL_KEY, JSON.stringify(v)) | |
| ) | |
| function toggleSidebar() { | |
| if (state.hoverExpanded) { | |
| state.hoverExpanded = false | |
| } else { | |
| state.collapsed = !state.collapsed | |
| } | |
| } | |
| function handleSidebarMouseEnter() { | |
| if (!state.collapsed) return | |
| state.hovered = true | |
| hoverTimer = window.setTimeout(() => { | |
| if (state.hovered) { | |
| state.hoverExpanded = true | |
| state.collapsed = false | |
| } | |
| }, HOVER_DELAY) | |
| } | |
| function handleSidebarMouseLeave() { | |
| state.hovered = false | |
| clearTimeout(hoverTimer) | |
| if (state.hoverExpanded) { | |
| state.collapsed = true | |
| state.hoverExpanded = false | |
| } | |
| } | |
| onBeforeUnmount(() => clearTimeout(hoverTimer)) | |
| // expose individual properties if you still need them | |
| return { | |
| ...toRefs(state), | |
| toggleSidebar, | |
| handleSidebarMouseEnter, | |
| handleSidebarMouseLeave, | |
| } | |
| } |
Benefits:
- State is grouped into one
reactiveobject. hoverTimerandHOVER_DELAYare simple locals (no extra refs).collapsedpersistence is handled declaratively with awatch.- Functionality remains identical.
| continue; | ||
| } | ||
|
|
||
| const type = chunk_json.type; |
There was a problem hiding this comment.
suggestion (code-quality): Prefer object destructuring when accessing and using properties. (use-object-destructuring)
Suggested change
| const type = chunk_json.type; | |
| const {type} = chunk_json; |
Explanation
Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.From the Airbnb Javascript Style Guide
|
怎么关掉了（ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #XYZ
Motivation / 动机
目前 chat.vue 里面太臃肿了，于是进行解耦，使其更好维护
Modifications / 改动点
Verification Steps / 验证步骤
Screenshots or Test Results / 运行截图或测试结果
Compatibility & Breaking Changes / 兼容性与破坏性变更
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Sourcery 总结
重构 ChatPage 组件，通过将其 UI 和逻辑解耦为可复用的子组件、服务模块和可组合工具
改进：
services/chat.api.ts模块中useChatStream)、侧边栏状态 (useSidebarState)、路由同步 (useChatRouteSync)、媒体缓存 (useMediaCache) 和日期格式化 (useDateFormat)ChatPage.css文件中，以实现集中式样式管理Original summary in English
Summary by Sourcery
Refactor the ChatPage component by decoupling its UI and logic into reusable subcomponents, service modules, and composable utilities
Enhancements:
services/chat.api.tsmoduleuseChatStream), sidebar state (useSidebarState), route synchronization (useChatRouteSync), media caching (useMediaCache), and date formatting (useDateFormat)ChatPage.cssfor centralized styling