🐛 Fix Kube Chess AI freeze and Kube Craft block selection#4110
🐛 Fix Kube Chess AI freeze and Kube Craft block selection#4110clubanderson merged 2 commits intomainfrom
Conversation
Reverts #4103 (active user count in navbar) — feature explicitly rejected by maintainer. Also disables the Auto-QA check that keeps filing issues about this feature. Signed-off-by: Andrew Anderson <andy@clubanderson.com>
KubeChess (#4083): AI gets stuck on "AI thinking..." indefinitely because the minimax search can explore too many positions, blocking the main thread. Fix adds a position evaluation counter (MAX_POSITIONS_EVALUATED = 50,000) that bails out when exceeded, move ordering for better alpha-beta pruning, a ref-based thinking guard to prevent effect re-triggering loops, and try/finally to ensure thinking state always resets even on errors. KubeCraft (#4081): Block selection state not reflected during placement because mouse handlers used stale refs via useCallback with empty deps. Fix replaces the ref-based indirection with proper useCallback dependencies on isErasing and selectedBlock state, and adds canvas coordinate scaling to handle CSS-scaled canvas in expanded mode. Fixes #4083 Fixes #4081 Signed-off-by: Andrew Anderson <andy@clubanderson.com>
kubestellar-prow
bot
added
the
dco-signoff: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
kubestellar-prow
bot
added
the
size/L
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
Fixes two Arcade game regressions in the Console frontend: Kube Chess AI hanging on “AI thinking…” and Kube Craft placing the wrong block due to stale handler state / incorrect coordinate mapping in expanded mode. Also updates Auto-QA workflow behavior around the “active user count” feature check.
Changes:
- Kube Craft: reworks mouse handling to use stateful callbacks (no stale refs) and adds coordinate scaling for CSS-scaled canvas.
- Kube Chess: adds a position-evaluation cap + move ordering to reduce minimax cost, and hardens “thinking” state transitions with a ref guard and try/finally.
- Auto-QA: disables the “active user count feature” check step.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| web/src/components/cards/KubeCraft.tsx | Replaces ref-indirection with dependency-correct callbacks; adds grid coordinate conversion for scaled canvas. |
| web/src/components/cards/KubeChess.tsx | Adds search limiting + move ordering and prevents AI “thinking” loop / stuck state via ref guard + try/finally. |
| .github/workflows/auto-qa.yml | Disables an Auto-QA feature-check step for active user count. |
| // Scale mouse coordinates to canvas logical size (handles CSS scaling in expanded mode) | ||
| const scaleX = CANVAS_WIDTH / rect.width | ||
| const scaleY = CANVAS_HEIGHT / rect.height | ||
| const x = Math.floor((e.clientX - rect.left) * scaleX / CELL_SIZE) | ||
| const y = Math.floor((e.clientY - rect.top) * scaleY / CELL_SIZE) |
There was a problem hiding this comment.
getGridCoords() divides by rect.width/rect.height; if either is 0 (or results in NaN), the bounds check won’t catch it (NaN comparisons are false) and placeBlock() can index world with NaN, throwing at runtime. Add a guard for rect.width/height <= 0 (and/or Number.isFinite(x/y)) before returning coords.
| // Scale mouse coordinates to canvas logical size (handles CSS scaling in expanded mode) | |
| const scaleX = CANVAS_WIDTH / rect.width | |
| const scaleY = CANVAS_HEIGHT / rect.height | |
| const x = Math.floor((e.clientX - rect.left) * scaleX / CELL_SIZE) | |
| const y = Math.floor((e.clientY - rect.top) * scaleY / CELL_SIZE) | |
| // If the canvas has no size, we cannot reliably map mouse coordinates | |
| if (rect.width <= 0 || rect.height <= 0) { | |
| return null | |
| } | |
| // Scale mouse coordinates to canvas logical size (handles CSS scaling in expanded mode) | |
| const scaleX = CANVAS_WIDTH / rect.width | |
| const scaleY = CANVAS_HEIGHT / rect.height | |
| if (!Number.isFinite(scaleX) || !Number.isFinite(scaleY)) { | |
| return null | |
| } | |
| const rawX = (e.clientX - rect.left) * scaleX / CELL_SIZE | |
| const rawY = (e.clientY - rect.top) * scaleY / CELL_SIZE | |
| if (!Number.isFinite(rawX) || !Number.isFinite(rawY)) { | |
| return null | |
| } | |
| const x = Math.floor(rawX) | |
| const y = Math.floor(rawY) |
| // Scale mouse coordinates to canvas logical size (handles CSS scaling in expanded mode) | ||
| const scaleX = CANVAS_WIDTH / rect.width | ||
| const scaleY = CANVAS_HEIGHT / rect.height | ||
| const x = Math.floor((e.clientX - rect.left) * scaleX / CELL_SIZE) | ||
| const y = Math.floor((e.clientY - rect.top) * scaleY / CELL_SIZE) | ||
|
|
There was a problem hiding this comment.
Coordinate scaling here assumes the canvas content is stretched to fill getBoundingClientRect() (independent scaleX/scaleY). In expanded mode the canvas uses objectFit: 'contain', which can introduce letterboxing when the container isn’t square; clicks in padded areas (or near edges) will map to the wrong grid cell. Consider computing a uniform scale (min of x/y), plus x/y offsets for the contained draw area, and ignore clicks in the padding.
| // Scale mouse coordinates to canvas logical size (handles CSS scaling in expanded mode) | |
| const scaleX = CANVAS_WIDTH / rect.width | |
| const scaleY = CANVAS_HEIGHT / rect.height | |
| const x = Math.floor((e.clientX - rect.left) * scaleX / CELL_SIZE) | |
| const y = Math.floor((e.clientY - rect.top) * scaleY / CELL_SIZE) | |
| const canvasAspect = CANVAS_WIDTH / CANVAS_HEIGHT | |
| const rectAspect = rect.width / rect.height | |
| // Compute the actual drawn content area within the canvas rect when objectFit: 'contain' is used. | |
| let drawnWidth: number | |
| let drawnHeight: number | |
| let offsetX: number | |
| let offsetY: number | |
| if (rectAspect > canvasAspect) { | |
| // Limited by height: full height used, width letterboxed | |
| drawnHeight = rect.height | |
| drawnWidth = rect.height * canvasAspect | |
| offsetX = (rect.width - drawnWidth) / 2 | |
| offsetY = 0 | |
| } else { | |
| // Limited by width: full width used, height letterboxed | |
| drawnWidth = rect.width | |
| drawnHeight = rect.width / canvasAspect | |
| offsetX = 0 | |
| offsetY = (rect.height - drawnHeight) / 2 | |
| } | |
| const mouseX = e.clientX - rect.left - offsetX | |
| const mouseY = e.clientY - rect.top - offsetY | |
| // Ignore clicks in the letterboxed padding outside the drawn content | |
| if (mouseX < 0 || mouseY < 0 || mouseX > drawnWidth || mouseY > drawnHeight) { | |
| return null | |
| } | |
| // Uniform scale from drawn pixels to logical canvas pixels | |
| const scale = CANVAS_WIDTH / drawnWidth | |
| const logicalX = mouseX * scale | |
| const logicalY = mouseY * scale | |
| const x = Math.floor(logicalX / CELL_SIZE) | |
| const y = Math.floor(logicalY / CELL_SIZE) |
| counter.count++ | ||
|
|
||
| // Bail out if we've evaluated too many positions to prevent UI freeze | ||
| if (counter.count > MAX_POSITIONS_EVALUATED) { |
There was a problem hiding this comment.
The position limit check is slightly off-by-one: counter is incremented before the check, and both this guard and the root-loop guard in findBestMove() use > rather than >=. As a result the search can exceed MAX_POSITIONS_EVALUATED (and still do an extra minimax call from the root) instead of enforcing a hard cap. Use >= (and/or check before increment) consistently to keep the limit strict.
| if (counter.count > MAX_POSITIONS_EVALUATED) { | |
| if (counter.count >= MAX_POSITIONS_EVALUATED) { |
| # === ACTIVE USER COUNT FEATURE (every run) === | ||
| - name: "Check: Active user count feature" | ||
| - name: "Check: Active user count feature (PERMANENTLY DISABLED)" | ||
| id: focus_user_count | ||
| working-directory: web | ||
| continue-on-error: true | ||
| run: | | ||
| echo "Checking active user count feature implementation..." | ||
| ISSUES="" | ||
|
|
||
| # Find user count component/feature | ||
| USER_COUNT_FILES=$(find src -name "*user*count*" -o -name "*active*user*" -o -name "*UserCount*" -o -name "*ActiveUser*" 2>/dev/null | grep "\.tsx\|\.ts" || true) | ||
| USER_COUNT_HOOKS=$(grep -rln "activeUsers\|userCount\|onlineUsers\|connectedUsers" src/ --include="*.tsx" --include="*.ts" 2>/dev/null || true) | ||
|
|
||
| if [ -z "$USER_COUNT_FILES" ] && [ -z "$USER_COUNT_HOOKS" ]; then | ||
| ISSUES="${ISSUES}### No active user count feature found\nConsider adding a real-time user count for console.kubestellar.io\n\n" | ||
| else | ||
| # Check for WebSocket or real-time updates | ||
| HAS_REALTIME=$(grep -rln "WebSocket\|socket\|useSubscription\|onmessage\|EventSource\|sse" src/ --include="*.tsx" --include="*.ts" 2>/dev/null | head -1 || true) | ||
| if [ -z "$HAS_REALTIME" ]; then | ||
| ISSUES="${ISSUES}### User count may not update in real-time\nConsider using WebSocket for live user count updates\n\n" | ||
| fi | ||
|
|
||
| # Check for demo mode handling in user count | ||
| USER_COUNT_DEMO=$(grep -rln "activeUser\|userCount" src/ --include="*.tsx" 2>/dev/null | while read f; do | ||
| HAS_DEMO=$(grep -l "demo\|Demo\|mock\|Mock" "$f" 2>/dev/null || true) | ||
| if [ -z "$HAS_DEMO" ]; then | ||
| basename "$f" | ||
| fi | ||
| done | head -5 || true) | ||
|
|
||
| if [ -n "$USER_COUNT_DEMO" ]; then | ||
| ISSUES="${ISSUES}### User count components without demo mode handling\n\`\`\`\n${USER_COUNT_DEMO}\n\`\`\`\n\n" | ||
| fi | ||
|
|
||
| # Check for error handling in user count | ||
| USER_COUNT_ERROR=$(grep -rln "activeUser\|userCount" src/ --include="*.tsx" 2>/dev/null | while read f; do | ||
| HAS_ERROR=$(grep -l "error\|Error\|catch\|fallback" "$f" 2>/dev/null || true) | ||
| if [ -z "$HAS_ERROR" ]; then | ||
| basename "$f" | ||
| fi | ||
| done | head -5 || true) | ||
|
|
||
| if [ -n "$USER_COUNT_ERROR" ]; then | ||
| ISSUES="${ISSUES}### User count without error handling\nShow fallback when count unavailable:\n\`\`\`\n${USER_COUNT_ERROR}\n\`\`\`\n\n" | ||
| fi | ||
|
|
||
| # Check for proper cleanup (unsubscribe) | ||
| SUBSCRIBE_COUNT=$(grep -rc "subscribe\|addEventListener\|onmessage" src/ --include="*user*" --include="*User*" 2>/dev/null | awk -F: '{sum+=$2} END {print sum}' || echo "0") | ||
| UNSUBSCRIBE_COUNT=$(grep -rc "unsubscribe\|removeEventListener\|close()" src/ --include="*user*" --include="*User*" 2>/dev/null | awk -F: '{sum+=$2} END {print sum}' || echo "0") | ||
|
|
||
| if [ "$SUBSCRIBE_COUNT" -gt 0 ] && [ "$UNSUBSCRIBE_COUNT" -eq 0 ]; then | ||
| ISSUES="${ISSUES}### User count subscriptions without cleanup\nEnsure WebSocket/EventSource connections are closed on unmount\n\n" | ||
| fi | ||
|
|
||
| # Check if user count is visible in header/footer | ||
| HEADER_USER=$(grep -rln "activeUser\|userCount\|UserCount" src/components/layout --include="*.tsx" src/components/header --include="*.tsx" src/components/Header* 2>/dev/null | head -1 || true) | ||
| if [ -z "$HEADER_USER" ]; then | ||
| ISSUES="${ISSUES}### User count not in header/layout\nConsider showing active users in the app header for visibility\n\n" | ||
| fi | ||
| fi | ||
|
|
||
| if [ -n "$ISSUES" ]; then | ||
| echo "found=true" >> "$GITHUB_OUTPUT" | ||
| printf "%b" "$ISSUES" > /tmp/focus-user-count.txt | ||
| echo "User count issues found" | ||
| else | ||
| echo "found=false" >> "$GITHUB_OUTPUT" | ||
| echo "User count implementation looks good" | ||
| fi | ||
| # Active user count in navbar is explicitly not wanted by maintainer. | ||
| # This check is permanently disabled to prevent re-filing. | ||
| echo "found=false" >> "$GITHUB_OUTPUT" | ||
| echo "Skipped — feature explicitly rejected" |
There was a problem hiding this comment.
This workflow change permanently disables the “Active user count feature” check, but the PR description/test plan are exclusively about Kube Chess and Kube Craft. Either document this CI/workflow change in the PR description (with rationale/link), or move it to a separate PR to keep scope focused and avoid surprising reviewers.
🔄 Auto-Applying Copilot Code ReviewCopilot code review found 3 code suggestion(s) and 1 general comment(s). @copilot Please apply all of the following code review suggestions:
Also address these general comments:
Push all fixes in a single commit. Run Auto-generated by copilot-review-apply workflow. |
- KubeCraft: guard getGridCoords against zero-size bounding rect to prevent NaN - KubeCraft: account for objectFit:contain letterboxing in expanded mode so clicks in padded areas are correctly rejected instead of mapping to wrong tiles - KubeChess: fix off-by-one in minimax position limit (use >= instead of >) Signed-off-by: Andrew Anderson <andy@clubanderson.com>
- KubeCraft: guard getGridCoords against zero-size bounding rect to prevent NaN - KubeCraft: account for objectFit:contain letterboxing in expanded mode so clicks in padded areas are correctly rejected instead of mapping to wrong tiles - KubeChess: fix off-by-one in minimax position limit (use >= instead of >) Signed-off-by: Andrew Anderson <andy@clubanderson.com>
Summary
Kube Chess (Card: Kube Chess (kube_chess) #4083): AI gets stuck on "AI thinking..." indefinitely because the minimax search explores too many positions, blocking the main thread. Added position evaluation limit (50k positions), move ordering for better alpha-beta pruning, ref-based thinking guard to prevent effect re-triggering loops, and try/finally to ensure thinking state always resets.
Kube Craft (Card: Kube Craft (kube_craft) #4081): Block selection state not reflected during placement because mouse handlers used stale refs via
useCallbackwith empty dependency arrays. Replaced ref-based indirection with properuseCallbackdependencies onisErasingandselectedBlockstate, and added canvas coordinate scaling for expanded mode.Test plan
Fixes #4083
Fixes #4081