From bcce6e2add97ae6247921384f3354b88fbcc2169 Mon Sep 17 00:00:00 2001 From: Abhinav Pola Date: Fri, 14 Aug 2026 15:08:19 -0700 Subject: [PATCH] refactor(vgi-bench): address review feedback from subtree sync - Un-export VGI_BENCH_DATASET_PATH, VGI_BENCH_TEMPERATURE, makeVgiBenchDatasetLayer, and vgiBenchSolver (never imported externally) - Fix test name: 'lowercase letters' -> the snapshot has uppercase letters - Remove duplicate downscaledVideoUrl test (identical input/output) - Merge five fragmented vgiBenchRecordToSample tests into a single test with an inline snapshot of the full sample plus canary-absence checks --- src/benchmarks/vgi-bench/benchmark.test.ts | 75 +++++++++++----------- src/benchmarks/vgi-bench/benchmark.ts | 8 +-- 2 files changed, 41 insertions(+), 42 deletions(-) diff --git a/src/benchmarks/vgi-bench/benchmark.test.ts b/src/benchmarks/vgi-bench/benchmark.test.ts index 5d557a1..fed5952 100644 --- a/src/benchmarks/vgi-bench/benchmark.test.ts +++ b/src/benchmarks/vgi-bench/benchmark.test.ts @@ -29,9 +29,19 @@ const VGI_RECORD: Readonly> = { }; describe("vgiBenchRecordToSample", () => { - it("builds the pinned multiple-choice prompt with lowercase letters", () => { + it("maps a record to a sample with uppercase prompt, video content part, target, and metadata", () => { const sample = vgiBenchRecordToSample(VGI_RECORD, 0); - expect(sample.input).toMatchInlineSnapshot(` + expect(sample).toMatchInlineSnapshot(` + { + "contentParts": [ + { + "type": "video_url", + "videoUrl": { + "url": "https://cdn.seldon.global/videos/clip_007.mp4", + }, + }, + { + "text": "Watch the video and answer the multiple choice question about it. The last line of your response should be of the following format: 'Answer: $LETTER' (without quotes) where LETTER is one of ABCD. Question: What color is the car in the first scene? @@ -40,38 +50,32 @@ describe("vgiBenchRecordToSample", () => { B) blue C) green D) yellow" - `); - }); - - it("places content parts: video_url first, then text", () => { - const sample = vgiBenchRecordToSample(VGI_RECORD, 0); - expect(sample.contentParts).toBeDefined(); - expect(sample.contentParts).toHaveLength(2); - expect(sample.contentParts![0]).toEqual({ - type: "video_url", - videoUrl: { url: "https://cdn.seldon.global/videos/clip_007.mp4" }, - }); - expect(sample.contentParts![1]!.type).toBe("text"); - }); - - it("derives the target letter from correct_answer index (uppercase)", () => { - const sample = vgiBenchRecordToSample(VGI_RECORD, 0); - expect(sample.target.text).toBe("B"); - }); + , + "type": "text", + }, + ], + "id": "vgi_bench-42", + "input": + "Watch the video and answer the multiple choice question about it. The last line of your response should be of the following format: 'Answer: $LETTER' (without quotes) where LETTER is one of ABCD. - it("uses question_id for a stable sample id", () => { - const sample = vgiBenchRecordToSample(VGI_RECORD, 0); - expect(sample.id).toBe("vgi_bench-42"); - }); + Question: What color is the car in the first scene? - it("records family and question_type metadata but never the canary", () => { - const sample = vgiBenchRecordToSample(VGI_RECORD, 0); - expect(sample.metadata?.["question_type"]).toBe( - "contrastive-hard-negative/transcript_visual" - ); - expect(sample.metadata?.["family"]).toBe("contrastive-hard-negative"); - expect(sample.metadata?.["video_id"]).toBe("clip_007"); - expect(sample.metadata?.["question_id"]).toBe(42); + A) red + B) blue + C) green + D) yellow" + , + "metadata": { + "family": "contrastive-hard-negative", + "question_id": 42, + "question_type": "contrastive-hard-negative/transcript_visual", + "video_id": "clip_007", + }, + "target": { + "text": "B", + }, + } + `); expect(JSON.stringify(sample.metadata)).not.toContain("canary"); expect(JSON.stringify(sample.input)).not.toContain("canary"); }); @@ -135,11 +139,6 @@ describe("downscaledVideoUrl", () => { "https://cdn.seldon.global/v/clip_007_proxy_v2" ); }); - it("does not splice into dots in the hostname for extension-less paths", () => { - expect(downscaledVideoUrl("https://cdn.seldon.global/v/clip_007")).toBe( - "https://cdn.seldon.global/v/clip_007_proxy_v2" - ); - }); it("operates only on the final path segment, not earlier directories", () => { expect( downscaledVideoUrl("https://cdn.seldon.global/v1.2/clip_007.mp4") @@ -157,7 +156,7 @@ describe("buildVgiBenchPrompt", () => { A) yes B) no" - `); + `); }); }); diff --git a/src/benchmarks/vgi-bench/benchmark.ts b/src/benchmarks/vgi-bench/benchmark.ts index 16b9e1d..775613e 100644 --- a/src/benchmarks/vgi-bench/benchmark.ts +++ b/src/benchmarks/vgi-bench/benchmark.ts @@ -21,7 +21,7 @@ import { defineChatBenchmark } from "../define-chat-benchmark"; import { mcqScorer } from "../scorers/mcq/scorer"; import type { Benchmark } from "../types"; -export const VGI_BENCH_DATASET_PATH = "Seldon-Technologies/VGIBench"; +const VGI_BENCH_DATASET_PATH = "Seldon-Technologies/VGIBench"; const VGI_BENCH_CONFIG = "default"; const VGI_BENCH_SPLIT = "train"; @@ -32,7 +32,7 @@ const PROXY_SUFFIX = "_proxy_v2"; const LETTERS = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; -export const VGI_BENCH_TEMPERATURE = 0; +const VGI_BENCH_TEMPERATURE = 0; export function buildVgiBenchPrompt( question: string, @@ -147,7 +147,7 @@ interface VgiBenchDatasetOpts extends VgiBenchRecordToSampleOptions { readonly revision?: string; } -export function makeVgiBenchDatasetLayer( +function makeVgiBenchDatasetLayer( opts?: VgiBenchDatasetOpts ): Layer { const config: HfDatasetConfig = { @@ -166,7 +166,7 @@ export function makeVgiBenchDatasetLayer( return makeHfDatasetLayer(config); } -export function vgiBenchSolver( +function vgiBenchSolver( model: ModelService, opts?: { readonly endpointId?: string;