From 951ad2f1a5bc111dabb18e94234debbc65921eac Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Sun, 9 Aug 2026 23:08:10 -0400 Subject: [PATCH 1/6] Add shared type hierarchy primitives --- client/dive-common/typeHierarchy.spec.ts | 151 ++++++++ client/dive-common/typeHierarchy.ts | 271 +++++++++++++++ server/dive_utils/type_hierarchy.py | 119 +++++++ server/tests/test_type_hierarchy.py | 62 ++++ testutils/typeHierarchy.spec.json | 420 +++++++++++++++++++++++ 5 files changed, 1023 insertions(+) create mode 100644 client/dive-common/typeHierarchy.spec.ts create mode 100644 client/dive-common/typeHierarchy.ts create mode 100644 server/dive_utils/type_hierarchy.py create mode 100644 server/tests/test_type_hierarchy.py create mode 100644 testutils/typeHierarchy.spec.json diff --git a/client/dive-common/typeHierarchy.spec.ts b/client/dive-common/typeHierarchy.spec.ts new file mode 100644 index 000000000..1a9778293 --- /dev/null +++ b/client/dive-common/typeHierarchy.spec.ts @@ -0,0 +1,151 @@ +import fs from 'fs-extra'; +import { + compileHierarchy, + normalizeTypeHierarchy, + resolveTypeHierarchy, + rewriteHierarchyType, + selectPairIndex, + TypeHierarchyError, +} from './typeHierarchy'; + +interface ErrorExpectation { + errorReason: string | null; + errorKind: 'malformed' | 'conflict' | null; +} + +interface NormalizationCase extends ErrorExpectation { + name: string; + input: unknown; + expected: Record | null; +} + +interface ResolutionCase extends ErrorExpectation { + name: string; + existing: unknown; + incomingPresent: boolean; + incoming?: unknown; + mode: 'save' | 'overwrite' | 'additive'; + expectedAction: 'none' | 'delete' | 'set' | null; + expected?: Record; +} + +interface RenameCase extends ErrorExpectation { + name: string; + hierarchy: Record; + currentType: string; + newType: string; + expected: Record | null; +} + +interface SelectionCase { + name: string; + hierarchy: Record; + pairs: [string, number][]; + passes: boolean[]; + expectedIndex: number; +} + +interface TypeHierarchyCorpus { + normalizationCases: NormalizationCase[]; + resolutionCases: ResolutionCase[]; + renameCases: RenameCase[]; + selectionCases: SelectionCase[]; +} + +const corpus = fs.readJSONSync('../testutils/typeHierarchy.spec.json') as TypeHierarchyCorpus; + +function expectHierarchyError( + callback: () => unknown, + expectedReason: string, + expectedKind: 'malformed' | 'conflict', +) { + try { + callback(); + throw new Error('Expected TypeHierarchyError'); + } catch (error) { + expect(error).toBeInstanceOf(TypeHierarchyError); + expect((error as TypeHierarchyError).reason).toBe(expectedReason); + expect((error as TypeHierarchyError).kind).toBe(expectedKind); + expect((error as TypeHierarchyError).message).toBe(expectedReason); + } +} + +describe('shared type hierarchy corpus', () => { + describe.each(corpus.normalizationCases)('normalization: $name', (testCase) => { + it('matches the shared result', () => { + if (testCase.errorReason !== null && testCase.errorKind !== null) { + expectHierarchyError( + () => normalizeTypeHierarchy(testCase.input), + testCase.errorReason, + testCase.errorKind, + ); + } else { + expect(normalizeTypeHierarchy(testCase.input)).toEqual(testCase.expected || undefined); + } + }); + }); + + describe.each(corpus.resolutionCases)('resolution: $name', (testCase) => { + it('matches the shared result', () => { + const resolve = () => resolveTypeHierarchy( + testCase.existing, + testCase.incomingPresent, + testCase.incoming, + testCase.mode, + ); + if (testCase.errorReason !== null && testCase.errorKind !== null) { + expectHierarchyError(resolve, testCase.errorReason, testCase.errorKind); + } else { + const write = resolve(); + expect(write.action).toBe(testCase.expectedAction); + if (write.action === 'set') { + expect(write.hierarchy).toEqual(testCase.expected); + } else { + expect('hierarchy' in write).toBe(false); + } + } + }); + }); + + describe.each(corpus.renameCases)('rename: $name', (testCase) => { + it('matches the shared result', () => { + const rewrite = () => rewriteHierarchyType( + testCase.hierarchy, + testCase.currentType, + testCase.newType, + ); + if (testCase.errorReason !== null && testCase.errorKind !== null) { + expectHierarchyError(rewrite, testCase.errorReason, testCase.errorKind); + } else { + expect(rewrite()).toEqual(testCase.expected || undefined); + } + }); + }); + + describe.each(corpus.selectionCases)('selection: $name', (testCase) => { + it('matches the shared result', () => { + const hierarchy = normalizeTypeHierarchy(testCase.hierarchy) || {}; + expect(selectPairIndex( + compileHierarchy(hierarchy), + testCase.pairs, + testCase.passes, + )).toBe(testCase.expectedIndex); + }); + }); +}); + +describe('type hierarchy index', () => { + const hierarchy = normalizeTypeHierarchy({ cod: 'fish', fish: 'animal' }) || {}; + const index = compileHierarchy(hierarchy); + + it('normalizes into a fresh map', () => { + const input = { cod: 'fish' }; + expect(normalizeTypeHierarchy(input)).not.toBe(input); + }); + + it('rejects pair/pass length mismatches', () => { + expect(() => selectPairIndex(index, [['cod', 0.9]], [])).toThrow( + 'passes and pairs must have the same length', + ); + }); +}); diff --git a/client/dive-common/typeHierarchy.ts b/client/dive-common/typeHierarchy.ts new file mode 100644 index 000000000..854381e71 --- /dev/null +++ b/client/dive-common/typeHierarchy.ts @@ -0,0 +1,271 @@ +export type TypeHierarchy = Readonly>; + +export type HierarchyWrite = + | { action: 'none' } + | { action: 'delete' } + | { action: 'set'; hierarchy: TypeHierarchy }; + +export class TypeHierarchyError extends Error { + readonly reason: string; + + readonly kind: 'malformed' | 'conflict'; + + constructor(reason: string, kind: 'malformed' | 'conflict' = 'malformed') { + super(reason); + this.name = 'TypeHierarchyError'; + this.reason = reason; + this.kind = kind; + } +} + +export interface TypeHierarchyIndex { + hierarchy: TypeHierarchy; + ancestors: Readonly>; +} + +// 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 { + 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); + for (let index = 0; index < sharedLength; index += 1) { + if (leftPoints[index] !== rightPoints[index]) { + return leftPoints[index] - rightPoints[index]; + } + } + return leftPoints.length - rightPoints.length; +} + +function sortedNames(names: readonly string[]): string[] { + return [...names].sort(codePointCompare); +} + +function hasOwn(hierarchy: TypeHierarchy, type: string): boolean { + return Object.prototype.hasOwnProperty.call(hierarchy, type); +} + +function cycleReason(hierarchy: TypeHierarchy): string | undefined { + const completed = new Set(); + const renderedCycles: string[] = []; + + sortedNames(Object.keys(hierarchy)).forEach((start) => { + if (completed.has(start)) { + return; + } + const path: string[] = []; + const positions = new Map(); + let current: string | undefined = start; + while (current !== undefined && hasOwn(hierarchy, current) + && !completed.has(current) && !positions.has(current)) { + positions.set(current, path.length); + path.push(current); + current = hierarchy[current]; + } + if (current !== undefined && positions.has(current)) { + const cycle = path.slice(positions.get(current) as number); + let smallestIndex = 0; + cycle.forEach((name, index) => { + if (codePointCompare(name, cycle[smallestIndex]) < 0) { + smallestIndex = index; + } + }); + const rotated = cycle.slice(smallestIndex).concat(cycle.slice(0, smallestIndex)); + renderedCycles.push([...rotated, rotated[0]].join(' -> ')); + } + path.forEach((name) => completed.add(name)); + }); + + if (renderedCycles.length === 0) { + return undefined; + } + renderedCycles.sort(codePointCompare); + return `cycle ${renderedCycles[0]}`; +} + +// Mirrors server/dive_utils/type_hierarchy.py so client saves and headless imports agree. +export function normalizeTypeHierarchy(value: unknown): TypeHierarchy | undefined { + if (value === null) { + return undefined; + } + if (typeof value !== 'object' || Array.isArray(value)) { + throw new TypeHierarchyError('expected an object'); + } + + const source = value as Record; + const keys = sortedNames(Object.keys(source)); + if (keys.length === 0) { + return undefined; + } + const entries: [string, string][] = []; + keys.forEach((child) => { + if (child.trim().length === 0) { + throw new TypeHierarchyError('empty child'); + } + const parent = source[child]; + if (typeof parent !== 'string') { + throw new TypeHierarchyError(`parent for "${child}" must be a string`); + } + if (parent.trim().length === 0) { + throw new TypeHierarchyError(`empty parent for "${child}"`); + } + if (child === parent) { + throw new TypeHierarchyError(`self edge "${child} -> ${parent}"`); + } + entries.push([child, parent]); + }); + + const normalized = Object.fromEntries(entries); + const reason = cycleReason(normalized); + if (reason !== undefined) { + throw new TypeHierarchyError(reason); + } + return normalized; +} + +function conflict(reason: string): TypeHierarchyError { + return new TypeHierarchyError(reason, 'conflict'); +} + +export function resolveTypeHierarchy( + existing: unknown, + incomingPresent: boolean, + incoming: unknown, + mode: 'save' | 'overwrite' | 'additive', +): HierarchyWrite { + if (!incomingPresent) { + return { action: 'none' }; + } + + const normalizedIncoming = normalizeTypeHierarchy(incoming); + if (normalizedIncoming === undefined) { + return mode === 'additive' ? { action: 'none' } : { action: 'delete' }; + } + if (mode !== 'additive') { + return { action: 'set', hierarchy: normalizedIncoming }; + } + + let normalizedExisting: TypeHierarchy | undefined; + try { + normalizedExisting = normalizeTypeHierarchy(existing); + } catch (error) { + if (error instanceof TypeHierarchyError) { + throw conflict(error.reason); + } + throw error; + } + + const merged = new Map(normalizedExisting + ? Object.entries(normalizedExisting) + : []); + sortedNames(Object.keys(normalizedIncoming)).forEach((child) => { + const incomingParent = normalizedIncoming[child]; + if (merged.has(child) && merged.get(child) !== incomingParent) { + throw conflict( + `conflicting parents for "${child}": "${merged.get(child)}" and "${incomingParent}"`, + ); + } + merged.set(child, incomingParent); + }); + + try { + return { + action: 'set', + hierarchy: normalizeTypeHierarchy(Object.fromEntries(merged)) as TypeHierarchy, + }; + } catch (error) { + if (error instanceof TypeHierarchyError) { + throw conflict(error.reason); + } + throw error; + } +} + +export function compileHierarchy(hierarchy: TypeHierarchy): TypeHierarchyIndex { + const normalized = normalizeTypeHierarchy(hierarchy) || {}; + const members = new Set(); + Object.entries(normalized).forEach(([child, parent]) => { + members.add(child); + members.add(parent); + }); + + const ancestorEntries: [string, readonly string[]][] = []; + sortedNames([...members]).forEach((member) => { + const memberAncestors: string[] = []; + let current = member; + while (hasOwn(normalized, current)) { + const parent = normalized[current]; + memberAncestors.push(parent); + current = parent; + } + ancestorEntries.push([member, memberAncestors]); + }); + + return { hierarchy: normalized, ancestors: Object.fromEntries(ancestorEntries) }; +} + +function ancestorsOf(index: TypeHierarchyIndex, type: string): readonly string[] { + return Object.prototype.hasOwnProperty.call(index.ancestors, type) + ? index.ancestors[type] + : []; +} + +export function rewriteHierarchyType( + hierarchy: TypeHierarchy, + currentType: string, + newType: string, +): TypeHierarchy | undefined { + const normalized = normalizeTypeHierarchy(hierarchy); + if (normalized === undefined) { + return undefined; + } + + const rewritten = new Map(); + const addEdge = (child: string, parent: string) => { + if (rewritten.has(child) && rewritten.get(child) !== parent) { + throw conflict( + `conflicting parents for "${child}": "${rewritten.get(child)}" and "${parent}"`, + ); + } + rewritten.set(child, parent); + }; + + sortedNames(Object.keys(normalized)) + .filter((child) => child !== currentType) + .forEach((child) => { + const parent = normalized[child] === currentType ? newType : normalized[child]; + addEdge(child, parent); + }); + if (hasOwn(normalized, currentType)) { + addEdge(newType, normalized[currentType]); + } + + try { + return normalizeTypeHierarchy(Object.fromEntries(rewritten)); + } catch (error) { + if (error instanceof TypeHierarchyError) { + throw conflict(error.reason); + } + throw error; + } +} + +export function selectPairIndex( + index: TypeHierarchyIndex, + pairs: readonly (readonly [string, number])[], + passes: readonly boolean[], +): number { + if (passes.length !== pairs.length) { + throw new Error('passes and pairs must have the same length'); + } + + const passingAncestorTypes = new Set(); + pairs.forEach(([type], pairIndex) => { + if (passes[pairIndex]) { + ancestorsOf(index, type).forEach((ancestor) => passingAncestorTypes.add(ancestor)); + } + }); + return pairs.findIndex(([type], pairIndex) => ( + passes[pairIndex] && !passingAncestorTypes.has(type) + )); +} diff --git a/server/dive_utils/type_hierarchy.py b/server/dive_utils/type_hierarchy.py new file mode 100644 index 000000000..8bd8175ec --- /dev/null +++ b/server/dive_utils/type_hierarchy.py @@ -0,0 +1,119 @@ +from typing import Dict, List, Literal, Optional + +from typing_extensions import NotRequired, TypedDict + + +class TypeHierarchyError(ValueError): + reason: str + kind: Literal['malformed', 'conflict'] + + def __init__( + self, + reason: str, + kind: Literal['malformed', 'conflict'] = 'malformed', + ) -> None: + super().__init__(reason) + self.reason = reason + self.kind = kind + + +class HierarchyWrite(TypedDict): + action: Literal['none', 'delete', 'set'] + hierarchy: NotRequired[Dict[str, str]] + + +def _cycle_reason(hierarchy: Dict[str, str]) -> Optional[str]: + completed = set() + rendered_cycles: List[str] = [] + for start in sorted(hierarchy): + if start in completed: + continue + path: List[str] = [] + positions: Dict[str, int] = {} + current = start + while current in hierarchy and current not in completed and current not in positions: + positions[current] = len(path) + path.append(current) + current = hierarchy[current] + if current in positions: + cycle = path[positions[current] :] + smallest = cycle.index(min(cycle)) + rotated = cycle[smallest:] + cycle[:smallest] + rendered_cycles.append(' -> '.join(rotated + [rotated[0]])) + completed.update(path) + if not rendered_cycles: + return None + rendered_cycles.sort() + return f'cycle {rendered_cycles[0]}' + + +# Mirrors client/dive-common/typeHierarchy.ts so headless imports and client saves agree. +def normalize_type_hierarchy(value: object) -> Optional[Dict[str, str]]: + if value is None: + return None + if not isinstance(value, dict): + raise TypeHierarchyError('expected an object') + if not value: + return None + if any(not isinstance(child, str) for child in value): + raise TypeHierarchyError('expected an object') + + normalized: Dict[str, str] = {} + for child in sorted(value): + if not child.strip(): + raise TypeHierarchyError('empty child') + parent = value[child] + if not isinstance(parent, str): + raise TypeHierarchyError(f'parent for "{child}" must be a string') + if not parent.strip(): + raise TypeHierarchyError(f'empty parent for "{child}"') + if child == parent: + raise TypeHierarchyError(f'self edge "{child} -> {parent}"') + normalized[child] = parent + + reason = _cycle_reason(normalized) + if reason is not None: + raise TypeHierarchyError(reason) + return normalized + + +def _conflict(reason: str) -> TypeHierarchyError: + return TypeHierarchyError(reason, 'conflict') + + +def resolve_type_hierarchy( + existing: object, + incoming_present: bool, + incoming: object, + mode: Literal['save', 'overwrite', 'additive'], +) -> HierarchyWrite: + if not incoming_present: + return {'action': 'none'} + + normalized_incoming = normalize_type_hierarchy(incoming) + if normalized_incoming is None: + if mode == 'additive': + return {'action': 'none'} + return {'action': 'delete'} + if mode != 'additive': + return {'action': 'set', 'hierarchy': normalized_incoming} + + try: + normalized_existing = normalize_type_hierarchy(existing) + except TypeHierarchyError as error: + raise _conflict(error.reason) from error + + merged = dict(normalized_existing or {}) + for child in sorted(normalized_incoming): + incoming_parent = normalized_incoming[child] + if child in merged and merged[child] != incoming_parent: + raise _conflict( + f'conflicting parents for "{child}": ' f'"{merged[child]}" and "{incoming_parent}"' + ) + merged[child] = incoming_parent + + try: + normalized_merged = normalize_type_hierarchy(merged) + except TypeHierarchyError as error: + raise _conflict(error.reason) from error + return {'action': 'set', 'hierarchy': normalized_merged or {}} diff --git a/server/tests/test_type_hierarchy.py b/server/tests/test_type_hierarchy.py new file mode 100644 index 000000000..2ae1ce62b --- /dev/null +++ b/server/tests/test_type_hierarchy.py @@ -0,0 +1,62 @@ +import json +from pathlib import Path +from typing import Any, Dict + +import pytest + +from dive_utils.type_hierarchy import ( + TypeHierarchyError, + normalize_type_hierarchy, + resolve_type_hierarchy, +) + +with (Path(__file__).parents[2] / 'testutils' / 'typeHierarchy.spec.json').open( + encoding='utf-8' +) as fp: + CORPUS = json.load(fp) + + +def _case_id(test_case: Dict[str, Any]) -> str: + return test_case['name'] + + +def _assert_hierarchy_error(error: TypeHierarchyError, test_case: Dict[str, Any]) -> None: + assert error.reason == test_case['errorReason'] + assert error.kind == test_case['errorKind'] + assert str(error) == test_case['errorReason'] + + +@pytest.mark.parametrize('test_case', CORPUS['normalizationCases'], ids=_case_id) +def test_normalization_case(test_case): + if test_case['errorReason'] is not None: + with pytest.raises(TypeHierarchyError) as error_info: + normalize_type_hierarchy(test_case['input']) + _assert_hierarchy_error(error_info.value, test_case) + else: + assert normalize_type_hierarchy(test_case['input']) == test_case['expected'] + + +@pytest.mark.parametrize('test_case', CORPUS['resolutionCases'], ids=_case_id) +def test_resolution_case(test_case): + incoming = test_case.get('incoming') + if test_case['errorReason'] is not None: + with pytest.raises(TypeHierarchyError) as error_info: + resolve_type_hierarchy( + test_case['existing'], + test_case['incomingPresent'], + incoming, + test_case['mode'], + ) + _assert_hierarchy_error(error_info.value, test_case) + else: + write = resolve_type_hierarchy( + test_case['existing'], + test_case['incomingPresent'], + incoming, + test_case['mode'], + ) + assert write['action'] == test_case['expectedAction'] + if write['action'] == 'set': + assert write['hierarchy'] == test_case['expected'] + else: + assert 'hierarchy' not in write diff --git a/testutils/typeHierarchy.spec.json b/testutils/typeHierarchy.spec.json new file mode 100644 index 000000000..06196dea2 --- /dev/null +++ b/testutils/typeHierarchy.spec.json @@ -0,0 +1,420 @@ +{ + "normalizationCases": [ + { + "name": "null is canonical absence", + "input": null, + "expected": null, + "errorReason": null, + "errorKind": null + }, + { + "name": "empty object is canonical absence", + "input": {}, + "expected": null, + "errorReason": null, + "errorKind": null + }, + { + "name": "single edge with heading-only parent", + "input": { "cod": "fish" }, + "expected": { "cod": "fish" }, + "errorReason": null, + "errorKind": null + }, + { + "name": "chain", + "input": { "juvenile cod": "cod", "cod": "fish" }, + "expected": { "cod": "fish", "juvenile cod": "cod" }, + "errorReason": null, + "errorKind": null + }, + { + "name": "forest", + "input": { "cod": "fish", "tern": "bird", "fish": "animal" }, + "expected": { "cod": "fish", "fish": "animal", "tern": "bird" }, + "errorReason": null, + "errorKind": null + }, + { + "name": "names are preserved byte-for-byte", + "input": { " cod ": " fish " }, + "expected": { " cod ": " fish " }, + "errorReason": null, + "errorKind": null + }, + { + "name": "whitespace-only child", + "input": { " ": "fish" }, + "expected": null, + "errorReason": "empty child", + "errorKind": "malformed" + }, + { + "name": "whitespace-only parent", + "input": { "cod": " " }, + "expected": null, + "errorReason": "empty parent for \"cod\"", + "errorKind": "malformed" + }, + { + "name": "non-string parent", + "input": { "cod": 7 }, + "expected": null, + "errorReason": "parent for \"cod\" must be a string", + "errorKind": "malformed" + }, + { + "name": "array is not an object map", + "input": ["cod", "fish"], + "expected": null, + "errorReason": "expected an object", + "errorKind": "malformed" + }, + { + "name": "scalar is not an object map", + "input": "cod", + "expected": null, + "errorReason": "expected an object", + "errorKind": "malformed" + }, + { + "name": "self edge", + "input": { "fish": "fish" }, + "expected": null, + "errorReason": "self edge \"fish -> fish\"", + "errorKind": "malformed" + }, + { + "name": "two-node cycle rotates to bytewise-smallest name", + "input": { "zebra": "ant", "ant": "zebra" }, + "expected": null, + "errorReason": "cycle ant -> zebra -> ant", + "errorKind": "malformed" + }, + { + "name": "cycle rotation uses UTF-8 byte order", + "input": { "𐀀 node": " node", " node": "𐀀 node" }, + "expected": null, + "errorReason": "cycle  node -> 𐀀 node ->  node", + "errorKind": "malformed" + }, + { + "name": "longer cycle", + "input": { "cod": "fish", "fish": "animal", "animal": "cod" }, + "expected": null, + "errorReason": "cycle animal -> cod -> fish -> animal", + "errorKind": "malformed" + }, + { + "name": "multiple cycles use bytewise-smallest rendered path", + "input": { + "𐀀 one": "𐀀 two", + "𐀀 two": "𐀀 one", + " one": " two", + " two": " one" + }, + "expected": null, + "errorReason": "cycle  one ->  two ->  one", + "errorKind": "malformed" + } + ], + "resolutionCases": [ + { + "name": "missing key is no instruction and does not validate existing", + "existing": { "a": "b", "b": "a" }, + "incomingPresent": false, + "mode": "save", + "expectedAction": "none", + "errorReason": null, + "errorKind": null + }, + { + "name": "direct null clears invalid existing storage", + "existing": { "a": "b", "b": "a" }, + "incomingPresent": true, + "incoming": null, + "mode": "save", + "expectedAction": "delete", + "errorReason": null, + "errorKind": null + }, + { + "name": "overwrite empty object clears", + "existing": { "cod": "fish" }, + "incomingPresent": true, + "incoming": {}, + "mode": "overwrite", + "expectedAction": "delete", + "errorReason": null, + "errorKind": null + }, + { + "name": "direct save sets complete incoming map", + "existing": { "cod": "fish" }, + "incomingPresent": true, + "incoming": { "tern": "bird" }, + "mode": "save", + "expectedAction": "set", + "expected": { "tern": "bird" }, + "errorReason": null, + "errorKind": null + }, + { + "name": "overwrite replaces complete map", + "existing": { "cod": "fish", "fish": "animal" }, + "incomingPresent": true, + "incoming": { "tern": "bird" }, + "mode": "overwrite", + "expectedAction": "set", + "expected": { "tern": "bird" }, + "errorReason": null, + "errorKind": null + }, + { + "name": "overwrite valid input repairs invalid existing storage", + "existing": { "a": "b", "b": "a" }, + "incomingPresent": true, + "incoming": { "cod": "fish" }, + "mode": "overwrite", + "expectedAction": "set", + "expected": { "cod": "fish" }, + "errorReason": null, + "errorKind": null + }, + { + "name": "additive explicit null is no instruction", + "existing": { "cod": "fish" }, + "incomingPresent": true, + "incoming": null, + "mode": "additive", + "expectedAction": "none", + "errorReason": null, + "errorKind": null + }, + { + "name": "additive empty object preserves invalid existing storage without validation", + "existing": { "a": "b", "b": "a" }, + "incomingPresent": true, + "incoming": {}, + "mode": "additive", + "expectedAction": "none", + "errorReason": null, + "errorKind": null + }, + { + "name": "additive identical edge coalesces", + "existing": { "cod": "fish" }, + "incomingPresent": true, + "incoming": { "cod": "fish" }, + "mode": "additive", + "expectedAction": "set", + "expected": { "cod": "fish" }, + "errorReason": null, + "errorKind": null + }, + { + "name": "additive non-conflicting maps merge", + "existing": { "cod": "fish" }, + "incomingPresent": true, + "incoming": { "fish": "animal", "tern": "bird" }, + "mode": "additive", + "expectedAction": "set", + "expected": { "cod": "fish", "fish": "animal", "tern": "bird" }, + "errorReason": null, + "errorKind": null + }, + { + "name": "additive conflicting parent rejects", + "existing": { "cod": "fish" }, + "incomingPresent": true, + "incoming": { "cod": "animal" }, + "mode": "additive", + "expectedAction": null, + "errorReason": "conflicting parents for \"cod\": \"fish\" and \"animal\"", + "errorKind": "conflict" + }, + { + "name": "additive combined cycle rejects", + "existing": { "cod": "fish" }, + "incomingPresent": true, + "incoming": { "fish": "cod" }, + "mode": "additive", + "expectedAction": null, + "errorReason": "cycle cod -> fish -> cod", + "errorKind": "conflict" + }, + { + "name": "valid additive input against invalid existing is conflict", + "existing": { "a": "b", "b": "a" }, + "incomingPresent": true, + "incoming": { "cod": "fish" }, + "mode": "additive", + "expectedAction": null, + "errorReason": "cycle a -> b -> a", + "errorKind": "conflict" + }, + { + "name": "invalid incoming remains malformed even against invalid existing", + "existing": { "a": "b", "b": "a" }, + "incomingPresent": true, + "incoming": { "cod": 3 }, + "mode": "additive", + "expectedAction": null, + "errorReason": "parent for \"cod\" must be a string", + "errorKind": "malformed" + } + ], + "renameCases": [ + { + "name": "rename child key", + "hierarchy": { "cod": "fish" }, + "currentType": "cod", + "newType": "haddock", + "expected": { "haddock": "fish" }, + "errorReason": null, + "errorKind": null + }, + { + "name": "rename parent value", + "hierarchy": { "cod": "fish", "tern": "bird" }, + "currentType": "fish", + "newType": "animal", + "expected": { "cod": "animal", "tern": "bird" }, + "errorReason": null, + "errorKind": null + }, + { + "name": "rename both key and value occurrences", + "hierarchy": { "cod": "fish", "fish": "animal" }, + "currentType": "fish", + "newType": "vertebrate", + "expected": { "cod": "vertebrate", "vertebrate": "animal" }, + "errorReason": null, + "errorKind": null + }, + { + "name": "rename coalesces identical destination edge", + "hierarchy": { "cod": "fish", "haddock": "fish" }, + "currentType": "cod", + "newType": "haddock", + "expected": { "haddock": "fish" }, + "errorReason": null, + "errorKind": null + }, + { + "name": "rename parent onto existing parent shares destination", + "hierarchy": { "cod": "fish", "tern": "bird" }, + "currentType": "fish", + "newType": "bird", + "expected": { "cod": "bird", "tern": "bird" }, + "errorReason": null, + "errorKind": null + }, + { + "name": "rename child to its parent creates self edge", + "hierarchy": { "cod": "fish" }, + "currentType": "cod", + "newType": "fish", + "expected": null, + "errorReason": "self edge \"fish -> fish\"", + "errorKind": "conflict" + }, + { + "name": "rename to child with different parent conflicts", + "hierarchy": { "cod": "fish", "haddock": "animal" }, + "currentType": "cod", + "newType": "haddock", + "expected": null, + "errorReason": "conflicting parents for \"haddock\": \"animal\" and \"fish\"", + "errorKind": "conflict" + }, + { + "name": "rename parent to descendant creates cycle", + "hierarchy": { "cod": "fish", "fish": "animal" }, + "currentType": "animal", + "newType": "cod", + "expected": null, + "errorReason": "cycle cod -> fish -> cod", + "errorKind": "conflict" + } + ], + "selectionCases": [ + { + "name": "monotone chain chooses leaf", + "hierarchy": { "juvenile cod": "cod", "cod": "fish" }, + "pairs": [["juvenile cod", 0.9], ["cod", 0.8], ["fish", 0.7]], + "passes": [true, true, true], + "expectedIndex": 0 + }, + { + "name": "non-monotone chain chooses passing leaf despite confidence order", + "hierarchy": { "juvenile cod": "cod", "cod": "fish" }, + "pairs": [["cod", 0.9], ["fish", 0.8], ["juvenile cod", 0.7]], + "passes": [true, true, true], + "expectedIndex": 2 + }, + { + "name": "unchecked leaf rolls up to passing parent", + "hierarchy": { "juvenile cod": "cod", "cod": "fish" }, + "pairs": [["juvenile cod", 0.9], ["cod", 0.8], ["fish", 0.7]], + "passes": [false, true, true], + "expectedIndex": 1 + }, + { + "name": "passing descendant wins when ancestor is unchecked", + "hierarchy": { "juvenile cod": "cod", "cod": "fish" }, + "pairs": [["cod", 0.9], ["juvenile cod", 0.8]], + "passes": [false, true], + "expectedIndex": 1 + }, + { + "name": "failing descendant leaves passing ancestor", + "hierarchy": { "juvenile cod": "cod", "cod": "fish" }, + "pairs": [["juvenile cod", 0.9], ["cod", 0.8]], + "passes": [false, true], + "expectedIndex": 1 + }, + { + "name": "no passing pair", + "hierarchy": { "cod": "fish" }, + "pairs": [["cod", 0.9], ["fish", 0.8]], + "passes": [false, false], + "expectedIndex": -1 + }, + { + "name": "duplicate type names preserve first passing occurrence", + "hierarchy": { "cod": "fish" }, + "pairs": [["fish", 0.95], ["cod", 0.8], ["cod", 0.8]], + "passes": [true, true, true], + "expectedIndex": 1 + }, + { + "name": "equal scores preserve original order between unrelated leaves", + "hierarchy": { "cod": "fish", "tern": "bird" }, + "pairs": [["tern", 0.8], ["cod", 0.8]], + "passes": [true, true], + "expectedIndex": 0 + }, + { + "name": "unrelated branches do not compare numeric depth", + "hierarchy": { "juvenile cod": "cod", "cod": "fish", "tern": "bird" }, + "pairs": [["tern", 0.9], ["juvenile cod", 0.8], ["cod", 0.7]], + "passes": [true, true, true], + "expectedIndex": 0 + }, + { + "name": "unknown hierarchy name remains unrelated", + "hierarchy": { "cod": "fish" }, + "pairs": [["mystery", 0.9], ["cod", 0.8], ["fish", 0.7]], + "passes": [true, true, true], + "expectedIndex": 0 + }, + { + "name": "empty hierarchy pure selector parity", + "hierarchy": {}, + "pairs": [["cod", 0.9], ["fish", 0.8]], + "passes": [false, true], + "expectedIndex": 1 + } + ] +} From 479dc1ed28d6bb078146ba14e42f1786ddfd9772 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Mon, 10 Aug 2026 00:36:36 -0400 Subject: [PATCH 2/6] Persist type hierarchy in dataset configuration --- client/dive-common/apispec.ts | 4 +- client/dive-common/components/Viewer.vue | 22 +- .../desktop/backend/native/common.spec.ts | 332 +++++++++++ .../platform/desktop/backend/native/common.ts | 404 ++++++++++--- .../desktop/backend/native/multicamExport.ts | 5 + .../platform/web-girder/views/Upload.spec.ts | 21 + client/src/TrackFilterControls.spec.ts | 153 ++++- client/src/TrackFilterControls.ts | 143 +++++ docs/DataFormats.md | 31 + server/dive_server/crud.py | 37 +- server/dive_server/crud_dataset.py | 141 ++++- server/dive_server/crud_rpc.py | 440 ++++++++++++-- server/dive_server/views_dataset.py | 67 ++- server/dive_tasks/convert_images.py | 68 ++- server/dive_tasks/utils.py | 120 +++- server/dive_utils/models.py | 8 +- server/dive_utils/type_hierarchy.py | 10 + server/tests/test_create_multicam.py | 8 + .../tests/test_hierarchy_ingestion_routes.py | 362 ++++++++++++ server/tests/test_multicam_export_clone.py | 165 +++++- server/tests/test_multicam_zip_import.py | 144 ++++- server/tests/test_update_metadata.py | 552 +++++++++++++++++- 22 files changed, 2975 insertions(+), 262 deletions(-) create mode 100644 server/tests/test_hierarchy_ingestion_routes.py diff --git a/client/dive-common/apispec.ts b/client/dive-common/apispec.ts index ded784c5d..d7f722df9 100644 --- a/client/dive-common/apispec.ts +++ b/client/dive-common/apispec.ts @@ -256,6 +256,7 @@ type DatasetInfoFields = Record; * The parts of dataset config a user should be able to modify. */ interface DatasetConfigMutable { + typeHierarchy?: Record | null; customTypeStyling?: Record; customGroupStyling?: Record; confidenceFilters?: Record; @@ -271,7 +272,7 @@ interface DatasetConfigMutable { cameraRegistrationSource?: RegistrationSource | null; error?: string; } -const DatasetConfigMutableKeys = ['attributes', 'confidenceFilters', 'timeFilters', 'imageEnhancements', 'customTypeStyling', 'customGroupStyling', 'attributeTrackFilters', 'datasetInfo', 'cameraHomographies', 'cameraCorrespondences', 'cameraTransformTypes', 'cameraRegistrationSource']; +const DatasetConfigMutableKeys = ['attributes', 'confidenceFilters', 'timeFilters', 'imageEnhancements', 'customTypeStyling', 'customGroupStyling', 'attributeTrackFilters', 'datasetInfo', 'cameraHomographies', 'cameraCorrespondences', 'cameraTransformTypes', 'cameraRegistrationSource', 'typeHierarchy']; /** * Cross-dataset color/style overrides, reused across every dataset when the * "shared" color scope is enabled (see clientSettings.typeSettings.colorScope). @@ -298,6 +299,7 @@ const MulticamSharedMutableKeys = [ 'customGroupStyling', 'attributeTrackFilters', 'datasetInfo', + 'typeHierarchy', ]; interface DatasetConfig extends DatasetConfigMutable { diff --git a/client/dive-common/components/Viewer.vue b/client/dive-common/components/Viewer.vue index 9e8d00564..3d86234de 100644 --- a/client/dive-common/components/Viewer.vue +++ b/client/dive-common/components/Viewer.vue @@ -386,7 +386,7 @@ export default defineComponent({ save: saveToServer, markChangesPending, discardChanges, - pendingSaveCount, + pendingSaveCount: rawPendingSaveCount, addCamera: addSaveCamera, removeCamera: removeSaveCamera, } = useSave(datasetId, readonlyState); @@ -670,6 +670,10 @@ export default defineComponent({ setType: setTrackType, removeTypes, }); + const pendingSaveCount = computed(() => Math.max( + 0, + rawPendingSaveCount.value - trackFilters.typeHierarchyPendingCountAdjustment(), + )); clientSettingsSetup(trackFilters.allTypes); @@ -935,13 +939,17 @@ export default defineComponent({ } try { await saveToServer({ - customTypeStyling: trackStyleManager.getTypeStyles(trackFilters.allTypes), + customTypeStyling: trackStyleManager.getTypeStyles( + trackFilters.usedPlusConfiguredTypes, + ), customGroupStyling: groupStyleManager.getTypeStyles(groupFilters.allTypes), confidenceFilters: trackFilters.confidenceFilters.value, timeFilters: trackFilters.timeFilters.value, imageEnhancements: imageEnhancements.value, + ...trackFilters.prepareTypeHierarchySavePatch(), // TODO Group confidence filters are not yet supported. }, saveSet); + trackFilters.markTypeHierarchyPersisted(); } catch (err) { let text = 'Unable to Save Data'; const saveErr = err as { response?: { status?: number } }; @@ -1494,7 +1502,17 @@ export default defineComponent({ scheduleGlobalStylePersist.flush(); // Close and reset sideBar context.resetActive(); + trackFilters.setTypeHierarchy(undefined); const meta = await loadConfig(datasetId.value); + trackFilters.setTypeHierarchy(meta.typeHierarchy); + const hierarchyWarning = trackFilters.consumeLoadWarning(); + if (hierarchyWarning) { + await prompt({ + title: 'Invalid Type Hierarchy', + text: hierarchyWarning, + positiveButton: 'OK', + }); + } baseMulticamDatasetId.value = datasetId.value; if (meta.multiCamMedia) { /* We're loading a multicamera dataset */ diff --git a/client/platform/desktop/backend/native/common.spec.ts b/client/platform/desktop/backend/native/common.spec.ts index a9bf0f35e..ab0c596ec 100644 --- a/client/platform/desktop/backend/native/common.spec.ts +++ b/client/platform/desktop/backend/native/common.spec.ts @@ -11,6 +11,7 @@ import { makeEmptyAnnotationFile } from 'platform/desktop/backend/serializers/di import { CameraCorrespondences, MultiTrackRecord } from 'dive-common/apispec'; import { Attribute } from 'vue-media-annotator/use/AttributeTypes'; +import { getResponseError } from 'vue-media-annotator/utils'; import * as common from './common'; import { createWorkingDirectory, buildTrainingExitManifest } from './utils'; import beginMultiCamImport from './multiCamImport'; @@ -949,6 +950,39 @@ describe('native.common', () => { )).rejects.toThrow('no bbox and no usable polygon segmentation'); }); + it.each([ + ['malformed', '{broken'], + ['null', 'null'], + ['number', '5'], + ['string', '"annotation"'], + ])('keeps an earlier annotation write when later %s JSON is invalid', async (kind, contents) => { + const valid = '/home/user/output/valid-before-malformed.coco.json'; + const malformed = `/home/user/output/${kind}-later.json`; + const project = common.getProjectDir(settings, 'projectid1'); + await fs.writeFile(valid, cocoWithRle(1)); + await fs.writeFile(malformed, contents); + const writeFile = vi.spyOn(fs, 'writeFile'); + + try { + await common.ingestDataFiles( + settings, + 'projectid1', + [valid, malformed], + ).catch(() => undefined); + + expect(await fs.pathExists( + npath.join(project.auxDirAbsPath, `imported_${npath.basename(valid)}`), + )).toBe(true); + const annotationWrites = writeFile.mock.calls.filter(([path, data]) => ( + npath.basename(String(path)).startsWith('result_') + && Object.prototype.hasOwnProperty.call(JSON.parse(String(data)).tracks, '1') + )); + expect(annotationWrites).toHaveLength(1); + } finally { + writeFile.mockRestore(); + } + }); + it('getPipelineList lists pipelines', async () => { const exists = await fs.pathExists(settings.viamePath); expect(exists).toBe(true); @@ -1026,6 +1060,275 @@ describe('native.common', () => { expect(data.imageData[1].timestamp).toBeUndefined(); }); + it('saveConfig sets, clears, and atomically rejects a type hierarchy', async () => { + const legacyProject = common.getProjectDir(settings, 'projectid1'); + await common.saveProjectConfig( + legacyProject.basePath, + await fs.readJSON(legacyProject.datasetFileAbsPath), + ); + await common.saveConfig(settings, 'projectid1', { + typeHierarchy: { shark: 'fish' }, + }); + let meta = await common.loadConfig(settings, 'projectid1', urlMapper); + expect(meta.typeHierarchy).toEqual({ shark: 'fish' }); + + const project = common.getProjectDir(settings, 'projectid1'); + const beforeInvalid = await fs.readFile(project.datasetFileAbsPath, 'utf8'); + await expect(common.saveConfig(settings, 'projectid1', { + typeHierarchy: { fish: 'fish' }, + confidenceFilters: { default: 0.9 }, + })).rejects.toThrow( + 'Type hierarchy is invalid: self edge "fish -> fish". No configuration was changed.', + ); + expect(await fs.readFile(project.datasetFileAbsPath, 'utf8')).toBe(beforeInvalid); + + await common.saveConfig(settings, 'projectid1', { typeHierarchy: null }); + meta = await common.loadConfig(settings, 'projectid1', urlMapper); + expect(meta.typeHierarchy).toBeUndefined(); + }); + + it('an unrelated direct save preserves invalid hierarchy storage until repaired', async () => { + let project = common.getProjectDir(settings, 'projectid1'); + const raw = await fs.readJSON(project.datasetFileAbsPath); + raw.typeHierarchy = ['corrupt']; + await common.saveProjectConfig(project.basePath, raw); + project = common.getProjectDir(settings, 'projectid1'); + + await common.saveConfig(settings, 'projectid1', { + confidenceFilters: { default: 0.7 }, + }); + let saved = await fs.readJSON(common.getProjectDir(settings, 'projectid1').datasetFileAbsPath); + expect(saved.typeHierarchy).toEqual(['corrupt']); + + await common.saveConfig(settings, 'projectid1', { typeHierarchy: {} }); + saved = await fs.readJSON(common.getProjectDir(settings, 'projectid1').datasetFileAbsPath); + expect(saved.typeHierarchy).toBeUndefined(); + await common.saveConfig(settings, 'projectid1', { + typeHierarchy: { tuna: 'fish' }, + }); + saved = await fs.readJSON(common.getProjectDir(settings, 'projectid1').datasetFileAbsPath); + expect(saved.typeHierarchy).toEqual({ tuna: 'fish' }); + }); + + it('imports overwrite, additive, and explicit-empty hierarchy instructions', async () => { + const overwrite = '/home/user/output/hierarchy-overwrite.json'; + const additive = '/home/user/output/hierarchy-additive.json'; + const empty = '/home/user/output/hierarchy-empty.json'; + await fs.writeJSON(overwrite, { typeHierarchy: { shark: 'fish' } }); + await fs.writeJSON(additive, { typeHierarchy: { tuna: 'fish' } }); + await fs.writeJSON(empty, { typeHierarchy: {} }); + + await common.dataFileImport(settings, 'projectid1', overwrite); + await common.dataFileImport(settings, 'projectid1', additive, true); + let meta = await common.loadConfig(settings, 'projectid1', urlMapper); + expect(meta.typeHierarchy).toEqual({ shark: 'fish', tuna: 'fish' }); + + await common.dataFileImport(settings, 'projectid1', empty, true); + meta = await common.loadConfig(settings, 'projectid1', urlMapper); + expect(meta.typeHierarchy).toEqual({ shark: 'fish', tuna: 'fish' }); + await common.dataFileImport(settings, 'projectid1', empty); + meta = await common.loadConfig(settings, 'projectid1', urlMapper); + expect(meta.typeHierarchy).toBeUndefined(); + }); + + it('rejects additive hierarchy conflicts without writes and allows corrected retry', async () => { + const imported = '/home/user/output/hierarchy-conflict.json'; + const legacyProject = common.getProjectDir(settings, 'projectid1'); + await common.saveProjectConfig( + legacyProject.basePath, + await fs.readJSON(legacyProject.datasetFileAbsPath), + ); + await common.saveConfig(settings, 'projectid1', { + typeHierarchy: { shark: 'fish' }, + confidenceFilters: { default: 0.2 }, + }); + await fs.writeJSON(imported, { + typeHierarchy: { shark: 'animal' }, + confidenceFilters: { default: 0.9 }, + }); + const project = await common.getValidatedProjectDir(settings, 'projectid1'); + const before = await fs.readFile(project.datasetFileAbsPath, 'utf8'); + + await expect(common.dataFileImport( + settings, + 'projectid1', + imported, + true, + )).rejects.toThrow( + 'Type hierarchy is invalid: conflicting parents for "shark": "fish" and "animal". ' + + 'No configuration was changed.', + ); + expect(await fs.readFile(project.datasetFileAbsPath, 'utf8')).toBe(before); + expect(await fs.pathExists(npath.join(project.auxDirAbsPath, 'imported_hierarchy-conflict.json'))) + .toBe(false); + + await fs.writeJSON(imported, { typeHierarchy: { tuna: 'fish' } }); + await common.dataFileImport(settings, 'projectid1', imported, true); + const saved = await common.loadConfig(settings, 'projectid1', urlMapper); + expect(saved.typeHierarchy).toEqual({ shark: 'fish', tuna: 'fish' }); + }); + + it('preflights ordered hierarchy config batches and cleans every auxiliary copy on failure', async () => { + const annotation = '/home/user/output/annotation-before-config.json'; + const first = '/home/user/output/hierarchy-first.json'; + const second = '/home/user/output/hierarchy-second.json'; + await fs.writeJSON(annotation, { 0: { trackId: 0 } }); + await fs.writeJSON(first, { + typeHierarchy: { shark: 'fish' }, + datasetInfo: { first: true, replaced: 'first' }, + }); + await fs.writeJSON(second, { typeHierarchy: { fish: 'shark' } }); + const project = await common.getValidatedProjectDir(settings, 'projectid1'); + const before = await fs.readFile(project.datasetFileAbsPath, 'utf8'); + const annotationsBefore = await fs.readFile(project.trackFileAbsPath, 'utf8'); + + await expect(common.ingestDataFiles( + settings, + 'projectid1', + [annotation, first, second], + undefined, + undefined, + true, + )).rejects.toThrow( + 'Type hierarchy is invalid: cycle fish -> shark -> fish. No configuration was changed.', + ); + expect(await fs.readFile(project.datasetFileAbsPath, 'utf8')).toBe(before); + expect(await fs.readFile(project.trackFileAbsPath, 'utf8')).toBe(annotationsBefore); + expect(await fs.pathExists(npath.join(project.auxDirAbsPath, 'imported_hierarchy-first.json'))) + .toBe(false); + expect(await fs.pathExists(npath.join(project.auxDirAbsPath, 'imported_hierarchy-second.json'))) + .toBe(false); + + await fs.writeJSON(second, { + typeHierarchy: { tuna: 'fish' }, + datasetInfo: { second: true, replaced: 'second' }, + }); + const result = await common.ingestDataFiles( + settings, + 'projectid1', + [first, second], + undefined, + undefined, + true, + ); + expect(result.meta.typeHierarchy).toEqual({ shark: 'fish', tuna: 'fish' }); + expect(result.meta.datasetInfo).toEqual({ + first: true, + second: true, + replaced: 'second', + }); + + const overwriteResult = await common.ingestDataFiles( + settings, + 'projectid1', + [first, second], + ); + expect(overwriteResult.meta.typeHierarchy).toEqual({ tuna: 'fish' }); + expect(overwriteResult.meta.datasetInfo).toEqual({ second: true, replaced: 'second' }); + }); + + it('parses each config once and executes the exact ordered hierarchy candidate', async () => { + const first = '/home/user/output/parse-once-first.json'; + const second = '/home/user/output/parse-once-second.json'; + const legacyProject = common.getProjectDir(settings, 'projectid1'); + await common.saveProjectConfig( + legacyProject.basePath, + await fs.readJSON(legacyProject.datasetFileAbsPath), + ); + await common.saveConfig(settings, 'projectid1', { + typeHierarchy: { shark: 'fish' }, + }); + await fs.writeJSON(first, { + typeHierarchy: { tuna: 'fish' }, + datasetInfo: { sequence: 'first' }, + }); + await fs.writeJSON(second, { + typeHierarchy: { mako: 'shark' }, + datasetInfo: { sequence: 'second' }, + }); + const readFile = vi.spyOn(fs, 'readFile'); + + try { + const result = await common.ingestDataFiles( + settings, + 'projectid1', + [first, second], + undefined, + undefined, + true, + ); + + expect(result.meta.typeHierarchy).toEqual({ + mako: 'shark', + shark: 'fish', + tuna: 'fish', + }); + expect(result.meta.datasetInfo).toEqual({ sequence: 'second' }); + expect(readFile.mock.calls.filter(([path]) => path === first)).toHaveLength(1); + expect(readFile.mock.calls.filter(([path]) => path === second)).toHaveLength(1); + } finally { + readFile.mockRestore(); + } + }); + + it('surfaces the exact hierarchy error through the desktop public import boundary', async () => { + const imported = '/home/user/output/public-import-conflict.json'; + const expected = 'Type hierarchy is invalid: conflicting parents for "shark": "fish" and ' + + '"animal". No configuration was changed.'; + const legacyProject = common.getProjectDir(settings, 'projectid1'); + await common.saveProjectConfig( + legacyProject.basePath, + await fs.readJSON(legacyProject.datasetFileAbsPath), + ); + await common.saveConfig(settings, 'projectid1', { + typeHierarchy: { shark: 'fish' }, + }); + await fs.writeJSON(imported, { typeHierarchy: { shark: 'animal' } }); + let surfacedError: unknown; + + try { + await common.dataFileImport(settings, 'projectid1', imported, true); + } catch (error) { + surfacedError = error; + } + + expect(surfacedError).toBeInstanceOf(Error); + expect((surfacedError as Error).message).toBe(expected); + expect(getResponseError(surfacedError)).toBe(expected); + }); + + it('exports only a valid non-empty type hierarchy and writes no invalid export', async () => { + const output = '/home/user/output/exported-config.json'; + const legacyProject = common.getProjectDir(settings, 'projectid1'); + await common.saveProjectConfig( + legacyProject.basePath, + await fs.readJSON(legacyProject.datasetFileAbsPath), + ); + await common.saveConfig(settings, 'projectid1', { + typeHierarchy: { shark: 'fish' }, + }); + await common.exportConfiguration(settings, { id: 'projectid1', path: output }); + expect((await fs.readJSON(output)).typeHierarchy).toEqual({ shark: 'fish' }); + + await common.saveConfig(settings, 'projectid1', { typeHierarchy: null }); + await common.exportConfiguration(settings, { id: 'projectid1', path: output }); + expect((await fs.readJSON(output)).typeHierarchy).toBeUndefined(); + + const project = common.getProjectDir(settings, 'projectid1'); + const raw = await fs.readJSON(project.datasetFileAbsPath); + raw.typeHierarchy = { fish: 'fish' }; + await fs.writeJSON(project.datasetFileAbsPath, raw); + await fs.remove(output); + await expect(common.exportConfiguration( + settings, + { id: 'projectid1', path: output }, + )).rejects.toThrow( + 'Type hierarchy is invalid: self edge "fish -> fish". ' + + 'No configuration file was exported.', + ); + expect(await fs.pathExists(output)).toBe(false); + }); + it('loadJsonConfig parses per-camera frame timestamps for multicam datasets', async () => { const data = await common.loadConfig(settings, 'stereoDataset', urlMapper); expect(data.multiCamMedia).not.toBeNull(); @@ -1340,7 +1643,12 @@ describe('native.common', () => { }; seededBase.cameraTransformTypes = { 'left::right': 'similarity' }; seededBase.cameraRegistrationSource = { model: 'seeded' }; + seededBase.typeHierarchy = { shark: 'fish' }; await fs.writeJSON(baseDir.datasetFileAbsPath, seededBase); + const cameraDir = common.getProjectDir(settings, `${baseId}/left`); + const seededCamera = await common.loadJsonConfig(cameraDir.datasetFileAbsPath); + seededCamera.typeHierarchy = { whale: 'mammal' }; + await fs.writeJSON(cameraDir.datasetFileAbsPath, seededCamera); await common.dataFileImport( settings, @@ -1361,6 +1669,30 @@ describe('native.common', () => { expect(baseMeta.cameraCorrespondences).toStrictEqual(seededBase.cameraCorrespondences); expect(baseMeta.cameraTransformTypes).toStrictEqual(seededBase.cameraTransformTypes); expect(baseMeta.cameraRegistrationSource).toStrictEqual(seededBase.cameraRegistrationSource); + + const hierarchyImport = '/home/user/output/multicam-hierarchy.json'; + await fs.writeJSON(hierarchyImport, { typeHierarchy: { shark: 'animal' } }); + const parentBeforeConflict = await fs.readFile(baseDir.datasetFileAbsPath, 'utf8'); + const cameraBeforeConflict = await fs.readFile(cameraDir.datasetFileAbsPath, 'utf8'); + await expect(common.dataFileImport( + settings, + `${baseId}/left`, + hierarchyImport, + true, + )).rejects.toThrow( + 'Type hierarchy is invalid: conflicting parents for "shark": "fish" and "animal". ' + + 'No configuration was changed.', + ); + expect(await fs.readFile(baseDir.datasetFileAbsPath, 'utf8')).toBe(parentBeforeConflict); + expect(await fs.readFile(cameraDir.datasetFileAbsPath, 'utf8')).toBe(cameraBeforeConflict); + + await fs.writeJSON(hierarchyImport, { typeHierarchy: { tuna: 'fish' } }); + await common.dataFileImport(settings, `${baseId}/left`, hierarchyImport, true); + const resolvedHierarchy = { shark: 'fish', tuna: 'fish' }; + expect((await common.loadConfig(settings, baseId, urlMapper)).typeHierarchy) + .toEqual(resolvedHierarchy); + expect((await common.loadConfig(settings, `${baseId}/left`, urlMapper)).typeHierarchy) + .toEqual(resolvedHierarchy); }); it('saveConfig writes per-camera registration files (pairs + points) and reloads them', async () => { diff --git a/client/platform/desktop/backend/native/common.ts b/client/platform/desktop/backend/native/common.ts index c7adedc49..7be5f49c0 100644 --- a/client/platform/desktop/backend/native/common.ts +++ b/client/platform/desktop/backend/native/common.ts @@ -58,6 +58,12 @@ import { cleanString, filterByGlob, makeid, strNumericCompare, } from 'platform/desktop/sharedUtils'; import { parseFrameTimestamp } from 'dive-common/frameTimestamp'; +import { + HierarchyWrite, + normalizeTypeHierarchy, + resolveTypeHierarchy, + TypeHierarchyError, +} from 'dive-common/typeHierarchy'; import processTrackAttributes from './attributeProcessor'; import { upgrade } from './migrations'; @@ -91,6 +97,16 @@ const PortableConfigFileNameLegacy = 'meta.json'; const CsvFileName = /^.*\.csv$/i; const YAMLFileName = /^.*\.ya?ml$/i; +const invalidHierarchyMessage = (reason: string) => ( + `Type hierarchy is invalid: ${reason}. No configuration was changed.` +); + +const corruptHierarchyExportMessage = (reason: string) => ( + `Type hierarchy is invalid: ${reason}. No configuration file was exported.` +); + +class DataFileJsonParseError extends Error {} + /** * Resolve the project dataset.json path: prefer dataset.json, fall back to * legacy meta.json for existing datasets, else the preferred name for new projects. @@ -341,7 +357,7 @@ async function _loadAsJson(abspath: string) { try { return JSON.parse(rawBuffer); } catch (err) { - throw new Error(`Unable to parse ${abspath}: ${err}`); + throw new DataFileJsonParseError(`Unable to parse ${abspath}: ${err}`); } } @@ -535,6 +551,15 @@ async function loadConfig( ): Promise { const projectDirData = await getValidatedProjectDir(settings, datasetId); const projectMetaData = await loadJsonConfig(projectDirData.datasetFileAbsPath); + const { parentId, cameraName } = parseCompositeDatasetId(datasetId); + if (cameraName) { + const hierarchy = await loadCanonicalHierarchy(settings, parentId); + if (hierarchy === null) { + delete projectMetaData.typeHierarchy; + } else { + projectMetaData.typeHierarchy = hierarchy as Record; + } + } // Load the standalone camera registration (transforms + correspondences) // from the per-camera *_registration.json files, if present; the dataset @@ -1227,51 +1252,76 @@ async function saveConfig(settings: Settings, datasetId: string, args: DatasetCo projectDirInfo.basePath, 'meta', ); - const existing = await loadJsonConfig( - resolveDatasetFileAbsPath(projectDirInfo.basePath), - ); - if (args.confidenceFilters) { - existing.confidenceFilters = args.confidenceFilters; - } - if (args.imageEnhancements) { - existing.imageEnhancements = args.imageEnhancements; - } - if (args.customTypeStyling) { - existing.customTypeStyling = args.customTypeStyling; - } - if (args.customGroupStyling) { - existing.customGroupStyling = args.customGroupStyling; - } - if (args.attributes) { - existing.attributes = args.attributes; - } - if (args.timeFilters !== undefined) { - existing.timeFilters = args.timeFilters; - } - if (args.error) { - existing.error = args.error; - } - if (args.datasetInfo) { - existing.datasetInfo = args.datasetInfo; - } - - // The camera registration (transforms + the points behind them) is - // persisted as standalone _to__registration.json files - // in the dataset directory rather than embedded in dataset.json, so each - // camera's registration is easy to find, hand-edit, and consume as a - // self-contained artifact. There is deliberately never a single all-pairs - // file. - if (args.cameraHomographies || args.cameraCorrespondences || args.cameraTransformTypes - || args.cameraRegistrationSource) { - await saveRegistrationToDatasetDir( - projectDirInfo.basePath, - args, - referenceCameraName(existing), + try { + const existing = await loadJsonConfig( + resolveDatasetFileAbsPath(projectDirInfo.basePath), ); - } + const { parentId, cameraName } = parseCompositeDatasetId(datasetId); + const hierarchyPresent = Object.prototype.hasOwnProperty.call(args, 'typeHierarchy'); + if (cameraName) { + if (hierarchyPresent) { + await saveConfig(settings, parentId, { typeHierarchy: args.typeHierarchy }); + } + delete existing.typeHierarchy; + } + let hierarchyWrite: HierarchyWrite; + try { + hierarchyWrite = resolveTypeHierarchy( + existing.typeHierarchy, + !cameraName && hierarchyPresent, + args.typeHierarchy, + 'save', + ); + } catch (error) { + if (error instanceof TypeHierarchyError) { + throw new Error(invalidHierarchyMessage(error.reason)); + } + throw error; + } + if (hierarchyWrite.action === 'set') { + existing.typeHierarchy = { ...hierarchyWrite.hierarchy }; + } else if (hierarchyWrite.action === 'delete') { + delete existing.typeHierarchy; + } + if (args.confidenceFilters) { + existing.confidenceFilters = args.confidenceFilters; + } + if (args.imageEnhancements) { + existing.imageEnhancements = args.imageEnhancements; + } + if (args.customTypeStyling) { + existing.customTypeStyling = args.customTypeStyling; + } + if (args.customGroupStyling) { + existing.customGroupStyling = args.customGroupStyling; + } + if (args.attributes) { + existing.attributes = args.attributes; + } + if (args.timeFilters !== undefined) { + existing.timeFilters = args.timeFilters; + } + if (args.error) { + existing.error = args.error; + } + if (args.datasetInfo) { + existing.datasetInfo = args.datasetInfo; + } - await saveProjectConfig(projectDirInfo.basePath, existing); - await release(); + // Registration files remain separate so each camera pair has one persisted owner. + if (args.cameraHomographies || args.cameraCorrespondences || args.cameraTransformTypes + || args.cameraRegistrationSource) { + await saveRegistrationToDatasetDir( + projectDirInfo.basePath, + args, + referenceCameraName(existing), + ); + } + + await saveProjectConfig(projectDirInfo.basePath, existing); + } finally { + await release(); + } } async function saveAttributes(settings: Settings, datasetId: string, args: SaveAttributeArgs) { @@ -1318,12 +1368,14 @@ async function saveAttributeTrackFilters( async function _ingestFilePath( settings: Settings, - datasetId: string, - path: string, - imageMap?: Map, - additive = false, - additivePrepend = '', -): Promise<[(DatasetConfigMutable & { fps?: number }), string[]] | null> { + plan: IngestFilePlan, + imageMap: Map | undefined, +): Promise<[ + (DatasetConfigMutable & { fps?: number }), string[], boolean, string, +] | null> { + const { + datasetId, path, additive, additivePrepend, configMeta, + } = plan; if (!fs.existsSync(path)) { return null; } @@ -1344,7 +1396,10 @@ async function _ingestFilePath( let annotations = dive.makeEmptyAnnotationFile(); const meta: DatasetConfigMutable & { fps?: number, execTime?: number } = {}; let metadataConfig = false; - if (JsonFileName.test(path)) { + if (configMeta) { + Object.assign(meta, configMeta); + metadataConfig = true; + } else if (JsonFileName.test(path)) { const jsonObject = await _loadAsJson(path); if (nistSerializers.confirmNistFormat(jsonObject)) { // NIST json file @@ -1352,10 +1407,6 @@ async function _ingestFilePath( annotations.tracks = data.tracks; annotations.groups = data.groups; meta.fps = data.fps; - } else if (DatasetConfigMutableKeys.some((key) => key in jsonObject)) { - // DIVE Configuration File (attributes, styles, FPS, …) - merge(meta, pick(jsonObject, DatasetConfigMutableKeys)); - metadataConfig = true; } else if (coco.isCocoJson(jsonObject)) { const [parsedAnnotations, parsedMeta, cocoWarnings] = await coco.parseFile(path); annotations = parsedAnnotations; @@ -1420,7 +1471,153 @@ async function _ingestFilePath( await _saveSerialized(settings, datasetId, annotations, true); } - return [meta, warnings]; + return [meta, warnings, metadataConfig, newPath]; +} + +type StagedConfigImport = DatasetConfigMutable & { fps?: number }; + +interface IngestFilePlan { + datasetId: string; + path: string; + additive: boolean; + additivePrepend: string; + configMeta?: StagedConfigImport; +} + +async function loadCanonicalHierarchy(settings: Settings, datasetId: string): Promise { + const { parentId, cameraName } = parseCompositeDatasetId(datasetId); + const canonicalId = cameraName ? parentId : datasetId; + const projectDir = getProjectDir(settings, canonicalId); + if (!await fs.pathExists(projectDir.datasetFileAbsPath)) { + return null; + } + const config = await loadJsonConfig(projectDir.datasetFileAbsPath); + return Object.prototype.hasOwnProperty.call(config, 'typeHierarchy') + ? config.typeHierarchy + : null; +} + +function mergeImportedConfig( + target: DatasetConfigMutable, + incoming: DatasetConfigMutable, +) { + const hierarchyPresent = Object.prototype.hasOwnProperty.call(incoming, 'typeHierarchy'); + const hierarchy = incoming.typeHierarchy; + const nonHierarchy = { ...incoming }; + delete nonHierarchy.typeHierarchy; + merge(target, nonHierarchy); + if (hierarchyPresent) { + if (hierarchy === null) { + // eslint-disable-next-line no-param-reassign + delete target.typeHierarchy; + } else { + // eslint-disable-next-line no-param-reassign + target.typeHierarchy = hierarchy ? { ...hierarchy } : hierarchy; + } + } +} + +function mergeStagedImportedConfig( + target: DatasetConfigMutable, + incoming: DatasetConfigMutable, + additive: boolean, +) { + const hierarchyPresent = Object.prototype.hasOwnProperty.call(incoming, 'typeHierarchy'); + const hierarchy = incoming.typeHierarchy; + const nonHierarchy = { ...incoming }; + delete nonHierarchy.typeHierarchy; + const { datasetInfo } = nonHierarchy; + delete nonHierarchy.datasetInfo; + merge(target, nonHierarchy); + if (datasetInfo) { + // eslint-disable-next-line no-param-reassign + target.datasetInfo = additive + ? { ...(target.datasetInfo || {}), ...datasetInfo } + : datasetInfo; + } + if (hierarchyPresent) { + // eslint-disable-next-line no-param-reassign + target.typeHierarchy = hierarchy === null ? null : { ...hierarchy }; + } +} + +async function preflightIngestFiles( + settings: Settings, + datasetId: string, + absPaths: string[], + multiCamResults: Record | undefined, + additive: boolean, + additivePrepend: string, +): Promise { + let hierarchyCandidate = await loadCanonicalHierarchy(settings, datasetId); + const plan: IngestFilePlan[] = [ + ...absPaths.map((path) => ({ + datasetId, + path, + additive, + additivePrepend, + })), + ...Object.entries(multiCamResults || {}).map(([cameraName, path]) => ({ + datasetId: `${datasetId}/${cameraName}`, + path, + additive: false, + additivePrepend: '', + })), + ]; + for (let index = 0; index < plan.length; index += 1) { + const entry = plan[index]; + const { path } = entry; + if (fs.existsSync(path) && fs.statSync(path).size > 0 && JsonFileName.test(path)) { + // Configuration entries keep their parsed, fully resolved metadata in this + // plan so execution cannot re-read the source or repeat hierarchy policy. + let jsonObject; + try { + // eslint-disable-next-line no-await-in-loop + jsonObject = await _loadAsJson(path); + } catch (error) { + if (error instanceof DataFileJsonParseError) { + // Defer syntax failures so earlier annotations retain ordered partial writes. + jsonObject = undefined; + } else { + throw error; + } + } + if (jsonObject !== undefined + && jsonObject !== null + && typeof jsonObject === 'object' + && !Array.isArray(jsonObject) + && !nistSerializers.confirmNistFormat(jsonObject) + && DatasetConfigMutableKeys.some((key) => key in jsonObject)) { + try { + const configMeta = pick( + jsonObject, + DatasetConfigMutableKeys, + ) as StagedConfigImport; + const write = resolveTypeHierarchy( + hierarchyCandidate, + Object.prototype.hasOwnProperty.call(jsonObject, 'typeHierarchy'), + jsonObject.typeHierarchy, + additive ? 'additive' : 'overwrite', + ); + delete configMeta.typeHierarchy; + if (write.action === 'set') { + hierarchyCandidate = write.hierarchy; + configMeta.typeHierarchy = { ...write.hierarchy }; + } else if (write.action === 'delete') { + hierarchyCandidate = null; + configMeta.typeHierarchy = null; + } + entry.configMeta = configMeta; + } catch (error) { + if (error instanceof TypeHierarchyError) { + throw new Error(invalidHierarchyMessage(error.reason)); + } + throw error; + } + } + } + } + return plan; } /** @@ -1450,35 +1647,39 @@ async function ingestDataFiles( warnings: string[]; }> { const processedFiles = []; // which files were processed to generate the detections - const meta = {}; + const meta: DatasetConfigMutable & { fps?: number } = {}; let outwarnings: string[] = []; - for (let i = 0; i < absPaths.length; i += 1) { - const path = absPaths[i]; - // eslint-disable-next-line no-await-in-loop - const results = await _ingestFilePath(settings, datasetId, path, imageMap, additive, additivePrepend); - if (results !== null) { - const [newMeta, warnings] = results; - outwarnings = outwarnings.concat(warnings); - merge(meta, newMeta); - processedFiles.push(path); - } - } - // processing of multiCam results - if (multiCamResults) { - const cameraAndPath = Object.entries(multiCamResults); - for (let i = 0; i < cameraAndPath.length; i += 1) { - const cameraName = cameraAndPath[i][0]; - const path = cameraAndPath[i][1]; - const cameraDatasetId = `${datasetId}/${cameraName}`; + const plan = await preflightIngestFiles( + settings, + datasetId, + absPaths, + multiCamResults, + additive, + additivePrepend, + ); + const importedConfigCopies: string[] = []; + try { + for (let i = 0; i < plan.length; i += 1) { + const entry = plan[i]; // eslint-disable-next-line no-await-in-loop - const results = await _ingestFilePath(settings, cameraDatasetId, path, imageMap); + const results = await _ingestFilePath( + settings, + entry, + imageMap, + ); if (results !== null) { - const [newMeta, warnings] = results; + const [newMeta, warnings, metadataConfig, auxiliaryPath] = results; outwarnings = outwarnings.concat(warnings); - merge(meta, newMeta); - processedFiles.push(path); + mergeStagedImportedConfig(meta, newMeta, additive); + if (metadataConfig) { + importedConfigCopies.push(auxiliaryPath); + } + processedFiles.push(entry.path); } } + } catch (error) { + await Promise.all(importedConfigCopies.map((path) => fs.remove(path))); + throw error; } return { processedFiles, meta, warnings: outwarnings }; @@ -2022,7 +2223,13 @@ async function dataFileImport(settings: Settings, id: string, path: string, addi additive, additivePrepend, ); - merge(jsonConfig, result.meta); + const { parentId, cameraName } = parseCompositeDatasetId(id); + const cameraMeta = { ...result.meta }; + if (cameraName) { + delete cameraMeta.typeHierarchy; + delete jsonConfig.typeHierarchy; + } + mergeImportedConfig(jsonConfig, cameraMeta); // Assign datasetInfo explicitly; the deep-merge above would keep keys an Overwrite // import meant to drop. Like the server, Overwrite replaces the block wholesale while // an additive import merges per-key (imported values win). @@ -2036,13 +2243,19 @@ async function dataFileImport(settings: Settings, id: string, path: string, addi // loaded by the viewer from the base dataset's metadata, so an import // targeted at one camera of a multicam dataset must update the base too. // Do not sync per-camera imageEnhancements or camera-registration fields. - const { parentId, cameraName } = parseCompositeDatasetId(id); - if (cameraName && MulticamSharedMutableKeys.some((key) => key in result.meta)) { + const hierarchyPresent = Object.prototype.hasOwnProperty.call(result.meta, 'typeHierarchy'); + if (cameraName && ( + hierarchyPresent || MulticamSharedMutableKeys.some((key) => key in result.meta) + )) { const baseProjectDir = getProjectDir(settings, parentId); if (await fs.pathExists(baseProjectDir.datasetFileAbsPath)) { const baseMeta = await loadJsonConfig(baseProjectDir.datasetFileAbsPath); const existingBaseDatasetInfo = baseMeta.datasetInfo; - merge(baseMeta, pick(result.meta, MulticamSharedMutableKeys)); + const parentMeta = pick(result.meta, MulticamSharedMutableKeys); + if (hierarchyPresent) { + parentMeta.typeHierarchy = result.meta.typeHierarchy; + } + mergeImportedConfig(baseMeta, parentMeta); if (result.meta.datasetInfo) { baseMeta.datasetInfo = additive ? { ...(existingBaseDatasetInfo ?? {}), ...result.meta.datasetInfo } @@ -2395,11 +2608,36 @@ async function exportDataset(settings: Settings, args: ExportDatasetArgs) { async function exportConfiguration(settings: Settings, args: ExportConfigurationArgs) { const projectDirInfo = await getValidatedProjectDir(settings, args.id); const meta = await loadJsonConfig(projectDirInfo.datasetFileAbsPath); + const { cameraName } = parseCompositeDatasetId(args.id); + if (cameraName) { + const hierarchy = await loadCanonicalHierarchy(settings, args.id); + if (hierarchy === null) { + delete meta.typeHierarchy; + } else { + meta.typeHierarchy = hierarchy as Record; + } + } const output: DatasetConfigMutable & { version: number} = { version: meta.version }; + let hierarchy; + try { + hierarchy = Object.prototype.hasOwnProperty.call(meta, 'typeHierarchy') + ? normalizeTypeHierarchy(meta.typeHierarchy) + : undefined; + } catch (error) { + if (error instanceof TypeHierarchyError) { + throw new Error(corruptHierarchyExportMessage(error.reason)); + } + throw error; + } if (DatasetConfigMutableKeys.some((key) => key in meta)) { // DIVE Configuration File fields (attributes, styles, FPS, …) merge(output, pick(meta, DatasetConfigMutableKeys)); } + if (hierarchy) { + output.typeHierarchy = { ...hierarchy }; + } else { + delete output.typeHierarchy; + } await fs.writeJSON(args.path, output); return args.path; } diff --git a/client/platform/desktop/backend/native/multicamExport.ts b/client/platform/desktop/backend/native/multicamExport.ts index 385f66262..cc73fed87 100644 --- a/client/platform/desktop/backend/native/multicamExport.ts +++ b/client/platform/desktop/backend/native/multicamExport.ts @@ -75,6 +75,7 @@ async function writeDatasetExportContents( datasetId: string, excludeBelowThreshold: boolean, typeFilter: Set, + includeHierarchy = true, ): Promise { const projectDirInfo = await getValidatedProjectDir(settings, datasetId); const meta = await loadJsonConfig(projectDirInfo.datasetFileAbsPath); @@ -86,6 +87,9 @@ async function writeDatasetExportContents( await fs.ensureDir(destDir); const exportMeta = buildExportMetaJson(meta); + if (!includeHierarchy) { + delete exportMeta.typeHierarchy; + } if (meta.metadataFile) { if (!await fs.pathExists(meta.metadataFile)) { throw new Error(`Metadata attachment is missing: ${meta.metadataFile}`); @@ -188,6 +192,7 @@ export async function exportMulticamEverything( `${parentId}/${cameraName}`, args.exclude, args.typeFilter, + false, ); } diff --git a/client/platform/web-girder/views/Upload.spec.ts b/client/platform/web-girder/views/Upload.spec.ts index d4478dfd2..249c72d06 100644 --- a/client/platform/web-girder/views/Upload.spec.ts +++ b/client/platform/web-girder/views/Upload.spec.ts @@ -326,6 +326,27 @@ describe('Upload pending rows', () => { ]); }); + it('keeps a validated type hierarchy configuration in the upload package', async () => { + pick([file('dive.mp4'), file('hierarchy.config.json')]); + vi.mocked(validateUploadGroup).mockResolvedValue({ + data: validation({ + roles: { + media: ['dive.mp4'], + datasetConfig: ['hierarchy.config.json'], + }, + }), + } as never); + + const wrapper = mountUpload(); + await wrapper.vm.openImport('video'); + + const [row] = wrapper.vm.pendingUploads; + expect(row.uploadFiles.map((entry: File) => entry.name)).toEqual([ + 'dive.mp4', + 'hierarchy.config.json', + ]); + }); + it('starts only one upload when Start upload is clicked twice', async () => { pick([file('dive.mp4')]); vi.mocked(validateUploadGroup).mockResolvedValue({ diff --git a/client/src/TrackFilterControls.spec.ts b/client/src/TrackFilterControls.spec.ts index a445370ca..a188ce88e 100644 --- a/client/src/TrackFilterControls.spec.ts +++ b/client/src/TrackFilterControls.spec.ts @@ -1,10 +1,27 @@ /// -import { nextTick } from 'vue'; +import { nextTick, ref } from 'vue'; import Track, { Feature } from './track'; import TrackFilterControls from './TrackFilterControls'; import GroupFilterControls from './GroupFilterControls'; +import type { MarkChangesPendingFilter } from './BaseFilterControls'; import CameraStore from './CameraStore'; import { AnnotationId } from './BaseAnnotation'; +import useSave from '../dive-common/use/useSave'; + +const apiMocks = vi.hoisted(() => ({ + saveConfig: vi.fn(), + saveDetections: vi.fn(), + saveAttributes: vi.fn(), + saveAttributeTrackFilters: vi.fn(), +})); + +vi.mock('dive-common/apispec', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + useApi: () => apiMocks, + }; +}); const markChangesPending = () => null; @@ -67,7 +84,7 @@ function makeGroupFilterControls(store: CameraStore) { }); } -function makeTrackFilterControls() { +function makeTrackFilterControls(markPending: MarkChangesPendingFilter = markChangesPending) { const cameraStore = makeCameraStore(); const groupFilterControls = makeGroupFilterControls(cameraStore); const setTrackType = ( @@ -87,7 +104,7 @@ function makeTrackFilterControls() { return new TrackFilterControls({ sorted: cameraStore.sortedTracks, remove, - markChangesPending, + markChangesPending: markPending, groupFilterControls, lookupGroups: cameraStore.lookupGroups, getTrack: (track: AnnotationId, camera = 'singleCam') => (cameraStore.getTrack(track, camera)), @@ -97,6 +114,136 @@ function makeTrackFilterControls() { } describe('useAnnotationFilters', () => { + it('loads absent and valid hierarchy state without creating a save instruction', () => { + const tf = makeTrackFilterControls(); + tf.setTypeHierarchy(undefined); + expect(tf.hierarchyActive.value).toBe(false); + expect(tf.invalidHierarchyReason.value).toBeNull(); + expect(tf.consumeLoadWarning()).toBeNull(); + expect(tf.typeHierarchySavePatch()).toEqual({}); + + tf.setTypeHierarchy({ shark: 'fish', 'great white shark': 'shark' }); + expect(tf.hierarchyActive.value).toBe(true); + expect(tf.allTypes.value).toEqual([ + 'foo', 'bar', 'baz', 'great white shark', 'shark', 'fish', + ]); + expect(tf.checkedTypes.value).toEqual(expect.arrayContaining([ + 'great white shark', 'shark', 'fish', + ])); + expect(tf.typeHierarchySavePatch()).toEqual({}); + }); + + it('disables an invalid stored hierarchy and emits its load warning once', () => { + const tf = makeTrackFilterControls(); + tf.setTypeHierarchy({ fish: 'fish' }); + + expect(tf.hierarchyActive.value).toBe(false); + expect(tf.invalidHierarchyReason.value).toBe('self edge "fish -> fish"'); + expect(tf.typeHierarchySavePatch()).toEqual({}); + expect(tf.consumeLoadWarning()).toBe( + 'The saved type hierarchy is invalid: self edge "fish -> fish". ' + + 'Hierarchical type selection is disabled until the configuration is corrected.', + ); + expect(tf.consumeLoadWarning()).toBeNull(); + }); + + it('re-arms hierarchy load warnings and removes stale hierarchy-only choices on reset', () => { + const tf = makeTrackFilterControls(); + tf.setTypeHierarchy({ shark: 'fish' }); + expect(tf.checkedTypes.value).toEqual(expect.arrayContaining(['shark', 'fish'])); + + tf.setTypeHierarchy(undefined); + expect(tf.allTypes.value).toEqual(['foo', 'bar', 'baz']); + expect(tf.checkedTypes.value).not.toEqual(expect.arrayContaining(['shark', 'fish'])); + + tf.setTypeHierarchy({ fish: 'fish' }); + expect(tf.consumeLoadWarning()).not.toBeNull(); + }); + + it('retains a hierarchy save patch until persistence succeeds', () => { + const tf = makeTrackFilterControls(); + tf.setTypeHierarchy({ shark: 'fish' }); + tf.updateTypeHierarchy({ shark: 'fish', tuna: 'fish' }); + + const expected = { typeHierarchy: { shark: 'fish', tuna: 'fish' } }; + expect(tf.typeHierarchySavePatch()).toEqual(expected); + expect(tf.typeHierarchySavePatch()).toEqual(expected); + tf.markTypeHierarchyPersisted(); + expect(tf.typeHierarchySavePatch()).toEqual({}); + }); + + it('retries every multicamera hierarchy target after the parent save fails', async () => { + const datasetId = 'multicam-dataset'; + const saveControls = useSave(ref(datasetId), ref(false)); + saveControls.removeCamera('singleCam'); + saveControls.addCamera('left'); + saveControls.addCamera('right'); + const tf = makeTrackFilterControls( + saveControls.markChangesPending as MarkChangesPendingFilter, + ); + tf.setTypeHierarchy({ shark: 'fish' }); + tf.updateTypeHierarchy({ shark: 'fish', tuna: 'fish' }); + const expected = { typeHierarchy: { shark: 'fish', tuna: 'fish' } }; + const visiblePendingCount = () => Math.max( + 0, + saveControls.pendingSaveCount.value - tf.typeHierarchyPendingCountAdjustment(), + ); + expect(saveControls.pendingSaveCount.value).toBe(1); + expect(visiblePendingCount()).toBe(1); + let rejectParent = true; + apiMocks.saveConfig.mockImplementation(async (id: string) => { + if (id === datasetId && rejectParent) { + rejectParent = false; + throw new Error('parent save failed'); + } + }); + + const firstPatch = tf.prepareTypeHierarchySavePatch(); + expect(saveControls.pendingSaveCount.value).toBe(1); + expect(tf.typeHierarchyPendingCountAdjustment()).toBe(0); + expect(visiblePendingCount()).toBe(1); + await expect(saveControls.save(firstPatch)).rejects.toThrow('parent save failed'); + expect(tf.typeHierarchySavePatch()).toEqual(expected); + + const retryPatch = tf.prepareTypeHierarchySavePatch(); + expect(retryPatch).toEqual(firstPatch); + expect(saveControls.pendingSaveCount.value).toBe(2); + expect(tf.typeHierarchyPendingCountAdjustment()).toBe(1); + expect(visiblePendingCount()).toBe(1); + await saveControls.save(retryPatch); + expect(apiMocks.saveConfig.mock.calls).toEqual([ + [`${datasetId}/left`, expected], + [`${datasetId}/right`, expected], + [datasetId, expected], + [`${datasetId}/left`, expected], + [`${datasetId}/right`, expected], + [datasetId, expected], + ]); + + tf.markTypeHierarchyPersisted(); + expect(tf.typeHierarchyPendingCountAdjustment()).toBe(0); + expect(visiblePendingCount()).toBe(0); + expect(tf.prepareTypeHierarchySavePatch()).toEqual({}); + }); + + it('uses an explicit delete patch for a locally cleared hierarchy', () => { + const tf = makeTrackFilterControls(); + tf.setTypeHierarchy({ shark: 'fish' }); + tf.updateTypeHierarchy(null); + expect(tf.typeHierarchySavePatch()).toEqual({ typeHierarchy: null }); + }); + + it('accepts corrected replacement and clear loads after invalid storage', () => { + const tf = makeTrackFilterControls(); + tf.setTypeHierarchy({ fish: 'fish' }); + tf.setTypeHierarchy({ shark: 'fish' }); + expect(tf.hierarchyActive.value).toBe(true); + expect(tf.invalidHierarchyReason.value).toBeNull(); + tf.setTypeHierarchy({}); + expect(tf.hierarchyActive.value).toBe(false); + expect(tf.typeHierarchySavePatch()).toEqual({}); + }); + it('updateTypeName', async () => { const tf = makeTrackFilterControls(); tf.setConfidenceFilters({ baz: 0.1, bar: 0.2, default: 0.1 }); diff --git a/client/src/TrackFilterControls.ts b/client/src/TrackFilterControls.ts index 5d2b8d512..0a0ae3463 100644 --- a/client/src/TrackFilterControls.ts +++ b/client/src/TrackFilterControls.ts @@ -1,6 +1,13 @@ import { computed, Ref, ref } from 'vue'; import { cloneDeep } from 'lodash'; import { clientSettings } from 'dive-common/store/settings'; +import { + compileHierarchy, + normalizeTypeHierarchy, + TypeHierarchy, + TypeHierarchyError, + TypeHierarchyIndex, +} from 'dive-common/typeHierarchy'; import { AnnotationId } from './BaseAnnotation'; import BaseFilterControls, { AnnotationWithContext, FilterControlsParams } from './BaseFilterControls'; import type Group from './Group'; @@ -22,9 +29,48 @@ export default class TrackFilterControls extends BaseFilterControls { enabledFilters: Ref; + typeHierarchy: Ref; + + hierarchyIndex: Ref; + + hierarchyActive: Ref; + + hierarchyMembers: Ref; + + usedPlusConfiguredTypes: Ref; + + invalidHierarchyReason: Ref; + + private hierarchyWarningConsumed = false; + + private hierarchyDirty = false; + + private hierarchySavePrepared = false; + + private hierarchySaveRearmCount = ref(0); + constructor(params: TrackFilterControlsParams) { super(params); + const flatAllTypes = this.allTypes; + this.usedPlusConfiguredTypes = flatAllTypes; + 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.hierarchyActive = computed(() => this.hierarchyIndex.value !== undefined); + this.allTypes = computed(() => Array.from(new Set([ + ...flatAllTypes.value, + ...this.hierarchyMembers.value, + ]))); + this.attributeFilters = ref([]); this.userDefinedValues = ref([]); @@ -113,6 +159,103 @@ export default class TrackFilterControls extends BaseFilterControls { }); } + private installTypeHierarchy(value: unknown, dirty: boolean) { + const previousMembers = new Set(this.hierarchyMembers.value); + let normalized: TypeHierarchy | undefined; + try { + normalized = normalizeTypeHierarchy(value === undefined ? null : value); + this.invalidHierarchyReason.value = null; + } catch (error) { + if (!(error instanceof TypeHierarchyError)) { + throw error; + } + if (dirty) { + throw error; + } + normalized = undefined; + this.invalidHierarchyReason.value = error.reason; + } + + const current = this.typeHierarchy.value; + const changed = !isEqual(current, normalized); + this.typeHierarchy.value = normalized; + if (changed) { + this.hierarchyIndex.value = normalized ? compileHierarchy(normalized) : undefined; + } + + const nextMembers = new Set(this.hierarchyMembers.value); + const baseline = new Set(this.usedPlusConfiguredTypes.value); + const checked = this.checkedTypes.value.filter( + (name) => baseline.has(name) || nextMembers.has(name), + ); + nextMembers.forEach((name) => { + if (!previousMembers.has(name) && !checked.includes(name)) { + checked.push(name); + } + }); + this.checkedTypes.value = checked; + + this.hierarchyDirty = dirty; + this.hierarchySavePrepared = false; + if (!dirty) { + this.hierarchySaveRearmCount.value = 0; + } + } + + /** Install hierarchy state loaded from a dataset or a successful config replacement. */ + setTypeHierarchy(value: unknown) { + this.hierarchyWarningConsumed = false; + this.installTypeHierarchy(value, false); + } + + /** Install a locally edited hierarchy and include it in the next metadata save. */ + updateTypeHierarchy(value: unknown) { + this.installTypeHierarchy(value, true); + this.markChangesPending({ action: 'meta' }); + } + + consumeLoadWarning(): string | null { + if (this.invalidHierarchyReason.value === null || this.hierarchyWarningConsumed) { + return null; + } + this.hierarchyWarningConsumed = true; + return `The saved type hierarchy is invalid: ${this.invalidHierarchyReason.value}. Hierarchical type selection is disabled until the configuration is corrected.`; + } + + typeHierarchySavePatch(): { typeHierarchy?: Record | null } { + if (!this.hierarchyDirty) { + return {}; + } + if (this.typeHierarchy.value === undefined) { + return { typeHierarchy: null }; + } + return { typeHierarchy: { ...(this.typeHierarchy.value || {}) } }; + } + + /** Re-arm metadata writes for every save attempt while hierarchy state is dirty. */ + prepareTypeHierarchySavePatch(): { typeHierarchy?: Record | null } { + const patch = this.typeHierarchySavePatch(); + if (Object.prototype.hasOwnProperty.call(patch, 'typeHierarchy')) { + if (this.hierarchySavePrepared) { + this.markChangesPending({ action: 'meta' }); + this.hierarchySaveRearmCount.value += 1; + } else { + this.hierarchySavePrepared = true; + } + } + return patch; + } + + typeHierarchyPendingCountAdjustment() { + return this.hierarchySaveRearmCount.value; + } + + markTypeHierarchyPersisted() { + this.hierarchyDirty = false; + this.hierarchySavePrepared = false; + this.hierarchySaveRearmCount.value = 0; + } + loadTrackAttributesFilter(trackAttributesFilter: Readonly) { this.attributeFilters.value = []; this.userDefinedValues.value = []; diff --git a/docs/DataFormats.md b/docs/DataFormats.md index da241eed3..21aff7838 100644 --- a/docs/DataFormats.md +++ b/docs/DataFormats.md @@ -180,6 +180,36 @@ This information provides the specification for an individual dataset. It consi * Edited from the [Dataset Info panel](UI-DatasetInfo.md). * Included in DIVE Configuration JSON as `datasetInfo`. * Included in [VIAME CSV](#viame-csv) and [COCO / KWCOCO](#coco-and-kwcoco) export, and restored on import. +* A track type hierarchy is stored in `typeHierarchy` as a child-type to immediate-parent-type map. + +For example, this configuration makes `fish` a heading-only parent (it does not need to be an +explicit configured type or appear in a track): + +```json +{ + "typeHierarchy": { + "shark": "fish", + "great white shark": "shark" + } +} +``` + +A type hierarchy is a single-parent forest. Child and parent names must be non-empty strings, +self-edges and cycles are invalid, and each child can have only one immediate parent. Names are +preserved exactly; whitespace is used only to determine whether a name is empty. + +A missing `typeHierarchy` leaves the saved hierarchy unchanged. On overwrite import or direct +save, `null` and `{}` delete it, while a non-empty map replaces it completely. On additive import, +`null` and `{}` make no change, while a non-empty map adds edges to the existing hierarchy. +Identical edges coalesce; a different parent for an existing child or a cycle rejects the whole +configuration without changing it. Invalid saves and imports report +`Type hierarchy is invalid: {reason}. No configuration was changed.` + +DIVE Configuration JSON exports include a valid non-empty hierarchy and omit an absent or empty +one. The `config.json` embedded in a dataset zip follows the same rules. Invalid stored hierarchy +prevents either configuration export and reports +`Type hierarchy is invalid: {reason}. No configuration file was exported.` Hierarchy is not +transported by DIVE Annotation JSON, COCO/KWCOCO, VIAME CSV, KPF, NIST, or `labels.txt`. When importing a DIVE Configuration JSON with `datasetInfo`, **Overwrite** import (the default) replaces the existing `datasetInfo` block; an additive import merges it per-key @@ -191,6 +221,7 @@ The full [DatasetMetaMutable definition can be found here](https://github.com/Ki ```typescript interface DatasetMetaMutable { version: number; + typeHierarchy?: Record | null; customTypeStyling?: Record; customGroupStyling?: Record; confidenceFilters?: Record; diff --git a/server/dive_server/crud.py b/server/dive_server/crud.py index a9d3dc40c..d0c10c78f 100644 --- a/server/dive_server/crud.py +++ b/server/dive_server/crud.py @@ -15,6 +15,7 @@ from pydantic.main import BaseModel from dive_utils import asbool, constants, fromMeta, models, strNumericCompare +from dive_utils.type_hierarchy import TypeHierarchyError from dive_utils.types import GirderModel, GirderUserModel @@ -130,12 +131,26 @@ def saveImportAttributes(folder, attributes, user): ) -def get_multicam_parent_folder(folder: GirderModel, user: GirderUserModel): - """Return the multicam parent if ``folder`` is one of its camera children, else None.""" +def hierarchy_rest_error( + error: TypeHierarchyError, + consequence: str = 'No configuration was changed.', +) -> RestException: + """Render a type hierarchy validation failure as a client-actionable REST error.""" + return RestException(f'Type hierarchy is invalid: {error.reason}. {consequence}') + + +def get_multicam_owner_folder(folder: GirderModel) -> Optional[GirderModel]: + """Return the multicam parent that registers ``folder`` as a camera child, else None. + + Camera-child membership is resolved without an ACL check: callers reach a camera child + through an already access-gated route, and the parent holds configuration that logically + belongs to the child. Callers that mutate the parent must gate on + :func:`get_multicam_parent_folder` instead. + """ parent_id = folder.get('parentId') if not parent_id: return None - parent = Folder().load(parent_id, level=AccessType.WRITE, user=user) + parent = Folder().load(parent_id, force=True) if parent is None or fromMeta(parent, constants.TypeMarker) != constants.MultiType: return None multi_cam = fromMeta(parent, constants.MultiCamMarker, default={}) or {} @@ -146,6 +161,22 @@ def get_multicam_parent_folder(folder: GirderModel, user: GirderUserModel): return parent +def get_multicam_parent_folder( + folder: GirderModel, + user: GirderUserModel, + level: AccessType = AccessType.WRITE, +): + """Return the multicam parent if ``folder`` is a camera child ``user`` may access at ``level``. + + Insufficient access yields None rather than an AccessException, so a caller holding only + child-level rights degrades to the single-folder path instead of failing the request. + """ + parent = get_multicam_owner_folder(folder) + if parent is None or not Folder().hasAccess(parent, user, level): + return None + return parent + + def get_multicam_camera_name(folder: GirderModel, parent: GirderModel) -> Optional[str]: """Return the camera name for ``folder`` within multicam ``parent``, else None.""" multi_cam = fromMeta(parent, constants.MultiCamMarker, default={}) or {} diff --git a/server/dive_server/crud_dataset.py b/server/dive_server/crud_dataset.py index fdad615f6..aa9d1ff5c 100644 --- a/server/dive_server/crud_dataset.py +++ b/server/dive_server/crud_dataset.py @@ -1,7 +1,7 @@ import copy import json from pathlib import Path -from typing import Any, Dict, Generator, Iterable, List, Optional, Tuple +from typing import Any, Dict, Generator, Iterable, List, Literal, Optional, Set, Tuple from bson.objectid import InvalidId, ObjectId import cherrypy @@ -28,6 +28,12 @@ types, ) from dive_utils.serializers import kwcoco +from dive_utils.type_hierarchy import ( + HierarchyWrite, + TypeHierarchyError, + normalize_type_hierarchy, + resolve_type_hierarchy, +) def get_url(dataset: types.GirderModel, item: types.GirderModel) -> str: @@ -146,6 +152,7 @@ def _create_multicam_soft_clone( cloned_child = _create_single_camera_soft_clone( owner, child, cloned_folder, cam_name, revision ) + remove_camera_type_hierarchy(cloned_child) new_cameras[cam_name] = { 'folderId': str(cloned_child['_id']), 'type': cam_info.get('type') or fromMeta(child, constants.TypeMarker), @@ -588,17 +595,49 @@ class Config: extra = 'forbid' -def update_metadata(dsFolder: types.GirderModel, data: dict, verify=True): +def validate_metadata_shape(data: dict) -> MetadataMutableUpdateArgs: + """Validate mutable metadata fields without changing storage.""" + return crud.get_validated_model(MetadataMutableUpdateArgs, **data) + + +def validate_type_hierarchy_update( + dsFolder: types.GirderModel, + data: dict, + hierarchy_mode: Literal['save', 'additive'] = 'save', +) -> HierarchyWrite: + """Resolve a metadata hierarchy instruction without changing storage.""" + try: + return resolve_type_hierarchy( + fromMeta(dsFolder, 'typeHierarchy'), + 'typeHierarchy' in data, + data.get('typeHierarchy'), + hierarchy_mode, + ) + except TypeHierarchyError as error: + raise crud.hierarchy_rest_error(error) from error + + +def update_metadata( + dsFolder: types.GirderModel, + data: dict, + verify=True, + hierarchy_mode: Literal['save', 'additive'] = 'save', +): """Update mutable metadata""" if verify: crud.verify_dataset(dsFolder) # Reload before save so concurrent convert_video metadata is not wiped. crud.refresh_folder_document(dsFolder) - validated: MetadataMutableUpdateArgs = crud.get_validated_model( - MetadataMutableUpdateArgs, **data - ) - for name, value in validated.dict(exclude_none=True).items(): + hierarchy_write = validate_type_hierarchy_update(dsFolder, data, hierarchy_mode) + validated = validate_metadata_shape(data) + validated_data = validated.dict(exclude_none=True) + validated_data.pop('typeHierarchy', None) + for name, value in validated_data.items(): dsFolder['meta'][name] = value + if hierarchy_write['action'] == 'set': + dsFolder['meta']['typeHierarchy'] = hierarchy_write['hierarchy'] + elif hierarchy_write['action'] == 'delete': + dsFolder['meta'].pop('typeHierarchy', None) # exclude_none drops explicit null, so a field the client nulls to clear it # must be popped by hand. timeFilters: null disables the filter; # cameraRegistrationSource: null drops a stale producer-provenance stamp when @@ -610,6 +649,30 @@ def update_metadata(dsFolder: types.GirderModel, data: dict, verify=True): return dsFolder['meta'] +def remove_camera_type_hierarchy(folder: types.GirderModel) -> bool: + """Remove a derived hierarchy copy from a multicamera child folder. + + Returns whether a stored copy was actually removed. + """ + if 'typeHierarchy' not in folder.get('meta', {}): + return False + folder['meta'].pop('typeHierarchy', None) + Folder().save(folder) + return True + + +def type_hierarchy_for_export( + dsFolder: types.GirderModel, + user: Optional[types.GirderUserModel] = None, +) -> Optional[Dict[str, str]]: + """Return a normalized export hierarchy, rejecting corrupt stored metadata.""" + try: + owner = crud.get_multicam_owner_folder(dsFolder) + return normalize_type_hierarchy(fromMeta(owner or dsFolder, 'typeHierarchy')) + except TypeHierarchyError as error: + raise crud.hierarchy_rest_error(error, 'No configuration file was exported.') from error + + class AttributeUpdateArgs(BaseModel): upsert: List[models.Attribute] = [] delete: List[str] = [] @@ -872,6 +935,7 @@ def _yield_single_dataset_export( includeDetections: bool, excludeBelowThreshold: bool, typeFilter: Iterable[str], + includeHierarchy: bool = True, ) -> Generator[bytes, None, None]: """Stream meta, annotations, media, and detections for one DIVE dataset folder.""" @@ -895,8 +959,13 @@ def makeMetajson(): """Include dataset metadata file with full export""" meta = get_dataset(dsFolder, user) media = get_media(dsFolder, user) + hierarchy = type_hierarchy_for_export(dsFolder, user) if includeHierarchy else None + meta_json = meta.dict(exclude_none=True) + meta_json.pop('typeHierarchy', None) + if hierarchy is not None: + meta_json['typeHierarchy'] = hierarchy output = { - **meta.dict(exclude_none=True), + **meta_json, **media.dict(exclude_none=True), } # Attachment locators never travel in an archive: a server-local item id means @@ -966,6 +1035,7 @@ def _yield_multicam_dataset_export( includeDetections: bool, excludeBelowThreshold: bool, typeFilter: Iterable[str], + children: Dict[str, types.GirderModel], ) -> Generator[bytes, None, None]: """Export a multicam parent plus each child camera folder.""" multi_cam = fromMeta(dsFolder, constants.MultiCamMarker) or {} @@ -994,13 +1064,7 @@ def makeMultiCamJson(): yield data for cam_name in _multicam_camera_order(multi_cam): - cam_info = multi_cam['cameras'][cam_name] - child = Folder().load(cam_info['folderId'], level=AccessType.READ, user=user) - if child is None: - raise RestException( - f'Camera folder for "{cam_name}" was not found', - code=404, - ) + child = children[cam_name] child_path = f'{zip_path}{cam_name}/' for data in _yield_single_dataset_export( z, @@ -1011,6 +1075,7 @@ def makeMultiCamJson(): includeDetections, excludeBelowThreshold, typeFilter, + includeHierarchy=False, ): yield data @@ -1024,6 +1089,38 @@ def export_datasets_zipstream( typeFilter: Optional[List[str]], ): failed_datasets = [] + skipped_dataset_ids: Set[str] = set() + multicam_children: Dict[str, Dict[str, types.GirderModel]] = {} + + # Validate while the REST endpoint is still building its response. Exceptions + # raised later by a streaming generator are replaced by CherryPy's generic 500 + # body, and the caller would lose the actionable hierarchy error. A batch export + # reports a per-dataset failure the way the stream does; a single-dataset export + # raises, because its caller has a dataset to act on. + single_dataset = len(dsFolders) == 1 + for dsFolder in dsFolders: + dataset_id = str(dsFolder['_id']) + try: + type_hierarchy_for_export(dsFolder, user) + if fromMeta(dsFolder, constants.TypeMarker) != constants.MultiType: + continue + multi_cam = fromMeta(dsFolder, constants.MultiCamMarker) or {} + children = {} + for cam_name in _multicam_camera_order(multi_cam): + cam_info = multi_cam['cameras'][cam_name] + child = Folder().load(cam_info['folderId'], level=AccessType.READ, user=user) + if child is None: + raise RestException( + f'Camera folder for "{cam_name}" was not found', + code=404, + ) + children[cam_name] = child + multicam_children[dataset_id] = children + except RestException as error: + if single_dataset: + raise + skipped_dataset_ids.add(dataset_id) + failed_datasets.append(f"Dataset: {dsFolder['name']} was not exported. {error}\n") def stream(): z = ziputil.ZipGenerator() @@ -1031,6 +1128,8 @@ def stream(): if nestedTypeFilter is None: nestedTypeFilter = set() for dsFolder in dsFolders: + if str(dsFolder['_id']) in skipped_dataset_ids: + continue zip_path = f"./{dsFolder['name']}/" source_type = fromMeta(dsFolder, constants.TypeMarker) try: @@ -1054,6 +1153,7 @@ def stream(): includeDetections, excludeBelowThreshold, nestedTypeFilter, + multicam_children[str(dsFolder['_id'])], ): yield data else: @@ -1563,6 +1663,7 @@ def create_multicam( multi_cam_cameras: Dict[str, Dict[str, str]] = {} for name in camera_order: child = loaded_children[name] + remove_camera_type_hierarchy(child) if child['name'] != name: child['name'] = name Folder().save(child) @@ -1606,7 +1707,14 @@ def create_multicam( metadata_file_item_id = str(md_item['_id']) metadata_file_name = md_item['name'] + mutable_keys = models.MetadataMutable.schema()['properties'].keys() + mutable_meta = { + key: value + for key, value in parent_folder_doc.get('meta', {}).items() + if key in mutable_keys + } parent_folder_doc['meta'] = { + **mutable_meta, constants.DatasetMarker: True, constants.TypeMarker: constants.MultiType, constants.SubTypeMarker: validated.subType, @@ -1641,8 +1749,11 @@ def create_multicam( else {} ), }, - constants.ConfidenceFiltersMarker: {'default': 0.1}, } + parent_folder_doc['meta'].setdefault( + constants.ConfidenceFiltersMarker, + {'default': 0.1}, + ) Folder().save(parent_folder_doc) crud.get_or_create_auxiliary_folder(parent_folder_doc, user) return parent_folder_doc diff --git a/server/dive_server/crud_rpc.py b/server/dive_server/crud_rpc.py index 14e98355b..65263406c 100644 --- a/server/dive_server/crud_rpc.py +++ b/server/dive_server/crud_rpc.py @@ -1,6 +1,6 @@ from datetime import datetime, timedelta import json -from typing import Dict, List, Optional, Tuple, TypedDict +from typing import Dict, List, Literal, Optional, Tuple, TypedDict, cast from girder.constants import AccessType from girder.exceptions import RestException @@ -30,6 +30,13 @@ ) from dive_utils.constants import TrainingModelExtensions from dive_utils.serializers import dive, kpf, kwcoco, viame +from dive_utils.type_hierarchy import ( + HierarchyWrite, + TypeHierarchyError, + apply_hierarchy_write, + normalize_type_hierarchy, + resolve_type_hierarchy, +) class RunTrainingArgs(BaseModel): @@ -599,6 +606,7 @@ def _frame_metadata_kept_warning(name: str) -> str: def _get_data_by_type( file: types.GirderModel, image_map: Optional[Dict[str, int]] = None, + configuration_only: bool = False, ) -> Tuple[Optional[GetDataReturnType], Optional[List[str]]]: """ Given an arbitrary Girder file model, figure out what kind of file it is and @@ -620,13 +628,29 @@ def _get_data_by_type( if file['exts'][-1] == 'csv': as_type = crud.FileType.VIAME_CSV elif file['exts'][-1] == 'json': - data_dict = json.loads(file_string) + try: + data_dict = json.loads(file_string) + except json.JSONDecodeError: + if configuration_only: + return None, None + raise if type(data_dict) is list: + if configuration_only: + return None, None raise RestException('No array-type json objects are supported') + if configuration_only and not isinstance(data_dict, dict): + return None, None if kwcoco.is_coco_json(data_dict): as_type = crud.FileType.COCO_JSON elif models.MetadataMutable.is_dive_configuration(data_dict): + hierarchy_present = 'typeHierarchy' in data_dict + normalized_hierarchy = None + if hierarchy_present: + normalized_hierarchy = normalize_type_hierarchy(data_dict['typeHierarchy']) data_dict = models.MetadataMutable(**data_dict).dict(exclude_none=True) + if hierarchy_present: + # Pydantic drops explicit null; preserve presence for the resolver. + data_dict['typeHierarchy'] = normalized_hierarchy as_type = crud.FileType.DIVE_CONF else: as_type = crud.FileType.DIVE_JSON @@ -635,6 +659,9 @@ def _get_data_by_type( else: raise RestException('Got file of unknown and unusable type') + if configuration_only and as_type != crud.FileType.DIVE_CONF: + return None, None + # Parse the file as the now known type if as_type == crud.FileType.VIAME_CSV: ( @@ -726,24 +753,8 @@ def _attach_swept_sidecar(folder: types.GirderModel, item: types.GirderModel): Folder().save(folder) -def process_items( - folder: types.GirderModel, - user: types.GirderUserModel, - additive=False, - additivePrepend='', - set='', -): - """ - Discover unprocessed items in a dataset and process them by type in order of creation - """ - attachment_item_id = crud_dataset.resolve_metadata_attachment_item_id(folder, user) - - def is_declared_sidecar(item: types.GirderModel) -> bool: - return str(item['_id']) == attachment_item_id or ( - frame_metadata.is_frame_metadata_source_name(item['name']) - ) - - unprocessed_items = list( +def _unprocessed_data_items(folder: types.GirderModel) -> list: + return list( Folder().childItems( folder, filters={ @@ -769,7 +780,274 @@ def is_declared_sidecar(item: types.GirderModel) -> bool: sort=[("created", pymongo.ASCENDING)], ) ) - aggregate_warnings: List[str] = [] + + +def _declared_sidecar_predicate(folder: types.GirderModel, user: types.GirderUserModel): + """Return the folder's attachment item id and a predicate identifying declared sidecars. + + Configuration staging and the import loop must agree on which items are sidecars, so + both resolve the attachment through this one helper. + """ + attachment_item_id = crud_dataset.resolve_metadata_attachment_item_id(folder, user) + + def is_declared_sidecar(item: types.GirderModel) -> bool: + return str(item['_id']) == attachment_item_id or ( + frame_metadata.is_frame_metadata_source_name(item['name']) + ) + + return attachment_item_id, is_declared_sidecar + + +def _parse_data_item( + item: types.GirderModel, + file: types.GirderModel, + image_map=None, + configuration_only=False, +): + try: + results, warnings = _get_data_by_type( + file, + image_map=image_map, + configuration_only=configuration_only, + ) + except TypeHierarchyError as error: + raise crud.hierarchy_rest_error(error) from error + except Exception as error: + Item().remove(item) + if isinstance(error, ValueError): + hint = ( + ' If this file is frame metadata rather than annotations, upload it ' + 'in the "Metadata File (Optional)" field on the upload page, or rename ' + 'it to frame-metadata.csv and re-upload.' + if constants.csvRegex.search(file['name']) + else '' + ) + raise RestException(f'Failed to import {file["name"]}: {error}{hint}') from error + raise RestException(f'{file["name"]} was not a supported file type: {error}') from error + if results is None and not configuration_only: + Item().remove(item) + raise RestException(f'Unknown file type for {file["name"]}') + return results, warnings + + +def _fresh_folder_snapshot(folder: types.GirderModel) -> types.GirderModel: + fresh = Folder().load(folder['_id'], force=True) + return cast(types.GirderModel, fresh) if isinstance(fresh, dict) else folder + + +def _resolve_configuration_hierarchy( + existing: object, + instructions: list, + additive: bool, +) -> HierarchyWrite: + candidate = existing + final_write: HierarchyWrite = {'action': 'none'} + for incoming_present, incoming in instructions: + next_write = resolve_type_hierarchy( + candidate, + incoming_present, + incoming, + 'additive' if additive else 'overwrite', + ) + if next_write['action'] == 'set': + candidate = next_write['hierarchy'] + final_write = next_write + elif next_write['action'] == 'delete': + candidate = None + final_write = next_write + return final_write + + +def _prepare_configuration_imports( + folder: types.GirderModel, + user: types.GirderUserModel, + additive: bool, +) -> dict: + """Resolve and stage every configuration file without changing dataset state.""" + fresh_folder = _fresh_folder_snapshot(folder) + parent = crud.get_multicam_parent_folder(fresh_folder, user) + fresh_parent = _fresh_folder_snapshot(parent) if parent is not None else None + canonical = fresh_parent if fresh_parent is not None else fresh_folder + config_results = [] + hierarchy_instructions = [] + parsed_json_items = {} + item_files = {} + + _attachment_item_id, is_declared_sidecar = _declared_sidecar_predicate(folder, user) + unprocessed_items = _unprocessed_data_items(folder) + for item in unprocessed_items: + file: Optional[types.GirderModel] = next(Item().childFiles(item), None) + if file is None: + raise RestException('Item had no associated files') + item_files[str(item['_id'])] = file + # A declared sidecar is frame metadata, never configuration; parsing it here + # would let a frame-metadata.json contribute a typeHierarchy instruction. + if is_declared_sidecar(item): + continue + if not file.get('exts') or file['exts'][-1] != 'json': + continue + results, warnings = _parse_data_item(item, file, configuration_only=True) + if results is None: + continue + parsed_json_items[str(item['_id'])] = (file, results, warnings) + if results['type'] != crud.FileType.DIVE_CONF: + continue + meta = results['meta'] or {} + hierarchy_instructions.append(('typeHierarchy' in meta, meta.get('typeHierarchy'))) + config_results.append(results) + + if ( + hierarchy_instructions + and parent is None + and crud.get_multicam_owner_folder(fresh_folder) is not None + ): + raise RestException( + 'Write access to the multicamera parent is required ' 'to change its type hierarchy.', + code=403, + ) + + try: + hierarchy_write = _resolve_configuration_hierarchy( + fromMeta(canonical, 'typeHierarchy'), + hierarchy_instructions, + additive, + ) + except TypeHierarchyError as error: + raise crud.hierarchy_rest_error(error) from error + + staged_meta = {} + staged_parent_meta = {} + working_dataset_info = fromMeta(fresh_folder, 'datasetInfo', {}) + working_parent_dataset_info = ( + fromMeta(fresh_parent, 'datasetInfo', {}) if fresh_parent is not None else {} + ) + for results in config_results: + meta = dict(results['meta'] or {}) + meta.pop('typeHierarchy', None) + meta = resolve_imported_dataset_info(working_dataset_info, meta, additive) + if 'datasetInfo' in meta: + working_dataset_info = meta['datasetInfo'] + staged_meta.update(meta) + if parent is not None: + shared_meta = crud.pick_multicam_shared_mutable(results['meta'] or {}) + shared_meta.pop('typeHierarchy', None) + shared_meta = resolve_imported_dataset_info( + working_parent_dataset_info, shared_meta, additive + ) + if 'datasetInfo' in shared_meta: + working_parent_dataset_info = shared_meta['datasetInfo'] + staged_parent_meta.update(shared_meta) + + preflight_meta = apply_hierarchy_write(staged_meta, hierarchy_write) + preflight_parent_meta = ( + apply_hierarchy_write(staged_parent_meta, hierarchy_write) if parent is not None else {} + ) + if preflight_meta: + models.MetadataMutable(**preflight_meta) + if preflight_parent_meta: + models.MetadataMutable(**preflight_parent_meta) + + return { + 'parent': parent, + 'unprocessed_items': unprocessed_items, + 'item_files': item_files, + 'parsed_json_items': parsed_json_items, + 'hierarchy_instructions': hierarchy_instructions, + 'additive': additive, + 'staged_meta': staged_meta, + 'staged_parent_meta': staged_parent_meta, + 'applied': False, + } + + +def _apply_configuration_imports( + folder: types.GirderModel, + configuration_plan: dict, +) -> HierarchyWrite: + if configuration_plan['applied']: + return configuration_plan['hierarchy_write'] + parent = configuration_plan['parent'] + canonical = parent if parent is not None else folder + fresh_canonical = _fresh_folder_snapshot(canonical) + promoted_write: HierarchyWrite = {'action': 'none'} + existing_hierarchy = fromMeta(fresh_canonical, 'typeHierarchy') + if parent is not None: + fresh_camera = _fresh_folder_snapshot(folder) + camera_hierarchy = fromMeta(fresh_camera, 'typeHierarchy') + if camera_hierarchy is not None: + try: + promoted_write = resolve_type_hierarchy( + existing_hierarchy, + True, + camera_hierarchy, + 'additive', + ) + except TypeHierarchyError as error: + configuration_plan.setdefault('warnings', []).append( + f'Camera "{folder["name"]}" type hierarchy was skipped: {error.reason}' + ) + else: + if promoted_write['action'] == 'set': + existing_hierarchy = promoted_write['hierarchy'] + try: + hierarchy_write = _resolve_configuration_hierarchy( + existing_hierarchy, + configuration_plan['hierarchy_instructions'], + configuration_plan['additive'], + ) + except TypeHierarchyError as error: + raise crud.hierarchy_rest_error(error) from error + if hierarchy_write['action'] == 'none': + hierarchy_write = promoted_write + + additive = configuration_plan['additive'] + staged_meta = dict(configuration_plan['staged_meta']) + staged_parent_meta = ( + apply_hierarchy_write(configuration_plan['staged_parent_meta'], hierarchy_write) + if parent is not None + else {} + ) + if parent is None: + staged_meta = apply_hierarchy_write(staged_meta, hierarchy_write) + hierarchy_mode: Literal['save', 'additive'] = 'additive' if additive else 'save' + if staged_meta: + crud_dataset.update_metadata(folder, staged_meta, False, hierarchy_mode=hierarchy_mode) + if parent is not None: + if staged_parent_meta: + crud_dataset.update_metadata( + parent, staged_parent_meta, False, hierarchy_mode=hierarchy_mode + ) + if crud_dataset.remove_camera_type_hierarchy(folder): + configuration_plan.setdefault('warnings', []).append( + f'Removed a type hierarchy stored on camera {folder["name"]}; ' + 'the type hierarchy for a multicamera dataset is stored on the parent.' + ) + configuration_plan['hierarchy_write'] = hierarchy_write + configuration_plan['applied'] = True + return hierarchy_write + + +def process_items( + folder: types.GirderModel, + user: types.GirderUserModel, + additive=False, + additivePrepend='', + set='', + configuration_plan=None, +): + """ + Discover unprocessed items in a dataset and process them by type in order of creation + """ + if configuration_plan is None: + configuration_plan = _prepare_configuration_imports(folder, user, additive) + _apply_configuration_imports(folder, configuration_plan) + unprocessed_items = configuration_plan['unprocessed_items'] + item_files = configuration_plan['item_files'] + parent = configuration_plan['parent'] + parsed_json_items = configuration_plan['parsed_json_items'] + + attachment_item_id, is_declared_sidecar = _declared_sidecar_predicate(folder, user) + aggregate_warnings: List[str] = list(configuration_plan.get('warnings', [])) # This sweep is also the convergence point for headless writers (assetstore/S3 import), # where nobody picked these files and nothing can be corrected pre-upload. Attachment @@ -818,9 +1096,7 @@ def is_declared_sidecar(item: types.GirderModel) -> bool: auxiliary = None for item in unprocessed_items: - file: Optional[types.GirderModel] = next(Item().childFiles(item), None) - if file is None: - raise RestException('Item had no associated files') + file = item_files[str(item['_id'])] # The single classification point: a declared sidecar is identified by attachment # identity or reserved name and never reaches the annotation classifier. Keep it in @@ -838,26 +1114,15 @@ def is_declared_sidecar(item: types.GirderModel) -> bool: aggregate_warnings.append(_frame_metadata_kept_warning(file['name'])) continue - try: - results, warnings = _get_data_by_type(file, image_map=image_map) - if warnings: - aggregate_warnings += warnings - except Exception as e: - Item().remove(item) - if isinstance(e, ValueError): - hint = ( - ' If this file is frame metadata rather than annotations, upload it ' - 'in the "Metadata File (Optional)" field on the upload page, or rename ' - 'it to frame-metadata.csv and re-upload.' - if constants.csvRegex.search(file['name']) - else '' - ) - raise RestException(f'Failed to import {file["name"]}: {e}{hint}') from e - raise RestException(f'{file["name"]} was not a supported file type: {e}') from e - - if results is None: - Item().remove(item) - raise RestException(f'Unknown file type for {file["name"]}') + # Configuration staging already parsed and validated every JSON item; reuse that + # result so an import is not parsed twice and cannot disagree with the plan. + cached = parsed_json_items.get(str(item['_id'])) + if cached is not None: + _cached_file, results, warnings = cached + else: + results, warnings = _parse_data_item(item, file, image_map) + if warnings: + aggregate_warnings += warnings if auxiliary is None: auxiliary = crud.get_or_create_auxiliary_folder(folder, user) @@ -881,7 +1146,7 @@ def is_declared_sidecar(item: types.GirderModel) -> bool: ) if results['attributes']: crud.saveImportAttributes(folder, results['attributes'], user) - if results['meta']: + if results['meta'] and results['type'] != crud.FileType.DIVE_CONF: meta = resolve_imported_dataset_info( fromMeta(folder, 'datasetInfo', {}), results['meta'], additive ) @@ -889,11 +1154,14 @@ def is_declared_sidecar(item: types.GirderModel) -> bool: # Mutable config (styling, thresholds, attributes, ...) is loaded by the # viewer from the multicam parent's metadata, so an import targeted at # one camera must update the parent too (mirrors desktop dataFileImport). - parent = crud.get_multicam_parent_folder(folder, user) if parent is not None: if results['attributes']: crud.saveImportAttributes(parent, results['attributes'], user) - shared_meta = crud.pick_multicam_shared_mutable(results['meta'] or {}) + shared_meta = ( + crud.pick_multicam_shared_mutable(results['meta'] or {}) + if results['type'] != crud.FileType.DIVE_CONF + else {} + ) if shared_meta: parent_meta = resolve_imported_dataset_info( fromMeta(parent, 'datasetInfo', {}), shared_meta, additive @@ -910,6 +1178,26 @@ def postprocess( additive=False, additivePrepend='', set='', +) -> dict: + return _postprocess( + user, + dsFolder, + skipJobs, + skipTranscoding, + additive, + additivePrepend, + set, + ) + + +def _postprocess( + user: types.GirderUserModel, + dsFolder: types.GirderModel, + skipJobs: bool, + skipTranscoding=False, + additive=False, + additivePrepend='', + set='', ) -> dict: """ Post-processing to be run after media/annotation import @@ -936,18 +1224,12 @@ def postprocess( if fromMeta(dsFolder, constants.TypeMarker) is None: raise RestException(f'{constants.TypeMarker} missing from metadata') - # Persist default confidence filter without a full stale meta write. Async - # convert_video (esp. skipTranscoding) can finish during CSV import and set - # annotate / originalFps / ffprobe_info; a later Folder().save of an old - # in-memory doc would wipe those keys. - crud.refresh_folder_document(dsFolder) - dsFolder.setdefault('meta', {})[constants.ConfidenceFiltersMarker] = {'default': 0.1} - Folder().save(dsFolder) + configuration_plan = _prepare_configuration_imports(dsFolder, user, additive) + # Folder-shape validation runs before any configuration write so a rejected + # postprocess leaves the dataset untouched. + zipItems: list = [] if not skipJobs and not isClone: - token = Token().createToken(user=user, days=2) - - # extract ZIP Files if not already completed zipItems = list( Folder().childItems( dsFolder, @@ -956,10 +1238,25 @@ def postprocess( ) if len(zipItems) > 1: raise RestException('There are multiple zip files in the folder.') + if zipItems and len(list(Folder().childItems(dsFolder))) > 1: + raise RestException('There are multiple files besides a zip, cannot continue') + + configuration_hierarchy_write = _apply_configuration_imports(dsFolder, configuration_plan) + + # Persist default confidence filter without a full stale meta write. Async + # convert_video (esp. skipTranscoding) can finish during CSV import and set + # annotate / originalFps / ffprobe_info; a later Folder().save of an old + # in-memory doc would wipe those keys. + crud.refresh_folder_document(dsFolder) + if constants.ConfidenceFiltersMarker not in dsFolder.setdefault('meta', {}): + dsFolder['meta'][constants.ConfidenceFiltersMarker] = {'default': 0.1} + Folder().save(dsFolder) + + if not skipJobs and not isClone: + token = Token().createToken(user=user, days=2) + + # extract ZIP Files if not already completed for item in zipItems: - total_items = len(list((Folder().childItems(dsFolder)))) - if total_items > 1: - raise RestException('There are multiple files besides a zip, cannot continue') convert_params = { 'user_id': str(user["_id"]), 'user_login': str(user["login"]), @@ -972,6 +1269,7 @@ def postprocess( itemId=str(item["_id"]), user_id=str(user["_id"]), user_login=str(user["login"]), + additive=additive, girder_job_title=(f"Extracting {item['name']} to folder {dsFolder['name']}"), girder_client_token=str(token["_id"]), girder_job_type="private" if job_is_private else "convert", @@ -987,7 +1285,11 @@ def postprocess( }, ) created_job_ids.append(job['_id']) - return {'folder': dsFolder, 'job_ids': created_job_ids} + return { + 'folder': dsFolder, + 'job_ids': created_job_ids, + 'configurationHierarchyWrite': configuration_hierarchy_write, + } # transcode VIDEO if necessary videoItems = Folder().childItems( @@ -1069,7 +1371,14 @@ def postprocess( dsFolder.setdefault('meta', {})[constants.DatasetMarker] = True Folder().save(dsFolder) - aggregate_warnings = process_items(dsFolder, user, additive, additivePrepend, set) + aggregate_warnings = process_items( + dsFolder, + user, + additive, + additivePrepend, + set, + configuration_plan=configuration_plan, + ) # Image sequences start at fps=-1 (auto). CSV import may have set a value; # otherwise default to 1. convert_images also resolves, but safe-image folders # skip that job and need this finalize step. @@ -1081,7 +1390,12 @@ def postprocess( if requested_fps != new_fps: dsFolder['meta'][constants.FPSMarker] = new_fps Folder().save(dsFolder) - return {'folder': dsFolder, 'warnings': aggregate_warnings, 'job_ids': created_job_ids} + return { + 'folder': dsFolder, + 'warnings': aggregate_warnings, + 'job_ids': created_job_ids, + 'configurationHierarchyWrite': configuration_hierarchy_write, + } def convert_large_image( diff --git a/server/dive_server/views_dataset.py b/server/dive_server/views_dataset.py index e5cf3f7db..13abdf30a 100644 --- a/server/dive_server/views_dataset.py +++ b/server/dive_server/views_dataset.py @@ -1,3 +1,4 @@ +import json from typing import List, Optional import cherrypy @@ -23,6 +24,14 @@ } +def _raw_rest_error(resource: Resource, error: RestException) -> str: + """Return a streaming/download validation error without Girder rewriting its body.""" + resource.setRawResponse() + cherrypy.response.status = error.code + cherrypy.response.headers['Content-Type'] = 'text/plain' + return str(error) + + class DatasetResource(Resource): """RESTful Dataset resource""" @@ -256,14 +265,20 @@ def set_dataset_metadata_file(self, folder, itemId): ) ) def get_configuration(self, folder): - setContentDisposition(f'{folder["name"]}.config.json') # A dataset configuration consists of MetadataMutable properties. expose = MetadataMutable.schema()['properties'].keys() - return crud_dataset.get_dataset(folder, self.getCurrentUser()).json( - exclude_none=True, - include=expose, - indent=2, + try: + hierarchy = crud_dataset.type_hierarchy_for_export(folder, self.getCurrentUser()) + except RestException as error: + return _raw_rest_error(self, error) + setContentDisposition(f'{folder["name"]}.config.json') + configuration = crud_dataset.get_dataset(folder, self.getCurrentUser()).dict( + exclude_none=True, include=expose ) + configuration.pop('typeHierarchy', None) + if hierarchy is not None: + configuration['typeHierarchy'] = hierarchy + return json.dumps(configuration, indent=2) @access.user @autoDescribeRoute( @@ -339,14 +354,17 @@ def export( girder_folders.append( Folder().load(folder, level=AccessType.READ, user=self.getCurrentUser()) ) - gen = crud_dataset.export_datasets_zipstream( - girder_folders, - self.getCurrentUser(), - includeMedia=includeMedia, - includeDetections=includeDetections, - excludeBelowThreshold=excludeBelowThreshold, - typeFilter=typeFilter, - ) + try: + gen = crud_dataset.export_datasets_zipstream( + girder_folders, + self.getCurrentUser(), + includeMedia=includeMedia, + includeDetections=includeDetections, + excludeBelowThreshold=excludeBelowThreshold, + typeFilter=typeFilter, + ) + except RestException as error: + return _raw_rest_error(self, error) zip_name = "batch_export.zip" if len(girder_folders) == 1: zip_name = f"{girder_folders[0]['name']}.zip" @@ -374,7 +392,28 @@ def validate_files(self, files): ) ) def patch_metadata(self, folder, data): - return crud_dataset.update_metadata(folder, data) + if 'typeHierarchy' not in data: + return crud_dataset.update_metadata(folder, data) + parent = crud.get_multicam_parent_folder(folder, self.getCurrentUser()) + if parent is None: + if crud.get_multicam_owner_folder(folder) is not None: + raise RestException( + 'Write access to the multicamera parent is required ' + 'to change its type hierarchy.', + code=403, + ) + return crud_dataset.update_metadata(folder, data) + camera_data = dict(data) + hierarchy = camera_data.pop('typeHierarchy') + hierarchy_data = {'typeHierarchy': hierarchy} + crud_dataset.validate_type_hierarchy_update(parent, hierarchy_data) + if camera_data: + crud_dataset.validate_metadata_shape(camera_data) + result = crud_dataset.update_metadata(parent, hierarchy_data) + if camera_data: + crud_dataset.update_metadata(folder, camera_data) + crud_dataset.remove_camera_type_hierarchy(folder) + return result @access.user @autoDescribeRoute( diff --git a/server/dive_tasks/convert_images.py b/server/dive_tasks/convert_images.py index 202dbcfc2..606ad2c7b 100644 --- a/server/dive_tasks/convert_images.py +++ b/server/dive_tasks/convert_images.py @@ -259,7 +259,14 @@ def convert_large_images(self: Task, folderId, user_id: str, user_login: str): @app.task(bind=True, acks_late=True, ignore_result=True) -def extract_zip(self: Task, folderId: str, itemId: str, user_id: str, user_login: str): +def extract_zip( + self: Task, + folderId: str, + itemId: str, + user_id: str, + user_login: str, + additive: bool = False, +): """ Discovery logic: * Find all folders that have at least one child file (potential datasets) @@ -337,32 +344,39 @@ def extract_zip(self: Task, folderId: str, itemId: str, user_id: str, user_login ) > 1 for folderName, folderType in discovered_folders.items(): subFolderName = folderName if make_subfolders else '' - if folderType == 'unstructured': - utils.upload_zipped_flat_media_files( - gc, - manager, - folderId, - _working_directory_path / folderName, - subFolderName, - ) - elif folderType == 'multicam': - utils.upload_exported_multicam_zipped_dataset( - gc, - manager, - folderId, - _working_directory_path / folderName, - subFolderName, - ) - elif folderType == 'dataset': - utils.upload_exported_zipped_dataset( - gc, - manager, - folderId, - _working_directory_path / folderName, - subFolderName, - ) - else: - manager.write(f'Ignoring {folderName}\n') + try: + if folderType == 'unstructured': + utils.upload_zipped_flat_media_files( + gc, + manager, + folderId, + _working_directory_path / folderName, + subFolderName, + additive, + ) + elif folderType == 'multicam': + utils.upload_exported_multicam_zipped_dataset( + gc, + manager, + folderId, + _working_directory_path / folderName, + subFolderName, + additive, + ) + elif folderType == 'dataset': + utils.upload_exported_zipped_dataset( + gc, + manager, + folderId, + _working_directory_path / folderName, + subFolderName, + additive, + ) + else: + manager.write(f'Ignoring {folderName}\n') + except utils.MalformedExportedConfigurationError: + gc.delete(f'item/{itemId}') + raise if make_subfolders: gc.sendRestRequest( diff --git a/server/dive_tasks/utils.py b/server/dive_tasks/utils.py index ff7f84dfc..c962cd571 100644 --- a/server/dive_tasks/utils.py +++ b/server/dive_tasks/utils.py @@ -21,6 +21,11 @@ from girder_worker.utils import JobManager, JobStatus from dive_utils import constants, models, multicam_camera_order +from dive_utils.type_hierarchy import ( + TypeHierarchyError, + apply_hierarchy_write, + resolve_type_hierarchy, +) TIMEOUT_COUNT = 'timeout_count' TIMEOUT_LAST_CHECKED = 'last_checked' @@ -40,6 +45,10 @@ class CanceledError(RuntimeError): pass +class MalformedExportedConfigurationError(RuntimeError): + pass + + def choose_annotation_fps( requested_fps, *, @@ -560,6 +569,7 @@ def upload_zipped_flat_media_files( folderId: str, working_directory: Path, create_subfolder=False, + additive: bool = False, ): """ Takes a flat folder of media files and/or annotation and generates a dataset from it. @@ -596,7 +606,11 @@ def upload_zipped_flat_media_files( {constants.TypeMarker: dataset_type, constants.FPSMarker: default_fps}, ) # After uploading the default files we do a the postprocess for video conversion now - gc.sendRestRequest("POST", f"/dive_rpc/postprocess/{str(root_folderId)}") + gc.sendRestRequest( + "POST", + f"/dive_rpc/postprocess/{str(root_folderId)}", + parameters={'additive': additive}, + ) else: manager.write(f"Message: {validation['message']}\n") manager.write("Please check the documentation for Zip files at:\ @@ -657,7 +671,7 @@ def create_sibling_dataset_from_media( return new_folder_id -def _load_exported_dataset_meta(working_directory: Path) -> dict: +def _exported_dataset_config_path(working_directory: Path) -> Path: list_of_names = os.listdir(working_directory) potential_meta_files = list(filter(constants.metaRegex.match, list_of_names)) if len(potential_meta_files) == 0: @@ -669,7 +683,11 @@ def _load_exported_dataset_meta(working_directory: Path) -> dict: or by_lower.get(constants.LegacyConfigFileName) or potential_meta_files[0] ) - with open(working_directory / chosen) as f: + return working_directory / chosen + + +def _load_exported_dataset_meta(working_directory: Path) -> dict: + with open(_exported_dataset_config_path(working_directory)) as f: return json.load(f) @@ -728,6 +746,8 @@ def _import_exported_dataset_directory( manager: JobManager, dest_folder_id: str, working_directory: Path, + additive: bool = False, + defer_postprocess: bool = False, ) -> None: """Import one exported single-camera dataset directory into dest_folder_id.""" working_directory = Path(working_directory) @@ -756,19 +776,17 @@ def _import_exported_dataset_directory( shutil.rmtree(aux_path) manager.updateStatus(JobStatus.PUSHING_OUTPUT) + config_path = _exported_dataset_config_path(working_directory) for entry in working_directory.iterdir(): if entry.name == 'metadata': continue + if defer_postprocess and entry == config_path: + continue gc.upload(str(entry), dest_folder_id) metadata_item_id = _upload_archive_metadata_attachment(gc, dest_folder_id, metadata_attachment) all_files = list(gc.listItem(dest_folder_id)) root_meta = { 'type': dataset_type, - 'attributes': meta.get('attributes', None), - 'customTypeStyling': meta.get('customTypeStyling', None), - 'customGroupStyling': meta.get('customGroupStyling', None), - 'confidenceFilters': meta.get('confidenceFilters', None), - 'imageEnhancements': meta.get('imageEnhancements', None), 'fps': meta['fps'], 'version': meta['version'], } @@ -807,7 +825,17 @@ def _import_exported_dataset_directory( root_meta[constants.DatasetMarker] = True gc.addMetadataToFolder(dest_folder_id, root_meta) - gc.post(f'dive_rpc/postprocess/{dest_folder_id}', data={'skipJobs': True}) + if not defer_postprocess: + try: + gc.post( + f'dive_rpc/postprocess/{dest_folder_id}', + data={'skipJobs': True, 'additive': additive}, + ) + except Exception as error: + retained = list(gc.listItem(dest_folder_id, name=config_path.name)) + if not retained: + raise MalformedExportedConfigurationError(str(error)) from error + raise def upload_exported_zipped_dataset( @@ -816,12 +844,13 @@ def upload_exported_zipped_dataset( folderId: str, working_directory: Path, create_subfolder='', + additive: bool = False, ): """Uploads a folder that is generated from the export of a zip file and sets metadata.""" working_directory = Path(working_directory) if (working_directory / constants.MultiCamJsonFileName).is_file(): upload_exported_multicam_zipped_dataset( - gc, manager, folderId, working_directory, create_subfolder + gc, manager, folderId, working_directory, create_subfolder, additive ) return try: @@ -833,7 +862,7 @@ def upload_exported_zipped_dataset( reuseExisting=True, ) dest_folder_id = str(sub_folder['_id']) - _import_exported_dataset_directory(gc, manager, dest_folder_id, working_directory) + _import_exported_dataset_directory(gc, manager, dest_folder_id, working_directory, additive) except ValueError as err: manager.write(f'{err}\n') raise Exception(str(err)) from err @@ -875,12 +904,29 @@ def _upload_stereo_calibration_files( return calibration_item_id +def _post_exported_configuration( + gc: GirderClient, + folder_id: str, + configuration: dict, + additive: bool, +) -> None: + with tempfile.TemporaryDirectory() as temp_dir: + configuration_path = Path(temp_dir) / constants.ConfigFileName + configuration_path.write_text(json.dumps(configuration)) + gc.uploadFileToFolder(folder_id, str(configuration_path)) + gc.post( + f'dive_rpc/postprocess/{folder_id}', + data={'skipJobs': True, 'additive': additive}, + ) + + def upload_exported_multicam_zipped_dataset( gc: GirderClient, manager: JobManager, folderId: str, working_directory: Path, create_subfolder='', + additive: bool = False, ): """ Import a multicam dataset produced by dive_dataset export (multiCam.json + per-camera folders). @@ -929,6 +975,7 @@ def upload_exported_multicam_zipped_dataset( imported_cameras: dict = {} media_type = None + camera_directories = {} for camera_name in camera_order: camera_dir = working_directory / camera_name if not camera_dir.is_dir(): @@ -939,12 +986,55 @@ def upload_exported_multicam_zipped_dataset( media_type = cam_type elif cam_type != media_type: raise Exception(f'Camera "{camera_name}" has type {cam_type}, expected {media_type}') + camera_directories[camera_name] = camera_dir + if 'typeHierarchy' in child_meta: + raise MalformedExportedConfigurationError( + f'Camera "{camera_name}" config.json contains typeHierarchy; ' + 'multicamera hierarchy must be stored only in the root config.json' + ) + + existing_hierarchy = (parent_folder.get('meta') or {}).get('typeHierarchy') + try: + hierarchy_write = resolve_type_hierarchy( + existing_hierarchy, + 'typeHierarchy' in parent_meta, + parent_meta.get('typeHierarchy'), + 'additive' if additive else 'overwrite', + ) + except TypeHierarchyError as error: + message = f'Type hierarchy is invalid: {error.reason}. No configuration was changed.' + if error.kind == 'malformed': + raise MalformedExportedConfigurationError(message) from error + raise RuntimeError(message) from error + + for camera_name in camera_order: + camera_dir = camera_directories[camera_name] manager.write(f'Importing camera "{camera_name}"…\n') child_folder = gc.createFolder(parent_folder_id, camera_name, reuseExisting=True) child_id = str(child_folder['_id']) - _import_exported_dataset_directory(gc, manager, child_id, camera_dir) + _import_exported_dataset_directory( + gc, + manager, + child_id, + camera_dir, + additive, + defer_postprocess=True, + ) imported_cameras[camera_name] = {'folderId': child_id} + # Camera configurations post before multicam registration: with no resolvable + # multicam parent yet, shared-mutable keys stay camera-local instead of being + # mirrored onto the parent. The parent post below is the sole parent-config writer. + for camera_name in camera_order: + camera_configuration = _load_exported_dataset_meta(camera_directories[camera_name]) + camera_configuration.pop('typeHierarchy', None) + _post_exported_configuration( + gc, + imported_cameras[camera_name]['folderId'], + camera_configuration, + additive, + ) + calibration_file_id = None if sub_type == 'stereo': calibration_file_id = _upload_stereo_calibration_files( @@ -975,3 +1065,9 @@ def upload_exported_multicam_zipped_dataset( parameters={'parentFolderId': parent_folder_id}, json=create_body, ) + + parent_configuration = apply_hierarchy_write( + {key: value for key, value in parent_meta.items() if key != 'typeHierarchy'}, + hierarchy_write, + ) + _post_exported_configuration(gc, parent_folder_id, parent_configuration, additive) diff --git a/server/dive_utils/models.py b/server/dive_utils/models.py index 6f92198f7..b3eae43e7 100644 --- a/server/dive_utils/models.py +++ b/server/dive_utils/models.py @@ -2,7 +2,7 @@ from typing import Any, Dict, List, Optional, Set, Tuple, Union from bson.objectid import ObjectId -from pydantic import BaseModel, Field, validator +from pydantic import BaseModel, Field, StrictStr, validator from typing_extensions import Literal from dive_utils import constants, types @@ -240,6 +240,7 @@ class PairHomography(BaseModel): CameraTransformType = Literal['translation', 'rigid', 'similarity', 'affine', 'homography'] +TypeHierarchy = Dict[StrictStr, StrictStr] class MetadataMutable(BaseModel): @@ -254,6 +255,7 @@ class MetadataMutable(BaseModel): attributes: Optional[Dict[str, Attribute]] attributeTrackFilters: Optional[Dict[str, AttributeTrackFilter]] datasetInfo: Optional[types.DatasetInfo] + typeHierarchy: Optional[TypeHierarchy] = None # Per-camera-pair alignment homographies, keyed by directional "left::right". # Each value holds the 3x3 AtoB / BtoA matrices. cameraHomographies: Optional[Dict[str, PairHomography]] @@ -282,7 +284,7 @@ def is_dive_configuration(value: dict): # the value is actually a configuration object. keys.remove("version") - return any([value.get(key, False) for key in keys]) + return 'typeHierarchy' in value or any([value.get(key, False) for key in keys]) class MediaResource(BaseModel): @@ -324,6 +326,8 @@ class MultiCamMedia(BaseModel): class GirderMetadataStatic(MetadataMutable): + # Reads preserve legacy malformed storage so the viewer can report and repair it. + typeHierarchy: Optional[Any] = None # Required id: str name: str diff --git a/server/dive_utils/type_hierarchy.py b/server/dive_utils/type_hierarchy.py index 8bd8175ec..f45784fcd 100644 --- a/server/dive_utils/type_hierarchy.py +++ b/server/dive_utils/type_hierarchy.py @@ -117,3 +117,13 @@ def resolve_type_hierarchy( except TypeHierarchyError as error: raise _conflict(error.reason) from error return {'action': 'set', 'hierarchy': normalized_merged or {}} + + +def apply_hierarchy_write(payload: dict, hierarchy_write: HierarchyWrite) -> dict: + """Return ``payload`` with the resolved typeHierarchy applied, leaving it alone on 'none'.""" + resolved = dict(payload) + if hierarchy_write['action'] == 'set': + resolved['typeHierarchy'] = hierarchy_write['hierarchy'] + elif hierarchy_write['action'] == 'delete': + resolved['typeHierarchy'] = None + return resolved diff --git a/server/tests/test_create_multicam.py b/server/tests/test_create_multicam.py index eea90b43f..f72394fac 100644 --- a/server/tests/test_create_multicam.py +++ b/server/tests/test_create_multicam.py @@ -31,6 +31,11 @@ def _dataset_parent(): def test_create_multicam_links_children(_verify, valid_images_mock, folder_cls, _aux): user = {'login': 'tester'} dataset_parent = _dataset_parent() + dataset_parent['meta'] = { + 'typeHierarchy': {'salmon': 'fish'}, + 'customTypeStyling': {'salmon': {'color': '#123456'}}, + 'confidenceFilters': {'default': 0.7, 'salmon': 0.85}, + } left = _child_folder('left-id', 'left') right = _child_folder('right-id', 'right') @@ -67,6 +72,9 @@ def load_folder(folder_id, level=None, user=None): assert saved_meta[constants.SubTypeMarker] == 'stereo' assert saved_meta[constants.MultiCamMarker]['cameraOrder'] == ['left', 'right'] assert set(saved_meta[constants.MultiCamMarker]['cameras'].keys()) == {'left', 'right'} + assert saved_meta['typeHierarchy'] == {'salmon': 'fish'} + assert saved_meta['customTypeStyling'] == {'salmon': {'color': '#123456'}} + assert saved_meta['confidenceFilters'] == {'default': 0.7, 'salmon': 0.85} @patch('dive_server.crud_dataset.Item') diff --git a/server/tests/test_hierarchy_ingestion_routes.py b/server/tests/test_hierarchy_ingestion_routes.py new file mode 100644 index 000000000..286f3e981 --- /dev/null +++ b/server/tests/test_hierarchy_ingestion_routes.py @@ -0,0 +1,362 @@ +import copy +from unittest.mock import MagicMock + +from girder.exceptions import RestException +import pytest + +from dive_server import crud_rpc +from dive_utils import constants +from dive_utils.type_hierarchy import TypeHierarchyError + + +def test_recognized_malformed_configuration_is_retained_for_correction_and_retry(monkeypatch): + item = {'_id': 'config-item'} + file = {'name': 'config.json'} + item_model = MagicMock() + monkeypatch.setattr(crud_rpc, 'Item', lambda: item_model) + parsed = { + 'annotations': None, + 'meta': {'typeHierarchy': {'salmon': 'fish'}}, + 'attributes': None, + 'type': crud_rpc.crud.FileType.DIVE_CONF, + } + get_data = MagicMock(side_effect=[TypeHierarchyError('expected an object'), (parsed, [])]) + monkeypatch.setattr(crud_rpc, '_get_data_by_type', get_data) + + with pytest.raises(RestException, match='Type hierarchy is invalid: expected an object'): + crud_rpc._parse_data_item(item, file, configuration_only=True) + item_model.remove.assert_not_called() + + result, warnings = crud_rpc._parse_data_item(item, file, configuration_only=True) + assert result == parsed + assert warnings == [] + item_model.remove.assert_not_called() + + +def test_camera_configuration_hierarchy_requires_parent_write_access(monkeypatch): + camera = {'_id': 'camera', 'parentId': 'parent', 'meta': {}} + owner = {'_id': 'parent', 'meta': {'type': constants.MultiType}} + item = {'_id': 'config-item'} + file = {'name': 'config.json', 'exts': ['json']} + parsed = { + 'annotations': None, + 'meta': {'typeHierarchy': {'salmon': 'fish'}}, + 'attributes': None, + 'type': crud_rpc.crud.FileType.DIVE_CONF, + } + item_model = MagicMock() + item_model.childFiles.return_value = iter([file]) + monkeypatch.setattr(crud_rpc, 'Item', lambda: item_model) + monkeypatch.setattr(crud_rpc, '_fresh_folder_snapshot', lambda target: target) + monkeypatch.setattr(crud_rpc, '_unprocessed_data_items', lambda _folder: [item]) + monkeypatch.setattr( + crud_rpc, + '_declared_sidecar_predicate', + lambda *_args: (None, lambda _: False), + ) + monkeypatch.setattr(crud_rpc, '_parse_data_item', lambda *_args, **_kwargs: (parsed, [])) + monkeypatch.setattr(crud_rpc.crud, 'get_multicam_parent_folder', lambda *_args: None) + monkeypatch.setattr(crud_rpc.crud, 'get_multicam_owner_folder', lambda _folder: owner) + + with pytest.raises( + RestException, + match='Write access to the multicamera parent is required', + ) as error: + crud_rpc._prepare_configuration_imports(camera, {'_id': 'user'}, additive=False) + + assert error.value.code == 403 + + +@pytest.mark.parametrize( + ('additive', 'incoming', 'expected'), + [ + (False, {'salmon': 'fish'}, {'salmon': 'fish'}), + (True, {'salmon': 'fish'}, {'fish': 'animal', 'salmon': 'fish'}), + (False, None, None), + ], +) +def test_single_dataset_configuration_write_matrix(monkeypatch, additive, incoming, expected): + folder = { + '_id': 'dataset', + 'meta': {'typeHierarchy': {'fish': 'animal'}}, + } + writes = [] + + def update_metadata(target, payload, _verify, hierarchy_mode='save'): + writes.append((target, copy.deepcopy(payload), hierarchy_mode)) + if payload.get('typeHierarchy') is None: + target['meta'].pop('typeHierarchy', None) + elif 'typeHierarchy' in payload: + target['meta']['typeHierarchy'] = copy.deepcopy(payload['typeHierarchy']) + return target['meta'] + + monkeypatch.setattr(crud_rpc, '_fresh_folder_snapshot', lambda target: target) + monkeypatch.setattr(crud_rpc.crud_dataset, 'update_metadata', update_metadata) + plan = { + 'parent': None, + 'hierarchy_instructions': [(True, incoming)], + 'additive': additive, + 'staged_meta': {}, + 'staged_parent_meta': {}, + 'applied': False, + } + + crud_rpc._apply_configuration_imports(folder, plan) + + assert folder['meta'].get('typeHierarchy') == expected + assert writes[0][1].get('typeHierarchy') == expected + + +def test_camera_configuration_updates_only_parent_hierarchy(monkeypatch): + parent = { + '_id': 'parent', + 'meta': { + 'type': constants.MultiType, + 'typeHierarchy': {'fish': 'animal'}, + }, + } + camera = { + '_id': 'camera', + 'name': 'left', + 'meta': {'typeHierarchy': {'legacy': 'copy'}}, + } + writes = [] + + def update_metadata(target, payload, _verify, hierarchy_mode='save'): + writes.append((target['_id'], copy.deepcopy(payload), hierarchy_mode)) + target['meta'].update(copy.deepcopy(payload)) + return target['meta'] + + def remove_copy(target): + return target['meta'].pop('typeHierarchy', None) is not None + + monkeypatch.setattr(crud_rpc, '_fresh_folder_snapshot', lambda target: target) + monkeypatch.setattr(crud_rpc.crud_dataset, 'update_metadata', update_metadata) + monkeypatch.setattr(crud_rpc.crud_dataset, 'remove_camera_type_hierarchy', remove_copy) + plan = { + 'parent': parent, + 'hierarchy_instructions': [(True, {'salmon': 'fish'})], + 'additive': True, + 'staged_meta': {'imageEnhancements': {'brightness': 1.1}}, + 'staged_parent_meta': {'confidenceFilters': {'default': 0.4}}, + 'applied': False, + } + + crud_rpc._apply_configuration_imports(camera, plan) + + assert writes == [ + ('camera', {'imageEnhancements': {'brightness': 1.1}}, 'additive'), + ( + 'parent', + { + 'confidenceFilters': {'default': 0.4}, + 'typeHierarchy': { + 'fish': 'animal', + 'legacy': 'copy', + 'salmon': 'fish', + }, + }, + 'additive', + ), + ] + assert 'typeHierarchy' not in camera['meta'] + assert parent['meta']['typeHierarchy'] == { + 'fish': 'animal', + 'legacy': 'copy', + 'salmon': 'fish', + } + assert plan['warnings'] == [ + 'Removed a type hierarchy stored on camera left; ' + 'the type hierarchy for a multicamera dataset is stored on the parent.' + ] + + +def test_camera_hierarchy_removal_warning_reaches_import_warnings(monkeypatch): + monkeypatch.setattr( + crud_rpc.crud_dataset, + 'resolve_metadata_attachment_item_id', + lambda _folder, _user: None, + ) + message = ( + 'Removed a type hierarchy stored on camera left; ' + 'the type hierarchy for a multicamera dataset is stored on the parent.' + ) + plan = { + 'parent': None, + 'unprocessed_items': [], + 'item_files': {}, + 'parsed_json_items': {}, + 'applied': True, + 'hierarchy_write': {'action': 'none'}, + 'warnings': [message], + } + + warnings = crud_rpc.process_items( + {'_id': 'camera', 'meta': {}}, {'_id': 'user-id'}, configuration_plan=plan + ) + + assert warnings == [message] + + +def test_camera_without_stored_hierarchy_import_warns_nothing(monkeypatch): + parent = {'_id': 'parent', 'meta': {'type': constants.MultiType}} + camera = {'_id': 'camera', 'name': 'left', 'meta': {}} + monkeypatch.setattr(crud_rpc, '_fresh_folder_snapshot', lambda target: target) + monkeypatch.setattr(crud_rpc.crud_dataset, 'update_metadata', MagicMock()) + monkeypatch.setattr( + crud_rpc.crud_dataset, + 'remove_camera_type_hierarchy', + lambda target: target['meta'].pop('typeHierarchy', None) is not None, + ) + plan = { + 'parent': parent, + 'hierarchy_instructions': [], + 'additive': False, + 'staged_meta': {}, + 'staged_parent_meta': {}, + 'applied': False, + } + + crud_rpc._apply_configuration_imports(camera, plan) + + assert 'warnings' not in plan + + +def test_camera_configuration_conflict_is_rejected_before_writes(monkeypatch): + parent = {'_id': 'parent', 'meta': {'typeHierarchy': {'salmon': 'fish'}}} + camera = {'_id': 'camera', 'meta': {}} + update_metadata = MagicMock() + monkeypatch.setattr(crud_rpc, '_fresh_folder_snapshot', lambda target: target) + monkeypatch.setattr(crud_rpc.crud_dataset, 'update_metadata', update_metadata) + plan = { + 'parent': parent, + 'hierarchy_instructions': [(True, {'salmon': 'mammal'})], + 'additive': True, + 'staged_meta': {}, + 'staged_parent_meta': {}, + 'applied': False, + } + + with pytest.raises(RestException, match='conflicting parents for "salmon"'): + crud_rpc._apply_configuration_imports(camera, plan) + update_metadata.assert_not_called() + + +def test_camera_stored_hierarchy_is_promoted_without_new_configuration(monkeypatch): + parent = {'_id': 'parent', 'meta': {'type': constants.MultiType}} + camera = { + '_id': 'camera', + 'name': 'left', + 'meta': {'typeHierarchy': {'salmon': 'fish'}}, + } + + def update_metadata(target, payload, _verify, hierarchy_mode='save'): + target['meta'].update(copy.deepcopy(payload)) + return target['meta'] + + monkeypatch.setattr(crud_rpc, '_fresh_folder_snapshot', lambda target: target) + monkeypatch.setattr(crud_rpc.crud_dataset, 'update_metadata', update_metadata) + monkeypatch.setattr( + crud_rpc.crud_dataset, + 'remove_camera_type_hierarchy', + lambda target: target['meta'].pop('typeHierarchy', None) is not None, + ) + plan = { + 'parent': parent, + 'hierarchy_instructions': [], + 'additive': False, + 'staged_meta': {}, + 'staged_parent_meta': {}, + 'applied': False, + } + + crud_rpc._apply_configuration_imports(camera, plan) + + assert parent['meta']['typeHierarchy'] == {'salmon': 'fish'} + assert 'typeHierarchy' not in camera['meta'] + + +def test_conflicting_camera_hierarchy_is_skipped_before_incoming_configuration(monkeypatch): + parent = {'_id': 'parent', 'meta': {'typeHierarchy': {'salmon': 'fish'}}} + camera = { + '_id': 'camera', + 'name': 'left', + 'meta': {'typeHierarchy': {'salmon': 'mammal'}}, + } + + def update_metadata(target, payload, _verify, hierarchy_mode='save'): + target['meta'].update(copy.deepcopy(payload)) + return target['meta'] + + monkeypatch.setattr(crud_rpc, '_fresh_folder_snapshot', lambda target: target) + monkeypatch.setattr(crud_rpc.crud_dataset, 'update_metadata', update_metadata) + monkeypatch.setattr( + crud_rpc.crud_dataset, + 'remove_camera_type_hierarchy', + lambda target: target['meta'].pop('typeHierarchy', None) is not None, + ) + plan = { + 'parent': parent, + 'hierarchy_instructions': [(True, {'shark': 'fish'})], + 'additive': True, + 'staged_meta': {}, + 'staged_parent_meta': {}, + 'applied': False, + } + + crud_rpc._apply_configuration_imports(camera, plan) + + assert parent['meta']['typeHierarchy'] == {'salmon': 'fish', 'shark': 'fish'} + assert plan['warnings'][0] == ( + 'Camera "left" type hierarchy was skipped: ' + 'conflicting parents for "salmon": "fish" and "mammal"' + ) + + +def test_failed_parent_save_leaves_camera_hierarchy_for_retry(monkeypatch): + parent = {'_id': 'parent', 'meta': {}} + camera = { + '_id': 'camera', + 'name': 'left', + 'meta': {'typeHierarchy': {'salmon': 'fish'}}, + } + monkeypatch.setattr(crud_rpc, '_fresh_folder_snapshot', lambda target: target) + monkeypatch.setattr( + crud_rpc.crud_dataset, + 'update_metadata', + MagicMock(side_effect=RuntimeError('parent save failed')), + ) + remove_copy = MagicMock() + monkeypatch.setattr(crud_rpc.crud_dataset, 'remove_camera_type_hierarchy', remove_copy) + plan = { + 'parent': parent, + 'hierarchy_instructions': [], + 'additive': False, + 'staged_meta': {}, + 'staged_parent_meta': {}, + 'applied': False, + } + + with pytest.raises(RuntimeError, match='parent save failed'): + crud_rpc._apply_configuration_imports(camera, plan) + + assert camera['meta']['typeHierarchy'] == {'salmon': 'fish'} + remove_copy.assert_not_called() + + +def test_postprocess_delegates_without_private_preflight_protocol(monkeypatch): + expected = {'folder': {'_id': 'dataset'}, 'job_ids': []} + postprocess = MagicMock(return_value=expected) + monkeypatch.setattr(crud_rpc, '_postprocess', postprocess) + + result = crud_rpc.postprocess( + {'_id': 'user'}, + {'_id': 'dataset'}, + True, + additive=True, + ) + + assert result == expected + postprocess.assert_called_once_with( + {'_id': 'user'}, {'_id': 'dataset'}, True, False, True, '', '' + ) diff --git a/server/tests/test_multicam_export_clone.py b/server/tests/test_multicam_export_clone.py index 70a0dd6bb..6c644a4ee 100644 --- a/server/tests/test_multicam_export_clone.py +++ b/server/tests/test_multicam_export_clone.py @@ -2,9 +2,11 @@ import json from unittest.mock import MagicMock, patch -from girder.exceptions import AccessException +from girder.exceptions import AccessException, RestException +import pytest -from dive_server import crud_dataset +from dive_server import crud, crud_dataset +from dive_server.crud_rpc import _get_data_by_type from dive_utils import constants @@ -73,6 +75,7 @@ def test_create_single_camera_soft_clone_copies_metadata( 'annotate': True, 'type': 'image-sequence', 'custom': {'labels': ['fish'], 'settings': {'enabled': True}}, + 'typeHierarchy': {'salmon': 'fish'}, }, } parent = {'_id': 'dest-parent'} @@ -85,17 +88,74 @@ def test_create_single_camera_soft_clone_copies_metadata( assert result is cloned_folder source['meta']['custom']['labels'].append('shark') source['meta']['custom']['settings']['enabled'] = False + source['meta']['typeHierarchy']['salmon'] = 'animal' assert cloned_folder['meta']['custom'] == { 'labels': ['fish'], 'settings': {'enabled': True}, } + assert cloned_folder['meta']['typeHierarchy'] == {'salmon': 'fish'} cloned_folder['meta']['custom']['labels'].append('ray') cloned_folder['meta']['custom']['settings']['enabled'] = None + cloned_folder['meta']['typeHierarchy']['tuna'] = 'fish' assert source['meta']['custom'] == { 'labels': ['fish', 'shark'], 'settings': {'enabled': False}, } + assert source['meta']['typeHierarchy'] == {'salmon': 'animal'} + + +@patch('dive_server.crud_dataset._clone_calibration_items') +@patch('dive_server.crud_dataset.crud_annotation.clone_annotations') +@patch('dive_server.crud_dataset.crud.get_or_create_auxiliary_folder') +@patch('dive_server.crud_dataset.crud.getCloneRoot') +@patch('dive_server.crud_dataset.Folder') +def test_create_multicam_soft_clone_preserves_only_parent_hierarchy( + folder_cls, + get_clone_root_mock, + _aux, + _clone_ann, + clone_calibration_mock, +): + owner = {'login': 'tester'} + source = _multi_parent_folder() + source['meta']['typeHierarchy'] = {'salmon': 'fish'} + left = _child_folder('left-id', 'left') + left['meta']['typeHierarchy'] = {'left salmon': 'fish'} + right = _child_folder('right-id', 'right') + right['meta']['typeHierarchy'] = {'right salmon': 'fish'} + destination = {'_id': 'dest-parent'} + cloned_parent = {'_id': 'clone-parent-id', 'name': 'Clone stereo'} + cloned_left = {'_id': 'clone-left-id', 'name': 'left'} + cloned_right = {'_id': 'clone-right-id', 'name': 'right'} + folder_cls.return_value.createFolder.side_effect = [ + cloned_parent, + cloned_left, + cloned_right, + ] + folder_cls.return_value.load.side_effect = lambda folder_id, **_kwargs: { + 'left-id': left, + 'right-id': right, + }[folder_id] + get_clone_root_mock.side_effect = lambda _owner, source_folder: source_folder + clone_calibration_mock.side_effect = lambda _owner, _source, _cloned, multi_cam: multi_cam + + result = crud_dataset.createSoftClone(owner, source, destination, 'Clone stereo', None) + + assert result is cloned_parent + assert cloned_parent['meta']['typeHierarchy'] == {'salmon': 'fish'} + assert 'typeHierarchy' not in cloned_left['meta'] + assert 'typeHierarchy' not in cloned_right['meta'] + cameras = cloned_parent['meta'][constants.MultiCamMarker]['cameras'] + assert cameras['left']['folderId'] == 'clone-left-id' + assert cameras['right']['folderId'] == 'clone-right-id' + + source['meta']['typeHierarchy']['salmon'] = 'animal' + left['meta']['typeHierarchy']['left tuna'] = 'fish' + right['meta']['typeHierarchy']['right salmon'] = 'animal' + assert cloned_parent['meta']['typeHierarchy'] == {'salmon': 'fish'} + assert 'typeHierarchy' not in cloned_left['meta'] + assert 'typeHierarchy' not in cloned_right['meta'] @patch('dive_server.crud_dataset.find_json_calibration_item_id', return_value=None) @@ -316,6 +376,7 @@ def add_file_side_effect(_maker, path): call for call in yield_single_mock.call_args_list if call.args[1] == './stereo-dataset/' ) assert parent_call.args[4] is True + assert folder_cls.return_value.load.call_count == 2 @patch('dive_server.crud_dataset.crud.getCloneRoot') @@ -464,8 +525,11 @@ def test_export_multicam_integration_zip_paths( parent = _multi_parent_folder() parent['meta'][constants.MetadataFileItemIdMarker] = 'metadata-id' parent['meta'][constants.MetadataFileOriginalNameMarker] = 'flight_log.csv' + parent['meta']['typeHierarchy'] = {'salmon': 'fish'} left = _child_folder('left-id', 'left') + left['meta']['typeHierarchy'] = {'left salmon': 'fish'} right = _child_folder('right-id', 'right') + right['meta']['typeHierarchy'] = {} user = {'login': 'tester'} zip_entries = {} @@ -485,14 +549,20 @@ def footer(self): z = RecordingZip() zip_gen_cls.return_value = z - get_dataset_mock.return_value = MagicMock( - dict=lambda exclude_none=True: { - 'id': 'parent-id', - 'type': constants.MultiType, - constants.MetadataFileItemIdMarker: 'metadata-id', - constants.MetadataFileOriginalNameMarker: 'flight_log.csv', - } - ) + def get_dataset(folder, _user): + data = {'id': folder['_id'], 'type': folder['meta']['type']} + if 'typeHierarchy' in folder['meta']: + data['typeHierarchy'] = folder['meta']['typeHierarchy'] + if constants.MetadataFileItemIdMarker in folder['meta']: + data[constants.MetadataFileItemIdMarker] = folder['meta'][ + constants.MetadataFileItemIdMarker + ] + data[constants.MetadataFileOriginalNameMarker] = folder['meta'][ + constants.MetadataFileOriginalNameMarker + ] + return MagicMock(dict=lambda exclude_none=True: data) + + get_dataset_mock.side_effect = get_dataset get_media_mock.return_value = MagicMock( dict=lambda exclude_none=True: {'imageData': [], 'video': None} ) @@ -538,13 +608,82 @@ def child_items(folder, filters=None, **kwargs): assert 'stereo-dataset/right/config.json' in zip_entries multi_cam = json.loads(zip_entries['stereo-dataset/multiCam.json'].decode()) assert multi_cam['defaultDisplay'] == 'left' - parent_meta = json.loads(zip_entries['stereo-dataset/config.json'].decode()) + parent_config = json.loads(zip_entries['stereo-dataset/config.json'].decode()) # The archive carries no attachment locator at all -- neither the server-local item id # nor the name -- because it is discovered at metadata/. Same key set the # desktop exporter writes (withoutMetadataAttachment in multicamExport.ts). - assert constants.MetadataFileItemIdMarker not in parent_meta - assert constants.MetadataFileOriginalNameMarker not in parent_meta + assert constants.MetadataFileItemIdMarker not in parent_config + assert constants.MetadataFileOriginalNameMarker not in parent_config assert 'stereo-dataset/metadata/flight_log.csv' in zip_entries + left_config = json.loads(zip_entries['stereo-dataset/left/config.json'].decode()) + right_config = json.loads(zip_entries['stereo-dataset/right/config.json'].decode()) + assert parent_config['typeHierarchy'] == {'salmon': 'fish'} + assert 'typeHierarchy' not in left_config + assert 'typeHierarchy' not in right_config + + with patch('dive_server.crud_rpc.File') as file_cls: + file_cls.return_value.download.return_value = lambda: [ + zip_entries['stereo-dataset/config.json'] + ] + imported, warnings = _get_data_by_type( + {'_id': 'file-id', 'name': 'config.json', 'exts': ['json']} + ) + assert warnings is None + assert imported['type'] == crud.FileType.DIVE_CONF + assert imported['meta']['typeHierarchy'] == {'salmon': 'fish'} + + +@patch('dive_server.crud_dataset.ziputil.ZipGenerator') +def test_export_zip_preflights_invalid_hierarchy_before_archive_header(zip_gen_cls): + folder = { + '_id': 'dataset-id', + 'name': 'dataset', + 'meta': { + 'annotate': True, + 'type': constants.VideoType, + 'fps': 5, + 'typeHierarchy': {'fish': 'fish'}, + }, + } + + with pytest.raises(RestException) as error_info: + crud_dataset.export_datasets_zipstream( + [folder], + {'_id': 'user-id'}, + includeMedia=True, + includeDetections=True, + excludeBelowThreshold=False, + typeFilter=None, + ) + + assert str(error_info.value) == ( + 'Type hierarchy is invalid: self edge "fish -> fish". ' + 'No configuration file was exported.' + ) + zip_gen_cls.assert_not_called() + + folder['meta']['typeHierarchy'] = {'salmon': 'fish'} + zip_gen_cls.return_value.footer.return_value = b'footer' + with ( + patch('dive_server.crud_dataset.get_media') as get_media_mock, + patch( + 'dive_server.crud_dataset._yield_single_dataset_export', + return_value=iter([b'config-entry']), + ), + ): + get_media_mock.return_value = MagicMock() + retry = crud_dataset.export_datasets_zipstream( + [folder], + {'_id': 'user-id'}, + includeMedia=True, + includeDetections=True, + excludeBelowThreshold=False, + typeFilter=None, + ) + chunks = list(retry()) + + assert chunks == [b'config-entry', b'footer'] + zip_gen_cls.assert_called_once() @patch('dive_server.crud_dataset.Folder') diff --git a/server/tests/test_multicam_zip_import.py b/server/tests/test_multicam_zip_import.py index bb07d7164..d822598e1 100644 --- a/server/tests/test_multicam_zip_import.py +++ b/server/tests/test_multicam_zip_import.py @@ -5,15 +5,14 @@ import pytest -# dive_tasks package __init__ imports girder_worker; stub it for unit tests. if 'girder_worker' not in sys.modules: - _gw = MagicMock() - sys.modules['girder_worker'] = _gw - sys.modules['girder_worker.task'] = _gw.task - sys.modules['girder_worker.utils'] = _gw.utils + worker = MagicMock() + sys.modules['girder_worker'] = worker + sys.modules['girder_worker.task'] = worker.task + sys.modules['girder_worker.utils'] = worker.utils from dive_tasks import utils # noqa: E402 -from dive_utils import constants +from dive_utils import constants # noqa: E402 def _write_image_sequence_export( @@ -22,6 +21,7 @@ def _write_image_sequence_export( fps: float = 5.0, metadata_name: str | None = None, extra_meta: dict | None = None, + hierarchy=None, ): """Write an exported image-sequence dataset directory. @@ -37,6 +37,8 @@ def _write_image_sequence_export( 'imageData': [{'filename': name} for name in images], **(extra_meta or {}), } + if hierarchy is not None: + meta['typeHierarchy'] = hierarchy if metadata_name: metadata_dir = target / 'metadata' metadata_dir.mkdir() @@ -50,11 +52,14 @@ def _write_multicam_export_tree( sub_type: str = 'stereo', with_calibration: bool = True, with_metadata: bool = False, + root_hierarchy='missing', + camera_hierarchy=None, ): _write_image_sequence_export( root / 'left', ['frame0.png'], metadata_name='left.csv' if with_metadata else None, + hierarchy=camera_hierarchy, ) _write_image_sequence_export(root / 'right', ['frame0.png']) multi_cam = { @@ -72,7 +77,10 @@ def _write_multicam_export_tree( 'fps': 5.0, 'version': 1, 'name': 'stereo-import', + 'confidenceFilters': {'default': 0.7}, } + if root_hierarchy != 'missing': + parent_meta['typeHierarchy'] = root_hierarchy if with_metadata: metadata_dir = root / 'metadata' metadata_dir.mkdir() @@ -85,7 +93,11 @@ def _write_multicam_export_tree( @pytest.fixture def mock_gc(): gc = MagicMock() - gc.getFolder.return_value = {'_id': 'parent-id', 'name': 'stereo-import'} + gc.getFolder.return_value = { + '_id': 'parent-id', + 'name': 'stereo-import', + 'meta': {'typeHierarchy': {'fish': 'animal'}}, + } gc.createFolder.side_effect = lambda parent_id, name, **kwargs: { '_id': f'{name}-id', 'name': name, @@ -124,30 +136,93 @@ def test_import_exported_dataset_rejects_multicam_root(tmp_path, mock_gc, mock_m utils._import_exported_dataset_directory(mock_gc, mock_manager, 'dest', root) -def test_upload_exported_multicam_imports_cameras_and_finalizes(tmp_path, mock_gc, mock_manager): - root = tmp_path / 'stereo-dataset' - _write_multicam_export_tree(root) +@pytest.mark.parametrize( + ('additive', 'root_hierarchy', 'expected'), + [ + (False, {'salmon': 'fish'}, {'salmon': 'fish'}), + ( + True, + {'salmon': 'fish'}, + {'fish': 'animal', 'salmon': 'fish'}, + ), + ], +) +def test_multicam_root_hierarchy_resolves_before_camera_creation( + tmp_path, mock_gc, additive, root_hierarchy, expected +): + root = tmp_path / 'stereo' + _write_multicam_export_tree(root, root_hierarchy=root_hierarchy) + manager = MagicMock() + uploaded = [] - utils.upload_exported_multicam_zipped_dataset(mock_gc, mock_manager, 'parent-id', root, '') + def capture(folder_id, path): + uploaded.append((folder_id, json.loads(Path(path).read_text()))) + + mock_gc.uploadFileToFolder.side_effect = capture + utils.upload_exported_multicam_zipped_dataset( + mock_gc, manager, 'parent-id', root, additive=additive + ) assert mock_gc.createFolder.call_args_list == [ call('parent-id', 'left', reuseExisting=True), call('parent-id', 'right', reuseExisting=True), ] - assert mock_gc.upload.call_count >= 3 mock_gc.sendRestRequest.assert_called_once() - (_method, path), kwargs = mock_gc.sendRestRequest.call_args - assert _method == 'POST' - assert path == '/dive_dataset/multicam' - assert kwargs['parameters'] == {'parentFolderId': 'parent-id'} - body = kwargs['json'] + (method, path), post_kwargs = mock_gc.sendRestRequest.call_args + assert (method, path) == ('POST', '/dive_dataset/multicam') + assert post_kwargs['parameters'] == {'parentFolderId': 'parent-id'} + body = post_kwargs['json'] assert body['subType'] == 'stereo' assert body['defaultDisplay'] == 'left' assert body['cameras'] == { 'left': {'folderId': 'left-id'}, 'right': {'folderId': 'right-id'}, } + assert body['cameraOrder'] == ['left', 'right'] assert body['calibrationFileId'] == 'cal-item-id' + final_uploads = uploaded[-3:] + assert [target for target, _config in final_uploads] == [ + 'left-id', + 'right-id', + 'parent-id', + ] + assert all('typeHierarchy' not in config for _target, config in final_uploads[:2]) + assert final_uploads[2][1]['typeHierarchy'] == expected + assert all(post.kwargs['data']['additive'] is additive for post in mock_gc.post.call_args_list) + + +@pytest.mark.parametrize( + ('root_hierarchy', 'existing', 'error_type', 'message'), + [ + ( + {'fish': 'fish'}, + {'fish': 'animal'}, + utils.MalformedExportedConfigurationError, + 'self edge', + ), + ( + {'salmon': 'mammal'}, + {'salmon': 'fish'}, + RuntimeError, + 'conflicting parents', + ), + ], +) +def test_multicam_root_hierarchy_rejection_precedes_camera_creation( + tmp_path, mock_gc, root_hierarchy, existing, error_type, message +): + root = tmp_path / 'stereo' + _write_multicam_export_tree(root, root_hierarchy=root_hierarchy) + mock_gc.getFolder.return_value['meta']['typeHierarchy'] = existing + + with pytest.raises(error_type, match=message): + utils.upload_exported_multicam_zipped_dataset( + mock_gc, MagicMock(), 'parent-id', root, additive=True + ) + + mock_gc.createFolder.assert_not_called() + mock_gc.sendRestRequest.assert_not_called() + mock_gc.uploadFileToFolder.assert_not_called() def test_upload_exported_zipped_dataset_redirects_when_multicam_json_present( @@ -163,7 +238,7 @@ def test_upload_exported_zipped_dataset_redirects_when_multicam_json_present( utils.upload_exported_zipped_dataset(mock_gc, mock_manager, 'parent-id', root, '') - multicam_mock.assert_called_once_with(mock_gc, mock_manager, 'parent-id', root, '') + multicam_mock.assert_called_once_with(mock_gc, mock_manager, 'parent-id', root, '', False) def _list_items_by_name(folder_id, name=None): @@ -363,3 +438,36 @@ def test_upload_exported_multicam_restores_shared_and_camera_metadata( if call_args.args[0] == 'right-id' ) assert constants.MetadataFileItemIdMarker not in right_meta + + +def test_multicam_camera_hierarchy_is_rejected_before_camera_creation(tmp_path, mock_gc): + root = tmp_path / 'stereo' + _write_multicam_export_tree( + root, + root_hierarchy={'fish': 'animal'}, + camera_hierarchy={'salmon': 'fish'}, + ) + + with pytest.raises( + utils.MalformedExportedConfigurationError, + match='Camera "left" config.json contains typeHierarchy', + ): + utils.upload_exported_multicam_zipped_dataset(mock_gc, MagicMock(), 'parent-id', root) + + mock_gc.createFolder.assert_not_called() + + +def test_missing_root_hierarchy_does_not_project_invalid_existing_storage(tmp_path, mock_gc): + root = tmp_path / 'stereo' + _write_multicam_export_tree(root) + mock_gc.getFolder.return_value['meta']['typeHierarchy'] = ['legacy-invalid'] + uploaded = [] + mock_gc.uploadFileToFolder.side_effect = lambda folder_id, path: uploaded.append( + (folder_id, json.loads(Path(path).read_text())) + ) + + utils.upload_exported_multicam_zipped_dataset( + mock_gc, MagicMock(), 'parent-id', root, additive=True + ) + + assert 'typeHierarchy' not in uploaded[-1][1] diff --git a/server/tests/test_update_metadata.py b/server/tests/test_update_metadata.py index 8c8e166f0..4470406d6 100644 --- a/server/tests/test_update_metadata.py +++ b/server/tests/test_update_metadata.py @@ -1,10 +1,13 @@ import json from unittest.mock import MagicMock, patch +from girder.exceptions import RestException, ValidationException import pytest from dive_server import crud, crud_dataset -from dive_server.crud_rpc import process_items, resolve_imported_dataset_info +from dive_server.crud_rpc import _get_data_by_type, process_items, resolve_imported_dataset_info +from dive_server.views_dataset import DatasetResource +from dive_utils import constants, models def _stub_folder_load_and_save(folder_cls, folder): @@ -83,6 +86,427 @@ def test_update_metadata_sets_time_filters(_verify, folder_cls, crud_folder_cls) assert folder['meta']['timeFilters'] == [10, 50] +@pytest.mark.parametrize('incoming', [None, {}], ids=['null', 'empty']) +@patch('dive_server.crud.Folder') +@patch('dive_server.crud_dataset.Folder') +@patch('dive_server.crud_dataset.crud.verify_dataset') +def test_update_metadata_sets_and_clears_type_hierarchy( + _verify, folder_cls, crud_folder_cls, incoming +): + folder = { + '_id': 'dataset-id', + 'meta': { + 'annotate': True, + 'type': 'video', + 'typeHierarchy': {'salmon': 'fish'}, + }, + } + _stub_folder_load_and_save(folder_cls, folder) + _stub_folder_load_and_save(crud_folder_cls, folder) + + crud_dataset.update_metadata(folder, {'typeHierarchy': incoming}) + + assert 'typeHierarchy' not in folder['meta'] + folder_cls.return_value.save.assert_called_once() + + +@patch('dive_server.crud.Folder') +@patch('dive_server.crud_dataset.Folder') +@patch('dive_server.crud_dataset.crud.verify_dataset') +def test_update_metadata_replaces_complete_type_hierarchy(_verify, folder_cls, crud_folder_cls): + folder = { + '_id': 'dataset-id', + 'meta': { + 'annotate': True, + 'type': 'video', + 'typeHierarchy': {'salmon': 'fish'}, + }, + } + _stub_folder_load_and_save(folder_cls, folder) + _stub_folder_load_and_save(crud_folder_cls, folder) + + crud_dataset.update_metadata(folder, {'typeHierarchy': {'orca': 'mammal'}}) + + assert folder['meta']['typeHierarchy'] == {'orca': 'mammal'} + folder_cls.return_value.save.assert_called_once() + + +@pytest.mark.parametrize('conflict', [False, True], ids=['merge', 'conflict']) +@patch('dive_server.crud.Folder') +@patch('dive_server.crud_dataset.Folder') +@patch('dive_server.crud_dataset.crud.verify_dataset') +def test_update_metadata_additive_hierarchy_uses_final_refresh( + _verify, folder_cls, crud_folder_cls, conflict +): + folder = { + '_id': 'dataset-id', + 'meta': { + 'annotate': True, + 'type': 'video', + 'confidenceFilters': {'default': 0.1}, + 'typeHierarchy': {'salmon': 'fish'}, + }, + } + fresh = { + **folder, + 'meta': { + **folder['meta'], + 'typeHierarchy': { + 'salmon': 'fish', + 'tuna' if conflict else 'shark': 'mammal' if conflict else 'fish', + }, + }, + } + crud_folder_cls.return_value.load.return_value = fresh + folder_cls.return_value.save = MagicMock(side_effect=lambda value: value) + + payload = { + 'confidenceFilters': {'default': 0.9}, + 'typeHierarchy': {'tuna': 'fish'}, + } + if conflict: + with pytest.raises(RestException) as error_info: + crud_dataset.update_metadata( + folder, + payload, + verify=False, + hierarchy_mode='additive', + ) + assert str(error_info.value) == ( + 'Type hierarchy is invalid: conflicting parents for "tuna": ' + '"mammal" and "fish". No configuration was changed.' + ) + assert folder['meta'] == fresh['meta'] + folder_cls.return_value.save.assert_not_called() + else: + crud_dataset.update_metadata( + folder, + payload, + verify=False, + hierarchy_mode='additive', + ) + assert folder['meta']['typeHierarchy'] == { + 'salmon': 'fish', + 'shark': 'fish', + 'tuna': 'fish', + } + assert folder['meta']['confidenceFilters'] == {'default': 0.9} + folder_cls.return_value.save.assert_called_once() + + +@patch('dive_server.crud.Folder') +@patch('dive_server.crud_dataset.Folder') +@patch('dive_server.crud_dataset.crud.verify_dataset') +def test_update_metadata_rejects_invalid_type_hierarchy_without_write( + _verify, folder_cls, crud_folder_cls +): + folder = { + '_id': 'dataset-id', + 'meta': { + 'annotate': True, + 'type': 'video', + 'confidenceFilters': {'default': 0.1}, + 'typeHierarchy': {'salmon': 'fish'}, + }, + } + _stub_folder_load_and_save(folder_cls, folder) + _stub_folder_load_and_save(crud_folder_cls, folder) + + with pytest.raises(RestException) as error_info: + crud_dataset.update_metadata( + folder, + { + 'confidenceFilters': {'default': 0.9}, + 'typeHierarchy': {'fish': 'fish'}, + }, + ) + + assert str(error_info.value) == ( + 'Type hierarchy is invalid: self edge "fish -> fish". ' 'No configuration was changed.' + ) + assert folder['meta'] == { + 'annotate': True, + 'type': 'video', + 'confidenceFilters': {'default': 0.1}, + 'typeHierarchy': {'salmon': 'fish'}, + } + folder_cls.return_value.save.assert_not_called() + + +@pytest.mark.parametrize( + ('payload', 'expected'), + [ + ({'confidenceFilters': {'default': 0.7}}, {'broken': 5}), + ({'typeHierarchy': None}, None), + ({'typeHierarchy': {}}, None), + ({'typeHierarchy': {'salmon': 'fish'}}, {'salmon': 'fish'}), + ], + ids=['missing-preserves', 'null-repairs', 'empty-repairs', 'replacement-repairs'], +) +@patch('dive_server.crud.Folder') +@patch('dive_server.crud_dataset.Folder') +@patch('dive_server.crud_dataset.crud.verify_dataset') +def test_update_metadata_direct_save_matrix_with_invalid_existing_hierarchy( + _verify, folder_cls, crud_folder_cls, payload, expected +): + folder = { + '_id': 'dataset-id', + 'meta': { + 'annotate': True, + 'type': 'video', + 'typeHierarchy': {'broken': 5}, + }, + } + _stub_folder_load_and_save(folder_cls, folder) + _stub_folder_load_and_save(crud_folder_cls, folder) + + crud_dataset.update_metadata(folder, payload) + + if expected is None: + assert 'typeHierarchy' not in folder['meta'] + else: + assert folder['meta']['typeHierarchy'] == expected + folder_cls.return_value.save.assert_called_once() + + +@patch('dive_server.crud.Folder') +@patch('dive_server.crud_dataset.Folder') +@patch('dive_server.crud_dataset.crud.verify_dataset') +def test_update_metadata_invalid_input_does_not_replace_invalid_existing_hierarchy( + _verify, folder_cls, crud_folder_cls +): + folder = { + '_id': 'dataset-id', + 'meta': { + 'annotate': True, + 'type': 'video', + 'confidenceFilters': {'default': 0.1}, + 'typeHierarchy': {'broken': 5}, + }, + } + _stub_folder_load_and_save(folder_cls, folder) + _stub_folder_load_and_save(crud_folder_cls, folder) + + with pytest.raises(RestException) as error_info: + crud_dataset.update_metadata( + folder, + { + 'confidenceFilters': {'default': 0.9}, + 'typeHierarchy': ['not', 'a', 'map'], + }, + ) + + assert str(error_info.value) == ( + 'Type hierarchy is invalid: expected an object. No configuration was changed.' + ) + assert folder['meta']['confidenceFilters'] == {'default': 0.1} + assert folder['meta']['typeHierarchy'] == {'broken': 5} + folder_cls.return_value.save.assert_not_called() + + +@pytest.mark.parametrize('hierarchy', [None, {}], ids=['null', 'empty']) +@patch('dive_server.crud_rpc.File') +def test_get_data_by_type_classifies_presence_only_type_hierarchy_as_config(file_cls, hierarchy): + file = {'_id': 'file-id', 'name': 'config.json', 'exts': ['json']} + file_cls.return_value.download.return_value = lambda: [ + json.dumps({'typeHierarchy': hierarchy}).encode() + ] + + result, warnings = _get_data_by_type(file) + + assert warnings is None + assert result['type'] == crud.FileType.DIVE_CONF + assert result['meta']['typeHierarchy'] is None + + +def test_metadata_mutable_does_not_classify_unrelated_json_as_config(): + assert models.MetadataMutable.is_dive_configuration({'tracks': {}, 'groups': {}}) is False + + +@pytest.mark.parametrize( + 'media_type', + [ + constants.VideoType, + constants.ImageSequenceType, + constants.LargeImageType, + constants.MultiType, + ], +) +@patch('dive_server.crud_dataset.get_multi_cam_media') +@patch('dive_server.crud_dataset.crud.verify_dataset') +def test_get_dataset_loads_type_hierarchy_for_each_media_type( + _verify, get_multi_cam_media, media_type +): + folder = { + '_id': 'dataset-id', + 'name': 'dataset', + 'created': '2025-01-01T00:00:00', + 'meta': { + 'annotate': True, + 'type': media_type, + 'fps': 5, + 'typeHierarchy': {'salmon': 'fish'}, + }, + } + if media_type == constants.MultiType: + folder['meta'].update( + { + 'subType': 'stereo', + 'multiCam': { + 'defaultDisplay': 'left', + 'cameras': {}, + }, + } + ) + get_multi_cam_media.return_value = models.MultiCamMedia( + defaultDisplay='left', cameras={}, cameraOrder=[] + ) + + loaded = crud_dataset.get_dataset(folder, {'_id': 'user-id'}) + + assert loaded.type == media_type + assert loaded.typeHierarchy == {'salmon': 'fish'} + + +def _unwrapped_endpoint(endpoint): + while hasattr(endpoint, '__wrapped__'): + endpoint = endpoint.__wrapped__ + return endpoint + + +def _configuration_endpoint(resource, folder): + return _unwrapped_endpoint(DatasetResource.get_configuration)(resource, folder) + + +@pytest.mark.parametrize( + ('stored', 'expected_present'), + [ + ({'salmon': 'fish'}, True), + ({}, False), + (None, False), + ], + ids=['non-empty', 'empty', 'absent'], +) +@patch('dive_server.views_dataset.setContentDisposition') +@patch('dive_server.crud_dataset.crud.verify_dataset') +def test_configuration_endpoint_includes_only_nonempty_type_hierarchy( + _verify, _set_content_disposition, stored, expected_present +): + folder = { + '_id': 'dataset-id', + 'name': 'dataset', + 'created': '2025-01-01T00:00:00', + 'meta': { + 'annotate': True, + 'type': constants.VideoType, + 'fps': 5, + **({'typeHierarchy': stored} if stored is not None else {}), + }, + } + resource = MagicMock() + resource.getCurrentUser.return_value = {'_id': 'user-id'} + + configuration = json.loads(_configuration_endpoint(resource, folder)) + + assert ('typeHierarchy' in configuration) is expected_present + if expected_present: + assert configuration['typeHierarchy'] == {'salmon': 'fish'} + + +@patch('dive_server.views_dataset.setContentDisposition') +@patch('dive_server.views_dataset.cherrypy.response') +def test_configuration_endpoint_rejects_invalid_stored_hierarchy_before_serialization( + response, + _set_content_disposition, +): + folder = { + '_id': 'dataset-id', + 'name': 'dataset', + 'meta': { + 'typeHierarchy': {'fish': 'fish'}, + }, + } + resource = MagicMock() + resource.getCurrentUser.return_value = {'_id': 'user-id'} + + result = _configuration_endpoint(resource, folder) + + assert result == ( + 'Type hierarchy is invalid: self edge "fish -> fish". ' + 'No configuration file was exported.' + ) + resource.setRawResponse.assert_called_once_with() + assert response.status == 400 + assert response.headers.__setitem__.call_args.args == ('Content-Type', 'text/plain') + _set_content_disposition.assert_not_called() + + +@patch('dive_server.views_dataset.setContentDisposition') +@patch('dive_server.views_dataset.cherrypy.response') +@patch('dive_server.views_dataset.crud_dataset.export_datasets_zipstream') +@patch('dive_server.views_dataset.Folder') +def test_zip_endpoint_returns_exact_invalid_hierarchy_error_before_download( + folder_cls, + export_zipstream, + response, + set_content_disposition, +): + folder_cls.return_value.load.return_value = {'_id': 'dataset-id', 'name': 'dataset'} + expected = ( + 'Type hierarchy is invalid: self edge "fish -> fish". ' + 'No configuration file was exported.' + ) + export_zipstream.side_effect = RestException(expected) + resource = MagicMock() + resource.getCurrentUser.return_value = {'_id': 'user-id'} + + result = _unwrapped_endpoint(DatasetResource.export)( + resource, + ['dataset-id'], + True, + True, + False, + None, + ) + + assert result == expected + resource.setRawResponse.assert_called_once_with() + assert response.status == 400 + assert response.headers.__setitem__.call_args.args == ('Content-Type', 'text/plain') + set_content_disposition.assert_not_called() + + +@patch('dive_server.crud_rpc.File') +@patch('dive_server.views_dataset.setContentDisposition') +@patch('dive_server.crud_dataset.crud.verify_dataset') +def test_configuration_endpoint_export_import_roundtrip( + _verify, _set_content_disposition, file_cls +): + folder = { + '_id': 'dataset-id', + 'name': 'dataset', + 'created': '2025-01-01T00:00:00', + 'meta': { + 'annotate': True, + 'type': constants.VideoType, + 'fps': 5, + 'typeHierarchy': {'salmon': 'fish'}, + }, + } + resource = MagicMock() + resource.getCurrentUser.return_value = {'_id': 'user-id'} + exported = _configuration_endpoint(resource, folder) + file_cls.return_value.download.return_value = lambda: [exported.encode()] + + imported, warnings = _get_data_by_type( + {'_id': 'file-id', 'name': 'dataset.config.json', 'exts': ['json']} + ) + + assert warnings is None + assert imported['type'] == crud.FileType.DIVE_CONF + assert imported['meta']['typeHierarchy'] == {'salmon': 'fish'} + + @patch('dive_server.crud.Folder') @patch('dive_server.crud_dataset.Folder') @patch('dive_server.crud_dataset.crud.verify_dataset') @@ -430,6 +854,7 @@ def test_pick_multicam_shared_mutable_keeps_only_shared_keys(): 'version': 1, 'confidenceFilters': {'default': 0.5}, 'datasetInfo': {'year': '2025'}, + 'typeHierarchy': {'salmon': 'fish'}, 'imageEnhancements': {'brightness': 1.2}, 'cameraHomographies': {'left::right': {'AtoB': [], 'BtoA': []}}, 'cameraCorrespondences': {'left::right': []}, @@ -443,6 +868,7 @@ def test_pick_multicam_shared_mutable_keeps_only_shared_keys(): 'datasetInfo': {'year': '2025'}, 'customTypeStyling': {'fish': {'color': '#0f0'}}, } + assert 'typeHierarchy' not in picked assert 'imageEnhancements' not in picked assert 'cameraHomographies' not in picked assert 'cameraCorrespondences' not in picked @@ -450,6 +876,96 @@ def test_pick_multicam_shared_mutable_keeps_only_shared_keys(): assert 'cameraRegistrationSource' not in picked +def test_camera_patch_rejects_invalid_hierarchy_before_any_write(monkeypatch): + import inspect + from types import SimpleNamespace + + from dive_server.views_dataset import DatasetResource + + camera = {'_id': 'camera', 'meta': {'imageEnhancements': {'brightness': 1}}} + parent = {'_id': 'parent', 'meta': {'typeHierarchy': {'salmon': 'fish'}}} + update_metadata = MagicMock() + monkeypatch.setattr(crud, 'get_multicam_parent_folder', lambda *_args: parent) + monkeypatch.setattr(crud_dataset, 'update_metadata', update_metadata) + resource = SimpleNamespace(getCurrentUser=lambda: {'_id': 'user'}) + + with pytest.raises(RestException, match='self edge "fish -> fish"'): + inspect.unwrap(DatasetResource.patch_metadata)( + resource, + camera, + { + 'imageEnhancements': {'brightness': 2}, + 'typeHierarchy': {'fish': 'fish'}, + }, + ) + + update_metadata.assert_not_called() + assert camera['meta'] == {'imageEnhancements': {'brightness': 1}} + assert parent['meta'] == {'typeHierarchy': {'salmon': 'fish'}} + + +def test_camera_patch_rejects_invalid_camera_metadata_before_parent_write(monkeypatch): + import inspect + from types import SimpleNamespace + + from dive_server.views_dataset import DatasetResource + + camera = {'_id': 'camera', 'meta': {'confidenceFilters': {'default': 0.5}}} + parent = {'_id': 'parent', 'meta': {'typeHierarchy': {'salmon': 'fish'}}} + writes = [] + + def update_metadata(target, data): + validated = crud_dataset.MetadataMutableUpdateArgs(**data) + target['meta'].update(validated.dict(exclude_none=True)) + writes.append(target['_id']) + return target['meta'] + + monkeypatch.setattr(crud, 'get_multicam_parent_folder', lambda *_args: parent) + monkeypatch.setattr(crud_dataset, 'update_metadata', update_metadata) + resource = SimpleNamespace(getCurrentUser=lambda: {'_id': 'user'}) + + with pytest.raises(ValidationException, match='unexpectedField'): + inspect.unwrap(DatasetResource.patch_metadata)( + resource, + camera, + { + 'unexpectedField': True, + 'typeHierarchy': {'tuna': 'fish'}, + }, + ) + + assert writes == [] + assert camera['meta'] == {'confidenceFilters': {'default': 0.5}} + assert parent['meta'] == {'typeHierarchy': {'salmon': 'fish'}} + + +def test_camera_patch_rejects_hierarchy_without_parent_write_access(monkeypatch): + import inspect + from types import SimpleNamespace + + from dive_server.views_dataset import DatasetResource + + camera = {'_id': 'camera', 'meta': {}} + parent = {'_id': 'parent', 'meta': {'typeHierarchy': {'salmon': 'fish'}}} + update_metadata = MagicMock() + monkeypatch.setattr(crud, 'get_multicam_parent_folder', lambda *_args: None) + monkeypatch.setattr(crud, 'get_multicam_owner_folder', lambda _folder: parent) + monkeypatch.setattr(crud_dataset, 'update_metadata', update_metadata) + resource = SimpleNamespace(getCurrentUser=lambda: {'_id': 'user'}) + + with pytest.raises(RestException) as error: + inspect.unwrap(DatasetResource.patch_metadata)( + resource, + camera, + {'typeHierarchy': {'tuna': 'fish'}}, + ) + + assert error.value.code == 403 + update_metadata.assert_not_called() + assert camera['meta'] == {} + assert parent['meta'] == {'typeHierarchy': {'salmon': 'fish'}} + + @patch('dive_server.crud.Folder') def test_get_multicam_parent_folder_returns_parent_for_registered_camera(folder_cls): from girder.constants import AccessType @@ -469,14 +985,38 @@ def test_get_multicam_parent_folder_returns_parent_for_registered_camera(folder_ }, } folder_cls.return_value.load.return_value = parent + folder_cls.return_value.hasAccess.return_value = True user = {'_id': 'user-id'} assert crud.get_multicam_parent_folder(camera, user) is parent - folder_cls.return_value.load.assert_called_once_with( - 'parent-id', - level=AccessType.WRITE, - user=user, - ) + # Membership is resolved without an ACL check so that insufficient access degrades to + # None instead of raising AccessException past the caller's guards. + folder_cls.return_value.load.assert_called_once_with('parent-id', force=True) + folder_cls.return_value.hasAccess.assert_called_once_with(parent, user, AccessType.WRITE) + + +@patch('dive_server.crud.Folder') +def test_get_multicam_parent_folder_returns_none_without_parent_access(folder_cls): + from dive_server import crud + from dive_utils import constants + + camera = {'_id': 'left-id', 'parentId': 'parent-id', 'meta': {'type': 'image-sequence'}} + parent = { + '_id': 'parent-id', + 'meta': { + 'type': constants.MultiType, + 'multiCam': { + 'defaultDisplay': 'left', + 'cameras': {'left': {'folderId': 'left-id', 'type': 'image-sequence'}}, + }, + }, + } + folder_cls.return_value.load.return_value = parent + folder_cls.return_value.hasAccess.return_value = False + + assert crud.get_multicam_parent_folder(camera, {'_id': 'user-id'}) is None + # The read-only owner lookup still resolves, so export keeps the parent's hierarchy. + assert crud.get_multicam_owner_folder(camera) is parent @patch('dive_server.crud.Folder') From 9cb45465443e5e555aafcd99ba736e6f8430f2e5 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Mon, 10 Aug 2026 01:46:17 -0400 Subject: [PATCH 3/6] Select and edit hierarchical track types safely --- client/dive-common/apispec.ts | 1 - client/dive-common/components/BottomPanel.vue | 1 + client/dive-common/components/Sidebar.vue | 3 + .../components/TrackDetailsPanel.spec.ts | 109 +++++ .../components/TrackDetailsPanel.vue | 26 +- .../components/TypeSettingsPanel.spec.ts | 69 +++ .../components/TypeSettingsPanel.vue | 11 + client/dive-common/components/Viewer.vue | 10 +- client/dive-common/use/useModeManager.spec.ts | 52 ++- client/dive-common/use/useModeManager.ts | 23 +- client/dive-common/use/useSave.ts | 25 +- client/src/BaseFilterControls.ts | 41 +- client/src/TrackFilterControls.spec.ts | 396 ++++++++++++++++-- client/src/TrackFilterControls.ts | 199 ++++++--- client/src/components/FilterList.spec.ts | 151 +++++++ client/src/components/LayerManager.spec.ts | 355 ++++++++++++++++ client/src/components/LayerManager.vue | 18 +- client/src/components/Tracks/TrackItem.vue | 4 + .../src/components/Tracks/TrackList.spec.ts | 180 ++++++++ client/src/components/Tracks/TrackList.vue | 24 +- .../bottombar/BottomBarTrackItemView.spec.ts | 69 +++ .../bottombar/BottomBarTrackItemView.vue | 3 +- client/src/components/TypeEditor.spec.ts | 137 ++++++ client/src/components/TypeEditor.vue | 72 +++- .../layers/AnnotationLayers/TailLayer.spec.ts | 30 ++ .../src/layers/AnnotationLayers/TailLayer.ts | 16 +- .../layers/AnnotationLayers/TextLayer.spec.ts | 51 +++ .../src/layers/AnnotationLayers/TextLayer.ts | 5 +- client/src/layers/LayerTypes.ts | 3 + client/src/provides.ts | 1 + client/src/use/useEventChart.spec.ts | 68 +++ client/src/use/useLineChart.spec.ts | 77 ++++ docs/UI-Type-List.md | 14 + 33 files changed, 2069 insertions(+), 175 deletions(-) create mode 100644 client/dive-common/components/TrackDetailsPanel.spec.ts create mode 100644 client/dive-common/components/TypeSettingsPanel.spec.ts create mode 100644 client/src/components/FilterList.spec.ts create mode 100644 client/src/components/LayerManager.spec.ts create mode 100644 client/src/components/Tracks/TrackList.spec.ts create mode 100644 client/src/components/Tracks/bottombar/BottomBarTrackItemView.spec.ts create mode 100644 client/src/components/TypeEditor.spec.ts create mode 100644 client/src/layers/AnnotationLayers/TailLayer.spec.ts create mode 100644 client/src/layers/AnnotationLayers/TextLayer.spec.ts create mode 100644 client/src/use/useEventChart.spec.ts create mode 100644 client/src/use/useLineChart.spec.ts diff --git a/client/dive-common/apispec.ts b/client/dive-common/apispec.ts index d7f722df9..57ba46287 100644 --- a/client/dive-common/apispec.ts +++ b/client/dive-common/apispec.ts @@ -299,7 +299,6 @@ const MulticamSharedMutableKeys = [ 'customGroupStyling', 'attributeTrackFilters', 'datasetInfo', - 'typeHierarchy', ]; interface DatasetConfig extends DatasetConfigMutable { diff --git a/client/dive-common/components/BottomPanel.vue b/client/dive-common/components/BottomPanel.vue index 638b31c02..aaf8bfcba 100644 --- a/client/dive-common/components/BottomPanel.vue +++ b/client/dive-common/components/BottomPanel.vue @@ -178,6 +178,7 @@ export default defineComponent({ diff --git a/client/dive-common/components/Sidebar.vue b/client/dive-common/components/Sidebar.vue index 3e37c0ebb..c8bc2aa2a 100644 --- a/client/dive-common/components/Sidebar.vue +++ b/client/dive-common/components/Sidebar.vue @@ -155,6 +155,7 @@ export default defineComponent({ readOnlyMode, styleManager, disableAnnotationFilters: trackFilterControls.disableAnnotationFilters, + hierarchyActive: trackFilterControls.hierarchyActive, confidenceFilters: trackFilterControls.confidenceFilters, visible, horizontalTabIcon, @@ -194,6 +195,7 @@ export default defineComponent({ @@ -396,6 +398,7 @@ export default defineComponent({ diff --git a/client/dive-common/components/TrackDetailsPanel.spec.ts b/client/dive-common/components/TrackDetailsPanel.spec.ts new file mode 100644 index 000000000..952a2d73a --- /dev/null +++ b/client/dive-common/components/TrackDetailsPanel.spec.ts @@ -0,0 +1,109 @@ +// @vitest-environment jsdom +/// +import { defineComponent, h, ref } from 'vue'; +import { shallowMount } from '@vue/test-utils'; +import Track from 'vue-media-annotator/track'; +import TrackDetailsPanel from './TrackDetailsPanel.vue'; + +const state = vi.hoisted(() => ({ + displayPairIndex: vi.fn(() => 1), + track: null as Track | null, +})); + +vi.mock('vue-media-annotator/provides', () => ({ + useSelectedTrackId: () => ref(1), + useEditingMode: () => ref(false), + useHandler: () => ({ + trackSelectNext: vi.fn(), + trackSplit: vi.fn(), + removeTrack: vi.fn(), + unstageFromMerge: vi.fn(), + setAttribute: vi.fn(), + deleteAttribute: vi.fn(), + removeGroup: vi.fn(), + toggleMerge: vi.fn(), + }), + useTrackFilters: () => ({ + allTypes: ref(['root', 'leaf']), + displayPairIndex: state.displayPairIndex, + }), + useAttributes: () => ref([]), + useMultiSelectList: () => ref([]), + useTime: () => ({ frame: ref(0) }), + useReadOnlyMode: () => ref(false), + useTrackStyleManager: () => ({ + typeStyling: ref({ color: (type: string) => `color:${type}` }), + }), + useEditingGroupId: () => ref(null), + useEditingMultiTrack: () => ref(false), + useGroupFilterControls: () => ({ allTypes: ref([]) }), + useCameraStore: () => ({ + camMap: ref(new Map([['singleCam', { groupStore: undefined }]])), + getAnyTrack: () => state.track, + getAnyPossibleTrack: () => state.track, + setTrackType: vi.fn(), + }), + useSelectedCamera: () => ref('singleCam'), +})); + +/** + * `@vue/test-utils` types a mount target as a Vue 2 constructor, which a `defineComponent` + * SFC is not, so the panel is rendered from a host that captures the real instance. It is + * left unstubbed to keep shallow semantics for its own children. + */ +function mountPanel() { + let child: InstanceType | undefined; + const Host = defineComponent({ + setup: () => () => h(TrackDetailsPanel, { + props: { hotkeysDisabled: false }, + ref: (instance) => { + if (instance && !(instance instanceof Element)) { + child = instance as InstanceType; + } + }, + }), + }); + const wrapper = shallowMount(Host, { stubs: { TrackDetailsPanel: false } }); + if (!child) { + throw new Error('TrackDetailsPanel did not mount'); + } + return { wrapper, vm: child }; +} + +describe('TrackDetailsPanel hierarchy summary', () => { + beforeEach(() => { + state.displayPairIndex.mockReturnValue(1); + state.track = new Track(1, { + confidencePairs: [['root', 0.9], ['leaf', 0.7]], + features: [{ frame: 0, bounds: [0, 0, 1, 1], keyframe: true }], + }); + }); + + it('passes the selected type, confidence index, and color to the summary TrackItem', () => { + const { wrapper } = mountPanel(); + const item = wrapper.findComponent({ name: 'TrackItem' }); + expect(item.exists()).toBe(true); + expect(item.props('trackType')).toBe('leaf'); + expect(item.props('displayPairIndex')).toBe(1); + expect(item.props('color')).toBe('color:leaf'); + }); + + it('falls back to the top pair when no pair passes the filters', () => { + state.displayPairIndex.mockReturnValue(-1); + const { wrapper } = mountPanel(); + const item = wrapper.findComponent({ name: 'TrackItem' }); + expect(item.exists()).toBe(true); + expect(item.props('trackType')).toBe('root'); + expect(item.props('displayPairIndex')).toBe(0); + expect(item.props('lockTypes')).toBe(false); + }); + + it('omits the header only for an empty confidence vector', () => { + state.track = new Track(1, { + confidencePairs: [], + features: [{ frame: 0, bounds: [0, 0, 1, 1], keyframe: true }], + }); + const { wrapper } = mountPanel(); + expect(wrapper.findComponent({ name: 'TrackItem' }).exists()).toBe(false); + }); +}); diff --git a/client/dive-common/components/TrackDetailsPanel.vue b/client/dive-common/components/TrackDetailsPanel.vue index 673a4be62..f5b40bd33 100644 --- a/client/dive-common/components/TrackDetailsPanel.vue +++ b/client/dive-common/components/TrackDetailsPanel.vue @@ -71,7 +71,8 @@ export default defineComponent({ const editingError: Ref = ref(null); const editingModeRef = useEditingMode(); const typeStylingRef = useTrackStyleManager().typeStyling; - const allTypesRef = useTrackFilters().allTypes; + const trackFilters = useTrackFilters(); + const allTypesRef = trackFilters.allTypes; const cameraStore = useCameraStore(); const multiCam = ref(cameraStore.camMap.value.size > 1); const selectedCamera = useSelectedCamera(); @@ -260,6 +261,19 @@ export default defineComponent({ cameraStore.setTrackType(track.id, type, 1, currentType); } + const displayRows = computed(() => selectedTrackList.value.map((track) => { + // trackFilters returns -1 when no confidence pair passes the filters, but this panel + // always shows the selected track, so clamp to pair 0. + const pairIndex = Math.max(trackFilters.displayPairIndex(track, 0), 0); + return { + track, + // Re-run when track confidence pairs change (see AttributesSubsection revision pattern) + revision: track.revision.value, + pairIndex, + pair: track.confidencePairs.length ? track.confidencePairs[pairIndex] : null, + }; + })); + return { selectedTrackIdRef, editingGroupIdRef, @@ -302,6 +316,8 @@ export default defineComponent({ updateSelectedTracksType, setTrackType, displayConfidencePairs, + displayRows, + trackFilters, }; }, }); @@ -423,7 +439,7 @@ export default defineComponent({ class="track-details" >
+import { + defineComponent, h, nextTick, reactive, +} from 'vue'; +import { shallowMount } from '@vue/test-utils'; +import { clientSettings } from 'dive-common/store/settings'; +import TypeSettingsPanel from './TypeSettingsPanel.vue'; + +interface PanelProps { + allTypes: string[]; + hierarchyActive: boolean; +} + +/** + * `@vue/test-utils` types a mount target as a Vue 2 constructor, which a `defineComponent` + * SFC is not, so the panel is rendered from a host and left unstubbed to keep shallow + * semantics for its own children. The host also stands in for `setProps`. + */ +function mountPanel(props: PanelProps) { + const state = reactive(props); + const Host = defineComponent({ + setup: () => () => h(TypeSettingsPanel, { props: state }), + }); + const wrapper = shallowMount(Host, { stubs: { TypeSettingsPanel: false } }); + const setProps = async (next: Partial) => { + Object.assign(state, next); + await nextTick(); + }; + return { wrapper, setProps }; +} + +describe('TypeSettingsPanel hierarchy state', () => { + afterEach(() => { + clientSettings.typeSettings.preventCascadeTypes = false; + }); + + it('disables Prevent Cascade with exact help while preserving its saved value', async () => { + clientSettings.typeSettings.preventCascadeTypes = true; + const { wrapper, setProps } = mountPanel({ allTypes: ['fish'], hierarchyActive: true }); + const preventSwitch = () => wrapper.findAll('v-switch').wrappers.find( + (item) => item.attributes('label') === 'Prevent Cascade Types', + ); + + expect(preventSwitch()?.attributes('disabled')).toBe('true'); + expect(wrapper.text()).toContain( + 'Not applicable to hierarchical types; DIVE selects the deepest qualifying type.', + ); + expect(clientSettings.typeSettings.preventCascadeTypes).toBe(true); + + await setProps({ hierarchyActive: false }); + expect(preventSwitch()?.attributes('disabled')).toBeUndefined(); + expect(wrapper.text()).not.toContain('Not applicable to hierarchical types'); + expect(clientSettings.typeSettings.preventCascadeTypes).toBe(true); + + await setProps({ hierarchyActive: true }); + expect(preventSwitch()?.attributes('disabled')).toBe('true'); + expect(clientSettings.typeSettings.preventCascadeTypes).toBe(true); + }); + + it('leaves the other type settings enabled', () => { + const { wrapper } = mountPanel({ allTypes: [], hierarchyActive: true }); + const switches = wrapper.findAll('v-switch').wrappers; + ['Show Empty', 'Lock Types', 'Filter Types by Frame', 'Show Max Count Button'].forEach( + (label) => expect(switches.find((item) => item.attributes('label') === label) + ?.attributes('disabled')).toBeUndefined(), + ); + }); +}); diff --git a/client/dive-common/components/TypeSettingsPanel.vue b/client/dive-common/components/TypeSettingsPanel.vue index 5c3211bb6..f08eb5fd8 100644 --- a/client/dive-common/components/TypeSettingsPanel.vue +++ b/client/dive-common/components/TypeSettingsPanel.vue @@ -16,6 +16,10 @@ export default defineComponent({ type: Array as PropType>, required: true, }, + hierarchyActive: { + type: Boolean, + required: true, + }, }, setup(props, { emit }) { const itemHeight = 45; // in pixels @@ -204,7 +208,14 @@ export default defineComponent({ class="my-0 ml-1 pt-0" dense hide-details + :disabled="hierarchyActive" /> +
+ Not applicable to hierarchical types; DIVE selects the deepest qualifying type. +
(cameraStore.getTrack(track, camera)), + getTracks: (track: AnnotationId) => cameraStore.getTrackAll(track), groupFilterControls: groupFilters, setType: setTrackType, removeTypes, }); - const pendingSaveCount = computed(() => Math.max( - 0, - rawPendingSaveCount.value - trackFilters.typeHierarchyPendingCountAdjustment(), - )); clientSettingsSetup(trackFilters.allTypes); @@ -946,7 +943,7 @@ export default defineComponent({ confidenceFilters: trackFilters.confidenceFilters.value, timeFilters: trackFilters.timeFilters.value, imageEnhancements: imageEnhancements.value, - ...trackFilters.prepareTypeHierarchySavePatch(), + ...trackFilters.typeHierarchySavePatch(), // TODO Group confidence filters are not yet supported. }, saveSet); trackFilters.markTypeHierarchyPersisted(); @@ -1502,7 +1499,6 @@ export default defineComponent({ scheduleGlobalStylePersist.flush(); // Close and reset sideBar context.resetActive(); - trackFilters.setTypeHierarchy(undefined); const meta = await loadConfig(datasetId.value); trackFilters.setTypeHierarchy(meta.typeHierarchy); const hierarchyWarning = trackFilters.consumeLoadWarning(); diff --git a/client/dive-common/use/useModeManager.spec.ts b/client/dive-common/use/useModeManager.spec.ts index b9cda22c6..798c86838 100644 --- a/client/dive-common/use/useModeManager.spec.ts +++ b/client/dive-common/use/useModeManager.spec.ts @@ -13,6 +13,7 @@ import { IDENTITY3 } from 'vue-media-annotator/alignedView/alignedView'; import type { Matrix3 } from 'vue-media-annotator/alignedView/homography'; import type { AggregateMediaController } from 'vue-media-annotator/components/annotators/mediaControllerType'; import type { AnnotationId } from 'vue-media-annotator/BaseAnnotation'; +import Track from 'vue-media-annotator/track'; import { ROTATION_ATTRIBUTE_NAME } from 'vue-media-annotator/utils'; import useModeManager from './useModeManager'; @@ -58,6 +59,7 @@ function makeHarness() { markChangesPending: () => undefined, lookupGroups: cameraStore.lookupGroups.bind(cameraStore), getTrack: (id: AnnotationId, camera = 'singleCam') => cameraStore.getTrack(id, camera), + getTracks: (id: AnnotationId) => cameraStore.getTrackAll(id), groupFilterControls, setType: () => undefined, removeTypes: () => [], @@ -151,6 +153,18 @@ describe('useModeManager aligned-view track mirroring', () => { expect(cameraStore.getTrack(trackId, 'left').features[0]?.bounds).toEqual([10, 20, 30, 40]); }); + it('mirrors the whole source vector onto a newly created counterpart', () => { + const { cameraStore, modeManager } = makeHarness(); + const trackId = modeManager.handler.trackAdd(); + const source = cameraStore.getTrack(trackId, 'left'); + source.setType('leaf', 0.8); + modeManager.handler.updateRectBounds(0, 0, [10, 20, 30, 40]); + + const mirrored = cameraStore.getTrack(trackId, 'right'); + expect(mirrored.confidencePairs).toEqual(source.confidencePairs); + expect(mirrored.confidencePairs).not.toBe(source.confidencePairs); + }); + it('does not mirror while the aligned view is suspended (registration picking)', () => { const { cameraStore, alignedView, modeManager } = makeHarness(); alignedView.setSuspended(true); @@ -182,6 +196,7 @@ function makeSingleCamHarness() { markChangesPending: () => undefined, lookupGroups: cameraStore.lookupGroups.bind(cameraStore), getTrack: (id: AnnotationId, camera = 'singleCam') => cameraStore.getTrack(id, camera), + getTracks: (id: AnnotationId) => cameraStore.getTrackAll(id), groupFilterControls, setType: () => undefined, removeTypes: () => [], @@ -194,9 +209,44 @@ function makeSingleCamHarness() { readonlyState: ref(false), recipes: [], }); - return { cameraStore, modeManager }; + return { cameraStore, modeManager, trackFilterControls }; } +describe('useModeManager counterpart creation', () => { + it('copies the source confidence vector onto the counterpart camera track', () => { + const { cameraStore, modeManager } = makeHarness(); + cameraStore.camMap.value.get('left')?.trackStore.insert(new Track(9, { + confidencePairs: [['root', 0.9], ['leaf', 0.8]], + features: [{ frame: 0, bounds: [0, 0, 1, 1], keyframe: true }], + })); + modeManager.selectedCamera.value = 'right'; + modeManager.handler.trackAdd(9); + + const source = cameraStore.getTrack(9, 'left'); + const counterpart = cameraStore.getTrack(9, 'right'); + expect(counterpart.confidencePairs).toEqual([['root', 0.9], ['leaf', 0.8]]); + expect(counterpart.confidencePairs).not.toBe(source.confidencePairs); + expect(counterpart.confidencePairs[0]).not.toBe(source.confidencePairs[0]); + }); +}); + +describe('TrackFilterControls construction', () => { + it('provides complete stored-track enumeration for hierarchy renames', () => { + const { cameraStore, trackFilterControls } = makeSingleCamHarness(); + const trackStore = cameraStore.camMap.value.get('singleCam')?.trackStore; + trackStore?.insert(new Track(7, { + confidencePairs: [['leaf', 1], ['root', 0.8]], + features: [{ frame: 0, bounds: [0, 0, 1, 1], keyframe: true }], + })); + trackStore?.setEnableSorting(); + trackFilterControls.setTypeHierarchy({ leaf: 'root' }); + trackFilterControls.updateTypeName({ currentType: 'leaf', newType: 'fin' }); + expect(cameraStore.getTrack(7).confidencePairs).toEqual([ + ['fin', 1], ['root', 0.8], + ]); + }); +}); + describe('useModeManager polygon clip on box resize', () => { // Triangle that sticks past x=20; clipping to [0,0,20,40] leaves a non-box shape. const stickingOutPolygon = { diff --git a/client/dive-common/use/useModeManager.ts b/client/dive-common/use/useModeManager.ts index 79fecd5e5..71707ec5c 100644 --- a/client/dive-common/use/useModeManager.ts +++ b/client/dive-common/use/useModeManager.ts @@ -1,7 +1,9 @@ import { computed, Ref, reactive, ref, onBeforeUnmount, toRef, } from 'vue'; -import { uniq, flatMapDeep, flattenDeep } from 'lodash'; +import { + cloneDeep, uniq, flatMapDeep, flattenDeep, +} from 'lodash'; import Track, { Feature, TrackId, TrackSupportedFeature } from 'vue-media-annotator/track'; import { RectBounds, @@ -523,11 +525,12 @@ export default function useModeManager({ handleEscapeMode(); const frame = selectedCameraFrame(); let trackType = trackSettings.value.newTrackSettings.type; + let sourceTrack: Track | undefined; if (overrideTrackId !== undefined) { - const track = cameraStore.getAnyPossibleTrack(overrideTrackId); - if (track !== undefined) { + sourceTrack = cameraStore.getAnyPossibleTrack(overrideTrackId); + if (sourceTrack !== undefined) { // eslint-disable-next-line prefer-destructuring - trackType = track.confidencePairs[0][0]; + trackType = sourceTrack.confidencePairs[0][0]; } } else { // eslint-disable-next-line no-param-reassign @@ -535,15 +538,18 @@ export default function useModeManager({ } const trackStore = cameraStore.camMap.value.get(selectedCamera.value)?.trackStore; if (trackStore) { - const newTrackId = trackStore.add( + const newTrack = trackStore.add( frame, trackType, selectedTrackId.value || undefined, overrideTrackId, - ).id; - selectTrack(newTrackId, true); + ); + if (sourceTrack) { + newTrack.confidencePairs = cloneDeep(sourceTrack.confidencePairs); + } + selectTrack(newTrack.id, true); creating = true; - return newTrackId; + return newTrack.id; } throw Error(`Could not find trackStore for Camera: ${selectedCamera.value}`); } @@ -658,6 +664,7 @@ export default function useModeManager({ undefined, trackId, ); + targetTrack.confidencePairs = cloneDeep(sourceTrack.confidencePairs); } // setFeature only upserts geometry by (key, type): drop mirrored // geometry the source no longer has so deletions propagate too. diff --git a/client/dive-common/use/useSave.ts b/client/dive-common/use/useSave.ts index 2f789d565..539f047c4 100644 --- a/client/dive-common/use/useSave.ts +++ b/client/dive-common/use/useSave.ts @@ -41,6 +41,7 @@ export default function useSave( readonlyMode: Ref>, ) { const pendingSaveCount = ref(0); + let globalMetadataPending = 0; const pendingChangeMaps: Record = { singleCam: { upsert: new Map(), @@ -66,7 +67,6 @@ export default function useSave( throw new Error('attempted to save in read only mode'); } const promiseList: Promise[] = []; - let globalMetadataUpdated = false; Object.entries(pendingChangeMaps).forEach(([camera, pendingChangeMap]) => { const saveId = camera === 'singleCam' ? datasetId.value : `${datasetId.value}/${camera}`; if ( @@ -91,15 +91,15 @@ export default function useSave( })); } if (datasetMeta && pendingChangeMap.meta > 0) { - // Save once for each camera into their own metadata file - promiseList.push(saveConfig(saveId, datasetMeta).then(() => { + const cameraMeta = saveId === datasetId.value + ? datasetMeta + : Object.fromEntries( + Object.entries(datasetMeta).filter(([key]) => key !== 'typeHierarchy'), + ); + promiseList.push(saveConfig(saveId, cameraMeta).then(() => { // eslint-disable-next-line no-param-reassign pendingChangeMap.meta = 0; })); - // Only update global if there are multiple cameras - if (saveId !== datasetId.value) { - globalMetadataUpdated = true; - } } if (pendingChangeMap.attributeUpsert.size || pendingChangeMap.attributeDelete.size) { promiseList.push(saveAttributes(datasetId.value, { @@ -121,9 +121,10 @@ export default function useSave( })); } }); - // Final save into the multi-cam metadata if multiple cameras exists - if (globalMetadataUpdated && datasetMeta && pendingChangeMaps) { - promiseList.push(saveConfig(datasetId.value, datasetMeta)); + if (globalMetadataPending > 0 && datasetMeta) { + promiseList.push(saveConfig(datasetId.value, datasetMeta).then(() => { + globalMetadataPending = 0; + })); } await Promise.all(promiseList); pendingSaveCount.value = 0; @@ -153,6 +154,9 @@ export default function useSave( // eslint-disable-next-line no-param-reassign pendingChangeMap.meta += 1; }); + if (!pendingChangeMaps.singleCam) { + globalMetadataPending += 1; + } pendingSaveCount.value += 1; } else if (pendingChangeMaps[cameraName]) { const pendingChangeMap = pendingChangeMaps[cameraName]; @@ -210,6 +214,7 @@ export default function useSave( pendingChangeMap.meta = 0; }); pendingSaveCount.value = 0; + globalMetadataPending = 0; } function addCamera(cameraName: string) { diff --git a/client/src/BaseFilterControls.ts b/client/src/BaseFilterControls.ts index 12c9ac0e9..3e727280a 100644 --- a/client/src/BaseFilterControls.ts +++ b/client/src/BaseFilterControls.ts @@ -52,8 +52,8 @@ export default abstract class BaseFilterControls { /* Time filtering values */ timeFilters: Ref<[number, number] | null>; - /* The types informed by meta configuration */ - private defaultTypes: Ref; + /* The types informed by explicit meta configuration */ + configuredTypes: Ref; /* Collect all known types from confidence pairs */ allTypes: Ref; @@ -61,6 +61,9 @@ export default abstract class BaseFilterControls { /* Types currently assigned to at least one annotation */ usedTypes: Ref; + /* Types that should be persisted through type/style configuration */ + usedPlusConfiguredTypes: Ref; + /* Categorical types checked "ON" by the user */ checkedTypes: Ref; @@ -92,7 +95,7 @@ export default abstract class BaseFilterControls { this.timeFilters = ref(null); - this.defaultTypes = ref([]); + this.configuredTypes = ref([]); this.sorted = params.sorted; @@ -106,29 +109,26 @@ export default abstract class BaseFilterControls { this.disableAnnotationFilters = ref(false); - this.allTypes = computed(() => { + this.usedTypes = computed(() => { const typeSet = new Set(); this.sorted.value.forEach((annotation) => { annotation.confidencePairs.forEach(([name]) => { typeSet.add(name); }); }); - this.defaultTypes.value.forEach((type) => { - typeSet.add(type); - }); return Array.from(typeSet); }); - this.usedTypes = computed(() => { - const typeSet = new Set(); - this.sorted.value.forEach((annotation) => { - annotation.confidencePairs.forEach(([name]) => { - typeSet.add(name); - }); + this.usedPlusConfiguredTypes = computed(() => { + const typeSet = new Set(this.usedTypes.value); + this.configuredTypes.value.forEach((type) => { + typeSet.add(type); }); return Array.from(typeSet); }); + this.allTypes = this.usedPlusConfiguredTypes; + this.checkedTypes = ref(Array.from(this.allTypes.value)); this.filteredAnnotations = ref([]); @@ -165,8 +165,8 @@ export default abstract class BaseFilterControls { importTypes(types: string[], userInteraction = true) { types.forEach((type) => { - if (!this.defaultTypes.value.includes(type)) { - this.defaultTypes.value.push(type); + if (!this.configuredTypes.value.includes(type)) { + this.configuredTypes.value.push(type); } }); if (userInteraction) { @@ -174,12 +174,17 @@ export default abstract class BaseFilterControls { } } - deleteType(type: string) { - if (this.defaultTypes.value.includes(type)) { - this.defaultTypes.value.splice(this.defaultTypes.value.indexOf(type), 1); + protected deleteTypeConfiguration(type: string) { + if (this.configuredTypes.value.includes(type)) { + this.configuredTypes.value.splice(this.configuredTypes.value.indexOf(type), 1); } delete this.confidenceFilters.value[type]; + } + + deleteType(type: string): boolean { + this.deleteTypeConfiguration(type); this.markChangesPending({ action: 'meta' }); + return true; } setConfidenceFilters(val?: Record) { diff --git a/client/src/TrackFilterControls.spec.ts b/client/src/TrackFilterControls.spec.ts index a188ce88e..260f2430b 100644 --- a/client/src/TrackFilterControls.spec.ts +++ b/client/src/TrackFilterControls.spec.ts @@ -7,6 +7,8 @@ import type { MarkChangesPendingFilter } from './BaseFilterControls'; import CameraStore from './CameraStore'; import { AnnotationId } from './BaseAnnotation'; import useSave from '../dive-common/use/useSave'; +import { clientSettings } from '../dive-common/store/settings'; +import { TypeHierarchyError } from '../dive-common/typeHierarchy'; const apiMocks = vi.hoisted(() => ({ saveConfig: vi.fn(), @@ -108,12 +110,44 @@ function makeTrackFilterControls(markPending: MarkChangesPendingFilter = markCha groupFilterControls, lookupGroups: cameraStore.lookupGroups, getTrack: (track: AnnotationId, camera = 'singleCam') => (cameraStore.getTrack(track, camera)), + getTracks: (track: AnnotationId) => cameraStore.getTrackAll(track), setType: setTrackType, removeTypes, }); } +function makePairFixture( + confidencePairs: [string, number][][], + markPending = vi.fn(), +) { + const cameraStore = new CameraStore({ markChangesPending: markPending }); + const trackStore = cameraStore.camMap.value.get('singleCam')?.trackStore; + confidencePairs.forEach((pairs, id) => { + trackStore?.insert(new Track(id, { confidencePairs: pairs, features })); + }); + trackStore?.setEnableSorting(); + const groupFilterControls = makeGroupFilterControls(cameraStore); + const filters = new TrackFilterControls({ + sorted: cameraStore.sortedTracks, + remove: (id) => cameraStore.removeTracks(id), + markChangesPending: markPending, + groupFilterControls, + lookupGroups: cameraStore.lookupGroups, + getTrack: (id, camera = 'singleCam') => cameraStore.getTrack(id, camera), + getTracks: (id) => cameraStore.getTrackAll(id), + setType: (id, type, confidence, current) => ( + cameraStore.setTrackType(id, type, confidence, current) + ), + removeTypes: (id, types) => cameraStore.removeTypes(id, types), + }); + return { cameraStore, filters, markPending }; +} + describe('useAnnotationFilters', () => { + afterEach(() => { + clientSettings.typeSettings.preventCascadeTypes = false; + }); + it('loads absent and valid hierarchy state without creating a save instruction', () => { const tf = makeTrackFilterControls(); tf.setTypeHierarchy(undefined); @@ -162,10 +196,10 @@ describe('useAnnotationFilters', () => { it('retains a hierarchy save patch until persistence succeeds', () => { const tf = makeTrackFilterControls(); - tf.setTypeHierarchy({ shark: 'fish' }); - tf.updateTypeHierarchy({ shark: 'fish', tuna: 'fish' }); + tf.setTypeHierarchy({ foo: 'root' }); + tf.updateTypeName({ currentType: 'root', newType: 'heading' }); - const expected = { typeHierarchy: { shark: 'fish', tuna: 'fish' } }; + const expected = { typeHierarchy: { foo: 'heading' } }; expect(tf.typeHierarchySavePatch()).toEqual(expected); expect(tf.typeHierarchySavePatch()).toEqual(expected); tf.markTypeHierarchyPersisted(); @@ -181,15 +215,10 @@ describe('useAnnotationFilters', () => { const tf = makeTrackFilterControls( saveControls.markChangesPending as MarkChangesPendingFilter, ); - tf.setTypeHierarchy({ shark: 'fish' }); - tf.updateTypeHierarchy({ shark: 'fish', tuna: 'fish' }); - const expected = { typeHierarchy: { shark: 'fish', tuna: 'fish' } }; - const visiblePendingCount = () => Math.max( - 0, - saveControls.pendingSaveCount.value - tf.typeHierarchyPendingCountAdjustment(), - ); + tf.setTypeHierarchy({ foo: 'root' }); + tf.updateTypeName({ currentType: 'root', newType: 'heading' }); + const expected = { typeHierarchy: { foo: 'heading' } }; expect(saveControls.pendingSaveCount.value).toBe(1); - expect(visiblePendingCount()).toBe(1); let rejectParent = true; apiMocks.saveConfig.mockImplementation(async (id: string) => { if (id === datasetId && rejectParent) { @@ -198,39 +227,25 @@ describe('useAnnotationFilters', () => { } }); - const firstPatch = tf.prepareTypeHierarchySavePatch(); + const firstPatch = tf.typeHierarchySavePatch(); expect(saveControls.pendingSaveCount.value).toBe(1); - expect(tf.typeHierarchyPendingCountAdjustment()).toBe(0); - expect(visiblePendingCount()).toBe(1); await expect(saveControls.save(firstPatch)).rejects.toThrow('parent save failed'); expect(tf.typeHierarchySavePatch()).toEqual(expected); - const retryPatch = tf.prepareTypeHierarchySavePatch(); + const retryPatch = tf.typeHierarchySavePatch(); expect(retryPatch).toEqual(firstPatch); - expect(saveControls.pendingSaveCount.value).toBe(2); - expect(tf.typeHierarchyPendingCountAdjustment()).toBe(1); - expect(visiblePendingCount()).toBe(1); + expect(saveControls.pendingSaveCount.value).toBe(1); await saveControls.save(retryPatch); expect(apiMocks.saveConfig.mock.calls).toEqual([ - [`${datasetId}/left`, expected], - [`${datasetId}/right`, expected], + [`${datasetId}/left`, {}], + [`${datasetId}/right`, {}], [datasetId, expected], - [`${datasetId}/left`, expected], - [`${datasetId}/right`, expected], [datasetId, expected], ]); tf.markTypeHierarchyPersisted(); - expect(tf.typeHierarchyPendingCountAdjustment()).toBe(0); - expect(visiblePendingCount()).toBe(0); - expect(tf.prepareTypeHierarchySavePatch()).toEqual({}); - }); - - it('uses an explicit delete patch for a locally cleared hierarchy', () => { - const tf = makeTrackFilterControls(); - tf.setTypeHierarchy({ shark: 'fish' }); - tf.updateTypeHierarchy(null); - expect(tf.typeHierarchySavePatch()).toEqual({ typeHierarchy: null }); + expect(saveControls.pendingSaveCount.value).toBe(0); + expect(tf.typeHierarchySavePatch()).toEqual({}); }); it('accepts corrected replacement and clear loads after invalid storage', () => { @@ -273,4 +288,321 @@ describe('useAnnotationFilters', () => { tf.removeTypeAnnotations(['baz']); expect(tf.allTypes.value).toEqual(['foo', 'bar', 'baz']); }); + + it('returns the caller fallback without recomputing flat pair selection', () => { + const { cameraStore, filters } = makePairFixture([ + [['root', 0.1], ['leaf', 0.9]], + ]); + const track = cameraStore.getTrack(0); + filters.checkedTypes.value = []; + filters.setConfidenceFilters({ default: 1 }); + filters.disableAnnotationFilters.value = true; + expect(filters.displayPairIndex(track, 1)).toBe(1); + expect(filters.displayPairIndex(track, -1)).toBe(-1); + }); + + it('preserves the complete flat filter matrix', () => { + const { filters } = makePairFixture([ + [['top', 0.5], ['fallback', 0.8]], + [], + ]); + filters.setConfidenceFilters({ top: 0.5, fallback: 0.8, default: 0.1 }); + filters.checkedTypes.value = ['top', 'fallback']; + expect(filters.filteredAnnotations.value.map(({ context }) => context.confidencePairIndex)) + .toEqual([0, -1]); + + filters.checkedTypes.value = ['fallback']; + expect(filters.filteredAnnotations.value.map(({ context }) => context.confidencePairIndex)) + .toEqual([1, -1]); + + const cascadeFixture = makePairFixture([ + [['top', 0.5], ['fallback', 0.8]], + ]).filters; + cascadeFixture.setConfidenceFilters({ top: 0.5, fallback: 0.8, default: 0.1 }); + cascadeFixture.checkedTypes.value = ['top', 'fallback']; + clientSettings.typeSettings.preventCascadeTypes = true; + expect(cascadeFixture.filteredAnnotations.value).toHaveLength(0); + cascadeFixture.setConfidenceFilters({ top: 0.49, fallback: 0.8, default: 0.1 }); + expect(cascadeFixture.filteredAnnotations.value.map(({ context }) => context.confidencePairIndex)) + .toEqual([0]); + + cascadeFixture.disableAnnotationFilters.value = true; + expect(cascadeFixture.filteredAnnotations.value.map(({ context }) => context.confidencePairIndex)) + .toEqual([0]); + }); + + it('selects deepest qualifying hierarchy pairs for monotone and non-monotone scores', () => { + const { cameraStore, filters } = makePairFixture([ + [['root', 0.9], ['child', 0.8], ['leaf', 0.7]], + [['root', 0.2], ['child', 0.9], ['leaf', 0.6]], + ]); + filters.setTypeHierarchy({ leaf: 'child', child: 'root' }); + filters.setConfidenceFilters({ default: 0.5 }); + expect(filters.displayPairIndex(cameraStore.getTrack(0), 0)).toBe(2); + expect(filters.displayPairIndex(cameraStore.getTrack(1), 0)).toBe(2); + + filters.setConfidenceFilters({ leaf: 0.7, default: 0.5 }); + expect(filters.displayPairIndex(cameraStore.getTrack(0), 0)).toBe(2); + filters.setConfidenceFilters({ leaf: 0.71, default: 0.5 }); + expect(filters.displayPairIndex(cameraStore.getTrack(0), 0)).toBe(1); + }); + + it('rolls up unchecked leaves and ignores Prevent Cascade in hierarchy mode', () => { + const { cameraStore, filters } = makePairFixture([ + [['root', 0.9], ['child', 0.8], ['leaf', 0.7]], + ]); + filters.setTypeHierarchy({ leaf: 'child', child: 'root' }); + filters.checkedTypes.value = ['root', 'child']; + clientSettings.typeSettings.preventCascadeTypes = false; + const withoutPrevent = filters.displayPairIndex(cameraStore.getTrack(0), 0); + clientSettings.typeSettings.preventCascadeTypes = true; + expect(filters.displayPairIndex(cameraStore.getTrack(0), 0)).toBe(withoutPrevent); + expect(withoutPrevent).toBe(1); + }); + + it('uses pair zero for the active-hierarchy disabled-filter bypass', () => { + const { cameraStore, filters } = makePairFixture([[['root', 0.1], ['leaf', 0.9]]]); + filters.setTypeHierarchy({ leaf: 'root' }); + filters.checkedTypes.value = []; + filters.disableAnnotationFilters.value = true; + expect(filters.displayPairIndex(cameraStore.getTrack(0), -1)).toBe(0); + }); + + it('excludes empty and entirely non-passing hierarchy vectors without inventing a pair', () => { + const { cameraStore, filters } = makePairFixture([[], [['root', 0.1]]]); + filters.setTypeHierarchy({ leaf: 'root' }); + filters.setConfidenceFilters({ default: 0.5 }); + expect(filters.displayPairIndex(cameraStore.getTrack(0), 0)).toBe(-1); + expect(filters.displayPairIndex(cameraStore.getTrack(1), 0)).toBe(-1); + expect(filters.filteredAnnotations.value).toEqual([]); + + filters.disableAnnotationFilters.value = true; + expect(filters.displayPairIndex(cameraStore.getTrack(0), 0)).toBe(-1); + expect(filters.filteredAnnotations.value.map(({ annotation, context }) => ({ + id: annotation.id, + confidencePairIndex: context.confidencePairIndex, + }))).toEqual([{ id: 1, confidencePairIndex: 0 }]); + }); + + it('compiles only when hierarchy content changes', () => { + const { filters } = makePairFixture([[['root', 1]]]); + filters.setTypeHierarchy({ leaf: 'root' }); + const firstIndex = filters.hierarchyIndex.value; + filters.setConfidenceFilters({ default: 0.9 }); + filters.checkedTypes.value = ['root']; + expect(filters.hierarchyIndex.value).toBe(firstIndex); + filters.setTypeHierarchy({ leaf: 'root' }); + expect(filters.hierarchyIndex.value).toBe(firstIndex); + filters.setTypeHierarchy({ leaf: 'root', fin: 'root' }); + expect(filters.hierarchyIndex.value).not.toBe(firstIndex); + }); + + it('keeps hierarchy-only members out of configured style persistence', () => { + const { filters } = makePairFixture([[['leaf', 1]]]); + filters.importTypes(['configured'], false); + filters.setTypeHierarchy({ leaf: 'heading' }); + expect(filters.allTypes.value).toEqual(['leaf', 'configured', 'heading']); + expect(filters.usedPlusConfiguredTypes.value).toEqual(['leaf', 'configured']); + expect(filters.checkedTypes.value).toEqual(expect.arrayContaining(['heading'])); + filters.setTypeHierarchy(undefined); + expect(filters.allTypes.value).toEqual(['leaf', 'configured']); + expect(filters.checkedTypes.value).not.toContain('heading'); + }); + + it('does not recheck an unchecked hierarchy member when style promotion configures it', () => { + const { filters } = makePairFixture([[['leaf', 1]]]); + filters.setTypeHierarchy({ leaf: 'heading' }); + filters.updateCheckedTypes(['leaf']); + filters.importTypes(['heading'], false); + expect(filters.configuredTypes.value).toContain('heading'); + expect(filters.checkedTypes.value).toEqual(['leaf']); + }); + + it('preserves an unchecked used type when hierarchy loading makes it a member', () => { + const { filters } = makePairFixture([[['leaf', 1]]]); + filters.updateCheckedTypes([]); + filters.setTypeHierarchy({ leaf: 'heading' }); + expect(filters.checkedTypes.value).toEqual(['heading']); + }); + + it('keeps generic group type imports checkbox-neutral', () => { + const cameraStore = makeCameraStore(); + const groupFilters = makeGroupFilterControls(cameraStore); + groupFilters.updateCheckedTypes([]); + groupFilters.importTypes(['unused group'], false); + expect(groupFilters.allTypes.value).toContain('unused group'); + expect(groupFilters.checkedTypes.value).toEqual([]); + }); + + it('preserves flat track and group configured-only rename behavior', () => { + const { filters } = makePairFixture([[['used', 0.8]]]); + filters.importTypes(['unused'], false); + filters.updateTypeName({ currentType: 'unused', newType: 'renamed' }); + expect(filters.configuredTypes.value).not.toContain('unused'); + expect(filters.configuredTypes.value).not.toContain('renamed'); + + const cameraStore = makeCameraStore(); + const groupFilters = makeGroupFilterControls(cameraStore); + groupFilters.importTypes(['unused group'], false); + groupFilters.updateTypeName({ currentType: 'unused group', newType: 'renamed group' }); + expect(groupFilters.configuredTypes.value).not.toContain('unused group'); + expect(groupFilters.configuredTypes.value).not.toContain('renamed group'); + }); + + it('rewrites hierarchy, annotations, configured types, filters, and checks on rename', () => { + const markPending = vi.fn(); + const { cameraStore, filters } = makePairFixture([[['leaf', 0.8], ['root', 0.7]]], markPending); + filters.importTypes(['leaf'], false); + filters.setConfidenceFilters({ leaf: 0.4, default: 0.1 }); + filters.setTypeHierarchy({ leaf: 'root' }); + markPending.mockClear(); + filters.updateTypeName({ currentType: 'leaf', newType: 'fin' }); + expect(filters.typeHierarchy.value).toEqual({ fin: 'root' }); + expect(cameraStore.getTrack(0).confidencePairs).toEqual([['fin', 0.8], ['root', 0.7]]); + expect(filters.configuredTypes.value).toEqual(['fin']); + expect(filters.confidenceFilters.value).toEqual({ fin: 0.4, default: 0.1 }); + expect(filters.checkedTypes.value).toContain('fin'); + expect(filters.typeHierarchySavePatch()).toEqual({ typeHierarchy: { fin: 'root' } }); + }); + + it('does not configure a hierarchy-only heading during a name-only rename', () => { + const { filters } = makePairFixture([[['leaf', 1]]]); + filters.setTypeHierarchy({ leaf: 'heading' }); + filters.updateTypeName({ currentType: 'heading', newType: 'renamed heading' }); + expect(filters.typeHierarchy.value).toEqual({ leaf: 'renamed heading' }); + expect(filters.allTypes.value).toEqual(['leaf', 'renamed heading']); + expect(filters.usedPlusConfiguredTypes.value).toEqual(['leaf']); + }); + + it('renames without collapsing, reordering, or rescoring a confidence-1 vector', () => { + const { cameraStore, filters } = makePairFixture([ + [['leaf', 1], ['root', 0.8], ['other', 0.2]], + ]); + filters.setTypeHierarchy({ leaf: 'root' }); + filters.updateTypeName({ currentType: 'leaf', newType: 'fin' }); + expect(cameraStore.getTrack(0).confidencePairs).toEqual([ + ['fin', 1], ['root', 0.8], ['other', 0.2], + ]); + }); + + it('preserves each camera confidence vector while renaming exact occurrences', () => { + const { cameraStore, filters } = makePairFixture([ + [['leaf', 1], ['root', 0.8]], + ]); + cameraStore.addCamera('right'); + cameraStore.camMap.value.get('right')?.trackStore.insert(new Track(0, { + confidencePairs: [['other', 0.6], ['leaf', 0.4]], + features, + })); + filters.setTypeHierarchy({ leaf: 'root' }); + filters.updateTypeName({ currentType: 'leaf', newType: 'fin' }); + expect(cameraStore.getTrack(0, 'singleCam').confidencePairs).toEqual([ + ['fin', 1], ['root', 0.8], + ]); + expect(cameraStore.getTrack(0, 'right').confidencePairs).toEqual([ + ['other', 0.6], ['fin', 0.4], + ]); + }); + + it('rejects invalid hierarchy renames before any mutation or pending event', () => { + const markPending = vi.fn(); + const { cameraStore, filters } = makePairFixture([[['leaf', 0.8]]], markPending); + filters.setTypeHierarchy({ leaf: 'root' }); + markPending.mockClear(); + expect(() => filters.updateTypeName({ currentType: 'leaf', newType: 'root' })) + .toThrow(TypeHierarchyError); + expect(filters.typeHierarchy.value).toEqual({ leaf: 'root' }); + expect(cameraStore.getTrack(0).confidencePairs).toEqual([['leaf', 0.8]]); + expect(markPending).not.toHaveBeenCalled(); + }); + + it('rejects a rename when one track already has both names', () => { + const markPending = vi.fn(); + const { cameraStore, filters } = makePairFixture([ + [['leaf', 0.8], ['fin', 0.7]], + ], markPending); + filters.setTypeHierarchy({ leaf: 'root' }); + markPending.mockClear(); + expect(() => filters.updateTypeName({ currentType: 'leaf', newType: 'fin' })) + .toThrow('track 0 already contains both "leaf" and "fin"'); + expect(cameraStore.getTrack(0).confidencePairs).toEqual([['leaf', 0.8], ['fin', 0.7]]); + expect(markPending).not.toHaveBeenCalled(); + }); + + it('rejects a collision in another camera before changing any stored vector', () => { + const markPending = vi.fn(); + const { cameraStore, filters } = makePairFixture([ + [['leaf', 1], ['root', 0.8]], + ], markPending); + cameraStore.addCamera('right'); + cameraStore.camMap.value.get('right')?.trackStore.insert(new Track(0, { + confidencePairs: [['leaf', 0.6], ['fin', 0.5]], + features, + })); + filters.setTypeHierarchy({ leaf: 'root' }); + markPending.mockClear(); + + expect(() => filters.updateTypeName({ currentType: 'leaf', newType: 'fin' })) + .toThrow('track 0 already contains both "leaf" and "fin"'); + expect(cameraStore.getTrack(0, 'singleCam').confidencePairs).toEqual([ + ['leaf', 1], ['root', 0.8], + ]); + expect(cameraStore.getTrack(0, 'right').confidencePairs).toEqual([ + ['leaf', 0.6], ['fin', 0.5], + ]); + expect(filters.typeHierarchy.value).toEqual({ leaf: 'root' }); + expect(markPending).not.toHaveBeenCalled(); + }); + + it('clears settings for unused parents and leaves hierarchy state unchanged', () => { + const markPending = vi.fn(); + const { filters } = makePairFixture([[['used', 1]]], markPending); + filters.importTypes(['leaf'], false); + filters.setConfidenceFilters({ leaf: 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); + }); + + it('blocks deleting a type that only a collapse-hidden camera still uses', () => { + const markPending = vi.fn(); + const { cameraStore, filters } = makePairFixture([ + [['fish', 0.9], ['tuna', 0.7]], + ], markPending); + cameraStore.addCamera('right'); + cameraStore.camMap.value.get('right')?.trackStore.insert(new Track(0, { + confidencePairs: [['shark', 1]], + features, + })); + filters.importTypes(['tuna'], false); + filters.setConfidenceFilters({ tuna: 0.4, default: 0.1 }); + filters.setTypeHierarchy({ tuna: 'fish' }); + markPending.mockClear(); + + expect(filters.usedTypes.value).toEqual(['shark']); + expect(filters.typeInUseOnAnyCamera('tuna')).toBe(true); + expect(filters.deleteType('tuna')).toBe(false); + expect(filters.typeHierarchy.value).toEqual({ tuna: 'fish' }); + expect(filters.configuredTypes.value).toContain('tuna'); + expect(filters.confidenceFilters.value).toHaveProperty('tuna', 0.4); + expect(filters.checkedTypes.value).toContain('tuna'); + expect(markPending).not.toHaveBeenCalled(); + }); + + it('keeps hierarchy active after clearing the final leaf settings', () => { + 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({}); + }); }); diff --git a/client/src/TrackFilterControls.ts b/client/src/TrackFilterControls.ts index 0a0ae3463..2a6c0f958 100644 --- a/client/src/TrackFilterControls.ts +++ b/client/src/TrackFilterControls.ts @@ -4,6 +4,8 @@ import { clientSettings } from 'dive-common/store/settings'; import { compileHierarchy, normalizeTypeHierarchy, + rewriteHierarchyType, + selectPairIndex, TypeHierarchy, TypeHierarchyError, TypeHierarchyIndex, @@ -18,6 +20,7 @@ interface TrackFilterControlsParams extends FilterControlsParams { lookupGroups: (annotationId: AnnotationId) => Group[]; getTrack: (annotationId: AnnotationId, camera?: string) => Track; groupFilterControls: BaseFilterControls; + getTracks: (annotationId: AnnotationId) => Track[]; } export default class TrackFilterControls extends BaseFilterControls { @@ -37,23 +40,22 @@ export default class TrackFilterControls extends BaseFilterControls { hierarchyMembers: Ref; - usedPlusConfiguredTypes: Ref; - invalidHierarchyReason: Ref; private hierarchyWarningConsumed = false; private hierarchyDirty = false; - private hierarchySavePrepared = false; + private checkedTypesSet = computed(() => new Set(this.checkedTypes.value)); - private hierarchySaveRearmCount = ref(0); + private getTracks: (annotationId: AnnotationId) => Track[]; constructor(params: TrackFilterControlsParams) { super(params); + this.getTracks = params.getTracks; + const flatAllTypes = this.allTypes; - this.usedPlusConfiguredTypes = flatAllTypes; this.typeHierarchy = ref(undefined); this.hierarchyIndex = ref(undefined); this.invalidHierarchyReason = ref(null); @@ -82,7 +84,7 @@ export default class TrackFilterControls extends BaseFilterControls { * for filtering based on group membership as well */ this.filteredAnnotations = computed(() => { - const checkedSet = new Set(this.checkedTypes.value); + const checkedSet = this.checkedTypesSet.value; const filteredGroupsSet = new Set(params.groupFilterControls.enabledAnnotations.value .map((v) => v.annotation.id)); const confidenceFiltersVal = cloneDeep(this.confidenceFilters.value); @@ -104,33 +106,42 @@ export default class TrackFilterControls extends BaseFilterControls { */ enabledInGroupFilters = groups.some((group) => filteredGroupsSet.has(group.id)); } - let confidencePairIndex = annotation.confidencePairs - .findIndex(([confkey, confval]) => { + let confidencePairIndex: number; + if (this.hierarchyActive.value) { + confidencePairIndex = this.displayPairIndex( + annotation as unknown as Readonly, + -1, + ); + } else { + confidencePairIndex = annotation.confidencePairs + .findIndex(([confkey, confval]) => { + const confidenceThresh = Math.max( + confidenceFiltersVal[confkey] || 0, + confidenceFiltersVal.default, + ); + return confval >= confidenceThresh && checkedSet.has(confkey); + }); + if (clientSettings.typeSettings.preventCascadeTypes) { + const [confkey, confval] = annotation.confidencePairs[0]; const confidenceThresh = Math.max( confidenceFiltersVal[confkey] || 0, confidenceFiltersVal.default, ); - return confval >= confidenceThresh && checkedSet.has(confkey); - }); - if (clientSettings.typeSettings.preventCascadeTypes) { - const [confkey, confval] = annotation.confidencePairs[0]; - const confidenceThresh = Math.max( - confidenceFiltersVal[confkey] || 0, - confidenceFiltersVal.default, - ); - if (checkedSet.has(confkey) && confval > confidenceThresh) { + if (checkedSet.has(confkey) && confval > confidenceThresh) { + confidencePairIndex = 0; + } else { + confidencePairIndex = -1; + } + } + if (this.disableAnnotationFilters.value) { confidencePairIndex = 0; - } else { - confidencePairIndex = -1; } } - if (this.disableAnnotationFilters.value) { - confidencePairIndex = 0; - } /* include annotations where at least 1 confidence pair is above * the threshold and part of the checked type set */ if ( - (confidencePairIndex >= 0 || annotation.confidencePairs.length === 0) + (confidencePairIndex >= 0 + || (!this.hierarchyActive.value && annotation.confidencePairs.length === 0)) && enabledInGroupFilters && !resultsIds.has(annotation.id) ) { let addValue = true; @@ -159,8 +170,31 @@ export default class TrackFilterControls extends BaseFilterControls { }); } + displayPairIndex(track: Readonly, flatFallbackIndex: number): number { + const index = this.hierarchyIndex.value; + if (index === undefined) { + return flatFallbackIndex; + } + if (track.confidencePairs.length === 0) { + return -1; + } + if (this.disableAnnotationFilters.value) { + return 0; + } + const checkedSet = this.checkedTypesSet.value; + const confidenceFilters = this.confidenceFilters.value; + const passes = track.confidencePairs.map(([confkey, confval]) => { + const confidenceThresh = Math.max( + confidenceFilters[confkey] || 0, + confidenceFilters.default, + ); + return confval >= confidenceThresh && checkedSet.has(confkey); + }); + return selectPairIndex(index, track.confidencePairs, passes); + } + private installTypeHierarchy(value: unknown, dirty: boolean) { - const previousMembers = new Set(this.hierarchyMembers.value); + const previousTypes = new Set(this.allTypes.value); let normalized: TypeHierarchy | undefined; try { normalized = normalizeTypeHierarchy(value === undefined ? null : value); @@ -189,17 +223,13 @@ export default class TrackFilterControls extends BaseFilterControls { (name) => baseline.has(name) || nextMembers.has(name), ); nextMembers.forEach((name) => { - if (!previousMembers.has(name) && !checked.includes(name)) { + if (!previousTypes.has(name) && !checked.includes(name)) { checked.push(name); } }); this.checkedTypes.value = checked; this.hierarchyDirty = dirty; - this.hierarchySavePrepared = false; - if (!dirty) { - this.hierarchySaveRearmCount.value = 0; - } } /** Install hierarchy state loaded from a dataset or a successful config replacement. */ @@ -208,10 +238,95 @@ export default class TrackFilterControls extends BaseFilterControls { this.installTypeHierarchy(value, false); } - /** Install a locally edited hierarchy and include it in the next metadata save. */ - updateTypeHierarchy(value: unknown) { - this.installTypeHierarchy(value, true); + /** Usage across every camera's stored vector, unlike the lossy merged `usedTypes`. */ + typeInUseOnAnyCamera(type: string): boolean { + return this.sorted.value.some((annotation) => this.getTracks(annotation.id) + .some((track) => track.confidencePairs.some(([name]) => name === type))); + } + + updateTypeName({ currentType, newType }: { currentType: string; newType: string }) { + if (!this.hierarchyActive.value) { + super.updateTypeName({ currentType, newType }); + 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); + }); + if (collision) { + throw new TypeHierarchyError( + `track ${collision.id} already contains both "${currentType}" and "${newType}"`, + 'conflict', + ); + } + + 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); + + this.sorted.value.forEach((annotation) => { + const storedTracks = this.getTracks(annotation.id); + const rewrittenPairs = storedTracks.map((track) => track.confidencePairs.map( + ([name, confidence]) => [ + name === currentType ? newType : name, + confidence, + ] as [string, number], + )); + const triggerPair = storedTracks + .flatMap((track) => track.confidencePairs) + .find(([name]) => name === currentType); + if (triggerPair) { + this.setType(annotation.id, newType, triggerPair[1], currentType); + storedTracks.forEach((track, index) => { + // setType emits the existing annotation notification. Restore the exact + // preflighted vector because its confidence-1 branch intentionally collapses pairs. + // eslint-disable-next-line no-param-reassign + track.confidencePairs = rewrittenPairs[index]; + }); + } + }); + if (!(newType in this.confidenceFilters.value) + && currentType in this.confidenceFilters.value) { + this.setConfidenceFilters({ + ...this.confidenceFilters.value, + [newType]: this.confidenceFilters.value[currentType], + }); + } + if (this.configuredTypes.value.includes(currentType) + && !this.configuredTypes.value.includes(newType)) { + this.configuredTypes.value.push(newType); + } + this.deleteTypeConfiguration(currentType); + if (hierarchyChanged) { + this.installTypeHierarchy(rewritten, 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); + } + this.checkedTypes.value = Array.from(checked); + this.markChangesPending({ action: 'meta' }); + } + + deleteType(type: string): boolean { + if (!this.hierarchyActive.value) { + return super.deleteType(type); + } + if (this.typeInUseOnAnyCamera(type)) { + return false; + } + this.deleteTypeConfiguration(type); this.markChangesPending({ action: 'meta' }); + return true; } consumeLoadWarning(): string | null { @@ -232,28 +347,8 @@ export default class TrackFilterControls extends BaseFilterControls { return { typeHierarchy: { ...(this.typeHierarchy.value || {}) } }; } - /** Re-arm metadata writes for every save attempt while hierarchy state is dirty. */ - prepareTypeHierarchySavePatch(): { typeHierarchy?: Record | null } { - const patch = this.typeHierarchySavePatch(); - if (Object.prototype.hasOwnProperty.call(patch, 'typeHierarchy')) { - if (this.hierarchySavePrepared) { - this.markChangesPending({ action: 'meta' }); - this.hierarchySaveRearmCount.value += 1; - } else { - this.hierarchySavePrepared = true; - } - } - return patch; - } - - typeHierarchyPendingCountAdjustment() { - return this.hierarchySaveRearmCount.value; - } - markTypeHierarchyPersisted() { this.hierarchyDirty = false; - this.hierarchySavePrepared = false; - this.hierarchySaveRearmCount.value = 0; } loadTrackAttributesFilter(trackAttributesFilter: Readonly) { diff --git a/client/src/components/FilterList.spec.ts b/client/src/components/FilterList.spec.ts new file mode 100644 index 000000000..14ce75d3d --- /dev/null +++ b/client/src/components/FilterList.spec.ts @@ -0,0 +1,151 @@ +// @vitest-environment jsdom +/// +import { + defineComponent, h, nextTick, ref, reactive, +} from 'vue'; +import { shallowMount } from '@vue/test-utils'; +import { clientSettings } from 'dive-common/store/settings'; +import FilterList from './FilterList.vue'; + +vi.mock('dive-common/vue-utilities/prompt-service', () => ({ + usePrompt: () => ({ prompt: vi.fn(), visible: () => false }), +})); + +/** + * `@vue/test-utils` types a mount target as a Vue 2 constructor, which a `defineComponent` + * SFC is not, so the list is rendered from a host that captures the real instance. It is left + * unstubbed to keep shallow semantics for its own children, and the host stands in for + * `setProps`. + */ +function mountFilterList(props: Record) { + const state = reactive(props); + let child: InstanceType | undefined; + const Host = defineComponent({ + setup: () => () => h(FilterList, { + props: state, + ref: (instance) => { + if (instance && !(instance instanceof Element)) { + child = instance as InstanceType; + } + }, + }), + }); + const wrapper = shallowMount(Host, { stubs: { FilterList: false } }); + if (!child) { + throw new Error('FilterList did not mount'); + } + const setProps = async (next: Record) => { + Object.assign(state, next); + await nextTick(); + }; + return { wrapper, vm: child, setProps }; +} + +vi.mock('../provides', () => ({ + useCameraStore: () => ({ + camMap: ref(new Map([['singleCam', { + trackStore: { + annotationMap: new Map(), + intervalTree: { search: () => [] }, + getPossible: () => undefined, + }, + }]])), + getAnyPossibleTrack: () => undefined, + }), + useHandler: () => ({ seekFrame: vi.fn() }), + useReadOnlyMode: () => ref(false), + useSelectedCamera: () => ref('singleCam'), + useTime: () => ({ frame: ref(0) }), + usePendingSaveCount: () => ref(0), +})); + +describe('FilterList hierarchy members', () => { + it('keeps members as ordinary, independently checked flat rows', async () => { + clientSettings.typeSettings.trackSortDir = 'a-z'; + clientSettings.typeSettings.filterTypesByFrame = false; + clientSettings.typeSettings.suppressionType = ''; + const checkedTypes = ref(['leaf', 'heading']); + const filterControls = Object.freeze({ + allTypes: ref(['leaf', 'heading']), + usedTypes: ref(['leaf']), + checkedTypes, + filteredAnnotations: ref([]), + confidenceFilters: ref({ default: 0.1 }), + disableAnnotationFilters: ref(false), + updateCheckedTypes: (types: string[]) => { checkedTypes.value = types; }, + removeTypeAnnotations: vi.fn(), + }); + const styleManager = Object.freeze({ + typeStyling: ref({ + color: () => '#fff', + strokeWidth: () => 1, + fill: () => false, + opacity: () => 1, + }), + }); + const { vm, setProps } = mountFilterList({ + filterControls, + styleManager, + showEmptyTypes: false, + height: 240, + headerHeight: 80, + }); + expect(vm.visibleTypes).toEqual(['leaf']); + expect(vm.virtualHeight).toBe(160); + + await setProps({ showEmptyTypes: true }); + expect(vm.visibleTypes).toEqual(['heading', 'leaf']); + expect(vm.virtualTypes.map(({ type }) => type)).toEqual(['heading', 'leaf']); + vm.updateCheckedType(false, 'heading'); + expect(checkedTypes.value).toEqual(['leaf']); + expect(vm.virtualTypes.find(({ type }) => type === 'heading')?.checked).toBe(false); + }); + + it('counts the type selected by each filtered annotation context', () => { + clientSettings.typeSettings.trackSortDir = 'a-z'; + clientSettings.typeSettings.filterTypesByFrame = false; + clientSettings.typeSettings.suppressionType = ''; + const confidencePairs: [string, number][] = [['root', 0.9], ['leaf', 0.7]]; + const filterControls = Object.freeze({ + allTypes: ref(['root', 'leaf']), + usedTypes: ref(['root', 'leaf']), + checkedTypes: ref(['root', 'leaf']), + filteredAnnotations: ref([{ + annotation: { + id: 1, + begin: 0, + end: 1, + confidencePairs, + getType: (index = 0) => confidencePairs[index][0], + }, + context: { confidencePairIndex: 1 }, + }]), + confidenceFilters: ref({ default: 0.1 }), + disableAnnotationFilters: ref(false), + updateCheckedTypes: vi.fn(), + removeTypeAnnotations: vi.fn(), + }); + const styleManager = Object.freeze({ + typeStyling: ref({ + color: (type: string) => `color:${type}`, + strokeWidth: () => 1, + fill: () => false, + opacity: () => 1, + }), + }); + const { vm } = mountFilterList({ + filterControls, + styleManager, + showEmptyTypes: true, + height: 240, + headerHeight: 80, + }); + + expect(vm.virtualTypes.find(({ type }) => type === 'leaf')).toEqual(expect.objectContaining({ + displayText: '1 : 0\u00A0 leaf', + color: 'color:leaf', + })); + expect(vm.virtualTypes.find(({ type }) => type === 'root')?.displayText) + .toBe('0 : 0\u00A0 root'); + }); +}); diff --git a/client/src/components/LayerManager.spec.ts b/client/src/components/LayerManager.spec.ts new file mode 100644 index 000000000..55fc360fa --- /dev/null +++ b/client/src/components/LayerManager.spec.ts @@ -0,0 +1,355 @@ +// @vitest-environment jsdom +/// +/* eslint-disable max-classes-per-file -- lightweight layer doubles */ +import { + defineComponent, h, ref, +} from 'vue'; +import { shallowMount } from '@vue/test-utils'; +import Track, { Feature } from '../track'; +import CameraStore from '../CameraStore'; +import TrackFilterControls from '../TrackFilterControls'; +import GroupFilterControls from '../GroupFilterControls'; +import type { AnnotationId } from '../BaseAnnotation'; +import LayerManager from './LayerManager.vue'; + +const layerMocks = vi.hoisted(() => { + const rectangleChangeData = vi.fn(); + + class MockLayer { + bus = { $on: vi.fn() }; + + featureLayer = {}; + + changeData = vi.fn(); + + disable = vi.fn(); + + setHoverAnnotations = vi.fn(); + + setClickTargetsOnly = vi.fn(); + + setDrawingOther = vi.fn(); + + updateSettings = vi.fn(); + + updateRenderAttributes = vi.fn(); + + setType = vi.fn(); + + setKey = vi.fn(); + + getMode = vi.fn(() => 'disabled'); + + clear = vi.fn(); + + updatePoints = vi.fn(); + + update = vi.fn(); + + addDOMWidget = vi.fn(); + + setToolTipWidget = vi.fn(); + + setDisplayTransform = vi.fn(); + } + + class MockRectangleLayer extends MockLayer { + changeData = rectangleChangeData; + } + + return { MockLayer, MockRectangleLayer, rectangleChangeData }; +}); + +const provided = vi.hoisted(() => ({ + values: null as null | Record, +})); + +vi.mock('../layers/AnnotationLayers/RectangleLayer', () => ({ + default: layerMocks.MockRectangleLayer, +})); +vi.mock('../layers/AnnotationLayers/PolygonLayer', () => ({ default: layerMocks.MockLayer })); +vi.mock('../layers/AnnotationLayers/PointLayer', () => ({ default: layerMocks.MockLayer })); +vi.mock('../layers/AnnotationLayers/LineLayer', () => ({ default: layerMocks.MockLayer })); +vi.mock('../layers/AnnotationLayers/TailLayer', () => ({ default: layerMocks.MockLayer })); +vi.mock('../layers/AnnotationLayers/OverlapLayer', () => ({ default: layerMocks.MockLayer })); +vi.mock('../layers/AnnotationLayers/RegistrationKeypointLayer', () => ({ + default: layerMocks.MockLayer, +})); +vi.mock('../layers/EditAnnotationLayer', () => ({ default: layerMocks.MockLayer })); +vi.mock('../layers/LassoSelectionLayer', () => ({ default: layerMocks.MockLayer })); +vi.mock('../layers/AnnotationLayers/TextLayer', () => ({ default: layerMocks.MockLayer })); +vi.mock('../layers/AnnotationLayers/AttributeLayer', () => ({ default: layerMocks.MockLayer })); +vi.mock('../layers/AnnotationLayers/AttributeBoxLayer', () => ({ + default: layerMocks.MockLayer, +})); +vi.mock('../layers/AnnotationLayers/SegmentationPointsLayer', () => ({ + default: layerMocks.MockLayer, +})); +vi.mock('../layers/UILayers/UILayer', () => ({ default: layerMocks.MockLayer })); +vi.mock('../layers/UILayers/ToolTipWidget.vue', () => ({ default: {} })); + +vi.mock('./annotators/useMediaController', () => ({ + injectAggregateController: () => provided.values?.aggregateController, +})); + +vi.mock('../provides', () => ({ + useHandler: () => provided.values?.handler, + useSelectedTrackId: () => provided.values?.selectedTrackId, + useTrackFilters: () => provided.values?.trackFilters, + useTrackStyleManager: () => provided.values?.trackStyleManager, + useEditingMode: () => provided.values?.editingMode, + useVisibleModes: () => provided.values?.visibleModes, + useSelectedKey: () => provided.values?.selectedKey, + useMultiSelectList: () => provided.values?.multiSelectList, + useAnnotatorPreferences: () => provided.values?.annotatorPreferences, + useGroupStyleManager: () => provided.values?.groupStyleManager, + useCameraStore: () => provided.values?.cameraStore, + useCameraRegistration: () => { throw new Error('not provided'); }, + useAlignedView: () => { throw new Error('not provided'); }, + useSelectedCamera: () => provided.values?.selectedCamera, + useAttributes: () => provided.values?.attributes, + useComparisonSets: () => provided.values?.comparisonSets, + useLassoModeContext: () => ({ setLassoDrawing: vi.fn() }), + useSegmentationPoints: () => provided.values?.segmentationPoints, + usePendingSaveCount: () => provided.values?.pendingSaveCount, +})); + +vi.mock('./layerManager/useLayerManagerAlignedView', () => ({ + default: () => ({ + alignedDisplayTransform: ref(null), + alignedDisplayInverse: ref(null), + mapDisplayPoint: (x: number, y: number) => ({ x, y }), + mapNativePoint: (x: number, y: number) => [x, y], + mapEditGeoJSONToNative: (value: unknown) => value, + featureToDisplay: (value: unknown) => value, + setupDisplayTransformWatches: vi.fn(), + }), +})); + +vi.mock('./layerManager/useSegmentationPointsLayer', () => ({ default: vi.fn() })); +vi.mock('./layerManager/useAnnotationClickHandling', () => ({ + default: () => ({ wireHandlers: vi.fn() }), +})); + +/** + * `@vue/test-utils` types a mount target as a Vue 2 constructor, which a `defineComponent` + * SFC is not, so the manager renders from a host. It stays unstubbed to keep shallow + * semantics for its own children. + */ +function mountLayerManager(props: Record = {}) { + const Host = defineComponent({ + setup: () => () => h(LayerManager, { props }), + }); + return shallowMount(Host, { stubs: { LayerManager: false } }); +} + +describe('LayerManager hierarchy frame data', () => { + it('drops a track whose own vector selects no pair without hiding valid frame data', () => { + const track = new Track(1, { + confidencePairs: [], + features: [{ frame: 0, bounds: [0, 0, 1, 1], keyframe: true }], + }); + const validTrack = new Track(2, { + confidencePairs: [['leaf', 0.8]], + features: [{ frame: 0, bounds: [1, 1, 2, 2], keyframe: true }], + }); + const getType = vi.spyOn(track, 'getType'); + const validGetType = vi.spyOn(validTrack, 'getType'); + const trackStore = { + intervalTree: { search: vi.fn(() => ['1', '2']) }, + getPossible: vi.fn((id: number) => (id === 1 ? track : validTrack)), + }; + const annotator = { + frame: ref(0), + flick: ref(0), + hasFrame: ref(true), + imageRevision: ref(0), + geoViewerRef: ref({}), + transition: vi.fn(), + }; + provided.values = { + aggregateController: ref({ + getController: vi.fn(() => annotator), + resizeTrigger: ref(0), + }), + handler: {}, + selectedTrackId: ref(null), + trackFilters: { + enabledAnnotations: ref([{ + annotation: track, + context: { confidencePairIndex: -1 }, + }, { + annotation: validTrack, + context: { confidencePairIndex: 0 }, + }]), + hierarchyActive: ref(true), + displayPairIndex: (candidate: Track) => (candidate.confidencePairs.length ? 0 : -1), + }, + trackStyleManager: { + stateStyles: {}, + typeStyling: ref({ color: vi.fn(() => '#000000') }), + }, + editingMode: ref(false), + visibleModes: ref(['rectangle']), + selectedKey: ref('bounds'), + multiSelectList: ref([]), + annotatorPreferences: ref({ + lockedCamera: { enabled: false }, + suppressionDisplay: {}, + trackTails: { before: 0, after: 0 }, + }), + groupStyleManager: { + stateStyles: {}, + typeStyling: ref({ color: vi.fn(() => '#000000') }), + }, + cameraStore: { + camMap: ref(new Map([['singleCam', { trackStore, groupStore: {} }]])), + lookupGroups: vi.fn(() => []), + defaultGroup: ['unknown', 1], + }, + selectedCamera: ref('singleCam'), + attributes: ref([]), + comparisonSets: ref([]), + segmentationPoints: ref({ points: [], labels: [], frameNum: 0 }), + pendingSaveCount: ref(0), + }; + + expect(() => mountLayerManager()).not.toThrow(); + expect(layerMocks.rectangleChangeData).toHaveBeenCalled(); + layerMocks.rectangleChangeData.mock.calls.forEach(([frameData]) => { + expect(frameData).toHaveLength(1); + expect(frameData[0]).toMatchObject({ + track: validTrack, + styleType: ['leaf', 0.8], + }); + }); + expect(getType).not.toHaveBeenCalled(); + expect(validGetType).toHaveBeenCalledWith(0); + }); +}); + +const features: Feature[] = [{ + frame: 0, bounds: [0, 0, 10, 10], keyframe: true, +}]; + +function makeMultiCamFixture( + left: [string, number][], + right: [string, number][], + hierarchy: Record, +) { + const cameraStore = new CameraStore({ markChangesPending: () => undefined }); + cameraStore.removeCamera('singleCam'); + cameraStore.addCamera('left'); + cameraStore.addCamera('right'); + [['left', left], ['right', right]].forEach(([camera, pairs]) => { + const store = cameraStore.camMap.value.get(camera as string)?.trackStore; + store?.insert(new Track(1, { + confidencePairs: pairs as [string, number][], + features, + })); + store?.setEnableSorting(); + }); + const groupFilterControls = new GroupFilterControls({ + sorted: cameraStore.sortedGroups, + remove: () => undefined, + markChangesPending: () => undefined, + setType: () => undefined, + removeTypes: () => [], + }); + const trackFilters = new TrackFilterControls({ + sorted: cameraStore.sortedTracks, + remove: () => undefined, + markChangesPending: () => undefined, + lookupGroups: cameraStore.lookupGroups.bind(cameraStore), + getTrack: (id: AnnotationId, camera = 'left') => cameraStore.getTrack(id, camera), + getTracks: (id: AnnotationId) => cameraStore.getTrackAll(id), + groupFilterControls, + setType: () => undefined, + removeTypes: () => [], + }); + trackFilters.setTypeHierarchy(hierarchy); + return { cameraStore, trackFilters }; +} + +function renderCamera( + cameraStore: CameraStore, + trackFilters: TrackFilterControls, + camera: string, +) { + const annotator = { + frame: ref(0), + flick: ref(0), + hasFrame: ref(true), + imageRevision: ref(0), + geoViewerRef: ref({}), + transition: vi.fn(), + }; + provided.values = { + aggregateController: ref({ + getController: vi.fn(() => annotator), + resizeTrigger: ref(0), + }), + handler: {}, + selectedTrackId: ref(null), + trackFilters, + trackStyleManager: { + stateStyles: {}, + typeStyling: ref({ color: vi.fn(() => '#000000') }), + }, + editingMode: ref(false), + visibleModes: ref(['rectangle']), + selectedKey: ref('bounds'), + multiSelectList: ref([]), + annotatorPreferences: ref({ + lockedCamera: { enabled: false }, + suppressionDisplay: {}, + trackTails: { before: 0, after: 0 }, + }), + groupStyleManager: { + stateStyles: {}, + typeStyling: ref({ color: vi.fn(() => '#000000') }), + }, + cameraStore, + selectedCamera: ref('left'), + attributes: ref([]), + comparisonSets: ref([]), + segmentationPoints: ref({ points: [], labels: [], frameNum: 0 }), + pendingSaveCount: ref(0), + }; + layerMocks.rectangleChangeData.mockClear(); + mountLayerManager({ camera }); + const { calls } = layerMocks.rectangleChangeData.mock; + return calls[calls.length - 1][0] as { styleType: [string, number] }[]; +} + +describe('LayerManager multicamera hierarchy selection', () => { + it('renders each camera deepest qualifying pair instead of the merged index', () => { + const { cameraStore, trackFilters } = makeMultiCamFixture( + [['root', 0.9], ['leaf', 0.8]], + [['root', 0.9]], + { leaf: 'root' }, + ); + expect(renderCamera(cameraStore, trackFilters, 'left')[0].styleType).toEqual(['leaf', 0.8]); + expect(renderCamera(cameraStore, trackFilters, 'right')[0].styleType).toEqual(['root', 0.9]); + }); + + it('resolves a camera whose vector orders the same types differently', () => { + const { cameraStore, trackFilters } = makeMultiCamFixture( + [['fish', 0.9], ['shark', 0.4]], + [['shark', 0.8], ['fish', 0.3]], + { shark: 'fish' }, + ); + expect(renderCamera(cameraStore, trackFilters, 'right')[0].styleType).toEqual(['shark', 0.8]); + }); + + it('hides only the camera whose own vector passes no filter', () => { + const { cameraStore, trackFilters } = makeMultiCamFixture( + [['root', 0.9], ['leaf', 0.8]], + [['root', 0.1]], + { leaf: 'root' }, + ); + trackFilters.setConfidenceFilters({ default: 0.5 }); + expect(renderCamera(cameraStore, trackFilters, 'left')[0].styleType).toEqual(['leaf', 0.8]); + expect(renderCamera(cameraStore, trackFilters, 'right')).toHaveLength(0); + }); +}); diff --git a/client/src/components/LayerManager.vue b/client/src/components/LayerManager.vue index 3b03d3563..b464c19f5 100644 --- a/client/src/components/LayerManager.vue +++ b/client/src/components/LayerManager.vue @@ -105,7 +105,8 @@ export default defineComponent({ if (!trackStore || !groupStore) { throw Error(`TrackStore: ${trackStore} or GroupStore: ${groupStore} are undefined for camera ${props.camera}`); } - const enabledTracksRef = useTrackFilters().enabledAnnotations; + const trackFilters = useTrackFilters(); + const enabledTracksRef = trackFilters.enabledAnnotations; const selectedTrackIdRef = useSelectedTrackId(); const multiSeletListRef = useMultiSelectList(); const editingModeRef = useEditingMode(); @@ -386,11 +387,18 @@ export default defineComponent({ (trackWithContext) => trackWithContext.annotation.id === trackId, ); if (enabledIndex !== -1) { + // The context index addresses the merged cross-camera vector; a + // camera-local track resolves its own hierarchy pair. + let { confidencePairIndex } = enabledTracks[enabledIndex].context; + if (trackFilters.hierarchyActive.value) { + confidencePairIndex = trackFilters.displayPairIndex(track, 0); + if (confidencePairIndex < 0) { + return; + } + } const [features] = track.getFeature(frame); const groups = cameraStore.lookupGroups(track.id); - const trackStyleType = track.getType( - enabledTracks[enabledIndex].context.confidencePairIndex, - ); + const trackStyleType = track.getType(confidencePairIndex); const groupStyleType = groups?.[0]?.getType() ?? cameraStore.defaultGroup; // A detection flagged with the suppression attribute (it is NOT // under a region — those are hidden above) stays visible but is @@ -408,6 +416,7 @@ export default defineComponent({ groups, features, styleType, + trackStyleType, suppressed, set: track.set, }; @@ -550,6 +559,7 @@ export default defineComponent({ groups: cameraStore.lookupGroups(editTrack.id), features: (features && features.interpolate) ? features : null, styleType: cameraStore.defaultGroup, // Won't be used + trackStyleType: cameraStore.defaultGroup, // Won't be used }; editingTracks.push(trackFrame); } diff --git a/client/src/components/Tracks/TrackItem.vue b/client/src/components/Tracks/TrackItem.vue index 0ae7c714b..89b4c1258 100644 --- a/client/src/components/Tracks/TrackItem.vue +++ b/client/src/components/Tracks/TrackItem.vue @@ -23,6 +23,10 @@ export default defineComponent({ type: String, required: true, }, + displayPairIndex: { + type: Number, + required: true, + }, track: { type: Object as PropType, required: true, diff --git a/client/src/components/Tracks/TrackList.spec.ts b/client/src/components/Tracks/TrackList.spec.ts new file mode 100644 index 000000000..663826760 --- /dev/null +++ b/client/src/components/Tracks/TrackList.spec.ts @@ -0,0 +1,180 @@ +// @vitest-environment jsdom +/// +import { + defineComponent, h, nextTick, ref, Ref, +} from 'vue'; +import { shallowMount } from '@vue/test-utils'; +import Track from '../../track'; +import TrackList from './TrackList.vue'; + +interface MockCameraStore { + camMap: Ref>; + getTracksMerged: (id: number) => Track | undefined; + getAnyPossibleTrack: (id: number) => Track | undefined; +} + +interface MockTrackFilters { + allTypes: Ref; + checkedIDs: Ref; + filteredAnnotations: Ref<{ + annotation: ReturnType; + context: { confidencePairIndex: number }; + }[]>; + hierarchyActive: Ref; +} + +const state = vi.hoisted(() => ({ + cameraStore: null as unknown as MockCameraStore, + trackFilters: null as unknown as MockTrackFilters, +})); + +vi.mock('dive-common/vue-utilities/prompt-service', () => ({ + usePrompt: () => ({ prompt: vi.fn() }), +})); + +vi.mock('../../use/useVirtualScrollTo', () => ({ + default: () => ({ virtualList: ref(null), scrollPreventDefault: vi.fn() }), +})); + +vi.mock('../../provides', () => ({ + useEditingMode: () => ref(false), + useHandler: () => ({ + trackSplit: vi.fn(), + removeTrack: vi.fn(), + trackAdd: vi.fn(), + trackSelect: vi.fn(), + trackSelectNext: vi.fn(), + }), + useSelectedTrackId: () => ref(null), + useTrackFilters: () => state.trackFilters, + useTime: () => ({ frame: ref(0), isPlaying: ref(false) }), + useReadOnlyMode: () => ref(false), + useTrackStyleManager: () => ({ + typeStyling: ref({ color: (type: string) => `color:${type}` }), + }), + useMultiSelectList: () => ref([]), + useCameraStore: () => state.cameraStore, + useSelectedCamera: () => ref('singleCam'), + usePendingSaveCount: () => ref(0), +})); + +function sortedTrack(track: Track) { + return { + id: track.id, + begin: track.begin, + end: track.end, + confidencePairs: track.confidencePairs, + getType: (index = 0) => track.confidencePairs[index][0], + }; +} + +function mountList( + tracks: Track[], + contextIndexes: number[], + hierarchyActive = true, + filtered = tracks.map((track, index) => ({ + annotation: sortedTrack(track), + context: { confidencePairIndex: contextIndexes[index] }, + })), +) { + const byId = new Map(tracks.map((track) => [track.id, track])); + state.trackFilters = { + allTypes: ref(['root', 'child', 'leaf']), + checkedIDs: ref(tracks.map(({ id }) => id)), + filteredAnnotations: ref(filtered), + hierarchyActive: ref(hierarchyActive), + }; + state.cameraStore = { + camMap: ref(new Map([['singleCam', { trackStore: undefined }]])), + getTracksMerged: (id: number) => byId.get(id), + getAnyPossibleTrack: (id: number) => byId.get(id), + }; + // `@vue/test-utils` types a mount target as a Vue 2 constructor, which a `defineComponent` + // SFC is not, so the list renders from a host that captures the real instance. It stays + // unstubbed to keep shallow semantics for its own children. + let child: InstanceType | undefined; + const Host = defineComponent({ + setup: () => () => h(TrackList, { + props: { + compact: true, + newTrackMode: 'Track', + newTrackType: 'unknown', + hotkeysDisabled: false, + }, + ref: (instance) => { + if (instance && !(instance instanceof Element)) { + child = instance as InstanceType; + } + }, + }), + }); + const wrapper = shallowMount(Host, { stubs: { TrackList: false } }); + if (!child) { + throw new Error('TrackList did not mount'); + } + return { wrapper, vm: child }; +} + +describe('TrackList hierarchy display', () => { + it.each([ + ['monotone leaf', [['root', 0.9], ['child', 0.8], ['leaf', 0.7]], 2, 'leaf'], + ['non-monotone leaf', [['root', 0.2], ['child', 0.9], ['leaf', 0.6]], 2, 'leaf'], + ['unchecked-leaf roll-up', [['root', 0.9], ['child', 0.8], ['leaf', 0.7]], 1, 'child'], + ] as [string, [string, number][], number, string][])( + 'passes the context-selected type, confidence index, and color for %s', + (_name, pairs, pairIndex, expectedType) => { + const track = new Track(1, { + confidencePairs: pairs, + features: [{ frame: 0, bounds: [0, 0, 1, 1], keyframe: true }], + }); + const { wrapper } = mountList([track], [pairIndex]); + const listView = wrapper.findComponent({ name: 'BottomBarTrackListView' }); + const items = listView.props('virtualListItems') as unknown[]; + const getItemProps = listView.props('getItemProps') as (item: unknown) => Record; + expect(getItemProps(items[0])).toMatchObject({ + trackType: expectedType, + displayPairIndex: pairIndex, + color: `color:${expectedType}`, + }); + }, + ); + + it('sorts hierarchy confidence by the context pair and flat confidence by pair zero', async () => { + const first = new Track(1, { + confidencePairs: [['root', 0.9], ['leaf', 0.4]], + features: [{ frame: 0, bounds: [0, 0, 1, 1], keyframe: true }], + }); + const second = new Track(2, { + confidencePairs: [['root', 0.5], ['leaf', 0.8]], + features: [{ frame: 0, bounds: [0, 0, 1, 1], keyframe: true }], + }); + const { wrapper, vm } = mountList([first, second], [1, 1]); + vm.handleSort('confidence'); + expect(vm.filteredTracks.map(({ annotation }) => annotation.id)).toEqual([2, 1]); + + state.trackFilters.hierarchyActive.value = false; + await nextTick(); + expect(vm.filteredTracks.map(({ annotation }) => annotation.id)).toEqual([1, 2]); + const listView = wrapper.findComponent({ name: 'BottomBarTrackListView' }); + const items = listView.props('virtualListItems') as unknown[]; + const getItemProps = listView.props('getItemProps') as (item: unknown) => Record; + expect(getItemProps(items[0])).toMatchObject({ displayPairIndex: 0 }); + }); + + it('renders no row or fake type for an excluded empty hierarchy vector', () => { + const track = new Track(1, { + confidencePairs: [], + features: [{ frame: 0, bounds: [0, 0, 1, 1], keyframe: true }], + }); + const annotation = sortedTrack(track); + const getType = vi.spyOn(annotation, 'getType'); + + const { wrapper } = mountList([track], [-1], true, [{ + annotation, + context: { confidencePairIndex: -1 }, + }]); + const listView = wrapper.findComponent({ name: 'BottomBarTrackListView' }); + expect(listView.props('virtualListItems')).toEqual([]); + expect(getType).not.toHaveBeenCalled(); + }); +}); diff --git a/client/src/components/Tracks/TrackList.vue b/client/src/components/Tracks/TrackList.vue index 8a83cded3..40d8a4a00 100644 --- a/client/src/components/Tracks/TrackList.vue +++ b/client/src/components/Tracks/TrackList.vue @@ -104,6 +104,14 @@ export default defineComponent({ const sortKey = ref('id'); const sortDirection = ref('asc'); + const displayConfidence = ( + track: ReturnType, + contextIndex: number, + ) => { + const pairIndex = trackFilters.hierarchyActive.value ? contextIndex : 0; + return track.confidencePairs[pairIndex]?.[1] ?? 0; + }; + const filterDetectionsByFrame = ref(clientSettings.trackSettings.trackListSettings.filterDetectionsByFrame); watch( () => clientSettings.trackSettings.trackListSettings.filterDetectionsByFrame, @@ -114,6 +122,11 @@ export default defineComponent({ const finalFilteredTracks = computed(() => { let tracks = filteredTracksRef.value; + if (trackFilters.hierarchyActive.value) { + tracks = tracks.filter(({ annotation, context }) => ( + annotation.confidencePairs[context.confidencePairIndex] !== undefined + )); + } if (filterDetectionsByFrame.value && !isPlaying.value) { // Depend on the edit counter so moving a suppression region re-runs the // filter (geometry mutations are not reactive track-set changes). @@ -228,8 +241,8 @@ export default defineComponent({ case 'endTime': return (trackA.end - trackB.end) * direction; case 'confidence': { - const confA = trackA.confidencePairs?.[0]?.[1] ?? 0; - const confB = trackB.confidencePairs?.[0]?.[1] ?? 0; + const confA = displayConfidence(trackA, a.context.confidencePairIndex); + const confB = displayConfidence(trackB, b.context.confidencePairIndex); return (confA - confB) * direction; } case 'type': { @@ -238,8 +251,8 @@ export default defineComponent({ const typeCompare = typeA.localeCompare(typeB); if (typeCompare !== 0) return typeCompare * direction; // Secondary sort by confidence within same type - const confA = trackA.confidencePairs?.[0]?.[1] ?? 0; - const confB = trackB.confidencePairs?.[0]?.[1] ?? 0; + const confA = displayConfidence(trackA, a.context.confidencePairIndex); + const confB = displayConfidence(trackB, b.context.confidencePairIndex); return (confA - confB) * direction; } case 'notes': { @@ -320,6 +333,9 @@ export default defineComponent({ editing: selected && item.editingTrack, color: typeStylingRef.value.color(trackType), types: item.allTypes, + displayPairIndex: trackFilters.hierarchyActive.value + ? item.filteredTrack.context.confidencePairIndex + : 0, }; } diff --git a/client/src/components/Tracks/bottombar/BottomBarTrackItemView.spec.ts b/client/src/components/Tracks/bottombar/BottomBarTrackItemView.spec.ts new file mode 100644 index 000000000..c0c24a28a --- /dev/null +++ b/client/src/components/Tracks/bottombar/BottomBarTrackItemView.spec.ts @@ -0,0 +1,69 @@ +// @vitest-environment jsdom +/// +import { defineComponent, h, ref } from 'vue'; +import { shallowMount } from '@vue/test-utils'; +import Track from '../../../track'; +import BottomBarTrackItemView from './BottomBarTrackItemView.vue'; + +const providerState = vi.hoisted(() => ({ setTrackType: vi.fn() })); + +vi.mock('../../../provides', () => ({ + useHandler: () => ({ trackSeek: vi.fn(), removeTrack: vi.fn(), trackEdit: vi.fn() }), + useReadOnlyMode: () => ref(false), + useTrackFilters: () => ({ allTypes: ref(['root', 'leaf']) }), + useCameraStore: () => ({ setTrackType: providerState.setTrackType }), +})); + +function mountItem(displayPairIndex: number) { + const track = new Track(1, { + confidencePairs: [['root', 0.9], ['leaf', 0.7]], + features: [{ frame: 0, bounds: [0, 0, 1, 1], keyframe: true }], + }); + Object.preventExtensions(track); + // `@vue/test-utils` types a mount target as a Vue 2 constructor, which a `defineComponent` + // SFC is not, so the row renders from a host and stays unstubbed to keep shallow semantics + // for its own children. + let child: InstanceType | undefined; + const Host = defineComponent({ + setup: () => () => h(BottomBarTrackItemView, { + ref: (instance) => { + if (instance && !(instance instanceof Element)) { + child = instance as InstanceType; + } + }, + props: { + track, + trackType: displayPairIndex === 1 ? 'leaf' : 'root', + displayPairIndex, + itemStyle: {}, + color: displayPairIndex === 1 ? '#leaf' : '#root', + editing: false, + inputValue: true, + toggleKeyframe: vi.fn(), + toggleInterpolation: vi.fn(), + toggleAllInterpolation: vi.fn(), + }, + }), + }); + const wrapper = shallowMount(Host, { stubs: { BottomBarTrackItemView: false } }); + if (!child) { + throw new Error('BottomBarTrackItemView did not mount'); + } + return { wrapper, vm: child, props: child.$props }; +} + +describe('BottomBarTrackItemView hierarchy display', () => { + it('renders and seeds editing from the selected hierarchy pair', () => { + const { wrapper, vm } = mountItem(1); + expect(wrapper.find('.track-type-compact').text()).toBe('leaf'); + expect(wrapper.find('.track-confidence-compact').text()).toBe('0.70'); + vm.startEditConfidence(new MouseEvent('click')); + expect(vm.editConfidenceValue).toBe('0.70'); + }); + + it('retains pair-zero type and confidence in flat mode', () => { + const { wrapper } = mountItem(0); + expect(wrapper.find('.track-type-compact').text()).toBe('root'); + expect(wrapper.find('.track-confidence-compact').text()).toBe('0.90'); + }); +}); diff --git a/client/src/components/Tracks/bottombar/BottomBarTrackItemView.vue b/client/src/components/Tracks/bottombar/BottomBarTrackItemView.vue index ef088ef34..382de7120 100644 --- a/client/src/components/Tracks/bottombar/BottomBarTrackItemView.vue +++ b/client/src/components/Tracks/bottombar/BottomBarTrackItemView.vue @@ -18,6 +18,7 @@ export default defineComponent({ props: { track: { type: Object as PropType, required: true }, trackType: { type: String, required: true }, + displayPairIndex: { type: Number, required: true }, itemStyle: { type: Object, required: true }, color: { type: String, required: true }, lockTypes: { type: Boolean, default: false }, @@ -61,7 +62,7 @@ export default defineComponent({ if (props.track.revision.value !== undefined && props.track.confidencePairs && props.track.confidencePairs.length > 0) { - return props.track.confidencePairs[0][1]; + return props.track.confidencePairs[props.displayPairIndex]?.[1] ?? null; } return null; }); diff --git a/client/src/components/TypeEditor.spec.ts b/client/src/components/TypeEditor.spec.ts new file mode 100644 index 000000000..561610c5d --- /dev/null +++ b/client/src/components/TypeEditor.spec.ts @@ -0,0 +1,137 @@ +// @vitest-environment jsdom +/// +import { defineComponent, h, ref } from 'vue'; +import { shallowMount } from '@vue/test-utils'; +import { TypeHierarchyError } from 'dive-common/typeHierarchy'; +import TrackFilterControls from '../TrackFilterControls'; +import TypeEditor from './TypeEditor.vue'; + +const promptMock = vi.hoisted(() => vi.fn()); + +vi.mock('dive-common/vue-utilities/prompt-service', () => ({ + usePrompt: () => ({ prompt: promptMock }), +})); + +vi.mock('../provides', () => ({ + useReadOnlyMode: () => ref(false), +})); + +function makeFilters() { + const filters = Object.create(TrackFilterControls.prototype) as TrackFilterControls; + filters.usedTypes = ref([]); + filters.typeInUseOnAnyCamera = vi.fn(() => false); + filters.updateTypeName = vi.fn(); + filters.importTypes = vi.fn(); + filters.deleteType = vi.fn(() => true); + return filters; +} + +function makeStyleManager() { + return Object.freeze({ + typeStyling: ref({ + color: () => '#123456', + strokeWidth: () => 3, + fill: () => false, + opacity: () => 0.8, + labelSettings: () => ({ showLabel: true, showConfidence: true }), + }), + updateTypeStyle: vi.fn(), + }); +} + +/** + * `@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()) { + const closeEvents: unknown[] = []; + let child: InstanceType | undefined; + const Host = defineComponent({ + setup: () => () => h(TypeEditor, { + props: { + selectedType: 'leaf', + filterControls: filters, + styleManager, + }, + on: { close: () => closeEvents.push([]) }, + ref: (instance) => { + if (instance && !(instance instanceof Element)) { + child = instance as InstanceType; + } + }, + }), + }); + const wrapper = shallowMount(Host, { stubs: { TypeEditor: false } }); + if (!child) { + throw new Error('TypeEditor did not mount'); + } + return { + filters, styleManager, wrapper, vm: child, closeEvents, + }; +} + +describe('TypeEditor hierarchy safety', () => { + beforeEach(() => promptMock.mockReset()); + + 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('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('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(); + expect(closeEvents).toHaveLength(0); + }); + + it('keeps the editor open for a rejected rename and allows a corrected retry', () => { + const { + filters, styleManager, vm, closeEvents, + } = mountEditor(); + vi.mocked(filters.updateTypeName).mockImplementationOnce(() => { + throw new TypeHierarchyError('self edge "root -> root"', 'conflict'); + }); + vm.data.editingType = 'root'; + vm.acceptChanges(); + expect(vm.data.renameError).toBe( + 'Type hierarchy is invalid: self edge "root -> root". No types were changed.', + ); + 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', + }); + expect(closeEvents).toHaveLength(1); + }); + + it('promotes a hierarchy-only heading only after a style value changes', () => { + const { filters, vm } = mountEditor(); + vm.acceptChanges(); + expect(filters.importTypes).not.toHaveBeenCalled(); + + const changed = mountEditor(); + changed.vm.data.editingColor = '#abcdef'; + changed.vm.acceptChanges(); + expect(changed.filters.importTypes).toHaveBeenCalledWith(['leaf'], false); + }); +}); diff --git a/client/src/components/TypeEditor.vue b/client/src/components/TypeEditor.vue index 1f6bfba83..89885ec40 100644 --- a/client/src/components/TypeEditor.vue +++ b/client/src/components/TypeEditor.vue @@ -1,16 +1,17 @@