PR #18856 review notes: Buddy-E Redux→BuddyAgent migration

pr-18856-review.md

PR #18856 Review Notes

Follow-up items

1. saveFn as constructor param

BuddyAgent takes a saveFn callback and wires debounce + flush-chaining inside its constructor:

// BuddyAgent.ts:91,103-121
constructor(private readonly config: BuddyAgentConfig) {
  this.debouncedSave = debounce(() => {
    config.saveFn(this.serialize())
  }, 500)

  this.on('save', ({ flush }) => {
    if (flush) {
      this.debouncedSave.cancel()
      const snapshot = this.serialize()
      saveChain = saveChain.catch(() => undefined).then(() => config.saveFn(snapshot))
    } else {
      this.debouncedSave()
    }
  })
}

Gen3 keeps the agent unaware of persistence. The agent just emits save, and the hook layer owns the debounce + save call:

// gen3/hooks/useInitGen3Agent.ts:103-128
const debouncedSave = useDebounced(
  useCallback((state: Gen3AgentState) => { save(state) }, [save]),
  2000,
)

useEffect(() => {
  const unsubscribe = agent.on('save', ({ flush }) => {
    debouncedSave(agent.store.getState())
    if (flush) debouncedSave.flush()
  })
  return () => {
    unsubscribe()
    debouncedSave(agent.store.getState())
    debouncedSave.flush()
  }
}, [agent, debouncedSave])

Follow-up: move save wiring out of the constructor into the provider/hook layer to match Gen3/Design.

---

2. toolContextProvider as constructor param

BuddyAgentConfig takes a toolContextProvider: () => BuddyEditorToolContext and the agent delegates to it in buildToolContext():

// BuddyAgent.ts:392-395
private buildToolContext(): BuddyEditorToolContext {
  return this.config.toolContextProvider()
}

Gen3 builds tool context internally on the agent, no injected provider:

// gen3/agent/Gen3Agent.ts:138-146
this.chat = new GammaChatManager({
  // ...
  buildToolContext: () => this.buildToolContext(),
})

// Gen3Agent builds it from its own fields
private buildToolContext(): Gen3ToolContext {
  return {
    agent: this,
    agentStore: this.store,
    messages: this.store.getState().messages,
    chatId: this.chatId,
    docGenerationId: this.config.docGenerationId,
    editor: this.editor,
  }
}

Follow-up: have BuddyAgent.buildToolContext() construct the context from its own state instead of delegating to an injected closure. Everything it needs is either on the agent (editor, chatId, docId) or in Redux (theme, imageOptions via getStore()). Build lazily at tool execution time like Gen3 does. This also removes useBuddyEditorToolContext from the provider level (it shouldn't be called that high) and eliminates the ref/callback dance (toolContextRef, getToolContext) in BuddyAgentProvider.

---

3. adaptBuddyTools + ToolSet

adaptBuddyTools wraps each Buddy tool to bridge the old 5-arg ToolSet signature to GammaChatManager's Tool interface:

// BuddyAgent.ts:54-79
function adaptBuddyTools(getInteractionId: () => string) {
  for (const [name, tool] of Object.entries(BUDDY_EDITOR_TOOLS)) {
    adapted[name] = {
      available: () => tool.available(),
      execute: async (input, context, toolCall, managerContext) => {
        return tool.execute(input, context, toolCall, managerContext.messageId, getInteractionId())
      },
    }
  }
}

The old signature (ToolSet.ts):

execute: (args, context, toolCall, messageId, interactionId) => Promise<...>

The new signature (Tool / AgentToolRegistry):

execute: (input, context, toolCall, managerContext: { messageId, abortController, span }) => Promise<...>

Gen3 and Design define tools directly against the new interface:

// gen3/ai/tools.ts:521
} satisfies AgentToolRegistry<'Gen3', Gen3ToolContext>

Follow-up: migrate BUDDY_EDITOR_TOOLS to satisfies AgentToolRegistry<'Buddy', BuddyEditorToolContext>, update each tool's execute to the 4-arg form, drop adaptBuddyTools and ToolSet.ts.

---

5. Eliminate module-level singleton (agent/singleton.ts)

A module-level let activeBuddyAgent with get/set accessors. Gen3 and Design don't have this pattern. All three consumers can be eliminated:

BuddySelectionPlugin -- reads editorSelection from the agent store, but the plugin already has the editor. The selection was derived from the same editor by useSyncBuddyEditorSelection, so data round-trips: editor → hook → agent store → singleton → plugin. The plugin could compute the selection directly from editor (which it already has) + Redux isBuddyChatOpen check.

// BuddySelectionPlugin.ts:36
const buddyEditorSelection =
  getActiveBuddyAgent()?.store.getState().editorSelection ?? null

useBuddyEditorToolContext -- reads agent/store/editor/chatId. Goes away if item #2 is addressed (agent builds tool context internally).

// useBuddyEditorToolContext.ts:105
const agent = getActiveBuddyAgent()
const buddyStore = agent?.store ?? null
const editor = agent?.editor ?? null

BuddyChatPanel.canClose -- prevents panel close while streaming. Now that the agent is headless at DocEditor, closing the panel is just a UI toggle -- the agent survives. There's no reason to block close or stop the stream. Remove the stop-on-unmount in ChatPane and let the request finish in the background:

// ChatPane.tsx:235-242 — remove this
useEffect(
  () => () => {
    if (agent.isLoading) {
      agent.stop()
    }
  },
  [agent],
)

Then canClose returns true unconditionally (or just delete the override), and this last singleton consumer goes away. Gen3 already does this -- their canClose is hardcoded return true:

// gen3/components/chat/Gen3ChatPanel.tsx:31-36
Gen3ChatPanel.canClose = (store) => {
  return true
}

With all three consumers eliminated, delete agent/singleton.ts.

Recommended approach: put the active BuddyAgent (or its Zustand store ref) in a Redux slice instead of the module-level singleton. BuddyAgentProvider dispatches on mount, clears on unmount. This gives one source of truth instead of two parallel distribution channels (context + singleton) for the same reference:

• React components: useAppSelector (or existing context hooks backed by Redux)
• canClose: reads from the store arg it already receives
• BuddySelectionPlugin: reads via getStore() like it does for all other Redux state
• useBuddyEditorToolContext: reads via useAppSelector

No new access patterns, no singleton, one lifecycle.

Replace BuddyAgentProvider + context with a plain hook (useBuddyAgentLifecycle(editor)) called directly in DocEditor. The hook does the orchestration (persistence, init, tool context), dispatches the agent to Redux on mount, clears on unmount. No provider component, no context. This matches Gen3's approach -- useInitGen3Agent is a hook called from the page component, not wrapped in a provider. Consumer hooks (useBuddyAgent, useBuddyStore, etc.) read the agent from Redux instead of context.

---

7. processFinishedMessageIntoResult should live on the agent, not in useBuddyLifecycleSync

Result processing on finish/stop is split across two layers with microtask coordination to get the ordering right:

Agent (BuddyAgent.ts:163-175) sets totalTokensUsed, clears beforeCheckpoint, queues a microtask to emit save:

this.chat.on('finish', ({ totalUsage }) => {
  this.store.setState({ totalTokensUsed: totalUsage.totalTokens, isLoading: false })
  this.beforeCheckpoint = null
  queueMicrotask(() => this.emit('save', { flush: true }))
})

React hook (useBuddyLifecycleSync.ts:72-86) subscribes to the same event and calls processFinishedMessageIntoResult, which writes results/checkpoints/suggestions to the store:

const unsubFinish = agent.chat.on('finish', ({ message }) => {
  const checkpoint = beforeCheckpointRef.current
  if (checkpoint) {
    processFinishedMessageIntoResult({ agent, message, beforeCheckpoint: checkpoint, ... })
    beforeCheckpointRef.current = null
  }
  markFinish()
})

The microtask exists so the hook runs first (sync during dispatch), then the deferred save captures those mutations. This coordination problem only exists because the work is split.

Gen3 handles everything in one place inside the agent:

// Gen3Agent.ts:152-156
this.chat.on('finish', ({ message, totalUsage }) => {
  this.store.setState({ totalUsage })
  processGen3Message(message, this.store)
  this.emit('save', {})
})

Follow-up: move processFinishedMessageIntoResult into BuddyAgent's chat.on('finish') handler. The microtask goes away, useBuddyLifecycleSync shrinks to just the error toast, and ordering is guaranteed by call order instead of microtask scheduling.

---

8. serialize() on the agent class

BuddyAgent owns its own serialization and calls it inside the save chain:

// BuddyAgent.ts:319-333
serialize(): SerializedChatStateV5 {
  const s = this.store.getState()
  return {
    chatId: s.chatId || '',
    messagesVersion: 'v5',
    messages: s.messages,
    checkpoints: s.checkpoints,
    // ...
  }
}

Gen3 keeps serialization in the persistence hook, separate from the agent:

// gen3/hooks/useGen3Persistence.ts:96-120
function mapAgentStateToJson(state: Gen3AgentState): Record<string, unknown> {
  return {
    messagesVersion: 'v1',
    messages: state.messages.map((msg) => ({ id: msg.id, role: msg.role, parts: msg.parts })),
    // ...
  }
}

Follows from #1 -- if persistence wiring moves out of the agent, serialization moves with it into useBuddyPersistence.