-
Notifications
You must be signed in to change notification settings - Fork 331
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix SyntaxError: f-string: unmatched '{' error #1259
base: main
Are you sure you want to change the base?
Conversation
I have fixed the error in return f"b:{json.dumps({ "toolCallId": chunk.toolCallId, "toolName": chunk.toolName })}\n" SyntaxError: f-string: unmatched '{'
@dArk10R4 is attempting to deploy a commit to the Simon Farshid's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe pull request introduces minor stylistic changes to the Changes
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
WalkthroughThe update addresses a SyntaxError in the Changes
Files selected (1)
Files ignored (0)InstructionsEmoji Descriptions:
Interact with the Bot:
Execute a command using the format:
Available Commands:
Tips for Using @bot Effectively:
Need More Help?📚 Visit our documentation for detailed guides on using Entelligence.AI. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
python/assistant-stream/src/assistant_stream/serialization/data_stream.py
(1 hunks)
elif chunk.type == "tool-result": | ||
return f"a:{json.dumps({ "toolCallId": chunk.tool_call_id, "result": chunk.result })}\n" | ||
return f'a:{json.dumps({ "toolCallId": chunk.tool_call_id, "result": chunk.result })}\n' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Apply consistent f-string handling and consider refactoring
This line needs the same fix as the others. Additionally, given the similar pattern across all three changes, consider extracting this into a helper method.
Immediate fix:
- return f'a:{json.dumps({ "toolCallId": chunk.tool_call_id, "result": chunk.result })}\n'
+ data = {"toolCallId": chunk.tool_call_id, "result": chunk.result}
+ return f'a:{json.dumps(data)}\n'
Consider this broader refactoring to reduce duplication:
def _encode_tool_data(self, prefix: str, data: dict) -> str:
return f'{prefix}:{json.dumps(data)}\n'
Then use it like:
# Example usage:
return self._encode_tool_data('b', {
"toolCallId": chunk.tool_call_id,
"toolName": chunk.tool_name
})
elif chunk.type == "tool-call-delta": | ||
return f"c:{json.dumps({ "toolCallId": chunk.tool_call_id, "argsTextDelta": chunk.args_text_delta })}\n" | ||
return f'c:{json.dumps({ "toolCallId": chunk.tool_call_id, "argsTextDelta": chunk.args_text_delta })}\n' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Apply consistent f-string handling
Similar to the previous case, this line would benefit from separating the JSON structure from the f-string interpolation.
Apply this change for consistency:
- return f'c:{json.dumps({ "toolCallId": chunk.tool_call_id, "argsTextDelta": chunk.args_text_delta })}\n'
+ data = {"toolCallId": chunk.tool_call_id, "argsTextDelta": chunk.args_text_delta}
+ return f'c:{json.dumps(data)}\n'
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
return f'c:{json.dumps({ "toolCallId": chunk.tool_call_id, "argsTextDelta": chunk.args_text_delta })}\n' | |
data = {"toolCallId": chunk.tool_call_id, "argsTextDelta": chunk.args_text_delta} | |
return f'c:{json.dumps(data)}\n' |
@@ -15,11 +15,11 @@ def encode_chunk(self, chunk: AssistantStreamChunk) -> str: | |||
if chunk.type == "text-delta": | |||
return f"0:{json.dumps(chunk.text_delta)}\n" | |||
elif chunk.type == "tool-call-begin": | |||
return f"b:{json.dumps({ "toolCallId": chunk.tool_call_id, "toolName": chunk.tool_name })}\n" | |||
return f'b:{json.dumps({ "toolCallId": chunk.tool_call_id, "toolName": chunk.tool_name })}\n' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix potential f-string parsing issues
While changing quotes helps, the current implementation might still be prone to f-string parsing issues due to nested curly braces. A more robust approach would be to separate the JSON creation from the f-string interpolation.
Here's a clearer and more maintainable solution:
- return f'b:{json.dumps({ "toolCallId": chunk.tool_call_id, "toolName": chunk.tool_name })}\n'
+ data = {"toolCallId": chunk.tool_call_id, "toolName": chunk.tool_name}
+ return f'b:{json.dumps(data)}\n'
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
return f'b:{json.dumps({ "toolCallId": chunk.tool_call_id, "toolName": chunk.tool_name })}\n' | |
data = {"toolCallId": chunk.tool_call_id, "toolName": chunk.tool_name} | |
return f'b:{json.dumps(data)}\n' |
return f'b:{json.dumps({ "toolCallId": chunk.tool_call_id, "toolName": chunk.tool_name })}\n' | ||
elif chunk.type == "tool-call-delta": | ||
return f"c:{json.dumps({ "toolCallId": chunk.tool_call_id, "argsTextDelta": chunk.args_text_delta })}\n" | ||
return f'c:{json.dumps({ "toolCallId": chunk.tool_call_id, "argsTextDelta": chunk.args_text_delta })}\n' | ||
elif chunk.type == "tool-result": | ||
return f"a:{json.dumps({ "toolCallId": chunk.tool_call_id, "result": chunk.result })}\n" | ||
return f'a:{json.dumps({ "toolCallId": chunk.tool_call_id, "result": chunk.result })}\n' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Tests needed for data stream serialization
The codebase lacks test coverage for the data stream serialization functionality. You should add unit tests to verify:
- Text delta encoding (
type == "text-delta"
) - Tool call begin encoding (
type == "tool-call-begin"
) - Tool call delta encoding (
type == "tool-call-delta"
) - Tool result encoding (
type == "tool-result"
) - Special character handling in all JSON fields
- Proper newline handling in the encoded output
- Media type return value
- Async stream encoding
🔗 Analysis chain
Verify the changes with unit tests
While the syntax fixes look correct, please ensure these changes are covered by unit tests, especially testing with various types of tool names, IDs, and results that might contain special characters.
Let's check for existing tests:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Look for test files related to data_stream
fd -e py test_data_stream
Length of output: 27
Script:
#!/bin/bash
# Let's try a broader search for test files in the python/assistant-stream directory
fd -e py test python/assistant-stream
# Also search for imports of data_stream to see test coverage
rg "from assistant_stream.serialization.data_stream" -A 5
Length of output: 715
Script:
#!/bin/bash
# Let's search for test files in a different way and look for any tests related to serialization
rg -l "test.*serialization" python/assistant-stream/tests
# Also check if there are any test files directly in the package directory
fd -e py . python/assistant-stream/tests
# Let's also look at the actual implementation to understand the test requirements better
cat python/assistant-stream/src/assistant_stream/serialization/data_stream.py
Length of output: 1854
Thank you so much, this is great! |
🔍 Review Summary
Purpose:
encode_chunk
method to prevent runtime errors.Changes:
data_stream.py
for accurate JSON data formatting.Impact:
Original Description
No existing description found