Skip to content

perf(roms): avoid hydrating full Rom rows for siblings on list endpoint#3432

Merged
gantoine merged 6 commits into
masterfrom
claude/awesome-gates-NUpke
May 25, 2026
Merged

perf(roms): avoid hydrating full Rom rows for siblings on list endpoint#3432
gantoine merged 6 commits into
masterfrom
claude/awesome-gates-NUpke

Conversation

@gantoine
Copy link
Copy Markdown
Member

The paginated ROM list eager-loaded sibling_roms via selectinload, which
hydrated full Rom ORM instances (including heavy JSON metadata columns)
for every sibling even though only an existence/count check was needed
on the frontend. On large collections this dominated request latency.

Split sibling handling by response shape:

  • SimpleRomSchema (list): siblings is now list[int]; populated per page
    by a single SELECT against the sibling_roms view projecting only
    (rom_id, sibling_rom_id) — no Rom row hydration.
  • DetailedRomSchema (detail): keeps full SiblingRomSchema objects, with
    load_only on (id, name, fs_name_no_tags, fs_name_no_ext) so sibling
    rows stop dragging in JSON metadata.

Frontend usage already only consumes siblings.length on list views; the
detail-page VersionSwitcher continues to receive the richer schema.

The paginated ROM list eager-loaded sibling_roms via selectinload, which
hydrated full Rom ORM instances (including heavy JSON metadata columns)
for every sibling even though only an existence/count check was needed
on the frontend. On large collections this dominated request latency.

Split sibling handling by response shape:
- SimpleRomSchema (list): siblings is now list[int]; populated per page
  by a single SELECT against the sibling_roms view projecting only
  (rom_id, sibling_rom_id) — no Rom row hydration.
- DetailedRomSchema (detail): keeps full SiblingRomSchema objects, with
  load_only on (id, name, fs_name_no_tags, fs_name_no_ext) so sibling
  rows stop dragging in JSON metadata.

Frontend usage already only consumes siblings.length on list views; the
detail-page VersionSwitcher continues to receive the richer schema.
Copilot AI review requested due to automatic review settings May 25, 2026 12:33
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR optimizes the ROM list endpoint by avoiding eager-loading full sibling Rom ORM instances (and their heavy JSON metadata) when the list UI only needs sibling existence/count semantics.

Changes:

  • Changed SimpleRomSchema.siblings (list endpoint) to list[int] and populated it via a single query against the sibling_roms view.
  • Kept detailed endpoint sibling objects, but limited sibling ROM column hydration via load_only(...).
  • Removed selectinload(Rom.sibling_roms) from the list-query options to prevent unnecessary ORM hydration.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.

File Description
frontend/src/generated/models/SimpleRomSchema.ts Updates generated TS type so list responses expose siblings: number[].
backend/handler/database/roms_handler.py Adds get_sibling_ids_for_roms helper and limits sibling hydration in detail queries via load_only.
backend/endpoints/roms/init.py Updates paginated list endpoint transformer to attach sibling ID lists per page.
backend/endpoints/responses/rom.py Splits siblings field by schema type: int[] for simple, SiblingRomSchema[] for detailed.
Files not reviewed (1)
  • frontend/src/generated/models/SimpleRomSchema.ts: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread backend/handler/database/roms_handler.py Outdated
Comment thread backend/endpoints/roms/__init__.py Outdated
Comment thread backend/endpoints/responses/rom.py
Comment thread backend/endpoints/roms/__init__.py Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 7 changed files in this pull request and generated 3 comments.

Files not reviewed (2)
  • frontend/src/generated/models/DetailedRomSchema.ts: Language not supported
  • frontend/src/generated/models/SimpleRomSchema.ts: Language not supported

Comment thread backend/handler/database/roms_handler.py Outdated
Comment thread backend/endpoints/responses/rom.py Outdated
Comment thread backend/handler/database/roms_handler.py Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 25, 2026

Test Results (postgresql)

    1 files      1 suites   4m 25s ⏱️
1 403 tests 1 403 ✅ 0 💤 0 ❌
1 405 runs  1 405 ✅ 0 💤 0 ❌

Results for commit fb3cc1d.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 25, 2026

Test Results (mariadb)

    1 files      1 suites   4m 8s ⏱️
1 403 tests 1 403 ✅ 0 💤 0 ❌
1 405 runs  1 405 ✅ 0 💤 0 ❌

Results for commit fb3cc1d.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 25, 2026

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
16558 11665 70% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
backend/endpoints/responses/rom.py 95% 🟢
backend/endpoints/roms/_init_.py 64% 🟢
backend/handler/database/roms_handler.py 58% 🟢
TOTAL 72% 🟢

updated for commit: fb3cc1d by action🐍

gantoine and others added 2 commits May 25, 2026 11:35
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@gantoine gantoine merged commit c261d5e into master May 25, 2026
13 checks passed
@gantoine gantoine deleted the claude/awesome-gates-NUpke branch May 25, 2026 16:26
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.

3 participants