Skip to content

docs(objectql): reword the #9625 cascade escalation justification after #9476 landed - #9818

Merged
os-elon merged 1 commit into
mainfrom
claude/issue-9781-cascade-comment-stale-premise
Aug 19, 2026
Merged

docs(objectql): reword the #9625 cascade escalation justification after #9476 landed#9818
os-elon merged 1 commit into
mainfrom
claude/issue-9781-cascade-comment-stale-premise

Conversation

@os-elon

Copy link
Copy Markdown
Collaborator

Fixes#9781

Two comments justified the #9625 escalation — set_null on a required FK escalating to restrict — partly by a clause that is now false:

today [] still satisfies required in the record validator (#9476), so this blanket refusal is what keeps an emptied required set from landing silently

The #9476 enforcement landed as 6a5e6adf1, so [] on a required multi-value field is rejected today. isEmptyForRequired in packages/objectql/src/validation/record-validator.ts is that judgment, and both required read sites call it.

What changed

Prose only, at the two sites the card names:

  • packages/objectql/src/engine.ts — the cascade-delete comment above the set_null + required escalation.
  • packages/objectql/src/engine-cascade-delete.test.ts — the comment inside the [#9625] refuses a required MULTI-VALUE lookup ... pin.

The escalation itself is untouched, and so are the pin's assertions.

The justification survives — and it never rested on the validator's tolerance

Checked rather than assumed, because a replacement clause that is subtly false would be worse than the stale one:

  • The escalation's own rationale is the paragraph directly above it in engine.ts — a required FK cannot be nulled, so set_null would issue an UPDATE the child's required-field validator rejects, naming a field that is not even on the object being deleted. That paragraph is independent of array emptiness and is still true.
  • The stale clause propped something narrower: why the refusal is left BLANKET for the multi-value case. That one support is genuinely gone, and the reword records that rather than swapping in a new claim of the same shape.

Two things I deliberately did NOT write, having measured them false or unsupportable:

  • "the refusal fires before any write" — the card's suggested wording, and not true as stated. cascadeDeleteRelations walks relations sequentially and writes per relation, and planCascadeAtomicity returns split for a cross-datasource cascade, where earlier relations commit before a later restrict throws. Both comments now say the refusal fires before THIS relation's own set_null write, which is what actually holds.
  • The claim that the validator would now catch the emptied set was verified, not inferred: the __referentialFieldClear marker on that write exempts only plugin-security's ownership-anchor guard (security-plugin.ts:2057), it does not skip record validation.

Verification at 493cebfa4

The diff is comment-only, proven mechanically rather than by eye — every changed line is a comment line:

git diff -U0 | grep -E '^[+-]' | grep -vE '^(\+\+\+|---)' | grep -vE '^[+-][[:space:]]*//'
(no output)

Gates re-derived with node scripts/pm/dispatch-gates.mjs (no paths passed, so the script takes its own change set from the merge base), all run at 493cebfa4:

gateresult
pnpm check:durability-log-levelexit 0
pnpm check:stack-collection-mapsexit 0
node scripts/check-engine-split-ratio.mjsexit 0
node scripts/docs-audit/check-affected-docs.mjsexit 0
pnpm check:query-options-erasureexit 0 — ratchet holds, no files added
pnpm check:engine-double-contractexit 0 — 321 pinned, no new double
pnpm check:where-matcherexit 0 — 257 matchers, none new
pnpm check:type-check-coverageexit 0

Package level, after pnpm --filter "@objectstack/objectql^..." build:

  • vitest run src/engine-cascade-delete.test.ts — 1 file, 9 tests passed
  • tsc --noEmit in packages/objectql — exit 0, zero errors

pnpm check:type-check-debt (the --re-measure ratchet half) is left to CI: it needs the whole workspace closure built and re-runs tsc across every ledger entry. The reading taken in its place is direct and covers the entry these paths touch — objectql's own tsc --noEmit is clean, the structural half of that gate is green above, and the diff neither adds nor removes an @ts-expect-error / @ts-ignore / @ts-nocheck, which is the only comment-borne construct that can move a tsc count.

Changeset

No changeset; the skip-changeset label instead — taken from the repo's own gate rather than assumed. changeset-check in pr-automation.yml recognises exactly two exemptions, and the label is the one that applies here. The empty-frontmatter route is shut in this repo: the step below the counter rejects an empty changeset a PR newly introduces (#5471), so choosing it would trade one red for another. Nothing user-visible changes — comments never reach dist.


Generated by Claude Code

#9476 landed
Two comments justified the required-FK set_null -> restrict escalation partly
by the then-true clause that an empty array still satisfied `required` in the
record validator. #9476's enforcement landed, so that clause is false: `[]` on
a required multi-value field is rejected today.
Prose only -- no executable line changes, the escalation stays exactly as
ruled, and the test pin's assertions are untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅

What this run could not see

Coarse fallback — 14 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 e124e58f9525e278bba7583cd9a505ab5681d92cpackageMentionDocs.

Which tree this was computed on

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

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@os-elonos-elon added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 19, 2026 — with Claude
@os-elon
os-elon marked this pull request as ready for review August 19, 2026 03:01
@os-elon
os-elon added this pull request to the merge queueAug 19, 2026
Merged via the queue into main with commit 88a4c7aAug 19, 2026
29 checks passed
@os-elon
os-elon deleted the claude/issue-9781-cascade-comment-stale-premise branch August 19, 2026 03:16
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Two #9625 cascade comments will state a stale premise once #9476 enforcement lands — "today [] still satisfies required in the record validator"

2 participants

@os-elon@claude