Fix session/tool-question UX regressions and stabilize streaming activity rendering (#451)
* fix: stale model variants in chat controls in draft session * fix: attention indicator for project tabs * fix: use part index and type for consistent IDs * fix question card navigation for unanswered questions
This commit is contained in:
committed by
GitHub
parent
50a66bb6f1
commit
22255cf2c0
@@ -541,11 +541,9 @@ export const ModelControls: React.FC<ModelControlsProps> = ({
|
||||
const inputModalityIcons = getModalityIcons(currentMetadata, 'input');
|
||||
const outputModalityIcons = getModalityIcons(currentMetadata, 'output');
|
||||
|
||||
// Providers/models can reload (directory switch/config sync) without changing
|
||||
// currentProviderId/currentModelId; include providers to avoid stale variants.
|
||||
const availableVariants = React.useMemo(() => {
|
||||
return getCurrentModelVariants();
|
||||
}, [getCurrentModelVariants]);
|
||||
// Compute from current model each render to avoid stale variants
|
||||
// in draft/session transitions.
|
||||
const availableVariants = getCurrentModelVariants();
|
||||
const hasVariants = availableVariants.length > 0;
|
||||
|
||||
const costRows = [
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import React from 'react';
|
||||
import { RiCheckLine, RiCloseLine, RiEditLine, RiListCheck3, RiQuestionLine } from '@remixicon/react';
|
||||
import { RiArrowRightSLine, RiCheckLine, RiCloseLine, RiEditLine, RiListCheck3, RiQuestionLine } from '@remixicon/react';
|
||||
import { Checkbox } from '@/components/ui/checkbox';
|
||||
|
||||
import { cn } from '@/lib/utils';
|
||||
@@ -76,25 +76,43 @@ export const QuestionCard: React.FC<QuestionCardProps> = ({ question }) => {
|
||||
const selectedForActive = selectedOptions[activeIndex] ?? [];
|
||||
const isCustomActive = Boolean(customMode[activeIndex]);
|
||||
|
||||
const requiredSatisfied = React.useMemo(() => {
|
||||
if (questions.length === 0) return false;
|
||||
|
||||
const unansweredIndexes = React.useMemo(() => {
|
||||
const pending: number[] = [];
|
||||
for (let index = 0; index < questions.length; index += 1) {
|
||||
const isCustom = Boolean(customMode[index]);
|
||||
if (isCustom) {
|
||||
const value = (customText[index] ?? '').trim();
|
||||
if (!value) return false;
|
||||
if (!value) pending.push(index);
|
||||
continue;
|
||||
}
|
||||
|
||||
const answers = selectedOptions[index] ?? [];
|
||||
if (answers.length === 0) {
|
||||
return false;
|
||||
pending.push(index);
|
||||
}
|
||||
}
|
||||
return pending;
|
||||
}, [customMode, customText, questions.length, selectedOptions]);
|
||||
|
||||
const requiredSatisfied = React.useMemo(() => {
|
||||
if (questions.length === 0) return false;
|
||||
return unansweredIndexes.length === 0;
|
||||
}, [questions.length, unansweredIndexes.length]);
|
||||
|
||||
const handleNextUnanswered = React.useCallback(() => {
|
||||
if (questions.length === 0 || unansweredIndexes.length === 0) return;
|
||||
|
||||
const start = isSummaryTab ? -1 : activeIndex;
|
||||
for (let offset = 1; offset <= questions.length; offset += 1) {
|
||||
const candidate = (start + offset + questions.length) % questions.length;
|
||||
if (unansweredIndexes.includes(candidate)) {
|
||||
setActiveTab(String(candidate));
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
return true;
|
||||
}, [customMode, customText, questions.length, selectedOptions]);
|
||||
setActiveTab(String(unansweredIndexes[0]));
|
||||
}, [activeIndex, isSummaryTab, questions.length, unansweredIndexes]);
|
||||
|
||||
const buildAnswersPayload = React.useCallback((): string[][] => {
|
||||
const answers: string[][] = [];
|
||||
@@ -197,6 +215,8 @@ export const QuestionCard: React.FC<QuestionCardProps> = ({ question }) => {
|
||||
{tabs.map((tab) => {
|
||||
const isActive = activeTab === tab.value;
|
||||
const isSummary = tab.value === SUMMARY_TAB;
|
||||
const tabIndex = isSummary ? -1 : Number(tab.value);
|
||||
const isAnswered = !isSummary && Number.isFinite(tabIndex) && !unansweredIndexes.includes(tabIndex);
|
||||
return (
|
||||
<button
|
||||
key={tab.value}
|
||||
@@ -205,8 +225,12 @@ export const QuestionCard: React.FC<QuestionCardProps> = ({ question }) => {
|
||||
className={cn(
|
||||
'px-2 py-0.5 typography-meta font-medium rounded transition-colors flex items-center gap-1',
|
||||
isActive
|
||||
? 'bg-interactive-selection/40 text-foreground'
|
||||
: 'text-muted-foreground hover:text-foreground hover:bg-interactive-hover/20'
|
||||
? 'bg-interactive-selection/40 text-foreground'
|
||||
: isSummary
|
||||
? 'text-muted-foreground hover:text-foreground hover:bg-interactive-hover/20'
|
||||
: isAnswered
|
||||
? 'text-muted-foreground/60 hover:text-muted-foreground hover:bg-interactive-hover/20'
|
||||
: 'text-foreground/85 hover:text-foreground hover:bg-interactive-hover/20'
|
||||
)}
|
||||
>
|
||||
{isSummary ? <RiListCheck3 className="h-3 w-3" /> : null}
|
||||
@@ -362,16 +386,16 @@ export const QuestionCard: React.FC<QuestionCardProps> = ({ question }) => {
|
||||
<div className="px-2 pb-1.5 pt-1 flex items-center gap-1.5 border-t border-border/20">
|
||||
<button
|
||||
type="button"
|
||||
onClick={handleConfirm}
|
||||
disabled={isResponding || !requiredSatisfied}
|
||||
onClick={requiredSatisfied ? handleConfirm : handleNextUnanswered}
|
||||
disabled={isResponding}
|
||||
className={cn(
|
||||
'flex items-center gap-1 px-2 py-1 typography-meta font-medium rounded transition-colors',
|
||||
'bg-[rgb(var(--status-success)/0.1)] text-[var(--status-success)] hover:bg-[rgb(var(--status-success)/0.2)]',
|
||||
'disabled:opacity-50 disabled:cursor-not-allowed'
|
||||
)}
|
||||
>
|
||||
<RiCheckLine className="h-3 w-3" />
|
||||
Confirm
|
||||
{requiredSatisfied ? <RiCheckLine className="h-3 w-3" /> : <RiArrowRightSLine className="h-3 w-3" />}
|
||||
{requiredSatisfied ? 'Submit' : 'Next'}
|
||||
</button>
|
||||
|
||||
<button
|
||||
|
||||
@@ -332,7 +332,6 @@ const getTurnActivityInfo = (turn: Turn, showTextJustificationActivity: boolean)
|
||||
}
|
||||
|
||||
const activityParts: TurnActivityPart[] = [];
|
||||
let syntheticIdCounter = 0;
|
||||
|
||||
turn.assistantMessages.forEach((msg) => {
|
||||
const messageId = msg.info.id;
|
||||
@@ -341,10 +340,8 @@ const getTurnActivityInfo = (turn: Turn, showTextJustificationActivity: boolean)
|
||||
// All earlier text messages are justification
|
||||
const isFinalSummaryMessage = messageId === lastTextMessageId;
|
||||
|
||||
msg.parts.forEach((part) => {
|
||||
const baseId = (typeof part.id === 'string' && part.id.trim().length > 0)
|
||||
? part.id
|
||||
: `${messageId}-activity-${syntheticIdCounter++}`;
|
||||
msg.parts.forEach((part, partIndex) => {
|
||||
const baseId = `${messageId}-part-${partIndex}-${part.type}`;
|
||||
|
||||
if (part.type === 'tool') {
|
||||
const state = (part as { state?: { time?: { end?: number | null | undefined } | null | undefined } | null | undefined }).state;
|
||||
@@ -418,13 +415,11 @@ const getTurnActivityInfo = (turn: Turn, showTextJustificationActivity: boolean)
|
||||
turn.assistantMessages.forEach((msg) => {
|
||||
const messageId = msg.info.id;
|
||||
|
||||
msg.parts.forEach((part) => {
|
||||
msg.parts.forEach((part, partIndex) => {
|
||||
if (part.type === 'tool') {
|
||||
const toolName = (part as { tool?: unknown }).tool;
|
||||
if (isActivityStandaloneTool(toolName)) {
|
||||
const toolPartId = typeof part.id === 'string' && part.id.trim().length > 0
|
||||
? part.id
|
||||
: `${messageId}-task-${taskOrder.length + 1}`;
|
||||
const toolPartId = `${messageId}-part-${partIndex}-${part.type}`;
|
||||
|
||||
if (!taskMessageById.has(toolPartId)) {
|
||||
taskMessageById.set(toolPartId, messageId);
|
||||
|
||||
@@ -201,7 +201,6 @@ const getTurnActivityInfo = (turn: Turn, showTextJustificationActivity: boolean)
|
||||
}
|
||||
|
||||
const activityParts: TurnActivityPart[] = [];
|
||||
let syntheticIdCounter = 0;
|
||||
|
||||
turn.assistantMessages.forEach((msg) => {
|
||||
const messageId = msg.info.id;
|
||||
@@ -210,11 +209,8 @@ const getTurnActivityInfo = (turn: Turn, showTextJustificationActivity: boolean)
|
||||
// All earlier text messages are justification
|
||||
const isFinalSummaryMessage = messageId === lastTextMessageId;
|
||||
|
||||
msg.parts.forEach((part) => {
|
||||
const baseId =
|
||||
(typeof part.id === 'string' && part.id.trim().length > 0)
|
||||
? part.id
|
||||
: `${messageId}-activity-${syntheticIdCounter++}`;
|
||||
msg.parts.forEach((part, partIndex) => {
|
||||
const baseId = `${messageId}-part-${partIndex}-${part.type}`;
|
||||
|
||||
if (part.type === 'tool') {
|
||||
const state = (part as { state?: { time?: { end?: number | null | undefined } | null | undefined } | null | undefined }).state;
|
||||
@@ -295,13 +291,11 @@ const getTurnActivityInfo = (turn: Turn, showTextJustificationActivity: boolean)
|
||||
turn.assistantMessages.forEach((msg) => {
|
||||
const messageId = msg.info.id;
|
||||
|
||||
msg.parts.forEach((part) => {
|
||||
msg.parts.forEach((part, partIndex) => {
|
||||
if (part.type === 'tool') {
|
||||
const toolName = (part as { tool?: unknown }).tool;
|
||||
if (isActivityStandaloneTool(toolName)) {
|
||||
const toolPartId = typeof part.id === 'string' && part.id.trim().length > 0
|
||||
? part.id
|
||||
: `${messageId}-task-${taskOrder.length + 1}`;
|
||||
const toolPartId = `${messageId}-part-${partIndex}-${part.type}`;
|
||||
|
||||
if (!taskMessageById.has(toolPartId)) {
|
||||
taskMessageById.set(toolPartId, messageId);
|
||||
|
||||
@@ -51,13 +51,11 @@ const getToolConnections = (
|
||||
const toolParts = parts.filter((p) => p.kind === 'tool');
|
||||
|
||||
toolParts.forEach((activity, index) => {
|
||||
const partId = activity.part.id;
|
||||
if (partId) {
|
||||
connections[partId] = {
|
||||
hasPrev: index > 0,
|
||||
hasNext: index < toolParts.length - 1,
|
||||
};
|
||||
}
|
||||
const partId = activity.id;
|
||||
connections[partId] = {
|
||||
hasPrev: index > 0,
|
||||
hasNext: index < toolParts.length - 1,
|
||||
};
|
||||
});
|
||||
|
||||
return connections;
|
||||
@@ -117,7 +115,7 @@ const ProgressiveGroup: React.FC<ProgressiveGroupProps> = ({
|
||||
const visibleInCollapsedIds = React.useMemo(() => {
|
||||
const ids = new Set<string>();
|
||||
visibleCollapsedParts.forEach((p) => {
|
||||
if (p.part.id) ids.add(p.part.id);
|
||||
ids.add(p.id);
|
||||
});
|
||||
return ids;
|
||||
}, [visibleCollapsedParts]);
|
||||
@@ -218,8 +216,8 @@ const ProgressiveGroup: React.FC<ProgressiveGroupProps> = ({
|
||||
</div>
|
||||
)}
|
||||
|
||||
{partsToRender.map((activity, index) => {
|
||||
const partId = activity.part.id || `group-part-${index}`;
|
||||
{partsToRender.map((activity) => {
|
||||
const partId = activity.id;
|
||||
const connection = connectionsToUse[partId];
|
||||
|
||||
const animationKey = `${partId}-exp${expansionKey}`;
|
||||
@@ -227,19 +225,19 @@ const ProgressiveGroup: React.FC<ProgressiveGroupProps> = ({
|
||||
// Determine if animation should be skipped:
|
||||
// 1. When expanding from collapsed: skip for items that were already visible
|
||||
// 2. When collapsed: skip for items already shown before (track in ref)
|
||||
const wasVisibleInCollapsed = activity.part.id ? visibleInCollapsedIds.has(activity.part.id) : false;
|
||||
const wasVisibleInCollapsed = visibleInCollapsedIds.has(activity.id);
|
||||
|
||||
let skipAnimation = false;
|
||||
if (justExpandedFromCollapsed && wasVisibleInCollapsed) {
|
||||
// Expanding: don't animate items that were already visible in collapsed state
|
||||
skipAnimation = true;
|
||||
} else if (!isExpanded && activity.part.id) {
|
||||
} else if (!isExpanded) {
|
||||
// Collapsed: animate only items that haven't been shown yet
|
||||
if (shownInCollapsedRef.current.has(activity.part.id)) {
|
||||
if (shownInCollapsedRef.current.has(activity.id)) {
|
||||
skipAnimation = true;
|
||||
} else {
|
||||
// Mark as shown for future renders
|
||||
shownInCollapsedRef.current.add(activity.part.id);
|
||||
shownInCollapsedRef.current.add(activity.id);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -250,7 +248,7 @@ const ProgressiveGroup: React.FC<ProgressiveGroupProps> = ({
|
||||
<ToolPart
|
||||
part={activity.part as ToolPartType}
|
||||
isExpanded={expandedTools.has(partId)}
|
||||
onToggle={onToggleTool}
|
||||
onToggle={() => onToggleTool(partId)}
|
||||
syntaxTheme={syntaxTheme}
|
||||
isMobile={isMobile}
|
||||
onContentChange={onContentChange}
|
||||
|
||||
@@ -425,7 +425,8 @@ export const Header: React.FC = () => {
|
||||
hasStreaming = true;
|
||||
}
|
||||
|
||||
if (session.id !== currentSessionId && sessionAttentionStates.get(session.id)?.needsAttention === true) {
|
||||
const isCurrentVisibleSession = session.id === currentSessionId && project.id === activeProjectId;
|
||||
if (!isCurrentVisibleSession && sessionAttentionStates.get(session.id)?.needsAttention === true) {
|
||||
hasNeedsAttention = true;
|
||||
}
|
||||
|
||||
@@ -442,7 +443,7 @@ export const Header: React.FC = () => {
|
||||
}
|
||||
|
||||
return result;
|
||||
}, [availableWorktreesByProject, currentSessionId, getSessionsByDirectory, projects, sessionAttentionStates, sessionStatus, sessionsByDirectory, showProjectTabs]);
|
||||
}, [activeProjectId, availableWorktreesByProject, currentSessionId, getSessionsByDirectory, projects, sessionAttentionStates, sessionStatus, sessionsByDirectory, showProjectTabs]);
|
||||
|
||||
React.useLayoutEffect(() => {
|
||||
if (!showProjectTabs) return;
|
||||
|
||||
@@ -45,6 +45,7 @@ declare global {
|
||||
const ENABLE_EMPTY_RESPONSE_DETECTION = false;
|
||||
const TEXT_SHRINK_TOLERANCE = 50;
|
||||
const RESYNC_DEBOUNCE_MS = 750;
|
||||
const QUESTION_RECONCILE_COOLDOWN_MS = 1500;
|
||||
|
||||
const textLengthCache = new WeakMap<Part[], number>();
|
||||
const computeTextLength = (parts: Part[] | undefined | null): number => {
|
||||
@@ -153,6 +154,38 @@ export const useEventStream = () => {
|
||||
return undefined;
|
||||
}, [activeSessionDirectory, fallbackDirectory]);
|
||||
|
||||
const bootstrapPendingQuestions = React.useCallback(async () => {
|
||||
try {
|
||||
const projects = useProjectsStore.getState().projects;
|
||||
const projectDirs = projects.map((project) => project.path);
|
||||
// Use getState() to avoid sessions dependency which causes cascading updates
|
||||
const currentSessions = useSessionStore.getState().sessions;
|
||||
const sessionDirs = currentSessions.map((session) => (session as { directory?: string | null }).directory);
|
||||
|
||||
const directories = [effectiveDirectory, ...projectDirs, ...sessionDirs];
|
||||
const pending = await opencodeClient.listPendingQuestions({ directories });
|
||||
if (pending.length === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
for (const request of pending) {
|
||||
addQuestion(request as unknown as QuestionRequest);
|
||||
}
|
||||
} catch {
|
||||
// ignored
|
||||
}
|
||||
}, [addQuestion, effectiveDirectory]);
|
||||
|
||||
const lastQuestionRefreshAtRef = React.useRef(0);
|
||||
const requestPendingQuestionsRefresh = React.useCallback((force = false) => {
|
||||
const now = Date.now();
|
||||
if (!force && now - lastQuestionRefreshAtRef.current < QUESTION_RECONCILE_COOLDOWN_MS) {
|
||||
return;
|
||||
}
|
||||
lastQuestionRefreshAtRef.current = now;
|
||||
void bootstrapPendingQuestions();
|
||||
}, [bootstrapPendingQuestions]);
|
||||
|
||||
React.useEffect(() => {
|
||||
let cancelled = false;
|
||||
|
||||
@@ -171,36 +204,13 @@ export const useEventStream = () => {
|
||||
}
|
||||
};
|
||||
|
||||
const bootstrapPendingQuestions = async () => {
|
||||
try {
|
||||
const projects = useProjectsStore.getState().projects;
|
||||
const projectDirs = projects.map((project) => project.path);
|
||||
// Use getState() to avoid sessions dependency which causes cascading updates
|
||||
const currentSessions = useSessionStore.getState().sessions;
|
||||
const sessionDirs = currentSessions.map((session) => (session as { directory?: string | null }).directory);
|
||||
|
||||
const directories = [effectiveDirectory, ...projectDirs, ...sessionDirs];
|
||||
|
||||
const pending = await opencodeClient.listPendingQuestions({ directories });
|
||||
if (cancelled || pending.length === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
for (const request of pending) {
|
||||
addQuestion(request as unknown as QuestionRequest);
|
||||
}
|
||||
} catch {
|
||||
// ignored
|
||||
}
|
||||
};
|
||||
|
||||
void bootstrapPendingPermissions();
|
||||
void bootstrapPendingQuestions();
|
||||
requestPendingQuestionsRefresh(true);
|
||||
|
||||
return () => {
|
||||
cancelled = true;
|
||||
};
|
||||
}, [addPermission, addQuestion, effectiveDirectory]);
|
||||
}, [addPermission, requestPendingQuestionsRefresh]);
|
||||
|
||||
const normalizeDirectory = React.useCallback((value: string | null | undefined): string | null => {
|
||||
if (typeof value !== 'string') return null;
|
||||
@@ -852,8 +862,15 @@ export const useEventStream = () => {
|
||||
const partTime = (messagePart as { time?: { end?: unknown } }).time;
|
||||
const partHasEnded = typeof partTime?.end === 'number';
|
||||
const toolState = (messagePart as { state?: { status?: unknown } }).state?.status;
|
||||
const toolName = typeof (messagePart as { tool?: unknown }).tool === 'string'
|
||||
? (messagePart as { tool: string }).tool.toLowerCase()
|
||||
: null;
|
||||
const textContent = (messagePart as { text?: unknown }).text;
|
||||
|
||||
if (partType === 'tool' && toolName === 'question') {
|
||||
requestPendingQuestionsRefresh();
|
||||
}
|
||||
|
||||
const isStreamingPart = (() => {
|
||||
if (partType === 'tool') {
|
||||
return toolState === 'running' || toolState === 'pending';
|
||||
@@ -1229,6 +1246,15 @@ export const useEventStream = () => {
|
||||
if (!hasParts && !completedFromServer && !hasCompletedStatus && !eventHasStopFinish) break;
|
||||
|
||||
if ((messageExt as { role?: unknown }).role === 'assistant' && hasParts) {
|
||||
const hasQuestionTool = partsArray.some((part) => (
|
||||
part?.type === 'tool'
|
||||
&& typeof (part as { tool?: unknown }).tool === 'string'
|
||||
&& (part as { tool: string }).tool.toLowerCase() === 'question'
|
||||
));
|
||||
if (hasQuestionTool) {
|
||||
requestPendingQuestionsRefresh();
|
||||
}
|
||||
|
||||
const incomingLen = computeTextLength(partsArray);
|
||||
const wouldShrink = existingLen > 0 && incomingLen + TEXT_SHRINK_TOLERANCE < existingLen;
|
||||
|
||||
@@ -1662,6 +1688,7 @@ export const useEventStream = () => {
|
||||
applySessionMetadata,
|
||||
trackMessage,
|
||||
reportMessage,
|
||||
requestPendingQuestionsRefresh,
|
||||
|
||||
updateSession,
|
||||
removeSessionFromStore,
|
||||
|
||||
Reference in New Issue
Block a user