Skip to content

Conversation

@Kannav02
Copy link
Collaborator

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.ts which would now handle sending the request to gemini for suggested questions

I have also included some changes in the frontend for the same

For reference this is what the feature looks like now

Screenshot 2025-03-20 at 2 11 59 PM

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!

@Kannav02 Kannav02 requested a review from luarss March 20, 2025 18:18
@Kannav02 Kannav02 mentioned this pull request Mar 20, 2025
2 tasks
@luarss
Copy link
Collaborator

luarss commented Mar 30, 2025

Hi @Kannav02 just to check - is this PR independent of #119? Which one should I review first?

@luarss luarss requested a review from Copilot March 30, 2025 13:09
Copy link
Contributor

Copilot AI left a 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

@Kannav02
Copy link
Collaborator Author

Kannav02 commented Apr 1, 2025

Hi @Kannav02 just to check - is this PR independent of #119? Which one should I review first?

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

@luarss luarss requested a review from error9098x April 1, 2025 02:39
@luarss
Copy link
Collaborator

luarss commented Apr 1, 2025

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

@Kannav02
Copy link
Collaborator Author

Kannav02 commented Apr 1, 2025

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!

@palaniappan-r
Copy link
Collaborator

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

We could add a separate route for this and for other helper utilities in the future. /helpers maybe?

@error9098x
Copy link
Collaborator

Thanks for starting the discussion. I agree

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

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.

image

@Kannav02
Copy link
Collaborator Author

Kannav02 commented Apr 1, 2025

Hey @error9098x , I totally agree with you about apiproxy.py, its kinda redundant, Its barely a proxy at this point, also about the gemini, its better to have them all at the backend rather than having a separate backend for next-js , makes it complicated, so I totally agree with you on this one as well

@luarss
Copy link
Collaborator

luarss commented Apr 2, 2025

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 helpers.py in backend/src/api/routers. Try to keep to the same style where possible (e.g. type models, variable naming, etc)

@Kannav02
Copy link
Collaborator Author

Kannav02 commented Apr 3, 2025

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!

@Kannav02
Copy link
Collaborator Author

Kannav02 commented Apr 8, 2025

Hey everyone!

So I made the changes at the backend, but when I am trying to build it, I am getting the following error

orassistant-backend   | WARNING:langchain_core.language_models.llms:Retrying vertexai.language_models._language_models._TextEmbeddingModel.get_embeddings in 4.0 seconds as it raised InternalServerError: 500 Internal error encountered..
orassistant-backend   | WARNING:langchain_core.language_models.llms:Retrying vertexai.language_models._language_models._TextEmbeddingModel.get_embeddings in 4.0 seconds as it raised InternalServerError: 500 Internal error encountered..
orassistant-backend   | INFO:sentence_transformers.cross_encoder.CrossEncoder:Use pytorch device: cpu
orassistant-backend   | INFO:root:Using Google VertexAI embeddings...
orassistant-backend   | INFO:root:Process HTML docs...
Loading HTML files: 100%|██████████| 1038/1038 [02:41<00:00,  6.43it/s]
orassistant-backend   | INFO:root:Adding ['./data/html/klayout_docs'] to FAISS database...
orassistant-backend   | 
orassistant-backend   | WARNING:langchain_core.language_models.llms:Retrying vertexai.language_models._language_models._TextEmbeddingModel.get_embeddings in 4.0 seconds as it raised InternalServerError: 500 Internal error encountered..
orassistant-backend   | WARNING:langchain_core.language_models.llms:Retrying vertexai.language_models._language_models._TextEmbeddingModel.get_embeddings in 4.0 seconds as it raised InternalServerError: 500 Internal error encountered..
orassistant-backend   | WARNING:langchain_core.language_models.llms:Retrying vertexai.language_models._language_models._TextEmbeddingModel.get_embeddings in 4.0 seconds as it raised InternalServerError: 500 Internal error encountered..

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!

@luarss
Copy link
Collaborator

luarss commented Apr 8, 2025

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

@palaniappan-r
Copy link
Collaborator

Hey everyone!

So I made the changes at the backend, but when I am trying to build it, I am getting the following error

orassistant-backend   | WARNING:langchain_core.language_models.llms:Retrying vertexai.language_models._language_models._TextEmbeddingModel.get_embeddings in 4.0 seconds as it raised InternalServerError: 500 Internal error encountered..
orassistant-backend   | WARNING:langchain_core.language_models.llms:Retrying vertexai.language_models._language_models._TextEmbeddingModel.get_embeddings in 4.0 seconds as it raised InternalServerError: 500 Internal error encountered..
orassistant-backend   | INFO:sentence_transformers.cross_encoder.CrossEncoder:Use pytorch device: cpu
orassistant-backend   | INFO:root:Using Google VertexAI embeddings...
orassistant-backend   | INFO:root:Process HTML docs...
Loading HTML files: 100%|██████████| 1038/1038 [02:41<00:00,  6.43it/s]
orassistant-backend   | INFO:root:Adding ['./data/html/klayout_docs'] to FAISS database...
orassistant-backend   | 
orassistant-backend   | WARNING:langchain_core.language_models.llms:Retrying vertexai.language_models._language_models._TextEmbeddingModel.get_embeddings in 4.0 seconds as it raised InternalServerError: 500 Internal error encountered..
orassistant-backend   | WARNING:langchain_core.language_models.llms:Retrying vertexai.language_models._language_models._TextEmbeddingModel.get_embeddings in 4.0 seconds as it raised InternalServerError: 500 Internal error encountered..
orassistant-backend   | WARNING:langchain_core.language_models.llms:Retrying vertexai.language_models._language_models._TextEmbeddingModel.get_embeddings in 4.0 seconds as it raised InternalServerError: 500 Internal error encountered..

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, please consider switching to locally run HF embeddings or the free Gemini tier as @luarss has suggested.

@Kannav02
Copy link
Collaborator Author

Kannav02 commented Apr 9, 2025

Perfect, i'll move ahead with the local embedding or try out the free version then, thanks!

@error9098x
Copy link
Collaborator

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: gemini-embedding-exp-03-07 (link), and the current stable one istext-embedding-004.

@Kannav02
Copy link
Collaborator Author

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

@Kannav02
Copy link
Collaborator Author

So I have got the frontend working with the backend as well, I am attaching the screenshots for the same, if you think any other changes are needed, do let me know and I will fix them, thanks!

On a side note, I am still using apiproxy.py as a proxy for the development and production environment for the same

Screenshot 2025-04-11 at 5 57 49 PM Screenshot 2025-04-11 at 5 58 05 PM

@Kannav02
Copy link
Collaborator Author

Kannav02 commented May 4, 2025

Just following up on this, Any updates regarding the same @luarss ?

Thank you!

@Kannav02 Kannav02 closed this May 4, 2025
@Kannav02 Kannav02 reopened this May 4, 2025
error9098x
error9098x previously approved these changes Jun 1, 2025
@luarss
Copy link
Collaborator

luarss commented Jun 5, 2025

@Kannav02 Could you please rebase, that will fix your linting errors.

@Kannav02
Copy link
Collaborator Author

Kannav02 commented Jun 5, 2025

@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

Kannav02 and others added 23 commits July 6, 2025 10:00
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]>
@luarss luarss force-pushed the suggested-question-fix branch from 52c1b78 to e23349f Compare July 6, 2025 10:01
luarss added 2 commits July 6, 2025 10:21
- 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]>
@luarss luarss linked an issue Jul 6, 2025 that may be closed by this pull request
2 tasks
@luarss luarss merged commit 68f7b1c into master Jul 6, 2025
2 checks passed
@luarss luarss deleted the suggested-question-fix branch July 6, 2025 12:24
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.

ContextSource migration

5 participants