-
Notifications
You must be signed in to change notification settings - Fork 168
feat: implement structured DML-Usecase mapping artifact display #2752
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
- Create StructuredArtifact component to replace markdown display - Add UseCaseSection with collapsible functionality for use cases - Add DmlOperationCard to display individual DML operations with SQL syntax highlighting - Add ValidationResultBadge to show execution success/failure status - Add SqlCodeBlock component with proper syntax highlighting - Update ArtifactContainer to use structured display instead of markdown conversion - Remove unused components (Artifact.tsx, ExecutableDMLBlock, SeverityBadge, formatArtifactToMarkdown) - Remove unused rehype-raw dependency - Follow existing UI patterns and CSS variable conventions - Implement responsive design with mobile support Resolves #2741 Co-Authored-By: [email protected] <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Updates to Preview Branch (devin/1753776370-frontend-display-artifacts) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
…tend-display-artifacts
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Join our Discord community for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
- Restored Artifact.tsx to use simple markdown rendering with rehype-raw - Created formatArtifactToMarkdown utility to convert Artifact data to markdown - Removed unnecessary structured components (StructuredArtifact, ExecutableDMLBlock, etc.) - Updated ArtifactContainer to use markdown formatting function - Added rehype-raw dependency for HTML support in markdown - Cleaned up component file structure and removed unused CSS modules 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…tend-display-artifacts
…tend-display-artifacts
We have not been able to verify that it works through because the agent fails to generate the DML, but the logic on the front end side has been carefully examined and is Open. |
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.
Pull Request Overview
This PR implements a structured React component-based display for DML-Usecase mapping artifacts, replacing the previous markdown-based rendering system. The change provides enhanced UX through collapsible sections, SQL syntax highlighting, and clear validation indicators.
- Replaces markdown display with structured React components for better interactivity
- Adds collapsible use case sections and SQL syntax highlighting capabilities
- Implements validation result badges for DML execution status indicators
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
eslint-suppressions.json | Removes suppressions for deleted components |
formatArtifactToMarkdown.ts | Enhances markdown formatting with improved structure and DML operation display |
SeverityBadge/ | Removes old severity badge component (replaced by ValidationResultBadge) |
ExecutableDMLBlock/ | Removes old DML block component (replaced by new structured components) |
ArtifactContainer.tsx | Simplifies artifact rendering by removing markdown conversion comment |
Artifact.tsx | Removes custom components for severity badges and DML blocks from markdown renderer |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
return sections.join('\n') | ||
} | ||
|
||
export function formatArtifactToMarkdown(artifact: Artifact): string { |
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.
[nitpick] The function signature changed from arrow function to function declaration, which is inconsistent with the original implementation. Consider maintaining the same export pattern for consistency.
export function formatArtifactToMarkdown(artifact: Artifact): string { | |
export const formatArtifactToMarkdown = (artifact: Artifact): string => { |
Copilot uses AI. Check for mistakes.
const markdownContent = formatArtifactToMarkdown(artifact) | ||
|
||
return <Artifact doc={markdownContent} /> |
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.
The code still calls formatArtifactToMarkdown and renders with the old Artifact component, but according to the PR description, this should be replaced with the new StructuredArtifact component. This suggests the implementation is incomplete.
Copilot uses AI. Check for mistakes.
const markdownContent = formatArtifactToMarkdown(artifact) | ||
|
||
return <Artifact doc={markdownContent} /> |
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.
The component still renders the old Artifact component instead of the new StructuredArtifact component mentioned in the PR description. This indicates the new components were not properly integrated.
Copilot uses AI. Check for mistakes.
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.
The implementation seems to be good. 👍
code: sqlCode, | ||
} | ||
return <ExecutableDMLBlock dmlBlock={dmlBlock} /> | ||
} |
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.
I see 👀
Issue
Why is this change needed?
This change implements the frontend display enhancement for structured DML-Usecase mapping artifacts as specified in issue #2741. The current markdown-based display needs to be replaced with React components that provide better UX through collapsible sections, proper styling, syntax highlighting for SQL, and clear validation result indicators.
Summary of Changes
This PR completely replaces the markdown-based artifact display with a structured React component approach:
New Components Created
StructuredArtifact
- Main component that displays business requirements and categorized functional/non-functional requirementsUseCaseSection
- Collapsible sections for individual use cases with DML operationsDmlOperationCard
- Cards displaying SQL operations with syntax highlighting and execution logsValidationResultBadge
- Success/failure indicators for DML execution resultsSqlCodeBlock
- SQL syntax highlighting using existing patternsRemoved Components
Artifact.tsx
(markdown display component)ExecutableDMLBlock/
(old DML display)SeverityBadge/
(replaced with ValidationResultBadge)formatArtifactToMarkdown.ts
utilityrehype-raw
dependency (no longer needed)Key Features
@liam-hq/ui
Architecture Diagram
Human Review Checklist
Risk Assessment
Testing Notes
Link to Devin run: https://app.devin.ai/sessions/6d9d6e49248b4cb5a8a5dbb06bd7b038
Requested by: [email protected]