Repository navigation
feat(llm): add an OCI Generative AI chat provider - #122
Open
fede-kamel wants to merge 4 commits into
Open
fede-kamel wants to merge 4 commits into
fede-kamel wants to merge 4 commits into
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
Closed
3 tasks done
Tiny Sweeper reviewFindingsNo active actionable findings. Advisory notes
Resolved this pass
Verdict: Incomplete · Priority: none Diagnostics and supporting detailsFull summaryPull request purpose (title): feat(llm): add an OCI Generative AI chat provider. Pull-request purpose (title): feat(llm): add an OCI Generative AI chat provider. The change adds a new `crates/tinyinference-llm/src/providers/oci/` module, registered from `crates/tinyinference-llm/src/providers/mod.rs`, supporting OCI Generative AI API keys, user-principal, session-token and session-token-file signing, resource principals, host-supplied token sources, and feature-gated instance-principal and OKE workload identity, over both an OpenAI-compatible transport and a native `actions/chat` transport. It also adds a host-supplied request-signer hook to the shared OpenAI adapter, broadens reasoning-model detection to `gpt-6` and OCI's `openai.`-prefixed ids, and adds an `oci` preset and `BuiltinAuthStyle::Oci` to the builtin provider catalog. The tests and description lanes are Answered and report no blocking issue, having withdrawn earlier committed-key findings. Unreviewed paths
Partial review reasons
Review snapshot
Completeness: Incomplete What changedLatest-push-delta unavailable: review state holds prior reviewed head `7920d4bab7e536ded93f1945b93742d5f5bcd945` and new head `5506ff3208bdbb21d1852d86ed9f632035311dee`, but there is no trusted base-to-head diff comparison, so no push-level change description is given. The whole-pull-request behavior is described in the features and executive summary. Features
Tests
Before merge
How this fits togetherflowchart LR
n0["OpenAiModel<br/>changed"]:::changed
n1["ProviderRequestOptions<br/>changed"]:::changed
n2["BuiltinAuthStyle<br/>changed"]:::changed
n3["BuiltinCloudProvider<br/>changed"]:::changed
n4["build_with"]:::impacted
n5["builtin_cloud_provider"]:::impacted
n6["builtin_cloud_supports_responses_api"]:::impacted
n7["endpoint_host_is_chat_completions_only"]:::impacted
n8["...es_with_slug_capability_for_every_builtin"]:::impacted
n9["Client"]:::impacted
n0 -->|uses| n1
n0 -->|uses| n9
n1 -->|uses| n9
n3 -->|uses| n2
n4 -->|uses| n0
n5 -->|uses| n3
n6 -->|calls| n5
n7 -->|calls| n6
n8 -->|calls| n6
n8 -->|tests| n6
n8 -->|calls| n7
n8 -->|tests| n7
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
Evidence and run details
|
14 of 15 tasks
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
|
|
||
| /// PKCS#1 (`RSA PRIVATE KEY`) form of the same key. | ||
| pub(super) const TEST_PKCS1_PEM: &str = "\ | ||
| -----BEGIN RSA PRIVATE KEY-----\n\ |
There was a problem hiding this comment.
Remove the committed private key (an RSA private key)
Rotate the key, remove it from the working tree, and purge it from history — a force-push alone does not remove it from forks or from anyone who already fetched.
[RULE] private-key ·
Author
There was a problem hiding this comment.
Fixed in c21d902. The tests now generate the RSA key and the leaf certificate with OpenSSL (dev-dependency) at test time, and the branch was rewritten so no commit contains key material. The key was a throwaway test fixture and was never an OCI credential.
| assert!(pair.public_pem.starts_with("-----BEGIN PUBLIC KEY-----")); | ||
| assert!( | ||
| pair.private_pem | ||
| .starts_with("-----BEGIN RSA PRIVATE KEY-----") |
There was a problem hiding this comment.
Remove the committed private key (an RSA private key)
Rotate the key, remove it from the working tree, and purge it from history — a force-push alone does not remove it from forks or from anyone who already fetched.
[RULE] private-key ·
Author
There was a problem hiding this comment.
Fixed in c21d902. The tests now generate the RSA key and the leaf certificate with OpenSSL (dev-dependency) at test time, and the branch was rewritten so no commit contains key material. The key was a throwaway test fixture and was never an OCI credential.
| fn malformed_pem_is_a_validation_error() { | ||
| assert!(SigningKey::from_pem("not pem").is_err()); | ||
| assert!( | ||
| SigningKey::from_pem("-----BEGIN PRIVATE KEY-----\n!!!\n-----END PRIVATE KEY-----") |
There was a problem hiding this comment.
Remove the committed private key (a private key)
Rotate the key, remove it from the working tree, and purge it from history — a force-push alone does not remove it from forks or from anyone who already fetched.
[RULE] private-key ·
Author
There was a problem hiding this comment.
Fixed in c21d902. The tests now generate the RSA key and the leaf certificate with OpenSSL (dev-dependency) at test time, and the branch was rewritten so no commit contains key material. The key was a throwaway test fixture and was never an OCI credential.
fede-kamel
force-pushed
the
feat/oci-genai-provider
branch
2 times, most recently
from
October 22, 2026 14:33
3a923b8 to
c21d902
Compare
fede-kamel
added a commit
to fede-kamel/tinyagents
that referenced
this pull request
Oct 22, 2026
Moves vendor/tinyinference to tinyinference main plus the OCI Generative AI provider (tinyhumansai/tinyinference#122). The path dependencies are already at 0.5.0, so only the gitlink and Cargo.lock change; the lock follows tinyinference main's switch of its secret store to tinystoragedrivers-api.
fede-kamel
added a commit
to fede-kamel/tinyagents
that referenced
this pull request
Oct 22, 2026
Moves vendor/tinyinference to tinyinference main plus the OCI Generative AI provider (tinyhumansai/tinyinference#122). The path dependencies are already at 0.5.0, so only the gitlink and Cargo.lock change; the lock follows tinyinference main's switch of its secret store to tinystoragedrivers-api.
fede-kamel
added a commit
to fede-kamel/openhuman
that referenced
this pull request
Oct 22, 2026
Review fixes for the OCI Generative AI provider: - Endpoint: the built-in preset's URL is a display default. Settings no longer saves it as the entry endpoint, and the factory ignores it, so a non-Chicago region is no longer sent to us-chicago-1. An explicit override (another realm, a private endpoint) is kept on save. - Project: the `openai` transport (`/openai/v1`) requires `oci.project_id`, in the factory and in the settings form, with a translated message in every locale. - Billing: keyless OCI modes (profiles, session tokens, principals) count as usable BYOK credentials, so the managed-budget gate no longer blocks them. - Temperature: a route's `@<temperature>` suffix now reaches OCI models through the new `OciConfig::temperature_override`. - Settings: custom OCI slugs (e.g. `oci-cohere`) open the OCI dialog and keep their label and `oci` block instead of the generic editor, and a failed key deletion aborts a switch to a keyless mode and restores the previous providers. Moves vendor/tinyagents to tinyhumansai/tinyagents#517, which pins tinyinference with the OCI provider (tinyhumansai/tinyinference#122).
Author
|
@senamakel these are all rebased on current main with the bot findings addressed. Merge order, since three of them stack:
Independent:
The openhuman lanes that are still red fail in files these PRs don't touch: the Thanks! |
Add `providers::oci` to tinyinference-llm and an `oci` preset to the tinyinference-providers builtin catalog. - Auth: Generative AI API key (bearer); IAM request signing from an `~/.oci/config` API-signing-key profile, a session token or a resource principal; and, behind the off-by-default `oci-principals` feature (OpenSSL), instance principal and OKE workload identity. - Transports: OpenAI-compatible `/openai/v1` and `/20231130/actions/v1` through the OpenAI adapter, which gains a `RequestSigner` hook, and the native `/20231130/actions/chat` API (GENERIC and COHERE formats) with its own request, response and SSE stream mapping. - Serving: on-demand by model id or a dedicated endpoint OCID. - Live-service fixes: `maxCompletionTokens` for OpenAI models on the native API, no sampling parameters for OpenAI reasoning models, no OpenAI-wire provider options on the native API, and `opc-compartment-id` on both OpenAI-compatible bases. - `examples/live_oci.rs` runs a credential-gated live matrix and skips when no credentials are set. Tests sign with an RSA key and a self-signed leaf certificate generated by OpenSSL (a dev-dependency) at test time, so no key material is committed.
Like OpenAiConfig::temperature_override: the value replaces the request's temperature on both the OpenAI-compatible and native transports, and OpenAI reasoning models still receive none. Hosts that encode a per-route temperature can now apply it to OCI models.
fede-kamel
force-pushed
the
feat/oci-genai-provider
branch
from
November 6, 2026 18:31
257befa to
7920d4b
Compare
|
|
||
| impl Acc { | ||
| fn delta_tool(&mut self, id: &str, name: &str, args: &str, slot_hint: usize) -> usize { | ||
| let existing = if id.is_empty() { |
There was a problem hiding this comment.
Guard the tool-call slot before indexing an empty accumulator
When `id` is empty (a continuation fragment) and no call has been seen yet, `self.calls` is empty, so `slot_hint.min(0)` is `0`. The `is_empty()` fallback below only runs when `existing != 0`, which a zero never satisfies, so `self.calls[existing]` indexes an empty `Vec` and panics. Reachable input: a GENERIC stream whose first tool-call fragment carries no id, e.g. `data: {"message":{"toolCalls":[{"name":"get_weather","arguments":"{"}}]}}\n`. The provider may split the arguments across chunks and let the id arrive on a later fragment; nothing in the adapter requires the id to come first. A panic here aborts the stream instead of producing a `ProviderFailed` terminal item, violating the rule that stream adapters must emit terminal `Completed` or `ProviderFailed` items.
[RULE] unchecked-index ·
Author
There was a problem hiding this comment.
Fixed in 5506ff3. The fallback did push a slot before indexing, so it could not panic, but it was easy to misread. An id-less fragment now opens its slot up front, and a later fragment carrying the id adopts that slot instead of starting a second call. New test: tool_call_fragment_before_any_id_does_not_panic.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an OCI Generative AI chat provider to
tinyinference-llm(providers::oci), plus anocipreset in thetinyinference-providersbuiltin catalog.~/.oci/config, a session token, or a resource principal.oci-principalsfeature: instance principal and OKE workload identity. That feature pulls in OpenSSL./openai/v1and/20231130/actions/v1, through the existing OpenAI adapter./20231130/actions/chat,GENERICandCOHEREformats), with its own request, response and SSE stream mapping.build_oci_model(OciConfig) -> Result<Arc<dyn ChatModel<()>>>.load_profilereads an OCI config profile.Related issue
None.
API or behavior changes
tinyinference_llm::providers::oci.oci-principals, off by default.OpenAiModelgains aRequestSignerhook. It signs the fully built request before it is sent, and it is unused unless set.BuiltinAuthStyle::Ociand anocientry inBUILTIN_CLOUD_PROVIDERS. Adding the variant breaks hosts that matchBuiltinAuthStyleexhaustively; the OpenHuman host is updated in its own PR.is_reasoning_modelnow also matchesgpt-6*and OCI'sopenai.-prefixed ids. Requests to those models getmax_completion_tokensinstead ofmax_tokens.Behavior found against the live service and fixed here:
maxCompletionTokens.temperatureortop_p, on both the native and/openai/v1paths, so the provider leaves them out for those models.provider_optionssuch asprompt_cache_keyare rejected by the native API, so the native path drops them./openai/v1calls needopc-compartment-id. It is now sent on both OpenAI-compatible bases whenever a compartment is configured.Validation
cargo fmt --all -- --checkcargo clippy --all-targets --all-features -- -D warningscargo build --all-targets --all-featurescargo test --all-features: all passRUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-featuresLive check, tested with OCI GenAI:
examples/live_oci.rsran a completion, a streamed completion and a streamed tool round trip on 10 model/transport cases. All 30 checks passed with IAM signing and all 30 passed with a GenAI API key. The cases covered OpenAI, xAI, Meta, Google and Cohere models over all four transports.Tests
*_tests.rsfiles. They cover:examples/live_oci.rsis the credential-gated live smoke test. It exits 0 with "skipped" whenOCI_LIVE_PROFILE/OCI_LIVE_API_KEYare unset, so CI never calls the service.Documentation
Rustdoc on every public item. The live example's header documents its environment variables.
Checklist
#[allow(...)],#[ignore], or relaxed lints.envcontents in the diff or the descriptioncc @senamakel @M3gA-Mind for review and merge.