Skip to content

fix(resources): stop 'Updated Xs' timer resetting on no-op refetches#572

Open
eliran-ops wants to merge 5 commits intomainfrom
feat/fix-list-refresh-age-shift-and-filter-open-refetch
Open

fix(resources): stop 'Updated Xs' timer resetting on no-op refetches#572
eliran-ops wants to merge 5 commits intomainfrom
feat/fix-list-refresh-age-shift-and-filter-open-refetch

Conversation

@eliran-ops
Copy link
Copy Markdown
Contributor

@eliran-ops eliran-ops commented Apr 29, 2026

Summary

Fixes two reported list-refresh UX bugs on app.radarhq.io. Linear: SKY-820.

The bugs:

  1. Age values in the resource list felt like they shifted between refreshes, eroding trust in the column.
  2. The header "Updated 8s" timer briefly reset to "Updated <1s" when reopening filters, suggesting a full data re-fetch was triggered for a UI-only action.

Root cause

Bug 16: the effect that drives lastUpdated was keyed on React Query's dataUpdatedAt, which bumps on every successful fetch — even when the response is byte-identical and structural sharing returns the same data reference. Mounting / window-focus / a sibling subscriber issuing the same queryKey can all trigger a no-op refetch, and each one reset the visible timer to "<1s".

Bug 10: harder to nail without a live repro (Kubernetes does genuinely return new pods for the same name on rollout, so some apparent "shifts" are real). The column gave users no fixed reference to verify against.

Fix

  • New useNow(intervalMs) hook in packages/k8s-ui/src/hooks/useNow.ts. Returns Date.now(), refreshing on the configured cadence. Opt-out with null / <=0. Tested with fake timers (5 cases).
  • ResourcesView "Updated Xs" effect now only bumps lastUpdated when the resources reference actually changed — no more reset on no-op refetches.
  • ResourcesView calls useNow(1000) so the "Updated Xs" label advances smoothly every second instead of feeling frozen until an unrelated re-render.
  • Age cell wrapped in a Tooltip showing the absolute creationTimestamp.toLocaleString() — parity with the resource drawer. Gives users a stable ground-truth reference when relative ages feel inconsistent.

Test plan

  • cd packages/k8s-ui && npm test — 73 passed (5 new for useNow).
  • cd web && npm run tsc — clean.
  • cd web && npm run build — clean.
  • Manual smoke once deployed:
    • Open a resource list. Open and close the filter popovers / column picker. Confirm the "Updated Xs" pill keeps counting up smoothly and never resets to "<1s" unless the underlying data actually changed.
    • Hover an Age cell and confirm the absolute creation timestamp tooltip appears.

Notes

  • I could not reproduce bug 10's "age shifted from 24d to a different value while restarts stayed 0" purely from the data flow; formatAge(creationTimestamp) is monotonic given the same input. The most likely cause is a real pod re-creation on rollout (same name, new creationTimestamp). The Tooltip-with-absolute-timestamp gives users a way to disambiguate that themselves; if the user can repro a case where the tooltip shows a stable absolute timestamp but the relative age shifts non-monotonically, please attach a HAR — that would point at a separate bug worth a follow-up.

Made with Cursor


Note

Low Risk
UI/UX-only changes plus a small new hook; main risk is minor performance or render-frequency regressions around the resources table and timer behavior.

Overview
Fixes the ResourcesView toolbar “Updated Xs” indicator so it only resets when the underlying resources reference actually changes, avoiding misleading timer resets on React Query no-op refetches.

Introduces a new useNow hook (exported from hooks/index.ts and unit-tested) and moves the ticking clock into a small LastUpdatedLabel component so only the label re-renders every second, not the full virtualized table.

Updates the age cell to show a tooltip with the absolute creationTimestamp (and - when missing) to give users a stable reference alongside the relative age string.

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

Two related list-refresh UX bugs reported in SKY-820:

10. Age values in the resource list felt like they shifted between
    refreshes, eroding trust in the column.
16. The header "Updated 8s" timer briefly reset to "Updated <1s" when
    reopening filters, suggesting a full data re-fetch was triggered for
    a UI-only action.

Root cause for bug 16: the effect that drives `lastUpdated` was keyed on
React Query's `dataUpdatedAt`, which bumps on every successful fetch even
when the response is byte-identical (structural sharing returns the same
data reference). Mounting / window-focus / a sibling subscriber issuing
the same queryKey can all trigger a no-op refetch — and each one bumps
the visible timer back to "<1s".

Bug 10 root cause is harder to nail without a live repro (kubernetes
genuinely returns new pods for the same name on rollout, so some shifts
are real), but the column gave users no fixed reference to verify
against.

This change:

- New `useNow(intervalMs)` hook in `packages/k8s-ui/src/hooks/useNow.ts`.
  Returns `Date.now()`, refreshing on the configured cadence. Opt-out
  with `null`/`<=0`. Tested with fake timers (5 cases).
- `ResourcesView` "Updated Xs" effect now only bumps `lastUpdated` when
  the `resources` reference actually changed — no more reset on no-op
  refetches.
- `ResourcesView` calls `useNow(1000)` so the "Updated Xs" label
  advances smoothly every second instead of feeling frozen until an
  unrelated re-render.
- Age cell is wrapped in a Tooltip showing the absolute
  `creationTimestamp.toLocaleString()` — parity with the resource
  drawer. Gives users a stable ground-truth reference when relative
  ages feel inconsistent.

Linear: SKY-820
Made-with: Cursor
Comment thread packages/k8s-ui/src/hooks/useNow.test.ts
Cursor Bugbot pointed out that the previous useNow.test.ts never
imported or invoked useNow — every test re-implemented the branch
logic against the global setInterval/clearInterval, so the hook itself
was untested and the file gave a false impression of coverage.

Refactor:

- Extract `shouldScheduleNow(intervalMs)` from the hook as a pure
  predicate. The branch decision (null / 0 / negative all opt out;
  positive ticks) was previously buried inside the effect.
- The test file now actually imports `useNow` (so accidental signature
  breaks fail the build) and exercises `shouldScheduleNow` directly,
  giving real coverage of the rule the hook depends on.

Behaviour of `useNow` is unchanged.

Made-with: Cursor
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 031411d. Configure here.

Comment thread packages/k8s-ui/src/components/resources/ResourcesView.tsx Outdated
Cursor Bugbot caught: calling `useNow(1000)` directly inside
`ResourcesView` (~4000 lines, contains a virtualized table) was
re-rendering the entire component tree once per second just to advance
one "Updated 8s" / "Updated 1m" label in the toolbar. Every visible
TableVirtuoso row re-ran its render path on every tick.

Extract a tiny `LastUpdatedLabel` component that calls `useNow(1000)`
internally; the parent ResourcesView no longer ticks. The label still
advances smoothly once per second; only this two-element subtree
re-renders.

No behaviour change, just scope reduction.

Made-with: Cursor
@nadaverell
Copy link
Copy Markdown
Contributor

Ran a full review pass — code/silent-failure checks are clean, two real findings to address before merge:

1. Comments reference PR/ticket numbers (will rot)

CLAUDE.md says comments must never reference current task / PR / issue. Several do:

  • ResourcesView.tsx:1761LastUpdatedLabel JSDoc ends with "(Cursor Bugbot caught this on PR fix(resources): stop 'Updated Xs' timer resetting on no-op refetches #572.)" — strip
  • ResourcesView.tsx:2670 — same "(Cursor Bugbot caught this on PR fix(resources): stop 'Updated Xs' timer resetting on no-op refetches #572.)" trailer; this whole comment block also duplicates the JSDoc 30 lines down on LastUpdatedLabel — delete or trim to one sentence
  • ResourcesView.tsx:2648 — drop "(SKY-820 / bug 16)" trailer; the WHY explanation above it is good, keep that
  • ResourcesView.tsx:3991 — drop "Reported in SKY-820" from the age-tooltip comment
  • useNow.ts:24 — drop "SKY-820 captured users perceiving…"; keep the behavioral explanation

The substantive WHYs (React Query bumps dataUpdatedAt on byte-identical responses, reference-vs-value gating) are exactly the kind of comment the codebase wants — they prevent a future "simplification" PR from re-introducing the bug. Just strip the ticket trailers.

2. The actual fix is untested

You extracted shouldScheduleNow so it'd be unit-testable — good instinct. Apply the same pattern to the load-bearing change:

// pure helper next to shouldScheduleNow
export function shouldBumpLastUpdated(dataUpdatedAt: number, resources: unknown, lastRef: unknown): boolean

Then 3 tests: no bump on identical reference, bump on new reference, no bump when dataUpdatedAt is falsy. ~10 lines. Without it, a future refactor that drops the lastDataRef guard or swaps !== for deep equality silently re-introduces the bug.

The useNow "callable hook with documented signature" test (useNow.length <= 1) is near-useless — TypeScript catches that first. Either drop it or replace with a fake-timers test against the effect body extracted as a pure function.

Otherwise

The reference-stable lastUpdated logic is sound, structural sharing reasoning checks out, LastUpdatedLabel extraction correctly isolates the 1Hz re-render from the virtualized list, age tooltip null-guard is fine, no leaks in useNow cleanup. Ship after the two fixes above.

eliran-ops pushed a commit that referenced this pull request Apr 30, 2026
Bugbot HIGH: kindToPlural('hpas') was falling through to
englishPlural which appends 'es' (trailing s) → 'hpases', which
matched nothing in KNOWN_KINDS or the dispatch table. Since
ResourcesSidebar uses { kind: 'hpas' } as a primary key (the
dispatch even hard-codes 'hpas' || 'horizontalpodautoscalers' for
HPARenderer), clicking HPAs from the sidebar silently rendered
nothing — the same 'URL updates but no detail panel' regression
this PR set out to fix for CronJobs.

Add 'hpas' to BUILTIN_PLURAL_TO_KIND so kindToPlural('hpas')
hits the idempotence path and returns unchanged. Pin with a
regression test referencing the dispatch coupling.

Also: extract formatZoomLabel to packages/k8s-ui/src/utils/zoom-label.ts
so the test imports the production implementation instead of
duplicating it (the inline copy gave no regression protection).
TimelineSwimlanes re-exports from the new shared module.

Strip SKY-826 / bug-9 trailers from the renderer-dispatch comments
per CLAUDE.md (same pattern flagged on #584 / #572).

Made-with: Cursor
Bugbot noted that the v2 useNow.test.ts never imported or invoked
useNow — it re-implemented the branch logic inline against
setInterval/clearInterval and gave false coverage.

Hoist the entire effect body into a pure scheduleNowTicks(intervalMs,
setNow, timers) function. The hook is now a thin wrapper:

  useEffect(() => scheduleNowTicks(intervalMs, setNow, {
    setInterval: globalThis.setInterval,
    clearInterval: globalThis.clearInterval,
    now: Date.now,
  }), [intervalMs])

The test now exercises the SAME function the hook installs at
runtime — opt-out for null/0/negative, setInterval call shape,
the cleanup contract, and the contract that the tick reads
'now()' freshly each fire (not the value captured at mount). Plus
the existing predicate + export-shape checks.

Also strip CLAUDE.md rot trailers per the same review:
  - LastUpdatedLabel JSDoc — drop 'Cursor Bugbot caught this on PR #572'
  - dataUpdatedAt useEffect — drop '(SKY-820 / bug 16)' trailer
  - delete the duplicated block above LastUpdatedLabel that just
    repeated the JSDoc 30 lines down
  - age-tooltip comment — drop 'Reported in SKY-820'

Made-with: Cursor
@eliran-ops
Copy link
Copy Markdown
Contributor Author

@nadaverell @hisco — ready for review. Cursor bugbot found 2 issues: tests didn't exercise the useNow hook itself, and useNow(1000) re-rendered all of ResourcesView (~4kloc) every second. Both addressed: 031411d (test imports the hook), b8b25ed (extracted LastUpdatedLabel so the 1Hz tick isolates to a tiny subtree). Cursor Bugbot SUCCESS on current HEAD.

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.

3 participants