Add support for model prediction in Video toolbar by wk9874 · Pull Request #271 · ukaea/toktagger

wk9874 · 2026-05-18T13:44:24Z

Description

Adds support for model predictions of single samples from the toolbar of the Video interface, to match what is available in time series.

Changes

Adds the modelPredictSample tool to toolbar in video mode as well as time series
Adds support in the backend (model baseclass and /samples/{sample_id}/models/{type) for passing dataParams for the selected sample to the backend (so that, eg, the frame number is available to the model)

To Test

Use scripts.setup to create the local Frame Project, then go into there and 'train' a model with the button in the top right. Once trained, open the sample, and use the model prediction in the toolbar. Should be an option to either predict on only one frame, or all frames. Predictions will be a box of width 50, height 50 at (0,0) with the id 'test'.

To clear previous predictions, press Clear at the top of the toolbar, then Save, then Clear again

To Dos

Noticed a formatting issue in the model schema form - checkbox and array element descriptions overfill the toolbar column when using the model predict sample tool, and no combination of widths / minWidth / maxWidth seemed to fix it. Currently have hardcoded the size to 2500, which makes it fit in the toolbar nicely, but means it looks odd in the predict modal. Need some help figuring out what is causing this issue Fixed by Josh, setting max width of toolbar
Inside modelPredictSample there is now some video specific logic, which I would like to remove ideally. This is because the dataParams stored inside sampleContext are not guaranteed to be correct when in video mode, and you need to access the frame from data directly to reconstruct the effectiveDataParams. Would prefer it to be that the dataParams in sampleContext is the single point of truth which both timeseries and video use Fixed by Abdullah
Unlike timeseries, video doesn't seem to update annotations automatically based on when setAnnotations is called from sampleContext. This means that once predictions are ready and returned from the backend, setAnnotations is called, but you dont see any new boxes until you refresh. Would be good to have it work similarly to time series and auto update if possible Fixed by abdullah
Remove temp model from temp.py and init.py once testing complete

Note Do not merge until #255 is merged

Closes #240
Closes #246

…edict endpoint

…r into wk9874/models/data_params

…model-prediction-untrained

…74/models/data_params

…predictions

…redictions, imports, frame navigation, and propagation without refresh.

…r into wk9874/models/data_params

…a_params

…r into wk9874/models/data_params

wk9874 · 2026-05-28T14:02:38Z

Model prediction boxes appear to not be selectable for some reason:

toktagger_prediction_not_selectable.mp4

abdullah-ukaea · 2026-05-28T15:23:11Z

Model prediction boxes appear to not be selectable for some reason:

toktagger_prediction_not_selectable.mp4

for some reason I cannot reproduce this

model_predict_selection_bug.mp4

abdullah-ukaea · 2026-05-28T15:24:54Z

If I have a model which creates a prediction over multiple frames, propagation of hand drawn annotations is disabled

model_predict_multiple.mp4
(not even sure what should happen here, so happy to be told that this is either expected behaviour, or is so insignificant that its not worth fixing!)

as discussed, this is actually working as expected based on how propagation is currently implemented. However now that model prediction is being integrated into toktagger I need to rethink how to only propagate hand drawn annotations and NOT propagate model predicted annotations.

This will be tackled in a separate follow up PR / needs to be documented in an issue

…l edits, and prevent Annotorious programmatic redraw events from being mistaken for user edits

abdullah-ukaea · 2026-06-01T12:00:06Z

could you give this branch another functional test to help me identify any bugs? I think I resolved the main reported bug now. @jblake42 @wk9874

wk9874 · 2026-06-01T12:55:09Z

Yep the instance numbering bug when importing looks fixed now :) other two I think are future work, have raised issues #275 #276

Found another bug, but I think this is probably a bug in my model Predict tooling implementation - if you ask for a model prediction on one frame, then go to the next frame and ask for a prediction on that one, itll remove the prediction from the first frame... suspect when I wrote the model predict tool only timeseries existed, and so it removes previous annotations from the given model before adding new ones (which ofc means that in the video case, it removes predictions from all frames before writing new predictions for the next requested frame). I'll look into fixing this today

abdullah-ukaea · 2026-06-01T13:07:27Z

Yep the instance numbering bug when importing looks fixed now :) other two I think are future work, have raised issues #275 #276

Found another bug, but I think this is probably a bug in my model Predict tooling implementation - if you ask for a model prediction on one frame, then go to the next frame and ask for a prediction on that one, itll remove the prediction from the first frame... suspect when I wrote the model predict tool only timeseries existed, and so it removes previous annotations from the given model before adding new ones (which ofc means that in the video case, it removes predictions from all frames before writing new predictions for the next requested frame). I'll look into fixing this today

sounds good, let me know if I need to change anything on the ui side to fix the prediction bug you mentioned

abdullah-ukaea · 2026-06-02T12:17:25Z

ready for code review @jblake42

jblake42

Mainly focussed on front end due to time constraints

jblake42 · 2026-06-08T08:23:04Z

-          {" "}
-          <em>{schema.description}</em>{" "}
-        </Text>
+        // For some reason anything other than hard coding a size here


@wk9874 - was this fixed by setting the toolbar width. Can we remove the comment etc?

Yep this is fixed, will remove the TODO

jblake42 · 2026-06-08T08:28:32Z

    updater: (annotations: Annotation[]) => Annotation[] | Annotation[],
  ) => void;
-  setDataParams: (params: DataParams) => void;
+  setDataParams: React.Dispatch<React.SetStateAction<DataParams>>;


Is this typed like this for a reason or can we be consistent with the other instances below?

Good spot, there wasn’t a good reason for it to be typed differently. I’ve changed setDataParams to match the other context setters and cleaned up the VideoView call site so the typing is simpler. fixed in commit ef902a8

jblake42 · 2026-06-08T08:34:16Z

+                return prev;
+              }
+              return {
+                ...prev,


We should not need to expand the existing previous frame here as you are overwriting all existing data anyway:

return { name: "image", frame, };

good spot, fixed in commit da4a93c

jblake42 · 2026-06-08T08:42:40Z

+): ImageAnnotation {
+  return stampCreator(stampLabelAndTrack(a, className, trackId), createdBy);
+}
+


The functions are a little messy here imo - you have split functionality into the different types of stamps and then combined them in higher level functions.

I personally think keep them all seperate and then chaining them at the point of use would improve the clarity rather than having multiple functions performing varying levels of stamping

Agreed, fixed in commit 8dd4d4c

jblake42 · 2026-06-08T08:45:06Z

  };
 }

+/** Read backend creator metadata, falling back to manual at export time. */


I think the comment is misleading - this function does not do any falling back as such, it just returns null. The logic that uses this does the falling back

I think falling back to manual is valid, but in that case this function should return manual and then you don't have to repeat the line created_by: getAnnotationCreator(a) ?? "manual", you could just do created_by: getAnnotationCreator(a)

I don't mind either way but whichever you go for be clear in the comment

I have gone with your second suggestion as I think a fall back to manual rather than null is more correct, so now the comment is actually correct. fixed in commit 2166214

jblake42 · 2026-06-08T08:52:50Z

        dbAnnotations={annotations ?? []}
+        setSampleAnnotations={setAnnotations}
        propagate={videoPropagate}
        setPropagate={setVideoPropagate}


There may be a good reason, but can the VideoSessionProvider not just grab the context itself rather than have the inner provider do it for it. It seems a bit redundant to me and bloats the props?

I agree, there wasn’t a strong reason to pass those through as props since they all come from SampleContext. I’ve moved those context reads into VideoSessionProvider directly, so VideoProvidersInner now only passes the video-specific props it needs to. Fixed in commit d09152b

jblake42 · 2026-06-08T08:57:12Z

+      continue;
+    }
+
+    entries.push(


Should we be more explicit here and check that the type is as we expect - this will catch if an additional type is added and the logic is not updated

fixed in commit 8cc8af1

jblake42 · 2026-06-08T08:58:20Z

+    });
+
+    const anno =
+      dbAnno.type === "video_bounding_box"


Same here with explicitly checking types

There are other instances below - I won't comment them all

fixed in commit 8cc8af1

There are other instances below - I won't comment them all

could you double check I got all the instances?

jblake42 · 2026-06-08T09:00:50Z

-    setByFrame((prev) => mapSetFrame(prev, f, list));
-    // Seeding should not mark the session dirty.
-  }, []);
+  useEffect(() => {


Keep useEffects grouped together

done in commit 876420a

Could you double check this commit is what you wanted me to do?

jblake42 · 2026-06-08T09:08:08Z

-    },
-    [byFrame.size, dirty, projectId, sampleId],
-  );
+    if (invalid > 0) {


Do duplicates frequently happen or is there any actions that need to be taken if duplicates happen - if they are frequent or there is no action to be taken we shouldn't flood the log

Good point, this was just leftover debug-style logging for a handled cleanup path. I removed the warnings and kept the duplicate cleanup behavior unchanged so we do not flood the console. fixed in commit 0d18293

Prepare for 0.2.0 release

Fix Pages CI

…eContext setters.

…nd creator helper directly.

…by assignments

…olygons

…FrameRef sync useEffect

…cleanup behavior

…rams

abdullah-ukaea · 2026-06-11T11:34:51Z

@jblake42 I have addressed your UI code review now. I think @wk9874 has one pending comment

Removed comments about container width issue in schemaForm.

wk9874 and others added 17 commits April 29, 2026 11:12

Add optional data params to predict model method and add to sample pr…

3390235

…edict endpoint

Add model predict to video toolbar

d5d00fa

Add dataparams to model predict tool

1484be3

Improve checkbox description formatting

0dabf2f

Fix description styling

3f2dc11

Fix description styling

6b2ef38

Make dataparams work with image

9388fd0

chore: update build output [skip ci]

238884d

Add temp test video ML model

124e194

Merge branch 'wk9874/models/data_params' of github.com:ukaea/toktagge…

54e061b

…r into wk9874/models/data_params

Merge branch 'wk9874/models/239-model-suffix' into wk9874/models/245-…

391ea3f

…model-prediction-untrained

Merge branch 'wk9874/models/245-model-prediction-untrained' into wk98…

26c101b

…74/models/data_params

Merge branch 'video/UI-cleanup' into wk9874/models/data_params

f4b45ac

Sync video annotations from context and use current data params for …

9dbfa07

…predictions

linting

e2ddf34

Refactor video annotations to sync through SampleContext and update p…

f8a564f

…redictions, imports, frame navigation, and propagation without refresh.

build files

5d2a27c

Base automatically changed from wk9874/models/245-model-prediction-untrained to dev May 20, 2026 09:23

Merge branch 'dev' into wk9874/models/data_params

3bdae14

wk9874 changed the base branch from dev to video/UI-cleanup May 20, 2026 09:41

chore: update build output [skip ci]

7acba13

wk9874 changed the base branch from video/UI-cleanup to dev May 20, 2026 09:46

wk9874 changed the base branch from dev to video/UI-cleanup May 20, 2026 09:47

wk9874 added 2 commits May 20, 2026 10:48

Merge branch 'video/UI-cleanup' into wk9874/models/data_params

08f01ba

Merge branch 'wk9874/models/data_params' of github.com:ukaea/toktagge…

82016c0

…r into wk9874/models/data_params

wk9874 marked this pull request as draft May 20, 2026 09:52

abdullah-ukaea and others added 4 commits May 20, 2026 11:26

Merge branch 'wk9874/models/data_params_VIDEO' into wk9874/models/dat…

92c14a4

…a_params

Make test model expect data params

c8d90b9

Merge branch 'wk9874/models/data_params' of github.com:ukaea/toktagge…

43e64a9

…r into wk9874/models/data_params

Set maximum width of toolbar

f2f134b

abdullah-ukaea and others added 2 commits May 29, 2026 16:23

Normalize imported video annotations to the same shape rules as manua…

3418f0b

…l edits, and prevent Annotorious programmatic redraw events from being mistaken for user edits

chore: update build output [skip ci]

793e69d

wk9874 and others added 2 commits June 1, 2026 14:58

Stop model predict sample removing eisting annotatinos

603903d

chore: update build output [skip ci]

014f345

jblake42 requested changes Jun 8, 2026

View reviewed changes

wk9874 and others added 13 commits June 9, 2026 10:21

Merge pull request #283 from ukaea/dev

3fa27e5

Prepare for 0.2.0 release

Fix pages workflow

404d73d

Merge pull request #287 from ukaea/hotfix/pages

6d9f710

Fix Pages CI

Make setDataParams use the same simple setter type as the other Sampl…

ef902a8

…eContext setters.

Simplify video dataParams updates to replace image params directly.

da4a93c

Simplify annotation stamping by chaining the separate label, track, a…

8dd4d4c

…nd creator helper directly.

Move creator fallback into getAnnotationCreator and simplify created_…

2166214

…by assignments

Move sample data and annotation context reads into VideoSessionProvider.

d09152b

Make video session annotation shape handling explicit for boxes and p…

8cc8af1

…olygons

Moved the external annotation sync useEffect up next to the nearby by…

876420a

…FrameRef sync useEffect

Remove noisy video annotation sync warnings while keeping duplicate …

0d18293

…cleanup behavior

Merge branch 'dev' into wk9874/models/data_params

868481c

Merge remote-tracking branch 'origin/main' into wk9874/models/data_pa…

0b0e97c

…rams

Dr. CI Bot and others added 4 commits June 11, 2026 11:35

chore: update build output [skip ci]

487424e

linting

291d871

Remove TODO in schemaForm.tsx

5be1311

Removed comments about container width issue in schemaForm.

chore: update build output [skip ci]

ba88dee

Conversation

wk9874 commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

To Test

To Dos

Uh oh!

wk9874 commented May 28, 2026

Uh oh!

abdullah-ukaea commented May 28, 2026

Uh oh!

abdullah-ukaea commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

abdullah-ukaea commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wk9874 commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

abdullah-ukaea commented Jun 1, 2026

Uh oh!

abdullah-ukaea commented Jun 2, 2026

Uh oh!

jblake42 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abdullah-ukaea commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

wk9874 commented May 18, 2026 •

edited

Loading

abdullah-ukaea commented May 28, 2026 •

edited

Loading

abdullah-ukaea commented Jun 1, 2026 •

edited

Loading

wk9874 commented Jun 1, 2026 •

edited

Loading

abdullah-ukaea commented Jun 11, 2026 •

edited

Loading