push: cleaner prompts, rename --overwrite-version to -o, refactor RunE#242
Merged
Conversation
Interactive flow
- Ask for the new version's name BEFORE the model file path.
- Prompt label changed to "Push as new, or overwrite existing".
- When the user picks overwrite without --eval, surface a one-line
NOTE pointing them at the UI's update-evaluate dialog (not shown
when --eval was passed).
- Auto-detect eval prompt: shorter labels (drop the inline hints,
the auto-detect note moves into the question itself), bold
"auto-detected" + bold "space" in the multi-select hint, hint
line broken onto its own line via a one-shot override of
survey.MultiSelectQuestionTemplate. <right>/<left> shortcuts
hidden via WithRemoveSelectAll/None.
- Plan output: single "This will:" verb across reset / update /
no-actions branches; update branch always appends the auto-detect
line.
--overwrite (-o) replaces --overwrite-version
- New flag --overwrite (-o) accepts an id OR a name.
- Old --overwrite-version stays as a deprecated alias (cobra prints a
one-line warning on use).
- ResolveVersionInfoFromRef:
* id lookup → searches all versions
* name lookup → only matches active versions
* ambiguous active name → interactive picker (same row format
as the no-flag flow); non-TTY falls back to a candidate-ids
error so scripts get usable output.
- Resolved target echoed ("Overwriting <name> (id <prefix>, matched
<ref>)") before any destructive work runs.
--update / -u stays as-is; short aliases now accepted
- metadata / metric / insights / visualization (viz) accepted in
addition to the legacy update_* tokens.
- --update now implies --eval (the old "requires --eval" error is
gone).
push.go refactor
- RunE split into named helpers: validatePushInputs, newPushState,
resolveOverwriteTarget, collectModelTypeAndName,
syncBranchSecretAndPython, resolveEvalPlan,
pushCodeAndWaitForParsing, applyModelToVersion, sendSuccessEvent,
triggerEvaluate.
- pushInputs struct holds all flag values; pushState threads ctx /
workspace / project / overwrite-target / analytics through the
pipeline.
- Single fail(stage, err) helper tags analytics and sends the fail
event; removes the per-callsite duplication that was scattered
across ~15 spots in the old RunE.
Refreshed help examples + a new docs/push-examples.md tour.
…ive) The interactive eval-plan prompt's planUpdateEvaluate routes ChangeMetadataUpdate / ChangeMetricsAddOrUpdate to EvaluatePlanReset because both invalidate derived artifacts (pickles, NN indexer, display_pe DBs) in ways update_evaluate can't patch. The CLI flag path was bypassing this — passing -u metadata or -u metric triggered the update_evaluate API call with that action, printing 'This will: Update metadata' but actually issuing a wrong patch instead of the full re-eval the user expected. New PlanFromUpdateActions encodes the same rule (METADATA or METRIC present → reset, else update with the parsed actions); resolveEvalPlan calls it before deciding between update_evaluate and the reset path.
- Flag type: int → string. Accepts a positive integer OR the literal 'latest' (case-insensitive), so users can lock in 'use the latest project batch size' without copy-pasting the value. - Short alias -b. - Omitting the flag keeps today's behaviour: prompt with the latest as the default. - Explicit 'latest' with no prior evaluation in the project errors out so the user picks a number rather than silently falling back to DEFAULT_BATCH_SIZE.
snir-shem-tov
approved these changes
May 28, 2026
The UI / interactive prompt already checks whether the override chain has an evaluated ancestor (HasEvaluatedAncestorOrSelf) and falls back to a fresh full re-eval when it doesn't — otherwise update_evaluate has nothing to patch. The CLI flag path (-u <action>) was bypassing this check: passing -u viz on an unevaluated chain would call UpdateEvaluateArtifact and the engine couldn't actually do the work. Lift the check into resolveOverwriteEvalPlan so both branches share it: when the parsed --update plan is the Update kind but the chain isn't evaluated, demote to Reset and log the same one-liner the interactive path uses.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's new in
leap pushInteractive flow
"Push as new, or overwrite existing".--eval, a yellow NOTE points them at the UI's update-evaluate dialog (suppressed when--evalwas passed).auto-detectedand boldspacein the multi-select instructions, and the instructions line now wraps onto its own line (template tweak —survey.MultiSelectQuestionTemplateoverridden once at init).<right>/<left>select-all shortcuts removed from the hint.This will:verb across reset / update / no-actions branches; update branch always appends anAuto-detect added meta/viz, metric-direction, insight-configline so users see the full picture.-oshortens--overwrite-version--overwrite-versionstays as a deprecated alias — existing scripts keep working but trigger cobra's one-line deprecation warning.ID lookups search across all versions (active or not); name lookups only consider active versions so the disambiguation picker matches what users see in the UI.
The picker reuses the row format from the no-flag interactive flow.
Non-TTY (CI) gracefully falls back to a candidate-ids error so scripts can re-run with an exact id.
The resolved target is echoed before any destructive work:
--updateaccepts short aliases + implies--evalmetadataupdate_metadatametricupdate_metricinsightsupdate_insightsvisualization/vizupdate_visualizationThe previous "--update requires --eval" error is gone — passing
-unow auto-sets--evaltotrue.cmd/root_cmd/push.gorefactorThe old 250-line
RunEclosure became a 40-line orchestration. Split into named helpers:Single
s.fail(stage, err)helper tags analytics and emits the fail event — replaces the ~15 inline-duplicated tagging blocks the oldRunEhad.Docs
New
docs/push-examples.mdwith a quick tour and a cheat sheet.Test plan
go build ./...cleango vet ./...cleanleap push --helprenders examples + new-oflagleap pushagainst a project with sessions + content — confirm name prompt before pathleap push -eagainst a project with an overwrite-able version — confirm new "Push as new, or overwrite existing" prompt + the multi-select with bold "auto-detected" and bold "space"leap push -o <id>— confirm "Overwriting … (id …)" echoleap push -o <duplicate-name> -e— confirm picker opensleap push -o <name> -u viz— confirm runs without explicit-eleap push --overwrite-version <id> -e— confirm deprecation warning