Make alphapy CLI exit 0 on success

[mrconway]

Summary

Every successful alphapy run was exiting with code 1, masking real failures in CI and local pipeline checks. Two unrelated cosmetic bugs combined to produce this:

1. main() in alphapy_main.py ended with return model. The setuptools entry point wraps it as sys.exit(main()); passing a non-int non-None object to sys.exit stringifies to stderr and exits 1. No code calls main() programmatically (siblings use main_pipeline directly), so the return is vestigial. Dropped.

2. plots.py had a top-level print(__doc__) that printed the literal string "None" on every import (the file's "docstring" is #-comments, not a string, so __doc__ is None). Dead noise since 2013. Removed.

Test plan

• pytest tests/ — 23 passed
• cd projects/kaggle && uv run alphapy — exit 0, clean output
• cd projects/pizza && uv run alphapy — exit 0, clean output
• cd projects/time-series && uv run alphapy — exit 0, clean output

[claude]

Claude finished @mrconway's task —— View job

---

Code Review Analysis

I've thoroughly analyzed this PR that fixes the CLI exit code issue. Here's my comprehensive feedback:

✅ Excellent Fix - Clean and Well-Executed

Problem Analysis

The PR correctly identifies and fixes two distinct but related issues that combined to cause successful alphapy runs to exit with code 1:

1. CLI Entry Point Issue (alphapy_main.py:710-712)

   • Root Cause: The main() function ended with return model
   • Why This Failed: Line 92 in pyproject.toml shows alphapy = "alphapy.alphapy_main:main" - the CLI entry point wraps this as sys.exit(main())
   • Technical Details: sys.exit(model_object) stringifies the model object to stderr and exits with code 1, regardless of success

2. Import-Time Output Pollution (plots.py:51-52)

   • Root Cause: Top-level print(__doc__) statement
   • Why This Occurred: The file uses # comments instead of string docstrings, so __doc__ is None
   • Impact: Printed literal string "None" on every import since 2013

Quality Assessment

Code Quality: A+

• ✅ Minimal, surgical changes - only removes problematic code without side effects
• ✅ Correct fix strategy - implicit None return triggers sys.exit(None) → exit code 0
• ✅ No functional impact - author correctly notes no code calls main() programmatically
• ✅ Clean diff - 5 deletions, 0 additions, no unnecessary changes

Best Practices: A

• ✅ Proper entry point design - CLI functions should return None or int for exit codes
• ✅ Good commit message - clear explanation of both issues and verification steps
• ✅ Vestigial code cleanup - removes dead return model from 2013

Test Coverage: A-

• ✅ Manual testing comprehensive - verified all 3 projects (kaggle, pizza, time-series)
• ✅ Pytest suite passes - 23 tests passed
• ⚠️ Missing automated CLI exit code test - consider adding integration test for subprocess.call(['alphapy']) exit codes

Technical Review

Security: ✅ No Issues

• Changes only remove code, no new attack vectors
• No handling of user input or external resources

Performance: ✅ Improvement

• Eliminates unnecessary string serialization of model object on exit
• Removes unwanted I/O operation (print) on every module import

Compatibility: ✅ No Breaking Changes

• CLI behavior improves (exit 0 vs exit 1)
• No API changes affecting programmatic consumers
• No dependency changes required

Edge Cases & Validation

Error Handling: ✅ Proper

• Existing exception handling in main() (lines 647-649) still works correctly
• sys.exit(error_string) will still exit 1 for actual failures
• Only successful completions now exit 0

Import Behavior: ✅ Clean

• plots.py import no longer pollutes stdout
• Module can be imported cleanly in Python REPL/notebooks

Minor Suggestions

1. Future Enhancement: Consider adding a simple integration test:

   def test_cli_exit_codes():
       # Test successful run exits 0
       result = subprocess.run(['alphapy', '--help'], capture_output=True)
       assert result.returncode == 0

2. Documentation: The fix is self-documenting, but a note in CHANGELOG about exit code fix would help users

Approval Recommendation

✅ Approve and Merge

This is an exemplary bug fix:

• Correctly diagnosed complex interaction between two separate issues
• Minimal, safe changes that improve CI/CD pipeline reliability
• Thoroughly tested with comprehensive manual verification
• Well-documented in commit message and PR description
• Zero risk of regression or breaking changes

The fix will immediately improve developer experience by making CI/CD pipelines work correctly and eliminating confusing "None" output.

---