Review: Hybrid search -- RRF blending + semantic search (Round 2)

review-1570-2026-06-22-r2 Review

review ready

Project

Verdict: APPROVED

Round 2 review. AC3 replaced with concrete test fixture assertion, neighbor gem dependency made explicit. All sections present, traceability complete, dependencies correctly documented. Scope is solid for a 3-point ticket.

Template Completeness

[x] Type -- Feature
[x] Lineage -- decomposed from paldocs#52, depends on #52 and #59
[x] Repo -- ldraney/paldocs
[x] User Story -- MCP layer needs Rails semantic/hybrid search
[x] Context -- explains relationship to #52 (embeddings) and #59 (stub)
[x] File Targets -- 2 modify/create, 3 do-not-touch boundaries
[x] Feature Flag -- none, replaces stub
[x] Acceptance Criteria -- 3 criteria, concrete and testable
[x] Test Expectations -- unit + 2 integration, run command specified
[x] Constraints -- RRF formula, neighbor gem usage, tsvector for keyword
[x] Checklist
[x] Related -- project, story, arch note, upstream/downstream issues

Traceability

[x] story:consolidation label -- story-paldocs-consolidation
[x] story note verified -- found in project-paldocs user-stories section
[x] arch:rails-api label -- rails API surface
[x] arch note verified -- arch-rails-api note exists in pal-e-docs (confirmed via get_note)
[x] Forgejo issue -- ldraney/paldocs#61, open

File Targets

[x] app/services/hybrid_search.rb -- to be created. app/services/ dir does not exist yet but will be created by agent. Standard Rails convention.
[x] app/controllers/api/search_controller.rb -- to be created. app/controllers/api/ dir does not exist yet; will be created by upstream #59 (search stub). Ticket correctly states this dependency.
[x] Do-not-touch boundaries are clear: block.rb, app/jobs/, db/migrate/ all correctly excluded

Repo Placement

OK. Issue filed on ldraney/paldocs, work targets ldraney/paldocs. Single-repo scope.

Dependencies

paldocs#52 (Embedding pipeline) -- in todo (board item #1562). Adds neighbor gem to Gemfile and creates blocks.embedding column. HARD PREREQUISITE. Ticket explicitly declares this dependency.
paldocs#59 (Search controller stub) -- in todo (board item #1569). Creates Api::SearchController with 501 stub for semantic endpoint. HARD PREREQUISITE. Ticket explicitly declares this dependency and says it replaces the stub.
paldocs#49 (Schema ownership) -- in in_progress (board item #1559). Transfers schema to Rails structure.sql. Transitive dependency via #52.
Downstream: paldocs#54 (MCP server) consumes these search endpoints.
All dependencies are documented in the Lineage and Constraints sections. Wave ordering must ensure #49 -> #52 -> #59 -> #61.

Acceptance Criteria

[x] AC1: GET /api/search/semantic?q=kubernetes returns blocks ranked by cosine similarity -- agent-verifiable with integration test
[x] AC2: GET /api/search/hybrid?q=kubernetes returns RRF-blended results -- agent-verifiable with integration test
[x] AC3: HybridSearch.rrf(keyword_results, semantic_results) with known test fixtures returns expected order -- concrete unit test assertion with known inputs, agent-verifiable
All 3 criteria are testable. AC3 was improved in Round 2 to specify test fixtures rather than abstract assertion.

Blast Radius

Low. This creates new files (service + controller actions) and does not modify existing code. The search controller stub from #59 will be replaced, which is the documented intent. No existing routes, models, or views are affected. The neighbor gem integration (nearest_neighbors call) is scoped to this service class only.

Decomposition Assessment

2 file targets in 1 repo. 3 acceptance criteria. Estimated agent work under 5 minutes. No decomposition needed.

Recommendation

No action needed. Ticket is well-scoped, dependencies are explicit, AC is concrete and testable. Ready for implementation once upstream tickets #52 and #59 are complete.