Fix : Walled Ai Guardrail changed to walledprotect #1316
Conversation
|
DescriptionSummary By MatterAI
🔄 What ChangedThis PR renames the 🔍 Impact of the ChangeThis change primarily refactors and rebrands the Walled AI guardrail plugin to align with the new 📁 Total Files Changed
🧪 Test AddedA new unit test has been added in 🔒Security VulnerabilitiesThe change from MotivationThis PR is motivated by the need to update the Walled AI guardrail plugin to reflect a rebranding from 'Guardrail' to 'WalledProtect' and to ensure it integrates correctly with the updated Walled AI API endpoint and authentication mechanism. The new test case enhances coverage for conversational AI interactions. Type of Change
How Has This Been Tested?
Screenshots (if applicable)N/A Checklist
Related IssuesN/A Tip Quality Recommendations
Tanka Poem ♫
Sequence DiagramsequenceDiagram
participant GH as GitHub
participant WH as Webhook Handler
participant API as GitHub API
participant DB as Database
participant WP as WalledProtect Service
GH->>+WH: POST /webhook (Pull Request Event)
Note over WH: Validate signature
alt Valid Signature
WH->>+API: GET /repos/:owner/:repo/pulls/:id (pullRequestData)
API-->>-WH: pullRequestData
WH->>+WP: analyzeText(text, config)
Note over WP: Apply guardrails
WP-->>-WH: analysisResult (verdict, data)
WH->>+DB: INSERT INTO analyses (analysisId, pullRequestData, analysisResult)
DB-->>-WH: analysisId
WH->>+API: POST /repos/:owner/:repo/issues/:id/comments (reviewComment)
Note over API: Post WalledProtect review
API-->>-WH: commentId
WH-->>-GH: 200 OK
else Invalid Signature
WH-->>-GH: 401 Unauthorized
end
|
![matter-code-review[bot]](https://avatars.githubusercontent.com/in/474549?s=60&v=4){kind=small}
| const requestBody = { | ||
| text: text, | ||
| text_type: parameters.text_type || 'prompt', | ||
| generic_safety_check: parameters.generic_safety_check ?? true, |
There was a problem hiding this comment.
🛠️ Code Refactor
Issue: Removed text_type parameter from request body without explanation
Fix: Consider documenting why this parameter was removed or if it's still needed for the new API
Impact: May affect API compatibility if the new endpoint expects this parameter
The removal of text_type parameter should be verified against the new API documentation to ensure compatibility.
| @@ -18,7 +18,7 @@ | |||
| 'Financial Data', | |||
| ]; | |||
|
|
|||
| const DEFAULT_GREETINGS_LIST = ['Casual & Friendly', 'Professional & Polite']; | |||
| const DEFAULT_GREETINGS_LIST = ['Casual & Friendly']; | |||
There was a problem hiding this comment.
🛠️ Code Refactor
Issue: Default greetings list reduced from two options to one
Fix: Verify if 'Professional & Polite' option is no longer supported by the new API
Impact: May change default behavior for users who relied on both greeting styles
| const DEFAULT_GREETINGS_LIST = ['Casual & Friendly']; | |
| const DEFAULT_GREETINGS_LIST = ['Casual & Friendly']; |
| let text = textArray | ||
| .filter((text) => text) | ||
| .join('\n') | ||
| .trim(); |
There was a problem hiding this comment.
⚡️ Performance Improvement
Issue: Text processing can be optimized with method chaining
Fix: Good improvement to add trim() and format the chain for better readability
Impact: Removes unnecessary whitespace and improves code maintainability
| let text = textArray | |
| .filter((text) => text) | |
| .join('\n') | |
| .trim(); | |
| let text = textArray | |
| .filter((text) => text) | |
| .join('\ | |
| ') | |
| .trim(); |
| it('should handle conversational text format', async () => { | ||
| const context = { | ||
| requestType: 'chatComplete', | ||
| request: { | ||
| json: { | ||
| messages: [ | ||
| { role: 'user', content: 'Hi' }, | ||
| { role: 'assistant', content: 'Hello, how can I help you?' }, | ||
| ], | ||
| }, | ||
| }, | ||
| response: {}, | ||
| }; | ||
|
|
||
| const parameters = { | ||
| credentials: testCreds, | ||
| text_type: 'prompt', | ||
| generic_safety_check: true, | ||
| greetings_list: ['Casual & Friendly', 'Professional & Polite'], | ||
| pii_list: ["Person's Name", 'Address'], | ||
| compliance_list: ['questions on medicine'], | ||
| }; | ||
|
|
||
| const eventType = 'beforeRequestHook'; | ||
|
|
||
| const result = await handler(context as any, parameters, eventType); | ||
| expect(result).toHaveProperty('verdict'); | ||
| expect(result).toHaveProperty('data'); | ||
| }); |
There was a problem hiding this comment.
🛠️ Code Refactor
Issue: New test case has hardcoded test data that could be extracted to constants
Fix: Consider extracting test messages and parameters to improve maintainability
Impact: Makes tests more readable and easier to maintain
The test looks comprehensive but could benefit from extracting the mock data to constants for better organization.
| @@ -138,6 +138,6 @@ export const plugins = { | |||
| intercept: panwPrismaAirsintercept, | |||
| }, | |||
| walledai: { | |||
| guardrails: walledaiguardrails, | |||
| walledprotect: walledaiguardrails, | |||
| }, | |||
| }; | |||
There was a problem hiding this comment.
🛠️ Code Refactor
Issue: Missing newline at end of file
Fix: Add newline at end of file to follow standard conventions
Impact: Improves code consistency and follows common formatting standards
| }; | |
| }; |
Description
Modified the existing walledai guardrail , changed to walledprotect and changed authentication mode
Motivation
WalledAi guardrail was renamed and the existing base url was updated and authentication mode as well.
Type of Change
How Has This Been Tested?
Screenshots (if applicable)
Checklist
Related Issues
#1197