Skip to content

json-schema: add datasets url #123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chlins merged 1 commit into from
Sep 19, 2025
Merged

json-schema: add datasets url #123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions schema/config-schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@
"sourceURL": {
"type": "string"
},
"datasetsURL": {
"type": "array",
"items": {
"type": "string"
}
Comment on lines +76 to +78
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To improve validation, it's a good practice to specify the format for URL strings. Consider adding "format": "uri" to the string item definition. This will ensure that the values in the datasetsURL array are valid URIs. You might consider applying this to docURL and sourceURL as well in a separate change to improve consistency across all URL fields.

            "items": {
              "type": "string",
              "format": "uri"
            }

},
"version": {
"type": "string"
},
Expand Down
23 changes: 23 additions & 0 deletions schema/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,29 @@ func TestConfig(t *testing.T) {
]
}
}
`,
fail: true,
},
// expected failure: datasetsURL is not an array
{
config: `
{
"descriptor": {
"name": "xyz-3-8B-Instruct",
"version": "3.1",
"sourceURL": "https://github.com/xyz/xyz3",
"datasetsURL": "https://example.com/dataset"
},
"config": {
"paramSize": "8b"
},
"modelfs": {
"type": "layers",
"diffIds": [
"sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"
]
}
}
`,
Comment on lines +208 to 226
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The JSON in this test case has inconsistent indentation. For better readability and to maintain a consistent style across the codebase, please format the JSON string with a standard 2-space indentation.

Suggested change
config: `
{
"descriptor": {
"name": "xyz-3-8B-Instruct",
"version": "3.1",
"sourceURL": "https://github.com/xyz/xyz3",
"datasetsURL": "https://example.com/dataset"
},
"config": {
"paramSize": "8b"
},
"modelfs": {
"type": "layers",
"diffIds": [
"sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"
]
}
}
`,
config: `
{
"descriptor": {
"name": "xyz-3-8B-Instruct",
"version": "3.1",
"sourceURL": "https://github.com/xyz/xyz3",
"datasetsURL": "https://example.com/dataset"
},
"config": {
"paramSize": "8b"
},
"modelfs": {
"type": "layers",
"diffIds": [
"sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"
]
}
}
`,

fail: true,
},
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While this negative test case is great for ensuring type safety, it would be beneficial to also add a positive test case to verify that a valid datasetsURL (an array of strings) passes validation. This would make the tests for this new field more comprehensive.

You could add a test case like this after the current one:

		// expected success: datasetsURL is an array
		{
			config: `
{
  "descriptor": {
    "name": "xyz-3-8B-Instruct",
    "version": "3.1",
    "sourceURL": "https://github.com/xyz/xyz3",
    "datasetsURL": [
      "https://example.com/dataset1",
      "https://example.com/dataset2"
    ]
  },
  "config": {
    "paramSize": "8b"
  },
  "modelfs": {
    "type": "layers",
    "diffIds": [
      "sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"
    ]
  }
}
`,
			fail: false,
		},

Expand Down
Loading