Add personality tone control plugin

[nhicks00]

Summary

• Adds a built-in personality_tone plugin that controls Code Puppy's response style without editing the core prompt directly.
• Adds /tone for quick switching between professional, neutral, default, and playful tones.
• Persists the selected tone in config via personality_tone and reloads the active agent so the updated prompt applies immediately.

Why

Code Puppy's default prompt is intentionally playful and sassy, which is useful for some users but too informal for others. This keeps the existing default behavior intact while giving users a lightweight way to opt into a dry, business-professional style when they want focused engineering work.

Usage

• /tone or /tone show: display the current tone and available options.
• /tone professional: dry, direct, business-professional responses.
• /tone neutral: friendly but restrained responses.
• /tone default: native Code Puppy personality with no override.
• /tone playful: explicitly playful while staying task-focused.
• Numeric aliases are supported: /tone 0, /tone 1, /tone 2, /tone 3.

Implementation Notes

• Implemented as a plugin under code_puppy/plugins/personality_tone/.
• Uses the existing load_prompt hook to append a tone override, so core prompt text is not rewritten.
• Uses the existing custom_command and custom_command_help hooks for /tone and help-menu integration.
• Defaults to default, so existing users see no behavior change unless they opt in.

Validation

• uv run pytest --no-cov tests/plugins/test_personality_tone.py tests/plugins/test_plugins_init_coverage.py -q
• uv run ruff check code_puppy/plugins/personality_tone tests/plugins/test_personality_tone.py

Note: I also attempted the broader command-handler coverage suite, but this local environment fails during import before reaching this feature because pydantic_ai.mcp cannot import the optional mcp.client.sse path.

[mpfaffenberger]

quality checks plz - I added a note in AGENTS.md

[mpfaffenberger]

PR 313 Deep Review: personality_tone Plugin

Thanks @nhicks00 for this plugin! It's a well-thought-out feature. Here's my deep dive:

---

✅ What's Great

1. Clean Plugin Architecture - Uses the existing hooks (load_prompt, custom_command, custom_command_help) correctly. No core changes needed. 🎯

2. Comprehensive Alias System - The aliases dict is thorough with numeric shortcuts (0, 1, 2, 3) making it easy to switch.

3. Good Test Coverage - Tests cover normalization, command handling, config persistence, and error cases.

4. Graceful Failure - _reload_current_agent wraps in try/except with a warning, so tone still saves even if reload fails.

5. Idempotent by Default - default tone returns empty string, preserving existing behavior for all users.

---

⚠️ Issues to Address

1. Missing Test for _show_tone() function - The test file doesn't directly test _show_tone(). It's tested indirectly via _handle_custom_command("/tone", "tone"), but a direct unit test would be clearer.

2. No Test for Playful Tone Profile - test_professional_tone_adds_override exists, but test_playful_tone_adds_override is missing. Consider adding one for completeness.

3. Potential Edge Case in _handle_custom_command - The check len(parts) > 2 accepts /tone a b c silently. Consider emitting a warning for extra arguments.

4. Module-level register_callback at Import Time - While this is the standard pattern in code_puppy, it means callbacks are registered when the module is imported. This is fine for the plugin system but worth documenting.

---

🔧 Nitpicks (Minor)

1. Unused Import - Callable from typing is imported but not directly used (only in type hints via string annotation). Could remove it.

2. Docstring Clarity - The get_tone_prompt_addition docstring mentions "Returning an empty string keeps the default prompt behavior unchanged" but on_load_prompt in callbacks.py returns a list, not a string. The plugin returns a string that gets added to the list. Consider clarifying.

3. TONE_PROFILES Order - The dict maintains insertion order (Python 3.7+), but _available_tones_text hardcodes the tuple. If a new tone is added, both places need updating. Consider iterating TONE_PROFILES.keys() instead.

---

🚀 Suggestions for Future Enhancement

1. Config-driven Tones - Allow users to define custom tones via config file.
2. Per-Conversation Tones - Support session-local tone that resets on new session.
3. Tone Preview - Show a sample response demonstrating each tone.

---

Verdict

Approve with minor suggestions. The implementation is solid, follows project conventions, and adds real value. The issues are minor and don't block merge.

---

Reviewed with 🐶 by Code Puppy

[mpfaffenberger]

PR 313 Deep Review: personality_tone Plugin 🔍

Hey @nhicks00 — Biscuit here, doing a deep sniff-through of this PR. I've also re-examined the existing review by @mpfaffenberger and found several points that need correction or deeper analysis.

---

✅ What's Genuinely Good

1. Plugin architecture is textbook — Uses load_prompt, custom_command, and custom_command_help hooks exactly as intended. Zero core changes. 🐾

2. Alias system is thoughtful — Numeric shortcuts (0-3) are a nice UX touch, and the alias map covers most intuitive synonyms without being bloated.

3. normalize_tone is clean and defensive — Handles None, empty string, whitespace, and unknown values gracefully. Always returns a valid tone name.

4. _reload_current_agent fails gracefully — The try/except with warning is correct: config still persists even if the agent can't reload. Users aren't left in a broken state.

5. ToneProfile is a frozen dataclass — Immutable, clean, minimal. Exactly right.

6. Default tone is zero-impact — Returns "", which means existing users who never touch /tone see zero behavior change. The PR description explicitly calls this out, and it's correct.

---

🔴 Issues That Should Be Fixed Before Merge

1. load_prompt returns "" for default tone — this is a systemic prompt pollution problem

This is the most important issue in this PR.

The on_load_prompt() hook collects ALL callback return values into a list via _trigger_callbacks_sync, and the callers join them:

# agent_code_puppy.py, agent_planning.py
prompt_additions = callbacks.on_load_prompt()
if len(prompt_additions):
    result += "\n".join(prompt_additions)

When the default tone is active, get_tone_prompt_addition() returns "". This gets appended to the results list as-is. If file_permission_handler is also active (it always is unless yolo mode), the list becomes:

["<file permission prompt>", ""]

"\n".join(["<file permission prompt>", ""]) produces "<file permission prompt>\n" — a trailing newline that adds whitespace to the system prompt.

Why this matters: LLMs are sensitive to prompt structure. A trailing newline might seem harmless, but when you have 2+ load_prompt plugins returning empty strings, you get multiple blank lines injected into the system prompt. This is a ticking time bomb.

Fix: Return None instead of "" for the default tone, and update get_tone_prompt_addition to return None when no override is needed. Then fix _trigger_callbacks_sync (or the callers) to filter out None/falsy values before joining. OR — simpler — just filter at the consumer level:

prompt_additions = [a for a in callbacks.on_load_prompt() if a]

⚠️ Note: The existing review says "Returning an empty string keeps the default prompt behavior unchanged while still satisfying the hook" — this is technically true but practically wrong. The empty string does get joined and does affect the final prompt text. The docstring should be corrected.

2. Leading newlines in tone prompts get doubled

All non-empty tone prompts start with \n (from the triple-quoted string), and the callers join with \n. So the effective prompt becomes:

...existing system prompt...
\n  ← from "\n".join()
\n  ← from the leading \n in the tone prompt
## Tone Override

That's a double-blank-line gap before the tone section. This isn't catastrophic, but it's sloppy prompt engineering. Strip the leading newline from each prompt, or strip it when joining.

3. normalize_tone silently swallows typos

If a user types /tone profesisonal (typo), normalize_tone falls back to "default" with no warning. The user has no idea their command was ignored. The _set_tone function only emits an error when the unknown value resolves to DEFAULT_TONE — but that check uses a clever heuristic that fails in edge cases (e.g., if someone adds an alias that points to "default", it would be treated as valid even if the raw input was gibberish that happened to map to default via an alias).

Fix: normalize_tone should return a tuple[str, bool] or raise for unknown values, rather than silently falling back. Let the caller decide whether to warn.

---

🟡 Issues Worth Addressing (Non-blocking)

4. _available_tones_text hardcodes the tone order

for tone_name in ("professional", "neutral", "default", "playful"):

This duplicates the key order of TONE_PROFILES. If someone adds a 5th tone, they must update both TONE_PROFILES AND this tuple. The existing review flagged this correctly — it's a DRY violation.

Fix: Just iterate TONE_PROFILES:

for tone_name, profile in TONE_PROFILES.items():

5. Callable import is dead code

from typing import Callable

With from __future__ import annotations, all annotations are stringified and never evaluated at runtime. Callable is used only in the type hint Callable[[], None] for _set_tone's default parameter. It's never referenced at runtime. The existing review flagged this correctly but called it a "nitpick" — I'd bump it to "should fix" since dead imports are lint noise.

Fix: Remove from typing import Callable.

6. No test for the playful tone prompt

The test file has test_professional_tone_adds_override but no equivalent for playful or neutral. The existing review flagged this. Even a simple assertion that the playful prompt contains "playful personality" would catch regressions.

7. No test for _reload_current_agent failure path

The _reload_current_agent function has a try/except that emits a warning. This failure path is untested. A test that patches get_current_agent() to raise and then asserts emit_warning was called would close the gap.

8. No test for the len(parts) > 2 error path

The _handle_custom_command emits an error for /tone foo bar, but no test exercises this. Simple addition.

---

🔧 Nitpicks

9. _set_tone unknown-tone detection heuristic is fragile

if tone == DEFAULT_TONE and raw_tone.strip().lower() not in {
    DEFAULT_TONE,
    *ALIASES,
}:

This checks: "if the resolved tone is default AND the raw input wasn't a known alias for default, then it's unknown." This works today, but it's clever-clever. If someone adds an alias "off" → "default", then /tone off would pass. But if someone adds an alias "unknown_alias" → "professional" by mistake, /tone unknown_alias would NOT be caught as unknown — it would silently set professional. The heuristic conflates "is this a valid input?" with "did this resolve to default?".

Fix: Use a set of all valid inputs instead:

VALID_INPUTS = set(TONE_PROFILES) | set(ALIASES)
if raw_tone.strip().lower() not in VALID_INPUTS:
    emit_error(...)
    return

10. The playful tone prompt doesn't say "This section supersedes earlier..."

The professional and neutral prompts both start with:

"This section supersedes earlier playful or sassy personality guidance."

But the playful prompt doesn't include this line. This means the core prompt's playful personality instructions AND the playful override both apply, potentially conflicting. The playful override says "keep it useful" but doesn't explicitly negate any of the core sassy instructions.

Fix: Add the same supersedes line to the playful prompt, or document that playful is intentionally additive rather than overriding.

11. get_current_tone calls get_value every time

Every call to get_tone_prompt_addition (which runs on every prompt assembly) reads from config. This is fine for a file-based config, but if config ever becomes remote, this becomes a latency issue. Not worth fixing now, just noting it.

---

🚫 Corrections to the Existing Review

The existing review by @mpfaffenberger made a few claims that need correction:

1. "Docstring clarity — on_load_prompt in callbacks.py returns a list, not a string" — The review implied the plugin's return type is wrong. It's not. Each callback returns a string, and _trigger_callbacks_sync collects them into a list. The plugin is correct to return a string. However, the review's underlying concern (that empty strings pollute the list) is valid — just the diagnosis was wrong.

2. "Potential Edge Case in _handle_custom_command — accepts /tone a b c silently" — This is actually handled! Look at line 178-179: if len(parts) > 2: emit_error(...). The existing review missed this and suggested a fix that's already implemented.

3. "Missing Test for _show_tone()" — The review says there's no test for _show_tone(). While there's no direct unit test, test_handle_tone_show_emits_current_tone_and_options exercises it thoroughly via _handle_custom_command("/tone", "tone"). I'd still prefer a direct test, but the claim that it's "not tested" is overstated.

---

🚀 Future Enhancement Ideas (Not for this PR)

1. Config-driven custom tones — Let users define their own tone profiles in ~/.code_puppy/config.toml
2. Per-session tones — A --tone CLI flag or session-local override that resets on new conversation
3. Tone preview — /tone preview professional shows a sample response in that tone
4. Hook into agent_reload — Instead of manually calling reload_code_generation_agent(), register an agent_reload callback so the tone plugin participates in the standard reload flow

---

Verdict

Request changes on items #1, #2, and #3. The plugin is architecturally sound and follows project conventions perfectly. But the empty-string-in-load_prompt issue (#1) is a real prompt hygiene problem that will compound as more load_prompt plugins are added. The leading newlines (#2) and silent typo swallowing (#3) are also worth fixing before this ships.

Items #4-#8 are strongly recommended but non-blocking. Items #9-#11 are nits for the author's consideration.

---

Reviewed with 🐶 by Biscuit (code-puppy-819fbe)

[mpfaffenberger]

Inline Review Comments

Detailed line-by-line observations from my deep review. See the main review for the full analysis.

Reviewed with 🐶 by Biscuit (code-puppy-819fbe)

[mpfaffenberger]

🔴 Empty string pollutes the prompt additions list

Returning "" here means _trigger_callbacks_sync appends an empty string to the results list. When another load_prompt plugin (like file_permission_handler) is also active, the join produces a trailing newline: "\n".join(["<perm prompt>", ""]) → "<perm prompt>\n".

Consider returning None and filtering falsy values at the consumer level, or at minimum strip the empty string before joining. See my review comment #1 for the full analysis.

[mpfaffenberger]

🔴 Leading newline gets doubled

This triple-quoted string starts with \n, and the caller joins prompt additions with "\n".join(...). The effective output becomes:

...existing prompt...\n\n## Tone Override

Two blank lines before the tone section. Strip the leading newline from the string, or .lstrip("\n") when returning.

[mpfaffenberger]

🟡 Dead import

With from __future__ import annotations at line 1, Callable is never evaluated at runtime. It's only used in the type hint Callable[[], None] for _set_tone, which is stringified by __future__ annotations. Safe to remove.

[mpfaffenberger]

🔴 Silent typo swallowing

If a user types /tone profesisonal (typo), normalize_tone returns "default" silently. The check on line 158 only catches this because the result equals DEFAULT_TONE, but the heuristic is fragile. If someone adds an alias that maps to "default", any unknown input that happens to resolve to default through that alias would be accepted without warning.

Prefer a VALID_INPUTS set: VALID_INPUTS = set(TONE_PROFILES) | set(ALIASES) and check membership directly.

[mpfaffenberger]

🟡 DRY violation — This hardcoded tuple duplicates the key order of TONE_PROFILES. If a 5th tone is added, both places need updating.

Prefer: for tone_name, profile in TONE_PROFILES.items():

[mpfaffenberger]

The playful tone prompt omits the "This section supersedes earlier playful or sassy personality guidance" line that both professional and neutral include. This means the core prompt's sassy instructions and the playful override both apply without a clear precedence. Is this intentional? If so, worth a comment. If not, add the supersedes line.

[mpfaffenberger]

🟡 Missing test coverage — There's a test for the professional tone prompt but none for playful or neutral. Adding:

def test_playful_tone_adds_override():
    with patch(
        "code_puppy.plugins.personality_tone.register_callbacks.get_value",
        return_value="playful",
    ):
        prompt = plugin.get_tone_prompt_addition()
    assert "playful personality" in prompt

would catch regressions if the playful prompt is edited.

[mpfaffenberger]

🟡 Missing test: _reload_current_agent failure path

The function has a try/except that emits emit_warning on failure, but no test exercises this. Also missing: a test for the len(parts) > 2 error in _handle_custom_command (e.g., /tone foo bar).