Skip to content

fix(cli): unbreak the interactive pickers and Go timestamp parsing on 3.10 - #824

Open
guill wants to merge 2 commits into
stack/09-pull-roundtrip-guardfrom
stack/10-various-fixes
Open

fix(cli): unbreak the interactive pickers and Go timestamp parsing on 3.10#824
guill wants to merge 2 commits into
stack/09-pull-roundtrip-guardfrom
stack/10-various-fixes

Conversation

@guill

@guill guill commented Aug 28, 2026

Copy link
Copy Markdown
Member
Stack of 7 — review bottom-up (this is the top)
  1. feat(build): add packaging, sync and Builder client primitives #801feat(build): packaging, sync and Builder client primitives — base main
  2. feat(build)!: replace the legacy build and distribution surface #802feat(build)!: replace the legacy build and distribution surface (breaking)
  3. feat(deploy): add the deployment control plane #803feat(deploy): the deployment control plane
  4. feat(deploy): add the asset and job data-plane clients #804feat(deploy): the asset and job data-plane clients
  5. feat(deploy): add comfy deploy run #805feat(deploy): comfy deploy run
  6. fix(build): a pull no longer deletes definition data silently #810fix(build): a pull no longer deletes definition data silently
  7. fix(cli): unbreak the interactive pickers and Go timestamp parsing on 3.10 #824fix(cli): unbreak the interactive pickers and Go timestamp parsing on 3.10   ← this PR

Each PR is based on the branch below it, so its own diff is only its commits. Merge in order.

TL;DR: Two independent defects found while exercising the stack by hand, neither caught by the suite. Every interactive picker in the CLI raised ValueError before it could draw, and roughly one Go-issued timestamp in ten was unparsable on Python 3.10 — the version CI actually runs. Stacked on #810.


1. ce93092 — every interactive picker raised before drawing

prompt_select and prompt_autocomplete defaulted default to "" and passed it straight to questionary. questionary validates that default is one of choices, and "" never is:

ValueError: Invalid `default` value passed. The value (``) does not exist in
the set of choices. Please make sure the default value is one of the available choices.

This fires on construction, before any prompt renders, so no caller with a real choice list worked. That is every picker in the CLI:

Call site Picker
build.py:2029 the Build picker comfy build pull opens when given no --id
deploy_compute.py:26 GPU class
deploy_compute.py:43 Region
uv.py:227 Python version
cmdline.py:1853, :1858 feedback scores

The fix is the correct sentinel: default: ChoiceType | None = None. questionary treats None as "no default" and selects the first choice, which is what the "" was reaching for.

Worth noting for #810's reviewer: the broken picker is the same one the adopt flow depends on — comfy build pull with no --id lists the workspace's builds and asks you to choose one.

2. f40f519 — Go's trimmed fractional seconds are unparsable on Python 3.10

Our Go services marshal time.Time, which encodes as RFC 3339 Nano — and that trims trailing zeros from the fractional part. A timestamp holding 437450 microseconds ships as ...:09.43745Z: five fractional digits.

datetime.fromisoformat on Python 3.10 — requires-python = ">=3.10", and the CI matrix runs 3.10 only — accepts exactly 3 or 6 fractional digits and rejects a bare Z. So that value raises. Trailing-zero trimming makes this roughly a one-in-ten event, and since a createdAt never changes, an affected row was permanently unreadable rather than intermittently so.

New utils.parse_rfc3339 parses any sub-second precision and always returns an aware datetime: it pads so .1 is a tenth rather than a microsecond, truncates so Go's nanosecond precision degrades instead of failing, and reads a missing zone as UTC because callers order these against each other and comparing naive to aware raises TypeError.

Three call sites move onto it, and each had its own failure shape:

  • deploy_resolve.deployment_selection_key — raised deploy_server_error on an affected deployment. The status and timestamp checks are also split apart now: one except (KeyError, ValueError) covered both and reported status=..., so a valid provisioning was printed as the evidence while the timestamp that actually failed went unmentioned. Each is now reported separately, naming the value it rejected.
  • build._release_order — compared createdAt as text, so a whole-second ...:16Z sorted above the strictly later ...:16.5Z (. precedes Z in ASCII), and comfy build release picked the wrong newest release. Now ordered on the parsed instant, with an _UNDATED floor so a badly dated release loses the tiebreak instead of winning it.
  • jobs._parse_epoch — swallowed the failure and returned 0.0, sorting an affected row below every dated one, precisely where jobs ls slices [:limit] and drops a fresh completion.

Validation

  • test_utils.py, test_deploy_resolve.py, test_jobs.py, test_build_release.py: 314 passed.
  • Full command + output suites on this branch: 3749 passed, 3 skipped. ruff check . / ruff format --check . clean at CI's pin (0.15.15).
  • Seven new tests, including property-based coverage that a release-order key stays comparable for every createdAt shape and that a dated terminal row never sinks to the epoch-zero floor.
  • The picker fix is verified by construction against the installed questionary: default="" raises, default=None constructs.

@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Aug 28, 2026
@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

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 Plus

Run ID: 298e0bdd-e797-4580-a5ee-b5bc4f2cd12e

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

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

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

Labels

bug Something isn't working size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant