refactor: units

[SuperCoolPencil]

• refactor: update downloader to calculate connection count based on remaining file size during resume
• refactor: extract effective size calculation to a helper method and add unit tests
• refactor: propagate worker errors to downloader to trigger immediate cancellation instead of re-queueing failed tasks
• refactor: move size constants to utils package and replace size_converter with standardized units
• refactor: replace types.MB with utils.MiB in concurrent downloader tests

Greptile Summary

This PR completes a units refactor across the whole codebase: the custom ConvertBytesToHumanReadable (base-1000) and the scattered KB/MB/GB constants in types and config packages are replaced with a single canonical utils.KiB/MiB/GiB set and FormatBytes/FormatSpeed helpers that use go-humanize's IEC standard. All speed values are now stored as raw bytes/s throughout, and display formatting is pushed entirely to the presentation layer.

• internal/utils/units.go is added as the single source of truth for size constants and byte/speed formatting; size_converter.go is deleted.
• internal/engine/types/config.go and internal/config/settings.go drop their local KB/MB/GB constants, importing utils instead.
• local_service.go, pool.go, and model.go are corrected to store and pass speeds in raw bytes/s, removing a unit-mismatch bug where completed-download speeds were displayed as ~1 million× too slow.

Confidence Score: 5/5

Safe to merge — all unit-mismatch bugs from the previous review round are corrected, the migration is consistent across all 47 changed files, and no regressions were found in the core download logic.

The refactor is mechanical and thorough: every former reference to types.KB/types.MB/config.MB has been replaced, speed storage is uniformly bytes/s, and the display layer now uses the single FormatBytes/FormatSpeed helpers. Previously broken files (switch_429_test.go conflict markers, completedSpeedMBps unit mismatch, model.go double-multiply) are all addressed.

No files require special attention. The two minor style nits are in cmd/cli_test.go and internal/utils/units.go.

Important Files Changed

Filename	Overview
internal/utils/units.go	New file consolidating IEC size constants (KiB/MiB/GiB/TiB) and FormatBytes/FormatSpeed helpers using go-humanize; replaces the old size_converter.go and duplicated constants
internal/utils/size_converter.go	Deleted — replaced by units.go with IEC-standard formatting via go-humanize
internal/engine/types/config.go	Removed KB/MB/GB constants, replaced with utils.KiB/MiB/GiB throughout; all constants now self-consistent with the IEC standard
internal/core/local_service.go	Renames completedSpeedMBps to completedSpeedBps and removes the MiB division so speed is stored as raw bytes/s
internal/download/pool.go	Removes the MB division when writing status.Speed, storing raw bytes/s; comment updated to match
internal/tui/model.go	Removes the erroneous * config.MB multiply when initializing dm.Speed from s.Speed, since Speed is now raw bytes/s
internal/engine/concurrent/switch_429_test.go	All types.KB/MB references replaced with utils.KiB/MiB; previously flagged conflict markers no longer present
cmd/cli_test.go	Test call updated to utils.FormatBytes and expected values updated to IEC labels; error message string still references old function name ConvertBytesToHumanReadable
internal/engine/concurrent/downloader.go	Adds getEffectiveSizeForWorkers helper to base connection count on remaining bytes during resume; replaces types.MB/KB with utils.MiB/KiB throughout

Comments Outside Diff (3)

1. internal/tui/update_events.go, line 170-189 (link)

   Same mismatch as in events.go: speed is computed in bytes/s (float64(d.Total) / elapsed.Seconds()), so dividing by utils.MiB produces MiB/s, but the format string reads "MB/s". This TUI log entry for download completion was missed during the unit label sweep.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: internal/tui/update_events.go
   Line: 170-189
   
   Comment:
   Same mismatch as in `events.go`: `speed` is computed in bytes/s (`float64(d.Total) / elapsed.Seconds()`), so dividing by `utils.MiB` produces MiB/s, but the format string reads "MB/s". This TUI log entry for download completion was missed during the unit label sweep.
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

2. internal/download/pool.go, line 741 (link)

   This comment was not updated to reflect the unit change. status.Speed is now stored as raw bytes/s, not MB/s.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: internal/download/pool.go
   Line: 741
   
   Comment:
   This comment was not updated to reflect the unit change. `status.Speed` is now stored as raw bytes/s, not MB/s.
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

3. internal/core/local_service.go, line 22-33 (link)

   completedSpeedMBps returns MiB/s but callers now expect bytes/s

   After this refactor, every other path that writes status.Speed stores raw bytes/s (e.g., pool.go line 248 and local_service.go line 453). However completedSpeedMBps still divides entry.AvgSpeed (bytes/s) by utils.MiB, returning MiB/s. Both List() (line 499) and GetStatus (line 751) assign its return value directly to status.Speed. Any downstream formatter that calls utils.FormatSpeed(d.Speed) or the TUI model path (dm.Speed = s.Speed) will treat the MiB/s value as bytes/s, displaying completed-download speeds as ~1 million times too slow (e.g., 10 MiB/s completed download shows as "10 B/s").

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: internal/core/local_service.go
   Line: 22-33
   
   Comment:
   **`completedSpeedMBps` returns MiB/s but callers now expect bytes/s**
   
   After this refactor, every other path that writes `status.Speed` stores raw bytes/s (e.g., `pool.go` line 248 and `local_service.go` line 453). However `completedSpeedMBps` still divides `entry.AvgSpeed` (bytes/s) by `utils.MiB`, returning MiB/s. Both `List()` (line 499) and `GetStatus` (line 751) assign its return value directly to `status.Speed`. Any downstream formatter that calls `utils.FormatSpeed(d.Speed)` or the TUI model path (`dm.Speed = s.Speed`) will treat the MiB/s value as bytes/s, displaying completed-download speeds as ~1 million times too slow (e.g., 10 MiB/s completed download shows as "10 B/s").
   
   How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (6): Last reviewed commit: "fix: check write return values in 429 te..." | Re-trigger Greptile}

[github-actions]

Binary Size Analysis

✅ Size Decreased or Stable

Version	Human Readable	Raw Bytes
Main	17.00 MB	17821988
PR	17.00 MB	17821988
Difference	0.00 B	0

[github-actions]

❌ Test Failures on ubuntu-latest

• github.com/SurgeDM/Surge/internal/engine/concurrent: TestConcurrentDownloader_503WithRetryAfterTreatedAsThrottle