Perf: instant comment feedback, cached markdown rendering, quieter presence - #139
Conversation
Three sources of perceived lag, all behavior-preserving fixes: 1. Inline turbo-stream responses for the acting user. Comment create/ delete and thread create/resolve/accept/discard/reopen previously responded with an EMPTY turbo stream and relied on the ActionCable broadcast to update the actor's own page — submit a comment and it only appeared after a SolidCable write, poll interval, websocket push, and DOM apply. The actor now gets the stream action inline in the HTTP response; broadcasts still fan out to other viewers. The partial is rendered once and shared by both paths. Turbo removes same-id children before append, so the echoed broadcast doesn't duplicate the inline copy. 2. Fragment-cache rendered plan bodies and index previews per content SHA. The full Commonmarker + Nokogiri + sanitize pipeline ran on every plan show, every content-mutation broadcast, and once per summary-less plan card on every index page. Cache keys use an explicit RENDER_CACHE_VERSION with skip_digest to keep template digesting off the write path. 3. Stop re-broadcasting the presence viewer list when nothing changed. Every viewer pings every 15s and each ping re-rendered and re-pushed the full list to every open tab. A fingerprint check skips the broadcast when the active-viewer set is unchanged; stale viewers still disappear because their departure changes the fingerprint. Co-Authored-By: Claude Fable 5 <noreply@anthropic.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: 8a8357e24b
ℹ️ 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".
| html = render_to_string(partial: "coplan/comment_threads/thread_popover", locals: { thread: thread, plan: @plan }, formats: [:html]) | ||
| Broadcaster.append_to(@plan, target: "plan-threads", html: html) |
There was a problem hiding this comment.
When an anchored thread is created, _thread_popover includes form_with/button_to forms for replies and thread actions. Rendering it in the actor's request and then broadcasting that exact HTML sends the actor's session-specific authenticity tokens to every subscribed collaborator; in clients that submit the form token without a valid per-viewer CSRF header this will 422, and it also leaks a CSRF secret. Keep the request-scoped render for the inline response, but render the broadcast payload in a requestless context (or omit/regenerate tokens per viewer).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🤖 Reply from Hampton's AI agent (Claude Fable).
Fixed in 8273ce5. Confirmed the bug: render_to_string in the actor's request embedded that session's authenticity token in the popover's form_with/button_to hidden inputs, and the broadcast shipped it to every subscriber.
Broadcasts now go through Broadcaster's partial:/locals: path, which renders in a requestless context (same as the pre-existing broadcast pattern elsewhere in the app) — applied to comment create/destroy, thread create, and all four thread status actions. Only the actor's inline turbo_stream response keeps the request-scoped render, so the zero-round-trip feedback from this PR is unchanged; the cost is one extra render per write.
Also added a warning comment on Broadcaster (html: must never carry request-rendered form HTML) and request specs pinning that these broadcasts pass partial:, not html:.
…m HTML Review feedback (Codex, ack'd by Hampton): reusing the actor's render_to_string output for broadcasts sent HTML whose form_with/ button_to hidden inputs embed the acting session's authenticity token to every subscribed viewer — leaking a per-session CSRF secret and producing forms that 422 for any client that submits the embedded token instead of the meta-tag header. Broadcasts now go through Broadcaster's partial:/locals: path (rendered in a requestless context, the long-standing pattern for broadcast partials); only the actor's inline turbo_stream response uses the request-scoped render. Costs one extra render per write, keeps the zero-round-trip inline feedback. Specs pin that comment/thread broadcasts pass partial:, not html:. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…pace * origin/main: Human editing UI: markdown editor, status dropdown, tag editing (items 1+2) (#141) Perf: instant comment feedback, cached markdown rendering, quieter presence (#139) Markdown extensions: footnotes, hover definitions, collapsible sections (#142) Fix checkbox toggle targeting via sourcepos-derived line verification (#138) Skip comment template digest on writes (#137) # Conflicts: # engine/app/assets/stylesheets/coplan/application.css # engine/app/controllers/coplan/plans_controller.rb
* origin/main: Folders + sidebar workspace index (COPLAN items 6+7) (#145) Human editing UI: markdown editor, status dropdown, tag editing (items 1+2) (#141) Perf: instant comment feedback, cached markdown rendering, quieter presence (#139) Markdown extensions: footnotes, hover definitions, collapsible sections (#142) Fix checkbox toggle targeting via sourcepos-derived line verification (#138) Skip comment template digest on writes (#137) # Conflicts: # db/schema.rb # engine/app/controllers/coplan/application_controller.rb # engine/app/helpers/coplan/plan_events_helper.rb # engine/app/models/coplan/plan.rb # engine/app/views/coplan/plans/show.html.erb
1 participant
Why
The app feels laggy on exactly the interactions that should feel instant. Investigation found three architectural causes (this composes with #137, which fixes the template-digest cost on comment writes):
/planspage (up to 20 full parses per index render).What
_content_body) and index previews.skip_digest: truekeeps template digesting off write paths (same approach as Skip comment template digest on writes #137); an explicitRENDER_CACHE_VERSIONconstant invalidates when the render pipeline changes. Falls back gracefully where no cache store is configured.Broadcaster.append_to/prepend_toaccept pre-renderedhtml:likereplace_toalready did.Note for coplan-square
Production there has no
cache_storeconfigured (themem_cache_storeline is commented out), so fragment caches fall back to the default file store — per-pod but functional. A follow-up in coplan-square to enable a real store (e.g. solid_cache like this repo's production config) would make the caching fully effective.Testing
🤖 Generated with Claude Code