[AI Datamapper] Fix mapping compilation error fields #904
Conversation
WalkthroughThe pull request refactors the data mapper repair flow by consolidating ad-hoc diagnostics handling into centralized helper functions ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/rpc-manager.ts (1)
665-686: Clarify success semantics when metadata is incomplete.The function returns
truewhendataMapperMetadataor required metadata fields are missing (lines 670, 678), but returnsfalseon catch errors (line 685). This creates inconsistent semantics:
- Missing metadata →
true(success)- Exception during refresh →
false(failure)Consider whether missing metadata should also return
falseor throw an error, to clearly distinguish between "success", "benign skip", and "failure".workspaces/ballerina/ballerina-extension/src/features/ai/dataMapping.ts (1)
1101-1108: Simplify attachments initialization.The attachments initialization can be streamlined:
Apply this diff:
- const mappingRequestPayload: MetadataWithAttachments = { - metadata: tempFileMetadata, - attachments: [] - }; - if (inlineMappingRequest.attachments.length > 0) { - mappingRequestPayload.attachments = inlineMappingRequest.attachments; - } + const mappingRequestPayload: MetadataWithAttachments = { + metadata: tempFileMetadata, + attachments: inlineMappingRequest.attachments || [] + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
workspaces/ballerina/ballerina-core/src/rpc-types/ai-panel/interfaces.ts(1 hunks)workspaces/ballerina/ballerina-extension/src/features/ai/dataMapping.ts(5 hunks)workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts(16 hunks)workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/types.ts(1 hunks)workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/rpc-manager.ts(1 hunks)workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/utils.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-10T15:05:11.309Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:224-242
Timestamp: 2025-11-10T15:05:11.309Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate VS Code extensions that are packaged and distributed independently, so they cannot share code via imports and must maintain their own implementations of common utilities.
Applied to files:
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts
📚 Learning: 2025-11-05T10:31:47.583Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 830
File: workspaces/ballerina/ballerina-extension/test/ai/post_proccess/post.test.ts:0-0
Timestamp: 2025-11-05T10:31:47.583Z
Learning: In the workspaces/ballerina/ballerina-extension project, the tsconfig.json uses "rootDirs": ["src", "test"], which allows test files to import from src using shorter relative paths. For example, from test/ai/post_proccess/, the import '../../stateMachine' correctly resolves to src/stateMachine.ts due to this configuration.
Applied to files:
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts
🧬 Code graph analysis (4)
workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/utils.ts (3)
workspaces/ballerina/ballerina-core/src/rpc-types/ai-panel/interfaces.ts (1)
Attachment(318-323)workspaces/ballerina/ballerina-core/src/interfaces/data-mapper.ts (2)
Mapping(104-116)DMModel(130-141)workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts (1)
DataMapperModelResponse(329-331)
workspaces/ballerina/ballerina-extension/src/features/ai/dataMapping.ts (4)
workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts (1)
AllDataMapperSourceRequest(316-318)workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/utils.ts (1)
generateMappingExpressionsFromModel(156-178)workspaces/ballerina/ballerina-core/src/interfaces/data-mapper.ts (1)
DMModel(130-141)workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/types.ts (1)
CodeRepairResult(56-59)
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts (9)
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/types.ts (1)
CodeRepairResult(56-59)workspaces/ballerina/ballerina-extension/src/features/ai/dataMapping.ts (6)
getCustomFunctionsContent(1359-1366)repairAndCheckDiagnostics(1036-1056)getFunctionDefinitionFromSyntaxTree(150-173)buildMappingFileArray(1551-1572)generateInlineMappingsSource(1062-1125)generateDataMapperModel(327-411)workspaces/ballerina/ballerina-extension/src/core/extended-language-client.ts (1)
ExtendedLangClient(503-1500)workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts (5)
SyntaxTree(504-509)AllDataMapperSourceRequest(316-318)DataMapperModelRequest(295-300)DataMapperModelResponse(329-331)DeleteMappingRequest(426-433)workspaces/ballerina/ballerina-extension/src/utils/modification.ts (1)
writeBallerinaFileDidOpenTemp(69-92)workspaces/ballerina/ballerina-core/src/interfaces/data-mapper.ts (2)
DMModel(130-141)Mapping(104-116)workspaces/ballerina/ballerina-extension/src/utils/source-utils.ts (1)
updateSourceCode(42-211)workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/utils.ts (1)
extractVariableDefinitionSource(301-328)workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/rpc-manager.ts (1)
openChatWindowWithCommand(701-703)
workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/rpc-manager.ts (2)
workspaces/ballerina/ballerina-extension/src/stateMachine.ts (2)
StateMachine(653-676)updateView(688-768)workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/utils.ts (1)
refreshDataMapper(407-419)
🔇 Additional comments (17)
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/types.ts (1)
56-59: LGTM!The
CodeRepairResultinterface is well-defined and clearly represents the result of code repair operations with separate content for main and custom functions files.workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/utils.ts (1)
156-178: LGTM!The addition of
CopilotEventHandlerfor progress tracking is well-implemented. The progress messages provide clear feedback to users during mapping generation.workspaces/ballerina/ballerina-extension/src/features/ai/dataMapping.ts (2)
475-512: LGTM!The
generateMappingsfunction correctly propagates theeventHandlerparameter and safely defaultsattachmentsto an empty array when not provided.
1320-1356: LGTM!The
repairCodeAndGetUpdatedContentfunction correctly returnsCodeRepairResultand properly handles both main content and custom functions content.workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts (13)
260-262: LGTM!The updated progress messages provide clear, user-friendly feedback during the mapping generation process.
366-391: LGTM! The file merging logic correctly handles same-file and different-file scenarios.The code properly merges main content with custom functions when they target the same file, and repairs them separately when they are different files. This ensures correct repair behavior in both cases.
393-422: LGTM! Critical diagnostic handling properly implements PR objectives.This code correctly implements the core feature of this PR: detecting and removing mapping fields that contain syntax errors after repair attempts. The flow is logical:
- Repair and check diagnostics for relevant files
- Handle BCE3032 check expression errors by adding
|errorto return types- Remove mappings with remaining diagnostics to prevent runtime failures
424-445: LGTM! Reading updated content after diagnostics ensures accuracy.Correctly reads the updated file contents after all repairs and diagnostic handling, ensuring that the generated source files and assistant response reflect the final, cleaned code.
457-478: LGTM! Assistant response properly displays attachments and override warnings.The response correctly:
- Lists attachment names when present
- Warns users when existing mappings will be overridden
- Uses updated content after diagnostic handling for accurate code blocks
655-726: LGTM! Syntax tree traversal utilities are well-implemented.The traversal functions correctly:
- Skip
parentandtypeDataproperties to avoid circular references- Handle arrays and nested objects properly
- Provide focused predicates for finding specific node types
728-765: Verify string replacement safety for return type modification.The
addErrorToReturnTypefunction usesreplaceAllon file content to add|errorto return types. While this works for the current use case, consider potential edge cases:
- Could
returnTypeSourceappear in comments or strings?- Could it match unintended occurrences outside the function signature?
The implementation should be safe if
returnTypeSourcefrom the syntax tree is unique, but consider using syntax tree-based modification (AST modification) for more robust editing if edge cases arise in the future.
849-924: LGTM! Core feature properly removes mappings with unresolved diagnostics.This function correctly implements the PR's main objective:
- Retrieves the updated data mapper model with mapping-level diagnostics
- Filters mappings that still have diagnostics after repairs
- Deletes each problematic mapping using the
deleteMappingAPI- Applies edits and re-checks diagnostics
This ensures that fields with syntax errors are removed, preventing runtime failures as intended.
982-1008: LGTM! Targeted error type addition for inline variables.The function correctly identifies the specific function containing an inline variable and adds
|errorto its return type only, rather than modifying all functions. This is more precise and avoids unnecessary changes.
1097-1153: LGTM! Inline mapping generation correctly implements diagnostic handling.The inline mapping flow properly mirrors the main mapping generation flow:
- Repairs and checks diagnostics
- Handles check expression errors
- Removes mappings with diagnostics
- Uses updated content for final output
1202-1236: LGTM! Context type generation properly validates and displays attachments.The function correctly validates that attachments are provided and generates a clear, informative assistant response showing which files were used for type generation.
1250-1296: LGTM! Text edit application correctly handles temp files.The function properly applies text edits to temporary files outside the VS Code workspace by:
- Sorting edits in reverse order to maintain line positions
- Handling both single-line and multi-line edits
- Using
writeBallerinaFileDidOpenTempto notify the language server
1298-1318: LGTM! AI panel refresh properly propagates updated metadata.The function correctly closes and reopens the AI panel with the generated data mapper model and metadata.
workspaces/ballerina/ballerina-core/src/rpc-types/ai-panel/interfaces.ts
Show resolved
Hide resolved
VellummyilumVinoth
changed the title
VellummyilumVinoth
force-pushed
the
remove-error-fields
branch
from
bfacc27 to
1ea8548
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts (1)
868-887: Consider adding error handling for model generation.The function correctly refreshes the AI panel with the generated data mapper model. However, if
generateDataMapperModelfails, the AI panel will be closed but not reopened, leaving the user without feedback.export async function openChatWindowWithCommand(): Promise<void> { + try { const langClient = StateMachine.langClient(); const context = StateMachine.context(); const model = await generateDataMapperModel({}, langClient, context); const { identifier, dataMapperMetadata } = context; // Automatically close and open AI mapping chat window with the generated model commands.executeCommand(CLOSE_AI_PANEL_COMMAND); commands.executeCommand(OPEN_AI_PANEL_COMMAND, { type: 'command-template', command: Command.DataMap, templateId: identifier ? TemplateId.MappingsForFunction : TemplateId.InlineMappings, ...(identifier && { params: { functionName: identifier } }), metadata: { ...dataMapperMetadata, mappingsModel: model.mappingsModel as DMModel } }); + } catch (error) { + console.error("Failed to generate data mapper model:", error); + // Reopen panel to allow user to retry + commands.executeCommand(OPEN_AI_PANEL_COMMAND); + throw error; + } }
🧹 Nitpick comments (5)
workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/rpc-manager.ts (1)
665-686: Consider adding error handling around the refreshDataMapper call.The conditional logic for refreshing the data mapper is well-structured with appropriate guard clauses. However, the
refreshDataMappercall on Line 681 could potentially throw an error that is not caught by the outer try-catch if it occurs during the execution. Consider wrapping this call in its own try-catch block to provide more specific error handling and logging.+ try { await refreshDataMapper(filePath, dataMapperMetadata.codeData, varName); + } catch (refreshError) { + console.error(">>> Failed to refresh data mapper", refreshError); + updateView(); + } return true;workspaces/ballerina/ballerina-extension/src/features/ai/dataMapping.ts (1)
1102-1108: Consider simplifying attachments initialization.The attachments handling is correct but could be more concise. Currently, it initializes an empty array and then conditionally replaces it.
Apply this diff for a simpler approach:
const mappingRequestPayload: MetadataWithAttachments = { metadata: tempFileMetadata, - attachments: [] + attachments: inlineMappingRequest.attachments || [] }; - if (inlineMappingRequest.attachments.length > 0) { - mappingRequestPayload.attachments = inlineMappingRequest.attachments; - }workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts (3)
890-952: Consider adding error handling for deleteMapping operations.The function correctly identifies and removes mappings with diagnostics. However, the
deleteMappingcalls (Lines 920-928) lack error handling. If a delete operation fails, the entire process could be interrupted without proper cleanup or user feedback.for (const mapping of mappingsWithDiagnostics) { + try { const deleteRequest: DeleteMappingRequest = { filePath: mainFilePath, codedata: updatedDataMapperMetadata.codedata, mapping: mapping, varName: inlineMappingsResult.allMappingsRequest.varName, targetField: variableName, }; const deleteResponse = await langClient.deleteMapping(deleteRequest); // Apply the text edits from the delete operation directly to temp files if (Object.keys(deleteResponse.textEdits).length > 0) { await applyTextEditsToTempFile(deleteResponse.textEdits, mainFilePath); await new Promise((resolve) => setTimeout(resolve, 100)); } + } catch (err) { + console.warn(`Failed to delete mapping for field ${mapping.output}:`, err); + // Continue with other mappings + } }
1060-1144: Consider adding error handling for deleteMapping operations.Similar to the inline version, this function lacks error handling around
deleteMappingcalls (Lines 1111-1127). Consider wrapping these operations in try-catch blocks to ensure robust error recovery.for (const mapping of mappingsWithDiagnostics) { + try { const deleteRequest: DeleteMappingRequest = { filePath: mainFilePath, codedata: updatedDataMapperMetadata.codedata, mapping: mapping, varName: allMappingsRequest.varName, targetField: targetFunctionName, }; const deleteResponse = await langClient.deleteMapping(deleteRequest); // Apply the text edits from the delete operation directly to temp files if (Object.keys(deleteResponse.textEdits).length > 0) { await applyTextEditsToTempFile(deleteResponse.textEdits, mainFilePath); await new Promise((resolve) => setTimeout(resolve, 100)); } + } catch (err) { + console.warn(`Failed to delete mapping for field ${mapping.output}:`, err); + // Continue with other mappings + } }
1147-1192: Consider adding bounds validation for text edit ranges.The function correctly applies text edits to temporary files without using VS Code workspace APIs. However, there's no validation to ensure edit ranges are within file bounds, which could cause runtime errors if diagnostics reference invalid line numbers.
// Apply each edit for (const edit of sortedEdits) { const startLine = edit.range.start.line; const startChar = edit.range.start.character; const endLine = edit.range.end.line; const endChar = edit.range.end.character; + // Validate bounds + if (startLine < 0 || startLine >= lines.length || endLine < 0 || endLine >= lines.length) { + console.warn(`Invalid edit range: ${startLine}-${endLine} for file with ${lines.length} lines`); + continue; + } + // Handle single line edit if (startLine === endLine) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
workspaces/ballerina/ballerina-core/src/rpc-types/ai-panel/interfaces.ts(1 hunks)workspaces/ballerina/ballerina-extension/src/features/ai/dataMapping.ts(5 hunks)workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts(16 hunks)workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/types.ts(1 hunks)workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/repair-utils.ts(1 hunks)workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/rpc-manager.ts(1 hunks)workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/utils.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- workspaces/ballerina/ballerina-core/src/rpc-types/ai-panel/interfaces.ts
- workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/utils.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-05T10:31:47.583Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 830
File: workspaces/ballerina/ballerina-extension/test/ai/post_proccess/post.test.ts:0-0
Timestamp: 2025-11-05T10:31:47.583Z
Learning: In the workspaces/ballerina/ballerina-extension project, the tsconfig.json uses "rootDirs": ["src", "test"], which allows test files to import from src using shorter relative paths. For example, from test/ai/post_proccess/, the import '../../stateMachine' correctly resolves to src/stateMachine.ts due to this configuration.
Applied to files:
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts
🧬 Code graph analysis (4)
workspaces/ballerina/ballerina-extension/src/features/ai/dataMapping.ts (4)
workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts (1)
AllDataMapperSourceRequest(316-318)workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/utils.ts (1)
generateMappingExpressionsFromModel(156-178)workspaces/ballerina/ballerina-core/src/interfaces/data-mapper.ts (1)
DMModel(130-141)workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/types.ts (1)
CodeRepairResult(56-59)
workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/rpc-manager.ts (2)
workspaces/ballerina/ballerina-extension/src/stateMachine.ts (2)
StateMachine(653-676)updateView(688-768)workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/utils.ts (1)
refreshDataMapper(407-419)
workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/repair-utils.ts (4)
workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts (2)
Diagnostics(148-151)SyntaxTree(504-509)workspaces/ballerina/ballerina-extension/src/features/ai/activator.ts (1)
langClient(33-33)workspaces/ballerina/ballerina-extension/src/core/extended-language-client.ts (1)
ExtendedLangClient(503-1500)workspaces/ballerina/ballerina-extension/src/utils/modification.ts (1)
writeBallerinaFileDidOpenTemp(69-92)
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts (8)
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/types.ts (1)
CodeRepairResult(56-59)workspaces/ballerina/ballerina-extension/src/features/ai/dataMapping.ts (6)
getCustomFunctionsContent(1359-1366)getFunctionDefinitionFromSyntaxTree(150-173)buildMappingFileArray(1551-1572)generateInlineMappingsSource(1062-1125)generateDataMapperModel(327-411)repairAndCheckDiagnostics(1036-1056)workspaces/ballerina/ballerina-extension/src/utils/source-utils.ts (1)
updateSourceCode(42-211)workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/utils.ts (1)
extractVariableDefinitionSource(301-328)workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts (4)
DataMapperModelRequest(295-300)DataMapperModelResponse(329-331)DeleteMappingRequest(426-433)AllDataMapperSourceRequest(316-318)workspaces/ballerina/ballerina-core/src/interfaces/data-mapper.ts (2)
DMModel(130-141)Mapping(104-116)workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/repair-utils.ts (1)
addCheckExpressionErrors(334-427)workspaces/ballerina/ballerina-extension/src/utils/modification.ts (1)
writeBallerinaFileDidOpenTemp(69-92)
🔇 Additional comments (13)
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/types.ts (1)
56-59: LGTM!The
CodeRepairResultinterface is well-defined with clear field names that accurately describe the repaired code content. The interface properly encapsulates the results of code repair operations.workspaces/ballerina/ballerina-extension/src/features/ai/dataMapping.ts (2)
476-486: LGTM! EventHandler integration is well-implemented.The
CopilotEventHandlerparameter is properly threaded through the mapping generation flow, enabling event-driven progress updates. The attachments fallback to an empty array is a good defensive practice.
1320-1356: LGTM! Return type change is properly implemented.The function now correctly returns
CodeRepairResultinstead of an anonymous object, improving type safety and code maintainability throughout the codebase.workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts (9)
259-260: LGTM! Improved progress messaging.The updated progress messages provide clearer feedback to users about the data mapping generation process.
361-390: LGTM! Proper handling of merged vs separate file repairs.The conditional logic correctly handles the case where the main file and custom functions file are the same by merging content before repair and clearing the custom functions content afterward.
393-413: Well-structured repair flow.The sequential calls to handle check expression errors and remove compilation error fields provide a clear, maintainable repair pathway. The separation of concerns makes the code easier to understand and test.
424-429: LGTM! Correctly uses updated content after all repair stages.The file array now reflects the final state after both LLM repairs and diagnostic-driven field removals, ensuring consistency between generated and displayed code.
441-461: LGTM! Enhanced user feedback with attachment information.The assistant response now clearly communicates which attachments were used and warns users about overriding existing mappings. Using
updatedCustomContentensures the displayed code matches what will be added to the project.
955-1011: LGTM! Properly handles BCE3032 errors for inline mappings.The function correctly identifies check expression errors, applies fixes, and updates the display code by extracting the variable definition. The logic parallels the non-inline version while handling inline-specific concerns.
1014-1057: LGTM! Well-structured check expression error handling.The function follows a clear pattern: detect BCE3032 diagnostics, apply fixes via code actions, and refresh file content. The logic is straightforward and aligns with the inline version for consistency.
820-844: LGTM! Improved attachment validation and messaging.Checking
attachments.length === 0is more explicit and correct than checking the attachments array directly. The enhanced assistant response with proper singular/plural handling improves the user experience.
697-794: LGTM! Inline flow changes mirror standard flow consistently.The inline mapping generation correctly applies the same repair and diagnostic handling patterns as the standard flow, maintaining consistency across both code paths while appropriately adapting for inline-specific requirements.
workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/repair-utils.ts (1)
376-382: Verify code action title matching against actual Ballerina LSP responses.The code uses a pattern-matching approach to find code actions for BCE3032 check expression errors, searching for titles containing "change", "return", and "error" (lines 376-382). While the language server does provide code actions for check expressions, the web search results mention actions like "Add 'check' error" and "Create variable and check error" but do not confirm the exact format of "Change return type to..." that your code assumes.
No test coverage was found for this function, and the pattern is more fragile than other code action matchers in the same file (which use exact matches or
startsWith). Before merging, manually verify that:
- The Ballerina language server actually provides code actions matching the pattern (containing all three keywords: "change", "return", "error")
- Test with a real BCE3032 diagnostic to confirm the code action is retrieved and applied correctly
VellummyilumVinoth
force-pushed
the
remove-error-fields
branch
from
1ea8548 to
b85066a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts (2)
1001-1058: Function mutatescodeRepairResultpassed by reference.The function modifies
codeRepairResult.finalContent(line 1034) andcodeRepairResult.customFunctionsContent(line 1049) as a side effect. While the values are re-read from disk anyway, this mutation pattern can be confusing since the caller passedcodeRepairResultbut the updated values come from the return object.Consider either:
- Documenting this mutation behavior with a comment
- Returning the updated content instead of mutating the parameter
Also, using
anyforlangClient(line 1003) loses type safety. If the actual type is complex, consider usingExtendedLangClientfrom the imports.async function handleInlineCheckExpressionErrorsAndRepair( - langClient: any, + langClient: ExtendedLangClient, projectRoot: string,
1060-1104: Consider stronger typing for parameters.Similar to the inline variant, this function uses
anyforlangClient(line 1062) andtempFileMetadata(line 1064). Using proper types would improve maintainability and catch errors at compile time.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
workspaces/ballerina/ballerina-extension/src/features/ai/dataMapping.ts(2 hunks)workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- workspaces/ballerina/ballerina-extension/src/features/ai/dataMapping.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.
📚 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
🧬 Code graph analysis (1)
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts (7)
workspaces/ballerina/ballerina-extension/src/features/ai/dataMapping.ts (2)
getFunctionDefinitionFromSyntaxTree(150-173)repairAndCheckDiagnostics(1036-1056)workspaces/ballerina/ballerina-core/src/rpc-types/ai-panel/interfaces.ts (1)
InlineMappingsSourceResult(127-132)workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.ts (4)
DataMapperModelRequest(295-300)DataMapperModelResponse(329-331)DeleteMappingRequest(426-433)AllDataMapperSourceRequest(316-318)workspaces/ballerina/ballerina-core/src/interfaces/data-mapper.ts (2)
DMModel(133-147)Mapping(106-118)workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/types.ts (1)
CodeRepairResult(56-59)workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/repair-utils.ts (1)
addCheckExpressionErrors(334-427)workspaces/ballerina/ballerina-extension/src/utils/modification.ts (1)
writeBallerinaFileDidOpenTemp(69-92)
🔇 Additional comments (7)
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts (7)
28-28: LGTM!The new imports for
DataMapperModelRequest,DeleteMappingRequest, andwriteBallerinaFileDidOpenTempare correctly added and used by the new helper functions.Also applies to: 44-44
401-422: LGTM!The integration of the new helper functions follows a logical flow: first handling BCE3032 check expression errors, then removing any remaining mappings with unresolved diagnostics. The destructured results are correctly used downstream.
441-446: LGTM!The
buildMappingFileArraycorrectly uses the updated content from the error mapping removal flow.
936-999: LGTM with minor type suggestion.The inline mapping removal logic correctly mirrors the non-inline version. The use of
anyforlangClient(line 938) is consistent with other new functions but could be improved with proper typing.
827-829: LGTM!The display logic correctly uses
updatedCustomContentfrom the error mapping removal flow.
789-794: Verify the change fromcodeRepairResult.finalContentto reading frommainFilePath.The code now reads from
mainFilePath(line 791) directly instead of usingcodeRepairResult.finalContent. This is consistent with the new flow where content is re-read after modifications, but ensure this is intentional since the original code may have used the repaired content from the CodeRepairResult.
1221-1234: I'll verify the concern about multi-linenewTexthandling in the datamapper code. Let me start by examining the full function and understanding how the edits are structured.
<function_calls>
#!/bin/bashFind and examine the applyTextEditsToTempFile function
rg -n "applyTextEditsToTempFile" --type=ts -A 50 | head -70
</function_calls><function_calls>
#!/bin/bashRead the specific file and lines mentioned in the review
cat -n workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts | sed -n '1193,1239p'
</function_calls><function_calls>
#!/bin/bashSearch for deleteMapping to understand the API structure
rg -n "deleteMapping" --type=ts -B 3 -A 10
</function_calls><function_calls>
#!/bin/bashSearch for textEdit structure and how it's used
rg -n "textEdit" --type=ts -B 2 -A 5 | head -100
</function_calls>
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts
Show resolved
Hide resolved
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts
Show resolved
Hide resolved
workspaces/ballerina/ballerina-extension/src/features/ai/service/datamapper/datamapper.ts
Show resolved
Hide resolved
2 participants
Description
This PR addresses syntax error handling in the AI Data Mapper by automatically removing fields that contain syntax errors after all repair attempts have been exhausted.
Resolves: wso2/product-ballerina-integrator#1811
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
Release Notes
Bug Fixes
Performance
✏️ Tip: You can customize this high-level summary in your review settings.