You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit 7465eeb
Browse filesBrowse the repository at this point in the historyBrowse files
fix(formula,objectql): name the working guard for an optional lookup in both traversal refusals (#20049)
Fixes#20007
Clause-②: no
## What was wrong
An author who wants to refuse an order whose OPTIONAL `line` lookup
points at a secret line, and say nothing about an order with no line,
was sent in a circle by two refusals:
1. `record.line != null && record.line.kind == 'secret'` reads `line`
both through the relationship and as a plain value, so the formula
conflict check refuses it (in `@objectstack/lint` at authoring time and
in the engine at write time). That refusal prescribed "Compare the id
explicitly: write `record.line.id` for the value comparison".
2. `record.line.id != null && record.line.kind == 'secret'` also reads
through `line`. So an order with no line is refused before evaluation as
"no single related record" (`resolveTraversalScope` gets a
`no-reference` binding). That prescription said "Guard the rule on the
reference being set, make it required, or …" and named no spelling for
the guard.
The spellings that work, a `conditional` wrapper or `required: true`,
were named in neither refusal.
## What changed
Only the prescription text. Every refusal keeps its verdict, its
`VALIDATION_FAILED` error, its `rule_violation` field error and its
`constraint`, and the same writes are refused.
- `packages/formula/src/relationship-traversal.ts`: the
`bare-and-traversed` message now says `.id` compares ids and is not a
null guard. For a plain value that tests for empty, it names the guard
and `required: true`.
- `packages/objectql/src/validation/rule-validator.ts`, the
`no-reference` arm of `traversalRefusal`: names the guard through the
landed `referenceGuardRepair(field)`, says `record.FIELD.id != null` is
no guard, and names `required: true`. The multi-value macro clause is
kept.
Engine texts at this head, printed from the built
`@objectstack/objectql` dist. The runtime text spells `RELATED_FIELD`
below as the placeholder `related field` in angle brackets.
```text
[mixed shape, worded by formula] … To compare the id, write `record.line.id` for the value comparison, and keep `record.line.RELATED_FIELD` for the traversal. `record.line.id` is not a null guard: it reads through `line` too, and a rule that reads through an empty `line` rejects the write instead of being skipped. If the plain value tests for empty, take that test out of this expression. To skip the rule while `line` is empty, guard it on `line` being set: make it the `then` of a `conditional` rule whose `when` is `record.line != null`. To refuse an empty `line`, make `line` required (`required: true`).
[no single related record, worded by objectql] … A predicate resolves ONE hop through a single reference. To skip the rule while `line` is empty, guard it on `line` being set: make it the `then` of a `conditional` rule whose `when` is `record.line != null` — `record.line.id != null` inside the rule is no guard, as it reads through `line` too. To refuse an empty `line`, make `line` required (`required: true`). For a multi-value reference, test it with a macro (`exists`, `size`) instead of reading through it.
```
## Premise check and PM hypotheses (measured at `origin/main`
`b3735968ba`)
- **H1 holds.** The new `#20007` block in
`engine-predicate-relationship.test.ts` was committed first
(`ae97576f7`) and run against the unchanged source: `2 failed | 2
passed`. Step 1: the natural spelling is refused as the mixed shape, and
the text lacked the guard. Step 2: `record.line.id != null && …` on an
empty `line` is refused before evaluation with a fault that ends `no
single related record`, and the text named no spelling. Both failures
were on text assertions. The code, field code and constraint assertions
above them passed.
- **H2 holds.** Two tests were green on the unchanged source, so the
text change is all this card needed:
- A `conditional` rule with `when: 'record.line != null'` wrapping
`record.line.kind == 'secret'` accepts an insert with no line, `line:
null` and a public line. It refuses a secret line with the rule's own
message (field `_record`, `rule_violation`, no `unevaluable`
constraint). On update, it refuses repointing an empty order at a secret
line and accepts clearing a set one.
- With `required: true`, an insert with no line is refused with exactly
one field error, `['line', 'required']`, because the rule is never
reached. A public line is accepted and a secret line is refused by the
rule.
- The wrapper parses as a spec `ValidationRule`
(`@objectstack/spec/data`, `safeParse` ok). Control: the same wrapper
without its `message` fails with `invalid_type` at `message`.
- **H3, the consumers of the formula text:** `findTraversalConflicts`
has two callers.
- `validateExpression` (formula) is used by `@objectstack/lint`'s
`validateStackExpressions` at `rule.condition`, the only site that
passes `traversalHydration: true`. Its output text changes. A probe
against the built lint dist gave: the natural spelling, one error with
the new text; the `.id` spelling, 0 issues (lint cannot know a reference
will be empty); the wrapped rule, 0 issues.
- ObjectQL's `resolveTraversalScope` passes the message through into
`detail`, so the engine text changes.
- Lint and objectql dists do not bundle the formula text: 0 hits for it
in `packages/lint/dist` and `packages/objectql/dist`. Control: 2 hits in
`packages/formula/dist`. The text ships from formula alone.
- Repo-wide grep for tests asserting the old texts ("Compare the id",
"no single related record", "MULTIPLE references", "make it required",
"value comparison" and more): hits only in the three suites this PR
edits. No lint test asserts the formula text.
- **H4, where the one wording lives:** formula may not import objectql,
and exporting the sentence from `@objectstack/formula` would add a
public export. That would widen the public surface, so the claim's
`Clause-②: no` would not hold. So the wording exists twice, and a test
holds the copies equal:
- formula has a module-local `referenceGuardRepair(root, field)`, not
exported;
- objectql keeps its landed module-local `referenceGuardRepair(field)`.
- Each docblock names the other copy and the pin.
`engine-predicate-relationship.test.ts` asserts one literal `GUARD` in
the engine refusal worded by formula (step 1) and in the one worded by
objectql (step 2).
- Ablation below: mutating only the formula copy turns step 1 red and
leaves step 2 green.
## Tests (at `0f4c443ac`)
- `@objectstack/formula`: `vitest run` 35 files / 978 passed;
`typecheck` exit 0.
- `@objectstack/objectql`:
- `--project local` 311 files / 5266 passed (run at `b8801356b`; since
then only one test's assertions changed, and that file re-ran 42/42 at
this head);
- `test:repo` 5/5;
- `typecheck` exit 0 (`check:test-typecheck` OK, ledger unchanged).
- `@objectstack/lint` (consumer): `vitest run` 108 files / 4138 passed,
with formula rebuilt.
- New pins:
- formula: the conflict message and the `validateExpression` refusal
name the guard and `required: true`, and both halves of the guarded rule
validate;
- objectql rule level: the `no-reference` case and the mixed case name
the guard;
- objectql engine end to end: the four `#20007` cases above.
- **Reverse verification:** the 2 red / 2 green run on the unchanged
source is quoted under H1.
- **Ablation** (through `dist`, because the objectql to formula import
is an unaliased pair in `KNOWN_UNALIASED_TEST_IMPORTS`).
`scripts/ablation-replace.mjs` rewrote `` whose `when` is ` `` to
`IS_ABLATED` in the formula copy only: anchor 1 to 0, blob
`002192dc9337` to `324bfd327a45`. After a formula build,
`ablation-dist-preflight` reported the marker present in 2 built files.
Results:
- objectql step 1 red, steps 2 to 4 green (`1 failed | 3 passed`);
- the two new formula text pins red (`2 failed | 30 passed`).
- **Restore:** the blob equals HEAD and `git diff HEAD` is empty. After
a rebuild, the marker is absent from all 6 built files and the tree is
clean. The pins are green again: objectql 4/4, formula 32/32.
## Gates
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` at `0f4c443ac` (merge base `b3735968b`)
derives 63 commands; 61 exit 0.
- **NOT MEASURED** (exit 3, PREREQUISITE NOT MET): `pnpm
check:dual-build-cjs-loads` and `pnpm check:type-check-debt`. Both read
the whole workspace's built `dist`, which needs a full `pnpm build` that
this seat did not run on the shared box. This diff changes string
literals and tests only. CI builds and runs both.
- `--ran`: `63 derived famil(ies) accounted for — 61 run, 2 NOT-MEASURED
(2 DERIVED from a recorded exit 3)`.
- `check:error-code-casing` first went red on a new test line
(`toMatchObject({ code: 'rule_violation' })`). The assertion now uses
the envelope shape the neighbouring pins use, and the gate exits 0.
- `node scripts/check-issue-citations.mjs --base b373596`: exit 0, 4
citations resolve.
- `pnpm check:nul-bytes`: exit 0.
- Narrowed ESLint at `0f4c443ac`: `eslint --no-inline-config --format
json` over the 5 changed `.ts` files gives 5 files, 0 errors and 0
warnings. `--print-config` shows each file is in the population with
`parserOptions.project` and `projectService` null. `eslint.config.mjs`
states that type-aware linting is never enabled, so this diff cannot
move a verdict on any untouched file. The full `pnpm lint` run belongs
to CI.
## Acceptance notes
- The pending `.changeset/18682-predicate-relationship-traversal.md`
table still reads "`record.account.id == 'acc_1'` for the value
comparison". That is correct for its example, which is an id comparison
and not a null test, so it is left alone.
- The shape "already holds an expanded record rather than an id" in the
`no-reference` arm still names no repair of its own. It is unchanged
here and outside this card.
- The engine's `ValidationError` carries `code: 'VALIDATION_FAILED'` and
`fields`, and no `status`. The REST layer maps the status, and this PR
does not change it.
- Out of this PR's surface, per the claim: `requiredWhen` /
`readonlyWhen` fault arms, `unevaluableRuleError`,
`packages/lint/src/validate-null-guards.ts` (draft PR #20028) and
`packages/spec`.
- Changeset: `@objectstack/formula` patch and `@objectstack/objectql`
patch.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
fix(formula,objectql): the two refusals a traversing validation rule on an optional lookup meets now name the repairs that work — a `conditional` wrapper or `required: true` (#20007)
7
+
8
+
Clause-②: no
9
+
10
+
An author who wants to refuse a write when an OPTIONAL lookup is set and its related record is secret writes `record.line != null && record.line.kind == 'secret'`. Two refusals then sent them in a circle:
11
+
12
+
1. That expression reads `line` both through the relationship and as a plain value, which cannot be served, and is refused. The refusal said to "compare the id explicitly" and write `record.line.id` for the value comparison.
13
+
2.`record.line.id != null && record.line.kind == 'secret'` reads through `line` too, so an order with no line is refused before the rule is evaluated, as "no single related record". That refusal said to "guard the rule on the reference being set" and named no spelling for the guard.
14
+
15
+
Which writes are refused is unchanged, and so are the error, the `rule_violation` field error and its `constraint` (`reason: 'unevaluable'` and the fault). `@objectstack/lint` passes the formula refusal through unchanged, so it shows the new text too. Only the prescriptions change. Both now name the two spellings measured to work for an optional reference, and the guard is worded exactly as in the delete-cleanup refusal:
16
+
17
+
```text
18
+
… To compare the id, write `record.line.id` for the value comparison, and keep
19
+
`record.line.<related field>` for the traversal. `record.line.id` is not a null guard: it
20
+
reads through `line` too, and a rule that reads through an empty `line` rejects the write
21
+
instead of being skipped. If the plain value tests for empty, take that test out of this
22
+
expression. To skip the rule while `line` is empty, guard it on `line` being set: make it
23
+
the `then` of a `conditional` rule whose `when` is `record.line != null`. To refuse an
24
+
empty `line`, make `line` required (`required: true`).
25
+
```
26
+
27
+
```text
28
+
… A predicate resolves ONE hop through a single reference. To skip the rule while `line`
29
+
is empty, guard it on `line` being set: make it the `then` of a `conditional` rule whose
30
+
`when` is `record.line != null` — `record.line.id != null` inside the rule is no guard, as
31
+
it reads through `line` too. To refuse an empty `line`, make `line` required
32
+
(`required: true`). For a multi-value reference, test it with a macro (`exists`, `size`)
33
+
instead of reading through it.
34
+
```
35
+
36
+
The repair as an author writes it, measured end to end on insert and update. It accepts an order with no line or a public line, and refuses a secret line with the rule's own message:
0 commit comments