Skip to content

Commit dfa1fcd

Browse files
committed
Prefer version over environment in loaders
1 parent 97e7959 commit dfa1fcd

2 files changed

Lines changed: 192 additions & 24 deletions

File tree

js/src/logger.test.ts

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import {
1010
initLogger,
1111
Prompt,
1212
BraintrustState,
13+
loadPrompt,
14+
loadParameters,
1315
wrapTraced,
1416
currentSpan,
1517
withParent,
@@ -464,6 +466,169 @@ test("init accepts dataset with id and version", () => {
464466
expect(datasetWithVersion.version).toBe("v2");
465467
});
466468

469+
describe("loader version precedence", () => {
470+
let state: BraintrustState;
471+
let getJson: ReturnType<typeof vi.spyOn>;
472+
const promptRow = {
473+
id: "11111111-1111-4111-8111-111111111111",
474+
_xact_id: "v1",
475+
project_id: "22222222-2222-4222-8222-222222222222",
476+
log_id: "p",
477+
org_id: "33333333-3333-4333-8333-333333333333",
478+
name: "Saved prompt",
479+
slug: "saved-prompt",
480+
description: null,
481+
tags: null,
482+
prompt_data: {
483+
prompt: {
484+
type: "chat",
485+
messages: [{ role: "user", content: "Hello" }],
486+
},
487+
options: { model: "gpt-5-mini" },
488+
},
489+
} satisfies {
490+
id: string;
491+
_xact_id: string;
492+
project_id: string;
493+
log_id: "p";
494+
org_id: string;
495+
name: string;
496+
slug: string;
497+
description: null;
498+
tags: null;
499+
prompt_data: {
500+
prompt: {
501+
type: "chat";
502+
messages: Array<{ role: "user"; content: string }>;
503+
};
504+
options: { model: string };
505+
};
506+
};
507+
const parametersRow = {
508+
id: "44444444-4444-4444-8444-444444444444",
509+
_xact_id: "v1",
510+
project_id: "55555555-5555-4555-8555-555555555555",
511+
name: "Saved parameters",
512+
slug: "saved-parameters",
513+
description: null,
514+
function_type: "parameters",
515+
function_data: {
516+
type: "parameters",
517+
data: { prefix: "hello" },
518+
__schema: {
519+
type: "object",
520+
properties: {
521+
prefix: { type: "string", default: "hello" },
522+
},
523+
additionalProperties: true,
524+
},
525+
},
526+
} satisfies {
527+
id: string;
528+
_xact_id: string;
529+
project_id: string;
530+
name: string;
531+
slug: string;
532+
description: null;
533+
function_type: "parameters";
534+
function_data: {
535+
type: "parameters";
536+
data: { prefix: string };
537+
__schema: {
538+
type: string;
539+
properties: { prefix: { type: string; default: string } };
540+
additionalProperties: boolean;
541+
};
542+
};
543+
};
544+
545+
beforeEach(async () => {
546+
state = await _exportsForTestingOnly.simulateLoginForTests();
547+
vi.spyOn(state, "login").mockResolvedValue(state);
548+
getJson = vi.spyOn(state.apiConn(), "get_json");
549+
});
550+
551+
afterEach(() => {
552+
_exportsForTestingOnly.simulateLogoutForTests();
553+
vi.restoreAllMocks();
554+
});
555+
556+
test("loadPrompt prefers version over environment for project lookup", async () => {
557+
getJson.mockResolvedValue({ objects: [promptRow] });
558+
559+
await loadPrompt({
560+
projectName: "test-project",
561+
slug: "saved-prompt",
562+
version: "v1",
563+
environment: "production",
564+
state,
565+
});
566+
567+
expect(getJson).toHaveBeenCalledWith(
568+
"v1/prompt",
569+
expect.objectContaining({
570+
project_name: "test-project",
571+
slug: "saved-prompt",
572+
version: "v1",
573+
}),
574+
);
575+
expect(getJson.mock.calls[0][1]).not.toHaveProperty("environment");
576+
});
577+
578+
test("loadPrompt prefers version over environment for id lookup", async () => {
579+
getJson.mockResolvedValue(promptRow);
580+
581+
await loadPrompt({
582+
id: promptRow.id,
583+
version: "v1",
584+
environment: "production",
585+
state,
586+
});
587+
588+
expect(getJson).toHaveBeenCalledWith(`v1/prompt/${promptRow.id}`, {
589+
version: "v1",
590+
});
591+
});
592+
593+
test("loadParameters prefers version over environment for project lookup", async () => {
594+
getJson.mockResolvedValue({ objects: [parametersRow] });
595+
596+
await loadParameters({
597+
projectName: "test-project",
598+
slug: "saved-parameters",
599+
version: "v1",
600+
environment: "production",
601+
state,
602+
});
603+
604+
expect(getJson).toHaveBeenCalledWith(
605+
"v1/function",
606+
expect.objectContaining({
607+
project_name: "test-project",
608+
slug: "saved-parameters",
609+
version: "v1",
610+
function_type: "parameters",
611+
}),
612+
);
613+
expect(getJson.mock.calls[0][1]).not.toHaveProperty("environment");
614+
});
615+
616+
test("loadParameters prefers version over environment for id lookup", async () => {
617+
getJson.mockResolvedValue(parametersRow);
618+
619+
await loadParameters({
620+
id: parametersRow.id,
621+
version: "v1",
622+
environment: "production",
623+
state,
624+
});
625+
626+
expect(getJson).toHaveBeenCalledWith(`v1/function/${parametersRow.id}`, {
627+
version: "v1",
628+
});
629+
});
630+
});
631+
467632
describe("prompt.build structured output templating", () => {
468633
test("applies nunjucks templating inside schema", () => {
469634
const prompt = new Prompt<false, false>(

js/src/logger.ts

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4073,6 +4073,7 @@ export type LoadParametersByIdOptions = LoadParametersBaseOptions & {
40734073
export type LoadParametersByIdWithEnvOptions = LoadParametersBaseOptions & {
40744074
id: string;
40754075
environment: string;
4076+
version?: string;
40764077
};
40774078

40784079
export type LoadParametersByProjectNameOptions = LoadParametersBaseOptions & {
@@ -4086,6 +4087,7 @@ export type LoadParametersByProjectNameWithEnvOptions =
40864087
projectName: string;
40874088
slug: string;
40884089
environment: string;
4090+
version?: string;
40894091
};
40904092

40914093
export type LoadParametersByProjectIdOptions = LoadParametersBaseOptions & {
@@ -4099,6 +4101,7 @@ export type LoadParametersByProjectIdWithEnvOptions =
40994101
projectId: string;
41004102
slug: string;
41014103
environment: string;
4104+
version?: string;
41024105
};
41034106

41044107
export type LoadParametersOptions =
@@ -4126,7 +4129,7 @@ type LoadParametersImplementationOptions = LoadParametersBaseOptions & {
41264129
* @param options.projectId The id of the project to load the prompt from. This takes precedence over `projectName` if specified.
41274130
* @param options.slug The slug of the prompt to load.
41284131
* @param options.version An optional version of the prompt (to read). If not specified, the latest version will be used.
4129-
* @param options.environment Fetch the version of the prompt assigned to the specified environment (e.g. "production", "staging"). Cannot be specified at the same time as `version`.
4132+
* @param options.environment Fetch the version of the prompt assigned to the specified environment (e.g. "production", "staging"). If both `version` and `environment` are provided, `version` takes precedence.
41304133
* @param options.id The id of a specific prompt to load. If specified, this takes precedence over all other parameters (project and slug).
41314134
* @param options.defaults (Optional) A dictionary of default values to use when rendering the prompt. Prompt values will override these defaults.
41324135
* @param options.noTrace If true, do not include logging metadata for this prompt when build() is called.
@@ -4162,11 +4165,11 @@ export async function loadPrompt({
41624165
forceLogin,
41634166
state: stateArg,
41644167
}: LoadPromptOptions) {
4165-
if (version && environment) {
4166-
throw new Error(
4167-
"Cannot specify both 'version' and 'environment' parameters. Please use only one (remove the other).",
4168-
);
4169-
}
4168+
const versionOrEnvironment = version
4169+
? { version }
4170+
: environment
4171+
? { environment }
4172+
: {};
41704173
if (id) {
41714174
// When loading by ID, we don't need project or slug
41724175
} else if (isEmpty(projectName) && isEmpty(projectId)) {
@@ -4187,10 +4190,9 @@ export async function loadPrompt({
41874190
});
41884191
if (id) {
41894192
// Load prompt by ID using the /v1/prompt/{id} endpoint
4190-
response = await state.apiConn().get_json(`v1/prompt/${id}`, {
4191-
...(version && { version }),
4192-
...(environment && { environment }),
4193-
});
4193+
response = await state
4194+
.apiConn()
4195+
.get_json(`v1/prompt/${id}`, versionOrEnvironment);
41944196
// Wrap single prompt response in objects array to match list API format
41954197
if (response) {
41964198
response = { objects: [response] };
@@ -4200,13 +4202,12 @@ export async function loadPrompt({
42004202
project_name: projectName,
42014203
project_id: projectId,
42024204
slug,
4203-
version,
4204-
...(environment && { environment }),
4205+
...versionOrEnvironment,
42054206
});
42064207
}
42074208
} catch (e) {
42084209
// If environment or version was specified, don't fall back to cache
4209-
if (environment || version) {
4210+
if (versionOrEnvironment) {
42104211
throw new Error(`Prompt not found with specified parameters: ${e}`);
42114212
}
42124213

@@ -4286,7 +4287,7 @@ export async function loadPrompt({
42864287
* @param options.projectId The id of the project to load the parameters from. This takes precedence over `projectName` if specified.
42874288
* @param options.slug The slug of the parameters to load.
42884289
* @param options.version An optional version of the parameters (to read). If not specified, the latest version will be used.
4289-
* @param options.environment Fetch the version of the parameters assigned to the specified environment (e.g. "production", "staging"). Cannot be specified at the same time as `version`.
4290+
* @param options.environment Fetch the version of the parameters assigned to the specified environment (e.g. "production", "staging"). If both `version` and `environment` are provided, `version` takes precedence.
42904291
* @param options.id The id of specific parameters to load. If specified, this takes precedence over all other parameters (project and slug).
42914292
* @param options.appUrl The URL of the Braintrust App. Defaults to https://www.braintrust.dev.
42924293
* @param options.apiKey The API key to use. If the parameter is not specified, will try to use the `BRAINTRUST_API_KEY` environment variable.
@@ -4339,11 +4340,15 @@ export async function loadParameters<
43394340
}: LoadParametersImplementationOptions): Promise<
43404341
RemoteEvalParameters<true, true, InferParameters<S>>
43414342
> {
4342-
if (version && environment) {
4343-
throw new Error(
4344-
"Cannot specify both 'version' and 'environment' parameters. Please use only one (remove the other).",
4345-
);
4346-
}
4343+
const effectiveEnvironment = version !== undefined ? undefined : environment;
4344+
const versionOrEnvironment =
4345+
version !== undefined
4346+
? { version }
4347+
: effectiveEnvironment !== undefined
4348+
? { environment: effectiveEnvironment }
4349+
: {};
4350+
const hasExplicitSelector =
4351+
version !== undefined || effectiveEnvironment !== undefined;
43474352
if (id) {
43484353
// When loading by ID, we don't need project or slug
43494354
} else if (isEmpty(projectName) && isEmpty(projectId)) {
@@ -4364,8 +4369,7 @@ export async function loadParameters<
43644369
});
43654370
if (id) {
43664371
response = await state.apiConn().get_json(`v1/function/${id}`, {
4367-
...(version && { version }),
4368-
...(environment && { environment }),
4372+
...versionOrEnvironment,
43694373
});
43704374
if (response) {
43714375
response = { objects: [response] };
@@ -4375,13 +4379,12 @@ export async function loadParameters<
43754379
project_name: projectName,
43764380
project_id: projectId,
43774381
slug,
4378-
version,
43794382
function_type: "parameters",
4380-
...(environment && { environment }),
4383+
...versionOrEnvironment,
43814384
});
43824385
}
43834386
} catch (e) {
4384-
if (environment || version) {
4387+
if (hasExplicitSelector) {
43854388
throw new Error(`Parameters not found with specified parameters: ${e}`);
43864389
}
43874390

0 commit comments

Comments
 (0)