Skip to content

fix: stabilize media cache CI tests#464

Closed
agent-p1p wants to merge 4 commits into
masterfrom
pip/whitenoise-mac-457-v2
Closed

fix: stabilize media cache CI tests#464
agent-p1p wants to merge 4 commits into
masterfrom
pip/whitenoise-mac-457-v2

Conversation

@agent-p1p

@agent-p1p agent-p1p commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Fixes #457

Replacement for #463, which was closed after its first CI run exposed a Swift contextual-type compile error. This branch includes the type annotation fix and has no inherited red CI history.

Summary

  • Reworks the tracked-footprint regression to corrupt and delete an entry through its valid post-fix: isolate media cache entries by plaintext hash #444 cache identity, preserving coverage that a subsequent store does not trigger an unintended full scan.
  • Serializes FakeMarmotRuntime download observations so concurrent attachment downloads cannot lose recorded references or corrupt the backing array.
  • Replaces the scheduler-dependent 100-yield media gate poll with a cancellation-safe one-shot gate/load race and awaits limiter cleanup on all test exits.
  • Latches the first gate/load terminal outcome under one lock, routes cancellation through the same transition, and atomically claims the gate continuation so late or concurrent calls cannot overwrite the winning result.

Verification

  • git diff --check
  • bash -n scripts/ci/macos-sanity-checks.sh
  • bash -n scripts/sync-bindings.sh
  • Independent adversarial pre-commit review: passed after two fix cycles
  • GitHub Actions run 29114189032:
    • Static Analysis: passed
    • Swift format lint: passed
    • macOS project sanity checks: passed
    • Full PR plan: 420 tests in 2 suites passed
    • Release arm64 smoke build: passed
  • All three issue-reported regressions passed explicitly:
    • messageMediaDiskCacheReadDeletionMaintainsTrackedFootprint
    • workspaceCoalescesAndCachesSourceEpochZeroMediaReferenceResolution
    • mediaAttachmentDownloadTimesOutAndReleasesLimiterSlot
  • Focused observer-order regressions passed:
    • load completion before late gate arrival remains false
    • gate arrival before load completion remains true

Sensitive paths

None. Test harness only; no crypto, MLS, CGKA, key, auth, trust-anchor, or group-state code changed.

Summary by CodeRabbit

  • Bug Fixes
    • Message and conversation timestamps now refresh automatically when the calendar day changes or the app becomes active.
    • Relative dates, times, weekdays, and metadata now update correctly using the current locale.
    • Sidebar and conversation timestamp displays stay synchronized with the latest date and regional formatting settings.
    • Improved handling of cached media when stored data is missing or corrupted.
    • Strengthened media download coordination for more reliable loading and cancellation behavior.

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: deeb5baa-b20e-4470-a7b8-e7581276bebc

📥 Commits

Reviewing files that changed from the base of the PR and between 0454dbf and bf28027.

📒 Files selected for processing (6)
  • whitenoise-mac/ContentView.swift
  • whitenoise-mac/Models/MessengerModels.swift
  • whitenoise-mac/Views/MessageMediaViews.swift
  • whitenoise-mac/Views/MessengerShellView.swift
  • whitenoise-mac/Views/SidebarViews.swift
  • whitenoise-macTests/whitenoise_macTests.swift

Walkthrough

The change introduces an environment-backed timestamp reference date, converts message and chat timestamps to on-demand formatting, propagates date and locale context through SwiftUI views, and expands media-cache and asynchronous download test coverage.

Changes

Timestamp refresh

Layer / File(s) Summary
Timestamp context state
whitenoise-mac/ContentView.swift
Stores the timestamp reference date, refreshes it on day changes and app activation, and injects it into the SwiftUI environment.
On-demand timestamp formatting
whitenoise-mac/Models/MessengerModels.swift
Removes stored time labels and formats message timestamps from the supplied date and locale context.
Message and sidebar integration
whitenoise-mac/Views/MessageMediaViews.swift, whitenoise-mac/Views/MessengerShellView.swift, whitenoise-mac/Views/SidebarViews.swift
Passes timestamp context through conversation rows, bubbles, timeline notices, and chat rows.
Timestamp regression coverage
whitenoise-macTests/whitenoise_macTests.swift
Adds reference-day refresh coverage and updates label assertions to use parameterized formatting methods.

Media test stability

Layer / File(s) Summary
Media-cache read-path regression
whitenoise-macTests/whitenoise_macTests.swift
Uses distinct cache fixtures, corrupts metadata explicitly, and verifies invalidated entries are deleted.
Download-gate observation model
whitenoise-macTests/whitenoise_macTests.swift
Adds locked download observations and cancellation-aware gate waiting and signaling.
Media download timing validation
whitenoise-macTests/whitenoise_macTests.swift
Adds gate-order tests and coordinates limiter slots, stalled downloads, cancellation, timeout, rollback, and cleanup.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The timestamp-reference and message-label refactors in app views/models are unrelated to the media-cache CI fixes. Split the unrelated timestamp-rendering changes into a separate PR and keep this one focused on cache regression and async test stabilization.
Docstring Coverage ⚠️ Warning Docstring coverage is 8.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly reflects the main goal of stabilizing media-cache CI tests and is concise.
Linked Issues check ✅ Passed The changes update the cache-key regression and stabilize the async media tests called out in #457.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch pip/whitenoise-mac-457-v2

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@stage-review

stage-review Bot commented Jul 10, 2026

Copy link
Copy Markdown

Ready to review this PR? Stage has broken it down into 5 individual chapters for you:

Title
1 Introduce dynamic timestamp reference and environment key
2 Refactor models to support dynamic timestamp generation
3 Wire dynamic timestamps through the UI layer
4 Update tests for dynamic timestamps
5 Stabilize media cache and download gate tests
Open in Stage

Chapters generated by Stage for commit bf28027 on Jul 13, 2026 11:14am UTC.

@agent-p1p

Copy link
Copy Markdown
Contributor Author

Pip adversarial review

Verdict: blocking — 1 finding.

Blocking

  • whitenoise-macTests/whitenoise_macTests.swift:17554-17579 — the new gate/load race does not reliably latch the first event. If finishMediaDownloadGateWaitWithoutReaching() runs before the observer is installed, it buffers false. A cancelled timeout operation can then reach passMediaDownloadGateIfArmed() late; that path sets mediaDownloadGateReached = true and clears the buffered false. A later waitForMediaDownloadGateReached() checks mediaDownloadGateReached first and returns true, even though the load completed before reaching the gate. This contradicts the helper's documented contract and lets the timeout regression pass without establishing its intended precondition on the slow-scheduler path it is meant to stabilize. Latch the first terminal outcome under mediaDownloadGateLock (a late gate must not overwrite buffered load completion), and add a focused regression for “load finishes, cancelled operation reaches gate late, observer starts last.”

Verified

  • The post-fix: isolate media cache entries by plaintext hash #444 cache-identity regression now exercises a valid same-key corrupt read and preserves the tracked-footprint sentinel.
  • Download observation count/reference mutation is serialized by one lock.
  • git diff --check, bash -n scripts/ci/macos-sanity-checks.sh, and bash -n scripts/sync-bindings.sh pass locally.
  • GitHub Actions run 29110025675 is green; the full plan reports 418 tests in 2 suites, including all three issue-reported tests.
  • Sensitive paths touched: none (test harness only).

@agent-p1p

Copy link
Copy Markdown
Contributor Author

Addressed the blocking finding in 73ecc32. The fake gate now keeps a one-shot terminal latch, routes cancellation through the same locked transition, and atomically claims the gate continuation so concurrent callers cannot overwrite it. Added deterministic load-first/late-gate and gate-first/load-finish regressions; local diff/shell checks pass. macOS CI is running.

* fix: refresh relative timestamps after midnight

Refresh chat and message timestamp projections when the calendar day changes or the app returns to the foreground. Keep formatting outside ordinary row renders and cover the day-boundary transition with a regression test.

Fixes #470

* style: align timestamp view modifiers

* refactor: simplify timestamp refresh state

Remove stale mapping-time labels, narrow the sidebar's equatable timestamp inputs, require explicit message-row clock inputs, and assert the exact weekday transition in the regression test.

---------

Co-authored-by: agent-p1p <agent-p1p@users.noreply.github.com>
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.

CI: restore the macOS PR test plan after media-cache changes

2 participants