-
Notifications
You must be signed in to change notification settings - Fork 196
Add .bat files for easier installation on windows and Update readme.md #184
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
WalkthroughThis PR introduces comprehensive Windows automation for environment setup and service orchestration, alongside a major README restructure. Four new batch scripts handle dependency validation, Python/Node.js environment configuration, virtual environment creation, and coordinated service startup. The README gains navigation anchors, expanded architecture documentation, and platform-specific setup instructions. Changes
Sequence DiagramsequenceDiagram
actor User
participant setup.bat
participant Backend
participant Frontend
participant User2 as User (Start Services)
participant start-all.bat
participant start-backend.bat
participant start-frontend.bat
User->>setup.bat: Execute setup.bat
rect rgb(200, 220, 255)
note over setup.bat: Python & Node.js Validation
setup.bat->>Backend: Create venv, install deps
setup.bat->>Backend: Initialize SQLite DB
setup.bat->>Frontend: Install npm dependencies
setup.bat->>Frontend: Generate .env (VITE_*)
end
setup.bat-->>User: ✓ Setup Complete
User2->>start-all.bat: Execute start-all.bat
rect rgb(200, 255, 220)
note over start-all.bat: Service Orchestration
start-all.bat->>start-backend.bat: Launch backend
start-all.bat->>start-frontend.bat: Launch frontend
start-backend.bat->>Backend: Validate venv, start API (port 8000)
start-frontend.bat->>Frontend: Detect available port, start dev server (port 8080)
end
start-backend.bat-->>User2: Backend running
start-frontend.bat-->>User2: Frontend running
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
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: 2
🧹 Nitpick comments (1)
README.md (1)
115-117: Replace generic link text with descriptive titles.
[link](...)blocks screen readers from conveying the destination and violates the MD059 lint rule that fired in CI. Replacing “link” with meaningful copy fixes the warning and improves accessibility.- 1. Highlight Creator: [link](https://www.youtube.com/watch?v=Dncn_0RWrro&list=PLhxAMFLSSK039xl1UgcZmoFLnb-qNRYQw&index=11) - 2. Text to Movie: [link](https://www.youtube.com/watch?v=QpnRxuEBDCc&list=PLhxAMFLSSK039xl1UgcZmoFLnb-qNRYQw&index=2) - 3. Video Search: [link](https://www.youtube.com/watch?v=kCiCI2KCnC8&list=PLhxAMFLSSK039xl1UgcZmoFLnb-qNRYQw&index=4) + 1. Highlight Creator: [Watch the highlight demo](https://www.youtube.com/watch?v=Dncn_0RWrro&list=PLhxAMFLSSK039xl1UgcZmoFLnb-qNRYQw&index=11) + 2. Text to Movie: [Watch the text-to-movie demo](https://www.youtube.com/watch?v=QpnRxuEBDCc&list=PLhxAMFLSSK039xl1UgcZmoFLnb-qNRYQw&index=2) + 3. Video Search: [Watch the video search demo](https://www.youtube.com/watch?v=kCiCI2KCnC8&list=PLhxAMFLSSK039xl1UgcZmoFLnb-qNRYQw&index=4)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(7 hunks)setup.bat(1 hunks)start-all.bat(1 hunks)start-backend.bat(1 hunks)start-frontend.bat(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md
115-115: Link text should be descriptive
(MD059, descriptive-link-text)
116-116: Link text should be descriptive
(MD059, descriptive-link-text)
117-117: Link text should be descriptive
(MD059, descriptive-link-text)
| > - Install Node.js 22.8.0 using nvm (Mac/Linux) | ||
| > - Verify Python 3.9+ and Node.js 22.8.0+ are installed (Windows) | ||
| > - Install Python and pip | ||
| > - Set up virtual environments for both frontend and backend. | ||
|
|
||
| > - Set up virtual environments for both frontend and backend | ||
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.
Clarify what setup.bat actually installs.
setup.bat exits if Python is missing on Windows; it never installs Python or pip. Leaving this bullet as-is sets the wrong expectation and ships misleading setup docs.
Apply this diff to reflect the real behavior:
-> - Install Python and pip
+> - Install backend Python dependencies inside a virtual environment📝 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.
| > - Install Node.js 22.8.0 using nvm (Mac/Linux) | |
| > - Verify Python 3.9+ and Node.js 22.8.0+ are installed (Windows) | |
| > - Install Python and pip | |
| > - Set up virtual environments for both frontend and backend. | |
| > - Set up virtual environments for both frontend and backend | |
| > - Install Node.js 22.8.0 using nvm (Mac/Linux) | |
| > - Verify Python 3.9+ and Node.js 22.8.0+ are installed (Windows) | |
| > - Install backend Python dependencies inside a virtual environment | |
| > - Set up virtual environments for both frontend and backend |
🤖 Prompt for AI Agents
In README.md around lines 186-190, the docs misleadingly imply setup.bat
installs Python and pip; instead update the bullets to state that setup.bat only
checks for Python on Windows and will exit if it is missing (it does not install
Python or pip), replace "Install Python and pip" with "Ensure Python 3.9+ and
pip are installed (install manually if missing)" and clarify that Windows
instructions verify Python/Node versions but that Python must be installed by
the user before running setup.bat.
| echo Initializing SQLite database... | ||
| set SQLITE_DB_PATH=director.db | ||
| %PY_CMD% director\db\sqlite\initialize.py >nul 2>&1 | ||
|
|
||
| call venv\Scripts\deactivate.bat >nul 2>&1 | ||
| popd | ||
| echo [OK] Backend ready | ||
|
|
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.
Run DB initialization inside the venv and fail fast on errors.
Right now %PY_CMD% reuses the global interpreter (ignoring the freshly activated venv), redirects its output away, and never checks the exit code. If the initializer needs packages you just installed, it crashes silently yet setup reports success.
-set SQLITE_DB_PATH=director.db
-%PY_CMD% director\db\sqlite\initialize.py >nul 2>&1
-
-call venv\Scripts\deactivate.bat >nul 2>&1
+set SQLITE_DB_PATH=director.db
+python director\db\sqlite\initialize.py >nul 2>&1 || goto :fail_backend
+
+call venv\Scripts\deactivate.bat >nul 2>&1🤖 Prompt for AI Agents
In setup.bat around lines 98 to 105, the script runs the DB initializer with
%PY_CMD% outside the activated virtualenv, suppresses all output, and ignores
the exit code so failures are silent; update the step to invoke the virtualenv's
python (venv\Scripts\python.exe) after activation, do not redirect stdout/stderr
to nul so errors are visible, and immediately check %ERRORLEVEL% (or use
conditional CALL/IF ERRORLEVEL) to abort the batch with a non‑zero exit and an
error message if the initializer fails.
Summary by CodeRabbit
Documentation
New Features