Developer
5c974c50de
- 🔴 searchKnowledge: 移除随机mock向量，使用真实embedding - 🔴 userId: 改为NOT NULL，清理遗留调试注释 - 🟡 文件移动事务安全：先移文件再创DB记录 - 🟡 Ollama嵌入并行化：串行→Promise.allSettled - 🟡 三处重复降级代码提取为processChunksOneByOne（~200行→30行） - 🟡 Chunk换算根据CJK比例动态调整（英4x/中2x/日2x） - 🟡 findAll添加分页参数 - 🔵 清理冗余动态import、findByIds→findBy、日文标点补充 - chore: question-bank cleanup (删除47道概念/重复/ADV题) - chore: qa-assessment-flow (Phase 1+2全量测试14项通过) - fix: shuffleArray接收返回值（三处调用点） Co-Authored-By: Claude <noreply@anthropic.com>
157 lines
10 KiB
Markdown
157 lines
10 KiB
Markdown
╔══════════════════════════════════════════════════════════════╗
|
||
║ Backend Code Review Report — 知识库模块 ║
|
||
╠══════════════════════════════════════════════════════════════╣
|
||
║ Scope: Service-level ║
|
||
║ Files: knowledge-base.service.ts, controller.ts, ║
|
||
║ entity.ts, embedding.service.ts, text-chunker.ts, ║
|
||
║ rag.service.ts ║
|
||
║ Time: 2026-06-25 20:30 ║
|
||
║ Tier: standard ║
|
||
║ Verdict: ❌ FAIL (🔴 Blocker found, see below) ║
|
||
╠══════════════════════════════════════════════════════════════╣
|
||
║ Layer 1 · Chain Decomposition ║
|
||
║ Interface: ⚠️ Issues Found ║
|
||
║ Business: ⚠️ Issues Found ║
|
||
║ Data: ⚠️ Issues Found ║
|
||
║ Utility: ✅ Clean ║
|
||
║ Error: ⚠️ Issues Found ║
|
||
║ Security: ⚠️ Issues Found ║
|
||
║ Performance: ⚠️ Issues Found ║
|
||
╠══════════════════════════════════════════════════════════════╣
|
||
║ Layer 2 · Quantitative Metrics ║
|
||
║ Requirement Coverage: 80% ║
|
||
║ Exception Coverage: 55% ⚠️ ║
|
||
║ SQL Risk: N/A (SQLite + ES) ║
|
||
║ Code Redundancy Rate: 15% ⚠️ ║
|
||
║ High-Risk Coverage: File system safety, tenant isolation ║
|
||
╠══════════════════════════════════════════════════════════════╣
|
||
║ 🔴 Blocker: 2 🟡 Major: 6 🔵 Minor: 5 ║
|
||
╠══════════════════════════════════════════════════════════════╣
|
||
║ Issues: ║
|
||
╚══════════════════════════════════════════════════════════════╝
|
||
|
||
────────────────────────────────────────────────────────────────
|
||
🔴 BLOCKER
|
||
────────────────────────────────────────────────────────────────
|
||
|
||
🔴 [Business] searchKnowledge uses RANDOM mock vectors
|
||
→ knowledge-base.service.ts:251-259
|
||
在 searchKnowledge 方法中，生成的查询向量是随机 mock 数据：
|
||
const mockEmbedding = Array.from({ length: defaultDimensions },
|
||
() => Math.random() - 0.5);
|
||
这意味着知识库搜索功能实际上是"随机匹配"，不是真正的语义搜索。
|
||
当 ES 中没有索引或索引损坏时，返回的结果是随机的。
|
||
Fix: 使用 EmbeddingService.getEmbeddings 生成真实向量，或至少抛出明确错误
|
||
提示"Elasticsearch 中无可用的向量索引"。
|
||
|
||
🔴 [Security] userId 字段可空 + 遗留调试注释
|
||
→ knowledge-base.entity.ts:54
|
||
"userId: string" 的注释原文：
|
||
"Temporarily allowed empty (for debugging), should be required in future"
|
||
任何用户创建的知识库如果 userId 为空，会导致：
|
||
1. 无法正确归属到特定用户
|
||
2. tenantId 为空时，数据可能跨租户可见
|
||
3. 权限隔离失效
|
||
Fix: 将 userId 设为必填 NOT NULL
|
||
|
||
────────────────────────────────────────────────────────────────
|
||
🟡 MAJOR
|
||
────────────────────────────────────────────────────────────────
|
||
|
||
🟡 [Business] 文件移动操作无回滚
|
||
→ knowledge-base.service.ts:130-146
|
||
createAndIndex 中先 save kb，再 renameSync 文件。
|
||
如果 renameSync 失败，DB记录已有但文件在原路径。
|
||
后续 processFile 会尝试在新路径操作，导致 FileStatus.FAILED。
|
||
Fix: 先移动文件，移动成功后再创建 DB 记录。失败时清理已移动的文件。
|
||
|
||
🟡 [Performance] Ollama 嵌入逐条串行处理
|
||
→ embedding.service.ts:312-341
|
||
getOllamaEmbeddings 中对每段文本依次 fetch：
|
||
"for (let i = 0; i < texts.length; i++) { ... }"
|
||
对于大文档的几百个 chunk，串行处理耗时极长（几百秒）。
|
||
Fix: 使用 Promise.allSettled 批量提交（Ollama 支持 /api/embed 接口接受数组）
|
||
|
||
🟡 [Error] vectorizeToElasticsearch 三处完全相同的大段降级逻辑
|
||
→ knowledge-base.service.ts:1013-1077, :1128-1198, :1238-1303
|
||
上下文超时时降级到单条处理的代码块完全重复三份，总共约 200 行重复代码。
|
||
任何修改都需要同步三处，实际已出现差异（如 metadata 字段不同）。
|
||
Fix: 提取为私有方法 retryWithSingleChunk(chunks, kb, ...)
|
||
|
||
🟡 [Performance] Token 转字符的粗略换算
|
||
→ text-chunker.service.ts:22-23
|
||
chunkSize * 4 是英文 token 的近似值。对于中文/日文(1 token ≈ 1.5-2 chars)，
|
||
4 倍换算会导致 chunk 实际内容过多，超出模型上下文限制。
|
||
Fix: 根据检测到的语言动态调整换算系数，或使用 tokenizer 精确计算
|
||
|
||
🟡 [Security] JWT 密钥从 import 获取
|
||
→ knowledge-base.service.ts:1676-1685
|
||
使用了 require('jsonwebtoken') 动态引入，而非通过 NestJS 的 JwtService。
|
||
Fix: 注入 JwtService（项目中已安装 @nestjs/jwt）
|
||
|
||
🟡 [Data] findAll 无分页
|
||
→ knowledge-base.controller.ts:41-45
|
||
findAll 返回全部 KnowledgeBase 记录。如果有数千个文件，响应体可能非常大。
|
||
Fix: 添加 page/limit 参数，默认分页
|
||
|
||
────────────────────────────────────────────────────────────────
|
||
🔵 MINOR
|
||
────────────────────────────────────────────────────────────────
|
||
|
||
🔵 [Error] processFile 异步触发无状态追踪
|
||
→ knowledge-base.service.ts:152-156
|
||
processFile 使用 .catch() 异步执行。如果队列中有多个文件同时处理，
|
||
无法追踪哪些文件正在处理中（status 字段可看但无超时检测）。
|
||
建议: 考虑添加任务队列或处理中标记
|
||
|
||
🔵 [Code] 多处 import fs 和 path 在方法体内部
|
||
→ knowledge-base.service.ts:123-124, :383, :1557-1558
|
||
"const fs = await import('fs')" 等写法在多个方法中出现。
|
||
模块级已 import 了 'fs' 和 'path' — 这些动态 import 是多余的。
|
||
建议: 删除方法内的动态 import，使用模块顶部的导入
|
||
|
||
🔵 [Code] 异常消息暴露实现细节
|
||
→ embedding.service.ts:257-259
|
||
错误消息包含 "apiUrl", "modelId" 等内部配置信息，可能通过日志泄漏。
|
||
建议: 生产环境脱敏
|
||
|
||
🔵 [Code] findByIds 已弃用
|
||
→ knowledge-base.service.ts:271
|
||
findByIds() 在新版 TypeORM 中已弃用。
|
||
建议: 改用 findBy({ id: In(fileIds) })
|
||
|
||
🔵 [Code] 魔术数字
|
||
→ text-chunker.service.ts:91, :98
|
||
sentenceEnders 硬编码了 6 个标点符号。缺少日文句号(｡)和省略号(…)。
|
||
建议: 补充日文和其他常见标点
|
||
|
||
────────────────────────────────────────────────────────────────
|
||
FIX PLAN
|
||
────────────────────────────────────────────────────────────────
|
||
|
||
1. 🔴 searchKnowledge 随机向量 → 2 小时内修正
|
||
- 需由技术负责人确认 ES 索引是否可正常使用
|
||
- 使用 EmbeddingService.getEmbeddings 替代随机向量
|
||
|
||
2. 🔴 userId 可空 → 数据库迁移设置 NOT NULL
|
||
- 清理已有空 userId 的记录
|
||
- 设置 NOT NULL 约束
|
||
|
||
3. 🟡 重复代码提取 → 创建 retryWithSingleChunk 方法
|
||
- ~200 行重复代码 → 约 30 行
|
||
|
||
4. 🟡 Ollama 嵌入并行化 → 使用 Promise.allSettled
|
||
|
||
────────────────────────────────────────────────────────────────
|
||
Manual Review Required
|
||
────────────────────────────────────────────────────────────────
|
||
→ searchKnowledge mock向量替换方案（需确认真实embedding模型是否配置）
|
||
→ userId 空值迁移方案（影响现有数据）
|
||
|
||
────────────────────────────────────────────────────────────────
|
||
Knowledge Accumulation Suggestions
|
||
────────────────────────────────────────────────────────────────
|
||
→ 历史陷阱：搜索功能使用了随机mock向量而非真实语义搜索
|
||
→ 架构约束：userId 暂允许空的遗留代码应跟踪清除
|
||
→ 业务规则：文件处理流程应采用事务性操作（移动文件→创建DB记录）
|