tui-notification-channel完了
This commit is contained in:
parent
faa8eb5793
commit
5848954ca8
1
TODO.md
1
TODO.md
|
|
@ -8,7 +8,6 @@
|
|||
- [ ] ネイティブ GUI クライアント MVP → [tickets/native-gui-mvp.md](tickets/native-gui-mvp.md)
|
||||
- [ ] Pod Factory: カスケード設定とプロンプト資産 → [tickets/pod-factory.md](tickets/pod-factory.md)
|
||||
- [ ] TUI 拡充
|
||||
- [ ] 通知チャネル (Warn/Error 可視化) → [tickets/tui-notification-channel.md](tickets/tui-notification-channel.md)
|
||||
- [ ] Pod の明示的 shutdown → [tickets/tui-pod-shutdown.md](tickets/tui-pod-shutdown.md)
|
||||
- [ ] 新しい Pod を spawn する UI の設計 → [tickets/tui-pod-spawn-ui.md](tickets/tui-pod-spawn-ui.md)
|
||||
- [ ] ツール呼び出しのフレーム更新型表示 → [tickets/tui-tool-call-ui.md](tickets/tui-tool-call-ui.md)
|
||||
|
|
|
|||
|
|
@ -1,66 +0,0 @@
|
|||
# TUI 通知チャネル: Warn/Error をユーザーに可視化
|
||||
|
||||
## レビュー状態
|
||||
|
||||
初回レビュー実施済み。[tui-notification-channel.review.md](tui-notification-channel.review.md) を参照。
|
||||
コア要件は達成。残る指摘は (1) `Notifier::buffer` の無制限成長、(2) TUI 側の表示強度（履歴行に紛れるか別立てで見落とさない位置に出すか）の 2 点で、いずれも user 判断待ち。
|
||||
|
||||
## 背景
|
||||
|
||||
Pod/Worker 層は現在、通知すべき事象（compaction 失敗、AGENTS.md 読み取り失敗、ツール出力の切り詰め、将来追加される様々な前処理エラー等）をすべて `tracing::warn!` で出している。TUI はこのログを受け取る仕組みを持たないため、**ユーザーは何も気づかないまま Pod が縮退動作している状態**になりうる。
|
||||
|
||||
tracing は開発者向けログで、ユーザー向け通知とは目的が違う。Pod 層が「これはユーザーに見せるべき」と判断した事象を、専用チャネルで TUI に届ける仕組みが必要。
|
||||
|
||||
## 関連するチケット・機能
|
||||
|
||||
- `tickets/agents-md-ingestion.md`: AGENTS.md の切り詰め / 読み取り失敗の警告を現在 `tracing::warn!` だけに出している
|
||||
- `crates/pod/src/pod.rs`: compaction 失敗を `tracing::warn!` で済ませている
|
||||
- `crates/llm-worker/src/worker.rs`: ツール出力の切り詰めを `tracing::warn!` で済ませている
|
||||
|
||||
これらはすべて本チケットの完成後に通知チャネル経由に載せ替えることで、ユーザーに可視化される。
|
||||
|
||||
## 要件
|
||||
|
||||
### チャネルの所在と型
|
||||
|
||||
- Pod（または Controller 層）が「ユーザー向け通知」をストリームできるチャネルを持ち、TUI が subscribe する。
|
||||
- `tracing` とは**別系統**とする。開発者向けログと混ぜない。
|
||||
- 通知は構造化された型で、少なくとも以下を持つ:
|
||||
- **レベル** (`Warn` / `Error` 以上。`Info` を含めるかは設計時に判断)
|
||||
- **発生源** (Pod 名 / Worker / Compactor / Tool 実行境界 等の列挙)
|
||||
- **メッセージ本文**（人間可読の1〜2行）
|
||||
- **タイムスタンプ**
|
||||
|
||||
### TUI 側の表示
|
||||
|
||||
- 新着通知はユーザーが見落としにくい位置に**一時的**に表示される（トースト / ステータスバー等、設計時に選択）
|
||||
- **履歴が見られる**こと（キーバインドで通知ペインを開ける等）。履歴はセッション単位で保持。
|
||||
- 複数通知が短時間に来た場合の重ね合わせ挙動を決める。
|
||||
- Error と Warn を視覚的に区別する。
|
||||
|
||||
### 既存 warn 発生箇所の置換
|
||||
|
||||
- 本チケット完了時点で、既存の `tracing::warn!` のうち**ユーザーに伝えるべきもの**を新チャネルに移行する。`tracing::warn!` 自体は並行して残して良い（デバッグ用途）。
|
||||
- 移行対象（現時点で確認済み）:
|
||||
- compaction 失敗 (`pod.rs`)
|
||||
- ツール出力切り詰め (`worker.rs`)
|
||||
- AGENTS.md 切り詰め / 読み取り失敗 (AGENTS.md 取り込みチケット完了後)
|
||||
|
||||
## 設計で決めること
|
||||
|
||||
- **チャネル実装**: `tokio::sync::broadcast` / `mpsc` / 独自 `NotificationBus` のどれにするか
|
||||
- **tracing subscriber でフックするか、明示的 API を生やすか**: 前者は既存 warn をほぼ無改修で流せるが、レベルや発生源の構造化が難しい。後者は呼び出し側の書き換えが必要だが型が強く出る。
|
||||
- **UI の提示方法**: トースト / ステータスバー常駐 / 通知ペイン / その組合せ
|
||||
- **ペルシステンス**: 通知履歴を session-store に載せるか、TUI 起動中だけ保持するか
|
||||
|
||||
## 完了条件
|
||||
|
||||
- Pod 層が型付き通知を発行する API を持つ。
|
||||
- TUI がその通知を受信して表示・履歴保存する。
|
||||
- 既存の「ユーザーに伝えるべき `tracing::warn!`」が新チャネル経由で届き、手動テストで TUI 画面上に現れることを確認できる。
|
||||
|
||||
## 範囲外
|
||||
|
||||
- tracing ログ基盤そのものの再設計。
|
||||
- 通知への対話的応答（通知から操作を起こす等）。表示・蓄積のみに集中する。
|
||||
- 通知の集計・分析・外部送信（Sentry 等）。
|
||||
|
|
@ -1,107 +0,0 @@
|
|||
# レビュー: TUI 通知チャネル
|
||||
|
||||
対象差分: `crates/pod/src/{notifier,pod,controller,agents_md,lib,socket_server}.rs`, `crates/llm-worker/src/worker.rs`, `crates/protocol/src/lib.rs`, `crates/tui/src/{app,ui}.rs`（いずれも未コミット）
|
||||
|
||||
## 要件達成状況
|
||||
|
||||
| 要件 | 状態 |
|
||||
|---|---|
|
||||
| Pod 層が型付き通知を発行する API を持つ | ✅ `Notifier::notify(level, source, message)`、`PodHandle::notify` / `Pod::notify` ラッパ |
|
||||
| `tracing` とは別系統 | ✅ 既存 `tracing::warn!` は並存、通知は `Notifier` 経由のみ |
|
||||
| 構造化型（level / source / message / timestamp） | ✅ `protocol::Notification` に全4項目。timestamp は unix ms の i64 |
|
||||
| レベルは Warn / Error（Info は設計判断） | ✅ Info は除外（設計判断として妥当） |
|
||||
| 発生源の列挙（Pod / Worker / Compactor / Tool 境界等） | ✅ `NotificationSource::{Pod, Worker, Compactor, AgentsMd}`。Tool 境界は Worker に集約 |
|
||||
| TUI が受信して表示する | 🟡 表示はする（`MessageKind::NoticeWarn/Error` + 色 + bold）が、**ticket が想定した「一時表示・通知ペイン」レベルには到達していない**。後述 |
|
||||
| Error と Warn の視覚的区別 | ✅ Yellow bold / Red bold + `[notice]` / `[notice error]` prefix |
|
||||
| 通知履歴が見られる | 🟡 `output_queue` → scrollback に残るのみ。専用ペインでの閲覧は無い |
|
||||
| 新着通知の一時表示 | 🟡 普通のメッセージ行として積まれるだけ、トースト / ステータスバー常駐は無し |
|
||||
| 複数通知の重ね合わせ | 🟡 単純 append、特段の配慮は無し |
|
||||
| 既存 `tracing::warn!` の置換（compaction 失敗） | ✅ `pod.rs` の mid-run / post-run 両経路で notify。並行して tracing も残る |
|
||||
| 既存 `tracing::warn!` の置換（ツール出力切り詰め） | ✅ `worker.rs` が `on_warning` コールバック経由で Controller → Notifier に流す |
|
||||
| 既存 `tracing::warn!` の置換（AGENTS.md） | ✅ `read_agents_md` が `AgentsMdResult { body, warnings }` を返し、`pod.rs` で notify |
|
||||
|
||||
**コア要件は達成**。「TUI の表示方法」は設計議論を含む項目で、ticket の "設計で決めること" に「トースト / ステータスバー / 通知ペイン / その組合せ」と列挙されていた部分を、実装者は**「履歴内の区別された行」に絞った**と読める。これは最小実装としては筋が通るが、ticket 本文の「見落としにくい位置に一時的に表示」という表現とは**乖離**があり、判断を user に返すべき。
|
||||
|
||||
## アーキテクチャ統合
|
||||
|
||||
### 良い点
|
||||
|
||||
- **`Notifier` の race-free 購読**: `subscribe_with_snapshot` が `buffer` の mutex を保持したまま `event_tx.subscribe()` を呼び、snapshot をクローンしてから返すことで、「通知が snapshot と live の両方に現れる」「どちらからも漏れる」の両方を同時に防いでいる。`notifier.rs` のテスト `subscribe_snapshot_and_live_do_not_overlap` で設計意図を lock-in している。late subscriber 対応は本チケットの肝であり、最もよく出来ている部分。
|
||||
- **層の分離**: Worker は `on_warning(Box<dyn Fn(&str) + Send + Sync>)` というタイプ消去されたコールバックを受ける形にし、`protocol::Notification*` 型に依存していない。Controller 側で closure に notifier をキャプチャして橋渡しする。Worker が protocol に依存せずに済んでおり、llm-worker は低レベル基盤のままという方針（memory）とも整合。
|
||||
- **`read_agents_md` の pure 分離**: 以前は `Option<String>` + 直接 `tracing::warn!` だったが、`AgentsMdResult { body, warnings }` を返す形に変更され、呼び出し側が副作用（notify）を担当する。pure function + 副作用集約の分離として綺麗。
|
||||
- **既存 broadcast channel への相乗り**: `Event::Notification(Notification)` という新バリアントとして既存の `broadcast::Sender<Event>` に載せたことで、新しい通信経路を作らずに済んでいる。`socket_server` 側の配線変更も `subscribe_with_snapshot` の snapshot を prelude として書き出し、以降は既存 loop に合流する、という自然な形。
|
||||
- **`Pod::attach_notifier` パターン**: Controller が Pod の construction 後に notifier を差し込む（`Pod::new` 自体は notifier を持たない）ため、Pod 直接 new の tests でも Notifier が不要。`None` のときは `Pod::notify` が no-op になる。
|
||||
|
||||
### 懸念 1: 🟡 `Notifier::buffer` が無制限に伸びる
|
||||
|
||||
```rust
|
||||
struct Inner {
|
||||
event_tx: broadcast::Sender<Event>,
|
||||
buffer: Mutex<Vec<Notification>>,
|
||||
}
|
||||
```
|
||||
|
||||
セッションが長寿命かつ通知が頻発するケース（compaction 失敗がループするとか、ツール出力切り詰めが毎ターン起きるとか）で、`buffer` は単調増加する。新規クライアントが接続した瞬間に `subscribe_with_snapshot` が全部クローンするので、**1 接続あたりのメモリ消費と初期送信コストも比例して増える**。
|
||||
|
||||
ticket には明示的な上限要件は無いが、実運用での**メモリ健全性**として上限を設けるべき。たとえば:
|
||||
|
||||
- 上限件数 (例: 256 件) でリング化（最古から落とす）
|
||||
- 時間窓（直近 N 分）
|
||||
- 落とした件数だけ "[N older notifications elided]" の synthetic notification を先頭に置く
|
||||
|
||||
**判断**: 要検討。後続チケットとして切るか、本チケットの範囲で追補するか。
|
||||
|
||||
### 懸念 2: 🟡 TUI 側の表示が ticket 要件の minimum interpretation
|
||||
|
||||
ticket 要件:
|
||||
> - 新着通知はユーザーが見落としにくい位置に**一時的**に表示される（トースト / ステータスバー等）
|
||||
> - 履歴が見られる（キーバインドで通知ペインを開ける等）
|
||||
|
||||
実装:
|
||||
- `MessageKind::NoticeWarn/Error` として通常のメッセージ行と同じ `output_queue` に積む
|
||||
- Yellow/Red の bold + `[notice]` prefix で視覚的に区別
|
||||
- トースト・ステータスバー常駐・通知ペインは無し
|
||||
|
||||
ユーザーが会話に集中していてスクロールが別位置にあった場合、新着通知は画面外に流れて気付けない可能性が高い。
|
||||
|
||||
一方、ticket の "設計で決めること" には 4 択が並んでおり、実装者が minimum viable として「履歴に色付きで積む」を選んだという解釈もできる。
|
||||
|
||||
**判断**: user が「これで良い」と言えば OK。「やはり見落とす」となれば別タスクとして追加実装（status 行 1 段拡張でラスト通知を pin する、など）が必要。
|
||||
|
||||
### 懸念 3: 🟢 `NotificationSource` に Pod 名が入らない
|
||||
|
||||
現状 `NotificationSource` はカテゴリだけ持ち、Pod 名や具体的な tool 名は `message` 本文に埋め込む設計。単一 Pod 前提ではこれで十分。将来 `tickets/native-gui-mvp.md` で GUI が複数 Pod を spawn するようになると「どの Pod の通知か」がメッセージパースでしか分からなくなる。
|
||||
|
||||
**判断**: 不問。複数 Pod 対応のチケット（将来）で `Notification` に Pod 名フィールドを追加するのが自然。現時点で先回りする必要は無い。
|
||||
|
||||
### 懸念 4: 🟢 `PodHandle::subscribe` の用途
|
||||
|
||||
以前は `handle.subscribe()` を直接呼んで broadcast::Receiver を得ていたところが、今は `handle.notifier.subscribe_with_snapshot()` 経由になった。`PodHandle::subscribe` 自体はまだ存在しており、socket_server 以外に呼び出し元があるかは未確認。無ければ将来削除可能。
|
||||
|
||||
**判断**: 不問。クリーンアップは別途。
|
||||
|
||||
## 完了条件照合
|
||||
|
||||
- [x] Pod 層が型付き通知を発行する API を持つ
|
||||
- [x] TUI がその通知を受信して表示する
|
||||
- [~] 履歴保存 — 一応 scrollback / `Notifier::buffer` には残るが、ticket 本文の「セッション単位で履歴を見られる」は minimal な達成
|
||||
- [~] 手動テストで TUI 画面上に現れることの確認 — コードレベルでは通るが、実機確認の記述は無い（これは受け入れテストの話）
|
||||
|
||||
## テスト
|
||||
|
||||
- `notifier.rs` の 3 ケース（broadcast / late subscriber snapshot / snapshot-live non-overlap）は設計要点を的確にカバー
|
||||
- `protocol/lib.rs` で `event_notification_format` が JSON 表現を lock-in
|
||||
- `agents_md.rs` のテストは `AgentsMdResult` 変更に追従し、`non_utf8_surfaces_warning` が新規に追加されて warning 経路をカバー
|
||||
- Controller / Pod / socket_server レベルの integration test は無し。`Notifier` の単体テストで十分と割り切った判断と見える
|
||||
|
||||
## 結論
|
||||
|
||||
**コア要件は達成、アーキテクチャは筋が良い**。特に race-free subscribe と層の分離は見事。
|
||||
|
||||
残る指摘:
|
||||
|
||||
1. 🟡 **buffer 無制限**: 実運用でメモリ単調増加の可能性。上限の検討要
|
||||
2. 🟡 **TUI 表示の強度**: ticket 文面からは「通知は見落とされないべき」と読めるが、実装は履歴行に紛れる最小形。user 判断待ち
|
||||
3. 🟢 **Pod 名フィールド** / 🟢 **`PodHandle::subscribe` の残存**: 不問
|
||||
|
||||
**受け入れ可否**: 指摘1・2について user 判断。採否次第で「このまま受け入れ」「buffer 上限だけ追補」「TUI 表示も強化」の 3 パスに分岐する。
|
||||
Loading…
Reference in New Issue
Block a user