fix: make durable playbook recovery deterministic - #443
Merged
Conversation
Reconstruct review evidence from the extraction run's persisted window, require an unambiguous user owner, and discover resumable work from the authoritative provider before relying on a bootstrap org. Preserve exception metadata only when it belongs to the current LiteLLM failure.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR strengthens LLM transient-error classification, makes organization discovery authoritative during scheduler ticks, enforces durable run ownership, and reconstructs playbook review windows from persisted interaction provenance. ChangesRecovery and review workflows
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ResumeWorker
participant PlaybookGenerationService
participant review_window
participant BaseStorage
ResumeWorker->>PlaybookGenerationService: provide extraction run and user_id
PlaybookGenerationService->>review_window: reconstruct source_interaction_ids
review_window->>BaseStorage: load persisted interactions and requests
BaseStorage-->>review_window: return validated evidence
review_window-->>PlaybookGenerationService: return ordered review window
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
reflexio/server/services/playbook/review_window.py (1)
31-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared load-and-validate block.
Lines 31-50 and 106-125 are identical. Both functions dedupe the IDs, reject an empty list, load the interactions, and reject missing IDs. A shared helper keeps the two fail-closed paths from drifting apart later.
♻️ Proposed helper extraction
+def _load_source_interactions( + *, + storage: BaseStorage, + source_interaction_ids: Sequence[int], + subject: str, +) -> dict[int, Interaction]: + """Load every cited interaction or fail closed.""" + source_ids = list(dict.fromkeys(source_interaction_ids)) + if not source_ids: + raise PlaybookReviewWindowError( + f"{subject} has no complete generation-window provenance" + ) + interactions_by_id = { + interaction.interaction_id: interaction + for interaction in storage.get_interactions_by_ids(source_ids) + } + missing_interaction_ids = [ + interaction_id + for interaction_id in source_ids + if interaction_id not in interactions_by_id + ] + if missing_interaction_ids: + raise PlaybookReviewWindowError( + f"{subject} is missing persisted generation-window interactions: " + f"{missing_interaction_ids}" + ) + return interactions_by_idThen both functions call it:
- source_ids = list(dict.fromkeys(source_interaction_ids)) - if not source_ids: - raise PlaybookReviewWindowError( - f"{subject} has no complete generation-window provenance" - ) - - interactions_by_id = { - interaction.interaction_id: interaction - for interaction in storage.get_interactions_by_ids(source_ids) - } - missing_interaction_ids = [ - interaction_id - for interaction_id in source_ids - if interaction_id not in interactions_by_id - ] - if missing_interaction_ids: - raise PlaybookReviewWindowError( - f"{subject} is missing persisted generation-window interactions: " - f"{missing_interaction_ids}" - ) + interactions_by_id = _load_source_interactions( + storage=storage, + source_interaction_ids=source_interaction_ids, + subject=subject, + )Also applies to: 106-125
🤖 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 `@reflexio/server/services/playbook/review_window.py` around lines 31 - 50, Extract the duplicated source-ID deduplication and interaction loading/validation logic into a shared helper in review_window.py. Have both affected functions call the helper, preserving the existing empty-source and missing-interaction PlaybookReviewWindowError messages and fail-closed behavior.
🤖 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.
Inline comments:
In `@tests/server/services/extraction/test_resume_worker.py`:
- Line 206: Update the pytest.raises call’s match pattern in the resume-worker
test to use a raw string literal, preserving the existing regular expression and
expected ValueError assertion.
---
Nitpick comments:
In `@reflexio/server/services/playbook/review_window.py`:
- Around line 31-50: Extract the duplicated source-ID deduplication and
interaction loading/validation logic into a shared helper in review_window.py.
Have both affected functions call the helper, preserving the existing
empty-source and missing-interaction PlaybookReviewWindowError messages and
fail-closed behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d54be0fc-8bb8-40eb-8ece-68589a407885
📒 Files selected for processing (22)
reflexio/server/api.pyreflexio/server/llm/_litellm_text_generation.pyreflexio/server/llm/_litellm_types.pyreflexio/server/services/extraction/README.mdreflexio/server/services/extraction/agent_run_records.pyreflexio/server/services/extraction/resumable_agent.pyreflexio/server/services/extraction/resume_scheduler.pyreflexio/server/services/extraction/resume_worker.pyreflexio/server/services/playbook/README.mdreflexio/server/services/playbook/playbook_service_utils.pyreflexio/server/services/playbook/review_service.pyreflexio/server/services/playbook/review_window.pyreflexio/server/services/playbook/service.pytests/eval/extraction/providers.pytests/server/llm/test_litellm_client_unit.pytests/server/services/extraction/test_resume_scheduler.pytests/server/services/extraction/test_resume_worker.pytests/server/services/playbook/test_extractor_polarity_integration.pytests/server/services/playbook/test_playbook_extractor.pytests/server/services/playbook/test_playbook_generation_service.pytests/server/services/playbook/test_playbook_generation_service_integration.pytests/server/services/storage/test_agent_run_helpers.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changes
Playbook review and ownership
user_idvalues for new runs and fail closed on missing, cross-user, or ambiguous evidence.Resume scheduling
Error classification
Documentation and tests
Test Plan
uv run pytest tests/ --ignore=tests/e2e_tests/ -o 'addopts=' -q— 5,665 passed, 13 skippeduv run pytest tests/e2e_tests/ -o 'addopts=' -q— 47 passed, 87 skippeduv run ruff check reflexio testsuv run ruff format --check reflexio testsuv run pyright --threads 4— 0 errorsSummary by CodeRabbit
Bug Fixes
Documentation