Skip to content

Commit e386411

Browse files
fix: address critical architectural issues identified by reviewers
- Fix LiteLLM provider TypeError by stripping 'provider' key from config - Fix DB adapter write-side gap allowing state store writes without conversation store - Fix tool stacks cleanup on LangfuseSink shutdown - Fix limit=0 logic error in get_runs/get_traces - Add exception chaining for ImportError - Add error handling for state store reads Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
1 parent f6659b6 commit e386411

3 files changed

Lines changed: 37 additions & 15 deletions

File tree

src/praisonai/praisonai/db/adapter.py

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -290,10 +290,7 @@ def on_run_start(
290290
metadata: Optional[Dict[str, Any]] = None,
291291
) -> None:
292292
"""Called when a new run (turn) starts."""
293-
if not self._conversation_store:
294-
return
295-
296-
# Store run start in state if available
293+
# Store run start in state if available (even without conversation store)
297294
if self._state_store:
298295
run_key = f"run:{session_id}:{run_id}"
299296
self._state_store.set(run_key, {
@@ -304,6 +301,10 @@ def on_run_start(
304301
"status": "running",
305302
"metadata": metadata or {}
306303
})
304+
305+
# Early return only applies to conversation store operations
306+
if not self._conversation_store:
307+
return
307308

308309
def on_run_end(
309310
self,
@@ -347,12 +348,16 @@ def get_runs(
347348

348349
runs = []
349350
for k in keys:
350-
run_data = self._state_store.get(k)
351-
if run_data:
351+
try:
352+
run_data = self._state_store.get(k)
353+
except Exception as exc:
354+
logger.warning("Failed to load run key '%s': %s", k, exc)
355+
continue
356+
if isinstance(run_data, dict):
352357
runs.append(run_data)
353358

354359
runs.sort(key=lambda r: r.get("started_at", 0), reverse=True)
355-
return runs[:limit] if limit else runs
360+
return runs[:limit] if limit is not None else runs
356361

357362
def export_session(
358363
self,
@@ -530,16 +535,20 @@ def get_traces(
530535

531536
traces = []
532537
for k in keys:
533-
trace_data = self._state_store.get(k)
534-
if trace_data:
538+
try:
539+
trace_data = self._state_store.get(k)
540+
except Exception as exc:
541+
logger.warning("Failed to load trace key '%s': %s", k, exc)
542+
continue
543+
if isinstance(trace_data, dict):
535544
traces.append(trace_data)
536545

537546
if session_id is not None:
538547
traces = [t for t in traces if t.get("session_id") == session_id]
539548
if user_id is not None:
540549
traces = [t for t in traces if t.get("user_id") == user_id]
541550
traces.sort(key=lambda t: t.get("started_at", 0), reverse=True)
542-
return traces[:limit] if limit else traces
551+
return traces[:limit] if limit is not None else traces
543552

544553
def close(self) -> None:
545554
"""Close all database connections."""

src/praisonai/praisonai/llm/registry.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -279,16 +279,19 @@ def __init__(self, model_id: str, config: Optional[Dict[str, Any]] = None):
279279
def generate(self, prompt: str, **kwargs):
280280
try:
281281
import litellm # lazy
282-
except ImportError:
282+
except ImportError as err:
283283
raise ImportError(
284284
"LiteLLM is required for built-in providers. "
285285
"Install with: pip install litellm"
286-
)
287-
full_model = f"{self.config.get('provider', '')}/{self.model_id}".strip("/")
286+
) from err
287+
provider_prefix = self.config.get("provider", "")
288+
full_model = f"{provider_prefix}/{self.model_id}".strip("/") if provider_prefix else self.model_id
289+
# Exclude internal 'provider' key from litellm.completion kwargs
290+
passthrough_config = {k: v for k, v in self.config.items() if k != "provider"}
288291
return litellm.completion(
289-
model=full_model or self.model_id,
292+
model=full_model,
290293
messages=[{"role": "user", "content": prompt}],
291-
**{**self.config, **kwargs},
294+
**{**passthrough_config, **kwargs},
292295
)
293296

294297
def _make_litellm_factory(provider_prefix: str):

src/praisonai/praisonai/observability/langfuse.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,16 @@ def close(self) -> None:
297297
try:
298298
# Close any remaining spans
299299
with self._lock:
300+
# End any in-flight tool spans
301+
for stack in self._tool_stacks.values():
302+
while stack:
303+
try:
304+
stack.pop().end()
305+
except Exception:
306+
pass
307+
self._tool_stacks.clear()
308+
309+
# End root spans
300310
for span in self._spans.values():
301311
try:
302312
span.end()

0 commit comments

Comments
 (0)