Added languageCode and multiSpeakerVoiceConfig properties to SpeechConfig #78
Conversation
WalkthroughThis pull request introduces multi-speaker voice configuration support for speech synthesis. Two new configuration classes— Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ladenedge
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/GenerativeAI/Types/ContentGeneration/Config/SpeechConfig.cs (1)
17-22: Good addition! Consider adding validation for mutual exclusivity.The property is well-documented, and the "Optional" designation is consistent with the nullable type. The XML documentation clearly states mutual exclusivity with
VoiceConfig.Consider adding validation (e.g., in a validation method or property setter) to enforce the mutual exclusivity between
VoiceConfigandMultiSpeakerVoiceConfig, preventing both from being set simultaneously.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/GenerativeAI/Types/ContentGeneration/Config/MultiSpeakerVoiceConfig.cs(1 hunks)src/GenerativeAI/Types/ContentGeneration/Config/SpeakerVoiceConfig.cs(1 hunks)src/GenerativeAI/Types/ContentGeneration/Config/SpeechConfig.cs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/GenerativeAI/Types/ContentGeneration/Config/SpeakerVoiceConfig.cs (1)
src/GenerativeAI/Types/ContentGeneration/Config/VoiceConfig.cs (1)
VoiceConfig(9-16)
src/GenerativeAI/Types/ContentGeneration/Config/SpeechConfig.cs (1)
src/GenerativeAI/Types/ContentGeneration/Config/MultiSpeakerVoiceConfig.cs (1)
MultiSpeakerVoiceConfig(9-16)
src/GenerativeAI/Types/ContentGeneration/Config/MultiSpeakerVoiceConfig.cs (1)
src/GenerativeAI/Types/ContentGeneration/Config/SpeakerVoiceConfig.cs (1)
SpeakerVoiceConfig(9-22)
🔇 Additional comments (4)
src/GenerativeAI/Types/ContentGeneration/Config/MultiSpeakerVoiceConfig.cs (2)
5-10: LGTM!The class structure and documentation are clear and appropriately reference the official API documentation.
1-1: No action needed — ImplicitUsings is enabled project-wide.The project has
ImplicitUsingsenabled across all .csproj files, which meansSystem.Collections.Genericis automatically imported. UsingList<T>without an explicit using directive is valid and correct.src/GenerativeAI/Types/ContentGeneration/Config/SpeechConfig.cs (1)
24-29: LGTM!The property is well-documented with clear format requirements (BCP 47) and an example. The "Optional" designation is consistent with the nullable type.
src/GenerativeAI/Types/ContentGeneration/Config/SpeakerVoiceConfig.cs (1)
5-9: LGTM!Clear documentation with a helpful reference to the official API documentation.
| /// <summary> | ||
| /// Required. All the enabled speaker voices. | ||
| /// </summary> | ||
| [JsonPropertyName("speakerVoiceConfigs")] | ||
| public List<SpeakerVoiceConfig>? SpeakerVoiceConfigs { get; set; } |
There was a problem hiding this comment.
🧩 Analysis chain
Inconsistency between "Required" documentation and nullable type.
The property is documented as "Required" but is declared as a nullable type (List<SpeakerVoiceConfig>?). This creates confusion about the API contract.
Please verify the Google API requirements and either:
- Remove the
?to make the property non-nullable if it is truly required, or - Change the XML documentation from "Required" to "Optional" if nullable is correct
🌐 Web query:
Google Generative AI MultiSpeakerVoiceConfig required fields
💡 Result:
Required fields for MultiSpeakerVoiceConfig (Google Generative AI / Gemini TTS):
- speakerVoiceConfigs (array) — required; must contain exactly two SpeakerVoiceConfig entries. [1][2]
Each SpeakerVoiceConfig must include:
- speaker (string) — required; the speaker name/alias used in the prompt. [1][2]
- voiceConfig (object) — required; the voice configuration for that speaker (e.g., a PrebuiltVoiceConfig with voiceName). [1][2]
Sources:
[1] Google Generative AI / GenerationConfig (MultiSpeakerVoiceConfig, SpeakerVoiceConfig).
[2] Cloud Text‑to‑Speech / MultiSpeakerVoiceConfig (MultispeakerPrebuiltVoice: speakerAlias, speakerId).
Remove nullable annotation to align with required API contract.
The speakerVoiceConfigs field is required by the Google Generative AI API, but the property is declared as nullable (List<SpeakerVoiceConfig>?). This contradicts the "Required" documentation and creates an invalid API contract.
Change public List<SpeakerVoiceConfig>? SpeakerVoiceConfigs to public List<SpeakerVoiceConfig> SpeakerVoiceConfigs (remove the ?).
🤖 Prompt for AI Agents
In src/GenerativeAI/Types/ContentGeneration/Config/MultiSpeakerVoiceConfig.cs
around lines 11-15, the SpeakerVoiceConfigs property is declared nullable
despite being required by the API; change the property signature from a nullable
List<SpeakerVoiceConfig>? to a non-nullable List<SpeakerVoiceConfig> (remove the
'?'), and ensure it's either initialized (e.g., new List<SpeakerVoiceConfig>()
in the declaration or constructor) or validated so it can never be null at
runtime; also remove or update any downstream null-checks that assume it may be
null.
| /// <summary> | ||
| /// Required. The name of the speaker to use. Should be the same as in the prompt. | ||
| /// </summary> | ||
| [JsonPropertyName("speaker")] | ||
| public string? Speaker { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Required. The configuration for the voice to use. | ||
| /// </summary> | ||
| [JsonPropertyName("voiceConfig")] | ||
| public VoiceConfig? VoiceConfig { get; set; } |
There was a problem hiding this comment.
Inconsistency: "Required" properties are nullable types.
Both properties are documented as "Required" but are declared as nullable types (string? and VoiceConfig?). This creates ambiguity in the API contract and could lead to runtime errors if consumers assume non-null values based on the documentation.
Consider one of these solutions:
Option 1 (C# 11+): Use the required modifier to enforce initialization:
/// <summary>
/// Required. The name of the speaker to use. Should be the same as in the prompt.
/// </summary>
[JsonPropertyName("speaker")]
- public string? Speaker { get; set; }
+ public required string Speaker { get; set; }
/// <summary>
/// Required. The configuration for the voice to use.
/// </summary>
[JsonPropertyName("voiceConfig")]
- public VoiceConfig? VoiceConfig { get; set; }
+ public required VoiceConfig VoiceConfig { get; set; }Option 2: If the properties are truly optional (e.g., for deserialization scenarios), update the documentation to remove "Required":
/// <summary>
- /// Required. The name of the speaker to use. Should be the same as in the prompt.
+ /// The name of the speaker to use. Should be the same as in the prompt.
/// </summary>
[JsonPropertyName("speaker")]
public string? Speaker { get; set; }
/// <summary>
- /// Required. The configuration for the voice to use.
+ /// The configuration for the voice to use.
/// </summary>
[JsonPropertyName("voiceConfig")]
public VoiceConfig? VoiceConfig { get; set; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// <summary> | |
| /// Required. The name of the speaker to use. Should be the same as in the prompt. | |
| /// </summary> | |
| [JsonPropertyName("speaker")] | |
| public string? Speaker { get; set; } | |
| /// <summary> | |
| /// Required. The configuration for the voice to use. | |
| /// </summary> | |
| [JsonPropertyName("voiceConfig")] | |
| public VoiceConfig? VoiceConfig { get; set; } | |
| /// <summary> | |
| /// Required. The name of the speaker to use. Should be the same as in the prompt. | |
| /// </summary> | |
| [JsonPropertyName("speaker")] | |
| public required string Speaker { get; set; } | |
| /// <summary> | |
| /// Required. The configuration for the voice to use. | |
| /// </summary> | |
| [JsonPropertyName("voiceConfig")] | |
| public required VoiceConfig VoiceConfig { get; set; } |
| /// <summary> | |
| /// Required. The name of the speaker to use. Should be the same as in the prompt. | |
| /// </summary> | |
| [JsonPropertyName("speaker")] | |
| public string? Speaker { get; set; } | |
| /// <summary> | |
| /// Required. The configuration for the voice to use. | |
| /// </summary> | |
| [JsonPropertyName("voiceConfig")] | |
| public VoiceConfig? VoiceConfig { get; set; } | |
| /// <summary> | |
| /// The name of the speaker to use. Should be the same as in the prompt. | |
| /// </summary> | |
| [JsonPropertyName("speaker")] | |
| public string? Speaker { get; set; } | |
| /// <summary> | |
| /// The configuration for the voice to use. | |
| /// </summary> | |
| [JsonPropertyName("voiceConfig")] | |
| public VoiceConfig? VoiceConfig { get; set; } |
🤖 Prompt for AI Agents
In src/GenerativeAI/Types/ContentGeneration/Config/SpeakerVoiceConfig.cs around
lines 11 to 21, the XML docs mark Speaker and VoiceConfig as "Required" yet the
properties are declared nullable (string? and VoiceConfig?), creating an
inconsistent API contract; fix by making the contract consistent: either (A)
enforce non-nullable initialization by removing the nullable mark and using the
C# 11+ required modifier (or add constructor enforcement/validation) so the
properties are non-nullable, or (B) if they truly can be missing for
deserialization, update the XML docs to remove "Required" (and optionally add
[JsonIgnore(Condition = WhenWritingNull)] or validation logic) — pick one
approach and apply it consistently to both properties.
2 participants
Adds two missing properties to the SpeechConfig class, including the new model classes that make up the multi-speaker config tree. Comments and JSON property names mirror the Google docs. This is meant to close #77.
Summary by CodeRabbit