From 0235bd59e9695a27ddcc12872f35b796d00f9afd Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Tue, 1 Sep 2026 17:23:19 -0400 Subject: [PATCH 1/9] Add atomic type hierarchy editing operations --- client/dive-common/typeHierarchy.spec.ts | 152 ++++++++++++++ client/dive-common/typeHierarchy.ts | 76 ++++++- client/src/TrackFilterControls.spec.ts | 244 +++++++++++++++++++++-- client/src/TrackFilterControls.ts | 140 ++++++++++--- 4 files changed, 563 insertions(+), 49 deletions(-) diff --git a/client/dive-common/typeHierarchy.spec.ts b/client/dive-common/typeHierarchy.spec.ts index 4eea571e1..264fe93a4 100644 --- a/client/dive-common/typeHierarchy.spec.ts +++ b/client/dive-common/typeHierarchy.spec.ts @@ -1,18 +1,22 @@ import fs from 'fs-extra'; import { acceptPairAsCorrect, + compareTypeNames, compileHierarchy, mergePairs, normalizeTypeHierarchy, reassignPairs, + removeHierarchyType, removePair, resolveConfidenceThreshold, resolveTypeHierarchy, rewriteHierarchyType, selectFlatPairIndex, selectPairIndex, + setHierarchyParent, setPairConfidence, TypeHierarchyError, + updateHierarchyTypeDefinition, } from './typeHierarchy'; interface ErrorExpectation { @@ -251,6 +255,154 @@ describe('type hierarchy index', () => { }); }); +describe('hierarchy editing transformations', () => { + describe('setHierarchyParent', () => { + it('sets the first edge from an absent hierarchy', () => { + expect(setHierarchyParent(undefined, 'cod', 'fish')).toEqual({ cod: 'fish' }); + }); + + it('reparents a child and clears an edge without disturbing other branches', () => { + const hierarchy = { cod: 'fish', tern: 'bird' }; + expect(setHierarchyParent(hierarchy, 'cod', 'animal')).toEqual({ + cod: 'animal', + tern: 'bird', + }); + expect(setHierarchyParent(hierarchy, 'cod', undefined)).toEqual({ tern: 'bird' }); + }); + + it('returns undefined after clearing the final edge', () => { + expect(setHierarchyParent({ cod: 'fish' }, 'cod', undefined)).toBeUndefined(); + }); + + it('rejects blank names, self edges, and cycles', () => { + expectHierarchyError( + () => setHierarchyParent(undefined, ' ', 'fish'), + 'empty child', + 'malformed', + ); + expectHierarchyError( + () => setHierarchyParent(undefined, 'cod', '\u001c'), + 'empty parent for "cod"', + 'malformed', + ); + expectHierarchyError( + () => setHierarchyParent(undefined, 'cod', 'cod'), + 'self edge "cod -> cod"', + 'malformed', + ); + expectHierarchyError( + () => setHierarchyParent({ cod: 'fish' }, 'fish', 'cod'), + 'cycle cod -> fish -> cod', + 'malformed', + ); + }); + + it('returns a fresh map without changing its input', () => { + const hierarchy = Object.freeze({ cod: 'fish', tern: 'bird' }); + const result = setHierarchyParent(hierarchy, 'cod', 'fish'); + expect(result).toEqual(hierarchy); + expect(result).not.toBe(hierarchy); + expect(hierarchy).toEqual({ cod: 'fish', tern: 'bird' }); + }); + + it('orders keys by code point', () => { + const bmp = '\uE000'; + const astral = '\u{10000}'; + const result = setHierarchyParent({ [astral]: 'root' }, bmp, 'root'); + expect(compareTypeNames(bmp, astral)).toBeLessThan(0); + expect(Object.keys(result || {})).toEqual([bmp, astral]); + }); + }); + + describe('removeHierarchyType', () => { + it('removes a middle node and promotes all of its children', () => { + expect(removeHierarchyType({ + cod: 'fish', + haddock: 'fish', + fish: 'animal', + tern: 'bird', + }, 'fish')).toEqual({ + cod: 'animal', + haddock: 'animal', + tern: 'bird', + }); + }); + + it('makes children of a removed top-level node top level', () => { + expect(removeHierarchyType({ + cod: 'fish', + haddock: 'fish', + tern: 'bird', + }, 'fish')).toEqual({ tern: 'bird' }); + }); + + it('removes a leaf and returns undefined when the final edge is removed', () => { + expect(removeHierarchyType({ cod: 'fish', tern: 'bird' }, 'cod')) + .toEqual({ tern: 'bird' }); + expect(removeHierarchyType({ cod: 'fish' }, 'cod')).toBeUndefined(); + }); + + it('normalizes an absent-type no-op into a fresh map', () => { + const hierarchy = Object.freeze({ cod: 'fish', tern: 'bird' }); + const result = removeHierarchyType(hierarchy, 'shark'); + expect(result).toEqual(hierarchy); + expect(result).not.toBe(hierarchy); + expect(hierarchy).toEqual({ cod: 'fish', tern: 'bird' }); + }); + }); + + describe('updateHierarchyTypeDefinition', () => { + it('builds a valid rename and reparent result without validating an invalid intermediate map', () => { + const hierarchy = { + cod: 'fish', + haddock: 'animal', + sole: 'cod', + tern: 'bird', + }; + expect(() => rewriteHierarchyType(hierarchy, 'cod', 'haddock')) + .toThrow('conflicting parents for "haddock": "animal" and "fish"'); + + expect(updateHierarchyTypeDefinition( + hierarchy, + 'cod', + 'haddock', + 'fish', + )).toEqual({ + haddock: 'fish', + sole: 'haddock', + tern: 'bird', + }); + }); + + it('rewrites child references while applying the final edited parent', () => { + expect(updateHierarchyTypeDefinition( + { cod: 'fish', fish: 'animal' }, + 'fish', + 'vertebrate', + 'life', + )).toEqual({ cod: 'vertebrate', vertebrate: 'life' }); + }); + + it('validates the complete final map and leaves its input unchanged', () => { + const hierarchy = Object.freeze({ cod: 'fish', fish: 'animal' }); + expectHierarchyError( + () => updateHierarchyTypeDefinition(hierarchy, 'fish', 'fish', 'cod'), + 'cycle cod -> fish -> cod', + 'malformed', + ); + expect(hierarchy).toEqual({ cod: 'fish', fish: 'animal' }); + }); + + it('rejects a blank final name even when it would have no edge', () => { + expectHierarchyError( + () => updateHierarchyTypeDefinition(undefined, 'cod', ' ', undefined), + 'empty child', + 'malformed', + ); + }); + }); +}); + describe('flat pair selection', () => { const pairs: [string, number][] = [['top', 0.5], ['fallback', 0.8]]; diff --git a/client/dive-common/typeHierarchy.ts b/client/dive-common/typeHierarchy.ts index 0a24f98fa..402dd5d94 100644 --- a/client/dive-common/typeHierarchy.ts +++ b/client/dive-common/typeHierarchy.ts @@ -64,7 +64,7 @@ export function selectFlatPairIndex( // Python orders strings by code point; JS compares UTF-16 units, which sorts astral // names before U+E000-U+FFFF. Compare code points so both platforms agree. -function codePointCompare(left: string, right: string): number { +export function compareTypeNames(left: string, right: string): number { const leftPoints = [...left].map((char) => char.codePointAt(0) as number); const rightPoints = [...right].map((char) => char.codePointAt(0) as number); const sharedLength = Math.min(leftPoints.length, rightPoints.length); @@ -77,7 +77,7 @@ function codePointCompare(left: string, right: string): number { } function sortedNames(names: readonly string[]): string[] { - return [...names].sort(codePointCompare); + return [...names].sort(compareTypeNames); } function hasOwn(hierarchy: TypeHierarchy, type: string): boolean { @@ -115,7 +115,7 @@ function cycleReason(hierarchy: TypeHierarchy): string | undefined { const cycle = path.slice(positions.get(current) as number); let smallestIndex = 0; cycle.forEach((name, index) => { - if (codePointCompare(name, cycle[smallestIndex]) < 0) { + if (compareTypeNames(name, cycle[smallestIndex]) < 0) { smallestIndex = index; } }); @@ -128,7 +128,7 @@ function cycleReason(hierarchy: TypeHierarchy): string | undefined { if (renderedCycles.length === 0) { return undefined; } - renderedCycles.sort(codePointCompare); + renderedCycles.sort(compareTypeNames); return `cycle ${renderedCycles[0]}`; } @@ -317,6 +317,72 @@ export function rewriteHierarchyType( } } +export function setHierarchyParent( + hierarchy: TypeHierarchy | undefined, + child: string, + parent: string | undefined, +): TypeHierarchy | undefined { + const normalized = normalizeTypeHierarchy(hierarchy || {}) || {}; + if (isBlankName(child)) { + throw new TypeHierarchyError('empty child'); + } + + const updated = new Map(Object.entries(normalized)); + updated.delete(child); + if (parent !== undefined) { + updated.set(child, parent); + } + return normalizeTypeHierarchy(Object.fromEntries(updated)); +} + +export function removeHierarchyType( + hierarchy: TypeHierarchy, + type: string, +): TypeHierarchy | undefined { + const normalized = normalizeTypeHierarchy(hierarchy) || {}; + const parent = hasOwn(normalized, type) ? normalized[type] : undefined; + const updated = new Map(); + + sortedNames(Object.keys(normalized)).forEach((child) => { + if (child === type) { + return; + } + const currentParent = normalized[child]; + if (currentParent !== type) { + updated.set(child, currentParent); + } else if (parent !== undefined) { + updated.set(child, parent); + } + }); + return normalizeTypeHierarchy(Object.fromEntries(updated)); +} + +/** Build the final hierarchy for an atomic type rename and parent edit. */ +export function updateHierarchyTypeDefinition( + hierarchy: TypeHierarchy | undefined, + currentType: string, + newType: string, + parent: string | undefined, +): TypeHierarchy | undefined { + const normalized = normalizeTypeHierarchy(hierarchy || {}) || {}; + if (isBlankName(currentType) || isBlankName(newType)) { + throw new TypeHierarchyError('empty child'); + } + + const updated = new Map(); + sortedNames(Object.keys(normalized)).forEach((child) => { + if (child === currentType || child === newType) { + return; + } + const currentParent = normalized[child]; + updated.set(child, currentParent === currentType ? newType : currentParent); + }); + if (parent !== undefined) { + updated.set(newType, parent); + } + return normalizeTypeHierarchy(Object.fromEntries(updated)); +} + // Assignment replaces the selected claim's lineage while retaining unrelated claims and the // stored ancestors still implied by the new type. No missing hierarchy members are synthesized. export function reassignPairs( @@ -381,7 +447,7 @@ export function mergePairs( }); }); return Array.from(confidenceByType.entries()) - .sort((left, right) => (right[1] - left[1]) || codePointCompare(left[0], right[0])); + .sort((left, right) => (right[1] - left[1]) || compareTypeNames(left[0], right[0])); } export function selectPairIndex( diff --git a/client/src/TrackFilterControls.spec.ts b/client/src/TrackFilterControls.spec.ts index f8cf9d394..0ec175251 100644 --- a/client/src/TrackFilterControls.spec.ts +++ b/client/src/TrackFilterControls.spec.ts @@ -747,6 +747,145 @@ describe('useAnnotationFilters', () => { expect(groupFilters.configuredTypes.value).not.toContain('renamed group'); }); + it('creates the first parent edge without changing stored pairs', () => { + const markPending = vi.fn(); + const { cameraStore, filters } = makePairFixture([ + [['leaf', 0.8], ['root', 0.4]], + ], markPending); + markPending.mockClear(); + + filters.updateTypeDefinition({ + currentType: 'leaf', + newType: 'leaf', + parent: 'root', + }); + + expect(filters.typeHierarchy.value).toEqual({ leaf: 'root' }); + expect(cameraStore.getTrack(0).confidencePairs).toEqual([ + ['leaf', 0.8], ['root', 0.4], + ]); + expect(filters.typeHierarchySavePatch()).toEqual({ typeHierarchy: { leaf: 'root' } }); + expect(markPending).toHaveBeenCalledTimes(1); + expect(markPending).toHaveBeenCalledWith({ action: 'meta' }); + }); + + it('reparents a used type without changing stored pairs and can detach the final edge', () => { + const markPending = vi.fn(); + const { cameraStore, filters } = makePairFixture([ + [['leaf', 0.8], ['root', 0.4], ['animal', 0.2]], + ], markPending); + filters.setTypeHierarchy({ leaf: 'root' }); + markPending.mockClear(); + + filters.updateTypeDefinition({ + currentType: 'leaf', + newType: 'leaf', + parent: 'animal', + }); + expect(filters.typeHierarchy.value).toEqual({ leaf: 'animal' }); + expect(cameraStore.getTrack(0).confidencePairs).toEqual([ + ['leaf', 0.8], ['root', 0.4], ['animal', 0.2], + ]); + + filters.updateTypeDefinition({ + currentType: 'leaf', + newType: 'leaf', + parent: undefined, + }); + expect(filters.typeHierarchy.value).toBeUndefined(); + expect(filters.hierarchyActive.value).toBe(false); + expect(filters.typeHierarchySavePatch()).toEqual({ typeHierarchy: null }); + expect(cameraStore.getTrack(0).confidencePairs).toEqual([ + ['leaf', 0.8], ['root', 0.4], ['animal', 0.2], + ]); + expect(markPending).toHaveBeenCalledTimes(2); + }); + + it('renames and reparents through one validated update', () => { + const markPending = vi.fn(); + const { cameraStore, filters } = makePairFixture([ + [['leaf', 0.8], ['root', 0.4], ['animal', 0.2]], + ], markPending); + filters.importTypes(['leaf'], false); + filters.setConfidenceFilters({ leaf: 0.5, default: 0.1 }); + filters.setTypeHierarchy({ leaf: 'root' }); + markPending.mockClear(); + + filters.updateTypeDefinition({ + currentType: 'leaf', + newType: 'fin', + parent: 'animal', + }); + + expect(filters.typeHierarchy.value).toEqual({ fin: 'animal' }); + expect(cameraStore.getTrack(0).confidencePairs).toEqual([ + ['fin', 0.8], ['root', 0.4], ['animal', 0.2], + ]); + expect(filters.configuredTypes.value).toEqual(['fin']); + expect(filters.confidenceFilters.value).toEqual({ fin: 0.5, default: 0.1 }); + expect(filters.checkedTypes.value).toContain('fin'); + expect(filters.typeHierarchySavePatch()).toEqual({ typeHierarchy: { fin: 'animal' } }); + expect(markPending.mock.calls.filter(([data]) => data?.action === 'meta')).toHaveLength(1); + }); + + it('validates only the final map when a parent edit resolves a rename conflict', () => { + const { filters } = makePairFixture([ + [['cod', 0.8], ['bird', 0.2]], + [['haddock', 0.7]], + ]); + filters.setTypeHierarchy({ + cod: 'fish', + haddock: 'animal', + sole: 'cod', + }); + + filters.updateTypeDefinition({ + currentType: 'cod', + newType: 'haddock', + parent: 'bird', + }); + + expect(filters.typeHierarchy.value).toEqual({ + haddock: 'bird', + sole: 'haddock', + }); + }); + + it('does nothing when the name and parent are unchanged', () => { + const markPending = vi.fn(); + const { filters } = makePairFixture([[['leaf', 1], ['root', 0.8]]], markPending); + filters.setTypeHierarchy({ leaf: 'root' }); + markPending.mockClear(); + + filters.updateTypeDefinition({ + currentType: 'leaf', + newType: 'leaf', + parent: 'root', + }); + + expect(filters.typeHierarchySavePatch()).toEqual({}); + expect(markPending).not.toHaveBeenCalled(); + }); + + it('repairs invalid stored hierarchy by assigning an available parent', () => { + const markPending = vi.fn(); + const { filters } = makePairFixture([[['leaf', 1], ['root', 0.8]]], markPending); + filters.setTypeHierarchy({ leaf: 'leaf' }); + expect(filters.invalidHierarchyReason.value).not.toBeNull(); + markPending.mockClear(); + + filters.updateTypeDefinition({ + currentType: 'leaf', + newType: 'leaf', + parent: 'root', + }); + + expect(filters.invalidHierarchyReason.value).toBeNull(); + expect(filters.typeHierarchy.value).toEqual({ leaf: 'root' }); + expect(filters.typeHierarchySavePatch()).toEqual({ typeHierarchy: { leaf: 'root' } }); + expect(markPending).toHaveBeenCalledTimes(1); + }); + it('renames assigned group pairs across camera replicas without collapsing the vector', () => { const cameraStore = new CameraStore({ markChangesPending }); cameraStore.removeCamera('singleCam'); @@ -796,6 +935,35 @@ describe('useAnnotationFilters', () => { expect(filters.typeHierarchySavePatch()).toEqual({ typeHierarchy: { fin: 'root' } }); }); + it('preserves an existing destination parent during a rename-only merge', () => { + const { filters } = makePairFixture([ + [['heading', 0.8]], + [['animal', 0.7]], + ]); + filters.setTypeHierarchy({ leaf: 'heading', animal: 'root' }); + + filters.updateTypeName({ currentType: 'heading', newType: 'animal' }); + + expect(filters.typeHierarchy.value).toEqual({ animal: 'root', leaf: 'animal' }); + }); + + it('preserves the landed conflicting-parent rejection for rename-only callers', () => { + const markPending = vi.fn(); + const { cameraStore, filters } = makePairFixture([ + [['cod', 0.8]], + [['haddock', 0.7]], + ], markPending); + filters.setTypeHierarchy({ cod: 'fish', haddock: 'animal' }); + markPending.mockClear(); + + expect(() => filters.updateTypeName({ currentType: 'cod', newType: 'haddock' })) + .toThrow('conflicting parents for "haddock"'); + expect(filters.typeHierarchy.value).toEqual({ cod: 'fish', haddock: 'animal' }); + expect(cameraStore.getTrack(0).confidencePairs).toEqual([['cod', 0.8]]); + expect(cameraStore.getTrack(1).confidencePairs).toEqual([['haddock', 0.7]]); + expect(markPending).not.toHaveBeenCalled(); + }); + it('does not configure a hierarchy-only heading during a name-only rename', () => { const { filters } = makePairFixture([[['leaf', 1]]]); filters.setTypeHierarchy({ leaf: 'heading' }); @@ -849,6 +1017,30 @@ describe('useAnnotationFilters', () => { expect(markPending).not.toHaveBeenCalled(); }); + it('rejects an invalid combined rename and parent before changing any state', () => { + const markPending = vi.fn(); + const { cameraStore, filters } = makePairFixture([[['leaf', 0.8]]], markPending); + filters.importTypes(['leaf'], false); + filters.setConfidenceFilters({ leaf: 0.4, default: 0.1 }); + filters.setTypeHierarchy({ leaf: 'root', child: 'leaf' }); + const checkedBefore = [...filters.checkedTypes.value]; + markPending.mockClear(); + + expect(() => filters.updateTypeDefinition({ + currentType: 'leaf', + newType: 'fin', + parent: 'child', + })).toThrow(TypeHierarchyError); + + expect(cameraStore.getTrack(0).confidencePairs).toEqual([['leaf', 0.8]]); + expect(filters.typeHierarchy.value).toEqual({ child: 'leaf', leaf: 'root' }); + expect(filters.configuredTypes.value).toEqual(['leaf']); + expect(filters.confidenceFilters.value).toEqual({ leaf: 0.4, default: 0.1 }); + expect(filters.checkedTypes.value).toEqual(checkedBefore); + expect(filters.typeHierarchySavePatch()).toEqual({}); + expect(markPending).not.toHaveBeenCalled(); + }); + it('rejects a rename when one track already has both names', () => { const markPending = vi.fn(); const { cameraStore, filters } = makePairFixture([ @@ -887,22 +1079,44 @@ describe('useAnnotationFilters', () => { expect(markPending).not.toHaveBeenCalled(); }); - it('clears settings for unused parents and leaves hierarchy state unchanged', () => { + it('deletes an unused parent, promotes its child, and clears type settings', () => { const markPending = vi.fn(); const { filters } = makePairFixture([[['used', 1]]], markPending); - filters.importTypes(['leaf'], false); - filters.setConfidenceFilters({ leaf: 0.4, default: 0.1 }); + filters.importTypes(['parent'], false); + filters.setConfidenceFilters({ parent: 0.4, default: 0.1 }); filters.setTypeHierarchy({ leaf: 'parent', parent: 'root' }); markPending.mockClear(); - const hierarchyBefore = { ...filters.typeHierarchy.value }; - const checkedBefore = [...filters.checkedTypes.value]; + expect(filters.deleteType('parent')).toBe(true); - expect(filters.deleteType('leaf')).toBe(true); - expect(filters.typeHierarchy.value).toEqual(hierarchyBefore); - expect(filters.configuredTypes.value).not.toContain('leaf'); - expect(filters.confidenceFilters.value).not.toHaveProperty('leaf'); - expect(filters.checkedTypes.value).toEqual(checkedBefore); - expect(markPending).toHaveBeenCalledTimes(2); + expect(filters.typeHierarchy.value).toEqual({ leaf: 'root' }); + expect(filters.typeHierarchySavePatch()).toEqual({ typeHierarchy: { leaf: 'root' } }); + expect(filters.configuredTypes.value).not.toContain('parent'); + expect(filters.confidenceFilters.value).not.toHaveProperty('parent'); + expect(filters.checkedTypes.value).not.toContain('parent'); + expect(markPending).toHaveBeenCalledTimes(1); + }); + + it('allows a used descendant when deleting its unused parent', () => { + const { cameraStore, filters } = makePairFixture([[['leaf', 1]]]); + filters.setTypeHierarchy({ leaf: 'parent', parent: 'root' }); + + expect(filters.deleteType('parent')).toBe(true); + expect(filters.typeHierarchy.value).toEqual({ leaf: 'root' }); + expect(cameraStore.getTrack(0).confidencePairs).toEqual([['leaf', 1]]); + }); + + it('keeps flat deletion behavior for a configured type outside the hierarchy', () => { + const markPending = vi.fn(); + const { filters } = makePairFixture([[['leaf', 1]]], markPending); + filters.setTypeHierarchy({ leaf: 'root' }); + filters.importTypes(['configured'], false); + markPending.mockClear(); + + expect(filters.deleteType('configured')).toBe(true); + expect(filters.typeHierarchy.value).toEqual({ leaf: 'root' }); + expect(filters.typeHierarchySavePatch()).toEqual({}); + expect(filters.configuredTypes.value).not.toContain('configured'); + expect(markPending).toHaveBeenCalledTimes(1); }); it('blocks deleting a type that a divergent camera still uses', () => { @@ -929,12 +1143,12 @@ describe('useAnnotationFilters', () => { expect(markPending).not.toHaveBeenCalled(); }); - it('keeps hierarchy active after clearing the final leaf settings', () => { + it('deletes the final edge and restores flat behavior', () => { const { filters } = makePairFixture([[['used', 1]]]); filters.setTypeHierarchy({ leaf: 'root' }); expect(filters.deleteType('leaf')).toBe(true); - expect(filters.hierarchyActive.value).toBe(true); - expect(filters.typeHierarchy.value).toEqual({ leaf: 'root' }); - expect(filters.typeHierarchySavePatch()).toEqual({}); + expect(filters.hierarchyActive.value).toBe(false); + expect(filters.typeHierarchy.value).toBeUndefined(); + expect(filters.typeHierarchySavePatch()).toEqual({ typeHierarchy: null }); }); }); diff --git a/client/src/TrackFilterControls.ts b/client/src/TrackFilterControls.ts index 2efb8008c..41c864aea 100644 --- a/client/src/TrackFilterControls.ts +++ b/client/src/TrackFilterControls.ts @@ -4,13 +4,14 @@ import { clientSettings } from 'dive-common/store/settings'; import { compileHierarchy, normalizeTypeHierarchy, + removeHierarchyType, resolveConfidenceThreshold, selectFlatPairIndex, - rewriteHierarchyType, selectPairIndex, TypeHierarchy, TypeHierarchyError, TypeHierarchyIndex, + updateHierarchyTypeDefinition, } from 'dive-common/typeHierarchy'; import { AnnotationId } from './BaseAnnotation'; import BaseFilterControls, { AnnotationWithContext, FilterControlsParams } from './BaseFilterControls'; @@ -260,60 +261,141 @@ export default class TrackFilterControls extends BaseFilterControls { this.deleteType(currentType); return; } - const tracks = this.sorted.value.flatMap((annotation) => this.getTracks(annotation.id)); - const collision = tracks.find((track) => { - const names = new Set(track.confidencePairs.map(([name]) => name)); - return names.has(currentType) && names.has(newType); + this.updateTypeDefinition({ + currentType, + newType, + parent: this.typeHierarchy.value?.[currentType], }); - if (collision) { + } + + updateTypeDefinition({ + currentType, + newType, + parent, + }: { + currentType: string; + newType: string; + parent: string | undefined; + }) { + if (parent === currentType && currentType !== newType) { + throw new TypeHierarchyError( + `the original type "${currentType}" cannot be its renamed type's parent`, + 'conflict', + ); + } + if (parent !== undefined && !this.allTypes.value.includes(parent)) { throw new TypeHierarchyError( - `track ${collision.id} already contains both "${currentType}" and "${newType}"`, + `parent "${parent}" is not an existing type`, 'conflict', ); } - const currentHierarchy = this.typeHierarchy.value as TypeHierarchy; - const rewritten = rewriteHierarchyType(currentHierarchy, currentType, newType); - const hierarchyChanged = !isEqual(currentHierarchy, rewritten); + const currentHierarchy = this.typeHierarchy.value; + const currentHasParent = currentHierarchy !== undefined + && Object.prototype.hasOwnProperty.call(currentHierarchy, currentType); + const destinationHasParent = currentHierarchy !== undefined + && Object.prototype.hasOwnProperty.call(currentHierarchy, newType); + const currentParent = currentHasParent ? currentHierarchy?.[currentType] : undefined; + let destinationParent = destinationHasParent ? currentHierarchy?.[newType] : undefined; + if (destinationParent === currentType) { + destinationParent = newType; + } + const parentChanged = parent !== currentParent; + let finalParent = parent; + if (currentType !== newType && destinationHasParent && !parentChanged) { + if (currentHasParent && currentParent !== destinationParent) { + throw new TypeHierarchyError( + `conflicting parents for "${newType}": "${destinationParent}" and "${currentParent}"`, + 'conflict', + ); + } + if (!currentHasParent) { + finalParent = destinationParent; + } + } + const nextHierarchy = updateHierarchyTypeDefinition( + currentHierarchy, + currentType, + newType, + finalParent, + ); + const nameChanged = currentType !== newType; + const hierarchyChanged = !isEqual(currentHierarchy, nextHierarchy) + || (this.invalidHierarchyReason.value !== null && finalParent !== undefined); + const hierarchyInvolved = currentHierarchy !== undefined || nextHierarchy !== undefined; + if (!nameChanged && !hierarchyChanged) { + return; + } + if (!hierarchyInvolved) { + this.updateTypeName({ currentType, newType }); + return; + } + + if (nameChanged) { + const collision = this.sorted.value + .flatMap((annotation) => this.getTracks(annotation.id)) + .find((track) => { + const names = new Set(track.confidencePairs.map(([name]) => name)); + return names.has(currentType) && names.has(newType); + }); + if (collision) { + throw new TypeHierarchyError( + `track ${collision.id} already contains both "${currentType}" and "${newType}"`, + 'conflict', + ); + } + } + const currentWasChecked = this.checkedTypes.value.includes(currentType); const newWasChecked = this.checkedTypes.value.includes(newType); + const currentWasConfigured = this.configuredTypes.value.includes(currentType); - this.sorted.value.forEach((annotation) => { - if (this.getTracks(annotation.id) - .some((track) => track.confidencePairs.some(([name]) => name === currentType))) { - this.renameTrackPair(annotation.id, currentType, newType); + if (nameChanged) { + this.sorted.value.forEach((annotation) => { + if (this.getTracks(annotation.id) + .some((track) => track.confidencePairs.some(([name]) => name === currentType))) { + this.renameTrackPair(annotation.id, currentType, newType); + } + }); + this.carryConfidenceFilter(currentType, newType); + if (currentWasConfigured && !this.configuredTypes.value.includes(newType)) { + this.configuredTypes.value.push(newType); } - }); - this.carryConfidenceFilter(currentType, newType); - if (this.configuredTypes.value.includes(currentType) - && !this.configuredTypes.value.includes(newType)) { - this.configuredTypes.value.push(newType); + this.deleteTypeConfiguration(currentType); } - this.deleteTypeConfiguration(currentType); if (hierarchyChanged) { - this.installTypeHierarchy(rewritten, true); + this.installTypeHierarchy(nextHierarchy, true); } const checked = new Set(this.checkedTypes.value); - if (!currentWasChecked && !newWasChecked) { - checked.delete(newType); - } else if (currentWasChecked) { - checked.add(newType); - } - if (!this.allTypes.value.includes(currentType)) { - checked.delete(currentType); + if (nameChanged) { + if (!currentWasChecked && !newWasChecked) { + checked.delete(newType); + } else if (currentWasChecked) { + checked.add(newType); + } + if (!this.allTypes.value.includes(currentType)) { + checked.delete(currentType); + } } this.checkedTypes.value = Array.from(checked); this.markChangesPending({ action: 'meta' }); } deleteType(type: string): boolean { - if (!this.hierarchyActive.value) { + const currentHierarchy = this.typeHierarchy.value; + const hierarchyMember = currentHierarchy !== undefined + && (Object.prototype.hasOwnProperty.call(currentHierarchy, type) + || Object.values(currentHierarchy).includes(type)); + if (!hierarchyMember) { return super.deleteType(type); } if (this.typeInUseOnAnyCamera(type)) { return false; } + const nextHierarchy = removeHierarchyType(currentHierarchy, type); + this.installTypeHierarchy(nextHierarchy, true); + this.checkedTypes.value = this.checkedTypes.value.filter((name) => name !== type); this.deleteTypeConfiguration(type); this.markChangesPending({ action: 'meta' }); return true; From 615e08ed0379ac1b2ea5d316cc3a7448641e8571 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Tue, 1 Sep 2026 17:23:31 -0400 Subject: [PATCH 2/9] Add parent editing to the type editor --- client/src/components/FilterList.spec.ts | 32 +++ client/src/components/FilterList.vue | 8 +- client/src/components/TypeEditor.spec.ts | 325 +++++++++++++++++++---- client/src/components/TypeEditor.vue | 176 +++++++++--- 4 files changed, 457 insertions(+), 84 deletions(-) diff --git a/client/src/components/FilterList.spec.ts b/client/src/components/FilterList.spec.ts index 1f875f451..56720455e 100644 --- a/client/src/components/FilterList.spec.ts +++ b/client/src/components/FilterList.spec.ts @@ -9,6 +9,7 @@ import BaseFilterControls from '../BaseFilterControls'; import Group from '../Group'; import CameraStore from '../CameraStore'; import FilterList from './FilterList.vue'; +import TypeEditor from './TypeEditor.vue'; vi.mock('dive-common/vue-utilities/prompt-service', () => ({ usePrompt: () => ({ prompt: vi.fn(), visible: () => false }), @@ -21,6 +22,7 @@ const provideMocks = vi.hoisted(() => ({ annotationMap: new Map(), selectedCameraValue: 'singleCam', selectedCameraRef: undefined as { value: string } | undefined, + datasetIdRef: undefined as { value: string } | undefined, })); /** @@ -64,6 +66,11 @@ vi.mock('../provides', () => ({ }]])), }), useHandler: () => ({ seekFrame: provideMocks.seekFrame }), + useDatasetId: () => { + const datasetId = ref('dataset-a'); + provideMocks.datasetIdRef = datasetId; + return datasetId; + }, useReadOnlyMode: () => ref(false), useSelectedCamera: () => { const selectedCamera = ref(provideMocks.selectedCameraValue); @@ -182,10 +189,35 @@ describe('FilterList hierarchy members', () => { provideMocks.annotationMap.clear(); provideMocks.selectedCameraValue = 'singleCam'; provideMocks.selectedCameraRef = undefined; + provideMocks.datasetIdRef = undefined; clientSettings.typeSettings.showTotalCount = true; clientSettings.typeSettings.showFrameCount = true; }); + it('discards an open Type Editor draft when the dataset changes', async () => { + const { filterControls, styleManager } = makeHierarchyFixture(); + const { vm, wrapper } = mountFilterList({ + filterControls, + styleManager, + showEmptyTypes: true, + height: 240, + headerHeight: 80, + }); + vm.clickEdit('leaf'); + await nextTick(); + expect(vm.data.showPicker).toBe(true); + expect(wrapper.findComponent(TypeEditor).exists()).toBe(true); + + if (!provideMocks.datasetIdRef) { + throw new Error('Dataset ID ref was not initialized'); + } + provideMocks.datasetIdRef.value = 'dataset-b'; + await nextTick(); + expect(vm.data.showPicker).toBe(false); + expect(vm.data.selectedType).toBe(''); + expect(wrapper.findComponent(TypeEditor).exists()).toBe(false); + }); + it('keeps members as ordinary, independently checked flat rows', async () => { clientSettings.typeSettings.trackSortDir = 'a-z'; clientSettings.typeSettings.filterTypesByFrame = false; diff --git a/client/src/components/FilterList.vue b/client/src/components/FilterList.vue index 702112322..5e0f60253 100644 --- a/client/src/components/FilterList.vue +++ b/client/src/components/FilterList.vue @@ -11,7 +11,7 @@ import { usePrompt } from 'dive-common/vue-utilities/prompt-service'; import { clientSettings } from 'dive-common/store/settings'; import { compileHierarchy } from 'dive-common/typeHierarchy'; import { - useCameraStore, useHandler, useReadOnlyMode, useSelectedCamera, useTime, + useCameraStore, useDatasetId, useHandler, useReadOnlyMode, useSelectedCamera, useTime, usePendingSaveCount, } from '../provides'; import TooltipBtn from './TooltipButton.vue'; @@ -109,6 +109,7 @@ export default defineComponent({ const { prompt } = usePrompt(); const handler = useHandler(); const readOnlyMode = useReadOnlyMode(); + const datasetId = useDatasetId(); const cameraStore = useCameraStore(); const selectedCamera = useSelectedCamera(); const { frame } = useTime(); @@ -160,6 +161,10 @@ export default defineComponent({ compactSharedLineage.value = true; }); } + watch(datasetId, () => { + data.showPicker = false; + data.selectedType = ''; + }); function clickEdit(type: string) { data.selectedType = type; @@ -738,6 +743,7 @@ export default defineComponent({ width="350" > vi.fn()); +const provideMocks = vi.hoisted(() => ({ readOnly: false })); vi.mock('dive-common/vue-utilities/prompt-service', () => ({ usePrompt: () => ({ prompt: promptMock }), })); vi.mock('../provides', () => ({ - useReadOnlyMode: () => ref(false), + useReadOnlyMode: () => ref(provideMocks.readOnly), })); -function makeFilters() { +function makeFilters({ + allTypes = ['leaf', 'root'], + hierarchy = { leaf: 'root' } as Record | undefined, +} = {}) { const filters = Object.create(TrackFilterControls.prototype) as TrackFilterControls; + filters.allTypes = ref(allTypes); filters.usedTypes = ref([]); + filters.typeHierarchy = ref(hierarchy); filters.typeInUseOnAnyCamera = vi.fn(() => false); + filters.updateTypeDefinition = vi.fn(); filters.updateTypeName = vi.fn(); filters.importTypes = vi.fn(); filters.deleteType = vi.fn(() => true); return filters; } +function makeGroupFilters() { + const filters = Object.create(BaseFilterControls.prototype) as BaseFilterControls; + filters.usedTypes = ref([]); + filters.updateTypeName = vi.fn(); + filters.deleteType = vi.fn(() => true); + return filters; +} + function makeStyleManager() { return Object.freeze({ typeStyling: ref({ - color: () => '#123456', + color: (type: string) => `color:${type}`, strokeWidth: () => 3, fill: () => false, opacity: () => 0.8, labelSettings: () => ({ showLabel: true, showConfidence: true }), }), updateTypeStyle: vi.fn(), + renameTypeStyle: vi.fn(), + deleteTypeStyle: vi.fn(), }); } +interface MountOptions { + selectedType?: string; + filters?: BaseFilterControls | TrackFilterControls | null; + styleManager?: ReturnType; + group?: boolean; + styleOnly?: boolean; +} + /** * `@vue/test-utils` types a mount target as a Vue 2 constructor, which a `defineComponent` * SFC is not, so the editor is rendered from a host that captures the real instance and its * emitted events. It is left unstubbed to keep shallow semantics for its own children. */ -function mountEditor(filters = makeFilters(), styleManager = makeStyleManager()) { +function mountEditor({ + selectedType = 'leaf', + filters = makeFilters(), + styleManager = makeStyleManager(), + group = false, + styleOnly = false, +}: MountOptions = {}) { const closeEvents: unknown[] = []; + const filterControls = filters ?? undefined; + const props = shallowReactive({ + selectedType, + filterControls, + styleManager, + group, + styleOnly, + }); let child: InstanceType | undefined; const Host = defineComponent({ setup: () => () => h(TypeEditor, { - props: { - selectedType: 'leaf', - filterControls: filters, - styleManager, - }, + props, on: { close: () => closeEvents.push([]) }, ref: (instance) => { if (instance && !(instance instanceof Element)) { @@ -64,68 +102,265 @@ function mountEditor(filters = makeFilters(), styleManager = makeStyleManager()) if (!child) { throw new Error('TypeEditor did not mount'); } + const setProps = async (next: Partial) => { + Object.assign(props, next); + await nextTick(); + }; return { - filters, styleManager, wrapper, vm: child, closeEvents, + filters: filterControls as TrackFilterControls, + styleManager, + wrapper, + vm: child, + closeEvents, + setProps, }; } -describe('TypeEditor hierarchy safety', () => { - beforeEach(() => promptMock.mockReset()); +describe('TypeEditor hierarchy editing', () => { + beforeEach(() => { + promptMock.mockReset(); + provideMocks.readOnly = false; + }); - it('allows clearing settings for an unused hierarchy parent', async () => { - const filters = makeFilters(); - promptMock.mockResolvedValue(true); - const { vm } = mountEditor(filters); - await vm.clickDeleteType('leaf'); - expect(promptMock).toHaveBeenCalled(); - expect(filters.deleteType).toHaveBeenCalledWith('leaf'); + it('shows Parent Type only for dataset track types and initializes its parent', () => { + const track = mountEditor(); + expect(track.vm.showParentType).toBe(true); + expect(track.vm.data.editingParent).toBe('root'); + const autocomplete = track.wrapper.find('v-autocomplete'); + expect(autocomplete.exists()).toBe(true); + expect(autocomplete.attributes('auto-select-first')).toBe(''); + expect(autocomplete.attributes('no-data-text')).toBe( + 'No matching type. Add it from Type Settings first.', + ); + + const group = mountEditor({ filters: makeGroupFilters(), group: true }); + expect(group.vm.showParentType).toBe(false); + expect(group.wrapper.find('v-autocomplete').exists()).toBe(false); + + const savedStyle = mountEditor({ filters: null, styleOnly: true }); + expect(savedStyle.vm.showParentType).toBe(false); + expect(savedStyle.wrapper.find('v-autocomplete').exists()).toBe(false); }); - it('disables deletion for a type used only by a camera the merged view hides', () => { - const filters = makeFilters(); - vi.mocked(filters.typeInUseOnAnyCamera).mockReturnValue(true); - const { vm, wrapper } = mountEditor(filters); - expect(vm.deleteBlocked).toBe(true); - expect(wrapper.text()).toContain('Only types without any annotations can be deleted.'); + it('bounds empty-query choices with the selected parent first', () => { + const names = [ + 'leaf', + 'current', + ...Array.from({ length: 100 }, (_, i) => `type${i.toString().padStart(3, '0')}`), + ]; + const { vm } = mountEditor({ + filters: makeFilters({ allTypes: names, hierarchy: { leaf: 'current' } }), + }); + expect(vm.parentOptions).toHaveLength(50); + expect(vm.parentOptions[0]).toBe('current'); + expect(vm.parentOptions.slice(1)).toEqual([...vm.parentOptions.slice(1)].sort()); }); - it('leaves an unused leaf unchanged when deletion is canceled', async () => { - promptMock.mockResolvedValue(false); - const { filters, vm, closeEvents } = mountEditor(); - await vm.clickDeleteType('leaf'); - expect(promptMock).toHaveBeenCalledTimes(1); - expect(filters.deleteType).not.toHaveBeenCalled(); + it('orders prefix matches before substring matches and excludes old and final names', async () => { + const { vm } = mountEditor({ + filters: makeFilters({ + allTypes: ['leaf', 'fin', 'current', 'alpine', 'alpha', 'coral'], + hierarchy: { leaf: 'current' }, + }), + }); + vm.data.editingType = 'fin'; + vm.data.parentSearch = 'al'; + await nextTick(); + expect(vm.parentOptions).toEqual(['current', 'alpha', 'alpine', 'coral']); + expect(vm.parentOptions).not.toContain('leaf'); + expect(vm.parentOptions).not.toContain('fin'); + }); + + it('sorts parent choices by Unicode code point', () => { + const privateUse = '\uE000'; + const astral = '\u{10000}'; + const { vm } = mountEditor({ + filters: makeFilters({ + allTypes: ['leaf', 'current', astral, privateUse], + hierarchy: { leaf: 'current' }, + }), + }); + expect(vm.parentOptions).toEqual(['current', privateUse, astral]); + }); + + it('does not save free text that was not selected', () => { + const { + filters, styleManager, vm, closeEvents, + } = mountEditor(); + vm.data.parentSearch = 'not an existing type'; + vm.acceptChanges(); + expect(vm.parentSearchUnresolved).toBe(true); + expect(filters.updateTypeDefinition).not.toHaveBeenCalled(); + expect(styleManager.updateTypeStyle).not.toHaveBeenCalled(); expect(closeEvents).toHaveLength(0); }); - it('keeps the editor open for a rejected rename and allows a corrected retry', () => { + it('accepts a cleared parent draft without pointer input', () => { + const { vm } = mountEditor(); + vm.data.editingParent = null; + vm.data.parentSearch = null; + expect(vm.data.editingParent).toBeNull(); + expect(vm.data.parentSearch).toBeNull(); + }); + + it('saves name and parent through one atomic operation', () => { + const { filters, vm, closeEvents } = mountEditor(); + vm.data.editingType = 'fin'; + vm.data.editingParent = 'branch'; + vm.data.parentSearch = 'branch'; + vm.acceptChanges(); + expect(filters.updateTypeDefinition).toHaveBeenCalledTimes(1); + expect(filters.updateTypeDefinition).toHaveBeenCalledWith({ + currentType: 'leaf', newType: 'fin', parent: 'branch', + }); + expect(closeEvents).toHaveLength(1); + }); + + it('applies first-edge creation and final-edge clearing through the same operation', () => { + const first = mountEditor({ + filters: makeFilters({ allTypes: ['leaf', 'root'], hierarchy: undefined }), + }); + first.vm.data.editingParent = 'root'; + first.vm.data.parentSearch = 'root'; + first.vm.acceptChanges(); + expect(first.filters.updateTypeDefinition).toHaveBeenCalledWith({ + currentType: 'leaf', newType: 'leaf', parent: 'root', + }); + + const final = mountEditor(); + final.vm.data.editingParent = null; + final.vm.data.parentSearch = null; + final.vm.acceptChanges(); + expect(final.filters.updateTypeDefinition).toHaveBeenCalledWith({ + currentType: 'leaf', newType: 'leaf', parent: undefined, + }); + }); + + it('keeps all style changes staged when hierarchy preflight fails', () => { const { filters, styleManager, vm, closeEvents, } = mountEditor(); - vi.mocked(filters.updateTypeName).mockImplementationOnce(() => { + vi.mocked(filters.updateTypeDefinition).mockImplementationOnce(() => { throw new TypeHierarchyError('self edge "root -> root"', 'conflict'); }); vm.data.editingType = 'root'; + vm.data.editingParent = 'root'; + vm.data.parentSearch = 'root'; + vm.data.editingColor = '#abcdef'; vm.acceptChanges(); - expect(vm.data.renameError).toBe( - 'Type hierarchy is invalid: self edge "root -> root". No types were changed.', + expect(vm.data.definitionError).toBe( + 'Type hierarchy is invalid: self edge "root -> root". No type changes were applied.', ); + expect(filters.importTypes).not.toHaveBeenCalled(); expect(styleManager.updateTypeStyle).not.toHaveBeenCalled(); expect(closeEvents).toHaveLength(0); + }); - vm.data.editingType = 'fin'; - vm.acceptChanges(); - expect(vm.data.renameError).toBe(''); - expect(filters.updateTypeName).toHaveBeenLastCalledWith({ - currentType: 'leaf', newType: 'fin', + it('preserves group and Saved Styles rename paths', () => { + const groupFilters = makeGroupFilters(); + const group = mountEditor({ filters: groupFilters, group: true }); + group.vm.data.editingType = 'renamed-group'; + group.vm.acceptChanges(); + expect(groupFilters.updateTypeName).toHaveBeenCalledWith({ + currentType: 'leaf', newType: 'renamed-group', }); - expect(closeEvents).toHaveLength(1); + + const style = mountEditor({ filters: null, styleOnly: true }); + style.vm.data.editingType = 'renamed-style'; + style.vm.acceptChanges(); + expect(style.styleManager.renameTypeStyle).toHaveBeenCalledWith('leaf', 'renamed-style'); + }); + + it('prompts truthfully when deleting hierarchy parents and leaves cancellation unchanged', async () => { + const middle = mountEditor({ + selectedType: 'branch', + filters: makeFilters({ + allTypes: ['root', 'branch', 'leaf'], + hierarchy: { branch: 'root', leaf: 'branch' }, + }), + }); + promptMock.mockResolvedValueOnce(false); + await middle.vm.clickDeleteType('branch'); + expect(promptMock).toHaveBeenLastCalledWith({ + title: 'Confirm', + text: 'Remove "branch" from the hierarchy? Its children will move under "root". Stored annotations will not be changed.', + confirm: true, + }); + expect(middle.filters.deleteType).not.toHaveBeenCalled(); + + const root = mountEditor({ + selectedType: 'root', + filters: makeFilters({ + allTypes: ['root', 'leaf'], hierarchy: { leaf: 'root' }, + }), + }); + promptMock.mockResolvedValueOnce(true); + await root.vm.clickDeleteType('root'); + expect(promptMock).toHaveBeenLastCalledWith({ + title: 'Confirm', + text: 'Remove "root" from the hierarchy? Its children will become top-level types. Stored annotations will not be changed.', + confirm: true, + }); + expect(root.filters.deleteType).toHaveBeenCalledWith('root'); + + const leaf = mountEditor(); + promptMock.mockResolvedValueOnce(true); + await leaf.vm.clickDeleteType('leaf'); + expect(promptMock).toHaveBeenLastCalledWith({ + title: 'Confirm', text: 'Delete the unused type "leaf"?', confirm: true, + }); + }); + + it('does not finish an asynchronous deletion after the editor unmounts', async () => { + let resolvePrompt: ((value: boolean) => void) | undefined; + promptMock.mockReturnValueOnce(new Promise((resolve) => { + resolvePrompt = resolve; + })); + const { + filters, vm, wrapper, closeEvents, + } = mountEditor(); + const deletion = vm.clickDeleteType('leaf'); + wrapper.destroy(); + if (!resolvePrompt) { + throw new Error('Prompt resolver was not initialized'); + } + resolvePrompt(true); + await deletion; + expect(filters.deleteType).not.toHaveBeenCalled(); + expect(closeEvents).toHaveLength(0); + }); + + it('blocks deletion for all-camera usage and disables both name fields in Read Only Mode', () => { + provideMocks.readOnly = true; + const filters = makeFilters(); + vi.mocked(filters.typeInUseOnAnyCamera).mockReturnValue(true); + const { vm, wrapper } = mountEditor({ filters }); + expect(vm.deleteBlocked).toBe(true); + expect(wrapper.text()).toContain('Only types without annotations can be deleted.'); + expect(wrapper.find('v-autocomplete').attributes('disabled')).toBe('true'); + expect(wrapper.find('v-text-field').attributes('disabled')).toBe('true'); + }); + + it('reinitializes the complete draft when the selected type changes', async () => { + const filters = makeFilters({ + allTypes: ['leaf', 'root', 'other', 'branch'], + hierarchy: { leaf: 'root', other: 'branch' }, + }); + const { vm, setProps } = mountEditor({ filters }); + vm.data.editingParent = 'branch'; + vm.data.editingColor = '#abcdef'; + await setProps({ selectedType: 'other' }); + expect(vm.data.selectedType).toBe('other'); + expect(vm.data.editingType).toBe('other'); + expect(vm.data.editingParent).toBe('branch'); + expect(vm.data.parentSearch).toBe('branch'); + expect(vm.data.editingColor).toBe('color:other'); }); it('promotes a hierarchy-only heading only after a style value changes', () => { - const { filters, vm } = mountEditor(); - vm.acceptChanges(); - expect(filters.importTypes).not.toHaveBeenCalled(); + const unchanged = mountEditor(); + unchanged.vm.acceptChanges(); + expect(unchanged.filters.importTypes).not.toHaveBeenCalled(); const changed = mountEditor(); changed.vm.data.editingColor = '#abcdef'; diff --git a/client/src/components/TypeEditor.vue b/client/src/components/TypeEditor.vue index 89885ec40..69cbe61f2 100644 --- a/client/src/components/TypeEditor.vue +++ b/client/src/components/TypeEditor.vue @@ -1,10 +1,10 @@ + + diff --git a/client/src/components/TypeEditor.spec.ts b/client/src/components/TypeEditor.spec.ts index 4a241dfdf..20b87609c 100644 --- a/client/src/components/TypeEditor.spec.ts +++ b/client/src/components/TypeEditor.spec.ts @@ -5,6 +5,7 @@ import { shallowMount } from '@vue/test-utils'; import { TypeHierarchyError } from 'dive-common/typeHierarchy'; import BaseFilterControls from '../BaseFilterControls'; import TrackFilterControls from '../TrackFilterControls'; +import ParentTypePicker from './ParentTypePicker.vue'; import TypeEditor from './TypeEditor.vue'; const promptMock = vi.hoisted(() => vi.fn()); @@ -99,7 +100,7 @@ function mountEditor({ }, }), }); - const wrapper = shallowMount(Host, { stubs: { TypeEditor: false } }); + const wrapper = shallowMount(Host, { stubs: { TypeEditor: false, ParentTypePicker: false } }); if (!child) { throw new Error('TypeEditor did not mount'); } @@ -127,75 +128,40 @@ describe('TypeEditor hierarchy editing', () => { const track = mountEditor(); expect(track.vm.showParentType).toBe(true); expect(track.vm.data.editingParent).toBe('root'); - const autocomplete = track.wrapper.find('v-autocomplete'); - expect(autocomplete.exists()).toBe(true); - expect(autocomplete.attributes('auto-select-first')).toBe(''); - expect(autocomplete.attributes('no-filter')).toBe(''); - expect(autocomplete.attributes('no-data-text')).toBe( - 'No matching type. Add it from Type Settings first.', - ); + expect(track.wrapper.findComponent(ParentTypePicker).props('value')).toBe('root'); expect(track.wrapper.find('v-text-field').attributes('hide-details')).toBe('auto'); const group = mountEditor({ filters: makeGroupFilters(), group: true }); expect(group.vm.showParentType).toBe(false); - expect(group.wrapper.find('v-autocomplete').exists()).toBe(false); + expect(group.wrapper.findComponent(ParentTypePicker).exists()).toBe(false); const savedStyle = mountEditor({ filters: null, styleOnly: true }); expect(savedStyle.vm.showParentType).toBe(false); - expect(savedStyle.wrapper.find('v-autocomplete').exists()).toBe(false); + expect(savedStyle.wrapper.findComponent(ParentTypePicker).exists()).toBe(false); }); - it('bounds empty-query choices with the selected parent first', () => { - const names = [ - 'leaf', - 'current', - ...Array.from({ length: 100 }, (_, i) => `type${i.toString().padStart(3, '0')}`), - ]; - const { vm } = mountEditor({ - filters: makeFilters({ allTypes: names, hierarchy: { leaf: 'current' } }), - }); - expect(vm.parentOptions).toHaveLength(50); - expect(vm.parentOptions[0]).toBe('current'); - expect(vm.parentOptions.slice(1)).toEqual([...vm.parentOptions.slice(1)].sort()); - }); - - it('orders prefix matches before substring matches and excludes old and final names', async () => { - const { vm } = mountEditor({ - filters: makeFilters({ - allTypes: ['leaf', 'fin', 'current', 'alpine', 'alpha', 'coral'], - hierarchy: { leaf: 'current' }, - }), - }); + it('passes available types and both edited names to the picker', async () => { + const { vm, wrapper, filters } = mountEditor(); vm.data.editingType = 'fin'; - vm.data.parentSearch = 'al'; await nextTick(); - expect(vm.parentOptions).toEqual(['current', 'alpha', 'alpine', 'coral']); - expect(vm.parentOptions).not.toContain('leaf'); - expect(vm.parentOptions).not.toContain('fin'); - }); - - it('sorts parent choices by Unicode code point', () => { - const privateUse = '\uE000'; - const astral = '\u{10000}'; - const { vm } = mountEditor({ - filters: makeFilters({ - allTypes: ['leaf', 'current', astral, privateUse], - hierarchy: { leaf: 'current' }, - }), - }); - expect(vm.parentOptions).toEqual(['current', privateUse, astral]); + const picker = wrapper.findComponent(ParentTypePicker); + expect(picker.props('allTypes')).toEqual(filters.allTypes.value); + expect(picker.props('excludedTypes')).toEqual(['leaf', 'fin']); }); - it('does not save free text that was not selected', () => { + it('blocks saving while the picker reports unresolved text and recovers when resolved', () => { const { - filters, styleManager, vm, closeEvents, + filters, styleManager, vm, closeEvents, wrapper, } = mountEditor(); - vm.data.parentSearch = 'not an existing type'; + const picker = wrapper.findComponent(ParentTypePicker); + picker.vm.$emit('search-valid', false); vm.acceptChanges(); - expect(vm.parentSearchUnresolved).toBe(true); + expect(vm.saveDisabled).toBe(true); expect(filters.updateTypeDefinition).not.toHaveBeenCalled(); expect(styleManager.updateTypeStyle).not.toHaveBeenCalled(); expect(closeEvents).toHaveLength(0); + picker.vm.$emit('search-valid', true); + expect(vm.saveDisabled).toBe(false); }); it('shows every known preflight restriction inline and disables Save', async () => { @@ -205,12 +171,11 @@ describe('TypeEditor hierarchy editing', () => { reason: 'cycle leaf -> branch -> leaf', }); parent.vm.data.editingParent = 'branch'; - parent.vm.data.parentSearch = 'branch'; await nextTick(); expect(parent.vm.parentDefinitionError).toBe( 'Type hierarchy is invalid: cycle leaf -> branch -> leaf.', ); - expect(parent.wrapper.find('v-autocomplete').attributes('error-messages')).toBe( + expect(parent.wrapper.findComponent(ParentTypePicker).props('errorMessage')).toBe( 'Type hierarchy is invalid: cycle leaf -> branch -> leaf.', ); expect(parent.vm.saveDisabled).toBe(true); @@ -231,21 +196,14 @@ describe('TypeEditor hierarchy editing', () => { 'Type hierarchy is invalid: track 4 already contains both "leaf" and "root".', ); expect(name.vm.saveDisabled).toBe(true); - - const unresolved = mountEditor(); - unresolved.vm.data.parentSearch = 'missing'; - await nextTick(); - expect(unresolved.vm.parentDefinitionError).toBe( - 'Select an existing type from the list, or clear the field.', - ); - expect(unresolved.vm.saveDisabled).toBe(true); }); it('saves name and parent through one atomic operation', () => { - const { filters, vm, closeEvents } = mountEditor(); + const { + filters, vm, closeEvents, wrapper, + } = mountEditor(); vm.data.editingType = 'fin'; - vm.data.editingParent = 'branch'; - vm.data.parentSearch = 'branch'; + wrapper.findComponent(ParentTypePicker).vm.$emit('input', 'branch'); vm.acceptChanges(); expect(filters.updateTypeDefinition).toHaveBeenCalledTimes(1); expect(filters.updateTypeDefinition).toHaveBeenCalledWith({ @@ -258,16 +216,14 @@ describe('TypeEditor hierarchy editing', () => { const first = mountEditor({ filters: makeFilters({ allTypes: ['leaf', 'root'], hierarchy: undefined }), }); - first.vm.data.editingParent = 'root'; - first.vm.data.parentSearch = 'root'; + first.wrapper.findComponent(ParentTypePicker).vm.$emit('input', 'root'); first.vm.acceptChanges(); expect(first.filters.updateTypeDefinition).toHaveBeenCalledWith({ currentType: 'leaf', newType: 'leaf', parent: 'root', }); const final = mountEditor(); - final.vm.data.editingParent = null; - final.vm.data.parentSearch = null; + final.wrapper.findComponent(ParentTypePicker).vm.$emit('input', null); final.vm.acceptChanges(); expect(final.filters.updateTypeDefinition).toHaveBeenCalledWith({ currentType: 'leaf', newType: 'leaf', parent: undefined, @@ -283,7 +239,6 @@ describe('TypeEditor hierarchy editing', () => { }); vm.data.editingType = 'root'; vm.data.editingParent = 'root'; - vm.data.parentSearch = 'root'; vm.data.editingColor = '#abcdef'; vm.acceptChanges(); expect(vm.data.definitionError).toBe( @@ -375,23 +330,25 @@ describe('TypeEditor hierarchy editing', () => { const { vm, wrapper } = mountEditor({ filters }); expect(vm.deleteBlocked).toBe(true); expect(wrapper.text()).toContain('Only types without annotations can be deleted.'); - expect(wrapper.find('v-autocomplete').attributes('disabled')).toBe('true'); + expect(wrapper.findComponent(ParentTypePicker).props('disabled')).toBe(true); expect(wrapper.find('v-text-field').attributes('disabled')).toBe('true'); }); - it('reinitializes the complete draft when the selected type changes', async () => { + it('reinitializes the draft and picker when types change, even with the same parent', async () => { const filters = makeFilters({ allTypes: ['leaf', 'root', 'other', 'branch'], - hierarchy: { leaf: 'root', other: 'branch' }, + hierarchy: { leaf: 'root', other: 'root' }, }); - const { vm, setProps } = mountEditor({ filters }); - vm.data.editingParent = 'branch'; + const { vm, setProps, wrapper } = mountEditor({ filters }); + const previousPicker = wrapper.findComponent(ParentTypePicker).vm; + previousPicker.$emit('search-valid', false); vm.data.editingColor = '#abcdef'; await setProps({ selectedType: 'other' }); expect(vm.data.selectedType).toBe('other'); expect(vm.data.editingType).toBe('other'); - expect(vm.data.editingParent).toBe('branch'); - expect(vm.data.parentSearch).toBe('branch'); + expect(vm.data.editingParent).toBe('root'); + expect(wrapper.findComponent(ParentTypePicker).vm).not.toBe(previousPicker); + expect(vm.data.parentSearchValid).toBe(true); expect(vm.data.editingColor).toBe('color:other'); }); diff --git a/client/src/components/TypeEditor.vue b/client/src/components/TypeEditor.vue index 913182e2c..3486c1b93 100644 --- a/client/src/components/TypeEditor.vue +++ b/client/src/components/TypeEditor.vue @@ -4,7 +4,7 @@ import { } from 'vue'; import { usePrompt } from 'dive-common/vue-utilities/prompt-service'; -import { compareTypeNames, TypeHierarchyError } from 'dive-common/typeHierarchy'; +import { TypeHierarchyError } from 'dive-common/typeHierarchy'; import TrackFilterControls from '../TrackFilterControls'; import BaseFilterControls from '../BaseFilterControls'; @@ -12,12 +12,13 @@ import type Group from '../Group'; import type StyleManager from '../StyleManager'; import type Track from '../track'; import { useReadOnlyMode } from '../provides'; - -const MAX_PARENT_OPTIONS = 50; +import ParentTypePicker from './ParentTypePicker.vue'; export default defineComponent({ name: 'TypeEditor', + components: { ParentTypePicker }, + props: { selectedType: { type: String, @@ -67,7 +68,7 @@ export default defineComponent({ selectedType: '', editingType: '', editingParent: null as string | null, - parentSearch: null as string | null, + parentSearchValid: true, editingColor: '', editingThickness: 5, editingFill: false, @@ -78,45 +79,9 @@ export default defineComponent({ definitionError: '', }); - const parentQuery = computed(() => { - const search = data.parentSearch ?? ''; - return search === data.editingParent ? '' : search; - }); - const parentOptions = computed(() => { - const controls = trackFilters.value; - if (!controls) { - return []; - } - const excluded = new Set([data.selectedType, data.editingType]); - const query = parentQuery.value.toLowerCase(); - const prefixMatches: string[] = []; - const substringMatches: string[] = []; - controls.allTypes.value.forEach((type) => { - if (excluded.has(type)) { - return; - } - const candidate = type.toLowerCase(); - if (candidate.startsWith(query)) { - prefixMatches.push(type); - } else if (candidate.includes(query)) { - substringMatches.push(type); - } - }); - prefixMatches.sort(compareTypeNames); - substringMatches.sort(compareTypeNames); - - const currentParent = data.editingParent; - const options = [...prefixMatches, ...substringMatches] - .filter((type) => type !== currentParent); - if (currentParent !== null && !excluded.has(currentParent)) { - options.unshift(currentParent); - } - return options.slice(0, MAX_PARENT_OPTIONS); - }); - const parentSearchUnresolved = computed(() => parentQuery.value !== ''); const definitionValidation = computed(() => { const controls = trackFilters.value; - if (!controls || !data.editingType.trim() || parentSearchUnresolved.value) { + if (!controls || !data.editingType.trim() || !data.parentSearchValid) { return undefined; } return controls.validateTypeDefinition({ @@ -130,16 +95,14 @@ export default defineComponent({ ? `Type hierarchy is invalid: ${definitionValidation.value.reason}.` : '' )); - const parentDefinitionError = computed(() => { - if (parentSearchUnresolved.value) { - return 'Select an existing type from the list, or clear the field.'; - } - return definitionValidation.value?.field === 'parent' + const parentDefinitionError = computed(() => ( + definitionValidation.value?.field === 'parent' ? `Type hierarchy is invalid: ${definitionValidation.value.reason}.` - : ''; - }); + : '' + )); const saveDisabled = computed(() => ( - !data.valid || nameDefinitionError.value !== '' || parentDefinitionError.value !== '' + !data.valid || !data.parentSearchValid + || nameDefinitionError.value !== '' || parentDefinitionError.value !== '' )); const currentStyleValue = () => ({ @@ -232,7 +195,7 @@ export default defineComponent({ data.selectedType = props.selectedType; data.editingType = props.selectedType; data.editingParent = trackFilters.value?.typeHierarchy.value?.[props.selectedType] ?? null; - data.parentSearch = data.editingParent; + data.parentSearchValid = true; const typeStyling = props.styleManager.typeStyling.value; data.editingColor = typeStyling.color(props.selectedType); data.editingThickness = typeStyling.strokeWidth(props.selectedType); @@ -255,8 +218,7 @@ export default defineComponent({ isStyleOnly, showParentType, readOnlyMode, - parentOptions, - parentSearchUnresolved, + parentTypes: computed(() => trackFilters.value?.allTypes.value ?? []), nameDefinitionError, parentDefinitionError, saveDisabled, @@ -326,20 +288,14 @@ export default defineComponent({ - From 87a94aba5db5910305ee6edaa6902630fc3d79c8 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Tue, 1 Sep 2026 21:32:42 -0400 Subject: [PATCH 8/9] Trim parent type selector documentation --- docs/UI-Type-List.md | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/docs/UI-Type-List.md b/docs/UI-Type-List.md index 9297b6741..30d971c22 100644 --- a/docs/UI-Type-List.md +++ b/docs/UI-Type-List.md @@ -40,7 +40,7 @@ highest-confidence pair, keeping the first stored pair when scores tie, because have each viewer's checked types and confidence thresholds. Viewer counts and filtering use the hierarchy-resolved displayed type. -Hierarchy members render as an expandable tree rather than ordinary flat rows. Parent checkboxes control their complete subtrees, and parent counts include descendants. To edit a type's hierarchy relationship, open its existing pencil editor and select its immediate **Parent Type**. The searchable list includes existing configured, used, and hierarchy-only track types. If the parent does not exist yet, add it through **+ Types** in Type Settings first. Clear **Parent Type** to make the edited type top-level. +Hierarchy members render as an expandable tree rather than ordinary flat rows. Parent checkboxes control their complete subtrees, and parent counts include descendants. A parent row's checkbox always toggles its complete subtree. The heading checkbox is narrower: it skips a parent shown only as ancestor context for a match below it. @@ -48,11 +48,6 @@ A parent row's checkbox always toggles its complete subtree. The heading checkbo While a hierarchy is active, **Prevent Cascade Types** is disabled and shows: `Not applicable to hierarchical types; DIVE selects the deepest qualifying type.` Its saved value is preserved and becomes active again when the hierarchy is removed. -Changes to **Type Name** and **Parent Type** are staged together and applied by one Save after the complete result passes validation. A rename involved in a hierarchy is rejected if one track already contains both the old and new type names. An invalid final hierarchy, such as a self-edge, conflicting parent, or cycle, is also rejected without applying either the name or parent change. -Restrictions that can be determined from the draft are shown directly under the affected field as soon as they are known, and Save remains disabled until the draft is valid. Save still validates the complete result again before applying it. - -Deleting an unused hierarchy heading removes that heading while preserving its descendants. Its immediate children move under the deleted heading's parent, or become top-level when the deleted heading had no parent. This promotion does not rename descendants or rewrite their stored annotation confidence pairs. - ## Type Style Editor ![Type Editor](images/TypeEditor.png){ align=right loading=lazy width=260 } @@ -61,7 +56,7 @@ The type style editor controls the visual appearance of annotations in all other * **Type Name** - You can change the name for the type and it will update all subsequent tracks that are using that Type. -* **Parent Type** - Search for and select the type's immediate parent. Clear the field to make the type top-level. Add a missing parent through **+ Types** in Type Settings before selecting it. +* **Parent Type** - Select a parent for the type, or clear it to make the type top-level. * **Show Label** - show the type name label in the text above each box. * **Show Confidence** - show the confidence value in the text above each box. * **Box Border Thickness** - the line thickness can be changed to make a type stand out more or less From ab8d1dff0e2d1add62ed5072eb98c0a17e1f2bd6 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Tue, 1 Sep 2026 21:38:01 -0400 Subject: [PATCH 9/9] Shorten Prevent Cascade documentation --- docs/UI-Type-List.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/UI-Type-List.md b/docs/UI-Type-List.md index 30d971c22..328e63247 100644 --- a/docs/UI-Type-List.md +++ b/docs/UI-Type-List.md @@ -46,7 +46,7 @@ A parent row's checkbox always toggles its complete subtree. The heading checkbo **Compact Parents** folds unused shared parent rows into a breadcrumb. Used or configured parents remain as rows. **Expand Parents** restores the full chain, and searching shows a match's complete path. -While a hierarchy is active, **Prevent Cascade Types** is disabled and shows: `Not applicable to hierarchical types; DIVE selects the deepest qualifying type.` Its saved value is preserved and becomes active again when the hierarchy is removed. +While a hierarchy is active, **Prevent Cascade Types** is disabled and shows: `Not applicable to hierarchical types; DIVE selects the deepest qualifying type.` ## Type Style Editor