open router config, crop issues & 1st Page Read & Fetch - #1
Conversation
|
Important Review skippedToo many files! This PR contains 109 files, which is 9 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (406)
📒 Files selected for processing (114)
You can disable this status message by setting the 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 |
priyanka-TL
left a comment
There was a problem hiding this comment.
Reviewed on 10.30 AM 4th Aug
| }, | ||
| "answers": [ | ||
| { | ||
| "question_id": "", |
There was a problem hiding this comment.
@bharathrSL How from csv how i will get to know the question ? Can we keep the question name as well in header with number ?
| min_image_quality: 0.35 | ||
| review_confidence_threshold: 0.72 | ||
| gemini_model: gemini-3.5-flash | ||
| openrouter_model: google/gemini-3.5-flash |
There was a problem hiding this comment.
@bharathrSL, can we move these configurations to environment variables? This will allow us to update and deploy the changes without requiring any code modifications.
| @@ -1,56 +0,0 @@ | |||
| """Google GenAI provider for checkbox interpretation and Hindi handwriting.""" | |||
There was a problem hiding this comment.
@bharathrSL, can we keep the direct Gemini integration? We can make it an environment-driven configuration so we can switch between the direct LLM integration and OpenRouter as needed, without any code changes.
| review_required=True, | ||
| raw_observations="OpenRouter was not called: OPENROUTER_API_KEY is unavailable", | ||
| ) | ||
| for q in questions |
There was a problem hiding this comment.
@bharathrSL Could we please follow standard naming conventions for variables? Let's avoid using single-letter variable names and use meaningful, descriptive names to improve code readability and maintainability.
|
|
||
| @dataclass | ||
| class StudentInfo: | ||
| student_name: str = "" |
There was a problem hiding this comment.
@bharathrSL Remove all the test input files and output files. Keep 1 or 2 for reference
| @@ -1,2 +1,2 @@ | |||
| # Copy this file to .env and paste your real key after the equals sign. | |||
| GEMINI_API_KEY= | |||
| OPENROUTER_API_KEY= | |||
There was a problem hiding this comment.
@bharathrSL Can we add one more env variable, LLM_PROVIDER which can be open router or gemini
based on this use handle it in code
| @@ -1,2 +1,2 @@ | |||
| # Copy this file to .env and paste your real key after the equals sign. | |||
| GEMINI_API_KEY= | |||
| OPENROUTER_API_KEY= | |||
There was a problem hiding this comment.
@bharathrSL Before starting the application, can we validate that all required configuration keys are present?
For example, if LLM_PROVIDER=openrouter, then OPENROUTER_API_KEY should be mandatory. Similarly, we can validate provider-specific required keys during application startup and fail fast with a clear error message if any are missing.
Refer this https://github.com/ELEVATE-Project/evidence-analysis-service/blob/release-2.2.0/env_variables.py
Also add the PORT should be configurable via env
| @@ -0,0 +1,77 @@ | |||
| { | |||
| "provider": "openrouter", | |||
| "model": "google/gemini-3.5-flash", | |||
|
|
||
| if args.command == "process" and not openrouter_credentials_present(): | ||
| parser.error("OPENROUTER_API_KEY is missing. Set it in .env or in this terminal before processing.") | ||
| print("processing============================================") |
| openpyxl>=3.1 | ||
| PyMuPDF>=1.23 | ||
| google-genai>=1.0 | ||
| httpx>=0.28 |
There was a problem hiding this comment.
@bharathrSL Can we use LiteLLM as the common abstraction for both OpenRouter and Gemini? It provides a consistent interface across LLM providers, simplifies provider-specific integrations, and makes it easier to add or switch providers in the future.
No description provided.