-
-
Notifications
You must be signed in to change notification settings - Fork 41
Unify Dual LLM Calls into One Structured Prompt #116
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
base: main
Are you sure you want to change the base?
Conversation
…ove SEO content method
|
@NayanUnni95 is attempting to deploy a commit to the lamsta Team on Vercel. A member of the Team first needs to authorize it. |
Summary of ChangesHello @NayanUnni95, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the AI-driven content generation for user profiles. It consolidates the separate calls for generating profile summaries and SEO content into a single, more efficient process. The AI generator now produces a unified JSON object containing both types of information, simplifying the integration and ensuring consistency across generated content. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request refactors the AI content generation by merging the profile summary and SEO content generation into a single method, which is a good simplification. However, I've identified a few critical issues with the implementation. The parsing of the JSON response from the AI is not handled safely, which could lead to application crashes. Additionally, error handling for the new data structure in utils/user.py is incomplete, which could cause TypeError exceptions. I've also noted a minor issue with an outdated docstring. My review includes specific suggestions to address these points.
| raise Exception("No response from AI model") | ||
|
|
||
| return response.choices[0].message.content | ||
| return json.loads(response.choices[0].message.content) |
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.
The call to json.loads() is not wrapped in a try...except block. If the AI model returns a response that is not valid JSON, this will raise a json.JSONDecodeError and crash the application. It's crucial to handle this potential error gracefully, similar to how it was handled in the removed generate_seo_contents method.
try:
return json.loads(response.choices[0].message.content)
except json.JSONDecodeError as e:
raise Exception(f"AI response was not valid JSON. Content: {response.choices[0].message.content}") from e|
|
||
| # Add summaries to profile data | ||
| profile_data['profile_summary'] = profile_summary | ||
| profile_data['profile_summary'] = profile_summary['about'] |
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.
profile_summary can be None if an exception occurs in ai_generator.generate_profile_summary. Accessing profile_summary['about'] will then raise a TypeError. You should add a check to handle the case where profile_summary is None.
Additionally, the SEO data returned from generate_profile_summary is being discarded. My suggestion includes storing this data in profile_data as well.
| profile_data['profile_summary'] = profile_summary['about'] | |
| if profile_summary: | |
| profile_data['profile_summary'] = profile_summary.get('about') | |
| profile_data['seo'] = profile_summary.get('seo') | |
| else: | |
| profile_data['profile_summary'] = None | |
| profile_data['seo'] = None |
| profile_data (dict): GitHub user profile data | ||
| Returns: | ||
| str: AI-generated profile summary |
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.
The docstring's Returns section is outdated. The function now returns a dictionary containing the profile summary and SEO content, not a string. Please update the docstring to accurately reflect the new return type.
| str: AI-generated profile summary | |
| dict: AI-generated profile summary and SEO data |
Summary
This PR replaces two separate LLM prompts with one unified prompt that generates both the about section and SEO data in a single JSON response. This cuts token usage, removes duplication, and simplifies the workflow.
Closes #114
Description
The previous setup made two LLM calls using the same input data. This update combines them into one structured prompt that returns the summary and SEO fields together. The new prompt is cleaner, consistent, and handles missing fields safely.
Motivation and Context
The old approach wasted tokens and added unnecessary complexity. Unifying the prompts reduces cost, speeds up execution, and keeps the codebase easier to maintain.
How has this been tested?
Screenshots:
Types of changes
Checklist: