Conversation
…dd-langsmith/langfuse-support-in-python
There was a problem hiding this comment.
Pull Request Overview
This PR adds Langfuse integration to the tracing system, allowing users to log POML calls to Langfuse alongside existing backends (Weave, AgentOps, MLflow). The integration follows the established pattern of requiring local tracing to be enabled first.
- Add Langfuse as a new tracing backend option
- Implement Langfuse logging integration with prompt creation and observation tracking
- Include manual test examples for both direct Langfuse usage and POML integration
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| python/poml/api.py | Adds Langfuse backend support and refactors result handling for consistency |
| python/poml/integration/langfuse.py | Implements Langfuse-specific logging functionality |
| python/tests/manual/example_langfuse_poml.py | Manual test demonstrating POML with Langfuse tracing |
| python/tests/manual/example_langfuse_original.py | Manual test showing direct Langfuse usage |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| enabled = [enabled] | ||
|
|
||
| global _trace_enabled, _trace_dir, _weave_enabled, _agentops_enabled, _mlflow_enabled | ||
| global _trace_enabled, _trace_dir, _weave_enabled, _agentops_enabled, _mlflow_enabled, _langsmith_enabled, _langfuse_enabled |
There was a problem hiding this comment.
The variable _langsmith_enabled is referenced in the global declaration but is not defined anywhere in the visible code. This will cause a NameError when the function is called.
| global _trace_enabled, _trace_dir, _weave_enabled, _agentops_enabled, _mlflow_enabled, _langsmith_enabled, _langfuse_enabled | |
| global _trace_enabled, _trace_dir, _weave_enabled, _agentops_enabled, _mlflow_enabled, _langfuse_enabled |
| type="text", | ||
| prompt=prompt | ||
| ) | ||
|
|
There was a problem hiding this comment.
The variable prompt_client is used before it's defined. It's defined on line 18 but used on line 15, which will cause a NameError.
| prompt_client = client.create_prompt( | |
| name=name, | |
| type="text", | |
| prompt=prompt | |
| ) | |
| @observe(name=name) | |
| def poml(prompt, context, stylesheet): | |
| client.update_current_generation(prompt=prompt_client) | |
| return result |
| prompt=prompt | ||
| ) | ||
|
|
||
| poml(prompt=prompt, context=context, stylesheet=stylesheet) |
There was a problem hiding this comment.
The function does not return the result of the poml call, but the function signature suggests it should return Any. Add return before the function call.
| poml(prompt=prompt, context=context, stylesheet=stylesheet) | |
| return poml(prompt=prompt, context=context, stylesheet=stylesheet) |
|
|
||
| if trace_record is not None: | ||
| trace_record["result"] = result | ||
| trace_record["result"] = result_dict |
There was a problem hiding this comment.
The variable result_dict may not be defined in all code paths. When format is "raw", result_dict is never assigned, but result is used directly. This should be trace_record["result"] = result to maintain consistency.
| trace_record["result"] = result_dict | |
| trace_record["result"] = result if format == "raw" else result_dict |
Summary
langsmithandlangfusetracing integrationsTesting
npm run build-webviewnpm run build-clinpm run lintnpm test(failed: hung at compile step)python -m pytest python/testshttps://chatgpt.com/codex/tasks/task_e_6899af15e0b0832e8c558f1fa495399d