feat: Composable eval construction from registry graders (Phase 1 CLI) #17 - #491
Closed
Shayne Boyer (spboyer) wants to merge 1 commit into
Closed
feat: Composable eval construction from registry graders (Phase 1 CLI) #17#491Shayne Boyer (spboyer) wants to merge 1 commit into
Shayne Boyer (spboyer) wants to merge 1 commit into
Conversation
Phase 1 of composable eval construction from a registry of graders. - `waza registry search <query>`: search configured registry indexes for reusable graders, evals, and datasets (--kind, --registry, --format). - `waza registry add <ref>`: append a ref-based grader entry to eval.yaml and record the resolved module in waza.lock (--eval, --name, --set, --weight, --allow-exec, --dry-run, --yes). - Adds `internal/registry` with ref parser, config with a default waza-evals public source, a stub search catalog, a lockfile writer, and a yaml.Node-based eval.yaml editor. - Resolver integration is stubbed with TODO(#15) markers pending the ref-resolver work from issue #15. Closes #17 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: dd7212db-34f3-4622-8ed7-f24aeef611aa
Contributor
There was a problem hiding this comment.
Pull request overview
Adds the Phase 1 “registry” surface to the Go CLI so users can discover stubbed registry artifacts and append remote grader refs into eval.yaml, while writing a placeholder waza.lock entry (with real resolution deferred to #15).
Changes:
- Introduces
waza registry search(table/JSON output) backed by a stub in-memory catalog. - Introduces
waza registry addto appendref:grader entries toeval.yamland upsert awaza.lockmodule entry. - Adds
internal/registryprimitives (ref parsing, search stub, lockfile I/O, YAML edit helpers) with unit + command tests.
Show a summary per file
| File | Description |
|---|---|
| internal/registry/config.go | Defines registry source/config defaults (public source). |
| internal/registry/evalfile.go | YAML edit helpers (AppendGraderRef) + --set parsing/coercion. |
| internal/registry/evalfile_test.go | Tests YAML editing, --set coercion, and lockfile round-trip. |
| internal/registry/lock.go | Implements waza.lock load/save/upsert and conversion from resolver output. |
| internal/registry/ref.go | Implements registry ref parsing + IsRemote detection. |
| internal/registry/ref_test.go | Tests ref parsing and IsRemote. |
| internal/registry/resolver.go | Adds resolver interface + stub resolver (Phase 1 placeholder). |
| internal/registry/search.go | Adds searcher interface + stub catalog and filtering helpers. |
| internal/registry/search_test.go | Tests search filtering by kind/query/registry + default config. |
| cmd/waza/root.go | Wires the new registry command into the root command. |
| cmd/waza/cmd_registry.go | Defines the waza registry parent command and subcommand wiring. |
| cmd/waza/cmd_registry_search.go | Implements waza registry search flags, validation, and output. |
| cmd/waza/cmd_registry_search_test.go | Tests search output modes and flag validation. |
| cmd/waza/cmd_registry_add.go | Implements waza registry add (eval.yaml append + lockfile upsert). |
| cmd/waza/cmd_registry_add_test.go | Tests add behavior, dry-run behavior, and input validation. |
Review details
Comments suppressed due to low confidence (1)
cmd/waza/cmd_registry_search.go:94
- validateSearchFlags accepts --kind program-grader, but the error message still says “want grader|eval|dataset”, which is confusing for users. Update the message to include program-grader (or remove support for it).
if f.kind != "" {
switch registry.Kind(f.kind) {
case registry.KindGrader, registry.KindEval, registry.KindDataset, registry.KindProgramGrader:
default:
return fmt.Errorf("unsupported --kind %q (want grader|eval|dataset)", f.kind)
}
- Files reviewed: 15/15 changed files
- Comments generated: 5
- Review effort level: Low
Comment on lines
+168
to
+184
| func ParseSetFlag(inputs []string) (map[string]any, error) { | ||
| out := map[string]any{} | ||
| for _, in := range inputs { | ||
| eq := strings.Index(in, "=") | ||
| if eq < 0 { | ||
| return nil, fmt.Errorf("--set %q: expected key=value", in) | ||
| } | ||
| key := strings.TrimSpace(in[:eq]) | ||
| val := strings.TrimSpace(in[eq+1:]) | ||
| if key == "" { | ||
| return nil, fmt.Errorf("--set %q: empty key", in) | ||
| } | ||
| parts := strings.Split(key, ".") | ||
| insertNested(out, parts, coerceScalar(val)) | ||
| } | ||
| return out, nil | ||
| } |
Comment on lines
+131
to
+144
| func mapToYAMLNode(m map[string]any) (*yaml.Node, error) { | ||
| node := &yaml.Node{Kind: yaml.MappingNode, Tag: "!!map"} | ||
| for k, v := range m { | ||
| valNode, err := valueToYAMLNode(v) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| node.Content = append(node.Content, | ||
| &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: k}, | ||
| valNode, | ||
| ) | ||
| } | ||
| return node, nil | ||
| } |
Comment on lines
+49
to
+51
| cmd.Flags().StringVar(&f.kind, "kind", "", "Filter results by artifact kind (grader|eval|dataset)") | ||
| cmd.Flags().StringVar(&f.registry, "registry", "", "Restrict search to a single configured registry source by name") | ||
| cmd.Flags().StringVar(&f.format, "format", "table", "Output format: table|json") |
Comment on lines
+70
to
+101
| config, err := registry.ParseSetFlag(f.sets) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // TODO(#15): call the real resolver. For now the stub returns | ||
| // syntax-derived metadata so we can still write the ref entry. | ||
| resolver := registry.StubResolver{} | ||
| resolution, err := resolver.Resolve(ref) | ||
| if err != nil { | ||
| return fmt.Errorf("resolving %s: %w", ref, err) | ||
| } | ||
|
|
||
| if resolution.Kind == registry.KindProgramGrader { | ||
| if !f.allowExec && !f.yes { | ||
| ok, err := confirmProgramGrader(out, in, ref.String()) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if !ok { | ||
| return errors.New("aborted by user; re-run with --allow-exec to skip the prompt") | ||
| } | ||
| } | ||
| resolution.Trusted = true | ||
| } | ||
|
|
||
| entry := registry.GraderRefEntry{ | ||
| Ref: ref.String(), | ||
| Name: f.name, | ||
| Weight: f.weight, | ||
| Config: config, | ||
| } |
Comment on lines
+59
to
+64
| if g["ref"] != "github.com/waza-evals/fact#factuality@v1.0.0" { | ||
| t.Errorf("ref: %v", g["ref"]) | ||
| } | ||
| if g["name"] != "factuality_strict" { | ||
| t.Errorf("name: %v", g["name"]) | ||
| } |
Member
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #17
Phase 1 of composable eval construction: introduces the
waza registryCLI subcommand tree for discovering and adding remote grader presets toeval.yaml, per §9 / §13 ofdocs/research/waza-eval-registry-design.md.What's new
CLI
waza registry search <query>(cmd/waza/cmd_registry_search.go)--kind grader|eval|dataset,--registry <name>,--format table|jsonref / kind / stars / descriptiontable or a JSON arraywaza registry add <ref>(cmd/waza/cmd_registry_add.go)--eval <path>(defaulteval.yaml),--name <alias>,--set key.path=value,--weight,--allow-exec,--dry-run,--yes- ref: …grader entry toeval.yaml(preserving surrounding formatting viayaml.Node) and upserts the resolved module intowaza.lockprogram-gradermodules unless--allow-execor--yesis setwaza registryparent command wired into the root cobra commandinternal/registrypackageref.go—Refparser for<host>/<owner>/<repo>[/path][#export]@<version>config.go—Source/Configwith a default publicwaza-evalsGitHub org sourcesearch.go—Searcherinterface + a stub catalog so the command is exercisable end-to-endresolver.go—Resolverinterface +StubResolverreturning ref-derived metadata (markedTODO(#15))lock.go—waza.lockwriter (schema_version 1, dedup by ref)evalfile.go—AppendGraderRefandParseSetFlag(dot-nested keys, scalar coercion)Dependency on #15
Full end-to-end resolution (fetching module manifests, verifying digests, populating
waza.lockfrom real registry metadata) depends on the ref resolver from #15, which is not yet merged. The resolver call site is stubbed with aStubResolverandTODO(#15)markers so this PR can land the CLI surface and the file/lockfile plumbing independently. Swapping in the real resolver from #15 will be a small, localized change incmd_registry_add.go.Tests
internal/registry/ref_test.go,search_test.go,evalfile_test.go— parser, search filter, yaml editing,--setcoercioncmd/waza/cmd_registry_search_test.go,cmd_registry_add_test.go— flag validation, table/JSON output, dry-run, eval.yaml + waza.lock writeAll packages pass
go test ./...andgolangci-lint runlocally.