Filter hook-injected synthetic user turns at HIGH detail and below by cboos · Pull Request #167 · daaain/claude-code-log

cboos · 2026-05-23T15:57:29Z

Summary

External tooling — clmail, clmail-monitor, and similar — uses Claude
Code's UserPromptSubmit hook to inject single-line notifications like
[monitor] alice idle or [clmail] You've got a new mail (#3017) into
the conversation. These arrive in the JSONL as full type: user entries
with no structural marker, so the renderer was treating them as ordinary
human prompts: one full ## 🤷 User heading per notification. In one
real cmail session at --detail low, that pollution amounted to 294
synthetic user-turn headings — drowning out the actual conversation.

This PR teaches the renderer to recognise these hook-injected turns,
render them compactly at DetailLevel.FULL, and drop them entirely at
HIGH and below. The behaviour matches the pre-existing "no system /
hook noise" semantics already applied to HookSummaryMessage /
HookAttachmentMessage at HIGH — same _HIGH_EXCLUDE_CLASSES filter,
same gating.

Detection

detect_hook_notification() in factories/user_factory.py:

Regex ^\s*\[(monitor|clmail)\]\s*(.*?)\s*\Z (sources list is a
module-level tuple — easy to extend).
Single-line only. A real user prompt that starts with
[monitor] foo\n\nActually, please continue... is preserved intact;
the multi-line guard rejects it from hook treatment so it renders as
a normal user turn.
Non-hook bracketed markers ([Request interrupted by user],
[Image #N], [@filename], [from alice]) are unaffected.

Rendering

A new UserHookNotificationMessage content type wraps detected turns:

FULL detail: compact inline marker. Markdown emits a single
italic line *[monitor] alice idle*; HTML emits a pill with
class="message user hook-notification", dimmed border, monospace
body. Full fidelity is preserved at FULL.
HIGH and below: filtered out by _HIGH_EXCLUDE_CLASSES (the
same mechanism already used for HookSummaryMessage and
HookAttachmentMessage).

Latent markdown-renderer fix

MarkdownRenderer._render_message previously only emitted content when
a title was present, silently dropping the body of any standalone
message with an empty title. The new headless-content branch (elif content: parts.append(content)) only catches genuine standalone
content carriers — paired partners short-circuit earlier via
is_middle_in_pair / is_last_in_pair.

Test plan

uv run pytest test/test_hook_user_notifications.py -v — 14 new
tests covering detector, factory wrapping, and FULL/HIGH/LOW
filtering for both Markdown and HTML.
just ci green (1997 tests / 0 ruff / 0 pyright / ty clean).
HTML snapshot tests updated (CSS additions for the new
.hook-notification pill) — reviewed before accepting.
Manual sanity check: re-render the cmail session that motivated
this change and confirm the 294 synthetic headings are gone at
LOW while real user turns survive.

External tooling (clmail, clmail-monitor, ...) uses Claude Code's UserPromptSubmit hook to inject single-line notifications such as `[monitor] alice idle` or `[clmail] You've got a new mail (#3017)`. These arrive in the JSONL as full `type: user` entries with no structural marker — they polluted rendered transcripts at every detail level (e.g. 294 such lines in one cmail session at LOW). Detection: bracket-prefix regex (`^\s*\[(monitor|clmail)\]\s*...\Z`) restricted to single-line payloads. A real user prompt that *starts* with `[monitor] foo` but continues onto another line is preserved intact. Rendering: - New `UserHookNotificationMessage` content type wraps detected hook turns. - At `DetailLevel.FULL`: compact inline marker (markdown italic, HTML pill with monospace body) — full fidelity preserved. - At `DetailLevel.HIGH` and below: dropped via the existing `_HIGH_EXCLUDE_CLASSES` post-render filter, matching the pre-existing "no system/hook noise" semantics already applied to `HookSummaryMessage` / `HookAttachmentMessage`. Also fixes a latent bug in `MarkdownRenderer._render_message` where standalone messages with an empty title silently dropped their body. Paired partners short-circuit earlier, so the new `elif content:` branch only catches genuine headless content carriers.

coderabbitai · 2026-05-23T15:57:38Z

Caution

Review failed

An error occurred during the review process. Please try again later.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch dev/filter-hook-turns

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

cboos · 2026-05-23T16:11:51Z

Build failures due to:

Update - We are seeing an increased rate of authentication failures for app installation tokens, affecting approximately 1% of tokens. We are continuing to investigate.
May 23, 2026 - 16:01 UTC

cboos · 2026-05-23T16:15:19Z

This PR teaches the renderer to recognise these hook-injected turns,
render them compactly at DetailLevel.FULL, and drop them entirely at
HIGH and below

While the example of what happens with this "clmail" example certainly happens with other plugins and can be generalized, hard-coding regexps for that specific plugin is certainly not the way to go. We'll need to integrate that filtering with the #166 effort.

Cross-cuts with PR #167 (alice's dev/filter-hook-turns): the hard-coded [clmail]/[monitor] regex in detect_hook_notification() should live with the plugin that causes those notifications, not in core. Same discovery/priority machinery covers both renderers and transformers. Key design landings (after a back-and-forth with alice via clmail #3088-#3104): - Single entry-point group `claude_code_log.plugins`; loader type-dispatches each entry on Protocol conformance (ToolRenderer vs MessageTransformer). No aggregator class. - Plugins own their MessageContent subclasses; visibility lives on the class via `detail_visibility: ClassVar[DetailLevel]`. Built-in migration from `_HIGH_EXCLUDE_CLASSES` is a follow-up PR, doesn't block #166. - Transformers run inside the factory dispatch chain (not post-factory), preserving existing ordering invariants. - Built-in detector priorities exposed as module constants with gaps of 100 so plugins position relative without renumbering. - v1: first-non-None-wins, no transformer chaining; applies_to accepts any MessageContent subtype (no user-only restriction). Worked example now reduces alice's detect_hook_notification() to ~12 lines as a plugin transformer that owns UserHookNotificationMessage outright.

Reverses the plugin-owned-classes direction added in 356fc0c. Per mid-thread consensus (alice clmail #3106, main clmail #3108): existing-variants-only is the simpler v1 surface and matches PR #167 today. Plugin-owned MessageContent subclasses become a v2 extension that v1 does not foreclose (purely additive migration). Decisive argument: filter-bucket inheritance ("visibility is a property of the target class, owned by core, inherited by every transformer that targets it") removes the need for any detail_visibility class attribute, plugin-side renderer registration, or built-in migration. The contract reduces to a single method (transform) returning an instance of an existing core MessageContent variant. Practical consequence for #167's eventual plugin migration: deletion-only refactor in core (remove the source tuple, regex, and call site). Class, renderer, title, and _HIGH_EXCLUDE_CLASSES membership all remain in core unchanged. Also addresses CodeRabbit feedback on PR #166: adds a _dispatch_format invocation-pattern section spelling out how the dispatcher picks render_input_markdown vs render_output_markdown based on content type (ToolUseContent vs ToolResultContent) and output format (markdown vs html with mistune fallback).

This commit substantially reverses the existing-variants-only v1 scope that landed in 74b5ca6. The architectural shape changes from two parallel plugin Protocols (ToolRenderer + MessageTransformer) to one unified MessageTransformer that handles both tool-rendering and hook-demotion cases. Triggering question (from user via main, clmail #3132): "What if we'd solve the primary need (rendering of generic tools) via v2 as well? That is, intercept the generic tool use/result messages emitted for mcp__plugin_clmail_clmail__communicate (and al.) and convert those to specific messages (ClMailToolUse, ClMailToolResult), THEN register parsing methods for them." Main delegated the decision; I (carol) committed YES (unify) in clmail #3133 with concrete mechanics; alice gave cautious yes in clmail #3135 with two technical asks, both folded in here. Architectural justification: - _dispatch_format already does MRO walk + class-based dispatch. - Built-in tools already use specialized subclasses (BashInputContent and friends) flowing through that dispatch. - The winners[tool_name] table in earlier RFC drafts was a workaround for plugins not having method-binding; eliminate the workaround and one mechanism (transformers + plugin-defined subclasses with class-side format/title methods) suffices for both cases. What the RFC now specifies: - Single MessageTransformer Protocol: matches MessageContent (any subtype via applies_to MRO filter), returns a plugin-defined subclass. - Plugin-defined MessageContent subclasses carry format_markdown / format_html / title as methods on themselves (class-method pattern, not renderer monkey-patching, not global registry). - detail_visibility ClassVar[DetailLevel] on the class governs filter-bucket membership; monotone-down semantics (rendered iff current_detail >= cls.detail_visibility). - _HIGH_EXCLUDE_CLASSES bridge for built-ins not yet migrated; resolution order pinned: class attribute first, registry second. - _dispatch_format resolution: renderer-side format_<ClassName> first (preserves all built-in dispatch unchanged), class-side format_<output> second, MRO walk continues. - Test-embedded reference plugin in test/_plugins/clmail/ doubles as layer-4 fixture and canonical author example. What this costs: - v1 surface expands. Plugins define MessageContent subclasses, register format-method contributions, declare detail_visibility. These were "v2" in earlier RFC drafts; "v1" now. - Rendering philosophy shifts: content classes now carry format_* methods. Today's classes are pure data. Deliberate expansion. - PR #167's eventual plugin migration becomes a full relocation (class + format/title + CSS + tests all move) rather than a deletion-only refactor in core. Tractable; one-shot. A new "Reversal context and trade-offs" section in the RFC names this explicitly so future readers understand the trajectory, not just the destination (per alice's framing ask in clmail #3135). PR #167 stays parked per main's standing instruction; user decides whether to merge as-is and migrate later, or hold.

* Tool-renderer plugin system: design proposal Design-only doc, no implementation. Proposes: - Entry-point discovery via importlib.metadata (stdlib, group "claude_code_log.tool_renderers"), recommended over pluggy. - ToolRenderer Protocol bundling InputModel / OutputModel / render_*_markdown (required) / render_*_html (optional, falls back to Markdown -> mistune). - Priority offset semantics: 0 = builtin, <0 supersedes, >0 fallback; tie-break stable alphabetical + warning. - min_detail only (no max_detail), with the rationale. - Worked example: clmail-communicate plugin replacing the synthetic "[clmail] new mail (#N)" line with the actual mail content at --detail low. Open for review; implementation gated on user approval. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * Extend RFC with message-transformer plugin capability Cross-cuts with PR #167 (alice's dev/filter-hook-turns): the hard-coded [clmail]/[monitor] regex in detect_hook_notification() should live with the plugin that causes those notifications, not in core. Same discovery/priority machinery covers both renderers and transformers. Key design landings (after a back-and-forth with alice via clmail #3088-#3104): - Single entry-point group `claude_code_log.plugins`; loader type-dispatches each entry on Protocol conformance (ToolRenderer vs MessageTransformer). No aggregator class. - Plugins own their MessageContent subclasses; visibility lives on the class via `detail_visibility: ClassVar[DetailLevel]`. Built-in migration from `_HIGH_EXCLUDE_CLASSES` is a follow-up PR, doesn't block #166. - Transformers run inside the factory dispatch chain (not post-factory), preserving existing ordering invariants. - Built-in detector priorities exposed as module constants with gaps of 100 so plugins position relative without renumbering. - v1: first-non-None-wins, no transformer chaining; applies_to accepts any MessageContent subtype (no user-only restriction). Worked example now reduces alice's detect_hook_notification() to ~12 lines as a plugin transformer that owns UserHookNotificationMessage outright. * Lock v1 scope to existing-variants-only + clarify _dispatch_format Reverses the plugin-owned-classes direction added in 356fc0c. Per mid-thread consensus (alice clmail #3106, main clmail #3108): existing-variants-only is the simpler v1 surface and matches PR #167 today. Plugin-owned MessageContent subclasses become a v2 extension that v1 does not foreclose (purely additive migration). Decisive argument: filter-bucket inheritance ("visibility is a property of the target class, owned by core, inherited by every transformer that targets it") removes the need for any detail_visibility class attribute, plugin-side renderer registration, or built-in migration. The contract reduces to a single method (transform) returning an instance of an existing core MessageContent variant. Practical consequence for #167's eventual plugin migration: deletion-only refactor in core (remove the source tuple, regex, and call site). Class, renderer, title, and _HIGH_EXCLUDE_CLASSES membership all remain in core unchanged. Also addresses CodeRabbit feedback on PR #166: adds a _dispatch_format invocation-pattern section spelling out how the dispatcher picks render_input_markdown vs render_output_markdown based on content type (ToolUseContent vs ToolResultContent) and output format (markdown vs html with mistune fallback). * Pin enforcement for text-equivalence guarantee Per alice's read-pass flag #3 (clmail #3119): the RFC stated the principle ("UserTextMessage.text byte-equivalent to extract_text_content") but didn't pin what enforces it. A future factory PR sneaking in normalization between extraction and assignment would silently break plugin regex behaviour. Add a sentence naming the enforcement mechanism: a dedicated equivalence test in the plugin-system test suite that walks the existing JSONL test corpus and asserts byte-equality for every user entry. A normalization-introducing PR fails this test, surfacing the contract break. * Reverse v1 scope to unified plugin mechanism This commit substantially reverses the existing-variants-only v1 scope that landed in 74b5ca6. The architectural shape changes from two parallel plugin Protocols (ToolRenderer + MessageTransformer) to one unified MessageTransformer that handles both tool-rendering and hook-demotion cases. Triggering question (from user via main, clmail #3132): "What if we'd solve the primary need (rendering of generic tools) via v2 as well? That is, intercept the generic tool use/result messages emitted for mcp__plugin_clmail_clmail__communicate (and al.) and convert those to specific messages (ClMailToolUse, ClMailToolResult), THEN register parsing methods for them." Main delegated the decision; I (carol) committed YES (unify) in clmail #3133 with concrete mechanics; alice gave cautious yes in clmail #3135 with two technical asks, both folded in here. Architectural justification: - _dispatch_format already does MRO walk + class-based dispatch. - Built-in tools already use specialized subclasses (BashInputContent and friends) flowing through that dispatch. - The winners[tool_name] table in earlier RFC drafts was a workaround for plugins not having method-binding; eliminate the workaround and one mechanism (transformers + plugin-defined subclasses with class-side format/title methods) suffices for both cases. What the RFC now specifies: - Single MessageTransformer Protocol: matches MessageContent (any subtype via applies_to MRO filter), returns a plugin-defined subclass. - Plugin-defined MessageContent subclasses carry format_markdown / format_html / title as methods on themselves (class-method pattern, not renderer monkey-patching, not global registry). - detail_visibility ClassVar[DetailLevel] on the class governs filter-bucket membership; monotone-down semantics (rendered iff current_detail >= cls.detail_visibility). - _HIGH_EXCLUDE_CLASSES bridge for built-ins not yet migrated; resolution order pinned: class attribute first, registry second. - _dispatch_format resolution: renderer-side format_<ClassName> first (preserves all built-in dispatch unchanged), class-side format_<output> second, MRO walk continues. - Test-embedded reference plugin in test/_plugins/clmail/ doubles as layer-4 fixture and canonical author example. What this costs: - v1 surface expands. Plugins define MessageContent subclasses, register format-method contributions, declare detail_visibility. These were "v2" in earlier RFC drafts; "v1" now. - Rendering philosophy shifts: content classes now carry format_* methods. Today's classes are pure data. Deliberate expansion. - PR #167's eventual plugin migration becomes a full relocation (class + format/title + CSS + tests all move) rather than a deletion-only refactor in core. Tractable; one-shot. A new "Reversal context and trade-offs" section in the RFC names this explicitly so future readers understand the trajectory, not just the destination (per alice's framing ask in clmail #3135). PR #167 stays parked per main's standing instruction; user decides whether to merge as-is and migrate later, or hold. * Address two CR Major flags on unified RFC 1. _dispatch_format pseudocode signature (CR #3293284687): pass message argument explicitly through both strategies rather than reading content.message (which is not part of the MessageContent contract). Plugin-defined class-side format methods already had the (self, renderer, message) signature throughout the worked example; only the dispatcher pseudocode was inconsistent. Added a paragraph below the pseudocode spelling out the call contract and the rationale for not storing a back-pointer on MessageContent. 2. Tie-break key stability (CR #3293284688): use (priority, __module__, __qualname__) instead of (priority, __name__) so two plugins shipping classes with identical short names don't get OS-dependent ordering. Warn message updated to print fully-qualified types. --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

cboos · 2026-05-24T19:45:24Z

(Claude) Closing in favour of the plugin system landed by PR #166 and PR #169.

Why this PR is being closed

PR #166 (plugin system RFC, merged at 9926c96) and PR #169 (implementation, awaiting merge ack) together provide a generic plugin mechanism — MessageTransformer + plugin-defined MessageContent subclasses with class-side format_markdown / format_html / title methods and a detail_visibility class attribute — that fully subsumes this PR's functionality. The upcoming claude-code-log-clmail plugin will ship the same [monitor] / [clmail] demotion via that contract, with the regex and detection logic moving from core to the plugin.

The framing here — hard-coding two specific external hook sources (monitor, clmail) in claude_code_log/factories/user_factory.py — turned out to be the wrong layer. Those sources belong with the plugin that causes the notifications, not in core's user-message factory. That observation from the user during review of this PR is what triggered the cross-cutting design discussion that produced PR #166's unified plugin design.

What carries forward into the plugin

This PR's design work shaped the v1 plugin contract in concrete ways:

The four-deliverable framing — matcher, MessageContent subclass, format / title methods, detail-level filter membership — became the structural skeleton of MessageTransformer plus plugin-defined subclasses in PR RFC: plugin system (unified message-transformer mechanism) #166.
The multi-line guard (real user prompts that start with [monitor] foo\n\nActually do X must be preserved, not demoted) is documented in PR RFC: plugin system (unified message-transformer mechanism) #166's worked example as a plugin-side concern, not a core contract — the right separation.
The CSS pill (div.hook-notification with dimmed border + monospace body) is reference styling future plugin authors can adopt or supersede.
The test fixture test/test_data/hook_user_notifications.jsonl (9 entries: pure hook lines, multi-line guard cases, real markers like [Image #N] / [Request interrupted by user] that must be preserved) is reusable as-is in the plugin's test suite.

A future claude-code-log-clmail plugin author can use this PR's diff as a near-complete reference for what to ship — every piece relocates, nothing is lost.

Migration path: full relocation, not deletion-only

An earlier point in the design discussion considered a smaller v1 scope (existing-variants-only) under which this PR's migration would have been deletion-only in core. The user's question that prompted PR #166's unified design changed that calculus: unified-v1 lets plugins own their MessageContent subclasses outright, which means the migration of this PR's contents is full relocation rather than deletion-only.

Concretely, the future clmail plugin will own:

UserHookNotificationMessage (the class)
format_UserHookNotificationMessage / title_UserHookNotificationMessage (as class-side format_markdown / title methods on the plugin-owned subclass)
The CSS pill styling
The 14 regression tests from test/test_hook_user_notifications.py
The _HIGH_EXCLUDE_CLASSES membership (migrating to detail_visibility = FULL on the class)

Bigger one-shot relocation, but architecturally cleaner: core stops carrying clmail-specific assumptions, and the plugin becomes the single source of truth for both the matchers and the rendering.

Branch retention

Branch dev/filter-hook-turns is left in place as a reference for the future plugin author. The fixture file and test cases are particularly reusable.

cboos added the plugin Feature is related to creating plugins label May 23, 2026

cboos mentioned this pull request May 23, 2026

RFC: plugin system (unified message-transformer mechanism) #166

Merged

5 tasks

cboos closed this May 24, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Filter hook-injected synthetic user turns at HIGH detail and below#167

Filter hook-injected synthetic user turns at HIGH detail and below#167
cboos wants to merge 1 commit into
mainfrom
dev/filter-hook-turns

cboos commented May 23, 2026

Uh oh!

coderabbitai Bot commented May 23, 2026 •

edited

Loading

Review failed

Uh oh!

cboos commented May 23, 2026

Uh oh!

cboos commented May 23, 2026

Uh oh!

cboos commented May 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

cboos commented May 23, 2026

Summary

Detection

Rendering

Latent markdown-renderer fix

Test plan

Uh oh!

coderabbitai Bot commented May 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Uh oh!

cboos commented May 23, 2026

Uh oh!

cboos commented May 23, 2026

Uh oh!

cboos commented May 24, 2026

Why this PR is being closed

What carries forward into the plugin

Migration path: full relocation, not deletion-only

Branch retention

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented May 23, 2026 •

edited

Loading