Skip to content

Why these approaches are taken? #30

Description

@Zack-Rider

Parser Performance Fixes

Context

The parser process page ($id.tsx) was making 5+ network calls per cell edit, had unbounded queries fetching 600+ records on page load, and the HTTP server was blocking for 2–5 minutes per file upload. This document covers every issue fixed, the approach taken, available alternatives, and why the chosen approach was selected.


Fixed Issues


1. Job History — Unbounded Query on Page Load

File: client/src/routes/$orgName/process/layout.tsx

Problem: useFindManyDocumentParserJob was firing on page load with no take limit, fetching all jobs for the org (600+). Search was also purely client-side with no debounce.

Approaches Available:

Approach Description Tradeoff
Pagination (pages) Fetch page 1, page 2, etc. with prev/next buttons Good for large datasets but UX is click-heavy
Infinite scroll Fetch next page as user scrolls Smooth UX but complex to implement
Lazy load on open + take limit Fetch nothing until popover opens, limit to 50 Simple, zero cost on load, sufficient for most orgs
Virtual scroll Render only visible rows from a large dataset Overkill — job history doesn't need all 600 in DOM

Chosen Fix:

  • Controlled Popover — query only fires when popover is open (enabled: jobHistoryOpened)
  • take: 50 on initial load
  • Debounced backend search (300ms) using existing useDebounce hook — only fires when filtering
  • Frontend-first results: instantly filter the 50 loaded jobs while backend search is in flight

Why this approach: Job history is a secondary UI element — most users glance at it occasionally. Zero cost on load is the right default. The 50 most recent jobs cover 99% of use cases without pagination complexity. Frontend-first search gives instant feedback on already-loaded data while the debounced backend call handles edge cases (jobs outside the 50).


2. Upload Modal — Unbounded Query Before Modal Open

File: client/src/routes/$orgName/process/-upload-modal.tsx

Problem: useFindManyDocumentParserJob was fetching all jobs even before the modal was opened.

Approaches Available:

Approach Description Tradeoff
take limit only Just add take: 100, always fetch Still fetches before modal open — wasteful
Lazy + infinite scroll Fetch only when modal opens, load more on scroll Zero cost until needed, handles large history
Simple lazy with fixed limit Fetch only when open, no pagination Simpler but list cuts off at fixed number

Chosen Fix:

  • useInfiniteFindManyDocumentParserJob with take: 20 per page
  • enabled: opened || isUploading — fetch only when modal is open or upload is in progress
  • Cursor-based pagination with useInfiniteScroll hook
  • Loader spinner during isFetchingNextPage

Why this approach: The modal's "Previous Files" list is used to check upload history — users scroll through it. Infinite scroll fits naturally here. Lazy fetch means zero queries on page load, which was the core problem.


3. Logs — Always Fetching Regardless of Panel State

File: client/src/routes/$orgName/process/-logs.tsx

Problem: useFindManyLogs was always enabled, fetching logs even when the logs panel was closed.

Approaches Available:

Approach Description Tradeoff
Always fetch but limit Keep always-on, just add take: 50 Still fetches unnecessarily on every load
Gate on panel visibility Only fetch when user opens logs panel Zero cost until needed
Prefetch on hover Start fetching when user hovers over logs button Slightly faster open, more complex

Chosen Fix:

  • enabled prop on ProcessLogs component — parent passes enabled={showLogs}
  • take: 50 limit
  • Skeleton loader while isLoading

Why this approach: Logs are purely informational — users open them occasionally to audit changes. Gating on visibility is the simplest zero-cost solution. Prefetching on hover adds complexity for minimal gain since log fetch is fast.


4. Cell Edit — findUnique Refetch After Every Update

File: client/src/routes/$orgName/process/$id.tsx

Problem: ZenStack auto-invalidates queries after every mutation, triggering a findUnique refetch on every single cell edit. 1 edit = 2 calls (update + refetch).

Approaches Available:

Approach Description Tradeoff
Keep auto-invalidation Let ZenStack refetch after every mutation Always fresh data but 2x network calls per edit
Debounce the mutation Batch edits before sending — reduce mutation frequency Reduces both writes and refetches but complex
invalidateQueries: false + cache patch Disable auto-invalidation, manually update cache with mutation response Zero extra network call, always consistent
Optimistic update Update cache before mutation, rollback on error Fastest UX but complex rollback logic

Chosen Fix:

  • useUpdateDocumentParserJob({ invalidateQueries: false })
  • onSuccess patches React Query cache with setQueryDataForTable using updated schemaSaveData from mutation response

Why this approach: The mutation response already contains the updated job. Patching the cache with the response is equivalent to a refetch with zero network cost. Optimistic updates were skipped because rollback logic on parse errors would be complex.


5. handleDeleteRows / handleAddRow / handleAddColumn — Broad Invalidation

File: client/src/routes/$orgName/process/-raw-data-table.tsx

Problem: Each handler called invalidateQueriesForTable('DocumentParserJob') — invalidating ALL job queries in cache, including job history.

Approaches Available:

Approach Description Tradeoff
Broad invalidation (old) Invalidate all DocumentParserJob queries Simple but refetches job history, upload modal list, etc.
Scoped invalidation by ID Invalidate only queries matching job.id Fewer refetches but still a network call
Cache patch with local data Compute newSchemaData locally, patch cache directly Zero network call — data is already known before mutation

Chosen Fix:

  • setQueryDataForTable('DocumentParserJob', updater, [job.id]) — patches only the current job's cache entry with locally computed newSchemaData

Why this approach: These operations (delete row, add row, add column) all compute the exact final newSchemaData before calling the mutation. The result is deterministic — there is no need to ask the server what changed. Cache patch is the fastest possible update.


6. handleJobDelete — Broad Invalidation

File: client/src/routes/$orgName/process/$id.tsx

Problem: After deleting a job, broad invalidation refetched the entire job list from the server.

Approaches Available:

Approach Description Tradeoff
Broad invalidation (old) Refetch everything Always consistent but wastes a roundtrip
Scoped invalidation Invalidate only job list query Still a network call
Filter deleted job from cache Remove deleted job from every matching cache entry locally Zero network call, instant

Chosen Fix:

  • setQueryDataForTable('DocumentParserJob', (old) => old.filter(j => j.id !== id))

Why this approach: After deletion, the job is gone. There is nothing to fetch — the new state is simply the old list minus the deleted item. Local filter is both faster and more correct than a refetch.


7. map-alias — Broad Invalidation + Blocking UI + Lodash Debounce

File: client/src/routes/$orgName/process/-map-alias.tsx

Problem:

  • Broad invalidateQueriesForTable on success
  • disabled={isLoading} on search select — UI blocked during fetch
  • Lodash debounce function used directly (not React-safe, causes stale closure issues)

Approaches Available for Debounce:

Approach Description Tradeoff
Lodash debounce directly (old) Create debounce function in component body Recreated on every render, stale closure risk
useMemo(() => debounce(...)) Memoize the debounce function Safer but verbose
useDebounce hook Value-based debounce — returns debounced value Clean, React lifecycle-safe, consistent with codebase
useDebouncedCallback Callback-based debounce Good for callbacks, overkill for simple value debounce

Approaches Available for Loading UX:

Approach Description Tradeoff
Disable select while loading (old) disabled={isLoading} Blocks user interaction — bad UX
Loader in rightSection Show spinner in select corner, keep interactive Non-blocking visual feedback
Skeleton placeholder Replace select with skeleton Too disruptive for a search interaction

Chosen Fix:

  • setQueryDataForTable cache patch instead of invalidation
  • useDebounce(search, 300) hook — query uses debounced value
  • Removed isLoading from disabled, added rightSection={isLoading ? <Loader size={12} /> : null}

Why this approach: Cache patch rationale same as above. useDebounce is already used elsewhere in the codebase — consistency matters. Keeping the select interactive while showing a loader is strictly better UX than disabling it.


8. handleConfirm (Accept AI Table) — Broad Invalidation

File: client/src/routes/$orgName/process/-raw-data-table.tsx

Problem: After accepting an AI table via saveAITable, both DocumentParserJob and Org were invalidated broadly.

Approaches Available:

Approach Description Tradeoff
Broad invalidation for both (old) Refetch all job + org queries Over-invalidates job history
Scoped job invalidation + broad org Invalidate only current job, keep org broad Minimal refetch, correct for org
Cache patch for job Patch job cache locally saveAITable response doesn't return full updated job schema reliably

Chosen Fix:

  • invalidateQueriesForTable('DocumentParserJob', [job.id]) — scoped to current job
  • invalidateQueriesForTable('Org') — kept broad

Why this approach: DocumentParserJob can be scoped because only the current job changed. Org must stay broad — saveAITable creates actual database tables that change the org's schema, and that state cannot be predicted or patched locally.


9. Match Popup — Slow to Close After Clicking a Match

File: client/src/lib/table/cells/cell-display.tsx

Problem: setPopupOpenId("") ran after dispatchEvent, which synchronously triggered handleAfterSave with a heavy JSON.parse(JSON.stringify(...)) clone — blocking the JS thread before the popup could close.

Approaches Available:

Approach Description Tradeoff
Swap order of operations Close popup first, then dispatch event Zero cost, instant fix
setTimeout(() => dispatchEvent, 0) Defer event to next tick Works but adds unnecessary async complexity
Move JSON clone to useEffect Make handleAfterSave async Large refactor, risk of stale data
Web Worker for JSON clone Offload clone to worker thread Massive overkill

Chosen Fix:

// Before
dispatchEvent  handleAfterSave (heavy clone)  setPopupOpenId("")

// After  
setPopupOpenId("")  dispatchEvent  handleAfterSave (heavy clone)

Why this approach: The simplest possible fix — one line swap. The popup close is a pure state update that React batches into an immediate repaint. The expensive work happens after the user already sees the popup close.


10. Match Popup — No Loading Indicator While Fetching Matches

File: client/src/lib/table/cells/cell-display.tsx

Problem: No visual feedback while findRowsByIds was loading.

Approaches Available:

Approach Description Tradeoff
Skeleton list items Show skeleton rows where matches will appear Good but heavier implementation
Disable popup contents Gray out everything while loading Blocks interaction unnecessarily
Small spinner next to heading <Loader size={12} /> inline with "Top N Matches" Minimal, non-blocking, clear signal

Chosen Fix:

  • isLoading: isMatchesLoading from query
  • {isMatchesLoading && <Loader size={12} color="gray" />} next to heading

Why this approach: A small inline spinner is the least intrusive way to communicate loading state. The matches list renders as it arrives — no need to block the entire popup.


11. Upload — parseDocumentJob Blocking HTTP Request

File: server/src/routers/upload.ts

Problem: await parseDocumentJob(...) kept the HTTP connection open for 2–5 minutes. Risk of client timeout. Server resources tied up per file.

Approaches Available:

Approach Description Tradeoff
Keep await (old) Block until parse complete, then respond Simple but 2-5 min HTTP hold, timeout risk
Fire and forget with .catch() Respond immediately, parse in background, catch errors Simple, unblocks server, handles failures
BullMQ job queue Push parse job to Redis queue, worker processes it Production-grade: retries, concurrency control, monitoring — but requires queue infrastructure
Separate microservice Dedicated parser service Best for scale, massive over-engineering for current stage
SSE / WebSocket progress Stream parse progress to client Good UX for progress but complex, not needed yet

Chosen Fix:

parseDocumentJob(c, newParserJob.id, ...).catch(async (err) => {
  await prisma.documentParserJob.update({
    where: { id: newParserJob.id },
    data: { parserStatus: ParserJobStatus.Error },
  });
});
return c.json({ success: true }); // responds in < 1 second

Why this approach: Fire and forget is the fastest path to unblocking the server with minimal code change. The .catch() ensures failed parses surface as Error status instead of getting stuck in Processing. BullMQ is the right long-term solution but requires Redis queue setup — deferred until concurrency becomes an actual problem.


12. RawDataTable — Bloating $id.tsx

File: client/src/routes/$orgName/process/$id.tsx

Problem: $id.tsx contained the RawDataTable component, making the file extremely large and hard to navigate.

Approaches Available:

Approach Description Tradeoff
Keep in same file No change File stays large, hard to find things
Move to -raw-data-table.tsx Separate file following project naming convention Clean, follows - prefix convention for co-located components
Move to lib/ Put in shared component library Overkill — component is specific to this route

Chosen Fix: Moved to client/src/routes/$orgName/process/-raw-data-table.tsx.

Why this approach: The - prefix convention is already used for co-located route components in this folder (-logs.tsx, -map-alias.tsx, etc.). Matching that pattern keeps the project consistent.


13. Type Safety Fixes

File: client/src/routes/$orgName/process/$id.tsx

Problems:

  • Object.keys(...)[0] typed as string | undefined
  • as any / as never on JSON schema objects
  • e.detail possibly undefined in handleAfterSave

Approaches Available:

Approach Description Tradeoff
@ts-ignore Suppress the error Hides the problem, masks future bugs
as any Cast to any Same as above, loses all type safety
[0]! + Record<string, unknown> + guard Non-null assertion, proper type cast, runtime guard Compile-time safe, no runtime impact
Refactor to avoid Object.keys()[0] pattern Use a typed key instead Good long-term but large refactor

Chosen Fix:

  • Object.keys(...)[0]! where key is guaranteed present
  • as Record<string, unknown> for JSON schema objects
  • if (!e.detail) return guard in handleAfterSave

Why this approach: These are compile-time only changes — zero runtime impact. Record<string, unknown> is the correct type for arbitrary JSON objects. The guard is the minimal safe fix for the e.detail nullable case without restructuring the event handling.


Remaining Issues

# Issue Impact Recommended Fix
1 N+1 findRowsByIds — each cell fires its own query for foreign key display High Aggregate all relationIds at table level, single query per foreign key column
2 Debounced save — every keystroke triggers a DB write via handleAfterSave High pendingSchemaRef + 800ms debounced mutation — accumulate edits, single write per burst
3 BullMQ job queue — no concurrency control on simultaneous parses Medium BullMQ with concurrency: 2, Redis already in project
4 Parser stuck in Processing — pre-existing stuck jobs before .catch() fix Medium One-time migration script to mark old stuck jobs as Error
5 saveDocumentMutation broad invalidation Low Scoped invalidateQueriesForTable with [job.id]
6 handleConfirm Org broad invalidation Low Necessary for correctness — cannot scope without knowing which org tables changed

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions