Model the include on every entry core can splice - #7
Merged
m7md7sien merged 3 commits intoAug 20, 2026
Merged
Conversation
Core resolves a ref on every node, but only the evaluator entry modelled it. A configuration with a ref on a dataset or an eval deployed and ran fine, and could not be opened by init, generate or the catalog writers at all: they read the file exactly as written, and the strict decoder refused the very key that pointed at the content. The schema said the same, so an editor underlined it too. Also: an include reached without a project root is refused rather than discarded. Resolution was skipped when there was nowhere to resolve against and the directive was then deleted, so a service mixing an include with inline content deployed only the inline half and failed later as a missing eval. And a relative source inside a refd file is named where it bites. Core rebases only the two path keys it owns, so such a path arrives verbatim and resolves against the configuration rather than the referenced file. The failure said the definition had not been generated; it now says why the path is wrong and what to do instead.
The editing read sees the directive, not the entry behind it, so the duplicate scan had nothing to match on and generate appended a second entry with the same name. The collision then surfaced on the next resolving read, naming a duplicate the author never wrote and could not see in the file in front of them. A configuration that will not resolve is left to the commands that resolve it: failing a generate over an unrelated broken include would be its own surprise. Also corrects the README, which said relative paths inside a refd file resolve against that file. True of the configuration as a whole, false for a ref on a single entry, which is exactly the trap. It now says so and steers to definition.
There was a problem hiding this comment.
Pull request overview
Models $ref includes across evaluation entries and improves diagnostics for unresolved includes and paths.
Changes:
- Adds dataset and eval entry include support to models and schema.
- Prevents silent include loss without a project root.
- Improves include documentation, catalog collision handling, and tests.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
schemas/azure.ai.eval.json |
Adds dataset and eval $ref schemas. |
README.md |
Documents nested include path behavior. |
internal/project/service_target_eval.go |
Rejects unresolved includes without a project root. |
internal/project/service_config_strict_test.go |
Tests missing-root rejection. |
internal/project/ref_resolution_test.go |
Documents unresolved evaluator paths. |
internal/project/ref_on_every_entry_test.go |
Tests editable dataset and eval includes. |
internal/project/eval_config.go |
Models $ref on datasets and evals. |
internal/project/config_keys_test.go |
Updates configuration key contracts. |
internal/messages/messages.go |
Adds actionable include diagnostics. |
internal/cmd/catalog.go |
Detects catalog names supplied by includes. |
internal/cmd/catalog_include_test.go |
Tests included catalog-name handling. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The check only caught a pure ref, where no name is visible. An entry written as a ref with an overlay name -- the shape the README recommends for a rubric -- matched by name and was updated in place, writing source beside the directive. Resolution then produced both a spliced rubric and a source, and the next read rejected the catalog for declaring the rubric twice. Neither shape is editable here. Datasets get the same path diagnostic evaluators got: a relative file inside a refd entry resolves against the configuration, not against that file. And the new include test no longer blesses that broken shape. It wrote the dataset path beside the referenced file and asserted only the name, which is the same silent endorsement this round criticised elsewhere.
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.
m7md7sien
merged commit Aug 20, 2026
974d002
into
feat/azure-ai-evaluations-consolidated
6 of 7 checks passed
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.
Three findings from the final review round.
A `` on a dataset or an eval could not be edited. Core splices any node,
but only the evaluator entry modelled the directive, so such a config deployed
and ran while
init, `generate` and the catalog writers refused to open it.Both structs and the schema now model it.
An include without a project root was discarded, not refused. A service
mixing an include with inline content deployed only the inline half; the failure
then named a missing eval rather than the include nobody could read.
A relative
source:inside a ``'d file is now named where it bites.Core rebases only the two path keys it owns, so the path arrives verbatim and
resolves against the configuration rather than the referenced file. This does not
fix the rebasing -- see below -- but it turns a mystifying "not generated yet"
into an actionable message.
Open question for reviewers
Should a ``'d declaration be allowed to carry a relative
source:atall? Options: refuse it at load, ask azd to rebase extension-owned path keys, or
leave it documented and steer authors to `definition:`. Left undecided rather
than settled unilaterally.
gofmt, vet (untagged, hero, live), the full suite and ajv are all green.