Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the application's argument parsing capabilities by allowing choices for arguments and flags to be dynamically sourced from environment variables. This provides greater flexibility in configuring the application's behavior based on the environment in which it is run.

Highlights

• Environment Variable Choices: This PR introduces the ability to source argument and flag choices from environment variables using the choices env="..." syntax.
• Consistent Resolution: The changes ensure that environment-backed choices are resolved consistently across parsing, help/documentation generation, shell completion, and Fig suggestions.
• KDL Round-Tripping: The PR includes updates to preserve the env property when serializing choices back to KDL, ensuring proper round-tripping.

Greptile Summary

This PR adds choices env=... support to the usage KDL spec format, allowing allowed values for args and flags to be sourced from an environment variable at parse and completion time rather than hard-coded in the spec. It is guarded by the unstable_choices_env feature flag.

Key changes:

• SpecChoices gains an optional env field; values_with_env(custom_env) resolves literal + env-backed choices, correctly using the caller-supplied map exclusively when Parser::with_env(...) is active and falling back to std::env::var only when no custom map is provided.
• Choice validation is refactored into choice_error / validate_choices / validate_choice_value helpers and threaded through all parse paths (explicit CLI values, env-var defaults, scalar and variadic defaults).
• Shell completion uses choices.values() (process env) which is correct for the interactive shell context; the new has_explicit_choices flag prevents file-path fallback when an env-backed choices constraint is configured but resolves to empty at completion time.
• The SpecArgBuilder::choices() builder method now preserves a previously set choices_env(), avoiding silent loss of the env key when both are combined.
• Help output and Markdown docs render the env var name directly instead of snapshotting live values, so documentation stays meaningful when the env var is absent at generation time.
• Fig static output continues to use only literal choices, avoiding baking runtime env values into the generated file.
• Tests cover splitting/deduplication, parser validation with a custom env map, empty-env error messages, builder ordering, KDL round-tripping, and shell completions.

Confidence Score: 4/5

• PR is on the happy path to merge; previous thread concerns are addressed and no new blocking issues found.
• All three concerns raised in prior review rounds appear to be resolved: values_with_env now correctly isolates the custom env map from the process environment; the Tera templates render the env key rather than snapshotting live values; and the new unstable_choices_env tests avoid process-env mutation entirely. No new P0/P1 issues were identified. Score is 4 rather than 5 only because the feature is brand-new and gated behind unstable_choices_env, warranting a brief stabilisation period before broader adoption.
• lib/src/parse.rs — three pre-existing tests (test_parser_with_custom_env_for_required_arg, test_parser_with_custom_env_for_required_flag, test_parser_with_custom_env_still_fails_when_missing) still call std::env::remove_var outside unsafe; this was flagged in a prior review thread and does not affect production correctness.

Important Files Changed

Sequence Diagram

sequenceDiagram
    participant CLI as CLI user
    participant PW as parse_partial_with_env
    participant VC as validate_choices
    participant CE as choice_error
    participant SC as SpecChoices::values_with_env
    participant ENV as Environment

    CLI->>PW: parse(["--env", "prod"], custom_env)
    PW->>VC: validate_choices(target, "prod", choices, custom_env)
    VC->>SC: values_with_env(custom_env)
    alt custom_env provided (Parser::with_env)
        SC->>SC: lookup key in custom_env map only
    else no custom_env
        SC->>ENV: std::env::var(env_key)
        ENV-->>SC: env value (or None)
    end
    SC-->>VC: ["dev", "staging"] (split on comma/whitespace, deduped)
    VC->>VC: is_help_arg? and value not in list?
    VC-->>PW: Ok(true) — push help error & return early
    VC->>CE: choice_error(target, "prod", choices, custom_env)
    alt values is empty (env unset)
        CE-->>VC: "no choices resolved from env DEPLOY_ENVS"
    else value not in list
        CE-->>VC: "expected one of dev, staging"
    else value found
        CE-->>VC: None (valid)
    end
    VC-->>PW: bail!(err) or Ok(false)
    PW-->>CLI: ParseOutput or Error

_{Reviews (6): Last reviewed commit: "parse: fast-path help choice checks befo..." | Re-trigger Greptile}

Addressed review feedback in 5519652.

Changes made:

• fixed SpecChoices::values_with_env(...) to avoid process-env fallback when Parser::with_env(...) provides a custom env map
• reverted Fig suggestions to literal choices only (no env snapshot at generation time)
• docs/help now preserve and render choices env: ... instead of resolving away env-backed source
• removed unnecessary std::env::remove_var(...) calls from parser tests
• added a guard test for no-fallback behavior with custom env maps

I could not run cargo test in this container because cargo is not available on PATH here.

Addressed the two additional review comments in c49eff4.

Updates:

• validate_choices(...) now rejects explicit values when choices env=... resolves to an empty set, with a targeted error message
• SpecArgBuilder::choices() now preserves an existing choices_env(...) configuration instead of replacing SpecChoices
• added focused tests for both cases (parse empty-env behavior and builder ordering preservation)

I also updated the PR description to the v3 message.

Addressed the latest completion feedback in de8e0d1.

Changes:

• completion now treats explicit arg.choices as authoritative and skips filesystem fallback even when resolved values are empty (e.g. choices env=... unset)
• added regression test: complete_word_choices_from_env_unset_returns_empty

Verification run:

• mise x -- cargo test -p usage-cli complete_word_choices_from_env (passes, including the new test).

Addressed the latest two review comments in 82adee1.

Fixes:

• revalidated env/default-populated arg/flag values against choices during Parser::parse (not only argv-consumed values)
• kept root path fallback behavior when default-subcommand arg has choices by narrowing when explicit choices suppress fallback

Regression tests added:

• test_parser_rejects_arg_env_value_not_in_choices_env
• test_parser_rejects_flag_default_value_not_in_choices_env
• complete_word_default_subcommand_choices_do_not_block_root_file_fallback

Targeted verification:

• mise x -- cargo test -p usage-lib test_parser_rejects_arg_env_value_not_in_choices_env
• mise x -- cargo test -p usage-lib test_parser_rejects_flag_default_value_not_in_choices_env
• mise x -- cargo test -p usage-cli complete_word_default_subcommand_choices_do_not_block_root_file_fallback

i think you need to rebase

Codecov Report

❌ Patch coverage is 67.06349% with 166 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.94%. Comparing base (1e81f1b) to head (bffa882).
⚠️ Report is 2 commits behind head on main.

@@            Coverage Diff             @@
##             main     #548      +/-   ##
==========================================
- Coverage   70.04%   61.94%   -8.10%     
==========================================
  Files          48       48              
  Lines        7080     7629     +549     
  Branches     7080     7629     +549     
==========================================
- Hits         4959     4726     -233     
- Misses       1322     1542     +220     
- Partials      799     1361     +562     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Addressed the latest review comment in 90f47e9.

What changed:

• for scalar defaults (var=false), choice validation now checks only the emitted default value (default[0]), not every configured default entry
• this applies to both scalar args and scalar nested flag-arg defaults
• kept full-list validation for variadic defaults (var=true)

Regression coverage added:

• test_arg_var_false_validates_only_first_default_choice
• test_flag_arg_default_var_false_validates_only_first_default_choice

Targeted verification:

• mise x -- cargo test -p usage-lib test_arg_var_false_validates_only_first_default_choice
• mise x -- cargo test -p usage-lib test_flag_arg_default_var_false_validates_only_first_default_choice
• mise x -- cargo test -p usage-lib test_parser_rejects_flag_default_value_not_in_choices_env

Addressed the latest scalar-default review point and the current CI lint failure in 26ab9f3.

Changes:

• scalar-default choice validation now checks only the emitted value (default[0]) for var=false
• variadic defaults (var=true) still validate all default values
• fixed clippy-denied patterns in parser/choices code (unnecessary_unwrap, field_reassign_with_default, and argument-count shape via ChoiceTarget)

Local verification:

• mise x -- cargo fmt --all
• mise x -- cargo clippy --all --all-features -- -D warnings
• focused tests for scalar-default and env/default choice behavior still pass

is it ready?

is it ready?

I hope so! Full disclaimer: heavily AI-assisted.