fix(timeline): off-by-one — exact-limit comments no longer triggers Show older

Pre-fix the gate was `len(comments) >= limit`, which fired even when the
issue had EXACTLY <limit> comments. The "Show older" affordance appeared,
the user clicked, the next page fetched zero rows. User flagged it on
MUL-1857 — "this issue happens to have 30 comments; the button shouldn't
appear in that case."

The fix is the standard over-fetch probe: ask the SQL for limit+1 rows; if
it returned more than limit, drop the extra and report hasMore=true.
Otherwise hasMore=false.

- New helper `commentOverflow(rows, limit) -> ([]db.Comment, bool)` replaces
  the count-based `hasMoreCommentsBeyond`. Works for both DESC (latest /
  before) and ASC (after / around-newer) since both want "keep first
  <limit>".
- All four mode handlers (latest, before, after, around) now ask for
  limit+1 comments and route through the helper.
- Activities still cap at <limit> with no overflow probe — they don't gate
  pagination (#1857), so the boundary doesn't matter for them.

Tests:
- TestCommentOverflow pins the truth table with the boundary case
  ("exactly limit comments" → hasMore=false).
- TestListTimeline_ExactlyLimitCommentsHidesShowOlder is the DB-backed
  regression: 30 comments, limit=30, asserts has_more_before=false and
  next_cursor=nil.

Co-authored-by: multica-agent <github@multica.ai>

packages/core/api/client.ts

@@ -519,7 +519,7 @@ export class ApiClient {
   async listTimeline(
     issueId: string,
     pageParam: TimelinePageParam = { mode: "latest" },
-    limit = 50,
+    limit = 30,
   ): Promise<TimelinePage> {
     const params = new URLSearchParams();
     params.set("limit", String(limit));

server/internal/handler/activity.go

@@ -55,39 +55,103 @@ type TimelineResponse struct {
 }
 
 const (
-	timelineDefaultLimit = 50
+	// timelineDefaultLimit governs the per-page COMMENT budget. Activities are
+	// fetched at the same per-call cap but do not consume the budget (#1857) —
+	// they decorate the comment stream. Without that split, an issue with
+	// sparse comments but dense activity (agent runs, status flips) triggered
+	// "show older" prematurely and felt like comments had vanished.
+	timelineDefaultLimit = 30
 	timelineMaxLimit     = 100
 )
 
-// timelineCursor encodes a (created_at, id) keyset position as opaque base64
-// JSON. The format is intentionally hidden from clients so future schema
-// evolution (e.g. switching to a sequence column) can replace the cursor
-// payload without breaking API consumers.
-type timelineCursor struct {
-	CreatedAt time.Time `json:"t"`
-	ID        string    `json:"i"`
+// cursorPos is a single (created_at, id) keyset position. Used per-pool —
+// see timelineCursor.
+type cursorPos struct {
+	T  pgtype.Timestamptz
+	ID pgtype.UUID
 }
 
-func encodeTimelineCursor(t pgtype.Timestamptz, id pgtype.UUID) string {
-	c := timelineCursor{CreatedAt: t.Time, ID: uuidToString(id)}
+// timelineCursor encodes per-pool keyset positions as opaque base64 JSON.
+// Comments and activities walk independently (#1857 follow-up): a single
+// shared cursor anchored on the merged-page boundary would let an activity
+// older than every visible comment hide all unreturned comments behind it,
+// since `ListCommentsBefore(activityCursor)` would skip the in-between rows.
+// The format is intentionally hidden from clients so future schema evolution
+// can replace the payload without breaking API consumers.
+type timelineCursor struct {
+	CommentT   time.Time `json:"ct"`
+	CommentID  string    `json:"ci"`
+	ActivityT  time.Time `json:"at"`
+	ActivityID string    `json:"ai"`
+}
+
+func encodeTimelineCursor(comment, activity cursorPos) string {
+	c := timelineCursor{
+		CommentT:   comment.T.Time,
+		CommentID:  uuidToString(comment.ID),
+		ActivityT:  activity.T.Time,
+		ActivityID: uuidToString(activity.ID),
+	}
 	b, _ := json.Marshal(c)
 	return base64.RawURLEncoding.EncodeToString(b)
 }
 
-func decodeTimelineCursor(s string) (pgtype.Timestamptz, pgtype.UUID, error) {
+func decodeTimelineCursor(s string) (comment, activity cursorPos, err error) {
 	raw, err := base64.RawURLEncoding.DecodeString(s)
 	if err != nil {
-		return pgtype.Timestamptz{}, pgtype.UUID{}, err
+		return cursorPos{}, cursorPos{}, err
 	}
 	var c timelineCursor
-	if err := json.Unmarshal(raw, &c); err != nil {
-		return pgtype.Timestamptz{}, pgtype.UUID{}, err
+	if err = json.Unmarshal(raw, &c); err != nil {
+		return cursorPos{}, cursorPos{}, err
 	}
-	id, err := parseUUIDStrict(c.ID)
+	cid, err := parseUUIDStrict(c.CommentID)
 	if err != nil {
-		return pgtype.Timestamptz{}, pgtype.UUID{}, err
+		return cursorPos{}, cursorPos{}, err
 	}
-	return pgtype.Timestamptz{Time: c.CreatedAt, Valid: true}, id, nil
+	aid, err := parseUUIDStrict(c.ActivityID)
+	if err != nil {
+		return cursorPos{}, cursorPos{}, err
+	}
+	return cursorPos{T: pgtype.Timestamptz{Time: c.CommentT, Valid: true}, ID: cid},
+		cursorPos{T: pgtype.Timestamptz{Time: c.ActivityT, Valid: true}, ID: aid},
+		nil
+}
+
+// commentBoundsDesc returns (oldest, newest) cursor positions from a DESC-
+// ordered comment slice. If the slice is empty, returns the supplied carry
+// position so the cursor walker keeps advancing the empty pool past
+// boundaries the caller already paged through.
+func commentBoundsDesc(rows []db.Comment, carry cursorPos) (oldest, newest cursorPos) {
+	if len(rows) == 0 {
+		return carry, carry
+	}
+	return cursorPos{T: rows[len(rows)-1].CreatedAt, ID: rows[len(rows)-1].ID},
+		cursorPos{T: rows[0].CreatedAt, ID: rows[0].ID}
+}
+
+func commentBoundsAsc(rows []db.Comment, carry cursorPos) (oldest, newest cursorPos) {
+	if len(rows) == 0 {
+		return carry, carry
+	}
+	return cursorPos{T: rows[0].CreatedAt, ID: rows[0].ID},
+		cursorPos{T: rows[len(rows)-1].CreatedAt, ID: rows[len(rows)-1].ID}
+}
+
+func activityBoundsDesc(rows []db.ActivityLog, carry cursorPos) (oldest, newest cursorPos) {
+	if len(rows) == 0 {
+		return carry, carry
+	}
+	return cursorPos{T: rows[len(rows)-1].CreatedAt, ID: rows[len(rows)-1].ID},
+		cursorPos{T: rows[0].CreatedAt, ID: rows[0].ID}
+}
+
+func activityBoundsAsc(rows []db.ActivityLog, carry cursorPos) (oldest, newest cursorPos) {
+	if len(rows) == 0 {
+		return carry, carry
+	}
+	return cursorPos{T: rows[0].CreatedAt, ID: rows[0].ID},
+		cursorPos{T: rows[len(rows)-1].CreatedAt, ID: rows[len(rows)-1].ID}
 }
 
 // parseUUIDStrict mirrors util.ParseUUID but returns a pgtype.UUID directly
@@ -190,7 +254,10 @@ func (h *Handler) listTimelineLegacy(w http.ResponseWriter, r *http.Request, iss
 		writeError(w, http.StatusInternalServerError, "failed to list activities")
 		return
 	}
-	entries := h.mergeTimelineDesc(r, comments, activities, legacyTimelineCap)
+	entries := h.mergeTimelineDesc(r, comments, activities)
+	if len(entries) > legacyTimelineCap {
+		entries = entries[:legacyTimelineCap]
+	}
 	// Old contract: ASC (oldest → newest).
 	for i, j := 0, len(entries)-1; i < j; i, j = i+1, j-1 {
 		entries[i], entries[j] = entries[j], entries[i]
@@ -203,19 +270,24 @@ func (h *Handler) listTimelineLegacy(w http.ResponseWriter, r *http.Request, iss
 	writeJSON(w, http.StatusOK, entries)
 }
 
-// listTimelineLatest fetches the most recent <limit> entries (no cursor).
-// Both tables are queried for <limit> rows each; the merge picks the top
-// <limit> overall. Any item the merge didn't include cannot rank higher than
-// the worst kept item in either pool, so this is exact, not approximate.
+// listTimelineLatest fetches the latest page (no cursor). <limit> is the
+// COMMENT page size (#1857); activity rows ride along at the same per-call
+// SQL cap but do not consume the page budget — has_more_before is gated on
+// comments alone, so a chatty agent's status flips can't push real comments
+// off-page.
 func (h *Handler) listTimelineLatest(w http.ResponseWriter, r *http.Request, issue db.Issue, limit int) {
 	ctx := r.Context()
-	comments, err := h.Queries.ListCommentsLatest(ctx, db.ListCommentsLatestParams{
-		IssueID: issue.ID, WorkspaceID: issue.WorkspaceID, Limit: int32(limit),
+	// Over-fetch comments by one so commentOverflow can distinguish "exactly
+	// <limit> comments exist" (no Show older needed) from ">limit comments
+	// exist" (Show older required).
+	rawComments, err := h.Queries.ListCommentsLatest(ctx, db.ListCommentsLatestParams{
+		IssueID: issue.ID, WorkspaceID: issue.WorkspaceID, Limit: int32(limit + 1),
 	})
 	if err != nil {
 		writeError(w, http.StatusInternalServerError, "failed to list comments")
 		return
 	}
+	comments, hasMoreComments := commentOverflow(rawComments, limit)
 	activities, err := h.Queries.ListActivitiesLatest(ctx, db.ListActivitiesLatestParams{
 		IssueID: issue.ID, Limit: int32(limit),
 	})
@@ -224,15 +296,30 @@ func (h *Handler) listTimelineLatest(w http.ResponseWriter, r *http.Request, iss
 		return
 	}
 
-	entries := h.mergeTimelineDesc(r, comments, activities, limit)
+	entries := h.mergeTimelineDesc(r, comments, activities)
 	resp := TimelineResponse{Entries: entries}
-	resp.HasMoreBefore = hasMoreBeyond(len(comments), len(activities), len(entries), limit)
+	resp.HasMoreBefore = hasMoreComments
+
+	// Per-pool boundaries. For latest mode there is no input cursor; if a
+	// pool returned no rows it carries from the other pool so the encoded
+	// payload stays self-contained. Future calls won't fetch new rows for
+	// the empty pool anyway (latest with 0 of one type means the issue has
+	// none), so the carry value is purely cosmetic.
+	cOldest, cNewest := commentBoundsDesc(comments, cursorPos{})
+	aOldest, aNewest := activityBoundsDesc(activities, cursorPos{})
+	if len(comments) == 0 {
+		cOldest, cNewest = aOldest, aNewest
+	}
+	if len(activities) == 0 {
+		aOldest, aNewest = cOldest, cNewest
+	}
+
 	if resp.HasMoreBefore && len(entries) > 0 {
-		c := encodeTimelineCursor(entryTimestamp(entries[len(entries)-1]), entryID(entries[len(entries)-1]))
+		c := encodeTimelineCursor(cOldest, aOldest)
 		resp.NextCursor = &c
 	}
 	if len(entries) > 0 {
-		c := encodeTimelineCursor(entryTimestamp(entries[0]), entryID(entries[0]))
+		c := encodeTimelineCursor(cNewest, aNewest)
 		resp.PrevCursor = &c
 	}
 	// has_more_after is always false on the latest page by definition.
@@ -241,40 +328,48 @@ func (h *Handler) listTimelineLatest(w http.ResponseWriter, r *http.Request, iss
 
 func (h *Handler) listTimelineBefore(w http.ResponseWriter, r *http.Request, issue db.Issue, cursor string, limit int) {
 	ctx := r.Context()
-	t, id, err := decodeTimelineCursor(cursor)
+	inComment, inActivity, err := decodeTimelineCursor(cursor)
 	if err != nil {
 		writeError(w, http.StatusBadRequest, "invalid cursor")
 		return
 	}
 
-	comments, err := h.Queries.ListCommentsBefore(ctx, db.ListCommentsBeforeParams{
+	rawComments, err := h.Queries.ListCommentsBefore(ctx, db.ListCommentsBeforeParams{
 		IssueID: issue.ID, WorkspaceID: issue.WorkspaceID,
-		Column3: t, Column4: id, Limit: int32(limit),
+		Column3: inComment.T, Column4: inComment.ID, Limit: int32(limit + 1),
 	})
 	if err != nil {
 		writeError(w, http.StatusInternalServerError, "failed to list comments")
 		return
 	}
+	comments, hasMoreComments := commentOverflow(rawComments, limit)
 	activities, err := h.Queries.ListActivitiesBefore(ctx, db.ListActivitiesBeforeParams{
-		IssueID: issue.ID, Column2: t, Column3: id, Limit: int32(limit),
+		IssueID: issue.ID, Column2: inActivity.T, Column3: inActivity.ID, Limit: int32(limit),
 	})
 	if err != nil {
 		writeError(w, http.StatusInternalServerError, "failed to list activities")
 		return
 	}
 
-	entries := h.mergeTimelineDesc(r, comments, activities, limit)
+	entries := h.mergeTimelineDesc(r, comments, activities)
 	resp := TimelineResponse{
 		Entries:      entries,
 		HasMoreAfter: true, // we're paging older from a known position, so newer exists
 	}
-	resp.HasMoreBefore = hasMoreBeyond(len(comments), len(activities), len(entries), limit)
+	resp.HasMoreBefore = hasMoreComments
+
+	// Per-pool boundaries. Empty pool carries forward from the input cursor
+	// so subsequent older pages keep advancing past previously-paginated rows
+	// in that pool.
+	cOldest, cNewest := commentBoundsDesc(comments, inComment)
+	aOldest, aNewest := activityBoundsDesc(activities, inActivity)
+
 	if resp.HasMoreBefore && len(entries) > 0 {
-		c := encodeTimelineCursor(entryTimestamp(entries[len(entries)-1]), entryID(entries[len(entries)-1]))
+		c := encodeTimelineCursor(cOldest, aOldest)
 		resp.NextCursor = &c
 	}
 	if len(entries) > 0 {
-		c := encodeTimelineCursor(entryTimestamp(entries[0]), entryID(entries[0]))
+		c := encodeTimelineCursor(cNewest, aNewest)
 		resp.PrevCursor = &c
 	}
 	writeJSON(w, http.StatusOK, resp)
@@ -282,40 +377,49 @@ func (h *Handler) listTimelineBefore(w http.ResponseWriter, r *http.Request, iss
 
 func (h *Handler) listTimelineAfter(w http.ResponseWriter, r *http.Request, issue db.Issue, cursor string, limit int) {
 	ctx := r.Context()
-	t, id, err := decodeTimelineCursor(cursor)
+	inComment, inActivity, err := decodeTimelineCursor(cursor)
 	if err != nil {
 		writeError(w, http.StatusBadRequest, "invalid cursor")
 		return
 	}
 
-	comments, err := h.Queries.ListCommentsAfter(ctx, db.ListCommentsAfterParams{
+	rawComments, err := h.Queries.ListCommentsAfter(ctx, db.ListCommentsAfterParams{
 		IssueID: issue.ID, WorkspaceID: issue.WorkspaceID,
-		Column3: t, Column4: id, Limit: int32(limit),
+		Column3: inComment.T, Column4: inComment.ID, Limit: int32(limit + 1),
 	})
 	if err != nil {
 		writeError(w, http.StatusInternalServerError, "failed to list comments")
 		return
 	}
+	// ASC fetch returns oldest-first; trimming to the first <limit> keeps
+	// the rows closest to the cursor and drops the (limit+1)th newest as the
+	// overflow probe.
+	comments, hasMoreComments := commentOverflow(rawComments, limit)
 	activities, err := h.Queries.ListActivitiesAfter(ctx, db.ListActivitiesAfterParams{
-		IssueID: issue.ID, Column2: t, Column3: id, Limit: int32(limit),
+		IssueID: issue.ID, Column2: inActivity.T, Column3: inActivity.ID, Limit: int32(limit),
 	})
 	if err != nil {
 		writeError(w, http.StatusInternalServerError, "failed to list activities")
 		return
 	}
 
-	// Both queries returned ASC (older→newer). Merge ASC, take the limit
-	// closest to the cursor (i.e. the oldest of the "after" set), then
-	// reverse to DESC for the response.
-	entries := h.mergeTimelineAscThenReverse(r, comments, activities, limit)
+	// Both queries returned ASC (older→newer); reverse to DESC for the
+	// response. No outer truncation: each pool is already capped by the SQL
+	// LIMIT, and dropping rows here would re-introduce the comments-pushed-
+	// off-page bug (#1857).
+	entries := h.mergeTimelineAscThenReverse(r, comments, activities)
 	resp := TimelineResponse{Entries: entries, HasMoreBefore: true}
-	resp.HasMoreAfter = hasMoreBeyond(len(comments), len(activities), len(entries), limit)
+	resp.HasMoreAfter = hasMoreComments
+
+	cOldest, cNewest := commentBoundsAsc(comments, inComment)
+	aOldest, aNewest := activityBoundsAsc(activities, inActivity)
+
 	if resp.HasMoreAfter && len(entries) > 0 {
-		c := encodeTimelineCursor(entryTimestamp(entries[0]), entryID(entries[0]))
+		c := encodeTimelineCursor(cNewest, aNewest)
 		resp.PrevCursor = &c
 	}
 	if len(entries) > 0 {
-		c := encodeTimelineCursor(entryTimestamp(entries[len(entries)-1]), entryID(entries[len(entries)-1]))
+		c := encodeTimelineCursor(cOldest, aOldest)
 		resp.NextCursor = &c
 	}
 	writeJSON(w, http.StatusOK, resp)
@@ -365,15 +469,17 @@ func (h *Handler) listTimelineAround(w http.ResponseWriter, r *http.Request, iss
 		afterLimit = 0
 	}
 
-	// Older half: keyset Before (anchor exclusive).
-	olderComments, err := h.Queries.ListCommentsBefore(ctx, db.ListCommentsBeforeParams{
+	// Older half: keyset Before (anchor exclusive). Over-fetch comments by
+	// one to detect overflow exactly.
+	rawOlderComments, err := h.Queries.ListCommentsBefore(ctx, db.ListCommentsBeforeParams{
 		IssueID: issue.ID, WorkspaceID: issue.WorkspaceID,
-		Column3: anchorTime, Column4: anchorID, Limit: int32(beforeLimit),
+		Column3: anchorTime, Column4: anchorID, Limit: int32(beforeLimit + 1),
 	})
 	if err != nil {
 		writeError(w, http.StatusInternalServerError, "failed to list comments")
 		return
 	}
+	olderComments, hasMoreOlderComments := commentOverflow(rawOlderComments, beforeLimit)
 	olderActivities, err := h.Queries.ListActivitiesBefore(ctx, db.ListActivitiesBeforeParams{
 		IssueID: issue.ID, Column2: anchorTime, Column3: anchorID, Limit: int32(beforeLimit),
 	})
@@ -381,17 +487,18 @@ func (h *Handler) listTimelineAround(w http.ResponseWriter, r *http.Request, iss
 		writeError(w, http.StatusInternalServerError, "failed to list activities")
 		return
 	}
-	olderEntries := h.mergeTimelineDesc(r, olderComments, olderActivities, beforeLimit)
+	olderEntries := h.mergeTimelineDesc(r, olderComments, olderActivities)
 
 	// Newer half: keyset After (anchor exclusive).
-	newerComments, err := h.Queries.ListCommentsAfter(ctx, db.ListCommentsAfterParams{
+	rawNewerComments, err := h.Queries.ListCommentsAfter(ctx, db.ListCommentsAfterParams{
 		IssueID: issue.ID, WorkspaceID: issue.WorkspaceID,
-		Column3: anchorTime, Column4: anchorID, Limit: int32(afterLimit),
+		Column3: anchorTime, Column4: anchorID, Limit: int32(afterLimit + 1),
 	})
 	if err != nil {
 		writeError(w, http.StatusInternalServerError, "failed to list comments")
 		return
 	}
+	newerComments, hasMoreNewerComments := commentOverflow(rawNewerComments, afterLimit)
 	newerActivities, err := h.Queries.ListActivitiesAfter(ctx, db.ListActivitiesAfterParams{
 		IssueID: issue.ID, Column2: anchorTime, Column3: anchorID, Limit: int32(afterLimit),
 	})
@@ -399,7 +506,7 @@ func (h *Handler) listTimelineAround(w http.ResponseWriter, r *http.Request, iss
 		writeError(w, http.StatusInternalServerError, "failed to list activities")
 		return
 	}
-	newerEntries := h.mergeTimelineAscThenReverse(r, newerComments, newerActivities, afterLimit)
+	newerEntries := h.mergeTimelineAscThenReverse(r, newerComments, newerActivities)
 
 	// Build the anchor entry inline using the existing single-entry path.
 	anchorEntry, ok := h.fetchSingleEntry(r, issue, target)
@@ -417,16 +524,26 @@ func (h *Handler) listTimelineAround(w http.ResponseWriter, r *http.Request, iss
 
 	resp := TimelineResponse{
 		Entries:       entries,
-		HasMoreBefore: hasMoreBeyond(len(olderComments), len(olderActivities), len(olderEntries), beforeLimit),
-		HasMoreAfter:  hasMoreBeyond(len(newerComments), len(newerActivities), len(newerEntries), afterLimit),
+		HasMoreBefore: hasMoreOlderComments,
+		HasMoreAfter:  hasMoreNewerComments,
 		TargetIndex:   &targetIdx,
 	}
+
+	// Per-pool boundaries on each half. Empty pools fall back to the anchor
+	// position, which is exclusive on both sides — so a follow-up Before /
+	// After call against the anchor returns no duplicates.
+	anchor := cursorPos{T: anchorTime, ID: anchorID}
+	olderCommentOldest, _ := commentBoundsDesc(olderComments, anchor)
+	olderActivityOldest, _ := activityBoundsDesc(olderActivities, anchor)
+	_, newerCommentNewest := commentBoundsAsc(newerComments, anchor)
+	_, newerActivityNewest := activityBoundsAsc(newerActivities, anchor)
+
 	if resp.HasMoreBefore {
-		c := encodeTimelineCursor(entryTimestamp(entries[len(entries)-1]), entryID(entries[len(entries)-1]))
+		c := encodeTimelineCursor(olderCommentOldest, olderActivityOldest)
 		resp.NextCursor = &c
 	}
 	if resp.HasMoreAfter {
-		c := encodeTimelineCursor(entryTimestamp(entries[0]), entryID(entries[0]))
+		c := encodeTimelineCursor(newerCommentNewest, newerActivityNewest)
 		resp.PrevCursor = &c
 	}
 	writeJSON(w, http.StatusOK, resp)
@@ -449,28 +566,32 @@ func (h *Handler) fetchSingleEntry(r *http.Request, issue db.Issue, id pgtype.UU
 	return TimelineEntry{}, false
 }
 
-// hasMoreBeyond reports whether entries exist beyond the page on the side the
-// caller is paginating away from (older for "before", newer for "after").
+// commentOverflow trims an over-fetched comment slice to <limit> and reports
+// whether the SQL returned more rows than the visible budget. Callers
+// over-fetch by one (limit+1) so the boolean is exact even when the issue has
+// EXACTLY <limit> comments — the prior `len >= limit` check returned true in
+// that case and rendered a "Show older" affordance that revealed nothing.
 //
-// Three independent signals, any of which means "more rows exist":
-//  1. comments >= limit — the comments query was capped, DB has more.
-//  2. activities >= limit — the activities query was capped, DB has more.
-//  3. comments+activities > entries — the in-memory merge dropped rows that
-//     could not all fit in the page (#2192). This is the case the original
-//     formula missed, which made older comments unreachable when neither
-//     individual query hit the limit but their combined total exceeded it.
-func hasMoreBeyond(comments, activities, entries, limit int) bool {
+// Activity rows do not gate pagination (#1857): a dense activity stream from
+// agent runs / status flips would otherwise trigger "show older" on issues
+// with only a handful of real comments. Activities therefore stay capped at
+// <limit> with no overflow probe.
+func commentOverflow(rows []db.Comment, limit int) ([]db.Comment, bool) {
 	if limit <= 0 {
-		return false
+		return rows, false
 	}
-	return comments >= limit || activities >= limit || comments+activities > entries
+	if len(rows) > limit {
+		return rows[:limit], true
+	}
+	return rows, false
 }
 
-// mergeTimelineDesc takes comments + activities sorted DESC by (created_at, id)
-// and returns the top <limit> merged entries, also DESC. Items the merge does
-// not include cannot rank higher than the worst kept item in either pool, so
-// the result is exact.
-func (h *Handler) mergeTimelineDesc(r *http.Request, comments []db.Comment, activities []db.ActivityLog, limit int) []TimelineEntry {
+// mergeTimelineDesc returns comments + activities merged DESC by
+// (created_at, id). No truncation: both pools are individually capped at the
+// SQL layer, and dropping rows here would re-introduce the bug where dense
+// activity pushed real comments off-page (#1857). Callers that need an outer
+// safety cap (legacy compat path) apply it themselves.
+func (h *Handler) mergeTimelineDesc(r *http.Request, comments []db.Comment, activities []db.ActivityLog) []TimelineEntry {
 	out := make([]TimelineEntry, 0, len(comments)+len(activities))
 	out = append(out, h.commentsToEntries(r, comments)...)
 	for _, a := range activities {
@@ -482,17 +603,14 @@ func (h *Handler) mergeTimelineDesc(r *http.Request, comments []db.Comment, acti
 		}
 		return out[i].ID > out[j].ID
 	})
-	if len(out) > limit {
-		out = out[:limit]
-	}
 	return out
 }
 
 // mergeTimelineAscThenReverse takes comments + activities sorted ASC by
-// (created_at, id) — the natural shape of an "after" keyset query — picks
-// the <limit> closest to the cursor (i.e. earliest of the after-set), and
-// returns them DESC for response consistency.
-func (h *Handler) mergeTimelineAscThenReverse(r *http.Request, comments []db.Comment, activities []db.ActivityLog, limit int) []TimelineEntry {
+// (created_at, id) — the natural shape of an "after" keyset query — and
+// returns them DESC for response consistency. No truncation, same reason as
+// mergeTimelineDesc.
+func (h *Handler) mergeTimelineAscThenReverse(r *http.Request, comments []db.Comment, activities []db.ActivityLog) []TimelineEntry {
 	out := make([]TimelineEntry, 0, len(comments)+len(activities))
 	out = append(out, h.commentsToEntries(r, comments)...)
 	for _, a := range activities {
@@ -504,9 +622,6 @@ func (h *Handler) mergeTimelineAscThenReverse(r *http.Request, comments []db.Com
 		}
 		return out[i].ID < out[j].ID
 	})
-	if len(out) > limit {
-		out = out[:limit]
-	}
 	// Reverse to DESC.
 	for i, j := 0, len(out)-1; i < j; i, j = i+1, j-1 {
 		out[i], out[j] = out[j], out[i]
@@ -567,19 +682,6 @@ func activityToEntry(a db.ActivityLog) TimelineEntry {
 	}
 }
 
-// entryTimestamp / entryID extract the cursor components for an emitted
-// TimelineEntry. CreatedAt is already an RFC3339 string at this point;
-// re-parse it for cursor encoding.
-func entryTimestamp(e TimelineEntry) pgtype.Timestamptz {
-	t, _ := time.Parse(time.RFC3339Nano, e.CreatedAt)
-	return pgtype.Timestamptz{Time: t, Valid: true}
-}
-
-func entryID(e TimelineEntry) pgtype.UUID {
-	id, _ := parseUUIDStrict(e.ID)
-	return id
-}
-
 // AssigneeFrequencyEntry represents how often a user assigns to a specific target.
 type AssigneeFrequencyEntry struct {
 	AssigneeType string `json:"assignee_type"`

server/internal/handler/activity_test.go

@@ -9,6 +9,9 @@ import (
 	"strings"
 	"testing"
 	"time"
+
+	"github.com/jackc/pgx/v5/pgtype"
+	db "github.com/multica-ai/multica/server/pkg/db/generated"
 )
 
 // fetchTimeline issues a GET /timeline request with the given query string and
@@ -92,19 +95,22 @@ func seedTimelineEntries(t *testing.T, issueID string, commentN, activityN int)
 
 func TestListTimeline_DefaultLatestPage(t *testing.T) {
 	issueID := createIssueForTimeline(t, "Latest page test")
-	seedTimelineEntries(t, issueID, 60, 60) // 120 total; default limit is 50
+	// 80 comments triggers the comment overflow signal; activities are
+	// excluded so the per-page count is unambiguous (#1857: activities don't
+	// gate has_more_before).
+	seedTimelineEntries(t, issueID, 80, 0)
 
 	// Empty query string is now reserved for the legacy compat path; new
 	// client always sends ?limit=... so emulate that here.
-	resp, code := fetchTimeline(t, issueID, "limit=50")
+	resp, code := fetchTimeline(t, issueID, "limit=30")
 	if code != http.StatusOK {
 		t.Fatalf("expected 200, got %d", code)
 	}
-	if len(resp.Entries) != 50 {
-		t.Fatalf("expected 50 entries on default page, got %d", len(resp.Entries))
+	if len(resp.Entries) != 30 {
+		t.Fatalf("expected 30 entries on default page, got %d", len(resp.Entries))
 	}
 	if !resp.HasMoreBefore {
-		t.Fatalf("expected has_more_before=true with 120 total entries")
+		t.Fatalf("expected has_more_before=true with 80 comments")
 	}
 	if resp.HasMoreAfter {
 		t.Fatalf("latest page must report has_more_after=false")
@@ -121,7 +127,9 @@ func TestListTimeline_DefaultLatestPage(t *testing.T) {
 
 func TestListTimeline_BeforeCursorWalksOlder(t *testing.T) {
 	issueID := createIssueForTimeline(t, "Before cursor test")
-	seedTimelineEntries(t, issueID, 30, 30) // 60 total
+	// Comments-only so the page-count assertions are stable. limit=20 →
+	// 20 comments per page, no activities to inflate the totals.
+	seedTimelineEntries(t, issueID, 60, 0)
 
 	first, _ := fetchTimeline(t, issueID, "limit=20")
 	if len(first.Entries) != 20 {
@@ -152,7 +160,9 @@ func TestListTimeline_BeforeCursorWalksOlder(t *testing.T) {
 
 func TestListTimeline_AfterCursorWalksNewer(t *testing.T) {
 	issueID := createIssueForTimeline(t, "After cursor test")
-	seedTimelineEntries(t, issueID, 30, 30)
+	// Comments-only seed so cursor walking depends on comment overflow alone
+	// (#1857: activities don't gate has_more_after either).
+	seedTimelineEntries(t, issueID, 60, 0)
 
 	first, _ := fetchTimeline(t, issueID, "limit=20")
 	if first.NextCursor == nil {
@@ -331,98 +341,226 @@ func TestListTimeline_LegacyShapeForPreCursorClients(t *testing.T) {
 	}
 }
 
-// TestHasMoreBeyond covers the truth table for the page-boundary helper. The
-// 8 rows enumerate every meaningful combination of (per-table cap hit) ×
-// (merge truncation), including the #2192 shape (case "merge truncation
-// without per-table cap") which the original formula missed.
-func TestHasMoreBeyond(t *testing.T) {
-	cases := []struct {
-		name                                 string
-		comments, activities, entries, limit int
-		want                                 bool
+// TestTimelineCursor_RoundTrip pins the dual-pool cursor format. Cursors carry
+// independent comment and activity positions (#1857 follow-up) so future
+// pages walk each pool past its own boundary instead of skipping rows when
+// one pool's oldest is older than the other's.
+func TestTimelineCursor_RoundTrip(t *testing.T) {
+	cT := time.Date(2026, 5, 1, 12, 0, 0, 0, time.UTC)
+	aT := time.Date(2026, 5, 1, 11, 0, 0, 0, time.UTC)
+	cID, _ := parseUUIDStrict("11111111-1111-1111-1111-111111111111")
+	aID, _ := parseUUIDStrict("22222222-2222-2222-2222-222222222222")
+
+	in := struct {
+		comment, activity cursorPos
 	}{
-		{"empty page", 0, 0, 0, 50, false},
-		{"partial page no truncation", 5, 3, 8, 50, false},
-		{"comments hit limit only", 50, 3, 50, 50, true},
-		{"activities hit limit only", 3, 50, 50, 50, true},
-		{"both hit limit", 50, 50, 50, 50, true},
-		// #2192: 48 comments + 49 activities, neither alone hits 50, merge
-		// truncated 47 rows. Old formula reported false; new reports true.
-		{"#2192 merge truncation", 48, 49, 50, 50, true},
-		{"exact-fit merge no truncation", 30, 20, 50, 50, false},
-		{"limit zero rejects", 100, 100, 0, 0, false},
+		comment:  cursorPos{T: pgtype.Timestamptz{Time: cT, Valid: true}, ID: cID},
+		activity: cursorPos{T: pgtype.Timestamptz{Time: aT, Valid: true}, ID: aID},
+	}
+
+	encoded := encodeTimelineCursor(in.comment, in.activity)
+	gotC, gotA, err := decodeTimelineCursor(encoded)
+	if err != nil {
+		t.Fatalf("decode: %v", err)
+	}
+	if !gotC.T.Time.Equal(cT) || !gotA.T.Time.Equal(aT) {
+		t.Fatalf("timestamps did not round-trip: comment=%s activity=%s", gotC.T.Time, gotA.T.Time)
+	}
+	if uuidToString(gotC.ID) != "11111111-1111-1111-1111-111111111111" {
+		t.Fatalf("comment id did not round-trip: %s", uuidToString(gotC.ID))
+	}
+	if uuidToString(gotA.ID) != "22222222-2222-2222-2222-222222222222" {
+		t.Fatalf("activity id did not round-trip: %s", uuidToString(gotA.ID))
+	}
+
+	// Garbage cursor → error, never panics.
+	if _, _, err := decodeTimelineCursor("not-base64"); err == nil {
+		t.Fatalf("expected decode error for garbage input")
+	}
+}
+
+// TestCommentOverflow pins the over-fetch / trim contract that gates "Show
+// older". Callers query the SQL with limit+1 and pass the raw rows in; the
+// helper trims to <limit> and reports hasMore. The boundary the user flagged
+// — exactly <limit> comments exist — must report hasMore=false so no
+// affordance appears for content that doesn't exist.
+func TestCommentOverflow(t *testing.T) {
+	mk := func(n int) []db.Comment {
+		out := make([]db.Comment, n)
+		return out
+	}
+	cases := []struct {
+		name        string
+		fetched     int // rows the SQL returned (caller asked for limit+1)
+		limit       int
+		wantTrimmed int
+		wantMore    bool
+	}{
+		{"empty page", 0, 30, 0, false},
+		{"partial page", 5, 30, 5, false},
+		// Issue has exactly limit comments — caller asked for limit+1 and got
+		// only limit back. No older content; "Show older" must NOT appear.
+		{"exactly limit comments", 30, 30, 30, false},
+		// Issue has more than limit — caller asked for limit+1 and got
+		// limit+1 back. Trim the probe row, set hasMore=true.
+		{"one over limit", 31, 30, 30, true},
+		{"well over limit", 100, 30, 30, true},
+		{"limit zero rejects", 100, 0, 100, false},
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
-			if got := hasMoreBeyond(tc.comments, tc.activities, tc.entries, tc.limit); got != tc.want {
-				t.Fatalf("hasMoreBeyond(c=%d, a=%d, e=%d, lim=%d) = %v, want %v",
-					tc.comments, tc.activities, tc.entries, tc.limit, got, tc.want)
+			rows, more := commentOverflow(mk(tc.fetched), tc.limit)
+			if len(rows) != tc.wantTrimmed {
+				t.Fatalf("trimmed length: got %d, want %d", len(rows), tc.wantTrimmed)
+			}
+			if more != tc.wantMore {
+				t.Fatalf("hasMore: got %v, want %v", more, tc.wantMore)
 			}
 		})
 	}
 }
 
-// TestListTimeline_MergeTruncationKeepsOlderReachable reproduces #2192 against
-// real DB rows: 48 comments dated older than 49 activities, default limit 50.
-// Pre-fix, the latest page reported has_more_before=false and the 47 older
-// comments were unreachable. Post-fix, the cursor + has_more_before flag let
-// the client walk to page 2 and recover them.
-func TestListTimeline_MergeTruncationKeepsOlderReachable(t *testing.T) {
-	issueID := createIssueForTimeline(t, "2192 merge truncation regression")
+// TestListTimeline_PerPoolCursorWalksAllComments reproduces the GPT-Boy
+// blocker on PR #2253: when activities sit older than every fetched comment,
+// a single shared cursor anchored on the merged-page boundary points at the
+// oldest activity, and the next "show older" call's `ListCommentsBefore` hits
+// activity time → skips every unreturned comment in between. The dual-pool
+// cursor walks each pool independently so the full comment list stays
+// reachable.
+func TestListTimeline_PerPoolCursorWalksAllComments(t *testing.T) {
+	issueID := createIssueForTimeline(t, "GPT-Boy per-pool cursor regression")
 	ctx := context.Background()
 
-	// 48 older comments, then 49 newer activities. The seedTimelineEntries
-	// helper inserts comments first (older block) then activities (newer
-	// block) — exactly the #2192 shape.
-	commentIDs, _ := seedTimelineEntries(t, issueID, 48, 49)
+	// Seed 30 activities in the older block, then 80 comments strictly newer
+	// than every activity. seedTimelineEntries inserts comments first then
+	// activities, which is the wrong order for this scenario, so seed manually.
+	const activityN, commentN = 30, 80
+	base := time.Now().UTC().Add(-time.Duration(activityN+commentN) * time.Minute)
 
-	first, code := fetchTimeline(t, issueID, "limit=50")
-	if code != http.StatusOK {
-		t.Fatalf("first page: expected 200, got %d", code)
-	}
-	if len(first.Entries) != 50 {
-		t.Fatalf("first page should be full at 50 entries, got %d", len(first.Entries))
-	}
-	if !first.HasMoreBefore {
-		t.Fatalf("first page must report has_more_before=true (47 older comments dropped by merge)")
-	}
-	if first.NextCursor == nil {
-		t.Fatalf("first page must emit next_cursor when has_more_before=true")
-	}
-
-	// Page 2: walk older. Must surface the 47 older comments that the merge
-	// dropped on page 1.
-	second, code := fetchTimeline(t, issueID, "limit=50&before="+*first.NextCursor)
-	if code != http.StatusOK {
-		t.Fatalf("second page: expected 200, got %d", code)
-	}
-	if len(second.Entries) != 47 {
-		t.Fatalf("second page should return the 47 dropped older comments, got %d", len(second.Entries))
-	}
-
-	// Spot-check: every entry on page 2 must be a comment (the activities
-	// block was strictly newer).
-	for i, e := range second.Entries {
-		if e.Type != "comment" {
-			t.Fatalf("page 2 entry %d: expected comment, got %s", i, e.Type)
+	for i := 0; i < activityN; i++ {
+		ts := base.Add(time.Duration(i) * time.Minute)
+		if _, err := testPool.Exec(ctx, `
+			INSERT INTO activity_log (workspace_id, issue_id, actor_type, actor_id, action, details, created_at)
+			VALUES ($1, $2, 'member', $3, 'status_changed', '{"from":"todo","to":"in_progress"}'::jsonb, $4)
+		`, testWorkspaceID, issueID, testUserID, ts); err != nil {
+			t.Fatalf("seed activity %d: %v", i, err)
 		}
 	}
+	commentIDs := make([]string, 0, commentN)
+	for i := 0; i < commentN; i++ {
+		var id string
+		ts := base.Add(time.Duration(activityN+i) * time.Minute)
+		if err := testPool.QueryRow(ctx, `
+			INSERT INTO comment (issue_id, workspace_id, author_type, author_id, content, type, created_at, updated_at)
+			VALUES ($1, $2, 'member', $3, $4, 'comment', $5, $5)
+			RETURNING id
+		`, issueID, testWorkspaceID, testUserID, fmt.Sprintf("comment %d", i), ts).Scan(&id); err != nil {
+			t.Fatalf("seed comment %d: %v", i, err)
+		}
+		commentIDs = append(commentIDs, id)
+	}
 
-	// Spot-check: page 2 must include the very oldest comment we seeded —
-	// otherwise the cursor walk lost data, which is precisely what #2192
-	// was about.
-	oldestSeeded := commentIDs[0]
-	found := false
-	for _, e := range second.Entries {
-		if e.ID == oldestSeeded {
-			found = true
+	// Walk older pages until exhausted, collecting every comment id seen.
+	seen := map[string]bool{}
+	cursor := ""
+	for page := 0; page < 10; page++ { // safety bound — true exit is has_more_before=false
+		query := "limit=30"
+		if cursor != "" {
+			query += "&before=" + cursor
+		}
+		resp, code := fetchTimeline(t, issueID, query)
+		if code != http.StatusOK {
+			t.Fatalf("page %d: expected 200, got %d", page, code)
+		}
+		for _, e := range resp.Entries {
+			if e.Type == "comment" {
+				seen[e.ID] = true
+			}
+		}
+		if !resp.HasMoreBefore {
 			break
 		}
-	}
-	if !found {
-		t.Fatalf("page 2 missing the oldest seeded comment %s — cursor walk lost data", oldestSeeded)
+		if resp.NextCursor == nil {
+			t.Fatalf("page %d: has_more_before=true but next_cursor missing", page)
+		}
+		cursor = *resp.NextCursor
 	}
 
-	// Sanity: don't leak DB internals if something later changes the helper.
-	_ = ctx
+	// All 80 seeded comments must be reachable through the cursor walk —
+	// pre-fix, the 50 unreturned comments after page 1 stayed hidden because
+	// the shared cursor skipped past them via the activity timestamp.
+	if len(seen) != commentN {
+		missing := []string{}
+		for _, id := range commentIDs {
+			if !seen[id] {
+				missing = append(missing, id)
+			}
+		}
+		t.Fatalf("expected to see all %d comments via cursor walk, saw %d. Missing: %v",
+			commentN, len(seen), missing)
+	}
+}
+
+// TestListTimeline_ExactlyLimitCommentsHidesShowOlder pins the boundary the
+// user flagged: an issue with exactly <limit> comments must NOT report
+// has_more_before. Pre-fix the gate was `len(comments) >= limit`, which
+// returned true and rendered a "Show older" button that revealed nothing —
+// older clicks fetched zero rows. The over-fetch + trim probe makes the
+// boundary exact.
+func TestListTimeline_ExactlyLimitCommentsHidesShowOlder(t *testing.T) {
+	issueID := createIssueForTimeline(t, "exactly limit comments boundary")
+	seedTimelineEntries(t, issueID, 30, 0)
+
+	resp, code := fetchTimeline(t, issueID, "limit=30")
+	if code != http.StatusOK {
+		t.Fatalf("expected 200, got %d", code)
+	}
+	if len(resp.Entries) != 30 {
+		t.Fatalf("expected 30 entries on first page, got %d", len(resp.Entries))
+	}
+	if resp.HasMoreBefore {
+		t.Fatalf("has_more_before must be false when comments == limit (issue has nothing older)")
+	}
+	if resp.NextCursor != nil {
+		t.Fatalf("next_cursor must be nil when has_more_before is false, got %q", *resp.NextCursor)
+	}
+}
+
+// TestListTimeline_DenseActivityDoesNotHideComments reproduces #1857: an issue
+// with sparse comments but dense activity (status flips, agent runs) used to
+// trigger has_more_before because activities consumed the same page budget.
+// Real comments would get pushed off the visible page and users would think
+// the discussion had vanished. Post-fix, has_more_before is gated on comments
+// alone, so the entire conversation stays visible without "show older".
+func TestListTimeline_DenseActivityDoesNotHideComments(t *testing.T) {
+	issueID := createIssueForTimeline(t, "1857 sparse comments dense activity")
+
+	// 10 comments — well under the 30-comment page budget — paired with 60
+	// activities (an agent that flipped status / completed runs many times).
+	// seedTimelineEntries inserts comments first (older block), then activities
+	// (newer block), matching the typical "issue created → discussion → many
+	// agent runs" timeline shape.
+	commentIDs, _ := seedTimelineEntries(t, issueID, 10, 60)
+
+	resp, code := fetchTimeline(t, issueID, "limit=30")
+	if code != http.StatusOK {
+		t.Fatalf("expected 200, got %d", code)
+	}
+	if resp.HasMoreBefore {
+		t.Fatalf("has_more_before must be false when comments < limit, even if activities are dense (#1857)")
+	}
+
+	// Every seeded comment must be on the first page — none should be hidden
+	// behind a "show older" gate on an issue with so few comments.
+	commentSeen := map[string]bool{}
+	for _, e := range resp.Entries {
+		if e.Type == "comment" {
+			commentSeen[e.ID] = true
+		}
+	}
+	for _, id := range commentIDs {
+		if !commentSeen[id] {
+			t.Fatalf("comment %s missing from latest page — #1857 regressed", id)
+		}
+	}
 }