Repository navigation
refactor(plugin-chatbot): drop the inert blanket cast on useChat's options object - #8401
Merged
Merged
Conversation
…options object
`useChat({...} as any)` widened the whole options argument to `any`, switching
off checking of `transport`, `onError` and excess properties, and blocking
inference of `UI_MESSAGE`. Measured: with the nested `(aiInitialMessages as any)`
in place, the outer cast hides nothing — both type-check projects stay at exit 0
with zero diagnostics without it — and the resolved instantiation is unchanged,
`UseChatHelpers<UIMessage<unknown, UIDataTypes, UITools>>` either way, because
`transport` is the only inference source and it is already pinned to that type.
This narrows the suppression rather than eliminating it: removing BOTH casts is
red (TS2322, `Record<string, unknown>` is not a `UIMessagePart`), so the nested
cast is the live one. A comment now records that, so the next reader does not
delete it blind or re-widen the call.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
…ng nothing Empty frontmatter, the gate's documented exemption. Argued from measurement: building @object-ui/plugin-chatbot from this branch and from the merge-base produced a byte-identical dist across all 34 published artifacts, and that comparison was proved live by a control mutation to an emitted string literal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
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
|
os-justin
marked this pull request as ready for review
September 7, 2026 22:23
akarma-synetal
pushed a commit
to akarma-synetal/objectui
that referenced
this pull request
Sep 28, 2026
…nd delete the last cast on useChat (objectui#8426) (objectstack-ai#10017) Fixes objectstack-ai#8426 The API-mode parts builder in `useObjectChat` declared its part array as `Array(Record(string, unknown))` and pushed plain objects into it. That is not the chat runtime's part union, and the mismatch was absorbed by an `as any` on the `messages` option rather than reported. This makes the builder **construct** the discriminated parts, so the option is checked and the cast is gone. Ruling carried in, not re-decided: director seat, decision batch objectstack-ai#86, 2026-09-08, **option A — contract-first**. Its first clause landed as objectui#9229; this is the remaining one. The three shortcuts the dispatch forbids by name (widen the declared type to `any`; keep the cast with a comment; re-litigate A) are all absent. ## The 2x2, re-run on this branch before repairing Each leg mutated on disk through an anchor that must hit, with blob-hash proof, and restored by state (`git diff HEAD` empty after every leg). Two type-check programs per leg: `tsc --noEmit` and `tsc -p tsconfig.test.json`. | outer `} as any)` | nested `(aiInitialMessages as any)` | main | test | |:--|:--|:--|:--| | present | present | exit 0 | exit 0 | | absent (today's `main`, PR objectstack-ai#8401) | present | exit 0 | exit 0 | | present | removed | exit 0 | exit 0 | | **absent** | **removed** | **exit 2**, 1x TS2322 | **exit 2**, 1x TS2322 | All four legs reproduce the card's table. The red leg's diagnostic, verbatim: ``` error TS2322: Type '{ id: string; role: "user" | "assistant" | "system"; parts: Record(string, unknown)[]; }[] | undefined' is not assignable to type 'UIMessage(unknown, UIDataTypes, UITools)[] | undefined' ... Type 'Record(string, unknown)' is not assignable to type 'UIMessagePart(UIDataTypes, UITools)' ``` (angle brackets transliterated to round ones above so the body survives sanitizing.)⚠️ **One thing did not survive, and it is a method note rather than a finding about the card.** The first attempt at the two nested-cast legs was a NO-OP: the mutation tool refused the write because the replacement text was a substring of the anchor, so its rise-count check could not distinguish the two. The file was restored and both legs then ran against a **pristine** tree — reading exit 0, i.e. a false green pointing the wrong way. They were re-run with a distinguishable replacement, and the table above is the re-run. Recorded because a silently-reverted ablation reads exactly like a passing one. ## After: both casts gone, both programs green ``` pnpm --filter @object-ui/plugin-chatbot type-check -> exit 0 (tsc --noEmit && tsc -p tsconfig.test.json) pnpm exec vitest run packages/plugin-chatbot/ -> exit 0 47 files, 522 tests passed ``` Program membership proved with a lit control rather than assumed — main program 1626 files: `src/useObjectChat.ts` 1, `src/mapMessages.ts` (positive control) 1, `useObjectChat.honestMessages.test.tsx` (negative control) 0, and the same negative pattern returns 1 against the test program's file list. Dependents, on a freshly built union closure (`Scope: 36 of 47` built, then `Scope: 7 of 47` type-checked): all seven pass, zero `error TS` — `plugin-chatbot`, `app-shell`, `console`, `site`, `example-schema-catalog`, `example-console-starter`, `example-byo-backend-console`. That is the measurement behind the claim that the published-input narrowing breaks nothing in this repository. ## Reverse verification — the green is not vacuous Both legs run from the **committed** fix, mutated through an anchor that must hit, restored to a byte-identical blob, and the restore leg re-run. 1. **Type.** Put the dead `toolName` back on a constructed tool arm. Predicted red; measured red: `exit 2`, `TS2353: Object literal may only specify known properties, and 'toolName' does not exist in type ...`. Restored: exit 0. 2. **Test.** Delete the HITL carve-out in the un-backed-approval report. Predicted red, and only the HITL test; measured exactly that: `Tests 1 failed | 11 passed (12)`, the failure being `says NOTHING when the invocation is an ObjectStack HITL approval`. Restored: 12 passed. ## What changed - **The three approval states are now reachable.** `approval-requested`, `approval-responded` and `output-denied` require the runtime's `approval` envelope beside them; `ChatToolInvocation` gained it in objectui#9229 and the builder now constructs those arms from it. The per-arm shapes differ (`approved` is forbidden on the outstanding request, required on the response, pinned to `false` on the denial) so each is built rather than spread. - **`tool-${toolName}` is expressible.** The dynamic member the card feared was the blocker is not one: the runtime's tool set is open, so the mapped type collapses to an index signature and the discriminant is a template. Pinned by a test. - **Legacy authoring states are folded, not passed through.** `partial-call` / `call` / `result` are not runtime states; passing them through left the round-trip reader refusing them, so the invocation came back stateless. Each test pairs the folded subject with a control asserting the authored spelling is still rejected by the runtime's own `validateUIMessages`. - **The dead `toolName` excess property is dropped.** Only the dynamic-tool arm declares one, and the reader derives the name off `type`, so this is behaviour-preserving. - **`UseObjectChatOptions.initialMessages`' optional `parts` narrows** to the store's own part array. Breaking for an external host that passes pre-built parts; `minor` per this repo's version policy, with the breaking note carried in the changeset. - **`mapMessages`' tool-invocation extraction lifts `approval`.** Assigned to this card when objectui#9229 shipped the additive half, and deliberately held until its first reader existed — lifting it earlier would have minted a declared-but-unread key. It closes the recorded asymmetry where the hydrated path carried the envelope and the live path dropped it. An invocation claiming an approval state with **no** envelope to back it is not constructible, and no envelope is invented for it (inventing `approval.id` is the fabrication AGENTS.md #0.1 forbids): the state is derived from the data the invocation does carry, and the producer is warned once, by name. An ObjectStack HITL approval is deliberately **not** warned about — it is carried by `pendingActionId` and a `pending_approval` result, and the mapper re-promotes the state from that result on the way back out, so the approval card survives the round trip. ## Acceptance notes⚠️ **The ruling's residual clause is NOT in this PR, and it is not silently dropped.** The ruling says the authoring `state` union should shed the three runtime-only approval states so that "a schema-authored invocation cannot claim `approval-requested` without an envelope". That union lives on `ChatToolInvocation` in `@object-ui/types`, which objectui#9229 left alone and whose own doc records the narrowing as this card's. The dispatch fences `packages/types` **out** with "if you find it incomplete, report; do not extend it here". So the conflict is reported rather than resolved by me: the narrowing is what would delete the un-backed-approval branch added here by construction, and it wants its own card or an explicit widening of this one's fence. Out of scope, noted, **not** filed: - `(aiMessages[idx] as any)?.metadata` in the same file is untouched and unrelated to this card's call. Successor: the next card on this file's message-output path. - objectui#9233 (`mergeToolResultsInto` rewriting `state` to `output-available` for every merged result) is recorded on this card as possibly changing what "done" means here. It is unchanged by this PR: building the arms does not by itself put an approval card in front of an operator on that sub-path. Successor: objectui#9233 itself, which is open. - objectui#9232 (the localStorage cache write side rebuilding tool parts without `approval`) is likewise untouched and open. Successor: objectui#9232. ## Gates run locally `check:control-bytes` 0 · `check:new-line-citations` 0 (`0 new citation(s)`) · `check:changeset-claims` 0 · `check:pending-changeset-literals` 0 · `check-changeset-presence` 0 (`1 changeset(s)`) · `check-changeset-no-major` 0 · `check-governed-queue-guard --test` — NOT GOVERNED · `eslint --no-inline-config` on the four changed source files exit 0, 9 pre-existing warnings, none from this diff. --- 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq --- _Generated by [Claude Code](https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
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 #8378
Removes
useChat({...} as any)inpackages/plugin-chatbot/src/useObjectChat.ts, and records in a comment what the measurement actually found.Spelling note: generic type arguments are written as WORDS where they appear, because GitHub's body sanitizer eats tag-shaped fragments.
The site, re-derived
The brief said the file had moved twice and not to trust
:735. Re-derived by content, not by line number:grep -n 'as any'returns four hits, of which one (:368) is a doc comment naming the cast objectui#8342 removed and one (:808) is an unrelated metadata read. The site ispackages/plugin-chatbot/src/useObjectChat.ts:735, the line} as any);, unique in the file.:735was still correct. PR #8377 landed above the site but the intervening edits were net line-neutral at this point in the file. I re-derived it independently rather than trusting it, and the base read below confirms the line number as well as the content — but the dispatch's expectation that the number had moved was wrong.Base side read with a literal sha (
fadf6cd7317e1ac8a011f4e6b986187223c7fdba), interrogated in both directions so it cannot be an index read of my own work:} as any);SdkChatMessage(PR #8377 landed marker)} as any);Base blob
ff06f46f94c81cda552f741484e36e36c5df08fe, branch blob0c3c0da6259ba6725ccb6540a1185428d16a0372.The card asked whether the outer cast is inert on its own merits, or only because the nested
(aiInitialMessages as any)absorbs the mismatch. Measured as a 2x2, each leg mutated on disk (hash proven to differ from the HEAD blob), each restored by state:tsc --noEmittsc -p tsconfig.test.jsonThe bottom row is the answer:
So the outer cast is inert only because the nested one is absorbing a real mismatch. The suppression count on this call goes 2 to 1, not 2 to 0. The card's headline — "this cast is DEAD" — is true only conditionally, and the condition is that the nested cast stays. Row 3 shows the converse: with the nested cast gone the outer one would have been load-bearing.
It is still worth landing, for a reason that is about scope rather than count. The outer cast widened the entire options argument to
any:transport,onError, excess-property checking and the generic instantiation all went unchecked. The nested cast suppresses exactly one property. Removing the outer restores compiler checking of everything exceptmessages. That is real checking newly acquired, not zero — but it is a different claim from the one the card makes, and the card should be re-read in that light.Which instantiation results
The card asked this explicitly. Probed with the TypeScript compiler API against the real program (no mutation), before and after:
chatResulttypeUseChatHelpers<UIMessage<unknown, UIDataTypes, UITools>>anyUseChatHelpers<UIMessage<unknown, UIDataTypes, UITools>>{ transport: DefaultChatTransport<UIMessage<unknown, UIDataTypes, UITools>> or undefined; messages: any; onError: ((err: Error) => void) or undefined; }Identical, in both the main and the test program. Inference does start running, and it lands on exactly the type the default already supplied, so nothing downstream of
chatResultmoves.The declaration-level argument — and where it stops
useChatis declareduseChat<UI_MESSAGE extends UIMessage = UIMessage>(options?: UseChatOptions<UI_MESSAGE>): UseChatHelpers<UI_MESSAGE>, andUseChatOptionsis({ chat: Chat<UI_MESSAGE> } | ChatInit<UI_MESSAGE>) & { throttle?; experimental_throttle?; resume? }. Only three properties are passed, and each contributes toUI_MESSAGEinference as follows:onErroris declaredChatOnErrorCallback, and that istype ChatOnErrorCallback = (error: Error) => void— no type parameter occurs in it at all. This one is genuinely declaration-level and permanent: whatever is passed, this property can never produce an inference candidate forUI_MESSAGE.messagesisanyat the call, and ananysource contributes no inference candidate (it carries none of the object/intersection flags the inference walk requires). Declaration-level as a rule of the algorithm, but contingent on the nested cast staying.transportis therefore the sole inference source, and it isDefaultChatTransport<UIMessage<unknown, UIDataTypes, UITools>>— itself pinned there becausenew DefaultChatTransport({ ... })at :637 supplies no explicit type argument and no candidate either, leaving its ownUI_MESSAGEto fall back to its constraint.I cannot make the strong claim. "This cast is a no-op for every possible instantiation" is not true here, and saying it would be dressing the result up. The honest claim is the weaker one the brief asks for:
That weakness is why the deletion ships with a comment rather than silently: the next reader who removes the nested cast expecting the last suppression to fall away needs to know what they will hit, and needs to not "fix" it by re-widening the call.
Instrument discipline
Both projects, with membership proof.
type-checkfor this package istsc --noEmit && tsc -p tsconfig.test.json; both were run separately for every leg.tsconfig.jsonexcludes tests by directory, andtsconfig.test.jsonincludes only*.test.ts(x), so membership was not assumed:--listFiles | grep -c 'src/useObjectChat\.ts$'= 1; lit control (a made-up filename on the same instrument) = 0useObjectChat.*suites, named in the run)The instrument is not merely present but demonstrably able to fail: row 4 turned both programs red on this exact file.
Restore-integrity / instrument-liveness control. Re-inserting the cast onto the implementation returns both projects to exit 0. Reported as what it is — evidence the tree is unbroken and the harness restores correctly — not as a second reading about the cast, since it restores code
mainkeeps green by construction.Dependents, downstream direction.
pnpm --workspace-concurrency=2 --filter '...@object-ui/plugin-chatbot' type-check, after building the full closure first so a staledistcould not masquerade as a verdict (the TS2307/TS2882 signature objectui#8342 hit).Zero
error TSacross the run; per package:packages/plugin-chatbotDone,packages/app-shellDone,apps/consoleDone,apps/siteDone,examples/schema-catalogDone,examples/console-starterDone,examples/byo-backend-consoleDone.Vitest. Run from the repo root as AGENTS.md requires (
pnpm exec vitest run packages/plugin-chatbot/), which reportsRUN v4.1.10 /home/user/objectui-issue-8378— the repo root, not a package dir. 40 test files passed, 474 tests passed.vitest listnames all sixuseObjectChat.*suites and shows zeroapps/consolefiles, so this is not the objectui#3378 false green. As the brief notes, vitest transpiles types away and can never see a type-level change; it is here to show nothing moved.Lint is the package-scoped
eslint .: exit 0,88 problems (0 errors, 88 warnings). Against the base copy of the same file on the same instrument:89 problems (0 errors, 89 warnings), withno-explicit-anyhits going 13 to 12. The deleted cast was a real reportedany, and exactly one warning went away.Mutation hygiene. Every leg: absolute-path
trap ... EXIT INT TERM, mutation proved on disk bygit hash-objectdiffering fromgit rev-parse HEAD:PATH(empty hash guarded as failure), anchor counted in both directions (deleted text 1 to 0, injected text 1 to 2), restore verified by state — hash equal again andgit diff HEADempty — never by an exit code. The ablation legs ran from a committed implementation.Changeset
node scripts/check-changeset-presence.mjsdecided it, and its verdict line is:Empty frontmatter, argued the way PR #8389 argued it — from a measurement, not by feel. Building
@object-ui/plugin-chatbotfrom this branch and from the merge-base produced a byte-identicaldistacross all 34 published artifacts (.d.ts,.js,.cjs,.css, compared bysha256summanifest).That "identical" verdict was itself proved live, because a comparison that can only ever say "identical" is not a measurement: a control mutation to an emitted string literal in the same file, verified to have reached
dist/index.jsbefore the comparison ran, made the same manifest report differing hashes forindex.jsandindex.umd.cjs. A first control attempt — an unused local const — was tree-shaken away and never reacheddist, and is reported here as void rather than as a dead instrument.scripts/check-changeset-no-major.mjs:✅ No changeset declares a 'major' bump.Noskip-changesetlabel was applied; it is a phantom label in this repo and a labelled bypass was declined twice.pnpm check:control-bytes:✅ check-control-bytes: OK (scanned 6653 tracked text file(s); skipped 85 binary), plus a directgrep -naPcontrol-byte scan of the edited file with a firing lit control.All readings above were re-taken at the final head
4a134d170with a clean working tree.Out of scope, and not filed — needs the PM
The nested
(aiInitialMessages as any)is now measured live, not merely suspected: it hides a genuine TS2322, because theaiInitialMessagesbuilder at :545 constructspartsasRecord<string, unknown>[], which is not aUIMessagePartunion. The real fix is at the producer (build realUIMessagePartvalues), not a wider cast at the consumer — contract-first, AGENTS.md #0.1.I could not file it as its own card: dedupe search was unavailable this session (MCP
search_issuesreturnedAPI rate limit already exceededon two attempts; repo-scoped REST returns 403GitHub access is not enabled for this session, an app-installation policy rather than a quota, while/rate_limititself answers 200). Filing without a dedupe read is not allowed, and dropping the finding silently is not allowed either, so it goes to the PM in the report to file.🤖 Generated with Claude Code
https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Generated by Claude Code