Skip to content

push: cleaner prompts, rename --overwrite-version to -o, refactor RunE#242

Merged
moshe-kabala merged 5 commits into
masterfrom
push-prompts-improvements
May 28, 2026
Merged

push: cleaner prompts, rename --overwrite-version to -o, refactor RunE#242
moshe-kabala merged 5 commits into
masterfrom
push-prompts-improvements

Conversation

@moshe-kabala
Copy link
Copy Markdown
Contributor

What's new in leap push

Interactive flow

  • Ask for the new version's name before the model file path. Matches how users mentally label a push ("what am I calling it → which file is it").
  • Prompt label: "Push as new, or overwrite existing".
  • When the user picks overwrite without --eval, a yellow NOTE points them at the UI's update-evaluate dialog (suppressed when --eval was passed).
  • The auto-detect eval prompt is tighter: in-prompt hint instead of a separate header block, bold auto-detected and bold space in the multi-select instructions, and the instructions line now wraps onto its own line (template tweak — survey.MultiSelectQuestionTemplate overridden once at init). <right> / <left> select-all shortcuts removed from the hint.
  • Plan output uses a single This will: verb across reset / update / no-actions branches; update branch always appends an Auto-detect added meta/viz, metric-direction, insight-config line so users see the full picture.

-o shortens --overwrite-version

leap push -o 6a16a0cf -e            # by id
leap push -o my-model -e            # by name (picker if ambiguous)
  • --overwrite-version stays 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:

    Overwriting model (id 6a16a0cf, matched "my-model")
    

--update accepts short aliases + implies --eval

leap push -o my-model -u viz                       # auto-implied --eval
leap push -o my-model -u metadata -u insights      # repeatable
short (legacy still accepted)
metadata update_metadata
metric update_metric
insights update_insights
visualization / viz update_visualization

The previous "--update requires --eval" error is gone — passing -u now auto-sets --eval to true.

cmd/root_cmd/push.go refactor

The old 250-line RunE closure became a 40-line orchestration. Split into named helpers:

runPush(ctx, in)
├── validatePushInputs
├── newPushState                  (auth + workspace + project + analytics map)
├── resolveOverwriteTarget        (--overwrite flag OR interactive picker)
├── collectModelTypeAndName       (SelectModelType + InitMessage)
├── syncBranchSecretAndPython
├── resolveEvalPlan               → (batchSize, updateActions, runUpdateEvaluate)
├── pushCodeAndWaitForParsing
├── applyModelToVersion           (ImportModel OR OverrideModel)
├── sendSuccessEvent
└── triggerEvaluate               (RunUpdateEvaluateArtifact OR RunEvaluate)

Single s.fail(stage, err) helper tags analytics and emits the fail event — replaces the ~15 inline-duplicated tagging blocks the old RunE had.

Docs

New docs/push-examples.md with a quick tour and a cheat sheet.

Test plan

  • go build ./... clean
  • go vet ./... clean
  • leap push --help renders examples + new -o flag
  • Interactive leap push against a project with sessions + content — confirm name prompt before path
  • Interactive leap push -e against 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 …)" echo
  • leap push -o <duplicate-name> -e — confirm picker opens
  • leap push -o <name> -u viz — confirm runs without explicit -e
  • leap push --overwrite-version <id> -e — confirm deprecation warning

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.
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.
@moshe-kabala moshe-kabala merged commit ca5e225 into master May 28, 2026
2 checks passed
@moshe-kabala moshe-kabala deleted the push-prompts-improvements branch May 28, 2026 12:05
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