Speed up comment creation flow#100
Conversation
Comment creation in production took ~460-830ms with only ~1ms in views and ~200ms in DB. The remaining time was synchronous Ruby work blocking the request: - Notifications were created and badge broadcasts pushed inline. - Each thread popover broadcast re-parsed the entire plan markdown via CommentThread#anchor_occurrence_index -> strip_markdown. - The comment partial fired a per-comment User.find_by lookup, and Comment#first_comment_in_thread? did an order+first scan on every insert. Changes: - Move Notifications::Create off the request path via the new CreateNotificationsJob (controllers + API). - Memoize Plan#stripped_content so repeated thread popover renders share one parse of the plan markdown. - Batch unread badge counts into a single grouped query in Notifications::Create#broadcast_badge_updates. - Extract Comment#author so the helper layer no longer issues queries directly; helper just memoizes the lookup per request. - Replace Comment#first_comment_in_thread? with a race-safe 'no comments with smaller id' check (UUID v7 IDs are sortable). - Wrap render_markdown in a fragment cache keyed by the comment record. Amp-Thread-ID: https://ampcode.com/threads/T-019dcf4e-942f-779b-9f51-12a2fcfb8e1d Co-authored-by: Amp <amp@ampcode.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc5c27e2ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| def first_comment_in_thread? | ||
| self == comment_thread.comments.order(:created_at).first | ||
| !comment_thread.comments.where("id < ?", id).exists? |
There was a problem hiding this comment.
The new id < ? check assumes UUIDv7 values are strictly insertion-ordered, but they are only time-ordered at coarse granularity and can be out of order across concurrent workers. When two comments are created close together, a later insert can still have a smaller UUID, causing first_comment_in_thread? to return true for more than one comment and enqueue duplicate “first comment” notifications. This path is triggered by after_create_commit, so it affects production concurrency; use a persisted ordering (e.g., created_at with an ID tie-breaker) or a uniqueness guard for the first-notification behavior.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
yeah, tihs seems like we should WRITE DOWN which is the parent? or actually- why does this matter at all?
1 participant
Comment creation in production took ~460-830ms with only ~1ms in views and ~200ms in DB. Production trace from CDN-50b8c3ab-161c-432f-8066-f667fe33255a:
That ~275ms of unaccounted-for Ruby was synchronous work blocking the request.
What was slow
CommentThread#anchor_occurrence_index->strip_markdown. For a plan with N threads, that's N full parses.User.find_bylookup.Comment#first_comment_in_thread?did anorder(:created_at).firstscan on every comment insert.Changes
Notifications::Createoff the request path via the newCreateNotificationsJob(web + API controllers).Plan#stripped_contentso repeated thread popover renders share one parse of the plan markdown.Notifications::Create#broadcast_badge_updates(was N COUNT queries).Comment#authorso the helper layer no longer issues queries directly; the helper just memoizes the lookup per request to dedupe across multiple comments by the same author.Comment#first_comment_in_thread?with a race-safeno comments with smaller idcheck (UUID v7 IDs are sortable).render_markdownin a fragment cache keyed by the comment record (comment bodies are immutable).Tests
cc @codex please review