memo api v2 user_id fix - #563
Conversation
GitHub Actions
Failure
GitHub Actions
Failure
|
@parthshr370 is attempting to deploy a commit to the Proxy Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes introduce template-based default value substitution in the schema processing logic, allowing placeholders like Changes
Sequence Diagram(s)sequenceDiagram
participant Executor as FunctionExecutor
participant Processor as inject_required_but_invisible_defaults
participant Context as Context Dict
Executor->>Processor: Call with schema, input_data, context
alt Default value is a template
Processor->>Context: Lookup key in context
Context-->>Processor: Return value
Processor->>Processor: Substitute default value
else No template
Processor->>Processor: Use static default value
end
Processor-->>Executor: Return processed data
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
✨ No issues found! Your code is sparkling clean! ✨ Need help? Join our Discord for support! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
backend/aci/common/processor.py (2)
54-54: Improve documentation specificityConsider being more specific about the template format and provide an example.
- Supports template substitution for values like {{linked_account_owner_id}}. + Supports template substitution for values using {{key}} format, e.g., {{linked_account_owner_id}}.
72-74: Enhance error message with more contextThe error message could be more helpful by including available context keys.
- raise Exception( - f"Template variable '{template_key}' not found in context for property: {prop}" - ) + available_keys = list(context.keys()) if context else [] + raise Exception( + f"Template variable '{template_key}' not found in context for property: {prop}. " + f"Available keys: {available_keys}" + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/aci/common/processor.py(3 hunks)backend/aci/server/function_executors/base_executor.py(1 hunks)backend/apps/mem0/functions.json(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
backend/apps/mem0/functions.json (1)
Learnt from: jiwei-aipolabs
PR: #538
File: backend/apps/microsoft_calendar/functions.json:276-308
Timestamp: 2025-07-22T19:31:44.823Z
Learning: In backend/apps/microsoft_calendar/functions.json and similar API function definitions, the additionalProperties: false restriction on query objects is intentionally designed for strict validation, even if it blocks some legitimate OData/HTTP headers. The user prefers explicit whitelisting of parameters over flexible schema validation.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Format, Lint, and Test
🔇 Additional comments (10)
backend/aci/common/processor.py (2)
51-51: LGTM: Function signature enhancementGood addition of the optional
contextparameter with a sensible default value ofNone.
86-86: LGTM: Context propagationCorrect propagation of the context parameter to recursive calls.
backend/aci/server/function_executors/base_executor.py (1)
70-74: LGTM: Clean context injection implementationThe implementation correctly creates the context dictionary and passes it to the processor function. The comments clearly explain the purpose of the template substitution feature.
backend/apps/mem0/functions.json (7)
53-55: LGTM: Template default value for user_idCorrectly adds the template default value for automatic user_id injection while maintaining the description.
74-74: LGTM: user_id removed from visible parametersCorrectly removes user_id from the visible array while keeping it in the required array, ensuring it's automatically injected but invisible to the LLM.
122-124: LGTM: Consistent template implementationProperly implements the same template pattern for the search function.
127-127: LGTM: Filters object made invisibleCorrectly makes the entire filters object invisible since user_id is the only required field and should be auto-injected.
182-184: LGTM: Consistent pattern in GET_MEMORIESMaintains consistency with the template substitution pattern across all affected functions.
187-187: LGTM: Consistent visibility handlingProperly removes user_id from visibility while maintaining the required constraint.
53-187: user_id template substitution is consistent across all MEM0 functionsAll six MEM0 endpoints were checked:
- MEM0__ADD_MEMORY, MEM0__SEARCH_MEMORIES, and MEM0__GET_MEMORIES include
user_idwithdefault: "{{linked_account_owner_id}}".- MEM0__GET_MEMORY, MEM0__UPDATE_MEMORY, and MEM0__DELETE_MEMORY do not reference
user_id, as expected.No further updates are needed.
| default_value = subschema["default"] | ||
| # Handle template substitution like {{linked_account_owner_id}} | ||
| if isinstance(default_value, str) and default_value.startswith("{{") and default_value.endswith("}}"): | ||
| template_key = default_value[2:-2] # Extract the key between {{ }} | ||
| if context and template_key in context: | ||
| default_value = context[template_key] | ||
| else: | ||
| raise Exception( | ||
| f"Template variable '{template_key}' not found in context for property: {prop}" | ||
| ) | ||
| input_data[prop] = default_value |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Verify template parsing robustness
The template parsing logic uses simple string operations. Consider edge cases and potential security implications.
The current implementation:
- Uses simple string slicing which could be fragile with malformed templates
- Doesn't validate the template key format
- Could potentially allow injection if context values aren't sanitized
Consider adding validation for the template key format and ensuring context values are properly sanitized.
🏁 Script executed:
#!/bin/bash
# Verify if there are any other template patterns or edge cases in the codebase
rg -A 3 -B 3 "{{.*}}" --type jsonLength of output: 117
🏁 Script executed:
#!/bin/bash
# Search for template patterns like {{key}} across the repository
rg -n '\{\{[^}]\+\}\}'Length of output: 24
Improve template parsing robustness in backend/aci/common/processor.py
The current logic around default‐value templating relies on simple startswith/endswith checks and unvalidated slicing, which may break on malformed inputs or enable unexpected values from context. To harden this:
• Validate the extracted template_key against an explicit pattern (e.g. ^[A-Za-z0-9_]+$) before lookup.
• Use a regex-based capture (e.g. re.match(r'^\{\{([A-Za-z0-9_]+)\}\}$', default_value)) instead of manual slicing.
• Sanitize or type‐check the resolved context[template_key] to prevent injection or misuse downstream.
Suggested diff snippet:
- if isinstance(default_value, str) and default_value.startswith("{{") and default_value.endswith("}}"):
- template_key = default_value[2:-2]
+ import re
+ if isinstance(default_value, str):
+ m = re.match(r'^\{\{([A-Za-z0-9_]+)\}\}$', default_value)
+ if m:
+ template_key = m.group(1)
+ else:
+ raise ValueError(f"Invalid template syntax for property '{prop}': {default_value}")
if context and template_key in context:
# Optional: further sanitize or validate context[template_key] here
default_value = context[template_key]
else:
raise KeyError(
f"Template variable '{template_key}' not found in context for property: {prop}"
)This enforces a strict key format, avoids fragile string slicing, and makes it clear where to apply sanitization.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In backend/aci/common/processor.py around lines 65 to 75, improve the template
parsing by replacing the manual startswith/endswith checks and slicing with a
regex match that captures the template key using a pattern like
^\{\{([A-Za-z0-9_]+)\}\}$. Validate the extracted template_key against this
pattern before using it. After retrieving the value from context, add
sanitization or type-checking to ensure it is safe and appropriate for
downstream use. This will make the template handling more robust and secure.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
cubic analysis
2 issues found across 3 files • Review in cubic
React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai to give feedback, ask questions, or re-run the review.
|
|
||
|
|
||
| def inject_required_but_invisible_defaults(parameters_schema: dict, input_data: dict) -> dict: | ||
| def inject_required_but_invisible_defaults(parameters_schema: dict, input_data: dict, context: dict = None) -> dict: |
There was a problem hiding this comment.
The context parameter is annotated as dict but defaulted to None, producing an invalid type hint and causing static-analysis noise.
Prompt for AI agents
Address the following comment on backend/aci/common/processor.py at line 51:
<comment>The context parameter is annotated as dict but defaulted to None, producing an invalid type hint and causing static-analysis noise.</comment>
<file context>
@@ -48,9 +48,10 @@ def filter(schema: dict) -> dict:
return filter(filtered_parameters_schema)
-def inject_required_but_invisible_defaults(parameters_schema: dict, input_data: dict) -> dict:
+def inject_required_but_invisible_defaults(parameters_schema: dict, input_data: dict, context: dict = None) -> dict:
"""
Recursively injects required but invisible properties with their default values into the input data.
</file context>
| def inject_required_but_invisible_defaults(parameters_schema: dict, input_data: dict, context: dict = None) -> dict: | |
| def inject_required_but_invisible_defaults(parameters_schema: dict, input_data: dict, context: dict | None = None) -> dict: |
| }, | ||
| "required": ["user_id"], | ||
| "visible": ["user_id"], | ||
| "visible": [], |
There was a problem hiding this comment.
Making user_id invisible here while allowing additionalProperties true lets callers supply their own user_id value and bypass the intended default injection.
Prompt for AI agents
Address the following comment on backend/apps/mem0/functions.json at line 127:
<comment>Making user_id invisible here while allowing additionalProperties true lets callers supply their own user_id value and bypass the intended default injection.</comment>
<file context>
@@ -118,11 +119,12 @@
"properties": {
"user_id": {
"type": "string",
- "description": "Filter memories by a specific user ID"
+ "description": "Filter memories by a specific user ID",
+ "default": "{{linked_account_owner_id}}"
}
},
"required": ["user_id"],
</file context>
| "visible": [], | |
| "visible": ["user_id"], |
GitHub Actions
Failure
GitHub Actions
Failure
1 participant
🏷️ Ticket
LLM hallucinates user_id during tool call of mem0 api v2
📝 Description
Add a function to inject linked_account_owner_id to the user_id field and make it invisible for the LLM to not edit
🎥 Demo (if applicable)
📸 Screenshots (if applicable)
✅ Checklist
Summary by cubic
Fixed an issue where the mem0 API v2 could hallucinate the user_id by automatically injecting the correct linked_account_owner_id and making it invisible to the LLM.
Summary by CodeRabbit
New Features
Style