feat(messaging): centralize banner formatting with opt-in timestamps

[mattnico]

Summary

This PR centralizes how Code Puppy renders banner tags (AGENT RESPONSE, EDIT FILE, SHELL COMMAND, THINKING, etc.) into a single helper, and uses that consolidation to introduce three opt-in formatting options. All three options default to historical behavior, so this is a no-op for users who don't enable them.

Motivation

Banner formatting was duplicated across three code paths:

1. RichConsoleRenderer._format_banner — message-bus rendering
2. event_stream_handler._print_thinking_banner / _print_response_banner — live-stream rendering
3. autosave_menu.display_resumed_history — session-resume rendering (which constructed an AGENT RESPONSE banner inline, bypassing _format_banner entirely)

That meant any new banner behavior had to be implemented in three places, and the resume-history path silently diverged from the other two. While experimenting with a banner-formatting plugin I kept hitting that fan-out — every monkey-patch had to know about all three sites. This PR fixes the underlying shape: one helper, one place to change.

What's in the PR

New module: code_puppy/messaging/banner.py

Single source of truth for banner markup:

def format_banner(banner_name: str, text: str, *, when: datetime | None = None) -> str

Builds the same Rich markup string the codebase has always produced, but allows opt-in additions controlled by config:

• A dim [HH:MM:SS] timestamp annotation on the same line, just outside the colored banner block
• A trailing newline so the banner sits alone on its line (matches what AGENT RESPONSE already did)

Three new config keys (all default-off)

Key	Type	Default	Effect when enabled
banner_timestamps_enabled	bool	false	Append [HH:MM:SS] after every banner tag
banner_timestamp_format	str	%H:%M:%S	strftime format for the annotation
banner_newline_after_tag	bool	false	Append \n after every banner tag

set_banner_timestamp_format() validates inputs by checking that the format produces different output for two distinct datetimes — Python's strftime is permissive and silently passes unknown directives through as literals (e.g. %Q-bogus becomes Q-bogus), so this catches typos that would otherwise produce a banner displaying garbage forever.

Refactored callers (now ~3-line delegations)

• RichConsoleRenderer._format_banner → calls format_banner(banner_name, text)
• event_stream_handler._print_thinking_banner / _print_response_banner → call format_banner("thinking", "THINKING") / format_banner("agent_response", "AGENT RESPONSE")
• autosave_menu.display_resumed_history → calls format_banner("agent_response", "AGENT RESPONSE", when=msg.timestamp)

Bonus fix: resumed sessions show original timestamps

pydantic-ai already stores msg.timestamp (UTC) on every message. When banner_timestamps_enabled=true, resumed-session banners now use that stored timestamp (converted to local time) instead of datetime.now(). So reloaded banners show when the message actually happened, not when you reloaded the session.

This required no new metadata, no sidecar storage, no content hashing — the data was already there.

Defaults & backward compatibility

Every new option defaults to behavior that matches main exactly:

>>> format_banner("agent_response", "AGENT RESPONSE")
'[bold white on medium_purple4] AGENT RESPONSE [/bold white on medium_purple4]'

Identical byte-for-byte to the previous markup. Existing users see zero change unless they explicitly opt in via /set banner_timestamps_enabled=true (or the other keys).

Tests

• New: tests/messaging/test_banner.py — 16 tests covering the strftime validator, timestamp-suffix helper (disabled / live / naive datetime / aware datetime / fallback), and format_banner (default behavior, timestamp-only, newline-only, both combined).
• Updated: tests/test_rich_renderer.py, tests/agents/test_event_stream_handler.py, tests/test_config.py — re-pointed mocks to code_puppy.config.get_banner_color (since _format_banner now delegates rather than calling its own _get_banner_color hook), and added the three new keys to the expected sorted-key list.

Full suite results on this branch: 9399 passed, 88 skipped, 1 xpassed, 4 failed. The 4 failures (test_run_pending_credentials_success, test_run_prompt_with_attachments_uses_spinner, TestDefaultAgent::test_default, test_opus_46_reverse_name_also_works) all reproduce on a clean checkout of origin/main and are unrelated to this change.

Out of scope

• Adding EDIT FILE / SHELL COMMAND banners to display_resumed_history (it currently shows tool calls/returns as collapsed dim text). Easy follow-up but it's a separable UX decision.
• A slash-command UI for toggling these options. Today they go through the existing /set command, which already discovers them via get_config_keys().

Try it

/set banner_timestamps_enabled=true
/set banner_timestamp_format=%H:%M:%S
/set banner_newline_after_tag=true

Then trigger any tool / response, or /resume a previous session, to see the new formatting.

[mpfaffenberger]

Would love to see some screenshots of the new setup

[mattnico]

Would love to see some screenshots of the new setup

[mpfaffenberger]

🐶 Code Puppy Review — PR #329

Solid PR — the centralization of banner formatting is a genuine DRY win and the backward compat story is airtight. A few things to address:

1. THINKING banner + banner_newline_after_tag bug — the trailing newline splits the banner from its ⚡ (see inline)
2. Dead code — _get_banner_color on RichConsoleRenderer is now orphaned. No production code calls it anymore. It's still monkey-patched by tests at tests/test_rich_renderer.py lines 91, 124, 154, 185, 317, 441 and tests/messaging/test_rich_renderer.py line 913. Should either remove it and update those tests to patch code_puppy.config.get_banner_color (like you did for test_format_banner_and_level_prefix), or document why it's being kept.
3. _coerce_bool should replace the 15+ copy-pasted bool coercions in config.py (see inline)
4. Minor nits: Optional vs X | None, missing integration test for THINKING+newline

Not requesting changes since the newline bug only fires when someone opts in. 🐾

[mpfaffenberger]

⚠️ Bug: trailing newline splits the ⚡ from the banner

When banner_newline_after_tag=True, format_banner() appends \n to the markup string. Then this line constructs:

Text.from_markup(f"{banner_markup} [dim]⚡ ")

The \n sits between the banner tag and the lightning bolt, so the ⚡ ends up on the next line, separated from the banner. That's almost certainly not what the user wants — the newline is meant to push the content below to a new line, not to split the banner from its own inline decoration.

Suggested fix: Either:

1. Strip the trailing \n here since you're already doing end="" (the THINKING banner doesn't need the newline because it appends streaming content on the same line), or
2. Add a newline: bool = True param to format_banner so callers can opt out when they'll add more inline content after the banner.

[mpfaffenberger]

Nice helper! But this is the 16th place in this file that does the str(val).lower() in ("1", "true", "yes", "on") dance. The other 15+ copy-pastes are at lines 75, 106, 121, 160, 253, 1110, 1142, 1160, 1247, 1384, 1806, 1831, 1942 (and more).

Would be a great follow-up PR to refactor those to use _coerce_bool. Not blocking for this PR, but since you've already written the canonical version, it'd be a shame not to DRY up the rest of the file.

[mpfaffenberger]

💡 This two-reference-datetime approach to validate strftime formats is genuinely clever. Python's strftime silently passing unknown directives through as literals is a real footgun, and this catches it without needing to parse the format string yourself. The %% test case in the parametrized tests is a nice touch too.

One minor note: _VALIDATION_DT_A and _VALIDATION_DT_B are module-level constants but their names don't make the purpose obvious to someone who stumbles on them. A brief comment like # Used by is_valid_strftime_format to detect non-functional directives would help. Not blocking.

[mpfaffenberger]

Nit: Since you already have from __future__ import annotations, you could use datetime | None instead of Optional[datetime] throughout this file. It's the modern Python 3.10+ style and avoids the typing import entirely. Not blocking — just a consistency nit.

[mpfaffenberger]

This local import to break the circular dependency is the right pattern. Just noting: if this module grows, consider whether a thin config_protocol module (that just defines the getter interfaces) might be cleaner. But for 148 lines, local imports are the pragmatic choice. 👍

[mpfaffenberger]

The %% escaping for configparser's BasicInterpolation is correct here, and get_banner_timestamp_format reads through get_value() which uses ConfigParser that undoes the escaping on read. Round-trip is ✅.

Just a heads-up: if anyone ever changes get_value() to use RawConfigParser (no interpolation), the %% would come back as %% instead of %, and the format string would break. Worth a brief comment in get_banner_timestamp_format noting that it relies on BasicInterpolation to un-escape the %%.

[mpfaffenberger]

Using getattr(msg, "timestamp", None) to fish the stored timestamp out of pydantic-ai messages is a nice touch — and the fact that it gives reloaded sessions original timestamps instead of datetime.now() is a genuine UX improvement that requires zero new storage. Well done.

One thought: if msg.timestamp is ever None (e.g., an older session saved before timestamps were added), the formatter falls back to datetime.now(), which is fine for live rendering but slightly misleading for resumed history (it'd show the reload time, not the original time). Maybe a dim annotation like [original time unknown] would be more honest? Low priority.

[mpfaffenberger]

Solid test coverage for the new module — 16 tests covering the validator, suffix helper (disabled/enabled/naive/aware/fallback), and full format_banner (default, timestamp-only, newline-only, both combined).

Gap: No test for the THINKING banner + banner_newline_after_tag=True interaction (the event_stream_handler path where the trailing newline would split the banner from the ⚡). This is where the real bug lives, and since the unit test for format_banner alone can't catch it, an integration-style test on the event stream handler would be valuable. See my inline comment on event_stream_handler.py:145.

[mpfaffenberger]

The format_banner signature is clean and the docstring is excellent. One thing that would make it even more future-proof: consider adding a **kwargs that gets passed through, so plugin authors can extend banner rendering without having to monkey-patch this function. But that's YAGNI until someone actually needs it — just planting the seed. 🌱