Implement unified plugin system from RFC #166#169
Merged
Conversation
Implements the unified plugin discovery + dispatch machinery from the RFC at work/tool-renderer-plugins.md: - claude_code_log.plugins module: MessageTransformer Protocol (runtime_checkable with required ClassVar metadata), entry-point loader at group "claude_code_log.plugins", priority sort using (priority, __module__, __qualname__) for stable cross-environment ordering, tie-break warning on duplicate (priority, applies_to), module-level cache with reset hook for tests. - claude_code_log.factories.priorities module: public constants for built-in detector positions (COMMAND_MESSAGE=100, ..., HOOK_NOTIFICATION=600, ..., TEXT_FALLBACK=1000, TOOL_INPUT_GENERIC=5000, TOOL_OUTPUT_GENERIC=5100). Plugins import these to position themselves relative to built-ins without renumbering on every core change. Gaps of 100 leave room for plugin insertion. apply_transformers() is the helper factories call: walks the priority-ordered list, calls transform() on the first transformer whose applies_to subclass-matches the candidate, returns first non-None replacement. Transformer exceptions are caught and logged so a buggy plugin can't crash conversion. No call sites are wired yet; that's the next commit. Loader stands alone and is fully covered by upcoming layer-1 tests.
Three insertion points where the factory finishes building its candidate MessageContent and the plugin pass should run: - create_user_message: split into a thin wrapper + the existing body renamed to _classify_user_message. The wrapper applies the plugin transformer pass to the result, so every classification path (slash-command, bash, teammate, task-notification, hook-detect, generic text) becomes rewriteable by a plugin, not just the fallback. Transformers whose applies_to doesn't subclass-match pass through with no-op cost. - create_tool_use_message: apply_transformers after ToolUseMessage construction, before the ToolItemResult return. - create_tool_result_message: apply_transformers after ToolResultMessage construction, before the ToolItemResult return. Without any plugin installed, all three sites are no-ops (the loader cache is empty). With a plugin installed, transformers can specialize the generic MessageContent into plugin-defined subclasses (e.g. ClmailCommunicateInputMessage from generic ToolUseMessage) and the renderer's _dispatch_format picks them up via the existing MRO walk plus the upcoming class-side fallback. The RFC's "in-factory-dispatch interleaved with built-in detectors" framing is implemented here as a *post-classification* pass: built-in detectors run first in their hardcoded order, plugin transformers run after. The effect is the same for every v1 use case (clmail hook-demotion, MCP tool rendering) because plugins always operate on a candidate the built-in chain has already classified (typically UserTextMessage or generic ToolUseMessage). Documented as a deviation in the plugins.py module docstring.
…ty bridge
Two related changes that let plugin-defined MessageContent subclasses
participate in rendering and filtering without modifying core.
_dispatch_format / _dispatch_title resolution-order extension
==============================================================
Both dispatchers now use a two-strategy resolution per MRO node:
1. Renderer-side format_<ClassName>(self, obj, message) — preserves
all existing built-in dispatch unchanged. The renderer class
continues to carry hand-written format_BashInput,
format_ToolUseMessage, etc.
2. Class-side format_<output>(self, renderer, message) on the
content class itself (where <output> is "markdown" or "html"
per Renderer._class_dispatch_format). Used by plugin-defined
subclasses that carry their own render methods.
Renderer-side wins first per MRO node. A plugin subclass that wants
to shadow a built-in's renderer method defines the class-side method
on the plugin subclass — the MRO walk visits it first.
HtmlRenderer overrides _class_dispatch_format = "html" so plugin
classes' format_html is consulted (with format_markdown as the
mistune fallback at the call site, not the dispatcher).
detail_visibility class attribute + _HIGH_EXCLUDE_CLASSES bridge
================================================================
A MessageContent subclass may declare detail_visibility: ClassVar[DetailLevel]
to opt into class-based filter membership. Monotone-down semantics:
a message is visible iff current_detail is at least as verbose as
the declared minimum, using the canonical DetailLevel ordering
FULL > HIGH > LOW > MINIMAL > USER_ONLY pinned in a module-level
_DETAIL_ORDER map.
_content_visible_at() helper resolves visibility in three steps:
1. Class attribute (plugin classes + future migrated built-ins)
2. _HIGH_EXCLUDE_CLASSES / _LOW_EXCLUDE_CLASSES / etc. bridge for
built-ins not yet migrated (isinstance semantics, so plugin
subclasses of a built-in inherit its filter membership unless
they override).
3. Default visible.
_filter_template_by_detail rewritten to call this helper. The LOW
keep-list (_LOW_KEEP_TOOLS = {"WebSearch","WebFetch","Task","Agent"})
is a positive filter at LOW (ToolUseMessage isn't in
_LOW_EXCLUDE_CLASSES, only in _MINIMAL_EXCLUDE_CLASSES) and bypassed
for plugin classes that declare detail_visibility — so a clmail
communicate plugin with detail_visibility=LOW shows up at --detail
low without core needing to update _LOW_KEEP_TOOLS.
Built-in migration to detail_visibility is a follow-up; the bridge
keeps existing behaviour identical for the 1892 unit tests.
A minimal claude-code-log-clmail-test package that exercises both
sides of the plugin contract and doubles as the canonical example
for third-party plugin authors.
Layout:
test/_plugins/clmail/
pyproject.toml # entry-point declarations
README.md # author-facing one-pager
src/claude_code_log_clmail_test/
__init__.py
transformers/
hook_demotion.py # UserTextMessage rewrite via "[testhook]" prefix
tool_communicate.py # ToolUseMessage rewrite for a specific MCP tool
Both transformers exercise the full v1 contract:
- Class-side format_markdown / format_html / title methods on
plugin-defined MessageContent subclasses (Strategy 2 of the new
_dispatch_format).
- detail_visibility ClassVar[DetailLevel] for filter-bucket
membership without touching core registries.
- applies_to MRO filter targeting an existing core class
(UserTextMessage, ToolUseMessage).
The tool_communicate transformer targets a stable test-fixture tool
name (mcp__test_plugin__clmail__communicate) so it doesn't collide
with any real tool that production fixtures might emit.
Wired as a dev-dependency via uv.sources with `path = ...,
editable = true`, so `uv sync` installs it automatically. Production
installs of claude-code-log don't see it; CI does. Forcing a
reinstall after pyproject changes: `uv sync --reinstall-package
claude-code-log-clmail-test`.
Four layers covering the contract end-to-end, per the RFC's test strategy section. Layer 1 — loader unit tests (no fixtures, no entry points): - _validate_transformer_class rejects missing name, non-int priority, empty applies_to, non-MessageContent applies_to entry. - _sort_and_warn orders by (priority, module, qualname) and emits a "priority tie" warning on duplicate (priority, applies_to). - load_transformers caches at module scope; force_reload=True invalidates the cache. Layer 2 — _dispatch_format four-way matrix: - both renderer-method and class-method present → renderer wins - renderer-method only → renderer wins - class-method only → class wins - neither → returns empty / None - HtmlRenderer subclass dispatches format_html via Strategy 2 (verifies _class_dispatch_format override at the class level) Layer 3 — transformer integration (uses the embedded reference plugin): - Matching "[testhook]" text demotes to TestHookNotificationMessage. - Non-matching text passes through as UserTextMessage. - Multi-line guard: real prompts starting with "[testhook]" but containing newlines pass through. - Specialized tool subclass renders via class-side format_markdown and title methods. - apply_transformers catches plugin exceptions, logs, passes through. Layer 4 — text-equivalence guarantee: - Walks the existing JSONL test corpus (dag_simple/dag_fork/cron_tools). - For each UserTextMessage candidate, asserts the joined text from UserTextMessage.items matches what the factory's extract_text_content would have produced. Catches future factory PRs that sneak normalization between extraction and assignment. Tests that depend on the embedded plugin use @reference_plugin_required to skip cleanly when it's not installed (the package is a dev-dep so `uv sync` brings it in; this skip is for stray environments). All 21 plugin-system tests pass. Full unit-test suite: 1892 passed, 7 skipped. just ci clean.
📝 WalkthroughWalkthroughAdds a post-classification plugin system: priority constants, entry-point loader/validator with caching, runtime dispatcher (apply_transformers) enforcing return contracts, integration into user/tool factories, renderer dispatch/detail-visibility generalization, a reference plugin package, and comprehensive tests. ChangesPlugin System Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
1. MessageTransformer Protocol docstring: document the v1 trust contract that transform()'s return SHOULD be an instance of one of applies_to (or a subclass). v1 doesn't runtime-enforce this; caller's typing narrowing (e.g. UserMessageContent in create_user_message) assumes it. A v2 enhancement may add an isinstance check. 2. renderer.py: assert _DETAIL_ORDER covers all DetailLevel values at module load. Without this, adding a new DetailLevel value would surface as a KeyError on first visibility check — silent until exercised. Fail loudly at import instead. 3. tool_communicate.py: clarify the priority comment in the embedded reference plugin. Under the v1 post-classification implementation, priority orders transformers among themselves (not against built-in classifiers, which have already run). The earlier "Run before generic tool classification" phrasing was mildly misleading for plugin-author readers consulting the canonical example. All three are doc / safety-rail changes; no behavioural impact. just ci clean.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
test/test_plugin_system.py (1)
493-503: ⚡ Quick winPrefix-based fallback makes the equivalence test too permissive.
text_content.startswith(reconstructed[:50])can pass even when strings diverge significantly, which weakens this regression guard.🔧 Suggested tightening
- assert text_content == reconstructed or text_content.startswith( - reconstructed[:50] - ), ( + assert text_content == reconstructed, ( f"text-equivalence drift in {path.name}: " f"factory saw {text_content[:80]!r}, " f"UserTextMessage carries {reconstructed[:80]!r}" )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/test_plugin_system.py` around lines 493 - 503, The current assertion uses text_content.startswith(reconstructed[:50]) which is too permissive; change the fallback to require an exact match on a fixed prefix (e.g. compare text_content[:50] == reconstructed[:50]) or require reconstructed to be an actual prefix (text_content.startswith(reconstructed)) with a minimum length guard, so that partial 50-char prefixes must be identical rather than allowing reconstructed[:50] to be a prefix of text_content; update the assertion condition that references text_content and reconstructed accordingly while keeping the same failure message (including path.name and the slices used).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@claude_code_log/plugins.py`:
- Around line 196-208: The current loop only compares adjacent items so it can
miss ties when multiple transformers share the same (priority, applies_to) but
are not adjacent; replace the pairwise adjacent check with a grouping pass over
transformers keyed by (priority, applies_to) (use the existing transformers
list) and emit logger.warning for any group with more than one member, including
the same contextual fields currently used (applies_to, priority and the
module/qualname of each colliding transformer) to list all colliding
transformers; keep the function return of transformers unchanged.
- Around line 269-270: The code currently accepts any non-None "replacement"
returned by a transformer; add a validation step before returning it: in the
block that checks "if replacement is not None:" (in plugins.py where
transformers are applied), validate the returned object with a whitelist/type
check or a dedicated validator (e.g., isinstance(replacement, ExpectedType) or
call validate_replacement(replacement)) and only return it if it passes;
otherwise log and raise/ignore (raise TypeError or continue) so incompatible
objects cannot break downstream code.
In `@claude_code_log/renderer.py`:
- Around line 4258-4274: _title_content currently bypasses _dispatch_title so
plugin subclasses like ToolUseMessage/ToolResultMessage never reach their
class-side title() because renderer-level
title_ToolUseMessage/title_ToolResultMessage shadow them; change title_content
to delegate to _dispatch_title (using the same resolution order) instead of its
renderer-only loop so nested tool input/output flows through _dispatch_title and
will call class_method title() on the message's MRO when present; update
references to title_content, _dispatch_title, title_ToolUseMessage and
title_ToolResultMessage to ensure calls use _dispatch_title return value and
preserve the existing fallback behavior when _dispatch_title returns None.
In
`@test/_plugins/clmail/src/claude_code_log_clmail_test/transformers/hook_demotion.py`:
- Around line 57-84: The regex _PATTERN currently uses "\s*" after "[testhook]"
which consumes newlines and lets "[testhook]\ncontinue" be treated as
single-line; update the pattern used in TestHookDemotion.transform so it only
allows horizontal whitespace after the marker (e.g., replace the "\s*"
immediately following "\[testhook\]" with "[ \t]*" or explicitly disallow "\n"),
e.g. change _PATTERN to something like r"^\s*\[testhook\][ \t]*(.*?)\s*\Z" so
the multi-line guard (the "\n" in m.group(1) check) correctly detects newlines
and prevents demotion.
In `@test/test_plugin_system.py`:
- Around line 146-152: The test mutates the shared class attribute by doing
a.__class__.priority = 100 so both instances end up with the same priority;
instead, assign different priority values on the instances to shadow the
ClassVar (e.g., set a.priority = 100 and b.priority = 0) so they are distinct,
then call _sort_and_warn([b, a]) and assert the ordering (use strict comparison
like sorted_[0].priority < sorted_[1].priority) to verify _sort_and_warn
correctly sorts by priority; references: test_sorts_by_priority,
_GoodTransformer, and _sort_and_warn.
---
Nitpick comments:
In `@test/test_plugin_system.py`:
- Around line 493-503: The current assertion uses
text_content.startswith(reconstructed[:50]) which is too permissive; change the
fallback to require an exact match on a fixed prefix (e.g. compare
text_content[:50] == reconstructed[:50]) or require reconstructed to be an
actual prefix (text_content.startswith(reconstructed)) with a minimum length
guard, so that partial 50-char prefixes must be identical rather than allowing
reconstructed[:50] to be a prefix of text_content; update the assertion
condition that references text_content and reconstructed accordingly while
keeping the same failure message (including path.name and the slices used).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9962d230-c063-49df-8a56-e6baaa546ff2
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
claude_code_log/factories/priorities.pyclaude_code_log/factories/tool_factory.pyclaude_code_log/factories/user_factory.pyclaude_code_log/html/renderer.pyclaude_code_log/plugins.pyclaude_code_log/renderer.pypyproject.tomltest/_plugins/clmail/README.mdtest/_plugins/clmail/pyproject.tomltest/_plugins/clmail/src/claude_code_log_clmail_test/__init__.pytest/_plugins/clmail/src/claude_code_log_clmail_test/transformers/__init__.pytest/_plugins/clmail/src/claude_code_log_clmail_test/transformers/hook_demotion.pytest/_plugins/clmail/src/claude_code_log_clmail_test/transformers/tool_communicate.pytest/test_plugin_system.py
1. plugins.py:_sort_and_warn — group by (priority, applies_to) via a dict instead of pairwise-adjacent zip. The sort key is (priority, module, qualname), so two transformers with the same priority but different applies_to can sit between two genuine collision partners; the adjacent check would have missed them. Group-by surfaces every collision regardless of sort position. 2. plugins.py:apply_transformers — runtime type-enforcement upgrade. v1 now actively rejects transformer returns that aren't MessageContent instances OR don't subclass-match applies_to. Both monk (#3175) and CR independently flagged this; promoting from documented-trust-contract to actively-enforced makes plugin authoring errors loud and keeps a buggy plugin from crashing downstream invisibly. 3. renderer.py:title_content — delegate to _dispatch_title instead of re-implementing the renderer-only MRO walk inline. Without this, plugin ToolUseMessage/ToolResultMessage subclasses' class-side title() methods are silently shadowed at the top level by title_ToolUseMessage / title_ToolResultMessage on the base renderer. Snapshot regression caught a related bug: title_ToolResultMessage returns "" (empty string, meaning "no header") for non-error results; the walrus truthy-check would incorrectly fall through to the message_type default. Replaced with explicit `if title is not None` to preserve the empty-string sentinel semantics. 4. hook_demotion.py — move the multi-line guard to a pre-regex whole-text check. The pattern's \s* after [testhook] would otherwise consume a newline immediately after the marker, slipping multi-line prompts past a body-only check. Made the plugin's TestHookNotificationMessage a UserTextMessage subclass (not a sibling) so it satisfies the new return-type enforcement. 5. test_plugin_system.py:test_sorts_by_priority — use local subclasses with explicit priorities instead of mutating _GoodTransformer's module-level ClassVar. Same concern monk raised as a parting note. Plus three new regression tests for the runtime enforcement (TestApplyTransformersReturnTypeEnforcement), the non-adjacent collision detection (TestSortAndWarnNonAdjacentCollision), and the newline-immediately-after-prefix case (test_newline_immediately_after_prefix_passes_through). Test summary: 1896 passed, 7 skipped (+4 from previous CR-fixup commit). just ci clean across all stages.
4 tasks
Adds dev-docs/plugins.md as the as-built reference for plugin authors writing third-party plugins via the entry-point group ``claude_code_log.plugins``. Designed so an external actor (e.g. a clmail-plugin project) can read this single page and produce a working plugin without further design context. Covers: - the MessageTransformer Protocol surface (name / priority / applies_to / transform); - the two-strategy _dispatch_format / _dispatch_title resolution order (renderer-side first per MRO node, class-side second); - detail_visibility semantics + the FULL > HIGH > LOW > MINIMAL > USER_ONLY ordering pinned in _DETAIL_ORDER; - runtime contract enforcement (exception safety + return-type isinstance check against applies_to); - entry-point discovery + priority ordering (with the v1 post-classification deviation from the RFC documented); - first non-None wins semantics; - a four-layer testing recipe mirroring test_plugin_system.py; - common patterns and pitfalls (defensive narrowing, format_html None-fallback to mistune, inline-code backtick widening, inheritance requirement for return-type enforcement, priority constants over literals, cache invalidation in tests); - a v2 directions section so contributors don't keep re-rediscovering them (interleaved dispatch, renderer-side plugin extension, priority namespacing). Heavily references the existing test-embedded reference plugin at test/_plugins/clmail/ as the canonical example template, per the RFC's intent that the test plugin double as the author reference (and stay in sync with working code by construction). Wires the new doc into both CLAUDE.md and dev-docs/application_model.md per the dev-docs sync convention. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dev-docs/plugins.md`:
- Around line 474-479: Update the paragraph describing the LOW keep-list
behavior to reflect actual runtime lookup: mention that visibility is read via
class-attribute lookup using getattr(type(content), "detail_visibility", None)
(not hasattr) and that the exclude-class bridge logic is applied afterward, so
inheriting a detail_visibility from a parent class affects visibility the same
as declaring it directly; reference detail_visibility, getattr(type(content),
"detail_visibility", None), and the exclude-class bridge/_LOW_KEEP_TOOLS to help
authors debug mismatches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1a203ac3-684d-45b1-a02a-930af0179498
📒 Files selected for processing (3)
CLAUDE.mddev-docs/application_model.mddev-docs/plugins.md
✅ Files skipped from review due to trivial changes (1)
- CLAUDE.md
Comment on lines
+474
to
+479
| **`detail_visibility` is checked via `hasattr` for the LOW keep-list | ||
| opt-out.** This means inheriting `detail_visibility` from a future | ||
| core-migrated parent class behaves the same as declaring it | ||
| yourself: the keep-list is bypassed. Usually what you want; mention | ||
| it if you're debugging a "why is my plugin visible at LOW even though | ||
| the tool isn't in `_LOW_KEEP_TOOLS`?" question. |
There was a problem hiding this comment.
Correct the detail_visibility mechanism description to match runtime behavior.
This section says visibility uses hasattr and frames behavior as a LOW keep-list opt-out, but runtime checks use class-attribute lookup (getattr(type(content), "detail_visibility", None)) and then the exclude-class bridge. Please update wording to avoid misleading plugin authors when debugging visibility behavior.
Suggested doc patch
-**`detail_visibility` is checked via `hasattr` for the LOW keep-list
-opt-out.** This means inheriting `detail_visibility` from a future
-core-migrated parent class behaves the same as declaring it
-yourself: the keep-list is bypassed. Usually what you want; mention
-it if you're debugging a "why is my plugin visible at LOW even though
-the tool isn't in `_LOW_KEEP_TOOLS`?" question.
+**`detail_visibility` is resolved from the content class before the
+legacy bridge.** Runtime checks read `type(content).detail_visibility`
+(when present) and apply monotone-down visibility directly; only when
+that attribute is absent does filtering fall back to the built-in
+exclude-class bridge. This means inheriting `detail_visibility` from a
+parent class behaves the same as declaring it on the subclass.Based on learnings: "Treat dev-docs/ as as-built reference documentation - the code is the authoritative source, and if dev-docs/ and code disagree, the documentation is wrong."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| **`detail_visibility` is checked via `hasattr` for the LOW keep-list | |
| opt-out.** This means inheriting `detail_visibility` from a future | |
| core-migrated parent class behaves the same as declaring it | |
| yourself: the keep-list is bypassed. Usually what you want; mention | |
| it if you're debugging a "why is my plugin visible at LOW even though | |
| the tool isn't in `_LOW_KEEP_TOOLS`?" question. | |
| **`detail_visibility` is resolved from the content class before the | |
| legacy bridge.** Runtime checks read `type(content).detail_visibility` | |
| (when present) and apply monotone-down visibility directly; only when | |
| that attribute is absent does filtering fall back to the built-in | |
| exclude-class bridge. This means inheriting `detail_visibility` from a | |
| parent class behaves the same as declaring it on the subclass. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@dev-docs/plugins.md` around lines 474 - 479, Update the paragraph describing
the LOW keep-list behavior to reflect actual runtime lookup: mention that
visibility is read via class-attribute lookup using getattr(type(content),
"detail_visibility", None) (not hasattr) and that the exclude-class bridge logic
is applied afterward, so inheriting a detail_visibility from a parent class
affects visibility the same as declaring it directly; reference
detail_visibility, getattr(type(content), "detail_visibility", None), and the
exclude-class bridge/_LOW_KEEP_TOOLS to help authors debug mismatches.
This was referenced May 25, 2026
cboos
added a commit
that referenced
this pull request
May 25, 2026
`work/` had accumulated a few plan docs whose contents have since landed. Triage per main #3198, executed for the three shipped-cleanly cases (the plugin-system RFC + the still-unshipped docs stay untouched in this PR — they belong to a separate phase). - **DELETE** `work/async-agents.md` — fully shipped in PR #132 (`294ed3b`) with the cross-link follow-up in PR #158 (`1ab9f13`). Content reflected in `dev-docs/agents.md`. - **SHRINK** `work/rendering-next.md` — drop the §2 ✅ COMPLETED signature-unification subsection (ancient history) and §3 "Additional Tool Output Parsers" (`GlobOutput` / `GrepOutput` / `WebSearchOutput` / `WebFetchOutput` all live in `models.py` now). Keep §1 (recursive template), §2 (visitor), and §3 (performance) — still speculative. Renumbered the survivors. - **SHRINK** `work/obsidian-friendly-output.md` — the main implementation shipped in PR #155 (`8de5b89`); per-message Markdown timestamps shipped in PR #165 (`68e5bfb`); the `--combined yes/no/only` flag and the `--expand-paths` bullet-list Markdown index both shipped too. Drop all of that; retain the seven open follow-ups (cache-freshness destination- awareness, archived projects with `--output`, the absolute- `--filter-path`-without-`--expand-paths` silent-exclude footgun, `--filter-path` auto-implying `--all-projects`, `--expand-paths` for single-session, `--dry-run` mode, plus Obsidian frontmatter / wikilinks / peek-debug-logging / symlinks). Untouched (still unshipped or shipped-but-pending follow-up): `work/refactor-reindex-with-ghosting.md`, `work/session-state-propagation.md`, `work/tool-renderer-plugins.md` (the RFC graduates to `dev-docs/plugins.md` only once PR #169 merges — phase 2). No code changes; existing inbound links from `dev-docs/{dag,messages}.md` to `rendering-next.md` remain valid (the file still exists, just shrunk). Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Collaborator
Author
|
We're going to merge that, as it "kind of works", but the real plugin I developed using this, though usable, had some shortcomings that are going to be addressed in a second phase. A follow-up PR will make code review easier. |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the unified message-transformer plugin system specified in
work/tool-renderer-plugins.md(RFC merged in #166).Five linear commits, each leaving
just ciclean:plugins.py(Protocol, entry-point loader, priority sort with tie-break, exception-safe dispatch) +factories/priorities.py(public constants for built-in detector positions).apply_transformerspass aftercreate_user_message,create_tool_use_message,create_tool_result_message._dispatch_format/_dispatch_titletwo-strategy resolution (renderer-side wins, class-side fallback);detail_visibilityclass attribute with_HIGH_EXCLUDE_CLASSESbridge for built-ins.test/_plugins/clmail/exercises both branches of the contract; doubles as the canonical author example. Installed via dev-dependency.Net diff: +1290 / -22, including the test plugin and 506 lines of tests.
v1 scope notes
plugins.pymodule docstring). Same effect for every v1 use case because plugins always operate on a candidate that the built-in chain has already classified (typicallyUserTextMessageor genericToolUseMessage).MessageContentsubclasses retain their_HIGH_EXCLUDE_CLASSESmembership in v1; migration to per-classdetail_visibilityattributes is mechanical follow-up.format_markdown/format_html/titlemethods +detail_visibilitydeclaration +applies_toMRO filter.Test plan
just test(1892 passed, 7 skipped — includes 21 new plugin tests)just ciclean (format + test-all + lint + pyright + ty)Summary by CodeRabbit
New Features
Documentation
Tests