feat(notifications,uploads): delete a notification; size images to server caps - #41
Merged
Merged
Conversation
…rver caps Two papercuts from work-consolidation.md §3. P1 — DELETE /api/notifications/:id exists and is documented, but iOS only had read and mark-all-read, so a notification could never be cleared from the tray. - APIClient.deleteNotification(id:) against the 204 route - Swipe-to-delete in NotificationsView, applied optimistically: the row goes on swipe and comes back only if the server rejects it, with its own alert. The alert is a separate channel from `errorMessage`, which drives the full-screen empty state and would have swallowed the message P3 — the client resized uploads to 2048px per side while the backend resizes every image to 1200px and 1.4MB regardless (messages, avatars and documents all route through resizeAvatarToLimit). Every photo posted therefore carried ~2.9x the pixels the server kept, for nothing. - New ServerLimits model + APIClient.serverLimits() against GET /api/limits (public, no auth), and ServerLimitsStore holding the result for the process - RootView refreshes it once at launch. Failure is not an error path: the fallback caps are the values the deployed backend reports today, so an upload is never blocked by a missing fetch - ImageUploadProcessor takes the caps as a parameter instead of hardcoding them, and derives its ladder from them: the server cap leads, and rungs at or above it are dropped. maxPixels 1200 gives [1200, 1000, 800]; a raised cap is honoured with no client release. The store is read off the main actor from a detached task, so it is lock-guarded rather than actor-isolated - ImageUploadProcessorTests pinned the old constants; they now pin the derived ladders and pass explicit caps, so results don't depend on whether GET /api/limits happened to answer during the run Also documents in CLAUDE.md that parallel worktrees must not share a simulator or DerivedData — doing so killed a test runner mid-suite during this work and restarted it into a sibling worktree's test bundle, producing a bogus failure. Verified: 861 tests, 0 failures (iPhone 16 Pro · D9B21F4D-…, isolated derivedDataPath, -parallel-testing-enabled NO, E2E skipped). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KD1sv3y8YWsDBiG31tUJWo
13 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
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
The two papercuts from Thread A: P1 (delete a notification) and P3 (adopt
GET /api/limits).P1 — delete a notification
DELETE /api/notifications/:idexists and is documented; iOS only had read and mark-all-read, so a notification could never actually be cleared from the tray.APIClient.deleteNotification(id:)against the 204 route.NotificationsView, applied optimistically: the row goes on swipe and returns only if the server rejects it. The failure alert is a separate channel fromerrorMessage, which drives the full-screen empty state and would have swallowed the message entirely.P3 — stop sending 2.9× the pixels the server keeps
The client resized uploads to 2048px per side. The backend resizes every image to 1200px / 1.4MB regardless — messages, avatars and document images all route through
resizeAvatarToLimit. Every photo posted carried roughly 2.9× the pixels the server kept, for nothing.ServerLimitsmodel +APIClient.serverLimits()againstGET /api/limits(public, no auth), held for the process byServerLimitsStore.RootViewrefreshes it once at launch. Failure is not an error path — the fallback caps are the values the deployed backend reports today, so an upload is never blocked by a missing fetch.ImageUploadProcessortakes the caps as a parameter instead of hardcoding them and derives its ladder from them: the server cap leads, rungs at or above it are dropped.maxPixels: 1200→[1200, 1000, 800]; a raised server cap is honoured with no client release.ImageUploadProcessorruns in a detached task), so it's lock-guarded rather than actor-isolated.Testing
861 tests, 0 failures (838 → 861).
APIClientLimitsTests— route, no-token path, full decode, and a payload missing the optionalvideo/messagesections.ServerLimitsStoreTests— fallback before refresh, adoption after, fetch failure keeping usable caps, and 50 concurrent reads during a refresh (it's read from a detached task, so this matters).ImageUploadProcessorTestspinned the old constants; they now pin the derived ladders and pass explicit caps, so results no longer depend on whetherGET /api/limitsanswered during the run. Added: a 4000px photo comes out at 1200, and a raised cap produces a larger image.APIClientNotificationsTests— delete path, bearer, percent-encoded id, 404 and 401.Note for whoever runs the suite next
This run initially reported a bogus failure: Threads B and C were testing concurrently on the same simulator UDID and DerivedData, which killed my runner mid-suite and restarted it into Thread B's test bundle (
AIArtifactTestsshowed up in my log). Re-run on a dedicated UDID +-derivedDataPath, it's clean. CLAUDE.md now says so under Build & Test.🤖 Generated with Claude Code