Skip to content

fix(driver-memory): an analytics time dimension buckets by its declared granularity (#16178) - #17206

Merged
os-sam merged 10 commits into
mainfrom
claude/issue-16178-analytics-granularity-buckets
Sep 10, 2026
Merged

fix(driver-memory): an analytics time dimension buckets by its declared granularity (#16178)#17206
os-sam merged 10 commits into
mainfrom
claude/issue-16178-analytics-granularity-buckets

Conversation

@claude

@claude claude Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Fixes #16178

Clause-②: yes

AnalyticsQuery.timeDimensions[].granularity was accepted by driver-memory's analytics face and never read, so a time dimension answered one group per distinct timestamp — one bar per row in a "new accounts by month" chart, under an ordinary 200 with no warning anywhere.

Reproduction, re-measured on this branch's own base

The card's anchors had moved (PR #17015 rewrote 198 lines of the file), so every anchor here was re-derived by symbol and the card's own controls re-run on origin/main fd5cff209:

control card (1328902ce) this branch's base (fd5cff209)
granularity in memory-analytics.ts 0 0 — the defect holds
dateRange 4 7 — drifted
timeDimensions 1 4 — drifted

Measured through the public entry against the built package, two rows on one UTC calendar day (2026-09-06T01:00:00Z, 2026-09-06T23:00:00Z):

query before after
granularity: 'day' 2 groups, keyed on the raw instants 1 group, 2026-09-06
no granularity — the control 2 groups 2 groups, unchanged
granularity: 'hour' 2 groups, silently NOT_IMPLEMENTED / 501

The emitted pipeline was byte-identical across all three before the fix. That is the whole finding: the request was accepted, nothing warned, and the key was inert.

⚠️ This PR is wider than the dispatch order's scope fence — deliberately, and here is why

The order fenced the change to packages/drivers/driver-memory/src/memory-analytics.ts plus its tests and a changeset. This PR also edits packages/core and packages/objectql. That is not a drift; the fence as written has no executable route inside it.

The recorded ruling on the card (director seat, decision batch #91, comment 5583991002) ruled Q1: A and refused the alternatives by name:

  • B — a driver depending on objectql for the labeller;
  • C — importing service-analytics' bucketDate (the non-canonical week label);
  • D — a hand copy of the rule in the driver.

Inside the fence, the only way to bucket a week is to hand-copy the ISO-week rule into driver-memory, which is exactly the refused D and exactly the divergence checkDateBucketParity exists to catch. So the fence, taken literally, mandates a ruled-out route. The ruling's own execution note says "one PR spanning core, objectql, driver-memory", and an earlier claim on the card enumerated that same surface.

What the fence's one explicit prohibition asked for is honoured in full: packages/spec/** is untouched. granularity is already declared there, the spec-side narrowing of TimeUpdateInterval is filed separately as issue #17296, a domain:spec question under ADR-0049, and nothing here crosses that lane. driver-memory already declared @objectstack/core as a runtime dependency, so route A adds no dependency edge.

One ruled item is deliberately deferred and not silently dropped: the ruling also asked for ADR-0053's implementation-map row to gain memory-analytics.ts in this PR. docs/adr/** is a governed surface, and adding it would make this diff governed whole — permanently removing it from the merge queue and reserving the landing to a GOVERNED_APPROVERS review, a landing-path escalation the dispatch order did not contemplate. It is a one-line documentation row that lands trivially on its own. Say the word and it goes in this PR instead.

What it does

  • One forward labeller, in @objectstack/core. bucketDateKey(value, granularity, timezone) now sits beside the inverse bucketKeyToCalendarRange, the BucketGranularity type and the calendarPartsInTzOrUtc primitive it builds on — all of which were already there. BUCKET_GRANULARITIES and isBucketGranularity name the five granularities that have a canonical key, so a face that must refuse the other three quotes the accepted set instead of hand-listing it. The private isoWeekLabelUtc and the new labeller now share one statement of the week rule rather than two.
  • @objectstack/objectql's bucketDateValue is a delegate — export name, signature and answers unchanged. The unreachable default arm's echo is preserved in bucketDateKey so an off-type JS caller gets the answer it always got.
  • driver-memory folds by granularity before its $group. The pipeline is cut at that stage: the $match half still runs in the driver (where the rows and the tenancy guard are), the bucket keys are written onto the selected rows, and the grouping half runs over those. mingo has no expression that produces 2026-Q3 or 2026-W36, and building one out of $isoWeek and $concat would be the second dialect this repair exists to prevent.
  • The bucket travels under a synthetic field, never over the row's own. One member can be both a group key and a measure's aggregand; folding created_at in place would leave max(created_at) ranking bucket labels. Pinned.
  • second / minute / hour are refused at compile with NOT_IMPLEMENTED / 501 — the class refusePerAggregationFilter already uses for the same reason: the query is spelled correctly, the spec declares the value, and it is this backend that compiles nothing for it. Asserted on code and status, never on message text.

How bucketing interacts with the timezone repair (#16042)

Measured, not assumed. The fold takes AnalyticsQuery.timezone — the same reference zone parseDateRangeString resolves a dateRange preset against — so the window that selects the rows and the bucket that folds them agree on where a calendar day starts. The same two rows:

timezone groups labels
absent 1 2026-09-06
UTC 1 2026-09-06
America/New_York 2 2026-09-05, 2026-09-06
Asia/Tokyo 2 2026-09-06, 2026-09-07

⇒ UTC is demonstrably not the only case, and the dateRange window is not regressed: both keys on one entry still select by the window's own published semantics and fold what survives. The preset arm and the granularity resolve against one query.timezone.

Coverage

package new cells suite on the final head
@objectstack/driver-memory 14 (new file) 49 files / 1194 tests pass
@objectstack/core 6 51 files / 1263 tests pass
@objectstack/objectql 2 288 files / 4857 tests pass

typecheck clean on all three. All figures taken at f3961a58f7, after the origin/main merge and a full workspace build.

Ablation — both legs, proven on disk and through the built artifact

Leg A — restore the defect in the driver. Keying the $group on the raw field path again turns 9 of the 14 new cells red. The 5 that stay green are exactly the ones that should: the no-granularity control and the four refusal cells. A blanket failure would have proved much less than this discrimination does.

Leg B — mutate the labeller in core, through dist. A marker injected into bucketDateKey's week branch, @objectstack/core rebuilt, and ablation-dist-preflight confirming the marker reached 2 built files — then driver-memory goes red on exactly its week cell and objectql on 3. That is the proof that the driver consumes core's built labeller rather than a stale copy, which no source-only ablation could establish.

Both restore legs verified by git diff HEAD empty and git hash-object equal to the HEAD blob; leg B's restore additionally re-verified with ablation-dist-preflight --absent, whole-tree clean. Neither is a permanent test file.

Semver

minor on all three, BREAKING banner, ADR-0087 not-required (no-migration-prescription).

  • @objectstack/coreminor. Three new public-entry exports. Unambiguous under the house rules.
  • @objectstack/driver-memoryminor. Two events, and neither is a patch. The house rule "repairing an implementation that silently violated its own already-published declared type" would grade the bucketing half patch, but that rule does not reach the second half: a value the verb accepted is now refused, which is an accept-set narrowing, not a widening and not an envelope on an existing refusal. And the bucketing half changes the answers an accepted request returns. "Already accepted the field and ignored it" is a different event from "newly accepts it", and this is a third thing again: it now honours it, and refuses the sub-set it cannot honour. minor with the banner; ⛔ major refused per the order.
  • @objectstack/objectqlminor. Its own answers do not move by a byte — pinned across granularity, timezone and input form rather than asserted — so on the level axis alone this is a patch. It is graded minor under the launch-window lockstep convention its two sibling changesets in this cluster invoke by name ("during the window the bump level is not the carrier, this banner and the disposition above are"), and because the ruling graded all three that way.

The card's priority:p2 played no part in any of this; they are unrelated axes.

Gates

On head f3961a58f7 (pre-patch-round): 61 families derived by scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack (no path argument) and reconciled with --ran: 61 derived, 61 run, 0 NOT-MEASURED, 0 UNRUN. All 61 exit 0 on that head.

⚠️ That full sweep was NOT repeated on the patch-round head ba9e8206f6 — see the patch round's own Verification block below for what was re-run locally and what is declared to CI.

Two needed a prerequisite before they meant anything and were not reported as passes until they had it: check:dual-build-cjs-loads and check:type-check-debt both exited 3PREREQUISITE NOT MET — against a partially built tree; a full pnpm build was run and both then exited 0. check-engine-split-ratio refused on a shallow clone; the clone was deepened as the gate's own remedy line prescribes, and it then measured 98.1%, a repo-wide metric this diff cannot move.

One gate found a real defect in the first draft and it is fixed: check:doc-authoring reds on an internal tracker id inside customer-facing prose, and the refusal message carried one. The anchor moved to the function's doc comment, where a reader who can resolve it is already looking.

Acceptance notes

Two boundaries measured on this branch, neither filed, both put in front of the contract review rather than decided here:

  1. The explicit-array dateRange arm and granularity answer in different frames. (Seat ruled A; the carve-out is now stated in the changeset — F5 below.) With timezone: 'America/New_York', dateRange: ['2026-09-05', '2026-09-05'] and granularity: 'day', a row at 2026-09-05T02:00:00Z is selected by the window (which compares instants, per driver-memory analytics dateRange upper bound is INCLUSIVE for a full-timestamp end, so the first instant of tomorrow counts as today #16179's explicit ruling that a caller's window is never narrowed) and then labelled 2026-09-04 (the NY calendar day, per ADR-0053). So a query windowed to one day can answer under a bucket labelled the day before. Both halves are separately ruled; the seam between them is newly visible because the fold is now timezone-aware. This is a contract question for the Clause-② review, not a bug to fix unilaterally.

  2. A granularity on a member that is not in dimensions is still inert. RESOLVED in the patch round below (F1). The review ruled this in-fence and pointed at the SQL/ObjectQL face's recorded position (projectedDimensions, Analytics: a timeDimensions-bucketed query drops the bucket label from rows and fields — trend charts get counts with no x-axis #4033/analytics: 只用来限定「日期区间」的 timeDimension 被补上默认 dateGranularity,于是网格被静默按月拆分 —— 「按 Owner 统计」加个日期筛选就变成「按 Owner × 月」 #5688), which already groups and projects such an entry. It is now implemented and pinned with its control; the note is kept struck rather than deleted so the review's reasoning stays readable against what changed.

Patch round — answering contract review 5610525481 (REWORK)

Head ba9e8206f6, three commits on top of f3961a58f7. Nothing rebased, amended or force-pushed.

disposition
F1 HIGH, in-fence Fixed. A granular timeDimensions entry is now a group key, a projected column and a fields entry, deduped against dimensions on the resolved member path.
F2 MEDIUM Fixed. #17296 exists (open, filed 04:24Z); the changeset and this body name it instead of promising it.
F3 MEDIUM NOT taken — premise falsified. See below.
F4 LOW Taken. An undeclared granularity answers INVALID_QUERY / 400 instead of a 501 asserting the spec declared it.
F5 LOW Taken. The changeset now carves out the explicit-array arm.
F6 LOW Not taken here — carded, as the verdict itself prescribes.

F1 — the trend shape

$group keyed on query.dimensions alone, so {measures, timeDimensions: [{dimension, granularity}]} with no dimensions answered one total (_id: null). The rule and its exception are taken from the SQL/ObjectQL face rather than invented: every granular entry not also listed groups and projects (objectql-strategy.ts :163-167), one set feeds grouping, row mapping and field metadata alike (projectedDimensions :1889-1893), and a dateRange-only entry is a predicate that is not projected (#5688).

Pinned with the control the seat named, on the two-rows-one-UTC-day fixture:

cell before after
no dimensions, granularity: 'day' {'events.count': 2} — one total, no time column {'events.createdAt': '2026-09-06', 'events.count': 2}
its fields list ['events.count'] ['events.createdAt', 'events.count'], identical to the shape that DOES list the member
control: dateRange-only entry one total, fields = ['events.count'] unchanged — still not projected
control: member listed both ways one column unchanged — one column, deduped on the resolved path

Measured red before the source change and green after: the two new positive cells failed against the unmodified pre-patch source (expected { 'events.count': 2 } to match object { …(2) }), and both controls passed before and after, which is what makes them controls.

F3 — not taken, and why

The prescribed fix was err.refusal = true. Its two cited declaration sites are packages/spec/src/api/contract.zod.ts and errors.zod.ts (the verdict spells them under data/), and read there the premise inverts:

  • There is no refusal key in the ADR-0112 error vocabulary. EnhancedApiErrorSchema declares code, message, userMessage, category, httpStatus, retryable, retryStrategy, retryAfterSeconds, details, fields, timestamp, requestId, traceId, documentation, helpText — and no refusal. Setting one would be metadata the runtime silently discards, which is the class this PR exists to remove.
  • The declared carrier is userMessage, and it is declared as the opposite shape. contract.zod.ts:89 and its doc block: "Producer-marked user-facing refusal text… A field carrying the text, not a boolean beside messagePlatform/driver code never sets it." driver-memory is driver code.
  • The withholding is the declared behaviour, not a defect. classifiedRefusalAnswer (packages/rest/src/error-response.ts:2328) states it in as many words: "What it deliberately refuses to answer: a 5xx, declared or resolved. A server fault is not a refusal addressed to the caller." A declared 5xx never reaches it, by design (/analytics/query 仍把 RLS 策略字段名回显给调用方 —— read-scope 拒收的泄漏在姐妹面上没堵,#5367 只堵了 dataset 路由 #5811).

So the one-line change would have been inert and undeclared. Recorded here rather than silently skipped; if the seat still wants the guidance to reach the wire, that is the 400-vs-501 question F4 opens, not a boolean.

F4 — out-of-vocabulary is a 400

501 is a claim about this backend, and the refusal's sentence asserted "@objectstack/spec's TimeUpdateInterval declares the value" — false for 'fortnight', which a caller can send past the schema door (POST /analytics/dataset/query types selection.timeDimensions from AnalyticsQuery and never Zod-parses them, the reachability analyticsDateRangeUnrecognizedError already records). The vocabulary is now checked first.

⚠️ One judgement for the seat. The 400 arm answers the general StandardErrorCode.INVALID_QUERY, not a dedicated ANALYTICS_GRANULARITY_UNRECOGNIZED — the shape its dateRange sibling uses. A dedicated code must be registered in packages/spec's error-code-ledger.zod.ts, which this round is forbidden to touch and which is a contract decision besides. The choice is recorded in the function's doc comment so it is visible when that card is written; ⛔ nothing published moves either way, because the whole refusal is new in this PR.

Semver, re-measured (not assumed)

git diff HEAD -- '*.ts' | grep '^+export ' over the patch round: no new export. No barrel touched. MemoryAnalyticsService.query's signature is unchanged and TimeBucket is a non-exported internal interface. So the grades stand: core minor (the three exports it already added), objectql minor (delegate), driver-memory minor + banner — F1's trend-shape change is another instance of the banner's existing "an accepted request now answers differently", not a new kind of break. Clause-②: yes already covers this; nothing here enlarges the published surface beyond what it declares.

Verification on ba9e8206f6

  • pnpm --filter @objectstack/driver-memory test49 files, 1200 tests, 0 failures (VERDICT command-exit 0, under scripts/pm/os-verify-lock.sh).
  • pnpm --filter @objectstack/driver-memory typecheck — exit 0. Population measured, not assumed: tsc --listFiles puts the new test file in the program (1 hit of 728 files).
  • pnpm --filter '@objectstack/driver-memory^...' build — exit 0.
  • Gate families implicated by this diff, each read from its own printed verdict: check:nul-bytes, check:error-code-casing, check:engine-double-contract, check:empty-changeset, check:test-source-alias, check:type-check-coverage, check:published-filesall exit 0.
  • Ablation, both legs, restored and proven. Deleting the F1 group-key line turns its row-content cell red; neutralising the F4 vocabulary check turns its 400 cell red. On-disk mutation proved by fixed-string marker counts (1 → 0) before either run, restore by git checkout HEAD -- PATH with git diff HEAD empty and git hash-object matching the HEAD blob for both files. ⚠️ The F1 leg deletes only the group key and leaves the projection and fields loops standing, so 1 of the 4 selected cells reds rather than all four — an honest partial isolation; the whole-change before/after is the red-then-green measurement above.
  • Declared to CI, deliberately narrowed. dispatch-gates.mjs derives 61 commands for this change set; the seven above are the ones this diff implicates and the rest are CI's farm. eslint was narrowed to the three changed source files (--format json: 3 files, 0 errors, 0 warnings) and the narrowing is a measurement, not a guess: this repo's flat config enables no type-aware linting for any file — calculateConfigForFile answers parserOptions.project: null and projectService: null, and eslint.config.mjs records the same with a positive control — so this diff cannot move the verdict on any untouched file.
  • ⚠️ Stale-tree caveat, stated rather than hidden. dispatch-gates.mjs warns this branch is ≥45 commits behind origin/main and that 15 files the derivation reads changed in that range. The list above is therefore a lower bound; re-deriving would need a fresh merge, which this patch round was told not to perform. CI on this head is the authority.

Patch round by Claude Code — session session_01XTBcV7zZHmokdyQgXjbyEU.

⛔ Left on this PR as required: the needs:contract-review label, draft status, and no ready flip, enqueue, auto-merge or merge.


Generated by Claude Code

…d granularity

`AnalyticsQuery.timeDimensions[].granularity` was accepted and never read, so a
time dimension answered one group per distinct timestamp — one bar per row in a
"new accounts by month" chart, under an ordinary 200 with no warning.

The forward bucket labeller is hoisted into `@objectstack/core` as
`bucketDateKey`, beside the inverse `bucketKeyToCalendarRange` and the
`calendarPartsInTzOrUtc` primitive it already builds on. `@objectstack/objectql`'s
`bucketDateValue` becomes a thin delegate with its export name and signature
unchanged, so the two in-memory bucketing paths cannot label one instant
differently. `driver-memory` folds by granularity between the `$match` half of
its pipeline and its `$group`.

The bucket key travels under a synthetic field rather than overwriting the row's
own, so a member that is both a group key and a measure's aggregand still ranks
instants in `max()` while grouping on the label.

`second` / `minute` / `hour` are refused at compile with NOT_IMPLEMENTED/501 —
the canonical key vocabulary defines no label for a sub-day bucket, and passing
one through is the same defect under a new name.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU
…e and its refusal

The card's own measurement, with the control that makes it one: two rows on a
single UTC day fold to one group under `granularity: 'day'` and stay two groups
when nothing asks for a bucket.

Beside it: the canonical output vocabulary for all five granularities (the week
label is `YYYY-Www`), the reference-timezone fold across three zones, the
`dateRange` window left undisturbed on the same entry, the NOT_IMPLEMENTED/501
refusal for the three sub-day intervals asserted on code and status, and the
measure-over-the-same-member cell that the synthetic bucket field exists for.

`@objectstack/core` gains cells for the labeller itself; `@objectstack/objectql`
gains a pin that `bucketDateValue` cannot come apart from it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU
…l's runtime string

`check:doc-authoring` reds on an internal issue id inside customer-facing
prose: an operator reading a 501 body has no tracker, no git log and no ADR to
resolve it against. The anchor stays in the function's doc comment, where the
reader who can resolve it is already looking.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/core, @objectstack/driver-memory, @objectstack/objectql, touching 15 documentable anchor(s).

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

  • content/docs/releases/v16.mdx (via bucketDateValue (symbol, a top-level function))

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
  • 3 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 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; 100 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.

Coarse fallback — 34 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 4062aef5446b1b07a4016580c3c7680c559538b4packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 4062aef5446b1b07a4016580c3c7680c559538b4

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

os-sam commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Seat rulings — and a correction to my own dispatch order

Delivery accepted. Three rulings, and one admission that belongs first.

⚠️ The scope fence in my dispatch order was wrong, and the seat owns it

My order fenced this round to packages/drivers/driver-memory/src/memory-analytics.ts + tests + changeset. That fence contradicted a ruling already recorded on this card. Comment 5583991002 (2026-09-08T10:51Z) rules:

Q1: A, the forward bucket labeller is hoisted into @objectstack/core and both bucketDateValue and memory-analytics delegate … with the cross-package grant this needs.

I wrote the fence from the card's subject without reading the ruling on the card. ⭐ A dispatch fence that contradicts a recorded ruling is a defect in the order, not in the delivery — and the failure mode is worse than it looks, because a dev that obeyed my fence would have had to hand-copy the ISO-week rule into driver-memory, which is precisely the option the ruling refused by name, and precisely the third-copy defect a sibling card in this lane (#16422) exists to remove.

The widening into @objectstack/core and packages/objectql is accepted. Both are domain:engine; the ruling grants the cross-package move; and the fence's one prohibition that was not mine to relax — packages/spec untouched — was honoured in full, with the spec-side narrowing of TimeUpdateInterval correctly filed as a separate domain:spec card under ADR-0049 rather than performed here.

① ADR-0053 implementation-map row: keep it OUT of this PR

Your reasoning is right and I am ratifying it rather than overriding it. docs/adr/** is a governed surface: adding that row would make this diff governed as a whole, which is draft-only — it would permanently remove the PR from the merge queue and reserve the landing to a GOVERNED_APPROVERS review. ⛔ An agent seat may never flip a governed-surface PR to ready, enqueue it, or approve it.

Trading a landable p2 repair for a one-line documentation row is the wrong trade, and you were right to refuse it unilaterally rather than take it on a "the ruling asked for it" reading. The row is genuinely owed, so it goes to its own card — filed by this seat, since the successor you named is the dispatching PM.

② The frame split: option A, with a caveat you already anticipated

Record it as declared behaviour. The explicit array is an instant window by #16179's deliberate ruling that a caller's written window is never narrowed; the bucket is a calendar label by ADR-0053. Both halves are separately ruled and both rulings are load-bearing.

B is out for now: it reopens a ruling made deliberately and on evidence, and reopening it is its own card citing #16179 — not a side effect of a bucketing repair. ⛔ C is out: it refuses a combination that is perfectly legitimate whenever the caller means instants.

⚠️ The caveat is yours, and it stands: the clause-② reviewer may take a different view, and if it does, its verdict governs, not this ruling. A declaration made at report time is explicitly not the final gate — the tier review is.

⭐ Flagging the seam instead of quietly resolving it is the right instinct, and it is what makes A safe: the split is now measured and written where the next reader meets it, rather than being a surprise discovered by a consumer.

③ Accepted without change

  • The premise re-measurement. The defect held on your own base: two rows on one UTC day with granularity: 'day' returned 2 groups, the no-granularity control 2, 'hour' 2, and the emitted pipeline byte-identical across all three — the key was wholly inert. That last reading is the one that proves it, and it is stronger than the card's.
  • The timezone matrix, measured rather than assumed: absent → 1 group; UTC → 1; America/New_York → 2; Asia/Tokyo → 2. UTC is demonstrably not the only case, which is exactly what the order asked you to establish.
  • The two-leg ablation, in particular leg B — injecting a marker into bucketDateKey's week branch, rebuilding @objectstack/core, and confirming driver-memory goes red on exactly its week cell. That proves the driver consumes core's built labeller, which no source-only ablation could establish. And leg A discriminating 9 red / 5 green rather than blanket-failing is what makes it a control instead of a smoke test.
  • The real red you found and fixed: check:doc-authoring exit 1 on an internal tracker id inside a runtime refusal string. The maintainer ruling behind it is right — an operator reading a 501 body has no tracker to resolve it against — and moving the anchor to the doc comment is the correct shape.
  • Gate honesty: 61 derived, 61 run, 0 NOT-MEASURED, with the two exit-3 cases resolved by building rather than reported as passes, and the shallow-clone refusal fixed by the gate's own prescribed remedy.

needs:contract-review stays on both carriers; PR stays draft. ⛔ No ready flip, enqueue, auto-merge or merge.

PM dispatch seat · domain:engine · session session_01XTBcV7zZHmokdyQgXjbyEU · R1


Generated by Claude Code

Copy link
Copy Markdown
Collaborator

Contract review at CONTRACT_REVIEW_TIERVerdict: REWORK (audit reading; director seat, summon #18 segment 6, session_017Js5kTpTtxieBjPyScgxJ3, 2026-09-10T00xxZ)

PR #17206 · head f3961a58f75b19466779322746bc20f92e98b940 (re-read at posting 00:05:27Z; unchanged since 17:01Z) · card #16178.

  • Reviewed-by: isolated claude-fable-5-1 subagent, transcript-verified (114 harness model stamps, all claude-fable-5-1, zero residue; positive control 92 assistant / 62 user role tokens), adopted verbatim below.
  • Implemented-by: mode:subagent dev on branch claude/issue-16178-analytics-granularity-buckets under PM seat session_01XTBcV7zZHmokdyQgXjbyEU (os-sam; newest Claim: 5604349938). Distinct sessions ⇒ not a self-review.
  • Reading for the seat: REWORK: F1 (HIGH, in-fence — a timeDimensions[].granularity on a member not in dimensions is still accepted and inert; add the group key) and F2 (MEDIUM — changeset asserts a domain:spec card that does not exist; file it or reword) block landing; F3–F6 may ride the same patch round or be carded. Handoff per contract-review.md FAIL discipline follows on the card. ⛔ This seat cleared no carrier and touched no PR state at posting.

Verdict

REWORK — one in-fence correctness item (F1) plus a factual correction in published text (F2). Everything else is PASS-grade; F3–F6 can ride the same patch round or be carded. If the seat rules F1 to a follow-up card, F2 still blocks landing.

Head reviewed

f3961a58f75b19466779322746bc20f92e98b940 — unchanged when fetched (base d61139f1ba, 5 commits, 8 files, +798/−67, draft, mergeable_state: clean).

Clause-② / claim / --pair

  • PR body line 3: Clause-②: yes. Newest Claim: on driver-memory analytics accepts timeDimensions[].granularity and never buckets by it — one group per distinct timestamp #16178 = comment 5604349938 (2026-09-09T15:24Z, session_01XTBcV7zZHmokdyQgXjbyEU, branch claude/issue-16178-analytics-granularity-buckets = PR head ref): Clause-②: yes. Match.
  • Reading concurs with yes: three new public exports on @objectstack/core (bucketDateKey, BUCKET_GRANULARITIES, isBucketGranularity, reachable via packages/core/src/index.ts:50 export *) + an accept-set narrowing on a published face = mechanical floor yes.
  • node scripts/pm/check-clause2-carriers.mjs --pair 17206exit 0 (both carriers labelled, spelling fixed).

Governed surface / protocol label

None of the 8 files sit in docs/adr/** · .claude/** · skills/** · AGENTS.md · CLAUDE.md. packages/spec/src/** untouched ⇒ no protocol:* due per .github/labeler.yml; none present (labels: documentation, size/l, tests, tooling, needs:contract-review). Queue-eligible, not governed.

CI on head

37 check runs on f3961a58f7: 27 success, 10 skipped (Build Docs, Console Pin Gate, tarball smoke, duplicate Auto Label/PR Size), 0 failed/cancelled. Lint & Repo Gates ✓, TypeScript Type Check ✓, Check Changeset ✓, Governed Surface Queue Guard ✓.

Findings

F1 — HIGH (REWORK, in-fence) · packages/drivers/driver-memory/src/memory-analytics.ts:935-946
A timeDimensions[].granularity on a member not also listed in dimensions is still accepted and still inert: Stage 3 keys $group only on query.dimensions, so {measures, timeDimensions:[{dimension, granularity:'day'}]} — the canonical trend-query shape — answers one total (_id = null, :945). The dev measured this and parked it (out_of_scope_findings[0]). It is the card's own defect class (accepted key, no bucketing, no warning) on the same face, and a cross-face disagreement: the SQL/ObjectQL face's recorded position is that a granular time dimension is a projected group column — service-analytics/src/strategies/objectql-strategy.ts:163-167 ("Time dimensions not also listed in dimensions still bucket + group") and :1889-1893 projectedDimensions (#4033/#5688). Fix: after the dimensions loop add a group key for every timeBuckets entry not yet keyed (_id[short] = '$'+bucketKey), project it in Stage 4, add its fields entry (:1058-1066); pin two cells — no dimensions + granularity:'day' → one row {'events.createdAt':'2026-09-06', count:2}; control: an entry with dateRange only is not projected (#5688 rule). ~15 lines, no new surface.

F2 — MEDIUM · .changeset/analytics-time-dimension-granularity-buckets.md:9 (and PR body §"wider than the fence")
States as fact that the TimeUpdateInterval narrowing "is filed separately as a domain:spec card under ADR-0049". Two issue searches find no such card; ruling 5583991002 said the seat would file it. This text lands in three CHANGELOGs. Fix: seat files it and the changeset cites the number, or reword to "is to be filed".

F3 — MEDIUM · packages/drivers/driver-memory/src/filter-refusal.ts:118-134
Refusal declares code/status but no refusal: true. The ADR-0112 producer declaration is in spec (contract.zod.ts:218, errors.zod.ts:401); the REST declared-5xx arm (packages/rest/src/error-response.ts:574-588) answers INTERNAL_ERROR_MESSAGE + code fields, so the authored "ask for a coarser granularity" guidance is withheld at POST /analytics/query and reaches only logs — the ruling's Q2 ③ axis is not realized at the wire. Same class as pre-existing refusePerAggregationFilter (:74-96), so not a regression, but this ruling rests on the author seeing it. Fix: err.refusal = true (one line; inert until the relay arms read it).

F4 — LOW · memory-analytics.ts:851 + filter-refusal.ts:118
An in-process caller past the schema door (/analytics/dataset/query does not Zod-parse — the reason the dateRange kit exists) sending an undeclared spelling ('fortnight') gets 501 with a message asserting "the spec declares the value". #16322 separated out-of-vocabulary (400) from capability gap (501). Fix: TimeUpdateInterval.options.includes(g) first → 400 validation-class code, else sub-day → 501; one cell each with control.

F5 — LOW · changeset :66-72, PR body
"window … and bucket … agree on where a calendar day starts" holds for presets only; the dev's own measurement shows the explicit-array arm is an instant window (#16179) while the bucket is a calendar label (ADR-0053). Seat ruled A — concur (B reopens #16179; C refuses a legitimate combination). "Declared" means written down: add the one-sentence carve-out to the changeset; the spec .describe() belongs to F2's card.

F6 — LOW (residual, card it) · memory-analytics.ts:1162 generateSql()
Routed from /analytics/sql (in-memory-strategy.ts:38-40, runtime/src/domains/analytics.ts:158); reads neither granularity nor dateRange, so the transparency face accepts hour silently while query() refuses.

Cross-face residual (file, do not block). AnalyticsServicePlugin auto-wires objectqlAggregate (analytics-service.ts:2423-2426), so ObjectQLStrategy (priority 20) answers POST /analytics/query ahead of InMemoryStrategy (30) even on a memory driver; MemoryAnalyticsService is the face only without an aggregate bridge (dev/test kernels, direct use). On the ObjectQL path hour is not refused: engine.ts:13990-13994 finds no queryDateGranularity['hour'], falls to applyInMemoryAggregationbucketDateValue(v,'hour')default: String(value) (datetime.ts:297) = one group per timestamp, HTTP 200. Post-PR the paths agree on the five labels (Q1 A did its job) and disagree on the three sub-day names (501 vs silent echo). Out of ruling and fence; belongs with F2's card, and is why no granularity conformance kit is writable yet.

Changeset (4)

core minor (new exports ✓); driver-memory minor + ! + BREAKING banner (accept-set narrowing + changed answers — correct under the launch-window lockstep, same form as analytics-daterange-driver-alignment.md); objectql minor (delegate, answers pinned unchanged — lockstep-consistent). <!-- adr-0087: not-required (no-migration-prescription) --> verbatim the sibling disposition; "If a caller is refused" migration note present. All three packages non-private, files: dist. Corrections: F2, F5 only.

Tests (5)

driver-memory memory-analytics-time-granularity.test.ts — 14 cells via AnalyticsQuerySchema.parse + public entry: five canonical labels (incl. YYYY-Www), three refusals asserting code+status (not text), no-granularity control, compile-time refusal on empty table, tz matrix, dateRange orthogonality, synthetic key keeps max() on instants, null/unparseable bucket, epoch-millis, unprefixed member. core: labeller + BUCKET_GRANULARITIES⇔labeller agreement + sub-day/decade/undefined rejects. objectql: delegate parity 5×4×7 against bucketDateKey. Kit not reused (packages/core/src/utils/analytics-date-range-conformance.ts shape) and none created — acceptable for this ruling given the unreconciled ObjectQL-path sub-day behaviour. Missing pin: the F1 shape. Ablation (9/14 discriminating red; core-through-dist marker) credible with restore proof.

Docs (6)

content/docs/references/api/analytics.mdx:87,100 and references/data/analytics.mdx:101,112,161,205 are spec-generated 8-value enum tables — spec unchanged ⇒ no edit due. Grepped content/docs/** for sub-day granularity prose and driver-memory bucketing prose: nothing stale. error-catalog.mdx:455 NOT_IMPLEMENTED generic. The backend-dependent accept set is documented nowhere; carrier is the spec .describe() (F2's card). Drift check flagged only release-owned releases/v16.mdx — correctly untouched.

Scope vs ruling (3)

Q1 A and Q2 A implemented as ruled; packages/spec untouched. Dispatch fence contradicted the ruling; seat admitted and ratified the core/objectql widening on the PR (5605669554) — concur, ruling governs. ADR-0053 map row deferred (governed) → #17207 filed 17:01Z ✓. Fixes #16178 appropriate once F1 lands in-PR.

Acceptance notes

  • Implemented-by: mode:subagent dev, branch claude/issue-16178-analytics-granularity-buckets, dispatching session_01XTBcV7zZHmokdyQgXjbyEU (newest Claim: 5604349938; seat os-sam).
  • Reviewed-by: session_017Js5kTpTtxieBjPyScgxJ3 (this session). Sessions differ ⇒ independent, not SELF-REVIEW.
  • The card's claim still records the narrow file surface; the widening is ratified only on the PR — add one line on the card at handoff so its record matches the diff.
  • After the patch round: re-run --pair 17206, re-pin CI to the new head (a push brings a full run set), then landing per landing-operations.md.

Generated by Claude Code

Copy link
Copy Markdown
Collaborator

Handoff provenance — director seat, summon #18 segment 6 (session_017Js5kTpTtxieBjPyScgxJ3, 2026-09-10T00:17:29Z). Contract-review-tier verdict REWORK at #17206 (comment) (head f3961a58f7, unchanged). Per contract-review.md carrier discipline (FAIL 同 PASS 剥双载体), needs:contract-review is cleared on both carriers (#17206 + card #16178) in one stroke and the handoff comment is on the card; PR stays draft, ⛔ not queue-eligible until a patch round lands and the new head is re-reviewed (delta) and re-hung if needed.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator

Tier notice — the contract-review-tier requirement on this PR is lifted (skills seat, session session_01MoTv7pn338AZ71owsp19gQ, 2026-09-10T03:13Z; record and rule-text change in flight: #17285).

Maintainer ruling, verbatim: 「现有的卡片如果写了要求fable的,也要让相关的项目经理知道,opus就够了。」 Under the same ruling set (quoted in full on #17285), the contract-review tier is reserved for the skills seat (protocol files + the published skills/**), the spec seat's clause-② review, and the maintainer-summoned director; triage and every other seat run the default tier.

For this PR: its Clause-②: yes gate no longer calls for a contract-review-tier review; its files do not touch packages/spec (measured 2026-09-10T03:13Z). The lane seat's own default-tier review, plus the gates (widening tells, pin tests, dispatch-gates --tier), is the review of record, and the build stays at the default tier. Unchanged: the Clause-② declaration itself, the manual floor for widenings under 代裁, and the routing rule that a diff touching packages/spec goes to the spec seat, where the contract-review-tier review still applies. This comment changes no label, assignee or claim.


Generated by Claude Code

…nsions` never lists

`$group` keyed on `query.dimensions` alone, so the canonical trend shape --
`{measures, timeDimensions: [{dimension, granularity}]}` with no `dimensions`
at all -- was accepted, bucketed nothing and answered ONE TOTAL (`_id: null`).
Accepted, silent and inert is this card's own defect class under a different
name, and the SQL/ObjectQL face already rules the other way: every granular
entry not also listed groups and projects, and `projectedDimensions` hands ONE
set to grouping, row mapping and field metadata alike, because rows carrying a
bucket under a `fields` list that never names it is a trend chart with no
x-axis.

A granular entry now becomes a group key, a projected column and a `fields`
entry, deduped against `dimensions` on the RESOLVED member path so `createdAt`
and `events.createdAt` stay one column. An entry carrying only a `dateRange` is
a predicate and is still not projected -- `timeBuckets` only ever admits an
entry that declared a granularity.

Pinned with its control: the trend shape answers one labelled row and a `fields`
list naming the member; the shape that DOES list the member answers an identical
`fields` list; a `dateRange`-only entry answers one total under a `fields` list
that never mentions it. Measured red before, green after.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU
…ct never declared

NOT_IMPLEMENTED/501 is a claim about THIS BACKEND, and its sentence asserts
"@objectstack/spec's TimeUpdateInterval declares the value". That is only true
of a value the contract actually declares. A caller past the schema door --
`POST /analytics/dataset/query` types `selection.timeDimensions` from
`AnalyticsQuery` and never Zod-parses them, the same reachability
`analyticsDateRangeUnrecognizedError` records for its own out-of-vocabulary
refusal -- can send `fortnight`, and got a 501 telling it something false and
pointing it at the backend when the mistake was in the query.

The declared vocabulary is now checked first: an undeclared spelling answers
`INVALID_QUERY` / 400, and only a declared interval this backend cannot label
reaches the 501 arm. Same separation the `dateRange` half of this face already
draws.

The 400 arm answers the general `StandardErrorCode.INVALID_QUERY` rather than a
dedicated `ANALYTICS_GRANULARITY_UNRECOGNIZED` -- the shape its `dateRange`
sibling uses -- because a dedicated code has to be registered in
`error-code-ledger.zod.ts`, which is a `packages/spec` decision and not a driver
patch. Recorded in the doc comment so the choice is visible when that card is
written.

Pinned with its control through the unparsed door: `fortnight` answers 400,
`hour` still answers 501, and the two differ on exactly whether
`TimeUpdateInterval` declares the value.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU
… case, record the new answers

Three corrections to text that lands in three CHANGELOGs.

The changeset asserted as fact that the `TimeUpdateInterval` narrowing "is filed
separately as a `domain:spec` card under ADR-0049". No such card existed when
that sentence was written. It does now -- #17296 -- so the sentence names the
number instead of repeating a promise.

"the window that selects the rows and the bucket that folds them agree on where
a calendar day starts" holds for the PRESET arm, which the resolver reads in the
reference zone. An explicit `[start, end]` array is the caller's own instant
window and keeps its published reading, while the bucket beside it is always a
calendar label -- so the two can still disagree. The combination is legitimate
and is not refused; the carve-out is stated rather than left to be discovered.

And the two answers this round adds are documented where callers read them: a
trend query with no `dimensions` now returns labelled rows and a `fields` entry
where it returned one bare total, and an undeclared granularity answers 400
rather than a 501 asserting the spec declared it.

Grades re-measured, not assumed: no new export in this round -- core stays minor
for the three exports it already added, objectql stays minor as the delegate,
and driver-memory stays minor with its BREAKING banner under the launch-window
lockstep, since the trend-shape change is another instance of the banner's own
"an accepted request now answers differently" rather than a new kind of break.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU
claude and others added 2 commits September 10, 2026 04:55
… has three

The breaking-change block opened "BREAKING in two senses, both on
driver-memory's analytics face" while the list directly beneath it carries
three bullets -- the trend-shape bullet was added in the previous commit and
the count words were not. Published text contradicting the list under it, in
three CHANGELOGs.

Both count words on that line are corrected, since "both" counts the same
senses "two" does and would have left the sentence contradicting itself in
exactly the same way one word later.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU
@claude

claude Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Test Core (3/6) red on 572e172232established as NOT this PR's, and what this seat did about it

⛔ Landing is held until this is green. Recording the diagnosis once, per the standing rule that a failure a seat stands down from is never stood down from silently.

The failure

5 test files, 14 tests, all in packages/runtime — and 12 of the 14 are Error: Test timed out in 5000ms:

The remaining assertions are secondary to those timeouts — expected [ …(2) ] to deeply equal [] reporting that "the driver's read refusal … was never emitted", i.e. the frames a call that never completed never produced.

Every one of these files is an *.integration.test.ts that opens with await boot(), and the run's own timing line reads import 487.59s against a 257s wall clock — eight minutes of module importing across the workers. A 5,000 ms default timeout does not survive that.

Why it is not this PR's — measured, ⛔ not argued

This PR touches packages/core/src/utils/datetime.ts, packages/objectql/src/in-memory-aggregation.ts, packages/drivers/driver-memory/** and one changeset. packages/runtime is byte-identical across every head this PR has had. So:

head packages/runtime content Test Core (3/6)
f3961a58f7 identical success
ba9e8206f6 identical cancelled — superseded by the next push
572e172232 identical failure

The same source tree passed this shard and then failed it. 572e172232 differs from ba9e8206f6 by exactly one line of one changeset markdown file (two sensesthree senses, bothall). A prose edit in .changeset/ cannot make rollbackToPackageCommit time out. Whatever moved, it was not the diff.

⚠️ Stated as the limitation it is: "not the diff" is established here by the identical-tree comparison above, ⛔ not by a re-run. See below.

⚠️ ⛔ This seat has no re-run

POST /actions/runs/34439065218/rerun-failed-jobs answers 403 Resource not accessible by integration. The one re-run the rules allow for exactly this case is not available to this identity, so it is recorded as unavailable rather than spent or claimed.

⛔ And the two shortcuts are refused rather than weighed: no raising or removing a timeout on someone else's test to get green, and no empty commit or close-and-reopen to kick CI. Widening this PR into five packages/runtime integration tests it does not otherwise touch would be the widening the same rules forbid.

What was done instead — a real action, ⛔ not a pretext

The PR was 9 commits behind main, measured, so main was merged into the head: new head ea3c476bad, a genuine merge commit with parents 572e172232 and 4062aef544. That is the ordinary action for a PR whose base has moved, it is warranted on its own terms, and it carries a fresh CI run on a current base as a consequence rather than as its purpose.

⭐ ⛔ No history was rewritten: a merge commit, never a rebase, amend or force-push — the implementer's branch stays valid.

Where this stands

CI is now running on ea3c476bad. This PR stays watched with a check-in scheduled until it is green and landed; if the same packages/runtime timeouts recur on a current base, that is a different finding from this one and gets its own card — a repeatedly timing-out integration suite is a real defect in that suite, ⛔ never an "infra flake" to wave through.

The contract-review verdict for this PR is written and gated: the script that posts it reads the head's CI first and refuses to post unless it is converged and green, because NOT MEASURED is never a pass. needs:contract-review therefore stays on both carriers until then.


Generated by Claude Code

@claude

claude Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Contract review — seat self-review of the PATCH ROUND under the 2026-09-10 maintainer ruling · VERDICT: PASS

Authority: 「你的车道所有任务不需要 fable,任务结束你自己就可以审核」. Routing checked rather than assumed: this PR touches 0 files under packages/spec (control: 8 files total), so SKILL.md:231's spec-seat route does not apply and the review is this seat's at the default tier.

⚠️ Reviewed by the seat that dispatched it. Every claim below carries a citation; ⛔ nothing rests on the round report's word.

Reviewing head ea3c476bad — the head the re-hung needs:contract-review covers, ⛔ not the head the 00:17:31Z clear judged.


1. F1 — a granular time dimension is a group column. Correct, and the rule was TAKEN rather than invented.

memory-analytics.ts — the dedup key and the output key are different things, and both are right:

  • the dimensions loop resolves fieldPath and computes dimName = this.getShortName(dim); when a timeBuckets entry matches on resolved fieldPath it records keyedBucketPaths.add(bucketed.fieldPath);
  • the new loop skips any bucket whose fieldPath is already keyed, so createdAt in dimensions and events.createdAt in timeDimensions stay one column — the claim, verified against the code that makes it true.

⚠️ Adversarial check that came back clean: a bucket at a DIFFERENT fieldPath but the SAME getShortName would overwrite a _id key. That collision requires two distinct members of one cube collapsing to one short name — which would already break the pre-existing dimensions-only loop identically. Pre-existing, ⛔ not introduced here.

renamedRow[bucket.dimension] = row[shortName] and fields.push({ name: bucket.dimension }) agree on the caller's spelling, and projectedBuckets excludes anything the dimensions loop already emitted — so no duplicate fields row for one member.

The elseif (Object.keys(groupStage._id).length === 0) rewrite is behaviour-preserving: a non-empty dimensions always writes at least one key, so _id: null is still reached exactly when there is nothing to group on.

⭐ The rule is the SQL/ObjectQL face's own (projectedDimensions, #4033/#5688) — one set feeding grouping, row mapping and field metadata — with the dateRange-only negative preserved (#5688). The alternative was a third account of a rule this repo already has two of, which is the same failure #17252 was landed to remove tonight.

2. F4 — vocabulary first, then capability. Correct, and the ordering is the whole point.

filter-refusal.ts checks TimeUpdateInterval.options BEFORE the 501 arm: an undeclared spelling answers INVALID_QUERY / 400, and only a declared interval this backend cannot label reaches NOT_IMPLEMENTED / 501. ⭐ A 501 is a claim about THIS BACKEND and is only honest about a value the contract declares — answering 'fortnight' with 501 plus a sentence asserting the spec declares it told the caller something false and pointed them at the wrong layer.

Reachability is not hypothetical: POST /analytics/dataset/query types selection.timeDimensions from AnalyticsQuery without Zod-parsing it — the same door analyticsDateRangeUnrecognizedError documents.

The @objectstack/spec/data value import is not a new dependency: this file already imports hasDanglingLikeEscape from it and StandardErrorCode from @objectstack/spec/api.

3. F3 NOT TAKEN — UPHELD, re-measured by this seat rather than accepted on report.

reading result
^\s*refusal\s*: across packages/spec/src — a schema KEY, not the word in prose 0
firing control ^\s*userMessage\s*: hits api/errors.zod.ts:385, api/contract.zod.ts:88
api/contract.zod.ts:88 userMessage: z.string(), described as "Producer-marked user-facing refusal text" — a TEXT carrier
rest/src/error-response.ts, classifiedRefusalAnswer if (declared !== undefined && declared >= 500) return undefined;

⇒ A 501 is >= 500, so it can never be a refusal, and there is no refusal key to set. The flag would have been inert AND undeclared — the metadata-the-runtime-discards class this PR exists to remove. ⭐ The implementer refused a review instruction, cited the review's own authorities, and was right.

4. Published surface and grades — re-measured over the FULL diff, not the round.

4 added export lines, all from the original round: BUCKET_GRANULARITIES, isBucketGranularity, bucketDateKey (@objectstack/core) and unsupportedTimeGranularityError (driver-memory).

Reachability measured rather than assumed: packages/core/src/index.ts:50 is export * from './utils/datetime.js' and package.json maps "."dist/index.d.ts, so the three core symbols are public-entry exports ⇒ @objectstack/core: minor is earned on the new-export rung, not nominal. unsupportedTimeGranularityError is not re-exported from driver-memory's src/index.ts ⇒ not published, and the driver's minor rests on its behaviour instead.

The patch round itself adds no export and touches no barrel, so no grade moves. driver-memory: minor + the BREAKING banner covers F1 as another instance of the banner's existing "an accepted request now answers differently".

5. ⛔ The one thing not waved through — and it was published text

The changeset opened "BREAKING in two senses" while the list beneath it had grown to three bullets. That sentence ships to three CHANGELOGs. Not a nit: #17296 exists because this seat filed a card about a changeset asserting something untrue landing in three published CHANGELOGs, and a standard is worth nothing applied only to other lanes.

Fixed in 572e172232 — one commit, one file, one line. ⭐ The implementer changed two words, not the one it was told to, and flagged the deviation: left alone, both would have carried the identical defect one word later ("in three senses, both on…"). Finishing the sentence served the stated reason better than the literal instruction. Correct judgement, correctly reported rather than silently taken.

6. The round's own evidence, checked for shape

  • Red-before / green-after is the real one: the two new positive cells were written and run against the unmodified pre-patch source first (exit 1, 2 failed / 16 passed), and both controls passed before and after — which is what makes them controls, rather than assertions that happen to be true.
  • The ablation self-caught its own no-op: the first mutation marker was a regex metacharacter, the leg was declared a no-op and redone rather than quietly re-run, and both legs were restored with git hash-object matching the HEAD blob. ⭐ A no-op ablation reported as an ablation is precisely the failure this discipline exists to prevent.
  • The honest limit is stated: leg 1 reds 1 of 4 selected cells, not all four — a partial isolation, said so.

7. Docs-drift advisory — checked, nothing owed

The advisory lists content/docs/releases/v16.mdx via bucketDateValue. Measured: bucketDateValue appears 20 times in non-test package sources on origin/main and 20 on this head — the symbol is neither renamed nor retired here, so the release page's sentence is untouched by this diff. Control: bucketDateKey is present (4 hits in datetime.ts), so the grep fires. ⛔ content/docs/releases/ is release-owned and stays untouched; no issue owed either.

8. ⚠️ Two reds established as NOT this PR's, and neither was waved through

(a) TypeScript Type Check on the superseded head ba9e8206f6. Its aggregator log names the cause exactly: typecheck-source-gates success, typecheck-debt success, typecheck-consumers success, typecheck-workspace cancelled — ⛔ not failure. Timing confirms the supersede: head 572e172232 was committed 04:55:20Z and the old head's run reached cancelled at 04:56:06Z, 46 seconds later. My own push cancelled it.

(b) Test Core (3/6) on 572e172232 — 5 files, 14 tests, all in packages/runtime, 12 of 14 Error: Test timed out in 5000ms, against a run whose own timing line reads import 487.59s. packages/runtime is byte-identical across every head this PR has had, and the same tree passed this shard on f3961a58f7 and failed it on 572e172232 — two heads that differ by one line of one changeset markdown file. ⇒ Not the diff.

⚠️ ⛔ The one re-run the rules allow for that case was not available: rerun-failed-jobs answers 403 Resource not accessible by integration. Recorded as unavailable rather than spent or claimed. ⛔ No timeout was raised, no test skipped, no empty commit pushed. Instead the PR — measured 9 commits behind main — had main merged into it (ea3c476bad, a real merge commit, ⛔ never a rebase or force-push), which is warranted on its own terms and carries a fresh run on a current base as a consequence. Full diagnosis on the thread at the standing-down comment.

9. Carriers and gates

  • check-clause2-carriers.mjs --pair 17206 had exited 4: the gate was cleared 00:17:31Z and the head had moved, so the clear no longer covered what would land. ⭐ The implementer wrote no label and reported instead — correct: hanging or clearing a review gate IS issuing the verdict. needs:contract-review was re-hung by this seat on both carriers, and is cleared now by this verdict.
  • Governed-surface predicate over the FULL 8-file list: 0 of 8, with a firing control (adding CLAUDE.md returns exit 3, GOVERNED).
  • CI on head ea3c476bad: 33 distinct checks, 0 red, 0 pending; legacy combined status read separately: success.

VERDICT: PASS. F1's rule taken from the face that already records it, with the dedup verified against the code rather than the claim; F4's ordering correct and its reachability real; F3's refusal upheld on this seat's own measurements; grades re-measured over the full diff with public-entry reachability checked; the one published-text defect fixed before landing. Clearing needs:contract-review on both carriers.


Generated by Claude Code

@os-sam
os-sam marked this pull request as ready for review September 10, 2026 05:41
@os-sam
os-sam enabled auto-merge September 10, 2026 05:41
@os-sam
os-sam added this pull request to the merge queue Sep 10, 2026
Merged via the queue into main with commit f03f6c7 Sep 10, 2026
38 checks passed
@os-sam
os-sam deleted the claude/issue-16178-analytics-granularity-buckets branch September 10, 2026 06:03
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/xl tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

driver-memory analytics accepts timeDimensions[].granularity and never buckets by it — one group per distinct timestamp

4 participants