Repository navigation
fix(mcp): give the similar-notes advisory its own similarity floor - #1722
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bba7993897
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5f50e430a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5f50e430a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0dc5d83eac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
write_note's "Similar existing notes" advisory used search's relevance floor (semantic_min_similarity, 0.55). On the default bge-small-en-v1.5 model the nearest neighbor of almost any note clears that, so a note on an unrelated topic was offered as a possible duplicate. Measured with the real model (#1718): rewrites of an existing note score 0.77-0.93 against it, while the nearest neighbor of a note on an unrelated topic scores at most 0.67. The advisory now asks for 0.70. The query is built by similar_notes_query so a real-model test runs the exact search write_note sends. Hybrid search's handling of gibberish queries is unchanged: with no FTS match, gibberish reached 0.66 on the Moby Dick vault while a correct zero-overlap paraphrase scored 0.62, so no threshold separates them. Refs #1718 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
Cosine scores are model-specific: OpenAI paraphrases cluster near 0.37, so the 0.70 floor measured on bge-small-en-v1.5 would hide real duplicates there. similar_notes_min_similarity returns the floor only for the default fastembed model (keeping a stricter user floor) and None otherwise, so other models keep semantic_min_similarity. Refs #1718 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
The advisory floor compared config strings exactly, so the canonical FastEmbed name (BAAI/bge-small-en-v1.5) or a provider written as "FastEmbed" fell back to the 0.55 search floor even though they load the same weights. Compare the identity the provider factory resolves: the provider lowercased and stripped, the model through FastEmbed's alias map (now exposed as FastEmbedEmbeddingProvider.resolve_model_name). Refs #1718 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
…dentity Three review rounds asked whether a config is "the measured model" one field at a time (model, provider spelling, prefixes). The codebase already answers that question: configured_embedding_provider_identity is what decides whether stored vectors are reusable. The advisory floor now applies only when that identity equals the default config's, which covers provider case, dimensions, and document/query prefixes in one comparison. The FastEmbed alias resolver added in the previous commit is reverted; alias spellings count as a different identity here exactly as they do for vector reuse, and fall back to the server's floor. Refs #1718 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
7ac46ca to
a37d862
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a37d862ba2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
BasicMemoryConfig() reads BASIC_MEMORY_* variables, so a model set through the environment produced the same identity on both sides of the comparison and wrongly got the advisory floor. model_construct() yields the field defaults without settings sources. Refs #1718 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Signed-off-by: Paul Hernandez <60959+phernandez@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b947690e44
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| content=content, | ||
| exclude_file_path=result.file_path, | ||
| exclude_permalink=result.permalink, | ||
| min_similarity=similar_notes_min_similarity(ConfigManager().config), |
There was a problem hiding this comment.
When the MCP process has the default FastEmbed config but get_project_client() routes this write to a cloud project using OpenAI or another lower-scoring model, this computes 0.70 from the local config and sends it as a per-query override; the routed server therefore cannot apply its own semantic_min_similarity, and the helper's own OpenAI example says real paraphrases near 0.37 will be hidden. Fresh evidence beyond the declined routing thread is the opposite route direction: that thread's non-default-local/default-cloud case returned None and preserved old behavior, while default-local/non-default-cloud now actively exports 0.70. Resolve the floor on the routed server, or send it only for a provably local route.
AGENTS.md reference: AGENTS.md:L344-L350
Useful? React with 👍 / 👎.
Refs #1718. This fixes the advisory half. The hybrid-search half stays open, because the measurements below show no threshold that separates gibberish from real matches.
What changed
write_note's similar-notes probe asks formin_similarity=0.70(SIMILAR_NOTES_MIN_SIMILARITY). It used to use search's relevance floor,semantic_min_similarity(0.55).similar_notes_query(), so the new real-model test runs exactly the searchwrite_notesends.Measurements
Default model bge-small-en-v1.5, real embedder (
semanticmarker, so not the test fake), production indexing and search path. Corpora:test-int/semanticAdvisory: top-1 vector score of the probe (title + first 900 characters).
The #1259 measurements on the Moby Dick vault put rewrites at 0.78-0.87. 0.70 sits between the highest unrelated neighbor (0.671) and the lowest rewrite (0.767). Related-but-distinct notes on a dense vault score 0.84 at the median (#1259), so they are still listed. The advisory remains a ranked question, as before.
Hybrid search: top-1 vector score when the hybrid FTS leg (strict, then relaxed) finds nothing, which is the only case a "drop vector-only hits below X" rule would touch.
Gibberish on Moby Dick (0.659) outscores a correct zero-overlap paraphrase on the everyday corpus ("vehicle upkeep" → car maintenance note, 0.618). Any floor that removes the noise also removes real paraphrase matches. The top-1/top-3 gap does not separate them either: on the templated technical corpus a correct paraphrase has a gap of 0.002, the same as noise.
Tests
test-int/semantic/test_similar_notes_advisory.py(real model): two rewrites still find their original, and three unrelated notes get no suggestions. With the floor set back to 0.55, two of the three unrelated cases fail.tests/mcp/test_tool_write_note.py: the probe payload carries the new floor.just fast-checkis clean, thetest_tool_write_note.pytests pass (69 passed), and the semantic tests pass.🤖 Generated with Claude Code
https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea