-
Notifications
You must be signed in to change notification settings - Fork 55
Refactor content formatting to use dispatcher pattern #67
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,8 +18,10 @@ | |
| IdeNotificationContent, | ||
| IdeOpenedFile, | ||
| IdeSelection, | ||
| ImageContent, | ||
| SlashCommandContent, | ||
| UserMemoryContent, | ||
| UserSlashCommandContent, | ||
| UserTextContent, | ||
| ) | ||
| from .tool_formatters import render_params_table | ||
|
|
@@ -193,37 +195,37 @@ def format_user_text_model_content(content: UserTextContent) -> str: | |
| """Format UserTextContent model as HTML. | ||
|
|
||
| Handles user text with optional IDE notifications, compacted summaries, | ||
| and memory input markers. | ||
| memory input markers, and inline images. | ||
|
|
||
| When `items` is set, iterates through the content items preserving order: | ||
| - TextContent: Rendered as preformatted text | ||
| - ImageContent: Rendered as inline <img> tag with base64 data URL | ||
| - IdeNotificationContent: Rendered as IDE notification blocks | ||
|
|
||
| Falls back to legacy text-only behavior when `items` is None. | ||
|
|
||
| Args: | ||
| content: UserTextContent with text and optional flags/notifications | ||
| content: UserTextContent with text/items and optional flags/notifications | ||
|
|
||
| Returns: | ||
| HTML string combining IDE notifications and main text content | ||
| HTML string combining all content items | ||
| """ | ||
| parts: list[str] = [] | ||
| # Import here to avoid circular dependency | ||
| from .assistant_formatters import format_image_content | ||
|
|
||
| # Add IDE notifications first if present | ||
| if content.ide_notifications: | ||
| notifications = format_ide_notification_content(content.ide_notifications) | ||
| parts.extend(notifications) | ||
| parts: list[str] = [] | ||
|
|
||
| # Format main text content based on type | ||
| if content.is_compacted: | ||
| # Render compacted summaries as markdown | ||
| text_html = render_markdown_collapsible( | ||
| content.text, "compacted-summary", line_threshold=20 | ||
| ) | ||
| elif content.is_memory_input: | ||
| # Render memory input as markdown | ||
| text_html = render_markdown_collapsible( | ||
| content.text, "user-memory", line_threshold=20 | ||
| ) | ||
| else: | ||
| # Regular user text as preformatted | ||
| text_html = format_user_text_content(content.text) | ||
| for item in content.items: | ||
| if isinstance(item, IdeNotificationContent): | ||
| notifications = format_ide_notification_content(item) | ||
| parts.extend(notifications) | ||
| elif isinstance(item, ImageContent): | ||
| parts.append(format_image_content(item)) | ||
| else: # TextContent | ||
| # Regular user text as preformatted | ||
| if item.text.strip(): | ||
| parts.append(format_user_text_content(item.text)) | ||
|
|
||
| parts.append(text_html) | ||
| return "\n".join(parts) | ||
|
|
||
|
|
||
|
|
@@ -263,6 +265,26 @@ def format_user_memory_content(content: UserMemoryContent) -> str: | |
| return f"<pre>{escaped_text}</pre>" | ||
|
|
||
|
|
||
| def format_user_slash_command_content(content: UserSlashCommandContent) -> str: | ||
| """Format slash command expanded prompt (isMeta) as HTML. | ||
|
|
||
| These are LLM-generated instruction text from slash commands, | ||
| rendered as collapsible markdown. | ||
|
|
||
| Args: | ||
| content: UserSlashCommandContent with markdown text | ||
|
|
||
| Returns: | ||
| HTML string with collapsible markdown rendering | ||
| """ | ||
| return render_markdown_collapsible( | ||
| content.text, | ||
| "slash-command", | ||
| line_threshold=30, | ||
| preview_line_count=10, | ||
| ) | ||
|
Comment on lines
+268
to
+285
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New function The function is defined but not included in the 🔎 Proposed fix __all__ = [
# Formatting functions
"format_slash_command_content",
"format_command_output_content",
"format_bash_input_content",
"format_bash_output_content",
"format_user_text_content",
"format_user_text_model_content",
"format_compacted_summary_content",
"format_user_memory_content",
"format_ide_notification_content",
+ "format_user_slash_command_content",
]🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| def _format_opened_file(opened_file: IdeOpenedFile) -> str: | ||
| """Format a single IDE opened file notification as HTML.""" | ||
| escaped_content = escape_html(opened_file.content) | ||
|
|
||
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.
Docstring claims fallback behavior that isn't implemented.
The docstring at line 38 states "Falls back to legacy text-only behavior when
itemsis None", but the code unconditionally iterates overcontent.itemswithout checking forNone. Ifitemscan beNone(as the docstring implies), this would raise aTypeError.Either:
itemsis always populated, orAdditionally, the
elsebranch at line 52 assumes any non-ImageContentitem isTextContent, which could causeAttributeErrorif an unexpected type is encountered. Consider adding an explicit type check.🔎 Proposed fix with explicit type handling
from ..models import ( AssistantTextContent, ImageContent, + TextContent, ThinkingContentModel, UnknownContent, )parts: list[str] = [] for item in content.items: if isinstance(item, ImageContent): parts.append(format_image_content(item)) - else: # TextContent + elif isinstance(item, TextContent): if item.text.strip(): text_html = render_markdown_collapsible( item.text, "assistant-text", line_threshold=line_threshold, preview_line_count=preview_line_count, ) parts.append(text_html) + # Other types are silently skipped return "\n".join(parts)🤖 Prompt for AI Agents