Skip to content

refactor(miner-ui): extract shared client-side table pagination hook and pager - #8492

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
rsnetworkinginc:fix-shared-table-pagination-8306
Jul 24, 2026
Merged

refactor(miner-ui): extract shared client-side table pagination hook and pager#8492
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
rsnetworkinginc:fix-shared-table-pagination-8306

Conversation

@rsnetworkinginc

Copy link
Copy Markdown
Contributor

What

Client-side table pagination in apps/loopover-miner-ui was independently duplicated three different ways across five routes. This PR extracts a single shared hook + pager component and converts all five call sites to consume it, so the logic lives in exactly one place.

The three prior shapes:

  • run-history.tsx and ranked-candidates.tsx each inlined the 5-line page/pageCount/isPaginated/safePage/visible computation and hand-rolled the full Pagination/PaginationContent/… JSX directly in the component.
  • ledgers.tsx and portfolio.tsx each defined their own local TablePagination component (byte-identical between the two files) and repeated the page-state computation twice per file (once per table).
  • attempts.tsx had already generalized the page-state into a local usePagedRows<T> hook plus a local TablePagination — the closest-to-canonical shape — but it stayed private to that one route.

Change

  • Extract attempts.tsx's usePagedRows shape into a shared src/lib/paged-rows.ts module, generalized to usePagedRows<T>(rows: T[], pageSize?: number) with the existing PAGE_SIZE = 20 as the default. The returned page is clamped via Math.min(page, pageCount - 1), so it stays valid when rows shrinks below the current page's start index.
  • Extract the shared presentational pager into src/components/table-pagination.tsx, built on the existing @loopover/ui-kit pagination primitives (the primitive itself is left unchanged, per the separate aria-disabled styling issue).
  • Convert all five routes (attempts, run-history, ranked-candidates, ledgers, portfolio) to import and use the shared hook + component, removing every local/inline duplicate. Net: +233 / −344 lines across 9 files.

This is a pure refactor — behavior is preserved exactly: PAGE_SIZE = 20, page clamping, aria-disabled on the boundary Previous/Next controls, and "paginate only past the first page" all unchanged. The five routes' existing integration-style tests continue to pass against the shared implementation.

Tests

Adds dedicated unit tests for the extracted code so it carries direct coverage rather than relying solely on the routes' integration tests:

  • src/lib/paged-rows.test.ts — empty rows, rows within a single page (the exclusive > pageSize boundary), rows spanning multiple pages with setPage, page-clamping when rows shrink below the current page, and the PAGE_SIZE default.
  • src/components/table-pagination.test.tsx — numbered/Previous/Next rendering, aria-disabled at both boundaries, and the low/high clamping arms of every click handler.

Every added line in all nine changed files has 0 uncovered statements and 0 uncovered branches under vitest --coverage (v8), keeping apps/loopover-miner-ui's local coverage floor intact.

Closes #8306

…SONbored#8306)

The client-side table pagination logic was independently duplicated three
ways across five miner-ui routes: run-history and ranked-candidates inlined
the page-state computation plus the full Pagination JSX, ledgers and
portfolio each defined their own local TablePagination and repeated the
page-state twice per file, and attempts had already generalized it into a
local usePagedRows hook.

Extract attempts.tsx's usePagedRows shape into a shared lib/paged-rows.ts
module and a shared components/table-pagination.tsx built on the existing
@loopover/ui-kit pagination primitives, then convert all five routes to
consume them. Behavior is preserved exactly (PAGE_SIZE = 20 default, page
clamping via Math.min(page, pageCount - 1), aria-disabled on the boundary
controls). Adds dedicated unit tests for the hook (empty, single-page,
multi-page, and page-clamping when rows shrink) and the pager component.
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 24, 2026
@loopover-orb

loopover-orb Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-24 15:40:07 UTC

9 files · 1 AI reviewer · no blockers · readiness 93/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This is a clean, mechanical extraction of a duplicated pagination pattern (5-line page-state computation + hand-rolled Pagination JSX) into a shared `usePagedRows<T>` hook and `TablePagination` component, with all five routes converted to use them. The logic is byte-for-byte preserved (PAGE_SIZE=20, page clamping via `Math.min(page, pageCount-1)`, `aria-disabled` boundaries, paginate-only-past-first-page), and the new hook/component both ship solid unit tests covering empty lists, exact-boundary sizing, multi-page slicing, page-clamping on shrink, and pager click/aria-disabled behavior at both boundaries. I don't see any behavioral drift introduced by the refactor.

Nits — 4 non-blocking
  • The PR description doesn't link an eligible open issue in the top-level summary shown here, though the code comments reference Client-side table pagination logic is independently duplicated 3 ways across 5 miner-ui routes / 6+ call sites #8306 throughout — confirm that issue is real and open before merging per the repo's issue-linkage requirement.
  • The external brief's 'magic number 8306' flags are just issue-number references inside comments/JSDoc, not numeric literals in logic — not an actionable nit.
  • apps/loopover-miner-ui/src/lib/paged-rows.ts: consider exporting `PagedRows<T>` from an index or documenting it alongside `usePagedRows` for discoverability, though this is optional given the small surface.
  • Nice touch keeping `TablePagination` purely presentational and building on the existing `@​loopover/ui-kit` primitives rather than duplicating them a fourth time.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #8306
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 35 registered-repo PR(s), 14 merged, 3 issue(s).
Contributor context ✅ Confirmed Gittensor contributor rsnetworkinginc; Gittensor profile; 35 PR(s), 3 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The PR extracts usePagedRows<T> to lib/paged-rows.ts and TablePagination to components/table-pagination.tsx, and converts all five routes (attempts, run-history, ranked-candidates, ledgers, portfolio) to use them while removing local/inline duplicates, matching the issue's exact requirements including PAGE_SIZE=20 and clamping behavior. It also adds dedicated unit tests for both the hook (empty, s

Review context
  • Author: rsnetworkinginc
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: TypeScript
  • Official Gittensor activity: 35 PR(s), 3 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Triage stale or unlinked PRs.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

Visual preview
Route Viewport Before (production) After (this PR's preview) Diff
/attempts desktop before /attempts
before /attempts
after /attempts
after /attempts
/attempts mobile before /attempts (mobile)
before /attempts (mobile)
after /attempts (mobile)
after /attempts (mobile)
/ledgers desktop before /ledgers
before /ledgers
after /ledgers
after /ledgers
/ledgers mobile before /ledgers (mobile)
before /ledgers (mobile)
after /ledgers (mobile)
after /ledgers (mobile)

Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy.

Scroll preview
Route Before (production) After (this PR's preview)
/attempts before /attempts (scroll)
before /attempts (scroll)
after /attempts (scroll)
after /attempts (scroll)
/ledgers before /ledgers (scroll)
before /ledgers (scroll)
after /ledgers (scroll)
after /ledgers (scroll)

A short scroll-through clip (desktop) — click either thumbnail to open the full animation. Evidence for scroll-linked behavior a single screenshot can't show.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit cc142df into JSONbored:main Jul 24, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Client-side table pagination logic is independently duplicated 3 ways across 5 miner-ui routes / 6+ call sites

1 participant