Skip to content

fix(storage): disambiguate storage entries with duplicate paths#9832

Merged
Light2Dark merged 2 commits into
mainfrom
Light2Dark/fix-storage-duplicate-paths
Jun 9, 2026
Merged

fix(storage): disambiguate storage entries with duplicate paths#9832
Light2Dark merged 2 commits into
mainfrom
Light2Dark/fix-storage-duplicate-paths

Conversation

@Light2Dark

@Light2Dark Light2Dark commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

📝 Summary

Split out from #9708.

Some backends (e.g. Google Drive) allow multiple files to share the same path, which made entry rows collide when keyed by path alone. This surfaces the backend's stable id through the fsspec backend and prefers it when keying entry rows in the storage inspector, falling back to path + index when no id is available.

📋 Pre-Review Checklist

  • For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on Discord, or the community discussions (Please provide a link if applicable).
  • Any AI generated code has been reviewed line-by-line by the human PR author, who stands by it.
  • Video or media evidence is provided for any visual changes (optional).

✅ Merge Checklist

  • I have read the contributor guidelines.
  • Documentation has been updated where applicable, including docstrings for API changes.
  • Tests have been added for the changes made.

Prefer the backend's stable id (e.g. Google Drive) when keying entry
rows, and surface that id through the fsspec backend so duplicate paths
no longer collide in the storage inspector.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
marimo-docs Ready Ready Preview, Comment Jun 9, 2026 10:11am

Request Review

@Light2Dark Light2Dark marked this pull request as ready for review June 9, 2026 09:47
Copilot AI review requested due to automatic review settings June 9, 2026 09:47
@Light2Dark Light2Dark added the bug Something isn't working label Jun 9, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Disambiguates remote storage entries that can legitimately share the same path (e.g. Google Drive) by surfacing a stable backend id to the frontend and using it for row identity in the storage inspector.

Changes:

  • Propagate backend-provided id into StorageEntry.metadata for fsspec-based storage entries.
  • Update the storage inspector to key/render rows using metadata.id when present, falling back to a derived key when absent.
  • Add backend + frontend tests to cover the new id plumbing and keying behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
tests/_data/_external_storage/test_storage_models.py Adds coverage to ensure backend id is preserved in StorageEntry.metadata.
marimo/_data/_external_storage/storage.py Includes id in the fsspec entry metadata created from ls(..., detail=True) results.
frontend/src/components/storage/storage-inspector.tsx Uses a stable row key based on backend id (with fallback) and threads it into row rendering.
frontend/src/components/storage/tests/storage-inspector.test.ts Adds unit tests for storageEntryKey behavior with/without backend ids.

Comment thread frontend/src/components/storage/storage-inspector.tsx Outdated
Comment thread frontend/src/components/storage/storage-inspector.tsx Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review completed

Architecture diagram
sequenceDiagram
    participant UI as Storage Inspector UI
    participant Backend as Backend API (fsspec)
    participant DB as Google Drive / Backend
    participant State as React Component State

    Note over UI,State: Entry listing from backend

    UI->>Backend: listFiles(namespace, path)
    Backend->>DB: Fetch file metadata
    DB-->>Backend: File list (name, id, ...)
    Backend-->>UI: StorageEntry[] (metadata.id populated)

    Note over UI,State: Render each entry with a stable key

    UI->>UI: For each entry + index → storageEntryKey()

    alt metadata.id is a non-empty string (e.g. Google Drive)
        UI->>UI: Use id as key (no index needed)
    else metadata.id missing / empty / non-string
        UI->>UI: Fallback to path::index
    end

    UI->>State: set entries with unique keys
    State-->>UI: Re-render rows

    Note over UI,State: Row selection / navigation

    UI->>UI: User clicks or selects row
    UI->>UI: Build value = namespace:rowKey
    UI->>UI: Handle click (expand dirs, open files)

    alt Directory click
        UI->>Backend: Recurse into directory (re-requests children)
        Backend-->>UI: New StorageEntry[] for sub-path
    else File click
        UI->>UI: onOpenFile(rowKey)
    end
Loading

Re-trigger cubic

Use each entry's position in the full list (not the filtered list) for
the fallback row key, so filtering no longer shifts keys and remounts
rows that lack a stable backend id.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@kirangadhave kirangadhave left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🚀 minor nit re exports

);
};

export const exportedForTesting = {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why not just export it normally?

or add barrel exports from an index.ts where we skip storageEntryKey, then the tests can export directly from the file.

@Light2Dark Light2Dark merged commit 0d71930 into main Jun 9, 2026
44 checks passed
@Light2Dark Light2Dark deleted the Light2Dark/fix-storage-duplicate-paths branch June 9, 2026 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants