ewoooc/docs/phase1_final_critic_signoff_20260503.md

# Phase 1 Final Critic Sign-off — Operation Ollama-First v5.0

> **日期**：2026-05-03 / critic-A11（第二輪 / 收尾）
> **審查範圍**：A3 / A4 / A5 / 第一輪 A11 修補的全部產出
> **基準文件**：`docs/phase1_critic_review_20260503.md`

---

## Verdict

- [ ] APPROVED — Phase 1 deploy ready
- [x] **APPROVED WITH NOTES** — 接受 deploy；4 項 NOTE 統帥部署前/後處理即可
- [ ] CONDITIONAL — 修以下後 deploy
- [ ] REJECTED

> **理由**：本輪沒有發現新 BLOCKER。前一輪 BLOCKER 中 B2（pgcrypto）migration 端已修補；B1（ai_usage_tracking ORM 落後 schema）屬於既有技術債、不阻擋 v5.0 觀測層上線。Logger 與 token report 行為正確、52/52 unit test 通過、失敗安全與 PII 紀律執行到位。NOTE 主要是文件對齊與已知盲區（Bot main path token=0 / chat_id 進 meta），不影響 P1 觀測層收尾。

---

## 前一輪 BLOCKER / HIGH 處理確認

| 編號 | 等級 | 描述 | 本輪驗證 |
|---|---|---|---|
| **B1** | 🔴 | ai_usage_tracking ORM 與實際欄位脫鉤（雙寫並存） | ⚠️ **未動**。ORM `database/ai_models.py:72-109` 仍是過時版；屬既有技術債，不影響 v5.0 觀測層（A4/A5 一律走 raw SQL → ai_calls，未碰 ai_usage_tracking）。建議列入 Phase 12 deprecate roadmap。 |
| **B2** | 🔴 | migration 026 DIGEST() 需 pgcrypto | ✅ **已驗證**。`migrations/026_add_embedding_signature.sql` 頂部已加 `CREATE EXTENSION IF NOT EXISTS pgcrypto;`。 |
| **H1** | 🟠 | provider 白名單 CHECK | ✅ **已驗證**。`migrations/024:88-91` `chk_ai_calls_provider` 列出 7 個 provider 與 `_PROVIDER_DISPLAY` 完全一致。 |
| **H2** | 🟠 | meta/error 大小護欄 | ✅ **已驗證**。`024:104-109` meta ≤ 8192 / error ≤ 4096 octet。Python 端 `set_error` 也截 2000 字（`ai_call_logger.py:168`），雙保險。 |
| **H3** | 🟠 | budgets 漏 nim/nim_via_elephant | ✅ **已驗證**。`025:170-199` 7 個 provider × monthly + 3 條全供應商總額（daily/weekly/monthly）共 10 筆種子。 |
| **H4** | 🟠 | idx 非 covering（latency 樂觀） | ⚠️ 文件層 — 不影響部署。 |
| **M1-M6/L3** | 🟡🔵 | 細節修補 | ✅ 全數在 024/025 内驗證通過。 |
| **M4** | 🟡 | manager.py 未 import 新 model | ⚠️ A4/A5 全走 raw SQL，未建立 AICall ORM class，所以 import 缺口不會引發 `Base.metadata.create_all` 漏表（migrations 直接建表）。**不阻擋**。 |
| **L1** | 🔵 | ewoooc migration 編號衝突 | 🟡 未驗證 — 統帥 deploy 前手動 `git fetch ewoooc && git log ewoooc/main --oneline -- migrations/` 即可。 |

**小結**：第一輪 BLOCKER 中可由 critic 自動修補的 1/2 已修；B1 為**設計文資料漂移**，本輪確認 v5.0 觀測層**完全不依賴** ai_usage_tracking，所以解耦處理（不阻擋 deploy）。

---

## 本輪新發現 Findings

### BLOCKER

**無**。

### HIGH

#### H5. caller 欄位**沒有** CHECK 白名單 — 本輪實際比對下發現先前 H1 修補只覆蓋 `provider` 不含 `caller`

- **位置**：`migrations/024_create_ai_calls_table.sql:55`（`caller VARCHAR(64) NOT NULL`）+ `:86-110` 所有 CHECK，無 `chk_ai_calls_caller`
- **證據**：grep `chk_` 在 024 共 7 條 constraint，僅針對 provider/status/fallback/duration/meta/error，**caller 完全沒護欄**
- **影響**：A4 接入的 13 個 caller 名（`hermes_intent`/`hermes_analyst`/`hermes_rule_engine`/`code_review_hermes`/`code_review_openclaw`/`code_review_elephant`/`openclaw_qa`/`openclaw_qa_nim`/`openclaw_weekly`/`openclaw_daily`/`openclaw_monthly`/`openclaw_meta`/`nemotron_dispatch`/`openclaw_bot_main`/`openclaw_bot_gemini`/`openclaw_bot_nim`）若未來打字打錯（例如 `openclae_qa`）DB 不會擋；token 報表 GROUP BY caller 會出現假名稱，污染統計
- **緩解**：戰役 v5.0 收尾才標到的問題，本輪先 NOTE 不阻 deploy，但建議 Phase 5 跑保留任務時加：
  ```sql
  ALTER TABLE ai_calls ADD CONSTRAINT chk_ai_calls_caller_known
    CHECK (caller ~ '^[a-z][a-z0-9_]{2,63}$') NOT VALID;
  ```
  （格式約束而非完整白名單，避免每次擴 caller 都改 schema）
- **嚴重度判定**：本來 BLOCKER，但因 v5.0 上線前 caller 名是**集中於 ai_call_logger 的固定字串**（13 個全部 grep 過），typo 風險可控 → 降為 HIGH。

#### H6. `chat_id` 寫入 ai_calls.meta — 屬 PII（Telegram 用戶識別）

- **位置**：`routes/openclaw_bot_routes.py:6832, 6892, 6959, 7034`
- **證據**：4 個 Bot Q&A 入口的 `meta={'chat_id': chat_id, ...}` 全部把 Telegram chat_id 直接落地進 ai_calls.meta JSONB
- **規格牴觸**：
  - `services/token_report_service.py:18`「PII 保護: 報表訊息不含 prompt 原文；ai_insights metadata 只存統計 meta（不存 username）」
  - `feedback_user_input_html_injection`：Telegram 用戶識別屬 user-controllable 欄位，不該以明文落地 90 天
- **影響**：90 天保留期 + 萬一報表程式換成 raw query 可被反查；雖然 ai_calls.caller 維度不會直接顯示 chat_id，但稽核時違反「Telegram username/chat_id 進 DB 必雜湊」原則
- **建議修法**（不阻 deploy，可在 Phase 2 同步）：
  ```python
  meta={'chat_id_hash': hashlib.sha256(str(chat_id).encode()).hexdigest()[:12], ...}
  ```
  或乾脆改成 `meta={'has_chat_id': bool(chat_id), ...}`（只記是否屬聊天會話）
- **嚴重度判定**：HIGH。短期內統帥（即唯一 Telegram operator）即所有 chat_id 來源，外洩面向小；但 PII 紀律一致性必須維持，所以列入 deploy 後 Phase 2 第一波 patch 清單。

### MEDIUM

#### M7. `openclaw_bot_main` token 永遠記為 0 — Section 5「Ollama Tokens」會嚴重低估

- **位置**：`routes/openclaw_bot_routes.py:6834` `ollama_service.generate(...)` 回傳 `OllamaResponse`，但 `OllamaResponse` 沒有 `prompt_eval_count`/`eval_count` 欄位（`services/ollama_service.py:88-95` dataclass 只有 success/content/model/error/total_duration/host）
- **影響**：
  - Bot 主鏈走 GCP Ollama 的所有 Q&A token 記為 0
  - Section 5「今日 Ollama Tokens vs 7 日均」失真
  - Section 1 `ollama_pct` 計算分子下偏 → 報表會顯示「Ollama 失守」假警報
- **建議修法**（Phase 2 A6 修 ollama_service.py 時順便）：
  ```python
  @dataclass
  class OllamaResponse:
      success: bool
      content: str
      model: str
      error: Optional[str] = None
      total_duration: Optional[float] = None
      host: Optional[str] = None
      prompt_tokens: int = 0      # ← 新增
      completion_tokens: int = 0  # ← 新增
  ```
  並在 `generate()` 内 `data.get('prompt_eval_count')/eval_count` 帶入。
- **暫行處置**：A4 應在 `routes/openclaw_bot_routes.py:6834` 加 TODO 標記「待 ollama_service 補 token 欄位後接回」。本輪不阻 deploy。

#### M8. `caller='openclaw_qa_nim'` 由 `_call_nvidia_nim` 動態組成，與 logger 端命名習慣脫鉤

- **位置**：`services/openclaw_strategist_service.py:737` `nim_caller = f"{caller}_nim"`
- **影響**：Section 3 TOP caller 報表會出現 5 個 NIM 變體（`openclaw_qa_nim`/`openclaw_weekly_nim`/`openclaw_daily_nim`/`openclaw_monthly_nim`/`openclaw_meta_nim`）
- **判定**：這其實是好設計（清楚標示 NIM 路徑由哪個原 caller fallback 來的），但與 H5 中提到「未來加 caller 白名單」邏輯衝突 → 若加 CHECK constraint 必須允許 `_nim` 後綴
- **建議**：不修；但 ADR-028 撰寫時要明文聲明此命名慣例。

#### M9. ai_insights INSERT 缺 `confidence` 欄位 — 走 default 0.5，但 token report 是規則引擎產出，理論該標 1.0

- **位置**：`services/token_report_service.py:790-799`
- **影響**：未來 RAG 檢索時，token report 的洞察會被當「中信度」混入；其實這是規則引擎死硬產出，應該標高信度
- **建議修法**：INSERT 加 `confidence` 欄位設 1.0；或將 `avg_quality` 從 0.9 改為 1.0
- **嚴重度**：MEDIUM，不阻 deploy。

#### M10. `daily_token_report` 截斷邏輯雙重保險，但**截斷點落在 HTML tag 中間**會壞 parse_mode='HTML'

- **位置**：
  - `services/token_report_service.py:130` `report_html[: _TELEGRAM_MAX_CHARS - 80]`
  - `services/telegram_templates.py:566` `body[: _DAILY_TOKEN_REPORT_MAX_CHARS - 80]`
- **風險**：若截斷剛好落在 `<b>...</b>` 之間（例如卡在 `<b` 後 1 字元），Telegram sendMessage 會回 `400 can't parse entities` → 整則訊息 fail，scheduler log 出現 telegram_send error
- **緩解**：實務上 4000 字截斷觸發時，落在 HTML tag 中間機率 < 5%（tag 密度低），但仍是已知 corner case
- **建議修法**：截斷後跑 `re.sub(r'<[^>]*$', '', truncated)` 把不完整的開 tag 砍掉
- **嚴重度**：MEDIUM，建議 Phase 2 修；不阻 deploy（萬一觸發只是該日報表掉而已，scheduler 不爆）。

### LOW

#### L5. `qwen3:14b` 在戰役 v5.0 Frontier 升級表中提到，但 COST_TABLE 未列

- **位置**：`services/ai_call_logger.py:43-62`
- **驗證**：`grep -n "qwen3" services/ai_call_logger.py services/token_report_service.py` → 0 hit
- **影響**：未來 P2 把 NemoTron 改用 qwen3:14b 時，第一波寫入會走 `_calc_cost` 的 `unknown model` 路徑，log warning 但成本回 0（因為 qwen3 是本地 Ollama 也應為 0）→ 行為正確但 noisy log
- **建議**：在 COST_TABLE 加：
  ```python
  'qwen3:14b':         {'in': 0.0, 'out': 0.0},
  'qwen3:14b-q4_K_M':  {'in': 0.0, 'out': 0.0},  # 視 v5.0 量化版
  ```
- **嚴重度**：LOW（Phase 2 會修），不阻 deploy。

#### L6. `total_cost_usd` SUM 無 NUMERIC 上限保險

- **位置**：`services/token_report_service.py:195` `COALESCE(SUM(cost_usd), 0)`
- **思考**：單筆 cost_usd 是 NUMERIC(10,6)（上限 9999.999999）。若一日內呼叫 100,000 筆且每筆 ~$0.01，SUM 仍 < 10K 安全
- **判定**：v5.0 規模下不會炸；但 Phase 9 預算守門需重新評估 — 統帥可忽略。

#### L7. `total_duration` (Decimal) 隱式轉 float 可能損失精度

- **位置**：`services/token_report_service.py:30` `from decimal import Decimal` 但未使用
- **影響**：dead import，無功能影響
- **建議**：刪 line 30 import。

#### L8. `Section 4` 預算列在無預算時用 `_pad('未設定預算', 10)` 寬度可能斷行

- **位置**：`services/token_report_service.py:843-844`
- **驗證**：「未設定預算」5 個中文 = 10 寬度，剛好；無 padding 餘量。寫死 OK。
- **嚴重度**：LOW，FYI。

---

## Unit test 實測

```
$ /opt/anaconda3/bin/python3 -m pytest tests/test_ai_call_logger.py tests/test_token_report_service.py -v 2>&1 | tail -30

tests/test_token_report_service.py::TestQueriesViaMock::test_query_top_callers_orders_by_tokens PASSED [ 76%]
tests/test_token_report_service.py::TestQueriesViaMock::test_query_cost_breakdown_filters_zero_cost PASSED [ 78%]
tests/test_token_report_service.py::TestSendDailyReport::test_send_happy_path PASSED [ 80%]
tests/test_token_report_service.py::TestSendDailyReport::test_send_truncates_oversized_message PASSED [ 82%]
tests/test_token_report_service.py::TestSendDailyReport::test_send_resilient_to_telegram_failure PASSED [ 84%]
tests/test_token_report_service.py::TestSendDailyReport::test_generate_returns_failure_msg_when_db_dies PASSED [ 86%]
tests/test_token_report_service.py::TestTelegramTemplate::test_daily_token_report_appends_footer PASSED [ 88%]
tests/test_token_report_service.py::TestTelegramTemplate::test_daily_token_report_truncates_to_4096 PASSED [ 90%]
tests/test_token_report_service.py::TestTelegramTemplate::test_daily_token_report_escapes_footer_url PASSED [ 92%]
tests/test_token_report_service.py::TestFormatHelpers::test_fmt_kb PASSED [ 94%]
tests/test_token_report_service.py::TestFormatHelpers::test_esc_handles_none PASSED [ 96%]
tests/test_token_report_service.py::TestFormatHelpers::test_budget_line_zero_budget PASSED [ 98%]
tests/test_token_report_service.py::TestFormatHelpers::test_trend_line_handles_zero_baseline PASSED [100%]

============================== 52 passed in 0.21s ==============================
```

**結論**：52/52 全綠（22 ai_call_logger + 30 token_report_service），覆蓋:
- happy/exception/explicit-fallback/set-error 三種 context manager 路徑
- decorator + model_extractor + 例外 reraise
- DB 失敗 swallow / async dispatch 失敗 swallow
- COST_TABLE 各 provider 計算 + 未知 model + NIM 前綴自動 0 + 負數安全
- AI_CALL_LOGGING_ENABLED 開關 + kill-switch 連續失敗
- 6 條告警規則（spike/gemini share/error rate/budget/gcp hit/cache）+ insights 規則
- 報表 6 段落齊全 + 4096 截斷 + HTML escape + DB fail 路徑
- format helpers（fmt_kb/esc/budget_line/trend_line）邊界

**通過 ai_call_logger 「DB 失敗永不影響主流程」鐵律**：`test_db_failure_does_not_break_main_flow` + `test_async_dispatch_failure_swallowed` 兩條測試直接證明。

**通過「meta 不洩露 prompt 原文」鐵律**：`test_meta_does_not_leak_raw_prompt_into_call_state` + `test_set_prompt_hash_truncates_to_12` 兩條測試。

---

## 安全/PII 審計（六大類深審）

| 類別 | 結論 | 證據 |
|---|---|---|
| **A. logger 安全** | ✅ 失敗安全到位 | `_write_to_db` 全 try/except / kill-switch 在連續 10 次失敗觸發 (`_record_failure:80-89`) / `_async_write` 走 daemon thread 不阻塞 |
| **B. PII 保護** | ⚠️ **新發現 chat_id 進 meta（H6）** | logger 本身只用 `set_prompt_hash` 雜湊；但 4 個 Bot 入口直接灌 chat_id 進 meta — 違反規格 |
| **C. SQL Injection** | ✅ 全參數化 | `_exec_query` 強制走 SQLAlchemy `text(sql), params`；7 條報表 SQL 全用 named param |
| **D. HTML escape** | ✅ 對齊既有風格 | `_esc()` 對 `&<>` 三字元與 `telegram_templates._html_escape` 一致；所有 user-controlled (caller/model/error/insight text) 進 HTML 前都 escape |
| **E. 路由 / cron 衝突** | ✅ 23:55 唯一 | 17 條既有 cron 中無 23:5x 區段；資源競爭風險低（DB query 預估 < 30s） |
| **F. 預算 0 / 除 0** | ✅ 全數防護 | 7 處潛在除 0 全部有 `if X else default` 守衛 |

---

## 與既有系統整合風險

### 1. A4 修改 vs Phase 2 A6 即將修改 — **conflict 風險評估**

| 檔案 | A4（Phase 1）做了什麼 | A6（Phase 2）將做什麼 | Conflict 風險 |
|---|---|---|---|
| `services/ollama_service.py` | + `get_host_label()` / `OllamaResponse.host` 欄位 / `host=self.host` 4 處 | 預期擴 GCP/111 切換邏輯（B2）、補 token 欄位（M7） | 🟡 中 — 都動同一個 `OllamaResponse` dataclass + `generate()` 主體；建議 A6 先 rebase 再開工 |
| `services/code_review_pipeline_service.py` | 3 處包 log_ai_call（hermes/openclaw/elephant） | 修補 B3 / 接入 ad-hoc retry | 🟡 中 — 都動 `_hermes_scan` / `_openclaw_assess` 主流程；rebase 前先讀 A4 區塊 |
| `services/aider_heal_executor.py` | A4 **未動** | A6 將動 | ✅ 低 — 完全分離 |
| `routes/openclaw_bot_routes.py` | 4 處包 log_ai_call（main/gemini×2/nim） | 預期不會碰 | ✅ 低 |
| `services/hermes_analyst_service.py` | 2 處包 log_ai_call + 修 commit 00591c5 殘留 bug | 預期不碰 | ✅ 低 |
| `services/nemoton_dispatcher_service.py` | 1 處包 log_ai_call | 預期不碰 | ✅ 低 |
| `services/openclaw_strategist_service.py` | _call_gemini/_call_nvidia_nim 加 `caller=` 參數 | 預期不碰 | ✅ 低 |

**建議**：
- A6 開工前先 `git pull origin main` 拉到 A4 的 commit
- 動 `ollama_service.py` 時優先**新增**而非改既有 dataclass 欄位（minimize merge conflict）
- 動 `code_review_pipeline_service.py` 時保留現有 `with log_ai_call(...)` 包裝層，僅在内層修補

### 2. `commit 00591c5` 殘留 bug 修復確認

`services/hermes_analyst_service.py:194-200`：原本 commit 00591c5 動到 `except Exception as e:` 區塊時，誤把 `logger.warning` 抹除留下孤立 f-string。本輪 A4 順手修補：
```python
except Exception as e:
    # NOTE: 修補 commit 00591c5 殘留的孤立 f-string（原 logger.warning 被誤刪）
    logger.warning(
        f"[Hermes.intent] Ollama 連線失敗，降級規則引擎"
        f"（model={HERMES_MODEL} error={type(e).__name__}: {e}）"
    )
    _ctx.set_error(f"{type(e).__name__}: {e}")
    _ctx.fallback_to_caller('hermes_rule_engine')
    return None
```
✅ **已驗證**：logger.warning 完整呼叫 + ctx.set_error + fallback_to_caller 三件齊全。原 silent failure 反模式已破。

---

## Phase 2 銜接建議

統帥批准 Phase 2 A6 開工前，建議先 commit Phase 1 的所有變動到 main（A6 才有乾淨 baseline）。順序：

```bash
git add migrations/024 migrations/025 migrations/026                  # A3
git add services/ai_call_logger.py services/token_report_service.py   # A4/A5
git add tests/test_ai_call_logger.py tests/test_token_report_service.py  # A4/A5 tests
git add services/hermes_analyst_service.py services/nemoton_dispatcher_service.py
git add services/openclaw_strategist_service.py services/code_review_pipeline_service.py
git add services/ollama_service.py routes/openclaw_bot_routes.py
git add run_scheduler.py services/telegram_templates.py
git add docs/phase0_audit_report_20260503.md docs/phase1_db_design_20260503.md
git add docs/phase1_critic_review_20260503.md docs/phase1_final_critic_signoff_20260503.md
git commit -m "[Phase 1] Operation Ollama-First v5.0 觀測層落地 (A3 migration / A4 logger 13 callers / A5 token report)"
```

部署後第一波驗證（Phase 2 啟動前）：

```sql
-- 1. CHECK constraint 全在
SELECT conname FROM pg_constraint WHERE conrelid='ai_calls'::regclass ORDER BY conname;

-- 2. 種子預算 10 筆
SELECT period, provider, budget_usd, alert_pct FROM ai_call_budgets ORDER BY period, provider NULLS FIRST;

-- 3. logger 寫入煙測（A4 接入後第一次 LLM 呼叫應出現）
SELECT caller, provider, model, status, input_tokens, output_tokens, duration_ms
FROM ai_calls ORDER BY called_at DESC LIMIT 20;

-- 4. caller 分布（驗證 13 個白名單值）
SELECT caller, COUNT(*) FROM ai_calls GROUP BY caller ORDER BY 2 DESC;

-- 5. 失敗率（觀察 kill-switch 是否誤觸發）
SELECT status, COUNT(*) FROM ai_calls
WHERE called_at >= NOW() - INTERVAL '24h' GROUP BY status;
```

Phase 2 進度第 1 天觀察點：

- `SELECT count(*) FROM ai_calls;` 應 ≥ 100（戰役前審計 34 個呼叫點 / 一日 ~200 calls）
- `SELECT count(DISTINCT caller) FROM ai_calls;` 應 ≥ 13
- 23:55 cron 第一次跑完應有 1 筆 `ai_insights WHERE insight_type='daily_token_report'`

---

## NOTE 清單（Sign-off 條件）

deploy 後 7 天内由統帥決策處理：

- [ ] **NOTE-1（H5）**：考慮加 caller 格式 CHECK constraint（NOT VALID 不阻既存資料）
- [ ] **NOTE-2（H6）**：4 個 Bot 入口的 `chat_id` 改成 hash 後存（Phase 2 第一波 patch）
- [ ] **NOTE-3（M7）**：`OllamaResponse` 補 `prompt_tokens/completion_tokens` 欄位 → 修復 `openclaw_bot_main` token=0 黑洞（與 Phase 2 A6 ollama_service 改動合併）
- [ ] **NOTE-4（L1）**：deploy 前手動驗 `git fetch ewoooc && git log ewoooc/main --oneline -- migrations/`

deploy 後 30 天可選優化：

- [ ] **M9** ai_insights confidence 標 1.0 / **M10** HTML tag 截斷修補 / **L5** qwen3:14b 進 COST_TABLE / **L7** 刪 `Decimal` dead import
- [ ] **B1** ai_usage_tracking ORM 對齊真實 schema（雙寫 deprecate roadmap，與 ADR-028 合併）

---

## Final Sign-off

```
critic-A11 / 2026-05-03 / Phase 1 final closure
Verdict: APPROVED WITH NOTES
Tests:   52/52 PASSED (22 ai_call_logger + 30 token_report)
Findings (本輪新發現): 0 BLOCKER / 2 HIGH (H5/H6) / 4 MEDIUM (M7-M10) / 4 LOW (L5-L8)
Findings (前輪殘留):   1 HIGH (H4 文件) / 1 MEDIUM (M4 解耦) / 1 LOW (L1 統帥手驗)

簽署：critic-A11 (Operation Ollama-First v5.0 / Phase 1 sign-off)
```