-
Notifications
You must be signed in to change notification settings - Fork 625
fix: think content better animation #992
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: think content better animation #992
Conversation
WalkthroughAdds a timer-driven elapsed-seconds display and reactive watchers to MessageBlockThink for live reasoning-time updates and cleanup; updates ThinkContent to compute and bind a typed shimmerStyle, adjust animation classes, and switch to a named props constant. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as MessageBlockThink.vue
participant Timer as Timer Scheduler
participant Data as reasoning_time / status
Note over UI,Data: Mount / initialization
UI->>Data: read status, reasoning_time.start (or usage timing)
alt status == "loading" and start available
UI->>Timer: scheduleNextUpdate(UPDATE_INTERVAL, UPDATE_OFFSET)
loop while status == "loading"
Timer-->>UI: tick -> updateDisplayedSeconds()
UI->>UI: headerText uses displayedSeconds
end
else status != "loading"
UI->>Timer: stopTimer()
UI->>UI: displayedSeconds set from final reasoningDuration (formatted)
end
Note over UI: Reactive watches
Data-->>UI: status or reasoning_time.start changed
UI->>Timer: start/stop and refresh display accordingly
Note over UI: Unmount
UI->>Timer: stopTimer()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/renderer/src/components/message/MessageBlockThink.vue (2)
43-43: Translate comment to English.As per coding guidelines, all comments must be in English.
Apply this diff:
- // 保留小数点后最多两位,去除尾随的0 + // Keep at most two decimal places, removing trailing zerosAs per coding guidelines.
1-125: Fix Prettier formatting.The pipeline shows a Prettier formatting check failure. Run
pnpm prettier --write src/renderer/src/components/message/MessageBlockThink.vueto fix the formatting issues.
🧹 Nitpick comments (2)
src/renderer/src/components/message/MessageBlockThink.vue (2)
32-33: Useconstfor constant values.
UPDATE_INTERVALandUPDATE_OFFSETare constant values and should be declared withconstinstead of plain assignment.Apply this diff:
-const UPDATE_INTERVAL = 1000 -const UPDATE_OFFSET = 80 +const UPDATE_INTERVAL = 1000 as const +const UPDATE_OFFSET = 80 as const
62-80: Timer scheduling logic looks correct.The calculation aligns updates to 1-second boundaries with an 80ms offset, achieving the synchronized cadence mentioned in the PR objectives. The early return on line 64 prevents scheduling when not needed.
Minor simplification: Line 74's
Math.max(..., 0)is unnecessary sinceUPDATE_INTERVAL - remainderis always >= 0 whenremainderis in range [0, UPDATE_INTERVAL).- const delay = Math.max(UPDATE_INTERVAL - remainder, 0) + UPDATE_OFFSET + const delay = UPDATE_INTERVAL - remainder + UPDATE_OFFSET
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/renderer/src/components/message/MessageBlockThink.vue(4 hunks)src/renderer/src/components/think-content/ThinkContent.vue(2 hunks)
🧰 Additional context used
📓 Path-based instructions (15)
src/renderer/src/**/*
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
src/renderer/src/**/*: All user-facing strings must use i18n keys (avoid hardcoded user-visible text in code)
Use the 'vue-i18n' framework for all internationalization in the renderer
Ensure all user-visible text in the renderer uses the translation system
Files:
src/renderer/src/components/think-content/ThinkContent.vuesrc/renderer/src/components/message/MessageBlockThink.vue
src/renderer/**/*.{vue,ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
渲染进程代码放在
src/renderer
Files:
src/renderer/src/components/think-content/ThinkContent.vuesrc/renderer/src/components/message/MessageBlockThink.vue
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/vue-best-practices.mdc)
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}: Use the Composition API for better code organization and reusability
Implement proper state management with Pinia
Utilize Vue Router for navigation and route management
Leverage Vue's built-in reactivity system for efficient data handling
Files:
src/renderer/src/components/think-content/ThinkContent.vuesrc/renderer/src/components/message/MessageBlockThink.vue
src/renderer/src/**/*.vue
📄 CodeRabbit inference engine (.cursor/rules/vue-best-practices.mdc)
Use scoped styles to prevent CSS conflicts between components
Files:
src/renderer/src/components/think-content/ThinkContent.vuesrc/renderer/src/components/message/MessageBlockThink.vue
src/renderer/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,tsx,vue}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Use TypeScript for all code; prefer types over interfaces.
Avoid enums; use const objects instead.
Use arrow functions for methods and computed properties.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Files:
src/renderer/src/components/think-content/ThinkContent.vuesrc/renderer/src/components/message/MessageBlockThink.vue
src/renderer/**/*.{vue,ts}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
Implement lazy loading for routes and components.
Files:
src/renderer/src/components/think-content/ThinkContent.vuesrc/renderer/src/components/message/MessageBlockThink.vue
src/renderer/**/*.{ts,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,vue}: Use useFetch and useAsyncData for data fetching.
Implement SEO best practices using Nuxt's useHead and useSeoMeta.Use Pinia for frontend state management (do not introduce alternative state libraries)
Files:
src/renderer/src/components/think-content/ThinkContent.vuesrc/renderer/src/components/message/MessageBlockThink.vue
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/renderer/src/components/think-content/ThinkContent.vuesrc/renderer/src/components/message/MessageBlockThink.vue
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Files:
src/renderer/src/components/think-content/ThinkContent.vuesrc/renderer/src/components/message/MessageBlockThink.vue
src/renderer/{src,shell,floating}/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/renderer/{src,shell,floating}/**/*.vue: Use Vue 3 Composition API for all components
All user-facing strings must use i18n keys via vue-i18n (no hard-coded UI strings)
Use Tailwind CSS utilities and ensure styles are scoped in Vue components
Files:
src/renderer/src/components/think-content/ThinkContent.vuesrc/renderer/src/components/message/MessageBlockThink.vue
src/renderer/src/components/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Organize UI components by feature within src/renderer/src/
Files:
src/renderer/src/components/think-content/ThinkContent.vuesrc/renderer/src/components/message/MessageBlockThink.vue
src/renderer/src/**
📄 CodeRabbit inference engine (AGENTS.md)
Put application code for the Vue app under src/renderer/src (components, stores, views, i18n, lib)
Files:
src/renderer/src/components/think-content/ThinkContent.vuesrc/renderer/src/components/message/MessageBlockThink.vue
src/renderer/src/**/*.{vue,ts}
📄 CodeRabbit inference engine (AGENTS.md)
All user-facing strings in the renderer must use vue-i18n keys defined in src/renderer/src/i18n
Files:
src/renderer/src/components/think-content/ThinkContent.vuesrc/renderer/src/components/message/MessageBlockThink.vue
**/*.{js,jsx,ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting: single quotes, no semicolons, max width 100
Files:
src/renderer/src/components/think-content/ThinkContent.vuesrc/renderer/src/components/message/MessageBlockThink.vue
src/renderer/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Name Vue components in PascalCase (e.g., ChatInput.vue)
Files:
src/renderer/src/components/think-content/ThinkContent.vuesrc/renderer/src/components/message/MessageBlockThink.vue
🪛 GitHub Actions: PR Check
src/renderer/src/components/message/MessageBlockThink.vue
[error] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues in this file.
🔇 Additional comments (8)
src/renderer/src/components/message/MessageBlockThink.vue (5)
47-53: LGTM!The defensive handling of non-finite values and the use of
Math.floorfor integer seconds is correct.
82-87: LGTM!Using
displayedSecondsinstead of the rawreasoningDurationsuccessfully throttles the header text updates to the 1-second cadence.
96-107: LGTM!The watcher correctly manages the timer lifecycle: updating the display immediately, then starting the timer when loading or stopping it when complete. The
immediate: trueoption ensures proper initialization.
109-116: LGTM!This watcher ensures the final reasoning duration is displayed after the thinking phase completes, without interfering with the timer updates during the loading state.
122-124: LGTM!Proper cleanup prevents timer leaks when the component is unmounted.
src/renderer/src/components/think-content/ThinkContent.vue (3)
20-20: LGTM!Explicitly setting the pulse animation duration to
1ssynchronizes the icon animations with the one-second update cadence, preventing flicker as mentioned in the PR objectives.Also applies to: 46-46
60-65: LGTM!Extracting
definePropsinto a named constant follows Vue 3 best practices and enables type-safe access to props within theshimmerStylecomputed property.
10-16:loading-shimmerclass and CSS variable verified
The.loading-shimmerclass insrc/renderer/src/assets/style.cssusesanimation: loading-shimmer var(--cot-shimmer-duration, 1.6s)…, so theshimmerStylebinding works as intended.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/components/message/MessageBlockThink.vue (1)
36-45: Replace Chinese comment with English.The comment at line 43 is in Chinese, which violates the coding guideline: "Use English for all logs and comments."
As per coding guidelines.
Apply this diff:
} - // 保留小数点后最多两位,去除尾随的0 + // Keep at most two decimal places, remove trailing zeros return parseFloat(duration.toFixed(2)) })
🧹 Nitpick comments (1)
src/renderer/src/components/message/MessageBlockThink.vue (1)
31-34: Document the UPDATE_OFFSET constant.The
UPDATE_OFFSETvalue of 80ms lacks explanation. Please add a brief comment explaining why this specific offset is needed (e.g., to ensure updates occur slightly after second boundaries, accounting for processing delays, preventing edge-case timing issues, etc.).Example:
const displayedSeconds = ref(0) const UPDATE_INTERVAL = 1000 +// 80ms offset ensures updates happen slightly after second boundaries +// to account for processing delays and maintain stable synchronization const UPDATE_OFFSET = 80 let updateTimer: ReturnType<typeof setTimeout> | null = null
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/renderer/src/components/message/MessageBlockThink.vue(4 hunks)
🧰 Additional context used
📓 Path-based instructions (15)
src/renderer/src/**/*
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
src/renderer/src/**/*: All user-facing strings must use i18n keys (avoid hardcoded user-visible text in code)
Use the 'vue-i18n' framework for all internationalization in the renderer
Ensure all user-visible text in the renderer uses the translation system
Files:
src/renderer/src/components/message/MessageBlockThink.vue
src/renderer/**/*.{vue,ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
渲染进程代码放在
src/renderer
Files:
src/renderer/src/components/message/MessageBlockThink.vue
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/vue-best-practices.mdc)
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}: Use the Composition API for better code organization and reusability
Implement proper state management with Pinia
Utilize Vue Router for navigation and route management
Leverage Vue's built-in reactivity system for efficient data handling
Files:
src/renderer/src/components/message/MessageBlockThink.vue
src/renderer/src/**/*.vue
📄 CodeRabbit inference engine (.cursor/rules/vue-best-practices.mdc)
Use scoped styles to prevent CSS conflicts between components
Files:
src/renderer/src/components/message/MessageBlockThink.vue
src/renderer/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,tsx,vue}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Use TypeScript for all code; prefer types over interfaces.
Avoid enums; use const objects instead.
Use arrow functions for methods and computed properties.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Files:
src/renderer/src/components/message/MessageBlockThink.vue
src/renderer/**/*.{vue,ts}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
Implement lazy loading for routes and components.
Files:
src/renderer/src/components/message/MessageBlockThink.vue
src/renderer/**/*.{ts,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,vue}: Use useFetch and useAsyncData for data fetching.
Implement SEO best practices using Nuxt's useHead and useSeoMeta.Use Pinia for frontend state management (do not introduce alternative state libraries)
Files:
src/renderer/src/components/message/MessageBlockThink.vue
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/renderer/src/components/message/MessageBlockThink.vue
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Files:
src/renderer/src/components/message/MessageBlockThink.vue
src/renderer/{src,shell,floating}/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/renderer/{src,shell,floating}/**/*.vue: Use Vue 3 Composition API for all components
All user-facing strings must use i18n keys via vue-i18n (no hard-coded UI strings)
Use Tailwind CSS utilities and ensure styles are scoped in Vue components
Files:
src/renderer/src/components/message/MessageBlockThink.vue
src/renderer/src/components/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Organize UI components by feature within src/renderer/src/
Files:
src/renderer/src/components/message/MessageBlockThink.vue
src/renderer/src/**
📄 CodeRabbit inference engine (AGENTS.md)
Put application code for the Vue app under src/renderer/src (components, stores, views, i18n, lib)
Files:
src/renderer/src/components/message/MessageBlockThink.vue
src/renderer/src/**/*.{vue,ts}
📄 CodeRabbit inference engine (AGENTS.md)
All user-facing strings in the renderer must use vue-i18n keys defined in src/renderer/src/i18n
Files:
src/renderer/src/components/message/MessageBlockThink.vue
**/*.{js,jsx,ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting: single quotes, no semicolons, max width 100
Files:
src/renderer/src/components/message/MessageBlockThink.vue
src/renderer/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Name Vue components in PascalCase (e.g., ChatInput.vue)
Files:
src/renderer/src/components/message/MessageBlockThink.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-check (x64)
🔇 Additional comments (8)
src/renderer/src/components/message/MessageBlockThink.vue (8)
14-14: LGTM: Import addition is appropriate.The addition of
onBeforeUnmountto the imports is correct and necessary for the cleanup logic introduced later in the component.
47-51: LGTM: Defensive normalization logic is solid.The
updateDisplayedSecondsfunction correctly handles edge cases withNumber.isFinitechecks and ensures non-negative integer values.
53-58: LGTM: Proper timer cleanup.The
stopTimerfunction correctly clears the timeout and resets the reference, preventing memory leaks.
60-77: Verify the timing synchronization logic.The
scheduleNextUpdatefunction implements complex timing logic to synchronize updates to second boundaries. The delay calculation at line 71 could result in delays as short asUPDATE_OFFSET(80ms) when called very close to a second boundary (e.g., ifremainderis 999ms, delay becomes 81ms).While this appears intentional for synchronization purposes, please verify:
- That rapid updates (< 1 second apart) in edge cases are acceptable
- The reasoning behind the 80ms offset value
- Whether additional documentation should be added to explain the synchronization strategy
Consider adding inline comments to clarify the timing logic:
const scheduleNextUpdate = () => { stopTimer() if (props.block.status !== 'loading') return const fallbackDuration = Number.isFinite(reasoningDuration.value) ? reasoningDuration.value * 1000 : 0 const startTimestamp = props.block.reasoning_time?.start ?? Date.now() - fallbackDuration const now = Date.now() const elapsed = Math.max(0, now - startTimestamp) // Calculate position within current second (0-999ms) const remainder = elapsed % UPDATE_INTERVAL // Schedule update at next second boundary + offset for synchronization const delay = Math.max(UPDATE_INTERVAL - remainder, 0) + UPDATE_OFFSET updateTimer = setTimeout(() => { updateDisplayedSeconds() scheduleNextUpdate() }, delay) }
79-84: LGTM: Correctly uses throttled display value.The
headerTextcomputed property now correctly referencesdisplayedSeconds.valueinstead ofreasoningDuration.value, achieving the throttling goal mentioned in the PR objectives. The i18n key usage is also correct per coding guidelines.
93-104: LGTM: Reactive timer management is well-structured.The watcher correctly handles status changes and reasoning_time updates, starting/stopping the timer as needed. The
immediate: trueoption ensures proper initialization on mount.
106-113: LGTM: Conditional update logic is appropriate.The watcher correctly updates the display only when not in loading state, avoiding interference with the timer-driven updates during active reasoning.
119-121: LGTM: Proper lifecycle cleanup.The
onBeforeUnmounthook correctly cleans up the timer, preventing potential memory leaks.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68ecaa79d3a8832c9ec6ff95730e57ea
Summary by CodeRabbit
New Features
Style