Skip to content

fix(metadata-protocol): an offset with no limit reports the filtered total - #28611

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/project-thread-7ter86
Nov 1, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/project-thread-7ter86

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Leehom · project thread

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:

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.
Red first: the suite against the base protocol.ts (310cb96)

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).

Ablation: revert only the arm condition on the committed fix

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 5dac11542031 -> 161f280e49ec
anchor-new count: 0
anchor-old count: 1
Tests  8 failed | 3 passed (11)
ablation-replace: ok restored: blob == HEAD (5dac11542031) and `git diff HEAD` is empty
Package suite, typecheck, lint
  • 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.

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

  • 1. Schemas start as Zod; packages/spec holds no runtime logic. (No spec change.)
  • 2. Every name I added uses the casing for its kind.
  • 3. No formatter ran; pnpm lint passes (scoped to the touched package locally; CI runs the full lint).
  • 4. One issue, one branch off an up-to-date main, one pull request.
  • 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.
  • 6. packages/spec touched: not touched.
  • 7. No hand edits to content/docs/references/, packages/*/CHANGELOG.md or content/docs/releases/.
  • 8. Title, body and comments are in English (the 维护者速读 block is the fleet's required Chinese summary).
  • 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.ai/code/session_01FEkQ5pNK9fm2KtGj8ijVqC


Generated by Claude Code

…total

`findData` treated a request as paged only when it carried a limit. Without
one it kept `total = records.length`, which is the match count minus the
offset once an offset is given (`?$skip=3&$count=true` over 14 rows answered
`total: 11`, and an offset past the end answered `total: 0`). An offset with
no limit now takes the same count path as a paged request, `$count=false`
arm included.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FEkQ5pNK9fm2KtGj8ijVqC
@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol, touching 1 documentable anchor(s).

3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/data-modeling/queries.mdx (via findData (symbol, a method of class ObjectStackProtocolImplementation))
  • content/docs/kernel/services-checklist.mdx (via findData (symbol, a method of class ObjectStackProtocolImplementation))
  • content/docs/protocol/objectql/query-syntax.mdx (via findData (symbol, a method of class ObjectStackProtocolImplementation))

⛔ 1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v15.mdx (via findData (symbol, a method of class ObjectStackProtocolImplementation))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 cross-cutting symbol(s) contributed no route anchor: findData (4 routes)
  • the SDK route bridge reached 60 of 213 client-bound route-ledger rows — the other 153 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 153: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 98 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 10 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json d94dd3a65d8ebdf2f5c7084591efaafb1a1f7fdb → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 4ce493f53bc92309db7285697bde87f5d03626b5 — the merge of head 0cd24491da48c6ef5aa53f4e67f8bfbed3631e92 into base d94dd3a65d8ebdf2f5c7084591efaafb1a1f7fdb, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 4ce493f53bc92309db7285697bde87f5d03626b5 && git checkout 4ce493f53bc92309db7285697bde87f5d03626b5
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin d94dd3a65d8ebdf2f5c7084591efaafb1a1f7fdb 0cd24491da48c6ef5aa53f4e67f8bfbed3631e92 && git checkout -B drift-repro d94dd3a65d8ebdf2f5c7084591efaafb1a1f7fdb && git merge --no-ff 0cd24491da48c6ef5aa53f4e67f8bfbed3631e92

node scripts/docs-audit/affected-docs.mjs --json d94dd3a65d8ebdf2f5c7084591efaafb1a1f7fdb

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs d94dd3a65d8ebdf2f5c7084591efaafb1a1f7fdb → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#3 · os-project-manager · session_01FEkQ5kE5BrPUPNMMyv6oCH

席位核对 · ACCEPT · PR #28611 · 2026-11-01T08:17Z

Head-sha: 0cd24491da48c6ef5aa53f4e67f8bfbed3631e92

维护者速读(席位终稿)

  • 改了什么:列表读取只给偏移($skip / offset)不给条数时,total 现在是符合筛选条件的总行数,不再少算偏移那么多行。
  • 为什么改:旧逻辑只在有条数时计数;14 行跳 3 行报 11,偏移超出末尾报 0(Data API: with $count=true and $skip (or offset) but no $top/limit, total is the match count minus the offset #28547)。只用 $skip 翻页的 API 客户端读到的是错数。
  • 风险与回滚:低。只改 findData 分页块的开门条件,有偏移无条数的请求多一次 COUNT;无偏移无条数、有条数、$count=false 三种行为不变。回滚即 revert。
  • 先看:protocol.ts 的 if (pageLimit !== undefined || pageOffset > 0) 与 $count=false 分支的 hasMore。
  • 两分钟验证:pnpm --filter @objectstack/metadata-protocol exec vitest run src/protocol.offset-no-limit-total.test.ts → 11/11。
  • 需要你做的:无。席位按常设授权落地。
  • 席位意见:修在单一读路径,红先与消融都钉住这一行;可落。

核对六项

  1. 形态:draft → main;Fixes #28547,本 PR 交付卡的全部期望,合并即关卡正确;其余闭合关键词零命中(已扫)。文件 3 个(protocol.ts、新测试、changeset),在认领申报面内。
  2. 前提实测:红先在 base 310cb96d 上 8/11 红(GET/POST 偏移 total 11≠14、越界 0≠14、带筛选 3≠4、$search 未进 count);消融只回退开门条件 → 同 8 红,复原后 blob 回 HEAD。
  3. 门禁:本 head check-runs 48 个,35 success、13 skipped、0 失败。dev 本地 61 族全测通过,期间合过一次 main(cd065d36)。
  4. 接受的偏差:无。dev 量过「records.length + offset」并否决(越界偏移会答 999),采用计数路径,合理。
  5. 范围外去向:stdio MCP query 与 runtime 兜底路径仍以 records.length 作 total,不在本卡读路径上,记为 Acceptance notes,不另立卡(同一形状的后续卡出现时引用本条)。Metadata-protocol: none,无 Skills-docs: 欠账。
  6. 落地条款:Clause-②: no 且未触 spec 路径、非收窄 ⇒ 不欠契约复核。check-governed-merges --pr 28611:NOT governed,222 行。落地授权,维护者原话:「按照 pm-dispatch 规则复核过的都授权」(2026-10-25T11:35Z)。

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review November 1, 2026 09:19
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Nov 1, 2026
Merged via the queue into main with commit 40e7bfc Nov 1, 2026
49 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-7ter86 branch November 1, 2026 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Data API: with $count=true and $skip (or offset) but no $top/limit, total is the match count minus the offset

1 participant