fix: use clean user message for auto-title instead of skill-bloated input - #4946
fix: use clean user message for auto-title instead of skill-bloated input#4946dieutx wants to merge 1 commit into
Conversation
fbc971c to
3bb6a4d
Compare
|
Rebased this onto current main.\n\nLocal: |
3bb6a4d to
227c68b
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for targeting a real title-quality problem: current main still passes the expanded message at cli.py:12558 and gateway/run.py:19030.
Problems
- The proposed
original_user_messageis not intrinsically clean. It ispersist_user_messageonly when provided, otherwise the model-facinguser_message(agent/turn_context.py:304; PR baserun_agent.py:7925). Normal CLI skill invocation queues the expanded payload (cli.py:9037-9044) and only supplies that override for voice mode (cli.py:12319-12346), so the PR would still title from skill scaffolding. - The added tests only construct local dicts and repeat the fallback expression; they never exercise either title caller with a skill-expanded message.
- The proposed result-dict hunk has moved: current result assembly is
agent/turn_finalizer.py:399-428, following refactor053025238434cfbf121873977b39888d7f27d1c1.
Suggested changes
- Use the existing
extract_user_instruction_from_skill_message()(agent/skill_commands.py:58-112) at the title boundary and add CLI/gateway regression tests that assertmaybe_auto_titlereceives the extracted instruction. Decide explicitly how a bare skill invocation should title, since the extractor returnsNonefor that case.
Automated hermes-sweeper review.
| "partial": False, # True only when stopped due to invalid tool calls | ||
| "interrupted": interrupted, | ||
| "response_previewed": getattr(self, "_response_was_previewed", False), | ||
| "original_user_message": original_user_message, |
There was a problem hiding this comment.
original_user_message is not guaranteed to be clean skill input: the turn setup defines it as persist_user_message only when supplied, otherwise the model-facing user_message (current agent/turn_context.py:304; PR base run_agent.py:7925). Normal CLI and gateway skill paths pass the expanded payload without that override, so this field would still contain the skill body. Use the canonical skill-instruction extractor before title generation instead.
Summary
Sessions started via skill invocation get meaningless auto-generated titles like "Skill Invocation and Content Loading" instead of the user's actual intent. Follow-up to #4940 which fixed the same issue for memory providers.
Root Cause
When a skill is active, the `message` variable contains the full skill activation payload: a `[SYSTEM: The user has invoked...]` prefix, the entire SKILL.md content (often 1K+ chars), and only then the user's actual instruction. Both `cli.py:6186` and `gateway/run.py:6031` pass this bloated `message` to `maybe_auto_title()`. The title generator truncates to 500 chars (`title_generator.py:29`), so it only sees skill boilerplate.
`run_conversation()` already computes `original_user_message` (line 6516) — the clean user input without skill injection. PR #4940 used it for memory providers with this exact comment:
But `original_user_message` was never added to the result dict, so callers couldn't access it.
Fix
Tests
4 tests: result dict includes original message, clean message preferred over bloated, fallback to raw message when missing, clean input fits within 500-char truncation.
```
4 passed
```