OpenConceptLab/ocl_online#454 | Page HEAD concept and mapping lists by ids first

[paynejd]

Relates to OpenConceptLab/ocl_online#454 (partial). This is independent of #936; either can merge first.

Summary

On a source's HEAD concept or mapping list, the page query runs its joins for every row a deep page's OFFSET skips, and then throws those rows away. The joins are the select_related from/to concepts for mappings, and created_by/updated_by for concepts.

This change selects the page's ids first and joins only those rows:

SELECT … FROM mappings LEFT OUTER JOIN concepts … LEFT OUTER JOIN concepts …
WHERE <list filters> AND mappings.id IN (
    SELECT id FROM mappings WHERE <list filters> ORDER BY updated_at DESC LIMIT 1000 OFFSET 299000
)
ORDER BY updated_at DESC

• CustomPaginator(page_by_ids=True) filters by the sliced ids instead of slicing the joined query.
• ListWithHeadersMixin.should_page_by_ids() is False by default. ConceptListView and MappingListView return True for a source's HEAD list. Repo-version lists already select their ids first (is_sliced), and search results are paged by Elasticsearch, so neither changes.
• Pages return the same rows in the same order, and the count and paging headers are unchanged. Rows that share an updated_at (rare) may swap places within a page; their order was already arbitrary.

Effect

I timed the page query alone on production data, warm cache:

Page query	master	this branch
A large source's mappings, 300k rows deep	0.73 s	0.25 s
A large source's concepts, 200k rows deep	0.36 s	0.23 s

Both return identical rows. Details are on the ticket.

Next step (not in this PR)

On a cold cache, most of a deep page's cost is the OFFSET walk itself: the ids subquery reads every skipped row from the table to get its id. A covering index would let that walk read the index alone: the list index's columns (parent_id, updated_at DESC) with INCLUDE (id), under the same predicate. That index needs this change, because master's page query reads whole rows. It's a migration on the concepts and mappings tables, so it's proposed on the ticket first.

Tests

• core/integration_tests/tests_list_pages.py (new) checks two things, for concepts and mappings:
  • paging a HEAD list 4 rows at a time gives the same rows, in the same order, as one page of everything;
  • the page query's LIMIT/OFFSET sits inside the ids subquery.
• The tests fail on master and pass on this branch.
• Locally, the full suite (2,537 tests) passes and pylint -j0 core/ is clean.

Review

• Codex pass 1: clean.
• Simplification review: 4 cuts, applied in 71095aa: a shorter should_page_by_ids, a redundant test setup step, and a test this change can't affect.
• Codex check of the cuts: clean.

🤖 Generated with Claude Code

[paynejd]

Codex review, pass 1 (codex-cli 0.162.1, commit 94e5ef0)

Verdict: clean. No findings.

findings.json

{
    "reviewer": "codex-cli 0.162.1",
    "perspective": "defects",
    "mode": "full",
    "commit": "94e5ef03",
    "verdict": "clean",
    "prior": [],
    "findings": []
}

[paynejd]

Simplification review (fresh Claude subagent, commit 94e5ef0)

Verdict: 4 cuts proposed, all applied in 71095aa.

• S1, S2 (shrink): should_page_by_ids reads parent_resource.is_head through pydash get. Applied in both views.
• S3 (delete): set_checksums() in the test setUp. The first, unpaged request already stores them. Applied.
• S4 (delete): the version-list test. Version lists never reach page_by_ids, so it can't catch a regression from this change. Applied.

After the cuts, the paging tests fail on master and pass here; MappingListViewTest, ConceptListViewTest and pylint -j0 core/ pass too.

simplify.json

{
    "reviewer": "claude subagent",
    "perspective": "simplify",
    "mode": "full",
    "commit": "94e5ef03",
    "verdict": "findings",
    "prior": [],
    "findings": [
        {
            "id": "S1",
            "severity": null,
            "tag": "shrink",
            "file": "core/concepts/views.py",
            "line": 177,
            "claim": "should_page_by_ids fetches parent_resource into a local and null-checks it; pydash get walks the dotted path (as core/common/mixins.py:187 does).",
            "scenario": null,
            "action": "return 'source' in self.kwargs and bool(get(self, 'parent_resource.is_head'))",
            "lines_delta": -1,
            "confidence": "high"
        },
        {
            "id": "S2",
            "severity": null,
            "tag": "shrink",
            "file": "core/mappings/views.py",
            "line": 71,
            "claim": "Same override as ConceptListView; shorten the same way.",
            "scenario": null,
            "action": "return 'source' in self.kwargs and bool(get(self, 'parent_resource.is_head'))",
            "lines_delta": -1,
            "confidence": "high"
        },
        {
            "id": "S3",
            "severity": null,
            "tag": "delete",
            "file": "core/integration_tests/tests_list_pages.py",
            "line": 34,
            "claim": "row.set_checksums() in setUp is redundant: the uncaptured limit=total request computes and stores every row's checksums before the page requests.",
            "scenario": null,
            "action": "nothing replaces it",
            "lines_delta": -1,
            "confidence": "medium"
        },
        {
            "id": "S4",
            "severity": null,
            "tag": "delete",
            "file": "core/integration_tests/tests_list_pages.py",
            "line": 64,
            "claim": "test_version_list_unchanged can't fail because of this PR: repo-version lists are is_sliced, so CustomPaginator never reads page_by_ids.",
            "scenario": null,
            "action": "nothing replaces it",
            "lines_delta": -10,
            "confidence": "low"
        }
    ]
}

[paynejd]

Codex review, check of cuts (codex-cli 0.162.1, commit 71095aa)

Verdict: clean. The simplification commit broke nothing.

findings.json

{
    "reviewer": "codex-cli 0.162.1",
    "perspective": "defects",
    "mode": "cuts",
    "commit": "71095aa8",
    "verdict": "clean",
    "prior": [],
    "findings": []
}