feat: generate article summaries with chat gpt #203
Conversation
|
Warning Rate limit exceeded@iankressin has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 25 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe pull request introduces significant updates to the project, primarily through the addition of new dependencies in Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (7)
server/apps/research/admin/article_admin.py (3)
6-6: Remove unused importsettingsThe
django.conf.settingsmodule is imported but not used in this file. Consider removing it to clean up the imports.Apply this diff to fix the issue:
- from django.conf import settings🧰 Tools
🪛 Ruff (0.8.2)
6-6:
django.conf.settingsimported but unusedRemove unused import:
django.conf.settings(F401)
10-10: Remove unused importasyncioAfter replacing
asyncio.runwithasync_to_sync, theasynciomodule is no longer used and can be removed.Apply this diff to fix the issue:
- import asyncio
21-21: Remove unnecessaryfprefix in stringThe string in the
placeholderattribute does not contain any variables, so thefprefix is unnecessary. Removing it avoids confusion.Apply this diff to fix the issue:
- self.fields['summary'].widget = TinyMCE(attrs={'cols': 80, 'rows': 30, 'id': "summary_richtext_field", 'placeholder': f"Article summary will be generated here"}) + self.fields['summary'].widget = TinyMCE(attrs={'cols': 80, 'rows': 30, 'id': "summary_richtext_field", 'placeholder': "Article summary will be generated here"})🧰 Tools
🪛 Ruff (0.8.2)
21-21: f-string without any placeholders
Remove extraneous
fprefix(F541)
server/apps/research/services/gpt_service.py (2)
41-41: Use logging instead of print statements in exception handlingUsing
loggingmodule to log exceptions appropriately.Apply this diff to fix the issue:
except Exception as e: - print(e) + import logging + logging.exception("Error calling OpenAI API:", exc_info=e) raise Exception(f"Error calling OpenAI API: {str(e)}") from eEnsure that your logging configuration is set up to handle logged exceptions.
42-42: Use exception chaining when re-raising exceptionsWhen re-raising an exception, use
raise ... from eto preserve the original stack trace and exception context. This aids in debugging and provides more informative error messages.Apply this diff to fix the issue:
except Exception as e: logging.exception("Error calling OpenAI API:", exc_info=e) - raise Exception(f"Error calling OpenAI API: {str(e)}") + raise Exception(f"Error calling OpenAI API: {str(e)}") from e🧰 Tools
🪛 Ruff (0.8.2)
42-42: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
server/apps/research/models/article.py (1)
28-29: Add validation methods for summary fieldsConsider adding methods to validate summary length and content to ensure they meet your requirements.
+ def clean(self): + super().clean() + if self.summary and len(self.summary) > 5000: # adjust limit as needed + raise ValidationError({ + 'summary': 'Summary is too long. Maximum length is 5000 characters.' + }) + if self.gpt_summary and len(self.gpt_summary) > 5000: + raise ValidationError({ + 'gpt_summary': 'GPT summary is too long. Maximum length is 5000 characters.' + })server/apps/research/migrations/0017_article_gpt_summary_alter_article_summary.py (1)
19-23: Consider adding a data migration strategyThe migration alters the
summaryfield type and addsgpt_summary, but doesn't handle existing data. Consider:
- Adding a data migration to populate
gpt_summaryfor existing articles- Ensuring consistency between
summaryandgpt_summaryfieldsWould you like me to help create a data migration strategy?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
requirements.txt(1 hunks)server/apps/research/admin/article_admin.py(3 hunks)server/apps/research/migrations/0017_article_gpt_summary_alter_article_summary.py(1 hunks)server/apps/research/models/article.py(1 hunks)server/apps/research/services/__init__.py(1 hunks)server/apps/research/services/gpt_service.py(1 hunks)server/core/config/base.py(1 hunks)server/static/css/article_admin.css(1 hunks)server/static/js/article_admin.js(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- server/static/css/article_admin.css
- server/apps/research/services/init.py
🧰 Additional context used
🪛 Ruff (0.8.2)
server/apps/research/services/gpt_service.py
42-42: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
server/apps/research/admin/article_admin.py
6-6: django.conf.settings imported but unused
Remove unused import: django.conf.settings
(F401)
21-21: f-string without any placeholders
Remove extraneous f prefix
(F541)
🪛 GitHub Check: CodeQL
server/apps/research/admin/article_admin.py
[warning] 57-57: Information exposure through an exception
Stack trace information flows to this location and may be exposed to an external user.
🔇 Additional comments (4)
server/apps/research/services/gpt_service.py (1)
1-2:
Correct the usage of OpenAI's asynchronous API
The AsyncOpenAI class does not exist in the OpenAI Python library. To use the asynchronous API, import the openai module and use await openai.ChatCompletion.acreate(...).
Apply this diff to fix the issue:
- from django.conf import settings
- from openai import AsyncOpenAI
+ import openai
+ from django.conf import settings
class GPTService:
"""Service for handling OpenAI GPT API interactions."""
def __init__(self):
- self.client = AsyncOpenAI(api_key=settings.OPENAI_API_KEY)
+ openai.api_key = settings.OPENAI_API_KEY
self.model = "gpt-3.5-turbo"
self.max_tokens = 500
async def prompt(self, system: str, user: str) -> str:
"""
Send a prompt to GPT and get the response.
"""
if not settings.OPENAI_API_KEY:
raise Exception("OpenAI API key is not configured")
try:
- completion = await self.client.chat.completions.create(
+ completion = await openai.ChatCompletion.acreate(
model=self.model,
messages=[
{"role": "system", "content": system},
{"role": "user", "content": user}
],
max_tokens=self.max_tokens
)
return completion.choices[0].message.content
except Exception as e:
print(e)
raise Exception(f"Error calling OpenAI API: {str(e)}") Likely invalid or redundant comment.
server/core/config/base.py (1)
201-202:
Add validation for required OpenAI API key
The current implementation allows None as a default value, which could lead to runtime errors. Since this is a required feature, we should fail fast if the API key is missing.
-OPENAI_API_KEY = config('OPENAI_API_KEY', default=None)
+OPENAI_API_KEY = config('OPENAI_API_KEY')Also, verify that the API key is properly set in your environment:
server/apps/research/models/article.py (1)
28-29: 🛠️ Refactor suggestion
Verify migration strategy for existing summaries
The change from TextField to HTMLField for summary requires careful consideration of existing data. Additionally, consider adding help text for both fields to clarify their purpose.
- summary = HTMLField(blank=True, null=True)
- gpt_summary = models.TextField(blank=True, null=True)
+ summary = HTMLField(
+ blank=True,
+ null=True,
+ help_text="Manual summary with rich text formatting support"
+ )
+ gpt_summary = models.TextField(
+ blank=True,
+ null=True,
+ help_text="Auto-generated summary by GPT"
+ )✅ Verification successful
Migration strategy for summary field is properly handled
The migration strategy is appropriate and safe:
- Initial migration (0001) shows
summarywas a TextField withblank=True - Current migration (0017) properly handles the field change from TextField to HTMLField while preserving nullability
- No data migration is needed as HTMLField is compatible with TextField data
Your suggestion to add help text is valuable for code clarity but not critical for the migration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for existing migrations related to summary field
fd -t f ".*_article.*\.py$" "server/apps/research/migrations" -x grep -l "summary"
Length of output: 162
Script:
#!/bin/bash
# Check the content of the migration file to understand the migration strategy
cat server/apps/research/migrations/0017_article_gpt_summary_alter_article_summary.py
Length of output: 680
Script:
#!/bin/bash
# Check for any previous migrations that might have defined the initial summary field
fd -t f "^[0-9]{4}_initial\.py$" "server/apps/research/migrations" -x grep -l "summary"
Length of output: 133
Script:
#!/bin/bash
# Look for any data migrations related to summary field
rg "migrations\.RunPython" "server/apps/research/migrations/0017_article_gpt_summary_alter_article_summary.py"
Length of output: 110
Script:
#!/bin/bash
# Check the initial migration to understand the original field type
cat server/apps/research/migrations/0001_initial.py
Length of output: 3374
Script:
#!/bin/bash
# Check if there are any existing data in the summary field that might need special handling
rg "summary\s*=\s*models\." server/apps/research/models/article.py -B2 -A2
Length of output: 365
server/apps/research/migrations/0017_article_gpt_summary_alter_article_summary.py (1)
14-18: Verify the impact of adding the new gpt_summary field
The migration adds a new nullable TextField for GPT summaries. Let's verify there are no existing constraints or triggers that might be affected.
✅ Verification successful
Let me check the complete Article model definition to verify any constraints or dependencies related to the summary fields.
Adding gpt_summary field is safe and won't affect existing functionality
After reviewing the Article model, I can confirm that:
- The new gpt_summary field is properly added as a nullable TextField
- There are no constraints, triggers, or model methods that depend on or reference the summary fields
- The save method and other model methods don't have any logic tied to summary or gpt_summary fields
- Both summary and gpt_summary being nullable maintains backward compatibility
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any database constraints or triggers related to the Article model
rg -A 5 "class Article" server/apps/research/models/
Length of output: 1016
Script:
#!/bin/bash
# Get the complete Article model definition
cat server/apps/research/models/article.py
Length of output: 5726
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
server/static/css/article_admin.css (1)
1-11: LGTM! Consider adding hover state for better UXThe button styling is well-defined with good visual feedback for disabled state.
Consider adding hover state for better user experience:
.generate-summary-btn { background-color: #0C4B33; color: white; padding: 10px 15px; border: none; border-radius: 4px; cursor: pointer; font-weight: bold; margin-left: 10px; align-self: baseline; } +.generate-summary-btn:hover:not(:disabled) { + background-color: #0d5c3f; +}server/apps/research/admin/article_admin.py (4)
6-6: Remove unused importThe
settingsimport is not used in the code.-from django.conf import settings🧰 Tools
🪛 Ruff (0.8.2)
6-6:
django.conf.settingsimported but unusedRemove unused import:
django.conf.settings(F401)
21-21: Remove unnecessary f-stringThe string doesn't contain any placeholders, making the f-string unnecessary.
- self.fields['gpt_summary'].widget = TinyMCE(attrs={'cols': 80, 'rows': 15, 'id': "gpt_summary_richtext_field", 'placeholder': f"GPT-generated summary will appear here"}) + self.fields['gpt_summary'].widget = TinyMCE(attrs={'cols': 80, 'rows': 15, 'id': "gpt_summary_richtext_field", 'placeholder': "GPT-generated summary will appear here"})🧰 Tools
🪛 Ruff (0.8.2)
21-21: f-string without any placeholders
Remove extraneous
fprefix(F541)
38-47: Consider making system prompt configurableThe system prompt is hardcoded in the code. Consider moving it to settings or a configuration file for easier maintenance.
+ SYSTEM_PROMPT = ( + "You are a professional summarizer at 2077 Research. Below is an article on Ethereum technical aspects. " + "Your goal is to produce a summary that is shorter than the original content, yet detailed enough for readers " + "to fully understand the piece without needing to read the original. Your summary should:\n" + "- Provide enough depth and detail so the user gets a complete understanding of the core ideas.\n" + "- Be in HTML format, use <h3> tags for headings if needed. Avoid other heading levels.\n" + "- Minimize the use of bullet points. If you need to list items, you can, but prefer concise paragraph formatting.\n\n" + ) + async def _generate_summary(self, content: str) -> str: - system_prompt = ( - "You are a professional summarizer at 2077 Research. Below is an article on Ethereum technical aspects. " - "Your goal is to produce a summary that is shorter than the original content, yet detailed enough for readers " - "to fully understand the piece without needing to read the original. Your summary should:\n" - "- Provide enough depth and detail so the user gets a complete understanding of the core ideas.\n" - "- Be in HTML format, use <h3> tags for headings if needed. Avoid other heading levels.\n" - "- Minimize the use of bullet points. If you need to list items, you can, but prefer concise paragraph formatting.\n\n" - ) - return await self.gpt_service.prompt(system_prompt, content) + return await self.gpt_service.prompt(self.SYSTEM_PROMPT, content)
65-65: Consider grouping related fieldsThe
gpt_summaryfield is placed betweensummaryandstatus. Consider grouping it withsummaryfor better organization.- ('Article Details', {'fields': ['title', 'slug', 'authors', 'acknowledgement', 'categories', 'thumb', 'content', 'summary', 'gpt_summary', 'status', 'scheduled_publish_time']}), + ('Article Details', {'fields': ['title', 'slug', 'authors', 'acknowledgement', 'categories', 'thumb', 'content', 'summary', 'status', 'scheduled_publish_time']}), + ('AI-Generated Content', {'fields': ['gpt_summary']}),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
server/apps/research/admin/article_admin.py(3 hunks)server/apps/research/migrations/0017_article_gpt_summary_alter_article_summary.py(1 hunks)server/apps/research/models/article.py(1 hunks)server/static/css/article_admin.css(1 hunks)server/static/js/article_admin.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- server/apps/research/migrations/0017_article_gpt_summary_alter_article_summary.py
- server/apps/research/models/article.py
🧰 Additional context used
🪛 Ruff (0.8.2)
server/apps/research/admin/article_admin.py
6-6: django.conf.settings imported but unused
Remove unused import: django.conf.settings
(F401)
21-21: f-string without any placeholders
Remove extraneous f prefix
(F541)
🪛 GitHub Check: CodeQL
server/apps/research/admin/article_admin.py
[warning] 57-57: Information exposure through an exception
Stack trace information flows to this location and may be exposed to an external user.
🔇 Additional comments (4)
server/static/js/article_admin.js (3)
25-52:
Previous race condition and error handling concerns still apply
The click handler needs improvements for preventing race conditions and handling timeouts.
Refer to the previous review comment about improving error handling and preventing race conditions.
57-85:
Enhance error handling in generateSummary function
The error handling could be more robust with better error message extraction.
Refer to the previous review comment about enhancing error handling and adding content validation.
47-48:
Fix undefined variable reference
The variable gptSummaryEditor is undefined. This should be summaryEditor.
- contentEditor.setMode('design');
- gptSummaryEditor.setMode('design');
+ contentEditor.setMode('design');
+ const summaryEditor = tinymce.get('gpt_summary_richtext_field');
+ if (summaryEditor) {
+ summaryEditor.setMode('design');
+ }Likely invalid or redundant comment.
server/apps/research/admin/article_admin.py (1)
49-58:
Previous security and asyncio concerns still apply
The view has two issues that were highlighted in previous reviews:
- Exception details exposure
- Use of
asyncio.runin a synchronous view
Refer to the previous review comments about:
- Avoiding exposing internal exception details
- Using
async_to_syncinstead ofasyncio.run
🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 57-57: Information exposure through an exception
Stack trace information flows to this location and may be exposed to an external user.
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
server/apps/research/admin/article_admin.py (4)
6-6: Remove unused importThe
settingsimport is not used in this file.-from django.conf import settings🧰 Tools
🪛 Ruff (0.8.2)
6-6:
django.conf.settingsimported but unusedRemove unused import:
django.conf.settings(F401)
21-21: Remove unnecessary f-stringThe string doesn't contain any placeholders, so the
fprefix is not needed.- self.fields['gpt_summary'].widget = TinyMCE(attrs={'cols': 80, 'rows': 15, 'id': "gpt_summary_richtext_field", 'placeholder': f"GPT-generated summary will appear here"}) + self.fields['gpt_summary'].widget = TinyMCE(attrs={'cols': 80, 'rows': 15, 'id': "gpt_summary_richtext_field", 'placeholder': "GPT-generated summary will appear here"})🧰 Tools
🪛 Ruff (0.8.2)
21-21: f-string without any placeholders
Remove extraneous
fprefix(F541)
27-29: Consider making GPTService a singleton or class-level instanceCreating a new GPTService instance for each ArticleAdmin instance could be inefficient. Consider making it a class-level attribute or implementing it as a singleton.
class ArticleAdmin(admin.ModelAdmin): """Admin interface for the Article model.""" form = ArticleForm + gpt_service = GPTService() def __init__(self, model, admin_site): super().__init__(model, admin_site) - self.gpt_service = GPTService()
38-47: Move system prompt to settings or constantsThe system prompt should be moved to a settings file or constants module for better maintainability and reusability.
+# In constants.py or settings.py +GPT_SYSTEM_PROMPT = """ +You are a professional summarizer at 2077 Research. Below is an article on Ethereum technical aspects. +Your goal is to produce a summary that is shorter than the original content, yet detailed enough for readers +to fully understand the piece without needing to read the original. Your summary should: +- Provide enough depth and detail so the user gets a complete understanding of the core ideas. +- Be in HTML format, use <h3> tags for headings if needed. Avoid other heading levels. +- Minimize the use of bullet points. If you need to list items, you can, but prefer concise paragraph formatting. +""" # In article_admin.py async def _generate_summary(self, content: str) -> str: - system_prompt = ( - "You are a professional summarizer at 2077 Research. Below is an article on Ethereum technical aspects. " - "Your goal is to produce a summary that is shorter than the original content, yet detailed enough for readers " - "to fully understand the piece without needing to read the original. Your summary should:\n" - "- Provide enough depth and detail so the user gets a complete understanding of the core ideas.\n" - "- Be in HTML format, use <h3> tags for headings if needed. Avoid other heading levels.\n" - "- Minimize the use of bullet points. If you need to list items, you can, but prefer concise paragraph formatting.\n\n" - ) - return await self.gpt_service.prompt(system_prompt, content) + return await self.gpt_service.prompt(settings.GPT_SYSTEM_PROMPT, content)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
server/apps/research/admin/article_admin.py(3 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
server/apps/research/admin/article_admin.py
6-6: django.conf.settings imported but unused
Remove unused import: django.conf.settings
(F401)
21-21: f-string without any placeholders
Remove extraneous f prefix
(F541)
56-56: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
🔇 Additional comments (2)
server/apps/research/admin/article_admin.py (2)
49-60:
Replace asyncio.run with async_to_sync
Using asyncio.run in a Django view can lead to performance issues. Use async_to_sync from asgiref.sync instead.
+from asgiref.sync import async_to_sync
def generate_summary_view(self, request):
if request.method == 'POST':
content = request.POST.get('content')
try:
- gpt_summary = asyncio.run(self._generate_summary(content))
+ gpt_summary = async_to_sync(self._generate_summary)(content)
return JsonResponse({'summary': gpt_summary})
except Exception as e:
import logging
logging.error("An error occurred while generating the summary", exc_info=True)
return JsonResponse({'error': 'An internal error has occurred!'}, status=500)
return JsonResponse({'error': 'Invalid request method'}, status=400)🧰 Tools
🪛 Ruff (0.8.2)
56-56: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
82-86: Verify static files existence and content
Ensure that the referenced static files exist and contain the necessary code for summary generation functionality.
✅ Verification successful
Static files are properly implemented with summary generation functionality
The static files exist and contain the necessary code:
- CSS file includes styles for the summary generation button and status indicators
- JS file implements the complete summary generation functionality:
- Integrates with TinyMCE editor
- Handles API calls to generate summaries
- Manages UI state and error handling
- Includes CSRF protection
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the static files exist and contain required code
echo "Checking for static files..."
fd -t f "article_admin\.(css|js)$"
echo "Checking CSS content for summary-related styles..."
rg -l "summary|gpt" "**/*article_admin.css"
echo "Checking JS content for summary generation functionality..."
rg -l "generateSummary|summary" "**/*article_admin.js"
Length of output: 639
Script:
#!/bin/bash
# Check content of the static files using the correct paths
echo "Checking CSS content for summary-related styles..."
cat "server/static/css/article_admin.css"
echo -e "\nChecking JS content for summary generation functionality..."
cat "server/static/js/article_admin.js"
Length of output: 4782
…ub.com:2077-Collective/2077-CMS into feat--generate-article-summaries-with-chat-gpt
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
server/apps/research/serializers/article_serializer.py (2)
29-29: Consider adding field-level configuration forgpt_summary.The
gpt_summaryfield might benefit from additional configuration:
- Consider making it read-only since it's generated by GPT service
- Consider grouping it with the
summaryfield for better organizationclass ArticleSerializer(serializers.ModelSerializer): authors = AuthorSerializer(many=True, read_only=True) categories = CategorySerializer(many=True) views = serializers.ReadOnlyField() min_read = serializers.ReadOnlyField() + gpt_summary = serializers.CharField(read_only=True) class Meta: model = Article fields = [ - 'id', 'slug', 'title', 'authors', 'thumb', 'categories', 'summary', - 'acknowledgement', 'content', 'min_read', 'status', 'views', 'gpt_summary', + 'id', 'slug', 'title', 'authors', 'thumb', 'categories', + 'summary', 'gpt_summary', # Group summary fields together + 'acknowledgement', 'content', 'min_read', 'status', 'views', 'created_at', 'updated_at', 'scheduled_publish_time', 'table_of_contents', 'is_sponsored', 'sponsor_color', 'sponsor_text_color' ]
Line range hint
10-10: Fix typo in ArticleListSerializer's include field.There's a missing comma in the
includelist which would cause a syntax error.class Meta: model = Article - include = ['categories' 'authors'] + include = ['categories', 'authors']
| @@ -26,7 +26,7 @@ class Meta: | |||
| model = Article | |||
| fields = [ | |||
| 'id', 'slug', 'title', 'authors', 'thumb', 'categories', 'summary', | |||
| 'acknowledgement', 'content', 'min_read', 'status', 'views', | |||
| 'acknowledgement', 'content', 'min_read', 'status', 'views', 'gpt_summary', | |||
There was a problem hiding this comment.
💡 Codebase verification
The gpt_summary field exists in the Article model but with a different field type
The gpt_summary field exists in both the Article model and the migration file, but there's a discrepancy in the field type:
- In the model:
models.TextField(blank=True, null=True) - In the migration:
tinymce.models.HTMLField(blank=True, null=True)
This inconsistency needs to be addressed to match the field types.
🔗 Analysis chain
Verify the Article model schema.
Ensure that the gpt_summary field exists in the Article model schema as referenced in both serializers.
Also applies to: 41-41
Let me gather more information about the gpt_summary field from the migration file and the complete Article model.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if gpt_summary field exists in Article model and its migration
# Check Article model
rg -A 5 "class Article" server/apps/research/models/
# Check migrations for gpt_summary field
rg "gpt_summary" server/apps/research/migrations/
Length of output: 1180
Script:
#!/bin/bash
# Check the complete Article model definition
cat server/apps/research/models/article.py
# Check the migration file that added gpt_summary
cat server/apps/research/migrations/0017_article_gpt_summary_alter_article_summary.py
Length of output: 6230
There was a problem hiding this comment.
There still remains an issue here to be resolved
There was a problem hiding this comment.
@losndu, to resolve the remaining issue, please ensure that the gpt_summary field in the Article model and the migration have the same field type. Currently, there is a mismatch:
-
In
server/apps/research/models/article.py,gpt_summaryis defined as:gpt_summary = models.TextField(blank=True, null=True)
-
In
server/apps/research/migrations/0017_article_gpt_summary_alter_article_summary.py, it is defined as:field=models.HTMLField(blank=True, null=True),
To fix the inconsistency, you can update the model to use HTMLField. Here's how:
-
Import
HTMLFieldfromtinymce.modelsin your model file:from tinymce.models import HTMLField
-
Update the
gpt_summaryfield in theArticlemodel:gpt_summary = HTMLField(blank=True, null=True)
Alternatively, if you prefer to keep gpt_summary as a TextField, you should update the migration file to reflect TextField instead of HTMLField. However, it's generally recommended to have rich text fields like summaries use HTMLField for better content management.
After making the changes, don't forget to run:
python manage.py makemigrations
python manage.py migrateThis will create a new migration to reflect the changes and apply it to your database.
Let me know if you have any questions or need further assistance!
| @@ -38,7 +38,7 @@ | |||
|
|
|||
| class Meta: | |||
| model = Article | |||
| fields = ['title', 'slug', 'categories', 'thumb', 'content', 'summary', 'acknowledgement', 'status', 'authors', 'scheduled_publish_time', 'is_sponsored', 'sponsor_color', 'sponsor_text_color'] | |||
| fields = ['title', 'slug', 'categories', 'thumb', 'content', 'summary', 'gpt_summary', 'acknowledgement', 'status', 'authors', 'scheduled_publish_time', 'is_sponsored', 'sponsor_color', 'sponsor_text_color'] | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve field list readability and handling.
The current implementation has the following issues:
- The line is too long, making it hard to read
- Field ordering could be improved by grouping related fields
- The
gpt_summaryfield might need special handling in create/update methods
class Meta:
model = Article
- fields = ['title', 'slug', 'categories', 'thumb', 'content', 'summary', 'gpt_summary', 'acknowledgement', 'status', 'authors', 'scheduled_publish_time', 'is_sponsored', 'sponsor_color', 'sponsor_text_color']
+ fields = [
+ # Basic info
+ 'title', 'slug', 'authors', 'categories',
+ # Content
+ 'thumb', 'content',
+ # Summaries
+ 'summary', 'gpt_summary',
+ # Additional info
+ 'acknowledgement', 'status',
+ 'scheduled_publish_time',
+ # Sponsorship
+ 'is_sponsored', 'sponsor_color', 'sponsor_text_color'
+ ]Also, consider adding validation in create/update methods to ensure gpt_summary isn't directly set through the API:
def create(self, validated_data: dict) -> Article:
"""Create a new article instance."""
+ # Ensure gpt_summary can't be set directly
+ validated_data.pop('gpt_summary', None)
request = self.context.get('request')
authors = validated_data.pop('authors', [])
categories = validated_data.pop('categories', [])
def update(self, instance: Article, validated_data: dict) -> Article:
"""Update an existing article instance."""
+ # Ensure gpt_summary can't be set directly
+ validated_data.pop('gpt_summary', None)
authors = validated_data.pop('authors', [])
categories = validated_data.pop('categories', [])📝 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.
| fields = ['title', 'slug', 'categories', 'thumb', 'content', 'summary', 'gpt_summary', 'acknowledgement', 'status', 'authors', 'scheduled_publish_time', 'is_sponsored', 'sponsor_color', 'sponsor_text_color'] | |
| fields = [ | |
| # Basic info | |
| 'title', 'slug', 'authors', 'categories', | |
| # Content | |
| 'thumb', 'content', | |
| # Summaries | |
| 'summary', 'gpt_summary', | |
| # Additional info | |
| 'acknowledgement', 'status', | |
| 'scheduled_publish_time', | |
| # Sponsorship | |
| 'is_sponsored', 'sponsor_color', 'sponsor_text_color' | |
| ] |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
server/apps/research/admin/article_admin.py (2)
7-7: Remove unused importThe
settingsimport is not used in this file.-from django.conf import settings🧰 Tools
🪛 Ruff (0.8.2)
7-7:
django.conf.settingsimported but unusedRemove unused import:
django.conf.settings(F401)
29-47: Consider refactoring TinyMCE widget configurationThe TinyMCE widget configuration is duplicated across multiple fields. Consider extracting the common configuration into a helper function.
+ def _get_tinymce_widget(self, rows=30, field_name=""): + return TinyMCE(attrs={ + 'cols': 80, + 'rows': rows, + 'id': f"{field_name}_richtext_field", + 'placeholder': f"Enter {field_name.title()} here" + }) def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.fields['acknowledgement'].widget = TinyMCE(attrs={ - 'cols': 80, - 'rows': 30, - 'id': "acknowledgement_richtext_field", - 'placeholder': "Enter Acknowledgement here" - }) - self.fields['content'].widget = TinyMCE(attrs={ - 'cols': 80, - 'rows': 30, - 'id': "content_richtext_field", - 'placeholder': "Enter Article Content here" - }) - self.fields['gpt_summary'].widget = TinyMCE(attrs={ - 'cols': 80, - 'rows': 15, - 'id': "gpt_summary_richtext_field", - 'placeholder': "GPT-generated summary will appear here" - }) + self.fields['acknowledgement'].widget = self._get_tinymce_widget(field_name="acknowledgement") + self.fields['content'].widget = self._get_tinymce_widget(field_name="content") + self.fields['gpt_summary'].widget = self._get_tinymce_widget(rows=15, field_name="gpt_summary")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
server/apps/research/admin/article_admin.py(4 hunks)server/apps/research/migrations/0017_article_gpt_summary_alter_article_summary.py(1 hunks)server/apps/research/models/article.py(1 hunks)server/apps/research/serializers/article_serializer.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- server/apps/research/migrations/0017_article_gpt_summary_alter_article_summary.py
- server/apps/research/serializers/article_serializer.py
- server/apps/research/models/article.py
🧰 Additional context used
🪛 Ruff (0.8.2)
server/apps/research/admin/article_admin.py
7-7: django.conf.settings imported but unused
Remove unused import: django.conf.settings
(F401)
81-81: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
🔇 Additional comments (4)
server/apps/research/admin/article_admin.py (4)
53-62: LGTM! Clean implementation of service initialization and URL configuration
The implementation follows Django best practices:
- Service initialization in
__init__ensures single instance per admin - URL configuration properly extends Django's admin URLs
119-124: LGTM! Proper setup of admin assets
The Media class is correctly configured to include custom CSS and JavaScript files for the summary generation feature.
75-85: 🛠️ Refactor suggestion
Improve async handling and error management
- Replace
asyncio.runwithasync_to_syncfor better Django integration - Enhance error handling with structured logging
+from asgiref.sync import async_to_sync
+import logging
def generate_summary_view(self, request):
if request.method == 'POST':
content = request.POST.get('content')
+ if not content:
+ return JsonResponse({'error': 'Content is required'}, status=400)
try:
- gpt_summary = asyncio.run(self._generate_summary(content))
+ gpt_summary = async_to_sync(self._generate_summary)(content)
return JsonResponse({'summary': gpt_summary})
except Exception as e:
- import logging
- logging.error("An error occurred while generating the summary", exc_info=True)
+ logging.error(
+ "Failed to generate summary",
+ extra={
+ 'content_length': len(content),
+ 'error_type': type(e).__name__
+ },
+ exc_info=True
+ )
return JsonResponse({'error': 'An internal error has occurred!'}, status=500)🧰 Tools
🪛 Ruff (0.8.2)
81-81: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
10-10: Verify GPT service integration
Let's verify the GPT service implementation and its dependencies.
✅ Verification successful
GPT service integration is properly implemented
The verification shows a complete and well-structured integration:
- GPTService is correctly implemented with async support and proper error handling
- The service is properly imported and instantiated in article_admin.py
- The generate_summary_view implementation includes proper error handling and async execution
- OpenAI API key configuration is properly handled through settings
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check GPT service implementation and related files
# Check if GPT service exists and has required methods
echo "Checking GPT service implementation..."
ast-grep --pattern 'class GPTService {
$$$
prompt($_, $_) {
$$$
}
$$$
}'
# Check for related static files
echo "Checking static files..."
fd -e css -e js "article_admin" server/static
# Check for potential circular imports
echo "Checking for circular imports..."
rg -l "from.*article_admin.*import" server/apps/research/
Length of output: 552
Script:
#!/bin/bash
# Let's check the actual GPTService implementation and its usage
# Check GPTService implementation
echo "Checking GPTService implementation in services directory..."
rg -A 10 "class GPTService" server/apps/research/services/
# Check how GPTService is used in article_admin
echo "Checking GPTService usage in article_admin..."
rg "GPTService" server/apps/research/admin/article_admin.py -A 5
# Check if there are any other files using GPTService
echo "Checking other GPTService usages..."
rg "GPTService" server/apps/research/ --type py
Length of output: 1940
Script:
#!/bin/bash
# Let's verify the complete integration by checking the generate_summary_view implementation
# Check the generate_summary_view implementation
echo "Checking generate_summary_view implementation..."
rg -A 15 "def generate_summary_view" server/apps/research/admin/article_admin.py
# Check if Article model has the necessary fields
echo "Checking Article model fields..."
rg "class Article" -A 10 server/apps/research/models.py
# Verify AsyncOpenAI import and settings
echo "Checking OpenAI settings..."
rg "OPENAI_API_KEY" server/apps/research/ --type py
Length of output: 1410
| async def _generate_summary(self, content: str) -> str: | ||
| system_prompt = ( | ||
| "You are a professional summarizer at 2077 Research. Below is an article on Ethereum technical aspects. " | ||
| "Your goal is to produce a summary that is shorter than the original content, yet detailed enough for readers " | ||
| "to fully understand the piece without needing to read the original. Your summary should:\n" | ||
| "- Provide enough depth and detail so the user gets a complete understanding of the core ideas.\n" | ||
| "- Be in HTML format, use <h3> tags for headings if needed. Avoid other heading levels.\n" | ||
| "- Minimize the use of bullet points. If you need to list items, you can, but prefer concise paragraph formatting.\n\n" | ||
| ) | ||
| return await self.gpt_service.prompt(system_prompt, content) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move system prompt to settings or constants
The system prompt should be moved to a settings file or constants module for better maintainability and reusability.
+# In apps/research/constants.py
+GPT_SYSTEM_PROMPT = """
+You are a professional summarizer at 2077 Research. Below is an article on Ethereum technical aspects.
+Your goal is to produce a summary that is shorter than the original content, yet detailed enough for readers
+to fully understand the piece without needing to read the original. Your summary should:
+- Provide enough depth and detail so the user gets a complete understanding of the core ideas.
+- Be in HTML format, use <h3> tags for headings if needed. Avoid other heading levels.
+- Minimize the use of bullet points. If you need to list items, you can, but prefer concise paragraph formatting.
+"""
# In article_admin.py
+from ..constants import GPT_SYSTEM_PROMPT
async def _generate_summary(self, content: str) -> str:
- system_prompt = (
- "You are a professional summarizer at 2077 Research. Below is an article on Ethereum technical aspects. "
- "Your goal is to produce a summary that is shorter than the original content, yet detailed enough for readers "
- "to fully understand the piece without needing to read the original. Your summary should:\n"
- "- Provide enough depth and detail so the user gets a complete understanding of the core ideas.\n"
- "- Be in HTML format, use <h3> tags for headings if needed. Avoid other heading levels.\n"
- "- Minimize the use of bullet points. If you need to list items, you can, but prefer concise paragraph formatting.\n\n"
- )
- return await self.gpt_service.prompt(system_prompt, content)
+ return await self.gpt_service.prompt(GPT_SYSTEM_PROMPT, content)📝 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.
| async def _generate_summary(self, content: str) -> str: | |
| system_prompt = ( | |
| "You are a professional summarizer at 2077 Research. Below is an article on Ethereum technical aspects. " | |
| "Your goal is to produce a summary that is shorter than the original content, yet detailed enough for readers " | |
| "to fully understand the piece without needing to read the original. Your summary should:\n" | |
| "- Provide enough depth and detail so the user gets a complete understanding of the core ideas.\n" | |
| "- Be in HTML format, use <h3> tags for headings if needed. Avoid other heading levels.\n" | |
| "- Minimize the use of bullet points. If you need to list items, you can, but prefer concise paragraph formatting.\n\n" | |
| ) | |
| return await self.gpt_service.prompt(system_prompt, content) | |
| async def _generate_summary(self, content: str) -> str: | |
| return await self.gpt_service.prompt(GPT_SYSTEM_PROMPT, content) |
Adds a new summary field, called
gpt_summaryto the article model, which is the field that will store the article summary generated by ChatGPT API.Also includes new GPT service, which takes a system prompt and a user prompt and return the result from the prompt. This is what's being used to generate summaries.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Style
Chores