feat: operator-initiated clarification escalation — 0.3.7#92
Conversation
Operators can now escalate a low-confidence clarification to the worker from any channel. Escalation is an auditable decision today; the actual side-instruction transport to the worker ships in 0.3.8. - CLI: thin-supervisor clarify <run_id> <question> [--escalate] - Daemon: escalate_clarification IPC + DaemonClient.escalate_clarification - Actions: do_escalate_clarification (daemon + local fallback) - TUI: 'E' keybind + escalate hint in format_clarification - IM: /escalate <run_id> [question] — defaults to last clarification_response - Emits a single clarification_escalated_to_worker event with transport="pending_0_3_8" so the audit trail is unambiguous.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 43 minutes and 46 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR introduces operator-initiated clarification escalation functionality in version 0.3.7. It adds a CLI Changes
Sequence DiagramsequenceDiagram
participant User as User (TUI)
participant TUI as TUI UI Layer
participant OpAction as Operator Action
participant Daemon as Daemon Server
participant SessionLog as Session Log
User->>TUI: Press 'E' key (low-confidence clarification)
TUI->>OpAction: do_escalate_clarification(question, confidence, reason="tui_low_confidence")
alt Daemon Available
OpAction->>Daemon: escalate_clarification(run_id, question, reason, operator, confidence)
Daemon->>SessionLog: append clarification_escalated_to_worker event
Daemon-->>OpAction: {ok: true, escalation_id}
OpAction-->>TUI: {source: "daemon", escalation_id}
else Local Mode
OpAction->>SessionLog: append clarification_escalated_to_worker event (transport: "pending_0_3_8")
OpAction-->>TUI: {source: "local", escalation_id}
end
TUI->>User: Display escalation_id in status bar
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request updates the version to 0.3.7 and introduces operator-initiated clarification escalation across CLI, TUI, and IM interfaces. Key additions include a new clarify command, daemon IPC support for escalation, and audit logging for these actions. Feedback highlights a logic error in the IM command dispatcher where the oldest rather than the newest clarification is selected for escalation, and a state management issue in the TUI where escalation hints persist across different runs.
| from supervisor.operator.api import timeline_from_session_log | ||
|
|
||
| events = timeline_from_session_log(log_path, limit=50) | ||
| for ev in events: |
There was a problem hiding this comment.
The loop iterates through events in chronological order (assuming timeline_from_session_log returns them as they appear in the log). This means it will find the oldest clarification response within the window, rather than the most recent one. This can lead to escalating an outdated question if multiple clarifications have occurred.
| for ev in events: | |
| for ev in reversed(events): |
| "escalation_recommended": True, | ||
| "language": pending_job.get("language", language), | ||
| } | ||
| status_msg = " Low-confidence answer — press 'E' to escalate, any other key to dismiss " |
There was a problem hiding this comment.
The last_clarify state and the associated status_msg are not cleared when the user navigates between runs (e.g., using 'j' or 'k'). This results in a stale "press 'E' to escalate" hint remaining visible while a different run is selected. If the user then presses 'E', it will escalate the clarification for the previous run while the UI highlights the current one, which is misleading and potentially error-prone.
- actions.do_escalate_clarification: strip+reject empty/whitespace questions at the top so the daemon path and local path behave consistently (CodeRabbit). - app.cmd_clarify: catch ConnectionRefusedError/FileNotFoundError/OSError around submit_clarification, each poll_job iteration, and do_escalate_clarification, matching the cmd_observe pattern so a daemon that dies mid-clarify surfaces a clean error instead of a traceback (CodeRabbit). - tui: drop the unimplemented "any other key to dismiss" promise from the escalation status; clear last_clarify when the operator navigates to a different run via j/k so a subsequent 'E' can't escalate a clarification for a run the operator is no longer looking at (gemini/CodeRabbit). - tests: add a regression test that proves /escalate resolves the newest clarification_response when multiple exist (gemini flagged ordering; verified newest-first is correct, now pinned by a test); add a unit test for the new empty-question guard.
Summary
do_escalate_clarificationaction:thin-supervisor clarify ... --escalateCLI, TUIEkeybind, and IM/escalate <run_id> [question].clarification_escalated_to_workertimeline event withtransport="pending_0_3_8". Actual side-instruction transport to the worker (queue drain by SupervisorLoop + worker-reply capture) lands in 0.3.8.What's included
supervisor/app.py—thin-supervisor clarify <run_id> <question> [--escalate] [--operator <name>]supervisor/daemon/server.py—_do_escalate_clarificationdispatch + handler (resolves both active and on-disk runs)supervisor/daemon/client.py—escalate_clarification(...)IPC methodsupervisor/operator/actions.py—do_escalate_clarification(ctx, ...)piggy-backing on the explain capability; daemon path onASYNC_DAEMON, direct session-log write onASYNC_LOCALsupervisor/operator/tui.py— tracks the last clarification answer; onescalation_recommended=Trueshows the hint and bindsEto escalatesupervisor/operator/command_dispatch.py—/escalate <run_id> [question]; with no override pulls question + confidence from the most recentclarification_responseeventpyproject.tomlbumped to0.3.7; CHANGELOG + README updatedTest plan
pytest -q— 1229 passedpython -m supervisor.app clarify --helprenders the new CLI parserthin-supervisor clarify <run_id> "why paused?" --escalateagainst a completed local run writes the expected event to.supervisor/runtime/runs/<run_id>/session_log.jsonlc→ low-confidence answer surfaces theEhint →Erecords the eventSummary by CodeRabbit
New Features
clarifyCLI command with optional escalation flag/escalatecommand for operator workflows