172 lines
7.0 KiB
Markdown
172 lines
7.0 KiB
Markdown
---
|
||
title: 代码审查报告
|
||
date: 2026-02-26
|
||
reviewer: Claude Sonnet 4.6
|
||
status: 完成
|
||
---
|
||
|
||
# 代码审查报告（2026-02-26）
|
||
|
||
## 总体评价
|
||
|
||
架构设计合理，双层记忆模型思路清晰，核心链路（embedding → pgvector → 检索注入）实现正确。`ON CONFLICT DO UPDATE` 幂等 upsert、异步 post-commit hook、`<retrieved_context>` 注入防护均为正确决策。
|
||
|
||
主要问题集中在：**数据安全**（静默删除好数据）、**性能浪费**（每次都重新 embed）、**误隔离**（blacklist 过宽导致 49% 文档未被索引）三个方向。
|
||
|
||
---
|
||
|
||
## Critical 问题
|
||
|
||
### C1 · 全量索引会静默删除 embedding 失败的文档
|
||
|
||
**文件**：`ingest_vault.py` 第 86-90 行
|
||
|
||
stale 清理逻辑 `db_primary_ids - valid_ids` 未排除 `failed_ids`。一次网络抖动导致 embedding 失败，下次全量重建就把之前已有的好向量删掉。数据静默丢失。
|
||
|
||
```python
|
||
# 当前（有 bug）
|
||
stale_primary_ids = sorted(db_primary_ids - valid_ids)
|
||
|
||
# 修复
|
||
stale_primary_ids = sorted(db_primary_ids - valid_ids - set(failed_ids))
|
||
```
|
||
|
||
---
|
||
|
||
### C2 · 增量索引从不检查 content_hash，每次都重新 embed
|
||
|
||
**文件**：`incremental_ingest.py`，`upsert_file()` 函数
|
||
|
||
`content_hash` 列已存在于 schema 并由全量索引填充，但增量索引完全忽略它。每个 `M` 事件都调一次 OpenRouter API，即使文件内容没有变化。这是 P95=1.4s 的直接原因，也在浪费 API 额度。
|
||
|
||
修复：在调用 `embed_text` 前，先查 `memory_primary` 的 `content_hash`，若匹配则跳过。
|
||
|
||
---
|
||
|
||
### C3 · changes 临时文件永远不删除
|
||
|
||
**文件**：`install-hook.sh` 第 24、29-30 行
|
||
|
||
hook 通过 `nohup` 后台运行 Python 进程，没有任何机制在处理完成后删除 `.memory-changes-<timestamp>-<pid>.txt`。每次涉及 `.md` 文件的 commit 都在 vault 根目录留下一个文件，长期无限积累。
|
||
|
||
```bash
|
||
# 修复：用子 shell 包装，处理完后清理
|
||
nohup bash -c "uv run ... --changes-file '$changes_file' >> '$log_file' 2>&1; rm -f '$changes_file'" &
|
||
```
|
||
|
||
---
|
||
|
||
### C4 · `api_key` 字面量过宽，导致 49% 文档被误隔离
|
||
|
||
**文件**：`blacklist.py` 第 17 行
|
||
|
||
字面量 `'api_key'` 会匹配任何包含该字符串的笔记，包括架构文档、项目笔记、本项目自身的文档。实测结果：79/160 文档被隔离（49%），接近一半的 vault 内容未被索引。
|
||
|
||
同文件第 33 行的正则 `(?i)\b(password|...api[_-]?key)\b\s*[:=]\s*\S{4,}` 才是正确做法（要求后面跟赋值符号）。应删除 `'api_key'` 字面量，或改为 `'api_key='`、`'api_key:'`。
|
||
|
||
---
|
||
|
||
### C5 · Windows 文件锁可能是非阻塞的
|
||
|
||
**文件**：`index_common.py` 第 97、101 行
|
||
|
||
`msvcrt.LK_LOCK` 在部分 Python/Windows 版本下行为不一致，可能不阻塞直接抛 `OSError`。两个并发 ingest 进程可能同时通过锁，导致数据竞争。需要用带重试的循环或换用更可靠的 Windows 锁原语。
|
||
|
||
---
|
||
|
||
## Important 问题
|
||
|
||
### I1 · ivfflat lists=100 对当前数据量完全无效
|
||
|
||
**文件**：`schema.sql` 第 20 行
|
||
|
||
pgvector 建议 `lists = rows / 1000`，81 行数据应用 `lists=1`。当前 lists 数量多于行数，查询规划器会忽略索引直接走全表扫描，索引只有写开销没有读收益。
|
||
|
||
---
|
||
|
||
### I2 · 两个 ingest 脚本的目录范围不一致
|
||
|
||
`ingest_vault.py` 只扫 `01_Projects` 和 `02_Areas`，但 `incremental_ingest.py` 处理 git diff 里任何 `.md` 文件。`03_Resources/` 里的文件会被增量索引，但下次全量重建时被删掉，造成数据不一致。
|
||
|
||
---
|
||
|
||
### I3 · 大文件被 API 静默截断
|
||
|
||
`text-embedding-3-small` 上限 8191 tokens，超长笔记的后半部分永远不会被 embed，且没有任何警告。`_sample_head_mid_tail` 函数已存在于 `index_common.py`（用于敏感扫描），但未用于 embedding。
|
||
|
||
---
|
||
|
||
### I4 · `agent-with-memory.sh` 的 VAULT_DIR 推导方式脆弱
|
||
|
||
脚本假设自己在 vault 根目录下两层（`.scripts/memory/`），迁移到独立项目后这个假设已不成立。应从 `.env.memory` 读取 `VAULT_DIR` 作为权威来源。
|
||
|
||
---
|
||
|
||
### I5 · 查询没有相似度阈值
|
||
|
||
无论相关性多低，始终返回 top_k 结果。查询完全不相关的内容时，会把最不相关的 5 个文档注入 Claude 上下文，产生噪音甚至误导。
|
||
|
||
建议加 `WHERE embedding <=> %s::vector < 0.5`（阈值需实测调整）。
|
||
|
||
---
|
||
|
||
### I6 · `memory_secure_audit` 不记录触发原因
|
||
|
||
`risk` 列永远是 `'excluded_or_sensitive'`，无法区分是路径规则、文件名规则还是内容规则触发的。调整 blacklist 时完全没有依据。
|
||
|
||
---
|
||
|
||
### I7 · hook 静默失败，用户无感知
|
||
|
||
PostgreSQL 挂了、`.env.memory` 不存在、Python 环境损坏，全部静默失败。用户不知道索引已经落后，只能手动查看 `.memory-sync.log`。
|
||
|
||
---
|
||
|
||
### I8 · 全量索引持有超长 DB 事务
|
||
|
||
`ingest_vault.py` 在单个事务内完成所有 embedding API 调用（81 次 HTTP 请求，可能数分钟）。事务期间持有连接和行锁，进程被杀时虽然回滚干净，但锁文件同时释放，可能导致并发 ingest 在部分更新状态下运行。
|
||
|
||
---
|
||
|
||
## 文档问题
|
||
|
||
| # | 文件 | 问题 |
|
||
|---|---|---|
|
||
| D1 | `agent-with-memory.sh` | 用法提示仍写旧路径 `.scripts/memory/` |
|
||
| D2 | `README.md` step 6 | `install-hook.sh` 路径是占位符，未说明如何确定实际路径 |
|
||
| D3 | `index.md` vs `status.md` | 冷启动状态描述不一致（"未测" vs "未完成量化"）|
|
||
| D4 | `README.md` | `OPENROUTER_EMBED_DIM` 标为必填，但代码有默认值 1536 |
|
||
| D5 | 所有文档 | Docker 示例使用默认密码 `postgres:postgres`，未提示修改 |
|
||
| D6 | 所有文档 | `eval_cold_start.py` 完全未被文档化 |
|
||
| D7 | `status.md` | 49% 隔离率记录为已知问题，但未分析根因（实为 C4 的直接证据）|
|
||
|
||
---
|
||
|
||
## 运营问题
|
||
|
||
| # | 问题 |
|
||
|---|---|
|
||
| O1 | `.memory-sync.log` 无限增长，无轮转机制 |
|
||
| O2 | 无健康检查命令，无法快速验证系统是否正常运行 |
|
||
| O3 | 无索引漂移检测机制，hook 连续失败时用户无感知 |
|
||
|
||
---
|
||
|
||
## 问题汇总
|
||
|
||
| 编号 | 严重度 | 文件 | 问题 |
|
||
|---|---|---|---|
|
||
| C1 | Critical | `ingest_vault.py` | stale 清理删除 embed 失败的文档 |
|
||
| C2 | Critical | `incremental_ingest.py` | 未用 content_hash，每次都重新 embed |
|
||
| C3 | Critical | `install-hook.sh` | changes 临时文件永远不删 |
|
||
| C4 | Critical | `blacklist.py` | `api_key` 字面量过宽，49% 误隔离 |
|
||
| C5 | Critical | `index_common.py` | Windows 锁可能非阻塞 |
|
||
| I1 | Important | `schema.sql` | ivfflat lists=100 对 81 行无效 |
|
||
| I2 | Important | 两个 ingest 脚本 | 目录范围不一致 |
|
||
| I3 | Important | 两个 ingest 脚本 | 大文件静默截断 |
|
||
| I4 | Important | `agent-with-memory.sh` | VAULT_DIR 推导脆弱 |
|
||
| I5 | Important | `query_pgvector.py` | 无相似度阈值 |
|
||
| I6 | Important | `schema.sql` | audit 表不记录触发原因 |
|
||
| I7 | Important | `install-hook.sh` | hook 静默失败 |
|
||
| I8 | Important | `ingest_vault.py` | 全量索引持有超长事务 |
|