Skip to content

feat: add evidence-bounded playbook diagnosis - #472

Merged
yyiilluu merged 1 commit into
mainfrom
codex/review-recuris-for-learnings
Aug 28, 2026
Merged

feat: add evidence-bounded playbook diagnosis#472
yyiilluu merged 1 commit into
mainfrom
codex/review-recuris-for-learnings

Conversation

@yyiilluu

@yyiilluu yyiilluu commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Explain observed playbook failures in the existing impact evaluation call, without introducing another optimizer or requiring new customer telemetry.
  • Keep diagnosis optional so existing evaluation-version-2 results remain readable and reusable.
  • Make the additive SQLite migration safe when multiple processes initialize the same database.

Changes

  • Add the five diagnosis categories, reasons, and cited interaction IDs to playbook verdicts; profiles retain null diagnosis.
  • Record input completeness and a content/trigger digest, and downgrade invalid citations to unknown.
  • Activate the updated impact prompt and persist the new fields in SQLite.
  • Acquire BEGIN IMMEDIATE before schema inspection, and roll back failed column additions.

Test Plan

  • 45 evaluator, SQLite migration, and retrieved-learning storage contract tests passed.
  • The new two-process startup regression reproduced duplicate column name: diagnosis before the fix and passes afterward.
  • A second regression verifies rollback of a partial migration and a successful retry.
  • Ruff and changed-file Pyright checks passed.

Compatibility

No forced evaluation backfill or evaluation-version bump. The SQLite columns are added on startup, leaving historical rows intact.

Summary by CodeRabbit

  • New Features

    • Retrieved-learning evaluations now include playbook diagnosis, supporting interaction references, evaluated playbook versions, and evidence-completeness status.
    • Diagnoses are validated and limited to evidence from the evaluated session.
    • Evaluation transcripts indicate when content was truncated.
  • Bug Fixes

    • Invalid or incomplete diagnostic evidence is safely excluded instead of producing unsupported diagnoses.
    • Existing evaluation results remain compatible with the updated format.
  • Tests

    • Added coverage for diagnosis validation, evidence handling, persistence, migrations, and legacy results.

Preserve legacy verdicts and bind optional diagnosis to evaluated content. Serialize SQLite schema upgrades across processes and verify rollback and concurrent startup.
@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: bebb84b3-1c7e-47bb-bb42-e2d79dfbfd68

📥 Commits

Reviewing files that changed from the base of the PR and between 424bdfe and a68972c.

📒 Files selected for processing (12)
  • reflexio/models/api_schema/domain/entities.py
  • reflexio/models/api_schema/playbook_diagnosis.py
  • reflexio/server/prompt/prompt_bank/retrieved_learning_impact/v1.0.0.prompt.md
  • reflexio/server/prompt/prompt_bank/retrieved_learning_impact/v1.1.0.prompt.md
  • reflexio/server/services/agent_success_evaluation/components/retrieved_learning_evaluator.py
  • reflexio/server/services/storage/sqlite_storage/_base.py
  • reflexio/server/services/storage/sqlite_storage/playbook/_eval_results.py
  • reflexio/server/services/storage/storage_base/retrieved_learning_state.py
  • tests/server/services/agent_success_evaluation/test_retrieved_learning_evaluator.py
  • tests/server/services/storage/sqlite_storage/test_retrieved_learning_interaction_migration.py
  • tests/server/services/storage/test_storage_contract_retrieved_learning_evals.py
  • tests/server/services/test_prompt_model_mapping.py

Included review availability: 1 review is currently available. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour.


📝 Walkthrough

Walkthrough

The change adds validated playbook diagnosis to retrieved-learning evaluations. It tracks evidence completeness and playbook digests, updates the evaluation prompt, persists the new fields in SQLite, and adds migration, concurrency, rollback, and round-trip tests.

Changes

Playbook diagnosis evaluation

Layer / File(s) Summary
Diagnosis contract and evaluation flow
reflexio/models/api_schema/playbook_diagnosis.py, reflexio/models/api_schema/domain/entities.py, reflexio/server/prompt/prompt_bank/retrieved_learning_impact/*, reflexio/server/services/agent_success_evaluation/components/retrieved_learning_evaluator.py, reflexio/server/services/storage/storage_base/retrieved_learning_state.py
Adds the PlaybookDiagnosis contract and evaluation fields. The evaluator validates evidence citations, tracks transcript completeness, computes user-playbook digests, and selects prompt version v1.1.0.
SQLite schema and result persistence
reflexio/server/services/storage/sqlite_storage/_base.py, reflexio/server/services/storage/sqlite_storage/playbook/_eval_results.py
Adds an idempotent transactional migration. Evaluation inserts and result reconstruction now handle diagnosis data, digests, and evidence-completeness flags.
Evaluation and storage validation
tests/server/services/agent_success_evaluation/test_retrieved_learning_evaluator.py, tests/server/services/storage/sqlite_storage/test_retrieved_learning_interaction_migration.py, tests/server/services/storage/test_storage_contract_retrieved_learning_evals.py, tests/server/services/test_prompt_model_mapping.py
Tests diagnosis evidence handling, digest calculation, legacy compatibility, concurrent migrations, rollback behavior, persistence round trips, and prompt version mapping.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to a6897

The PR adds optional diagnosis data and concurrency-safe SQLite migration behavior without any supplied current-head merge-blocking risk; it is merge-ready after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant AgentSuccessEvaluation
  participant retrieved_learning_evaluator
  participant PlaybookDiagnosis
  participant SQLiteStorage
  participant retrieved_learning_evaluation
  AgentSuccessEvaluation->>retrieved_learning_evaluator: evaluate transcript and retrieved learning
  retrieved_learning_evaluator->>PlaybookDiagnosis: validate diagnosis and evidence IDs
  retrieved_learning_evaluator->>SQLiteStorage: persist diagnosis, digest, and completeness
  SQLiteStorage->>retrieved_learning_evaluation: insert evaluation row
  SQLiteStorage-->>AgentSuccessEvaluation: reconstruct evaluation result
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 10 files. (2 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding evidence-bounded playbook diagnosis.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 10 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/review-recuris-for-learnings

Warning

Your free Security trial is over. An organization admin can activate Security or dismiss this notice.


Comment @coderabbitai help to get the list of available commands.

@yyiilluu
yyiilluu merged commit 22a02f4 into main Aug 28, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant