Skip to content

System-level cleanup: fewer wake-ups, less main-thread work, and a launch crash fixed - #13

Merged
ctimothe merged 16 commits into
dynamic-island-parityfrom
claude/system-cleanup-optimizations-f83666
Sep 22, 2026
Merged

ctimothe merged 16 commits into
dynamic-island-parityfrom
claude/system-cleanup-optimizations-f83666

Conversation

@ctimothe

Copy link
Copy Markdown
Owner

A batch of small, system-level fixes: fewer wake-ups, less main-thread work, correct sleep and wake handling, clear ownership, no test warnings, and less dead code. Each commit stands alone and names the test that holds it.

Fixes

  • The Now Playing helper survives a wake. The silence watchdog, and the helper's own heartbeat, now count awake time (systemUptime) instead of wall-clock time. A closed lid, or the clock being set, no longer looks like silence, so a healthy helper is not killed and the island does not go blank. The helper's 2 s poll also gets a 10% tolerance so the system can coalesce it.
  • A dark display stays quiet. A lock that arrives after the display has slept no longer restarts the media clock. The geometry watchdog stops with the display and starts again on wake. A rebuild while the display is dark no longer restarts the pointer sampler or reopens the panel. Test: LockedClockTests.testALockAfterTheDisplaySleptLeavesTheClockStopped.
  • Docking a display above the notch re-cuts the island's pointer rects, so they stop reaching into the new display or falling short of the new top edge.
  • A song moving from Music to Spotify keeps its lyric nudges under Spotify. The player is adopted before the track is published. Test: LyricsCoordinatorTests.testTheSameSongMovingToAnotherPlayerIsKeyedToThatPlayer.
  • Lyrics folders:
    • A missing folder no longer abandons the whole rescan, and it keeps its documents.
    • A rescan that finds nothing new no longer rewrites the index or sends the current track back through lookup.
    • Bookmarks resolve without UI and without mounting.
    • A folder inside another no longer crashes the app at every launch. That bug predates this branch: the rescan indexed such a folder's files twice, and the next rescan trapped on the repeated path, so the app crashed on every launch from then on.
    • Tests: LocalLyricsLibraryTests (three new).
  • The online lyrics cache has a limit. It keeps the 1000 newest answers, drops expired misses when it loads, and encodes off the main thread. Test: OnlineLyricsTests.testTheCacheKeepsItsNewestAnswersPastItsCapacity.
  • Ownership is stated explicitly. CoreAudio device names are read with the +1 ownership the API documents, and artwork decoding captures self weakly from its outer closure.

Performance

  • Verification hooks (DI_*) are read once at launch, in DebugTrail, instead of on every hit test, watchdog tick and media snapshot. Each read rebuilt the whole environment dictionary.
  • The shelf bookmarks each card once, instead of re-bookmarking the whole shelf on every save. A drop or a screenshot arriving while the shelf is already showing no longer runs a full refresh. Tests: ShelfStoreTests (three new) and FirstRunTests (two new).
  • A drag over the island reads its pasteboard once per drag, not on every move, and a drag crossing back onto a shelf already on screen no longer redoes its contents. Tests: ShelfDropTests.testADragReadsWhatItCarriesOnceNotOnEveryMove, FirstRunTests.testADragReenteringAnOpenShelfDoesNotRefreshIt.

Cleanup

  • Removed the lock-level arithmetic that nothing has called since the SkyLight space, and four members nothing reads. CLAUDE.md now describes the lock path as it is.
  • The test target builds without warnings.

Verification

  • ./scripts/check passed: all 11 steps, the same ones CI runs, 482 tests. The 4 skipped tests only run when their live-network or render variables are set.
  • Reviewed twice: once here, and once by an independent read-only pass over every commit. Both found the nested-folder crash; everything else it raised is listed below.
  • Every new test failed before its fix.
  • Installed to /Applications, relaunched, and the helper is running. LaunchServices lists only /Applications/Isla.app.

Deliberately not in this batch

  • SkyLight space teardown order on unlock: needs a physical lock test.
  • AudioWatch re-reading the whole device on each volume change: only the lock card pays for it.
  • PointerWatcher still times on the wall clock.
  • The Translator GenerationOptions deprecation: the replacement exists only in the macOS 27 SDK, and CI builds with Xcode 26.6.
  • screenLocked()'s wiring has no test, only the pure rule it calls (lockKeepsMediaRunning) does: NotchController has no seam a test can drive.
  • Two racing cache writes could in principle land out of order, leaving cache.json one answer behind. It needs a write slower than the 400 ms debounce, and the next answer rewrites the file.

🤖 Generated with Claude Code

The feed's silence watchdog measured the helper's quiet on the wall clock,
which keeps running while the Mac sleeps. A lid closed for ten minutes read
as ten minutes of silence, so whenever the watchdog's first tick after
waking beat the helper's first line, the helper was killed, a failure was
charged against the restart budget, and the island went blank for the
restart delay. A clock step (NTP, or the time set by hand) did the same.
Silence is now counted in awake time (systemUptime, the clock
MediaController already uses for its anchors).

The helper's heartbeat counts in the same awake time, so the contract is
exact — a repeat at least every 5 s of awake time against a 12 s timeout —
and a clock set back can no longer suppress unchanged payloads long enough
to look like death. Its 2 s poll now carries a tenth of its interval as
tolerance, so macOS can fold that wake-up into others.

Also drops the unused-result warning on the pid-name eviction.

No unit test can put the Mac to sleep; swift test stays green (472).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…he SkyLight space

LockScreenPresence.lockedLevel(base:shield:) lost its last caller on
2026-08-21, when the public route (a panel raised past
CGShieldingWindowLevel) lost the physical lock test and the panel moved
into a SkyLight space of its own. Three tests kept certifying a contract
the shipping code had abandoned; the 2026-09-03 audit flagged both. The
class comment and CLAUDE.md still described the public route as the one
taken — both now say what the code does.

Also gone: a comment in NotchStores.stop() promising a flush that left
with Notes on 2026-08-22, and a "Menu bar item" mark over the panel hooks
of an app that has no menu bar item.

swift test: 469 (the three removed tests were the arithmetic's only callers).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Seven warnings, three causes. LyricsStage.clickTarget and
PillPresence.appearance are pure functions on view types, which the SDK
isolates to the main actor, so tests calling them from a nonisolated
context warned; both are nonisolated now, like
NowPlayingFeed.seekWireLine, along with the constant clickTarget reads.
TranslatorTests mutated a main-actor property from XCTest's nonisolated
setUp; it takes the nonisolated(unsafe) form the lyrics-library tests
already use. OnlineLyricsSearchTests handed a double optional to
XCTAssertNil, which coerced it to Any?; it now says which layer it
means, the way OnlineLyricsTests does, and asserts the searched miss is
remembered as a miss rather than merely as something.

swift test: 469, green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…chdog tick

Thirty checks for the DI_* launch switches each asked
ProcessInfo.processInfo.environment, which builds a fresh dictionary of
the whole environment on every read — 22 µs with 57 variables, measured.
A normal run paid it in NotchRootView.hitTest, on every tick of the 2 s
geometry watchdog, on every media snapshot (twice), and on every lyric
event, only to learn that nothing was set. DebugTrail now reads each
switch once and every site asks it; note() takes an autoclosure, so a
message is formatted only when something will be written. The lyric
stage's onChange watched a formatted debug string that it rebuilt on
every body pass whatever the switch said; the value is gated now.

Behaviour under each hook is unchanged. The geometry watchdog's comment
and CLAUDE.md no longer say the watchdog is armed only under DI_GEOM —
it runs on every launch; only its trail line is gated.

swift test: 469, green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…metry watchdog

The display-sleep handler stops every poll, and three paths started them
again while the screen was still dark:

- A lock arriving after the display slept — any "require password after"
  delay above zero orders them that way, and "immediately" races — ran
  media.setActive(true) for the lock card: with a song playing, the 10 Hz
  position ticker and a once-a-second Apple event into the player, for a
  card nobody could see, until the display woke. Music keeps a Mac awake
  with its display off, so that could be hours. The lock now leaves the
  clock stopped while the display is dark; the wake handler already starts
  it for the card (NotchController.lockKeepsMediaRunning).
- A rebuild in the dark (an external display dropping off the bus moves the
  notch display's frame) restarted the pointer sampler, and could reopen the
  panel — and with it the media clock — under a cursor left on the notch.
- The 2 s geometry watchdog was never stopped at all: thirty wake-ups a
  minute all night. It now sleeps and wakes with the display, and teardown
  invalidates it.

LockedClockTests.testALockAfterTheDisplaySleptLeavesTheClockStopped pins
the lock rule; the rebuild and watchdog paths have no controller seam.
swift test: 470, green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three repeats, all on the main thread:

- persist() bookmarked every card on every save. A save follows every
  change — each copied screenshot, with the panel shut — so one capture
  meant up to sixty reads of file and volume metadata, touching files in
  Desktop and Downloads that load() goes out of its way not to touch; the
  unified log carried hundreds of CFURLCreateBookmarkData per session. Each
  card is now bookmarked once, when it arrives or is read back at launch
  (stale bookmarks excepted), and reused until it leaves. A failure is not
  kept, so it is tried again at the next save, as before.
- A drop re-showed the shelf the drag had already brought up, and a
  screenshot arriving with the shelf on screen did the same. Each re-show
  ran refreshFromDisk: a reachability check and a fresh QuickLook request
  per card, every preview landing as a whole-panel redraw. Both now leave
  a showing shelf alone; the welcome still comes down on a drop.
- Every card built and configured a RelativeDateTimeFormatter for its age
  on every pass of the shelf. One formatter serves them all.

ShelfStoreTests: testEachCardIsBookmarkedOnceNotOnEverySave,
testAFailedBookmarkIsTriedAgain, testAReloadedShelfReusesItsBookmarks.
FirstRunTests: testADropOnTheShelfDoesNotRefreshItAgain,
testAScreenshotArrivingOnTheShelfDoesNotRefreshIt (both red before).
swift test: 475, green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two compiler warnings, both about ownership the code only got right by
accident.

AudioOutputs read kAudioObjectPropertyName into a CFString variable.
CoreAudio returns that string retained (+1) through a raw pointer, so the
placeholder was overwritten without a release and Swift's release at the
end of scope happened to consume CoreAudio's reference. It is read as
Unmanaged<CFString> and taken retained now; checked against this Mac's
default output, which reads back as "MacBook Pro Speakers" three times
running.

MediaController.decodeArtwork asked for a weak self only in the closures
that hop back to the main queue, so the decoding closure captured self
strongly to hand it to them and the weak never covered the decode. The
outer closure is weak now. No cycle existed — the controller lives for the
app — but the code now says what it meant.

The one warning left is Translator's deprecated GenerationOptions
initialiser: its replacement exists only in the macOS 27 SDK, and CI
builds with Xcode 26.6.

swift test: 475, green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…folder stays quiet

A folder whose bookmark no longer resolved — a disk unplugged, a share
not mounted, a folder deleted — made rescanFolders() throw. The rescan
was abandoned where it stood: at launch every folder after the missing
one went unread for the session, each later rescan failed the same way,
and addFolder skipped its save. An unreachable folder now keeps the
documents it had (so a track bound to one finds the same id when the
folder is back) and is recorded as unreadable, and the others are read.
Bookmarks resolve without mounting and without UI, as the shelf's do:
this runs on the main thread at launch.

Every folder-watcher event also rewrote index.json and advanced the
revision, even when nothing the library holds had changed — a file of
any kind added beside the .lrc files was enough — and a new revision
sends the track on screen back through resolution, which can cancel and
re-send its online lookup. The rescan now saves and announces only when
the documents, their contents or the issues changed.

LocalLyricsLibraryTests: testAMissingFolderDoesNotHideTheOthersAndKeepsItsDocuments,
testARescanThatFindsNothingNewLeavesTheRevisionAlone (both red before).
swift test: 477, green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e main thread

Nothing ever left OnlineLyricsCache. A hit carries a song's whole
timeline and never expired; an expired miss was ignored by cached() but
stayed in the dictionary. Every song ever looked up stayed in memory and
in cache.json for the life of the install — and at each new answer the
whole dictionary was JSON-encoded on the main actor, at a track change,
before the debounced write even started.

The cache now keeps the 1000 answers checked most recently, pushing out
the oldest as new ones arrive (a song pushed out is only asked again the
next time it plays). Expired misses, and misses from before the search
existed — both of which cached() already asks again for — are left
behind at load. The encode moved into the debounced background write,
which takes a copy of the dictionary. clear() and count, which nothing
called and nothing could reach from the UI, are gone.

OnlineLyricsTests.testTheCacheKeepsItsNewestAnswersPastItsCapacity (red
before); the existing cache and search tests hold the rest.
swift test: 478, green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… move

draggingUpdated arrives on every move of the pointer, and periodically
while it rests, and each call read the dragged items back off the drag
pasteboard — a URL object per file, or promise objects for Mail and
Photos — to learn whether the drag carries files, an answer that cannot
change within one drag. Once the panel has opened for a drag the whole
700×444 window takes it, so a long hover over the shelf was a steady
stream of pasteboard reads on the main thread. The answer is kept per
drag session, keyed by its sequence number.

ShelfDropTests.testADragReadsWhatItCarriesOnceNotOnEveryMove (red
before: twenty moves, twenty reads).
swift test: 479, green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- NotchController.privacy handed PrivacyMode to the menu-bar item, which
  went on 2026-08-25.
- NotchGeometry.hoverRect, the collapsed hover target without a width;
  every caller asks hoverRect(for:) or collapsedHoverRect(for:).
- OnlineLyrics.Response.plainLyrics and SearchRow.albumName were decoded
  and never read, although both types say only what Isla reads is
  decoded — and plainLyrics is a song's whole untimed sheet, allocated per
  answer for nothing. LRCLIB still sends both; decoding ignores them.

Each checked with grep across Sources and Tests. swift test: 479, green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ects

Every rect that reaches the top of the notch display grows two points
past it, so a pointer thrown against the top edge counts as on the
island — but only where no display sits directly above, or the growth
would land inside that display. Whether one does is read from
NSScreen.screens when a rect is cut, and a screen-parameters change that
leaves the notch display where it was (a monitor docked above a MacBook
that stays the main display, or undocked) cut nothing. The rects kept
reaching into a display just docked above — its bottom edge lit the
island and held the sampler at its fast rate — or stopped short of an
edge that had just become one, so a click at the very top fell through,
until a track change or an open happened to re-cut them.

The same-display branch now re-cuts the warm and cool zones, the close
rect and the collapsed or open rects — the calls a track change already
makes. No controller seam exists to test the wiring; HoverRectTests holds
the geometry. swift test: 479, green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… Spotify

MediaController published the new track before adopting the player
playing it. @published tells subscribers before it stores, and the
lyrics coordinator builds its local identity — whose player id keys
bindings and per-track timing offsets — from the displayed player during
that call, so it read the player the last song came from. A later
duration change rebuilt the identity only when the length moved by more
than a second; the same song moving from Music to Spotify never did, so a
nudge or a binding made while it played in Spotify was saved under Music.

The player is now adopted first, as the Spotify metadata is already
cleared first and for the same reason.

LyricsCoordinatorTests.testTheSameSongMovingToAnotherPlayerIsKeyedToThatPlayer
(red before: "music"), with the controller cut off from every real
player. swift test: 480, green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adding a folder that sits inside one the library already reads made
the rescan walk it twice: on its own, and as part of its parent. Each
walk indexed every file again. A file already known went into the
index twice under the same id, and the next rescan built its
path-to-id table with Dictionary(uniqueKeysWithValues:), which traps
on the repeated path. The library rescans as it opens, so from then on
the app crashed at every launch. A file arriving later got an id per
walk instead, and every lookup for its song became a choice between
two copies of one file.

The rescan now keeps the set of paths it has indexed and skips a
repeat, both for files it walks and for the documents an unreachable
folder keeps. The path-to-id table takes the first id for a path rather
than trapping, so an index an earlier build already wrote with a
repeat is read and rewritten clean.

LocalLyricsLibraryTests.testAFolderInsideAnotherReadsEachFileOnce
covers the file already known, the file arriving later, and an index
doctored to hold a path twice. It trapped with "Duplicate values for
key" before the fix, at the rescan and, with only the scan fixed, at
the relaunch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every drag entering the island set the tab to the shelf, and assigning
the tab is what refreshes the shelf from disk — a reachability check
and a fresh QuickLook request for every card, each preview landing as
a whole-panel redraw. A drag leaving the island and crossing back is
another draggingEntered, and so is every later drag, so a shelf that
was already on screen was passed over again and again.

The tab is now assigned only when the shelf is really coming into
view. The two cases where it is stay: a panel left closed on the shelf
that the drag is about to open, and a shelf under the welcome.

FirstRunTests.testADragReenteringAnOpenShelfDoesNotRefreshIt holds all
three. It failed on every one of them before the fix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The paragraph said the island's space sits "one above the lock
screen's", which reads as 401 next to the 400 it also names. The lock
screen's space is at absolute level 300, as LockScreenPresence.SkyLight
says; 400 is over it with room to spare.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ctimothe
ctimothe merged commit ed3802e into dynamic-island-parity Sep 22, 2026
1 check passed
@ctimothe
ctimothe deleted the claude/system-cleanup-optimizations-f83666 branch September 22, 2026 21:30
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.

2 participants