feat(agents): add initialization_parameters to evaluator YAML config - #9472
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 19 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Note
This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.
3e2a543 to
80e23c8
Compare
056395d to
93ad82a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
cli/azd/extensions/azure.ai.agents/internal/pkg/agents/opt_eval/yaml.go:496
- This introduces a separate user-facing YAML/API option that is absent from the PR description and has no parsing, forwarding, omission, or bounds-validation tests. Either remove the unrelated
max_concurrent_agent_runschange from this PR, or document its contract and add validation/tests before exposing it. [azd-code-reviewer]
}
// Read reads a YAML config file (eval or optimize format).
func Read(path string) (*Config, error) {
cli/azd/extensions/azure.ai.agents/internal/pkg/agents/opt_eval/yaml.go:102
EvaluatorList.MarshalYAMLstill emits an evaluator as a scalar whenever version and local URI are empty, so an entry containing onlyinitialization_parametersis rewritten as just its name and silently loses the new data throughConfig.Write. Include this field when deciding whether an entry requires mapping form, and add a write/read round-trip case without a version. [azd-code-reviewer]
// InitializationParameters holds evaluator-specific configuration parameters
// such as regex_match's ``patterns``. Sent inline per-evaluator in the
// evaluators array of the API request.
InitializationParameters map[string]any `yaml:"initialization_parameters,omitempty" json:"-"`
cli/azd/extensions/azure.ai.agents/internal/cmd/optimize_config.go:259
- The first line of
mergeEvaluators' comment was deleted, leaving this paragraph starting with the fragment “exists in base” and no longer identifying what is being described. Restore the deleted// mergeEvaluators appends add to base, skipping entries whose name alreadyline. [azd-code-reviewer]
// new entries from add. Used to layer --evaluator flags on top of config
93ad82a to
53c2cfb
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
cli/azd/extensions/azure.ai.agents/internal/pkg/agents/opt_eval/yaml.go:100
- [azd-code-reviewer]
EvaluatorList.MarshalYAMLstill emits the scalar form whenever version and local URI are empty. An evaluator containing only a name plus this new map is therefore written as just its name byWrite/WriteEvalConfig, silently dropping all initialization parameters. Treat a ref as scalar only whenInitializationParametersis also empty, and cover this with a write/read round-trip test.
InitializationParameters map[string]any `yaml:"initialization_parameters,omitempty" json:"-"`
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (4)
cli/azd/extensions/azure.ai.agents/internal/pkg/agents/optimize_api/models.go:84
- [azd-code-reviewer]
StartOptimizemarshals this field inside eachevaluators[]object, but the optimizer contract described by this PR readsEvaluatorInitParamsMapfrom the top-level request. These values therefore will not reach the optimizer. Keep the wireEvaluatorRefas name/version, add and populate the top-level request map, and assert the serialized request shape.
InitializationParameters map[string]any `json:"initialization_parameters,omitempty"`
cli/azd/extensions/azure.ai.agents/internal/pkg/agents/opt_eval/yaml.go:100
- [azd-code-reviewer]
EvaluatorList.MarshalYAMLstill emits an evaluator as a scalar whenever version andlocal_uriare empty. An evaluator that only sets this new map is therefore written as just its name byWrite, silently dropping its initialization parameters on a write/read round trip. Include non-empty initialization parameters in the structured-entry condition and cover that round trip.
InitializationParameters map[string]any `yaml:"initialization_parameters,omitempty" json:"-"`
cli/azd/extensions/azure.ai.agents/internal/pkg/agents/opt_eval/yaml.go:94
- [azd-code-reviewer] The previous two-line
EvaluatorRefcomment remains directly above this addition, so the Go doc now repeats itself and first lists an outdated set of fields. Replace the four lines with the updated two-line comment once.
This issue also appears on line 100 of the same file.
// EvaluatorRef describes an evaluator. It can be a simple string name or a
// structured entry with name, version, local_uri, and initialization_parameters.
cli/azd/extensions/azure.ai.agents/internal/cmd/optimize_config.go:258
- [azd-code-reviewer] The opening line of the
mergeEvaluatorscomment was deleted, leaving this documentation starting mid-sentence with “exists in base.” Restore the removedmergeEvaluators appends...line so the comment is complete and names the function it documents.
// exists in base (case-sensitive). Order is preserved: base first, then any
53c2cfb to
02ed171
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
cli/azd/extensions/azure.ai.agents/internal/pkg/agents/optimize_api/models.go:84
- The optimize service expects these values in the top-level
OptimizeRequest.EvaluatorInitParamsMap, not inside eachevaluators[]entry.StartOptimizewill currently serializeinitialization_parameterson the evaluator and never emit the top-level map, so the optimizer cannot resolve the required parameters. Keep the wireEvaluatorRefname/version-only and populate the request-level map inToRequest. (azd-code-reviewer)
InitializationParameters map[string]any `json:"initialization_parameters,omitempty"`
Enables evaluators that require configuration (e.g. builtin.regex_match
patterns) to be fully specified in the optimize YAML config.
Add initialization_parameters field to EvaluatorRef (YAML only,
not serialized per-evaluator in the API array) and wire it through to
EvaluatorInitParamsMap in the OptimizeRequest top-level field.
The C# API stores this in Cosmos; the Python optimizer reads it via
cosmos_loader._resolve_evaluator_init_params_map.
Usage:
evaluators:
- name: builtin.regex_match
version: 4
initialization_parameters:
patterns:
- (?i)Answer:\s*GT
02ed171 to
3169f0b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (2)
cli/azd/extensions/azure.ai.agents/internal/pkg/agents/optimize_api/models.go:84
- [azd-code-reviewer] This serializes
initialization_parametersinside eachevaluators[]item, but the optimizer contract described by this PR expects a top-levelevaluatorInitParamsMap.OptimizeRequesthas no such field, so the backend will not receive the map and evaluators such asbuiltin.regex_matchstill cannot be initialized. Keep this value YAML-only on the evaluator reference, add/populate the top-level request map keyed by evaluator, and assert the marshaled request shape.
InitializationParameters map[string]any `json:"initialization_parameters,omitempty"`
cli/azd/extensions/azure.ai.agents/internal/pkg/agents/opt_eval/yaml.go:94
- [azd-code-reviewer] There are now two consecutive
EvaluatorRefdoc comments, and the first still claims structured entries only supportname,version, andlocal_uri. Remove the obsolete first comment so the public type has one accurate description.
// EvaluatorRef describes an evaluator. It can be a simple string name or a
// structured entry with name, version, local_uri, and initialization_parameters.
John Miller (therealjohn)
left a comment
There was a problem hiding this comment.
approved scenario
|
/check-enforcer override |
d4acaee
into
Azure:main
Closes #9624
Problem
Evaluators that require configuration parameters (e.g.
builtin.regex_matchwhich requirespatterns) couldn't be fully specified from the optimize YAML config.EvaluatorRefonly had name and version.Change
initialization_parameters map[string]anytoEvaluatorRefin the YAML config structOptimizeRequestevaluatorsarray (the C# API readsinitialization_parametersper-entry, not from a top-level map)MarshalYAML: an evaluator with onlyinitialization_parameters(no version/local_uri) was previously emitted as a plain scalar, silently dropping the params on round-tripTestConfig_RoundTrip_EvaluatorInitializationParameterscovering plain, init-params-only, and version+init-params casesUsage