-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
OpenAI Integration Fixes and Prompt Enhancements #477
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
WalkthroughOne file had only a whitespace change, removing a trailing newline. Another file updated a prompt by dynamically inserting valid enum values for a field, ensuring output adheres to allowed options, and made a minor formatting correction. No changes were made to public APIs or function/class declarations. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~6 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/backend/app/agent/providers/openai.py(3 hunks)apps/backend/app/core/config.py(1 hunks)apps/backend/app/prompt/structured_job.py(1 hunks)
🔇 Additional comments (4)
apps/backend/app/prompt/structured_job.py (2)
1-4: Excellent implementation of dynamic enum values.The dynamic generation of
remote_status_valuesfromRemoteStatusEnumensures the prompt stays synchronized with schema changes automatically. This is a robust approach that prevents manual maintenance issues.
12-12: Great improvement to prompt specificity.Adding the specific allowed values for
remoteStatusdirectly in the prompt will significantly improve the accuracy of structured output from the OpenAI model by providing clear constraints.apps/backend/app/agent/providers/openai.py (2)
61-61: Good fix for embedding response access.The change from dictionary-style access (
response["data"][0]["embedding"]) to attribute-style access (response.data[0].embedding) correctly addresses the issue mentioned in the PR objectives whereCreateEmbeddingResponseobject was not subscriptable.
15-15: No action needed:gpt-4.1-miniis a valid OpenAI model
- apps/backend/app/agent/providers/openai.py:15 – the default model
"gpt-4.1-mini"is officially supported (a smaller, cost-efficient variant of GPT-4.1). No change required.
| "top_p": generation_args.get("top_p", 0.9), | ||
| "top_k": generation_args.get("top_k", 40), | ||
| "max_tokens": generation_args.get("max_length", 20000), | ||
| "max_output_tokens": generation_args.get("max_length", 20000), |
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.
💡 Verification agent
🧩 Analysis chain
Verify the parameter name change.
The parameter was changed from "max_tokens" to "max_output_tokens". This aligns with the PR objective of fixing parameter issues due to OpenAI API updates.
🌐 Web query:
What is the correct parameter name for limiting output tokens in the current OpenAI API - is it "max_tokens" or "max_output_tokens"?
💡 Result:
In the OpenAI API, the parameter used to limit the number of tokens generated in a model's response is max_tokens. This parameter sets the maximum number of tokens that the model will generate in the completion. It's important to note that the total number of tokens in your prompt plus the max_tokens value cannot exceed the model's context length. (help.openai.com)
For newer reasoning models like o3, o4-mini, and gpt-4.1, OpenAI has introduced the max_completion_tokens parameter. This parameter specifically limits the number of tokens generated in the model's response. However, for earlier models, max_tokens remains the appropriate parameter to use. (help.openai.com)
Therefore, if you're working with newer reasoning models, you should use max_completion_tokens to limit the output tokens. For earlier models, max_tokens is the correct parameter.
Citations:
- 1: https://help.openai.com/en/articles/5072518-controlling-the-length-of-openai-model-responses?utm_source=openai
- 2: https://help.openai.com/en/articles/5072518-controlling-the-length-of-openai-model-responses?utm_source=openai
Use the correct OpenAI API parameter name
We’ve confirmed that the OpenAI API expects max_tokens (or, for newer “o3”-style models, max_completion_tokens) rather than max_output_tokens. Please update the call accordingly.
• apps/backend/app/agent/providers/openai.py:39
- "max_output_tokens": generation_args.get("max_length", 20000),
+ "max_tokens": generation_args.get("max_length", 20000),If you’re targeting a newer reasoning model (e.g. o3, o4-mini, gpt-4.1), switch to:
- "max_output_tokens": generation_args.get("max_length", 20000),
+ "max_completion_tokens": generation_args.get("max_length", 20000),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "max_output_tokens": generation_args.get("max_length", 20000), | |
| "max_tokens": generation_args.get("max_length", 20000), |
🤖 Prompt for AI Agents
In apps/backend/app/agent/providers/openai.py at line 39, the parameter name
"max_output_tokens" is incorrect for the OpenAI API. Replace "max_output_tokens"
with "max_tokens" for standard models or "max_completion_tokens" if targeting
newer "o3"-style models like "o3", "o4-mini", or "gpt-4.1" to ensure
compatibility with the API.
apps/backend/app/core/config.py
Outdated
| SESSION_SECRET_KEY: Optional[str] | ||
| DB_ECHO: bool = False | ||
| PYTHONDONTWRITEBYTECODE: int = 1 | ||
| OPENAI_API_KEY: str = os.getenv('OPENAI_API_KEY') |
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.
🛠️ Refactor suggestion
Fix type annotation to handle potential None value.
The type annotation indicates str but os.getenv('OPENAI_API_KEY') returns None if the environment variable is not set, causing a type mismatch.
Apply this diff to fix the type annotation:
- OPENAI_API_KEY: str = os.getenv('OPENAI_API_KEY')
+ OPENAI_API_KEY: Optional[str] = os.getenv('OPENAI_API_KEY')Alternatively, if you want to ensure a string value:
- OPENAI_API_KEY: str = os.getenv('OPENAI_API_KEY')
+ OPENAI_API_KEY: str = os.getenv('OPENAI_API_KEY', '')🤖 Prompt for AI Agents
In apps/backend/app/core/config.py at line 17, the variable OPENAI_API_KEY is
annotated as str but os.getenv can return None if the environment variable is
not set. To fix this, change the type annotation to Optional[str] to reflect
that the value can be either a string or None. Import Optional from typing if
not already imported.
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.
cubic analysis
2 issues found across 3 files • Review in cubic
React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai to give feedback, ask questions, or re-run the review.
apps/backend/app/core/config.py
Outdated
| SESSION_SECRET_KEY: Optional[str] | ||
| DB_ECHO: bool = False | ||
| PYTHONDONTWRITEBYTECODE: int = 1 | ||
| OPENAI_API_KEY: str = os.getenv('OPENAI_API_KEY') |
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.
Rule violated: Detect Unused or Redundant Configuration Parameters
OPENAI_API_KEY is added to Settings, but the rest of the codebase never references settings.OPENAI_API_KEY; all consumers still call os.getenv("OPENAI_API_KEY"). This makes the new configuration parameter redundant and misleading, violating the guideline on removing unused configuration parameters.
Prompt for AI agents
Address the following comment on apps/backend/app/core/config.py at line 17:
<comment>`OPENAI_API_KEY` is added to `Settings`, but the rest of the codebase never references `settings.OPENAI_API_KEY`; all consumers still call `os.getenv("OPENAI_API_KEY")`. This makes the new configuration parameter redundant and misleading, violating the guideline on removing unused configuration parameters.</comment>
<file context>
@@ -14,6 +14,7 @@ class Settings(BaseSettings):
SESSION_SECRET_KEY: Optional[str]
DB_ECHO: bool = False
PYTHONDONTWRITEBYTECODE: int = 1
+ OPENAI_API_KEY: str = os.getenv('OPENAI_API_KEY')
model_config = SettingsConfigDict(
</file context>
apps/backend/app/core/config.py
Outdated
| SESSION_SECRET_KEY: Optional[str] | ||
| DB_ECHO: bool = False | ||
| PYTHONDONTWRITEBYTECODE: int = 1 | ||
| OPENAI_API_KEY: str = os.getenv('OPENAI_API_KEY') |
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.
Annotating OPENAI_API_KEY as str while defaulting to os.getenv() can produce a None value that violates the declared type and defeats Pydantic’s built-in env loading; declare it Optional[str] and let BaseSettings handle the environment lookup instead.
Prompt for AI agents
Address the following comment on apps/backend/app/core/config.py at line 17:
<comment>Annotating OPENAI_API_KEY as str while defaulting to os.getenv() can produce a None value that violates the declared type and defeats Pydantic’s built-in env loading; declare it Optional[str] and let BaseSettings handle the environment lookup instead.</comment>
<file context>
@@ -14,6 +14,7 @@ class Settings(BaseSettings):
SESSION_SECRET_KEY: Optional[str]
DB_ECHO: bool = False
PYTHONDONTWRITEBYTECODE: int = 1
+ OPENAI_API_KEY: str = os.getenv('OPENAI_API_KEY')
model_config = SettingsConfigDict(
</file context>
| OPENAI_API_KEY: str = os.getenv('OPENAI_API_KEY') | |
| OPENAI_API_KEY: Optional[str] = None |
Pull Request Title
Fix user journey with openai API key. e.g: resume or job description parsing etc.
Related Issue
#421
#430
Description
Fixed OpenAI integration issues: resolved
top_kparam update, updated default model, handledCreateEmbeddingResponseaccess error, added missing API key config, and improvedstructure_jobprompt to correctly detect remoteStatus.Type
Proposed Changes
top_kissue happened due to openai API update.app.agent.exceptions.ProviderError: OpenAI - error generating embedding: 'CreateEmbeddingResponse' object is not subscriptablestructure_jobprompt to detect correct value ofremoteStatus.How to Test
Checklist
Summary by cubic
Fixed issues with the OpenAI integration so users can now parse resumes and job descriptions without errors.
Summary by CodeRabbit
Enhancements
Chores