feat: generate viewer links dynamically for Zarr datasets#307
feat: generate viewer links dynamically for Zarr datasets#307allison-truhlar wants to merge 51 commits intomainfrom
Conversation
- Custom viewers (validator, vol-e) now check for multiscales - Ensures viewers only display for OME-Zarr datasets, not plain Zarr arrays - Update DataToolLinks alt text to use displayName for E2E test compatibility
Add useCallback to memoize the getCompatibleViewers function in ViewersContext to prevent unnecessary recalculations and improve performance when filtering compatible viewers based on metadata.
Add more detailed error logging and ensure graceful degradation when: - Capability manifests fail to load - Viewer configuration parsing fails - No valid viewers are configured The application will continue with an empty viewer list and clear console messages to help users troubleshoot configuration issues.
neomorphic
left a comment
There was a problem hiding this comment.
Looks good to me. I was able to enable and disable a viewer using the viewers.config.yaml. I added a few comments about some the code I wasn't sure on.
| export type N5OpenWithToolUrls = { | ||
| copy: string; | ||
| neuroglancer: string; | ||
| validator: null; |
There was a problem hiding this comment.
I don't know what this data structure is doing? Why did three of the viewers get removed, but not neuroglancer?
There was a problem hiding this comment.
This is only tangentially related to the purpose of this PR - when the support for N5 metadata was added, this type was added and exactly matched the the OpenWithToolUrls type used for the Zarr metadata URL tools display. However, of these tools, N5s can only be opened in Neuroglancer (or copied), so I removed the unneeded tool keys that will never be populated for N5s.
| validator: null, | ||
| vole: null, | ||
| avivator: null | ||
| neuroglancer: '' |
There was a problem hiding this comment.
Again, I am not sure what this dict is doing, but why does neuroglacer remain and not the others?
- previously, if a logo path wasn't found, a 404 URL was still created, so it was never falling through to the fallback_logo.
- includes checking that a fallback_logo is used when no logo is found for a viewer
- this minimizes the number of files we need to edit to change anything related to the viewers
- still requires some custom validation related to the capability manifests
| if (viewer.key === 'neuroglancer') { | ||
| // Extract base URL from template (everything before #!) | ||
| const neuroglancerBaseUrl = viewer.urlTemplate.split('#!')[0] + '#!'; | ||
| if (disableNeuroglancerStateGeneration) { |
There was a problem hiding this comment.
We are generating state for Neuroglancer links in Fileglancer, but the Neuroglancer template url currently used in the bioimagetools/capability-manifests assumes you're not https://github.com/BioImageTools/capability-manifest/blob/9e65138b0608b31c886e7a4ec7f54782794b6256/public/viewers/neuroglancer.yaml#L5C3-L5C123. This is just a temporary fix for now - we should think about whether it makes sense to rely on those template urls, how they need to be formatted, and, in the case of Neuroglancer, if it will expect generated state or not
| if (url) { | ||
| // Extract base URL from template (everything before #!) | ||
| const neuroglancerBaseUrl = | ||
| viewer.urlTemplate.split('#!')[0] + '#!'; |
There was a problem hiding this comment.
see comment above - this is a temporary fix
test: refine error message expectations in viewersConfig tests
test: fix mock hoisting in DataToolLinks test test: simplify capability manifest mock structure test: update tests for new capability-manifest API test fix test fix
…to mimic intended setup
| - "/opt/data" # Another shared directory | ||
| - "~/" # User's home directory | ||
| - "/groups/scicomp/data" # Shared data directory | ||
| - "/opt/data" # Another shared directory |
There was a problem hiding this comment.
The whitespace was useful here, I think?
| "source=${localEnv:HOME}/.claude,target=/home/vscode/.claude,type=bind", | ||
| "source=fileglancer-pixi,target=${containerWorkspaceFolder}/.pixi,type=volume" | ||
| "source=fileglancer-pixi,target=${containerWorkspaceFolder}/.pixi,type=volume", | ||
| "source=/groups/scicompsoft/home/truhlara/gh-repos/capability-manifest,target=/workspaces/capability-manifest,type=bind" |
There was a problem hiding this comment.
We shouldn't add personal paths like this to git. Maybe there is another way to mount this directory?
|
|
||
| **Quick Setup:** | ||
|
|
||
| 1. Edit the configuration file at `frontend/src/config/viewers.config.yaml` to enable/disable viewers or customize URLs |
There was a problem hiding this comment.
It's best not to edit committed source code to change configuration. If users change version controlled code then they will have problems staying up to date with upstream changes. One alternative pattern for this would be to allow users to copy this file into the main directory with their own changes, and then during the build use it to override the default config.
| return config.viewers; | ||
| } catch (error) { | ||
| log.error('Error parsing viewers configuration:', error); | ||
| throw error; |
There was a problem hiding this comment.
I think that in most cases we should either handle an error or throw it. If the error is logged and then rethrown, it will likely be logged again, creating duplicate logs. But I might be missing something.
| } catch (manifestError) { | ||
| log.error('Failed to load capability manifests:', manifestError); | ||
| throw new Error( | ||
| `Failed to load viewer manifests: ${manifestError instanceof Error ? manifestError.message : 'Unknown error'}` |
There was a problem hiding this comment.
This is another case where we likely want to pick where this error should get logged, and not propagate errors that were already handled.
| openWithToolUrls.avivator = null; | ||
| // Convert our metadata to OmeZarrMetadata format for capability checking | ||
| const omeZarrMetadata = { | ||
| version: effectiveZarrVersion === 3 ? '0.5' : '0.4', |
There was a problem hiding this comment.
This is extremely fragile code from my original vibe-coded prototype. We need a better way to get the NGFF version.
Starting in NGFF 0.5, there is an attribute that specifies the version (https://ngff.openmicroscopy.org/specifications/0.5/index.html#ome-zarr-metadata).
I'm not sure if there's a reliable way to tell for versions prior to 0.5. @mkitti do you know? We could take a look at the OME-NGFF Validator, which somehow prints the version.
Clickup id: 86advt10e
This PR implements dynamic viewer configuration for Zarr datasets via a build-time viewers.config.yaml file and integration of the @bioimagetools/capability-manifest library for automatic dataset-viewer compatibility detection.
Changes
Created viewers.config.yaml with documentation for customizing viewer configuration. This file specifies available viewers for a Fileglancer deployment. The default configuration is our current viewers: Neuroglancer, Avivator, Vol-E, and OME-Zarr Validator. Configuration is bundled at build time.
Added ViewersContext for managing viewer configuration. This context:
/publicfolder bundled into the application during the build (this is currently the case since the manifests are not present on our main branch, only in this PR branch)getCompatibleViewers()function to determine dataset-viewer compatibility using the capability-manifest library for compatibility detectionRefactored useZarrMetadata to dynamically generate
openWithToolUrlsusing ViewersContext. URLs are generated only for compatible viewers based on thegetCompatibleViewersfunction.Updated DataToolLinks to render viewer buttons dynamically by mapping over valid viewers from ViewersContext. The "Copy URL" tool is always available when a data link is available.
Added logo management system with convention-based naming (
{name}.png), custom logo override support, and fallback logo for missing assets.Added unit tests for config parsing and component tests for DataToolLinks.
Updated documentation with a ViewersConfiguration.md guide and updated CLAUDE.md with viewer configuration instructions.