-
Notifications
You must be signed in to change notification settings - Fork 13
Backend Proxy For Suggested Question #123
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
Conversation
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.
Pull Request Overview
This PR implements a backend proxy for the suggested questions feature by introducing a new Next.js API route and making several related updates in the frontend.
- Implements an API route for suggested questions with an external Gemini API call.
- Updates several frontend components (SuggestedQuestions, ChatHistory, etc.) to integrate the new backend functionality.
- Adds environment configuration, ESLint configuration, and a Flask-based API proxy as part of the overall update.
Reviewed Changes
Copilot reviewed 34 out of 36 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| next.config.mjs | Adds GEMINI_API_KEY environment variable. |
| hooks/useWindowSize.ts | Introduces a new hook to track window dimensions. |
| eslint.config.mjs | Sets up ESLint configuration with Next.js and Prettier integration. |
| components/ThemeProvider.tsx | Adds a theme provider wrapper wrapping next-themes. |
| components/SuggestedQuestions.tsx | Integrates a fetch call to the new suggested questions API route. |
| components/SourceList.tsx, MessageList.tsx, ChatHistory.tsx | Implements UI changes to support chat history and suggestions. |
| app/page.tsx | Adjusts the main page to use the new API endpoints and components. |
| app/api/suggestedQuestions/route.ts | Implements the backend API route to fetch suggested questions. |
| app/api/chat.ts | Creates a chat API route that proxies requests to an external endpoint. |
| apiproxy.py | Adds a Flask-based proxy server for handling API calls. |
| README.md | Updates documentation with setup and configuration instructions. |
| .pre-commit-config.yaml | Modifies pre-commit hooks configuration. |
Files not reviewed (2)
- frontend/nextjs-frontend/.env.example: Language not supported
- frontend/nextjs-frontend/app/globals.css: Language not supported
Comments suppressed due to low confidence (3)
frontend/nextjs-frontend/components/SuggestedQuestions.tsx:35
- Accessing nested properties without prior validation may result in runtime errors if the API response does not match the expected structure. Consider adding checks to ensure that the candidates array and nested properties exist before parsing.
const suggestedQuestionsText = data.candidates[0].content.parts[0].text;
frontend/nextjs-frontend/app/page.tsx:197
- [nitpick] Invoking handleSubmit with a dummy event object may lead to unexpected behavior. Consider extracting the submission logic into a separate function for programmatic calls.
handleSubmit({ preventDefault: () => {} } as React.FormEvent<HTMLFormElement>);
frontend/nextjs-frontend/.pre-commit-config.yaml:10
- [nitpick] The removal of the 'check-added-large-files' hook could allow inadvertently large files to be committed. Reconsider if this removal aligns with the project's code quality and repository size objectives.
- id: check-added-large-files
So this PR is kinda built upon the #119 PR itself, it is kind of a small feature to be integrated in it, so I think this should be reviewed first and merged into the #119 PR for the same rather than being merged into the main for now at least |
|
@error9098x Could you please review the frontend changes? @Kannav02 I think one issue is that we might have to shift the suggested questions feature to backend (ie make it an actual route in our fastapi backend). @palaniappan-r where is a good place to insert this? |
|
Oh sure, that would work as well, right now what is happening is we're using Next.js API as the backend here, but I could redirect it to work as a proxy and add the route to the backend. do let me know when you want me to work on this Thanks! |
We could add a separate route for this and for other helper utilities in the future. |
|
Thanks for starting the discussion. I agree
Looking at the diagram, we've got apiproxy.py and the Next.js router for Gemini that Kannav improved with router.ts - it's definitely cleaner now. But here's the thing: we shouldn't really need apiproxy.py at all. Ideally, this should all be sorted out in the backend by allowing CORS to let Next.js call it directly. We don't want to mix Python into our frontend stuff. Also, those Gemini API calls? They should be hooked up to the backend instead. If we keep them on the frontend, we're still gonna have issues with API leaks. So, what do you guys think about moving all this to the backend? It'd clean things up on the frontend and make our setup more secure overall. |
|
Hey @error9098x , I totally agree with you about |
|
My thoughts are: apiproxy.py can still be used but only in an experimental way (when you want to verify the frontend but don't want to spin up the backend) - it should not be used as a permanent fixture. @Kannav02 could you work on this new route in this PR please? Following up on what Pal said - maybe create |
|
Sure, I will keep on pushing atomic commits, please do let me know if you would want any changes in that, I will push something by EOD today Thank you everyone for the help! |
|
Hey everyone! So I made the changes at the backend, but when I am trying to build it, I am getting the following error I believe this is because I only had free credits for my service account for about 3 months and I guess those expired, I do want to test these changes out, please do let me know how do I proceed with the service account for the same For now the changes are only made at the backend, once I test it out, I will change everything at the frontend as well Thank You! |
|
Hi @Kannav02, have you tried regenerating the service account json? On a side note - https://ai.google.dev/gemini-api/docs/pricing also shows a pretty generous free tier limit. Maybe we can consider switching models to Gemini if Vertex AI issues continue |
Hi, please consider switching to locally run HF embeddings or the free Gemini tier as @luarss has suggested. |
|
Perfect, i'll move ahead with the local embedding or try out the free version then, thanks! |
|
Hi, you can try both approaches — using HF embeddings should work well locally. I'm not sure if the free Google embeddings are fully functional, since they're subject to rate limits, so if you can test that, it would be great. Also, they’ve recently released a new experimental embedding model: |
|
quick update, I tried out HF embeddings, it isn't working for some reason and taking a long time on my machine, so I think I will try out the new experiment embedding model for gemini , should be good for our use case I will keep on updating as soon as I find something new |
|
Just following up on this, Any updates regarding the same @luarss ? Thank you! |
|
@Kannav02 Could you please rebase, that will fix your linting errors. |
sure, working on pushing a lot of changes, will rebase one those are done, thanks |
Signed-off-by: Kannav02 <[email protected]>
Signed-off-by: Kannav02 <[email protected]>
Signed-off-by: Kannav02 <[email protected]>
Signed-off-by: Kannav02 <[email protected]>
Signed-off-by: Kannav02 <[email protected]>
Signed-off-by: Kannav02 <[email protected]>
… nextjs-frontend Signed-off-by: Kannav02 <[email protected]>
Signed-off-by: Kannav02 <[email protected]>
Signed-off-by: Kannav02 <[email protected]>
Signed-off-by: Kannav02 <[email protected]>
Signed-off-by: Kannav02 <[email protected]>
Signed-off-by: Kannav02 <[email protected]>
Signed-off-by: Kannav02 <[email protected]>
Signed-off-by: Jack Luar <[email protected]>
Signed-off-by: Jack Luar <[email protected]>
Signed-off-by: Jack Luar <[email protected]>
Signed-off-by: Jack Luar <[email protected]>
Signed-off-by: Jack Luar <[email protected]>
Signed-off-by: Kannav02 <[email protected]>
Signed-off-by: Kannav02 <[email protected]>
Signed-off-by: Kannav02 <[email protected]>
Signed-off-by: Kannav02 <[email protected]>
Signed-off-by: Kannav02 <[email protected]>
52c1b78 to
e23349f
Compare
- cleanup readme - remove unused env vars Signed-off-by: Jack Luar <[email protected]>
- remove frontend post-processing from fetchSuggestedQuestions, shift to backend - update suggested_questions_prompt_template to ask more interactive questions - use openai client for flexible backend later Signed-off-by: Jack Luar <[email protected]>



This PR addresses the issue with the suggested questions feature by implementing a proper API route in our Next.js frontend to handle communication with the backend service.
I've created a API route at the path
frontend/nextjs-frontend/app/api/suggestedQuestions/route.tswhich would now handle sending the request to gemini for suggested questionsI have also included some changes in the frontend for the same
For reference this is what the feature looks like now
Please let me know if you want any other changes to be made for the same!
Should merging this PR be part of a batch-merge or should we merge this individually?
Thank you!