Repository navigation
Let a job type declare what it needs, and an algorithm say whether it can be retrained - #1501
Open
mohamedelabbas1996 wants to merge 14 commits into
Open
mohamedelabbas1996 wants to merge 14 commits into
mohamedelabbas1996 wants to merge 14 commits into
Conversation
A project needs the same occurrences again later: comparing two classifiers only means something if both saw the same rows, and a review pass wants the list it started from. A filter answers differently as data arrives, so the membership is stored rather than described. An OccurrenceSet holds its occurrences and the projects it belongs to. A set with no project is global and is offered everywhere, following how TaxaList already treats a list with no project. Membership is decided when the set is created and nothing adds to or removes from one afterwards, because anything recorded against a set was measured on exactly those occurrences. Creating, renaming and deleting are gated on new project permissions, held by the roles that already curate a project's data. A global set has no single project to check against, so it cannot be edited through the API at all. The occurrence list takes an occurrence_set filter, and the sets are offered as choices the same way capture sets are.
Adds the occurrence set to the occurrence filter panel, picked from the set choices endpoint the same way a capture set is. A set is only useful if you can look at what is in it, and this is where someone reviewing one starts. The field is carried over from other views like the existing filters, so arriving with a set already chosen shows it in the panel where it can be cleared.
Someone filtering the occurrence list to the rows they care about had no way to keep that selection. Selecting occurrences now offers saving them as a set, next to the identification actions already there. The action does not change any occurrence, so it is not behind update rights on them; the endpoint gates it on the project's own permission instead.
Registering a filter in the shared list is not enough for it to appear: the occurrences page renders one FilterControl per field it offers, and the set was missing from that list, so the filter existed everywhere except on screen. It sits under More filters beside the capture set, and that section now opens on arrival when a set is already applied, as it does for the other filters there.
The selection bar holds identification actions and is hidden from anyone without update rights on the occurrences, so saving a set — which changes none of them — was unavailable to a reader who could still create one. It also sat there as an unlabelled icon among three others. It now sits beside Export as a labelled button, and appears only while something is selected, since that is the only time it does anything.
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
✅ Deploy Preview for antenna-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
mohamedelabbas1996
added this pull request to stack #1505
October 9, 2026 05:05
mohamedelabbas1996
force-pushed
the
feat/retrain-job-plumbing
branch
from
October 15, 2026 17:34
d6a3cc3 to
36aea4f
Compare
kra
reviewed
Oct 19, 2026
| key = "ml" | ||
| # The one type a person starts by hand today. A pipeline is not declared required here: | ||
| # single-capture runs created from a session page carry one, but the API has always | ||
| # accepted a job without it and failing it at creation would change that. |
Collaborator
There was a problem hiding this comment.
I'm not understanding this comment. The first sentence means "a person can start this job type by hand", right? "Today" means a different thing every day :) and the fact that this is the only type that this is true for won't be accurate forever, the comment should ideally only document this code, not other code.
I don't understand the other sentence, is this documenting the API and pipeline?
| required_params: tuple[str, ...] = () | ||
|
|
||
| # Whether a person can start one from the UI. The rest are created by the platform: | ||
| # a data sync, an export, a regroup. Nothing should offer those as a choice. |
Collaborator
There was a problem hiding this comment.
I don't understand this comment, does this mean
# True if this JobType can be started by a person from the UI. If False, then the UI should not offer the JobType from the UI.
# Some examples of JobTypes that cannot be started from the UI are ...
I don't personally think that this should document subclasses that have particular values set for this, because that's documenting nonlocal code (and it's the kind of thing that can be found with a quick grep anyway)
| job_type_key = serializers.SlugField(write_only=True, default=MLJob.key) | ||
| # Settings a job type reads from its own params, such as which algorithm to retrain and | ||
| # what split to hold out. Kept as one field rather than a column per type: only the type | ||
| # knows what its params mean, and it is the type that validates them when it runs. |
Collaborator
There was a problem hiding this comment.
Reads from its own params as opposed to what? Class or instance variables?
Which params? "params" is kind of general, should this get a more specific name? Are the params given when the job is invoked, or created, or run, or?
| # Each job type says how long one of its jobs may go untouched. A type that | ||
| # hands work to an external service and waits to be called back writes nothing | ||
| # to its own row meanwhile, so silence is not evidence that it died. Candidates | ||
| # are gathered above at the default, then each is judged by its own deadline. |
Collaborator
There was a problem hiding this comment.
I don't understand "gathered above at the default".
Collaborator
|
Can this be split up to make it reviewable? For example, do we need the model and the view and the serializer etc at once? Do we need all the applications to be in one change? |
Retraining needs two things an algorithm did not carry: whether the processing service can retrain it at all, and the settings to retrain it with. Asking the service on every page load would make the UI depend on the service being up, so both are mirrored onto the algorithm when it registers. The config is seeded from the service and then owned here, so an admin's edits survive the next /info read. The settings are split on purpose: Antenna reads the dataset half when it builds the training set and passes the fitting half through, since the service owns the fitting. for_run() takes a job's params on top of the config and checks them, because a test_fraction outside (0, 1) either holds out everything or scores a head on the rows it just learned, and both finish looking like a successful run. training_info is the other direction: every retrain makes a new algorithm version, so each version records where its weights came from.
The trainable flag and the training settings were on the algorithm but nothing ever filled them: a service could say it retrains a head and Antenna stored false anyway, so the job form offered nothing and the job refused every algorithm. Found by registering a real service that does report both. They now come from the /info response at registration, like task_type and uri. The settings are seeded only when the algorithm is new, so an admin's edits are not overwritten the next time the pipelines are re-registered. AlgorithmTrainingConfig moves up the module because the config response now refers to it.
The training job form offers a classifier to retrain, and listing every algorithm in the project would offer detectors and heads no service can train. The flag is already mirrored from the service, so the list endpoint now filters on it.
…go quiet Four things a job type knows about itself that nothing could ask it: ``required_fields`` and ``required_params`` are what a job of this type cannot run without, so a gap becomes a 400 when the job is created rather than a failure minutes later in a worker. ``user_creatable`` says whether a person starts one by hand, since the rest are made by the platform as a side effect of something else and nothing should offer those as a choice. ``stalled_after_minutes`` is how long one may go untouched before the stale check calls it dead, which the default of ten minutes gets wrong for any type that waits on something external. Declared here on their own; the checks that read them follow.
A job form had no way to send a type's own settings: params were readable on the model and absent from the serializer, so which algorithm to retrain and what split to hold out could not reach the API at all. They are now one writable field rather than a column per type, because only the type knows what its params mean. Creation also refuses a job its type could not run, naming the setting that is missing. The alternative is a worker discovering the gap minutes later, which leaves a failed job in the list and a person guessing what they got wrong. Not enforced: user_creatable. Several types the platform creates as a side effect are also posted directly by existing clients, so it says which types a job form should offer, which is the UI's question rather than the API's.
The stale sweep fails any job untouched for ten minutes. A job that hands work to an external service and waits to be called back writes nothing to its own row meanwhile, so for those types silence is not evidence that the job died, and a training run that takes an hour was being marked lost while it was still running. Candidates are still gathered at the default, then each is judged against the deadline its own type declares. The type is looked up rather than read off the job, so an unrecognised job_type_key leaves that one job alone instead of raising and stopping the whole sweep.
required_params (keys inside Job.params) is what TrainClassifierJob actually needs checked at creation time. required_fields had no job type setting it anywhere, so it was dead vocabulary on JobType and a dead check in JobListSerializer.validate(). Co-Authored-By: Claude <noreply@anthropic.com>
Redone after rebasing onto Michael's latest feat/detection-embeddings-task (which removed the embedding key field and reader.py, replacing stored vector length with Algorithm.embedding_dimensions). Co-Authored-By: Claude <noreply@anthropic.com> # Conflicts: # ui/src/utils/language.ts
feat/occurrence-sets' own 0098/0099 collide with Michael's base 0098_classification_algorithm_result.py and 0099_classification_algorithm_result_index.py. Renumbered to 0100/0101 and fixed their dependencies, same fix needed every time this branch is rebased onto a moved base. Co-Authored-By: Claude <noreply@anthropic.com>
mohamedelabbas1996
force-pushed
the
feat/retrain-job-plumbing
branch
from
October 20, 2026 16:00
e03a4c6 to
d2960e5
Compare
Contributor
Author
|
Thanks @kra for review, I will look into it. |
This was referenced Oct 20, 2026
mihow
added
the
occurrence-sets
This was referenced Oct 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Groundwork for retraining a classifier head, with nothing about training in it. Split out of #1494 so it can be read on its own.
A job type can describe itself
Four declarations on
JobType, and the two checks that read them:required_fieldsandrequired_paramsare what a job of this type cannot run without. The API refuses to create one that is missing any, so a gap is a 400 when the job is made rather than a failure minutes later in a worker.user_creatablesays whether a person starts one by hand. It is not enforced on create: several types the platform makes as a side effect are also posted directly by existing clients. It says which types a job form should offer, which is the UI's question.stalled_after_minutesis how long one may go untouched before the stale check calls it dead.paramsalso becomes writable when a job is created, so a type's own settings can reach it. One field rather than a column per type, because only the type knows what its params mean.The stale sweep judges a job by its own type
The sweep failed anything untouched for ten minutes. A job that hands work to an external service and waits to be called back writes nothing to its own row meanwhile, so for those types silence is not evidence that the job died. Candidates are still gathered at the default, then each is judged against its own type's deadline. The type is looked up rather than read off the job, so an unrecognised
job_type_keyleaves that one job alone instead of stopping the whole sweep.An algorithm says whether it can be retrained
trainable,training_configandtraining_infoonAlgorithm, mirrored from the service's/infoat registration the waytask_typeandurialready are.This was a real gap: a service could report
trainable: trueand Antenna storedfalse, because nothing read the field. Found by registering a service that does report it.The settings are seeded only when the algorithm is new, so an admin's edits are not overwritten the next time pipelines are re-registered. Algorithms can also be filtered by
trainable, which is what a form offering "retrain this one" needs.Stack
Based on
feat/detection-embeddings-taskbecause the algorithm migration follows that branch's0030, merged withfeat/occurrence-sets(#1492): the training-set step needs anOccurrenceSetto scope a run, and #1492 is already the shared base for the evaluation stack (#1493), so it is merged in here rather than duplicated in #1502.