feat: run shadow comparisons at publish time#307
Conversation
Decouple interaction-level shadow head-to-head judging from sampled session-level group evaluation. Add a bounded background worker, request-transcript prompt v1.1.0, storage capability checks, and focused regression coverage.
📝 WalkthroughWalkthroughShadow comparison verdict generation moves from session-level group evaluation to publish-time dispatch. A new v1.1.0 prompt, dispatcher module, and bounded background worker handle per-turn judging, replacing the removed dispatch logic in the group evaluation runner. Storage backends gain a capability probe, and GenerationService schedules the new flow. ChangesPublish-time shadow comparison feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant GenerationService
participant ShadowComparisonWorker
participant ShadowComparisonDispatcher
participant ShadowComparisonJudge
participant Storage
Client->>GenerationService: run() publish request
GenerationService->>GenerationService: store interactions
GenerationService->>GenerationService: _schedule_post_publish_evaluations(interactions)
alt interaction has shadow_content
GenerationService->>ShadowComparisonWorker: enqueue(ShadowComparisonJob)
end
GenerationService->>GenerationService: schedule group evaluation (no shadow dispatch)
ShadowComparisonWorker->>ShadowComparisonWorker: resolve get_reflexio(org_id)
ShadowComparisonWorker->>ShadowComparisonDispatcher: dispatch_shadow_comparison_judge(storage, llm_client, interactions)
ShadowComparisonDispatcher->>Storage: supports_shadow_comparison_verdicts()
Storage-->>ShadowComparisonDispatcher: True/False
alt supported and eligible turns found
ShadowComparisonDispatcher->>ShadowComparisonJudge: judge_turn(conversation_context)
ShadowComparisonJudge-->>ShadowComparisonDispatcher: verdict or None
ShadowComparisonDispatcher->>Storage: save_shadow_comparison_verdict(verdict)
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.
🧹 Nitpick comments (1)
tests/server/services/shadow_comparison/test_dispatcher.py (1)
69-273: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for
save_shadow_comparison_verdictfailure paths.The dispatcher handles two distinct save-failure branches that are not covered by any test:
NotImplementedErrorfromsave_shadow_comparison_verdict(dispatcher.py:86-92) — stops the batch and returns early.- Generic
Exceptionfromsave_shadow_comparison_verdict(dispatcher.py:93-98) — logs and continues to the next interaction.These branches affect batch-level behavior (stop vs. continue) and are easy to test with a
MagicMockwhosesave_shadow_comparison_verdictside_effect raises.🧪 Suggested test additions
+def test_dispatcher_stops_on_not_implemented_error(monkeypatch) -> None: + """NotImplementedError from save should stop the batch early.""" + storage = MagicMock() + storage.supports_shadow_comparison_verdicts.return_value = True + storage.save_shadow_comparison_verdict.side_effect = NotImplementedError + + monkeypatch.setattr( + "reflexio.server.services.shadow_comparison.judge." + "ShadowComparisonJudge.judge_turn", + lambda self, *, interaction, **_: _verdict(interaction), + ) + + dispatch_shadow_comparison_judge( + storage=storage, + interactions=[ + _interaction( + interaction_id=1, + role="assistant", + content="a", + shadow_content="s", + ), + _interaction( + interaction_id=2, + role="assistant", + content="b", + shadow_content="t", + ), + ], + session_id="s1", + agent_version="v1", + request_context=_request_context(), # type: ignore[arg-type] + llm_client=MagicMock(), + ) + + # First save raises NotImplementedError → batch stops, second turn never judged. + storage.save_shadow_comparison_verdict.assert_called_once() + + +def test_dispatcher_continues_after_save_failure(monkeypatch) -> None: + """Generic save exception should not abort the batch.""" + storage = MagicMock() + storage.supports_shadow_comparison_verdicts.return_value = True + saved: list = [] + + def _save(verdict): + if len(saved) == 0: + raise RuntimeError("db write failed") + saved.append(verdict) + + storage.save_shadow_comparison_verdict.side_effect = _save + + monkeypatch.setattr( + "reflexio.server.services.shadow_comparison.judge." + "ShadowComparisonJudge.judge_turn", + lambda self, *, interaction, **_: _verdict(interaction), + ) + + dispatch_shadow_comparison_judge( + storage=storage, + interactions=[ + _interaction( + interaction_id=1, + role="assistant", + content="a", + shadow_content="s", + ), + _interaction( + interaction_id=2, + role="assistant", + content="b", + shadow_content="t", + ), + ], + session_id="s1", + agent_version="v1", + request_context=_request_context(), # type: ignore[arg-type] + llm_client=MagicMock(), + ) + + # First save fails, second succeeds — both turns judged. + assert storage.save_shadow_comparison_verdict.call_count == 2 + assert len(saved) == 1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/server/services/shadow_comparison/test_dispatcher.py` around lines 69 - 273, Add coverage for the two save failure paths in dispatch_shadow_comparison_judge: one test where storage.save_shadow_comparison_verdict raises NotImplementedError and the dispatcher returns early without processing later interactions, and another where it raises a generic Exception and the dispatcher logs then continues to the next interaction. Use the existing dispatch_shadow_comparison_judge, _interaction, and MagicMock-based storage setup in test_dispatcher.py, and assert the batch behavior changes (stop vs continue) via save call counts and judged interactions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/server/services/shadow_comparison/test_dispatcher.py`:
- Around line 69-273: Add coverage for the two save failure paths in
dispatch_shadow_comparison_judge: one test where
storage.save_shadow_comparison_verdict raises NotImplementedError and the
dispatcher returns early without processing later interactions, and another
where it raises a generic Exception and the dispatcher logs then continues to
the next interaction. Use the existing dispatch_shadow_comparison_judge,
_interaction, and MagicMock-based storage setup in test_dispatcher.py, and
assert the batch behavior changes (stop vs continue) via save call counts and
judged interactions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a8423d8f-2d32-41cd-8c64-3247e5c2ab73
📒 Files selected for processing (18)
reflexio/models/config_schema.pyreflexio/server/README.mdreflexio/server/prompt/prompt_bank/shadow_comparison/v1.0.0.prompt.mdreflexio/server/prompt/prompt_bank/shadow_comparison/v1.1.0.prompt.mdreflexio/server/services/agent_success_evaluation/README.mdreflexio/server/services/agent_success_evaluation/runner.pyreflexio/server/services/generation_service.pyreflexio/server/services/shadow_comparison/dispatcher.pyreflexio/server/services/shadow_comparison/judge.pyreflexio/server/services/shadow_comparison/worker.pyreflexio/server/services/storage/sqlite_storage/_shadow_verdicts.pyreflexio/server/services/storage/storage_base/_shadow_verdicts.pytests/models/test_config_f1_fields.pytests/server/prompt/test_shadow_comparison_prompt_present.pytests/server/services/agent_success_evaluation/test_group_evaluation_shadow_integration.pytests/server/services/shadow_comparison/test_dispatcher.pytests/server/services/test_generation_service_scheduling.pytests/server/services/test_prompt_model_mapping.py
💤 Files with no reviewable changes (1)
- reflexio/server/services/agent_success_evaluation/runner.py
Summary
shadow_content.v1.1.0to include request-local transcript context and intentionally start a new dashboard epoch.Changes
shadow_comparison.dispatcherand a bounded background worker for immediate best-effort per-request judging.GenerationService._schedule_post_publish_evaluations(...), including the learning-stall path.run_group_evaluation(...)so regen/session evaluation no longer duplicates verdict writes.shadow_comparison/v1.1.0, deactivatedv1.0.0, and updated tests/docs.Test Plan
uv run pytest open_source/reflexio/tests/server/services/test_generation_service_scheduling.py -o 'addopts=' -vuv run pytest open_source/reflexio/tests/server/services/shadow_comparison open_source/reflexio/tests/server/services/agent_success_evaluation/test_group_evaluation_shadow_integration.py -o 'addopts=' -vuv run pytest open_source/reflexio/tests/models/test_config_f1_fields.py open_source/reflexio/tests/server/prompt/test_shadow_comparison_prompt_present.py open_source/reflexio/tests/server/services/test_prompt_model_mapping.py -o 'addopts=' -vuv run ruff check ...anduv run pyright ...on staged Python fileshttp://localhost:18091with MiniMax and Supabase: verified zero-sampling shadow verdict creation, no-shadow no-op, and sampled group evaluation + shadow verdict rows.Summary by CodeRabbit
New Features
Bug Fixes
Documentation