-
Notifications
You must be signed in to change notification settings - Fork 491
feat: Add auto-validate toggle and import dialog for GitHub Issues #446
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
feat: Add auto-validate toggle and import dialog for GitHub Issues #446
Conversation
- Add githubAutoValidate state to app-store with persistence - Add auto-validate toggle (Zap icon + Switch) to Issues header - Implement auto-validation effect that validates open issues in batches of 3 - Add Import button to header for bulk importing issues as tasks - Create ImportIssuesDialog component with: - Status summary showing valid/invalid/needs clarification/not validated counts - Checkboxes to select issues (only valid issues are importable) - Select All Valid button for convenience - Bulk import functionality Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe changes introduce automatic validation batching for GitHub issues, enabling batch processing of up to 3 issues per cycle, along with a new bulk import dialog for importing validated issues as tasks. A new auto-validate toggle was added to the app store and wired into the UI, with corresponding enhancements to the IssuesListHeader component and a new ImportIssuesDialog component for managing the import workflow. Changes
Sequence DiagramsequenceDiagram
participant User
participant View as github-issues-view
participant Store as app-store
participant Header as issues-list-header
participant Dialog as import-issues-dialog
participant API as Validation Logic
User->>Header: Enables auto-validate toggle
Header->>View: onAutoValidateChange(true)
View->>Store: setGithubAutoValidate(true)
activate View
View->>API: Auto-validate batch (up to 3 issues)
loop Every batch
API->>Store: Update cached validations
end
deactivate View
User->>Header: Clicks Import button
Header->>View: onImportClick()
View->>Dialog: Show import dialog with validated issues
User->>Dialog: Select issues to import
User->>Dialog: Click Import
Dialog->>View: onImport(selected issues)
View->>View: Convert to tasks & report results
Dialog->>Dialog: Close on success
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
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 |
Summary of ChangesHello @aleslednej, 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 GitHub Issues integration by introducing features that automate the validation of issues and provide a streamlined way to bulk import these validated issues into tasks. These additions aim to improve efficiency and user experience when managing GitHub issues within the application, reducing manual effort for issue processing. 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. 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
|
|
Closing - need to follow CONTRIBUTING.md properly: run all pre-checks, test functionality, add unit tests |
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: 2
🤖 Fix all issues with AI agents
In @apps/ui/src/components/views/github-issues-view.tsx:
- Around line 191-225: handleConvertToTask currently swallows errors and doesn't
return status, so update its signature to return a boolean (true on success,
false on failure) and ensure it returns false when any error occurs instead of
only showing toast; keep toast behavior but avoid throwing. Then update
handleImportIssues to await the boolean from handleConvertToTask (instead of
assuming success) and increment successCount only when it returns true,
increment errorCount when it returns false or when validation is missing; keep
existing toast summaries. Also update any other callers of handleConvertToTask
to handle the new boolean return type.
In
@apps/ui/src/components/views/github-issues-view/dialogs/import-issues-dialog.tsx:
- Around line 130-136: The comment above the handleOpenChange function is
incorrect: it states "Reset selection when dialog opens" but the implementation
resets selection when !newOpen (on close). Update the comment to accurately
reflect behavior (e.g., "Reset selection when dialog closes") or remove it;
reference handleOpenChange, setSelectedIssues, and onOpenChange to locate the
code to modify.
🧹 Nitpick comments (1)
apps/ui/src/components/views/github-issues-view/components/issues-list-header.tsx (1)
41-89: Consider consolidatingTooltipProviderwrappers.Each tooltip has its own
TooltipProvider, but a single provider can wrap multiple tooltips. This reduces component nesting and is slightly more efficient.♻️ Suggested refactor
- <div className="flex items-center gap-3"> - {/* Auto-validate toggle */} - <TooltipProvider> - <Tooltip> - <TooltipTrigger asChild> - <div className="flex items-center gap-2"> + <TooltipProvider> + <div className="flex items-center gap-3"> + {/* Auto-validate toggle */} + <Tooltip> + <TooltipTrigger asChild> + <div className="flex items-center gap-2"> <Zap className={cn( 'h-4 w-4', autoValidate ? 'text-yellow-500' : 'text-muted-foreground' )} /> <Switch checked={autoValidate} onCheckedChange={onAutoValidateChange} aria-label="Auto-validate issues" /> </div> </TooltipTrigger> <TooltipContent> <p>Auto-validate: {autoValidate ? 'ON' : 'OFF'}</p> <p className="text-xs text-muted-foreground"> {autoValidate ? 'Issues are validated automatically when loaded' : 'Click validate button for each issue'} </p> </TooltipContent> </Tooltip> - </TooltipProvider> - {/* Import button */} - <TooltipProvider> + {/* Import button */} <Tooltip> <TooltipTrigger asChild> <Button variant="outline" size="sm" onClick={onImportClick}> <Import className="h-4 w-4" /> </Button> </TooltipTrigger> <TooltipContent> <p>Import issues as tasks</p> </TooltipContent> </Tooltip> - </TooltipProvider> - {/* Refresh button */} - <Button variant="outline" size="sm" onClick={onRefresh} disabled={refreshing}> - <RefreshCw className={cn('h-4 w-4', refreshing && 'animate-spin')} /> - </Button> - </div> + {/* Refresh button */} + <Button variant="outline" size="sm" onClick={onRefresh} disabled={refreshing}> + <RefreshCw className={cn('h-4 w-4', refreshing && 'animate-spin')} /> + </Button> + </div> + </TooltipProvider>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/ui/src/components/views/github-issues-view.tsxapps/ui/src/components/views/github-issues-view/components/issues-list-header.tsxapps/ui/src/components/views/github-issues-view/dialogs/import-issues-dialog.tsxapps/ui/src/components/views/github-issues-view/dialogs/index.tsapps/ui/src/store/app-store.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from shared packages (@automaker/*), never from old relative paths
Files:
apps/ui/src/components/views/github-issues-view/dialogs/import-issues-dialog.tsxapps/ui/src/components/views/github-issues-view/components/issues-list-header.tsxapps/ui/src/store/app-store.tsapps/ui/src/components/views/github-issues-view.tsxapps/ui/src/components/views/github-issues-view/dialogs/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
resolveModelString()from @automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Files:
apps/ui/src/components/views/github-issues-view/dialogs/import-issues-dialog.tsxapps/ui/src/components/views/github-issues-view/components/issues-list-header.tsxapps/ui/src/store/app-store.tsapps/ui/src/components/views/github-issues-view.tsxapps/ui/src/components/views/github-issues-view/dialogs/index.ts
🧠 Learnings (1)
📚 Learning: 2025-12-30T01:02:07.114Z
Learnt from: illia1f
Repo: AutoMaker-Org/automaker PR: 324
File: apps/ui/src/components/views/board-view/components/kanban-card/kanban-card.tsx:122-131
Timestamp: 2025-12-30T01:02:07.114Z
Learning: Tailwind CSS v4 uses postfix syntax for the important modifier: append ! at the end of the utility class (e.g., backdrop-blur-[0px]! or hover:bg-red-500!). The older v3 style used a prefix (!) at the start (e.g., !backdrop-blur-[0px]); prefer the postfix form for consistency across TSX files.
Applied to files:
apps/ui/src/components/views/github-issues-view/dialogs/import-issues-dialog.tsxapps/ui/src/components/views/github-issues-view/components/issues-list-header.tsxapps/ui/src/components/views/github-issues-view.tsx
🧬 Code graph analysis (2)
apps/ui/src/components/views/github-issues-view/components/issues-list-header.tsx (2)
apps/ui/src/lib/utils.ts (1)
cn(6-8)apps/ui/src/components/ui/button.tsx (1)
Button(108-108)
apps/ui/src/components/views/github-issues-view.tsx (4)
apps/ui/src/components/views/github-issues-view/types.ts (1)
ValidateIssueOptions(17-24)apps/ui/src/lib/electron.ts (1)
GitHubIssue(257-268)apps/ui/src/components/views/github-issues-view/dialogs/import-issues-dialog.tsx (1)
ImportIssuesDialog(50-281)apps/ui/src/components/views/github-issues-view/dialogs/index.ts (1)
ImportIssuesDialog(2-2)
🔇 Additional comments (5)
apps/ui/src/components/views/github-issues-view/dialogs/index.ts (1)
1-2: LGTM!Clean barrel export following the existing pattern with
ValidationDialog.apps/ui/src/store/app-store.ts (1)
560-562: LGTM!The new
githubAutoValidatestate property and its setter follow the established patterns in the store. The default value offalseis appropriate for an opt-in feature.Also applies to: 930-932, 1189-1189, 1853-1855
apps/ui/src/components/views/github-issues-view/dialogs/import-issues-dialog.tsx (1)
50-128: LGTM!The component logic is well-structured with proper state management, memoized computations, and appropriate disabled states. The
handleImportcorrectly uses try/finally to ensureimportingis reset even on errors.apps/ui/src/components/views/github-issues-view.tsx (2)
56-102: LGTM on auto-validation logic!The batch validation approach is well-designed:
- The
refcorrectly prevents duplicate validations across effect re-runs- The cascading behavior (effect re-runs when
cachedValidationschanges) naturally picks up the next batch- The batch size of 3 provides reasonable throttling
- Clearing the ref when disabled allows re-validation on re-enable
363-372: LGTM!The
ImportIssuesDialogis correctly wired withopenIssues(rather than all issues), which makes sense since only open issues should be importable.
| // Handle bulk import of issues as tasks | ||
| const handleImportIssues = useCallback( | ||
| async (issues: GitHubIssue[]) => { | ||
| if (!currentProject?.path) { | ||
| toast.error('No project selected'); | ||
| return; | ||
| } | ||
|
|
||
| let successCount = 0; | ||
| let errorCount = 0; | ||
|
|
||
| for (const issue of issues) { | ||
| const validation = cachedValidations.get(issue.number); | ||
| if (!validation?.result) { | ||
| errorCount++; | ||
| continue; | ||
| } | ||
|
|
||
| try { | ||
| await handleConvertToTask(issue, validation.result); | ||
| successCount++; | ||
| } catch { | ||
| errorCount++; | ||
| } | ||
| } | ||
|
|
||
| if (successCount > 0) { | ||
| toast.success(`Imported ${successCount} issue${successCount !== 1 ? 's' : ''} as tasks`); | ||
| } | ||
| if (errorCount > 0) { | ||
| toast.error(`Failed to import ${errorCount} issue${errorCount !== 1 ? 's' : ''}`); | ||
| } | ||
| }, | ||
| [currentProject?.path, cachedValidations, handleConvertToTask] | ||
| ); |
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.
handleConvertToTask doesn't indicate success/failure to caller, causing incorrect counts.
handleConvertToTask handles errors internally with toast.error but doesn't return a success/failure indicator. This means handleImportIssues will always increment successCount for every issue, even when the task creation fails.
🐛 Suggested fix
First, modify handleConvertToTask to return a boolean:
const handleConvertToTask = useCallback(
- async (issue: GitHubIssue, validation: IssueValidationResult) => {
+ async (issue: GitHubIssue, validation: IssueValidationResult): Promise<boolean> => {
if (!currentProject?.path) {
toast.error('No project selected');
- return;
+ return false;
}
try {
const api = getElectronAPI();
if (api.features?.create) {
// ... build description and feature ...
const result = await api.features.create(currentProject.path, feature);
if (result.success) {
toast.success(`Created task: ${issue.title}`);
+ return true;
} else {
toast.error(result.error || 'Failed to create task');
+ return false;
}
}
+ return false;
} catch (err) {
logger.error('Convert to task error:', err);
toast.error(err instanceof Error ? err.message : 'Failed to create task');
+ return false;
}
},
[currentProject?.path, defaultProfile, currentBranch]
);Then update handleImportIssues to use the return value:
try {
- await handleConvertToTask(issue, validation.result);
- successCount++;
+ const success = await handleConvertToTask(issue, validation.result);
+ if (success) {
+ successCount++;
+ } else {
+ errorCount++;
+ }
} catch {
errorCount++;
}🤖 Prompt for AI Agents
In @apps/ui/src/components/views/github-issues-view.tsx around lines 191 - 225,
handleConvertToTask currently swallows errors and doesn't return status, so
update its signature to return a boolean (true on success, false on failure) and
ensure it returns false when any error occurs instead of only showing toast;
keep toast behavior but avoid throwing. Then update handleImportIssues to await
the boolean from handleConvertToTask (instead of assuming success) and increment
successCount only when it returns true, increment errorCount when it returns
false or when validation is missing; keep existing toast summaries. Also update
any other callers of handleConvertToTask to handle the new boolean return type.
| // Reset selection when dialog opens | ||
| const handleOpenChange = (newOpen: boolean) => { | ||
| if (!newOpen) { | ||
| setSelectedIssues(new Set()); | ||
| } | ||
| onOpenChange(newOpen); | ||
| }; |
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.
Misleading comment: the selection is reset when dialog closes, not opens.
The comment says "Reset selection when dialog opens" but the code resets when !newOpen (i.e., when closing). The logic is correct; only the comment is inaccurate.
📝 Suggested fix
- // Reset selection when dialog opens
+ // Reset selection when dialog closes
const handleOpenChange = (newOpen: boolean) => {
if (!newOpen) {
setSelectedIssues(new Set());
}
onOpenChange(newOpen);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Reset selection when dialog opens | |
| const handleOpenChange = (newOpen: boolean) => { | |
| if (!newOpen) { | |
| setSelectedIssues(new Set()); | |
| } | |
| onOpenChange(newOpen); | |
| }; | |
| // Reset selection when dialog closes | |
| const handleOpenChange = (newOpen: boolean) => { | |
| if (!newOpen) { | |
| setSelectedIssues(new Set()); | |
| } | |
| onOpenChange(newOpen); | |
| }; |
🤖 Prompt for AI Agents
In
@apps/ui/src/components/views/github-issues-view/dialogs/import-issues-dialog.tsx
around lines 130 - 136, The comment above the handleOpenChange function is
incorrect: it states "Reset selection when dialog opens" but the implementation
resets selection when !newOpen (on close). Update the comment to accurately
reflect behavior (e.g., "Reset selection when dialog closes") or remove it;
reference handleOpenChange, setSelectedIssues, and onOpenChange to locate the
code to modify.
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.
Code Review
This pull request introduces two significant features to the GitHub Issues view: an auto-validation toggle and a bulk import dialog. The implementation is solid, adding new state to the app-store, creating a new dialog component, and updating the header. My review includes a few suggestions to improve performance, adhere to component library best practices, and enhance code robustness by preventing potential stale state issues.
| let successCount = 0; | ||
| let errorCount = 0; | ||
|
|
||
| for (const issue of issues) { | ||
| const validation = cachedValidations.get(issue.number); | ||
| if (!validation?.result) { | ||
| errorCount++; | ||
| continue; | ||
| } | ||
|
|
||
| try { | ||
| await handleConvertToTask(issue, validation.result); | ||
| successCount++; | ||
| } catch { | ||
| errorCount++; | ||
| } | ||
| } |
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.
For a better user experience with bulk imports, you can process the issue imports in parallel instead of sequentially. Using Promise.allSettled will execute all import operations concurrently, which can significantly speed up the process when importing multiple issues. This also allows for more robust error handling by collecting all results, whether fulfilled or rejected.
const results = await Promise.allSettled(
issues.map((issue) => {
const validation = cachedValidations.get(issue.number);
if (!validation?.result) {
return Promise.reject(new Error(`Issue #${issue.number} is not validated.`));
}
return handleConvertToTask(issue, validation.result);
})
);
const successCount = results.filter((r) => r.status === 'fulfilled').length;
const errorCount = results.length - successCount;
if (errorCount > 0) {
const failedReasons = results
.filter((r): r is PromiseRejectedResult => r.status === 'rejected')
.map((r) => r.reason);
logger.error('Failed to import some issues:', failedReasons);
}
| <div className="flex items-center gap-3"> | ||
| {/* Auto-validate toggle */} | ||
| <TooltipProvider> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <div className="flex items-center gap-2"> | ||
| <Zap | ||
| className={cn( | ||
| 'h-4 w-4', | ||
| autoValidate ? 'text-yellow-500' : 'text-muted-foreground' | ||
| )} | ||
| /> | ||
| <Switch | ||
| checked={autoValidate} | ||
| onCheckedChange={onAutoValidateChange} | ||
| aria-label="Auto-validate issues" | ||
| /> | ||
| </div> | ||
| </TooltipTrigger> | ||
| <TooltipContent> | ||
| <p>Auto-validate: {autoValidate ? 'ON' : 'OFF'}</p> | ||
| <p className="text-xs text-muted-foreground"> | ||
| {autoValidate | ||
| ? 'Issues are validated automatically when loaded' | ||
| : 'Click validate button for each issue'} | ||
| </p> | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| </TooltipProvider> | ||
|
|
||
| {/* Import button */} | ||
| <TooltipProvider> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <Button variant="outline" size="sm" onClick={onImportClick}> | ||
| <Import className="h-4 w-4" /> | ||
| </Button> | ||
| </TooltipTrigger> | ||
| <TooltipContent> | ||
| <p>Import issues as tasks</p> | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| </TooltipProvider> | ||
|
|
||
| {/* Refresh button */} | ||
| <Button variant="outline" size="sm" onClick={onRefresh} disabled={refreshing}> | ||
| <RefreshCw className={cn('h-4 w-4', refreshing && 'animate-spin')} /> | ||
| </Button> | ||
| </div> |
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.
It's a best practice to wrap your application (or a section of it) in a single <TooltipProvider>. This is more efficient and cleaner than using a separate provider for each tooltip. You can wrap the div containing all the buttons with one <TooltipProvider>.
<TooltipProvider>
<div className="flex items-center gap-3">
{/* Auto-validate toggle */}
<Tooltip>
<TooltipTrigger asChild>
<div className="flex items-center gap-2">
<Zap
className={cn(
'h-4 w-4',
autoValidate ? 'text-yellow-500' : 'text-muted-foreground'
)}
/>
<Switch
checked={autoValidate}
onCheckedChange={onAutoValidateChange}
aria-label="Auto-validate issues"
/>
</div>
</TooltipTrigger>
<TooltipContent>
<p>Auto-validate: {autoValidate ? 'ON' : 'OFF'}</p>
<p className="text-xs text-muted-foreground">
{autoValidate
? 'Issues are validated automatically when loaded'
: 'Click validate button for each issue'}
</p>
</TooltipContent>
</Tooltip>
{/* Import button */}
<Tooltip>
<TooltipTrigger asChild>
<Button variant="outline" size="sm" onClick={onImportClick}>
<Import className="h-4 w-4" />
</Button>
</TooltipTrigger>
<TooltipContent>
<p>Import issues as tasks</p>
</TooltipContent>
</Tooltip>
{/* Refresh button */}
<Button variant="outline" size="sm" onClick={onRefresh} disabled={refreshing}>
<RefreshCw className={cn('h-4 w-4', refreshing && 'animate-spin')} />
</Button>
</div>
</TooltipProvider>
| const handleToggleIssue = (issueNumber: number) => { | ||
| const newSelected = new Set(selectedIssues); | ||
| if (newSelected.has(issueNumber)) { | ||
| newSelected.delete(issueNumber); | ||
| } else { | ||
| newSelected.add(issueNumber); | ||
| } | ||
| setSelectedIssues(newSelected); | ||
| }; |
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.
To prevent potential issues with stale state, it's a good practice to use the functional update form of setSelectedIssues. This ensures that you're always working with the most recent state, especially if this handler were to be used in a context where it's not re-created on every render.
const handleToggleIssue = (issueNumber: number) => {
setSelectedIssues((currentSelected) => {
const newSelected = new Set(currentSelected);
if (newSelected.has(issueNumber)) {
newSelected.delete(issueNumber);
} else {
newSelected.add(issueNumber);
}
return newSelected;
});
};
| setSelectedIssues(new Set()); | ||
| onOpenChange(false); |
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.
Summary
Changes
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
✏️ Tip: You can customize this high-level summary in your review settings.