Skip to content

Propose plan: apply BFF/server-pagination pattern app-wide - #2268

Open
harry-rhesis wants to merge 2 commits into
mainfrom
docs/bff-pagination-migration-plan
Open

Propose plan: apply BFF/server-pagination pattern app-wide#2268
harry-rhesis wants to merge 2 commits into
mainfrom
docs/bff-pagination-migration-plan

Conversation

@harry-rhesis

Copy link
Copy Markdown
Contributor

Purpose

PR #2267 replaced client-side "fetch everything and paginate/filter in memory"
on the metrics and behaviors directory pages with a server-driven pattern:
a Server Component prefetches the first page through the BFF proxy
(eliminating the first-load spinner), and a client hook owns pagination/
filtering against server-pushed OData queries instead of an in-memory dataset.

This is a docs-only PR proposing how to roll that pattern out to the rest
of the app's directory/list pages (projects, models, tools, tokens, tests,
test sets, endpoints, knowledge, test runs, tasks, experiments, annotations,
traces, team members), based on a full audit of their current fetch/pagination
implementations. No application code changes.

What Changed

  • Added docs/frontend-bff-pagination-migration-plan.md, covering:
    • Background on the prefetchList / usePaginatedList primitives from
      PR Add server-driven pagination and initial fetch for metrics/behaviors #2267 and what they replace.
    • Backend foundations the pattern depends on — explicit QueryBuilder
      eager-loading, the two-phase ("id-then-join") pagination query pattern,
      the X-Total-Count header / PaginatedResponse contract, OData $filter
      support (including any() navigation filters through many-to-many
      relationships), custom non-OData params like metric_scope (JSONB @>)
      and $select, and the automatic tenant-isolation before_compile hook
      that only applies to ORM/Core queries.
    • Goals/non-goals — explicitly does not propose migrating pages
      already on useGridQuery/React Query away from that hook; those just
      need SSR prefetch added, not a rewrite.
    • A full survey of every directory page in
      apps/frontend/src/app/(protected)/, split into:
      • Already migrated (metrics, behaviors).
      • True pagination gaps — pages that still fetch everything or a hardcoded
        cap and filter in memory: projects, models, tools, tokens, explorer.
      • Already server-paginated pages that just need SSR prefetch added:
        endpoints, tests, test sets, knowledge, test runs, tasks, experiments,
        annotations, team members.
      • Special cases deferred for now: traces (custom telemetry query model).
    • Guardrails codifying the bug this exact pattern caused on behaviors:
      switching create/edit/delete to "refetch the current page" silently hid
      newly-created rows when the list was sorted alphabetically and the new
      row didn't land on page 0. The fix there (and the guidance here) is to
      mutate local state directly (sorted insert/update/remove) rather than
      trust a blind refetch, unless sort order guarantees visibility.
    • Suggested execution order (quick SSR-prefetch wins first, then true
      pagination fixes, then heavier grids, then special cases), each as its
      own small PR per repo convention.
    • Open questions — e.g. whether SSR-prefetched data should hydrate
      React Query's cache directly vs. just seed a prop; whether entities
      returning non-standard shapes ({ data, totalCount } / { data, total })
      should get a shape adapter or be normalized to PaginatedResponse;
      backend follow-ups needed for Tokens ($filter support) and Explorer
      (total-count-bearing response shape) before they can adopt the pattern.

Additional Context

Testing

N/A — documentation only. No app code touched.

Scope rolling the metrics/behaviors server-driven pagination pattern
(prefetchList + usePaginatedList from PR #2267) out to the rest of the
app's directory pages, with a survey of every candidate page, the
backend foundations the pattern depends on, and the CRUD guardrails
learned from the behaviors sort-order bug.

@peqy peqy Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Solid plan and the guardrails (CRUD visibility + sorted insert) are particularly useful.

Two doc issues to fix before merging:

  • The referenced prefetchList / usePaginatedList files aren’t present on main yet (broken links unless #2267 is merged first / you use permalinks).
  • The “Survey” link is not resolvable from the repo.


PR #2267 replaced this with two reusable primitives:

- **[`prefetchList`](../apps/frontend/src/utils/server-prefetch.ts)** — a server-side

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Improvement: The doc links to ../apps/frontend/src/utils/server-prefetch.ts / ../apps/frontend/src/hooks/usePaginatedList.ts, but those paths (and prefetchList/usePaginatedList symbols) don’t exist on main right now.

Fix: either link to a permalink in PR #2267 / the commit that introduces them, or add a note that this plan assumes #2267 has merged (and ensure merge order), so we don’t end up with broken links on main.


## 5. Survey of candidate pages

Audit performed via [Survey](fac00d4a-907f-4db2-abec-2a50bb2aee8d) against

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Improvement: “Audit performed via Survey” is a dead reference from the repo’s perspective.

Fix: replace with a real URL (Notion/Jira/GDoc), a relative path to an included artifact, or remove it and briefly describe the audit method in text.

PR #2267 review added an enabled option to usePaginatedList (and wired
it into metrics/behaviors) after this plan was written, closing a gap
where the hook fired list requests for users about to see
AccessDenied. Document the option and add it as a guardrail/checklist
item for future migrations.

@peqy peqy Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Docs plan is solid, but I’d fix the two doc-quality blockers before merging:

  • The links to ../apps/frontend/src/utils/server-prefetch.ts and ../apps/frontend/src/hooks/usePaginatedList.ts are broken on main right now (unless #2267 merges first). Please switch to permalinks in #2267 / the introducing commit, or add an explicit merge-order assumption.
  • “Audit performed via Survey” is a dead reference. Please replace with a real URL / included artifact, or remove it and describe the audit method in text.

@harry-rhesis

Copy link
Copy Markdown
Contributor Author

@nicolai-rhesis, would you also please check this proposed plan? We can tackle this after @EmanueleDeRossi1's optimizations on the backend.

@EmanueleDeRossi1

Copy link
Copy Markdown
Collaborator

Hi @harry-rhesis, thanks for drafting this plan! It tackles much-needed things in the app with direct UX implications ( I also like the suggestion of making page.tsx a Next.js Server Component -> remove the blank page/spinner on first load). I'm on board with the direction.

Maybe a couple ideas:

  1. We should centralize the OData filter construction instead of one helper per entity. Right now odata-filter.ts has ~7 near-duplicate convert<Entity>FilterItemToOData functions, each re-implementing the filter logic Adding buildProjectODataFilter, buildModelODataFilter, etc. as in the plan continues this same repetion. Proposing instead: one shared operator engine + a per-entity field-map (grid field → backend OData path) and a short list of special cases.

  2. On the open question in the plan (§8) I would propose:

  • Have the server-fetched data go directly into React Query's cache, not just get passed down as a prop. If we just pass it as a prop, the page will flash: data shows up instantly, then a split second later a loading spinner appears over it anyway (because React Query doesn't know the data's already there and fetches again). Putting it straight into the cache avoids that.
  • For Tasks/Annotations/Team members, which return their data in a slightly different shape than everyone else, let's just make those 3 match the standard shape, rather than teaching the shared code to handle multiple shapes forever.

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.

2 participants