Skip to content

useObjectChat rebuilds its DefaultChatTransport on every render — the memo's body dep is an inline literal every caller recreates (finding, dormant) #4187

Description

@yinlianghui

Observation-class. Nothing a user hits today; filing so it is on the record rather than rediscovered.

packages/plugin-chatbot/src/useObjectChat.ts memoizes the transport:

const transport = useMemo(() => {
  if (!isApiMode) return undefined;
  return new DefaultChatTransport({ api: api!, ... });
}, [isApiMode, api, headers, body, model, systemPrompt, streamingEnabled, conversationId]);

body and headers are object props, and every caller passes a fresh literal on each render. AiChatPage's ChatPane is the clearest case:

body: {
  context: {
    activeApp: 'AI',
    agentName: activeAgent,
    autoPublishAiBuilds: getRuntimeConfig().features.autoPublishAiBuilds,
    ...(editPackageId ? { packageId: editPackageId } : {}),
  },
},

That object has a new identity on every render, so the memo never hits and a DefaultChatTransport is constructed on every render of every chat surface — during a streaming turn, that is once per token batch.

Why it is dormant rather than broken

@ai-sdk/react@4.0.59's useChat does not rebuild its Chat when the transport identity changes. It stores the latest transport in a ref and its own chatOptions.transport delegates through getTransport():

latestRef.current = { ..., transport: options.transport };
const chatOptions = { ..., transport: { sendMessages: (o) => getTransport().sendMessages(o), ... } };
const shouldRecreateChat = "chat" in options && options.chat !== chatRef.current
  || "id" in options && options.id != null && chatRef.current.id !== options.id;

useObjectChat passes neither chat nor id, so the Chat object — and with it the whole message list — survives. The wasted construction is the entire cost.

Why it is worth recording anyway

The safety here is entirely a property of the SDK's internals, not of this call site. The memo looks like it is protecting something, and the dep list reads as if a changing transport would be disruptive. An SDK upgrade that starts keying the Chat on the transport (or a future id being threaded through) would turn a silent inefficiency into a thread that resets on every render — and the memo would give a reviewer no warning, because it is already written as though it works.

Two candidate directions, neither obviously right without measuring:

  • memoize at the call sites (body / headers behind useMemo), which fixes it wherever it is done but is unenforceable and will re-rot;
  • drop body/headers from the dep list and read them through a ref inside prepareSendMessagesRequest, which is what the file already does for model (modelRef) and for the handoff id (parentConvRef) — the established idiom in this very hook, and the only one that cannot be undone by a new caller.

Found while working #2627 (the AI build thread's blank-out was a different mechanism entirely — a pane remount driven by useChatConversation clearing the conversation id; PR #4183). This was ruled out as its cause and is unrelated to it.

Activity

  1. claude commented on Aug 11, 2026

    @claude
    Contributor

    Findings triage (first grade; premise check @ origin/main e1ade8f): HOLD — finding stands.

    Anchors verified live: the transport memo at packages/plugin-chatbot/src/useObjectChat.ts:383 (new DefaultChatTransport at :385), and — the detail that makes the card's second direction concrete — the ref idiom it proposes is already the file's own house style two dozen lines above: modelRef at :357-358 and parentConvRef at :371-376. So the suggested fix is not a new pattern, it is the pattern this hook already uses for exactly this problem.

    Held rather than promoted because the card is honest about what it measured: the waste is real, the breakage is not — @ai-sdk/react's Chat survives transport identity changes, so nothing a user hits today. And the two candidate directions differ in more than mechanics (call-site memoization is unenforceable; the ref rewrite changes when body/headers are sampled), which is a judgement no one has taken. Both are cheap to revisit.

    ⚠️ Recorded restart condition, so this does not just age: an @ai-sdk/react major bump, or any change that starts passing chat/id through useObjectChat, converts this from waste into a per-render thread reset. A dependency bump touching @ai-sdk/react is the trigger to re-grade this immediately rather than at cadence.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  2. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    Findings cadence re-check @ origin/main 6314e87: HOLD — finding stands.

    Premise live: packages/plugin-chatbot/src/useObjectChat.ts:383-398 — the transport useMemo closes over body and lists it in its deps ([isApiMode, api, headers, body, …]), so any caller passing an inline body literal recreates DefaultChatTransport every render, exactly as filed. Held: perf-class, no correctness fault observed; the fix (stable serialization or ref-equality guard) is small but touches every caller's contract expectations, natural to bundle with the next chatbot-transport change.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  3. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    Findings cadence re-check @ origin/main 6d01319: HOLD — finding stands, premise frozen.

    The memo dep list is verbatim at useObjectChat.ts:417 ([isApiMode, api, headers, body, …]), @ai-sdk/react is still ^4.0.59 in plugin-chatbot/package.json:34, and the file is untouched since filing — so the dormancy contract (SDK stores transport in a ref, no chat/id keying) still holds and the cost remains construction-only. Held: the card's second direction (ref-read body/headers inside prepareSendMessagesRequest, matching the hook's own modelRef/parentConvRef idiom) is the durable fix, but nothing user-visible pushes on it today. Restart condition: promote before any @ai-sdk/react major/minor bump that touches useChat's chat-recreation keying, or the moment an id is threaded through useObjectChat — either flips this from waste to a thread-resetting bug.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  4. added theissue type on Aug 17, 2026
  5. os-zhuang commented on Aug 17, 2026

    @os-zhuang
    Contributor

    Findings state conversion (batch 3): finding → pm:on-hold, type Task.

    Held three times without leaving the label — converting per the 2026-08-13 state-semantics ruling.

    Hold rationale (unchanged): construction waste only — @ai-sdk/react@4.0.59 keeps the Chat alive across transport identity changes, so nothing user-visible breaks today. The durable fix (ref-read body/headers inside prepareSendMessagesRequest, matching the hook's own modelRef/parentConvRef idiom) is recorded on the card.

    Restart-when: the @ai-sdk/react version in packages/plugin-chatbot/package.json moves off ^4.0.59, or chat/id starts being passed through useObjectChat — either flips this from waste to a thread-resetting bug. Re-grade IMMEDIATELY on that trigger, not at cadence.

    Trigger files: packages/plugin-chatbot/package.json, packages/plugin-chatbot/src/useObjectChat.ts.

    Hold date: 2026-08-17. 本评论来自分诊座位 Routine,不构成认领。


    Generated by Claude Code

  6. os-zhuang commented on Aug 19, 2026

    @os-zhuang
    Contributor

    On-hold daily batch (triage seat, round of ~14:47Z fire): Restart-when fired — plugin-chatbot/package.json now carries "@ai-sdk/react": "^4.0.68" (off the pinned ^4.0.59), and this hold's own instruction is "re-grade IMMEDIATELY on that trigger, not at cadence". pm:on-hold → pm:queue. Claimer: re-verify the transport question against 4.0.68's actual API before assuming the hold-era analysis still describes it.


    Generated by Claude Code

  7. added
    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seat
    on Aug 21, 2026
  8. self-assigned this
    on Aug 21, 2026
  9. os-sales commented on Aug 21, 2026

    @os-sales
    Collaborator

    Claim. PM seat domain:ui (seat post #5560), round 6. Session session_012u2pRjcqAYtoEjgr3wwhnK. Branch claude/issue-4187-chat-transport-memo. pm:queue → pm:dispatched.

    Route ruled — the ref rewrite, not call-site memoization. The card leaves two directions open and calls the choice unmeasured. Deciding it here so the implementer does not re-open it: read body and headers through refs inside prepareSendMessagesRequest and drop them from the memo dep list. Reason: it is the idiom this hook already uses for exactly this problem (modelRef and parentConvRef sit two dozen lines above), and it is the only one of the two a future caller cannot silently undo. Call-site memoization is unenforceable by construction — it fixes today's callers and re-rots on the next one.

    Premise must be re-measured before implementing, not assumed. The hold's restart condition fired: packages/plugin-chatbot/package.json now carries @ai-sdk/react at ^4.0.68, off the ^4.0.59 the entire dormancy analysis was written against. The card's claim that this is waste-only rests on 4.0.59 storing the transport in a ref and keying Chat recreation solely on chat/id. Verify that against the installed 4.0.68 source in node_modules before writing any fix, and report what you find either way. If 4.0.68 keys Chat on transport identity, this stops being a dormant perf finding and becomes a live per-render thread reset — say so in the PR and treat the severity as changed.

    Behavioural note to settle in the PR. The two routes differ in more than mechanics: moving to refs changes when body/headers are sampled — from render time to send time. State explicitly which values a send observes after the change and pin it with a test, because that is the one real behavioural difference and it should not be discovered later.


    Generated by Claude Code

  10. os-sales commented on Aug 21, 2026

    @os-sales
    Collaborator
    {
      "issue": 4187,
      "status": "done",
      "branch": "claude/issue-4187-chat-transport-memo",
      "pr": "https://github.com/objectstack-ai/objectui/pull/5603",
      "premise_still_valid": true,
      "summary": "Premise re-measured against the INSTALLED @ai-sdk/react@4.0.68 before writing anything, and it survives unchanged: useChat still keeps the transport in latestRef and delegates through getTransport() (dist/index.js:301-322), and shouldRecreateChat is still keyed only on `chat`/`id` (dist/index.js:347), neither of which useObjectChat passes. So this is NOT a live per-render thread reset — severity is unchanged from the hold-era analysis: construction waste only. Implemented the ruled route: `body` and `headers` are read through bodyRef/headersRef inside prepareSendMessagesRequest and dropped from the transport useMemo dep list, matching the modelRef/parentConvRef idiom already in the file. Merge order is preserved in both directions (caller body still loses to the hook-owned conversationId/model/systemPrompt/stream keys; a per-send header still beats the caller's headers, which required re-merging rather than replacing the SDK's base headers, since returning headers from prepare REPLACES them). One consequence named in the PR: headers leaving the constructor also takes them off the reconnectToStream path, which has zero consumers in this repo (grep for resumeStream/reconnectToStream over packages/ and apps/ returns nothing outside node_modules).",
      "sampling_timing_answer": "After the change a send observes the body/headers of the MOST RECENT COMPLETED RENDER, read at send time: the refs are assigned during every render, and prepareSendMessagesRequest runs synchronously at the top of HttpChatTransport.sendMessages, itself reached through getTransport() at that instant. Before, the transport spread body/headers into itself at CONSTRUCTION time, so a send observed the values of the last render that REBUILT the transport — in practice also the last render, because every caller passes a fresh literal and the memo therefore never hit, which is why nothing changes for any caller that exists today. The old guarantee was incidental (it held only because the memo missed); the new one is unconditional and can never be staler. Divergence is observable only for a caller whose body identity is stable, which case 4 of the new test pins.",
      "diff_shape": "3 files, +272/-6 at 0cd066a7c. packages/plugin-chatbot/src/useObjectChat.ts (+47/-6: two refs added beside modelRef; `headers` and `...body` removed from the DefaultChatTransport constructor; prepareSendMessagesRequest now spreads bodyRef.current under reqBody and returns re-merged headers; dep list drops to [isApiMode, api, model, systemPrompt, streamingEnabled, conversationId]). New packages/plugin-chatbot/src/__tests__/useObjectChat.transportIdentity.test.tsx (199 lines, 5 cases). New .changeset/chat-transport-memo-4187.md (patch, @object-ui/plugin-chatbot).",
      "tests": "All at 0cd066a7c with `git status --porcelain` empty; exit codes captured before any pipe, verdict lines quoted from each gate. VITEST: `pnpm exec vitest run packages/plugin-chatbot/` EXIT=0 -> `Test Files  23 passed (23)` / `Tests  326 passed (326)`. New file alone with --reporter=verbose EXIT=0, all five lines naming `packages/plugin-chatbot/src/__tests__/useObjectChat.transportIdentity.test.tsx`, `Tests  5 passed (5)`. Consumer sweep (7 files outside the package that name plugin-chatbot/useObjectChat: app-shell useAiUsage/useAiSurface/surfaceAgent/useChatConversation/ChatDock/studioChatDockPersistence + components palette) EXIT=0 -> `Test Files  7 passed (7)` / `Tests  75 passed (75)`. TYPE-CHECK: `pnpm --filter @object-ui/plugin-chatbot type-check` EXIT=0 (first attempt EXIT=2 with 17 TS2307s — dependency closure unbuilt; `pnpm --workspace-concurrency=2 --filter '@object-ui/plugin-chatbot^...' build` EXIT=0 first, then clean). LINT: `pnpm --filter @object-ui/plugin-chatbot lint` EXIT=0 -> `86 problems (0 errors, 86 warnings)`; `eslint . --format json` in-package: 55 files, 0 errors, 86 warnings, new test file contributes 0 of each. The +2 warnings are react-hooks/refs 'Cannot access refs during render' at useObjectChat.ts:460 and :462 — the identical warning the sibling idiom `modelRef.current = model` already draws at :423 (four pre-existing in this file); left unsuppressed for consistency. GATES all EXIT=0: check-control-bytes ('OK (scanned 4655 tracked text file(s); skipped 85 binary)'), check-changeset-presence ('2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)'), check-changeset-no-major, check-changeset-fixed, check-type-check-coverage ('45/46 ... 41/41 packages compile their tests'), check-lint-coverage ('46/46 packages linted, 0 with outstanding errors'), check-phantom-dependencies, check-package-self-import, check-spec-symbol-derivation, check-i18n-call-site-keys, check-i18n-en-drift ('No en value changed in this range.'), check-skills-paths, check-node-esm-load --specifiers-only. ABLATION (reverse-verification, both legs restored by `trap ... EXIT INT TERM`): NO REBUILD WAS REQUIRED OR SKIPPED — the test imports the hook by relative path from the same package's src, so vitest transforms the mutated source directly; the dist-staleness trap does not apply here (no `exports`-resolved dist in the path). Each mutation proved on disk with anchored two-directional `grep -Fc` BEFORE reading any result. Leg A (dispatched form — deps restored to the memo list): fixed-form 1->0, ablated-form 0->1, `git diff --stat` = `1 file changed, 1 insertion(+), 1 deletion(-)`; vitest EXIT=1, case 1 red with `AssertionError: expected 4 to be 1` (four renders, four constructions), other 4 green. Leg B (whole hook reverted to origin/main): marker `bodyRef.current = body;` 1->0, pre-fix marker `headers: { ...headers },` 0->1; vitest EXIT=1, cases 1 and 4 red (`expected 4 to be 1`, `expected 1 to be 2` — the construction-time frozen body), cases 2/3/5 green, exactly the predicted split. `git diff --stat` was EMPTY for leg B because `git checkout ref -- path` writes the index too, so that diff was staged — the two-directional grep is the load-bearing proof there, stated rather than papered over. After both legs: `git status --porcelain` empty AND `git diff HEAD --name-only` empty, i.e. byte-identical to HEAD. DECLARED NARROWING: `pnpm lint` (= `turbo run lint`, the same `eslint .` fanned across 46 packages) was run for the one package whose files changed; the flat config extends tseslint.configs.recommended with no `project`/`projectService`, so type-aware linting is OFF and this diff cannot move the verdict on a file it does not contain. Repo-wide sharded `pnpm test` left to CI; the changed package ran in full plus every test file outside it naming plugin-chatbot/useObjectChat. check-eager-closure-budget EXIT=2 for a reason unrelated to this change — it reads apps/console/dist/eager-closure.json from a console `vite build` and calls itself 'a broken gauge, not a passing budget' when absent; it belongs to the performance-budget job. doc-snippet/doc-component gates not run: no exported signature, option or type changed.",
      "case_3_is_not_a_regression_guard": "Case 3 ('a send carries the body/headers of the MOST RECENT render') is green both before and after — it is a contract pin, not a guard. Stated here and in the PR rather than counted as ablation evidence.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #5605 (finding, unassigned): `maxToolRoundtrips` is authorable in packages/types (complex.ts:586 + complex.zod.ts:264), threaded by renderer.tsx at :88/:306/:439, destructured with a default at useObjectChat.ts:363 — and never read; documented in content/docs/plugins/plugin-chatbot.mdx:155 with a stated default of 5. ADR-0049 enforce-or-remove shape. Surfaced by the lint run on this card; not touched in this PR."
      ]
    }

    PR #5603 is open as a draft against main, first line Fixes #4187. Not flipped ready, no auto-merge, not merged — the PM lands it after review. CI convergence deliberately not waited on; gate status at report time is whatever the run at 0cd066a7c reports.


    Generated by Claude Code

  11. os-sales commented on Aug 21, 2026

    @os-sales
    Collaborator

    Addendum after PM review of #5603 — supersedes the sha and gate rows of the report above. Everything else in that report stands unchanged.

    {
      "issue": 4187,
      "status": "done",
      "branch": "claude/issue-4187-chat-transport-memo",
      "pr": "https://github.com/objectstack-ai/objectui/pull/5603",
      "head_sha": "72bd9df02",
      "premise_still_valid": true,
      "summary": "PM review asked for one addition: put the reconnectToStream consequence in the FILE, not only in the PR body. Added a six-line comment on the exact line the `headers` option used to occupy in the DefaultChatTransport constructor, naming the invariant — headers are applied per-send in prepareSendMessagesRequest; reconnectToStream does NOT run that hook and reads the constructor's headers through prepareReconnectToStreamRequest; the first consumer to wire up stream resumption must merge headersRef.current there too; nothing in this repo resumes a stream today. Comment only, one file, +6/-0, no behaviour and no executable surface. New commit 72bd9df02 on top of 0cd066a7c — no force-push, no amend. PR body updated so its gate heading names the new head instead of leaving a stale sha, and so the consequence paragraph says the invariant is recorded in the file.",
      "tests": "Re-ran the three gates that can see a comment change, on the new head, tree clean (`git status --porcelain` empty at 72bd9df02); exit codes captured before any pipe, verdict lines quoted. `pnpm --filter @object-ui/plugin-chatbot type-check` EXIT=0 -> `tsc --noEmit && tsc -p tsconfig.test.json` clean (dependency closure rebuilt first in the fresh worktree: `pnpm --workspace-concurrency=2 --filter '@object-ui/plugin-chatbot^...' build` EXIT=0 — without it type-check returns EXIT=2 with TS2307s that have nothing to do with the diff). `pnpm --filter @object-ui/plugin-chatbot lint` EXIT=0 -> `86 problems (0 errors, 86 warnings)`, unchanged from the previous head: a comment adds no warning, and the two react-hooks/refs warnings at :460/:462 stay unsuppressed as ruled. `pnpm exec vitest run packages/plugin-chatbot/` from the repo root EXIT=0 -> `Test Files  23 passed (23)` / `Tests  326 passed (326)`. Also re-ran `node scripts/check-control-bytes.mjs` EXIT=0 -> `OK (scanned 4655 tracked text file(s); skipped 85 binary)`, since any edit is in its scope. NO ABLATION for this commit, and none is owed: a comment has nothing executable to pin, so a mutation of it could not turn any test red — reporting an ablation here would be a fabricated reading, not evidence. The rest of the gate set was measured at 0cd066a7c and is unaffected: the only difference between the two heads is six lines of comment inside one already-covered file.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #5605 (finding, unassigned): `maxToolRoundtrips` is authorable in packages/types (complex.ts:586 + complex.zod.ts:264), threaded by renderer.tsx at :88/:306/:439, destructured with a default at useObjectChat.ts:363 — and never read; documented in content/docs/plugins/plugin-chatbot.mdx:155 with a stated default of 5. ADR-0049 enforce-or-remove shape. Unchanged by this addendum."
      ]
    }

    PR #5603 verified after the update: still draft: true, auto_merge: null, base main, head 72bd9df02, first body line Fixes #4187. Not flipped ready, not merged — yours to land once CI is green on the new head.


    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:dispatched

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions