Skip to content

fix(service-analytics): judge both ObjectQL doors by one filter-member view (#10759) - #10862

Merged
huangyiirene merged 1 commit into
mainfrom
claude/issue-10759-crossobject-conjunct-refusal
Aug 22, 2026
Merged

fix(service-analytics): judge both ObjectQL doors by one filter-member view (#10759)#10862
huangyiirene merged 1 commit into
mainfrom
claude/issue-10759-crossobject-conjunct-refusal

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Fixes#10759

The defect, reproduced before it was touched

The card was read from the code, not executed. Both doors were fired against one fixture in one run on origin/main at 5f2e54cc6 (PR #10758 merged), and they answer differently for the same query:

cube: sales_by_account (object opportunity, include ['account'],
dimension region -> account.region, measure revenue -> sum amount)
where: { $or: [{ 'account.region': 'West' }, { stage: 'won' }] }
BEFORE execute() ACCEPTED engine.aggregate got
{"$and":[{"$or":[{"account.region":"West"},{"stage":"won"}]}]}
generateSql() REFUSED INVALID_FIELD / 400
cannot evaluate a cross-object filter ("account.region")
AFTER execute() REFUSED same message, same INVALID_FIELD / 400
generateSql() REFUSED unchanged

planCrossObject reads Object.keys(filter) and nothing else, and the two call sites handed it different things. generateSql() handed it every member the where touches, flattened out of the tree. execute() handed it the built engine filter — where an AND-ed leaf sits at the top level and is seen, but anything structural (an $or, a $not, a nested $and that cannot merge) has been folded into filter.$and, so the only readable key was the literal $and, which is never a field name.

engine.aggregate cannot join. The half that returned rows was not answering the cross-object query: the disjunct naming a column the base object does not have can never match, so the query silently collapsed to its remaining branches and reported a narrower figure as if it were the answer. That is the silent mis-bucket #3654's loud refusal exists to prevent, and the file already stated the invariant it was breaking:

generateSql() calls this too, so the preview accepts/rejects the same set.

The change

Both callers now derive the member list from one filterMemberView(cube, query), so that sentence holds by construction rather than by two call sites happening to agree. The alignment direction is the one the inherited ruling chose: a loud 400 over a silent mis-bucket, and the echo door — which was already right — is untouched.

One member view removed from execute()'s inventory, deliberately: the time-dimension windows, which live in dateRangeBounds rather than in where. They need no arm there — planCrossObject's own first loop over query.timeDimensions already refuses a cross-object time dimension, and refuses it as the dimension the author wrote rather than as the lowered predicate it becomes, which is the better diagnostic and the reason that loop runs first. This makes the two doors' inventories identical rather than merely equivalent.

Is the widened refusal set inside the declared invariant?

Yes, and measurably so. Both call sites now compute the view from the identical expression, so the set execute() newly refuses is exactly the set generateSql() already refused today — no more. Nothing was widened past what the invariant covers, and no new judgement about the refusal set was made.

The #10758 interaction, measured

PR #10758 added a second producer of the $and conjunct shape (a dataset's definition-level filter), so the question was whether a query that used to pass now starts hitting this defect. Measured on today's tree, over the same fixture set:

inputbeforeafter
ordinary dataset filter (is_deleted: false), no whereaccepted, both doorsaccepted, both doors — unchanged
ordinary dataset filter + caller $or naming account.regionexecute accepted / echo refusedrefused, both doors
cross-object dataset filter (account.region), no whereaccepted, both doorsaccepted, both doors — unchanged

So an ordinary dataset scope does not become a refusal, and it does not shield a cross-object member in the caller's own combinator either. The third row is a real defect and is not this one: both doors agree there, because neither call site's member view contains the dataset scope at all, so there is no preview/execution divergence to restore. Filed separately as #10861 (a sub-issue of #10413) rather than widened into this PR — refusing it is a placement decision (query-time versus a contract-first rejection in dataset-compiler.ts) rather than a mechanical fix. Its current behaviour is pinned as-is in the new test file, with the paragraph above it explaining why, so the day that decision lands the pin goes red and points at it.

Pins, in four directions

packages/services/service-analytics/src/__tests__/crossobject-conjunct-refusal.test.ts, 14 tests:

  1. a cross-object member nested in $or, in $not, and two levels deep is refused on the execution door with the ADR-0112 envelope — codeandstatus, plus the member and param the request spelled, and engine.aggregate never reached. Asserting only that it throws would pass on a bare Error and report the platform broken for a caller mistake.
  2. a combinator with no cross-object member still passes both doors and still reaches the engine carrying its disjunction. Without this row an implementation that refuses every combinator scores green and breaks every legitimate $or query shipping today.
  3. the generateSql() door is unchanged, asserted as a two-door agreement over one query rather than as two independent expectations, so the invariant itself is what is pinned.
  4. the /api/v1/analytics/query still drops per-measure and dataset-level filter on the ObjectQL path — engine.aggregate receives no filter at all #10413-phase-1 dataset filter conjunct is not misread — it travels in $and exactly where a combinator travels.

Zero-hit counter-check: the same member at the top level is pinned in the same file. It is refused on both doors before and after, so it proves the fixture, the cube and the detection path work, and cannot be read as evidence for the change. The nested rows are the only ones that moved.

Ablation, direction predicted before running

Predicted: reverting execute()'s third argument to the pre-fix engine-filter view turns exactly 7 of the 14 new tests red — the three envelope rows, the three agreement rows, and the dataset-scope-does-not-shield row — while the top-level control, the three clean-combinator rows, the cross-object-dimension row and both dataset-filter rows stay green. Ordinary direction; no inversion available (nothing counts verdicts, no rule was narrowed).

Measured: 7 failed | 1755 passed (1762), and the seven names are exactly the predicted seven.

Restore proved byte-identical: git hash-object on the strategy reads 720c84d0f2a4d091dce27ad76cfe9d3d705b9da3 both before the mutation and after the restore, and the restored leg is 80 files / 1762 passed.

Positive control for the resolution path: service-analytics/dist was present and byte-identical across both legs (all six artifacts, verified by git hash-object) — no rebuild happened between them — and the mutation was observed anyway. So this suite resolves the strategy through src/, measured rather than assumed; a dist-resolving suite would have stayed green through the mutated leg.

Verification

whatresult
@objectstack/service-analytics testsTest Files 80 passed (80) · Tests 1762 passed (1762)
@objectstack/rest (downstream consumer) testsTest Files 133 passed (133) · Tests 2171 passed (2171)
package tsc --noEmit10 pre-existing errors, all in other test files (this package's frozen TEST_DEBT); zero in either file this PR touches

Gate union derived with node scripts/pm/dispatch-gates.mjs, no path arguments, on a clean tree at the final commit ddfdf335f; every exit code captured before any pipe. All green:

check:changeset-gate-self-tests · check:objectui-changeset · check:slot-lookup · check:test-source-alias · check:type-source-resolution · check-adr-0087-registration · check-changeset-no-major · check-empty-changeset · check-plugin-teardown-shape · check-affected-docs · check:query-options-erasure · check:type-check-coverage · check:type-check-debt (--re-measure, on the built closure) · check:engine-double-contract · check:where-matcher · check:nul-bytes · check:route-envelope · check:dispatcher-error-vocabulary

The last two are the class-#10309 pair, run explicitly (each self-tests first); the path derivation did not name them.

Changeset

minor, declared breaking. A widened refusal set means a query that returns rows today can start returning 400 INVALID_FIELD — narrow, and only on a deployment whose driver reports objectqlAggregate but not nativeSql, but real. ADR-0087 disposition: not-required (no-migration-prescription) — the affected input is an ad-hoc request body, not a metadata surface; no authorable key, export or config field is removed or renamed, so objectstack migrate meta has nothing to rewrite and no stored shape converts. The gate reports it as ADR-0087 exemption (no-migration-prescription).

Generated by Claude Code


Generated by Claude Code

…r view (#10759)
`ObjectQLStrategy.planCrossObject` reads `Object.keys(filter)` and nothing else,
and the two call sites handed it different things. `generateSql()` handed it every
member the `where` touches, flattened out of the tree; `execute()` handed it the
built engine filter, where anything structural (`$or`, `$not`, an unmergeable
nested `$and`) has been folded into `filter.$and` and the only readable key is the
literal `$and` — never a field name.
So a cross-object reference nested in a combinator was refused by the preview and
accepted by the execution door. `engine.aggregate` cannot join, and the accepted
half did not answer the cross-object query: the branch naming a column the base
object does not have can never match, so the query silently collapsed to its
remaining branches. That is the silent mis-bucket #3654's loud refusal exists to
prevent, and the file already stated the invariant it was breaking.
Both callers now derive the member list from one `filterMemberView`, so "the
preview accepts/rejects the same set" holds by construction instead of being
restated at two call sites that can drift.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/releases/v14.mdx(via generateSql (symbol))
  • content/docs/releases/v17.mdx(via ObjectQLStrategy (symbol), generateSql (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
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 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

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 db82944559893eda5e0eb171c8bdf9fff23638a7packageMentionDocs.

Which tree this was computed on

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

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

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 21, 2026
@huangyiirene
huangyiirene marked this pull request as ready for review August 22, 2026 01:26
@huangyiirene
huangyiirene added this pull request to the merge queueAug 22, 2026
Merged via the queue into main with commit 57e4571Aug 22, 2026
32 checks passed
@huangyiirene
huangyiirene deleted the claude/issue-10759-crossobject-conjunct-refusal branch August 22, 2026 01:43
@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 32543579286 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Console Pin Gate — 失败步骤: Build the Console SPA at the pinned objectui SHA

    ✗ Build failed in 4.92s
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

跨 PR 相同签名(24h,按失败测试文件聚合):

  • ⚠️本次没有可用的聚合签名(日志里没有能解析出测试文件名的 FAIL 行)—— 这不是「没有同签名的其他 PR」,是这一轮没测到。跨 PR 聚合本次不可用,请手工比对其他 PR 的同类评论。
  • ⚠️ 24h 评论账本没读完(超过 5 页仍未读到窗口尽头),所以上面的「不同 PR 数」是下界,不是全量。

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 60 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

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.

ObjectQLStrategy.execute cannot see a cross-object filter nested in a conjunct, so it accepts a set /analytics/sql rejects

3 participants

@os-warren@huangyiirene@claude