Skip to content

Conversation

@Nischay-VideoDB
Copy link

@Nischay-VideoDB Nischay-VideoDB commented Nov 7, 2025

  • Add a Windows-first setup flow via setup.bat
  • Add launcher scripts (start-all.bat, start-backend.bat, start-frontend.bat) for consistent local runs
  • Update README with Windows setup instructions and a navigable table of contents

Summary by CodeRabbit

  • Documentation

    • Enhanced README with Table of Contents and in-page navigation anchors.
    • Added Architecture Overview and Reasoning Engine sections.
    • Reorganized Getting Started with clearer, platform-specific installation steps.
  • New Features

    • Added Windows setup automation scripts for environment configuration.
    • Added Windows scripts to launch backend and frontend services.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 7, 2025

Walkthrough

This 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

Cohort / File(s) Summary
Documentation
README.md
Restructured with Table of Contents, in-page navigation anchors, expanded Key Features and Architecture Overview sections, reorganized Getting Started with platform-specific setup flows (Windows/macOS/Linux), and integrated setup script references.
Windows Setup & Startup Scripts
setup.bat, start-all.bat, start-backend.bat, start-frontend.bat
Introduces four batch scripts: setup.bat orchestrates full environment initialization (Python venv, Node.js validation, dependency installation, .env configuration); start-all.bat coordinates backend and frontend service launch; start-backend.bat validates backend initialization and starts the API server; start-frontend.bat manages frontend vite server startup with available port detection and fallback logic.

Sequence Diagram

sequenceDiagram
    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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Batch script validation logic: Verify version parsing (Node.js 22.8.0+), venv detection, and error handling paths in setup.bat and start-backend.bat
  • Port detection and fallback mechanisms: Review PowerShell-based port probing and loopback fallback logic in start-frontend.bat for correctness and edge cases
  • Environment file manipulation: Ensure set_env_value helper correctly updates or appends key-value pairs without duplication across scripts
  • Cross-script dependencies: Validate that start-all.bat correctly orchestrates start-backend.bat and start-frontend.bat with proper error propagation
  • README navigation structure: Confirm all anchor tags are correctly formatted and table of contents links resolve properly

Possibly related PRs

  • Ashu/fix launch v1 #96: Both PRs modify and expand README.md with updates to introduction, features, architecture sections, and Getting Started documentation, forming part of a broader documentation reorganization effort.

Suggested reviewers

  • ashish-spext
  • ankit-v2-3

Poem

🐰 A rabbit hops through setup's gate,
With batch scripts neat and first-rate,
Windows now sings, services start,
README's maps show every part,
Jump links guide from end to start!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: addition of Windows batch files for installation and README updates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@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: 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

📥 Commits

Reviewing files that changed from the base of the PR and between e323157 and 373249a.

📒 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)

Comment on lines +186 to 190
> - 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
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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.

Suggested change
> - 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.

Comment on lines +98 to +105
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

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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.

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.

1 participant