Files
multica/server/pkg/db/queries/comment.sql
Bohan Jiang 6f5fbb7813 feat(comments): thread-aware list with composite cursor (MUL-2340) (#2787)
* feat(comments): thread-aware list with composite cursor (MUL-2340)

Adds three optional query params to GET /api/issues/{id}/comments and the
matching `multica issue comment list` flags:

- `thread=<comment-uuid>` resolves the anchor to the thread root via a
  recursive CTE (defends against any future nested replies) and returns
  root + all descendants chronologically. Anchor can be any comment in
  the thread, root or reply.
- `recent=<N>` returns the newest N comments for the issue, ordered
  chronologically in the response.
- `before=<RFC3339>` + `before-id=<uuid>` form a composite cursor for
  stable pagination of `recent`. Both must be set together; a
  timestamp-only cursor is rejected because ties on `created_at` would
  let the existing `(created_at ASC, id ASC)` total order skip or
  duplicate rows across pages.

Flag combination rules: `thread` is exclusive with `recent` and the
cursor; both may combine with `since`. Server and CLI enforce the same
matrix; the CLI fails fast locally so callers don't pay for a 400
round-trip.

Default behaviour (no params) is unchanged — full chronological dump
capped at commentHardCap — so the desktop UI and existing `--since`
polling are untouched. Agent prompt updates land in a follow-up PR so
the new CLI capabilities ship and bake first.

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

* fix(comments): reject cursor without recent and align CLI/server on invalid --recent (MUL-2340)

Elon's PR #2787 second review flagged two gaps in the flag combination
matrix:

- server: GET /comments?before=...&before_id=... without `recent` was
  silently dropped by fetchCommentsForList (RecentN=0 fell through to
  the default / since path), so callers got the full timeline instead
  of the documented "before X" semantics. Now returns 400.
- CLI: --recent 0 / --recent -3 were collapsed with "flag not passed"
  by `recent > 0`, so an explicit invalid value silently fell back to
  the default list. Switched to Flags().Changed("recent") so explicit
  non-positive values fail loudly. Also enforces that --before /
  --before-id only appear with explicit --recent (mirrors the new
  server-side rule).

Tests:
- server flag matrix gains `before + before_id without recent → 400`.
- CLI gains TestRunIssueCommentListFlagGuards covering `--recent 0`,
  `--recent -3`, cursor-without-recent, and the thread/recent
  exclusivity path under the new Changed()-based check. The mock
  server fatals if a request reaches /comments, proving the guards
  fire before any HTTP round-trip.

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

* feat(comments): make `recent` thread-grouped with a thread cursor (MUL-2340)

Bohan pushed back on the row-based `recent=N` shape: comments form a tree,
not a list, and the newest N rows can come from N unrelated threads, giving
the agent N disjoint conversational tails. Replace the row-based query with
a thread-grouped one before #2787 merges so we never ship the wrong shape:

- `recent=N` now returns the N most recently active threads (root + every
  descendant per thread). A thread's recency is MAX(created_at) across its
  whole subtree, so a stale-but-recently-replied thread outranks an old
  quiet one — exactly the property row-recent loses.
- The cursor is now a *thread* cursor: `before` = a thread's
  last_activity_at, `before_id` = its root comment id. The pair walks
  threads strictly less recent than the page's oldest-active thread. The
  cursor surfaces via `X-Multica-Next-Before` / `X-Multica-Next-Before-Id`
  response headers (empty when there are no older threads); the CLI
  forwards the same pair to stderr after listing.
- Row-based `recent` is gone — there is no internal caller and the prompt
  update has not shipped yet, so there is no compat surface to preserve.
- Response body shape unchanged (flat JSON array, chronological). Default
  and `--since` paths untouched. Desktop UI keeps working.

Tests:
- recent=1 returns the freshest-active thread fully; recent=2 returns both
  with the older-active thread first (oldest-active → freshest tail).
- Stale-but-fresh: a thread whose root is older but has a fresh reply
  outranks a thread whose root is newer but quiet.
- Cursor headers emitted only on full pages; empty on the final page.
- Pagination walks threads root2 → root1 → empty, no skips/duplicates.
- Tie-break: three threads sharing last_activity_at paginate one-at-a-time
  using (last_activity_at, root_id) ordering — verifies the timestamp-only
  cursor failure mode is fixed for the thread case too.

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

---------

Co-authored-by: multica-agent <github@multica.ai>
2026-05-18 19:28:26 +08:00

191 lines
7.3 KiB
SQL

-- name: ListCommentsForIssue :many
-- All comments for an issue in chronological order, capped at $3 (DB safety
-- net). Issue p99 is ~30 comments, max ever observed in prod is ~1.1k, so
-- the handler-side cap of 2000 is purely defensive.
SELECT * FROM comment
WHERE issue_id = $1 AND workspace_id = $2
ORDER BY created_at ASC, id ASC
LIMIT $3;
-- name: ListCommentsSinceForIssue :many
-- Comments created strictly after $3 in chronological order, capped at $4.
-- Powers the CLI's `--since` agent-polling flow.
SELECT * FROM comment
WHERE issue_id = $1 AND workspace_id = $2 AND created_at > $3
ORDER BY created_at ASC, id ASC
LIMIT $4;
-- name: ListThreadCommentsForIssue :many
-- Returns the root of the thread containing @anchor_id plus every descendant
-- (recursive — defends against any future deeper nesting; today's data is two
-- layers because the CreateComment path collapses replies to root, but the
-- schema does not enforce that). @anchor_id may itself be a root or a reply.
-- Output is chronological so it can be fed straight to the agent.
WITH RECURSIVE root_of AS (
-- Walk up from the anchor until parent_id IS NULL.
SELECT c.id, c.parent_id
FROM comment c
WHERE c.id = @anchor_id AND c.issue_id = @issue_id AND c.workspace_id = @workspace_id
UNION ALL
SELECT p.id, p.parent_id
FROM comment p
JOIN root_of r ON p.id = r.parent_id
),
thread_root AS (
SELECT id FROM root_of WHERE parent_id IS NULL LIMIT 1
),
descendants AS (
-- Start from the root, then keep adding any comment whose parent is
-- already in the set. Cycle-safe under PK constraint (a comment cannot
-- be its own ancestor).
SELECT c.id, c.issue_id, c.author_type, c.author_id, c.content, c.type,
c.created_at, c.updated_at, c.parent_id, c.workspace_id,
c.resolved_at, c.resolved_by_type, c.resolved_by_id
FROM comment c
JOIN thread_root tr ON c.id = tr.id
UNION
SELECT c.id, c.issue_id, c.author_type, c.author_id, c.content, c.type,
c.created_at, c.updated_at, c.parent_id, c.workspace_id,
c.resolved_at, c.resolved_by_type, c.resolved_by_id
FROM comment c
JOIN descendants d ON c.parent_id = d.id
WHERE c.issue_id = @issue_id AND c.workspace_id = @workspace_id
)
SELECT id, issue_id, author_type, author_id, content, type,
created_at, updated_at, parent_id, workspace_id,
resolved_at, resolved_by_type, resolved_by_id
FROM descendants
ORDER BY created_at ASC, id ASC
LIMIT @row_limit;
-- name: ListRecentThreadCommentsForIssue :many
-- Returns the N most recently active threads (root + every descendant) rather
-- than the N most recent rows. A thread's "last activity" is MAX(created_at)
-- over its whole subtree; threads are ranked by (last_activity_at DESC,
-- root_id DESC) and the top N are expanded.
--
-- Why thread-grouped instead of row-recent: with row-recent the newest 20
-- comments can come from 8 different threads — the agent sees 8 unrelated
-- tails. With thread-grouped the agent sees N complete conversational arcs,
-- which matches how a human reads an issue (#2340).
--
-- Response ordering:
-- threads: (thread_last_activity_at ASC, root_id ASC)
-- in-thread: (created_at ASC, id ASC)
-- So the oldest-active thread appears first and the most recently-active
-- thread is at the tail, closest to "now" in an agent prompt.
--
-- Cursor scrolls back through threads. When @has_cursor=TRUE only threads
-- with (last_activity_at, root_id) < (@before_at, @before_id) are eligible.
-- The cursor is a THREAD cursor — both values identify a thread (its last
-- activity timestamp and its root comment id), not a single row.
--
-- The recursive `membership` CTE labels each comment with its thread root by
-- walking down from every root. It does not assume any maximum nesting depth,
-- which preserves correctness even if the schema ever allows reply-of-reply
-- (the agent path in TaskService.createAgentComment collapses to root today,
-- but the user-facing CreateComment handler does not enforce it).
WITH RECURSIVE membership(id, root_id, comment_created_at) AS (
-- Each root maps to itself.
SELECT c.id, c.id AS root_id, c.created_at
FROM comment c
WHERE c.issue_id = @issue_id
AND c.workspace_id = @workspace_id
AND c.parent_id IS NULL
UNION ALL
-- Each descendant inherits its parent's root_id.
SELECT c.id, m.root_id, c.created_at
FROM comment c
JOIN membership m ON c.parent_id = m.id
WHERE c.issue_id = @issue_id
AND c.workspace_id = @workspace_id
),
thread_stats AS (
SELECT root_id, MAX(comment_created_at)::timestamptz AS last_activity_at
FROM membership
GROUP BY root_id
),
picked AS (
SELECT ts.root_id, ts.last_activity_at
FROM thread_stats ts
WHERE (
@has_cursor::boolean = FALSE
OR (ts.last_activity_at, ts.root_id) < (@before_at::timestamptz, @before_id::uuid)
)
ORDER BY ts.last_activity_at DESC, ts.root_id DESC
LIMIT @thread_limit
)
SELECT c.id, c.issue_id, c.author_type, c.author_id, c.content, c.type,
c.created_at, c.updated_at, c.parent_id, c.workspace_id,
c.resolved_at, c.resolved_by_type, c.resolved_by_id,
p.root_id AS thread_root_id,
p.last_activity_at AS thread_last_activity_at
FROM picked p
JOIN membership m ON m.root_id = p.root_id
JOIN comment c ON c.id = m.id
ORDER BY p.last_activity_at ASC, p.root_id ASC, c.created_at ASC, c.id ASC;
-- name: CountComments :one
SELECT count(*) FROM comment
WHERE issue_id = $1 AND workspace_id = $2;
-- name: GetComment :one
SELECT * FROM comment
WHERE id = $1;
-- name: GetCommentInWorkspace :one
SELECT * FROM comment
WHERE id = $1 AND workspace_id = $2;
-- name: CreateComment :one
INSERT INTO comment (issue_id, workspace_id, author_type, author_id, content, type, parent_id)
VALUES ($1, $2, $3, $4, $5, $6, sqlc.narg(parent_id))
RETURNING *;
-- name: UpdateComment :one
UPDATE comment SET
content = $2,
updated_at = now()
WHERE id = $1
RETURNING *;
-- name: HasAgentCommentedSince :one
SELECT EXISTS (
SELECT 1 FROM comment
WHERE issue_id = @issue_id
AND author_type = 'agent'
AND author_id = @author_id
AND created_at >= @since
) AS commented;
-- name: HasAgentRepliedInThread :one
-- Returns true if the given agent has posted a reply in the thread rooted at
-- the specified parent comment. Used to detect agent participation in a
-- member-started thread so that follow-up member replies still trigger the agent.
SELECT count(*) > 0 AS has_replied FROM comment
WHERE parent_id = @parent_id AND author_type = 'agent' AND author_id = @agent_id;
-- name: DeleteComment :exec
DELETE FROM comment WHERE id = $1;
-- name: ResolveComment :one
-- Idempotent: re-resolving keeps the original resolved_at + resolver. Always
-- returns the row so the handler can surface the canonical state.
UPDATE comment SET
resolved_at = COALESCE(resolved_at, now()),
resolved_by_type = COALESCE(resolved_by_type, $2),
resolved_by_id = COALESCE(resolved_by_id, $3),
updated_at = CASE WHEN resolved_at IS NULL THEN now() ELSE updated_at END
WHERE id = $1
RETURNING *;
-- name: UnresolveComment :one
-- Idempotent: a no-op clear (already unresolved) just returns the row.
UPDATE comment SET
resolved_at = NULL,
resolved_by_type = NULL,
resolved_by_id = NULL,
updated_at = CASE WHEN resolved_at IS NOT NULL THEN now() ELSE updated_at END
WHERE id = $1
RETURNING *;