Skip to content

fix(spec): the stored-filter conversion's TODO for a null-valued key is true on every block, and no longer says to drop the key - #20709

Merged
objectstack-fleet[bot] merged 9 commits into
mainfrom
claude/issue-20662-null-key-reason
Sep 29, 2026
Merged

objectstack-fleet[bot] merged 9 commits into
mainfrom
claude/issue-20662-null-key-reason

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #20662

Clause-②: no (author-shown wording only; no accept or reject moves)

What changes

The ADR-0087 D2 conversion page-component-filter-record-to-rule-array leaves a record-form filter with a null-valued key as stored and reports it as a TODO, which os migrate meta --stored prints. Its reason said the renderer skips that key, so it "constrains nothing today", and told the operator to "Drop the key". At the .objectui-sha pin dd3f7e1be356 that is true only on a block that queries an object. On a block whose rows are inline, the key selects the rows whose value is null, so dropping it widens the block.

Following triage's direction (comment 5893989151: one wording true on both kinds of block, no "drop the key" advice), the reason now:

  • states what the key does on each kind of block: skipped where the block queries an object, so it constrains nothing; matched where its rows are inline (data: { provider: 'value' } or staticData), so it selects the rows whose value is null;
  • says that no one rule keeps both;
  • names the rule for the rows with no value, {"field":"owner_id","operator":"is_null"} (built from the key), and says that a filter which leaves the key unconstrained has no rule for it. It advises neither rewrite. The choice is the author's.

The verdict does not move. The filter is still declined, left byte-identical and reported as one TODO, on any block. The reason text does not branch on where the rows come from; the conversion never reads that.

Round 2 (seat note 5897754447, a claim amendment): the empty-operator-object reason beside it ({ amount: {} }) said the object "constrains nothing". At the pin the renderer refuses it instead. Where the block queries an object, convertFiltersToAST throws through refuseEmptyOperatorMap (INVALID_FILTER, 400). Where the block's rows are inline, ValueDataSource.find answers no rows through zeroKeyConditionRefusal. The reason and its docblock sentence now say that, say that no rule spells an operator object with no operator, and keep the renderer's own remedy, dropping the key. The verdict does not move.

Files:

  • packages/spec/src/conversions/registry.ts: the reason string in recordFilterToRules, the sentence in its docblock, and the conversion entry docblock's parenthetical in "What is left exactly as stored" ("the renderer skips that key today"). That parenthetical is a fourth copy of the same claim in the same file. It is text only and fixed in place: same defect, same file under this claim, same gates. Round 2 rewrites the empty-operator-object declined reason and its docblock sentence, text only. Round 3 (review 5898951990) drops "a data array" from the null-key reason's inline list: at the pin a bare data array reaches no ValueDataSource.find. Round 4 makes the rationale in the conversion entry docblock's ## Reach paragraph name the inline sources the filter reaches at the pin (data: { provider: 'value' } or staticData), and adds that a bare data array reaches none of them (object-calendar draws it unfiltered; object-map / object-gantt do not take it as a record source). Comment text only; the verdict sentence is unchanged.
  • packages/spec/src/migrations/entries/semantic/18.element-data-source-and-object-block-filter-rule-array.ts: the same claim in the D3 entry's reason. packages/spec/src/migrations/registry.ts is regenerated by gen:migration-registry and not edited by hand. Round 3 narrows the inline list in its older sentence ("None of this depends on where a block's rows come from …") to data: { provider: 'value' } or staticData.
  • packages/spec/src/conversions/page-component-filter-record-to-rule-array.test.ts: the DECLINED_ROWS comment that restated the null-key claim; the all-or-nothing test's comment, which named owner_id: null for a row that is deleted_at: { $null: true }; and two new pins. A null-valued key gets the same reason on an inline-row object-map and an object-bound one. That reason names the is_null rule for the key, and the block's door takes that rule; the control is that the door refuses the stored record. An empty operator object gets the same reason on both blocks, and that reason names INVALID_FILTER, a code in StandardErrorCode.
  • .changeset/20662-null-key-todo-reason.md: @objectstack/spec patch, covering both reasons.

Verification record

Pin reading (dd3f7e1be356, raw source):

  • packages/core/src/utils/filter-converter.ts convertFiltersToAST skips a key whose value is null/undefined (skippedNullKeys).
  • packages/core/src/adapters/ValueDataSource.ts find sends an object $filter to matchesFilter, and its simple-equality arm compares with comparandEquals, which is value === target. Its is_null arm is value === null || value === undefined. Server-side, parseFilterAST lowers is_null to { $null: true }.
  • The inline branches of ObjectMap / ObjectCalendar pass useResolvedFilter(schema.filter) to new ValueDataSource(...).find. filter-tokens.ts resolveContextTokens returns a null value unchanged.
  • The spec: retire the inline-row decline in page-component-filter-record-to-rule-array once the objectui pin carries objectui#10767 #20305 dev's live probe (report 5893209491) measured that find with { owner_id: null } selects only the null row, and not the 'u1' row or the row that has no key.

Lit, at base 31ed067639 (the stored-migration pass and formatStoredMigrationReport, the function os migrate meta --stored prints through, over a one-page stub sys_metadata holding an object-grid that queries deal and an object-map with data: { provider: 'value' }, both filter: { owner_id: null }):

      TODO page-component-filter-record-to-rule-array: {"owner_id":null} left as stored at pages[0].regions[0].components[1].properties.filter — On the `object-map` block `inline`, this filter has the key `owner_id` set to null: the renderer skips a null-valued key, so today it constrains nothing, while an `equals` rule would test for null. Drop the key, or write a rule that tests for null if that is what it should select. Left as stored, it keeps loading unchanged, but it is not the rule-array form its door declares — rewrite it by hand.

The object-grid line carried the same sentence.

Dark, at a51c02fe83 (same probe, spec rebuilt):

      TODO page-component-filter-record-to-rule-array: {"owner_id":null} left as stored at pages[0].regions[0].components[1].properties.filter — On the `object-map` block `inline`, this filter has the key `owner_id` set to null, and what that key selects depends on where the block's rows come from, so no one rule keeps it: where the block queries an object, the renderer skips a null-valued key, so it constrains nothing; where its rows are inline (`data: { provider: 'value' }`, a `data` array or `staticData`), it selects the rows whose `owner_id` is null. Decide which rows it should select: the rows with no `owner_id` value are the rule `{"field":"owner_id","operator":"is_null"}`, and a filter that leaves `owner_id` unconstrained has no rule for it. Left as stored, it keeps loading unchanged, but it is not the rule-array form its door declares — rewrite it by hand.

In both runs the row is still skipped with two TODOs. The probe file was temporary and is not in the diff.

Round 2, empty operator object (same printer probe, with filter: { amount: {} } on the same two blocks). Lit at c5eed1b4d3: "... this filter has the key amount set to an empty operator object, which constrains nothing — and no rule says "nothing". Drop the key. Left as stored, ...". Dark at 3fcedfb564: "... set to an empty operator object, which names the field and no operator, so no rule spells it. The renderer does not ignore it today: where the block queries an object, it refuses the filter (INVALID_FILTER, 400); where its rows are inline, it answers no rows. Drop the key. Left as stored, ...". Both runs: row skipped, two TODOs. Pin reading at dd3f7e1be356: filter-converter.ts:818 calls refuseEmptyOperatorMap, whose FilterOperatorError has code = 'INVALID_FILTER' and httpStatus = 400; ValueDataSource.find answers [] when zeroKeyConditionRefusal returns a refusal.

Round 3, the inline list (the null-key printer probe, rows null / u1 / missing). Lit at 3fcedfb564: "... where its rows are inline (data: { provider: 'value' }, a data array or staticData), it selects the rows whose owner_id is null. ...". Dark at a6e54de377: "... where its rows are inline (data: { provider: 'value' } or staticData), it selects the rows whose owner_id is null. ...". Pin reading at dd3f7e1be356: record-source.ts:303-307 folds staticData to { provider: 'value', items }, and the value branches hand the resolved filter to ValueDataSource.find (ObjectMap.tsx:950-952, ObjectCalendar.tsx:708-710, ObjectTree.tsx:911-913, ObjectGantt.tsx:1009-1010). A bare data array reaches none of them: ObjectCalendar.tsx:387-389, 599-600, 647 draws it with no fetch and no filter, and the view-data arm refuses an array (record-source.ts:179). The round-1 Dark quote above is the a51c02fe83 reading, before this narrowing.

Tests and gates, at 6f1396efa2 (this branch merged with origin/main 671d4c164f through scripts/pm/os-regen-merge.sh; the delta against main is exactly the five files above). Round 4's one-comment commit eaf2d6e6e8 re-ran the conversions and migrations set (1034 passed), spec typecheck, check:generated (15 up to date), check:doc-authoring and check:issue-citations, all green; the derived gate list is unchanged:

  • @objectstack/spec local project: 575 files, 16972 passed, 1 todo. typecheck, including the test layer, passed.
  • repo project, narrowed to the three files that read the conversion and migration registries (conversions-major18-merge, step18-rationale-merge, retired-key-migrate-sentence): 35 passed. The full repo project did not finish inside the foreground cap and is NOT MEASURED locally; CI runs it.
  • check:generated: all 15 artifacts are up to date against a spec rebuilt after the merge.
  • dispatch-gates --commands derived 87 commands. 84 exited 0, including check:migration-registry, check:spec-changes, check:upgrade-guide, check:docs, check:api-surface, check:authorable-surface, check:objectui-pin-citations, check:doc-authoring, check:nul-bytes and check:adr-0087-registration. Three exited 3 with PREREQUISITE NOT MET, because they need a whole-workspace build: check:dual-build-cjs-loads, check:lean-entry-closure and check:type-check-debt. They are NOT MEASURED locally. --ran reconciles 87 derived: 84 run, 3 NOT-MEASURED, 0 unrun.
  • Ablation of the new pin (scripts/ablation-replace.mjs, from the committed state): the anchor operator: isNull })} was replaced so the reason named an equals/null rule instead. The new test went red: expected the reason to contain {"field":"owner_id","operator":"is_null"}. The other five null-related tests stayed green. The file was restored: its blob matches HEAD and git diff HEAD is empty.
  • Round-2 ablation of the empty-operator pin, from committed bcda701b88: the anchor "block queries an object, it refuses the filter (INVALID_FILTER, 400); where its rows " was replaced with "block queries an object, it constrains nothing; where its rows ". The pin went red: expected the reason to contain INVALID_FILTER. The file was restored: its blob matches HEAD f1f29e6f4802 and git diff HEAD is empty.

Acceptance notes

  • No test pinned the false clause. The existing rows assert only the prefix "has the key owner_id set to null", so there was nothing to re-pin. The new pin checks named subjects: the same reason on both blocks, the is_null rule, and that the door takes it. It does not pin prose. The empty-operator reason was the same: only its prefix was asserted.
  • packages/spec/CHANGELOG.md's 17.5.0 entry carries the old null-key sentence. It is a released record and is not edited; the corrected text ships in this PR's changeset.

Generated by Claude Code

…s true on every block

`page-component-filter-record-to-rule-array` declines a record-form filter
with a null-valued key. Its reason said the renderer skips that key, so it
constrains nothing, and to drop it. At the objectui pin that holds only on a
block that queries an object; on a block whose rows are inline,
`ValueDataSource.find` matches the key and selects the rows whose value is
null, so dropping it widens the block.

The reason, its docblocks and the protocol-18 D3 entry now state both
behaviours, name the `is_null` rule for the rows with no value, and leave
which rows to select to the author. The verdict does not move.

Claude-Session: https://claude.ai/code/session_014EJ1ED8X4MMrT18BhVx4tx
Co-authored-by: Claude <[email protected]>
@github-actions github-actions Bot added size/s documentation Improvements or additions to documentation tests tooling labels Sep 29, 2026
@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 2 documentable anchor(s).

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

  • content/docs/deployment/cli.mdx (via is_null (literal, a string literal in recordFilterToRules))
What this run could not see
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 137 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 b291fcdae9ac6dd4152367082905cfe81ddf5fb0 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from d66495a5d4dbaa11130925990f31259b5c6602de — the merge of head eaf2d6e6e8d55e3d164ed72a786ee7d260997a6b into base b291fcdae9ac6dd4152367082905cfe81ddf5fb0, 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 d66495a5d4dbaa11130925990f31259b5c6602de && git checkout d66495a5d4dbaa11130925990f31259b5c6602de
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin b291fcdae9ac6dd4152367082905cfe81ddf5fb0 eaf2d6e6e8d55e3d164ed72a786ee7d260997a6b && git checkout -B drift-repro b291fcdae9ac6dd4152367082905cfe81ddf5fb0 && git merge --no-ff eaf2d6e6e8d55e3d164ed72a786ee7d260997a6b

node scripts/docs-audit/affected-docs.mjs --json b291fcdae9ac6dd4152367082905cfe81ddf5fb0

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

…he key, not that it constrains nothing

`page-component-filter-record-to-rule-array` declines a record-form filter
whose key is an empty operator object (`{ amount: {} }`). Its reason said the
object constrains nothing. At the objectui pin the renderer refuses it:
`convertFiltersToAST` throws INVALID_FILTER (400) through
`refuseEmptyOperatorMap` where a block queries an object, and
`ValueDataSource.find` answers no rows through `zeroKeyConditionRefusal`
where a block's rows are inline. The reason and its docblock now say so and
keep the renderer's own remedy, dropping the key. The verdict does not move.

Also corrects a test comment that named `owner_id: null` for a row that is
`deleted_at: { $null: true }`.

Claude-Session: https://claude.ai/code/session_014EJ1ED8X4MMrT18BhVx4tx
Co-authored-by: Claude <[email protected]>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 3fcedfb564c7750cbaffbc654fc291bd3260886a
Local-runs: none

Inputs: card #20662 (body, triage direction 5893989151, claim 5896419836, round-1 report 5897718691, seat note 5897754447, round-2 report 5898670276), PR #20709 (body, comments, five-file list, net diff against origin/main 5757463712, which is the merge base), the objectui sources at pin dd3f7e1be356 (filter-converter.ts, ValueDataSource.ts, record-source.ts, filter-tokens.ts, useResolvedFilter.ts, ObjectMap.tsx, ObjectCalendar.tsx and its index.tsx, ObjectTree.tsx, ObjectGantt.tsx), and the 42 check-runs on this head, read once.

① Derived judgments

1. The null-key declined reason (recordFilterToRules, the value === null branch) — text plus one local const isNull = 'is_null' satisfies ViewFilterOperator that feeds JSON.stringify; the branch still returns { declined }, the filter is left byte-identical, one TODO. Right, clause by clause, except one enumerated item:

  • "where the block queries an object, the renderer skips a null-valued key, so it constrains nothing" — RIGHT. convertFiltersToAST counts the key in skippedNullKeys and continues; a filter of only such keys lowers to undefined.
  • "where its rows are inline …, it selects the rows whose KEY is null" — RIGHT for rows that reach ValueDataSource.find. The object $filter goes to matchesFilter; null fails the operator-branch guard (condition && typeof condition === 'object') and takes the simple-equality arm, comparandEquals(value, null), which is value === null: an explicit-null row is selected, a row lacking the key is not. The reason says "whose KEY is null", which is exactly that.
  • "the rows with no KEY value are the rule {"field":KEY,"operator":"is_null"}" — RIGHT on both kinds. Inline: the is_null arm is value === null || value === undefined, so it takes the null row and the row lacking the key. Object-bound: the rule lowers to the server's $null: true. The reason does not call this rule equivalent to the stored key ("no one rule keeps it"), which is the truth: on inline rows they differ on a row that lacks the key.
  • "a filter that leaves KEY unconstrained has no rule for it" — RIGHT; absence is the only no-constraint spelling in an AND list.
  • No "drop the key" advice for the null key anywhere: reason, docblock, entry-docblock parenthetical, D3 text, changeset. RIGHT.
  • Reach: at the pin the inline branches pass useResolvedFilter(schema.filter, scope) — resolveContextTokens returns a null value unchanged (if (value == null) return value) and a token-free filter is handed out as authored — straight to new ValueDataSource({ items }).find('', { $filter }) in ObjectMap, ObjectCalendar, ObjectTree, and through resolveDataSource to the same adapter in ObjectGantt. RIGHT.
  • WRONG — the enumeration inside the inline clause, "(data: { provider: 'value' }, a data array or staticData)", repeated in the changeset. data: { provider: 'value', items } (the 'view-data' ladder returns it verbatim on map and gantt) and staticData (every ladder folds it to { provider: 'value', items }) do reach ValueDataSource.find. A bare data array does not, on any block at the pin. object-calendar — the one door at this head that declares data as an array, described "Pre-fetched records — skips the internal fetch" — forwards it as the data prop through resolveExternalData (Array.isArray(raw) ? raw : undefined), hasExternalData returns the fetch effect before any query, and setData(externalData) draws the rows as given: no key of the filter is applied. The object-map and object-gantt doors say "the bare-array shortcut is refused", resolveRecordSourceConfig on the 'view-data' arm does not take an array as a record source, and the map commits a host data prop array without a query. So on a data-array block the null key selects nothing — as does every other key — and the clause is false for that spelling. Bounded: neither rewrite the reason offers moves a row there, because the whole filter is inert. Inherited: the same triple is the card body's own framing and stands, unchanged by this diff, in the D3 entry's next sentence. But this diff is what puts the triple into the reason string os migrate meta --stored prints and into the changeset, under the ruling that the wording be true on the block in front of the operator. This is the FAIL reason; the pins do not assert the enumeration, so the remedy is text only.

2. The recordFilterToRules docblock sentence. Names convertFiltersToAST (skips) and ValueDataSource.find through comparandEquals (matches); says "where a block's rows are inline" without the enumeration; "This entry never reads where a block's rows come from" matches the function (no such read). RIGHT.

3. The conversion entry docblock's parenthetical ("skipped where a block queries an object, matched where its rows are inline — no one rule keeps both"). RIGHT.

4. The D3 entry reason parenthetical and migrations/registry.ts. The parenthetical states both behaviours, names the is_null rule for "the rows with no value", and leaves the choice to the author; it carries no enumeration. RIGHT. The step-18 semantic literal for this id in migrations/registry.ts is identical to the entry file modulo indentation (108 trimmed lines compared) and sits inside the generated region: generator-only. PROTOCOL_MAJOR is 17 at this head; build-spec-changes and build-upgrade-guide loop from the support floor to 17, spec-changes.json holds no element-data-source-and-object-block-filter-rule-array record, and docs/protocol-upgrade-guide.md ends at "Protocol 16 to 17". Major 18 is not projected; both artifacts unaffected. RIGHT.

5. The empty-operator-object reason and its docblock sentence. "names the field and no operator, so no rule spells it" — RIGHT. "where the block queries an object, it refuses the filter (INVALID_FILTER, 400)" — RIGHT: filter-converter.ts:818 calls refuseEmptyOperatorMap, which throws FilterOperatorError with code = 'INVALID_FILTER', httpStatus = 400. "where its rows are inline, it answers no rows" — RIGHT: find's object arm runs zeroKeyConditionRefusal before any row, a zero-key object condition is lowered through toFilterNodeSafely, refused, and result = []. "Drop the key" kept — RIGHT: the renderer's own refusal ends "Choose an operator … or remove the key", and the conversion cannot choose an operator for the author, so the drop is the one remedy it can state. The docblock's refuseEmptyOperatorMap and zeroKeyConditionRefusal exist at the pin in the cited roles. (The same data-array caveat applies to "inline" here, but this reason does not enumerate spellings, so it is only as loose as the word.)

6. Verdict unchanged. The registry.ts diff touches two declined: literals, one new local const, and comment text; no if, no id, no return shape, no other conversion's body. The two new pins assert named subjects: the is_null rule's JSON, the object-map door taking [rule] (zero issues at filter) with the stored record refused as control, the same reason on a staticData block and an objectName block, the filter left byte-identical, and INVALID_FILTER present in StandardErrorCode.options and in the reason. No prose pinned. RIGHT.

7. Test comments (DECLINED_ROWS; the all-or-nothing row, which is deleted_at: { $null: true }). RIGHT. CHANGELOG untouched per the seat's ruling. RIGHT.

Gate coverage (42 check-runs on 3fcedfb564: 36 success, 5 skipped, 1 in progress at the read; no failure):

  • NOT concluded at the read: Check Changeset in run 36629793626 (fired after the PR body edit). The same job on the same head concluded success in run 36626149495. Named, not presumed.
  • Skipped: Build Docs, Console Pin Gate, Packed-tarball smoke (opt-in), and the re-fired Auto Label / Check PR Size of run 36629793626 (their first runs succeeded).
  • The dev's three local NOT MEASURED: check:dual-build-cjs-loads and check:lean-entry-closure live in ci.yml's Build Core — success; check:type-check-debt in lint.yml's Type Check · debt ledger — success. The full spec repo vitest project: Test Core 1/6 to 6/6 — success.
  • check:migration-registry, check:doc-authoring, check:issue-citations, check:generated --reconcile-only: Lint & Repo Gates — success. check:spec-changes, check:upgrade-guide, check:objectui-pin-citations, check:authorable-surface: Type Check · source gates — success. check:api-surface: Type Check · consumer gates — success. check:adr-0087-registration, check:changeset-no-major: Check Changeset (run 36626149495) — success. Governed Surface Queue Guard — success; the five paths touch no governed surface, and the head repo is the base repo.
  • Docs Drift Check (advisory) flagged content/docs/deployment/cli.mdx via the is_null literal; no page under content/docs restates the old or the new reason (searched "constrains nothing", "renderer skips", "Drop the key", "set to null").

② Semver level

.changeset/20662-null-key-todo-reason.md: @objectstack/spec patch. The diff publishes changed author-shown string literals and docblocks in @objectstack/spec; no schema, export, accept set, conversion id or return shape moves, and spec-changes.json and the upgrade guide are unaffected. patch is right; skip-changeset would be wrong, since published text changes.

Clause-②: the PR body reads Clause-②: no (author-shown wording only; no accept or reject moves). Through scripts/pm/clause2-line.mjs, matchValueToken takes no as the first token after the colon; readArmToken sees a parenthetical opening with author, which is neither of CLAUSE2_ARMS nor an arm-family near miss, so it reads { arm: null } — a declared no with no arm, not malformed. The changeset's own last line, Clause-②: no, reads the same. Right: nothing widens, nothing narrows.

③ Boundary flags

Dev flags, round 1 (5897718691):

  • Lit/dark through the stored-migration protocol and formatStoredMigrationReport over a stub engine, not a live database — answered: adequate for a text-only change; the printer is the CLI's own.
  • Fourth copy (entry docblock parenthetical) fixed in place — answered: adopted by the seat in 5897754447; verified in the diff.
  • Connective change in the docblock — superseded by round 2 ("for a different reason:").
  • No test pinned the false clause; one named-subject pin added instead — answered: verified (① 6).
  • Wording: is_null as "the rows with no KEY value", the stored key as "the rows whose KEY is null" — answered: exactly the pin's two arms (=== null || === undefined against === null); right.
  • NOT MEASURED locally: the full spec repo project and three whole-workspace gates — answered by CI: Test Core, Build Core, Type Check · debt ledger, all success.
  • Attribution trailer, origin/main advance, worktree kept — not contract matters.

Dev flags, round 2 (5898670276):

  • Changeset edited beyond the listed surface ("Both filters") — answered: right; the prior "Nothing else changes" sentence would have been false.
  • The pin's named subject is the refusal code, checked against StandardErrorCode.options — answered: verified; INVALID_FILTER is in the enum at this head.
  • Only "Drop the key" kept; "Choose an operator" not added — answered: right per the order, and per what the conversion can state (① 5).
  • Lock retry; lit at c5eed1b4d3 and dark at 3fcedfb564 with an empty spec diffstat between the merges; CHANGELOG untouched; attribution; worktree — not contract matters; the CHANGELOG disposition is the seat's ruling and holds.

open_questions: none in either round. Round-1 out-of-scope findings: the empty-operator reason (folded in round 2; verified), the released CHANGELOG entry (dropped by the seat), the all-or-nothing test comment (folded in round 2; verified).

ESCALATED to the seat:

  • The D3 entry's pre-existing sentence, unchanged at this head and released in 17.5.0, carries the same triple and says the four blocks "match a rule array against those rows" for a data array too — the same over-claim at the pin. Whether it is tightened in the same round under the dev's own bounded in-place exemption (same file, same defect, text only) or left as a released record is the seat's call; the FAIL reason below is confined to the text this diff introduces.

FAIL reason (one):

  • The null-key reason's inline enumeration "(data: { provider: 'value' }, a data array or staticData)", repeated in the changeset, files a bare data array under "it selects the rows whose KEY is null". At the pin no block routes a bare data array through ValueDataSource.find: object-calendar draws it as pre-fetched rows with the internal fetch skipped and no filter applied; object-map and object-gantt refuse the bare array at the door and do not take it as a record source. Remedy: strike "a data array" from the reason's parenthetical and from the changeset, text only — no verdict, id or pin moves, since no pin asserts the enumeration — and re-run the conversion test file.

Implemented-by: claude/issue-20662-null-key-reason
Reviewed-by: session_014EJ1ED8X4MMrT18BhVx4tx

VERDICT: FAIL


Generated by Claude Code

… objectui pin

The null-valued-key TODO reason, the changeset and the protocol-18 D3 entry
listed the inline row sources as `data: { provider: 'value' }`, a `data`
array or `staticData`. At the objectui pin a bare `data` array reaches no
`ValueDataSource.find`: `object-calendar` draws it as pre-fetched rows with
no filter applied, and `object-map` / `object-gantt` do not take it as a
record source. The list now names `data: { provider: 'value' }` and
`staticData` only; `migrations/registry.ts` is regenerated. Nothing else
moves.

Claude-Session: https://claude.ai/code/session_014EJ1ED8X4MMrT18BhVx4tx
Co-authored-by: Claude <[email protected]>
…r reaches at the objectui pin

The rationale under the conversion's `## Reach` verdict said the in-memory
renderers match the inline rows of every listed source. At the objectui pin
they take those rows from `data: { provider: 'value' }` or `staticData`; a
bare `data` array reaches none of them (`object-calendar` draws it unfiltered,
`object-map` / `object-gantt` do not take it as a record source). Comment text
only; the verdict sentence is unchanged.

Claude-Session: https://claude.ai/code/session_014EJ1ED8X4MMrT18BhVx4tx
Co-authored-by: Claude <[email protected]>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: eaf2d6e6e8d55e3d164ed72a786ee7d260997a6b
Local-runs: none

Re-judging after the FAIL at 3fcedfb564 (review 5898951990, one reason). Inputs: card #20662 (body; triage direction 5893989151; claim 5896419836; seat notes 5897754447 and 5898978326; dev reports 5897718691, 5898670276, round 3 5900465814, round 4 5900584237), PR #20709 (body as patched through round 4, comments, the five-file list, the net diff against the merge base 671d4c164f, which is where origin/main was merged last; origin/main has since moved to b291fcdae9 on thirteen files, none of the five), the objectui sources at pin dd3f7e1be356 (record-source.ts, filter-converter.ts, ValueDataSource.ts, filter-tokens.ts, useResolvedFilter.ts, resolveDataSource.ts, ObjectMap.tsx, ObjectCalendar.tsx and plugin-calendar/src/index.tsx, ObjectTree.tsx, ObjectGantt.tsx, the map and gantt registrations), and the 41 check-runs on this head, read once.

① Derived judgments

1. The previous FAIL reason is cleared. "a data array" is gone from every place this PR's text lists the inline sources the filter reaches: the null-key declined literal in recordFilterToRules now reads "(data: { provider: 'value' } or staticData)"; the changeset's first paragraph reads the same; the D3 entry's older sentence ("None of this depends on where a block's rows come from …") reads the same, and its migrations/registry.ts copy matches. The recordFilterToRules docblock ("where a block's rows are inline") and the entry docblock's parenthetical ("matched where its rows are inline") never enumerated, so nothing was owed there. The only remaining bare-array mentions in the five files are the ## Reach paragraph (judged in 6 below), a pre-existing test row that asserts the conversion's verdict on an object-kanban and not the list, and unrelated text. RIGHT.

2. The remaining list is true at the pin. resolveRecordSourceConfig (record-source.ts:291-317) returns an authored data verbatim when it is on the block's declared arm (:176-181: 'view-data' takes a non-array, 'array' takes an array, 'undeclared' takes anything truthy) and folds staticData to { provider: 'value', items } on every ladder (:303-308). The value branches then hand useResolvedFilter(schema.filter, scope) — resolveContextTokens returns a null value unchanged (filter-tokens.ts:171), and a token-free filter is held as authored — to new ValueDataSource({ items }).find('', { $filter }): ObjectMap.tsx:950-952 (arm 'view-data', :188), ObjectTree.tsx:911-913 (arm 'undeclared', :632), ObjectGantt.tsx:1009-1010 through resolveDataSource case 'value' (resolveDataSource.ts:69-73; arm 'view-data', :669), and ObjectCalendar.tsx:708-710 (arm 'array', :481-487). In find's object arm (ValueDataSource.ts:1315-1325) the record goes to matchesFilter; a null condition fails the operator-branch guard (condition && typeof condition === 'object', :1044) and takes the simple-equality arm, comparandEquals(value, null), which is value === target (:433-439, :1060-1062): the explicit-null row is selected, a row lacking the key is not. "it selects the rows whose KEY is null" is exactly that. The is_null arm is value === null || value === undefined (:509-510), so "the rows with no KEY value" is exactly that rule, on both kinds of block (server-side it lowers to $null: true). Where the block queries an object, convertFiltersToAST counts the key in skippedNullKeys and continues (filter-converter.ts:633-635), and a filter of only such keys lowers to undefined (:1029-1031). No "drop the key" advice for the null key anywhere. RIGHT.

3. The calendar precision boundary the dev flags (round 3) — judged: the un-qualified list does not over-claim. On object-calendar the ladder runs on the 'array' arm, so a data: { provider: 'value' } object is off-arm and is never a record source; the spec's own door says the same — ObjectCalendarPropsSchema is a strictObject whose data is z.array(z.unknown()) ("Pre-fetched records — skips the internal fetch") and whose staticData is z.array(z.unknown()), so that spelling is refused at the door and, stored, yields no inline rows (the ladder falls to staticData, then objectName). The reason's sentence is "where its rows are inline (X or Y), it selects the rows whose KEY is null": its antecedent is a block that HAS inline rows from X or Y, and at the pin no block takes either spelling as a record source and then withholds the filter. That is the material difference from the struck bare array, which the calendar takes, draws, and never filters — the antecedent held and the consequent was false. The list is a gloss of "inline" over the four renderers, not a per-block door census, and triage's direction (5893989151) forbids making the reason's text depend on the block kind. So the parenthetical is true wherever it applies, and there is no block in front of an operator on which following it moves the wrong rows. RIGHT, with the boundary recorded here.

4. The recordFilterToRules docblock sentence and the entry docblock's "What is left exactly as stored" parenthetical. Unchanged since the previous review; the docblock names convertFiltersToAST (skips) and ValueDataSource.find through comparandEquals (matches), says "This entry never reads where a block's rows come from" (the function has no such read), and neither enumerates. RIGHT.

5. The D3 entry reason and migrations/registry.ts. The null parenthetical states both behaviours, names the is_null rule for "the rows with no value" and leaves the choice to the author. The older sentence now lists "(data: { provider: 'value' } or staticData)" and says the four blocks "match a rule array against those rows and select the rows the stored form selected": true of the rows those two spellings produce (on the calendar, staticData's only), and the verdict half ("rewritten or left exactly as it would be on a block that queries an object") is true because the conversion never reads the source. Generator-only: the step-18 literal for this id in migrations/registry.ts (line 9333, inside the region the file header marks GENERATED and says to regenerate through gen:migration-registry) equals the entry file modulo indentation — 107 trimmed lines, zero mismatches — and the file's net diff is exactly the entry's 9-line change. main's copy of the entry carries no conversionIds (#20716's optional key), so the merges dropped nothing there. PROTOCOL_MAJOR is still derived from a 17.x PROTOCOL_VERSION; spec-changes.json holds no record for this entry; check:spec-changes and check:upgrade-guide are green on this head. RIGHT.

6. Round 4's ## Reach edit — comment only, verdict sentence unchanged, and now true. The diff's context lines show "A block whose rows ride on the node (data: { provider: 'value' }, a data array, staticData) is rewritten exactly as a block that queries an object" byte-unchanged, which is true for a bare array precisely because the conversion never reads the source. The rationale now says the in-memory renderers "take those rows from data: { provider: 'value' } or staticData" — a disjunction over the four, true as in 2 and 3 — and that "A bare data array reaches none of them: object-calendar draws it as pre-fetched rows with no filter applied, and object-map / object-gantt do not take it as a record source." Verified: plugin-calendar/src/index.tsx:204-205 forwards rest.data only when it is an array; ObjectCalendar.tsx:387-389 sets hasExternalData, :599-600 draws it as given, and :647 returns from the fetch effect, which is the only place queryFilter is read (:710, :793). On the map and gantt the 'view-data' arm rejects an array (record-source.ts:179), the ladder falls through (ObjectGantt.tsx:658-669; the map registration's own data description says a bare array "is not a record source"), and SchemaRenderer stops spreading an authored data key as a prop for object-arm blocks (record-source.ts:337-340). The tree is not named in that gloss; at the pin its 'undeclared' arm returns a truthy array verbatim, the result carries no provider, so neither the object (ObjectTree.tsx:776) nor the value (:872) arm runs and no find happens — "reaches none of them" holds for the tree as well, and the spec's tree door refuses a bare array anyway. The gloss names three of the four; incomplete, not false. RIGHT.

7. The empty-operator-object reason and its docblock sentence. Unchanged since the previous review; re-read at the pin: filter-converter.ts:818 calls refuseEmptyOperatorMap (:517), which throws FilterOperatorError with code = 'INVALID_FILTER', httpStatus = 400 (:84-85); find's object arm runs zeroKeyConditionRefusal before any row (ValueDataSource.ts:1320), a zero-key condition is lowered through toFilterNodeSafely, refused, and result = [] (:1119-1139, :1322-1323). "Drop the key" kept — the renderer's own refusal ends "or remove the key". RIGHT.

8. Verdict unchanged; pins on named subjects. The registry.ts diff touches two declined: literals, one local const isNull = 'is_null' satisfies ViewFilterOperator feeding JSON.stringify, and comment text — no if, no id, no return shape, no other conversion's body. The test file adds one import and two pins that assert named subjects: the is_null rule's JSON, the object-map door taking [rule] at filter with the stored record refused as control, the same reason on a staticData block and an objectName block, the filter left byte-identical, and INVALID_FILTER in StandardErrorCode.options and in the reason. No prose pinned; no pin asserts the list, so round 3 rightly moved none. The DECLINED_ROWS and all-or-nothing comments are right. RIGHT.

9. The merges lost nothing. Five merges of origin/main on the branch (c5eed1b4d3, 3fcedfb564, then round 3's 5e2d4aa584, 38663afe0a, 6f1396efa2); the net diff against the merge base 671d4c164f is exactly the five files (+112 / -21) and no other path moves, so nothing from main was dropped and nothing of the branch's was lost; eaf2d6e6e8 on top of 6f1396efa2 is +6 / -2 in conversions/registry.ts alone. RIGHT.

10. Not this PR's text. .changeset/20305-inline-row-filter-converts.md is untouched by this diff (zero diff lines against the merge base); its bare-array over-claim is the seat's to record in the ACCEPT. The released packages/spec/CHANGELOG.md 17.5.0 entry is untouched per the seat's ruling. The PR body's round-1 Dark quote still shows the old list, but it is labelled the a51c02fe83 reading and the Round 3 paragraph says so — a dated quote of an earlier head's output, not a claim about this one. RIGHT.

Gate coverage (41 check-runs on eaf2d6e6e8 at the read: 30 success, 5 skipped, 6 in progress, 0 failure):

  • NOT concluded at the read, named and not presumed: Lint & Repo Gates (run 36641255250) — the home of check:migration-registry, check:doc-authoring, check:spec-docblock-symbol-anchors, check:issue-citations and check:nul-bytes; Test Core 1/6, 3/6, 4/6, 5/6, 6/6 (run 36641255327) — the vitest projects including the spec repo project (2/6 is success). The dev reports the same five gates run at eaf2d6e6e8 with exit 0 and both jobs green at 6f1396efa2, with a comment-only delta since; that is the dev's report, not this read.
  • Skipped: Build Docs, Console Pin Gate, Packed-tarball smoke (opt-in), and the re-fired Auto Label / Check PR Size of run 36642206081 (their first runs succeeded).
  • Success: Build Core (check:dual-build-cjs-loads, check:lean-entry-closure); Type Check · debt ledger (check:type-check-debt) — the dev's three NOT MEASURED; Type Check · source gates (check:generated --reconcile-only, check:spec-changes, check:upgrade-guide, check:authorable-surface, check:docs, check:objectui-pin-citations — the docblock's pin citation); Type Check · consumer gates (check:api-surface); Type Check · workspace and TypeScript Type Check (the satisfies ViewFilterOperator const compiles); Check Changeset twice (check:adr-0087-registration, check:changeset-no-major, check:empty-changeset); Governed Surface Queue Guard — the five paths touch no governed surface and the head repo is the base repo; Spec property liveness; the three Dogfood shards and their rollup; Dogfood Verify CLI; Temporal Conformance; the card, branch and single-writer guards.
  • Docs Drift Check (advisory) flags content/docs/deployment/cli.mdx via the is_null literal; no page under content/docs restates the old or the new reason (searched "constrains nothing", "renderer skips", "Drop the key", "set to null" — only unrelated hits in flows.mdx and batch.mdx).

② Semver level

.changeset/20662-null-key-todo-reason.md: @objectstack/spec patch. The diff publishes changed author-shown string literals and docblocks in @objectstack/spec; no schema, export, accept set, conversion id or return shape moves, and spec-changes.json and the upgrade guide are unaffected. patch is right; skip-changeset would be wrong, since published text changes. The changeset's body covers both reasons and states the verdicts do not move.

Clause-②: the PR body reads Clause-②: no (author-shown wording only; no accept or reject moves). Read through scripts/pm/clause2-line.mjs: matchValueToken takes no as the first token after the colon; readArmToken sees a parenthetical opening with author, which is neither of CLAUSE2_ARMS nor an arm-family near miss, so it reads { arm: null } — a declared no with no arm, not malformed. The changeset's last line, Clause-②: no, reads the same. Right: nothing widens, nothing narrows.

③ Boundary flags

Dev flags, round 3 (5900465814):

  • The docblock sentence and the entry parenthetical carry no enumeration, so nothing was struck there — answered: verified (① 4).
  • The ## Reach rationale left for the seat under "Nothing else moves" — answered: the seat ordered it in round 4; verified (① 6).
  • Calendar precision boundary (a data: { provider: 'value' } object is not a record source there; only staticData is) — answered: judged in ① 3; the un-qualified list is true wherever its antecedent holds and no block at the pin contradicts it.
  • Three merges instead of one, and main moving on to b291fcdae9 unmerged — answered: net diff exactly the five files (① 9); the merge base is unchanged and the queue rebuilds on current main.
  • The §1 INLINE test row with a bare data array on object-kanban asserts the conversion's verdict, not the list — answered: verified at head; right to leave.
  • Attribution, worktree, no MCP calls — not contract matters.

Dev flags, round 4 (5900584237):

  • Two docblock-reading gates run beyond the listed set — answered: the pin-citation gate is green on this head (Type Check · source gates); the symbol-anchor gate's home is not concluded at the read (named above).
  • No PR body edit by the dev — answered: the seat applied the round-3 and round-4 pr_body_lines; the body's files list and verification record match the diff.

open_questions: none in rounds 3 or 4. Round-3 out-of-scope finding: the same bare-array over-claim in the ## Reach rationale (folded in round 4; verified) and in the landed .changeset/20305-inline-row-filter-converts.md — confirmed untouched by this diff; not this PR's text; the seat records it in the ACCEPT.

Everything the previous review judged right is still right at this head: the null-key and empty-operator reasons, the is_null rule, the verdict unchanged, the pins on named subjects, generator-only regeneration, patch, Clause-②: no.

ESCALATED: none.

FAIL reasons: none.

Implemented-by: claude/issue-20662-null-key-reason
Reviewed-by: session_014EJ1ED8X4MMrT18BhVx4tx

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review September 29, 2026 23:06
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Sep 29, 2026
Merged via the queue into main with commit 99786f9 Sep 29, 2026
44 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-20662-null-key-reason branch September 29, 2026 23:29
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/m tests tooling

Projects

None yet

2 participants