-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Multi condition supports. Allow smart categories in rules #270
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
Changes from all commits
840c548
17603c3
cac3562
166c221
3cbfb4b
c9cdf8f
9f7c50d
1f80327
0d80ef1
5471ae8
80a8d96
d52a886
6da5a8a
ecab8f2
61767ef
10a0421
e4bf5e1
313e57c
c0f6e4d
0ec71cd
a460c03
64b2eeb
aa1a59e
33fd5a9
116429c
5aa72c7
fe75996
2c3acb6
1824231
41e053a
281abd2
32e9d8c
25dd063
89c9518
2a2e249
2973b3c
a085ab3
37e8ac7
e63a06a
416a31d
67c71fc
37d8662
e7847c8
dac204e
3ecbf70
c037166
b7729a6
d7c3198
7a4d29b
237bd00
1c4271d
51d019d
b921c2b
5798d61
236c232
25490c4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,50 @@ | ||||||||||||||||||||||||||||||||||||||
| name: Run Tests | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| on: | ||||||||||||||||||||||||||||||||||||||
| push: | ||||||||||||||||||||||||||||||||||||||
| branches: [main] | ||||||||||||||||||||||||||||||||||||||
| pull_request: | ||||||||||||||||||||||||||||||||||||||
| branches: [main] | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| jobs: | ||||||||||||||||||||||||||||||||||||||
| test: | ||||||||||||||||||||||||||||||||||||||
| runs-on: ubuntu-latest | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| steps: | ||||||||||||||||||||||||||||||||||||||
| - uses: actions/checkout@v4 | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| - name: Setup Node.js | ||||||||||||||||||||||||||||||||||||||
| uses: actions/setup-node@v4 | ||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||
| node-version: "22" | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| - name: Install pnpm | ||||||||||||||||||||||||||||||||||||||
| uses: pnpm/action-setup@v2 | ||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||
| version: 8 | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| - name: Get pnpm store directory | ||||||||||||||||||||||||||||||||||||||
| shell: bash | ||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||
| echo "STORE_PATH=\"$(pnpm store path --silent)\"" >> $GITHUB_ENV | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| - uses: actions/cache@v4 | ||||||||||||||||||||||||||||||||||||||
| name: Setup pnpm cache | ||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||
| path: ~/.pnpm-store | ||||||||||||||||||||||||||||||||||||||
| key: ${{ runner.os }}-pnpm-store-${{ hashFiles('**/pnpm-lock.yaml') }} | ||||||||||||||||||||||||||||||||||||||
| restore-keys: | | ||||||||||||||||||||||||||||||||||||||
| ${{ runner.os }}-pnpm-store- | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| - name: Install dependencies | ||||||||||||||||||||||||||||||||||||||
| run: pnpm install | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| - name: Run tests | ||||||||||||||||||||||||||||||||||||||
| run: pnpm -F inbox-zero-ai test | ||||||||||||||||||||||||||||||||||||||
| env: | ||||||||||||||||||||||||||||||||||||||
| RUN_AI_TESTS: false | ||||||||||||||||||||||||||||||||||||||
| DATABASE_URL: "postgresql://postgres:postgres@localhost:5432/postgres" | ||||||||||||||||||||||||||||||||||||||
| NEXTAUTH_SECRET: "secret" | ||||||||||||||||||||||||||||||||||||||
| GOOGLE_CLIENT_ID: "client_id" | ||||||||||||||||||||||||||||||||||||||
| GOOGLE_CLIENT_SECRET: "client_secret" | ||||||||||||||||||||||||||||||||||||||
| GOOGLE_PUBSUB_TOPIC_NAME: "topic" | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+42
to
+50
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add PostgreSQL service container and secure credentials. Two critical issues need to be addressed:
Add a PostgreSQL service container and use GitHub secrets: jobs:
test:
runs-on: ubuntu-latest
+ services:
+ postgres:
+ image: postgres:latest
+ env:
+ POSTGRES_PASSWORD: postgres
+ ports:
+ - 5432:5432
+ options: >-
+ --health-cmd pg_isready
+ --health-interval 10s
+ --health-timeout 5s
+ --health-retries 5
steps:
# ... existing steps ...
- name: Run tests
run: pnpm -F inbox-zero-ai test
env:
RUN_AI_TESTS: false
DATABASE_URL: "postgresql://postgres:postgres@localhost:5432/postgres"
- NEXTAUTH_SECRET: "secret"
- GOOGLE_CLIENT_ID: "client_id"
- GOOGLE_CLIENT_SECRET: "client_secret"
- GOOGLE_PUBSUB_TOPIC_NAME: "topic"
+ NEXTAUTH_SECRET: ${{ secrets.NEXTAUTH_SECRET }}
+ GOOGLE_CLIENT_ID: ${{ secrets.GOOGLE_CLIENT_ID }}
+ GOOGLE_CLIENT_SECRET: ${{ secrets.GOOGLE_CLIENT_SECRET }}
+ GOOGLE_PUBSUB_TOPIC_NAME: ${{ secrets.GOOGLE_PUBSUB_TOPIC_NAME }}📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,15 @@ | ||
| import { describe, expect, test, vi } from "vitest"; | ||
| import { getActionItemsWithAiArgs } from "@/utils/ai/choose-rule/ai-choose-args"; | ||
| import { type Action, ActionType, RuleType } from "@prisma/client"; | ||
| import { type Action, ActionType, LogicalOperator } from "@prisma/client"; | ||
| import type { RuleWithActions } from "@/utils/types"; | ||
|
|
||
| // pnpm test-ai ai-choose-args | ||
|
|
||
| const isAiTest = process.env.RUN_AI_TESTS === "true"; | ||
|
|
||
| vi.mock("server-only", () => ({})); | ||
|
|
||
| describe("getActionItemsWithAiArgs", () => { | ||
| describe.skipIf(!isAiTest)("getActionItemsWithAiArgs", () => { | ||
| test("should return actions unchanged when no AI args needed", async () => { | ||
| const actions = [getAction({})]; | ||
| const rule = getRule("Test rule", actions); | ||
|
|
@@ -167,7 +172,10 @@ function getAction(action: Partial<Action> = {}): Action { | |
| }; | ||
| } | ||
|
|
||
| function getRule(instructions: string, actions: Action[] = []) { | ||
| function getRule( | ||
| instructions: string, | ||
| actions: Action[] = [], | ||
| ): RuleWithActions { | ||
| return { | ||
| instructions, | ||
| name: "Test Rule", | ||
|
|
@@ -183,9 +191,10 @@ function getRule(instructions: string, actions: Action[] = []) { | |
| subject: null, | ||
| body: null, | ||
| to: null, | ||
| type: RuleType.AI, | ||
| enabled: true, | ||
| categoryFilterType: null, | ||
| conditionalOperator: LogicalOperator.AND, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Test coverage needed for OR and NOT logical operators The current test files only include test cases for
🔗 Analysis chainVerify test coverage for different logical operators While the default
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check if there are test cases for different logical operators
# Search for test cases using different logical operators
rg -l "LogicalOperator\.(OR|NOT)" "apps/web/__tests__"
# Search for potential complex condition tests
rg -l "conditionalOperator.*LogicalOperator" "apps/web/__tests__"
Length of output: 204 Script: #!/bin/bash
# Let's examine the actual test cases in these files
rg -B 2 -A 2 "conditionalOperator.*LogicalOperator" "apps/web/__tests__/ai-choose-rule.test.ts" "apps/web/__tests__/ai-choose-args.test.ts"
Length of output: 764 |
||
| type: null, | ||
| }; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,114 +1,101 @@ | ||
| import { expect, test, vi } from "vitest"; | ||
| import { chooseRule } from "@/utils/ai/choose-rule/choose"; | ||
| import { type Action, ActionType, RuleType } from "@prisma/client"; | ||
| import { describe, expect, test, vi } from "vitest"; | ||
| import { aiChooseRule } from "@/utils/ai/choose-rule/ai-choose-rule"; | ||
| import { type Action, ActionType, LogicalOperator } from "@prisma/client"; | ||
|
|
||
| vi.mock("server-only", () => ({})); | ||
|
|
||
| test("Should return no rule when no rules passed", async () => { | ||
| const result = await chooseRule({ | ||
| rules: [], | ||
| email: getEmail(), | ||
| user: getUser(), | ||
| }); | ||
|
|
||
| expect(result).toEqual({ reason: "No rules" }); | ||
| }); | ||
| // pnpm test-ai ai-choose-rule | ||
|
|
||
| test("Should return correct rule when only one rule passed", async () => { | ||
| const rule = getRule( | ||
| "Match emails that have the word 'test' in the subject line", | ||
| ); | ||
| const isAiTest = process.env.RUN_AI_TESTS === "true"; | ||
|
|
||
| const result = await chooseRule({ | ||
| email: getEmail({ subject: "test" }), | ||
| rules: [rule], | ||
| user: getUser(), | ||
| }); | ||
| vi.mock("server-only", () => ({})); | ||
|
|
||
| expect(result).toEqual({ rule, reason: expect.any(String), actionItems: [] }); | ||
| }); | ||
| describe.skipIf(!isAiTest)("aiChooseRule", () => { | ||
| test("Should return no rule when no rules passed", async () => { | ||
| const result = await aiChooseRule({ | ||
| rules: [], | ||
| email: getEmail(), | ||
| user: getUser(), | ||
| }); | ||
|
|
||
| test("Should return correct rule when multiple rules passed", async () => { | ||
| const rule1 = getRule( | ||
| "Match emails that have the word 'test' in the subject line", | ||
| ); | ||
| const rule2 = getRule( | ||
| "Match emails that have the word 'remember' in the subject line", | ||
| ); | ||
|
|
||
| const result = await chooseRule({ | ||
| rules: [rule1, rule2], | ||
| email: getEmail({ subject: "remember that call" }), | ||
| user: getUser(), | ||
| expect(result).toEqual({ reason: "No rules" }); | ||
| }); | ||
|
|
||
| expect(result).toEqual({ | ||
| rule: rule2, | ||
| reason: expect.any(String), | ||
| actionItems: [], | ||
| test("Should return correct rule when only one rule passed", async () => { | ||
| const rule = getRule( | ||
| "Match emails that have the word 'test' in the subject line", | ||
| ); | ||
|
|
||
| const result = await aiChooseRule({ | ||
| email: getEmail({ subject: "test" }), | ||
| rules: [rule], | ||
| user: getUser(), | ||
| }); | ||
|
|
||
| expect(result).toEqual({ | ||
| rule, | ||
| reason: expect.any(String), | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| test("Should generate action arguments", async () => { | ||
| const rule1 = getRule( | ||
| "Match emails that have the word 'question' in the subject line", | ||
| ); | ||
| const rule2 = getRule("Match emails asking for a joke", [ | ||
| { | ||
| id: "id", | ||
| createdAt: new Date(), | ||
| updatedAt: new Date(), | ||
| type: ActionType.REPLY, | ||
| ruleId: "ruleId", | ||
| label: null, | ||
| subject: null, | ||
| content: "{{Write a joke}}", | ||
| to: null, | ||
| cc: null, | ||
| bcc: null, | ||
|
|
||
| labelPrompt: null, | ||
| subjectPrompt: null, | ||
| contentPrompt: null, | ||
| toPrompt: null, | ||
| ccPrompt: null, | ||
| bccPrompt: null, | ||
| }, | ||
| ]); | ||
|
|
||
| const result = await chooseRule({ | ||
| rules: [rule1, rule2], | ||
| email: getEmail({ subject: "Joke", content: "Tell me a joke about sheep" }), | ||
| user: getUser(), | ||
| test("Should return correct rule when multiple rules passed", async () => { | ||
| const rule1 = getRule( | ||
| "Match emails that have the word 'test' in the subject line", | ||
| ); | ||
| const rule2 = getRule( | ||
| "Match emails that have the word 'remember' in the subject line", | ||
| ); | ||
|
|
||
| const result = await aiChooseRule({ | ||
| rules: [rule1, rule2], | ||
| email: getEmail({ subject: "remember that call" }), | ||
| user: getUser(), | ||
| }); | ||
|
|
||
| expect(result).toEqual({ | ||
| rule: rule2, | ||
| reason: expect.any(String), | ||
| }); | ||
| }); | ||
|
|
||
|
Comment on lines
+39
to
58
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add test cases for different LogicalOperator values The test suite only tests AND operator. Consider adding test cases for other LogicalOperator values. test("Should handle OR operator correctly", async () => {
const rule = getRule("Match emails with 'test' or 'remember'");
rule.conditionalOperator = LogicalOperator.OR;
const result = await aiChooseRule({
rules: [rule],
email: getEmail({ subject: "remember that call" }),
user: getUser(),
});
expect(result.rule).toBe(rule);
}); |
||
| console.debug("Generated content:\n", result.actionItems?.[0].content); | ||
|
|
||
| expect(result).toEqual({ | ||
| rule: rule2, | ||
| reason: expect.any(String), | ||
| actionItems: [ | ||
| test("Should generate action arguments", async () => { | ||
| const rule1 = getRule( | ||
| "Match emails that have the word 'question' in the subject line", | ||
| ); | ||
| const rule2 = getRule("Match emails asking for a joke", [ | ||
| { | ||
| bcc: null, | ||
| cc: null, | ||
| content: expect.any(String), | ||
| id: "id", | ||
| createdAt: new Date(), | ||
| updatedAt: new Date(), | ||
| type: ActionType.REPLY, | ||
| ruleId: "ruleId", | ||
| label: null, | ||
| subject: null, | ||
| content: "{{Write a joke}}", | ||
| to: null, | ||
| type: "REPLY", | ||
| cc: null, | ||
| bcc: null, | ||
|
|
||
| labelPrompt: null, | ||
| subjectPrompt: null, | ||
| contentPrompt: null, | ||
| toPrompt: null, | ||
| ccPrompt: null, | ||
| bccPrompt: null, | ||
| createdAt: expect.any(Date), | ||
| updatedAt: expect.any(Date), | ||
| id: "id", | ||
| ruleId: "ruleId", | ||
| }, | ||
| ], | ||
| ]); | ||
|
|
||
| const result = await aiChooseRule({ | ||
| rules: [rule1, rule2], | ||
| email: getEmail({ | ||
| subject: "Joke", | ||
| content: "Tell me a joke about sheep", | ||
| }), | ||
| user: getUser(), | ||
| }); | ||
|
|
||
| expect(result).toEqual({ | ||
| rule: rule2, | ||
| reason: expect.any(String), | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
|
|
@@ -129,9 +116,9 @@ function getRule(instructions: string, actions: Action[] = []) { | |
| subject: null, | ||
| body: null, | ||
| to: null, | ||
| type: RuleType.AI, | ||
| enabled: true, | ||
| categoryFilterType: null, | ||
| conditionalOperator: LogicalOperator.AND, | ||
| }; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,13 @@ | ||
| import { describe, it, expect, vi } from "vitest"; | ||
| import { aiDiffRules } from "@/utils/ai/rule/diff-rules"; | ||
|
|
||
| // pnpm test-ai ai-diff-rules | ||
|
|
||
| const isAiTest = process.env.RUN_AI_TESTS === "true"; | ||
|
|
||
| vi.mock("server-only", () => ({})); | ||
|
|
||
| describe("aiDiffRules", () => { | ||
| describe.skipIf(!isAiTest)("aiDiffRules", () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Inconsistent timeout values and missing documentation found across AI test files
🔗 Analysis chainDocument the timeout rationale. The 15-second timeout is used for AI operations. Consider adding a comment explaining why this specific duration was chosen and if it should be consistent across all AI-related tests. Also applies to: 52-52 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check for consistency in timeout values across AI test files
rg "(\d+_000)" "apps/web/__tests__/ai-*.test.ts"
Length of output: 122 Script: #!/bin/bash
# Let's first find the correct test files
fd "ai.*test" apps/web/__tests__
# Then search for timeout values in these files
rg "timeout|(\d+_000)" apps/web/__tests__
Length of output: 1148 |
||
| it("should correctly identify added, edited, and removed rules", async () => { | ||
| const user = { | ||
| email: "user@test.com", | ||
|
|
||
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.
🛠️ Refactor suggestion
Consider expanding test coverage.
The current configuration only runs tests for the
inbox-zero-aipackage, but the PR includes changes to web components and utilities. Consider running tests for all affected packages.📝 Committable suggestion