perf: run blocking data endpoints in a threadpool, not on the event loop

[anxkhn]

The data-heavy routes are declared async def while doing synchronous, blocking
pandas file I/O (read_table_safe -> pd.read_csv/read_excel/read_parquet,
save_table_safe -> df.to_csv/to_excel) and CPU-bound pandas work. In
Starlette an async def path operation runs directly on the asyncio event loop,
so a single request parsing a CSV/XLSX (up to the 10MB upload limit, which can
take hundreds of milliseconds to seconds) blocks every other concurrent request
for that whole duration. GET /projects/get/{id} is the clearest case: it reads
the entire file on every paginated page view, then slices in memory.

FastAPI runs plain def path operations in an external threadpool for exactly
this reason, keeping the loop free. This PR moves the blocking work off the event
loop with the smallest change that preserves behavior:

• Convert the routes that perform blocking I/O and hold no await from
  async def to plain def, so FastAPI runs them in its threadpool:
  • projects.py: get_project_details, save_project, revert_to_checkpoint,
    export_project, undo_last_transformation
  • transformations.py: transform_project
  • profiling.py: get_dataset_summary, get_column_profile,
    get_all_column_profiles, get_correlation_matrix
• upload_project must stay async def because it awaits upload validation,
  so wrap its blocking store_upload (disk write) and read_table_safe (parse)
  calls in starlette.concurrency.run_in_threadpool instead.

rename_project_endpoint and delete_project_endpoint are intentionally left
async def: they touch only DB metadata and do no pandas file I/O, so keeping
the diff off them stays focused on the actual problem.

This is a pure concurrency fix with no change to request/response behavior.

No existing issue tracks this, so there is no Fixes # reference.

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update

How Has This Been Tested?

Added dataloom-backend/tests/test_endpoint_concurrency.py with three tests:

• Structural: the ten blocking routes are plain def (not coroutines), so FastAPI
  offloads them to the threadpool.
• Structural: upload_project stays async def and its body offloads both
  store_upload and read_table_safe via run_in_threadpool.
• Behavioral: with read_table_safe patched to sleep, five overlapping
  GET /projects/get/{id} requests must finish well under the serialized floor
  (they overlap in the threadpool instead of running one after another on the loop).

Verified red -> green: on the pre-fix async def code the behavioral test
serializes five 0.4s reads to about 2.1s and all three tests fail; after the fix
they overlap and finish in about 0.5s and pass.

Commands (from dataloom-backend/, Python 3.12, DATABASE_URL=sqlite:///./test.db):

• uv run ruff check . -> All checks passed

• uv run ruff format --check . -> already formatted

• uv run pytest -> 482 passed (the pre-existing SQLModel session.query
  deprecation warnings are unrelated to this change)

• Existing tests pass

• New tests added

• Manual testing

Screenshots (if applicable)

N/A (backend-only concurrency change).

Checklist

• My code follows the project's style guidelines
• I have performed a self-review
• I have added/updated documentation as needed (no docs change needed; behavior is unchanged)
• My changes generate no new warnings
• Tests pass locally

---

Files changed (diff = origin/main..HEAD)

dataloom-backend/app/api/endpoints/profiling.py       |   8 +-
dataloom-backend/app/api/endpoints/projects.py        |  15 +--
dataloom-backend/app/api/endpoints/transformations.py |   2 +-
dataloom-backend/tests/test_endpoint_concurrency.py   | 106 +++++++++++++++++++
4 files changed, 119 insertions(+), 12 deletions(-)

[ivantha]

Automated review summary

Triage: bug | Recommendation: needs-revision
GSoC scope: no overlap with reserved GSoC scope

Findings

• [major/bug] dataloom-backend/app/api/endpoints/transformations.py:104 Converting transform_project (and analogously save_project, revert_to_checkpoint, undo_last_transformation in projects.py) from async def to plain def removes the incidental serialization that came from blocking a single-threaded event loop. These handlers still do read-file -> mutate -> write-file on project.file_path (see lines 121/126/133) with no per-project lock or optimistic-concurrency check. Previously, a blocking async def call froze the whole event loop, which accidentally prevented two requests for the same project from interleaving; now that the same code runs in a genuine threadpool, two truly concurrent requests against the same project_id (double-submit, two open tabs, a save racing an undo) can interleave and produce a lost update. The PR's own new test only exercises concurrency on a read-only route (get_project_details); nothing covers concurrent writes to the same project.
• [minor/test] dataloom-backend/tests/test_endpoint_concurrency.py:103 test_slow_read_does_not_block_other_requests asserts on wall-clock elapsed time (elapsed < read_delay * concurrency * 0.6, i.e. ~1.2s budget vs a ~2.0s serialized floor). The margin is generous enough that it's unlikely to flake, but timing-based assertions are inherently sensitive to CI runner load and threadpool warmup; worth keeping an eye on if this test ever goes flaky, since a small structural/behavioral check wouldn't depend on wall-clock thresholds.

Conventions

• PR title: follows Conventional Commits
• Description: clear and complete
• Commits (1): clean, conventional history

This is a well-scoped, well-tested backend concurrency fix: it moves ten route handlers that were doing blocking pandas/file I/O directly inside async def bodies (silently stalling the whole event loop for every other request) to plain def so FastAPI offloads them to its threadpool, and wraps upload_project's two blocking calls in run_in_threadpool since that route must stay async. The new test file pins the contract both structurally (routes are sync; upload offloads correctly) and behaviorally (concurrent requests no longer serialize), and CI (backend lint, backend tests, e2e) is green aside from a pre-existing, unrelated frontend-lint formatting failure. The one thing worth a second look before merge: this same change removes the incidental serialization that was masking a lost-update race when transform_project/save_project/revert_to_checkpoint/undo_last_transformation are hit concurrently for the same project, and no test covers that path -- worth a follow-up (per-project lock or optimistic concurrency) even if it doesn't block this PR. PR title and description are exemplary (clear Conventional Commit title, thorough rationale and scoping notes, explicit red/green testing methodology), and the single commit is clean.

Generated by /review-prs. A maintainer will follow up.

[anxkhn]

addressed the race in 09869b3. mutating operations now take a per-project lock across their file and change-log work, while different projects and read-only previews remain concurrent. added a regression test that runs two same-project transforms concurrently and verifies both updates survive.

also rebased onto current main and confirmed the frontend prettier check, frontend lint, backend ruff, and all 648 backend tests pass locally. @ivantha could you please re-review?

[ivantha]

Automated review summary

Triage: bug | Recommendation: needs-revision
GSoC scope: no overlap with reserved GSoC scope

Findings

• [major/bug] dataloom-backend/app/api/endpoints/projects.py:118 Preview-mode transform_project (unlocked fallback) plus the read-only routes get_project_details (projects.py:118), export_project (projects.py:319), and the four profiling routes (profiling.py) never acquire project_write_lock, yet they all read the same project.file_path that save_project, revert_to_checkpoint, undo_last_transformation, and non-preview transform_project now write to from a genuinely separate threadpool thread. Because file_formats.py's writers (_write_csv, _write_json, _write_xlsx, _write_parquet) truncate and rewrite the destination path in place instead of writing atomically (temp file plus rename), a concurrently running unlocked reader can now observe a partially written file mid-write, which read_table_safe surfaces as a 400 or 500 to the client, and export_project's native-format FileResponse can stream a torn mix of old and new bytes. This race was structurally impossible before this PR, since every blocking call previously ran serialized on the single event-loop thread; it only becomes reachable because this PR moves these handlers onto real OS threads, and none of the PR's new tests exercise a concurrent read against an in-flight write.
• [minor/quality] dataloom-backend/app/api/endpoints/transformations.py:121 Unlike save_project (projects.py:196-210), which scopes project_write_lock tightly around just the file read and checkpoint creation, transform_project pulls its entire body, including pagination slicing and dataframe_to_response construction, inside the lock by calling _transform_project from within the with project_write_lock block; revert_to_checkpoint (projects.py:243) and undo_last_transformation (projects.py:438) do the same via their own helpers. None of that trailing in-memory work touches the shared file or database row, so holding the lock through it needlessly extends the critical section and reduces the concurrency win this PR is aiming for whenever two requests target the same project back to back.
• [nit/style] dataloom-backend/app/api/endpoints/transformations.py:125 The extracted helper _transform_project(project_id, transformation_input, preview, page, page_size, db, project) drops every type hint that transform_project's original FastAPI parameter declarations carried (uuid.UUID, schemas.TransformationInput, bool, int, Session, models.Project). The same pattern repeats in projects.py's new _revert_to_checkpoint (line 247) and _undo_last_transformation (line 442), inconsistent with the rest of the typed codebase, e.g. _unlink_if_exists(path: str) -> None a few lines above in projects.py.

Conventions

• PR title: follows Conventional Commits
• Description: clear and complete
• Commits (1): clean, conventional history

This PR correctly diagnoses and fixes a real bug: ten data-heavy routes ran blocking pandas I/O directly on the asyncio event loop, so converting them to plain def and offloading upload_project's blocking calls via run_in_threadpool is the right fix, and the new project_write_lock is a necessary, correctly implemented companion change that prevents lost-update races between concurrent writers on the same project. However, the lock only guards the write endpoints: get_project_details, export_project, the four profiling routes, and preview-mode transform_project still read the same working-copy file unlocked, and since file_formats.py's writers truncate and rewrite the file in place rather than atomically, a concurrently running read can now observe a partially written file and return a spurious 400 or 500, a new failure mode this PR's own tests do not cover. Recommend closing that read or write gap (or making the writers atomic via temp file plus rename) before merging. The lock-scope inconsistency between save_project's tight critical section and the other three endpoints holding the lock for their entire body, including pure response construction, is a smaller, non-blocking follow-up.

Generated by /review-prs. A maintainer will follow up.

[ivantha]

Automated review summary

Triage: bug | Recommendation: needs-revision
GSoC scope: no overlap with reserved GSoC scope

Findings

• [major/bug] dataloom-backend/app/api/endpoints/projects.py:124 get_project_details reads project.file_path without taking project_write_lock, while save_project/revert_to_checkpoint/undo_last_transformation/transform_project now hold that lock only while writing the same path from a genuinely concurrent OS thread (routes are plain def, run via FastAPI's threadpool). The four format writers in app/utils/file_formats.py (_write_csv/_write_xlsx/_write_json/_write_parquet) all open the target path directly and truncate it immediately, with no temp-file-plus-atomic-rename, so a concurrent read can now land mid-write and see a truncated or partial file (parse error, or in principle malformed data) — a race window that was effectively impossible before this PR because every blocking call ran to completion on the single event-loop thread before anything else could run. The same unlocked-read-vs-locked-write gap applies to profiling.py's four endpoints (via load_project_df), export_project's read path, and transform_project when preview=True.
• [minor/quality] dataloom-backend/app/api/endpoints/transformations.py:82 transform_project duplicates the call to _transform_project(project_id, transformation_input, preview, page, page_size, db, project) across both branches of if not preview (lines 83-85) solely to decide whether project_write_lock is held. A conditional context manager (e.g. contextlib.nullcontext() when preview is true) would express the same branching without repeating the 7-argument call.
• [nit/test] dataloom-backend/tests/test_endpoint_concurrency.py:143 project_write_lock's docstring promises it serializes 'without blocking other projects,' but no test exercises two distinct project_ids concurrently to confirm that. test_slow_read_does_not_block_other_requests reuses a single pid for all 5 concurrent requests, and it calls get_project_details, which never acquires project_write_lock in the first place, so the cross-project-independence claim is untested.

Conventions

• PR title: follows Conventional Commits
• Description: clear and complete
• Commits (2): clean, conventional history

This PR converts ten routes doing blocking pandas file I/O from async def to plain def so FastAPI runs them in its threadpool instead of the event loop, wraps upload_project's blocking calls in run_in_threadpool, and adds a new per-project threading.Lock (project_write_lock) around the four write endpoints (save, revert, undo, non-preview transform) to prevent lost updates now that those writes can run truly concurrently. The core fix is sound and well tested (structural checks plus a behavioral overlap test), but the new lock only covers writer-vs-writer races: read-only routes (get_project_details, all four profiling endpoints, export, and preview-mode transform) still read the same file path unlocked, and since the format writers truncate-and-write the target path directly rather than via temp-file-plus-rename, a read can now race an in-flight write and see a partial file — a window that was effectively closed before this PR by the single event-loop thread. Recommend closing that read/write gap (e.g., a shared/read variant of the lock, or making saves atomic via temp file + os.replace) before or shortly after merge. Two minor cleanups are also worth a look: transform_project duplicates its dispatch call across both branches of the preview check, and the lock's "doesn't block other projects" guarantee isn't directly tested.

Generated by /review-prs. A maintainer will follow up.