🎯 fix: Preserve Selected Artifact When Clicking Artifact Button

[starchow]

Summary

Clicking an artifact button in the conversation incorrectly resets the view to the latest artifact instead of opening the clicked one. This happens because:

1. ArtifactButton was calling resetCurrentArtifactId() and then relying on a setTimeout to set the correct ID 15ms later — a race condition with the useArtifacts effect.
2. The useArtifacts effect unconditionally overwrites currentArtifactId with the latest artifact whenever orderedArtifactIds changes, clobbering the user's selection.

This PR fixes both issues:

1. ArtifactButton now sets the artifact ID directly via setCurrentArtifactId(artifact.id) instead of the reset + setTimeout workaround.
2. The auto-select effect uses a currentArtifactIdRef to read the current selection without subscribing as a reactive dependency. This preserves valid user selections when new artifacts are added, while still falling back to the latest artifact when no valid selection exists. The ref pattern avoids a toggle-close regression that occurs when currentArtifactId is added directly to the effect's dependency array.
3. The streaming effect (effect The backend program does not provide it. It cannot be linked to the backend #3) remains unchanged from base — it unconditionally tracks the latest artifact during active generation. A userHasManualSelection guard was explored but reverted because it cannot distinguish manual clicks from system auto-selection, which blocked auto-advancement to new artifacts during streaming.

Three new tests cover the behavioral contracts: selection preservation on new artifact, auto-advancement during streaming, and reset stability (no bounce-back after toggle-close).

Change Type

• Bug fix (non-breaking change which fixes an issue)

Testing

1. Start a conversation and generate multiple artifacts.
2. Click on an earlier artifact button in the message stream.
3. Verify the side panel opens showing the clicked artifact, not the latest one.
4. Generate a new artifact — verify the panel auto-selects the new one.
5. Click a selected artifact to close the panel — verify it stays closed and the button deselects.
6. Switch conversations and return — verify the latest artifact is selected by default.

Checklist

• My code adheres to this project's style guidelines
• I have performed a self-review of my own code
• My changes do not introduce new warnings
• Local unit tests pass with my changes

[danny-avila]

Hey @starchow, thanks for digging into this one — the diagnosis is right and dropping the setTimeout race in ArtifactButton is the correct call. A few things came up while I was reviewing that I'd like you to take another pass on before we merge.

1. Toggle-close regression in useArtifacts.ts

Adding currentArtifactId to the effect's dependency array introduces a feedback loop on the close path. When a user clicks an already-selected artifact button, ArtifactButton runs:

if (isSelected) {
  resetCurrentArtifactId();   // currentArtifactId → null
  setVisible(false);
  return;
}

That null change re-fires the effect (because currentArtifactId is now in its deps). The first guard (length === 0) doesn't apply, the second guard (currentArtifactId != null && includes(...)) skips because the id is null, and the effect falls through to setCurrentArtifactId(latestArtifactId) — undoing the reset.

The visible symptom: clicking a selected artifact hides the panel, but the button stays in its selected state with the "click to close" label, and a second click does nothing (the panel is already hidden). The toggle is effectively broken.

The cleanest fix is to read currentArtifactId through a ref so the effect doesn't have to react to it:

const currentArtifactIdRef = useRef(currentArtifactId);
currentArtifactIdRef.current = currentArtifactId;

useEffect(() => {
  if (orderedArtifactIds.length === 0) return;
  const currentId = currentArtifactIdRef.current;
  if (currentId != null && orderedArtifactIds.includes(currentId)) return;
  setCurrentArtifactId(orderedArtifactIds[orderedArtifactIds.length - 1]);
}, [orderedArtifactIds, setCurrentArtifactId]);

That preserves the "skip if already valid" behavior you wanted, but restores the original firing frequency so an explicit reset stays a reset.

2. The streaming effect still clobbers manual selection

A few lines below (the "Manage artifact selection and code tab switching for non-enclosed artifacts" effect) there's an unconditional setCurrentArtifactId(latestArtifactId) that runs on every content update while isSubmitting is true:

setCurrentArtifactId(latestArtifactId);
lastContentRef.current = latestArtifact?.content ?? null;

If a user clicks an earlier artifact while a new one is still streaming, the next chunk snaps them right back to the latest artifact — so step 2 of your test plan ("Click on an earlier artifact button in the message stream") only really works after generation finishes. Could you apply the same guard pattern here so a valid manual selection isn't overwritten mid-stream?

const userHasManualSelection =
  currentArtifactId != null &&
  currentArtifactId !== latestArtifactId &&
  orderedArtifactIds.includes(currentArtifactId);

if (!userHasManualSelection) {
  setCurrentArtifactId(latestArtifactId);
}

3. Tests for the new behavior

Could you add a couple of tests to client/src/hooks/Artifacts/__tests__/useArtifacts.test.ts covering:

• Selection preserved when a new artifact is added — render the hook with currentArtifactId = 'artifact-1', rerender with a second artifact appended to orderedArtifactIds, assert setCurrentArtifactId was not called.
• Selection stays null after an explicit reset — render with an existing selection, simulate currentArtifactId transitioning to null, assert it doesn't bounce back to the latest artifact. This is the test that would have caught complete copyCode for code blocks #1.

Heads up: the existing setup does (useRecoilState as jest.Mock).mockReturnValue([null, mockSetCurrentArtifactId]), which returns null regardless of what the setter is called with — you'll need to vary the mock's return value across rerenders for these tests to actually exercise the feedback loop.

---

Let me know if anything is unclear or you'd like me to expand on the fix for #1. Appreciate the contribution!

[danny-avila]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9db368cce9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[starchow]

Hi @danny-avila I've changed the code as your feedbacks, please review it again. Thanks.