-
Couldn't load subscription status.
- Fork 5
Langfuse: Add assistant_id as tag in traces #368
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
Conversation
WalkthroughIntroduces an optional tags parameter to LangfuseTracer.start_trace and propagates a tag from process_response by passing the request.assistant_id. The tags are forwarded to the underlying Langfuse trace creation. No other logic or public APIs are altered beyond the updated method signature. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant API as API Route (process_response)
participant Tracer as LangfuseTracer
participant LF as Langfuse Backend
Client->>API: HTTP request
API->>Tracer: start_trace(name, input, metadata, tags=[assistant_id])
Note right of Tracer: New: pass-through tags
Tracer->>LF: create trace(name, input, metadata, tags)
LF-->>Tracer: trace initialized
Tracer-->>API: trace handle
API-->>Client: Response (flow unchanged)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/app/api/routes/responses.py (2)
197-204: Tag overwrite drops assistant_id; include both assistant_id and response.idupdate_trace replaces the tag set, removing the assistant_id you added. Include both IDs or rely on tracer to merge.
Apply this minimal fix if you don’t update the tracer:
- tracer.update_trace( - tags=[response.id], + tracer.update_trace( + tags=[request.assistant_id, response.id], output={ "status": "success", "message": response.output_text, "error": None, }, )
95-101: Bug: file search results builder uses the wrong source list (always empty)You iterate over the accumulating results list instead of the tool call’s results, producing empty chunks.
def get_file_search_results(response): results: list[FileResultChunk] = [] for tool_call in response.output: if tool_call.type == "file_search_call": - results.extend( - [FileResultChunk(score=hit.score, text=hit.text) for hit in results] - ) + if getattr(tool_call, "results", None): + results.extend( + FileResultChunk(score=hit.score, text=hit.text) + for hit in tool_call.results + ) return results
🧹 Nitpick comments (2)
backend/app/api/routes/responses.py (1)
42-44: Avoid bare except; log parse failures explicitlyCatching all exceptions hides real issues. Scope the except and log.
- except: - pass + except ValueError as parse_err: + logger.debug("Failed to parse OpenAI error body: %s", parse_err)backend/app/core/langfuse/langfuse.py (1)
40-43: Minor: fetch_traces expects a list for tagsPass a list to avoid SDK edge cases.
- if response_id: - traces = self.langfuse.fetch_traces(tags=response_id).data + if response_id: + traces = self.langfuse.fetch_traces(tags=[response_id]).data
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
backend/app/api/routes/responses.py(1 hunks)backend/app/core/langfuse/langfuse.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/app/api/routes/responses.py (1)
backend/app/tests/api/routes/test_assistants.py (1)
assistant_id(25-26)
🔇 Additional comments (1)
backend/app/api/routes/responses.py (1)
147-148: Assistant tag added to trace start – good, but watch for later overwriteThis correctly attaches assistant_id at trace start. Note that later updates overwrite tags, which will drop this assistant_id unless addressed (see below).
Added assistant_id as a tag in Langfuse tracing to improve observability and filtering capabilities.
Checklist
Before submitting a pull request, please ensure that you mark these task.
fastapi run --reload app/main.pyordocker compose upin the repository root and test.Notes
Please add here if any other information is required for the reviewer.
Summary by CodeRabbit