Skip to content

[spec] parseFilterAST lowers ['a', '=', { $field: 'b' }] to a bare field spec no backend reads as equality #7597

Closed
#7672
Closed
[spec] parseFilterAST lowers ['a', '=', { $field: 'b' }] to a bare field spec no backend reads as equality#7597
#7672
Assignees
os-zhuang
Labels
bugSomething isn't workingdomain:spec

Description

@os-zhuang
os-zhuang
opened on Aug 11, 2026

Measured while implementing #5222 (cross-field push-down). Filing unassigned — recording, not claiming.

The fact

parseFilterAST lowers an authored triple differently depending on the operator spelling. Measured on current main:

Authored Lowered to
[['amount', '>', { $field: 'budget' }]] {"amount":{"$gt":{"$field":"budget"}}}
[['amount', 'gt', { $field: 'budget' }]] {"amount":{"$gt":{"$field":"budget"}}}
[['amount', '=', { $field: 'budget' }]] {"amount":{"$field":"budget"}} ← bare
[['amount', 'equals', { $field: 'budget' }]] {"amount":{"$field":"budget"}} ← bare

The equality spellings drop the operator, because a literal comparand's implicit-equality form is { field: value } — correct for a literal, but for a field reference it produces a field spec whose only key is $field.

Nothing reads that shape as an equality. In matches-filter.ts, evalField sees an all-$ key set and dispatches $field to evalOp, which has no arm for it and returns its fail-closed false. So the filter silently matches no record. On the SQL side the same shape arrives as an OPERATOR named $field.

Why it matters

Two authored spellings of one intent, two fates: ['amount', '>', ref] works (post-#5222 on SQL, and always in memory), ['amount', '=', ref] silently matches nothing. The failure is invisible on the path that produces it.

#5222 narrowed the SQL side to give this shape an actionable refusal naming $eq rather than a generic operator list — but that is a driver-side mitigation of a lowering defect, and it deliberately did NOT compile the shape, because compiling it would make SQL answer rows for a filter the memory evaluator answers false for (a new divergence in the change that closed one).

The decision this needs

Most likely: parseFilterAST should lower an equality triple whose comparand is a FieldReferenceSchema to { field: { $eq: ref } } rather than the bare form — the spelling that IS implemented on both paths. That is a one-sink change (parseFilterAST is the single lowering sink per #5158), but it is a semantics change at the authoring face, so it wants a ruling rather than a drive-by fix.

Alternatively, matches-filter.ts could read a bare { $field } field spec as an implicit equality — but that changes the evaluator's unknown-operator posture, which #6520 examined and deliberately KEPT.

Refs

Activity

  1. os-zhuang
    added
    bugSomething isn't working
    domain:spec
    needs-user-decision
    on Aug 11, 2026

  2. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    os-zhuang
    on Aug 11, 2026 – with Claude
    ContributorAuthor

    Triage: needs-user-decision appended; domain:spec (filer-applied) confirmed correct.

    本评论来自分诊座位 Routine（#5474 试点），不构成认领。

    Generated by Claude Code

  3. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    os-zhuang
    on Aug 11, 2026 – with Claude
    ContributorAuthor

    Maintainer ruling recorded 2026-08-11 (PM session, executing the maintainer's direct instruction in chat — verbatim: 「接受你的全部建议，请更新 issue 的状态和标签」, accepting the four-lens decision-inbox review in full).

    Ruling: fix the lowering. parseFilterAST lowers an equality triple whose comparand is a FieldReferenceSchema to { field: { $eq: ref } } — the spelling already implemented on both evaluation paths. ⛔ Do NOT change the memory evaluator's unknown-operator posture; #6520's decision stands. Single-sink change per #5158.

    Rationale (four-lens): silent zero-row matching is the worst failure shape; the fix is one sink, no new capability surface, and removes a trap AI-authored filters walk straight into.

    State: needs-user-decision → pm:queue (domain:spec seat).

    Generated by Claude Code

  4. os-zhuang
    added
    pm:queue
    and removed
    needs-user-decision
    on Aug 11, 2026

  5. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    os-zhuang
    on Aug 11, 2026 – with Claude
    ContributorAuthor

    Round-1 selection note (spec lane, session session_01JY2Q5Xto1u8YHADgrZDTnk): deferred to the NEXT round by the batch cap — round 1 took #7495/#7496/#7513 (fan-out-ordered). Not shelved; this card rides first in round 2, ahead of #7596 (same-file serialization: both edit packages/spec/src/data/filter.zod.ts, and this ruled silent-zero-rows fix outranks the removal).

    Facts recorded for the dispatching round (all re-verified 09:18Z on origin/main):

    Generated by Claude Code

  6. os-zhuang
    added
    pm:dispatched
    and removed
    pm:queue
    on Aug 11, 2026
  7. os-zhuang
    self-assigned this
    on Aug 11, 2026

  8. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    os-zhuang
    on Aug 11, 2026 – with Claude
    ContributorAuthor

    Claim: PM loop round 1 backfill (spec lane — slot freed by #7495's voiding; maintainer instructed 加速派发)
    Session: session_01JY2Q5Xto1u8YHADgrZDTnk
    Branch: claude/issue-7597-equality-field-ref-lowering
    Worktree: cloud dev session (own container)
    Domain: domain:spec
    File surface: packages/spec/src/data/filter.zod.ts (parseFilterAST, ~:1415) + its tests + the cross-field conformance corpus (cross-field-conformance-cases.ts region, additive). ⛔ Not the $between/$in/$nin unions — that region is #7596's, serialized behind this card. Stop on breach; explain in the report.
    Container & model: M (ruled authoring-face semantics change with cross-path conformance obligations), mode:cloud, model: opus
    Serial constraints cleared: #7596 (same file) is explicitly queued BEHIND this card (hot-file serial queue, seat post #6017; defer note on #7596 09:27Z). No in-flight PR touches filter.zod.ts (checked 09:20Z; #7593 merged this morning — anchors re-verified post-merge). #7617 (in flight, sibling sub-lane) declares a docs-prose face.

    Generated by Claude Code

  9. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    os-zhuang
    on Aug 11, 2026 – with Claude
    ContributorAuthor

    OS-DEV-REPORT — #7597

    os-dev-report

    Draft PR: #7672 · Branch: claude/issue-7597-equality-field-ref-lowering · Base: origin/main @ cc3555e

    Premise verified, then fixed. Measured on origin/main before the first edit: all four $eq spellings (=, ==, equals, eq) lowered ['amount', '<op>', { $field: 'budget' }] to the bare {"amount":{"$field":"budget"}} and the in-memory evaluator returned [], while > / gt lowered to {"$gt":ref} and returned ["1"]. After the fix all four lower to {"amount":{"$eq":{"$field":"budget"}}} and return ["3","6"] — the conformance corpus's declared $eq row set. parseFilterAST is at filter.zod.ts:1415 as the dispatch said (the triage comment's :1205 was stale); confirmed single sink — no second definition.

    Full spelling set measured, not assumed. AST_OPERATOR_MAP folds exactly those four onto $eq, and convertComparison's implicit-equality branch intercepts exactly those four, so the two lists coincide. A new test sweeps the whole exported VALID_AST_OPERATORS vocabulary and fails if any spelling lowers a reference comparand to a bare field spec.

    Compile-surface enumeration (full evidence in the PR body): face 1 driver-sql already conformant and now exercised by the new authoring arm; face 2 turso RemoteTransport already conformant (loud INVALID_FILTER before and after); face 3 read-scope-sql not reachable (read scopes are policy-compiled, never through this sink) and owned by #7598; face 5 formula is the reference implementation; half-face objectql having out of reach (having is a FilterCondition object, never AST-lowered); driver-memory/driver-mongodb out of scope per the #5499 freeze.

    ⚠️ One measured consequence the PM should weigh — face 4, service-analytics filter-normalizer. The equality triple moves there from a loud INVALID_FILTER/400 ("Unsupported filter operator $field") into the same SILENT bucket the ordering spellings have occupied since #3948: assertCompilableComparand gates only the LIKE family and $in/$nin members, so a scalar operator's object comparand falls through to toSqlBindValue and is bound as JSON text. Not a new defect class — one more spelling entering an existing one — and the same shift lands on the frozen driver-memory. Left unfixed deliberately, on the precedent this exact surface set: #7604's triage records that #5222's dev seat declined to widen analytics as a side effect and the PM called that right; refusing only $eq there would create a fresh asymmetry inside the face, and refusing all six is #7598's open question 1. Measurement posted to #7598.

    Re-pricing #7596: untouched. The list positions are unchanged by this PR (in/nin/between lowerings pass their comparand through as before), and the two diff hunks start at :1350/:1376, so #7596's unions at :319-320 and :819-820 are byte-identical and unshifted. It gets marginally cleaner to argue — after this fix the remaining declared-but-unexecutable $field positions are exactly the ones #7596 removes — but neither easier nor harder to do.

    Region boundary respected: the $between/$in/$nin unions were not touched.

    {
  "issue": 7597,
  "status": "done",
  "branch": "claude/issue-7597-equality-field-ref-lowering",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/7672",
  "premise_still_valid": true,
  "summary": "parseFilterAST's implicit-equality shorthand now branches on the comparand: an equality triple whose comparand is a FieldReferenceSchema lowers to { field: { $eq: ref } } instead of the bare { field: { $field } } that no backend reads as an equality, so ['amount','=',ref] finally behaves like ['amount','>',ref] on both evaluation paths. All four $eq spellings are covered and a vocabulary sweep test keeps a fifth from re-opening it; literal comparands, non-string $field carriers, and the evaluator's #6520 unknown-operator posture are untouched — a hand-authored bare field spec keeps today's fate on every backend. The cross-field conformance corpus gains an AUTHORING arm (CROSS_FIELD_AUTHORED_CASES) that enters through the sink and holds the lowered filter to the same both-paths-same-rows rule, run by driver-sql per dialect cell and by driver-sqlite-wasm. One consequence to weigh: at service-analytics' filter-normalizer (and the frozen driver-memory) the equality spelling shifts from a loud refusal into the silent JSON-bind bucket the ordering spellings already sit in — measured, reported, and left to #7598 rather than widening analytics as a side effect.",
  "tests": "pnpm --filter @objectstack/spec test => 377 files / 9880 tests passed (includes the new src/data/filter-field-reference-lowering.test.ts). pnpm --filter @objectstack/driver-sql test => 86 passed | 4 skipped (90) files, 1399 passed | 50 skipped tests; the 15 new authoring-arm cases confirmed running by name under -t '#7597'. pnpm --filter @objectstack/driver-sqlite-wasm test => 24 files / 374 tests passed. pnpm --filter @objectstack/formula test => 25 files / 631 tests passed. typecheck (spec, driver-sql, driver-sqlite-wasm) clean. Gates: check:driver-conformance OK (37 covered cells, 3 DEBT, 0 exempt); check:merge-driver, check:adr-anchors, check:spec-parsed-alias OK; check:authorable-surface OK (1276 defaults unchanged, no anchor diff); scripts/check-nul-bytes.mjs OK (7069 files). REVERSE VERIFICATION, direction predicted first: removing the fix via a patch file must turn the AUTHORING arm red and leave the hand-authored bare-form refusal green. Measured exactly that -- 14 authoring cases plus 'the equality TRIPLE no longer lowers to that bare spelling' failed with \"expected { amount: { '$field': 'budget' } } to deeply equal { amount: { '$eq': ... } }\", while the '>' control and the bare-form refusal pin stayed green; restored with git apply. CI status at report time: in_progress (draft PR just opened; PM owns convergence).",
  "open_questions": [
    {
      "question": "service-analytics' filter-normalizer (face 4) and the frozen driver-memory now answer the equality-with-$field shape SILENTLY (JSON-bound comparand, zero rows) where they previously refused loudly. Fix inside #7598's scope, or does the PM want a narrow refusal landed sooner?",
      "options": [
        "A - leave it to #7598 (queued, domain:services, owns both analytics emitters; its open question 1 is implement-vs-refuse for exactly these faces)",
        "B - land a narrow $field refusal at the analytics scalar-comparand door in a follow-up card, ahead of #7598's larger decision, so the face stays loud for all six operators"
      ],
      "recommendation": "A. Refusing only $eq at that door would create a fresh asymmetry inside the analytics face -- the exact defect class this card closes -- and refusing all six pre-empts #7598's ruling on whether these emitters implement $field instead. The shift is not a new defect class: the ordering spellings have been silent there since #3948, so this is one more spelling joining an existing, already-filed hole. The measurement has been posted to #7598 so its executor inherits it rather than re-deriving it."
    }
  ],
  "out_of_scope_findings": [
    "not filed (existing card): the analytics silent-bind residue above belongs to #7598 (#7604 was closed as its duplicate) - measurement posted there as a comment rather than a twin issue",
    "FieldReferenceSchema's execution-support docblock (packages/spec/src/data/filter.zod.ts, ~:36-64) still says SQL push-down refuses a field reference and that compiling it 'is tracked as its own capability in #5222' - stale since #7582 landed that compilation. Doc-only, adjacent to but not part of this card's change; left untouched, flagged here for the PM to route (a docs-accuracy pass on the same file would also catch it)."
  ]
}

    Generated by Claude Code

  10. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    os-zhuang
    on Aug 11, 2026 – with Claude
    ContributorAuthor

    Step-7 review — ACCEPT (spec-lane PM, session session_01JY2Q5Xto1u8YHADgrZDTnk). PR: #7672 (draft, Fixes #7597).

    Verified against GitHub, not the report's claims:

    Landing plan: flip to ready + auto-merge on gate-job conclusions (flip check armed); the merge queue serializes it against #7655/#7657 mechanically.

    Generated by Claude Code

  11. os-zhuang
    closed this as completedin #7672on Aug 11, 2026
  12. os-zhuang
    removed
    pm:dispatched
    on Aug 11, 2026
  13. akarma-synetal
    added 2 commits that reference this issue on Aug 17, 2026

    fix(spec): lower equality triples with a $field comparand to {$eq: re…

    6a9dec6

    feat(service-analytics): serve a `$field` RLS rule by declining nativ…

    9a75790
  14. akarma-synetal
    added a commit that references this issue on Oct 7, 2026

    test(spec): the third data/ file group's test titles state each cited…

    969ffba
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingdomain:spec

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions