Skip to content

fix(analytics): boolean filters & group-by compare against real boolean, not stringified '1'#2287

Merged
os-zhuang merged 2 commits into
mainfrom
claude/strange-kowalevski-54f247
Jun 24, 2026
Merged

fix(analytics): boolean filters & group-by compare against real boolean, not stringified '1'#2287
os-zhuang merged 2 commits into
mainfrom
claude/strange-kowalevski-54f247

Conversation

@baozhoutao

Copy link
Copy Markdown
Contributor

Problem

On the analytics dataset query path (POST /api/v1/analytics/dataset/query), a metric widget filtered on a boolean field (e.g. { is_critical: true }) always returned 0, and pie/donut/bar charts grouped by a boolean dimension failed to bucket.

Root cause

A lossy string roundtrip in the filter normalizer:

  1. stringifyForCube serialized boolean true'1' (filter-normalizer.ts) — indistinguishable from a numeric 1.
  2. ObjectQLStrategy then coerced '1' back to the number 1 via coerceFilterValueForSql and passed { is_critical: 1 } into engine.aggregate.
  3. The stored field holds a real boolean true, so 1 !== true — the comparison never matched.

The SQL strategy needs 1/0 (better-sqlite3 cannot bind a JS boolean), but the ObjectQL engine compares against the stored runtime type — sharing one coercer was the defect.

Fix

  • stringifyForCube serializes booleans as the tokens 'true'/'false', preserving boolean identity through the string[] pipeline form (SQL output unchanged: 'true'1).
  • New coerceFilterValueForObjectQL recovers a real true/false for the ObjectQL engine; SQL path keeps recovering 1/0. Shared numeric recovery extracted into recoverNumber.
  • ObjectQLStrategy.convertFilter uses the ObjectQL coercer.

Test

New objectql-strategy-boolean-filter.test.ts — the mock engine filters/buckets in-memory rows by strict === (so a stringified 1 fails it):

  • { is_critical: true } → real boolean reaches the engine, count is non-zero (3);
  • { is_critical: false } → matches its rows (2);
  • group-by on a boolean dimension → produces both true (3) and false (2) buckets.

Full service-analytics suite (154 tests) passes; package builds clean.

Context: discovered while building an ObjectStack dashboard; the downstream project worked around it with a numeric-range filter + string mirror field. This is the real fix in framework.

…, not stringified '1'

The analytics filter normalizer stringified boolean `true` → `'1'`, which the
ObjectQL strategy then coerced back to the number `1` before passing it into
`engine.aggregate`. Stored boolean fields hold a real `true`/`false`, so
`1 !== true` never matched: boolean metric widgets always returned 0 and
boolean group-by dimensions failed to bucket.

`'1'`/`'0'` was indistinguishable from a numeric 1/0, so the boolean identity
could not be recovered. Fix the roundtrip:

- `stringifyForCube` now serializes booleans as the tokens `'true'`/`'false'`,
  preserving the boolean identity through the `string[]` pipeline form.
- New `coerceFilterValueForObjectQL` recovers a real `true`/`false` for the
  ObjectQL engine (compares against the stored runtime type); the SQL path keeps
  recovering `1`/`0` since better-sqlite3 cannot bind a JS boolean. Shared
  numeric recovery extracted into `recoverNumber`.
- ObjectQLStrategy.convertFilter uses the new ObjectQL coercer.

Adds a regression test whose mock engine filters/buckets in-memory rows by
STRICT `===`: a `{ is_critical: true }` filter yields a non-zero count, `false`
matches its rows, and a boolean group-by produces both true/false buckets.
@vercel

vercel Bot commented Jun 24, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
spec Ready Ready Preview, Comment Jun 24, 2026 1:59pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): packages/services.

5 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/guides/packages.mdx (via packages/services)
  • content/docs/guides/runtime-services/audit-service.mdx (via packages/services)
  • content/docs/guides/runtime-services/index.mdx (via packages/services)
  • content/docs/guides/runtime-services/settings-service.mdx (via packages/services)
  • content/docs/protocol/objectos/i18n-standard.mdx (via packages/services)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tooling labels Jun 24, 2026
@os-zhuang
os-zhuang merged commit 910a8f0 into main Jun 24, 2026
16 checks passed
@os-zhuang
os-zhuang deleted the claude/strange-kowalevski-54f247 branch June 24, 2026 14:20
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/s tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants