Skip to content

[Review Only] SDK regeneration#53

Draft
chiragjn wants to merge 1 commit into
mainfrom
nm-publish-1
Draft

[Review Only] SDK regeneration#53
chiragjn wants to merge 1 commit into
mainfrom
nm-publish-1

Conversation

@chiragjn
Copy link
Copy Markdown
Member

No description provided.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 12, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: cb9d8bee-2980-4b21-9367-5f689a49e513

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch nm-publish-1

Comment @coderabbitai help to get the list of available commands and usage tips.

@chiragjn
Copy link
Copy Markdown
Member Author

@coderabbitai Review this PR

  1. First, Resolve all previous comments, all of this is autogenerated code, I will not address any of your concerns
  2. Then, Please specifically point out interface breaking changes. These include but not limited to
  • Some argument was removed from some function that belongs in the public api client.v1...
    You can read reference.md at root of repo to figure out the changed methods. Ignore the internal group
  • breaking changes in types - fields getting removed or type changing
  • types being removed completely
  • enum members getting removed
  • Anything that break code relying on this SDK

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 12, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

I'll start by resolving previous comments and gathering the information needed for the breaking change analysis.


🏁 Script executed:

#!/bin/bash
# List all review comments by coderabbitai on this PR
gh pr view 53 --repo truefoundry/truefoundry-python-sdk --json number,title

# List review threads to find comment IDs to resolve
gh api graphql -f query='
{
  repository(owner: "truefoundry", name: "truefoundry-python-sdk") {
    pullRequest(number: 53) {
      reviewThreads(first: 50) {
        nodes {
          id
          isResolved
          comments(first: 1) {
            nodes {
              author { login }
              body
              id
            }
          }
        }
      }
    }
  }
}'

Length of output: 125


🏁 Script executed:

#!/bin/bash
# Read reference.md at root of repo
cat reference.md | head -500

Length of output: 6451


🏁 Script executed:

#!/bin/bash
# Get the list of changed files in the PR
gh pr diff 53 --repo truefoundry/truefoundry-python-sdk --name-only

Length of output: 2214


🏁 Script executed:

#!/bin/bash
# Read full reference.md to understand the full public API surface
wc -l reference.md
cat reference.md

Length of output: 161305


🏁 Script executed:

#!/bin/bash
# Get the full diff to examine breaking changes - focus on removed lines (starts with -)
# Check __init__.py for removed exports
gh pr diff 53 --repo truefoundry/truefoundry-python-sdk -- src/truefoundry_sdk/__init__.py 2>/dev/null | grep "^-" | grep -v "^---" | head -100

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Check diff for types that might have breaking changes - look at removals
for f in \
  src/truefoundry_sdk/types/application_type.py \
  src/truefoundry_sdk/types/global_settings.py \
  src/truefoundry_sdk/types/guardrail_settings.py \
  src/truefoundry_sdk/types/identity_provider_mapping.py \
  src/truefoundry_sdk/types/model_cost_metric.py \
  src/truefoundry_sdk/types/model_provider_account.py \
  src/truefoundry_sdk/types/role_with_resource.py \
  src/truefoundry_sdk/types/session.py \
  src/truefoundry_sdk/types/subject_type.py \
  src/truefoundry_sdk/types/team.py \
  src/truefoundry_sdk/types/team_manifest.py \
  src/truefoundry_sdk/types/usage_code_snippet.py \
  src/truefoundry_sdk/types/user_metadata.py \
  src/truefoundry_sdk/types/virtual_account_manifest.py \
  src/truefoundry_sdk/types/__init__.py \
  src/truefoundry_sdk/applications/client.py \
  src/truefoundry_sdk/base_client.py; do
  echo "=== $f ==="
  gh pr diff 53 --repo truefoundry/truefoundry-python-sdk -- "$f" 2>/dev/null | grep "^-" | grep -v "^---" | head -50
done

Length of output: 894


🏁 Script executed:

#!/bin/bash
# Try git diff approach comparing the PR branch to the base
git log --oneline -5
git diff HEAD~1 HEAD -- src/truefoundry_sdk/__init__.py | grep "^-" | grep -v "^---" | head -60

Length of output: 138


🏁 Script executed:

#!/bin/bash
# Try to get the full diff of the PR differently
git diff HEAD~1 HEAD --stat
echo "---"
# Check for removals in key files
for f in \
  src/truefoundry_sdk/types/application_type.py \
  src/truefoundry_sdk/types/subject_type.py \
  src/truefoundry_sdk/types/model_cost_metric.py \
  src/truefoundry_sdk/types/global_settings.py \
  src/truefoundry_sdk/types/guardrail_settings.py \
  src/truefoundry_sdk/types/session.py \
  src/truefoundry_sdk/types/virtual_account_manifest.py \
  src/truefoundry_sdk/types/team_manifest.py \
  src/truefoundry_sdk/types/__init__.py \
  src/truefoundry_sdk/__init__.py \
  src/truefoundry_sdk/applications/client.py; do
  echo "=== REMOVALS in $f ==="
  git diff HEAD~1 HEAD -- "$f" | grep "^-" | grep -v "^---" | head -40
done

Length of output: 4295


🏁 Script executed:

#!/bin/bash
# Get detailed diff for remaining files with changes
for f in \
  src/truefoundry_sdk/types/user_metadata.py \
  src/truefoundry_sdk/types/model_cost_metric.py \
  src/truefoundry_sdk/types/usage_code_snippet.py \
  src/truefoundry_sdk/types/internal_artifact_version.py \
  src/truefoundry_sdk/types/internal_list_artifact_versions_response_data_item.py \
  src/truefoundry_sdk/types/flyte_task_template.py \
  src/truefoundry_sdk/types/flyte_task_custom_truefoundry.py \
  src/truefoundry_sdk/applications/client.py \
  src/truefoundry_sdk/base_client.py \
  src/truefoundry_sdk/internal/artifact_versions/client.py; do
  echo "=== FULL DIFF: $f ==="
  git diff HEAD~1 HEAD -- "$f"
  echo ""
done

Length of output: 17821


@chiragjn: No previous review comments were found on this PR, so there is nothing to resolve. Below is the breaking change analysis based on a detailed diff examination.


⚠️ Interface Breaking Changes

1. 🔴 ApplicationType.SPARK_NOTEBOOK enum member removed

File: src/truefoundry_sdk/types/application_type.py

The SPARK_NOTEBOOK = "spark-notebook" member has been removed from the ApplicationType enum. The visitor method's spark_notebook parameter was also removed.

Impact: Any code using ApplicationType.SPARK_NOTEBOOK, passing "spark-notebook" as the application_type filter in client.applications.list(), or using the visitor pattern with a spark_notebook callback will break at runtime.


2. 🔴 UserMetadata.is_primary_sso field removed

File: src/truefoundry_sdk/types/user_metadata.py

-    is_primary_sso: typing_extensions.Annotated[
-        typing.Optional[bool], FieldMetadata(alias="isPrimarySSO"), pydantic.Field(alias="isPrimarySSO")
-    ] = None

Impact: Any code reading .is_primary_sso on a UserMetadata instance will raise an AttributeError.


3. 🔴 FlyteTaskTemplate changed from a concrete class to a Union type

File: src/truefoundry_sdk/types/flyte_task_template.py

-class FlyteTaskTemplate(UniversalBaseModel):
-    id: FlyteTaskId
-    custom: FlyteTaskCustom
+FlyteTaskTemplate = typing.Union[TruefoundryFlyteTaskTemplate, NativeSnowflakeFlyteTaskTemplate]

Impact: Code that:

  • Instantiates FlyteTaskTemplate(id=..., custom=...) directly will break.
  • Accesses .id or .custom attributes directly on a FlyteTaskTemplate value without a discriminated type check will break.
  • Uses isinstance(x, FlyteTaskTemplate) will break (Union types are not valid for isinstance).

ℹ️ Potentially Impactful (Non-strictly-breaking) Changes

4. 🟡 ModelCostMetric union expanded

File: src/truefoundry_sdk/types/model_cost_metric.py

Two new members added: PerMillionCharactersCostMetric and PerMinuteOfAudioCostMetric.

ModelCostMetric = typing.Union[
    PerThousandTokensCostMetric,
    PerThousandEmbeddingTokensCostMetric,
    PerMillionCharactersCostMetric,   # NEW
    PerMinuteOfAudioCostMetric,       # NEW
    PublicCostMetric,
]

Impact: Additive change (no removal), but any exhaustive isinstance/match/if-elif chains over the union variants will now have unhandled cases.


5. 🟡 InternalArtifactVersion.manifest changed from required → optional

File: src/truefoundry_sdk/types/internal_artifact_version.py (internal type)

-    manifest: ArtifactManifest = pydantic.Field()
+    manifest: typing.Optional[ArtifactManifest] = pydantic.Field(default=None)

Impact: This is an internal type, but any code that assumed .manifest is always non-None will now need a None guard.


✅ Non-breaking Additions (summary)

  • New types added: SmallestAiProviderAccount, SmallestAiModel, SmallestAiApiKeyAuth, SnowflakeConfig, SnowflakeSql, SnowflakeTaskConfig, NativeSnowflakeFlyteTaskTemplate, TruefoundryFlyteTaskTemplate, PerMillionCharactersCostMetric, PerMinuteOfAudioCostMetric, GatewayRequestMetadataFilterOperator, SpanAttributeFilterOperator, SpanFieldFilterOperator, IdentityProviderMapping, McpServerOAuth2.
  • New fields added to: GlobalSettings, GuardrailSettings, Session, Team, TeamManifest, VirtualAccountManifest, RoleWithResource, ModelProviderAccount, AgentSkillVersion, UsageCodeSnippet (new icon field).
  • SubjectType enum: new member(s) added.
  • FlyteTaskCustomTruefoundry union: SnowflakeTaskConfig added.
  • InternalListArtifactVersionsResponseDataItem union: PromptVersion and AgentSkillVersion added (internal type).
  • client.internal.artifact_versions.list(): new optional artifact_types parameter added (backward-compatible, internal group).
  • base_client.py: environments and workspaces properties were reordered in the source — no behavioral change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant