Skip to content

Feat: Add a Files screen to the Demo app - #470

Open
nezhyborets wants to merge 8 commits into
mainfrom
demo/files-screen
Open

nezhyborets wants to merge 8 commits into
mainfrom
demo/files-screen

Conversation

@nezhyborets

@nezhyborets nezhyborets commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

What

Adds a Files screen to the Demo app (Misc → Files) that exercises every method of the openAI.files namespace added in #401:

  • List: files with a purpose filter, pull-to-refresh, "Load More" paging, swipe to delete, and an empty state (files.list(query:)).
  • Upload: a sheet that picks any file, a purpose (the six upload purposes), and an optional expiration of 1–720 hours (files.create(query:), with expiresAfter).
  • Detail: all file fields, Refresh (files.retrieve(id:)), Download Content with a text preview and Share (files.content(id:)), and Delete with confirmation (files.delete(id:)).

Other providers see the existing "OpenAI-only" placeholder. API errors are shown in an alert.

FilesStore depends only on FilesEndpointProtocol, the way ResponsesStore depends on the Responses endpoint, so its tests use a fake endpoint: filter and page size, loading the next page, upload fields including expiration, delete, and error handling (6 tests).

Screenshots

List Upload Detail Purpose filter Empty state

Why

The Demo is currently the only way to exercise the package end to end against the live API: the test suite uses mocked sessions and CI doesn't build the Demo. This is the first step of #468, which tracks covering every public API in the Demo.

Testing

Run in the iOS Simulator against the live API while #401 was in review. It found two bugs in #401, which were fixed there before it merged:

  • Empty pages failed to decode. The API returns null for first_id and last_id on an empty page. Filtering by a purpose with no files now shows the empty state.
  • files.content(id:) returned API errors as content. Downloading a file whose purpose doesn't allow downloads now shows "Not allowed to download files of purpose: user_data".

Also checked live: uploads with user_data (with a 1-hour expiry) and evals (the API returns "purpose": "evals", which the edited OpenAIFile decodes), retrieve, content download, and delete. Test files were deleted afterwards.

DemoChat tests pass (41), and the Demo app builds for the iOS Simulator.

Affected Areas

  • Demo/DemoChat/Sources/FilesStore.swift, Demo/DemoChat/Sources/UI/Files/ (new)
  • Demo/DemoChat/Sources/UI/Misc/MiscView.swift: Files link
  • Demo/DemoChat/Tests/FilesStoreTests.swift (new)

No changes to the package.

🤖 Generated with Claude Code

nezhyborets and others added 2 commits October 14, 2026 11:11
Misc > Files lists the organization's files with a purpose filter,
pull-to-refresh and paging, uploads a file with a chosen purpose and an
optional expiration, and opens a file to refresh its details, show and
share its content, or delete it. It exercises every method of
openAI.files end to end. Other providers see the OpenAI-only placeholder.

FilesStore takes only FilesEndpointProtocol, like ResponsesStore takes
the responses endpoint, so its tests use a fake endpoint.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
After a failed reload, for example under a new purpose filter, the list
kept showing files that may not match the filter.

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

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.

🟡 Changes recommended

Concurrent loads can publish stale results, and downloaded filenames permit unsafe temporary-file writes.

4 open findings
What changed in this PR

Adds a Demo Files workflow that exercises the complete openAI.files API.

Changes:

  • Adds list, filter, pagination, upload, detail, download, sharing, and deletion flows.
  • Adds a protocol-backed Files store with unit tests.
  • Links the feature from Misc while retaining provider gating.
File Description
FilesStore.swift Implements Files API state and operations.
FilesView.swift Adds the paginated, filterable file list.
UploadFileView.swift Adds file selection and upload options.
FileDetailView.swift Adds metadata, content, sharing, and deletion.
MiscView.swift Adds Files navigation.
FilesStoreTests.swift Tests core store requests and state handling.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread Demo/DemoChat/Sources/UI/Files/FileDetailView.swift Outdated
Comment thread Demo/DemoChat/Sources/FilesStore.swift
Comment thread Demo/DemoChat/Sources/FilesStore.swift Outdated
Comment thread Demo/DemoChat/Sources/UI/Files/FileDetailView.swift Outdated
nezhyborets and others added 4 commits October 14, 2026 14:55
The detail screen wrote the downloaded content to the temporary directory
under the file's API-provided name before sharing it. A name such as
../x resolved outside that directory, files with the same name overwrote
each other's copy, and a failed write was silently ignored.

Share the in-memory data through a Transferable instead, suggesting only
the name's last path component, or the file ID when nothing usable is
left.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reloads and Load More could overlap: changing the purpose filter, pulling
to refresh or loading more while another request was in flight let a
slower response overwrite a newer one, and the first request to finish
cleared isLoading while the other was still running.

Skip a load while another one is running, and disable the purpose filter
while loading so that a filter change is never skipped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
FilesStore is main-actor isolated and read the picked file synchronously
before the upload's first suspension point, so a large file (uploads can
be up to 512 MB) froze the UI until it was read. Read it in a detached
task, keeping security-scoped access for the duration of the read.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The detail screen decoded the whole download as a String and counted its
characters on every re-render, before cutting it to 5,000 characters. For
a large file, such as batch output, that is a second full-size copy and a
full traversal on the main thread each time the view updates.

Build the preview once when the download finishes, from at most the first
20,000 bytes, and decide truncation from the byte count. A cut that splits
a multi-byte UTF-8 character drops up to 3 trailing bytes instead of
misreporting the content as binary.

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

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.

🟡 Changes recommended

Pagination after deletion, overlapping list mutations, and test synchronization need correction.

1 open finding
4 resolved since last review
Previously missed (2)

In code that hasn't changed since last review

Medium severity Load More stops paginating after all loaded files are deleted

Demo/​DemoChat/​Sources/​FilesStore.swift:46

Deleting every loaded file while hasMore is true leaves files.last nil. Load More then returns without requesting the remaining files, and the screen incorrectly shows an empty state. Allow Load More to request the first remaining page when the local list is empty.

Medium severity Stale list responses overwrite concurrent uploads and deletions

Demo/​DemoChat/​Sources/​FilesStore.swift:59

The loading guard serializes list requests, but uploads and deletions can still change files while a list request is suspended. For example, swipe deletion can succeed before an older reload response arrives; this assignment then restores the deleted row, whose subsequent requests fail. A reload can likewise overwrite a newly inserted upload. Coordinate mutations with list publication by reconciling overlapping changes or invalidating stale responses and refetching.

🧠 Review effort: Balanced

Comment thread Demo/DemoChat/Tests/FilesStoreTests.swift
nezhyborets and others added 2 commits October 16, 2026 23:09
A list response could undo a change made while it was in flight: a
deletion during a reload brought the deleted row back, an upload during a
reload disappeared from the list, and a reload started during a deletion
could return the file before the server removed it. FilesStore now runs
one request that changes the list at a time, refusing uploads and
deletions while loading and skipping loads while one is in flight; the
UI disables the controls that start them meanwhile.

Load More gave up once every loaded file had been deleted, because it
used the last loaded file as the cursor. It still does, as the API
rejects a deleted file as the cursor (404), but it now loads the first
page when no loaded file remains.

The tests' fake endpoint is now main-actor isolated like the tests, so
recording a request and holding it can't interleave with a test's checks,
which could leave a test waiting forever.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Lists, uploads and deletions all only need to know whether any request
is in flight, so replace isLoading and isChanging with one stored flag.

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

Copy link
Copy Markdown
Collaborator Author

Fixes for the two findings Copilot listed under "Previously missed", which have no review threads of their own:

Load More stops paginating after all loaded files are deleted: fixed in 50a722a. The cursor is still the last loaded file that remains in the list, because the API rejects a deleted file as after with a 404 (checked against the live API). When every loaded file has been deleted, Load More now fetches the first page instead of doing nothing.

Stale list responses overwrite concurrent uploads and deletions: fixed in 50a722a and 508c150. FilesStore now runs one request that changes the list at a time (list, upload or delete), tracked by a single isBusy flag: uploads and deletions are refused while a list request is in flight, and list requests are skipped while an upload or deletion is. The UI disables the controls that start them while busy: the purpose filter, +, swipe to delete, Load More, and Delete on the detail screen.

Both have tests, and the new tests fail when the fixes are reverted.

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.

🟡 Changes recommended

Files can retain outdated credentials, upload cancellation only dismisses the sheet, and the empty-state overlay can obstruct controls.

2 open issues
1 previously missed issue

In code that hasn't changed since last review

Medium severity Make empty-state overlay non-interactive

Demo/​DemoChat/​Sources/​UI/​Files/​FilesView.swift:76

This empty-state overlay covers the entire list, including the purpose picker. When a filter returns no files, it can intercept taps needed to select another filter; it also sits over Load More when all loaded files have been deleted but more pages remain. Make this decorative overlay non-hit-testable so the underlying controls remain usable.

1 resolved since last review

Review effort: Balanced

Comment on lines +15 to +16
public init(client: FilesEndpointProtocol) {
_store = StateObject(wrappedValue: FilesStore(client: client))
.navigationBarTitleDisplayMode(.inline)
.toolbar {
ToolbarItem(placement: .cancellationAction) {
Button("Cancel") { dismiss() }
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.

3 participants