Skip to content

fix(service-analytics): the ObjectQL face echoes an offset with no limit as a statement the dialect runs - #21440

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-21365-objectql-echo-window
Oct 2, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-21365-objectql-echo-window

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #21365
Clause-②: no

What this changes

This is the remainder of #21365 after PR #21399 (6d67ad5ec, which read Part of). That PR narrowed the window contract and made the native face render an offset-only window with the dialect's no-limit spelling. The ObjectQL face's echo still wrote its own window.

ObjectQLStrategy.generateSql (packages/services/service-analytics/src/strategies/objectql-strategy.ts) wrote LIMIT n when a limit was set, then OFFSET n when an offset was. An offset with no limit therefore echoed a bare OFFSET, and SQLite's grammar has no bare OFFSET. Those two lines are now one call:

  • windowClauseSql(query.limit, query.offset, sqlDialectFor(ctx, tableName))
  • windowClauseSql is the function the native face runs, already exported from native-sql-strategy.ts. That file is not edited, and there is no second spelling table.

The ObjectQL face's echoed sql and the POST /api/v1/analytics/sql body (both are generateSql) now carry the same window clause the native face executes on the same driver.

Measured at the real route

The harness is a scratch copy of packages/runtime/src/analytics-query-window-validity.test.ts. It uses the real dispatcher-plugin mount over AnalyticsServicePlugin and a real ObjectQL engine with SqlDriver (better-sqlite3). It boots the default composition and one narrowed to the engine aggregate (the ObjectQL face). The query is order: { note: 'asc' }, offset: 1, with no limit. Each echoed statement was then run on the same SQLite database through knex, to read SQLite's own diagnostic. The harness was deleted and never committed.

main bdd3654f2 this branch
ObjectQL face, /query rows x y z x y z
ObjectQL face, echoed sql = /sql body … ORDER BY "note" ASC OFFSET 1 … ORDER BY "note" ASC LIMIT -1 OFFSET 1
that statement on SQLite near "OFFSET": syntax error x y z
native face, executed = echoed = /sql … ASC LIMIT -1 OFFSET 1, x y z unchanged
CONTROL limit: 2, offset: 1, ObjectQL echo … LIMIT 2 OFFSET 1, runs unchanged

On this query the ObjectQL echo now equals, byte for byte, the statement the native face runs on SQLite.

The dialect source (dispatch, Zone 2 item 2)

generateSql already has the strategy context in scope, and it already reads sqlDialectFor(ctx, tableName) for its read-scope compile. That is the same sqlDialect hook, read the same way, that NativeSQLStrategy.generateSql uses with sqlDialectFor(ctx, this.extractObjectName(cube)). tableName here is this.extractObjectName(cube) too. AnalyticsServicePlugin wires that hook from the data engine's driver whatever the query capabilities are, so the narrowed composition above reads sqlite. No new hook.

Pin

packages/services/service-analytics/src/__tests__/objectql-echo-offset-only-window.test.ts has three tests per cell:

  • The card's row. The ObjectQL face runs no raw statement and does run the engine aggregate. It answers x y z. Its echo ends with the dialect's window. generateSql (the /analytics/sql body) equals the echo, with no params. From ORDER BY on, the echo equals the statement the native face executes on the same driver. Run through the engine's raw-SQL bridge, the echo answers the face's rows.
  • The unknown arm. An ObjectQLStrategy whose context wires no sqlDialect hook echoes LIMIT 9223372036854775807 OFFSET 1, and that statement runs.
  • CONTROL. limit: 2, offset: 1 keeps its bytes, and the echo runs.

The cells:

  • sqlite: executed on every run.
  • live postgres: executed where OS_TEST_POSTGRES_URL is set, and a named skip otherwise. No CI step provisions that variable for this package, so CI runs only the SQLite cell. I ran it locally against PostgreSQL 16.14. There the echo keeps OFFSET 1 alone, byte-identical to before. The comparison with the native statement starts at ORDER BY because on PostgreSQL the native face also casts the summed column.

Ablation at committed 2cfcc44ee, through node scripts/ablation-replace.mjs. The anchor hit once, and the blob changed 9ed31b50c50b to e8b910c3c13f. The mutation restored the two old window lines. The pin reads src/ with no dist/ leg (objectql is aliased to source, and the strategy is imported from src). Result, with live PostgreSQL: 3 failed, 3 passed.

  • Red: the SQLite card row (the echo ends with a bare OFFSET 1), and the unknown arm on both engines.
  • Green: both controls, and the PostgreSQL card row, whose bytes this change does not move.

I predicted that direction before the run. Restore was proven: the blob after restore equals HEAD 9ed31b50c50b, and git diff HEAD is empty.

Tests and gates

All of these ran at head b9456bff3, which is this branch with origin/main 8b123c0ae merged in. That merge brought PR #21424's native-sql-strategy.ts change. windowClauseSql is unchanged by it.

  • pnpm --filter @objectstack/service-analytics test, with OS_TEST_POSTGRES_URL set to a local PostgreSQL 16.14: Test Files 170 passed (170), Tests 3957 passed (3957). os-verify-lock printed VERDICT command-exit 0.
  • pnpm --filter @objectstack/service-analytics typecheck: VERDICT command-exit 0. tsc --noEmit --listFiles lists the new pin once. The package tsconfig includes src and does not exclude tests.
  • Gates: node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands (no paths) derived 63 commands. All 63 ran at b9456bff3. 62 exited 0 on the first run. pnpm check:dual-build-cjs-loads first answered PREREQUISITE NOT MET (exit 3), because the workspace was not fully built. After a full turbo run build (72 tasks, exit 0), it exited 0. --ran reconciles 63 derived, 63 run, 0 NOT-MEASURED, 0 UNRUN. The list includes check:nul-bytes, check:changeset-gate-self-tests, check:adr-0087-registration, check:changeset-no-major and check:test-source-alias. No gate touched AGENTS.md, and the tree stayed clean.
  • eslint: eslint --no-inline-config --format json on the 2 touched .ts files read 2 files, 0 errors, 0 warnings, none ignored. eslint.config.mjs enables no type-aware linting (no parserOptions.project), so an untouched file's verdict cannot move. The repo-wide pnpm lint is left to CI.

Docs

I grepped content/docs/** outside releases/ for the analytics echo, /analytics/sql and window rendering. api/data-api.mdx describes /analytics/sql as a dry run returning { sql, params }, and api/client-sdk.mdx shows client.analytics.explain. Neither says how a window renders. No sentence is made false, so there is no docs edit.

Acceptance notes

  • The bucketed echo is a separate truth (dispatch, Zone 2 item 3), not fixed here. A month-bucketed dimension echoes date_trunc('month', closed_on). That holds on both faces, and in the default composition too, because the native face declines granularity. SQLite refuses it with no such function: date_trunc, measured on this branch after the window fix. On main the same statement failed earlier, at the bare OFFSET. The driver runs strftime('%Y-%m', …) there (driver-sql sql-driver.ts). The comment over dimExpr says the echo renders "the SQL shape the driver's own bucketing implements", and on SQLite that is not so. This is dimExpr, not the two window lines, so it is reported to the seat rather than fixed here.
  • mysql. The window cell is windowClauseSql's own (LIMIT 18446744073709551615). It is NOT MEASURED on either face here, because no MySQL server is available. That is the same declared skip PR fix(analytics)!: a query window outside the non-negative integers is refused at the door, and an offset with no limit runs on SQLite #21399 carries.
  • Unknown dialect on PostgreSQL. A host whose sqlDialect hook names no dialect now echoes LIMIT 9223372036854775807 OFFSET n where it echoed OFFSET n. The native face already runs that statement for such a host. It answers the same rows on PostgreSQL and SQLite, as the pin's unknown arm measures.

Generated by Claude Code

claude added 4 commits October 2, 2026 15:37
…w the dialect runs

generateSql's two window lines now call windowClauseSql (exported from
native-sql-strategy.ts) with sqlDialectFor(ctx, tableName), the same
dialect read the echo's read scope already makes. On SQLite an offset
with no limit echoes LIMIT -1 OFFSET n, the statement the native face
runs, instead of a bare OFFSET that SQLite refuses.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
…atement's from ORDER BY on

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
…ow; add the changeset

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 1 documentable anchor(s).

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

  • content/docs/releases/v14.mdx (via generateSql (symbol, a method of class ObjectQLStrategy))
  • content/docs/releases/v17/17-5.mdx (via generateSql (symbol, a method of class ObjectQLStrategy))

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
  • 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 3a6d92f78bb6a160b762dfee0738fd3b0b7ae6c2 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 3a6d92f78bb6a160b762dfee0738fd3b0b7ae6c2

⚠️ 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 3a6d92f78bb6a160b762dfee0738fd3b0b7ae6c2 → 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 16:51
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 2, 2026 16:51
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 2, 2026
Merged via the queue into main with commit d7d5b4f Oct 2, 2026
36 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-21365-objectql-echo-window branch October 2, 2026 17:13
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