Skip to content

test: combine #2855 cortex review bot with #2948 snowpark deploy --prune fixe#3106

Draft
sfc-gh-kolszewski wants to merge 7 commits into
mainfrom
ko/test-cortex-bot
Draft

test: combine #2855 cortex review bot with #2948 snowpark deploy --prune fixe#3106
sfc-gh-kolszewski wants to merge 7 commits into
mainfrom
ko/test-cortex-bot

Conversation

@sfc-gh-kolszewski

Copy link
Copy Markdown
Contributor

Pre-review checklist

  • If my changes add or modify user-facing interface (commands, flags, output formats), I consulted maintainers and got sign-off beforehand (how).
  • I've confirmed that instructions included in README.md are still correct after my changes in the codebase.
  • I've added or updated unit tests to verify correctness of my new code.
  • I've added or updated integration tests to verify correctness of my new code.
  • Manually tested on macOS
  • Manually tested on Windows
  • I've confirmed my changes are up-to-date with the target branch.
  • I've described my changes in RELEASE-NOTES.md (see when and how).
  • I've updated documentation if behavior changed.

Changes description

...

sfc-gh-olorek and others added 6 commits June 10, 2026 11:37
Adds a GitHub Actions workflow and orchestration script that uses the Cortex Code CLI as an autonomous agent to perform E2E verification of CLI behavioral changes on pull requests. The agent gets a dedicated Snowflake playground database and verifies happy paths, edge cases, breaking changes, and side effects.

The workflow is opt-in per PR: it only runs when the "enable-ai-e2e-review" label is present. Adding the label triggers a run; pushes, reopens, and ready-for-review re-run only while the label remains.
Gate the cortex-review job on the PR author's association so only org
owners/members and repo collaborators can trigger the autonomous E2E
review for now, in addition to the existing same-repo and opt-in label
checks.
author_association only vouches for the PR author. A triage-access
contributor can apply the enable-ai-e2e-review label to a trusted
member's PR and trigger the privileged, secret-bearing review job.

Add an authorize gate job that checks the triggering actor's repo
permission via the collaborator-permission API (the label applier on
labeled events). Triage reports as read, so requiring write/admin
excludes it. The review job needs: this gate, so it never starts for
an unauthorized trigger; set -e makes any API error fail closed.
`snow snowpark deploy --prune` now also drops procedures and functions
that exist in the target schemas but are no longer declared in
`snowflake.yml`. Previously `--prune` only cleared stage contents, so
removed entities lingered in Snowflake until dropped manually.

Only schemas that contain at least one declared entity of the same type
are scanned, and builtin functions are skipped. Signature-based matching
is used so overloads are preserved correctly.

Closes #2218
…e notes

The `--prune` help text changed but the help-message snapshot was not
regenerated, failing test_help_messages[snowpark.deploy]. The rebase also
re-introduced two release-note entries that already live under v3.17.0, and
left the prune entry under a released version; move it to Unreleased.
@sfc-gh-kolszewski sfc-gh-kolszewski added the enable-ai-e2e-review Opt in to autonomous Cortex AI E2E review on this PR label Jun 12, 2026
@sfc-gh-kolszewski sfc-gh-kolszewski marked this pull request as ready for review June 12, 2026 14:50
@sfc-gh-kolszewski sfc-gh-kolszewski requested review from a team as code owners June 12, 2026 14:50
@github-actions

Copy link
Copy Markdown

Cortex AI E2E Review

Model: claude-opus-4-6 | Commit: 519342f | Duration: 4m 13s | Reviewed at: 2026-06-14T17:08:45Z

Summary

PR #3106 extends the snow snowpark deploy --prune flag to additionally drop procedures and functions that exist in Snowflake but are no longer declared in the project's snowflake.yml. Previously --prune only cleared stage contents. The new behavior was E2E-verified against a live Snowflake environment: stale objects are correctly dropped, declared objects are preserved, overloaded signatures are handled correctly, and the default --no-prune behavior remains unchanged.

E2E Test Results

Command Exit Code Result
snow snowpark deploy --help 0 Updated --prune description shown correctly
snow snowpark deploy --prune --replace (with stale objects) 1* Prune phase succeeded: STALE_PROC and STALE_FN dropped. *Exit 1 due to pre-existing 'packages' KeyError in procedure creation (unrelated to this PR)
snow snowpark deploy --no-prune --replace (with stale objects) 1* No prune phase executed, stale objects preserved. *Same pre-existing error
snow snowpark deploy --replace (default, no prune flag) 1* No prune phase executed (default is --no-prune). *Same pre-existing error
snow snowpark deploy --prune --replace (overloaded function) 1* Correctly dropped KEEP_FN(NUMBER, NUMBER) overload not declared in project, preserved KEEP_FN(VARCHAR)

*Note: Exit code 1 in all deploy cases is from a pre-existing KeyError: 'packages' in common.py:175 when handling SQL-language procedures (not Python/Snowpark). This is unrelated to the prune feature.

Breaking Changes

Behavioral change to --prune flag (opt-in, non-default):

The --prune flag now has expanded semantics. Previously it only cleared stage contents before uploading artifacts. Now it additionally drops procedures/functions from Snowflake schemas that are not declared in the project definition.

This is a behavioral extension of an existing flag, but:

  • The default remains --no-prune (no change to default behavior)
  • Users must explicitly pass --prune to trigger the new behavior
  • Only schemas that contain at least one declared entity of the same type are affected
  • Built-in objects are excluded

Risk assessment: Users who currently use --prune expecting it to ONLY clear stage contents will now also get their undeclared procedures/functions dropped. This is a behavior change for existing --prune users that could be surprising, but it's documented in the help text update and release notes.

Side-Effect Verification

Query Before --prune After --prune
SHOW USER PROCEDURES IN SCHEMA ...PUBLIC KEEP_PROC, STALE_PROC (2 rows) KEEP_PROC only (1 row)
SHOW USER FUNCTIONS IN SCHEMA ...PUBLIC KEEP_FN, STALE_FN (2 rows) KEEP_FN only (1 row)
Overload test: KEEP_FN(VARCHAR) + KEEP_FN(NUMBER,NUMBER) Both exist Only KEEP_FN(VARCHAR) retained

All DROP operations used IF EXISTS (safe against race conditions).

Potential Risks

  1. Overload safety in production: The prune logic matches by name AND argument types using same_type(). If same_type() has edge cases with complex types (e.g., ARRAY, OBJECT, VARIANT with qualifiers), it could incorrectly drop a valid overload. The existing same_type tests cover common mappings but complex nested types are not exhaustively tested.

  2. FQN resolution fallback: When an entity doesn't declare database/schema explicitly, the code falls back to entity.udf_sproc_identifier.identifier_with_arg_types via FQN.from_string(). If this resolution fails or returns unexpected values, it could target the wrong schema.

  3. Concurrent deployment: If multiple users deploy to the same schema simultaneously with --prune, one user's valid procedures could be dropped by another user's prune operation if they have different project definitions.

  4. The 'packages' KeyError at common.py:175 is a pre-existing bug that surfaces when deploying to schemas with SQL-language procedures, but it's unrelated to this PR.

Verdict

PASS — The new --prune behavior correctly drops undeclared snowpark entities, preserves declared ones (including overload-safe matching), and the default --no-prune behavior is unchanged. The feature is opt-in and well-scoped.

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

Labels

enable-ai-e2e-review Opt in to autonomous Cortex AI E2E review on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants