Skip to content

Conversation

@caozhuozi
Copy link
Contributor

@caozhuozi caozhuozi commented Jul 2, 2025

The field is borrowed from https://github.com/ggml-org/ggml/blob/master/docs/gguf.md#general-metadata:

general.languages

The PR can be considered as part of #55.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Summary of Changes

Hello @caozhuozi, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a new languages capability to the model configuration, allowing for explicit declaration of the languages a model supports. This enhancement improves the descriptive metadata for models and provides clearer documentation for users.

Highlights

  • New Configuration Field: I've added a new optional languages field to the model artifact configuration. This field is intended to specify the languages a model can speak, using ISO 639 two-letter codes, aligning with the ggml project's gguf.md specification.
  • Documentation Update: The docs/config.md file has been updated to include the definition of the new languages field and an example demonstrating its usage within the model artifact configuration JSON.
  • Reference Link: A new reference link to the ISO 639 language codes Wikipedia page has been added to the documentation for clarity and easy lookup.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a languages field to the model configuration documentation. The review identified that the field's type is incorrectly specified in the documentation, and the necessary changes to the Go structs and JSON schema to support this new field are missing, rendering the PR incomplete. A suggestion to align the language tag format with the more expressive IETF BCP 47 standard for better interoperability, as used by the GGUF spec from which this feature is borrowed, has also been provided.

"reasoning": true,
"toolUsage": false
"toolUsage": false,
"languages": ["en", "zh"]
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

The languages field is introduced in the documentation with an example, but the corresponding changes in the Go structs (specs-go/v1/config.go) and JSON schema (schema/config-schema.json) are missing. This discrepancy will lead to inconsistencies and validation failures. The languages field should be added to the ModelCapabilities struct in specs-go/v1/config.go. The JSON schema (schema/config-schema.json) needs to be updated to include the capabilities object in ModelConfig and the languages property within it. New test cases should be added to schema/config_test.go to validate the languages field, including valid and invalid examples.

docs/config.md Outdated

- **languages** _string_, OPTIONAL

What languages can the model speak. Encoded as [ISO 639] two letter codes.
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The description specifies "ISO 639 two letter codes" for the languages field. Considering the PR description mentions this field is derived from the GGUF spec, which employs IETF BCP 47 language tags, it would be beneficial to align with the GGUF spec by using BCP 47 for enhanced interoperability. BCP 47 offers greater expressiveness, accommodating regional variations (e.g., en-US, en-GB, zh-CN), and is a widely recognized standard for language tags.

[oci-media-type]: https://github.com/opencontainers/image-spec/blob/main/descriptor.md#properties
[rfc3339-s5.6]: https://tools.ietf.org/html/rfc3339#section-5.6
[spdx-license-expression]: https://spdx.github.io/spdx-spec/v2.3/SPDX-license-expressions/
[iso-639]: https://en.wikipedia.org/wiki/List_of_ISO_639_language_codes
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Consider updating the link to point to the ISO 639-1 or IETF BCP 47 language tags to align with the language code format being used.

@caozhuozi caozhuozi force-pushed the borrow-gguf-2 branch 2 times, most recently from a68ad8f to e6d666d Compare July 2, 2025 04:16
bergwolf
bergwolf previously approved these changes Jul 7, 2025
aftersnow
aftersnow previously approved these changes Jul 7, 2025
Copy link
Contributor

@aftersnow aftersnow left a comment

Choose a reason for hiding this comment

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

lgtm

chlins
chlins previously approved these changes Jul 11, 2025
Copy link
Member

@chlins chlins left a comment

Choose a reason for hiding this comment

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

lgtm

@chlins chlins enabled auto-merge July 11, 2025 09:36
@chlins
Copy link
Member

chlins commented Jul 11, 2025

@caozhuozi Please resolve the conflicts.

auto-merge was automatically disabled July 15, 2025 08:35

Head branch was pushed to by a user without write access

@caozhuozi caozhuozi dismissed stale reviews from chlins, aftersnow, and bergwolf via bcac9fc July 15, 2025 08:35
@caozhuozi
Copy link
Contributor Author

@chlins rebased. PLTA.

@bergwolf bergwolf added the documentation Improvements or additions to documentation label Jul 31, 2025
Copy link
Member

@bergwolf bergwolf left a comment

Choose a reason for hiding this comment

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

lgtm!

Copy link
Member

@chlins chlins left a comment

Choose a reason for hiding this comment

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

lgtm

@chlins chlins merged commit 209ada0 into modelpack:main Jul 31, 2025
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants