Skip to content

fix(desktop): clamp Goal.progress to its documented 0-100 range#8003

Draft
eulicesl wants to merge 3 commits into
mainfrom
fix/macos-goal-progress-clamp
Draft

fix(desktop): clamp Goal.progress to its documented 0-100 range#8003
eulicesl wants to merge 3 commits into
mainfrom
fix/macos-goal-progress-clamp

Conversation

@eulicesl

Copy link
Copy Markdown
Collaborator

What

Goal.progress (in desktop/macos/Desktop/Sources/APIClient.swift) is documented as returning a percentage in the range 0-100, but it only guarded against divide-by-zero — it did not clamp the result:

// before
var progress: Double {
  guard targetValue != minValue else { return 0 }
  return ((currentValue - minValue) / (targetValue - minValue)) * 100.0
}

When currentValue is below minValue (e.g. a goal whose baseline is above zero, or a value that regresses below its starting point), this returns a negative percentage. Consumers only clamp the upper bound:

  • MainWindow/Components/GoalsWidget.swift:164min(goal.progress / 100.0, 1.0)
  • MainWindow/Components/GoalsWidget.swift:775.trim(from: 0, to: min(goal.progress / 100, 1.0))
  • ProactiveAssistants/.../TaskPrioritizationService.swift:185"(\(Int(goal.progress))% complete)"

So a negative value passes straight through: Path.trim(from: 0, to: <negative>) renders an empty/inverted arc, and the prompt text reads e.g. "(-40% complete)".

Fix

Clamp at the source so the getter honors its own 0-100 contract:

// after
var progress: Double {
  guard targetValue != minValue else { return 0 }
  let pct = ((currentValue - minValue) / (targetValue - minValue)) * 100.0
  return min(max(pct, 0), 100)
}

For all in-range values the output is identical — only out-of-range inputs change, toward the documented contract. Existing caller-side upper-bound clamps are left in place (harmless/defensive).

Tests

Adds Desktop/Tests/GoalProgressTests.swift (decoder-based, matching the repo's existing test style):

  • mid-range → 50
  • currentValue < minValue0 (the regression)
  • currentValue > targetValue100
  • targetValue == minValue → 0 (divide-by-zero guard)

Verification

  • Pure-value clamp; no schema/IO/migration. Revertible in one commit.
  • ⚠️ Not compiled/run locally — authored in a Linux container with no Apple SDK. Needs a macOS build (xcrun swift test --package-path Desktop --filter GoalProgressTests) / CI to confirm.

Draft until a macOS build/test pass confirms it green.

🤖 Generated with Claude Code


Generated by Claude Code

claude added 3 commits June 18, 2026 00:07
current_value below min_value produced a negative percentage that leaked
into the goal progress ring (Path.trim) and the "% complete" prompt text.
Callers only clamped the upper bound, so clamp at the source instead.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K6oBTTKdSzfrvKNg1yXnuB
Adds GoalProgressTests: mid-range, below-min clamps to 0 (the regression),
overachievement clamps to 100, and the target==min divide-by-zero guard.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K6oBTTKdSzfrvKNg1yXnuB
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K6oBTTKdSzfrvKNg1yXnuB
@eulicesl eulicesl force-pushed the fix/macos-goal-progress-clamp branch from 11644e8 to ec9eb41 Compare June 18, 2026 00:09
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