Conversation
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://witty-hill-08172a210-290.centralus.5.azurestaticapps.net |
|
@codex make a detailed review of this PR |
There was a problem hiding this comment.
Pull request overview
This PR removes the legacy “Assembly Plan” editor/object type and shifts workflow UI focus to the Transformations panel by expanding the Transformation wizard into a multi-step “Design → Build → Collection → Execute” flow.
Changes:
- Removed the Assembly panel type and Assembly object type from the frontend registries.
- Deleted the Assembly editor panel/wizard implementation.
- Expanded
TransformationWizardwith additional workflow steps and configuration UI (assembly method/compiler/machine selections).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
frontend/src/panels.js |
Removes the Assembly panel type registration. |
frontend/src/objectTypes.js |
Removes the Assembly object type registration. |
frontend/src/components/panels/transformations/TransformationWizard.jsx |
Adds a 4-step transformation workflow UI and build configuration selections. |
frontend/src/components/panels/assembly-editor/AssemblyWizard.jsx |
Deletes the previous Assembly wizard implementation. |
frontend/src/components/panels/assembly-editor/AssemblyPanel.jsx |
Deletes the previous Assembly panel implementation. |
frontend/src/activities.js |
Removes Assembly object type from the Explorer activity’s listed object types. |
Comments suppressed due to low confidence (2)
frontend/src/panels.js:114
- After removing the
AssemblyPlanCreatorpanel type,GiThorHammerappears to be unused in this file. Please remove the unused icon import to avoid dead code and potential lint/build warnings.
Experiment: {
id: "synbio.panel-type.data-collector",
title: "Data Collector",
component: CollectionPanel,
objectTypes: [ ObjectTypes.Experiments.id ],
icon: FcAddDatabase,
frontend/src/objectTypes.js:105
- With the
Assemblyobject type removed, theGiThorHammericon import at the top of this file is now unused. Please remove the unused import to keep the ObjectTypes module clean and avoid lint/build warnings.
subdirectory: "strains",
downloadable: true,
},
Transformations: {
id: "synbio.object-type.transformations",
title: "Transformations",
listTitle: "Transformations",
fileNameMatch: /\.json$/,
icon: GiSewingMachine,
createable: true,
extension: '.json',
subdirectory: "transformations"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| objectTypesToList: [ | ||
| ObjectTypes.Plasmids.id, | ||
| ObjectTypes.Strains.id, | ||
| ObjectTypes.Assembly.id, | ||
| ObjectTypes.Transformations.id, | ||
| ] |
There was a problem hiding this comment.
This PR removes ObjectTypes.Assembly, but there is still code elsewhere that classifies files/folders as synbio.object-type.assembly-plan (e.g., OneDrive mapping for the assemblyPlans subdirectory). After this change, those files will become unclassified/invisible in the explorer. Consider updating the remaining classifier/mapping logic or providing a migration path so existing assemblyPlans content is handled intentionally.
| @@ -13,44 +37,126 @@ export default function TransformationWizard() { | |||
| const nextStep = () => setActiveStep((current) => (current < numSteps ? current + 1 : current)) | |||
There was a problem hiding this comment.
nextStep uses current < numSteps even though activeStep === numSteps - 1 is treated as the last step. With 4 steps (indexes 0–3), this allows activeStep to become 4, which puts Mantine Stepper into an out-of-range/completed state (no <Stepper.Completed> is defined) and can render nothing. Update the increment condition to cap at numSteps - 1 (or compute numSteps from the actual Stepper children) so the last valid step index is not exceeded.
| const nextStep = () => setActiveStep((current) => (current < numSteps ? current + 1 : current)) | |
| const nextStep = () => setActiveStep((current) => (current < numSteps - 1 ? current + 1 : current)) |
| const nextStep = () => setActiveStep((current) => (current < numSteps ? current + 1 : current)) | ||
| const prevStep = () => setActiveStep((current) => (current > 0 ? current - 1 : current)) | ||
|
|
||
| const [assemblyMethod, setAssemblyMethod] = usePanelProperty(panelId, "assemblyMethod", false, AssemblyMethods.MOCLO) |
There was a problem hiding this comment.
The const [assemblyMethod, ...] = usePanelProperty(...) line is mis-indented relative to the rest of the function body, which makes the state block harder to scan and looks accidental. Align it with the other const declarations.
| const [assemblyMethod, setAssemblyMethod] = usePanelProperty(panelId, "assemblyMethod", false, AssemblyMethods.MOCLO) | |
| const [assemblyMethod, setAssemblyMethod] = usePanelProperty(panelId, "assemblyMethod", false, AssemblyMethods.MOCLO) |
| <Button | ||
| variant="outline" | ||
| fullWidth | ||
| style={{ marginBottom: 16 }} | ||
| > | ||
| Insert Plasmids from SynBioHub | ||
| Select Abstract Design | ||
| </Button> | ||
| <Button | ||
| variant="outline" | ||
| fullWidth | ||
| style={{ marginBottom: 16 }} | ||
| > | ||
| Select Plasmids | ||
| </Button> | ||
| </Stepper.Step> | ||
| <Stepper.Step | ||
| allowStepSelect={activeStep > 1} | ||
| label="Collections" | ||
| > | ||
| <Button | ||
| variant="outline" | ||
| fullWidth | ||
| style={{ marginBottom: 16 }} | ||
| > | ||
| Import Strains from SynBioHub | ||
| Select Backbone | ||
| </Button> | ||
| <Button | ||
| variant="outline" | ||
| fullWidth | ||
| style={{ marginBottom: 16 }} | ||
| > | ||
| Select Chassis | ||
| </Button> |
There was a problem hiding this comment.
Several UI controls are rendered as active buttons but have no onClick handler (e.g., “Select Abstract Design”, “Select Plasmids”, “Select Backbone”, “Select Chassis”, “Select Destination Collection”). As-is they appear functional but do nothing, which is confusing in the workflow. Either wire these to the intended selection workflows, or mark them disabled / replace with non-interactive placeholders until implemented.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13231a1ec1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ObjectTypes.Plasmids.id, | ||
| ObjectTypes.Strains.id, | ||
| ObjectTypes.Assembly.id, | ||
| ObjectTypes.Transformations.id, |
There was a problem hiding this comment.
Dropping ObjectTypes.Assembly.id from the Build activity removes the only explorer bucket that can surface existing assembly-plan assets, but the app still classifies OneDrive files under assemblyPlans/*.json as synbio.object-type.assembly-plan (mapOneDriveItemToObjectType in frontend/src/microsoft-utils/oneDrive/fetchFromOneDrive.js). After this change, users with pre-existing assembly plans can no longer find/open those files from the workflow UI, which is a regression in data accessibility unless a migration/fallback path is added.
Useful? React with 👍 / 👎.
No description provided.