From 50cd9f9588391e940fb9db79839e680c762f64d6 Mon Sep 17 00:00:00 2001 From: Thiago Hora Date: Thu, 20 Aug 2026 19:57:46 +0200 Subject: [PATCH 1/2] [OPIK-8023] Add page-boundary coverage for dataset-item JSON-key sorting Follow-up to #7935. Extend the parameterized version-DAO sort test with a page=2,size=2 request per namespace/direction, asserting the single trailing item on the boundary page and that total stays at the full matching count. This exercises the push-top-limit OFFSET :top_offset + outer LIMIT path, previously covered only at page=1,size=10. Co-Authored-By: Claude Opus 4.8 --- .../resources/v1/priv/DatasetVersionResourceTest.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetVersionResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetVersionResourceTest.java index d1622a5198a..bb79eb391c2 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetVersionResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetVersionResourceTest.java @@ -3591,6 +3591,15 @@ void sortByJsonKeyThroughPushTopLimit(String namespace, String jsonKey, Directio assertThat(sorted.content()) .usingRecursiveFieldByFieldElementComparatorIgnoringFields(IGNORED_FIELDS_DATA_ITEM) .containsExactlyElementsOf(expected); + + // Page boundary: with size=2, page 2 returns only the trailing item in sort order, exercising the + // push-top-limit OFFSET :top_offset + outer LIMIT path; total stays at the full matching count. + var pageTwo = datasetResourceClient.getDatasetItemsWithExperimentItems( + datasetId, List.of(experimentId), null, null, sorting, 2, 2, API_KEY, TEST_WORKSPACE); + assertThat(pageTwo.total()).isEqualTo(count); + assertThat(pageTwo.content()) + .usingRecursiveFieldByFieldElementComparatorIgnoringFields(IGNORED_FIELDS_DATA_ITEM) + .containsExactly(expected.get(count - 1)); } @Test From bc173cd53b3d2bf787b4b0d77979865146c4d4a4 Mon Sep 17 00:00:00 2001 From: Thiago Hora Date: Fri, 21 Aug 2026 11:11:31 +0200 Subject: [PATCH 2/2] [OPIK-8023] Materialize aggregates so the version-DAO sort test hits push-top-limit Per review: applyPushTopLimit requires hasAggregated && !hasRaw. Without materialized aggregates the query fell back to the raw path with an ordinary OFFSET, so the push-top-limit CTE (and its bound JSON key + top_offset) went untested. Populate experiment_item_aggregates via ExperimentAggregatesService before the sort/page-boundary assertions so the test deterministically exercises the push-top-limit branch. Co-Authored-By: Claude Opus 4.8 --- .../v1/priv/DatasetVersionResourceTest.java | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetVersionResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetVersionResourceTest.java index bb79eb391c2..edf1953eb91 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetVersionResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetVersionResourceTest.java @@ -51,11 +51,14 @@ import com.comet.opik.domain.SpanEnrichmentOptions; import com.comet.opik.domain.TestIdGeneratorFactory; import com.comet.opik.domain.TraceEnrichmentOptions; +import com.comet.opik.domain.experiments.aggregations.ExperimentAggregatesService; import com.comet.opik.extensions.DropwizardAppExtensionProvider; import com.comet.opik.extensions.RegisterApp; +import com.comet.opik.infrastructure.auth.RequestContext; import com.comet.opik.podam.PodamFactoryUtils; import com.comet.opik.utils.JsonUtils; import com.fasterxml.jackson.databind.JsonNode; +import com.google.inject.Injector; import com.redis.testcontainers.RedisContainer; import org.apache.hc.core5.http.HttpStatus; import org.junit.jupiter.api.AfterAll; @@ -152,9 +155,10 @@ class DatasetVersionResourceTest { private TraceResourceClient traceResourceClient; private SpanResourceClient spanResourceClient; private TransactionTemplate mySqlTemplate; + private ExperimentAggregatesService experimentAggregatesService; @BeforeAll - void setUpAll(ClientSupport client, TransactionTemplate mySqlTemplate) { + void setUpAll(ClientSupport client, TransactionTemplate mySqlTemplate, Injector injector) { this.baseURI = TestUtils.getBaseUrl(client); this.mySqlTemplate = mySqlTemplate; @@ -166,6 +170,7 @@ void setUpAll(ClientSupport client, TransactionTemplate mySqlTemplate) { experimentResourceClient = new ExperimentResourceClient(client, baseURI, factory); traceResourceClient = new TraceResourceClient(client, baseURI); spanResourceClient = new SpanResourceClient(client, baseURI); + experimentAggregatesService = injector.getInstance(ExperimentAggregatesService.class); } @AfterAll @@ -3572,6 +3577,15 @@ void sortByJsonKeyThroughPushTopLimit(String namespace, String jsonKey, Directio experimentResourceClient.createExperimentItem(Set.of(item), API_KEY, TEST_WORKSPACE); }); + // Materialize experiment_item_aggregates so the query takes the push-top-limit branch + // (applyPushTopLimit requires hasAggregated && !hasRaw); otherwise it falls back to the raw + // path with an ordinary OFFSET and the push-top-limit CTE would go untested. + experimentAggregatesService.populateAggregations(experimentId) + .contextWrite(ctx -> ctx + .put(RequestContext.USER_NAME, USER) + .put(RequestContext.WORKSPACE_ID, WORKSPACE_ID)) + .block(); + // Baseline fetch (no sorting) captures the full objects as returned; the assertion only tests order. var baseline = datasetResourceClient.getDatasetItemsWithExperimentItems( datasetId, List.of(experimentId), null, null, null, API_KEY, TEST_WORKSPACE).content();