[Pro] Reduce tag-index cache work during streaming

[justin808]

Summary:

• batch tag-index reads for multi-tag registration with read_multi when available
• defer streamed cache writes until after queued stream chunks drain
• use the same completion-time cache options for writes and tag registration

Tests:

• bundle exec rspec spec/react_on_rails_pro/cache/tag_index_spec.rb spec/react_on_rails_pro/cache_spec.rb
• cd react_on_rails_pro/spec/dummy && bundle exec rspec spec/helpers/react_on_rails_pro_helper_spec.rb:907 spec/helpers/react_on_rails_pro_helper_spec.rb:2253
• bundle exec rubocop
• git diff --check

Fixes #4319

Summary by CodeRabbit

• Bug Fixes
  • Streamed/async React caching now defers cache writes and normalized cache tag registration until queued response chunks finish draining, using completion-time TTL/options.
  • Misses that are already expired are skipped, and deferred cache write failures after draining are handled safely with a warning (no crash).
• Performance
  • Tag-index updates are more efficient by batching existing index reads when registering multiple tags.
• Tests
  • Expanded tests for streamed/async cache write timing, batching behavior, TTL handling, and failure scenarios.

[coderabbitai]

Warning

Review limit reached

@justin808, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 8 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4639abcb-9b82-4e6d-9101-86ff72c464f0

📥 Commits

Reviewing files that changed from the base of the PR and between c73e044 and 31aa0df.

📒 Files selected for processing (2)

• react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rb
• react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb

Walkthrough

This PR defers streamed cache writes until streaming completes, uses write-time cache options for async cache tagging, and batches tag-index reads and updates across multiple tags.

Changes

Deferred cache writes and batched tag indexing

Layer / File(s)	Summary
Defer and flush streamed cache writes react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb, react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rb, react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb	handle_stream_cache_miss queues pending cache-write payloads, streaming flushes them after drain or on error, and specs cover deferred writes, failures, stale entries, and immediate writes when no flush is active.
Use write-time cache options for async rendering react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb, react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb	Async rendering stops passing miss-time cache options through, and tag registration now uses the cache options computed at write time.
Batch tag-index reads and writes react_on_rails_pro/lib/react_on_rails_pro/cache/tag_index.rb, react_on_rails_pro/spec/react_on_rails_pro/cache/tag_index_spec.rb, CHANGELOG.md	TagIndex batches multi-tag index reads, updates stored key lists and expiry per tag, and adds coverage plus a changelog note.

Estimated code review effort: 4 (Complex) | ~45 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title concisely reflects the main change to reduce streaming tag-index cache work.
Linked Issues check	✅ Passed	The PR batches tag-index work, defers streamed cache writes, aligns completion-time cache options, and adds the requested concurrency and TTL coverage.
Out of Scope Changes check	✅ Passed	The changes stay focused on streamed cache-write batching, TTL consistency, and tests, with only related changelog updates.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch jg-codex/batch-c-pro-cache-4319

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

This PR reduces cache work during React on Rails Pro streaming by batching tag-index reads and deferring stream cache writes until after all queued chunks are drained. The async (non-streaming) path is also simplified to use a single set of write options for both the cache entry and tag registration instead of computing options twice at miss time and completion time.

• tag_index.rb: Replaces per-tag append_entry_key calls with a single read_multi batch read across all tags before writing each updated index entry, saving N−1 cache round-trips for N tags on stores that support read_multi.
• stream.rb: Introduces StreamCacheWrites module; on_complete now enqueues write metadata rather than writing immediately, and flush_pending_stream_cache_writes processes all deferred writes after drain_streams_concurrently completes.
• react_on_rails_pro_helper.rb: Removes the cache_options_at_miss argument from the async write helper; both the cache write and tag registration now share the single completion-time options, closing the bug in [Pro] Per-tag index registration adds blocking Rails.cache round-trips inside the streaming reactor; index/entry TTLs stamped from different clocks #4319.

Confidence Score: 3/5

Safe to merge after addressing the unclosed stream risk: a cache store error during flush skips response.stream.close and hangs the client connection.

The batch read_multi and deferred-write changes are well-structured and tested. The single actionable concern is that flush_pending_stream_cache_writes sits between drain_streams_concurrently and response.stream.close with no local rescue. A cache-store exception at flush time (Redis timeout, Memcache gone) re-raises through the rescue StandardError handler, skipping stream closure and leaving the HTTP connection open on the client side. This is a new failure mode not present before this PR.

react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rb — specifically the placement of flush_pending_stream_cache_writes relative to response.stream.close and the lack of error isolation around the flush call.

Important Files Changed

Filename	Overview
react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rb	Adds StreamCacheWrites module and defers all stream cache writes to after drain_streams_concurrently; a cache-store exception in flush_pending_stream_cache_writes will skip response.stream.close.
react_on_rails_pro/lib/react_on_rails_pro/cache/tag_index.rb	Converts per-tag append_entry_key to batch append_entry_keys with read_multi, extracting parse_index_payload for reuse; logic is correct and well-guarded.
react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb	Removes redundant miss-time cache_options_at_miss parameter from async path and pushes stream cache writes to the pending queue instead of writing inline inside on_complete.
react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb	Tests updated to match deferred-write semantics; tag_index_cache_options variable now has a misleading name in the async tag test.
react_on_rails_pro/spec/react_on_rails_pro/cache/tag_index_spec.rb	Adds a targeted spec confirming read_multi is called once for multi-tag registration; straightforward and correct.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant C as Controller
    participant S as Stream module
    participant H as Helper (on_complete)
    participant Q as Pending Writes Queue
    participant SC as StreamCacheWrites
    participant RC as Rails.cache

    C->>S: stream_view_containing_react_components
    S->>S: "init pending_stream_cache_writes = []"
    S->>S: drain_streams_concurrently
    loop per component chunk
        H->>H: on_complete callback fires
        H->>Q: "push {cache_key, chunks, tags, raw_options}"
    end
    S->>SC: flush_pending_stream_cache_writes
    loop per pending write
        SC->>RC: cache_write_expired? check
        SC->>RC: cache_write_options (computed at flush time)
        SC->>RC: write(cache_key, chunks, cache_options)
        SC->>RC: register_normalized_tags(tags, cache_key, cache_options)
    end
    S->>S: response.stream.close

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant C as Controller
    participant S as Stream module
    participant H as Helper (on_complete)
    participant Q as Pending Writes Queue
    participant SC as StreamCacheWrites
    participant RC as Rails.cache

    C->>S: stream_view_containing_react_components
    S->>S: "init pending_stream_cache_writes = []"
    S->>S: drain_streams_concurrently
    loop per component chunk
        H->>H: on_complete callback fires
        H->>Q: "push {cache_key, chunks, tags, raw_options}"
    end
    S->>SC: flush_pending_stream_cache_writes
    loop per pending write
        SC->>RC: cache_write_expired? check
        SC->>RC: cache_write_options (computed at flush time)
        SC->>RC: write(cache_key, chunks, cache_options)
        SC->>RC: register_normalized_tags(tags, cache_key, cache_options)
    end
    S->>S: response.stream.close

Loading

Comments Outside Diff (1)

1. react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rb, line 108-112 (link)

   Cache error skips response.stream.close

   flush_pending_stream_cache_writes calls Rails.cache.write for each deferred entry. If the cache store raises (e.g., Redis connection timeout, Memcached gone), the exception propagates through the rescue StandardError handler, which re-raises — skipping response.stream.close if close_stream_at_end on line 112. The HTTP response body is complete at that point but the connection is never terminated, leaving clients hanging.

   The comment on line 109 intentionally excludes response.stream.close from the ensure block (so error handlers can still write to the stream after render failures). But that reasoning does not apply here: cache write failures happen after streaming is fully done, and the stream close must still happen. A narrow rescue around the flush — logging the error and not re-raising — would keep the close unconditional without changing the existing error-propagation contract for render errors.

_{Reviews (1): Last reviewed commit: "Document Pro tag-index cache improvement..." | Re-trigger Greptile}

[justin808]

+ci-run-hosted

[claude]

Review summary

Solid PR overall — the tag-index read_multi batching and unifying the cache-options used for the write vs. tag registration (previously computed at two different times, which could diverge) are both genuine improvements, and the accompanying tests are well-targeted.

One correctness concern flagged inline in react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rb: deferring all stream cache writes to a single end-of-response flush (instead of writing each component's cache entry the moment that component's own stream completes, as before) means an unrelated component's post-first-chunk failure now causes the whole batch — including already-successfully-streamed components' cache writes and tag registrations — to be silently discarded. Since each view_cache_key is scoped per-component, this looks like an unintended side effect of the batching rather than an intentional trade-off. A related, smaller issue: the flush loop itself has no per-entry error isolation, so one flaky Rails.cache.write call drops every other still-pending entry in the same batch too.

Nothing else stood out from a security or performance standpoint — the read_multi change is a clear win for multi-tag registration, and the keys.one? fallback to the single-key path is reasonable.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2e197c1d8b

ℹ️ 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".

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rb (1)

106-125: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Keep cache flush from blocking stream close. If flush_pending_stream_cache_writes raises here, response.stream.close if close_stream_at_end is skipped and the stream can remain open. Wrap the flush in a best-effort rescue so shutdown still runs.

🤖 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 `@react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rb` around lines
106 - 125, flush_pending_stream_cache_writes in Stream#drain_streams should not
prevent response.stream.close from running when close_stream_at_end is set; if
the flush raises, the current rescue path skips stream shutdown and can leave
the response open. Update the shutdown flow in
react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rb so the cache flush
is best-effort and failures are rescued/logged locally before the close call,
while preserving the existing drain_streams_concurrently, `@async_barrier`&.stop,
and ensure cleanup behavior.

🤖 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 `@react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rb`:
- Around line 106-125: flush_pending_stream_cache_writes in Stream#drain_streams
should not prevent response.stream.close from running when close_stream_at_end
is set; if the flush raises, the current rescue path skips stream shutdown and
can leave the response open. Update the shutdown flow in
react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rb so the cache flush
is best-effort and failures are rescued/logged locally before the close call,
while preserving the existing drain_streams_concurrently, `@async_barrier`&.stop,
and ensure cleanup behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7c482a84-f79c-4841-804c-6e62a867423f

📥 Commits

Reviewing files that changed from the base of the PR and between ca28fe8 and 2e197c1.

📒 Files selected for processing (5)

• react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb
• react_on_rails_pro/lib/react_on_rails_pro/cache/tag_index.rb
• react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rb
• react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
• react_on_rails_pro/spec/react_on_rails_pro/cache/tag_index_spec.rb

[github-actions]

Hosted CI Requested

Triggered 9 workflow(s) for b48626b0703b.
Mode: optimized hosted CI (path-selected by script/ci-changes-detector).
Added ready-for-hosted-ci, so future commits will keep running optimized hosted CI until +ci-stop-hosted is used.

View progress in the Actions tab.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b48626b070

ℹ️ 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".

[claude]

Review summary

Reviewed the tag-index/stream cache-write changes in this PR. Overall the direction is good (batching tag-index reads with read_multi, aligning write/tag-registration options, deferring stream cache writes until after drain), and the doc comments are updated to match. One correctness issue found; left as an inline comment on react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rb:

Bug: ReactOnRailsPro::StreamCacheWrites.flush wraps its entire Array(pending_writes).each loop in one rescue StandardError, rather than rescuing per-entry. If a page streams multiple cached components and the first queued Rails.cache.write/register_normalized_tags call raises (e.g. a transient cache-backend blip), every subsequent component's cache write/tag registration in that same response is silently skipped — even though those components rendered successfully and are otherwise cacheable. Previously each component wrote to cache independently at its own on_complete callback, so this batching regresses that failure isolation. Suggest moving the rescue inside the loop (one write failure shouldn't cost every other write in the same response), and adding a spec with 2+ queued entries where the first raises to confirm the rest still get written.

Minor/non-blocking consideration: flush_pending_stream_cache_writes now runs synchronously between write_rsc_stream_observability_mark and response.stream.close. Since all response chunks are already flushed to the client by that point, this holds the ActionController::Live connection/thread open for the duration of every queued cache write before the socket closes — worth confirming that added tail latency before close is intentional given the PR's goal of reducing cache work.

Everything else looked solid:

• read_indexes batching in tag_index.rb correctly preserves per-tag TTL/dedup semantics; the keys.one? / respond_to?(:read_multi) fallback is safe since a plain read_index per key gives the same result.
• Recomputing cache_write_options at actual write/flush time instead of at miss/completion time is a genuine improvement — it aligns the effective TTL with the moment the entry is actually written rather than a stale snapshot, and the updated specs correctly reflect the new call counts.
• The stream cache-write failure path (rescue + log_failure, with its own internal rescue) correctly keeps the response stream from staying open on a cache failure, and the new spec covers this for the single-entry case.

[claude]

Review

This PR (1) batches tag-index reads across multiple tags via read_multi, (2) defers streamed cache writes + tag registration until after the response stream drains, and (3) makes async/stream cache-miss tag registration use the same completion-time cache options as the write (fixing the TTL-snapshot mismatch from #4319). The tag_index.rb batching change and completion-time-options fix both look correct and are well covered by tests.

Found two related correctness issues in the new deferred-flush mechanism in react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rb:

1. StreamCacheWrites.flush aborts all remaining writes if one fails (react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rb:23-38)

def flush(pending_writes)
  Array(pending_writes).each do |cache_write|
    ...
    Rails.cache.write(cache_write[:cache_key], cache_write[:chunks], cache_options)
    ReactOnRailsPro::Cache.register_normalized_tags(...)
  end
rescue StandardError => e
  log_failure(e)
end

The rescue wraps the entire each loop rather than each iteration. If a page streams 3 cached components and Rails.cache.write raises for the 2nd entry (e.g. a transient cache-store error, oversized payload, etc.), the exception unwinds the loop immediately — the 3rd entry is never attempted and never cached, even though it is logically independent of the 2nd. Before this PR, each component wrote to cache directly from its own on_complete callback, so one component's write failure had no effect on another's. Consider rescuing per-entry so one failure doesn't silently drop unrelated cache writes for the rest of the page.

The existing spec ("closes the stream when a deferred stream cache write fails after drain") only covers a single pending write failing — it doesn't catch this multi-entry partial-failure regression.

2. A non-disconnect error during streaming skips the flush entirely, losing all completed components' cache writes (react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rb:117-131)

drain_streams_concurrently(parent_task)
write_rsc_stream_observability_mark
flush_pending_stream_cache_writes
...
rescue StandardError
  @async_barrier&.stop
  raise
end

flush_pending_stream_cache_writes only runs on the happy path, after drain_streams_concurrently returns. That method only swallows client-disconnect errors (IOError/EPIPE/ECONNRESET/ECONNABORTED) internally; any other StandardError from a producer propagates through @async_barrier.wait's raise e, hits the outer rescue StandardError, and re-raises without ever calling flush_pending_stream_cache_writes. So if component A finishes successfully (queuing its cache write) but component B later raises, component A's completed render is now silently never cached — a real behavior change from before this PR, where each component's cache write happened independently and immediately in its own on_complete, unaffected by a sibling component's later failure.

No test exercises a non-disconnect error from drain_streams_concurrently after a prior successful pending write, so this regression isn't caught by the current suite.

Both issues stem from the same root cause: batching writes into a single deferred list trades per-component write isolation for a single all-or-nothing flush point. Worth considering whether flush should iterate with per-entry rescue, and whether the flush should also run (best-effort) in the error path so completed components aren't penalized by a sibling's failure.

[justin808]

Address-review summary

Scan scope: full PR history for #4443 after review-fix push 1587d6444.

Mattered

• Fixed streamed cache write isolation: one cache/tag failure is now logged per entry and does not skip later pending entries.
• Fixed rescue-path cache preservation: completed pending cache writes are flushed before re-raising a later stream failure.
• Fixed direct/manual lifecycle behavior: cached stream misses write immediately when no managed stream-view flush queue is active.

Optional

• Resolved stale variable-name feedback: current async cache-options spec uses distinct read/write option names.

Skipped

• None.

Next default scan starts after this comment. Say check all reviews to rescan the full PR.

[claude]

Review summary

Focused on react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rb, react_on_rails_pro/lib/react_on_rails_pro/cache/tag_index.rb, and react_on_rails_pro/app/helpers/react_on_rails_pro_helper.rb.

Good:

• The read_multi-based batching in TagIndex.append_entry_keys/read_indexes is a clean, backwards-compatible win (falls back to per-key reads for single-tag registration or stores without read_multi).
• Computing cache_options once per write and reusing it for both Rails.cache.write and register_normalized_tags correctly fixes the TTL-snapshot mismatch described in [Pro] Per-tag index registration adds blocking Rails.cache round-trips inside the streaming reactor; index/entry TTLs stamped from different clocks #4319, for both the stream and async paths.
• The failure handling in StreamCacheWrites.flush/write/log_failure is careful: one entry's write failure doesn't abort the rest, and even the logging itself is guarded so it can't keep a drained response open. Nice test coverage for the partial-failure and re-raise-after-flush cases.

Left inline comments on two points worth a second look:

1. Deferring all streamed cache writes until after the entire page's stream drains (rather than per-component, at each component's own completion) means a fast component's effective TTL is now measured from the page's total stream duration, not its own. With a short expires_in/expires_at and a slow sibling component, this can silently drop a cache write that would have succeeded under the old per-component timing — worth confirming this trade-off is intentional and perhaps calling it out in docs.
2. The deferred writes are flushed with a plain sequential each rather than being fanned out over the existing Async::Barrier, so N blocking cache round trips now happen serially right before closing the response stream, holding the request's thread open longer than before (this work previously overlapped with other components' streaming).

Also flagged a minor readability nit around the &.stop.then { ... } endless-method definition — it works correctly today (via NilClass#then) but is easy to misread and fragile to future edits.

No security issues spotted (cache keys/tags go through the existing digest/normalization path; no new user input is trusted).

[claude]

Review summary

This PR batches tag-index reads (read_multi), defers streamed cache writes until after all queued stream chunks drain, and aligns the cache options used for the write with those used for tag registration. The TagIndex read-batching change and the render_async_react_component_with_cache completion-time-options fix both look correct and are well covered by tests.

One correctness issue found (inline comment on react_on_rails_pro/lib/react_on_rails_pro/concerns/stream.rb:61-70):

StreamCacheWrites.cache_write_options converts an absolute :expires_at into a relative :expires_in snapshot computed at build time (when an individual streamed component's on_complete fires), but the resulting options are only consumed later at flush time (after all streamed components on the page have drained, via flush_pending_stream_cache_writes). Since ActiveSupport::Cache's :expires_in is relative to the moment Rails.cache.write actually runs, this causes both the cached entry and its tag-index registration to expire later than the caller's original expires_at — by however long the other components took to finish streaming. This directly undercuts the PR's own goal of avoiding stale/inconsistent cached content.

The non-streaming path (ReactOnRailsPro::Cache.cache_write_options in cache.rb) already handles this correctly by keeping :expires_at as an absolute timestamp when the store supports it — that logic is immune to deferred-write drift and could simply be reused here instead of the custom snapshot-to-expires_in conversion.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6c068519f2

ℹ️ 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".

[claude]

Review summary

This PR optimizes Pro's streamed/tagged cache path: it batches tag-index reads with read_multi when the store supports it, defers streamed cache writes until after response chunks drain, and unifies the cache options used for the write and for tag registration (fixing the drift between miss-time and completion-time options that #4319 was about). The refactor is well-structured and the new/updated specs cover the intended behavior thoroughly.

Left 3 inline comments after tracing the deferred-write path end-to-end:

1. concerns/stream.rb:23-30 (correctness, narrow but real) — On ActiveSupport < 7.0, StreamCacheWrites.build bakes a relative expires_in at on_complete time via Cache.cache_write_options, but the actual Rails.cache.write doesn't happen until the whole response drains (flush_pending_stream_cache_writes). The entry ends up living past its configured :expires_at by the queuing delay. This path isn't covered by the new specs (which only exercise cache_supports_expires_at? == true) and isn't exercised by CI (Gemfile.lock pins activesupport 7.2.3). Worth a fix or an explicit "AS < 7.0 no longer fully supported for streamed caching" note if that version is out of scope.
2. concerns/stream.rb:131 (performance, disclosed tradeoff) — Cache writes for all streamed+cached components on a page are now flushed serially at the very end of the response instead of overlapping with rendering, adding tail latency proportional to the number of cached components. This is intentional per the CHANGELOG, just flagging that it isn't quantified there and could be run concurrently via the Async::Barrier already in scope.
3. cache/tag_index.rb:199-200 (minor consistency) — read_indexes's respond_to?(:read_multi) check doesn't guard against stores that only inherit the non-batched default the way delete_entries's base_delete_multi? does for deletes, so the "batch" branch can be taken with zero actual round-trip savings on some stores.

Nothing here blocks merge in my view — #1 is the one worth a closer look before release given it's an untested code path.