Skip to content

Commit 367aa17

Browse files
Merge pull request #1738 from kubeflow/main
[pull] main from kubeflow:main
2 parents 6360f79 + 17b7835 commit 367aa17

25 files changed

Lines changed: 866 additions & 242 deletions

File tree

catalog/internal/catalog/modelcatalog/loader.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,9 +329,11 @@ func (l *ModelLoader) updateDatabase(ctx context.Context) error {
329329
}
330330
}
331331

332-
for _, handler := range l.handlers {
333-
handler(ctx, record)
332+
for _, handler := range l.handlers {
333+
if err := handler(ctx, record); err != nil {
334+
glog.Errorf("%s: event handler error: %v", *attr.Name, err)
334335
}
336+
}
335337
}()
336338
}
337339
}()

catalog/internal/catalog/modelcatalog/performance_metrics.go

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,11 @@ func (pr *performanceRecord) UnmarshalJSON(data []byte) error {
107107
if id, ok := raw["id"].(string); ok {
108108
pr.ID = id
109109
}
110+
if pr.ID == "" {
111+
if configID, ok := raw["config_id"].(string); ok && configID != "" {
112+
pr.ID = configID
113+
}
114+
}
110115
if modelID, ok := raw["model_id"].(string); ok {
111116
pr.ModelID = modelID
112117
}
@@ -505,19 +510,28 @@ func createAccuracyMetricsArtifact(evalRecords []evaluationRecord, modelID int32
505510
// Properties can be empty or contain general metadata
506511
properties := []models.Properties{}
507512

508-
// Create custom properties - simple mapping of benchmark_name to score_value
509-
customProperties := []models.Properties{}
510-
513+
// Create custom properties - simple mapping of benchmark_name to score_value.
514+
// Deduplicate by benchmark name: if multiple evaluation records share the same
515+
// benchmark, keep the last score encountered. This prevents DB constraint violations
516+
// on the (artifact_id, name, is_custom_property) composite primary key.
517+
benchmarkScores := make(map[string]float64, len(evalRecords))
511518
for _, evalRecord := range evalRecords {
512-
// Add the benchmark score as a named property (e.g., "aime24": 63.3333)
513519
if score, ok := evalRecord.CustomProperties["score"].(float64); ok {
514-
customProperties = append(customProperties, models.Properties{
515-
Name: evalRecord.Benchmark,
516-
DoubleValue: &score,
517-
})
520+
if _, duplicate := benchmarkScores[evalRecord.Benchmark]; duplicate {
521+
glog.Warningf("Duplicate benchmark %q for model %d, using latest score", evalRecord.Benchmark, modelID)
522+
}
523+
benchmarkScores[evalRecord.Benchmark] = score
518524
}
519525
}
520526

527+
customProperties := make([]models.Properties, 0, len(benchmarkScores)+1)
528+
for benchmark, score := range benchmarkScores {
529+
customProperties = append(customProperties, models.Properties{
530+
Name: benchmark,
531+
DoubleValue: &score,
532+
})
533+
}
534+
521535
// Add overall_average custom property from metadata.json overall_accuracy field
522536
if overallAccuracy != nil {
523537
customProperties = append(customProperties, models.Properties{

catalog/internal/catalog/modelcatalog/performance_metrics_test.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,88 @@ func TestOverallAccuracyToOverallAverage(t *testing.T) {
330330
})
331331
}
332332

333+
func TestCreateAccuracyMetricsArtifact_DuplicateBenchmarks(t *testing.T) {
334+
t.Run("duplicate benchmarks are deduplicated using last score", func(t *testing.T) {
335+
evalRecords := []evaluationRecord{
336+
{Benchmark: "mmlu", CustomProperties: map[string]any{"score": 80.0}},
337+
{Benchmark: "aime24", CustomProperties: map[string]any{"score": 63.3}},
338+
{Benchmark: "mmlu", CustomProperties: map[string]any{"score": 85.0}},
339+
}
340+
341+
artifact := createAccuracyMetricsArtifact(evalRecords, 1, 100, nil, nil, nil)
342+
343+
// Count occurrences of each benchmark name
344+
benchmarkCounts := map[string]int{}
345+
benchmarkScores := map[string]float64{}
346+
for _, prop := range *artifact.CustomProperties {
347+
benchmarkCounts[prop.Name]++
348+
if prop.DoubleValue != nil {
349+
benchmarkScores[prop.Name] = *prop.DoubleValue
350+
}
351+
}
352+
353+
// "mmlu" should appear exactly once (deduplicated)
354+
if benchmarkCounts["mmlu"] != 1 {
355+
t.Errorf("expected mmlu to appear once, got %d", benchmarkCounts["mmlu"])
356+
}
357+
358+
// The last score (85.0) should win
359+
if benchmarkScores["mmlu"] != 85.0 {
360+
t.Errorf("expected mmlu score 85.0, got %v", benchmarkScores["mmlu"])
361+
}
362+
363+
// "aime24" should still be present
364+
if benchmarkCounts["aime24"] != 1 {
365+
t.Errorf("expected aime24 to appear once, got %d", benchmarkCounts["aime24"])
366+
}
367+
if benchmarkScores["aime24"] != 63.3 {
368+
t.Errorf("expected aime24 score 63.3, got %v", benchmarkScores["aime24"])
369+
}
370+
})
371+
372+
t.Run("no duplicates produces all benchmarks", func(t *testing.T) {
373+
evalRecords := []evaluationRecord{
374+
{Benchmark: "mmlu", CustomProperties: map[string]any{"score": 90.0}},
375+
{Benchmark: "aime24", CustomProperties: map[string]any{"score": 63.3}},
376+
{Benchmark: "gpqa", CustomProperties: map[string]any{"score": 72.5}},
377+
}
378+
379+
artifact := createAccuracyMetricsArtifact(evalRecords, 1, 100, nil, nil, nil)
380+
381+
benchmarkNames := map[string]bool{}
382+
for _, prop := range *artifact.CustomProperties {
383+
benchmarkNames[prop.Name] = true
384+
}
385+
386+
for _, expected := range []string{"mmlu", "aime24", "gpqa"} {
387+
if !benchmarkNames[expected] {
388+
t.Errorf("expected benchmark %q not found in custom properties", expected)
389+
}
390+
}
391+
})
392+
393+
t.Run("all records with same benchmark produce single property", func(t *testing.T) {
394+
evalRecords := []evaluationRecord{
395+
{Benchmark: "mmlu", CustomProperties: map[string]any{"score": 80.0}},
396+
{Benchmark: "mmlu", CustomProperties: map[string]any{"score": 82.0}},
397+
{Benchmark: "mmlu", CustomProperties: map[string]any{"score": 85.0}},
398+
}
399+
400+
artifact := createAccuracyMetricsArtifact(evalRecords, 1, 100, nil, nil, nil)
401+
402+
count := 0
403+
for _, prop := range *artifact.CustomProperties {
404+
if prop.Name == "mmlu" {
405+
count++
406+
}
407+
}
408+
409+
if count != 1 {
410+
t.Errorf("expected exactly 1 mmlu property, got %d", count)
411+
}
412+
})
413+
}
414+
333415
func TestEvaluationRecordUnmarshalJSON(t *testing.T) {
334416
tests := []struct {
335417
name string

clients/python/poetry.lock

Lines changed: 39 additions & 39 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

clients/python/pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ aiohttp-retry = "^2.8.3"
2424
# allows for reentrant event loops (used for sync client) - Python 3.14 compatible
2525
nest-asyncio2 = "^1.7.1"
2626

27-
huggingface-hub = { version = ">=0.20.1,<1.13.0", optional = true }
27+
huggingface-hub = { version = ">=0.20.1,<1.14.0", optional = true }
2828
olot = { version = "^0.1.17", optional = true }
2929
boto3 = { version = "^1.37.34", optional = true }
3030
rh-model-signing = { version = "1.0.1", optional = true }

clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistryView/registerAndStoreFields.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,9 @@ class RegisterAndStoreFields {
4848
}
4949

5050
selectNamespace(name: string) {
51-
this.findNamespaceSelectCombobox().scrollIntoView().click({ force: true });
51+
this.findNamespaceSelectCombobox().should('not.be.disabled');
52+
this.findNamespaceSelectCombobox().scrollIntoView().click();
53+
cy.findByRole('listbox').should('be.visible');
5254
cy.findByRole('option', { name }).click();
5355
}
5456

@@ -66,11 +68,12 @@ class RegisterAndStoreFields {
6668
}
6769

6870
shouldHaveNamespaceOptions(namespaces: string[]) {
69-
this.findNamespaceSelectCombobox().scrollIntoView().click({ force: true });
71+
this.findNamespaceSelectCombobox().should('not.be.disabled');
72+
this.findNamespaceSelectCombobox().scrollIntoView().click();
7073
namespaces.forEach((namespace) => {
7174
cy.findByRole('option', { name: namespace }).should('exist');
7275
});
73-
this.findNamespaceSelectCombobox().scrollIntoView().click({ force: true });
76+
this.findNamespaceSelectCombobox().scrollIntoView().click();
7477
return this;
7578
}
7679

@@ -301,9 +304,18 @@ class RegisterAndStoreFields {
301304
this.findSourceS3SecretAccessKeyInput().type(secretAccessKey);
302305
}
303306

307+
/** Sets model type (required on register page). Uses Predictive by default. */
308+
selectModelType(
309+
optionName: 'Predictive Model' | 'Generative AI model (Example, LLM)' = 'Predictive Model',
310+
) {
311+
cy.get('#register-model-type-toggle').click();
312+
cy.findByRole('option', { name: optionName }).click();
313+
}
314+
304315
// Convenience method to fill all required fields for submission
305316
fillAllRequiredFields() {
306317
this.fillModelName('test-model');
318+
this.selectModelType();
307319
this.fillVersionName('v1.0.0');
308320
this.fillJobName('my-transfer-job');
309321
this.fillSourceEndpoint('https://s3.amazonaws.com');

clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/registerAndStoreFields.cy.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,7 @@ describe('Register and Store Fields - Credential Validation', () => {
331331
it('Should have submit button disabled when S3 access key ID is missing', () => {
332332
// Fill all fields except S3 access key ID
333333
registerAndStoreFields.fillModelName('test-model');
334+
registerAndStoreFields.selectModelType();
334335
registerAndStoreFields.fillVersionName('v1.0.0');
335336
registerAndStoreFields.fillJobName('my-transfer-job');
336337
registerAndStoreFields.fillSourceEndpoint('https://s3.amazonaws.com');
@@ -349,6 +350,7 @@ describe('Register and Store Fields - Credential Validation', () => {
349350
it('Should have submit button disabled when S3 secret access key is missing', () => {
350351
// Fill all fields except S3 secret access key
351352
registerAndStoreFields.fillModelName('test-model');
353+
registerAndStoreFields.selectModelType();
352354
registerAndStoreFields.fillVersionName('v1.0.0');
353355
registerAndStoreFields.fillJobName('my-transfer-job');
354356
registerAndStoreFields.fillSourceEndpoint('https://s3.amazonaws.com');
@@ -367,6 +369,7 @@ describe('Register and Store Fields - Credential Validation', () => {
367369
it('Should have submit button disabled when OCI username is missing', () => {
368370
// Fill all fields except OCI username
369371
registerAndStoreFields.fillModelName('test-model');
372+
registerAndStoreFields.selectModelType();
370373
registerAndStoreFields.fillVersionName('v1.0.0');
371374
registerAndStoreFields.fillJobName('my-transfer-job');
372375
registerAndStoreFields.fillSourceEndpoint('https://s3.amazonaws.com');
@@ -385,6 +388,7 @@ describe('Register and Store Fields - Credential Validation', () => {
385388
it('Should have submit button disabled when OCI password is missing', () => {
386389
// Fill all fields except OCI password
387390
registerAndStoreFields.fillModelName('test-model');
391+
registerAndStoreFields.selectModelType();
388392
registerAndStoreFields.fillVersionName('v1.0.0');
389393
registerAndStoreFields.fillJobName('my-transfer-job');
390394
registerAndStoreFields.fillSourceEndpoint('https://s3.amazonaws.com');

0 commit comments

Comments
 (0)