ListView.convertFilterGroupToAST drops a fresh Is null / Is not null row as incomplete, so the list filter silently applies nothing

Measured on claude/issue-4736-list-exists-operator-gap (branched from origin/main at 69251bfb5). Found while implementing #4736; a different axis from that card and not addressed by it.

What happens

convertFilterGroupToAST (packages/plugin-list/src/ListView.tsx) drops any condition whose value is null / '' / [] as an unfinished row, and exempts exactly two operators:

.filter(c => {
  // isEmpty/isNotEmpty carry no value input — always keep them.
  if (c.operator === 'isEmpty' || c.operator === 'isNotEmpty') return true;
  const v = c.value;
  return !(v == null || v === '' || (Array.isArray(v) && v.length === 0));
})

But the FilterBuilder renders no value input for six operators, not two — needsValueInput in packages/components/src/custom/filter-builder.tsx excludes isEmpty, isNotEmpty, isNull, isNotNull, exists, notExists. So for the four it does not exempt, "no value" is the row's finished state and it is treated as unfinished.

Measured, one condition per operator with value: '':

FRESH-ROW isEmpty    -> ["name","=",null]
FRESH-ROW isNotEmpty -> ["name","!=",null]
FRESH-ROW isNull     -> []
FRESH-ROW isNotNull  -> []
FRESH-ROW exists     -> []
FRESH-ROW notExists  -> []

[] is "no filter". Nothing is sent, nothing errors, the grid returns every row.

Why it is reachable in the normal flow

addCondition inserts { field: firstColumn, operator: 'equals', value: '' }, and the operator dropdown's onValueChange updates operator alone — value is preserved, so on a fresh row it stays ''. Picking Is null as the first thing you do therefore always produces the dropped shape. The only way to reach a non-empty value is to type one under a value-taking operator and then switch, which is the uncommon path.

isNull / isNotNull are fully expressible on this dialect — mapOperator maps them to isnull / isnotnull, both members of VALID_AST_OPERATORS. The bridge is fine; the row never reaches it.

Why nothing catches it

The saved-view side already solved this and the live-grid side never got the same treatment:

packages/app-shell/src/views/viewFilterFold.ts keeps VALUELESS_FILTER_OPERATORS, which lists all six, and viewFilterFold.emptyValue.test.ts pins that set against the builder's own needsValueInput list by reading the source, so the two cannot drift.
convertFilterGroupToAST has the same concept written inline as a two-operator literal, with no parity assertion against needsValueInput at all.

The two paths are documented as agreeing — the fold's header says "what is not applied is not persisted" and cites this very function — and on these four operators they do not.

Scope note

exists / notExists appear in the measurement above but stop being reachable from this toolbar once #4736 lands (they move behind OPT_IN_OPERATORS). The live surface of this bug after that is isNull / isNotNull, which stay offered because both dialects genuinely express them. #4736 is a separate concern: it is about operators the dialect has no word for, not about value-completeness.

Suggested shape (not a decision)

Give the live-grid path the same shared notion of a value-less operator the fold has, and pin it against needsValueInput the same way — one set, two consumers, one assertion. Where that set should live is the open question: VALUELESS_FILTER_OPERATORS is currently exported from app-shell, which depends on plugin-list, so plugin-list cannot import it as-is.

Filed unassigned for triage.

First-touch grading (repo:objectui PM seat, session session_01RnQd8iMMUwXQEV1crFmQiQ) — accepted → pm:queue, dispatch deliberately held until PR #4745 merges (same files: ListView.tsx, filter-builder.tsx — hot-file serial discipline, and the card's own scope note assumes #4736 has landed).

This is a live, user-visible correctness bug on the common path: addCondition seeds value: '', the operator dropdown preserves value, so picking Is null as the first action always yields the dropped shape — no filter, no error, every row returned. The measurement table (six fresh-row emissions) makes the defect and its boundary exact.

Route ruling under delegation (the card's open question): the shared value-less-operator set moves to @object-ui/components, next to needsValueInput — that placement is forced by the dependency graph (app-shell → plugin-list → components; plugin-list cannot import from app-shell), and it puts the set in the same module as the source of truth it mirrors. Then:

convertFilterGroupToAST's inline two-operator literal is replaced by the shared set;
viewFilterFold's VALUELESS_FILTER_OPERATORS re-exports or aliases it (keeping its existing emptyValue parity test);
both consumers' parity pins assert against the one set — one set, two consumers, one assertion, exactly the card's suggested shape;
the fresh-row emissions for isNull/isNotNull become the regression test (measured [] today → a real AST node after).

Dispatch on the merge event for PR #4745.

Generated by Claude Code

Dispatched by the repo:objectui PM seat (session session_01RnQd8iMMUwXQEV1crFmQiQ), branch claude/issue-4744-valueless-row-drop. PR #4745 has merged, so the serial hold is lifted — branch from current origin/main, which contains the OPT_IN_OPERATORS withdrawal this card's scope note assumes.

Adjudication (grading comment above stands): the shared value-less-operator set moves to @object-ui/components, next to needsValueInput — placement forced by the dependency graph (plugin-list cannot import from app-shell). Deliverables 1–4 as graded: replace convertFilterGroupToAST's inline two-operator literal with the shared set; viewFilterFold's VALUELESS_FILTER_OPERATORS re-exports/aliases it (its emptyValue parity test kept green); both consumers' parity pins assert against the one set; the measured fresh-row emissions (isNull → [] today) become the regression test asserting a real AST node after.

Mechanism assumptions (from the card, re-verify at your tree): addCondition seeds value: '' and the operator dropdown preserves value; the drop filter exempts only isEmpty/isNotEmpty; needsValueInput excludes six operators; mapOperator maps isnull/isnotnull (both in VALID_AST_OPERATORS) — the bridge is fine, the row never reaches it.

Generated by Claude Code

{
  "issue": 4744,
  "branch": "claude/issue-4744-valueless-row-drop",
  "pr": "https://github.com/objectstack-ai/objectui/pull/4749",
  "pr_number": 4749,
  "head_sha": "dfc67673ee67213f30342d9eb6df887ecbbf1bdd",
  "status": "done",
  "premise_still_valid": true,
  "summary": "The adjudicated route landed as graded. VALUELESS_FILTER_BUILDER_OPERATORS is exported from @object-ui/components next to needsValueInput, which is now defined as its complement so the two cannot drift; convertFilterGroupToAST replaced its inline isEmpty/isNotEmpty literal with it, so a fresh row for any value-less operator survives the incomplete-row drop; app-shell's VALUELESS_FILTER_OPERATORS is now that set spread plus the four canonical spec spellings only it sees. A value-less row emits its real AST node with null in the third slot rather than c.value, because the operator dropdown preserves the previous operator's value and the spec's own convertComparison ignores that slot for $null — so the emission is a function of the operator alone.",
  "mechanism_assumptions_checked": [
    "addCondition seeds value: '' — CONFIRMED, filter-builder.tsx:358-368 ({ id, field: fields[0]?.value || '', operator: 'equals', value: '' })",
    "the operator dropdown preserves value — CONFIRMED, filter-builder.tsx:608-610, onValueChange calls updateCondition(id, { operator: value }) only",
    "the drop filter exempts only isEmpty/isNotEmpty — CONFIRMED, ListView.tsx pre-fix line 414-415",
    "needsValueInput excludes six operators — CONFIRMED, filter-builder.tsx:406-408 (isEmpty, isNotEmpty, isNull, isNotNull, exists, notExists)",
    "mapOperator maps isnull/isnotnull and both are in VALID_AST_OPERATORS — CONFIRMED, ListView.tsx:154-155, and spec/dist/data AST_OPERATOR_MAP has isnull/isnotnull keys (VALID_AST_OPERATORS = new Set(Object.keys(AST_OPERATOR_MAP))); convertComparison lowers both to { [field]: { $null: true|false } } and ignores the third slot",
    "PR #4745's OPT_IN_OPERATORS withdrawal is present at the branch point — CONFIRMED, branched from origin/main at 616a2a547 which is that merge",
    "the bug is invisible to the existing parity guard — MEASURED during ablation leg A: list-offered-operator-expressible-parity.test.ts stayed green with the fix removed, because its probe supplies a value per operator so the incomplete-row drop never fires"
  ],
  "tests": "All at head dfc67673e. Union: pnpm exec vitest run packages/components/ packages/plugin-list/ + app-shell fold surface (viewFilterFold.test.ts, viewFilterFold.emptyValue.test.ts, view-filter-fold.ratchet.test.ts, runtime-metadata-persistence.viewItemSpec.test.ts) + FilterConditionField.operators.test.ts + view-operator-builder-parity.test.ts -> 'Test Files 179 passed (179) / Tests 1849 passed (1849)'. Reverse verification, direction predicted before running, two legs: LEG A (restore the two-operator literal in ListView) predicted red in convertFilterGroupToAST.test.ts only — observed 9 failed / 74 passed, reproducing the card's table verbatim (expected [] to deeply equal [ 'closed_at', 'isnull', null ]; same for isNotNull/exists/notExists; stale-value case emitted [ 'f', 'isnull', 'typed under equals' ]; mixed group collapsed to [ 'stage', '=', 'won' ]). LEG B (give needsValueInput a second condition, || operator === \"isNull\" — the class the old regex scrape could not see) predicted red in the new components render test and green in app-shell's set comparison — observed exactly that, 1 failed / 16 passed. Both legs restored with git checkout from the committed branch; no git stash at any point. No ablation ran against dist: these vitest projects resolve @object-ui/* through root aliases to src (vitest.config.mts:245-271), so no rebuild leg applies.",
  "gates": {
    "targeted_tests": "pass — 179 files / 1849 tests at dfc67673e",
    "type-check": "pass — turbo run type-check --filter components/plugin-list/app-shell, 32 tasks successful (includes the dependency build closure)",
    "lint": "pass — turbo run lint, same three packages, 0 errors (warnings are pre-existing no-explicit-any)",
    "check-changeset-presence": "pass — 7 source files across 3 released packages, 1 changeset declared; check-changeset-fixed and check-changeset-no-major also pass (patch, no major)",
    "check:control-bytes": "pass — 4263 tracked text files scanned; self-scan with grep -naP over the seven touched files also clean",
    "check:phantom-deps": "pass — the new cross-package imports are declared (plugin-list has @object-ui/components as peer+dev, app-shell as a dependency)",
    "check:spec-symbols": "pass",
    "check:i18n-keys / check:i18n-drift / check:i18n-dead-keys": "pass",
    "lint:coverage / type-check:coverage": "pass",
    "ci": "in_progress — reported at draft-PR time per the dispatch contract, not waited on"
  },
  "deviations": [
    "Deliverable 3's 'emptyValue parity test stays green unmodified if possible' was NOT possible, and the reason is structural: that test regex-scraped filter-builder.tsx as TEXT for the literal inside needsValueInput, and the fix's whole point is that the literal no longer exists there. It now imports the shared set (deliverable 4: both pins assert against the one set). The half a scrape could never assert — that the builder really renders no value input for those ids — moved to a new components test, filter-builder-valueless-operators.test.tsx, which drives the real render for every member plus a control case. Ablation leg B exists to show that test discriminates.",
    "A FOURTH hand-written copy of the six ids was found and re-pointed: const VALUELESS in plugin-list's list-offered-operator-expressible-parity.test.ts, used to mirror the fold's treatment. Fixed in place under the 'do not create a third hand-written copy' instruction — same defect class, mechanical, no new verification surface — and named in the PR body rather than folded in silently.",
    "Named the components export VALUELESS_FILTER_BUILDER_OPERATORS, not VALUELESS_FILTER_OPERATORS. app-shell's export of the latter name is a strict SUPERSET (builder ids plus the canonical spec spellings is_empty/is_not_empty/is_null/is_not_null), and two same-named exports where one silently contains more than the other is a footgun at every import site. app-shell keeps its name and derives by spreading the shared set.",
    "The emitted triple's third slot is null rather than c.value for value-less operators. Not in the card: it follows from the card's own mechanism — the operator dropdown preserves value, so an Is null row can carry a leftover string the user can no longer see or edit, and emitting it would put junk on the wire. The spec's convertComparison ignores that slot for $null, and the isEmpty arms already used null. Pinned by an explicit stale-value test.",
    "exists/notExists are kept by the drop-exemption (they are in the shared set) even though mapOperator has no row for them and the emitted node fails isFilterAST. This is unreachable from this toolbar — #4745 withheld them and list-offered-operator-expressible-parity.test.ts forces that equality — and excluding them would have required a second narrower set in plugin-list, i.e. exactly the copy the adjudication forbids. Their emission is pinned so a future decision to offer them lands as a red test.",
    "gh CLI is not installed in this container; the draft PR was opened through the GitHub MCP instead. No other effect."
  ],
  "followups_filed": [],
  "out_of_scope_findings": [],
  "open_questions": []
}

Generated by Claude Code

Navigation Menu

Repository navigation

`ListView.convertFilterGroupToAST` drops a fresh `Is null` / `Is not null` row as incomplete, so the list filter silently applies nothing #4744

What happens

Why it is reachable in the normal flow

Why nothing catches it

Scope note

Suggested shape (not a decision)

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

ListView.convertFilterGroupToAST drops a fresh Is null / Is not null row as incomplete, so the list filter silently applies nothing #4744

Description

What happens

Why it is reachable in the normal flow

Why nothing catches it

Scope note

Suggested shape (not a decision)

Activity

yinlianghui commented on Aug 15, 2026

yinlianghui commented on Aug 15, 2026

yinlianghui commented on Aug 15, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

`ListView.convertFilterGroupToAST` drops a fresh `Is null` / `Is not null` row as incomplete, so the list filter silently applies nothing #4744