Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ npm run test:integration
```bash
# Run just the files integration tests
npm run test:integration -- tests/integration/client/files.test.ts
npm run test:integration -- --testPathPatterns="skills" -t "skill-driven extraction"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

For better readability and consistency with other examples in this file, please add a comment explaining what this command does.

Suggested change
npm run test:integration -- --testPathPatterns="skills" -t "skill-driven extraction"
# Run just the skill-driven extraction integration tests
npm run test:integration -- --testPathPatterns="skills" -t "skill-driven extraction"


# Run
npm run test:e2e -- tests/e2e/client/chat-completions.test.ts
Expand Down
63 changes: 62 additions & 1 deletion tests/integration/client/skills.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { config } from "dotenv";
config({ path: ".env.test" });

import { VlmRun } from "../../../src/index";
import { SkillInfo } from "../../../src/client/types";
import { SkillInfo, PredictionResponse } from "../../../src/client/types";

jest.setTimeout(60000);

Expand Down Expand Up @@ -379,4 +379,65 @@ describe("Integration: Skills", () => {
expect(result.download_url.length).toBeGreaterThan(0);
});
});

describe("create() and document.generate() — skill-driven extraction", () => {
const testFilePath = "tests/integration/assets/google_invoice.pdf";

it("should create a skill and use it for batch document generation via file_id", async () => {
const skill = await client.skills.create({
prompt: INVOICE_SKILL_PROMPT,
jsonSchema: INVOICE_SCHEMA,
name: `invoice-doc-generate-${Date.now()}`,
description:
"Invoice skill for document generation integration test.",
});

expect(skill).toBeTruthy();
expect(skill.id).toBeDefined();
expect(skill.name).toBeDefined();

const uploadedFile = await client.files.upload({
filePath: testFilePath,
purpose: "vision",
checkDuplicate: true,
});

expect(uploadedFile).toHaveProperty("id");

const result: PredictionResponse = await client.document.generate({
fileId: uploadedFile.id,
model: "vlm-1",
domain: "document.invoice",
batch: true,
config: {
skills: [
{
skill_name: skill.name,
version: skill.version,
},
],
} as any,
Comment on lines +412 to +419
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The use of as any here is a workaround for a type mismatch and should be avoided as it compromises TypeScript's type safety. The config object is being passed a skills property which is not defined in the GenerationConfigParams type.

To address this correctly, the following changes are needed in files not included in this pull request:

  1. Update GenerationConfigParams in src/client/types.ts: Add the optional skills property.
  2. Update generate method in src/client/predictions.ts: Ensure the skills property from the config is passed through to the API request. The current implementation appears to filter unknown properties from the config object.

I recommend including these changes in this pull request to ensure the new test is valid and type-safe.

metadata: {
environment: "dev",
allowTraining: false,
},
});

expect(result).toHaveProperty("id");
expect(typeof result.id).toBe("string");
expect(result).toHaveProperty("created_at");
expect(typeof result.created_at).toBe("string");
expect(result).toHaveProperty("status");
expect(result.status).toBe("pending");

const completed: PredictionResponse = await client.predictions.wait(
result.id,
);

expect(completed.status).toBe("completed");
expect(completed).toHaveProperty("response");
expect(completed.response).toBeTruthy();
expect(completed).toHaveProperty("completed_at");
});
});
});