Skip to content

fix(push): stop progress bar colliding with staging log lines (D3) - #391

Merged
shujaatTracebloc merged 1 commit into
developfrom
fix/progress-bar-render-timing
Jul 23, 2026
Merged

fix(push): stop progress bar colliding with staging log lines (D3)#391
shujaatTracebloc merged 1 commit into
developfrom
fix/progress-bar-render-timing

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes the progress-bar collision in dataset push (backlog item D3). During a guided ingest the staging progress bar mashed into the setup log line:

Copying tsc_train 0% |…| ( 0 B/53 kB) [0s:0s]Opened a secure channel to your secure environment's storage.

Root cause

The bar is built in data.go with progressbar.OptionSetRenderBlankState(true), so schollz paints a 0% bar at construction time — before push.Stage prints its setup lines ("Opened a secure channel…", "Preparing the copy…") and before the up-to-StagePodReadyTimeout (5 min) pod-ready wait. Two problems fall out of that:

  1. The 0% bar sits frozen through the multi-minute pod-ready wait, reading as "stuck".
  2. The setup lines are plain \n writes to the same terminal; the bar redraws with \r. They land on the same physical line → garbled output.

StreamLayout prints nothing itself during the transfer — the bar's only output is its own \r renders — so every collision comes from that premature construction-time render.

Fix

Flip to OptionSetRenderBlankState(false). The bar's first paint now happens on the first Add() inside StreamLayout, i.e. after every setup line has already printed on its own clean line. Result:

Opened a secure channel to your secure environment's storage.
Preparing the copy (up to 5m0s)…
Copying 1 files (53 kB) into "tsc_train"…
[bar animates on its own line, then clears on finish]
Copied 1 files into "tsc_train".

This is also better UX independent of the collision: no misleading frozen 0% bar during the pod-ready wait.

A thorough comment on the option (and a cross-reference in stage.go) documents the ordering contract so nobody flips it back or adds a mid-transfer status print.

Scope / risk

  • No copy strings change — this is purely render timing. The copy catalog golden is unaffected (verified: full go test ./... green, including the catalog drift test).
  • Touches only internal/push/{progress.go,stage.go}; TTY-only behavior (non-TTY already uses the no-op sink).
  • Not unit-testable without a PTY dependency (the bar only renders to a real terminal); the existing progress_test.go covers the non-TTY branch, and the change is a single documented option flip with well-defined schollz semantics.

Test plan

  • go build ./...
  • go vet ./internal/push/
  • go test ./... (all packages green, incl. catalog drift)
  • Manual: run a real dataset push on a TTY and confirm the setup lines and bar no longer collide

🤖 Generated with Claude Code


Note

Low Risk
TTY-only render timing change in push progress UI; no copy strings or transfer logic changes.

Overview
Fixes garbled dataset push TTY output where the staging progress bar was drawn at 0% before setup log lines, so \r bar redraws collided with \n status text on the same line.

NewProgress now sets progressbar.OptionSetRenderBlankState(false) so the bar’s first paint happens on the first Add() inside StreamLayout, after messages like “Opened a secure channel…” and the pod-ready wait. That removes the frozen 0% bar during the wait and keeps setup lines on clean rows. Comments in progress.go and stage.go document the ordering contract (no new status prints after the bar starts).

Reviewed by Cursor Bugbot for commit a3add66. Bugbot is set up for automated code reviews on this repo. Configure here.

The staging progress bar was built with RenderBlankState(true), so
schollz painted a 0% bar at construction time — before Stage prints its
setup lines ("Opened a secure channel…", "Preparing the copy…") and
before the up-to-5-minute pod-ready wait. Two problems:
1. The 0% bar sat frozen through the pod-ready wait, reading as
"stuck".
2. Those setup lines (plain \n writes) collided with the bar's \r
redraw on the same terminal line, producing garbled output like
"Copying x 0% |…| (0 B/53 kB) [0s:0s]Opened a secure channel…".
Flip to RenderBlankState(false): the bar first paints on the first
Add() inside StreamLayout, after every setup line has printed on its own
clean line. Better UX too — no misleading frozen bar during the
pod-ready wait. No copy strings change; catalog golden unaffected.
Backlog item: D3.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodkaLukasWodka self-assigned this Jul 22, 2026
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit a3add66. Configure here.

@shujaatTracebloc
shujaatTracebloc merged commit 27fd92a into developJul 23, 2026
26 checks passed
@shujaatTracebloc
shujaatTracebloc deleted the fix/progress-bar-render-timing branch July 23, 2026 14:04
Sign up for freeto 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

@LukasWodka@shujaatTracebloc