-
Notifications
You must be signed in to change notification settings - Fork 59
[AI Datamapper] Fix crash when LLM returns no mappings #1024
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
WalkthroughThe PR adds three defensive guards in the AI datamapper to detect empty LLM-generated mappings, emit a user-facing warning, stop DataMap processing, and return early to prevent further repair or generation steps. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (1)
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts (1)
710-721: LGTM! Inline mapping path is now protected from the crash.The guard correctly handles the empty mappings scenario for inline code generation, matching the pattern used in
generateMappingCodeCore.Consider extracting the warning message into a shared constant to eliminate duplication:
const NO_MAPPINGS_WARNING = `**No Relevant Mappings Generated**\n\n` + `The AI was unable to identify compatible field mappings between the input and output structures.\n\n` + `**Suggestions:**\n` + `- Check if input and output record structures are correct\n` + `- Try providing mapping hints or examples\n`;Then use it at both lines 351 and 718:
- const warningMessage = `**No Relevant Mappings Generated**\n\n` + - `The AI was unable to identify compatible field mappings between the input and output structures.\n\n` + - `**Suggestions:**\n` + - `- Check if input and output record structures are correct\n` + - `- Try providing mapping hints or examples\n`; - - eventHandler({ type: "content_block", content: warningMessage }); + eventHandler({ type: "content_block", content: NO_MAPPINGS_WARNING });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-11-26T06:37:07.886Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/inline/map3.bal.txt:6-8
Timestamp: 2025-11-26T06:37:07.886Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/ directories, BAL test data files (such as those in array-inner/inline/, array-root/inline/, basic/inline/, and their reusable counterparts) may intentionally contain type errors and other violations. These are comparison files used to test data mapper functionality and error handling, so such errors should not be flagged as issues.
Applied to files:
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts
📚 Learning: 2025-11-26T06:35:19.217Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:173-178
Timestamp: 2025-11-26T06:35:19.217Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the commented-out debugging block in the verifyFileContent function (lines 172-177 containing console.log, page.pause, and updateDataFileSync) is intentionally kept as a developer utility for updating test data files when needed. It should not be removed.
Applied to files:
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts
📚 Learning: 2025-11-26T06:34:09.752Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:128-134
Timestamp: 2025-11-26T06:34:09.752Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the goPrevViewBreadcrumb() method is only called when in a focused view, ensuring breadcrumbs are always present. No guard for empty breadcrumbs is needed.
Applied to files:
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts
📚 Learning: 2025-11-26T07:49:56.428Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:136-141
Timestamp: 2025-11-26T07:49:56.428Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the goPrevViewBackButton() method is only called when in a focused view, ensuring breadcrumbs are always present. No guard for empty breadcrumbs is needed.
Applied to files:
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts
📚 Learning: 2025-11-26T06:33:22.950Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:86-112
Timestamp: 2025-11-26T06:33:22.950Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, empty catch blocks around progress ring waitForSelector calls (state: 'attached' and 'detached') are intentional. The progress ring duration depends on machine performance and may appear very briefly, causing the wait to miss the event. The try-catch allows the test to proceed gracefully regardless of timing.
Applied to files:
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts
🔇 Additional comments (1)
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts (1)
343-354: LGTM! Crash prevention guard is correctly implemented.The early-return guard successfully prevents the operation timeout crash by handling empty mapping results gracefully before attempting to update source code. The warning message is user-friendly and provides actionable suggestions.
One minor note: verify that the temporary directory created at line 323 is cleaned up appropriately when this early return path is taken. Confirm this is handled by a cleanup mechanism elsewhere (such as a try/finally block or resource cleanup handler) to avoid potential resource leaks.
3dfd8e1 to
2172e21
Compare
Description
It resolves: wso2/product-ballerina-integrator#2031
Goals
Approach
UI Component Development
npm run storybookfrom the root directory to view current components.Manage Icons
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.