Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

finding(app-shell): ActionParamDialog's isLookupParam restates the picker family over RAW param spellings, so a degraded master_detail param loses the #3405 affordances #5654

Closed

Description

@os-sales

发现于 #5312 的实施(PR 见该卡)。不在 #5312 范围内 —— 该卡的文件面只有
packages/app-shell/src/utils/paramToField.ts,且这一条不是同一条规则:#5312 收敛的是
「哪些 widget 要带 reference 目标」,这里是「哪些 param 在降级后要拿到提示」。单独立此单。

事实(实读 origin/main @ aa3b81062,与 #5312 分支 7784e137e 同树)

packages/app-shell/src/views/ActionParamDialog.tsx:351:

// A lookup-typed param that fell back to text (no referenceTo)
// keeps the "paste an ID" placeholder/help hints.
const isLookupParam = param.type === 'lookup' || param.type === 'reference';

两个读者::355 的 placeholder(actionDialog.lookupPlaceholder)与 :447 的 help text
(actionDialog.lookupHelpText)。两处都还要求 field.type === 'text',即只在降级发生
后
才生效。

而真正执行降级的是 packages/app-shell/src/utils/paramToField.ts,它测的是解析后的
widget key
:

const LOOKUP_WIDGET_TYPES = new Set(['lookup', 'master_detail']);
...
if (LOOKUP_WIDGET_TYPES.has(type) && !param.referenceTo) { /* warn + type = 'text' */ }

测得的分歧

两个集合在任一方向上都不是子集关系:

拼写 paramToField 降级? 对话框给提示?
lookup 是 是
reference 是(PARAM_TYPE_ALIASES 先折成 lookup) 是
master_detail 是 否
  • master_detail 只在 paramToField 一侧:一个无 referenceTo 的 master_detail param
    确实降级成纯文本框、确实在 dev 下告警,但对话框既不给「粘贴 ID」的 placeholder
    也不给 help text —— 恰好是 fix(fields): 注册路径的全屏长文本对话框接上 i18n(#3404) #3405 为这个状态补上的两个可见提示。用户看到的是一个没有任何
    说明、要求裸 UUID 的空文本框。
  • reference 只在对话框一侧,因为它在 paramToField 里已被 alias 折走 —— 于是这行把
    alias 表的一条内容又手抄了一遍。

master_detail 是可达的:ActionParamDef['type'] 是 string
(packages/core/src/actions/ActionRunner.ts:584),且 paramToField.test.ts 的 drift guard
断言 FORM_FIELD_TYPES 的每个成员(含 master_detail)都解析到自己的 widget。

为什么不在 #5312 里顺手修

三条都不满足「有界就地修」:① 不是同一条规则(reference-bearing vs degraded-hint);
② 没有可派生的权威 —— 正确形态是
LOOKUP_WIDGET_TYPES.has(resolveParamWidgetType(param.type)),而 LOOKUP_WIDGET_TYPES
是 surface-local 的、目前还没有导出;③ 改了会改变行为(master_detail 从此开始拿到
placeholder 与 help text),那是修复,不是机械收敛,需要单独的证据与测试。

修法方向

把这行改成读解析后的 widget key,而不是原始拼写 —— 即从 paramToField 导出降级判据
(例如 paramDegradesWithoutTarget(param)),让「谁会降级」与「谁在降级后拿提示」由同一个
判据回答,而不是两处各自维护。⛔ 不要反过来把 master_detail 手加进对话框那行:那是把同
一个成员集抄成第三份。

参考

Activity

  1. added
    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seat
    on Aug 22, 2026
  2. huangyiirene commented on Aug 22, 2026

    @huangyiirene
    Collaborator

    Concentration-round first-touch (triage seat): promoted finding → pm:queue, Task S. The card's own repair direction is adopted as the dispatch shape: export the degradation predicate from paramToField (e.g. paramDegradesWithoutTarget(param)) and have the dialog's hint condition read it, so "who degrades" and "who gets hints after degrading" are one member set answered once. ⛔ Hand-adding master_detail to the dialog line stays forbidden (a third copy of the set is the disease, not a cure). The behaviour change is the point (a degraded master_detail gains the #3405 placeholder + help text) and takes its own test per spelling row of the card's table. Convergence lineage #4770/#4790/#4815 noted.


    Generated by Claude Code

  3. claude commented on Aug 23, 2026

    @claude
    Contributor

    Claiming this card.

    • session id: 01EuPCi56cnGyykygi3z9w4m
    • branch: claude/issue-5654-param-degrade-predicate
    • seat: domain:ui os-dev

    Shape per the triage ruling: export one degradation predicate from packages/app-shell/src/utils/paramToField.ts and consume it at ActionParamDialog.tsx:351, leaving exactly one membership table in the repo.


    Generated by Claude Code

  4. claude commented on Aug 23, 2026

    @claude
    Contributor

    os-dev-report

    {
      "issue": 5654,
      "status": "done",
      "branch": "claude/issue-5654-param-degrade-predicate",
      "pr": "https://github.com/objectstack-ai/objectui/pull/5882",
      "premise_still_valid": true,
      "summary": "Verified the card's premise on origin/main @1939c9610 before writing code: ActionParamDialog.tsx:351 still held `param.type === 'lookup' || param.type === 'reference'` over RAW spellings, while paramToField.ts degraded over RESOLVED widget keys (LOOKUP_WIDGET_TYPES = {lookup, master_detail}) — the two sets still overlapping without either containing the other. Fixed in the ruled direction: paramToField now exports paramDegradesWithoutTarget(param), and both its own fallback branch and the dialog's two hint readers ask it, leaving exactly ONE membership table on this surface. `master_detail` was NOT hand-added to the dialog line. The dialog's `field.type === 'text'` conjunct was dropped as subsumed (it was a proxy for 'did this degrade?'), licensed by an equivalence pin. Deliberate behaviour change: a targetless master_detail param now gets actionDialog.lookupPlaceholder + actionDialog.lookupHelpText; declared in a patch changeset. No doc touch — paramToField is internal to @object-ui/app-shell (not in its index.ts), so the new export changes no documented surface.",
      "tests": "All legs at final HEAD 67d6d8af1, from the repo root, path-filtered; heavy legs through scripts/pm/os-verify-lock.sh, verdict lines quoted rather than a bare $?.\n\nTESTS — pnpm exec vitest run packages/app-shell/src/utils/paramToField.test.ts packages/app-shell/src/views/ActionParamDialog.test.tsx --maxWorkers=2 → 'Test Files 2 passed (2) / Tests 72 passed (72)', 'os-verify-lock: VERDICT command-exit 0'. The verbose run names all four new dialog cases and all three new predicate cases as ✓, so none was silently skipped.\n\nREVERSE VERIFICATION (discriminating leg) — reverted ONLY the consumer hunk (git checkout HEAD~1 -- packages/app-shell/src/views/ActionParamDialog.tsx), keeping the exported predicate, so the failure is an assertion and not a missing-export module error. Revert confirmed on disk before measuring (grep -c degradedPicker → 0; retired literal back at line 351; git diff HEAD --stat showed the file). Result: '× a targetless master_detail param degrades to text AND now carries both #3405 hints (objectui#5654) — AssertionError: expected null to be actionDialog.lookupPlaceholder', 'Test Files 1 failed | 1 passed', 'Tests 1 failed | 71 passed (72)'. Exactly one test moved. Restored with git checkout HEAD -- <file>; git diff HEAD --stat EMPTY and git status --porcelain EMPTY; suite green again.\n\nLEGS THAT DO NOT DISCRIMINATE (stated, not staged): the three other new dialog cases stay GREEN under that revert — they are controls pinning behaviour that must not move (`reference` keeps its hints, master_detail WITH a target mounts the picker and gets none, an unknown type renders text and gets none). The paramToField predicate cases also stay green under it (producer untouched); reverting the PRODUCER instead removes the symbol the dialog imports, so that leg fails as a module error rather than a behaviour signal — it was not run as evidence.\n\nANTI-VACUITY CONTROL inside the equivalence sweep: the loop asserts predicate(param) ⇔ paramToField replaced the param's own widget with the text fallback over every FORM_FIELD_TYPES member plus every alias, and then asserts the degrading rows are exactly ['lookup','master_detail','reference'] — a sweep where nothing degraded would otherwise pass as false===false on every row.\n\nTYPE-CHECK — dependency closure built first: pnpm --workspace-concurrency=2 --filter '@object-ui/app-shell^...' build → 'VERDICT command-exit 0'. Then pnpm --filter @object-ui/app-shell type-check → script name echoed ('> @object-ui/app-shell@17.6.0 type-check' / '> tsc --noEmit && tsc -p tsconfig.test.json'), so not a zero-match silent pass; 'os-verify-lock: VERDICT command-exit 0 · held the lock 61s · waited 103s'.\n\nREPO GATES (inputs touched), each quoting its own verdict line, exit code captured before any pipe:\n  check-changeset-presence.mjs EXIT=0 — '✅ 4 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s): .changeset/action-param-degrade-predicate-5654.md.'\n  check-changeset-no-major.mjs EXIT=0 — '✅ No changeset declares a `major` bump.'\n  check-control-bytes.mjs EXIT=0 — '✅ check-control-bytes: OK (scanned 4891 tracked text file(s); skipped 85 binary).'\n  check-i18n-call-site-keys.mjs EXIT=0 — 'Every in-scope call-site key resolves against the en pack (2928 keys)…' (no new keys: both hint keys already ship in all ten packs).\nDoc gates not run: no docs touched, and no version literal was written into prose anywhere.\n\nLINT — narrowing DECLARED. eslint was run over the four changed source files, not the repo. (i) Population came from eslint's own resolution of those paths, not from my guess about scope; (ii) the count is read from --format json: 4 files linted, 0 errors, 22 warnings; (iii) invariance: eslint.config.js declares no `project`/`projectService`, so no rule here is type-aware and this diff cannot move a verdict on an untouched file — the repo's `ratchet` rules are per-file AST rules, so a per-file run is a complete measurement of the diff. All 22 warnings are pre-existing (no-explicit-any / react-refresh / react-hooks); 0 land on a line this branch added, checked by intersecting the reporter's line numbers with the added-line set of git diff -U0 $(git merge-base HEAD origin/main) HEAD. The full farm runs in CI.\n\nCI: reported at draft-PR time per the dispatch contract; gate convergence is the PM's read, not awaited here.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #5880: the target-required degrade set {lookup, master_detail} is stated once per package — app-shell's paramToField.ts:54 and plugin-grid's bulkParamToField.ts:40 — with nothing mechanical tying them; members identical today (no live defect), and the two predicates key on different fields (referenceTo vs object), so merging them is a decision rather than a mechanical convergence. Labels finding + domain:ui, unassigned, no pm:queue.",
        "not filed (already covered): app-shell's resolveActionParams.ts:489 restates the reference-bearing family — that is #5874's table, already open with finding + domain:ui, and #5875 covers the stale 'fourth and last private copy' prose in the two files this PR edits. This PR does not touch those sentences; expect a textual merge with #5875, not a contradiction."
      ]
    }

    Generated by Claude Code


    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

domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatpm:queue

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions