Broken fallback for uploaded report name#1481
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes incorrect folder-name fallback behavior for folder-style uploads (profiler/performance), ensuring non-Safari browsers that provide the folder name only via relative filenames still save reports under the correct parent directory.
Changes:
- Centralized folder-name resolution via
resolve_parent_folder_name()and updated profiler/performance upload handlers to use the resolved value consistently. - Enhanced folder-upload destination-path handling to avoid double-prefixing the report folder for Chromium-style
webkitRelativePathfilenames. - Added unit + integration regression tests covering Safari/Chromium upload shapes and expected on-disk layout.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
backend/ttnn_visualizer/views.py |
Uses a shared folder-name resolver and passes the resolved name into file-saving for profiler/performance uploads. |
backend/ttnn_visualizer/file_uploads.py |
Adds resolve_parent_folder_name() and dedupes folder segment in folder-upload destination path construction. |
backend/ttnn_visualizer/tests/test_file_uploads.py |
Adds targeted regression tests for folder-name resolution, dedup behavior, and end-to-end upload layouts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
smountenay-tt
approved these changes
May 13, 2026
Merged
dcblundell
added a commit
that referenced
this pull request
May 14, 2026
Automated PR to merge `dev` into `main` after [v0.85.0](https://github.com/tenstorrent/ttnn-visualizer/releases/tag/v0.85.0) was published. **package.json version on `dev`:** 0.85.0 --- ## Copilot generated summary This release (v0.85.0) centres on two major additions: a full **MLIR graph visualisation** feature built on ReactFlow, and meaningful hardening of the **file upload and database** layers. It also delivers targeted UX improvements to the operation graph navigator and a substantial expansion of test coverage across both frontend and backend. ## Notable Changes ### MLIR Graph Viewer (new feature) - New `MLIRViewReactFlow` component renders MLIR operation graphs using `@xyflow/react` and `@dagrejs/dagre` for layout, replacing the removed `MLIRView` (which depended on `hpcc-js` and `zustand`, now dropped). - Graph construction split into focused modules: `mlirGraphBuilder`, `mlirGraphIndexBuilder`, `mlirGraphPartitioner`, `mlirLayoutWorker`, and `useMlirLayoutWorker` (layout runs off the main thread via a web worker hook). - MLIR upload is **disabled in hosted/server mode** as an explicit security boundary; `MlirProcessingStatus` surfaces async processing state to users. - New route (`/mlir`), nav entry, and `MLIR.tsx` route component; feature is guarded behind a dev-mode flag. - New test suites (`mlirGraphBuilder.spec`, `mlirGraphIndexBuilder.spec`, `mlirGraphPartitioner.spec`) with shared fixture builders and scenario helpers provide thorough coverage of graph logic. ### File Upload Hardening - `resolve_folder_name` extracted into a shared helper so both upload handlers use identical folder-naming logic; fixes broken fallback for uploaded report names (#1481). - Malware scanning verified to run on MLIR file uploads; new `test_file_uploads.py` (~600 lines) covers upload validation end-to-end. - Input validation tightened; `sanitiseFileName` utility added on the frontend. ### Database Layer - **Alembic** introduced for schema migrations (`alembic~=1.18.0`); `database_migrations.py` and `flask:migrate` npm script orchestrate it; `alembic.ini` and migration env bundled into package data. - DB queries refactored to use **column-name access** instead of positional indices (#1478), eliminating fragile index-dependent row parsing across `Buffer`, `StackTrace`, and related types. - Directory existence now ensured before SQLite DB creation, preventing startup errors. ### Operation Graph Navigation (UX) - Replaced the "recenter" tooltip with a **Locate button** that highlights and blinks the current operation node. - Input/output nodes highlighted relative to the focused operation via a `NodeRelation` enum; blink animation uses `tinycolor2`-lightened colours. - Focus properly initialised on first graph load and preserved during navigation (#1480, #1473). - Selected tensor scrolled into view when navigating from the tensor list (#1476). ### Testing - `useRemote.spec.tsx` added (124 lines) covering remote hook behaviour. - `test_queries.py` extended alongside the column-name query refactor. ### Dependencies - **Added:** `@xyflow/react ^12.10.2`, `@dagrejs/dagre ^3.0.0`, `@types/dagre ^0.7.54` (frontend); `alembic~=1.18.0` (backend). - **Removed:** `hpcc-js`, `zustand` (frontend). ## Areas Needing Careful Review - **Security boundary for MLIR uploads:** confirm the hosted-mode guard is enforced on every upload path and cannot be bypassed via alternate routes. - **Alembic migration bootstrap:** verify the first migration runs cleanly on existing production databases with varying schema versions. - **Column-name DB queries:** the refactor touches most query functions; validate against real report SQLite files, especially older schema versions.
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
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.
This pull request improves the handling and validation of folder-style file uploads in the backend, ensuring that uploaded files from different browsers (Chromium, Firefox, Safari) are consistently placed in the correct directory structure and that invalid mixed-folder uploads are rejected. It introduces a new helper for resolving the destination folder name, strengthens validation to prevent silent misplacement of files, and adds comprehensive tests to cover these behaviors.
Folder name resolution and validation improvements:
resolve_parent_folder_nameto consistently determine the destination folder for uploads, preferring an explicit form field when present (Safari) and falling back to filename inference (Chromium/Firefox). This centralizes logic and prevents files from being saved at the root or in incorrect directories. [1] [2] [3] [4]validate_filesto reject uploads that span multiple inferred parent folders, preventing cases where files from different folders would be incorrectly nested under a single report folder. [1] [2]File path construction fixes:
construct_dest_pathto deduplicate the leading folder segment in Chromium-style uploads, avoiding double-prefixing (e.g.,report/report/file). This ensures files always land under the correct directory regardless of browser.Testing enhancements:
resolve_parent_folder_name,validate_files, andconstruct_dest_pathcovering all major browser behaviors and edge cases (e.g., empty fields, mixed-folder uploads, path traversal posture). [1] [2]These changes ensure robust, predictable handling of folder-style uploads across browsers and prevent subtle bugs from misplacing or nesting files incorrectly.