fix(upgrade): delta apply progress bar shows percentage only (no GB scare) (#1355)

## What

The delta-upgrade apply progress bar's byte total sums every hop's
`newSize`, so for multi-hop chains it can far exceed the final binary —
e.g. the user sees `Applying 3 patch(es) [████░░░░] 1.5 GB / 3.1 GB` for
what ends up being a 310 MB install. A scary inflated number that ticks
away without ever closing the gap.

Switch the apply bar to render **percentage only** (no byte counter).
The pre-apply phases (`download`, `read`) keep the byte counter since
their totals are honest sizes.

## Why

Mirror of the Lore fix at
[BYK/loreai#1519](BYK/loreai#1519)
(and follow-up #1522). Same code path — binpatch emits byte events with
summed `newSize` across hops; the consumer bar formats them.

## How

- **`packages/cli/src/lib/progress.ts`** — `makeByteProgress` gains
`format: 'pct'` (default `'bytes'`). When set, the render is `${label}
[${bar}] ${pct}%` (no `/` separator, no byte counts). Indeterminate byte
counters (`totalBytes: null`) ignore the flag — they always show the
live byte total.
- **`packages/cli/src/lib/delta-upgrade.ts`** — apply phase passes `{
format: 'pct' }`. Pre-apply phases default to `'bytes'`. The
phase-change branch was refactored to keep lint complexity ≤ 15.
- **API churn**: the 4th positional arg (`nowMs`) moves into a
`MakeByteProgressOptions` bag (`{ format?, nowMs? }`). This keeps the
helper under the lint `useMaxParams: 4` budget; existing callers that
passed `nowMs` (tests only) are updated in the same commit. The other
call site in `upgrade.ts` (`streamDecompressToFile`) doesn't use `nowMs`
and is unaffected — it falls through `options = {}` default.

## Tests

- 3 new tests in `packages/cli/test/lib/progress.test.ts`:
- pct-only rendering at 100% (`[████████] 100%`, no
`B`/`KB`/`MB`/`GB`/`TB` substring, no `/`)
- pct ignored for indeterminate (`null` total) counters — still shows
live byte count, no `%`
- Regression assertion: reads `delta-upgrade.ts` source and asserts the
apply-phase `makeByteProgress` call passes `"pct"`.
**Mutation-verified**: reverting the `pct` arg makes the test red.
- All 10 `progress.test.ts` tests pass.
- Full `test/lib` suite: **6540 passed | 9 skipped (6549)**, 308 files.
- `pnpm run lint` clean.
- `pnpm run typecheck` clean.
- `pnpm run check:errors` clean.
- `pnpm run check:deps` clean.

## Out of scope

- Pre-apply (`download`/`read`) byte counters are intentional — they're
honest sizes.
- The `binpatch` library still emits `newSize`-summed totals; the
consumer chooses how to render them (this matches the established
`binpatch` contract — library emits, consumer renders).

Author: BYK

packages/cli/src/lib/delta-upgrade.ts

@@ -404,16 +404,27 @@ function makeProgressHandler(setMessage?: SetMessage): ProgressHandler {
   let phase: string | undefined;
   let previousWritten = 0;
   return (event) => {
-    if (event.type === "bytes") {
-      if (!progress || phase !== event.phase) {
-        phase = event.phase;
-        previousWritten = 0;
-        progress = makeByteProgress(
-          `${event.phase === "apply" ? "Applying" : "Processing"} patch(es)`,
-          event.total,
-          setMessage
-        );
-      }
+    if (
+      event.type === "bytes" &&
+      (progress === undefined || phase !== event.phase)
+    ) {
+      // New phase: spin up a fresh bar. The apply phase totals bytes across
+      // every hop's `newSize`, which for multi-hop chains far exceeds the
+      // final binary (e.g. 930 MB shown for a 310 MB install). Switch to
+      // percent-only rendering so users see a sane progress fraction rather
+      // than a scary inflated byte count. Pre-apply ("download"/"read")
+      // phases still show bytes since their totals are honest sizes.
+      phase = event.phase;
+      previousWritten = 0;
+      const isApply = event.phase === "apply";
+      progress = makeByteProgress(
+        `${isApply ? "Applying" : "Processing"} patch(es)`,
+        event.total,
+        setMessage,
+        { format: isApply ? "pct" : "bytes" }
+      );
+    }
+    if (event.type === "bytes" && progress) {
       progress.onProgress(event.written - previousWritten);
       previousWritten = event.written;
     } else if (event.type === "done") {

packages/cli/src/lib/progress.ts

@@ -21,6 +21,19 @@ import { formatBytes } from "./formatters/numbers.js";
 /** Callback that sets the surrounding spinner's message text. */
 export type SetMessage = (message: string) => void;
 
+/**
+ * How a determinate bar renders its progress fraction.
+ *
+ * - `"bytes"` (default) renders the accumulated/total byte count alongside the
+ *   bar — useful when the total is a meaningful size the user can reason about
+ *   (e.g. a full-binary download).
+ * - `"pct"` renders only the percentage — useful when the byte total is
+ *   misleading (e.g. a multi-hop patch chain whose summed `newSize` far
+ *   exceeds the final binary, so "1.5 GB / 3.1 GB" would scare the user
+ *   about an install that's actually 310 MB).
+ */
+export type ByteProgressFormat = "bytes" | "pct";
+
 export type ByteProgress = {
   /** Report `bytes` additional bytes processed since the last call. */
   onProgress: (bytes: number) => void;
@@ -37,6 +50,24 @@ function renderBar(frac: number, width = BAR_WIDTH): string {
   return "█".repeat(filled) + "░".repeat(width - filled);
 }
 
+/**
+ * Options for {@link makeByteProgress} beyond the first three required args.
+ *
+ * Kept as a separate options bag so the helper stays under the lint max-params
+ * budget (`useMaxParams: 4`) — four-position call sites don't change.
+ */
+export type MakeByteProgressOptions = {
+  /**
+   * Determinate render format. `"bytes"` (default) shows the accumulated/total
+   * byte count; `"pct"` shows only the percentage (no byte counter). Ignored
+   * when `totalBytes` is null — indeterminate byte counters always show the
+   * live byte total.
+   */
+  format?: ByteProgressFormat;
+  /** Injectable clock for tests. Defaults to `Date.now`. */
+  nowMs?: () => number;
+};
+
 /**
  * Create a byte-progress reporter that feeds a spinner `setMessage` callback.
  *
@@ -46,23 +77,27 @@ function renderBar(frac: number, width = BAR_WIDTH): string {
  * @param setMessage - Spinner message setter (from `withProgress`). When
  *   undefined (JSON mode / non-TTY / no surrounding spinner), this is a no-op
  *   so nothing is drawn.
- * @param nowMs - Injectable clock for tests.
+ * @param options - Optional render tweaks (format, injectable clock).
  */
 export function makeByteProgress(
   label: string,
   totalBytes: number | null,
-  setMessage?: SetMessage,
-  nowMs: () => number = Date.now
+  setMessage: SetMessage | undefined,
+  options: MakeByteProgressOptions = {}
 ): ByteProgress {
+  const { format = "bytes", nowMs = Date.now } = options;
   let written = 0;
   let lastEmit = 0;
 
-  const format = (): string => {
+  const render = (): string => {
     if (totalBytes === null || totalBytes <= 0) {
       return `${label} ${formatBytes(written)}`;
     }
     const frac = Math.min(written / totalBytes, 1);
     const pct = Math.round(frac * 100);
+    if (format === "pct") {
+      return `${label} [${renderBar(frac)}] ${pct}%`;
+    }
     return (
       `${label} [${renderBar(frac)}] ` +
       `${formatBytes(written)} / ${formatBytes(totalBytes)} (${pct}%)`
@@ -72,7 +107,7 @@ export function makeByteProgress(
   const emit = (): void => {
     // Cosmetic only — a formatting or callback failure must never propagate.
     try {
-      setMessage?.(format());
+      setMessage?.(render());
     } catch {
       // ignore — progress is cosmetic
     }

packages/cli/test/lib/progress.test.ts

@@ -8,9 +8,14 @@
  *
  * Coverage: determinate vs indeterminate formatting, byte accumulation,
  * full-bar clamping, update throttling, a final un-throttled done() emit, the
- * no-op path when no setMessage is provided, and the never-throws contract.
+ * no-op path when no setMessage is provided, the never-throws contract, and
+ * the `format: "pct"` mode that suppresses the inflated byte counter for
+ * multi-hop patch chains.
  */
 
+import { readFileSync } from "node:fs";
+import { join } from "node:path";
+
 import { describe, expect, test, vi } from "vitest";
 
 import { makeByteProgress } from "../../src/lib/progress.js";
@@ -23,7 +28,9 @@ describe("makeByteProgress", () => {
       "Applying 3 patch(es)",
       1000,
       (m) => msgs.push(m),
-      () => now
+      {
+        nowMs: () => now,
+      }
     );
     p.onProgress(500); // first call: now=0, lastEmit=0 → throttled (0-0 < 100)
     now = 200;
@@ -38,12 +45,9 @@ describe("makeByteProgress", () => {
   test("formats an indeterminate byte counter when total is null", () => {
     const msgs: string[] = [];
     let now = 0;
-    const p = makeByteProgress(
-      "Downloading",
-      null,
-      (m) => msgs.push(m),
-      () => now
-    );
+    const p = makeByteProgress("Downloading", null, (m) => msgs.push(m), {
+      nowMs: () => now,
+    });
     now = 200;
     p.onProgress(2048);
     const last = msgs.at(-1) ?? "";
@@ -55,12 +59,9 @@ describe("makeByteProgress", () => {
   test("accumulates bytes across calls and clamps the bar at full", () => {
     const msgs: string[] = [];
     let now = 0;
-    const p = makeByteProgress(
-      "Applying",
-      100,
-      (m) => msgs.push(m),
-      () => now
-    );
+    const p = makeByteProgress("Applying", 100, (m) => msgs.push(m), {
+      nowMs: () => now,
+    });
     p.onProgress(50);
     now = 500;
     p.onProgress(9000); // way over total → clamp to 100%
@@ -73,9 +74,9 @@ describe("makeByteProgress", () => {
   test("throttles updates so a fast byte stream doesn't spam the spinner", () => {
     const set = vi.fn();
     const now = 1000;
-    const p = makeByteProgress("Applying", 1000, set, () => now);
+    const p = makeByteProgress("Applying", 1000, set, { nowMs: () => now });
     // Many calls within the same throttle window → at most one emit.
-    for (let i = 0; i < 50; i++) {
+    for (let i = 0; i < 50; i += 1) {
       p.onProgress(10);
     }
     expect(set.mock.calls.length).toBeLessThanOrEqual(1);
@@ -84,7 +85,7 @@ describe("makeByteProgress", () => {
   test("done() emits a final, un-throttled message reflecting the total", () => {
     const set = vi.fn();
     const now = 0;
-    const p = makeByteProgress("Applying", 100, set, () => now);
+    const p = makeByteProgress("Applying", 100, set, { nowMs: () => now });
     p.onProgress(100); // throttled (now-0 < 100), no emit yet
     set.mockClear();
     p.done(); // must emit regardless of throttle
@@ -95,7 +96,7 @@ describe("makeByteProgress", () => {
   test("is a no-op when no setMessage is provided (JSON/non-TTY)", () => {
     // Nothing to assert beyond: it must not throw and must still track bytes
     // so a later done() with a setMessage-less reporter is harmless.
-    const p = makeByteProgress("Applying", 100);
+    const p = makeByteProgress("Applying", 100, undefined);
     expect(() => {
       p.onProgress(50);
       p.done();
@@ -109,11 +110,120 @@ describe("makeByteProgress", () => {
       () => {
         throw new Error("boom");
       },
-      () => 1000
+      { nowMs: () => 1000 }
     );
     expect(() => {
       p.onProgress(100);
       p.done();
     }).not.toThrow();
   });
+
+  test("renders percentage only when format='pct' (apply bar suppresses GB scare)", () => {
+    // Multi-hop chains sum newSize across hops, so event.total can far
+    // exceed the final binary size (e.g. 930 MB for a 3-hop 310 MB chain).
+    // The apply bar should show only percentage in that case so users
+    // don't see "applied 1.5 GB / 3.1 GB" for what ends up being a
+    // 310 MB install.
+    const msgs: string[] = [];
+    let now = 0;
+    const p = makeByteProgress(
+      "Applying 3 patch(es)",
+      930 * 1024 * 1024,
+      (m) => msgs.push(m),
+      { format: "pct", nowMs: () => now }
+    );
+    now = 200;
+    p.onProgress(310 * 1024 * 1024); // 33%
+    now = 400;
+    p.onProgress(310 * 1024 * 1024); // 66%
+    now = 600;
+    p.onProgress(310 * 1024 * 1024); // 100%
+    p.done();
+
+    const last = msgs.at(-1) ?? "";
+    expect(last).toContain("Applying 3 patch(es)");
+    expect(last).toMatch(/\[█+░*\] 100%/);
+    // No byte count in pct mode
+    expect(last).not.toMatch(/\d+\s*(B|KB|MB|GB|TB)/);
+    expect(last).not.toContain("/");
+  });
+
+  test("format='pct' is ignored for indeterminate (null total) byte counters", () => {
+    // Indeterminate bars have no meaningful percentage — they always show
+    // the live byte total regardless of the format option.
+    const msgs: string[] = [];
+    let now = 0;
+    const p = makeByteProgress("Downloading", null, (m) => msgs.push(m), {
+      format: "pct",
+      nowMs: () => now,
+    });
+    now = 200;
+    p.onProgress(2048);
+    const last = msgs.at(-1) ?? "";
+    expect(last).toContain("Downloading");
+    expect(last).toContain("2.0 KB");
+    expect(last).not.toContain("%");
+  });
+
+  test("apply bar call site passes 'pct' format (regression: multi-hop GB scare)", () => {
+    // Regression: delta-upgrade.ts apply bar previously called
+    // makeByteProgress(label, total, setMessage) without passing format, so
+    // the bar fell back to bytes mode and the "pct only" UX never applied.
+    // Reads the source to assert the apply-phase call site actually wires
+    // "pct", so a future regression to bytes mode is caught even if the
+    // helper itself still defaults to "bytes".
+    const src = readFileSync(
+      join(__dirname, "../../src/lib/delta-upgrade.ts"),
+      "utf8"
+    );
+    // Find the makeByteProgress call inside the apply bar branch by scanning
+    // for balanced parentheses, so nested calls like
+    // makeByteProgress("label", computeTotal()) still match. Regex-based
+    // extraction breaks on nested parens, which a real refactor could
+    // easily introduce.
+    const matches = extractCalls(src, "makeByteProgress");
+    const applyCall = matches.find((m) => m.includes("isApply"));
+    expect(applyCall).toBeDefined();
+    expect(applyCall).toMatch(/["']pct["']/);
+  });
 });
+
+/**
+ * Walk a source string and return every `name(`...`)` call as a string slice.
+ * Uses a balanced-parentheses scan rather than a regex so nested calls like
+ * `makeByteProgress("label", computeTotal())` still extract cleanly.
+ */
+function extractCalls(source: string, name: string): string[] {
+  const out: string[] = [];
+  let i = 0;
+  while (i < source.length) {
+    const found = source.indexOf(name, i);
+    if (found === -1) {
+      break;
+    }
+    const open = source.indexOf("(", found);
+    if (open === -1) {
+      break;
+    }
+    let depth = 1;
+    let j = open + 1;
+    while (j < source.length && depth > 0) {
+      const ch = source[j];
+      if (ch === "(") {
+        depth += 1;
+      } else if (ch === ")") {
+        depth -= 1;
+      }
+      if (depth > 0) {
+        j += 1;
+      }
+    }
+    if (depth === 0) {
+      out.push(source.slice(found, j + 1));
+      i = j + 1;
+    } else {
+      break; // unbalanced, stop scanning
+    }
+  }
+  return out;
+}