fix(retrieval): always return top hit + skip-and-continue on token budget overflow (B-MCP-8)

[h2devx]

Summary

Closes #31. Fixes B-MCP-8 (medium, discovered in Phase-14 smoke against the dogfood DB): mem.recall returned total_candidates>0 but hits=0 whenever the top-ranked candidate alone exceeded max_tokens.

The old loop in RecallMemoryUseCase.rankAndSlice did if (runningTokens + tokens > max) break from the first iteration, leaving out=[]. The dogfood DB tripped this because learning rows store full content un-truncated (decisions and entities truncate at 600 chars; learnings and turns do not), so a single long learning could cost more tokens than the 4000-token default budget.

Why it surfaced now (beta.4) and not before (beta.3)

Pre-B-MCP-7 (beta.3), the embedding worker never drained the queue, so vector hits were always empty and recall fell back to BM25-only. BM25 prioritized entries with exact lexical matches — typically short titles. With B-MCP-7 fixed in beta.4, the worker now populates embeddings end-to-end, and the hybrid scorer began ranking long-form learnings higher (because their semantic vectors matched). Those long learnings then triggered the budget-overflow break.

Fix

Three changes:

1. Always include the top-ranked hit, even if it solo exceeds the budget. Returning zero hits when there ARE candidates surprises callers and degrades the semantic-recall promise; one slightly-oversized result is strictly more useful than no result.
2. Use continue (not break) on overflow for subsequent hits, so a mid-loop oversized candidate doesn't suppress smaller hits behind it in the ranking.
3. Bump RecallMemoryFacadeAdapter.DEFAULT_MAX_TOKENS from 4000 → 8000 for consistency with GetContextFacadeAdapter (recall returns full ranked entries with previews, so a tighter budget than the bundle made no sense).

Tests (VALUES not SHAPE — Phase-9 rule)

The pre-existing unit test ("trims the tail when cumulative token cost would exceed maxTokens") used expect(getEntries().length).toBeLessThanOrEqual(1) — that loose assertion silently passed even at length=0, exactly the bug. It's now toBe(1) plus an exact id assertion.

• Tighten "trims the tail" + "respects the filters.limit slice" to assert exact lengths.
• New unit test: top hit is always returned when it solo exceeds budget.
• New unit test: a mid-ranking oversized hit is skipped and smaller hits behind it still surface (continue-vs-break semantics).
• New integration test: reproduces the dogfood scenario by recording a 1.8 KiB learning and asserting mem.recall("GitFlow", max_tokens: 50) returns hits.length >= 1.
• New integration test: the new 8000-token default lets multiple hits surface for a literal "hexagonal" query against the seeded corpus.

5+1 EXIT=0

Check	Result
npm run typecheck	EXIT=0
npm run lint	EXIT=0
npm run lint:tests	EXIT=0
npm run validate:modules	PASS — no module violations (retrieval×46 + curator×10 cross-imports as ADR-001)
npm run build	EXIT=0
npm test	2557 passing in 211 files (+4 vs 2553 baseline)

Test plan

• Pre-fix unit test reproducing the bug (always includes the top-ranked hit even when it solo exceeds maxTokens (B-MCP-8)) — fails on develop, passes on this branch.
• Pre-fix integration test reproducing the dogfood scenario (returns the top hit even when its preview alone exceeds maxTokens (B-MCP-8)) — fails on develop, passes on this branch.
• All 2557 existing tests still pass (no regressions).
• CI green (typecheck + lint + lint:tests + validate:modules + build + test:coverage + SonarQube quality gate strict).
• Post-merge: cut release/0.1.2-beta.5 and validate fix end-to-end against the real dogfood DB.

🤖 Generated with Claude Code