Skip to content

Commit 16ba9d6

Browse files
committed
fix(vscode): address review findings on the design overhaul
Guard approval and question dialogs against duplicate in-flight responses, use a clipboard sentinel to detect an empty terminal selection, promote token counts that round to one million into the m unit, stop claiming the config file is missing after a load error, group sessions by calendar-day boundaries so DST does not shift them, and drop a stray console statement.
1 parent e131e66 commit 16ba9d6

7 files changed

Lines changed: 54 additions & 20 deletions

File tree

apps/vscode/src/integrations/chat-context.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,20 +91,22 @@ export function registerChatContext(deps: ChatContextDeps): vscode.Disposable[]
9191
vscode.commands.registerCommand("pythinker.addTerminalSelection", async () => {
9292
// Clipboard round-trip: workbench.action.terminal.copySelection is the
9393
// only way to read the terminal selection, so save and restore the
94-
// user's clipboard around it.
94+
// user's clipboard around it. A sentinel distinguishes "no selection"
95+
// (copySelection is a no-op) from a selection that happens to equal the
96+
// old clipboard content.
9597
const previousClipboard = await vscode.env.clipboard.readText();
98+
const sentinel = `__pythinker_no_selection_${Date.now()}__`;
9699
let selection = "";
97100
try {
101+
await vscode.env.clipboard.writeText(sentinel);
98102
await vscode.commands.executeCommand("workbench.action.terminal.copySelection");
99103
selection = (await vscode.env.clipboard.readText()).trim();
100104
} catch (error) {
101105
deps.logError("Unable to read the terminal selection", error);
102106
} finally {
103107
await vscode.env.clipboard.writeText(previousClipboard);
104108
}
105-
// copySelection is a no-op without a selection, which leaves the old
106-
// clipboard content in place — do not insert that.
107-
if (!selection || selection === previousClipboard.trim()) return;
109+
if (!selection || selection === sentinel) return;
108110
await vscode.commands.executeCommand("pythinker.webview.focus");
109111
deps.insertText(`Terminal output:\n\`\`\`\n${selection}\n\`\`\``);
110112
}),

apps/vscode/webview-ui/src/components/ApprovalDialog.tsx

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ export function ApprovalDialog() {
1212
const [selectedIndex, setSelectedIndex] = useState(1);
1313
const [expanded, setExpanded] = useState(false);
1414
const cardRef = useRef<HTMLDivElement>(null);
15+
// The request stays in the store until the RPC settles, so repeated key
16+
// presses would send duplicate responses without this guard.
17+
const inFlightRef = useRef(false);
1518

1619
const req = pending[0];
1720

@@ -28,10 +31,16 @@ export function ApprovalDialog() {
2831
const hasDisplay = req.display && req.display.length > 0;
2932

3033
const handleResponse = async (response: ApprovalResponse) => {
31-
await respondToRequest(req.id, response);
32-
setSelectedIndex(1);
33-
setExpanded(false);
34-
focusComposer();
34+
if (inFlightRef.current) return;
35+
inFlightRef.current = true;
36+
try {
37+
await respondToRequest(req.id, response);
38+
setSelectedIndex(1);
39+
setExpanded(false);
40+
focusComposer();
41+
} finally {
42+
inFlightRef.current = false;
43+
}
3544
};
3645

3746
const options = [

apps/vscode/webview-ui/src/components/ChatStatus.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ function subscribeToSpeed(listener: () => void): () => void {
6060

6161
/** Compact token count in Cline's style: 999, 12.3k, 1.2m. */
6262
function formatTokens(count: number): string {
63-
if (count >= 1e6) return `${(count / 1e6).toFixed(1)}m`;
63+
// 999_950+ rounds to 1.0m; without this the k branch prints "1000.0k".
64+
if (count >= 999_950) return `${(count / 1e6).toFixed(1)}m`;
6465
if (count >= 1e3) return `${(count / 1e3).toFixed(1)}k`;
6566
return String(count);
6667
}

apps/vscode/webview-ui/src/components/QuestionDialog.tsx

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ export function QuestionDialog() {
1212
const [questionIndex, setQuestionIndex] = useState(0);
1313
const [answers, setAnswers] = useState<Record<string, string>>({});
1414
const cardRef = useRef<HTMLDivElement>(null);
15+
// The question stays pending until the RPC settles, so repeated key presses
16+
// would submit duplicate answers without this guard.
17+
const inFlightRef = useRef(false);
1518

1619
const questions = pendingQuestion?.questions ?? [];
1720
const question = questions[questionIndex];
@@ -39,8 +42,14 @@ export function QuestionDialog() {
3942
setCustomInput("");
4043
setSelectedIndex(1);
4144
} else {
42-
await respondQuestion(nextAnswers);
43-
focusComposer();
45+
if (inFlightRef.current) return;
46+
inFlightRef.current = true;
47+
try {
48+
await respondQuestion(nextAnswers);
49+
focusComposer();
50+
} finally {
51+
inFlightRef.current = false;
52+
}
4453
}
4554
};
4655

apps/vscode/webview-ui/src/components/SessionList.tsx

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,16 @@ interface SessionListProps {
1717
onClose: () => void;
1818
}
1919

20-
const DAY_MS = 86400000;
20+
interface GroupBoundaries {
21+
startOfToday: number;
22+
startOfYesterday: number;
23+
startOfWeekWindow: number;
24+
}
2125

22-
function getGroupLabel(timestamp: number, startOfToday: number): string {
23-
if (timestamp >= startOfToday) return "Today";
24-
if (timestamp >= startOfToday - DAY_MS) return "Yesterday";
25-
if (timestamp >= startOfToday - 6 * DAY_MS) return "This week";
26+
function getGroupLabel(timestamp: number, boundaries: GroupBoundaries): string {
27+
if (timestamp >= boundaries.startOfToday) return "Today";
28+
if (timestamp >= boundaries.startOfYesterday) return "Yesterday";
29+
if (timestamp >= boundaries.startOfWeekWindow) return "This week";
2630
return "Older";
2731
}
2832

@@ -131,11 +135,17 @@ export function SessionList({ onClose }: SessionListProps) {
131135
const sorted = [...filtered].sort((a, b) => (sortOrder === "recent" ? b.updatedAt - a.updatedAt : a.updatedAt - b.updatedAt));
132136

133137
const now = new Date();
134-
const startOfToday = new Date(now.getFullYear(), now.getMonth(), now.getDate()).getTime();
138+
// Calendar-day arithmetic, not fixed 24h offsets — a DST change makes a
139+
// local day 23 or 25 hours, which would shift sessions between groups.
140+
const boundaries = {
141+
startOfToday: new Date(now.getFullYear(), now.getMonth(), now.getDate()).getTime(),
142+
startOfYesterday: new Date(now.getFullYear(), now.getMonth(), now.getDate() - 1).getTime(),
143+
startOfWeekWindow: new Date(now.getFullYear(), now.getMonth(), now.getDate() - 6).getTime(),
144+
};
135145
// Map preserves insertion order, so group order follows the sort direction
136146
const groups = new Map<string, SessionInfo[]>();
137147
for (const session of sorted) {
138-
const label = getGroupLabel(session.updatedAt, startOfToday);
148+
const label = getGroupLabel(session.updatedAt, boundaries);
139149
const bucket = groups.get(label);
140150
if (bucket) {
141151
bucket.push(session);

apps/vscode/webview-ui/src/components/WelcomeScreen.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ export function WelcomeScreen() {
1818
const events = await bridge.loadSessionHistory(session.id);
1919
await loadSession(session.id, events);
2020
} catch (error) {
21-
console.error("[WelcomeScreen] Failed to load session:", error);
2221
toast.error(`Unable to open the conversation: ${error instanceof Error ? error.message : String(error)}`);
2322
}
2423
};

apps/vscode/webview-ui/src/components/confighub/ConfigFileSection.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,11 @@ export function ConfigFileSection() {
4848
</div>
4949
)}
5050

51-
{info === null || info.path === null || !info.exists ? (
51+
{info === null ? (
52+
// A load error with no result means the state is unknown — the error
53+
// banner above is the whole story, so do not claim the file is missing.
54+
error === undefined && <p className="text-xs text-muted-foreground text-center py-10">No config file was found.</p>
55+
) : info.path === null || !info.exists ? (
5256
<p className="text-xs text-muted-foreground text-center py-10">No config file was found.</p>
5357
) : (
5458
<>

0 commit comments

Comments
 (0)