-
Notifications
You must be signed in to change notification settings - Fork 614
refactor: reorganize artifact processing logic and improve attribute parsing #927
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
WalkthroughRefactors block content parsing to a staged, data-driven parser that scans for tags and builds ProcessedPart arrays. Adds attribute parsing, artifact/tool lifecycle handling, and preserves fallback behavior for plain text. useBlockContent now derives processedContent via generatePart based on block content and status, tracking parser state and tool-call indices. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor UI
participant useBlockContent
participant Parser
participant State as ParserState
UI->>useBlockContent: render(block)
useBlockContent->>Parser: generatePart(block.content, block.status)
Parser->>State: init(currentPosition, currentToolCallIndex)
loop scan content
alt tag detected (thinking/artifact/tool_*)
Parser->>Parser: parseAttributes + content slice
alt tool_call
Parser->>State: set status=calling, name
else tool_response
Parser->>State: set status=response, attach output
else tool_call_end
Parser->>State: set status=end
else tool_call_error
Parser->>State: set status=error, attach error
end
Parser-->>useBlockContent: push ProcessedPart
else plain text
Parser-->>useBlockContent: push text part
end
end
alt no recognizable content
Parser-->>useBlockContent: single text fallback
end
useBlockContent-->>UI: { processedContent }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 (8)
src/renderer/src/composables/useArtifacts.ts (8)
26-26: Translate comments to English (repo guideline).Comments must be in English. Please update these and similar comments across the file.
Apply these examples:
-// 定义可接受的artifact类型 +// Accepted artifact MIME types-// 辅助函数:解析标签属性 +// Helper: parse tag attributes-// antThinking 标签 (闭合) +// antThinking tag (closed)Also applies to: 46-47, 64-76
240-246: Preserve original whitespace in text parts.Trimming alters formatting. Keep original text but still skip pure-whitespace segments.
Apply this diff:
- const text = content.substring(currentPosition, earliestMatch.index).trim() - if (text) { + const text = content.substring(currentPosition, earliestMatch.index) + if (text.trim()) { parts.push({ type: 'text', content: text }) }- const remainingText = content.substring(currentPosition).trim() - if (remainingText) { + const remainingText = content.substring(currentPosition) + if (remainingText.trim()) { parts.push({ type: 'text', content: remainingText }) }Also applies to: 452-458
223-225: Avoid re-instantiating regex every iteration.These are static patterns without global/y flags, so lastIndex issues don’t apply. Reuse the pre-declared regex to reduce allocations and noise from static analysis.
Apply this diff:
- const regex = new RegExp(pattern.regex) + const regex = pattern.regex as RegExp
255-256: Guard tag end lookup to avoid -1 results.If '>' is missing, currentPosition can regress or stall. Ensure forward progress.
Apply this diff:
- const tagEndIndex = content.indexOf('>', earliestMatch.index) + 1 + const end = content.indexOf('>', earliestMatch.index) + const tagEndIndex = end === -1 ? content.length : end + 1- const tagEndIndex = content.indexOf('>', earliestMatch.index) + 1 + const end = content.indexOf('>', earliestMatch.index) + const tagEndIndex = end === -1 ? content.length : end + 1- currentPosition = content.indexOf('>', earliestMatch.index) + 1 + { + const end = content.indexOf('>', earliestMatch.index) + currentPosition = end === -1 ? content.length : end + 1 + }- currentPosition = content.indexOf('>', earliestMatch.index) + 1 + { + const end = content.indexOf('>', earliestMatch.index) + currentPosition = end === -1 ? content.length : end + 1 + }Also applies to: 291-291, 367-368, 405-406
336-339: Reset currentToolCallIndex after terminal states.Prevents later tool events from mutating a completed/error call.
Apply this diff:
parts[currentToolCallIndex].loading = false parts[currentToolCallIndex].tool_call!.status = 'end' + currentToolCallIndex = -1parts[currentToolCallIndex].loading = false parts[currentToolCallIndex].tool_call!.status = 'error' + currentToolCallIndex = -1Also applies to: 374-377
432-445: Simplify pointer advancement for closed tags.Use the matched length; no need to re-search for closing tags.
Apply this diff:
- } else { - // 闭合标签,移动到结束标签之后 - const fullTagLength = - pattern.name === 'thinking' - ? match[0].length - : content - .substring(earliestMatch.index) - .indexOf( - '</ant' + pattern.name.charAt(0).toUpperCase() + pattern.name.slice(1) + '>' - ) + - ('</ant' + pattern.name.charAt(0).toUpperCase() + pattern.name.slice(1) + '>') - .length - - currentPosition = earliestMatch.index + fullTagLength - } + } else { + // Closed tag: jump past the matched segment + currentPosition = earliestMatch.index + match[0].length + }
52-57: Make attribute parsing more robust.Support hyphenated/colon names and other valid attribute identifiers.
Apply this diff:
- const attributeRegex = /(\w+)="([^"]*)"/g + const attributeRegex = /([^\s=]+)="([^"]*)"/g
3-24: Prefer type alias over interface (repo guideline).Switch ProcessedPart to a type alias for consistency with code style.
Example:
export type ProcessedPart = { type: 'text' | 'thinking' | 'artifact' | 'tool_call' content: string loading?: boolean artifact?: { /* ... */ } tool_call?: { /* ... */ } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/renderer/src/composables/useArtifacts.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
**/*.{js,jsx,ts,tsx}: Use OxLint for JS/TS code; pre-commit hooks run lint-staged and typecheck
Use camelCase for variables and functions
Use PascalCase for types and classes
Use SCREAMING_SNAKE_CASE for constants
Files:
src/renderer/src/composables/useArtifacts.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/renderer/src/composables/useArtifacts.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪(如适用)、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/renderer/src/composables/useArtifacts.ts
src/renderer/src/**/*
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
src/renderer/src/**/*: All user-facing strings must use i18n keys (avoid hardcoded user-visible text in code)
Use the 'vue-i18n' framework for all internationalization in the renderer
Ensure all user-visible text in the renderer uses the translation system
Files:
src/renderer/src/composables/useArtifacts.ts
src/renderer/**/*.{vue,ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
渲染进程代码放在
src/renderer
Files:
src/renderer/src/composables/useArtifacts.ts
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/vue-best-practices.mdc)
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}: Use the Composition API for better code organization and reusability
Implement proper state management with Pinia
Utilize Vue Router for navigation and route management
Leverage Vue's built-in reactivity system for efficient data handling
Files:
src/renderer/src/composables/useArtifacts.ts
src/renderer/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,tsx,vue}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Use TypeScript for all code; prefer types over interfaces.
Avoid enums; use const objects instead.
Use arrow functions for methods and computed properties.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Files:
src/renderer/src/composables/useArtifacts.ts
src/renderer/**/*.{vue,ts}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
Implement lazy loading for routes and components.
Files:
src/renderer/src/composables/useArtifacts.ts
src/renderer/**/*.{ts,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,vue}: Use useFetch and useAsyncData for data fetching.
Implement SEO best practices using Nuxt's useHead and useSeoMeta.Use Pinia for frontend state management (do not introduce alternative state libraries)
Files:
src/renderer/src/composables/useArtifacts.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/renderer/src/composables/useArtifacts.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Files:
src/renderer/src/composables/useArtifacts.ts
src/renderer/src/**
📄 CodeRabbit inference engine (AGENTS.md)
Put application code for the Vue app under src/renderer/src (components, stores, views, i18n, lib)
Files:
src/renderer/src/composables/useArtifacts.ts
src/renderer/src/**/*.{vue,ts}
📄 CodeRabbit inference engine (AGENTS.md)
All user-facing strings in the renderer must use vue-i18n keys defined in src/renderer/src/i18n
Files:
src/renderer/src/composables/useArtifacts.ts
**/*.{js,jsx,ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting: single quotes, no semicolons, max width 100
Files:
src/renderer/src/composables/useArtifacts.ts
🧬 Code graph analysis (1)
src/renderer/src/composables/useArtifacts.ts (1)
src/shared/chat.d.ts (1)
AssistantMessageBlock(84-137)
🪛 ast-grep (0.39.5)
src/renderer/src/composables/useArtifacts.ts
[warning] 223-223: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(pattern.regex)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 260-260: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(<${tagName}(?:\\s+([^>]*))?>)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ 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). (1)
- GitHub Check: build-check (x64)
🔇 Additional comments (3)
src/renderer/src/composables/useArtifacts.ts (3)
35-44: Good refactor: derived processedContent with clear fallback.The computed wrapper and fallback to a single text part look clean. Please confirm that skipping tool-related tag detection while status === 'loading' (Lines 209-221) matches the intended UX for streaming.
223-223: Static analysis note: variable-based RegExp is safe here.Patterns stem from hardcoded lists, not user input; ReDoS risk is negligible. Still, the refactor to reuse pre-declared regex (above) reduces the warning surface.
If any part of content/tag names becomes user-supplied in future, revisit with stricter tokenization (lexer) instead of regex scans.
Also applies to: 260-262
409-413: Incorrect — no hard-coded "Maximum tool calls reached"; localize the <maximum_tool_calls_reached> tag insteadNo literal "Maximum tool calls reached" exists in src/renderer/src/composables/useArtifacts.ts; generatePart already matches a <maximum_tool_calls_reached> tag. Add an i18n key (e.g. chat.maximumToolCallsReached) to src/renderer/src/i18n/en-US and call t('chat.maximumToolCallsReached') when converting that tag into a text part (either inside generatePart in useArtifacts.ts or at the renderer layer).
Likely an incorrect or invalid review comment.
* fix: katex style (#926) * refactor: reorganize artifact processing logic and improve attribute parsing (#927) * fix(thinkingBlock): pre style (#930) * refactor: remove DOMPurify dependency and replace MessageBlockThink `v-html` with vue-markdown-renderer (#931) * refactor: remove DOMPurify dependency and replace MessageBlockThink `v-html` with vue-markdown-renderer * chore: update * feat: add Qwen3-VL and Qwen3-Max 0924 (#932) * refactor: update vue-renderer-markdown to 0.0.54-beta.6 and remove Re… (#934) * refactor: update vue-renderer-markdown to 0.0.54-beta.6 and remove ReferenceNode component * chore: format * chore: version 0.3.7 --------- Co-authored-by: Simon He <57086651+Simon-He95@users.noreply.github.com> Co-authored-by: yyhhyyyyyy <yyhhyyyyyy8@gmail.com>
Pull Request Description
Is your feature request related to a problem? Please describe.
A clear and concise description of what the problem is.
*For example: I'm always frustrated when [...] *
Describe the solution you'd like
A clear and concise description of what you want to happen.
UI/UX changes for Desktop Application
If this PR introduces UI/UX changes, please describe them in detail.
Platform Compatibility Notes
If this PR has specific platform compatibility considerations (Windows, macOS, Linux), please describe them here.
Additional context
Add any other context about the pull request here.
Pull Request Description (中文)
你的功能请求是否与某个问题有关?请描述一下。
请对问题进行清晰扼要的描述。
*例如:我增加了 [...] 的功能 *
请描述你希望的解决方案
请对你希望实现的效果进行清晰扼要的描述。
桌面应用程序的 UI/UX 更改
如果此 PR 引入了 UI/UX 更改,请详细描述它们。
平台兼容性注意事项
如果此 PR 具有特定的平台兼容性考虑因素(Windows、macOS、Linux),请在此处描述。
附加背景
在此处添加关于此 Pull Request 的任何其他背景信息。
Summary by CodeRabbit
Bug Fixes
Refactor