Story 2428: AI-Assisted Description for Blog and News Posts#2473
Story 2428: AI-Assisted Description for Blog and News Posts#2473javiercoronadonarvaez wants to merge 26 commits into
Conversation
javiercoronadonarvaez
changed the title
javiercoronadonarvaez
changed the title
javiercoronadonarvaez
force-pushed
the
javiercoronarv/2428-ai-assisted-desc-blog-news
branch
from
d4af119 to
b6e124d
Compare
javiercoronadonarvaez
force-pushed
the
javiercoronarv/2428-ai-assisted-desc-blog-news
branch
from
df4c171 to
6f862f8
Compare
jlchilders11
left a comment
jlchilders11
left a comment
There was a problem hiding this comment.
Overall good work, but I have a few comments, and a question over the summation that we are performing.
Also a note, it looks like the description for this PR links itself, rather than the the issue number.
| <path d="M3 3h2v18H3V3zm16 0H5v2h14v14H5v2h16V3h-2zm-8 6h2V7h-2v2zm2 8h-2v-6h2v6z" /> | ||
| {% elif icon_name == "check" %} | ||
| <path d="M18 6H20V8H18V6ZM16 10V8H18V10H16ZM14 12V10H16V12H14ZM12 14H14V12H12V14ZM10 16H12V14H10V16ZM8 16V18H10V16H8ZM6 14H8V16H6V14ZM6 14H4V12H6V14Z" /> | ||
| {% elif icon_name == "saving" %} |
There was a problem hiding this comment.
If these always require a certain viewbox, I'd like to see these broken out into their own if/else check to guarantee that, rather than always having to pass that viewbox.
There was a problem hiding this comment.
Great catch! Just addressed.
| NOTE: intentionally not login-gated yet (local testing). This endpoint calls | ||
| a paid LLM, so add @login_required (and rate limiting) before it ships. | ||
| """ | ||
| content = (request.POST.get("content") or "").strip() |
There was a problem hiding this comment.
| content = (request.POST.get("content") or "").strip() | |
| content = (request.POST.get("content") or "").strip() | |
| content = (request.POST.get("content", "")).strip() |
Rather than using or here, our default is to provide the default as part of the get call.
There was a problem hiding this comment.
Got it! Updated title to follow the same convention.
javiercoronadonarvaez
changed the title
javiercoronadonarvaez
changed the title
javiercoronadonarvaez
changed the title
javiercoronadonarvaez
changed the title
herzog0
left a comment
herzog0
left a comment
There was a problem hiding this comment.
Currently looking good to me, just finishing some tests before approval!
| NOTE: intentionally not login-gated yet (local testing). This endpoint calls | ||
| a paid LLM, so add @login_required (and rate limiting) before it ships. |
There was a problem hiding this comment.
Should we address this before merging?
There was a problem hiding this comment.
I was thinking of leaving this as part of a separate rate limiting PR first of all because seems more natural and second to expedite the auto generation process for Links, since this principle applies to both this PR and that of Links.
Let me know what you think about this.
There was a problem hiding this comment.
@javiercoronadonarvaez If possible, I think including adding @login_required in this PR and leave the rate limiting to a separate PR would be great, especially when we don't have a ticket for rate limiting prioritized just yet 😄
|
Looks good! Besides the existing feedback there's just this thing that I believe I also addressed in the What's New PR. What happens is Could we extract the core into a plain helper and keep the task as a thin wrapper, so the view calls the helper directly (with its own timeout) and the background path keeps its retries? Not a blocker, just a cleanup. It would be something like this: |
jlchilders11
left a comment
jlchilders11
left a comment
There was a problem hiding this comment.
LGTM, Pending the other feedback
@ycanales great insight! Just addressed. |
javiercoronadonarvaez
force-pushed
the
javiercoronarv/2428-ai-assisted-desc-blog-news
branch
from
8c72043 to
8a4e778
Compare
javiercoronadonarvaez
force-pushed
the
javiercoronarv/2428-ai-assisted-desc-blog-news
branch
from
8a4e778 to
51d552c
Compare
| @@ -43,7 +43,7 @@ class Meta: | |||
| class NewsForm(EntryForm): | |||
| class Meta: | |||
| model = News | |||
| fields = ["title", "publish_at", "content", "image"] | |||
| fields = ["title", "publish_at", "content", "summary", "image"] | |||
There was a problem hiding this comment.
Hi @javiercoronadonarvaez ! Can we gate the addition of the summary field to just V3? Currently this change is leaking to the legacy view as well, which is something we want to make sure not to do, as this feature is meant for V3 only
| Develop Branch | This Branch |
|---|---|
 |
 |
- Does not display anything if the user hasn't started typing - Display Saving text + Icon while user types and display Saved text + Icon once he's finished typing
- Corroborate auto generate description works with wysiwyg component
- Fix in accordance to GitHub Actions failure
…ated - Add 'Hold on! We are generating a description for your content, it may take a few seconds' as placeholder in description box
…ter post has been created
…ry.summary - Persist user-typed summary - Skip background regeneration when set
…task keeps retries
- Truncation to comply with 1000 character limit from Design templates - Optimize prompt
- Update pre commit config file to match black's version in requirements (26.1.0)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
javiercoronadonarvaez
force-pushed
the
javiercoronarv/2428-ai-assisted-desc-blog-news
branch
from
1bbccec to
0f47b2d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/views.py (1)
1952-1962:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the leading newline from the markdown demo literal.
Line 1952 currently starts
dedent("""on its own line, which injects an initial blank line into rendered markdown and can cause unintended top spacing.Suggested fix
- "markdown": dedent(""" - - ######Insert anything Required + "markdown": dedent("""\ + ######Insert anything Required ... - """), + """),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/views.py` around lines 1952 - 1962, The markdown demo literal under the "markdown" key uses dedent(""" on its own line which injects a leading blank line; update the dedent call so the opening triple-quote immediately precedes the content (e.g., dedent("""######Insert anything Required...) to remove the initial newline and avoid extra top spacing when rendered, leaving the rest of the content and indentation handling in place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@core/views.py`:
- Around line 1952-1962: The markdown demo literal under the "markdown" key uses
dedent(""" on its own line which injects a leading blank line; update the dedent
call so the opening triple-quote immediately precedes the content (e.g.,
dedent("""######Insert anything Required...) to remove the initial newline and
avoid extra top spacing when rendered, leaving the rest of the content and
indentation handling in place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 36ee5165-f1d0-4cfb-8889-0d6aaf6b5017
📒 Files selected for processing (41)
.pre-commit-config.yamlak/views.pyconfig/settings.pyconfig/urls.pycore/context_processors.pycore/tests/test_managers.pycore/tests/test_tasks.pycore/views.pyfrontend/wysiwyg-editor.jsgunicorn.conf.pylibraries/admin.pylibraries/forms.pylibraries/tests/fixtures.pylibraries/tests/test_commands.pylibraries/tests/test_tasks.pymailing_list/management/commands/sync_mailinglist_stats.pymailing_list/tasks.pymarketing/tests.pynews/acl.pynews/constants.pynews/forms.pynews/tasks.pynews/tests/test_forms.pynews/urls.pynews/views.pyslack/management/commands/clear_slack_activity.pyslack/management/commands/fetch_slack_activity.pystatic/css/v3/create-post-page.cssstatic/js/v3/wysiwyg-editor.jstemplates/includes/icon.htmltemplates/news/v3/create.htmlusers/forms.pyusers/tests/test_urls.pyusers/tests/test_views.pyusers/urls.pyusers/views.pyversions/releases.pyversions/tasks.pyversions/tests/test_ai_tasks.pyversions/tests/test_api.pyversions/views.py
💤 Files with no reviewable changes (17)
- versions/tests/test_api.py
- news/urls.py
- versions/tests/test_ai_tasks.py
- ak/views.py
- versions/releases.py
- users/tests/test_views.py
- slack/management/commands/clear_slack_activity.py
- slack/management/commands/fetch_slack_activity.py
- libraries/tests/test_commands.py
- users/urls.py
- users/forms.py
- gunicorn.conf.py
- users/tests/test_urls.py
- core/tests/test_managers.py
- news/acl.py
- mailing_list/tasks.py
- core/tests/test_tasks.py
✅ Files skipped from review due to trivial changes (8)
- news/constants.py
- marketing/tests.py
- versions/views.py
- libraries/tests/test_tasks.py
- mailing_list/management/commands/sync_mailinglist_stats.py
- users/views.py
- core/context_processors.py
- libraries/tests/fixtures.py
🚧 Files skipped from review as they are similar to previous changes (8)
- config/settings.py
- frontend/wysiwyg-editor.js
- news/tests/test_forms.py
- news/tasks.py
- templates/news/v3/create.html
- versions/tasks.py
- static/css/v3/create-post-page.css
- config/urls.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@news/forms.py`:
- Line 25: Replace the mutable list literals used for Meta.fields with immutable
tuples to satisfy linting and avoid mutable class attributes: locate each Meta
class in news/forms.py where you see Meta.fields = ["title", "publish_at",
"content", "image"] (and the two other Meta.fields occurrences around lines 46
and 52) and change the RHS from a list literal to a tuple literal (use
parentheses with the same string items). Ensure you update all three Meta.fields
occurrences consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 58611065-5015-41bf-bdad-69b006511be0
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
news/forms.pynews/tests/test_forms.pynews/views.py
🚧 Files skipped from review as they are similar to previous changes (1)
- news/views.py
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@news/forms.py`:
- Line 25: Replace the mutable list literals used for Meta.fields with immutable
tuples to satisfy linting and avoid mutable class attributes: locate each Meta
class in news/forms.py where you see Meta.fields = ["title", "publish_at",
"content", "image"] (and the two other Meta.fields occurrences around lines 46
and 52) and change the RHS from a list literal to a tuple literal (use
parentheses with the same string items). Ensure you update all three Meta.fields
occurrences consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 58611065-5015-41bf-bdad-69b006511be0
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
news/forms.pynews/tests/test_forms.pynews/views.py
🚧 Files skipped from review as they are similar to previous changes (1)
- news/views.py
🛑 Comments failed to post (1)
news/forms.py (1)
25-25:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse tuples for
Meta.fieldsto avoid mutable class attributes.Line 25, Line 46, and Line 52 currently use list literals for
Meta.fields, which triggers RuffRUF012. Switching to tuples removes the mutable class-attribute risk and aligns with lint expectations.Proposed fix
class BlogPostForm(EntryForm): class Meta: model = BlogPost - fields = ["title", "publish_at", "content", "image"] + fields = ("title", "publish_at", "content", "image") @@ class NewsForm(EntryForm): class Meta: model = News - fields = ["title", "publish_at", "content", "image"] + fields = ("title", "publish_at", "content", "image") @@ class V3BlogPostForm(BlogPostForm): class Meta(BlogPostForm.Meta): - fields = ["title", "publish_at", "content", "summary", "image"] + fields = ("title", "publish_at", "content", "summary", "image") @@ class V3NewsForm(NewsForm): class Meta(NewsForm.Meta): - fields = ["title", "publish_at", "content", "summary", "image"] + fields = ("title", "publish_at", "content", "summary", "image")Also applies to: 46-46, 52-52
🧰 Tools
🪛 Ruff (0.15.15)
[warning] 25-25: Mutable default value for class attribute
(RUF012)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@news/forms.py` at line 25, Replace the mutable list literals used for Meta.fields with immutable tuples to satisfy linting and avoid mutable class attributes: locate each Meta class in news/forms.py where you see Meta.fields = ["title", "publish_at", "content", "image"] (and the two other Meta.fields occurrences around lines 46 and 52) and change the RHS from a list literal to a tuple literal (use parentheses with the same string items). Ensure you update all three Meta.fields occurrences consistently.Source: Linters/SAST tools
…y task to avoid Gateway time out error
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
news/views.py (1)
528-533:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExtract the profile-completeness guard instead of calling
dispatch()directly.Line 530 already runs the full handler chain on
self. Any 200 response then falls through to Line 533 and dispatches again, so GETs render twice and 200 POST paths like invalid post types / invalid forms execute twice as well. That can duplicate messages and doubles the work on those requests.Proposed fix
class AllTypesCreateView(LoginRequiredMixin, TemplateView): template_name = "news/create.html" http_method_names = ["get"] # This is a "create news" multiplexer (by news type) + def _profile_completeness_response(self, request): + """User must have a profile photo and a name to post an entry.""" + if request.user.is_authenticated: + missing_data = [] + + if not request.user.display_name: + missing_data.append("your name") + + if missing_data: + messages.warning( + request, f"Please add {' and '.join(missing_data)} first." + ) + return redirect("profile-account") + return None + def dispatch(self, request, *args, **kwargs): - """User must have a profile photo and a name to post an entry.""" - if request.user.is_authenticated: - missing_data = [] - - if not request.user.display_name: - missing_data.append("your name") - - if missing_data: - messages.warning( - request, f"Please add {' and '.join(missing_data)} first." - ) - return redirect("profile-account") - + if response := self._profile_completeness_response(request): + return response return super().dispatch(request, *args, **kwargs) class V3AllTypesCreateView(V3Mixin, AllTypesCreateView): v3_template_name = "news/v3/create.html" http_method_names = ["get", "post"] def dispatch(self, request, *args, **kwargs): - # Run AllTypesCreateView's profile-completeness guard before V3Mixin takes over. - response = AllTypesCreateView.dispatch(self, request, *args, **kwargs) - if response.status_code != 200: - return response + if response := self._profile_completeness_response(request): + return response return super().dispatch(request, *args, **kwargs)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@news/views.py` around lines 528 - 533, The current override calls AllTypesCreateView.dispatch(self, request, ...) which runs the full handler chain on self and then calls super().dispatch again, causing handlers to run twice; instead extract the profile-completeness guard logic out of AllTypesCreateView.dispatch into a dedicated helper (e.g., AllTypesCreateView.profile_completeness_guard or ensure_profile_complete) and call that helper from this dispatch in news.views (returning its HttpResponse if present), then proceed to return super().dispatch(...) only when the guard returns None/OK; update AllTypesCreateView to expose the guard method and replace its internal usage to avoid duplicating dispatch calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@news/views.py`:
- Around line 528-533: The current override calls
AllTypesCreateView.dispatch(self, request, ...) which runs the full handler
chain on self and then calls super().dispatch again, causing handlers to run
twice; instead extract the profile-completeness guard logic out of
AllTypesCreateView.dispatch into a dedicated helper (e.g.,
AllTypesCreateView.profile_completeness_guard or ensure_profile_complete) and
call that helper from this dispatch in news.views (returning its HttpResponse if
present), then proceed to return super().dispatch(...) only when the guard
returns None/OK; update AllTypesCreateView to expose the guard method and
replace its internal usage to avoid duplicating dispatch calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 285def1b-7fb0-496e-abf8-390af2af366f
📒 Files selected for processing (2)
news/tasks.pynews/views.py
🚧 Files skipped from review as they are similar to previous changes (1)
- news/tasks.py
5 participants
Issue: #2428
Summary & Context
Adds AI-assisted Description to the v3 create-post page for Blog and News posts.
A 1,000 character Description field with a live counter, an Auto-Generate Description button that calls the existing summarization pipeline synchronously, per-post-type draft persistence in
localStoragefor both Content and Description, and a Saving / Saved indicator that announces state changes to assistive tech. User-provided descriptions are persisted to the existingEntry.summaryfield and short-circuit the model's background auto-generation.Changes
Frontend (
templates/news/v3/create.html,static/css/v3/create-post-page.css,templates/includes/icon.html)text/primary, tracking −0.12px, line-height 1.2.:disabledbound togeneratingstate.7134-42553).savedicon added toicon.html) under both Content and Description. Spinner uses CSS keyframes and respectsprefers-reduced-motion.localStorage-backed draft persistence keyed per post type:boost:createPost:contentDraft:<postType>for Content (markdown).boost:createPost:descriptionDraft:<postType>for Description (plain text).form.summary.value), and clears both on a validated submit.wysiwyg-updatecustom event (carries plain-text character count, markdown value, and aprogrammaticflag) and awysiwyg-set-contentwindow listener so the host page can restore drafts into the editor.resize: verticalfrom.wysiwyg-editor__bodyonto the inner.wysiwyg-editor__prose(scoped to.create-post-pageso other pages keep their existing behavior).--color-text-primary/secondary/tertiary,--color-surface-weak,--color-stroke-weak) that are already redefined underhtml.darkinthemes.css.role="status" aria-live="polite"on both Saving/Saved indicators and on the generating placeholder;<label for>on every field; char counters markedaria-hidden="true";:aria-invalid/:aria-describedbyon Content; liverole="alert"error<p>s on each field for client validation.--space-sinstead of a hardcoded4px,--line-height-relaxedinstead of1.24).Backend (
news/views.py,news/forms.py,news/tasks.py,news/constants.py,config/urls.py)POST /v3/news/generate-description/(name="v3-news-generate-description"): callssummarize_contentsynchronously, capped at the newDESCRIPTION_SUMMARY_MAX_LENGTH = 970(under the 1,000-char field cap for model leeway), and returns{"description": "..."}. Decorated with@require_POST. Intentionally not yet@login_required— see Risks.summarize_contentnow accepts amax_lengthparameter (default256preserved for existing callers).summarytoBlogPostForm.Meta.fieldsandNewsForm.Meta.fields.V3AllTypesCreateView.post()remaps the posteddescriptionfield tosummarybefore instantiating the form, so the frontend wiring staysname="description"while the model field stayssummary(no migration needed).Tooling / CI (
.pre-commit-config.yaml)static/js/v3/wysiwyg-editor.jsandstatic/js/v3/fuse.min.jsfrom the pre-commit hooks. Thetrailing-whitespacehook was modifying these and risked corrupting minified string/regex content inside them; bundles are now treated as generated artifacts.Tests
test_blogpost_formandtest_news_forminnews/tests/test_forms.pyto expect the newsummaryfield in each form'sMeta.fields.I did not modify the summariser prompt and overall functionality and parameters (temperature, top_k, top_p, etc.) because didn't want to alter the background process which is carried out when the user does not provide a description, but it's worth taking experimentation into account when deciding the token allocation schema and machine learning testing pipeline. As a whole, the prompt works well, but currently there's no way to guarantee it'll always comply with the
max_lengthparameter or if the generated description is actually good. A testing procedure is currently being discussed with @herzog0, to be implemented if deemed appropriate by the team.generate-descriptionendpoint is not yet login-gated. Left@login_requiredoff for local testing (view docstring flags this). Before deploy: add@login_requiredand consider rate-limiting — every call hits OpenRouter and costs API budget. The endpoint is an open, unauthenticated AI proxy until then.Description is posted as
descriptionbut persisted assummaryvia a view-level remap. Deliberately unusual: we kept the existing frontend naming (Alpine state, x-model, localStorage key, char counter binding) untouched. The single point of translation is a three-linerequest.POST.copy()block inV3AllTypesCreateView.post()with a comment explaining why.Shared WYSIWYG module (
frontend/wysiwyg-editor.js) gained an event bridge. Added aneditor.on("update")listener that dispatcheswysiwyg-updateon every change (computesturndown(editor.getHTML())per dispatch), plus awindow-levelwysiwyg-set-contentlistener for restore. These are opt-in (no-ops if no host page listens / dispatches), but they execute for every editor instance globally. Worth a sanity check from someone familiar with the editor.Resize grip alignment CSS is scoped to
.create-post-pagebecause it moves the resize target from.wysiwyg-editor__bodyonto.wysiwyg-editor__prose. Other pages embedding the editor keep their existing grip placement.Entry.save()auto-dispatchessummary_dispatcheronly whensummaryis empty — unchanged. The new flow means a user-typed (or auto-generated) description now populatessummaryon submit, which short-circuits the background task. Ifsummaryis blank on submit, the background generation runs exactly as before.Dev-only quirk: with DevTools "Disable cache" on a cold network, the 670 KB editor bundle can be slow enough that clicking Submit before the editor finishes initializing leaves the hidden
#field-contenttextarea empty → client validation rejects with "Content is required." Production caches the bundle normally; not an issue post-deploy.Pre-existing hardcoded values in
create-post-page.css(the title/subtitleline-height: 1/1.33and the cardborder-radius: 16px) were flagged during the token audit but left untouched — they come from upstream commit26e8902f(Story Webpage UI: Create a Post #2111) and are out of scope for this PR.Screenshots
Responsiveness and Layout
responsiveness.mov
Localstorage Persistence
localstorage.mov
Auto Generation Functionality
auto-generated-summary.mov
Self-review Checklist
Frontend
Summary by CodeRabbit
New Features
Chores