feat(recall): rework ranking, bound recall output, add memory_get - #4
Open
beardthelion wants to merge 15 commits into
Open
feat(recall): rework ranking, bound recall output, add memory_get#4beardthelion wants to merge 15 commits into
beardthelion wants to merge 15 commits into
Conversation
Pins current recall behaviour before the ranker changes land, so each later unit has a mechanical flip to point at instead of a claim that things feel better. tests/relevance.test.ts scores four entries, which is too small for term rarity, coverage ties, or superseded-note ordering to mean anything. The corpus is synthetic and self-contained: no filesystem reads outside tests/, no env-supplied path, no network, enforced by reading the module's own source. Ranking goes through rankMemories directly because tests/setup.ts freezes the caps at five entries per namespace, so the client path would trip quota before it ranked anything. Three pairs the ranker fails today carry test.failing markers naming the unit set that closes them. test.todo is not used: its body does not execute under plain bun test, which is what CI runs, so a todo marker cannot flip. Three further pairs are accepted residuals phase 1 does not close (vocabulary mismatch, tie width, supersession) and are reported rather than asserted. Also lands two boundary checks, each proven to fail before being trusted: a transitive deny rule that no module under src/ outside src/mcp/ reaches client/ or src/mcp/ (an allowlist would go green when a new module reaches plaintext through src/mcp/tools.ts), and a single-origin check that stubs fetch at the tools layer and asserts every request goes to the configured URL. src/mcp/server.ts is import-unsafe, so the check runs against the tools/client layer. Held-out baseline: 2/12 top-1 correct, every pair held to at most one shared content term post-stemming so the set cannot be satisfied by restatement. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Code review found every one of the harness's three guards could go green while the thing it guards was broken, and two were demonstrated doing so. Each fix below was proven by planting the attack, observing red, reverting, and observing green, because accepting a fix on a still-green suite is the same mistake one level up. The single-origin check recorded the request origin and discarded the body, so it proved traffic went to one host and said nothing about what was in it, which is the invariant it was credited with enforcing. It now drives a plaintext sentinel through save and asserts the sentinel reaches no URL and no body, and that the uploaded entry values are base64 envelopes. The import deny rule skipped bare specifiers, so a src/ module could reach the crypto code by the package's own name; package.json self-maps @gitlawb/memlawb/crypto to client/crypto.ts and bun resolves it. It also missed template-literal and variable dynamic imports and require(). Bare specifiers now resolve through package.json exports, require() is matched, and any import()/require() whose argument is not a quoted literal counts as a violation, since an edge that cannot be resolved cannot be shown safe. The corpus provenance check greped one file, and an imported helper doing readFileSync pulled 39,724 bytes of a real instruction file into the ranked corpus while it stayed green. It now walks the corpus module's import graph and requires it to import nothing, matches more hooks, and strips comments first so a doc comment mentioning process.env does not false-positive. test.failing treats any throw as the expected failure, and the markers looked their pairs up inside the failing body, so one extra space in a query left the suite at 14 pass while decoupling a marker from the ranker permanently. A plain test now resolves every marker query against TUNING, reading this file's own source so it catches drift from either side. Also rewrites the fixture corpus as notes for an invented project, so the module's claim that every byte is invented is true rather than a statement about restraint. The load-bearing properties were preserved and re-verified: the shared token that makes 'deployment process' pick the wrong entry, the repeated Why: lines that produce the 18-wide tie, the stray unstemmed token that wins the pull-request query, entry count, and all twelve held-out pairs at or under the one-shared-term floor. The reported held-out baseline moves from 2/12 to 4/12 because the surrounding corpus no longer has accidental higher scorers; nothing was relaxed to get it, and a higher baseline is a stricter bar for the Definition of Done. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Follow-up to the review pass. Four checks asserted outcomes a broken version of themselves would also produce, so each is now proven red against the failure it exists to catch. The guard against a dead import walk asserted the size of a set seeded by the directory scan, so stubbing edge extraction to return nothing left it green. It now counts edges resolved inside the traversal (33 today, asserted at a floor of 20) and requires one known transitive edge to have been reached. The test named for generator determinism asserted an unrelated constant and never called the generator, which had no callers anywhere. It now compares two generations byte for byte and checks the entry count at n=2000, where the zero-pad width decides whether keys collide; a planted collision passes at n=64 and fails at n=2000, which is why the larger size is there. The held-out rate was printed but never asserted, so a ranker regressing to returning nothing would produce an identical green run. Its measured baseline of 4 of 12 is now a floor with a dated comment, to be raised by later units and never lowered to accommodate a regression. The overlap stopword list claimed to be kept in step with the ranker's with nothing enforcing it. The ranker does not export it and this unit may not change src/, so the test parses the literal out of the source and asserts set equality, failing closed if the declaration stops matching rather than reading an unparseable list as empty. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Matches morphological variants so a query saying "deployment" reaches a note saying "deploy". Written test-first: the acceptance tests were run against the old tokenizer and observed failing before the normalizer existed. The guards are the contract, not polish. An unguarded suffix stripper was measured mangling short words (thing to th, king to k, bring to br, sing to s), which manufactures false matches. So: a stem shorter than four characters means do not strip, a trailing s preceded by s is never stripped so class and process survive, and ies maps to y so queries reaches query. A damage test runs the stemmer over a generated 2,043-word vocabulary and asserts no word stems below three characters and that none of the known-bad mangles occur. The guards were proven load-bearing by mutation: relaxing the minimum length turns five tests red, deleting the ss guard turns three red. Removes the harness's second stemmer rather than checking two implementations for agreement. The overlap floor is supposed to measure the terms the ranker actually sees, so it now takes the ranker's stemmer as a required argument and the corpus module declares none of its own, with a test asserting it stays that way. One implementation means drift is impossible rather than merely detected. Also corrects a fixture that never reproduced the failure it was built for. The pull-request pair was measured as "returns nothing, stemming finds the rule", but the committed corpus gave a competing entry a literal open token, so that entry won both before and after stemming and no lexical unit could close the pair. The competitor's token is now stemmable, which restores the measured behavior: with stemming neutralized the query returns nothing, and with stemming it returns the rule. The marker flips on that evidence, not on the assertion having been made easier. Two of the three flippable tuning pairs are now hard assertions. The push query stays test.failing: it needs U3's soft coverage as well, and it was left failing rather than reattributed to keep the closing-set claim honest. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Three changes that cannot ship apart. Term weight now scales inversely with how many of the passed entries contain the term. Coverage degrades a score instead of eliminating it, so the zero-coverage hard drop is gone. And the floor replaces what that drop was silently doing: bounding the result set. The floor has two conditions because one of them alone is inert. The relative cut drops results below a fraction of the query's top score, but it can never trim the top hit, since the top is always the whole of itself. So an absolute minimum decides whether anything is returned at all. That absolute half is the only thing that makes no-match reachable, and it was written and watched failing before the hard drop came out, which is what proves the two are one change rather than two. The floor's threshold is measured against this corpus and this scoring, not inherited: relevant and irrelevant queries were run and their score ranges recorded in the pin comment, including the fact that they overlap. They do overlap, and the comment says which probes still leak rather than implying a clean separation. Extends the stoplist with function words. A modal verb occurring once in the corpus was being rated maximally informative, so an irrelevant gardening query outscored a must-pass pair two to one. Corpus size does not fix this: measured at 26, 226 and 2026 entries the gap never closes, because whether document frequency downweights a function word depends on corpus composition. A stoplist handles them at any size. Domain adjectives were deliberately left indexable: a short timeout and a long poll are things worth recalling. rankMemories keeps its signature and delegates to a new rankMemoriesDetailed carrying the searched and below-floor counts U7 needs, rather than widening the return across twenty call sites where each edited test is a chance to weaken an assertion while fixing a type error. Also repairs the fixture and guards it. Two target entries had lost match arity in the fiction rewrite: they matched one query term where the original measurement had three, so the pairs tied and lost on key sort rather than reproducing the failures they were built from. Arity was restored by writing natural prose, never by copying a query into its own answer, and a new test pins each pair's matched-term count and requires the target to beat every competitor outright. That guard was proven by removing a term and watching it go red. It exists because this defect was found three times, one pair at a time, and the rewrite brief never named arity as load-bearing. All four non-residual tuning pairs now pass as hard assertions with no markers left. The three accepted residuals stay reported and unasserted. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
MEMORY.md is a link list, so it carries the whole namespace's vocabulary without carrying any of its content. On a real 301-entry store it ranked first for three of six probe queries and appeared in the top three for four, which was the most fixture-independent of the measured failures. It is filtered out of the input before anything counts, not filtered out of the results afterwards. A post-filter would hide the entry while leaving it in the corpus statistics, and the statistics are half the problem: an index that repeats a term makes that term look common, so a note genuinely carrying it loses the rarity weight it earned. The denominator drops from 26 to 25 here, and a test pins that by asserting the ranking over a corpus containing the index is string-identical to the ranking over a corpus with it deleted, which fails if either the document frequency or the entry count is still counting it. Exclusion is exact-key, so a nested project/MEMORY.md stays an ordinary rankable entry. The nested fixture lives in the unit test rather than the shared corpus deliberately: adding it there would have restored the denominator to 26 and shifted every document frequency under the search pin, the arity guard and the held-out set, for no additional coverage. Only the ranker changed. search is substring-based, never calls the ranker, and still finds the index by name, which is correct and is now pinned by a test asserting recall omits it while search returns it. The floor constant was re-measured rather than refitted: scores rose with one fewer entry in the denominator but the relevant and irrelevant populations did not converge, so 0.6 still separates and the comment records the re-check. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Recall is about to stop returning whole bodies, so something has to be able to return one. Without this, bounding recall output would remove a capability with no replacement: save overwrites, search truncates at 200 chars, and list returns keys only. Three properties are contract rather than accident, and say so in the code. The output is bounded by the caller naming exactly one key per call, so unlike recall a namespace cannot amplify a single request; an agent looping over memory_list keys can still reassemble the namespace, one host-visible call at a time, and a batch variant is deliberately out of scope. The body is returned verbatim as data: nothing in it is parsed, and no value derived from entry content picks the key or namespace echoed back, which matters because entries are written by agents processing untrusted material. And it reuses pull, so it pays a whole-namespace decrypt per single-entry read, accepted here because no client-side single-entry fetch exists and the client is out of scope. A missing key returns an ok-shaped result naming the key and namespace and pointing at memory_list, never an isError transport failure and never an empty success, because a denial that renders as success is worse than one that renders as an error. Key lookup uses Object.hasOwn so a key like __proto__ misses instead of resolving off the prototype. Extends the single-origin boundary check to drive the new tool, which U1 left as a placeholder for this unit. Raising its request-count guard was not enough: save alone issues two requests, so the check still passed with the get call deleted. It now tracks the requests attributable to get specifically, and that was proven by deleting the call and watching it go red. Cap-boundary and pull-failure cases run against an in-memory stub implementing only the four client methods the tools touch, exported for U6 to reuse. They cannot run on the real-server harness because the test environment pins the namespace quota at five entries and 5000 bytes, so a 250KB save trips the gate before the tool is reached. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Recall was emitting every hit's full body, so five hits could cost five whole files. With entries capped at 250KB and limit reaching 20, that is the path a crafted namespace would use to push megabytes into an agent's context. This bounds it: a per-hit cap of 600 chars and an aggregate cap of 4000 across the call, both proven by raising them and watching the assertions fail. The region is the block that best matches the query, not the first N chars. Blocks are blank-line separated, tagged with the heading they sit under, and a fenced code block is held atomic so no boundary can fall inside one. Terms are stemmed with the ranker's own exported stemmer, and weighted by how few blocks of that entry contain them, so a word repeated throughout the entry cannot outvote the rare word that actually located the answer. A hit that scored on its key or description alone falls back to the opening block rather than picking an arbitrary one. 600 sits between the measured paragraph median of 310 and p90 of 766, so a typical region survives whole while the 50,000-char tail is cut. 4000 is roughly a thousand tokens and the default limit of 5 never reaches it, so the aggregate only binds on the wide calls it exists for. Budget is spent in rank order and hits it cannot reach are reported by count rather than dropped silently. Frontmatter is stripped before splitting, so it can never be returned. An oversized fence is elided with a marker rather than half-included, since half a code block is worse than none. Every clipped hit carries a pointer that memory_get returns the entry whole, which is why that tool had to land first. snippet() is untouched. Its only caller is search, which is pinned byte-exact and must not change in phase 1, so generalizing it would have been risk against a frozen surface for no gain. Note on the plan: D11 said to fall back to the enclosing heading section when a paragraph is oversized, which cannot work, since the section contains the paragraph and is necessarily larger. Implemented as keeping the section heading alongside the clipped paragraph, which is what the fallback was for. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
The miss already named the namespace and the query, so this is not about making it legible. It is about the next move. An agent that gets a bare miss concludes the fact was never recorded and saves it again under a second key, and phase 1 ships no supersession to reconcile the pair, so the duplicate is permanent. The miss now reports how many entries were searched and how many fell below the floor, and says to check memory_list before concluding a fact is unrecorded. The counts come from the ranker, not a recount here: recall now calls rankMemoriesDetailed and the plain rankMemories import is gone, so a local recount is not reachable. That distinction is observable rather than asserted. A stopword-only query over four entries reports three searched, because the namespace index is not a candidate; a recount over the entry map would say four, and a belowFloor derived as "everything not returned" would say three where the ranker says zero. Below-floor entries stay withheld, and withheld means absent. The count is a number and nothing more: no key, no filename, no fragment of body text. An entry the ranker judged irrelevant does not become relevant by being summarized in the miss, and naming it would spend the caller's context on the thing that was just rejected. A test builds a namespace of distinctively named entries and asserts none of them appear; planting a "closest match" hint makes it fail. The empty-namespace message is untouched and still distinct, because "nothing is stored" and "nothing matched your wording" call for different next moves. Closes the guidance sync for phase 1. Both surfaces now describe the same tool surface and the same recovery flow, and both explain a residual that otherwise reads as an inconsistency: recall never returns MEMORY.md, yet agents are still told to keep it. An index matches a little of every query and answers none, so ranking it spends a slot a real entry should have had. It stays as the table of contents an agent reads deliberately with memory_get, or scans alongside memory_list when orienting or following up a miss. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Whole-branch review found twenty issues. The implementation was largely sound; the guards around it were not, and two of them had been holed twice each by someone finding a route their enumeration did not list. Adding the newly found route to each list would have been the third instance of the same mistake, so both now fail closed: they flag anything they cannot positively account for. The corpus provenance guard enumerated forbidden calls, and Bun.spawnSync was not among them, so a subprocess pulled 39,724 bytes of a real instruction file into the ranked fixture while the check stayed green. It now asserts the property actually wanted: the fixture module is imported in two subprocesses under different cwd and env and its serialized exports must be byte-identical. An identifier allowlist covers the one case reproducibility cannot, an absolute-path read that is stable under both. Neither half is sufficient alone and the code says so. The import boundary seeded only .ts files, so a .mjs under src/ reaching the crypto module was invisible. It now seeds every extension bun executes, treats an unclassified extension as a violation so shrinking the list cannot hide a file, and stops enumerating loader forms: accounted imports are removed and any surviving import or require token is a violation, with callee roots and bare specifiers on pinned allowlists. Verified against routes named nowhere in the review, including eval of a dynamic import and a Function constructor reached through globalThis. The wire check ignored headers entirely and stringified a typed-array body to something unreadable, so plaintext could pass either way. It now captures headers, normalizes every body shape it knows, and fails closed on one it does not. More importantly it only ever ran against an empty namespace, so the check whose purpose is proving no plaintext leaves the process had never seen a namespace containing any; it now drives a populated one over genuine ciphertext. Two real defects the guards were hiding. Recall could render a hit as a bare key with no region and no pointer, a denial shaped like success. And the region picker weighted query terms by within-entry rarity without the ranker's stoplist, which inverted its own stated intent: a stopword confined to one block took the maximum weight while the topical term that made the entry rank collapsed to the minimum, so "should I use retry" returned the block about something else. Both layers now share the ranker's tokenizer. Also: the absolute floor gated only the top hit, so members below it were returned; the stopword-only query reported zero withheld rather than saying it carried no content words; an index-only namespace reported zero searched; and recall's tool description still promised whole memories, which for an MCP tool is the contract an agent reads and would have defeated the bounded-region design. That last fix had no mechanical proof available, since nothing reads tool descriptions and the server cannot be imported under test, so it now has a source-text guard that fails closed if it cannot parse the registration. Every fix was proven by planting the failure, observing red, reverting, and observing green. The client stubs were unified and their casts removed so the compiler enforces the real client contract, and the arity guard now pins where a match sits rather than only how many there are, since a target losing its frontmatter description kept its count while its score collapsed. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Round 2 found that round 1's fix repeated round 1's defect one level up. The two boundary guards were inverted from enumeration to fail-closed, which was right, but the inversion was only ever tested as a whole: plant a violation, see red. Nothing checked that any particular rule inside it still worked, so five of its sub-checks could be neutered independently and the suite stayed green. A composite guard tested only in composite is a guard that cannot fail. Twenty-one rules now have a positive control each, and each control was proven by removing the rule it covers and watching that control, specifically, go red. Every control asserts the exact violation rather than that something fired, and three carry negative controls beside them so a rule that reports everything fails too. The root cause of both P1s was one regex. The declaration matcher, written for method definitions, also matched any call in statement position, so the callee was recorded as locally declared and skipped the allowlist entirely. A module under src/ reached the crypto code through a statement-position Function call with the whole suite green, and the same hole let a seeder pull real disk contents into the ranked fixture, which is the round-1 incident reproduced against its own fix. Declarations now require a body, an arrow, or a return type, so a bare call falls through to the allowlist. Three further escapes are closed: the boundary classified by lexical path so a symlink into client/ evaded it and now every path is resolved through realpath; the corpus identifier allowlist permitted String while String.constructor is Function and the capability sweep ran over source with string literals blanked, so both spellings of the dangerous properties are rejected and the sweep now reads the unstripped text; and the wire check read only init.body, so a body carried on a Request object was never seen. Also fixes a false positive I found by planting ordinary code rather than an attack: an ES private field had its hash stripped and the bare name read as an undeclared global, so a normal class writing this.#cache.get(k) was reported as calling an unvetted global named cache. A guard that fires on a standard language feature gets deleted by the first contributor it blocks, which costs more than the hole it closes, so the false-positive control now carries that shape alongside Map, generics, classes and re-exports. And it writes down what these guards do not do. The provenance check catches programmatic and accidental ingestion; it cannot catch real content pasted in as a string literal, because that is reproducible, names no identifier and uses no capability. The control for deliberate ingestion is a human reading the diff, and the code now says so rather than implying coverage it does not have. Product fixes in the same pass: a fenced block sized between the cap and the cap minus its heading prefix skipped elision and was truncated mid-fence, leaving an opening fence with no closer, which in an agent's context swallows everything after it. Recall's aggregate budget never charged its joiners or its lead line, so real output measured 4158 against a documented bound of 4000, now 3990 with the test pinned to the constant. The region picker's stoplist was over-corrected in round 1: sharing the ranker's stoplist hid the temporal words that distinguish sections, so "after I push" returned the before-push section. Topical terms now decide and function words only break ties, which holds both that case and the round-1 case where a stopword confined to one block took the region. The suggested fix for that one did not survive contact and was replaced rather than shipped. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
The comment said five shapes of ordinary code; there are eight. The list grew when the private-field case was added, and that case is the only real false positive the review found, so the comment now says to add to the list rather than trim it and records why that entry is there. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Contributor
Author
|
One thing to settle before this merges:
Blast radius is small since nothing external consumes recall yet, so patch is defensible. I'd just rather the call be deliberate than fall out of the commit subjects. |
The comment above ABSOLUTE_FLOOR said 0.6 sits "above the strongest irrelevant query the floor can reach (0.55)". That reads as a bound on what an irrelevant query can score. It is not: 0.55 is the strongest irrelevant score the 26-entry fixture happens to produce. The same query scores 7.19 against a real store. Measured against 838 real entries, truncated to the short one-fact shape the memory guidance prescribes and well inside the entry and byte caps in config.ts: 60 natural-language questions retrieved their correct entry 0 times, 48 of 60 correct entries fell below this floor and were never returned at any depth, and 54 of 60 scored below the worst off-domain junk. Median score of the true target was 0.00 against 6.33 for the top-ranked wrong entry. A positive control rules out a broken harness: queries built from an entry's own four rarest terms retrieved it at rank 1, 60 of 60. The block already said the constant does not transfer. It did not say what non-transfer costs, and the answer is that no value works at that scale. Raising it does not help, because the gate was never separating relevant from irrelevant. It separated "matched nothing" from "matched something", and in a 26-entry fixture those coincide because incidental term overlap is rare. Add entries and the irrelevant population climbs to meet a target still sitting at zero, since a natural question shares no vocabulary with the note that answers it. That is the vocabulary-mismatch residual phase 1 already accepted, and it sits upstream of anything this constant can do. The comment now also warns off the trap that produced the wrong first diagnosis. The preserved harness builds its relevant probes from each entry's own rarest terms, which is the easiest query an entry can receive, so it reports a comfortable separation at every corpus size. Only real queries show the inversion. Comment only. No behavior change; the constant is untouched. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Contributor
Author
|
Following up on my earlier note about the version bump, with the concrete ask. Suggested squash title: The @kevincodex1 this is ready when you have a moment. |
…ed ones Two ways recall could return a confident answer that hid what it left out, plus the comment that made a reviewer file a third. `regionFor` computed `partial` against the frontmatter-stripped body, so stripping never counted as returning less than the whole entry. The ranker weights `description:` at +3, so an entry can be selected for terms that live only in its frontmatter; a short single-block body then came back unmarked and without a pointer. A query matching `description: never deploy on Friday` returned `Deploys use Fly.` with no mention of Friday and no way to reach it. Compare against the whole entry instead, which subsumes the old check since with no frontmatter to strip it is exactly the body. Hits cut by `limit` were dropped in silence while budget omissions got a counted tail, so 15 entries above the floor with a limit of 5 rendered exactly like a namespace holding 5. `belowFloor` cannot cover this: it is computed before the slice and excludes limit trimming by definition. The ranker now reports `trimmedByLimit` and the tail counts both causes, naming whichever applies. The budget reserves the worst case on both axes so the aggregate cap still bounds the whole string. The `ABSOLUTE_FLOOR` scale note recorded 48 of 60 targets falling below the floor without recording that those 60 questions were written by the person who then read the results. Measured against 65 held-out bug reports, each asserted from timestamps to predate every entry answering it, the floor emptied 0 of 65 at full length and 0 of 65 truncated to the one-fact shape. The note now says so, and says what does govern the hit rate. Both fixes verified load-bearing: reverting each turns its test red. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
…arry The ABSOLUTE_FLOOR note told the reader to reproduce the scale measurement with `docs/plans/measurements/floor-vs-scale.ts`. That path is in .git/info/exclude and has never been committed, so for anyone cloning the repo it resolves to nothing and the instruction cannot be followed. Keep the part that is useful without the file: what the original harness got wrong, so a replacement is not built the same way. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Contributor
Author
|
Heads up that the head moved since I pinged you: Two correctness bugs in recall, both fixed with tests that fail without the fix: An entry selected on its frontmatter Hits cut by The second commit removes a comment reference to a path this repo does not carry. CI is green on the new head, and the |
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
1 participant
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.
Reworks memory recall. Recall previously returned whole entry bodies ranked by a
term-frequency scorer with no stemming, no rarity weighting, and a hard drop when
a query term did not match, which produced silent misses and dumped whole files
into the caller's context.
What changed
The ranker now stems tokens, weights terms by how rare they are in the corpus,
degrades partial coverage instead of dropping an entry outright, and excludes the
namespace-root index from both ranking and the document-frequency denominator.
Results are bounded by a two-part relevance floor: a relative cut trims the tail,
and an absolute minimum decides whether anything is returned at all.
Recall returns the matching region of each hit rather than the whole body, capped
per hit and in aggregate, with a pointer to the new
memory_gettool for the fullentry. A miss now reports how many entries were searched and fell below the floor,
and says to check
memory_listbefore concluding a fact was never recorded, sincean agent that wrongly concludes that saves it again under a second key.
What it fixes, measured
Six failures were measured against a fixture corpus before any code changed.
Three are closed:
Three remain and are reported by the suite rather than asserted, because no
lexical ranker closes them: a query whose wording shares no terms with its answer,
multi-way ties on common words, and a superseded note ranking level with the note
that replaced it. The last one needs the confidence and supersession work that is
deliberately not in this change.
Separately, the namespace index no longer appears in results at all. On a real
301-entry store it had ranked first for three of six probe queries.
Notes for review
The invariant that the server never sees plaintext now has two mechanical checks
rather than none: no module in the crypto-blind server path may reach the client
or MCP code, and every request the tools make must go to the configured origin
carrying no plaintext. Both fail closed, flagging anything they cannot account
for rather than matching a list of known-bad forms, and every rule inside them has
a control test that was verified by removing the rule and watching that control
fail.
searchis unchanged by design. It has nolimitparameter, so a commonsubstring returns every hit in the namespace; that is a real defect, it is
recorded rather than fixed here, and it means the output bound this change adds
is per-surface rather than product-wide.
Recall's output shape changed, so anything parsing it will need updating.
Fixture content is synthetic. The provenance check that enforces that catches
programmatic reads at import time and content that varies by environment; it
cannot catch text pasted in directly, which is reproducible and uses no
capability. That limit is stated in the code, and diff review is the control.
Verification
bunx biome ci,bun run type-check, andbun testall pass; 167 tests, up from39 on main. The suite includes a regression corpus with a held-out set that is
never tuned against and is ratcheted so it cannot silently regress, a latency
budget for ranking a near-cap namespace, and the boundary checks above.