feat: operator channel polish — 0.3.6#91
Conversation
- deep_explainer_model (+ temperature/max_tokens) opts drift assessment into a heavier reasoner while routine explanations stay on explainer_model. Falls back to the routine model, then the structured stub, when the deep model is unset or the call fails. - DriftAssessment and ExchangeView frozen dataclasses (from_dict accepts legacy last_*_summary and post-explainer shapes). Unknown drift status collapses to "watch"; non-numeric confidence collapses to None. - Dedicated explainer_answer session-log event with source="explainer", emitted alongside the existing clarification_response so future worker-side clarification flows can be distinguished without touching existing consumers. - Confidence-gated escalation: when answer confidence < clarification_escalation_confidence (default 0.4), the result carries escalation_recommended=True and a clarification_escalation_recommended event is written. Advisory only — operator still routes.
|
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 50 minutes and 3 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 (11)
✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request upgrades the supervisor to version 0.3.6, introducing a tiered LLM explainer system and new frozen dataclasses for structured drift assessment and exchange views. It also adds a confidence-gated escalation mechanism for operator clarifications. Review feedback suggests refactoring duplicated logic found in the daemon and operator modules—specifically for confidence value parsing and the processing of clarification results—into shared utility functions to ensure consistency.
| conf = data.get("confidence") | ||
| if conf is not None: | ||
| try: | ||
| conf = float(conf) | ||
| except (TypeError, ValueError): | ||
| conf = None |
There was a problem hiding this comment.
The logic for parsing and coercing the confidence value is duplicated in ExchangeView.from_dict and DriftAssessment.from_dict (lines 287-292), as well as in server.py and actions.py. Consider extracting this into a shared utility function to ensure consistent behavior and reduce boilerplate.
| confidence = result.get("confidence") | ||
| try: | ||
| conf_value = float(confidence) if confidence is not None else None | ||
| except (TypeError, ValueError): | ||
| conf_value = None | ||
| escalate = conf_value is not None and conf_value < escalation_threshold | ||
| result["escalation_recommended"] = escalate | ||
|
|
||
| _write_event("explainer_answer", { | ||
| "source": "explainer", | ||
| "question": question, | ||
| "answer": result.get("answer", ""), | ||
| "confidence": confidence, | ||
| }) | ||
| _write_event("clarification_response", { | ||
| "question": question, | ||
| "answer": result.get("answer", ""), | ||
| "confidence": result.get("confidence"), | ||
| "confidence": confidence, | ||
| "source": "explainer", | ||
| "escalation_recommended": escalate, | ||
| }) | ||
| if escalate: | ||
| _write_event("clarification_escalation_recommended", { | ||
| "question": question, | ||
| "confidence": confidence, | ||
| "threshold": escalation_threshold, | ||
| }) |
There was a problem hiding this comment.
This block of logic for processing the clarification result and emitting timeline events is identical to the implementation in supervisor/operator/actions.py (lines 419-461). Duplicating this business logic (including the escalation threshold check and the specific event payloads) increases the risk of inconsistencies when the protocol or requirements change. Consider refactoring this into a shared helper function.
| confidence = result.get("confidence") | ||
| try: | ||
| conf_value = float(confidence) if confidence is not None else None | ||
| except (TypeError, ValueError): | ||
| conf_value = None | ||
| escalate = conf_value is not None and conf_value < escalation_threshold | ||
| result["escalation_recommended"] = escalate | ||
|
|
||
| # Emit explainer_answer — explicit source marker distinguishes | ||
| # this from a future worker-side response. | ||
| append_timeline_event( | ||
| ctx.session_log_path, ctx.run_id, | ||
| "explainer_answer", | ||
| { | ||
| "source": "explainer", | ||
| "question": question, | ||
| "answer": result.get("answer", ""), | ||
| "confidence": confidence, | ||
| }, | ||
| ) | ||
| # Record the answer in the timeline (legacy event kept for | ||
| # backward compatibility with existing consumers). | ||
| append_timeline_event( | ||
| ctx.session_log_path, ctx.run_id, | ||
| "clarification_response", | ||
| { | ||
| "question": question, | ||
| "answer": result.get("answer", ""), | ||
| "confidence": result.get("confidence"), | ||
| "confidence": confidence, | ||
| "source": "explainer", | ||
| "escalation_recommended": escalate, | ||
| }, | ||
| ) | ||
| if escalate: | ||
| append_timeline_event( | ||
| ctx.session_log_path, ctx.run_id, | ||
| "clarification_escalation_recommended", | ||
| { | ||
| "question": question, | ||
| "confidence": confidence, | ||
| "threshold": escalation_threshold, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
This block of logic for processing the clarification result and emitting timeline events is identical to the implementation in supervisor/daemon/server.py (lines 997-1023). Duplicating this business logic (including the escalation threshold check and the specific event payloads) increases the risk of inconsistencies when the protocol or requirements change. Consider refactoring this into a shared helper function.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Fix Devin finding: assess_drift used `is not None` on deep_model while line 118 used a falsy check, so an empty-string SUPERVISOR_DEEP_EXPLAINER_MODEL env var fell into the deep branch and kept deep_temperature/deep_max_tokens even though the effective model dropped back to the routine one. Switch to a consistent falsy check so empty-string is treated as unset. - Extract coerce_confidence() helper in operator.models — single source of truth for non-numeric → None coercion, now used by ExchangeView, DriftAssessment, and the clarification pipeline. - Extract finalize_clarification() helper in operator.clarification so the daemon and local-async paths share the same event sequence (explainer_answer + clarification_response + optional escalation) and escalation threshold semantics. Eliminates the two copies flagged by gemini-code-assist.
1 participant
Summary
deep_explainer_modelsoassess_driftcan opt into a heavier reasoner while routine explanations stay on the cheapexplainer_model(with graceful fallback to the routine model and then the stub).DriftAssessmentandExchangeViewdataclasses (supervisor.operator.models) withfrom_dict()that accepts both legacylast_*_summaryand post-explainer shapes — type-safe projections for TUI / IM channel adapters.explainer_answertimeline event taggedsource="explainer"alongside the existingclarification_response, and adds a confidence-gatedescalation_recommendedflag +clarification_escalation_recommendedevent when answer confidence falls belowclarification_escalation_confidence(default0.4). Escalation is advisory — the operator still chooses whether to route to the worker.Test plan
pytest tests/test_operator_models.py(new — 13 tests)pytest tests/test_operator_actions.py(existing test updated to assert 4-event sequence +escalation_recommended)pytest— 1216 passedthin-supervisor inspectagainst a live run and confirm clarification now surfacesescalation_recommended(after merge / tag)