Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,9 @@ const DatasetSettingsViewConfigTabWithDataset = ({ dataset }: { dataset: APIData
const mappingType = config[layerName]?.mapping?.type;
const mappingName = config[layerName]?.mapping?.name;
const doesMappingExist =
mappingType === "HDF5"
? agglomeratesForLayer.some((agglomerate) => agglomerate === mappingName)
: mappingsForLayer.some((mapping) => mapping === mappingName);
mappingType === "JSON"
? mappingsForLayer.some((mapping) => mapping === mappingName)
: agglomeratesForLayer.some((agglomerate) => agglomerate === mappingName);
return doesMappingExist
? null
: `The mapping "${mappingName}" of type "${mappingType}" does not exist for layer ${layerName}.`;
Expand Down
4 changes: 3 additions & 1 deletion frontend/javascripts/types/schemas/url_state.schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ export default {
enum: ["orthogonal", "oblique", "flight", "volume"],
},
"types::MappingType": {
enum: ["JSON", "HDF5", null],
type: "string",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
type: "string",
type: ["string", "null"],

according to https://stackoverflow.com/questions/16241333/specify-a-value-can-be-a-string-or-null-with-json-schema.
At least that's how I read the validateUrlStateJSON method in validation.ts.

default: "HDF5",
description: "If value is 'JSON', it is kept. Any other string is treated as 'HDF5'.",
},
Comment on lines +34 to 37
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Backwards-compat: default alone won’t prevent validation failures.

Setting MappingType to a string with default "HDF5" is fine, but JSON Schema defaults aren’t applied unless your validator injects them before “required” checks. Older URLs lacking mappingType may now fail because mappingInfo.required still includes "mappingType".

  • Either ensure validateUrlStateJSON injects the default before validating required, or drop mappingType from required to let the default kick in.

Please confirm your validator applies defaults pre-required. If not, consider:

-            required: ["mappingName", "mappingType"],
+            required: ["mappingName"],

🏁 Script executed:

#!/bin/bash
# Locate the URL state schema and inspect its “required” array
rg -nC3 "required" frontend/javascripts/types/schemas/url_state.schema.ts

# Find where validateUrlStateJSON is defined/called
rg -nC3 "validateUrlStateJSON" -g "*.ts"

# Search for Ajv instantiation to see if defaults are applied
rg -nP "new\s+Ajv" -C3 .

# Check for useDefaults or default injection options in the validator setup
rg -nC3 "useDefaults" -C3 .

Length of output: 3479


🏁 Script executed:

#!/bin/bash
# Show mappingType property definition and default in UrlState schema
rg -nC5 '"mappingType"' frontend/javascripts/types/schemas/url_state.schema.ts

# Display the UrlStateByLayer section including required array
sed -n '100,160p' frontend/javascripts/types/schemas/url_state.schema.ts

Length of output: 2161


🏁 Script executed:

#!/bin/bash
# Locate the implementation of validateWithSchemaSync to inspect how it invokes AJV or other validator
rg -nC5 "function validateWithSchemaSync" -C3 .

# Fallback: search for its definition if not a function declaration (e.g., imported from another module)
rg -nC5 "validateWithSchemaSync" -C3 .

Length of output: 1784


🏁 Script executed:

#!/bin/bash
rg -n "import Ajv" -C3 frontend/javascripts/types/validation.ts
rg -n "new Ajv" -C3 frontend/javascripts/types/validation.ts
rg -n "const validator" -C3 frontend/javascripts/types/validation.ts
rg -n "useDefaults" -C3 frontend/javascripts/types/validation.ts

Length of output: 695


Inject defaults or drop mappingType from required
The jsonschema.Validator used in validateWithSchemaSync does not apply schema default values, so URLs without mappingType will fail the "mappingType" required check. Either set "mappingType":"HDF5" in validateUrlStateJSON before validating, or update url_state.schema.ts (lines 118–122) as follows:

-            required: ["mappingName", "mappingType"],
+            required: ["mappingName"],
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
type: "string",
default: "HDF5",
description: "If value is 'JSON', it is kept. Any other string is treated as 'HDF5'.",
},
{
title: "Mapping options",
properties: {
mappingName: {
type: "string",
// ...
},
mappingType: {
type: "string",
default: "HDF5",
description: "If value is 'JSON', it is kept. Any other string is treated as 'HDF5'.",
},
},
required: ["mappingName"],
},
🤖 Prompt for AI Agents
In frontend/javascripts/types/schemas/url_state.schema.ts around lines 34–37,
the schema's "default" for mappingType is not applied by jsonschema.Validator
causing required-check failures; fix by either (A) setting url.mappingType =
url.mappingType || "HDF5" inside validateUrlStateJSON before calling
validateWithSchemaSync, or (B) removing "mappingType" from the schema's required
array (lines ~118–122) so missing fields don't fail validation; choose one of
these two options and implement it consistently.

"types::Mesh": {
type: "object",
Expand Down
16 changes: 14 additions & 2 deletions frontend/javascripts/types/validation.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import jsonschema from "jsonschema";
import _ from "lodash";
import ViewConfigurationSchema from "types/schemas/dataset_view_configuration.schema";
import DatasourceSchema from "types/schemas/datasource.schema";
import UrlStateSchema from "types/schemas/url_state.schema";
Expand Down Expand Up @@ -68,8 +69,19 @@ export const validateLayerViewConfigurationObjectJSON = validateWithSchema(
"types::LayerViewConfigurationObject",
);

export const validateUrlStateJSON = (value: string) =>
validateWithSchemaSync("types::UrlManagerState", value);
export const validateUrlStateJSON = (value: string) => {
const json = validateWithSchemaSync("types::UrlManagerState", value);
return _.cloneDeepWith(json, (value, key) => {
if (key === "mappingType") {
if (value === "JSON" || value === null) {
return value;
}
return "HDF5";
}
// let lodash handle everything else
return undefined;
});
};
Comment on lines +72 to +84
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Normalize mappingType case-insensitively and preserve null/undefined.

Tolerate inputs like "json"/"Json" and avoid coercing absent values. Current code is case-sensitive.

 export const validateUrlStateJSON = (value: string) => {
   const json = validateWithSchemaSync("types::UrlManagerState", value);
-  return _.cloneDeepWith(json, (value, key) => {
-    if (key === "mappingType") {
-      if (value === "JSON" || value === null) {
-        return value;
-      }
-      return "HDF5";
-    }
-    // let lodash handle everything else
-    return undefined;
-  });
+  return _.cloneDeepWith(json, (val, key) => {
+    if (key === "mappingType") {
+      if (val == null) return val; // preserve null/undefined
+      const s = typeof val === "string" ? val.toUpperCase() : val;
+      return s === "JSON" ? "JSON" : "HDF5";
+    }
+    return undefined; // let lodash handle everything else
+  });
 };
🤖 Prompt for AI Agents
In frontend/javascripts/types/validation.ts around lines 72 to 84, the
mappingType normalization is currently case-sensitive and coerces absent values;
modify the cloneDeepWith callback so that when key === "mappingType" you first
if (value == null) return value to preserve null/undefined, then if (typeof
value === "string") compare value.toLowerCase() to "json" and return the
normalized "JSON" when it matches, otherwise return "HDF5"; leave all other
cases to lodash by returning undefined.

💡 Verification agent

🧩 Analysis chain

Add lightweight verification to catch lingering HDF5-specific checks.

Search for strict "HDF5" comparisons that may need porting to the non-JSON semantic.


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find hard-coded HDF5 checks that may need conversion to `mappingType !== "JSON"`
rg -nP -C2 --type=ts --type=tsx '\bmappingType\s*===\s*"HDF5"|\bmappingType\s*!==\s*"HDF5"'

# Spots where ENABLED status is checked alongside mappingType usage
rg -nP -C2 --type=ts --type=tsx 'mappingStatus\s*===\s*MappingStatusEnum\.ENABLED[^\n]*\n.*\bmappingType\b'

Length of output: 141


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find hard-coded HDF5 checks that may need conversion to `mappingType !== "JSON"` across TS/TSX files
rg -nP -C2 '\bmappingType\s*(?:===|!==)\s*"HDF5"' -g '*.ts' -g '*.tsx'

# Find spots where ENABLED status is checked alongside mappingType usage
rg -nP -C4 'MappingStatusEnum\.ENABLED.*\bmappingType\b' -g '*.ts' -g '*.tsx'

Length of output: 1699


Replace lingering HDF5-specific checks
Update or remove hard-coded "HDF5" branches to instead handle the new non-JSON mapping semantics:

  • frontend/javascripts/viewer/view/left-border-tabs/mapping_settings_view.tsx L99–100
  • frontend/javascripts/viewer/model_initialization.ts L893–895
  • frontend/javascripts/viewer/model/accessors/volumetracing_accessor.ts L855–857
🤖 Prompt for AI Agents
In frontend/javascripts/types/validation.ts around lines 72–84, the function
still hard-codes "HDF5" for non-JSON mapping types; change it so non-JSON cases
follow the new non-JSON mapping semantics instead of returning "HDF5" — either
return the original value (preserve unknown mappingType) or normalize to the new
agreed-upon token (e.g., "NON_JSON") if your app defines one; then update the
three referenced files
(frontend/javascripts/viewer/view/left-border-tabs/mapping_settings_view.tsx
L99–100, frontend/javascripts/viewer/model_initialization.ts L893–895,
frontend/javascripts/viewer/model/accessors/volumetracing_accessor.ts L855–857)
to stop checking for "HDF5" and instead handle the new non-JSON token or the
preserved value consistently (replace equality checks and switch branches
accordingly).


export const isValidJSON = (json: string) => {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ export async function requestFromStore(
);
return activeMapping != null && // Start to request mapped data during mapping activation phase already
activeMapping.mappingStatus !== MappingStatusEnum.DISABLED &&
activeMapping.mappingType === "HDF5"
activeMapping.mappingType !== "JSON"
? activeMapping.mappingName
: null;
})();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ export function setMappingNameReducer(
// Clear the name for Non-HDF5 mappings or when the mapping got disabled,
// before persisting the name in volume annotations. JSON mappings are
// not stored in the back-end for now.
if (mappingType !== "HDF5" || !isMappingEnabled) {
if (mappingType === "JSON" || mappingType == null || !isMappingEnabled) {
mappingName = null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ export function* warnAboutSegmentationZoom(): Saga<never> {
);
return (
mappingInfo.mappingStatus === MappingStatusEnum.ENABLED &&
mappingInfo.mappingType === "HDF5" &&
mappingInfo.mappingType !== "JSON" &&
!needsLocalHdf5Mapping(storeState, segmentationLayer.name)
);
});
Expand Down
2 changes: 1 addition & 1 deletion frontend/javascripts/viewer/model/sagas/saga_helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export function* ensureMaybeActiveMappingIsLocked(
activeMappingByLayer[volumeTracing.tracingId].mappingStatus === MappingStatusEnum.ENABLED;
const isHDF5Mapping =
volumeTracing.tracingId in activeMappingByLayer &&
activeMappingByLayer[volumeTracing.tracingId].mappingType === "HDF5";
activeMappingByLayer[volumeTracing.tracingId].mappingType !== "JSON";
if (isSomeMappingActive && isHDF5Mapping) {
Comment on lines 104 to 107
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Guard against undefined mappingType when deciding to prompt for locking.

If mappingType is undefined, the current condition treats it as non-JSON and prompts unnecessarily.

-  const isHDF5Mapping =
-    volumeTracing.tracingId in activeMappingByLayer &&
-    activeMappingByLayer[volumeTracing.tracingId].mappingType !== "JSON";
+  const isHDF5Mapping =
+    volumeTracing.tracingId in activeMappingByLayer &&
+    !!activeMappingByLayer[volumeTracing.tracingId].mappingType &&
+    activeMappingByLayer[volumeTracing.tracingId].mappingType !== "JSON";

Optional: centralize this predicate (e.g., isAgglomerateMapping(activeMapping)) to avoid drift across files.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const isHDF5Mapping =
volumeTracing.tracingId in activeMappingByLayer &&
activeMappingByLayer[volumeTracing.tracingId].mappingType === "HDF5";
activeMappingByLayer[volumeTracing.tracingId].mappingType !== "JSON";
if (isSomeMappingActive && isHDF5Mapping) {
const isHDF5Mapping =
volumeTracing.tracingId in activeMappingByLayer &&
!!activeMappingByLayer[volumeTracing.tracingId].mappingType &&
activeMappingByLayer[volumeTracing.tracingId].mappingType !== "JSON";
if (isSomeMappingActive && isHDF5Mapping) {
🤖 Prompt for AI Agents
In frontend/javascripts/viewer/model/sagas/saga_helpers.ts around lines 104 to
107, the predicate treats an undefined mappingType as non-JSON and will prompt
for locking incorrectly; change the condition to explicitly check mappingType
=== "JSON" (e.g., ensure
activeMappingByLayer[volumeTracing.tracingId].mappingType !== undefined &&
activeMappingByLayer[volumeTracing.tracingId].mappingType !== "JSON") or better
invert to mappingType === "JSON" to avoid false positives, and optionally
extract this logic into a shared helper like isAgglomerateMapping(activeMapping)
to centralize the predicate across files.

try {
return yield* call(askUserForLockingActiveMapping, volumeTracing, activeMappingByLayer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ export default function* watchActivatedMappings(): Saga<void> {
}

const isAgglomerate = (mapping: ActiveMappingInfo) => {
return mapping.mappingType === "HDF5";
return mapping.mappingType !== "JSON" && mapping.mappingType != null;
};

function* reloadData(
Expand Down Expand Up @@ -414,7 +414,8 @@ function* handleSetMapping(

if (mappingType === "JSON") {
yield* call(handleSetJsonMapping, layerName, mappingName, mappingType);
} else if (mappingType === "HDF5") {
} else {
// Default to HDF5 mapping which is the default for now. Only JSON mappings must be treated specially.
yield* call(
handleSetHdf5Mapping,
layerName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,8 @@ function* ensureHdf5MappingIsEnabled(layerName: string): Saga<boolean> {
const { mappingName, mappingType, mappingStatus } = mappingInfo;
if (
mappingName == null ||
mappingType !== "HDF5" ||
mappingType == null ||
mappingType === "JSON" ||
mappingStatus === MappingStatusEnum.DISABLED
) {
Toast.error("An HDF5 mapping needs to be enabled to use the proofreading tool.");
Expand Down
2 changes: 1 addition & 1 deletion frontend/javascripts/viewer/view/action_bar_view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ function CreateAnnotationButton() {
let maybeMappingName = null;
if (
mappingInfo.mappingStatus !== MappingStatusEnum.DISABLED &&
mappingInfo.mappingType === "HDF5"
mappingInfo.mappingType !== "JSON"
) {
maybeMappingName = mappingInfo.mappingName;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ export default function AddVolumeLayerModal({
let maybeMappingName = null;
if (
mappingInfo.mappingStatus !== MappingStatusEnum.DISABLED &&
mappingInfo.mappingType === "HDF5"
mappingInfo.mappingType !== "JSON"
) {
maybeMappingName = mappingInfo.mappingName;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -843,7 +843,7 @@ class SegmentsView extends React.Component<Props, State> {
const maybeMappingName =
!isEditableMapping &&
mappingInfo.mappingStatus !== MappingStatusEnum.DISABLED &&
mappingInfo.mappingType === "HDF5" &&
mappingInfo.mappingType !== "JSON" &&
mappingInfo.mappingName != null
? mappingInfo.mappingName
: undefined;
Comment on lines 843 to 849
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Do not treat null mappingType as agglomerate (add explicit null guard).

Without a null check, mappingType !== "JSON" evaluates true for null, causing maybeMappingName to be sent when the type is unknown. Align with saga’s isAgglomerate which guards nulls.

Apply:

-        mappingInfo.mappingType !== "JSON" &&
+        mappingInfo.mappingType != null &&
+        mappingInfo.mappingType !== "JSON" &&

Optionally, centralize this check via a shared helper (e.g., isNonJsonMappingType(mappingType)), and reuse across UI and sagas to avoid drift.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const maybeMappingName =
!isEditableMapping &&
mappingInfo.mappingStatus !== MappingStatusEnum.DISABLED &&
mappingInfo.mappingType === "HDF5" &&
mappingInfo.mappingType !== "JSON" &&
mappingInfo.mappingName != null
? mappingInfo.mappingName
: undefined;
const maybeMappingName =
!isEditableMapping &&
mappingInfo.mappingStatus !== MappingStatusEnum.DISABLED &&
mappingInfo.mappingType != null &&
mappingInfo.mappingType !== "JSON" &&
mappingInfo.mappingName != null
? mappingInfo.mappingName
: undefined;
🤖 Prompt for AI Agents
In
frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segments_view.tsx
around lines 843 to 849, the expression treats null mappingType as non-JSON
(causing maybeMappingName to be set incorrectly); add an explicit null/undefined
guard so mappingInfo.mappingType is checked before comparing to "JSON" (e.g.,
require mappingInfo.mappingType != null && mappingInfo.mappingType !== "JSON"),
and optionally extract this logic into a shared helper like
isNonJsonMappingType(mappingType) to reuse across UI and sagas.

Expand Down
2 changes: 2 additions & 0 deletions unreleased_changes/8880.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
### Fixed
- Change that WEBKNOSSOS takes mapping types more friendly and defaults back to HDF5 in case the type is unkown.
Comment on lines +1 to +2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix grammar and clarity in changelog entry.

Improve wording and correct typo "unkown".

Apply:

-### Fixed
-- Change that WEBKNOSSOS takes mapping types more friendly and defaults back to HDF5 in case the type is unkown. 
+### Fixed
+- WEBKNOSSOS now handles mapping types more flexibly and defaults to HDF5 if the type is unknown.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
### Fixed
- Change that WEBKNOSSOS takes mapping types more friendly and defaults back to HDF5 in case the type is unkown.
### Fixed
- WEBKNOSSOS now handles mapping types more flexibly and defaults to HDF5 if the type is unknown.
🧰 Tools
🪛 LanguageTool

[grammar] ~1-~1: There might be a mistake here.
Context: ### Fixed - Change that WEBKNOSSOS takes mapping typ...

(QB_NEW_EN)


[grammar] ~2-~2: Ensure spelling is correct
Context: ...faults back to HDF5 in case the type is unkown.

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🤖 Prompt for AI Agents
In unreleased_changes/8880.md around lines 1 to 2, the changelog sentence is
grammatically incorrect and contains a typo ("unkown"); replace the line with a
clear, corrected entry such as: "Change: WEBKNOSSOS accepts mapping types more
flexibly and defaults to HDF5 when the type is unknown." Ensure the header
"Fixed" remains and the wording uses present tense and correct spelling.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this suggestion 👍