[FIRE-1020] feature: add compile_prompt function to the client as well as replace the package manager poetry with uv #458
Conversation
…l as replace the package manager poetry with uv
WalkthroughMigrates the project from Poetry to UV/Hatchling, updates CI and Docker to use UV, converts many dataclasses to Pydantic models, adds Client.compile_prompt, updates serialization to Pydantic model_dump_json, and adjusts tooling/ignore/config files. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (3)
✏️ Tip: You can disable this entire section by setting 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyproject.toml (1)
136-147: Coverage configuration sections are incorrectly named and will be ignored.The coverage configuration has two issues:
- Sections at lines 139, 142, and 145 are missing the
tool.prefix required inpyproject.toml[coverage.run]at line 142 duplicates[tool.coverage.run]at line 136These sections will be silently ignored, meaning
branch = true,fail_under = 50, andshow_missing = truewon't take effect.🐛 Proposed fix
[tool.coverage.run] -source = ["tests"] - -[coverage.paths] -source = "qualifire" - -[coverage.run] -branch = true +source = ["qualifire"] +branch = true -[coverage.report] +[tool.coverage.paths] +source = ["qualifire"] + +[tool.coverage.report] fail_under = 50 show_missing = trueAlso note:
source = ["tests"]in[tool.coverage.run]measures coverage of test files themselves rather than thequalifiresource code—this is likely unintentional.
🤖 Fix all issues with AI agents
In @.flake8:
- Line 2: The flake8 max-line-length (max-line-length = 120 in .flake8)
conflicts with Black/isort's line-length in pyproject.toml (currently 88); fix
by making them consistent: either change .flake8 max-line-length to 88, or
update pyproject.toml's [tool.black] and [tool.isort] line-length settings to
120 so all three (max-line-length, tool.black.line-length,
tool.isort.line_length) match; update only the setting(s) and run the
formatter/linter to verify no remaining mismatches.
In @.github/workflows/build.yml:
- Around line 33-34: Fix the misspelled GitHub Actions step name by replacing
"Creadentials search in source code - TruffleHog OSS" with "Credentials search
in source code - TruffleHog OSS" in the workflow step that uses
trufflesecurity/trufflehog@v3.69.0 so the step name reads correctly.
- Around line 13-22: Fix the typo "Creadentials" -> "Credentials" in the GitHub
Actions workflow (search for the "Creadentials" string in
.github/workflows/build.yml), remove the redundant step that runs "uv pip
install pytest" because pytest is already pulled in via "uv sync" from
pyproject.toml, and if tests require openai add "openai" to the dev-dependencies
in pyproject.toml so it is installed by "uv sync"; ensure the actions steps
using actions/checkout@v6 and actions/setup-python@v6 remain unchanged.
In `@CONTRIBUTING.md`:
- Around line 5-6: Update the CONTRIBUTING.md sentence that reads "If you dont
have `uv`, you should install..." to correct the typo by replacing "dont" with
"don't" so the sentence reads "If you don't have `uv`, you should
install..."—locate this exact sentence in CONTRIBUTING.md and apply the
single-word fix.
In `@pyproject.toml`:
- Around line 30-33: Update the project URLs to use hyphens and the correct
repository name: replace the current Homepage and Repository values under
[project.urls] so they point to
"https://github.com/qualifire-dev/qualifire-python-sdk" (update the Homepage and
Repository entries accordingly).
In `@qualifire/client.py`:
- Around line 299-326: The compile_prompt method currently calls
response.raise_for_status() which is inconsistent with evaluate() and
invoke_evaluation(); change compile_prompt to check the response (e.g., if not
response.ok) and raise the same style of Exception used elsewhere with a clear
message including response.status_code and response.text (for example: "Error
compiling prompt: <status> - <response_text>") before returning
CompilePromptResponse(**response.json()); keep the rest of the request logic and
header handling intact and reference the compile_prompt function name when
applying the change.
In `@qualifire/types.py`:
- Around line 26-29: The LLMToolCall Pydantic model marks id as Optional[str]
but omits a default, making it required in Pydantic v2; update the LLMToolCall
class so the id field has an explicit default of None (e.g., set id to None) to
make it truly optional and ensure callers can omit it. Reference: class
LLMToolCall and the id field.
🧹 Nitpick comments (5)
qualifire/client.py (1)
299-304: Consider adding a docstring for this public API method.The
evaluate()method has comprehensive documentation. Adding a docstring tocompile_promptwould improve API discoverability and usability.Also, verify if
params: Dict[str, str]is intentionally restricted to string values, or if it should beDict[str, Any]to support other variable types.Makefile (1)
10-14: Platform-specific UV installation may limit portability.The
brew install uvcommand only works on macOS. Consider using a platform-agnostic installation method or documenting that this target is macOS-only.♻️ Suggested improvement for cross-platform support
#* UV .PHONY: uv-download uv-download: - brew install uv + curl -LsSf https://astral.sh/uv/install.sh | sh uv --version.github/workflows/build.yml (1)
24-27: Potentially redundant dependency installation.If
pytestandopenaiare already defined as dev dependencies inpyproject.toml, theuv pip install pytest openaistep may be redundant afteruv sync. Consider whether these are needed or ifuv syncalready handles them.pyproject.toml (2)
24-28: Movetypes-requeststo dev dependencies.
types-requestsis a type stub package used for static type checking (mypy). It provides no runtime functionality and should be in the[dependency-groups] devsection rather than runtime dependencies.♻️ Proposed fix
dependencies = [ "pydantic>=2.10.6", "traceloop-sdk==0.40.4 ; python_version >= '3.10'", - "types-requests>=2.32.0.20241016", ]Then add to the dev group:
dev = [ # ... existing deps ... "types-requests>=2.32.0.20241016", ]
14-23: Classifiers are incomplete for supported Python versions.The classifiers only list Python 3.8 and 3.9, but
requires-python = ">=3.8,<4"implies support for 3.10, 3.11, 3.12, etc. Consider adding the missing classifiers for accurate PyPI metadata.♻️ Proposed fix
classifiers = [ "Development Status :: 3 - Alpha", "Intended Audience :: Developers", "Operating System :: OS Independent", "Topic :: Software Development :: Libraries :: Python Modules", "License :: OSI Approved :: MIT License", "Programming Language :: Python :: 3", "Programming Language :: Python :: 3.8", "Programming Language :: Python :: 3.9", + "Programming Language :: Python :: 3.10", + "Programming Language :: Python :: 3.11", + "Programming Language :: Python :: 3.12", ]
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
assets/images/coverage.svgis excluded by!**/*.svgpoetry.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.dockerignore.flake8.github/workflows/build.yml.github/workflows/python-publish.yml.gitignore.pre-commit-config.yamlCLAUDE.mdCONTRIBUTING.mdMakefiledocker/Dockerfiledocker/README.mdpyproject.tomlqualifire/client.pyqualifire/types.pyrequirements.txtsetup.pytests/test_utils.py
💤 Files with no reviewable changes (3)
- .gitignore
- setup.py
- requirements.txt
🧰 Additional context used
📓 Path-based instructions (4)
qualifire/client.py
📄 CodeRabbit inference engine (CLAUDE.md)
qualifire/client.py: The mainClientclass withevaluate()andinvoke_evaluation()methods should be located inqualifire/client.py
API key can be passed directly or viaQUALIFIRE_API_KEYenvironment variable
Base URL defaults tohttps://proxy.qualifire.ai/but can be overridden viaQUALIFIRE_BASE_URLenvironment variable
Files:
qualifire/client.py
qualifire/{client,types}.py
📄 CodeRabbit inference engine (CLAUDE.md)
Legacy content moderation checks (dangerous_content_check, harassment_check, etc.) are deprecated in favor of unified
content_moderation_check
Files:
qualifire/client.pyqualifire/types.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Tests should be located in
tests/directory and use pytest with parametrized test cases
Files:
tests/test_utils.py
qualifire/types.py
📄 CodeRabbit inference engine (CLAUDE.md)
qualifire/types.py: All data classes, enums, type definitions includingEvaluationRequest,EvaluationResponse,LLMMessage,LLMToolCall,LLMToolDefinition,ModelMode, andSyntaxCheckArgsshould be located inqualifire/types.py
TheEvaluationRequestdataclass should validate inputs in__post_init__(e.g., tool_selection_quality_check requires both messages and available_tools)
Files:
qualifire/types.py
🧠 Learnings (10)
📚 Learning: 2026-01-07T14:35:35.603Z
Learnt from: CR
Repo: qualifire-dev/qualifire-python-sdk PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T14:35:35.603Z
Learning: Run tests with coverage using `make test`; run specific tests with `poetry run pytest` commands
Applied to files:
MakefileCONTRIBUTING.mdCLAUDE.md.github/workflows/build.yml
📚 Learning: 2026-01-07T14:35:35.603Z
Learnt from: CR
Repo: qualifire-dev/qualifire-python-sdk PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T14:35:35.603Z
Learning: Install dependencies and set up environment using `make install` and `make pre-commit-install`
Applied to files:
MakefileCONTRIBUTING.md
📚 Learning: 2026-01-07T14:35:35.603Z
Learnt from: CR
Repo: qualifire-dev/qualifire-python-sdk PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T14:35:35.603Z
Learning: Format code using `make codestyle` (isort, black, pyupgrade); check without modifying using `make check-codestyle`
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-07T14:35:35.603Z
Learnt from: CR
Repo: qualifire-dev/qualifire-python-sdk PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T14:35:35.603Z
Learning: Applies to qualifire/client.py : The main `Client` class with `evaluate()` and `invoke_evaluation()` methods should be located in `qualifire/client.py`
Applied to files:
qualifire/client.py
📚 Learning: 2026-01-07T14:35:35.603Z
Learnt from: CR
Repo: qualifire-dev/qualifire-python-sdk PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T14:35:35.603Z
Learning: Applies to qualifire/client.py : API key can be passed directly or via `QUALIFIRE_API_KEY` environment variable
Applied to files:
tests/test_utils.py
📚 Learning: 2026-01-07T14:35:35.603Z
Learnt from: CR
Repo: qualifire-dev/qualifire-python-sdk PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T14:35:35.603Z
Learning: Applies to qualifire/utils.py : Helper functions for API key and base URL resolution from environment variables should be located in `qualifire/utils.py`
Applied to files:
tests/test_utils.py
📚 Learning: 2026-01-07T14:35:35.603Z
Learnt from: CR
Repo: qualifire-dev/qualifire-python-sdk PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T14:35:35.603Z
Learning: Applies to qualifire/client.py : Base URL defaults to `https://proxy.qualifire.ai/` but can be overridden via `QUALIFIRE_BASE_URL` environment variable
Applied to files:
tests/test_utils.py
📚 Learning: 2026-01-07T14:35:35.603Z
Learnt from: CR
Repo: qualifire-dev/qualifire-python-sdk PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T14:35:35.603Z
Learning: Applies to qualifire/types.py : All data classes, enums, type definitions including `EvaluationRequest`, `EvaluationResponse`, `LLMMessage`, `LLMToolCall`, `LLMToolDefinition`, `ModelMode`, and `SyntaxCheckArgs` should be located in `qualifire/types.py`
Applied to files:
qualifire/types.py
📚 Learning: 2026-01-07T14:35:35.603Z
Learnt from: CR
Repo: qualifire-dev/qualifire-python-sdk PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T14:35:35.603Z
Learning: Applies to qualifire/types.py : The `EvaluationRequest` dataclass should validate inputs in `__post_init__` (e.g., tool_selection_quality_check requires both messages and available_tools)
Applied to files:
qualifire/types.py
📚 Learning: 2026-01-07T14:35:35.603Z
Learnt from: CR
Repo: qualifire-dev/qualifire-python-sdk PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T14:35:35.603Z
Learning: Applies to qualifire/{client,types}.py : Legacy content moderation checks (dangerous_content_check, harassment_check, etc.) are deprecated in favor of unified `content_moderation_check`
Applied to files:
qualifire/types.py
🧬 Code graph analysis (1)
qualifire/client.py (1)
qualifire/types.py (1)
CompilePromptResponse(157-163)
🪛 checkmake (0.2.2)
Makefile
[warning] 119-119: Missing required phony target "all"
(minphony)
[warning] 119-119: Missing required phony target "clean"
(minphony)
🔇 Additional comments (25)
qualifire/client.py (3)
9-9: LGTM!The import of
CompilePromptResponseis correctly added to support the newcompile_promptmethod.
236-238: LGTM!The migration to
model_dump_json()is correct for Pydantic v2 and aligns with the dataclass-to-Pydantic migration intypes.py.
284-284: LGTM!Consistent use of
model_dump_json()for serialization.qualifire/types.py (7)
5-6: LGTM!Correct imports for Pydantic v2 with
BaseModelandmodel_validator.
38-39: LGTM!Clean migration to Pydantic
BaseModel.
70-76: LGTM!The
@model_validator(mode="after")correctly replaces the dataclass__post_init__pattern. The validation logic is properly preserved for input validation and deprecated field handling. Based on learnings, this maintains the expected validation behavior.
111-127: LGTM!Consistent validation pattern with
EvaluationRequest.
130-138: LGTM!The
claimfield correctly usesOptional[str] = Nonepattern, making it truly optional. Other fields are required as expected.
141-149: LGTM!Clean model definitions. The
evaluationResultsfield uses camelCase to match the API response format.
152-163: LGTM!New models
ToolResponseandCompilePromptResponseare well-defined and match the expected API contract structure. Based on learnings, all type definitions are correctly placed inqualifire/types.py.CLAUDE.md (1)
20-23: LGTM!The test invocation commands are correctly updated to use
uv run pytest, consistent with the Poetry-to-UV migration across the repository. Based on learnings, these commands previously usedpoetry run pytest.CONTRIBUTING.md (1)
15-15: LGTM!The updated guidance correctly explains that UV automatically manages virtual environments when using
uv run.tests/test_utils.py (2)
29-29: LGTM!Using
Falseexplicitly for theraisingparameter is the correct approach. WhileNoneworked due to Python's falsy evaluation,Falseis semantically clearer and matches the expected boolean type.
54-54: Consistent fix applied here as well..dockerignore (1)
31-39: LGTM!Good additions to exclude build artifacts (
dist/,build/,*.egg-info/) and test coverage files (htmlcov/,.coverage,.coverage.*) from Docker builds. These align with the UV/Hatchling migration and keep Docker contexts clean..github/workflows/python-publish.yml (2)
34-38: LGTM!The build commands correctly use UV idioms:
uv buildfor package buildinguv tool run twine check dist/*for wheel validation without requiring twine as a project dependency
23-32: All action versions are valid and properly released.
actions/checkout@v6is a current release (updated credential handling)actions/setup-python@v6is a current release (Node 20 → Node 24)astral-sh/setup-uv@v7is current- UV version
0.9.22was released January 6, 2026No changes needed.
.pre-commit-config.yaml (1)
16-30: LGTM!The migration from Poetry to
uv tool runfor the local isort and black hooks is correct. The settings paths are preserved, maintaining consistent formatting behavior.Makefile (2)
16-63: LGTM!The Poetry-to-UV migration is comprehensive and correct:
uv syncreplacespoetry installuv runreplacespoetry runfor tool executionuv add --devreplacespoetry add --devfor dependency management
109-121: LGTM!The new build targets (
build,build-wheel,build-sdist) usinguv buildare good additions that complement the UV migration.docker/Dockerfile (2)
16-22: Consider adding explicit package installation.After
uv syncinstalls dependencies andCOPY . .adds the source code, the package itself may not be installed in the environment. If this container is intended to run thequalifirepackage, you may need to adduv pip install -e .oruv pip install .after copying the source.If this container is purely for development/testing purposes (as suggested by
CMD ["/bin/bash"]), this may be intentional.
1-24: LGTM on the UV migration pattern.The Dockerfile correctly migrates from Poetry to UV:
- Copies the UV binary from the official image
- Sets appropriate UV environment variables
- Uses
uv sync --frozen --no-cachefor reproducible installs- Conditional
--no-devflag for production buildsdocker/README.md (1)
25-45: LGTM!The documentation updates are clear and consistent with the Dockerfile changes:
- Dev build instructions with
INSTALL_DEV=trueare helpful- Mount path updated to match the new
/appworking directory- Test execution example using
uv run pytestis correctpyproject.toml (2)
34-52: Dev dependencies look reasonable, though some versions are dated.The dependency constraints are well-structured. Note that some tools have newer major versions available (e.g., black 24.x, pytest 8.x), but the current pinning ensures stability.
54-58: No action needed. The build configuration is correct and matches the package structure. The project uses a flat layout withqualifire/at the root level, and the build targets correctly include["qualifire"].
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| @@ -1,5 +1,5 @@ | |||
| [flake8] | |||
| max-line-length = 88 | |||
| max-line-length = 120 | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for line-length/max-line configurations in common config files
echo "=== Searching for line-length configurations ==="
rg -n "line.?length|max.?line" --type=toml --type=ini --type=yaml --type=cfg 2>/dev/null | head -50
echo -e "\n=== Checking pyproject.toml for formatter settings ==="
fd -t f "pyproject.toml" -x cat {} 2>/dev/null
echo -e "\n=== Checking setup.cfg for flake8/formatter settings ==="
fd -t f "setup.cfg" -x cat {} 2>/dev/null
echo -e "\n=== Checking .flake8 file ==="
cat .flake8 2>/dev/null || echo "File not found"
echo -e "\n=== Checking for tox.ini ==="
fd -t f "tox.ini" -x cat {} 2>/dev/nullRepository: qualifire-dev/qualifire-python-sdk
Length of output: 3946
Fix configuration mismatch between flake8 and Black/isort.
The .flake8 change to max-line-length = 120 conflicts with pyproject.toml which configures Black and isort to line-length = 88. Update either:
- Black and isort in
pyproject.tomltoline-length = 120, or .flake8back tomax-line-length = 88
Without this alignment, you'll have inconsistent line-length enforcement between formatting and linting.
🤖 Prompt for AI Agents
In @.flake8 at line 2, The flake8 max-line-length (max-line-length = 120 in
.flake8) conflicts with Black/isort's line-length in pyproject.toml (currently
88); fix by making them consistent: either change .flake8 max-line-length to 88,
or update pyproject.toml's [tool.black] and [tool.isort] line-length settings to
120 so all three (max-line-length, tool.black.line-length,
tool.isort.line_length) match; update only the setting(s) and run the
formatter/linter to verify no remaining mismatches.
| - uses: actions/checkout@v6 | ||
| - name: Set up Python ${{ matrix.python-version }} | ||
| uses: actions/setup-python@v4.7.0 | ||
| uses: actions/setup-python@v6 | ||
| with: | ||
| python-version: ${{ matrix.python-version }} | ||
| - name: Install and configure Poetry | ||
| uses: snok/install-poetry@v1 | ||
|
|
||
| - name: Install uv | ||
| uses: astral-sh/setup-uv@v7 | ||
| with: | ||
| version: 1.6.1 | ||
| version: "0.9.22" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/build.ymlRepository: qualifire-dev/qualifire-python-sdk
Length of output: 1183
🏁 Script executed:
find . -name "pyproject.toml" -type f | head -5Repository: qualifire-dev/qualifire-python-sdk
Length of output: 94
🏁 Script executed:
cat -n ./pyproject.tomlRepository: qualifire-dev/qualifire-python-sdk
Length of output: 4540
🌐 Web query:
actions/checkout latest version GitHub
💡 Result:
The current latest release is actions/checkout v6.0.2 (released Jan 9, 2026). [1][2]
Sources:
[1] actions/checkout releases (v6.0.2).
[2] actions/checkout repository README (Checkout v6).
🌐 Web query:
actions/setup-python latest version GitHub v6
💡 Result:
Latest v6 tag is v6.1.0 — released Nov 25, 2025. [1][2]
Sources:
[1] actions/setup-python releases (v6.1.0).
[2] actions/setup-python README / Marketplace showing usage with @v6.
Fix the typo on line 33: "Creadentials" should be "Credentials".
The GitHub Actions versions (checkout@v6 and setup-python@v6) are valid and available. However, the uv pip install pytest on line 27 is redundant since pytest is already listed in pyproject.toml dev dependencies and will be installed via uv sync. Additionally, openai is not declared in pyproject.toml dev dependencies—consider adding it there if it's a required test dependency.
🤖 Prompt for AI Agents
In @.github/workflows/build.yml around lines 13 - 22, Fix the typo
"Creadentials" -> "Credentials" in the GitHub Actions workflow (search for the
"Creadentials" string in .github/workflows/build.yml), remove the redundant step
that runs "uv pip install pytest" because pytest is already pulled in via "uv
sync" from pyproject.toml, and if tests require openai add "openai" to the
dev-dependencies in pyproject.toml so it is installed by "uv sync"; ensure the
actions steps using actions/checkout@v6 and actions/setup-python@v6 remain
unchanged.
| - name: Creadentials search in source code - TruffleHog OSS | ||
| uses: trufflesecurity/trufflehog@v3.69.0 |
There was a problem hiding this comment.
Fix typo in step name.
"Creadentials" should be "Credentials".
📝 Proposed fix
- - name: Creadentials search in source code - TruffleHog OSS
+ - name: Credentials search in source code - TruffleHog OSS📝 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.
| - name: Creadentials search in source code - TruffleHog OSS | |
| uses: trufflesecurity/trufflehog@v3.69.0 | |
| - name: Credentials search in source code - TruffleHog OSS | |
| uses: trufflesecurity/trufflehog@v3.69.0 |
🤖 Prompt for AI Agents
In @.github/workflows/build.yml around lines 33 - 34, Fix the misspelled GitHub
Actions step name by replacing "Creadentials search in source code - TruffleHog
OSS" with "Credentials search in source code - TruffleHog OSS" in the workflow
step that uses trufflesecurity/trufflehog@v3.69.0 so the step name reads
correctly.
| We use `uv` to manage the [dependencies](https://docs.astral.sh/uv/). | ||
| If you dont have `uv`, you should install with `make uv-download` or follow the [official installation guide](https://docs.astral.sh/uv/getting-started/installation/). |
There was a problem hiding this comment.
Minor typo: "dont" → "don't".
Proposed fix
-If you dont have `uv`, you should install with `make uv-download` or follow the [official installation guide](https://docs.astral.sh/uv/getting-started/installation/).
+If you don't have `uv`, you should install with `make uv-download` or follow the [official installation guide](https://docs.astral.sh/uv/getting-started/installation/).📝 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.
| We use `uv` to manage the [dependencies](https://docs.astral.sh/uv/). | |
| If you dont have `uv`, you should install with `make uv-download` or follow the [official installation guide](https://docs.astral.sh/uv/getting-started/installation/). | |
| We use `uv` to manage the [dependencies](https://docs.astral.sh/uv/). | |
| If you don't have `uv`, you should install with `make uv-download` or follow the [official installation guide](https://docs.astral.sh/uv/getting-started/installation/). |
🤖 Prompt for AI Agents
In `@CONTRIBUTING.md` around lines 5 - 6, Update the CONTRIBUTING.md sentence that
reads "If you dont have `uv`, you should install..." to correct the typo by
replacing "dont" with "don't" so the sentence reads "If you don't have `uv`, you
should install..."—locate this exact sentence in CONTRIBUTING.md and apply the
single-word fix.
| [project.urls] | ||
| Homepage = "https://github.com/qualifire_dev/qualifire" | ||
| Repository = "https://github.com/qualifire_dev/qualifire" | ||
|
|
There was a problem hiding this comment.
URLs appear incorrect - GitHub uses hyphens, not underscores.
Based on the PR URL (github.com/qualifire-dev/qualifire-python-sdk), these URLs should use hyphens and the correct repository name.
🐛 Proposed fix
[project.urls]
-Homepage = "https://github.com/qualifire_dev/qualifire"
-Repository = "https://github.com/qualifire_dev/qualifire"
+Homepage = "https://github.com/qualifire-dev/qualifire-python-sdk"
+Repository = "https://github.com/qualifire-dev/qualifire-python-sdk"📝 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.
| [project.urls] | |
| Homepage = "https://github.com/qualifire_dev/qualifire" | |
| Repository = "https://github.com/qualifire_dev/qualifire" | |
| [project.urls] | |
| Homepage = "https://github.com/qualifire-dev/qualifire-python-sdk" | |
| Repository = "https://github.com/qualifire-dev/qualifire-python-sdk" | |
🤖 Prompt for AI Agents
In `@pyproject.toml` around lines 30 - 33, Update the project URLs to use hyphens
and the correct repository name: replace the current Homepage and Repository
values under [project.urls] so they point to
"https://github.com/qualifire-dev/qualifire-python-sdk" (update the Homepage and
Repository entries accordingly).
| def compile_prompt( | ||
| self, | ||
| prompt_id: str, | ||
| revision_id: Optional[str] = None, | ||
| params: Optional[Dict[str, str]] = None, | ||
| ) -> CompilePromptResponse: | ||
| url = f"{self._base_url}/api/v1/studio/prompts/{prompt_id}/compile" | ||
| if revision_id: | ||
| url = f"{url}?revision={revision_id}" | ||
|
|
||
| headers = { | ||
| "X-Qualifire-API-Key": self._api_key, | ||
| "Content-Type": "application/json", | ||
| } | ||
|
|
||
| if not params: | ||
| params = {} | ||
| request_body = json.dumps({"variables": params}) | ||
|
|
||
| response = requests.post( | ||
| url=url, | ||
| data=request_body, | ||
| headers=headers, | ||
| verify=self._verify, | ||
| ) | ||
| response.raise_for_status() | ||
|
|
||
| return CompilePromptResponse(**response.json()) |
There was a problem hiding this comment.
Inconsistent error handling pattern.
The compile_prompt method uses response.raise_for_status() which raises an HTTPError, while evaluate() and invoke_evaluation() raise a custom Exception with a formatted message including the response text. This inconsistency could confuse SDK users who expect uniform error handling.
Consider aligning with the existing pattern:
Proposed fix
response = requests.post(
url=url,
data=request_body,
headers=headers,
verify=self._verify,
)
- response.raise_for_status()
+ if response.status_code != 200:
+ message = f"Qualifire API error: {response.status_code}"
+ if response.text:
+ message += f" - {response.text}"
+ raise Exception(message)
return CompilePromptResponse(**response.json())📝 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.
| def compile_prompt( | |
| self, | |
| prompt_id: str, | |
| revision_id: Optional[str] = None, | |
| params: Optional[Dict[str, str]] = None, | |
| ) -> CompilePromptResponse: | |
| url = f"{self._base_url}/api/v1/studio/prompts/{prompt_id}/compile" | |
| if revision_id: | |
| url = f"{url}?revision={revision_id}" | |
| headers = { | |
| "X-Qualifire-API-Key": self._api_key, | |
| "Content-Type": "application/json", | |
| } | |
| if not params: | |
| params = {} | |
| request_body = json.dumps({"variables": params}) | |
| response = requests.post( | |
| url=url, | |
| data=request_body, | |
| headers=headers, | |
| verify=self._verify, | |
| ) | |
| response.raise_for_status() | |
| return CompilePromptResponse(**response.json()) | |
| def compile_prompt( | |
| self, | |
| prompt_id: str, | |
| revision_id: Optional[str] = None, | |
| params: Optional[Dict[str, str]] = None, | |
| ) -> CompilePromptResponse: | |
| url = f"{self._base_url}/api/v1/studio/prompts/{prompt_id}/compile" | |
| if revision_id: | |
| url = f"{url}?revision={revision_id}" | |
| headers = { | |
| "X-Qualifire-API-Key": self._api_key, | |
| "Content-Type": "application/json", | |
| } | |
| if not params: | |
| params = {} | |
| request_body = json.dumps({"variables": params}) | |
| response = requests.post( | |
| url=url, | |
| data=request_body, | |
| headers=headers, | |
| verify=self._verify, | |
| ) | |
| if response.status_code != 200: | |
| message = f"Qualifire API error: {response.status_code}" | |
| if response.text: | |
| message += f" - {response.text}" | |
| raise Exception(message) | |
| return CompilePromptResponse(**response.json()) |
🤖 Prompt for AI Agents
In `@qualifire/client.py` around lines 299 - 326, The compile_prompt method
currently calls response.raise_for_status() which is inconsistent with
evaluate() and invoke_evaluation(); change compile_prompt to check the response
(e.g., if not response.ok) and raise the same style of Exception used elsewhere
with a clear message including response.status_code and response.text (for
example: "Error compiling prompt: <status> - <response_text>") before returning
CompilePromptResponse(**response.json()); keep the rest of the request logic and
header handling intact and reference the compile_prompt function name when
applying the change.
| class LLMToolCall(BaseModel): | ||
| name: str | ||
| arguments: Dict[str, Any] | ||
| id: Optional[str] |
There was a problem hiding this comment.
Missing default for id field makes it required despite Optional annotation.
In Pydantic v2, Optional[str] without a default value means the field is still required (callers must explicitly pass id=None). If the intent is to make id truly optional (can be omitted), add a default value.
Proposed fix
class LLMToolCall(BaseModel):
name: str
arguments: Dict[str, Any]
- id: Optional[str]
+ id: Optional[str] = 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.
| class LLMToolCall(BaseModel): | |
| name: str | |
| arguments: Dict[str, Any] | |
| id: Optional[str] | |
| class LLMToolCall(BaseModel): | |
| name: str | |
| arguments: Dict[str, Any] | |
| id: Optional[str] = None |
🤖 Prompt for AI Agents
In `@qualifire/types.py` around lines 26 - 29, The LLMToolCall Pydantic model
marks id as Optional[str] but omits a default, making it required in Pydantic
v2; update the LLMToolCall class so the id field has an explicit default of None
(e.g., set id to None) to make it truly optional and ensure callers can omit it.
Reference: class LLMToolCall and the id field.
Description
Related Issue
https://linear.app/qualifire/issue/FIRE-1028/sdk-integration-add-compile-prompt-function-to-receive-the-request
Type of Change
Checklist
CODE_OF_CONDUCT.mddocument.CONTRIBUTING.mdguide.make codestyle.Summary by CodeRabbit
New Features
Chores
Other
✏️ Tip: You can customize this high-level summary in your review settings.