Draft
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughIntroduces a shallow rendering mode that filters transcripts to display only user and assistant messages, omitting tools, system, and thinking content. The feature is implemented as a CLI flag Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as CLI (cli.py)
participant Converter as Converter (converter.py)
participant Renderer as Renderer (renderer.py)
participant HtmlRenderer as HtmlRenderer (html/renderer.py)
participant Output as Rendered Output
User->>CLI: --shallow flag
CLI->>Converter: shallow=True parameter
Converter->>Renderer: get_renderer(shallow=True)
Renderer->>Renderer: renderer.shallow = True
Renderer-->>Converter: Renderer instance
Converter->>HtmlRenderer: instantiate with shallow=True
HtmlRenderer->>Renderer: generate_template_messages(shallow=True)
Renderer->>Renderer: _filter_shallow(messages)
Renderer->>Renderer: filter pre-render
Renderer-->>HtmlRenderer: filtered messages
HtmlRenderer->>HtmlRenderer: render template
Renderer->>Renderer: _filter_shallow_template_messages()
Renderer->>Renderer: filter post-render
Renderer-->>HtmlRenderer: filtered TemplateMessages
HtmlRenderer-->>Output: HTML (user/assistant only)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
test/test_shallow_mode.py (1)
561-639: Add a cache mode-toggle regression test (full → shallowandshallow → full).Current tests mostly generate from fresh outputs, so they won’t catch stale reuse when the same cached target is rendered with different modes.
Also applies to: 751-774
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/test_shallow_mode.py` around lines 561 - 639, Add a regression test that exercises cache-mode toggling by rendering the same output target twice with different modes: first generate a cached HTML/MD with --shallow and assert tool messages are omitted, then re-run the CLI for the same output path without --shallow (full) and assert tool messages are present (and vice versa: start with full then run --shallow) to ensure cached artifacts are not incorrectly reused across modes; implement this using the existing test helpers and CliRunner.invoke(main, ...) and reuse the same output_file path to force cache reuse, checking presence/absence of "class='message tool_use" / "class='message tool_result" (or tool name like "Bash") and exit_code == 0 for each invocation.claude_code_log/renderer.py (1)
527-544: Missing docstring forshallowparameter.The
shallowparameter was added to the function signature but not documented in the Args section of the docstring.📝 Add documentation for the shallow parameter
"""Generate root messages and session navigation from transcript messages. This is the format-neutral rendering step that produces data structures ready for template rendering by any format-specific renderer. Args: messages: List of transcript entries to process. + shallow: If True, filter to only user and assistant text messages, + excluding tools, thinking, system messages, and sidechains. Returns:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@claude_code_log/renderer.py` around lines 527 - 544, The docstring for generate_template_messages is missing documentation for the shallow parameter; update the Args section to include a one-line description of shallow (its type bool, default False) and what it controls (e.g., whether to perform a shallow rendering that skips deep parsing/child population or only generates top-level session headers), so readers understand its effect when calling generate_template_messages(messages, shallow=True/False).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@claude_code_log/converter.py`:
- Around line 701-702: Staleness checks and early exits are ignoring the new
shallow render mode, so add the shallow flag into the cache/staleness identity
used wherever staleness is computed for combined, paginated, and session
outputs: include shallow in the metadata written to and read from cache
files/marker files and incorporate it into whatever key/hash/compute_staleness
function is used by the early-exit logic (the code paths that currently accept
shallow: bool and return Path). Update the marker read/write and the
is_stale/check_staleness logic to compare the shallow value as well as
timestamps/inputs so switching shallow vs full on the same output path won't
return the wrong artifact.
In `@test/test_shallow_mode.py`:
- Around line 126-130: Rename the outdated references to _filter_compact to the
current function name _filter_shallow in the test header and docstring: update
the section comment and the TestFilterShallow class docstring so they mention
_filter_shallow (and any other lingering _filter_compact mentions) to accurately
reflect the tested symbol and intent.
---
Nitpick comments:
In `@claude_code_log/renderer.py`:
- Around line 527-544: The docstring for generate_template_messages is missing
documentation for the shallow parameter; update the Args section to include a
one-line description of shallow (its type bool, default False) and what it
controls (e.g., whether to perform a shallow rendering that skips deep
parsing/child population or only generates top-level session headers), so
readers understand its effect when calling generate_template_messages(messages,
shallow=True/False).
In `@test/test_shallow_mode.py`:
- Around line 561-639: Add a regression test that exercises cache-mode toggling
by rendering the same output target twice with different modes: first generate a
cached HTML/MD with --shallow and assert tool messages are omitted, then re-run
the CLI for the same output path without --shallow (full) and assert tool
messages are present (and vice versa: start with full then run --shallow) to
ensure cached artifacts are not incorrectly reused across modes; implement this
using the existing test helpers and CliRunner.invoke(main, ...) and reuse the
same output_file path to force cache reuse, checking presence/absence of
"class='message tool_use" / "class='message tool_result" (or tool name like
"Bash") and exit_code == 0 for each invocation.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
claude_code_log/cli.pyclaude_code_log/converter.pyclaude_code_log/html/renderer.pyclaude_code_log/markdown/renderer.pyclaude_code_log/renderer.pytest/test_shallow_mode.py
Comment on lines
+701
to
702
| shallow: bool = False, | ||
| ) -> Path: |
There was a problem hiding this comment.
Cache invalidation does not account for render mode (shallow vs full).
shallow is now threaded through generation, but staleness checks/early exits are still mode-agnostic. That can serve the wrong artifact when users switch modes on the same output path.
🛠️ Minimum mitigation direction
@@ def convert_jsonl_to(...):
- if (
+ if (
cache_manager is not None
and not cache_was_updated
and from_date is None
and to_date is None
+ and not shallow
):
...
@@
- should_regenerate = (
+ should_regenerate = shallow or (
is_stale
or renderer.is_outdated(output_path)
or from_date is not None
or to_date is not None
or not output_path.exists()
)
@@
- if format == "html" and cache_manager is not None:
+ if format == "html" and cache_manager is not None and not shallow:
cache_manager.update_html_cache(...)A complete fix should include render mode in staleness identity (cache metadata and/or file marker checks) for combined, paginated, and session outputs.
Also applies to: 921-922, 1026-1027, 1073-1074, 1126-1127, 1489-1490, 1525-1526, 1673-1674, 1830-1831
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@claude_code_log/converter.py` around lines 701 - 702, Staleness checks and
early exits are ignoring the new shallow render mode, so add the shallow flag
into the cache/staleness identity used wherever staleness is computed for
combined, paginated, and session outputs: include shallow in the metadata
written to and read from cache files/marker files and incorporate it into
whatever key/hash/compute_staleness function is used by the early-exit logic
(the code paths that currently accept shallow: bool and return Path). Update the
marker read/write and the is_stale/check_staleness logic to compare the shallow
value as well as timestamps/inputs so switching shallow vs full on the same
output path won't return the wrong artifact.
Comment on lines
+126
to
+130
| # -- Unit tests for _filter_compact ------------------------------------------ | ||
|
|
||
|
|
||
| class TestFilterShallow: | ||
| """Test the _filter_compact function directly on parsed TranscriptEntry lists.""" |
There was a problem hiding this comment.
Rename stale _filter_compact references to _filter_shallow.
The section header/docstring no longer match the tested symbol, which makes test intent harder to follow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/test_shallow_mode.py` around lines 126 - 130, Rename the outdated
references to _filter_compact to the current function name _filter_shallow in
the test header and docstring: update the section comment and the
TestFilterShallow class docstring so they mention _filter_shallow (and any other
lingering _filter_compact mentions) to accurately reflect the tested symbol and
intent.
5 tasks
Collaborator
Author
Collaborator
Author
|
After a little brainstorming session with ChatGPT: ✅ Final polished version Does that sound right? |
Owner
Yes, I think these sound great! Please put those comments as details in the CLI help too! |
Filters out tool use/result, thinking, and system messages early in the rendering pipeline via _filter_compact() in renderer.py. Threaded through CLI, converter, and both HTML/Markdown renderers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
27 tests covering unit (_filter_compact), integration (generate_template_messages), HTML/Markdown rendering, CLI flag, real project data, and bundled test data files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…-derived types Use isinstance checks on content classes instead of message_type strings, since SlashCommandMessage, CommandOutputMessage, and CompactedSummaryMessage all return "user" as their message_type. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add explicit ContentItem annotation and cast for narrowing the UserTranscriptEntry/AssistantTranscriptEntry union on .message assignment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add isinstance narrowing for TranscriptEntry union types before accessing .message and .isSidechain attributes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rename CLI flag, parameters, functions, constants, test file, and all references from "compact" to "shallow" to avoid confusion with the existing CompactedSummary message type and future compact styling mode. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
2 participants
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.
With
--shallow, we focus on the gist of the conversation, user prompts and assistant responses. All the rest is skipped from the output. Note that this PR is a temporary one, as I need that feature. The actual next steps I'd like to take are the implementation of the DAG #85 (well, conversation tree, more accurately), for which I have a pretty advanced branch already.Add
--shallowrendering optionSummary
--shallowCLI flag that renders only user and assistant text messages, stripping all tools, thinking, system messages, bash I/O, slash commands, compacted summaries, and sidechainsTranscriptEntry(drops non-user/assistant, strips tool/thinking content) and post-render onTemplateMessage(removes text-derived types like bash commands and slash commands usingisinstancechecks)Test plan
uv run pytest -n auto test/test_shallow_mode.py -v— 31 tests passuv run pyright— 0 errorsuv run ty check— 0 warningsclaude-code-log --shallow path/to/file.jsonlproduces clean user/assistant-only outputclaude-code-log --shallow --format md path/to/file.jsonlsame for MarkdownSummary by CodeRabbit
Release Notes
New Features
--shallowCLI flag to generate simplified output containing only user and assistant messages, excluding system messages, tool interactions, and thinking content.Tests