Skip to content

feat(prompts): add tool function support #3153

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

Merged
merged 14 commits into from
Jul 21, 2025

Conversation

galzilber
Copy link
Contributor

@galzilber galzilber commented Jul 20, 2025

  • I have added tests that cover my changes.
  • If adding a new instrumentation or changing an existing one, I've added screenshots from some observability platform showing the change.
  • PR name follows conventional commits format: feat(instrumentation): ... or fix(instrumentation): ....
  • (If applicable) I have updated the documentation accordingly.

Important

Add support for tool functions and JSON schema response formats in prompt management, with corresponding tests.

  • New Features:
    • Added tool_choice and tools to ModelConfig in model.py for specifying tool functions.
    • Added ResponseFormat to ModelConfig in model.py for JSON schema response formats.
    • Updated render_prompt() in client.py to handle tool functions and clear tool_choice if no tools are configured.
  • Tests:
    • Added test_prompt_management_with_tools() and test_prompt_management_with_response_format() in test_prompt_management.py to validate tool function and response format handling.
    • Added YAML test files for tool and response format scenarios.

This description was created by Ellipsis for c8a1433. You can customize this summary. It will automatically update as commits are pushed.


Summary by CodeRabbit

  • New Features
    • Added support for specifying tool choices and collections of tools in model configuration, allowing enhanced customization with tool-related metadata.
    • Improved prompt rendering logic to automatically clear tool selections when no tools are configured.
  • Tests
    • Added tests for prompts using tool calls and structured JSON schema response formats to ensure correct behavior and data validation.

Copy link

coderabbitai bot commented Jul 20, 2025

"""

Walkthrough

Three new Pydantic models—ToolFunction, Tool, and ResponseFormat—were added to represent function-based tools and JSON schema response formats. The ModelConfig class now includes optional fields for tool choice, tools list, and response format. The render_prompt method in PromptRegistryClient clears tool_choice if no tools are configured. New tests cover prompts using tools and response formats. Test cassettes were added or updated to reflect these changes and API interaction details.

Changes

File Change Summary
traceloop/sdk/prompts/model.py Added ToolFunction, Tool, and ResponseFormat Pydantic models; extended ModelConfig with optional tool_choice, tools, and response_format fields.
traceloop/sdk/prompts/client.py Updated render_prompt to reset tool_choice to None when llm_config.tools is empty or not set.
tests/test_prompt_management.py Added tests for prompt management with tool calls and JSON schema response formats, including new prompt JSON definitions and validation logic.
tests/cassettes/test_prompt_management/*.yaml Added new cassettes for prompt management with tools and response formats; updated existing cassette with new request/response details and headers.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant PromptRegistryClient
    participant ModelConfig

    User->>PromptRegistryClient: Call render_prompt()
    PromptRegistryClient->>ModelConfig: Check llm_config.tools and tool_choice
    alt tools empty or None
        PromptRegistryClient->>ModelConfig: Set tool_choice to None
    end
    PromptRegistryClient-->>User: Return rendered prompt
Loading

Suggested reviewers

  • nirga

Poem

In the warren of code, new tools appear,
With functions and choices, the purpose is clear.
ModelConfig now knows which tool to deploy,
And formats responses with schema joy!
When tools are gone, choice resets with care,
A bunny’s delight in code everywhere! 🐇✨
"""


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c8a1433 and fe1311c.

📒 Files selected for processing (1)
  • packages/traceloop-sdk/traceloop/sdk/prompts/client.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/traceloop-sdk/traceloop/sdk/prompts/client.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Test Packages (3.9)
  • GitHub Check: Build Packages (3.11)
  • GitHub Check: Lint
✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed everything up to 025b928 in 46 seconds. Click for details.
  • Reviewed 30 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/traceloop/sdk/prompts/model.py:41
  • Draft comment:
    Consider specifying a more explicit type for the 'parameters' field in ToolFunction (e.g. Dict[str, Any]) and adding a brief docstring to clarify its expected structure.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
2. packages/traceloop-sdk/traceloop/sdk/prompts/model.py:46
  • Draft comment:
    Using Literal['function'] for the Tool type is fine if only one tool type is expected, but consider a more flexible design if additional tool types may be needed in the future.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
3. packages/traceloop-sdk/traceloop/sdk/prompts/model.py:59
  • Draft comment:
    Adding 'tool_choice' and 'tools' to ModelConfig is a good start. Consider including field descriptions or validators to clearly document their intended use and expected values.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_MpJrhouXmlj5vdDo

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
packages/traceloop-sdk/traceloop/sdk/prompts/model.py (3)

41-44: Consider using a more specific type for the parameters field.

The implementation is correct, but the parameters field could benefit from more specific typing. According to OpenAI's function calling specification, parameters should follow JSON Schema format.

Consider this improvement:

-    parameters: dict
+    parameters: dict[str, any]  # or consider using a JSONSchema type if available

Alternatively, you could add validation to ensure the parameters follow the expected JSON Schema structure.


46-49: Remove trailing whitespace and consider future extensibility.

The Tool class structure is well-designed using discriminated unions. Minor formatting issue: remove the trailing whitespace on line 49.

 class Tool(RegistryObjectBaseModel):
     type: Literal["function"]
     function: ToolFunction
-    
+

The design with Literal["function"] is excellent as it allows for future extension with other tool types while maintaining type safety.


59-60: Consider more specific typing for the tool_choice field.

The implementation correctly adds optional tool support to the model configuration. However, the tool_choice field could benefit from more specific typing.

According to OpenAI's API specification, tool_choice typically accepts specific values:

-    tool_choice: Optional[str] = None
+    tool_choice: Optional[Literal["none", "auto", "required"]] = None

Alternatively, if you need to support specifying a particular tool, you might need a Union type to handle both string literals and tool specification objects.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a750944 and 025b928.

📒 Files selected for processing (1)
  • packages/traceloop-sdk/traceloop/sdk/prompts/model.py (2 hunks)

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed c6c2a3c in 38 seconds. Click for details.
  • Reviewed 36 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/traceloop/sdk/prompts/model.py:1
  • Draft comment:
    Consider grouping and ordering imports consistently. Moving 'from typing_extensions import Annotated' after the pydantic import may conflict with common import grouping practices.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
2. packages/traceloop-sdk/traceloop/sdk/prompts/model.py:41
  • Draft comment:
    The additional blank line between ToolFunction and Tool improves readability. Ensure consistency with the rest of the codebase.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
3. packages/traceloop-sdk/traceloop/sdk/prompts/model.py:61
  • Draft comment:
    A blank line was added after the 'tools' field in ModelConfig. Verify that this formatting aligns with your project's style guidelines.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_S4OwX1ksrrlZZ1uL

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed cb6905f in 38 seconds. Click for details.
  • Reviewed 17 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/traceloop/sdk/prompts/client.py:78
  • Draft comment:
    The if condition redundantly checks for an empty tools list. Since an empty list (or None) already evaluates as False, the 'or len(prompt_version.llm_config.tools) == 0' check is unnecessary. Consider simplifying the condition and adding a brief comment explaining why tool_choice is reset to None when no tools are provided.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_kAhs2sacAQvA2lFS

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@coderabbitai coderabbitai bot left a 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

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 025b928 and cb6905f.

📒 Files selected for processing (2)
  • packages/traceloop-sdk/traceloop/sdk/prompts/client.py (1 hunks)
  • packages/traceloop-sdk/traceloop/sdk/prompts/model.py (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/traceloop-sdk/traceloop/sdk/prompts/model.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Lint
  • GitHub Check: Build Packages (3.11)

Comment on lines +78 to +83
if (
not prompt_version.llm_config.tools
or len(prompt_version.llm_config.tools) == 0
) and prompt_version.llm_config.tool_choice is not None:
prompt_version.llm_config.tool_choice = None

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix potential AttributeError and simplify the condition.

The current implementation has a potential bug and can be simplified:

  1. AttributeError risk: If tools is None, the len(prompt_version.llm_config.tools) call will raise an AttributeError.
  2. Redundant condition: The not prompt_version.llm_config.tools check already handles both None and empty list cases, making the length check unnecessary.

Apply this diff to fix the issue and improve readability:

-        if (
-            not prompt_version.llm_config.tools
-            or len(prompt_version.llm_config.tools) == 0
-        ) and prompt_version.llm_config.tool_choice is not None:
-            prompt_version.llm_config.tool_choice = None
+        if (
+            not prompt_version.llm_config.tools
+            and prompt_version.llm_config.tool_choice is not None
+        ):
+            prompt_version.llm_config.tool_choice = None
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (
not prompt_version.llm_config.tools
or len(prompt_version.llm_config.tools) == 0
) and prompt_version.llm_config.tool_choice is not None:
prompt_version.llm_config.tool_choice = None
if (
not prompt_version.llm_config.tools
and prompt_version.llm_config.tool_choice is not None
):
prompt_version.llm_config.tool_choice = None
🤖 Prompt for AI Agents
In packages/traceloop-sdk/traceloop/sdk/prompts/client.py around lines 78 to 83,
the condition checking if tools is empty or None uses both a falsy check and a
length check, which risks an AttributeError if tools is None and is redundant.
Simplify the condition by removing the length check and only use the falsy check
on prompt_version.llm_config.tools to cover both None and empty list cases, then
proceed to set tool_choice to None if needed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed 6a1a99c in 35 seconds. Click for details.
  • Reviewed 23 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/traceloop/sdk/prompts/model.py:52
  • Draft comment:
    Consider adding a docstring to ResponseFormat to clarify its purpose and structure, including details for the json_schema field.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
2. packages/traceloop-sdk/traceloop/sdk/prompts/model.py:67
  • Draft comment:
    Document the response_format field in ModelConfig to explain when and how it should be used.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_dHUk7EOEF7kr2H7w

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Changes requested ❌

Reviewed 77e55f6 in 1 minute and 33 seconds. Click for details.
  • Reviewed 142 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/tests/test_prompt_management.py:67
  • Draft comment:
    New 'prompts_with_tools_json' block added. Ensure the JSON structure exactly matches the expected schema for tool configuration.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to ensure that the JSON structure matches the expected schema. This falls under the category of asking the author to double-check or ensure something, which is against the rules.
2. packages/traceloop-sdk/tests/test_prompt_management.py:170
  • Draft comment:
    The new test 'test_prompt_management_with_tools' properly validates tool-related attributes. Consider adding negative tests for missing or invalid tool config fields in the future.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% While the suggestion to add negative tests is reasonable from a testing perspective, it's more of a general testing best practice suggestion rather than a specific issue with the code changes. The current changes properly implement the happy path test case. Adding negative tests could be done as a separate improvement. The comment does point out a legitimate testing gap. Negative test cases are important for robust test coverage. However, per the rules, we should not make purely informative comments or suggestions that aren't clearly required changes. The current implementation is correct and complete for its scope. The comment should be removed as it's more of a nice-to-have suggestion rather than a required change. The current test implementation is valid and complete.

Workflow ID: wflow_DCBYEhtfQGGkzQTu

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

}
}
}
]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File ends without a newline. Please add a newline at EOF to adhere to best practices.

Suggested change
]

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Changes requested ❌

Reviewed 908e0b7 in 1 minute and 38 seconds. Click for details.
  • Reviewed 145 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/tests/test_prompt_management.py:142
  • Draft comment:
    The added JSON fixture (prompts_with_response_format_json) starts with a leading newline. Consider removing it or trimming whitespace to prevent unintended formatting issues.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While the comment is technically correct about an inconsistency, the leading newline in a JSON string literal is extremely unlikely to cause any actual issues. The json.loads() function handles whitespace without problems. Looking at the test that uses this string (test_prompt_management_with_response_format), it works fine. This seems like an extremely minor style nitpick that doesn't affect functionality. The inconsistency in style between the JSON strings could indicate a copy-paste error or oversight that might be worth fixing for code cleanliness. While consistent style is good, this is far too minor to warrant a PR comment. The code works perfectly fine and the inconsistency has no practical impact. This comment should be deleted as it points out an extremely minor style issue that has no practical impact on the code's functionality.
2. packages/traceloop-sdk/tests/test_prompt_management.py:251
  • Draft comment:
    The test for tool function support now only asserts the tool call name. If the detailed prompt metadata is important, consider adding checks for key attributes (e.g. prompt key, version, etc.) to ensure nothing regress.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_iEjAmmAaLOJCYc6B

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

spans = exporter.get_finished_spans()
open_ai_span = spans[0]
completion = open_ai_span.attributes.get(f"{SpanAttributes.LLM_COMPLETIONS}.0.content")
try:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using a try/except block with a redundant 'assert True', consider asserting that the parsed JSON includes expected keys (e.g., 'joke') to strengthen the test's coverage.

Copy link

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (1)
packages/traceloop-sdk/tests/test_prompt_management.py (1)

268-268: File still ends without a newline.

🧹 Nitpick comments (1)
packages/traceloop-sdk/tests/test_prompt_management.py (1)

243-252: Test correctly validates tool function integration.

The test properly verifies that tool calls are captured in tracing spans. Consider enhancing the assertion to also validate tool call parameters for more comprehensive coverage.

-    completion = open_ai_span.attributes.get(f"{SpanAttributes.LLM_COMPLETIONS}.0.tool_calls.0.name")
-    assert completion == "get_joke"
+    tool_call_name = open_ai_span.attributes.get(f"{SpanAttributes.LLM_COMPLETIONS}.0.tool_calls.0.name")
+    tool_call_args = open_ai_span.attributes.get(f"{SpanAttributes.LLM_COMPLETIONS}.0.tool_calls.0.function.arguments")
+    assert tool_call_name == "get_joke"
+    assert "style" in tool_call_args  # Verify parameters are passed
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f207398 and 908e0b7.

📒 Files selected for processing (1)
  • packages/traceloop-sdk/tests/test_prompt_management.py (2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/traceloop-sdk/tests/test_prompt_management.py (6)
packages/traceloop-sdk/tests/conftest.py (1)
  • exporter (18-26)
packages/opentelemetry-instrumentation-openai/tests/conftest.py (1)
  • openai_client (42-43)
packages/traceloop-sdk/traceloop/sdk/prompts/client.py (1)
  • PromptRegistryClient (37-149)
packages/traceloop-sdk/traceloop/sdk/prompts/__init__.py (1)
  • get_prompt (4-5)
packages/traceloop-sdk/traceloop/sdk/utils/in_memory_span_exporter.py (1)
  • get_finished_spans (40-43)
packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (1)
  • SpanAttributes (36-229)
🪛 Ruff (0.12.2)
packages/traceloop-sdk/tests/test_prompt_management.py

266-266: Do not assert False (python -O removes these calls), raise AssertionError()

Replace assert False

(B011)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Build Packages (3.11)
  • GitHub Check: Lint
🔇 Additional comments (1)
packages/traceloop-sdk/tests/test_prompt_management.py (1)

67-216: Well-structured JSON prompt definitions for new features.

The JSON configurations correctly extend the original prompt structure to support tool functions and response formats. The tool configuration properly defines the function schema, and the response format uses the appropriate JSON schema structure for OpenAI's structured output.

Comment on lines 254 to 267
@pytest.mark.vcr
def test_prompt_management_with_response_format(exporter, openai_client):
PromptRegistryClient()._registry.load(prompts_json=json.loads(prompts_with_response_format_json))
prompt_args = get_prompt(key="joke_generator", variables={"style": "pirate"})
openai_client.chat.completions.create(**prompt_args)

spans = exporter.get_finished_spans()
open_ai_span = spans[0]
completion = open_ai_span.attributes.get(f"{SpanAttributes.LLM_COMPLETIONS}.0.content")
try:
json.loads(completion)
except json.JSONDecodeError:
assert False
assert True
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix assertion pattern for better test reliability.

The test correctly validates JSON response format, but the assertion pattern should be improved for reliability.

-    try:
-        json.loads(completion)
-    except json.JSONDecodeError:
-        assert False
-    assert True
+    try:
+        json.loads(completion)
+    except json.JSONDecodeError as e:
+        pytest.fail(f"Response is not valid JSON: {e}")

This addresses the static analysis warning about assert False being removed by python -O and makes the test failure more informative.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@pytest.mark.vcr
def test_prompt_management_with_response_format(exporter, openai_client):
PromptRegistryClient()._registry.load(prompts_json=json.loads(prompts_with_response_format_json))
prompt_args = get_prompt(key="joke_generator", variables={"style": "pirate"})
openai_client.chat.completions.create(**prompt_args)
spans = exporter.get_finished_spans()
open_ai_span = spans[0]
completion = open_ai_span.attributes.get(f"{SpanAttributes.LLM_COMPLETIONS}.0.content")
try:
json.loads(completion)
except json.JSONDecodeError:
assert False
assert True
@pytest.mark.vcr
def test_prompt_management_with_response_format(exporter, openai_client):
PromptRegistryClient()._registry.load(prompts_json=json.loads(prompts_with_response_format_json))
prompt_args = get_prompt(key="joke_generator", variables={"style": "pirate"})
openai_client.chat.completions.create(**prompt_args)
spans = exporter.get_finished_spans()
open_ai_span = spans[0]
completion = open_ai_span.attributes.get(f"{SpanAttributes.LLM_COMPLETIONS}.0.content")
try:
json.loads(completion)
except json.JSONDecodeError as e:
pytest.fail(f"Response is not valid JSON: {e}")
🧰 Tools
🪛 Ruff (0.12.2)

266-266: Do not assert False (python -O removes these calls), raise AssertionError()

Replace assert False

(B011)

🤖 Prompt for AI Agents
In packages/traceloop-sdk/tests/test_prompt_management.py around lines 254 to
267, the test uses a try-except block with assert False inside except to check
JSON decoding, which is unreliable and can be removed by python -O optimization.
Replace this pattern by directly asserting that json.loads(completion) does not
raise an exception, for example by using pytest.raises or by asserting the
result is not None, to make the test failure more informative and reliable.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed 3203be0 in 1 minute and 4 seconds. Click for details.
  • Reviewed 42 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/tests/test_prompt_management.py:245
  • Draft comment:
    Multiline formatting improvements look good. Consider adding a trailing comma after the last argument (e.g., after json.loads(prompts_with_tools_json)) for consistency and to ease future diffs.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
2. packages/traceloop-sdk/tests/test_prompt_management.py:272
  • Draft comment:
    In the response format test, instead of using 'assert False' in the except block and a redundant 'assert True' after, consider using pytest.fail() with a meaningful message for clarity.
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_vgoH0bqkMF5vAjsI

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed 2249327 in 34 seconds. Click for details.
  • Reviewed 11 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/tests/test_prompt_management.py:274
  • Draft comment:
    Good change: using pytest.fail with a descriptive message improves clarity over assert False. Optionally, the final 'assert True' is redundant since the test passes if no exception is raised.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_scOqExnVqdOkcEZP

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

…readability and update request/response details
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed 0b3cb15 in 49 seconds. Click for details.
  • Reviewed 478 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/tests/cassettes/test_prompt_management/test_prompt_management.yaml:18
  • Draft comment:
    Confirm that updated header values (e.g., traceparent, user-agent, runtime version) are intentional and match new configuration.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
2. packages/traceloop-sdk/tests/cassettes/test_prompt_management/test_prompt_management_with_response_format.yaml:4
  • Draft comment:
    Ensure that the response_format JSON schema is correctly structured and comprehensive for the intended response validation.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
3. packages/traceloop-sdk/tests/cassettes/test_prompt_management/test_prompt_management_with_tools.yaml:4
  • Draft comment:
    Verify that the tool support configuration (tool_choice and tools array) follows the expected schema and includes all required fields.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_hCOQROses5EEeL6R

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Comment on lines +78 to +82
if (
not prompt_version.llm_config.tools
or len(prompt_version.llm_config.tools) == 0
) and prompt_version.llm_config.tool_choice is not None:
prompt_version.llm_config.tool_choice = None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a explanation for this
how could it be tool choice="something" and tools is empty? if tool_choice is None but tools has values, it's ok?

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Changes requested ❌

Reviewed c8a1433 in 1 minute and 16 seconds. Click for details.
  • Reviewed 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/traceloop/sdk/prompts/client.py:78
  • Draft comment:
    The added comment contains grammatical errors. Consider rephrasing for clarity. For example, change "setted its throw an error" to "set, it throws an error".
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_ZzjeUtS96l9pGvv9

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
@galzilber galzilber merged commit 865f5cb into main Jul 21, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants