From 3ede3bb87a11c9cc6b8bde5e693fa0a5bfd0cbbe Mon Sep 17 00:00:00 2001 From: Danilo Alonso Date: Tue, 8 Sep 2026 23:13:36 -0400 Subject: [PATCH 1/2] feat(tui): show download progress while installing an update (#88) * feat(tui): track update download progress from observer events The updater has emitted received/total every 512KB and a retry notice all along; nothing in the TUI listened. Mirrors useTransferProgress so the update screen can show real progress instead of a bare spinner. * feat(tui): render download progress on the update screen The installing state was a bare spinner for the whole of a ~70MB download, indistinguishable from a hang. Shows MB, percent and a bar once the total is known, MB alone when the server sends no Content-Length, and the retry reason so a resume does not read as one. * refactor(cli): correct a comment promising a fallback that never existed The comment claimed piped output falls back to periodic newlines; it prints nothing, and suppression is right, since a tick per 512KB would bury a CI log. Behaviour is unchanged, the early return just replaces formatting a line that was already being discarded. * docs(tui): add the Update screen to the screen reference The screen reference had no entry for it at all, so the download progress, the unknown-size fallback and the resume notice were undocumented. * docs(update): bring the spec up to standard and drop a drawn mockup The spec was missing the change tree, outline, flows, risks and change log a sibling spec two commits earlier already carried; its approaches table moves to a design doc. The screen reference loses a hand-drawn progress bar, the artifact class the VHS pipeline exists to retire. * chore(changeset): add update download progress Every user-facing change in this repo carries one; this had none. --- .changeset/update-download-progress.md | 13 ++ docs/design/update-download-progress.md | 47 ++++++ docs/spec/update-download-progress.md | 152 +++++++++++++++++++ docs/tui.md | 20 +++ src/cli/update.ts | 15 +- src/tui/hooks/index.ts | 7 + src/tui/hooks/useUpdateProgress.ts | 162 +++++++++++++++++++++ src/tui/screens/UpdateScreen.tsx | 32 +++- tests/cli/hooks/useUpdateProgress.test.tsx | 131 +++++++++++++++++ 9 files changed, 568 insertions(+), 11 deletions(-) create mode 100644 .changeset/update-download-progress.md create mode 100644 docs/design/update-download-progress.md create mode 100644 docs/spec/update-download-progress.md create mode 100644 src/tui/hooks/useUpdateProgress.ts create mode 100644 tests/cli/hooks/useUpdateProgress.test.tsx diff --git a/.changeset/update-download-progress.md b/.changeset/update-download-progress.md new file mode 100644 index 00000000..7c654f8e --- /dev/null +++ b/.changeset/update-download-progress.md @@ -0,0 +1,13 @@ +--- +'@noormdev/cli': minor +--- + +Show download progress while the TUI installs an update + +The updater has always emitted byte counts every 512KB, and the CLI has always +rendered them, but the TUI's update screen ignored them: a ~70MB binary +download sat behind a bare spinner for its whole duration, indistinguishable +from a hang. The installing state now shows received and total megabytes, a +percent, and a progress bar, falling back to megabytes alone when the server +sends no `Content-Length`. A stalled attempt that resumes says so, and keeps +the progress it had, because the retry resumes from the bytes already on disk. diff --git a/docs/design/update-download-progress.md b/docs/design/update-download-progress.md new file mode 100644 index 00000000..9efc782f --- /dev/null +++ b/docs/design/update-download-progress.md @@ -0,0 +1,47 @@ +# Update download progress (TUI) + + +## Problem + + +`src/core/update/updater.ts` already emits `update:progress` (`{version, received, total}`) every 512KB during the binary download, plus `update:retry` when a stalled attempt resumes. `src/cli/update.ts` already renders both as a carriage-return status line. + +The TUI does not listen. `UpdateScreen`'s installing branch is a bare `Spinner` for the whole of a ~70MB download, so the screen is indistinguishable from a hang for as long as the download runs. The data needed to fix that is already flowing through the process; nothing subscribes to it. + + +## Goals / Non-goals + + +Goal: the TUI shows the same download progress the CLI already shows, sourced from the same events. + +Non-goals: + +- Any change to the download, retry, resume, checksum, or swap mechanism. That code is correct and this work does not touch it. +- A new event. `update:progress` already carries everything needed. +- Progress for the npm install path. `npm install -g` owns its own output and reports no byte counts. + + +## Approaches + + +| # | Approach | Sketch | Cost | Risk | +|---|----------|--------|------|------| +| A | Separate `useUpdateProgress` hook, consumed by the screen alongside `useUpdateChecker` | Mirrors `useRunProgress` / `useTransferProgress` / `useChangeProgress` | low | none material | +| B | Fold progress state into the existing `useUpdateChecker` | One hook owns check, install and progress | low | breaks `useUpdateChecker.test.tsx`; merges two lifecycles | +| C | Subscribe with `useOnEvent` directly inside `UpdateScreen` | No new file | trivial | reducer logic in a render path, untestable without rendering the screen | + + +## Recommendation + + +**A.** The repo has already answered this question three times: `useRunProgress`, `useTransferProgress` and `useChangeProgress` are each a hook that turns an observer event stream into screen state. Update progress has the same shape, so it takes the same shape, and a reader who knows one knows all four. + +**B** looks like the smaller diff but fails for a structural reason rather than a stylistic one. `useOnEvent` requires the `NoormObserver` provider, and `tests/cli/hooks/useUpdateChecker.test.tsx` renders that hook with no provider around it. Folding the subscription in breaks an existing passing test, and the repair would be to wrap that test in a provider it never needed, which is a worse trade than adding a file. + +**C** puts reducer logic somewhere it can only be exercised by rendering the whole screen. That matters more than usual here: reaching the installing branch requires `installUpdate` to be mid-flight, which cannot be arranged without `mock.module`, and Bun's mock registry is process-global and never restores, so a mock in a screen test poisons every later file in its CI group. Keeping the state transitions in a hook keeps them testable with real `observer.emit` calls and no mocks at all. + + +## Open questions + + +None. The event contract is fixed and already in production behind the CLI. diff --git a/docs/spec/update-download-progress.md b/docs/spec/update-download-progress.md new file mode 100644 index 00000000..154dc511 --- /dev/null +++ b/docs/spec/update-download-progress.md @@ -0,0 +1,152 @@ +# Update download progress (TUI) + + +## Goal + + +Show live download progress on the TUI's update screen. `src/core/update/updater.ts` already emits `update:progress` (`{version, received, total}`) every 512KB and `update:retry` on a resumed attempt, and `src/cli/update.ts` already renders both. The TUI ignores them: `UpdateScreen` shows a bare `Spinner` for the whole of a ~70MB download, so the screen looks identical to a hang. + +Consume the events that are already flowing. No change to `core/update`. + + +## Non-goals + + +- Any change to the download, retry, resume, checksum, or swap mechanism in `src/core/update/updater.ts`. It is already correct. +- Changing how the CLI renders progress. That is already shipped, beyond one comment that describes a fallback the code does not implement. +- Progress for the npm install path, since `npm install -g` owns its own output and emits no byte counts. +- A new event. `update:progress` already carries everything needed. + + +## Success criteria + + +- [ ] A `useUpdateProgress` hook exists in `src/tui/hooks/`, subscribes via `useOnEvent` to `update:progress` and `update:retry`, and exposes `{ state, reset }` following the shape of the sibling `useTransferProgress`. +- [ ] It is a **separate hook**, not an extension of `useUpdateChecker`: `useOnEvent` requires the `NoormObserver` provider, and `tests/cli/hooks/useUpdateChecker.test.tsx` renders that hook without one, so folding the subscription in would break it. +- [ ] Exported from the `src/tui/hooks/index.ts` barrel alongside its state type. +- [ ] `UpdateScreen`'s installing state renders received/total MB, a floored integer percent, and an `@inkjs/ui` `ProgressBar` when the total is known; falls back to MB-only with the spinner when it is not. +- [ ] A retry shows the attempt (`n/max`) and its reason, so a resumed download does not read as a stall. +- [ ] Progress state resets when a new install starts, so a second attempt in one session does not begin at the previous run's percentage. +- [ ] Hook tests live in `tests/cli/hooks/useUpdateProgress.test.tsx`, wrap in `NoormObserver`, and drive real `observer.emit(...)` calls, following the `useTransferProgress.test.tsx` pattern rather than module mocks. +- [ ] `src/cli/update.ts`'s non-TTY comment matches its behaviour: it claims a "fall back to periodic newlines" that the code does not implement (`onProgress` writes nothing when `!isTty`). +- [ ] `bun run typecheck`, `bun run lint`, and the CLI test group pass. + + +## Approach + + +**A**, a separate `useUpdateProgress` hook consumed by the screen. See [the design doc](../design/update-download-progress.md). + + +## Change tree + + +``` +src/tui/hooks/ +├── useUpdateProgress.ts .................... A (useUpdateProgress; UpdateProgressState/UpdatePhase/UpdateRetryInfo) +└── index.ts ................................ M (barrel export) +src/tui/screens/ +└── UpdateScreen.tsx ........................ M (installing branch: MB, percent, ProgressBar, retry line, reset on install) +src/cli/ +└── update.ts ............................... M (non-TTY comment corrected to match behaviour; early return) +tests/cli/hooks/ +└── useUpdateProgress.test.tsx .............. A (real observer.emit under NoormObserver, no module mocks) +docs/ +├── tui.md .................................. M (### Update entry in the screen reference) +├── design/update-download-progress.md ...... A (approaches, recommendation) +└── spec/update-download-progress.md ........ A (this file) +``` + + +## Outline + + +``` +src/tui/hooks/useUpdateProgress.ts + UpdatePhase — 'idle' | 'downloading' | 'complete' + UpdateRetryInfo — attempt (0-based, as the event carries it), maxAttempts, error + UpdateProgressState — phase, received, total, retry + useUpdateProgress — subscribes the four update events, returns { state, reset } + update:installing — phase to downloading and counters cleared, so a second install in one session does not start at the prior run's numbers + update:progress — received/total + update:retry — records the retry without zeroing received, since the download resumes from bytes already on disk + update:complete — phase to complete + reset — back to INITIAL_STATE, for the caller to clear at keypress time + +src/tui/hooks/index.ts + useUpdateProgress export — hook plus UpdateProgressState, UpdatePhase, UpdateRetryInfo + +src/tui/screens/UpdateScreen.tsx + installing branch — MB to one decimal, floored percent, ProgressBar gated on total > 0, MB-only fallback when total is 0, retry line rendering attempt + 1 + handleInstall — resetProgress() before performUpdate() + +src/cli/update.ts + onProgress — early return off-TTY rather than formatting a line to discard; comment states suppression rather than a newline fallback that never existed + +tests/cli/hooks/useUpdateProgress.test.tsx + progress updates received/total + a fresh update:installing resets counters from a prior run + a retry is recorded without zeroing received + complete sets the phase + +docs/tui.md + ### Update — screen reference entry: keys, the progress display, the unknown-size fallback, the resume notice +``` + + +## Flows + + +**Watching a binary update download** + +1. User presses `u` from Home, then `i` to install. +2. `handleInstall` calls `resetProgress()`, clearing any prior run's numbers before anything is emitted. +3. `installUpdate` emits `update:installing` synchronously, before its first `await`; the hook moves phase to `downloading` and clears counters again, which keeps the hook correct for any consumer that did not call `reset` itself. +4. The download loop emits `update:progress` every 512KB. The screen renders `received / total MB (percent%)` and a `ProgressBar`. +5. On the final chunk the updater emits once more, so the display lands on the true total rather than the last 512KB boundary. +6. `update:complete` moves phase to `complete`; the screen's own `done` state takes over from `performUpdate`'s resolved result. + +**A stalled download resuming** + +1. No bytes arrive for 30s, so the updater aborts that attempt and emits `update:retry` with the reason and the attempt number. +2. The hook records the retry and leaves `received` alone. +3. The screen shows the reason and `attempt + 1` of `maxAttempts` in yellow, so the pause reads as a resume rather than a hang. +4. The retried request resumes from the bytes already on disk via an HTTP range request, so progress continues from where it stopped rather than restarting at zero. + +**A server that sends no Content-Length** + +1. `update:progress` arrives with `total` of 0. +2. The screen renders received megabytes alone, with no percent and no `ProgressBar`, rather than a bar stuck at 0% or NaN%. + + +## Checkpoints + + +| # | Checkpoint | Files/areas | Agent | Est. files | Verifies | +|---|------------|-------------|-------|------------|----------| +| 1 | `useUpdateProgress` hook + barrel export + hook tests driving real `observer.emit` under `NoormObserver` | `src/tui/hooks/useUpdateProgress.ts`, `src/tui/hooks/index.ts`, `tests/cli/hooks/useUpdateProgress.test.tsx` | atomic-implementer (surgical) | ~3 | `bun test --serial tests/cli/hooks`; `bun run typecheck` | +| 2 | Render it: `UpdateScreen` installing state gains MB / percent / `ProgressBar` / retry notice, and resets on install start | `src/tui/screens/UpdateScreen.tsx` | atomic-implementer (surgical) | 1 | `bun run typecheck`; `bun run lint`; screen renders progress rather than a bare spinner | +| 3 | Correct the CLI's non-TTY comment to match behaviour | `src/cli/update.ts` | atomic-implementer (surgical) | 1 | comment and code agree | +| 4 | Document the TUI progress display | `docs/tui.md` | atomic-implementer (surgical) | 1 | update screen section describes what the user sees | + + +## Risks + + +| Risk | Likelihood | Mitigation | +|------|-----------|-----------| +| The installing branch cannot be reached in a test without `mock.module`, whose registry is process-global in Bun and never restores, so a screen test would poison every later file in its CI group | high | Keep all state transitions in the hook, where real `observer.emit` calls test them with no mocks; verify the JSX by rendering the screen out-of-process, where pollution cannot reach the suite | +| A second install in one session starts at the previous run's percentage | med | Reset on `update:installing` inside the hook, and call `reset()` from `handleInstall` before `performUpdate()`; covered by a hook test | +| A retry zeroes displayed progress, making a resume look like a restart | med | The retry handler leaves `received` untouched, matching the range-request resume the updater actually performs; covered by a hook test | +| A `ProgressBar` renders at 0% or NaN% when the server sends no `Content-Length` | med | Gate both the percent and the bar on `total > 0`, falling back to bare megabytes | +| The TUI and CLI drift into different vocabulary for the same event | low | The retry line mirrors `src/cli/update.ts`'s wording verbatim, including its `attempt + 1` conversion | + + +## Change log + + +### 2026-09-08 — spec brought up to repo standard + +**What changed:** Added the `## Change tree`, `## Outline`, `## Flows`, `## Risks` and `## Change log` sections the standing spec-currency rule requires. The `## Approaches` table and `## Recommendation` argument moved to `docs/design/update-download-progress.md`, leaving a one-line `## Approach` pointer in their place. + +**Why:** Audit finding. `docs/spec/sdk-with-schema.md`, authored a few commits earlier in this repo, carries all five sections and the pointer form, so the convention was in force when this spec was drafted and this spec simply did not follow it. diff --git a/docs/tui.md b/docs/tui.md index aa00744a..1a2fdbba 100644 --- a/docs/tui.md +++ b/docs/tui.md @@ -439,6 +439,26 @@ status, `[a]` acquires, `[r]` releases, and `[f]` force-breaks a stale lock. See [Locking](/dev/lock). +### Update + +Press `u` from Home to check GitHub releases for a newer noorm. `[i]` +installs an update if one is available, `[r]` re-checks, and `esc` goes +back. + +The binary runs to around 70MB, so the install reports where it has got to +rather than leaving you watching a spinner. Below the spinner you get the +byte count as `42.0 / 73.0 MB (57%)`, and a progress bar tracking the same +percent. When the server sends no `Content-Length` there's nothing to +measure against, so the count drops to a bare `9.0 MB received` and the bar +is omitted rather than sitting at zero. + +If a download stalls and resumes, a yellow line names why and which attempt +it's on, for example `download stalled, no data for 30s — resuming (attempt +1/5)...`. Progress isn't reset by the retry, since it resumes from the bytes +already on disk rather than starting the file over. The npm install path +(`npm install -g`) owns its own output, so none of this applies there. + + ## Tips diff --git a/src/cli/update.ts b/src/cli/update.ts index 2aa4582d..36ef4d38 100644 --- a/src/cli/update.ts +++ b/src/cli/update.ts @@ -92,22 +92,21 @@ const updateCommand = defineCommand({ process.stdout.write(`Update available: ${currentVersion} → ${checkResult.latestVersion}\n`); - // Render a live progress line for the binary download. TTY output uses a - // carriage return to update in place; when stdout is piped (e.g. CI, JSON - // mode) there's no cursor to rewind, so fall back to periodic newlines. + // Render a live progress line for the binary download, TTY only. Piped + // output (CI, JSON mode) has no cursor to rewind, so every tick would + // land on its own line and bury the log under a few hundred of them. + // The static "Installing" line below is that case's only progress note. const isTty = Boolean(process.stdout.isTTY) && !args.json; const onProgress = ({ received, total }: { received: number; total: number }) => { + if (!isTty) return; + const pct = total > 0 ? ` (${Math.floor((received / total) * 100)}%)` : ''; const of = total > 0 ? ` / ${toMb(total)}` : ''; const line = `Downloading ${toMb(received)}${of} MB${pct}`; - if (isTty) { - - process.stdout.write(`\r${line} `); - - } + process.stdout.write(`\r${line} `); }; diff --git a/src/tui/hooks/index.ts b/src/tui/hooks/index.ts index 5bb1a852..e83d4984 100644 --- a/src/tui/hooks/index.ts +++ b/src/tui/hooks/index.ts @@ -24,6 +24,13 @@ export { export { useUpdateChecker, type UseUpdateCheckerResult } from './useUpdateChecker.js'; +export { + useUpdateProgress, + type UpdateProgressState, + type UpdatePhase, + type UpdateRetryInfo, +} from './useUpdateProgress.js'; + export { useChangeProgress, type ChangeProgressState, diff --git a/src/tui/hooks/useUpdateProgress.ts b/src/tui/hooks/useUpdateProgress.ts new file mode 100644 index 00000000..aa01b523 --- /dev/null +++ b/src/tui/hooks/useUpdateProgress.ts @@ -0,0 +1,162 @@ +/** + * Hook for tracking update download progress via observer events. + * + * Subscribes to update:* events and maintains state for + * displaying download progress in TUI screens. + * + * @example + * ```tsx + * function UpdateScreen() { + * const { state, reset } = useUpdateProgress(); + * + * return ( + * + * {state.received} / {state.total} bytes + * {state.retry && retry {state.retry.attempt + 1}/{state.retry.maxAttempts}: {state.retry.error}} + * + * ); + * } + * ``` + */ +import { useState, useCallback } from 'react'; + +import { useOnEvent } from './useObserver.js'; + +/** + * Phase of the update installation. + */ +export type UpdatePhase = 'idle' | 'downloading' | 'complete'; + +/** + * Last retry recorded by useUpdateProgress. + */ +export interface UpdateRetryInfo { + + /** Attempt number, 0-based */ + attempt: number; + + /** Maximum number of attempts */ + maxAttempts: number; + + /** Reason for the retry */ + error: string; + +} + +/** + * State tracked by useUpdateProgress. + */ +export interface UpdateProgressState { + + /** Current phase of the update */ + phase: UpdatePhase; + + /** Bytes received so far */ + received: number; + + /** Total bytes expected, 0 when unknown */ + total: number; + + /** Most recent retry, or null if none has occurred */ + retry: UpdateRetryInfo | null; + +} + +/** + * Initial state for update progress. + */ +const INITIAL_STATE: UpdateProgressState = { + phase: 'idle', + received: 0, + total: 0, + retry: null, +}; + +type UseUpdateProgressReturn = { + state: UpdateProgressState; + reset: () => void; +}; + +/** + * Hook for tracking update download progress. + * + * Returns the current state and a reset function to prepare + * for a new install. + */ +export function useUpdateProgress(): UseUpdateProgressReturn { + + const [state, setState] = useState(INITIAL_STATE); + + /** + * Reset state for a new install. + */ + const reset = useCallback(() => { + + setState(INITIAL_STATE); + + }, []); + + // Subscribe to update:installing + useOnEvent( + 'update:installing', + () => { + + setState(() => ({ + ...INITIAL_STATE, + phase: 'downloading', + })); + + }, + [], + ); + + // Subscribe to update:progress + useOnEvent( + 'update:progress', + (data) => { + + setState((prev) => ({ + ...prev, + received: data.received, + total: data.total, + })); + + }, + [], + ); + + // Subscribe to update:retry + useOnEvent( + 'update:retry', + (data) => { + + setState((prev) => ({ + ...prev, + retry: { + attempt: data.attempt, + maxAttempts: data.maxAttempts, + error: data.error, + }, + })); + + }, + [], + ); + + // Subscribe to update:complete + useOnEvent( + 'update:complete', + () => { + + setState((prev) => ({ + ...prev, + phase: 'complete', + })); + + }, + [], + ); + + return { state, reset }; + +} diff --git a/src/tui/screens/UpdateScreen.tsx b/src/tui/screens/UpdateScreen.tsx index 31e39730..9a6cba59 100644 --- a/src/tui/screens/UpdateScreen.tsx +++ b/src/tui/screens/UpdateScreen.tsx @@ -11,8 +11,8 @@ import { Box, Text, useInput } from 'ink'; import type { ScreenProps } from '../types.js'; import { useFocusScope } from '../focus.js'; import { useRouter } from '../router.js'; -import { useToast, Spinner } from '../components/index.js'; -import { useUpdateChecker } from '../hooks/index.js'; +import { useToast, Spinner, ProgressBar } from '../components/index.js'; +import { useUpdateChecker, useUpdateProgress } from '../hooks/index.js'; /** * Update screen showing current version, update availability, and install action. @@ -29,12 +29,15 @@ export function UpdateScreen(_props: ScreenProps): ReactElement { performUpdate, recheckForUpdate, } = useUpdateChecker(); + const { state: progress, reset: resetProgress } = useUpdateProgress(); const [done, setDone] = useState(false); const [error, setError] = useState(null); const handleInstall = useCallback(async (): Promise => { + resetProgress(); + const result = await performUpdate(); if (!result) return; @@ -55,7 +58,7 @@ export function UpdateScreen(_props: ScreenProps): ReactElement { } - }, [performUpdate, showToast]); + }, [performUpdate, showToast, resetProgress]); useInput((input, key) => { @@ -105,11 +108,34 @@ export function UpdateScreen(_props: ScreenProps): ReactElement { // Installing state if (installing) { + const receivedMb = (progress.received / 1024 / 1024).toFixed(1); + const totalMb = (progress.total / 1024 / 1024).toFixed(1); + const percent = progress.total > 0 ? Math.floor((progress.received / progress.total) * 100) : null; + return ( + + {percent !== null ? ( + {receivedMb} / {totalMb} MB ({percent}%) + ) : ( + {receivedMb} MB received + )} + + {percent !== null && ( + + + + )} + {progress.retry && ( + + + {progress.retry.error} — resuming (attempt {progress.retry.attempt + 1}/{progress.retry.maxAttempts})... + + + )} ); diff --git a/tests/cli/hooks/useUpdateProgress.test.tsx b/tests/cli/hooks/useUpdateProgress.test.tsx new file mode 100644 index 00000000..793fdc4f --- /dev/null +++ b/tests/cli/hooks/useUpdateProgress.test.tsx @@ -0,0 +1,131 @@ +/** + * useUpdateProgress hook tests. + * + * Tests event handlers for update download progress tracking. + */ +import { describe, it, expect } from 'bun:test'; +import { render } from 'ink-testing-library'; +import React from 'react'; +import { Text, Box } from 'ink'; + +import { observer } from '../../../src/core/observer.js'; +import { NoormObserver } from '../../../src/tui/observer-context.js'; +import { useUpdateProgress } from '../../../src/tui/hooks/useUpdateProgress.js'; + +/** + * Test component that renders update progress state. + */ +function UpdateProgressView() { + + const { state } = useUpdateProgress(); + + return ( + + phase:{state.phase} + received:{state.received} + total:{state.total} + retry:{state.retry ? `${state.retry.attempt}/${state.retry.maxAttempts}:${state.retry.error}` : 'none'} + + ); + +} + +/** + * Wrap component with NoormObserver provider for testing. + */ +function WithProvider({ children }: { children: React.ReactNode }) { + + return {children}; + +} + +describe('cli: hooks/useUpdateProgress', () => { + + it('should update received/total on update:progress', async () => { + + const { lastFrame, unmount } = render(); + + await new Promise((r) => setTimeout(r, 50)); + + observer.emit('update:installing', { version: '1.2.3' }); + await new Promise((r) => setTimeout(r, 50)); + + expect(lastFrame()).toContain('phase:downloading'); + + observer.emit('update:progress', { version: '1.2.3', received: 512000, total: 2048000 }); + await new Promise((r) => setTimeout(r, 50)); + + expect(lastFrame()).toContain('received:512000'); + expect(lastFrame()).toContain('total:2048000'); + + unmount(); + + }); + + it('should reset counters on a fresh update:installing after a prior run', async () => { + + const { lastFrame, unmount } = render(); + + await new Promise((r) => setTimeout(r, 50)); + + observer.emit('update:installing', { version: '1.2.3' }); + await new Promise((r) => setTimeout(r, 50)); + + observer.emit('update:progress', { version: '1.2.3', received: 1500000, total: 2048000 }); + await new Promise((r) => setTimeout(r, 50)); + + expect(lastFrame()).toContain('received:1500000'); + + // Second install in the same session should not start at the previous percentage + observer.emit('update:installing', { version: '1.3.0' }); + await new Promise((r) => setTimeout(r, 50)); + + expect(lastFrame()).toContain('received:0'); + expect(lastFrame()).toContain('total:0'); + expect(lastFrame()).toContain('phase:downloading'); + + unmount(); + + }); + + it('should record a retry without zeroing received', async () => { + + const { lastFrame, unmount } = render(); + + await new Promise((r) => setTimeout(r, 50)); + + observer.emit('update:installing', { version: '1.2.3' }); + await new Promise((r) => setTimeout(r, 50)); + + observer.emit('update:progress', { version: '1.2.3', received: 800000, total: 2048000 }); + await new Promise((r) => setTimeout(r, 50)); + + observer.emit('update:retry', { version: '1.2.3', attempt: 0, maxAttempts: 3, error: 'stalled' }); + await new Promise((r) => setTimeout(r, 50)); + + expect(lastFrame()).toContain('retry:0/3:stalled'); + expect(lastFrame()).toContain('received:800000'); + + unmount(); + + }); + + it('should set phase to complete on update:complete', async () => { + + const { lastFrame, unmount } = render(); + + await new Promise((r) => setTimeout(r, 50)); + + observer.emit('update:installing', { version: '1.2.3' }); + await new Promise((r) => setTimeout(r, 50)); + + observer.emit('update:complete', { previousVersion: '1.2.0', newVersion: '1.2.3' }); + await new Promise((r) => setTimeout(r, 50)); + + expect(lastFrame()).toContain('phase:complete'); + + unmount(); + + }); + +}); From e9ac9f4529801efbeeb9f4b04a4bc2ec4889a474 Mon Sep 17 00:00:00 2001 From: Danilo Alonso Date: Thu, 10 Sep 2026 16:13:52 -0400 Subject: [PATCH 2/2] fix(tui): make progress bars advance --- .changeset/tui-ink7-and-row-inspection.md | 1 + src/tui/hooks/useChangeProgress.ts | 2 +- src/tui/hooks/useRunProgress.ts | 3 ++- src/tui/hooks/useTransferProgress.ts | 2 +- src/tui/screens/change/ChangeFFScreen.tsx | 11 ++++++++-- src/tui/screens/change/ChangeNextScreen.tsx | 11 ++++++++-- src/tui/screens/change/ChangeRevertScreen.tsx | 3 ++- src/tui/screens/change/ChangeRewindScreen.tsx | 11 ++++++++-- src/tui/screens/change/ChangeRunScreen.tsx | 3 ++- src/tui/screens/db/DbTransferScreen.tsx | 11 +++------- src/tui/screens/run/RunBuildScreen.tsx | 4 ++-- src/tui/screens/run/RunDirScreen.tsx | 4 ++-- src/tui/screens/run/RunExecScreen.tsx | 4 ++-- src/tui/utils/index.ts | 1 + src/tui/utils/progress.ts | 16 ++++++++++++++ tests/cli/utils/progress.test.ts | 21 +++++++++++++++++++ 16 files changed, 83 insertions(+), 25 deletions(-) create mode 100644 src/tui/utils/progress.ts create mode 100644 tests/cli/utils/progress.test.ts diff --git a/.changeset/tui-ink7-and-row-inspection.md b/.changeset/tui-ink7-and-row-inspection.md index c55f6b99..0022f30d 100644 --- a/.changeset/tui-ink7-and-row-inspection.md +++ b/.changeset/tui-ink7-and-row-inspection.md @@ -22,3 +22,4 @@ * `fix(explore):` Column, index and parameter lists re-flowed per row, so the type column landed on a different offset on nearly every row. * `fix(explore):` "Total Objects" counted categories the screen does not list, so it exceeded the rows a reader could see. * `fix(tui):` The help screen and log viewer drew past the bottom of the window, putting their first lines out of reach. +* `fix(tui):` Build, file, change, and transfer progress bars now advance with completed work instead of remaining visually empty until completion. diff --git a/src/tui/hooks/useChangeProgress.ts b/src/tui/hooks/useChangeProgress.ts index ad28a91e..47e12cd8 100644 --- a/src/tui/hooks/useChangeProgress.ts +++ b/src/tui/hooks/useChangeProgress.ts @@ -92,7 +92,7 @@ export function useChangeProgress(): ChangeProgressState { useOnEvent('change:file', (data) => { setCurrentFile(data.filepath); - setFileProgress({ current: data.index, total: data.total }); + setFileProgress({ current: data.index + 1, total: data.total }); }, []); diff --git a/src/tui/hooks/useRunProgress.ts b/src/tui/hooks/useRunProgress.ts index 1cb9b8c4..e1ea5083 100644 --- a/src/tui/hooks/useRunProgress.ts +++ b/src/tui/hooks/useRunProgress.ts @@ -17,7 +17,7 @@ * return ( * * Running: {state.currentFile} - * + * * * {state.filesRun}/{state.filesTotal} files * ({state.filesSkipped} skipped, {state.filesFailed} failed) @@ -118,6 +118,7 @@ const INITIAL_STATE: RunProgressState = { type UseRunProgressReturn = { state: RunProgressState; + /** Reset all counters before starting a run with the given file count. */ reset: (totalFiles: number) => void; }; diff --git a/src/tui/hooks/useTransferProgress.ts b/src/tui/hooks/useTransferProgress.ts index 95d261fc..31527571 100644 --- a/src/tui/hooks/useTransferProgress.ts +++ b/src/tui/hooks/useTransferProgress.ts @@ -17,7 +17,7 @@ * return ( * * Transferring: {state.currentTable} - * + * * * {state.rowsTransferred} rows transferred * diff --git a/src/tui/screens/change/ChangeFFScreen.tsx b/src/tui/screens/change/ChangeFFScreen.tsx index 35c661d7..f0dbb241 100644 --- a/src/tui/screens/change/ChangeFFScreen.tsx +++ b/src/tui/screens/change/ChangeFFScreen.tsx @@ -32,7 +32,14 @@ import { } from '../../components/index.js'; import { checkConfigPolicy } from '../../../core/policy/index.js'; import { useChangeProgress, useAsyncEffect } from '../../hooks/index.js'; -import { getErrorMessage, loadChangesWithStatus, buildPendingChangeList, createChangeManager, isConfigGuarded } from '../../utils/index.js'; +import { + getErrorMessage, + loadChangesWithStatus, + buildPendingChangeList, + createChangeManager, + isConfigGuarded, + progressPercentage, +} from '../../utils/index.js'; import { validateChangeContent } from '../../../core/change/validation.js'; import { createConnection } from '../../../core/connection/factory.js'; @@ -288,7 +295,7 @@ export function ChangeFFScreen({ params: _params }: ScreenProps): ReactElement { // Running if (step === 'running') { - const progressValue = progress.total > 0 ? progress.current / progress.total : 0; + const progressValue = progressPercentage(progress.current, progress.total); return ( diff --git a/src/tui/screens/change/ChangeNextScreen.tsx b/src/tui/screens/change/ChangeNextScreen.tsx index 044fcbd6..4ad70ebb 100644 --- a/src/tui/screens/change/ChangeNextScreen.tsx +++ b/src/tui/screens/change/ChangeNextScreen.tsx @@ -34,7 +34,14 @@ import { } from '../../components/index.js'; import { checkConfigPolicy } from '../../../core/policy/index.js'; import { useChangeProgress, useAsyncEffect } from '../../hooks/index.js'; -import { getErrorMessage, loadChangesWithStatus, buildPendingChangeList, createChangeManager, isConfigGuarded } from '../../utils/index.js'; +import { + getErrorMessage, + loadChangesWithStatus, + buildPendingChangeList, + createChangeManager, + isConfigGuarded, + progressPercentage, +} from '../../utils/index.js'; import { createConnection } from '../../../core/connection/factory.js'; /** @@ -343,7 +350,7 @@ export function ChangeNextScreen({ params }: ScreenProps): ReactElement { // Running if (step === 'running') { - const progressValue = progress.total > 0 ? progress.current / progress.total : 0; + const progressValue = progressPercentage(progress.current, progress.total); return ( diff --git a/src/tui/screens/change/ChangeRevertScreen.tsx b/src/tui/screens/change/ChangeRevertScreen.tsx index e2f04eb8..57fa262e 100644 --- a/src/tui/screens/change/ChangeRevertScreen.tsx +++ b/src/tui/screens/change/ChangeRevertScreen.tsx @@ -37,6 +37,7 @@ import { getErrorMessage, loadChangesWithStatus, createChangeManager, isConfigGuarded, + progressPercentage, } from '../../utils/index.js'; import { createConnection } from '../../../core/connection/factory.js'; @@ -276,7 +277,7 @@ export function ChangeRevertScreen({ params }: ScreenProps): ReactElement { // Reverting if (step === 'reverting') { - const progressValue = fileProgress.total > 0 ? fileProgress.current / fileProgress.total : 0; + const progressValue = progressPercentage(fileProgress.current, fileProgress.total); return ( diff --git a/src/tui/screens/change/ChangeRewindScreen.tsx b/src/tui/screens/change/ChangeRewindScreen.tsx index 990f1aa0..7aa4cad6 100644 --- a/src/tui/screens/change/ChangeRewindScreen.tsx +++ b/src/tui/screens/change/ChangeRewindScreen.tsx @@ -35,7 +35,14 @@ import { } from '../../components/index.js'; import { checkConfigPolicy } from '../../../core/policy/index.js'; import { useChangeProgress, useAsyncEffect } from '../../hooks/index.js'; -import { getErrorMessage, loadChangesWithStatus, buildAppliedChangeList, createChangeManager, isConfigGuarded } from '../../utils/index.js'; +import { + getErrorMessage, + loadChangesWithStatus, + buildAppliedChangeList, + createChangeManager, + isConfigGuarded, + progressPercentage, +} from '../../utils/index.js'; import { createConnection } from '../../../core/connection/factory.js'; /** @@ -411,7 +418,7 @@ export function ChangeRewindScreen({ params }: ScreenProps): ReactElement { // Running if (step === 'running') { - const progressValue = progress.total > 0 ? progress.current / progress.total : 0; + const progressValue = progressPercentage(progress.current, progress.total); return ( diff --git a/src/tui/screens/change/ChangeRunScreen.tsx b/src/tui/screens/change/ChangeRunScreen.tsx index ed21f54b..90aaf5bf 100644 --- a/src/tui/screens/change/ChangeRunScreen.tsx +++ b/src/tui/screens/change/ChangeRunScreen.tsx @@ -37,6 +37,7 @@ import { getErrorMessage, loadChangesWithStatus, createChangeManager, isConfigGuarded, + progressPercentage, } from '../../utils/index.js'; import { validateChangeContent } from '../../../core/change/validation.js'; import { createConnection } from '../../../core/connection/factory.js'; @@ -272,7 +273,7 @@ export function ChangeRunScreen({ params }: ScreenProps): ReactElement { // Running if (step === 'running') { - const progressValue = fileProgress.total > 0 ? fileProgress.current / fileProgress.total : 0; + const progressValue = progressPercentage(fileProgress.current, fileProgress.total); return ( diff --git a/src/tui/screens/db/DbTransferScreen.tsx b/src/tui/screens/db/DbTransferScreen.tsx index a0643d06..0071d51f 100644 --- a/src/tui/screens/db/DbTransferScreen.tsx +++ b/src/tui/screens/db/DbTransferScreen.tsx @@ -34,7 +34,7 @@ import { attempt } from '@logosdx/utils'; import type { ReactElement } from 'react'; import type { ScreenProps } from '../../types.js'; -import { getErrorMessage } from '../../utils/index.js'; +import { getErrorMessage, progressPercentage } from '../../utils/index.js'; import { useRouter } from '../../router.js'; import { useFocusScope } from '../../focus.js'; @@ -1258,13 +1258,8 @@ export function DbTransferScreen({ params: _params }: ScreenProps): ReactElement const modeLabel = transferMode === 'export' ? 'Exporting' : transferMode === 'import' ? 'Importing' : 'Transferring'; const titleLabel = transferMode === 'export' ? 'Export' : transferMode === 'import' ? 'Import' : 'Data Transfer'; - const tableProgress = progress.tableCount > 0 - ? progress.tablesCompleted / progress.tableCount - : 0; - - const rowProgress = progress.currentRowsTotal > 0 - ? progress.currentRowsTransferred / progress.currentRowsTotal - : 0; + const tableProgress = progressPercentage(progress.tablesCompleted, progress.tableCount); + const rowProgress = progressPercentage(progress.currentRowsTransferred, progress.currentRowsTotal); return ( diff --git a/src/tui/screens/run/RunBuildScreen.tsx b/src/tui/screens/run/RunBuildScreen.tsx index fb6ebf71..07c2b2f2 100644 --- a/src/tui/screens/run/RunBuildScreen.tsx +++ b/src/tui/screens/run/RunBuildScreen.tsx @@ -31,7 +31,7 @@ import { getEffectiveBuildPaths } from '../../../core/settings/rules.js'; import { discoverFiles, runBuild } from '../../../core/runner/index.js'; import { filterFilesByPaths, findUnmatchedIncludePatterns } from '../../../core/shared/index.js'; import { checkConfigPolicy } from '../../../core/policy/index.js'; -import { getErrorMessage, resolveScreenIdentity, buildRunContext, withScreenConnection } from '../../utils/index.js'; +import { getErrorMessage, resolveScreenIdentity, buildRunContext, withScreenConnection, progressPercentage } from '../../utils/index.js'; import { attempt } from '@logosdx/utils'; type Phase = 'loading' | 'confirm' | 'running' | 'complete' | 'error'; @@ -353,7 +353,7 @@ export function RunBuildScreen({ params: _params }: ScreenProps): ReactElement { if (phase === 'running') { const processed = progress.filesRun + progress.filesSkipped + progress.filesFailed + progress.filesDryRun; - const progressValue = files.length > 0 ? processed / files.length : 0; + const progressValue = progressPercentage(processed, files.length); return ( diff --git a/src/tui/screens/run/RunDirScreen.tsx b/src/tui/screens/run/RunDirScreen.tsx index ff4ba830..5a271f13 100644 --- a/src/tui/screens/run/RunDirScreen.tsx +++ b/src/tui/screens/run/RunDirScreen.tsx @@ -23,7 +23,7 @@ import { Panel, Spinner, Confirm, SelectList, FilePicker, KeyHandler, useToast } import { useRunProgress, useAsyncEffect, modeBannerRows } from '../../hooks/index.js'; import { discoverFiles, runFiles, checkFilesStatus } from '../../../core/runner/index.js'; import type { FilesStatusResult } from '../../../core/runner/index.js'; -import { getErrorMessage, resolveScreenIdentity, buildRunContext, withScreenConnection } from '../../utils/index.js'; +import { getErrorMessage, resolveScreenIdentity, buildRunContext, withScreenConnection, progressPercentage } from '../../utils/index.js'; import { useConnection } from '../../hooks/index.js'; import { attempt } from '@logosdx/utils'; @@ -688,7 +688,7 @@ export function RunDirScreen({ params }: ScreenProps): ReactElement { if (phase === 'running') { const processed = progress.filesRun + progress.filesSkipped + progress.filesFailed + progress.filesDryRun; - const progressValue = fileCount > 0 ? processed / fileCount : 0; + const progressValue = progressPercentage(processed, fileCount); return ( diff --git a/src/tui/screens/run/RunExecScreen.tsx b/src/tui/screens/run/RunExecScreen.tsx index 73942359..835f8b2a 100644 --- a/src/tui/screens/run/RunExecScreen.tsx +++ b/src/tui/screens/run/RunExecScreen.tsx @@ -28,7 +28,7 @@ import { useSettings, useGlobalModes, useAppContext } from '../../app-context.js import { Panel, Spinner, SelectList, type SelectListItem, Confirm, KeyHandler, useToast } from '../../components/index.js'; import { useRunProgress, useAsyncEffect, modeBannerRows } from '../../hooks/index.js'; import { discoverFiles, runFiles } from '../../../core/runner/index.js'; -import { getErrorMessage, resolveScreenIdentity, buildRunContext, withScreenConnection } from '../../utils/index.js'; +import { getErrorMessage, resolveScreenIdentity, buildRunContext, withScreenConnection, progressPercentage } from '../../utils/index.js'; import { attempt } from '@logosdx/utils'; type Phase = 'loading' | 'picker' | 'confirm' | 'running' | 'complete' | 'error'; @@ -316,7 +316,7 @@ export function RunExecScreen({ params: _params }: ScreenProps): ReactElement { if (phase === 'running') { const processed = progress.filesRun + progress.filesSkipped + progress.filesFailed + progress.filesDryRun; - const progressValue = selectedFiles.size > 0 ? processed / selectedFiles.size : 0; + const progressValue = progressPercentage(processed, selectedFiles.size); return ( diff --git a/src/tui/utils/index.ts b/src/tui/utils/index.ts index 278642c8..128cb7ff 100644 --- a/src/tui/utils/index.ts +++ b/src/tui/utils/index.ts @@ -8,6 +8,7 @@ export { resolveScreenIdentity } from './identity.js'; export { createChangeManager, type CreateChangeManagerOptions } from './change-context.js'; export { buildRunContext, type BuildRunContextOptions } from './run-context.js'; export { withScreenConnection, STOPPED_WAITING_MESSAGE } from './connection.js'; +export { progressPercentage } from './progress.js'; export { loadChangesWithStatus, buildPendingChangeList, diff --git a/src/tui/utils/progress.ts b/src/tui/utils/progress.ts new file mode 100644 index 00000000..a3a1e28e --- /dev/null +++ b/src/tui/utils/progress.ts @@ -0,0 +1,16 @@ +/** + * Convert completed work into the 0–100 value expected by Ink's ProgressBar. + * + * Returns zero until a positive total is known and clamps over-counted event + * streams so rendering remains within the component contract. + * + * @example + * const value = progressPercentage(3, 4); // 75 + */ +export function progressPercentage(completed: number, total: number): number { + + if (total <= 0) return 0; + + return Math.min(100, Math.max(0, (completed / total) * 100)); + +} diff --git a/tests/cli/utils/progress.test.ts b/tests/cli/utils/progress.test.ts new file mode 100644 index 00000000..3edd39e3 --- /dev/null +++ b/tests/cli/utils/progress.test.ts @@ -0,0 +1,21 @@ +import { describe, expect, it } from 'bun:test'; + +import { progressPercentage } from '../../../src/tui/utils/progress.js'; + +describe('tui progress: progressPercentage', () => { + + it('should convert completed work to the ProgressBar percentage scale', () => { + + expect(progressPercentage(1, 2)).toBe(50); + + }); + + it('should keep unknown and over-counted progress within component bounds', () => { + + expect(progressPercentage(4, 0)).toBe(0); + expect(progressPercentage(-1, 4)).toBe(0); + expect(progressPercentage(5, 4)).toBe(100); + + }); + +});