-
Notifications
You must be signed in to change notification settings - Fork 55
Consolidate Rendering Architecture #74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add UserMemoryMessage for <user-memory-input> tags - Add token_usage field to ThinkingMessage and AssistantTextMessage - Add raw_text_content field to AssistantTextMessage - Add gitBranch field to BaseTranscriptEntry - Update GrepInput fields (multiline, head_limit, offset) - Update TaskInput fields (run_in_background, resume) - Expand ToolOutput union with all output types - Add DedupNoticeMessage to CSS class table and registry 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
The raw_text_content field was stored on every UserTextMessage and AssistantTextMessage, but only used for deduplication of sidechain assistant messages against Task results. Instead, compute the text on-demand when needed during dedup checking. - Remove raw_text_content from UserTextMessage and AssistantTextMessage - Update renderer.py to extract text inline during dedup check - Update factories to not compute/set the field - Update messages.md documentation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Instead of replacing duplicate sidechain assistant messages with a DedupNoticeMessage (a forward link), simply drop them from the children list entirely. This simplifies the codebase by removing: - DedupNoticeMessage class from models.py - format_dedup_notice_content from system_formatters.py - CSS class registry entry for dedup-notice - HTML and Markdown renderer methods - All related imports and documentation The deduplication logic now just removes the duplicate message from the sidechain children list when it matches the Task result text. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Remind to always use -n auto for parallel execution, with optional -x to stop on first failure during development. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Merge 5 historical refactoring documents into coherent architecture docs: **New files:** - rendering-architecture.md: Current rendering pipeline and class hierarchy - rendering-next.md: Future work (recursive templates, visitor pattern, etc.) **Removed files (content consolidated):** - MESSAGE_REFACTORING.md (Phase 1 refactoring plan) - MESSAGE_REFACTORING2.md (Phase 2 refactoring plan) - RENAME_CONTENT_TO_MESSAGE.md (naming conventions) - TEMPLATE_MESSAGE_CHILDREN.md (tree architecture) - TEMPLATE_MESSAGE_REFACTORING.md (TemplateMessage simplification) **Updates:** - messages.md: Fix cross-references to new docs - models.py: Remove obsolete file reference in comment 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
The lenient parsers (_parse_*_lenient functions) were removed because: 1. **Minimal test coverage**: Only TaskInput had lenient parser tests. The others (BashInput, EditInput, TodoWriteInput, AskUserQuestionInput) were essentially untested dead code. 2. **Limited value vs Pydantic strict validation**: Lenient parsers provided defaults for missing optional fields, but Pydantic model_validate() already handles optional fields via Field(default=...). The only edge case helped was coercing list items (e.g., string → dict in todos), which is rare. 3. **Bug in original code**: The lenient parser call wasn't wrapped in try/except, so exceptions would crash instead of falling back to ToolUseContent. This made them unreliable. 4. **ToolUseContent is sufficient fallback**: When strict parsing fails, falling back to ToolUseContent preserves all original data for display. The only difference is generic titles (e.g., "TodoWrite" instead of "📝 Todo List"), which is acceptable for malformed inputs. 5. **Code reduction**: Removes ~140 lines of complexity including 5 lenient parser functions and the TOOL_LENIENT_PARSERS registry. Snapshot tests updated to reflect the expected behavior change: malformed tool inputs now show generic tool name titles instead of specialized ones. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Both HtmlRenderer and MarkdownRenderer now use the shared export_image() utility for image handling, ensuring consistent support for all three image export modes (embedded, referenced, placeholder). Changes: - Refactor export_image() to return src only (format-agnostic) Returns data URL for embedded, relative path for referenced, None for others - Add HtmlRenderer._format_image() method that formats src as HTML img tag - Add image_formatter callback to format_assistant_text_content() and format_user_text_model_content() for flexible image handling - Update format_image_content() to use export_image() for embedded mode - Set _output_dir and reset _image_counter in HtmlRenderer.generate() - Add comprehensive tests for all export_image() code paths Defaults: - HTML: "embedded" (base64 data URLs in <img> tags) - Markdown: "referenced" (external files in images/ directory) Coverage of image_export.py improved from 76.92% to 96.00%. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Remove unused functions from tool_formatters.py (~87 lines) - Remove from __all__ exports in both tool_formatters.py and __init__.py - Remove unused imports (Optional, ToolInput) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
The function is only called with ThinkingContent (checked at call site), so the ContentItem parameter type and isinstance check were unnecessary. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Remove unused message_timings parameter (always passed empty list) - Inline timing report in _flatten_preorder instead of generate() - Simplify _flatten_preorder return type (no longer returns timing data) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Since JSONL parsing always produces None or dict, branches for UsageInfo instances and SDK types with attributes were never reached. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Remove TemplateMessage.has_markdown property (template uses content.has_markdown) - Remove slash-command pairing logic (caveat messages have type "user", not "system") - Remove slash_command_by_parent from PairingIndices - Remove slash-command indexing in _build_pairing_indices - Remove system + slash-command pairing in _try_pair_by_index - Remove slash_command_pair_index in _reorder_paired_messages - Remove _extract_task_result_text function, inline with walrus operators - Add tests for assistant message image rendering (future image generation) - Update messages.md to clarify has_markdown access pattern 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
All format_* and title_* methods now receive both parameters: - The typed object (e.g., BashInput) for type-safe field access - The TemplateMessage for context (paired messages, ancestry, etc.) Changes: - Update _dispatch_format() and _dispatch_title() to pass both args - Update all format_/title_ methods in Renderer, HtmlRenderer, MarkdownRenderer - Use _ or _message for unused parameters (LSP compliance) - Remove unused cast import from markdown/renderer.py - Update test_todowrite_rendering.py to pass TemplateMessage wrapper - Update rendering-architecture.md and rendering-next.md documentation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- README.md: Slim down to user-focused content (features, usage, install) - CONTRIBUTING.md: New file for contributors (setup, testing, architecture) - CLAUDE.md: Reference @CONTRIBUTING.md, keep Claude-specific tips Changes: - Move development setup, testing, release process to CONTRIBUTING.md - Add prerequisites and getting started sections - Keep `-n auto` testing tip and timeline component note in CLAUDE.md - Add date filtering examples and output files to CLAUDE.md 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
📝 WalkthroughWalkthroughThis PR refactors the rendering pipeline to use a content-centric dispatch pattern. All format_* and title_* methods now accept both the content object and a TemplateMessage parameter. Removes dedup infrastructure (DedupNoticeMessage, raw_text_content fields), lenient tool parsing, and refactors image export. Updates documentation with architecture overview. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
CONTRIBUTING.md (3)
61-61: Consider simplifying "CLI interface" to "CLI".The term "CLI" already stands for "Command Line Interface," making "interface" redundant.
🔎 Proposed fix
-- Click for CLI interface +- Click for CLI
20-54: Add language specifier to code block for better rendering.The file structure code block would benefit from a language identifier (e.g.,
textorplaintext) for consistent formatting across Markdown renderers.🔎 Proposed fix
-``` +```text claude_code_log/
195-205: Add language specifier to data flow diagram code block.The data flow diagram would benefit from a language identifier (e.g.,
textorplaintext) for consistent formatting.🔎 Proposed fix
-``` +```text JSONL File
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
CLAUDE.mdCONTRIBUTING.mdREADME.mdclaude_code_log/factories/assistant_factory.pyclaude_code_log/factories/tool_factory.pyclaude_code_log/factories/transcript_factory.pyclaude_code_log/factories/user_factory.pyclaude_code_log/html/__init__.pyclaude_code_log/html/assistant_formatters.pyclaude_code_log/html/renderer.pyclaude_code_log/html/system_formatters.pyclaude_code_log/html/tool_formatters.pyclaude_code_log/html/user_formatters.pyclaude_code_log/html/utils.pyclaude_code_log/image_export.pyclaude_code_log/markdown/renderer.pyclaude_code_log/models.pyclaude_code_log/renderer.pyclaude_code_log/renderer_timings.pydev-docs/MESSAGE_REFACTORING.mddev-docs/MESSAGE_REFACTORING2.mddev-docs/RENAME_CONTENT_TO_MESSAGE.mddev-docs/TEMPLATE_MESSAGE_CHILDREN.mddev-docs/TEMPLATE_MESSAGE_REFACTORING.mddev-docs/messages.mddev-docs/rendering-architecture.mddev-docs/rendering-next.mdtest/__snapshots__/test_snapshot_html.ambrtest/__snapshots__/test_snapshot_markdown.ambrtest/test_image_export.pytest/test_sidechain_agents.pytest/test_todowrite_rendering.pytest/test_tool_result_image_rendering.py
💤 Files with no reviewable changes (7)
- dev-docs/TEMPLATE_MESSAGE_CHILDREN.md
- dev-docs/MESSAGE_REFACTORING2.md
- claude_code_log/html/system_formatters.py
- dev-docs/MESSAGE_REFACTORING.md
- dev-docs/RENAME_CONTENT_TO_MESSAGE.md
- dev-docs/TEMPLATE_MESSAGE_REFACTORING.md
- claude_code_log/html/utils.py
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Use ruff for code formatting and linting, with ruff check --fix for automatic fixes
Use pyright and mypy for type checking in Python code
Target Python 3.10+ with support for modern Python features and type hints
Files:
claude_code_log/factories/assistant_factory.pyclaude_code_log/html/user_formatters.pyclaude_code_log/html/tool_formatters.pyclaude_code_log/image_export.pyclaude_code_log/renderer_timings.pyclaude_code_log/html/assistant_formatters.pytest/test_image_export.pytest/test_tool_result_image_rendering.pytest/test_sidechain_agents.pyclaude_code_log/factories/user_factory.pyclaude_code_log/html/__init__.pyclaude_code_log/factories/transcript_factory.pyclaude_code_log/models.pyclaude_code_log/renderer.pyclaude_code_log/factories/tool_factory.pytest/test_todowrite_rendering.pyclaude_code_log/html/renderer.pyclaude_code_log/markdown/renderer.py
claude_code_log/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use dateparser for natural language date parsing to support date range filtering with expressions like 'today', 'yesterday', 'last week', and relative dates
Files:
claude_code_log/image_export.pyclaude_code_log/renderer_timings.pyclaude_code_log/models.pyclaude_code_log/renderer.py
claude_code_log/renderer_timings.py
📄 CodeRabbit inference engine (CLAUDE.md)
Implement performance timing instrumentation via the CLAUDE_CODE_LOG_DEBUG_TIMING environment variable to identify performance bottlenecks in rendering phases, with detailed timing for initialization, deduplication, session summary processing, main message loop, and template rendering
Files:
claude_code_log/renderer_timings.py
test/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Organize tests into categories with pytest markers to avoid async event loop conflicts: unit tests (no mark), TUI tests (@pytest.mark.tui), browser tests (@pytest.mark.browser), and snapshot tests
Files:
test/test_image_export.pytest/test_tool_result_image_rendering.pytest/test_sidechain_agents.pytest/test_todowrite_rendering.py
claude_code_log/models.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use Pydantic models for parsing and validating transcript JSON data, including TranscriptEntry (union of UserTranscriptEntry, AssistantTranscriptEntry, SummaryTranscriptEntry), UsageInfo, and ContentItem
Files:
claude_code_log/models.py
claude_code_log/renderer.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use mistune for quick Markdown rendering with syntax highlighting support in server-side template rendering
Files:
claude_code_log/renderer.py
🧠 Learnings (11)
📚 Learning: 2025-11-30T17:16:32.495Z
Learnt from: CR
Repo: daaain/claude-code-log PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-30T17:16:32.495Z
Learning: Applies to claude_code_log/cli.py : Use Click for CLI interface and argument parsing in Python CLI files
Applied to files:
CONTRIBUTING.mdREADME.mdCLAUDE.md
📚 Learning: 2025-11-30T17:16:32.495Z
Learnt from: CR
Repo: daaain/claude-code-log PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-30T17:16:32.495Z
Learning: Applies to claude_code_log/templates/**/*.html : Use Jinja2 templates for HTML generation, including session navigation with table of contents, message rendering with different content types, and token display for individual messages and session totals
Applied to files:
dev-docs/rendering-architecture.mdREADME.mdCLAUDE.mddev-docs/messages.mdtest/test_todowrite_rendering.pyclaude_code_log/html/renderer.py
📚 Learning: 2025-11-30T17:16:32.494Z
Learnt from: CR
Repo: daaain/claude-code-log PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-30T17:16:32.494Z
Learning: When adding new message types or modifying CSS class generation in renderer.py, ensure the timeline's message type detection logic in the JavaScript timeline component (timeline.html) is updated accordingly to maintain feature parity
Applied to files:
dev-docs/rendering-architecture.mdCLAUDE.mdclaude_code_log/renderer.pydev-docs/messages.mdclaude_code_log/html/renderer.pyclaude_code_log/markdown/renderer.py
📚 Learning: 2025-11-09T22:35:33.367Z
Learnt from: cboos
Repo: daaain/claude-code-log PR: 42
File: claude_code_log/templates/transcript.html:91-98
Timestamp: 2025-11-09T22:35:33.367Z
Learning: In the claude-code-log fold UI (claude_code_log/templates/transcript.html), the fold button tooltips describe the ACTION on click, not the current state. Button 1 (fold-one) when showing ▼ will "Fold (all levels)" because hiding immediate children transitively hides all descendants. Button 2 (fold-all) when showing ▼▼ will "Fold (to 1st level)" because it keeps immediate children visible while hiding deeper descendants. See dev-docs/FOLD_STATE_DIAGRAM.md for the complete state machine.
Applied to files:
dev-docs/rendering-architecture.md
📚 Learning: 2025-11-30T17:16:32.495Z
Learnt from: CR
Repo: daaain/claude-code-log PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-30T17:16:32.495Z
Learning: Structure the project with separate modules for parser.py (data extraction), renderer.py (HTML generation), converter.py (high-level orchestration), cli.py (CLI interface), models.py (Pydantic data structures), tui.py (Textual TUI), and cache.py (cache management)
Applied to files:
README.mdCLAUDE.mdclaude_code_log/html/renderer.py
📚 Learning: 2025-11-30T17:16:32.495Z
Learnt from: CR
Repo: daaain/claude-code-log PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-30T17:16:32.495Z
Learning: Applies to claude_code_log/models.py : Use Pydantic models for parsing and validating transcript JSON data, including TranscriptEntry (union of UserTranscriptEntry, AssistantTranscriptEntry, SummaryTranscriptEntry), UsageInfo, and ContentItem
Applied to files:
README.mdclaude_code_log/factories/assistant_factory.pyCLAUDE.mdclaude_code_log/factories/transcript_factory.pyclaude_code_log/models.py
📚 Learning: 2025-11-30T17:16:32.495Z
Learnt from: CR
Repo: daaain/claude-code-log PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-30T17:16:32.495Z
Learning: Applies to claude_code_log/renderer_timings.py : Implement performance timing instrumentation via the CLAUDE_CODE_LOG_DEBUG_TIMING environment variable to identify performance bottlenecks in rendering phases, with detailed timing for initialization, deduplication, session summary processing, main message loop, and template rendering
Applied to files:
claude_code_log/renderer_timings.pyCLAUDE.mdclaude_code_log/renderer.pyclaude_code_log/html/renderer.py
📚 Learning: 2025-11-30T17:16:32.495Z
Learnt from: CR
Repo: daaain/claude-code-log PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-30T17:16:32.495Z
Learning: Applies to claude_code_log/tui.py : Use Textual for implementing the interactive Terminal User Interface (TUI) in Python
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-30T17:16:32.495Z
Learnt from: CR
Repo: daaain/claude-code-log PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-30T17:16:32.495Z
Learning: Applies to claude_code_log/renderer.py : Use mistune for quick Markdown rendering with syntax highlighting support in server-side template rendering
Applied to files:
CLAUDE.mdclaude_code_log/html/renderer.pyclaude_code_log/markdown/renderer.py
📚 Learning: 2025-12-09T23:52:47.578Z
Learnt from: daaain
Repo: daaain/claude-code-log PR: 59
File: test/test_cache.py:135-165
Timestamp: 2025-12-09T23:52:47.578Z
Learning: SQLite supports NULLS FIRST and NULLS LAST in ORDER BY since v3.30.0 (Oct 2019). Do not flag SQL that uses these clauses as an error when reviewing Python tests or code that interacts with SQLite. If reviewing SQL strings, verify the target SQLite version supports NULLS FIRST/LAST and ensure the syntax is used correctly for the database in use.
Applied to files:
test/test_image_export.pytest/test_tool_result_image_rendering.pytest/test_sidechain_agents.pytest/test_todowrite_rendering.py
📚 Learning: 2025-11-30T23:24:07.840Z
Learnt from: cboos
Repo: daaain/claude-code-log PR: 54
File: claude_code_log/renderer.py:2912-2945
Timestamp: 2025-11-30T23:24:07.840Z
Learning: In claude_code_log/renderer.py, the agentId field is currently only set on Task tool_result messages, not on tool_use messages, because the agentId is generated after the tool_use is logged. The _reorder_sidechain_template_messages function relies on this to avoid duplicate sidechain insertion.
Applied to files:
test/test_sidechain_agents.pytest/test_todowrite_rendering.py
🧬 Code graph analysis (10)
claude_code_log/html/user_formatters.py (2)
claude_code_log/models.py (3)
UserTextMessage(531-549)ImageContent(70-78)IdeNotificationContent(510-527)claude_code_log/html/assistant_formatters.py (1)
format_image_content(95-111)
claude_code_log/html/assistant_formatters.py (2)
claude_code_log/image_export.py (1)
export_image(15-64)claude_code_log/models.py (1)
ImageContent(70-78)
test/test_image_export.py (2)
claude_code_log/image_export.py (1)
export_image(15-64)claude_code_log/models.py (2)
ImageContent(70-78)ImageSource(62-67)
test/test_tool_result_image_rendering.py (2)
claude_code_log/html/assistant_formatters.py (1)
format_assistant_text_content(33-69)claude_code_log/models.py (6)
AssistantTextMessage(571-598)ImageContent(70-78)ImageSource(62-67)MessageMeta(239-268)TextContent(55-59)ToolResultContent(99-104)
claude_code_log/factories/user_factory.py (1)
claude_code_log/models.py (1)
UserTextMessage(531-549)
claude_code_log/factories/transcript_factory.py (1)
claude_code_log/models.py (1)
UsageInfo(81-89)
claude_code_log/renderer.py (2)
claude_code_log/models.py (12)
BashInputMessage(399-409)BashOutputMessage(413-424)CompactedSummaryMessage(432-449)HookSummaryMessage(344-356)SessionHeaderMessage(654-667)SlashCommandMessage(367-380)ThinkingMessage(602-624)ToolResultMessage(678-700)ToolUseMessage(704-717)UnknownMessage(632-643)AssistantTextMessage(571-598)UserTextMessage(531-549)claude_code_log/markdown/renderer.py (4)
title_SlashCommandMessage(329-333)title_UserTextMessage(305-310)title_AssistantTextMessage(658-669)title_ThinkingMessage(641-656)
test/test_todowrite_rendering.py (1)
claude_code_log/renderer.py (2)
TemplateMessage(138-291)format_ToolUseMessage(2175-2179)
claude_code_log/html/renderer.py (8)
claude_code_log/models.py (12)
ImageContent(70-78)SystemMessage(321-332)HookSummaryMessage(344-356)SessionHeaderMessage(654-667)UserTextMessage(531-549)UserSlashCommandMessage(469-481)SlashCommandMessage(367-380)AssistantTextMessage(571-598)ThinkingMessage(602-624)UnknownMessage(632-643)BashInput(727-734)TaskInput(800-808)claude_code_log/markdown/renderer.py (2)
_format_image(136-149)generate(735-770)claude_code_log/image_export.py (1)
export_image(15-64)claude_code_log/html/system_formatters.py (3)
format_system_content(21-32)format_hook_summary_content(35-78)format_session_header_content(81-91)claude_code_log/html/user_formatters.py (2)
format_user_text_model_content(196-235)format_user_slash_command_content(274-291)claude_code_log/html/assistant_formatters.py (3)
format_assistant_text_content(33-69)format_thinking_content(72-92)format_unknown_content(114-124)claude_code_log/html/tool_formatters.py (3)
format_bash_input(492-505)format_task_input(511-523)render_params_table(529-588)claude_code_log/renderer_timings.py (1)
report_timing_statistics(113-134)
claude_code_log/markdown/renderer.py (3)
claude_code_log/image_export.py (1)
export_image(15-64)claude_code_log/models.py (18)
SystemMessage(321-332)HookSummaryMessage(344-356)SessionHeaderMessage(654-667)UserTextMessage(531-549)SlashCommandMessage(367-380)CommandOutputMessage(384-395)BashInputMessage(399-409)BashOutputMessage(413-424)CompactedSummaryMessage(432-449)UserMemoryMessage(453-465)AssistantTextMessage(571-598)ThinkingMessage(602-624)UnknownMessage(632-643)BashInput(727-734)ToolUseContent(92-96)BashOutput(952-960)ToolUseMessage(704-717)TaskOutput(964-971)claude_code_log/utils.py (2)
generate_unified_diff(359-386)strip_error_tags(339-356)
🪛 LanguageTool
CONTRIBUTING.md
[style] ~61-~61: This phrase is redundant (‘I’ stands for ‘interface’). Use simply “CLI”.
Context: ... with uv package management - Click for CLI interface - Textual for Terminal User Interface -...
(ACRONYM_TAUTOLOGY)
🪛 markdownlint-cli2 (0.18.1)
CONTRIBUTING.md
20-20: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
195-195: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: test (ubuntu-latest, 3.14)
- GitHub Check: test (windows-latest, 3.13)
- GitHub Check: test (windows-latest, 3.14)
- GitHub Check: test (ubuntu-latest, 3.12)
- GitHub Check: test (windows-latest, 3.11)
- GitHub Check: test (ubuntu-latest, 3.13)
- GitHub Check: test (ubuntu-latest, 3.10)
- GitHub Check: test (windows-latest, 3.10)
- GitHub Check: test (windows-latest, 3.12)
- GitHub Check: test (ubuntu-latest, 3.11)
🔇 Additional comments (43)
claude_code_log/factories/user_factory.py (1)
467-467: LGTM! Constructor aligns with updated model.The removal of
raw_text_contentfrom the UserTextMessage constructor is correct and aligns with the PR objective to compute text content on-demand rather than storing it.README.md (1)
215-217: LGTM! Clean documentation consolidation.Moving detailed development setup and testing documentation to CONTRIBUTING.md improves maintainability and follows common open-source conventions.
claude_code_log/models.py (1)
723-725: LGTM! Clear documentation.The comment accurately describes the Tool Input Models section.
claude_code_log/factories/tool_factory.py (1)
83-85: LGTM! Documentation accurately reflects strict validation.The updated docstring clearly explains the strict validation approach and fallback behavior.
CLAUDE.md (2)
88-92: LGTM! Important maintenance note for timeline synchronization.The Timeline Component section provides crucial guidance for maintaining consistency between Python renderer CSS classes and JavaScript timeline detection logic.
94-99: LGTM! Architecture documentation improves discoverability.The cross-references to detailed architecture documentation help developers navigate the codebase.
claude_code_log/html/tool_formatters.py (1)
21-21: Optional import removal is correct.The removal of
Optionalfrom the typing imports is valid—there are no usages ofOptionalanywhere in the file after the deletion ofget_tool_summaryandformat_tool_use_title.claude_code_log/factories/transcript_factory.py (1)
78-82: No issues found. The simplifiednormalize_usage_infosignature correctly accepts onlydict[str, Any] | None. The single call site at line 193 passesmessage_copy["usage"], which is a dictionary value extracted from message data. No callers pass UsageInfo instances, confirming the signature change is safe and fully backward-compatible with all current usage.dev-docs/messages.md (1)
76-77: LGTM! Comprehensive documentation updates align with the rendering refactor.The documentation accurately reflects the major architectural changes:
- Content-centric access patterns (
has_markdownviacontent.has_markdown)- New message type (
UserMemoryMessage) with proper CSS class mapping- Expanded
ToolOutputunion to include all specialized output types- New metadata fields (
token_usage,gitBranch)- Updated references to new rendering architecture docs
All changes are consistent and well-structured.
Also applies to: 266-277, 405-405, 411-420, 480-480, 501-501, 725-725, 776-776, 887-888
test/test_sidechain_agents.py (1)
54-80: LGTM! Test correctly reflects the new deduplication behavior.The test has been properly updated to validate that sidechain assistant final messages are now dropped entirely when they duplicate the Task result, rather than being replaced with a forward link. The assertion correctly verifies the content appears only once.
test/__snapshots__/test_snapshot_markdown.ambr (1)
287-319: LGTM! Snapshot correctly reflects the new TodoWrite rendering format.The snapshot has been updated to show the new structured JSON representation of TodoWrite content, replacing the previous markdown list format. This change maintains all the necessary information (todos array with id, content, status, priority fields) and aligns with the content-centric rendering architecture.
claude_code_log/renderer_timings.py (1)
113-134: LGTM! Simplified timing statistics reporting.The function has been streamlined to focus on operation-level timing statistics, removing the previous per-message timing reporting. The docstring accurately reflects the updated signature and behavior. This simplification aligns with the broader refactoring goals.
test/test_todowrite_rendering.py (1)
18-18: LGTM! Tests correctly updated to use the new rendering signature.The tests have been properly updated to create
TemplateMessageinstances and pass them as the second parameter toformat_ToolUseMessage, aligning with the new content-centric rendering architecture where formatters receive both the content object and the TemplateMessage context.Also applies to: 234-237
test/test_tool_result_image_rendering.py (1)
1-12: LGTM! Comprehensive test coverage for assistant message images.The new test section provides thorough coverage for future image generation capabilities in assistant messages, including:
- Single image rendering
- Interleaved text and images
- Multiple images in one message
The tests follow consistent patterns with proper model construction using
MessageMeta, validate data URL generation, and check CSS classes. This forward-looking addition ensures the rendering pipeline is ready for Claude's potential image generation features.Also applies to: 164-272
test/test_image_export.py (1)
1-88: LGTM! Excellent test coverage for image export functionality.This new test file provides comprehensive coverage of the
export_imagefunction across all supported modes:
- Placeholder mode: Correctly validates
Nonereturn- Embedded mode: Validates data URL generation
- Referenced mode: Tests both
output_dir=Nonecase and file creation with proper path generation- Unsupported mode: Validates graceful degradation
The tests are well-organized into separate classes by mode and validate both success and edge cases (e.g., missing
output_dir, different counter values).claude_code_log/factories/assistant_factory.py (1)
79-99: LGTM! Improved type safety with ThinkingContent parameter.The function signature has been strengthened to take
ThinkingContentdirectly instead of a genericContentItem, providing better type safety and eliminating the need for runtime type checking. The direct field access (thinking.thinking,thinking.signature) is clearer and more maintainable. This aligns well with the broader move toward typed, content-centric rendering.claude_code_log/html/user_formatters.py (2)
11-12: LGTM: Appropriate type imports.The addition of
CallableandOptionalimports supports the newimage_formatterparameter type annotation.
196-199: LGTM: Clean injection pattern for custom image formatting.The implementation correctly adds an optional callback parameter that:
- Preserves backward compatibility (defaults to
format_image_content)- Uses appropriate type hints (
Optional[Callable[[ImageContent], str]])- Follows a clear fallback pattern at line 221
- Maintains the existing late-import strategy to avoid circular dependencies
The docstring accurately documents the new parameter and its behavior. This change aligns well with the PR's goal of consolidating image export logic while allowing format-specific customization.
Also applies to: 207-207, 212-213, 221-221, 229-229
test/__snapshots__/test_snapshot_html.ambr (3)
9987-9987: Verify the title change aligns with UX expectations.The header changed from a descriptive "📝 Todo List" to the tool name "🛠️ TodoWrite". While this aligns with the dispatcher refactor to use consistent parameters, the generic tool name may be less user-friendly than the previous semantic title.
9995-10036: Verify that raw JSON rendering is the intended user experience.The rendering has shifted from a formatted todo list to raw JSON displayed in a collapsible
<details>block. While this aligns with the data-driven rendering architecture mentioned in the PR, displaying raw JSON is typically less user-friendly than a formatted presentation.Confirm whether:
- Custom formatters for TodoWrite should still render todos as a formatted list, or
- The raw parameter view is the intended final UX for this tool
10001-10001: Clarify the "broken_todo" test data.The first element in the todos array is the string
"broken_todo", which appears to be test data. This seems unusual as it's not an object like the other todo items. Verify this is intentional test data to validate error handling or mixed-type array behavior.Also applies to: 10007-10007
dev-docs/rendering-next.md (1)
1-153: Well-structured future work documentation.The document provides clear guidance on potential improvements while accurately documenting the completed signature refactoring. The code examples effectively illustrate both current patterns and proposed alternatives.
Minor note: Line 119 lists
AskUserQuestionOutputandExitPlanModeOutputas "Currently parsed" tool outputs, which aligns with the implementation in the codebase.dev-docs/rendering-architecture.md (1)
1-346: Comprehensive and well-organized architecture documentation.This document provides excellent coverage of the rendering pipeline with clear diagrams and code examples. The naming conventions table (Section 2) and the dispatch mechanism explanation (Section 7) are particularly helpful for understanding the content-centric approach.
claude_code_log/image_export.py (1)
20-64: Clean API design with explicit None returns.The updated return type
str | Nonecorrectly signals to callers that they must handle the placeholder/unsupported cases. The docstring clearly documents the return values for each mode.claude_code_log/html/__init__.py (1)
42-44: Appropriate cleanup of public API exports.The removal of
format_tool_use_title,get_tool_summary, andDedupNoticeMessagealigns with the PR's goal of consolidating the rendering architecture. Therender_params_tableremains as a generic fallback for unknown tool inputs.Also applies to: 125-127
claude_code_log/html/assistant_formatters.py (3)
24-25: Good type alias for image formatter callback.The
ImageFormattertype alias clearly defines the expected signature for custom image formatters, enabling proper type checking in callers.
55-59: Clean injection pattern for image formatting.The fallback
formatter = image_formatter or format_image_contentprovides backward compatibility while enabling custom image handling in renderers that need different export modes.
95-111: Correct handling of export_image return value.The
if src is None: return "[Image]"properly handles the placeholder and unsupported mode cases, ensuring graceful degradation when image embedding isn't available.claude_code_log/renderer.py (6)
2005-2021: Correct implementation of content-centric dispatch.The
_dispatch_format(obj, message)and_dispatch_title(obj, message)methods now consistently pass both the typed object and theTemplateMessagecontext to handlers, enabling type-safe field access and context lookups.
2023-2056: Properly updated entry points for dispatch.
format_contentandtitle_contentcorrectly passmessage.contentalong with themessageto the dispatch methods, maintaining the content-centric pattern throughout the call chain.
2064-2131: Consistent title method signatures.All
title_*methods now follow the(content: ContentType, _: TemplateMessage)pattern. The underscore convention for unused parameters is appropriate and maintains LSP compliance for overrides.
2134-2156: Tool dispatch methods correctly forward context.
title_ToolUseMessageandtitle_ToolResultMessageproperly call_dispatch_title(content.input/output, message)to forward both the typed input/output and the message context.
2175-2185: Tool content formatters dispatch correctly.
format_ToolUseMessageandformat_ToolResultMessageproperly delegate to_dispatch_format(content.input/output, message), completing the content-centric dispatch chain.
1311-1340: Improved sidechain duplicate detection.The refactored code now extracts text on-demand from
TaskOutput.resultandAssistantTextMessage.itemsfor dedup comparison, eliminating the need for storedraw_text_contentfields. The_normalize_for_deduphelper correctly handles agentId line stripping.claude_code_log/html/renderer.py (4)
155-175: Clean image export integration.The
_format_imagemethod correctly:
- Increments counter before export (ensuring unique filenames)
- Passes all required parameters to
export_image- Returns placeholder "[Image]" when
src is None
181-259: Consistent content formatter signatures.All format methods correctly implement the
(content: ContentType, _: TemplateMessage)signature. Theimage_formatter=self._format_imageinjection forUserTextMessageandAssistantTextMessageenables mode-aware image rendering.
265-349: Tool formatters follow consistent pattern.All tool input and output formatters use the new
(input/output: Type, _: TemplateMessage)signature, delegating to the existing formatter functions while maintaining the content-centric API.
475-551: Proper output_dir propagation for image export.The
generateandgenerate_sessionmethods correctly setself._output_dirand resetself._image_counterbefore rendering, ensuring referenced images are written to the correct location.claude_code_log/markdown/renderer.py (5)
141-149: Correct image export handling for Markdown.The
_format_imagemethod properly returns"[Image]"for placeholder mode andfor embedded/referenced modes, matching Markdown syntax expectations.
252-286: System formatters correctly access content fields.The formatters properly access
content.level,content.text,content.hook_errors, etc. directly from the typed content objects, enabling type-safe field access.
532-563: Smart Q&A pairing in AskUserQuestionOutput.The use of
message.pair_firstandself._ctx.get()to look up the pairedAskUserQuestionInputenables rendering questions with their options alongside answers. This is a good use of the message context parameter.
577-591: Appropriate special-casing for TodoWrite output.TodoWrite success messages are rendered as plain text rather than code-fenced, which is the correct behavior for user-facing status messages. The fallback to
strip_error_tagsand code fence for other tools maintains consistent handling.
641-669: Context-aware title generation.The
title_ThinkingMessageandtitle_AssistantTextMessagemethods properly useself._ctxfor pair lookups andmessage.is_first_in_pair/message.is_last_in_pairto generate appropriate titles, demonstrating effective use of the message context parameter.
|
I wanted to get rid of the various refactoring plans which are now mostly done, but still wanted to keep an overview of how things work now, hence I also wanted to increase the code coverage, and for that I tried to remove the code that was not covered when I thought it was no longer needed. Our test coverage is still not enough for cache.py, cli.py, and converter.py but those are precisely the areas I know the less about and I haven't touch (yet!). Not to mention that the cache.py should be overhauled anyway. |
Summary
Major cleanup and consolidation of the rendering system. Removes ~840 lines of dead code and obsolete documentation while adding comprehensive architecture docs.
Stats: 33 files changed, +1594 / -2432 lines (net -838 lines)
Key Changes
1. Dead Code Removal
2. Dispatcher Signature Refactoring
All
format_*andtitle_*methods now receive consistent(obj, message)parameters:This gives handlers access to both the typed object (for type-safe field access) and the full context (for paired message lookups, ancestry, etc.).
3. Unified Image Export
Consolidated image export logic into
image_export.pyshared by both HTML and Markdown renderers. Removes code duplication and ensures consistent behavior.4. Documentation Consolidation
Removed obsolete planning docs:
Added comprehensive architecture docs:
dev-docs/rendering-architecture.md- Data flow, factories, renderer hierarchydev-docs/rendering-next.md- Future work and completed refactorings5. User Documentation Reorganization
Test Plan
just testjust test-tuijust test-browseruv run pyright && uv run ty checkSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.