feat(experts): kb.experts — rank entities by evidence density on a topic (closes #315)#347
feat(experts): kb.experts — rank entities by evidence density on a topic (closes #315)#347e11734937-beep wants to merge 8 commits into
Conversation
📝 WalkthroughWalkthroughAdds a read-only Changeskb.experts ranking feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as vouch experts
participant MCP as kb_experts
participant JSONL as kb.experts handler
participant Experts as rank_experts
participant Store as KBStore
CLI->>Experts: rank_experts(store, topic, limit, min_claims, weight)
MCP->>Experts: rank_experts(store, topic, limit, min_claims, weight)
JSONL->>Experts: rank_experts(store, topic, limit, min_claims, weight)
Experts->>Store: query matching claims and entities
Store-->>Experts: matched claims and entities
Experts-->>CLI: ranked expert rows
Experts-->>MCP: ranked expert rows
Experts-->>JSONL: ranked expert rows
Related issues: Suggested labels: enhancement, feature Suggested reviewers: vouchdev-maintainers 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/vouch/experts.py (1)
17-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReaching into a private helper across module boundaries.
_substring_entity_idsis underscore-prefixed, signaling it's internal tosalience. Importing it directly fromexperts.pycouples the two modules to an unstable private API. Consider exposing a public wrapper (e.g.substring_entity_ids) insalience.pyif this matching logic is meant to be reused.🤖 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 `@src/vouch/experts.py` at line 17, The import in experts.py is reaching into the private helper _substring_entity_ids from salience.py, which couples modules to an internal API. Expose a public wrapper or renamed public function in salience.py, such as substring_entity_ids, and update experts.py to import and use that public symbol instead of the underscore-prefixed helper. Keep the matching logic in salience.py and route any cross-module reuse through the public entry point.
🤖 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/test_experts.py`:
- Around line 1-126: `tests/test_experts.py` only exercises `rank_experts`
directly; add coverage for the `kb.experts` JSONL request/response envelope so
both success and failure cases are validated. Introduce tests around the
`kb.experts` entrypoint that assert a request yields `{id, ok, result}` on
success and `{id, ok: false, error}` on failure, using the existing
`rank_experts`/`KBStore` setup to keep the assertions aligned with the current
ranking behavior.
---
Nitpick comments:
In `@src/vouch/experts.py`:
- Line 17: The import in experts.py is reaching into the private helper
_substring_entity_ids from salience.py, which couples modules to an internal
API. Expose a public wrapper or renamed public function in salience.py, such as
substring_entity_ids, and update experts.py to import and use that public symbol
instead of the underscore-prefixed helper. Keep the matching logic in
salience.py and route any cross-module reuse through the public entry point.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d8578c4d-65a1-4ccc-8e2e-e2065cfcd901
📒 Files selected for processing (6)
src/vouch/capabilities.pysrc/vouch/cli.pysrc/vouch/experts.pysrc/vouch/jsonl_server.pysrc/vouch/server.pytests/test_experts.py
release 1.2.0: fresh-install fixes + compile/company-brain feature set
release 1.2.1: container images + version self-report
docs: promote readme restructure to main
docs: promote contributing refresh to main
docs: promote vouch webapp guidance to main
docs: promote pages-usage guidance to main
Closes vouchdev#315. Add a read-only kb.experts query: given a free-text topic, rank the entities carrying the most matched evidence (count / recency / citation weightings) identically across mcp / jsonl / cli. Aggregates approved, live claims only — excludes superseded/archived/redacted so a non-live claim never inflates a score; no proposals, writes, network, or llm. Ranking lives in a new src/vouch/experts.py, wired at the four registration sites.
The suite exercised rank_experts() directly but not the kb.experts JSONL
entrypoint. Add two envelope tests mirroring tests/test_jsonl_server.py:
a well-formed request returns {id, ok, result} with the ranking under
result["experts"], and a request missing the required `topic` param returns
the {id, ok: false, error} failure envelope (code "missing_param").
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
81b2b55 to
e967470
Compare
|
Rebased onto latest |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_experts.py (1)
49-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo test coverage for
weight="recency".Tests cover
count,citation, and fallback-to-countfor unknown weights, butrecency(decay-weighted byupdated_at/last_confirmed_atper PR objectives) has no dedicated test. Consider adding a case that seeds claims with distinct timestamps and asserts recency ordering differs from plain count ordering.🤖 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/test_experts.py` around lines 49 - 127, Add a dedicated test for rank_experts with weight="recency" to cover the decay-based ranking behavior that is currently missing. In tests/test_experts.py, extend the existing rank_experts-related coverage by seeding claims with distinct updated_at and/or last_confirmed_at values so you can assert newer evidence ranks ahead of older evidence even when claim counts are the same or differ. Make the test clearly reference rank_experts, the recency weight branch, and the seeded timestamps so it validates the intended ordering rather than the count-based fallback.
🤖 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/test_experts.py`:
- Around line 49-127: Add a dedicated test for rank_experts with
weight="recency" to cover the decay-based ranking behavior that is currently
missing. In tests/test_experts.py, extend the existing rank_experts-related
coverage by seeding claims with distinct updated_at and/or last_confirmed_at
values so you can assert newer evidence ranks ahead of older evidence even when
claim counts are the same or differ. Make the test clearly reference
rank_experts, the recency weight branch, and the seeded timestamps so it
validates the intended ordering rather than the count-based fallback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f06a26ca-bd45-4fe2-8d45-89d619b210c0
📒 Files selected for processing (6)
src/vouch/capabilities.pysrc/vouch/cli.pysrc/vouch/experts.pysrc/vouch/jsonl_server.pysrc/vouch/server.pytests/test_experts.py
🚧 Files skipped from review as they are similar to previous changes (5)
- src/vouch/experts.py
- src/vouch/server.py
- src/vouch/cli.py
- src/vouch/jsonl_server.py
- src/vouch/capabilities.py
|
@e11734937-beep – For future reference, please ensure that PRs are raised against the test branch. And congratulations on your first contribution to Vouch – great to have you on board! 💪 |
|
Hello @plind-junior 👋 #346 — #347 — kb.experts (closes #315): There's absolutely no rush but I'm just flagging them in case they slipped the queue. I'm happy to rebase or adjust anything to comply with the project's conventions. Thank you for keeping vouch going! 🙏 |
Summary
Closes #315. Adds
kb.experts— a read-only query that answers "who/what does the kb know about X." Given a free-text topic, it ranks the entities carrying the most matched evidence, so an agent can pull the right write-ups or ask the right follow-up.Pure read: it aggregates approved, live claims and returns a ranking — no
propose_*, noapprove, no writes, no network, no LLM. The review gate is untouched by construction.Surface
vouch experts "<topic>" [--limit N] [--min-claims N] [--weight count|recency|citation] [--json]kb.experts(topic, limit=10, min_claims=1, weight="count"), wired at all four registration sites —server.py(kb_expertsMCP tool),jsonl_server.py(_h_experts+HANDLERS),capabilities.py(METHODS),cli.py(vouch experts).test_capabilitiesstays green.index_db.search) plus the substring pass on entity name/aliases (salience._substring_entity_ids); aggregate the entities referenced by the matched claims.count(matched-claim count),recency(half-life decay onlast_confirmed_at/updated_at),citation(distinct evidence ids × confidence). An unknown weight falls back tocount(never raises).[{entity_id, name, type, claim_count, citation_count, score, top_claim_ids}], ordered by descending score with a stable tie-break onentity_id.Scope & correctness
Ranking/status logic lives in a new dedicated
src/vouch/experts.py(notrecall.py, notstorage.pywhich stays pure I/O). Claims with statussuperseded/archived/redactedare excluded so a non-live claim never inflates a score (issue #78). Runs entirely against the local.vouch/kb; zero network, zero LLM.Tests
tests/test_experts.pycovers each weight mode, the status-exclusion filter,min_claims/limit, unknown-weight fallback, empty-kb / no-match, and the deterministic tie-break.tests/test_capabilities.pystays green withkb.expertspresent at all sites.Verification
python -m ruff check src tests— cleanpython -m mypy src— clean (0 errors, 80 files)python -m pytest— full suite green (7 new tests; no regressions)Summary by CodeRabbit
kb.experts).limit,min-claims, and scoringweight.