-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat: Complete uv migration - remove requirements.txt, add workspace config, and modernize docs #513
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?
Conversation
- Replace requirements.txt with pyproject.toml dependencies - Set up uv workspace configuration at project root - Update all npm scripts to use uv commands - Generate uv.lock for reproducible dependency resolution - Update documentation (README, SETUP, CONTRIBUTING) - Add environment setup instructions for .env files - Update error messages to suggest uv commands - Add UV_MIGRATION.md with comprehensive migration guide - Improve development workflow with faster dependency management Closes srbhr#409
WalkthroughMigrates Python dependency management to uv across backend and docs. Updates commands, scripts, and messages to use uv, introduces workspace config, replaces requirements.txt with pyproject.toml and dev tooling, adjusts VSCode Python settings, and revises setup/READMEs. Minor backend messages updated for DOCX dependency guidance. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||||
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/backend/install_docx_deps.py (1)
15-26: Critical: Script still uses pip instead of uv.The script's guidance message (line 51) tells users to use
uv add, but the script itself usespip installat line 22. This creates a critical inconsistency where the script's behavior doesn't match the project's migration to uv.Apply this diff to update the installation logic to use uv:
def install_dependencies(): """Install the missing dependencies for DOCX processing""" print("Installing markitdown with DOCX support...") dependencies = [ - "markitdown[all]==0.1.2" + "markitdown[all]>=0.1.2" ] for dep in dependencies: print(f"Installing {dep}...") try: - subprocess.check_call([sys.executable, "-m", "pip", "install", dep]) + subprocess.check_call(["uv", "add", dep]) print(f"✓ Successfully installed {dep}") except subprocess.CalledProcessError as e: print(f"✗ Failed to install {dep}: {e}") return FalseAlso update the version constraint from
==0.1.2to>=0.1.2for consistency with other files in this PR.
🧹 Nitpick comments (1)
apps/backend/pyproject.toml (1)
46-55: Dev dependencies are duplicated.Development dependencies are specified in both
[project.optional-dependencies](lines 47-55) and[tool.uv](lines 76-84) sections. This duplication can lead to maintenance issues and version drift.Choose one approach:
Option 1: Use only [project.optional-dependencies] (standard)
-[tool.uv] -dev-dependencies = [ - "pytest>=8.0.0", - "pytest-asyncio>=0.21.0", - "pytest-cov>=4.0.0", - "black>=23.0.0", - "isort>=5.12.0", - "mypy>=1.0.0", - "ruff>=0.1.0", -]Option 2: Use only [tool.uv] (uv-specific)
-[project.optional-dependencies] -dev = [ - "pytest>=8.0.0", - "pytest-asyncio>=0.21.0", - "pytest-cov>=4.0.0", - "black>=23.0.0", - "isort>=5.12.0", - "mypy>=1.0.0", - "ruff>=0.1.0", -]Note: The
[project.optional-dependencies]approach is more standard and tool-agnostic, while[tool.uv]is uv-specific. If using uv exclusively, Option 2 is cleaner. If maintaining compatibility with other tools, Option 1 is preferred.Also applies to: 75-84
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.github/CONTRIBUTING.md(2 hunks).vscode/settings.json(1 hunks)README.md(2 hunks)SETUP.md(3 hunks)UV_MIGRATION.md(1 hunks)apps/backend/app/services/resume_service.py(2 hunks)apps/backend/install_docx_deps.py(2 hunks)apps/backend/pyproject.toml(1 hunks)apps/backend/requirements.txt(0 hunks)apps/backend/test_docx_dependencies.py(2 hunks)docs/CONFIGURING.md(1 hunks)package.json(1 hunks)pyproject.toml(1 hunks)
💤 Files with no reviewable changes (1)
- apps/backend/requirements.txt
🧰 Additional context used
📓 Path-based instructions (3)
apps/backend/app/{models,services}/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use SQLAlchemy ORM with async sessions for database access
Files:
apps/backend/app/services/resume_service.py
apps/backend/app/services/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
apps/backend/app/services/**/*.py: Use temporary files for document processing and always clean up after processing
Implement async processing for file operations in backend services
Cache processed results when appropriate in backend services
Files:
apps/backend/app/services/resume_service.py
apps/backend/app/{agent,services}/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use structured prompts and validate AI model responses against Pydantic models
Files:
apps/backend/app/services/resume_service.py
🪛 markdownlint-cli2 (0.18.1)
UV_MIGRATION.md
70-70: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
SETUP.md
48-48: Blank line inside blockquote
(MD028, no-blanks-blockquote)
🔇 Additional comments (10)
pyproject.toml (1)
1-5: LGTM! Clean workspace configuration.The uv workspace setup correctly identifies the backend as a workspace member, and the explanatory comments help clarify the separation between Python (uv) and Node.js (npm) dependency management.
apps/backend/app/services/resume_service.py (1)
50-50: LGTM! Updated to uv command.The log message correctly reflects the migration from pip to uv for dependency installation.
UV_MIGRATION.md (1)
1-92: Excellent migration documentation!This comprehensive guide provides clear before/after comparisons, benefits, new commands, workspace structure, and a verification checklist. It will help developers understand and adopt the uv workflow effectively.
apps/backend/install_docx_deps.py (1)
40-42: LGTM! Correctly updated for uv workspace.The directory validation now checks for
pyproject.tomlinstead ofrequirements.txt, aligning with the uv-based project structure.apps/backend/test_docx_dependencies.py (1)
31-31: LGTM! Consistent uv command updates.All three installation guidance messages have been correctly updated to use
uv add 'markitdown[all]>=0.1.2', maintaining consistency with the project's migration to uv-based dependency management.Also applies to: 36-36, 88-88
apps/backend/pyproject.toml (2)
19-44: LGTM! Well-structured dependencies.All dependencies use minimum version constraints (
>=), which provides flexibility while ensuring compatibility. This aligns with the project's migration to uv.
89-132: Comprehensive tool configurations.The black, isort, mypy, ruff, and pytest configurations are well-defined and follow best practices for Python development tooling.
.github/CONTRIBUTING.md (3)
54-140: Excellent setup documentation!The updated installation instructions provide multiple setup paths (quick setup via npm, alternative methods via scripts, and manual setup), clear environment file examples with actual values, and helpful troubleshooting guidance. This significantly improves the contributor experience.
141-186: Clear development workflow with uv.The development workflow section effectively documents the transition to uv, providing common commands for both package management and development tasks, along with required environment variables. This helps developers quickly understand the new tooling.
205-211: LGTM! Flexible pre-commit installation options.Offering both
uv tool installand system-widepip installgives developers flexibility while encouraging the use of uv. This is a pragmatic approach during the migration period.
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.
5 issues found across 13 files
Prompt for AI agents (all 5 issues)
Understand the root cause of the following 5 issues and fix them.
<file name="UV_MIGRATION.md">
<violation number="1" location="UV_MIGRATION.md:24">
`UV_MIGRATION.md` states that a root-level `uv.lock` file was added, but the file is missing from the repository. Please either add the lockfile or adjust the documentation to reflect the current state.</violation>
</file>
<file name=".github/CONTRIBUTING.md">
<violation number="1" location=".github/CONTRIBUTING.md:58">
The quick-setup `npm install` step will fail on a fresh machine because the generated script immediately calls `uv sync`, but the docs don’t warn that uv must already be installed. Please call out the uv prerequisite here or adjust the instructions so the step succeeds.</violation>
</file>
<file name="apps/backend/pyproject.toml">
<violation number="1" location="apps/backend/pyproject.toml:64">
The new console script maps to `app.main:app`, but that symbol is the FastAPI application instance, not a zero-argument callable. Building/installing this package will generate a CLI that immediately crashes because packaging invokes the target as a function with no parameters, while `FastAPI` objects require ASGI scope arguments. Please point the script to an actual CLI function or remove it.</violation>
</file>
<file name=".vscode/settings.json">
<violation number="1" location=".vscode/settings.json:2">
VS Code settings force Conda as the default Python environment, which conflicts with the uv-based workflow and can lead to inconsistent environments. Remove this key or configure the interpreter to use the project’s uv-managed virtual environment.</violation>
</file>
<file name="SETUP.md">
<violation number="1" location="SETUP.md:3">
Broken Markdown image: the alt text contains list items, preventing the image from rendering and mangling the prerequisites list. Restore proper image syntax and place the bullet list below the image.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
Pull Request Title
feat: Complete UV migration - remove requirements.txt, add workspace config, and modernize docs
Related Issue
#512 (already closed, maintainer confirmed UV compatibility)
Description
This pull request finalizes the migration to UV. It removes old files, adds new configs, and updates the documentation so the project fully supports UV.
Type
Proposed Changes
requirements.txtand move dependencies topyproject.tomluv.lockfor reproducible installsREADME,SETUP,CONTRIBUTING).envsetup instructionsUV_MIGRATION.mdwith a step-by-step migration guideChecklist
Additional Information
This is my first open-source contribution. Feedback is welcome so I can improve further!
Summary by cubic
Completes migration from pip to uv to speed up installs and simplify setup. Dependencies now live in pyproject.toml, with a uv workspace and lockfile added, and all scripts and docs updated.
Summary by CodeRabbit
Documentation
Chores
Tests