Skip to content

fix(service-analytics): refuse a cross-object per-measure filter on both ObjectQL doors - #12022

Merged
os-trump merged 1 commit into
mainfrom
claude/issue-11461-measure-filter-crossobject-refusal
Aug 25, 2026
Merged

fix(service-analytics): refuse a cross-object per-measure filter on both ObjectQL doors#12022
os-trump merged 1 commit into
mainfrom
claude/issue-11461-measure-filter-crossobject-refusal

Conversation

@claude

@claudeclaudeBot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Fixes#11461

⭐ Clause-② declared: this widens a refusal set on a served path

Accept/reject behaviour changes by construction — a query that was served (with a wrong number) now answers 400 INVALID_FIELD. needs:contract-review is hung on this PR as well as the card (dual carrier). This PR stays draft; the label is not cleared, ready is not flipped, auto-merge is not armed.

What was open

ObjectQLStrategy judges every query by one member view (filterMemberView), documented as "two producers, one inventory" (#10861): the caller's own where, and the compiled dataset's definition-level filter.

#10413 phase 2 added a third producer with the same reach and none of the coverage — a compiled measure's own filter, lowered onto that measure's aggregations[].filter entry (#10576). The view enumerated exactly two origins (type FilterMemberOrigin = 'where' | 'dataset-filter'), and planCrossObject's query.measures arm reads only each measure's resolved field, never its filter. So a cross-object leaf there reached engine.aggregate unrefused on both doors.

Reproduced before it was fixed

The card was explicit that it was code-read, not executed, so this was measured first — one fixture (a dataset with include: ['account'], a cross-object dimension, and a measure whose own filter names account.region), on the ObjectQL door (nativeSql: false), against an honest in-memory engine: one that applies aggregations[].filter as a property match on the base row, which is all engine.aggregate can do, because it cannot join.

BEFORE execute() ACCEPTED — engine.aggregate reached once with
[{field:"*",method:"count",alias:"total_count"},
{field:"*",method:"count",alias:"west_count",
filter:{"account.region":"West"}}]
rows: [{stage:"won", total_count:3, west_count:0},
{stage:"lost",total_count:1, west_count:0}]
⇒ THE SILENT 0. The truthful west_count for "won" is 2,
and total_count 3 is RIGHT — so the wrong number came
back inside the same response shape as the right one.
generateSql() ACCEPTED — SELECT stage AS "stage", COUNT(*) AS "total_count",
COUNT(CASE WHEN account.region = $1 THEN 1 END) AS "west_count"
FROM "opportunity" GROUP BY stage
⇒ a conditional aggregate over a column no FROM in that
statement joins.
AFTER both doors REFUSED — INVALID_FIELD / 400, engine never reached (0 calls).

The change

Mirrors #10861 exactly — the ruled precedent for this hazard class (maintainer, 2026-08-22, Option A: refuse at query time, folding the leaves into the one member view), not a fresh design.

  • FilterMemberOrigin becomes a record so provenance can carry what the key cannot: the view is keyed by resolved field name (account.region), and the actionable locator for a measure filter is the measure whose filter named it. Two measures in one dataset can name the same field and mean two different edits.
  • filterMemberView folds each requested measure's filter leaves in under 'measure-filter'. Only the requested ones: both doors' aggregation loops read measureFilters[m] for m of query.measures and nothing else, so a filter on a measure the query never asks for reaches no engine, and refusing on it would reject a query for a member that was never going to be evaluated.
  • planCrossObject gains a third arm with the same INVALID_FIELD/400 envelope as its two neighbours — same physical verdict, this engine has no join. param is absent for the A cross-object dataset-level filter reaches engine.aggregate unrefused on the ObjectQL path — both doors accept what the engine cannot join #10861 reason, and it bites harder here: member is the cross-object field, so member + param: 'measures' would send a reader to look for account.region inside measures, where it is not and cannot be. cube plus the message carry the real locator.
  • Insertion order (measure-filter → dataset-filter → where, last write wins) means every shape refused before this card keeps the exact message it had, and a member the request also names keeps the caller's own diagnostic.

Both doors are closed in one placeexecute() and generateSql() each call planCrossObject(cube, query, filterMemberView(cube, query, ctx)), so the "preview accepts/rejects the same set" invariant is structural rather than restated.

⭐ The independent justification: a published refusal promise that did not hold

Beyond the internal three-producer inconsistency, content/docs/api/data-api.mdx:143-145 is a published Request | Result table promising 400 INVALID_FIELD for a bad field in an aggregations entry. The same aggregations object kept that promise in the field position and broke it in the filter position200 with a silent 0. That is precisely the failure class the section's own preamble names as its reason for existing: "answer 200 with something that looked exactly like a served query."

The page was hand-read (docs-drift is symbol-anchored, #9192, and cannot see this tension). It needs no edit: no sentence in it describes the exception, so making the behaviour loud simply makes the page true again on this path. It was deliberately not "fixed" by writing the exception into the doc — that would promote a silent wrong answer to a documented feature.

Pins — ⑦ and ⑧ in crossobject-conjunct-refusal.test.ts

Six new directions on one fixture, so the distinctions are structural rather than three fixtures that happen to differ. revenue (no filter), won_revenue (ordinary filter), west_revenue (cross-object filter) — the last two one measure apart, travelling the identical measureFiltersaggregations[].filter route.

  • ⑦ the cross-object per-measure filter is REFUSED on both doors, with code/status/member/cube, the absent param, calls == [], and a message naming the measure.
  • load-bearing, twice. An ordinary per-measure filter is still SERVED and still reaches the engine carrying its own aggregations[].filter (a "refuse every measure filter" implementation breaks every conditional aggregate shipping today); and a cross-object filter on a measure this query does not ask for changes nothing (a "judge the whole measureFilters map" implementation refuses a query for a member it was never going to evaluate, taking every other measure on a dataset down with one unserveable one).
  • Plus the KNOWN-PRESENT where control on the same cube, and the ordering pin that the caller's own where wins the diagnostic when both name the same member.

The test harness now captures options.aggregations, not just the whole-call filter — this producer never lands in the whole-call filter, so a harness watching only options.filter could not have seen the defect at all.

Verification — all on the final commit 783657d7b5

Ablation — mutation: remove the ...measureLeaves, spread from filterMemberView (ablates the load-bearing fold, leaving the new arm with nothing to find). Prediction written before the run: turns red, exactly two failures — execute() refuses… on expect(execute).toBeInstanceOf(Error), and both doors agree on [false,false] receiving [true,true] — with the other 22 passing. Observed exactly that: Tests 2 failed | 22 passed (24), AssertionError: accepted — the measure's own filter was invisible to the envelope check: expected undefined to be an instance of Error, and expected [ true, true ] to deeply equal [ false, false ].

Mutation confirmed on disk by anchored greps, not by an editor exit code: removed text count 0, surviving declaration count 1. Restore was armed as a trap … EXIT INT TERM running git checkout HEAD -- <path> (never git checkout <ref> --, which stages), and verified byte-identical afterwards: disk / index / HEAD all 293e0004bf7dc54ddcfdfe7caaf43c809e63fdb8, git status --porcelain empty. No rebuild was needed for these runs — the tests import ../analytics-service.js from source, so no dist/ sits in the resolution path.

Gates — derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (provenance line: "gate list derived from the tree of 'objectstack-ai/objectstack' at commit 783657d"; change set taken by the script itself from the merge base d63b01436, three-dot). Every exit code captured before any pipe; each gate quoted by its own verdict line.

gateits own verdict line
service-analytics suiteTest Files 80 passed (80) · Tests 1778 passed (1778)
check:engine-double-contractOK — 405 pinned, 133 in the DEBT ledger, 2 exempt.
check:where-matcher✓ where-matcher conformance holds: 297 matcher(s) … none new. baseline key set verified against d63b014: no files added.
check:query-options-erasure✓ query-options-erasure ratchet holds: 67 unswept non-test site(s) … none new
check:cross-package-test-inputsOK: 16 package(s) read outside themselves, all declared
check:type-check-coverageOK — 65/78 workspace packages type-checked (plus the root)
check:type-check-debt--re-measure: OK — 32 ledger entr(ies) re-measured in 222.5s, 1898 raw tsc error(s) total, none above its recorded number.
check:nul-bytesOK (scanned 6662 text file(s) … no raw ASCII control bytes).
check:test-source-aliasOK — 72 packages with tests scanned
check:type-source-resolutionOK — 93 tsc program(s) across 77 packages scanned
check:published-files✓ … 69 publishable package(s) … declare a files whitelist
check:slot-lookup✓ slot-lookup ratchet holds: 107 unswept site(s) … none new
check:empty-changeset✓ No empty-frontmatter changeset introduced by this diff
check:changeset-no-major✓ This diff introduces no major bump.
check-adr-0087-registration✓ … adds no declared-breaking changeset
check:changeset-gate-self-testsall three self-tests
check:objectui-changeset✓ objectui-range --self-test: all checks passed
release-rehearsal-clone --self-test✓ self-test passed
check:plugin-teardown-shape✓ … 63 Plugin implementation(s) across 4647 source(s)
pnpm lint (whole repo)eslint . --no-inline-configexit 0, no output; this is the full repo-wide scan, not a narrowing

check:type-check-debt refused on its first attempt ("--re-measure cannot run: 47 workspace dependenc(ies) … have no built type entry point on disk"). That throw is NOT MEASURED, never a pass, so the full closure was built (turbo run build --filter='./packages/*' --filter='./packages/*/*'Tasks: 70 successful, 70 total) and the gate re-run to the green above. Every heavy command ran through scripts/pm/os-verify-lock.sh; each round's VERDICT line reports command-exit 0.

Not run locally: the rest of the ~162-family farm, which CI runs exactly once on this PR regardless.

Generated by Claude Code


Generated by Claude Code

…oth ObjectQL doors
`ObjectQLStrategy` judges every query by one member view (`filterMemberView`),
documented as "two producers, one inventory" (#10861): the caller's `where` and
the compiled dataset's definition-level `filter`. #10413 phase 2 added a third
producer with the same reach and none of the coverage — a compiled measure's own
`filter`, lowered onto that measure's `aggregations[].filter` entry (#10576) —
and `planCrossObject`'s `query.measures` arm reads only each measure's resolved
FIELD, never its filter. So a cross-object leaf there reached `engine.aggregate`
unrefused on both doors.
Reproduced first (the card was code-read, not executed), one fixture, an honest
in-memory engine that applies `aggregations[].filter` as a property match — all
`engine.aggregate` can do, since it cannot join:
BEFORE execute() ACCEPTED, engine reached once with
{field:"*",method:"count",alias:"west_count",
filter:{"account.region":"West"}} and answered
west_count 0 where the truth is 2 — beside a correct
total_count 3, so the wrong number came back in the
same response shape as the right one
generateSql() ACCEPTED, rendering COUNT(CASE WHEN account.region = $1
THEN 1 END) over a FROM with no join in it at all
AFTER both doors REFUSED, INVALID_FIELD/400, engine never reached
Mirrors #10861 exactly, on the same maintainer ruling (2026-08-22, Option A —
refuse at query time, folding the leaves into the one member view): the origin
becomes a record so provenance can carry the MEASURE the field cannot, only the
REQUESTED measures are folded (both aggregation loops read `measureFilters[m]`
for `m of query.measures` and nothing else), and insertion order keeps every
shape refused before this card on the exact message it already had.
Beyond the internal inconsistency this squares the door with a published
promise: `content/docs/api/data-api.mdx` documents that a bad field in an
`aggregations` entry answers 400 INVALID_FIELD. The same `aggregations` object
kept that promise in the `field` position and broke it in the `filter` position
— 200 with a silent 0, the exact failure class that page's preamble names as
its reason for existing. The page needs no edit: no sentence in it describes
the exception, so it is simply true again.
Fixes#11461
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-analytics, touching 6 documentable anchor(s).

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

  • content/docs/data-modeling/analytics.mdx(via account.region (literal))
  • content/docs/permissions/rls.mdx(via account.region (literal))
  • content/docs/ui/dashboards.mdx(via account.region (literal))

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

  • content/docs/releases/v17.mdx(via ObjectQLStrategy (symbol))

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
  • the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: 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 — 8 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 1620c1de2805ade35890c38470a57c9b4fe3eb70packageMentionDocs.

Which tree this was computed on

This run read content/docs from 93db1606438c4a54c7f7d60931985a9ccdf68d94 — the merge of head 783657d7b5fb865441df6a18735b6ee764ea49cf into base 1620c1de2805ade35890c38470a57c9b4fe3eb70, 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 93db1606438c4a54c7f7d60931985a9ccdf68d94 && git checkout 93db1606438c4a54c7f7d60931985a9ccdf68d94
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 1620c1de2805ade35890c38470a57c9b4fe3eb70 783657d7b5fb865441df6a18735b6ee764ea49cf && git checkout -B drift-repro 1620c1de2805ade35890c38470a57c9b4fe3eb70 && git merge --no-ff 783657d7b5fb865441df6a18735b6ee764ea49cf
node scripts/docs-audit/affected-docs.mjs --json 1620c1de2805ade35890c38470a57c9b4fe3eb70

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

@os-trumpClaude

Copy link
Copy Markdown
Collaborator

Contract review PASS at CONTRACT_REVIEW_TIER (verdict + readings on the card: #11461). Label cleared on both carriers; flipping ready and entering the merge queue.


Generated by Claude Code

Merged via the queue into main with commit 399ecadAug 25, 2026
38 checks passed
@os-trump
os-trump deleted the claude/issue-11461-measure-filter-crossobject-refusal branch August 25, 2026 09:50
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A cross-object per-measure filter reaches engine.aggregate unrefused on the ObjectQL path — the third producer #10861's member view does not cover

2 participants

@os-trump@claude