Add azd ai dataset extension for Foundry datasets - #9499
Add azd ai dataset extension for Foundry datasets#9499Mohamed Hessien (m7md7sien) wants to merge 40 commits into
Conversation
Datasets are a general Foundry asset that fine-tuning and other scenarios need as much as evaluation does, so the CRUD group ships as its own extension rather than inside azure.ai.evaluations. Generation stays with the evaluation extension, because it writes that extension's config file. Standalone: no dependency on azure.ai.evaluations, its own go module, lint workflow and release pipeline.
Adds internal/messages, holding the extension's errors, warnings and status lines behind one named function each, grouped under Datasets, Config, Output and Talking to the service. Wording is unchanged; the call sites now name the situation instead of spelling out the sentence. Cobra help, wire constants and table headers stay where they are.
Three things carried over from the extension this was carved out of. The User-Agent announced azure-ai-evaluations and the debug log was written to azd-ai-eval-<date>.log, so a dataset request could not be told from an eval one in either a service log or a local one. The FORMAT column was worse than wrong: format is a field the API accepts on upload and never sends back, so the column was blank for every dataset ever listed. It shows type, which the service does return.
Carved from the same code, so it had the same two: a nil Environment or GetValue response panicking inside the endpoint cascade, and an azd client abandoned when endpoint or credential setup failed.
Same credential failure and same 401/403 path as the eval extension, which this was carved from.
Same intermittent credential failure as the eval extension.
Same three as the eval extension, which this was carved from.
|
Azure Pipelines: 7 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
A BOM at the head of a .jsonl is uploaded as part of the first row's first key, so every consumer of the dataset sees one malformed record -- and nothing fails until something tries to read that row. Notepad, VS Code on Windows and PowerShell all write one. A file holding nothing but a BOM is still empty, and is still refused as such.
Found by bug-bashing the off-path cases. A missing dataset reported "has no versions", which is not what happened -- a dataset cannot exist with no versions, and the service answers an unknown name with an empty list rather than a 404. Listing its versions was worse: it said "No datasets found", as though the project had none at all. Both now say the name is not a dataset and point at the listing, matching how the evaluations extension already answers the same question. A name with a space took a file upload and then a 400 carrying four levels of nested JSON, with the sentence that matters at the bottom. The rule is simple and documented, so it is checked before the round trip.
A mistyped path surfaced as 'GetFileAttributesEx <path>: The system cannot find the file specified.' The syscall name is noise to the person who mistyped it.
--from-file <file> resolved to the file's DIRECTORY, and the upload helper then took whichever .jsonl sorted first. Pointing at one dataset in a folder holding several registered a different one under that name. A test pinned the old behaviour: 'a file resolves to the directory the upload scans'.
The unit tests cover the client layer and the helpers; nothing exercised the command surface a user touches - flag parsing, exit codes, the rendered table, and whether -o json emits something a script can consume. 18 tests behind the live tag, gated on AZURE_AI_DATASET_E2E_LIVE, type-checked by ci-test.ps1 like the eval extension's suites.
|
Azure Pipelines: 7 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.
Pull request overview
Adds a standalone Foundry dataset extension with CRUD/versioning commands, API clients, tests, documentation, and release automation.
Changes:
- Adds dataset creation, updates, listing, inspection, deletion, and version listing.
- Adds endpoint resolution, validation, structured errors, and JSON/table output.
- Adds unit/live tests plus build, lint, and release infrastructure.
Reviewed changes
Copilot reviewed 49 out of 50 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
eng/pipelines/release-ext-azure-ai-dataset.yml |
Adds extension release pipeline. |
cli/azd/extensions/azure.ai.dataset/version.txt |
Sets initial beta version. |
cli/azd/extensions/azure.ai.dataset/tests/cli/harness_test.go |
Adds live CLI harness. |
cli/azd/extensions/azure.ai.dataset/tests/cli/dataset_test.go |
Tests dataset CLI workflows. |
cli/azd/extensions/azure.ai.dataset/README.md |
Documents installation and commands. |
cli/azd/extensions/azure.ai.dataset/main.go |
Adds extension entry point. |
cli/azd/extensions/azure.ai.dataset/internal/version/version.go |
Defines build metadata. |
cli/azd/extensions/azure.ai.dataset/internal/pkg/dataset_api/version_test.go |
Tests version handling. |
cli/azd/extensions/azure.ai.dataset/internal/pkg/dataset_api/uri_test.go |
Tests URI resolution. |
cli/azd/extensions/azure.ai.dataset/internal/pkg/dataset_api/upload_version_test.go |
Tests conflict-aware uploads. |
cli/azd/extensions/azure.ai.dataset/internal/pkg/dataset_api/operations.go |
Implements dataset and storage operations. |
cli/azd/extensions/azure.ai.dataset/internal/pkg/dataset_api/operations_wire_test.go |
Tests API request contracts. |
cli/azd/extensions/azure.ai.dataset/internal/pkg/dataset_api/models.go |
Defines API models and file handling. |
cli/azd/extensions/azure.ai.dataset/internal/pkg/dataset_api/list.go |
Implements listing and version ordering. |
cli/azd/extensions/azure.ai.dataset/internal/pkg/dataset_api/download_wire_test.go |
Tests download HTTP behavior. |
cli/azd/extensions/azure.ai.dataset/internal/pkg/dataset_api/download_test.go |
Tests blob selection logic. |
cli/azd/extensions/azure.ai.dataset/internal/pkg/dataset_api/bom_test.go |
Tests BOM and file handling. |
cli/azd/extensions/azure.ai.dataset/internal/messages/messages.go |
Centralizes user-facing messages. |
cli/azd/extensions/azure.ai.dataset/internal/foundry/projectctx/validator.go |
Validates Foundry endpoints. |
cli/azd/extensions/azure.ai.dataset/internal/foundry/projectctx/types.go |
Defines endpoint-resolution types. |
cli/azd/extensions/azure.ai.dataset/internal/foundry/projectctx/store.go |
Reads shared project context. |
cli/azd/extensions/azure.ai.dataset/internal/foundry/projectctx/resolver.go |
Implements endpoint precedence. |
cli/azd/extensions/azure.ai.dataset/internal/foundry/projectctx/resolver_test.go |
Tests endpoint resolution. |
cli/azd/extensions/azure.ai.dataset/internal/exterrors/errors.go |
Adds structured error helpers. |
cli/azd/extensions/azure.ai.dataset/internal/exterrors/codes.go |
Defines structured error codes. |
cli/azd/extensions/azure.ai.dataset/internal/cmd/table_test.go |
Tests table rendering. |
cli/azd/extensions/azure.ai.dataset/internal/cmd/surface_test.go |
Tests command surface and flags. |
cli/azd/extensions/azure.ai.dataset/internal/cmd/root.go |
Builds the command tree. |
cli/azd/extensions/azure.ai.dataset/internal/cmd/output.go |
Implements JSON and text output. |
cli/azd/extensions/azure.ai.dataset/internal/cmd/output_test.go |
Tests output formatting. |
cli/azd/extensions/azure.ai.dataset/internal/cmd/names.go |
Validates dataset names. |
cli/azd/extensions/azure.ai.dataset/internal/cmd/names_test.go |
Tests naming constraints. |
cli/azd/extensions/azure.ai.dataset/internal/cmd/helpers_test.go |
Tests command helpers. |
cli/azd/extensions/azure.ai.dataset/internal/cmd/debug.go |
Configures debug logging. |
cli/azd/extensions/azure.ai.dataset/internal/cmd/dataset.go |
Implements dataset commands. |
cli/azd/extensions/azure.ai.dataset/internal/cmd/context.go |
Creates command service context. |
cli/azd/extensions/azure.ai.dataset/internal/cmd/artifacts.go |
Defines persistence helpers. |
cli/azd/extensions/azure.ai.dataset/internal/cmd/apiversions.go |
Defines service API versions. |
cli/azd/extensions/azure.ai.dataset/go.mod |
Defines the Go module. |
cli/azd/extensions/azure.ai.dataset/extension.yaml |
Declares extension metadata. |
cli/azd/extensions/azure.ai.dataset/cspell.yaml |
Configures spell checking. |
cli/azd/extensions/azure.ai.dataset/ci-test.ps1 |
Adds CI test execution. |
cli/azd/extensions/azure.ai.dataset/ci-build.ps1 |
Adds CI binary builds. |
cli/azd/extensions/azure.ai.dataset/CHANGELOG.md |
Records the initial release. |
cli/azd/extensions/azure.ai.dataset/build.sh |
Adds Unix cross-platform builds. |
cli/azd/extensions/azure.ai.dataset/build.ps1 |
Adds Windows cross-platform builds. |
cli/azd/extensions/azure.ai.dataset/.golangci.yaml |
Configures Go linting. |
cli/azd/extensions/azure.ai.dataset/.gitignore |
Ignores generated artifacts. |
.github/workflows/lint-ext-azure-ai-dataset.yml |
Adds GitHub lint workflow. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Two lines of ci-test.ps1 assumed PowerShell 6 or later. $IsWindows does not exist on Windows PowerShell 5.1, so the .exe suffix was never appended; and Join-Path there takes a single child path, so the three-argument call failed outright. The script then found no gotestsum, printed the fallback notice, and ran bare `go test` with no JUnit report -- while still exiting 0, which is why it went unnoticed.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 58 out of 59 changed files in this pull request and generated no new comments.
Suppressed comments (13)
cli/azd/extensions/azure.ai.dataset/internal/cmd/dataset.go:89
- [azd-code-reviewer] The point-read probe also treats every error as “not found.” If authentication expires or the service fails after an empty listing,
updatereports that the asset is absent. Ignore only 404 responses and surface other probe failures.
for _, v := range firstDatasetVersions {
if _, err := ec.datasetClient.GetDataset(
ctx, name, v, ProjectEndpointAPIVersion,
); err == nil {
exists = true
cli/azd/extensions/azure.ai.dataset/ci-build.ps1:45
- [azd-code-reviewer] These linker targets still use the evaluation module path. The release pipeline invokes this script, so the assignments miss
azureaidataset/internal/version.Versionand every published binary sendsazd-ext-azure-ai-dataset/devinstead of its release version. Stamp the dataset module path here.
"-X 'azureaieval/internal/version.Version=$Version' " +
"-X 'azureaieval/internal/version.Commit=$SourceVersion' " +
"-X 'azureaieval/internal/version.BuildDate=$(Get-Date -Format o)' "
cli/azd/extensions/azure.ai.dataset/build.sh:36
- [azd-code-reviewer] The local cross-platform build also stamps the copied evaluation module path, so these binaries retain the
devuser-agent version. PointVERSION_PATHat this module.
VERSION_PATH="azureaieval/internal/version"
cli/azd/extensions/azure.ai.dataset/build.ps1:44
- [azd-code-reviewer] This PowerShell build stamps
azureaieval, not the dataset module, leavingversion.Versionatdev. Use the module path declared by this extension.
$VERSION_PATH = "azureaieval/internal/version"
cli/azd/extensions/azure.ai.dataset/internal/cmd/dataset.go:80
- [azd-code-reviewer] A listing failure is currently converted into
exists == false. In particular,updateturns a 403, timeout, or service failure into the misleading “dataset does not exist” error instead of preserving the real failure. Only a 404/empty result should enter the consistency probe.
This issue also appears on line 85 of the same file.
existing, err := ec.datasetClient.ListDatasetVersions(
ctx, name, ProjectEndpointAPIVersion,
)
exists := err == nil && existing != nil && len(existing.Value) > 0
if !exists {
cli/azd/extensions/azure.ai.dataset/internal/cmd/dataset.go:366
- [azd-code-reviewer] This contradicts the new idempotent-delete contract and its live test: whenever the service returns 404 rather than 204 for an absent version, cleanup fails. Treat not-found as successful deletion and only wrap other failures.
if err := ec.datasetClient.DeleteDatasetVersion(
ctx, name, version, ProjectEndpointAPIVersion,
); err != nil {
if dataset_api.IsNotFound(err) {
return messages.DatasetVersionNotFound(name, version)
cli/azd/extensions/azure.ai.dataset/internal/cmd/root.go:52
- [azd-code-reviewer] None of these runnable commands registers SDK flag options for inherited
--output. As a result, arbitrary values such as-o yamlsucceed and silently render a table, while generated metadata cannot advertise the supported values. Registerjson/tablewith atabledefault on each runnable command, followingextensions/azure.ai.skills/internal/cmd/skill_list.go:51.
rootCmd.AddCommand(
newDatasetCreateCommand(),
newDatasetUpdateCommand(),
newDatasetListCommand(),
newDatasetShowCommand(),
newDatasetDeleteCommand(),
newDatasetVersionsCommand(),
)
cli/azd/extensions/azure.ai.dataset/internal/messages/messages.go:358
- [azd-code-reviewer] This helper is also called when
--no-promptwas not supplied, sodataset create <name>anddataset delete <name>falsely tell users they are running non-interactively. Since these commands do not prompt in either mode, report only that the flag is required.
cli/azd/extensions/azure.ai.dataset/README.md:31 - [azd-code-reviewer] The companion extension exposes
azd ai eval generate, notazd ai eval dataset generate, so this onboarding command fails with “unknown command.” The companion PR also namesazure.eval.yaml; avoid pinning the staleevals/eval.yamlpath here.
Generation is `azd ai eval dataset generate`, in `azure.ai.evaluations`, and
stays there: it writes the `datasets:` entry in `evals/eval.yaml`, which is that
extension's file. Splitting the two would leave a generated dataset registered
with the service but absent from the configuration, so `azd up` would not
reconcile it and no eval could name it.
cli/azd/extensions/azure.ai.dataset/README.md:41
- [azd-code-reviewer] This documented cascade omits two implemented sources and therefore gives the wrong precedence. Global project context is checked before host variables, and
AZURE_AI_PROJECT_ENDPOINTis a fallback at both environment levels.
Every command resolves the Foundry project endpoint in this order:
1. `--project-endpoint`
2. `FOUNDRY_PROJECT_ENDPOINT` in the active azd environment
3. the host environment variable of the same name
cli/azd/extensions/azure.ai.dataset/.gitignore:5
- [azd-code-reviewer] Debug mode creates
azd-ai-dataset-<date>.log, but this copied pattern ignores eval logs instead. Dataset debug logs will remain untracked files in the extension directory.
# Debug log written when --debug or AZD_EXT_DEBUG is set.
azd-ai-eval-*.log
cli/azd/extensions/azure.ai.dataset/tests/cli/dataset_test.go:51
- [azd-code-reviewer] Cleanup silently returns when listing or JSON decoding fails, and it also ignores each delete result. A transient cleanup failure therefore leaves live Foundry datasets behind while the suite still passes. Assert every cleanup operation so leaked state is reported.
cli/azd/extensions/azure.ai.dataset/CHANGELOG.md:17 - [azd-code-reviewer] No CLI command calls
DownloadDatasetContent; only internal tests do. This release note therefore advertises a content-read capability users cannot access. Remove the bullet or add the corresponding public command before release.
- Reads dataset content back, whether the service hands out a blob URI or the
container holding it.
The feed still serves beta.5, which predates the SAS redaction, the version-selection error propagation and pagination. Bumping so a bug bash tester can tell which build they have.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 58 out of 59 changed files in this pull request and generated no new comments.
Suppressed comments (15)
cli/azd/extensions/azure.ai.dataset/ci-build.ps1:45
- These linker targets still use the evaluation module path, so the release pipeline's
-Xflags do not stamp this extension'sVersion,Commit, orBuildDate. In particular, the dataset User-Agent continues to reportdev, defeating the identity/versioning change. Targetazureaidataset/internal/versioninstead.
"-X 'azureaieval/internal/version.Version=$Version' " +
"-X 'azureaieval/internal/version.Commit=$SourceVersion' " +
"-X 'azureaieval/internal/version.BuildDate=$(Get-Date -Format o)' "
cli/azd/extensions/azure.ai.dataset/build.ps1:44
- The local PowerShell build also points its linker stamps at the evaluation module, so locally packaged dataset binaries retain the default
dev/none/unknownvalues. Use this module's import path.
$VERSION_PATH = "azureaieval/internal/version"
cli/azd/extensions/azure.ai.dataset/build.sh:36
- The shell build stamps
azureaieval, which is not this module, so its generated binaries do not receive the dataset version metadata. Use the dataset module path.
VERSION_PATH="azureaieval/internal/version"
cli/azd/extensions/azure.ai.dataset/README.md:28
- Both references are stale relative to the companion evaluation extension: its surface is
azd ai eval generate, and its canonical configuration isazure.eval.yaml, not aneval dataset generatecommand writingevals/eval.yaml. As written, the documented command does not exist.
Generation is `azd ai eval dataset generate`, in `azure.ai.evaluations`, and
stays there: it writes the `datasets:` entry in `evals/eval.yaml`, which is that
cli/azd/extensions/azure.ai.dataset/README.md:41
- This advertised resolution order omits two implemented sources: the shared project context is checked before host variables, and
AZURE_AI_PROJECT_ENDPOINTis a fallback at both environment levels. Documenting only three levels makes endpoint selection surprising when the omitted higher-precedence value wins.
Every command resolves the Foundry project endpoint in this order:
1. `--project-endpoint`
2. `FOUNDRY_PROJECT_ENDPOINT` in the active azd environment
3. the host environment variable of the same name
cli/azd/extensions/azure.ai.dataset/.gitignore:5
- Debug logging now writes
azd-ai-dataset-*.log, but this copied evaluation pattern no longer matches it. Running with--debugfrom the extension directory leaves generated logs unignored.
azd-ai-eval-*.log
cli/azd/extensions/azure.ai.dataset/internal/cmd/dataset.go:80
- A non-404 listing failure is discarded here. For example, a 403 or timeout makes
updatereport that the dataset does not exist, whilecreateproceeds toward an upload without establishing that the name is free. Only not-found/empty results should enter the eventual-consistency probe; propagate other listing failures.
existing, err := ec.datasetClient.ListDatasetVersions(
ctx, name, ProjectEndpointAPIVersion,
)
exists := err == nil && existing != nil && len(existing.Value) > 0
if !exists {
cli/azd/extensions/azure.ai.dataset/internal/cmd/dataset.go:299
- This treats the eventually consistent version listing as authoritative even though the upload path explicitly handles it lagging after a publish. Immediately after
create,showcan incorrectly return “no dataset”; immediately afterupdate, it can show the previous version as latest. Resolve latest with point reads past the listed version (bounded, as the upload conflict walk is) before rendering or declaring the dataset absent.
list, err := ec.datasetClient.ListDatasetVersions(ctx, name, ProjectEndpointAPIVersion)
if err != nil {
return messages.ResolvingLatestDatasetVersion(name, err)
}
// The service answers an unknown name with an empty list rather
cli/azd/extensions/azure.ai.dataset/internal/cmd/output.go:29
- No leaf command registers
azdext.RegisterFlagOptions, so unsupported values such as-o yamlare accepted and silently fall through to human output. Register the supportedjson/tablevalues (and the intended default) on every runnable command so help, metadata, completion, and parse-time validation agree with the renderer.
v, err := cmd.Flags().GetString("output")
if err != nil {
return ""
}
return strings.ToLower(v)
cli/azd/extensions/azure.ai.dataset/internal/pkg/dataset_api/models.go:44
- The service compatibility fields leak into
-o json, because commands encodeDatasetdirectly. The same logical URI is therefore emitted asdataUriordata_uridepending on which service spelling arrived, giving scripts an unstable schema. Keep both fields for decoding, but render a normalized output DTO with one canonical field name.
cli/azd/extensions/azure.ai.dataset/README.md:6 - This first-run command cannot work in this PR: the same README states that the registry entry is deliberately absent, so
azd extension install azure.ai.datasetcannot resolve. Replace this with complete local pack/publish instructions until the registry entry lands, or defer the install snippet with that release change.
$ azd extension install azure.ai.dataset
cli/azd/extensions/azure.ai.dataset/README.md:22
- The documented delete invocation omits the required
--versionflag, so copying it always fails with “--version is required.” Include the required argument in the command table.
| `azd ai dataset delete <name>` | Delete a dataset version |
cli/azd/extensions/azure.ai.dataset/tests/cli/dataset_test.go:51
- Cleanup silently returns when listing/decoding fails and ignores every delete result, so failed live runs can leave datasets behind while still reporting successful cleanup. Make cleanup failures visible and require each delete to succeed.
cli/azd/extensions/azure.ai.dataset/internal/foundry/projectctx/resolver.go:139 - These environment-variable reads are not fully reflected in the repository's environment-variable reference:
FOUNDRY_PROJECT_ENDPOINTcurrently names onlyazure.ai.agents, andAZURE_AI_PROJECT_ENDPOINThas no entry. Updatecli/azd/docs/environment-variables.mdso both new extension inputs and their precedence are documented.
cli/azd/extensions/azure.ai.dataset/internal/cmd/root.go:21 - The PR description says every user-facing string lives in
internal/messages/messages.go, but this root help text and the command/flag help strings indataset.goare defined inline. Either centralize those strings as claimed or narrow the PR description; the current “69 messages in one file” review guarantee is not true.
Short: fmt.Sprintf(
"Register and version Foundry datasets from your terminal. %s",
color.YellowString("(Beta)"),
Both pass, so this is coverage rather than a fix, but neither was guarded and both are the kind that fail silently. A protocol-relative nextLink (//otherhost/path) inherits our scheme, so adding ResolveReference is precisely the change that could have opened a bypass; the test asserts the other host receives zero requests, since the pipeline attaches the caller's token. And a two-hop A-to-B-to-A cycle defeats any guard that only remembers the previous link, which is what the sibling eval paginator turned out to have.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 59 out of 60 changed files in this pull request and generated no new comments.
Suppressed comments (10)
cli/azd/extensions/azure.ai.dataset/internal/cmd/dataset.go:232
- The API client explicitly treats a 404 from this endpoint as an unknown/versionless dataset, but this command returns that 404 before reaching the advertised empty-list behavior. Convert
IsNotFound(err)to an emptyDatasetList; otherwiseversions list <unknown>still exits nonzero.
list, err := ec.datasetClient.ListDatasetVersions(ctx, name, ProjectEndpointAPIVersion)
if err != nil {
return messages.ListingDatasetVersions(name, err)
}
cli/azd/extensions/azure.ai.dataset/ci-build.ps1:45
- These linker targets still use the evaluation module path. This module is
azureaidataset, so the release build does not stamp itsVersion,Commit, orBuildDatevariables and the dataset User-Agent remains.../dev.
"-X 'azureaieval/internal/version.Version=$Version' " +
"-X 'azureaieval/internal/version.Commit=$SourceVersion' " +
"-X 'azureaieval/internal/version.BuildDate=$(Get-Date -Format o)' "
cli/azd/extensions/azure.ai.dataset/build.ps1:44
- This copied evaluation module path means local PowerShell builds do not stamp
azureaidataset/internal/version; the binary keeps the development version in its User-Agent. PointVERSION_PATHat this module.
$VERSION_PATH = "azureaieval/internal/version"
cli/azd/extensions/azure.ai.dataset/build.sh:36
- This copied evaluation module path means shell builds do not stamp
azureaidataset/internal/version; the binary keeps the development version in its User-Agent. PointVERSION_PATHat this module.
VERSION_PATH="azureaieval/internal/version"
cli/azd/extensions/azure.ai.dataset/.gitignore:5
- Debug logging creates
azd-ai-dataset-YYYY-MM-DD.log, but this pattern only ignores the old evaluation filename. Dataset debug logs will remain unignored and can be committed accidentally.
azd-ai-eval-*.log
cli/azd/extensions/azure.ai.dataset/ci-test.ps1:10
- The live harness checks
AZURE_AI_DATASET_E2E_LIVE, so this setup instruction names an environment variable that cannot enable the suite.
# AZURE_AI_EVAL_E2E_LIVE and a project endpoint. They are still type-checked
cli/azd/extensions/azure.ai.dataset/README.md:28
- Both references are stale: the evaluation extension exposes
azd ai eval generate, notazd ai eval dataset generate, and its configuration isazure.eval.yaml. As written, the documented command fails and points readers at the obsolete filename.
Generation is `azd ai eval dataset generate`, in `azure.ai.evaluations`, and
stays there: it writes the `datasets:` entry in `evals/eval.yaml`, which is that
cli/azd/extensions/azure.ai.dataset/README.md:41
- This omits two implemented resolution levels and the
AZURE_AI_PROJECT_ENDPOINTfallback. Users relying on shared project context or the legacy key cannot infer why a value wins; document the same four-level order implemented byprojectctx.Resolve.
1. `--project-endpoint`
2. `FOUNDRY_PROJECT_ENDPOINT` in the active azd environment
3. the host environment variable of the same name
cli/azd/extensions/azure.ai.dataset/internal/cmd/dataset.go:93
- Listing failures are currently indistinguishable from an absent dataset. A 403 or transport error makes
existsfalse, and both point probes discard their errors too, soupdatereports “does not exist” whilecreatemay proceed despite being unable to verify existence. Propagate non-404 errors and only treat empty/404 responses as absence.
This issue also appears on line 229 of the same file.
exists := err == nil && existing != nil && len(existing.Value) > 0
cli/azd/extensions/azure.ai.dataset/CHANGELOG.md:17
- No command in this extension calls
DownloadDatasetContent, so the first-release changelog promises a read-back capability that users cannot invoke. Remove this feature bullet or add the corresponding public command before release.
- Reads dataset content back, whether the service hands out a blob URI or the
container holding it.
The version listing is eventually consistent, and latestRegisteredVersion already documents that an unknown dataset and a listing that has not caught up are indistinguishable -- both come back empty with no error. Gating update on that emptiness meant a create followed by an update was refused for a dataset that plainly existed, and sent to create, which fails in turn once the listing catches up and reports the name taken. The two errors pointed at each other. So absence now has to be confirmed by an outright 404 before it refuses. An empty listing is inconclusive and publishes instead, which is what the caller asked for either way: the upload does not care whether the name was new. This is the same tolerance UploadNextVersion already applies to a stale listing on 409. The version probe stays as a fast path, but it can only ever prove existence -- a dataset whose early versions were deleted has none left to find -- so it was never able to close this on its own.
build.ps1, build.sh and ci-build.ps1 began as copies of the eval extension's and kept its module path, so -X named azureaieval/internal/version in a module called azureaidataset. The linker discards -X for a symbol that does not exist, so this failed silently: every release binary reported version dev in its User-Agent and no service-side log could tell which build a caller was running. The same copy left .gitignore ignoring azd-ai-eval-*.log while debug.go writes azd-ai-dataset-*.log, so debug logs were never ignored, and ci-test.ps1 documenting AZURE_AI_EVAL_E2E_LIVE while the harness reads AZURE_AI_DATASET_E2E_LIVE, so anyone following it would set the wrong variable and see the live suite skip while reporting success. A test now compares the linker path in all three scripts against the module in go.mod, because a silently discarded flag is exactly the kind of regression nothing else would catch. Reported by the Copilot reviewer on the PR.
datasetUploadSource accepts DATA.JSONL both as a named file and when scanning a directory, but ReadFirstJSONLFile compared the extension exactly. The CLI resolves a directory to a single file before calling it, so this was not reachable from either extension's commands, but the two layers disagreed and any caller passing a directory would hit it. Reported by the Copilot reviewer.
…eate README named �zd ai eval dataset generate and evals/eval.yaml; the eval extension's surface is �zd ai eval generate and it writes evals/azure.eval.yaml, reading a legacy eval.yaml only when that is what the project already has. A version that does not exist pointed the reader at �zd ai dataset list, which lists datasets and not versions, so the suggested command could not answer the question that was asked. It now names �ersions list <name>. The shared create/update builder registered --version on both, but the upload derives the next version from it, so create --version 4.0 published 5.0 rather than a first version -- and left the existence probe, which looks for the versions a first publish can carry, unable to find what it had just written. Verified against --help that create no longer offers it and update still does. All reported by the Copilot reviewer.
Copilot review threads — dispositionWorking through the open Copilot threads. All of them predate the current head, so several were already addressed; the rest are fixed in the commits below. Grouping them rather than replying 31 times. Fixed earlier in this PR
Fixed now
Also addressed, by a different route than suggested
Not taking
Resolving these threads now. Anything I have mischaracterised, please reopen. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 60 out of 61 changed files in this pull request and generated no new comments.
Suppressed comments (9)
cli/azd/extensions/azure.ai.dataset/internal/cmd/dataset.go:370
- [azd-code-reviewer] This 404 branch contradicts
TestCLIDeleteIsIdempotent: a missing version still exits nonzero. Treat not-found as a successful no-op and continue to the normal JSON/text result; only other delete failures should be returned.
return err
}
defer ec.Close()
if err := ec.datasetClient.DeleteDatasetVersion(
cli/azd/extensions/azure.ai.dataset/README.md:28
- [azd-code-reviewer] The referenced generation command does not exist in the companion extension; its surface is
azd ai eval generate, and its configuration isazure.eval.yaml. As written, users are sent to an unknown command and the wrong configuration name.
Generation is `azd ai eval generate`, in `azure.ai.evaluations`, and stays
there: it writes the `datasets:` entry in `evals/azure.eval.yaml`, which is that
cli/azd/extensions/azure.ai.dataset/README.md:41
- [azd-code-reviewer] This list omits two sources the implementation checks before the host environment: the
azure.ai.agentsglobal project context and theAZURE_AI_PROJECT_ENDPOINTfallback. Document the actual precedence so users can diagnose why a particular endpoint was selected.
1. `--project-endpoint`
2. `FOUNDRY_PROJECT_ENDPOINT` in the active azd environment
3. the host environment variable of the same name
cli/azd/extensions/azure.ai.dataset/internal/cmd/dataset.go:237
- [azd-code-reviewer] An unknown dataset can return 404 here, so this exits before
renderDatasetscan produce the documented empty-success result.latestRegisteredVersionalready treats this response as versionless. Translate 404 to an empty list so the new CLI tests and implementation agree.
ec, err := newDatasetContext(ctx, endpointFlg)
if err != nil {
return err
cli/azd/extensions/azure.ai.dataset/internal/foundry/projectctx/validator.go:72
- [azd-code-reviewer] Every caller discards
pathWarning, so an endpoint missing/api/projects/<project>is silently accepted and requests are sent to the wrong base path. Either make this path check a validation error or propagate the warning throughResolveand render it before the request.
cli/azd/extensions/azure.ai.dataset/README.md:6 - [azd-code-reviewer] This onboarding command cannot currently resolve the extension because the registry entry is deliberately absent, as lines 54–57 acknowledge. Replace it with complete local
azd x pack/azd x publishinstructions until the registry entry lands, otherwise the first documented step always fails.
$ azd extension install azure.ai.dataset
cli/azd/extensions/azure.ai.dataset/README.md:22
- [azd-code-reviewer] The documented delete invocation omits the mandatory
--versionflag, so copying it fails with “--version is required.” Include the required flag in the command table.
| `azd ai dataset delete <name>` | Delete a dataset version |
cli/azd/extensions/azure.ai.dataset/tests/cli/dataset_test.go:51
- [azd-code-reviewer] Cleanup silently returns when listing or JSON decoding fails and ignores every delete result, so failed cleanup leaves datasets in the shared live project while the suite still reports success. Assert each cleanup operation and decode through the existing helpers so leaked service state is visible.
cli/azd/extensions/azure.ai.dataset/internal/foundry/projectctx/resolver.go:143 - [azd-code-reviewer] This adds reads of
AZURE_AI_PROJECT_ENDPOINT, but that variable is absent from the repository’s environment-variable reference, and the existingFOUNDRY_PROJECT_ENDPOINTentry mentions only the agents extension. Updatecli/azd/docs/environment-variables.mdto document both variables and this extension’s precedence.
The lint/go-fix gate rewrites strings.Split to the Go 1.24 iterator and fails when that rewrite is not already applied.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 60 out of 61 changed files in this pull request and generated no new comments.
Suppressed comments (9)
cli/azd/extensions/azure.ai.dataset/internal/cmd/dataset.go:374
- This reverses the documented and tested idempotence guarantee: when the service returns 404 for an already-absent version,
deleteexits nonzero. Treat not-found as an already-completed delete and continue to the normal human/JSON success response. [azd-code-reviewer]
if dataset_api.IsNotFound(err) {
return messages.DatasetVersionNotFound(name, version)
cli/azd/extensions/azure.ai.dataset/README.md:41
- The documented cascade omits both the persisted
azd ai project setvalue and theAZURE_AI_PROJECT_ENDPOINTfallback, so users may troubleshoot or override the wrong source. Document all four implemented levels in their actual precedence order. [azd-code-reviewer]
Every command resolves the Foundry project endpoint in this order:
1. `--project-endpoint`
2. `FOUNDRY_PROJECT_ENDPOINT` in the active azd environment
3. the host environment variable of the same name
cli/azd/extensions/azure.ai.dataset/internal/cmd/dataset.go:243
- The advertised empty-and-success behavior does not handle the API's 404 form for an unknown dataset.
TestUploadNextVersionTreatsAnUnknownDatasetAsVersionlessexplicitly models/versionsreturning 404, but this command returns that error instead of[]. Normalize only not-found here and keep propagating other failures. [azd-code-reviewer]
list, err := ec.datasetClient.ListDatasetVersions(ctx, name, ProjectEndpointAPIVersion)
if err != nil {
return messages.ListingDatasetVersions(name, err)
}
cli/azd/extensions/azure.ai.dataset/README.md:22
- This usage is incomplete: the implementation requires
--version, so copying the documented command always fails before deletion. Include the required flag in the command table. [azd-code-reviewer]
| `azd ai dataset delete <name>` | Delete a dataset version |
cli/azd/extensions/azure.ai.dataset/internal/foundry/projectctx/resolver.go:139
- This extension newly reads
AZURE_AI_PROJECT_ENDPOINT, but that variable is absent fromcli/azd/docs/environment-variables.md, and the existingFOUNDRY_PROJECT_ENDPOINTentry still says it is agents-only. Update the environment-variable reference so this fallback is discoverable and its precedence is accurate. [azd-code-reviewer]
cli/azd/extensions/azure.ai.dataset/tests/cli/harness_test.go:130 - This test harness cannot actually detect the hang asserted by
TestCLIRequiredValuesFailInsteadOfHanging:exec.Commandhas no deadline, so a regression blocks until the entirego testtimeout. Run subprocesses withexec.CommandContextand a short per-command timeout, and fail with the captured output when it expires. [azd-code-reviewer]
cli/azd/extensions/azure.ai.dataset/CHANGELOG.md:17 - The release note claims a user-visible read-back feature, but no command calls
DownloadDatasetContent; only tests do, and the package is internal to this module. Either expose the claimed capability through the CLI or remove it from the release notes. [azd-code-reviewer]
- Reads dataset content back, whether the service hands out a blob URI or the
container holding it.
cli/azd/extensions/azure.ai.dataset/.golangci.yaml:12
- The repository's Go guidance sets
lllto 125 characters (cli/azd/AGENTS.md:97-108), but this module allows 220, so its dedicated CI accepts lines the repository standard rejects. Restore the 125-character limit and wrap the existing overlong source line. [azd-code-reviewer]
line-length: 220
cli/azd/extensions/azure.ai.dataset/internal/cmd/surface_test.go:245
- Remove the accidental
//embedded in this sentence and wrap it to the repository's 125-character limit. [azd-code-reviewer]
// A message pointing at `azd ai eval dataset ...` is almost always the copy// these commands came from rather than a deliberate cross-extension pointer.
The Blob service answers a container listing one page at a time and returns a NextMarker, which this dropped -- it parsed the first page and stopped. DownloadDatasetContent falls back to listing the container and taking the first .jsonl by name, so a container holding more objects than fit in a page could report no file at all, or a different one, depending on where the page happened to end. The marker is the service's own opaque value replayed into a URL this code builds, so the walk carries none of the risk that following a body-supplied link would. Ported from the evaluations extension, which already had this and the tests for it; this copy had neither. Also makes blobHTTPClient a single shared client, as that copy does. Eighth divergence found between the two copies of dataset_api. Reported by a review agent diffing them.
The URI is the SAS the dataset service issued, not caller input, which is why the evaluations copy already suppresses G704 on the same call. Porting the paginated listing without it failed golangci-lint.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 61 out of 62 changed files in this pull request and generated no new comments.
Suppressed comments (8)
cli/azd/extensions/azure.ai.dataset/internal/cmd/dataset.go:375
- [azd-code-reviewer] This explicitly turns a not-found delete into failure, while
TestCLIDeleteIsIdempotentdefines deleting an unregistered version as successful. Treat 404 as an idempotent success so cleanup scripts do not fail when the version is already absent.
if dataset_api.IsNotFound(err) {
return messages.DatasetVersionNotFound(name, version)
}
cli/azd/extensions/azure.ai.dataset/internal/cmd/dataset.go:99
- [azd-code-reviewer] A non-404 listing failure leaves
exists=false, the probes ignore their errors, andcreateis then allowed to continue. If the transient failure clears whenUploadNextVersionlists again,createcan publish a new version of an existing dataset instead of failing as already-exists. Propagate an inconclusive list error forcreate.
if err := checkAssetExistence(
verb, "dataset", name, exists, dataset_api.IsNotFound(listErr),
); err != nil {
cli/azd/extensions/azure.ai.dataset/README.md:41
- [azd-code-reviewer] This omits two implemented resolution levels:
AZURE_AI_PROJECT_ENDPOINTis a fallback at both environment levels, and persistedazd ai project setcontext is checked before host variables. Users following this order can diagnose or update the wrong source. Document the actual four-level cascade.
Every command resolves the Foundry project endpoint in this order:
1. `--project-endpoint`
2. `FOUNDRY_PROJECT_ENDPOINT` in the active azd environment
3. the host environment variable of the same name
cli/azd/extensions/azure.ai.dataset/internal/cmd/root.go:16
- [azd-code-reviewer] The inherited
--environment/AZD_ENVIRONMENTvalue is discarded here. Both endpoint resolution and version persistence callEnvironment().GetCurrent(), which resolves the project's default environment, so-e stagingcan target the default Foundry project and writeEVAL_DATASET_VERSIONinto the wrong environment. ThreadextCtx.Environmentthrough the context/resolver and use the named environment when it is set.
rootCmd, _ := azdext.NewExtensionRootCommand(azdext.ExtensionCommandOptions{
cli/azd/extensions/azure.ai.dataset/internal/cmd/dataset.go:243
- [azd-code-reviewer] A 404 still returns an error here, contradicting the new command contract and the live tests that require an unknown name to produce an empty successful list. Normalize
IsNotFound(err)to an emptyDatasetList; otherwiseversions list <unknown>still exits 1 on services that report absence with 404.
list, err := ec.datasetClient.ListDatasetVersions(ctx, name, ProjectEndpointAPIVersion)
if err != nil {
return messages.ListingDatasetVersions(name, err)
}
cli/azd/extensions/azure.ai.dataset/internal/cmd/output.go:34
- [azd-code-reviewer] Unsupported output values are silently treated as human/table output because only
jsonis recognized. For example,-o yamlexits successfully with non-YAML text, which can break automation. Register per-command output options with the extension SDK (for examplejsonandtable) so unsupported values fail validation.
return strings.ToLower(v)
}
// isJSON reports whether the command should emit machine-readable output.
func isJSON(cmd *cobra.Command) bool {
return outputFormat(cmd) == outputJSON
cli/azd/extensions/azure.ai.dataset/internal/messages/messages.go:357
- [azd-code-reviewer] This message always claims
--no-promptis active, butrequireFlagis also called during ordinary interactive invocations such asdataset delete <name>. The resulting error is factually wrong; either prompt when allowed or use a mode-neutral required-flag message.
cli/azd/extensions/azure.ai.dataset/README.md:22 - [azd-code-reviewer] The documented delete invocation always fails because the implementation requires
--version. Include the required flag so the command table is directly runnable.
| `azd ai dataset delete <name>` | Delete a dataset version |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 61 out of 62 changed files in this pull request and generated no new comments.
Suppressed comments (7)
cli/azd/extensions/azure.ai.dataset/internal/cmd/dataset.go:376
- A 404 currently becomes
DatasetVersionNotFound, so deleting an already-absent version exits non-zero. This directly contradicts the added idempotence test. Treat not-found as successful deletion and only return other service failures. (azd-code-reviewer)
if dataset_api.IsNotFound(err) {
return messages.DatasetVersionNotFound(name, version)
}
return messages.DeletingDatasetVersion(name, version, err)
cli/azd/extensions/azure.ai.dataset/README.md:42
- The documented cascade omits two implemented sources: persisted
azd ai project setcontext and theAZURE_AI_PROJECT_ENDPOINTfallback at both environment levels. Users can therefore get an endpoint they did not expect without the README explaining its precedence. Document all four levels in resolver order. (azd-code-reviewer)
1. `--project-endpoint`
2. `FOUNDRY_PROJECT_ENDPOINT` in the active azd environment
3. the host environment variable of the same name
cli/azd/extensions/azure.ai.dataset/internal/cmd/dataset.go:249
- An unknown dataset is returned as HTTP 404 by this API, so this branch currently exits with an error even though the new CLI tests require
versions listto succeed with an empty array. Normalize a not-found response to an emptyDatasetList; preserve other failures. (azd-code-reviewer)
list, err := ec.datasetClient.ListDatasetVersions(ctx, name, ProjectEndpointAPIVersion)
if err != nil {
return messages.ListingDatasetVersions(name, err)
cli/azd/extensions/azure.ai.dataset/internal/cmd/dataset.go:309
- The API can return 404 for this version listing, but this path wraps it as a raw “resolving latest version” failure. That contradicts
TestCLIUnknownDatasetIsBrief, which expects the same conciseDatasetNotFoundresult used for an empty listing. Translate not-found before wrapping other errors. (azd-code-reviewer)
list, err := ec.datasetClient.ListDatasetVersions(ctx, name, ProjectEndpointAPIVersion)
if err != nil {
return messages.ResolvingLatestDatasetVersion(name, err)
cli/azd/extensions/azure.ai.dataset/README.md:22
- This documented invocation always fails because
deleterequires--version. Include the required flag so the command table is executable as written. (azd-code-reviewer)
| `azd ai dataset delete <name>` | Delete a dataset version |
cli/azd/extensions/azure.ai.dataset/internal/pkg/dataset_api/models.go:44
- These transport aliases are serialized directly by every
-o jsonpath. A response usingdata_uritherefore emitsdata_uri, while a response usingdataUriemitsdataUri, so scripts receive different schemas for the same field. MapDatasetto a stable CLI output type that always exposes one resolveddataUrifield. (azd-code-reviewer)
cli/azd/extensions/azure.ai.dataset/tests/cli/dataset_test.go:51 - Cleanup silently returns when listing or JSON decoding fails and ignores every delete exit code, so the live suite can pass while leaving datasets behind. Make cleanup failures fail the test instead of swallowing them. (azd-code-reviewer)
Adds
azd ai datasetΓÇö a standalone extension for registering and versioning Foundry datasets.Why its own extension
A dataset is a general Foundry asset. Fine-tuning and other scenarios need one as much as evaluation does, so the CRUD group does not belong inside the evaluation extension. This extension has its own Go module, lint workflow and release pipeline, and no dependency on
azure.ai.evaluations.The line drawn with the evaluation extension: CRUD moved, generation stayed.
generatewrites thedatasets:entry into the evaluation configuration, so moving it would strand a config write across an extension boundary ΓÇö a generated dataset registered with the service and absent from configuration, whichazd upwould not reconcile.create/update/list/show/delete/versionsneed no configuration at all, which is why they could move.Surface
Notable
internal/messages/messages.go) so the whole voice of the CLI can be reviewed in one sitting. 69 messages.TYPE, notFORMAT. The API acceptsformaton upload and never returns it, so the previous column was blank for every dataset ever listed.azd-ext-azure-ai-evaluations) and debug log name (azd-ai-eval-*.log), so a dataset request could not be told from an eval one in a service log or a local one.Verified
go build,go vet -tags live,hero,go test ./...andgofmtclean. Exercised live against a Foundry project: create → 1.0, update → 2.0,versions list,show,list, and published to a local registry for a bug bash through realazd.Not included, deliberately
No
registry.jsonentry and nomicrosoft.foundrybundle dependency. Both need published artifacts; listing an unpublished extension in the bundle breaksazd extension install microsoft.foundryfor everyone. Those land after the first release.Open question for reviewers
azd ai dataset versions list <unknown-name>errors and exits 1 here, while the same command in the evaluation extension (#9500) lists nothing and exits 0. I tried making the eval side match this one and it broke two tests whose names are the argument against it ΓÇö "an unknown name lists nothing rather than failing" and "deleting an unregistered dataset is idempotent, not an error", the second because delete idempotence is verified by listing what is left.A list is a filter rather than a lookup, so empty-and-successful is defensible and
-o jsoncallers range over the array instead of branching on an error. By that reasoning this extension'sversions listis the one that should move.dataset showon an unknown name is not in question: that one is a lookup and should fail, as it does.Draft ΓÇö not requesting reviewers yet.
Fixes #9550