fix(learner): make ace-learn logs informative and user-friendly#111
fix(learner): make ace-learn logs informative and user-friendly#111withsivram wants to merge 1 commit into
Conversation
Users running `ace-learn` had no visibility into what was happening — the progress output was opaque and silent. This commit adds clear, step-by-step progress messages at every key stage of the learn command: - "Scanning for latest Claude Code session..." on startup - Session ID, transcript path, and project displayed before analysis - "Analyzing session..." with session ID and line count indented below - "Running Reflector (analyzing session patterns)..." during Reflector - "Running SkillManager (extracting strategies)..." during SkillManager - Per-session strategy summary: "N added, M updated, K removed" - Final summary: total strategies in skillbook and net change this session - --verbose flag now shows task context, feedback, and per-strategy details Also adjusts logging level to WARNING when not verbose (was INFO) since user-facing messages are now delivered via print() directly, keeping the logger for debug/internal tracing only. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR improves the ace-learn CLI user experience by adding explicit, step-by-step progress output (primarily via print()), and adjusts default logging verbosity to reduce noise when not running in verbose mode.
Changes:
- Add user-facing progress messages during transcript discovery and session analysis.
- Add
verbosemode details (task context, feedback, and per-operation strategy details). - Change default logging level to
WARNINGwhen not verbose.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| print(" Running Reflector (analyzing session patterns)...") | ||
| logger.info("Running Reflector...") | ||
| reflection = self._run_reflector_with_retry( |
There was a problem hiding this comment.
When --verbose is enabled, this prints a user-facing progress line and also logs Running Reflector... at INFO, which results in duplicate progress output (stdout + stderr). Prefer making the logger call DEBUG, or suppressing one of the two in verbose mode.
| print(" Running SkillManager (extracting strategies)...") | ||
| logger.info("Running SkillManager...") | ||
| skill_manager_output = self._run_skill_manager_with_retry( |
There was a problem hiding this comment.
Same duplication issue as Reflector: in verbose mode you'll get both the print(...) progress message and logger.info("Running SkillManager..."). Consider using DEBUG for the logger message or gating one of them.
| if verbose: | ||
| print(f" Task context: {task[:100]}{'...' if len(task) > 100 else ''}") | ||
| print(f" Feedback: {feedback}") | ||
|
|
There was a problem hiding this comment.
New verbose behavior introduces additional stdout output (task context + feedback) but there are no existing unit tests covering verbose=True/False output differences. Consider adding tests (e.g., using pytest capsys) to assert the presence/absence of these lines so future refactors don’t regress the CLI UX.
| print("Analyzing session...") | ||
| success = learner.learn_from_transcript( |
There was a problem hiding this comment.
PR description says the "Analyzing session..." step should include the session ID and line count, but this message is currently generic. Either include {session_id} / {lines_desc} here, or update the PR description to match the actual output.
| Args: | ||
| transcript_path: Path to Claude Code transcript JSONL | ||
| start_line: Start learning from this line (for incremental learning) | ||
| verbose: Print detailed progress to stdout | ||
|
|
There was a problem hiding this comment.
The verbose parameter docstring says it controls printing detailed progress, but this method now prints multiple progress lines even when verbose=False (session id, Reflector/SkillManager, strategy summary). Update the docstring to reflect that verbose only enables additional details, or gate the non-verbose prints behind verbose as well to match the documentation.
| logger.info( | ||
| f"Skipping trivial session ({actual_lines} lines, minimum {MIN_LINES})" | ||
| ) |
There was a problem hiding this comment.
In verbose mode (logging.basicConfig(... level=DEBUG)), this will emit logger.info(...) and also print(...), causing duplicated user-facing messages. Consider either removing the logger.info here, downgrading it to logger.debug, or wrapping it in if not verbose to avoid duplicate output.
| logger.info( | |
| f"Skipping trivial session ({actual_lines} lines, minimum {MIN_LINES})" | |
| ) | |
| if not verbose: | |
| logger.info( | |
| f"Skipping trivial session ({actual_lines} lines, minimum {MIN_LINES})" | |
| ) |
| # Count skills before update so we can report the delta | ||
| skills_before = len(self.skillbook.skills()) | ||
|
|
There was a problem hiding this comment.
skills_before is assigned but never used, which looks like leftover code and can trigger linting failures. Either remove it, or use it to report a per-transcript net change after _persist_skillbook_update(...) succeeds.
| # Count skills before update so we can report the delta | |
| skills_before = len(self.skillbook.skills()) |
Summary
Users running
ace-learnhad no visibility into what was happening — the progress output was opaque and silent. This adds clear, step-by-step progress messages at every key stage:--verboseflag now shows task context, feedback, and per-strategy detailsAlso adjusts logging level to WARNING when not verbose (was INFO) since user-facing messages are now delivered via
print()directly.Test plan
ace-learn→ see step-by-step progress messagesace-learn --verbose→ see additional detail per strategyFixes #74
🤖 Generated with Claude Code