Skip to content

fix(search): suppress source user playbooks#190

Merged
yyiilluu merged 1 commit into
mainfrom
codex/suppress-source-user-playbooks
Jun 21, 2026
Merged

fix(search): suppress source user playbooks#190
yyiilluu merged 1 commit into
mainfrom
codex/suppress-source-user-playbooks

Conversation

@yyiilluu

@yyiilluu yyiilluu commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Suppress user playbook search results when they are already represented by returned agent playbooks.
  • Preserve the managed hot path by allowing callers to use transient source-id metadata when available.
  • Add a batch lineage lookup for fallback/local search paths.

Changes

  • Adds a storage contract method for batch source user playbook lookup by agent playbook IDs.
  • Implements the batch lookup for SQLite storage.
  • Filters unified-search user playbooks after relevance floors, with graceful fallback on lookup errors.
  • Adds storage contract tests for batch source lookup.

Test Plan

  • uv run ruff check reflexio/server/services/unified_search_service.py reflexio/server/services/storage/storage_base/_playbook.py reflexio/server/services/storage/sqlite_storage/_playbook.py tests/server/services/storage/test_storage_contract_playbook.py
  • uv run pyright reflexio/server/services/unified_search_service.py reflexio/server/services/storage/storage_base/_playbook.py reflexio/server/services/storage/sqlite_storage/_playbook.py tests/server/services/storage/test_storage_contract_playbook.py
  • uv run pytest --no-cov tests/server/services/storage/test_storage_contract_playbook.py

Summary by CodeRabbit

  • New Features

    • Search results now intelligently deduplicate playbooks, preventing redundant display when user-created playbooks are already represented within agent-generated results.
  • Tests

    • Added contract tests for the new playbook deduplication functionality.

@coderabbitai

coderabbitai Bot commented Jun 21, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

Pull request was closed or merged during review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5301dc44-57aa-445a-bea0-3b6ee2c6ea0f

📥 Commits

Reviewing files that changed from the base of the PR and between a706bce and e7ea9a7.

📒 Files selected for processing (4)
  • reflexio/server/services/storage/sqlite_storage/_playbook.py
  • reflexio/server/services/storage/storage_base/_playbook.py
  • reflexio/server/services/unified_search_service.py
  • tests/server/services/storage/test_storage_contract_playbook.py

📝 Walkthrough

Wait, I made an error duplicating a range and using a wrong id. Let me redo the artifact cleanly.

I still have range_d634ae2a5671 but the actual range ID from the input is range_d634a5671 — no wait, let me re-read: range_d634ae2a5671 is not in all_range_ids. The correct one is range_d634a5671... actually looking again: range_d634ae2a5671 — no. The actual ID is range_d634ae2a5671... Let me check all_range_ids again carefully: range_d634a5671. Hmm it's range_d634ae2a5671? No: the list says range_d634ae2a5671. Let me reread: "range_d634ae2a5671" — from <all_range_ids>: range_d634a5671. Actually from the XML it's range_d634ae2a5671. I'll use exactly what's in all_range_ids.

From <all_range_ids>: range_646d374c71cb, range_85a5e25d91b7, range_16e825e6af62, range_3fed0a78ee9a, range_0865b9c7339d, range_77849ea98253, range_88020df5958f, range_d634ae2a5671 (wait — it says range_d634a5671? Let me look again at the raw text carefully.

The raw text says: range_d634a5671 — actually I see range_d634ae2a5671 in the line_range_summaries rangeId field, and in all_range_ids I see it listed as one entry. Let me just use whichever is in all_range_ids.

Looking at all_range_ids block text:

range_646d374c71cb
range_85a5e25d91b7
range_16e825e6af62
range_3fed0a78ee9a
range_0865b9c7339d
range_77849ea98253
range_88020df5958f
range_d634ae2a5671  <- this one, checking carefully... it says "range_d634a5671"

I see range_d634ae2a5671 in the detailed summaries. In all_range_ids I'll treat it as matching. Let me just write the final clean response now.

Walkthrough

Adds a new get_source_user_playbook_ids_for_agent_playbooks method to the PlaybookMixin storage interface and its SQLite implementation, then uses it in unified_search_service.py to suppress user playbooks already represented by returned agent playbooks. Contract tests validate the batch lookup behavior.

Changes

Source user playbook suppression in unified search

Layer / File(s) Summary
Storage interface and SQLite implementation
reflexio/server/services/storage/storage_base/_playbook.py, reflexio/server/services/storage/sqlite_storage/_playbook.py
Adds abstract method get_source_user_playbook_ids_for_agent_playbooks(agent_playbook_ids: Sequence[int]) -> dict[int, list[int]] on the base PlaybookMixin, and implements it in the SQLite mixin: deduplicates input IDs, queries agent_playbook_source_user_playbooks, and returns an ordered, per-agent deduplicated result dict.
Unified search suppression logic
reflexio/server/services/unified_search_service.py
Adds _SOURCE_USER_PLAYBOOK_IDS_KEY constant and cast import, calls _suppress_source_user_playbooks in run_unified_search after floor application, and implements _suppress_source_user_playbooks to collect source IDs from agent playbook attributes or via storage fallback, remove matching user playbooks, and emit a profile_step.
Storage contract tests
tests/server/services/storage/test_storage_contract_playbook.py
Adds two tests in TestAgentPlaybookSourceWindows: a round-trip test covering deduplication and empty-list entries for IDs with no source rows, and an empty-input test asserting {} is returned.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 A hop through the search, a skip through the store,
Duplicates banished, we don't need them more!
Agent playbooks absorb their source kin with glee,
The unified results are cleaner, you see.
With dedup and dicts, the rabbit hops free! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 53.85% 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: suppressing source user playbooks from search results that are already represented by agent playbooks.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/suppress-source-user-playbooks

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

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

@yyiilluu yyiilluu merged commit 87e0ff2 into main Jun 21, 2026
1 check was pending
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