Skip to content

Speed up comment creation flow #100

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.

Sign up for GitHub

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

Merged
HamptonMakes merged 1 commit into from
Apr 27, 2026
Merged

Speed up comment creation flow #100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions engine/app/controllers/coplan/api/v1/comments_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ def create
)

reason = comment.agent? ? "agent_response" : "new_comment"
Notifications::Create.call(
comment_thread: thread,
CreateNotificationsJob.perform_later(
comment_thread_id: thread.id,
actor_id: api_actor_id,
comment: comment,
comment_id: comment.id,
reason: reason
)

Expand Down Expand Up @@ -59,7 +59,7 @@ def resolve
end

thread.resolve!(current_user)
Notifications::Create.call(comment_thread: thread, actor_id: current_user.id, reason: "status_change")
CreateNotificationsJob.perform_later(comment_thread_id: thread.id, actor_id: current_user.id, reason: "status_change")
broadcast_thread_update(thread)

render json: { thread_id: thread.id, status: thread.status }
Expand All @@ -79,7 +79,7 @@ def discard
end

thread.discard!(current_user)
Notifications::Create.call(comment_thread: thread, actor_id: current_user.id, reason: "status_change")
CreateNotificationsJob.perform_later(comment_thread_id: thread.id, actor_id: current_user.id, reason: "status_change")
broadcast_thread_update(thread)

render json: { thread_id: thread.id, status: thread.status }
Expand All @@ -100,10 +100,10 @@ def reply
)

reason = comment.agent? ? "agent_response" : "reply"
Notifications::Create.call(
comment_thread: thread,
CreateNotificationsJob.perform_later(
comment_thread_id: thread.id,
actor_id: api_actor_id,
comment: comment,
comment_id: comment.id,
reason: reason
)

Expand Down
14 changes: 7 additions & 7 deletions engine/app/controllers/coplan/comment_threads_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ def create
body_markdown: params[:comment_thread][:body_markdown]
)

Notifications::Create.call(
comment_thread: thread,
CreateNotificationsJob.perform_later(
comment_thread_id: thread.id,
actor_id: current_user.id,
comment: comment,
comment_id: comment.id,
reason: "new_comment"
)

Expand All @@ -53,31 +53,31 @@ def create
def resolve
authorize!(@thread, :resolve?)
@thread.resolve!(current_user)
Notifications::Create.call(comment_thread: @thread, actor_id: current_user.id, reason: "status_change")
CreateNotificationsJob.perform_later(comment_thread_id: @thread.id, actor_id: current_user.id, reason: "status_change")
broadcast_thread_replace(@thread)
respond_with_stream_or_redirect("Thread resolved.")
end

def accept
authorize!(@thread, :accept?)
@thread.accept!(current_user)
Notifications::Create.call(comment_thread: @thread, actor_id: current_user.id, reason: "status_change")
CreateNotificationsJob.perform_later(comment_thread_id: @thread.id, actor_id: current_user.id, reason: "status_change")
broadcast_thread_replace(@thread)
respond_with_stream_or_redirect("Thread accepted.")
end

def discard
authorize!(@thread, :discard?)
@thread.discard!(current_user)
Notifications::Create.call(comment_thread: @thread, actor_id: current_user.id, reason: "status_change")
CreateNotificationsJob.perform_later(comment_thread_id: @thread.id, actor_id: current_user.id, reason: "status_change")
broadcast_thread_replace(@thread)
respond_with_stream_or_redirect("Thread discarded.")
end

def reopen
authorize!(@thread, :reopen?)
@thread.update!(status: "pending", resolved_by_user: nil)
Notifications::Create.call(comment_thread: @thread, actor_id: current_user.id, reason: "status_change")
CreateNotificationsJob.perform_later(comment_thread_id: @thread.id, actor_id: current_user.id, reason: "status_change")
broadcast_thread_replace(@thread)
respond_with_stream_or_redirect("Thread reopened.")
end
Expand Down
6 changes: 3 additions & 3 deletions engine/app/controllers/coplan/comments_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ def create
body_markdown: params[:comment][:body_markdown]
)

Notifications::Create.call(
comment_thread: @thread,
CreateNotificationsJob.perform_later(
comment_thread_id: @thread.id,
actor_id: current_user.id,
comment: comment,
comment_id: comment.id,
reason: "reply"
)

Expand Down
14 changes: 4 additions & 10 deletions engine/app/helpers/coplan/comments_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,10 @@ def comment_author_name(comment)
end

def comment_author_user(comment)
case comment.author_type
when "human"
CoPlan::User.find_by(id: comment.author_id)
when "local_agent"
CoPlan::User
.joins(:api_tokens)
.where(coplan_api_tokens: { id: comment.author_id })
.first
else
nil
@_comment_author_cache ||= {}
cache_key = "#{comment.author_type}:#{comment.author_id}"
@_comment_author_cache.fetch(cache_key) do
@_comment_author_cache[cache_key] = comment.author
end
end
end
Expand Down
16 changes: 16 additions & 0 deletions engine/app/jobs/coplan/create_notifications_job.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
module CoPlan
class CreateNotificationsJob < ApplicationJob
queue_as :default

def perform(comment_thread_id:, actor_id:, comment_id: nil, reason:)
thread = CommentThread.find(comment_thread_id)
comment = comment_id ? Comment.find(comment_id) : nil
Notifications::Create.call(
comment_thread: thread,
actor_id: actor_id,
comment: comment,
reason: reason
)
end
end
end
13 changes: 12 additions & 1 deletion engine/app/models/coplan/comment.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,21 @@ def agent?
agent_name.present? || author_type.in?(%w[local_agent cloud_persona])
end

# Resolves the comment author to a CoPlan::User instance, or nil for
# author types that don't map to a user (cloud_persona, system).
def author
case author_type
when "human"
CoPlan::User.find_by(id: author_id)
when "local_agent"
CoPlan::User.joins(:api_tokens).where(coplan_api_tokens: { id: author_id }).first
end
end

private

def first_comment_in_thread?
self == comment_thread.comments.order(:created_at).first
!comment_thread.comments.where("id < ?", id).exists?
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Detect first thread comment with persisted timestamp order

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 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

@HamptonMakes HamptonMakes Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, tihs seems like we should WRITE DOWN which is the parent? or actually- why does this matter at all?

end

def notify_plan_author
Expand Down
2 changes: 1 addition & 1 deletion engine/app/models/coplan/comment_thread.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ def anchor_occurrence_index

# When anchor_start is known, count occurrences before it.
if anchor_start.present?
stripped, pos_map = self.class.strip_markdown(content)
stripped, pos_map = plan.stripped_content
# Map raw anchor_start to its position in the stripped string.
# Use >= to find the closest valid position if anchor_start falls
# on a stripped formatting character.
Expand Down
10 changes: 10 additions & 0 deletions engine/app/models/coplan/plan.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,16 @@ def current_content
current_plan_version&.content_markdown
end

# Memoized stripped-markdown + position map for the current content.
# Reused by multiple CommentThread#anchor_occurrence_index calls within
# the same request to avoid re-parsing the full plan for each thread.
def stripped_content
@stripped_content ||= begin
content = current_content
content.present? ? Plans::MarkdownTextExtractor.call(content) : [+"", []]
end
end

def tag_names
tags.pluck(:name)
end
Expand Down
13 changes: 6 additions & 7 deletions engine/app/services/coplan/notifications/create.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def call
subscriber_ids.delete(@actor_id)
return if subscriber_ids.empty?

notifications = subscriber_ids.map do |user_id|
subscriber_ids.each do |user_id|
Notification.create!(
user_id: user_id,
plan_id: @comment_thread.plan_id,
Expand All @@ -27,8 +27,7 @@ def call
)
end

broadcast_badge_updates(notifications)
notifications
broadcast_badge_updates(subscriber_ids)
end

private
Expand Down Expand Up @@ -75,13 +74,13 @@ def thread_participant_ids
ids
end

def broadcast_badge_updates(notifications)
notifications.group_by(&:user_id).each do |user_id, _|
count = Notification.where(user_id: user_id).unread.count
def broadcast_badge_updates(subscriber_ids)
counts = Notification.where(user_id: subscriber_ids).unread.group(:user_id).count
subscriber_ids.each do |user_id|
Broadcaster.update_to(
"coplan_notifications:#{user_id}",
target: "inbox-badge",
html: count.to_s
html: (counts[user_id] || 0).to_s
)
end
end
Expand Down
4 changes: 3 additions & 1 deletion engine/app/views/coplan/comments/_comment.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
· <%= time_ago_in_words(comment.created_at) %> ago
</div>
<div class="comment__body">
<%= render_markdown(comment.body_markdown) %>
<%= cache(comment) do %>
<%= render_markdown(comment.body_markdown) %>
<% end %>
</div>
</div>
Loading