Skip to content

Add embedding generation APIs#244

Open
chubes4 wants to merge 1 commit into
WordPress:trunkfrom
chubes4:issue/242-embedding-generation
Open

Add embedding generation APIs#244
chubes4 wants to merge 1 commit into
WordPress:trunkfrom
chubes4:issue/242-embedding-generation

Conversation

@chubes4

@chubes4 chubes4 commented Jun 3, 2026

Copy link
Copy Markdown

Summary

  • Adds provider-agnostic embedding generation contracts, result DTO, model config options, and fluent/traditional client APIs.
  • Covers single and batch embedding generation through mock model tests.

Fixes #242.

Testing

  • composer phpcs
  • composer phpstan
  • composer test:unit -- --filter 'testGenerateEmbeddingResult|testGenerateEmbeddingReturnsFirstVector|testGenerateEmbeddingsReturnsBatchVectors|testGenerateEmbeddingResultWithInvalidModel|testGenerateEmbedding|testGenerateEmbeddings|EmbeddingResultTest'
  • php -d error_reporting='E_ALL & ~E_DEPRECATED' vendor/bin/phpunit --testsuite unit

AI assistance

  • AI assistance: Yes
  • Tool(s): OpenCode (gpt-5.5)
  • Used for: Drafted the embedding API implementation and tests; Chris remains responsible for review and validation.

@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: chubes4 <extrachill@git.wordpress.org>
Co-authored-by: dkotter <dkotter@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@codecov

codecov Bot commented Jun 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.10526% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.39%. Comparing base (99a65e8) to head (019dfaa).

Files with missing lines Patch % Lines
src/Results/DTO/EmbeddingResult.php 58.25% 43 Missing ⚠️
src/Builders/PromptBuilder.php 81.25% 9 Missing ⚠️
src/Providers/Models/DTO/ModelConfig.php 96.29% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              trunk     #244      +/-   ##
============================================
- Coverage     88.12%   87.39%   -0.74%     
- Complexity     1213     1258      +45     
============================================
  Files            60       61       +1     
  Lines          3934     4124     +190     
============================================
+ Hits           3467     3604     +137     
- Misses          467      520      +53     
Flag Coverage Δ
unit 87.39% <72.10%> (-0.74%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dkotter

dkotter commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

I know we already have #132 open to add embedding support and while that PR is fairly old now, it has already gone through an initial review. I've not compared the code there to here but curious if the approach taken here is different than that and if so, what the benefits are here as compared to that? Ideally we proceed with a single approach to solve the same problem to not overtax contributors here.

@chubes4

chubes4 commented Jun 4, 2026

Copy link
Copy Markdown
Author

Thanks for pointing that out. I honestly missed #132 before opening this, and there's definitely overlap. My apologies for the duplicated effort.

The main distinction I see is that this PR is provider-agnostic, while #132 includes OpenAI-specific implementation that I think belongs in the OpenAI provider package, not php-ai-client.

Happy to close this if maintainers want #132 to remain canonical, or take ownership here if this narrower direction makes more sense.

@dkotter

dkotter commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Happy to close this if maintainers want #132 to remain canonical, or take ownership here if this narrower direction makes more sense.

I'd defer to @JasonTheAdams and/or @felixarntz on that, I think ideal to have a single PR we're working from in order to not duplicate effort but I also know that original PR has sat for months and likely needs work to get it across the finish line so may be worth closing that out and proceeding here instead.

@JasonTheAdams JasonTheAdams self-requested a review June 10, 2026 04:13

@JasonTheAdams JasonTheAdams left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is great! Thanks for the PR, @chubes4! One key suggestion:

Let's introduce an Embedding Value Object that's iterable and immutable, takes a list<int|float> of values and an integer for dimensions. This will contain all the validations (cleaning up EmbeddingResult and make it a first-class type.

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.

Implement embedding generation contracts and client APIs

3 participants