Skip to content

fix: forward hybrid/keyword search params through all search paths and update docs#251

Open
nkanu17 wants to merge 9 commits intomainfrom
docs/add-hybrid-search-references
Open

fix: forward hybrid/keyword search params through all search paths and update docs#251
nkanu17 wants to merge 9 commits intomainfrom
docs/add-hybrid-search-references

Conversation

@nkanu17
Copy link
Copy Markdown
Contributor

@nkanu17 nkanu17 commented Mar 26, 2026

Ensure search_mode, hybrid_alpha, and text_scorer parameters are forwarded through every search path in the system, and update documentation to accurately reflect hybrid and keyword search capabilities.

Fixes

  • Hybrid/keyword search params silently dropped (3 paths)
  • client.py _resolve_search_memory: resolve_tool_call() accepted search_mode, hybrid_alpha, text_scorer from LLM tool calls but never passed them to search_memory_tool() - all LLM-initiated searches silently defaulted to semantic.
  • mcp.py memory_prompt: Missing all three params in function signature and SearchRequest construction - MCP prompt hydration was locked to semantic search.
  • client.py hydrate_memory_prompt: Missing all three params - convenience method couldn't use keyword or hybrid search.
  • Fixed tuple unpacking order: memory, created → created, memory to match tuple[bool, WorkingMemory] return type.
  • Removed dead MemoryNotFoundError catch from get_or_create_working_memory example (method creates instead of raising).
  • _resolve_search_memory defaulted max_results to 5 but the tool schema and search_memory_tool signature both use 10 - changed to 10

Documentation & Description Fixes

  • Updated all SDK docs (Python, TypeScript, Java, JS) to reference keyword and hybrid search.
  • Changed "exact term matching" → "full-text matching" in 4 files (keyword search uses BM25, not exact matching).
  • Changed "vector search" → search-mode-agnostic language in tool schema descriptions and docstrings.
  • Updated deprecated tool/function references (get_working_memory → get_or_create_working_memory, add_memory_to_working_memory → lazily_create_long_term_memory).
  • Clarified feature bullet: "Flexible search: semantic (vector embeddings), keyword (full-text), and hybrid (combined)".
  • LangChain search_mode parameter changed from str to Literal["semantic", "keyword", "hybrid"] for better LLM schema generation.

Tool schema updates (shown to LLMs):

  • agent-memory-client/agent_memory_client/client.py: search_memory tool description
  • agent-memory-client/agent_memory_client/integrations/langchain.py: LangChain tool description
Path Status
MCP search_long_term_memory Already forwards all 3 params
REST API search_long_term_memory Already forwards via SearchRequest model
REST API memory_prompt Already forwards via SearchRequest model
Client search_long_term_memory Already forwards all 3 params
Client search_memory_tool Already forwards all 3 params
Client memory_prompt Dict passthrough - params flow if included
long_term_memory.py core search Already accepts and uses all 3 params

Note

Medium Risk
Behavior changes in long-term memory retrieval: LLM tool calls and MCP memory_prompt now honor search_mode/hybrid_alpha/text_scorer, which can change returned context and ranking; otherwise changes are mostly documentation and schema wording.

Overview
Fixes several paths where keyword/hybrid search parameters were accepted but silently dropped, ensuring search_mode, hybrid_alpha, text_scorer, pagination, and optimize_query are forwarded consistently through the Python client tool resolver (_resolve_search_memory), the client convenience prompt hydrator (hydrate_memory_prompt), and the MCP memory_prompt tool.

Updates tool schemas/docstrings and SDK docs to describe semantic/keyword/hybrid search and clarify optimize_query applies only to semantic search; also refreshes examples to use get_or_create_working_memory semantics and aligns defaults (e.g., max_results to 10) with the published tool schema.

Written by Cursor Bugbot for commit 49008ec. This will update automatically on new commits. Configure here.

Update documentation and tool schemas that previously only mentioned
semantic search to also reference keyword and hybrid search modes.

Documentation updates:
- README.md: features list and architecture diagram
- docs/index.md: feature descriptions and context finding
- docs/long-term-memory.md: overview, feature table, search capabilities
- docs/mcp.md: search_long_term_memory tool description
- docs/quick-start.md: search step, summary table, checklist
- docs/use-cases.md: search optimization best practices
- docs/README.md: feature cross-reference table
- docs/agent-examples.md: long-term memory description
- docs/java-sdk.md: keyword and hybrid search examples
- docs/typescript-sdk.md: keyword and hybrid search examples
- examples/README.md: long-term memory description
- agent-memory-client/README.md: search mode examples
- agent-memory-client/agent-memory-client-js/README.md: search mode examples

Tool schema updates (shown to LLMs):
- agent-memory-client/agent_memory_client/client.py: search_memory tool description
- agent-memory-client/agent_memory_client/integrations/langchain.py: LangChain tool description
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. No suggestions at this time.

@jit-ci
Copy link
Copy Markdown

jit-ci bot commented Mar 26, 2026

🛡️ Jit Security Scan Results

CRITICAL HIGH MEDIUM

✅ No security findings were detected in this PR


Security scan by Jit

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates repo documentation and LLM-facing tool descriptions so “search” is described consistently as supporting semantic, keyword, and hybrid modes (instead of only semantic), aligning docs/SDK examples with the server’s supported search_mode behavior.

Changes:

  • Expanded documentation references from “semantic search” to “semantic, keyword, and hybrid search” across guides and examples.
  • Added/updated SDK usage examples (TypeScript/JS, Java, Python client README) to demonstrate keyword and hybrid search modes (including hybridAlpha/hybrid_alpha).
  • Updated LLM tool descriptions in the Python client and LangChain integration to mention the additional search modes.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
README.md Updates feature list and architecture text to include keyword/hybrid search.
docs/index.md Updates high-level marketing/feature bullets to include keyword/hybrid search.
docs/long-term-memory.md Expands long-term memory overview and adds explicit semantic/keyword/hybrid sections and JSON examples.
docs/mcp.md Updates MCP tool descriptions for long-term memory search modes.
docs/quick-start.md Updates narrative/checklist language to include keyword/hybrid search.
docs/use-cases.md Updates best-practice bullet to reference semantic/keyword/hybrid search + filters.
docs/README.md Updates feature cross-reference row to call out semantic/keyword/hybrid search.
docs/agent-examples.md Updates travel agent description to include keyword/hybrid search capabilities.
docs/java-sdk.md Adds keyword and hybrid search request examples.
docs/typescript-sdk.md Adds keyword and hybrid search examples using searchMode/hybridAlpha.
examples/README.md Updates travel agent feature bullet to include keyword/hybrid search.
agent-memory-client/README.md Adds Python client examples for keyword and hybrid search.
agent-memory-client/agent-memory-client-js/README.md Adds JS client examples for keyword and hybrid search and clarifies default semantic mode.
agent-memory-client/agent_memory_client/client.py Updates search_memory tool schema description to mention keyword/hybrid modes.
agent-memory-client/agent_memory_client/integrations/langchain.py Updates LangChain tool description to mention keyword/hybrid modes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…trings, and docs

- langchain.py: fix search_memory description to mention semantic/keyword/hybrid search;
  add search_mode, hybrid_alpha, text_scorer params to _create_search_memory_func
- client.py: update tool schema descriptions to use non-deprecated names
  (get_or_create_working_memory, lazily_create_long_term_memory,
  eagerly_create_long_term_memory); fix docstrings for deprecated resolvers
- agent-memory-client/README.md: use get_or_create_working_memory in examples
- docs/agent-examples.md: use get_or_create_working_memory in code example
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- agent-memory-client/README.md: fix (memory, created) -> (created, memory)
  to match get_or_create_working_memory() return type; remove impossible
  MemoryNotFoundError catch (get_or_create never raises it); fix duplicate
  except block
- docs/index.md: clarify that keyword search is full-text, not vector-based
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 8 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…rate comments

Bug fix:
- client.py _resolve_search_memory: forward search_mode, hybrid_alpha,
  text_scorer to search_memory_tool (previously silently dropped)

Accuracy fixes:
- client.py: query param description 'vector search' -> 'search query'
- langchain.py: use Literal type for search_mode parameter
- 4 files: 'exact term matching' -> 'full-text matching' for keyword search
  (agent-memory-client/README.md, agent-memory-client-js/README.md,
  typescript-sdk.md, java-sdk.md)
@nkanu17 nkanu17 changed the title docs: add hybrid and keyword search references across all documentation fix: search_memories not forwarding hybrid and keyword parameters and docs update for hybrid and keyword references Mar 27, 2026
…_memory_prompt

Both functions were missing search_mode, hybrid_alpha, and text_scorer
parameters, forcing all searches through these paths to default to
semantic-only search.

- mcp.py memory_prompt: accept and forward all 3 params to SearchRequest
- client.py hydrate_memory_prompt: accept and forward all 3 params to
  long_term_search dict
@nkanu17 nkanu17 changed the title fix: search_memories not forwarding hybrid and keyword parameters and docs update for hybrid and keyword references fix: forward hybrid/keyword search params through all search paths and update docs Mar 27, 2026
@nkanu17 nkanu17 requested review from Copilot and vishal-bala March 27, 2026 01:56
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…idation

- Fix optimize_query docs: clarify it only applies to semantic search,
  ignored for keyword/hybrid modes
- Fix default value docs: 3 occurrences said 'default: True' but
  signature is False
- Add distance_threshold + search_mode validation to MCP memory_prompt
  (matches existing validation in search_long_term_memory)
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

agent-memory-client/agent_memory_client/client.py:2707

  • Newly supported tool-call parameters (search_mode, hybrid_alpha, text_scorer) in _resolve_search_memory aren’t covered by existing tool-call resolution tests (e.g., tests/test_client_tool_calls.py only exercises query/max_results). Adding a test that calls resolve_function_call/resolve_tool_call with these fields and asserts they’re passed into search_memory_tool() would prevent regressions (including the silent-dropping issue fixed in this PR).
        search_mode = args.get("search_mode", "semantic")
        hybrid_alpha = args.get("hybrid_alpha")
        text_scorer = args.get("text_scorer")

        return await self.search_memory_tool(
            query=query,
            search_mode=search_mode,
            hybrid_alpha=hybrid_alpha,
            text_scorer=text_scorer,
            topics=topics,
            entities=entities,
            memory_type=memory_type,
            max_results=max_results,

agent-memory-client/agent_memory_client/client.py:2709

  • _resolve_search_memory forwards search_mode/hybrid_alpha/text_scorer, but it still ignores the tool-call argument optimize_query even though the tool schema exposes it and search_memory_tool() supports it. This means LLM callers can set optimize_query and it will be silently dropped. Consider parsing optimize_query from args and passing it through to search_memory_tool (or removing it from the schema if intentionally unsupported).
        max_results = args.get("max_results", 5)
        min_relevance = args.get("min_relevance")
        user_id = args.get("user_id")
        search_mode = args.get("search_mode", "semantic")
        hybrid_alpha = args.get("hybrid_alpha")
        text_scorer = args.get("text_scorer")

        return await self.search_memory_tool(
            query=query,
            search_mode=search_mode,
            hybrid_alpha=hybrid_alpha,
            text_scorer=text_scorer,
            topics=topics,
            entities=entities,
            memory_type=memory_type,
            max_results=max_results,
            min_relevance=min_relevance,
            user_id=user_id,

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…a (5 -> 10)

Bug fix:
- _resolve_search_memory defaulted max_results to 5, but the OpenAI tool
  schema and search_memory_tool signature both default to 10

Test:
- Extend test_memory_prompt_parameter_passing to assert search_mode,
  hybrid_alpha, and text_scorer are forwarded through MCP memory_prompt
@nkanu17 nkanu17 requested a review from Copilot March 27, 2026 02:25
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Both params are in the tool schema but were not extracted or passed
to search_memory_tool(), so LLM-provided values were silently ignored.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 2803 to 2808
"""Resolve eagerly_create_long_term_memory (and deprecated create_long_term_memory alias) function call."""
memories_data = args.get("memories")
if not memories_data:
raise ValueError(
"memories parameter is required for create_long_term_memory"
"memories parameter is required for eagerly_create_long_term_memory"
)
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The resolver explicitly supports the deprecated create_long_term_memory alias, but the error message only references eagerly_create_long_term_memory. This can be confusing when the caller invoked the deprecated name. Consider making the error message function-name-agnostic (e.g., just “memories parameter is required”) or mentioning both names.

Copilot uses AI. Check for mistakes.
Comment on lines +747 to +750
if distance_threshold is not None and search_mode != SearchModeEnum.SEMANTIC:
raise ValueError(
"distance_threshold is only supported for semantic search mode"
)
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raising a bare ValueError from an MCP tool often surfaces as an unstructured/internal error to tool clients. If the MCP framework supports structured tool errors, prefer raising the project’s standard “bad request”/tool validation exception type so callers reliably get a 4xx-style, user-actionable response.

Copilot uses AI. Check for mistakes.
Comment on lines 13 to 17
| **Storage** | Redis with vector indexing |
| **Search** | Semantic vector search |
| **Search** | Semantic, keyword, and hybrid search |
| **Capacity** | Unlimited (with compaction) |
| **Use Case** | Knowledge base, user preferences |
| **Indexing** | Vector embeddings + metadata |
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This page now advertises keyword/full-text and hybrid search, but the “Storage” and “Indexing” rows still only mention vector indexing/embeddings. Update these rows to reflect that keyword/hybrid require full-text indexing as well (so the capability description matches the underlying implementation expectations).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@vishal-bala vishal-bala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good - there are a couple bot comments that are worth covering.

Comparing this to #253, we could expand this PR to cover the rest of the gaps between the MCP and core AMS - tbd whether that larger scope is what we want right away but we can discuss that later.

Cataloging the items from #253 not covered in this PR here:

  • MCP memory_prompt still does not expose full SearchRequest parity:
    • event_date
    • recency_*
    • server_side_recency
  • MCP search_long_term_memory still does not expose:
    • event_date
    • recency_*
    • server_side_recency
  • Namespace injection still targets hydrate_memory_prompt instead of memory_prompt
  • MCP memory_prompt still cannot represent all REST modes:
    • session only
    • long_term_search=True
    • explicit long_term_search payload
  • MCP get_working_memory still differs from REST:
    • missing namespace, user_id, model_name, context_window_max
    • returns empty session instead of 404 on not found
  • Several REST capabilities still have no MCP equivalent:
    • forgetting
    • list sessions
    • delete working memory
    • summary view CRUD/run/list
    • task status
  • Python docs/examples still show stale REST-shaped memory_prompt() usage that does not match the current client API
  • Test coverage is still missing for:
    • event_date
    • recency controls
    • server_side_recency
    • memory_prompt mode selection
    • namespace injection for memory_prompt

@tylerhutcherson
Copy link
Copy Markdown
Contributor

Agree with @vishal-bala comments here and on Monday will want @abrookins and @bsbodden input -- we should go ahead and fix all of these gaps. The larger question - does it even make sense though to expose some of these params to the LLM? Like do we expect them to know how to use or toggle them appropriately? What is typical in the industry?

…text indexing

- _resolve_create_long_term_memory error message now says 'memories
  parameter is required' instead of naming a specific tool (handles
  both eagerly_create and deprecated create aliases)
- docs/long-term-memory.md Storage and Indexing rows now mention
  full-text indexing alongside vector indexing
@jit-ci
Copy link
Copy Markdown

jit-ci bot commented Mar 27, 2026

❌ Jit Scanner failed - Our team is investigating

Jit Scanner failed - Our team has been notified and is working to resolve the issue. Please contact support if you have any questions.


💡 Need to bypass this check? Comment @sera bypass to override.

@nkanu17
Copy link
Copy Markdown
Contributor Author

nkanu17 commented Mar 27, 2026

Agree with @vishal-bala comments here and on Monday will want @abrookins and @bsbodden input -- we should go ahead and fix all of these gaps. The larger question - does it even make sense though to expose some of these params to the LLM? Like do we expect them to know how to use or toggle them appropriately? What is typical in the industry?

@tylerhutcherson
I think a lot of these 'hyperparameters', we should let the devs/user configure them through a service config. They should not be pushed to the LLM.

Example:

If a developer wants hybrid search with BM25 and a 0.7 alpha, the alpha, recency config etc. parameters should be configured that at the server/session level and not hope the LLM picks the right values. Hybrid/Semantic is questionable for me but I can see merit in letting the LLM choose. Or split them into different tools. It even makes sense to make hybrid default for search.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants