diff --git a/client/dive-common/typeHierarchy.spec.ts b/client/dive-common/typeHierarchy.spec.ts index 4eea571e1..aad1dca14 100644 --- a/client/dive-common/typeHierarchy.spec.ts +++ b/client/dive-common/typeHierarchy.spec.ts @@ -1,10 +1,12 @@ import fs from 'fs-extra'; import { acceptPairAsCorrect, + compareTypeNames, compileHierarchy, mergePairs, normalizeTypeHierarchy, reassignPairs, + removeHierarchyType, removePair, resolveConfidenceThreshold, resolveTypeHierarchy, @@ -13,6 +15,7 @@ import { selectPairIndex, setPairConfidence, TypeHierarchyError, + updateHierarchyTypeDefinition, } from './typeHierarchy'; interface ErrorExpectation { @@ -251,6 +254,134 @@ describe('type hierarchy index', () => { }); }); +describe('hierarchy editing transformations', () => { + 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('sets, reparents, and clears edges without mutating unrelated branches', () => { + expect(updateHierarchyTypeDefinition(undefined, 'cod', 'cod', 'fish')) + .toEqual({ cod: 'fish' }); + const hierarchy = { cod: 'fish', tern: 'bird' }; + expect(updateHierarchyTypeDefinition(hierarchy, 'cod', 'cod', 'animal')).toEqual({ + cod: 'animal', + tern: 'bird', + }); + expect(updateHierarchyTypeDefinition(hierarchy, 'cod', 'cod', undefined)) + .toEqual({ tern: 'bird' }); + expect(updateHierarchyTypeDefinition({ cod: 'fish' }, 'cod', 'cod', undefined)) + .toBeUndefined(); + }); + + 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 blank parents and self edges', () => { + expectHierarchyError( + () => updateHierarchyTypeDefinition(undefined, 'cod', 'cod', '\u001c'), + 'empty parent for "cod"', + 'malformed', + ); + expectHierarchyError( + () => updateHierarchyTypeDefinition(undefined, 'cod', 'cod', 'cod'), + 'self edge "cod -> cod"', + 'malformed', + ); + }); + + it('returns a fresh map ordered by code point', () => { + const bmp = '\uE000'; + const astral = '\u{10000}'; + const hierarchy = Object.freeze({ [astral]: 'root' }); + const result = updateHierarchyTypeDefinition(hierarchy, bmp, bmp, 'root'); + expect(compareTypeNames(bmp, astral)).toBeLessThan(0); + expect(Object.keys(result || {})).toEqual([bmp, astral]); + expect(result).not.toBe(hierarchy); + expect(hierarchy).toEqual({ [astral]: 'root' }); + }); + + 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..b956d21d8 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,54 @@ export function rewriteHierarchyType( } } +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 +429,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..87193478e 100644 --- a/client/src/TrackFilterControls.spec.ts +++ b/client/src/TrackFilterControls.spec.ts @@ -747,6 +747,162 @@ 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('keeps a hierarchy-only leaf when detaching its final parent edge', () => { + const { filters } = makePairFixture([[['used', 1]]]); + filters.setTypeHierarchy({ leaf: 'root' }); + + filters.updateTypeDefinition({ + currentType: 'leaf', + newType: 'leaf', + parent: undefined, + }); + + expect(filters.typeHierarchy.value).toBeUndefined(); + expect(filters.configuredTypes.value).toContain('leaf'); + expect(filters.configuredTypes.value).toContain('root'); + expect(filters.allTypes.value).toContain('leaf'); + expect(filters.allTypes.value).toContain('root'); + }); + + 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 +952,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 +1034,74 @@ 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('preflights all type-definition restrictions without changing state', () => { + const markPending = vi.fn(); + const { cameraStore, filters } = makePairFixture([ + [['leaf', 0.8], ['fin', 0.7]], + ], markPending); + filters.setTypeHierarchy({ leaf: 'root', child: 'leaf' }); + markPending.mockClear(); + + expect(filters.validateTypeDefinition({ + currentType: 'leaf', newType: 'leaf', parent: 'child', + })).toEqual({ + field: 'parent', reason: 'cycle child -> leaf -> child', + }); + expect(filters.validateTypeDefinition({ + currentType: 'leaf', newType: 'fin', parent: 'root', + })).toEqual({ + field: 'name', reason: 'track 0 already contains both "leaf" and "fin"', + }); + expect(filters.validateTypeDefinition({ + currentType: 'leaf', newType: 'renamed', parent: 'missing', + })).toEqual({ + field: 'parent', reason: 'parent "missing" is not an existing type', + }); + expect(filters.validateTypeDefinition({ + currentType: 'leaf', newType: 'renamed', parent: 'leaf', + })).toEqual({ + field: 'parent', + reason: 'the original type "leaf" cannot be its renamed type\'s parent', + }); + expect(filters.validateTypeDefinition({ + currentType: 'leaf', newType: 'root', parent: 'root', + })).toEqual({ + field: 'name', reason: 'self edge "root -> root"', + }); + expect(filters.validateTypeDefinition({ + currentType: 'leaf', newType: 'leaf', parent: 'root', + })).toBeUndefined(); + + expect(cameraStore.getTrack(0).confidencePairs).toEqual([['leaf', 0.8], ['fin', 0.7]]); + expect(filters.typeHierarchy.value).toEqual({ child: 'leaf', leaf: 'root' }); + 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 +1140,55 @@ 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 a hierarchy-only child when deleting its top-level parent', () => { + const { filters } = makePairFixture([[['used', 1]]]); + filters.setTypeHierarchy({ leaf: 'root' }); + + expect(filters.deleteType('root')).toBe(true); + expect(filters.typeHierarchy.value).toBeUndefined(); + expect(filters.configuredTypes.value).toContain('leaf'); + expect(filters.allTypes.value).toContain('leaf'); + expect(filters.allTypes.value).not.toContain('root'); + }); + + 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 +1215,14 @@ 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.configuredTypes.value).toContain('root'); + expect(filters.allTypes.value).toContain('root'); + expect(filters.typeHierarchySavePatch()).toEqual({ typeHierarchy: null }); }); }); diff --git a/client/src/TrackFilterControls.ts b/client/src/TrackFilterControls.ts index 2efb8008c..ff57f537c 100644 --- a/client/src/TrackFilterControls.ts +++ b/client/src/TrackFilterControls.ts @@ -4,13 +4,15 @@ import { clientSettings } from 'dive-common/store/settings'; import { compileHierarchy, normalizeTypeHierarchy, + removeHierarchyType, resolveConfidenceThreshold, - selectFlatPairIndex, rewriteHierarchyType, + selectFlatPairIndex, selectPairIndex, TypeHierarchy, TypeHierarchyError, TypeHierarchyIndex, + updateHierarchyTypeDefinition, } from 'dive-common/typeHierarchy'; import { AnnotationId } from './BaseAnnotation'; import BaseFilterControls, { AnnotationWithContext, FilterControlsParams } from './BaseFilterControls'; @@ -22,6 +24,29 @@ export interface TypeHierarchySavePatch { typeHierarchy?: Record | null; } +export interface TypeDefinitionParams { + currentType: string; + newType: string; + parent: string | undefined; +} + +export interface TypeDefinitionValidationError { + field: 'name' | 'parent'; + reason: string; +} + +interface PreparedTypeDefinition { + nextHierarchy: TypeHierarchy | undefined; + nameChanged: boolean; + hierarchyChanged: boolean; + hierarchyInvolved: boolean; + survivingHierarchyTypes: Set; +} + +type TypeDefinitionPreflight = + | { prepared: PreparedTypeDefinition; error?: undefined } + | { prepared?: undefined; error: TypeDefinitionValidationError & { cause: TypeHierarchyError } }; + interface TrackFilterControlsParams extends FilterControlsParams { lookupGroups: (annotationId: AnnotationId) => Group[]; groupFilterControls: BaseFilterControls; @@ -33,6 +58,21 @@ interface TrackFilterControlsParams extends FilterControlsParams { ) => [string, number][]; } +function hierarchyIncludesType(hierarchy: TypeHierarchy | undefined, type: string): boolean { + return hierarchy !== undefined + && (Object.prototype.hasOwnProperty.call(hierarchy, type) + || Object.values(hierarchy).includes(type)); +} + +function hierarchyTypes(hierarchy: TypeHierarchy | undefined): Set { + const types = new Set(); + Object.entries(hierarchy || {}).forEach(([child, parent]) => { + types.add(child); + types.add(parent); + }); + return types; +} + export default class TrackFilterControls extends BaseFilterControls { filteredAnnotations: Ref[]>; @@ -72,14 +112,7 @@ export default class TrackFilterControls extends BaseFilterControls { this.typeHierarchy = ref(undefined); this.hierarchyIndex = ref(undefined); this.invalidHierarchyReason = ref(null); - this.hierarchyMembers = computed(() => { - const members = new Set(); - Object.entries(this.typeHierarchy.value || {}).forEach(([child, parent]) => { - members.add(child); - members.add(parent); - }); - return Array.from(members); - }); + this.hierarchyMembers = computed(() => Array.from(hierarchyTypes(this.typeHierarchy.value))); this.hierarchyActive = computed(() => this.hierarchyIndex.value !== undefined); this.allTypes = computed(() => Array.from(new Set([ ...flatAllTypes.value, @@ -248,6 +281,20 @@ export default class TrackFilterControls extends BaseFilterControls { .some((track) => track.confidencePairs.some(([name]) => name === type))); } + private configureStandaloneTypes( + types: ReadonlySet, + hierarchy: TypeHierarchy | undefined, + ) { + const hierarchyMembers = hierarchyTypes(hierarchy); + const retainedTypes = new Set(this.usedPlusConfiguredTypes.value); + types.forEach((type) => { + if (!hierarchyMembers.has(type) && !retainedTypes.has(type)) { + this.configuredTypes.value.push(type); + retainedTypes.add(type); + } + }); + } + updateTypeName({ currentType, newType }: { currentType: string; newType: string }) { if (!this.hierarchyActive.value) { this.sorted.value.forEach((annotation) => { @@ -260,60 +307,159 @@ 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) { - throw new TypeHierarchyError( - `track ${collision.id} already contains both "${currentType}" and "${newType}"`, - 'conflict', - ); + } + + private preflightTypeDefinition({ + currentType, + newType, + parent, + }: TypeDefinitionParams): TypeDefinitionPreflight { + let prepared: PreparedTypeDefinition; + let structuralErrorField: TypeDefinitionValidationError['field'] = 'parent'; + try { + 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( + `parent "${parent}" is not an existing type`, + 'conflict', + ); + } + + const currentHierarchy = this.typeHierarchy.value; + const currentParent = currentHierarchy?.[currentType]; + const nameChanged = currentType !== newType; + const parentChanged = parent !== currentParent; + structuralErrorField = nameChanged && !parentChanged ? 'name' : 'parent'; + const survivingHierarchyTypes = hierarchyTypes(currentHierarchy); + if (nameChanged && survivingHierarchyTypes.delete(currentType)) { + survivingHierarchyTypes.add(newType); + } + const nextHierarchy = nameChanged && !parentChanged && currentHierarchy !== undefined + ? rewriteHierarchyType(currentHierarchy, currentType, newType) + : updateHierarchyTypeDefinition(currentHierarchy, currentType, newType, parent); + prepared = { + nextHierarchy, + nameChanged, + hierarchyChanged: !isEqual(currentHierarchy, nextHierarchy), + hierarchyInvolved: currentHierarchy !== undefined || nextHierarchy !== undefined, + survivingHierarchyTypes, + }; + } catch (error) { + if (error instanceof TypeHierarchyError) { + return { + error: { field: structuralErrorField, reason: error.reason, cause: error }, + }; + } + throw error; + } + + if (prepared.nameChanged && prepared.hierarchyInvolved) { + 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) { + const cause = new TypeHierarchyError( + `track ${collision.id} already contains both "${currentType}" and "${newType}"`, + 'conflict', + ); + return { + error: { field: 'name', reason: cause.reason, cause }, + }; + } + } + return { prepared }; + } + + validateTypeDefinition(params: TypeDefinitionParams): TypeDefinitionValidationError | undefined { + const { error } = this.preflightTypeDefinition(params); + return error && { field: error.field, reason: error.reason }; + } + + updateTypeDefinition(params: TypeDefinitionParams) { + const preflight = this.preflightTypeDefinition(params); + if (preflight.error) { + throw preflight.error.cause; + } + const { + nextHierarchy, + nameChanged, + hierarchyChanged, + hierarchyInvolved, + survivingHierarchyTypes, + } = preflight.prepared; + const { currentType, newType } = params; + if (!nameChanged && !hierarchyChanged) { + return; + } + if (!hierarchyInvolved) { + this.updateTypeName({ currentType, newType }); + return; } - const currentHierarchy = this.typeHierarchy.value as TypeHierarchy; - const rewritten = rewriteHierarchyType(currentHierarchy, currentType, newType); - const hierarchyChanged = !isEqual(currentHierarchy, rewritten); 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); + this.configureStandaloneTypes(survivingHierarchyTypes, nextHierarchy); 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; + if (currentHierarchy === undefined || !hierarchyIncludesType(currentHierarchy, type)) { return super.deleteType(type); } if (this.typeInUseOnAnyCamera(type)) { return false; } + const nextHierarchy = removeHierarchyType(currentHierarchy, type); + const survivingHierarchyTypes = hierarchyTypes(currentHierarchy); + survivingHierarchyTypes.delete(type); + this.configureStandaloneTypes(survivingHierarchyTypes, nextHierarchy); + this.installTypeHierarchy(nextHierarchy, true); + this.checkedTypes.value = this.checkedTypes.value.filter((name) => name !== type); this.deleteTypeConfiguration(type); this.markChangesPending({ action: 'meta' }); return true; 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.validateTypeDefinition = vi.fn(() => undefined); + 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 +103,302 @@ 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-filter')).toBe(''); + expect(autocomplete.attributes('no-data-text')).toBe( + 'No matching type. Add it from Type Settings first.', + ); + 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); + + 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('shows every known preflight restriction inline and disables Save', async () => { + const parent = mountEditor(); + vi.mocked(parent.filters.validateTypeDefinition).mockReturnValue({ + field: 'parent', + 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( + 'Type hierarchy is invalid: cycle leaf -> branch -> leaf.', + ); + expect(parent.vm.saveDisabled).toBe(true); + parent.vm.acceptChanges(); + expect(parent.filters.updateTypeDefinition).not.toHaveBeenCalled(); + + const name = mountEditor(); + vi.mocked(name.filters.validateTypeDefinition).mockReturnValue({ + field: 'name', + reason: 'track 4 already contains both "leaf" and "root"', + }); + name.vm.data.editingType = 'root'; + await nextTick(); + expect(name.vm.nameDefinitionError).toBe( + 'Type hierarchy is invalid: track 4 already contains both "leaf" and "root".', + ); + expect(name.wrapper.find('v-text-field').attributes('error-messages')).toBe( + '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(); + 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..7e3a58726 100644 --- a/client/src/components/TypeEditor.vue +++ b/client/src/components/TypeEditor.vue @@ -1,10 +1,10 @@