feat: read discover pagination from the request body [#51]

docs/adr/0017-discover-pagination.md

@@ -0,0 +1,61 @@
+# ADR-0017 — Discover pagination: signed body, applied page, capped `total`
+
+**Status:** Accepted
+**Date:** 2026-10-27
+
+## Context
+
+0.1.0 read `limit` and `offset` from the query string, which the request
+signature does not cover. The OAN profile of the spec moves them into
+`DiscoverAction` and answers with `DiscoverPagination` (`total`, `limit`,
+`offset`, all required), counted in resources rather than catalogs.
+
+## Decision
+
+1. **The page is read from the signed body.** `?limit=` or `?offset=` on
+   `/discover` is refused with `SCH_INVALID_FORMAT`, naming
+   `message.limit`/`message.offset`.
+2. **The unit is the resource.** The page is a slice of ranked resource ids;
+   a catalog may span pages.
+3. **An oversize `limit` is clamped to `maxPageSize`; a page past the
+   retrieval depth is refused.** `maxPageSize` may not sit below
+   `defaultPageSize`, which config validation enforces.
+4. **The response echoes the applied page**, not the requested one. `limit`
+   is the page size applied, not the count on the page: a short last page
+   still reports it.
+5. **`total` is the ranked list's length, capped at
+   `maxCandidatesPerMode`** — the last page a caller can fetch.
+6. **The cap is applied to the fused list before the page is cut.**
+   `maxCandidatesPerMode` counts ids *per retrieval mode*, so fusing several
+   modes yields a list longer than any one of them returned. Slicing first and
+   capping the count afterwards produces a last page carrying more resources
+   than `total` leaves room for, and `offset + count == total` never becomes
+   true. The storage layer caps, and the request layer reports what it is given.
+7. **Order is stable across pages.** RRF breaks score ties by id.
+
+## Alternatives considered
+
+- **Ignore query-string pagination.** Answers the default page with a 200; a
+  client walking offsets loops on page one with nothing telling it why.
+- **Refuse an oversize `limit`.** The caller would get no results for a
+  query that has them; clamping and echoing the applied value loses nothing.
+- **Echo the requested page.** A caller that asked for 5000 and got 100 would
+  compute its next offset from 5000 and skip results.
+- **An exact `total` from a count query.** About 100x the cost of the search
+  it accompanies, measured in A19, for a number no caller can page to.
+- **Order by score alone.** Equal scores are common — every id only one mode
+  returned, at the same rank — and two that swap between runs put one result
+  on two pages and another on none.
+
+## Consequences
+
+- Breaking for 0.1.0 callers that paginate with the query string.
+- `total` understates a broad query with more matches than the retrieval
+  depth. The spec allows this: a CN must treat it as the end of what is
+  pageable, not as an absolute match count.
+- A page past the retrieval depth is a 400, not an empty page.
+- `offset >= maxCandidatesPerMode` is the refusal test, not
+  `offset + limit > maxCandidatesPerMode`. `pageOf` already truncates a page
+  whose tail runs past the end, so testing the tail refuses the last page at
+  every limit that does not divide the depth exactly, and the caller never
+  reaches the `offset + count == total` that marks the end of the results.

docs/adr/README.md

@@ -29,6 +29,7 @@ Start from [`0000-template.md`](0000-template.md).
 | [0014](0014-seams-that-ship-with-only-a-no-op.md) | `CatalogReplicator`: what a seam must carry to ship | A7 |
 | [0015](0015-master-catalogs-and-inheritance-refused.md) | Master catalogs and resource inheritance are refused at intake | A1 |
 | [0016](0016-router-net-http-servemux.md) | `net/http.ServeMux` for HTTP routing, superseding 0001 | D1 |
+| [0017](0017-discover-pagination.md) | Discover pagination: signed body, applied page, capped `total` | #51 |
 
 **Source** points at the design document: `D` rows are the Technology Decisions
 table, `T` rows are TRD Alignment, `A` rows are Amendments.

examples/verify.sh

@@ -366,15 +366,16 @@ refusal_case 18-discover-unparsable-jsonpath.json \
   "SCH_INVALID_JSONPATH" "\$.message.intent.filters.expression"
 
 printf '%s\n' "--- pagination"
-body="$(curl -sS -X POST "$BASE/discover?limit=1&offset=0" \
-          -H 'Content-Type: application/json' \
-          --data-binary @"$DIR/03-discover-schema-context.json")"
+body="$(python3 -c 'import json,sys; b=json.load(open(sys.argv[1])); b["message"].update(limit=1, offset=0); print(json.dumps(b))' \
+          "$DIR/03-discover-schema-context.json" |
+        curl -sS -X POST "$BASE/discover" \
+          -H 'Content-Type: application/json' --data-binary @-)"
 got="$(resources "$body")"
 n="$(printf '%s' "$got" | awk -F, 'NF{print NF}')"
 if [ "${n:-0}" = "1" ]; then
-  ok "?limit=1 -> 1 resource [$got]"
+  ok "limit=1 -> 1 resource [$got]"
 else
-  bad "?limit=1 -> ${n:-0} resources [$got]"
+  bad "limit=1 -> ${n:-0} resources [$got]"
 fi
 echo
 

src/beckn/actions.go

@@ -137,16 +137,25 @@ type CatalogStats struct {
 // DiscoverAction is the `message` of a discover request.
 type DiscoverAction struct {
 	Intent Intent `json:"intent"`
+	Limit  int    `json:"limit,omitempty"`
+	Offset int    `json:"offset,omitempty"`
 }
 
-// OnDiscoverAction is the `message` of the discover response, and it is exactly
-// one field.
+// OnDiscoverAction is the `message` of the discover response.
 //
 // It must NOT grow a Degraded field. The v2.0.0 schema is
-// additionalProperties:false with `catalogs` as its only property, so an extra
+// additionalProperties:false with only `catalogs` and `pagination`, so an extra
 // key is a response that fails its own schema — and `omitempty` would hide that
 // on the ordinary path while shipping the invalid body on the degraded one, the
 // path that matters. The degraded list travels as X-Beckn-Degraded (C11).
 type OnDiscoverAction struct {
-	Catalogs []Catalog `json:"catalogs"`
+	Catalogs   []Catalog           `json:"catalogs"`
+	Pagination *DiscoverPagination `json:"pagination,omitempty"`
+}
+
+// DiscoverPagination is the page the answer applied (ADR-0017).
+type DiscoverPagination struct {
+	Limit  int `json:"limit"`
+	Offset int `json:"offset"`
+	Total  int `json:"total"`
 }

src/beckn/schema_conformance_test.go

@@ -69,6 +69,7 @@ func bindings() []binding {
 
 		{"DiscoverAction", DiscoverAction{}, schemaPath("DiscoverAction")},
 		{"OnDiscoverAction", OnDiscoverAction{}, schemaPath("OnDiscoverAction")},
+		{"DiscoverPagination", DiscoverPagination{}, schemaPath("DiscoverPagination")},
 
 		{"Nack", Nack{}, schemaPath("NackBadRequest")},
 		{"NackMessage", NackMessage{}, schemaPath("NackBadRequest", "properties", "message")},

src/discover/controller.go

@@ -2,10 +2,8 @@ package discover
 
 import (
 	"encoding/json"
-	"fmt"
 	"net/http"
 	"net/url"
-	"strconv"
 	"strings"
 	"time"
 
@@ -68,19 +66,19 @@ func (c *Controller) Discover(w http.ResponseWriter, r *http.Request) {
 
 	observeIntent(ctx, envelope.Context, action.Intent)
 
-	page, err := pageFrom(r.URL.Query())
-	if err != nil {
+	if err := rejectQueryStringPagination(r.URL.Query()); err != nil {
 		httpx.WriteNack(ctx, w, c.errors, envelope.Context.MessageID, err)
 		return
 	}
+	page := pageFrom(action)
 
-	catalogs, degraded, err := c.service.Discover(ctx, envelope.Context, action.Intent, page)
+	answer, degraded, err := c.service.Discover(ctx, envelope.Context, action.Intent, page)
 	if err != nil {
 		httpx.WriteNack(ctx, w, c.errors, envelope.Context.MessageID, err)
 		return
 	}
 
-	observeResult(ctx, catalogs)
+	observeResult(ctx, answer.Catalogs)
 
 	// Before the body: WriteJSON writes the status line, and a header set after
 	// that is a header nobody receives.
@@ -90,45 +88,26 @@ func (c *Controller) Discover(w http.ResponseWriter, r *http.Request) {
 
 	body := httpx.Envelope[beckn.OnDiscoverAction]{
 		Context: responseContext(envelope.Context),
-		Message: beckn.OnDiscoverAction{Catalogs: catalogs},
+		Message: answer,
 	}
 	if err := httpx.WriteJSON(ctx, w, http.StatusOK, body); err != nil {
 		httpx.WriteNack(ctx, w, c.errors, envelope.Context.MessageID, err)
 	}
 }
 
-// pageFrom reads pagination off the query string, where it lives rather than
-// inside the intent.
-//
-// An unreadable value is refused, not defaulted: zero already means "the default
-// page size" to the mapper, so reading `limit=twenty` as zero would answer a page
-// the caller never asked for and say nothing about it.
-func pageFrom(query url.Values) (Page, error) {
-	limit, err := intParam(query, "limit")
-	if err != nil {
-		return Page{}, err
-	}
-	offset, err := intParam(query, "offset")
-	if err != nil {
-		return Page{}, err
+// rejectQueryStringPagination refuses a page the request signature does not
+// cover (ADR-0017).
+func rejectQueryStringPagination(query url.Values) error {
+	if query.Has("limit") || query.Has("offset") {
+		return apperrors.Schema(beckn.CodeSchemaInvalidFormat,
+			"pagination moved from the query string to message.limit and message.offset").At(messageRoot)
 	}
-	return Page{Limit: limit, Offset: offset}, nil
+	return nil
 }
 
-// intParam reads one numeric query parameter. Absent is zero, which the mapper
-// resolves to its own default; present and unreadable is a refusal.
-func intParam(query url.Values, name string) (int, error) {
-	raw := query.Get(name)
-	if raw == "" {
-		return 0, nil
-	}
-
-	value, err := strconv.Atoi(raw)
-	if err != nil {
-		return 0, apperrors.Schema(beckn.CodeSchemaInvalidFormat,
-			fmt.Sprintf("%s is not a whole number", name)).At("$." + name)
-	}
-	return value, nil
+// pageFrom reads the page out of the signed body.
+func pageFrom(action beckn.DiscoverAction) Page {
+	return Page{Limit: action.Limit, Offset: action.Offset}
 }
 
 // responseContext turns the request's envelope into the response's. The

src/discover/controller_test.go

@@ -312,14 +312,12 @@ func TestAnUnavailableModeIsA400WhenTheDeploymentAsksToBe(t *testing.T) {
 	}
 }
 
-// Pagination arrives as query parameters rather than inside the intent, and an
-// oversize limit is clamped rather than refused (C11's neighbour: the caller
-// still gets the results they asked about).
-func TestLimitAndOffsetAreReadFromTheQueryString(t *testing.T) {
+// Pagination arrives in the body; an oversize limit is clamped (ADR-0017).
+func TestLimitAndOffsetAreReadFromTheBody(t *testing.T) {
 	repo := &stubRepo{capabilities: everything()}
 	handler := mount(t, repo, settings())
 
-	if got := post(t, handler, "/discover?limit=1000&offset=40", wheat).Code; got != http.StatusOK {
+	if got := post(t, handler, "/discover", `{"context":{"action":"discover"},"message":{"intent":{"textSearch":"wheat"},"limit":1000,"offset":40}}`).Code; got != http.StatusOK {
 		t.Fatalf("status = %d, want 200 — an oversize limit is clamped, not refused", got)
 	}
 	if repo.gotQuery.Limit != settings().Search.MaxPageSize {
@@ -330,13 +328,37 @@ func TestLimitAndOffsetAreReadFromTheQueryString(t *testing.T) {
 	}
 }
 
+// A 0.1.0 caller still sending ?limit= must get a 400, not a silent default page.
+func TestQueryStringPaginationIsRefused(t *testing.T) {
+	for _, path := range []string{"/discover?limit=5", "/discover?offset=40"} {
+		t.Run(path, func(t *testing.T) {
+			repo := &stubRepo{capabilities: everything()}
+
+			recorder := post(t, mount(t, repo, settings()), path, wheat)
+			if recorder.Code != http.StatusBadRequest {
+				t.Fatalf("status = %d, want 400; body: %s", recorder.Code, recorder.Body)
+			}
+			body := recorder.Body.String()
+			if !strings.Contains(body, string(beckn.CodeSchemaInvalidFormat)) {
+				t.Errorf("body = %s, want SCH_INVALID_FORMAT", body)
+			}
+			if !strings.Contains(body, "message.limit") {
+				t.Errorf("body = %s, want the refusal to name message.limit/offset", body)
+			}
+			if repo.calls != 0 {
+				t.Errorf("the backend was searched %d times; a refused page runs no query", repo.calls)
+			}
+		})
+	}
+}
+
 // A limit that is not a number is refused rather than read as zero. Zero means
 // "the default page size", so a typo would be answered confidently with a page
 // the caller did not ask for.
 func TestAPaginationParameterThatIsNotANumberIsRefused(t *testing.T) {
 	repo := &stubRepo{capabilities: everything()}
 
-	recorder := post(t, mount(t, repo, settings()), "/discover?limit=twenty", wheat)
+	recorder := post(t, mount(t, repo, settings()), "/discover", `{"context":{"action":"discover"},"message":{"intent":{"textSearch":"wheat"},"limit":"twenty"}}`)
 	if recorder.Code != http.StatusBadRequest {
 		t.Fatalf("status = %d, want 400; body: %s", recorder.Code, recorder.Body)
 	}
@@ -345,13 +367,12 @@ func TestAPaginationParameterThatIsNotANumberIsRefused(t *testing.T) {
 	}
 }
 
-// The same refusal for offset, which pageFrom reads with its own call to
-// intParam rather than reusing limit's — a valid limit must not mask an
-// unreadable offset.
+// The same refusal for offset — a valid limit must not mask an unreadable
+// offset.
 func TestAnUnreadableOffsetIsRefused(t *testing.T) {
 	repo := &stubRepo{capabilities: everything()}
 
-	recorder := post(t, mount(t, repo, settings()), "/discover?limit=10&offset=twenty", wheat)
+	recorder := post(t, mount(t, repo, settings()), "/discover", `{"context":{"action":"discover"},"message":{"intent":{"textSearch":"wheat"},"limit":10,"offset":"twenty"}}`)
 	if recorder.Code != http.StatusBadRequest {
 		t.Fatalf("status = %d, want 400; body: %s", recorder.Code, recorder.Body)
 	}

src/discover/intent_mapper.go

@@ -14,7 +14,7 @@ import (
 	"github.com/OpenAgriNet/discovery-service/src/platform/jsonpath"
 )
 
-// Page is the request's pagination, which arrives as HTTP query parameters
+// Page is the request's pagination, which arrives in the message body
 // rather than inside the intent. A struct rather than two adjacent ints, because
 // `limit, offset` and `offset, limit` compile identically.
 type Page struct {
@@ -207,12 +207,7 @@ func mapSchemaContext(envelope beckn.Context) ([]domain.SchemaFilter, []domain.F
 	return out, faults
 }
 
-// mapPage resolves the page, clamping what can be clamped honestly and refusing
-// what cannot.
-//
-// A limit over MaxPageSize is clamped, because the caller still gets the results
-// they asked about. A page past MaxCandidatesPerMode is refused, because it would
-// come back empty and an empty page 26 is indistinguishable from the end.
+// mapPage clamps an oversize limit, refuses a page past the depth (ADR-0017).
 func mapPage(page Page, search config.Search) (limit, offset int, faults []domain.Fault) {
 	limit = page.Limit
 	if limit <= 0 {
@@ -227,13 +222,13 @@ func mapPage(page Page, search config.Search) (limit, offset int, faults []domai
 		offset = 0
 	}
 
-	if offset+limit > search.MaxCandidatesPerMode {
+	if search.OffsetBeyondDepth(offset) {
 		faults = append(faults, domain.Fault{
 			Path: "$['offset']",
 			Code: string(beckn.CodeSchemaInvalidFormat),
 			Message: fmt.Sprintf(
-				"offset %d plus limit %d passes the retrieval depth of %d",
-				offset, limit, search.MaxCandidatesPerMode),
+				"offset %d starts past the retrieval depth of %d",
+				offset, search.MaxCandidatesPerMode),
 		})
 	}
 	return limit, offset, faults

src/discover/intent_mapper_test.go

@@ -501,16 +501,18 @@ func TestValidateConstraintReportsEveryFaultNotJustTheFirst(t *testing.T) {
 	}
 }
 
-// mapPage's own boundary: `>` refuses, so a page landing EXACTLY on the
-// retrieval depth must be answered, not refused. TestLimitIsClampedAndAPagePastTheRetrievalDepthIsRefused
-// only asserts the over side of this line.
-func TestAPageExactlyAtTheRetrievalDepthIsNotRefused(t *testing.T) {
+// mapPage's own boundary: the guard reads the offset alone, so the last
+// reachable offset must be answered however large the limit is. A limit that
+// overhangs the depth is the tail the page slice truncates, not a refusal —
+// otherwise the final results are unreachable at any limit the depth does not
+// divide evenly.
+func TestTheLastReachableOffsetIsNotRefused(t *testing.T) {
 	cfg := settings()
-	page := discover.Page{Limit: 100, Offset: cfg.Search.MaxCandidatesPerMode - 100}
+	page := discover.Page{Limit: 100, Offset: cfg.Search.MaxCandidatesPerMode - 1}
 
 	_, fatal, _ := discover.MapIntent(beckn.Intent{TextSearch: "wheat"}, beckn.Context{}, page, cfg)
 	if len(fatal) != 0 {
-		t.Fatalf("fatal = %s, want none — offset+limit lands exactly on the depth, not past it", codesOf(fatal))
+		t.Fatalf("fatal = %s, want none — the offset is inside the depth, only the limit overhangs it", codesOf(fatal))
 	}
 }