Conversation
There was a problem hiding this comment.
🧪 PR Review is completed: Review of Release v5.4.1: Rebranding to Axon Code and Sticky User Messages.
Key findings:
- Critical logic flaw in sticky message scrolling for virtualized lists.
- Invalid Tailwind class in new component.
- Potential division by zero in settings.
- Incomplete state cleanup in reset script.
Skipped files
apps/kilocode-docs/.kilocode/rules/memory-bank/tech.md: Skipped file patternapps/kilocode-docs/docs/getting-started/installing.md: Skipped file patternapps/kilocode-docs/docs/seats/getting-started.md: Skipped file patternapps/kilocode-docs/i18n/zh-CN/docusaurus-plugin-content-docs/current/getting-started/installing.md: Skipped file patternapps/kilocode-docs/i18n/zh-CN/docusaurus-theme-classic/footer.json: Skipped file patternwebview-ui/src/i18n/locales/en/chat.json: Skipped file pattern
⬇️ Low Priority Suggestions (2)
webview-ui/src/components/kilocode/StickyUserMessage.tsx (1 suggestion)
Location:
webview-ui/src/components/kilocode/StickyUserMessage.tsx(Lines 67-70)🔵 CSS/Style
Issue:
ml-0.9is not a valid Tailwind CSS class (unless customized). Standard spacing scale uses 0.5, 1, 1.5, etc.Fix: Use
ml-1(4px) or an arbitrary value likeml-[3.5px]if precision is needed.Impact: Ensures consistent styling and avoids invalid class names.
- className={cn( - "px-2 py-2 flex flex-col gap-1 relative ml-0.9 mr-1", - "rounded-lg", - "border border-[var(--color-matterai-border)]", + className={cn( + "px-2 py-2 flex flex-col gap-1 relative ml-1 mr-1", + "rounded-lg", + "border border-[var(--color-matterai-border)]",
scripts/reset-kilocode-state.sh (1 suggestion)
Location:
scripts/reset-kilocode-state.sh(Lines 8-10)🟡 Configuration
Issue: The script deletes keys matching
workbench.view.extension.kilo-code%. With the rebranding toaxon-code, the view container ID likely changed toaxon-code. This script might miss cleaning up the new view state.Fix: Include the new view ID pattern in the DELETE statement.
Impact: Ensures complete state reset for the rebranded extension.
- key = 'matterai.axon-code' OR \ - key LIKE 'workbench.view.extension.kilo-code%' OR \ - key LIKE 'secret://{\"extensionId\":\"matterai.axon-code\",%';" + key = 'matterai.axon-code' OR \ + key LIKE 'workbench.view.extension.kilo-code%' OR \ + key LIKE 'workbench.view.extension.axon-code%' OR \ + key LIKE 'secret://{\"extensionId\":\"matterai.axon-code\",%';"
| const stickyHeight = stickyHeaderRef.current?.offsetHeight ?? 40 | ||
| const scrollerRect = scroller.getBoundingClientRect() | ||
| const threshold = scrollerRect.top + stickyHeight | ||
|
|
||
| let bestIndex: number | null = null | ||
| let passedRenderedRegion = false | ||
|
|
||
| for (const idx of userFeedbackIndices) { | ||
| const el = scroller.querySelector(`[data-item-index="${idx}"]`) as HTMLElement | null | ||
|
|
||
| if (!el) { | ||
| if (passedRenderedRegion) { | ||
| // Past the rendered region — below viewport, stop | ||
| break | ||
| } | ||
| // Not yet in rendered region — above viewport (scrolled past) | ||
| bestIndex = idx | ||
| continue | ||
| } | ||
|
|
||
| passedRenderedRegion = true | ||
| const elTop = el.getBoundingClientRect().top | ||
| if (elTop <= threshold) { | ||
| bestIndex = idx | ||
| } else { | ||
| // Below the threshold, everything after will be too | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
🟠 Logic Error
Issue: The sticky header logic assumes that if an element is not found in the DOM (!el), it must be above the viewport. In a virtualized list (Virtuoso), items below the viewport are also not in the DOM. This causes the sticky header to incorrectly show future messages when at the top of the chat if the first user message is outside the initial render window.
Fix: Determine the first rendered index by querying the DOM once, then use it to decide if a missing element is above or below the viewport.
Impact: Prevents sticky header from showing incorrect future messages.
| const stickyHeight = stickyHeaderRef.current?.offsetHeight ?? 40 | |
| const scrollerRect = scroller.getBoundingClientRect() | |
| const threshold = scrollerRect.top + stickyHeight | |
| let bestIndex: number | null = null | |
| let passedRenderedRegion = false | |
| for (const idx of userFeedbackIndices) { | |
| const el = scroller.querySelector(`[data-item-index="${idx}"]`) as HTMLElement | null | |
| if (!el) { | |
| if (passedRenderedRegion) { | |
| // Past the rendered region — below viewport, stop | |
| break | |
| } | |
| // Not yet in rendered region — above viewport (scrolled past) | |
| bestIndex = idx | |
| continue | |
| } | |
| passedRenderedRegion = true | |
| const elTop = el.getBoundingClientRect().top | |
| if (elTop <= threshold) { | |
| bestIndex = idx | |
| } else { | |
| // Below the threshold, everything after will be too | |
| break | |
| } | |
| } | |
| const stickyHeight = stickyHeaderRef.current?.offsetHeight ?? 40 | |
| const scrollerRect = scroller.getBoundingClientRect() | |
| const threshold = scrollerRect.top + stickyHeight | |
| // Find the first rendered item to determine our position relative to the virtual list | |
| const firstRenderedEl = scroller.querySelector('[data-item-index]') | |
| const firstRenderedIndex = firstRenderedEl ? parseInt(firstRenderedEl.getAttribute('data-item-index') || "0", 10) : 0 | |
| let bestIndex: number | null = null | |
| for (const idx of userFeedbackIndices) { | |
| const el = scroller.querySelector(`[data-item-index="${idx}"]`) as HTMLElement | null | |
| if (!el) { | |
| if (idx < firstRenderedIndex) { | |
| // Not in DOM and index is lower than first rendered -> Above viewport | |
| bestIndex = idx | |
| continue | |
| } else { | |
| // Not in DOM and index is higher -> Below viewport | |
| break | |
| } | |
| } | |
| const elTop = el.getBoundingClientRect().top | |
| if (elTop <= threshold) { | |
| bestIndex = idx | |
| } else { | |
| // Below the threshold, everything after will be too | |
| break | |
| } | |
| } |
| const usagePercentage = | ||
| profileData?.usagePercentage !== undefined | ||
| ? profileData.usagePercentage | ||
| : profileData?.usedCredits !== undefined && profileData?.totalCredits !== undefined | ||
| ? (profileData.usedCredits / profileData.totalCredits) * 100 | ||
| : null |
There was a problem hiding this comment.
🟡 Robustness
Issue: Potential division by zero if profileData.totalCredits is 0, which could result in NaN or Infinity displayed in the UI.
Fix: Add a check to ensure totalCredits is greater than 0 before dividing.
Impact: Prevents UI display errors.
| const usagePercentage = | |
| profileData?.usagePercentage !== undefined | |
| ? profileData.usagePercentage | |
| : profileData?.usedCredits !== undefined && profileData?.totalCredits !== undefined | |
| ? (profileData.usedCredits / profileData.totalCredits) * 100 | |
| : null | |
| const usagePercentage = | |
| profileData?.usagePercentage !== undefined | |
| ? profileData.usagePercentage | |
| : profileData?.usedCredits !== undefined && (profileData?.totalCredits || 0) > 0 | |
| ? (profileData.usedCredits / profileData.totalCredits) * 100 | |
| : null |
- ChatView.tsx: Fix sticky header logic for virtualized list by checking first rendered index to correctly determine if missing elements are above or below viewport - KiloCode.tsx: Add division-by-zero guard for usagePercentage calculation
|
✅ Reviewed the changes: The changes correctly address the virtualization logic in ChatView.tsx and the division by zero issue in KiloCode.tsx. The implementation is robust and follows the previous recommendations. |
Summary
This release includes several UI improvements, bug fixes, and feature enhancements.
Changes
Files Changed
Testing
Checklist