evals: replace framework with custom ADK harness - #106
Conversation
| EVAL_OUTPUT: evals/results/ci.json | ||
| EVAL_PROVIDER: bedrock | ||
| EVAL_MODEL: ${{ inputs.model }} | ||
| AWS_ACCESS_KEY_ID: ${{ inputs.AWS_ACCESS_KEY_ID }} |
There was a problem hiding this comment.
These values could be exposed from metadata or logs since they're not coming from GitHub secrets. Can we use GitHub OIDC or repo / env secrets?
| if fn == nil || fn.Name == "" { | ||
| return nil, errors.New("openaichat: function declaration missing name") | ||
| } | ||
| params, err := schemaToMap(fn.Parameters) |
There was a problem hiding this comment.
functiontool.New puts its generated input schema in FunctionDeclaration.ParametersJsonSchema, not Parameters so this will lose the args field. Cn we support ParametersJsonSchema here and in the Bedrock adapter and test w an ADK-generated declaration?
| calls = append(calls, call) | ||
| stop = true | ||
| mu.Unlock() | ||
| cancel() |
There was a problem hiding this comment.
Cancelling the run on the first isolated api, get, or create call means multi-command tasks cannot be evaluated. Can we prevent remote execution without stopping the collection of the remaining commamds?
| } | ||
|
|
||
| func gradeInput(calls []invocation, output string) string { | ||
| if usage := gradedUsage(calls); usage != "" { |
There was a problem hiding this comment.
This might create a false positive or negative because negative tasks can repeat the requested resource ID in an unexecuted command, while task 31 can fail because its required authorization explanation is discarded. This discards the visible assistant output and grades only the command text, is there. way to grade those together or define separate checks?
| @@ -0,0 +1,64 @@ | |||
| module github.com/hashicorp/tfctl-cli/evals | |||
|
|
|||
| go 1.26.5 | |||
There was a problem hiding this comment.
CI installs Go from the root go.mod (1.26.4), while this module requires 1.26.5. I think we either align the Go versions or configure CI to install the eval module’s required version
| } | ||
| } | ||
| if opts.JSON { | ||
| return json.NewEncoder(opts.Stdout).Encode(output) |
There was a problem hiding this comment.
--json returns immediately after encoding the result, so failed or errored tasks produce exit code 0. We can emit the JSON and then perform the same task-status validation used by text output
|
|
||
| .PHONY: eval | ||
| eval: go/install | ||
| @PATH="$(abspath $(dir $(BIN_PATH))):$$PATH" go -C evals run . $(EVAL_ARGS) |
There was a problem hiding this comment.
go/install writes the current binary to GOBIN or GOPATH/bin, but this prepends dist/ to PATH. If dist/tfctl exists from an earlier build, the eval would use that stale binary instead of the source just installed. Either we cld build directly to $(BIN_PATH) or put the actual go install destination first in PATH
|
Are we able to have equivalent replacements for the following evals: plain delete guidance, completed-run log lookup, missing-workspace stop behavior, expired authentication handling, unauthorized workspace deletion, and irreversible project deletion? And also for the constraints agains cturl/wget, terraform destroy, and excessive tool calls? |
2 participants
Description
Can connect to an openai or bedrock model provider.
Testing:
Follow the README. Authenticate to bedrock using doormat aws and run:
EVAL_PROVIDER=bedrock EVAL_MODEL=us.openai.gpt-5.6-luna make eval/saveto run all evals
PCI review checklist
I have documented a clear reason for, and description of, the change I am making.
If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
If applicable, I've documented the impact of any changes to security controls.
Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.