-
Notifications
You must be signed in to change notification settings - Fork 1.8k
[OPIK-8102] [QA] Proposed e2e specs for the LLM-judge Gemini thinking level (from #8158 exploration) #8160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: AndreiCautisanu/OPIK-8102/flash-lite-thinking-none
Are you sure you want to change the base?
[OPIK-8102] [QA] Proposed e2e specs for the LLM-judge Gemini thinking level (from #8158 exploration) #8160
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -181,6 +181,28 @@ export interface AutomationRuleDetail { | |
| triggerScope: string; | ||
| } | ||
|
|
||
| /** | ||
| * The `code.model` block of an LLM-as-judge rule, as the REST view answers it. | ||
| * | ||
| * Read raw rather than through the pinned SDK for the same reason as | ||
| * `AutomationRuleDetail`: the SDK's evaluator shape has no | ||
| * `custom_parameters`, and this is the field that decides what the judge call | ||
| * actually sends to the provider. | ||
| */ | ||
| export interface LlmJudgeModelDetail { | ||
| /** The model id the rule will call, e.g. `gemini-3.5-flash-lite`. */ | ||
| name: string; | ||
| /** | ||
| * `custom_parameters` verbatim, `null` when the rule carries none. | ||
| * | ||
| * Kept nullable rather than defaulted to `{}` because absent and empty are | ||
| * different answers here: a thinking level of "none" must REMOVE the block, | ||
| * and a spec that cannot tell "no block" from "empty block" cannot assert | ||
| * that. | ||
| */ | ||
| customParameters: Record<string, unknown> | null; | ||
| } | ||
|
|
||
| /** One line of a rule's user-facing log stream. */ | ||
| export interface AutomationRuleLogRef { | ||
| level: string; | ||
|
|
@@ -1580,6 +1602,41 @@ export function makeBackendClient(apiKey: string | null = null, workspaceName: s | |
| }; | ||
| }, | ||
|
|
||
| /** | ||
| * The model block of an LLM-as-judge rule — the model id and the | ||
| * `custom_parameters` the judge call will carry. | ||
| * | ||
| * Throws rather than defaulting when the rule is not an LLM-as-judge one: | ||
| * a Python-code rule has no `code.model` at all, and answering `null` for | ||
| * it would let a mis-typed spec assert "no custom parameters" about a rule | ||
| * that could never have had any. | ||
| */ | ||
| async getLlmJudgeModel(ruleId: string): Promise<LlmJudgeModelDetail> { | ||
| const { status, message, json } = await rawFetch( | ||
| 'GET', | ||
| `/v1/private/automations/evaluators/${ruleId}`, | ||
| ); | ||
| if (status !== 200) { | ||
| throw new Error(`getLlmJudgeModel: ${ruleId} answered ${status}: ${message}`); | ||
| } | ||
| const model = (json as { code?: { model?: unknown } } | null)?.code?.model as | ||
| | { name?: unknown; custom_parameters?: unknown } | ||
| | undefined; | ||
| if (!model || typeof model.name !== 'string') { | ||
| throw new Error( | ||
| `getLlmJudgeModel: ${ruleId} carries no code.model.name — not an LLM-as-judge rule?`, | ||
| ); | ||
| } | ||
| const custom = model.custom_parameters; | ||
| return { | ||
| name: model.name, | ||
| customParameters: | ||
| custom === null || custom === undefined | ||
| ? null | ||
| : (custom as Record<string, unknown>), | ||
|
Comment on lines
+1633
to
+1636
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Accepts malformed custom parameters
Want Baz to fix this for you? Activate Fixer Other fix methodsPrompt for AI Agents |
||
| }; | ||
| }, | ||
|
|
||
| /** | ||
| * A rule's user-facing log stream — the lines `/automation-logs` renders. | ||
| * | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,20 +49,52 @@ function endpoint(path = ''): string { | |
| return `${env.apiBaseUrl}/v1/private/llm-provider-key${path}`; | ||
| } | ||
|
|
||
| export async function findProviderKeyByName(providerName: string): Promise<ProviderKeyRef | null> { | ||
| async function listProviderKeys(): Promise<ProviderKeyRef[]> { | ||
| const response = await fetch(endpoint(), { headers: restHeaders() }); | ||
| if (!response.ok) throw new Error(`list provider keys returned ${response.status}`); | ||
| const body = (await response.json()) as { content: ProviderKeyRef[] }; | ||
| return body.content.find((key) => key.provider_name === providerName) ?? null; | ||
| return body.content; | ||
|
Comment on lines
55
to
+56
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Malformed list responses cause opaque lookup crashes
Want Baz to fix this for you? Activate Fixer Other fix methodsPrompt for AI Agents |
||
| } | ||
|
|
||
| export async function findProviderKeyByName(providerName: string): Promise<ProviderKeyRef | null> { | ||
| const keys = await listProviderKeys(); | ||
| return keys.find((key) => key.provider_name === providerName) ?? null; | ||
| } | ||
|
|
||
| /** | ||
| * Find a key by its PROVIDER slug (`gemini`, `vertex-ai`, …) rather than by | ||
| * `provider_name`, which only the custom/bedrock/ollama providers carry. | ||
| * | ||
| * The workspace holds at most one key per built-in provider, so this is the | ||
| * only way to ask "is Gemini already configured here" — and a caller that | ||
| * seeds one has to ask, because creating a second would collide with whatever | ||
| * key the environment already had. | ||
| */ | ||
| export async function findProviderKeyByProvider(provider: string): Promise<ProviderKeyRef | null> { | ||
| const keys = await listProviderKeys(); | ||
| return keys.find((key) => key.provider === provider) ?? null; | ||
| } | ||
|
|
||
| /** | ||
| * Create a provider key, and report its id from the `Location` header — | ||
| * creation answers 201 with an empty body, so that header is the only place | ||
| * the id appears. | ||
| * | ||
| * `null` rather than a throw when the header is missing, so a caller that | ||
| * cleans up by `provider_name` is not made to care: only a caller that has to | ||
| * address the key by id needs the header, and that caller is the one that | ||
| * should complain about it. | ||
| */ | ||
| export async function createProviderKey(payload: { | ||
| provider: string; | ||
| provider_name: string; | ||
| base_url: string; | ||
| /** Only the custom/bedrock/ollama providers name their keys. */ | ||
| provider_name?: string; | ||
| base_url?: string; | ||
| /** The provider secret. Built-in providers key off this rather than `auth_config`. */ | ||
| api_key?: string; | ||
| configuration?: Record<string, string>; | ||
| auth_config?: ProviderAuthConfig; | ||
| }): Promise<void> { | ||
| }): Promise<string | null> { | ||
| const response = await fetch(endpoint(), { | ||
| method: 'POST', | ||
| headers: restHeaders(), | ||
|
|
@@ -71,21 +103,26 @@ export async function createProviderKey(payload: { | |
| if (!response.ok) { | ||
| throw new Error(`create provider key returned ${response.status}: ${await response.text()}`); | ||
| } | ||
| return response.headers.get('location')?.split('/').filter(Boolean).pop() ?? null; | ||
| } | ||
|
|
||
| export async function deleteProviderKeyByName(providerName: string): Promise<void> { | ||
| const found = await findProviderKeyByName(providerName); | ||
| if (!found) return; | ||
| export async function deleteProviderKeyById(id: string): Promise<void> { | ||
| const response = await fetch(endpoint('/delete'), { | ||
| method: 'POST', | ||
| headers: restHeaders(), | ||
| body: JSON.stringify({ ids: [found.id] }), | ||
| body: JSON.stringify({ ids: [id] }), | ||
| }); | ||
| if (!response.ok) { | ||
| throw new Error(`delete provider key returned ${response.status}`); | ||
| } | ||
| } | ||
|
|
||
| export async function deleteProviderKeyByName(providerName: string): Promise<void> { | ||
| const found = await findProviderKeyByName(providerName); | ||
| if (!found) return; | ||
| await deleteProviderKeyById(found.id); | ||
| } | ||
|
|
||
| /** Carries the HTTP status so callers can classify a failure instead of parsing prose. */ | ||
| export class AuthConfigCheckError extends Error { | ||
| constructor( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
REST response type hides wire-format field
The handwritten raw REST type exposes backend
custom_parametersascustomParameters, whilegetLlmJudgeModelreadsmodel.custom_parameters, so the type no longer matches the payload and callers rely on an undocumented mapping — should we rename the field and returned object tocustom_parameters, or introduce a separately mapped camelCase DTO?Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents