-
Notifications
You must be signed in to change notification settings - Fork 483
feat: implement pipeline step exclusion functionality #637
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
Conversation
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Added support for excluding specific pipeline steps in feature management, allowing users to skip certain steps during execution. - Introduced a new `PipelineExclusionControls` component for managing exclusions in the UI. - Updated relevant dialogs and components to handle excluded pipeline steps, including `AddFeatureDialog`, `EditFeatureDialog`, and `MassEditDialog`. - Enhanced the `getNextStatus` method in `PipelineService` to account for excluded steps when determining the next status in the pipeline flow. - Updated tests to cover scenarios involving excluded pipeline steps.
Summary of ChangesHello @Shironex, 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 introduces a significant enhancement to feature management by enabling the selective exclusion of custom pipeline steps. This allows for more flexible and tailored feature workflows, where certain automated stages can be bypassed as needed. The changes encompass both the server-side logic, ensuring that pipeline execution correctly accounts for excluded steps, and the user interface, providing intuitive controls for configuring these exclusions within various feature management dialogs. The overall impact is improved adaptability in handling diverse feature development processes. 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
|
📝 WalkthroughWalkthroughThis PR adds per-feature pipeline step exclusions (types, UI controls, and plumbing) and updates backend pipeline progression to skip excluded steps, including resume handling when the current or all remaining steps are excluded. It also expands several provider/type exports and bumps package versions. Changes
Sequence DiagramsequenceDiagram
participant User as User (UI)
participant Dialog as Edit/Add/MassEdit Dialog
participant UI as BoardView / Badges
participant Service as AutoModeService
participant Pipeline as PipelineService
participant DB as Feature Storage
User->>Dialog: Toggle pipeline step exclusions & save
Dialog->>DB: Persist feature with `excludedPipelineSteps`
UI->>Service: Trigger resume/execute for feature
Service->>Service: Build allSortedSteps (unfiltered)
Service->>Service: Check if current step is excluded
alt Current step is excluded
Service->>Pipeline: getNextStatus(currentStatus, config, skipTests, excludedStepIds)
Pipeline->>Pipeline: Filter steps by excludedStepIds and compute next status
Pipeline-->>Service: Return next status
alt Next status is pipeline step
Service->>Service: Determine next start index and execute remaining non-excluded steps
else Next status is final
Service->>DB: Mark feature complete with final status
Service->>UI: Emit completion event
end
else Current step not excluded
Service->>Service: Execute filtered steps (omit exclusions)
end
alt No steps remain after filtering
Service->>DB: Mark feature completed
Service->>UI: Emit completion event
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
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 functionality to exclude specific pipeline steps during feature execution, enhancing flexibility in workflow management. The changes span across backend services for logic implementation and frontend components for UI controls and visual indicators. The core logic for skipping excluded steps in PipelineService and AutoModeService appears robust, and the new UI components for managing these exclusions are well-integrated. Comprehensive test cases have been added to cover various scenarios of pipeline step exclusion, which is excellent for ensuring correctness.
…8936017583-e6ni # Conflicts: # apps/ui/src/components/views/board-view.tsx
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)
apps/server/src/services/auto-mode-service.ts (1)
1730-1803: Resume step number can be off when earlier steps are excluded.
startFromStepIndexis based on the unfiltered list but is logged against the filtered length, which can yield confusing messages (e.g., “step 3/2”). Consider computing the display index relative to the filtered list.🛠️ Suggested adjustment for display index
- const sortedSteps = allSortedSteps.filter((step) => !excludedStepIds.has(step.id)); + const sortedSteps = allSortedSteps.filter((step) => !excludedStepIds.has(step.id)); + const resumeIndex = stepsToExecute.length + ? sortedSteps.findIndex((s) => s.id === stepsToExecute[0].id) + : -1; + const resumeDisplayIndex = resumeIndex >= 0 ? resumeIndex + 1 : 1; - console.log( - `[AutoMode] Resuming pipeline for feature ${featureId} from step ${startFromStepIndex + 1}/${sortedSteps.length}` - ); + console.log( + `[AutoMode] Resuming pipeline for feature ${featureId} from step ${resumeDisplayIndex}/${sortedSteps.length}` + ); - this.emitAutoModeEvent('auto_mode_progress', { + this.emitAutoModeEvent('auto_mode_progress', { featureId, projectPath, branchName: branchName ?? null, - content: `Resuming from pipeline step ${startFromStepIndex + 1}/${sortedSteps.length}`, + content: `Resuming from pipeline step ${resumeDisplayIndex}/${sortedSteps.length}`, });
🧹 Nitpick comments (1)
apps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsx (1)
40-67: Normalize excluded step order before mixed-value comparison.Current JSON string comparison is order-sensitive and can show “mixed” even when the same steps are excluded in different orders.
♻️ Proposed tweak
function getMixedValues(features: Feature[]): Record<string, boolean> { if (features.length === 0) return {}; const first = features[0]; - const firstExcludedSteps = JSON.stringify(first.excludedPipelineSteps || []); + const normalizeSteps = (steps?: string[]) => [...(steps ?? [])].sort(); + const firstExcludedSteps = JSON.stringify(normalizeSteps(first.excludedPipelineSteps)); return { model: !features.every((f) => f.model === first.model), thinkingLevel: !features.every((f) => f.thinkingLevel === first.thinkingLevel), planningMode: !features.every((f) => f.planningMode === first.planningMode), requirePlanApproval: !features.every( (f) => f.requirePlanApproval === first.requirePlanApproval ), priority: !features.every((f) => f.priority === first.priority), skipTests: !features.every((f) => f.skipTests === first.skipTests), branchName: !features.every((f) => f.branchName === first.branchName), - excludedPipelineSteps: !features.every( - (f) => JSON.stringify(f.excludedPipelineSteps || []) === firstExcludedSteps - ), + excludedPipelineSteps: !features.every( + (f) => JSON.stringify(normalizeSteps(f.excludedPipelineSteps)) === firstExcludedSteps + ), }; }
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 (3)
apps/server/src/services/auto-mode-service.ts (2)
1819-1823: Inconsistent index and count in log message.
startFromStepIndexis an index intoallSortedSteps(the unfiltered list), butsortedSteps.lengthis the count of filtered (non-excluded) steps. This produces misleading log output.For example, with 5 total steps where 2 are excluded:
allSortedSteps.length= 5sortedSteps.length= 3- If resuming from index 4, log shows "step 5/3"
🔧 Suggested fix
// Use the filtered steps for counting const sortedSteps = allSortedSteps.filter((step) => !excludedStepIds.has(step.id)); + const stepIndexInFiltered = sortedSteps.findIndex((s) => s.id === allSortedSteps[startFromStepIndex].id); console.log( - `[AutoMode] Resuming pipeline for feature ${featureId} from step ${startFromStepIndex + 1}/${sortedSteps.length}` + `[AutoMode] Resuming pipeline for feature ${featureId} from step ${stepIndexInFiltered + 1}/${sortedSteps.length}` );Alternatively, use consistent numbering from the unfiltered list:
${startFromStepIndex + 1}/${allSortedSteps.length}and note excluded steps separately.
1878-1883: Same inconsistency in progress event message.This event uses the same mixed indices pattern as the console log above.
🔧 Suggested fix
this.emitAutoModeEvent('auto_mode_progress', { featureId, projectPath, branchName: branchName ?? null, - content: `Resuming from pipeline step ${startFromStepIndex + 1}/${sortedSteps.length}`, + content: `Resuming from pipeline step ${stepIndexInFiltered + 1}/${sortedSteps.length}`, });Use the same
stepIndexInFilteredvariable suggested in the previous comment.apps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsx (1)
54-68: Order-dependent array comparison may show false "mixed values" indicator.
JSON.stringifycomparison is order-sensitive. If two features have the same excluded steps but in different order (e.g.,['step1', 'step2']vs['step2', 'step1']), they'll incorrectly display as "Mixed values".Suggested fix using sorted comparison
function getMixedValues(features: Feature[]): Record<string, boolean> { if (features.length === 0) return {}; const first = features[0]; - const firstExcludedSteps = JSON.stringify(first.excludedPipelineSteps || []); + const firstExcludedSteps = JSON.stringify([...(first.excludedPipelineSteps || [])].sort()); return { model: !features.every((f) => f.model === first.model), thinkingLevel: !features.every((f) => f.thinkingLevel === first.thinkingLevel), planningMode: !features.every((f) => f.planningMode === first.planningMode), requirePlanApproval: !features.every( (f) => f.requirePlanApproval === first.requirePlanApproval ), priority: !features.every((f) => f.priority === first.priority), skipTests: !features.every((f) => f.skipTests === first.skipTests), branchName: !features.every((f) => f.branchName === first.branchName), excludedPipelineSteps: !features.every( - (f) => JSON.stringify(f.excludedPipelineSteps || []) === firstExcludedSteps + (f) => JSON.stringify([...(f.excludedPipelineSteps || [])].sort()) === firstExcludedSteps ), }; }
🧹 Nitpick comments (1)
apps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsx (1)
384-400: Consider conditionally rendering the Pipeline Steps section when project context is unavailable.
projectPathis optional and can be undefined when passed toMassEditDialog(as seen inboard-view.tsxwhere it's set tocurrentProject?.path). When undefined,usePipelineConfigdisables its query andPipelineExclusionControlsreturnsnullbecause no pipeline steps are available. However, theFieldWrapperstill renders its container with the "Pipeline Steps" label and checkbox, creating an empty-looking field that may confuse users.Suggested fix: wrap in conditional
{/* Pipeline Exclusion */} + {projectPath && ( <FieldWrapper label="Pipeline Steps" isMixed={mixedValues.excludedPipelineSteps} willApply={applyState.excludedPipelineSteps} onApplyChange={(apply) => setApplyState((prev) => ({ ...prev, excludedPipelineSteps: apply })) } > <PipelineExclusionControls projectPath={projectPath} excludedPipelineSteps={excludedPipelineSteps} onExcludedStepsChange={setExcludedPipelineSteps} testIdPrefix="mass-edit-pipeline" /> </FieldWrapper> + )}Alternatively, if the section should always be visible, consider adding a loading or empty state message inside the
FieldWrapper.
PipelineExclusionControlscomponent for managing exclusions in the UI.AddFeatureDialog,EditFeatureDialog, andMassEditDialog.getNextStatusmethod inPipelineServiceto account for excluded steps when determining the next status in the pipeline flow.Summary by CodeRabbit
New Features
Bug Fixes
Other
✏️ Tip: You can customize this high-level summary in your review settings.