Skip to content

fix(cli): make --last agree across log, tune and policy-history - #737

Open
dchaudhari7177 wants to merge 2 commits into
DobermanCore:mainfrom
dchaudhari7177:fix/716-last-option-range
Open

dchaudhari7177 wants to merge 2 commits into
DobermanCore:mainfrom
dchaudhari7177:fix/716-last-option-range

Conversation

@dchaudhari7177

Copy link
Copy Markdown

Closes #716 (and its PostHog twin #729).

What changes

  • --last on log, tune and policy-history gets min=0. A negative value is now Typer's usage error (exit 2) instead of an empty listing. 0 stays allowed and still means zero rows (storage: --last 0 returns every row instead of none #430).
  • tune's --last gets the -n short form the other three commands already have.
  • docs/CLI.md:
    • tune's options column now reads --last/-n;
    • the exit-code table gets a log, tune, policy-history | 2 row ("--last is negative; 0 is allowed and means zero rows");
    • log and policy-history leave the "Commands not listed" sentence, since they now have an exit-2 case.

tui is unchanged: it keeps min=1, since an empty TUI isn't useful.

Tests

tests/unit/test_cli_last_option.py:

  • --last -1 and -n -1 exit 2 on each of the three commands (6 cases);
  • --last 0 still exits 0 on each;
  • tune -n 5 runs, and its output is identical to tune --last 5.

On main: 7 failed, 3 passed. On this branch the new file, test_cli_log_jsonl.py (including its #430 --last 0 case), test_storage_last_zero.py and test_cli_help.py all pass. ruff check and format are clean.

AI assistance: I used an AI assistant while writing this change and the tests. I reviewed the diff and ran the tests above myself.

log, tune and policy-history took a negative --last and quietly showed
nothing, and tune had no -n short form. Add min=0 to all three, so a
negative value is a usage error (exit 2) while 0 still means zero rows
(DobermanCore#430), and give tune's --last the -n alias.

Closes DobermanCore#716

This branch has not been deployed

No deployments
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.

cli: --last disagrees across log, tune and policy-history

1 participant