Skip to content

Commit e19958f

Browse files
committed
fix: address review findings
1 parent b30273b commit e19958f

10 files changed

Lines changed: 84 additions & 39 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
/** Frame interval for the rainbow flow animation. */
2+
export const HATCH_FRAME_MS = 110;
3+
/** How long the rainbow flows before settling (fading out, or freezing). */
4+
export const HATCH_FLOW_MS = 3000;

apps/pythinker-code/src/tui/controllers/editor-keyboard.ts

Lines changed: 32 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -527,8 +527,12 @@ export class EditorKeyboardController {
527527
void this.host.session?.cancel();
528528
}
529529

530+
/** Guards Shift-Tab cycling while a setThinking round-trip is still pending. */
531+
private thinkingCycleInFlight = false;
532+
530533
/** Shift-Tab: cycle the thinking effort to the current model's next level (wraps). */
531534
private async cycleThinkingEffort(): Promise<void> {
535+
if (this.thinkingCycleInFlight) return;
532536
const { host } = this;
533537
if (host.state.appState.streamingPhase !== 'idle' || host.state.appState.isCompacting) {
534538
host.showError('Cannot change thinking effort while streaming — press Esc or Ctrl-C first.');
@@ -549,32 +553,37 @@ export class EditorKeyboardController {
549553
host.showNotice(`${alias} does not offer selectable thinking effort levels.`);
550554
return;
551555
}
552-
const prev = host.state.appState.thinkingEffort;
553-
const currentIndex = levels.indexOf(prev);
554-
// An out-of-list live effort (e.g. a provider-specific value) restarts the
555-
// cycle from the off entry when offered, else from the first level.
556-
const startIndex = currentIndex !== -1 ? currentIndex + 1 : Math.max(0, levels.indexOf('off'));
557-
const next = levels[startIndex % levels.length] ?? levels[0]!;
558-
if (host.session !== undefined) {
559-
try {
560-
await host.session.setThinking(next);
561-
} catch (error) {
562-
host.showError(`Failed to set thinking effort: ${formatErrorMessage(error)}`);
556+
this.thinkingCycleInFlight = true;
557+
try {
558+
const prev = host.state.appState.thinkingEffort;
559+
const currentIndex = levels.indexOf(prev);
560+
// An out-of-list live effort (e.g. a provider-specific value) restarts the
561+
// cycle from the off entry when offered, else from the first level.
562+
const startIndex = currentIndex !== -1 ? currentIndex + 1 : Math.max(0, levels.indexOf('off'));
563+
const next = levels[startIndex % levels.length] ?? levels[0]!;
564+
if (host.session !== undefined) {
565+
try {
566+
await host.session.setThinking(next);
567+
} catch (error) {
568+
host.showError(`Failed to set thinking effort: ${formatErrorMessage(error)}`);
569+
return;
570+
}
571+
} else if (!host.engineV2) {
572+
host.showError(NO_ACTIVE_SESSION_MESSAGE);
563573
return;
564574
}
565-
} else if (!host.engineV2) {
566-
host.showError(NO_ACTIVE_SESSION_MESSAGE);
567-
return;
575+
// v2 session-less: carry the choice into the first lazy-created session,
576+
// the same way a session-only Alt+S choice is applied on creation.
577+
const patch: Partial<AppState> = { thinkingEffort: next };
578+
if (host.session === undefined) patch.lazySessionThinking = next;
579+
host.setAppState(patch);
580+
host.track('thinking_toggle', { enabled: next !== 'off', effort: next, from: prev });
581+
// No transcript notice: the footer already shows the new level live, and
582+
// rapid cycling would stack a line per keypress in the chat history.
583+
await this.persistDefaultEffort(alias, model, next);
584+
} finally {
585+
this.thinkingCycleInFlight = false;
568586
}
569-
// v2 session-less: carry the choice into the first lazy-created session,
570-
// the same way a session-only Alt+S choice is applied on creation.
571-
const patch: Partial<AppState> = { thinkingEffort: next };
572-
if (host.session === undefined) patch.lazySessionThinking = next;
573-
host.setAppState(patch);
574-
host.track('thinking_toggle', { enabled: next !== 'off', effort: next, from: prev });
575-
// No transcript notice: the footer already shows the new level live, and
576-
// rapid cycling would stack a line per keypress in the chat history.
577-
await this.persistDefaultEffort(alias, model, next);
578587
}
579588

580589
/** Best-effort persist of the cycled effort as the config default. */

apps/pythinker-code/src/tui/easter-eggs/hatch.ts

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
2-
* `/hatch` easter egg — everything it needs lives in this one file: the
3-
* rainbow text coloring, the animation state machine, and the command handler.
4-
* Removing the feature is "delete this file + its import sites".
2+
* `/hatch` easter egg — the rainbow text coloring, the animation state
3+
* machine, and the command handler. Removing the feature is "delete this
4+
* file, `constant/hatch.ts`, and the import sites".
55
*
66
* It is deliberately NOT registered in BUILTIN_SLASH_COMMANDS, so it stays out
77
* of `/help` and autocomplete; `executeSlashCommand` calls the handler as a
@@ -13,13 +13,9 @@ import chalk from 'chalk';
1313

1414
import type { SlashCommandHost } from '../commands/dispatch';
1515
import type { ParsedSlashInput } from '../commands/types';
16+
import { HATCH_FLOW_MS, HATCH_FRAME_MS } from '../constant/hatch';
1617
import { currentTheme } from '../theme';
1718

18-
/** Frame interval for the rainbow flow animation. */
19-
export const HATCH_FRAME_MS = 110;
20-
/** How long the rainbow flows before settling (fading out, or freezing). */
21-
export const HATCH_FLOW_MS = 3000;
22-
2319
const DARK_RAINBOW = [
2420
'#4FA8FF',
2521
'#5BC0BE',

apps/pythinker-code/src/utils/usage/debug-timing.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,12 @@ export function computeDecodeTps(
4646
outputTokens: number | undefined,
4747
streamMs: number | undefined,
4848
): number | null {
49-
if (outputTokens === undefined || outputTokens <= 0) return null;
50-
if (streamMs === undefined || streamMs < MIN_STREAM_MS_FOR_TPS) return null;
49+
if (outputTokens === undefined || !Number.isFinite(outputTokens) || outputTokens <= 0) {
50+
return null;
51+
}
52+
if (streamMs === undefined || !Number.isFinite(streamMs) || streamMs < MIN_STREAM_MS_FOR_TPS) {
53+
return null;
54+
}
5155
return outputTokens / (streamMs / 1000);
5256
}
5357

apps/pythinker-code/test/tui/controllers/editor-keyboard.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,25 @@ describe('EditorKeyboardController Shift-Tab effort cycle', () => {
490490
});
491491
});
492492

493+
it('ignores shift-tab pressed again while a thinking update is in flight', async () => {
494+
const h = createEffortHarness({ supportEfforts: ['low', 'high', 'max'] });
495+
let release!: () => void;
496+
const gate = new Promise<void>((resolve) => {
497+
release = resolve;
498+
});
499+
h.setThinking.mockImplementation(() => gate);
500+
501+
h.onShiftTab();
502+
await new Promise((resolve) => setImmediate(resolve));
503+
h.onShiftTab();
504+
release();
505+
await settle();
506+
507+
expect(h.setThinking).toHaveBeenCalledTimes(1);
508+
expect(h.setThinking).toHaveBeenCalledWith('low');
509+
expect(h.statePatches.at(-1)?.['thinkingEffort']).toBe('low');
510+
});
511+
493512
it('refuses to cycle while a turn is streaming', async () => {
494513
const h = createEffortHarness({
495514
supportEfforts: ['low', 'high'],

apps/pythinker-code/test/tui/easter-eggs/hatch.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,14 @@ import chalk from 'chalk';
22
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
33

44
import {
5-
HATCH_FLOW_MS,
6-
HATCH_FRAME_MS,
75
getRainbowHatchView,
86
installRainbowHatch,
97
RainbowHatch,
108
rainbowText,
119
setRainbowHatch,
1210
tryHandleHatchCommand,
1311
} from '#/tui/easter-eggs/hatch';
12+
import { HATCH_FLOW_MS, HATCH_FRAME_MS } from '#/tui/constant/hatch';
1413
import type { SlashCommandHost } from '#/tui/commands/dispatch';
1514
import { darkColors } from '#/tui/theme/colors';
1615

apps/pythinker-code/test/utils/usage/debug-timing.test.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { describe, expect, it } from 'vitest';
22

3-
import { formatStepDebugTiming } from '#/utils/usage/debug-timing';
3+
import { computeDecodeTps, formatStepDebugTiming } from '#/utils/usage/debug-timing';
44

55
describe('formatStepDebugTiming', () => {
66
it('returns undefined when timing fields are missing', () => {
@@ -145,3 +145,16 @@ describe('formatStepDebugTiming', () => {
145145
expect(result).toContain('10.0s');
146146
});
147147
});
148+
149+
describe('computeDecodeTps', () => {
150+
it('rejects non-finite inputs', () => {
151+
expect(computeDecodeTps(Number.NaN, 1000)).toBeNull();
152+
expect(computeDecodeTps(Number.POSITIVE_INFINITY, 1000)).toBeNull();
153+
expect(computeDecodeTps(200, Number.NaN)).toBeNull();
154+
expect(computeDecodeTps(200, Number.POSITIVE_INFINITY)).toBeNull();
155+
});
156+
157+
it('computes the ratio for finite values', () => {
158+
expect(computeDecodeTps(200, 5000)).toBeCloseTo(40);
159+
});
160+
});

apps/pythinker-web/src/components/chat/ToolRow.vue

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
<!-- apps/pythinker-web/src/components/chat/ToolRow.vue -->
22
<script setup lang="ts">
33
import { inject, nextTick, ref } from 'vue';
4+
import type { ToolStatus } from '../../types';
45
import Icon from '../ui/Icon.vue';
56
import Tooltip from '../ui/Tooltip.vue';
67
import StatusDot from '../ui/StatusDot.vue';
78
89
withDefaults(
910
defineProps<{
10-
status: 'running' | 'ok' | 'error' | 'suspended';
11+
status: ToolStatus;
1112
/** Inline-SVG glyph string (toolGlyph), or empty for none. */
1213
icon?: string;
1314
name: string;

apps/pythinker-web/src/components/chat/tool-calls/EditTool.vue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ const emit = defineEmits<{
3636
3737
const { t } = useI18n();
3838
39-
const status = computed<'running' | 'ok' | 'error'>(() => props.tool.status as 'running' | 'ok' | 'error');
39+
const status = computed(() => props.tool.status);
4040
const label = computed(() => toolLabel(props.tool.name));
4141
const glyph = computed(() => toolGlyph(props.tool.name));
4242
const isWrite = computed(() => normalizeToolName(props.tool.name) === 'write');

apps/pythinker-web/src/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ export interface WorkspaceGroup {
8787
/** Sidebar session-list scope: only the active workspace, or all workspaces. */
8888
export type WorkspaceScope = 'current' | 'all';
8989

90-
export type ToolStatus = 'ok' | 'running' | 'error';
90+
export type ToolStatus = 'ok' | 'running' | 'error' | 'suspended';
9191

9292
export interface ToolCall {
9393
id: string;

0 commit comments

Comments
 (0)