Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
80 commits
Select commit Hold shift + click to select a range
086162e
feat: Phase 1 - add PieProgressView component (#297)
eonist May 8, 2026
b05078b
feat: Phase 2 - combined stats header, remove System section + Quit f…
eonist May 8, 2026
dce66a3
feat: Phase 3 - action row redesign + startedAgo (#302)
eonist May 8, 2026
119cae7
feat: Phase 4 - inline job rows, remove Active Jobs section (#304)
eonist May 8, 2026
93f2695
feat: Phase 5 - ScrollView + Load more pagination (#305)
eonist May 8, 2026
4556f24
feat: Phase 6 - conditional runners sub-section (#307)
eonist May 8, 2026
70189fe
fix: apply review findings #1–#11 from PR #311 feedback
eonist May 8, 2026
87fff9b
fix: #314 remaining merit items #5 #6 #8 #9
eonist May 8, 2026
114cfa8
fix: resolve SwiftLint violations in PopoverMainView
eonist May 8, 2026
4a4a0c5
ci: retrigger SwiftLint — local lint clean (0 violations in 40 files)
eonist May 8, 2026
f998973
fix: rename r → radius in PieProgressView (identifier_name SwiftLint)
eonist May 8, 2026
e8864f7
fix: remove halo at progress>=1 in PieProgressView; drop misleading p…
eonist May 8, 2026
b29ac09
fix: raise RunnerStore actions/jobs display caps for pagination (#305)
eonist May 8, 2026
668224d
fix: raise store caps + refactor PopoverMainView subviews + expand/co…
eonist May 8, 2026
51aadb1
fix: resolve swiftlint errors in PopoverMainView (contains_over_filte…
eonist May 8, 2026
7368e8f
fix: address PR #311 review feedback
eonist May 8, 2026
043d484
fix: runners section above actions + actionDotColor uses group.conclu…
eonist May 8, 2026
6c41d6b
fix: wire runner CPU/MEM metrics into RunnersListView (#311)
eonist May 8, 2026
0d20819
fix: Phase 5 'No more actions' text + Phase 6 busy-only runner filter…
eonist May 8, 2026
15021c0
fix: resolve all gaps from issue #323 (phases 4, 5 & 6 follow-up)
eonist May 8, 2026
07d6414
fix: resolve SwiftLint CI failures (missing_docs, file_length, functi…
eonist May 8, 2026
9e731bc
fix: remove superfluous function_body_length disable; rename `m` → `m…
eonist May 8, 2026
75550c9
fix: cherry-pick 9 items from #313 into #311 (closes #366)
eonist May 9, 2026
e00ce1c
refactor: add ActiveJob.progressFraction; remove redundant helper in …
eonist May 9, 2026
888288d
fix: resolve SwiftLint CI failures in RunnerStore.swift and GitHub.swift
eonist May 9, 2026
d70d046
fix: resolve remaining SwiftLint violations
eonist May 9, 2026
43a214c
fix: resolve all remaining SwiftLint violations
eonist May 9, 2026
2da30b8
fix: add file_length disable to RunnerStore.swift (411 lines, cohesiv…
eonist May 9, 2026
c8e8118
fix: replace bare \\u2014 and \\u001B with braced Swift unicode escapes
eonist May 9, 2026
500bbd3
fix: remove swiftlint:enable file_length from GitHub.swift and Runner…
eonist May 9, 2026
6a3fe6d
fix: resolve all compile errors across 7 files
eonist May 9, 2026
28d973c
fix(swiftlint): extract ActionRunsResponse DTOs to drop ActionGroup.s…
eonist May 9, 2026
0085057
fix: raise file_length warning to 500; remove superfluous disables fr…
eonist May 9, 2026
1a143ce
fix: add missing_docs to ActionRunsResponse.swift; rename pr → prRef …
eonist May 9, 2026
3e1513c
fix(swiftlint): remove stray file_length disable from RunnerStore.swift
eonist May 9, 2026
751a4b7
fix: 3 compile errors — ActionGroup Equatable, makeActiveJob Date fla…
eonist May 9, 2026
e274658
fix: resolve all remaining SwiftLint violations
eonist May 9, 2026
0e387db
fix(swiftlint): add missing_docs for RunnerStoreState internal declar…
eonist May 9, 2026
2d98df2
fix: use macOS 13-compatible onChange(of:perform:) in PopoverMainView
eonist May 9, 2026
0ee6d84
fix: replace Unicode block bar with native SwiftUI bar view in stat c…
eonist May 9, 2026
779ade0
fix: PieProgressView pie wedge (GeometryReader frame), ActionDetailVi…
eonist May 9, 2026
e592344
fix: resolve all Swift compiler warnings
eonist May 9, 2026
8a39ad9
fix(swiftlint): restore missing_docs on ActiveJob and JobStep properties
eonist May 9, 2026
8bcadaa
fix: resize popover on navigate() so ActionDetailView/SettingsView ge…
eonist May 9, 2026
dd3718a
fix(swiftlint): missing_docs on WorkflowRunRef, RunnerMetrics; remove…
eonist May 9, 2026
77a7d8e
fix: defer fittingSize read to next run-loop tick after rootView swap…
eonist May 9, 2026
7162557
fix(swiftlint): remove superfluous disables, rename hc, add missing_d…
eonist May 9, 2026
4555aa4
fix: ActionDetailView live data + fit-to-content height (#296)
eonist May 9, 2026
8775c11
fix: revert ActionDetailView to safe layout; fix height via AppDelega…
eonist May 9, 2026
c80f4fc
fix(swiftlint): rename all short vars in AppDelegate, doc extension R…
eonist May 9, 2026
64b0fdd
revert: restore AppDelegate + ActionDetailView verbatim to 77a7d8e (p…
eonist May 9, 2026
a052859
fix(swiftlint): suppress missing_docs on static func == in ActionGroup
eonist May 9, 2026
2668474
fix(swiftlint): rename hc->hostCtrl h->fixedH in AppDelegate; fix Act…
eonist May 9, 2026
c2e3935
fix: reload observable while popover open; reduce detailHeight to 320
eonist May 9, 2026
1e7c776
fix(swiftlint): rename all short vars hc/h/fit in AppDelegate (identi…
eonist May 9, 2026
dc4a801
fix: eliminate side-jumping — navigate() is rootView swap only, zero …
eonist May 9, 2026
8c3619f
fix: remove ScrollView + .frame(maxHeight:) from ActionsListView — vi…
eonist May 9, 2026
ddeb34f
fix(swiftlint): suppress missing_docs on static func == (Equatable)
eonist May 9, 2026
4770a9f
fix: stable inline jobs + maxHeight cap + openPopover mirrors main
eonist May 9, 2026
6a6c330
Create .gitignore
eonist May 10, 2026
900b8b3
Merge branch 'feature/296-popover-main-view-redesign' of https://gith…
eonist May 10, 2026
1c79c90
fix(swiftlint): remove superfluous disable directive in ActionGroup ==
eonist May 10, 2026
ef19719
fix(swiftlint): inline Equatable into ActionGroup struct
eonist May 10, 2026
e8e3da8
fix(swiftlint): silence missing_docs on Equatable ==
eonist May 10, 2026
4c5d8a3
fix: inline job status labels + openPopover minHeight for detail views
eonist May 10, 2026
6793202
fix(swiftlint): file-wide disable missing_docs in ActionGroup
eonist May 10, 2026
b5f1150
fix(swiftlint): suppress opening_brace in PopoverMainView
eonist May 10, 2026
fb2ee8e
revert: restore AppDelegate.swift sizing contract — remove bad minHeight
eonist May 10, 2026
dd95aa6
fix: flush layout before reading fittingSize in openPopover()
eonist May 10, 2026
fbbc3e9
fix: restore !popoverIsOpen guard on reload() — matches main branch c…
eonist May 10, 2026
07f65bc
fix: defer fittingSize read by one tick; enrich group jobs on-demand …
eonist May 10, 2026
4067591
fix: suppress unused-binding warnings in openPopover() outer guard
eonist May 10, 2026
5222fc9
fix: resize popover to fittingSize on navigate; show In Progress for …
eonist May 10, 2026
7a33f86
fix: inline jobs show only in_progress jobs per spec #178; dynamic wi…
eonist May 10, 2026
d34de9b
fix: sticky header + scrollable body in PopoverMainView; header never…
eonist May 10, 2026
b01f70a
fix: remove maxWidth:.infinity from root frame (fixes side-jump), fix…
eonist May 10, 2026
94a5a91
fix: restore zero-size navigate() contract + remove ScrollView (fixes…
eonist May 10, 2026
eb84978
fix: restore openPopover() to fully synchronous sizing like main (fix…
eonist May 10, 2026
1d56db5
fix: strip @MainActor/@unchecked Sendable — match main class structur…
eonist May 10, 2026
f41d3ab
fix: cap ScrollView maxHeight in drill-down views to prevent side-jum…
eonist May 10, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
.build
dist
8 changes: 4 additions & 4 deletions .swiftlint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ excluded:
- .build
- Package.swift

# ── Opt-in rules ────────────────────────────────────────────────────────────────────────
# ── Opt-in rules ────────────────────────────────────────────────────────────────────────────────────
opt_in_rules:
- missing_docs # require /// on all internal/public declarations
- closure_spacing # consistent spacing inside closure braces
Expand All @@ -23,7 +23,7 @@ opt_in_rules:
- empty_collection_literal # prefer isEmpty over == []
- first_where # prefer first(where:) over filter().first

# ── Rule configuration ───────────────────────────────────────────────────────────────────
# ── Rule configuration ─────────────────────────────────────────────────────────────────────────────────────
missing_docs:
warning:
- internal
Expand All @@ -40,7 +40,7 @@ line_length:
ignores_urls: true

file_length:
warning: 400
warning: 500
error: 600

type_body_length:
Expand All @@ -61,7 +61,7 @@ nesting:
function_level:
warning: 3

# ── Disabled rules ───────────────────────────────────────────────────────────────────────
# ── Disabled rules ──────────────────────────────────────────────────────────────────────────────────────
disabled_rules:
- todo # TODOs are acceptable during active development
- trailing_comma # SwiftPM/Xcode auto-formatter conflicts with this
58 changes: 45 additions & 13 deletions Sources/RunnerBar/ActionDetailView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,24 @@ import SwiftUI
// ═══════════════════════════════════════════════════════════════════════════════
//
// ── FRAME CONTRACT ──────────────────────────────────────────────────────────────────────────────────────
// Receives the same FIXED frame from AppDelegate as JobDetailView.
// Sized once at openPopover() from mainView()'s fittingSize; never changes.
// ScrollView absorbs overflow — do NOT fight the frame.
// Architecture 1: sizingOptions = .preferredContentSize.
// Root: .frame(maxWidth: .infinity, alignment: .top) — NO maxHeight: .infinity
// ScrollView: .frame(maxHeight: 75% of visible screen) — REQUIRED to prevent side-jump (#370)
//
// ── WHY THE ScrollView CAP IS REQUIRED ──────────────────────────────────────────────────────────────────
// Without .frame(maxHeight:), ScrollView reports its full content height as ideal height.
// NSHostingController publishes this as preferredContentSize.height.
// NSPopover re-anchors on any contentSize change → side-jump on every navigation.
// The cap makes preferredContentSize.height predictable and stable.
//
// ── LAYOUT RULES ────────────────────────────────────────────────────────────────────────────────────────
// ✔ Root: .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .top)
// ✔ Root: .frame(maxWidth: .infinity, alignment: .top) — NO maxHeight: .infinity
// ✔ ScrollView: .frame(maxHeight: NSScreen.main.map { $0.visibleFrame.height * 0.75 } ?? 600)
// ✔ Job list MUST be inside ScrollView
// ✔ Header (back button + title + Divider) MUST be OUTSIDE ScrollView
// ❌ NEVER put header inside ScrollView
// ❌ NEVER add .idealWidth or .frame(height:) to root
// ❌ NEVER remove the .frame(maxHeight:) from ScrollView — side-jump regression #370
// ❌ NEVER call navigate() directly — use onBack / onSelectJob callbacks
// ═══════════════════════════════════════════════════════════════════════════════

Expand Down Expand Up @@ -124,6 +132,9 @@ struct ActionDetailView: View {
Divider()

// ── Jobs list: INSIDE ScrollView
// ⚠️ .frame(maxHeight:) is REQUIRED — do NOT remove.
// Without it, ScrollView reports full content height as ideal height,
// causing preferredContentSize.height to spike → NSPopover side-jump (#370).
ScrollView(.vertical, showsIndicators: true) {
VStack(alignment: .leading, spacing: 0) {
if group.jobs.isEmpty {
Expand All @@ -136,9 +147,12 @@ struct ActionDetailView: View {
ForEach(group.jobs) { job in
Button(action: { onSelectJob(job) }, label: {
HStack(spacing: 8) {
Circle()
.fill(jobDotColor(for: job))
.frame(width: 7, height: 7)
// ⚠️ PieProgressView — not plain Circle().
PieProgressView(
progress: job.progressFraction,
color: jobDotColor(for: job),
size: 7
)
Text(job.name)
.font(.system(size: 12))
.foregroundColor(job.isDimmed ? .secondary : .primary)
Expand Down Expand Up @@ -174,8 +188,13 @@ struct ActionDetailView: View {
}
.frame(maxWidth: .infinity, alignment: .leading)
}
// ⚠️ REQUIRED — caps preferredContentSize.height under Architecture 1.
// Prevents NSPopover side-jump on navigation (#370).
// ❌ NEVER remove this modifier.
.frame(maxHeight: NSScreen.main.map { $0.visibleFrame.height * 0.75 } ?? 600)
}
.frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .top)
// ⚠️ NO maxHeight: .infinity — height is driven by preferredContentSize via sizingOptions
.frame(maxWidth: .infinity, alignment: .top)
.onAppear {
tickTimer?.invalidate()
tickTimer = Timer.scheduledTimer(withTimeInterval: 1, repeats: true) { _ in tick += 1 }
Expand All @@ -191,20 +210,33 @@ struct ActionDetailView: View {
// MARK: - Job row helpers

private func jobDotColor(for job: ActiveJob) -> Color {
if job.isDimmed { return .secondary }
return job.status == "in_progress" ? .yellow : .gray
switch job.status {
case "in_progress": return .yellow
case "queued": return group.groupStatus == .inProgress ? .yellow : .blue
default:
if job.isDimmed { return .gray }
return job.conclusion == "success" ? .green : .red
}
}

/// Returns the status label for a job without a conclusion.
/// Per spec #178: queued jobs inside an in-progress group show "In Progress"
/// because they are part of an active workflow run.
private func jobStatusLabel(for job: ActiveJob) -> String {
switch job.status {
case "in_progress": return "In Progress"
case "queued": return "Queued"
default: return "Pending"
case "queued":
return group.groupStatus == .inProgress ? "In Progress" : "Queued"
default: return "Pending"
}
}

private func jobStatusColor(for job: ActiveJob) -> Color {
job.status == "in_progress" ? .yellow : .secondary
switch job.status {
case "in_progress": return .yellow
case "queued": return group.groupStatus == .inProgress ? .yellow : .secondary
default: return .secondary
}
}

private func conclusionLabel(_ c: String) -> String {
Expand Down
130 changes: 58 additions & 72 deletions Sources/RunnerBar/ActionGroup.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import Foundation
// swiftlint:disable opening_brace identifier_name missing_docs orphaned_doc_comment
// swiftlint:disable opening_brace identifier_name orphaned_doc_comment missing_docs

// MARK: - GroupStatus

Expand All @@ -20,10 +20,15 @@ enum GroupStatus {
/// Holds only the data needed for display and job fetching — deliberately
/// minimal so the full job list lives on the parent ActionGroup instead.
struct WorkflowRunRef: Identifiable {
/// Unique GitHub run identifier.
let id: Int
let name: String // workflow file name, e.g. "SonarQube", "vitest"
/// Workflow file name, e.g. "SonarQube", "vitest".
let name: String
/// Current run status: `queued`, `in_progress`, or `completed`.
let status: String
/// Final outcome when status is `completed`.
let conclusion: String?
/// URL to the run's page on github.com.
let htmlUrl: String?
}

Expand All @@ -36,7 +41,7 @@ struct WorkflowRunRef: Identifiable {
/// Hierarchy: ActionGroup → jobs (flat across all sibling runs) → JobStep → log.
/// `ActionDetailView` drills into the flat job list; `JobDetailView`/`StepLogView`
/// are reused unchanged below that.
struct ActionGroup: Identifiable {
struct ActionGroup: Identifiable, Equatable {
let headSha: String // head_sha — kept as the underlying group identity
let label: String // "#1270" if PR, else "d6281b" (sha[:7])
let title: String // commit/PR message first line (≤40 chars)
Expand All @@ -58,6 +63,7 @@ struct ActionGroup: Identifiable {
/// Timestamps derived from job data, not run-level API fields.
/// Mirrors ci-dash.py's `first_job_started_at` / `last_job_completed_at`.
var firstJobStartedAt: Date?
/// Last job completion time across all runs in this group.
var lastJobCompletedAt: Date?

/// Fallback creation time from the representative run.
Expand Down Expand Up @@ -116,77 +122,57 @@ struct ActionGroup: Identifiable {

/// Name of the first in-progress job, or first queued, or "—".
var currentJobName: String {
if let j = jobs.first(where: { $0.status == "in_progress" }) { return j.name }
if let j = jobs.first(where: { $0.status == "queued" }) { return j.name }
if let job = jobs.first(where: { $0.status == "in_progress" }) { return job.name }
if let job = jobs.first(where: { $0.status == "queued" }) { return job.name }
return "—"
}

/// How long ago the group started, as a short human string, e.g. "3m ago", "1h ago".
/// Uses `firstJobStartedAt` when available, falls back to `createdAt`.
/// Returns "—" if neither timestamp is available.
/// Delegates to `RelativeTimeFormatter` for testable, injectable formatting.
var startedAgo: String {
guard let ref = firstJobStartedAt ?? createdAt else { return "—" }
return RelativeTimeFormatter.string(from: ref)
}

/// Elapsed time derived from min(job.startedAt) → max(job.completedAt).
var elapsed: String {
if let start = firstJobStartedAt {
let end = lastJobCompletedAt ?? Date()
let sec = Int(end.timeIntervalSince(start))
guard sec >= 0 else { return "00:00" }
let m = sec / 60; let s = sec % 60
return String(format: "%02d:%02d", m, s)
let minutes = sec / 60; let seconds = sec % 60
return String(format: "%02d:%02d", minutes, seconds)
}
guard let start = createdAt else { return "00:00" }
let sec = Int(Date().timeIntervalSince(start))
guard sec >= 0 else { return "00:00" }
let m = sec / 60; let s = sec % 60
return String(format: "%02d:%02d", m, s)
let minutes = sec / 60; let seconds = sec % 60
return String(format: "%02d:%02d", minutes, seconds)
}
}

// MARK: - Codable helpers (private to this file)

private struct ActionRunsResponse: Codable {
let workflowRuns: [RunPayload]
enum CodingKeys: String, CodingKey { case workflowRuns = "workflow_runs" }
}

private struct RunPayload: Codable {
let id: Int
let name: String
let status: String
let conclusion: String?
let headBranch: String?
let headSha: String
let displayTitle: String?
let createdAt: String?
let updatedAt: String?
let htmlUrl: String?
let headCommit: HeadCommit?
let pullRequests: [PRRef]?

enum CodingKeys: String, CodingKey {
case id, name, status, conclusion
case headBranch = "head_branch"
case headSha = "head_sha"
case displayTitle = "display_title"
case createdAt = "created_at"
case updatedAt = "updated_at"
case htmlUrl = "html_url"
case headCommit = "head_commit"
case pullRequests = "pull_requests"
// MARK: - Progress fraction (model layer — keeps view bodies clean)

/// Completion fraction 0.0–1.0 for the pie-progress dot, or `nil` (indeterminate)
/// when status is queued or no job data is available.
///
/// - `nil` → indeterminate (queued / no jobs loaded yet)
/// - `1.0` → completed
/// - `0..<1` → partial (jobsDone / jobsTotal)
var progressFraction: Double? {
switch groupStatus {
case .queued: return nil
case .completed: return 1.0
case .inProgress:
guard jobsTotal > 0 else { return nil }
return Double(jobsDone) / Double(jobsTotal)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 ActionGroup.Equatable omits firstJobStartedAt — spurious equality causes stale inline job rows

The custom == implementation checks id, isDimmed, jobs, and runs.map(.id) but omits firstJobStartedAt (and lastJobCompletedAt). ActionRowView uses .onChange(of: store.actions) to reset visibleCount; if firstJobStartedAt changes (a new job starts timing) but no job is added/removed, the groups compare as equal. The bigger risk is that SwiftUI may skip re-rendering action rows whose timing data changed, keeping elapsed/startedAgo labels stale until the next structural change.

Suggested change
}
static func == (lhs: ActionGroup, rhs: ActionGroup) -> Bool {
lhs.id == rhs.id
&& lhs.isDimmed == rhs.isDimmed
&& lhs.jobs == rhs.jobs
&& lhs.runs.map({ $0.id }) == rhs.runs.map({ $0.id })
&& lhs.firstJobStartedAt == rhs.firstJobStartedAt
&& lhs.lastJobCompletedAt == rhs.lastJobCompletedAt
}
AI Fix Prompt
Fix the following High (Logic Error) issue in `Sources/RunnerBar/ActionGroup.swift` at line 170:

Problem: The custom == implementation checks id, isDimmed, jobs, and runs.map(\.id) but omits firstJobStartedAt (and lastJobCompletedAt). ActionRowView uses .onChange(of: store.actions) to reset visibleCount; if firstJobStartedAt changes (a new job starts timing) but no job is added/removed, the groups compare as equal. The bigger risk is that SwiftUI may skip re-rendering action rows whose timing data changed, keeping elapsed/startedAgo labels stale until the next structural change.

Suggested fix:
static func == (lhs: ActionGroup, rhs: ActionGroup) -> Bool {
    lhs.id == rhs.id
        && lhs.isDimmed == rhs.isDimmed
        && lhs.jobs == rhs.jobs
        && lhs.runs.map({ $0.id }) == rhs.runs.map({ $0.id })
        && lhs.firstJobStartedAt == rhs.firstJobStartedAt
        && lhs.lastJobCompletedAt == rhs.lastJobCompletedAt
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 ActionGroup.Equatable: runs compared by ID array only, not status/conclusion

ActionGroup.== compares lhs.runs.map({ $0.id }) == rhs.runs.map({ $0.id }) — run IDs only. A run that transitions from in_progresscompleted (same ID, different status/conclusion) will make two ActionGroups compare equal when their run IDs match. This means onChange(of: store.actions) in PopoverMainView will NOT fire when a run completes without adding/removing runs, so visibleCount won't reset and the UI won't reflect the completion state change. Consider including runs.map({ $0.status }) or runs.map({ ($0.id, $0.status, $0.conclusion) }) in the comparison.

Suggested change
}
static func == (lhs: ActionGroup, rhs: ActionGroup) -> Bool {
lhs.id == rhs.id
&& lhs.isDimmed == rhs.isDimmed
&& lhs.jobs == rhs.jobs
&& lhs.runs.map({ ($0.id, $0.status, $0.conclusion ?? "") })
== rhs.runs.map({ ($0.id, $0.status, $0.conclusion ?? "") })
}
AI Fix Prompt
Fix the following Medium (Logic Error) issue in `Sources/RunnerBar/ActionGroup.swift` at lines 168-170:

Problem: `ActionGroup.==` compares `lhs.runs.map({ $0.id }) == rhs.runs.map({ $0.id })` — run IDs only. A run that transitions from `in_progress` → `completed` (same ID, different status/conclusion) will make two ActionGroups compare equal when their run IDs match. This means `onChange(of: store.actions)` in `PopoverMainView` will NOT fire when a run completes without adding/removing runs, so `visibleCount` won't reset and the UI won't reflect the completion state change. Consider including `runs.map({ $0.status })` or `runs.map({ ($0.id, $0.status, $0.conclusion) })` in the comparison.

Suggested fix:
static func == (lhs: ActionGroup, rhs: ActionGroup) -> Bool {
    lhs.id == rhs.id
        && lhs.isDimmed == rhs.isDimmed
        && lhs.jobs == rhs.jobs
        && lhs.runs.map({ ($0.id, $0.status, $0.conclusion ?? "") })
            == rhs.runs.map({ ($0.id, $0.status, $0.conclusion ?? "") })
}

}
}

private struct HeadCommit: Codable { let message: String }
private struct PRRef: Codable { let number: Int }

// MARK: - PR label

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 ActionGroup.Equatable conformance line exceeds line_length rule and is hard to maintain

The custom == implementation packs four conditions onto one line, exceeding the configured line_length warning threshold and making it hard to verify correctness. The runs comparison uses map({ $0.id }) == which works but allocates two arrays on every equality check — common during SwiftUI diffing. A minor readability and allocation concern.

Suggested change
static func == (lhs: ActionGroup, rhs: ActionGroup) -> Bool {
lhs.id == rhs.id
&& lhs.isDimmed == rhs.isDimmed
&& lhs.jobs == rhs.jobs
&& lhs.runs.map(\.id) == rhs.runs.map(\.id)
}
AI Fix Prompt
Fix the following Medium (Style) issue in `Sources/RunnerBar/ActionGroup.swift` at line 172:

Problem: The custom == implementation packs four conditions onto one line, exceeding the configured line_length warning threshold and making it hard to verify correctness. The runs comparison uses map({ $0.id }) == which works but allocates two arrays on every equality check — common during SwiftUI diffing. A minor readability and allocation concern.

Suggested fix:
static func == (lhs: ActionGroup, rhs: ActionGroup) -> Bool {
    lhs.id == rhs.id
        && lhs.isDimmed == rhs.isDimmed
        && lhs.jobs == rhs.jobs
        && lhs.runs.map(\.id) == rhs.runs.map(\.id)
}

/// Derives the short identifier for an action group row.
/// Priority: PR number → branch-embedded number → sha[:7].
private func prLabel(from run: RunPayload) -> String {
if let pr = run.pullRequests?.first { return "#\(pr.number)" }
if let branch = run.headBranch,
let range = branch.range(of: #"/(\d+)/"#, options: .regularExpression) {
let digits = branch[range].filter { $0.isNumber }
return "#\(digits)"
static func == (lhs: ActionGroup, rhs: ActionGroup) -> Bool {
lhs.id == rhs.id && lhs.isDimmed == rhs.isDimmed && lhs.jobs == rhs.jobs && lhs.runs.map({ $0.id }) == rhs.runs.map({ $0.id })
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 ActionGroup.== uses runs.map({$0.id}) comparison which ignores run status/conclusion changes

The custom Equatable implementation compares lhs.id, isDimmed, jobs, and runs.map({$0.id}). It does NOT compare run status or conclusion. A run transitioning from in_progress to completed changes its status and conclusion but not its id — so lhs == rhs would return true and SwiftUI's .onChange(of: store.actions) in PopoverMainView would NOT fire, leaving visibleCount unreset and the dot color stale until the next full poll that changes the jobs array. Since ActionGroup.id is headSha and isDimmed captures dimming, consider adding run status/conclusion into the equality check or removing == entirely and relying on the default synthesised comparison (if all stored properties are Equatable).

Suggested change
lhs.id == rhs.id && lhs.isDimmed == rhs.isDimmed && lhs.jobs == rhs.jobs && lhs.runs.map({ $0.id }) == rhs.runs.map({ $0.id })
static func == (lhs: ActionGroup, rhs: ActionGroup) -> Bool {
lhs.id == rhs.id
&& lhs.isDimmed == rhs.isDimmed
&& lhs.jobs == rhs.jobs
&& lhs.runs.map({ ($0.id, $0.status, $0.conclusion) }) == rhs.runs.map({ ($0.id, $0.status, $0.conclusion) })
}
AI Fix Prompt
Fix the following Medium (Logic Error) issue in `Sources/RunnerBar/ActionGroup.swift` at lines 172-174:

Problem: The custom Equatable implementation compares lhs.id, isDimmed, jobs, and runs.map({$0.id}). It does NOT compare run status or conclusion. A run transitioning from in_progress to completed changes its status and conclusion but not its id — so lhs == rhs would return true and SwiftUI's .onChange(of: store.actions) in PopoverMainView would NOT fire, leaving visibleCount unreset and the dot color stale until the next full poll that changes the jobs array. Since ActionGroup.id is headSha and isDimmed captures dimming, consider adding run status/conclusion into the equality check or removing == entirely and relying on the default synthesised comparison (if all stored properties are Equatable).

Suggested fix:
static func == (lhs: ActionGroup, rhs: ActionGroup) -> Bool {
    lhs.id == rhs.id
        && lhs.isDimmed == rhs.isDimmed
        && lhs.jobs == rhs.jobs
        && lhs.runs.map({ ($0.id, $0.status, $0.conclusion) }) == rhs.runs.map({ ($0.id, $0.status, $0.conclusion) })
}

}
return String(run.headSha.prefix(7))
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 WorkflowRunRef missing Equatable — ActionGroup.== accesses run IDs but WorkflowRunRef has no conformance

ActionGroup is now Equatable and its == implementation compares lhs.runs.map({ $0.id }) == rhs.runs.map({ $0.id }). This compiles because [Int] is Equatable, but WorkflowRunRef itself is not declared Equatable. If any future code tries to compare [WorkflowRunRef] directly (e.g. in a onChange(of: group.runs) or a set operation), it will fail to compile. Conformance should be declared now while the intent is clear.

Suggested change
}
struct WorkflowRunRef: Identifiable, Equatable {
let id: Int
let name: String
let status: String
let conclusion: String?
let htmlUrl: String?
}
AI Fix Prompt
Fix the following Medium (Bug) issue in `Sources/RunnerBar/ActionGroup.swift` at lines 174-176:

Problem: `ActionGroup` is now `Equatable` and its `==` implementation compares `lhs.runs.map({ $0.id }) == rhs.runs.map({ $0.id })`. This compiles because `[Int]` is `Equatable`, but `WorkflowRunRef` itself is not declared `Equatable`. If any future code tries to compare `[WorkflowRunRef]` directly (e.g. in a `onChange(of: group.runs)` or a set operation), it will fail to compile. Conformance should be declared now while the intent is clear.

Suggested fix:
struct WorkflowRunRef: Identifiable, Equatable {
    let id: Int
    let name: String
    let status: String
    let conclusion: String?
    let htmlUrl: String?
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 ActionGroup.== compares runs by ID array — order-sensitive, not set equality

The Equatable implementation uses lhs.runs.map({ $0.id }) == rhs.runs.map({ $0.id }), which is order-sensitive array equality. If fetchActionGroups returns runs in a different order between polls (e.g. after server-side sorting changes), two logically identical groups will compare as not-equal, causing onChange to fire and visibleCount to reset unnecessarily.

Suggested change
}
static func == (lhs: ActionGroup, rhs: ActionGroup) -> Bool {
lhs.id == rhs.id
&& lhs.isDimmed == rhs.isDimmed
&& lhs.jobs == rhs.jobs
&& Set(lhs.runs.map { $0.id }) == Set(rhs.runs.map { $0.id })
}
AI Fix Prompt
Fix the following Low (Logic Error) issue in `Sources/RunnerBar/ActionGroup.swift` at lines 174-176:

Problem: The Equatable implementation uses lhs.runs.map({ $0.id }) == rhs.runs.map({ $0.id }), which is order-sensitive array equality. If fetchActionGroups returns runs in a different order between polls (e.g. after server-side sorting changes), two logically identical groups will compare as not-equal, causing onChange to fire and visibleCount to reset unnecessarily.

Suggested fix:
static func == (lhs: ActionGroup, rhs: ActionGroup) -> Bool {
    lhs.id == rhs.id
        && lhs.isDimmed == rhs.isDimmed
        && lhs.jobs == rhs.jobs
        && Set(lhs.runs.map { $0.id }) == Set(rhs.runs.map { $0.id })
}


// MARK: - Fetch + Group
Expand Down Expand Up @@ -299,27 +285,27 @@ func fetchActionGroups(for scope: String, cache: [String: ActionGroup] = [:]) ->
// MARK: - Private helpers

/// Constructs an `ActiveJob` from a decoded `JobPayload`.
func makeActiveJob(from j: JobPayload, iso: ISO8601DateFormatter,
func makeActiveJob(from payload: JobPayload, iso: ISO8601DateFormatter,
isDimmed: Bool = false) -> ActiveJob {
let steps: [JobStep] = (j.steps ?? []).enumerated().map { idx, s in
let steps: [JobStep] = (payload.steps ?? []).enumerated().map { idx, step in
JobStep(
id: idx + 1,
name: s.name,
status: s.status,
conclusion: s.conclusion,
startedAt: s.startedAt.flatMap { iso.date(from: $0) },
completedAt: s.completedAt.flatMap { iso.date(from: $0) }
name: step.name,
status: step.status,
conclusion: step.conclusion,
startedAt: step.startedAt,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 JobStep.startedAt / completedAt silently changed from String? to Date? — makeActiveJob call in enrichStepsIfNeeded passes raw String fields

In this diff, makeActiveJob now passes step.startedAt and step.completedAt directly (without iso.date(from:)) because the diff removes the flatMap conversion for JobStep fields. However, JobPayload.steps is typed as [JobStep]? and JobStep inherits its field types from the struct definition. If JobStep.startedAt and .completedAt were changed from String? to Date? (as suggested by the removal of iso.date(from:) calls), then JobPayload must also decode them as Date? — but JobPayload.steps is decoded from JSON which delivers ISO 8601 strings, not Date objects. This would cause a silent decode failure (steps array decoded as nil/empty) every time enrichStepsIfNeeded runs, causing step data to never populate. Verify that JobStep's Decodable conformance includes a custom date-decoding strategy, or revert the step field types to String? and restore the flatMap.

Suggested change
startedAt: step.startedAt,
// Option A: restore the iso conversion in makeActiveJob
let steps: [JobStep] = (payload.steps ?? []).enumerated().map { idx, step in
JobStep(
id: idx + 1,
name: step.name,
status: step.status,
conclusion: step.conclusion,
startedAt: step.startedAt.flatMap { iso.date(from: $0) },
completedAt: step.completedAt.flatMap { iso.date(from: $0) }
)
}
// Option B: configure JSONDecoder with .iso8601 dateDecodingStrategy before decoding JobPayload
AI Fix Prompt
Fix the following High (Bug) issue in `Sources/RunnerBar/ActionGroup.swift` at lines 296-312:

Problem: In this diff, makeActiveJob now passes step.startedAt and step.completedAt directly (without iso.date(from:)) because the diff removes the flatMap conversion for JobStep fields. However, JobPayload.steps is typed as [JobStep]? and JobStep inherits its field types from the struct definition. If JobStep.startedAt and .completedAt were changed from String? to Date? (as suggested by the removal of iso.date(from:) calls), then JobPayload must also decode them as Date? — but JobPayload.steps is decoded from JSON which delivers ISO 8601 strings, not Date objects. This would cause a silent decode failure (steps array decoded as nil/empty) every time enrichStepsIfNeeded runs, causing step data to never populate. Verify that JobStep's Decodable conformance includes a custom date-decoding strategy, or revert the step field types to String? and restore the flatMap.

Suggested fix:
// Option A: restore the iso conversion in makeActiveJob
let steps: [JobStep] = (payload.steps ?? []).enumerated().map { idx, step in
    JobStep(
        id: idx + 1,
        name: step.name,
        status: step.status,
        conclusion: step.conclusion,
        startedAt: step.startedAt.flatMap { iso.date(from: $0) },
        completedAt: step.completedAt.flatMap { iso.date(from: $0) }
    )
}
// Option B: configure JSONDecoder with .iso8601 dateDecodingStrategy before decoding JobPayload

completedAt: step.completedAt
)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 JobStep.startedAt/completedAt type change silently drops ISO parsing errors

In makeActiveJob(), the step construction previously called iso.date(from: $0) on startedAt/completedAt strings (String? → Date?), matching JobStep's original field types. The diff now passes step.startedAt and step.completedAt directly without parsing. If JobStep.startedAt/completedAt were changed from String? to Date? in ActiveJob.swift (the JobPayload's steps are [JobStep]?), then JobPayload must also return Date? for those fields — but JobPayload.steps is typed as [JobStep]? and JobStep is Decodable. Since JobPayload decodes JSON from the GitHub API (which returns ISO strings), JobStep's Decodable conformance must now parse dates itself. If JobStep's CodingKeys / init(from:) don't implement custom ISO 8601 decoding, the dates will decode as nil silently, breaking elapsed time display and step progress in InlineJobRowView and StepLogView. Verify that JobStep has a custom Decodable implementation that handles the GitHub ISO 8601 format (including fractional seconds), or revert to explicit iso.date(from:) parsing in makeActiveJob().

AI Fix Prompt
Fix the following High (Bug) issue in `Sources/RunnerBar/ActionGroup.swift` at lines 296-299:

Problem: In makeActiveJob(), the step construction previously called iso.date(from: $0) on startedAt/completedAt strings (String? → Date?), matching JobStep's original field types. The diff now passes step.startedAt and step.completedAt directly without parsing. If JobStep.startedAt/completedAt were changed from String? to Date? in ActiveJob.swift (the JobPayload's steps are [JobStep]?), then JobPayload must also return Date? for those fields — but JobPayload.steps is typed as [JobStep]? and JobStep is Decodable. Since JobPayload decodes JSON from the GitHub API (which returns ISO strings), JobStep's Decodable conformance must now parse dates itself. If JobStep's CodingKeys / init(from:) don't implement custom ISO 8601 decoding, the dates will decode as nil silently, breaking elapsed time display and step progress in InlineJobRowView and StepLogView. Verify that JobStep has a custom Decodable implementation that handles the GitHub ISO 8601 format (including fractional seconds), or revert to explicit iso.date(from:) parsing in makeActiveJob().

return ActiveJob(
id: j.id,
name: j.name,
status: j.status,
conclusion: j.conclusion,
startedAt: j.startedAt.flatMap { iso.date(from: $0) },
createdAt: j.createdAt.flatMap { iso.date(from: $0) },
completedAt: j.completedAt.flatMap { iso.date(from: $0) },
htmlUrl: j.htmlUrl,
id: payload.id,
name: payload.name,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 makeActiveJob step dates passed as String? instead of Date? after refactor

In the refactored makeActiveJob, the step construction now passes step.startedAt and step.completedAt directly — but JobStep.startedAt and JobStep.completedAt are Date? while JobPayload's step payloads (accessed as payload.steps) are [JobStep]. If JobStep stores pre-parsed Dates this is fine, but the original code used s.startedAt.flatMap { iso.date(from: $0) }, implying the raw value was a String that required ISO 8601 parsing. Verify that JobStep in the diff stores Date? (not String?) — if it still stores String?, the dates silently fail to parse and inline job elapsed timers will show '00:00' for all steps.

Suggested change
name: payload.name,
// Confirm JobStep.startedAt / .completedAt are Date? not String?.
// If they are String?, restore the ISO parsing:
startedAt: step.startedAt.flatMap { iso.date(from: $0) },
completedAt: step.completedAt.flatMap { iso.date(from: $0) }
AI Fix Prompt
Fix the following Medium (Bug) issue in `Sources/RunnerBar/ActionGroup.swift` at lines 297-302:

Problem: In the refactored makeActiveJob, the step construction now passes `step.startedAt` and `step.completedAt` directly — but `JobStep.startedAt` and `JobStep.completedAt` are `Date?` while `JobPayload`'s step payloads (accessed as `payload.steps`) are `[JobStep]`. If JobStep stores pre-parsed Dates this is fine, but the original code used `s.startedAt.flatMap { iso.date(from: $0) }`, implying the raw value was a String that required ISO 8601 parsing. Verify that JobStep in the diff stores Date? (not String?) — if it still stores String?, the dates silently fail to parse and inline job elapsed timers will show '00:00' for all steps.

Suggested fix:
// Confirm JobStep.startedAt / .completedAt are Date? not String?.
// If they are String?, restore the ISO parsing:
startedAt: step.startedAt.flatMap { iso.date(from: $0) },
completedAt: step.completedAt.flatMap { iso.date(from: $0) }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 JobStep date strings passed raw — ISO parsing removed only for steps

makeActiveJob previously called iso.date(from:) on step.startedAt and step.completedAt before passing them to JobStep. This PR changes the call to pass step.startedAt and step.completedAt directly. However, JobStep stores these as String? (not Date?), so they are already raw strings — the iso.date(from:) calls were wrong before (JobStep doesn't have Date fields). Verify that JobStep.startedAt / JobStep.completedAt are indeed String? and that no elapsed-time computation relies on them being Date. If JobStep properties were Date?, this would silently store nil for all step timestamps.

Suggested change
name: payload.name,
// Verify JobStep definition in ActiveJob.swift — if startedAt/completedAt are String?, the change is correct.
// If they are Date?, revert to:
startedAt: step.startedAt.flatMap { iso.date(from: $0) },
completedAt: step.completedAt.flatMap { iso.date(from: $0) }
AI Fix Prompt
Fix the following Medium (Bug) issue in `Sources/RunnerBar/ActionGroup.swift` at lines 290-302:

Problem: makeActiveJob previously called iso.date(from:) on step.startedAt and step.completedAt before passing them to JobStep. This PR changes the call to pass step.startedAt and step.completedAt directly. However, JobStep stores these as String? (not Date?), so they are already raw strings — the iso.date(from:) calls were wrong before (JobStep doesn't have Date fields). Verify that JobStep.startedAt / JobStep.completedAt are indeed String? and that no elapsed-time computation relies on them being Date. If JobStep properties were Date?, this would silently store nil for all step timestamps.

Suggested fix:
// Verify JobStep definition in ActiveJob.swift — if startedAt/completedAt are String?, the change is correct.
// If they are Date?, revert to:
startedAt: step.startedAt.flatMap { iso.date(from: $0) },
completedAt: step.completedAt.flatMap { iso.date(from: $0) }

status: payload.status,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 JobStep.startedAt / completedAt type changed from String? to Date? — makeActiveJob skips ISO parsing

In the diff, makeActiveJob now assigns step.startedAt and step.completedAt directly (without flatMap { iso.date(from: $0) }). This is only correct if JobStep.startedAt/completedAt are already typed as Date?. If they remain String? (as the diff for ActiveJob.swift shows JobPayload still has String? fields), this will be a type error at compile time — or worse, if JobStep was silently changed to Date? without updating Codable conformance, step timestamps will decode as nil from JSON. Verify that JobStep's stored type matches what makeActiveJob expects.

Suggested change
status: payload.status,
// If JobStep.startedAt is String?, restore the parse:
startedAt: step.startedAt.flatMap { iso.date(from: $0) },
completedAt: step.completedAt.flatMap { iso.date(from: $0) }
AI Fix Prompt
Fix the following Medium (Bug) issue in `Sources/RunnerBar/ActionGroup.swift` at lines 298-304:

Problem: In the diff, makeActiveJob now assigns step.startedAt and step.completedAt directly (without flatMap { iso.date(from: $0) }). This is only correct if JobStep.startedAt/completedAt are already typed as Date?. If they remain String? (as the diff for ActiveJob.swift shows JobPayload still has String? fields), this will be a type error at compile time — or worse, if JobStep was silently changed to Date? without updating Codable conformance, step timestamps will decode as nil from JSON. Verify that JobStep's stored type matches what makeActiveJob expects.

Suggested fix:
// If JobStep.startedAt is String?, restore the parse:
startedAt: step.startedAt.flatMap { iso.date(from: $0) },
completedAt: step.completedAt.flatMap { iso.date(from: $0) }

conclusion: payload.conclusion,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Step timestamps silently dropped — ISO parse removed from makeActiveJob

In the refactored makeActiveJob, step.startedAt and step.completedAt are now assigned directly to JobStep fields instead of being parsed through ISO8601DateFormatter. If JobStep.startedAt/completedAt are String? fields (as they were before), the compiler may accept this but the Date fields on JobStep will be wrong — they'll receive the raw ISO string or fail to assign. If the fields were changed to String? in JobStep in this PR, then elapsed and progress calculations that depend on Date comparisons will silently receive nil. Either way the timestamps that drive PieProgressView progress and the step elapsed timer are now potentially broken.

Suggested change
conclusion: payload.conclusion,
startedAt: step.startedAt.flatMap { iso.date(from: $0) },
completedAt: step.completedAt.flatMap { iso.date(from: $0) }
AI Fix Prompt
Fix the following High (Bug) issue in `Sources/RunnerBar/ActionGroup.swift` at lines 299-305:

Problem: In the refactored `makeActiveJob`, `step.startedAt` and `step.completedAt` are now assigned directly to `JobStep` fields instead of being parsed through `ISO8601DateFormatter`. If `JobStep.startedAt`/`completedAt` are `String?` fields (as they were before), the compiler may accept this but the Date fields on JobStep will be wrong — they'll receive the raw ISO string or fail to assign. If the fields were changed to `String?` in `JobStep` in this PR, then `elapsed` and progress calculations that depend on `Date` comparisons will silently receive `nil`. Either way the timestamps that drive `PieProgressView` progress and the step elapsed timer are now potentially broken.

Suggested fix:
startedAt: step.startedAt.flatMap { iso.date(from: $0) },
completedAt: step.completedAt.flatMap { iso.date(from: $0) }

startedAt: payload.startedAt.flatMap { iso.date(from: $0) },
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 JobStep.startedAt / completedAt type change may silently drop timestamps

In makeActiveJob, the diff changes JobStep construction from startedAt: step.startedAt.flatMap { iso.date(from: $0) } to startedAt: step.startedAt — implying JobStep.startedAt is now String? rather than Date?. Verify that JobStep's stored type was intentionally changed from Date? to String?. If JobStep.startedAt is still Date?, this line now passes a String? and will not compile. If it was changed to String?, callers that previously compared timestamps (e.g. elapsed computation in ActionGroup) need to parse those strings themselves — confirm no silent data loss path exists.

Suggested change
startedAt: payload.startedAt.flatMap { iso.date(from: $0) },
// Confirm JobStep stores String? (not Date?) for startedAt/completedAt.
// If Date? is intended, restore:
startedAt: step.startedAt.flatMap { iso.date(from: $0) },
completedAt: step.completedAt.flatMap { iso.date(from: $0) }
AI Fix Prompt
Fix the following Medium (Bug) issue in `Sources/RunnerBar/ActionGroup.swift` at lines 313-321:

Problem: In `makeActiveJob`, the diff changes `JobStep` construction from `startedAt: step.startedAt.flatMap { iso.date(from: $0) }` to `startedAt: step.startedAt` — implying `JobStep.startedAt` is now `String?` rather than `Date?`. Verify that `JobStep`'s stored type was intentionally changed from `Date?` to `String?`. If `JobStep.startedAt` is still `Date?`, this line now passes a `String?` and will not compile. If it was changed to `String?`, callers that previously compared timestamps (e.g. `elapsed` computation in `ActionGroup`) need to parse those strings themselves — confirm no silent data loss path exists.

Suggested fix:
// Confirm JobStep stores String? (not Date?) for startedAt/completedAt.
// If Date? is intended, restore:
startedAt: step.startedAt.flatMap { iso.date(from: $0) },
completedAt: step.completedAt.flatMap { iso.date(from: $0) }

createdAt: payload.createdAt.flatMap { iso.date(from: $0) },
completedAt: payload.completedAt.flatMap { iso.date(from: $0) },
htmlUrl: payload.htmlUrl,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 JobStep.startedAt/completedAt type changed silently — verify callers handle Date? not String?

In makeActiveJob(from:iso:isDimmed:), the step construction now passes step.startedAt and step.completedAt directly (previously s.startedAt.flatMap { iso.date(from: $0) }). If JobStep.startedAt and JobStep.completedAt were already Date? (not String?) before this PR, this is correct. But if JobPayload.steps contains [JobStep] where JobStep stores startedAt: String?, the ISO parsing is now silently skipped, and all step timestamps will be nil. Verify that JobStep.startedAt and .completedAt are Date? in the current JobStep struct definition.

Suggested change
htmlUrl: payload.htmlUrl,
// Verify Sources/RunnerBar/ActiveJob.swift — JobStep must declare:
// let startedAt: Date?
// let completedAt: Date?
// If they are String?, restore the flatMap:
// startedAt: step.startedAt.flatMap { iso.date(from: $0) },
// completedAt: step.completedAt.flatMap { iso.date(from: $0) }
AI Fix Prompt
Fix the following Nit (Bug) issue in `Sources/RunnerBar/ActionGroup.swift` at lines 308-315:

Problem: In `makeActiveJob(from:iso:isDimmed:)`, the step construction now passes `step.startedAt` and `step.completedAt` directly (previously `s.startedAt.flatMap { iso.date(from: $0) }`). If `JobStep.startedAt` and `JobStep.completedAt` were already `Date?` (not `String?`) before this PR, this is correct. But if `JobPayload.steps` contains `[JobStep]` where `JobStep` stores `startedAt: String?`, the ISO parsing is now silently skipped, and all step timestamps will be `nil`. Verify that `JobStep.startedAt` and `.completedAt` are `Date?` in the current `JobStep` struct definition.

Suggested fix:
// Verify Sources/RunnerBar/ActiveJob.swift — JobStep must declare:
// let startedAt: Date?
// let completedAt: Date?
// If they are String?, restore the flatMap:
// startedAt: step.startedAt.flatMap { iso.date(from: $0) },
// completedAt: step.completedAt.flatMap { iso.date(from: $0) }

isDimmed: isDimmed,
steps: steps
)
Expand Down Expand Up @@ -381,4 +367,4 @@ private func statusPriority(_ status: GroupStatus) -> Int {
case .completed: return 2
}
}
// swiftlint:enable opening_brace identifier_name missing_docs orphaned_doc_comment
// swiftlint:enable opening_brace identifier_name orphaned_doc_comment missing_docs
Loading
Loading