feat: oracle tab pagination + sortable columns#116
Conversation
- Fix query ordering: was ascending (oldest-first), now descending (newest-first) - Add offset/pagination to ORACLE_SNAPSHOTS query (25 rows per page) - Add ORACLE_SNAPSHOTS_COUNT query for total row count - New Pagination component with first/prev/next/last controls - Sortable columns: Time, Oracle OK, Price, Price Diff, Reporters, Block - Remove client-side .reverse() (was showing oldest 25 records at top) - Page resets to 1 on sort change or search change
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Stale comment
This PR adds Oracle tab pagination and introduces sortable Oracle table columns, plus a pagination component and updated Oracle queries. The feature direction is good, but there are correctness gaps that currently make sorting/pagination behavior inconsistent and can produce misleading data views.
ui-dashboard/src/app/pool/[poolId]/page.tsx🔴 Critical
page.tsx:1225: Sorting is applied client-side after fetching a timestamp-paginated slice (limit/offset), so non-timestamp sorts are page-local, not global. This yields incorrect ordering semantics across pages.page.tsx:1304: Charts are driven bysortedRows, so table sort interactions (e.g. by price/reporters) reorder chart points and break time-series interpretation.🟡 Important
page.tsx:1203:setPage(1)is called during render when query changes. Render should be pure; this needs to move to an effect.page.tsx:1181: Oracle page size is hardcoded to25while the shared tablimitUI remains visible, so Oracle ignores the global limit control and behaves inconsistently with other tabs.page.tsx:1197-1428: New pagination/sort logic has no test coverage for page transitions, sort toggling, and chart stability under sort changes.Final verdict
🚫 Needs changes
Sent by Cursor Automation: PR Review
| // Reverse once to get newest-first order, then filter | ||
| const orderedRows = useMemo(() => [...rows].reverse(), [rows]); | ||
| // Client-side sort of the current page | ||
| const sortedRows = useMemo(() => { |
There was a problem hiding this comment.
This sorts only the currently fetched page. Because fetch pagination is still timestamp desc (limit + offset), sorting by oraclePrice, numReporters, etc. is not globally correct across the dataset.
Concrete failure mode: page 1 and page 2 are partitioned by timestamp first, then each page is locally re-ordered by the selected column, so users never get a true full-table sort.
Please move sorting into the GraphQL query (order_by driven by sort state) so offset applies to the chosen sort order.
| const arrow = (col: OracleSortCol) => | ||
| sortCol === col ? (sortDir === "asc" ? " ↑" : " ↓") : ""; | ||
|
|
||
| // Charts always use all fetched rows (current page) for context |
There was a problem hiding this comment.
chartRows is tied to table sort state. After sorting by a non-time column, both oracle charts are no longer chronological, which misrepresents time-series data.
Charts should be based on a stable timestamp ordering independent of table sort (e.g. always timestamp asc/desc), while table rows can follow user-selected sort.
|
|
||
| // Reset to page 1 when search changes (derived from query length changes) | ||
| const prevQueryRef = React.useRef(query); | ||
| if (prevQueryRef.current !== query) { |
There was a problem hiding this comment.
This calls setPage during render. Even though guarded, it's still a render-phase state update and can cause extra render churn / strict-mode surprises.
Please move this reset into useEffect(() => setPage(1), [query]) (with guard if needed).
| | "numReporters" | ||
| | "blockNumber"; | ||
|
|
||
| const ORACLE_PAGE_SIZE = 25; |
There was a problem hiding this comment.
Oracle now has a fixed page size, but the shared LimitSelect in the tab header is still visible. In Oracle tab, changing limit in URL/UI has no effect, which is inconsistent with other tabs.
Either wire Oracle pagination to the same limit state or hide/disable the limit control for Oracle.
…solation Fixes all 5 issues found in 3-pass union review: 🔴 Sort is now server-side via $orderBy variable passed to Hasura — column ordering is global across the full dataset, not just the current page 🔴 Search fetches up to 500 rows at offset 0 when active — filtering is global, never limited to the current page window 🔴 Charts receive timestamp-ascending data derived from raw rows, decoupled from table sort state — oracle history is never reordered/truncated by the table controls 🟡 setPage reset moved from useEffect to a wrapped handleSearchChange callback — no more state mutation during render 🟡 countError surfaced: Pagination falls back to rows.length and shows an amber warning when the count query fails
There was a problem hiding this comment.
Stale comment
Adds Oracle-tab server-side sorting/pagination and introduces a reusable pagination control, plus a new aggregate count query for total rows.
ui-dashboard/src/app/pool/[poolId]/page.tsx
- 🔴 CRITICAL (
1225-1234,1246-1250,1296-1305): Table sorting/pagination state is driving the same dataset used by both charts. Sorting byoraclePrice/priceDifference(or paging away from page 1) changes which rows are fetched, so charts become a partial/arbitrary slice instead of the expected time-series view. This is incorrect behavior and can mislead operators.- 🟡 IMPORTANT (
1207-1210,1270-1278,1411-1416): New page/sort state is local-only and not reflected in URL, while this screen already uses URL-backed state for tab/search/limit. This creates inconsistent behavior (no deep-link/share/back-forward for sort/page).- 🟡 IMPORTANT (
1207-1278,1410-1422): New pagination/sort/count logic shipped without coverage. Existing tests for this page do not assertORACLE_SNAPSHOTS_COUNT, sort toggling, page reset on sort/search, or pagination rendering transitions.
ui-dashboard/src/components/pagination.tsx
- 🟢 SUGGESTION (
34-63): Addtype="button"on all controls to avoid accidental form submission if this component is ever rendered inside a<form>.Final verdict
🚫 Needs changes
Sent by Cursor Automation: PR Review
| const isSearching = query.length > 0; | ||
| const fetchLimit = isSearching ? ORACLE_SEARCH_LIMIT : ORACLE_PAGE_SIZE; | ||
| const fetchOffset = isSearching ? 0 : (page - 1) * ORACLE_PAGE_SIZE; | ||
| const orderBy = buildOrderBy(sortCol, sortDir); |
There was a problem hiding this comment.
orderBy (and below, offset/limit) is shared by table + chart consumers. Because of that, clicking sortable headers (e.g. oraclePrice) changes the fetched record set itself, not just table row order. The chart then renders a non-representative subset.
Recommend splitting data paths:
- Keep one query for table (
orderBy+offset+ page size). - Use a separate chart query pinned to timestamp ordering (or derive from a dedicated timestamp-ordered fetch), independent of table sort/page state.
| </tbody> | ||
| </Table> | ||
| {!isSearching && ( | ||
| <Pagination |
There was a problem hiding this comment.
Pagination is currently local component state only (page), unlike existing URL-backed tab/search/limit state on this page. This makes sort/page non-shareable and breaks expected back/forward behavior.
Please persist page/sort params in URL (same pattern used by setURL/setTabSearch) so state is durable and consistent across tabs/features.
| }, [orderedRows, query, sym0]); | ||
| }, [rows, query, sym0]); | ||
|
|
||
| function toggleSort(col: OracleSortCol) { |
There was a problem hiding this comment.
This introduces substantial new behavior (toggleSort, page reset, count-query-based pagination) but there are no tests covering these paths.
Please add assertions for at least:
- sort toggle direction + default direction per column
- page reset to 1 on search/sort changes
- count query failure fallback behavior
- pagination visibility when searching vs not searching
🔴 buildOrderBy must return array: Hasura $orderBy is [OracleSnapshot_order_by!]! not a plain object — was causing runtime type mismatch 🟡 orderBy memoized with useMemo([sortCol, sortDir]) so useGQL doesn't see a new object reference on every render and fire spurious refetches 🟡 countError fallback preserves last known total via ref so pagination stays visible when count query fails temporarily 🟡 Charts now use a dedicated ORACLE_SNAPSHOTS_CHART query (200 most recent rows, always timestamp-asc) — fully decoupled from table pagination and sort state 🟡 Pagination: guard pageSize<=0 and total<=0 to prevent Infinity totalPages from divide-by-zero 💡 toggleSort wrapped in useCallback for consistency with handleSearchChange 💡 Dead !isLoading guard removed from empty-rows check 💡 countError warning only shown when total is also unknown (not just on error)
There was a problem hiding this comment.
Stale comment
This PR adds server-side sorting and pagination to the Oracle tab, plus dedicated chart/count queries. The direction is good, but there are correctness regressions in pagination behavior that need to be fixed before merge.
ui-dashboard/src/app/pool/[poolId]/page.tsx
🔴 CRITICAL
ui-dashboard/src/app/pool/[poolId]/page.tsx:1187-ui-dashboard/src/app/pool/[poolId]/page.tsx:1192+ui-dashboard/src/app/pool/[poolId]/page.tsx:1233-ui-dashboard/src/app/pool/[poolId]/page.tsx:1238: offset pagination is paired withorder_bythat can be a single non-unique field (e.g.oracleOk,numReporters,priceDifference). That creates unstable page boundaries (duplicates/missing rows across pages and refreshes). You need a deterministic secondary/tertiary sort key (e.g.timestamp+id) appended to every sort mode.🟡 IMPORTANT
ui-dashboard/src/app/pool/[poolId]/page.tsx:1430: on count-query failure,totalfalls back torows.length(page-local, max 25). This collapses paginator visibility and can strand users on partial data with no way to reach older pages. The fallback must preserve navigability (e.g. retain last known total, or derivehasNextfrom page size and keep pagination controls usable).
ui-dashboard/src/app/pool/[poolId]/page.tsx:1208-ui-dashboard/src/app/pool/[poolId]/page.tsx:1293andui-dashboard/src/app/pool/[poolId]/page.tsx:1328-ui-dashboard/src/app/pool/[poolId]/page.tsx:1438: substantial new logic (sort toggling, page transitions, count-error fallback) landed without corresponding tests. Current pool page tests don’t assert these paths, so regressions in pagination correctness won’t be caught.Final verdict: 🚫 Needs changes.
Sent by Cursor Automation: PR Review
| col: OracleSortCol, | ||
| dir: "asc" | "desc", | ||
| ): Array<Partial<Record<OracleSortCol, "asc" | "desc">>> { | ||
| return [{ [col]: dir }]; |
There was a problem hiding this comment.
Using only the selected column in order_by is unsafe with offset pagination when values tie (which is common for oracleOk, numReporters, etc.). Please append deterministic tiebreakers to every sort mode (for example primary col + timestamp + id) so rows do not jump/duplicate across pages.
| <Pagination | ||
| page={page} | ||
| pageSize={ORACLE_PAGE_SIZE} | ||
| total={countError ? rows.length : total} |
There was a problem hiding this comment.
rows.length here is page-local, not dataset-total. When count fails, this can hide pagination and make older pages unreachable even though data exists. Please change fallback logic to preserve navigation (e.g. keep last known total, or switch to has-next pagination semantics on count error).
| const { network } = useNetwork(); | ||
| const query = normalizeSearch(search); | ||
|
|
||
| const [page, setPage] = React.useState(1); |
There was a problem hiding this comment.
This introduces new stateful behavior (pagination/sorting) but there are no assertions covering it in the pool page tests. Please add tests for sort toggling, page transitions, and count-error fallback so these regressions are caught automatically.
- Stable pagination: buildOrderBy appends {timestamp:desc, id:asc} as
tiebreakers when sorting by non-unique fields (oracleOk, numReporters,
priceDifference, blockNumber) — prevents duplicate/missing rows across
page boundaries on cursor-based offset pagination
- Pagination buttons: add type='button' to prevent accidental form submit
if component is ever rendered inside a <form>
- Hide global limit selector on oracle tab: tab manages its own page size
(ORACLE_PAGE_SIZE=25), showing the shared limit control was misleading
- Remove 'Reporters' column (always 1 on Mento) - Remove 'Block' column - Link Source badge and Time to the transaction on the block explorer - Human-readable Price Diff: shows X.X% of threshold (tooltip shows raw bps) - Human-readable Threshold: shows X.XX% (tooltip shows raw bps) - type='button' on sort header buttons (mirrors pagination fix) - Remove numReporters/blockNumber from sort type and search fields - Add txHash to OracleSnapshot type and both oracle queries - Search now includes txHash
There was a problem hiding this comment.
Stale comment
This PR adds server-driven sorting/pagination to the Oracle tab, introduces a reusable pagination component, and splits Oracle data into table/chart/count queries. The direction is good, but there are correctness regressions in pagination/search behavior that should be fixed before merge.
Changed files reviewed:
- ui-dashboard/src/app/pool/[poolId]/page.tsx
- ui-dashboard/src/components/pagination.tsx
- ui-dashboard/src/lib/queries.ts
Findings:
- ui-dashboard/src/app/pool/[poolId]/page.tsx
- 🔴
L1199:timestampsort path drops deterministic tie-breakers, so offset pagination can skip/duplicate rows when timestamps collide.- 🟡
L1187andL1436: search mode hard-caps data at 500 and disables pagination, so search is incomplete with no truncation signal.- 🟡
L1440: count-query failure collapses pagination torows.length(usually one page), hiding additional data during transient count errors.- 🟡
L1205: no tests added for new oracle pagination/sorting/count-failure flows.Final verdict: 🚫 Needs changes
Sent by Cursor Automation: PR Review
| dir: "asc" | "desc", | ||
| ): Array<Partial<Record<string, "asc" | "desc">>> { | ||
| const primary: Partial<Record<string, "asc" | "desc">> = { [col]: dir }; | ||
| if (col === "timestamp") return [primary]; |
There was a problem hiding this comment.
timestamp sort currently returns only [primary]. With offset pagination, this is non-deterministic when multiple rows share the same timestamp (common for on-chain events), which can cause duplicate/missing rows across pages.
Please always include a stable unique tie-breaker for timestamp sorting too (e.g. append { id: "asc" }).
|
|
||
| const ORACLE_PAGE_SIZE = 25; | ||
| // When search is active, fetch up to this many rows so filtering is global | ||
| const ORACLE_SEARCH_LIMIT = 500; |
There was a problem hiding this comment.
ORACLE_SEARCH_LIMIT = 500 combined with suppressed pagination in search mode means search is silently incomplete once a pool has >500 snapshots.
This can produce false negatives ("No oracle snapshots match your search.") even when matches exist outside the first 500 rows. Please paginate search server-side or explicitly handle/signal truncation.
| <Pagination | ||
| page={page} | ||
| pageSize={ORACLE_PAGE_SIZE} | ||
| total={countError ? rows.length : total} |
There was a problem hiding this comment.
On countError, this falls back to rows.length instead of the total value you already preserve via lastKnownTotalRef. That collapses pagination to a single page during transient count failures and can hide older data.
Please use the preserved total fallback here and keep a warning whenever count is unavailable.
| } | ||
|
|
||
| function OracleTab({ | ||
| poolId, |
There was a problem hiding this comment.
Large behavior change (sorting, pagination, count fallback, search-mode fetch behavior), but no corresponding oracle-tab regression tests were added.
Please add coverage in page.test.tsx for: sort toggle correctness, page navigation, count error fallback behavior, and search behavior when result set exceeds the fetch cap.
There was a problem hiding this comment.
Stale comment
This PR adds Oracle tab pagination/sorting and decouples chart/table queries, but it currently introduces a runtime-breaking query/schema mismatch and a couple of pagination/search correctness gaps.
ui-dashboard/src/lib/queries.ts
- 🔴 Critical
L204,L229: BothORACLE_SNAPSHOTSandORACLE_SNAPSHOTS_CHARTnow requesttxHashonOracleSnapshot, but the in-repo indexer schema (indexer-envio/schema.graphql,OracleSnapshot) does not definetxHash. This is a breaking API/schema mismatch: Hasura will reject these queries at runtime, and the Oracle tab will fail to load.
ui-dashboard/src/app/pool/[poolId]/page.tsx
- 🟡 Important
L1185,L1229-L1233: The code/comments claim search is global, but search mode only fetches the first500rows (ORACLE_SEARCH_LIMIT). Any matches beyond that window are silently unsearchable.L1251-L1255,L1464: You computelastKnownTotalRefas a count-error fallback, butPaginationis fedrows.lengthwhencountErroris set. On count failures, this can collapse/hide pagination and leave users stranded on non-first pages.L1216-L1218: Sort/page state is local component state only. Unlike the rest of this page (tab/search/limit), it is not URL-backed, so back/forward/share loses Oracle table state.
ui-dashboard/src/app/pool/[poolId]/page.test.tsx
- 🟡 Important
L247and test blockL314-L422: tests were only minimally updated for fixture shape. There is no coverage for new Oracle pagination, server sorting toggles, count-error fallback behavior, or the 500-row search cap path.Final verdict: 🚫 Needs changes
Sent by Cursor Automation: PR Review
txHash was never added to the OracleSnapshot entity in the indexer schema. Querying it causes a Hasura runtime error and breaks the oracle tab entirely. Revert txHash from: - OracleSnapshot type (types.ts) - ORACLE_SNAPSHOTS and ORACLE_SNAPSHOTS_CHART queries (queries.ts) - OracleTab row rendering and search fields (page.tsx) - Test fixture (page.test.tsx) Source/Time tx linking is a follow-up: requires indexer schema change + handler update + redeploy (see planned issue in WORK.md).
There was a problem hiding this comment.
Stale comment
This PR adds oracle-tab server-side sorting, dedicated pagination controls, and separate chart/count queries, while removing the global tab limit selector for that tab.
ui-dashboard/src/app/pool/[poolId]/page.tsx
🟡 IMPORTANT
- Misleading/broken search behavior (
L1279-L1285,L1331): the UI says users can search by "tx hash", but the filter terms do not include tx hash (andOracleSnapshotcurrently has notxHash). This is user-visible incorrect behavior.
- Fix: either remove "tx hash" from the placeholder or add
txHashto schema/query/type/filter terms.- "Global" search is silently truncated (
L1184-L1185,L1229-L1233): search mode hard-caps fetches to 500 rows. Matches older than the newest 500 snapshots are never returned, but the code/comments describe search as global.
- Fix: either paginate server-side while searching, or clearly communicate truncation in UI.
- State regression: pagination/sort not URL-backed (
L1216-L1218,L1289-L1299,L1435-L1439):page,sortCol, andsortDirare local-only state. Reload/back/share loses table state, inconsistent with existing URL-driven tab/search/limit behavior.
- Fix: persist
oraclePage,oracleSort,oracleDirin query params.- Missing non-happy-path handling for chart query (
L1264-L1271): chart data fetch errors are ignored; charts can render empty with no explanation even when table data exists.
- Fix: surface chart fetch error or fallback charts to table rows.
- No test coverage for new oracle behaviors (
L1176-L1450): this introduces substantial pagination/sorting/count-error logic, but there are no corresponding assertions for sort toggling, page transitions, URL persistence, or count-query failure fallback.🟢 SUGGESTIONS
buildOrderBy()currently returnsPartial<Record<string, ...>>(L1192-L1199), which is typo-prone. Prefer a stricterOracleSnapshotOrderBytype so invalid field keys fail at compile time.Final verdict: 🚫 Needs changes.
Sent by Cursor Automation: PR Review
Every oracle event now carries its transaction hash. This was always the right design; txHash was just missing from the schema. Indexer: - schema.graphql: add txHash: String! to OracleSnapshot entity - sortedOracles.ts: populate from event.transaction.hash in both OracleReported and MedianUpdated handlers - fpmm.ts: populate in update_reserves and rebalanced snapshot constructors (FPMM handlers also create OracleSnapshots on reserve/rebalance events) UI: - types.ts: restore txHash on OracleSnapshot type - queries.ts: restore txHash in ORACLE_SNAPSHOTS and ORACLE_SNAPSHOTS_CHART - page.tsx: restore tx links on Source badge and Time column - page.test.tsx: add txHash to test fixture
There was a problem hiding this comment.
Stale comment
This PR adds
txHashtoOracleSnapshotend-to-end (indexer + dashboard types/queries) and introduces Oracle tab table pagination + sortable columns with explorer links. The direction is good, but there are correctness and test coverage gaps that should be fixed before merge.
ui-dashboard/src/app/pool/[poolId]/page.tsx
- 🟡 IMPORTANT (
1185,1232-1233,1273-1288): Search is silently incomplete. The code comments claim search is global, but it only filters the first500rows (ORACLE_SEARCH_LIMIT) after sorting. For pools with more than 500 oracle snapshots, valid matches outside that window are never shown.- 🟡 IMPORTANT (
1251-1255,1464): Count fallback is implemented but then bypassed.lastKnownTotalRef/totalare computed to preserve pagination on count errors, but pagination receivesrows.lengthwhencountErroris set. A transient aggregate-query failure collapses pagination to one page and hides navigable data.- 🟡 IMPORTANT (
1216-1218): New Oracle pagination/sort state is local-only, while the rest of this page uses URL state (tab,limit,*Q). This breaks shareability and browser navigation consistency for the new behavior.- 🟢 SUGGESTION (
1343-1378): Sortable headers are buttons, but the table headers do not exposearia-sort. Addaria-sorton the corresponding<th>so assistive tech can announce current sort state.
ui-dashboard/src/app/pool/[poolId]/page.test.tsx
- 🟡 IMPORTANT (
268-286): The test suite does not cover the new Oracle pagination/sorting/count-fallback behavior. TheuseGQLmock map only wiresORACLE_SNAPSHOTS, notORACLE_SNAPSHOTS_COUNT/ORACLE_SNAPSHOTS_CHART, so the new logic paths are effectively untested.Final verdict: 🚫 Needs changes
Sent by Cursor Automation: PR Review
Address the actionable parts of Cursor review 4044389616: - add aria-sort to sortable oracle headers - make shared Th forward th props so accessibility attrs reach the DOM - expand pool page tests to mock ORACLE_SNAPSHOTS_COUNT and ORACLE_SNAPSHOTS_CHART - add coverage for pagination metadata, query wiring, and oracle sort semantics
Add regression coverage for the new OracleSnapshot txHash field in the indexer test suite so schema/entity drift gets caught immediately.
There was a problem hiding this comment.
Stale comment
This PR adds tx-hash-backed oracle snapshot links and refactors the oracle tab to use server-side sorting plus pagination with dedicated chart/count queries. The direction is good, but there are correctness/performance gaps in non-happy paths that should be fixed before merge.
ui-dashboard/src/app/pool/[poolId]/page.tsx
- 🟡 IMPORTANT:
L1243-L1245fetcheslimit = totalfor any active search. On large pools this becomes an unbounded full-table pull per search commit and can easily regress latency and Hasura load. Add a bounded search page size (or server-side filtered search + paginated results) instead of loading all rows.- 🟡 IMPORTANT:
L1502-L1508handles count-query failures inconsistently. When count fails,totalis set torows.length(current page only), which can hide pagination and silently truncate navigation to later pages; warning text only appears whentotal === 0. This non-happy path should keep pagination semantics stable and always surface a warning when count is unavailable.
ui-dashboard/src/app/pool/[poolId]/page.test.tsx
- 🟡 IMPORTANT: New pagination/sort logic is only partially covered. Current tests validate static metadata and one sort-toggle state, but do not verify page transitions (
offsetchanges) or the count-error fallback behavior. Add interaction tests for next/prev page and count failure to prevent regressions.Final verdict: 🚫 Needs changes
Sent by Cursor Automation: PR Review
| // dataset, not just the current page. Bootstrap with a bounded window before | ||
| // count resolves, then expand to the full row count. | ||
| const isSearching = query.length > 0; | ||
| const searchFetchLimit = total > 0 ? total : ORACLE_SEARCH_BOOTSTRAP_LIMIT; |
There was a problem hiding this comment.
searchFetchLimit = total means an active search can trigger a full snapshot fetch for the pool. This scales poorly as history grows and can create noticeable query/load spikes. Please switch to bounded server-side filtering + pagination (or at minimum cap this limit) instead of requesting the full dataset.
| <Pagination | ||
| page={page} | ||
| pageSize={ORACLE_PAGE_SIZE} | ||
| total={countError ? rows.length : total} |
There was a problem hiding this comment.
When countError is set, using rows.length as total (current page size only) can collapse pagination and make later pages unreachable without any clear signal. This should preserve navigability semantics (or explicitly disable paging with a visible warning) for all count-error states, not only when total === 0.
| expect(html).toContain(">median-feed</a>"); | ||
| }); | ||
|
|
||
| it("loads chart and count oracle queries and renders pagination metadata", () => { |
There was a problem hiding this comment.
Coverage gap: this test asserts rendered pagination metadata, but it does not exercise pagination interactions (onPageChange -> offset update) or the countError fallback path. Please add interaction tests for page navigation and count-query failure behavior.
There was a problem hiding this comment.
Stale comment
This PR adds
txHashtoOracleSnapshotend-to-end (schema, indexer handlers, dashboard types/queries) and introduces sorting + pagination UX for the Oracle tab. The indexing-side changes look correct, but there are two important regressions in the Oracle tab behavior.
ui-dashboard/src/app/pool/[poolId]/page.tsx
- 🟡 URL-as-state regression for new controls (
1217-1219,1293-1302,1499-1504):page,sortCol, andsortDirare kept only in local React state. Refresh, deep-linking, and browser back/forward lose Oracle table state, which breaks the existing URL-backed interaction model used by this page (tab,limit, per-tab search).
- Suggested fix:
// read const page = Number(searchParams.get("oraclePage") ?? "1"); const sortCol = (searchParams.get("oracleSort") as OracleSortCol) ?? "timestamp"; const sortDir = (searchParams.get("oracleDir") as "asc" | "desc") ?? "desc"; // write const p = getCurrentParams(); p.set("oraclePage", String(nextPage)); p.set("oracleSort", nextCol); p.set("oracleDir", nextDir); replaceURL(p);
- 🟡 Count-error fallback can silently truncate navigation (
1230-1237,1502-1509): whenORACLE_SNAPSHOTS_COUNTfails,Pagination.totalfalls back torows.length(current page only). If page 1 returns 25 rows but more exist, controls can disappear without any warning. The warning is only rendered whentotal === 0, so partial/incomplete pagination is silent.
- Suggested fix:
const paginationTotal = countError ? Math.max(lastKnownTotalRef.current, page * ORACLE_PAGE_SIZE + (rows.length === ORACLE_PAGE_SIZE ? 1 : 0)) : total; {countError && ( <p className="px-1 pt-1 text-xs text-amber-400"> Total count unavailable; pagination may be incomplete. </p> )}
ui-dashboard/src/app/pool/[poolId]/page.test.tsx
- 🟡 Coverage gap for new Oracle state/error paths (
415-462): tests cover query wiring and aria attributes, but there is no test proving pagination/sort survive URL round-trip, and no test for count-query failure behavior. Those are the highest-risk paths in this PR and should be covered.Final verdict: 🚫 Needs changes
Sent by Cursor Automation: PR Review
Address Cursor review 4044517666: - cap client-side oracle search fetches at 2,000 rows instead of pulling the entire table when count resolves - keep pagination total stable on count-query failure by using the preserved aggregate total instead of rows.length - always surface the count-warning banner when aggregate loading fails - add regression tests for page transitions, count-error fallback, and capped search behavior
There was a problem hiding this comment.
Stale comment
This PR adds
txHashto oracle snapshots and rewires the oracle tab to use server-side pagination/sorting with separate chart/count queries. The direction is good, but there are blocking compatibility risks and a pagination correctness gap.
indexer-envio/schema.graphql
- 🔴 Critical (
L63):OracleSnapshot.txHashwas added as non-null (String!) with no migration/backfill path. On existing indexed environments, adding a required column to a populated table is a breaking schema change unless data migration/reindex ordering is explicitly handled.
ui-dashboard/src/lib/queries.ts
- 🔴 Critical (
L204,L229): both oracle queries now always requesttxHash. Any environment still on the previous indexer schema (notxHashcolumn) will hard-fail the oracle tab query at runtime.
ui-dashboard/src/app/pool/[poolId]/page.tsx
- 🟡 Important (
L1219-L1221,L1251,L1318-L1321,L1505-L1510):pageis local state and is never clamped/reset when the underlying dataset changes (notably network switch with same route, or count shrinking). A stale high page causesoffsetto overshoot and renders “No oracle snapshots yet” even when snapshots exist.- 🟡 Important (
L1219-L1221,L1505-L1510): pagination/sort state is not URL-backed, unlike existing tab/search/limit state. This breaks shareability and back/forward consistency for the oracle view.
ui-dashboard/src/app/pool/[poolId]/page.test.tsx
- 🟡 Important (
L351-L565): new tests cover happy-path pagination/sort, but there is no regression test for stale-page behavior when dataset/count changes (the bug above). Missing this leaves the non-happy path unguarded.Final verdict: 🚫 Needs changes
Sent by Cursor Automation: PR Review
Codify the lesson from PR #116: - add a repo-level operating rule for cross-layer/stateful data changes - add a dedicated checklist for schema/query/UI/degraded-mode/test review - point root + package AGENTS files at the checklist so agents see it early
There was a problem hiding this comment.
Stale comment
This PR adds Oracle tab server-side pagination/sorting with dedicated chart/count queries, and extends
OracleSnapshotend-to-end withtxHashso source/time can link to explorer tx pages.
ui-dashboard/src/app/pool/[poolId]/page.tsx
- 🟡 IMPORTANT (
~1245-1251,~1512-1521): search silently degrades when count fails before first success. In search mode, fetch is capped toORACLE_SEARCH_BOOTSTRAP_LIMIT(500), but the cap warning is gated bytotal > 2000, and the count-error warning is gated by!isSearching. Result: users can get false "No oracle snapshots match your search" for older rows with no indication search was partial.
- Fix direction: always surface a degraded-mode banner when
isSearching && countError, including the active cap (500), e.g."Could not load total count; search is currently limited to the most recent 500 snapshots.".- 🟡 IMPORTANT (
~1219-1221and surrounding URL state wiring): Oracle page/sort state is local-only while other tab state is URL-backed. This breaks shareability/back-forward consistency for a stateful table and is inconsistent with the rest of this page’s URL-as-state pattern.
indexer-envio/src/handlers/fpmm.ts
- 🟡 IMPORTANT (
~535,~627) +indexer-envio/test/Test.ts: newtxHashwrites were added forUpdateReserves/Rebalancedsnapshots, but tests only added txHash assertions fororacle_reported/oracle_median_updatedpaths (Test.tsaround~924-1073). This leaves two writer paths unguarded for a required schema field (txHash: String!).
- Fix direction: add assertions in existing
UpdateReservesandRebalancedsnapshot test flows to verify persistedOracleSnapshot.txHashequals the triggering event tx hash.Final verdict: 🚫 Needs changes.
Sent by Cursor Automation: PR Review
- indexer: add txHash assertions for UpdateReserves and Rebalanced OracleSnapshot writer paths — the two test paths introduced in the success-path rebalancing state tests now pass specific tx hashes and assert snapshot.txHash matches - ui: add degraded search banner when count query fails before first success (isSearching && countError) — previously the user would see 'No results' with no indication that search was limited to 500 rows
There was a problem hiding this comment.
Stale comment
This PR adds Oracle snapshot
txHashend-to-end (indexer schema/handlers/tests) and significantly upgrades the Oracle tab with server-side sorting, pagination, capped search, and degraded-mode messaging.
ui-dashboard/src/app/pool/[poolId]/page.tsx
- 🔴 CRITICAL (
L1245-L1263,L1513-L1522): capped search is documented/rendered as “most recent 2,000 snapshots,” but the fetch window is actually driven by the active sort (orderBy). If the user changes sort (e.g.Price Diff), search no longer scans the most recent window; it scans the first 2,000 rows of that sort order, which can produce false negatives and misleading warning text.
ui-dashboard/src/app/pool/[poolId]/page.test.tsx
- 🟡 IMPORTANT (
L523-L533): there is no regression test for the capped-search + non-default-sort interaction. Current tests only assert the capped-search behavior under default sort, so the incorrect windowing behavior above is not covered.Final verdict: 🚫 Needs changes.
Sent by Cursor Automation: PR Review
| const fetchLimit = isSearching ? searchFetchLimit : ORACLE_PAGE_SIZE; | ||
| const isSearchCapped = isSearching && total > ORACLE_SEARCH_MAX_LIMIT; | ||
| const fetchOffset = isSearching ? 0 : (page - 1) * ORACLE_PAGE_SIZE; | ||
| const orderBy = useMemo( |
There was a problem hiding this comment.
searchFetchLimit is presented as a recency cap ("most recent ... snapshots"), but search queries still use the interactive orderBy from sortCol/sortDir. That means once sort is changed, the search window is no longer "most recent" and can silently miss matches.
Please either:
- force search-mode fetch ordering to
timestamp desc(and keep table sorting as a separate client-side step for the bounded set), or - change UX copy to explicitly state search is limited to the first N rows in the current sort order.
Right now behavior and user messaging disagree.
| ); | ||
| }); | ||
|
|
||
| it("caps oracle search fetch size and shows a warning for large result sets", () => { |
There was a problem hiding this comment.
Coverage gap: this only validates capped search under default sort. Add a regression that changes sort first (e.g. click Price Diff), then searches with oracleCount > 2000, and asserts the expected fetch orderBy/window semantics.
Without that, the capped-search windowing bug can regress unnoticed.
When search is active, always use timestamp desc so the bounded fetch
window is chronologically consistent with the user-facing warning text
('most recent N snapshots'). Previously, if the user had sorted by e.g.
Price Diff, a search would fetch the lowest-price-diff rows, producing
false negatives and a misleading warning.
Add regression test: oracle search orderBy must never contain priceDifference,
oracleOk, or oraclePrice regardless of active table sort.
There was a problem hiding this comment.
Stale comment
This PR adds
txHashtoOracleSnapshotacross indexer+UI and introduces oracle-tab pagination/sort/search with dedicated chart/count queries.
ui-dashboard/src/app/pool/[poolId]/page.tsx
- 🟡 Important (
1253,1325): page bounds are not clamped when total count drops. A user can be left on an out-of-range page (offsetpoints past data), then hit the misleading empty state ("No oracle snapshots yet") even though snapshots still exist on earlier pages.- 🟡 Important (
1262,1369-1429): during search, backend ordering is forcibly timestamp-desc, but sort controls remain active and updatearia-sort/arrow labels as if sorting is applied. The UI announces one sort state while rendering another.Final verdict
🚫 Needs changes
Sent by Cursor Automation: PR Review
| : ORACLE_SEARCH_BOOTSTRAP_LIMIT; | ||
| const fetchLimit = isSearching ? searchFetchLimit : ORACLE_PAGE_SIZE; | ||
| const isSearchCapped = isSearching && total > ORACLE_SEARCH_MAX_LIMIT; | ||
| const fetchOffset = isSearching ? 0 : (page - 1) * ORACLE_PAGE_SIZE; |
There was a problem hiding this comment.
fetchOffset uses the current page, but page is never clamped when total shrinks.
Repro: user navigates to page 3, then aggregate count drops to <= 25 rows. offset remains 50, query returns empty, and the component later renders the pool-level empty state (No oracle snapshots yet) even though rows still exist on earlier pages.
Please clamp/reset page when total pages change (e.g. setPage((p) => Math.min(p, Math.max(totalPages, 1)))).
| // Search always uses newest-first so the bounded window is chronologically | ||
| // consistent with what the warning text says ("most recent N snapshots") | ||
| const searchOrderBy = useMemo(() => buildOrderBy("timestamp", "desc"), []); | ||
| const orderBy = isSearching ? searchOrderBy : tableOrderBy; |
There was a problem hiding this comment.
When isSearching is true, orderBy is forced to timestamp desc, but header sort controls remain clickable and continue updating aria-sort/arrow state.
That creates a mismatch: the UI claims sorting by Price Diff/Oracle OK etc., while the rendered data stays timestamp-ordered. Either disable sort controls during search, or apply client-side sorting to the searched rows so visual/ARIA sort state matches actual order.
1. Clamp oracle page to valid range when total count changes. rawPage is an internal counter; the derived 'page' is clamped to [1, totalPages] so a stale page index never sends the user to an empty offset past the last row. 2. Suppress sort arrows and aria-sort during search. While search is active, the backend sort is overridden to timestamp desc regardless of what the user previously sorted by. The sort buttons remain clickable (staged for when search clears) but arrows and aria-sort='descending|ascending' are hidden so the UI does not announce a sort that isn't applied to the visible rows. Add regression tests for both behaviors.
There was a problem hiding this comment.
This PR wires OracleSnapshot.txHash through indexer + dashboard and refactors the oracle tab to use server-side ordering with pagination, capped search, and dedicated chart/count queries.
indexer-envio/schema.graphql
- 🔴 CRITICAL (
indexer-envio/schema.graphql:63):OracleSnapshot.txHashwas introduced as non-null (String!) without a migration/backfill strategy for already-indexed environments. This is a schema-breaking change in staggered deployments: if UI/Hasura roll before a full reindex/backfill, oracle queries can fail hard instead of degrading.
ui-dashboard/src/app/pool/[poolId]/page.tsx
- 🟡 IMPORTANT (
ui-dashboard/src/app/pool/[poolId]/page.tsx:1375-1413): the oracle table drops the previously visibleReportersandBlockcolumns entirely. That is a functional regression (loss of observability data) and also conflicts with the PR scope text that says those columns are sortable.
Final verdict: 🚫 Needs changes
Sent by Cursor Automation: PR Review
| rebalanceThreshold: Int! | ||
| source: String! | ||
| blockNumber: BigInt! | ||
| txHash: String! |
There was a problem hiding this comment.
txHash is now required (String!), but this introduces a schema-breaking rollout path unless you guarantee full backfill/reindex before any consumer reads this schema. Please either:
- make this field nullable for a safe rollout window, then enforce non-null after reindex, or
- document/enforce an atomic deploy + reindex sequence so older rows/environments never serve a schema that lacks populated
txHash.
As written, staggered environments can fail hard on oracle reads.
| Price Diff{arrow("priceDifference")} | ||
| </button> | ||
| </Th> | ||
| <Th align="right">Threshold</Th> |
There was a problem hiding this comment.
This refactor removes Reporters and Block from the oracle table output (they were present before), so users lose those data points in the detail view. If the removal is intentional, it should be explicitly called out as a scope change; otherwise this is a regression.
Given the PR description also says those columns are sortable, I think this should be restored (or the scope/docs updated to match reality).
Codify the lesson from PR #116: - add a repo-level operating rule for cross-layer/stateful data changes - add a dedicated checklist for schema/query/UI/degraded-mode/test review - point root + package AGENTS files at the checklist so agents see it early


Problem
The oracle tab was fetching 25 rows ordered oldest-first, then reversing client-side. This meant you were always looking at the 25 oldest oracle events — some from pool deployment 20 days ago — never the recent ones.
Changes
order_by: timestamp desc(newest first) +offsetsupport for paginationORACLE_SNAPSHOTS_COUNTquery to get total row count for pagination controlsPaginationcomponent — first/prev/next/last buttons with page N of M counterlimitURL param from OracleTab; fixed page size is 25 rowsTesting