Repository navigation
perf(next): avoid idle observation recomputation - #30565
Conversation
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Packages/macOS/CmuxNext/Tests/CmuxNextSettingsTests/AppleAerialsCatalogTests.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Tests/CmuxNextSettingsTests/AppleAerialsCatalogTests.swift:83">
P2: This test never checks invalidation. `BackdropCatalog(bundledOnly:)` always yields the same catalog, so a stale cache would still pass. Add a case where the catalog changes, such as a catalog built with a different `aerialNames` or `pendingVideoPaths` via `BackdropCatalog(systemDirectory:fileManager:aerials:)` on a temp folder, and assert `builds == 2` with the new rows.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Sources/CmuxNextSettings/BackdropCatalog+Page.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Sources/CmuxNextSettings/BackdropCatalog+Page.swift:40">
P3: The cache can serve stale localized wallpaper labels because its key contains no locale. Include the localization context in the cache key or invalidate the rows cache when the app locale changes.</violation>
<violation number="2" location="Packages/macOS/CmuxNext/Sources/CmuxNextSettings/BackdropCatalog+Page.swift:40">
P2: Cache hits still linearly compare the entire wallpaper catalog, so every host-list read retains catalog-sized idle work. Compare a monotonic catalog revision/generation instead of using full value equality.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| for _ in 0..<100 { _ = cache.rows(for: catalog) } | ||
|
|
||
| #expect(cache.rows(for: catalog) == catalog.pageRows) | ||
| #expect(cache.builds == 1) |
There was a problem hiding this comment.
P2: This test never checks invalidation. BackdropCatalog(bundledOnly:) always yields the same catalog, so a stale cache would still pass. Add a case where the catalog changes, such as a catalog built with a different aerialNames or pendingVideoPaths via BackdropCatalog(systemDirectory:fileManager:aerials:) on a temp folder, and assert builds == 2 with the new rows.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Tests/CmuxNextSettingsTests/AppleAerialsCatalogTests.swift, line 83:
<comment>This test never checks invalidation. `BackdropCatalog(bundledOnly:)` always yields the same catalog, so a stale cache would still pass. Add a case where the catalog changes, such as a catalog built with a different `aerialNames` or `pendingVideoPaths` via `BackdropCatalog(systemDirectory:fileManager:aerials:)` on a temp folder, and assert `builds == 2` with the new rows.</comment>
<file context>
@@ -72,3 +72,13 @@ struct AppleAerialsCatalogTests {
+ for _ in 0..<100 { _ = cache.rows(for: catalog) }
+
+ #expect(cache.rows(for: catalog) == catalog.pageRows)
+ #expect(cache.builds == 1)
+}
</file context>
| public init() {} | ||
|
|
||
| public mutating func rows(for catalog: BackdropCatalog) -> JSONValue { | ||
| if self.catalog != catalog { |
There was a problem hiding this comment.
P2: Cache hits still linearly compare the entire wallpaper catalog, so every host-list read retains catalog-sized idle work. Compare a monotonic catalog revision/generation instead of using full value equality.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Sources/CmuxNextSettings/BackdropCatalog+Page.swift, line 40:
<comment>Cache hits still linearly compare the entire wallpaper catalog, so every host-list read retains catalog-sized idle work. Compare a monotonic catalog revision/generation instead of using full value equality.</comment>
<file context>
@@ -24,3 +24,24 @@ public extension BackdropCatalog {
+ public init() {}
+
+ public mutating func rows(for catalog: BackdropCatalog) -> JSONValue {
+ if self.catalog != catalog {
+ self.catalog = catalog
+ cached = catalog.pageRows
</file context>
| public init() {} | ||
|
|
||
| public mutating func rows(for catalog: BackdropCatalog) -> JSONValue { | ||
| if self.catalog != catalog { |
There was a problem hiding this comment.
P3: The cache can serve stale localized wallpaper labels because its key contains no locale. Include the localization context in the cache key or invalidate the rows cache when the app locale changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Sources/CmuxNextSettings/BackdropCatalog+Page.swift, line 40:
<comment>The cache can serve stale localized wallpaper labels because its key contains no locale. Include the localization context in the cache key or invalidate the rows cache when the app locale changes.</comment>
<file context>
@@ -24,3 +24,24 @@ public extension BackdropCatalog {
+ public init() {}
+
+ public mutating func rows(for catalog: BackdropCatalog) -> JSONValue {
+ if self.catalog != catalog {
+ self.catalog = catalog
+ cached = catalog.pageRows
</file context>
There was a problem hiding this comment.
2 existing issues remain and no new issues found across 6 files
Requires human review: Auto-approval blocked because this review re-detected 2 unresolved issues already reported by Cubic.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
Review (subagent claude-opus-5-5 medium): SHIP at 6877edf. The stale SidebarBridge cache was removed; the remaining liveTabs and wallpaper projection caches are safe. Follow-ups are recorded in the PR body. |
|
Tip note: this PR and #30565 together added +15 CmuxNextApp Swift lines, so the arch ratchet's |
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
GitHub Actions
Error
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Not verified: macOS jobs did not run: compile and app tests were skipped on CI passes on Written by Shipped in:
|
The NEXT main thread repeatedly rebuilt observation-derived structures while idle. Settings host-list reads rebuilt wallpaper page rows, and the restored New Tab computation walked the workspace graph even when only row status changed. This caches wallpaper page rows until the catalog changes and exposes an indexed tab snapshot whose observation dependency is only the tab-list revision.
Performance:
wallpaperPageRowsAreMemoizedUntilTheCatalogChangesRestoredNewTabPages.idswithDaemonStore.liveTabsThe wallpaper cache stops when the catalog value changes.
liveTabsobservestabListRevision, which changes on tab/pane topology updates, so terminal status changes do not re-arm the structural observation. No timer or observer is introduced.The other
newTabPagescallers remain intentionally uncached because they build one-off intent and layout payloads:SidebarBridge+Intents.swift:203,SidebarBridge.swift:191, andSidebarBridge.swift:237. The structural sidebar pass atSidebarBridge.swift:148uses the same safeliveTabspath.Cause B from the cpu-audit idle main-thread sample. Compile with
cmux-ci compile-check --ref 6877edf1111c3b13d430a940a1d745f9681fa936 --app.Agent session: agent session