Skip to content

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

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

objectstack-fleet[bot] merged 7 commits into
mainfrom
claude/issue-21365-analytics-window

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Part of #21365
Clause-②: yes (narrowing)

What this changes

Two halves of one family: an analytics query window that had no single answer across drivers and faces.

1. The contract (packages/spec). AnalyticsQuerySchema.limit and .offset were a bare z.number(). They are z.number().int().nonnegative() now (packages/spec/src/data/analytics.zod.ts), each with a .describe().

  • DatasetSelectionSchema reads the same two declarations off AnalyticsQuerySchema.shape, and AnalyticsQueryRequestSchema extends the query. So all three doors hold one accept set, and there is no second declaration. A spec pin asserts that the dataset selection's two members are the same instances.
  • The doors already parse with these schemas: POST /analytics/query and POST /analytics/sql (packages/runtime/src/domains/analytics.ts) and POST /analytics/dataset/query (packages/rest/src/analytics-selection-door.ts). A negative or fractional value now answers 400 VALIDATION_FAILED, with details.fields[].field naming limit or offset (selection.limit / selection.offset at the dataset door), before any engine runs. There is no new refusal code and no runtime code change.
  • limit: 0 stays legal (LIMIT 0, no rows). An offset with no limit stays legal.
  • The TypeScript types are unchanged (number). Only the parse narrows.

2. The offset-only window on the native face (service-analytics). NativeSQLStrategy.assembleStatement wrote OFFSET n with no LIMIT in front of it, which SQLite cannot parse. The window clause is now windowClauseSql(limit, offset, dialect) in native-sql-strategy.ts, with the dialect read off the existing sqlDialect hook (sqlDialectFor, filled by the plugin from the executing driver's dialectName). An offset with no limit takes the dialect's no-limit spelling:

dialect offset-only window evidence
sqlite LIMIT -1 OFFSET n executed (better-sqlite3)
postgres OFFSET n (bytes unchanged) executed (PostgreSQL 16.14)
unknown (no hook wired) LIMIT 9223372036854775807 OFFSET n executed on both engines above
mysql LIMIT 18446744073709551615 OFFSET n NOT MEASURED, text only (no MySQL server here)

The named-dialect cells are the spelling the driver's own query compiler emits for the same window (knex 3.3.0: limit -1 in the sqlite3 compiler, limit 18446744073709551615 in the mysql one, nothing in the base compiler PostgreSQL uses). unknown is everything the hook could not name, SQLite included, so its cell is the largest LIMIT every LIMIT dialect parses. That follows the residue-arm reasoning text-match-sql.ts already records for the same hook. The door does not refuse the offset-only window.

The echoed sql and POST /analytics/sql on the native face are the statement that ran, byte for byte (pinned).

Measured before and after, at the route

Real POST /api/v1/analytics/query and /analytics/sql (createDispatcherPlugin), with AnalyticsServicePlugin over a real ObjectQL engine and SqlDriver. Two compositions: the default one (native face) and one narrowed to the engine aggregate (ObjectQL face). Four groups by note (w, x, y, z), order { note: 'asc' }, SQLite (better-sqlite3) and a private PostgreSQL 16.14 server.

window main ee75aae1a: native SQLite native PostgreSQL ObjectQL face, both drivers this branch, every cell
limit: -1 200, every row 500 200, w x y 400 VALIDATION_FAILED (limit)
limit: 1.5 500 200, w x 200, w 400 VALIDATION_FAILED (limit)
offset: -1 500 500 200, every row 400 VALIDATION_FAILED (offset)
offset: 1, no limit 500 (near "OFFSET": syntax error) 200, x y z 200, x y z 200, x y z
CONTROL limit: 2, offset: 1 x y x y x y x y

On main, /analytics/sql answered 200 for every row, rendering the statement. On this branch it answers the same 400 for rows 1 to 3. The dataset door's selection check (datasetSelectionRefusal) answers 400 VALIDATION_FAILED naming selection.limit / selection.offset for the same four refused values, and passes { offset: 1 } and { limit: 2, offset: 1 }. That was measured at the helper, not over HTTP.

Pins

  • packages/spec/src/data/analytics-query-window-integer.test.ts: on each of the three schemas, a negative limit, a fractional limit, a negative offset and a fractional offset are refused at the member's own path, with zod's issue code (never its message). Integer windows, limit: 0, offset: 0, an offset with no limit and no window are each a whole-safeParse success. Plus the identity pin on the dataset selection's two members.
  • packages/services/service-analytics/src/__tests__/native-sql-offset-only-window.test.ts (SQLite always, PostgreSQL where OS_TEST_POSTGRES_URL is set):
    • the card's row 4 answers the same rows on both faces;
    • the native statement ends with the dialect's spelling, and its echo and generateSql equal the statement that ran;
    • offset 0 and an offset past every row;
    • the dataset door's pushed-down offset-only page on both faces;
    • the unknown arm (a NativeSQLStrategy whose context wires no sqlDialect) runs and answers the same rows;
    • CONTROL: an integer window keeps its bytes;
    • windowClauseSql per dialect, with the MySQL cell as text only.
  • packages/runtime/src/analytics-query-window-validity.test.ts (route level, SQLite always, PostgreSQL where OS_TEST_POSTGRES_URL is set): rows 1 to 3 plus offset: 1.5 answer 400 VALIDATION_FAILED naming the member at both routes, on both faces, and the engine sees no raw statement and no aggregate. Row 4 answers x y z on both faces, and the native echo and /analytics/sql equal the statement that ran. CONTROLs: limit 2, offset 1 and limit 0.

No CI step provisions OS_TEST_POSTGRES_URL for these two packages, so the PostgreSQL cells are red-capable and un-run in CI. The numbers below include them, run against a private local PostgreSQL 16.14.

Ablations

Each leg ran on the committed tree at b1a8ea073 through scripts/ablation-replace.mjs. The anchor was proven to land (count and blob hash), and the restore was proven by blob hash equal to HEAD and an empty git diff HEAD. The spec and service-analytics pins import from src/, and the runtime suite aliases @objectstack/spec to src/, so none of these legs goes through a dist/.

leg spec pin (28) route pin (16, both drivers) service pin (14, both drivers)
.int() dropped (both members) 6 red: the fractional rows on all three schemas 4 red: limit: 1.5 and offset: 1.5, both cells. Native PostgreSQL answered 200 with LIMIT 1.5 and OFFSET 1.5 not run
.nonnegative() dropped 6 red: the negative rows 4 red: limit: -1 and offset: -1, both cells not run
no-limit spelling removed (windowClauseSql writes nothing before OFFSET) not run not run 7 red: the per-dialect text pin; on SQLite row 4, the echo, offset 0 and past, the dataset page and the unknown arm; on PostgreSQL only the unknown arm. 7 green: both CONTROLs and the PostgreSQL cells that run a bare OFFSET

Each leg turned red in the direction expected beforehand. The route pin's row 4 reads service-analytics from dist/ (KNOWN_UNALIASED_TEST_IMPORTS), so the no-limit leg was run on the service pin, which imports from src/.

ADR-0087 disposition

The changeset is declared breaking and carries the disposition registered analytics-query-window-non-negative-integer. That is a new D3 semantic entry, packages/spec/src/migrations/entries/semantic/18.analytics-query-window-non-negative-integer.ts, and registry.ts was regenerated by gen:migration-registry. The gate's other categories do not apply: the package is published, nothing was registered before, the changeset carries a FROM → TO prescription (so no-migration-prescription is refused), and this is a metadata parse, not a runtime interface or a type surface. No D2 conversion: the window is a query-time request field, not a stored sys_metadata shape, and the refused values meant different windows per backend. spec-changes.json and the upgrade guide fold majors up to 17, so a major-18 entry leaves both byte-identical. check:spec-changes and check:upgrade-guide pass.

Acceptance notes

  • The ObjectQL face's echo still renders an offset-only window as OFFSET n with no LIMIT on SQLite. objectql-strategy.ts generateSql, the two window lines. The face's rows are right. Only the echoed text cannot run on SQLite. Measured on this branch: a month-bucketed order + offset: 1 (which lands on that face on every composition) echoes … ORDER BY "closed_on" ASC OFFSET 1 on both drivers. That echo also renders date_trunc(…), which SQLite cannot run either. A non-bucketed query on a composition narrowed to the engine aggregate echoes … ORDER BY "note" ASC OFFSET 1. Rendering it with windowClauseSql is a two-line change, but objectql-strategy.ts is outside this claim's file surface and held by another claim (domain:engine#1's #5930 step 4 (domain:engine): the engine-fed faces delete their hand-copied filter meaning (driver-sql, turso remote, memory query, mongodb, formula, having); the memory reference matcher retires (D6) #20822 group 4, prose only), so it is not changed here. It is raised in the report.
  • MySQL. The mysql cell is text only. Separately, and NOT MEASURED: the native statement quotes identifiers with ANSI double quotes, which MySQL without ANSI_QUOTES reads as string literals (the CI workflow's own comment says this repo's MySQL runs without it). So the native face may not run on MySQL at all, whatever its window. This is an inference from reading the code, not a measurement, and is noted only.
  • In-process callers. AnalyticsService.query does not parse its input, so a host that builds a query in code is not refused by this narrowing. The changeset and the semantic entry say so. The census found no producer of a negative or fractional window: none in examples, fixtures, docs or published skills, and the one stored producer that lowers into a selection, a dashboard widget's limit, is already z.number().int().positive(). The sibling console repository was not checked out here, so its census is NOT MEASURED.
  • Integers above Number.MAX_SAFE_INTEGER are refused too (zod's .int()). That is stated in the changeset.
  • Docs. The generated references (content/docs/references/{api,data}/analytics.mdx) were regenerated by check:generated --fix, and it proved only check:docs stale. No hand-written page in content/docs/** (outside releases/) or skills/** says anything this change makes false.

Verification (head b1a8ea073, after merging main 6d487d209)

  • Full turbo run build (72 tasks) after the merge. Then:
    • pnpm --filter @objectstack/service-analytics test with live PostgreSQL: 168 files, 3883 tests passed;
    • pnpm --filter @objectstack/spec test: 599 files, 17557 passed, 1 todo;
    • @objectstack/runtime: the new route pin, 16 passed on both cells.
  • typecheck of @objectstack/spec, @objectstack/service-analytics and @objectstack/runtime: exit 0. Each new test file is in a compiled program (tsc --listFiles: the spec and runtime test configs, the service-analytics main config), and check:test-typecheck holds both debt ledgers unchanged.
  • Gates: node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derived 116 commands at b1a8ea073, and all 116 exited 0. --ran reconciles 116 derived, 116 run, 0 NOT-MEASURED, 0 UNRUN.
  • Lint, narrowed to the 7 touched .ts files: eslint --no-inline-config --format json read 7 files, 0 errors, 0 warnings, none ignored. eslint.config.mjs enables no type-aware linting (no parserOptions.project), so this diff cannot change the verdict on an untouched file. The repo-wide pnpm lint is left to CI.
  • packages/cli and packages/rest are untouched. @objectstack/runtime changes by one test file only.

Generated by Claude Code

claude added 7 commits October 2, 2026 10:20
…der an offset-only window per dialect

AnalyticsQuerySchema's limit and offset become non-negative integers (the
dataset selection reads the same declarations). The native statement takes
the dialect's no-limit spelling in front of an OFFSET with no LIMIT.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
…et-only window per dialect

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
…ivers and both faces

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation protocol:data 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 2 package(s): @objectstack/service-analytics, @objectstack/spec, touching 6 documentable anchor(s).

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

  • content/docs/releases/v17/17-4.mdx (via AnalyticsQuerySchema (symbol, a top-level const))
  • content/docs/releases/v17/17-6.mdx (via /api/v1/analytics/query (route, a path literal in reason; a path literal in semantic))

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
  • 8 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 — 138 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 ecb6ca0258176466767588a6805363387c5777a6 → packageMentionDocs.

Which tree this was computed on

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

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

⚠️ 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 ecb6ca0258176466767588a6805363387c5777a6 → 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 12:24
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 2, 2026
Merged via the queue into main with commit 6d67ad5 Oct 2, 2026
51 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-21365-analytics-window branch October 2, 2026 12:48
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 protocol:data size/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants