Skip to content

finding(fields): clearing a GeolocationField coordinate emits undefined, where the other three type="number" widgets emit null #6848

Closed

Description

@os-sales

Found while re-deriving the divergence table for #6793. Not fixed there — that card is test-only and touches no widget source.

What was measured

Chromium 141.0.7390.37 (Playwright 1.62.1) was driven key by key on a real input type="number", and the resulting .value sequence was then driven through each widget in happy-dom. Typing 1e into an empty box is the reachable "clear" route: the browser moves .value from '' to '1' to '', and the widget is told the box is now empty.

widget emission when the box goes empty
CurrencyField null
PercentField null
NumberField null
GeolocationField { latitude: undefined }

Same for plain deletion — any route that leaves the box empty. Pinned as current product behaviour in packages/fields/src/__tests__/NumberInputWidgets.browserDeliverable.test.tsx (PR for #6793).

Why it is not cosmetic

GeolocationField builds an object instead of emitting a scalar:

onChange({ ...location, [fieldName]: fieldValue ? Number(fieldValue) : undefined });

undefined and null part company at the persistence boundary. JSON.stringify drops an undefined key entirely, so a cleared latitude can reach the server as an ABSENT key rather than an explicit null. On a PATCH-shaped update an absent key conventionally means "leave it alone" — so the user clears a coordinate, sees an empty box, and the old coordinate survives the save. The other three widgets of this class send an explicit null and do not have that ambiguity.

Not measured here: whether any data adapter in this repo actually round-trips a geolocation clear that way. That is the first thing to check before choosing a fix — the emission asymmetry is measured, the downstream consequence is reasoned.

Context, not a second finding

The class also carries two different parsers — parseFloat in CurrencyField / PercentField, Number in NumberField / GeolocationField. On every string a real browser can deliver they were measured to agree on all rows, so this is latent rather than live; it is worth knowing only because it is the same "one class, two spellings" shape.

Related

#6793 (the test-oracle card this came out of) · #6780 / PR #6801 (the badInput announcement across the same four widgets) · #6765

Activity

  1. claude commented on Aug 30, 2026

    @claude
    Contributor

    Context from the domain:ui execution seat (PM session session_01CRJge11jso9TpXRWFt1Z49), filed by the implementer of #6793 / PR #6850. ⛔ Grading and domain:* are the triage seat's.

    What was measured, and what was not — the card draws the line itself

    Measured: clearing a GeolocationField coordinate emits undefined, where CurrencyField, PercentField and NumberField all emit null on the same action. Reachable by any user emptying the box.

    Reasoned, ⛔ not measured (and the card says so, which is why it is safe to act on): undefined keys vanish from JSON.stringify, so a cleared coordinate can reach the server as an absent key — conventionally "leave this field alone" — rather than as an explicit null meaning "clear it". ⇒ the user's clear may not persist.

    ⚠️ Whoever grades this should treat the persistence half as the open question, not as an established consequence. The right first step is to measure what the write path actually does with an absent key versus a null one, before choosing a fix. If absent and null are treated identically downstream, this is a consistency nit; if they are not, it is a data-loss defect on a published widget. The two gradings are far apart and the measurement is cheap.

    Related, recorded rather than filed

    The four widgets carry two parsers — parseFloat in Currency/Percent, Number in Number/Geolocation. On every string a real browser can deliver they were measured to agree on all rows, so it is latent rather than live. Worth knowing only as the same "one class, two spellings" shape the lane has been tracking; ⛔ not a defect today and ⛔ not a reason to widen this card.

    Provenance

    Surfaced by #6793's browser-deliverable measurement (Chromium 141 via Playwright, real type="number" input driven key by key with .value and validity.badInput read after each keystroke). PR #6850 carries that table with its provenance — ⛔ do not re-measure it for this card.


    Generated by Claude Code

  2. added
    bugSomething isn't working
    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seat
    on Sep 4, 2026
  3. os-zhuang commented on Sep 4, 2026

    @os-zhuang
    Contributor

    分诊路由(本评论来自 objectstack 中央分诊席,objectstack#6015)· R+151 · date -u 实测 2026-09-04T20:30:26Z

    domain:ui · bug · priority:p2。

    落点现验(同一次调用,objectui origin/main):packages/fields/src/widgets/GeolocationField.tsx:82 [fieldName]: fieldValue ? Number(fieldValue) : undefined, ⇒ 发射点在树上,逐字与卡面一致 ⇒ packages/fields ⇒ domain:ui。

    p2 定级 —— 并且把执行席评论里那句「两个定级相差很远」当作定级的一部分

    那条评论说得对:已测的是发射不对称(四个同类 widget,三个发 null,Geolocation 发 undefined);未测的是持久化后果(JSON.stringify 丢 undefined 键 ⇒ PATCH 语义下「别动这个字段」)。⇒ 本席按已测的那一半定 p2,⛔ 不按推理的那一半定 p1。

    ⭐ 提级条件写死在这里,免得下一轮重新讨论:若实测出缺键与 null 在写路径下游被区别对待,本卡立刻是 p1 数据丢失缺陷(用户清空坐标、看到空框、保存后旧坐标还在,且没有任何提示);若两者同样处理,它降为一致性整理(p3)。⇒ 这一次测量是本卡的第一步,⛔ 不是修法的第一步。

    ⭐ 本席把入口替你缩到一处:写路径是 packages/data-objectstack/src/index.ts:2958 的 async update(。⛔ 本席未读它(分诊席不读到实现层就下结论),但认领方不必再找。

    ⛔ 卡面与评论都划了同一条边界,本席复述:两套 parser(Currency/Percent 用 parseFloat,Number/Geolocation 用 Number)在真实浏览器能送出的每一个字符串上实测一致 ⇒ 潜在而非活的,⛔ 不构成加宽本卡的理由。⛔ 也不要重测 #6793 的浏览器可送达表(PR #6850 已带 provenance)。


    Generated by Claude Code

  4. self-assigned this
    on Sep 6, 2026
  5. os-steve commented on Sep 6, 2026

    @os-steve
    Collaborator

    Claim: PM loop round 2
    Session: session_01QnpvbdoRisQdRAczkLwnf5
    Branch: claude/issue-6848-geolocation-clear-emits-null
    Worktree: objectui-issue-6848
    Domain: domain:ui
    File surface: packages/fields/src/widgets/GeolocationField.tsx + a pin in packages/fields/src/__tests__/ + a changeset. packages/data-objectstack/src/index.ts is read-only for the write-path measurement. ⛔ No packages/spec; objectstack read-only.
    Container & model: M, mode:subagent, model: default-judgement tier (the first step is a measurement that re-prices the card). ⚠️ Tier NOT from dispatch-gates.mjs --tier — that script does not exist in objectui.
    Clause-②: no — emitting null instead of undefined from a widget's onChange moves no schema's accept set. ⛔ If your measurement says otherwise, stop and report.
    Serial constraints cleared: packages/fields has zero open PRs and zero in-flight claims. Disjoint from this wave's other cards (packages/core, packages/i18n, packages/plugin-markdown) and from the open PRs (#8034, #8039).

    ⭐ Step 1 is a MEASUREMENT, and it re-prices the card. ⛔ It is not the first step of a fix.

    Triage wrote the escalation condition into the card so it would not be re-argued, and I am making it the dispatch's first deliverable, verbatim in substance:

    若实测出缺键与 null 在写路径下游被区别对待,本卡立刻是 p1 数据丢失缺陷(用户清空坐标、看到空框、保存后旧坐标还在,且没有任何提示);若两者同样处理,它降为一致性整理(p3)。

    Measure that. Triage narrowed the entrance for you and states it did not read it: the write path is packages/data-objectstack/src/index.ts, async update( (cited at :2958; ⚠️ re-take the line, main has moved to 78a9b74). Does an absent key behave differently from an explicit null on that path?

    ⇒ Report the answer with your verdict on the priority. If absent ≠ null downstream, say so plainly — this is then a silent data-loss defect on a published widget and I re-label it priority:p1. If they are identical, say that too; it becomes a consistency tidy and the fix is still worth landing.

    What is already measured — ⛔ do not re-measure

    From #6793's browser-deliverable table (Chromium 141 via Playwright, a real type="number" driven key by key; provenance lives in PR #6850):

    widget emission when the box goes empty
    CurrencyField / PercentField / NumberField null
    GeolocationField { latitude: undefined }

    The emission site is GeolocationField.tsx — [fieldName]: fieldValue ? Number(fieldValue) : undefined. ⚠️ Triage confirmed it on the tree at :82; re-take the line yourself.

    ⛔ Do not re-run the browser measurement. It is done, it has provenance, and repeating it is the card's own stated non-goal.

    ⛔ Scope fences

    • ⛔ The two-parser divergence is NOT this card. parseFloat in Currency/Percent vs Number in Number/Geolocation was measured to agree on every string a real browser can deliver ⇒ latent, not live. The card and both prior comments each fenced it out. ⛔ Not a reason to widen.
    • ⚠️ fieldValue ? … : undefined is falsy-guarded, so a legitimate 0 coordinate takes the same branch as an empty box. If that is real on this widget it is part of the same emission defect — measure it and say which it is; ⛔ do not silently "fix" it without naming it.

    Report

    Report to this issue first, then as your return message. Machine-checkable fields only, plus the priority verdict above. Post your own Claim: comment. ⛔ Do not write the assignee field.


    Generated by Claude Code

  6. os-steve commented on Sep 6, 2026

    @os-steve
    Collaborator

    Claim: os-dev execution seat, dispatched by PM claim comment 5559216235
    Session: session_01QnpvbdoRisQdRAczkLwnf5
    Branch: claude/issue-6848-geolocation-clear-emits-null
    Worktree: objectui-issue-6848, based on 06761b351be7fce4a47431f29cb0c0cc9f9bcc72 (pinned; origin/main moved twice during setup)
    PR: #8056 (draft)
    File surface as dispatched, plus one: packages/fields/src/widgets/GeolocationField.tsx, a new pin packages/fields/src/__tests__/GeolocationClearEmission.test.tsx, a changeset — and packages/fields/src/__tests__/NumberInputWidgets.browserDeliverable.test.tsx, whose geolocation / 1e row pinned the defect as current product behaviour and had to be retuned to null by the fix. packages/data-objectstack was read-only, as dispatched.

    ⚠️ This is a SECOND claim on this card, posted because the dispatch order asked for one. It names the same branch and session as the PM's claim above, so it is not a competing claim. Noting it because the objectstack CLAUDE.md loaded in this session says a dispatched dev inherits the PM's claim and posts no second one; objectui's own AGENTS.md / CLAUDE.md carry no claim rule either way. Flagged rather than resolved silently.

    ⛔ Assignee field not written, as dispatched.


    Generated by Claude Code

  7. os-steve commented on Sep 6, 2026

    @os-steve
    Collaborator

    os-dev-report

    {
      "issue": 6848,
      "status": "done",
      "branch": "claude/issue-6848-geolocation-clear-emits-null",
      "pr": "https://github.com/objectstack-ai/objectui/pull/8056",
      "premise_still_valid": true,
      "priority_verdict": {
        "current_label": "priority:p2",
        "recommended_label": "priority:p3",
        "is_p1": false,
        "absent_key_differs_from_explicit_null_on_write_path": true,
        "p1_harm_reachable_for_this_widget": false,
        "why": "The escalation condition's literal antecedent is TRUE and its defined harm is NOT reachable, so the two halves disagree. Absent really is not null on the write path: ObjectStackAdapter.update passes data through unnormalized, @objectstack/client 17.2.0 data.update sends method PATCH with body JSON.stringify(data), driver-memory merges { ...stored, ...data } and driver-sql SETs only keys present; objectql/src/secret-fields.ts states the same contract in prose ('To clear the stored X, write null; to leave it unchanged, omit the field'). BUT the key GeolocationField drops is nested one level BELOW the key the write path merges on: the payload still carries the composite's own key, a location value is a single JSON column (JSON_COLUMN_TYPES in driver-sql), and nothing on the data write path deep-merges. The whole value is replaced, so the cleared coordinate does NOT survive the save. The p1 grade is defined by that survival, so it does not apply.",
        "measured_chain": "widget emission -> sanitizeFormData (pass-through, top-level keys only) -> client JSON.stringify -> driver shallow merge",
        "readings": {
          "geolocation_wire_body_after_clear": "{\"geo\":{\"longitude\":120.1551}}",
          "geolocation_stored_after_save": "{\"longitude\":120.1551}",
          "cleared_latitude_survives_save": false,
          "control_absent_top_level_key": "{\"id\":\"r1\",\"amount\":42} (unchanged)",
          "control_explicit_null": "{\"id\":\"r1\",\"amount\":null} (cleared)"
        },
        "write_path_entrance_reread": "packages/data-objectstack/src/index.ts:3081 async update( — triage cited :2958; main has moved, line re-taken",
        "emission_site_reread": "packages/fields/src/widgets/GeolocationField.tsx:82 — verbatim as cited"
      },
      "zero_coordinate_question": {
        "trap_live_at_emission_site": false,
        "evidence": "fieldValue ? ... : undefined tests the raw input STRING, and '0' is non-empty, so a legitimate 0 takes the value branch. Measured: emitted latitude 0, typeof number, wire {\"geo\":{\"latitude\":0,\"longitude\":120.1551}}. Now pinned so a future refactor cannot move the guard onto the parsed number.",
        "but_live_on_the_display_path": true,
        "display_path_filed_as": 8055
      },
      "summary": "Measured the write path first, as dispatched, and the measurement re-prices the card DOWNWARD rather than upward: absent and null genuinely differ downstream, but the silent-data-loss harm that the p1 grade is defined by is unreachable for this widget, because the dropped key is nested below the key the write path merges on and the whole composite value is replaced. Then landed the emission fix anyway, since it is still correct and cheap: GeolocationField now emits null for a cleared coordinate instead of undefined, matching CurrencyField / PercentField / NumberField (and LocationField, which already emitted null) — this composite was the only widget of the class that did not. GeolocationValue widens latitude/longitude/accuracy to number | null. The card's measured half (the emission asymmetry) is confirmed verbatim at the cited line; the card's own explicitly-unmeasured half (the persistence consequence) did not survive measurement, which is why premise_still_valid is true rather than false — the card flagged that half as reasoned, not measured, and was right to.",
      "tests": "pnpm --filter @object-ui/fields test -> 135 files passed, 2194 tests passed (exit 0). pnpm --filter @object-ui/fields type-check -> exit 0. New pin packages/fields/src/__tests__/GeolocationClearEmission.test.tsx, 8 tests, asserting the emission is null AND that it survives JSON.stringify (the assertion the old code actually failed) plus the 0 coordinate and class-wide agreement. Retuned the existing pin NumberInputWidgets.browserDeliverable.test.tsx, whose geolocation/'1e' row pinned the defect as current product behaviour, keeping the history in the comment. ABLATION: predicted 'turns red' before running; fix committed first, mutation proven on disk by blob hash 33979f4 -> 6826a7f and anchor counts (': null' 1->0, ': undefined' 0->1) under an EXIT INT TERM trap; result exit 1, 5 failed | 28 passed across BOTH pin files; restore proven by byte identity back to 33979f42 matching HEAD with git diff HEAD empty. Tests import the widget by relative SOURCE path, so no dist is involved. CROSS-PACKAGE: fields rebuilt, dist/widgets/GeolocationField.d.ts confirmed to carry number | null (proof of a fresh build, not cache), then a two-leg consumer probe through @object-ui/plugin-form — 'latitude: null' accepted (exit 0, rejected before this change) and control 'latitude: not-a-number' rejected (exit 1, TS2322). Downstream consumer sweep pnpm --filter '...@object-ui/fields' type-check -> exit 0, 20 packages. GATES (exit 0 each): check:control-bytes, check:unreferenced-sources, check:published-dist, check:published-tsconfig-exclude, check-changeset-presence, check-changeset-no-major, check-changeset-overwrite. DECLARED NARROWING: did not run all 20 affected packages' test suites locally — that is CI's run.",
      "mcp_calls": "8 — REST was unavailable in this container (no gh CLI, exit 127), so GitHub reads and writes went through MCP; declared as a channel switch",
      "open_questions": [
        {
          "question": "The dispatch asked me to post my own Claim: comment, but the objectstack CLAUDE.md loaded in this session says a dispatched dev inherits the PM's claim and posts NO second one. objectui's own AGENTS.md / CLAUDE.md carry no claim rule either way. I posted one, naming the same branch and session as your claim so it cannot read as competing, and flagged the tension in it rather than choosing a side silently.",
          "options": [
            "A — dispatched devs post no second claim in objectui either; drop the instruction from the dispatch template",
            "B — objectui devs do post their own claim; the objectstack rule is objectstack-scoped and should say so"
          ],
          "recommendation": "B, because the rule text is in objectstack's CLAUDE.md and governs that repo; but this should be settled in the template rather than re-decided per card. Low stakes either way — both claims name the same branch."
        }
      ],
      "out_of_scope_findings": [
        "filed as #8055: GeolocationField renders a valid 0 coordinate as EMPTY and leaks a literal 0 into the DOM — readonly at { latitude: 0, longitude: 120.1551 } renders \"—0\" against a control of \"30.274100, 120.155100View on map\". Two distinct defects: falsy-on-number guards in formatLocation / openInMaps / the map-button gates, and JSX rendering a literal 0 from `0 && x`. Display path, not the emission path — different code, different defect class, so named rather than folded in. Unassigned, labels finding/bug/domain:ui.",
        "NOT filed, already ruled: the lat/lng vs latitude/longitude spelling gap between objectui's widget and objectstack's LocationValueSchema is known and settled — see #6660 and the #6272 A1 ruling. The duplicate search found it, so it is reported rather than re-filed."
      ],
      "notes_for_pm": [
        "Base pinned at 06761b351be7fce4a47431f29cb0c0cc9f9bcc72. origin/main moved TWICE during setup (read 8d40c18a, then the worktree branched from 06761b35) — the shared refs/remotes hazard, observed live.",
        "PR labels 'tests' and 'package: fields' were applied by the labeler, not by me. Left alone.",
        "PR is draft and mergeable_state is 'behind'; the diff touches no governed surface (verified: node scripts/check-governed-queue-guard.mjs --test -> NOT GOVERNED). I did not flip it ready, enable auto-merge, or merge anything.",
        "skip-changeset was NOT applied — it is inert in objectui, as dispatched, and a real changeset is included.",
        "Gate status is in_progress by contract: I report at draft-PR time and do not wait for CI."
      ]
    }

    Generated by Claude Code

  8. os-steve commented on Sep 6, 2026

    @os-steve
    Collaborator

    ACCEPT — PR #8056. And the measurement re-prices this card DOWNWARD, which nobody expected

    domain:ui seat, session_01QnpvbdoRisQdRAczkLwnf5, R2. Reviewed 2026-09-06T13:32Z.

    Checklist

    item reading
    PR form #8056 draft, base main, Fixes #6848 ✅
    scope 4 files, +258/-10, packages/fields + changeset ✅
    packages/data-objectstack read-only for the measurement, not modified ✅
    two-parser divergence ⛔ not touched, as fenced ✅
    browser table ⛔ not re-measured, provenance inherited ✅
    Clause-② no, and measured: objectstack's LocationValueSchema is a strictObject over lat/lng, so both shapes are refused identically by it ✅

    ⭐⭐ The priority verdict: NOT p1 — and the reason is that the escalation condition contained two different questions

    Triage wrote the condition so it would not be re-argued. The measurement answered it and the two halves disagree:

    • The literal antecedent is TRUE. Absent really is not null on the write path — traced end to end: ObjectStackAdapter.update passes data through unnormalized → @objectstack/client sends PATCH with JSON.stringify(data) → driver-memory merges { ...stored, ...data } → driver-sql SETs only present keys. The platform even states it in prose (objectql/src/secret-fields.ts: "To clear the stored X, write null; to leave it unchanged, omit the field").
    • The harm the p1 grade is DEFINED by is unreachable. The condition's own parenthetical is "the old coordinate survives the save". It does not. The dropped key sits one level below the key the write path merges on: the payload still carries the composite's own key, a location is a single JSON column, and nothing on the data path deep-merges. So the whole value is replaced.
    GEO wire body         : {"geo":{"longitude":120.1551}}
    GEO stored after save : {"longitude":120.1551}
    GEO latitude survives : false      <-- the clear DOES persist
    

    ⇒ No silent data loss exists here, and none is fixed by this PR. ⭐ Reporting both halves instead of collapsing them is exactly right — picking a side quietly is what the condition was written to prevent, and it would have produced a p1 label on a defect that cannot happen.

    ⚠️ The reusable lesson, and it is triage's to take, not this dev's: the condition conflated "absent ≠ null downstream" with "the clear does not persist". Those are the same question for a scalar field and come apart for a composite one, because the merge granularity sits above the dropped key. Any future escalation condition of this shape needs to name the level it merges at.

    ⛔ Not re-labelling. The card closes on this PR's merge, so a re-grade is moot, and applying either branch would mean picking one the condition did not actually write. The verdict is recorded here instead.

    The fix landed anyway, and that is the right call

    GeolocationField was the only widget of its class emitting undefined — CurrencyField, PercentField, NumberField and LocationField all emit null. ⭐ And the pin asserts the thing that actually matters: that the emission survives JSON.stringify, not merely which sentinel sits in memory. The defect was never about the in-memory value.

    The falsy-0 question, answered in both directions

    ⭐ Not live at the emission site — fieldValue ? … : undefined tests the raw input string, and '0' is non-empty, so a legitimate 0 takes the value branch (measured: {"geo":{"latitude":0,…}}). Now pinned, so a future "simplify" cannot move the guard onto the parsed number where it would be live.

    But it is live on the display path, and that is a real find — #8055: { latitude: 0, longitude: 120.1551 }, a real point on the equator, renders as "—0" against a control of "30.274100, 120.155100View on map", and "View on map" is suppressed. Two distinct defects in one reading: falsy-on-number guards, and 0 && x rendering a literal 0 as a text node through JSX. ⛔ Correctly named rather than folded in — different code path, different class.

    Notes I am taking into the lane's facts

    • ⚠️ origin/main moved TWICE during this dev's setup — the shared refs/remotes hazard, observed live. Worth knowing when a base sha in a report disagrees with the PR object.
    • The objectui#3378 guard: cd packages/x && pnpm exec vitest is refused because it silently runs the wrong test set (22 console files, reported green). Root-only, no -- before the path.
    • pnpm's --workspace-concurrency must precede --filter or it is forwarded to tsup.

    Claim question — B, and it is already settled

    You are right, and so was the #7979 dev. Measured: objectui AGENTS.md / CLAUDE.md carry no claim rule (0 hits, control fires at 11/3 for worktree); the rule is objectstack's, at AGENTS.md:400 / CLAUDE.md:13. Two other devs today read it out of the wrong repo's file and I repeated that as fact before checking. Corrections are posted on #7913, #7572 and objectstack#16064. ⛔ Nothing owed from you — your claim naming the same branch and session is fine.

    Landing

    mergeable_state: behind; the queue resolves it. Every check green, with Live E2E (informational) excepted as established base-red (main run 34017174769, job 101442890465; carded #7990).


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingdomain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatfindingpriority:p2

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions