refactor: enforce minimal, why-only comments — codify the rule, then apply it everywhere

.agents/skills/writing-code/SKILL.md

@@ -38,28 +38,64 @@ Code follows a spec. If there is no spec for the change, stop and use the
 6. **Protocol-oriented** — the provider architecture depends on it. Each
    provider is a struct or actor conforming to `AIProvider`. The hub talks to
    the protocol, never to a concrete provider.
-7. **Comments earn their place** — never restate what the code says. Comment
-   only business decisions, non-obvious "why", or provider-specific quirks
-   (e.g., "z.ai uses a 5-hour rolling window, not calendar hours"). If code
-   needs a comment to be understood, refactor the code first.
-   - **Never describe past behavior or the change itself.** Comments exist to
-     explain the current code, not to justify what it replaced or how it
-     "fixes" something. Words like "replaces", "used to", "previously",
-     "instead of", or "before this change" are a smell — rewrite the comment
-     to describe only what the code does now.
-   - When a comment refers to a spec, cite it in **MLA form** — `(topic NN)`,
-     e.g. `// peak-hours calculation follows the window defined in (providers 02)`.
-     See [`writing-specs`](../writing-specs/SKILL.md).
-   - **Any acceptance-criteria (AC) marker** — e.g. `// AC1`, `// ── AC1 ──`,
-     `/// AC3 — refresh cadence` — Overall, acceptance criteria SHOULD not be
-     used unless they provide a clarification that is critical to understand
-     a piece of code. Must code DO NOT NEED IT, the spec reference (MLA) is
-     enough most of the time. If it needs to be used then itMUST include the MLA
-     citation of the spec that owns it. A bare `AC1` is meaningless without its
-     spec; always write
-     `// AC1: totals invariant (core 01)` or
-     `// ── AC3: refresh-on-foreground (core 01) ──`. Code review must reject
-     any AC marker that lacks a parenthesised spec citation.
+7. **Comments are a last resort — the code is the comment.** The default is
+   no comment at all. Specs justify the code's existence; the code justifies
+   its own behavior. Most functions, types, and blocks in this repo need
+   zero comments. New code that adds comments to justify its existence is
+   wrong — delete the comments, not the code.
+
+   A comment earns its place **only** when it carries information that the
+   code and the spec together cannot convey. The valid cases, exhaustively:
+   - A non-obvious **why** — a business rule, external constraint, or
+     provider-specific quirk the code cannot express by itself. Example:
+     `// z.ai bills a 5-hour rolling window, not calendar hours`.
+   - A surprising platform behavior that forced a workaround — stated as a
+     fact about the current code, never about what it replaced.
+
+   A comment is **not** justified when it:
+   - **Restates what the code does.** If `Int(percentage.rounded())` is
+     preceded by `// round to the nearest percent`, the comment is the
+     problem, not the code. Delete it.
+   - **Justifies the code's existence or ties it to an acceptance
+     criterion.** That is the spec's job. "AC1", "satisfies", "implements
+     spec", or any phrase that exists to prove the line belongs are smells —
+     cut them and the comment around them.
+   - **Documents a type or function whose name + signature already convey
+     it** — e.g. `/// Saves the API key` above `func save(...)`.
+   - **Describes past behavior or the change itself.** Words like
+     "replaces", "used to", "previously", "instead of", or "before this
+     change" are a smell — describe only what the code does now, or delete
+     the comment.
+
+   **If a comment refers to a spec**, cite it in **MLA form** — `(topic NN)`,
+   e.g. `// peak-hours window matches (providers 02)`. See
+   [`writing-specs`](../writing-specs/SKILL.md). Spec citations are optional
+   and rare — add one only when the *why* genuinely comes from the spec and
+   the code would read as misleading without it. Do not sprinkle citations
+   to "prove" the code belongs; that is restating existence, which is banned
+   above.
+
+   **Acceptance-criteria markers are forbidden.** `// AC1`, `// ── AC3 ──`,
+   `/// AC5 — fallback`, and every variant are noise: the spec already
+   enumerates its ACs, the code is the evidence, and a marker cannot be
+   verified against either. If an AC genuinely shapes a non-obvious
+   decision, write a plain-English `// why` comment and cite the spec in MLA
+   form. Delete every existing AC marker on sight.
+
+   **Doc comments (`///`)** — SwiftFormat's `docComments` rule (default-on,
+   no `.swiftformat` opt-out in this repo) is the authority on `//` vs `///`:
+   any comment directly above a declaration (public or not) **must** be `///`,
+   and any comment *not* above a declaration **must** be `//`. So a surviving
+   *why* that documents a type/function/property is written as `///` — never
+   downgraded to `//` to "make it less official". That said, the rule is about
+   the *syntax* of comments that exist, not a license to add them: most
+   declarations still need no comment at all. Never write a doc comment that
+   restates the signature.
+
+   When in doubt, delete the comment. If the code then becomes unreadable,
+   write a better comment that explains only the non-obvious *why* — do not
+   restore the restatement.
+
 8. **Dependencies** — if a package is not in `Package.swift`, add it before
    using it. Prefer Apple frameworks over third-party packages.
 9. **Concurrency** — use Swift's structured concurrency (`async/await`,
@@ -176,8 +212,11 @@ changed** and verify:
 - [ ] **Test quality** — do the tests assert the right things, or are they just
   "tests to pass"?
 - [ ] **Scope creep** — did you change anything beyond the spec?
-- [ ] **MLA citations** — any comment referencing a spec uses `(topic NN)`
-  form.
+- [ ] **Comments are minimal** — every remaining comment explains a
+  non-obvious *why*, not *what* the code does. No restating the signature,
+  no AC markers, no justifying existence. When in doubt the comment is gone.
+- [ ] **MLA citations** — any comment that *does* reference a spec uses
+  `(topic NN)` form. No bare AC markers anywhere.
 - [ ] **No secrets in logs** — no API keys, tokens, or auth headers in
   `print()`, `os_log`, or debug output.
 

Sources/App/APIKeyEntryState.swift

@@ -1,7 +1,5 @@
 import Foundation
 
-/// Transient UI state for an API-key entry row. The key remains only in the
-/// secure field until a successful Keychain write clears it.
 struct APIKeyEntryState {
     private(set) var input = ""
     private(set) var errorMessage: String?

Sources/App/AppMain.swift

@@ -23,11 +23,6 @@ struct AppMain: App {
     }
 
     var body: some Scene {
-        // AC1: Menu Bar popover (ui 02). The visible menu-bar content is the
-        // `label:` view — a live status ring (ui 10). The `MenuBarStatusIcon`
-        // carries its own accessibility label: a per-state sentence for
-        // window/balance modes (ui 10 AC9), and "Filbert" for the fallback,
-        // preserving the original menu-bar announcement.
         MenuBarExtra {
             QuotaView(viewModel: viewModel)
                 .frame(width: 280)
@@ -36,7 +31,6 @@ struct AppMain: App {
         }
         .menuBarExtraStyle(.window)
 
-        // AC1: Standalone Settings scene (ui 02)
         Settings {
             SettingsView(viewModel: viewModel)
         }
@@ -51,8 +45,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate {
         NSApplication.shared.setActivationPolicy(.accessory)
 
         // The app runs as a SwiftPM executable without a full `.app` bundle, so
-        // set the application icon programmatically from the bundled `.icns`.
-        // This is what Notification Center widgets and any window chrome pick up.
+        // the icon is set programmatically from the bundled `.icns`.
         let iconURL = Bundle.module.url(forResource: "AppIcon", withExtension: "icns")
         let icon = iconURL.flatMap(NSImage.init(contentsOf:))
         if let icon {

Sources/App/AppearanceSettingsView.swift

@@ -14,8 +14,6 @@ struct AppearanceTab: View {
                 heading: String(localized: "Provider order"),
                 description: String(localized: "Drag providers to change their order.")
             ) {
-                // (ui 16) Only configured providers are listed; an empty
-                // configured set shows a hint instead of a blank list.
                 if viewModel.configuredProvidersOrdered.isEmpty {
                     providerOrderEmptyHint
                 } else {
@@ -45,10 +43,6 @@ struct AppearanceTab: View {
     }
 
     private var providerOrderRows: some View {
-        // (ui 16) The rendered list is the configured subset, but the drop
-        // delegate and `moveProvider` resolve indices against the full
-        // ordered list so saved positions of unconfigured (hidden) providers
-        // are preserved.
         let visible = viewModel.configuredProvidersOrdered
         return VStack(spacing: 0) {
             ForEach(Array(visible.enumerated()), id: \.element.id) { index, provider in
@@ -104,9 +98,8 @@ struct AppearanceTab: View {
         }
     }
 
-    /// Moves a visible row in the direction indicated. Indices are resolved
-    /// against the full ordered list (`registeredProvidersOrdered`) so the
-    /// saved positions of unconfigured (hidden) providers stay intact (ui 16).
+    /// Indices resolve against the full ordered list (`registeredProvidersOrdered`)
+    /// so saved positions of unconfigured (hidden) providers stay intact.
     private func moveProvider(at visibleIndex: Int, direction: Int) {
         let visible = viewModel.configuredProvidersOrdered
         let destinationVisibleIndex = visibleIndex + direction

Sources/App/MenuBarStatusIcon.swift

@@ -1,17 +1,11 @@
 import Core
 import SwiftUI
 
-// MARK: - Menu-bar live status icon (ui 10)
+// MARK: - Menu-bar live status icon
 
-/// The menu-bar label: a ring + short text reflecting the top-most configured
-/// provider's live status (ui 10 AC1/AC2).
-///
 /// The ring is rendered into a bitmap `NSImage` instead of a SwiftUI `Shape`
 /// because `MenuBarExtra`'s label layer on macOS 14 silently drops arbitrary
-/// `Shape` / `Canvas` content — only `Text` and `Image` reliably render. The
-/// bitmap is cached per 10% bucket (ui 10 AC6 — rounding) so a refresh that
-/// stays inside the same bucket never re-rasterizes (ui 10 AC7 — no extra cost
-/// on the existing 5-minute cadence).
+/// `Shape` / `Canvas` content — only `Text` and `Image` reliably render.
 @MainActor
 struct MenuBarStatusIcon: View {
     let viewModel: QuotaViewModel
@@ -79,15 +73,13 @@ struct MenuBarStatusIcon: View {
         }
     }
 
-    /// AC5: no usable data — fall back to the static SF Symbol, no text (ui 10).
     private var fallbackIcon: some View {
         Image(systemName: "brain.head.profile")
             .accessibilityLabel(String(localized: "Filbert"))
     }
 
     // MARK: - Resolution
 
-    /// The top-most configured provider's resolved status (ui 10 AC2/AC5).
     private var resolvedStatus: Resolved? {
         guard let id = viewModel.configuredProviderIds.first,
               case let .loaded(quota) = viewModel.providerStates[id]
@@ -99,36 +91,28 @@ struct MenuBarStatusIcon: View {
         return Resolved(providerName: quota.providerName, status: status)
     }
 
-    /// Quantizes a `[0, 1]` fraction into a 10%-bucketed `clampedFraction`
-    /// (ui 10 AC6). Rounds to the nearest 10%: 0.03 → 0, 0.05 → 0.1, 0.22 → 0.2,
-    /// 0.97 → 1. Clamped to `[0, 1]` first so out-of-range values stay bounded.
     private func bucket(from fraction: Double) -> Double {
         let clamped = QuotaStatusResolver.clampedFraction(fraction)
         return (clamped * 10).rounded() / 10
     }
 
     private func percentageText(_ percentage: Double) -> String {
-        // AC9: rounded to the nearest whole percent for the visible label.
         let rounded = Int(percentage.rounded())
         return String(localized: "\(rounded)%")
     }
 
     private func accessibilityPercentage(providerName: String, percentage: Double) -> String {
-        // AC9/AC10: a localized human sentence, e.g. "Claude Code: 42% used".
         let pct = Int(percentage.rounded())
         return String(localized: "\(providerName): \(pct)% used")
     }
 
     private func accessibilityBalance(providerName: String, total: Double) -> String {
-        // AC9/AC10: a localized human sentence, e.g. "DeepSeek: $12.34 remaining".
         let amount = QuotaStatusResolver.amountText(
             for: UsageLine(label: "", total: total, unit: nil)
         ) ?? String(format: "%.2f", total)
         return String(localized: "\(providerName): \(amount) remaining")
     }
 
-    /// Bundles the resolved status with the provider name so accessibility
-    /// labels can produce a human sentence without re-reading the view model.
     private struct Resolved {
         let providerName: String
         let status: QuotaStatusResolver.Status
@@ -137,15 +121,9 @@ struct MenuBarStatusIcon: View {
 
 // MARK: - Ring image (bitmap-backed)
 
-/// SwiftUI wrapper around the cached bitmap ring for a given 10% bucket.
-///
-/// The image is drawn in solid black (`Color.black`); `MenuBarExtra` applies
-/// its own template tint (black in light mode, white in dark mode, blue while
-/// highlighted), so the menu-bar ring respects the OS chrome (ui 10 AC8).
+/// The image is drawn in solid black because `MenuBarExtra` applies its own
+/// template tint, so the menu-bar ring respects the OS chrome.
 private struct MenuBarRingImage: View {
-    /// `[0, 1]` quantized to 10% buckets. Expected to come from
-    /// `MenuBarStatusIcon.bucket(from:)`; passing an unquantized value still
-    /// works but defeats the cache.
     let bucket: Double
 
     var body: some View {
@@ -156,14 +134,9 @@ private struct MenuBarRingImage: View {
     }
 }
 
-/// Lazily-built, process-wide cache of ring bitmaps keyed by 10% bucket.
-///
-/// 11 images cover the full 0–100% range (ui 10 AC6). Re-renders only fire
-/// when the resolved bucket changes between refreshes, never on every tick
-/// (ui 10 AC7).
 private enum MenuBarRingImageCache {
     /// `nonisolated(unsafe)`: SwiftUI renders on the main actor, so the lazy
-    /// populate is single-threaded in practice (ci 04 Plan §4).
+    /// populate is single-threaded in practice.
     private nonisolated(unsafe) static var cache: [Int: Image] = [:]
 
     static func image(for bucket: Double) -> Image {
@@ -177,43 +150,29 @@ private enum MenuBarRingImageCache {
         return image
     }
 
-    /// Maps a quantized fraction to its 0…10 integer bucket key.
     private static func bucketKey(_ bucket: Double) -> Int {
         let clamped = QuotaStatusResolver.clampedFraction(bucket)
         return Int((clamped * 10).rounded())
     }
 }
 
-/// Draws the ring — a tracked background arc plus a filled foreground arc —
-/// into a bitmap `NSImage` using `CGContext`.
-///
-/// Drawn in solid black so `MenuBarExtra` can template-tint it (ui 10 AC8).
-/// The geometry matches the open-arc look the spec describes: ~85% of the
-/// circumference is the drawable arc, the remaining 15% is a fixed visual gap
-/// so 100% still reads as a ring with an opening rather than a closed pie
-/// (ui 10 AC3/Plan §2).
+/// Drawn in solid black so `MenuBarExtra` can template-tint it.
 private enum MenuBarRingRenderer {
-    /// Pixel side-length of the rendered bitmap. @2x for crispness on Retina.
     private static let pixels = 28
 
-    /// 85% of the circumference is drawable; 15% is the fixed gap.
     private static let drawableRatio: CGFloat = 0.85
 
-    /// Visible stroke width in points (the bitmap is `pixels / 2` points wide).
     private static let lineWidth: CGFloat = 4
 
     static func render(fraction: Double) -> NSImage {
         let points = CGFloat(pixels) / 2
         guard let context = makeContext() else {
-            // If bitmap allocation fails, return a transparent image — the
-            // fallback SF Symbol takes over via the icon's fallback branch.
             return NSImage(size: NSSize(width: points, height: points))
         }
         drawRing(into: context, fraction: fraction)
         return makeImage(from: context, points: points)
     }
 
-    /// Allocates the grayscale, alpha-only bitmap the ring is drawn into.
     private static func makeContext() -> CGContext? {
         CGContext(
             data: nil,
@@ -226,15 +185,12 @@ private enum MenuBarRingRenderer {
         )
     }
 
-    /// Strokes the tracked background arc and the foreground fill arc.
     /// `CGContext`'s default arc origin is 3 o'clock (0° = positive X), so the
-    /// context is rotated -90° first to make the arc start at 12 o'clock
-    /// (ui 10 AC3).
+    /// context is rotated -90° to make the arc start at 12 o'clock.
     ///
-    /// At 100% the gap closes — the arc spans the full `2π` so a complete
-    /// budget reads as a complete circle rather than a 99% ring. Buckets 0–90%
-    /// keep the 85% drawable ratio so partial progress still has the open-arc
-    /// look (ui 10 AC3/Plan §2).
+    /// At 100% the arc spans the full `2π` so a complete budget reads as a
+    /// complete circle rather than a 99% ring; buckets 0–90% keep the open-arc
+    /// gap.
     private static func drawRing(into context: CGContext, fraction: Double) {
         let clamped = QuotaStatusResolver.clampedFraction(fraction)
         let bounds = CGRect(x: 0, y: 0, width: pixels, height: pixels)
@@ -249,8 +205,7 @@ private enum MenuBarRingRenderer {
         context.rotate(by: -.pi / 2)
         context.translateBy(x: -center.x, y: -center.y)
 
-        // Track: the full drawable arc at low opacity. Always uses the gap
-        // look — even at 100% the track shows where the closing seam sits.
+        // Track: the full drawable arc at low opacity.
         context.addArc(
             center: center,
             radius: radius,
@@ -261,7 +216,6 @@ private enum MenuBarRingRenderer {
         context.setStrokeColor(CGColor(gray: 0, alpha: 0.2))
         context.strokePath()
 
-        // Fill: the foreground arc, solid black so MenuBarExtra can tint it.
         let fillEnd = 2 * .pi * spanRatio * clamped
         guard fillEnd > 0 else { return }
         context.addArc(
@@ -275,8 +229,6 @@ private enum MenuBarRingRenderer {
         context.strokePath()
     }
 
-    /// Wraps the rendered bitmap as a template `NSImage` so MenuBarExtra
-    /// applies its own OS tint (ui 10 AC8).
     private static func makeImage(from context: CGContext, points: CGFloat) -> NSImage {
         guard let cgImage = context.makeImage() else {
             return NSImage(size: NSSize(width: points, height: points))
@@ -303,7 +255,7 @@ private struct MenuBarMacFaceImage: View {
 
 private enum MenuBarMacFaceCache {
     /// `nonisolated(unsafe)`: SwiftUI renders on the main actor, so the lazy
-    /// populate is single-threaded in practice (ci 04 Plan §4).
+    /// populate is single-threaded in practice.
     private nonisolated(unsafe) static var cache: [QuotaStatusResolver.Tier: Image] = [:]
 
     static func image(for tier: QuotaStatusResolver.Tier) -> Image {

Sources/App/ProviderVisualStyle.swift

@@ -9,8 +9,8 @@ enum ProviderVisualStyle {
     static let cardCornerRadius: CGFloat = 10
     static let neutralContainerFill = Color.secondary.opacity(0.08)
 
-    /// Light values retain the contrast-tuned palette from (ui 11); dark mode
-    /// uses the semantic system colors so it follows the active appearance.
+    /// Light mode uses a contrast-tuned custom palette; dark mode uses semantic
+    /// system colors to follow the active appearance.
     static func tierColor(
         _ tier: QuotaStatusResolver.Tier,
         scheme: ColorScheme