Skip to content

fix(learner): make ace-learn logs informative and user-friendly #111

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
withsivram wants to merge 1 commit into
base: main
Choose a base branch
from
+79 −8
Open

fix(learner): make ace-learn logs informative and user-friendly #111

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
87 changes: 79 additions & 8 deletions ace/integrations/claude_code/learner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -771,13 +771,15 @@ def learn_from_transcript(
self,
transcript_path: Path,
start_line: int = 0,
verbose: bool = False,
) -> bool:
"""
Learn from a transcript file.

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

Comment on lines 779 to 783

Copilot AI Mar 20, 2026

Copy link

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Returns:
True if learning succeeded
Expand All @@ -792,8 +794,14 @@ def learn_from_transcript(
logger.info(
f"Skipping trivial session ({actual_lines} lines, minimum {MIN_LINES})"
)
Comment on lines 794 to 796

Copilot AI Mar 20, 2026

Copy link

Choose a reason for hiding this comment

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

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.

Suggested change
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})"
)

Copilot uses AI. Check for mistakes.
print(
f" Skipping: session too short ({actual_lines} lines, minimum {MIN_LINES})"
)
return True

session_id = _extract_session_id(transcript_path)
print(f" Session: {session_id} ({actual_lines} lines)")

# Get TOON-compressed transcript
toon_trace = toon_transcript(transcript_path, start_line)

Expand All @@ -802,6 +810,10 @@ def learn_from_transcript(
feedback = _get_transcript_feedback(transcript_path, start_line)
cwd = _extract_cwd_from_transcript(transcript_path) or self.cwd

if verbose:
print(f" Task context: {task[:100]}{'...' if len(task) > 100 else ''}")
print(f" Feedback: {feedback}")

Comment on lines +813 to +816

Copilot AI Mar 20, 2026

Copy link

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
# Create AgentOutput for Reflector
agent_output = AgentOutput(
reasoning=toon_trace,
Expand All @@ -810,7 +822,11 @@ def learn_from_transcript(
raw={"total_lines": total_lines, "start_line": start_line},
)

# Count skills before update so we can report the delta
skills_before = len(self.skillbook.skills())

Comment on lines +825 to +827

Copilot AI Mar 20, 2026

Copy link

Choose a reason for hiding this comment

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

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.

Suggested change
# Count skills before update so we can report the delta
skills_before = len(self.skillbook.skills())

Copilot uses AI. Check for mistakes.
# Run Reflector
print(" Running Reflector (analyzing session patterns)...")
logger.info("Running Reflector...")
reflection = self._run_reflector_with_retry(
Comment on lines +829 to 831

Copilot AI Mar 20, 2026

Copy link

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
task=task,
Expand All @@ -819,16 +835,48 @@ def learn_from_transcript(
)

# Run SkillManager
print(" Running SkillManager (extracting strategies)...")
logger.info("Running SkillManager...")
skill_manager_output = self._run_skill_manager_with_retry(
Comment on lines +838 to 840

Copilot AI Mar 20, 2026

Copy link

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
reflection=reflection,
cwd=cwd,
progress=f"lines {start_line + 1}-{total_lines}",
)

# Report what the SkillManager decided to do
ops = skill_manager_output.update.operations
added = [op for op in ops if op.type == "ADD"]
updated = [op for op in ops if op.type == "UPDATE"]
removed = [op for op in ops if op.type == "REMOVE"]
tagged = [op for op in ops if op.type == "TAG"]

op_parts = []
if added:
op_parts.append(f"{len(added)} added")
if updated:
op_parts.append(f"{len(updated)} updated")
if removed:
op_parts.append(f"{len(removed)} removed")
if tagged:
op_parts.append(f"{len(tagged)} tagged")

if op_parts:
print(f" Strategies: {', '.join(op_parts)}")
if verbose:
for op in added:
snippet = (op.content or "")[:80]
print(f" + [{op.section}] {snippet}")
for op in updated:
print(f" ~ [{op.section}] id={op.skill_id}")
for op in removed:
print(f" - id={op.skill_id}")
else:
print(" Strategies: no changes")

# Persist update
if self._persist_skillbook_update(skill_manager_output.update):
logger.info(f"Skillbook now has {len(self.skillbook.skills())} skills")
skills_after = len(self.skillbook.skills())
logger.info(f"Skillbook now has {skills_after} skills")

return True

Expand Down Expand Up @@ -875,6 +923,8 @@ def cmd_learn(args):

Use --lines N to optionally limit to the last N lines (for very large sessions).
"""
verbose = getattr(args, "verbose", False)

# Get project context for filtering (if specified)
filter_project = None
if hasattr(args, "project") and args.project:
Expand All @@ -883,6 +933,7 @@ def cmd_learn(args):
filter_project = Path(env_dir).resolve()

# Find latest transcript (uses history.jsonl for correct /resume handling)
print("Scanning for latest Claude Code session...")
transcript_path = find_latest_transcript(filter_project)
if not transcript_path:
print("No transcript found.")
Expand All @@ -895,6 +946,7 @@ def cmd_learn(args):

# Extract session info
total_lines = _count_transcript_lines(transcript_path)
session_id = _extract_session_id(transcript_path)
cwd = _extract_cwd_from_transcript(transcript_path) or str(Path.cwd())

# Determine project root
Expand All @@ -913,28 +965,47 @@ def cmd_learn(args):
lines_limit = getattr(args, "lines", None)
if lines_limit and lines_limit < total_lines:
start_line = total_lines - lines_limit
print(f"Learning from transcript (last {lines_limit} lines): {transcript_path}")
lines_desc = f"last {lines_limit} lines"
else:
start_line = 0
print(f"Learning from transcript ({total_lines} lines): {transcript_path}")
lines_desc = f"{total_lines} lines"

print(f"Project: {project_root}")
print(f"Found session: {session_id} ({lines_desc})")
print(f"Transcript: {transcript_path}")
print(f"Project: {project_root}")
print()

# Setup logging
logging.basicConfig(
level=logging.DEBUG if getattr(args, "verbose", False) else logging.INFO,
level=logging.DEBUG if verbose else logging.WARNING,
format="%(asctime)s [%(levelname)s] %(message)s",
)

# Run learning
try:
learner = ACELearner(cwd=cwd, project_root=project_root)
success = learner.learn_from_transcript(transcript_path, start_line=start_line)

# Snapshot skillbook size before learning so we can report the delta
skills_before = len(learner.skillbook.skills())

print("Analyzing session...")
success = learner.learn_from_transcript(
Comment on lines +991 to +992

Copilot AI Mar 20, 2026

Copy link

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
transcript_path,
start_line=start_line,
verbose=verbose,
)

if success:
print("\n✓ Learning complete!")
print(f"Updated: {project_root / 'CLAUDE.md'}")
skills_after = len(learner.skillbook.skills())
new_skills = skills_after - skills_before

print()
print("✓ Learning complete!")
print(f" Updated: {project_root / 'CLAUDE.md'}")
print(
f" Skillbook: {skills_after} strategies total"
+ (f" ({new_skills:+d} this session)" if new_skills != 0 else " (no net change)")
)
else:
print("\n✗ Learning failed")
print(
Expand Down
Loading