Project setup - #2
Open
ghanshyam2005singh wants to merge 6 commits into
Open
Conversation
|
Warning Review limit reached
Next review available in: 48 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughScholarAI adds a Cloudflare Python Worker with AI-backed research endpoints and a static frontend. The interface supports PDF extraction, question answering, summarization, literature discovery, and literature reviews. Wrangler configuration, documentation, and ignore rules are included. ChangesScholarAI application
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant Worker
participant ResearchEndpoint
participant CloudflareAI
Browser->>Worker: Submit research request
Worker->>ResearchEndpoint: Dispatch API route
ResearchEndpoint->>CloudflareAI: Send prompts
CloudflareAI-->>ResearchEndpoint: Return generated text
ResearchEndpoint-->>Browser: Return JSON result
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/pipeline.py`:
- Around line 34-42: The global _embeddings is constructed at import time which
can fail and block app startup; change this to lazy initialization by replacing
the module-level _embeddings with a getter (e.g., implement _get_embeddings())
that constructs and caches a HuggingFaceEmbeddings instance on first call (using
EMBEDDING_MODEL), then update _get_vectorstore() to call _get_embeddings() for
the embedding_function instead of referencing _embeddings directly; ensure the
pattern mirrors existing _get_llm()—create, cache, and return the instance so
imports no longer run heavy initialization.
- Around line 78-79: The code currently calls vs.add_documents(chunks) without
capturing returned IDs and later uses the private _collection.delete(where=...)
API; update the ingest path that calls _get_vectorstore() and vs.add_documents
to capture and persist the returned document IDs (e.g., map user_id → list of
IDs) and then replace any direct uses of _collection.delete(where=...) with the
public vs.delete(ids=stored_ids) call to remove documents; if metadata-based
deletion is required and IDs cannot be stored, wrap Chroma in a small adapter
you control instead of touching _collection.
In `@app/server.py`:
- Around line 37-40: Do not trust the incoming user_id from
request.form/JSON/URL; derive the tenant/user identity on the server from the
authenticated context (e.g., session, auth token, or current_user) and use that
server-side tenant_id when calling the storage functions; update the places
referencing the request-derived user_id (the local variable user_id in server.py
and the code paths that call pipeline.store_chunk, pipeline.read_chunks,
pipeline.delete_chunks) to accept and use the server-derived tenant_id instead
of request data, and add validation/authorization checks where
pipeline.store_chunk, read_chunks, and delete_chunks are invoked to ensure
operations are performed only for the authenticated tenant.
- Around line 47-51: The handlers that catch exceptions (e.g., the try/except
around index_document(str(temp_file), user_id=user_id)) must stop returning
str(e) to clients; instead call app.logger.exception(e) to record the full
server-side error and return a generic JSON error (e.g., {"ok": False, "error":
"Internal server error"}) with HTTP 500. Update all similar catch blocks (the
one around index_document and the other handlers that currently return str(e))
to log via app.logger.exception(e) and send a non-revealing error message to the
caller.
- Around line 17-21: The Flask app instantiation (app = Flask(...)) lacks a
MAX_CONTENT_LENGTH configuration, so set app.config["MAX_CONTENT_LENGTH"] from
an environment variable (e.g., UPLOAD_MAX_BYTES) with a sensible default of 25 *
1024 * 1024 (25MB) to prevent unbounded uploads; update the startup code that
creates the Flask app (the block around app = Flask(...)) to read the env var,
parse it to an integer if present, fallback to 25MB, and assign it to
app.config["MAX_CONTENT_LENGTH"] so oversized requests return 413.
- Around line 82-83: Make the debug flag in the main entry conditional on an
environment variable instead of hardcoding True: in the if __name__ ==
"__main__": block where app.run(...) is called, read a DEBUG (or
FLASK_DEBUG/APP_DEBUG) env var via os.getenv and convert it to a boolean, then
pass that boolean as the debug parameter to app.run(host="0.0.0.0",
port=int(os.getenv("PORT", "5000")), debug=...). Ensure you use the existing
os.getenv import and name the env var clearly so local developers can
enable/disable debug mode.
- Around line 59-64: Validate that request.get_json(silent=True) returns a dict
before calling .get; if not, return a 400 JSON error. Retrieve raw_question =
data.get("question") and raw_user_id = data.get("user_id") and ensure both are
instances of str before calling .strip(); if they are missing or not strings,
return a 400 with a clear error message. Add type hints to the route handler
signature and use the module logger to log malformed requests or type errors
(include the raw payload) before returning the 400 so failures are recorded.
Update references in code to request.get_json, raw_question/raw_user_id, and the
route handler function name when implementing these checks.
In `@app/static/style.css`:
- Line 23: The font-family declaration uses quoted single-word family 'Sora'
which triggers the stylelint rule; update the font-family property (the
font-family line setting 'Sora', Arial, sans-serif) to remove the quotes so it
reads Sora, Arial, sans-serif to comply with linting.
In `@app/templates/index.html`:
- Around line 30-33: The mobile menu button lacks ARIA attributes and dynamic
state updates; add aria-controls="mobile-menu" and aria-expanded="false" to the
button element (id="mobile-menu-button") and ensure the toggle logic that
shows/hides the element with id="mobile-menu" updates
button.setAttribute('aria-expanded', 'true'|'false') whenever the menu is opened
or closed (also apply the same attribute additions/updates to the other menu
instance around the second occurrence of mobile-menu/mobile-menu-button).
- Around line 71-83: The uploadForm and askForm are missing CSRF protection; add
a CSRF token field (e.g., a hidden input named csrf_token) to both forms
(uploadForm and askForm) or inject a meta token and ensure client-side
submission includes it (associated with inputs pdfFile and question), then wire
server-side validation in the corresponding handlers that process file uploads
and question submissions to reject requests with missing/invalid tokens; ensure
the template prints a server-generated token variable (e.g., csrfToken) safely
and that your server-side middleware verifies it for those endpoints.
- Around line 70-81: Add explicit <label> elements tied to the existing control
ids: add a label for="pdfFile" (e.g., "PDF file") inside the uploadForm and a
label for="question" (e.g., "Your question") inside the askForm; if you need the
labels visually hidden for design, apply your project's visually-hidden CSS
class so they remain accessible. Also ensure each form includes the app's CSRF
protection token (e.g., a hidden input or template CSRF tag) and add ARIA
attributes where helpful (aria-describedby or aria-required) to the controls
(ids: pdfFile, question, forms: uploadForm, askForm) to improve screen-reader
behavior. Ensure you do not remove existing required attributes and keep input
accept=".pdf" intact.
In `@Prockfile`:
- Line 1: The file is misnamed "Prockfile" which prevents process managers from
detecting the web process; rename the file to "Procfile" and keep the existing
entry "web: gunicorn -b 0.0.0.0:$PORT app.server:app" unchanged so the web
process is discovered and started correctly.
In `@README.md`:
- Around line 1-2: Add a blank line after the top-level heading "# ScholarAI"
and ensure the file ends with exactly one trailing newline; also apply the same
fix (add a blank line after and ensure single trailing newline) to any other
top-level headings in the file (e.g., the other H1 near the end) so the file
conforms to MD022/MD047.
In `@requirements.txt`:
- Around line 1-12: requirements.txt currently lists unpinned packages which
risks incompatibilities; update it to pin all langchain-related packages
(langchain, langchain-community, langchain-text-splitters,
langchain-google-genai, langchain-chroma, langchain-huggingface) to the same
minor version range (e.g., langchain>=0.3,<0.4) and lock chromadb to a specific,
tested version (e.g., chromadb==0.4.x) to avoid schema breakage, and add a note
or create a separate constraints/requirements-lock.txt (from pip-compile or pip
freeze) to ensure reproducible installs and to document any required migration
steps for chromadb upgrades.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6bb78706-2ccf-4973-936e-5222e71aa1cd
⛔ Files ignored due to path filters (1)
app/static/images/logo.pngis excluded by!**/*.png
📒 Files selected for processing (11)
.env.example.gitignoreProckfileREADME.mdapp/__init__.pyapp/pipeline.pyapp/server.pyapp/static/app.jsapp/static/style.cssapp/templates/index.htmlrequirements.txt
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitignore:
- Around line 1-8: The repo is missing ignore rules for Wrangler's generated
workspace and a committed temp bundle with absolute machine-specific imports;
update the .gitignore to add .wrangler/ and .wrangler/tmp/** (or at least
.wrangler/tmp/**) so Wrangler artifacts are ignored, then remove the committed
bundle from the branch history by deleting the file from the index (git rm
--cached <committed-bundle-path>) and commit the removal so the temp bundle is
no longer tracked; ensure no other absolute `/home/...` imports remain in the
tree before merging.
In `@README.md`:
- Around line 11-20: The README's project-tree fenced code block is missing a
language tag causing MD040; update the fence in README.md by adding a language
identifier (e.g., use "text" or "plaintext") immediately after the opening
triple backticks so the block becomes ```text ... ``` to satisfy markdownlint.
In `@src/worker.py`:
- Around line 163-165: The three compound conditionals that append to the parts
list (checking title, abstract, and content) should be split into separate
statements to satisfy PEP 8 E701; replace the single-line forms like "if title:
parts.append(...)" with separate lines using an if statement then a newline to
call parts.append for each of the variables title, abstract, and content (note
the content append should still slice content[:10000]); update the code around
the parts list in the prompt assembly so each conditional is on its own line.
- Around line 191-200: The discover_papers() flow currently uses
system_prompt/user_prompt to ask the LLM for specific paper metadata (including
"likely authors/year") without any retrieval validation, which risks
hallucinated citations; fix by wiring discover_papers() to a real retrieval
layer: call your paper index/search API (e.g., paper_search(query, limit,
field_ctx) or vector_store.search(query, k=limit)) inside discover_papers(), use
the returned documents' verified metadata to build the response (title, authors,
year, relevance) and only fall back to the LLM for ranking/summary;
alternatively, if a retrieval layer is not available yet, change the user_prompt
to avoid claiming specific papers/authors/years and instead request query
reformulations, keyword expansions, and research directions to prevent
hallucinated citations.
- Around line 89-94: The parse_body function currently swallows all exceptions
and returns {} which hides malformed JSON; modify parse_body to catch
json.JSONDecodeError specifically when calling await request.text() and
json.loads(text) and return error_resp("Invalid JSON body", 400) on that error,
while preserving the existing behavior for an empty body (return {}) and leaving
other unexpected exceptions to propagate or be handled elsewhere; update
references to parse_body and error_resp accordingly so handlers receive a 400
for malformed JSON.
- Around line 146-150: Replace public exposure of raw exception text in the AI
handlers: in the try/except blocks that call run_ai and return
json_resp/error_resp, stop returning f"AI error: {e}" and instead log the full
exception server-side (use a module logger you add to this worker) and return a
generic error message like "AI error" with the 500 status via error_resp; update
each handler that calls run_ai (and any similar handlers) to call
logger.exception(...) or logger.error(..., exc_info=True) before returning the
generic error response, and add a logging setup at the top of the file so
exceptions are captured for troubleshooting.
- Around line 104-117: The get_html function currently caches and returns
resp.text() from env.ASSETS.fetch without checking HTTP status and interpolates
the raw exception into client HTML; update get_html to check the Response.status
(only treat 200 as success) before reading/caching the body into _HTML_CACHE and
on non-200 return a generic error HTML (do not cache the error response), and on
exceptions log the exception to your server logger (e.g., console.error or the
module's logger) but return a generic safe error message to clients; ensure
callers like html_resp still get an appropriate status (propagate non-200 when
possible or return 500 for unexpected errors) and reference get_html,
env.ASSETS.fetch, and _HTML_CACHE when making changes.
- Around line 181-190: In handle_discover, validate and normalize inputs before
using them: replace the blind int(...) and join usage by 1) validating limit:
attempt to convert body.get("limit") to int inside a try/except and if
conversion fails or the value is negative return error_resp("invalid 'limit'
must be an integer") with a 400 status; 2) validating fields: accept None -> [],
if fields is a str wrap it as [fields], else require fields to be a list and
ensure all items are strings (otherwise return error_resp("invalid 'fields' must
be an array of strings") with 400); update references to field_ctx to use the
normalized fields list. Ensure all error_resp calls use 400 for malformed input
and keep function name handle_discover and variables query, limit, fields,
field_ctx unchanged.
In `@static/index.html`:
- Around line 256-269: Add keyboard navigation to the existing tab system by
attaching a "keydown" listener to the tab container (select ".tab-bar") that
intercepts ArrowRight and ArrowLeft, computes the next/previous index among
elements matching ".tab-btn", and moves the active state by updating classes and
aria-selected on the current and target ".tab-btn" and toggling the
corresponding ".tab-panel" (you can reuse the existing click logic by calling
targetBtn.click() or mirror its behavior: remove "active" and set
aria-selected="false" on the current, set "active" and aria-selected="true" on
the target, update panels, and call targetBtn.focus(); wrap index arithmetic so
navigation wraps around).
- Line 31: The button with id "menuBtn" (class "menu-btn") lacks an explicit
type, which can default to type="submit" inside forms; update the element with
an explicit type attribute (type="button") to prevent unintended form
submissions when toggling the mobile menu.
- Around line 440-449: The callApi function can hang indefinitely if the server
doesn't respond; wrap the fetch in an AbortController with a timeout: create an
AbortController, pass controller.signal into fetch in callApi, start a
setTimeout that calls controller.abort() after a chosen timeout (e.g., 10s), and
clear the timeout after fetch completes; ensure you catch the abort error and
throw a clear Error (e.g., "Request timed out") so callers can handle it.
- Around line 35-41: The mobile menu uses role="menu" which implies an
application-style widget; update the element with id="mobileMenu" (class
"mobile-menu") to use role="navigation" for simple anchor-based navigation, or
if you intend a full menu widget, add role="menuitem" to each child <a> and
implement proper keyboard/focus management in the associated JS; pick one
approach and make the corresponding change to the element with id="mobileMenu"
and/or its child anchors to align ARIA semantics with the actual behavior.
- Around line 100-105: Add an explicit type="button" attribute to each tab
button to prevent accidental form submissions: update the elements with class
"tab-btn" and ids "t-ask", "t-summarize", "t-discover", and "t-review" (the
buttons with role="tab" and
aria-controls="tab-ask"/"tab-summarize"/"tab-discover"/"tab-review") so each
includes type="button" while preserving all existing attributes (aria-selected,
aria-controls, id, data-tab, and class).
- Line 236: Add Subresource Integrity (SRI) to the external PDF.js script tag by
adding an integrity attribute with the SHA384 (or appropriate) base64 hash for
the referenced file
"https://cdnjs.cloudflare.com/ajax/libs/pdf.js/3.11.174/pdf.min.js" and include
crossorigin="anonymous"; update the <script
src="https://cdnjs.cloudflare.com/ajax/libs/pdf.js/3.11.174/pdf.min.js"></script>
tag to include these attributes so the browser verifies the file against the
expected SRI hash when loading.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: fabe5062-dd39-4287-89e2-8f53c215b1c1
⛔ Files ignored due to path filters (3)
.wrangler/tmp/dev-ePVlVV/ProxyServerWorker.js.mapis excluded by!**/*.mappackage-lock.jsonis excluded by!**/package-lock.jsonstatic/images/logo.pngis excluded by!**/*.png
📒 Files selected for processing (10)
.gitignore.wrangler/tmp/bundle-lfsPwi/middleware-insertion-facade.js.wrangler/tmp/bundle-lfsPwi/middleware-loader.entry.ts.wrangler/tmp/dev-ePVlVV/ProxyServerWorker.jsREADME.mdpackage.jsonsrc/worker.pystatic/index.htmlstatic/style.csswrangler.jsonc
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 29-47: Update the Wrangler workflow in the README to document the
Node.js/npm prerequisite, add local installation using npm i -D wrangler@latest,
and invoke Wrangler through npx wrangler (or committed package scripts) for
login, development, and deployment. Preserve the existing command order and
local URL, and include the referenced Cloudflare installation guide.
In `@src/worker.py`:
- Around line 96-103: Update parse_body to validate that the json.loads result
is a dict and raise InvalidJSONError for arrays, scalars, or null; preserve the
existing malformed-JSON handling. Ensure InvalidJSONError produces a 400
response, and add coverage for malformed JSON plus each non-object JSON body
type.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d476bb72-7650-4100-b24f-d8635e3d1893
📒 Files selected for processing (5)
.gitignoreREADME.mdsrc/worker.pystatic/index.htmlwrangler.toml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Screencast.from.2026-03-16.20-27-02.mp4
Summary
These changes provide the initial end-to-end ScholarAI experience, from PDF-based research input to AI-generated results.