[snyaggarwal]

OpenConceptLab/ocl_online#660

Summary

Expanding a concept with many children in the hierarchy view timed out (503). /children/ already paged, but building even the first page was too slow: it gathered every child URI into Python and then ran uri__in with the whole list.

• /children/ and /parents/ now build the list in one SQL subquery on HierarchicalConcepts. It returns the same concepts as before: the HEADs of what's linked to the concept, its HEAD and its latest version.
• has_children is worked out as part of the list query (Concept.annotate_has_children), so there are no extra queries per row. The has_children property returns that value when it's there.
• get_hierarchy_queryset, which $cascade uses for hierarchy, uses the same subquery. With a repo version, it keeps returning the linked versions, not their HEADs.
• Paging and the response headers are unchanged.

Not in this PR: under a source version URL, /children/ returns the HEAD children, not that version's. That will be tracked separately.

Benchmark (local, LOINC_PARTS_CONTAINER, 43,675 children)

Request	Before	After
/2.82/…/children/	503 after 33.8s	200 in 1.8s (first call), 0.5s after that
HEAD /children/, page 1 or 2	503 after 31.5s	200 in 0.5s
HEAD /children/, page 1000	503	200 in 3.6s
$cascade, levels=1, hierarchy view, 2.82	17.6s	5.0s
$cascade, levels=1, hierarchy view, HEAD	8.4s	6.8s

Checked against the old code on local LOINC data:

• /children/ and /parents/: the old and new sets are identical for HEAD, the latest version and an older version.
• get_hierarchy_queryset: no mismatches in 238 comparisons across 6 repo versions.
• has_children: the annotation matches the property on 252 rows, 132 of which have children.

Tests

• New:
  • paging and has_children through /children/
  • children and parents found through versions
  • the versioned get_hierarchy_queryset
  • the annotation matching the property
• core.concepts, core.collections, core.integration_tests.tests_concepts and tests_cross_repo_access: all 722 tests pass.
• pylint -j0 core/ exits 0.

🤖 Generated with Claude Code

[snyaggarwal]

Benchmark: LOINC_PARTS_CONTAINER (43,675 children), local data, LIST_QUERY_TIMEOUT_MS 30s

Request	Before	After
/orgs/Regenstrief/sources/LOINC/2.82/concepts/LOINC_PARTS_CONTAINER/children/	503 after 33.8s	200 in 1.8s (first call), 0.5s after that
HEAD /children/, page 1	503 after 31.6s	200 in 0.5s
HEAD /children/?page=2&limit=25	503 after 31.5s	200 in 0.5s
HEAD /children/?page=1000&limit=25	503	200 in 3.6s
$cascade/?cascadeLevels=1&view=hierarchy&cascadeMappings=false, 2.82	17.6s	5.0s
$cascade/?cascadeLevels=1&view=hierarchy&cascadeMappings=false, HEAD	8.4s	6.8s

Building just the hierarchy queryset, without serializing:

Query	Before	After
Children of HEAD, for HEAD	2.44s	0.28s
Children of HEAD, for 2.82	6.76s	0.15s
Children of the latest version, for HEAD	1.38s	0.20s
Children of the latest version, for 2.82	1.79s	0.13s

The results match the old code:

• /children/ and /parents/: the same 43,675 children for HEAD, the latest version and an older version.
• get_hierarchy_queryset ($cascade): no mismatches in 238 comparisons across 6 repo versions.
• has_children: the annotation matches the property on 252 rows.
• $cascade responses are the same size as before.

$cascade still truncates at 1,000 results, so the hierarchy view keeps using /children/ paging. TBv3 loads 25 at a time with Show more.

PRs: #988, OpenConceptLab/oclweb3#98

[snyaggarwal]

Reviewed together with OpenConceptLab/oclweb3#98. One paging finding below.

[snyaggarwal]

[P2] Give /children/ a stable tie-breaker before the web client pages through it. ConceptChildrenView.default_qs_sort_attr is only mnemonic; different sources can have children with the same mnemonic, so separate OFFSET requests can order tied rows differently at a page boundary. That can repeat one child and skip another in Show more. Ordering by mnemonic and a unique field such as id would make the pages stable (and a cross-source duplicate-mnemonic test would cover it).

[paynejd]

Claude review, with a Codex second opinion (commit ca411f4)

Verdict: approve with nits. The one-query children and parents list gives the same concepts as before, and the visibility filter still runs in both views. No P0 or P1. Two small things to settle, and a merge order with #985.

Fix before merge

• Stable paging (P2, @snyaggarwal, already in your own review). ConceptChildrenView orders by mnemonic only. Children from different repositories can share a mnemonic, so Show more can repeat one child and skip another. Use ['mnemonic', 'id'] and add a duplicate-mnemonic test. This is what makes "following next reaches every child" true.
• Merge order with #985. #985 returns a source version's own members under a version URL, in the same two views, so views.py conflicts. The version URL in the ticket (.../2.82/concepts/LOINC_PARTS_CONTAINER/children/) would then use #985's query, not this one. I'd merge #988 first, then rebase #985 so its children branch is wrapped in Concept.annotate_has_children(...), and re-time that 2.82 request.

Not blocking

• annotate_has_children counts children linked from any active version flagged latest. The old property looked only at the newest one. They differ only when a concept has two versions flagged latest (the case #987 repairs). Restrict it to the newest, or say the difference is accepted. (P2)
• Tests: nothing covers retired children or /parents/ paging through the new queryset. Access control is already covered in tests_cross_repo_access.py. (P3)
• The timings don't say which user made the request. A non-staff request also runs the visibility filter; worth one timing on LOINC.

Acceptance criteria

• First page of the 2.82 children within the timeout, correct num_found, next reaches every child: partly. Your timings support it. It needs the tie-breaker, and the URL changes path once #985 lands.
• Same concepts as before for a concept, its versioned object and its latest version: met (code read; tests compare against the old URL lists).
• has_children correct, no per-row queries: met, apart from the two-latest-versions case above.
• TBv3 loads 25 at a time with an error and retry (oclweb3#98): met (see that PR).

How it was checked

Read the diff against the old Python path, the through-table model and its indexes, the visibility filter, the paging mixin and every caller (both views and the four $cascade methods for source and collection versions). Compared with #985 and #987 by merge. Did not re-run the LOINC timings or the oclapi2 tests. A Codex blind pass on the diff was clean; a Codex verification pass confirmed the first two points above and partly confirmed the merge-order and test points.

findings.json

[
  {"id": "C1", "severity": "P2", "file": "core/concepts/views.py", "line": 609, "claim": "children ordered by mnemonic only", "action": "order by mnemonic, id", "source": "both", "codex_check": "confirmed", "introduced_by_pr": false},
  {"id": "C2", "severity": "P2", "file": "core/concepts/models.py", "line": 1437, "claim": "has_children annotation matches any latest-flagged version, the property only the newest", "action": "restrict to newest or accept", "source": "claude", "codex_check": "confirmed", "introduced_by_pr": true},
  {"id": "C3", "severity": "P2", "file": "core/concepts/views.py", "line": 620, "claim": "conflict with #985; version URL would bypass this query and the annotation", "action": "merge #988 first, rebase #985, annotate its children branch, re-time 2.82", "source": "claude", "codex_check": "partly", "introduced_by_pr": false},
  {"id": "C4", "severity": "P3", "file": "core/integration_tests/tests_concepts.py", "line": 4140, "claim": "no tests for retired children or parents paging", "action": "add small tests", "source": "claude", "codex_check": "partly", "introduced_by_pr": false}
]

[paynejd]

Approving at 5e0b3d7. The points from the earlier review are fixed:

• Children are ordered by ['mnemonic', 'id'], with a test that pages through three children sharing a mnemonic. parents/ gets the same tie-breaker.
• annotate_has_children follows only the newest active version flagged latest, the same rule as the has_children property, with a test for two versions flagged latest.
• New tests cover retired children and parents/ paging.

One small follow-up, not blocking: a source can hold two HEAD rows with the same URI. make_hierarchy's resolve already allows for this. The old child_concept_urls and parent_concept_urls returned such a URI once, and the counts counted it once. The new versions read rows by id, so they list and count both. Deduplicating by URI would restore the old behaviour, for example with .order_by().values_list('uri', flat=True).distinct() and .order_by().values('uri').distinct().count().

Checked: read the new commit, the list and CSV paths (the CSV export reads get_queryset, so moving the annotation into filter_queryset keeps it out), and get_latest_version. CI passed. A Codex check of 5e0b3d7 confirmed the three fixes and raised the URI point above.

The merge order from the earlier review still stands: this before #985, with #985 rebased onto it.