Taxonomy FE Integration#7729
Open
nverjinski wants to merge 15 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis pull request implements taxonomy-backed attribute values as an alternative to simple explicit values. It introduces an ontology type system to distinguish between annotation ontologies and taxonomies, extends the backend AttributeSpec with an optional taxonomy field enforcing mutual exclusivity with values, validates taxonomy settings in label schemas, updates the frontend form model to track values mode (simple vs. taxonomy), implements state handlers to manage taxonomy eligibility based on attribute type and component, and renders a UI toggle in AttributeFormContent to switch between ValuesList and OntologyPicker modes. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@fiftyone/core/annotation/validate_label_schemas.py`:
- Line 729: The function _validate_taxonomy_setting currently lacks an explicit
return type; add a return type annotation "-> None" to its definition (i.e.,
change "def _validate_taxonomy_setting(field_name, taxonomy):" to include "->
None") so it follows repository conventions and satisfies static analysis;
update the function signature where _validate_taxonomy_setting is defined and
ensure any imports or type aliases used for parameters remain unchanged.
- Around line 408-411: The error message raised in the categorical field
validator uses the word "incompatible" for foac.TAXONOMY and foac.VALUES but the
string-field validator uses "mutually exclusive"; update the ValueError message
raised where foac.TAXONOMY and foac.VALUES are checked (the raise referencing
field_name, foac.TAXONOMY and foac.VALUES) to use the exact phrasing "mutually
exclusive" so both validators use consistent wording.
In `@fiftyone/server/routes/ontology.py`:
- Around line 201-202: Replace the hard-coded string "taxonomy" in the if-check
with the OntologyType constant: change the condition to compare doc.get("type")
against OntologyType.TAXONOMY (the same place where doc and name are used) so
the code uses the imported OntologyType enum/constant rather than a literal
string for better maintainability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 793e2be3-4d8e-4b40-bd28-c33b2dae4560
📒 Files selected for processing (16)
app/packages/core/src/components/Modal/Sidebar/Annotate/SchemaManager/EditFieldLabelSchema/ApplyOntologySection.tsxapp/packages/core/src/components/Modal/Sidebar/Annotate/SchemaManager/EditFieldLabelSchema/GUIContent/AttributeFormContent.tsxapp/packages/core/src/components/Modal/Sidebar/Annotate/SchemaManager/EditFieldLabelSchema/GUIContent/useAttributeForm.tsapp/packages/core/src/components/Modal/Sidebar/Annotate/SchemaManager/EditFieldLabelSchema/OntologyPicker.tsxapp/packages/core/src/components/Modal/Sidebar/Annotate/SchemaManager/EditFieldLabelSchema/useOntologies.tsapp/packages/core/src/components/Modal/Sidebar/Annotate/SchemaManager/utils.tsfiftyone/core/annotation/attributes.pyfiftyone/core/annotation/constants.pyfiftyone/core/annotation/nodes.pyfiftyone/core/annotation/validate_label_schemas.pyfiftyone/server/routes/__init__.pyfiftyone/server/routes/ontology.pytests/unittests/annotation/attribute_spec_tests.pytests/unittests/annotation/validate_label_schemas_tests.pytests/unittests/ontology_class_tests.pytests/unittests/server/routes/test_ontology.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/packages/core/src/components/Modal/Sidebar/Annotate/SchemaManager/EditFieldLabelSchema/GUIContent/AttributeFormContent.tsx`:
- Around line 91-101: The taxonomy tab gating only checks isTaxonomyEligible and
ignores ontology-derived read-only semantics; update the tab definition (id:
"taxonomy") to also consider isFromOntology from useAttributeForm.ts (where
isFromOntology is computed as !!formState._source) so the tab is disabled when
isFromOntology is true (e.g., disabled: !isTaxonomyEligible || isFromOntology)
and set the tooltip to indicate the attribute is read-only due to originating
from the ontology when isFromOntology is true; ensure this change prevents mode
switching for ontology-derived attributes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ad257ba3-a34e-4148-bdaa-5067224370c8
📒 Files selected for processing (1)
app/packages/core/src/components/Modal/Sidebar/Annotate/SchemaManager/EditFieldLabelSchema/GUIContent/AttributeFormContent.tsx
Returns the tree for a named Taxonomy, with optional node-rooting and
depth-truncation for lazy-fetch flows.
- New `OntologyTaxonomy` Starlette endpoint in
`fiftyone/server/routes/ontology.py`, registered at
`/ontologies/{name}/taxonomy`.
- Path param `name` resolves a Taxonomy via `load_ontology` (slug
normalized internally). 404 when the name doesn't exist or resolves
to a non-taxonomy ontology.
- Query param `?node=<name>` reframes the response root at a
descendant node. 404 when no such node exists in the taxonomy.
- Query param `?depth=<n>` limits recursion to N levels below the
response root. `depth=0` returns just the response root with no
children. Truncated branches that had children are serialized with
`values=[]` so the wire format distinguishes them from real leaves
(which have no `values` key).
- Response envelope `{"taxonomy": {...}}` matches the precedent in
this file. The base `Ontology.to_dict` is called explicitly so the
full root tree isn't serialized just to be overwritten.
Tree-walking helpers live on `Node`:
- `Node.find(name)` — first-match descendant lookup (node names are
unique within a taxonomy).
- `Node.truncated(depth)` — returns a copy truncated to N levels.
Tests cover all paths: full-tree fetch, node-query subtree, unknown
node 404, depth=0 root-only, depth=1 with truncated-branch and
real-leaf markers, node+depth combined, invalid/negative depth 400.
…g Nodes For large taxonomies the dominant cost of the previous endpoint was ``Node.from_dict`` recursively constructing dataclass instances over the whole tree (~400ms for 50k nodes). The endpoint is read-only and only needs to slice and project a subtree, so it now operates on the dict returned by pymongo and never hydrates Nodes. - ``OntologyTaxonomy.get`` is async-native via motor; no ``run_sync_task`` wrapper. - The Mongo fetch lives in ``_load_taxonomy_doc(name)``; the subtree filter (``?node=``, ``?depth=``) lives in ``_select_subtree(...)``; ``_load_taxonomy_response`` composes them. - ``find_in_dict`` and ``truncate_dict`` move out of the route file into ``fiftyone/core/annotation/nodes.py`` alongside ``Node``. The earlier ``Node.find`` / ``Node.truncated`` methods are removed — the endpoint was their only caller. Benchmark on the 50k-node stress taxonomy (whole-pipeline wall time): query before after full ~575 ms ~69 ms (8.3x) depth=0 ~554 ms ~21 ms (26.7x) depth=1 ~469 ms ~20 ms (23.1x) depth=2 ~548 ms ~20 ms (27.3x) Small / medium taxonomies were already sub-10ms and stay there. Tests: swap the ``Node.find`` / ``Node.truncated`` tests for equivalent ``find_in_dict`` / ``truncate_dict`` tests; the existing endpoint tests pass without modification.
…d label schema validation to support the new taxonomy attribute
…abel schema validation tests
…isplay currently selected value
… tooltip if they are not taxonomy elligible
… to be consistent with the rest of the file
nverjinski
force-pushed
the
taxonomy/fe-integration-attribute-schema
branch
from
d616c58 to
faeec89
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/packages/core/src/components/Modal/Sidebar/Annotate/SchemaManager/utils.ts`:
- Around line 387-396: The taxonomy branch is still serializing the default
value even though default inputs are hidden; update the object returned when
data.valuesMode === "taxonomy" to omit the default property (do not include the
"default" key) so stale unseen defaults are not persisted. Locate the branch
that checks data.valuesMode === "taxonomy" in utils.ts and remove or
conditionally exclude the default: defaultValue entry (ensure other fields like
name, type, component, range, read_only, taxonomy remain unchanged).
In `@tests/unittests/ontology_class_tests.py`:
- Around line 1194-1257: The file contains duplicate test class definitions for
FindInDictTests and TruncateDictTests that shadow earlier definitions; remove
the second set of definitions (the redefinitions of the classes FindInDictTests
and TruncateDictTests) so only the original test classes remain, preserving the
individual test methods that should be kept and ensuring imports of find_in_dict
and truncate_dict used in those tests remain in place.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9c642689-de0d-405e-b7cb-b35655e02905
📒 Files selected for processing (12)
app/packages/core/src/components/Modal/Sidebar/Annotate/SchemaManager/EditFieldLabelSchema/ApplyOntologySection.tsxapp/packages/core/src/components/Modal/Sidebar/Annotate/SchemaManager/EditFieldLabelSchema/GUIContent/AttributeFormContent.tsxapp/packages/core/src/components/Modal/Sidebar/Annotate/SchemaManager/EditFieldLabelSchema/GUIContent/useAttributeForm.tsapp/packages/core/src/components/Modal/Sidebar/Annotate/SchemaManager/EditFieldLabelSchema/OntologyPicker.tsxapp/packages/core/src/components/Modal/Sidebar/Annotate/SchemaManager/EditFieldLabelSchema/useOntologies.tsapp/packages/core/src/components/Modal/Sidebar/Annotate/SchemaManager/utils.tsfiftyone/core/annotation/attributes.pyfiftyone/core/annotation/constants.pyfiftyone/core/annotation/validate_label_schemas.pytests/unittests/annotation/attribute_spec_tests.pytests/unittests/annotation/validate_label_schemas_tests.pytests/unittests/ontology_class_tests.py
erik-nieh
requested changes
Jun 8, 2026
erik-nieh
left a comment
erik-nieh
left a comment
Contributor
There was a problem hiding this comment.
Generally looks good, just some minor comments.
| <ToggleSwitch | ||
| key={formState.valuesMode} | ||
| size={Size.Sm} | ||
| defaultIndex={formState.valuesMode === "taxonomy" ? 1 : 0} |
Contributor
There was a problem hiding this comment.
because these strings are referenced in many places it might be nice to convert them to enums
| ); | ||
|
|
||
| const handleValuesModeChange = useCallback( | ||
| (mode: "simple" | "taxonomy") => { |
Contributor
There was a problem hiding this comment.
and then enum would make this type implicit i think
| TextColor, | ||
| TextVariant, | ||
| Toggle, | ||
| ToggleSwitch, |
Contributor
There was a problem hiding this comment.
is this functional with the voodoo change? like could merge this and then bump the voodoo lesion version later?
|
|
||
|
|
||
| def _validate_applied_ontology(field_name: str, value: str) -> None: | ||
| def _validate_taxonomy_setting(field_name, taxonomy): |
Contributor
There was a problem hiding this comment.
I think it'd be worth extending this validation to make the ontology actually loads. Look in the validate applied ontology just below this.
|
|
||
|
|
||
| def _validate_applied_ontology(field_name: str, value: str) -> None: | ||
| def _validate_taxonomy_setting(field_name, taxonomy): |
Contributor
There was a problem hiding this comment.
Also, this is not strictly enforced in the code base, but I think it's a good precedence. It's just making sure these functions are typed. field_name: str, taxonomy_name: str
7 tasks
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
2 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.
Note:
This PR should be merged after: #7605
This PR should have its Voodo version updated and be merged after: voxel51/design-system#142
📋 What changes are proposed in this pull request?
This PR seeks to extend field schemas in order to make it possible to define attributes that leverage Taxonomies for their values. When a user creates a new attribute of
strorlist<str>type and also sets the component type todropdown, they should be able to select a predefinedTaxonomyas the values of the dropdown. I extended the BE to accept a newtaxonomyattribute onAttributeSpecand added validation relating to this new attribute invalidate_label_schemas.py. This PR also includes changes to the front end UI.A separate, upcoming PR will update the annotation smart form to consume this new attribute and render the new voodo
TreeSelectcomponent.🧪 How is this patch tested? If it is not, please explain why.
Define a taxonomy with this script:
create_vehicle_taxonomy.py
stringorstring listas the type.Dropdownas the component.taxonomyfrom the dropdownScreen.Recording.2026-06-05.at.3.01.24.PM.mov
📝 Release Notes
Is this a user-facing change that should be mentioned in the release notes?
notes for FiftyOne users.
Added ability to choose a predefined taxonomy in lieu of manually defining values for a dropdown based schema attribute.
What areas of FiftyOne does this PR affect?
fiftyonePython library changesSummary by CodeRabbit
Release Notes
New Features
Improvements
Enterprise sync PR: https://github.com/voxel51/fiftyone-teams/pull/2822