From f22382752e823b9f458b796a5769c0c14069bcbc Mon Sep 17 00:00:00 2001 From: shanglei Date: Sat, 25 Apr 2026 11:17:19 +0800 Subject: [PATCH 1/2] feat(pagination): preserve pagination state on truncation and natural end --- internal/client/client_test.go | 52 +++++++++++++++++++++++++++++++++- internal/client/pagination.go | 26 +++++++++++++++-- 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/internal/client/client_test.go b/internal/client/client_test.go index 5a97cecbb..66927e131 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -208,7 +208,7 @@ func TestPaginateAll_PageLimitStopsPagination(t *testing.T) { ac, errBuf := newTestAPIClient(t, rt) - _, err := ac.PaginateAll(context.Background(), RawApiRequest{ + result, err := ac.PaginateAll(context.Background(), RawApiRequest{ Method: "GET", URL: "/open-apis/test", As: "bot", @@ -223,6 +223,56 @@ func TestPaginateAll_PageLimitStopsPagination(t *testing.T) { if !strings.Contains(errBuf.String(), "reached page limit (2), stopping. Use --page-all --page-limit 0 to fetch all pages.") { t.Errorf("expected page limit log, got: %s", errBuf.String()) } + + // Truncation must surface in the merged output: has_more stays true and the + // next page_token is preserved so callers can detect loss and resume. + resultMap, _ := result.(map[string]interface{}) + data, _ := resultMap["data"].(map[string]interface{}) + if hasMore, _ := data["has_more"].(bool); !hasMore { + t.Errorf("expected has_more=true when page limit truncates, got false") + } + if token, _ := data["page_token"].(string); token != "next" { + t.Errorf("expected page_token=\"next\" preserved on truncation, got %q", token) + } +} + +func TestPaginateAll_NaturalEndClearsPageToken(t *testing.T) { + apiCalls := 0 + rt := roundTripFunc(func(req *http.Request) (*http.Response, error) { + apiCalls++ + hasMore := apiCalls < 2 + body := map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "items": []interface{}{map[string]interface{}{"id": apiCalls}}, + "has_more": hasMore, + }, + } + if hasMore { + body["data"].(map[string]interface{})["page_token"] = "next" + } + return jsonResponse(body), nil + }) + + ac, _ := newTestAPIClient(t, rt) + + result, err := ac.PaginateAll(context.Background(), RawApiRequest{ + Method: "GET", + URL: "/open-apis/test", + As: "bot", + }, PaginationOptions{PageLimit: 10, PageDelay: 0}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + resultMap, _ := result.(map[string]interface{}) + data, _ := resultMap["data"].(map[string]interface{}) + if hasMore, _ := data["has_more"].(bool); hasMore { + t.Errorf("expected has_more=false at natural end, got true") + } + if _, exists := data["page_token"]; exists { + t.Errorf("expected page_token absent at natural end, got %v", data["page_token"]) + } } func TestBuildApiReq_QueryParams(t *testing.T) { diff --git a/internal/client/pagination.go b/internal/client/pagination.go index ecddcc1c9..965b28aa4 100644 --- a/internal/client/pagination.go +++ b/internal/client/pagination.go @@ -71,9 +71,29 @@ func mergePagedResults(w io.Writer, results []interface{}) interface{} { mergedData[k] = v } mergedData[arrayField] = merged - mergedData["has_more"] = false - delete(mergedData, "page_token") - delete(mergedData, "next_page_token") + + // Preserve the last page's real pagination state so callers can detect + // truncation when --page-limit stops the loop before the API is exhausted. + lastHasMore := false + var lastPageToken, lastNextPageToken string + if lastMap, ok := results[len(results)-1].(map[string]interface{}); ok { + if lastData, ok := lastMap["data"].(map[string]interface{}); ok { + lastHasMore, _ = lastData["has_more"].(bool) + lastPageToken, _ = lastData["page_token"].(string) + lastNextPageToken, _ = lastData["next_page_token"].(string) + } + } + mergedData["has_more"] = lastHasMore + if lastHasMore && lastPageToken != "" { + mergedData["page_token"] = lastPageToken + } else { + delete(mergedData, "page_token") + } + if lastHasMore && lastNextPageToken != "" { + mergedData["next_page_token"] = lastNextPageToken + } else { + delete(mergedData, "next_page_token") + } result := make(map[string]interface{}) for k, v := range firstMap { From 2636502d694c639d1a7ac14aa5abb36ee120f3fe Mon Sep 17 00:00:00 2001 From: shanglei Date: Sat, 25 Apr 2026 16:47:10 +0800 Subject: [PATCH 2/2] feat(pagination): drop page_token from merged output to reflect aggregate view --- internal/client/client_test.go | 9 +++++---- internal/client/pagination.go | 21 ++++++--------------- 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/internal/client/client_test.go b/internal/client/client_test.go index 66927e131..88e991068 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -224,15 +224,16 @@ func TestPaginateAll_PageLimitStopsPagination(t *testing.T) { t.Errorf("expected page limit log, got: %s", errBuf.String()) } - // Truncation must surface in the merged output: has_more stays true and the - // next page_token is preserved so callers can detect loss and resume. + // Truncation must surface in the merged output: has_more stays true so + // callers can detect loss. page_token is intentionally dropped from the + // aggregate view — to fetch more, re-run with a larger --page-limit. resultMap, _ := result.(map[string]interface{}) data, _ := resultMap["data"].(map[string]interface{}) if hasMore, _ := data["has_more"].(bool); !hasMore { t.Errorf("expected has_more=true when page limit truncates, got false") } - if token, _ := data["page_token"].(string); token != "next" { - t.Errorf("expected page_token=\"next\" preserved on truncation, got %q", token) + if _, exists := data["page_token"]; exists { + t.Errorf("expected page_token to be dropped from merged output, got %v", data["page_token"]) } } diff --git a/internal/client/pagination.go b/internal/client/pagination.go index 965b28aa4..3476db9ad 100644 --- a/internal/client/pagination.go +++ b/internal/client/pagination.go @@ -72,28 +72,19 @@ func mergePagedResults(w io.Writer, results []interface{}) interface{} { } mergedData[arrayField] = merged - // Preserve the last page's real pagination state so callers can detect - // truncation when --page-limit stops the loop before the API is exhausted. + // Surface the last page's real has_more so callers can detect truncation + // when --page-limit stops the loop before the API is exhausted. Page tokens + // are intentionally dropped: the merged view is an aggregate, not a resume + // cursor — to fetch more, re-run with a larger --page-limit. lastHasMore := false - var lastPageToken, lastNextPageToken string if lastMap, ok := results[len(results)-1].(map[string]interface{}); ok { if lastData, ok := lastMap["data"].(map[string]interface{}); ok { lastHasMore, _ = lastData["has_more"].(bool) - lastPageToken, _ = lastData["page_token"].(string) - lastNextPageToken, _ = lastData["next_page_token"].(string) } } mergedData["has_more"] = lastHasMore - if lastHasMore && lastPageToken != "" { - mergedData["page_token"] = lastPageToken - } else { - delete(mergedData, "page_token") - } - if lastHasMore && lastNextPageToken != "" { - mergedData["next_page_token"] = lastNextPageToken - } else { - delete(mergedData, "next_page_token") - } + delete(mergedData, "page_token") + delete(mergedData, "next_page_token") result := make(map[string]interface{}) for k, v := range firstMap {