Skip to content

Collapse image pull progress into loading indicator#82

Merged
silv-io merged 3 commits intomainfrom
silv-io/flc-400
Mar 9, 2026
Merged

Collapse image pull progress into loading indicator#82
silv-io merged 3 commits intomainfrom
silv-io/flc-400

Conversation

@silv-io
Copy link
Member

@silv-io silv-io commented Mar 6, 2026

Motivation

When pulling a Docker image, the CLI printed every individual layer progress line from Docker (downloading, extracting, etc. for each layer). This flooded the terminal with dozens of rapidly updating lines, making the output noisy and hard to follow.

Changes

  • Added a new PullProgress Bubble Tea component backed by bubbles/progress that aggregates pull progress across all Docker layers into a single animated progress bar. Shows the dominant phase (Downloading/Extracting), completed layer count, and overall byte progress with a Nimbo-themed gradient.
  • The pull phase now shows a spinner ("Pulling image...") with the aggregated progress bar rendered below it, replacing the per-layer log spam.
  • Pull errors now emit a styled ErrorEvent instead of a raw error string, consistent with how other failures (e.g., Docker not available) are displayed.
  • Non-interactive output suppresses per-layer progress events entirely, showing only "Pulling..." and "Pulled" lines.

Tests

  • Unit tests for the PullProgress component covering aggregation, layer counting, phase detection, visibility, and edge cases.
  • Unit tests for the TUI app verifying pull progress shows/hides on the correct status events and progress events update the component without appending lines.
  • Updated existing plain sink tests to match the new suppression behavior.

Screenshots

image image

@silv-io silv-io marked this pull request as ready for review March 6, 2026 14:57
@coderabbitai
Copy link

coderabbitai bot commented Mar 6, 2026

📝 Walkthrough

Walkthrough

This PR introduces visual progress feedback for container image pulls by adding a new TUI progress component, emitting pull-related events from the container service, suppressing plain text progress output, and enabling Cobra's silent error handling for custom error display.

Changes

Cohort / File(s) Summary
Cobra Error Handling
cmd/root.go
Suppresses Cobra's automatic error and usage output by setting SilenceErrors and SilenceUsage flags to enable custom error handling.
Dependency Management
go.mod
Removes direct pflag dependency from root require block and adds harmonica as indirect dependency; pflag becomes indirect via Cobra.
Container Pull Events
internal/container/start.go
Emits spinner and error/success events during image pulls; wraps pull failures in silent errors while preserving UI event signals.
Plain Text Output Handling
internal/output/plain_sink.go, internal/output/plain_sink_test.go
Short-circuits ProgressEvent output in plain text sink to suppress progress display; updated tests verify event suppression and remove ProgressEvent from formatter parity checks.
UI Application Layer
internal/ui/app.go, internal/ui/app_test.go
Integrates pullProgress component into App; routes ContainerStatusEvent and ProgressEvent to progress display and handles progress.FrameMsg updates; added tests for pull phase visibility and event handling.
Pull Progress Component
internal/ui/components/pull_progress.go, internal/ui/components/pull_progress_test.go
New component providing multi-layer progress tracking with per-layer status, aggregated progress calculation, and display of dominant phase (Downloading/Extracting/Pulling); comprehensive test coverage for visibility, progress updates, and phase determination.

Sequence Diagram

sequenceDiagram
    participant Container as Container Service
    participant EventSink as Event Sink
    participant UIApp as UI App
    participant ProgressComp as Pull Progress<br/>Component
    participant View as Terminal View

    Container->>Container: Start pulling image
    Container->>EventSink: Emit spinner start event
    Container->>Container: Pull image (with layer updates)
    
    loop For each layer progress update
        Container->>EventSink: Emit ProgressEvent (layer ID, status, current, total)
        EventSink->>UIApp: Route ProgressEvent
        UIApp->>ProgressComp: SetProgress(event)
        ProgressComp->>ProgressComp: Update layer state & aggregate %
        ProgressComp-->>UIApp: Return updated component + cmd
    end
    
    alt Pull succeeds
        Container->>EventSink: Emit spinner stop + success
        EventSink->>UIApp: Route success event
        UIApp->>View: Append success message
    else Pull fails
        Container->>EventSink: Emit error event + spinner stop
        EventSink->>UIApp: Route error event
        UIApp->>ProgressComp: Hide()
        UIApp->>View: Append error message
    end
    
    UIApp->>View: Render (spinner + progress layers + output)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • carole-lavillonniere
  • anisaoshafi
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main objective of the PR: collapsing noisy per-layer Docker image pull progress into a single aggregated loading indicator component.
Description check ✅ Passed The description comprehensively explains the motivation, all key changes, testing strategy, and includes visual examples directly related to the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch silv-io/flc-400

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/container/start.go`:
- Around line 97-98: The code is emitting the same "Pulling <image>" twice by
calling both output.EmitSpinnerStart(sink, fmt.Sprintf("Pulling %s", c.Image))
and output.EmitStatus(sink, "pulling", c.Image, ""), which duplicates output in
non-interactive/PlainSink modes; remove the redundant EmitStatus call (or guard
it behind an interactive/terminal check) so that only the spinner start is
emitted here and let the existing ContainerStatusEvent path produce the status
once (locate calls to EmitSpinnerStart and EmitStatus in start.go around the
image pull logic).

In `@internal/output/plain_sink.go`:
- Around line 33-35: Remove the special-case ProgressEvent early return from
PlainSink so sinks no longer own suppression logic; instead update
FormatEventLine(event any) to detect ProgressEvent and return ok=false for it,
then have PlainSink call FormatEventLine and skip writing when ok==false.
Specifically, delete the switch/case that returns for ProgressEvent in
PlainSink, ensure FormatEventLine treats ProgressEvent as non-renderable
(ok=false), and make PlainSink follow the common path of using FormatEventLine's
(text, ok) result to decide whether to emit the line.

In `@internal/ui/components/pull_progress.go`:
- Around line 56-58: The aggregate progress regresses because each update
blindly replaces layer.bytes with the phase-specific values (e.Status,
e.Current, e.Total) causing Extracting phases to reduce totals and
cached/terminal states to contribute zero; change the update logic in the layer
update path (where layer.status, layer.current, layer.total are set) and in
aggregatePercent() to be phase-aware: preserve the maximum observed bytes
per-layer (e.g., track seenTotal/seenCurrent) and treat terminal states like
"Already exists"/"Pull complete" as fully-complete for that layer (use seenTotal
or mark as complete to add its bytes to totalBytes) so totalBytes never goes to
zero and progress never moves backward. Ensure any code paths referenced by
aggregatePercent(), layer.status, layer.current, and layer.total are updated to
use the new seen/complete fields.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 8c79b5a3-90b9-4dc6-81fa-d834c762b7c9

📥 Commits

Reviewing files that changed from the base of the PR and between 4819f11 and c238617.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (9)
  • cmd/root.go
  • go.mod
  • internal/container/start.go
  • internal/output/plain_sink.go
  • internal/output/plain_sink_test.go
  • internal/ui/app.go
  • internal/ui/app_test.go
  • internal/ui/components/pull_progress.go
  • internal/ui/components/pull_progress_test.go

Copy link
Contributor

@anisaoshafi anisaoshafi left a comment

Choose a reason for hiding this comment

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

LGTM!
Thanks for cleaning up that noise, and adding many tests 👏🏼
image
image

@silv-io silv-io merged commit 5bd9446 into main Mar 9, 2026
7 checks passed
@silv-io silv-io deleted the silv-io/flc-400 branch March 9, 2026 07:46
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