Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .Jules/palette.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,7 @@
## 2024-05-24 - CLI Output Scannability and Human-Readability
**Learning:** In text-based CLI menus, using internal identifiers (like `lesson-1`) and verbose status strings (like `pending` or `completed`) makes lists hard to scan and feels too technical.
**Action:** When displaying lists of items with states, use familiar visual symbols (like `[x]` vs `[ ]`) and human-readable titles (like "Variables") to improve scannability and create a more friendly, intuitive interface.

## 2024-05-25 - Actionable Empty States
**Learning:** In CLI applications, empty states (e.g., "No items found") can leave users at a dead end, unsure of what to do next.
**Action:** Always provide actionable instructions or commands in empty state messages to guide the user on how to create the missing resource.
4 changes: 3 additions & 1 deletion src/python_learning_orchestrated/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,9 @@ def main(
if args.checkpoint_command == "list":
checkpoints = checkpoint_store.list_checkpoints()
if not checkpoints:
output_fn("No checkpoints found.")
output_fn(
"No checkpoints found. Create one using 'checkpoint create <name>'."
)
Comment on lines +226 to +228

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The command name create is hardcoded in this message. This can lead to inconsistencies if the command is ever renamed, as the change would need to be made in multiple places (e.g., the argument parser definition). This also leads to duplication of the message string in the test suite. To improve maintainability, consider defining a constant for the command name and/or the full message. This would ensure the implementation, help text, and tests stay in sync.

References
  1. Hardcoded strings for identifiers or messages that are used in multiple places or are likely to change can make code difficult to maintain. They should be defined as constants to improve maintainability and avoid duplication.

return
output_fn(f"Checkpoints ({len(checkpoints)}):")
for checkpoint in checkpoints:
Expand Down
8 changes: 8 additions & 0 deletions tests/test_cli_smoke.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,14 @@ def test_cli_export_and_import_progress_commands(tmp_path, capsys) -> None:
assert len(session_payload["attempts"]) == 1


def test_cli_checkpoint_list_empty(capsys) -> None:
main(["checkpoint", "list"])
output = capsys.readouterr().out
assert (
"No checkpoints found. Create one using 'checkpoint create <name>'." in output
)
Comment on lines +97 to +99

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This assertion duplicates the output string from cli.py. This makes the code more brittle, as any change to the message will require updates in both the implementation and the test. It would be better to define this message as a constant in cli.py, import it here, and use it in the assertion. This follows the Don't Repeat Yourself (DRY) principle and makes your tests more maintainable.

Additionally, using in for assertion is less precise than an equality check. Since the command should only print this one line and exit, a more robust assertion would be:

assert output.strip() == EXPECTED_MESSAGE_CONSTANT
References
  1. Don't Repeat Yourself (DRY): Avoid duplicating information or logic. In this case, the output message string is duplicated between the implementation and the test code, making the code harder to maintain.



def test_cli_checkpoint_create_and_list(tmp_path, capsys, monkeypatch) -> None:
session_file = tmp_path / "session.json"
checkpoint_dir = tmp_path / "checkpoints"
Expand Down
Loading