Skip to content

Fix/visualise path segments#998

Closed
milesAraya wants to merge 2025 commits intodevelop-mainfrom
fix/visualise-path-segments
Closed

Fix/visualise path segments#998
milesAraya wants to merge 2025 commits intodevelop-mainfrom
fix/visualise-path-segments

Conversation

@milesAraya
Copy link
Copy Markdown
Collaborator

Content

Summary

  • The Visualize panel never showed the "Visualization data deleted" message when an experiment's intermediate data had been deleted by the expiration lifecycle job. Plots continued to render even when has_intermediates was false in the database.
  • Root cause: getExperimentUidFromFilePath() extracted the wrong path segment (segments[2], the function node ID) instead of the experiment UID (segments[1]). The selector always returned true (default for unknown UID), so the degradation guard never triggered.
  • Added unit tests for getExperimentUidFromFilePath() covering all output path formats and edge cases.

Design Decisions

  • segments[1] not segments[2] -- Output paths from the backend are normalized to {workspaceId}/{experimentUid}/{functionNodeId}/filename by normalize_output_path(). The original comment incorrectly described the format as having a prepended nodeId. Verified by inspecting actual API responses across all algorithm types (Suite2P, CaImAn, LCCD, PCA, CCA, LDA) and tutorial experiments.
  • Exported the function -- getExperimentUidFromFilePath was module-private. Exported it so the test file can import it directly without rendering the full React component.

Evidence

The bug: The original code extracted segments[2] from the file path, based on an incorrect comment:

// BEFORE (wrong):
// Path format: "{nodeId}/{workspaceId}/{uniqueId}/{nodeId}/filename"
return segments.length >= 3 ? segments[2] : ""

Actual path format from live API response (GET /experiments/8):

8/a5feff3e/cca_e9mbfm8vck/coef.json
│ │          │
│ │          └─ segments[2] = "cca_e9mbfm8vck" (function node ID -- WRONG)
│ └─ segments[1] = "a5feff3e" (experiment UID -- CORRECT)
└─ segments[0] = "8" (workspace ID)

Because segments[2] returned a function node ID (e.g. cca_e9mbfm8vck), the selector selectExperimentHasIntermediates("cca_e9mbfm8vck") never found a matching experiment and defaulted to true via ?? true, bypassing the guard entirely.

Live verification on development environment:

  • Set has_intermediates = 0 on experiment a5feff3e in workspace 8
  • Experiment table correctly showed grayed-out name with tooltip: "Visualization data deleted due to subscription expiration. NWB download and reproduce still available."
  • Visualize panel still rendered plots (bug confirmed -- the guard in DisplayDataItem.tsx was ineffective)

References

Files changed

  • frontend/src/components/Workspace/Visualize/DisplayDataItem.tsx -- Fix path segment index from segments[2] to segments[1]; correct the path format comment; export getExperimentUidFromFilePath for testability
  • frontend/src/components/Workspace/Visualize/__tests__/DisplayDataItem.test.ts -- New: 7 tests covering standard output paths, timeseries, tiff subdirectories, and null/empty/single-segment edge cases

Manual Testcases

  1. Set has_intermediates = 0 on an experiment record in the database
  2. Navigate to the Visualize tab for that experiment
  3. Select any visualization output (e.g. a plot from CCA, Suite2P ROI, etc.)
  4. Verify the panel shows "Visualization data deleted due to subscription expiration." instead of the plot
  5. Revert the flag (has_intermediates = 1) and refresh -- plots should render normally
  6. Run frontend tests in Docker:
    npx jest --testPathPattern="DisplayDataItem" --passWithNoTests
    
    Expected: 7 tests pass

Unit, Integration, Contract Test Coverage

  • DisplayDataItem.test.ts -- 7 unit tests: standard output path, timeseries path, tiff subdirectory path, null input, undefined input, empty string, single-segment path

Others

Difficulties

  • The bug was not caught by existing tests because DisplayDataItem had no test coverage and the function was not exported. The incorrect path comment made it appear intentional.

Risk Assessment

Area Risk Notes
Visualize panel degradation Low Only affects display when has_intermediates is false; no change to data serving or other UI states
Path extraction logic Low All backend output paths go through normalize_output_path() which guarantees {workspaceId}/{uid}/... format; verified across all algorithm types
Export of getExperimentUidFromFilePath Low Pure function with no side effects; export only adds test access

tsuchiyama-araya and others added 30 commits March 9, 2026 15:28
Fix cost tracker, add to monthly-maintenance
Maintenance procedure documentation, scripts and email alerts
…ync-error

Mitigate snakemake yaml sync error
…ad-experiment

Fix MockStorageController.upload_experiment
tsuchiyama-araya and others added 29 commits April 1, 2026 13:16
- Adjusted tests.yaml (separate test units, improve efficiency)
- Improved efficiency of Makefile/test_run_all
- Organize and optimize duplicate test runs
Explicit ENV_PREFIX to prevent cross-environment contamination
…ions

Display platform information in Log viewer
…eanup

Event-driven cleanup when an EC2 instance terminate
Add environment filtering to premium instance discovery
ALB creation retry to fix race condition
@milesAraya milesAraya closed this Apr 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants