diff --git a/.changeset/useobjectchat-honest-message-type-4424.md b/.changeset/useobjectchat-honest-message-type-4424.md new file mode 100644 index 0000000000..81eccffa47 --- /dev/null +++ b/.changeset/useobjectchat-honest-message-type-4424.md @@ -0,0 +1,16 @@ +--- +'@object-ui/plugin-chatbot': minor +'@object-ui/app-shell': patch +--- + +`useObjectChat` declares the message shape it actually hands back + +The hook typed `messages` — and the `onSend(content, messages)` callback fed from it — as `@object-ui/types`' authoring `ChatMessage`. That was true in local mode only. In API mode the values came out of the runtime mapper and were asserted into place with `as OuiChatMessage[]`, and the authoring contract declares none of what they carry: `buildProgress`, `blueprintProgress`, `charts`, and `pendingActionId` / `draftReview` / `proposedPlan` / `proposedChanges` / `builderHandoff` on every tool invocation. Those keys are the HITL approval card, the "Review N changes" affordance, the proposed-plan card, the build panel and the inline charts. They survived only because nothing on the path ever rebuilt a message; anyone writing the obvious thing — reconstruct a message field-by-field from its declared type — deleted all of them, with the compiler agreeing, because the declared type genuinely did not have them. + +The declaration is now the truth, published as `ObjectChatMessage`. The survey behind it found the honest type to be neither of the two `ChatMessage` types on either side, because neither is true of both modes: it stays **wide** where local mode is wide (an authored `'tool'` role and the legacy `'partial-call'` / `'call'` / `'result'` tool states reach this surface unchanged and are folded only at the render seam), **narrow** where both modes are narrow (`timestamp` is `string`, never `Date` — API mode never produces one and local mode absorbs it before emitting), and adds the render-only keys API mode really carries. The `as OuiChatMessage[]` assertion is deleted rather than moved: the mapper's output satisfies the declared type, so the compiler checks that assignment instead of being told to stop looking. + +Nothing about the values changed, and nothing correct breaks. `ObjectChatMessage` is a **subtype** of the authoring `ChatMessage` it replaces, so every consumer that accepted the old declaration still accepts these values — including a host `onSend` callback that types its parameter as `ChatMessage[]`, which keeps type-checking by contravariance. Naming `ObjectChatMessage` is what lets a host *read* the keys above. The one observable narrowing is deliberate: code that branched on `timestamp instanceof Date` was handling a value this hook cannot emit, and now says so at compile time. + +The seam below it (`chatMessageAdapter.ts`, from objectui#4399) is still necessary and unchanged in behaviour — `'tool'` and the legacy tool states still have to be narrowed for the renderers. What changed is that its pass-through is no longer an act of faith: its input type (`SeamChatMessage`, also exported, alongside `SeamToolInvocation`) names the render-only keys, so the spread preserves them as declared properties the compiler can see, and the pass-through tests type their API-mode fixture directly instead of casting it past the compiler. A cast returning to the hook is now caught by a test rather than by a future outage. + +App-shell carries a comment-only correction on the same family: `AiChatPage` still described `@object-ui/plugin-chatbot` as exporting a second, minimal legacy `ChatMessage` alongside the enhanced one. That collision was retired in objectui#4383 — the barrel publishes one contract and `ChatbotEnhancedMessage` is a deprecated alias of it — so the paragraph was sending readers to look for a hazard that no longer exists. diff --git a/content/docs/plugins/plugin-chatbot.mdx b/content/docs/plugins/plugin-chatbot.mdx index 15e2265da9..15087f2741 100644 --- a/content/docs/plugins/plugin-chatbot.mdx +++ b/content/docs/plugins/plugin-chatbot.mdx @@ -98,7 +98,7 @@ const schema = { autoResponse?: boolean, autoResponseText?: string, autoResponseDelay?: number, - onSend?: (content: string, messages: ChatMessage[]) => void, + onSend?: (content: string, messages: ObjectChatMessage[]) => void, className?: string, // AI / service-ai integration fields api?: string, @@ -494,6 +494,29 @@ const aiSchema: ChatbotSchema = { } ``` +### What comes back out: `ObjectChatMessage` + +You AUTHOR with `@object-ui/types`' `ChatMessage`. What `useObjectChat` HANDS +BACK — from `messages` and from `onSend(content, messages)` — is +`ObjectChatMessage`, exported from `@object-ui/plugin-chatbot`: + +```plaintext +import type { ObjectChatMessage } from '@object-ui/plugin-chatbot' +``` + +It is the authoring shape plus the render-only keys API mode really carries +(`buildProgress`, `blueprintProgress`, `charts`, and `pendingActionId` / +`draftReview` / `proposedPlan` / `proposedChanges` / `builderHandoff` on each +tool invocation — the approval card, the "Review N changes" affordance, the plan +card, the build panel, the inline charts), with `timestamp` narrowed to `string` +because both modes absorb an authored `Date` before emitting. + +It is a subtype of the authoring `ChatMessage`, so an `onSend` callback that +already declares `ChatMessage[]` keeps type-checking; naming `ObjectChatMessage` +is what lets it read those keys. Rebuilding a message field-by-field from the +authoring type drops every one of them — silently, and with the compiler's +agreement (objectui#4424). + ## Related Documentation - [Plugin System Overview](/docs/guide/plugins) diff --git a/packages/app-shell/src/console/ai/AiChatPage.tsx b/packages/app-shell/src/console/ai/AiChatPage.tsx index 94af2beaf2..e11a9d57ab 100644 --- a/packages/app-shell/src/console/ai/AiChatPage.tsx +++ b/packages/app-shell/src/console/ai/AiChatPage.tsx @@ -77,14 +77,17 @@ import { type ChatbotEnhancedToolInvocation, // The ENHANCED message shape — the one `` renders and the // one this file actually produces (`toolInvocations`, `buildProgress`). - // `@object-ui/plugin-chatbot` ALSO exports a minimal legacy `ChatMessage` - // from its own barrel module (id/role/content/timestamp/avatar only), and - // that is what this import used to resolve to. The mismatch compiled because - // every construction site spreads the extra keys conditionally - // (`...(x ? { toolInvocations } : {})`), which defeats excess-property - // checking — so the declared type was narrower than every value flowing - // through it, and `AiChatPage.hydration.test.ts` could not read - // `toolInvocations` off its own function's return (objectui#4040). + // + // `@object-ui/plugin-chatbot` publishes ONE chat-message contract: its + // barrel's `ChatMessage` IS this type, and `ChatbotEnhancedMessage` is a + // deprecated alias of the same declaration, kept so this import (PR #4379) + // keeps compiling. The collision that made the alias necessary is gone — + // the barrel used to DECLARE a second, minimal `ChatMessage` of its own + // (id/role/content/timestamp/avatar only) and the natural name resolved to + // it, which is how this file once could not read `toolInvocations` off its + // own function's return (objectui#4040). Retired in objectui#4383 / PR + // #4400; pinned in the plugin's `chat-message-contract.test.ts`. New code + // here should spell `ChatMessage`. type ChatbotEnhancedMessage as ChatMessage, } from '@object-ui/plugin-chatbot'; diff --git a/packages/plugin-chatbot/README.md b/packages/plugin-chatbot/README.md index 5c3c076ed4..bc4155b14b 100644 --- a/packages/plugin-chatbot/README.md +++ b/packages/plugin-chatbot/README.md @@ -110,6 +110,30 @@ function MyChat() { } ``` +#### What `messages` actually contains + +`messages` — and the `onSend(content, messages)` callback fed from it — is +typed `ObjectChatMessage[]`, exported from this package. It is deliberately +neither of the two `ChatMessage` types nearby, because neither is true of both +modes (objectui#4424): + +- **Not `@object-ui/types`' authoring `ChatMessage`.** In API mode the values + come from the runtime mapper and carry `buildProgress`, `blueprintProgress`, + `charts`, and `pendingActionId` / `draftReview` / `proposedPlan` / + `proposedChanges` / `builderHandoff` on each tool invocation — the approval + card, the "Review N changes" affordance, the plan card, the build panel and + the inline charts. The authoring contract declares none of them, so rebuilding + a message field-by-field from it deletes all of them, and the compiler agrees. +- **Not this package's runtime `ChatMessage` either.** In local mode an authored + `'tool'` role and the legacy `'partial-call'` / `'call'` / `'result'` tool + states pass through untouched; they are folded only at the render seam + (`toRuntimeMessages`, `chatMessageAdapter.ts`). + +`ObjectChatMessage` is a subtype of the authoring `ChatMessage`, so anything +already typed against that keeps compiling — naming `ObjectChatMessage` is what +lets you *read* the keys above. `timestamp` is always a `string` here: both +modes absorb an authored `Date` before emitting. + ## Schema-Driven Usage ### Discovering Backend Agents diff --git a/packages/plugin-chatbot/src/__tests__/chat-message-contract.test.ts b/packages/plugin-chatbot/src/__tests__/chat-message-contract.test.ts index d7d5b3eb6a..5c09f25e69 100644 --- a/packages/plugin-chatbot/src/__tests__/chat-message-contract.test.ts +++ b/packages/plugin-chatbot/src/__tests__/chat-message-contract.test.ts @@ -42,7 +42,17 @@ import type { ChatMessage as BarrelChatMessage, ChatbotEnhancedMessage } from '. import type { ChatMessage as EnhancedChatMessage } from '../ChatbotEnhanced'; /** The OTHER side of the seam: the JSON/SDUI authoring contract. */ import type { ChatMessage as AuthoredChatMessage } from '@object-ui/types'; -import type { authoredToRuntimeMessage, toRuntimeMessages } from '../chatMessageAdapter'; +import type { + authoredToRuntimeMessage, + toRuntimeMessages, + SeamChatMessage, +} from '../chatMessageAdapter'; +/** The hook's own output contract (objectui#4424). */ +import type { + ObjectChatMessage, + UseObjectChatOptions, + UseObjectChatReturn, +} from '../useObjectChat'; type Assert = T; type IsAny = 0 extends 1 & T ? true : false; @@ -204,6 +214,145 @@ describe('the authoring ↔ runtime seam is an adapter, not a cast', () => { }); }); +describe("useObjectChat's declared message type is honest about both modes", () => { + it('is pinned at compile time', () => { + // objectui#4424. The hook declared `messages: OuiChatMessage[]` (the + // AUTHORING contract) and produced it honestly in local mode only — API + // mode built RUNTIME messages and cast them with `as OuiChatMessage[]`, + // erasing `buildProgress`, `blueprintProgress`, `charts` and every HITL / + // draft-review extension on the tool invocations. The values survived + // because nothing on the path rebuilt a message; the declaration was the + // trap, and these pins are what keeps it closed. + + // Probe hygiene first — an `any`/`unknown` here answers every question. + type _HookMsgNotAny = Assert, false>>; + type _HookMsgNotUnknown = Assert, false>>; + type _NoIndexSignature = Assert, false>>; + + // 1. The hook's two published surfaces speak it — `messages` and the + // `onSend(content, messages)` callback, which the survey measured as + // carrying the SAME values (both modes hand it the thread they are + // about to hold). + type _ReturnIsHonest = Assert>; + type _OnSendIsHonest = Assert< + Equal< + NonNullable, + (content: string, messages: ObjectChatMessage[]) => void + > + >; + + // 2. WIDE where local mode is wide. An authored `'tool'` role and the + // legacy tool states reach this surface unchanged (`normalizeMessages` + // narrows neither) and are folded only at the render seam. This is the + // line that proves the runtime type would have been a LIE about local + // mode — i.e. why the honest answer is neither of the two existing + // contracts. + type _KeepsToolRole = Assert< + Equal + >; + type _KeepsLegacyToolStates = Assert< + Equal< + Extract< + NonNullable[number]['state']>, + 'partial-call' | 'call' | 'result' + >, + 'partial-call' | 'call' | 'result' + > + >; + + // 3. NARROW where both modes are narrow. Neither mode can emit a `Date`: + // API mode never produces one, and local mode absorbs it in + // `normalizeMessages` before handing anything out. Declaring + // `string | Date` asked every consumer to handle a value that cannot + // arrive. + type _TimestampIsString = Assert>; + type _TimestampRejectsDate = Assert< + Equal, false> + >; + + // 4. …plus the render-only keys API mode really carries. Named one by one + // so a future narrowing says WHICH capability it dropped — each of these + // is a rendered affordance: the build panel, the design panel, inline + // charts, the approval card, the "Review N changes" entry point, the + // proposed-plan card, the confirm-changes card, "Open in Builder". + type _HasBuildProgress = Assert>; + type _HasBlueprintProgress = Assert>; + type _HasCharts = Assert>; + type HookToolInvocation = NonNullable[number]; + type _HasPendingActionId = Assert>; + type _HasDraftReview = Assert>; + type _HasProposedPlan = Assert>; + type _HasProposedChanges = Assert>; + type _HasBuilderHandoff = Assert>; + + // 5. The compatibility statement, and the reason this is a MINOR and not a + // break: the honest type is a SUBTYPE of the authoring one it replaces. + // Every consumer that correctly accepted `@object-ui/types`' ChatMessage + // still accepts these values — including a host `onSend` typed against + // the authoring contract, which type-checks by contravariance. + type _StillAnAuthoredMessage = Assert< + ObjectChatMessage extends AuthoredChatMessage ? true : false + >; + + // 6. And the seam is still NECESSARY — this is not option 2 in disguise + // (API mode made to produce the authoring type, i.e. the field-by-field + // rebuild). The hook's shape is deliberately NOT assignable to the + // runtime one: `'tool'` and the legacy states still have to be narrowed, + // by `chatMessageAdapter.ts`, at the render seam. + type _AdapterStillNeeded = Assert< + Equal + >; + // The other direction DOES hold, and that is what deleted the cast: the + // runtime mapper's output is assignable to the hook's declared type, so + // API mode assigns instead of asserting. It is also what keeps a host's + // own narrowing cast to the runtime type legal. + type _RuntimeFlowsInWithoutACast = Assert< + EnhancedChatMessage extends ObjectChatMessage ? true : false + >; + + expect(true).toBe(true); + }); +}); + +describe("the seam's input contract names the keys it passes through", () => { + it('is pinned at compile time', () => { + // objectui#4424. PR #4416's adapter preserved the runtime-only keys with a + // spread over a parameter typed as the AUTHORING message — so the keys were + // preserved by faith: `tsc` could not see one of them, and the pass-through + // test had to cast its fixture into place. `SeamChatMessage` names them. + type _SeamNotAny = Assert, false>>; + type _SeamHasBuildProgress = Assert>; + type _SeamHasBlueprintProgress = Assert>; + type _SeamHasCharts = Assert>; + type SeamTool = NonNullable[number]; + type _SeamToolHasPendingActionId = Assert>; + type _SeamToolHasDraftReview = Assert>; + + // It is the seam's INPUT, so it stays wide enough for both callers: a host + // holding plain authored messages (the reason the adapter is exported from + // the barrel at all) and the hook's own output. + type _AuthoredStillAccepted = Assert< + AuthoredChatMessage extends SeamChatMessage ? true : false + >; + type _HookOutputAccepted = Assert; + + // Which is why it keeps the `Date` the hook's output has already absorbed: + // this is where an authored `Date` still dies (`toRuntimeTimestamp`), and + // narrowing it here would make that documented decision unreachable. + type _SeamStillTakesADate = Assert< + Equal + >; + + // The adapter reads it: a signature that drifted back to the bare authoring + // type would re-blind the pass-through without changing a line of its body. + type _AdapterTakesTheSeamType = Assert< + Equal[0], SeamChatMessage> + >; + + expect(true).toBe(true); + }); +}); + describe('the three renderer call sites go through the adapter', () => { // The runtime net over the block above, for the `pnpm test` lane that erases // type assertions. The failure it catches is a cast coming BACK: `as any` on @@ -258,6 +407,31 @@ describe('the Date → ISO absorption is expressed once', () => { 'copy here is the third dialect the card exists to prevent.', ).toBe(false); }); + + it('asserts API-mode messages into the authoring type no more', () => { + // objectui#4424, the runtime net over the compile-time pins above. The + // defect was ONE expression: `uiMessagesToChatMessages(...)` asserted into + // the authoring array type. A cast coming back here re-erases every + // render-only key without changing any other line — and `tsc` stays green, + // because a cast is exactly the instruction to stop checking. + // + // Comment lines are dropped first, and that is not incidental: the hook + // NAMES the deleted expression in the comment that explains why it is gone + // (as does this test), so a guard reading raw source would fire on the + // documentation of its own fix. Prose about a cast is not a cast. + const codeOnly = source + .split('\n') + .filter((line) => !/^\s*(\/\/|\/\*|\*)/.test(line)) + .join('\n'); + expect( + /as\s+OuiChatMessage\[\]/.test(codeOnly), + 'useObjectChat.ts casts its API-mode messages into the @object-ui/types ' + + 'authoring contract again. That cast erases `buildProgress`, ' + + '`blueprintProgress`, `charts` and the HITL / draft-review extensions on ' + + 'every tool invocation (objectui#4424). The hook declares ' + + '`ObjectChatMessage`, which those values satisfy — assign, do not assert.', + ).toBe(false); + }); }); describe('the barrel no longer declares a message shape of its own', () => { diff --git a/packages/plugin-chatbot/src/__tests__/chatMessageAdapter.test.ts b/packages/plugin-chatbot/src/__tests__/chatMessageAdapter.test.ts index 0e4125d52a..cc9e3c6a24 100644 --- a/packages/plugin-chatbot/src/__tests__/chatMessageAdapter.test.ts +++ b/packages/plugin-chatbot/src/__tests__/chatMessageAdapter.test.ts @@ -26,6 +26,7 @@ import { toRuntimeTimestamp, toRuntimeToolInvocation, toRuntimeToolState, + type SeamChatMessage, } from '../chatMessageAdapter'; const base: AuthoredChatMessage = { id: 'm1', role: 'user', content: 'hi' }; @@ -106,13 +107,21 @@ describe('toolInvocations: the legacy state vocabulary (the fourth drift)', () = }); describe('pass-through: the API-mode payload survives the seam', () => { - // In API mode `useObjectChat` hands the renderer RUNTIME messages wearing the - // authoring type (`uiMessagesToChatMessages(...) as OuiChatMessage[]`), so the - // values arriving here carry keys the authoring contract does not declare. - // The `as any` casts preserved them by accident; the adapter must preserve - // them on purpose, or the HITL approval card, the "Review N changes" - // affordance and the build panel all vanish from the SDUI renderers. - const apiModeMessage = { + // In API mode `useObjectChat` hands the renderer messages carrying keys the + // authoring contract does not declare. The `as any` casts preserved them by + // accident; the adapter must preserve them on purpose, or the HITL approval + // card, the "Review N changes" affordance and the build panel all vanish from + // the SDUI renderers. + // + // objectui#4424 — read the ANNOTATION, it is half the test. This fixture used + // to need `as unknown as AuthoredChatMessage`: the authoring type declares + // none of these keys, so the only way to hand them to the adapter was to + // assert past the compiler, which is the same blindness the hook's own + // `as OuiChatMessage[]` had. `SeamChatMessage` names them, so the fixture is + // now type-checked: misspell `buildProgress`, or give `draftReview` the wrong + // shape, and this file goes red at compile time instead of quietly testing a + // payload the seam was never going to see. + const apiModeMessage: SeamChatMessage = { id: 'm2', role: 'assistant', content: 'built it', @@ -128,7 +137,7 @@ describe('pass-through: the API-mode payload survives the seam', () => { draftReview: { items: [{ type: 'object', name: 'Loan' }] }, }, ], - } as unknown as AuthoredChatMessage; + }; it('keeps the runtime-only message keys', () => { const runtime = authoredToRuntimeMessage(apiModeMessage); diff --git a/packages/plugin-chatbot/src/__tests__/useObjectChat.honestMessages.test.tsx b/packages/plugin-chatbot/src/__tests__/useObjectChat.honestMessages.test.tsx new file mode 100644 index 0000000000..bbc3065a7b --- /dev/null +++ b/packages/plugin-chatbot/src/__tests__/useObjectChat.honestMessages.test.tsx @@ -0,0 +1,181 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * `useObjectChat` — the declared message type tells the truth about both modes + * (objectui#4424). + * + * The behavioural half of the card. `chat-message-contract.test.ts` pins the + * TYPE; this file pins the VALUES the type is a statement about, and the two + * only mean something together: the defect was a declaration that disagreed + * with the values flowing through it, so proving either one alone reproduces + * exactly the blindness that shipped. + * + * Read the ASSERTIONS as type assertions too. Every `messages[0].buildProgress` + * / `.toolInvocations?.[0]?.pendingActionId` below is written WITHOUT a cast — + * under the old declaration (`messages: OuiChatMessage[]`, the authoring + * contract) not one of them would compile, which is precisely why nothing was + * reading these keys off the hook and why a field-by-field rebuild anywhere on + * the path would have deleted them with the compiler's blessing. + */ + +import { renderHook, act } from '@testing-library/react'; +import { vi, describe, it, expect } from 'vitest'; +import { useObjectChat, type UseObjectChatOptions } from '../useObjectChat'; + +const API = 'https://example.test/api/v1/ai/agents/build/chat'; + +/** + * One assistant turn as the AI SDK holds it: text, a tool call whose result is + * an ADR-0033 pending-approval envelope, the reconciled build-progress part and + * an inline chart. Every runtime-only key the card names is reachable from it. + */ +const { AI_MESSAGES } = vi.hoisted(() => ({ + AI_MESSAGES: [ + { + id: 'm-built', + role: 'assistant', + metadata: { conversationId: 'conv-1' }, + parts: [ + { type: 'text', text: 'Built your app.' }, + { + type: 'tool-apply_blueprint', + toolCallId: 'call_build_1', + state: 'output-available', + input: { appLabel: 'Loans' }, + output: { status: 'pending_approval', pendingActionId: 'pa_42' }, + }, + { + type: 'data-build-progress', + id: 'bp-1', + data: { + phase: 'done', + appLabel: 'Loans', + items: [{ type: 'app', name: 'loans' }], + done: 1, + total: 1, + }, + }, + { + type: 'data-chart', + id: 'chart-1', + data: { + chartType: 'bar', + title: 'Loans by status', + data: [{ status: 'open', n: 2 }], + xAxisKey: 'status', + series: [{ dataKey: 'n' }], + }, + }, + ], + }, + ], +})); + +vi.mock('@ai-sdk/react', () => ({ + useChat: () => ({ + messages: AI_MESSAGES, + status: 'ready', + error: undefined, + sendMessage: vi.fn(), + regenerate: vi.fn(), + stop: vi.fn(), + setMessages: vi.fn(), + }), +})); + +describe('API mode: the runtime-only keys are on the hook, not just under it', () => { + it('exposes buildProgress, charts and metadata on the returned messages', () => { + const { result } = renderHook(() => useObjectChat({ api: API, conversationId: 'c1' })); + + const [message] = result.current.messages; + expect(message.content).toBe('Built your app.'); + // The build panel. Erased by the old `as OuiChatMessage[]`. + expect(message.buildProgress).toMatchObject({ phase: 'done', appLabel: 'Loans', total: 1 }); + // Inline charts — same erasure. + expect(message.charts).toHaveLength(1); + expect(message.charts?.[0]).toMatchObject({ chartType: 'bar', title: 'Loans by status' }); + // `metadata` is the key the hook splices back in; the authoring contract + // DID declare this one, so it is the control in this experiment. + expect(message.metadata).toEqual({ conversationId: 'conv-1' }); + }); + + it('exposes the HITL approval extension on the tool invocation', () => { + const { result } = renderHook(() => useObjectChat({ api: API, conversationId: 'c1' })); + + const tool = result.current.messages[0]?.toolInvocations?.[0]; + expect(tool?.toolName).toBe('apply_blueprint'); + // The approve/reject card's entire wiring: without `pendingActionId` there + // is no id to POST a decision for, and `useHitlInChat` indexes nothing. + expect(tool?.pendingActionId).toBe('pa_42'); + expect(tool?.state).toBe('approval-requested'); + }); + + it('hands onSend the SAME shape it hands `messages`', () => { + // The survey's second half. `onSend(content, messages)` is fed from the + // same array, so a declaration that was honest for one and not the other + // would be a new drift rather than a fix. + const onSend = vi.fn(); + const { result } = renderHook(() => + useObjectChat({ api: API, conversationId: 'c1', onSend }), + ); + + act(() => { + result.current.sendMessage('and now a chart'); + }); + + expect(onSend).toHaveBeenCalledTimes(1); + // Typed by the PUBLISHED option type, so this line reads the same + // declaration a host would — including the element type under test. + const [content, messages] = onSend.mock.calls[0] as Parameters< + NonNullable + >; + expect(content).toBe('and now a chart'); + // …the thread as it will be after this send: the built turn, then the new + // user message. The built turn still carries its render-only keys. + expect(messages).toHaveLength(2); + expect(messages[0]?.buildProgress?.phase).toBe('done'); + expect(messages[0]?.toolInvocations?.[0]?.pendingActionId).toBe('pa_42'); + expect(messages[1]).toMatchObject({ role: 'user', content: 'and now a chart' }); + }); +}); + +describe('local mode: the authoring-only values the runtime type would have lied about', () => { + it("keeps an authored 'tool' role and absorbs an authored Date", () => { + // This is the measurement that decided the shape of the honest type. If + // local mode were also runtime-shaped, `ChatbotEnhanced`'s ChatMessage + // would simply BE the truth; it is not, in two named ways — and both are + // deliberate (`normalizeMessages` narrows neither role nor tool state; the + // fold happens at the render seam). + const { result } = renderHook(() => + useObjectChat({ + initialMessages: [ + { + id: 'a1', + role: 'tool', + content: 'tool said x', + timestamp: new Date('2026-08-12T02:08:13.000Z'), + toolInvocations: [ + { toolCallId: 'call_legacy', toolName: 'lookup', state: 'result' }, + ], + }, + ], + }), + ); + + expect(result.current.isApiMode).toBe(false); + const [message] = result.current.messages; + // Wide where local mode is wide: the runtime contract has no 'tool' role + // and no 'result' state, so declaring it here would have been a lie. + expect(message.role).toBe('tool'); + expect(message.toolInvocations?.[0]?.state).toBe('result'); + // Narrow where BOTH modes are narrow: an authored `Date` never leaves this + // hook, so `timestamp?: string` is the honest declaration. + expect(message.timestamp).toBe('2026-08-12T02:08:13.000Z'); + }); +}); diff --git a/packages/plugin-chatbot/src/chatMessageAdapter.ts b/packages/plugin-chatbot/src/chatMessageAdapter.ts index 5ff6311bc0..3ce6572df1 100644 --- a/packages/plugin-chatbot/src/chatMessageAdapter.ts +++ b/packages/plugin-chatbot/src/chatMessageAdapter.ts @@ -40,21 +40,37 @@ * ## Pass-through — why this is a conversion and not a reconstruction * * The adapter narrows the drifting fields **by name** and spreads the rest. It - * deliberately does NOT rebuild the message field-by-field from the authoring - * type, because in API mode the values arriving here are already RUNTIME - * messages wearing the authoring type: `useObjectChat` builds them with - * `uiMessagesToChatMessages(...) as OuiChatMessage[]` (see - * `useObjectChat.ts`), so at runtime they carry `buildProgress`, - * `blueprintProgress`, `charts` and tool invocations bearing the HITL / - * draft-review extensions — every one of which the authoring type erases. A - * field-by-field rebuild would silently drop the approval cards, the "Review N - * changes" affordance and the build panel. The cast used to preserve them by - * accident; the spread preserves them on purpose. + * deliberately does NOT rebuild the message field-by-field, because the values + * arriving here are not always plain authored ones: in API mode they are + * RUNTIME messages carrying `buildProgress`, `blueprintProgress`, `charts` and + * tool invocations bearing the HITL / draft-review extensions. A field-by-field + * rebuild would silently drop the approval cards, the "Review N changes" + * affordance and the build panel. + * + * What changed in objectui#4424: those keys are no longer preserved *by faith*. + * The hook used to declare its output as the authoring type and reach the + * runtime values through `uiMessagesToChatMessages(...) as OuiChatMessage[]`, + * so this module could only ever have SPREAD keys it could not see. The hook + * now declares {@link ObjectChatMessage} — the truth about both of its modes — + * and this seam accepts {@link SeamChatMessage}, which names the runtime-only + * keys as optional members. The spread therefore carries them as DECLARED + * properties: `tsc` can see what survives, and the pass-through tests type + * their API-mode fixture directly instead of casting it into place. * * The compile-time value of the seam is unaffected by that: a new authored * `role` makes {@link toRuntimeRole} unassignable, and a newly REQUIRED runtime * key makes {@link authoredToRuntimeMessage}'s return type red. Both are the * type error the cast used to swallow. + * + * ## Why the seam's INPUT is wider than the hook's OUTPUT + * + * {@link SeamChatMessage} keeps authoring's `timestamp?: string | Date`, while + * {@link ObjectChatMessage} narrows it to `string`. That is not an oversight: + * both hook modes absorb the `Date` before they emit (via + * {@link toRuntimeTimestamp}), but this module is exported from the barrel for + * hosts holding raw authored messages — the `Date` still has to die somewhere, + * and this is the somewhere. `AuthoredChatMessage` remains assignable to + * `SeamChatMessage`, so every existing caller is untouched. */ import type { @@ -66,6 +82,55 @@ import type { ChatToolInvocation as RuntimeToolInvocation, } from './ChatbotEnhanced'; +/** + * The render-only MESSAGE keys — declared by the runtime contract, never by the + * authoring one. Named here (rather than spelled out at each use) so that + * adding a render-only key to `ChatbotEnhanced.ChatMessage` and forgetting this + * list is a one-line fix in one place. objectui#4424. + */ +type RuntimeOnlyMessageKeys = Pick< + RuntimeChatMessage, + 'buildProgress' | 'blueprintProgress' | 'charts' +>; + +/** + * The render-only TOOL-INVOCATION keys — the HITL approval id, the ADR-0033 + * draft review, the proposed plan / changes cards and the ADR-0057 P4 builder + * handoff. `mapMessages.ts` lifts every one of them out of a tool result; the + * authoring contract declares none of them. objectui#4424. + */ +type RuntimeOnlyToolInvocationKeys = Pick< + RuntimeToolInvocation, + 'pendingActionId' | 'draftReview' | 'proposedPlan' | 'proposedChanges' | 'builderHandoff' +>; + +/** + * One tool invocation as it actually crosses this seam: the authoring contract, + * plus the render-only extensions it may ALREADY be carrying when it arrives + * from API mode. + * + * `AuthoredToolInvocation` stays assignable to this (every added key is + * optional), so a host holding plain authored invocations is unaffected. + */ +export type SeamToolInvocation = AuthoredToolInvocation & + Partial; + +/** + * One message as it actually crosses this seam — the seam's INPUT contract. + * + * The authoring shape (`role: 'tool'`, `timestamp: Date`, legacy tool states — + * all still narrowed below) widened by the render-only keys an API-mode value + * already carries. This is what lets the pass-through spread preserve those + * keys as declared properties rather than as invisible runtime baggage + * (objectui#4424); see "Pass-through" in the module doc. + * + * `AuthoredChatMessage` is assignable to it, and so is the hook's + * `ObjectChatMessage` — the two callers this seam serves. + */ +export type SeamChatMessage = Omit & { + toolInvocations?: SeamToolInvocation[]; +} & Partial; + /** * `timestamp: string | Date` -> `string | undefined`. * @@ -158,23 +223,30 @@ export function toRuntimeToolState( * Only `state` drifts; everything else is spread through, which is what keeps * the runtime-only extensions (`pendingActionId`, `draftReview`, * `proposedPlan`, `proposedChanges`, `builderHandoff`) alive on the API-mode - * path — see the module doc. + * path. Since objectui#4424 the parameter names them ({@link + * SeamToolInvocation}), so `passthrough` carries them as declared properties + * instead of as keys the compiler cannot see — see the module doc. */ export function toRuntimeToolInvocation( - tool: AuthoredToolInvocation, + tool: SeamToolInvocation, ): RuntimeToolInvocation { const { state, ...passthrough } = tool; return { ...passthrough, state: toRuntimeToolState(state) }; } /** - * One authored chat message -> the message shape the chat components render. + * One chat message -> the message shape the chat components render. * * This is the seam. The three `messages as any` casts in `renderer.tsx` are * this function now. + * + * The parameter is {@link SeamChatMessage} rather than the bare authoring type + * (objectui#4424): both are accepted — `AuthoredChatMessage` is assignable to + * it — but naming the render-only keys is what makes the pass-through + * compiler-visible instead of a documented act of faith. */ export function authoredToRuntimeMessage( - message: AuthoredChatMessage, + message: SeamChatMessage, ): RuntimeChatMessage { const { role, timestamp, toolInvocations, ...passthrough } = message; return { @@ -195,7 +267,7 @@ export function authoredToRuntimeMessage( * and memos off the `messages` prop. */ export function toRuntimeMessages( - messages: readonly AuthoredChatMessage[] | undefined, + messages: readonly SeamChatMessage[] | undefined, ): RuntimeChatMessage[] { return (messages ?? []).map(authoredToRuntimeMessage); } diff --git a/packages/plugin-chatbot/src/index.tsx b/packages/plugin-chatbot/src/index.tsx index 83ef3a5a81..0344f737a7 100644 --- a/packages/plugin-chatbot/src/index.tsx +++ b/packages/plugin-chatbot/src/index.tsx @@ -242,6 +242,14 @@ export { Chatbot, TypingIndicator } // Export the composable chat hook for custom integrations export { useObjectChat } from './useObjectChat'; export type { UseObjectChatOptions, UseObjectChatReturn } from './useObjectChat'; +/** + * What `useObjectChat` emits from `messages` and `onSend` — neither the + * `@object-ui/types` AUTHORING contract (which the hook used to declare while + * API mode cast RUNTIME values into it) nor the runtime one (which would be a + * lie about local mode's authored `'tool'` role and legacy tool states). See + * the type's own doc for the survey that decided it (objectui#4424). + */ +export type { ObjectChatMessage } from './useObjectChat'; // ADR-0057 #8 — the AI-usage-indicator refresh seam (emitted on turn-finish / 429). export { AI_USAGE_REFRESH_EVENT, emitAiUsageRefresh } from './useObjectChat'; @@ -371,6 +379,16 @@ export { toRuntimeToolState, } from './chatMessageAdapter'; +/** + * The seam's INPUT contract: the authoring shape widened by the render-only + * keys an API-mode value already carries. A host converting its own messages + * with `toRuntimeMessages` never needs to name this — `ChatMessage` from + * `@object-ui/types` is assignable to it — but a host that HOLDS such values + * (having driven `useChat` itself, say) can now say so instead of casting + * (objectui#4424). + */ +export type { SeamChatMessage, SeamToolInvocation } from './chatMessageAdapter'; + // Display helpers used internally by ChatbotEnhanced. Exported so app // authors composing their own chat surface get the same pretty tool-call // rendering and friendly error summaries for free. diff --git a/packages/plugin-chatbot/src/renderer.tsx b/packages/plugin-chatbot/src/renderer.tsx index 236e94e629..5b68db740c 100644 --- a/packages/plugin-chatbot/src/renderer.tsx +++ b/packages/plugin-chatbot/src/renderer.tsx @@ -8,11 +8,12 @@ import { useMemo } from 'react'; import { ComponentRegistry } from '@object-ui/core'; -import type { ChatbotSchema, ChatMessage } from '@object-ui/types'; +import type { ChatbotSchema } from '@object-ui/types'; import { Chatbot } from './index'; import { ChatbotEnhanced } from './ChatbotEnhanced'; import { FloatingChatbot } from './FloatingChatbot'; import { useObjectChat } from './useObjectChat'; +import type { ObjectChatMessage } from './useObjectChat'; import { toRuntimeMessages } from './chatMessageAdapter'; /** @@ -32,7 +33,10 @@ import { toRuntimeMessages } from './chatMessageAdapter'; * - Schema fields: autoResponse, autoResponseText, autoResponseDelay * * Both modes support the `onSend` callback: - * - Signature: `onSend(content: string, messages: ChatMessage[]): void` + * - Signature: `onSend(content: string, messages: ObjectChatMessage[]): void` + * — the hook's own message shape (objectui#4424). A host callback that + * declares `@object-ui/types`' `ChatMessage[]` still type-checks; naming + * `ObjectChatMessage` is what lets it read the render-only keys. */ ComponentRegistry.register('chatbot', ({ schema, className, ...props }: { schema: ChatbotSchema & { @@ -46,7 +50,7 @@ ComponentRegistry.register('chatbot', autoResponse?: boolean; autoResponseText?: string; autoResponseDelay?: number; - onSend?: (content: string, messages: ChatMessage[]) => void; + onSend?: (content: string, messages: ObjectChatMessage[]) => void; }; className?: string; [key: string]: any }) => { const { messages, @@ -252,7 +256,7 @@ ComponentRegistry.register('chatbot-enhanced', autoResponse?: boolean; autoResponseText?: string; autoResponseDelay?: number; - onSend?: (content: string, messages: ChatMessage[]) => void; + onSend?: (content: string, messages: ObjectChatMessage[]) => void; onClear?: () => void; }; className?: string; [key: string]: any }) => { const { @@ -383,7 +387,7 @@ ComponentRegistry.register('chatbot-floating', autoResponse?: boolean; autoResponseText?: string; autoResponseDelay?: number; - onSend?: (content: string, messages: ChatMessage[]) => void; + onSend?: (content: string, messages: ObjectChatMessage[]) => void; onClear?: () => void; }; className?: string; [key: string]: any }) => { const { diff --git a/packages/plugin-chatbot/src/useObjectChat.ts b/packages/plugin-chatbot/src/useObjectChat.ts index c3251bf163..1262875482 100644 --- a/packages/plugin-chatbot/src/useObjectChat.ts +++ b/packages/plugin-chatbot/src/useObjectChat.ts @@ -13,6 +13,49 @@ import { DefaultChatTransport } from 'ai'; import { generateUniqueId } from './utils'; import { uiMessagesToChatMessages } from './mapMessages'; import { toRuntimeTimestamp } from './chatMessageAdapter'; +import type { SeamChatMessage } from './chatMessageAdapter'; + +/** + * What `useObjectChat` actually emits — from `messages` and from the + * `onSend(content, messages)` callback fed from it (objectui#4424). + * + * The hook used to declare both as the `@object-ui/types` AUTHORING contract. + * In local mode that was true; in API mode it was a cast over values produced + * by the RUNTIME mapper, so the declared type was narrower than the values in + * exactly the direction that hides capability: anyone rebuilding a message + * field-by-field from its declared type deleted the HITL approval card, the + * "Review N changes" affordance, the proposed-plan card, the build panel and + * the inline charts, with the compiler agreeing. + * + * This type is what the survey found to be true of BOTH modes — neither the + * authoring type nor the runtime type, but the shape that admits both: + * + * - **wide where local mode is wide.** An authored `'tool'` role and the + * legacy `'partial-call'`/`'call'`/`'result'` tool states reach this + * surface unchanged and are folded only at the render seam, which is the + * decision `chatMessageAdapter.ts` records. So the runtime type would have + * been a lie about local mode. + * - **narrow where BOTH modes are narrow.** `timestamp` is `string`, never + * `Date`: API mode never produces one and local mode absorbs it in + * `normalizeMessages` before it is ever handed out. Declaring `Date` here + * asks every consumer to handle a value that cannot arrive. + * - **plus the render-only keys API mode really carries** — + * `buildProgress`, `blueprintProgress`, `charts`, and the HITL / + * draft-review / proposed-plan / builder-handoff extensions on each tool + * invocation. + * + * It is a SUBTYPE of `@object-ui/types`' `ChatMessage`, which is what makes the + * change invisible to correct consumers: anything that accepted the authoring + * type still accepts these values, including a host `onSend` callback that + * declares its parameter as `ChatMessage[]`. + */ +export type ObjectChatMessage = Omit & { + /** + * Always a string here (or absent). Both modes absorb an authored `Date` + * via `toRuntimeTimestamp` before emitting — see `chatMessageAdapter.ts`. + */ + timestamp?: string; +}; /** * Window event the AI usage indicator (ADR-0057 #8) listens for to refetch its @@ -222,16 +265,25 @@ export interface UseObjectChatOptions { autoResponseDelay?: number; /** * External send callback (fires for both modes). + * + * `messages` is the thread as it will be after this send, in the same shape + * the hook's own `messages` uses — see {@link ObjectChatMessage}. A callback + * that declares the parameter as `@object-ui/types`' `ChatMessage[]` still + * type-checks (the emitted shape is a subtype); declaring it as + * `ObjectChatMessage[]` is what lets you READ the render-only keys. */ - onSend?: (content: string, messages: OuiChatMessage[]) => void; + onSend?: (content: string, messages: ObjectChatMessage[]) => void; } /** * Return type of useObjectChat. */ export interface UseObjectChatReturn { - /** Current chat messages */ - messages: OuiChatMessage[]; + /** + * Current chat messages — see {@link ObjectChatMessage} for why this is + * neither the authoring nor the runtime `ChatMessage` (objectui#4424). + */ + messages: ObjectChatMessage[]; /** Whether the assistant is currently generating a response */ isLoading: boolean; /** Current error, if any */ @@ -265,12 +317,15 @@ export interface UseObjectChatReturn { * still applies at this point because the hook's own `messages` output — and * the `onSend(content, messages)` callback fed from it — has always handed * hosts an ISO string rather than a `Date`; one expression, two consumers. + * Since objectui#4424 the return type SAYS so, which is the half of the + * statement that used to be missing. * * Roles are deliberately NOT narrowed here: an authored `'tool'` message keeps - * its authored role for the whole of the hook's (authoring-typed) surface and - * is folded to `'assistant'` only at the render seam. + * its authored role for the whole of the hook's surface and is folded to + * `'assistant'` only at the render seam. That is precisely why the honest + * output type is not the runtime one — see {@link ObjectChatMessage}. */ -function normalizeMessages(msgs?: OuiChatMessage[]): OuiChatMessage[] { +function normalizeMessages(msgs?: OuiChatMessage[]): ObjectChatMessage[] { return (msgs ?? []).map((msg, idx) => ({ id: msg.id || `msg-${idx}`, role: msg.role || 'user', @@ -477,7 +532,7 @@ export function useObjectChat(options: UseObjectChatOptions = {}): UseObjectChat }, [chatStatus]); // --- Local/legacy mode state --- - const [localMessages, setLocalMessages] = useState( + const [localMessages, setLocalMessages] = useState( () => normalizeMessages(initialMessages) ); const [localIsLoading, setLocalIsLoading] = useState(false); @@ -514,22 +569,31 @@ export function useObjectChat(options: UseObjectChatOptions = {}): UseObjectChat const isLoading = status === 'submitted' || status === 'streaming'; - // Vercel AI SDK v6 UIMessage → OUI ChatMessage. The shared mapper handles - // parts (text, reasoning, tool-*, source-*), streaming-cursor flagging, - // and legacy `msg.toolInvocations` fallback. We splice `metadata` back in - // because `ChatbotEnhanced.ChatMessage` doesn't carry it but OUI's does. - const apiMessages: OuiChatMessage[] = uiMessagesToChatMessages(aiMessages, { + // Vercel AI SDK v6 UIMessage → the runtime ChatMessage. The shared mapper + // handles parts (text, reasoning, tool-*, source-*), streaming-cursor + // flagging, and legacy `msg.toolInvocations` fallback. We splice `metadata` + // back in because `ChatbotEnhanced.ChatMessage` doesn't carry it but the + // authoring contract does. + // + // objectui#4424: this used to end in `as OuiChatMessage[]`, and that cast was + // the card. It erased `buildProgress`, `blueprintProgress`, `charts` and + // every HITL / draft-review extension on the tool invocations, because the + // authoring type declares none of them — the values survived only because + // nothing downstream rebuilt a message. There is no assertion here now: the + // mapper's output IS an `ObjectChatMessage`, so the compiler checks the + // assignment instead of being told to stop looking. + const apiMessages: ObjectChatMessage[] = uiMessagesToChatMessages(aiMessages, { isStreaming: isLoading, }).map((m, idx) => ({ ...m, metadata: (aiMessages[idx] as any)?.metadata, - })) as OuiChatMessage[]; + })); const sendMessage = useCallback( (content: string) => { const trimmed = content.trim(); if (!trimmed) return; - const nextMessages: OuiChatMessage[] = [ + const nextMessages: ObjectChatMessage[] = [ ...apiMessages, { id: generateUniqueId('msg'), role: 'user', content: trimmed }, ]; @@ -563,7 +627,7 @@ export function useObjectChat(options: UseObjectChatOptions = {}): UseObjectChat const localSendMessage = useCallback((content: string) => { if (!content.trim()) return; - const userMessage: OuiChatMessage = { + const userMessage: ObjectChatMessage = { id: generateUniqueId('msg'), role: 'user', content: content.trim(), @@ -581,7 +645,7 @@ export function useObjectChat(options: UseObjectChatOptions = {}): UseObjectChat if (autoResponse) { setLocalIsLoading(true); autoResponseTimerRef.current = setTimeout(() => { - const assistantMessage: OuiChatMessage = { + const assistantMessage: ObjectChatMessage = { id: generateUniqueId('msg'), role: 'assistant', content: autoResponseText || 'Thank you for your message!',