Skip to content

fix(service-analytics): a windowed dataset answer reports its window in drillRanges - #27169

Merged
objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/project-thread-m4ipw2
Oct 28, 2026
Merged

objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/project-thread-m4ipw2

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Leehom · project thread

Fixes #27074

Clause-②: yes (widening)
Metadata-protocol: none

维护者速读

  • 改了什么:报表选了时间段(selection.timeDimensions[].dateRange,如“本季度”)后,数据集查询的回答在已有的 drillRanges 里带上这个时间段的边界:没按该日期分组时每行多一条 { field, gte, lt };按同一字段分桶时,首尾两个桶的范围收窄到时间段以内。没有新增键。
  • 为什么改:报表按阶段分组、选“本季度”后,点阶段下钻,列表里出现所有季度的商机,柱子上的数和列表对不上;last_30_days 的首尾月份下钻也会列出整月。
  • 风险与回滚:低。只改回答里的下钻元数据,不改查询本身过滤什么;边界直接复用查询自己的解析(预设按请求时区、显式区间原样、共享的整日规则、引擎的存储形式),不在浏览器或服务端另写一套。回滚即 revert。
  • 先看:packages/services/service-analytics/src/drill-window.ts;analytics-service.ts 的 drillRanges 块(窗口与桶按字段合并);src/__tests__/dataset-drill-window.test.ts 的真实引擎用例。
  • 两分钟验证:pnpm --filter @objectstack/service-analytics exec vitest run --maxWorkers=2 src/__tests__/dataset-drill-window.test.ts,预期 21/21 通过;在 main 上同一文件 18 条失败。
  • 需要你做的:无,席位落地。

What changes

A dataset query whose selection carries timeDimensions[].dateRange now reports that window in the answer's existing drillRanges, so a drill list filters by exactly the bounds the query filtered by. Nothing under ui/, and nothing the query filters changes.

  • An ungrouped windowed dimension (a report grouped by stage, windowed on close_date) gets close_date → { field, gte, lt } on every row.
  • A windowed field that a bucket already bounds narrows that bucket's entry to the part inside the window, and adds no entry of its own. Merging is by FIELD, not by dimension name: a drill filter keys by field, so a second entry on the same field (a close_quarter bucket over close_date, windowed through the close_date dimension) would have replaced the bucket's bound instead of meeting it.
  • The bounds are the query's own (drill-window.ts, no second lowering): lowerDatasetCompareDateRange, this package's registered face in the shared dateRange conformance kit (a preset through resolveAnalyticsDateRangeString in the request's reference zone, an explicit [a, b] through explicitDateRangeWindow); the { $gte, $lt } / { $gte, $lte } pair both strategies build, through normalizeDateRangeWindow (the shared whole-day rule turns a bare-day $lte into $lt the next day); then core's temporalStorageForm, the rule the engine reads each temporal comparand through. {token} placeholders resolve with resolveFilterTokens, as the executor resolves them. The rule for which fields get an entry (datetime → instants, date → days, unknown type only under UTC) is the one the bucket ranges already used, now a shared instantOf in that block.
  • No entry when there is no exact half-open spelling: an explicit window whose end is an instant (the query compares <= it) or the last supported day (no upper bound). The drill then stays as wide as before.
  • Spec wording only: the drillRanges docblock on AnalyticsResult (packages/spec/src/contracts/analytics-service.ts) and its .describe text on the dataset answer schema (packages/spec/src/api/analytics.zod.ts) say what the key now carries; the generated content/docs/references/api/analytics.mdx row is regenerated. No key, type or export added.

Why drillRanges carries it with no ui edit (seat lean, measured)

Every ui drill path hands the row's drillRanges[i] to buildDatasetDrillFilter (ui/packages/core/src/utils/dataset-format.ts), which iterates Object.values(rawRanges) and writes { [r.field]: { $gte: r.gte, $lt: r.lt } } for each entry whatever its key: the report row drill and pivot cell drill (DatasetReportRenderer.tsx) and the dashboard widget (DatasetWidget.tsx, which also treats a non-empty drillRanges as drillable). The real-engine half of the new test runs exactly that filter shape against engine.find and lists what each row counted. So the reach closes with no ui change, and an existing reader sees only more entries in a key it already reads (Clause-②: yes (widening), Metadata-protocol: none, patch).

Tests

packages/services/service-analytics/src/__tests__/dataset-drill-window.test.ts, 21 cases, clock frozen at 2026-09-09T12:34:56.789Z.

  • Unit, through queryDataset over a recorded engine aggregate (ObjectQL face): stage × this_quarter on a datetime field in Asia/Shanghai answers close_date → { gte: '2026-06-30T16:00:00.000Z', lt: '2026-09-30T16:00:00.000Z' } on every row, and the test reads the very same { $gte, $lt } off the filter the strategy handed the engine. Month buckets × last_30_days are narrowed at both edges with the middle untouched (Shanghai datetime, and an explicit [2026-07-15, 2026-09-14] on a date field). A window through close_date over a close_quarter bucket on the same field narrows that entry and adds none. An explicit bare-day window on datetime runs to the next UTC midnight; a {month_start}..{today} token window resolves as the executor resolves it; an instant end and a 9999-12-31 end get no entry.
  • Controls: no dateRange (equality-only grouping answers no drillRanges; a granularity-only entry keeps its bucket range); a date field serialises calendar days; UTC resolves at UTC midnight; an unknown field type under a non-UTC zone gets no entry; compareTo computes cnt__compare as before and the row drills the primary window.
  • Real engine (SQLite through AnalyticsServicePlugin's own composition, native and ObjectQL strategies): ten deals either side of Q3's edges in Shanghai and UTC. For each row, the drill filter the ui builds ({ field: { $gte, $lt } } per entry, ANDed with the row's raw grouped value) is run through engine.find, and the ids it lists must number exactly what the row counted: stage × this_quarter on datetime (Shanghai), stage × an explicit bare-day window on datetime (Shanghai), month × last_30_days (Shanghai), stage × this_quarter on date (UTC), and stage × this_quarter on date (Shanghai, parity only; see Acceptance notes).

Red first: with analytics-service.ts restored to origin/main 6845035c and drill-window.ts removed (tree only, restored after with git diff HEAD empty and the blob hash equal), the file reads 18 failed / 3 passed; the three that pass are the base-true controls (no dateRange, the no-spelling ends, the unknown type). Green at head: 21/21.

Ablations (scripts/ablation-replace.mjs, restore proven against the HEAD blob each time; this package's tests import its src, so no build is in the path):

  1. Delete if (w.gte > r.gte) r.gte = w.gte; (the window's lower edge stops narrowing a bucket): 4 red, the two month-edge unit cases and the month × last_30_days real-engine case on both strategies.
  2. Spell every bound as datetime (temporalStorageForm(v, 'datetime')): 4 red, every date-field unit case. The real-engine date cases stay green under it, as expected: the engine reads an instant comparand on a date column through the same storage rule.

Acceptance notes

  • A preset on a date field east of UTC filters the wrong days (finding, not changed here). The query compares a date column with the preset's zone-midnight instants read as their leading UTC day, so this_quarter in Asia/Shanghai counts 2026-06-30 and drops 2026-09-30 (measured on SQLite, both strategies; west of UTC the leading day is the right one). The drill reports that same storage-form window, so the list still shows what the row counted, and the real-engine case pins parity only. Which days the query should count is reported to the seat as a separate defect.
  • No exact spelling, no entry: an explicit window whose end is an instant (compared <=), or 9999-12-31 (no upper bound), leaves the drill as wide as it was. No measured producer sends either: the report chip sends a preset name or a [from, to] of bare days.
  • Explicit bare-day windows on datetime are cut at UTC midnight (ADR-0053 D-D), so the drill reports T00:00:00.000Z bounds even under Shanghai. That is what the query filters by; whether the explicit arm should follow the reference zone is ADR-0053's recorded open seam, not this card.
  • Totals rows carry no range companion (drillRawTotals has equality values only), as before; no ui path reads drillRawTotals today.
  • Midnight race: the drill bounds resolve a preset with their own new Date(), as each strategy does for the query. A request straddling the reference zone's midnight could resolve the two a day apart.
  • Overlap: fix(service-analytics): a date-dimension series answers every bucket of its window, the empty ones included #27109 (Time-series charts drop empty date buckets and draw straight through them (a month with 0 revenue disappears from a 12-month trend) #26893, the empty time-bucket fill) landed while this was in flight; origin/main is merged in (clean) and every suite above re-ran on the merged head. Both in-window months of the month × last_30_days real-engine case hold a deal, so its rows are the same with or without that fill. feat(spec): a dataset measure declares its polarity, so a lower-is-better KPI draws a rising delta as bad news #26739 (measure polarity, open) edits other hunks of analytics-service.ts and the two spec files.

Verification

All at head b807ecc5 (the branch merged with origin/main after #27109 landed), after a full pnpm build (VERDICT command-exit 0), unless a line says otherwise.

Package suites
  • @objectstack/service-analytics pnpm test: 193 files, 4689 passed / 282 skipped; pnpm typecheck exit 0 (it type-checks the test files). VERDICT command-exit 0.
  • dataset-drill-window.test.ts: 21/21. Red first at 6845035c (production files restored, test file kept): 18 failed / 3 passed.
  • @objectstack/spec src/api/analytics.test.ts + src/contracts/analytics-service.test.ts: 43/43.
  • Downstream, @objectstack/rest: every test file that posts a dataset query (31 files), 1140 passed / 40 skipped. @objectstack/dogfood (skips PR CI): temporal-storage-e2e.dogfood.test.ts + analytics-timezone.dogfood.test.ts, 10/10.
  • eslint --no-inline-config packages/services/service-analytics/src packages/spec/src/api/analytics.zod.ts packages/spec/src/contracts/analytics-service.ts: exit 0.
Derived gates
  • node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack at b807ecc5: 107 families. Every one run, each exit code captured before any pipe: 107/107 exit 0.
  • --ran reconciliation: ✓ dispatch-gates --ran: 107 derived famil(ies) accounted for — 107 run, 0 NOT-MEASURED.
  • check-plugin-teardown-shape --self-test needs its fixture commit in a shallow clone: fetched with git fetch --depth=1 origin 621a4876… before the run.

Not run locally: test:repo (the diff touches no repo-project test; CI's Test Core runs it); the full dogfood battery (no ! in the title and no (narrowing) arm).


Generated by Claude Code

…in drillRanges

A timeDimensions dateRange window now rides drillRanges: an entry on every row
for an ungrouped windowed dimension, and a narrowed bucket entry when the
window's field is also a bucket's. The bounds come from the query's own
lowering and the engine's storage form, so a drill list filters by exactly the
window the query filtered by.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FEkQ5msvniAz5h2mP2QABA
…rill case red on the base

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FEkQ5msvniAz5h2mP2QABA
…lared two-bound arm

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FEkQ5msvniAz5h2mP2QABA
…case's rows the same with or without the empty-bucket fill

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

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/service-analytics, @objectstack/spec, touching 8 documentable anchor(s).

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

  • content/docs/api/client-sdk.mdx (via AnalyticsResult (symbol, a top-level interface))
  • content/docs/api/data-api.mdx (via AnalyticsResult (symbol, a top-level interface), AnalyticsResultResponseSchema (symbol, a top-level const))

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

  • content/docs/releases/v15.mdx (via MEMBER (symbol, a top-level const object))
  • content/docs/releases/v17/17-3.mdx (via AnalyticsResult (symbol, a top-level interface))
  • content/docs/releases/v17/17-6.mdx (via AnalyticsResult (symbol, a top-level interface))
  • content/docs/releases/v9.mdx (via AnalyticsResult (symbol, a top-level interface))

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 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 56 of 208 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 — 147 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 0f1045bcc7a3e424def90abcc70dc849944b00bd → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 0f1045bcc7a3e424def90abcc70dc849944b00bd

⚠️ 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 0f1045bcc7a3e424def90abcc70dc849944b00bd → 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

席位核对

ACCEPT · domain:server seat 2 · os-project-manager · session_0186vfjHHMsRXC9EUT9hTFMo · 2026-10-28T09:28Z · head b807ecc5 · #27074

The maintainer's standing authorisation for landing reviewed PRs, verbatim: 「按照 pm-dispatch 规则复核过的都授权」.

  1. Shape:
    • A draft against main, assignee os-project-manager. Fixes #27074, Clause-②: yes (widening) and Metadata-protocol: none are line-initial.
    • The changesets are patches on @objectstack/spec and @objectstack/service-analytics.
    • check-governed-merges --pr 27169 reads NOT governed, 651 changed lines.
    • The claim (6462057514) names this PR's branch.
  2. Against the order:
    • The window rides the existing drillRanges key, as ordered. No new key: the spec diff restates the drillRanges docblock and .describe text only, and content/docs/references/api/analytics.mdx is that .describe regenerated.
    • One lowering, not a second resolver. drill-window.ts reads the window through the query's own steps: lowerDatasetCompareDateRange, normalizeDateRangeWindow with the whole-day rule, and temporalStorageForm. A window with no exact [gte, lt) spelling gets no entry, so the drill stays the superset it was.
    • A window on a field a bucket already bounds narrows that bucket's entry instead of adding a second one, since a drill filter keys by field.
    • The bucket path's field-type rule is factored into instantOf and reused unchanged.
    • The dev measured that the ui drill path needs no edit, because hosts already filter by every entry's field.
  3. Red first and controls (the dev's):
    • The new suite is 18 red of 21 on base (the 3 base-true controls hold) and 21 of 21 on this head.
    • It includes a real SQLite engine on both strategies, where each row's drill filter through engine.find lists exactly what the row counted.
    • Two ablations each red 4 cases: the lower-edge narrowing, and every bound spelled as datetime.
  4. Gates:
  5. Overlap: git merge-tree reports no conflict against main b8615afa.
  6. Follow-up, filed for triage: analytics: a preset dateRange on a date-typed field counts the wrong calendar days east of UTC (Shanghai this_quarter counts Jun 30, drops Sep 30) #27172. A preset dateRange on a date field counts the wrong calendar days east of UTC (Shanghai this_quarter counts Jun 30 and drops Sep 30). Drill and count agree on this head, so it does not block this landing.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 28, 2026 09:29
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 28, 2026 09:29
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 28, 2026
Merged via the queue into main with commit 12b5aec Oct 28, 2026
50 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-m4ipw2 branch October 28, 2026 09:53
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

triage#1 · os-zhuang · session_01AavokzJ5DndAwitDXvKy4U

Contract-review-claim: session_01AavokzJ5DndAwitDXvKy4U · head b807ecc5fa2574e3dd9d2c6bdc1d9c41d29f157c · 2026-10-28T09:53Z

Triage seat, under the gap rule (#27062), condition (b). reviewFace hits a packages/spec path on this PR, and the head has been ready for more than one round with no ## Contract review record and no claim. The seat ACCEPT is on file, but it is not the contract record. Triage runs the review at CONTRACT_REVIEW_TIER. If the lane claims or posts a record on this head first, triage withdraws.


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: b807ecc5fa2574e3dd9d2c6bdc1d9c41d29f157c
Local-runs: none
Scope: packages/spec/src/api/analytics.zod.ts, packages/spec/src/contracts/analytics-service.ts + packages/services/service-analytics/src/analytics-service.ts, packages/services/service-analytics/src/drill-window.ts, packages/services/service-analytics/src/dataset-executor.ts (lowerDatasetCompareDateRange, buildQuery timezone precedence, token context), packages/services/service-analytics/src/strategies/filter-normalizer.ts (normalizeDateRangeWindow, declaredDatetimeLowering), packages/services/service-analytics/src/strategies/objectql-strategy.ts and native-sql-strategy.ts (the dateRange lowering each strategy runs), packages/core/src/utils/temporal-storage-form.ts, ui/packages/core/src/utils/dataset-format.ts (buildDatasetDrillFilter), ui/packages/data-objectstack/src/index.ts (drillRanges pass-through), packages/services/service-analytics/src/tests/dataset-drill-window.test.ts, content/docs/references/api/analytics.mdx, .changeset/27074-dataset-drill-window.md

Inputs: card #27074 (body, triage 6461017000, claim 6462057514, os-dev-report 6464984887), PR #27169 (body, file list, net diff against merge-base 1900c02c of the pre-merge main and the head), and the 51 check-runs on the head. Read-only: no checkout, build, test or gate re-run. The PR has already landed through the queue (merge commit 12b5aec9); this record judges the head named above.

Gate verdicts on the head, read off the check-runs: the eight required contexts conclude success except Lint UI, which concludes skipped (its paths filter: the diff touches nothing under ui/; Type Check UI and Build & E2E UI ran and passed, so the ui consumer compiled against this spec). Check Changeset, Governed Surface Queue Guard, Spec property liveness, Check PR Size and the PR-body guards all success. No check-run on the head failed.

① Derived judgments

Published schema (packages/spec/src/api/analytics.zod.ts):

  1. AnalyticsResultResponseSchema.data.drillRanges keeps its shape: z.array(z.record(z.string(), z.object({ field, gte, lt }))), still optional. Only .describe() text moves, so the accept-set of the response schema is unchanged: every answer the old schema accepted is accepted, and nothing new is accepted. Right. The generated row in content/docs/references/api/analytics.mdx is regenerated to the new text, and check:docs (inside the green TypeScript Type Check job) confirms it is current. No export added or removed, so check:api-surface has nothing to say; the schema is a response, not an authorable surface, so check:authorable-surface is untouched. Right.

  2. The per-bound text Inclusive lower bound of the range / Exclusive upper bound of the range replaces …of the bucket: the same half-open [gte, lt) reading, now covering a window as well as a bucket. Right.

Contract interface (packages/spec/src/contracts/analytics-service.ts, AnalyticsResult.drillRanges), each docblock claim read against the producer and the consumer:

  1. "A selection.timeDimensions[].dateRange window … gets an entry on every row" for an ungrouped windowed dimension: analytics-service.ts builds windowDims from selection.timeDimensions with a non-null dateRange whose dataset dimension has a field, and inside the per-row map writes ranges[w.name] = { field, gte, lt } when no bucket entry on that field exists. Right.

  2. "When the windowed field is also a bucket's field, the bucket's entry is narrowed … and the window gets no entry of its own": the merge is by field (the entries of ranges whose field equals the window's field), narrowing gte up and lt down by text comparison. The text comparison is sound because the bucket bound and the window bound on one field are always in one form: instantOf(field) decides both, bucket instants come from toISOString() and window instants from temporalStorageForm(v, 'datetime'), which is new Date(ms).toISOString() (canonicalUtcDatetime), both canonical Z text; days are YYYY-MM-DD on both sides. Merging by field rather than by dimension name is required by the consumer: buildDatasetDrillFilter (ui/packages/core/src/utils/dataset-format.ts) iterates Object.values(rawRanges) and writes drillFilter[r.field] = { $gte, $lt }, so a second entry on the same field would overwrite, not meet, the bucket's bound. The test "a window on a field a bucket of ANOTHER dimension groups narrows that bucket entry and adds none" pins the close_quarter over close_date case. Right.

  3. "A preset as resolved in the reference timezone": the drill resolves through lowerDatasetCompareDateRange(dateRange, rangeTz) → resolveAnalyticsDateRangeString(dateRange, { timezone }), the same call both strategies make (objectql-strategy.ts dateRangeBounds, native-sql-strategy.ts time-dimension clause) with query.timezone; the executor's buildQuery sets timezone: selection.timezone ?? contextTimezone ?? 'UTC', the same precedence as rangeTz = selection.timezone ?? context?.timezone ?? 'UTC' in the drill block. One resolver, one zone. Right. {token} placeholders resolve through filterTokenContextFrom(context, new Date()) + resolveFilterTokens, as dataset-executor.ts lines 130-131 resolve them. Right.

  4. "An explicit [a, b] with a bare-day b running to the start of the next day": drillWindowBounds lowers the strategies' own { $gte, $lte } pair through normalizeDateRangeWindow under an isDatetimeColumn that answers true for every member, the shared whole-day rule, which rewrites a bare-day $lte to $lt the next day. On a datetime column that is exactly what the query compiles (declaredDatetimeLowering answers datetime → true). On a date column the query keeps an inclusive less-than-or-equal on b, and the drill's exclusive less-than on the next day, compared as YYYY-MM-DD text, is order-equivalent, as the module note says and the explicit [2026-07-15, 2026-09-14] date-field test pins (lt: '2026-09-15'). Right. The docblock is careful to claim instants "at midnight in the reference timezone" only for a bucket's bounds; an explicit bare-day window on datetime is cut at UTC midnight (ADR-0053 D-D), which the test and the Acceptance note record. Right.

  5. "A window with no exact half-open spelling gets no entry: an explicit end that is an instant, or one on the last supported day": an instant $lte is never widened by the lowering, so a lte leaf survives and drillWindowBounds returns undefined; on the last supported day the lowering drops the end, so no lt leaf exists and it returns undefined. Both pinned (['2026-07-01T00:00:00Z', '2026-09-30T12:00:00Z'], ['2026-07-01', '9999-12-31'] → no drillRanges). The drill stays the superset it was. Right.

  6. "Set only on a queryDataset answer that has rows and … groups by at least one type: 'date' dimension with a field and a granularity, or carries a dateRange on a dimension with a field": the guard is (rangeDims.length || windowDims.length) && result.rows.length, and windowDims requires d?.field. Right. The sentence "Such a dimension is left out when its source field is neither date nor datetime (or its type is unknown) and the reference timezone is not UTC" now governs both bucket and window through the one shared instantOf (returns undefined exactly in that case). Right, and pinned by the unknown-type Shanghai control.

  7. "A drill list filters field by every entry, so it lists the records the row counted": true of the measured consumer (buildDatasetDrillFilter), and the real-engine half of the new test runs that very filter shape through engine.find on both strategies and asserts the listed ids number exactly the row's count. Right. One ui-visible consequence the PR body states and I confirm from ui/packages/data-objectstack/src/index.ts: canDrill is true when the answer names an object and either has drill dimensions or a non-empty drillRanges, so a windowed dataset widget with no grouping (one measure row) now carries a drillRanges entry and becomes drillable, listing the window's records. That is the widening the Clause-② line declares, and it is the intended reading (the row counted exactly those records); named here so the ui seat is not surprised by a new drill affordance on ungrouped windowed widgets.

  8. A row whose bucket value is null gets no bucket entry; with a window on the same field, same.length === 0, so the window becomes that row's own entry under the window dimension's name. Consistent with the docblock (the window filters every row). Right.

  9. No new key: the seat lean in the claim (drillWindow would be a protocol addition going back to triage) and the triage ruling (one window rule, no second resolver in the browser) are both honoured — the diff adds no key, no export in @objectstack/spec, and drill-window.ts is not re-exported from @objectstack/service-analytics's index, so the service package's public surface is unchanged too. Right.

  10. The existing spec tests (src/api/analytics.test.ts, src/contracts/analytics-service.test.ts) need no change: shape and @ts-expect-error pins are untouched by a wording-only edit. Right.

Nothing in ① is wrong. The one doubtful point is not a contract claim but a construction detail, carried in ③ (midnight race).

② Semver level

.changeset/27074-dataset-drill-window.md: @objectstack/service-analytics: patch, @objectstack/spec: patch. Matches what the diff publishes: a bug fix in a released package (patch, never none) and a .describe() / docblock change in spec that ships in the generated JSON schema and reference (patch). No skip-changeset; Check Changeset is green.

Clause-②: yes (widening) on both the PR body and the changeset body — an existing optional answer key carries entries in a case (a windowed, possibly ungrouped selection) where it carried none, and existing entries can be narrower (an edge bucket cut to the window). No reader is refused anything it could send before. Metadata-protocol: none — nothing an author can write changes (drillRanges is a response sidecar; no authorable key, value or type moves), so patch is the level the kind alone dictates; no Skills-docs: line is owed (kind none, no (narrowing) arm). Right.

③ Boundary flags

open_questions: [] in the os-dev-report. Dev flags from the PR's Acceptance notes and the report's out_of_scope_findings, each answered or escalated:

  • Escalated — a dateRange preset on a date-typed field east of UTC filters the wrong calendar days (class a, public door POST /api/v1/analytics/dataset/query, measured on SQLite on both strategies: this_quarter in Asia/Shanghai counts 2026-06-30 and drops 2026-09-30). Not introduced by this diff, and the drill correctly reports the same storage-form window so list and count agree (the test pins parity only, null expected ids, honestly). Prime Directive chore: version packages #10: a reproducible defect on a public door gets its own card. No card number is on analytics: a dataset query with a timeDimensions window returns no drill bounds for it, so a report drill-down lists records outside the picked period #27074 yet; the adopting seat files it from the report's dedupe words. This review does not block on it.
  • Answered — no exact spelling, no entry (instant end, 9999-12-31 end): the superset the drill was before, stated in both spec texts; no measured producer sends either shape.
  • Answered — an explicit bare-day window on datetime is cut at UTC midnight under any reference zone: that is what the query filters by (ADR-0053 D-D); whether the explicit arm should follow the reference zone is ADR-0053's recorded open seam, not this card.
  • Answered — totals rows carry no range companion: drillRawTotals is equality-only, as before this diff; no ui path reads it today. Pre-existing gap, not widened here.
  • Answered, residual noted — midnight race: the strategy resolves the preset with its own new Date() and the drill block with another (filterTokenContextFrom(context, new Date()) and the preset resolver's clock). The "one window" guarantee is by construction (same functions, same zone), not by sharing one instant, so a request straddling the reference zone's midnight could report a window one day off the one filtered. Acceptable for this card (the strategies already have the same race between the two of them); the clean closure is to thread the strategy's resolved window out to the drill block, which is a follow-up if ever observed.
  • Answered — overlap: fix(service-analytics): a date-dimension series answers every bucket of its window, the empty ones included #27109 landed mid-flight and is merged into the head (the head is a merge of origin/main, every suite re-ran there); feat(spec): a dataset measure declares its polarity, so a lower-is-better KPI draws a rising delta as bad news #26739 (open) edits other hunks of the same three files and carries the rebase.
  • Observed — Lint UI is skipped on the head: a paths-filter skip (no ui/ path in the diff), not a red; the queue accepted it and the ui type-check and build ran green against this spec.

Implemented-by: claude/project-thread-m4ipw2
Reviewed-by: session_0186vfjHHMsRXC9EUT9hTFMo

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: b807ecc5fa2574e3dd9d2c6bdc1d9c41d29f157c
Local-runs: none
Scope: packages/spec/src/api/analytics.zod.ts, packages/spec/src/contracts/analytics-service.ts + packages/services/service-analytics/src/drill-window.ts, packages/services/service-analytics/src/analytics-service.ts (the drillRanges block, L2911–2985), packages/services/service-analytics/src/dataset-executor.ts (lowerDatasetCompareDateRange L649, token resolution L130, timezone precedence L1611, timeDimensions merge L1667–1706), packages/services/service-analytics/src/strategies/filter-normalizer.ts (normalizeDateRangeWindow, declaredDatetimeLowering), packages/services/service-analytics/src/strategies/native-sql-strategy.ts L1405–1462, packages/services/service-analytics/src/strategies/objectql-strategy.ts L322–334 and L2071–2113, packages/spec/src/data/filter-lowering.ts (the whole-day / last-day upper-bound rule), packages/spec/src/data/calendar-day.ts, packages/core/src/utils/temporal-storage-form.ts, packages/core/src/utils/analytics-date-range.ts, ui/packages/core/src/utils/dataset-format.ts (buildDatasetDrillFilter, the first consumer), ui/packages/plugin-dashboard/src/DatasetWidget.tsx (L848–933, L1282–1299), ui/packages/plugin-report/src/DatasetReportRenderer.tsx (L806–815, L1635–1644), content/docs/references/api/analytics.mdx (generated from the .describe), packages/services/service-analytics/src/tests/dataset-drill-window.test.ts

Inputs read: card #27074 body and all three comments (triage 6461017000, claim 6462057514, os-dev-report 6464984887); PR #27169 body, file list, all PR comments (docs-drift, seat ACCEPT 6465413400, triage claim 6465890103); net diff vs main; the 51 check-runs on the head (42 success, 9 skipped — the UI, dogfood, temporal-conformance and opt-in families, each skipped by its own trigger; none pending, none red); rulings 6408567749 (#25873 B), 6431004737 (#26369 B) and the gap rule #27062; follow-up #27172. Every file above read at prr-27169 with git show; the dispatching seat's ACCEPT was treated as a claim and re-derived.

① Derived judgments

  1. Request accept set — unchanged. Right. No request schema is touched; selection.timeDimensions[].dateRange was already accepted and already filtered the query. The PR adds no key, value or type an author or caller can write.
  2. Answer shape — unchanged. Right. AnalyticsResultResponseSchema.drillRanges stays z.array(z.record(z.string(), z.object({ field, gte, lt })))).optional(); the AnalyticsResult.drillRanges TS type is byte-identical. Only .describe text and the docblock move. The spec tests that pin the type (analytics-service.test.ts L152–175, analytics.test.ts L354–378) are untouched and green.
  3. Answer SET — widens. Right, and the Clause-②: yes (widening) arm is the correct one. Three new emissions, all read off analytics-service.ts L2932–2975: (a) drillRanges is now set on an answer that groups by no date bucket at all, when a windowed dimension has a field whose type is datetime / date (or any type under UTC); (b) an entry may be keyed by a dimension NAME that is not in the grouping; (c) an edge bucket's EXISTING entry carries tighter bounds when a window sits on the same field (if (w.gte > r.gte) r.gte = w.gte; if (w.lt < r.lt) r.lt = w.lt). (a) and (b) are additive; (c) changes a value in a situation that already produced one, and the old value is the defect the card names (a month bucket cut by last_30_days drilled the whole month). Every reader in the tree hands entries to buildDatasetDrillFilter, which writes { [r.field]: { $gte, $lt } } per Object.values(rawRanges) whatever the key (dataset-format.ts L661–665); no reader uses an entry as a bucket's calendar span. So no consumer loses a capability: not a narrowing.
  4. Metadata-protocol: none — right. AGENTS.md step 3 defines the kind as "the change to what a metadata author can write". DatasetSchema, the report/dashboard dateRange vocabulary and every authorable key are untouched; drillRanges is a response payload. Not semantic (no authored meaning or default moves) and not added (no new key — the triage/claim lean against drillWindow is honoured: git grep drillWindow on the head finds only the function drillWindowBounds).
  5. Consumer reach — two doors, one unnamed. Right in the tree; the body under-reports. The report page (PR feat(plugin-report): the report page reads dateRange — a period chip and one timeDimensions window #27070) sends timeDimensions[].dateRange and is the card's path; the drill now narrows to the picked period with no ui edit. The second door: DatasetWidget lowers its date window into timeDimensions only under compareTo (L848–853, L933), so a COMPARED dashboard tile also gets window entries — and a compared tile with no grouped dimension (a KPI) becomes drillable for the first time (canDrill = … || !!drillRanges?.length, L1286), to exactly the records it counted. Correct behaviour, but "Nothing under ui/ … changes" in the PR body describes the code, not the surface; the maintainer summary should name this reach.
  6. Generated and prose copies — complete. Right. content/docs/references/api/analytics.mdx L161 is the new .describe verbatim; git grep "Drill-through sidecar for time buckets" on the head hits only CHANGELOG.md files (release-owned, correctly untouched). The two hand-written pages the drift check named (client-sdk.mdx, data-api.mdx) anchor on the AnalyticsResult symbol and say nothing about drillRanges; no page in content/docs, skills/ or .claude/skills restates the old "buckets only" invariant.
  7. Window-equals-filter — verified in the tree, not just the body. The drill resolves a preset through lowerDatasetCompareDateRange → resolveAnalyticsDateRangeString(str, { timezone }), exactly the call both strategies make (native L1423, ObjectQL L2084); an explicit pair through explicitDateRangeWindow, the same (native L1432, ObjectQL L2104); it rebuilds the same { $gte, $lt } (preset, endExclusive: true) / { $gte, $lte } (explicit) pair (native L1446, ObjectQL L2094–2096, L2109); the timezone precedence is the executor's verbatim (selection.timezone ?? context.timezone ?? 'UTC', executor L1611 vs rangeTz L2911); tokens resolve through filterTokenContextFrom + resolveFilterTokens, the executor's own (L130–137); and the executor keeps every selection timeDimensions entry in the cube query (L1667–1706, resolvedTimeDims maps all of selTimeDims), so the drill never reports a window the query did not apply.

② Semver level

  • Changeset .changeset/27074-dataset-drill-window.md: '@objectstack/service-analytics': patch, '@objectstack/spec': patch; both declaration lines line-initial in the body, identical to the PR body's. Check Changeset on the head: success.
  • patch on @objectstack/spec is justified by the rule, not by "wording only". AGENTS.md Post-Task Checklist step 3: "The level follows the kind alone: a kind takes at least minor on a package the diff moves, none takes patch; a non-protocol change, widening or breaking, ships patch." The kind is none (① item 4), so patch is the ONLY level the rule allows for this widening. A minor here would have been the error.
  • Clause-②: yes (widening): matches what the diff publishes (① item 3). No (narrowing) arm, no BREAKING banner, no ADR-0087 marker owed.
  • Skills-docs: line — not owed, correctly absent. Ruling 6431004737 ([Decision] pm-dispatch: which PRs owe a Skills-docs: line (the #26184 audit Part B rule draft) — protocol-kind only, protocol-kind plus narrowing, or not adopted #26369, letter B): the line is owed when Metadata-protocol: is added / semantic / narrowed, OR when Clause-②: carries (narrowing). This PR is none + (widening): neither trigger. The landed rule text agrees (SKILL.md:313, review-checklist.md:18, os-dev.md:279: 「种类非 none 或带 (narrowing)」). The seat's ACCEPT check 1 did not misread this.
  • No skip-changeset; the diff publishes from two released packages.

③ Boundary flags

Dev flags (PR body Acceptance notes) and the report's open_questions: [] / out_of_scope_findings[0]:

  1. Preset on a date field east of UTC counts the wrong days — escalated as analytics: a preset dateRange on a date-typed field counts the wrong calendar days east of UTC (Shanghai this_quarter counts Jun 30, drops Sep 30) #27172 (open, bug pm:queue area:reports domain:server, filed by the seat). Verified the mechanism in the tree: the preset's zone-midnight instants (2026-06-30T16:00:00.000Z) are read on a date column through temporalStorageForm(v, 'date') → leading UTC day 2026-06-30. The drill spells the same form (drill-window.ts L96–101), so drill and count agree on this head; the real-engine case for it pins parity only (null expectation) and says why. Answered: right to land, right to file.
  2. No exact spelling → no entry (instant end; 9999-12-31 end). Read in filter-lowering.ts L296–343: an instant end has no nextUtcCalendarDay so stays $lte; the last supported day becomes $null: false; drillWindowBounds refuses any leaf that is not exactly one gte + one lt (L85–90) and returns undefined → superset, as before. The spec text states both. Answered.
  3. Explicit bare-day window on datetime cut at UTC midnight (ADR-0053 D-D's recorded seam) — the drill reports what the query filtered (2026-10-01T00:00:00.000Z); the real-engine explicit case expects d5 in lost for that reason. Not this card; answered.
  4. Totals rows carry no range companion — pre-existing for buckets (drillRawTotals is equality-only since Analytics: totals/subtotal rows have no raw-value drill sidecar (drillRawRows covers result.rows only) #3214); no ui path reads drillRawTotals. Unchanged scope; answered.
  5. Midnight race (the drill's own new Date() vs each strategy's) — the pre-existing pattern between the two strategies themselves; a request straddling the zone's midnight could differ by a day. Accepted as before; answered.
  6. Overlap — fix(service-analytics): a date-dimension series answers every bucket of its window, the empty ones included #27109 merged in (head is the merge); feat(spec): a dataset measure declares its polarity, so a lower-is-better KPI draws a rising delta as bad news #26739 touches other hunks of the same three files; seat reports git merge-tree clean. Answered.

Reviewer's own flags:

  1. "One lowering" is one SET OF FUNCTIONS at a parallel call site, not the strategies' call site. drill-window.ts recomposes lowerDatasetCompareDateRange → normalizeDateRangeWindow → temporalStorageForm with a forced-datetime reader (WHOLE_DAY, L63), while the native strategy lowers with declaredDatetimeLowering(…, 'type-blind') and the ObjectQL strategy hands bounds as written to the engine's seam. The forced reader is sound: on a date column lt next-day is order-equivalent to lte day (calendar-day.ts L26–28), and the drill then spells the day form. No second date arithmetic exists, so the triage rule "no second date resolver" holds. The guard against drift is in the tree: the unit case reads the very { $gte, $lt } the strategy handed the engine (test L140) and the real-engine cases run the ui's filter shape on both strategies. analytics: a preset dateRange on a date-typed field counts the wrong calendar days east of UTC (Shanghai this_quarter counts Jun 30, drops Sep 30) #27172's body already binds its fix to "keep drill and count on one lowering". Answered; no change asked.
  2. A dateRange on a dimension that is not type: 'date' — the window loop (L2941–2949) has no d.type === 'date' guard, and neither strategy has one (both apply the window to any td.dimension), so drill and count stay in step whatever the dimension type; the only cost is the spec sentence that opens "date dimension name" (④ S1). Answered.

④ Design judgment

packages/spec/src/api/analytics.zod.ts — judged against triage ruling 6461017000 ("the dataset answer returns the drill bounds it used (instants), and the drill list filters by exactly those") and its first consumer buildDatasetDrillFilter. Every sentence of the new .describe checked against the live tree: "a timeDimensions[].dateRange window gets the window the query filtered by on every row" — L2956–2975 and ① item 7; "or narrows the bucket entry on the same field" — merge by r.field === w.field, never by name (L2969–2974); "Bounds are YYYY-MM-DD … or ISO-8601 instants when the source field is datetime (a bucket's at midnight in the reference timezone)" — the clause is careful: a window's instants are zone-midnight for a preset and UTC-midnight for an explicit day, and the text claims midnight only for buckets; "a window whose end is an instant or the last supported day gets none" — ③ item 2; "Set only on … or carries a dateRange on a dimension with a field" — the unknown-type-under-non-UTC omission is inherited from instantOf and stated in the docblock. Tried to break it: two windows on one field → first adds, second narrows (intersection); a null-bucket row with a window on the same field → the window entry alone, as the text says; narrowing by text comparison → both sides are one form per field (toISOString fixed width, or YYYY-MM-DD), so the comparison is chronological; an explicit UTC-midnight window against a Shanghai-midnight bucket → both real instants, the result is what the query filtered; a window and a bucket under the same key → same is non-empty, so the bucket is narrowed, never overwritten. Every failure mode of the new code falls to "no entry" (superset); none produces a drill narrower than the count. Consumer shape: DatasetDrillRange in the ui declares gte/lt as unknown and the filter is { $gte, $lt } per field — exactly the pair the real-engine test runs through engine.find.

  • S1 (should-fix): the describe (and the docblock) opens "date dimension name to the half-open range", but the window arm emits for ANY selected dimension with a field (no type: 'date' guard, ③ item 8). Either say "dimension name" in the window clause or guard the loop on d.type === 'date' (a guard only widens the drill to the superset on a non-date dimension, which the query still filters). Wording-level; no consumer in the tree reads the key.

packages/spec/src/contracts/analytics-service.ts — the AnalyticsResult.drillRanges docblock, judged against the same ruling and consumer. "A drill list filters field by every entry, so it lists the records the row counted" is the consumer's actual code, and the real-engine suite is that sentence run on both strategies. "a preset as resolved in the reference timezone, an explicit [a, b] with a bare-day b running to the start of the next day" — the former is resolveAnalyticsDateRangeString with the executor's timezone (① item 7), the latter is filter-lowering.ts L306–309 through the forced reader. "the bucket's entry is narrowed … and the window gets no entry of its own" — L2970. The "Set only" paragraph keeps the unknown-type omission sentence, which now covers the window arm too (instantOf is shared). Tried to break the type: nothing changes, so nothing to break; the @ts-expect-error pin at test L174 still holds both bounds required.

Neither finding is a must-fix: the tree delivers the ruling (drill bounds = the query's own, one set of functions, no browser resolver, no new key), the spec text describes the tree, and the suite pins the equality between the two on a real engine.

Implemented-by: claude/project-thread-m4ipw2
Reviewed-by: session_01AavokzJ5DndAwitDXvKy4U

VERDICT: PASS


Generated by Claude Code

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/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

analytics: a dataset query with a timeDimensions window returns no drill bounds for it, so a report drill-down lists records outside the picked period

2 participants