From 7c517dabde233b65ac25117a80ad82f84d379276 Mon Sep 17 00:00:00 2001
From: Kabiru Mwenja <k.mwenja@openproject.com>
Date: Thu, 28 May 2026 17:40:16 +0300
Subject: [PATCH 1/7] Paginate work package activities tab at the database
 level
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The activity tab paginator previously loaded every journal and
changeset for a work package on each request, merged and sorted them
in Ruby, then sliced for the requested page. The eager-loading
wrapper ran four queries sized to the full pre-pagination set
(capped at limit × MAX_PAGES = 2000 rows by a Ruby-side ceiling).
Anchor jumps materialised the same array to call `find_index`.

Pagination now happens in Postgres. A `UNION ALL` of journals and
changesets, ordered by `(activity_at, id) DESC`, is paginated by
pagy at the database level. The eager-loading wrapper runs against
the page slice only — typically 20 rows instead of up to 2000.
Anchor jumps resolve to a `(created_at, id)` tuple and count rows
ahead in one query, so the path is independent of history size and
the MAX_PAGES cap goes away.

The `internal_visible` predicate is also exposed as a `Journal`
class scope (`internal_visible_for(project:, user:)`) so the
activities feed and anchor lookups share one visibility definition.
Existing fluent callers (`work_package.journals.internal_visible`)
keep working via a delegating shim on the has_many extension.
Anchor lookups go through the same scope so unviewable internal
journals fall back to page 1 rather than leaking through URL
behaviour.

A concurrent index on `journals (journable_type, journable_id,
created_at DESC, id DESC)` serves the new `WHERE … ORDER BY` plan
and supports the row-constructor predicate
`(created_at, id) > (?, ?)` used by anchor counting.

The `activity-N` anchor format (resolves a journal's
`sequence_version` via a LATERAL `ROW_NUMBER`) is preserved for
old bookmarks.
---
 app/models/journal.rb                         |   3 +-
 .../journals/scopes/internal_visible_for.rb   |  57 ++++++
 app/models/work_package/journalized.rb        |   8 +-
 .../work_packages/activities_tab/paginator.rb | 180 ++++++++++--------
 .../paginator/activities_query.rb             |  99 ++++++++++
 ...ex_journals_on_journable_and_created_at.rb |  14 ++
 .../activities_tab/paginator_spec.rb          | 100 ++++++++--
 7 files changed, 352 insertions(+), 109 deletions(-)
 create mode 100644 app/models/journals/scopes/internal_visible_for.rb
 create mode 100644 app/services/work_packages/activities_tab/paginator/activities_query.rb
 create mode 100644 db/migrate/20260528112247_add_index_journals_on_journable_and_created_at.rb

diff --git a/app/models/journal.rb b/app/models/journal.rb
index 192bae3f9305..047aabcded1d 100644
--- a/app/models/journal.rb
+++ b/app/models/journal.rb
@@ -122,7 +122,8 @@ class Journal < ApplicationRecord
 
   include ::Scopes::Scoped
 
-  scopes :with_sequence_version
+  scopes :with_sequence_version,
+         :internal_visible_for
 
   # Scopes to all journals excluding the initial journal - useful for change
   # logs like the history on issue#show
diff --git a/app/models/journals/scopes/internal_visible_for.rb b/app/models/journals/scopes/internal_visible_for.rb
new file mode 100644
index 000000000000..1653eaec22ae
--- /dev/null
+++ b/app/models/journals/scopes/internal_visible_for.rb
@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+# -- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+# ++
+
+module Journals::Scopes
+  # Restricts journals to those the given user is allowed to see in the given
+  # project. Internal journals are hidden unless the project has internal
+  # comments enabled, the Enterprise add-on is active, and the user holds
+  # `view_internal_comments`.
+  #
+  # This is a class scope (rather than a `has_many` extension) so it composes
+  # inside a subquery — for example, the journals leg of a UNION.
+  #
+  # The `user:` default of `User.current` assumes request context. Pass `user:`
+  # explicitly from background jobs where `User.current` is not set.
+  module InternalVisibleFor
+    extend ActiveSupport::Concern
+
+    class_methods do
+      def internal_visible_for(project:, user: User.current)
+        if EnterpriseToken.allows_to?(:internal_comments) &&
+            project.enabled_internal_comments &&
+            user.allowed_in_project?(:view_internal_comments, project)
+          all
+        else
+          where(internal: false)
+        end
+      end
+    end
+  end
+end
diff --git a/app/models/work_package/journalized.rb b/app/models/work_package/journalized.rb
index 01347465f5ec..f9eee8753df9 100644
--- a/app/models/work_package/journalized.rb
+++ b/app/models/work_package/journalized.rb
@@ -34,13 +34,7 @@ module WorkPackage::Journalized
   included do
     acts_as_journalized journals_association_extension: proc {
       def internal_visible
-        if EnterpriseToken.allows_to?(:internal_comments) &&
-            proxy_association.owner.project.enabled_internal_comments &&
-            User.current.allowed_in_project?(:view_internal_comments, proxy_association.owner.project)
-          all
-        else
-          where(internal: false)
-        end
+        merge(Journal.internal_visible_for(project: proxy_association.owner.project))
       end
     }
 
diff --git a/app/services/work_packages/activities_tab/paginator.rb b/app/services/work_packages/activities_tab/paginator.rb
index 7efccf5cb4c5..5b78acee6447 100644
--- a/app/services/work_packages/activities_tab/paginator.rb
+++ b/app/services/work_packages/activities_tab/paginator.rb
@@ -28,7 +28,8 @@
 # See COPYRIGHT and LICENSE files for more details.
 #++
 
-# Paginates work package activities (journals and changesets) with support for filtering and anchor navigation.
+# Paginates work package activities (journals and changesets) with support for
+# filtering and anchor navigation.
 #
 # Filter modes:
 # - :all - Shows all activities (default)
@@ -39,6 +40,11 @@
 # - "comment-{journal_id}" - Navigate to specific journal by ID
 # - "activity-{sequence_version}" - Navigate to journal by sequence version
 #
+# Internally, the activities feed is materialised as a single UNION ALL
+# relation of journals and changesets (see {ActivitiesQuery}) and paginated
+# by pagy at the database level. Only the page slice is hydrated and wrapped
+# for eager-loading, keeping per-request cost independent of total history size.
+#
 # @param work_package [WorkPackage] The work package to paginate activities for
 # @param params [Hash] Pagination and filtering parameters
 #
@@ -52,8 +58,6 @@ class WorkPackages::ActivitiesTab::Paginator
   include Pagy::Method
   include WorkPackages::ActivitiesTab::JournalSortingInquirable
 
-  MAX_PAGES = 100
-
   def self.paginate(work_package, params = {})
     new(work_package, params).call
   end
@@ -67,124 +71,138 @@ def initialize(work_package, params = {})
   end
 
   def call
-    anchor_type, target_record_id = extract_target_record_id
+    anchor_type, target_record_id = parse_anchor
 
-    pagy, records =
+    pagy_obj, page_relation =
       if anchor_type && target_record_id
-        @filter = :all # Ignore filter when jumping to specific journal
-        pagy_array_for_target_journal(anchor_type, target_record_id)
+        @filter = :all
+        pagy_at_anchor(anchor_type, target_record_id)
       else
-        pagy(:offset, capped_journals, **pagy_options)
+        pagy(:offset, activities_scope, **pagy_options)
       end
 
-    # For UI display: if user wants "oldest first" UI, reverse the array
-    records = records.reverse if journal_sorting.asc?
+    activities = load_activities(page_relation)
+    activities = activities.reverse if journal_sorting.asc?
 
-    [pagy, records]
+    [pagy_obj, activities]
   end
 
   private
 
+  def activities_query
+    @activities_query ||= ActivitiesQuery.new(work_package, filter:)
+  end
+
+  def activities_scope
+    @activities_scope ||= activities_query.call
+  end
+
+  def limit
+    params[:limit] || Pagy::DEFAULT[:limit]
+  end
+
   def pagy_options
     {
       page: params[:page] || 1,
-      limit: params[:limit] || Pagy::DEFAULT[:limit],
+      limit:,
       request: { params: }
     }.compact
   end
 
-  def extract_target_record_id
+  def parse_anchor
     anchor = params[:anchor] # e.g., "comment-78758" (without #)
-    return nil unless anchor
+    return unless anchor
 
     match = anchor.match(/^(comment|activity)-(\d+)$/)
-    match && match.length == 3 ? [match[1].inquiry, match[2].to_i] : []
-  end
-
-  def pagy_array_for_target_journal(anchor_type, target_record_id)
-    journals = base_journals
-
-    target_index = journals.find_index do |record|
-      if anchor_type.comment?
-        record.id == target_record_id
-      elsif anchor_type.activity?
-        record.sequence_version == target_record_id
-      else
-        false
-      end
-    end
+    return unless match
 
-    if target_index
-      limit = pagy_options[:limit]
-      target_page = (target_index / limit) + 1
-      pagy(:offset, journals, **pagy_options, page: target_page)
-    else
-      # Journal might be filtered out or deleted - fallback to page 1
-      pagy(:offset, journals, **pagy_options, page: 1)
-    end
+    [match[1].inquiry, match[2].to_i]
   end
 
-  def base_journals
-    combine_and_sort_records(fetch_journals, fetch_revisions)
+  # An unresolvable anchor (deleted, never existed, not visible to the user)
+  # falls back to the default page from `pagy_options`.
+  def pagy_at_anchor(anchor_type, target_record_id)
+    options = pagy_options
+    options[:page] = page_for_anchor(anchor_type, target_record_id) || options[:page]
+    pagy(:offset, activities_scope, **options)
   end
 
-  def capped_journals
-    max_records = (params[:limit] || Pagy::DEFAULT[:limit]) * MAX_PAGES
-    base_journals.first(max_records)
+  # Resolves an anchor to its target page by counting records ahead of it.
+  # Returns nil when the anchor is unresolvable (e.g. deleted) so the caller
+  # falls back to page 1.
+  def page_for_anchor(anchor_type, target_record_id)
+    activity_at, anchor_id = locate_anchor(anchor_type, target_record_id)
+    return nil unless activity_at && anchor_id
+
+    rows_ahead = activities_scope
+      .where("(activities.activity_at, activities.id) > (?, ?)", activity_at, anchor_id)
+      .count(:all)
+
+    (rows_ahead / limit) + 1
   end
 
-  def fetch_journals
-    API::V3::Activities::ActivityEagerLoadingWrapper.wrap(fetch_ar_journals)
+  # Anchors must observe the same visibility rules as the activities feed.
+  # Otherwise the count-ahead would route an unviewable journal to a page
+  # number and leak the existence of internal journals through the URL.
+  def locate_anchor(anchor_type, target_record_id)
+    if anchor_type.comment?
+      activities_query.visible_journals
+        .where(id: target_record_id)
+        .pick(:created_at, :id)
+    elsif anchor_type.activity?
+      locate_anchor_by_sequence_version(target_record_id)
+    end
   end
 
-  def fetch_ar_journals
-    journals = work_package
-      .journals
-      .internal_visible
-      .includes(
-        :user,
-        :customizable_journals,
-        :attachable_journals,
-        :storable_journals,
-        :notifications
-      )
-      .reorder(version: :desc) # Always fetch newest first for pagination
+  def locate_anchor_by_sequence_version(sequence_version)
+    activities_query.visible_journals
       .with_sequence_version
-
-    case filter
-    when :only_comments then apply_comments_only_filter(journals)
-    when :only_changes then apply_changes_only_filter(journals)
-    else
-      journals
-    end
+      .where(ranked: { sequence_version: sequence_version })
+      .pick(:created_at, :id)
   end
 
-  def fetch_revisions
-    return Changeset.none if filter == :only_comments
+  def load_activities(page_relation)
+    activity_refs = page_relation.pluck(Arel.sql("activities.kind"), Arel.sql("activities.id"))
+    activities_by_kind = load_page_activities_by_kind(activity_refs)
 
-    work_package.changesets.includes(:user, :repository)
+    ordered_activities = activity_refs.filter_map { |kind, id| activities_by_kind[kind][id] }
+    eager_load_journals(ordered_activities)
   end
 
-  def combine_and_sort_records(journals, revisions)
-    (journals + revisions).sort_by do |record|
-      timestamp = record_timestamp(record)
-      [-timestamp, -record.id] # Always sort DESC (newest first)
-    end
+  def load_page_activities_by_kind(activity_refs)
+    ids_by_kind = activity_refs.group_by(&:first).transform_values { it.map(&:last) }
+    {
+      ActivitiesQuery::KIND_JOURNAL => load_page_journals(ids_by_kind[ActivitiesQuery::KIND_JOURNAL] || []),
+      ActivitiesQuery::KIND_CHANGESET => load_page_changesets(ids_by_kind[ActivitiesQuery::KIND_CHANGESET] || [])
+    }
   end
 
-  def record_timestamp(record)
-    if record.is_a?(API::V3::Activities::ActivityEagerLoadingWrapper)
-      record.created_at&.to_i
-    elsif record.is_a?(Changeset)
-      record.committed_on.to_i
-    end
+  def load_page_journals(ids)
+    return {} if ids.empty?
+
+    Journal
+      .where(id: ids)
+      .with_sequence_version
+      .includes(:user, :customizable_journals, :attachable_journals, :storable_journals, :notifications)
+      .index_by(&:id)
   end
 
-  def apply_comments_only_filter(scope)
-    scope.where.not(notes: [nil, ""])
+  def load_page_changesets(ids)
+    return {} if ids.empty?
+
+    Changeset
+      .where(id: ids)
+      .includes(:user, :repository)
+      .index_by(&:id)
   end
 
-  def apply_changes_only_filter(scope)
-    JournalChangesFilter.apply(scope)
+  # Substitutes journals with their eager-loading wrappers so the wrapper's
+  # batch queries (journable, predecessor, data, notifications) run against
+  # the page slice only. Order from the input is preserved.
+  def eager_load_journals(activities)
+    journals = activities.grep(Journal)
+    wrapped_by_id = API::V3::Activities::ActivityEagerLoadingWrapper.wrap(journals).index_by(&:id)
+
+    activities.map { it.is_a?(Journal) ? wrapped_by_id[it.id] : it }
   end
 end
diff --git a/app/services/work_packages/activities_tab/paginator/activities_query.rb b/app/services/work_packages/activities_tab/paginator/activities_query.rb
new file mode 100644
index 000000000000..90a5f7d71555
--- /dev/null
+++ b/app/services/work_packages/activities_tab/paginator/activities_query.rb
@@ -0,0 +1,99 @@
+# frozen_string_literal: true
+
+# -- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+# ++
+
+# Builds the UNION ALL of journals and changesets for a work package's activity
+# feed, normalised to `(id, kind, activity_at)` rows ordered newest-first.
+#
+# Hydration is intentionally left out: the paginator slices the page first, then
+# loads Journal/Changeset records and applies the eager-loading wrapper. That
+# keeps the wrapper running against the page slice instead of the full history.
+#
+# The carrier class is `Journal` for AR composition only — rows in this relation
+# are never materialised as Journal records.
+class WorkPackages::ActivitiesTab::Paginator::ActivitiesQuery
+  KIND_JOURNAL = Journal.name
+  KIND_CHANGESET = Changeset.name
+
+  def initialize(work_package, filter:)
+    @work_package = work_package
+    @filter = filter
+  end
+
+  def call
+    Journal
+      .from(Arel.sql("(#{union_sql}) AS activities"))
+      .select(Arel.sql("activities.id, activities.kind, activities.activity_at"))
+      .order(Arel.sql("activities.activity_at DESC, activities.id DESC"))
+  end
+
+  def visible_journals
+    work_package.journals.internal_visible
+  end
+
+  private
+
+  attr_reader :work_package, :filter
+
+  def union_sql
+    parts = [journals_leg_sql]
+    parts << changesets_leg_sql unless filter == :only_comments
+    parts.join(" UNION ALL ")
+  end
+
+  def journals_leg_sql
+    apply_filter(visible_journals)
+      .unscope(:order)
+      .select("journals.id, #{quote(KIND_JOURNAL)} AS kind, journals.created_at AS activity_at")
+      .to_sql
+  end
+
+  def apply_filter(scope)
+    case filter
+    when :only_comments
+      scope.where.not(notes: [nil, ""])
+    when :only_changes
+      WorkPackages::ActivitiesTab::Paginator::JournalChangesFilter.apply(scope)
+    else
+      scope
+    end
+  end
+
+  def changesets_leg_sql
+    work_package
+      .changesets
+      .unscope(:order)
+      .select("changesets.id, #{quote(KIND_CHANGESET)} AS kind, changesets.committed_on AS activity_at")
+      .to_sql
+  end
+
+  def quote(value)
+    ActiveRecord::Base.connection.quote(value)
+  end
+end
diff --git a/db/migrate/20260528112247_add_index_journals_on_journable_and_created_at.rb b/db/migrate/20260528112247_add_index_journals_on_journable_and_created_at.rb
new file mode 100644
index 000000000000..b5f28a7283e9
--- /dev/null
+++ b/db/migrate/20260528112247_add_index_journals_on_journable_and_created_at.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+class AddIndexJournalsOnJournableAndCreatedAt < ActiveRecord::Migration[8.1]
+  disable_ddl_transaction!
+
+  def change
+    add_index :journals,
+              %i[journable_type journable_id created_at id],
+              order: { created_at: :desc, id: :desc },
+              name: "index_journals_on_journable_and_created_at",
+              algorithm: :concurrently,
+              if_not_exists: true
+  end
+end
diff --git a/spec/services/work_packages/activities_tab/paginator_spec.rb b/spec/services/work_packages/activities_tab/paginator_spec.rb
index e6323340eb67..c780b76efadc 100644
--- a/spec/services/work_packages/activities_tab/paginator_spec.rb
+++ b/spec/services/work_packages/activities_tab/paginator_spec.rb
@@ -206,43 +206,84 @@
       end
     end
 
-    context "when record count exceeds MAX_PAGES cap" do
+    context "with a large activity history" do
       let(:test_limit) { 2 }
 
       before do
-        stub_const("#{described_class}::MAX_PAGES", 3)
         8.times do |i|
           create(:work_package_journal, user:, notes: "Comment #{i + 1}", journable: work_package, version: i + 2)
         end
         params[:limit] = test_limit
       end
 
-      it "caps total records to limit * MAX_PAGES" do
-        pagy, _records = paginator.call
+      it "resolves an anchor to the oldest journal on the last page" do
+        oldest_journal = work_package.journals.order(:version).first
+        params[:anchor] = "comment-#{oldest_journal.id}"
+        pagy, records = paginator.call
 
-        expect(pagy.count).to eq(test_limit * 3)
-        expect(pagy.pages).to eq(3)
+        expect(records.map(&:id)).to include(oldest_journal.id)
+        expect(pagy.page).to eq(5)
       end
 
-      it "keeps the newest records when truncating" do
-        _pagy, records = paginator.call
+      it "issues a bounded number of queries regardless of history size" do
+        described_class.new(work_package, params).call # warm autoloads + caches
+
+        50.times { |i| create(:work_package_journal, user:, journable: work_package, version: 10 + i) }
+
+        recorder = ActiveRecord::QueryRecorder.new do
+          described_class.new(work_package, params).call
+        end
 
-        notes = records.map(&:notes)
-        expect(notes).to include("Comment 8")
-        expect(notes).not_to include("Comment 1")
+        # The wrapper runs against the page slice, so query count does not scale
+        # with history size. Ceiling leaves headroom for an extra eager-load
+        # without becoming brittle; actual count today is ~10.
+        expect(recorder.count).to be < 20,
+                                  "expected query count bounded regardless of history; got #{recorder.count}:\n" \
+                                  "#{recorder.log.join("\n")}"
       end
+    end
 
-      it "still resolves an anchor to a journal beyond the cap" do
-        oldest_journal = work_package.journals.order(:version).first
-        params[:anchor] = "comment-#{oldest_journal.id}"
-        pagy, records = paginator.call
+    # The journal factory chains each predecessor's `validity_period` upper
+    # bound to the next journal's `created_at`; two journals at the same
+    # `created_at` produce an empty range and trip the
+    # `validity_period_not_empty` constraint. A journal + changeset pair
+    # sidesteps that while still exercising the secondary id sort.
+    context "with a journal and a changeset at the same timestamp" do
+      let(:repository) { create(:repository_subversion, project:) }
+      let(:shared_time) { 1.day.ago }
+      let!(:tied_journal) do
+        create(:work_package_journal,
+               user:,
+               notes: "Tied with changeset",
+               journable: work_package,
+               version: 2,
+               created_at: shared_time)
+      end
+      let!(:tied_changeset) do
+        create(:changeset, repository:, committed_on: shared_time, revision: "tied")
+      end
 
-        expect(records.map(&:id)).to include(oldest_journal.id)
+      before do
+        # Default sort can be :asc, which reverses the records. Pin to :desc so
+        # the relation's `id DESC` secondary sort surfaces in the result.
+        user.pref.update!(comments_sorting: :desc)
+        work_package.changesets << tied_changeset
+      end
+
+      it "breaks ties by id descending" do
+        _pagy, records = paginator.call
+
+        # Journals and changesets have independent id sequences, so the kind
+        # holding the higher id is determined at runtime — but whichever it
+        # is must come first under the `id DESC` secondary sort.
+        ordered_pairs = records.map { |r| r.is_a?(Changeset) ? [:changeset, r.id] : [:journal, r.id] }
+        journal_pos   = ordered_pairs.index([:journal, tied_journal.id])
+        changeset_pos = ordered_pairs.index([:changeset, tied_changeset.id])
 
-        aggregate_failures "breaks out of capped limit" do
-          expect(pagy.count).to eq(work_package.journals.count)
-          expect(pagy.pages).to eq(5)
-          expect(pagy.page).to eq(5), "expected oldest journal on last page (page 5), got page #{pagy.page}"
+        if tied_journal.id > tied_changeset.id
+          expect(journal_pos).to be < changeset_pos
+        else
+          expect(changeset_pos).to be < journal_pos
         end
       end
     end
@@ -294,6 +335,25 @@
           expect(journal_notes).not_to include("Internal comment")
           expect(journal_notes).to include("Public comment")
         end
+
+        it "falls back to page 1 when anchoring to an internal journal" do
+          params[:anchor] = "comment-#{internal_journal.id}"
+          pagy, _records = described_class.new(work_package, params).call
+
+          expect(pagy.page).to eq(1)
+        end
+
+        it "falls back to page 1 when anchoring to an internal journal by sequence_version" do
+          internal_sequence_version = Journal
+            .where(journable: work_package)
+            .with_sequence_version
+            .where(id: internal_journal.id)
+            .pick("ranked.sequence_version")
+          params[:anchor] = "activity-#{internal_sequence_version}"
+          pagy, _records = described_class.new(work_package, params).call
+
+          expect(pagy.page).to eq(1)
+        end
       end
     end
 

From 789635646ed6a5becac5546a6a20fada62847a87 Mon Sep 17 00:00:00 2001
From: Kabiru Mwenja <k.mwenja@openproject.com>
Date: Thu, 28 May 2026 21:21:34 +0300
Subject: [PATCH 2/7] Polish work package activities paginator after code
 review
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Anchored navigation always opens page 1 when the target record is
unresolvable; any `params[:page]` sent alongside is ignored — the
anchor is the explicit navigation intent.

Drop the `@filter = :all` mutation in `call`. The anchor path builds
an unfiltered scope via `activities_scope(filter: :all)` and threads
it into `page_for_anchor`, so the public `filter` reader returns the
user's filter unchanged regardless of anchor use.

Collapse the parallel `activities_query`/`anchor_query` pairs into
one `activities_scope(filter:)`. `visible_journals` moves to the
Paginator where its two consumers live; `ActivitiesQuery` inlines
its single use of the same chain.

Revert the `Journal.internal_visible_for` class-scope extraction.
No caller used it directly — `internal_visible` body is restored
inline on the `has_many :journals` extension.

Tests cover both the page-1 fallback when an anchor is unresolvable
and the preservation of `paginator.filter` across an anchored call.
---
 app/models/journal.rb                         |  3 +-
 .../journals/scopes/internal_visible_for.rb   | 57 -------------------
 app/models/work_package/journalized.rb        |  8 ++-
 .../work_packages/activities_tab/paginator.rb | 40 +++++++------
 .../paginator/activities_query.rb             |  6 +-
 .../activities_tab/paginator_spec.rb          | 16 ++++--
 6 files changed, 42 insertions(+), 88 deletions(-)
 delete mode 100644 app/models/journals/scopes/internal_visible_for.rb

diff --git a/app/models/journal.rb b/app/models/journal.rb
index 047aabcded1d..192bae3f9305 100644
--- a/app/models/journal.rb
+++ b/app/models/journal.rb
@@ -122,8 +122,7 @@ class Journal < ApplicationRecord
 
   include ::Scopes::Scoped
 
-  scopes :with_sequence_version,
-         :internal_visible_for
+  scopes :with_sequence_version
 
   # Scopes to all journals excluding the initial journal - useful for change
   # logs like the history on issue#show
diff --git a/app/models/journals/scopes/internal_visible_for.rb b/app/models/journals/scopes/internal_visible_for.rb
deleted file mode 100644
index 1653eaec22ae..000000000000
--- a/app/models/journals/scopes/internal_visible_for.rb
+++ /dev/null
@@ -1,57 +0,0 @@
-# frozen_string_literal: true
-
-# -- copyright
-# OpenProject is an open source project management software.
-# Copyright (C) the OpenProject GmbH
-#
-# This program is free software; you can redistribute it and/or
-# modify it under the terms of the GNU General Public License version 3.
-#
-# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
-# Copyright (C) 2006-2013 Jean-Philippe Lang
-# Copyright (C) 2010-2013 the ChiliProject Team
-#
-# This program is free software; you can redistribute it and/or
-# modify it under the terms of the GNU General Public License
-# as published by the Free Software Foundation; either version 2
-# of the License, or (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
-#
-# See COPYRIGHT and LICENSE files for more details.
-# ++
-
-module Journals::Scopes
-  # Restricts journals to those the given user is allowed to see in the given
-  # project. Internal journals are hidden unless the project has internal
-  # comments enabled, the Enterprise add-on is active, and the user holds
-  # `view_internal_comments`.
-  #
-  # This is a class scope (rather than a `has_many` extension) so it composes
-  # inside a subquery — for example, the journals leg of a UNION.
-  #
-  # The `user:` default of `User.current` assumes request context. Pass `user:`
-  # explicitly from background jobs where `User.current` is not set.
-  module InternalVisibleFor
-    extend ActiveSupport::Concern
-
-    class_methods do
-      def internal_visible_for(project:, user: User.current)
-        if EnterpriseToken.allows_to?(:internal_comments) &&
-            project.enabled_internal_comments &&
-            user.allowed_in_project?(:view_internal_comments, project)
-          all
-        else
-          where(internal: false)
-        end
-      end
-    end
-  end
-end
diff --git a/app/models/work_package/journalized.rb b/app/models/work_package/journalized.rb
index f9eee8753df9..01347465f5ec 100644
--- a/app/models/work_package/journalized.rb
+++ b/app/models/work_package/journalized.rb
@@ -34,7 +34,13 @@ module WorkPackage::Journalized
   included do
     acts_as_journalized journals_association_extension: proc {
       def internal_visible
-        merge(Journal.internal_visible_for(project: proxy_association.owner.project))
+        if EnterpriseToken.allows_to?(:internal_comments) &&
+            proxy_association.owner.project.enabled_internal_comments &&
+            User.current.allowed_in_project?(:view_internal_comments, proxy_association.owner.project)
+          all
+        else
+          where(internal: false)
+        end
       end
     }
 
diff --git a/app/services/work_packages/activities_tab/paginator.rb b/app/services/work_packages/activities_tab/paginator.rb
index 5b78acee6447..b28055b3adfb 100644
--- a/app/services/work_packages/activities_tab/paginator.rb
+++ b/app/services/work_packages/activities_tab/paginator.rb
@@ -36,10 +36,13 @@
 # - :only_comments - Shows only journals with notes
 # - :only_changes - Shows only journals with detected changes using SQL heuristics
 #
-# Anchor format (filter is reset to :all when using anchors):
+# Anchor format:
 # - "comment-{journal_id}" - Navigate to specific journal by ID
 # - "activity-{sequence_version}" - Navigate to journal by sequence version
 #
+# Anchored navigation bypasses the active filter so a deep link to a record
+# that wouldn't match the filter still resolves.
+#
 # Internally, the activities feed is materialised as a single UNION ALL
 # relation of journals and changesets (see {ActivitiesQuery}) and paginated
 # by pagy at the database level. Only the page slice is hydrated and wrapped
@@ -75,7 +78,6 @@ def call
 
     pagy_obj, page_relation =
       if anchor_type && target_record_id
-        @filter = :all
         pagy_at_anchor(anchor_type, target_record_id)
       else
         pagy(:offset, activities_scope, **pagy_options)
@@ -89,12 +91,15 @@ def call
 
   private
 
-  def activities_query
-    @activities_query ||= ActivitiesQuery.new(work_package, filter:)
+  # Activities relation (UNION of journals and changesets). Anchored
+  # navigation passes `filter: :all` so a deep link to a record that
+  # wouldn't match the active filter still resolves.
+  def activities_scope(filter: self.filter)
+    ActivitiesQuery.new(work_package, filter:).call
   end
 
-  def activities_scope
-    @activities_scope ||= activities_query.call
+  def visible_journals
+    work_package.journals.internal_visible
   end
 
   def limit
@@ -120,21 +125,22 @@ def parse_anchor
   end
 
   # An unresolvable anchor (deleted, never existed, not visible to the user)
-  # falls back to the default page from `pagy_options`.
+  # opens the tab at the newest page, the same as opening it without an
+  # anchor. Any `params[:page]` sent alongside is intentionally ignored: the
+  # anchor was the explicit navigation intent.
   def pagy_at_anchor(anchor_type, target_record_id)
-    options = pagy_options
-    options[:page] = page_for_anchor(anchor_type, target_record_id) || options[:page]
-    pagy(:offset, activities_scope, **options)
+    scope = activities_scope(filter: :all)
+    page = page_for_anchor(scope, anchor_type, target_record_id) || 1
+    pagy(:offset, scope, **pagy_options, page:)
   end
 
   # Resolves an anchor to its target page by counting records ahead of it.
-  # Returns nil when the anchor is unresolvable (e.g. deleted) so the caller
-  # falls back to page 1.
-  def page_for_anchor(anchor_type, target_record_id)
+  # Returns nil when the anchor is unresolvable so the caller falls back.
+  def page_for_anchor(scope, anchor_type, target_record_id)
     activity_at, anchor_id = locate_anchor(anchor_type, target_record_id)
     return nil unless activity_at && anchor_id
 
-    rows_ahead = activities_scope
+    rows_ahead = scope
       .where("(activities.activity_at, activities.id) > (?, ?)", activity_at, anchor_id)
       .count(:all)
 
@@ -146,16 +152,14 @@ def page_for_anchor(anchor_type, target_record_id)
   # number and leak the existence of internal journals through the URL.
   def locate_anchor(anchor_type, target_record_id)
     if anchor_type.comment?
-      activities_query.visible_journals
-        .where(id: target_record_id)
-        .pick(:created_at, :id)
+      visible_journals.where(id: target_record_id).pick(:created_at, :id)
     elsif anchor_type.activity?
       locate_anchor_by_sequence_version(target_record_id)
     end
   end
 
   def locate_anchor_by_sequence_version(sequence_version)
-    activities_query.visible_journals
+    visible_journals
       .with_sequence_version
       .where(ranked: { sequence_version: sequence_version })
       .pick(:created_at, :id)
diff --git a/app/services/work_packages/activities_tab/paginator/activities_query.rb b/app/services/work_packages/activities_tab/paginator/activities_query.rb
index 90a5f7d71555..5c4856e53082 100644
--- a/app/services/work_packages/activities_tab/paginator/activities_query.rb
+++ b/app/services/work_packages/activities_tab/paginator/activities_query.rb
@@ -53,10 +53,6 @@ def call
       .order(Arel.sql("activities.activity_at DESC, activities.id DESC"))
   end
 
-  def visible_journals
-    work_package.journals.internal_visible
-  end
-
   private
 
   attr_reader :work_package, :filter
@@ -68,7 +64,7 @@ def union_sql
   end
 
   def journals_leg_sql
-    apply_filter(visible_journals)
+    apply_filter(work_package.journals.internal_visible)
       .unscope(:order)
       .select("journals.id, #{quote(KIND_JOURNAL)} AS kind, journals.created_at AS activity_at")
       .to_sql
diff --git a/spec/services/work_packages/activities_tab/paginator_spec.rb b/spec/services/work_packages/activities_tab/paginator_spec.rb
index c780b76efadc..de941c5202b7 100644
--- a/spec/services/work_packages/activities_tab/paginator_spec.rb
+++ b/spec/services/work_packages/activities_tab/paginator_spec.rb
@@ -181,6 +181,14 @@
 
             expect(pagy.page).to eq(1)
           end
+
+          it "falls back to page 1 even when params[:page] is set alongside an unresolvable anchor" do
+            params[:anchor] = "comment-999999"
+            params[:page] = 3
+            pagy, _records = paginator.call
+
+            expect(pagy.page).to eq(1)
+          end
         end
 
         context "with activity anchor" do
@@ -751,19 +759,17 @@
           params[:anchor] = "comment-#{journal_with_notes.id}"
           _pagy, records = paginator.call
 
-          expect(paginator.filter).to eq(:all) # resets to :all when anchoring
           expect(records.map(&:id)).to include(journal_with_notes.id)
         end
       end
 
       context "when anchoring to a journal that doesn't match the filter" do
-        it "ignores the filter and shows all journals (fallback behavior)" do
+        it "ignores the filter without mutating the filter reader" do
           params[:anchor] = "comment-#{journal_without_notes.id}"
           _pagy, records = paginator.call
 
-          expect(paginator.filter).to eq(:all) # resets to :all when anchoring
-          expect(records.map(&:id)).to include(journal_without_notes.id)
-          expect(records.map(&:id)).to include(journal_with_notes.id)
+          expect(records.map(&:id)).to include(journal_without_notes.id, journal_with_notes.id)
+          expect(paginator.filter).to eq(:only_comments)
         end
       end
     end

From 2ac821c03a673b80dcb9b4ae28e842b9a888277f Mon Sep 17 00:00:00 2001
From: Kabiru Mwenja <k.mwenja@openproject.com>
Date: Sat, 30 May 2026 10:33:25 +0300
Subject: [PATCH 3/7] Address activities paginator review feedback

Cache attachment lookups in the journal formatter through the
request-scoped JournalFormatterCache, matching the other association
formatters. A feed rendering many journals now issues one lookup per
attachment instead of one per attachment detail. The previously
suggested `includes` cannot help here: the formatter resolves
attachments via `Attachment.find_by` on an id parsed from the detail
key, bypassing the association graph.

Run the eager-loading wrapper inside load_page_journals and drop the
separate eager_load_journals pass, so hydration no longer sniffs record
types. Rendered order stays anchored to the database-ordered page slice.

Add a trailing `kind DESC` to the activities ORDER BY so a journal and
changeset sharing both timestamp and id still order deterministically.
---
 .../work_packages/activities_tab/paginator.rb | 30 ++++++++-----------
 .../paginator/activities_query.rb             |  2 +-
 .../journal_formatter/attachment.rb           | 18 +++++++++--
 .../journal_formatter/attachment_spec.rb      | 18 +++++++++++
 .../activities_tab/paginator_spec.rb          | 16 ++++++----
 5 files changed, 56 insertions(+), 28 deletions(-)

diff --git a/app/services/work_packages/activities_tab/paginator.rb b/app/services/work_packages/activities_tab/paginator.rb
index b28055b3adfb..ec9cfdca53cc 100644
--- a/app/services/work_packages/activities_tab/paginator.rb
+++ b/app/services/work_packages/activities_tab/paginator.rb
@@ -165,30 +165,34 @@ def locate_anchor_by_sequence_version(sequence_version)
       .pick(:created_at, :id)
   end
 
+  # The page slice arrives ordered by the database; that ordering is the source
+  # of truth for the feed, and rebuilding the list in ref order preserves it.
   def load_activities(page_relation)
-    activity_refs = page_relation.pluck(Arel.sql("activities.kind"), Arel.sql("activities.id"))
-    activities_by_kind = load_page_activities_by_kind(activity_refs)
+    ordered_refs = page_relation.pluck(Arel.sql("activities.kind"), Arel.sql("activities.id"))
+    records_by_kind = load_page_activities_by_kind(ordered_refs)
 
-    ordered_activities = activity_refs.filter_map { |kind, id| activities_by_kind[kind][id] }
-    eager_load_journals(ordered_activities)
+    ordered_refs.filter_map { |kind, id| records_by_kind[kind][id] }
   end
 
-  def load_page_activities_by_kind(activity_refs)
-    ids_by_kind = activity_refs.group_by(&:first).transform_values { it.map(&:last) }
+  def load_page_activities_by_kind(ordered_refs)
+    ids_by_kind = ordered_refs.group_by(&:first).transform_values { it.map(&:last) }
     {
       ActivitiesQuery::KIND_JOURNAL => load_page_journals(ids_by_kind[ActivitiesQuery::KIND_JOURNAL] || []),
       ActivitiesQuery::KIND_CHANGESET => load_page_changesets(ids_by_kind[ActivitiesQuery::KIND_CHANGESET] || [])
     }
   end
 
+  # Journals are wrapped for eager-loading here so the wrapper's batch queries
+  # (journable, predecessor, data, notifications) run against the page slice only.
   def load_page_journals(ids)
     return {} if ids.empty?
 
-    Journal
+    journals = Journal
       .where(id: ids)
       .with_sequence_version
       .includes(:user, :customizable_journals, :attachable_journals, :storable_journals, :notifications)
-      .index_by(&:id)
+
+    API::V3::Activities::ActivityEagerLoadingWrapper.wrap(journals).index_by(&:id)
   end
 
   def load_page_changesets(ids)
@@ -199,14 +203,4 @@ def load_page_changesets(ids)
       .includes(:user, :repository)
       .index_by(&:id)
   end
-
-  # Substitutes journals with their eager-loading wrappers so the wrapper's
-  # batch queries (journable, predecessor, data, notifications) run against
-  # the page slice only. Order from the input is preserved.
-  def eager_load_journals(activities)
-    journals = activities.grep(Journal)
-    wrapped_by_id = API::V3::Activities::ActivityEagerLoadingWrapper.wrap(journals).index_by(&:id)
-
-    activities.map { it.is_a?(Journal) ? wrapped_by_id[it.id] : it }
-  end
 end
diff --git a/app/services/work_packages/activities_tab/paginator/activities_query.rb b/app/services/work_packages/activities_tab/paginator/activities_query.rb
index 5c4856e53082..aa6f3186dffe 100644
--- a/app/services/work_packages/activities_tab/paginator/activities_query.rb
+++ b/app/services/work_packages/activities_tab/paginator/activities_query.rb
@@ -50,7 +50,7 @@ def call
     Journal
       .from(Arel.sql("(#{union_sql}) AS activities"))
       .select(Arel.sql("activities.id, activities.kind, activities.activity_at"))
-      .order(Arel.sql("activities.activity_at DESC, activities.id DESC"))
+      .order(Arel.sql("activities.activity_at DESC, activities.id DESC, activities.kind DESC"))
   end
 
   private
diff --git a/lib/open_project/journal_formatter/attachment.rb b/lib/open_project/journal_formatter/attachment.rb
index cdce0ed09f7c..9bbe983e4ebb 100644
--- a/lib/open_project/journal_formatter/attachment.rb
+++ b/lib/open_project/journal_formatter/attachment.rb
@@ -41,7 +41,7 @@ def render(key, values, options = { html: true })
     if options[:html]
       label, old_value, value = *format_html_details(label, old_value, value)
 
-      value = format_html_attachment_detail(key.to_s.sub("attachments_", ""), value)
+      value = format_html_attachment_detail(key.to_s.sub("attachments_", ""), value, options[:cache])
     end
 
     render_attachment_detail_text(label, value, old_value)
@@ -67,11 +67,23 @@ def controller
     nil
   end
 
-  def format_html_attachment_detail(key, value)
-    if value.present? && a = Attachment.find_by(id: key.to_i)
+  def format_html_attachment_detail(key, value, cache)
+    if value.present? && (a = find_attachment(key.to_i, cache))
       link_to_attachment(a, only_path: false)
     elsif value.present?
       value
     end
   end
+
+  # Resolve through the request-scoped cache so a feed rendering many journals
+  # issues one lookup per attachment instead of one per detail occurrence.
+  def find_attachment(id, cache)
+    if cache
+      cache.fetch(Attachment, id) do # rubocop:disable Lint/UselessDefaultValueArgument
+        Attachment.find_by(id:)
+      end
+    else
+      Attachment.find_by(id:)
+    end
+  end
 end
diff --git a/spec/lib/open_project/journal_formatter/attachment_spec.rb b/spec/lib/open_project/journal_formatter/attachment_spec.rb
index 280843eed637..e8dfcf822174 100644
--- a/spec/lib/open_project/journal_formatter/attachment_spec.rb
+++ b/spec/lib/open_project/journal_formatter/attachment_spec.rb
@@ -105,5 +105,23 @@ def self.default_url_options
 
       it { expect(instance.render(key, [attachment.filename.to_s, nil], html: false)).to eq(expected) }
     end
+
+    describe "attachment resolution" do
+      it "resolves the attachment through the request cache, querying once across renders" do
+        cache = JournalFormatterCache.new
+
+        recorder = ActiveRecord::QueryRecorder.new do
+          2.times { instance.render(key, [nil, attachment.filename.to_s], html: true, cache:) }
+        end
+
+        attachment_queries = recorder.log.grep(/FROM "attachments"/i)
+        expect(attachment_queries.size).to eq(1)
+      end
+
+      it "still resolves the attachment when no cache is supplied" do
+        expect(instance.render(key, [nil, attachment.filename.to_s], html: true))
+          .to include(attachment.filename.to_s)
+      end
+    end
   end
 end
diff --git a/spec/services/work_packages/activities_tab/paginator_spec.rb b/spec/services/work_packages/activities_tab/paginator_spec.rb
index de941c5202b7..cc3449087ce3 100644
--- a/spec/services/work_packages/activities_tab/paginator_spec.rb
+++ b/spec/services/work_packages/activities_tab/paginator_spec.rb
@@ -283,7 +283,9 @@
 
         # Journals and changesets have independent id sequences, so the kind
         # holding the higher id is determined at runtime — but whichever it
-        # is must come first under the `id DESC` secondary sort.
+        # is must come first under the `id DESC` secondary sort. (Should a
+        # journal and changeset ever collide on both timestamp and id, the
+        # trailing `kind DESC` sort keeps the order deterministic.)
         ordered_pairs = records.map { |r| r.is_a?(Changeset) ? [:changeset, r.id] : [:journal, r.id] }
         journal_pos   = ordered_pairs.index([:journal, tied_journal.id])
         changeset_pos = ordered_pairs.index([:changeset, tied_changeset.id])
@@ -331,6 +333,13 @@
       context "when user cannot see internal comments" do
         let(:member_role) { create(:project_role, permissions: %i[view_work_packages]) }
         let(:member_user) { create(:user, member_with_roles: { project => member_role }) }
+        let(:internal_sequence_version) do
+          Journal
+            .where(journable: work_package)
+            .with_sequence_version
+            .where(id: internal_journal.id)
+            .pick("ranked.sequence_version")
+        end
 
         before do
           allow(User).to receive(:current).and_return(member_user)
@@ -352,11 +361,6 @@
         end
 
         it "falls back to page 1 when anchoring to an internal journal by sequence_version" do
-          internal_sequence_version = Journal
-            .where(journable: work_package)
-            .with_sequence_version
-            .where(id: internal_journal.id)
-            .pick("ranked.sequence_version")
           params[:anchor] = "activity-#{internal_sequence_version}"
           pagy, _records = described_class.new(work_package, params).call
 

From d8e12138c39a20a4309cf94d8a91277b267bcd07 Mon Sep 17 00:00:00 2001
From: Kabiru Mwenja <k.mwenja@openproject.com>
Date: Mon, 1 Jun 2026 08:18:08 +0300
Subject: [PATCH 4/7] Simplify activities tie-break spec to a single ordered
 assertion

Pair one journal with a changeset at the same timestamp so the page holds
exactly two records, letting the test assert the `id DESC` secondary sort
in one line instead of branching on which sequence won at runtime.
---
 .../activities_tab/paginator_spec.rb          | 32 +++++++------------
 1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/spec/services/work_packages/activities_tab/paginator_spec.rb b/spec/services/work_packages/activities_tab/paginator_spec.rb
index cc3449087ce3..b3d85b63e9c4 100644
--- a/spec/services/work_packages/activities_tab/paginator_spec.rb
+++ b/spec/services/work_packages/activities_tab/paginator_spec.rb
@@ -259,14 +259,13 @@
     context "with a journal and a changeset at the same timestamp" do
       let(:repository) { create(:repository_subversion, project:) }
       let(:shared_time) { 1.day.ago }
-      let!(:tied_journal) do
-        create(:work_package_journal,
-               user:,
-               notes: "Tied with changeset",
-               journable: work_package,
-               version: 2,
-               created_at: shared_time)
+      let(:work_package) do
+        create(:work_package,
+               project:,
+               author: user,
+               journals: { shared_time => { notes: "Tied with changeset" } })
       end
+      let(:tied_journal) { work_package.journals.sole }
       let!(:tied_changeset) do
         create(:changeset, repository:, committed_on: shared_time, revision: "tied")
       end
@@ -281,20 +280,11 @@
       it "breaks ties by id descending" do
         _pagy, records = paginator.call
 
-        # Journals and changesets have independent id sequences, so the kind
-        # holding the higher id is determined at runtime — but whichever it
-        # is must come first under the `id DESC` secondary sort. (Should a
-        # journal and changeset ever collide on both timestamp and id, the
-        # trailing `kind DESC` sort keeps the order deterministic.)
-        ordered_pairs = records.map { |r| r.is_a?(Changeset) ? [:changeset, r.id] : [:journal, r.id] }
-        journal_pos   = ordered_pairs.index([:journal, tied_journal.id])
-        changeset_pos = ordered_pairs.index([:changeset, tied_changeset.id])
-
-        if tied_journal.id > tied_changeset.id
-          expect(journal_pos).to be < changeset_pos
-        else
-          expect(changeset_pos).to be < journal_pos
-        end
+        # Journals and changesets draw ids from independent sequences, so which
+        # record holds the higher id is only known at runtime — but the higher
+        # id must lead under the `id DESC` secondary sort.
+        ids = [tied_journal.id, tied_changeset.id]
+        expect(records.map(&:id)).to eq([ids.max, ids.min])
       end
     end
 

From 5a4ef61c19f8065e27c3396945b185e99cb1afb7 Mon Sep 17 00:00:00 2001
From: Kabiru Mwenja <k.mwenja@openproject.com>
Date: Mon, 1 Jun 2026 08:35:54 +0300
Subject: [PATCH 5/7] Clarify activity hydration comments and ref destructuring

Explain why the union is plucked as refs and rehydrated per kind, and name
the tuple slots in the kind grouping so the reassembly reads at a glance.
---
 .../work_packages/activities_tab/paginator.rb    | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/app/services/work_packages/activities_tab/paginator.rb b/app/services/work_packages/activities_tab/paginator.rb
index ec9cfdca53cc..7ae26e08b0a1 100644
--- a/app/services/work_packages/activities_tab/paginator.rb
+++ b/app/services/work_packages/activities_tab/paginator.rb
@@ -165,17 +165,27 @@ def locate_anchor_by_sequence_version(sequence_version)
       .pick(:created_at, :id)
   end
 
-  # The page slice arrives ordered by the database; that ordering is the source
-  # of truth for the feed, and rebuilding the list in ref order preserves it.
+  # The union relation yields only lightweight `(kind, id)` rows in the database's
+  # order — it cannot be materialised into real Journal/Changeset records. So the
+  # page is plucked as ordered refs, the real records are hydrated in one batch per
+  # kind, and the refs are walked once more to emit the records in that same order.
   def load_activities(page_relation)
     ordered_refs = page_relation.pluck(Arel.sql("activities.kind"), Arel.sql("activities.id"))
     records_by_kind = load_page_activities_by_kind(ordered_refs)
 
+    # A ref resolves to nothing if its journal was aggregated away between selecting
+    # the page and loading its records; that ref is skipped rather than left as a gap.
     ordered_refs.filter_map { |kind, id| records_by_kind[kind][id] }
   end
 
+  # Journals and changesets load through separate code paths, so each kind is keyed
+  # on its own. Both keys are always present — empty when the page holds none of that
+  # kind — so the lookup that reassembles the page never nils on a missing kind.
   def load_page_activities_by_kind(ordered_refs)
-    ids_by_kind = ordered_refs.group_by(&:first).transform_values { it.map(&:last) }
+    ids_by_kind = ordered_refs
+      .group_by { |kind, _id| kind }
+      .transform_values { |refs| refs.map { |_kind, id| id } }
+
     {
       ActivitiesQuery::KIND_JOURNAL => load_page_journals(ids_by_kind[ActivitiesQuery::KIND_JOURNAL] || []),
       ActivitiesQuery::KIND_CHANGESET => load_page_changesets(ids_by_kind[ActivitiesQuery::KIND_CHANGESET] || [])

From 5185349dc4e1a6127592986f436535ceaf1ae009 Mon Sep 17 00:00:00 2001
From: Kabiru Mwenja <k.mwenja@openproject.com>
Date: Mon, 1 Jun 2026 08:46:17 +0300
Subject: [PATCH 6/7] Coerce activities page limit and document paginator edge
 cases

Read the page limit as a positive integer so a size arriving as a query string
divides cleanly when counting rows ahead of an anchor, and a non-positive value
floors to one page instead of dividing by zero. Add comments recording why the
page is reversed for ascending display and why pagy receives an explicit request
context inside a service object.
---
 .../work_packages/activities_tab/paginator.rb        | 12 ++++++++++--
 .../work_packages/activities_tab/paginator_spec.rb   |  9 +++++++++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/app/services/work_packages/activities_tab/paginator.rb b/app/services/work_packages/activities_tab/paginator.rb
index 7ae26e08b0a1..09fa64ff46f7 100644
--- a/app/services/work_packages/activities_tab/paginator.rb
+++ b/app/services/work_packages/activities_tab/paginator.rb
@@ -56,7 +56,7 @@
 # @option params [Symbol] :filter Filter mode (:all, :only_comments, :only_changes)
 # @option params [String] :anchor Anchor to navigate to specific journal
 #
-# @return [Array<Pagy, Array>] Pagy pagination object and array of activity records
+# @return [(Pagy, Array)] Pagy pagination object and array of activity records
 class WorkPackages::ActivitiesTab::Paginator
   include Pagy::Method
   include WorkPackages::ActivitiesTab::JournalSortingInquirable
@@ -84,6 +84,8 @@ def call
       end
 
     activities = load_activities(page_relation)
+    # The page always loads newest-first; ascending display depends on this per-page
+    # reversal together with the reversed page order, so neither half stands alone.
     activities = activities.reverse if journal_sorting.asc?
 
     [pagy_obj, activities]
@@ -102,10 +104,16 @@ def visible_journals
     work_package.journals.internal_visible
   end
 
+  # Coerced to a positive integer to match how pagy reads the same option: a page
+  # size arriving as a query string still divides cleanly when counting rows ahead
+  # of an anchor, and a non-positive value floors to one page rather than dividing
+  # by zero.
   def limit
-    params[:limit] || Pagy::DEFAULT[:limit]
+    (params[:limit] || Pagy::DEFAULT[:limit]).to_i.clamp(1..)
   end
 
+  # `request:` supplies pagy the request context it would otherwise look up via a
+  # controller; this runs in a service object, so the params are passed explicitly.
   def pagy_options
     {
       page: params[:page] || 1,
diff --git a/spec/services/work_packages/activities_tab/paginator_spec.rb b/spec/services/work_packages/activities_tab/paginator_spec.rb
index b3d85b63e9c4..dd8ceb332506 100644
--- a/spec/services/work_packages/activities_tab/paginator_spec.rb
+++ b/spec/services/work_packages/activities_tab/paginator_spec.rb
@@ -189,6 +189,15 @@
 
             expect(pagy.page).to eq(1)
           end
+
+          it "resolves the anchor page when the limit arrives as a query string" do
+            params[:anchor] = "comment-#{journal_1.id}"
+            params[:limit] = test_limit.to_s
+            pagy, records = paginator.call
+
+            expect(pagy.page).to eq(2)
+            expect(records.map(&:id)).to include(journal_1.id)
+          end
         end
 
         context "with activity anchor" do

From 79a638f1e82e3f6e0e0dabbdfa4f96a575a72852 Mon Sep 17 00:00:00 2001
From: Kabiru Mwenja <k.mwenja@openproject.com>
Date: Mon, 1 Jun 2026 08:48:08 +0300
Subject: [PATCH 7/7] Refine explainer comments; less jargon and verbosity

---
 .../work_packages/activities_tab/paginator.rb | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/app/services/work_packages/activities_tab/paginator.rb b/app/services/work_packages/activities_tab/paginator.rb
index 09fa64ff46f7..b3ead73cc86e 100644
--- a/app/services/work_packages/activities_tab/paginator.rb
+++ b/app/services/work_packages/activities_tab/paginator.rb
@@ -84,8 +84,7 @@ def call
       end
 
     activities = load_activities(page_relation)
-    # The page always loads newest-first; ascending display depends on this per-page
-    # reversal together with the reversed page order, so neither half stands alone.
+    # For UI display: if user wants "oldest first" UI, reverse the collection
     activities = activities.reverse if journal_sorting.asc?
 
     [pagy_obj, activities]
@@ -104,16 +103,12 @@ def visible_journals
     work_package.journals.internal_visible
   end
 
-  # Coerced to a positive integer to match how pagy reads the same option: a page
-  # size arriving as a query string still divides cleanly when counting rows ahead
-  # of an anchor, and a non-positive value floors to one page rather than dividing
-  # by zero.
+  # Coerced to a positive integer to match how pagy reads the same option:
+  # a non-positive value floors to one page, avoiding a ZeroDivisionError.
   def limit
     (params[:limit] || Pagy::DEFAULT[:limit]).to_i.clamp(1..)
   end
 
-  # `request:` supplies pagy the request context it would otherwise look up via a
-  # controller; this runs in a service object, so the params are passed explicitly.
   def pagy_options
     {
       page: params[:page] || 1,
@@ -155,9 +150,6 @@ def page_for_anchor(scope, anchor_type, target_record_id)
     (rows_ahead / limit) + 1
   end
 
-  # Anchors must observe the same visibility rules as the activities feed.
-  # Otherwise the count-ahead would route an unviewable journal to a page
-  # number and leak the existence of internal journals through the URL.
   def locate_anchor(anchor_type, target_record_id)
     if anchor_type.comment?
       visible_journals.where(id: target_record_id).pick(:created_at, :id)
@@ -173,7 +165,7 @@ def locate_anchor_by_sequence_version(sequence_version)
       .pick(:created_at, :id)
   end
 
-  # The union relation yields only lightweight `(kind, id)` rows in the database's
+  # The union relation yields only shallow `(kind, id)` rows in the database's
   # order — it cannot be materialised into real Journal/Changeset records. So the
   # page is plucked as ordered refs, the real records are hydrated in one batch per
   # kind, and the refs are walked once more to emit the records in that same order.
@@ -186,9 +178,6 @@ def load_activities(page_relation)
     ordered_refs.filter_map { |kind, id| records_by_kind[kind][id] }
   end
 
-  # Journals and changesets load through separate code paths, so each kind is keyed
-  # on its own. Both keys are always present — empty when the page holds none of that
-  # kind — so the lookup that reassembles the page never nils on a missing kind.
   def load_page_activities_by_kind(ordered_refs)
     ids_by_kind = ordered_refs
       .group_by { |kind, _id| kind }