fix(tui): show Conns: 1 for single-connection downloads by mvanhorn · Pull Request #315 · SurgeDM/Surge

mvanhorn · 2026-04-03T06:47:56Z

Summary

When a download doesn't support range requests (SingleDownloader), the TUI shows "Conns: 0" even though there's an active connection transferring data.

Why this matters

Showing 0 connections while a download is actively running with nonzero speed is confusing. The SingleDownloader uses one connection but the ActiveWorkers atomic counter doesn't track it, so ProgressState.GetProgress() returns 0 for the connections field.

Changes

internal/tui/view.go (line 912): When a download is active (speed > 0) and reports 0 connections, display 1 instead. An active download with speed must have at least one connection.

Testing

Verified the change only affects the display string in the TUI stats section
Active multi-connection downloads still show the correct count from d.Connections
The fallback to 1 only triggers when d.Connections == 0 AND the download is active

Fixes #287

This contribution was developed with AI assistance (Claude Code).

Greptile Summary

This PR fixes a TUI cosmetic issue where single-connection downloads (served by SingleDownloader, which never increments ActiveWorkers) displayed "Conns: 0" during an active transfer. The fix adds a display-layer fallback in renderFocusedDetails: when isActive is true and d.Connections == 0, the rendered value is coerced to 1.

Confidence Score: 5/5

Safe to merge — change is display-only, scoped to a single render path, and has no effect on download logic or data integrity.
All findings are P2 (comment quality, missing test, architectural suggestion). No logic bugs or data correctness issues were identified. The guard condition correctly mirrors the existing isActive check and cannot over-report connections for multi-connection downloads.
No files require special attention.

Important Files Changed

Filename	Overview
internal/tui/view.go	Display-layer fix to show "Conns: 1" instead of "Conns: 0" for active single-connection downloads; compensates for SingleDownloader never incrementing ActiveWorkers, but leaves the root value in ProgressState.GetProgress() still reporting 0.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[renderFocusedDetails] --> B{isActive?\nnot done, not paused,\nSpeed > 0}
    B -- No --> C[Skip Conns row]
    B -- Yes --> D{d.Connections == 0?}
    D -- No --> E[conns = d.Connections\nfrom ActiveWorkers]
    D -- Yes --> F["conns = 1\n(SingleDownloader fallback)"]
    E --> G[Render 'Conns: N']
    F --> G

    subgraph Root Cause
        H[SingleDownloader.Download] --> I[ActiveWorkers never\nincremented/decremented]
        I --> J[GetProgress returns\nconnections = 0]
        J --> D
    end

Prompt To Fix All With AI

This is a comment left during a code review.
Path: internal/tui/view.go
Line: 914

Comment:
**Comment explains "what", not "why"**

The comment restates what the guard does rather than explaining why this assumption is safe. A reader still needs to know *why* `conns` can be 0 for an active download. Consider something like: `// ActiveWorkers is only incremented by the concurrent downloader; SingleDownloader doesn't update it, so speed > 0 with 0 connections means exactly one single-threaded connection`.

```suggestion
			conns = 1 // ActiveWorkers is only updated by concurrent workers; SingleDownloader never increments it, so speed>0 with 0 reported conns means one active single-connection transfer
```

**Rule Used:** What: Comments must explain *why* code exists, not... ([source](https://app.greptile.com/review/custom-context?memory=4e45ef13-32c2-4753-8060-a44ef767144d))

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: internal/tui/view.go
Line: 911-918

Comment:
**Consider fixing at the source rather than the display layer**

This compensates for the missing `ActiveWorkers` increment in `SingleDownloader.Download()`. Patching the display layer means any other consumer of `ProgressState.GetProgress()` (e.g. remote service, polling events) still sees 0 connections for single-connection downloads. Incrementing `ActiveWorkers` at the start of `Download()` and decrementing it on return would fix the value at the source and remove the need for this view-layer special case.

**Rule Used:** # Code Review Rule: Suggest Architectural Improvem... ([source](https://app.greptile.com/review/custom-context?memory=4d82344e-33c4-4e14-bbc7-81d79af11b7e))

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: internal/tui/view.go
Line: 911-918

Comment:
**No test coverage for this render path**

`view_test.go` exists but has no test for `renderFocusedDetails` with `Connections == 0 && Speed > 0`. Adding a case there would prevent a future regression where someone removes the guard and the UI silently reverts to showing "Conns: 0".

**Rule Used:** What: All code changes must include tests for edge... ([source](https://app.greptile.com/review/custom-context?memory=2b22782d-3452-4d55-b059-e631b2540ce8))

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(tui): show Conns: 1 for single-conne..." | Re-trigger Greptile}

Greptile also left 3 inline comments on this PR.

Context used (3)

Rule used - What: Comments must explain why code exists, not... (source)
Rule used - # Code Review Rule: Suggest Architectural Improvem... (source)
Rule used - What: All code changes must include tests for edge... (source)

When a download is active with speed > 0 but reports 0 connections (SingleDownloader, range requests not supported), display 1 instead of 0. An active download with speed must have at least one connection. Fixes SurgeDM#287

greptile-apps · 2026-04-03T06:50:42Z

 	if isActive {
-		connStr := "0"
-		if d.Connections > 0 {
-			connStr = fmt.Sprintf("%d", d.Connections)
+		conns := d.Connections
+		if conns == 0 {
+			conns = 1 // Single-connection download (range requests not supported)
 		}
+		connStr := fmt.Sprintf("%d", conns)
 		leftColItems = append(leftColItems, lipgloss.JoinHorizontal(lipgloss.Left, StatsLabelStyle.Width(7).Render("Conns:"), StatsValueStyle.Render(connStr)))
 	}


Consider fixing at the source rather than the display layer

This compensates for the missing ActiveWorkers increment in SingleDownloader.Download(). Patching the display layer means any other consumer of ProgressState.GetProgress() (e.g. remote service, polling events) still sees 0 connections for single-connection downloads. Incrementing ActiveWorkers at the start of Download() and decrementing it on return would fix the value at the source and remove the need for this view-layer special case.

Rule Used: # Code Review Rule: Suggest Architectural Improvem... (source)

Prompt To Fix With AI

This is a comment left during a code review. Path: internal/tui/view.go Line: 911-918 Comment: **Consider fixing at the source rather than the display layer** This compensates for the missing `ActiveWorkers` increment in `SingleDownloader.Download()`. Patching the display layer means any other consumer of `ProgressState.GetProgress()` (e.g. remote service, polling events) still sees 0 connections for single-connection downloads. Incrementing `ActiveWorkers` at the start of `Download()` and decrementing it on return would fix the value at the source and remove the need for this view-layer special case. **Rule Used:** # Code Review Rule: Suggest Architectural Improvem... ([source](https://app.greptile.com/review/custom-context?memory=4d82344e-33c4-4e14-bbc7-81d79af11b7e)) How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-04-03T06:50:43Z

 	if isActive {
-		connStr := "0"
-		if d.Connections > 0 {
-			connStr = fmt.Sprintf("%d", d.Connections)
+		conns := d.Connections
+		if conns == 0 {
+			conns = 1 // Single-connection download (range requests not supported)
 		}
+		connStr := fmt.Sprintf("%d", conns)
 		leftColItems = append(leftColItems, lipgloss.JoinHorizontal(lipgloss.Left, StatsLabelStyle.Width(7).Render("Conns:"), StatsValueStyle.Render(connStr)))
 	}


No test coverage for this render path

view_test.go exists but has no test for renderFocusedDetails with Connections == 0 && Speed > 0. Adding a case there would prevent a future regression where someone removes the guard and the UI silently reverts to showing "Conns: 0".

Rule Used: What: All code changes must include tests for edge... (source)

Prompt To Fix With AI

This is a comment left during a code review. Path: internal/tui/view.go Line: 911-918 Comment: **No test coverage for this render path** `view_test.go` exists but has no test for `renderFocusedDetails` with `Connections == 0 && Speed > 0`. Adding a case there would prevent a future regression where someone removes the guard and the UI silently reverts to showing "Conns: 0". **Rule Used:** What: All code changes must include tests for edge... ([source](https://app.greptile.com/review/custom-context?memory=2b22782d-3452-4d55-b059-e631b2540ce8)) How can I resolve this? If you propose a fix, please make it concise.

SuperCoolPencil

Hi @mvanhorn, Thanks for the PR!

I would love if as suggested by AI reviewer, we could fix the problem at source rather than TUI layer so that the problem is mitigated in future user layers as well.

Also, some tests that prevent against future regressions would be really appreciated.

mvanhorn · 2026-04-04T21:58:43Z

Appreciate the merge, @SuperCoolPencil.

SuperCoolPencil · 2026-04-04T22:17:30Z

Thanks for contributing 🚀

fix(tui): show Conns: 1 for single-connection downloads

9fd4805

When a download is active with speed > 0 but reports 0 connections (SingleDownloader, range requests not supported), display 1 instead of 0. An active download with speed must have at least one connection. Fixes SurgeDM#287

greptile-apps Bot reviewed Apr 3, 2026

View reviewed changes

SuperCoolPencil requested changes Apr 4, 2026

View reviewed changes

SuperCoolPencil merged commit 93fec16 into SurgeDM:main Apr 4, 2026
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(tui): show Conns: 1 for single-connection downloads#315

fix(tui): show Conns: 1 for single-connection downloads#315
SuperCoolPencil merged 1 commit intoSurgeDM:mainfrom
mvanhorn:fix/tui-conns-zero

mvanhorn commented Apr 3, 2026 •

edited by greptile-apps Bot

Loading

Uh oh!

Uh oh!

greptile-apps Bot Apr 3, 2026

Uh oh!

greptile-apps Bot Apr 3, 2026

Uh oh!

SuperCoolPencil left a comment

Uh oh!

Uh oh!

mvanhorn commented Apr 4, 2026

Uh oh!

SuperCoolPencil commented Apr 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

mvanhorn commented Apr 3, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why this matters

Changes

Testing

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Context used (3)

Uh oh!

Uh oh!

greptile-apps Bot Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

SuperCoolPencil left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mvanhorn commented Apr 4, 2026

Uh oh!

SuperCoolPencil commented Apr 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mvanhorn commented Apr 3, 2026 •

edited by greptile-apps Bot

Loading