Skip to content

Conversation

indranil786
Copy link
Contributor

Description

Integrating walledai plugin, for guardrails on Portkey platform
Issue Link: #1197

Motivation

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)

How Has This Been Tested?

  • Unit Tests
  • Integration Tests
  • Manual Testing

Screenshots (if applicable)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Related Issues

Copy link
Contributor

matter-code-review bot commented Jul 1, 2025

Code Quality new feature bug fix refactoring

Description

Summary By MatterAI MatterAI logo

🔄 What Changed

This Pull Request integrates the WalledAI plugin into the Portkey platform, providing guardrail functionality. Key changes include:

  • Replaced the getText utility with getCurrentContentPart in plugins/walledai/guardrails.ts for more robust and accurate content extraction from various request/response structures.
  • Enhanced error handling for empty content, providing a more specific error message and setting data to null in such cases.
  • Refactored plugins/walledai/walledai.test.ts to improve test readability and maintainability by introducing a makeContext helper function and streamlining test cases.

🔍 Impact of the Change

The changes enhance the WalledAI plugin's reliability by ensuring correct content is always processed for moderation. Improved error handling provides clearer feedback when content is missing. The refactored tests make it easier to understand, maintain, and extend the plugin's test suite, contributing to overall code quality and future development efficiency.

📁 Total Files Changed

  • plugins/walledai/guardrails.ts: Updated content extraction logic and error handling for empty text.
  • plugins/walledai/walledai.test.ts: Significant refactoring of test setup and individual test cases for clarity and conciseness.

🧪 Test Added

The following integration tests were added/modified to verify the plugin's functionality:

  • it('returns verdict=true for safe text'): Verifies that the plugin correctly identifies and returns a true verdict for safe input text.
  • it('returns verdict=false for unsafe text'): Confirms that the plugin accurately identifies and returns a false verdict for unsafe input text.
  • it('returns error if apiKey is missing'): Tests the plugin's error handling mechanism when the required API key is not provided in the credentials.
  • it('returns error if text is empty'): Validates the plugin's behavior and error reporting when the input content for moderation is empty.
  • it('uses default values for missing optional parameters'): Ensures that the plugin correctly applies default values for optional parameters if they are not explicitly provided in the plugin configuration.

🔒Security Vulnerabilities

No new security vulnerabilities were detected in this Pull Request. The changes primarily focus on content extraction, error handling, and test improvements, which do not introduce new attack vectors.

Motivation

Integrating the WalledAI plugin provides a new guardrail capability for the Portkey platform, addressing issue #1197 by enabling content moderation.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)

How Has This Been Tested?

  • Unit Tests
  • Integration Tests
  • Manual Testing

Screenshots (if applicable)

N/A

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Tip

Quality Recommendations

  1. Consider defining more structured error types (e.g., custom error objects or enums) for specific error conditions like missing API keys, instead of relying on string matching in tests. This improves robustness and maintainability for error handling.

  2. Add JSDoc comments to the handler function in guardrails.ts and the makeContext helper in walledai.test.ts to clearly describe their purpose, parameters, and return values, enhancing code documentation.

Sequence Diagram

sequenceDiagram
    participant User as "End User"
    participant PluginHandler as "WalledAI Plugin Handler"
    participant Utils as "Plugin Utils"
    participant WalledAIApi as "WalledAI API"

    User->>PluginHandler: triggerHook(context, params, eventType)
    Note over PluginHandler: beforeRequestHook / afterResponseHook

    PluginHandler->>PluginHandler: validateCredentials(parameters.credentials)
    alt API Key Missing
        PluginHandler-->>User: { error: "apiKey missing", verdict: true, data: null }
    else API Key Present
        PluginHandler->>+Utils: getCurrentContentPart(context, eventType)
        Utils-->>-PluginHandler: { content, textArray }

        alt Content is Empty
            PluginHandler-->>User: { error: { message: 'request or response json is empty' }, verdict: true, data: null }
        else Content Available
            PluginHandler->>PluginHandler: prepareText(textArray)
            Note over PluginHandler: text = textArray.filter((text) => text).join('
')

            PluginHandler->>PluginHandler: prepareRequestBody(text, parameters)
            PluginHandler->>PluginHandler: prepareRequestOptions(apiKey)

            PluginHandler->>+Utils: post(API_URL, requestBody, requestOptions)
            Note over Utils: HTTP POST to https://services.walled.ai/v1/guardrail/moderate

            Utils->>+WalledAIApi: POST /v1/guardrail/moderate (requestBody)
            WalledAIApi-->>-Utils: API Response (moderationResult)
            Utils-->>-PluginHandler: moderationResult

            PluginHandler->>PluginHandler: processApiResponse(moderationResult)
            PluginHandler-->>User: { verdict, error, data }
        end
    end
Loading

Copy link
Contributor

@matter-code-review matter-code-review bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR adds a new WalledAI guardrails plugin for content moderation. The implementation looks solid with good test coverage, but I've identified a few improvements that could enhance error handling and code quality.

Skipped files
  • conf.json: Skipped file pattern
  • plugins/walledai/manifest.json: Skipped file pattern

Comment on lines 63 to 66
console.log(e)
error = e instanceof Error ? e.message : String(e);
verdict = true;
data = null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Code Refactor

Issue: The error handling in the catch block uses console.log which is not ideal for production environments and doesn't provide structured logging.
Fix: Replace direct console.log with a more structured error handling approach.
Impact: Improves error traceability and debugging in production environments.

Suggested change
console.log(e)
error = e instanceof Error ? e.message : String(e);
verdict = true;
data = null;
console.error('WalledAI API error:', e);
error = e instanceof Error ? e.message : String(e);
verdict = true;
data = null;

Comment on lines 57 to 61
if (
data.safety[0]?.isSafe==false
) {
verdict = false;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Code Refactor

Issue: The condition for checking if content is unsafe is not robust and could be more explicit.
Fix: Add a more comprehensive check that handles potential undefined values and provides clearer intent.
Impact: Prevents potential runtime errors and makes the code more maintainable.

Suggested change
if (
data.safety[0]?.isSafe==false
) {
verdict = false;
}
if (
data?.safety?.[0]?.isSafe === false
) {
verdict = false;
}

Comment on lines +38 to +44
// Prepare request body
const requestBody = {
text: text,
text_type: parameters.text_type || 'prompt',
generic_safety_check: parameters.generic_safety_check ?? true,
greetings_list: parameters.greetings_list || ['generalgreetings'],
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚡️ Performance Improvement

Issue: The requestBody object is created with unnecessary property assignments when default values could be used more efficiently.
Fix: Use object spread with defaults to create a more concise and efficient requestBody.
Impact: Improves code readability and slightly optimizes object creation.

Suggested change
// Prepare request body
const requestBody = {
text: text,
text_type: parameters.text_type || 'prompt',
generic_safety_check: parameters.generic_safety_check ?? true,
greetings_list: parameters.greetings_list || ['generalgreetings'],
};
// Prepare request body
const requestBody = {
text,
text_type: parameters.text_type || 'prompt',
generic_safety_check: parameters.generic_safety_check ?? true,
greetings_list: parameters.greetings_list || ['generalgreetings'],
...(parameters.pii_list && { pii_list: parameters.pii_list }),
...(parameters.compliance_list && { compliance_list: parameters.compliance_list })
};

Copy link
Collaborator

@narengogi narengogi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code looks good, I've not been able to sign up on https://www.walled.ai/#contactEmailSection

If you're associated with walled.ai and if possible please provide a working API key over discord
https://discord.gg/nqxNXj3W
I'm @segadog in discord

plugins/index.ts Outdated
import { handler as panwPrismaAirsintercept } from './panw-prisma-airs/intercept';
import { handler as defaultjwt } from './default/jwt';
import { handler as defaultrequiredMetadataKeys } from './default/requiredMetadataKeys';
import { handler as defaultregexMatch } from "./default/regexMatch"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use the included .prettierrc for formatting, please revert these formatting changes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @narengogi I added a new commit and used the .prettierrc formatting

Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use Matter AI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with Matter AI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

textGuard: pangeatextGuard,
pii: pangeapii,
},
promptfoo: {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add these back, have you removed them by accident?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi , I was building the plugins using npm run build-plugin command with the provided config.json ,and all the services were not mentioned in the config file , hence when i build it , the other plugins did not get added in the index file . However I have manually added and updated the index file , in my latest commit . This issue with the index files has been solved @narengogi

Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use Matter AI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with Matter AI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

Copy link
Collaborator

@narengogi narengogi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, verified with the key provided
example curl

curl --location 'http://localhost:8787/v1/chat/completions' \
--header 'x-portkey-provider: openai' \
--header 'authorization: sk-' \
--header 'Content-Type: application/json' \
--header 'x-portkey-config: {"afterRequestHooks":[{"type":"guardrail","id":"my_solid_guardrail","checks":[{"id":"walledai.guardrails","parameters":{"credentials":{"apiKey":""}}}]}],"cache":{"mode":"semantic"}}' \
--data '{
    "model": "gpt-3.5-turbo",
    "max_tokens": 100,
    "stream": false,
    "messages": [
        {
            "role": "system",
            "content": "You are a saahelpful aaassistant"
        },
        {
            "role": "user",
            "content": "hey there"
        }
    ]
}'

narengogi
narengogi previously approved these changes Jul 2, 2025
Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use Matter AI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with Matter AI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

@narengogi narengogi linked an issue Jul 14, 2025 that may be closed by this pull request
Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use Matter AI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with Matter AI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use Matter AI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with Matter AI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use Matter AI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with Matter AI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

@VisargD VisargD merged commit ccc7f62 into Portkey-AI:main Aug 1, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Walledai plugin for guardrails

4 participants