diff --git a/CHANGELOG.md b/CHANGELOG.md index c0ff6a5..1472de9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,7 +37,16 @@ All notable changes to SyncMill are documented here. checkpoint and before the task, with the excerpts and memory context shrinking to make room. A resumed conversation keeps what earlier rounds sent it, so a round that would resume one in a room with earlier lessons adds a note about - `--fresh-session`. TUI-launched rounds do not send lessons yet. + `--fresh-session`. +- The TUI Rooms tab records verdicts and promotes lessons (#174): `j` records + accepted/rejected on an applied room, bound to the patch digest the dialog + showed; `p` promotes a lesson from a judged room and refuses if the verdict + changed while the dialog was open (`RoomCoordinator.promote_lesson` gains + `expected_judgment_id`). The room pane shows the outcome. TUI rounds now send + the project's approved lessons: the round dialog lists them with checkboxes, + applies the CLI's skip and budget rules live, and refuses to start when the + prompt leaves no room for the checked lessons; the history note shows there, + before you confirm, for the lineage you pick. The round conversation record is now `schema_version` 2: it adds the selected lesson ids and each member's delivery, shown by `room compare`, `room export` and the TUI (`rounds[].conversation.lessons`, @@ -70,6 +79,16 @@ All notable changes to SyncMill are documented here. ### Fixed +- The TUI no longer exits when the board's decisions or the selected item + cannot be read during a Board-tab refresh; it shows `DEGRADED` and keeps + the last view, as it already did for the item list. +- A TUI round, verdict or promotion acts on the room its dialog was opened + for. A round dialog used to launch on whichever room was selected when you + confirmed, which a refresh could have changed meanwhile. +- A verdict recorded from any surface now marks an open TUI Compare or + Details snapshot as changed; a verdict changes neither the room's revision + nor its candidates, so the snapshot used to keep showing the old outcome + without saying so. - `syncmill start` suggests verification commands only from files whose name matches exactly, so each suggestion names a file that is on disk. On a case-insensitive filesystem (the macOS default) a repository with only a diff --git a/CLAUDE.md b/CLAUDE.md index d1eea0a..a2496ef 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -31,7 +31,7 @@ uv run syncmill room export --include patch: [--include output:| # room rounds record their effective starting tree + gate settings (v2 adds the gate's env_allowlist: null = inherited, [] distinct) in rounds.provenance_json (rooms.round_provenance_status, writer and readers share it, readers accept v1+v2; NULL = legacy); compare/export/TUI show them as starting tree / gate settings beside the report-derived gate profile, whose JSON fields are unchanged; a v1 round's environment reads not recorded and never groups as same settings; `room apply --verify` still runs the current `[gate]` and prints a `verify:` notice (`rooms.verify_settings_notice`) unless the round's v2 record equals it uv run syncmill room select [--expected-revision N --expected-digest SHA256 --rationale TEXT] # verifies the artifact, records a bound selection (rooms.selection_json); apply refuses an inconsistent record uv run syncmill room judge accepted|rejected [--expected-digest SHA256 --rationale TEXT] # #174: append-only human verdict on the APPLIED candidate (room_judgments, bound to candidate + patch digest + applied run; never inferred from select/apply, never touches memory, no state/revision change); rooms.room_outcome_status is the one read model (not_selected / selected_not_applied / applied_unreviewed / accepted / rejected / unknown — a damaged latest record is unknown, never an older verdict) shown by compare/export as room.outcome -uv run syncmill room promote --lesson TEXT [--scope GOAL --supersedes ID] # #174 PR 2: needs the board; only a judged room (accepted/rejected); a board decision bound to project_id with provenance_json (room/judgment/verdict/candidate/digest/run) and a unique source_key (retry returns the existing decision, never reactivates a superseded one); `run` shows then injects Board.project_lessons (project-wide scope only; board.select_lessons budget; --skip-lesson/--no-lessons, forwarded in _inner_argv) via Supervisor.run(lessons=) AFTER routing classified the raw prompt; board items get unbound + own-project decisions; room rounds: `room round`/`start` show them (cli._show_round_lessons, rooms.round_lessons_fit) and pass RoundLesson pairs to run_round(lessons=) (detached: a second anonymous fd, --lessons-fd); the coordinator never opens the board; lessons sit after the checkpoint, before the task, excerpts budgeted around them; TUI rounds send none (note only) +uv run syncmill room promote --lesson TEXT [--scope GOAL --supersedes ID] # #174 PR 2: needs the board; only a judged room (accepted/rejected); a board decision bound to project_id with provenance_json (room/judgment/verdict/candidate/digest/run) and a unique source_key (retry returns the existing decision, never reactivates a superseded one); `run` shows then injects Board.project_lessons (project-wide scope only; board.select_lessons budget; --skip-lesson/--no-lessons, forwarded in _inner_argv) via Supervisor.run(lessons=) AFTER routing classified the raw prompt; board items get unbound + own-project decisions; room rounds: `room round`/`start` show them (cli._show_round_lessons, rooms.round_lessons_fit) and pass RoundLesson pairs to run_round(lessons=) (detached: a second anonymous fd, --lessons-fd); the coordinator never opens the board; lessons sit after the checkpoint, before the task, excerpts budgeted around them; TUI (Rooms `j` verdict / `p` promote, `tui.py` `_confirm_room_judge`/`_confirm_lesson_promote`): the round dialog reads `project_lessons` once on the Textual thread (`RoundLessonsView`), recomputes `select_lessons(skip=unchecked)` live, refuses (never drops) when `round_lessons_fit` fails on `round_prompt`'s fallback rule, and launches with exactly those pairs; promote passes `expected_judgment_id` (checked on the same outcome read as the provenance); the round/verdict/promote dialog callbacks act on the captured room id + store root uv run syncmill conversations list uv run syncmill conversations show uv run syncmill conversations close diff --git a/docs/architecture/tui.md b/docs/architecture/tui.md index bc1ef40..3cfc3cb 100644 --- a/docs/architecture/tui.md +++ b/docs/architecture/tui.md @@ -86,6 +86,29 @@ the store root changes or the store becomes unavailable. Both views read rows from the snapshot (no artifact read on the event loop), so their gate/eligibility/usage/blocker wording is the CLI's; the Output view (`o`) interprets none of it. +Rooms review actions (#174): `j` records a verdict (`RoomCoordinator.judge`, +`surface="tui"`, fenced on the patch digest the dialog showed), `p` promotes a +lesson (`promote_lesson(expected_judgment_id=)`, refused when the latest +judgment is no longer the one the dialog showed), and the `x` round dialog +shows the project's approved lessons (`RoundLessonsView`, read once when it +opens) and recomputes the CLI's `select_lessons(skip=)` on every checkbox, +prompt or lineage change. It refuses instead of dropping lessons when the +prompt leaves no room (`round_prompt` gives the fit check and the launch one +fallback rule), and the worker gets exactly the shown pairs. The round, verdict +and promote dialogs capture `(room id, store root)` when they open; its callback acts on those and +refuses when the store was rebound, never reading the ambient selection. Judge +and promote run on the Textual thread on the existing connections (SQLite is +thread-affine; the board connection must not enter a worker), accepting up to +the stores' 5 s busy timeout of blocking under write contention. The evidence +snapshot also records the room's judgment count, read before the build: a +verdict changes neither revision nor candidates, so `changed` compares the +count too; a failed count read, at the build or now, degrades to a "cannot +tell" line instead of aborting the staged room update. Checkbox labels built from stored text go +through `plain(...)` (Textual parses a `str` label as markup); the +`markup_sinks` guard flags any other non-literal label. A lesson's checkbox +label is only its id: a `Checkbox` label never wraps, so a long lesson would be +cut off at the dialog edge while every word of it is sent. The full text is a +wrapping `PlainStatic` under it. EVERY poll path is degrade-guarded (`_ROOM_POLL_ERRORS`/`_SESSION_POLL_ERRORS`/ `_JOB_POLL_ERRORS`) — an unguarded read escapes the interval callback and kills the app, so a store failure must show DEGRADED and keep the last good view. diff --git a/docs/assets/tui/09-rooms-diff.svg b/docs/assets/tui/09-rooms-diff.svg index 2b42eb0..3f0e39d 100644 --- a/docs/assets/tui/09-rooms-diff.svg +++ b/docs/assets/tui/09-rooms-diff.svg @@ -172,7 +172,7 @@ - + ⭘syncmill tui BoardRunsReportsTeamsStatusSessionsRooms @@ -186,10 +186,10 @@ ▊▎│ ▊▎│Recent events ▊▎│round 1 completed -▊▎│candidate selected -▊▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▎│Candidate: -┌──────────────────────────────────────────────────────────────────────┐│a1b2c3d4-5e6f-4a7b-8c9d-0e1f2a3b4c5d | Diff -▊a1b2c3d4 codex completed ✓▎│sha256: fedcba9876543210 +▊▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▎│candidate selected +┌──────────────────────────────────────────────────────────────────────┐│Candidate: +▊a1b2c3d4 codex completed ✓▎│a1b2c3d4-5e6f-4a7b-8c9d-0e1f2a3b4c5d | Diff +▊▎│sha256: fedcba9876543210 ▊▎│ ▊▎│diff --git a/room-smoke.txt b/room-smoke.txt ▊▎│new file mode 100644 @@ -208,11 +208,11 @@ ▊▎│ ▊▎│ ▊▎│ -▊▎│ -▊▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▎│ -rooms:1 | candidates:1 | view:diff - -a create | x round | t terminal | d/o/m/c diff/output/details/compare | e select | g apply | R recover | k close +▊▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▎│ +rooms:1 | candidates:1 | view:diff + +a create | x round | t terminal | d/o/m/c diff/output/details/compare | e select | g apply | j verdict | p promote | R +recover | k close 1-7 tabs | r refresh | ? help | q quit diff --git a/docs/assets/tui/10-rooms-output.svg b/docs/assets/tui/10-rooms-output.svg index a22c202..61513bd 100644 --- a/docs/assets/tui/10-rooms-output.svg +++ b/docs/assets/tui/10-rooms-output.svg @@ -172,7 +172,7 @@ - + ⭘syncmill tui BoardRunsReportsTeamsStatusSessionsRooms @@ -186,10 +186,10 @@ ▊▎│ ▊▎│Recent events ▊▎│round 1 completed -▊▎│candidate selected -▊▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▎│Candidate: -┌──────────────────────────────────────────────────────────────────────┐│a1b2c3d4-5e6f-4a7b-8c9d-0e1f2a3b4c5d -▊a1b2c3d4 codex completed ✓▎│[Output] +▊▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▎│candidate selected +┌──────────────────────────────────────────────────────────────────────┐│Candidate: +▊a1b2c3d4 codex completed ✓▎│a1b2c3d4-5e6f-4a7b-8c9d-0e1f2a3b4c5d +▊▎│[Output] ▊▎│State: completed | exit: 0 | 18.2s ▊▎│Gate, usage, eligibility: see Details (m) or ▊▎│Compare (c) @@ -208,11 +208,11 @@ ▊▎│ ▊▎│ ▊▎│ -▊▎│ -▊▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▎│ -rooms:1 | candidates:1 | view:output - -a create | x round | t terminal | d/o/m/c diff/output/details/compare | e select | g apply | R recover | k close +▊▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▎│ +rooms:1 | candidates:1 | view:output + +a create | x round | t terminal | d/o/m/c diff/output/details/compare | e select | g apply | j verdict | p promote | R +recover | k close 1-7 tabs | r refresh | ? help | q quit diff --git a/docs/assets/tui/11-rooms-details.svg b/docs/assets/tui/11-rooms-details.svg index 5627db9..3faaaa9 100644 --- a/docs/assets/tui/11-rooms-details.svg +++ b/docs/assets/tui/11-rooms-details.svg @@ -173,7 +173,7 @@ - + ⭘syncmill tui BoardRunsReportsTeamsStatusSessionsRooms @@ -187,10 +187,10 @@ ▊▎│ ▊▎│Recent events ▊▎│round 1 completed -▊▎│candidate selected -▊▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▎│Candidate: -┌──────────────────────────────────────────────────────────────────────┐│a1b2c3d4-5e6f-4a7b-8c9d-0e1f2a3b4c5d -▊a1b2c3d4 codex completed ✓▎│[Details] +▊▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▎│candidate selected +┌──────────────────────────────────────────────────────────────────────┐│Candidate: +▊a1b2c3d4 codex completed ✓▎│a1b2c3d4-5e6f-4a7b-8c9d-0e1f2a3b4c5d +▊▎│[Details] ▊▎│evidence as of 12:00:00 ▊▎│Member: codex (codex) ▊▎│Round: 1 | attempt: 1 @@ -200,20 +200,20 @@ ▊▎│Gate settings: commands, timeout 300s, ▊▎│environment inherited (no allowlist): ▊▎│pytest -q -▊▎│Conversations: each member has its own; +▊▎│Conversations: each member has its own;▁▁ ▊▎│sessions: cold 3; memory context: none -▊▎│found; approved lessons: not recorded (an▆▆ +▊▎│found; approved lessons: not recorded (an ▊▎│older round); checkpoint excerpts are ▊▎│capped at 4,000 bytes each ▊▎│Conversation: cold session; no earlier ▊▎│candidates; no memory context found ▊▎│Checkpoint candidates: none ▊▎│State: completed | eligible: yes -▊▎│Exit: 0 | duration: 18.2s -▊▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▎│Gate: passed -rooms:1 | candidates:1 | view:details - -a create | x round | t terminal | d/o/m/c diff/output/details/compare | e select | g apply | R recover | k close +▊▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▎│Exit: 0 | duration: 18.2s +rooms:1 | candidates:1 | view:details + +a create | x round | t terminal | d/o/m/c diff/output/details/compare | e select | g apply | j verdict | p promote | R +recover | k close 1-7 tabs | r refresh | ? help | q quit diff --git a/docs/assets/tui/12-rooms-compare.svg b/docs/assets/tui/12-rooms-compare.svg index 8791979..5fe9302 100644 --- a/docs/assets/tui/12-rooms-compare.svg +++ b/docs/assets/tui/12-rooms-compare.svg @@ -173,7 +173,7 @@ - + ⭘syncmill tui BoardRunsReportsTeamsStatusSessionsRooms @@ -187,10 +187,10 @@ ▊▎│ ▊▎│Recent events ▊▎│round 1 completed -▊▎│candidate selected -▊▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▎│[Compare]  room room-000 -┌──────────────────────────────────────────────────────────────────────┐│(awaiting_decision) -▊a1b2c3d4 codex completed ✓▎│Round 1 of 1 (complete) +▊▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▎│candidate selected +┌──────────────────────────────────────────────────────────────────────┐│[Compare]  room room-000 +▊a1b2c3d4 codex completed ✓▎│(awaiting_decision) +▊▎│Round 1 of 1 (complete) ▊▎│starting tree: snapshot (requested lineage: ▊▎│continue-own) ▊▎│gate settings: commands, timeout 300s, @@ -200,8 +200,8 @@ ▊▎│conversations: each member has its own; ▊▎│sessions: cold 3; memory context: none ▊▎│found; approved lessons: not recorded (an -▊▎│older round); checkpoint excerpts are -▊▎│capped at 4,000 bytes each▂▂ +▊▎│older round); checkpoint excerpts are▅▅ +▊▎│capped at 4,000 bytes each ▊▎│source: unchanged since the room started ▊▎│Selected: none ▊▎│evidence as of 12:00:00 @@ -209,11 +209,11 @@ ▊▎│> a1b2c3d4 codex completed ▊▎│  conv: cold, full, 0 cand, mem none ▊▎│  gate passed · 18.2s · in 40.1k out 2.3k -▊▎│  146 B · 1 files · eligible · blockers 0 -▊▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▎│  e5f6a7b8 claude completed -rooms:1 | candidates:2 | view:compare - -a create | x round | t terminal | d/o/m/c diff/output/details/compare | e select | g apply | R recover | k close +▊▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▎│  146 B · 1 files · eligible · blockers 0 +rooms:1 | candidates:2 | view:compare + +a create | x round | t terminal | d/o/m/c diff/output/details/compare | e select | g apply | j verdict | p promote | R +recover | k close 1-7 tabs | r refresh | ? help | q quit diff --git a/docs/user-guide.ko.md b/docs/user-guide.ko.md index 0f22cd0..50cb6e0 100644 --- a/docs/user-guide.ko.md +++ b/docs/user-guide.ko.md @@ -888,8 +888,11 @@ Room 라운드에도 같은 프로젝트 전체 교훈이 들어갑니다. `room 내용이 그대로 남습니다. 교훈을 빼면 다시 보내지 않을 뿐, 이미 보낸 것을 거둘 수는 없습니다. 이전 라운드에 교훈이 있던 방에서 대화를 이어 가는 라운드라면 목록에 `--fresh-session`을 안내하는 문장이 붙습니다. 이 안내는 목록을 보여 준 시점의 방 상태를 기준으로 하므로, -detached worker가 시작하기 전에 끝난 라운드는 반영하지 않습니다. TUI에서 시작한 라운드는 아직 교훈을 보내지 않고 같은 안내를 -보여 줍니다. `room compare`는 라운드가 고른 교훈과 후보마다 교훈이 들어갔는지를 보여 줍니다. +detached worker가 시작하기 전에 끝난 라운드는 반영하지 않습니다. TUI에서 시작한 라운드도 +교훈을 보냅니다. 라운드 대화상자가 교훈마다 체크박스를 보여 주고(체크 해제 = `--skip-lesson`), +예산 때문에 빠진 교훈을 표시하며, 프롬프트가 체크한 교훈을 담을 자리를 남기지 않으면 교훈을 +빼고 진행하는 대신 시작을 거부합니다. 이전 교훈 안내는 고른 lineage에 맞춰 대화상자 안에 +나옵니다. `room compare`는 라운드가 고른 교훈과 후보마다 교훈이 들어갔는지를 보여 줍니다. `room apply --verify`는 다른 `--verify`와 마찬가지로 라운드에 기록된 설정이 아니라 현재 `[gate]`로 검증합니다. 기록을 다시 쓰면 지금 설정한 것보다 넓은 환경이 verify에 넘어갈 수 @@ -1065,7 +1068,13 @@ Rooms의 `a`는 prompt와 member만 기록하며 provider를 호출하지 않습 잘림 상태). 완료된 후보는 Diff로, 실패한 후보는 Output으로 열려요. `R`은 `room recover`로 **프로젝트 전체**의 죽은 room 워커/중단된 apply를 복구하고, `k`는 선택한 room을 -닫습니다(후보와 영수증은 보존). +닫습니다(후보와 영수증은 보존). room 화면에는 결과(outcome)가 나옵니다. `j`는 적용된 room에 +대한 판정을 기록합니다(`room judge`: accepted 또는 rejected를 고르고 이유를 덧붙일 수 있으며, +판정은 대화상자에 보인 patch digest에 묶입니다). `p`는 판정된 room에서 교훈을 승격합니다 +(`room promote`, board 필요). 대화상자는 교훈에 기록될 출처를 보여 주고, 열려 있는 동안 판정이 +바뀌었으면 거부합니다. `x`의 라운드 대화상자는 프로젝트의 승인된 교훈을 보여 줍니다(Rooms 참고). +이 대화상자들은 그사이 선택이 바뀌어도 처음 연 room에만 적용됩니다. 어디서든 판정이 기록되면 +열려 있는 Compare/Details 스냅샷에 바뀌었다는 표시가 붙습니다. 실행은 Textual이 아니라 분리된 백그라운드 매니저가 맡아요. `q`는 화면만 닫고 실행은 계속합니다. 다시 `syncmill tui`를 열면 실행 상태와 로컬 rolling 로그에 재연결돼요. diff --git a/docs/user-guide.md b/docs/user-guide.md index 28f3691..808ded7 100644 --- a/docs/user-guide.md +++ b/docs/user-guide.md @@ -964,8 +964,11 @@ conversation keeps whatever earlier rounds sent it: leaving a lesson out stops i being sent again, it cannot take it back. When a round would resume a conversation in a room whose earlier rounds had lessons, the list adds a note pointing to `--fresh-session`. The note reflects the room as of the listing: a round that -finishes before a detached worker starts is not taken into account. Rounds started from the TUI do not send lessons yet and show -the same note. `room compare` shows which lessons a round selected and, per +finishes before a detached worker starts is not taken into account. Rounds started from +the TUI send lessons too: the round dialog lists them with a checkbox each (unchecking is +`--skip-lesson`), names any left out by the budget, and refuses to start, instead of +dropping them, when the prompt leaves no room for the checked ones; the history note +appears in the dialog for the lineage you pick. `room compare` shows which lessons a round selected and, per candidate, whether they were included. `room apply --verify` runs the current `[gate]`, like every `--verify`, not the @@ -1175,7 +1178,15 @@ code, per-check gate results, files, digests, and truncation state). A completed candidate opens on Diff; a failed one opens on Output. `R` recovers dead room workers/interrupted applies for the whole project via `room recover`, and `k` closes the -selected room (candidates and receipts are preserved). +selected room (candidates and receipts are preserved). The room pane shows its +outcome. `j` records your verdict on an applied room (`room judge`: you pick +accepted or rejected, add an optional rationale, and the verdict is bound to the +patch digest the dialog showed). `p` promotes a lesson from a judged room +(`room promote`, needs the board); the dialog shows the source the lesson will +record and refuses if the verdict changed while it was open. `x` lists the +project's approved lessons in the round dialog (see Rooms). These dialogs act on +the room they were opened for, even if the selection moves meanwhile. A verdict +recorded anywhere marks an open Compare or Details snapshot as changed. Runs are owned by a detached manager, not Textual. Only one TUI-managed worker per board is allowed, but ordinary external `board run` processes remain legal. Pressing diff --git a/src/syncmill/rooms.py b/src/syncmill/rooms.py index 9f1f533..65c520b 100644 --- a/src/syncmill/rooms.py +++ b/src/syncmill/rooms.py @@ -2959,6 +2959,15 @@ def list_judgments(self, room_id: str) -> list[dict[str, Any]]: ).fetchall() return [dict(row) for row in rows] + def judgment_count(self, room_id: str) -> int: + """How many judgment rows the room holds: a cheap freshness signal for + a view built from an earlier read (a verdict changes neither the + room's revision nor its candidates).""" + row = self._conn.execute( + "SELECT COUNT(*) FROM room_judgments WHERE room_id=?", (room_id,) + ).fetchone() + return int(row[0]) + def outcome(self, room_id: str) -> OutcomeCheck: """The room's human outcome, read in one snapshot.""" with self._read_snapshot() as conn: @@ -4242,13 +4251,18 @@ def promote_lesson( *, scope: str = "", supersedes: str | None = None, + expected_judgment_id: str | None = None, ) -> tuple[Any, bool]: """Promote a short, human-written lesson from a judged room into the board's decisions, bound to this project (#174). Only a room whose latest judgment binds (accepted or rejected) can be promoted: the lesson's provenance names that judgment, and a retry of - the same promotion returns the decision it already created.""" + the same promotion returns the decision it already created. + ``expected_judgment_id`` refuses the promotion unless that judgment is + still the latest, checked on the same read the provenance comes from, + so a verdict recorded after the person reviewed it is never promoted + in its name.""" text = normalize_lesson(lesson) if not isinstance(scope, str): raise RoomError("lesson scope must be text") @@ -4263,6 +4277,14 @@ def promote_lesson( f"room outcome is {outcome.status}{detail}; record a verdict with " "`syncmill room judge` before promoting a lesson" ) + if ( + expected_judgment_id is not None + and judgment["judgment_id"] != expected_judgment_id + ): + raise RoomError( + "the room's verdict changed since you reviewed it; review the " + "promotion again" + ) provenance = { "kind": "room-judgment", "room_id": room_id, diff --git a/src/syncmill/tui.py b/src/syncmill/tui.py index b8260a9..c7170ae 100644 --- a/src/syncmill/tui.py +++ b/src/syncmill/tui.py @@ -75,9 +75,18 @@ build_room_comparison, find_round, find_row, + outcome_lines, render_round_table, ) -from syncmill.rooms import RoomCoordinator, RoomError, RoomStore, launch_room_worker +from syncmill.rooms import ( + JUDGMENT_VERDICTS, + RoomCoordinator, + RoomError, + RoomStore, + RoundLesson, + launch_room_worker, + round_lessons_fit, +) from syncmill.session import SessionLaunchError from syncmill.review_candidates import ( DEFAULT_GOAL as REVIEW_DEFAULT_GOAL, @@ -119,8 +128,11 @@ from syncmill.tui_collaboration import ( CandidateSelectModal, CollaborationConfirmModal, + LessonPromoteModal, RoomCreateModal, + RoomJudgeModal, RoomRoundModal, + RoundLessonsView, SessionOpenModal, _patch_is_binary, render_candidate_details, @@ -128,6 +140,7 @@ render_candidate_output, render_room, render_session, + round_prompt, ) _CLI_SHIM = "from syncmill.cli import cli\ncli(prog_name='syncmill')" @@ -368,7 +381,7 @@ def render_artifact(manifest: dict, result: dict | None) -> str: "sessions-tab": "a open | x attach | c checkpoint | k close", "rooms-tab": ( "a create | x round | t terminal | d/o/m/c diff/output/details/compare | " - "e select | g apply | R recover | k close" + "e select | g apply | j verdict | p promote | R recover | k close" ), } @@ -662,15 +675,22 @@ def on_button_pressed(self, event: Button.Pressed) -> None: def build_room_evidence( store_root: Path, output_max_bytes: int, repo_root: Path, room_id: str, now: float -) -> tuple[RoomComparison, int, frozenset]: +) -> tuple[RoomComparison, int, frozenset, int | None]: """Build one room's evidence snapshot. Runs in a thread worker: SQLite connections are thread-affine, so it opens its own store and closes it. - Returns the comparison plus the room revision and candidate ids read - BEFORE the build, so anything that changes during it counts as a change.""" + Returns the comparison plus the room revision, candidate ids and judgment + count read BEFORE the build, so anything that changes during it counts as + a change. A verdict changes neither revision nor candidates, hence the + count; it is None when it could not be read (freshness then rests on the + other two).""" store = RoomStore(store_root, output_max_bytes) try: room = store.get_room(room_id) candidate_ids = frozenset(c["id"] for c in store.list_candidates(room_id)) + try: + judgments: int | None = store.judgment_count(room_id) + except sqlite3.Error: + judgments = None comparison = build_room_comparison( store, room_id, @@ -680,7 +700,7 @@ def build_room_evidence( ) finally: store.close() - return comparison, int(room["revision"]), candidate_ids + return comparison, int(room["revision"]), candidate_ids, judgments def select_room_candidate( @@ -738,6 +758,8 @@ class SyncmillTuiApp(App[None]): ("c", "checkpoint", "Checkpoint"), ("K", "force_stop", "Force stop"), ("R", "recover_runs", "Recover"), + ("j", "judge", "Verdict"), + ("p", "promote", "Promote"), ("?", "help", "Help"), ("q", "quit", "Quit UI"), ] @@ -761,6 +783,11 @@ class SyncmillTuiApp(App[None]): .form-buttons { height: auto; align-horizontal: right; } #selection-body { height: 1fr; } #selection-rationale { height: 6; } + .lesson-text { width: 1fr; padding: 0 0 0 4; } + /* Editors inside a scrolling dialog body get a fixed height: TextArea's + default 1fr collapses to its border there, hiding what is typed. */ + #round-prompt { height: 8; } + #judge-rationale, #promote-lesson { height: 6; } .form-buttons Button { margin-left: 2; } _FormModal { align: center middle; } .warning { color: $warning; margin: 1 0; } @@ -1236,7 +1263,7 @@ def action_quit(self) -> None: def action_help(self) -> None: self.notify( - "1-7 tabs; Sessions a/x/c/k; Rooms a/x/t/d/o/m/c/e/g/R/k; " + "1-7 tabs; Sessions a/x/c/k; Rooms a/x/t/d/o/m/c/e/g/j/p/R/k; " "Board a/i/d/e/u/h/s/x/g; Runs v/f/k/K/R (Artifacts: d/g/k); " "q detaches" ) @@ -1332,7 +1359,12 @@ def _refresh_board(self) -> None: try: items = self._board.list(state=self._filter) stats = self._board.stats() + decisions = len(self._board.active_decisions()) + selected_exists = bool( + self._selected_item_id and self._board.get(self._selected_item_id) + ) except BoardError as exc: + # Every read is in here: one escaping a poll tick kills the app. self._board_error = str(exc) self._update_static("#board-status", f"DEGRADED: {exc}") return @@ -1371,12 +1403,12 @@ def _refresh_board(self) -> None: ) table.move_cursor(row=index) table.scroll_to(y=min(scroll_y, max(0, len(rows) - 1)), animate=False) - if self._selected_item_id and self._board.get(self._selected_item_id): + if selected_exists and self._selected_item_id: self._show_item(self._selected_item_id) self._update_static( "#board-status", f"items:{stats.total} | filter:{self._filter or 'all'} | " - f"decisions:{len(self._board.active_decisions())}", + f"decisions:{decisions}", ) def on_data_table_row_highlighted(self, event: DataTable.RowHighlighted) -> None: @@ -2439,7 +2471,12 @@ def _stage_room(self, room_id: str, *, reset_candidate: bool = False) -> tuple: room = self._rooms.store.get_room(room_id) candidates = self._rooms.store.list_candidates(room_id) events = self._rooms.store.events(room_id) - detail = render_room(room, candidates, events) + try: + outcome = outcome_lines(self._rooms.store.outcome(room_id)) + except _ROOM_POLL_ERRORS as exc: + # Local: a damaged judgment table must not hide the room. + outcome = [f"Outcome: unavailable ({printable_name(str(exc))})"] + detail = render_room(room, candidates, events, outcome) ids = [candidate["id"] for candidate in candidates] entries = [ ( @@ -2674,7 +2711,7 @@ def _apply_evidence( generation: int, room_id: str, root: str, - result: tuple[RoomComparison, int, frozenset] | None, + result: tuple[RoomComparison, int, frozenset, int | None] | None, error: str | None, built_at: float, ) -> None: @@ -2692,13 +2729,14 @@ def _apply_evidence( and (room_id, root) == current ): if result is not None: - comparison, revision, candidate_ids = result + comparison, revision, candidate_ids, judgments = result self._evidence = { "root": root, "room_id": room_id, "comparison": comparison, "revision": revision, "candidate_ids": candidate_ids, + "judgment_count": judgments, "as_of": time.strftime("%H:%M:%S", time.localtime(built_at)), } self._evidence_error = None @@ -2745,6 +2783,7 @@ def _stage_evidence_view( ] target_round = max(numbers) if numbers else None changed = False + freshness_error = None if evidence is not None and room_id: room = self._rooms.store.get_room(room_id) current_ids = frozenset( @@ -2754,6 +2793,21 @@ def _stage_evidence_view( int(room["revision"]) != evidence["revision"] or current_ids != evidence["candidate_ids"] ) + if not changed and evidence.get("judgment_count") is None: + # The build could not count verdicts, so a later one cannot be + # detected against it: say so instead of implying "unchanged". + freshness_error = "this snapshot could not count verdicts" + elif not changed: + # A verdict (from any surface) changes neither of the above. + # Degraded locally: an unreadable judgments table must not take + # the whole staged room update down with it. + try: + changed = ( + self._rooms.store.judgment_count(room_id) + != evidence["judgment_count"] + ) + except _ROOM_POLL_ERRORS as exc: + freshness_error = str(exc) or type(exc).__name__ key = ( "evidence", room_id, @@ -2762,6 +2816,7 @@ def _stage_evidence_view( self._evidence_build_id, error, changed, + freshness_error, building, target_round, stored_rounds, @@ -2779,6 +2834,11 @@ def _stage_evidence_view( return key, "\n".join(lines), view, pages, False if changed: lines.append("room changed since this snapshot — press r") + if freshness_error is not None: + lines.append( + f"cannot tell whether a verdict changed since this snapshot " + f"({printable_name(freshness_error)}) — press r" + ) comparison: RoomComparison = evidence["comparison"] as_of = evidence["as_of"] found = find_row(comparison, candidate_id) if candidate_id else None @@ -2910,39 +2970,109 @@ def _apply_room_create(self, fields: dict | bool | None) -> None: self._selected_room_id = room["id"] self._refresh_rooms() - def _confirm_room_round(self) -> None: + def _room_target(self) -> tuple[str, str] | None: + """The selected room and the store it lives in, captured when a dialog + opens: its callback acts on these, never on whatever is selected when + the person confirms (a poll can move the selection meanwhile).""" if self._rooms is None or not self._selected_room_id: self.notify("select a room", severity="warning") + return None + return self._selected_room_id, str(self._rooms.store.root) + + def _room_store_is(self, store_root: str) -> bool: + """Whether the Rooms store shown now is the one a dialog was opened on; + warns when it is not (the TUI rebound to another project).""" + if self._rooms is not None and str(self._rooms.store.root) == store_root: + return True + self.notify( + "the Rooms store changed while the dialog was open; nothing was done", + severity="warning", + ) + return False + + def _round_lessons(self, room_id: str, room: dict) -> RoundLessonsView: + """The approved lessons a TUI round can send (#174), read once on the + Textual thread when the dialog opens (the board connection is + thread-affine). Advisory like the CLI's: an unreadable board means the + round runs without lessons, said in the dialog.""" + history: dict[str, bool] = {} + for lineage in ("continue-own", "branch-from"): + # Best-effort: a lineage that cannot be checked gets no note. + with contextlib.suppress(*_ROOM_POLL_ERRORS, OSError): + history[lineage] = self._rooms.resumes_with_lesson_history( + room_id, lineage=lineage + ) + creation_prompt = str(room["prompt"]) + + def fits(prompt: str | None, texts: list[str]) -> bool: + return round_lessons_fit( + prompt if prompt is not None else creation_prompt, texts + ) + + if not self._config.board.enabled: + return RoundLessonsView(board_off=True, history=history, fits=fits) + if self._board is None: + return RoundLessonsView( + unavailable=self._board_error or "the board did not open", + history=history, + fits=fits, + ) + try: + lessons = tuple(self._board.project_lessons(self._rooms.project_id)) + except (BoardError, sqlite3.Error) as exc: + return RoundLessonsView(unavailable=str(exc), history=history, fits=fits) + return RoundLessonsView(lessons=lessons, history=history, fits=fits) + + def _confirm_room_round(self) -> None: + target = self._room_target() + if target is None: return + room_id, store_root = target try: - room = self._rooms.store.get_room(self._selected_room_id) + room = self._rooms.store.get_room(room_id) # Prefill with the PREVIOUS round's prompt (creation prompt on a # fresh room) — the modal is where per-round instructions land now. - rounds = self._rooms.store.list_rounds(self._selected_room_id) + rounds = self._rooms.store.list_rounds(room_id) default_prompt = rounds[-1]["prompt"] if rounds else room["prompt"] eligible = [ row - for row in self._rooms.store.list_candidates(self._selected_room_id) + for row in self._rooms.store.list_candidates(room_id) if row.get("eligible") ] - except _ROOM_POLL_ERRORS as exc: + lessons = self._round_lessons(room_id, room) + except (*_ROOM_POLL_ERRORS, OSError) as exc: self.notify(str(exc), severity="error") return self.push_screen( - RoomRoundModal(room, default_prompt, eligible), - self._apply_room_round, + RoomRoundModal(room, default_prompt, eligible, lessons), + lambda fields, room_id=room_id, store_root=store_root, shown=lessons.lessons: ( + self._apply_room_round(fields, room_id, store_root, shown) + ), ) - def _apply_room_round(self, fields: dict | bool | None) -> None: - if not isinstance(fields, dict) or self._rooms is None or not self._selected_room_id: + def _apply_room_round( + self, + fields: dict | bool | None, + room_id: str, + store_root: str, + shown: tuple = (), + ) -> None: + if not isinstance(fields, dict) or not self._room_store_is(store_root): return - # Preserve the prompt VERBATIM (the raw-user-text contract the stored - # rounds.prompt keeps, and what --prompt-file does): strip only to - # decide whether an all-whitespace entry means "no prompt". - raw_prompt = str(fields.get("prompt") or "") - prompt = raw_prompt if raw_prompt.strip() else None + assert self._rooms is not None + # The same rule the dialog's fit check used (round_prompt): the text + # verbatim, or None to reuse the room creation prompt. + prompt = round_prompt(fields.get("prompt")) lineage = str(fields.get("lineage") or "continue-own") base_candidate = fields.get("base_candidate") + # Exactly the lessons the dialog showed as going in, from the snapshot + # it was built from — never a second board read. + by_id = {lesson.id: lesson for lesson in shown} + chosen = [str(lesson_id) for lesson_id in fields.get("lessons") or ()] + if any(lesson_id not in by_id for lesson_id in chosen): + self.notify("a chosen lesson was not in the dialog; nothing was run", severity="error") + return + lessons = tuple(RoundLesson(lesson_id, by_id[lesson_id].text) for lesson_id in chosen) if prompt is None: self.notify( "no round prompt — reusing the room creation prompt", @@ -2950,49 +3080,221 @@ def _apply_room_round(self, fields: dict | bool | None) -> None: ) try: self._rooms.preflight_round( - self._selected_room_id, + room_id, prompt, lineage=lineage, base_candidate_id=base_candidate, ) pid = launch_room_worker( self._project_root(), - self._selected_room_id, + room_id, prompt, lineage=lineage, base_candidate_id=base_candidate, resolved_config=self._rooms.config, config_base=self._toml_path.parent, + lessons=lessons, ) - self._rooms.store.record_event( - self._selected_room_id, "worker.launched", {"pid": pid} - ) + self._rooms.store.record_event(room_id, "worker.launched", {"pid": pid}) # ProjectError covers a repository that disappeared under us: git_root # reports an unusable path that way rather than letting subprocess's # OSError through, and an unguarded raise here kills the whole app. - except (RoomError, ArtifactError, WorkspaceError, ProjectError, OSError) as exc: + except ( + *_ROOM_POLL_ERRORS, ArtifactError, WorkspaceError, ProjectError, OSError + ) as exc: + # The room store's error boundary too: a row that turned unreadable + # after the dialog opened must not escape the callback. + self.notify(str(exc) or type(exc).__name__, severity="error") + return + sent = f", {len(lessons)} approved lesson(s)" if lessons else "" + self.notify(f"room worker started (pid {pid}{sent})") + self._refresh_rooms() + + def action_judge(self) -> None: + if self._active() == "rooms-tab": + self._confirm_room_judge() + + def action_promote(self) -> None: + if self._active() == "rooms-tab": + self._confirm_lesson_promote() + + def _confirm_room_judge(self) -> None: + target = self._room_target() + if target is None: + return + room_id, store_root = target + try: + outcome = self._rooms.store.outcome(room_id) + if outcome.status not in ("applied_unreviewed", *JUDGMENT_VERDICTS): + reason = f" ({outcome.reason})" if outcome.reason else "" + self.notify( + f"room outcome is {outcome.status}{reason}; only an applied " + "change can be accepted or rejected", + severity="warning", + ) + return + row = self._rooms.store.get_candidate(str(outcome.candidate_id)) + except (*_ROOM_POLL_ERRORS, OSError) as exc: + self.notify(str(exc), severity="error") + return + p = printable_name + detail = "\n".join( + [ + f"Room: {p(room_id)}", + f"Applied candidate: {p(str(outcome.candidate_id))}", + f"Patch sha256: {p(str(row['patch_sha256']))}", + "", + # The current verdict as compare shows it (who, when, why), so + # the person sees what a new verdict would supersede. + *outcome_lines(outcome), + "", + "The verdict is bound to this patch digest; a new one supersedes " + "the current one and both stay in the history. Nothing is shared " + "to memory.", + ] + ) + self.push_screen( + RoomJudgeModal(detail), + lambda fields, room_id=room_id, store_root=store_root, digest=row[ + "patch_sha256" + ]: self._apply_room_judge(fields, room_id, store_root, digest), + ) + + def _apply_room_judge( + self, fields: dict | bool | None, room_id: str, store_root: str, digest: str + ) -> None: + if not isinstance(fields, dict) or not self._room_store_is(store_root): + return + assert self._rooms is not None + try: + # The rationale goes as typed: the store decides what an empty or + # padded one means, exactly as for `room judge --rationale`. + record, created = self._rooms.judge( + room_id, + str(fields["verdict"]), + expected_patch_sha256=digest, + rationale=fields.get("rationale"), + surface="tui", + ) + except (*_ROOM_POLL_ERRORS, OSError) as exc: self.notify(str(exc), severity="error") return - self.notify(f"room worker started (pid {pid})") - # The TUI does not show or inject approved lessons yet (#174), but a - # resumed conversation may still hold ones earlier rounds sent. Read - # after the launch and best-effort: the worker is already running, so - # a failed read must not look like a failed launch. - lesson_history = False - with contextlib.suppress(*_ROOM_POLL_ERRORS): - lesson_history = self._rooms.resumes_with_lesson_history( - self._selected_room_id, lineage=lineage - ) - if lesson_history: + if created: self.notify( - "approved lessons are not sent from the TUI; resumed conversations " - "may still hold lessons sent in earlier rounds. The fresh lineage, " - "or `syncmill room round --fresh-session`, starts them without " - "that history", - severity="warning", + f"recorded {record['verdict']} for candidate " + f"{printable_name(str(record['candidate_id'])[:8])}" + ) + else: + self.notify( + f"already recorded: {record['verdict']} (judgment " + f"{record['sequence']}); nothing changed" ) self._refresh_rooms() + def _confirm_lesson_promote(self) -> None: + target = self._room_target() + if target is None: + return + room_id, store_root = target + if self._board is None: + self.notify( + ( + f"promoting a lesson needs the board, which did not open: " + f"{self._board_error or 'unknown error'}" + ) + if self._config.board.enabled + else "promoting a lesson needs the board, where lessons are kept " + "as decisions: set SYNCMILL_BOARD__ENABLED=true", + severity="error", + ) + return + try: + project = self._rooms.project_id + outcome = self._rooms.store.outcome(room_id) + judgment = outcome.judgment + if outcome.status not in JUDGMENT_VERDICTS or judgment is None: + reason = f" ({outcome.reason})" if outcome.reason else "" + self.notify( + f"room outcome is {outcome.status}{reason}; record a verdict " + "with j before promoting a lesson", + severity="warning", + ) + return + # active_decisions(project_id=) also returns unbound decisions, + # which the board refuses to let a project lesson replace. + supersedable = [ + decision + for decision in self._board.active_decisions(project_id=project) + if decision.project_id == project + ] + except (*_ROOM_POLL_ERRORS, BoardError, OSError) as exc: + self.notify(str(exc), severity="error") + return + p = printable_name + detail = ( + "The lesson is recorded with where it came from:\n" + f"Room: {p(room_id)}\n" + f"Candidate: {p(str(judgment['candidate_id']))}\n" + f"Verdict: {p(str(judgment['verdict']))} by a human (judgment " + f"{p(str(judgment['judgment_id']))})\n" + f"Patch sha256: {p(str(judgment['patch_sha256']))}\n" + f"Applied as run: {p(str(judgment['applied_run_id']))}\n" + f"Project: {p(project)}\n\n" + "Runs of this project show the lessons they use before starting, and " + "each can be left out per run." + ) + choices = [ + ( + f"{d.id[:8]} [{d.scope or 'whole project'}] {d.text}", + d.id, + d.scope, + ) + for d in supersedable + ] + self.push_screen( + LessonPromoteModal(detail, choices), + lambda fields, room_id=room_id, store_root=store_root, judgment_id=judgment[ + "judgment_id" + ]: self._apply_lesson_promote(fields, room_id, store_root, judgment_id), + ) + + def _apply_lesson_promote( + self, fields: dict | bool | None, room_id: str, store_root: str, judgment_id: str + ) -> None: + if not isinstance(fields, dict) or not self._room_store_is(store_root): + return + assert self._rooms is not None + if self._board is None: + self.notify("the board is no longer open; nothing was promoted", severity="error") + return + try: + # Text and scope go as typed, as for `room promote`; the + # coordinator and the board validate both. + decision, created = self._rooms.promote_lesson( + room_id, + self._board, + str(fields.get("lesson") or ""), + scope=str(fields.get("scope") or ""), + supersedes=fields.get("supersedes"), + expected_judgment_id=judgment_id, + ) + except (*_ROOM_POLL_ERRORS, BoardError, OSError) as exc: + self.notify(str(exc), severity="error") + return + where = ( + f"goal {printable_name(decision.scope)}" if decision.scope else "the whole project" + ) + if created: + self.notify(f"promoted lesson {decision.id[:8]} for {where}") + else: + state = ( + f"superseded by {decision.superseded_by[:8]}" + if decision.superseded_by + else "active" + ) + self.notify(f"already promoted as lesson {decision.id[:8]} ({state}); nothing changed") + self._refresh_board() + def _confirm_candidate_selection(self) -> None: if ( self._rooms is None diff --git a/src/syncmill/tui_collaboration.py b/src/syncmill/tui_collaboration.py index 88f5b85..2228621 100644 --- a/src/syncmill/tui_collaboration.py +++ b/src/syncmill/tui_collaboration.py @@ -2,7 +2,9 @@ from __future__ import annotations +import dataclasses import re +from collections.abc import Callable from textual.app import ComposeResult from textual.containers import Horizontal, Vertical, VerticalScroll @@ -10,6 +12,7 @@ from textual.widgets import Button, Checkbox, Input, Select, TextArea from syncmill.artifacts import printable_name, printable_text +from syncmill.board import Decision, lesson_source, select_lessons from syncmill.config import SyncmillConfig from syncmill.room_compare import ( CandidateRow, @@ -26,7 +29,7 @@ usage_label, ) from syncmill.session import interactive_agents -from syncmill.tui_render import PlainLabel, PlainStatic, plain_select +from syncmill.tui_render import PlainLabel, PlainStatic, plain, plain_select class _CollabModal(ModalScreen[dict | bool | None]): @@ -108,6 +111,45 @@ def on_button_pressed(self, event: Button.Pressed) -> None: ) +def round_prompt(raw: object) -> str | None: + """The round prompt a submitted TextArea value means: the text VERBATIM + (the raw-user-text contract the stored rounds.prompt keeps), or None — + reuse the room creation prompt — when it is empty or all whitespace. The + lesson fit check and the launch both use this, so they cannot disagree.""" + text = str(raw or "") + return text if text.strip() else None + + +@dataclasses.dataclass(frozen=True) +class RoundLessonsView: + """What the round dialog shows about approved lessons (#174), read once + when the dialog opens. + + ``board_off`` says the board is disabled, so no lessons can be sent. + ``lessons`` are the project's active project-wide lessons, oldest first; + the dialog re-runs the CLI's ``select_lessons`` on every change, so a + skipped lesson frees budget exactly as ``--skip-lesson`` does. + ``unavailable`` is why lessons could not be read (the round runs without + them). ``history`` maps a lineage to whether a round launched with it now + would resume a conversation that earlier rounds sent lessons to (a lineage + missing from it could not be checked). ``fits(prompt, texts)`` is the + coordinator's own fit check, with a None prompt meaning the creation + prompt.""" + + lessons: tuple[Decision, ...] = () + board_off: bool = False + unavailable: str | None = None + history: dict[str, bool] = dataclasses.field(default_factory=dict) + fits: Callable[[str | None, list[str]], bool] = lambda prompt, texts: True + + +_HISTORY_NOTE = ( + "Resumed conversations may still hold lessons sent in earlier rounds: " + "unchecking a lesson stops sending it, it does not take back what a " + "conversation was already sent. The fresh lineage starts without that history." +) + + class RoomRoundModal(_CollabModal): """Per-round prompt + lineage input for the Rooms tab. @@ -124,11 +166,18 @@ class RoomRoundModal(_CollabModal): ("branch-from (seed all from one candidate)", "branch-from"), ] - def __init__(self, room: dict, default_prompt: str, eligible: list[dict]): + def __init__( + self, + room: dict, + default_prompt: str, + eligible: list[dict], + lessons: RoundLessonsView | None = None, + ): super().__init__() self._room = room self._default_prompt = default_prompt self._eligible = eligible + self._lessons = lessons if lessons is not None else RoundLessonsView() def compose(self) -> ComposeResult: members = self._room.get("members", []) @@ -137,38 +186,133 @@ def compose(self) -> ComposeResult: f"Run round {int(self._room.get('round_count') or 0) + 1} — " f"{len(members)} provider call(s)" ) - area = TextArea(id="round-prompt", language=None) - # Escaped prefill: new prompts cannot contain controls (the - # coordinator rejects them), but a row stored before that - # validation could — never hand raw sequences to the terminal. - area.text = escape_controls(self._default_prompt) - yield area - yield plain_select( - self._LINEAGES, - value="continue-own", - allow_blank=False, - id="round-lineage", - ) - if self._eligible: + # Everything but the confirmation and the buttons scrolls, so they + # stay reachable at 80x24 with a full lesson list. + with VerticalScroll(id="round-body"): + area = TextArea(id="round-prompt", language=None) + # Escaped prefill: new prompts cannot contain controls (the + # coordinator rejects them), but a row stored before that + # validation could — never hand raw sequences to the terminal. + area.text = escape_controls(self._default_prompt) + yield area yield plain_select( - [ - (f"{row['id'][:8]} {row['member_id']}", row["id"]) - for row in self._eligible - ], - allow_blank=True, - prompt="base candidate (branch-from only)", - id="round-base", + self._LINEAGES, + value="continue-own", + allow_blank=False, + id="round-lineage", + ) + if self._eligible: + yield plain_select( + [ + (f"{row['id'][:8]} {row['member_id']}", row["id"]) + for row in self._eligible + ], + allow_blank=True, + prompt="base candidate (branch-from only)", + id="round-base", + ) + if self._lessons.board_off: + yield PlainStatic( + "Lessons: the board is off, so this round sends no approved " + "lessons." + ) + if self._lessons.unavailable is not None: + yield PlainStatic( + f"Lessons unavailable ({printable_name(self._lessons.unavailable)}); " + "this round runs without them.", + classes="warning", + ) + if self._lessons.lessons: + yield PlainStatic( + "Approved project lessons (sent to every member's prompt " + "this round; uncheck to leave one out of this round):" + ) + for index, lesson in enumerate(self._lessons.lessons): + # The label is only the id: a Checkbox label does not + # wrap, so a long lesson would be cut off at the dialog + # edge while every word of it is sent. The full text is + # a wrapping PlainStatic below it. plain(): a str label + # is parsed as markup by Checkbox. + yield Checkbox( + plain(f"lesson {lesson.id[:8]}"), + value=True, + id=f"round-lesson-{index}", + ) + yield PlainStatic( + lesson.text, classes="lesson-text", id=f"round-lesson-text-{index}" + ) + source = lesson_source(lesson) + if source: + yield PlainStatic(source, classes="lesson-text") + yield PlainStatic( + "Replace a lesson with p on a judged room, or " + '`syncmill board decide --supersedes ID ""`.' + ) + yield PlainStatic("", id="round-lessons-status") + yield PlainStatic( + "The detached worker keeps running if the TUI closes. An empty " + "prompt reuses the room creation prompt.", + classes="warning", ) - yield PlainStatic( - "The detached worker keeps running if the TUI closes. An empty " - "prompt reuses the room creation prompt.", - classes="warning", - ) yield Checkbox("I reviewed this exact action", id="confirmed") with Horizontal(classes="form-buttons"): yield Button("Run round", variant="warning", id="apply") yield Button("Cancel", id="cancel") + def on_mount(self) -> None: + self._update_lessons_status() + + def _lesson_choice(self) -> tuple[list[Decision], list[Decision], list[Decision]]: + """``(used, skipped, over_budget)`` for the boxes as they stand: the + CLI's selection, with an unchecked box as ``--skip-lesson``.""" + skip = frozenset( + lesson.id + for index, lesson in enumerate(self._lessons.lessons) + if not self.query_one(f"#round-lesson-{index}", Checkbox).value + ) + return select_lessons(list(self._lessons.lessons), skip=skip) + + def _lineage(self) -> object: + return self.query_one("#round-lineage", Select).value + + def _status_text(self) -> tuple[str, bool]: + """The lessons status block, and whether the chosen lessons fit.""" + lines: list[str] = [] + fits = True + if self._lessons.lessons: + used, skipped, over = self._lesson_choice() + prompt = round_prompt(self.query_one("#round-prompt", TextArea).text) + fits = not used or self._lessons.fits(prompt, [d.text for d in used]) + lines.append(f"Lessons this round: {len(used)} of {len(self._lessons.lessons)}") + lines.extend(f" left out: {d.id[:8]} (unchecked)" for d in skipped) + lines.extend( + f" left out: {d.id[:8]} (over the per-run lesson budget)" for d in over + ) + if not fits: + lines.append( + " this round's prompt leaves no room for the checked lessons: " + "uncheck some or shorten the prompt" + ) + if self._lessons.history.get(str(self._lineage())): + lines.append(_HISTORY_NOTE) + return "\n".join(lines), fits + + def _update_lessons_status(self) -> None: + text, _ = self._status_text() + self.query_one("#round-lessons-status", PlainStatic).update(text) + + def on_checkbox_changed(self, event: Checkbox.Changed) -> None: + if (event.checkbox.id or "").startswith("round-lesson-"): + self._update_lessons_status() + + def on_select_changed(self, event: Select.Changed) -> None: + if event.select.id == "round-lineage": + self._update_lessons_status() + + def on_text_area_changed(self, event: TextArea.Changed) -> None: + if event.text_area.id == "round-prompt" and self._lessons.lessons: + self._update_lessons_status() + def on_button_pressed(self, event: Button.Pressed) -> None: if event.button.id != "apply": self.dismiss(None) @@ -176,7 +320,20 @@ def on_button_pressed(self, event: Button.Pressed) -> None: if not self.query_one("#confirmed", Checkbox).value: self.app.notify("confirm the action", severity="warning") return - lineage = self.query_one("#round-lineage", Select).value + used: list[Decision] = [] + if self._lessons.lessons: + used, _, _ = self._lesson_choice() + _, fits = self._status_text() + if not fits: + # Refused, not dropped: the dialog showed these lessons as going + # in, so running without them would not be what was confirmed. + self.app.notify( + "this round's prompt leaves no room for the checked lessons; " + "uncheck some or shorten the prompt", + severity="warning", + ) + return + lineage = self._lineage() base = None if self._eligible: value = self.query_one("#round-base", Select).value @@ -195,6 +352,7 @@ def on_button_pressed(self, event: Button.Pressed) -> None: "prompt": self.query_one("#round-prompt", TextArea).text, "lineage": lineage, "base_candidate": base, + "lessons": [lesson.id for lesson in used], } ) @@ -262,6 +420,121 @@ def on_button_pressed(self, event: Button.Pressed) -> None: ) +class RoomJudgeModal(_CollabModal): + """Record a human verdict on a room's applied change (#174), with an + optional rationale. The verdict has no default, so a stray keypress cannot + record one. Dumb like the other modals: the store validates the text.""" + + _VERDICTS = [("accepted", "accepted"), ("rejected", "rejected")] + + def __init__(self, detail: str): + super().__init__() + self._detail = detail + + def compose(self) -> ComposeResult: + with Vertical(classes="wide-dialog"): + yield PlainLabel("Record your verdict on the applied change") + with VerticalScroll(id="judge-body"): + yield PlainStatic(self._detail) + yield plain_select( + self._VERDICTS, + allow_blank=True, + prompt="verdict", + id="judge-verdict", + ) + yield PlainLabel("Rationale (optional) — why you accept or reject it:") + yield TextArea(id="judge-rationale", language=None) + yield Checkbox("I reviewed this exact change", id="confirmed") + with Horizontal(classes="form-buttons"): + yield Button("Record", variant="warning", id="apply") + yield Button("Cancel", id="cancel") + + def on_button_pressed(self, event: Button.Pressed) -> None: + if event.button.id != "apply": + self.dismiss(None) + return + verdict = self.query_one("#judge-verdict", Select).value + if verdict not in ("accepted", "rejected"): + self.app.notify("choose accepted or rejected", severity="warning") + return + if not self.query_one("#confirmed", Checkbox).value: + self.app.notify("confirm the action", severity="warning") + return + self.dismiss( + { + "verdict": verdict, + "rationale": self.query_one("#judge-rationale", TextArea).text, + } + ) + + +class LessonPromoteModal(_CollabModal): + """Promote a lesson the person writes from a judged room (#174). + + ``supersedable`` are ``(label, decision id, scope)`` for this project's + active lessons. A replacement must have the same scope (the board refuses + otherwise); the dialog checks that before closing so the error sits next + to the fields.""" + + def __init__(self, detail: str, supersedable: list[tuple[str, str, str]]): + super().__init__() + self._detail = detail + self._supersedable = supersedable + + def compose(self) -> ComposeResult: + with Vertical(classes="wide-dialog"): + yield PlainLabel("Promote a lesson for reuse in this project's runs") + with VerticalScroll(id="promote-body"): + yield PlainStatic(self._detail) + yield PlainLabel("Lesson (your own words, at most 1000 bytes):") + yield TextArea(id="promote-lesson", language=None) + yield PlainLabel("Scope: a board goal, or blank for the whole project") + yield Input(placeholder="blank = whole project", id="promote-scope") + yield plain_select( + [ + ("(replaces nothing)", ""), + *((label, decision_id) for label, decision_id, _ in self._supersedable), + ], + value="", + allow_blank=False, + id="promote-supersedes", + ) + yield Checkbox("I reviewed this lesson and its source", id="confirmed") + with Horizontal(classes="form-buttons"): + yield Button("Promote", variant="warning", id="apply") + yield Button("Cancel", id="cancel") + + def on_button_pressed(self, event: Button.Pressed) -> None: + if event.button.id != "apply": + self.dismiss(None) + return + if not self.query_one("#confirmed", Checkbox).value: + self.app.notify("confirm the action", severity="warning") + return + scope = self.query_one("#promote-scope", Input).value + value = self.query_one("#promote-supersedes", Select).value + supersedes = value if isinstance(value, str) and value else None + if supersedes is not None: + target_scope = next( + (scope_ for _, decision_id, scope_ in self._supersedable + if decision_id == supersedes), + None, + ) + if target_scope != scope: + self.app.notify( + "a lesson can only replace one of the same scope", + severity="warning", + ) + return + self.dismiss( + { + "lesson": self.query_one("#promote-lesson", TextArea).text, + "scope": scope, + "supersedes": supersedes, + } + ) + + def _patch_is_binary(head: bytes) -> bool: """Whether a patch head carries binary content. Derived — no stored flag: patches are captured with `git diff --binary`, whose binary hunks are @@ -295,7 +568,12 @@ def render_session(value: dict) -> str: ) -def render_room(room: dict, candidates: list[dict], events: list[dict]) -> str: +def render_room( + room: dict, + candidates: list[dict], + events: list[dict], + outcome: list[str] | tuple[str, ...] = (), +) -> str: members = ", ".join(_p(member["member_id"]) for member in room.get("members", [])) tail = events[-5:] lines = [ @@ -306,11 +584,12 @@ def render_room(room: dict, candidates: list[dict], events: list[dict]) -> str: f"Selected: {_p(room.get('selected_candidate_id') or '-')}", f"Apply run: {_p(room.get('apply_run_id') or '-')}", f"Applied run: {_p(room.get('applied_run_id') or '-')}", + *outcome, "", "Recent events:", *(f"{_p(event['sequence'])} {_p(event['kind'])}" for event in tail), "", - "x run round | e select candidate | g apply selected", + "x run round | e select candidate | g apply selected | j verdict | p promote", ] return "\n".join(lines) diff --git a/tests/test_tui.py b/tests/test_tui.py index 4ba921d..040df98 100644 --- a/tests/test_tui.py +++ b/tests/test_tui.py @@ -1197,7 +1197,7 @@ def build(store_root, output_max, repo_root, room_id, now): calls.append(room_id) comparison = _comparison(rows, room_id=room_id, **kwargs) ids = frozenset(r.candidate_id for r in rows) - return comparison, comparison.revision, ids + return comparison, comparison.revision, ids, 0 monkeypatch.setattr("syncmill.tui.build_room_evidence", build) @@ -1210,6 +1210,9 @@ class _FakeCandidateRoomStore: root = "/nonexistent-room-store" candidates = type("_Candidates", (), {"output_max_bytes": 1})() + def judgment_count(self, room_id): + return 0 + def __init__(self, patch: bytes = b"diff --git a/x b/x\n+line\n" * 20_000): self.patch = patch self.rows = { @@ -3019,11 +3022,16 @@ class _FakeRoundRooms: def __init__(self, *, rounds=None, candidates=None): self.config = object() + self.project_id = "proj-1" self.preflights: list[tuple] = [] self.events: list[tuple] = [] + self.history_asked: list[tuple] = [] + self.history: dict = {} outer = self class _Store: + root = "/fake-room-store" + @staticmethod def get_room(room_id): return { @@ -3051,10 +3059,20 @@ def preflight_round(self, room_id, prompt, *, lineage, base_candidate_id): self.preflights.append((room_id, prompt, lineage, base_candidate_id)) return {} + def resumes_with_lesson_history(self, room_id, *, lineage): + self.history_asked.append((room_id, lineage)) + value = self.history.get(lineage, False) + if isinstance(value, Exception): + raise value + return value + def close(self): return None +_ROUND_STORE = "/fake-room-store" + + def test_room_round_modal_prefills_previous_round_prompt(tmp_path, git_repo, monkeypatch): async def scenario(): app = _room_apply_app(tmp_path, git_repo) @@ -3101,7 +3119,8 @@ def fake_launch(root, room_id, prompt=None, **kwargs): app._apply_room_round( {"prompt": "edited round prompt", "lineage": "fresh", - "base_candidate": None} + "base_candidate": None}, + "room-a", _ROUND_STORE, ) await pilot.pause() assert rooms.preflights == [ @@ -3114,66 +3133,61 @@ def fake_launch(root, room_id, prompt=None, **kwargs): # An EMPTY prompt falls back (None) with a visible warning. app._apply_room_round( - {"prompt": " ", "lineage": "continue-own", "base_candidate": None} + {"prompt": " ", "lineage": "continue-own", "base_candidate": None}, + "room-a", _ROUND_STORE, ) await pilot.pause() assert launched["prompt"] is None + assert launched["lessons"] == () # A bool (the legacy confirm shape) or None is ignored outright. before = len(rooms.preflights) - app._apply_room_round(True) - app._apply_room_round(None) + app._apply_room_round(True, "room-a", _ROUND_STORE) + app._apply_room_round(None, "room-a", _ROUND_STORE) assert len(rooms.preflights) == before drive(scenario()) -@pytest.mark.parametrize("history", [True, False, "raises"]) -def test_room_round_launch_notes_lessons_a_resumed_conversation_may_hold( - tmp_path, git_repo, monkeypatch, history +def test_room_round_reads_lesson_history_per_lineage_before_confirmation( + tmp_path, git_repo, monkeypatch ): - """The TUI sends no approved lessons yet (#174), but warns when a resumed - conversation may hold ones earlier rounds sent. The read comes after the - launch, so a failed read never turns a started worker into an error.""" + """The history note (#174) is decided when the dialog opens, per lineage + that can resume (fresh never does), so the person can still pick fresh; + a lineage whose check fails gets no note. Nothing is said after launch.""" + from syncmill.rooms import RoomError async def scenario(): app = _room_apply_app(tmp_path, git_repo) async with app.run_test() as pilot: await pilot.pause() rooms = _FakeRoundRooms() - asked: list = [] - - def resumes(room_id, **kwargs): - asked.append((room_id, kwargs)) - if history == "raises": - from syncmill.rooms import RoomError - - raise RoomError("store hiccup") - return history - - rooms.resumes_with_lesson_history = resumes + rooms.history = {"continue-own": True, "branch-from": RoomError("hiccup")} app._rooms = rooms app._selected_room_id = "room-a" - launched: dict = {} + staged: dict = {} monkeypatch.setattr( - "syncmill.tui.launch_room_worker", - lambda root, room_id, prompt=None, **kwargs: launched.update(kwargs) or 4242, + app, "push_screen", + lambda screen, callback: staged.update(screen=screen, cb=callback), ) - monkeypatch.setattr(app, "_refresh_rooms", lambda: None) + app._confirm_room_round() + view = staged["screen"]._lessons + assert view.history == {"continue-own": True} + assert rooms.history_asked == [ + ("room-a", "continue-own"), ("room-a", "branch-from") + ] + notes: list = [] monkeypatch.setattr( app, "notify", lambda message, **kwargs: notes.append((message, kwargs)) ) - app._apply_room_round( - {"prompt": "go", "lineage": "continue-own", "base_candidate": None} + monkeypatch.setattr( + "syncmill.tui.launch_room_worker", lambda *a, **k: 4242 ) + monkeypatch.setattr(app, "_refresh_rooms", lambda: None) + staged["cb"]({"prompt": "go", "lineage": "continue-own", + "base_candidate": None, "lessons": []}) await pilot.pause() - assert "lessons" not in launched - assert asked == [("room-a", {"lineage": "continue-own"})] - assert notes[0][0] == "room worker started (pid 4242)" - warned = [m for m, kw in notes if "may still hold lessons" in m] - assert len(warned) == (1 if history is True else 0) - assert all("--fresh-session" in m and "fresh lineage" in m for m in warned) - assert not any(kw.get("severity") == "error" for _, kw in notes) + assert [m for m, _ in notes] == ["room worker started (pid 4242)"] drive(scenario()) @@ -3209,7 +3223,7 @@ async def scenario(): await pilot.pause() assert results == [{ "prompt": "prefill", "lineage": "branch-from", - "base_candidate": "cand-12345678", + "base_candidate": "cand-12345678", "lessons": [], }] drive(scenario()) @@ -3274,7 +3288,7 @@ async def scenario(): app._apply_room_round({ "prompt": " keep\n my indentation \n", "lineage": "continue-own", "base_candidate": None, - }) + }, "room-a", _ROUND_STORE) await pilot.pause() assert launched["prompt"] == " keep\n my indentation \n" @@ -3837,7 +3851,7 @@ def unavailable(repo_root): monkeypatch.setattr("syncmill.tui.repository_fingerprint", unavailable) room, *_ = _seeded_room(tmp_path) try: - comparison, _, _ = build_room_evidence( + comparison, *_ = build_room_evidence( room.store.root, 10**6, tmp_path, room.id, 0.0 ) finally: @@ -3910,7 +3924,7 @@ def build(store_root, output_max, repo_root, room_id, now): gate.wait(5) running["now"] -= 1 rows = [_candidate_row(candidate_id="cand-ok", member_id=f"build{running['builds']}")] - return _comparison(rows), 1, frozenset({"cand-ok"}) + return _comparison(rows), 1, frozenset({"cand-ok"}), 0 monkeypatch.setattr("syncmill.tui.build_room_evidence", build) rendered: list[str] = [] @@ -3960,7 +3974,7 @@ def test_a_store_root_change_retires_an_in_flight_build(tmp_path, git_repo, monk def build(store_root, output_max, repo_root, room_id, now): started.set() gate.wait(5) - return _comparison([_candidate_row(candidate_id="cand-ok")]), 1, frozenset({"cand-ok"}) + return _comparison([_candidate_row(candidate_id="cand-ok")]), 1, frozenset({"cand-ok"}), 0 monkeypatch.setattr("syncmill.tui.build_room_evidence", build) @@ -4036,7 +4050,7 @@ async def scenario(): _fake_evidence(monkeypatch, [_candidate_row(candidate_id="cand-ok")]) monkeypatch.setattr( "syncmill.tui.build_room_evidence", - lambda *a: (_comparison([_candidate_row(candidate_id="cand-ok")]), 1, frozenset({"cand-ok"})), + lambda *a: (_comparison([_candidate_row(candidate_id="cand-ok")]), 1, frozenset({"cand-ok"}), 0), ) await pilot.press("7") await pilot.pause() @@ -4126,7 +4140,7 @@ async def scenario(): ) monkeypatch.setattr( "syncmill.tui.build_room_evidence", - lambda *a: (comparison, 1, frozenset()), + lambda *a: (comparison, 1, frozenset(), 0), ) await pilot.press("7") await pilot.pause() @@ -4352,7 +4366,7 @@ def build(store_root, output_max, repo_root, room_id, now): started.set() gate.wait(5) rows = [_candidate_row(candidate_id="cand-ok", member_id=f"built-for-{room_id}")] - return _comparison(rows, room_id=room_id), 1, frozenset({"cand-ok", "cand-bad"}) + return _comparison(rows, room_id=room_id), 1, frozenset({"cand-ok", "cand-bad"}), 0 monkeypatch.setattr("syncmill.tui.build_room_evidence", build) @@ -4448,7 +4462,7 @@ def build(store_root, output_max, repo_root, room_id, now): started.set() gate.wait(5) rows = [_candidate_row(candidate_id="cand-ok", member_id=f"built-for-{room_id}")] - return _comparison(rows, room_id=room_id), 1, frozenset({"cand-ok", "cand-bad"}) + return _comparison(rows, room_id=room_id), 1, frozenset({"cand-ok", "cand-bad"}), 0 monkeypatch.setattr("syncmill.tui.build_room_evidence", build) @@ -4609,7 +4623,7 @@ async def scenario(): monkeypatch.setattr(app, "_reload_config", lambda: True) monkeypatch.setattr( "syncmill.tui.build_room_evidence", - lambda *a: (_comparison([]), 1, frozenset()), + lambda *a: (_comparison([]), 1, frozenset(), 0), ) await pilot.press("7") await pilot.pause() @@ -4650,7 +4664,7 @@ async def scenario(): app._rooms = _FakeRooms(store=_Damaged()) monkeypatch.setattr(app, "_reload_config", lambda: True) monkeypatch.setattr( - "syncmill.tui.build_room_evidence", lambda *a: (damaged, 1, frozenset()) + "syncmill.tui.build_room_evidence", lambda *a: (damaged, 1, frozenset(), 0) ) await pilot.press("7") await pilot.pause() @@ -4766,7 +4780,7 @@ async def scenario(): app._rooms = _FakeRooms(store=_OnlyDamaged()) monkeypatch.setattr(app, "_reload_config", lambda: True) monkeypatch.setattr( - "syncmill.tui.build_room_evidence", lambda *a: (damaged, 1, frozenset()) + "syncmill.tui.build_room_evidence", lambda *a: (damaged, 1, frozenset(), 0) ) await pilot.press("7") await pilot.pause() diff --git a/tests/test_tui_render.py b/tests/test_tui_render.py index 7cd7333..ec9c440 100644 --- a/tests/test_tui_render.py +++ b/tests/test_tui_render.py @@ -281,7 +281,9 @@ def markup_sinks(source: str, path: str) -> list[str]: ``plain_select`` exist for that); an ``OptionList`` without ``markup=False``; a DataTable row or column not wrapped in ``plain``; ``notify`` with ``markup=True`` or called on something other than the app - (``Widget.notify`` passes ``markup=True`` itself).""" + (``Widget.notify`` passes ``markup=True`` itself); a ``Checkbox`` whose + label is neither a string literal nor ``plain(...)`` (Textual parses a + ``str`` label as markup, so stored text must arrive as ``Text``).""" tree = ast.parse(source) found = [] @@ -307,6 +309,13 @@ def is_plain(node: ast.AST) -> bool: if cls is None and isinstance(func, ast.Name): if func.id in ("Static", "Label", "Select", "Option"): found.append(f"{where} {func.id}(") + if func.id == "Checkbox" and node.args: + label = node.args[0] + literal = isinstance(label, ast.Constant) and isinstance( + label.value, str + ) + if not (literal or is_plain(label)): + found.append(f"{where} Checkbox label without plain") if func.id == "OptionList": value = keyword(node, "markup") if not (isinstance(value, ast.Constant) and value.value is False): @@ -348,6 +357,8 @@ def test_textual_modules_render_no_text_as_markup(): ("OptionList(id='a')\n", "m.py:1 OptionList without markup=False"), ("t.add_row(name)\n", "m.py:1 add_row without plain"), ("t.add_columns('a', 'b')\n", "m.py:1 add_columns without plain"), + ("Checkbox(text, id='a')\n", "m.py:1 Checkbox label without plain"), + ("Checkbox(f'{lesson.text}')\n", "m.py:1 Checkbox label without plain"), ("class S:\n def f(self):\n self.notify('x')\n", "m.py:3 self.notify"), ("class SyncmillTuiApp:\n def f(self):\n self.notify('x', markup=True)\n", "m.py:3 notify markup"), @@ -361,6 +372,7 @@ def test_the_markup_guard_allows_the_plain_forms(): source = ( "PlainStatic('x')\nplain_select([], id='s')\nOptionList(id='a', markup=False)\n" "PlainOption('x', id='a')\n" + "Checkbox('I reviewed this', id='c')\nCheckbox(plain(text), value=True)\n" "t.add_row(*map(plain, cells), key=k)\nt.add_columns(plain('a'))\n" "class DashboardApp:\n def f(self):\n self.notify('x')\n" " super().notify('x', markup=markup)\n" diff --git a/tests/test_tui_room_review.py b/tests/test_tui_room_review.py new file mode 100644 index 0000000..66e6a00 --- /dev/null +++ b/tests/test_tui_room_review.py @@ -0,0 +1,1013 @@ +"""The TUI Rooms tab's human review actions (#174): recording a verdict (j), +promoting a lesson (p), and choosing approved lessons for a round. Driven +against a real RoomCoordinator and a real Board.""" + +from __future__ import annotations + +import asyncio +import sqlite3 + +import pytest +from textual.widgets import Button, Checkbox, Input, Select, TextArea + +from test_room_judgments import _applied_room +from test_rooms import WritingAgent, _config +from syncmill.board import Board, BoardError +from syncmill.config import SyncmillConfig +from syncmill.rooms import _ROOM_PROMPT_MAX_BYTES, RoomCoordinator, RoomError +from syncmill.tui import SyncmillTuiApp +from syncmill.tui_collaboration import ( + LessonPromoteModal, + RoomJudgeModal, + RoomRoundModal, + RoundLessonsView, + round_prompt, +) + + +def drive(coro): + return asyncio.run(coro) + + +def _app(tmp_path, git_repo, *, board: Board | None, enabled: bool = True, + board_error: str | None = None) -> SyncmillTuiApp: + return SyncmillTuiApp( + SyncmillConfig( + repo_root=str(git_repo), + runs={"root": str(tmp_path / "runs")}, + board={"enabled": enabled, "db_path": str(tmp_path / "board.db")}, + ), + board=board, + board_error=board_error, + toml_path=git_repo / "syncmill.toml", + refresh_seconds=3600, + initial_tab="board", + ) + + +def _coordinator(git_repo, tmp_path) -> RoomCoordinator: + return RoomCoordinator( + _config(tmp_path), git_repo, agents=[WritingAgent("claude"), WritingAgent("codex")] + ) + + +def _notes(app, monkeypatch) -> list[tuple[str, dict]]: + notes: list[tuple[str, dict]] = [] + real = app.notify + + def notify(message, **kwargs): + notes.append((message, kwargs)) + real(message, **kwargs) + + monkeypatch.setattr(app, "notify", notify) + return notes + + +def _captured(app, monkeypatch) -> dict: + staged: dict = {} + monkeypatch.setattr( + app, "push_screen", + lambda screen, callback: staged.update(screen=screen, cb=callback), + ) + return staged + + +async def _press_apply(pilot, modal) -> None: + modal.on_button_pressed(Button.Pressed(modal.query_one("#apply", Button))) + await pilot.pause() + + +# ------------------------------------------------------------------ verdict + + +def test_a_verdict_recorded_in_the_tui_is_bound_and_marked_tui( + git_repo, tmp_path, monkeypatch +): + coordinator = _coordinator(git_repo, tmp_path) + room_id, candidate, run_id = _applied_room(coordinator) + async def scenario(): + app = _app(tmp_path, git_repo, board=None, enabled=False) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room_id + notes = _notes(app, monkeypatch) + app._confirm_room_judge() + await pilot.pause() + modal = app.screen + assert isinstance(modal, RoomJudgeModal) + assert candidate["patch_sha256"] in modal._detail + assert f"Outcome: applied as run {run_id}, not reviewed" in modal._detail + + # No default verdict: confirming without one records nothing. + modal.query_one("#confirmed", Checkbox).value = True + await _press_apply(pilot, modal) + assert app.screen is modal + assert coordinator.store.outcome(room_id).status == "applied_unreviewed" + + modal.query_one("#judge-verdict", Select).value = "accepted" + modal.query_one("#judge-rationale", TextArea).text = " matches the spec " + await _press_apply(pilot, modal) + outcome = coordinator.store.outcome(room_id) + assert outcome.status == "accepted" + assert outcome.judgment["surface"] == "tui" + assert outcome.judgment["patch_sha256"] == candidate["patch_sha256"] + # Normalized by the store, as for `room judge --rationale`. + assert outcome.judgment["rationale"] == "matches the spec" + assert notes[-1][0] == f"recorded accepted for candidate {candidate['id'][:8]}" + assert "Outcome: ACCEPTED (human)" in app.room_detail_text + + drive(scenario()) + + +@pytest.mark.parametrize("stage", ["created", "ran", "selected"]) +def test_a_verdict_needs_an_applied_change(git_repo, tmp_path, monkeypatch, stage): + coordinator = _coordinator(git_repo, tmp_path) + room = coordinator.create("compare", ["claude", "codex"]) + if stage != "created": + coordinator.run_round(room["id"]) + if stage == "selected": + first = coordinator.store.list_candidates(room["id"])[0] + coordinator.select(room["id"], first["id"]) + async def scenario(): + app = _app(tmp_path, git_repo, board=None, enabled=False) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room["id"] + notes = _notes(app, monkeypatch) + staged = _captured(app, monkeypatch) + app._confirm_room_judge() + assert staged == {} + assert "only an applied change can be accepted or rejected" in notes[-1][0] + + drive(scenario()) + + +def test_a_repeated_verdict_says_nothing_changed(git_repo, tmp_path, monkeypatch): + coordinator = _coordinator(git_repo, tmp_path) + room_id, candidate, _ = _applied_room(coordinator) + async def scenario(): + app = _app(tmp_path, git_repo, board=None, enabled=False) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room_id + notes = _notes(app, monkeypatch) + root = str(coordinator.store.root) + fields = {"verdict": "rejected", "rationale": ""} + app._apply_room_judge(fields, room_id, root, candidate["patch_sha256"]) + app._apply_room_judge(fields, room_id, root, candidate["patch_sha256"]) + assert notes[-1][0] == "already recorded: rejected (judgment 1); nothing changed" + assert coordinator.store.outcome(room_id).judgment_count == 1 + + drive(scenario()) + + +def test_a_verdict_is_fenced_on_the_digest_and_store_it_was_reviewed_on( + git_repo, tmp_path, monkeypatch +): + coordinator = _coordinator(git_repo, tmp_path) + room_id, candidate, _ = _applied_room(coordinator) + async def scenario(): + app = _app(tmp_path, git_repo, board=None, enabled=False) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room_id + notes = _notes(app, monkeypatch) + fields = {"verdict": "accepted", "rationale": ""} + app._apply_room_judge(fields, room_id, str(coordinator.store.root), "0" * 64) + assert notes[-1][1].get("severity") == "error" + assert "differs from the one you reviewed" in notes[-1][0] + app._apply_room_judge(fields, room_id, "/another/store", candidate["patch_sha256"]) + assert "Rooms store changed while the dialog was open" in notes[-1][0] + assert coordinator.store.outcome(room_id).judgment_count == 0 + + drive(scenario()) + + +def test_a_verdict_dialog_targets_the_room_it_was_opened_on( + git_repo, tmp_path, monkeypatch +): + coordinator = _coordinator(git_repo, tmp_path) + room_id, candidate, _ = _applied_room(coordinator) + async def scenario(): + app = _app(tmp_path, git_repo, board=None, enabled=False) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room_id + staged = _captured(app, monkeypatch) + app._confirm_room_judge() + app._selected_room_id = "some-other-room" # a poll moved the selection + staged["cb"]({"verdict": "accepted", "rationale": ""}) + assert coordinator.store.outcome(room_id).status == "accepted" + + drive(scenario()) + + +def test_a_broken_judgment_table_never_kills_the_app(git_repo, tmp_path, monkeypatch): + coordinator = _coordinator(git_repo, tmp_path) + room_id, candidate, _ = _applied_room(coordinator) + async def scenario(): + app = _app(tmp_path, git_repo, board=None, enabled=False) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room_id + notes = _notes(app, monkeypatch) + staged = _captured(app, monkeypatch) + app._confirm_room_judge() # opens on a healthy store + coordinator.store._conn.execute("DROP TABLE room_judgments") + staged["cb"]({"verdict": "accepted", "rationale": ""}) + assert notes[-1][1].get("severity") == "error" + app._confirm_room_judge() + assert notes[-1][1].get("severity") == "error" + assert "no such table" in notes[-1][0] + # The room pane stays up and says why the outcome is missing. + app._refresh_rooms() + assert "Outcome: unavailable (" in app.room_detail_text + assert app.is_running + + drive(scenario()) + + +# ------------------------------------------------------------------ promote + + +def _judged(coordinator, verdict="accepted"): + room_id, candidate, run_id = _applied_room(coordinator) + record, _ = coordinator.judge(room_id, verdict) + return room_id, record + + +def test_promoting_needs_the_board(git_repo, tmp_path, monkeypatch): + coordinator = _coordinator(git_repo, tmp_path) + room_id, _ = _judged(coordinator) + async def scenario(): + app = _app(tmp_path, git_repo, board=None, enabled=False) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room_id + notes = _notes(app, monkeypatch) + app._confirm_lesson_promote() + assert "set SYNCMILL_BOARD__ENABLED=true" in notes[-1][0] + app._config.board.enabled = True + app._board_error = "board.db is not a database" + app._confirm_lesson_promote() + assert notes[-1][0] == ( + "promoting a lesson needs the board, which did not open: " + "board.db is not a database" + ) + + drive(scenario()) + + +def test_promoting_needs_a_verdict(git_repo, tmp_path, monkeypatch): + coordinator = _coordinator(git_repo, tmp_path) + room_id, _, _ = _applied_room(coordinator) + board = Board(str(tmp_path / "board.db")) + async def scenario(): + app = _app(tmp_path, git_repo, board=board) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room_id + notes = _notes(app, monkeypatch) + staged = _captured(app, monkeypatch) + app._confirm_lesson_promote() + assert staged == {} + assert "room outcome is applied_unreviewed" in notes[-1][0] + assert "record a verdict with j" in notes[-1][0] + + drive(scenario()) + + +def test_a_lesson_promoted_in_the_tui_carries_the_judgment_it_was_reviewed_with( + git_repo, tmp_path, monkeypatch +): + coordinator = _coordinator(git_repo, tmp_path) + room_id, record = _judged(coordinator) + board = Board(str(tmp_path / "board.db")) + async def scenario(): + app = _app(tmp_path, git_repo, board=board) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room_id + notes = _notes(app, monkeypatch) + app._confirm_lesson_promote() + await pilot.pause() + modal = app.screen + assert isinstance(modal, LessonPromoteModal) + assert record["judgment_id"] in modal._detail + modal.query_one("#promote-lesson", TextArea).text = "prefer small diffs" + modal.query_one("#confirmed", Checkbox).value = True + await _press_apply(pilot, modal) + [lesson] = board.project_lessons(coordinator.project_id) + assert lesson.text == "prefer small diffs" + assert lesson.provenance["judgment_id"] == record["judgment_id"] + assert notes[-1][0] == f"promoted lesson {lesson.id[:8]} for the whole project" + + # The same promotion again changes nothing. + app._apply_lesson_promote( + {"lesson": "prefer small diffs", "scope": "", "supersedes": None}, + room_id, str(coordinator.store.root), record["judgment_id"], + ) + assert notes[-1][0] == ( + f"already promoted as lesson {lesson.id[:8]} (active); nothing changed" + ) + assert len(board.project_lessons(coordinator.project_id)) == 1 + + drive(scenario()) + + +def test_a_verdict_changed_during_the_dialog_is_not_promoted( + git_repo, tmp_path, monkeypatch +): + coordinator = _coordinator(git_repo, tmp_path) + room_id, _ = _judged(coordinator, "accepted") + board = Board(str(tmp_path / "board.db")) + async def scenario(): + app = _app(tmp_path, git_repo, board=board) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room_id + notes = _notes(app, monkeypatch) + staged = _captured(app, monkeypatch) + app._confirm_lesson_promote() + coordinator.judge(room_id, "rejected") # another terminal + staged["cb"]({"lesson": "keep it", "scope": "", "supersedes": None}) + assert "verdict changed since you reviewed it" in notes[-1][0] + assert board.project_lessons(coordinator.project_id) == [] + + drive(scenario()) + + +def test_only_this_projects_lessons_can_be_replaced(git_repo, tmp_path, monkeypatch): + coordinator = _coordinator(git_repo, tmp_path) + room_id, _ = _judged(coordinator) + board = Board(str(tmp_path / "board.db")) + project = coordinator.project_id + prov = {"kind": "room-judgment", "room_id": "r", "judgment_id": "j", + "verdict": "accepted", "candidate_id": "c", "patch_sha256": "a" * 64, + "applied_run_id": "run"} + own, _ = board.promote_lesson("own", project_id=project, provenance=prov, + source_key="k1") + goal, _ = board.promote_lesson("goal", project_id=project, provenance=prov, + source_key="k2", scope="backend") + board.promote_lesson("foreign", project_id="other", provenance=prov, source_key="k3") + board.add_decision("unbound") + async def scenario(): + app = _app(tmp_path, git_repo, board=board) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room_id + app._confirm_lesson_promote() + await pilot.pause() + modal = app.screen + assert [(i, s) for _, i, s in modal._supersedable] == [ + (own.id, ""), (goal.id, "backend") + ] + # A replacement must share the scope: refused in the dialog. + modal.query_one("#promote-lesson", TextArea).text = "better" + modal.query_one("#promote-supersedes", Select).value = goal.id + modal.query_one("#confirmed", Checkbox).value = True + await _press_apply(pilot, modal) + assert app.screen is modal + modal.query_one("#promote-scope", Input).value = "backend" + await _press_apply(pilot, modal) + [replacement] = [ + d for d in board.active_decisions(project_id=project) if d.text == "better" + ] + assert replacement.scope == "backend" + assert board.active_decisions(project_id=project).count(goal) == 0 + + drive(scenario()) + + +def test_a_board_that_fails_after_promotion_degrades_instead_of_crashing( + git_repo, tmp_path, monkeypatch +): + coordinator = _coordinator(git_repo, tmp_path) + room_id, record = _judged(coordinator) + board = Board(str(tmp_path / "board.db")) + async def scenario(): + app = _app(tmp_path, git_repo, board=board) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room_id + + def broken(*args, **kwargs): + raise BoardError("active_decisions failed: disk I/O error") + + monkeypatch.setattr(board, "active_decisions", broken) + app._apply_lesson_promote( + {"lesson": "prefer small diffs", "scope": "", "supersedes": None}, + room_id, str(coordinator.store.root), record["judgment_id"], + ) + assert len(board.project_lessons(coordinator.project_id)) == 1 + assert "DEGRADED: active_decisions failed" in str( + app.query_one("#board-status").render() + ) + app.action_tab("board-tab") + await pilot.pause() + app._poll() + assert app.is_running + + drive(scenario()) + + +def test_the_board_tab_survives_an_unreadable_selected_item( + git_repo, tmp_path, monkeypatch +): + board = Board(str(tmp_path / "board.db")) + item = board.add("a task") + async def scenario(): + app = _app(tmp_path, git_repo, board=board) + async with app.run_test() as pilot: + await pilot.pause() + app._selected_item_id = item.id + + def broken(*args, **kwargs): + raise BoardError("get failed: database is locked") + + monkeypatch.setattr(board, "get", broken) + app._refresh_board() + assert "DEGRADED: get failed" in str(app.query_one("#board-status").render()) + assert app.is_running + + drive(scenario()) + + +# ------------------------------------------------------------ round lessons + + +def _lessons(board, project, count, *, text="lesson {i}"): + prov = {"kind": "room-judgment", "room_id": "r", "judgment_id": "j", + "verdict": "accepted", "candidate_id": "c", "patch_sha256": "a" * 64, + "applied_run_id": "run"} + return [ + board.promote_lesson(text.format(i=i), project_id=project, provenance=prov, + source_key=f"k{i}")[0] + for i in range(count) + ] + + +def _launches(monkeypatch) -> list[dict]: + launched: list[dict] = [] + + def fake_launch(root, room_id, prompt=None, **kwargs): + launched.append({"room_id": room_id, "prompt": prompt, **kwargs}) + return 4242 + + monkeypatch.setattr("syncmill.tui.launch_room_worker", fake_launch) + return launched + + +def test_a_round_sends_exactly_the_checked_lessons_with_the_clis_budget( + git_repo, tmp_path, monkeypatch +): + coordinator = _coordinator(git_repo, tmp_path) + room = coordinator.create("compare", ["claude", "codex"]) + board = Board(str(tmp_path / "board.db")) + lessons = _lessons(board, coordinator.project_id, 21) # the budget is 20 + async def scenario(): + app = _app(tmp_path, git_repo, board=board) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room["id"] + launched = _launches(monkeypatch) + app._confirm_room_round() + await pilot.pause() + modal = app.screen + assert isinstance(modal, RoomRoundModal) + status = str(modal.query_one("#round-lessons-status").render()) + assert "Lessons this round: 20 of 21" in status + assert f"left out: {lessons[0].id[:8]} (over the per-run lesson budget)" in status + + # Unchecking one of the newest frees budget for the oldest, as + # --skip-lesson does. + modal.query_one("#round-lesson-20", Checkbox).value = False + await pilot.pause() + status = str(modal.query_one("#round-lessons-status").render()) + assert f"left out: {lessons[20].id[:8]} (unchecked)" in status + assert "over the per-run lesson budget" not in status + + modal.query_one("#confirmed", Checkbox).value = True + await _press_apply(pilot, modal) + [launch] = launched + assert [(x.id, x.text) for x in launch["lessons"]] == [ + (x.id, x.text) for x in lessons[:20] + ] + assert launch["room_id"] == room["id"] + + drive(scenario()) + + +def test_a_round_that_cannot_fit_its_lessons_is_refused_until_it_does( + git_repo, tmp_path, monkeypatch +): + coordinator = _coordinator(git_repo, tmp_path) + room = coordinator.create("compare", ["claude", "codex"]) + board = Board(str(tmp_path / "board.db")) + _lessons(board, coordinator.project_id, 1, text="x" * 900 + "{i}") + async def scenario(): + app = _app(tmp_path, git_repo, board=board) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room["id"] + notes = _notes(app, monkeypatch) + launched = _launches(monkeypatch) + app._confirm_room_round() + await pilot.pause() + modal = app.screen + modal.query_one("#round-prompt", TextArea).text = "p" * (_ROOM_PROMPT_MAX_BYTES - 200) + modal.query_one("#confirmed", Checkbox).value = True + await _press_apply(pilot, modal) + assert app.screen is modal and launched == [] + assert "leaves no room for the checked lessons" in notes[-1][0] + + # Shortening the prompt lets it through with the lesson. + modal.query_one("#round-prompt", TextArea).text = "short" + await _press_apply(pilot, modal) + assert len(launched) == 1 and len(launched[0]["lessons"]) == 1 + + drive(scenario()) + + +def test_the_fit_check_and_the_launch_read_a_blank_prompt_the_same_way( + git_repo, tmp_path +): + coordinator = _coordinator(git_repo, tmp_path) + room = coordinator.create("compare", ["claude", "codex"]) + board = Board(str(tmp_path / "board.db")) + async def scenario(): + app = _app(tmp_path, git_repo, board=board) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + huge = {"prompt": "p" * (_ROOM_PROMPT_MAX_BYTES - 200)} + view = app._round_lessons(room["id"], huge) + assert round_prompt(" \n\t") is None + assert round_prompt(" keep ") == " keep " + # None = the creation prompt, which here leaves no room. + assert not view.fits(round_prompt(" "), ["x" * 900]) + assert view.fits(round_prompt("short"), ["x" * 900]) + + drive(scenario()) + + +def test_a_round_without_usable_lessons_says_why_and_sends_none( + git_repo, tmp_path, monkeypatch +): + coordinator = _coordinator(git_repo, tmp_path) + room = coordinator.create("compare", ["claude", "codex"]) + board = Board(str(tmp_path / "board.db")) + async def scenario(): + app = _app(tmp_path, git_repo, board=board) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + r = coordinator.store.get_room(room["id"]) + assert app._round_lessons(room["id"], r).lessons == () + + def broken(project): + raise BoardError("project_lessons failed: disk I/O error") + + monkeypatch.setattr(board, "project_lessons", broken) + view = app._round_lessons(room["id"], r) + assert view.unavailable == "project_lessons failed: disk I/O error" + app._board = None + app._board_error = "board.db is locked" + assert app._round_lessons(room["id"], r).unavailable == "board.db is locked" + app._config.board.enabled = False + view = app._round_lessons(room["id"], r) + assert view.unavailable is None and view.lessons == () + + launched = _launches(monkeypatch) + app._apply_room_round( + {"prompt": "go", "lineage": "fresh", "base_candidate": None, "lessons": []}, + room["id"], str(coordinator.store.root), + ) + assert launched[0]["lessons"] == () + + drive(scenario()) + + +def test_a_lesson_id_the_dialog_did_not_show_is_refused(git_repo, tmp_path, monkeypatch): + coordinator = _coordinator(git_repo, tmp_path) + room = coordinator.create("compare", ["claude", "codex"]) + async def scenario(): + app = _app(tmp_path, git_repo, board=None, enabled=False) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + launched = _launches(monkeypatch) + notes = _notes(app, monkeypatch) + app._apply_room_round( + {"prompt": "go", "lineage": "fresh", "base_candidate": None, + "lessons": ["00000000-0000-4000-8000-000000000000"]}, + room["id"], str(coordinator.store.root), + ) + assert launched == [] and notes[-1][1].get("severity") == "error" + + drive(scenario()) + + +def test_lesson_text_in_the_round_dialog_is_never_markup(git_repo, tmp_path): + coordinator = _coordinator(git_repo, tmp_path) + room = coordinator.create("compare", ["claude", "codex"]) + board = Board(str(tmp_path / "board.db")) + [lesson] = _lessons( + board, coordinator.project_id, 1, text="[b]bold[/b] [oops \u202eRTL{i}" + ) + + async def scenario(): + app = _app(tmp_path, git_repo, board=board) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room["id"] + app._confirm_room_round() + await pilot.pause() + label = app.screen.query_one("#round-lesson-0", Checkbox).label + assert label.plain == f"lesson {lesson.id[:8]}" and not label.spans + shown = app.screen.query_one("#round-lesson-text-0").render() + assert "[b]bold[/b] [oops \\u202eRTL0" in str(shown) + assert not getattr(shown, "spans", None) + + drive(scenario()) + + +def test_a_long_lesson_is_shown_whole_in_the_round_dialog_at_80x24(git_repo, tmp_path): + """Every word of a lesson is sent, so every word must be readable: a long + lesson wraps inside the dialog instead of running past its edge.""" + coordinator = _coordinator(git_repo, tmp_path) + room = coordinator.create("compare", ["claude", "codex"]) + board = Board(str(tmp_path / "board.db")) + _lessons(board, coordinator.project_id, 1, text="word " * 80 + "FINALWORDS{i}") + + async def scenario(): + app = _app(tmp_path, git_repo, board=board) + async with app.run_test(size=(80, 24)) as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room["id"] + app._confirm_room_round() + await pilot.pause() + body = app.screen.query_one("#round-body") + text = app.screen.query_one("#round-lesson-text-0") + assert body.max_scroll_x == 0 + assert text.region.right <= body.region.right + assert text.region.height > 1 # wrapped, not cut + text.scroll_visible(animate=False) + await pilot.pause() + lines = [ + "".join(segment.text for segment in strip) + for strip in text.render_lines(text.region.reset_offset) + ] + assert "FINALWORDS0" in "".join(lines) + + drive(scenario()) + + +def test_the_round_dialog_keeps_its_buttons_reachable_at_80x24(git_repo, tmp_path): + coordinator = _coordinator(git_repo, tmp_path) + room = coordinator.create("compare", ["claude", "codex"]) + board = Board(str(tmp_path / "board.db")) + _lessons(board, coordinator.project_id, 21) + async def scenario(): + app = _app(tmp_path, git_repo, board=board) + async with app.run_test(size=(80, 24)) as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room["id"] + app._confirm_room_round() + await pilot.pause() + for selector in ("#confirmed", "#apply", "#cancel"): + region = app.screen.query_one(selector).region + assert region.height > 0 and region.bottom <= 24, selector + + drive(scenario()) + + +def test_the_history_note_follows_the_chosen_lineage(git_repo, tmp_path): + coordinator = _coordinator(git_repo, tmp_path) + room = coordinator.create("compare", ["claude", "codex"]) + async def scenario(): + app = _app(tmp_path, git_repo, board=None, enabled=False) + async with app.run_test() as pilot: + await pilot.pause() + app.push_screen( + RoomRoundModal( + coordinator.store.get_room(room["id"]), "p", [], + RoundLessonsView(history={"continue-own": True, "branch-from": False}), + ) + ) + await pilot.pause() + modal = app.screen + status = lambda: str(modal.query_one("#round-lessons-status").render()) # noqa: E731 + assert "may still hold lessons sent in earlier rounds" in status() + modal.query_one("#round-lineage", Select).value = "fresh" + await pilot.pause() + assert status() == "" + + drive(scenario()) + + +# ---------------------------------------------------------------- freshness + + +def _snapshot(app, coordinator, room_id): + from syncmill.tui import build_room_evidence + + result = build_room_evidence( + coordinator.store.root, coordinator.store.candidates.output_max_bytes, + coordinator.repo_root, room_id, 0.0, + ) + app._apply_evidence( + app._evidence_generation, room_id, str(coordinator.store.root), result, None, 0.0 + ) + + +def test_a_verdict_from_anywhere_marks_the_compare_snapshot_stale( + git_repo, tmp_path +): + coordinator = _coordinator(git_repo, tmp_path) + room_id, _, _ = _applied_room(coordinator) + async def scenario(): + app = _app(tmp_path, git_repo, board=None, enabled=False) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room_id + _snapshot(app, coordinator, room_id) + + def text(): + return app._stage_evidence_view(room_id, None, "compare", 1)[1] + + assert "room changed since this snapshot" not in text() + coordinator.judge(room_id, "accepted") # e.g. `room judge` elsewhere + assert "room changed since this snapshot" in text() + _snapshot(app, coordinator, room_id) + coordinator.judge(room_id, "accepted") # idempotent: no new row + assert "room changed since this snapshot" not in text() + + coordinator.store._conn.execute("DROP TABLE room_judgments") + shown = text() + assert "cannot tell whether a verdict changed since this snapshot" in shown + assert "Outcome:" in shown # the snapshot still renders + + drive(scenario()) + + +# ------------------------------------------------------------- coordinator + + +def test_promote_lesson_refuses_a_verdict_other_than_the_one_reviewed( + git_repo, tmp_path +): + coordinator = _coordinator(git_repo, tmp_path) + board = Board(str(tmp_path / "board.db")) + try: + room_id, first = _judged(coordinator, "accepted") + coordinator.judge(room_id, "rejected") + with pytest.raises(RoomError, match="verdict changed since you reviewed it"): + coordinator.promote_lesson( + room_id, board, "a lesson", expected_judgment_id=first["judgment_id"] + ) + assert board.project_lessons(coordinator.project_id) == [] + latest = coordinator.store.outcome(room_id).judgment + lesson, created = coordinator.promote_lesson( + room_id, board, "a lesson", expected_judgment_id=latest["judgment_id"] + ) + assert created and lesson.provenance["judgment_id"] == latest["judgment_id"] + finally: + board.close() + coordinator.close() + + +def test_judgment_count_counts_rows(git_repo, tmp_path): + coordinator = _coordinator(git_repo, tmp_path) + try: + room_id, _, _ = _applied_room(coordinator) + assert coordinator.store.judgment_count(room_id) == 0 + coordinator.judge(room_id, "accepted") + coordinator.judge(room_id, "rejected") + assert coordinator.store.judgment_count(room_id) == 2 + assert coordinator.store.judgment_count("no-such-room") == 0 + finally: + coordinator.close() + + +def test_a_blank_round_prompt_is_checked_against_the_creation_prompt( + git_repo, tmp_path, monkeypatch +): + """A whitespace-only prompt reuses the creation prompt, so the dialog's + fit check must measure that one, not the whitespace.""" + coordinator = _coordinator(git_repo, tmp_path) + room = coordinator.create("c" * (_ROOM_PROMPT_MAX_BYTES - 200), ["claude", "codex"]) + board = Board(str(tmp_path / "board.db")) + _lessons(board, coordinator.project_id, 1, text="x" * 900 + "{i}") + + async def scenario(): + app = _app(tmp_path, git_repo, board=board) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room["id"] + launched = _launches(monkeypatch) + app._confirm_room_round() + await pilot.pause() + modal = app.screen + modal.query_one("#round-prompt", TextArea).text = " \n" + modal.query_one("#confirmed", Checkbox).value = True + await _press_apply(pilot, modal) + assert app.screen is modal and launched == [] + + drive(scenario()) + + +def test_a_room_that_turns_unreadable_after_the_round_dialog_opened_is_reported( + git_repo, tmp_path, monkeypatch +): + coordinator = _coordinator(git_repo, tmp_path) + room = coordinator.create("compare", ["claude", "codex"]) + + async def scenario(): + app = _app(tmp_path, git_repo, board=None, enabled=False) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room["id"] + notes = _notes(app, monkeypatch) + staged = _captured(app, monkeypatch) + launched = _launches(monkeypatch) + app._confirm_room_round() + with coordinator.store._tx() as conn: + conn.execute( + "UPDATE rooms SET members_json='{not json' WHERE id=?", (room["id"],) + ) + staged["cb"]({"prompt": "go", "lineage": "fresh", "base_candidate": None, + "lessons": []}) + await pilot.pause() + assert launched == [] + assert notes[-1][1].get("severity") == "error" + assert app.is_running + + drive(scenario()) + + +def test_a_snapshot_that_could_not_count_verdicts_never_claims_it_is_current( + git_repo, tmp_path, monkeypatch +): + from syncmill.rooms import RoomStore + + coordinator = _coordinator(git_repo, tmp_path) + room_id, _, _ = _applied_room(coordinator) + real = RoomStore.judgment_count + + async def scenario(): + app = _app(tmp_path, git_repo, board=None, enabled=False) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room_id + + def failing(self, room_id): + raise sqlite3.OperationalError("database is locked") + + monkeypatch.setattr(RoomStore, "judgment_count", failing) + _snapshot(app, coordinator, room_id) # the build cannot count + monkeypatch.setattr(RoomStore, "judgment_count", real) + coordinator.judge(room_id, "accepted") + text = app._stage_evidence_view(room_id, None, "compare", 1)[1] + assert "cannot tell whether a verdict changed since this snapshot" in text + assert "this snapshot could not count verdicts" in text + + drive(scenario()) + + +def test_the_round_dialog_says_when_the_board_is_off(git_repo, tmp_path): + coordinator = _coordinator(git_repo, tmp_path) + room = coordinator.create("compare", ["claude", "codex"]) + + async def scenario(): + app = _app(tmp_path, git_repo, board=None, enabled=False) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room["id"] + app._confirm_room_round() + await pilot.pause() + shown = " ".join( + str(widget.render()) for widget in app.screen.query("PlainStatic") + ) + assert "the board is off, so this round sends no approved lessons" in shown + + drive(scenario()) + + +def test_the_verdict_dialog_shows_the_verdict_it_would_supersede(git_repo, tmp_path): + coordinator = _coordinator(git_repo, tmp_path) + room_id, _, _ = _applied_room(coordinator) + coordinator.judge(room_id, "rejected", rationale="broke the parser") + + async def scenario(): + app = _app(tmp_path, git_repo, board=None, enabled=False) + async with app.run_test() as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room_id + app._confirm_room_judge() + await pilot.pause() + detail = app.screen._detail + assert "Outcome: REJECTED (human)" in detail + assert "judged via API" in detail # coordinator.judge default surface + assert "broke the parser" in detail + + drive(scenario()) + + +def test_lessons_can_be_toggled_and_submitted_with_the_mouse_at_80x24( + git_repo, tmp_path, monkeypatch +): + coordinator = _coordinator(git_repo, tmp_path) + room = coordinator.create("compare", ["claude", "codex"]) + board = Board(str(tmp_path / "board.db")) + lessons = _lessons(board, coordinator.project_id, 21) + + async def scenario(): + app = _app(tmp_path, git_repo, board=board) + async with app.run_test(size=(80, 24)) as pilot: + await pilot.pause() + app._rooms = coordinator + app._selected_room_id = room["id"] + launched = _launches(monkeypatch) + app._confirm_room_round() + await pilot.pause() + modal = app.screen + box = modal.query_one("#round-lesson-20", Checkbox) + box.scroll_visible(animate=False) + await pilot.pause() + await pilot.click("#round-lesson-20") + await pilot.pause() + assert box.value is False + await pilot.click("#confirmed") + await pilot.pause() + await pilot.click("#apply") + await pilot.pause() + [launch] = launched + assert [x.id for x in launch["lessons"]] == [x.id for x in lessons[:20]] + + drive(scenario()) + + +@pytest.mark.parametrize("size", [(80, 24), (120, 40)]) +@pytest.mark.parametrize("dialog", ["round", "judge", "promote"]) +def test_text_typed_into_a_dialog_editor_stays_visible(tmp_path, git_repo, size, dialog): + """A TextArea's default 1fr height collapses to its border inside a + scrolling dialog body: typing changed the value but showed nothing.""" + board = Board(str(tmp_path / "board.db")) + lessons = tuple(_lessons(board, "proj", 3)) + + async def scenario(): + app = _app(tmp_path, git_repo, board=None, enabled=False) + async with app.run_test(size=size) as pilot: + await pilot.pause() + modal, selector = { + "round": ( + RoomRoundModal( + {"id": "r", "members": [], "round_count": 0}, "", [], + RoundLessonsView(lessons=lessons), + ), + "#round-prompt", + ), + "judge": (RoomJudgeModal("Room: r\n" * 8), "#judge-rationale"), + "promote": (LessonPromoteModal("source\n" * 8, []), "#promote-lesson"), + }[dialog] + app.push_screen(modal) + await pilot.pause() + editor = app.screen.query_one(selector, TextArea) + editor.focus() + editor.scroll_visible(animate=False) + await pilot.pause() + await pilot.press(*"visible words") + await pilot.pause() + assert editor.text == "visible words" + assert editor.content_region.height >= 3 + shown = "".join( + "".join(segment.text for segment in editor.render_line(y)) + for y in range(editor.region.height) + ) + assert "visible words" in shown + + drive(scenario())