Merged
Conversation
….digest，优化计算速度。 2. ai分析任务与上传解耦，且实现持久化 3. ai分析任务控制面板，状态指示器。
📝 WalkthroughWalkthroughThis PR introduces worker-based file uploading with deduplication and background AI analysis. Changes include new Web Workers for hashing and concurrent uploads, backend endpoint for file uploads, AI task management infrastructure (UploadManager), database schema extensions (image_hash field), and enhanced frontend components with status indicators and batch operations. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User<br/>(Browser)
participant Dialog as AddEmojiDialog
participant HashWorker as HashWorker
participant UploadWorker as UploadWorker
participant Server as Backend
participant DB as Database
participant AI as AI Processor
User->>Dialog: Select files + submit
Dialog->>HashWorker: postMessage({files, concurrency})
activate HashWorker
HashWorker->>HashWorker: Compute SHA-256 per file
HashWorker->>Dialog: postMessage({type:'hash',hash})
deactivate HashWorker
HashWorker->>Dialog: postMessage({type:'done'})
Dialog->>Dialog: Deduplicate by hash,<br/>detect duplicates in batch
Dialog->>Server: Upload unique files via UploadWorker
Dialog->>UploadWorker: postMessage({files,url,concurrency})
activate UploadWorker
UploadWorker->>Server: POST /upload FormData<br/>(file,name,category,tags,aiAnalysis)
Server->>DB: Check image_hash (dedup)
alt Already exists
Server->>Dialog: Error (duplicate)
else New emoji
Server->>DB: Create emoji record
Server->>AI: Queue aiAnalysis task<br/>(if aiAnalysis=true)
Server->>Dialog: Success response
end
UploadWorker->>Dialog: postMessage({type:'progress'})
deactivate UploadWorker
UploadWorker->>Dialog: postMessage({type:'done',errors?})
Dialog->>Dialog: Show result:<br/>success/partial-fail
alt Partial failure
Dialog->>User: Keep open, show warnings
else Full success
Dialog->>User: Close + show notification
end
par Background AI Processing
AI->>AI: Process queued tasks<br/>(concurrency control)
AI->>DB: Update emoji name/category/tags
end
sequenceDiagram
participant Manager as EmojiManager
participant Service as EmojiLunaService
participant UploadMgr as UploadManager
participant Processor as AITaskProcessor
participant DB as Database
Manager->>Service: getAiTaskStats()
Service->>UploadMgr: getAITaskStats()
UploadMgr->>Manager: {pending,processing,succeeded,failed,paused}
Manager->>Manager: Render status indicators on cards<br/>(based on aiStats)
Manager->>Service: reanalyzeBatch(selectedIds)
Service->>UploadMgr: reanalyzeBatch(ids)
UploadMgr->>UploadMgr: Enqueue tasks for each id<br/>(skip if already processing)
loop Background Processing Loop
UploadMgr->>UploadMgr: processAITask(task)
alt Task paused
UploadMgr->>UploadMgr: Wait/retry with backoff
else Process
UploadMgr->>Processor: analyzeEmoji(imageBase64)
Processor->>Processor: AI analysis
Processor->>UploadMgr: AIAnalysisResult
UploadMgr->>UploadMgr: Cache result (hash→result)
UploadMgr->>Processor: updateEmojiInfo(id,updates)
Processor->>DB: Update emoji
alt Success
UploadMgr->>UploadMgr: Mark task succeeded
else Failure
UploadMgr->>UploadMgr: Track attempts, backoff,<br/>mark failed after max
end
end
end
Manager->>Service: getFailedAiEmojiIds()
Service->>UploadMgr: getFailedAIEmojiIds()
UploadMgr->>Manager: [failedIds]
Manager->>Manager: Highlight failed items in UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @yabo083, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求旨在通过引入Web Worker进行客户端哈希计算和并发上传，以及将AI分析任务解耦并持久化到数据库，从而显著提升表情包上传的性能和用户体验。同时，新增的AI任务控制面板和状态指示器为用户提供了对AI分析过程的透明度和管理能力。 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly refactors the emoji upload and management system, primarily by introducing a robust AI analysis task queue and enhancing the user interface to reflect AI processing status. Key changes include refactoring the AddEmojiDialog.vue component to use Web Workers for client-side file hashing (for deduplication) and parallel uploads, which the reviewer suggests extracting into separate files for better maintainability and using Blob API for simpler buffer handling. The EmojiManager.vue component gains new UI elements: filter options for AI analysis status (unanalyzed, analyzed), an 'AI Stats' tag displaying task progress and status, an 'AI Reanalyze' button for selected emojis, and a dedicated 'AI Control Panel' dialog for managing AI task concurrency, batch delays, and retrying failed tasks. The reviewer noted a redundant onMounted hook and suggested implementing server-side filtering for AI analysis status to improve performance. On the backend, new console listeners and a /upload endpoint were added to support the new AI task management and file uploads, with the reviewer highlighting the need for authentication on the /upload endpoint and better type safety. The package.json was updated with formidable for file uploads. The core EmojiLunaService was extended to manage AI tasks, including a persistent task queue, duplicate image hash checking, and a background processor for AI analysis, with new database tables (emojiluna_ai_tasks, emojiluna_ai_results) to store task states and cached results. The reviewer pointed out that several AI_TASK_STATUS string literals should be replaced with defined constants for consistency and that the processAiTask function's task parameter should be strongly typed. Additionally, two methods, getDuplicateReason and updateConfig, were identified as unused and recommended for removal.
client/components/EmojiManager.vue
Outdated
Comment on lines
527
to
537
| if (result && filterStatus.value !== 'all') { | ||
| if (filterStatus.value === 'unanalyzed') { | ||
| result = result.filter((e: EmojiItem) => e.tags.length === 0 || (e.tags.length <= 1 && e.category === '其他')) | ||
| } else if (filterStatus.value === 'analyzed') { | ||
| result = result.filter((e: EmojiItem) => e.tags.length > 1 || e.category !== '其他') | ||
| } | ||
| // Handle client-side pagination for filtered results | ||
| total.value = result.length | ||
| const start = (currentPage.value - 1) * pageSize.value | ||
| emojis.value = result.slice(start, start + pageSize.value) | ||
| } else { |
There was a problem hiding this comment.
当前是在客户端进行状态过滤，这需要一次性获取大量数据（最多1000条），然后在前端进行分页和过滤。当表情包数量很大时，这可能会导致性能问题和内存占用过高。此外，如果未分析的表情包超过1000个，这个过滤器将无法显示全部结果。
建议：在后端实现按分析状态（analyzed, unanalyzed）进行过滤的功能，并支持分页。这样可以显著减少前端的负载和数据传输量，提高可伸缩性。
src/service.ts
Outdated
|
|
||
| async getAiTaskStats() { | ||
| const [pending, processing, succeeded, failed] = await Promise.all([ | ||
| this.ctx.database.select('emojiluna_ai_tasks').where({ status: 'pending' }).execute(row => $.count(row.id)), |
There was a problem hiding this comment.
这里直接使用了魔法字符串 'pending'。在文件顶部已经定义了 AI_TASK_STATUS 常量，建议在这里使用 AI_TASK_STATUS.PENDING 来代替，以提高代码的一致性和可维护性。这个问题在 getAiTaskStats, retryFailedTasks, processAiTask, startAiTaskProcessor 等多个函数中都存在。
this.ctx.database.select('emojiluna_ai_tasks').where({ status: AI_TASK_STATUS.PENDING }).execute(row => $.count(row.id)),
src/service.ts
Outdated
| return count | ||
| } | ||
|
|
||
| private async processAiTask(task: any) { |
There was a problem hiding this comment.
processAiTask 的 task 参数被声明为 any 类型。这个 task 对象来自于 emojiluna_ai_tasks 表，它的结构是已知的。建议为它定义一个明确的类型接口，或者直接使用 Tables 类型，这可以提高类型安全性和代码可读性。
private async processAiTask(task: Tables['emojiluna_ai_tasks']) {
src/backend.ts
Outdated
Comment on lines
398
to
481
| ctx.server.post(`${config.backendPath}/upload`, async (koa) => { | ||
| try { | ||
| // Check if body is already parsed by upstream middleware (e.g. koa-body) | ||
| const request = koa.request as any | ||
| let fields: any = {} | ||
| let files: any = {} | ||
| let file: any = null | ||
|
|
||
| if (request.files) { | ||
| // Already parsed | ||
| fields = request.body || {} | ||
| files = request.files | ||
| file = Array.isArray(files.file) ? files.file[0] : files.file | ||
| } else { | ||
| // Not parsed, use formidable | ||
| const storageDir = resolve(ctx.baseDir, config.storagePath, 'uploads') | ||
| await fs.mkdir(storageDir, { recursive: true }) | ||
|
|
||
| const form = formidable({ | ||
| uploadDir: storageDir, | ||
| keepExtensions: true, | ||
| maxFileSize: config.maxEmojiSize * 1024 * 1024, | ||
| multiples: false | ||
| }) | ||
|
|
||
| try { | ||
| const [parsedFields, parsedFiles] = await new Promise<[any, any]>((resolve, reject) => { | ||
| form.parse(koa.req, (err, fields, files) => { | ||
| if (err) reject(err) | ||
| else resolve([fields, files]) | ||
| }) | ||
| }) | ||
| fields = parsedFields | ||
| files = parsedFiles | ||
| file = Array.isArray(files.file) ? files.file[0] : files.file | ||
| } catch (err) { | ||
| ctx.logger.error(`Formidable parse error: ${err.message}`) | ||
| koa.status = 500 | ||
| koa.body = { success: false, message: `Upload parsing failed: ${err.message}` } | ||
| return | ||
| } | ||
| } | ||
|
|
||
| if (!file) { | ||
| ctx.logger.error('Upload failed: No file found in request') | ||
| throw new Error('No file uploaded') | ||
| } | ||
|
|
||
| // Extract metadata from fields | ||
| const name = Array.isArray(fields.name) ? fields.name[0] : fields.name | ||
| const category = Array.isArray(fields.category) ? fields.category[0] : fields.category | ||
| const tagsStr = Array.isArray(fields.tags) ? fields.tags[0] : fields.tags | ||
| const aiAnalysisStr = Array.isArray(fields.aiAnalysis) ? fields.aiAnalysis[0] : fields.aiAnalysis | ||
|
|
||
| let tags = [] | ||
| try { | ||
| tags = tagsStr ? JSON.parse(tagsStr) : [] | ||
| } catch (e) { | ||
| ctx.logger.warn(`Failed to parse tags JSON: ${tagsStr}`) | ||
| } | ||
| const aiAnalysis = aiAnalysisStr === 'true' | ||
|
|
||
| // Handle file path (formidable vs koa-body/multer differences) | ||
| // Formidable v3 uses filepath, some others use path | ||
| const filePath = file.filepath || file.path | ||
| if (!filePath) { | ||
| ctx.logger.error('Upload failed: File object missing path property', file) | ||
| throw new Error('Invalid file object received') | ||
| } | ||
|
|
||
| const emoji = await ctx.emojiluna.addEmojiFromPath({ | ||
| name: name || file.originalFilename?.replace(/\.[^/.]+$/, "") || file.name?.replace(/\.[^/.]+$/, "") || "uploaded", | ||
| category: category || '其他', | ||
| tags: tags | ||
| }, filePath, aiAnalysis) | ||
|
|
||
| koa.status = 200 | ||
| koa.body = { success: true, emoji } | ||
| } catch (err) { | ||
| ctx.logger.error(`Upload endpoint error: ${err.message}`, err.stack) | ||
| koa.status = 500 | ||
| koa.body = { success: false, message: err.message } | ||
| } | ||
| }) |
There was a problem hiding this comment.
This /upload endpoint is unauthenticated, posing a Denial of Service (DoS) risk by allowing any user to upload files and potentially exhaust disk space. Additionally, the extensive use of the any type for request, fields, files, and file within this function weakens TypeScript's type safety. Consider defining more specific types, for example, by importing File and Fields from formidable to improve code readability and robustness.
client/components/AddEmojiDialog.vue
Outdated
Comment on lines
344
to
524
| const hashWorkerScript = ` | ||
| self.onmessage = async (e) => { | ||
| const { files, sampleSize = 10240, concurrency = 4 } = e.data; | ||
| const results = []; | ||
| let idx = 0; | ||
|
|
||
| const readSample = async (file) => { | ||
| const size = file.size; | ||
| const needFull = size <= sampleSize * 3; | ||
| const parts = []; | ||
| if (needFull) { | ||
| parts.push(await file.arrayBuffer()); | ||
| } else { | ||
| const head = file.slice(0, sampleSize); | ||
| const tail = file.slice(size - sampleSize, size); | ||
| const midStart = Math.max(Math.floor(size / 2) - Math.floor(sampleSize / 2), sampleSize); | ||
| const mid = file.slice(midStart, midStart + sampleSize); | ||
| parts.push(await head.arrayBuffer()); | ||
| parts.push(await mid.arrayBuffer()); | ||
| parts.push(await tail.arrayBuffer()); | ||
| } | ||
| // concat | ||
| let totalLen = 0; | ||
| for (const p of parts) totalLen += p.byteLength; | ||
| const tmp = new Uint8Array(totalLen); | ||
| let offset = 0; | ||
| for (const p of parts) { | ||
| tmp.set(new Uint8Array(p), offset); | ||
| offset += p.byteLength; | ||
| } | ||
| const digest = await crypto.subtle.digest('SHA-256', tmp.buffer); | ||
| const hex = Array.from(new Uint8Array(digest)).map(b => b.toString(16).padStart(2, '0')).join(''); | ||
| return hex; | ||
| }; | ||
|
|
||
| const workerLoop = async () => { | ||
| while (true) { | ||
| const i = idx++; | ||
| if (i >= files.length) break; | ||
| const item = files[i]; | ||
| try { | ||
| const hash = await readSample(item.file); | ||
| self.postMessage({ type: 'hash', index: i, name: item.name, hash }); | ||
| } catch (err) { | ||
| self.postMessage({ type: 'error', index: i, name: item.name, error: err.message }); | ||
| } | ||
| } | ||
| const reader = new FileReader() | ||
| reader.readAsDataURL(file.raw) | ||
| reader.onload = () => { | ||
| const base64 = (reader.result as string).split(',')[1] | ||
| resolve({ | ||
| name: file.name.replace(/\.[^/.]+$/, ''), | ||
| category: form.category || '其他', | ||
| tags: form.tags, | ||
| imageData: base64, | ||
| //mimeType: file.raw.type, | ||
| }) | ||
| }; | ||
|
|
||
| const workers = []; | ||
| for (let w = 0; w < Math.min(concurrency, files.length); w++) { | ||
| workers.push(workerLoop()); | ||
| } | ||
| await Promise.all(workers); | ||
| self.postMessage({ type: 'done' }); | ||
| }; | ||
| ` | ||
|
|
||
| const hashBlob = new Blob([hashWorkerScript], { type: 'application/javascript' }); | ||
| const hashWorkerUrl = URL.createObjectURL(hashBlob); | ||
| const hashWorker = new Worker(hashWorkerUrl); | ||
|
|
||
| const hashes: { index: number; name: string; hash: string }[] = [] | ||
| const errors: any[] = [] | ||
|
|
||
| const hashPromise = new Promise<void>((resolve, reject) => { | ||
| hashWorker.onmessage = (e) => { | ||
| const data = e.data; | ||
| if (data.type === 'hash') { | ||
| hashes.push({ index: data.index, name: data.name, hash: data.hash }); | ||
| } else if (data.type === 'error') { | ||
| errors.push({ index: data.index, name: data.name, error: data.error }); | ||
| } else if (data.type === 'done') { | ||
| resolve(); | ||
| } | ||
| reader.onerror = (error) => reject(error) | ||
| }) | ||
| }; | ||
| hashWorker.onerror = (err) => reject(err); | ||
| }) | ||
|
|
||
| const emojisData = await Promise.all(filesToUpload) | ||
| await send('emojiluna/addEmojis', emojisData, form.aiAnalysis) | ||
| // Start hashing | ||
| hashWorker.postMessage({ files: filesRaw, sampleSize: 10240, concurrency: 4 }); | ||
| await hashPromise; | ||
| hashWorker.terminate(); | ||
| URL.revokeObjectURL(hashWorkerUrl); | ||
|
|
||
| if (errors.length > 0) { | ||
| console.warn('Some hash calculations failed:', errors); | ||
| } | ||
|
|
||
| // Deduplicate by hash within this batch | ||
| const seen = new Map<string, number>(); | ||
| const uniqueFiles: typeof filesRaw = []; | ||
| const duplicates: string[] = []; | ||
| // Map index -> hash | ||
| const indexHash = new Map<number, string>(); | ||
| for (const h of hashes) indexHash.set(h.index, h.hash); | ||
|
|
||
| filesRaw.forEach((item, i) => { | ||
| const hash = indexHash.get(i); | ||
| if (!hash) { | ||
| uniqueFiles.push(item); | ||
| return; | ||
| } | ||
| if (!seen.has(hash)) { | ||
| seen.set(hash, i); | ||
| uniqueFiles.push(item); | ||
| } else { | ||
| duplicates.push(item.name); | ||
| } | ||
| }); | ||
|
|
||
| if (duplicates.length > 0) { | ||
| ElMessage.info(`已在本次选择中去重 ${duplicates.length} 个重复文件`) | ||
| } | ||
|
|
||
| // 2) 准备上传唯一文件，使用原有的 upload worker 机制 | ||
| const baseUrl = await send('emojiluna/getBaseUrl') | ||
| let uploadUrl = `${baseUrl}/upload` | ||
| if (!uploadUrl.startsWith('http')) { | ||
| uploadUrl = new URL(uploadUrl, window.location.origin).toString() | ||
| } | ||
|
|
||
| const concurrency = 4 // Browser concurrency for uploads | ||
| const files = uniqueFiles.map(f => ({ | ||
| name: f.name, | ||
| category: f.category, | ||
| tags: f.tags, | ||
| aiAnalysis: f.aiAnalysis, | ||
| file: f.file | ||
| })) | ||
|
|
||
| const workerScript = ` | ||
| self.onmessage = async (e) => { | ||
| const { files, url, concurrency } = e.data; | ||
| let active = 0; | ||
| let index = 0; | ||
| let completed = 0; | ||
| let errors = []; | ||
|
|
||
| const processNext = async () => { | ||
| if (index >= files.length) return; | ||
| const currentIndex = index++; | ||
| const item = files[currentIndex]; | ||
| active++; | ||
|
|
||
| try { | ||
| const formData = new FormData(); | ||
| formData.append('file', item.file); | ||
| formData.append('name', item.name); | ||
| formData.append('category', item.category); | ||
| formData.append('tags', item.tags); | ||
| formData.append('aiAnalysis', item.aiAnalysis); | ||
|
|
||
| const response = await fetch(url, { | ||
| method: 'POST', | ||
| body: formData | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| const text = await response.text(); | ||
| throw new Error(\`Upload failed: \${response.status} \${text}\`); | ||
| } | ||
| self.postMessage({ type: 'progress', current: ++completed, total: files.length }); | ||
| } catch (err) { | ||
| errors.push({ file: item.name, error: err.message }); | ||
| console.error(\`Upload error for \${item.name}:\`, err); | ||
| } finally { | ||
| active--; | ||
| if (index < files.length) { | ||
| processNext(); | ||
| } else if (active === 0) { | ||
| self.postMessage({ type: 'done', errors }); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| for (let i = 0; i < Math.min(concurrency, files.length); i++) { | ||
| processNext(); | ||
| } | ||
| }; | ||
| ` |
There was a problem hiding this comment.
这两个内联的 Web Worker 脚本（hashWorkerScript 和 workerScript）使组件文件变得非常庞大和难以维护。将它们提取到单独的文件中（例如 src/workers/hash.worker.ts 和 src/workers/upload.worker.ts）可以带来以下好处：
- 代码分离：将UI逻辑与复杂的后台处理逻辑分开。
- 可维护性：独立的 worker 文件可以获得更好的语法高亮、linting 和类型检查。
- 可重用性：如果其他地方需要，这些 worker 更容易被重用。
虽然内联 worker 可以避免一些构建配置的麻烦，但对于这么复杂的逻辑，分离文件是更好的长期选择。
client/components/AddEmojiDialog.vue
Outdated
Comment on lines
350
to
377
| const readSample = async (file) => { | ||
| const size = file.size; | ||
| const needFull = size <= sampleSize * 3; | ||
| const parts = []; | ||
| if (needFull) { | ||
| parts.push(await file.arrayBuffer()); | ||
| } else { | ||
| const head = file.slice(0, sampleSize); | ||
| const tail = file.slice(size - sampleSize, size); | ||
| const midStart = Math.max(Math.floor(size / 2) - Math.floor(sampleSize / 2), sampleSize); | ||
| const mid = file.slice(midStart, midStart + sampleSize); | ||
| parts.push(await head.arrayBuffer()); | ||
| parts.push(await mid.arrayBuffer()); | ||
| parts.push(await tail.arrayBuffer()); | ||
| } | ||
| // concat | ||
| let totalLen = 0; | ||
| for (const p of parts) totalLen += p.byteLength; | ||
| const tmp = new Uint8Array(totalLen); | ||
| let offset = 0; | ||
| for (const p of parts) { | ||
| tmp.set(new Uint8Array(p), offset); | ||
| offset += p.byteLength; | ||
| } | ||
| const digest = await crypto.subtle.digest('SHA-256', tmp.buffer); | ||
| const hex = Array.from(new Uint8Array(digest)).map(b => b.toString(16).padStart(2, '0')).join(''); | ||
| return hex; | ||
| }; |
There was a problem hiding this comment.
在 readSample 函数中，手动拼接 ArrayBuffer 的方式有些繁琐且容易出错。可以使用 Blob API 来简化这个过程，代码会更简洁易读。
const readSample = async (file) => {
const size = file.size;
const needFull = size <= sampleSize * 3;
const parts = [];
if (needFull) {
parts.push(file);
} else {
const head = file.slice(0, sampleSize);
const tail = file.slice(size - sampleSize, size);
const midStart = Math.max(Math.floor(size / 2) - Math.floor(sampleSize / 2), sampleSize);
const mid = file.slice(midStart, midStart + sampleSize);
parts.push(head, mid, tail);
}
// Use Blob to concatenate file parts, which is cleaner than manual buffer manipulation.
const blob = new Blob(parts);
const buffer = await blob.arrayBuffer();
const digest = await crypto.subtle.digest('SHA-256', buffer);
const hex = Array.from(new Uint8Array(digest)).map(b => b.toString(16).padStart(2, '0')).join('');
return hex;
};
client/components/EmojiManager.vue
Outdated
Comment on lines
76
to
83
| <el-tag | ||
| v-if="totalTasks > 0" | ||
| :type="statusTagType" | ||
| style="margin-left: 8px; cursor: pointer; height: 32px; padding: 0 12px;" | ||
| effect="light" | ||
| round | ||
| @click="aiControlVisible = true" | ||
| > |
There was a problem hiding this comment.
这里使用了内联样式。为了更好的代码可维护性和关注点分离，建议将这些样式移到 <style> 块中，并使用一个 class 来应用它们。例如，添加一个 ai-stats-tag class，并在 <style> 部分定义它的样式。
<el-tag
v-if="totalTasks > 0"
:type="statusTagType"
class="ai-stats-tag"
effect="light"
round
@click="aiControlVisible = true"
>
src/service.ts
Outdated
Comment on lines
1237
to
1254
| private async getDuplicateReason( | ||
| imageBase64: string, | ||
| emojiId: string | ||
| ): Promise<string | null> { | ||
| const buffer = Buffer.from(imageBase64, 'base64') | ||
| const hash = this.calculateFileHash(buffer) | ||
|
|
||
| const existing = await this.ctx.database.get('emojiluna_emojis', { | ||
| id: { $ne: emojiId }, | ||
| image_hash: hash | ||
| }) | ||
|
|
||
| if (existing.length > 0) { | ||
| return `与现有表情包 ${existing[0].name} 重复` | ||
| } | ||
|
|
||
| return null | ||
| } |
There was a problem hiding this comment.
这个 getDuplicateReason 方法在代码库中没有被任何地方调用。如果它不是为未来的功能准备的，那么它就是死代码，应该被移除以保持代码库的整洁。
src/service.ts
Outdated
Comment on lines
1476
to
1479
| public updateConfig(config: Config) { | ||
| this.config = config | ||
| this.ctx.logger.info('EmojiLuna 配置已更新') | ||
| } |
There was a problem hiding this comment.
这个 updateConfig 方法没有在任何地方被调用。Koishi 插件有自己的配置热更新机制。这个方法似乎是多余的，建议移除。
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
client/components/AddEmojiDialog.vue (2)
302-303:⚠️ Potential issue | 🟡 MinorHardcoded Chinese strings bypass i18n.
handleUrlError(Line 302),handleSubmit(Line 316), and the dedup message (Line 456) use hardcoded Chinese text instead oft(...). This is inconsistent with the rest of the dialog that usesvue-i18n.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/components/AddEmojiDialog.vue` around lines 302 - 303, Replace the hardcoded Chinese strings in handleUrlError, handleSubmit, and the dedup message with i18n lookups using the t(...) function used elsewhere in the component; locate the functions handleUrlError and handleSubmit and the dedup message usage and swap the literal messages (e.g. '图片URL无效或无法加载' and the submit/dedup strings) for calls to t with appropriate translation keys (ensure the component imports/uses useI18n or the existing t() from setup and use consistent keys like 'emoji.invalidImageUrl', 'emoji.submitSuccess', 'emoji.duplicate' or similar), and update any ElMessage.* calls to pass t('...') instead of raw literals.
563-566:⚠️ Potential issue | 🔴 Critical
String.fromCharCode.applywill throwRangeErrorfor images larger than ~64 KB.
Function.prototype.applyhas a platform-dependent argument-count limit (commonly ~65 536). Any image exceeding that size will blow the call stack.Proposed fix using a chunked approach
- const uint8Array = new Uint8Array(buffer) - const base64 = btoa(String.fromCharCode.apply(null, Array.from(uint8Array))) + const uint8Array = new Uint8Array(buffer) + let binary = '' + const chunkSize = 8192 + for (let i = 0; i < uint8Array.length; i += chunkSize) { + binary += String.fromCharCode(...uint8Array.subarray(i, i + chunkSize)) + } + const base64 = btoa(binary)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/components/AddEmojiDialog.vue` around lines 563 - 566, The current conversion using btoa(String.fromCharCode.apply(null, Array.from(uint8Array))) will hit the apply argument limit for large images; change it to build the binary string in chunks (e.g., CHUNK = 0x8000) by looping over uint8Array, slicing into chunk arrays, converting each chunk with String.fromCharCode.apply(null, chunk) and concatenating parts into a full binary string, then call btoa on that string (update the code around uint8Array/base64 in AddEmojiDialog.vue to use this chunked approach).
🧹 Nitpick comments (6)
client/components/AddEmojiDialog.vue (2)
475-523: Upload worker:processNext()fire-and-forget infinallycan silently swallow thedonesignal when all uploads fail.If every single
processNext()call throws before reachingfinally(e.g., network-down scenario wherefetchrejects immediately), theactivecounter is still correctly decremented, so thedonemessage will eventually fire. However, theerrorsarray captured in the worker is never communicated back for partial-success scenarios that also hit theprocessNextrecursion. This looks functionally OK but is fragile.More importantly:
processNextisasyncbut called withoutawaitinsidefinally, meaning any rejection from the recursively spawned call is unhandled inside the worker and will surface as anunhandledrejectionon the worker global — which is not caught byworker.onerror. Consider adding a.catch():} finally { active--; if (index < files.length) { - processNext(); + processNext().catch(() => {}); } else if (active === 0) { self.postMessage({ type: 'done', errors }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/components/AddEmojiDialog.vue` around lines 475 - 523, The worker's async function processNext is invoked fire-and-forget inside the finally block which can create unhandled rejections and lose error context; modify the finally in processNext so the recursive call is handled (either await processNext() or call processNext().catch(...)) and ensure any error from that recursive invocation is captured into the same errors array and still results in the self.postMessage({ type: 'done', errors }) when active reaches 0; update identifiers: processNext, errors, active, index, self.postMessage to locate where to add .catch(...) (or convert to await) and push any recursive-call errors into errors before decrementing active and emitting done.
344-399: Inline worker scripts as template literals hurt maintainability and lose type-checking.Both the hash worker (~55 lines) and the upload worker (~50 lines) are embedded as raw strings. They cannot be linted, type-checked, or unit-tested. Consider extracting them into separate
.tsworker files and importing them, or at minimum into standalone string constants in a dedicated module, so they can be validated independently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/components/AddEmojiDialog.vue` around lines 344 - 399, The inline worker scripts (the hash worker containing readSample and workerLoop and the upload worker) should be moved out of the template literal into proper worker modules so they can be linted/type-checked and tested: create dedicated worker files (e.g., workers/hashWorker.ts and workers/uploadWorker.ts), move the self.onmessage handler, readSample, workerLoop and any helper logic into those files, export no default (keep using self.postMessage/self.onmessage), define and export shared message types/interfaces in a common module for strict typing, then replace the template-literal usage in AddEmojiDialog.vue with Worker instantiation using new Worker(new URL('./workers/hashWorker.ts', import.meta.url), { type: 'module' }) (and similarly for the upload worker), and update the code that posts/receives messages to use the moved message types.client/components/EmojiManager.vue (1)
912-925:getStatusreturns'success'for every analyzed emoji, causing all cards to display a green dot.If the vast majority of emojis are analyzed, every card will show a green status indicator, which adds visual noise without conveying useful information. Consider returning
undefinedfor analyzed emojis so only pending/error states are visible:Proposed change
- // Otherwise treat as success (analyzed) - return 'success' + // Analyzed emojis don't need a status indicator + return undefined🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/components/EmojiManager.vue` around lines 912 - 925, The getStatus function currently returns 'success' for analyzed emojis which makes the green indicator show for almost all items; update getStatus (the function referencing aiCategorizingId, failedEmojiIds, emoji.tags and emoji.category) so that after checking pending (aiCategorizingId) and error (failedEmojiIds) it returns undefined for emojis that are analyzed (i.e., have tags.length > 0 or a category not equal to '其他') instead of returning 'success' — only return 'pending' or 'error' explicitly and leave analyzed/default state as undefined so the UI shows indicators only for pending/error states.src/service.ts (2)
1519-1550: New table definitions use shorthand syntax while existing ones use long-form — consider consistency.
emojiluna_ai_tasksandemojiluna_ai_resultsuse shorthand ('string','integer') whileemojiluna_emojisandemojiluna_categoriesuse{ type: 'string', length: 254 }. The shorthand doesn't specify string length constraints, which could matter for database backends with strict column sizing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/service.ts` around lines 1519 - 1550, The new table declarations use shorthand types; update the ctx.database.extend calls for 'emojiluna_ai_tasks' and 'emojiluna_ai_results' to use the same long-form column descriptors as the other tables (e.g., { type: 'string', length: 254 } or appropriate length for text/integers) so column lengths and types are explicit; modify each field in the extend calls for 'emojiluna_ai_tasks' (id, emoji_id, image_path, image_hash, status, attempts, last_error, next_retry_at, created_at, updated_at) and for 'emojiluna_ai_results' (hash, result_json, created_at) to use { type: ..., length: ... } or { type: 'text' } for large blobs to match the pattern used by the emojiluna_emojis/emojiluna_categories definitions.
1237-1254: Remove the unusedgetDuplicateReasonmethod or add aTODOcomment explaining its intended purpose.The private
getDuplicateReasonmethod (lines 1237–1254) is never called anywhere in the codebase. While a similar method exists insrc/autoCollector.tsand is actively used, the one insrc/service.tsis dead code and should be removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/service.ts` around lines 1237 - 1254, The private method getDuplicateReason in src/service.ts is dead code; either delete the whole getDuplicateReason(imageBase64: string, emojiId: string): Promise<string | null> implementation from the class, or keep it but add a clear TODO comment above the method explaining its intended purpose and why it is unused (e.g., reference to the active implementation in autoCollector.ts) so future maintainers know whether to re-enable or remove it; locate the method by its name getDuplicateReason and update accordingly.src/backend.ts (1)
261-263: Parameterconfigshadows the outerconfig: ConfigfromapplyBackend.While functionally correct, this shadows the function's
configparameter and is confusing to read. Rename toruntimeConfigorcfg.Proposed fix
- ctx.console.addListener('emojiluna/setRuntimeConfig', async (config: any) => { - return ctx.emojiluna.setRuntimeConfig(config) + ctx.console.addListener('emojiluna/setRuntimeConfig', async (runtimeCfg: any) => { + return ctx.emojiluna.setRuntimeConfig(runtimeCfg) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend.ts` around lines 261 - 263, The callback parameter named config in the ctx.console.addListener('emojiluna/setRuntimeConfig', async (config: any) => ...) shadows the outer applyBackend(config: Config) parameter; rename the inner parameter to a non-conflicting name like runtimeConfig or cfg and update the call to ctx.emojiluna.setRuntimeConfig(...) accordingly so the outer applyBackend config remains unshadowed and the intent is clear (refer to ctx.console.addListener and emojiluna.setRuntimeConfig).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/components/EmojiManager.vue`:
- Around line 539-548: The loadEmojis logic double-fetches emojis to compute
total: when filterStatus.value === 'all' and no searchKeyword, after calling
send('emojiluna/getEmojiList', ...) with pagination it calls send again without
limits to get the full list; change this to either request total from the
paginated API (have the backend include a total field on
send('emojiluna/getEmojiList') responses) or call a new lightweight
send('emojiluna/getEmojiCount', { category, tags }) endpoint and set total.value
to that count; update the loadEmojis flow to use the returned total (or count)
instead of re-fetching all items and remove the second unbounded send call.
- Around line 349-358: The pause switch is doing an optimistic local mutation
via v-model="aiStats.paused" while `@change` calls send('emojiluna/setAiPaused',
!!val) with no rollback on failure; change the handler to an async method (e.g.,
onAiPausedChange) that receives the new value, stores the previous value, calls
send('emojiluna/setAiPaused', !!val) and on success leaves aiStats.paused as-is,
but on failure sets aiStats.paused back to the previous value and surfaces an
error (toast/log). Replace the inline `@change` lambda with
`@change`="onAiPausedChange" and ensure the method references aiStats.paused,
sends the RPC, and reverts state on rejection.
- Around line 622-627: The component currently registers onMounted twice causing
refreshData (and loadEmojis) to run twice; remove the duplicate onMounted
registration that directly calls refreshData (the second onMounted(refreshData)
entry) and keep the single onMounted(() => { refreshData(); updateAiStats();
aiStatsTimer = setInterval(updateAiStats, 5000); }) so updateAiStats scheduling
and aiStatsTimer setup remain intact; verify no other duplicate onMounted calls
for refreshData remain.
In `@package.json`:
- Line 33: Update the pinned dependency "formidable" in package.json from
"^3.5.1" to at least "3.5.3" (preferably "3.5.4"), then run your package manager
to refresh the lockfile (npm install or yarn install) so package-lock.json /
yarn.lock is updated; finally run tests and an audit (npm audit or yarn audit)
to ensure no remaining vulnerabilities related to CVE-2025-46653 remain.
In `@src/backend.ts`:
- Around line 398-481: Upload endpoint is unauthenticated allowing any client to
upload; add an auth gate to prevent unauthorized uploads. Modify the handler
registered via ctx.server.post (and the surrounding ctx.inject usage) to require
and validate a token or session before parsing files: check a configured secret
header (e.g., Authorization or X-Upload-Token from config) or integrate the
existing console auth middleware/session check at the start of the function and
return 401 on failure. Ensure the check occurs before any filesystem work
(before creating storageDir or calling formidable) and log rejected attempts;
keep the existing call to ctx.emojiluna.addEmojiFromPath intact once auth
passes.
- Around line 243-251: The listener registered for
'emojiluna/getFailedAiEmojiIds' is bypassing the service layer by querying
ctx.database directly; add a new method getFailedAiEmojiIds() on
EmojiLunaService that encapsulates the DB call and logging (mirror the provided
snippet behavior) and change the listener callback in
ctx.console.addListener('emojiluna/getFailedAiEmojiIds') to delegate to
ctx.emojiluna.getFailedAiEmojiIds() and return its result; ensure the service
method uses this.ctx.database.get('emojiluna_ai_tasks', { status: 'failed' }),
maps to emoji_id, filters falsy values, and logs warnings on errors using
this.ctx.logger.warn.
- Around line 441-444: The check that currently logs via ctx.logger.error and
then does throw new Error('No file uploaded') should return a 400 client error
instead of letting the outer catch produce a 500; replace the throw with an
explicit client-response (e.g., set ctx.status = 400 and ctx.body = { error: 'No
file uploaded' } and return) or use the framework helper (e.g., ctx.throw(400,
'No file uploaded')) so the handler short-circuits with a 400; keep the
ctx.logger.error call but ensure the code path uses ctx.status/ctx.throw or an
early return to avoid the outer catch converting this into a 500.
In `@src/service.ts`:
- Around line 1393-1474: There's a TOCTOU race in startAiTaskProcessor: it
counts processing tasks via the DB then calls processAiTask(task) which sets the
task status to "processing" asynchronously, allowing the loop to exceed
concurrency; fix by ensuring the in-memory active count is incremented before
dispatching and decremented when processAiTask finishes (or by awaiting the DB
status transition inside processAiTask before starting the next task).
Concretely, add an in-memory semaphore/counter (e.g.,
this._inMemoryProcessingCount) used alongside the DB check in
startAiTaskProcessor to reserve a slot before calling processAiTask, increment
it immediately when reserving, decrement it in processAiTask completion/failure
handlers, and ensure the concurrency calculation uses
Math.min(this._runtimeConfig.concurrency || this.config.aiConcurrency,
remainingSlots) so the loop cannot dispatch more tasks than allowed.
- Around line 454-473: In addEmoji, calculateFileHash(imageData) is called twice
(used for duplicate check and later for AI task persistence/DB upsert); compute
imageHash once at the start of the function (or at least before the
duplicate-check block), store it in a single function-scoped variable (e.g.,
imageHash) and reuse that variable everywhere (duplicate check, fs.writeFile
flow, the aiAnalysis && this.config.persistAiTasks branch, and the DB upsert
code) — remove the inner redeclaration of imageHash so there’s only one hash
computation used throughout addEmoji.
- Around line 325-371: The duplicate-detection must run before moving/removing
the original file: in addEmojiFromPath compute imageBuffer and imageHash (using
calculateFileHash) as you already do, then call
ctx.database.get('emojiluna_emojis', { image_hash: imageHash }) and throw if a
duplicate exists (so sourcePath is not touched); only after that perform
fs.mkdir + fs.rename (with EXDEV fallback to fs.copyFile/fs.unlink) and proceed
to write destPath; also ensure existing cleanup logic still removes destPath if
a later error occurs.
- Around line 1298-1326: The reanalyzeBatch method currently only checks for
existing tasks with status 'pending' and reads+hashes the file; update
reanalyzeBatch to query ctx.database.get('emojiluna_ai_tasks', { emoji_id: id,
status: ['pending','processing'] }) (or equivalent OR condition) so it skips
creating duplicates when a task is already processing, and stop reading
files/calling calculateFileHash for every emoji — instead fetch the existing
image_hash from the emojiluna_emojis record (use the emoji data in
this._emojiStorage or a direct query to 'emojiluna_emojis') and use that
image_hash in the create call, removing the fs.readFile and calculateFileHash
usage.
---
Outside diff comments:
In `@client/components/AddEmojiDialog.vue`:
- Around line 302-303: Replace the hardcoded Chinese strings in handleUrlError,
handleSubmit, and the dedup message with i18n lookups using the t(...) function
used elsewhere in the component; locate the functions handleUrlError and
handleSubmit and the dedup message usage and swap the literal messages (e.g.
'图片URL无效或无法加载' and the submit/dedup strings) for calls to t with appropriate
translation keys (ensure the component imports/uses useI18n or the existing t()
from setup and use consistent keys like 'emoji.invalidImageUrl',
'emoji.submitSuccess', 'emoji.duplicate' or similar), and update any ElMessage.*
calls to pass t('...') instead of raw literals.
- Around line 563-566: The current conversion using
btoa(String.fromCharCode.apply(null, Array.from(uint8Array))) will hit the apply
argument limit for large images; change it to build the binary string in chunks
(e.g., CHUNK = 0x8000) by looping over uint8Array, slicing into chunk arrays,
converting each chunk with String.fromCharCode.apply(null, chunk) and
concatenating parts into a full binary string, then call btoa on that string
(update the code around uint8Array/base64 in AddEmojiDialog.vue to use this
chunked approach).
---
Nitpick comments:
In `@client/components/AddEmojiDialog.vue`:
- Around line 475-523: The worker's async function processNext is invoked
fire-and-forget inside the finally block which can create unhandled rejections
and lose error context; modify the finally in processNext so the recursive call
is handled (either await processNext() or call processNext().catch(...)) and
ensure any error from that recursive invocation is captured into the same errors
array and still results in the self.postMessage({ type: 'done', errors }) when
active reaches 0; update identifiers: processNext, errors, active, index,
self.postMessage to locate where to add .catch(...) (or convert to await) and
push any recursive-call errors into errors before decrementing active and
emitting done.
- Around line 344-399: The inline worker scripts (the hash worker containing
readSample and workerLoop and the upload worker) should be moved out of the
template literal into proper worker modules so they can be linted/type-checked
and tested: create dedicated worker files (e.g., workers/hashWorker.ts and
workers/uploadWorker.ts), move the self.onmessage handler, readSample,
workerLoop and any helper logic into those files, export no default (keep using
self.postMessage/self.onmessage), define and export shared message
types/interfaces in a common module for strict typing, then replace the
template-literal usage in AddEmojiDialog.vue with Worker instantiation using new
Worker(new URL('./workers/hashWorker.ts', import.meta.url), { type: 'module' })
(and similarly for the upload worker), and update the code that posts/receives
messages to use the moved message types.
In `@client/components/EmojiManager.vue`:
- Around line 912-925: The getStatus function currently returns 'success' for
analyzed emojis which makes the green indicator show for almost all items;
update getStatus (the function referencing aiCategorizingId, failedEmojiIds,
emoji.tags and emoji.category) so that after checking pending (aiCategorizingId)
and error (failedEmojiIds) it returns undefined for emojis that are analyzed
(i.e., have tags.length > 0 or a category not equal to '其他') instead of
returning 'success' — only return 'pending' or 'error' explicitly and leave
analyzed/default state as undefined so the UI shows indicators only for
pending/error states.
In `@src/backend.ts`:
- Around line 261-263: The callback parameter named config in the
ctx.console.addListener('emojiluna/setRuntimeConfig', async (config: any) =>
...) shadows the outer applyBackend(config: Config) parameter; rename the inner
parameter to a non-conflicting name like runtimeConfig or cfg and update the
call to ctx.emojiluna.setRuntimeConfig(...) accordingly so the outer
applyBackend config remains unshadowed and the intent is clear (refer to
ctx.console.addListener and emojiluna.setRuntimeConfig).
In `@src/service.ts`:
- Around line 1519-1550: The new table declarations use shorthand types; update
the ctx.database.extend calls for 'emojiluna_ai_tasks' and
'emojiluna_ai_results' to use the same long-form column descriptors as the other
tables (e.g., { type: 'string', length: 254 } or appropriate length for
text/integers) so column lengths and types are explicit; modify each field in
the extend calls for 'emojiluna_ai_tasks' (id, emoji_id, image_path, image_hash,
status, attempts, last_error, next_retry_at, created_at, updated_at) and for
'emojiluna_ai_results' (hash, result_json, created_at) to use { type: ...,
length: ... } or { type: 'text' } for large blobs to match the pattern used by
the emojiluna_emojis/emojiluna_categories definitions.
- Around line 1237-1254: The private method getDuplicateReason in src/service.ts
is dead code; either delete the whole getDuplicateReason(imageBase64: string,
emojiId: string): Promise<string | null> implementation from the class, or keep
it but add a clear TODO comment above the method explaining its intended purpose
and why it is unused (e.g., reference to the active implementation in
autoCollector.ts) so future maintainers know whether to re-enable or remove it;
locate the method by its name getDuplicateReason and update accordingly.
src/backend.ts
Outdated
Comment on lines
243
to
251
| ctx.console.addListener('emojiluna/getFailedAiEmojiIds', async () => { | ||
| try { | ||
| const tasks = await ctx.database.get('emojiluna_ai_tasks', { status: 'failed' }) | ||
| return tasks.map((t: any) => t.emoji_id).filter(Boolean) | ||
| } catch (e) { | ||
| ctx.logger.warn(`Failed to fetch failed AI tasks: ${e.message}`) | ||
| return [] | ||
| } | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
getFailedAiEmojiIds queries the database directly, bypassing the service layer.
All other listeners delegate to ctx.emojiluna.*. This one queries ctx.database directly, which breaks the architectural pattern. Consider adding a getFailedAiEmojiIds() method to EmojiLunaService. Based on learnings, service layer should handle emoji storage, management, AI integration, and database operations in EmojiLunaService.
Proposed fix
ctx.console.addListener('emojiluna/getFailedAiEmojiIds', async () => {
- try {
- const tasks = await ctx.database.get('emojiluna_ai_tasks', { status: 'failed' })
- return tasks.map((t: any) => t.emoji_id).filter(Boolean)
- } catch (e) {
- ctx.logger.warn(`Failed to fetch failed AI tasks: ${e.message}`)
- return []
- }
+ return await ctx.emojiluna.getFailedAiEmojiIds()
})Then add to EmojiLunaService:
async getFailedAiEmojiIds(): Promise<string[]> {
try {
const tasks = await this.ctx.database.get('emojiluna_ai_tasks', { status: 'failed' })
return tasks.map(t => t.emoji_id).filter(Boolean)
} catch (e) {
this.ctx.logger.warn(`Failed to fetch failed AI tasks: ${e.message}`)
return []
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/backend.ts` around lines 243 - 251, The listener registered for
'emojiluna/getFailedAiEmojiIds' is bypassing the service layer by querying
ctx.database directly; add a new method getFailedAiEmojiIds() on
EmojiLunaService that encapsulates the DB call and logging (mirror the provided
snippet behavior) and change the listener callback in
ctx.console.addListener('emojiluna/getFailedAiEmojiIds') to delegate to
ctx.emojiluna.getFailedAiEmojiIds() and return its result; ensure the service
method uses this.ctx.database.get('emojiluna_ai_tasks', { status: 'failed' }),
maps to emoji_id, filters falsy values, and logs warnings on errors using
this.ctx.logger.warn.
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/components/EmojiManager.vue (1)
499-543:⚠️ Potential issue | 🟠 Major
filterStatusis never used inloadEmojis— the analyzed/unanalyzed filter buttons are non-functional.
handleFilterCommandsetsfilterStatus.valueand callsloadEmojis()(lines 668-671), butloadEmojisnever readsfilterStatusto apply any filtering. Clicking "仅显示未分析" or "仅显示已分析" resets the page but returns the same unfiltered results.You need to either pass
filterStatusto the backend as a query parameter, or apply client-side filtering based on it withinloadEmojis.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/components/EmojiManager.vue` around lines 499 - 543, loadEmojis currently ignores filterStatus so the "仅显示已/未分析" buttons (set by handleFilterCommand -> filterStatus) do nothing; update loadEmojis to read filterStatus.value and apply it: when calling send('emojiluna/getEmojiList', options) and send('emojiluna/getEmojiCount', ...) include a filter param (e.g., analyzed: filterStatus.value === 'analyzed' ? true : filterStatus.value === 'unanalyzed' ? false : undefined) so the backend can filter, and when using search mode (send('emojiluna/searchEmoji', ...)) apply the same client-side filtering to the result array before pagination (filter by e.analyzed matching filterStatus.value); ensure both list and total use the same filter logic.
🧹 Nitpick comments (9)
src/backend.ts (1)
244-253:getEmojiCountfetches the full emoji list just to return.length— inefficient at scale.This transfers and materializes every matching emoji row in memory only to count them. Consider adding a dedicated count method on the service layer that uses a database
COUNTquery or equivalent, especially as the emoji collection grows.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend.ts` around lines 244 - 253, The current handler registered at ctx.console.addListener('emojiluna/getEmojiCount') calls ctx.emojiluna.getEmojiList and returns list.length, which loads all rows into memory; change this to call a new service method (e.g., ctx.emojiluna.getEmojiCount or EmojiLunaService.countEmojis) that performs a DB COUNT query with the same filter options and returns the numeric count; update the backend listener to await that count method and log/return 0 on error as before, and implement the corresponding countEmojis/count method in the emojiluna service to use the database COUNT operation instead of pulling full records.client/components/EmojiManager.vue (2)
37-40: Filter dropdown items use hardcoded Chinese — should use i18n.
"重置筛选","显示全部","仅显示未分析","仅显示已分析", and"按分类筛选"(line 41) are hardcoded while the rest of the UI usest(...).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/components/EmojiManager.vue` around lines 37 - 40, Replace the hardcoded Chinese labels in the EmojiManager.vue dropdown items with i18n keys and the t(...) function (the items with commands "reset", "filter:all", "filter:unanalyzed", "filter:analyzed" and the nearby label "按分类筛选"); update each <el-dropdown-item> to call t('...') with descriptive keys (e.g. emoji.resetFilter, emoji.showAll, emoji.showUnanalyzed, emoji.showAnalyzed, emoji.filterByCategory), add those keys to the locale files, and keep the :disabled bindings (filterStatus) unchanged so the behavior remains identical.
585-649: Multiple hardcoded Chinese strings bypass i18n.Lines 591, 594, 601, 604, 613, 618, 641, 647 use hardcoded Chinese strings like
'配置已应用','AI 已暂停','已提交 ${count} 个表情包到 AI 分析队列', etc. Since the project has an i18n setup, these should uset(...)for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/components/EmojiManager.vue` around lines 585 - 649, Replace all hardcoded Chinese user-facing strings in applyAiConfig, retryFailedTasks, onAiPausedChange, and handleBatchReanalyze with i18n lookups (t(...)). Specifically, change ElMessage.success/error calls and any status labels to use t('key') and pass interpolation params where needed (e.g., t('ai.submittedCount', { count })) instead of template literals, and use t('ai.paused') / t('ai.resumed') (or your existing keys) for paused/resumed messages; ensure you import/use the t function from your i18n composable/context and preserve existing logic (including rollback in onAiPausedChange and clearing selection in handleBatchReanalyze).client/workers/hash.worker.ts (1)
3-3:resultsarray is declared but never used — dead code.- const results: any[] = []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/workers/hash.worker.ts` at line 3, The declared variable "results" in client/workers/hash.worker.ts is unused dead code; remove the const results: any[] = [] declaration (or, if the intention was to collect outputs, wire the appropriate logic into the existing handler/worker function to push items into results and return/emit it). Locate the unused symbol "results" in hash.worker.ts and either delete the declaration or implement its intended use within the worker's processing function so the variable is no longer unused.client/components/AddEmojiDialog.vue (1)
360-383:hashBlobUrlis alwaysnull— dead code left over from Blob URL approach.
hashBlobUrlis initialized tonull(line 360), never reassigned, so theif (hashBlobUrl) URL.revokeObjectURL(hashBlobUrl)guard on line 383 is always false. Same applies touploadBlobUrlon line 433/442. Remove these vestiges.Proposed fix
const hashWorker = new HashWorker() - const hashBlobUrl: string | null = null ... hashWorker.terminate() - if (hashBlobUrl) URL.revokeObjectURL(hashBlobUrl)const uploadWorker = new UploadWorker() - const uploadBlobUrl: string | null = null ... uploadWorker.terminate() - if (uploadBlobUrl) URL.revokeObjectURL(uploadBlobUrl) ... - if (uploadBlobUrl) URL.revokeObjectURL(uploadBlobUrl)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/components/AddEmojiDialog.vue` around lines 360 - 383, Remove the dead Blob URL code: delete the unused constant hashBlobUrl and uploadBlobUrl declarations and remove the corresponding conditional revokeObjectURL calls (the if (hashBlobUrl) URL.revokeObjectURL(hashBlobUrl) and the uploadBlobUrl revoke block) in AddEmojiDialog.vue; ensure you still terminate the worker (hashWorker.terminate()) and keep the hashing flow (hashWorker.postMessage / await hashPromise) intact so behavior is unchanged after removing these vestigial variables and guards.待改进.md (1)
1-202: Consider removing this review-digest file from the repository.Committing review feedback as a tracked file in the codebase is unconventional. These items would be better tracked as GitHub issues or in a project board. Shipping this in the repo adds noise and will quickly become stale.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@待改进.md` around lines 1 - 202, The file "待改进.md" contains PR review digests that should not be tracked in the codebase; remove the file from the commit (delete "待改进.md") and instead track these items as GitHub issues or on the project board and/or move any long-term guidance into a non-versioned place (e.g., a project board or an internal wiki); if you must keep a copy, convert it to a non-tracked archive (add to .gitignore or store in a docs-only location like .github/ or internal docs) and update the PR description or repo CONTRIBUTING.md to reference the canonical issue(s) instead.src/service.ts (3)
1371-1375: Redundant DB write: task is alreadyprocessingaftertryAtomicClaim.
tryAtomicClaim(called at line 1509 by the loop before dispatching this function) already setsstatus → processing. Re-writing the same status insideprocessAiTaskis a no-op extra DB round-trip.🔧 Proposed fix
- // Best-effort mark in DB - await this.ctx.database.set('emojiluna_ai_tasks', task.id, { - status: AI_TASK_STATUS.PROCESSING, - updated_at: Date.now() - }) - // Read image const buffer = await fs.readFile(task.image_path)If keeping
processAiTaskcallable outside the processor loop (e.g., for testing), retain the write but note the call-path already claims the task.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/service.ts` around lines 1371 - 1375, The DB write in processAiTask that calls this.ctx.database.set('emojiluna_ai_tasks', task.id, { status: AI_TASK_STATUS.PROCESSING, ... }) is redundant because tryAtomicClaim (invoked by the processor loop) already sets status → processing; remove that extra await this.ctx.database.set call from processAiTask to avoid the no-op round-trip, or if you need processAiTask to be independently callable (e.g., in tests), guard the write so it only runs when the task status is not already PROCESSING (check task.status) before calling this.ctx.database.set; reference functions tryAtomicClaim, processAiTask and the this.ctx.database.set('emojiluna_ai_tasks', task.id, ...) call when making the change.
1431-1434: Unnecessarytry/catcharound infallible operations infinally.
Set.prototype.delete()and integer subtraction/Math.max()cannot throw. Wrapping them intry {} catch (e) {}adds noise without any protective value.🔧 Proposed fix
} finally { - try { this._processingSet.delete(task.id) } catch (e) {} - try { this._localActiveCount = Math.max(0, this._localActiveCount - 1) } catch (e) {} + this._processingSet.delete(task.id) + this._localActiveCount = Math.max(0, this._localActiveCount - 1) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/service.ts` around lines 1431 - 1434, Remove the unnecessary try/catch wrappers in the finally block: directly call this._processingSet.delete(task.id) and assign this._localActiveCount = Math.max(0, this._localActiveCount - 1) without surrounding try/catch, since Set.prototype.delete and Math.max/number arithmetic are not going to throw; update the finally block around those lines (the locations using this._processingSet.delete and this._localActiveCount = Math.max(...)) to perform the operations plainly.
1278-1284: The$.count(row.id)syntax is correct for Koishi 4.18.x; consider consolidating into a single GROUP BY query for efficiency.The aggregate evaluator
$.count(row.id)in.execute()is valid minato API. However, four parallel database queries to count each status can be consolidated into a single query using.groupBy(row => row.status)to reduce database round-trips.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/service.ts` around lines 1278 - 1284, Consolidate the four separate count queries in getAiTaskStats into a single grouped query: use this.ctx.database.select('emojiluna_ai_tasks').groupBy(row => row.status).execute(...) with $.count(row.id) as the aggregator, then map the returned rows to AI_TASK_STATUS keys (AI_TASK_STATUS.PENDING, PROCESSING, SUCCEEDED, FAILED) to populate pending, processing, succeeded, failed variables; keep the select table name and $.count usage but replace the Promise.all with one DB call and default missing statuses to zero when constructing the result.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/components/AddEmojiDialog.vue`:
- Around line 314-320: The fallback chain in the mapping that builds the failed
files list is duplicated (it.file appears twice); update the map used to compute
the const failed in the error handling block (the arrow function mapping
result.errors) to use the correct property instead of the redundant it.file —
e.g., change it.file || it.fileName || it.file || 'unknown' to it.file ||
it.fileName || it.name || 'unknown' so the third fallback uses it.name (or the
appropriate file-name property).
- Around line 415-456: The upload worker is never given the configured upload
token so requests get 401; update the AddEmojiDialog flow to obtain the upload
token and include it in the worker message (add an uploadToken property to the
object passed to UploadWorker.postMessage), and update the UploadWorker
implementation (the fetch logic in upload.worker.ts) to read that token from the
posted message and set an 'x-upload-token' header on each fetch request; keep
existing abort/cleanup (uploadBlobUrl revoke and uploadWorker.terminate) logic
intact.
In `@client/components/EmojiManager.vue`:
- Around line 912-925: The getStatus function uses the hardcoded string '其他' to
detect an unanalyzed emoji; replace that fragile check with the canonical
default category value used across the app (either import the shared
DEFAULT_CATEGORY constant or read the translation key emojiluna.defaultCategory
via the existing i18n helper) so emoji.category comparison uses the
authoritative value; update the condition in getStatus (which references
aiCategorizingId, failedEmojiIds, emoji.tags, emoji.category) to compare against
that imported/derived default instead of the literal '其他'.
In `@client/workers/upload.worker.ts`:
- Around line 36-43: In the finally block where processNext() is invoked, avoid
fire-and-forget by attaching a rejection handler: call processNext().catch(err
=> { errors.push(err); /* optionally post error via
self.postMessage({type:'error', error: String(err)}) */ }); so any thrown error
(e.g., from processNext accessing files[currentIndex]) is caught and recorded;
update the finally branch that currently references active, files,
self.postMessage, and errors to use this .catch() wrapper instead of an
unhandled call.
- Around line 32-43: The progress counter only increments on success (completed
is incremented inside the try), so failed uploads stall the UI; move or
duplicate the increment so completed is advanced for every finished attempt:
increment completed (e.g., ++completed) in the catch or better in the finally
block before sending progress, and ensure the self.postMessage({ type:
'progress', current: completed, total: files.length }) is called for both
successes and failures; update references in upload.worker.ts (variables:
completed, files, processNext, active, errors) so errors.push still records
failures but progress always advances and the final done message is emitted when
active === 0.
In `@src/backend.ts`:
- Around line 506-519: In the catch block that handles upload errors, avoid
accessing err.message and err.stack before confirming err is an Error: first
check "if (err instanceof Error)" and call ctx.logger.error with the Error's
message and stack; otherwise call ctx.logger.error with a safe stringified
representation (e.g., String(err) or JSON.stringify fallback). Also update the
response assignment (koa.body.message) and any message checks used for status
decisions to use the safe message value (const msg = err instanceof Error ?
err.message : String(err)) so subsequent comparisons and status setting (the
existing checks for 'No file uploaded', 'parsing failed', '表情包已存在') operate on a
defined string.
In `@src/index.ts`:
- Line 92: The console event signature for 'emojiluna/getEmojiCount' currently
uses a weak any type for the options parameter; change the declaration of
'emojiluna/getEmojiCount' so that the parameter is a concrete type (e.g., define
an interface EmojiCountOptions and use it) or at minimum use unknown and perform
runtime validation in the handler; update the event declaration where
'emojiluna/getEmojiCount': (options: any) => Promise<number> appears to use
(options: EmojiCountOptions) => Promise<number> (or (options: unknown) =>
Promise<number>) and adjust callers/handler code to narrow/validate the shape of
options accordingly.
In `@src/service.ts`:
- Line 113: The call to startAiTaskProcessor() is currently invoked without
awaiting or handling rejections; make it an explicit fire-and-forget by invoking
it and attaching a rejection handler (e.g., startAiTaskProcessor().catch(...))
or using void startAiTaskProcessor().catch(...)), logging errors via the
existing logger (e.g., processLogger.error or this.logger) so any thrown errors
from startAiTaskProcessor are captured; locate the invocation of
startAiTaskProcessor and add the .catch(error => ...) handler that logs context
and the error.
- Around line 416-444: The branch in addEmojiFromPath silently skips AI analysis
when aiAnalysis is true but this.config.persistAiTasks is false and no cached
emojiluna_ai_results exists; also the aiTaskCreated flag is unused. Update
addEmojiFromPath so that when aiAnalysis is true and there is no cached result
and persistAiTasks is false it falls back to the same blocking analysis logic
used in addEmoji (mirror the else-if aiAnalysis handling in addEmoji), and
remove the dead variable aiTaskCreated; ensure you reference the
emojiluna_ai_results cache lookup, the persistAiTasks config check, and the
addEmoji blocking-analysis code path when copying behavior.
- Around line 531-540: When creating AI tasks in addEmoji and addEmojiFromPath
ensure the record includes next_retry_at: 0 (like reanalyzeBatch does) so new
tasks match the processor filter next_retry_at: { $lte: Date.now() } and don't
stall; modify the create calls that insert into 'emojiluna_ai_tasks' (the await
this.ctx.database.create(...) blocks) to set next_retry_at: 0 and, optionally,
add initial: 0 to the schema definition for next_retry_at in the table schema
(referenced near the schema block around line 1595) to make the default
explicit.
- Around line 63-79: The preferred-path in tryAtomicClaim unconditionally
returns true after calling op.where(...).set(...), but Koishi/Minato's set()
doesn't provide affected-row counts so this can claim a task that wasn't
updated; change the logic in tryAtomicClaim (the block that builds op =
dbAny.update('emojiluna_ai_tasks') and calls op.where(...).set(...)) to NOT
return true unconditionally — either inspect the set() result for an
affected-row indicator and only return true when rows were actually modified,
otherwise fall through to the existing read-check-set fallback; if the driver
provides no such indicator, remove the immediate return true so the code
proceeds to the safer fallback path that verifies status before setting.
---
Outside diff comments:
In `@client/components/EmojiManager.vue`:
- Around line 499-543: loadEmojis currently ignores filterStatus so the
"仅显示已/未分析" buttons (set by handleFilterCommand -> filterStatus) do nothing;
update loadEmojis to read filterStatus.value and apply it: when calling
send('emojiluna/getEmojiList', options) and send('emojiluna/getEmojiCount', ...)
include a filter param (e.g., analyzed: filterStatus.value === 'analyzed' ? true
: filterStatus.value === 'unanalyzed' ? false : undefined) so the backend can
filter, and when using search mode (send('emojiluna/searchEmoji', ...)) apply
the same client-side filtering to the result array before pagination (filter by
e.analyzed matching filterStatus.value); ensure both list and total use the same
filter logic.
---
Duplicate comments:
In `@client/components/EmojiManager.vue`:
- Line 927: There's a leftover commented duplicate call to
onMounted(refreshData) in EmojiManager.vue; remove that commented line so only
the single onMounted handler remains (the active onMounted that calls
refreshData() and updateAiStats()), ensuring the onMounted block that invokes
refreshData and updateAiStats is the sole initialization entry point.
- Around line 526-534: The frontend now calls the dedicated RPC
'emojiluna/getEmojiCount' which is good, but the backend handler for that RPC
(the function implementing getEmojiCount in backend.ts) still retrieves all
matching emoji rows and returns length; update that handler to perform an
efficient COUNT query (e.g., SQL COUNT or database count API) using the same
filters (category and tags) instead of fetching full records, and return the
numeric count so the client total.value stays correct without unnecessary data
transfer.
- Around line 608-620: onAiPausedChange correctly rolls back optimistic UI on
failure but should await the follow-up refresh and guard error formatting:
ensure you await updateAiStats() after send('emojiluna/setAiPaused', !!val) (so
UI is updated deterministically), and normalize the caught error when setting
ElMessage.error (e.g., extract e?.message ?? String(e)) to avoid undefined
messages; references: onAiPausedChange, aiStats, send, updateAiStats.
In `@src/backend.ts`:
- Around line 256-258: The listener handler currently wraps the service call in
an unnecessary async/await; change the callback passed to
ctx.console.addListener('emojiluna/getFailedAiEmojiIds', ...) to directly return
ctx.emojiluna.getFailedAiEmojiIds() (remove the async keyword and await) so the
value/promise is delegated to the service layer cleanly and errors propagate
unchanged.
- Around line 461-466: The handler correctly checks for missing file but mixes
contexts (uses ctx.logger.error then sets koa.status and koa.body), which can
cause runtime errors; change the response assignment to use ctx consistently
(replace koa.status and koa.body with ctx.status and ctx.body) alongside the
existing ctx.logger.error and the file variable check so the route returns a 400
with the proper body via ctx.
- Around line 405-420: The new upload endpoint enforces
runtimeConfig.uploadToken but the front-end upload worker used by
AddEmojiDialog.vue never sends this token, which will break uploads when a token
is configured; update the upload worker (the code path invoked by
AddEmojiDialog.vue's upload action) to include the token in requests (either set
the Authorization: Bearer <token> header or x-upload-token header taken from the
same config/source the UI uses), or alternatively make the endpoint accept
unauthenticated requests from that worker by adding a safe bypass (e.g., a
shared client key) if you prefer; ensure the change references the upload
request logic used by AddEmojiDialog.vue and runtimeConfig.uploadToken so the
header is always sent when a token is configured.
In `@src/service.ts`:
- Around line 1342-1343: In reanalyzeBatch, avoid reading each emoji file: query
the emojiluna_emojis table for the stored image_hash for each emoji (or
batch-select hashes for the emoji IDs) and use that value instead of calling
fs.readFile(emoji.path) and calculateFileHash; only fall back to reading
emoji.path and running calculateFileHash when the DB image_hash is missing or
null. Update the logic in reanalyzeBatch to use the DB-returned image_hash,
compare as needed, and remove the unconditional fs.readFile + calculateFileHash
path to eliminate unnecessary I/O.
---
Nitpick comments:
In `@client/components/AddEmojiDialog.vue`:
- Around line 360-383: Remove the dead Blob URL code: delete the unused constant
hashBlobUrl and uploadBlobUrl declarations and remove the corresponding
conditional revokeObjectURL calls (the if (hashBlobUrl)
URL.revokeObjectURL(hashBlobUrl) and the uploadBlobUrl revoke block) in
AddEmojiDialog.vue; ensure you still terminate the worker
(hashWorker.terminate()) and keep the hashing flow (hashWorker.postMessage /
await hashPromise) intact so behavior is unchanged after removing these
vestigial variables and guards.
In `@client/components/EmojiManager.vue`:
- Around line 37-40: Replace the hardcoded Chinese labels in the
EmojiManager.vue dropdown items with i18n keys and the t(...) function (the
items with commands "reset", "filter:all", "filter:unanalyzed",
"filter:analyzed" and the nearby label "按分类筛选"); update each <el-dropdown-item>
to call t('...') with descriptive keys (e.g. emoji.resetFilter, emoji.showAll,
emoji.showUnanalyzed, emoji.showAnalyzed, emoji.filterByCategory), add those
keys to the locale files, and keep the :disabled bindings (filterStatus)
unchanged so the behavior remains identical.
- Around line 585-649: Replace all hardcoded Chinese user-facing strings in
applyAiConfig, retryFailedTasks, onAiPausedChange, and handleBatchReanalyze with
i18n lookups (t(...)). Specifically, change ElMessage.success/error calls and
any status labels to use t('key') and pass interpolation params where needed
(e.g., t('ai.submittedCount', { count })) instead of template literals, and use
t('ai.paused') / t('ai.resumed') (or your existing keys) for paused/resumed
messages; ensure you import/use the t function from your i18n composable/context
and preserve existing logic (including rollback in onAiPausedChange and clearing
selection in handleBatchReanalyze).
In `@client/workers/hash.worker.ts`:
- Line 3: The declared variable "results" in client/workers/hash.worker.ts is
unused dead code; remove the const results: any[] = [] declaration (or, if the
intention was to collect outputs, wire the appropriate logic into the existing
handler/worker function to push items into results and return/emit it). Locate
the unused symbol "results" in hash.worker.ts and either delete the declaration
or implement its intended use within the worker's processing function so the
variable is no longer unused.
In `@src/backend.ts`:
- Around line 244-253: The current handler registered at
ctx.console.addListener('emojiluna/getEmojiCount') calls
ctx.emojiluna.getEmojiList and returns list.length, which loads all rows into
memory; change this to call a new service method (e.g.,
ctx.emojiluna.getEmojiCount or EmojiLunaService.countEmojis) that performs a DB
COUNT query with the same filter options and returns the numeric count; update
the backend listener to await that count method and log/return 0 on error as
before, and implement the corresponding countEmojis/count method in the
emojiluna service to use the database COUNT operation instead of pulling full
records.
In `@src/service.ts`:
- Around line 1371-1375: The DB write in processAiTask that calls
this.ctx.database.set('emojiluna_ai_tasks', task.id, { status:
AI_TASK_STATUS.PROCESSING, ... }) is redundant because tryAtomicClaim (invoked
by the processor loop) already sets status → processing; remove that extra await
this.ctx.database.set call from processAiTask to avoid the no-op round-trip, or
if you need processAiTask to be independently callable (e.g., in tests), guard
the write so it only runs when the task status is not already PROCESSING (check
task.status) before calling this.ctx.database.set; reference functions
tryAtomicClaim, processAiTask and the
this.ctx.database.set('emojiluna_ai_tasks', task.id, ...) call when making the
change.
- Around line 1431-1434: Remove the unnecessary try/catch wrappers in the
finally block: directly call this._processingSet.delete(task.id) and assign
this._localActiveCount = Math.max(0, this._localActiveCount - 1) without
surrounding try/catch, since Set.prototype.delete and Math.max/number arithmetic
are not going to throw; update the finally block around those lines (the
locations using this._processingSet.delete and this._localActiveCount =
Math.max(...)) to perform the operations plainly.
- Around line 1278-1284: Consolidate the four separate count queries in
getAiTaskStats into a single grouped query: use
this.ctx.database.select('emojiluna_ai_tasks').groupBy(row =>
row.status).execute(...) with $.count(row.id) as the aggregator, then map the
returned rows to AI_TASK_STATUS keys (AI_TASK_STATUS.PENDING, PROCESSING,
SUCCEEDED, FAILED) to populate pending, processing, succeeded, failed variables;
keep the select table name and $.count usage but replace the Promise.all with
one DB call and default missing statuses to zero when constructing the result.
In `@待改进.md`:
- Around line 1-202: The file "待改进.md" contains PR review digests that should
not be tracked in the codebase; remove the file from the commit (delete
"待改进.md") and instead track these items as GitHub issues or on the project board
and/or move any long-term guidance into a non-versioned place (e.g., a project
board or an internal wiki); if you must keep a copy, convert it to a non-tracked
archive (add to .gitignore or store in a docs-only location like .github/ or
internal docs) and update the PR description or repo CONTRIBUTING.md to
reference the canonical issue(s) instead.
Comment on lines
+314
to
+320
| // If any upload errors occurred, show only the failed notification and do not show success | ||
| if (result && result.errors && result.errors.length > 0) { | ||
| const failed = result.errors.map((it: any) => it.file || it.fileName || it.file || 'unknown') | ||
| ElMessage.warning(t('emojiluna.uploadPartialFailed', { | ||
| count: result.errors.length, | ||
| files: failed.slice(0, 5).join(', ') + (failed.length > 5 ? '...' : '') | ||
| })) |
There was a problem hiding this comment.
Duplicate fallback: it.file is checked twice in the || chain.
Line 316: it.file || it.fileName || it.file || 'unknown' — the third it.file is redundant. Likely meant to be it.name or another property.
Proposed fix
- const failed = result.errors.map((it: any) => it.file || it.fileName || it.file || 'unknown')
+ const failed = result.errors.map((it: any) => it.file || it.fileName || 'unknown')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // If any upload errors occurred, show only the failed notification and do not show success | |
| if (result && result.errors && result.errors.length > 0) { | |
| const failed = result.errors.map((it: any) => it.file || it.fileName || it.file || 'unknown') | |
| ElMessage.warning(t('emojiluna.uploadPartialFailed', { | |
| count: result.errors.length, | |
| files: failed.slice(0, 5).join(', ') + (failed.length > 5 ? '...' : '') | |
| })) | |
| // If any upload errors occurred, show only the failed notification and do not show success | |
| if (result && result.errors && result.errors.length > 0) { | |
| const failed = result.errors.map((it: any) => it.file || it.fileName || 'unknown') | |
| ElMessage.warning(t('emojiluna.uploadPartialFailed', { | |
| count: result.errors.length, | |
| files: failed.slice(0, 5).join(', ') + (failed.length > 5 ? '...' : '') | |
| })) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/components/AddEmojiDialog.vue` around lines 314 - 320, The fallback
chain in the mapping that builds the failed files list is duplicated (it.file
appears twice); update the map used to compute the const failed in the error
handling block (the arrow function mapping result.errors) to use the correct
property instead of the redundant it.file — e.g., change it.file || it.fileName
|| it.file || 'unknown' to it.file || it.fileName || it.name || 'unknown' so the
third fallback uses it.name (or the appropriate file-name property).
Comment on lines
+415
to
+456
| // 2) 准备上传唯一文件，使用原有的 upload worker 机制 | ||
| const baseUrl = await send('emojiluna/getBaseUrl') | ||
| let uploadUrl = `${baseUrl}/upload` | ||
| if (!uploadUrl.startsWith('http')) { | ||
| uploadUrl = new URL(uploadUrl, window.location.origin).toString() | ||
| } | ||
|
|
||
| const concurrency = 4 // Browser concurrency for uploads | ||
| const files = uniqueFiles.map(f => ({ | ||
| name: f.name, | ||
| category: f.category, | ||
| tags: f.tags, | ||
| aiAnalysis: f.aiAnalysis, | ||
| file: f.file | ||
| })) | ||
|
|
||
| // Initialize Upload Worker | ||
| const uploadWorker = new UploadWorker() | ||
| const uploadBlobUrl: string | null = null | ||
|
|
||
| return new Promise<{ errors: any[] }>((resolve, reject) => { | ||
| uploadWorker.onmessage = (e: any) => { | ||
| const { type, current, total, errors } = e.data | ||
| if (type === 'progress') { | ||
| // could update UI progress here | ||
| } else if (type === 'done') { | ||
| uploadWorker.terminate() | ||
| if (uploadBlobUrl) URL.revokeObjectURL(uploadBlobUrl) | ||
| if (errors && errors.length > 0) { | ||
| console.warn('Some uploads failed:', errors) | ||
| } | ||
| resolve({ errors: errors || [] }) | ||
| } | ||
| reader.onerror = (error) => reject(error) | ||
| }) | ||
| } | ||
|
|
||
| uploadWorker.onerror = (err) => { | ||
| uploadWorker!.terminate() | ||
| if (uploadBlobUrl) URL.revokeObjectURL(uploadBlobUrl) | ||
| reject(err) | ||
| } | ||
|
|
||
| uploadWorker.postMessage({ files, url: uploadUrl, concurrency }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the relevant files
fd -type f -name "*.worker.ts" -o -name "upload.worker.ts"Repository: ChatLunaLab/emojiluna
Length of output: 235
🏁 Script executed:
# Find backend.ts
fd -type f -name "backend.ts"Repository: ChatLunaLab/emojiluna
Length of output: 235
🏁 Script executed:
# Check for upload-related files
fd -type f -name "*upload*" | head -20Repository: ChatLunaLab/emojiluna
Length of output: 235
🏁 Script executed:
# Find upload worker and backend files
find . -name "*upload*.worker.ts" -o -name "upload.worker.ts" -o -name "backend.ts" 2>/dev/null | head -20Repository: ChatLunaLab/emojiluna
Length of output: 115
🏁 Script executed:
# Search for files containing fetch calls and token handling
rg -l "uploadWorker|UploadWorker" --type tsRepository: ChatLunaLab/emojiluna
Length of output: 47
🏁 Script executed:
# Search for backend.ts specifically
rg -l "x-upload-token|uploadToken" --type tsRepository: ChatLunaLab/emojiluna
Length of output: 93
🏁 Script executed:
# Read the upload worker file
cat -n ./client/workers/upload.worker.tsRepository: ChatLunaLab/emojiluna
Length of output: 1775
🏁 Script executed:
# Read backend.ts focusing on token validation (around lines 408-420)
wc -l ./src/backend.tsRepository: ChatLunaLab/emojiluna
Length of output: 85
🏁 Script executed:
# Read backend.ts lines 408-420 to check token validation
sed -n '408,420p' ./src/backend.tsRepository: ChatLunaLab/emojiluna
Length of output: 723
🏁 Script executed:
# Also check around the upload endpoint implementation
rg -n "x-upload-token|uploadToken|upload.*header" ./src/backend.ts -A 3 -B 3Repository: ChatLunaLab/emojiluna
Length of output: 856
🏁 Script executed:
# Check AddEmojiDialog.vue to see if token is passed to worker
rg -n "uploadWorker.postMessage" ./client/components/AddEmojiDialog.vue -A 2 -B 2Repository: ChatLunaLab/emojiluna
Length of output: 188
🏁 Script executed:
# Check if uploadToken is typically configured in config
rg -n "uploadToken" ./src/config.ts -B 3 -A 3Repository: ChatLunaLab/emojiluna
Length of output: 558
🏁 Script executed:
# Check if there's any token handling in AddEmojiDialog.vue
rg -n "token\|Token" ./client/components/AddEmojiDialog.vueRepository: ChatLunaLab/emojiluna
Length of output: 47
🏁 Script executed:
# Check if send() function has access to auth tokens in client
rg -n "send.*auth\|auth.*token" ./client --type ts -A 2 -B 2 | head -30Repository: ChatLunaLab/emojiluna
Length of output: 47
Upload worker does not send the x-upload-token header — uploads will fail with 401 when a token is configured.
The backend checks x-upload-token / authorization headers (backend.ts line 416), requiring the token if runtimeConfig.uploadToken is set. However, the upload worker's fetch call (upload.worker.ts line 22-25) sends no auth headers, and AddEmojiDialog.vue does not pass any token to the worker (line 456). When uploadToken is configured in the backend, every upload from the UI will be rejected.
Pass uploadToken from AddEmojiDialog to the worker and include it in the fetch headers as x-upload-token, or omit the token check for same-origin requests if the token is only needed for remote backends.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/components/AddEmojiDialog.vue` around lines 415 - 456, The upload
worker is never given the configured upload token so requests get 401; update
the AddEmojiDialog flow to obtain the upload token and include it in the worker
message (add an uploadToken property to the object passed to
UploadWorker.postMessage), and update the UploadWorker implementation (the fetch
logic in upload.worker.ts) to read that token from the posted message and set an
'x-upload-token' header on each fetch request; keep existing abort/cleanup
(uploadBlobUrl revoke and uploadWorker.terminate) logic intact.
Comment on lines
912
to
925
| // Determine per-emoji status for UI indicator | ||
| const getStatus = (emoji: EmojiItem): 'pending' | 'failed' | undefined => { | ||
| // If this emoji is currently being categorized in preview, mark as pending | ||
| if (aiCategorizingId.value && emoji.id === aiCategorizingId.value) return 'pending' | ||
|
|
||
| // If backend reports a failed AI task for this emoji, mark as failed | ||
| if (failedEmojiIds.value && failedEmojiIds.value.has(emoji.id)) return 'failed' | ||
|
|
||
| // Heuristic: mark as pending when it appears unanalyzed | ||
| if ((!emoji.tags || emoji.tags.length === 0) && (!emoji.category || emoji.category === '其他')) return 'pending' | ||
|
|
||
| // Otherwise treat as succeeded (analyzed) | ||
| return undefined | ||
| } |
There was a problem hiding this comment.
getStatus heuristic uses a hardcoded '其他' string — fragile and inconsistent with i18n.
Line 921 checks emoji.category === '其他' as a proxy for "unanalyzed." If the default category name ever changes (it's defined as an i18n key emojiluna.defaultCategory), this heuristic will silently break. Use a shared constant or the i18n value consistently.
Proposed fix
- if ((!emoji.tags || emoji.tags.length === 0) && (!emoji.category || emoji.category === '其他')) return 'pending'
+ if ((!emoji.tags || emoji.tags.length === 0) && (!emoji.category || emoji.category === t('emojiluna.defaultCategory'))) return 'pending'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/components/EmojiManager.vue` around lines 912 - 925, The getStatus
function uses the hardcoded string '其他' to detect an unanalyzed emoji; replace
that fragile check with the canonical default category value used across the app
(either import the shared DEFAULT_CATEGORY constant or read the translation key
emojiluna.defaultCategory via the existing i18n helper) so emoji.category
comparison uses the authoritative value; update the condition in getStatus
(which references aiCategorizingId, failedEmojiIds, emoji.tags, emoji.category)
to compare against that imported/derived default instead of the literal '其他'.
client/workers/upload.worker.ts
Outdated
Comment on lines
32
to
43
| self.postMessage({ type: 'progress', current: ++completed, total: files.length }) | ||
| } catch (err: any) { | ||
| errors.push({ file: item.name, error: err?.message || String(err) }) | ||
| console.error(`Upload error for ${item.name}:`, err) | ||
| } finally { | ||
| active-- | ||
| if (index < files.length) { | ||
| processNext() | ||
| } else if (active === 0) { | ||
| self.postMessage({ type: 'done', errors }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Progress counter skips failed uploads — UI progress can stall.
completed only increments on success (line 32). If some uploads fail, the progress will never reach total, potentially leaving the UI showing an incomplete progress indicator. Consider also incrementing completed in the catch or finally block so progress always advances.
Proposed fix
- self.postMessage({ type: 'progress', current: ++completed, total: files.length })
} catch (err: any) {
errors.push({ file: item.name, error: err?.message || String(err) })
console.error(`Upload error for ${item.name}:`, err)
} finally {
+ self.postMessage({ type: 'progress', current: ++completed, total: files.length })
active--📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| self.postMessage({ type: 'progress', current: ++completed, total: files.length }) | |
| } catch (err: any) { | |
| errors.push({ file: item.name, error: err?.message || String(err) }) | |
| console.error(`Upload error for ${item.name}:`, err) | |
| } finally { | |
| active-- | |
| if (index < files.length) { | |
| processNext() | |
| } else if (active === 0) { | |
| self.postMessage({ type: 'done', errors }) | |
| } | |
| } | |
| } catch (err: any) { | |
| errors.push({ file: item.name, error: err?.message || String(err) }) | |
| console.error(`Upload error for ${item.name}:`, err) | |
| } finally { | |
| self.postMessage({ type: 'progress', current: ++completed, total: files.length }) | |
| active-- | |
| if (index < files.length) { | |
| processNext() | |
| } else if (active === 0) { | |
| self.postMessage({ type: 'done', errors }) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/workers/upload.worker.ts` around lines 32 - 43, The progress counter
only increments on success (completed is incremented inside the try), so failed
uploads stall the UI; move or duplicate the increment so completed is advanced
for every finished attempt: increment completed (e.g., ++completed) in the catch
or better in the finally block before sending progress, and ensure the
self.postMessage({ type: 'progress', current: completed, total: files.length })
is called for both successes and failures; update references in upload.worker.ts
(variables: completed, files, processNext, active, errors) so errors.push still
records failures but progress always advances and the final done message is
emitted when active === 0.
src/service.ts
Outdated
| await this.initializeAI() | ||
| this._isInitialized = true | ||
| this._readyResolve() | ||
| this.startAiTaskProcessor() |
There was a problem hiding this comment.
startAiTaskProcessor() should be explicitly fire-and-forget with an error handler.
Calling an async function without await and without a .catch() means any unexpected rejection from the method is an unhandled Promise rejection.
🔧 Proposed fix
- this.startAiTaskProcessor()
+ void this.startAiTaskProcessor().catch(err =>
+ this.ctx.logger.error('AI Task Processor crashed:', err)
+ )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/service.ts` at line 113, The call to startAiTaskProcessor() is currently
invoked without awaiting or handling rejections; make it an explicit
fire-and-forget by invoking it and attaching a rejection handler (e.g.,
startAiTaskProcessor().catch(...)) or using void
startAiTaskProcessor().catch(...)), logging errors via the existing logger
(e.g., processLogger.error or this.logger) so any thrown errors from
startAiTaskProcessor are captured; locate the invocation of startAiTaskProcessor
and add the .catch(error => ...) handler that logs context and the error.
src/service.ts
Outdated
Comment on lines
531
to
540
| await this.ctx.database.create('emojiluna_ai_tasks', { | ||
| id: randomUUID(), | ||
| emoji_id: id, | ||
| image_path: filePath, | ||
| image_hash: imageHash, | ||
| status: AI_TASK_STATUS.PENDING, | ||
| created_at: Date.now(), | ||
| updated_at: Date.now(), | ||
| attempts: 0 | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the specific lines mentioned in the review
head -n 550 src/service.ts | tail -n 30Repository: ChatLunaLab/emojiluna
Length of output: 1528
🏁 Script executed:
# Check lines 433-442 for addEmojiFromPath
sed -n '433,442p' src/service.tsRepository: ChatLunaLab/emojiluna
Length of output: 493
🏁 Script executed:
# Check line 1354 for reanalyzeBatch
sed -n '1350,1360p' src/service.tsRepository: ChatLunaLab/emojiluna
Length of output: 327
🏁 Script executed:
# Check line 1487 for the processor query
sed -n '1485,1490p' src/service.tsRepository: ChatLunaLab/emojiluna
Length of output: 364
🏁 Script executed:
# Search for all occurrences of next_retry_at to understand the pattern
rg -n "next_retry_at" src/service.ts -B 2 -A 2Repository: ChatLunaLab/emojiluna
Length of output: 1197
🏁 Script executed:
# Check the schema definition for next_retry_at
rg -n "next_retry_at" src/service.ts | head -20Repository: ChatLunaLab/emojiluna
Length of output: 316
🏁 Script executed:
# Find the defineDatabase function and the schema definition
rg -n "defineDatabase\|next_retry_at.*type" src/service.ts -A 5Repository: ChatLunaLab/emojiluna
Length of output: 47
🏁 Script executed:
# Check if there's any documentation or pattern about default values in the codebase
rg -n "initial:" src/service.ts | head -20Repository: ChatLunaLab/emojiluna
Length of output: 47
🏁 Script executed:
# Check if there are any other integer fields with initial values in the schema
sed -n '1580,1600p' src/service.tsRepository: ChatLunaLab/emojiluna
Length of output: 790
🏁 Script executed:
# Search for any Koishi/Minato version info or configuration
fd package.json | head -5 | xargs grep -l "koishi\|minato" 2>/dev/nullRepository: ChatLunaLab/emojiluna
Length of output: 77
🏁 Script executed:
# Look at the full schema definition
sed -n '1575,1610p' src/service.tsRepository: ChatLunaLab/emojiluna
Length of output: 1210
Set next_retry_at: 0 when creating AI tasks to prevent them from stalling indefinitely in the queue.
The processor loop at line 1487 filters by next_retry_at: { $lte: Date.now() }. Both addEmoji (here, lines 531–540) and addEmojiFromPath (lines 433–442) omit next_retry_at when creating tasks, causing the field to be NULL. Since NULL ≤ Date.now() evaluates to false in SQL, these newly queued tasks will never match the processor's query and will stall indefinitely. reanalyzeBatch (line 1354) correctly sets it to 0 explicitly. Apply the same pattern to both task creation sites and optionally add initial: 0 to the schema definition at line 1595.
🔧 Proposed fix
// In addEmoji (lines 531–540):
await this.ctx.database.create('emojiluna_ai_tasks', {
...
attempts: 0,
+ next_retry_at: 0
})
// In addEmojiFromPath (lines 433–442):
await this.ctx.database.create('emojiluna_ai_tasks', {
...
attempts: 0,
+ next_retry_at: 0
})
// In schema definition (line 1595, optional but recommended):
- next_retry_at: { type: 'integer' },
+ next_retry_at: { type: 'integer', initial: 0 },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/service.ts` around lines 531 - 540, When creating AI tasks in addEmoji
and addEmojiFromPath ensure the record includes next_retry_at: 0 (like
reanalyzeBatch does) so new tasks match the processor filter next_retry_at: {
$lte: Date.now() } and don't stall; modify the create calls that insert into
'emojiluna_ai_tasks' (the await this.ctx.database.create(...) blocks) to set
next_retry_at: 0 and, optionally, add initial: 0 to the schema definition for
next_retry_at in the table schema (referenced near the schema block around line
1595) to make the default explicit.
Member
|
这几天有空我看看，还需要你多等一会噢 |
…ager and improve code quality Major Changes: - Created new UploadManager class to handle AI task orchestration, caching, and concurrent processing - Decoupled AI analysis from emoji upload flow for better separation of concerns - Implemented in-memory AI result caching to avoid redundant analysis - Moved AI task persistence and processing logic to dedicated UploadManager - Improved Web Workers (hash.worker.ts, upload.worker.ts) with better error handling and timeout support Backend Improvements: - Refactored service.ts to delegate AI task management to UploadManager - Simplified EmojiLunaService by removing ~600 lines of AI task processing logic - Enhanced file handling with proper EXDEV error handling for cross-device moves - Added proper hash validation and early duplicate detection - Improved database schema cleanup by removing AI task persistence tables UI/UX Enhancements: - Reformatted all Vue components for consistent indentation and readability - Updated emoji URL endpoints to use ID-based access for reliability - Improved drag-select composable with better coordinate calculations - Enhanced worker communication with improved error handling - Removed redundant AI configuration options from UI (concurrency, batch delay moved to backend-only) Code Quality: - Fixed ESLint configuration (removed redundant prettier options) - Unified code formatting across all TypeScript and Vue files - Improved error messages and logging consistency - Better TypeScript type definitions and imports - Simplified i18n configuration Performance: - Full-file SHA-256 hashing (removed sampling for accuracy) - Optimized concurrent upload processing with configurable timeouts - Reduced memory overhead by using in-process cache instead of DB for results - Parallel Web Worker processing for hash computation Breaking Changes: - Removed emojiluna_ai_tasks and emojiluna_ai_results database tables - Removed runtime config endpoints (setRuntimeConfig) - AI configuration now backend-only (not user-configurable via UI) - Changed emoji URL format from name-based to ID-based This refactor improves maintainability, reduces complexity, and enables better performance optimization.
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
client/components/TagDetail.vue (1)
374-390:⚠️ Potential issue | 🟡 MinorEmpty catch block silently swallows non-cancel errors.
Line 389 catches all errors including network failures. Only
ElMessageBoxcancel should be silently caught.Proposed fix
- } catch (e) {} + } catch (e) { + if (e !== 'cancel') { + console.error('Failed to delete emoji:', e) + ElMessage.error('删除失败') + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/components/TagDetail.vue` around lines 374 - 390, The empty catch in handleEmojiDelete swallows all errors; change it to only silently ignore user cancellation from ElMessageBox.confirm (check for 'cancel' or 'close' rejection values) and for other errors log and surface them (e.g., console.error(e) and ElMessage.error('删除失败: ' + String(e))). Keep the await send('emojiluna/deleteEmoji', emoji.id) and refreshData() behavior but ensure non-cancel/network errors are reported rather than swallowed.client/components/CategoriesManager.vue (1)
266-268:⚠️ Potential issue | 🟡 MinorInconsistent emoji URL construction: use
emoji.idconsistently across components.
CategoriesManager.vueconstructs the URL as/get/${emoji.name}whileEmojiCard.vueuses/get/${emoji.id}. Although the backend endpoint handles both patterns via fallback (tryinggetEmojiById()first, thengetEmojiByName()), this inconsistency reduces clarity. Unify to useemoji.idthroughout for consistency with the primary lookup pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/components/CategoriesManager.vue` around lines 266 - 268, The getEmojiUrl helper currently builds URLs using emoji.name; change it to use emoji.id for consistency with EmojiCard.vue and the primary backend lookup: update getEmojiUrl(emoji: EmojiItem) to return `${baseUrl.value}/get/${emoji.id}` and ensure EmojiItem provides id (or guard if id can be undefined) so all components use the same identifier pattern.src/service.ts (1)
570-578:⚠️ Potential issue | 🟡 MinorInconsistent default category:
'其他'here vs'Other'inaddEmoji/addEmojiFromPath.Line 574 uses
'其他'(Chinese) as the default category for batch-created emojis, whileaddEmojiFromPath(Line 408) andaddEmoji(Line 522) use'Other'(English). This inconsistency means batch-uploaded emojis default to a different category than individually uploaded ones.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/service.ts` around lines 570 - 578, The batch-upload path sets a default category of '其他' causing inconsistency with addEmoji and addEmojiFromPath which use 'Other'; update the batch-create logic (the createOptions construction where aiAnalysis triggers await this.addEmoji(...)) to use the same default string ('Other') as used in addEmoji and addEmojiFromPath so all upload flows share a consistent default category.client/components/TagsManager.vue (2)
262-268:⚠️ Potential issue | 🟠 MajorFallback SVG set on a hidden element — it will never render.
targetandimgare the sameevent.target. Line 264 setsdisplay = 'none'on the element, then line 266-267 assigns a fallbacksrcto that same hidden element. The fallback image is invisible. Additionally, settingimg.srcinside anonerrorhandler without first nulling the handler can re-triggeronerrorif the new src fails.Either remove the
display = 'none'line (letting the fallback SVG show inline), or remove theimg.srcassignment and instead updatetagCovers[tag.name]tonullso Vue'sv-elseplaceholder takes over.🐛 Proposed fix (show fallback inline)
const handleImageError = (event: Event) => { - const target = event.target as HTMLElement - target.style.display = 'none' - const img = event.target as HTMLImageElement + const img = event.target as HTMLImageElement + img.onerror = null // prevent infinite error loop img.src = 'data:image/svg+xml;base64,...' }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/components/TagsManager.vue` around lines 262 - 268, The onerror handler handleImageError hides the same element it then sets a fallback src on (event.target), so the fallback SVG will never be visible and may re-trigger onerror; fix by NOT hiding the element and instead either (A) remove the line that sets target.style.display = 'none' so the assigned fallback src will render, or (B) prefer a Vue-reactive approach: do not set img.src in handleImageError—instead clear the image's onerror to prevent loops and set the reactive tagCovers[tag.name] = null so the component falls back to the v-else placeholder; reference handleImageError, event.target/event.target as HTMLImageElement, and tagCovers[tag.name] when making the change.
258-260:⚠️ Potential issue | 🟠 MajorUse
emoji.idinstead ofemoji.namein the emoji URL.The
/get/endpoint is ID-based (seesrc/backend.ts:404). While the backend route has a fallback to resolve by name, the frontend should construct URLs usingemoji.idto align with the intended ID-based endpoint design. The backend's own generated URLs also useemoji.id(seesrc/backend.ts:37).Change in TagsManager.vue (lines 258-260)
const getEmojiUrl = (emoji: EmojiItem) => { return `${baseUrl.value}/get/${emoji.id}` }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/components/TagsManager.vue` around lines 258 - 260, Update the emoji URL builder to use the emoji's ID instead of its name: in the getEmojiUrl function (and any callers constructing `${baseUrl.value}/get/...`) use emoji.id from the EmojiItem to build `${baseUrl.value}/get/${emoji.id}` so the frontend targets the backend's ID-based /get/ endpoint (ensure baseUrl.value and EmojiItem references remain unchanged).
♻️ Duplicate comments (5)
client/workers/upload.worker.ts (2)
88-90: InitialprocessNext()calls are also fire-and-forget.Same unhandled-rejection risk as the recursive call on line 80.
Proposed fix
for (let i = 0; i < Math.min(concurrency, files.length); i++) { - processNext() + processNext().catch((err) => console.error('processNext error:', err)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/workers/upload.worker.ts` around lines 88 - 90, The initial calls to processNext inside the for loop are fire-and-forget and can produce unhandled rejections; change the loop that starts up to Math.min(concurrency, files.length) workers so you capture and await their Promises (e.g., collect each processNext() invocation into an array and await Promise.all on it) or attach proper .catch handling, ensuring processNext (the worker starter) errors are surfaced and handled for the initial concurrent launches over files with respect to the concurrency variable.
70-85: Progress counter still skips failed uploads — UI will stall;processNext()still fire-and-forget.Both issues from the prior review remain unaddressed:
completedonly increments on success (line 72). Failed uploads won't advance progress, so UI never reaches 100%.processNext()(line 80) returns a Promise that is never awaited or.catch()-ed.Proposed fix for both
try { await uploadWithTimeout( item.file, item.name, item.category, item.tags, String(item.aiAnalysis) ) - self.postMessage({ - type: 'progress', - current: ++completed, - total: files.length - }) } catch (err) { errors.push({ file: item.name, error: err?.message || String(err) }) } finally { + self.postMessage({ + type: 'progress', + current: ++completed, + total: files.length + }) active-- if (index < files.length && !done) { - processNext() + processNext().catch((err) => console.error('processNext error:', err)) } else if (active === 0 && !done) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/workers/upload.worker.ts` around lines 70 - 85, The progress counter only increments on success and processNext() is fire-and-forget; move the completed increment so it always increments (e.g., ++completed in the finally block before posting progress) and ensure processNext()'s returned Promise is handled (either await it if the surrounding function is async, or append .catch(err => { errors.push({ file: item.name || 'unknown', error: String(err) }); }) to processNext()) so rejections are captured and won't silently fail; update the finally logic around active, done, self.postMessage({type:'progress', current: completed, total: files.length}), and the processNext() invocation to use a caught/awaited call to avoid UI stalling and unhandled promise rejections.src/service.ts (1)
77-77:⚠️ Potential issue | 🟡 Minor
startAITaskProcessor()called fire-and-forget without.catch().An unhandled rejection from the async method before its internal loop starts would crash silently.
Proposed fix
- this._uploadManager.startAITaskProcessor() + this._uploadManager.startAITaskProcessor().catch(err => + this.ctx.logger.error('AI Task Processor crashed:', err) + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/service.ts` at line 77, The call to this._uploadManager.startAITaskProcessor() is fired without handling rejections; wrap the returned promise so any immediate rejection is caught and logged (e.g., append a .catch(...) or explicitly await it inside an async IIFE with try/catch) to prevent unhandled rejections from crashing the process; update the invocation of startAITaskProcessor() to catch errors and log them via the existing logger/processLogger so failures before the internal loop starts are reported.client/components/EmojiManager.vue (1)
1029-1048:getStatusstill uses hardcoded'其他'— fragile and inconsistent with i18n.Line 1042 checks
emoji.category === '其他'as a proxy for "unanalyzed." This was flagged in a previous review. Use a shared constant or the i18n value to avoid breakage if the default category name changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/components/EmojiManager.vue` around lines 1029 - 1048, getStatus currently treats emoji.category === '其他' as "unanalyzed" which is hardcoded and brittle; update getStatus to compare against the canonical unanalyzed/default category constant or i18n value instead of the literal string. Replace the literal '其他' check in getStatus (function) with a reference to the shared symbol (e.g., DEFAULT_CATEGORY or UNANALYZED_CATEGORY) or to the localized label from the i18n helper used elsewhere, ensuring aiCategorizingId, failedEmojiIds and EmojiItem logic remains unchanged. Import or access the shared constant or i18n key where EmojiManager.vue defines getStatus so the check stays correct if the default category name changes.src/backend.ts (1)
567-586:⚠️ Potential issue | 🟡 Minor
err.messageanderr.stackaccessed beforeinstanceof Errorguard — still present.Lines 569–570 access
err.messageanderr.stackunconditionally, buterris untyped. Theinstanceof Errorcheck only happens at Line 572. If a non-Error is thrown, these will beundefined. Line 586 also useserr.messageoutside the guard.Proposed fix
} catch (err) { - ctx.logger.error( - `Upload endpoint error: ${err.message}`, - err.stack - ) - if (err instanceof Error) { + const message = err instanceof Error ? err.message : String(err) + const stack = err instanceof Error ? err.stack : undefined + ctx.logger.error(`Upload endpoint error: ${message}`, stack) + if (err instanceof Error) { if ( err.message.includes('No file uploaded') || err.message.includes('parsing failed') ) { koa.status = 400 } else if (err.message.includes('表情包已存在')) { koa.status = 409 } else { koa.status = 500 } } else { koa.status = 500 } - koa.body = { success: false, message: err.message } + koa.body = { success: false, message }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend.ts` around lines 567 - 586, In the catch block around the upload endpoint where ctx.logger.error and koa.body are using err.message and err.stack, first narrow err with an instanceof Error check (or normalize it into const error = err instanceof Error ? err : new Error(String(err))) before accessing .message/.stack; then call ctx.logger.error with the safe message/stack and set koa.status based on error.message (falling back to 500 for non-Error cases), and set koa.body = { success: false, message: error.message } using the normalized error; refer to the catch scope, ctx.logger.error invocation, and the koa.body assignment to locate the changes.
🧹 Nitpick comments (4)
client/components/TagDetail.vue (1)
410-430: UnboundedPromise.allfor batch operations may overwhelm the backend.
handleBatchDelete(line 422),handleBatchRemoveTag(line 445), andhandleImportConfirm(line 465) all firePromise.allwith no concurrency limit. If a user selects hundreds of emojis, this sends hundreds of simultaneous requests. Consider batching with a concurrency limit (e.g.,p-limitor a simple chunking approach).Also applies to: 432-460, 462-480
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/components/TagDetail.vue` around lines 410 - 430, The batch handlers handleBatchDelete, handleBatchRemoveTag and handleImportConfirm currently call Promise.all over the entire selected list which can flood the backend; change them to use a bounded-concurrency approach (e.g., implement a simple chunking loop or use p-limit) that sends at most N concurrent send(...) requests (suggest N=5), by splitting selectedEmojis/import items into chunks and awaiting Promise.all for each chunk sequentially (or wrap send calls with a p-limit limiter) and keep existing success/failure handling, clearing selectedEmojis and refreshing only after all chunks complete.client/components/CategoriesManager.vue (1)
332-341: Remove verbose developer notes from production code.Lines 332–341 contain stream-of-consciousness reasoning about backend capabilities. Consider replacing with a concise
// TODO: implement updateCategory backend endpointif this is a known limitation, or remove entirely if not actionable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/components/CategoriesManager.vue` around lines 332 - 341, Remove the verbose inline developer notes inside the editingCategory.value branch and replace them with a concise, actionable comment or FIXME/TODO; specifically clean up the block around editingCategory.value where you currently call ElMessage.info('重命名分类功能暂未在后端实现') and the surrounding multi-line reasoning—either leave a single-line comment like // TODO: implement updateCategory backend endpoint or implement the proper backend call (e.g., add an updateCategory API) and update the branch to call that method instead.src/uploadManager.ts (2)
355-377:retryFailedTasksandreanalyzeBatchre-read files from disk to recalculate hashes.Both methods call
fs.readFile+calculateHashfor every emoji, even thoughimage_hashis already stored in the database. Consider retrieving the stored hash instead to avoid unnecessary disk I/O on potentially large files.Sketch for retryFailedTasks
for (const emojiId of failedIds) { const emoji = await this._taskProcessor.getEmojiById(emojiId) if (!emoji) continue - const buffer = await fs.readFile(emoji.path) - const hash = this.calculateHash(buffer) + // Retrieve stored hash from DB or extend getEmojiById to include image_hash + const [record] = await this.ctx.database.get('emojiluna_emojis', { id: emojiId }, { fields: ['image_hash'] }) + const hash = record?.image_hash || this.calculateHash(await fs.readFile(emoji.path))Note: This would require either extending
AITaskProcessor.getEmojiByIdto includeimage_hashor adding a direct DB query. The current approach works but is inefficient for large files.Also applies to: 384-407
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/uploadManager.ts` around lines 355 - 377, The retryFailedTasks and reanalyzeBatch functions currently call fs.readFile and calculateHash for each emoji; change them to use the persisted image_hash instead to avoid disk I/O by updating _taskProcessor.getEmojiById (or adding a new method like getEmojiWithHash) to return image_hash and use that value when populating _aiTaskQueue (and the reanalysis queue) instead of re-reading the file; if image_hash is missing/null, fall back to reading the file and computing calculateHash, then persist the computed hash back to the DB so subsequent runs use the stored image_hash.
454-474: Permanent failures are double-logged.When
attempts >= AIMaxAttempts, Line 458–460 logs a "permanently failed" warning, and then Line 468–470 also logs a generic "failed" warning for the same task. Consider usingelseorreturnto avoid the redundant log:Proposed fix
if (attempts >= this.config.AIMaxAttempts) { this._aiFailedIds.add(task.emojiId) this.ctx.logger.warn( `AI Task ${task.id} permanently failed after ${attempts} attempts: ${err?.message || err}` ) } else { const backoff = this.config.AIBackoffBase * Math.pow(2, attempts - 1) task.attempts = attempts task.nextRetryAt = Date.now() + backoff this._aiTaskQueue.push(task) + this.ctx.logger.warn( + `AI Task ${task.id} failed (attempt ${attempts}), retrying in ${backoff}ms: ${err?.message || err}` + ) } - this.ctx.logger.warn( - `AI Task ${task.id} failed: ${err?.message || err}` - )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/uploadManager.ts` around lines 454 - 474, The catch block double-logs permanent failures: when attempts >= this.config.AIMaxAttempts you log "permanently failed" and then unconditionally log "failed"; change the control flow so the generic this.ctx.logger.warn(`AI Task ${task.id} failed...`) only runs for non-permanent retries — either add an else around that generic log (so it executes only when you requeue the task) or return immediately after marking _aiFailedIds and logging permanent failure; keep the finally block as-is so _processingSet deletion and _localActiveCount adjustment always run.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/components/CategoryDetail.vue`:
- Around line 374-382: previewEmojiUrl and previewEmojiLink build download links
using previewEmoji.value.name which is fragile; update both computed properties
(previewEmojiUrl and previewEmojiLink) to use previewEmoji.value.id instead of
.name and keep the existing guard (if (!previewEmoji.value) return '') so the
URL becomes `${baseUrl.value}/get/${previewEmoji.value.id}` to match
EmojiManager's ID-based routing.
- Around line 385-427: The loadEmojis function currently makes a second
unbounded send('emojiluna/getEmojiList', { category: props.category.name }) just
to compute total count; change it to call the lightweight count endpoint by
sending send('emojiluna/getEmojiCount', { category: props.category.name }) and
assign total.value from that response (falling back to props.category.emojiCount
if needed), keeping the existing paged getEmojiList call for emojis.value and
preserving the searchKeyword branch logic in loadEmojis.
- Around line 662-697: The catch blocks (e.g., in handleAIAnalyze and other
handlers like handleBatchDelete) access error.message directly but TypeScript's
strict mode types catch params as unknown; update each catch to narrow the error
before reading .message — for example, inside handleAIAnalyze (and other
functions where send/async work is used and aiCategorizingId or similar state is
reset) derive a message with a safe check (if (error instanceof Error) use
error.message else String(error)) and use that variable in ElMessage.error and
console.error, ensuring aiCategorizingId.value is still cleared in the finally
block.
In `@client/components/EmojiCard.vue`:
- Around line 16-32: The ARIA attributes in the EmojiCard template are using
bare aria-hidden attributes which render as empty strings; update each
occurrence (the div with class "status-indicator", the span.status-backdrop, the
span.status-dot, and the conditional span.status-symbol) to set
aria-hidden="true" explicitly so assistive tech correctly recognizes them as
hidden; locate these elements in the EmojiCard.vue template by the surrounding
symbols/status-indicator, status-backdrop, status-dot, and status-symbol and
change their aria-hidden attributes accordingly.
In `@client/components/EmojiManager.vue`:
- Around line 622-663: loadEmojis currently ignores filterStatus so the UI
filter does nothing; modify loadEmojis to apply filtering whenever results are
obtained (both in the search branch where result is client-side list and in the
standard branch after fetching emojis.value) by using the getStatus heuristic on
each EmojiItem and keep only items matching filterStatus !== 'all' (e.g. include
only items where getStatus(e) === filterStatus for 'analyzed'/'unanalyzed'), and
update total.value to reflect the filtered length (or fetch count server-side if
you switch to server filtering); reference loadEmojis, filterStatus, getStatus,
emojis.value, total.value and ensure pagination logic accounts for the
client-side filter (slice after filtering).
In `@client/components/ImageSelector.vue`:
- Around line 226-261: The current client-side logic calls
send('emojiluna/getEmojiList', {}) to fetch the entire emoji set and then
filters by props.excludeCategory and filterCategory, which is a scalability
risk; update the call to pass a bounded query (e.g., include a limit/offset or
maxFetch param derived from pageSize and a sensible MAX_FETCH) and prefer
server-side support for excludeCategory (or add a short TODO to implement
backend excludeCategory in the emojiluna/getEmojiList handler), then apply
filtering/pagination using total, currentPage, pageSize and emojis from the
bounded response; also replace the long inline rationale comments with a single
concise TODO comment referencing props.excludeCategory and backend support.
In `@client/components/TagDetail.vue`:
- Around line 294-297: The preview URL currently builds with
previewEmoji.value.name but other components (e.g., EmojiCard.vue and
CategoriesManager) expect the backend to resolve by emoji ID; change the
computed previewEmojiUrl to use the emoji ID (previewEmoji.value.id) instead of
name and ensure previewEmoji and baseUrl are used as before so the URL becomes
`${baseUrl.value}/get/${previewEmoji.value.id}`; update any references or tests
that assumed name-based URLs to use id-based URLs consistently.
- Around line 500-513: The watcher on props.tagName uses { immediate: true } and
already calls loadEmojis() on mount, so remove the redundant onMounted(() =>
loadEmojis()) invocation; keep the watch block (which also resets currentPage,
searchKeyword, and selectedEmojis) and delete the onMounted call to avoid
double-loading in TagDetail.vue.
- Around line 322-330: The code currently calls send('emojiluna/getEmojiList',
options) and then calls send('emojiluna/getEmojiList', { tags: [props.tagName]
}) again to set total.value, causing duplicate requests; change the backend
response (or client call) so the paginated call returns total metadata (e.g., {
items, total }) and update the frontend to read total from that response: use
the first send('emojiluna/getEmojiList', options) result to set emojis.value =
result.items (or result) and total.value = result.total (or
result.pagination.total), removing the second send call and any use of
props.tagName for an unpaginated fetch.
In `@client/components/TagsManager.vue`:
- Around line 302-313: The loops that call send('emojiluna/updateEmojiTags',
...) sequentially (the delete flow around emojisWithTag and the rename flow in
the same file) can leave data partially updated on failure and are slow; change
both loops to map each emoji to a send(...) promise and run them with
Promise.all so updates happen in parallel, wrap the Promise.all in try/catch to
handle/report failures (using the same process/logger or ElMessage pattern) and
ensure refreshData() and success messages only run after the Promise.all
resolves; update references: the call sites using send('emojiluna/getEmojiList',
{ tags: [tag.name] }) and send('emojiluna/updateEmojiTags', emoji.id, newTags)
(and the analogous rename send calls) to implement the parallelized Promise.all
pattern.
In `@client/workers/upload.worker.ts`:
- Line 8: The variable errors is declared as a zero-length tuple (const errors:
[]) which prevents calling errors.push(...) in upload.worker.ts; change its type
to a proper array type such as const errors: { file: File | string; error:
unknown }[] = [] (or another appropriate shape for your error objects) so the
later push({ file: ..., error: ... }) call compiles; update any related uses of
errors to match the chosen element type.
In `@src/service.ts`:
- Around line 374-417: When aiAnalysis is true and a cached result exists, the
current logic still falls into the blocking analysis branch because
aiTaskCreated stays false; fix by ensuring the blocking path only runs when
there is neither a cached result nor a queued task. Update the condition around
the analyzeEmoji call (or introduce a hasCachedResult boolean from
this._uploadManager.getCachedAIResult) to require !aiTaskCreated && aiAnalysis
&& !hasCachedResult (or set aiTaskCreated = true when a cached result is
applied), referencing getCachedAIResult, queueAIAnalysis, analyzeEmoji and
finalOptions so cached results are respected and not overwritten.
- Around line 332-334: The private method calculateFileHash(buffer: Buffer) is
unused and duplicates logic already provided by
this._uploadManager.calculateHash(); remove the calculateFileHash method from
the class, search for any remaining references (e.g., in addEmoji and
addEmojiFromPath) and ensure those use this._uploadManager.calculateHash(buffer)
instead if needed; delete the unused method definition to avoid duplication and
dead code.
In `@src/uploadManager.ts`:
- Around line 163-166: The branch checking if mimeType is falsy is unreachable
because getImageType always returns a string; change the validation to
explicitly verify allowed types instead: call getImageType(imageData) into
mimeType and validate it against a whitelist of accepted MIME types (e.g.,
'image/jpeg','image/png','image/gif','image/webp') and return 'Invalid image
format' if not in that set, or alternatively modify getImageType to return null
for unknown signatures and keep the existing falsy check—refer to getImageType
and the mimeType variable in uploadManager.ts when making the change.
- Around line 100-126: The loadExistingHashes function currently limits database
reads to 10,000 rows (this.ctx.database.get with limit: 10000), which silently
drops hashes and breaks in-memory dedupe (emojiHashes/emojiHashMap) — fix by
removing the hard cap or implementing pagination: repeatedly call
this.ctx.database.get (preserving fields ['id','image_hash']) with a
cursor/offset/lastId until no more rows are returned and add each image_hash to
emojiHashes and emojiHashMap; keep the final logger message and error handling
intact so validateNewEmoji still benefits from a complete in-memory index.
---
Outside diff comments:
In `@client/components/CategoriesManager.vue`:
- Around line 266-268: The getEmojiUrl helper currently builds URLs using
emoji.name; change it to use emoji.id for consistency with EmojiCard.vue and the
primary backend lookup: update getEmojiUrl(emoji: EmojiItem) to return
`${baseUrl.value}/get/${emoji.id}` and ensure EmojiItem provides id (or guard if
id can be undefined) so all components use the same identifier pattern.
In `@client/components/TagDetail.vue`:
- Around line 374-390: The empty catch in handleEmojiDelete swallows all errors;
change it to only silently ignore user cancellation from ElMessageBox.confirm
(check for 'cancel' or 'close' rejection values) and for other errors log and
surface them (e.g., console.error(e) and ElMessage.error('删除失败: ' + String(e))).
Keep the await send('emojiluna/deleteEmoji', emoji.id) and refreshData()
behavior but ensure non-cancel/network errors are reported rather than
swallowed.
In `@client/components/TagsManager.vue`:
- Around line 262-268: The onerror handler handleImageError hides the same
element it then sets a fallback src on (event.target), so the fallback SVG will
never be visible and may re-trigger onerror; fix by NOT hiding the element and
instead either (A) remove the line that sets target.style.display = 'none' so
the assigned fallback src will render, or (B) prefer a Vue-reactive approach: do
not set img.src in handleImageError—instead clear the image's onerror to prevent
loops and set the reactive tagCovers[tag.name] = null so the component falls
back to the v-else placeholder; reference handleImageError,
event.target/event.target as HTMLImageElement, and tagCovers[tag.name] when
making the change.
- Around line 258-260: Update the emoji URL builder to use the emoji's ID
instead of its name: in the getEmojiUrl function (and any callers constructing
`${baseUrl.value}/get/...`) use emoji.id from the EmojiItem to build
`${baseUrl.value}/get/${emoji.id}` so the frontend targets the backend's
ID-based /get/ endpoint (ensure baseUrl.value and EmojiItem references remain
unchanged).
In `@src/service.ts`:
- Around line 570-578: The batch-upload path sets a default category of '其他'
causing inconsistency with addEmoji and addEmojiFromPath which use 'Other';
update the batch-create logic (the createOptions construction where aiAnalysis
triggers await this.addEmoji(...)) to use the same default string ('Other') as
used in addEmoji and addEmojiFromPath so all upload flows share a consistent
default category.
---
Duplicate comments:
In `@client/components/EmojiManager.vue`:
- Around line 1029-1048: getStatus currently treats emoji.category === '其他' as
"unanalyzed" which is hardcoded and brittle; update getStatus to compare against
the canonical unanalyzed/default category constant or i18n value instead of the
literal string. Replace the literal '其他' check in getStatus (function) with a
reference to the shared symbol (e.g., DEFAULT_CATEGORY or UNANALYZED_CATEGORY)
or to the localized label from the i18n helper used elsewhere, ensuring
aiCategorizingId, failedEmojiIds and EmojiItem logic remains unchanged. Import
or access the shared constant or i18n key where EmojiManager.vue defines
getStatus so the check stays correct if the default category name changes.
In `@client/workers/upload.worker.ts`:
- Around line 88-90: The initial calls to processNext inside the for loop are
fire-and-forget and can produce unhandled rejections; change the loop that
starts up to Math.min(concurrency, files.length) workers so you capture and
await their Promises (e.g., collect each processNext() invocation into an array
and await Promise.all on it) or attach proper .catch handling, ensuring
processNext (the worker starter) errors are surfaced and handled for the initial
concurrent launches over files with respect to the concurrency variable.
- Around line 70-85: The progress counter only increments on success and
processNext() is fire-and-forget; move the completed increment so it always
increments (e.g., ++completed in the finally block before posting progress) and
ensure processNext()'s returned Promise is handled (either await it if the
surrounding function is async, or append .catch(err => { errors.push({ file:
item.name || 'unknown', error: String(err) }); }) to processNext()) so
rejections are captured and won't silently fail; update the finally logic around
active, done, self.postMessage({type:'progress', current: completed, total:
files.length}), and the processNext() invocation to use a caught/awaited call to
avoid UI stalling and unhandled promise rejections.
In `@src/backend.ts`:
- Around line 567-586: In the catch block around the upload endpoint where
ctx.logger.error and koa.body are using err.message and err.stack, first narrow
err with an instanceof Error check (or normalize it into const error = err
instanceof Error ? err : new Error(String(err))) before accessing
.message/.stack; then call ctx.logger.error with the safe message/stack and set
koa.status based on error.message (falling back to 500 for non-Error cases), and
set koa.body = { success: false, message: error.message } using the normalized
error; refer to the catch scope, ctx.logger.error invocation, and the koa.body
assignment to locate the changes.
In `@src/service.ts`:
- Line 77: The call to this._uploadManager.startAITaskProcessor() is fired
without handling rejections; wrap the returned promise so any immediate
rejection is caught and logged (e.g., append a .catch(...) or explicitly await
it inside an async IIFE with try/catch) to prevent unhandled rejections from
crashing the process; update the invocation of startAITaskProcessor() to catch
errors and log them via the existing logger/processLogger so failures before the
internal loop starts are reported.
---
Nitpick comments:
In `@client/components/CategoriesManager.vue`:
- Around line 332-341: Remove the verbose inline developer notes inside the
editingCategory.value branch and replace them with a concise, actionable comment
or FIXME/TODO; specifically clean up the block around editingCategory.value
where you currently call ElMessage.info('重命名分类功能暂未在后端实现') and the surrounding
multi-line reasoning—either leave a single-line comment like // TODO: implement
updateCategory backend endpoint or implement the proper backend call (e.g., add
an updateCategory API) and update the branch to call that method instead.
In `@client/components/TagDetail.vue`:
- Around line 410-430: The batch handlers handleBatchDelete,
handleBatchRemoveTag and handleImportConfirm currently call Promise.all over the
entire selected list which can flood the backend; change them to use a
bounded-concurrency approach (e.g., implement a simple chunking loop or use
p-limit) that sends at most N concurrent send(...) requests (suggest N=5), by
splitting selectedEmojis/import items into chunks and awaiting Promise.all for
each chunk sequentially (or wrap send calls with a p-limit limiter) and keep
existing success/failure handling, clearing selectedEmojis and refreshing only
after all chunks complete.
In `@src/uploadManager.ts`:
- Around line 355-377: The retryFailedTasks and reanalyzeBatch functions
currently call fs.readFile and calculateHash for each emoji; change them to use
the persisted image_hash instead to avoid disk I/O by updating
_taskProcessor.getEmojiById (or adding a new method like getEmojiWithHash) to
return image_hash and use that value when populating _aiTaskQueue (and the
reanalysis queue) instead of re-reading the file; if image_hash is missing/null,
fall back to reading the file and computing calculateHash, then persist the
computed hash back to the DB so subsequent runs use the stored image_hash.
- Around line 454-474: The catch block double-logs permanent failures: when
attempts >= this.config.AIMaxAttempts you log "permanently failed" and then
unconditionally log "failed"; change the control flow so the generic
this.ctx.logger.warn(`AI Task ${task.id} failed...`) only runs for non-permanent
retries — either add an else around that generic log (so it executes only when
you requeue the task) or return immediately after marking _aiFailedIds and
logging permanent failure; keep the finally block as-is so _processingSet
deletion and _localActiveCount adjustment always run.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
.eslintrc.ymlclient/components/CategoriesManager.vueclient/components/CategoryDetail.vueclient/components/EmojiCard.vueclient/components/EmojiDialog.vueclient/components/EmojiManager.vueclient/components/FolderImportDialog.vueclient/components/ImageSelector.vueclient/components/TagDetail.vueclient/components/TagsManager.vueclient/composables/useDragSelect.tsclient/i18n.tsclient/icons/emoji.vueclient/workers/hash.worker.tsclient/workers/upload.worker.tssrc/backend.tssrc/config.tssrc/imageProcessor.tssrc/index.tssrc/service.tssrc/uploadManager.ts
✅ Files skipped from review due to trivial changes (3)
- client/composables/useDragSelect.ts
- client/components/EmojiDialog.vue
- client/components/FolderImportDialog.vue
🚧 Files skipped from review as they are similar to previous changes (2)
- client/i18n.ts
- src/config.ts
Comment on lines
374
to
382
| const previewEmojiUrl = computed(() => { | ||
| if (!previewEmoji.value) return '' | ||
| return `${baseUrl.value}/get/${previewEmoji.value.name}` | ||
| if (!previewEmoji.value) return '' | ||
| return `${baseUrl.value}/get/${previewEmoji.value.name}` | ||
| }) | ||
|
|
||
| const previewEmojiLink = computed(() => { | ||
| if (!previewEmoji.value) return '' | ||
| return `${baseUrl.value}/get/${previewEmoji.value.name}` | ||
| if (!previewEmoji.value) return '' | ||
| return `${baseUrl.value}/get/${previewEmoji.value.name}` | ||
| }) |
There was a problem hiding this comment.
URL uses name instead of id — inconsistent with EmojiManager.vue.
previewEmojiUrl and previewEmojiLink construct the URL with previewEmoji.value.name, while EmojiManager.vue (lines 604–611) uses previewEmoji.value.id. Using name is fragile: names may contain special characters, may not be unique, and the backend's /get/:id route resolves by ID first. Align with the ID-based approach for consistency and reliability.
const previewEmojiUrl = computed(() => {
if (!previewEmoji.value) return ''
- return `${baseUrl.value}/get/${previewEmoji.value.name}`
+ return `${baseUrl.value}/get/${previewEmoji.value.id}`
})
const previewEmojiLink = computed(() => {
if (!previewEmoji.value) return ''
- return `${baseUrl.value}/get/${previewEmoji.value.name}`
+ return `${baseUrl.value}/get/${previewEmoji.value.id}`
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const previewEmojiUrl = computed(() => { | |
| if (!previewEmoji.value) return '' | |
| return `${baseUrl.value}/get/${previewEmoji.value.name}` | |
| if (!previewEmoji.value) return '' | |
| return `${baseUrl.value}/get/${previewEmoji.value.name}` | |
| }) | |
| const previewEmojiLink = computed(() => { | |
| if (!previewEmoji.value) return '' | |
| return `${baseUrl.value}/get/${previewEmoji.value.name}` | |
| if (!previewEmoji.value) return '' | |
| return `${baseUrl.value}/get/${previewEmoji.value.name}` | |
| }) | |
| const previewEmojiUrl = computed(() => { | |
| if (!previewEmoji.value) return '' | |
| return `${baseUrl.value}/get/${previewEmoji.value.id}` | |
| }) | |
| const previewEmojiLink = computed(() => { | |
| if (!previewEmoji.value) return '' | |
| return `${baseUrl.value}/get/${previewEmoji.value.id}` | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/components/CategoryDetail.vue` around lines 374 - 382, previewEmojiUrl
and previewEmojiLink build download links using previewEmoji.value.name which is
fragile; update both computed properties (previewEmojiUrl and previewEmojiLink)
to use previewEmoji.value.id instead of .name and keep the existing guard (if
(!previewEmoji.value) return '') so the URL becomes
`${baseUrl.value}/get/${previewEmoji.value.id}` to match EmojiManager's ID-based
routing.
Comment on lines
385
to
+427
| const loadEmojis = async () => { | ||
| if (!props.category) return | ||
|
|
||
| loading.value = true | ||
| try { | ||
| const options = { | ||
| category: props.category.name, | ||
| limit: pageSize.value, | ||
| offset: (currentPage.value - 1) * pageSize.value | ||
| } | ||
| if (!props.category) return | ||
|
|
||
| let result | ||
| if (searchKeyword.value.trim()) { | ||
| // Client side filter after global search | ||
| const allResults = await send('emojiluna/searchEmoji', searchKeyword.value.trim()) | ||
| result = allResults?.filter((emoji: EmojiItem) => emoji.category === props.category?.name) || [] | ||
| emojis.value = result | ||
| total.value = result.length | ||
| } else { | ||
| result = await send('emojiluna/getEmojiList', options) | ||
| emojis.value = result || [] | ||
|
|
||
| // Get total count for category | ||
| // Optimization: The category object passed in prop has 'emojiCount'. | ||
| // But it might be stale. | ||
| // Let's rely on backend if we can, or just use the prop. | ||
| // Re-fetching category list to get fresh count might be better. | ||
| // Or separate API for count. | ||
| // For now, let's fetch all (limitless) to count? No, that's heavy. | ||
| // Let's use `getEmojiList` without limit for this category to count? Still heavy? | ||
| // Actually `getEmojiList` without limit returns all ids, which is okay for < 1000 items. | ||
| const allInCategory = await send('emojiluna/getEmojiList', { category: props.category.name }) | ||
| total.value = allInCategory?.length || 0 | ||
| } | ||
| loading.value = true | ||
| try { | ||
| const options = { | ||
| category: props.category.name, | ||
| limit: pageSize.value, | ||
| offset: (currentPage.value - 1) * pageSize.value | ||
| } | ||
|
|
||
| // Also load all categories for Move Dialog | ||
| if (allCategories.value.length === 0) { | ||
| allCategories.value = await send('emojiluna/getCategories') || [] | ||
| } | ||
| if (!baseUrl.value) { | ||
| baseUrl.value = await send('emojiluna/getBaseUrl') || '/emojiluna' | ||
| } | ||
| let result | ||
| if (searchKeyword.value.trim()) { | ||
| // Client side filter after global search | ||
| const allResults = await send( | ||
| 'emojiluna/searchEmoji', | ||
| searchKeyword.value.trim() | ||
| ) | ||
| result = | ||
| allResults?.filter( | ||
| (emoji: EmojiItem) => | ||
| emoji.category === props.category?.name | ||
| ) || [] | ||
| emojis.value = result | ||
| total.value = result.length | ||
| } else { | ||
| result = await send('emojiluna/getEmojiList', options) | ||
| emojis.value = result || [] | ||
|
|
||
| // Get total count for category | ||
| // Optimization: The category object passed in prop has 'emojiCount'. | ||
| // But it might be stale. | ||
| // Let's rely on backend if we can, or just use the prop. | ||
| // Re-fetching category list to get fresh count might be better. | ||
| // Or separate API for count. | ||
| // For now, let's fetch all (limitless) to count? No, that's heavy. | ||
| // Let's use `getEmojiList` without limit for this category to count? Still heavy? | ||
| // Actually `getEmojiList` without limit returns all ids, which is okay for < 1000 items. | ||
| const allInCategory = await send('emojiluna/getEmojiList', { | ||
| category: props.category.name | ||
| }) | ||
| total.value = allInCategory?.length || 0 | ||
| } |
There was a problem hiding this comment.
Double-fetch to get total count — use getEmojiCount like EmojiManager.vue.
Lines 423–426 make a second unbounded getEmojiList call solely to count items. EmojiManager.vue already fixed this by using the emojiluna/getEmojiCount endpoint. Apply the same approach here to halve the data transfer.
Proposed fix
} else {
result = await send('emojiluna/getEmojiList', options)
emojis.value = result || []
-
- const allInCategory = await send('emojiluna/getEmojiList', {
- category: props.category.name
- })
- total.value = allInCategory?.length || 0
+ total.value = await send('emojiluna/getEmojiCount', {
+ category: props.category.name
+ })
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/components/CategoryDetail.vue` around lines 385 - 427, The loadEmojis
function currently makes a second unbounded send('emojiluna/getEmojiList', {
category: props.category.name }) just to compute total count; change it to call
the lightweight count endpoint by sending send('emojiluna/getEmojiCount', {
category: props.category.name }) and assign total.value from that response
(falling back to props.category.emojiCount if needed), keeping the existing
paged getEmojiList call for emojis.value and preserving the searchKeyword branch
logic in loadEmojis.
Comment on lines
662
to
697
| const handleAIAnalyze = async () => { | ||
| if (!previewEmoji.value) return | ||
| if (!previewEmoji.value) return | ||
|
|
||
| aiCategorizingId.value = previewEmoji.value.id | ||
| aiCategorizingId.value = previewEmoji.value.id | ||
|
|
||
| try { | ||
| const result = await send('emojiluna/analyzeEmoji', previewEmoji.value.id) | ||
| try { | ||
| const result = await send( | ||
| 'emojiluna/analyzeEmoji', | ||
| previewEmoji.value.id | ||
| ) | ||
|
|
||
| if (result.success) { | ||
| const { updates, newData } = result | ||
| if (result.success) { | ||
| const { updates, newData } = result | ||
|
|
||
| if (updates.length > 0) { | ||
| ElMessage.success(`AI分析完成，已更新：${updates.join(', ')}`) | ||
| if (updates.length > 0) { | ||
| ElMessage.success(`AI分析完成，已更新：${updates.join(', ')}`) | ||
|
|
||
| if (previewEmoji.value) { | ||
| previewEmoji.value.category = newData.category | ||
| previewEmoji.value.tags = newData.tags | ||
| } | ||
| if (previewEmoji.value) { | ||
| previewEmoji.value.category = newData.category | ||
| previewEmoji.value.tags = newData.tags | ||
| } | ||
|
|
||
| await refreshData() | ||
| } else { | ||
| ElMessage.info('没有检测到需要更新的内容') | ||
| } | ||
| } else { | ||
| ElMessage.info(result.message || 'AI分析未返回结果') | ||
| await refreshData() | ||
| } else { | ||
| ElMessage.info('没有检测到需要更新的内容') | ||
| } | ||
| } else { | ||
| ElMessage.info(result.message || 'AI分析未返回结果') | ||
| } | ||
| } catch (error) { | ||
| console.error('AI分析失败:', error) | ||
| ElMessage.error(`AI分析失败: ${error.message || error}`) | ||
| } finally { | ||
| aiCategorizingId.value = '' | ||
| } | ||
| } catch (error) { | ||
| console.error('AI分析失败:', error) | ||
| ElMessage.error(`AI分析失败: ${error.message || error}`) | ||
| } finally { | ||
| aiCategorizingId.value = '' | ||
| } | ||
| } |
There was a problem hiding this comment.
error.message accessed without type narrowing in multiple catch blocks.
TypeScript strict mode types catch parameters as unknown. Accessing .message directly (Lines 693, also Line 52 in the backend error pattern) will fail type checking. This pattern also appears in handleBatchDelete and other handlers.
Example fix for `handleAIAnalyze`
} catch (error) {
console.error('AI分析失败:', error)
- ElMessage.error(`AI分析失败: ${error.message || error}`)
+ ElMessage.error(`AI分析失败: ${error instanceof Error ? error.message : String(error)}`)
} finally {As per coding guidelines, strict TypeScript configuration should be used throughout the project.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const handleAIAnalyze = async () => { | |
| if (!previewEmoji.value) return | |
| if (!previewEmoji.value) return | |
| aiCategorizingId.value = previewEmoji.value.id | |
| aiCategorizingId.value = previewEmoji.value.id | |
| try { | |
| const result = await send('emojiluna/analyzeEmoji', previewEmoji.value.id) | |
| try { | |
| const result = await send( | |
| 'emojiluna/analyzeEmoji', | |
| previewEmoji.value.id | |
| ) | |
| if (result.success) { | |
| const { updates, newData } = result | |
| if (result.success) { | |
| const { updates, newData } = result | |
| if (updates.length > 0) { | |
| ElMessage.success(`AI分析完成，已更新：${updates.join(', ')}`) | |
| if (updates.length > 0) { | |
| ElMessage.success(`AI分析完成，已更新：${updates.join(', ')}`) | |
| if (previewEmoji.value) { | |
| previewEmoji.value.category = newData.category | |
| previewEmoji.value.tags = newData.tags | |
| } | |
| if (previewEmoji.value) { | |
| previewEmoji.value.category = newData.category | |
| previewEmoji.value.tags = newData.tags | |
| } | |
| await refreshData() | |
| } else { | |
| ElMessage.info('没有检测到需要更新的内容') | |
| } | |
| } else { | |
| ElMessage.info(result.message || 'AI分析未返回结果') | |
| await refreshData() | |
| } else { | |
| ElMessage.info('没有检测到需要更新的内容') | |
| } | |
| } else { | |
| ElMessage.info(result.message || 'AI分析未返回结果') | |
| } | |
| } catch (error) { | |
| console.error('AI分析失败:', error) | |
| ElMessage.error(`AI分析失败: ${error.message || error}`) | |
| } finally { | |
| aiCategorizingId.value = '' | |
| } | |
| } catch (error) { | |
| console.error('AI分析失败:', error) | |
| ElMessage.error(`AI分析失败: ${error.message || error}`) | |
| } finally { | |
| aiCategorizingId.value = '' | |
| } | |
| } | |
| const handleAIAnalyze = async () => { | |
| if (!previewEmoji.value) return | |
| aiCategorizingId.value = previewEmoji.value.id | |
| try { | |
| const result = await send( | |
| 'emojiluna/analyzeEmoji', | |
| previewEmoji.value.id | |
| ) | |
| if (result.success) { | |
| const { updates, newData } = result | |
| if (updates.length > 0) { | |
| ElMessage.success(`AI分析完成，已更新：${updates.join(', ')}`) | |
| if (previewEmoji.value) { | |
| previewEmoji.value.category = newData.category | |
| previewEmoji.value.tags = newData.tags | |
| } | |
| await refreshData() | |
| } else { | |
| ElMessage.info('没有检测到需要更新的内容') | |
| } | |
| } else { | |
| ElMessage.info(result.message || 'AI分析未返回结果') | |
| } | |
| } catch (error) { | |
| console.error('AI分析失败:', error) | |
| ElMessage.error(`AI分析失败: ${error instanceof Error ? error.message : String(error)}`) | |
| } finally { | |
| aiCategorizingId.value = '' | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/components/CategoryDetail.vue` around lines 662 - 697, The catch
blocks (e.g., in handleAIAnalyze and other handlers like handleBatchDelete)
access error.message directly but TypeScript's strict mode types catch params as
unknown; update each catch to narrow the error before reading .message — for
example, inside handleAIAnalyze (and other functions where send/async work is
used and aiCategorizingId or similar state is reset) derive a message with a
safe check (if (error instanceof Error) use error.message else String(error))
and use that variable in ElMessage.error and console.error, ensuring
aiCategorizingId.value is still cleared in the finally block.
Comment on lines
+16
to
+32
| <!-- Status Indicator --> | ||
| <div | ||
| v-if="status" | ||
| class="status-indicator" | ||
| :data-status="status" | ||
| aria-hidden | ||
| > | ||
| <span class="status-backdrop" aria-hidden></span> | ||
| <span class="status-dot" aria-hidden></span> | ||
| <span | ||
| v-if="status === 'failed'" | ||
| class="status-symbol" | ||
| aria-hidden | ||
| > | ||
| × | ||
| </span> | ||
| </div> |
There was a problem hiding this comment.
aria-hidden needs explicit "true" value.
In Vue 3 templates, bare aria-hidden (without a value) renders as aria-hidden="", which is not a valid ARIA value. Assistive technologies may not interpret this as hidden. Use aria-hidden="true" on lines 21, 23, 24, and 28.
Proposed fix
<div
v-if="status"
class="status-indicator"
:data-status="status"
- aria-hidden
+ aria-hidden="true"
>
- <span class="status-backdrop" aria-hidden></span>
- <span class="status-dot" aria-hidden></span>
+ <span class="status-backdrop" aria-hidden="true"></span>
+ <span class="status-dot" aria-hidden="true"></span>
<span
v-if="status === 'failed'"
class="status-symbol"
- aria-hidden
+ aria-hidden="true"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <!-- Status Indicator --> | |
| <div | |
| v-if="status" | |
| class="status-indicator" | |
| :data-status="status" | |
| aria-hidden | |
| > | |
| <span class="status-backdrop" aria-hidden></span> | |
| <span class="status-dot" aria-hidden></span> | |
| <span | |
| v-if="status === 'failed'" | |
| class="status-symbol" | |
| aria-hidden | |
| > | |
| × | |
| </span> | |
| </div> | |
| <!-- Status Indicator --> | |
| <div | |
| v-if="status" | |
| class="status-indicator" | |
| :data-status="status" | |
| aria-hidden="true" | |
| > | |
| <span class="status-backdrop" aria-hidden="true"></span> | |
| <span class="status-dot" aria-hidden="true"></span> | |
| <span | |
| v-if="status === 'failed'" | |
| class="status-symbol" | |
| aria-hidden="true" | |
| > | |
| × | |
| </span> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/components/EmojiCard.vue` around lines 16 - 32, The ARIA attributes in
the EmojiCard template are using bare aria-hidden attributes which render as
empty strings; update each occurrence (the div with class "status-indicator",
the span.status-backdrop, the span.status-dot, and the conditional
span.status-symbol) to set aria-hidden="true" explicitly so assistive tech
correctly recognizes them as hidden; locate these elements in the EmojiCard.vue
template by the surrounding symbols/status-indicator, status-backdrop,
status-dot, and status-symbol and change their aria-hidden attributes
accordingly.
Comment on lines
622
to
+663
| const loadEmojis = async () => { | ||
| loading.value = true | ||
| try { | ||
| const options: EmojiSearchOptions = { | ||
| limit: pageSize.value, | ||
| offset: (currentPage.value - 1) * pageSize.value | ||
| loading.value = true | ||
| try { | ||
| const options: EmojiSearchOptions = { | ||
| limit: pageSize.value, | ||
| offset: (currentPage.value - 1) * pageSize.value | ||
| } | ||
|
|
||
| if (selectedCategory.value) { | ||
| options.category = selectedCategory.value | ||
| } | ||
|
|
||
| if (selectedTag.value) { | ||
| options.tags = [selectedTag.value] | ||
| } | ||
|
|
||
| let result | ||
| if (searchKeyword.value.trim()) { | ||
| result = await send( | ||
| 'emojiluna/searchEmoji', | ||
| searchKeyword.value.trim() | ||
| ) | ||
| // Client-side filtering for category/tags if search is used | ||
| if (selectedCategory.value) { | ||
| result = result.filter( | ||
| (e: EmojiItem) => e.category === selectedCategory.value | ||
| ) | ||
| } | ||
| // Client-side pagination for search results | ||
| total.value = result.length | ||
| const start = (currentPage.value - 1) * pageSize.value | ||
| emojis.value = result.slice(start, start + pageSize.value) | ||
| } else { | ||
| // Standard paginated fetch | ||
| emojis.value = await send('emojiluna/getEmojiList', options) | ||
|
|
||
| // Fetch total count separately for pagination | ||
| total.value = await send('emojiluna/getEmojiCount', { | ||
| category: selectedCategory.value || undefined, | ||
| tags: selectedTag.value ? [selectedTag.value] : undefined | ||
| }) | ||
| } |
There was a problem hiding this comment.
filterStatus is never consumed by loadEmojis — filter dropdown is non-functional.
The filter dropdown (Lines 31–73) lets users select "仅显示未分析" or "仅显示已分析", which sets filterStatus and calls loadEmojis(). However, loadEmojis never reads filterStatus.value to filter the results. The UI appears to offer filtering but it has no effect.
You need to apply the filter in loadEmojis, e.g. after fetching, filter based on the getStatus heuristic when filterStatus is not 'all':
Sketch
} else {
// Standard paginated fetch
- emojis.value = await send('emojiluna/getEmojiList', options)
+ let fetched = await send('emojiluna/getEmojiList', options)
+
+ if (filterStatus.value === 'unanalyzed') {
+ fetched = fetched.filter((e: EmojiItem) => getStatus(e) === 'pending')
+ } else if (filterStatus.value === 'analyzed') {
+ fetched = fetched.filter((e: EmojiItem) => getStatus(e) === undefined)
+ }
+
+ emojis.value = fetchedNote: Client-side filtering after paginated fetch will show fewer items than pageSize. For a proper solution, consider server-side filtering or fetch without pagination when filtering.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/components/EmojiManager.vue` around lines 622 - 663, loadEmojis
currently ignores filterStatus so the UI filter does nothing; modify loadEmojis
to apply filtering whenever results are obtained (both in the search branch
where result is client-side list and in the standard branch after fetching
emojis.value) by using the getStatus heuristic on each EmojiItem and keep only
items matching filterStatus !== 'all' (e.g. include only items where
getStatus(e) === filterStatus for 'analyzed'/'unanalyzed'), and update
total.value to reflect the filtered length (or fetch count server-side if you
switch to server filtering); reference loadEmojis, filterStatus, getStatus,
emojis.value, total.value and ensure pagination logic accounts for the
client-side filter (slice after filtering).
| let active = 0 | ||
| let index = 0 | ||
| let completed = 0 | ||
| const errors: [] = [] |
There was a problem hiding this comment.
Type bug: errors: [] is an empty tuple type — push() on line 76 won't compile under strict TypeScript.
const errors: [] = [] declares a zero-length tuple. The .push({ file: ..., error: ... }) call on line 76 is incompatible with this type. Use a proper array type.
Proposed fix
- const errors: [] = []
+ const errors: Array<{ file: string; error: string }> = []📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const errors: [] = [] | |
| const errors: Array<{ file: string; error: string }> = [] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/workers/upload.worker.ts` at line 8, The variable errors is declared
as a zero-length tuple (const errors: []) which prevents calling
errors.push(...) in upload.worker.ts; change its type to a proper array type
such as const errors: { file: File | string; error: unknown }[] = [] (or another
appropriate shape for your error objects) so the later push({ file: ..., error:
... }) call compiles; update any related uses of errors to match the chosen
element type.
Comment on lines
+332
to
+334
| private calculateFileHash(buffer: Buffer): string { | ||
| return createHash('sha256').update(buffer).digest('hex') | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify calculateFileHash is never called in the codebase
echo "=== Searching for calculateFileHash usage ==="
rg -n 'calculateFileHash' --type ts
echo -e "\n=== Searching for references in all file types ==="
rg -n 'calculateFileHash'Repository: ChatLunaLab/emojiluna
Length of output: 312
🏁 Script executed:
#!/bin/bash
# Verify addEmoji and addEmojiFromPath use _uploadManager.calculateHash instead
echo "=== Searching for _uploadManager.calculateHash usage ==="
rg -n '_uploadManager\.calculateHash' --type ts -A 2
echo -e "\n=== Looking at addEmoji and addEmojiFromPath implementations ==="
rg -n 'addEmoji|addEmojiFromPath' src/service.ts -A 5 -B 1Repository: ChatLunaLab/emojiluna
Length of output: 2282
Remove unused calculateFileHash method — it's completely unused in the codebase.
The method duplicates SHA-256 logic already provided by this._uploadManager.calculateHash(), which is used in both addEmoji (line 347) and addEmojiFromPath (line 462).
Proposed fix
- private calculateFileHash(buffer: Buffer): string {
- return createHash('sha256').update(buffer).digest('hex')
- }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private calculateFileHash(buffer: Buffer): string { | |
| return createHash('sha256').update(buffer).digest('hex') | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/service.ts` around lines 332 - 334, The private method
calculateFileHash(buffer: Buffer) is unused and duplicates logic already
provided by this._uploadManager.calculateHash(); remove the calculateFileHash
method from the class, search for any remaining references (e.g., in addEmoji
and addEmojiFromPath) and ensure those use
this._uploadManager.calculateHash(buffer) instead if needed; delete the unused
method definition to avoid duplication and dead code.
Comment on lines
374
to
+417
| let finalOptions = { ...options } | ||
|
|
||
| // Try cache lookup first if AI is requested | ||
| let aiTaskCreated = false | ||
| if (aiAnalysis) { | ||
| const imageBase64 = imageData.toString('base64') | ||
| const cachedResult = | ||
| this._uploadManager.getCachedAIResult(imageHash) | ||
| if (cachedResult) { | ||
| finalOptions = { | ||
| name: cachedResult.name || options.name, | ||
| category: | ||
| cachedResult.category || options.category || 'Other', | ||
| tags: [ | ||
| ...new Set([ | ||
| ...(options.tags || []), | ||
| ...(cachedResult.tags || []) | ||
| ]) | ||
| ], | ||
| description: cachedResult.description | ||
| } | ||
| } else { | ||
| // Queue for async AI analysis | ||
| this._uploadManager.queueAIAnalysis(id, destPath, imageHash) | ||
| aiTaskCreated = true | ||
| } | ||
| } | ||
|
|
||
| if (!aiTaskCreated && aiAnalysis) { | ||
| // No cached result and no task created - do legacy blocking analysis | ||
| const imageBase64 = imageBuffer.toString('base64') | ||
| const aiResult = await this.analyzeEmoji(imageBase64) | ||
| if (aiResult) { | ||
| finalOptions = { | ||
| name: aiResult.name || options.name, | ||
| category: aiResult.category || options.category || '其他', | ||
| category: aiResult.category || options.category || 'Other', | ||
| tags: [ | ||
| ...new Set([...(options.tags || []), ...aiResult.tags]) | ||
| ], | ||
| description: aiResult.description | ||
| } | ||
| } else { | ||
| throw new Error('AI分析失败，无法添加表情包') | ||
| throw new Error('AI analysis failed, unable to add emoji') | ||
| } | ||
| } |
There was a problem hiding this comment.
Logic bug: when a cached AI result is found, blocking analysis overwrites it.
Walk through the flow when aiAnalysis=true and a cached result exists:
- Line 381:
cachedResultis truthy →finalOptionsis updated with cached data. - Line 397:
aiTaskCreatedremainsfalse(only set totruein theelsebranch). - Line 401:
!aiTaskCreated && aiAnalysis→!false && true→true→ falls into the blocking analysis path, which overwritesfinalOptionswith a fresh (and redundant) AI call, or throws if analysis fails.
The condition on Line 401 should be the inverse — it should only trigger when there's no cached result AND no task was created:
Proposed fix
- let aiTaskCreated = false
if (aiAnalysis) {
const cachedResult =
this._uploadManager.getCachedAIResult(imageHash)
if (cachedResult) {
finalOptions = {
name: cachedResult.name || options.name,
category:
cachedResult.category || options.category || 'Other',
tags: [
...new Set([
...(options.tags || []),
...(cachedResult.tags || [])
])
],
description: cachedResult.description
}
} else {
// Queue for async AI analysis
this._uploadManager.queueAIAnalysis(id, destPath, imageHash)
- aiTaskCreated = true
}
}
-
- if (!aiTaskCreated && aiAnalysis) {
- // No cached result and no task created - do legacy blocking analysis
- const imageBase64 = imageBuffer.toString('base64')
- const aiResult = await this.analyzeEmoji(imageBase64)
- if (aiResult) {
- finalOptions = {
- name: aiResult.name || options.name,
- category: aiResult.category || options.category || 'Other',
- tags: [
- ...new Set([...(options.tags || []), ...aiResult.tags])
- ],
- description: aiResult.description
- }
- } else {
- throw new Error('AI analysis failed, unable to add emoji')
- }
- }If a blocking fallback is truly needed (when no cache and no task queue), the condition should be refactored to track the three states explicitly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/service.ts` around lines 374 - 417, When aiAnalysis is true and a cached
result exists, the current logic still falls into the blocking analysis branch
because aiTaskCreated stays false; fix by ensuring the blocking path only runs
when there is neither a cached result nor a queued task. Update the condition
around the analyzeEmoji call (or introduce a hasCachedResult boolean from
this._uploadManager.getCachedAIResult) to require !aiTaskCreated && aiAnalysis
&& !hasCachedResult (or set aiTaskCreated = true when a cached result is
applied), referencing getCachedAIResult, queueAIAnalysis, analyzeEmoji and
finalOptions so cached results are respected and not overwritten.
Comment on lines
+100
to
+126
| async loadExistingHashes(): Promise<void> { | ||
| try { | ||
| const emojis = await this.ctx.database.get( | ||
| 'emojiluna_emojis', | ||
| {}, | ||
| { | ||
| limit: 10000, | ||
| fields: ['id', 'image_hash'] | ||
| } | ||
| ) | ||
| if (emojis && emojis.length > 0) { | ||
| for (const emoji of emojis) { | ||
| if (emoji.image_hash) { | ||
| this.emojiHashes.add(emoji.image_hash) | ||
| this.emojiHashMap.set(emoji.image_hash, emoji.id) | ||
| } | ||
| } | ||
| } | ||
| this.ctx.logger.info( | ||
| `UploadManager: Loaded ${this.emojiHashes.size} existing emoji hashes` | ||
| ) | ||
| } catch (err) { | ||
| this.ctx.logger.warn( | ||
| `UploadManager: Failed to load existing hashes: ${err?.message || err}` | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
loadExistingHashes is capped at 10,000 rows — silently drops hashes for larger datasets.
Line 106 sets limit: 10000. If more than 10K emojis exist, the remaining hashes won't be loaded, causing deduplication misses (duplicate uploads will slip through the in-memory check and only be caught by the slower DB query in validateNewEmoji).
Consider removing the limit or paginating:
Proposed fix (remove limit)
const emojis = await this.ctx.database.get(
'emojiluna_emojis',
{},
{
- limit: 10000,
fields: ['id', 'image_hash']
}
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/uploadManager.ts` around lines 100 - 126, The loadExistingHashes function
currently limits database reads to 10,000 rows (this.ctx.database.get with
limit: 10000), which silently drops hashes and breaks in-memory dedupe
(emojiHashes/emojiHashMap) — fix by removing the hard cap or implementing
pagination: repeatedly call this.ctx.database.get (preserving fields
['id','image_hash']) with a cursor/offset/lastId until no more rows are returned
and add each image_hash to emojiHashes and emojiHashMap; keep the final logger
message and error handling intact so validateNewEmoji still benefits from a
complete in-memory index.
Comment on lines
+163
to
+166
| const mimeType = getImageType(imageData) | ||
| if (!mimeType) { | ||
| return 'Invalid image format' | ||
| } |
There was a problem hiding this comment.
getImageType always returns a non-empty string — this validation branch is unreachable.
Looking at src/utils.ts (lines 96–109), getImageType defaults to 'image/jpeg' when no signature matches. So mimeType can never be falsy, and 'Invalid image format' will never be returned.
If you want to reject unrecognized formats, you'd need to update getImageType to return null for unknown signatures, or add explicit format checking here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/uploadManager.ts` around lines 163 - 166, The branch checking if mimeType
is falsy is unreachable because getImageType always returns a string; change the
validation to explicitly verify allowed types instead: call
getImageType(imageData) into mimeType and validate it against a whitelist of
accepted MIME types (e.g., 'image/jpeg','image/png','image/gif','image/webp')
and return 'Invalid image format' if not in that set, or alternatively modify
getImageType to return null for unknown signatures and keep the existing falsy
check—refer to getImageType and the mimeType variable in uploadManager.ts when
making the change.
Contributor
Author
|
好！ |
dingyi222666
added a commit
that referenced
this pull request
Feb 23, 2026
Manually resolve merge conflicts across frontend and backend by combining PR #2 feature work (AI task controls, worker-based upload dedupe, new upload manager, status indicators) with existing pagination and formatting updates. Preserve functional improvements from both branches while keeping code style consistent and retaining paginated APIs in emoji/category views. Refs: PR #2
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
已经过本地测试，可正常使用。
Summary by CodeRabbit
Release Notes
New Features
Improvements