Skip to content

Commit 5ae65b7

Browse files
fix: replace queue/loop bg analysis with promise-chain pattern
The queue/loop pattern had too many failure modes (running flags, queue clearing, effect ordering). Replaced with a simple promise chain: each new drill node gets its analysis chained onto a single ref promise. No queue, no running flag, no separate loop callback. - bgChainRef: single promise chain, analysis tasks appended via .then() - bgAnalyzedFensRef: Set tracks which FENs have been scheduled - Drill change: cancel flag + chain reset - stopBackgroundAnalysis: sets cancel + stops stockfish + awaits chain Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 85444e6 commit 5ae65b7

1 file changed

Lines changed: 27 additions & 105 deletions

File tree

src/hooks/useOpeningDrillController/useOpeningDrillController.ts

Lines changed: 27 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -906,16 +906,13 @@ export const useOpeningDrillController = (
906906
// Background analysis: run Maia + Stockfish on drill positions as they are
907907
// played so post-drill analysis has less (or no) work to do.
908908
//
909-
// There is a single Stockfish WASM instance, so we MUST NOT run background
910-
// and post-drill Stockfish concurrently. When the drill ends we:
911-
// 1. Set bgCancelledRef = true (loop exits after current position)
912-
// 2. Call stockfish.stopEvaluation() to abort any in-progress eval
913-
// 3. Await bgLoopPromiseRef to ensure the loop has fully exited
914-
// 4. Only then start ensureDrillAnalysis
915-
const bgQueueRef = useRef<GameNode[]>([])
916-
const bgRunningRef = useRef(false)
909+
// Uses a simple promise-chain pattern: each node's analysis is chained onto
910+
// a single promise ref. No queue management, no running flags — just append
911+
// work to the chain. Nodes are tracked by FEN in a Set to avoid duplicates.
912+
const bgChainRef = useRef<Promise<void>>(Promise.resolve())
913+
const bgAnalyzedFensRef = useRef<Set<string>>(new Set())
917914
const bgCancelledRef = useRef(false)
918-
const bgLoopPromiseRef = useRef<Promise<void> | null>(null)
915+
const bgDrillIdRef = useRef<string | null>(null)
919916
const ensureMaiaRef = useRef(ensureMaiaForNode)
920917
const ensureStockfishRef = useRef(ensureStockfishForNode)
921918
useEffect(() => {
@@ -925,69 +922,20 @@ export const useOpeningDrillController = (
925922
ensureStockfishRef.current = ensureStockfishForNode
926923
}, [ensureStockfishForNode])
927924

928-
const runBgLoop = useCallback(async () => {
929-
if (bgRunningRef.current) {
930-
console.log('[bg] loop already running, skipping')
931-
return
932-
}
933-
bgRunningRef.current = true
934-
console.log('[bg] loop STARTED, queue size:', bgQueueRef.current.length)
935-
try {
936-
while (bgQueueRef.current.length > 0) {
937-
if (bgCancelledRef.current) {
938-
console.log('[bg] cancelled flag set, breaking')
939-
break
940-
}
941-
const node = bgQueueRef.current[0]
942-
const fen = node.fen.split(' ').slice(0, 3).join(' ')
943-
944-
console.log('[bg] maia start:', fen)
945-
await ensureMaiaRef.current(node)
946-
const hasMaia =
947-
node.analysis.maia &&
948-
MAIA_MODELS.every((m) => node.analysis.maia?.[m])
949-
console.log('[bg] maia done:', fen, 'hasMaia:', hasMaia)
950-
if (bgCancelledRef.current) {
951-
console.log('[bg] cancelled after maia, breaking')
952-
break
953-
}
954-
955-
console.log('[bg] stockfish start:', fen)
956-
await ensureStockfishRef.current(node)
957-
const sfDepth = node.analysis.stockfish?.depth ?? 0
958-
console.log('[bg] stockfish done:', fen, 'depth:', sfDepth)
959-
bgQueueRef.current.shift()
960-
}
961-
} catch (error) {
962-
console.error('[bg] error:', error)
963-
} finally {
964-
bgRunningRef.current = false
965-
console.log('[bg] loop ENDED, remaining:', bgQueueRef.current.length)
966-
}
967-
// eslint-disable-next-line react-hooks/exhaustive-deps
968-
}, [])
969-
970-
// Enqueue new drill positions for background analysis.
971-
// Also handles drill-change detection (must be in the same effect to avoid
972-
// the cancellation effect clearing the queue after the enqueue effect fills it).
973-
const bgDrillIdRef = useRef<string | null>(null)
974925
useEffect(() => {
975926
if (!currentDrillGame || isAnalyzingDrill) {
976-
// No active drill or post-drill analysis is running — cancel background
977-
if (bgDrillIdRef.current !== null) {
978-
bgCancelledRef.current = true
979-
bgQueueRef.current = []
980-
bgDrillIdRef.current = null
981-
}
982927
return
983928
}
984929

985-
// If drill changed, reset background state for the new drill
930+
// If drill changed, reset for the new drill
986931
if (currentDrillGame.id !== bgDrillIdRef.current) {
987932
bgCancelledRef.current = true
988-
bgQueueRef.current = []
933+
// Let any in-flight work finish with the cancelled flag, then reset
934+
bgChainRef.current = bgChainRef.current.then(() => {
935+
bgCancelledRef.current = false
936+
})
937+
bgAnalyzedFensRef.current = new Set()
989938
bgDrillIdRef.current = currentDrillGame.id
990-
bgCancelledRef.current = false
991939
}
992940

993941
const mainLine = gameTree.getMainLine()
@@ -997,53 +945,27 @@ export const useOpeningDrillController = (
997945
: 0
998946
const drillNodes = mainLine.slice(startIndex)
999947

1000-
const queuedFens = new Set(bgQueueRef.current.map((n) => n.fen))
1001-
1002-
const newNodes = drillNodes.filter((node) => {
1003-
if (queuedFens.has(node.fen)) return false
1004-
const hasMaia =
1005-
node.analysis.maia &&
1006-
MAIA_MODELS.every((model) => node.analysis.maia?.[model])
1007-
const hasStockfish =
1008-
node.analysis.stockfish &&
1009-
node.analysis.stockfish.depth >= DRILL_STOCKFISH_TARGET_DEPTH
1010-
return !hasMaia || !hasStockfish
1011-
})
948+
for (const node of drillNodes) {
949+
if (bgAnalyzedFensRef.current.has(node.fen)) continue
950+
bgAnalyzedFensRef.current.add(node.fen)
1012951

1013-
console.log(
1014-
'[bg] enqueue effect: drillNodes:',
1015-
drillNodes.length,
1016-
'newNodes:',
1017-
newNodes.length,
1018-
'queueSize:',
1019-
bgQueueRef.current.length,
1020-
'running:',
1021-
bgRunningRef.current,
1022-
'cancelled:',
1023-
bgCancelledRef.current,
1024-
)
1025-
if (newNodes.length > 0) {
1026-
bgQueueRef.current.push(...newNodes)
1027-
const promise = runBgLoop()
1028-
if (promise) bgLoopPromiseRef.current = promise
952+
// Chain this node's analysis onto the promise chain
953+
bgChainRef.current = bgChainRef.current.then(async () => {
954+
if (bgCancelledRef.current) return
955+
console.log('[bg] analyzing:', node.fen.split(' ')[0].slice(-20))
956+
await ensureMaiaRef.current(node)
957+
if (bgCancelledRef.current) return
958+
await ensureStockfishRef.current(node)
959+
console.log('[bg] done, sf depth:', node.analysis.stockfish?.depth ?? 0)
960+
})
1029961
}
1030-
}, [
1031-
currentDrillGame,
1032-
gameTree,
1033-
isAnalyzingDrill,
1034-
runBgLoop,
1035-
treeController.currentNode,
1036-
])
962+
}, [currentDrillGame, gameTree, isAnalyzingDrill, treeController.currentNode])
1037963

1038-
// Helper: stop background loop and wait for it to fully exit
964+
// Helper: stop background analysis and wait for it to fully settle
1039965
const stopBackgroundAnalysis = useCallback(async () => {
1040966
bgCancelledRef.current = true
1041-
bgQueueRef.current = []
1042967
stockfish.stopEvaluation()
1043-
if (bgLoopPromiseRef.current) {
1044-
await bgLoopPromiseRef.current
1045-
bgLoopPromiseRef.current = null
1046-
}
968+
await bgChainRef.current
1047969
}, [stockfish])
1048970

1049971
const ensureDrillAnalysis = useCallback(

0 commit comments

Comments
 (0)