Repository navigation
fix(types,rest): a /data write's NOT NULL refusal names the field by its label - #24759
Conversation
…its label (WIP) The REST /data create and update doors relabel the NOT NULL refusal the classification builds from the driver's text: the field's label in the caller's locale, and for a field declared storage.notNull a hint that prescribes neither required nor os migrate. The classification's own hint stops prescribing required. Claude-Session: https://claude.ai/code/session_01SYEJqDL3NXxrJRBVqaMu9T Co-authored-by: Claude <noreply@anthropic.com>
The door pins run on a real driver: PATCH and create name the field by its label (and in zh-CN by its translated label, in the validator's own catalog sentence), a declared storage.notNull column gets a hint without required or os migrate, and the drift and undeclared-column controls keep today's answer. The classification's half and the label helper's declines are unit pinned. The 20963 hint pin drops the required prescription it asserted. Claude-Session: https://claude.ai/code/session_01SYEJqDL3NXxrJRBVqaMu9T Co-authored-by: Claude <noreply@anthropic.com>
…stack/rest Claude-Session: https://claude.ai/code/session_01SYEJqDL3NXxrJRBVqaMu9T Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 2 package(s): 5 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 20 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 933ab24e9ceadb6dadedb02aeb7f902fd3e0050e && git checkout 933ab24e9ceadb6dadedb02aeb7f902fd3e0050e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 580fd4ed3c98a1752dfb0041b45cc713644fef7e 055d54575682dc859c8b245ad882b24febb616cf && git checkout -B drift-repro 580fd4ed3c98a1752dfb0041b45cc713644fef7e && git merge --no-ff 055d54575682dc859c8b245ad882b24febb616cf
node scripts/docs-audit/affected-docs.mjs --json 580fd4ed3c98a1752dfb0041b45cc713644fef7e
|
The NOT NULL relabel adds three exports to @objectstack/types' entry (notNullRefusalSite, relabelNotNullRefusal, type NotNullRefusalSite), consumed by @objectstack/rest's /data doors: an additive public-surface widening. No accept set, status or code moves. Claude-Session: https://claude.ai/code/session_01SYEJqDL3NXxrJRBVqaMu9T Co-authored-by: Claude <noreply@anthropic.com>
席位核对 — ACCEPT (the lane seat's review of record for a
|
Fixes #24715
Clause-②: yes (widening: three additive exports,
notNullRefusalSite,relabelNotNullRefusaland the typeNotNullRefusalSite, consumed by@objectstack/rest's/datadoors; no accept set, status or code moves)Basis, per package:
@objectstack/typeswidens its published surface by exactly those three names, which reach its built entry throughexport *(packages/types/src/index.ts:98). Measured on the builtdist/index.d.tsexport list,origin/main580fd4ed3against this branch: 122 → 125 names, three added, none removed. Its changeset is thereforeminor,Clause-②: yes (widening).@objectstack/restchanges message text only: the refusal'sstatus,codeandfields[]keys and codes do not move, and its new module is not on its entry. Its changeset stayspatch,Clause-②: no.What changes
A field declared
storage: { notNull: true }withoutrequiredis ADR-0113's engine-populated column. The record validator judgesrequiredonly, so a write with no value for it reaches the driver and the database refuses it.mapDataErrorclassifies that refusal from the driver's text, which names the column and nothing else.@objectstack/types. The NOT NULL branch stays field-map-free (per triage, no field map is threaded into it). It now brands the body it builds. The caller that holds the field map usesnotNullRefusalSite(answer)to recognise that answer, and no lookalike, and gets back the column and the table where the dialect names one (SQLite, Postgres). It callsrelabelNotNullRefusal(answer, { message, declaresNotNull })to swap the sentence and, for a declaredstorage.notNullfield, the hint. The branch's own hint no longer adds "or declare the fieldrequired". That hint is what every door that does not relabel still serves, so it must hold for both ways in: a declared NOT NULL column and drift.@objectstack/rest,POST /data/:objectandPATCH /data/:object/:id. The catch relabels a NOT NULL refusal (not-null-refusal-label.ts). The field is named the way the record validator names its ownrequiredrefusal. The validator's builder is not on@objectstack/objectql's public entry, andrestdoes not depend on the engine at run time, so no new export inpackages/objectqlwas needed or made. Every piece that builder composes is a@objectstack/spec/systemexport, and this door reuses them:requiredsentence throughrenderValidationMessage, with thevalidation.field.requiredoverride honoured;t(objectFieldLabelKey(...)), then the declaredlabel, then the API name;ExecutionContext.locale(the one the engine's validator was given), negotiated byresolveBundleLocaleagainst the i18n service's locales, as the engine does./datarequest. That read is now one private method,objectItemReader, shared by both callers. The exposure gate's behaviour is unchanged.status,code,fields[0].field,fields[0].code,objectand the key set, on the body and on the field.Before and after, at both
/datadoorsMeasured on a real store:
SqlDriveron better-sqlite3:memory:, the realObjectQLandObjectStackProtocolImplementation. The field isunit_price(label "Sales Price"), declaredstorage: { notNull: true }with norequired.RestServerdoor, before (origin/main7e7e605de).PATCH /api/v1/data/OBJECT/IDwith{ "unit_price": null }, andPOSTwith no value, both answered:{"status":400,"body":{"error":"unit_price is required","code":"VALIDATION_FAILED","fields":[{"field":"unit_price","code":"required","message":"unit_price is required"}],"hint":"The database column for 'unit_price' requires a value: provide it, or declare the field `required` in the object metadata. If the object declares neither `required` nor `storage: { notNull: true }` for 'unit_price', the physical schema has drifted from metadata instead: run 'os migrate' to reconcile (or reset the dev database).","object":"line_item_probe"}}RestServerdoor, after (this branch).PATCHandPOSTboth answer:{"status":400,"body":{"error":"Sales Price is required","code":"VALIDATION_FAILED","fields":[{"field":"unit_price","code":"required","message":"Sales Price is required"}],"hint":"The field 'unit_price' is declared `storage: { notNull: true }`, so its database column requires a value: send one, or omit the field when a default value or a hook populates it (an update that omits it keeps the stored value).","object":"line_item_24715"}}zh-CNcaller with azh-CNbundle reads"error":"销售价格不能为空". That is the validator's own zh-CNrequiredtemplate; a barezhnegotiates to the same text.drift_col, declared with neitherrequirednorstorage.notNullover a NOT NULL column):"error":"Drift Column is required", and the classification's hint, which leads with "requires a value: provide one" and keeps the conditionalos migratesentence.gone_col):"error":"gone_col is required", with the same hint.Dispatcher
/datadoor (packages/runtime), before = after. Its premise does not hold: this door never answered theVALIDATION_FAILED/requiredenvelope.domains/data.tsdoes not catch, soHttpDispatcher.dispatch('PATCH', '/data/OBJECT/ID', { unit_price: null })lets the rawSqliteErrorout ofdispatch(). That error carriescode: SQLITE_CONSTRAINT_NOTNULL, no status, and the messageupdate [statement and bound values redacted] - NOT NULL constraint failed: OBJECT.unit_price.declaredEnvelopeForThrowdeclines it.resolveThrownHttpErroranswers500 INTERNAL_ERROR, andlooksLikeInternalErrorLeakis true for the message.@objectstack/hono's catch-all answers500 {"success":false,"error":{"code":"INTERNAL_ERROR","message":"Internal server error"}}, anddispatcher-plugin's exit answers the same status and code.packages/runtimeis untouched. The report carries this as an open question and a finding.Pins
packages/rest/src/data-not-null-field-label-24715.test.ts: both doors on the real driver.PATCHandPOSTasserterror,fields[0].messageand the hint's kind: it names the field andstorage: { notNull: true }, and names neitherrequirednor drift. They also assert the unchanged envelope and key sets, and that the row did not move.requiredsibling field, on the same request shape, and the barezhcase is checked too.renderValidationMessage'srequiredsentence for the label, never as a literal.packages/rest/src/not-null-refusal-label.test.ts: the helper's declines, each with the source it must not ask: another table, another answer, a failed read, an undeclared column.packages/types/src/data-error-classification.not-null-label.test.ts:requiredrefusal not recognised, and a copied body not recognised;Ablations
One per change, each through
node scripts/ablation-replace.mjs: the anchor hit once, landed on disk, and was restored. Each restore was proven by blob (git hash-objectequal to the HEAD blob, andgit diff HEADempty). Each leg went red, the expected direction. The subjects resolve from source here (relative imports), so nodistbuild sits between mutation and run.required". Result:data-error-classification.not-null-label.test.tswent 1 failed / 9 passed, and the hint case is the one that failed. Blob61ebd27afc24→2f324ae3d717→61ebd27afc24.61ebd27afc24→b3d98ac5a64e→61ebd27afc24.labelNotNullRefusalwas made to decline every answer. Result: 6 failed / 6 passed.PATCH,POST, zh-CN,zh, the drift label and the helper's CONTROL went red. The fixture case, the undeclared-column control and the four declines stayed green. Blobf2a76857df43→e2383610aa51→f2a76857df43.Census of the old-text pins
git grepover every package's tests for the hint's wording found one pin this change made false:packages/rest/src/data-not-null-hint-20963.test.ts, which asserted that the hint's first sentence contains`required`. It now asserts the opposite. Its harness registers no stored-metadata family, so the door's object read fails there and the classification's answer goes out unchanged. That makes it the fail-safe control for the read, and its docblock now says so.X is requiredtext. The other tests that carry it either callmapDataErrordirectly, where the sentence is unchanged (rest.test.ts,rest-unclassified-fault-status.test.ts,rest-unique-violation-dialects.test.ts), or assert the record validator's own messages, or an import row's report. None drives a labelled declared field through the relabelled doors. None changed.Tests and gates
The test suites, typecheck and the first lint ran at
ee57acee6. The one later commit,055d54575, edits.changeset/24715-not-null-field-label-types.mdalone (minor,Clause-②: yes (widening)). The derived gates and lint were re-run at055d54575.@objectstack/types:local29 files, 774 passed, exit 0;repo1 file, 11 passed, exit 0.@objectstack/rest:local304 files, 6124 passed / 326 skipped, exit 0;repo5 files, 280 passed / 18 skipped, exit 0.pnpm --filter @objectstack/types typecheckandpnpm --filter @objectstack/rest typecheckboth exit 0, including rest'scheck:test-typecheck(0 debt).tsc --listFilesconfirms that each package's program compiles the new test files.node scripts/pm/dispatch-gates.mjs --commandsderived 67 commands, and all 67 were run with their exit codes recorded.ee57acee6:pnpm check:dual-build-cjs-loadsfirst exited 3 (PREREQUISITE NOT MET, nodist/). It exited 0 after a fullturbo run build.dispatch-gates --ranthen printed:67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN.055d54575(the same 67 derived): all 67 exit 0, the changeset readers included (check-changeset-no-major.mjs,check-adr-0087-registration.mjs,check-empty-changeset.mjs,check:changeset-gate-self-tests,check:pm-changeset-deadline-census,check:published-files).dispatch-gates --ranprinted:67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN.check-changeset-no-major.mjs --eventagainst this PR's payload: it passes with the body declaringno, and it passes with this body declaringyes (widening)("the declared widening is accounted for").pnpm lint(eslint . --no-inline-config, the whole repository, the lane's full union) exits 0 with no findings, atee57acee6and again at055d54575.Acceptance notes
/data/:object/batchand/batchrows, import rows, the public form submit, and data actions still name the column when the database refuses NOT NULL. Those that carry the classification's hint now carry it without therequiredprescription (an import row carries no hint). None was measured with a labelled field, so none is filed here./datadoor. See above. A driver refusal leavesdispatch()raw and is answered500.requiredsentence for both create and update, per the card. The validator words its own update refusal of arequiredfield withrequired_cleared("is required and cannot be cleared"); that sentence was not adopted.Generated by Claude Code