fix(evaluation): keep verdicts through learning merges

[wenchanghan]

Why

A retrieved-learning link records the ID that was shown to the agent. Reflexio may merge or supersede that learning before someone runs the on-demand evaluation. The evaluator currently looks up only the original ID, so a valid learning can disappear from a regenerated evaluation simply because the library consolidated it.

This especially undercounts actively maintained learnings. In one opt-in development database, there are already 45 user-playbook merge events and 50 user-playbook rows with a successor pointer.

Before / changed / after

	Behavior
Before	Evaluation looks up the recorded ID directly. If that row has merged, the learning is skipped.
Changed	Evaluation follows Reflexio's existing lineage pointers to the current live learning, then applies the normal owner, status, expiration, and approval checks.
After	The current learning is judged once under its current ID and content, even when several recorded IDs now lead to it.

flowchart LR
    A["Recorded learning ID"] --> B["Follow existing lineage"]
    B --> C{"Current live learning?"}
    C -->|"yes"| D["Apply normal eligibility rules"]
    D --> E["Judge current ID + content once"]
    C -->|"missing or purged"| F["Skip and count the reason"]

Loading

Example: if an interaction recorded user playbook 12, and 12 later merged into 31, the evaluator judges playbook 31 and stores the verdict under 31. It never judges blanked content, and two old IDs that both lead to 31 produce one candidate.

Safety and observability

• Existing lineage is the only source of truth; this adds no table or parallel linking mechanism.
• Missing, purged, and ineligible survivors are skipped with separate diagnostics. Remapped IDs are counted as resolved_via_lineage.
• Commit-time attachment checks use the same lineage equivalence, so a valid survivor result is not dropped merely because the interaction stored an older ID.
• If a candidate merges, is purged, or becomes unresolvable while its LLM judgment is running, the commit returns stale without replacing prior results. The next forced run can evaluate current state instead of caching a false not_applicable outcome.
• This work runs only in explicit/on-demand evaluation. It adds no database work to interaction publishing, normal learning, or retrieval.

Boundaries

• Reflexio follows explicit pointers only. A retired record with no successor cannot be inferred safely and remains an observable skip.
• An already cached terminal evaluation is not automatically invalidated by a later library merge. A forced regeneration, or the next evaluation after the existing cache window, recomputes against current lineage.

Verification

• Focused evaluator, runner-integration, and storage-contract tests: 35 passed.
• Repository-wide coverage in isolation-safe invocations: 4,745 passed, 124 skipped. The existing module-contract test was run separately because it intentionally removes an imported module from sys.modules.
• Ruff: passed.
• Pyright: passed.
• git diff --check: passed.

[coderabbitai]

📝 Walkthrough

Walkthrough

Retrieved-learning evaluation now resolves merged, superseded, and purged references through lineage. Candidate evaluation uses survivor records, while result commits reject lineage changes detected during evaluation. Tests cover evaluator behavior, forced regeneration, and stale commit races.

Changes

Retrieved-learning lineage handling

Layer / File(s)	Summary
Lineage-aware candidate evaluation reflexio/models/api_schema/domain/entities.py, reflexio/server/services/agent_success_evaluation/components/retrieved_learning_evaluator.py, tests/server/services/agent_success_evaluation/test_retrieved_learning_evaluator.py	Candidate references resolve to current entities, duplicate survivors are collapsed, purged and ineligible references are counted, and prompts use survivor content.
Lineage-aware result commit fencing reflexio/server/services/storage/sqlite_storage/playbook/_eval_results.py, reflexio/server/services/storage/storage_base/playbook/_eval_results.py, tests/server/services/storage/test_storage_contract_retrieved_learning_evals.py	Session attachments and proposed results are checked against current lineage identities; commits become stale when merges occur during evaluation.
Merge and regeneration integration tests/server/services/agent_success_evaluation/test_retrieved_learning_runner_integration.py	Forced regeneration after a playbook merge produces results for only the surviving learning and restores complete status.

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

Sequence Diagram(s)

sequenceDiagram
  participant Session as Evaluation Session
  participant Evaluator as RetrievedLearningEvaluator
  participant Lineage as resolve_current
  participant LLM
  participant Store as replace_retrieved_learning_evaluation_results

  Session->>Evaluator: Load attached learning references
  Evaluator->>Lineage: Resolve references to current identities
  Lineage-->>Evaluator: Return survivor or purged status
  Evaluator->>LLM: Evaluate eligible survivor content
  LLM-->>Evaluator: Return evaluation results
  Evaluator->>Store: Commit proposed results
  Store->>Lineage: Recheck proposed identities
  Lineage-->>Store: Confirm current or changed lineage
  Store-->>Session: Return complete or stale disposition

Loading

Possibly related PRs

• ReflexioAI/reflexio#329: Modifies the retrieved-learning evaluation pipeline and SQLite result replacement with lineage-aware behavior.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: preserving evaluation verdicts as learnings merge or are superseded.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

reflexio/server/services/storage/storage_base/playbook/_eval_results.py (1)

224-230: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Document the lineage-change stale path in the abstract contract.

The SQLite implementation returns stale before filtering when a proposed identity resolves elsewhere or becomes purged, preserving the prior result set. This doc currently only says failed rows are dropped and the prior set is replaced, which could lead another backend to implement the fencing contract incorrectly.

Suggested clarification

         recomputes the session fingerprint from live rows and requires it to
         equal ``session_fingerprint`` (else ``stale``), requires every result
         to match a session attachment after lineage resolution, and rechecks
         its source row for retrieval eligibility. Rows that fail either check
         are dropped before the prior session set is atomically replaced.
+        If a proposed identity no longer resolves to itself or is purged,
+        the method returns ``stale`` without replacing the prior result set.

🤖 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/storage/storage_base/playbook/_eval_results.py`
around lines 224 - 230, Clarify the abstract contract around the
lineage-resolution and purge checks in the documented transaction behavior: when
a proposed identity resolves elsewhere or is purged, return the `stale` outcome
before filtering and preserve the prior session result set. Retain the existing
row-filtering and atomic replacement behavior for results that pass the lineage
and retrieval-eligibility checks.

🤖 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 `@reflexio/server/services/storage/sqlite_storage/playbook/_eval_results.py`:
- Around line 282-293: Update _rle_lineage_changed so an unresolvable reference
is treated as stale: return True when resolve_current returns None, while
preserving the existing stale checks for purged records or changed IDs and
continuing to ignore only the specified resolution exceptions. Add a regression
test covering deletion or a malformed lineage during evaluation.

---

Nitpick comments:
In `@reflexio/server/services/storage/storage_base/playbook/_eval_results.py`:
- Around line 224-230: Clarify the abstract contract around the
lineage-resolution and purge checks in the documented transaction behavior: when
a proposed identity resolves elsewhere or is purged, return the `stale` outcome
before filtering and preserve the prior session result set. Retain the existing
row-filtering and atomic replacement behavior for results that pass the lineage
and retrieval-eligibility checks.

🪄 Autofix (Beta)

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 Plus

Run ID: 348c08cc-fcfe-465b-9d13-1cce8f2ec2b5

📥 Commits

Reviewing files that changed from the base of the PR and between 66fccb7 and 4f25095.

📒 Files selected for processing (7)

• reflexio/models/api_schema/domain/entities.py
• reflexio/server/services/agent_success_evaluation/components/retrieved_learning_evaluator.py
• reflexio/server/services/storage/sqlite_storage/playbook/_eval_results.py
• reflexio/server/services/storage/storage_base/playbook/_eval_results.py
• tests/server/services/agent_success_evaluation/test_retrieved_learning_evaluator.py
• tests/server/services/agent_success_evaluation/test_retrieved_learning_runner_integration.py
• tests/server/services/storage/test_storage_contract_retrieved_learning_evals.py

[wenchanghan]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[wenchanghan]

Closing — Yi will handle the fix upstream (evaluate tombstoned rows instead of skipping them).