Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// # Returns | ||
| /// * `Ok(VarBuilder)` containing all loaded variables, ready for model instantiation. | ||
| /// * `Err(SurrealError)` with any errors. | ||
| pub fn load_model_vars(paths: &[PathBuf], dtype: DType) -> Result<VarBuilder, SurrealError> { | ||
| pub fn load_model_vars(paths: &[PathBuf], dtype: DType) -> Result<VarBuilder<'_>, SurrealError> { | ||
| // TO DO - For now we hardcode Device::Cpu, because elsewhere in the config we haven't supported CUDA yet. |
There was a problem hiding this comment.
The new signature pub fn load_model_vars(...) -> Result<VarBuilder<'_>, SurrealError> uses the '_ lifetime placeholder in the function’s return type without introducing any lifetime parameter to satisfy it. In Rust this is not allowed (rustc raises E0106 “expected named lifetime parameter”), so the llms crate will not even compile after this change. Previously returning VarBuilder compiled because the type carried a concrete lifetime supplied by Candle. Please revert to the concrete lifetime (e.g. VarBuilder/VarBuilder<'static>) so the crate builds again.
Useful? React with 👍 / 👎.
| [dependencies] | ||
| regex = "1.11.1" | ||
| ort = { version = "2.0.0-rc.9", features = [ "cuda", "ndarray" ]} | ||
| ndarray = "0.16.1" | ||
| once_cell = "1.18.0" | ||
| bytes = "1.10.1" | ||
| futures-util = "0.3.31" | ||
| futures-core = "0.3.31" | ||
| thiserror = "2.0.12" | ||
| onnx-embedding = "0.1.1" | ||
| serde = { version = "1.0.219", features = ["derive"] } | ||
| tempfile = "3.19.1" | ||
| regex.workspace = true | ||
| ort = { workspace = true, features = ["cuda", "ndarray"] } | ||
| ndarray.workspace = true | ||
| once_cell.workspace = true |
There was a problem hiding this comment.
In the build-context core crate you now declare dependencies as regex.workspace = true, once_cell.workspace = true, etc. (modules/c-wrapper/build-context/core lines 20‑30). This crate is not listed as a member of the top-level workspace, so Cargo errors out with “failed to find a workspace root” when running cargo build inside that directory (or when the Docker build context tries to compile it). Before this change the manifest listed explicit versions and compiled fine; after it, the manifest cannot even be parsed. Either add this crate to the workspace or restore explicit versions so the build-context tooling works again.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| candle-transformers = "0.9.1" | ||
| serde_json = "1.0.140" | ||
| safetensors = "0.6.0" | ||
| tokenizers = "0.21.2" | ||
| tokenizers = "0.21.4" |
There was a problem hiding this comment.
The workspace dependency you added at Cargo.toml:34 only specifies tokenizers = "0.21.4". Because crates such as modules/tokenizers/Cargo.toml now inherit the dependency via workspace = true, Cargo ignores their local default-features = false override (Cargo even warns about this). As a result, the surrealml-tokenizers crate is rebuilt with tokenizers’ default feature set enabled, pulling in native onig/SIMD code and breaking the wasm32 and minimal builds that previously relied on the slim unstable_wasm configuration. Please move default-features = false (and the desired feature list) into the workspace dependency so member crates still build without the heavy defaults.
Useful? React with 👍 / 👎.
1 participant
No description provided.