Lighten Overview menu rows#1675
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed June 20, 2026, 5:49 AM ET / 09:49 UTC. Summary Reproducibility: no. high-confidence live reproduction was established in this read-only review. The linked issue provides macOS 27 sample fragments and current source maps to the hosted Overview row path, but this PR lacks after-fix runtime proof. Review metrics: 2 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land exactly one profiled fix for #1674 after maintainers compare this compact-row approach with #1676 on a provider-heavy macOS 27 setup. Do we have a high-confidence way to reproduce the issue? No high-confidence live reproduction was established in this read-only review. The linked issue provides macOS 27 sample fragments and current source maps to the hosted Overview row path, but this PR lacks after-fix runtime proof. Is this the best way to solve the issue? Unclear. The compact row is a plausible lower-cost implementation and now resolves the live model consistently, but maintainers still need to choose between the information-density tradeoff and the rich-row alternative. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 8d7f1e53bbd4. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
Follow-up after ClawBot review:
The remaining blocker is still the product/proof question: maintainers need to choose lite rows vs #1676, and an interactive macOS 27 scroll profile would still be useful before merging either direction. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
There was a problem hiding this comment.
Pull request overview
This PR targets the Overview menu scroll stutter by replacing the rich Overview provider row content with a compact LiteSummary, avoiding chart-heavy/inline dashboard SwiftUI views inside the Overview list while still supporting live-refresh behavior via MenuCardRefreshMonitor.
Changes:
- Introduces
OverviewMenuCardRowView.LiteSummaryto render a compact, first-metric-focused summary (with optional progress bar). - Refactors
OverviewMenuCardRowViewto avoid rendering the full usage/dashboard sections in Overview rows and to resolve a live model viaMenuCardRefreshMonitor. - Adds
OverviewMenuCardRowViewTeststo validate lite summary selection and monitor-resolved updates.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Tests/CodexBarTests/OverviewMenuCardRowViewTests.swift | Adds coverage for lite summary selection, refreshed model resolution through MenuCardRefreshMonitor, and dashboard-only suppression. |
| Sources/CodexBar/StatusItemController+MenuTypes.swift | Refactors Overview rows to a lighter header + LiteSummary + storage line, including live-model resolution support. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Text(subtitle.text) | ||
| .font(.footnote) | ||
| .foregroundStyle(self.subtitleColor(for: subtitle.style)) | ||
| .lineLimit(1) | ||
| .truncationMode(.tail) | ||
| .layoutPriority(1) |
| private func storageLine(_ storageText: String) -> some View { | ||
| HStack(alignment: .firstTextBaseline, spacing: 4) { | ||
| Text("\(L("Storage")):") | ||
| Text(storageText) | ||
| .lineLimit(1) | ||
| .truncationMode(.tail) | ||
| Spacer(minLength: 8) | ||
| } | ||
| .font(.footnote) | ||
| .foregroundStyle(MenuHighlightStyle.secondary(self.isHighlighted)) | ||
| } |
|
|
||
| let summary = try #require(row.liteSummary) | ||
| #expect(summary.progressPercent == 75) | ||
| #expect(summary.progressAccessibilityLabel == "Usage remaining") |
Refs #1674.
Summary
LiteSummaryrow.InlineUsageDashboardContent/ chart-heavy usage sections in the Overview list.MenuCardRefreshMonitor-resolved live model.Why this direction
The scroll sample in #1674 points at repeated SwiftUI/AppKit view layout work during every menu scroll event. This version reduces the amount of SwiftUI content each Overview row owns, so the menu has less to relayout while scrolling. It is the lower-risk performance direction if maintainers are comfortable with a denser/lighter Overview presentation.
This is an alternative to the rich-row PR; they are not meant to merge together.
Tests
swift test --filter OverviewMenuCardRowViewTestsswift test --filter "overview row"swift buildmake checkgit diff --checkFollow-up validation
overview lite summary uses monitor resolved refreshed model, covering a stale row model resolving to refreshed quota/progress throughMenuCardRefreshMonitorwhile keeping the compact row free of dashboard-heavy SwiftUI content.make checkis clean after the follow-up commit.6f680eeb.Not covered