mirror of
https://github.com/multica-ai/multica.git
synced 2026-06-29 02:19:19 +02:00
Compare commits
3 Commits
agent/lamb
...
agent/j/f0
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
e02029337d | ||
|
|
bb49820146 | ||
|
|
669f259947 |
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user