Skip to content

Commit 40e7bfc

Browse files
fix(metadata-protocol): an offset with no limit reports the filtered total (#28611)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1X2pz77pK1zfwZ6XMswiajh2)_ Fixes #28547 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么:** 列表读取只给偏移(`$skip` / `offset`)不给条数时,`total` 现在是符合筛选条件的总行数,不再是“总行数减偏移”。 - **为什么改:** 旧逻辑只在有 limit 时才做计数,没 limit 时直接拿返回行数当总数;有偏移时就少算了偏移那么多行(14 行跳 3 行报 11,偏移超出末尾报 0)。 - **风险与回滚:** 低。只改 `findData` 分页块的一个条件:有偏移无条数的请求走已有的计数路径(多一次 COUNT 查询)。无偏移无条数、有条数、`$count=false` 三种行为不变。回滚即 revert 本 PR。 - **先看:** `packages/metadata-protocol/src/protocol.ts` 中 `if (pageLimit !== undefined || pageOffset > 0)` 一行及 `$count=false` 分支的 `hasMore`;`protocol.offset-no-limit-total.test.ts`。 - **两分钟验证:** `pnpm --filter @objectstack/metadata-protocol exec vitest run src/protocol.offset-no-limit-total.test.ts`,预期 11 个用例全绿。 - **需要你做的:** 无,席位落地。 ## What changes `findData` (`packages/metadata-protocol/src/protocol.ts`) treated a list read as paged only when it carried a limit. Without one it kept `total = records.length`, on the comment "without a limit the full result set is returned". With an offset that is false: the read returns every match after the offset, so `total` short-counted by exactly the offset. The paged arm now opens on `pageLimit !== undefined || pageOffset > 0`. An offset with no limit runs the same count a paged request runs, over the same `where`, `search` and `searchFields` (#22790). Inside that arm: - `$count` absent or true: one `engine.count()`, and `total` is the filtered count. `hasMore` is true only while the offset plus the rows returned falls short of the count, so it is false when every row after the offset came back. - `$count=false`: no count is issued and `total` is omitted (#10757). `hasMore` now reads `pageLimit !== undefined && records.length === pageLimit`, so with no limit it is false (every row after the offset came back). Before, the guard was not needed because this arm was reached only with a limit. Adding the offset back to `records.length` was measured and rejected: an offset past the end returns no rows, so it would answer `total: 999` for `{"offset":999}` over 14 rows. The count path gives 14. A read with neither an offset nor a limit is unchanged: no count, `total` is the set length. An explicit `offset: 0` is no offset (`pageOffset` is computed as `> 0`). ## Why The card's expected result: "`total` is the number of rows matching the filter, whatever the offset, as it already is when a limit is present (and as #22690 / #22588 settled for `$top=0`)." The Console always sends `$top`, so API clients paging with `$skip` alone were the ones reading the wrong number. ## Tests New suite `packages/metadata-protocol/src/protocol.offset-no-limit-total.test.ts`, on an engine double that honours `offset` and `limit` as written and counts over the same `where` (the shape of `protocol.zero-limit-total.test.ts`), with 14 rows like the showcase's `showcase_account`: - `GET ?$skip=3&$count=true`: 11 rows, `total` 14, `hasMore` false, one count. - `POST {"offset":3,"count":true}`: the same. - `{"offset":999,"count":true}`: no rows, `total` 14. - The same offset with and without `$top=100` gives the same `total` and rows. - Absent `$count` with `skip=3`: `total` 14. - CONTROL: a filter (`tier=b`, 4 rows) with `$skip=1` gives 3 rows and `total` 4, and the count carries the `where`. A `$search` is passed to both `find` and `count`. - CONTROL, unchanged arms: no offset and no limit issues no count; `offset: 0` issues no count; `offset:3, limit:5` issues one count, `total` 14, `hasMore` true; `$count=false` with an offset (both the GET and the POST spelling) issues no count, omits `total`, returns 11 rows, `hasMore` false. <details><summary>Red first: the suite against the base protocol.ts (310cb96)</summary> The base blob (`81ff63a0`) was written over the file and the suite run, then restored with `git checkout HEAD -- …` (blob back to `5dac1154`, `git diff HEAD` empty). ``` Tests 8 failed | 3 passed (11) AssertionError: expected 11 to be 14 (GET ?$skip=3&$count=true) AssertionError: expected 11 to be 14 (POST {"offset":3,"count":true}) AssertionError: expected +0 to be 14 ({"offset":999,"count":true}) AssertionError: expected 3 to be 4 (tier=b, $skip=1) expected "vi.fn()" to be called 1 times, but got 0 times ($search) {"$skip":"3","$count":"false"}: expected true to be false ($count=false still reported total) ``` The 3 that stay green are the unchanged-arm controls (no offset and no limit, `offset: 0`, a limit). </details> <details><summary>Ablation: revert only the arm condition on the committed fix</summary> `node scripts/ablation-replace.mjs --file packages/metadata-protocol/src/protocol.ts --anchor 'if (pageLimit !== undefined || pageOffset > 0) {' --replacement 'if (pageLimit !== undefined) {'`. The test imports `./protocol.js` from `src/`, so no build is involved. ``` ablation-replace: ok mutation landed: anchor 1 -> 0, blob 5dac115 -> 161f280e49ec anchor-new count: 0 anchor-old count: 1 Tests 8 failed | 3 passed (11) ablation-replace: ok restored: blob == HEAD (5dac115) and `git diff HEAD` is empty ``` </details> <details><summary>Package suite, typecheck, lint</summary> - `pnpm --filter @objectstack/metadata-protocol build && … typecheck && … test` under the verify lock: `VERDICT command-exit 0`, `Test Files 290 passed | 4 skipped (294)`, `Tests 21905 passed | 26 skipped (21931)`. The package `tsconfig.json` includes `src/**/*`, so typecheck covers the new test file. - Neighbouring suites `protocol.zero-limit-total.test.ts` and `protocol.count-opt-out.test.ts` stay green (27 of 27 with the new suite). The count-opt-out pin "`$count=false` without a limit is a no-op" has no offset and is unchanged. - `eslint packages/metadata-protocol --no-inline-config`: exit 0. </details> ## Acceptance notes Other producers of a list `total`, each with a conclusion: - **RestServer `GET /data/:object` and `POST /data/:object/query`** (`packages/rest/src/crud-routes.ts:164`, `:379`): both call `p.findData`. Fixed here. - **Dispatcher `GET /data/:object` and `POST /data/:object/query`** (`runtime/src/domains/data.ts` → `action-execution.ts` `callData('query')` → `protocol.findData`, `:779`): fixed here. - **`findData`'s aggregate branch** (`groupBy` / `aggregations`): no offset reaches `engine.aggregate`, and `total` is the length of the full grouped set. Not affected. - **MCP HTTP `query_records`** (`packages/mcp/src/mcp-http-tools.ts:1127`, via `runtime/src/domains/mcp.ts:711` `callData('query')`): always sends a limit (`Math.min(limit ?? maxLimit, maxLimit)`), so it already took the count arm. Already correct, and it now shares the fix if that ever changes. - **Out of scope, observed only (not filed):** `packages/mcp/src/stdio-data-bridge.ts:971` (stdio MCP `query`) and the no-protocol fallback in `runtime/src/action-execution.ts` (`total: all.length`, reached only when the metadata-protocol service is not registered) report `records.length` as `total` whatever the offset or limit. That is a different defect (page size reported as total even with a limit), read from source and not measured at a public door, so it is not filed. Carrier: none. The fix costs one `COUNT` per offset-only read, the same cost a limited read already pays. A caller that wants to avoid it sends `$count=false`. ## Verification Derived gate list: `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` at `0cd24491` (61 commands, after merging `origin/main` once at `cd065d36` because #28583 touched `protocol.ts`), each run with its exit code captured before any pipe, reconciled with `--ran`. - `--ran` reconciliation: `61 derived, 61 run, 0 NOT-MEASURED, 0 UNRUN` (`RAN_EXIT=0`), every family exit 0, including `check:type-check-debt` ("0 raw tsc error(s) total, none above its recorded number"), `check:dual-build-cjs-loads`, `check:lean-entry-closure`, `check:engine-double-contract`, `check:query-options-erasure`, `check:objectql-double-limit` and `check:where-matcher`. - Package suite re-run on the merged head: `Test Files 291 passed | 4 skipped (295)`, `Tests 21908 passed | 26 skipped (21934)`, typecheck exit 0, `eslint packages/metadata-protocol --no-inline-config` exit 0. - The first pass on `d5bcd30c` (before the merge) also accounted for all 61; three families answered exit 3 (prerequisite not met in a fresh worktree) and were green after the prerequisite was met. CI runs the full farm (Test Core shards, lint, type-check lanes) on this PR. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (No spec change.) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes (scoped to the touched package locally; CI runs the full lint). - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset (`.changeset/28547-offset-no-limit-total.md`, `@objectstack/metadata-protocol` patch), and both declaration lines are filled in here and in the changeset. - [x] 6. `packages/spec` touched: not touched. - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English (the 维护者速读 block is the fleet's required Chinese summary). - [x] The *Before Submitting* list in CONTRIBUTING.md is done. Report on the card: the `os-dev-report` comment from `domain:server#3 · os-project-manager · session_01FEkQ5pNK9fm2KtGj8ijVqC`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FEkQ5pNK9fm2KtGj8ijVqC --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5pNK9fm2KtGj8ijVqC)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent de3fafc commit 40e7bfc

3 files changed

Lines changed: 219 additions & 3 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@objectstack/metadata-protocol': patch
3+
---
4+
5+
fix(metadata-protocol): an offset with no limit reports the filtered total
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
A Data API list read that skipped rows without naming a page size reported a wrong `total`: the number of matching rows minus the offset. `GET /api/v1/data/OBJECT?$skip=3&$count=true` over 14 rows answered `total: 11`, while the same query with `$top=100` answered 14, and `POST /api/v1/data/OBJECT/query` with `{"offset":999,"count":true}` answered `total: 0`. `total` is now the number of rows matching the filter whatever the offset, as it already was when a limit was present.
11+
12+
Such a request now runs the same count a paged request runs, over the same filter and search. `$count=false` with an offset issues no count and omits `total`, as it does with a limit. A read with neither an offset nor a limit is unchanged and issues no count.
13+
14+
What to do: nothing. A client that pages with `$skip` (or `offset`) alone now reads the true total.
Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #28547 — a list's `total` is the filtered count whatever the offset, also
5+
* when no limit is given.
6+
*
7+
* ## What was wrong
8+
*
9+
* `findData` decided "is this a paged request?" from the limit alone. Without
10+
* one it kept `total = records.length` on the reasoning that "the full result
11+
* set is returned". With an offset that reasoning is false: the read returns
12+
* every match AFTER the offset, so the total short-counted by exactly the
13+
* offset. Measured on the stock showcase (`showcase_account`, 14 rows):
14+
*
15+
* GET ?$skip=3&$count=true 11 rows, total 11 ← the defect
16+
* GET ?$skip=3&$top=100&$count=true 11 rows, total 14
17+
* POST {"offset":3,"count":true} total 11 ← the defect
18+
* POST {"offset":999,"count":true} total 0 ← the defect
19+
*
20+
* ## What this suite pins
21+
*
22+
* 1. An offset with no limit reports the filtered count, on both the GET
23+
* dialect (`$skip`) and the POST body form (`offset`), and an offset past
24+
* the end still reports it (so "add the offset back" is not a fix: it
25+
* would answer 999 there).
26+
* 2. CONTROL — a filter and a search move the count like any other paged
27+
* request (#22790: the COUNT carries what the rows were read with).
28+
* 3. The neighbouring arms keep their meaning: no offset and no limit issues
29+
* no COUNT; a limit is unchanged; `$count=false` with an offset issues no
30+
* COUNT and omits `total` (#10757).
31+
*/
32+
33+
import { describe, it, expect, vi } from 'vitest';
34+
import { ObjectStackProtocolImplementation } from './protocol.js';
35+
36+
const SCHEMA = {
37+
name: 'mz_account',
38+
nameField: 'name',
39+
fields: {
40+
name: { name: 'name', type: 'text' },
41+
tier: { name: 'tier', type: 'text' },
42+
},
43+
};
44+
45+
// 14 rows, like the showcase's seeded `showcase_account`; 4 of them in tier `b`.
46+
const ROWS = Array.from({ length: 14 }, (_, i) => ({
47+
id: `a${i}`,
48+
name: `a${i}`,
49+
tier: i % 4 === 3 || i === 0 ? 'b' : 'a',
50+
}));
51+
const TIER_B = ROWS.filter((r) => r.tier === 'b').length;
52+
53+
/**
54+
* Equality-only `where`, the shape the implicit field filter `?tier=b` lowers
55+
* to. Anything else is REFUSED rather than answered silently wrong.
56+
*/
57+
function matches(row: Record<string, unknown>, where: unknown): boolean {
58+
if (where == null) return true;
59+
if (typeof where !== 'object' || Array.isArray(where)) {
60+
throw new Error(`test double: unexpected where shape ${JSON.stringify(where)}`);
61+
}
62+
return Object.entries(where as Record<string, unknown>).every(([k, v]) => {
63+
if (k.startsWith('$')) throw new Error(`test double: combinator '${k}' is not implemented`);
64+
if (v !== null && typeof v === 'object') throw new Error(`test double: operator on '${k}' is not implemented`);
65+
return row[k] === v;
66+
});
67+
}
68+
69+
/**
70+
* An engine double that honours `offset` and `limit` as written, and counts
71+
* over the same `where`.
72+
*/
73+
function makeProtocol() {
74+
const find = vi.fn(async (_object: string, opts: any = {}) => {
75+
const hits = ROWS.filter((r) => matches(r, opts.where));
76+
const offset = typeof opts.offset === 'number' ? opts.offset : 0;
77+
const sliced = hits.slice(offset);
78+
return opts.limit !== undefined ? sliced.slice(0, opts.limit) : sliced;
79+
});
80+
const count = vi.fn(async (_object: string, opts: any = {}) => ROWS.filter((r) => matches(r, opts.where)).length);
81+
const engine = {
82+
registry: { getObject: (n: string) => (n === SCHEMA.name ? SCHEMA : undefined) },
83+
find,
84+
count,
85+
aggregate: vi.fn(async () => [] as unknown[]),
86+
};
87+
return { p: new ObjectStackProtocolImplementation(engine as any), find, count };
88+
}
89+
90+
async function list(query: Record<string, unknown>) {
91+
const h = makeProtocol();
92+
const result: any = await h.p.findData({ object: SCHEMA.name, query } as never);
93+
return { result, ...h };
94+
}
95+
96+
describe('[#28547] an offset with no limit reports the filtered total', () => {
97+
it('GET ?$skip=3&$count=true answers the match count, not the count after the offset', async () => {
98+
const { result, count } = await list({ $skip: '3', $count: 'true' });
99+
100+
expect(result.records).toHaveLength(11);
101+
expect(result.total).toBe(14);
102+
expect(result.hasMore).toBe(false);
103+
expect(count).toHaveBeenCalledTimes(1);
104+
});
105+
106+
it('POST {"offset":3,"count":true} answers the same', async () => {
107+
const { result } = await list({ offset: 3, count: true });
108+
109+
expect(result.records).toHaveLength(11);
110+
expect(result.total).toBe(14);
111+
expect(result.hasMore).toBe(false);
112+
});
113+
114+
it('an offset past the end returns no rows and still reports the match count', async () => {
115+
const { result } = await list({ offset: 999, count: true });
116+
117+
expect(result.records).toEqual([]);
118+
expect(result.total).toBe(14);
119+
expect(result.hasMore).toBe(false);
120+
});
121+
122+
it('agrees with the same offset under a limit', async () => {
123+
const withoutLimit = await list({ $skip: '3', $count: 'true' });
124+
const withLimit = await list({ $skip: '3', $top: '100', $count: 'true' });
125+
126+
expect(withoutLimit.result.total).toBe(withLimit.result.total);
127+
expect(withoutLimit.result.records).toEqual(withLimit.result.records);
128+
});
129+
130+
it('an absent $count reports the same total as $count=true', async () => {
131+
const { result } = await list({ skip: '3' });
132+
133+
expect(result.total).toBe(14);
134+
});
135+
});
136+
137+
describe('[#28547] CONTROL — the count is the filtered count', () => {
138+
it('a filter moves the total, and the rows are read past the offset', async () => {
139+
const { result, count } = await list({ $skip: '1', tier: 'b' });
140+
141+
expect(result.records).toHaveLength(TIER_B - 1);
142+
expect(result.total).toBe(TIER_B);
143+
expect((count.mock.calls[0] as unknown[])[1]).toMatchObject({ where: { tier: 'b' } });
144+
});
145+
146+
it('a search is counted with the search the rows were read with (#22790)', async () => {
147+
const { count, find } = await list({ $skip: '2', $search: 'a1' });
148+
149+
expect(count).toHaveBeenCalledTimes(1);
150+
expect((count.mock.calls[0] as unknown[])[1]).toMatchObject({ search: 'a1' });
151+
expect((find.mock.calls[0] as unknown[])[1]).toMatchObject({ search: 'a1' });
152+
});
153+
});
154+
155+
describe('[#28547] the neighbouring arms keep their meaning', () => {
156+
it('no offset and no limit: no COUNT, total is the set length', async () => {
157+
const { result, count } = await list({ $count: 'true' });
158+
159+
expect(count).not.toHaveBeenCalled();
160+
expect(result.total).toBe(14);
161+
expect(result.hasMore).toBe(false);
162+
});
163+
164+
it('an explicit offset of 0 is no offset: no COUNT', async () => {
165+
const { result, count } = await list({ offset: 0, count: true });
166+
167+
expect(count).not.toHaveBeenCalled();
168+
expect(result.total).toBe(14);
169+
});
170+
171+
it('a limit is unchanged: one COUNT, total is the match count, hasMore from the page', async () => {
172+
const { result, count } = await list({ offset: 3, limit: 5, count: true });
173+
174+
expect(count).toHaveBeenCalledTimes(1);
175+
expect(result.records).toHaveLength(5);
176+
expect(result.total).toBe(14);
177+
expect(result.hasMore).toBe(true);
178+
});
179+
180+
it('$count=false with an offset issues no COUNT and omits total (#10757)', async () => {
181+
for (const query of [{ $skip: '3', $count: 'false' }, { offset: 3, count: false }]) {
182+
const { result, count } = await list(query);
183+
184+
expect(count, JSON.stringify(query)).not.toHaveBeenCalled();
185+
expect('total' in result, JSON.stringify(query)).toBe(false);
186+
expect(result.records, JSON.stringify(query)).toHaveLength(11);
187+
expect(result.hasMore, JSON.stringify(query)).toBe(false);
188+
}
189+
});
190+
});

‎packages/metadata-protocol/src/protocol.ts‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14064,7 +14064,17 @@ export class ObjectStackProtocolImplementation implements
1406414064
// page, so `records.length` is the page size — NOT the match total. Run a
1406514065
// count over the same `where` so the client can render total pages and know
1406614066
// whether more pages remain (true server-side pagination). Without a limit
14067-
// the full result set is returned, so its length already IS the total.
14067+
// AND without an offset the full result set is returned, so its length
14068+
// already IS the total.
14069+
//
14070+
// [#28547] An offset with no limit is a paged request too. The read
14071+
// returns every match AFTER the offset, so `records.length` is the
14072+
// match count MINUS the offset: `?$skip=3&$count=true` over 14 rows
14073+
// answered `total: 11` while `?$skip=3&$top=100&$count=true` answered
14074+
// 14, and an offset past the end answered `total: 0`. Adding the offset
14075+
// back is not the fix (it says 999 for `offset: 999` over 14 rows), so
14076+
// such a request takes the one count path below, `$count=false` arm
14077+
// included: `total` is the filtered count whatever the offset.
1406814078
//
1406914079
// [#22790] The count takes the query's `search` / `searchFields` too: the
1407014080
// engine expands them through the one ADR-0061 expander `find` uses, so
@@ -14087,7 +14097,7 @@ export class ObjectStackProtocolImplementation implements
1408714097
const pageOffset = typeof options.offset === 'number' && options.offset > 0 ? options.offset : 0;
1408814098
let total: number | undefined = records.length;
1408914099
let hasMore = false;
14090-
if (pageLimit !== undefined) {
14100+
if (pageLimit !== undefined || pageOffset > 0) {
1409114101
// `distinct` used to suppress the count here too — #4286 finding 2:
1409214102
// the flag's ONLY observable effect platform-wide, on a capability
1409314103
// that never deduplicated a row. Removed with `query.distinct`
@@ -14110,7 +14120,9 @@ export class ObjectStackProtocolImplementation implements
1411014120
// `hasMore` is still answered, from the page alone: a FULL page
1411114121
// means there may be more. It never over-reports the data — it
1411214122
// can only say "maybe more" on an exactly-full last page.
14113-
hasMore = records.length === pageLimit;
14123+
// With an offset and no limit, every row after the offset came
14124+
// back, so there is no further page.
14125+
hasMore = pageLimit !== undefined && records.length === pageLimit;
1411414126
total = undefined;
1411514127
} else {
1411614128
// [#10757] `counted` is a separate, always-assigned local so

0 commit comments

Comments
 (0)