diff --git a/client/src/components/Artifacts/ArtifactButton.tsx b/client/src/components/Artifacts/ArtifactButton.tsx index c42d75ebc0..64aa26292e 100644 --- a/client/src/components/Artifacts/ArtifactButton.tsx +++ b/client/src/components/Artifacts/ArtifactButton.tsx @@ -63,16 +63,12 @@ const ArtifactButton = ({ artifact }: { artifact: Artifact | null }) => { return; } - resetCurrentArtifactId(); + setCurrentArtifactId(artifact.id); setVisible(true); if (artifacts?.[artifact.id] == null) { setArtifacts(visibleArtifacts); } - - setTimeout(() => { - setCurrentArtifactId(artifact.id); - }, 15); }; const buttonClass = cn( diff --git a/client/src/hooks/Artifacts/__tests__/useArtifacts.test.ts b/client/src/hooks/Artifacts/__tests__/useArtifacts.test.ts index e84a6788d6..fa7eaca4e7 100644 --- a/client/src/hooks/Artifacts/__tests__/useArtifacts.test.ts +++ b/client/src/hooks/Artifacts/__tests__/useArtifacts.test.ts @@ -372,6 +372,75 @@ describe('useArtifacts', () => { }); }); + describe('artifact selection preservation', () => { + it('should preserve selection when a new artifact is added', () => { + const artifact1 = createArtifact({ id: 'artifact-1', lastUpdateTime: 1000 }); + + (useRecoilValue as jest.Mock).mockReturnValue({ 'artifact-1': artifact1 }); + (useRecoilState as jest.Mock).mockReturnValue(['artifact-1', mockSetCurrentArtifactId]); + + const { rerender } = renderHook(() => useArtifacts()); + + mockSetCurrentArtifactId.mockClear(); + + /** Append a second artifact; mock still returns 'artifact-1' as current */ + const artifact2 = createArtifact({ id: 'artifact-2', lastUpdateTime: 2000 }); + (useRecoilValue as jest.Mock).mockReturnValue({ + 'artifact-1': artifact1, + 'artifact-2': artifact2, + }); + + rerender(); + + expect(mockSetCurrentArtifactId).not.toHaveBeenCalled(); + }); + + it('should advance to new artifact during streaming', () => { + const artifact1 = createArtifact({ id: 'artifact-1', lastUpdateTime: 1000, content: 'c1' }); + + (useRecoilValue as jest.Mock).mockReturnValue({ 'artifact-1': artifact1 }); + (useRecoilState as jest.Mock).mockReturnValue(['artifact-1', mockSetCurrentArtifactId]); + (useArtifactsContext as jest.Mock).mockReturnValue({ + ...defaultContext, + isSubmitting: true, + latestMessageId: 'msg-1', + }); + + const { rerender } = renderHook(() => useArtifacts()); + mockSetCurrentArtifactId.mockClear(); + + const artifact2 = createArtifact({ id: 'artifact-2', lastUpdateTime: 2000, content: 'c2' }); + (useRecoilValue as jest.Mock).mockReturnValue({ + 'artifact-1': artifact1, + 'artifact-2': artifact2, + }); + + rerender(); + + expect(mockSetCurrentArtifactId).toHaveBeenCalledWith('artifact-2'); + }); + + it('should keep selection null after an explicit reset', () => { + const artifact1 = createArtifact({ id: 'artifact-1', lastUpdateTime: 1000 }); + + /** First render: valid selection */ + (useRecoilValue as jest.Mock).mockReturnValue({ 'artifact-1': artifact1 }); + (useRecoilState as jest.Mock).mockReturnValue(['artifact-1', mockSetCurrentArtifactId]); + + const { rerender } = renderHook(() => useArtifacts()); + + mockSetCurrentArtifactId.mockClear(); + + /** Rerender: currentArtifactId transitions to null (user closed the panel) */ + (useRecoilState as jest.Mock).mockReturnValue([null, mockSetCurrentArtifactId]); + + rerender(); + + /** Should NOT bounce back to 'artifact-1' */ + expect(mockSetCurrentArtifactId).not.toHaveBeenCalled(); + }); + }); + describe('cleanup on unmount', () => { it('should reset artifacts when unmounting', () => { (useRecoilValue as jest.Mock).mockReturnValue({}); diff --git a/client/src/hooks/Artifacts/useArtifacts.ts b/client/src/hooks/Artifacts/useArtifacts.ts index 25063741c6..55f26feadd 100644 --- a/client/src/hooks/Artifacts/useArtifacts.ts +++ b/client/src/hooks/Artifacts/useArtifacts.ts @@ -51,12 +51,19 @@ export default function useArtifacts() { }; }, [conversationId, resetArtifacts, resetCurrentArtifactId]); + /** + * Read currentArtifactId in effects without subscribing as a dependency. + * Adding it to effect deps fires auto-select on every reset, breaking toggle-close. + */ + const currentArtifactIdRef = useRef(currentArtifactId); + currentArtifactIdRef.current = currentArtifactId; + useEffect(() => { - if (orderedArtifactIds.length > 0) { - const latestArtifactId = orderedArtifactIds[orderedArtifactIds.length - 1]; - setCurrentArtifactId(latestArtifactId); - } - }, [setCurrentArtifactId, orderedArtifactIds]); + if (orderedArtifactIds.length === 0) return; + const currentId = currentArtifactIdRef.current; + if (currentId != null && orderedArtifactIds.includes(currentId)) return; + setCurrentArtifactId(orderedArtifactIds[orderedArtifactIds.length - 1]); + }, [orderedArtifactIds, setCurrentArtifactId]); /** * Manage artifact selection and code tab switching for non-enclosed artifacts