mirror of
https://github.com/stablyai/orca.git
synced 2026-10-01 00:02:10 +00:00
fix(mobile): carry the turn key instead of caching a handler in a ref
Builds on the scope-isolation fix: that kept (and extended) a ref that is written during render — once to memoize a per-turn handler, once to prune dead turns, once to reset on a scope change. React Doctor's "Ref mutated during render" is what CI's `check:react-doctor:changed` was failing on (x2), and on mobile it is a real hazard rather than a style note: react-freeze discards renders, and a discarded render would leave the cache mutated. Pass the settled turn's key down the row instead and let it call one stable handler with it. That preserves both properties the cache was bought for — per scope isolation, and identity stability so a streaming transcript does not defeat the row's memo — with no ref writes and no pruning to get wrong. The scope-keyed expanded set and the 128-turn cap are untouched; their tests move to the new contract and one now pins handler identity across a re-render. Note for future changes here: `check:code-quality:changed` does NOT cover this. CI additionally runs the standalone react-doctor CLI, which has rules the oxlint plugin config does not enable.
This commit is contained in:
@@ -105,6 +105,7 @@ function MobileNativeChatMessageImpl({
|
||||
onOpenFile,
|
||||
turnStatus,
|
||||
turnExpanded,
|
||||
turnKey,
|
||||
onToggleTurn,
|
||||
activeTurnIsWorking,
|
||||
structuredActivityUi = false
|
||||
@@ -122,7 +123,10 @@ function MobileNativeChatMessageImpl({
|
||||
turnStatus?: NativeChatTurnStatus | null
|
||||
/** Whether the turn caret has disclosed this turn's activity. */
|
||||
turnExpanded?: boolean
|
||||
onToggleTurn?: () => void
|
||||
/** Set only when this row's turn has settled and can disclose its activity. */
|
||||
turnKey?: string
|
||||
/** Stable across renders; the row supplies its own key when tapped. */
|
||||
onToggleTurn?: (turnKey: string) => void
|
||||
/** Session-level working state for this message's turn; gates the live tool row. */
|
||||
activeTurnIsWorking?: boolean
|
||||
/** Structured lane only: live tool progress plus the turn-status disclosure. */
|
||||
@@ -230,7 +234,7 @@ function MobileNativeChatMessageImpl({
|
||||
thinking={turnStatus.thinking}
|
||||
workedSeconds={turnStatus.workedSeconds}
|
||||
expanded={turnExpanded ?? false}
|
||||
onToggleExpanded={turnStatus.workedSeconds != null ? onToggleTurn : undefined}
|
||||
onToggleExpanded={turnKey && onToggleTurn ? () => onToggleTurn(turnKey) : undefined}
|
||||
/>
|
||||
) : null}
|
||||
</>
|
||||
|
||||
@@ -275,6 +275,7 @@ export function MobileNativeChatView({
|
||||
onScrollToMessage={onScrollToMessage}
|
||||
onOpenFile={onOpenFile}
|
||||
structuredActivityUi={structuredActivityUi}
|
||||
onToggleTurn={turns.onToggleTurn}
|
||||
{...turns.resolveRow(index, item)}
|
||||
/>
|
||||
),
|
||||
|
||||
@@ -99,8 +99,18 @@ describe('useMobileNativeChatTurnDisclosure', () => {
|
||||
.findByType('result')
|
||||
.props.disclosure.resolveRow(0, refreshed[0])
|
||||
|
||||
expect(first.onToggleTurn).toBeTypeOf('function')
|
||||
expect(second.onToggleTurn).toBe(first.onToggleTurn)
|
||||
// The row carries the key; the handler itself lives on the hook and stays
|
||||
// stable for the scope, so a re-render never disturbs a row's memo.
|
||||
expect(first.turnKey).toBe('u1')
|
||||
expect(second.turnKey).toBe('u1')
|
||||
const firstHandler = renderer!.root.findByType('result').props.disclosure.onToggleTurn
|
||||
expect(firstHandler).toBeTypeOf('function')
|
||||
act(() => {
|
||||
renderer?.update(
|
||||
createElement(Harness, { messages: [...refreshed], enabled: true, isWorking: false })
|
||||
)
|
||||
})
|
||||
expect(renderer!.root.findByType('result').props.disclosure.onToggleTurn).toBe(firstHandler)
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
@@ -124,10 +134,9 @@ describe('useMobileNativeChatTurnDisclosure', () => {
|
||||
act(() => {
|
||||
renderer?.update(createElement(Harness, { messages, enabled: true, isWorking: false }))
|
||||
})
|
||||
const row = renderer!.root
|
||||
.findByType('result')
|
||||
.props.disclosure.resolveRow(index, messages[index])
|
||||
act(() => row.onToggleTurn())
|
||||
const disclosureNow = renderer!.root.findByType('result').props.disclosure
|
||||
const row = disclosureNow.resolveRow(index, messages[index])
|
||||
act(() => disclosureNow.onToggleTurn(row.turnKey))
|
||||
}
|
||||
|
||||
const disclosure = renderer!.root.findByType('result').props.disclosure
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useCallback, useMemo, useRef, useState } from 'react'
|
||||
import { useCallback, useMemo, useState } from 'react'
|
||||
import type { NativeChatMessage } from '../../../src/shared/native-chat-types'
|
||||
import {
|
||||
MOBILE_UNANCHORED_TURN_KEY,
|
||||
@@ -13,7 +13,8 @@ const MAX_EXPANDED_TURNS = 128
|
||||
export type MobileNativeChatTurnRow = {
|
||||
turnStatus: NativeChatTurnStatus | null
|
||||
turnExpanded: boolean
|
||||
onToggleTurn?: () => void
|
||||
/** Set only on a settled turn — the one row that has activity to disclose. */
|
||||
turnKey?: string
|
||||
activeTurnIsWorking: boolean
|
||||
}
|
||||
|
||||
@@ -35,6 +36,7 @@ export function useMobileNativeChatTurnDisclosure({
|
||||
active: NativeChatTurnStatus | null
|
||||
/** True when the live turn has no user message to hang its status row under. */
|
||||
activeTurnIsUnanchored: boolean
|
||||
onToggleTurn: (turnKey: string) => void
|
||||
resolveRow: (index: number, message: NativeChatMessage) => MobileNativeChatTurnRow
|
||||
} {
|
||||
const turnStatuses = useMobileNativeChatTurnStatus({
|
||||
@@ -67,55 +69,20 @@ export function useMobileNativeChatTurnDisclosure({
|
||||
},
|
||||
[scopeKey]
|
||||
)
|
||||
// Why: a fresh closure per row per render defeats the message row's memo, and a
|
||||
// streaming turn re-renders the list ~20x/s. One stable handler per turn instead.
|
||||
const toggleHandlers = useRef<{
|
||||
scopeKey: string
|
||||
byTurn: Map<string, () => void>
|
||||
}>({ scopeKey, byTurn: new Map() })
|
||||
const toggleHandlerFor = useCallback(
|
||||
(turnKey: string): (() => void) => {
|
||||
if (toggleHandlers.current.scopeKey !== scopeKey) {
|
||||
toggleHandlers.current = { scopeKey, byTurn: new Map() }
|
||||
}
|
||||
const existing = toggleHandlers.current.byTurn.get(turnKey)
|
||||
if (existing) {
|
||||
return existing
|
||||
}
|
||||
const handler = (): void => toggleExpandedTurn(turnKey)
|
||||
toggleHandlers.current.byTurn.set(turnKey, handler)
|
||||
return handler
|
||||
},
|
||||
[scopeKey, toggleExpandedTurn]
|
||||
)
|
||||
|
||||
// Resolve each row's turn boundary once — a findLast per row is quadratic on a
|
||||
// long transcript.
|
||||
const turnKeys = useMemo(() => {
|
||||
if (toggleHandlers.current.scopeKey !== scopeKey) {
|
||||
toggleHandlers.current = { scopeKey, byTurn: new Map() }
|
||||
}
|
||||
if (!enabled) {
|
||||
toggleHandlers.current.byTurn.clear()
|
||||
return EMPTY_TURN_KEYS
|
||||
}
|
||||
let turnKey: string | undefined
|
||||
const keys = messages.map((message) => {
|
||||
return messages.map((message) => {
|
||||
if (message.role === 'user') {
|
||||
turnKey = message.id
|
||||
}
|
||||
return turnKey
|
||||
})
|
||||
// Drop handlers for turns that left the transcript so a long session does not
|
||||
// retain a closure per turn forever.
|
||||
const live = new Set(keys.filter((key): key is string => key !== undefined))
|
||||
for (const key of toggleHandlers.current.byTurn.keys()) {
|
||||
if (!live.has(key)) {
|
||||
toggleHandlers.current.byTurn.delete(key)
|
||||
}
|
||||
}
|
||||
return keys
|
||||
}, [enabled, messages, scopeKey])
|
||||
}, [enabled, messages])
|
||||
|
||||
const { active, activeTurnKey, completedByTurn } = turnStatuses
|
||||
const resolveRow = useCallback(
|
||||
@@ -132,10 +99,11 @@ export function useMobileNativeChatTurnDisclosure({
|
||||
return {
|
||||
turnStatus,
|
||||
turnExpanded: turnKey ? expandedTurnIds.has(turnKey) : false,
|
||||
// Only a settled turn has anything to disclose; leaving the handler off
|
||||
// every other row keeps their props identity-stable.
|
||||
onToggleTurn:
|
||||
turnKey && turnStatus?.workedSeconds != null ? toggleHandlerFor(turnKey) : undefined,
|
||||
// Why: the key travels and the row calls one stable handler with it. A
|
||||
// closure per row would be a new identity every render of a streaming
|
||||
// transcript, defeating the row's memo; caching one per turn would mean
|
||||
// writing a ref during render, which react-freeze can discard.
|
||||
turnKey: turnKey && turnStatus?.workedSeconds != null ? turnKey : undefined,
|
||||
// With no user boundary at all, the session's working state stays authoritative.
|
||||
activeTurnIsWorking:
|
||||
enabled &&
|
||||
@@ -144,20 +112,13 @@ export function useMobileNativeChatTurnDisclosure({
|
||||
(turnKey === undefined && activeTurnKey === MOBILE_UNANCHORED_TURN_KEY))
|
||||
}
|
||||
},
|
||||
[
|
||||
turnKeys,
|
||||
enabled,
|
||||
activeTurnKey,
|
||||
active,
|
||||
completedByTurn,
|
||||
expandedTurnIds,
|
||||
toggleHandlerFor,
|
||||
isWorking
|
||||
]
|
||||
[turnKeys, enabled, activeTurnKey, active, completedByTurn, expandedTurnIds, isWorking]
|
||||
)
|
||||
|
||||
return {
|
||||
active,
|
||||
/** Stable for a given chat scope, so it never disturbs a row's memo. */
|
||||
onToggleTurn: toggleExpandedTurn,
|
||||
activeTurnIsUnanchored:
|
||||
enabled && active != null && activeTurnKey === MOBILE_UNANCHORED_TURN_KEY,
|
||||
resolveRow
|
||||
|
||||
Reference in New Issue
Block a user