-
Notifications
You must be signed in to change notification settings - Fork 29
Refresh AI job UI #8873
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
base: master
Are you sure you want to change the base?
Refresh AI job UI #8873
Conversation
📝 WalkthroughWalkthroughBackend job-cost mapping extended for new JobCommand variants; frontend replaces modal-based AI-job UX with a drawer-based system—removing modal components and adding drawer tabs, contexts, selectors, validators, credit UI, training/run/alignment pages, helpers, import/path updates, styles, docs, and admin API/type renames. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
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.
impressive pr 👍 I finally made it through all line changes and left a couple of remarks. I did not test yet (will do so after the feedback was incorporated) and I also plan to have another high-level look at the modules, since some stuff felt a bit repetitive (but it might be very well inheritent to the domain problem).
{ | ||
key: "open_ai_training_button", | ||
onClick: () => Store.dispatch(setAIJobDrawerStateAction("open_ai_training")), | ||
label: "Train AI model", |
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.
maybe "train new ai model"?
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.
Done
|
||
const handleStartAnalysis = useCallback(async () => { | ||
try { | ||
startAlignSectionsJob( |
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.
needs to be awaited?
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.
Done
{ | ||
name: "Align Sections", | ||
comment: | ||
"Align all sections of this dataset along the Z axis using features in neighboring sections. Optimized for datasets with a single tile per sections (no stitching needed).", |
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.
I'd replace "optimized for..." with sth else (e.g., "only supports"). see my other comment in the md files.
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.
Done
padding: "16px", | ||
opacity: item.disabled ? 0.5 : 1, | ||
cursor: item.disabled ? "not-allowed" : "pointer", | ||
transition: "backgroun-color 0.2s ease-in-out", |
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.
typo in background. maybe remove it if it wasn't noticable anyway.
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.
Done
name="maxDistanceNm" | ||
label="Max Distance (nm)" | ||
rules={[{ required: true, message: "Please enter a positive number" }]} | ||
tooltip='The maximum cross-section length or distance ("diameter") for each identified object in nm e.g. Nuclei: 1000nm, Vesicles: 80nm' |
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.
I think I've seen a slightly different description for the same thing somewhere else. can this be dry'd somehow?
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.
The docs contains the same sentence.
"Train a new AI model for EM neuron segmentation based on the annotations in this dataset. Optimized dense neuronal tissue from SEM, FIB-SEM, SBEM, Multi-SEM microscopes.", | ||
id: "train-neuron-model", | ||
jobType: APIJobType.TRAIN_NEURON_MODEL, | ||
image: "/assets/images/neuron_inferral_example.jpg", | ||
}, | ||
{ | ||
name: "EM Instances Model", | ||
comment: | ||
"Train a new AI model for EM instance segmentation based on the annotations in this dataset. Optimized for nuclei, mitochondria and other cell types.", |
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.
this is duplicated (including typo) from the ai_model_selector.tsx. DRY?
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.
The description for training an AI model is unique but similar to the one for running AI models. After all training a model a certain way will influence what can be executed for inference.
colorLayerName: selection.imageDataLayer!, | ||
segmentationLayerName: selection.groundTruthLayer!, |
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.
same here with the !
errors.push("Total volume of bounding boxes cannot be zero."); | ||
} | ||
|
||
const MIN_BBOX_EXTENT_IN_EACH_DIM = 32; |
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.
hoist to top level?
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.
Done
const groupedBoxes = groupBy(nonMultipleBoxes, "annotationId"); | ||
warningNode = ( | ||
<div style={{ whiteSpace: "pre-wrap" }}> | ||
{`For optimal training, all bounding boxes should have dimensions that are multiples of the smallest box dimensions (${minDimensions.x}x${minDimensions.y}x${minDimensions.z} vx). The following boxes do not fit well:`} |
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.
can we improve "do not fit well"? maybe "the following boxes are not ideal"?
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.
Done
…_selector.tsx Co-authored-by: Philipp Otto <[email protected]>
Co-authored-by: Philipp Otto <[email protected]>
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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/automation/ai_training.md (1)
59-60
: Polish summary sentence.The sentence has a grammatical glitch and the
>
formatting lacks spaces. Suggest tweaking as below.-Once the training is finished, you can find and overview of all your trained model from the `Admin` >`AI Models` page in the navbar. Please refer to the [AI Segmentation](./ai_segmentation.md) guide for more information on how to apply your trained models to your dataset. +Once the training is finished, you can find an overview of all your trained models on the `Admin` > `AI Models` page in the navbar. Please refer to the [AI Segmentation](./ai_segmentation.md) guide for more information on how to apply your trained models to your dataset.frontend/javascripts/viewer/view/ai_jobs/utils.ts (1)
89-104
: Use Math.ceil for min-extent check to avoid under-enforcing the requirementRounding can under-require extents (e.g., 19.2 rounds to 19). Ceil ensures bbox/dataset must be at least the minimum after scaling.
Apply this diff:
- const minExtentInMag1 = minBBoxExtentInModelMag.map((extent, i) => - Math.round(extent * mag[i]), - ) as Vector3; + const minExtentInMag1 = minBBoxExtentInModelMag.map((extent, i) => + Math.ceil(extent * mag[i]), + ) as Vector3;
🧹 Nitpick comments (15)
frontend/javascripts/viewer/view/action_bar_view.tsx (4)
344-344
: Make isAdminOrDatasetManager a strict boolean.Prevents strict-boolean-expression issues and clarifies intent.
- const isAdminOrDatasetManager = activeUser && isUserAdminOrTeamManager(activeUser); + const isAdminOrDatasetManager = + activeUser != null && isUserAdminOrTeamManager(activeUser);
45-45
: Remove commented import.Dead/commented code should be cleaned up.
-// import { StartAIJobModal } from "./action-bar/ai_job_modals/start_ai_job_modal";
301-301
: Avoid Store.dispatch in components.Prefer using dispatch from props (via connect) for testability and inversion of control.
Also applies to: 308-308, 315-315
329-332
: Optional: Use Tooltip for disabled reason.title on a disabled button is inconsistent across browsers; Tooltip + wrapper span yields better UX.
frontend/javascripts/viewer/view/ai_jobs/utils.ts (5)
17-26
: Narrow getMinimumDSSize() input type to inference job types to prevent runtime throwsCurrently the function accepts APIJobType (broad) but only supports four inference types; others will hit the default error at runtime. Narrow the parameter type so mismatches are caught at compile time.
Apply this diff:
-export const getMinimumDSSize = (jobType: APIJobType) => { +export const getMinimumDSSize = ( + jobType: + | APIJobType.INFER_MITOCHONDRIA + | APIJobType.INFER_NEURONS + | APIJobType.INFER_NUCLEI + | APIJobType.INFER_INSTANCES, +) => { switch (jobType) { case APIJobType.INFER_NEURONS: case APIJobType.INFER_NUCLEI: case APIJobType.INFER_INSTANCES: return MIN_BBOX_EXTENT[jobType].map((dim) => dim * 2); case APIJobType.INFER_MITOCHONDRIA: return MIN_BBOX_EXTENT[jobType].map((dim) => dim + 80); - default: - throw new Error(`Unknown job type: ${jobType}`); + default: + // Unreachable due to narrowed type. + throw new Error(`Unknown job type: ${jobType}`); } };
306-318
: Align Form validator typing with antd v5 and simplify returnMinor type polish: the first arg is typically RuleObject; also no need to construct Error explicitly for message-only rejections.
Apply this diff:
-export const colorLayerMustNotBeUint24Rule = { - validator: (_: Rule, value: APIDataLayer) => { +export const colorLayerMustNotBeUint24Rule = { + validator: (_: Rule, value: APIDataLayer) => { if (value && value.elementClass === "uint24") { - return Promise.reject( - new Error( - "The selected layer of type uint24 is not supported. Please select a different one.", - ), - ); + return Promise.reject( + "The selected layer of type uint24 is not supported. Please select a different one.", + ); } return Promise.resolve(); }, };If you prefer stricter typing, switch the first param to RuleObject from antd’s form typings. Verify against your exact [email protected] types. Based on learnings
320-323
: Return an empty list instead of null for getMagsForColorLayerAvoids null-checks downstream and keeps return type uniform (array).
Apply this diff:
-export const getMagsForColorLayer = (colorLayers: APIDataLayer[], layerName: string) => { +export const getMagsForColorLayer = (colorLayers: APIDataLayer[], layerName: string) => { const colorLayer = colorLayers.find((layer) => layer.name === layerName); - return colorLayer != null ? getMagInfo(colorLayer.resolutions).getMagList() : null; + return colorLayer != null ? getMagInfo(colorLayer.resolutions).getMagList() : []; };
325-343
: Make getIntersectingMagList return a stable array and simplify intersection
- Always return an array (not undefined).
- Use Array.some instead of find in a filter to make intent clear.
Apply this diff:
export const getIntersectingMagList = ( annotation: APIAnnotation, dataset: APIDataset, groundTruthLayerName: string, imageDataLayerName: string, ) => { const colorLayers = getColorLayers(dataset); const dataLayerMags = getMagsForColorLayer(colorLayers, imageDataLayerName); + if (dataLayerMags.length === 0) return []; const segmentationLayer = getSegmentationLayerByHumanReadableName( dataset, annotation, groundTruthLayerName, ); const groundTruthLayerMags = getMagInfo(segmentationLayer.resolutions).getMagList(); - return groundTruthLayerMags?.filter((groundTruthMag) => - dataLayerMags?.find((mag) => V3.equals(mag, groundTruthMag)), - ); + return groundTruthLayerMags.filter((gtMag) => + dataLayerMags.some((mag) => V3.equals(mag, gtMag)), + ); };
1-2
: Optional: prefer importing RuleObject or drop the explicit typeIf you keep the explicit type for the validator’s first parameter, RuleObject is the more precise type in antd v5; otherwise omit it to avoid tight coupling to internal paths.
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_model_selector.tsx (6)
66-76
: Use a typed job state instead of a string literal.Avoid brittle string comparisons for job states.
Apply this diff:
- return models.filter( - (aiModel) => aiModel.trainingJob == null || aiModel.trainingJob.state === "SUCCESS", - ); + return models.filter( + (aiModel) => aiModel.trainingJob == null || aiModel.trainingJob.state === APIJobState.SUCCESS, + );Additionally, add the import:
-import { APIJobType, type AiModel } from "types/api_types"; +import { APIJobType, APIJobState, type AiModel } from "types/api_types";
125-129
: Use AntD v5 theme tokens; remove hard-coded colors.Unify primary color via tokens for consistent theming.
Apply these diffs:
- <ExperimentOutlined style={{ color: "#1890ff" }} /> + <ExperimentOutlined style={{ color: token.colorPrimary }} />- border: - selectedModel?.id === item.id - ? "1px solid var(--color-wk-blue)" - : "1px solid #d9d9d9", + border: + selectedModel?.id === item.id + ? `1px solid ${token.colorPrimary}` + : `1px solid ${token.colorBorder}`,- border: selectedModel?.id === item.id ? "1px solid #1890ff" : "1px solid #d9d9d9", + border: + selectedModel?.id === item.id + ? `1px solid ${token.colorPrimary}` + : `1px solid ${token.colorBorder}`,And add token usage:
-import { Avatar, Card, Input, List, Space, Spin, Tag, Typography } from "antd"; +import { Avatar, Card, Input, List, Space, Spin, Tag, Typography, theme } from "antd";Inside the component (after state declarations):
+ const { token } = theme.useToken();
Based on learnings
Also applies to: 146-156, 197-201
215-218
: Show specific category labels (NEURONS / NUCLEI / MITOCHONDRIA) for clarity.Improves UX and aligns with the expanded job types.
Apply this diff:
- <Tag> - {(item as AiModel).category === APIAiModelCategory.EM_NEURONS - ? "NEURONS" - : "INSTANCES"} - </Tag> + <Tag>{labelFromCategory((item as AiModel).category)}</Tag>Add this helper near mapCategoryToJobType:
const labelFromCategory = (category: APIAiModelCategory): string => { switch (category) { case APIAiModelCategory.EM_NEURONS: return "NEURONS"; case APIAiModelCategory.EM_NUCLEI: return "NUCLEI"; case APIAiModelCategory.EM_MITOCHONDRIA: return "MITOCHONDRIA"; default: return "UNKNOWN"; } };
145-159
: Add keyboard accessibility to clickable list items (pre-trained).Make items operable via keyboard.
Apply this diff:
- <List.Item + <List.Item style={{ border: selectedModel?.id === item.id ? "1px solid var(--color-wk-blue)" : "1px solid #d9d9d9", borderRadius: "8px", marginBottom: "16px", padding: "16px", opacity: item.disabled ? 0.5 : 1, cursor: item.disabled ? "not-allowed" : "pointer", }} className="hoverable-list-item" - onClick={() => !item.disabled && onSelectModel(item)} + role="button" + tabIndex={item.disabled ? -1 : 0} + onClick={() => !item.disabled && onSelectModel(item)} + onKeyDown={(e) => { + if (!item.disabled && (e.key === "Enter" || e.key === " ")) onSelectModel(item); + }} >
194-204
: Add keyboard accessibility to clickable list items (custom).Same as above for custom models.
Apply this diff:
- <List.Item + <List.Item className="hoverable-list-item" style={{ border: selectedModel?.id === item.id ? "1px solid #1890ff" : "1px solid #d9d9d9", borderRadius: "8px", marginBottom: "16px", padding: "16px", cursor: "pointer", }} - onClick={() => onSelectModel(item)} + role="button" + tabIndex={0} + onClick={() => onSelectModel(item)} + onKeyDown={(e) => { + if (e.key === "Enter" || e.key === " ") onSelectModel(item); + }} >
205-210
: Drop unnecessary non-null assertion.AiModel.name is a required string.
Apply this diff:
- <Avatar shape="square" size={64}> - {item.name!.charAt(0)} - </Avatar> + <Avatar shape="square" size={64}>{item.name.charAt(0)}</Avatar>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/automation/ai_training.md
(1 hunks)docs/automation/alignment.md
(1 hunks)frontend/javascripts/viewer/view/action_bar_view.tsx
(4 hunks)frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_model_selector.tsx
(1 hunks)frontend/javascripts/viewer/view/ai_jobs/utils.ts
(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-12T12:53:07.439Z
Learnt from: hotzenklotz
PR: scalableminds/webknossos#8262
File: docs/volume_annotation/tools.md:31-35
Timestamp: 2024-12-12T12:53:07.439Z
Learning: In `docs/volume_annotation/tools.md`, the SAM2 model used in the Quick Select tool predicts on several layers at once and is capable of understanding image sequences. The AI mode processes multiple layers at a time.
Applied to files:
docs/automation/ai_training.md
🧬 Code graph analysis (3)
frontend/javascripts/viewer/view/action_bar_view.tsx (1)
frontend/javascripts/viewer/model/actions/ui_actions.ts (1)
setAIJobDrawerStateAction
(136-140)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_model_selector.tsx (4)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_image_segmentation_job_context.tsx (1)
useRunAiModelJobContext
(276-282)frontend/javascripts/libs/react_helpers.tsx (1)
useGuardedFetch
(54-90)frontend/javascripts/admin/api/jobs.ts (1)
getAiModels
(443-449)frontend/javascripts/types/api_types.ts (1)
AiModel
(833-844)
frontend/javascripts/viewer/view/ai_jobs/utils.ts (4)
frontend/javascripts/viewer/view/ai_jobs/constants.ts (1)
MIN_BBOX_EXTENT
(24-35)frontend/javascripts/types/api_types.ts (3)
APIDataLayer
(114-114)APIAnnotation
(589-591)APIDataset
(243-246)frontend/javascripts/viewer/model.ts (1)
getColorLayers
(86-91)frontend/javascripts/viewer/model/accessors/volumetracing_accessor.ts (1)
getSegmentationLayerByHumanReadableName
(139-162)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-smoketest-push
- GitHub Check: backend-tests
🔇 Additional comments (9)
docs/automation/alignment.md (1)
6-7
: Replace “dialog” with “drawer” to match the current UIThe flow now opens a drawer, not a dialog, so the doc is incorrect.
-This will open a dialog where you can configure and start the alignment. +This opens a drawer where you can configure and start the alignment.docs/automation/ai_training.md (1)
25-28
: Update docs to match drawer-based training UI.The UI now opens the AI Jobs drawer with a “Model Training” tab, not a dialog. Please fix the wording to avoid confusing users.
-## Configuring the Training -To start a training, click on the `AI Analysis` button in the toolbar and select `Train AI model` from the dropdown menu. -This will open a dialog where you can configure and start your training job. +## Configuring the Training +To start training, click the `AI Analysis` button in the toolbar and choose `Train AI model`. +This opens the AI Jobs drawer on the `Model Training` tab where you can configure and start your training job.frontend/javascripts/viewer/view/ai_jobs/utils.ts (3)
10-11
: LGTM: moving to public accessors for color/segmentation helpersImports look correct and align with the new shared helper surface.
48-55
: LGTM: treat INFER_INSTANCES like MITO for mag selectionExtending the union and selecting finest mag for instances is consistent with the updated behavior.
Please confirm this mirrors worker behavior in voxelytics (select_mag_for_model_prediction) for instances to avoid UI/worker discrepancies.
120-125
: LGTM: consistent narrowing of segmentationType unionType narrowing aligns with supported inference types in size checks.
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_model_selector.tsx (4)
112-119
: Fix typos and clarify empty-state message; rename variable.Adjust copy and variable name for clarity.
Apply this diff:
- const switchToTraininButton = ( + const noCustomModelsMessage = ( <> - You don't have any custom models yet. Training custom model on your data is coming soon. + You don't have any custom models yet. Training custom models on your data is coming soon. {/* <Button onClick={() => dispatch(setAIJobDrawerStateAction("open_ai_training"))} type="link"> Train an AI Model on your data </Button> */} </> );Also update the usage:
- searchTerm.length > 0 ? "No models match your search." : switchToTraininButton, + searchTerm.length > 0 ? "No models match your search." : noCustomModelsMessage,
78-90
: Remove ts-ignore by narrowing types; drop redundant cast.Use AllowedJobType for jobType and remove the unnecessary category cast.
Apply this diff:
const onSelectModel = (model: AiModel | PretrainedModel) => { - let jobType: APIJobType; + let jobType: AllowedJobType; if ("category" in model) { - jobType = mapCategoryToJobType(model.category as APIAiModelCategory); + jobType = mapCategoryToJobType(model.category); } else { // Hard-coded, pre-trained models jobType = model.jobType; } setSelectedModel(model); - // @ts-ignore jobType covers move possible job type then expected for selectedJob setSelectedJobType(jobType); };
12-19
: Introduce AllowedJobType and narrow PretrainedModel.jobType.This removes unsafe widening and enables compile-time checks across the flow.
Apply this diff:
+type AllowedJobType = + | APIJobType.INFER_NEURONS + | APIJobType.INFER_NUCLEI + | APIJobType.INFER_MITOCHONDRIA + | APIJobType.INFER_INSTANCES; + type PretrainedModel = { name: string; comment: string; id: string; - jobType: APIJobType; + jobType: AllowedJobType; image: string; disabled?: boolean; };
123-124
: No changes required forCard type="inner
AntD v5.22.0 still supports thetype="inner"
prop onCard
.
const menuItems = [ | ||
{ | ||
key: "open_ai_inference_button", | ||
onClick: () => Store.dispatch(setAIJobDrawerStateAction("open_ai_inference")), | ||
label: "Run AI model", | ||
}, | ||
...(isSuperUser | ||
? [ | ||
{ | ||
key: "open_ai_training_button", | ||
onClick: () => Store.dispatch(setAIJobDrawerStateAction("open_ai_training")), | ||
label: "Train AI model", | ||
}, | ||
] | ||
: []), | ||
{ | ||
key: "open_ai_alignment_button", | ||
onClick: () => Store.dispatch(setAIJobDrawerStateAction("open_ai_alignment")), | ||
label: "Run AI Alignment", | ||
}, | ||
]; |
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.
Fix Dropdown item clicks (antd v5: onClick belongs on menu, not items).
Item-level onClick is ignored with Dropdown.menu in antd v5; clicks won't dispatch. Move handlers to menu.onClick and switch by key.
Apply this diff:
const isSuperUser = this.props.activeUser?.isSuperUser === true;
-const menuItems = [
- {
- key: "open_ai_inference_button",
- onClick: () => Store.dispatch(setAIJobDrawerStateAction("open_ai_inference")),
- label: "Run AI model",
- },
- ...(isSuperUser
- ? [
- {
- key: "open_ai_training_button",
- onClick: () => Store.dispatch(setAIJobDrawerStateAction("open_ai_training")),
- label: "Train AI model",
- },
- ]
- : []),
- {
- key: "open_ai_alignment_button",
- onClick: () => Store.dispatch(setAIJobDrawerStateAction("open_ai_alignment")),
- label: "Run AI Alignment",
- },
-];
+const menuItems = [
+ { key: "open_ai_inference_button", label: "Run AI model" },
+ ...(isSuperUser ? [{ key: "open_ai_training_button", label: "Train AI model" }] : []),
+ { key: "open_ai_alignment_button", label: "Run AI Alignment" },
+];
return (
<div>
<Dropdown
key="ai-job-dropdown"
menu={{
- items: menuItems,
+ items: menuItems,
+ onClick: ({ key }) => {
+ switch (key) {
+ case "open_ai_inference_button":
+ Store.dispatch(setAIJobDrawerStateAction("open_ai_inference"));
+ break;
+ case "open_ai_training_button":
+ Store.dispatch(setAIJobDrawerStateAction("open_ai_training"));
+ break;
+ case "open_ai_alignment_button":
+ Store.dispatch(setAIJobDrawerStateAction("open_ai_alignment"));
+ break;
+ }
+ },
}}
disabled={disabled}
>
Based on learnings
Also applies to: 323-327
🤖 Prompt for AI Agents
In frontend/javascripts/viewer/view/action_bar_view.tsx around lines 298-318
(and similarly lines 323-327), the current Dropdown menu items attach onClick
handlers directly to each item which is ignored by antd v5; instead add a single
menu.onClick handler on the Dropdown.menu prop and remove item-level onClick
attributes. Give each item a unique key (already present), build the items array
with only key and label (and optional danger/disabled), then implement
menu.onClick = ({ key }) => { switch(key) { case "open_ai_inference_button":
Store.dispatch(setAIJobDrawerStateAction("open_ai_inference")); break; case
"open_ai_training_button":
Store.dispatch(setAIJobDrawerStateAction("open_ai_training")); break; case
"open_ai_alignment_button":
Store.dispatch(setAIJobDrawerStateAction("open_ai_alignment")); break; } } and
pass that menu to the Dropdown; ensure typings for the event param are correct
and remove the per-item onClick code.
const mapCategoryToJobType = ( | ||
category: APIAiModelCategory, | ||
): APIJobType.INFER_NEURONS | APIJobType.INFER_INSTANCES => { | ||
switch (category) { | ||
case APIAiModelCategory.EM_NEURONS: | ||
return APIJobType.INFER_NEURONS; | ||
case APIAiModelCategory.EM_NUCLEI: | ||
return APIJobType.INFER_INSTANCES; | ||
default: | ||
throw new Error(`Unsupported category: ${category}`); | ||
} | ||
}; |
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.
Fix incorrect/incomplete category→jobType mapping (align with new JobCommand variants).
Map EM_NUCLEI to INFER_NUCLEI (not INSTANCES) and add EM_MITOCHONDRIA → INFER_MITOCHONDRIA to avoid wrong pipelines/costs when selecting custom models.
Apply this diff (assumes AllowedJobType is introduced per a separate comment):
-const mapCategoryToJobType = (
- category: APIAiModelCategory,
-): APIJobType.INFER_NEURONS | APIJobType.INFER_INSTANCES => {
+const mapCategoryToJobType = (
+ category: APIAiModelCategory,
+): AllowedJobType => {
switch (category) {
case APIAiModelCategory.EM_NEURONS:
return APIJobType.INFER_NEURONS;
case APIAiModelCategory.EM_NUCLEI:
- return APIJobType.INFER_INSTANCES;
+ return APIJobType.INFER_NUCLEI;
+ case APIAiModelCategory.EM_MITOCHONDRIA:
+ return APIJobType.INFER_MITOCHONDRIA;
default:
throw new Error(`Unsupported category: ${category}`);
}
};
📝 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.
const mapCategoryToJobType = ( | |
category: APIAiModelCategory, | |
): APIJobType.INFER_NEURONS | APIJobType.INFER_INSTANCES => { | |
switch (category) { | |
case APIAiModelCategory.EM_NEURONS: | |
return APIJobType.INFER_NEURONS; | |
case APIAiModelCategory.EM_NUCLEI: | |
return APIJobType.INFER_INSTANCES; | |
default: | |
throw new Error(`Unsupported category: ${category}`); | |
} | |
}; | |
const mapCategoryToJobType = ( | |
category: APIAiModelCategory, | |
): AllowedJobType => { | |
switch (category) { | |
case APIAiModelCategory.EM_NEURONS: | |
return APIJobType.INFER_NEURONS; | |
case APIAiModelCategory.EM_NUCLEI: | |
return APIJobType.INFER_NUCLEI; | |
case APIAiModelCategory.EM_MITOCHONDRIA: | |
return APIJobType.INFER_MITOCHONDRIA; | |
default: | |
throw new Error(`Unsupported category: ${category}`); | |
} | |
}; |
🤖 Prompt for AI Agents
In frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_model_selector.tsx
around lines 49 to 60, the switch maps EM_NUCLEI to APIJobType.INFER_INSTANCES
and omits EM_MITOCHONDRIA; update the mapping so EM_NEURONS -> INFER_NEURONS,
EM_NUCLEI -> INFER_NUCLEI, and add EM_MITOCHONDRIA -> INFER_MITOCHONDRIA; also
update the function return type (or allowed union) to include INFER_NUCLEI and
INFER_MITOCHONDRIA so types align with the new JobCommand variants and keep the
default error for unsupported categories.
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_model_selector.tsx
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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
♻️ Duplicate comments (1)
docs/automation/ai_training.md (1)
25-28
: Doc still references the old dialog UI.The new flow opens a drawer with a “Model Training” tab. Leaving “dialog” here misleads users. Please update the wording to match the drawer-based UI.
-## Configuring the Training -To start a training, click on the `AI Analysis` button in the toolbar and select `Train AI model` from the dropdown menu. -This will open a dialog where you can configure and start your training job. +## Configuring the Training +To start training, click the `AI Analysis` button in the toolbar and choose `Train AI model`. +This opens a drawer with a `Model Training` tab where you can configure and start your training job.
🧹 Nitpick comments (1)
docs/automation/ai_training.md (1)
56-58
: Fix the typo in the closing paragraph.“Find and overview” should be “find an overview.”
-Once the training is finished, you can find and overview of all your trained model from the `Admin` >`AI Models` page in the navbar. +Once the training is finished, you can find an overview of all your trained models on the `Admin` > `AI Models` page in the navbar.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/automation/ai_training.md
(1 hunks)docs/automation/alignment.md
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/automation/alignment.md
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
♻️ Duplicate comments (4)
frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts (4)
65-79
: Stop pushing to (userBoundingBoxes || []); mutate the actual arrayGuard above ensures userBoundingBoxes is an array, so drop the fallback to avoid mutating a temporary. Also return the variable directly.
- if (annotation.task?.boundingBox) { - const existingIds = (userBoundingBoxes || []).map(({ id }) => id); - const largestId = existingIds.length > 0 ? Math.max(...existingIds) : -1; - (userBoundingBoxes || []).push({ + if (annotation.task?.boundingBox) { + const existingIds = userBoundingBoxes.map(({ id }) => id); + const largestId = existingIds.length > 0 ? Math.max(...existingIds) : -1; + userBoundingBoxes.push({ name: "Task Bounding Box", boundingBox: Utils.computeBoundingBoxFromBoundingBoxObject(annotation.task.boundingBox), color: [0, 0, 0], isVisible: true, id: largestId + 1, }); } @@ - volumeTracingMags: volumeTracingMags, - userBoundingBoxes: userBoundingBoxes || [], + volumeTracingMags, + userBoundingBoxes,Also applies to: 80-90, 98-104
15-113
: Extract helpers to reduce cognitive loadSplit into small helpers: resolveInputsToAnnotationIds(inputs) and buildAnnotationBundle(annotationId). This improves readability and testability.
1-1
: File placement nit: not a React hookThis module isn't a hook. Consider moving from hooks/ to utils/ or api/.
21-27
: Robust URL parsing (handle query/fragment/trailing slash) and clearer var nameCurrent extraction breaks for URLs like ".../123?foo=bar" or with trailing "/". Decode and strip query/fragment. Also align naming with intent (annotationId).
Apply:
- if (taskOrAnnotationIdOrUrl.includes("/")) { - const lastSegment = taskOrAnnotationIdOrUrl.split("/").at(-1); - if (lastSegment) { - annotationIdsForTraining.push(lastSegment); - } - } else { + if (taskOrAnnotationIdOrUrl.includes("/")) { + // Extract annotation id from URL-like input (strip trailing slashes, query, fragment) + let lastPathSegment = ""; + try { + const u = new URL(taskOrAnnotationIdOrUrl); + lastPathSegment = u.pathname.replace(/\/+$/, "").split("/").at(-1) ?? ""; + } catch { + // Not an absolute URL; treat as path-like string + lastPathSegment = taskOrAnnotationIdOrUrl.replace(/\/+$/, "").split("/").at(-1) ?? ""; + } + const annotationId = lastPathSegment.split(/[?#]/, 1)[0]; + if (annotationId) { + annotationIdsForTraining.push(decodeURIComponent(annotationId)); + } else { + Toast.warning(`Could not extract annotation id from "${taskOrAnnotationIdOrUrl}"`); + } + } else {
🧹 Nitpick comments (5)
frontend/javascripts/viewer/view/ai_jobs/utils.ts (1)
325-343
: Wrap getIntersectingMagList call in ai_training_data_selector.tsx in try/catchgetIntersectingMagList invokes getSegmentationLayerByHumanReadableName, which throws if the layer isn’t found; handling this prevents uncaught errors in the component.
frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts (4)
33-34
: Avoid magic string for annotation stateReplace "Finished" with a shared enum/constant if available to prevent drift.
Do we have a central AnnotationState/TaskState enum in types? If yes, import and use it here.
48-50
: Deduplicate annotation IDs before fetching to avoid duplicate requests/UI entriesUse a Set to dedupe.
- const newAnnotationsWithDatasets = await Promise.all( - annotationIdsForTraining.map(async (annotationId) => { + const uniqueAnnotationIdsForTraining = Array.from(new Set(annotationIdsForTraining)); + const newAnnotationsWithDatasets = await Promise.all( + uniqueAnnotationIdsForTraining.map(async (annotationId) => {
53-60
: Use enum instead of string literal for layer type + parallelize dataset and tracing fetches
- Prefer AnnotationLayerEnum.Volume for consistency (you already use the enum for Skeleton).
- Fetch dataset and tracings concurrently to reduce latency.
- const dataset = await getDataset(annotation.datasetId); - - const volumeServerTracings: ServerVolumeTracing[] = await Promise.all( - annotation.annotationLayers - .filter((layer) => layer.typ === "Volume") - .map( - (layer) => - getTracingForAnnotationType(annotation, layer) as Promise<ServerVolumeTracing>, - ), - ); + const [dataset, volumeServerTracings] = await Promise.all([ + getDataset(annotation.datasetId), + Promise.all( + annotation.annotationLayers + .filter((layer) => layer.typ === AnnotationLayerEnum.Volume) + .map( + (layer) => + getTracingForAnnotationType(annotation, layer) as Promise<ServerVolumeTracing>, + ), + ), + ]);
19-20
: De-dup unfinished tasks to avoid repeated names in the toastUse Set to collect, then spread on display.
- const unfinishedTasks: string[] = []; + const unfinishedTasks = new Set<string>(); @@ - unfinishedTasks.push(taskOrAnnotationIdOrUrl); + unfinishedTasks.add(taskOrAnnotationIdOrUrl); @@ - if (unfinishedTasks.length > 0) { - Toast.warning( - `The following tasks have no finished annotations: ${unfinishedTasks.join(", ")}`, - ); + if (unfinishedTasks.size > 0) { + Toast.warning( + `The following tasks have no finished annotations: ${[...unfinishedTasks].join(", ")}`, + ); }Also applies to: 37-38, 107-111
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
frontend/javascripts/types/api_types.ts
(1 hunks)frontend/javascripts/viewer/view/action-bar/create_animation_modal.tsx
(1 hunks)frontend/javascripts/viewer/view/action-bar/download_modal_view.tsx
(1 hunks)frontend/javascripts/viewer/view/ai_jobs/components/bounding_box_selection_form_item.tsx
(1 hunks)frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts
(1 hunks)frontend/javascripts/viewer/view/ai_jobs/utils.ts
(5 hunks)frontend/javascripts/viewer/view/left-border-tabs/layer_settings_tab.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/javascripts/viewer/view/action-bar/create_animation_modal.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/javascripts/types/api_types.ts
- frontend/javascripts/viewer/view/ai_jobs/components/bounding_box_selection_form_item.tsx
- frontend/javascripts/viewer/view/action-bar/download_modal_view.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts (7)
frontend/javascripts/viewer/view/ai_jobs/utils.ts (1)
AnnotationInfoForAITrainingJob
(141-147)frontend/javascripts/types/api_types.ts (2)
APIAnnotation
(589-591)ServerVolumeTracing
(974-1001)frontend/javascripts/admin/api/tasks.ts (1)
getAnnotationsForTask
(29-34)frontend/javascripts/admin/rest_api.ts (3)
getUnversionedAnnotationInformation
(618-629)getDataset
(1032-1043)getTracingForAnnotationType
(740-781)frontend/javascripts/viewer/model/reducers/volumetracing_reducer.ts (1)
serverVolumeToClientVolumeTracing
(268-324)frontend/javascripts/viewer/model/reducers/reducer_helpers.ts (1)
convertUserBoundingBoxesFromServerToFrontend
(68-85)frontend/javascripts/viewer/constants.ts (1)
Vector3
(14-14)
frontend/javascripts/viewer/view/ai_jobs/utils.ts (3)
frontend/javascripts/viewer/view/ai_jobs/constants.ts (1)
MIN_BBOX_EXTENT
(24-35)frontend/javascripts/types/api_types.ts (3)
APIDataLayer
(114-114)APIAnnotation
(589-591)APIDataset
(243-246)frontend/javascripts/viewer/model/accessors/volumetracing_accessor.ts (1)
getSegmentationLayerByHumanReadableName
(139-162)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-smoketest-push
- GitHub Check: frontend-tests
- GitHub Check: backend-tests
🔇 Additional comments (5)
frontend/javascripts/viewer/view/left-border-tabs/layer_settings_tab.tsx (1)
127-127
: Import path update verified
The file exists at the new location and no imports reference the oldai_job_modals/materialize_volume_annotation_modal
path.frontend/javascripts/viewer/view/ai_jobs/utils.ts (4)
17-28
: LGTM! Previous review feedback addressed.The addition of
INFER_INSTANCES
case and the default error throw correctly address the previous review comments. The grouping withINFER_NEURONS
andINFER_NUCLEI
follows the suggested pattern, andMIN_BBOX_EXTENT
includes the necessary entry forINFER_INSTANCES
.
45-87
: LGTM! Consistent INFER_INSTANCES support.The function signature correctly extends
jobType
to includeINFER_INSTANCES
, and the logic appropriately groups it withINFER_MITOCHONDRIA
to select the finest magnification. This is consistent with the comment indicating these models always infer on the finest mag.
89-139
: LGTM! Type signatures correctly updated.Both
isBBoxTooSmall
andisDatasetOrBoundingBoxTooSmall
have theirsegmentationType
parameters appropriately extended to includeINFER_INSTANCES
. The underlying logic remains sound and will work correctly with the new job type.
307-318
: LGTM! Well-structured form validator.The
colorLayerMustNotBeUint24Rule
follows antd's validator pattern correctly. The null safety check, validation logic, and error message are all appropriate. This will integrate cleanly with antd Form components.
…sis_settings.tsx Co-authored-by: Philipp Otto <[email protected]>
…nto new_ai_job_ui
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: 13
♻️ Duplicate comments (12)
frontend/javascripts/viewer/view/action_bar_view.tsx (1)
298-334
: Critical issue from previous review remains unfixed: onClick handlers on menu items are ignored in antd v5.The onClick handlers on individual menu items (lines 301, 308, 315) are ignored by antd v5's Dropdown component. The AI Analysis dropdown buttons will not work. This was flagged as a critical issue in the previous review and the fix was not applied.
Apply the previously suggested fix by moving onClick logic to
menu.onClick
:const isSuperUser = this.props.activeUser?.isSuperUser === true; const menuItems = [ - { - key: "open_ai_inference_button", - onClick: () => Store.dispatch(setAIJobDrawerStateAction("open_ai_inference")), - label: "Run AI model", - }, - ...(isSuperUser - ? [ - { - key: "open_ai_training_button", - onClick: () => Store.dispatch(setAIJobDrawerStateAction("open_ai_training")), - label: "Train new AI model", - }, - ] - : []), - { - key: "open_ai_alignment_button", - onClick: () => Store.dispatch(setAIJobDrawerStateAction("open_ai_alignment")), - label: "Run AI Alignment", - }, + { key: "open_ai_inference_button", label: "Run AI model" }, + ...(isSuperUser ? [{ key: "open_ai_training_button", label: "Train new AI model" }] : []), + { key: "open_ai_alignment_button", label: "Run AI Alignment" }, ]; return ( <div> <Dropdown key="ai-job-dropdown" menu={{ items: menuItems, + onClick: ({ key }) => { + switch (key) { + case "open_ai_inference_button": + Store.dispatch(setAIJobDrawerStateAction("open_ai_inference")); + break; + case "open_ai_training_button": + Store.dispatch(setAIJobDrawerStateAction("open_ai_training")); + break; + case "open_ai_alignment_button": + Store.dispatch(setAIJobDrawerStateAction("open_ai_alignment")); + break; + } + }, }} disabled={disabled} > <Button disabled={disabled} icon={<i className="fas fa-magic" />} title={tooltipText}> AI Analysis </Button> </Dropdown> </div> );Based on learnings
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx (3)
41-44
: Add null checks for colorLayer to prevent runtime errors.If a dataset has no color layers,
colorLayers[0]
will beundefined
, causinggetBoundingBoxesForLayers([colorLayer])[0]
to fail andcolorLayer.name
(line 61) to throw. This issue was previously flagged but remains unresolved.Apply this diff to add guards:
const dataset = useWkSelector((state) => state.dataset); const annotationId = useWkSelector((state) => state.annotation.annotationId); const colorLayers = getColorLayers(dataset); - const colorLayer = colorLayers[0]; + const colorLayer = colorLayers[0] ?? null; - const selectedBoundingBox = getBoundingBoxesForLayers([colorLayer])[0]; + const selectedBoundingBox = colorLayer ? getBoundingBoxesForLayers([colorLayer])[0] : null;
52-55
: Update validation to include colorLayer check.The validation doesn't check whether
colorLayer
exists, buthandleStartAnalysis
requires it. This will allow the Start button to be enabled even when the job will fail.Apply this diff:
const areParametersValid = useMemo( - () => Boolean(selectedTask && selectedJobType && newDatasetName), - [selectedTask, selectedJobType, newDatasetName], + () => Boolean(selectedTask && selectedJobType && newDatasetName && colorLayer), + [selectedTask, selectedJobType, newDatasetName, colorLayer], );
57-71
: Add early return guard for invalid parameters.The handler should verify parameters before attempting the API call. Without this guard, the function will crash on
colorLayer.name
whencolorLayer
isnull
.Apply this diff:
const handleStartAnalysis = useCallback(async () => { + if (!areParametersValid || !colorLayer) { + Toast.error("Please ensure all required parameters are set and a color layer exists."); + return; + } try { await startAlignSectionsJob( dataset.id, - colorLayer.name, + colorLayer!.name, newDatasetName, shouldUseManualMatches ? annotationId : undefined, ); Toast.success("Alignment started successfully!"); dispatch(setAIJobDrawerStateAction("invisible")); } catch (error) { console.error(error); Toast.error("Failed to start alignment."); } - }, [dataset.id, dispatch, colorLayer.name, newDatasetName, annotationId, shouldUseManualMatches]); + }, [dataset.id, dispatch, colorLayer, newDatasetName, annotationId, shouldUseManualMatches, areParametersValid]);frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_analysis_settings.tsx (3)
149-158
: Type‑narrow before mag/size validation.Guard to inference job types to avoid invalid calls.
- if (value && selectedLayer && selectedJobType) { + if ( + value && + selectedLayer && + (selectedJobType === APIJobType.INFER_MITOCHONDRIA || + selectedJobType === APIJobType.INFER_NEURONS || + selectedJobType === APIJobType.INFER_NUCLEI || + selectedJobType === APIJobType.INFER_INSTANCES) + ) {
61-65
: Fix layer change handler: Select returns a string name.- if ("selectedLayer" in changedValues) { - setSelectedLayer( - colorLayers.find((l) => l.name === changedValues.selectedLayer.name) as APIDataLayer, - ); - } + if ("selectedLayer" in changedValues) { + const name = changedValues.selectedLayer as string; + setSelectedLayer(colorLayers.find((l) => l.name === name) as APIDataLayer); + }
127-139
: Make uint24 validation effective for Select.The field stores a layer name (string). Validate against the resolved selectedLayer.
<Form.Item name="selectedLayer" label="Image Data Layer" rules={[ { required: true, message: "Please select an image data layer" }, - colorLayerMustNotBeUint24Rule, + { + validator: () => { + if (selectedLayer && (selectedLayer as any).elementClass === "uint24") { + return Promise.reject( + new Error( + "The selected layer of type uint24 is not supported. Please select a different one.", + ), + ); + } + return Promise.resolve(); + }, + }, ]} >docs/automation/ai_training.md (1)
26-28
: Update “dialog” to “drawer” and reference the tab.The new UX is a drawer with a “Model Training” tab; adjust copy accordingly.
-## Configuring the Training -To start a training, click on the `AI Analysis` button in the toolbar and select `Train AI model` from the dropdown menu. -This will open a dialog where you can configure and start your training job. +## Configuring the Training +To start training, click the `AI Analysis` button in the toolbar and select `Train AI model`. +This opens a drawer with a `Model Training` tab where you can configure and start your training job.frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_model_selector.tsx (1)
54-65
: Fix category→jobType mapping; include NUCLEI and MITOCHONDRIA.EM_NUCLEI maps to INFER_NUCLEI (not INSTANCES). Also add EM_MITOCHONDRIA → INFER_MITOCHONDRIA to avoid wrong pipelines/costs and errors on selection.
-const mapCategoryToJobType = ( - category: APIAiModelCategory, -): APIJobType.INFER_NEURONS | APIJobType.INFER_INSTANCES => { +const mapCategoryToJobType = ( + category: APIAiModelCategory, +): AllowedJobType => { switch (category) { case APIAiModelCategory.EM_NEURONS: return APIJobType.INFER_NEURONS; case APIAiModelCategory.EM_NUCLEI: - return APIJobType.INFER_INSTANCES; + return APIJobType.INFER_NUCLEI; + case APIAiModelCategory.EM_MITOCHONDRIA: + return APIJobType.INFER_MITOCHONDRIA; default: throw new Error(`Unsupported category: ${category}`); } };Also update types where used:
Type alias (near Lines 13-24):
type AllowedJobType = | APIJobType.INFER_NEURONS | APIJobType.INFER_NUCLEI | APIJobType.INFER_MITOCHONDRIA | APIJobType.INFER_INSTANCES; type PretrainedModel = { /* ... */ jobType: AllowedJobType; /* ... */ };onSelectModel (Lines 84-88):
let jobType: AllowedJobType;frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (3)
11-11
: Remove lodash/every import; use native checks.Drop the dependency and avoid truthiness pitfalls.
-import every from "lodash/every";
78-82
: Avoid non-null assertion in queryFn.Read id from queryKey to drop the unsafe "!".
- const { data: initialFullAnnotation } = useQuery({ - queryKey: ["initialAnnotation", currentAnnotation.annotationId], - queryFn: () => getUnversionedAnnotationInformation(currentAnnotation.annotationId!), - enabled: !!currentAnnotation.annotationId, - }); + const { data: initialFullAnnotation } = useQuery({ + queryKey: ["initialAnnotation", currentAnnotation.annotationId], + queryFn: ({ queryKey }) => getUnversionedAnnotationInformation(queryKey[1] as string), + enabled: Boolean(currentAnnotation.annotationId), + });
124-131
: Replace fragile validity check; accept enum 0 and validate Vector3.Current check can misclassify valid values and blocks TS narrowing. Use explicit checks and a type guard.
- const areParametersValid = useMemo(() => { - const areSelectionsValid = every( - selectedAnnotations, - (s) => s.imageDataLayer && s.groundTruthLayer && s.magnification, - ); - - return every([modelName, selectedJobType, areSelectionsValid, selectedAnnotations.length > 0]); - }, [modelName, selectedJobType, selectedAnnotations]); + const areParametersValid = useMemo(() => { + const hasAtLeastOne = selectedAnnotations.length > 0; + const areSelectionsValid = selectedAnnotations.every(isCompleteSelection); + return modelName.trim().length > 0 && selectedJobType != null && hasAtLeastOne && areSelectionsValid; + }, [modelName, selectedJobType, selectedAnnotations]);Add near the interfaces:
type CompleteSelection = AiTrainingAnnotationSelection & Required<Pick<AiTrainingAnnotationSelection, "imageDataLayer" | "groundTruthLayer" | "magnification">>; const isVector3 = (v: unknown): v is Vector3 => Array.isArray(v) && v.length === 3 && v.every((n) => typeof n === "number" && Number.isFinite(n)); const isCompleteSelection = (s: AiTrainingAnnotationSelection): s is CompleteSelection => typeof s.imageDataLayer === "string" && s.imageDataLayer.length > 0 && typeof s.groundTruthLayer === "string" && s.groundTruthLayer.length > 0 && isVector3(s.magnification);Based on learnings
🧹 Nitpick comments (4)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_model_selector.tsx (1)
224-228
: Label custom model categories precisely.Show NUCLEI and MITOCHONDRIA tags instead of generic INSTANCES.
- <Tag> - {(item as AiModel).category === APIAiModelCategory.EM_NEURONS - ? "NEURONS" - : "INSTANCES"} - </Tag> + <Tag> + {(item as AiModel).category === APIAiModelCategory.EM_NEURONS + ? "NEURONS" + : (item as AiModel).category === APIAiModelCategory.EM_NUCLEI + ? "NUCLEI" + : (item as AiModel).category === APIAiModelCategory.EM_MITOCHONDRIA + ? "MITOCHONDRIA" + : "INSTANCES"} + </Tag>frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_analysis_settings.tsx (1)
191-197
: Avoid defaultValue on a controlled Form field.Remove defaultValue to prevent controlled/uncontrolled mismatch.
- <InputNumber + <InputNumber min={0.1} suffix="nm" style={{ width: "100%" }} - defaultValue={1000} disabled={!isInstanceModel} />frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (2)
148-152
: Typo: rename commonJobArgmuments → commonJobArguments.Small clarity tweak.
- const commonJobArgmuments = { + const commonJobArguments = { trainingAnnotations: trainingAnnotations, name: modelName, comment: comments, }; @@ - ...commonJobArgmuments, + ...commonJobArguments, @@ - ...commonJobArgmuments, + ...commonJobArguments,Also applies to: 159-159, 165-165
183-188
: Expose setters with the declared narrower signatures.Wrap useState dispatchers to match AiTrainingJobContextType and avoid passing the broader SetStateAction signature.
const value = { selectedJobType, selectedTask, - setSelectedJobType, - setSelectedTask, + setSelectedJobType: setSelectedJobTypeValue, + setSelectedTask: setSelectedTaskValue, handleStartAnalysis,Add near the other callbacks:
const setSelectedJobTypeValue = useCallback((jobType: APIJobType) => setSelectedJobType(jobType), []); const setSelectedTaskValue = useCallback((task: AiTrainingTask) => setSelectedTask(task), []);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
docs/automation/ai_training.md
(1 hunks)frontend/javascripts/components/stacked_bar_chart.tsx
(2 hunks)frontend/javascripts/viewer/view/action_bar_view.tsx
(4 hunks)frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx
(1 hunks)frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_model_selector.tsx
(1 hunks)frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_settings.tsx
(1 hunks)frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts
(1 hunks)frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_analysis_settings.tsx
(1 hunks)frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_model_selector.tsx
(1 hunks)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_data_selector.tsx
(1 hunks)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx
(1 hunks)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx
(1 hunks)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_settings.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts
- frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_settings.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-27T11:32:54.033Z
Learnt from: knollengewaechs
PR: scalableminds/webknossos#8796
File: frontend/javascripts/viewer/view/action-bar/ai_job_modals/forms/neuron_segmentation_form.tsx:68-77
Timestamp: 2025-08-27T11:32:54.033Z
Learning: In the webknossos frontend AI job forms, `datasetConfiguration.layers[colorLayer.name]` is assumed to always be non-null when accessing layer properties like `isInverted`. The selected color layer is guaranteed to exist in the dataset configuration.
Applied to files:
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx
🧬 Code graph analysis (10)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_model_selector.tsx (5)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_image_segmentation_job_context.tsx (1)
useRunAiModelJobContext
(276-282)frontend/javascripts/libs/react_helpers.tsx (1)
useGuardedFetch
(54-90)frontend/javascripts/admin/api/jobs.ts (1)
getAiModels
(443-449)frontend/javascripts/types/api_types.ts (1)
AiModel
(833-844)frontend/javascripts/theme.tsx (1)
ColorWKBlue
(12-12)
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_model_selector.tsx (2)
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx (1)
useAlignmentJobContext
(90-96)frontend/javascripts/theme.tsx (1)
ColorWKBlue
(12-12)
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_settings.tsx (3)
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx (1)
useAlignmentJobContext
(90-96)frontend/javascripts/theme.tsx (1)
ColorWKBlue
(12-12)frontend/javascripts/viewer/view/ai_jobs/components/should_use_trees_form_item.tsx (1)
ShouldUseManualMatchesFormItem
(7-61)
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_data_selector.tsx (6)
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (2)
AiTrainingAnnotationSelection
(22-29)useAiTrainingJobContext
(204-210)frontend/javascripts/viewer/view/ai_jobs/utils.ts (2)
getIntersectingMagList
(325-343)colorLayerMustNotBeUint24Rule
(307-318)frontend/javascripts/libs/utils.ts (1)
computeVolumeFromBoundingBox
(314-317)frontend/javascripts/libs/format_utils.ts (1)
formatVoxels
(515-538)frontend/javascripts/theme.tsx (1)
ColorWKBlue
(12-12)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/annotations_csv_input.tsx (1)
AnnotationsCsvInput
(8-79)
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (9)
frontend/javascripts/types/api_types.ts (2)
APIAnnotation
(589-591)APIDataset
(243-246)frontend/javascripts/viewer/constants.ts (1)
Vector3
(14-14)frontend/javascripts/viewer/store.ts (1)
UserBoundingBox
(108-110)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx (1)
AiTrainingTask
(11-18)frontend/javascripts/libs/react_hooks.ts (1)
useWkSelector
(292-294)frontend/javascripts/viewer/model/accessors/tracing_accessor.ts (1)
getUserBoundingBoxesFromState
(98-101)frontend/javascripts/admin/rest_api.ts (1)
getUnversionedAnnotationInformation
(618-629)frontend/javascripts/admin/api/jobs.ts (3)
AiModelTrainingAnnotationSpecification
(371-376)runInstanceModelTraining
(402-407)runNeuronTraining
(386-391)frontend/javascripts/viewer/model/actions/ui_actions.ts (1)
setAIJobDrawerStateAction
(136-140)
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx (2)
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (1)
useAiTrainingJobContext
(204-210)frontend/javascripts/theme.tsx (1)
ColorWKBlue
(12-12)
frontend/javascripts/viewer/view/action_bar_view.tsx (1)
frontend/javascripts/viewer/model/actions/ui_actions.ts (1)
setAIJobDrawerStateAction
(136-140)
frontend/javascripts/components/stacked_bar_chart.tsx (1)
frontend/javascripts/viewer/model/helpers/iterator_utils.ts (1)
sum
(29-31)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_analysis_settings.tsx (10)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_image_segmentation_job_context.tsx (1)
useRunAiModelJobContext
(276-282)frontend/javascripts/libs/react_hooks.ts (1)
useWkSelector
(292-294)frontend/javascripts/types/api_types.ts (1)
APIDataLayer
(114-114)frontend/javascripts/viewer/view/ai_jobs/components/collapsible_split_merger_evaluation_settings.tsx (2)
SplitMergerEvaluationSettings
(4-9)CollapsibleSplitMergerEvaluationSettings
(11-90)frontend/javascripts/theme.tsx (1)
ColorWKBlue
(12-12)frontend/javascripts/admin/dataset/dataset_components.tsx (1)
getDatasetNameRules
(68-85)frontend/javascripts/viewer/view/ai_jobs/utils.ts (3)
colorLayerMustNotBeUint24Rule
(307-318)getBestFittingMagComparedToTrainingDS
(45-87)isDatasetOrBoundingBoxTooSmall
(116-139)frontend/javascripts/viewer/store.ts (1)
UserBoundingBox
(108-110)frontend/javascripts/libs/utils.ts (1)
computeArrayFromBoundingBox
(299-308)frontend/javascripts/viewer/view/ai_jobs/bounding_box_selector.tsx (1)
BoundingBoxSelector
(15-38)
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx (7)
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_model_selector.tsx (1)
AlignmentTask
(11-18)frontend/javascripts/viewer/store.ts (1)
UserBoundingBox
(108-110)frontend/javascripts/libs/react_hooks.ts (1)
useWkSelector
(292-294)frontend/javascripts/viewer/view/ai_jobs/utils.ts (1)
getBoundingBoxesForLayers
(30-40)frontend/javascripts/admin/api/jobs.ts (1)
startAlignSectionsJob
(344-363)frontend/javascripts/viewer/model/actions/ui_actions.ts (1)
setAIJobDrawerStateAction
(136-140)frontend/javascripts/libs/toast.tsx (1)
error
(211-218)
🪛 GitHub Check: frontend-tests
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx
[failure] 139-139:
Type '{ annotationId: string; colorLayerName: string | undefined; segmentationLayerName: string | undefined; mag: Vector3 | undefined; }[]' is not assignable to type 'AiModelTrainingAnnotationSpecification[]'.
🪛 LanguageTool
docs/automation/ai_training.md
[style] ~7-~7: For conciseness, consider replacing this expression with an adverb.
Context: ...webknossos.org](https://webknossos.org) at the moment. If you want to set up on-premise ...
(AT_THE_MOMENT)
[grammar] ~46-~46: There might be a mistake here.
Context: ... Give your new model a descriptive name. * Comments: Add any comments or notes ab...
(QB_NEW_EN)
[grammar] ~58-~58: Ensure spelling is correct
Context: ... the training is finished, you can find and overview of all your trained model from...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~58-~58: There might be a mistake here.
Context: ...all your trained model from the Admin
>AI Models
page in the navbar. Please refe...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-smoketest-push
- GitHub Check: backend-tests
🔇 Additional comments (2)
frontend/javascripts/components/stacked_bar_chart.tsx (1)
1-1
: LGTM! Modular import improves bundle size.The switch from the default lodash import to a specific function import (
lodash/sum
) is a recommended best practice that enables tree-shaking and reduces bundle size. The usage is updated correctly on line 20.Based on learnings
Also applies to: 20-20
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_settings.tsx (1)
10-55
: LGTM!The alignment settings component is well-structured with proper context integration, controlled form state, and appropriate validation rules. The two-column layout cleanly separates dataset naming from manual match configuration.
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_model_selector.tsx
Outdated
Show resolved
Hide resolved
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_analysis_settings.tsx
Outdated
Show resolved
Hide resolved
label="Magnification" | ||
required | ||
rules={[{ required: true, message: "Please select a magnification" }]} | ||
validateStatus={magnificationValidationError ? "error" : undefined} | ||
help={magnificationValidationError} | ||
> |
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.
Add name
for Magnification so required rule can trigger.
- <Form.Item
+ <Form.Item
+ name={["annotations", annotationId, "magnification"]}
label="Magnification"
required
rules={[{ required: true, message: "Please select a magnification" }]}
validateStatus={magnificationValidationError ? "error" : undefined}
help={magnificationValidationError}
>
📝 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.
label="Magnification" | |
required | |
rules={[{ required: true, message: "Please select a magnification" }]} | |
validateStatus={magnificationValidationError ? "error" : undefined} | |
help={magnificationValidationError} | |
> | |
<Form.Item | |
name={["annotations", annotationId, "magnification"]} | |
label="Magnification" | |
required | |
rules={[{ required: true, message: "Please select a magnification" }]} | |
validateStatus={magnificationValidationError ? "error" : undefined} | |
help={magnificationValidationError} | |
> |
🤖 Prompt for AI Agents
In
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_data_selector.tsx
around lines 184-189, the Form.Item for "Magnification" is missing the name prop
so the required validation rule won't trigger; add a name="magnification" (or
whatever form field key is used elsewhere for magnification) to the Form.Item so
the Ant Design form can wire up and enforce the required rule, keeping existing
rules, validateStatus and help as-is.
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx
Outdated
Show resolved
Hide resolved
const trainingAnnotations: AiModelTrainingAnnotationSpecification[] = selectedAnnotations.map( | ||
(selection) => ({ | ||
annotationId: selection.annotation.id, | ||
colorLayerName: selection.imageDataLayer, | ||
segmentationLayerName: selection.groundTruthLayer, | ||
mag: selection.magnification, | ||
}), | ||
); |
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.
Fix TS error: map only complete selections to satisfy AiModelTrainingAnnotationSpecification.
Use the type guard to ensure all fields are defined.
- const trainingAnnotations: AiModelTrainingAnnotationSpecification[] = selectedAnnotations.map(
- (selection) => ({
- annotationId: selection.annotation.id,
- colorLayerName: selection.imageDataLayer,
- segmentationLayerName: selection.groundTruthLayer,
- mag: selection.magnification,
- }),
- );
+ const completeSelections = selectedAnnotations.filter(isCompleteSelection);
+ const trainingAnnotations: AiModelTrainingAnnotationSpecification[] = completeSelections.map(
+ (selection) => ({
+ annotationId: selection.annotation.id,
+ colorLayerName: selection.imageDataLayer,
+ segmentationLayerName: selection.groundTruthLayer,
+ mag: selection.magnification,
+ }),
+ );
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: frontend-tests
[failure] 139-139:
Type '{ annotationId: string; colorLayerName: string | undefined; segmentationLayerName: string | undefined; mag: Vector3 | undefined; }[]' is not assignable to type 'AiModelTrainingAnnotationSpecification[]'.
🤖 Prompt for AI Agents
In
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx
around lines 139-146, the current map over selectedAnnotations can produce
undefined fields and fails the AiModelTrainingAnnotationSpecification type; add
a type-guarded filter step (or inline filter using a user-defined type guard)
that ensures selection.annotation and selection.annotation.id,
selection.imageDataLayer, selection.groundTruthLayer, and
selection.magnification are all defined, then map the filtered array to produce
AiModelTrainingAnnotationSpecification objects—this guarantees the compiler that
all required fields are present and keeps the resulting array typed correctly.
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx
Outdated
Show resolved
Hide resolved
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx
Outdated
Show resolved
Hide resolved
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: 1
♻️ Duplicate comments (1)
frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts (1)
20-21
: Clarify support for "task URLs" in documentation.The function comment claims to parse "task IDs, annotation IDs, or annotation URLs," but past review discussions indicate that task URLs don't exist in the system. The current implementation only handles annotation URLs (lines 27-31) by extracting the annotation ID from the path. Consider updating the JSDoc and parameter name to accurately reflect this limitation.
🧹 Nitpick comments (2)
frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts (2)
27-31
: Strengthen URL parsing to handle edge cases.The current split-by-slash approach may fail if the URL contains trailing slashes, query parameters, or fragment identifiers. A URL ending in
/#
or including?param=value
could produce an incorrect or empty annotation ID.Consider using the URL API for more robust parsing:
if (taskOrAnnotationIdOrUrl.includes("/")) { - const annotationId = taskOrAnnotationIdOrUrl.split("/").at(-1); + try { + const url = new URL(taskOrAnnotationIdOrUrl, window.location.origin); + const annotationId = url.pathname.split("/").filter(Boolean).at(-1); + } catch { + // If URL parsing fails, fall back to simple split + const annotationId = taskOrAnnotationIdOrUrl.split("/").filter(Boolean).at(-1); + } if (annotationId) { annotationIdsForTraining.push(annotationId); }
86-120
: LGTM! Consider clarifying the comment about the original bug.The bounding box extraction logic is solid: it correctly handles empty volume tracings via optional chaining, falls back to skeleton layers when needed, throws an appropriate error if neither exists, and properly appends the task bounding box.
However, the comment on line 93 references "the original code had a bug" which might confuse future readers since the bug is no longer present.
Consider updating the comment to be more forward-looking:
- // The original code had a bug here (!userBoundingBoxes) which was always false for an empty array. + // Check if the array is empty (not just falsy) const skeletonLayer = annotation.annotationLayers.find(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts (8)
frontend/javascripts/admin/api/tasks.ts (1)
getAnnotationsForTask
(29-34)frontend/javascripts/types/api_types.ts (2)
APIAnnotation
(589-591)ServerVolumeTracing
(974-1001)frontend/javascripts/admin/rest_api.ts (3)
getTracingForAnnotationType
(740-781)getUnversionedAnnotationInformation
(618-629)getDataset
(1032-1043)frontend/javascripts/viewer/constants.ts (1)
Vector3
(14-14)frontend/javascripts/viewer/store.ts (1)
VolumeTracing
(192-213)frontend/javascripts/viewer/model/reducers/reducer_helpers.ts (1)
convertUserBoundingBoxesFromServerToFrontend
(68-85)frontend/javascripts/viewer/view/ai_jobs/utils.ts (1)
AnnotationInfoForAITrainingJob
(141-147)frontend/javascripts/viewer/model/reducers/volumetracing_reducer.ts (1)
serverVolumeToClientVolumeTracing
(268-324)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-smoketest-push
- GitHub Check: backend-tests
🔇 Additional comments (4)
frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts (4)
55-67
: LGTM!The function correctly filters volume layers and fetches their tracings concurrently.
69-79
: LGTM!The magnification extraction correctly handles both legacy annotations (without
mags
) and modern ones, defaulting to[1,1,1]
as expected.
122-150
: LGTM!The function properly orchestrates fetching and transforming all required annotation data with appropriate parallelization.
158-181
: LGTM!The main exported function orchestrates the complete workflow effectively: resolving IDs, parallel fetching, user warnings for unfinished tasks, and comprehensive error handling with console logging and toast notifications. Returning an empty array on error is reasonable since
Promise.all
means partial success isn't possible.
frontend/javascripts/viewer/view/ai_jobs/hooks/fetch_annotation_infos.ts
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…sis_settings.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…nto new_ai_job_ui
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: 1
♻️ Duplicate comments (8)
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (2)
11-11
: Replace lodashevery
with explicit validation.The lodash
every
call is fragile because it relies on truthiness, which can incorrectly reject valid falsy values (e.g.,0
for an enum). Additionally,every([])
returnstrue
, though you mitigate this separately. Use explicit boolean checks for clarity and correctness.Apply this diff:
-import every from "lodash/every"; import type React from "react";
const areParametersValid = useMemo(() => { - const areSelectionsValid = every( - selectedAnnotations, - (s) => s.imageDataLayer && s.groundTruthLayer && s.magnification, - ); - - return every([modelName, selectedJobType, areSelectionsValid, selectedAnnotations.length > 0]); + const areSelectionsValid = + selectedAnnotations.length > 0 && + selectedAnnotations.every( + (s) => + Boolean(s.imageDataLayer) && + Boolean(s.groundTruthLayer) && + Array.isArray(s.magnification) && + s.magnification.length === 3, + ); + return modelName.trim().length > 0 && selectedJobType != null && areSelectionsValid; }, [modelName, selectedJobType, selectedAnnotations]);Also applies to: 124-131
139-146
: Filter for complete selections before mapping.TypeScript fails because optional fields can be
undefined
, butAiModelTrainingAnnotationSpecification
requires all fields to be defined. Filter the selections first using a type guard.Add a type guard before the mapping:
+ // Type guard to ensure all required fields are present + const isCompleteSelection = ( + selection: AiTrainingAnnotationSelection, + ): selection is Required<AiTrainingAnnotationSelection> => + Boolean(selection.imageDataLayer) && + Boolean(selection.groundTruthLayer) && + Boolean(selection.magnification); + + const completeSelections = selectedAnnotations.filter(isCompleteSelection); + if (completeSelections.length !== selectedAnnotations.length) { + Toast.error("Some annotations are missing required fields."); + return; + } + - const trainingAnnotations: AiModelTrainingAnnotationSpecification[] = selectedAnnotations.map( + const trainingAnnotations: AiModelTrainingAnnotationSpecification[] = completeSelections.map( (selection) => ({frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx (1)
24-24
: Fix grammar: add missing preposition.The phrase "Optimized dense neuronal tissue" is missing "for" before "dense."
Apply this diff:
- "Train a new AI model for EM neuron segmentation based on the annotations in this dataset. Optimized dense neuronal tissue from SEM, FIB-SEM, SBEM, Multi-SEM microscopes.", + "Train a new AI model for EM neuron segmentation based on the annotations in this dataset. Optimized for dense neuronal tissue from SEM, FIB-SEM, SBEM, Multi-SEM microscopes.",frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_analysis_settings.tsx (3)
61-65
: Critical: Fix the selectedLayer change handler.The
Select
component returns the layer name as a string (becauseoptions
mapsvalue: l.name
), but line 63 incorrectly accesseschangedValues.selectedLayer.name
. This will causeselectedLayer
to remainundefined
.Apply this diff:
if ("selectedLayer" in changedValues) { setSelectedLayer( - colorLayers.find((l) => l.name === changedValues.selectedLayer.name) as APIDataLayer, + colorLayers.find((l) => l.name === changedValues.selectedLayer) as APIDataLayer, ); }
127-139
: Critical: uint24 validation is ineffective.The field stores a string (layer name), but
colorLayerMustNotBeUint24Rule
expects anAPIDataLayer
object. The validator will never reject uint24 layers becausevalue.elementClass
is undefined on strings.Apply this diff to validate against the resolved layer:
<Form.Item name="selectedLayer" label="Image Data Layer" rules={[ { required: true, message: "Please select an image data layer" }, - colorLayerMustNotBeUint24Rule, + { + validator: () => { + if (selectedLayer && selectedLayer.elementClass === "uint24") { + return Promise.reject( + new Error( + "The selected layer of type uint24 is not supported. Please select a different one.", + ), + ); + } + return Promise.resolve(); + }, + }, ]} >
149-169
: Critical: Add type guard before calling inference-only functions.Both
getBestFittingMagComparedToTrainingDS
andisDatasetOrBoundingBoxTooSmall
accept only inference job types (INFER_MITOCHONDRIA | INFER_NEURONS | INFER_NUCLEI | INFER_INSTANCES
). Calling them with a training or alignment job type will cause type errors.Add a type guard at line 150:
{ validator: (_, value: UserBoundingBox) => { if (value && selectedLayer && selectedJobType) { + // Only validate for inference jobs + const inferenceJobTypes = ["infer_mitochondria", "infer_neurons", "infer_nuclei", "infer_instances"]; + if (!inferenceJobTypes.includes(selectedJobType)) { + return Promise.resolve(); + } + const boundingBox = computeArrayFromBoundingBox(value.boundingBox); const mag = getBestFittingMagComparedToTrainingDS( selectedLayer, dataset.dataSource.scale, selectedJobType, );Alternatively, if
selectedJobType
has a discriminated union type, use a type predicate or check a discriminant property.docs/automation/ai_training.md (2)
26-27
: Update UI copy: modal → drawer, include tab name.The UI now uses a drawer with a “Model Training” tab, not a dialog.
-To start a training, click on the `AI Analysis` button in the toolbar and select `Train AI model` from the dropdown menu. -This will open a dialog where you can configure and start your training job. +To start training, click the `AI Analysis` button in the toolbar and choose `Train AI model`. +This opens a drawer with a `Model Training` tab where you can configure and start your training job.
7-8
: Tighten style and fix “on‑premise” → “on‑premises”.- AI Model Training is only available on [webknossos.org](https://webknossos.org) at the moment. - If you want to set up on-premise automated analysis at your institute/workplace, then [please contact sales](mailto:[email protected]). + AI Model Training is currently available on [webknossos.org](https://webknossos.org). + If you want to set up on‑premises automated analysis at your institute/workplace, [please contact sales](mailto:[email protected]).
🧹 Nitpick comments (3)
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (1)
80-80
: Avoid non-null assertion; use type guard.The
!
is safe here because the query isenabled
only whenannotationId
is truthy, but non-null assertions can mask refactoring risks and are a code smell.Apply this diff to use optional chaining instead:
- queryFn: () => getUnversionedAnnotationInformation(currentAnnotation.annotationId!), + queryFn: () => + currentAnnotation.annotationId + ? getUnversionedAnnotationInformation(currentAnnotation.annotationId) + : Promise.reject(new Error("No annotation ID")), enabled: !!currentAnnotation.annotationId,docs/automation/ai_training.md (2)
23-23
: Grammar/clarity: labeling paragraph.-3. **Label segments:** Within your bounding boxes, label the segmentation of your structures of interest. Use the volume annotation tool to manually annotate structures. This will be your ground truth data. For neurons, we recommend to densely label each structure with a unique ID. For instances segmentations you only need to label the structures you want to train on, e.g. nuclei, mitochondria, soma, vesicles, etc. +3. **Label segments:** Within your bounding boxes, label the structures of interest using the volume annotation tool. This will be your ground truth data. For neurons, we recommend densely labeling each structure with a unique ID. For instance segmentations, you only need to label the structures you want to train on (e.g., nuclei, mitochondria, soma, vesicles).
48-48
: Units/examples formatting.-* **Max Distance (nm):** (Only for EM Instances Model) The maximum cross-section length ("diameter") for each identified object in nanometers e.g. Nuclei: 1000nm, Vesicles: 80nm. +* **Max Distance (nm):** (Only for EM Instances Model) The maximum cross‑section length (“diameter”) for each identified object in nanometers (e.g., nuclei: 1,000 nm; vesicles: 80 nm).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/automation/ai_training.md
(1 hunks)frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_model_selector.tsx
(1 hunks)frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_analysis_settings.tsx
(1 hunks)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx
(1 hunks)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_model_selector.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_analysis_settings.tsx (10)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_image_segmentation_job_context.tsx (1)
useRunAiModelJobContext
(276-282)frontend/javascripts/libs/react_hooks.ts (1)
useWkSelector
(292-294)frontend/javascripts/types/api_types.ts (1)
APIDataLayer
(114-114)frontend/javascripts/viewer/view/ai_jobs/components/collapsible_split_merger_evaluation_settings.tsx (2)
SplitMergerEvaluationSettings
(4-9)CollapsibleSplitMergerEvaluationSettings
(11-90)frontend/javascripts/theme.tsx (1)
ColorWKBlue
(12-12)frontend/javascripts/admin/dataset/dataset_components.tsx (1)
getDatasetNameRules
(68-85)frontend/javascripts/viewer/view/ai_jobs/utils.ts (3)
colorLayerMustNotBeUint24Rule
(307-318)getBestFittingMagComparedToTrainingDS
(45-87)isDatasetOrBoundingBoxTooSmall
(116-139)frontend/javascripts/viewer/store.ts (1)
UserBoundingBox
(108-110)frontend/javascripts/libs/utils.ts (1)
computeArrayFromBoundingBox
(299-308)frontend/javascripts/viewer/view/ai_jobs/bounding_box_selector.tsx (1)
BoundingBoxSelector
(15-38)
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (8)
frontend/javascripts/types/api_types.ts (2)
APIAnnotation
(589-591)APIDataset
(243-246)frontend/javascripts/viewer/constants.ts (1)
Vector3
(14-14)frontend/javascripts/viewer/store.ts (1)
UserBoundingBox
(108-110)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx (1)
AiTrainingTask
(11-18)frontend/javascripts/viewer/model/accessors/tracing_accessor.ts (1)
getUserBoundingBoxesFromState
(98-101)frontend/javascripts/admin/rest_api.ts (1)
getUnversionedAnnotationInformation
(618-629)frontend/javascripts/admin/api/jobs.ts (3)
AiModelTrainingAnnotationSpecification
(371-376)runInstanceModelTraining
(402-407)runNeuronTraining
(386-391)frontend/javascripts/viewer/model/actions/ui_actions.ts (1)
setAIJobDrawerStateAction
(136-140)
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx (2)
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (1)
useAiTrainingJobContext
(204-210)frontend/javascripts/theme.tsx (1)
ColorWKBlue
(12-12)
🪛 GitHub Check: frontend-tests
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx
[failure] 139-139:
Type '{ annotationId: string; colorLayerName: string | undefined; segmentationLayerName: string | undefined; mag: Vector3 | undefined; }[]' is not assignable to type 'AiModelTrainingAnnotationSpecification[]'.
🪛 LanguageTool
docs/automation/ai_training.md
[style] ~7-~7: For conciseness, consider replacing this expression with an adverb.
Context: ...webknossos.org](https://webknossos.org) at the moment. If you want to set up on-premise ...
(AT_THE_MOMENT)
[grammar] ~46-~46: There might be a mistake here.
Context: ... Give your new model a descriptive name. * Comments: Add any comments or notes ab...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-smoketest-push
- GitHub Check: backend-tests
🔇 Additional comments (6)
frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (3)
31-55
: LGTM! Context type is now correctly aligned.The
handleStartAnalysis
return type now correctly reflects the async implementation (Promise<void>
), andhandleSelectionChange
properly restricts updates to only the mutable fields viaPick<...>
.
101-122
: LGTM! Magnification reset logic is sound.The implementation correctly resets
magnification
when layers change, preventing stale mag values from mismatching the new layer's available magnifications.
154-166
: LGTM! Job dispatch logic correctly branches by job type.The conditional dispatch calls the appropriate API based on
selectedJobType
, passing the correct category and parameters for each training variant.frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx (3)
82-82
: LGTM! Avatar.src now correctly receives a URL string.The previous nested
<img>
element has been fixed, andAvatar
now receives the image URL directly viasrc={item.image}
.
42-50
: LGTM! Task selection handler is correct.The callback correctly checks for enabled items with a valid
jobType
before updating bothselectedTask
andselectedJobType
in the context.
66-79
: LGTM! Visual feedback for selection and disabled states is clear.The styling correctly applies a blue border for the selected item and reduces opacity with a
not-allowed
cursor for disabled items, providing clear visual feedback to users.
docs/automation/ai_training.md
Outdated
WEBKNOSSOS allows you to train your own AI models for image segmentation. This feature is currently in early access. | ||
|
||
<!-- Keep info in sync with docs/automation/ai_segmentation.md --> | ||
!!!info |
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.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Admonition syntax: verify “!!!info” vs “!!! info”.
Many MkDocs setups require a space after “!!!”. Please verify repo-wide usage and renderer behavior; adjust if needed.
🏁 Script executed:
#!/bin/bash
# Verify admonition style usage across docs
rg -nC1 '^\s*!!!info\b|^\s*!!!\s+info\b' docs
# Count occurrences by style
echo "Counts:"
printf "no-space: "; rg -n '^\s*!!!info\b' docs | wc -l
printf "with-space: "; rg -n '^\s*!!!\s+info\b' docs | wc -l
Length of output: 2887
Unify admonition syntax to include a space after “!!!”
Repo-wide usage mixes !!!info
(5 occurrences) and !!! info
(4 occurrences). MkDocs Material requires a space; update all to !!! info
for consistent rendering.
🤖 Prompt for AI Agents
In docs/automation/ai_training.md around line 6, the admonition uses "!!!info"
but MkDocs Material requires a space after "!!!"; update this line to "!!!
info". Also search the repo for other occurrences of "!!!info" and replace them
with "!!! info" (there are 5 occurrences of the no-space form) to ensure
consistent rendering across all markdown files.
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: 3
♻️ Duplicate comments (2)
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx (1)
37-41
: Critical issue from previous review remains unresolved: guard against missing color layers.The code still assumes
colorLayers[0]
exists. If the dataset has no color layers,colorLayer
is undefined and line 40 crashes when derivingselectedBoundingBox
. Lines 48-51 validation and lines 53-67handleStartAnalysis
also fail to checkcolorLayer
existence before usingcolorLayer.name
.Apply this diff to add guards:
const dataset = useWkSelector((state) => state.dataset); const annotationId = useWkSelector((state) => state.annotation.annotationId); const colorLayers = getColorLayers(dataset); - const colorLayer = colorLayers[0]; + const colorLayer = colorLayers[0] ?? null; - const selectedBoundingBox = getBoundingBoxesForLayers([colorLayer])[0]; + const selectedBoundingBox = colorLayer ? getBoundingBoxesForLayers([colorLayer])[0] : null;Update validation to include colorLayer check:
const areParametersValid = useMemo( - () => Boolean(selectedTask?.jobType && newDatasetName), - [selectedTask, newDatasetName], + () => Boolean(selectedTask?.jobType && newDatasetName && colorLayer), + [selectedTask, newDatasetName, colorLayer], );Short-circuit start when colorLayer is missing:
const handleStartAnalysis = useCallback(async () => { + if (!colorLayer) { + Toast.error("No color layer available. Please ensure the dataset has a color layer."); + return; + } try { await startAlignSectionsJob( dataset.id, colorLayer.name,docs/automation/ai_segmentation.md (1)
7-7
: Unresolved from previous review: terminology mismatch with UI.The text still says "dialog" but the UI uses a "drawer". Update to match the implementation.
Apply this diff:
-You can launch the AI analysis dialog using the `AI Analysis` button in the toolbar at the top. This will open a dropdown menu with three options: +You can launch the AI analysis using the `AI Analysis` button in the toolbar at the top. This will open a dropdown menu with three options. Selecting an option opens the AI Jobs drawer:
🧹 Nitpick comments (2)
frontend/javascripts/viewer/view/ai_jobs/credit_information.tsx (2)
65-65
: Remove unprofessional comment.The inline comment "This is a shitty way to do it" should be removed or reworded professionally. If the approach is temporary, use a standard TODO or FIXME marker.
Apply this diff:
- // sum all training volumes into a single bounding box - // This is a shitty way to do it, but it works for now. + // Sum all training volumes into a single synthetic bounding box for cost estimation const totalVolume = selectedAnnotations.reduce(
64-86
: Synthetic cubic bounding box obscures actual training data distribution.The code sums volumes from all training annotations and creates a synthetic cube with side length
Math.cbrt(totalVolume)
. This approach loses information about the actual spatial distribution and shape of the training data. Consider whether the cost API can accept multiple bounding boxes or whether documentation should clarify this is an approximation.If the API supports it, pass multiple bounding boxes and sum costs. Otherwise, add a comment explaining the cubic approximation:
+ // Note: Training cost is estimated using a synthetic cubic bounding box with + // volume equal to the sum of all training annotation volumes. This approximates + // the actual spatial distribution for cost calculation purposes. const totalVolume = selectedAnnotations.reduce(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
docs/automation/ai_segmentation.md
(1 hunks)docs/automation/ai_training.md
(1 hunks)docs/data/wkw.md
(1 hunks)docs/proofreading/proofreading_tool.md
(1 hunks)docs/sharing/dataset_sharing.md
(1 hunks)frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx
(1 hunks)frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_model_selector.tsx
(1 hunks)frontend/javascripts/viewer/view/ai_jobs/credit_information.tsx
(1 hunks)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- docs/data/wkw.md
- docs/sharing/dataset_sharing.md
- docs/proofreading/proofreading_tool.md
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_model_selector.tsx
- frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_model_selector.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-27T11:32:54.033Z
Learnt from: knollengewaechs
PR: scalableminds/webknossos#8796
File: frontend/javascripts/viewer/view/action-bar/ai_job_modals/forms/neuron_segmentation_form.tsx:68-77
Timestamp: 2025-08-27T11:32:54.033Z
Learning: In the webknossos frontend AI job forms, `datasetConfiguration.layers[colorLayer.name]` is assumed to always be non-null when accessing layer properties like `isInverted`. The selected color layer is guaranteed to exist in the dataset configuration.
Applied to files:
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx
🧬 Code graph analysis (2)
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx (7)
frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_model_selector.tsx (1)
AlignmentTask
(11-18)frontend/javascripts/viewer/store.ts (1)
UserBoundingBox
(108-110)frontend/javascripts/libs/react_hooks.ts (1)
useWkSelector
(292-294)frontend/javascripts/viewer/view/ai_jobs/utils.ts (1)
getBoundingBoxesForLayers
(30-40)frontend/javascripts/admin/api/jobs.ts (1)
startAlignSectionsJob
(344-363)frontend/javascripts/viewer/model/actions/ui_actions.ts (1)
setAIJobDrawerStateAction
(136-140)frontend/javascripts/libs/toast.tsx (1)
error
(211-218)
frontend/javascripts/viewer/view/ai_jobs/credit_information.tsx (9)
frontend/javascripts/viewer/view/ai_jobs/run_ai_model/ai_image_segmentation_job_context.tsx (1)
useRunAiModelJobContext
(276-282)frontend/javascripts/viewer/view/ai_jobs/alignment/ai_alignment_job_context.tsx (1)
useAlignmentJobContext
(84-90)frontend/javascripts/viewer/view/ai_jobs/train_ai_model/ai_training_job_context.tsx (1)
useAiTrainingJobContext
(204-210)frontend/javascripts/libs/utils.ts (2)
computeVolumeFromBoundingBox
(314-317)computeArrayFromBoundingBox
(299-308)frontend/javascripts/viewer/store.ts (2)
UserBoundingBoxWithoutId
(102-107)UserBoundingBox
(108-110)frontend/javascripts/types/api_types.ts (1)
AiModel
(833-844)frontend/javascripts/libs/react_hooks.ts (1)
useWkSelector
(292-294)frontend/javascripts/admin/api/jobs.ts (2)
JobCreditCostInfo
(80-86)getJobCreditCost
(88-97)frontend/javascripts/libs/format_utils.ts (2)
formatVoxels
(515-538)formatCreditsString
(559-563)
🪛 GitHub Check: frontend-tests
frontend/javascripts/viewer/view/ai_jobs/credit_information.tsx
[failure] 46-46:
Type 'APIJobType | null | undefined' is not assignable to type 'APIJobType | null'.
🪛 LanguageTool
docs/automation/ai_segmentation.md
[grammar] ~18-~18: There might be a mistake here.
Context: ...and focused ion beam-SEM (FIB-SEM) data. * Mitochondria Detection: Run a pre-trai...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...et that the model will use for analysis. * Bounding Box: The region of interest t...
(QB_NEW_EN)
[style] ~47-~47: Consider using an alternative to strengthen your wording.
Context: ...analysis features in the future. If you want to work with us on an automated analysi...
(WANT_KEEN)
[grammar] ~47-~47: There might be a mistake here.
Context: ...ntact us](mailto:[email protected]). We would love to integrate analysis solu...
(QB_NEW_EN)
[style] ~52-~52: For conciseness, consider replacing this expression with an adverb.
Context: ...webknossos.org](https://webknossos.org) at the moment. If you want to set up on-premise ...
(AT_THE_MOMENT)
[grammar] ~52-~52: There might be a mistake here.
Context: ...(https://webknossos.org) at the moment. If you want to set up on-premise automat...
(QB_NEW_EN)
[grammar] ~53-~53: There might be a mistake here.
Context: ...act sales](mailto:[email protected]).
(QB_NEW_EN)
docs/automation/ai_training.md
[style] ~7-~7: For conciseness, consider replacing this expression with an adverb.
Context: ...webknossos.org](https://webknossos.org) at the moment. If you want to set up on-premise ...
(AT_THE_MOMENT)
[grammar] ~7-~7: There might be a mistake here.
Context: ...(https://webknossos.org) at the moment. If you want to set up on-premise automat...
(QB_NEW_EN)
[grammar] ~46-~46: There might be a mistake here.
Context: ... Give your new model a descriptive name. * Comments: Add any comments or notes ab...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: backend-tests
- GitHub Check: build-smoketest-push
AI Model Training is only available on [webknossos.org](https://webknossos.org) at the moment. | ||
If you want to set up on-premise automated analysis at your institute/workplace, then [please contact sales](mailto:[email protected]). |
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.
Tighten wording and use “on‑premises”.
Swap “at the moment” for “currently” (or drop it) and change “on-premise” to the correct “on‑premises” phrasing to keep the copy concise and polished.
🧰 Tools
🪛 LanguageTool
[style] ~7-~7: For conciseness, consider replacing this expression with an adverb.
Context: ...webknossos.org](https://webknossos.org) at the moment. If you want to set up on-premise ...
(AT_THE_MOMENT)
[grammar] ~7-~7: There might be a mistake here.
Context: ...(https://webknossos.org) at the moment. If you want to set up on-premise automat...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In docs/automation/ai_training.md around lines 7 to 8, tighten the wording:
replace "at the moment" with "currently" (or remove it) and change "on-premise"
to the correct "on‑premises" phrasing; update the sentence to read concisely
(e.g., "AI Model Training is only available on webknossos.org currently. If you
want to set up on‑premises automated analysis at your institute/workplace,
please contact sales.") ensuring punctuation and link remain intact.
To start a training, click on the `AI Analysis` button in the toolbar and select `Train AI model` from the dropdown menu. | ||
This will open a dialog where you can configure and start your training job. | ||
### Select AI Training Task |
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.
Update instructions to match the drawer-based UI.
The AI job UI no longer opens a modal dialog—it now launches a drawer with a “Model Training” tab. Leaving the old wording misleads users following these steps. Please update this section to reference the drawer and tab so the docs stay accurate.
🤖 Prompt for AI Agents
In docs/automation/ai_training.md around lines 26 to 28, the text still refers
to a modal dialog for starting AI training; update it to reference the new
drawer-based UI and the “Model Training” tab. Change the sentence to instruct
users to click the AI Analysis button, select Train AI model, then use the
opened drawer and switch to the “Model Training” tab to configure and start the
job, ensuring wording clearly replaces “dialog” with “drawer” and mentions the
tab name.
return ( | ||
<CreditInformation | ||
selectedModel={selectedTask} | ||
selectedJobType={selectedTask?.jobType} |
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.
Fix TypeScript type mismatch.
selectedTask?.jobType
returns APIJobType | null | undefined
, but selectedJobType
prop expects APIJobType | null
. The optional chaining can produce undefined
, which must be coalesced to null
.
Apply this diff:
selectedModel={selectedTask}
- selectedJobType={selectedTask?.jobType}
+ selectedJobType={selectedTask?.jobType ?? null}
selectedBoundingBox={selectedBoundingBox}
📝 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.
selectedJobType={selectedTask?.jobType} | |
selectedModel={selectedTask} | |
selectedJobType={selectedTask?.jobType ?? null} | |
selectedBoundingBox={selectedBoundingBox} |
🧰 Tools
🪛 GitHub Check: frontend-tests
[failure] 46-46:
Type 'APIJobType | null | undefined' is not assignable to type 'APIJobType | null'.
🤖 Prompt for AI Agents
In frontend/javascripts/viewer/view/ai_jobs/credit_information.tsx around line
46, the prop selectedJobType is being passed selectedTask?.jobType which can be
undefined due to optional chaining while the prop type expects APIJobType |
null; change the expression to coalesce undefined to null (e.g., replace
selectedTask?.jobType with an expression that yields null when undefined) so the
value matches the expected type.
This PR updates and refreshes the AI job UI. This is a complete re-write from scratch avoiding the one-form-to-rule-them-all approach for starting job that we had until now. Most components and logic for alignment, AI training and inference is separate und tuned for that usecase. They still share a similar structure and some components but otherwise are independent, which should let us expand them individually as needed in the future.
URL of deployed dev instance (used for testing):
Steps to test:
TODOs:
Issues:
(Please delete unneeded items, merge only when none are left open)
$PR_NUMBER.md
file inunreleased_changes
or use./tools/create-changelog-entry.py
)