perf(api): exact keyset cursors for the new sort

CHANGELOG.md

@@ -8,6 +8,20 @@ Format loosely follows [Keep a Changelog](https://keepachangelog.com/).
 This run took the PoC from a rough, non-booting prototype to a running,
 test-covered Reddit clone. Highlights, newest first:
 
+### Exact cursors for the `new` sort
+- **`new` no longer pages by window.** Its key, `(created_at, id)`, never moves
+  once a post is written, so the database seeks straight to the cursor row with a
+  row-value comparison and returns only the page — no depth cap, nothing scanned.
+  Walking backwards runs the comparison the other way and flips the rows, so
+  callers always see one order.
+- A cursor id that no longer exists makes the subquery NULL, so the comparison is
+  NULL and no rows return — a stale cursor reads as "nothing after this".
+- The **ranked** sorts keep the windowed cap, which is a property of the data:
+  `hot`/`controversial`/`rising` rank on live vote counts, so a cursor into them
+  is approximate however it is implemented. `Posts.KEYSET_SORTS` lists what
+  qualifies, and `get_listing` asserts if a cursor is passed with a sort that
+  does not — that failure would be silent and subtly wrong otherwise.
+
 ### The JSON API stops reading the whole table for a page
 - **Listing endpoints fetched every matching row** and then ranked them in Lua,
   even after the web listings moved to SQL. They now pass `sort` to

app/spec/api_pagination_spec.lua

@@ -132,6 +132,91 @@ describe("api listing pagination", function()
 		end)
 	end)
 
+	describe("keyset cursors on the `new` sort", function()
+		-- `new` orders by (created_at, id), which never moves once a post is
+		-- written, so the database can seek straight to the cursor row instead of
+		-- scanning a windowed fetch. The ranked sorts cannot -- their key is live
+		-- vote counts.
+		local function ids_of(json)
+			local out = {}
+			for _, child in ipairs(json.data.children) do
+				out[#out + 1] = child.data.id
+			end
+			return out
+		end
+
+		it("walks forward exactly, with no repeats or gaps", function()
+			local _, all = get("/api/listing/new?limit=" .. POSTS)
+			local expected = ids_of(all)
+			assert.same(POSTS, #expected)
+
+			local walked, cursor = {}, nil
+			repeat
+				local path = "/api/listing/new?limit=4" .. (cursor and ("&after=" .. cursor) or "")
+				local _, json = get(path)
+				for _, id in ipairs(ids_of(json)) do
+					walked[#walked + 1] = id
+				end
+				cursor = json.data.after
+			until not cursor
+			assert.same(expected, walked)
+		end)
+
+		it("walks backward to the page before a cursor", function()
+			local _, first = get("/api/listing/new?limit=4")
+			local _, second = get("/api/listing/new?limit=4&after=" .. first.data.after)
+			assert.is_truthy(second.data.before)
+
+			local _, back = get("/api/listing/new?limit=4&before=" .. second.data.before)
+			assert.same(ids_of(first), ids_of(back))
+		end)
+
+		-- Capture the filters the endpoint hands the model, which is the only way
+		-- to tell the keyset path from the windowed one -- both are a single
+		-- query, so a query count cannot distinguish them.
+		local function filters_for(path)
+			local captured
+			local real = Posts.get_listing
+			Posts.get_listing = function(self, filters) -- luacheck: ignore 122
+				captured = filters
+				return real(self, filters)
+			end
+			local ok, err = pcall(get, path)
+			Posts.get_listing = real -- luacheck: ignore 122
+			assert.is_true(ok, tostring(err))
+			return captured
+		end
+
+		it("asks the database to seek, rather than fetching a window", function()
+			local _, first = get("/api/listing/new?limit=2")
+
+			local cursored = filters_for("/api/listing/new?limit=2&after=" .. first.data.after)
+			assert.is_truthy(cursored.after_id, "should have passed a keyset cursor")
+			-- Just the page plus one lookahead row, not S.MAX_DEPTH.
+			assert.same(3, cursored.limit)
+
+			-- A ranked sort has no stable key, so it still pages by window.
+			local ranked = filters_for("/api/listing/hot?limit=2&after=" .. first.data.after)
+			assert.is_nil(ranked.after_id)
+			assert.same(S.MAX_DEPTH, ranked.limit)
+		end)
+
+		it("treats a vanished cursor row as the end of the listing", function()
+			local _, json = get("/api/listing/new?limit=4&after=" .. S.fullname("link", 999999))
+			assert.same(0, #json.data.children)
+			assert.is_nil(json.data.after)
+		end)
+
+		it("refuses a keyset cursor on a sort with no stable key", function()
+			-- Guard against a caller wiring after_id into a ranked sort, where the
+			-- key moves between requests and the result would silently be wrong.
+			local ok = pcall(function()
+				return Posts:get_listing({ sort = "hot", after_id = 1 })
+			end)
+			assert.is_false(ok)
+		end)
+	end)
+
 	describe("GET /api/subreddits", function()
 		it("bounds its window and still paginates", function()
 			local status, json = get("/api/subreddits?limit=1")

app/src/api.lua

@@ -77,8 +77,36 @@ local function link_listing(self, filters)
 		filters.exclude_hidden_for = user.id
 	end
 	filters.sort = sort
-	filters.limit = S.window(self.params)
 
+	-- `new` has a stable ordering key, so the database can seek straight to the
+	-- cursor row and return only the page -- no depth cap, nothing scanned. The
+	-- ranked sorts cannot: their key is live vote counts, so they page by
+	-- window (S.window) and `paginate` finds the cursor among those rows.
+	local cursor = self.params.after or self.params.before
+	local cursor_id
+	if cursor then
+		-- `x and f()` would truncate to one value, dropping the id.
+		local _
+		_, cursor_id = S.parse_fullname(cursor)
+	end
+	if Posts.KEYSET_SORTS[sort] and cursor_id then
+		local limit = S.clamp_limit(self.params.limit)
+		filters.limit = limit + 1
+		if self.params.after then
+			filters.after_id = cursor_id
+		else
+			filters.before_id = cursor_id
+		end
+		local rows = Posts:get_listing(filters)
+		local page, after, before = S.paginate_keyset(rows, limit, "link", true)
+		local children = {}
+		for _, p in ipairs(page) do
+			children[#children + 1] = S.link(p)
+		end
+		return { json = S.listing(children, { after = after, before = before }) }
+	end
+
+	filters.limit = S.window(self.params)
 	local rows = Posts:get_listing(filters)
 	local page, after, before = S.paginate(rows, self.params, "link")
 	local children = {}

app/src/models/posts.lua

@@ -124,6 +124,15 @@ local ORDER_BY = {
 		ELSE 0 END) DESC]],
 }
 
+--- Sorts whose ordering key is stable enough to address with a cursor.
+--
+-- Only `new`: its key is `(created_at, id)`, which never changes once a post is
+-- written. Every other sort ranks on live vote counts, so a row's position moves
+-- between requests and a cursor into it could not be exact however it was
+-- implemented -- those page by window instead (see utils/api_serialize).
+Posts.KEYSET_SORTS = { new = true }
+local KEYSET_SORTS = Posts.KEYSET_SORTS
+
 --- Listing rows for a frontpage / subreddit, with the vote and comment
 -- aggregates the templates expect.
 --
@@ -199,14 +208,45 @@ function Posts:get_listing(filters)
 		)
 	end
 
+	-- Keyset cursors, for the sorts whose key is stable (see KEYSET_SORTS).
+	-- Comparing the whole key as a row value -- `(created_at, id) < (...)` --
+	-- expresses "strictly past that row in this order" in one shot, including
+	-- the tiebreaker. A cursor id that no longer exists makes the subquery NULL,
+	-- so the comparison is NULL, so no rows come back: a stale cursor reads as
+	-- "nothing after this", which is what the API wants.
+	local reversed = false
+	if filters.after_id or filters.before_id then
+		assert(
+			KEYSET_SORTS[filters.sort or "new"],
+			"keyset cursors need a stable sort key; '" .. tostring(filters.sort) .. "' has none"
+		)
+		if filters.after_id then
+			restrict(
+				"(a.created_at, a.id) < (SELECT created_at, id FROM posts WHERE id = ?)",
+				tonumber(filters.after_id)
+			)
+		else
+			-- Walking backwards: take the rows *above* the cursor in ascending
+			-- order (so LIMIT keeps the ones nearest it) and flip them back.
+			restrict(
+				"(a.created_at, a.id) > (SELECT created_at, id FROM posts WHERE id = ?)",
+				tonumber(filters.before_id)
+			)
+			reversed = true
+		end
+	end
+
 	-- `a.id DESC` breaks ties deterministically. Without it, equal-ranked rows
 	-- could swap between requests and LIMIT/OFFSET paging would repeat or skip
 	-- them -- the in-memory `table.sort` this replaces was likewise unstable.
 	local order = ORDER_BY[filters.sort] or ORDER_BY.new
+	if reversed then
+		order = "a.created_at ASC"
+	end
 	if filters.sticky_first then
 		order = "a.stickied DESC, " .. order
 	end
-	query = query .. " ORDER BY " .. order .. ", a.id DESC"
+	query = query .. " ORDER BY " .. order .. (reversed and ", a.id ASC" or ", a.id DESC")
 
 	if filters.limit then
 		query = query .. " LIMIT ? OFFSET ?"
@@ -216,6 +256,13 @@ function Posts:get_listing(filters)
 
 	local rows = db.select(query, unpack(params))
 
+	if reversed then
+		-- Hand the caller the same descending order every other path returns.
+		for i = 1, math.floor(#rows / 2) do
+			rows[i], rows[#rows - i + 1] = rows[#rows - i + 1], rows[i]
+		end
+	end
+
 	for _, post in ipairs(rows) do
 		post.permalink = "/r/" .. post.subreddit .. "/comments/" .. post.id
 		-- Prefer the stored host; fall back to parsing for any pre-backfill row.

app/src/utils/api_serialize.lua

@@ -337,6 +337,29 @@ local clamp_limit = M.clamp_limit
 -- and a cursor into them is inherently approximate.
 M.MAX_DEPTH = 1000
 
+--- Build a page and its cursors from an already-windowed keyset result.
+--
+-- The counterpart to `paginate` for callers that let the database do the
+-- seeking: the rows are already exactly the page (plus one lookahead row), so
+-- nothing is scanned to find a cursor.
+-- @tparam table rows up to `limit + 1` rows, in display order
+-- @tparam number limit the page size
+-- @tparam string kind the rows' kind, for fullnames
+-- @tparam boolean has_prev whether a cursor got us here (so a `before` exists)
+-- @treturn table page rows
+-- @treturn string|nil after fullname
+-- @treturn string|nil before fullname
+function M.paginate_keyset(rows, limit, kind, has_prev)
+	local more = #rows > limit
+	local page = {}
+	for i = 1, math.min(#rows, limit) do
+		page[i] = rows[i]
+	end
+	local after = (more and page[#page]) and M.fullname(kind, page[#page].id) or nil
+	local before = (has_prev and page[1]) and M.fullname(kind, page[1].id) or nil
+	return page, after, before
+end
+
 --- How many rows a listing endpoint should fetch for this request.
 --
 -- Without a cursor -- the overwhelmingly common case, and every first page --

docs/sqlite-features.md

@@ -158,12 +158,31 @@ endpoints size their window to the request (`S.window`):
 - **with a cursor**: `S.MAX_DEPTH` (1000) rows, which caps how deep a cursor can
   address.
 
-Capping is honest for this data rather than a shortcut. True keyset pagination
-would need `WHERE (rank, id) < (cursor_rank, cursor_id)`, and for `hot` /
-`controversial` / `rising` the rank is computed from live vote counts — it moves
-between requests, so a cursor into a ranked listing is inherently approximate no
-matter how it is implemented. Search engines and Reddit itself cap deep paging
-for the same reason.
+**`new` skips the cap entirely.** Its key, `(created_at, id)`, never moves once a
+post is written, so the database can seek straight to the cursor row with a
+row-value comparison and return only the page:
+
+```sql
+WHERE (a.created_at, a.id) < (SELECT created_at, id FROM posts WHERE id = ?)
+```
+
+Comparing the whole key as a row value expresses "strictly past that row in this
+order" in one shot, tiebreaker included. A cursor id that no longer exists makes
+the subquery NULL, so the comparison is NULL and no rows come back — a stale
+cursor reads as "nothing after this", which is exactly the wanted answer.
+Walking *backwards* runs the comparison the other way in ascending order and
+flips the rows, so the caller always sees one order.
+
+The **ranked** sorts keep the window-and-cap treatment, and that is a property
+of the data rather than a shortcut: `hot`, `controversial` and `rising` compute
+rank from live vote counts, so a row's position moves between requests and a
+cursor into one is approximate however it is implemented. Keyset would relocate
+the inaccuracy, not remove it. Search engines and Reddit cap deep paging for the
+same reason.
+
+`Posts.KEYSET_SORTS` is the list, and `get_listing` **asserts** when a cursor is
+passed with a sort that is not on it — an unstable-key cursor would fail
+silently and subtly otherwise.
 
 A cursor that is not in the window (past the cap, or a row since deleted) now
 returns an **empty page**. It used to silently restart at the top, which left a