Skip to content

Conversation

@SamMalayek
Copy link
Contributor

@SamMalayek SamMalayek commented Nov 2, 2025

🧩 Summary

This PR adds a CI workflow for end-to-end embedding CLI tests (none exist today). It establishes a small, fast, reproducible baseline for validating embedding behavior (dimensions + determinism) using tiny GGUF models.

Discussion / design context: See the companion RFC in Discussions for the longer-term plan to add a native server endpoint: #16957

⚙️ What this PR includes

  • A GitHub Actions job (embeddings.yml) that runs E2E embedding CLI tests with cached tiny models (e.g., TinyLlama).
  • Checks output dimensions and deterministic behavior.
  • Keeps runs lightweight and fast; an optional large-model stress test can be added later.

@SamMalayek SamMalayek requested a review from CISC as a code owner November 2, 2025 16:00
@github-actions github-actions bot added examples python python script changes devops improvements to build systems and github actions labels Nov 2, 2025
@SamMalayek SamMalayek force-pushed the feature/test-embedding-raw branch from 015351e to 075c324 Compare November 2, 2025 18:39
@SamMalayek SamMalayek force-pushed the feature/test-embedding-raw branch from 075c324 to c1c3d99 Compare November 2, 2025 18:55
@CISC
Copy link
Collaborator

CISC commented Nov 2, 2025

While testing is great I have issues with this PR:

  • The CI only runs when the example code change, not when actual embedding code changes
  • The tests are not testing anything likely to break, nor indeed even anything useful

To sum up; overall as a CI job this will not run when it matters, and when it is run it will not catch any problems of consequence.

Lastly, why create a new PR instead of arguing your case in the original PR and maybe iron out concerns there?

@SamMalayek SamMalayek requested a review from ggerganov as a code owner November 2, 2025 22:01
@SamMalayek
Copy link
Contributor Author

SamMalayek commented Nov 2, 2025

While testing is great I have issues with this PR:

  • The CI only runs when the example code change, not when actual embedding code changes
  • The tests are not testing anything likely to break, nor indeed even anything useful

To sum up; overall as a CI job this will not run when it matters, and when it is run it will not catch any problems of consequence.

I kept them in examples alongside the CLI code as a pragmatic, incremental, small first step. The intent was to expand scope and relocate to tests/e2e/embedding/ once the embedding path itself was integrated into the server runtime (or simply commonized as an intermediary step). That said, I can move them now and broaden coverage to better match expectations (done).

Lastly, why create a new PR instead of arguing your case in the original PR and maybe iron out concerns there?

That PR was closed with minimal comment. There appeared to be no room for discussion.

@SamMalayek SamMalayek force-pushed the feature/test-embedding-raw branch from e5a5b26 to 82dbeee Compare November 2, 2025 22:12
@github-actions github-actions bot added the testing Everything test related label Nov 2, 2025
@SamMalayek SamMalayek force-pushed the feature/test-embedding-raw branch from 82dbeee to 2de1e68 Compare November 2, 2025 23:01
@SamMalayek
Copy link
Contributor Author

Note: Empty force-pushes are to re-trigger the CI workflow following an intermittent failure.

@SamMalayek
Copy link
Contributor Author

Putting this in draft while work on the RFC continues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

devops improvements to build systems and github actions examples python python script changes testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants