feat: cap directory listing at max_directory_count to avoid full scans by mkitti · Pull Request #370 · JaneliaSciComp/fileglancer

mkitti · 2026-05-11T21:19:00Z

Summary

yield_file_infos_paginated previously called list(os.scandir(...)) which read every entry in a directory before paginating, causing slow responses for large directories
Now uses itertools.islice(scanner, max_count + 1) to read at most max_directory_count + 1 entries (default 10,000, configurable via Settings.max_directory_count / FGC_MAX_DIRECTORY_COUNT)
~~When truncated, entries are returned in filesystem order (sort skipped) and is_truncated: true is included in the API response~~ (There's a complication here. See below.)
Frontend disables column sorting when isTruncated is true

Test plan

Run backend tests: pixi run -e test test-backend
Run frontend type check: pixi run node-check
Test with a large directory (>10,000 files) and verify fast first-page response
Verify is_truncated appears in the API response for large directories
Verify sort controls are disabled in the UI when a directory is truncated

🤖 Generated with Claude Code

Large directories caused slow responses because yield_file_infos_paginated read and sorted all entries regardless of pagination. Now at most max_directory_count+1 entries are read via itertools.islice; if truncated, entries are returned in filesystem order (sort skipped) and is_truncated is set in the API response. The frontend disables column sorting when is_truncated is true. max_directory_count defaults to 10000 and is configurable via settings. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ount Without sorting in the truncated case, entries were returned in arbitrary filesystem order, breaking cursor-based pagination and causing each page request to re-scan all max_count entries from scratch to locate the cursor. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR introduces a cap on directory entry scanning for paginated listings to avoid full directory scans on very large folders, and surfaces truncation state to the API and UI.

Changes:

Backend: cap yield_file_infos_paginated scanning/counting via max_count (defaulting to Settings.max_directory_count) and return an is_truncated flag.
API + frontend: propagate is_truncated to the client and disable table sorting when a listing is truncated.
Tests: update pagination tests for the new return shape and add a truncation test.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`fileglancer/filestore.py`	Limits scandir reads to `max_count + 1`, caps `total_count`, and returns `is_truncated`.
`fileglancer/settings.py`	Adds `max_directory_count` setting (default 10,000).
`fileglancer/server.py`	Adds `is_truncated` to the paginated directory listing API response.
`frontend/src/queries/fileQueries.ts`	Adds `is_truncated` to response typing and maps it to `isTruncated` in query data.
`frontend/src/components/ui/BrowsePage/FileTable.tsx`	Disables column sorting when the listing is truncated.
`tests/test_pagination.py`	Updates tuple unpacking for new return value and adds a truncation-focused test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+            directory has more entries than max_count.
        """
+        if max_count is None:
+            max_count = get_settings().max_directory_count


mkitti · 2026-05-11T22:17:23Z

+        # Read max_count + 1 entries: the extra one detects truncation without
+        # reading the entire directory.
+        with os.scandir(full_path) as scanner:
+            entries = list(itertools.islice(scanner, max_count + 1))
        total_count = len(entries)
+        is_truncated = total_count > max_count
+        if is_truncated:
+            entries = entries[:max_count]
+            total_count = max_count
+        entries.sort(key=lambda e: (not e.is_dir(follow_symlinks=False), e.name))


The sorting and cursor methods may need to be reevaluated in the case of truncation.

+    def test_truncation(self, pagination_dir):
+        """is_truncated is True and entries are in filesystem order (not sorted) when truncated."""
+        fsp = FileSharePath(zone="test", name="test", mount_path=pagination_dir)
+        store = Filestore(fsp)
+        # max_count=5 on a 13-entry directory
+        infos, has_more, next_cursor, total_count, is_truncated = (
+            store.yield_file_infos_paginated(None, limit=10, max_count=5)
+        )
+        assert total_count == 5
+        assert is_truncated is True
+        assert len(infos) == 5  # limit=10 > max_count=5, so all 5 returned


mkitti · 2026-05-11T22:18:38Z

We may need to re-evaluated how sorting or cursors work when truncation is applied. The comment about is_dir is also intriguing regarding the performance impact.

The core requirement is a stable, consistent ordering so cursor-based pagination works without re-scanning on every page. There are a few alternatives:

Sort by name only (drop is_dir) — The expensive part on NFS is is_dir(): when the NFS server
returns DT_UNKNOWN for d_type, each call requires a stat RPC. Sorting by name alone gives stable
pagination ordering without those extra network calls:
entries.sort(key=lambda e: e.name)

Directories and files would be interleaved rather than dirs-first, but pagination would work
correctly and cheaply.

Cache the sorted entry list — Scan and sort once, store the result in memory (e.g., keyed by (fsp,
path, mtime)) with a short TTL. Subsequent page requests hit the cache instead of NFS. The first
load is still slow, but pages 2–50 are instant.

Offset-based pagination instead of cursor-based — Replace the cursor (filename string) with an
integer offset. Each page does entries[offset : offset+limit] after the islice. Avoids searching for
the cursor name on every page, but loses the resilience to insertions/deletions mid-browse that
cursors provide.

Of these, sort by name only is the simplest change with the best NFS performance. Caching would be
the most impactful for repeated browsing of the same large directory. Want to pursue either of those?

allison-truhlar · 2026-05-13T18:28:32Z

I added a notification for when the folder is truncated - thoughts on the wording?

krokicki · 2026-05-13T18:59:08Z

@allison-truhlar LGTM

mkitti · 2026-05-13T19:12:36Z

+              type: 'warning',
+              title: 'Folder contents truncated',
+              message:
+                'Only showing the first 10,000 items in this folder. Reorganize contents into sub-folders to see more.'


Currently, I have the maximum number of items configurable in settings.py. Would there be a better place to define this so that the notification matches the setting?

Could we just pass the value through the response along with is_truncated?

See commit a04628b?

mkitti · 2026-05-13T19:22:33Z

I added a notification for when the folder is truncated - thoughts on the wording?

I would say the the following and avoid the recommendation regarding subfolder. The recommendation would likely corrupt the neuroglancer precomputed layout here.

This directory contains more than 10,000 items and only the first 10,000 items are shown.

Do we use the term "folder" consistently? I associate "folder" with the GUI appearance and directory as the technical term for the file system object.

mkitti · 2026-05-13T19:23:46Z

I think chromatic.yml needs to specify repository. It does not work since I submitted this from my fork.

Copilot suggests the following.

- uses: actions/checkout@v5
  with:
    repository: ${{ github.event.pull_request.head.repo.full_name || github.repository }}
    ref: ${{ github.event.pull_request.head.sha || github.sha }}
    fetch-depth: 0

allison-truhlar · 2026-05-13T19:40:20Z

Do we use the term "folder" consistently? I associate "folder" with the GUI appearance and directory as the technical term for the file system object.

At some point we did talk about using "folder" rather than "directory", with the assumption that this was how most people outside SciComp would refer to a directory on their computer. However, looking through the UI, there are several places we do use the word "directory" instead - some steps in the "tours" (although other steps use "folder"), tooltip for the "favorite" button in the toolbar, change permissions dialog, the permissions table in the properties panel, Napari tab in the "how to use your data link" dialog, and the job options listing. I counted 10 places where we use "folder", so it's about 3:5 directory:folder. I will note that we should standardize the language going forward, but I lean towards keeping folder now since that was our intended language.

allison-truhlar · 2026-05-13T19:45:54Z

I think chromatic.yml needs to specify repository. It does not work since I submitted this from my fork.

Copilot suggests the following.
- uses: actions/checkout@v5
  with:
    repository: ${{ github.event.pull_request.head.repo.full_name || github.repository }}
    ref: ${{ github.event.pull_request.head.sha || github.sha }}
    fetch-depth: 0

I think I will just disable this for external pull requests since it requires a secret to run - Claude suggested that no matter how it's configured, the secret won't be accessible to a pull_request from a fork. Chromatic suggests configuring the action to run on push, but I didn't want the action to run on every push. I think external pull requests are rare enough I think I will add a workflow_dispatch to manually trigger this with.

For this PR, I will just merge it because I know we're not changing any of the new design system components.

allison-truhlar · 2026-05-13T19:56:48Z

Revised wording:

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

frontend/src/components/ui/BrowsePage/FileTable.tsx:340

Placing the truncation warning inside FileTable means it only renders when the parent renders the table. FileBrowser renders this component only when the filtered displayFiles array is non-empty, so a truncated directory whose returned entries are all hidden by filters (for example dotfiles) will show the empty-state instead of warning that results were truncated; render the warning at the browser level or ensure the table is mounted for truncated results.

      {isTruncated ? (
        <div
          className={`${getNotificationStyles('warning').container} p-4 rounded-md mb-3`}
        >
          <NotificationItem
            notification={{
              id: 0,
              type: 'warning',
              title: 'Folder contents truncated',
              message: `This folder contains more than ${truncationLimit} items. Only ${truncationLimit} items are shown.`
            }}
            showDismissButton={false}
          />
        </div>
      ) : null}

…ation The truncated subset contains whatever max_count+1 entries os.scandir returned first; scandir order is filesystem-dependent, so the test's assertion that dirs appear first was a coincidence that held locally but not on CI. Assert what yield_file_infos_paginated actually guarantees: the returned subset is sorted, and repeat calls are stable.

allison-truhlar · 2026-05-15T15:46:55Z

Merging this after addressing all the Copilot review comments.

mkitti and others added 4 commits May 11, 2026 17:15

Fix linting issues

17be27e

chore: prettier formatting with pinned 3.8.3

de61f20

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

mkitti requested review from Copilot and krokicki May 11, 2026 22:11

Copilot started reviewing on behalf of mkitti May 11, 2026 22:12 View session

Copilot AI reviewed May 11, 2026

View reviewed changes

feat: show notification in ui when folder is truncated

d9fe05c

mkitti commented May 13, 2026

View reviewed changes

refactor: pass configured truncation limit in file query response

a04628b

mkitti requested a review from Copilot May 15, 2026 04:20

Copilot started reviewing on behalf of mkitti May 15, 2026 04:21 View session

Copilot AI reviewed May 15, 2026

View reviewed changes

Comment thread fileglancer/server.py Outdated

Comment thread fileglancer/settings.py

Comment thread tests/test_pagination.py

Comment thread frontend/src/components/ui/BrowsePage/FileTable.tsx Outdated

allison-truhlar added 5 commits May 15, 2026 10:06

fix: require max_count be passed to yield_file_infos_paginated

eded320

fix: validate max_directory_count is greater than 0

9f44f03

fix(test): truncated dirs are sorted dir-first then alphabetical

fde2d3b

fix: add language for sorting disabled in file table due to truncation

0c2f0a1

allison-truhlar merged commit 8b7045c into JaneliaSciComp:main May 15, 2026
4 of 5 checks passed

Conversation

mkitti commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

mkitti May 11, 2026

Choose a reason for hiding this comment

Uh oh!

mkitti commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

allison-truhlar commented May 13, 2026

Uh oh!

krokicki commented May 13, 2026

Uh oh!

mkitti May 13, 2026

Choose a reason for hiding this comment

Uh oh!

allison-truhlar May 13, 2026

Choose a reason for hiding this comment

Uh oh!

allison-truhlar May 13, 2026

Choose a reason for hiding this comment

Uh oh!

mkitti commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mkitti commented May 13, 2026

Uh oh!

allison-truhlar commented May 13, 2026

Uh oh!

allison-truhlar commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

allison-truhlar commented May 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

allison-truhlar commented May 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mkitti commented May 11, 2026 •

edited

Loading

mkitti commented May 11, 2026 •

edited

Loading

mkitti commented May 13, 2026 •

edited

Loading

allison-truhlar commented May 13, 2026 •

edited

Loading