diff --git a/cli/azd/extensions/azure.ai.evaluations/README.md b/cli/azd/extensions/azure.ai.evaluations/README.md index 148b7fbb711..dcdde896504 100644 --- a/cli/azd/extensions/azure.ai.evaluations/README.md +++ b/cli/azd/extensions/azure.ai.evaluations/README.md @@ -23,30 +23,31 @@ services: evals: host: azure.ai.eval uses: [ai-project] - $ref: ./evals/azure.yaml + $ref: ./evals/azure.eval.yaml ``` ```yaml -# evals/azure.yaml +# evals/azure.eval.yaml datasets: - name: support-golden - source: ./datasets/support-golden.jsonl + file: ./datasets/support-golden.jsonl evaluators: - name: support-quality source: ./evaluators/support-quality.json -evalGroups: +evals: - name: support-quality dataset: support-golden + evaluation_level: turn evaluators: - - builtin.task_adherence - - support-quality + - evaluator: builtin.task_adherence + initialization_parameters: + model: gpt-4.1-nano + - evaluator: support-quality target: type: agent name: support-agent - options: - eval_model: gpt-4.1-nano ``` `azd up` reconciles **datasets → evaluators → eval groups**, in that order, @@ -64,7 +65,7 @@ keys you authored — the service adds `data_schema`, `init_parameters` and `metrics` of its own. Eval groups are immutable, so a change to a group's evaluators, target or -options creates a new group and a new id. The id is cached in the azd +sampling creates a new group and a new id. The id is cached in the azd environment so repeat runs stay comparable. ## Commands diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/catalog.go b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/catalog.go index e92a76321f2..0b8acd61887 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/catalog.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/catalog.go @@ -28,16 +28,16 @@ func addDatasetToCatalog(cmd *cobra.Command, evalDir string, ref *project.Artifa if cfg.Datasets[i].Name == ref.Name { // Regeneration overwrites the file in place, so the entry only // changes when the artifact moved. - if cfg.Datasets[i].Source == ref.Source { + if cfg.Datasets[i].File == ref.Source { return false } - cfg.Datasets[i].Source = ref.Source + cfg.Datasets[i].File = ref.Source return true } } cfg.Datasets = append(cfg.Datasets, project.DatasetDecl{ - Name: ref.Name, - Source: ref.Source, + Name: ref.Name, + File: ref.Source, }) return true }) @@ -88,7 +88,7 @@ func updateCatalog( } defer unlock() - cfg, err := project.OpenEvalConfig(evalDir) + cfg, err := project.OpenEvalConfigForEdit(evalDir) if err != nil { return err } diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/context.go b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/context.go index b6256a30565..f35d4b1e0c2 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/context.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/context.go @@ -9,6 +9,7 @@ import ( "fmt" "log" "os" + "path/filepath" "strings" "azureaieval/internal/foundry/projectctx" @@ -417,12 +418,21 @@ const ( // // 1. --path // 2. the path `init` recorded in the azd environment -// 3. ./evals +// 3. the `$ref` on the `azure.ai.eval` service in azure.yaml +// 4. ./evals // -// The middle level is what stops `--path` from having to be repeated on every -// later command. Without it, `init --path ./quality` wrote a configuration that -// `run` then looked for under ./evals and reported as missing -- while -// azure.yaml's $ref pointed at it correctly the whole time. +// The middle levels are what stop `--path` from having to be repeated on every +// later command. Without the recorded one, `init --path ./quality` wrote a +// configuration that `run` then looked for under ./evals and reported as +// missing -- while azure.yaml's $ref pointed at it correctly the whole time. +// +// That $ref is now read rather than only written, which is what makes the rule +// survive a fresh clone. The recorded path lives in the azd environment, and an +// azd environment is not in the repository: check the project out somewhere +// else and level 2 is empty, so a configuration the project declares perfectly +// well under ./config was reported missing by every command while `azd up` +// deployed it. Reading the declaration is also what keeps one answer to "where +// is the configuration" instead of one for deploy and one for everything else. // // This is the whole rule, and every command that reads the configuration goes // through it. Stating it here and applying it on only some paths is how @@ -432,10 +442,17 @@ const ( // // recorded tells absence apart from failure, and the two get different // answers. A project with no azd environment has genuinely recorded nothing, -// so ./evals is right. An azd that could not be asked has said nothing at all, -// and defaulting on that would write the second configuration all over again -- -// this time for a reason nobody could reproduce. -func evalDirCascade(flagValue string, recorded func() (string, error)) (string, error) { +// so the next level is right. An azd that could not be asked has said nothing +// at all, and defaulting on that would write the second configuration all over +// again -- this time for a reason nobody could reproduce. +// +// declared is best-effort by contrast: outside an azd project there is no +// azure.yaml to read, which is ordinary rather than a failure. +func evalDirCascade( + flagValue string, + recorded func() (string, error), + declared func() string, +) (string, error) { if flagValue != "" { return flagValue, nil } @@ -446,9 +463,48 @@ func evalDirCascade(flagValue string, recorded func() (string, error)) (string, if path != "" { return path, nil } + if declared != nil { + if dir := declared(); dir != "" { + return dir, nil + } + } return project.DefaultEvalDir, nil } +// declaredEvalConfig reads the location azure.yaml's `$ref` points at. +// +// The service entry is the project's own statement of where its evaluation +// configuration lives, and `azd up` has always deployed from it. The full path +// is returned rather than its directory: the `$ref` names a file, and a project +// declaring `./config/nightly.yaml` means that file, not whatever +// `azure.eval.yaml` happens to sit beside it. +// +// Returns empty outside an azd project, or when nothing declares the eval host. +func declaredEvalConfig(ctx context.Context, azdClient *azdext.AzdClient) string { + if azdClient == nil { + return "" + } + resp, err := azdClient.Project().Get(ctx, &azdext.EmptyRequest{}) + if err != nil || resp.GetProject() == nil { + return "" + } + for _, svc := range resp.GetProject().GetServices() { + if svc.GetHost() != project.EvalHost { + continue + } + props := svc.GetAdditionalProperties() + if props == nil { + continue + } + ref, _ := props.AsMap()["$ref"].(string) + if ref == "" { + continue + } + return filepath.Clean(filepath.FromSlash(ref)) + } + return "" +} + // evalDir is the cascade for a command that already holds an azd connection. func (ec *evalContext) evalDir(ctx context.Context, flagValue string) (string, error) { return evalDirCascade(flagValue, func() (string, error) { @@ -456,6 +512,8 @@ func (ec *evalContext) evalDir(ctx context.Context, flagValue string) (string, e return "", nil } return readRecordedEvalPath(ctx, ec.azdClient, ec.envName) + }, func() string { + return declaredEvalConfig(ctx, ec.azdClient) }) } @@ -494,6 +552,13 @@ func resolveEvalDir(ctx context.Context, flagValue string) (string, error) { return "", nil } return readRecordedEvalPath(ctx, azdClient, env.GetEnvironment().GetName()) + }, func() string { + azdClient, err := azdext.NewAzdClient() + if err != nil { + return "" + } + defer azdClient.Close() + return declaredEvalConfig(ctx, azdClient) }) } diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/eval_group.go b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/eval_group.go index 75c5f717e36..8298b0801ff 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/eval_group.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/eval_group.go @@ -74,7 +74,7 @@ func newEvalCreateCommand() *cobra.Command { baseDir := filepath.Dir(path) datasetPath := "" if decl, ok := cfg.DatasetDeclaration(eval.Dataset); ok { - datasetPath = project.ResolveSource(baseDir, decl.Source) + datasetPath = project.ResolveSource(baseDir, decl.File) } reconciler := &evalReconciler{ec: ec} diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/evaldir_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/evaldir_test.go index 14f32ba4d8d..a50de3a8a33 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/evaldir_test.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/evaldir_test.go @@ -82,20 +82,68 @@ func TestEvalDirCascadeAnswersInOrder(t *testing.T) { t.Run(tc.name, func(t *testing.T) { got, err := evalDirCascade(tc.flag, func() (string, error) { return tc.recorded, nil - }) + }, nil) require.NoError(t, err) assert.Equal(t, tc.want, got) }) } } +// azure.yaml's `$ref` is read, not only written. +// +// The recorded path lives in the azd environment, and an azd environment is not +// in the repository. Check the project out somewhere else and that level is +// empty, so a configuration the project declares under ./config was reported +// missing by every command while `azd up` deployed it from the same `$ref`. +func TestEvalDirCascadeReadsTheDeclaredRef(t *testing.T) { + got, err := evalDirCascade("", + func() (string, error) { return "", nil }, + func() string { return "config" }) + + require.NoError(t, err) + assert.Equal(t, "config", got) +} + +// The recorded path is what `--path` wrote on this machine, so it answers over +// a declaration that may predate it. +func TestEvalDirCascadePrefersTheRecordedPathOverTheDeclaredRef(t *testing.T) { + got, err := evalDirCascade("", + func() (string, error) { return "quality", nil }, + func() string { return "config" }) + + require.NoError(t, err) + assert.Equal(t, "quality", got) +} + +// Outside an azd project there is no azure.yaml to read, which is ordinary. +func TestEvalDirCascadeFallsBackWhenNothingIsDeclared(t *testing.T) { + got, err := evalDirCascade("", + func() (string, error) { return "", nil }, + func() string { return "" }) + + require.NoError(t, err) + assert.Equal(t, project.DefaultEvalDir, got) +} + +// A --path that was given is the answer on its own, so neither level is asked. +func TestEvalDirCascadeSkipsBothLookupsWhenPathWasGiven(t *testing.T) { + var declaredAsked int + got, err := evalDirCascade("./given", + func() (string, error) { return "recorded", nil }, + func() string { declaredAsked++; return "config" }) + + require.NoError(t, err) + assert.Equal(t, "./given", got) + assert.Equal(t, 0, declaredAsked, "a --path that was given should not cost a round trip") +} + // A read that failed is not a project that recorded nothing. Defaulting on it // is how `generate` would write a second configuration under ./evals for a // reason nobody could reproduce, so the failure has to come back out. func TestEvalDirCascadeDoesNotDefaultOnAFailedRead(t *testing.T) { boom := errors.New("the environment could not be read") - got, err := evalDirCascade("", func() (string, error) { return "", boom }) + got, err := evalDirCascade("", func() (string, error) { return "", boom }, nil) require.ErrorIs(t, err, boom) assert.Empty(t, got, "a failed read must not answer with the default") @@ -106,7 +154,7 @@ func TestEvalDirCascadeDoesNotDefaultOnAFailedRead(t *testing.T) { func TestEvalDirCascadeIgnoresAFailedReadWhenPathWasGiven(t *testing.T) { got, err := evalDirCascade("./given", func() (string, error) { return "", errors.New("the environment could not be read") - }) + }, nil) require.NoError(t, err) assert.Equal(t, "./given", got) @@ -118,7 +166,7 @@ func TestEvalDirCascadeAsksForTheRecordedPathOnce(t *testing.T) { got, err := evalDirCascade("", func() (string, error) { asked++ return "", nil - }) + }, nil) require.NoError(t, err) assert.Equal(t, project.DefaultEvalDir, got) @@ -128,7 +176,7 @@ func TestEvalDirCascadeAsksForTheRecordedPathOnce(t *testing.T) { _, err = evalDirCascade("./given", func() (string, error) { asked++ return "", nil - }) + }, nil) require.NoError(t, err) assert.Equal(t, 0, asked, "a --path that was given should not cost a round trip") } diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/generate_commands.go b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/generate_commands.go index 1a1b90d6fc9..6cdf390e2fe 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/generate_commands.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/generate_commands.go @@ -90,7 +90,7 @@ func resolvePlan(f *generateFlags, name string, defaultOutputDir string) (genera Agent: firstNonEmpty(f.target, declaredTarget(f.path)), Model: f.model, Instruction: instruction, - BaseDir: f.path, + BaseDir: project.EvalDirOf(f.path), OutputDir: firstNonEmpty(f.outputDir, "./"+defaultOutputDir), } if plan.Model == "" && plan.Agent == "" { diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/init.go b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/init.go index e025ee1f69a..8d5690e74ee 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/init.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/init.go @@ -137,7 +137,7 @@ func newInitCommand() *cobra.Command { // reporting it as created would claim a file it only added to. _, configExistedErr := os.Stat(configPath) configExisted := configExistedErr == nil - cfg, err := project.OpenEvalConfig(path) + cfg, err := project.OpenEvalConfigForEdit(path) if err != nil { return err } @@ -181,7 +181,7 @@ func newInitCommand() *cobra.Command { } defer unlockConfig() - cfg, err = project.OpenEvalConfig(path) + cfg, err = project.OpenEvalConfigForEdit(path) if err != nil { return err } @@ -196,10 +196,13 @@ func newInitCommand() *cobra.Command { cfg.RemoveEval(evalName) } - if err := os.MkdirAll(filepath.Join(path, project.DefaultDatasetsDir), 0o750); err != nil { + // The location may be the file azure.yaml names rather than the + // directory holding it, and artifacts sit beside the configuration. + evalDir := project.EvalDirOf(path) + if err := os.MkdirAll(filepath.Join(evalDir, project.DefaultDatasetsDir), 0o750); err != nil { return messages.CreatingDatasetsDir(err) } - if err := os.MkdirAll(filepath.Join(path, project.DefaultEvaluatorsDir), 0o750); err != nil { + if err := os.MkdirAll(filepath.Join(evalDir, project.DefaultEvaluatorsDir), 0o750); err != nil { return messages.CreatingEvaluatorsDir(err) } @@ -236,8 +239,8 @@ func newInitCommand() *cobra.Command { "eval": evalName, "evalConfig": configPath, "service": serviceName, - "datasetsDir": filepath.Join(path, project.DefaultDatasetsDir), - "evaluatorsDir": filepath.Join(path, project.DefaultEvaluatorsDir), + "datasetsDir": filepath.Join(evalDir, project.DefaultDatasetsDir), + "evaluatorsDir": filepath.Join(evalDir, project.DefaultEvaluatorsDir), "rootConfig": rootWiring, "target": target, "source": source, @@ -401,7 +404,7 @@ func planScaffold(in scaffoldInput) scaffold { } eval.Dataset = datasetName out.datasetName = datasetName - addDatasetDecl(cfg, project.DatasetDecl{Name: datasetName, Source: datasetSource}) + addDatasetDecl(cfg, project.DatasetDecl{Name: datasetName, File: datasetSource}) } // Every evaluator carries the judge deployment, because that is where the diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/init_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/init_test.go index 7eca43c1e76..8a6415de585 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/init_test.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/init_test.go @@ -370,7 +370,7 @@ func TestScaffold_DatasetReferenceForms(t *testing.T) { }) decl, ok := cfg.DatasetDeclaration("golden") require.True(t, ok) - require.Equal(t, "../tests/golden.jsonl", decl.Source, + require.Equal(t, "../tests/golden.jsonl", decl.File, "a dataset outside the eval dir must be reached with ..") require.Equal(t, "golden", plan.eval.Dataset) require.False(t, plan.generateDataset, @@ -383,7 +383,7 @@ func TestScaffold_DatasetReferenceForms(t *testing.T) { }) decl, ok := cfg.DatasetDeclaration("prod-sample") require.True(t, ok) - require.Empty(t, decl.Source, "a registered dataset must not get a local source") + require.Empty(t, decl.File, "a registered dataset must not get a local source") require.Equal(t, "prod-sample", plan.eval.Dataset) require.False(t, plan.generateDataset) }) @@ -396,7 +396,7 @@ func TestScaffold_DatasetReferenceForms(t *testing.T) { "the dataset is named after the eval") decl, ok := cfg.DatasetDeclaration("support-agent-smoke") require.True(t, ok) - require.Contains(t, decl.Source, "support-agent-smoke.jsonl") + require.Contains(t, decl.File, "support-agent-smoke.jsonl") require.True(t, plan.generateDataset) }) } diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/reconciler.go b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/reconciler.go index c06f3db7f52..705b15f07dc 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/reconciler.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/reconciler.go @@ -361,18 +361,32 @@ func (r *evalReconciler) latestDatasetVersion(ctx context.Context, name string) // EnsureEvaluator publishes a new version when the local definition differs // from what the service holds. // -// The two kinds of evaluator are told apart by the source's extension: `.py` -// is code, anything else is a rubric. They also detect change differently. A -// rubric definition comes back inline, so it is compared directly; a code -// definition's source is not read back in a form worth comparing, so a -// fingerprint of the script is kept in the azd environment, the same way -// datasets work. +// A definition reaches this three ways: written in the configuration, named as +// a file, or neither -- in which case the evaluator has to already exist on the +// service. The first two are the same publish once the rubric is in hand; they +// differ only in what there is to hash. func (r *evalReconciler) EnsureEvaluator( ctx context.Context, decl project.EvaluatorDecl, localPath string, ) (string, bool, error) { - if localPath == "" { + var body json.RawMessage + var digest string + + switch { + case decl.Definition != nil: + // Also how a `$ref` to a rubric file arrives: resolution has already + // spliced the file's keys in, so there is nothing left to read. + raw, err := json.Marshal(decl.Definition) + if err != nil { + return "", false, messages.EvaluatorProblem(decl.Name, err) + } + if body, err = normalizeRubricBody(decl.Name, raw); err != nil { + return "", false, messages.EvaluatorProblem(decl.Name, err) + } + digest = project.FingerprintBytes(body) + + case localPath == "": raw, err := r.ec.evalClient.GetEvaluatorRaw( ctx, decl.Name, decl.Version, ProjectEndpointAPIVersion, ) @@ -380,34 +394,34 @@ func (r *evalReconciler) EnsureEvaluator( return "", false, messages.EvaluatorNotLocalNorFound(decl.Name, err) } return versionFromRaw(raw, decl.Version), false, nil - } - if _, err := os.Stat(localPath); err != nil { - if errors.Is(err, fs.ErrNotExist) { - return "", false, messages.EvaluatorNotGeneratedYet(decl.Name, localPath) + default: + if _, err := os.Stat(localPath); err != nil { + if errors.Is(err, fs.ErrNotExist) { + return "", false, messages.EvaluatorNotGeneratedYet(decl.Name, localPath) + } + return "", false, messages.EvaluatorSource(localPath, err) } - return "", false, messages.EvaluatorSource(localPath, err) - } - raw, err := project.ReadFileNoBOM(localPath) - if err != nil { - return "", false, messages.EvaluatorSource(localPath, err) - } + raw, err := project.ReadFileNoBOM(localPath) + if err != nil { + return "", false, messages.EvaluatorSource(localPath, err) + } - body, err := normalizeRubricBody(decl.Name, raw) - if err != nil { - return "", false, messages.EvaluatorProblem(decl.Name, err) + if body, err = normalizeRubricBody(decl.Name, raw); err != nil { + return "", false, messages.EvaluatorProblem(decl.Name, err) + } + + if digest, err = project.Fingerprint(localPath); err != nil { + return "", false, messages.EvaluatorSource(localPath, err) + } } - // The author's own file decides whether there is anything to publish. + // The author's own definition decides whether there is anything to publish. // Comparing against the service cannot: it enriches a definition with // fields nobody authored, so sameDefinition only looks for authored keys on // the service and a key the author *deleted* — a pass_threshold, say — is // still there to be found, and the deletion never publishes. - digest, err := project.Fingerprint(localPath) - if err != nil { - return "", false, messages.EvaluatorSource(localPath, err) - } digestKey := project.FingerprintKey("evaluator", decl.Name) prior := r.ec.getEnvValue(ctx, digestKey) authorEdited := prior != "" && prior != digest diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/run.go b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/run.go index a5b4f3e5d91..ec9e9fb882a 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/run.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/run.go @@ -703,13 +703,13 @@ func localDatasetPath(configPath string, group *project.Eval) string { return "" } decl, ok := cfg.DatasetDeclaration(group.Dataset) - if !ok || decl.Source == "" { + if !ok || decl.File == "" { return "" } - if filepath.IsAbs(decl.Source) { - return decl.Source + if filepath.IsAbs(decl.File) { + return decl.File } - return filepath.Join(filepath.Dir(configPath), decl.Source) + return filepath.Join(filepath.Dir(configPath), decl.File) } // declaredDatasetVersion is the `version:` the catalog pins this dataset to. diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/run_datasource_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/run_datasource_test.go index 76948cb9fb8..426bbf58cd3 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/run_datasource_test.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/run_datasource_test.go @@ -26,7 +26,7 @@ func writeDataset(t *testing.T, rows string) string { require.NoError(t, os.MkdirAll(filepath.Join(dir, "datasets"), 0o750)) require.NoError(t, os.WriteFile(filepath.Join(dir, "datasets", "d.jsonl"), []byte(rows), 0o600)) configPath := filepath.Join(dir, "eval.yaml") - config := "datasets:\n - name: d\n source: ./datasets/d.jsonl\n" + config := "datasets:\n - name: d\n file: ./datasets/d.jsonl\n" require.NoError(t, os.WriteFile(configPath, []byte(config), 0o600)) return configPath } diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/scored_version_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/scored_version_test.go index 5402a356c38..9f0c6eec3da 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/scored_version_test.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/scored_version_test.go @@ -16,16 +16,16 @@ import ( "github.com/stretchr/testify/require" ) -// writeCatalog writes a configuration whose dataset carries the given source -// and version, either of which may be empty. -func writeCatalog(t *testing.T, source, version string) string { +// writeCatalog writes a configuration whose dataset carries the given file and +// version, either of which may be empty. +func writeCatalog(t *testing.T, file, version string) string { t.Helper() dir := t.TempDir() entry := " - name: golden" - if source != "" { - require.NoError(t, os.WriteFile(filepath.Join(dir, source), []byte("{\"query\":\"hi\"}\n"), 0o600)) - entry += "\n source: ./" + source + if file != "" { + require.NoError(t, os.WriteFile(filepath.Join(dir, file), []byte("{\"query\":\"hi\"}\n"), 0o600)) + entry += "\n file: ./" + file } if version != "" { entry += "\n version: \"" + version + "\"" @@ -44,7 +44,7 @@ evals: // The label a run carries has to follow the same branch its rows did. // -// A declaration with both `source:` and `version:` reads the file from disk, so +// A declaration with both `file:` and `version:` reads the file from disk, so // the pin says nothing about what was scored: the recorded version is the one // this file's content published, and checkDatasetRegistered has already // confirmed the rows match it. @@ -64,7 +64,7 @@ func TestARunOverALocalFileIsLabelledWithWhatTheFilePublished(t *testing.T) { } // And with no fingerprint there is nothing tying the file to that version. A -// dataset that was registered and has since gained a `source:` has a recorded +// dataset that was registered and has since gained a `file:` has a recorded // version and no fingerprint, and the rows are whatever the file now holds. func TestALocalFileWithNoFingerprintIsLabelledWithNothing(t *testing.T) { env := &testEnvServer{values: map[string]string{ diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/cmd/wiring_ref_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/wiring_ref_test.go new file mode 100644 index 00000000000..945766afcc6 --- /dev/null +++ b/cli/azd/extensions/azure.ai.evaluations/internal/cmd/wiring_ref_test.go @@ -0,0 +1,59 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package cmd + +import ( + "testing" + + "azureaieval/internal/messages" + + "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/types/known/structpb" +) + +func serviceWithRef(t *testing.T, ref string) *azdext.ServiceConfig { + t.Helper() + if ref == "" { + return &azdext.ServiceConfig{Name: "evals"} + } + props, err := structpb.NewStruct(map[string]any{"$ref": ref}) + require.NoError(t, err) + return &azdext.ServiceConfig{Name: "evals", AdditionalProperties: props} +} + +// The `$ref` an entry already carries decides whether the wiring is present, +// and it is compared as a path rather than as text. +// +// Matching on name and host alone reported the wiring present after +// `init --path` moved the configuration, and `azd up` went on deploying the +// file left behind. Comparing the text alone would have called +// `evals/azure.eval.yaml` and `./evals/azure.eval.yaml` two different answers. +func TestServiceRefIsComparedAsAPath(t *testing.T) { + assert.True(t, sameRefTarget("./evals/azure.eval.yaml", "evals/azure.eval.yaml"), + "the same file written two ways is one answer") + assert.True(t, sameRefTarget("evals/../evals/azure.eval.yaml", "./evals/azure.eval.yaml")) + assert.False(t, sameRefTarget("./evals/azure.eval.yaml", "./quality/azure.eval.yaml"), + "a different file is what the guard exists to catch") +} + +// An entry with no `$ref` has nothing to disagree with. +func TestServiceConfigRefReadsTheDeclaredValue(t *testing.T) { + assert.Equal(t, "./evals/azure.eval.yaml", + serviceConfigRef(serviceWithRef(t, "./evals/azure.eval.yaml"))) + assert.Empty(t, serviceConfigRef(serviceWithRef(t, ""))) +} + +// The refusal names both paths, because the reader is the one who has to decide +// which of the two configurations they meant to keep. +func TestServiceRefConflictNamesBothPaths(t *testing.T) { + err := messages.ServiceRefPointsElsewhere( + "support-agent-evals", "./evals/azure.eval.yaml", "./quality/azure.eval.yaml") + + require.Error(t, err) + assert.Contains(t, err.Error(), "./evals/azure.eval.yaml") + assert.Contains(t, err.Error(), "./quality/azure.eval.yaml") + assert.Contains(t, err.Error(), "support-agent-evals") +} diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/messages/messages.go b/cli/azd/extensions/azure.ai.evaluations/internal/messages/messages.go index e77445512dc..26f5d2e0506 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/messages/messages.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/messages/messages.go @@ -2005,6 +2005,27 @@ func EvaluatorVersionWithSource(index int, evaluator string) error { "on the project", index, evaluator) } +// EvaluatorVersionWithDefinition reports the same pin against a rubric written +// out in the configuration rather than named as a file. +func EvaluatorVersionWithDefinition(index int, evaluator string) error { + return fmt.Errorf( + "evaluators[%d] (%s): `version` cannot be set with `definition`, because "+ + "the service assigns the version when it publishes. Drop `version` to "+ + "publish this rubric, or drop `definition` to reference a version "+ + "already on the project", index, evaluator) +} + +// EvaluatorRubricDeclaredTwice reports a rubric both named and written out. +// +// Publishing uses the written one, so leaving this to the schema alone would +// mean the file quietly never got read. +func EvaluatorRubricDeclaredTwice(index int, evaluator string) error { + return fmt.Errorf( + "evaluators[%d] (%s): `source` and `definition` both give the rubric; "+ + "declare one. Keep `definition` to publish what is written here, or "+ + "keep `source` to publish the file it names", index, evaluator) +} + // DatasetAndSourceDeclareTheSameThing reports it where there is no index. func DatasetAndSourceDeclareTheSameThing() error { return errors.New("`dataset` and `source` both say where rows come from; declare one") diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/anchor_ref_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/anchor_ref_test.go new file mode 100644 index 00000000000..4e191dfe8fd --- /dev/null +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/anchor_ref_test.go @@ -0,0 +1,54 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package project + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Resolving a `$ref` round-trips the document through a map, which expands YAML +// anchors. The expansion has to be faithful, because the alias is how authors +// avoid repeating a judge model across evaluators. +func TestAnchorsSurviveRefResolution(t *testing.T) { + dir := t.TempDir() + + require.NoError(t, os.MkdirAll(filepath.Join(dir, "evaluators"), 0o755)) + require.NoError(t, os.WriteFile( + filepath.Join(dir, "evaluators", "quality.json"), + []byte(`{"type":"rubric","dimensions":[{"id":"tone","weight":3}]}`), + 0o600)) + + path := filepath.Join(dir, EvalConfigBase) + require.NoError(t, os.WriteFile(path, []byte(` +evaluators: + - $ref: ./evaluators/quality.json + name: quality + +evals: + - name: nightly + dataset: golden + evaluators: + - evaluator: builtin.relevance + initialization_parameters: &judge + model: gpt-5.6-luna + - evaluator: builtin.coherence + initialization_parameters: *judge +`), 0o600)) + + cfg, err := LoadEvalConfig(path) + require.NoError(t, err, "an anchor is not a mistyped key") + require.Len(t, cfg.Evals, 1) + require.Len(t, cfg.Evals[0].Evaluators, 2) + + assert.Equal(t, cfg.Evals[0].Evaluators[0].InitializationParameters, + cfg.Evals[0].Evaluators[1].InitializationParameters, + "the alias has to carry the same parameters the anchor declared") + assert.NotEmpty(t, cfg.Evals[0].Evaluators[1].InitializationParameters, + "an alias that expanded to nothing would silently drop the judge model") +} diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/config_keys_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/config_keys_test.go index 1711765f7ca..22fb17482de 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/project/config_keys_test.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/config_keys_test.go @@ -80,10 +80,17 @@ func TestSourceDeclKeys(t *testing.T) { yamlKeys(t, SourceDecl{})) } -// The catalogs are named, reusable assets: a name and where it comes from. +// The catalogs are named, reusable assets. A dataset says where its rows come +// from. An evaluator says where its rubric is -- named as a file, or written +// out under `definition` -- and may instead be pulled in with `$ref`, which is +// modelled rather than only resolved so that a command which reads, modifies +// and saves the file writes the author's include back out instead of inlining +// it. func TestCatalogKeys(t *testing.T) { - assert.ElementsMatch(t, []string{"name", "source", "version"}, yamlKeys(t, DatasetDecl{})) - assert.ElementsMatch(t, []string{"name", "source", "version"}, yamlKeys(t, EvaluatorDecl{})) + assert.ElementsMatch(t, []string{"name", "file", "version"}, yamlKeys(t, DatasetDecl{})) + assert.ElementsMatch(t, + []string{"$ref", "name", "source", "version", "definition"}, + yamlKeys(t, EvaluatorDecl{})) } // The spec's casing table: eval.yaml uses the API's snake_case throughout, so diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/config_location_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/config_location_test.go new file mode 100644 index 00000000000..326b4f19984 --- /dev/null +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/config_location_test.go @@ -0,0 +1,78 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package project + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// azure.yaml's `$ref` names a file, not a directory. +// +// A project is free to declare `./config/nightly.yaml`. Reading only the +// directory out of that and then looking for `azure.eval.yaml` beside it +// reports the configuration missing while `azd up` deploys it from the very +// same `$ref` -- the two-answers-to-one-question shape this cascade exists to +// close. +func TestLocationMayNameTheFileTheRefDeclares(t *testing.T) { + dir := t.TempDir() + declared := filepath.Join(dir, "nightly.yaml") + require.NoError(t, os.WriteFile(declared, []byte("evals:\n - name: nightly\n"), 0o600)) + + path, err := ResolveEvalConfigPath(declared) + require.NoError(t, err) + assert.Equal(t, declared, path, "the declared file is the configuration") + + cfg, err := OpenEvalConfig(declared) + require.NoError(t, err) + require.NotNil(t, cfg) + require.Len(t, cfg.Evals, 1) + assert.Equal(t, "nightly", cfg.Evals[0].Name) + + assert.Equal(t, dir, EvalDirOf(declared), + "artifacts sit beside the configuration, not inside it") +} + +// A directory keeps the naming convention it always had. +func TestLocationMayStillBeTheDirectory(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile( + filepath.Join(dir, EvalConfigBase), []byte("evals:\n - name: nightly\n"), 0o600)) + + path, err := ResolveEvalConfigPath(dir) + require.NoError(t, err) + assert.Equal(t, filepath.Join(dir, EvalConfigBase), path) + assert.Equal(t, dir, EvalDirOf(dir)) +} + +// A location that does not exist yet is a directory: it is what `init` is given +// before it writes anything. +func TestAMissingLocationIsReadAsADirectory(t *testing.T) { + dir := filepath.Join(t.TempDir(), "not-created-yet") + + assert.Equal(t, filepath.Join(dir, EvalConfigBase), EvalConfigPath(dir)) + assert.Equal(t, dir, EvalDirOf(dir)) +} + +// The both-names guard is about a directory holding two candidates. A location +// that already names the file has nothing to disambiguate. +func TestADeclaredFileSkipsTheAmbiguityGuard(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile( + filepath.Join(dir, EvalConfigBase), []byte("evals: []\n"), 0o600)) + require.NoError(t, os.WriteFile( + filepath.Join(dir, LegacyEvalConfigBase), []byte("evals: []\n"), 0o600)) + + _, err := ResolveEvalConfigPath(dir) + require.Error(t, err, "a directory holding both names is still refused") + + declared := filepath.Join(dir, EvalConfigBase) + got, err := ResolveEvalConfigPath(declared) + require.NoError(t, err, "naming the file is how a project says which one it means") + assert.Equal(t, declared, got) +} diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/edit_read_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/edit_read_test.go new file mode 100644 index 00000000000..ef0b5cdc33d --- /dev/null +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/edit_read_test.go @@ -0,0 +1,93 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package project + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// A command that writes the configuration back reads it as written. +// +// `init` and `generate` read, modify and save the same file. Handing them a +// resolved configuration inlined the author's includes, orphaned the files they +// named, and left the paths inside those files resolving against the wrong +// directory: a `source: ./quality.json` written beside `evaluators/quality.yaml` +// came back pointing at the project root. Nothing reported it, because from the +// writer's point of view it had saved what it read. +func TestEditingReadsLeaveIncludesAlone(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "evaluators"), 0o755)) + require.NoError(t, os.WriteFile( + filepath.Join(dir, "evaluators", "quality.yaml"), + []byte("name: quality\nsource: ./quality.json\n"), 0o600)) + + path := filepath.Join(dir, EvalConfigBase) + require.NoError(t, os.WriteFile(path, []byte(`datasets: + - name: golden + file: ./datasets/golden.jsonl + +evaluators: + - $ref: ./evaluators/quality.yaml + +evals: + - name: nightly + dataset: golden +`), 0o600)) + + cfg, err := OpenEvalConfigForEdit(dir) + require.NoError(t, err) + require.NotNil(t, cfg) + require.NoError(t, SaveEvalConfig(dir, cfg)) + + after, err := os.ReadFile(path) + require.NoError(t, err) + text := string(after) + + assert.Contains(t, text, "$ref: ./evaluators/quality.yaml", + "the author's include has to survive a command that saves the file") + assert.NotContains(t, text, "source: ./quality.json", + "inlining it would leave that path resolving against the wrong directory") +} + +// The reader that commands *use* still resolves, so the two do not drift apart. +func TestConsumingReadsStillResolve(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "evaluators"), 0o755)) + require.NoError(t, os.WriteFile( + filepath.Join(dir, "evaluators", "quality.yaml"), + []byte("name: quality\nsource: ./quality.json\n"), 0o600)) + + require.NoError(t, os.WriteFile(filepath.Join(dir, EvalConfigBase), []byte(`evaluators: + - $ref: ./evaluators/quality.yaml + +evals: + - name: nightly +`), 0o600)) + + cfg, err := OpenEvalConfig(dir) + require.NoError(t, err) + require.Len(t, cfg.Evaluators, 1) + assert.Equal(t, "quality", cfg.Evaluators[0].Name) +} + +// An include is the only thing the editing reader treats differently. A +// configuration without one decodes identically either way, so a mistyped key +// is still refused on the path that writes. +func TestEditingReadsAreStillStrict(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, EvalConfigBase), + []byte("datasets:\n - name: golden\n fiel: ./x.jsonl\n"), 0o600)) + + _, err := OpenEvalConfigForEdit(dir) + + require.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "fiel"), + "the typo has to be named on the path that saves the file too") +} diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config.go index 643f84c6ceb..4e46395be50 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config.go @@ -26,11 +26,15 @@ type EvalConfig struct { Evals []Eval `yaml:"evals,omitempty" json:"evals,omitempty"` } -// DatasetDecl is a catalog entry. A local Source is uploaded on deploy; without +// DatasetDecl is a catalog entry. A local File is uploaded on deploy; without // one the name must already resolve to a registered dataset. +// +// Deliberately not a `$ref`: that directive replaces a definition with one +// loaded from a YAML or JSON file, and these rows are an artifact to publish. +// A `.jsonl` is neither, so there would be nothing to splice. type DatasetDecl struct { Name string `yaml:"name" json:"name"` - Source string `yaml:"source,omitempty" json:"source,omitempty"` + File string `yaml:"file,omitempty" json:"file,omitempty"` Version string `yaml:"version,omitempty" json:"version,omitempty"` } @@ -39,10 +43,25 @@ type DatasetDecl struct { // // Source names a `.json` file holding a rubric: a list of weighted scoring // dimensions. +// +// Ref carries the `$ref` an author wrote. Commands that use the configuration +// never see it -- resolution has already replaced the entry with the file's +// content by then -- but the commands that read, modify and save the file do, +// and modelling it is what lets the include survive being written back. Name is +// omitempty for the same reason: an entry that is only a `$ref` has no name of +// its own until the file it names supplies one. +// +// Definition is the rubric written out in place of naming a file, which is what +// lets a `$ref` name a rubric: resolution splices the file's keys in here, and +// they have to land on a field to survive strict decoding. It is deliberately +// one named key rather than a catch-all: a catch-all would swallow every +// misspelling in the entry and publish it to the service as rubric content. type EvaluatorDecl struct { - Name string `yaml:"name" json:"name"` - Source string `yaml:"source,omitempty" json:"source,omitempty"` - Version string `yaml:"version,omitempty" json:"version,omitempty"` + Ref string `yaml:"$ref,omitempty" json:"$ref,omitempty"` + Name string `yaml:"name,omitempty" json:"name,omitempty"` + Source string `yaml:"source,omitempty" json:"source,omitempty"` + Version string `yaml:"version,omitempty" json:"version,omitempty"` + Definition map[string]any `yaml:"definition,omitempty" json:"definition,omitempty"` } // Eval is one evaluation defined over the catalogs. @@ -205,7 +224,7 @@ func (c *EvalConfig) CustomEvaluators() []EvaluatorDecl { func (c *EvalConfig) LocalDatasets() []DatasetDecl { var owned []DatasetDecl for _, decl := range c.Datasets { - if decl.Source == "" { + if decl.File == "" { continue } owned = append(owned, decl) @@ -324,6 +343,14 @@ func (c *EvalConfig) validateCatalogs() error { if e.Source != "" && e.Version != "" { return messages.EvaluatorVersionWithSource(i, e.Name) } + if e.Definition != nil { + if e.Source != "" { + return messages.EvaluatorRubricDeclaredTwice(i, e.Name) + } + if e.Version != "" { + return messages.EvaluatorVersionWithDefinition(i, e.Name) + } + } } return nil } diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_ambiguity_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_ambiguity_test.go index cd41c8d87a8..7560d1b81b7 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_ambiguity_test.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_ambiguity_test.go @@ -17,7 +17,7 @@ func writeFile(t *testing.T, dir, name, body string) { require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte(body), 0o600)) } -const oneEvalConfig = "datasets:\n - name: d\n source: ./d.jsonl\n" +const oneEvalConfig = "datasets:\n - name: d\n file: ./d.jsonl\n" // azure.yaml references one configuration by name. With both files present the // CLI would edit whichever it preferred while azd up deployed whichever the @@ -54,7 +54,7 @@ func TestSaveEvalConfig_WritesBackToTheLegacyFile(t *testing.T) { writeFile(t, dir, LegacyEvalConfigBase, oneEvalConfig) require.NoError(t, SaveEvalConfig(dir, &EvalConfig{ - Datasets: []DatasetDecl{{Name: "d", Source: "./d.jsonl"}}, + Datasets: []DatasetDecl{{Name: "d", File: "./d.jsonl"}}, })) assert.FileExists(t, filepath.Join(dir, LegacyEvalConfigBase)) @@ -77,7 +77,7 @@ func TestSaveEvalConfig_WritesTheCurrentNameWhenThereIsNoFile(t *testing.T) { func TestValidate_RefusesATargetWithNoName(t *testing.T) { dir := t.TempDir() writeFile(t, dir, EvalConfigBase, - "datasets:\n - name: d\n source: ./d.jsonl\n"+ + "datasets:\n - name: d\n file: ./d.jsonl\n"+ "evals:\n - name: e\n dataset: d\n"+ " target:\n type: agent\n"+ " evaluators:\n - evaluator: builtin.relevance\n") diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_atomic_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_atomic_test.go index f41a20dcd66..a7312ae629a 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_atomic_test.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_atomic_test.go @@ -30,7 +30,7 @@ func TestSaveEvalConfigNeverExposesAHalfWrittenFile(t *testing.T) { path := filepath.Join(dir, "azure.eval.yaml") full := &EvalConfig{ - Datasets: []DatasetDecl{{Name: "golden", Source: "./datasets/golden.jsonl"}}, + Datasets: []DatasetDecl{{Name: "golden", File: "./datasets/golden.jsonl"}}, Evals: []Eval{ {Name: "first", EvaluationLevel: "turn"}, {Name: "second", EvaluationLevel: "turn"}, diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_keys_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_keys_test.go index fc3338e994e..8165eb60b06 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_keys_test.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_keys_test.go @@ -41,7 +41,7 @@ func TestExplainUnknownKeys_LeavesOtherErrors(t *testing.T) { func TestKeysOfTypeCoversTheDeclarations(t *testing.T) { assert.Contains(t, keysOfType("project.Eval"), "evaluators") assert.Contains(t, keysOfType("project.EvalConfig"), "datasets") - assert.Contains(t, keysOfType("project.DatasetDecl"), "source") + assert.Contains(t, keysOfType("project.DatasetDecl"), "file") assert.Empty(t, keysOfType("project.Unknown")) } diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_name_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_name_test.go index b74ab1cdf96..f1570a5cc6c 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_name_test.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_name_test.go @@ -12,7 +12,7 @@ import ( "github.com/stretchr/testify/require" ) -const minimalConfig = "datasets:\n - name: golden\n source: ./datasets/golden.jsonl\n" +const minimalConfig = "datasets:\n - name: golden\n file: ./datasets/golden.jsonl\n" // The file is named for azd, the way azure.yaml is. func TestEvalConfigPath_IsTheAzdPrefixedName(t *testing.T) { @@ -44,7 +44,7 @@ func TestSaveEvalConfig_WritesBackOverALegacyFile(t *testing.T) { require.NoError(t, os.WriteFile(legacy, []byte(minimalConfig), 0o600)) require.NoError(t, SaveEvalConfig(dir, &EvalConfig{ - Datasets: []DatasetDecl{{Name: "added", Source: "./datasets/added.jsonl"}}, + Datasets: []DatasetDecl{{Name: "added", File: "./datasets/added.jsonl"}}, })) body, err := os.ReadFile(legacy) diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_roundtrip_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_roundtrip_test.go index 27cbcc1fe27..f47afeb9bc8 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_roundtrip_test.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_roundtrip_test.go @@ -29,7 +29,7 @@ func TestEvalConfigRoundTripKeepsWhatTheAuthorWrote(t *testing.T) { - evaluator: builtin.task_adherence datasets: - name: golden - source: ./datasets/golden.jsonl + file: ./datasets/golden.jsonl ` require.NoError(t, os.WriteFile(path, []byte(authored), 0o600)) @@ -77,7 +77,7 @@ func TestEvalConfigAcceptsEveryKeyItWrites(t *testing.T) { require.NoError(t, os.WriteFile(path, []byte(`datasets: - name: golden - source: ./datasets/golden.jsonl + file: ./datasets/golden.jsonl version: "2" evaluators: - name: quality diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_store.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_store.go index aefd74e551d..258f9077de0 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_store.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_store.go @@ -16,6 +16,7 @@ import ( "azureaieval/internal/messages" + "github.com/azure/azure-dev/cli/azd/pkg/foundry" "go.yaml.in/yaml/v3" ) @@ -39,14 +40,39 @@ const EvalConfigBase = "azure.eval.yaml" // not silently grow a second configuration beside it. const LegacyEvalConfigBase = "eval.yaml" -// EvalConfigPath is the configuration file inside an eval directory. It is -// exported for error messages and for the azure.yaml $ref; readers should -// prefer OpenEvalConfig. -func EvalConfigPath(evalDir string) string { - return filepath.Join(evalDir, EvalConfigBase) +// EvalConfigPath is the configuration file at a location. It is exported for +// error messages and for the azure.yaml $ref; readers should prefer +// OpenEvalConfig. +// +// A location is normally the eval directory, and the file inside it is named by +// convention. It may also be the file itself, because azure.yaml's `$ref` names +// one by name rather than by directory: a project is free to declare +// `./config/nightly.yaml`, and looking for `azure.eval.yaml` beside it would +// report the configuration missing while `azd up` deployed it. +func EvalConfigPath(location string) string { + if namesAFile(location) { + return location + } + return filepath.Join(location, EvalConfigBase) +} + +// EvalDirOf is the directory a location's relative paths resolve against. +func EvalDirOf(location string) string { + if namesAFile(location) { + return filepath.Dir(location) + } + return location +} + +// namesAFile reports whether a location is the configuration file rather than +// the directory holding it. A path that does not exist is read as a directory, +// which is what `init` is given before it writes anything. +func namesAFile(location string) bool { + info, err := os.Stat(location) + return err == nil && !info.IsDir() } -// ResolveEvalConfigPath is the configuration this directory actually holds: +// ResolveEvalConfigPath is the configuration this location actually holds: // the current name, or the legacy one when that is the only file there. // // It refuses a directory holding both, rather than leaving that to the caller. @@ -55,21 +81,24 @@ func EvalConfigPath(evalDir string) string { // while `run`, `init` and `generate` all refused. Returning an error is what // makes the guard unavoidable: there is no longer a way to ask this question // and not be told. -func ResolveEvalConfigPath(evalDir string) (string, error) { - if err := checkOneConfig(evalDir); err != nil { +func ResolveEvalConfigPath(location string) (string, error) { + if err := checkOneConfig(location); err != nil { return "", err } - return resolvedConfigPath(evalDir), nil + return resolvedConfigPath(location), nil } // resolvedConfigPath is the naming rule on its own, for the two functions that // have already applied the guard. -func resolvedConfigPath(evalDir string) string { - current := EvalConfigPath(evalDir) +func resolvedConfigPath(location string) string { + if namesAFile(location) { + return location + } + current := EvalConfigPath(location) if _, err := os.Stat(current); err == nil { return current } - legacy := filepath.Join(evalDir, LegacyEvalConfigBase) + legacy := filepath.Join(location, LegacyEvalConfigBase) if _, err := os.Stat(legacy); err == nil { return legacy } @@ -80,10 +109,14 @@ func resolvedConfigPath(evalDir string) string { // // Preferring one silently is the dangerous answer: `azure.yaml` `$ref`s a // single file by name, so the CLI would edit one configuration while `azd up` -// deployed the other, and nothing would say so. -func checkOneConfig(evalDir string) error { - current := EvalConfigPath(evalDir) - legacy := filepath.Join(evalDir, LegacyEvalConfigBase) +// deployed the other, and nothing would say so. A location that already names +// the file has nothing to disambiguate. +func checkOneConfig(location string) error { + if namesAFile(location) { + return nil + } + current := EvalConfigPath(location) + legacy := filepath.Join(location, LegacyEvalConfigBase) if _, err := os.Stat(current); err != nil { return nil } @@ -93,36 +126,176 @@ func checkOneConfig(evalDir string) error { return messages.AmbiguousEvalConfig(current, legacy) } -// OpenEvalConfig reads the configuration under evalDir. +// OpenEvalConfig reads the configuration at a location, with `$ref` includes +// resolved. This is the reader for commands that *use* the configuration. // // A missing file returns (nil, nil): generate runs before init, so "no // configuration yet" is an ordinary state rather than a failure. -func OpenEvalConfig(evalDir string) (*EvalConfig, error) { - if err := checkOneConfig(evalDir); err != nil { +// +// Commands that write the configuration back must use OpenEvalConfigForEdit +// instead. Resolution and editing do not mix: what comes back here is the +// configuration with every include spliced in, and saving that replaces the +// author's `$ref` with its content. +func OpenEvalConfig(location string) (*EvalConfig, error) { + return openEvalConfig(location, true) +} + +// OpenEvalConfigForEdit reads the configuration exactly as written, leaving +// `$ref` directives alone. +// +// `init` and `generate` read, modify and write the same file. Handing them a +// resolved configuration and saving the result inlined the author's includes, +// orphaned the files they named, and left the paths inside those files +// resolving against the wrong directory -- a `source: ./quality.json` written +// beside `evaluators/quality.yaml` came back pointing at the project root. +// None of it was reported, because from the writer's point of view it had +// simply saved what it read. +func OpenEvalConfigForEdit(location string) (*EvalConfig, error) { + return openEvalConfig(location, false) +} + +func openEvalConfig(location string, resolve bool) (*EvalConfig, error) { + if err := checkOneConfig(location); err != nil { return nil, err } - cfg, err := LoadEvalConfig(resolvedConfigPath(evalDir)) + cfg, err := loadEvalConfig(resolvedConfigPath(location), resolve) if errors.Is(err, fs.ErrNotExist) { return nil, nil } return cfg, err } -// LoadEvalConfig reads a configuration from an explicit path. The path is used -// verbatim, relative to the process working directory — never re-rooted. +// LoadEvalConfig reads a configuration from an explicit path, with `$ref` +// includes resolved. The path is used verbatim, relative to the process working +// directory — never re-rooted. // // Decoded strictly: a key this extension does not know is a typo, and reading // it as nothing leaves a configuration that looks fine and fails later // somewhere else. `agent:` written under `target:` instead of `type:`/`name:` // used to produce an empty target and a run that complained about the target. func LoadEvalConfig(path string) (*EvalConfig, error) { + return loadEvalConfig(path, true) +} + +func loadEvalConfig(path string, resolve bool) (*EvalConfig, error) { data, err := ReadFileNoBOM(path) if err != nil { return nil, messages.ReadingEvalConfig(path, err) } + if resolve { + data, err = resolveConfigRefs(data, filepath.Dir(path), path) + if err != nil { + return nil, err + } + } return DecodeEvalConfig(data, path) } +// resolveConfigRefs expands `$ref` includes before the strict decode. +// +// Core owns the resolver but does not run it for us: it hands each extension +// the entry with `$ref` still in it. The service target has always called it, +// and this path did not, so `azd up` accepted an include that every CLI command +// then refused as an unknown key — the same file meaning two different things +// depending on which command opened it. +// +// A configuration with no `$ref` is returned untouched rather than round-tripped +// through a map, so the overwhelmingly common case keeps the decoder's own line +// numbers in its diagnostics. +func resolveConfigRefs(data []byte, baseDir, name string) ([]byte, error) { + if !bytes.Contains(data, []byte("$ref")) { + return data, nil + } + + var raw map[string]any + if err := yaml.Unmarshal(data, &raw); err != nil { + return nil, messages.ParsingEvalConfig(name, err) + } + if raw == nil { + return data, nil + } + + resolved, err := resolveEvalRefs(raw, baseDir) + if err != nil { + return nil, err + } + + out, err := yaml.Marshal(resolved) + if err != nil { + return nil, messages.ParsingEvalConfig(name, err) + } + return out, nil +} + +// resolveEvalRefs is the one place `$ref` is resolved, so the CLI and `azd up` +// cannot disagree about what an include means. +// +// They read the configuration by different routes -- off disk, and out of the +// service entry -- and each used to resolve for itself. Every rule then had to +// be added twice, and twice it was not: an include `azd up` accepted and every +// CLI command refused, and later the reverse. Callers differ in how they obtain +// the map and what they do with it; everything between is here. +func resolveEvalRefs(values map[string]any, baseDir string) (map[string]any, error) { + resolved, err := foundry.ResolveFileRefs(values, baseDir) + if err != nil { + return nil, messages.ResolvingServiceRefs(err) + } + // `$ref` is a directive rather than configuration, and the strict decoder + // would report the leftover as a mistyped key. + delete(resolved, "$ref") + nestSplicedRubrics(resolved) + return resolved, nil +} + +// evaluatorDeclKeys are the keys an evaluator entry declares in its own right. +// Anything else at entry level was spliced in by a `$ref`. +var evaluatorDeclKeys = map[string]bool{ + "$ref": true, "name": true, "source": true, "version": true, "definition": true, +} + +// nestSplicedRubrics moves a rubric that a `$ref` spliced in at entry level +// down under `definition`. +// +// `$ref` splices the referenced file's top-level keys into the entry, and a +// rubric file is a bare `{type, dimensions}` -- the shape `generate` downloads +// from the service -- so its keys land beside `name` and the strict decoder +// rejects them. Moving them is what lets a `$ref` name a rubric. +// +// `dimensions` is what marks the leftovers as a rubric rather than a typo, and +// it is the same key normalizeRubricBody insists on before it will treat a +// document as a definition. Without that gate this would be a catch-all by +// another name, filing a misspelled `name` as rubric content and publishing it +// to the service instead of reporting it. +// +// Structural rather than positional on purpose: an earlier version marked +// entries by index before resolution, which cannot see the evaluators inside a +// config that is itself behind a `$ref` -- the layout the README documents. +func nestSplicedRubrics(resolved map[string]any) { + entries, _ := resolved["evaluators"].([]any) + for _, entry := range entries { + m, ok := entry.(map[string]any) + if !ok { + continue + } + // A file already shaped `{name, definition}` needs no rescue, and + // merging into it would guess at which one the author meant. + if _, has := m["definition"]; has { + continue + } + if _, isRubric := m["dimensions"]; !isRubric { + continue + } + rubric := map[string]any{} + for key, value := range m { + if !evaluatorDeclKeys[key] { + rubric[key] = value + delete(m, key) + } + } + m["definition"] = rubric + } +} + // DecodeEvalConfig is the one strict decoder, so every route into a // configuration reports a mistyped key the same way. // diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_test.go index ba81f5fcdf3..4a49cd660db 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_test.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/eval_config_test.go @@ -16,7 +16,7 @@ import ( const sampleEvalConfig = ` datasets: - name: support-golden - source: ./datasets/support-golden.jsonl + file: ./datasets/support-golden.jsonl version: "1" - name: prod-registered @@ -64,7 +64,7 @@ func TestLoadEvalConfig_ParsesAllSections(t *testing.T) { require.Len(t, cfg.Datasets, 2) require.Equal(t, "support-golden", cfg.Datasets[0].Name) - require.Equal(t, "./datasets/support-golden.jsonl", cfg.Datasets[0].Source) + require.Equal(t, "./datasets/support-golden.jsonl", cfg.Datasets[0].File) require.Equal(t, "1", cfg.Datasets[0].Version) require.Len(t, cfg.Evaluators, 1) @@ -159,7 +159,7 @@ func TestDeclarationLookups(t *testing.T) { ds, ok := cfg.DatasetDeclaration("support-golden") require.True(t, ok) - require.Equal(t, "./datasets/support-golden.jsonl", ds.Source) + require.Equal(t, "./datasets/support-golden.jsonl", ds.File) _, ok = cfg.DatasetDeclaration("missing") require.False(t, ok) @@ -193,7 +193,7 @@ func TestOpenEvalConfig_MissingIsNotAnError(t *testing.T) { func TestSaveEvalConfig_CreatesTheDirectory(t *testing.T) { dir := filepath.Join(t.TempDir(), "evals") require.NoError(t, SaveEvalConfig(dir, &EvalConfig{ - Datasets: []DatasetDecl{{Name: "generated", Source: "./datasets/generated.jsonl"}}, + Datasets: []DatasetDecl{{Name: "generated", File: "./datasets/generated.jsonl"}}, })) cfg, err := OpenEvalConfig(dir) @@ -311,7 +311,7 @@ func TestValidate_Rejects(t *testing.T) { }, { name: "dataset without a name", - body: "datasets:\n - source: ./d.jsonl\n" + oneEval, + body: "datasets:\n - file: ./d.jsonl\n" + oneEval, wantErr: "'name' is required", }, { @@ -329,6 +329,18 @@ func TestValidate_Rejects(t *testing.T) { body: "evaluators:\n - name: q\n source: ./q.json\n version: \"3\"\n" + oneEval, wantErr: "cannot be set with `source`", }, + { + name: "rubric both named and written out", + body: "evaluators:\n - name: q\n source: ./q.json\n" + + " definition:\n dimensions: []\n" + oneEval, + wantErr: "both give the rubric", + }, + { + name: "version pinned alongside a definition", + body: "evaluators:\n - name: q\n version: \"3\"\n" + + " definition:\n dimensions: []\n" + oneEval, + wantErr: "cannot be set with `definition`", + }, { name: "no evals", body: "datasets:\n - name: d\n", diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/max_samples_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/max_samples_test.go index 218f5c159bb..5e65bd60a06 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/project/max_samples_test.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/max_samples_test.go @@ -17,7 +17,7 @@ import ( // the opposite of what a cap asks for, and with nothing said about it. func TestNegativeMaxSamplesIsRefused(t *testing.T) { cfg := &EvalConfig{ - Datasets: []DatasetDecl{{Name: "golden", Source: "./datasets/golden.jsonl"}}, + Datasets: []DatasetDecl{{Name: "golden", File: "./datasets/golden.jsonl"}}, Evals: []Eval{{ Name: "support-quality", Dataset: "golden", @@ -38,7 +38,7 @@ func TestNegativeMaxSamplesIsRefused(t *testing.T) { // Zero is how a config says "send every row", and has to keep working. func TestUnsetMaxSamplesIsStillAllowed(t *testing.T) { cfg := &EvalConfig{ - Datasets: []DatasetDecl{{Name: "golden", Source: "./datasets/golden.jsonl"}}, + Datasets: []DatasetDecl{{Name: "golden", File: "./datasets/golden.jsonl"}}, Evals: []Eval{{ Name: "support-quality", Dataset: "golden", diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/nested_ref_rubric_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/nested_ref_rubric_test.go new file mode 100644 index 00000000000..58fbaefcd88 --- /dev/null +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/nested_ref_rubric_test.go @@ -0,0 +1,74 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package project + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The documented layout puts the whole eval config behind a `$ref` on the +// service, and the evaluators inside it carry `$ref`s of their own. +func TestARefdRubricInsideARefdConfig(t *testing.T) { + dir := t.TempDir() + evals := filepath.Join(dir, "evals") + require.NoError(t, os.MkdirAll(filepath.Join(evals, "evaluators"), 0o755)) + + require.NoError(t, os.WriteFile( + filepath.Join(evals, "evaluators", "quality.json"), + []byte(`{"type":"rubric","dimensions":[{"id":"tone","weight":3}]}`), + 0o600)) + + require.NoError(t, os.WriteFile( + filepath.Join(evals, EvalConfigBase), []byte(` +evaluators: + - $ref: ./evaluators/quality.json + name: quality + +evals: + - name: nightly + dataset: golden +`), 0o600)) + + svc := serviceWith(t, map[string]any{"$ref": "./evals/" + EvalConfigBase}) + + cfg, err := EvalConfigFromService(svc, dir) + require.NoError(t, err, "the layout the README and spec document has to deploy") + require.Len(t, cfg.Evaluators, 1) + assert.Equal(t, "quality", cfg.Evaluators[0].Name) + assert.Equal(t, "rubric", cfg.Evaluators[0].Definition["type"]) +} + +// `dimensions` is what tells a spliced rubric from a mistake, so a `$ref` to a +// file that is not a rubric is still reported rather than filed away. +// +// This is the whole difference between the rescue and a catch-all, so it is +// worth a test of its own: widening the gate would make every misspelling in an +// evaluator entry publishable content. +func TestARefToSomethingThatIsNotARubricIsStillRejected(t *testing.T) { + dir := t.TempDir() + + require.NoError(t, os.MkdirAll(filepath.Join(dir, "evaluators"), 0o755)) + require.NoError(t, os.WriteFile( + filepath.Join(dir, "evaluators", "quality.json"), + []byte(`{"nmae":"quality","weight":3}`), 0o600)) + + path := filepath.Join(dir, EvalConfigBase) + require.NoError(t, os.WriteFile(path, []byte(` +evaluators: + - $ref: ./evaluators/quality.json + +evals: + - name: nightly + dataset: golden +`), 0o600)) + + _, err := LoadEvalConfig(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "nmae", "the error has to name the key that is wrong") +} diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/one_resolver_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/one_resolver_test.go new file mode 100644 index 00000000000..d2c30404750 --- /dev/null +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/one_resolver_test.go @@ -0,0 +1,54 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package project + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// `$ref` is resolved in exactly one place. +// +// The CLI and `azd up` reach a configuration by different routes and each used +// to resolve for itself, so every rule had to be added twice -- and twice it +// was not, each time producing an include one route accepted and the other +// refused. A second caller of ResolveFileRefs is how that comes back, and it +// comes back silently, so it is worth failing the build over. +func TestRefsAreResolvedInOnePlace(t *testing.T) { + const resolver = "resolveEvalRefs" + + callers := map[string][]int{} + require.NoError(t, filepath.WalkDir(".", func(path string, d os.DirEntry, err error) error { + if err != nil || d.IsDir() || !strings.HasSuffix(path, ".go") { + return err + } + if strings.HasSuffix(path, "_test.go") { + return nil // including this file, which names the call it is counting + } + body, err := os.ReadFile(path) + if err != nil { + return err + } + for i, line := range strings.Split(string(body), "\n") { + if strings.Contains(line, "foundry.ResolveFileRefs(") { + callers[path] = append(callers[path], i+1) + } + } + return nil + })) + + total := 0 + for _, lines := range callers { + total += len(lines) + } + assert.Equal(t, 1, total, + "ResolveFileRefs has more than one caller (%v); route it through %s instead, "+ + "or the next `$ref` rule will land on one path and not the other", + callers, resolver) +} diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/readfile_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/readfile_test.go index 07ba4497226..b205a89b32b 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/project/readfile_test.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/readfile_test.go @@ -59,7 +59,7 @@ func TestLoadEvalConfig_AcceptsAByteOrderMark(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, EvalConfigBase) body := append([]byte{0xEF, 0xBB, 0xBF}, - []byte("datasets:\n - name: d\n source: ./d.jsonl\n")...) + []byte("datasets:\n - name: d\n file: ./d.jsonl\n")...) require.NoError(t, os.WriteFile(path, body, 0o600)) cfg, err := LoadEvalConfig(path) diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/readme_example_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/readme_example_test.go new file mode 100644 index 00000000000..71c8e81b573 --- /dev/null +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/readme_example_test.go @@ -0,0 +1,53 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package project + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +// The README's configuration example has to be one the CLI can load. +// +// It documented `options.eval_model`, which is not a key this decoder has ever +// had: following the README produced `unknown key "options"`. Nothing caught it +// because the example was prose, so correcting the keys alone would only have +// reset the clock on the same drift. +func TestTheREADMEExampleLoads(t *testing.T) { + readme, err := os.ReadFile(filepath.Join("..", "..", "README.md")) + require.NoError(t, err) + + body := evalConfigExample(t, string(readme)) + + path := filepath.Join(t.TempDir(), EvalConfigBase) + require.NoError(t, os.WriteFile(path, []byte(body), 0o600)) + + _, err = LoadEvalConfig(path) + require.NoError(t, err, "the README example has to survive the decoder it documents") +} + +// evalConfigExample returns the fenced yaml block the README labels as the eval +// configuration, identified by the file name comment on its first line. +func evalConfigExample(t *testing.T, readme string) string { + t.Helper() + + const marker = "```yaml\n# evals/" + EvalConfigBase + "\n" + + readme = strings.ReplaceAll(readme, "\r\n", "\n") + + start := strings.Index(readme, marker) + require.NotEqual(t, -1, start, + "the README no longer opens the example with `# evals/%s`; retarget this test "+ + "rather than deleting it", EvalConfigBase) + + rest := readme[start+len(marker):] + end := strings.Index(rest, "```") + require.NotEqual(t, -1, end, "the example's fence is unterminated") + + return rest[:end] +} diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/ref_directive_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/ref_directive_test.go new file mode 100644 index 00000000000..88729042ac6 --- /dev/null +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/ref_directive_test.go @@ -0,0 +1,52 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package project + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// `$ref` decodes rather than being refused, and survives being written back. +// +// It was a directive the strict decoder rejected, on the reasoning that one +// reaching the decoder meant resolution had been skipped. That reasoning only +// held while every reader resolved. The commands that read, modify and save the +// file deliberately do not, because saving a resolved configuration inlines the +// author's includes -- so the decoder has to carry the directive through +// untouched instead of naming it a typo. +// +// TestRefResolvesOnTheCLIPathToo covers the resolved route; +// TestEditingReadsLeaveIncludesAlone covers the round trip. +func TestARefDirectiveDecodesAndSurvives(t *testing.T) { + withRef := []byte(` +evaluators: + - $ref: ./evaluators/quality.yaml +evals: + - name: nightly +`) + + cfg, err := DecodeEvalConfig(withRef, "azure.eval.yaml") + + require.NoError(t, err, "the editing readers hand this straight to the decoder") + require.Len(t, cfg.Evaluators, 1) + assert.Equal(t, "./evaluators/quality.yaml", cfg.Evaluators[0].Ref) + assert.Empty(t, cfg.Evaluators[0].Name, + "an entry that is only a $ref has no name until the file it names supplies one") + + // The spelling that needs no resolution at all. + cfg, err = DecodeEvalConfig([]byte(` +evaluators: + - name: quality + source: ./evaluators/quality.json +evals: + - name: nightly +`), "azure.eval.yaml") + + require.NoError(t, err) + assert.Equal(t, "./evaluators/quality.json", cfg.Evaluators[0].Source) + assert.Empty(t, cfg.Evaluators[0].Ref) +} diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/ref_resolution_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/ref_resolution_test.go new file mode 100644 index 00000000000..71e0d2978c1 --- /dev/null +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/ref_resolution_test.go @@ -0,0 +1,232 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package project + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// `$ref` means the same thing to `azd up` and to the CLI commands. +// +// Core owns the resolver but does not run it for us -- it hands each extension +// the entry with `$ref` still in it. The service target called it and this path +// did not, so an include deployed fine and then failed every `azd ai eval` +// command with `unknown key "$ref"`: one file, two meanings, decided by which +// command opened it. +func TestRefResolvesOnTheCLIPathToo(t *testing.T) { + dir := t.TempDir() + + require.NoError(t, os.MkdirAll(filepath.Join(dir, "evaluators"), 0o755)) + require.NoError(t, os.WriteFile( + filepath.Join(dir, "evaluators", "quality.yaml"), + []byte("name: support-agent-quality\nsource: ./quality.json\n"), + 0o600)) + + path := filepath.Join(dir, "azure.eval.yaml") + require.NoError(t, os.WriteFile(path, []byte(` +datasets: + - name: golden + file: ./datasets/golden.jsonl + +evaluators: + - $ref: ./evaluators/quality.yaml + +evals: + - name: nightly + dataset: golden +`), 0o600)) + + cfg, err := LoadEvalConfig(path) + require.NoError(t, err, "the include the service target resolves has to resolve here too") + require.Len(t, cfg.Evaluators, 1) + assert.Equal(t, "support-agent-quality", cfg.Evaluators[0].Name, + "the referenced file's content replaces the directive") + assert.Equal(t, "./quality.json", cfg.Evaluators[0].Source) +} + +// A `$ref` can name the rubric itself, not only a pointer to one. +// +// This is the shape the spec documents, and it works because resolution splices +// the referenced file's keys into the entry: they have to land on fields of the +// declaration or strict decoding rejects them. `definition` is that field. +func TestRefCanNameTheRubricItself(t *testing.T) { + dir := t.TempDir() + + require.NoError(t, os.MkdirAll(filepath.Join(dir, "evaluators"), 0o755)) + require.NoError(t, os.WriteFile( + filepath.Join(dir, "evaluators", "quality.json"), + []byte(`{"name":"support-agent-quality",`+ + `"definition":{"type":"rubric","dimensions":[{"name":"tone","weight":1}]}}`), + 0o600)) + + path := filepath.Join(dir, "azure.eval.yaml") + require.NoError(t, os.WriteFile(path, []byte(` +evaluators: + - $ref: ./evaluators/quality.json + +evals: + - name: nightly + dataset: golden +`), 0o600)) + + cfg, err := LoadEvalConfig(path) + require.NoError(t, err, "a rubric named by $ref has to decode") + require.Len(t, cfg.Evaluators, 1) + assert.Equal(t, "support-agent-quality", cfg.Evaluators[0].Name) + assert.Equal(t, "rubric", cfg.Evaluators[0].Definition["type"], + "the rubric travels with the declaration, so there is no second file to find") + assert.Empty(t, cfg.Evaluators[0].Source, + "a definition in hand is not a path to resolve against anything") +} + +// Both routes into a configuration have to agree about a `$ref`'d rubric. +// +// The deploy path resolves includes itself rather than going through +// LoadEvalConfig, so a rescue added on one side only would recreate the exact +// asymmetry that started this work -- an include `azd up` accepted and every +// CLI command refused, in mirror image. +func TestBothRoutesReadARefdRubricTheSameWay(t *testing.T) { + dir := t.TempDir() + + require.NoError(t, os.MkdirAll(filepath.Join(dir, "evaluators"), 0o755)) + require.NoError(t, os.WriteFile( + filepath.Join(dir, "evaluators", "quality.json"), + []byte(`{"type":"rubric","dimensions":[{"id":"tone","weight":3}]}`), + 0o600)) + + path := filepath.Join(dir, "azure.eval.yaml") + require.NoError(t, os.WriteFile(path, []byte(` +evaluators: + - $ref: ./evaluators/quality.json + name: quality + +evals: + - name: nightly + dataset: golden +`), 0o600)) + + fromDisk, err := LoadEvalConfig(path) + require.NoError(t, err) + + svc := serviceWith(t, map[string]any{ + "evaluators": []any{map[string]any{ + "$ref": "./evaluators/quality.json", + "name": "quality", + }}, + "evals": []any{map[string]any{"name": "nightly", "dataset": "golden"}}, + }) + fromService, err := EvalConfigFromService(svc, dir) + require.NoError(t, err, "`azd up` has to read what the CLI reads") + + assert.Equal(t, fromDisk.Evaluators, fromService.Evaluators, + "one file, one meaning, whichever command opened it") +} + +// A `$ref` can name a bare rubric file, which is the shape the spec documents +// and the shape `generate` downloads from the service. +// +// `$ref` splices the file's top-level keys into the entry, so `dimensions` and +// friends land beside `name` and used to be rejected outright. They are moved +// under `definition` instead. Wrapping the file would have been the smaller +// change and the wrong one: the tool writes that file, so the config has to +// read what the tool writes. +func TestRefCanNameABareRubricFile(t *testing.T) { + dir := t.TempDir() + + require.NoError(t, os.MkdirAll(filepath.Join(dir, "evaluators"), 0o755)) + require.NoError(t, os.WriteFile( + filepath.Join(dir, "evaluators", "support-agent-quality.json"), + []byte(`{"type":"rubric","pass_threshold":0.7,`+ + `"dimensions":[{"id":"resolves_issue","weight":9,"description":"Resolves it."}]}`), + 0o600)) + + path := filepath.Join(dir, "azure.eval.yaml") + require.NoError(t, os.WriteFile(path, []byte(` +evaluators: + - $ref: ./evaluators/support-agent-quality.json + name: support-agent-quality + +evals: + - name: nightly + dataset: golden +`), 0o600)) + + cfg, err := LoadEvalConfig(path) + require.NoError(t, err, "the spec's own example has to load") + require.Len(t, cfg.Evaluators, 1) + assert.Equal(t, "support-agent-quality", cfg.Evaluators[0].Name, + "the sibling name stays the author's, not a key from the rubric") + assert.Equal(t, "rubric", cfg.Evaluators[0].Definition["type"]) + assert.Equal(t, 0.7, cfg.Evaluators[0].Definition["pass_threshold"], + "every rubric key travels, not just the ones this decoder happens to know") + assert.Len(t, cfg.Evaluators[0].Definition["dimensions"], 1) +} + +// The rescue above is scoped to entries written as a `$ref`, so a misspelling +// in a hand-written entry is still an error rather than rubric content. +func TestAMisspelledEvaluatorKeyIsStillRejected(t *testing.T) { + dir := t.TempDir() + + path := filepath.Join(dir, "azure.eval.yaml") + require.NoError(t, os.WriteFile(path, []byte(` +evaluators: + - nmae: support-agent-quality + definition: + type: rubric +`), 0o600)) + + _, err := LoadEvalConfig(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "nmae", "the error has to name the key that is wrong") +} + +// Sibling keys overlay the loaded file, which is what lets a name live in the +// configuration while the definition it names lives beside the code it grades. +func TestRefSiblingKeysOverlayTheLoadedFile(t *testing.T) { + dir := t.TempDir() + + require.NoError(t, os.MkdirAll(filepath.Join(dir, "evaluators"), 0o755)) + require.NoError(t, os.WriteFile( + filepath.Join(dir, "evaluators", "quality.yaml"), + []byte("name: from-the-file\nsource: ./quality.json\n"), + 0o600)) + + path := filepath.Join(dir, "azure.eval.yaml") + require.NoError(t, os.WriteFile(path, []byte(` +evaluators: + - $ref: ./evaluators/quality.yaml + name: from-the-configuration + +evals: + - name: nightly +`), 0o600)) + + cfg, err := LoadEvalConfig(path) + require.NoError(t, err) + require.Len(t, cfg.Evaluators, 1) + assert.Equal(t, "from-the-configuration", cfg.Evaluators[0].Name) +} + +// A configuration with no include is handed to the decoder untouched, so its +// diagnostics keep the line numbers of the file the author actually wrote. +func TestConfigWithoutRefIsNotRoundTripped(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "azure.eval.yaml") + require.NoError(t, os.WriteFile(path, []byte(` +datasets: + - name: golden + fiel: ./datasets/golden.jsonl +`), 0o600)) + + _, err := LoadEvalConfig(path) + + require.Error(t, err) + assert.Contains(t, err.Error(), "line 4", + "a typo is reported where it was written, not where a re-marshal put it") +} diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/service_config_strict_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/service_config_strict_test.go index 73f59c69ff7..8bb3dad0a04 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/project/service_config_strict_test.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/service_config_strict_test.go @@ -45,7 +45,7 @@ func TestEvalConfigFromServiceRejectsAMistypedKey(t *testing.T) { // the door on the authoring style it is meant to serve. func TestEvalConfigFromServiceAcceptsADeclaredConfig(t *testing.T) { svc := serviceWith(t, map[string]any{ - "datasets": []any{map[string]any{"name": "golden", "source": "./datasets/golden.jsonl"}}, + "datasets": []any{map[string]any{"name": "golden", "file": "./datasets/golden.jsonl"}}, "evals": []any{map[string]any{ "name": "support-agent-eval", "dataset": "golden", diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/service_target_eval.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/service_target_eval.go index 92951143f2d..539717d07cc 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/project/service_target_eval.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/service_target_eval.go @@ -15,7 +15,6 @@ import ( "azureaieval/internal/messages" "github.com/azure/azure-dev/cli/azd/pkg/azdext" - "github.com/azure/azure-dev/cli/azd/pkg/foundry" "go.yaml.in/yaml/v3" "google.golang.org/protobuf/types/known/structpb" ) @@ -154,14 +153,14 @@ func (p *EvalServiceTargetProvider) Deploy( // 1. Datasets the configuration owns. Paths are kept so an eval that names // one can derive its columns without reading the blob back. // - // A declaration with no `source:` is included rather than skipped: it names + // A declaration with no `file:` is included rather than skipped: it names // a dataset that is already registered, and reconciling it is what confirms // it is really there and settles which version a `version:` pin selected. // Skipping it would leave a misspelled name to surface as a failed run. datasetPaths := map[string]string{} for _, decl := range cfg.Datasets { report(progress, messages.ReconcilingDataset(decl.Name)) - localPath := ResolveSource(baseDir, decl.Source) + localPath := ResolveSource(baseDir, decl.File) datasetPaths[decl.Name] = localPath version, changed, err := reconciler.EnsureDataset(ctx, decl, localPath) if err != nil { @@ -291,9 +290,9 @@ func EvalConfigFromService(svc *azdext.ServiceConfig, projectRoot string) (*Eval values := props.AsMap() if projectRoot != "" { - resolved, err := foundry.ResolveFileRefs(values, projectRoot) + resolved, err := resolveEvalRefs(values, projectRoot) if err != nil { - return nil, messages.ResolvingServiceRefs(err) + return nil, err } values = resolved } @@ -382,6 +381,13 @@ func Fingerprint(path string) (string, error) { return hex.EncodeToString(sum[:]), nil } +// FingerprintBytes hashes content that was never a file, which is how a rubric +// written in the configuration gets the change detection a rubric file has. +func FingerprintBytes(data []byte) string { + sum := sha256.Sum256(data) + return hex.EncodeToString(sum[:]) +} + // FingerprintGroup hashes an eval's own declaration. // // Change detection on upstream artifacts is not sufficient: editing a group's diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/service_target_eval_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/service_target_eval_test.go index e20ecd49009..90442c47014 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/project/service_target_eval_test.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/service_target_eval_test.go @@ -71,7 +71,7 @@ func TestEvalConfigFromServiceReadsInlineConfig(t *testing.T) { Name: "support-agent-evals", AdditionalProperties: propsFrom(t, map[string]any{ "datasets": []any{ - map[string]any{"name": "golden", "source": "./datasets/golden.jsonl"}, + map[string]any{"name": "golden", "file": "./datasets/golden.jsonl"}, }, "evals": []any{ map[string]any{ diff --git a/cli/azd/extensions/azure.ai.evaluations/internal/project/unknown_keys_depth_test.go b/cli/azd/extensions/azure.ai.evaluations/internal/project/unknown_keys_depth_test.go index e8c9dc47cf4..1ec60ac56b1 100644 --- a/cli/azd/extensions/azure.ai.evaluations/internal/project/unknown_keys_depth_test.go +++ b/cli/azd/extensions/azure.ai.evaluations/internal/project/unknown_keys_depth_test.go @@ -41,9 +41,9 @@ func TestUnknownKeysAreNamedAtEveryDepth(t *testing.T) { }, { where: "inside a dataset declaration", - body: "datasets:\n - name: golden\n sourse: ./rows.jsonl\n", - key: "sourse", - nearer: "source", + body: "datasets:\n - name: golden\n fil: ./rows.jsonl\n", + key: "fil", + nearer: "file", line: "line 3", }, } diff --git a/cli/azd/extensions/azure.ai.evaluations/schemas/azure.ai.eval.json b/cli/azd/extensions/azure.ai.evaluations/schemas/azure.ai.eval.json new file mode 100644 index 00000000000..711fb58d30a --- /dev/null +++ b/cli/azd/extensions/azure.ai.evaluations/schemas/azure.ai.eval.json @@ -0,0 +1,249 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "$id": "https://raw.githubusercontent.com/Azure/azure-dev/main/cli/azd/extensions/azure.ai.evaluations/schemas/azure.ai.eval.json", + "title": "Azure AI Foundry evaluation service", + "description": "Service-level configuration for a host: azure.ai.eval entry. The service key is the evaluation configuration name. The body is normally supplied with $ref to an evals/azure.eval.yaml holding the dataset and evaluator catalogs and every eval defined over them; relative paths inside that file resolve against the file's own directory.", + "type": "object", + "additionalProperties": true, + "properties": { + "datasets": { + "type": "array", + "description": "Catalog of named datasets. An entry with a file is uploaded on deploy; without one the name must already resolve to a registered dataset.", + "items": { "$ref": "#/definitions/DatasetDecl" } + }, + "evaluators": { + "type": "array", + "description": "Catalog of named custom evaluators. An entry is a declaration, or a $ref to a file holding one. Built-in evaluators are referenced straight from an eval and are never declared here.", + "items": { + "oneOf": [ + { "$ref": "#/definitions/EvaluatorDecl" }, + { "$ref": "#/definitions/FileRef" } + ] + } + }, + "evals": { + "type": "array", + "description": "The evaluations defined over the catalogs. A list, because one target is normally gated by more than one evaluation.", + "items": { "$ref": "#/definitions/Eval" } + } + }, + "definitions": { + "FileRef": { + "type": "object", + "required": ["$ref"], + "additionalProperties": true, + "description": "Replace an inline definition with a reference to an external YAML or JSON file. Sibling properties on the same object act as overlay overrides on top of the loaded file. Cloned rather than referenced across extensions, following azure.ai.projects.", + "properties": { + "$ref": { + "type": "string", + "description": "Path to a YAML or JSON file containing the definition. Relative paths resolve from the file containing this $ref. Absolute paths are also accepted; remote URLs are not supported." + } + } + }, + "DatasetDecl": { + "type": "object", + "required": ["name"], + "additionalProperties": false, + "properties": { + "name": { + "type": "string", + "description": "Name the dataset is registered under. Letters, digits, dashes and underscores, up to 255 characters." + }, + "file": { + "type": "string", + "description": "Path to a local .jsonl whose rows are uploaded on deploy, relative to this file. Deliberately not a $ref: the rows are a data artifact to publish, not a definition to splice in, and a .jsonl is neither YAML nor JSON. Omit to use a dataset already registered under this name." + }, + "version": { + "type": "string", + "description": "Pin to an already-registered version. Omit to publish and track the newest." + } + } + }, + "EvaluatorDecl": { + "type": "object", + "required": ["name"], + "additionalProperties": false, + "properties": { + "name": { + "type": "string", + "description": "Name the evaluator is registered under. Letters, digits, dashes and underscores, up to 255 characters." + }, + "source": { + "type": "string", + "description": "Path to a local .json rubric -- a list of weighted scoring dimensions -- relative to this file. Published on deploy and fingerprinted locally so a later deploy can tell an edit here from a version published elsewhere." + }, + "definition": { + "type": "object", + "description": "The rubric written out here instead of named as a file. This is also what a $ref to a rubric file becomes once it is resolved. Its keys belong to the evaluator service, so they are not constrained here.", + "required": ["dimensions"], + "properties": { + "type": { + "type": "string", + "description": "Defaults to rubric when omitted." + }, + "dimensions": { + "type": "array", + "description": "The weighted scoring dimensions the evaluator grades on." + } + } + }, + "version": { + "type": "string", + "description": "Pin to an already-registered version. Omit to publish and track the newest." + } + }, + "allOf": [ + { + "$comment": "A rubric is named or written out, never both.", + "if": { "required": ["definition"] }, + "then": { "properties": { "source": false } } + } + ] + }, + "Eval": { + "type": "object", + "required": ["name"], + "additionalProperties": false, + "description": "One evaluation. dataset and source are alternatives for where rows come from; target is a separate axis and means what gets invoked, so an eval can read traces and invoke nothing.", + "properties": { + "name": { + "type": "string", + "description": "Name of the eval. The azd environment records the created eval id under it, which is what --eval resolves." + }, + "id": { + "type": "string", + "description": "An existing eval id, for an eval created outside this configuration." + }, + "description": { "type": "string" }, + "dataset": { + "type": "string", + "description": "Name of an entry in datasets[]. Mutually exclusive with source." + }, + "source": { + "$ref": "#/definitions/SourceDecl", + "description": "Where rows come from when they are not a catalog dataset. Mutually exclusive with dataset." + }, + "evaluation_level": { + "type": "string", + "enum": ["turn", "conversation"], + "description": "Whether each row is graded as a single turn or a whole conversation. The service default is turn." + }, + "max_samples": { + "type": "integer", + "minimum": 0, + "description": "Cap on rows graded per run." + }, + "evaluators": { + "type": "array", + "description": "The evaluators this eval grades with. Entries are either builtin. or a name declared in evaluators[].", + "items": { "$ref": "#/definitions/EvaluatorRef" } + }, + "target": { + "$ref": "#/definitions/Target", + "description": "What the run invokes to produce the responses being graded. Omit to grade the responses already present in the rows." + } + }, + "allOf": [ + { + "$comment": "Rows come from a catalog dataset or from a source, never both.", + "if": { "required": ["dataset"] }, + "then": { "properties": { "source": false } } + } + ] + }, + "SourceDecl": { + "type": "object", + "required": ["type"], + "additionalProperties": false, + "properties": { + "type": { + "type": "string", + "enum": ["traces", "responses"], + "description": "traces reads recorded spans from the project's Application Insights; responses reads named response ids." + }, + "lookback_hours": { + "type": "integer", + "minimum": 0, + "maximum": 87600, + "description": "How far back the trace window reaches from its end, capped at ten years (project.MaxLookbackHours). Zero means unset. Only for type: traces." + }, + "max_traces": { + "type": "integer", + "minimum": 0, + "description": "Cap on traces read. Only for type: traces. Removing it restores the service default." + }, + "agent_name": { + "type": "string", + "description": "Name of the agent whose traces are read. Only for type: traces." + }, + "agent_version": { + "type": "string", + "description": "Pin the traces to one agent version." + }, + "response_ids": { + "type": "array", + "items": { "type": "string" }, + "description": "Explicit response ids to grade. Only for type: responses." + }, + "max_turns": { + "type": "integer", + "minimum": 0, + "description": "Cap on turns taken from each conversation." + }, + "start_time": { + "type": "string", + "description": "Start of the window, RFC 3339. With end_time this pins an absolute window instead of a lookback." + }, + "end_time": { + "type": "string", + "description": "End of the window, RFC 3339." + } + } + }, + "EvaluatorRef": { + "type": "object", + "required": ["evaluator"], + "additionalProperties": false, + "properties": { + "evaluator": { + "type": "string", + "description": "builtin. for a built-in, or the name of an entry in evaluators[]." + }, + "name": { + "type": "string", + "description": "Label for this evaluator within the eval, when the same evaluator is used more than once." + }, + "version": { + "type": "string", + "description": "Pin this eval to one evaluator version, so a later publish does not change what it grades." + }, + "initialization_parameters": { + "type": "object", + "additionalProperties": true, + "description": "Passed to the evaluator when the run starts; model names the judge deployment." + }, + "data_mapping": { + "type": "object", + "additionalProperties": { "type": "string" }, + "description": "Binds the evaluator's required inputs to dataset columns, for inputs beyond the agent shape such as ground_truth or context." + } + } + }, + "Target": { + "type": "object", + "required": ["type", "name"], + "additionalProperties": false, + "properties": { + "type": { + "type": "string", + "enum": ["agent", "model"], + "description": "What kind of thing is invoked." + }, + "name": { + "type": "string", + "description": "Name of the agent service or the model deployment to invoke." + } + } + } + } +} diff --git a/cli/azd/extensions/azure.ai.evaluations/schemas/examples/inline.azure.yaml b/cli/azd/extensions/azure.ai.evaluations/schemas/examples/inline.azure.yaml new file mode 100644 index 00000000000..8f191be32a5 --- /dev/null +++ b/cli/azd/extensions/azure.ai.evaluations/schemas/examples/inline.azure.yaml @@ -0,0 +1,58 @@ +# yaml-language-server: $schema=https://raw.githubusercontent.com/Azure/azure-dev/main/schemas/v1.0/azure.yaml.json +# The same configuration written inline, which is what the $ref form resolves to. +name: support-agent-app + +services: + support-agent-evals: + host: azure.ai.eval + uses: [ai-project, support-agent] + + datasets: + - name: support-agent-regression + file: ./datasets/support-agent-regression.jsonl + - name: prod-golden + file: ./datasets/prod-golden.jsonl + version: "2" + + evaluators: + - name: support-agent-quality + source: ./evaluators/support-agent-quality.json + # Or pulled in from its own file, the way agents and projects do it. + - $ref: ./evaluators/tone.yaml + name: tone + # Or written out here, which is also what a $ref to a rubric resolves to. + - name: brevity + definition: + type: rubric + dimensions: + - name: length + weight: 1 + description: Answers the question without restating it. + + evals: + - name: support-agent-regression-eval + dataset: support-agent-regression + evaluation_level: turn + max_samples: 50 + evaluators: + - evaluator: builtin.task_adherence + initialization_parameters: + model: gpt-5.6-luna + - evaluator: support-agent-quality + version: "2" + data_mapping: + ground_truth: "{{item.expected}}" + target: + type: agent + name: support-agent + + - name: support-agent-trace-eval + source: + type: traces + agent_name: support-agent + lookback_hours: 24 + max_traces: 20 + evaluators: + - evaluator: builtin.task_adherence + initialization_parameters: + model: gpt-5.6-luna diff --git a/cli/azd/extensions/azure.ai.evaluations/schemas/examples/ref.azure.yaml b/cli/azd/extensions/azure.ai.evaluations/schemas/examples/ref.azure.yaml new file mode 100644 index 00000000000..695b8dae4a0 --- /dev/null +++ b/cli/azd/extensions/azure.ai.evaluations/schemas/examples/ref.azure.yaml @@ -0,0 +1,10 @@ +# yaml-language-server: $schema=https://raw.githubusercontent.com/Azure/azure-dev/main/schemas/v1.0/azure.yaml.json +# The shape `azd ai eval init` writes: the eval body lives in its own file, and +# relative paths inside that file resolve against the file, not the project root. +name: support-agent-app + +services: + support-agent-evals: + host: azure.ai.eval + uses: [ai-project, support-agent] + $ref: ./evals/azure.eval.yaml diff --git a/cli/azd/extensions/azure.ai.evaluations/tests/hero/init_test.go b/cli/azd/extensions/azure.ai.evaluations/tests/hero/init_test.go index 1ab00592ce1..2f839f02319 100644 --- a/cli/azd/extensions/azure.ai.evaluations/tests/hero/init_test.go +++ b/cli/azd/extensions/azure.ai.evaluations/tests/hero/init_test.go @@ -383,6 +383,6 @@ func TestHeroInitSuppliedDatasetIsNotGenerated(t *testing.T) { body, err := os.ReadFile(filepath.Join(dir, "evals", "azure.eval.yaml")) require.NoError(t, err) require.Contains(t, string(body), "dataset: prod-golden") - require.NotContains(t, string(body), "source:", + require.NotContains(t, string(body), "file:", "a registered dataset has nothing to upload") } diff --git a/schemas/alpha/azure.yaml.json b/schemas/alpha/azure.yaml.json index d103b88fb2a..17190ce1973 100644 --- a/schemas/alpha/azure.yaml.json +++ b/schemas/alpha/azure.yaml.json @@ -253,7 +253,8 @@ "azure.ai.connection", "azure.ai.toolbox", "azure.ai.skill", - "azure.ai.routine" + "azure.ai.routine", + "azure.ai.eval" ] }, "language": { @@ -557,6 +558,26 @@ } } }, + { + "comment": "Azure AI Foundry evaluation host - code-less resource service; the service key is the evaluation configuration name and it uses: the project and whatever it grades", + "if": { + "properties": { + "host": { "const": "azure.ai.eval" } + } + }, + "then": { + "allOf": [ + { "$ref": "https://raw.githubusercontent.com/Azure/azure-dev/main/cli/azd/extensions/azure.ai.evaluations/schemas/azure.ai.eval.json" } + ], + "properties": { + "project": false, + "runtime": false, + "docker": false, + "image": false, + "config": false + } + } + }, { "comment": "Legacy Microsoft Foundry host - compatibility for old non-network files; new provisioning uses azure.ai.project", "if": { diff --git a/schemas/v1.0/azure.yaml.json b/schemas/v1.0/azure.yaml.json index f7a7c9b5895..76a057e3e8e 100644 --- a/schemas/v1.0/azure.yaml.json +++ b/schemas/v1.0/azure.yaml.json @@ -214,7 +214,8 @@ "azure.ai.connection", "azure.ai.toolbox", "azure.ai.skill", - "azure.ai.routine" + "azure.ai.routine", + "azure.ai.eval" ] }, "language": { @@ -517,6 +518,26 @@ } } }, + { + "comment": "Azure AI Foundry evaluation host - code-less resource service; the service key is the evaluation configuration name and it uses: the project and whatever it grades", + "if": { + "properties": { + "host": { "const": "azure.ai.eval" } + } + }, + "then": { + "allOf": [ + { "$ref": "https://raw.githubusercontent.com/Azure/azure-dev/main/cli/azd/extensions/azure.ai.evaluations/schemas/azure.ai.eval.json" } + ], + "properties": { + "project": false, + "runtime": false, + "docker": false, + "image": false, + "config": false + } + } + }, { "comment": "Legacy Microsoft Foundry host - compatibility for old non-network files; new provisioning uses azure.ai.project", "if": {