Skip to content

Align CLI help output#248

Merged
lewtun merged 2 commits intomainfrom
codex/align-cli-help
May 8, 2026
Merged

Align CLI help output#248
lewtun merged 2 commits intomainfrom
codex/align-cli-help

Conversation

@lewtun
Copy link
Copy Markdown
Member

@lewtun lewtun commented May 8, 2026

Summary

  • Replace the hard-coded /help text block with row-based formatting that pads command names and arguments consistently.
  • Keep current commands, including /resume, aligned in one stable description column.
  • Shorten the longest help descriptions so the default terminal width does not wrap the table.

Testing

  • uv run pytest tests/unit/test_cli_rendering.py -q
  • uv run ruff check agent/utils/terminal_display.py tests/unit/test_cli_rendering.py
  • uv run ruff format --check agent/utils/terminal_display.py tests/unit/test_cli_rendering.py

Co-authored-by: OpenAI Codex <codex@openai.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 8, 2026

Claude finished @lewtun's task in 2m 43s —— View job


Claude PR Review

No blocking issues — 2 P2.

This is a small, well-scoped change: replacing the hand-spaced help block with row-based padding driven by HELP_ROWS. The alignment logic at agent/utils/terminal_display.py:475-480 pads off the raw (pre-markup) string lengths, which is correct because Rich consumes markup tags before display, so visible widths match the lengths used for padding. New test in tests/unit/test_cli_rendering.py:33-47 correctly catches alignment regressions by checking len(set(description_columns)) == 1.

I traced:

  • The longest rendered line ("/share-traces [public|private] Show or change HF trace visibility") comes out to ~71 visible columns including the _I + " " prefix, so it fits a default 80-column terminal as the PR description claims.
  • escape() on args correctly handles [index|id|path] / [public|private] / [level] so Rich treats the brackets as literals rather than markup tags — defensive but necessary given args values contain [.
  • The single caller of print_help is the /help slash command at agent/main.py:860; no other consumers of the old HELP_TEXT string formatting.
  • The dropped enumeration of effort levels from the /effort row description is recovered by the no-arg branch of /effort at agent/main.py:931-936, which prints the full level list.

Findings

P2 — Test console doesn't carry _THEME (tests/unit/test_cli_rendering.py:34)
The test instantiates a bare Console(file=output, color_system=None, width=120) without _THEME, so [muted] is an unknown style there. It happens to be harmless because color_system=None produces no ANSI either way, but the test isn't exercising production rendering exactly. Could pass theme=terminal_display._THEME to the test console to keep them in sync (or use get_console() and redirect via console.file). Informational; the alignment property under test isn't sensitive to theme.

P2 — Width constants computed at import time (agent/utils/terminal_display.py:471-472)
_HELP_COMMAND_WIDTH and _HELP_ARGS_WIDTH are module-level, so adding a new row that's longer than the current max requires touching only HELP_ROWS — good. Just worth noting that HELP_TEXT itself is also frozen at import time (line 483), so any future runtime mutation of HELP_ROWS wouldn't take effect. Not a bug today; flagging only because a future contributor adding a register_help_row() helper would have to remember to rebuild HELP_TEXT.

Nice-to-haves (not flagged)

  • description_columns collection in the test could also assert each match is unique to avoid the (currently impossible) substring-collision case if a description ever contains a command-like prefix. Out of scope for this PR.

  • Read PR diff against origin/main
  • Inspect agent/utils/terminal_display.py
  • Inspect new tests in tests/unit/test_cli_rendering.py
  • Check for prior reviews / convergence rules
  • Post final review
    · Branch

Co-authored-by: OpenAI Codex <codex@openai.com>
@lewtun
Copy link
Copy Markdown
Member Author

lewtun commented May 8, 2026

Addressed both review notes in 353cfca:

  • The help rendering test now uses terminal_display._THEME, matching production console styling.
  • Help text is now formatted at print time via format_help_text(), so widths are recomputed from the current rows instead of freezing HELP_TEXT at import time.
  • Added a regression test with a synthetic longer row to prove future wider rows still align.

Checks:

  • uv run pytest tests/unit/test_cli_rendering.py -q
  • uv run ruff check agent/utils/terminal_display.py tests/unit/test_cli_rendering.py
  • uv run ruff format --check agent/utils/terminal_display.py tests/unit/test_cli_rendering.py

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 8, 2026

Claude finished @lewtun's task in 1m 36s —— View job


Claude PR Review (re-review of 353cfca)

LGTM.

Both prior P2s are addressed cleanly:

  • Test console theme (tests/unit/test_cli_rendering.py:38) — now passes theme=terminal_display._THEME, so [muted] resolves under the same theme as production. With color_system=None, no ANSI is emitted, so the line.index(description) alignment check still works on raw widths.
  • Runtime help formatting (agent/utils/terminal_display.py:495-510) — format_help_text() computes column widths from HELP_ROWS (or a passed-in tuple) at call time, replacing the import-time HELP_TEXT constant. print_help() at line 515 calls it on each invocation.
  • New regression test (tests/unit/test_cli_rendering.py:56-74) — appends a synthetic ("/longer-command", "[longer-args]", ...) row and confirms description columns still line up, which is exactly the property a future contributor adding a wider row would need to preserve.

Verified during review:

  • Only consumer of the old HELP_TEXT was print_help(); no other call sites left dangling (only unrelated session_uploader.py:656 is argparse's print_help, different namespace).
  • escape() on args continues to handle the [index|id|path] / [public|private] literals correctly.
  • Padding math (args_width - len(args) + 2) still produces a constant visible column for the description regardless of whether args is empty (args_markup collapses to "", but args_gap keeps the column) — same property the test asserts.

No P0/P1 findings.
· Branch

@lewtun lewtun merged commit fde34dc into main May 8, 2026
3 checks passed
@lewtun lewtun deleted the codex/align-cli-help branch May 8, 2026 12:18
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.

1 participant