Conversation
WalkthroughThis pull request introduces enhanced deprecation and experimental stage detection for OpenAPI operations and schemas through new extension methods, updates the deprecation evaluation across multiple models to use these methods, refactors test structure to validate ResponseStream generation, and applies conditional Experimental attributes to generated Twitch API client methods for .NET 8+. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/tests/AutoSDK.UnitTests/NamingTests.Methods.cs (1)
46-52: Consider extracting repeatedSettingsinitialization into a shared helper.This same setup appears in multiple tests and can be centralized to reduce noise and maintenance cost.
♻️ Suggested refactor
public partial class NamingTests { + private static Settings CreateSummaryNamingSettings() => Settings.Default with + { + Namespace = "LangSmith", + ClassName = "LangSmithClient", + ClsCompliantEnumPrefix = "x", + MethodNamingConvention = MethodNamingConvention.Summary, + }; + [TestMethod] public void SummaryMethodNames_StripExperimentalPrefixesFromIdentifiers() { - var settings = Settings.Default with - { - Namespace = "LangSmith", - ClassName = "LangSmithClient", - ClsCompliantEnumPrefix = "x", - MethodNamingConvention = MethodNamingConvention.Summary, - }; + var settings = CreateSummaryNamingSettings(); ... } [TestMethod] public void FernAvailabilityBeta_MapsToExperimentalStage() { - var settings = Settings.Default with - { - Namespace = "LangSmith", - ClassName = "LangSmithClient", - ClsCompliantEnumPrefix = "x", - MethodNamingConvention = MethodNamingConvention.Summary, - }; + var settings = CreateSummaryNamingSettings(); ... } }Also applies to: 77-83, 112-118
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tests/AutoSDK.UnitTests/NamingTests.Methods.cs` around lines 46 - 52, Several tests repeatedly construct the same Settings instance (Settings.Default with Namespace="LangSmith", ClassName="LangSmithClient", ClsCompliantEnumPrefix="x", MethodNamingConvention=MethodNamingConvention.Summary); extract this into a shared helper or factory (e.g., CreateDefaultTestSettings or GetBaselineSettings) and have the tests call that helper and modify only fields they need, then replace the duplicate initializations in NamingTests.Methods (and the other occurrences mentioned) with calls to the helper to centralize and reduce duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/libs/AutoSDK/Extensions/OpenApiExtensions.cs`:
- Around line 822-827: The current detection using trimmed.StartsWith(prefix,
...) on ExperimentalStagePrefixes can match inside words (e.g., "Alphabet"), so
update the check in OpenApiExtensions.cs where you iterate
ExperimentalStagePrefixes to ensure the prefix is a standalone token: after
matching the prefix, verify either the prefix reaches the end of the string or
the next character is a non-letter/non-digit (or use a word-boundary regex)
before returning stage; apply the same token-boundary logic to the other similar
checks (the other loops using ExperimentalStagePrefixes and StartsWith at the
other occurrences) to avoid false positives.
- Around line 787-790: TryGetAvailability returns a normalized availability
value (e.g., GenerallyAvailable) but NormalizeExperimentalStage only checks for
the hyphenated string "GENERALLY-AVAILABLE", causing GA items to be treated as
experimental; update NormalizeExperimentalStage (and its callers) to do a
case-insensitive, punctuation-insensitive comparison (e.g., remove
hyphens/underscores and compare to "GENERALLYAVAILABLE") or accept the
normalized enum/value from TryGetAvailability directly and treat that as GA,
ensuring both "GenerallyAvailable" and "GENERALLY-AVAILABLE" map to
non-experimental; adjust the checks used in NormalizeExperimentalStage (and
similar logic at the other occurrences referenced) to use this normalized
comparison.
In `@src/libs/AutoSDK/Models/MethodParameter.cs`:
- Line 97: The current MethodParameter construction only uses
context.Schema.IsDeprecated() and therefore misses parameter-level deprecation
flags; update the IsDeprecated assignment in the MethodParameter builder to
consider both the OpenApiParameter.Deprecated flag and the schema deprecation
(e.g., check context.Parameter?.Deprecated or equivalent and OR it with
context.Schema.IsDeprecated()), so MethodParameter.IsDeprecated reflects either
source of deprecation.
---
Nitpick comments:
In `@src/tests/AutoSDK.UnitTests/NamingTests.Methods.cs`:
- Around line 46-52: Several tests repeatedly construct the same Settings
instance (Settings.Default with Namespace="LangSmith",
ClassName="LangSmithClient", ClsCompliantEnumPrefix="x",
MethodNamingConvention=MethodNamingConvention.Summary); extract this into a
shared helper or factory (e.g., CreateDefaultTestSettings or
GetBaselineSettings) and have the tests call that helper and modify only fields
they need, then replace the duplicate initializations in NamingTests.Methods
(and the other occurrences mentioned) with calls to the helper to centralize and
reduce duplication.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 76c3d040-0f9e-4fe7-baf6-ddf03a23b8c1
📒 Files selected for processing (62)
src/libs/AutoSDK.CLI/Commands/GenerateCommand.cssrc/libs/AutoSDK/Extensions/OpenApiExtensions.cssrc/libs/AutoSDK/Models/EndPoint.cssrc/libs/AutoSDK/Models/MethodParameter.cssrc/libs/AutoSDK/Models/ModelData.cssrc/libs/AutoSDK/Models/PropertyData.cssrc/libs/AutoSDK/Models/TypeData.cssrc/libs/AutoSDK/Naming/Methods/SummaryGenerator.cssrc/libs/AutoSDK/Sources/Data.cssrc/libs/AutoSDK/Sources/Sources.Http.cssrc/tests/AutoSDK.IntegrationTests.Cli/CliTests.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.GuestStarClient.AssignGuestStarSlot.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.GuestStarClient.CreateGuestStarSession.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.GuestStarClient.DeleteGuestStarInvite.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.GuestStarClient.DeleteGuestStarSlot.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.GuestStarClient.EndGuestStarSession.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.GuestStarClient.GetChannelGuestStarSettings.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.GuestStarClient.GetGuestStarInvites.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.GuestStarClient.GetGuestStarSession.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.GuestStarClient.SendGuestStarInvite.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.GuestStarClient.UpdateChannelGuestStarSettings.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.GuestStarClient.UpdateGuestStarSlot.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.GuestStarClient.UpdateGuestStarSlotSettings.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.IGuestStarClient.AssignGuestStarSlot.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.IGuestStarClient.CreateGuestStarSession.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.IGuestStarClient.DeleteGuestStarInvite.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.IGuestStarClient.DeleteGuestStarSlot.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.IGuestStarClient.EndGuestStarSession.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.IGuestStarClient.GetChannelGuestStarSettings.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.IGuestStarClient.GetGuestStarInvites.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.IGuestStarClient.GetGuestStarSession.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.IGuestStarClient.SendGuestStarInvite.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.IGuestStarClient.UpdateChannelGuestStarSettings.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.IGuestStarClient.UpdateGuestStarSlot.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/CLI/twitch/_#G.IGuestStarClient.UpdateGuestStarSlotSettings.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.GuestStarClient.AssignGuestStarSlot.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.GuestStarClient.CreateGuestStarSession.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.GuestStarClient.DeleteGuestStarInvite.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.GuestStarClient.DeleteGuestStarSlot.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.GuestStarClient.EndGuestStarSession.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.GuestStarClient.GetChannelGuestStarSettings.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.GuestStarClient.GetGuestStarInvites.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.GuestStarClient.GetGuestStarSession.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.GuestStarClient.SendGuestStarInvite.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.GuestStarClient.UpdateChannelGuestStarSettings.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.GuestStarClient.UpdateGuestStarSlot.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.GuestStarClient.UpdateGuestStarSlotSettings.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.IGuestStarClient.AssignGuestStarSlot.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.IGuestStarClient.CreateGuestStarSession.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.IGuestStarClient.DeleteGuestStarInvite.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.IGuestStarClient.DeleteGuestStarSlot.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.IGuestStarClient.EndGuestStarSession.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.IGuestStarClient.GetChannelGuestStarSettings.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.IGuestStarClient.GetGuestStarInvites.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.IGuestStarClient.GetGuestStarSession.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.IGuestStarClient.SendGuestStarInvite.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.IGuestStarClient.UpdateChannelGuestStarSettings.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.IGuestStarClient.UpdateGuestStarSlot.g.verified.cssrc/tests/AutoSDK.SnapshotTests/Snapshots/twitch/SystemTextJson/_#G.IGuestStarClient.UpdateGuestStarSlotSettings.g.verified.cssrc/tests/AutoSDK.UnitTests/DataTests.cssrc/tests/AutoSDK.UnitTests/HttpTests.cssrc/tests/AutoSDK.UnitTests/NamingTests.Methods.cs
| if (TryGetAvailability(operation.Extensions, out var availability)) | ||
| { | ||
| return NormalizeExperimentalStage(availability); | ||
| } |
There was a problem hiding this comment.
Fix generally-available normalization mismatch (currently treated as experimental).
TryGetAvailability normalizes to GenerallyAvailable, but NormalizeExperimentalStage only suppresses GENERALLY-AVAILABLE (hyphenated). That returns a non-empty stage and can incorrectly mark GA operations as experimental.
💡 Proposed fix
private static string NormalizeExperimentalStage(string? stage)
{
var normalized = stage?.Trim() ?? string.Empty;
if (normalized.Length == 0)
{
return string.Empty;
}
return normalized.ToUpperInvariant() switch
{
"ALPHA" => "Alpha",
"BETA" => "Beta",
"EXPERIMENTAL" => "Experimental",
- "GENERALLY-AVAILABLE" => string.Empty,
+ "GENERALLY-AVAILABLE" or "GENERALLYAVAILABLE" => string.Empty,
"DEPRECATED" => string.Empty,
_ => normalized,
};
}Also applies to: 971-979, 990-996
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/libs/AutoSDK/Extensions/OpenApiExtensions.cs` around lines 787 - 790,
TryGetAvailability returns a normalized availability value (e.g.,
GenerallyAvailable) but NormalizeExperimentalStage only checks for the
hyphenated string "GENERALLY-AVAILABLE", causing GA items to be treated as
experimental; update NormalizeExperimentalStage (and its callers) to do a
case-insensitive, punctuation-insensitive comparison (e.g., remove
hyphens/underscores and compare to "GENERALLYAVAILABLE") or accept the
normalized enum/value from TryGetAvailability directly and treat that as GA,
ensuring both "GenerallyAvailable" and "GENERALLY-AVAILABLE" map to
non-experimental; adjust the checks used in NormalizeExperimentalStage (and
similar logic at the other occurrences referenced) to use this normalized
comparison.
| foreach (var (prefix, stage) in ExperimentalStagePrefixes) | ||
| { | ||
| if (trimmed.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| return stage; | ||
| } |
There was a problem hiding this comment.
Use token-boundary checks for stage prefixes to avoid false positives.
Current StartsWith on plain prefixes (Alpha, Beta, Experimental) will match unrelated words (e.g., Alphabet), causing wrong stage detection/summary stripping.
💡 Proposed fix
foreach (var (prefix, stage) in ExperimentalStagePrefixes)
{
- if (trimmed.StartsWith(prefix, StringComparison.OrdinalIgnoreCase))
+ if (trimmed.StartsWith(prefix, StringComparison.OrdinalIgnoreCase) &&
+ (prefix.StartsWith("[", StringComparison.Ordinal) ||
+ trimmed.Length == prefix.Length ||
+ " \t:-_".Contains(trimmed[prefix.Length])))
{
return stage;
}
} foreach (var (prefix, _) in ExperimentalStagePrefixes)
{
- if (!trimmed.StartsWith(prefix, StringComparison.OrdinalIgnoreCase))
+ if (!trimmed.StartsWith(prefix, StringComparison.OrdinalIgnoreCase) ||
+ (!prefix.StartsWith("[", StringComparison.Ordinal) &&
+ trimmed.Length > prefix.Length &&
+ !" \t:-_".Contains(trimmed[prefix.Length])))
{
continue;
}
var remainder = trimmed.Substring(prefix.Length).TrimStart(' ', '\t', ':', '-', '_');
return string.IsNullOrWhiteSpace(remainder) ? trimmed : remainder;
}Also applies to: 842-850, 953-961
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/libs/AutoSDK/Extensions/OpenApiExtensions.cs` around lines 822 - 827, The
current detection using trimmed.StartsWith(prefix, ...) on
ExperimentalStagePrefixes can match inside words (e.g., "Alphabet"), so update
the check in OpenApiExtensions.cs where you iterate ExperimentalStagePrefixes to
ensure the prefix is a standalone token: after matching the prefix, verify
either the prefix reaches the end of the string or the next character is a
non-letter/non-digit (or use a word-boundary regex) before returning stage;
apply the same token-boundary logic to the other similar checks (the other loops
using ExperimentalStagePrefixes and StartsWith at the other occurrences) to
avoid false positives.
| Explode: parameter.Explode, | ||
| Settings: context.Settings, | ||
| IsDeprecated: context.Schema.Deprecated, | ||
| IsDeprecated: context.Schema.IsDeprecated(), |
There was a problem hiding this comment.
Preserve parameter-level deprecation when building MethodParameter.
This currently reads only schema deprecation and can miss OpenApiParameter.Deprecated.
💡 Proposed fix
- IsDeprecated: context.Schema.IsDeprecated(),
+ IsDeprecated: parameter.Deprecated || context.Schema.IsDeprecated(),📝 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.
| IsDeprecated: context.Schema.IsDeprecated(), | |
| IsDeprecated: parameter.Deprecated || context.Schema.IsDeprecated(), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/libs/AutoSDK/Models/MethodParameter.cs` at line 97, The current
MethodParameter construction only uses context.Schema.IsDeprecated() and
therefore misses parameter-level deprecation flags; update the IsDeprecated
assignment in the MethodParameter builder to consider both the
OpenApiParameter.Deprecated flag and the schema deprecation (e.g., check
context.Parameter?.Deprecated or equivalent and OR it with
context.Schema.IsDeprecated()), so MethodParameter.IsDeprecated reflects either
source of deprecation.
Summary by CodeRabbit
New Features
x-fern-availabilityextension to properly identify deprecated and beta APIs.Tests