Files
multica/server/internal/handler/activity.go
Multica Eve c3f5df8bf4 MUL-5492: fix timeline cap dropping newest entries + stop double-broadcasting descriptions (#6175)
* fix(timeline): cap the issue timeline at the newest end and report the clamp

The per-issue timeline cap was applied with ORDER BY created_at ASC LIMIT
2000, so once an issue accumulated more than 2000 comments or activities
the cap discarded the NEWEST rows. The timeline appeared to stop at some
point in the past and every later event was invisible, with nothing in
the response indicating anything was missing.

Activity is machine-paced — description autosave, every agent run, status
and assignee changes all write rows — so this was reachable in normal use,
not only on pathological issues.

Take the window with the keyset ordering (created_at DESC, id DESC) in a
subquery and re-sort ascending in the outer query. This keeps the
chronological contract for every existing caller, including the comment
list endpoint that shares ListCommentsForIssue, and is served as an
index-only scan by the idx_*_keyset indexes already added in migration
068 — no new migration, no call-site changes.

Two things beyond the ordering flip:

- Clamp both lists to a shared window floor. The two caps are applied
  independently, so each list has its own floor. Merging windows with
  different floors produces a timeline that looks continuous but, below
  the higher floor, contains only one of the two kinds — e.g. comments
  with no interleaved activity. That is worse than a timeline that
  visibly stops, because nothing about it looks wrong. Both lists are
  now clamped to the newest floor, so the result is a contiguous,
  correctly interleaved slice.

- Stop truncating silently. The unpaginated response is a bare JSON
  array with nowhere to put a flag, so the clamp is reported via
  X-Timeline-Truncated and X-Timeline-Window-From, added to
  ExposedHeaders because a custom response header is otherwise
  unreadable from browser JS. The legacy wrapped shape's has_more_before
  is now truthful instead of hardcoded false.

Queries read one row past the cap so "hit the cap" is distinguishable
from "holds exactly 2000 rows", which would otherwise report a complete
timeline as truncated and drag the other list's window down with it.

Regression tests cover all four properties and were confirmed to fail
against both the original query and a floor-less DESC flip.

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

* perf(realtime): stop broadcasting two full descriptions on issue:updated

issue:updated carried prev_description alongside the new description in
the issue object, and the WS forwarder reuses the producer's payload map
verbatim. Every debounced description autosave therefore pushed two full
copies of the description to every connection in the workspace, including
users who did not have the issue open. The DB write is O(1); the fanout
was O(connections x description size), and it repeats on every pause in
an editing session.

prev_description and prev_title exist only for in-process listeners —
subscriber_listeners adds newly @mentioned users, notification_listeners
builds mention notifications, activity_listeners records the title
change. No client reads them: IssueUpdatedPayload in
packages/core/types/events.ts does not declare either field.

Project the payload on the way out. The bus dispatches bus.Subscribe
handlers before the SubscribeAll forwarder, so the in-process consumers
are unaffected, and projecting at the forwarder covers both the single-
node Hub and the Redis relays since that is where the frame is
serialized. The producer's map is copied rather than mutated.

The removed keys are listed in a table rather than an if on one event
type. The bug was structural, not a typo: the next large field added to
a published payload inherits the same cost silently, and a declarative
list puts the internal/external payload boundary in one reviewable
place.

issue.description itself is deliberately kept — clients apply it to
their cache, so stripping it would trade fanout bytes for N refetches.
Cutting the remaining fanout needs the per-issue scope routing already
scaffolded server-side for MUL-1138, which is blocked on the client
sending subscribe frames.

Tests assert both halves: the keys are absent from the serialized frame,
and the in-process listener still receives them.

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

* fix(timeline): keep comment threads whole under the newest-N cap

Review found that the newest-N window can orphan a reply, and an orphaned
reply is invisible rather than merely mis-nested: the timeline builds its
top level from "activities + comments with no parent_id" and renders
replies by looking them up under their parent, so an orphan sits in the
map with no card to render it. MUL-1847 / #2263 was exactly this shape —
1 root + 29 replies, root dropped, all 29 vanished from the UI while the
API returned them.

Root cause of the regression: capping with the OLDEST n could never
orphan anything, because a reply is always newer than its parent, so a
prefix of the timeline is closed under "parent of". A newest-n window is
a suffix and has no such property. Flipping which end the cap bites
silently invalidated a structural property the comment tree relies on.

Two changes.

Drop the cross-kind clamp. The previous revision trimmed both lists to a
shared floor so the window was provably contiguous. That was the wrong
trade and it was also the dominant source of orphans. Comments are
human-paced (p99 ~30, max ever observed ~1.1k) and essentially never
reach the cap, while activity is machine-paced and reaches it routinely
— so the shared floor was almost always the activity floor deleting
comments that had been fetched successfully and would have rendered
fine. On an issue with thirty comments it was pure loss. Each list now
reports its own truncation and X-Timeline-Truncated names which kinds
were affected. Not clamping costs only activity density in the older
part of the range, which is metadata rather than content, and it is
reported rather than hidden.

Complete parent chains for the case that remains — comments themselves
exceeding the cap. ListMissingAncestorComments walks parent_id upward via
a recursive CTE and returns the ancestors not already held; the handler
merges them and restores the ascending order. This only ever ADDS rows,
so unlike clamping it cannot hide anything the caller would have seen,
and it is bounded by the number of distinct missing ancestors. Whole-
thread windowing was considered and rejected: a single thread can exceed
any row budget, so its degradation is not definable.

Applied to the shared query's default list path too, not just the
timeline. foldResolvedThreads documents a COMPLETE-thread set as its
precondition and comment.go asserts the default list mode satisfies it;
a half thread made that assertion false and a resolved thread whose root
was cut stopped folding correctly.

Also drops X-Timeline-Window-From. It was second-precision RFC3339 while
the real ordering key is (created_at, id) at full precision, so it could
not resume a read without skipping or repeating rows inside a shared
second. A resumable cursor should be opaque and carry both halves; worth
designing when there is a consumer rather than shipping as a lossy
approximation.

Tests: the reviewer's exact scenario, plus a no-orphaned-replies
invariant on both endpoints, the fold-still-works case, and a guard that
activity truncation does not delete comments. Each was confirmed to fail
with the fix disabled. TestListTimeline_JointWindowHasNoOneSidedRegion
was rewritten rather than deleted — it pinned the clamp behaviour being
abandoned here, so leaving it would lock in the wrong contract and
deleting it would drop the coverage.

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

* fix(comments): bound parent-chain completion and stop folding partial threads

Second review round on MUL-5492. Four must-fixes, all stemming from one
conflation: parent-chain closure, a newest-N window, and a complete
thread are three different things. Closure makes a reply renderable; it
does not license thread-level derivations.

Do not fold a truncated read. fetchCommentsForList closes parent chains,
but older siblings and descendants of a retained reply stay outside the
window, so the set holds partial threads. Folding them produced wrong
answers rather than incomplete ones: a resolution reply outside the
window made a resolved thread look unresolved, and folded_count reported
a total derived only from retained replies. The previous revision claimed
closure restored foldResolvedThreads' COMPLETE-thread precondition; it
did not, and that claim is removed. --recent and untailed --thread still
return whole threads and still fold.

Bound the walk. The recursive CTE climbed to the root with no depth
limit, so a deep chain could drag its entire ancestry back and defeat the
row cap it was meant to preserve. Depth is genuinely unbounded in stored
data: the general write path stores the exact comment being replied to
(only the agent path collapses to the thread root), so chains can run far
deeper than the two levels the UI renders. Replaced with a layered walk
under explicit budgets — 2000 extra rows, 64 levels — making a response
provably bounded by 4000 comments, or 6000 timeline entries with
activities.

Scope every level to the tenant. The CTE's recursive branch matched on
parent_id alone. parent_id carries a foreign key to comment(id) but not
to a matching issue, so a stray cross-issue parent reference is
representable, and the walk would have followed it into another issue's
comments. The replacement filters issue_id and workspace_id on every
level. A negative test confirms the leak: with the filter removed it
reports "a comment from another issue leaked into this issue's response".

Degrade by pruning, not by orphaning. When a budget is exhausted, a
parent row is missing, or a parent is out of scope, keepRootConnected
drops the affected comments instead of returning replies the UI cannot
render. Dropping a node also drops its descendants, since their chains
run through it. Returning fewer new replies is conservative and already
signalled as a truncated read; leaking another tenant's data, returning
an unbounded response, or emitting invisible orphans are all worse.

Also: probe read on the comment list so exactly-2000 is not misreported
as truncated, which would needlessly suppress the fold; CommentsTruncated
is carried on fetchCommentsResult rather than inferred from the result
length, which is meaningless once completion adds rows. The new query
returns db.Comment directly instead of a hand-copied row, which is how
quick_action_id came to be dropped after the rebase — a backfilled
quick-action root would have rendered as a raw prompt.

Corrected three inaccurate comments: the "index-only scan" claim (the
index avoids the sort but does not cover SELECT *), the "write path
collapses replies to root" claim, and a test header still describing the
abandoned contiguous-window behaviour.

Tests cover exactly-at-cap still folding, truncated reads not folding
(both reply-resolved and root-resolved), depth beyond budget pruned not
orphaned, shared ancestors fetched once, cross-issue parents never
crossing the boundary, and quick_action_id surviving backfill. Each was
confirmed to fail with its specific fix disabled; the reply-resolved fold
test was reshaped after the first version passed for the wrong reason.

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

* fix(timeline): preserve complete threads under comment cap

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

* fix(comments): preserve newest bounded views

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

---------

Co-authored-by: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>
2026-07-31 14:40:16 +08:00

396 lines
15 KiB
Go

package handler
import (
"encoding/json"
"net/http"
"sort"
"github.com/go-chi/chi/v5"
"github.com/jackc/pgx/v5/pgtype"
db "github.com/multica-ai/multica/server/pkg/db/generated"
)
// TimelineEntry represents a single entry in the issue timeline, which can be
// either an activity log record or a comment.
type TimelineEntry struct {
Type string `json:"type"` // "activity" or "comment"
ID string `json:"id"`
ActorType string `json:"actor_type"`
ActorID string `json:"actor_id"`
CreatedAt string `json:"created_at"`
// Activity-only fields
Action *string `json:"action,omitempty"`
Details json.RawMessage `json:"details,omitempty"`
// Comment-only fields
Content *string `json:"content,omitempty"`
ParentID *string `json:"parent_id,omitempty"`
UpdatedAt *string `json:"updated_at,omitempty"`
CommentType *string `json:"comment_type,omitempty"`
// Set only on comments produced by a quick action run. Unforgeable: there
// is no request field for it on the generic comment endpoint.
QuickActionID *string `json:"quick_action_id,omitempty"`
Reactions []ReactionResponse `json:"reactions,omitempty"`
Attachments []AttachmentResponse `json:"attachments,omitempty"`
ResolvedAt *string `json:"resolved_at,omitempty"`
ResolvedByType *string `json:"resolved_by_type,omitempty"`
ResolvedByID *string `json:"resolved_by_id,omitempty"`
SourceTaskID *string `json:"source_task_id,omitempty"`
}
// timelineHardCap bounds the per-issue timeline payload. Sized as a defensive
// safety net, not a UX page window: see commentHardCap in comment.go for the
// data-shape rationale (#1929).
const timelineHardCap = 2000
// timelineProbeLimit reads one row past the cap so "we hit the cap" can be
// distinguished from "the issue happens to have exactly timelineHardCap rows".
// Without the probe row an issue sitting exactly on the boundary would report a
// complete timeline as truncated and pay a needless ancestor-backfill query.
const timelineProbeLimit = timelineHardCap + 1
// Truncation is signalled with a response header rather than a body field
// because the unpaginated response is a bare JSON array (TimelineEntriesSchema =
// z.array(TimelineEntrySchema) in packages/core/api/schemas.ts) with nowhere to
// put a flag. The header is additive: existing clients keep validating.
//
// The value names which kinds were truncated ("activity", "comment", or
// "activity,comment") because the two caps are independent — in practice it is
// almost always activity alone.
//
// There is deliberately no companion "window from" header. An earlier revision
// emitted one, but TimestampToString is second-precision RFC3339 while the real
// ordering key is (created_at, id) at full precision, so it could not be used to
// resume a read without skipping or repeating rows inside a shared second. A
// resumable cursor needs to be opaque and carry both halves; that is worth
// designing when there is a consumer, not shipping as a lossy approximation.
//
// Exported so the CORS layer can reference the same identifier
// (corsExposedHeaders in server/cmd/server/router.go). A custom response header
// is invisible to browser JS unless explicitly exposed, so a rename here that
// did not reach the CORS list would silently switch the signal back off.
const HeaderTimelineTruncated = "X-Timeline-Truncated"
// truncatedKinds renders the X-Timeline-Truncated value, "" when nothing was
// truncated.
func truncatedKinds(comments, activities bool) string {
switch {
case comments && activities:
return "activity,comment"
case comments:
return "comment"
case activities:
return "activity"
default:
return ""
}
}
// takeNewest trims a timelineProbeLimit read down to timelineHardCap and reports
// whether the probe row proved older rows exist. rows must be ascending, so the
// newest timelineHardCap entries are the tail.
func takeNewest[T any](rows []T) ([]T, bool) {
if len(rows) <= timelineHardCap {
return rows, false
}
return rows[len(rows)-timelineHardCap:], true
}
// timelinePaginatedResponse mirrors the wrapper shape produced by the prior
// cursor-paginated ListTimeline (#2128). It is preserved as a backward-compat
// surface for installed Desktop builds and stale Web bundles between #2128 and
// #1929 that send `?limit=`/`?before=`/`?after=`/`?around=` and parse the
// response with the old TimelinePageSchema (entries + cursors). Cursors are
// always nil and `has_more_after` is always false: the new server returns the
// whole timeline in one shot. `has_more_before` is now truthful — it reports the
// hard-cap clamp — instead of being hardcoded false.
type timelinePaginatedResponse struct {
Entries []TimelineEntry `json:"entries"`
NextCursor *string `json:"next_cursor"`
PrevCursor *string `json:"prev_cursor"`
HasMoreBefore bool `json:"has_more_before"`
HasMoreAfter bool `json:"has_more_after"`
TargetIndex *int `json:"target_index,omitempty"`
}
// ListTimeline returns the full issue timeline (comments + activities merged).
// Two response shapes coexist for boundary compatibility (#1929):
//
// - No pagination params → flat ASC `TimelineEntry[]`. Matches the legacy
// desktop contract (Multica.app ≤ v0.2.25) and the new client.
// - Any of `limit` / `before` / `after` / `around` present → wrapped object
// with DESC entries + null cursors + has_more_after=false. Matches what a
// stale v0.2.26+ build expects when it parses the response with
// TimelinePageSchema; cursor-walking is now a no-op so the client just
// sees a single full page.
//
// Both shapes carry the same set of entries — paging and ordering differ.
// Time-based pagination was removed because it split reply threads at page
// boundaries, and at observed data sizes (p99 ~30 comments per issue) the
// cursor machinery was pure overhead.
//
// When the hard cap fires each list is independently reduced to its newest
// entries and X-Timeline-Truncated names which kinds were affected. Comment
// threads cut by the window are completed afterwards within a bounded context
// budget; a thread that cannot be completed is omitted as one unit (MUL-5492).
func (h *Handler) ListTimeline(w http.ResponseWriter, r *http.Request) {
id := chi.URLParam(r, "id")
issue, ok := h.loadIssueForUser(w, r, id)
if !ok {
return
}
ctx := r.Context()
comments, err := h.Queries.ListCommentsForIssue(ctx, db.ListCommentsForIssueParams{
IssueID: issue.ID,
WorkspaceID: issue.WorkspaceID,
Limit: timelineProbeLimit,
})
if err != nil {
writeError(w, http.StatusInternalServerError, "failed to list comments")
return
}
activities, err := h.Queries.ListActivitiesForIssue(ctx, db.ListActivitiesForIssueParams{
IssueID: issue.ID,
Limit: timelineProbeLimit,
})
if err != nil {
writeError(w, http.StatusInternalServerError, "failed to list activities")
return
}
q := r.URL.Query()
wantWrapped := q.Get("limit") != "" || q.Get("before") != "" ||
q.Get("after") != "" || q.Get("around") != ""
// Each list is capped independently and reports its own truncation. The two
// are deliberately NOT clamped to a shared floor.
//
// An earlier revision did clamp them, to guarantee the returned window was a
// contiguous correctly-interleaved slice with no region holding only one of
// the two kinds. That traded the wrong way round. Comments are human-paced
// (p99 ~30, max ever observed ~1.1k) so they essentially never reach the cap,
// while activity is machine-paced and reaches it routinely — so the shared
// floor was almost always the ACTIVITY floor cutting away comments that had
// been fetched successfully and would have rendered fine. It deleted real
// content to buy a cosmetic property, and on a busy issue with thirty
// comments it was pure loss.
//
// Not clamping costs only activity density in the older part of the range,
// which is metadata, not content — and it is reported rather than hidden.
// It also keeps the comment set from being cut at an arbitrary row, which is
// why completeCommentThreads repairs any affected thread below.
comments, commentsTruncated := takeNewest(comments)
activities, activitiesTruncated := takeNewest(activities)
// Timeline clients derive resolution bars, author lists, and folded counts
// from the comments in this response. A parent-only repair would therefore
// make a partial thread look complete. Restore all missing siblings and
// descendants for affected roots within the shared context budget; if the
// walk cannot prove a thread complete, omit that thread as one unit.
if commentsTruncated {
var err error
comments, err = h.completeCommentThreads(ctx, issue.ID, issue.WorkspaceID, comments)
if err != nil {
writeError(w, http.StatusInternalServerError, "failed to complete comment threads")
return
}
}
if kinds := truncatedKinds(commentsTruncated, activitiesTruncated); kinds != "" {
w.Header().Set(HeaderTimelineTruncated, kinds)
}
if wantWrapped {
entries := h.mergeTimeline(r, comments, activities, false)
if entries == nil {
entries = []TimelineEntry{}
}
resp := timelinePaginatedResponse{
Entries: entries,
HasMoreBefore: commentsTruncated || activitiesTruncated,
}
// `around=<id>`: locate the anchor in the DESC slice so the legacy
// client can scroll-to-highlight without a follow-up request.
if anchor := q.Get("around"); anchor != "" {
for i, e := range entries {
if e.ID == anchor {
idx := i
resp.TargetIndex = &idx
break
}
}
}
writeJSON(w, http.StatusOK, resp)
return
}
entries := h.mergeTimeline(r, comments, activities, true)
if entries == nil {
entries = []TimelineEntry{}
}
writeJSON(w, http.StatusOK, entries)
}
// mergeTimeline merges comments and activities and returns them sorted by
// (created_at, id). When ascending=true, oldest first (the new flat-array
// contract); otherwise newest first (the wrapped legacy contract).
func (h *Handler) mergeTimeline(r *http.Request, comments []db.Comment, activities []db.ActivityLog, ascending bool) []TimelineEntry {
out := make([]TimelineEntry, 0, len(comments)+len(activities))
out = append(out, h.commentsToEntries(r, comments)...)
for _, a := range activities {
out = append(out, activityToEntry(a))
}
sort.Slice(out, func(i, j int) bool {
if out[i].CreatedAt != out[j].CreatedAt {
if ascending {
return out[i].CreatedAt < out[j].CreatedAt
}
return out[i].CreatedAt > out[j].CreatedAt
}
if ascending {
return out[i].ID < out[j].ID
}
return out[i].ID > out[j].ID
})
return out
}
// commentsToEntries fetches reactions + attachments for the given comments in
// one batch each and returns enriched TimelineEntry slices preserving order.
func (h *Handler) commentsToEntries(r *http.Request, comments []db.Comment) []TimelineEntry {
if len(comments) == 0 {
return nil
}
ids := make([]pgtype.UUID, len(comments))
for i, c := range comments {
ids[i] = c.ID
}
reactions := h.groupReactions(r, ids)
attachments := h.groupAttachments(r, ids)
out := make([]TimelineEntry, len(comments))
for i, c := range comments {
content := c.Content
commentType := c.Type
updatedAt := timestampToString(c.UpdatedAt)
cid := uuidToString(c.ID)
out[i] = TimelineEntry{
Type: "comment",
ID: cid,
ActorType: c.AuthorType,
ActorID: uuidToString(c.AuthorID),
Content: &content,
CommentType: &commentType,
QuickActionID: uuidToPtr(c.QuickActionID),
ParentID: uuidToPtr(c.ParentID),
CreatedAt: timestampToString(c.CreatedAt),
UpdatedAt: &updatedAt,
Reactions: reactions[cid],
Attachments: attachments[cid],
ResolvedAt: timestampToPtr(c.ResolvedAt),
ResolvedByType: textToPtr(c.ResolvedByType),
ResolvedByID: uuidToPtr(c.ResolvedByID),
SourceTaskID: uuidToPtr(c.SourceTaskID),
}
}
return out
}
func activityToEntry(a db.ActivityLog) TimelineEntry {
action := a.Action
actorType := ""
if a.ActorType.Valid {
actorType = a.ActorType.String
}
return TimelineEntry{
Type: "activity",
ID: uuidToString(a.ID),
ActorType: actorType,
ActorID: uuidToString(a.ActorID),
Action: &action,
Details: a.Details,
CreatedAt: timestampToString(a.CreatedAt),
}
}
// AssigneeFrequencyEntry represents how often a user assigns to a specific target.
type AssigneeFrequencyEntry struct {
AssigneeType string `json:"assignee_type"`
AssigneeID string `json:"assignee_id"`
Frequency int64 `json:"frequency"`
}
// GetAssigneeFrequency returns assignee usage frequency for the current user,
// combining data from assignee change activities and initial issue assignments.
func (h *Handler) GetAssigneeFrequency(w http.ResponseWriter, r *http.Request) {
userID, ok := requireUserID(w, r)
if !ok {
return
}
workspaceID := h.resolveWorkspaceID(r)
// Aggregate frequency from both data sources.
freq := map[string]int64{} // key: "type:id"
// Source 1: assignee_changed activities by this user.
activityCounts, err := h.Queries.CountAssigneeChangesByActor(r.Context(), db.CountAssigneeChangesByActorParams{
WorkspaceID: parseUUID(workspaceID),
ActorID: parseUUID(userID),
})
if err != nil {
writeError(w, http.StatusInternalServerError, "failed to get assignee frequency")
return
}
for _, row := range activityCounts {
aType, _ := row.AssigneeType.(string)
aID, _ := row.AssigneeID.(string)
if aType != "" && aID != "" {
freq[aType+":"+aID] += row.Frequency
}
}
// Source 2: issues created by this user with an assignee.
issueCounts, err := h.Queries.CountCreatedIssueAssignees(r.Context(), db.CountCreatedIssueAssigneesParams{
WorkspaceID: parseUUID(workspaceID),
CreatorID: parseUUID(userID),
})
if err != nil {
writeError(w, http.StatusInternalServerError, "failed to get assignee frequency")
return
}
for _, row := range issueCounts {
if !row.AssigneeType.Valid || !row.AssigneeID.Valid {
continue
}
key := row.AssigneeType.String + ":" + uuidToString(row.AssigneeID)
freq[key] += row.Frequency
}
// Build sorted response.
result := make([]AssigneeFrequencyEntry, 0, len(freq))
for key, count := range freq {
// Split "type:id" — type is always "member" or "agent" (no colons).
var aType, aID string
for i := 0; i < len(key); i++ {
if key[i] == ':' {
aType = key[:i]
aID = key[i+1:]
break
}
}
result = append(result, AssigneeFrequencyEntry{
AssigneeType: aType,
AssigneeID: aID,
Frequency: count,
})
}
sort.Slice(result, func(i, j int) bool {
return result[i].Frequency > result[j].Frequency
})
writeJSON(w, http.StatusOK, result)
}