Skip to content

feat(referrer): add cursor-based pagination to discover endpoints#2896

Merged
migmartri merged 6 commits intochainloop-dev:mainfrom
migmartri:add-pagination-referrer
Mar 26, 2026
Merged

feat(referrer): add cursor-based pagination to discover endpoints#2896
migmartri merged 6 commits intochainloop-dev:mainfrom
migmartri:add-pagination-referrer

Conversation

@migmartri
Copy link
Copy Markdown
Member

@migmartri migmartri commented Mar 20, 2026

Summary

  • Add optional cursor-based pagination (keyset on created_at, id) to DiscoverPrivate and DiscoverPublicShared RPCs, defaulting to 20 items per page
  • CLI referrer discover gains --limit and --next flags following the workflow run list pattern
  • Backward-compatible: existing clients that omit pagination transparently receive the first page

closes #2890

Test plan

  • Integration tests cover: limit > total, limit = total, limit < total, limit=1 cursor traversal, nil backward compat
  • Existing referrer integration tests updated and passing
  • Unit tests pass for controlplane and CLI

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 19 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="app/controlplane/pkg/data/referrer.go">

<violation number="1" location="app/controlplane/pkg/data/referrer.go:243">
P1: Ordering is not actually descending: `ent.Desc()` is called without fields, so keyset pagination uses the wrong sort direction and can return incorrect pages.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread app/controlplane/pkg/data/referrer.go Outdated
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 3 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="app/controlplane/internal/service/referrer.go">

<violation number="1" location="app/controlplane/internal/service/referrer.go:114">
P2: This now uses the global cursor default (10) when pagination is present without `limit`, which changes the referrer endpoint default page size from 20 to 10.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread app/controlplane/internal/service/referrer.go Outdated
@migmartri migmartri requested review from Piskoo, javirln and jiparis March 26, 2026 11:23
Copy link
Copy Markdown
Member

@jiparis jiparis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @migmartri . Please check those failing tests

Add optional cursor-based pagination to DiscoverPrivate and
DiscoverPublicShared RPCs. References inside ReferrerItem are now
paginated using keyset pagination on (created_at, id), defaulting
to 20 items per page. Existing clients that omit pagination get
the first page transparently.

CLI gains --limit and --next flags on `referrer discover`.

Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Entire-Checkpoint: 2495d6f80461
ent.Desc() with no arguments is a no-op, so the query was ordering
ASC instead of DESC. This caused CompositeLT cursor comparisons to
find no rows after the first page. Use ent.Desc(fields...) to match
the pattern used in workflowrun.go.

Also regenerate CLI docs to include the new pagination flags.

Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
- Return nil pagination opts when proto request has no pagination field,
  preserving backward compatibility (all references returned for old clients).
- Merge CLI pagination hint into a single log message without trailing \n.
- Make test assertions order-independent since DESC ordering changes
  reference order non-deterministically when timestamps are equal.

Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
ent.Desc() without field arguments is a no-op. Use
sql.OrderDesc() as OrderTermOption to ByCreatedAt/ByID
for correct descending sort in pagination queries.

Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Entire-Checkpoint: 2dc882476ce5
When a CursorPaginationRequest is present without an explicit limit,
default to 20 instead of the global cursor default of 10. This matches
the spec requirement for the referrer endpoints.

Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Entire-Checkpoint: 7b9d8e75f733
Return the full discover result (with nextCursor) in the JSON response
instead of only the referrer item. Also fix staticcheck lint warning
about redundant embedded field selector in integration test.

Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
@migmartri migmartri force-pushed the add-pagination-referrer branch from c44744c to 0ab9454 Compare March 26, 2026 21:39
@migmartri migmartri merged commit a20a930 into chainloop-dev:main Mar 26, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Referrer discover API returns unbounded references

2 participants