Fix board view crash when feature status lags during race conditions#825
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the stability and user experience of the board view by addressing critical rendering issues and improving how feature statuses are managed during rapid updates. It also refines the specification parsing logic to provide more comprehensive summaries, ensuring that important contextual information within subsections is not inadvertently truncated. These changes collectively lead to a more robust and accurate display of feature progress and details. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughServer spec summary extraction now preserves nested "###" subsections and truncates Summary to 500 characters; tests updated. UI: added Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a crash in the board view caused by race conditions and improves the handling of stale feature statuses. It also includes improvements to the specification parser. The changes involve removing duplicate cache invalidation, adding memoization to the KanbanBoard component, updating the board view row click handler, adding a menu item block in RowActions, and fixing the extractSummary() regex. The code changes look good and address the issues described in the pull request description.
| {!isCurrentAutoTask && | ||
| isRunningTask && | ||
| (feature.status === 'backlog' || | ||
| feature.status === 'ready' || | ||
| feature.status === 'interrupted' || | ||
| feature.status === 'merge_conflict') && ( |
There was a problem hiding this comment.
The condition feature.status === 'backlog' || feature.status === 'ready' || feature.status === 'interrupted' || feature.status === 'merge_conflict' is repeated from board-view.tsx. Consider creating a shared helper function to avoid duplication and ensure consistency.
const isBacklogLike = (status: string) => {
return status === 'backlog' || status === 'ready' || status === 'interrupted' || status === 'merge_conflict';
};
{!isCurrentAutoTask &&
isRunningTask &&
isBacklogLike(feature.status) && (
| // Use \n## [^#] to stop at same-level headers (## Foo) but NOT subsections (### Root Cause) | ||
| const sectionMatches = text.matchAll(/##\s*Summary\s*\n+([\s\S]*?)(?=\n## [^#]|\n\*\*|$)/gi); |
There was a problem hiding this comment.
The comment is helpful in explaining the purpose of the regex, but it could be improved by clarifying why stopping at same-level headers is important for the summary extraction. Consider adding a brief explanation of the problem this solves.
| const isBacklogLike = | ||
| feature.status === 'backlog' || | ||
| feature.status === 'merge_conflict' || | ||
| feature.status === 'ready' || | ||
| feature.status === 'interrupted'; |
There was a problem hiding this comment.
This isBacklogLike variable includes merge_conflict, ready, and interrupted statuses. It might be helpful to add a comment explaining why these statuses are considered 'backlog-like' in this context, especially since merge_conflict and interrupted might not be immediately obvious.
| {/* Running task with stale status (backlog/ready/interrupted but tracked as running). | ||
| These features are placed in the in_progress column by useBoardColumnFeatures | ||
| but their actual status hasn't updated yet, so no other menu block matches. */} |
There was a problem hiding this comment.
The comment explains the purpose of this code block, but it could be improved by adding a link to the relevant code or a more specific reference to the function useBoardColumnFeatures to help future developers quickly understand the context.
| // NOTE: auto_mode_feature_start and auto_mode_feature_complete are NOT handled here | ||
| // for feature list reloading. That is handled by useAutoModeQueryInvalidation which | ||
| // invalidates the features.all query on those events. Duplicate invalidation here | ||
| // caused a re-render cascade through DndContext that triggered React error #185 | ||
| // (maximum update depth exceeded), crashing the board view with an infinite spinner | ||
| // when a new feature was added and moved to in_progress. |
There was a problem hiding this comment.
The comment explains why auto_mode_feature_start is not handled here, but it could be improved by mentioning the specific code or hook that handles it (useAutoModeQueryInvalidation) to make it easier to trace the logic.
Address PR AutoMaker-Org#825 review feedback: - Extract duplicated backlog-like status check into shared helper in constants.ts - Improve spec-parser regex comment to clarify subsection preservation - Add file path reference in row-actions.tsx comment for traceability Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/ui/src/components/views/board-view/components/list-view/row-actions.tsx (1)
435-469: Use the shared helper consistently in primary-action stale-running logic.
isBacklogLikeStatusis used here, but Lines 145-150 still hardcode the same status list ingetPrimaryAction, which can drift.♻️ Suggested consistency patch
- if ( - isRunningTask && - (feature.status === 'backlog' || - feature.status === 'merge_conflict' || - feature.status === 'ready' || - feature.status === 'interrupted') && - handlers.onForceStop - ) { + if (isRunningTask && isBacklogLikeStatus(feature.status) && handlers.onForceStop) { return { icon: StopCircle, label: 'Stop', onClick: handlers.onForceStop, variant: 'destructive', }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/src/components/views/board-view/components/list-view/row-actions.tsx` around lines 435 - 469, getPrimaryAction contains a hardcoded list of "backlog-like" statuses that duplicates the logic in isBacklogLikeStatus; replace that duplicated status check inside getPrimaryAction with a call to isBacklogLikeStatus(feature.status) so the primary-action stale-running branch uses the shared helper consistently (update any conditional branches or guards in getPrimaryAction that currently compare against the hardcoded statuses to instead call isBacklogLikeStatus).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/server/src/services/spec-parser.ts`:
- Around line 219-224: The regex used to extract the "Summary" block in
spec-parser.ts (see variables sectionMatches, sectionMatch and getLastMatch) has
an overly broad lookahead (?=\n## [^#]|\n\*\*|$) that prematurely stops at bold
lines; remove the "\n\*\*" branch so the lookahead only ends on same-level "##"
headers or end-of-string (i.e., use (?=\n## [^#]|$)), then keep the existing
trimming/substring logic intact so bold markup inside the summary is preserved.
---
Nitpick comments:
In
`@apps/ui/src/components/views/board-view/components/list-view/row-actions.tsx`:
- Around line 435-469: getPrimaryAction contains a hardcoded list of
"backlog-like" statuses that duplicates the logic in isBacklogLikeStatus;
replace that duplicated status check inside getPrimaryAction with a call to
isBacklogLikeStatus(feature.status) so the primary-action stale-running branch
uses the shared helper consistently (update any conditional branches or guards
in getPrimaryAction that currently compare against the hardcoded statuses to
instead call isBacklogLikeStatus).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/server/src/services/spec-parser.tsapps/ui/src/components/views/board-view.tsxapps/ui/src/components/views/board-view/components/list-view/row-actions.tsxapps/ui/src/components/views/board-view/constants.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/server/src/services/event-hook-service.ts (1)
598-604: Minor simplification: Extract common pathname assignment.Both branches set
url.pathname = '/board'. Consider moving it outside the conditional to reduce duplication.♻️ Suggested simplification
try { const url = new URL(clickUrl); - // Add featureId as query param for deep linking to board with feature output modal - if (context.featureId) { - url.pathname = '/board'; + url.pathname = '/board'; + if (context.featureId) { url.searchParams.set('featureId', context.featureId); - } else { - url.pathname = '/board'; } clickUrl = url.toString();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/services/event-hook-service.ts` around lines 598 - 604, Move the shared assignment url.pathname = '/board' out of the if/else to eliminate duplication: always set url.pathname = '/board' first, then only run the conditional body to call url.searchParams.set('featureId', context.featureId) when context.featureId is truthy (leave out the else branch entirely). Update the block that currently references context.featureId, url.pathname and url.searchParams.set so pathname is assigned once and the conditional only handles adding the featureId query param.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/server/src/services/event-hook-service.ts`:
- Around line 598-604: Move the shared assignment url.pathname = '/board' out of
the if/else to eliminate duplication: always set url.pathname = '/board' first,
then only run the conditional body to call url.searchParams.set('featureId',
context.featureId) when context.featureId is truthy (leave out the else branch
entirely). Update the block that currently references context.featureId,
url.pathname and url.searchParams.set so pathname is assigned once and the
conditional only handles adding the featureId query param.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/server/src/services/event-hook-service.tsapps/server/tests/unit/services/event-hook-service.test.ts
Address PR #825 review feedback: - Extract duplicated backlog-like status check into shared helper in constants.ts - Improve spec-parser regex comment to clarify subsection preservation - Add file path reference in row-actions.tsx comment for traceability Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Address PR AutoMaker-Org#825 review feedback: - Extract duplicated backlog-like status check into shared helper in constants.ts - Improve spec-parser regex comment to clarify subsection preservation - Add file path reference in row-actions.tsx comment for traceability Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
useBoardFeaturesthat triggered cascading re-renders through DndContextChanges
Board View Crash Fix
useBoardFeatureshook forauto_mode_feature_startevents -useAutoModeQueryInvalidationalready handles this centrally, preventing cascade re-renders through DndContextmemo()to prevent parent re-renders from propagating through the entire DndContext subtreeHandling Stale Feature Status During Race Conditions
runningAutoTasksAllWorktrees) even if status is stale (still showing 'backlog'/'ready'/'interrupted')RowActionsfor running tasks with stale status - displays "View Logs", "Edit", "Spawn Sub-Task", and "Force Stop" optionsSpecification Parser Improvements
extractSummary()regex to stop at same-level## Headersbut preserve### Subsectionswithin Summary sections### Root Causeand### Fix Appliedsubsections to remain in extracted summarySummary by CodeRabbit
New Features
Improvements
Performance
Tests