Skip to content

OpenConceptLab/ocl_issues#1854 | References API can conditionally include resolved repo versions#853

Open
snyaggarwal wants to merge 1 commit intomasterfrom
issues#1854
Open

OpenConceptLab/ocl_issues#1854 | References API can conditionally include resolved repo versions#853
snyaggarwal wants to merge 1 commit intomasterfrom
issues#1854

Conversation

@snyaggarwal
Copy link
Copy Markdown
Contributor

No description provided.

@snyaggarwal snyaggarwal requested a review from paynejd April 15, 2026 07:52
@snyaggarwal snyaggarwal linked an issue Apr 15, 2026 that may be closed by this pull request
@paynejd
Copy link
Copy Markdown
Member

paynejd commented Apr 21, 2026

Nice, focused change — the INCLUDE_* opt-in pattern and the model-method extraction both match existing conventions. Two blockers before merge:

1. Tests. The conditional field toggle and the new get_resolved_repo_versions_serialized() both need coverage:

  • ?includeResolvedRepoVersions=true → field present with expected shape
  • default → field omitted from response
  • reference with only system, only valueset, both, and neither

2. N×resolution performance. With the flag on, every reference triggers resolve_system_version + resolve_valueset_versions, each calling the full resolve_reference_expression algorithm (URL registry lookups, namespace match, version pinning). A collection with 100+ references will do 100+ resolution chains per list request. Worth addressing before this ships — a few options, roughly by effort:

  • Request-scoped memoization (cheapest). Many refs in a collection point to the same system. Cache by (namespace, url, version) across the request — the expansion builder already does exactly this at core/collections/models.py:1363. Lift that helper into the serializer.
  • Read from the expansion's M2M instead of re-resolving. Expansion.evaluated_source_versions / evaluated_collection_versions / explicit_* already hold resolution results. Prefetch rather than recompute. This is also closer to what ocl_issues#1854 asks for (the expansion-dropdown requirement implies resolved repo should be contextual to an expansion, not re-derived from reference fields) — may warrant a follow-up ticket.
  • Fast path for the canonical-URL branch. Batch via Source.objects.filter(canonical_url__in=[...]) for refs that don't hit URL-registry logic, fall back per-row otherwise. More code paths to maintain — only worth it if memoization isn't enough.

My suggestion: add memoization here to unblock merge, and split the expansion-context question into a follow-up since it shapes the API contract.

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.

Consumer: Reference

2 participants