Skip to content

Conversation

@sonianuj287
Copy link

@sonianuj287 sonianuj287 commented Oct 18, 2025

Pull Request Title

Related Issue

Description

Type

  • Bug Fix
  • Feature Enhancement
  • Documentation Update
  • Code Refactoring
  • Other (please specify):

Proposed Changes

Screenshots / Code Snippets (if applicable)

How to Test

Checklist

Additional Information


Summary by cubic

Removed the legacy FIX_ISSUE_409.md and added compatibility stubs to guide Streamlit users to the new FastAPI + Next.js setup. Updated CONTRIBUTING and README with the new setup flow and local run commands.

  • Migration
    • Added run_first.py and streamlit_app.py stubs that provide migration instructions and can attempt auto-setup.
    • Updated .github/CONTRIBUTING.md and README with the new architecture, setup scripts, and dev endpoints.
    • Removed FIX_ISSUE_409.md since the DOCX conversion issue (Error processing file conversion DocxConverter #409) is resolved.

Summary by CodeRabbit

  • Documentation

    • README updated with compatibility notes and migration guidance for FastAPI + Next.js.
    • Added step-by-step migration and quick-start guidance for legacy Streamlit users.
    • Removed the CONTRIBUTING guide and an older troubleshooting/POC document.
  • Chores

    • Added lightweight compatibility helpers to guide and optionally bootstrap migration from legacy workflow.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 18, 2025

Warning

Rate limit exceeded

@sonianuj287 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 25 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 958e973 and 0017ceb.

📒 Files selected for processing (4)
  • .github/CONTRIBUTING.md (0 hunks)
  • README.md (1 hunks)
  • run_first.py (1 hunks)
  • streamlit_app.py (1 hunks)

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Removes contributor guidance file, adds README notes for legacy users, and introduces two compatibility stub scripts (run_first.py, streamlit_app.py) to guide/migrate users from Streamlit to a FastAPI + Next.js stack. Also deletes FIX_ISSUE_409.md.

Changes

Cohort / File(s) Summary
Deleted contributor guide
\.github/CONTRIBUTING.md
Entire contributing guide removed (bug reporting, testing, setup, deployment, pre-commit, community links).
README updates
README.md
Adds "Legacy Users" note explaining run_first.py and streamlit_app.py act as compatibility stubs and instructing legacy commands for migration guidance.
Compatibility stubs (new)
run_first.py, streamlit_app.py
Adds run_first.py (migration banner, setup-file checks, OS-aware auto-setup attempts, CLI flow) and streamlit_app.py (migration banner, architecture comparison, setup checks, provide instructions, attempt to launch new app via npm). Multiple public functions added.
Deleted issue note
FIX_ISSUE_409.md
Removes documentation/POC for Issue #409 (diagnosis, solution, install/test instructions).

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Runner as run_first.py
    participant StreamStub as streamlit_app.py
    participant Setup as setup scripts (setup.sh / setup.ps1)
    participant NPM as npm / next dev

    User->>Runner: python run_first.py
    Runner->>User: Print migration banner & check setup files
    Runner->>Setup: Verify existence / optionally execute auto-setup
    Setup-->>Runner: status (found / executed / failed)
    Runner->>User: Suggest OS-aware next steps or confirm auto-setup

    User->>StreamStub: python streamlit_app.py
    StreamStub->>User: Print migration banner & show comparison
    StreamStub->>Setup: check_new_setup (package.json, scripts)
    Setup-->>StreamStub: ready / missing
    StreamStub->>User: Provide setup instructions
    alt setup ready & user confirms
        StreamStub->>NPM: npm run dev (launch)
        NPM-->>User: App output / running
    else missing or failure
        StreamStub->>User: Provide resources & manual steps
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

  • srbhr

Poem

🐇 A little hop, a helpful script in tow,

I guide old runners where new sidewalks go.
From Streamlit nests to FastAPI trees,
Next.js breezes and smoother keys.
Hop on — migration blooms, steady and slow.

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 pull request title "Add compatibility stubs for missing run_first.py and streamlit_app.py" directly aligns with the core objective and primary changes in the changeset. The PR's main purpose, as documented in the objectives, is to add two new compatibility stub files to guide users migrating from Streamlit to the new FastAPI + Next.js architecture. The title is specific and clear, accurately identifying the files being added and their purpose (compatibility stubs), and avoids vague or generic terminology. While the PR also includes secondary changes such as deleting CONTRIBUTING.md and FIX_ISSUE_409.md and updating README.md, the title appropriately focuses on the primary change, which is appropriate for a summary-level description.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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.

@qodo-merge-pro
Copy link

qodo-merge-pro bot commented Oct 18, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🟢
🎫 #533
🟢 Provide or restore the files named run_first.py and streamlit_app.py so users can find
them after cloning.
Ensure the repository clearly guides users on how to run the application given the
migration away from Streamlit.
Update documentation to clarify the new locations/roles of these entry points or their
replacements.
Confirm whether those files were removed/renamed and offer a working path for users
following older docs.
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@sonianuj287 sonianuj287 changed the title chore: remove FIX_ISSUE_409.md as the issue has been resolved Add compatibility stubs for missing run_first.py and streamlit_app.py Oct 18, 2025
@qodo-merge-pro
Copy link

qodo-merge-pro bot commented Oct 18, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Capture and show setup script errors

Capture and display the standard error output from the subprocess.run call to
provide users with detailed error messages if the setup script fails.

streamlit_app.py [256-262]

 result = subprocess.run([
     "powershell", "-ExecutionPolicy", "Bypass",
     "-File", str(setup_script)
-])
+], capture_output=True, text=True)
 if result.returncode != 0:
     print("❌ Setup failed. Please run setup manually.")
+    if result.stderr:
+        print(f"   Error: {result.stderr}")
     return False
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out a lack of error reporting and proposes capturing and displaying stderr, which significantly improves the debuggability and user experience of the setup script.

Medium
Possible issue
Prevent potential None value from markdown

To prevent a potential None value from markdown.markdown, provide an empty
string as a fallback for the updated_resume variable.

apps/backend/app/services/score_improvement_service.py [251]

-"updated_resume": markdown.markdown(text=updated_resume),
+"updated_resume": markdown.markdown(text=updated_resume or ""),

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that updated_resume could be None, and provides a simple fix to prevent markdown.markdown from returning None, which improves the robustness of the API response.

Low
  • Update

@sonianuj287
Copy link
Author

Hi @srbhr , Could you please review this PR and suggest me changes.

Thanks :)

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 (11)
README.md (1)

76-77: Clarify that stubs won’t launch the old app

To prevent confusion, explicitly state these stubs don’t start the legacy Streamlit app; they only guide migration, and link SETUP.md once more right here.

Apply:

-> **📝 Note for Legacy Users**: If you're looking for `run_first.py` or `streamlit_app.py`, these files now exist as compatibility stubs that will guide you through the migration to our new FastAPI + Next.js architecture. Simply run `python run_first.py` or `python streamlit_app.py` for migration instructions and automatic setup assistance.
+> **📝 Note for Legacy Users**: `run_first.py` and `streamlit_app.py` are compatibility stubs. They will not launch the old Streamlit app; they only guide you to the new FastAPI + Next.js setup. See SETUP.md for steps. To view guidance, run `python run_first.py` or `python streamlit_app.py`.
run_first.py (4)

119-131: Safer PowerShell resolution (+ pwsh fallback) and explicit binary path

Avoid relying on a partial executable name (S607) and prefer pwsh when available. Also add -NoProfile.

+import shutil
@@
-            setup_script = root_dir / "setup.ps1"
+            setup_script = root_dir / "setup.ps1"
             if setup_script.exists():
-                print("  Running Windows setup script...")
-                result = subprocess.run([
-                    "powershell", "-ExecutionPolicy", "Bypass", 
-                    "-File", str(setup_script)
-                ], capture_output=True, text=True)
+                print("  Running Windows setup script...")
+                ps_exe = shutil.which("pwsh") or shutil.which("powershell")
+                if not ps_exe:
+                    print("  ❌ PowerShell not found (pwsh/powershell). Please run setup manually.")
+                    return False
+                result = subprocess.run(
+                    [ps_exe, "-NoProfile", "-ExecutionPolicy", "Bypass", "-File", str(setup_script)],
+                    capture_output=True,
+                    text=True
+                )

137-150: Improve POSIX subprocess robustness

Optionally set check=True to surface failures via exceptions and echo stdout on failure for easier debugging.

-                result = subprocess.run([str(setup_script)], capture_output=True, text=True)
+                result = subprocess.run([str(setup_script)], capture_output=True, text=True)
                 
                 if result.returncode == 0:
                     print("  ✅ Setup completed successfully!")
                     print("  🌐 You can now run: npm run dev")
                     return True
                 else:
-                    print(f"  ❌ Setup failed: {result.stderr}")
+                    print(f"  ❌ Setup failed:\nSTDERR:\n{result.stderr}\nSTDOUT:\n{result.stdout}")
                     return False

152-154: Avoid bare except Exception

Catching all exceptions obscures real errors (BLE001). Narrow the scope to OS/subprocess errors and let unexpected exceptions bubble up.

-    except Exception as e:
+    except (OSError, subprocess.SubprocessError) as e:
         print(f"  ❌ Auto-setup failed: {e}")
         return False
-    except Exception as e:
+    except (OSError, subprocess.SubprocessError) as e:
         print(f"\n❌ Error: {e}")
         print("Please follow the manual setup instructions above.")

Also applies to: 176-178


164-173: Don’t block in non-interactive environments

Guard input() behind a TTY check so CI or headless users don’t hang.

-    try:
-        response = input("\n❓ Would you like to attempt automatic setup? (y/N): ").strip().lower()
+    try:
+        if not sys.stdin.isatty():
+            response = "n"
+        else:
+            response = input("\n❓ Would you like to attempt automatic setup? (y/N): ").strip().lower()
.github/CONTRIBUTING.md (2)

135-157: Call out prerequisite for uv explicitly

You mention uv sync elsewhere in docs/workflow; add a one-liner here to install uv so new contributors don’t get stuck.

 7. **New Setup Process**:
 
    Follow the automated setup instructions in [SETUP.md](../SETUP.md):
 
+   Prerequisite (backend deps):
+   - Install uv (recommended): `pipx install uv` (or `pip install uv`)
+
    **Linux/macOS:**

154-156: Recommend a current LTS for Node

To reduce version drift and build issues, specify Node “20 LTS or 22 LTS” instead of a loose “18+”. Adjust if your toolchain mandates a specific version.

streamlit_app.py (4)

253-260: Use explicit PowerShell path with pwsh fallback

Resolve the PowerShell binary via shutil.which and add -NoProfile to reduce flakiness (addresses S607).

+import shutil
@@
-                if setup_script.exists():
-                    result = subprocess.run([
-                        "powershell", "-ExecutionPolicy", "Bypass",
-                        "-File", str(setup_script)
-                    ])
+                if setup_script.exists():
+                    ps_exe = shutil.which("pwsh") or shutil.which("powershell")
+                    if not ps_exe:
+                        print("❌ PowerShell not found (pwsh/powershell).")
+                        return False
+                    result = subprocess.run([ps_exe, "-NoProfile", "-ExecutionPolicy", "Bypass", "-File", str(setup_script)])

278-285: Resolve npm binary explicitly; surface a clearer error

Find npm via PATH so the error is immediate and actionable. Optional: add fallback to pnpm/yarn if you support them.

-        process = subprocess.Popen(
-            ["npm", "run", "dev"],
+        npm_bin = shutil.which("npm")
+        if not npm_bin:
+            print("❌ npm not found in PATH. Please install Node.js (includes npm) and try again.")
+            return False
+        process = subprocess.Popen(
+            [npm_bin, "run", "dev"],
             cwd=root_dir,
             stdout=subprocess.PIPE,
             stderr=subprocess.STDOUT,
             universal_newlines=True,
             bufsize=1
         )

223-230: Document uv installation for manual backend setup

You instruct “uv sync” but don’t tell users to install uv. Add a prerequisite note.

-    print("5. Install backend: cd apps/backend && uv sync")
+    print("5. Install backend: cd apps/backend && uv sync  (install uv first: pipx install uv)")

287-295: Optional: terminate the whole dev process tree

npm run dev can spawn children; consider terminating the process group for a clean shutdown.

-        except KeyboardInterrupt:
-            print("\n🛑 Shutting down Resume Matcher...")
-            process.terminate()
+        except KeyboardInterrupt:
+            print("\n🛑 Shutting down Resume Matcher...")
+            try:
+                if sys.platform.startswith('win'):
+                    process.terminate()
+                else:
+                    os.killpg(os.getpgid(process.pid), 15)
+            except Exception:
+                process.terminate()
             process.wait()
             print("👋 Resume Matcher stopped.")
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 89b509c and d11e614.

📒 Files selected for processing (5)
  • .github/CONTRIBUTING.md (1 hunks)
  • FIX_ISSUE_409.md (0 hunks)
  • README.md (1 hunks)
  • run_first.py (1 hunks)
  • streamlit_app.py (1 hunks)
💤 Files with no reviewable changes (1)
  • FIX_ISSUE_409.md
🧰 Additional context used
🧬 Code graph analysis (2)
run_first.py (2)
streamlit_app.py (1)
  • main (306-335)
setup.sh (1)
  • success (57-57)
streamlit_app.py (1)
run_first.py (1)
  • main (158-178)
🪛 Ruff (0.14.0)
run_first.py

85-85: f-string without any placeholders

Remove extraneous f prefix

(F541)


124-124: subprocess call: check for execution of untrusted input

(S603)


124-127: Starting a process with a partial executable path

(S607)


142-142: subprocess call: check for execution of untrusted input

(S603)


152-152: Do not catch blind exception: Exception

(BLE001)


176-176: Do not catch blind exception: Exception

(BLE001)

streamlit_app.py

195-195: f-string without any placeholders

Remove extraneous f prefix

(F541)


256-256: subprocess call: check for execution of untrusted input

(S603)


256-259: Starting a process with a partial executable path

(S607)


267-267: subprocess call: check for execution of untrusted input

(S603)


279-279: Starting a process with a partial executable path

(S607)


297-297: Consider moving this statement to an else block

(TRY300)


302-302: Do not catch blind exception: Exception

(BLE001)

🔇 Additional comments (1)
.github/CONTRIBUTING.md (1)

154-161: No action needed — CONTRIBUTING.md documentation is accurate and complete

The referenced setup files all exist (SETUP.md, setup.sh, setup.ps1, package.json), and the localhost:3000 and localhost:8000 endpoints are correctly configured. While webapp/README.md doesn't exist, the codebase uses an apps/frontend/ structure with its own README.md that matches the documented setup. Contributors cloning fresh will successfully follow these instructions.

Likely an incorrect or invalid review comment.

@sonianuj287 sonianuj287 force-pushed the run-first-and-streamlit-app-stubs branch from 69cd042 to a52cb8a Compare October 18, 2025 19:03
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: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/CONTRIBUTING.md (1)

123-157: Clarify setup path and align Python version requirements

The review comment is correct. The documentation has conflicting information that needs alignment:

  1. Python version conflict: The old pyenv approach (lines 100-115) references Python 3.11.0, but the backend pyproject.toml requires Python ≥ 3.12, and SETUP.md specifically recommends Python 3.12 for Windows.

  2. Package manager ambiguity: The pip install -r requirements.txt step (line 126) is positioned between the legacy pyenv section and the new "Architecture Update" warning, making it unclear whether it applies to the old or new workflow. The authoritative SETUP.md guide uses setup.sh/setup.ps1 with uv instead, so this pip step appears to be legacy-only.

  3. Setup workflow clarity: The new workflow (SETUP.md + setup.sh/setup.ps1) handles all dependency installation automatically; users should not need the manual pip install step when following the new path.

Suggested fixes:

  • Clearly mark the old pyenv approach (lines 100-115) as deprecated/legacy, or remove it entirely
  • Update Python version to 3.12 in any retained legacy section to match backend requirements
  • Move the pip install -r requirements.txt step into the old/legacy section only, or remove it if legacy support is ending
  • Ensure new users are directed to SETUP.md as the single recommended setup path
🧹 Nitpick comments (9)
.github/CONTRIBUTING.md (2)

86-121: Fix minor typo and heading consistency for pyenv block

  • Typo: “enviroment” → “environment”.
  • Consider promoting “OPTIONAL (For pyenv users)” to a proper subheading for readability.
-   - pyenv with virtual enviroment
+   - pyenv with virtual environment

158-163: Endpoints list matches scripts; add Node.js/Python versions for quick ref

Add “Requires Node.js 18+ and Python 3.12+” here to reduce context switching.

run_first.py (4)

73-91: Resolve path robustly and keep guidance; minor tweak

Use resolve() to avoid symlink quirks; keep chmod tip.

-    root_dir = Path(__file__).parent
+    root_dir = Path(__file__).resolve().parent

113-156: Narrow exceptions and preflight executables; add timeouts

  • Pre-check powershell/npm with shutil.which to avoid FileNotFoundError.
  • Narrow broad Exception to (OSError, subprocess.SubprocessError) and print captured stdout on failures.
  • Add a reasonable timeout to setup scripts to avoid hanging.
+import shutil
@@
-    try:
+    try:
         if sys.platform.startswith('win'):
             setup_script = root_dir / "setup.ps1"
             if setup_script.exists():
-                print("  Running Windows setup script...")
-                result = subprocess.run([
-                    "powershell", "-ExecutionPolicy", "Bypass", 
-                    "-File", str(setup_script)
-                ], capture_output=True, text=True)
+                print("  Running Windows setup script...")
+                pwsh = shutil.which("powershell") or shutil.which("pwsh")
+                if not pwsh:
+                    print("  ❌ PowerShell not found in PATH")
+                    return False
+                result = subprocess.run(
+                    [pwsh, "-ExecutionPolicy", "Bypass", "-File", str(setup_script)],
+                    capture_output=True, text=True, timeout=900
+                )
@@
-                else:
-                    print(f"  ❌ Setup failed: {result.stderr}")
+                else:
+                    print(f"  ❌ Setup failed: {result.stderr or result.stdout}")
                     return False
         else:
             setup_script = root_dir / "setup.sh"
             if setup_script.exists():
                 # Make executable if not already
                 setup_script.chmod(0o755)
                 print("  Running Linux/macOS setup script...")
-                result = subprocess.run([str(setup_script)], capture_output=True, text=True)
+                result = subprocess.run([str(setup_script)], capture_output=True, text=True, timeout=900)
@@
-    except Exception as e:
+    except (OSError, subprocess.SubprocessError) as e:
         print(f"  ❌ Auto-setup failed: {e}")
         return False

158-179: Non-interactive mode support

Add a flag/env to auto-answer the prompt (useful for CI or codespaces). Example: --yes or RESUME_MATCHER_AUTO_SETUP=yes.

-def main():
+def main():
@@
-        response = input("\n❓ Would you like to attempt automatic setup? (y/N): ").strip().lower()
+        auto = os.environ.get("RESUME_MATCHER_AUTO_SETUP", "").lower()
+        if not auto:
+            auto = next((arg.split("=")[1] for arg in sys.argv if arg.startswith("--auto-setup=")), "")
+        response = auto or input("\n❓ Would you like to attempt automatic setup? (y/N): ").strip().lower()

57-71: Tiny copy edit in banner box

Missing space alignment (“is no longer used in Resume Matcher.” line wraps oddly). Purely cosmetic.

streamlit_app.py (3)

169-199: check_new_setup(): good checks; consider resolving root path

Use resolve() and keep messages.

-    root_dir = Path(__file__).parent
+    root_dir = Path(__file__).resolve().parent

231-305: Harden subprocess usage; address Ruff S603/S607/BLE001/TRY300

  • Pre-check npm with shutil.which; fail fast with a helpful message.
  • Narrow broad Exception to (OSError, subprocess.SubprocessError).
  • Prefer text=True over universal_newlines; set encoding for robustness.
  • Optionally annotate subprocess calls with noqa if keeping current form.
  • Move final return True to an else block to satisfy TRY300.
+import shutil
@@
-    try:
+    try:
         # First check if package.json exists
         package_json = root_dir / "package.json"
@@
-            if sys.platform.startswith('win'):
+            if sys.platform.startswith('win'):
                 setup_script = root_dir / "setup.ps1"
                 if setup_script.exists():
-                    result = subprocess.run([
-                        "powershell", "-ExecutionPolicy", "Bypass",
-                        "-File", str(setup_script)
-                    ])
+                    pwsh = shutil.which("powershell") or shutil.which("pwsh")
+                    if not pwsh:
+                        print("❌ PowerShell not found. Install PowerShell 7+ or use setup.sh on WSL.")
+                        return False
+                    result = subprocess.run(
+                        [pwsh, "-ExecutionPolicy", "Bypass", "-File", str(setup_script)],
+                        check=False, timeout=900  # noqa: S603,S607
+                    )
@@
-                if setup_script.exists():
+                if setup_script.exists():
                     setup_script.chmod(0o755)
-                    result = subprocess.run([str(setup_script)])
+                    result = subprocess.run([str(setup_script)], check=False, timeout=900)  # noqa: S603
@@
-        process = subprocess.Popen(
-            ["npm", "run", "dev"],
+        npm_bin = shutil.which("npm")
+        if not npm_bin:
+            print("❌ npm not found. Please install Node.js (v18+) and ensure npm is on PATH.")
+            return False
+        process = subprocess.Popen(
+            [npm_bin, "run", "dev"],  # noqa: S607
             cwd=root_dir,
-            stdout=subprocess.PIPE,
-            stderr=subprocess.STDOUT,
-            universal_newlines=True,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.STDOUT,
+            text=True, encoding="utf-8", errors="replace",
             bufsize=1
         )
@@
-        try:
+        try:
             for line in iter(process.stdout.readline, ''):
                 print(line, end='')
         except KeyboardInterrupt:
             print("\n🛑 Shutting down Resume Matcher...")
             process.terminate()
             process.wait()
             print("👋 Resume Matcher stopped.")
-            
-        return True
+        else:
+            return True
@@
-    except Exception as e:
+    except (OSError, subprocess.SubprocessError) as e:
         print(f"❌ Failed to start application: {e}")
         return False

200-230: Manual setup: add fallback if uv isn’t installed

Add one line hint: “If uv is not installed, install it or use pip in apps/backend (see README).”

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d11e614 and a01c0ae.

📒 Files selected for processing (4)
  • .github/CONTRIBUTING.md (1 hunks)
  • README.md (1 hunks)
  • run_first.py (1 hunks)
  • streamlit_app.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • README.md
🧰 Additional context used
🧬 Code graph analysis (2)
run_first.py (1)
streamlit_app.py (1)
  • main (306-335)
streamlit_app.py (1)
run_first.py (1)
  • main (158-178)
🪛 Ruff (0.14.0)
run_first.py

124-124: subprocess call: check for execution of untrusted input

(S603)


124-127: Starting a process with a partial executable path

(S607)


142-142: subprocess call: check for execution of untrusted input

(S603)


152-152: Do not catch blind exception: Exception

(BLE001)


176-176: Do not catch blind exception: Exception

(BLE001)

streamlit_app.py

256-256: subprocess call: check for execution of untrusted input

(S603)


256-259: Starting a process with a partial executable path

(S607)


267-267: subprocess call: check for execution of untrusted input

(S603)


279-279: Starting a process with a partial executable path

(S607)


297-297: Consider moving this statement to an else block

(TRY300)


302-302: Do not catch blind exception: Exception

(BLE001)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: cubic · AI code reviewer
🔇 Additional comments (5)
.github/CONTRIBUTING.md (2)

129-166: Docs: Architecture update is clear; nice!

The migration note + new run steps read well and align with the new stack. Good guidance and links.


168-177: No docker-compose file exists in repository; update docs to clarify Compose command variants

The repository contains no docker-compose file to verify the port configuration against. However, the documentation should clarify that Compose v2+ uses docker compose (without hyphen) instead of the legacy docker-compose command. Consider updating lines 168–177 to document both command styles for user clarity.

Likely an incorrect or invalid review comment.

run_first.py (1)

1-50: LGTM: Clear migration stub and onboarding banner

The banner and guidance are concise and helpful. Good job setting expectations for legacy users.

streamlit_app.py (2)

1-130: LGTM: Thorough migration messaging

Great depth in the docstring—clearly sets context for users arriving from Streamlit.


311-336: User prompt flow is friendly; good UX

Clear prompt, graceful KeyboardInterrupt handling, and helpful links.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 4 files

Prompt for AI agents (all 2 issues)

Understand the root cause of the following 2 issues and fix them.


<file name="streamlit_app.py">

<violation number="1" location="streamlit_app.py:317">
The compatibility stub is invoked through `streamlit run streamlit_app.py`, but Streamlit runs scripts without an interactive stdin, so calling input() here raises EOFError and the stub crashes before showing migration instructions.</violation>
</file>

<file name="run_first.py">

<violation number="1" location="run_first.py:142">
Capturing the setup script output makes the auto-setup appear frozen and hides any interactive prompts; let the scripts stream directly to the console instead.</violation>
</file>

React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.

@sonianuj287 sonianuj287 force-pushed the run-first-and-streamlit-app-stubs branch 2 times, most recently from 674097b to 958e973 Compare October 19, 2025 10:06
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)
streamlit_app.py (1)

253-270: Consider extracting shared setup logic.

The setup script execution logic (lines 253-270) is similar to run_first.py lines 120-151. Since these are temporary compatibility stubs intended for eventual deprecation, the duplication is acceptable. However, if these stubs will be maintained long-term, consider extracting the common logic to a shared utility module.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a01c0ae and 958e973.

📒 Files selected for processing (5)
  • .github/CONTRIBUTING.md (0 hunks)
  • FIX_ISSUE_409.md (0 hunks)
  • README.md (1 hunks)
  • run_first.py (1 hunks)
  • streamlit_app.py (1 hunks)
💤 Files with no reviewable changes (2)
  • FIX_ISSUE_409.md
  • .github/CONTRIBUTING.md
🧰 Additional context used
🧬 Code graph analysis (2)
streamlit_app.py (1)
run_first.py (1)
  • main (159-179)
run_first.py (2)
streamlit_app.py (1)
  • main (306-337)
setup.sh (1)
  • success (57-57)
🪛 Ruff (0.14.0)
streamlit_app.py

256-256: subprocess call: check for execution of untrusted input

(S603)


256-259: Starting a process with a partial executable path

(S607)


267-267: subprocess call: check for execution of untrusted input

(S603)


279-279: Starting a process with a partial executable path

(S607)


297-297: Consider moving this statement to an else block

(TRY300)


302-302: Do not catch blind exception: Exception

(BLE001)

run_first.py

125-125: subprocess call: check for execution of untrusted input

(S603)


125-128: Starting a process with a partial executable path

(S607)


143-143: subprocess call: check for execution of untrusted input

(S603)


153-153: Do not catch blind exception: Exception

(BLE001)


177-177: Do not catch blind exception: Exception

(BLE001)

🔇 Additional comments (4)
run_first.py (1)

159-182: LGTM!

The main function correctly orchestrates the migration flow with appropriate exception handling for a user-facing compatibility stub.

streamlit_app.py (2)

1-130: Excellent migration documentation!

The comprehensive docstring provides clear guidance on the architecture changes, migration path, and setup instructions for legacy users.


231-304: Good real-time output streaming implementation!

The function correctly streams subprocess output directly to the console without buffering, providing better UX during setup and launch. The exception handling appropriately guides users to manual setup on failures.

README.md (1)

76-77: Clear migration guidance for legacy users!

The note effectively communicates the purpose of the compatibility stubs and guides legacy users through the migration process.

@sonianuj287 sonianuj287 force-pushed the run-first-and-streamlit-app-stubs branch from 958e973 to c9fc4ad Compare October 19, 2025 10:12
@sonianuj287
Copy link
Author

sonianuj287 commented Oct 21, 2025

Hi @srbhr @atarora @iHirenDev , not sure whom to tag. Could you please review this PR. Thanks :)

@survivant
Copy link
Contributor

@sonianuj287 why are you tagging me ? I'm not in this project, I just opened a PR like you. please remove me from this PR.

@sonianuj287
Copy link
Author

Hi @srbhr @rrvrs @elliotclee @Sayvai @SubramanyamChalla24 , could you please review this PR and suggest me changes. Thanks :)

@elliotclee
Copy link
Contributor

Hi @srbhr @rrvrs @elliotclee @Sayvai @SubramanyamChalla24 , could you please review this PR and suggest me changes. Thanks :)

SRBHR is the man you want to connect with. If he doesn't respond here, try catching him on the RM discord. He has been sick the past ten days so I don't know when he'll be back in action though. I am not really up to date with RM, especially with the rewrite that SRBHR is doing, so I can't really be of much help with reviewing the PR right now.

@srbhr
Copy link
Owner

srbhr commented Oct 27, 2025

Thanks @elliotclee for being there while I was away.

@srbhr
Copy link
Owner

srbhr commented Oct 27, 2025

@sonianuj287 where are the instructions for running the streamlit application? We moved away from the steamlit app a few months ago towards a FastAPI/Next based code.

@srbhr
Copy link
Owner

srbhr commented Oct 27, 2025

Would merging this be helpful?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants