Repository navigation
Commit 7465eeb
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>1 parent 0bf85ea commit 7465eeb
6 files changed
Lines changed: 284 additions & 5 deletions
File tree
- .changeset
- packages
- formula/src
- objectql/src
- validation
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
121 | 121 | | |
122 | 122 | | |
123 | 123 | | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
124 | 143 | | |
125 | 144 | | |
126 | 145 | | |
| |||
189 | 208 | | |
190 | 209 | | |
191 | 210 | | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
192 | 234 | | |
193 | 235 | | |
194 | 236 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
221 | 221 | | |
222 | 222 | | |
223 | 223 | | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
224 | 243 | | |
225 | 244 | | |
226 | 245 | | |
| |||
250 | 269 | | |
251 | 270 | | |
252 | 271 | | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
253 | 278 | | |
254 | 279 | | |
255 | 280 | | |
| |||
259 | 284 | | |
260 | 285 | | |
261 | 286 | | |
262 | | - | |
263 | | - | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
264 | 295 | | |
265 | 296 | | |
266 | 297 | | |
| |||
Lines changed: 136 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
735 | 735 | | |
736 | 736 | | |
737 | 737 | | |
| 738 | + | |
| 739 | + | |
| 740 | + | |
| 741 | + | |
| 742 | + | |
| 743 | + | |
| 744 | + | |
| 745 | + | |
| 746 | + | |
| 747 | + | |
| 748 | + | |
| 749 | + | |
| 750 | + | |
| 751 | + | |
| 752 | + | |
| 753 | + | |
| 754 | + | |
| 755 | + | |
| 756 | + | |
| 757 | + | |
| 758 | + | |
| 759 | + | |
| 760 | + | |
| 761 | + | |
| 762 | + | |
| 763 | + | |
| 764 | + | |
| 765 | + | |
| 766 | + | |
| 767 | + | |
| 768 | + | |
| 769 | + | |
| 770 | + | |
| 771 | + | |
| 772 | + | |
| 773 | + | |
| 774 | + | |
| 775 | + | |
| 776 | + | |
| 777 | + | |
| 778 | + | |
| 779 | + | |
| 780 | + | |
| 781 | + | |
| 782 | + | |
| 783 | + | |
| 784 | + | |
| 785 | + | |
| 786 | + | |
| 787 | + | |
| 788 | + | |
| 789 | + | |
| 790 | + | |
| 791 | + | |
| 792 | + | |
| 793 | + | |
| 794 | + | |
| 795 | + | |
| 796 | + | |
| 797 | + | |
| 798 | + | |
| 799 | + | |
| 800 | + | |
| 801 | + | |
| 802 | + | |
| 803 | + | |
| 804 | + | |
| 805 | + | |
| 806 | + | |
| 807 | + | |
| 808 | + | |
| 809 | + | |
| 810 | + | |
| 811 | + | |
| 812 | + | |
| 813 | + | |
| 814 | + | |
| 815 | + | |
| 816 | + | |
| 817 | + | |
| 818 | + | |
| 819 | + | |
| 820 | + | |
| 821 | + | |
| 822 | + | |
| 823 | + | |
| 824 | + | |
| 825 | + | |
| 826 | + | |
| 827 | + | |
| 828 | + | |
| 829 | + | |
| 830 | + | |
| 831 | + | |
| 832 | + | |
| 833 | + | |
| 834 | + | |
| 835 | + | |
| 836 | + | |
| 837 | + | |
| 838 | + | |
| 839 | + | |
| 840 | + | |
| 841 | + | |
| 842 | + | |
| 843 | + | |
| 844 | + | |
| 845 | + | |
| 846 | + | |
| 847 | + | |
| 848 | + | |
| 849 | + | |
| 850 | + | |
| 851 | + | |
| 852 | + | |
| 853 | + | |
| 854 | + | |
| 855 | + | |
| 856 | + | |
| 857 | + | |
| 858 | + | |
| 859 | + | |
| 860 | + | |
| 861 | + | |
| 862 | + | |
| 863 | + | |
| 864 | + | |
| 865 | + | |
| 866 | + | |
| 867 | + | |
| 868 | + | |
| 869 | + | |
| 870 | + | |
| 871 | + | |
| 872 | + | |
| 873 | + | |
Lines changed: 8 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
248 | 248 | | |
249 | 249 | | |
250 | 250 | | |
| 251 | + | |
| 252 | + | |
251 | 253 | | |
252 | 254 | | |
253 | 255 | | |
| |||
272 | 274 | | |
273 | 275 | | |
274 | 276 | | |
275 | | - | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
276 | 283 | | |
277 | 284 | | |
278 | 285 | | |
| |||
0 commit comments