Repository navigation
Add in-app problem reporting and improve tile fetch prioritization - #324
Merged
stevevanhooser merged 25 commits intoOct 13, 2026
Merged
stevevanhooser merged 25 commits into
stevevanhooser merged 25 commits into
Conversation
…dow closed Launch window: closeLaunchWindow() cleared _window, and the very next progress.note() went through _mode() -> _launchWindow(), found no window and built a fresh "starting ..." one that nothing ever closed. A _window_closed latch now stops the rebuild (same fix in genepyramid, where the bug is latent). View tile tally: the fetch counter only saw fetches as napari issued them, so it could say "12 in flight" but not how many the view needed. New viewtiles module works that out from napari's data_level, corner_pixels and dims point plus each level's chunk grid and stored chunk list (now exposed as ImagePyramidLoader.levelMeta), skipping sparse chunks. A main-thread QTimer polls it and feeds the counter, so the status bar reads "view 37/120 tiles ready | ...", the canvas overlay reads "Loading view: 37 / 120 tiles (12 in flight)" and hides when the view is complete, and stderr logs "view complete: N tile(s) in Xs". Skipped in single-level and viewport-clip modes, where the level index does not line up with the level documents. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
…k silently When the tally could not read napari's state it quietly reverted to the in-flight-only status, which looks identical to running without the feature. The poll now logs "view tile tally: on (...)" at start and "view tile tally: cannot count this view -- <reason>" whenever the reason changes (level/doc count mismatch, unreadable layer state, axis mapping, view too large); openPyramid logs when single-level or viewport-clip mode turns it off. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
…d past napari slices on one thread, so zooming through an intermediate level started a slice that then held up the slice for where the user stopped until every one of its tiles had arrived, and the fetch pool was FIFO on top of that. _ChunkFetcher now runs its own priority queue instead of a ThreadPoolExecutor: newest requests first, and chunks in the current view (handed over by the view-tally poll via setView) ahead of everything -- queued immediately even if napari has not asked yet. A blocking read for a chunk that has left the view is released with StaleChunkRead, so the stale slice ends and napari moves on; the fetch itself stays queued at low priority and lands in the cache for when the user zooms back. Reads are only judged against a view set AFTER they began (the poll lags napari), the coarsest level is never released (auto-contrast samples it), and an unknown view releases nothing. StaleChunkRead is raised rather than answered with fill_value so napari's dask cache never keeps a blank block; the viewer filters it out of napari's error notifications. Measured under real napari with async slicing (1 s simulated fetches, 2 workers, zoom through a 20-tile level to a 4-tile view): final view complete 11.9 s after the last zoom before, 2.7 s after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
Port of NDI-matlab#1023 (75f937a76): wrappers for the in-app problem
report endpoints from ndi-cloud-node#158.
- submitFeedback(app, category, description, *, appVersion, datasetId,
documentId, environment, state, log, traceback) POSTs /feedback with
schemaVersion 1. Optional fields keep MATLAB's camelCase names because
they are the shared payload keys; None omits a field, as MATLAB's
missing / struct.empty do. Returns an APIResponse whose data is the
stored report id.
- deleteFeedback(feedbackID) DELETEs /feedback/{feedbackId} (204).
- FeedbackCategory validator in _validators.py (bug/performance/other).
- Bridge entries for both; url bumped to 75f937a76 for the two new
routes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B22A1gLVKa1E3uQm3tv4t8
Port of NDI-matlab feedbackTest.m (75f937a76). - tests/test_cloud_api_feedback.py: real CloudClient over a stub transport; pins the exact payload, optional-field omission, URL and method, MATLAB-style unpacking, client-side validation, and the 400/401/403/404/413/429 error mapping. POST is not retried. - tests/test_cloud_feedback_live.py: the live cases, skipped without credentials or where the endpoint is not deployed (404 on a malformed id); every submitted report is deleted, with a teardown backstop. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B22A1gLVKa1E3uQm3tv4t8
…branch NDI-python main records MATLAB files (the lightsheet OME-Zarr functions, LightsheetZarrManager, resolveOrganizationId, and six ndi_common document files) that exist only on VH-Lab/NDI-matlab claude/lightsheet-zarr-ndi-viewer-djp5vk, which has not merged. CI checks out NDI-matlab main, so the bridge job has been red on main since 2026-09-26, reporting them as stale paths. Add a matlab_branch field (and MATLAB_BRANCH_PENDING for ndi_common) that names the branch, and check the claim rather than skip it: - the branch must exist and the file be on it (ndi_common: byte-identical); - drift is measured against the branch tip instead of HEAD; - once the branch merges into NDI-matlab main, the marker must be removed. Helpers in tests/_matlab_branch.py, tested against a throwaway git repo in tests/test_matlab_branch_pending.py. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B22A1gLVKa1E3uQm3tv4t8
…sheet drift - matlab_branch on resolveOrganizationId, LightsheetZarrManager and the eight +ndi/+fun/+doc/+lightsheet entries. - fromOMEZarr / makePyramid / chooseTileShape: examined against 5c9e8a83 on that branch (default tile budget 8 MB -> 32 MB). No Python counterpart yet, nothing to follow; noted in each decision_log. - decodeJwt: examined against 4a571dce (NDI-matlab#1024, Java base64 decoder replaced by matlab.net.base64decode). Python uses the stdlib; nothing to follow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B22A1gLVKa1E3uQm3tv4t8
Section 8 of docs/developer_notes/ndi_matlab_python_bridge.yaml defines when an entry may name an NDI-matlab branch and how the checks hold it to account; the field table and the porting guide point there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B22A1gLVKa1E3uQm3tv4t8
…edback Add ndi.cloud.api.feedback; fix bridge CI for MATLAB files on an unmerged branch
Brings in ndi.cloud.api.feedback (submitFeedback, deleteFeedback) and its tests (#323) plus the bridge checks for MATLAB files that live on an NDI-matlab branch, ahead of the viewer's Report-a-problem button. Conflict: src/ndi/fun/doc_lightsheet/ndi_matlab_python_bridge.yaml. Both sides recorded the NDI-matlab 5c9e8a8 tile-budget bump; took main's entries, which add the matlab_branch field its new bridge checks use. Semantic fix: this branch's CloudClient re-authenticates on 401/403 when _can_reauth is set (initialised in __init__). The feedback tests build a client with CloudClient.__new__, so their helper now sets _can_reauth = False, as test_cloud_client_retry's helper does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
A beta viewer's problems mostly go unreported: a user launched from MATLAB has no terminal to copy a log from, and writing an email with versions and context is too much. The NDI Cloud panel now has a "Report a problem..." button under the BETA badge. The dialog asks what kind of problem and what the user saw, shows exactly what will be sent, and sends it through ndi.cloud.api.feedback.submitFeedback; Save to file and Copy cover a user who is not signed in or offline. New shared module ndi.gui.feedback (the gene-pyramid viewer can reuse it): * LogBuffer / installLogBuffer: tees stderr and the logging root into a ring of the last 5,000 lines from launch on. * scrub: removes signed-URL credentials (X-Amz-Signature/Credential/ Security-Token), bearer tokens, JWTs and secret-named values -- viewer logs do carry signed URLs. * buildReport: the submitFeedback kwargs, scrubbed, with versions and NDI_* flags as environment, cut to the server's size limits (log keeps its END). * sendReport / saveReport / describeSendError and the Qt dialog (send runs off the UI thread; one report per dialog; errors say what to do). The viewer's collector attaches the pyramid and level geometry, the view tally, each layer's level and region, fetch counts, loader stats and the log, each probe guarded so a report about a broken viewer still builds. _FetchCounter now resolves sys.stderr at print time so its lines reach the log buffer. Checked in real napari: the button docks, the dialog's report carries the view tally, layer level, document id and log, and a signed-URL signature in the log is scrubbed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
… viewer The gene expression viewer gets the same report button as the lightsheet viewer. On a cloud session it sits on the NDI Cloud panel, where the reader signs in (which sending needs); a local session has no cloud panel, so it gets a small "Feedback" dock with just the button -- Save to file and Copy work without signing in. The button itself moves into ndi.gui.feedback.reportButton so both viewers share one definition (text, tooltip, dialog); the lightsheet panel now uses it. ndi.gui.feedback.napariState gives every viewer the same picture of the canvas (dims, camera, each layer's type, visibility, level and region); the gene viewer's collector adds the pyramid id, density mode, the gene selection size, the cloud dataset id and the recent log, and openPyramid starts the log buffer at launch. addCloudPanel and addAllPanels take an optional report_collect; without it nothing changes. Checked in real napari: one button in the cloud and in the local layout, and the report carries app, document id, layers and section. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
NDI-matlab 2f9b0eaf routes both MATLAB launches of the napari viewer (ndi.fun.doc.lightsheet.view and LightsheetZarrManager's View dialog) through the new ndi.fun.doc.lightsheet.launchViewer, which sets NDI_LIGHTSHEET_DEBUG=1 and keeps the viewer's output in lightsheet_YYYYMMDDHHMMSS.txt in NDI's temp folder. * view and LightsheetZarrManager: bumped to 2f9b0eaf with a dated note; nothing on the Python side follows (it is the launched process and already honours NDI_LIGHTSHEET_DEBUG). * launchViewer, viewerLogFile, viewerShellCommand: new matlab_only entries -- Python calls the viewer directly, with its output on the caller's own stderr and in the in-viewer log buffer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
2 of 5 tasks
…id levelSources) * makePyramid: bumped to 04ce18f9 with a dated note -- levelSources and channelNames added for fromSPIM; default OME-Zarr behaviour unchanged. * fromSPIM, reduceBlocks: new porting_deferred entries. The Python side is a viewer today; both port after the MATLAB version is validated, on top of a makePyramid port. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
CI's "MATLAB/Python bridge completeness" checks the bridge against NDI-matlab main, where +ndi/+cloud/+download/+internal/signedUrlDiskCache.m and +ndi/+cloud/+sync/+internal/fetchManifest.m do not exist yet: both live on claude/lightsheet-zarr-ndi-viewer-djp5vk (VH-Lab/NDI-matlab#979). Add matlab_branch to the two entries, as the bridge reference prescribes for files on an unmerged branch. Passes against NDI-matlab main and against the branch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
668beb13 preallocates fromSPIM's per-ladder arrays (no behaviour change) alongside a TestFromSPIM expectation fix; nothing to port. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
Contributor
Author
|
Cloud API (User 1, prod) is red because of the cloud test fixture, not this PR's code. The failing test is Why this isn't this PR's:
No code fix exists in this PR's scope. The fix is to rebuild the fixture with the Build lightsheet blob fixture workflow ( Generated by Claude Code |
The previous fixture dataset (6ab84b54...) lost a file blob in S3 (NoSuchKey), failing the production pyramid integration test. Rebuilt it with the Build lightsheet blob fixture workflow (run 38265299422). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
…ds (ndi-cloud-node#162) Python half of VH-Lab/NDI-matlab#1025. - ndi.fun.file.crc32 / crc32ToBase64: a file's CRC-32 in the cloud's encoding (base64 of the 4-byte big-endian value), via zlib in chunks. - getFileUploadURL(..., checksum_crc32=) sends checksumCRC32; putFiles / putFileBytes(..., checksum_crc32=) send the x-amz-checksum-crc32 header that such a URL signs. Malformed values are refused. - ndi.cloud.api.files.lookupFiles: POST /files/lookup, split at 5000 uids. - uploadSingleFile sends the checksum by default (checksum=True). - getSignedURLSetAll merges the pages' fileInfo. - Bridge: lookupFiles, crc32, crc32ToBase64 recorded with matlab_branch claude/file-crc32-lookup; getFileUploadURL, putFiles, uploadSingleFile, listFiles(All), getSignedURLSet(All) and url synced to NDI-matlab 8fa6ea5e. - Tests: offline tests/test_cloud_api_files_crc32.py; live TestFileChecksums in tests/test_cloud_live.py (skips where the server predates the route). Two test_phase2_gaps uploadSingleFile tests pass checksum=False, since their file does not exist and the default now reads it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SwsqFA9Zs1HGm6YM4g2XRV
A skip hid whether the tests ran at all; an environment without the lookup route or fileInfo now fails these tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SwsqFA9Zs1HGm6YM4g2XRV
lookupFiles, crc32 and crc32ToBase64 are on NDI-matlab main, so the ordinary bridge checks apply. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SwsqFA9Zs1HGm6YM4g2XRV
…ookup NDI Cloud file CRC-32: lookupFiles, checksummed uploads (ndi-cloud-node#162)
…a-gybqx6 # Conflicts: # src/ndi/cloud/api/files.py # src/ndi/cloud/api/ndi_matlab_python_bridge.yaml
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
Contributor
Author
|
Two cloud checks failed on
No fix exists in this PR for either. I'm re-running each failed job once. If either fails again, I'll treat it as real. Generated by Claude Code |
…e dask pool A tile takes seconds on the cloud, almost all of it waiting on the network, but only ~CPU-count tiles were ever in flight, and every fetcher worker opened a whole NDI session at launch just so it could call database_openbinarydoc on its own thread. Resolving a chunk needs no database. The level document is in memory, its series manifest names each member's uid, and DID's cached_path_for_uid (documented as safe from any thread) finds a member on disk with no SQL. A member not on disk is now fetched through download_file_from_cloud with the same context DID's open_doc passes (manifest location as source, seriesName and member uid in the context), into DID's file cache under its uid, so DID and later lookups find it exactly where open_doc would have put it. The session path stays as the fallback (manifest not on disk yet, no ndic:// location), and a worker opens a session only on first need. NDI_LIGHTSHEET_DIRECT=0 turns the session-free path off. - Default worker count 8 -> 32 (NDI_LIGHTSHEET_WORKERS still overrides). - configureDaskPool: napari reads through dask's threaded scheduler, whose pool defaults to one thread per CPU; the viewer now sizes it to the worker count so napari can keep every worker busy. - fetch_cloud_file records per-thread stage timing (signing, batch or per-file, transfer, bytes); under NDI_LIGHTSHEET_DEBUG each slow resolve logs "[lightsheet] fetched chunk.bin_N in Xs: sign ..s, transfer ..s, .. MB, .. MB/s", so the log says where a tile's time goes. - stats_summary reports how many resolves went via a session. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
32 tiles at once on a ~100 Mb/s line took ~30 s per tile against ~5 s with 8 on a faster one: past the line's capacity every tile in flight shares the bandwidth, so each arrives later, the ones in view included. The right number depends on the connection, so the reader picks it. - _ChunkFetcher starts max_workers threads (MAX_WORKERS = 64 from levelArrays) and lets at most `workers` fetch at once. setWorkers(n) changes that live (clamped to 1..maxWorkers): threads past the limit wait, running fetches finish undisturbed. Workers are plain threads since the session-free change, so 64 idle ones cost nothing. - Default limit 16 (NDI_LIGHTSHEET_WORKERS still overrides). - The NDI Cloud panel gets a "Tiles in flight" drop-down (8/16/32/64, plus the launch value if different) wired to setWorkers. dask's pool is sized to maxWorkers so any choice can actually be used. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
stevevanhooser
merged commit Oct 13, 2026
4c7637e
into
claude/lightsheet-zarr-ndi-viewer-djp5vk
13 checks passed
3 of 6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds comprehensive in-app problem reporting capabilities to NDI viewers and significantly improves the tile fetching system's responsiveness by prioritizing chunks in the current view.
Key Changes
In-App Problem Reporting (
ndi.gui.feedbackandndi.cloud.api.feedback)New feedback module (
src/ndi/gui/feedback.py): Enables users to report problems directly from viewers without needing to find logs or copy version infoLogBuffer: Captures the last 5,000 lines of stderr and logging output for inclusion in reportsscrub(): Removes credentials (AWS signatures, bearer tokens, JWTs, API keys) from logs before transmissionbuildReport(): Constructs the report payload with scrubbed content, respecting server size limitsFeedbackDialog: Qt dialog for collecting user description and submitting reportsopenFeedbackDialog()andreportButton(): Integration points for viewersCloud API feedback endpoint (
src/ndi/cloud/api/feedback.py):submitFeedback(): POST endpoint to send reports to NDI ClouddeleteFeedback(): DELETE endpoint for cleanupComprehensive test coverage:
tests/test_gui_feedback.py: Tests scrubbing, log buffering, and report buildingtests/test_cloud_api_feedback.py: Unit tests for API payload and error handlingtests/test_cloud_feedback_live.py: Live integration tests against real APITile Fetch Prioritization (
ndi.pyramid.multiscale)Priority queue system: Replaced FIFO queue with a heap-based priority system
New classes and exceptions:
StaleChunkRead: Exception raised when a blocking read is released because the view moved on_Job: Represents a queued or running chunk fetch, shared by all waitersView-aware fetching:
setView(): Allows the viewer to communicate which chunks are currently visible_never_stale_docs: Prevents certain documents (e.g., auto-contrast samples) from being released as staleWorker thread management: Improved thread lifecycle and synchronization with condition variables
View Tally System (
ndi.gui.app.lightsheetZarr.viewtiles)chunksInBox(): Pure function computing stored chunk names covering a data regionlayerBox()/viewTally(): Read napari state and compute tile requirementsattachViewTally(): Main-thread polling that feeds view state to the fetch counterViewer Integration
lightsheetZarr viewer (
src/ndi/gui/app/lightsheetZarr/viewer.py):_FetchCounterto display view tally informationviewtiles.attachViewTally()for main-thread pollinggenepyramid viewer (
src/ndi/gui/app/genepyramid/):_reportCollector(): Gathers viewer state for problem reportsaddReportPanel(): UI integration for feedback submissionBridge and Testing Infrastructure
matlab_branchsupport for tracking MATLAB counterparts on pending branchestests/_matlab_branch.py: Helpershttps://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L