Skip to content

fix(analytics): pin the matrix reports' date-bucket intent and drop the empty column (#523) - #558

Merged
yinlianghui merged 1 commit into
mainfrom
claude/crm-matrix-report-date-granularity-i704uu
Jul 31, 2026
Merged

fix(analytics): pin the matrix reports' date-bucket intent and drop the empty column (#523)#558
yinlianghui merged 1 commit into
mainfrom
claude/crm-matrix-report-date-granularity-i704uu

Conversation

@yinlianghui

Copy link
Copy Markdown
Collaborator

Description

The v9 single-form migration dropped groupingsAcross[].dateGranularity and never carried it to the dataset dimensions, so every matrix report's date axis groups by the raw timestamp — one column per distinct value instead of one per month / quarter / day.

Re-declaring dateGranularity today does not fix that — it empties the report. The blockage was measured against the pinned packages, not assumed. On @objectstack 16.1:

  1. Read scope. A granular dimension is refused by NativeSQLStrategy (canHandle bails on any timeDimension carrying a granularity), so the query falls through to ObjectQLStrategy → the auto-bridged executeAggregate, which calls engine.aggregate() with no ExecutionContext. The sharing middleware then sees an empty principal and composes { id: '__deny_all__' } for every sharingModel: 'private' object — which is all of ours. Measured on lead_metrics, booted kernel with the sharing middleware live:

    dimension rows
    last_contacted_date raw 21
    last_contacted_date + dateGranularity: 'month' 0
  2. Bucketing. A Field.datetime() column is INTEGER epoch millis in SQLite, and 16.1 buckets with a bare strftime('%Y-%m', col) → NULL for every row, i.e. one column. Field.date() columns are TEXT and bucket correctly (close_date2026-Q1 … 2027-Q1), but still hit (1).

Both are fixed in 17.0.0-rc.0 — the bridge threads context and the strategy applies the read scope itself (upstream #3602/#3597), and driver-sql normalises epoch storage before formatting. That is also why #500 could not honour dateGranularity, and why the declarations belong with the 17.0 upgrade rather than ahead of it: landing them now would take three matrix reports (and, on the shared close_date, three dashboard revenue trends) from wrong buckets to no data.

So this ships what 16.1 can honour and pins the rest so the migration cannot be lost again or land silently unfinished.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • Code refactoring (comments corrected to match the code)

Related Issues

Related to #523
Context: #500 (the commit that removed the granularity notes' predecessors), upstream objectstack#3602 / #3597 (read-scope fix), objectstack#3839 ((null) → real null), objectstack#3916 (no column ordering)

Changes Made

  • New guard test/dataset-granularity.test.ts — records the intended bucket for every date dimension and for each matrix report's axis, including the dedicated quarter dimension pipeline_coverage_by_quarter needs (close_date is shared with the Sales/CRM/Executive revenue trends, which want month — a second intent needs a second dimension). It fails if anyone re-declares a bucket on 16.x, and flips to demanding every declaration once the platform pin crosses 17, so the upgrade branch cannot go green with the migration unfinished. Verified in both directions by temporarily pinning the deps to 17.0.0-rc.0.
  • lead_inflow_by_month_source / cases_opened_by_day_priority now exclude records with no date — that empty group is what rendered as the headerless column. Lead report: 21 groups → 20, no null bucket; case report unchanged (no null dates in seeds, so the filter is forward-looking).
  • Comment corrections. opportunity.report.ts claimed "the dataset's dateGranularity buckets it into quarters" for a declaration that does not exist; lead.report.ts and case.report.ts claimed to exercise the month/day bucket paths. The dataset notes now carry the verified mechanism (one canonical note on opportunity_metrics.close_date, pointers from the others) instead of a one-line guess.

Testing

  • Unit tests pass (pnpm test — 12 files, 146 passed, 2 skipped: the 17+ branch of the new guard)
  • Linting passes (pnpm lint — 1 warning, pre-existing and unrelated)
  • Build succeeds (pnpm build, plus pnpm validate and pnpm typecheck)
  • Manual testing completed — a booted kernel (sqlite-wasm + ObjectQL + sharing + analytics) was used to measure every claim above: the 21 → 0 deny-all, the per-field bucket keys (Field.date() buckets, Field.datetime() → NULL), and the column disappearing from the lead report
  • New tests added

Additional Notes

Two symptoms in #523 stay open on purpose and are not CRM-side:

  • Column ordering (07-01, 07-05, …, 07-02) — upstream objectstack#3916. ReportSchema on 16.1 has no order/sort field at all (checked), so an author cannot express it; adding dateGranularity only makes the keys sortable, it does not sort them.
  • The column for reasons other than a missing date — objectstack#3839's '(null)' → real null change. What is fixable here (the genuinely empty group) is fixed above.

When the 17.0 upgrade lands, the migration is: declare the buckets in INTENDED_BUCKET, add close_quarter over the same close_date field, and repoint pipeline_coverage_by_quarter to it. The guard names each of those in its failure output.


Generated by Claude Code

…he empty column (#523)

The v9 single-form migration dropped `groupingsAcross[].dateGranularity` and
never carried it to the dataset dimensions, so every matrix report's date axis
groups by the raw timestamp — one column per distinct value instead of one per
month/quarter/day.

Re-declaring it is blocked on the platform, not on us, and the block was
measured rather than assumed. On the pinned @objectstack 16.1 a bucketed
dimension does not bucket the axis, it empties the surface:

  1. A granular dimension is refused by NativeSQLStrategy (`canHandle` bails on
     any timeDimension carrying a granularity), so the query falls through to
     ObjectQLStrategy → the auto-bridged `executeAggregate`, which calls
     `engine.aggregate()` with no ExecutionContext. The sharing middleware then
     composes `{ id: '__deny_all__' }` for every `sharingModel: 'private'`
     object — all of ours. Measured on lead_metrics: 21 rows → 0.
  2. A `Field.datetime()` column is INTEGER epoch millis in SQLite and 16.1
     buckets it with a bare `strftime()`, so every row lands in one NULL bucket
     (`crm_lead.last_contacted_date`, `crm_case.created_date`). A
     `Field.date()` column is TEXT and buckets correctly, but still hits (1).

Both are fixed in 17.0.0-rc.0 — the bridge threads `context` and the strategy
applies the read scope itself (#3602/#3597), and driver-sql normalises epoch
storage before formatting. That is why #500 could not honour `dateGranularity`
either, and why these declarations belong with the 17.0 upgrade rather than
ahead of it.

So this ships what 16.1 can honour and pins the rest:

  · `test/dataset-granularity.test.ts` records the intended bucket for every
    date dimension and for each matrix report's axis — including the dedicated
    quarter dimension `pipeline_coverage_by_quarter` needs, since `close_date`
    is shared with the Sales/CRM/Executive revenue trends, which want month.
    The guard fails if anyone re-declares a bucket on 16.x, and flips to
    demanding every declaration once the platform pin crosses 17, so the
    upgrade cannot go green with the migration unfinished.
  · `lead_inflow_by_month_source` and `cases_opened_by_day_priority` exclude
    records with no date — that empty group is what rendered as a headerless
    `—` column (lead report: 21 groups → 20, no null bucket).
  · Three report comments described a `dateGranularity` no dataset declares;
    they now say what the code actually does, and the dataset notes carry the
    verified mechanism instead of a one-line guess.

Verified: pnpm validate (no new warnings) + typecheck + build + test (12 files,
146 passed), plus a booted kernel with the sharing middleware live to measure
each claim above and to confirm the `—` column is gone.

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

vercel Bot commented Jul 31, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
hotcrm Ignored Ignored Jul 31, 2026 2:35am

Request Review

@yinlianghui
yinlianghui marked this pull request as ready for review July 31, 2026 02:59
@yinlianghui
yinlianghui merged commit 9d4d1d3 into main Jul 31, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants