mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-03 19:20:07 +02:00
c3f5df8bf4
* 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>