Files
awoooi/docs/adr/ADR-047-phase-r-architecture-review.md
OG T 22de22c989 refactor(phase-s): Phase S 技術債清理 - 五項架構改善
S-01: generate_alert_fingerprint() 移至 alert_analyzer_service (Router→Service)
S-02: 移除廢棄 USE_NEW_ENGINE config (Phase R 已完成歷史使命)
S-03: github_webhook.py linter 清理 (Field unused + delivery_id noqa)
S-04: Pydantic v2 遷移 - approval/incident models (class Config → ConfigDict)
S-05: Skill 09 v1.1 更新 (USE_NEW_ENGINE 廢棄說明)

測試: 393 passed, 零失敗

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-01 13:12:02 +08:00

246 lines
9.5 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.
# ADR-047: Phase R 架構大掃除完整審查
| 項目 | 內容 |
|------|------|
| **狀態** | ✅ 審查通過 (97/100 OUTSTANDING) |
| **日期** | 2026-04-01 |
| **審查者** | 首席架構師 |
| **涵蓋範圍** | Phase R R1-R4 全部 + ADR-046 IncidentConverter |
---
## 審查總覽
| Phase | 任務 | 評分 | 狀態 |
|-------|------|------|------|
| R1 | 絞殺者包裝 (USE_NEW_ENGINE) | 25/25 | ✅ |
| R2 | 移除內嵌重複邏輯 | 24/25 | ✅ |
| R3 | Repository 層抽取 | 25/25 | ✅ |
| R4 | Router 層瘦身 | 23/25 | ✅ |
| ADR-046 | IncidentConverter 型別統一 | --- | ✅ 加分 |
| **合計** | | **97/100** | **OUTSTANDING** |
---
## R1: 絞殺者包裝 ✅ 25/25
### 設計意圖
`USE_NEW_ENGINE` 旗標為開關,保護遷移期間的服務可用性。
Strangler Fig Pattern Phase 1 (Identify + Wrap)。
### 驗證結果
| 檢查項 | 結果 |
|--------|------|
| `USE_NEW_ENGINE=True` 預設值 | ✅ 確認 |
| 新舊路徑可切換 | ✅ 架構完整 |
| 不影響現有行為 | ✅ 漸進遷移 |
| Phase 16 R1 commit 完成 | ✅ 歷史確認 |
> **P2-03 補充**: `USE_NEW_ENGINE` 旗標於 Phase R-R2 後已名存實亡(移除舊路徑後無法回滾至舊實作)。已正式標記失效,回滾方式改為 `git revert c7b3f8f d17b67c`。處置正確。
---
## R2: 移除內嵌重複邏輯 ✅ 24/25
### 刪除量
| 檔案 | 刪除行數 | 說明 |
|------|---------|------|
| `incident_memory.py` | -480 行 | DualIncidentMemory + IIncidentMemory 內嵌版 |
| `incident_engine.py` | -490 行 | IncidentEngine 內嵌版 + Lua 死碼 |
| **合計** | **-970 行** | |
### P0+P1 修復確認 (commit `d17b67c`)
| 優先級 | 項目 | 狀態 |
|--------|------|------|
| P0 | Redis key prefix 修正 (`awoooi:incidents:`) | ✅ |
| P0 | `_record_to_incident` 型別標注修正 (`Any`) | ✅ |
| P0 | 死碼 `LUA_SCRIPT` 移除 | ✅ |
| P1 | `IIncidentEngine.update_status` 簽名對齊 brain | ✅ |
### P2 技術債追蹤 (ADR-046)
| # | 問題 | 狀態 |
|---|------|------|
| P2-01 | `signal_worker` `persisted_to_pg` AttributeError | ✅ `getattr` 防禦 |
| P2-02 | `IIncidentEngine` Protocol 簽名不符 brain | ✅ `update_status` 對齊 |
| P2-03 | `USE_NEW_ENGINE` 旗標名存實亡 | ✅ 標記失效 + 回滾路徑更新 |
**扣分說明**: 文件同步略有延遲 (-1)ADR-024 執行進度表已更新。
---
## R3: Repository 層抽取 ✅ 25/25
### 9 個 Repository 建立完成
| Repository | 職責 | 狀態 |
|-----------|------|------|
| `approval_repository.py` | 簽核記錄 CRUD | ✅ |
| `audit_log_repository.py` | 稽核日誌 | ✅ |
| `embedding_repository.py` | 向量嵌入 | ✅ |
| `incident_repository.py` | 事件記錄 | ✅ |
| `interfaces.py` | Repository Interface | ✅ |
| `k8s_repository.py` | K8s 狀態查詢 | ✅ |
| `learning_repository.py` | 學習記錄 | ✅ |
| `metrics_repository.py` | 指標數據 | ✅ |
| `playbook_repository.py` | 劇本查詢 | ✅ |
### 架構符合性
| ADR-024 標準 | 結果 |
|-------------|------|
| Repository 只含 SQL/ORM + Redis 操作 | ✅ |
| Service 層透過 Repository 存取資料 | ✅ |
| Router 層不直接呼叫 Repository | ✅ (Phase 22 P1 已清除) |
**Phase 22 首席架構師審查已通過,得分確認。**
---
## R4: Router 層瘦身 ✅ 23/25
### 四項任務完成狀態
| # | 任務 | 位置 | 狀態 | 修復 Phase |
|---|------|------|------|-----------|
| 127 | incidents.py Redis 直接操作 | `api/v1/incidents.py` | ✅ | Phase 17 P0 |
| 128 | approvals.py Lua Script | `api/v1/approvals.py` | ✅ | Phase 17 P0 |
| 129 | webhooks.py 業務邏輯抽取 | `api/v1/webhooks.py` | ✅ | 本 Session (`4118808`) |
| 130 | telegram.py 格式化 | `api/v1/telegram.py` | ✅ | Phase 22 P2 |
### #129 遷移成果詳細 (commit `4118808`)
| 遷移內容 | 舊位置 | 新位置 |
|---------|--------|--------|
| `AlertAnalyzer` class (141 行) | `webhooks.py:625-765` | `services/alert_analyzer_service.py` |
| `AlertPayload` model | `webhooks.py` | `models/webhook.py` |
| `AlertResponse` model | `webhooks.py` | `models/webhook.py` |
| `normalize_resource_name` 使用 | `webhooks.py` (直接呼叫) | `alert_analyzer_service.py` (ADR-016 整合) |
| **net 減少** | **-243 行** | |
### 現況驗證
```python
# webhooks.py 現在的 import 模式 ✅
from src.models.webhook import AlertPayload, AlertResponse # models 層
from src.services.alert_analyzer_service import AlertAnalyzer # services 層
from src.services.incident_service import get_incident_service # Phase 17 P0
```
### 殘留項目說明
`generate_alert_fingerprint()` (22 行) 仍在 `webhooks.py:344`
**分析**:
- 純函數,僅使用 `hashlib.sha256()` 對字串做 Hash
- 無 Redis/DB/外部 API 存取(符合 ADR-024 的核心禁令)
- 技術上為「業務邏輯 >10 行」,屬 P3 可接受殘留
- 建議未來 Phase 中移至 `services/alert_analyzer_service.py`
**扣分**: -2殘留的 >10 行業務邏輯函數;但不含外部存取,風險極低)
---
## ADR-046: IncidentConverter 型別統一(加分項)
### 實作完成度
| 項目 | 檔案 | 狀態 |
|------|------|------|
| `brain_to_local()` | `utils/incident_converter.py` | ✅ |
| `local_to_brain()` | `utils/incident_converter.py` | ✅ |
| `IncidentEngineAdapter` | `services/incident_engine.py` | ✅ |
| `proposal_service` 清理 (#123) | `services/proposal_service.py` | ✅ commit `44840f5` |
### 邊界轉換點確認
| 位置 | 方向 | 狀態 |
|------|------|------|
| `IncidentDbAdapter._record_to_incident()` | DB→BrainIncident (brain 內部) | ✅ 邊界清晰 |
| `IncidentEngineAdapter.process_signal()` | BrainIncident→LocalIncident | ✅ |
| `IncidentEngineAdapter.get_incident()` | BrainIncident→LocalIncident | ✅ |
| `proposal_service._persist_incident()` | LocalIncident→BrainIncident (save) | ✅ |
### proposal_service #123 清理確認
```python
# 修復前 ❌
from src.core.redis_client import get_redis # Router-like 直接 Redis
INCIDENT_KEY_PREFIX = "incident:" # 錯誤 key prefix
# 修復後 ✅ (commit 44840f5)
async def _load_incident(self, incident_id: str) -> Incident | None:
return await get_incident_engine().get_incident(incident_id) # ADR-046
async def _persist_incident(self, incident: Incident) -> None:
brain_incident = local_to_brain(incident)
await get_incident_memory().save_incident(brain_incident) # 正確 prefix
```
---
## 整體架構符合性
### ADR-024 四層架構最終狀態
```
┌─────────────────────────────────────────────────┐
│ Router Layer (api/v1/*.py) │
│ ✅ 無 Redis/DB 直接存取 │
│ ✅ 無 AlertAnalyzer 等業務邏輯 │
│ ⚠️ generate_alert_fingerprint() 純函數 (P3) │
└─────────────────────────────────────────────────┘
┌─────────────────────────────────────────────────┐
│ Service Layer (services/*.py) │
│ ✅ AlertAnalyzer 正確位置 │
│ ✅ IncidentEngineAdapter 轉換邊界 │
│ ✅ ProposalService 委派 IncidentEngine │
└─────────────────────────────────────────────────┘
┌─────────────────────────────────────────────────┐
│ Repository Layer (repositories/*.py) │
│ ✅ 9 個 Repository 全部建立 │
│ ✅ 介面分離 (interfaces.py) │
└─────────────────────────────────────────────────┘
┌─────────────────────────────────────────────────┐
│ Model Layer (models/*.py) │
│ ✅ AlertPayload/AlertResponse 正確位置 │
│ ✅ IncidentConverter 邊界明確 │
└─────────────────────────────────────────────────┘
```
---
## P2 技術債清單Phase S 追蹤)
| # | 項目 | 位置 | 優先級 |
|---|------|------|--------|
| S-01 | `generate_alert_fingerprint()` 移至 Service | `webhooks.py:344` | P3 |
| S-02 | `USE_NEW_ENGINE` config 項清除 | `core/config.py` | P3 |
---
## 審查結論
**評分: 97/100 OUTSTANDING**
Phase R 絞殺者模式四階段全部完成。核心指標:
- **-970 行** 重複邏輯移除(最大技術債清除)
- **9 個 Repository** 建立,分層架構完整
- **AlertAnalyzer** (141 行業務邏輯) 從 Router 遷出
- **ADR-046 IncidentConverter** 型別邊界清晰
遺留 2 分:`generate_alert_fingerprint()` 純函數尚在 Router 文件,
為 P3 低風險殘留,不影響系統正確性。
> **首席架構師簽核**: Phase R 完整通過,可進入下一個 Phase。
> **簽核日期**: 2026-04-01 (台北時間)