Skip to content

Commit 6e446b7

Browse files
authored
fix(core): clarify use_target resolution errors (#917)
* fix(core): clarify use_target resolution errors * style(core): format use_target error fix
1 parent 624ffe8 commit 6e446b7

4 files changed

Lines changed: 117 additions & 17 deletions

File tree

packages/core/src/evaluation/orchestrator.ts

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ import {
1717
import { readJsonFile } from './file-utils.js';
1818
import { createBuiltinProviderRegistry, createProvider } from './providers/index.js';
1919
import { discoverProviders } from './providers/provider-discovery.js';
20-
import { type ResolvedTarget, resolveTargetDefinition } from './providers/targets.js';
20+
import {
21+
type ResolvedTarget,
22+
resolveDelegatedTargetDefinition,
23+
resolveTargetDefinition,
24+
} from './providers/targets.js';
2125
import type {
2226
EnvLookup,
2327
Message,
@@ -356,22 +360,10 @@ export async function runEvaluation(
356360
if (resolvedTargetsByName.has(name)) {
357361
return resolvedTargetsByName.get(name);
358362
}
359-
// Follow use_target chain to find the concrete definition
360-
let definition = targetDefinitions.get(name);
363+
const definition = resolveDelegatedTargetDefinition(name, targetDefinitions, envLookup);
361364
if (!definition) {
362365
return undefined;
363366
}
364-
for (let depth = 0; depth < 5; depth++) {
365-
const useTarget = definition.use_target;
366-
if (typeof useTarget !== 'string' || useTarget.trim().length === 0) break;
367-
// Resolve ${{ ENV_VAR }} syntax
368-
const envMatch = useTarget.trim().match(/^\$\{\{\s*([A-Z0-9_]+)\s*\}\}$/i);
369-
const resolvedName = envMatch ? (envLookup[envMatch[1]] ?? '') : useTarget.trim();
370-
if (resolvedName.length === 0) break;
371-
const next = targetDefinitions.get(resolvedName);
372-
if (!next) break;
373-
definition = next;
374-
}
375367
const resolved = resolveTargetDefinition(definition, envLookup, evalFilePath);
376368
resolvedTargetsByName.set(name, resolved);
377369
return resolved;

packages/core/src/evaluation/providers/index.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,11 @@ import { PiCliProvider } from './pi-cli.js';
1919
import { PiCodingAgentProvider } from './pi-coding-agent.js';
2020
import { ProviderRegistry } from './provider-registry.js';
2121
import type { ResolvedTarget } from './targets.js';
22-
import { COMMON_TARGET_SETTINGS, resolveTargetDefinition } from './targets.js';
22+
import {
23+
COMMON_TARGET_SETTINGS,
24+
resolveDelegatedTargetDefinition,
25+
resolveTargetDefinition,
26+
} from './targets.js';
2327
import type { EnvLookup, Provider, TargetDefinition } from './types.js';
2428
import { VSCodeProvider } from './vscode-provider.js';
2529

@@ -59,7 +63,7 @@ export type {
5963
VSCodeResolvedConfig,
6064
} from './targets.js';
6165

62-
export { COMMON_TARGET_SETTINGS, resolveTargetDefinition };
66+
export { COMMON_TARGET_SETTINGS, resolveDelegatedTargetDefinition, resolveTargetDefinition };
6367
export { readTargetDefinitions, listTargetNames } from './targets-file.js';
6468
export {
6569
ensureVSCodeSubagents,

packages/core/src/evaluation/providers/targets.ts

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -652,6 +652,8 @@ export const COMMON_TARGET_SETTINGS = [
652652
'fallbackTargets',
653653
] as const;
654654

655+
const USE_TARGET_ENV_PATTERN = /^\$\{\{\s*([A-Z0-9_]+)\s*\}\}$/i;
656+
655657
const BASE_TARGET_SCHEMA = z
656658
.object({
657659
name: z.string().min(1, 'target name is required'),
@@ -727,6 +729,68 @@ function resolveRetryConfig(target: z.infer<typeof BASE_TARGET_SCHEMA>): RetryCo
727729
};
728730
}
729731

732+
export function resolveDelegatedTargetDefinition(
733+
name: string,
734+
definitions: ReadonlyMap<string, TargetDefinition>,
735+
env: EnvLookup = process.env,
736+
): TargetDefinition | undefined {
737+
let definition = definitions.get(name);
738+
if (!definition) {
739+
return undefined;
740+
}
741+
742+
const visited = [definition.name];
743+
744+
for (let depth = 0; depth < 10; depth++) {
745+
const rawUseTarget =
746+
typeof definition.use_target === 'string' ? definition.use_target.trim() : undefined;
747+
if (!rawUseTarget) {
748+
return definition;
749+
}
750+
751+
const envMatch = rawUseTarget.match(USE_TARGET_ENV_PATTERN);
752+
const envVarName = envMatch?.[1];
753+
const resolvedName = envVarName ? (env[envVarName]?.trim() ?? '') : rawUseTarget;
754+
755+
if (resolvedName.length === 0) {
756+
if (envVarName) {
757+
throw new Error(
758+
`Target "${definition.name}" uses use_target: \${{ ${envVarName} }}, but ${envVarName} is not set. Set ${envVarName} to the name of a concrete target (for example, "azure") before running the eval.`,
759+
);
760+
}
761+
762+
throw new Error(
763+
`Target "${definition.name}" has an empty use_target value. Point it at a concrete target name before running the eval.`,
764+
);
765+
}
766+
767+
const next = definitions.get(resolvedName);
768+
if (!next) {
769+
if (envVarName) {
770+
throw new Error(
771+
`Target "${definition.name}" uses use_target: \${{ ${envVarName} }}, which resolved to "${resolvedName}", but no target named "${resolvedName}" exists.`,
772+
);
773+
}
774+
775+
throw new Error(
776+
`Target "${definition.name}" uses use_target: "${resolvedName}", but no target named "${resolvedName}" exists.`,
777+
);
778+
}
779+
780+
if (visited.includes(next.name)) {
781+
const chain = [...visited, next.name].join(' -> ');
782+
throw new Error(`Circular use_target reference detected: ${chain}`);
783+
}
784+
785+
definition = next;
786+
visited.push(definition.name);
787+
}
788+
789+
throw new Error(
790+
`Target "${name}" exceeded the maximum use_target resolution depth (10). Check for a delegation loop or overly deep alias chain.`,
791+
);
792+
}
793+
730794
export function resolveTargetDefinition(
731795
definition: TargetDefinition,
732796
env: EnvLookup = process.env,

packages/core/test/evaluation/providers/targets.test.ts

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,49 @@ mock.module('@ai-sdk/google', () => ({
6060
}));
6161

6262
const providerModule = await import('../../../src/evaluation/providers/index.js');
63-
const { resolveTargetDefinition, createProvider } = providerModule;
63+
const { resolveDelegatedTargetDefinition, resolveTargetDefinition, createProvider } =
64+
providerModule;
6465
const { extractLastAssistantContent } = await import('../../../src/evaluation/providers/types.js');
6566

67+
describe('resolveDelegatedTargetDefinition', () => {
68+
it('throws a helpful error when an env-backed use_target variable is missing', () => {
69+
const definitions = new Map([
70+
['grader', { name: 'grader', use_target: '${{ GRADER_TARGET }}' }],
71+
['azure', { name: 'azure', provider: 'azure' }],
72+
]);
73+
74+
expect(() => resolveDelegatedTargetDefinition('grader', definitions, {})).toThrow(
75+
/GRADER_TARGET is not set/i,
76+
);
77+
});
78+
79+
it('throws a helpful error when an env-backed use_target resolves to a missing target', () => {
80+
const definitions = new Map([
81+
['grader', { name: 'grader', use_target: '${{ GRADER_TARGET }}' }],
82+
]);
83+
84+
expect(() =>
85+
resolveDelegatedTargetDefinition('grader', definitions, {
86+
GRADER_TARGET: 'azure',
87+
}),
88+
).toThrow(/resolved to "azure".*no target named "azure" exists/i);
89+
});
90+
91+
it('resolves a delegated target chain to a concrete definition', () => {
92+
const definitions = new Map([
93+
['grader', { name: 'grader', use_target: '${{ GRADER_TARGET }}' }],
94+
['llm', { name: 'llm', use_target: 'azure' }],
95+
['azure', { name: 'azure', provider: 'azure' }],
96+
]);
97+
98+
const resolved = resolveDelegatedTargetDefinition('grader', definitions, {
99+
GRADER_TARGET: 'llm',
100+
});
101+
102+
expect(resolved).toEqual({ name: 'azure', provider: 'azure' });
103+
});
104+
});
105+
66106
describe('resolveTargetDefinition', () => {
67107
beforeEach(() => {
68108
generateTextMock.mockClear();

0 commit comments

Comments
 (0)