You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit c5ac1b6
Browse filesBrowse the repository at this point in the historyBrowse files
fix(service-analytics)!: the draft preview refuses one column asked raw beside its own time bucket, as the live faces do (#28794)
domain:server#3 · os-project-manager · session_01FEkQ5kE5BrPUPNMMyv6oCH
<!-- claude-projects-attribution: {"github_login":"os-project-manager"}
-->
_Requested by **Leehom** · [project
thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XB1hsVtuncZ5F5C4zpiftHC)_Fixes#28765
Clause-②: no (narrowing) — the draft preview (`queryDataset` with
`previewDrafts: true`) now refuses `INVALID_QUERY` / 400 a selection
that asks one column for two values under one name (a dimension `X`
beside a `timeDimensions` entry `OBJECT.X` with a granularity, or two
buckets of one column), with the message, `path`, `param`, `object` and
`fields` the native and ObjectQL strategies already answer; it served
both columns. Nothing is widened: no package-root export, public member,
answer member, error code or status is added, and the code was already
on the wire.
Metadata-protocol: none
Skills-docs: none — no file under `skills/**` or `content/docs` says the
draft preview serves a column raw beside its own bucket.
**BREAKING** accept-set narrowing on the `queryDataset` draft preview
(`@objectstack/service-analytics`).
Card thread: `session_01FEkQ5kbUJTTWAxjV9nqr1P`.
## 维护者速读
- **改了什么**:草稿预览遇到「同一列既要原值又要它的时间桶」(例如维度 `closed_at` 加上按月分桶的
`对象名.closed_at`),或同一列的两个时间桶时,改为 400 `INVALID_QUERY` 拒绝,消息、`path` 等字段与原生
SQL、ObjectQL 两面逐字一致。之前预览照样给出两列,而发布后线上会拒绝。
- **为什么改**:卡 #28765(p3),是 PR #28762 开发时实测到的同族缺口:作者在草稿里看到一张表,发布后同一个查询却
400。
-
**风险与回滚**:这是预览面的收窄,需要合约评审(席位跑)。只拒绝线上本来就拒绝的写法;不同列的时间桶、维度本身也作为时间维度(一个桶)、同列两个维度(#28744)照样服务,都有测试钉住。线上两面行为不变。回滚即
revert。
- **先看**:新文件
`packages/services/service-analytics/src/preview-group-column-door.ts`;`strategies/objectql-strategy.ts`
抽成模块级函数的 `aggregateGroupKeys`(不从包根导出,类的公开成员不变);`analytics-service.ts`
预览闸门列表末尾的调用。
- **两分钟验证**:`pnpm --filter @objectstack/service-analytics exec vitest
run src/__tests__/preview-time-filter-fk-parity.test.ts`,预期 27 passed。
- **需要你做的**:无,席位落地(收窄需先过合约评审)。
## What changes
- **`strategies/objectql-strategy.ts`.** The direct path's `groupBy`
build moves, unchanged, into the module-level `aggregateGroupKeys(query,
cube)`, which `execute` calls. It reads only the cube, through
`lookupCubeMember` and `resolveCubeFieldName`, which are now
module-level too; the strategy's private `lookupMember`,
`resolveFieldName` and `extractObjectName` delegate to them and to
`aggregateObjectOf(cube)` (`cube.sql.trim()`). None of these is exported
from the package root (`src/index.ts` names `ObjectQLStrategy` alone
from this module), and the class gains no public member: the built
`dist/index.d.ts` names none of `aggregateGroupKeys`,
`aggregateObjectOf`, `AggregateGroupKey`. No behaviour change on the
live faces.
- **`preview-group-column-door.ts` (new).**
`assertPreviewGroupColumnsDistinct(query, cube)` builds the strategy's
keys with `aggregateGroupKeys`, reads the object the way the strategy
does (`aggregateObjectOf`), and refuses keys that project one column
twice: `INVALID_QUERY` / 400 with the engine's message, `path`
(`groupBy.N` / `groupBy.N.field`), `param: 'groupBy'`, `object` and
`fields`, plus `httpStatus`.
- **Not a shared helper, and why.** The live refusal is
`@objectstack/objectql`'s `assertAggregatedColumnsAreDistinct`
(`aggregate-query-shape-door.ts`). `@objectstack/objectql` is a dev
dependency of `service-analytics`, never a runtime one, and its root
does not export the door, so the preview cannot call it without a new
runtime dependency or a `packages/objectql` export. The order allows a
copy in that case: only the `groupBy` half is copied (the strategy's
keys carry no `alias`), and the test compares the preview's refusal with
both live faces field by field, message included, so a change to the
engine's wording reds it (ablation C2).
- **`analytics-service.ts`.** The preview gate list asks the door last,
after `assertPreviewJoinsNothing`: `assertPreviewGroupColumnsDistinct(q,
cube)`. The positions it names are the engine's by construction, since
the strategy's `execute` builds its `groupBy` with the same function.
- **Contract review rework (`d26d29b4`).** The first head (`722fbe7c`)
added a public `groupByOf` to the root-exported `ObjectQLStrategy`,
which widened the published surface under a narrowing declaration, and
the service held the strategy by name. Both are gone: the builder is
module-level (remedy b), and the door takes the cube and reads the
object off it (should-fix 1).
- Tests: four cases added to
`src/__tests__/preview-time-filter-fk-parity.test.ts` (now 27, real
SQLite).
- Changeset
`.changeset/28765-preview-refuses-column-raw-beside-bucket.md`,
`@objectstack/service-analytics` patch, with the adr-0087 not-required
marker.
## Why
Measured on `origin/main` `850eab2a` (real SQLite, every face), dataset
over a deal object (`region`, `closed_at` datetime) with dimensions
`region`, `closed_at` (date):
| selection | native | ObjectQL | preview before | preview after |
|:--|:--|:--|:--|:--|
| dims `closed_at` + td `OBJECT.closed_at` month | `INVALID_QUERY`,
`groupBy.1.field` … `groupBy.0` | same | served raw instants + months |
same refusal as live |
| dims `region`, `closed_at` + td `OBJECT.closed_at` month |
`INVALID_QUERY`, `groupBy.2.field` … `groupBy.1` | same | served | same
refusal as live |
| td `closed_at` month + td `OBJECT.closed_at` year | `INVALID_QUERY`,
`groupBy.1.field` … `groupBy.0.field` | same | served | same refusal as
live |
| dims `region` + td `closed_at` month | served | served | served |
served (unchanged) |
| dims `closed_at` + td `closed_at` month | one bucket | one bucket |
one bucket | one bucket (unchanged) |
## Tests
<details><summary>Red first, green, ablations</summary>
- Red on base: `analytics-service.ts` and `objectql-strategy.ts`
restored to `850eab2a` and the new door removed, tests at `722fbe7c`:
`Tests 3 failed | 24 passed (27)`; the three refusal cases fail on the
preview leg. Restored, `git status` clean.
- Green at `d26d29b4`: `27 passed (27)`.
- Ablations at `d26d29b4`, via `node scripts/ablation-replace.mjs` (each
anchor 1 → 0, restore blob == HEAD, `git diff HEAD` empty):
- C1 `--delete` the preview's door call: `3 failed | 24 passed` (the
three refusals).
- C2 one word of the copied message changed (`NOT run` → `not run`): `3
failed | 24 passed` (the field-by-field comparison with the live faces).
- C3 a bucketed key's path spelled `groupBy.N` instead of
`groupBy.N.field`: `3 failed | 24 passed`.
- C4 the door refusing any second key, whatever its column: `3 failed |
24 passed` (region + month served, #28744 beside a month bucket, and the
later-position refusal); so the still-served pins bite.
- C5 the door's object read as `cube.name` instead of
`aggregateObjectOf(cube)`: `3 failed | 24 passed`.
- `pnpm --filter @objectstack/service-analytics exec vitest run` at
`d26d29b4`: `Test Files 203 passed (203)`, `Tests 4866 passed | 282
skipped`.
- `pnpm --filter @objectstack/rest exec vitest run src/analytics` at
`d26d29b4`: `Test Files 23 passed (23)`, `Tests 298 passed | 22
skipped`.
- `pnpm --filter @objectstack/service-analytics typecheck` at `d26d29b4`
exit 0; `eslint` on the touched source files exit 0.
</details>
## Acceptance notes
- Consumers of the preview: the Studio paths that pass `previewDrafts`
(`ui/packages/app-shell` dataset and chart previews) already receive the
preview's other 400 refusals (#28555 and earlier); this adds one more
selection to that set, never one a published query serves.
- Two buckets of one column (month and year) are refused too: it is the
same engine refusal on the live faces, measured, and the card's root
(one column, two values, one name).
- The preview's refusal runs after every other preview gate, so a
selection refused for another reason keeps that refusal.
## Verification
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack`: 62 families derived on `d26d29b4` (5 paths
vs merge base `8afaf12d`; not stale against `origin/main` `8196881e`,
which touches no `service-analytics` file); each run with stdin from
`/dev/null`, exit captured before any pipe, after a full `turbo run
build --filter=!@objectstack/docs --concurrency=2` on the same merge
base and a rebuild of `service-analytics` at `d26d29b4` (lock VERDICT
command-exit 0). `--ran`: **62 derived, 62 run, 0 NOT-MEASURED, 0
UNRUN**, every one `exit 0`.
- Hand gates: `node scripts/check-platform-checklist.mjs --changed-from
origin/main` exit 0; `node scripts/check-commit-card-trailers.mjs
--range origin/main..HEAD` exit 0.
- Dogfood battery (Clause-② carries a narrowing arm), on `d26d29b4`:
`pnpm --filter @objectstack/dogfood test` under the lock, VERDICT
command-exit 0, `Test Files 376 passed | 3 skipped (379)`, `Tests 3069
passed | 28 skipped (3097)`, no failure (also on `722fbe7c`, same
counts).
- CI families: read from this PR's check-runs.
<details><summary>The 62 derived families and their exits</summary>
- `node scripts/check-adr-0087-registration.mjs --base origin/main` ::
exit 0
- `node scripts/check-adr-0087-registration.mjs --self-test` :: exit 0
- `node scripts/check-changeset-no-major.mjs --base origin/main` :: exit
0
- `node scripts/check-closing-keyword-parity.mjs` :: exit 0
- `node scripts/check-closing-keyword-parity.mjs --self-test` :: exit 0
- `node scripts/check-comment-mask-adoption.mjs` :: exit 0
- `node scripts/check-comment-mask-adoption.mjs --self-test` :: exit 0
- `node scripts/check-comment-mask-corpus.mjs` :: exit 0
- `node scripts/check-dts-emitted.mjs --self-test` :: exit 0
- `node scripts/check-empty-changeset.mjs --base origin/main` :: exit 0
- `node scripts/check-empty-changeset.mjs --self-test` :: exit 0
- `node scripts/check-issue-citations.mjs` :: exit 0
- `node scripts/check-plugin-teardown-shape.mjs` :: exit 0
- `node scripts/check-plugin-teardown-shape.mjs --self-test` :: exit 0
- `node scripts/check-tenant-audit-census.mjs` :: exit 0
- `node scripts/check-tenant-audit-census.mjs --self-test` :: exit 0
- `node scripts/ci/assert-turbo-remote-cache.mjs --self-test` :: exit 0
- `node scripts/ci/plan-test-core-shards.mjs --self-test` :: exit 0
- `node scripts/ci/prune-turbo-cache.mjs --self-test` :: exit 0
- `node scripts/docs-audit/check-affected-docs.mjs` :: exit 0
- `node scripts/docs-audit/check-drift-comment.mjs` :: exit 0
- `node scripts/pm/release-rehearsal-clone.mjs --self-test` :: exit 0
- `node scripts/release-pending-publish.mjs --self-test` :: exit 0
- `pnpm --filter @objectstack/spec run check:duration-unit-keys` :: exit
0
- `pnpm check:changeset-gate-self-tests` :: exit 0
- `pnpm check:ci-filter-parity` :: exit 0
- `pnpm check:closed-ceremony-text` :: exit 0
- `pnpm check:cross-package-test-inputs` :: exit 0
- `pnpm check:dispatcher-error-vocabulary` :: exit 0
- `pnpm check:doc-authoring` :: exit 0
- `pnpm check:driver-memory-census` :: exit 0
- `pnpm check:dts-closure` :: exit 0
- `pnpm check:dual-build-cjs-loads` :: exit 0
- `pnpm check:engine-double-contract` :: exit 0
- `pnpm check:error-status-conformance` :: exit 0
- `pnpm check:gitlink-declared` :: exit 0
- `pnpm check:issue-citations` :: exit 0
- `pnpm check:keyed-text-bounds` :: exit 0
- `pnpm check:lean-entry-closure` :: exit 0
- `pnpm check:logger-receiver-detach` :: exit 0
- `pnpm check:nul-bytes` :: exit 0
- `pnpm check:objectql-double-limit` :: exit 0
- `pnpm check:org-identifier` :: exit 0
- `pnpm check:page-declaration-shape` :: exit 0
- `pnpm check:platform-object-tenancy-census` :: exit 0
- `pnpm check:pm-changeset-deadline-census` :: exit 0
- `pnpm check:published-files` :: exit 0
- `pnpm check:query-options-erasure` :: exit 0
- `pnpm check:refd-timer-probe` :: exit 0
- `pnpm check:registry-log-declared` :: exit 0
- `pnpm check:rest-log-spy-declared` :: exit 0
- `pnpm check:slot-lookup` :: exit 0
- `pnpm check:sourcemap-no-sources-content` :: exit 0
- `pnpm check:swallow-census-controls` :: exit 0
- `pnpm check:system-context-census` :: exit 0
- `pnpm check:test-source-alias` :: exit 0
- `pnpm check:tier-file-adoption` :: exit 0
- `pnpm check:type-check-coverage` :: exit 0
- `pnpm check:type-check-debt` :: exit 0
- `pnpm check:undeclared-dep-imports` :: exit 0
- `pnpm check:watch-hint-literal` :: exit 0
- `pnpm check:where-matcher` :: exit 0
</details>
## Human Minimum
- [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic.
(No `packages/spec` change.)
- [x] 2. Every name I added uses the casing for its kind.
- [x] 3. No formatter ran; `pnpm lint` passes (scoped run above; the
repo-wide run is CI's).
- [x] 4. One issue, one branch off an up-to-date `main`, one pull
request.
- [x] 5. A change that publishes carries a changeset, 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
dispatch contract's).
- [x] The *Before Submitting* list in CONTRIBUTING.md is done.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01FEkQ5kbUJTTWAxjV9nqr1P)_
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01FEkQ5kbUJTTWAxjV9nqr1P
---------
Co-authored-by: Claude <noreply@anthropic.com>
fix(service-analytics)!: the draft preview refuses one column asked raw beside its own time bucket, as the live faces do
6
+
7
+
Clause-②: no (narrowing) — the draft preview (`queryDataset` with `previewDrafts: true`) now refuses `INVALID_QUERY` / 400 a selection that asks one column for two values under one name (a dimension `X` beside a `timeDimensions` entry `OBJECT.X` with a granularity, or two buckets of one column), with the message, `path`, `param`, `object` and `fields` the native and ObjectQL strategies already answer; it served both columns. Nothing is widened: no export, answer member, error code or status is added, and the code was already on the wire.
8
+
Metadata-protocol: none
9
+
10
+
<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export or type of the package root changes: a selection both live faces already refuse is now refused by the draft preview too, with their refusal. There is nothing for a migration to rewrite and no tombstone. -->
11
+
12
+
**BREAKING** accept-set narrowing on the `queryDataset` draft preview (`@objectstack/service-analytics`).
13
+
14
+
-**Before.** Measured on real SQLite: `dimensions: ['closed_at']` with `timeDimensions: [{ dimension: 'OBJECT.closed_at', granularity: 'month' }]` was refused by both live strategies with the engine's duplicate-column refusal (`groupBy.1.field names the column 'closed_at', which groupBy.0 already names`), while the draft preview answered one row per raw timestamp carrying both columns. The same held for a month and a year bucket of one column.
15
+
-**Now.** The preview judges the keys the ObjectQL strategy would hand the engine for the selection, at the same positions, and refuses with the engine's refusal for them, so all three faces answer one `INVALID_QUERY` / 400 with one message.
16
+
-**Unchanged.** A raw dimension beside a bucket of a different column, a dimension that is also its own time dimension (one bucket), and two dimensions over one column with no bucket (#28744) are still served on every face. The live refusal does not move.
17
+
-**Who sees it.** An author previewing a draft dataset with such a selection: the preview now says what publish would say, instead of a table publish refuses.
18
+
19
+
**The remedy.** Ask for the column once: as the bucketed time dimension, or as the raw dimension.
0 commit comments