fix: paginate Explorer CRE load and batch-hydrate all_cres#931
Conversation
Summary by CodeRabbit
WalkthroughBackend replaces per-CRE SQL hydration with batch hydration; API enforces a 100-item page cap. Frontend scopes DataProvider to Explorer, implements paginated incremental loading with IndexedDB cache, infinite scroll, and full-data preload for graph views. Tests and route wiring updated. ChangesN+1 Elimination and Paginated Explorer
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed due to a network error. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
5ce2efc to
fe29d55
Compare
Fix production H12 timeouts from GET /all_cres?per_page=1000 by batching N+1 link hydration in the DB layer, capping per_page at 100, scoping DataProvider to Explorer routes with incremental page loads, and using ensureFullExplorerData for graph views. Closes #930. Related: #847, #848. Co-authored-by: Cursor <cursoragent@cursor.com>
fe29d55 to
ad73bc8
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@application/frontend/src/pages/Explorer/visuals/circles/circles.tsx`:
- Around line 24-27: ensureFullExplorerData() is being called inside useEffect
without handling rejections so any pagination/full-load failure is ignored and
LoadingAndErrorIndicator gets error={null}; update the useEffect(s) that call
ensureFullExplorerData (the ones around the top useEffect and the block
referenced at lines 454-457) to await or chain the returned promise and catch
errors, storing the caught error in component state (e.g., setFullLoadError) and
pass that state into LoadingAndErrorIndicator's error prop; alternatively wrap
the call in try/catch inside an async IIFE or add .catch(err =>
setFullLoadError(err)) to ensure failures surface to the UI.
In `@application/frontend/src/providers/DataProvider.tsx`:
- Around line 165-169: The early-return in loadPage uses dataStoreRef.current
when loadingPageRef.current is set, which returns a stale snapshot instead of
the active fetch; change loadPage to return the in-flight promise for the
requested page (i.e., make loadingPageRef/current hold or reference the active
Promise and return that) so concurrent callers await the same fetch; apply the
same fix to the similar logic referenced around ensureFullExplorerData and the
block covering lines ~217-240 so all concurrent callers receive the active page
Promise rather than the stale dataStoreRef snapshot.
- Around line 165-203: The loadPage function currently swallows request errors
by using try/finally and falling back to an empty tree; change it to
try/catch/finally so you capture any axios error, set or propagate an explicit
error state (e.g., call a setter like setDataLoadError or rethrow) before the
finally block clears loading, and avoid updating dataStoreRef/current state with
an empty result on failure; specifically update the logic inside loadPage
(references: loadPage, loadingPageRef, setIsLoadingMore, dataStoreRef,
setDataStore, persistCache, rebuildDataTree) to only merge and persist on
successful fetch and to expose the error to the caller/UI, and apply the same
fix to the other similar loader function with identical behavior elsewhere in
this file.
- Around line 103-115: The buildTree recursion is mutating the same keyPath
array (pushed at line with keyPath.push(selfKey)), causing sibling calls to see
descendants as already visited; fix by not mutating the shared array—create a
new path for each recursion (e.g., clone keyPath or use keyPath.concat(selfKey))
and pass that new array into the recursive buildTree(x.document, store,
newKeyPath) calls so each branch has its own visited set; update references in
the function where keyPath is extended and where buildTree is invoked.
In `@application/frontend/src/routes.tsx`:
- Around line 115-127: ROUTES() currently calls withExplorerLayout(...) inline
for Explorer, ExplorerCircles, and ExplorerForceGraph which creates new
component identities on each call and causes remounts (resetting DataProvider
state); fix by creating and exporting/declaring persistent wrapped components
once (e.g., ExplorerWithLayout, ExplorerCirclesWithLayout,
ExplorerForceGraphWithLayout = withExplorerLayout(...)) outside of ROUTES(),
then update the route objects to reference those pre-wrapped components instead
of calling withExplorerLayout inside ROUTES().
In `@application/frontend/www/index.html`:
- Line 1: Update the viewport meta tag in index.html to allow pinch-zoom: remove
the user-scalable=no and maximum-scale=1 parameters from the meta element (the
<meta name="viewport" ...> in the head) so it reads something like
width=device-width, initial-scale=1 (you can keep minimum-scale=1 if desired) to
restore browser zoom for accessibility.
In `@application/tests/db_test.py`:
- Around line 2532-2534: The parity assertion is order-sensitive because
paginated_cre.todict() and collection.get_CREs(...)[0].todict() can have links
in different orders; update the test loop over paginated_cres to canonicalize
both CRE representations before asserting equality: fetch expected =
collection.get_CREs(external_id=paginated_cre.id)[0], convert both
expected.todict() and paginated_cre.todict() to comparable forms by normalizing
link lists (e.g., sort links or replace link lists with sets) and then assert
equality on the normalized dicts so order differences in links won't cause flaky
failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: abd50bfd-4872-4d56-85a5-1235209d2a6b
📒 Files selected for processing (15)
application/database/db.pyapplication/frontend/src/App.tsxapplication/frontend/src/const.tsapplication/frontend/src/pages/Explorer/ExplorerLayout.tsxapplication/frontend/src/pages/Explorer/explorer.tsxapplication/frontend/src/pages/Explorer/visuals/circles/circles.tsxapplication/frontend/src/pages/Explorer/visuals/force-graph/forceGraph.tsxapplication/frontend/src/providers/DataProvider.tsxapplication/frontend/src/routes.tsxapplication/frontend/www/bundle.jsapplication/frontend/www/bundle.js.LICENSE.txtapplication/frontend/www/index.htmlapplication/tests/db_test.pyapplication/tests/web_main_test.pyapplication/web/web_main.py
Fix buildTree sibling keyPath mutation, serialize loadPage via promise chain with exposed dataLoadError, hoist Explorer layout wrappers, surface load failures in graph views, restore viewport zoom, and harden pagination link parity test. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@application/frontend/www/index.html`:
- Line 1: The HTML document is missing the root <html> element; add an opening
<html> tag immediately after the <!doctype html> declaration and a closing
</html> tag immediately after the existing </body> to properly wrap the <head>,
<body>, and the <div id="mount"> content and ensure valid HTML5 structure.
- Line 1: Update the protocol-relative CDN URL used for the Semantic UI
stylesheet so it explicitly uses HTTPS: locate the <link rel="stylesheet"
href="//cdn.jsdelivr.net/npm/semantic-ui@2.4.1/dist/semantic.min.css"/> entry in
the HTML and change the href to begin with "https://cdn.jsdelivr.net/npm/..." to
ensure a secure, explicit connection.
- Line 1: The document is missing a lang attribute on the root <html> element;
update the top-level HTML element so it includes a language declaration (for
example lang="en")—i.e., ensure the document starts with <!doctype html>
followed by an <html lang="en"> that wraps the existing <head> and <body> (and
corresponding closing </html>), or set the appropriate language value if
different.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: f95b762e-b152-4c31-8bce-ea89f1d7e2d4
📒 Files selected for processing (9)
application/frontend/src/index.htmlapplication/frontend/src/pages/Explorer/explorer.tsxapplication/frontend/src/pages/Explorer/visuals/circles/circles.tsxapplication/frontend/src/pages/Explorer/visuals/force-graph/forceGraph.tsxapplication/frontend/src/providers/DataProvider.tsxapplication/frontend/src/routes.tsxapplication/frontend/www/bundle.jsapplication/frontend/www/index.htmlapplication/tests/db_test.py
✅ Files skipped from review due to trivial changes (1)
- application/frontend/src/index.html
🚧 Files skipped from review as they are similar to previous changes (5)
- application/frontend/src/routes.tsx
- application/frontend/src/pages/Explorer/visuals/force-graph/forceGraph.tsx
- application/frontend/src/pages/Explorer/explorer.tsx
- application/tests/db_test.py
- application/frontend/src/providers/DataProvider.tsx
Summary
Fixes production H12 timeouts caused by
GET /rest/v1/all_cres?page=1&per_page=1000taking ~28–30s (N+1 link hydration + global frontend fetch on every page load).get_CREs/ pagination paths (~4–5 queries per page vs N+1). Capper_pageat 100 on/rest/v1/all_cres.DataProviderfrom globalAppto Explorer-onlyExplorerLayout. Tree view keeps same UX with scroll-to-load viaIntersectionObserver. Force graph and circles callensureFullExplorerData()for full dataset.DataProviderfor paginated fetch (20 CREs/page), IndexedDB cache v2, idle prefetch, and legacy full-store cache compatibility.Closes #930. Related: #847, #848.
Performance (local, 522 CREs,
standards_cache.sqlite)per_page=1000per_page=20page 1/all_cres/all_crescallsTest plan
make test— 279 tests, 0 failuresblack .— passednpx prettier --checkon frontend — passedmake frontend/ webpack build — passed/all_cres), 50× concurrent paginated API (all 200)/explorertree renders;/explorer/force_graphpopulates dropdowns/all_cresin Heroku logsNote:
make mypyhas ~1052 pre-existing repo errors onmain; not introduced by this branch.Made with Cursor