Home

dev / openkara

publicthedavidweng/OpenKara· sync paused
Code Branches Pull requestsIssuesInsights
main
Home Code PRsIssues

#386 "Download complete" progress bar can reappear without a new download

closed

Opened by dev · yesterday

devopened this issueAuthor· yesterday

Line references are pinned to main at afbd7c45.

Pre-existing on main. Not introduced by #374 — that PR moved this logic out of GlobalProgressBar.tsx into a pure module, reproducing the same semantics. Do not bisect to it. Pre-split proof below.

Where

src/lib/task-progress.ts L223-252:

export function createModelDownloadFlash(
  emit: (visible: boolean) => void,
  flashMs: number = MODEL_DOWNLOAD_FLASH_MS,
): ModelDownloadFlash {
  let previous: ModelBootstrapState | undefined;
  let timers: ReturnType<typeof setTimeout>[] = [];

  const clearTimers = () => { ... };

  return {
    observe(state) {
      if (state === previous) return;

      const downloadSettled = previous === "downloading" && state === "ready";
      previous = state;
      clearTimers();
      if (!downloadSettled) return;      // <- no emit(false)

      timers.push(
        globalThis.setTimeout(() => {
          emit(true);
          timers.push(globalThis.setTimeout(() => emit(false), flashMs));
        }, 0),
      );
    },
    dispose: clearTimers,
  };
}

Consumed by src/hooks/use-active-tasks.ts L16, which feeds deriveActiveTasks.

What goes wrong

observe clears pending timers on every state change but never emits false. Once emit(true) has fired, a state change away from ready cancels the hide timer and leaves the consumer's value stuck at true indefinitely.

The stale true is invisible at first, because deriveActiveTasks gates the task on the current state as well:

// L151
if (modelDownloadCompleteFlash && modelBootstrap?.state === "ready") {

So the bar stays hidden while the state is not ready. But the flag is never cleared, so the next time the state returns to ready the gate opens again and the "download complete" bar reappears — for the full remainder of a timer that was cancelled, i.e. until something else changes the flag. No new download occurred.

Conditions

A completed model download (which sets the flag), followed by the bootstrap state leaving ready within the 2.8 s flash window, followed by it returning to ready later without passing through downloading. Model deletion, a failed re-probe, a variant switch, or any transient bootstrap re-check between those two points will do it. The window is short, which is why this is rare rather than constant.

Pre-existing proof

32ec36c6:src/components/Layout/GlobalProgressBar.tsx L112-140, useModelDownloadCompleteFlash: the effect returns early without setFlash(false) when the transition is not a download settle, and its cleanup only clears timers. Identical semantics.

Fix direction (my view, not the reviewer's)

The suggestion was to track a shown boolean and emit false whenever an active or scheduled flash is interrupted. That works, and it is a small patch.

I would rather remove the state that makes it possible. The flash is a derived fact — "a download settled less than flashMs ago and we are still ready" — and modelling it as an imperative latch plus two timers is what creates a value that can outlive its own timer.

Record the settle timestamp instead of a boolean, and let deriveActiveTasks decide whether the bar is visible from that timestamp, the current state and the clock. Then there is nothing to reset: leaving ready hides the bar because the gate already checks the state, and returning to ready after the window has passed hides it because the timestamp is stale. One timer remains, purely to schedule the re-render at the moment the window expires, and cancelling it can no longer strand a value.

This also fits where the code has already moved: deriveActiveTasks is a pure function, and this is the last piece of that computation still expressed as a mutable side-channel.

If the smaller shown-flag patch is preferred, keep dispose free of emissions so unmount does not update a removed consumer.

Not fixing here

Filed for tracking only.

Sign in to comment.

Linked pull requests

No linked pull requests yet.