-
Notifications
You must be signed in to change notification settings - Fork 484
Migrate pip to uv #479
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
base: main
Are you sure you want to change the base?
Migrate pip to uv #479
Conversation
- Dockerfile: switch base image to python:3.11-slim, install supervisor, copy uv binary, use pyproject.toml + uv.lock and `uv sync` to install deps, set VIRTUAL_ENV/PATH, run NLTK download via `uv run`. - .github/workflows/pre-commit.yml: install uv in workflow and install pre-commit using `uv tool install` instead of pip. - start.sh / start.ps1: require `uv`, run `uv sync`, run migrations and services via `uv run` (alembic, uvicorn/gunicorn, celery); remove direct pip/.venv installs. - start_event_listener.sh / start_event_worker.sh: remove .venv checks and use `uv run` to start Python and Celery. - stop.sh: comment out `docker compose down` to avoid automatically tearing down services.
…compatible with python 3.11
- update base images from python:3.10-slim to python:3.11-slim - replace requirements/pip installs with astral-sh/uv: copy uv, copy pyproject.toml & uv.lock, run `uv sync` - run nltk downloads via `uv run python ...` - ensure virtualenv bin on PATH (VIRTUAL_ENV/PATH) - install supervisor in image and clean apt cache - update .gitignore to include .ruff_cache and .pytest_cache
…-compose.yaml - This makes sure that shutting down the containers keeps the data persistent
…t, and add Bandit settings - bump pre-commit-hooks to v5.0.0 - bump ruff-pre-commit to v0.8.2 and add ruff-format (exclude ^app/alembic/) - remove Black hook - bump Bandit to 1.7.10 and add args/additional_dependencies/exclude for TOML config - add [tool.bandit] block to pyproject.toml
WalkthroughThis PR migrates the project to Python 3.11 and replaces pip-based workflows with the uv package manager across CI, docs, Dockerfiles, and startup scripts; it updates dependency manifests, pre-commit hooks, adds gitignore entries, adjusts a small agent call, and adds tolerant imports for a tool. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer / CI
participant Shell as Shell / Script
participant UV as uv (package manager)
participant Venv as .venv (uv-managed)
participant App as Application (alembic, uvicorn, celery)
rect `#E8F6EF`
Dev->>Shell: run setup/start script / pre-commit step
Shell->>UV: check `uv` available
end
rect `#FFF3E0`
Shell->>UV: `uv sync --frozen --no-cache`
UV->>Venv: create/prepare .venv and install deps
Venv-->>Shell: PATH updated (VIRTUAL_ENV/bin prepended)
end
rect `#E8F0FF`
Shell->>UV: `uv run alembic upgrade heads` / `uv run uvicorn` / `uv run celery`
UV->>App: run requested command inside .venv
App-->>Dev: command output / status
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
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: 6
♻️ Duplicate comments (8)
deployment/prod/convo-server/convo.Dockerfile (2)
17-17: Note: The--no-cacheconcern applies here as well.Same as noted in deployment/prod/celery/celery.Dockerfile line 17.
24-24: Note: NLTK error handling concern applies here too.Same as noted in deployment/prod/celery/celery.Dockerfile line 24.
deployment/prod/mom-api/api.Dockerfile (2)
17-17: Note: The--no-cacheconcern applies here as well.Same as noted in deployment/prod/celery/celery.Dockerfile line 17.
24-24: Note: NLTK error handling concern applies here too.Same as noted in deployment/prod/celery/celery.Dockerfile line 24.
deployment/stage/mom-api/api.Dockerfile (2)
17-17: Note: The--no-cacheconcern applies here as well.Same as noted in deployment/prod/celery/celery.Dockerfile line 17.
24-24: Note: NLTK error handling concern applies here too.Same as noted in deployment/prod/celery/celery.Dockerfile line 24.
deployment/stage/convo-server/convo.Dockerfile (2)
17-17: Note: The--no-cacheconcern applies here as well.Same as noted in deployment/prod/celery/celery.Dockerfile line 17.
24-24: Note: NLTK error handling concern applies here too.Same as noted in deployment/prod/celery/celery.Dockerfile line 24.
🧹 Nitpick comments (3)
pyproject.toml (1)
54-55: pytest appears in both main and dev dependencies.Both
pytest>=8.4.2(line 54) andpytest-asyncio>=1.2.0(line 55) are listed in the main dependencies section, and they are also repeated in the[dependency-groups] devsection (lines 82-83). Test frameworks typically belong only in development dependencies.Consider removing pytest and pytest-asyncio from the main dependencies list to avoid duplication and keep the production environment lean:
"pyjwt>=2.10.1", - "pytest>=8.4.2", - "pytest-asyncio>=1.2.0", "python-dotenv>=1.2.1",README.md (1)
206-207: Add clarity to configuration block formatting.The model format explanation is helpful but could benefit from clearer delineation. Ensure consistent formatting with other configuration blocks in the document.
start_event_listener.sh (1)
18-24: Improve error handling for uv command execution.The script uses
nohup uv run pythonbut doesn't validate if the command succeeded. Ifuv runfails, the script still reports success, which could mask startup issues.Consider adding error checking:
echo "🎧 Starting event listener in background..." -nohup uv run python test_event_listener.py > event_listener.log 2>&1 & -LISTENER_PID=$! +nohup uv run python test_event_listener.py > event_listener.log 2>&1 & +LISTENER_PID=$! + +# Verify the process started +sleep 1 +if ! kill -0 $LISTENER_PID 2>/dev/null; then + echo "❌ Failed to start event listener. Check event_listener.log for details." + exit 1 +fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
.github/workflows/pre-commit.yml(1 hunks).gitignore(1 hunks).pre-commit-config.yaml(2 hunks).python-version(1 hunks)GETTING_STARTED.md(3 hunks)README.md(10 hunks)app/modules/intelligence/agents/chat_agents/pydantic_agent.py(0 hunks)app/modules/intelligence/tools/code_query_tools/code_analysis.py(1 hunks)contributing.md(5 hunks)deployment/prod/celery/celery.Dockerfile(1 hunks)deployment/prod/convo-server/convo.Dockerfile(1 hunks)deployment/prod/mom-api/api.Dockerfile(1 hunks)deployment/stage/celery/celery.Dockerfile(1 hunks)deployment/stage/convo-server/convo.Dockerfile(1 hunks)deployment/stage/mom-api/api.Dockerfile(1 hunks)docker-compose.yaml(3 hunks)dockerfile(1 hunks)pyproject.toml(1 hunks)requirements.txt(1 hunks)start.ps1(1 hunks)start.sh(1 hunks)start_event_listener.sh(1 hunks)start_event_worker.sh(1 hunks)
💤 Files with no reviewable changes (1)
- app/modules/intelligence/agents/chat_agents/pydantic_agent.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-26T07:15:11.952Z
Learnt from: DeepeshKalura
Repo: potpie-ai/potpie PR: 233
File: app/potpie.py:431-431
Timestamp: 2025-01-26T07:15:11.952Z
Learning: In the potpie project, environment variables use camelCase naming convention (e.g., `defaultUsername`) instead of the standard uppercase format.
Applied to files:
GETTING_STARTED.mdcontributing.md
🪛 LanguageTool
GETTING_STARTED.md
[uncategorized] ~65-~65: The official name of this software platform is spelled with a capital “H”.
Context: ...eployment with Firebase authentication, Github access, Secret Management etc ## Fireb...
(GITHUB)
README.md
[grammar] ~130-~130: Use a hyphen to join words.
Context: ...nstall.sh | sh`) ### Potpie UI An easy to use interface to interact with your A...
(QB_NEW_EN_HYPHEN)
[grammar] ~130-~130: Use a hyphen to join words.
Context: ...all.sh | sh`) ### Potpie UI An easy to use interface to interact with your Agen...
(QB_NEW_EN_HYPHEN)
[uncategorized] ~387-~387: Do not mix variants of the same word (‘initialise’ and ‘initialize’) within a single text.
Context: ...oolsdirectory for your custom agents. Initialise the tools in the app/modules/intellig...
(EN_WORD_COHERENCY)
🪛 markdownlint-cli2 (0.18.1)
README.md
174-174: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🪛 OSV Scanner (2.2.4)
requirements.txt
[HIGH] 131-131: llama-index-core 0.10.68.post1: LlamaIndex vulnerable to DoS attack through uncontrolled recursive JSON parsing
[HIGH] 131-131: llama-index-core 0.10.68.post1: LlamaIndex affected by a Denial of Service (DOS) in JSONReader
[HIGH] 131-131: llama-index-core 0.10.68.post1: llama-index-core insecurely handles temporary files
🔇 Additional comments (22)
app/modules/intelligence/tools/code_query_tools/code_analysis.py (1)
569-572: LGTM! Good defensive import pattern.The try-except fallback correctly handles LangChain's package reorganization, prioritizing the newer
langchain_core.toolsimport path while maintaining backward compatibility with older releases. This aligns well with the PR's dependency updates..gitignore (1)
76-78: LGTM!Adding
.ruff_cache/and.pytest_cache/to.gitignoreis appropriate given the migration to ruff-based linting and the inclusion of pytest in the development dependencies..python-version (1)
1-1: Verify Python version consistency with pyproject.toml.The
.python-versionfile pins Python to 3.11, whilepyproject.tomlspecifiesrequires-python = ">=3.10". This discrepancy means the development environment will use Python 3.11, but the project claims compatibility with Python 3.10+.If the project truly supports Python 3.10+, consider testing with Python 3.10 to ensure compatibility. If Python 3.11+ is actually required, update
pyproject.tomltorequires-python = ">=3.11"..pre-commit-config.yaml (4)
3-3: LGTM!The pre-commit hook version updates align well with the migration to uv-based dependency management and the new pyproject.toml configuration.
12-12: Good addition!Adding the
check-tomlhook is appropriate given the introduction ofpyproject.tomlas the central configuration file in this PR.
19-21: Appropriate exclusion pattern.Excluding
app/alembic/from ruff checks makes sense since Alembic migrations are often auto-generated and should not be subject to the same linting standards as application code.
27-29: Good integration with pyproject.toml.Configuring bandit to read from
pyproject.tomland adding thebandit[toml]dependency ensures consistent security scanning configuration across the project.start_event_worker.sh (1)
20-20: LGTM!The migration from
.venv/bin/celerytouv run celeryis correct and aligns with the broader uv adoption strategy in this PR.start.ps1 (2)
54-66: LGTM!The migration to uv-based environment management is implemented correctly with appropriate error handling. The
uv synccommand replaces the previous pip-based dependency installation.
69-82: Consistent uv integration.All service startup commands (
alembic,uvicorn,celery) correctly use theuv runprefix to execute within the uv-managed environment, maintaining the same arguments and options as before.start.sh (2)
29-40: LGTM!The uv availability check and environment synchronization are implemented correctly with proper error handling. The approach is consistent with the PowerShell script (
start.ps1) for cross-platform consistency.
43-50: Consistent migration to uv.All service startup commands (
alembic,gunicorn,celery) correctly useuv runto execute within the managed environment, preserving all original arguments and configuration.pyproject.toml (2)
66-66: No action needed—torch 2.9.0 is the latest stable version.Verification confirms that torch 2.9.0 is the current latest available version on PyPI, making the constraint
torch>=2.9.0valid and appropriate. The original concern that this version "may not exist yet" was unfounded.
78-78: Ignore this review comment—the concern is based on incorrect timing.The black version constraint
black>=25.9.0is valid. The latest available version of Black on PyPI is 25.9.0, and using Black's YY.MM.PATCH format, this refers to September 2025, which has already passed relative to the PR date (November 2025). The constraint is appropriate and requires no changes.Likely an incorrect or invalid review comment.
deployment/stage/celery/celery.Dockerfile (1)
10-24: Solid uv integration with proper layer caching and environment setup.The Dockerfile correctly handles uv-based dependency management:
- Frozen lock file ensures reproducible builds
- Virtual environment is properly configured on PATH
- NLTK data download is executed within the managed environment
Verify that the supervisord configuration file exists at
deployment/stage/celery/celery-api-supervisord.confto ensure the image builds correctly.GETTING_STARTED.md (1)
1-61: Excellent setup documentation with clear uv-based workflow.The development setup section is comprehensive and provides clear step-by-step instructions for the new uv-based environment. The model configuration examples and provider documentation links are helpful for users.
contributing.md (2)
32-61: Well-structured contributor onboarding with clear setup guidance.The Getting Started section effectively guides contributors through prerequisites, forking, and environment setup. The reference to README for detailed setup is appropriate.
89-125: Clear testing and FAQ sections enhance contributor clarity.The Testing section appropriately emphasizes test coverage, and the FAQ provides valuable clarification on ENV vs isDevelopmentMode configuration, which aligns with project conventions.
requirements.txt (1)
1-10: Verify uv.lock will be committed alongside requirements.txt update.This migration introduces a
requirements.txtthat appears to be auto-generated frompyproject.toml. Ensureuv.lockis committed to the repository to guarantee reproducible builds and prevent dependency drift in CI/CD and deployments.Also applies to: 20-50
docker-compose.yaml (1)
11-12: Persistent volumes correctly added for data durability.Named volumes for PostgreSQL, Neo4j, and Redis ensure data persists across container lifecycle changes, which is critical for local development and staging environments. Volume naming and mount paths follow standard practices.
Also applies to: 32-34, 43-44, 48-52
dockerfile (1)
10-24: Well-executed uv-based Docker build with proper dependency caching.The Dockerfile correctly implements uv-based dependency management with optimized layer caching and frozen dependencies. Environment configuration and NLTK data setup are properly managed.
Confirm that
supervisord.confexists in the repository root to ensure the image builds successfully.deployment/prod/celery/celery.Dockerfile (1)
17-17: Clarify the intent of--no-cacheonuv sync.The
--no-cacheflag skips uv's internal package cache, which could slow down iterative local builds. Confirm whether this is intentional for CI/CD consistency or an oversight.
| - name: Install uv | ||
| run: curl -LsSf https://astral.sh/uv/install.sh | sh | ||
|
|
||
| - name: Install dependencies | ||
| run: pip install --upgrade pip pre-commit # Ensure latest pre-commit version | ||
| run: | | ||
| export PATH="$HOME/.local/bin:$PATH" | ||
| uv tool install pre-commit |
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.
PATH may not persist for the pre-commit execution step.
The export PATH="$HOME/.local/bin:$PATH" on line 28 only affects the current step. The "Run pre-commit" step (lines 31-32) executes in a separate shell and won't have access to this PATH modification, which could cause the pre-commit command to fail.
Apply this diff to ensure the PATH is available for pre-commit execution:
- name: Install uv
run: curl -LsSf https://astral.sh/uv/install.sh | sh
- name: Install dependencies
run: |
export PATH="$HOME/.local/bin:$PATH"
uv tool install pre-commit
- name: Run pre-commit
- run: pre-commit run --all-files --show-diff-on-failure || true # Run all hooks without failing
+ run: |
+ export PATH="$HOME/.local/bin:$PATH"
+ pre-commit run --all-files --show-diff-on-failure || true # Run all hooks without failingCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .github/workflows/pre-commit.yml around lines 23 to 29, the step that exports
PATH only affects that single step so the following "Run pre-commit" step won't
see the change; update the workflow so the path is persisted across steps by
appending "$HOME/.local/bin" to the GITHUB_PATH file (or by setting PATH in the
job env) after installing uv and pre-commit, e.g. write the path to $GITHUB_PATH
so subsequent steps can run the pre-commit binary.
| RUN pip install --no-cache-dir nltk | ||
| RUN python -c "import nltk; nltk.download('punkt');" | ||
| # Download required NLTK data inside the managed environment | ||
| RUN uv run python -c "import nltk; nltk.download('punkt')" |
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.
Add error handling for NLTK data download.
If nltk.download('punkt') fails silently, the image builds successfully but may fail at runtime when NLTK data is accessed. Consider explicitly verifying the download or letting failures propagate to fail the build.
Consider this approach to ensure the download succeeds:
- RUN uv run python -c "import nltk; nltk.download('punkt')"
+ RUN uv run python -c "import nltk; nltk.download('punkt', raise_errors=True)" || (echo 'NLTK download failed' && exit 1)📝 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.
| RUN uv run python -c "import nltk; nltk.download('punkt')" | |
| RUN uv run python -c "import nltk; nltk.download('punkt', raise_errors=True)" || (echo 'NLTK download failed' && exit 1) |
| langsmith==0.4.41 | ||
| litellm==1.79.1 | ||
| llama-index-agent-openai==0.2.9 | ||
| llama-index-core==0.10.68.post1 |
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.
🧩 Analysis chain
CRITICAL: Resolve HIGH severity vulnerabilities in llama-index-core.
OSV Scanner flagged three HIGH severity vulnerabilities in llama-index-core==0.10.68.post1:
- GHSA-3wxx-q3gv-pvvv: DoS attack via uncontrolled recursive JSON parsing
- GHSA-7753-xrfw-ch36: DoS in JSONReader
- GHSA-cr7q-2w66-hjcm: Insecure temporary file handling
These vulnerabilities pose production risk and must be resolved before merge.
Please verify a patched version is available by checking:
If no patched version exists, either:
- Downgrade to a known-secure version
- Remove this dependency if no longer needed
- Document the risk mitigation strategy if the vulnerability is not exploitable in your use case
🌐 Web query:
llama-index-core security patches and latest version
💡 Result:
Latest published llama-index-core release: 0.14.7 (released Oct 30, 2025). [1][2]
Known security issues and the fixed versions:
- CVE-2025-7647 (insecure temporary file / predictable cache dir) — fixed in 0.13.0; upgrade to ≥0.13.0. [3]
- CVE-2025-5472 (uncontrolled recursive JSON parsing / DoS) — fixed in ≥0.12.38. [4]
- Path-traversal in encode_image — fixed in 0.12.41. [4]
- CVE-2025-3108 (unsafe JsonPickleSerializer / unsafe pickle fallback) — addressed/renamed and warned in 0.12.41. [5]
- CVE-2024-3271 (safe_eval command injection) — fixed in ≥0.10.24. [4]
Recommendation: upgrade to the latest stable llama-index-core (0.14.7 as of Oct 30, 2025) and audit any use of serializers, file-path inputs, and shared /tmp usage. [1][3][4]
Sources:
[1] PyPI llama-index-core release listing.
[2] GitHub releases (v0.14.7).
[3] GitLab advisory / CVE-2025-7647.
[4] GitLab advisories summary (multiple fixes; versions noted).
[5] GitLab advisory / CVE-2025-3108.
Upgrade llama-index-core to 0.14.7 to resolve HIGH severity vulnerabilities.
The current pinned version (0.10.68.post1) is vulnerable to multiple HIGH severity issues that have been fixed in later releases:
- CVE-2025-7647 (insecure temporary file / predictable cache directory) — fixed in 0.13.0+
- CVE-2025-5472 (uncontrolled recursive JSON parsing DoS) — fixed in 0.12.38+
- Path-traversal in encode_image — fixed in 0.12.41+
Action: Upgrade to the latest stable version (0.14.7, released Oct 30, 2025) and audit any use of serializers, file-path inputs, and shared /tmp usage.
🧰 Tools
🪛 OSV Scanner (2.2.4)
[HIGH] 131-131: llama-index-core 0.10.68.post1: LlamaIndex vulnerable to DoS attack through uncontrolled recursive JSON parsing
[HIGH] 131-131: llama-index-core 0.10.68.post1: LlamaIndex affected by a Denial of Service (DOS) in JSONReader
[HIGH] 131-131: llama-index-core 0.10.68.post1: llama-index-core insecurely handles temporary files
🤖 Prompt for AI Agents
In requirements.txt around line 131, replace the pinned
llama-index-core==0.10.68.post1 with llama-index-core==0.14.7 to address
multiple HIGH severity vulnerabilities; after updating, run pip-compile or your
dependency manager to update lockfiles, then audit the codebase for uses of
serializers, any file-path inputs (especially encode_image), and shared /tmp or
temporary file usage to ensure path sanitization and non-predictable temp file
creation.
|



This pull request introduces several improvements to the development and setup workflow for Potpie, focusing on modernizing Python dependency management, updating documentation, and refining pre-commit tooling. The main changes include switching to the
uvpackage manager for Python dependencies, updating documentation to reflect the new setup process, and upgrading pre-commit hooks and configurations.Development workflow and dependency management:
pipto the fasteruvpackage manager in both.github/workflows/pre-commit.ymland documentation, and ensured Python 3.11 is the required version across the project. (.github/workflows/pre-commit.yml,.python-version,README.md,GETTING_STARTED.md)README.md,GETTING_STARTED.md, andcontributing.mdto useuvfor dependency installation and clarified environment setup steps for both development and production.Pre-commit and code quality tooling:
pre-commit-hooks,ruff-pre-commit,bandit) and refined configuration, including new exclusions and arguments for specific hooks. Removedblackhook and addedcheck-tomlandruff-formathooks. (.pre-commit-config.yaml)Documentation and onboarding improvements:
GETTING_STARTED.mdandREADME.mdto clarify setup steps, environment configuration, and integration with third-party services (Firebase, PostHog, GitHub, Google Cloud). Added step-by-step instructions and improved formatting for easier onboarding.Minor codebase fixes:
universal_analyze_code_toolfor compatibility with different LangChain versions. (app/modules/intelligence/tools/code_query_tools/code_analysis.py) (Required for python 3.11)result_typeargument in agent creation for cleaner code. (app/modules/intelligence/agents/chat_agents/pydantic_agent.py) (Was removed in the pydantic builds for python 3.11)These changes collectively modernize the developer experience, streamline dependency management, and improve onboarding and contribution processes.
Summary by CodeRabbit
Documentation
Chores