-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Fixed JSON formatting #3475
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
Fixed JSON formatting #3475
Conversation
| sources_branch = None | ||
|
|
||
| # Try to find an existing "Sources Used" branch | ||
| for sub_child in child.children: | ||
| if "Sources Used" in str(sub_child.label): | ||
| sources_branch = sub_child | ||
| break | ||
| else: | ||
| sources_branch = child.add("Sources Used") | ||
| sources_branch.add(f"✅ {memory_type} ({query_time_ms:.2f}ms)") | ||
| break | ||
|
|
||
| # If not found, create it under this child | ||
| if sources_branch is None: | ||
| sources_branch = child.add("🧠 Sources Used") | ||
|
|
||
| sources_branch.add(f"✅ {memory_type} ({query_time_ms:.2f}ms)") | ||
| break | ||
|
|
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.
This is was a logic change done, because of linter.
| sources_branch = None | ||
|
|
||
| # Try to find an existing "Sources Used" branch | ||
| for sub_child in child.children: | ||
| if "Sources Used" in str(sub_child.label): | ||
| sources_branch = sub_child | ||
| break | ||
| else: | ||
|
|
||
| # If not found, create it under this child | ||
| if sources_branch is None: | ||
| sources_branch = child.add("🧠 Sources Used") | ||
| sources_branch.add(f"❌ {memory_type} - Error: {error}") | ||
| break | ||
|
|
||
| sources_branch.add(f"❌ {memory_type} - Error: {error}") | ||
| break | ||
|
|
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.
This one as well
|
Requesting @lorenzejay, @lucasgomide for review |
|
|
||
| sources_branch.add(f"✅ {memory_type} ({query_time_ms:.2f}ms)") | ||
| break | ||
|
|
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.
Bug: Memory Query Handling Fails
The refactored logic in handle_memory_query_completed and handle_memory_query_failed incorrectly replaces the for...else construct. The sources_branch variable is reassigned to each inner_child during iteration, making the subsequent if sources_branch is None: check ineffective. This leads to new memory entries being added to the wrong child when no "Sources Used" branch is found, or duplicated if one is found and updated.
Additional Locations (1)
| def __init__(self, verbose: bool = False): | ||
| self.console = Console(width=None) | ||
| self.verbose = verbose | ||
| self.tool_usage_counts: dict[str, int] = {} |
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.
Bug: Instance Variable Shadows Class Variable
The tool_usage_counts instance variable, initialized in __init__, shadows the class variable of the same name. This prevents tool usage from being tracked globally across all instances, leading to each instance having its own isolated count instead of a shared one.


This fixes the tool input console formatting
Fixes #3474