Skip to content

EN-2255: Warn on push when the latest version has insight-settings overrides#250

Merged
roytl merged 6 commits into
masterfrom
push-warn-insight-overrides
Jun 21, 2026
Merged

EN-2255: Warn on push when the latest version has insight-settings overrides#250
roytl merged 6 commits into
masterfrom
push-warn-insight-overrides

Conversation

@roytl

@roytl roytl commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

What

On leap push, if the latest version of the project has any insight-settings overrides (metric direction/active, ignored/added domains, ignored latent spaces), warn the user that those overrides are per-version and won't carry to the new version, and require acknowledgement before proceeding:

  • --yes bypasses the prompt;
  • interactively, a survey.Confirm (defaults to No);
  • non-interactive (no TTY) without --yesabort (never silently override someone's overrides).

How

  • pkg/model/insights.go: GetLatestProjectVersion, GetInsightsSettingsOverrides, HasAnyInsightsOverride (+ unit tests).
  • cmd/root_cmd/push.go: --yes flag + confirmInsightsSettingsOverrides() called early in runPush.
  • pkg/tensorleapapi/insights_settings.go: hand-added GetInsightsSettings client method + types, mirroring the generated style.

⚠️ Dependencies / merge order

  • Requires node-server's getInsightsSettings endpoint (node-server PR #1750) to be released. By design there's no graceful degradation — if the endpoint is missing the call errors and the push aborts — so this should land only after that endpoint is live in deployed servers.
  • The hand-added SDK file is a stopgap; once node-server master has the endpoint, run make update-server-api to replace it with the generated client.

Test plan

  • go build ./..., go vet, go test ./pkg/model/ -run TestHasAnyInsightsOverride pass.
  • Manual: against a server with the endpoint, set overrides on a version, leap push → warning; --yes skips; piped stdin without --yes aborts.

@roytl roytl force-pushed the push-warn-insight-overrides branch from 5716951 to 236f026 Compare June 16, 2026 14:37
@roytl roytl force-pushed the push-warn-insight-overrides branch from 236f026 to 0f73b3c Compare June 16, 2026 14:42
roytl and others added 3 commits June 16, 2026 17:53
The getInsightsSettings endpoint now lands in the generated client
(api_default.go + model_*.go), so the hand-added stopgap file
collided with the generated declarations and broke the build.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The generated InsightsSettingsOverrides uses map[string]interface{} for
its override maps; update the test literals to match.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…Space

From @tensorleap/engine-contract 0.0.417 via node-server. The field
drops out of requiredProperties, so listing versions whose stored
graphValidationData predates the field no longer fails to decode.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@asafyehezkel asafyehezkel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@roytl roytl merged commit bfe4d73 into master Jun 21, 2026
2 checks passed
@roytl roytl deleted the push-warn-insight-overrides branch June 21, 2026 06:57
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.

2 participants