Repository navigation
fix(plugin-chatbot): fold an authored 'tool' role before it seeds the AI SDK store - #8837
Merged
Merged
Conversation
… AI SDK store `useObjectChat`'s `aiInitialMessages` builder asserted the authoring role into the SDK's three roles at both of its arms — `as 'user' | 'assistant' | 'system'` — rather than performing the fold the package already owns by name. The authoring contract accepts `role: 'tool'`, `normalizeMessages` deliberately does not narrow it, and `renderer.tsx` passes `schema.messages` straight through as `initialMessages`, so in API mode the store was seeded with a value outside its own union while declaring one of three roles it was not. Measured on the real @ai-sdk/react 4.0.68 / ai 7.0.65 before choosing the fix: the client store holds the out-of-union role verbatim (nothing recognises it, nothing renders it distinctly) and the SDK's own downstream entry points reject it — `convertToModelMessages` throws AI_MessageConversionError: Unsupported role: tool, `validateUIMessages` throws AI_TypeValidationError naming ["system","user","assistant"] at [0].role. Both read against an in-union control on the identical shape, which converted and validated cleanly. Both arms now call `toRuntimeRole(...)` (objectui#4399's named decision, the same fold the render seam applies). Deleting the assertions also re-arms the compile-time tripwire chatMessageAdapter.ts documents: "a new authored `role` makes toRuntimeRole unassignable". Adds the API-mode test leg the defect was invisible to — the existing 'tool' fixture runs in LOCAL mode, where the builder's output never reaches `useChat`. The import pin in chat-message-contract.test.ts now matches the adapter import list by membership: it guarded against the timestamp coercion being restated inline, and would otherwise have fired on a companion fold arriving. Fixes #8443 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01611D6ZaRaMmwTNQmSbk8MH
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
Contributor
Author
PM 复核 — 通过。已翻 ready,auto-merge 已武装。⛔ PR 状态由本席翻转,你不要再写 ⛔ 先记本席自己的一次断线读数本席想独立复测那条决定性前提(SDK 的 ⇒ 那不是读数,是探针没接上。只有对照救了它。 记在这里,因为本席不打算把一个仪器故障冒充成一次独立验证。 ⇒ 本席没有在本轮重跑那次运行时测量。采信它的理由不是「dev 说了」,而是下面这一条 —— ⭐⭐ 你把那次测量放进了测试套件,而不是放进 commit message裁决把「继续」挂在一条 zone-2 测量上。你不只是量了它,你让第四条腿在 CI 上每次都重新建立它: ⭐ 这个切分正是对的:mock 掉容器,保留裁判。 于是「SDK 拒绝第四种角色」不再是一句会过期的断言 —— 它是一条带前值对照、跑在真依赖上的常驻检查。SDK 哪天真的接受了第四种角色,这条腿会告诉我们,而不是悄悄变成一句陈年谎话。 ⇒ 一个会过期的前提,被改造成了 CI 每轮重新确立的前提。 本席采纳为后续派发的标准要求。 ⭐⭐ 你写了那条加分 pin,然后删掉了它,并说明为什么这是本 PR 里判断力最高的一处,值得单独点名:
⇒ 一条在正确修复上变红的 pin,是噪音,不是警报。 而且你指出真正的报警器已经在两处存在: 文档那句话,逐字地,从编译器嘴里出来。 ⭐ 交一个「我写了然后删了,理由如下」,比交一个凑数的 pin 诚实得多。 ⭐ 消融被它自己的前检抓住了一次幻影
|
os-zhuang
marked this pull request as ready for review
September 9, 2026 13:32
os-zhuang
enabled auto-merge
September 9, 2026 13:32
This was referenced Sep 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8443
useObjectChat'saiInitialMessagesbuilder asserted the authoring role into the SDK's three roles at both of its arms instead of performing the fold this package already owns by name. Bothas 'user' | 'assistant' | 'system'casts are gone; both arms now calltoRuntimeRole(...)— objectui#4399's named decision, the same fold the render seam already applies.The zone-2 measurement, first
The card was explicit that it had not measured what the AI SDK store does with an out-of-union role, and the ruling made proceeding conditional on that reading. Measured on
origin/mainbefore anything was changed, against the real@ai-sdk/react4.0.68 /ai7.0.65 (@ai-sdk/reactdeliberately unmocked):role: 'tool'useChatstore itself[{"id":"m-tool","role":"tool"}]["user","system","assistant"]read back distinct ⇒ the instrument reads the authored role, not a constantuseObjectChat({api})messagesasrole: 'tool', end to enduser/systemarrive as themselvesconvertToModelMessagesAI_MessageConversionError: Unsupported role: toolassistanton the identical shape converts:ok:[{"role":"assistant","content":[…]}]validateUIMessagesAI_TypeValidationError—invalid_valueat[0].role,values: ["system","user","assistant"]assistantresolvesBranch selected: the first one. Nothing recognises
'tool', nothing renders it distinctly, nothing folds it — the client store is a plain container, and the SDK's own two downstream entry points (the ones a backend route runs over exactly these values) reject it. So folding to'assistant'is a fix and not a regression, and the ruling stands as written.That measurement is now in the suite rather than in a commit message: the fourth test runs the SDK's own
validateUIMessagesover the builder's output, with the pre-fix value as its control.The API-mode test leg
useObjectChat.honestMessages.test.tsx:157-167already authors a'tool'message, but in LOCAL mode, where the builder's output never reachesuseChat.useObjectChat.runtimeRole-8443.test.tsxadds the leg that was missing:apiis set, and the double captures themessagesoption — the store seed — so the assertions read what the store actually receives.Four tests: the content path (through
normalizeMessages), the pre-builtpartspath (the card names both arms; fixing one would leave the defect reachable from any schema that authorsparts), a lit control that the other three roles arrive unchanged ('system'in particular keeps its centred pill — a builder that hard-coded'assistant'would satisfy the first two assertions and destroy that), and the SDK-validator leg above.Reverse verification
Direction predicted before the run: red. Run under the shared verify lock at
4cb197732:Every one of the four legs reddened, naming the value:
expected { id: 'a-tool', role: 'tool' } to match object { id: 'a-tool', role: 'assistant' }, and the validator leg withAI_TypeValidationError … invalid_value at [1].role. Restore proven by state, not by exit code:git diff HEADempty, on-disk blob back to5b604f133a…,git statusclean. The mutation script carries atraponEXIT INT TERMthat restores fromHEAD, with an absolute path.Ground 2 — measured, and deliberately NOT given an assertion of its own
The ruling's second ground is that the casts also switched off the alarm
chatMessageAdapter.ts:60-61says the seam exists to provide: "a new authoredrolemakes {@link toRuntimeRole} unassignable". With the casts deleted,toRuntimeRoleis back on the builder's path, so that alarm is armed.Ablated to check it is real, not just documented. Widening
ChatMessage['role']by one member and rebuilding@object-ui/types(marker proven present inpackages/types/dist/complex.d.ts:922before reading any result — the first attempt of this ablation was caught as a phantom by that preflight, because the marker had reacheddist/complex.d.tsand not thedist/index.d.tsI first grepped):The doc's sentence, verbatim, from the compiler. Restored and rebuilt: source marker 0 hits, dist marker 0 hits,
git diff HEAD -- packages/typesempty. (Restoring source alone would have left the mutated types in force for every later run in this tree.)Excludeof the authored union minus'tool'constrained to extend the runtime union — encodes "the fold handles exactly'tool'", not "the fold is total". A contributor who meets a new authored role correctly, by folding it too, makestoRuntimeRolecompile again and makes that pin red. A pin that reddens on the correct fix is a nuisance, not an alarm. The union's own spelling is already pinned bychat-message-contract.test.ts's exact-equality pin onAuthoredChatMessage['role'], and the tripwire itself is the type-check gate — which the ablation above shows fires. The reasoning is written into the test file so the next reader does not re-derive it.The one collateral edit
chat-message-contract.test.ts's import pin matched the whole lineimport { toRuntimeTimestamp } from './chatMessageAdapter';. It guards against the Date→ISO coercion being restated inline; a second fold arriving from the same module is the opposite of that, so the regex now matches by membership of the import list. Nothing else about the guard moved.Gate verdicts
Package build / type-check / test / lint run whole, each quoting its own verdict line. Heavy runs went through the container's shared verify lock (
OS_VERIFY_LOCK_SLOT=dev-8443-objectui).pnpm --workspace-concurrency=2 --filter '@object-ui/plugin-chatbot^...' buildos-verify-lock: VERDICT command-exit 0 · held the lock 48spnpm --filter @object-ui/plugin-chatbot buildVERDICT command-exit 0 · held the lock 11s·✓ built in 9.10spnpm --filter @object-ui/plugin-chatbot type-check(tsc --noEmit && tsc -p tsconfig.test.json)VERDICT command-exit 0pnpm --filter @object-ui/plugin-chatbot testVERDICT command-exit 0·Test Files 43 passed (43)·Tests 490 passed (490)pnpm --filter @object-ui/plugin-chatbot lintLINT_EXIT=0·✖ 88 problems (0 errors, 88 warnings)— all pre-existingreact-hooks/refs/set-state-in-effect, zero on any changed filecheck-governed-queue-guard --teston the 4 changed paths✅ NOT GOVERNED — 4 path(s) checked against 5 governed surface(s); none matched.(exit 0)--test AGENTS.md⛔ GOVERNED — 1 of 1 path(s)(exit 3) ⇒ the instrument distinguishes, so theNOT GOVERNEDabove is a readinggrep -naPover the 4 changed paths for the C0 rangetype-checkreally covers the new file — proven withtsc -p tsconfig.test.json --listFiles, which namesuseObjectChat.runtimeRole-8443.test.tsx, rather than assumed.Not run locally, left to CI: the repo-wide
pnpm lint/pnpm testfarm. This PR's changes are confined to one package; CI owns the full sweep.验收备注
Observations found in passing, not filed and not fixed — each with the successor that will meet it:
mapMessages.ts:862—role: (msg.role ?? 'assistant') as ChatMessage['role']. The inbound mirror of the same class: a cast where the union is already right, so it asserts nothing but disables the same tripwire on the way in. Not the same defect — its input is already the SDK's three roles, so no out-of-union value flows — which is why it is an observation and not a card. Successor: objectui#8426, which is in this sameuseMemo's neighbourhood and already owns thepartshalf.useObjectChat.ts:712'smessages: … as anystays. Its own comment says it absorbspartsbeing an array ofRecordofstringtounknown, and that is objectui#8426's, not this card's. Deleting it here would turn the call red with TS2322. Successor: objectui#8426, by name, in its own comment.'tool'role on the hook's ownmessagessurface: local keeps'tool'(pinned,honestMessages.test.tsx:157-167), API mode now shows'assistant'. That is the ruling's intended behaviour change andObjectChatMessagestays honest about it (wide where local mode is wide). Recorded because it is the fact a reader of the two tests together will notice. Successor: none needed — it is the changeset's headline.维护者速读(草稿)
改了什么 —
plugin-chatbot的useObjectChat在 API 模式下,把作者写的role: 'tool'消息交给 AI SDK 之前,原来是用一个类型断言「宣称」它是三种角色之一,实际值原封不动传了下去。现在改成真正做一次折叠:'tool'→'assistant'(这是本包早就有名字的既定决定,渲染层一直在用)。另外补了一条以前完全没人跑过的测试腿。为什么改 — 动手前先实测了「这个值传下去到底会怎样」:客户端 store 原样存着,谁都不认识它;而 SDK 自己的两个下游入口(后端路由正是拿这些值去调的)直接拒绝——一个抛
Unsupported role: tool,一个抛类型校验错误并点名只接受三种角色。同一形状换成合法角色则一切正常。所以这不是「可能有问题」,是一个已经会炸的值,只是被断言挡住了编译器的视线。断言还顺手关掉了这个接缝专门装的编译期报警器:以后谁给作者类型加一个新角色,本该在这里编译不过、逼人处理,断言会让它静默溜过去。这一条也做了消融实测:临时加一个角色,type-check确实红,报错句子和文档写的一模一样。风险与代价(含回滚) — 有一个行为变化,范围很窄:API 模式下,作者写的
'tool'消息现在以'assistant'身份进入会话线程,内容、parts、工具调用全部不动;另外三种角色不受影响('system'仍然是居中的那个 pill,有专门的对照测试守着)。本地模式完全没动。这个变化本来就是卡片和裁决明确要的,changeset 里写成了标题。回滚成本低:一次 revert 即可,改动只有一个包的四个文件,没有新导出、没有新 prop、没有数据迁移。席位意见 —
你要做的 — 确认那条行为变化可以接受(作者写的
'tool'消息在 API 模式下会显示成 assistant 气泡,这与它在本地模式渲染出来的样子一致)。除此之外没有需要你决定的东西。本 PR 不在受管面上(已用check-governed-queue-guard --test测过,并用AGENTS.md做了亮对照),走正常评审与合并队列即可。🤖 Generated with Claude Code
https://claude.ai/code/session_01611D6ZaRaMmwTNQmSbk8MH
Generated by Claude Code