fix(providers): honour the search limit so every provider gets the same result budget - #65
Open
rajarshidattapy wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #64 — Supermemory hardcoded
limit: 30while the search phase asked every provider for10, so it received ~3x the answer-prompt context of the providers it was being compared with.
Also affected: Zep (the issue's table under-counts it)
While verifying the per-provider table I found a second violation of the same contract. Zep
searches edges and nodes as two separate queries and concatenates them:
With
limit: 10that requests 10 of each and returns up to 20, not the 10 the issue'stable records. Fixing only Supermemory would have left Zep with roughly double its peers'
context, so the comparison would still not have been like-for-like.
Confirmed compliant:
mem0(top_k: options.limit || 30),filesystem,rag.Changes
providers/supermemory—limit: options.limit || 30, matching how the same call alreadytreats
options.thresholdone line below. Also normalised the tab-indentedsearchMode/includeblock, since those are the lines the fix touches and the file was the one file in thisdiff that was already failing
prettier --checkat HEAD.providers/zep— the limit is now split between the two scopes instead of applied to each,via an extracted
splitSearchBudget(). Kept edge-heavy at 2:1 to preserve the previous20-edge / 10-node intent, so
limit: 10becomes 7 edges + 3 nodes rather than 10 + 10.orchestrator/phases/search— the limit is now enforced at the point results are consumed,not just requested. A provider that over-returns is truncated to the shared budget and logged:
This is the part that makes the fairness property hold going forward. Patching the two current
offenders fixes today's numbers; the chokepoint means a provider added next month cannot quietly
reintroduce the same advantage. The magic
10and0.3are now named constants, with thek = 10coupling tocalculateRetrievalMetricswritten down.Note the truncation had to come with the Zep fix rather than instead of it: edges are pushed
before nodes, so a blind
slice(0, 10)on unfixed Zep output would have silently dropped everynode result and turned Zep into an edges-only provider.
types/provider— documentedlimitas a hard cap rather than a hint, including the"split the budget across scopes" rule, since
SearchOptionsis where a provider author looks.The issue suggested surfacing this as a declared capability; documenting the contract and
enforcing it centrally seemed better than adding an interface knob no provider needs yet.
Tests
src/orchestrator/search-limit.test.ts:splitSearchBudgetnever exceeds the caller's limit across limits 1..50, always leaves atleast one node slot, and stays edge-heavy.
runSearchPhaseagainst a stub provider that returns 30 results for a limit of 10: assertsboth the checkpoint and the persisted result file are capped at 10. The file matters
independently — the answer phase reads context from disk, not from the checkpoint, so a fix
that capped only one of the two would still have leaked the advantage into the prompt.
quietly become a floor.
bun test3/3,tsc --noEmitclean,prettier --checkclean on all touched files.Effect on existing numbers
Any Supermemory or Zep run recorded before this change is not comparable with runs from other
providers, and not comparable with post-fix runs of its own: accuracy,
avgContextTokens, andthe MemScore token component all move. Published leaderboard entries for those two providers
should be regenerated rather than mixed with new ones.
Follow-up worth its own issue
Result count is now equal, but result size is not. Supermemory requests
include: { summaries: true, chunks: true }, so one result can carry substantially more textthan one mem0 memory or one Zep edge.
contextTokensis measured per prompt so it reports thishonestly, but "10 results" still does not mean equal context across providers. Capping by
token budget rather than result count would be the real like-for-like fix, and that is a
design decision rather than a bug fix.