Skip to content

fix(service-analytics): the ObjectQL face applies a query's order, offset and limit, as its echoed sql says - #21363

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-21316-objectql-face-order-limit
Oct 2, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-21316-objectql-face-order-limit

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #21316
Clause-②: no

What this changes

The ObjectQL face applies order, then offset and limit, to the aggregated answer, as its echoed sql and POST /api/v1/analytics/sql say it does (packages/services/service-analytics/src/strategies/objectql-strategy.ts, orderAndWindow).

  • engine.aggregate declares no ordering or window grammar (EngineAggregateOptionsSchema, packages/spec/src/data/data-engine.zod.ts), and neither does the executeAggregate contract (StrategyContext.executeAggregate, packages/spec/src/contracts/analytics-service.ts). So there is nothing to push down, and the strategy applies the three keys itself. It does this on the direct path and on the cross-object (FK-expand) path, after the re-bucket. Before the re-bucket, a limit would cut FK groups that merge into a group it keeps.
  • It runs on the remapped rows, which are keyed by the member spellings the caller selected. So an order key names a column exactly as the analytics door's order-key rule (order-key-door.ts, analytics: on the native-SQL strategy an order key that is not a selected member answers 500 (ambiguous, must appear in GROUP BY, or no such column); the ObjectQL face answers 200 for the same queries #21267) admitted it. order is read verbatim, in its own key order, and no implicit ordering is added. A bare limit slices the engine's order, the same as LIMIT without ORDER BY.
  • One ordering rule. The comparison is applyOrdering and the window is applyWindow. Both come from dataset-executor.ts, the package's one row comparator and window, which the dataset door's post-pass already applies to every grid. Nothing is copied. Other comparators exist in or near the package, and none was reused: preview-evaluator.ts has its own compare for draft rows, and driver-memory's applySort is private.

The dataset door no longer applies a pushed-down offset twice (dataset-executor.ts). For a single-query selection, DatasetExecutor pushes order, limit and offset down to the strategy and then windowed the answer again. The second slice applies offset a second time. This is a shipped defect on the native face, and this change would have extended it to the ObjectQL face, which was right before only because it dropped the window. The post-pass now windows only a grid it could not push down. It still re-sorts every grid.

Measured before / after

The measurement went through the real POST /api/v1/analytics/query and /analytics/sql route (createDispatcherPlugin), using AnalyticsServicePlugin with its default composition and with capabilities narrowed to the engine aggregate, a real ObjectQL engine and SqlDriver. It ran on SQLite (better-sqlite3) and PostgreSQL 16.14 (C.UTF-8 collation).

query (ObjectQL face) main 97239c3, SQLite main, PostgreSQL this branch, both
timeDimensions [closed_on, month], order { closed_on: desc }, limit 1 every month, ascending every month, 04, 03, 05 the newest month alone
dimensions [note], order { note: desc } unordered unordered the native face's rows
order { amount_sum: desc }, limit 2, offset 1 every group every group the native face's rows

On both drivers the echoed sql rendered … ORDER BY "closed_on" DESC LIMIT 1 (and the others), before and after.

Dataset door, limit 2, offset 1, ordered by a measure over five groups. Before, the native face answered one row (the third) on both drivers, and the ObjectQL face answered the second and third. After, both faces answer the second and third.

Pins (src/__tests__/objectql-face-order-limit.test.ts, SQLite always, PostgreSQL where OS_TEST_POSTGRES_URL is set)

Ablations (committed tree, each leg through scripts/ablation-replace.mjs with the anchor proven to land and the restore proven by blob hash plus an empty git diff HEAD; the pins import the strategy from src/, so no dist/ is involved)

Recorded at c6c4ae9d0. The later merge of main (a3179775d) leaves packages/services/service-analytics/ byte-identical: git diff c6c4ae9d0 a3179775d over that path is empty. The pin file has 26 cases, 13 per driver.

  • Ordering removed (applyWindow(rows, …)): 17 red, 9 green. Every order pin is red on SQLite. The 9 green are the controls, limit 0, and windows that the arrival order happens to satisfy on one driver.
  • Window removed (applyOrdering(rows, query.order)): 18 red. Every limit/offset pin is red on both drivers. The order-only pins and the controls stay green.
  • Ordering inverted (.reverse()): 20 red. Every native-equality pin and both controls are red on both drivers. The dataset-door pins stay green, because the executor re-sorts a middle window that happens to be symmetric. The next leg owns those pins.
  • Executor re-windows a pushed-down page: exactly the 4 dataset-door pins are red (2 per driver), and nothing else.
  • The first run of that last leg was a no-op that the tool refused (its replacement text was a substring of the anchor, so the count could not rise). It was re-run with a distinct replacement. The first runs of the other three legs, on an earlier fixture, showed that SQLite's ascending group order satisfied two order pins. The fixture was changed so that no order pin matches that arrival order, and all four legs were re-run.

Acceptance notes

  • Comparator semantics against the native face. Both faces answer the same rows for numbers and for text of single-case ASCII letters. They can differ on NULL, '', numeric text and mixed case. This was measured on this branch at the route:
    • The native ORDER BY places NULL lowest on SQLite and highest on PostgreSQL, and orders text by the column collation (codepoint on SQLite, and on PostgreSQL under C.UTF-8).
    • applyOrdering keeps NULL and '' last in both directions, compares numeric text numerically, and compares other text with localeCompare.
    • This is the dataset door's existing, documented rule ("Null / empty buckets sort last in both directions", content/docs/data-modeling/analytics.mdx). It is reused, not changed. The open question is in the report.
  • limit / offset outside the non-negative integers. AnalyticsQuerySchema declares both as z.number(). Measured on this branch:
    • limit: -1: native SQLite returns every row, native PostgreSQL answers 500. The ObjectQL face now answers applyWindow's slice (all but the last row), as the dataset door already does for the same value. Before, it answered every row.
    • limit: 1.5: native SQLite answers 500, PostgreSQL rounds to 2 rows, the ObjectQL face answers 1 row.
    • offset: -1: the native face answers 500 on both drivers.
    • No answer agrees across drivers. These are in the report as a finding; this PR leaves them as they are.
  • offset without limit on SQLite. The native face renders OFFSET n with no LIMIT, which SQLite refuses (near "OFFSET": syntax error, 500). The ObjectQL face answers it, and its echo renders the same statement. This is in the report as a finding.
  • Docs: no page in content/docs/** (outside releases/) or skills/** says the ObjectQL face ignores order/limit, so no doc sentence becomes false. Code comments that said so, in the dataset-executor.ts header and post-pass and in the order-key-door.ts header, are corrected.

Verification (head a3179775d, after merging main)

  • pnpm --filter @objectstack/service-analytics test with live PostgreSQL 16.14 (OS_TEST_POSTGRES_URL, so no cell skipped): 167 files, 3869 tests passed. Its dependency closure was rebuilt first.
  • pnpm --filter @objectstack/service-analytics typecheck: exit 0. Both touched test files are in its program (tsc --listFiles).
  • The derived gate union (node scripts/pm/dispatch-gates.mjs --commands, merge base 96b12b589) has 63 commands. All 63 exited 0, and --ran reconciles 63 derived, 63 run, 0 NOT-MEASURED. On the earlier head, pnpm check:dual-build-cjs-loads first answered PREREQUISITE NOT MET (no dist/). It was run again after a full turbo run build and answered 0, and it answered 0 again on this head.
  • Lint, narrowed to the 5 touched .ts files: eslint --no-inline-config --format json reads 5 files with 0 errors and 0 warnings. eslint.config.mjs enables no type-aware linting (no parserOptions.project), so this diff cannot change the verdict on any untouched file. The repo-wide pnpm lint is left to CI.

Generated by Claude Code

claude added 4 commits October 2, 2026 06:20
…fset and limit

engine.aggregate has no ordering or window grammar, so the ObjectQL face
answered every group in the engine's arrival order while its echoed sql and
/analytics/sql rendered ORDER BY ... LIMIT ... OFFSET. Every date-bucketed
query lands on this face, so a "top N" or ordered trend answered unordered and
unlimited.

The face now orders the remapped answer by the query's order (the package's
one row comparator, applyOrdering) and then applies offset and limit
(applyWindow), on the base path and the cross-object path.

The dataset door pushes a single query's window down and then re-windowed the
answer, applying offset a second time; it now re-sorts only when the window was
pushed down.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
…mit on SQLite and PostgreSQL

The card's rows on both drivers: a bucketed time dimension ordered and
limited, a selected dimension and a selected measure equal to the native
face, two order keys, limit 0, the cross-object path, the echoed sql run on
the same driver, the no-window control, and the dataset door's pushed-down
page on both faces. The #21267 controls now assert the ObjectQL face's order.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
…ing order does not already give

SQLite answers a grouping in ascending key order, so a pin whose expected
order matched it stayed green with the face's ordering removed. The fixture's
amounts now make every ordered pin, and every window, differ from that order.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added the size/m label Oct 2, 2026
@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Oct 2, 2026
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-analytics, touching 4 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/services/service-analytics/src/order-key-door.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

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

  • content/docs/releases/v17/17-0.mdx (via ObjectQLStrategy (symbol, a top-level class))

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 changed file(s) yielded no anchor (packages/services/service-analytics/src/order-key-door.ts) — pages documenting those are invisible to this run
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 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; 97 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 16eefc6c0e136cd9f21397b0c64229bce0008271 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 4f6f163a501a15d0e32dfc7a28554c891e7a3870 — the merge of head a3179775d3dc3577f6c0d4c3624f5a74a68c6857 into base 16eefc6c0e136cd9f21397b0c64229bce0008271, 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 4f6f163a501a15d0e32dfc7a28554c891e7a3870 && git checkout 4f6f163a501a15d0e32dfc7a28554c891e7a3870
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 16eefc6c0e136cd9f21397b0c64229bce0008271 a3179775d3dc3577f6c0d4c3624f5a74a68c6857 && git checkout -B drift-repro 16eefc6c0e136cd9f21397b0c64229bce0008271 && git merge --no-ff a3179775d3dc3577f6c0d4c3624f5a74a68c6857

node scripts/docs-audit/affected-docs.mjs --json 16eefc6c0e136cd9f21397b0c64229bce0008271

⚠️ 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 16eefc6c0e136cd9f21397b0c64229bce0008271 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 2, 2026 08:03
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 2, 2026 08:03
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 2, 2026
Merged via the queue into main with commit fbe2deb Oct 2, 2026
36 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-21316-objectql-face-order-limit branch October 2, 2026 08:22
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

2 participants