Skip to content

Commit a331eb4

Browse files
fix: resolve critical architectural issues from reviewer feedback
- Fix IndexRegistry singleton broken in knowledge/index.py (P0) - Fix sync/async unified dispatch default inconsistency - Fix message slicing logic that drops first user message - Add thread safety to all singleton patterns with double-checked locking - Enhance Ollama adapter to handle both dict and list-shaped tool payloads Addresses feedback from Greptile, Gemini, CodeRabbit, and Copilot reviewers. Fixes remaining critical issues in architectural gap PR. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
1 parent e64c245 commit a331eb4

File tree

5 files changed

+43
-18
lines changed

5 files changed

+43
-18
lines changed

src/praisonai-agents/praisonaiagents/agent/agent.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,11 @@ def __init__(self):
182182
def get_default_instance():
183183
"""Get default global registry for backward compatibility."""
184184
if not hasattr(ServerRegistry, '_default_instance'):
185-
ServerRegistry._default_instance = ServerRegistry()
185+
import threading
186+
# Double-checked locking pattern for thread safety
187+
with threading.Lock():
188+
if not hasattr(ServerRegistry, '_default_instance'):
189+
ServerRegistry._default_instance = ServerRegistry()
186190
return ServerRegistry._default_instance
187191

188192
def is_server_started(self, port: int) -> bool:

src/praisonai-agents/praisonaiagents/agent/chat_mixin.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -580,9 +580,10 @@ def _chat_completion(self, messages, temperature=1.0, tools=None, stream=True, r
580580
)
581581
else:
582582
# Non-streaming with custom LLM - direct execution
583+
has_system = bool(messages and messages[0].get('role') == 'system')
583584
final_response = self.llm_instance.get_response(
584-
prompt=messages[1:],
585-
system_prompt=messages[0]['content'] if messages and messages[0]['role'] == 'system' else None,
585+
prompt=messages[1:] if has_system else messages,
586+
system_prompt=messages[0]['content'] if has_system else None,
586587
temperature=temperature,
587588
tools=formatted_tools if formatted_tools else None,
588589
verbose=self.verbose,
@@ -1834,8 +1835,8 @@ async def _achat_impl(self, prompt, temperature, tools, output_json, output_pyda
18341835
formatted_tools = self._format_tools_for_completion(tools)
18351836

18361837
# NEW: Unified protocol dispatch path (Issue #1304) - Async version
1837-
# Check if unified dispatch is enabled (opt-in for backward compatibility)
1838-
if getattr(self, '_use_unified_llm_dispatch', False):
1838+
# Enable unified dispatch by default for DRY and feature parity (sync/async consistent)
1839+
if getattr(self, '_use_unified_llm_dispatch', True):
18391840
# Use composition instead of runtime class mutation for safety
18401841
response = await self._execute_unified_achat_completion(
18411842
messages=messages,

src/praisonai-agents/praisonaiagents/knowledge/index.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,13 @@ def __init__(self):
109109
def default(cls) -> "IndexRegistry":
110110
"""Get a default global registry instance for convenience."""
111111
if not hasattr(cls, '_default_instance'):
112-
cls._default_instance = cls()
112+
import threading
113+
# Use lock for thread safety
114+
if not hasattr(cls, '_init_lock'):
115+
cls._init_lock = threading.Lock()
116+
with cls._init_lock:
117+
if not hasattr(cls, '_default_instance'):
118+
cls._default_instance = cls()
113119
return cls._default_instance
114120

115121
def register(self, name: str, factory: Callable[..., IndexProtocol]) -> None:
@@ -136,7 +142,7 @@ def clear(self) -> None:
136142

137143
def get_index_registry() -> IndexRegistry:
138144
"""Get the global index registry instance."""
139-
return IndexRegistry()
145+
return IndexRegistry.default()
140146

141147
class KeywordIndex:
142148
"""

src/praisonai-agents/praisonaiagents/llm/adapters/__init__.py

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -126,16 +126,24 @@ def recover_tool_calls_from_text(self, response_text: str, tools: List[Dict[str,
126126
try:
127127
import json
128128
response_json = json.loads(response_text.strip())
129-
if isinstance(response_json, dict) and "name" in response_json:
130-
# Convert Ollama format to standard tool_calls format
131-
return [{
132-
"id": f"call_{response_json['name']}_{hash(response_text) % 10000}",
133-
"type": "function",
134-
"function": {
135-
"name": response_json["name"],
136-
"arguments": json.dumps(response_json.get("arguments", {}))
137-
}
138-
}]
129+
130+
# Normalize to list so both single and multi-tool payloads are supported
131+
if isinstance(response_json, dict):
132+
response_json = [response_json]
133+
134+
if isinstance(response_json, list):
135+
tool_calls: List[Dict[str, Any]] = []
136+
for idx, tool_json in enumerate(response_json):
137+
if isinstance(tool_json, dict) and "name" in tool_json:
138+
tool_calls.append({
139+
"id": f"call_{tool_json['name']}_{idx}_{hash(response_text) % 10000}",
140+
"type": "function",
141+
"function": {
142+
"name": tool_json["name"],
143+
"arguments": json.dumps(tool_json.get("arguments", {}))
144+
}
145+
})
146+
return tool_calls if tool_calls else None
139147
except (json.JSONDecodeError, TypeError, KeyError):
140148
pass
141149

src/praisonai-agents/praisonaiagents/memory/adapters/registry.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,13 @@ def __init__(self):
3737
def get_default_memory_registry() -> MemoryAdapterRegistry:
3838
"""Get the default global registry instance for convenience."""
3939
if not hasattr(get_default_memory_registry, '_default_instance'):
40-
get_default_memory_registry._default_instance = MemoryAdapterRegistry()
40+
import threading
41+
# Use lock for thread safety
42+
if not hasattr(get_default_memory_registry, '_init_lock'):
43+
get_default_memory_registry._init_lock = threading.Lock()
44+
with get_default_memory_registry._init_lock:
45+
if not hasattr(get_default_memory_registry, '_default_instance'):
46+
get_default_memory_registry._default_instance = MemoryAdapterRegistry()
4147
return get_default_memory_registry._default_instance
4248

4349

0 commit comments

Comments
 (0)