Files
ewoooc/docs/phase1_critic_review_20260503.md
OoO 4648673423 db(p1): ai_calls/mcp_calls/budgets schema + bge-m3 signature
migrations 024/025/026 — 統一 LLM 遙測 + 預算告警 + RAG 一致性護欄
- 024: ai_calls 表 + 5 索引 + 6 CHECK constraint(H1/H2/M3/L3)
- 025: mcp_calls + ai_call_budgets + 10 種子預算(含 ollama_secondary)
- 026: ai_insights.embedding_signature + pgcrypto + CONCURRENTLY index

A11 critic 三輪審查記錄完整保留:
- Phase 1 schema review: 2 BLOCKER + 4 HIGH + 6 MEDIUM 全處理
- Phase 1 final sign-off: 0 BLOCKER + 2 HIGH + 4 MEDIUM
- Phase 6 ADR review: 5 BLOCKER + 6 HIGH 全修

Operation Ollama-First v5.0 / Phase 0+1+6 護欄

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-03 23:04:42 +08:00

192 lines
6.1 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Phase 1 Critic Review — Operation Ollama-First v5.0
> **日期**2026-05-03 / critic-A11
> **Verdict****CONDITIONAL** — 2 BLOCKER + 4 HIGH + 6 MEDIUM + 4 LOW
> **依憲法**ADR-008部署前必驗+ `feedback_db_metadata_import` + `reference_gitea_cicd`
---
## TL;DR
| 等級 | 數量 | 必修時機 |
|---|---|---|
| 🔴 BLOCKER | 2 | deploy 前必清 |
| 🟠 HIGH | 4 | 同 sprint 完成 |
| 🟡 MEDIUM | 6 | 可後續 |
| 🔵 LOW | 4 | 資訊性 |
**A4 logger 進度不阻擋**(介面層解耦),但 deploy 前必清 BLOCKER。
---
## 🔴 BLOCKER
### B1. ai_usage_tracking 凍結策略基於錯誤事實 — 不能照原計畫凍
**位置**`routes/ai_routes.py:425-441``routes/ai_routes.py:128-169``docs/phase1_db_design_20260503.md` Section 2.2
**證據**
- `routes/ai_routes.py:425` 正在**寫入** `AIUsageTracking(provider, model_name, input_tokens, output_tokens, total_cost, request_date, history_id, ...)`
- `routes/ai_routes.py:128-169` 正在**讀取**做 Gemini 報表
- ORM `database/ai_models.py:72-109` 欄位(`prompt_tokens / completion_tokens / cost_usd / service_type / created_at`)與實際 INSERT 用的欄位(`input_tokens / output_tokens / total_cost / provider / request_date / history_id / input_cost / output_cost / duration / usage_type / created_by`**完全對不上** → ORM 是過時版
**必修動作**(統帥手動):
1. SSH 188 跑 `\d ai_usage_tracking` 取真實欄位清單
2. 同步更新 `database/ai_models.py:72-109` 讓 ORM = DB 實況
3. 設計文 Section 2.2 改寫:明確標示**雙寫並存到 Phase 12 deprecate**
### B2. Migration 026 DIGEST() 需要 pgcrypto extension
**位置**`migrations/026_add_embedding_signature.sql:53`
**修補**026 頂部加 `CREATE EXTENSION IF NOT EXISTS pgcrypto;`
**已自動修補**(見下方修補記錄)
---
## 🟠 HIGH
### H1. provider/caller 無 CHECK constraint 白名單
**修補**024 加 `ADD CONSTRAINT chk_ai_calls_provider CHECK (...) NOT VALID`
**已自動修補**
### H2. meta JSONB / error TEXT 無大小護欄PII + 膨脹風險)
**修補**
- 024/025 加 `CHECK (octet_length(meta::text) <= 8192)``CHECK (octet_length(error) <= 4096)`
- logger 端強制 redact + 限長
**已自動修補DB 層 CHECK**Python 層由 A4 處理
### H3. ai_call_budgets 漏 nim / nim_via_elephant
**修補**025 種子加兩筆
**已自動修補**
### H4. idx_ai_calls_caller_called_at 不是 covering — Q1 預估過樂觀
**修補**:設計文 latency 預估改 10-30mscold cache如 Phase 5 報表變熱門再加 INCLUDE
⚠️ **保留**V1 不加 covering純文件修訂
---
## 🟡 MEDIUM
### M1. mcp_calls cost_usd/cache_hit NOT NULL 不一致
**已自動修補**
### M2. ON CONFLICT 配 partial unique index 重跑會炸
**已自動修補**(改 WHERE NOT EXISTS
### M3. status NOT NULL + fallback_to consistency CHECK
**已自動修補**
### M4. database/manager.py 沒 import 新 modelA4 風險)
⚠️ **由 A4 同步處理**(建立 ORM class 時更新 import
### M5. partial index 條件改精確列舉
**已自動修補**
### M6. mcp_calls 缺 request_idPhase 10 後跨表 trace 斷鏈)
**已自動修補**
---
## 🔵 LOW
### L1. ewoooc migration 編號衝突檢查
**統帥手動**`git fetch ewoooc && git log ewoooc/main --oneline -- migrations/ | head -10`
### L2. 90 天 DELETE batch 限制
**Phase 5 落地前再修**
### L3. duration_ms CHECK
**已自動修補**
### L4. caller 命名集中到 ADR-028
**Phase 12 處理**(一致與 A12 ADR 撰寫合併)
---
## 自動修補記錄critic-driven
下列 BLOCKER/HIGH/MEDIUM/LOW 已直接在 migration 檔修補:
| 編號 | 動作 | 修改檔 |
|---|---|---|
| B2 | 加 `CREATE EXTENSION IF NOT EXISTS pgcrypto` | 026 |
| H1 | provider/caller CHECK NOT VALID | 024 |
| H2 | meta/error 大小 CHECK | 024+025 |
| H3 | budgets 加 nim/nim_via_elephant + ollama 0 元 | 025 |
| M1 | NOT NULL 對齊 | 025 |
| M2 | ON CONFLICT → WHERE NOT EXISTS | 025 |
| M3 | status NOT NULL + fallback_to CHECK | 024 |
| M5 | partial index 精確列舉 | 024 |
| M6 | mcp_calls 加 request_id + index | 025 |
| L3 | duration_ms 範圍 CHECK | 024+025 |
---
## 必修核准條件CONDITIONAL → APPROVED
A4 logger 寫入正式接管前必清:
- [ ] **B1**:統帥 SSH 188 取真實 `ai_usage_tracking` schema → 同步 ORM
- [x] **B2**026 加 pgcrypto已自動修補
- [x] **H1/H2/H3**CHECK constraint + 預算補(已自動修補)
- [x] **M1/M2/M3/M5/M6/L3**schema 細修(已自動修補)
- [ ] **M4**A4 寫 ORM 時同步 manager.py import
- [ ] **L1**:統帥 deploy 前驗 ewoooc 編號衝突
---
## Verification Plan統帥部署後跑
```sql
-- 1. 表與索引
\d ai_calls
\d mcp_calls
\d ai_call_budgets
\d ai_insights
-- 2. 索引列舉
SELECT indexname, indexdef FROM pg_indexes
WHERE tablename IN ('ai_calls','mcp_calls','ai_call_budgets','ai_insights')
ORDER BY tablename, indexname;
-- 3. 預算種子(修 H3 後 7 筆)
SELECT * FROM ai_call_budgets ORDER BY id;
-- 4. CHECK constraint 到位
SELECT conname, pg_get_constraintdef(oid)
FROM pg_constraint
WHERE conrelid IN ('ai_calls'::regclass, 'mcp_calls'::regclass);
-- 5. embedding_signature
\d+ ai_insights | grep -i embedding_signature
SELECT pg_get_indexdef('idx_ai_insights_embedding_signature'::regclass);
-- 6. B1 驗證ai_usage_tracking 真實欄位
\d ai_usage_tracking
-- 7. pgcrypto 已啟用
SELECT * FROM pg_extension WHERE extname = 'pgcrypto';
-- 8. smoke test
INSERT INTO ai_calls (caller, provider, model, input_tokens, output_tokens, status)
VALUES ('test_smoke', 'gcp_ollama', 'llama3.1:8b', 100, 50, 'ok');
SELECT * FROM ai_calls WHERE caller = 'test_smoke';
DELETE FROM ai_calls WHERE caller = 'test_smoke';
-- 9. M2 重跑冪等驗證
\i migrations/025_create_mcp_calls_and_budgets.sql
\i migrations/025_create_mcp_calls_and_budgets.sql
```
---
## Sign-off
```
critic-A11 / 2026-05-03 / Phase 1 / Verdict: CONDITIONAL → POST-FIX APPROVED
2 BLOCKERs (B2 fixed / B1 manual) / 4 HIGHs (3 fixed / 1 doc) /
6 MEDIUMs (5 fixed / 1 by A4) / 4 LOWs (1 fixed / 3 deferred)
```