[object Object]

← back to Cli Printing Press

fix(cli): infer pagination defaults from plain params; preserve cursor=0 in paginatedGet (#1115)

10cc48a822022450b12109c1d13fd09dddb04033 · 2026-05-11 15:00:49 -0700 · Trevin Chow

* fix(cli): infer pagination defaults from plain params; preserve cursor=0 in paginatedGet

The profiler only counted cursorParams/pageSizeParams when an endpoint had an
explicit pagination: block. Internal YAML specs that declare offset/count/limit
as plain query params fell back to the literal "after"/"limit" defaults, so
generated sync code requested the wrong param names against APIs like OneDev,
Clerk, and Loops.

When no explicit pagination: block is set on an endpoint, fall back to
inferring cursor and page-size names from plain non-positional, non-path
params using shared candidate sets (also reused by hasRequiredScopeParams).

paginatedGet's clean loop stripped "0"/"false" defaults for every param.
Reasonable for filter flags, wrong for the cursor param when the API uses
offset-style pagination: an int cursor flag defaulting to 0 formats to "0",
gets stripped, and the API rejects the request as missing offset. Exempt the
cursor param so its "0" survives the strip while non-cursor "0"/"false"
filters are still dropped.

Refs #927

* test(cli): tighten paginatedGet cursor-strip test to pin semantics, not exact source

Greptile flagged the prior assertion as brittle to whitespace and semantically
equivalent reformulations. Scope the contains/not-contains assertions to the
clean-loop body (between 'clean := map[string]string{}' and 'if !fetchAll'),
and pin two structural properties:

- The clean loop references cursorParam (proves cursor-awareness).
- The old unconditional v != "" && v != "0" && v != "false" filter is gone.

Either property failing implies a real regression; gofmt-style reformulations
that keep both properties pass.

Refs #927

Files touched

Diff

commit 10cc48a822022450b12109c1d13fd09dddb04033
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Mon May 11 15:00:49 2026 -0700

    fix(cli): infer pagination defaults from plain params; preserve cursor=0 in paginatedGet (#1115)
    
    * fix(cli): infer pagination defaults from plain params; preserve cursor=0 in paginatedGet
    
    The profiler only counted cursorParams/pageSizeParams when an endpoint had an
    explicit pagination: block. Internal YAML specs that declare offset/count/limit
    as plain query params fell back to the literal "after"/"limit" defaults, so
    generated sync code requested the wrong param names against APIs like OneDev,
    Clerk, and Loops.
    
    When no explicit pagination: block is set on an endpoint, fall back to
    inferring cursor and page-size names from plain non-positional, non-path
    params using shared candidate sets (also reused by hasRequiredScopeParams).
    
    paginatedGet's clean loop stripped "0"/"false" defaults for every param.
    Reasonable for filter flags, wrong for the cursor param when the API uses
    offset-style pagination: an int cursor flag defaulting to 0 formats to "0",
    gets stripped, and the API rejects the request as missing offset. Exempt the
    cursor param so its "0" survives the strip while non-cursor "0"/"false"
    filters are still dropped.
    
    Refs #927
    
    * test(cli): tighten paginatedGet cursor-strip test to pin semantics, not exact source
    
    Greptile flagged the prior assertion as brittle to whitespace and semantically
    equivalent reformulations. Scope the contains/not-contains assertions to the
    clean-loop body (between 'clean := map[string]string{}' and 'if !fetchAll'),
    and pin two structural properties:
    
    - The clean loop references cursorParam (proves cursor-awareness).
    - The old unconditional v != "" && v != "0" && v != "false" filter is gone.
    
    Either property failing implies a real regression; gofmt-style reformulations
    that keep both properties pass.
    
    Refs #927
---
 internal/generator/generator_test.go               |  22 ++++
 internal/generator/templates/helpers.go.tmpl       |   8 +-
 internal/profiler/profiler.go                      |  52 ++++++---
 internal/profiler/profiler_test.go                 | 119 +++++++++++++++++++++
 .../internal/cli/helpers.go                        |   8 +-
 .../printing-press-golden/internal/cli/helpers.go  |   8 +-
 6 files changed, 198 insertions(+), 19 deletions(-)

diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go
index 4680161a..82938ebe 100644
--- a/internal/generator/generator_test.go
+++ b/internal/generator/generator_test.go
@@ -3589,6 +3589,28 @@ func TestCompactListFieldsPreservesUnknownShapes(t *testing.T) {
 	}
 }
 
+// The cursor param's "0" must survive paginatedGet's zero-value strip:
+// offset-paginated APIs require offset=0 on the first page request.
+func TestPaginatedGetExemptsCursorParamFromZeroStripping(t *testing.T) {
+	t.Parallel()
+
+	path := filepath.Join("templates", "helpers.go.tmpl")
+	data, err := os.ReadFile(path)
+	require.NoError(t, err, "template must exist: %s", path)
+	body := string(data)
+
+	cleanStart := strings.Index(body, "clean := map[string]string{}")
+	require.GreaterOrEqual(t, cleanStart, 0, "paginatedGet must declare a clean map")
+	loopEnd := strings.Index(body[cleanStart:], "if !fetchAll")
+	require.GreaterOrEqual(t, loopEnd, 0, "expected fetchAll branch after clean loop")
+	cleanBlock := body[cleanStart : cleanStart+loopEnd]
+
+	assert.Contains(t, cleanBlock, "cursorParam",
+		"paginatedGet's clean loop must reference cursorParam so the cursor key is exempt from zero-stripping")
+	assert.NotContains(t, cleanBlock, `if v != "" && v != "0" && v != "false"`,
+		`the unconditional v != "" && v != "0" && v != "false" filter incorrectly drops cursor="0" for offset-paginated APIs`)
+}
+
 // TestPipedJsonGateRespectsExplicitFormatFlags pins the contract: the
 // piped-output auto-JSON gate must defer to explicit --csv / --quiet /
 // --plain flags so piped consumers that asked for a non-JSON format
diff --git a/internal/generator/templates/helpers.go.tmpl b/internal/generator/templates/helpers.go.tmpl
index 952f6521..5b7ab10c 100644
--- a/internal/generator/templates/helpers.go.tmpl
+++ b/internal/generator/templates/helpers.go.tmpl
@@ -587,10 +587,14 @@ func replacePathParam(path, name, value string) string {
 func paginatedGet(c interface {
 	GetWithHeaders(path string, params map[string]string, headers map[string]string) (json.RawMessage, error)
 }, path string, params map[string]string, headers map[string]string, fetchAll bool, cursorParam, nextCursorPath, hasMoreField string) (json.RawMessage, error) {
-	// Clean zero-value params
+	// Cursor params are exempt from the "0"/"false" strip: offset-paginated
+	// APIs send offset=0 on the first page.
 	clean := map[string]string{}
 	for k, v := range params {
-		if v != "" && v != "0" && v != "false" {
+		if v == "" {
+			continue
+		}
+		if k == cursorParam || (v != "0" && v != "false") {
 			clean[k] = v
 		}
 	}
diff --git a/internal/profiler/profiler.go b/internal/profiler/profiler.go
index bed949a0..ec9130c6 100644
--- a/internal/profiler/profiler.go
+++ b/internal/profiler/profiler.go
@@ -397,6 +397,21 @@ func Profile(s *spec.APISpec) *APIProfile {
 				if endpoint.Pagination.LimitParam != "" {
 					pageSizeParams[endpoint.Pagination.LimitParam]++
 				}
+			} else {
+				// Fallback for specs that expose pagination via plain params
+				// instead of a structured pagination: block.
+				for _, param := range endpoint.Params {
+					if param.PathParam || param.Positional {
+						continue
+					}
+					lower := strings.ToLower(param.Name)
+					if cursorParamCandidates[lower] {
+						cursorParams[param.Name]++
+					}
+					if pageSizeParamCandidates[lower] {
+						pageSizeParams[param.Name]++
+					}
+				}
 			}
 			if endpoint.ResponsePath != "" {
 				responsePaths[endpoint.ResponsePath]++
@@ -672,26 +687,37 @@ func dataFit(v bool) int {
 	return 1
 }
 
-// hasRequiredScopeParams returns true if the endpoint has required query parameters
-// that aren't pagination-related. These are "scoped list" endpoints (e.g., GetFriendList
+// Lowercase-keyed candidate sets shared by the profiler's pagination
+// inference path and hasRequiredScopeParams.
+var (
+	pageSizeParamCandidates = map[string]bool{
+		"limit": true, "per_page": true, "page_size": true, "pagesize": true,
+		"first": true, "count": true, "max_results": true, "page[size]": true,
+	}
+	cursorParamCandidates = map[string]bool{
+		"after": true, "cursor": true, "page_token": true, "offset": true,
+		"page": true, "before": true, "starting_after": true, "page[cursor]": true,
+	}
+)
+
+// hasRequiredScopeParams flags "scoped list" endpoints (e.g., GetFriendList
 // requires steamid) that can't be synced without runtime context.
 func hasRequiredScopeParams(endpoint spec.Endpoint) bool {
-	paginationParams := map[string]bool{
-		"limit": true, "per_page": true, "page_size": true, "pageSize": true, "first": true, "count": true, "max_results": true,
-		"after": true, "cursor": true, "page_token": true, "offset": true, "page": true, "before": true, "starting_after": true,
-		"page[cursor]": true, "page[size]": true,
+	temporalOrFormatParams := map[string]bool{
 		"since": true, "updated_after": true, "modified_since": true, "since_id": true,
-		"key": true, "format": true, // auth and format params, not scope
+		"key": true, "format": true,
 	}
 	for _, param := range endpoint.Params {
 		if param.Required && !param.Positional && !param.PathParam {
-			if !paginationParams[param.Name] && !paginationParams[strings.ToLower(param.Name)] {
-				// Enum params with 2+ values are handled by enum expansion, not scope
-				if len(param.Enum) >= 2 {
-					continue
-				}
-				return true
+			lower := strings.ToLower(param.Name)
+			if pageSizeParamCandidates[lower] || cursorParamCandidates[lower] || temporalOrFormatParams[lower] {
+				continue
 			}
+			// Enum params with 2+ values are handled by enum expansion, not scope
+			if len(param.Enum) >= 2 {
+				continue
+			}
+			return true
 		}
 	}
 	return false
diff --git a/internal/profiler/profiler_test.go b/internal/profiler/profiler_test.go
index 6e350b02..4a77663a 100644
--- a/internal/profiler/profiler_test.go
+++ b/internal/profiler/profiler_test.go
@@ -1696,3 +1696,122 @@ func TestProfileSpecWalker_MultiPlaceholderPathWarns(t *testing.T) {
 		assert.True(t, found, "walker with explicit key_param must produce a dependent entry")
 	})
 }
+
+// Specs that declare pagination via plain offset+count query params (no
+// explicit pagination: block) must infer the cursor and limit names from
+// those params instead of falling back to "after"/"limit".
+func TestProfilePagination_InfersFromPlainParamsWhenNoExplicitBlock(t *testing.T) {
+	s := &spec.APISpec{
+		Name: "plain-param-pagination",
+		Resources: map[string]spec.Resource{
+			"agents": {
+				Endpoints: map[string]spec.Endpoint{
+					"list": {
+						Method:   "GET",
+						Path:     "/agents",
+						Params:   []spec.Param{{Name: "offset", Type: "int"}, {Name: "count", Type: "int"}},
+						Response: spec.ResponseDef{Type: "array"},
+					},
+				},
+			},
+			"builds": {
+				Endpoints: map[string]spec.Endpoint{
+					"list": {
+						Method:   "GET",
+						Path:     "/builds",
+						Params:   []spec.Param{{Name: "offset", Type: "int"}, {Name: "count", Type: "int"}},
+						Response: spec.ResponseDef{Type: "array"},
+					},
+				},
+			},
+		},
+	}
+
+	profile := Profile(s)
+	assert.Equal(t, "offset", profile.Pagination.CursorParam, "plain offset param must be picked up")
+	assert.Equal(t, "count", profile.Pagination.PageSizeParam, "plain count param must be picked up as limit")
+}
+
+// Explicit pagination: blocks must continue to win over plain-param inference.
+// Mixing the two on the same endpoint would otherwise double-count or let
+// inference shadow the author's deliberate choice.
+func TestProfilePagination_ExplicitBlockWinsOverInference(t *testing.T) {
+	s := &spec.APISpec{
+		Name: "explicit",
+		Resources: map[string]spec.Resource{
+			"items": {
+				Endpoints: map[string]spec.Endpoint{
+					"list": {
+						Method: "GET",
+						Path:   "/items",
+						Params: []spec.Param{
+							{Name: "offset", Type: "int"},
+							{Name: "count", Type: "int"},
+						},
+						Pagination: &spec.Pagination{
+							Type:        "cursor",
+							CursorParam: "foo",
+							LimitParam:  "bar",
+						},
+						Response: spec.ResponseDef{Type: "array"},
+					},
+				},
+			},
+		},
+	}
+
+	profile := Profile(s)
+	assert.Equal(t, "foo", profile.Pagination.CursorParam, "explicit cursor_param must win")
+	assert.Equal(t, "bar", profile.Pagination.PageSizeParam, "explicit limit_param must win")
+}
+
+// Specs with no recognizable pagination shape must keep the historical
+// after/limit defaults so existing golden output doesn't churn.
+func TestProfilePagination_NoPaginationParamsKeepsDefaults(t *testing.T) {
+	s := &spec.APISpec{
+		Name: "no-pagination",
+		Resources: map[string]spec.Resource{
+			"things": {
+				Endpoints: map[string]spec.Endpoint{
+					"list": {
+						Method:   "GET",
+						Path:     "/things",
+						Params:   []spec.Param{{Name: "filter", Type: "string"}},
+						Response: spec.ResponseDef{Type: "array"},
+					},
+				},
+			},
+		},
+	}
+
+	profile := Profile(s)
+	assert.Equal(t, "after", profile.Pagination.CursorParam)
+	assert.Equal(t, "limit", profile.Pagination.PageSizeParam)
+}
+
+// Inference must skip path params and positional args even when their names
+// match candidate sets (e.g. an /items/{page} path segment named "page").
+func TestProfilePagination_SkipsPathAndPositionalParams(t *testing.T) {
+	s := &spec.APISpec{
+		Name: "scoped",
+		Resources: map[string]spec.Resource{
+			"items": {
+				Endpoints: map[string]spec.Endpoint{
+					"list": {
+						Method: "GET",
+						Path:   "/items/{page}",
+						Params: []spec.Param{
+							{Name: "page", Type: "string", PathParam: true},
+							{Name: "offset", Type: "int", Positional: true},
+						},
+						Response: spec.ResponseDef{Type: "array"},
+					},
+				},
+			},
+		},
+	}
+
+	profile := Profile(s)
+	assert.Equal(t, "after", profile.Pagination.CursorParam, "path-param 'page' must not be treated as a cursor")
+	assert.Equal(t, "limit", profile.Pagination.PageSizeParam)
+}
diff --git a/testdata/golden/expected/generate-golden-api-rich-auth/printing-press-rich-auth/internal/cli/helpers.go b/testdata/golden/expected/generate-golden-api-rich-auth/printing-press-rich-auth/internal/cli/helpers.go
index 88a781db..74a5990e 100644
--- a/testdata/golden/expected/generate-golden-api-rich-auth/printing-press-rich-auth/internal/cli/helpers.go
+++ b/testdata/golden/expected/generate-golden-api-rich-auth/printing-press-rich-auth/internal/cli/helpers.go
@@ -387,10 +387,14 @@ func newTabWriter(w io.Writer) *tabwriter.Writer {
 func paginatedGet(c interface {
 	GetWithHeaders(path string, params map[string]string, headers map[string]string) (json.RawMessage, error)
 }, path string, params map[string]string, headers map[string]string, fetchAll bool, cursorParam, nextCursorPath, hasMoreField string) (json.RawMessage, error) {
-	// Clean zero-value params
+	// Cursor params are exempt from the "0"/"false" strip: offset-paginated
+	// APIs send offset=0 on the first page.
 	clean := map[string]string{}
 	for k, v := range params {
-		if v != "" && v != "0" && v != "false" {
+		if v == "" {
+			continue
+		}
+		if k == cursorParam || (v != "0" && v != "false") {
 			clean[k] = v
 		}
 	}
diff --git a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/helpers.go b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/helpers.go
index 50097e02..13562857 100644
--- a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/helpers.go
+++ b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/helpers.go
@@ -390,10 +390,14 @@ func replacePathParam(path, name, value string) string {
 func paginatedGet(c interface {
 	GetWithHeaders(path string, params map[string]string, headers map[string]string) (json.RawMessage, error)
 }, path string, params map[string]string, headers map[string]string, fetchAll bool, cursorParam, nextCursorPath, hasMoreField string) (json.RawMessage, error) {
-	// Clean zero-value params
+	// Cursor params are exempt from the "0"/"false" strip: offset-paginated
+	// APIs send offset=0 on the first page.
 	clean := map[string]string{}
 	for k, v := range params {
-		if v != "" && v != "0" && v != "false" {
+		if v == "" {
+			continue
+		}
+		if k == cursorParam || (v != "0" && v != "false") {
 			clean[k] = v
 		}
 	}

← c626efca fix(cli): emit default Accept */* header in generated HTTP c  ·  back to Cli Printing Press  ·  fix(cli): suppress max_pages_cap_hit warning under --latest- 5f2bde7f →