feat: add LLM wrapper and interceptors for LLM calls#131
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces an LLM wrapper system with interceptors for LLM calls, enabling observability and control over LLM interactions. The implementation adds infrastructure for monitoring, tracing, and intercepting LLM operations before, after, and on error.
Key changes:
- New
LLMClientWrapperandLLMInterceptorRegistryfor wrapping LLM clients and managing interceptors - Modified workflow step execution to include
step_idin context for better traceability - Enhanced service layer to integrate LLM wrapper with interceptor registration methods
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/memu/llm/wrapper.py | Implements core LLM wrapper infrastructure including interceptor registry, client wrapper, and associated dataclasses for call context, request/response views, and usage tracking |
| src/memu/app/service.py | Integrates LLM wrapper into the service layer by adding interceptor registry, wrapping LLM clients with metadata, and exposing interceptor registration methods |
| src/memu/workflow/step.py | Ensures step_id is always included in step context for LLM call traceability |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
|
|
||
| def _build_embedding_response_view(response: Sequence[Sequence[float]]) -> LLMResponseView: | ||
| vector_dim = len(response[0]) if response else 0 |
There was a problem hiding this comment.
Potential IndexError if response is an empty sequence of sequences. The condition checks if response is truthy, but an empty list evaluates to False, while a list containing empty sequences like [[]] would pass the check but fail on response[0]. Consider checking response and response[0] or handling the case where response contains empty sequences.
| await self._run_before(snapshot.before, call_ctx, request_view) | ||
| start_time = time.perf_counter() | ||
| try: | ||
| result = call_fn() |
There was a problem hiding this comment.
The call_fn() invocation is missing await. Since call_fn is expected to return an awaitable (as indicated by lines 363-364), this should be result = await call_fn() to properly await the coroutine. The current code relies on checking inspect.isawaitable(result) afterwards, but this creates an unawaited coroutine that may trigger runtime warnings.
af5b7f9 to
c54857a
Compare
No description provided.