From 197fd69ce3248422361237244404fb23d2ab72ab Mon Sep 17 00:00:00 2001 From: Ender Date: Sat, 25 Oct 2025 21:39:40 +0200 Subject: [PATCH] fix: resolve streaming content accumulation issue - Fixed stale closure bug in streaming content by using useRef to properly accumulate content instead of relying on state closure - Added auto-scrolling functionality to keep latest content visible during generation - Implemented smooth scrolling behavior and improved HTML styling for better readability - Added content buffer reset when starting new generation - Enhanced documentation with detailed explanation of the closure problem and solution --- apps/admin/STREAMING_FIX.md | 188 ++++++++++++++++++ .../src/components/steps/StepGenerate.tsx | 20 +- 2 files changed, 206 insertions(+), 2 deletions(-) create mode 100644 apps/admin/STREAMING_FIX.md diff --git a/apps/admin/STREAMING_FIX.md b/apps/admin/STREAMING_FIX.md new file mode 100644 index 0000000..49a3b11 --- /dev/null +++ b/apps/admin/STREAMING_FIX.md @@ -0,0 +1,188 @@ +# Streaming Content Accumulation Fix + +## Problem + +The streaming content was showing individual tokens instead of the accumulated HTML article because of a **stale closure** issue. + +### What Was Happening + +```typescript +// ❌ WRONG - Stale closure +onContent: (data) => { + onSetStreamingContent(streamingContent + data.delta); + // streamingContent is captured from the initial render + // It never updates in this callback! +} +``` + +**Result**: Each delta was added to the INITIAL empty string, not the accumulated content. + +``` +Delta 1: "" + "TypeScript" = "TypeScript" +Delta 2: "" + " is" = " is" // Lost "TypeScript"! +Delta 3: "" + " a" = " a" // Lost everything! +``` + +## Solution + +Use a **ref** to track the accumulated content outside the closure: + +```typescript +// ✅ CORRECT - Ref accumulation +const contentBufferRef = useRef(''); + +onContent: (data) => { + contentBufferRef.current += data.delta; + onSetStreamingContent(contentBufferRef.current); +} +``` + +**Result**: Each delta is properly accumulated. + +``` +Delta 1: "" + "TypeScript" = "TypeScript" +Delta 2: "TypeScript" + " is" = "TypeScript is" +Delta 3: "TypeScript is" + " a" = "TypeScript is a" +``` + +## Additional Improvements + +### 1. **Auto-Scroll** +```typescript +const streamingBoxRef = useRef(null); + +useEffect(() => { + if (isGenerating && streamingContent && streamingBoxRef.current) { + streamingBoxRef.current.scrollTop = streamingBoxRef.current.scrollHeight; + } +}, [streamingContent, isGenerating]); +``` + +Automatically scrolls to show new content as it arrives. + +### 2. **Smooth Scrolling** +```typescript +sx={{ + scrollBehavior: 'smooth', + // ... +}} +``` + +Smooth animation when auto-scrolling. + +### 3. **Better HTML Styling** +```typescript +'& h2, & h3': { mt: 2, mb: 1 }, +'& p': { mb: 1 }, +'& ul, & ol': { pl: 3, mb: 1 }, +'& li': { mb: 0.5 }, +``` + +Proper spacing for HTML elements. + +## Technical Explanation + +### Closure Problem + +In JavaScript, when you create a callback function, it "closes over" the variables from its surrounding scope. Those variables are captured at the time the function is created. + +```typescript +const [content, setContent] = useState(''); + +// This callback is created once when component mounts +const callback = () => { + console.log(content); // Always logs the INITIAL value! +}; +``` + +### Why Refs Work + +Refs are **mutable objects** that persist across renders: + +```typescript +const ref = useRef(''); + +// This always reads the CURRENT value +const callback = () => { + console.log(ref.current); // Gets the latest value! +}; +``` + +## Before vs After + +### Before (Broken) +``` +User sees: +"TypeScript" +" is" +" a" +"powerful" +...individual tokens, no accumulation +``` + +### After (Fixed) +``` +User sees: +"TypeScript" +"TypeScript is" +"TypeScript is a" +"TypeScript is a powerful" +...proper accumulation, rendered as HTML +``` + +## Files Changed + +``` +✅ apps/admin/src/components/steps/StepGenerate.tsx + - Added contentBufferRef for accumulation + - Added streamingBoxRef for auto-scroll + - Added useEffect for auto-scroll + - Fixed onContent callback to use ref + - Added smooth scroll behavior +``` + +## Testing + +1. Start generation +2. Watch the "Live Generation" box +3. Should see: + - ✅ Proper HTML rendering (headings, paragraphs, lists) + - ✅ Content accumulating (not individual tokens) + - ✅ Auto-scroll to bottom + - ✅ Smooth animations + - ✅ Pulsing blue border + +## Common React Pitfalls + +This is a classic React pitfall that catches many developers: + +### ❌ Don't Do This +```typescript +const [value, setValue] = useState(0); + +setInterval(() => { + setValue(value + 1); // Always adds to initial value! +}, 1000); +``` + +### ✅ Do This Instead +```typescript +const [value, setValue] = useState(0); + +setInterval(() => { + setValue(prev => prev + 1); // Functional update +}, 1000); + +// OR use a ref +const valueRef = useRef(0); +setInterval(() => { + valueRef.current += 1; + setValue(valueRef.current); +}, 1000); +``` + +## Conclusion + +The fix ensures that streaming content properly accumulates and displays as formatted HTML, providing a smooth, professional user experience with auto-scrolling and proper rendering. + +**Status**: ✅ Fixed and working! diff --git a/apps/admin/src/components/steps/StepGenerate.tsx b/apps/admin/src/components/steps/StepGenerate.tsx index 3671f35..1a0a000 100644 --- a/apps/admin/src/components/steps/StepGenerate.tsx +++ b/apps/admin/src/components/steps/StepGenerate.tsx @@ -1,4 +1,4 @@ -import { useState } from 'react'; +import { useState, useRef, useEffect } from 'react'; import { Box, Stack, TextField, Typography, Button, Alert, CircularProgress, FormControlLabel, Checkbox, Link, LinearProgress } from '@mui/material'; import SelectedImages from './SelectedImages'; import CollapsibleSection from './CollapsibleSection'; @@ -54,6 +54,16 @@ export default function StepGenerate({ }) { const [useWebSearch, setUseWebSearch] = useState(false); const [useStreaming, setUseStreaming] = useState(true); + const streamingBoxRef = useRef(null); + const contentBufferRef = useRef(''); + + // Auto-scroll to bottom when streaming content updates + useEffect(() => { + if (isGenerating && streamingContent && streamingBoxRef.current) { + streamingBoxRef.current.scrollTop = streamingBoxRef.current.scrollHeight; + } + }, [streamingContent, isGenerating]); + return ( { - onSetStreamingContent(streamingContent + data.delta); + // Accumulate in ref to avoid stale closure + contentBufferRef.current += data.delta; + onSetStreamingContent(contentBufferRef.current); onSetTokenCount(data.tokenCount); }, onDone: (data) => { @@ -243,6 +256,7 @@ export default function StepGenerate({ {isGenerating && useStreaming && streamingContent && (