Compare commits

...

3 Commits

Author SHA1 Message Date
Jiang Bohan
e02029337d fix(prompt): clarify --recent is paging, not a replacement (MUL-2387)
Address Elon's second-pass nit on #2816: the assignment-trigger workflow
in runtime_config.go used "you may switch to --recent 20", which reads as
a replacement for the mandatory full-history rule. Rephrase --recent as a
paging strategy ("read the full history page-by-page, not a shortcut that
replaces it") so it cannot conflict with the rule it lives next to.

The default per-turn prompt in prompt.go opened with "If you need comment
history" — that soft conditional contradicts the runtime workflow's
mandatory read. Move it to a neutral "For comment history, follow the
rule in your runtime workflow file" framing that defers to whatever the
workflow says (mandatory for assignment, optional elsewhere) instead of
encoding its own policy.

Keep the runtime/prompt dual-layer fallback intact — different runtimes
propagate the config file vs. the per-turn user prompt with varying
fidelity, so both surfaces need the guidance.

Tests pin the new phrasing against regression:

- TestBuildPromptDefaultMentionsRecent now also forbids "If you need
  comment history" from sneaking back in.
- TestInjectRuntimeConfigAssignmentTriggerMentionsRecent now also forbids
  "you may switch to" / "switch to `--recent" replacement phrasing.

Co-authored-by: multica-agent <github@multica.ai>
2026-05-19 14:31:46 +08:00
Jiang Bohan
bb49820146 fix(comments): suppress recent+since cursor when head thread past since (MUL-2387)
Previous suppression only tripped when the `since` filter emptied the
page. That missed the mixed case Elon flagged in #2787's second review:
the page keeps rows from fresher threads but the head (oldest-active)
thread already sits at or before `since`, so every older page is
guaranteed empty too. Predicating on `headLast <= since` covers both
cases.

Add a recent=2 + since fixture that pins the mixed scenario: root1
(last_activity = base+3m) is filtered out, root2 stays, and the cursor
is suppressed even though the body is non-empty.

Co-authored-by: multica-agent <github@multica.ai>
2026-05-18 19:56:57 +08:00
Jiang Bohan
669f259947 feat(prompt): thread-first comment reads for agent runs (MUL-2387)
PR #2787 added --thread / --recent / --before / --before-id to the
ListComments API and CLI but kept the agent prompt steering at the
legacy "dump everything" recipe. On a long-running issue the flat dump
burns context on chatter unrelated to the trigger; agents acting on the
trigger want the trigger's thread first.

Prompt updates:

- Comment-triggered Workflow (runtime_config.go) now anchors step 2 on
  `multica issue comment list <issue-id> --thread <trigger-comment-id>
  --output json`. Fallback offers `--recent 20 --output json` with the
  stderr `Next thread cursor: --before <ts> --before-id <root-id>` line
  feeding the next-page cursor. `--since` is preserved and explicitly
  marked combinable with --thread / --recent.
- Per-turn buildCommentPrompt (prompt.go) carries the same thread-first
  guidance so a Codex-style runtime that re-reads the per-turn message
  every iteration gets the same steering, even if it ignores the
  injected runtime config.
- Assignment-triggered Workflow keeps the mandatory full-history rule
  (MUL-1124) but now also points at `--recent 20` as the long-issue
  alternative — this is the place that previously had no thread-aware
  guidance at all.
- Default fallback prompt (no trigger comment, no chat, no autopilot,
  no quick-create) gains the same --recent hint without --thread (no
  comment to anchor on).
- Available Commands core line surfaces the new flags so the discovery
  path matches the workflow guidance.

Default CLI/API semantics are unchanged: the unparameterized list still
returns the full chronological dump capped at 2000, --since still works
on its own, and the desktop UI is untouched.

Tests:

- prompt_test.go: TestBuildPromptCommentTriggerPromotesThreadReads pins
  --thread <triggerID>, --recent 20, the stderr cursor phrasing, and
  the absence of the legacy "returns all comments" prose.
- prompt_test.go: TestBuildPromptDefaultMentionsRecent guards the
  no-trigger fallback (mentions --recent, must NOT mention --thread).
- execenv_test.go: TestInjectRuntimeConfigCommentTriggerThreadFirstReads
  asserts the comment-triggered Workflow steers at --thread/--recent,
  the Available Commands line surfaces the new flags, and the legacy
  "read the conversation (returns all comments...)" string is gone.
- execenv_test.go: TestInjectRuntimeConfigAssignmentTriggerMentionsRecent
  keeps the mandatory full-history rule pinned AND asserts --recent is
  offered as the long-issue alternative.

Also fixes the recent+since cursor nit Elon flagged in #2787's second
review: when `since` empties the page, the `len(seenRoot) >= recentN`
check used to emit a cursor anyway. Pagination walks threads in
strictly decreasing last_activity_at — if every comment in this page is
<= since, every older thread's last_activity is also <= since by
transitivity, so the cursor would only invite the caller into a
guaranteed-empty walk. Now suppressed; new tests pin both branches
(suppressed when empty, retained when at least one row passes since).

MUL-2387

Co-authored-by: multica-agent <github@multica.ai>
2026-05-18 19:41:01 +08:00
6 changed files with 315 additions and 8 deletions

View File

@@ -2807,3 +2807,125 @@ func TestInjectRuntimeConfigSquadLeaderCommentTriggeredNoAction(t *testing.T) {
t.Errorf("non-squad-leader CLAUDE.md should NOT contain squad leader rule")
}
}
// TestInjectRuntimeConfigCommentTriggerThreadFirstReads locks in MUL-2387:
// the runtime config's comment-triggered Workflow section must steer the
// agent at the thread-aware reads from PR #2787 first (--thread anchored on
// the trigger comment id, then --recent N with the stderr cursor for
// pagination) rather than the legacy "dump the whole comment list" recipe.
// The Available Commands core line also has to surface the new flags so the
// agent has a single place to discover them.
func TestInjectRuntimeConfigCommentTriggerThreadFirstReads(t *testing.T) {
t.Parallel()
const (
issueID = "issue-thread-1"
triggerID = "trigger-comment-1"
)
dir := t.TempDir()
ctx := TaskContextForEnv{
IssueID: issueID,
TriggerCommentID: triggerID,
}
if _, err := InjectRuntimeConfig(dir, "claude", ctx); err != nil {
t.Fatalf("InjectRuntimeConfig failed: %v", err)
}
data, err := os.ReadFile(filepath.Join(dir, "CLAUDE.md"))
if err != nil {
t.Fatalf("read CLAUDE.md: %v", err)
}
s := string(data)
// Workflow step 2 must read the trigger's thread with --thread anchored
// on the exact trigger comment id from this task.
for _, want := range []string{
"--thread " + triggerID,
"multica issue comment list " + issueID + " --thread " + triggerID + " --output json",
// --recent fallback at the documented default N=20.
"multica issue comment list " + issueID + " --recent 20 --output json",
// Cursor walks via the stderr line the CLI emits, not invented flags.
"Next thread cursor:",
"--before",
"--before-id",
// --since is still available and combinable.
"--since",
"may combine with `--thread` or `--recent`",
// Explicit pushback on the legacy full-dump recipe so the model has
// no reason to fall back to it on long issues.
"Avoid the unfiltered",
"wastes context",
} {
if !strings.Contains(s, want) {
t.Errorf("comment-triggered Workflow missing %q\n---\n%s", want, s)
}
}
// Available Commands core line must surface the new flags (this is the
// single discovery point for non-workflow CLI use cases).
for _, want := range []string{
"[--thread <comment-id>",
"--recent N [--before <ts> --before-id <root-id>]",
"Next thread cursor:",
} {
if !strings.Contains(s, want) {
t.Errorf("Available Commands core line missing %q\n---\n%s", want, s)
}
}
// The legacy step-2 phrasing this PR replaces must not regress.
if strings.Contains(s, "read the conversation (returns all comments, capped server-side at 2000)") {
t.Errorf("comment-triggered Workflow still carries the legacy full-dump phrasing\n---\n%s", s)
}
}
// TestInjectRuntimeConfigAssignmentTriggerMentionsRecent pins that the
// assignment-triggered Workflow keeps full-history reading as the mandatory
// default (the agent must still ingest earlier comments — that rule was
// added in MUL-1124) but ALSO points at `--recent N` as the long-issue
// alternative. Without this, the prompt would still be the only place
// telling the agent about --recent on busy issues.
func TestInjectRuntimeConfigAssignmentTriggerMentionsRecent(t *testing.T) {
t.Parallel()
dir := t.TempDir()
if _, err := InjectRuntimeConfig(dir, "claude", TaskContextForEnv{IssueID: "issue-1"}); err != nil {
t.Fatalf("InjectRuntimeConfig failed: %v", err)
}
data, err := os.ReadFile(filepath.Join(dir, "CLAUDE.md"))
if err != nil {
t.Fatalf("read CLAUDE.md: %v", err)
}
s := string(data)
// Mandatory full-history rule (MUL-1124) must stay.
for _, want := range []string{
"multica issue comment list issue-1 --output json",
"this is mandatory, not optional",
"Skipping this step is the most common cause",
} {
if !strings.Contains(s, want) {
t.Errorf("assignment Workflow regressed mandatory-history rule, missing %q\n---\n%s", want, s)
}
}
// AND --recent must be offered as the long-issue alternative.
for _, want := range []string{
"--recent 20 --output json",
"Next thread cursor:",
} {
if !strings.Contains(s, want) {
t.Errorf("assignment Workflow missing --recent guidance %q\n---\n%s", want, s)
}
}
// The previous wording framed `--recent` as a replacement ("you may
// switch to ..."), which conflicts with the mandatory full-history
// rule. Pin that the replacement semantics never reappears — `--recent`
// is a paging strategy, not a shortcut.
for _, banned := range []string{
"you may switch to",
"switch to `--recent",
} {
if strings.Contains(s, banned) {
t.Errorf("assignment Workflow regressed to replacement-style --recent phrasing %q\n---\n%s", banned, s)
}
}
}

View File

@@ -113,7 +113,7 @@ func buildMetaSkillContent(provider string, ctx TaskContextForEnv) string {
b.WriteString("The default brief includes the commands needed for the core agent loop and common issue create/update tasks. For everything else, run `multica --help`, `multica <command> --help`, or `multica <command> <subcommand> --help`; prefer `--output json` when the command supports it.\n\n")
b.WriteString("### Core\n")
b.WriteString("- `multica issue get <id> --output json` — Get full issue details.\n")
b.WriteString("- `multica issue comment list <issue-id> [--since <RFC3339>] --output json` — List comments on an issue; use `--since` for incremental polling.\n")
b.WriteString("- `multica issue comment list <issue-id> [--thread <comment-id> | --recent N [--before <ts> --before-id <root-id>]] [--since <RFC3339>] --output json` — List comments on an issue. Default returns everything (server cap 2000). On busy issues prefer the thread-aware reads: `--thread <comment-id>` returns one conversation (root + every reply), `--recent N` returns the N most recently active threads. `--before` / `--before-id` pair (printed as a `Next thread cursor:` line on stderr after a `--recent` page) scrolls to older threads. `--since` is for incremental polling and may combine with `--thread` or `--recent`.\n")
b.WriteString("- `multica issue create --title \"...\" [--description \"...\" | --description-stdin | --description-file <path>] [--priority X] [--status X] [--assignee X | --assignee-id <uuid>] [--parent <issue-id>] [--project <project-id>] [--due-date <RFC3339>] [--attachment <path>]` — Create a new issue; `--attachment` may be repeated.\n")
b.WriteString("- `multica issue update <id> [--title X] [--description X | --description-stdin | --description-file <path>] [--priority X] [--status X] [--assignee X | --assignee-id <uuid>] [--parent <issue-id>] [--project <project-id>] [--due-date <RFC3339>]` — Update issue fields; use `--parent \"\"` to clear parent.\n")
b.WriteString("- `multica repo checkout <url> [--ref <branch-or-sha>]` — Check out a repository into the working directory (creates a git worktree with a dedicated branch; use `--ref` for review/QA on a specific branch, tag, or commit)\n")
@@ -237,9 +237,10 @@ func buildMetaSkillContent(provider string, ctx TaskContextForEnv) string {
// Comment-triggered: focus on reading and replying
b.WriteString("**This task was triggered by a NEW comment.** Your primary job is to respond to THIS specific comment, even if you have handled similar requests before in this session.\n\n")
fmt.Fprintf(&b, "1. Run `multica issue get %s --output json` to understand the issue context\n", ctx.IssueID)
fmt.Fprintf(&b, "2. Run `multica issue comment list %s --output json` to read the conversation (returns all comments, capped server-side at 2000)\n", ctx.IssueID)
b.WriteString(" - For incremental polling, use `--since <RFC3339-timestamp>` to fetch only comments newer than a known cursor\n")
fmt.Fprintf(&b, "3. Find the triggering comment (ID: `%s`) and understand what is being asked — do NOT confuse it with previous comments\n", ctx.TriggerCommentID)
fmt.Fprintf(&b, "2. Read the triggering thread first — that is what this comment is actually about: `multica issue comment list %s --thread %s --output json` returns the root and every reply in the same thread as the trigger.\n", ctx.IssueID, ctx.TriggerCommentID)
fmt.Fprintf(&b, " - If the thread alone is not enough context, pull the most recently active threads on the issue: `multica issue comment list %s --recent 20 --output json`. Each `--recent` page also prints a `Next thread cursor: --before <ts> --before-id <root-id>` line on stderr; pass the same pair back as `--before <ts> --before-id <root-id>` to scroll to older threads when 20 still isn't enough.\n", ctx.IssueID)
b.WriteString(" - Avoid the unfiltered `multica issue comment list <issue-id> --output json` form on long-running issues — it dumps the entire flat timeline (cap 2000) and wastes context on chatter unrelated to the trigger. `--since <RFC3339-timestamp>` is still available for incremental polling against a known cursor and may combine with `--thread` or `--recent`.\n")
fmt.Fprintf(&b, "3. Find the triggering comment (ID: `%s`) inside the thread you just read and understand what is being asked — do NOT confuse it with previous comments\n", ctx.TriggerCommentID)
if ctx.IsSquadLeader {
b.WriteString("4. **Decide whether a reply is warranted.** If you produced actual work this turn (investigated, fixed, answered a real question), post the result via step 6 — that is a normal reply, not a noise comment. If the triggering comment was a pure acknowledgment / thanks / sign-off from another agent AND you produced no work this turn, do NOT post a reply — and do NOT post a comment saying 'No reply needed' or similar. Simply exit with no output. Silence is a valid and preferred way to end agent-to-agent conversations.\n")
fmt.Fprintf(&b, " - **Squad leader rule:** If your evaluation outcome is `no_action`, call `multica squad activity %s no_action --reason \"...\"` and then EXIT IMMEDIATELY. DO NOT post any comment whose only purpose is to announce that you are taking no action, exiting silently, or acknowledging another agent. A comment like \"No action needed\" or \"Exiting silently\" is noise — the `squad activity` call already records your decision in the timeline.\n", ctx.IssueID)
@@ -254,7 +255,7 @@ func buildMetaSkillContent(provider string, ctx TaskContextForEnv) string {
// Assignment-triggered: defer to agent Skills for workflow specifics.
b.WriteString("You are responsible for managing the issue status throughout your work.\n\n")
fmt.Fprintf(&b, "1. Run `multica issue get %s --output json` to understand your task\n", ctx.IssueID)
fmt.Fprintf(&b, "2. Run `multica issue comment list %s --output json` to read the full comment history (returns all comments, capped server-side at 2000) — this is mandatory, not optional. Earlier comments often carry context the issue body lacks (e.g. which repo to work in, the prior agent's findings, the reason the issue was reassigned to you). Skipping this step is the most common cause of agents acting on stale or incomplete instructions.\n", ctx.IssueID)
fmt.Fprintf(&b, "2. Run `multica issue comment list %s --output json` to read the full comment history (returns all comments, capped server-side at 2000) — this is mandatory, not optional. Earlier comments often carry context the issue body lacks (e.g. which repo to work in, the prior agent's findings, the reason the issue was reassigned to you). Skipping this step is the most common cause of agents acting on stale or incomplete instructions. When the flat dump is too large to ingest in one shot, treat `--recent 20 --output json` plus the `--before` / `--before-id` cursor (from the stderr `Next thread cursor:` line) as a paging strategy: keep walking older threads until you have read enough history to satisfy this mandatory step. `--recent` is a way to read the full history page-by-page, not a shortcut that replaces it.\n", ctx.IssueID)
fmt.Fprintf(&b, "3. Run `multica issue status %s in_progress`\n", ctx.IssueID)
b.WriteString("4. Follow your Skills and Agent Identity to complete the task (write code, investigate, etc.)\n")
if ctx.IsSquadLeader {

View File

@@ -31,7 +31,7 @@ func BuildPrompt(task Task, provider string) string {
b.WriteString("You are running as a local coding agent for a Multica workspace.\n\n")
fmt.Fprintf(&b, "Your assigned issue ID is: %s\n\n", task.IssueID)
fmt.Fprintf(&b, "Start by running `multica issue get %s --output json` to understand your task, then complete it.\n", task.IssueID)
fmt.Fprintf(&b, "If you need comment history, `multica issue comment list %s --output json` returns all comments for the issue (server caps at 2000). Pass `--since <RFC3339>` to fetch only comments newer than a known cursor.\n", task.IssueID)
fmt.Fprintf(&b, "For comment history, follow the rule in your runtime workflow file (assignment-triggered tasks treat the read as mandatory). `multica issue comment list %s --output json` returns all comments for the issue (server caps at 2000). On long-running issues use `--recent 20 --output json` to read the 20 most recently active threads, then page older threads via the stderr `Next thread cursor: ...` line and the matching `--before` / `--before-id` until you have enough history. `--since <RFC3339>` is still available for incremental polling and may combine with `--recent`.\n", task.IssueID)
return b.String()
}
@@ -147,7 +147,7 @@ func buildCommentPrompt(task Task, provider string) string {
}
}
fmt.Fprintf(&b, "Start by running `multica issue get %s --output json` to understand your task, then decide how to proceed.\n\n", task.IssueID)
fmt.Fprintf(&b, "If you need comment history, `multica issue comment list %s --output json` returns all comments for the issue (server caps at 2000). Pass `--since <RFC3339>` to fetch only comments newer than a known cursor.\n\n", task.IssueID)
fmt.Fprintf(&b, "For comment history, read the triggering thread first: `multica issue comment list %s --thread %s --output json` returns the root and every reply in the same thread as the trigger comment. If you still need more context, `multica issue comment list %s --recent 20 --output json` pulls the 20 most recently active threads on the issue (each `--recent` page prints a `Next thread cursor: --before <ts> --before-id <root-id>` line on stderr — pass the same pair back to scroll older threads). Avoid the unfiltered `--output json` form on long-running issues; it dumps the full flat timeline (cap 2000) and wastes context. `--since <RFC3339>` is still available for incremental polling and may combine with `--thread` or `--recent`.\n\n", task.IssueID, task.TriggerCommentID, task.IssueID)
b.WriteString(execenv.BuildCommentReplyInstructions(provider, task.IssueID, task.TriggerCommentID))
return b.String()
}

View File

@@ -200,6 +200,86 @@ func TestBuildPromptSquadLeaderNoActionForAgentTrigger(t *testing.T) {
}
}
// TestBuildPromptCommentTriggerPromotesThreadReads pins MUL-2387: the
// per-turn prompt for a comment-triggered task must steer the agent at the
// thread-aware reads first (--thread anchored on the trigger comment id,
// then --recent N as a fallback with cursor guidance) instead of the legacy
// "dump the entire comment list" recipe. Locking this in test stops the
// guidance from decaying back to a full-flat-dump on prompt edits.
func TestBuildPromptCommentTriggerPromotesThreadReads(t *testing.T) {
const (
issueID = "issue-thread-1"
triggerID = "trigger-comment-1"
)
task := Task{
IssueID: issueID,
TriggerCommentID: triggerID,
TriggerCommentContent: "anything",
TriggerAuthorType: "member",
TriggerAuthorName: "Bohan",
}
out := BuildPrompt(task, "claude")
mustContain := []string{
// Thread-first read pinned by trigger comment id.
"--thread " + triggerID,
"`multica issue comment list " + issueID + " --thread " + triggerID + " --output json`",
// --recent fallback uses the documented default N=20.
"--recent 20 --output json",
// Cursor walks via the stderr line the CLI emits, not invented flags.
"Next thread cursor:",
"--before",
"--before-id",
// --since is preserved as an additional, combinable knob.
"--since",
"may combine with `--thread` or `--recent`",
// Discourage the unfiltered full dump on long-running issues.
"Avoid the unfiltered",
"wastes context",
}
for _, s := range mustContain {
if !strings.Contains(out, s) {
t.Errorf("buildCommentPrompt missing thread-first guidance %q\n--- output ---\n%s", s, out)
}
}
// The old "dump everything via --output json alone" prose is exactly the
// pattern this PR is replacing — guard against the legacy phrasing
// sneaking back in.
if strings.Contains(out, "returns all comments for the issue (server caps at 2000)") {
t.Errorf("buildCommentPrompt still carries the legacy full-dump phrasing")
}
}
// TestBuildPromptDefaultMentionsRecent pins that the catch-all fallback
// prompt (no trigger comment, no chat, no autopilot, no quick-create) also
// teaches the agent about --recent as the long-issue-friendly alternative
// to the flat dump, even though it cannot anchor a --thread without a
// trigger comment id.
func TestBuildPromptDefaultMentionsRecent(t *testing.T) {
out := BuildPrompt(Task{IssueID: "issue-default-1"}, "claude")
for _, s := range []string{
"--recent 20 --output json",
"Next thread cursor:",
"--since",
} {
if !strings.Contains(out, s) {
t.Errorf("default BuildPrompt missing %q\n--- output ---\n%s", s, out)
}
}
// And the default path must NOT inject a --thread example, because there
// is no trigger comment id to anchor on.
if strings.Contains(out, "--thread") {
t.Errorf("default BuildPrompt should NOT mention --thread (no trigger comment to anchor on)\n--- output ---\n%s", out)
}
// The legacy "If you need comment history" soft phrasing conflicts with
// the assignment-trigger runtime workflow, which treats reading comments
// as mandatory. Guard against it sneaking back in.
if strings.Contains(out, "If you need comment history") {
t.Errorf("default BuildPrompt still carries the legacy 'If you need' soft phrasing that conflicts with the mandatory workflow\n--- output ---\n%s", out)
}
}
// TestBuildPromptNonSquadLeaderNoRule verifies that non-squad-leader agents
// do NOT get the squad leader no_action rule injected.
func TestBuildPromptNonSquadLeaderNoRule(t *testing.T) {

View File

@@ -382,8 +382,22 @@ func (h *Handler) fetchCommentsForList(ctx context.Context, args fetchCommentsAr
// Only emit a cursor when the page is full. Fewer threads than
// requested ⇒ the SELECT exhausted matching threads, so there is
// no older page to scroll to.
//
// Additionally suppress the cursor when `since` is set and the head
// thread's last_activity_at is already <= since. The pagination
// walks threads in strictly decreasing last_activity_at, so every
// older page has last_activity_at strictly less than the head's —
// if the head itself can't satisfy `> since`, no older thread can
// either. Predicating on the head (not on whether `comments` is
// empty) also catches the mixed case where this page keeps rows
// from fresher threads but the head thread is already past `since`.
// Flagged by Elon in #2787's second review (MUL-2340 nit).
out := fetchCommentsResult{Comments: comments}
if len(seenRoot) >= args.RecentN && headRoot.Valid && headLast.Valid {
emitCursor := len(seenRoot) >= args.RecentN && headRoot.Valid && headLast.Valid
if emitCursor && args.Since.Valid && !headLast.Time.After(args.Since.Time) {
emitCursor = false
}
if emitCursor {
out.NextBefore = headLast.Time.UTC().Format(time.RFC3339Nano)
out.NextBeforeID = uuidToString(headRoot)
}

View File

@@ -559,6 +559,96 @@ func TestListComments_FlagCombinationRules(t *testing.T) {
}
}
// TestListComments_RecentWithSinceFilteredEmptySuppressesCursor pins
// Elon's nit on PR #2787 / MUL-2340: a `recent + since` page whose every
// row gets dropped by the `since` filter must NOT emit a next-page cursor.
//
// Pagination walks threads in strictly decreasing last_activity_at. If the
// `since` filter empties this page, every comment in this page is <= since
// ⇒ head.last_activity_at <= since ⇒ every older thread (strictly less
// recent than head) also has last_activity_at < since ⇒ guaranteed empty.
// Emitting a cursor in that case would invite the caller into a wasted
// walk of pages that can never produce a row.
func TestListComments_RecentWithSinceFilteredEmptySuppressesCursor(t *testing.T) {
if testHandler == nil || testPool == nil {
t.Skip("database not available")
}
fx := newCommentListFixture(t)
// `since` = base + 1h (i.e. AFTER every comment in the fixture, where the
// freshest row sits at base + 12m). With recent=1 the page is technically
// "full" (1 thread out of 1 requested) so the legacy `len(seen) >= N`
// check would emit a cursor — but every row in that page is <= since, so
// the body is empty AND no older page can ever yield a >since row.
v := url.Values{}
v.Set("recent", "1")
v.Set("since", fx.Base.Add(1*time.Hour).UTC().Format(time.RFC3339Nano))
w, rows := listComments(t, fx.IssueID, v.Encode())
if len(rows) != 0 {
t.Fatalf("expected empty page after since-filter, got %d rows: %v", len(rows), ids(rows))
}
nb, nbid := nextThreadCursor(w)
if nb != "" || nbid != "" {
t.Fatalf("recent+since empty page must NOT emit cursor; got before=%q before_id=%q (this walks the caller into a guaranteed-empty pagination loop)", nb, nbid)
}
}
// TestListComments_RecentWithSinceKeepsCursorWhenPageHasRows is the
// counterpoint to TestListComments_RecentWithSinceFilteredEmptySuppressesCursor:
// if the `since` filter leaves at least one row in the page, the cursor must
// still be emitted (the suppression rule is narrowly scoped to the empty
// case so it can't accidentally swallow legitimate next-page hints).
func TestListComments_RecentWithSinceKeepsCursorWhenPageHasRows(t *testing.T) {
if testHandler == nil || testPool == nil {
t.Skip("database not available")
}
fx := newCommentListFixture(t)
// since = base + 11m30s drops everything from root1 (last_activity = base+3m)
// and from root2 except r2b (base+12m). With recent=1 (the freshest-active
// thread, root2), the page keeps r2b and the cursor still points at root2
// so the caller can scroll older threads if they want.
v := url.Values{}
v.Set("recent", "1")
v.Set("since", fx.Base.Add(11*time.Minute+30*time.Second).UTC().Format(time.RFC3339Nano))
w, rows := listComments(t, fx.IssueID, v.Encode())
eqIDs(t, ids(rows), []string{fx.R2b}, "recent=1 + since keeps r2b")
nb, nbid := nextThreadCursor(w)
if nb == "" || nbid != fx.Root2 {
t.Fatalf("non-empty recent+since page must keep cursor; got before=%q before_id=%q want root_id=%q", nb, nbid, fx.Root2)
}
}
// TestListComments_RecentWithSinceSuppressesCursorWhenHeadPastSince covers
// the mixed case from Elon's #2787 second review: the page is full (so the
// legacy `len(seen) >= N` check would emit a cursor) AND the `since` filter
// keeps rows from fresher threads (so the legacy `len(comments) == 0`
// suppression does NOT trip), but the head (oldest-active) thread already
// sits at or before `since`. Older pages walk strictly less-recent threads,
// so none of them can produce a `> since` comment — emitting a cursor here
// would still drag the caller into a guaranteed-empty next-page fetch.
//
// Fixture: root1 last_activity = base+3m, root2 last_activity = base+12m.
// recent=2 ⇒ both threads on the page, head = root1. since = base+5m drops
// everything in root1, keeps root2 entirely. Expect body = root2 thread,
// no cursor.
func TestListComments_RecentWithSinceSuppressesCursorWhenHeadPastSince(t *testing.T) {
if testHandler == nil || testPool == nil {
t.Skip("database not available")
}
fx := newCommentListFixture(t)
v := url.Values{}
v.Set("recent", "2")
v.Set("since", fx.Base.Add(5*time.Minute).UTC().Format(time.RFC3339Nano))
w, rows := listComments(t, fx.IssueID, v.Encode())
eqIDs(t, ids(rows), []string{fx.Root2, fx.R2a, fx.R2b}, "recent=2 + since keeps root2 thread")
nb, nbid := nextThreadCursor(w)
if nb != "" || nbid != "" {
t.Fatalf("head thread (root1, last_activity = base+3m) is already <= since (base+5m); older pages can't beat it. cursor must be suppressed, got before=%q before_id=%q", nb, nbid)
}
}
// TestListComments_ThreadWithSinceFiltersWithinThread proves the allowed
// combination from the rules: `thread + since` returns only comments in
// that thread newer than `since`. The since filter is applied in-memory