feat: render task notifications as styled cards#122
feat: render task notifications as styled cards#122Psypeal wants to merge 1 commit intomatt1398:mainfrom
Conversation
Parse <task-notification> XML blocks from user messages and render them as compact cards with status icon, command name, exit code, and output filename — instead of displaying raw XML. - Add parseTaskNotifications() to contentSanitizer.ts - Strip <task-notification> tags and trailing output instructions from display text - Render parsed notifications in UserChatGroup.tsx as styled cards Closes matt1398#121 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience by transforming raw, verbose XML task notifications into clear, concise, and visually appealing cards within the chat interface. This change improves readability and reduces visual clutter, making it easier for users to quickly grasp the status and details of background tasks without being exposed to technical XML markup. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a great feature to render task notifications as styled cards, which significantly improves user experience by replacing raw XML with a structured and readable format. The implementation is well-structured, with parsing logic cleanly separated in contentSanitizer.ts and rendering handled in the UserChatGroup.tsx component.
I've identified a few areas for improvement, mainly related to performance and code clarity. Specifically, I've suggested moving regular expression creations out of loops/render functions to avoid unnecessary re-compilation. I've also pointed out a small refinement in a type guard for better code conciseness.
Overall, this is a solid contribution. Addressing these minor points will make the code more efficient and maintainable.
| ? userGroup.message.content | ||
| : Array.isArray(userGroup.message.content) | ||
| ? userGroup.message.content | ||
| .filter((b): b is { type: 'text'; text: string } => b.type === 'text' && 'text' in b) |
There was a problem hiding this comment.
The type guard here is a bit verbose. If userGroup.message.content is an array of a discriminated union based on the type property, the check b.type === 'text' should be sufficient for TypeScript to infer that b has a text property. The additional 'text' in b check is likely redundant and can be removed for cleaner code.
| .filter((b): b is { type: 'text'; text: string } => b.type === 'text' && 'text' in b) | |
| .filter((b): b is { type: 'text'; text: string } => b.type === 'text') |
| const cmdMatch = /"([^"]+)"/.exec(notif.summary); | ||
| const cmdName = cmdMatch?.[1] ?? notif.summary; | ||
| // Extract exit code | ||
| const exitMatch = /\(exit code (\d+)\)/.exec(notif.summary); | ||
| const exitCode = exitMatch?.[1]; |
There was a problem hiding this comment.
For performance, these regular expressions should be defined as constants outside of the component render function. This prevents them from being re-created on every render and for every item in the taskNotifications array.
For example, you can define them at the top of the file:
const CMD_NAME_REGEX = /"([^"]+)"/;
const EXIT_CODE_REGEX = /\(exit code (\d+)\)/;Then use them inside the map callback:
const cmdMatch = CMD_NAME_REGEX.exec(notif.summary);
// ...
const exitMatch = EXIT_CODE_REGEX.exec(notif.summary);| export function parseTaskNotifications(content: string): TaskNotification[] { | ||
| const notifications: TaskNotification[] = []; | ||
| const pattern = /<task-notification>([\s\S]*?)<\/task-notification>/gi; | ||
| let match; | ||
|
|
||
| while ((match = pattern.exec(content)) !== null) { | ||
| const block = match[1]; | ||
| notifications.push({ | ||
| taskId: /<task-id>([^<]*)<\/task-id>/.exec(block)?.[1] ?? '', | ||
| status: /<status>([^<]*)<\/status>/.exec(block)?.[1] ?? '', | ||
| summary: /<summary>([\s\S]*?)<\/summary>/.exec(block)?.[1]?.trim() ?? '', | ||
| outputFile: /<output-file>([^<]*)<\/output-file>/.exec(block)?.[1] ?? '', | ||
| }); | ||
| } | ||
|
|
||
| return notifications; | ||
| } |
There was a problem hiding this comment.
For improved performance and code clarity, the regular expressions used for parsing should be defined once before the while loop. Re-creating them on each iteration is inefficient, especially if a message contains multiple task notifications.
export function parseTaskNotifications(content: string): TaskNotification[] {
const notifications: TaskNotification[] = [];
const pattern = /<task-notification>([\s\S]*?)<\/task-notification>/gi;
const taskIdRegex = /<task-id>([^<]*)<\/task-id>/;
const statusRegex = /<status>([^<]*)<\/status>/;
const summaryRegex = /<summary>([\s\S]*?)<\/summary>/;
const outputFileRegex = /<output-file>([^<]*)<\/output-file>/;
let match;
while ((match = pattern.exec(content)) !== null) {
const block = match[1];
notifications.push({
taskId: taskIdRegex.exec(block)?.[1] ?? '',
status: statusRegex.exec(block)?.[1] ?? '',
summary: summaryRegex.exec(block)?.[1]?.trim() ?? '',
outputFile: outputFileRegex.exec(block)?.[1] ?? '',
});
}
return notifications;
}
📝 WalkthroughWalkthroughThe PR adds support for parsing and rendering task notification XML blocks as styled UI cards. It introduces a TaskNotification interface and parseTaskNotifications utility function to extract notification data, and updates the chat renderer to display notifications with status icons, command names, exit codes, and output file information. Changes
Suggested labels
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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 Tip CodeRabbit can generate a title for your PR based on the changes with custom instructions.Set the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/renderer/components/chat/UserChatGroup.tsx (2)
521-521: Loose equality check for exitCode display.
exitCode != nulluses loose equality which catches bothnullandundefined. SinceexitCodeis derived from a regex match (exitMatch?.[1]), it will beundefinedwhen no match is found. The loose check works but consider using strict equality for clarity:{exitCode !== undefined && <span>exit {exitCode}</span>}This is a minor stylistic preference.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/chat/UserChatGroup.tsx` at line 521, The conditional rendering uses a loose equality check for exitCode; update the check in the UserChatGroup component to use strict comparison against undefined (since exitCode comes from exitMatch?.[1]) — replace the `exitCode != null` condition with `exitCode !== undefined` so the display logic is explicit about handling undefined values from `exitMatch?.[1]`.
499-531: Consider keyboard accessibility for task notification cards.The cards are currently non-interactive divs. If users might want to interact with them (e.g., click to view output file), consider making them focusable or actionable. For now, as purely informational displays, this is acceptable.
The truncation of
outputFilewith.split('/').pop()(line 525) works for Unix paths but would fail to extract the filename correctly on Windows paths with backslashes.♻️ Cross-platform filename extraction
- <span className="truncate">{notif.outputFile.split('/').pop()}</span> + <span className="truncate">{notif.outputFile.split(/[/\\]/).pop()}</span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/chat/UserChatGroup.tsx` around lines 499 - 531, The task card uses notif.outputFile.split('/').pop() which fails on Windows paths and the card is a non-interactive div; fix by extracting the filename with a cross-platform helper (e.g., add a getFileName(path) util used where notif.outputFile is rendered that splits on both '/' and '\' or uses a regex like /[\\/]/ to return the last segment), and if the card should be actionable make the outer div (the container rendering StatusIcon/FileText/cmdName) keyboard-accessible by converting it to a button/link or adding tabIndex and a role plus key handlers for Enter/Space (refer to the component rendering the card in UserChatGroup.tsx and the notif.outputFile/FileText usage).src/shared/utils/contentSanitizer.ts (2)
182-198: Regex uses global flag with exec() - ensure stateless usage.The
patternregex has thegflag, andexec()is called in a while loop. This works correctly here becausepatternis declared inside the function, so each call gets a fresh regex. Good implementation.However, the inner field extraction regexes (lines 190-193) don't use
[\s\S]*?consistently -<summary>uses it but<task-id>,<status>, and<output-file>use[^<]*. This means multi-line content in those fields would fail to match.If
task-id,status, oroutput-filecould contain newlines or XML-like content, consider using[\s\S]*?for consistency:♻️ Suggested consistency improvement
notifications.push({ - taskId: /<task-id>([^<]*)<\/task-id>/.exec(block)?.[1] ?? '', - status: /<status>([^<]*)<\/status>/.exec(block)?.[1] ?? '', + taskId: /<task-id>([\s\S]*?)<\/task-id>/.exec(block)?.[1]?.trim() ?? '', + status: /<status>([\s\S]*?)<\/status>/.exec(block)?.[1]?.trim() ?? '', summary: /<summary>([\s\S]*?)<\/summary>/.exec(block)?.[1]?.trim() ?? '', - outputFile: /<output-file>([^<]*)<\/output-file>/.exec(block)?.[1] ?? '', + outputFile: /<output-file>([\s\S]*?)<\/output-file>/.exec(block)?.[1]?.trim() ?? '', });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/utils/contentSanitizer.ts` around lines 182 - 198, In parseTaskNotifications, the inner field-extraction regexes for taskId, status, and outputFile use [^<]* which fails on multi-line content; update the three regexes (/\<task-id\>.../, /\<status\>.../, /\<output-file\>.../) to use non-greedy multi-line capture ([\s\S]*?) (and apply .trim() like summary where appropriate) so they correctly capture content with newlines or XML-like text while keeping the outer <task-notification> loop and fallbacks unchanged.
25-30: Pattern may not match all path formats.The pattern
[^\s]+matches any non-whitespace, but file paths could potentially contain spaces (especially on Windows or in quoted paths). Additionally, the leading optional space?might leave behind residual whitespace when the instruction appears mid-content.Consider whether edge cases with quoted or escaped paths need handling, though for most Unix-style paths this should work correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/utils/contentSanitizer.ts` around lines 25 - 30, TASK_OUTPUT_INSTRUCTION_PATTERN is too narrow: it uses ` ?` and `[^\s]+` which won't match paths containing spaces or quoted paths and may leave residual whitespace; update the regex used in TASK_OUTPUT_INSTRUCTION_PATTERN to trim any surrounding whitespace and accept quoted or unquoted paths that may contain spaces (for example: match optional leading whitespace, the literal "Read the output file to retrieve the result:", optional space, then either a quoted string "(?:\"[^\"]+\"|'[^']+') or an unquoted path that captures any non-newline characters until whitespace or line end), keep the global flag, and ensure the pattern removes the whole instruction including surrounding whitespace so no extra spaces remain after replacement.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/renderer/components/chat/UserChatGroup.tsx`:
- Line 521: The conditional rendering uses a loose equality check for exitCode;
update the check in the UserChatGroup component to use strict comparison against
undefined (since exitCode comes from exitMatch?.[1]) — replace the `exitCode !=
null` condition with `exitCode !== undefined` so the display logic is explicit
about handling undefined values from `exitMatch?.[1]`.
- Around line 499-531: The task card uses notif.outputFile.split('/').pop()
which fails on Windows paths and the card is a non-interactive div; fix by
extracting the filename with a cross-platform helper (e.g., add a
getFileName(path) util used where notif.outputFile is rendered that splits on
both '/' and '\' or uses a regex like /[\\/]/ to return the last segment), and
if the card should be actionable make the outer div (the container rendering
StatusIcon/FileText/cmdName) keyboard-accessible by converting it to a
button/link or adding tabIndex and a role plus key handlers for Enter/Space
(refer to the component rendering the card in UserChatGroup.tsx and the
notif.outputFile/FileText usage).
In `@src/shared/utils/contentSanitizer.ts`:
- Around line 182-198: In parseTaskNotifications, the inner field-extraction
regexes for taskId, status, and outputFile use [^<]* which fails on multi-line
content; update the three regexes (/\<task-id\>.../, /\<status\>.../,
/\<output-file\>.../) to use non-greedy multi-line capture ([\s\S]*?) (and apply
.trim() like summary where appropriate) so they correctly capture content with
newlines or XML-like text while keeping the outer <task-notification> loop and
fallbacks unchanged.
- Around line 25-30: TASK_OUTPUT_INSTRUCTION_PATTERN is too narrow: it uses ` ?`
and `[^\s]+` which won't match paths containing spaces or quoted paths and may
leave residual whitespace; update the regex used in
TASK_OUTPUT_INSTRUCTION_PATTERN to trim any surrounding whitespace and accept
quoted or unquoted paths that may contain spaces (for example: match optional
leading whitespace, the literal "Read the output file to retrieve the result:",
optional space, then either a quoted string "(?:\"[^\"]+\"|'[^']+') or an
unquoted path that captures any non-newline characters until whitespace or line
end), keep the global flag, and ensure the pattern removes the whole instruction
including surrounding whitespace so no extra spaces remain after replacement.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0e0c69a8-0ef5-49e6-9baf-cebc1d214412
📒 Files selected for processing (2)
src/renderer/components/chat/UserChatGroup.tsxsrc/shared/utils/contentSanitizer.ts
1 participant
Summary
Parse
<task-notification>XML blocks from user messages and render them as compact styled cards instead of raw XML.Closes #121
Before
Raw XML displayed verbatim in user message bubbles — confusing and noisy.
After
Compact cards showing: status icon (green check / red X), command name, exit code, and output filename.
Changes
contentSanitizer.ts: AddparseTaskNotifications()parser + strip<task-notification>tags and trailing "Read the output file..." instructions from display textUserChatGroup.tsx: Parse task notifications from original message content, render as styled cards below the user message bubbleTest plan
pnpm typecheck— no type errorspnpm lint:fix— no lint errorsSummary by CodeRabbit