Add OllamaEmbeddingClient and harden Ollama endpoint URL handling - #22
Merged
Conversation
marevol
force-pushed
the
worktree-content-chunk-embedding-phase1
branch
from
August 1, 2026 12:29
4003001 to
ac1a874
Compare
marevol
marked this pull request as ready for review
August 1, 2026 12:33
This was referenced Aug 2, 2026
Merged
…ndling Implement Fess core's EmbeddingClient SPI for Ollama so the content-chunking RAG pipeline can generate chunk vectors through a local Ollama server, and fix the endpoint-URL handling shared by both clients in this plugin. Requires a Fess core 15.8.0-SNAPSHOT carrying codelibs/fess#3184 (the EmbeddingClient SPI) and codelibs/fess#3202 (AbstractEmbeddingClient and CredentialUrlUtil). Both are merged. OllamaEmbeddingClient --------------------- Extends AbstractEmbeddingClient, calls POST /api/embed, and is registered as ollamaEmbeddingClient in fess_llm++.xml. It initializes only when content_chunker.embedding.name selects ollama, and stays fully inert otherwise. Only Ollama-specific members are declared here; the provider-agnostic ones (getDimension, isContentChunkerEnabled, getAvailabilityCheckInterval, buildHttpClient, getConnectTimeout and the two-tier timeout setup in init()) come from the base class. The one init() addition is the model/prefix diagnostic below, which is genuinely Ollama-specific: only this provider drives the document/query distinction through a text prefix whose correct value depends on the model family. - The default model is embeddinggemma, with the task prefixes that go with it ("title: none | text: " / "task: search result | query: "). Ollama's embeddinggemma template is a pass-through, so the caller has to supply these. nomic-embed-text, the obvious alternative, is trained on English: measured on a 14-document Japanese corpus every document scored between 0.76 and 0.83 for a paraphrase query and the expected document ranked last, while embeddinggemma ranked that document first on the same corpus. Both emit 768-dimensional vectors, so content_chunker.embedding.dimension is unchanged and switching between them costs a re-index and no mapping change. - Each prefix is independently configurable and can be cleared with an empty value. A WARN fires when the configured prefixes do not follow the same convention as the configured model; nomic-embed and embeddinggemma are both recognized, so changing the model and leaving the prefixes alone is caught in either direction. Blanking both prefixes suppresses the warning. A mismatch is worth warning about because it still returns valid vectors of the right dimension while silently degrading relevance. - Input is split into sub-batches of at most 128 items and concatenated in input order, for parity with the OpenAI and Gemini clients. Order, count and all-or-nothing failure semantics are preserved. - The truncate flag is sent explicitly, defaulting to true (Ollama's own server-side default, so behavior is unchanged). truncate=false fails an over-context chunk loudly instead of indexing a silently cut-down vector. An unparseable value keeps the default and warns. - Response validation rejects a vector-count mismatch, a dimension mismatch, a non-numeric component and a non-finite component: a magnitude such as 1e999 parses as a JSON number whose value is Infinity and would otherwise have been stored as an unusable component. - Retries cover retryable HTTP statuses and connect-time IOExceptions with exponential backoff and jitter. Availability is probed via /api/tags, tolerating the ":latest" tag form. - Every content_chunker.embedding.ollama.* key resolves through the inherited getConfigString/getConfigInt/getConfigLong, i.e. the conf/system.properties channel that every other content_chunker.* setting uses and that the admin UI displays. The api.url key named in diagnostics is derived from getConfigPrefix() rather than spelled out a second time, so it cannot drift from the key actually read. Endpoint URL handling --------------------- Ollama does not authenticate and this plugin has no credential setting for it, so the endpoint URL is the one place a secret can appear: an operator points api.url at a reverse proxy guarding Ollama and carries the shared secret in the URL itself. A new OllamaUrlUtil holds the handling once, because the two clients sit in different packages and a package-private helper could not be reached from the LLM side. Detecting and masking a credential inside a URL is provider-agnostic and delegates to fess core's CredentialUrlUtil; what stays here is the /api-segment normalization, the query/fragment-preserving path append, and the wording of the refusal. - Credentials in a logged URL are masked, covering both forms a secret takes: the api_key/key/token/access_token query parameters (case-insensitive) and the user:password@ userinfo component. All twelve log sites in the two clients that name the endpoint route through maskCredentialInUrl; nine of those are pre-existing OllamaLlmClient DEBUG/WARN lines that had no masking at all. - An endpoint carrying a query string now works. Both clients built their request URL by concatenation (apiUrl + "/api/tags"), which is silently wrong whenever the endpoint carries a query: "http://gateway/?api_key=..." + "/api/tags" produced "http://gateway/?api_key=.../api/tags", whose request target is the path "/" with the API path glued onto the credential's value. Nothing rejects that URL - it is well-formed - so the request simply went somewhere else with a mangled secret. appendPath splices the API path in front of the query or fragment instead, and normalizeBaseUrl is query-aware too: "http://gateway/ollama/api?api_key=..." was previously left untouched, because the literal string does not end in "/api". Verified against a live server, which now logs GET "/api/tags?api_key=..." 200 where it previously saw the corrupted path. - A malformed endpoint used to reach the log three ways at once, because the URI parser quotes the whole offending value in its message and that message rode on the WARN, on its attached throwable, and on the cause handed to the caller. Masking could not have helped, since a value is unparseable precisely because it holds a character the masking patterns exclude. All construction points now go through createHttpGet/createHttpPost, which keep only the parser's reason and failure index, attach no cause, and name the configuration key to inspect instead of failing generically. - An api.url that embeds credentials in the authority is refused up front. RFC 9110 section 4.2.4 forbids generating userinfo in an http(s) target URI and httpclient5 enforces that unconditionally, so such an endpoint could never have issued a request. Both clients now name the supported alternative (http.proxy.host/port/username/password) instead of failing opaquely at execute time. The availability path reports unavailable and explains once at ERROR rather than throwing, because both clients reach it synchronously from an eager container init method. No part of the configured value reaches the log, the exception, or its cause chain. README documents the new content_chunker.embedding.ollama.* keys, the model-to-prefix mapping, that changing the model requires re-running the chunk-vector job, and the correct configuration channel for each property group, including rag.llm.name, which Fess core reads from conf/system.properties but which was documented here as a fess_config.properties key. Tests: 177 pass. OllamaEmbeddingClientTest (60) and OllamaUrlUtilTest (15) are new; OllamaLlmClientTest (100) gains the masking, malformed-endpoint and assembled-URL cases; OllamaLlmClientRetryTest (2) is unchanged. Shared scaffolding lives in org.codelibs.fess.unit: LogCapturingAppender renders an event's throwable as well as its message, because a message-only assertion cannot see a value that reaches the log file through a stack trace and would pass while the rendered log still leaked; TestSystemProperties lets the tests drive the production config getters against the real systemProperties component rather than only a hand-written override, so a config-channel regression is detectable in either direction. No public API change; the only DI addition is the ollamaEmbeddingClient component.
marevol
force-pushed
the
worktree-content-chunk-embedding-phase1
branch
from
August 2, 2026 11:58
6a9eb96 to
fbddc8f
Compare
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
OllamaEmbeddingClient, implementing Fess core'sEmbeddingClientSPI (embedDocuments/embedQuery) on top of Ollama'sPOST /api/embed, so the content-chunking RAG pipeline can generate chunk vectors through a local Ollama server. It extendsAbstractEmbeddingClient, is registered asollamaEmbeddingClientinfess_llm++.xmlalongside the existingOllamaLlmClient, and is selected viacontent_chunker.embedding.name=ollama.OllamaUrlUtiland routes both clients through it, closing four ways the configured endpoint URL could reachfess.log, an exception message, or the wrong server.content_chunker.embedding.ollama.*keys in the README, along with the model-to-prefix mapping, and corrects which configuration channel each property group belongs to.OllamaEmbeddingClient
Only Ollama-specific members are declared here.
getDimension,isContentChunkerEnabled,getAvailabilityCheckInterval,buildHttpClient,getConnectTimeoutand the two-tier timeout setup ininit()come fromAbstractEmbeddingClient. The singleinit()addition is the model/prefix diagnostic, which is genuinely provider-specific: only Ollama drives the document/query distinction through a text prefix whose correct value depends on the model family.embeddinggemma, with the task prefixes that go with it (title: none | text:/task: search result | query:). Ollama'sembeddinggemmatemplate is a pass-through, so the caller has to supply these.nomic-embed-text, the obvious alternative, is trained on English: measured on a 14-document Japanese corpus, every document scored between 0.76 and 0.83 for a paraphrase query and the expected document ranked last, whileembeddinggemmaranked that document first on the same corpus. Both emit 768-dimensional vectors, socontent_chunker.embedding.dimensionis unchanged and switching between them costs a re-index and no mapping change.nomic-embedandembeddinggemmaare recognized, so changing the model and leaving the prefixes alone is caught in either direction. Blanking both prefixes suppresses the warning. A mismatch is worth warning about because it still returns valid vectors of the right dimension while silently degrading relevance.truncateflag is sent explicitly, defaulting totrue(Ollama's own server-side default, so behavior is unchanged).truncate=falsefails an over-context chunk loudly instead of indexing a silently cut-down vector. An unparseable value keeps the default and warns.1e999parses as a JSON number whose value isInfinityand would otherwise have been stored as an unusable component.IOExceptions with exponential backoff and jitter. Availability is probed via/api/tags, tolerating the:latesttag form.Endpoint URL handling
Ollama does not authenticate and this plugin has no credential setting for it, so the endpoint URL is the one place a secret can appear: an operator points
api.urlat a reverse proxy guarding Ollama and carries the shared secret in the URL itself. A sharedOllamaUrlUtilholds the handling once, because the two clients sit in different packages and a package-private helper could not be reached from the LLM side. Detecting and masking a credential inside a URL is provider-agnostic and delegates to fess core'sCredentialUrlUtil; what stays here is the/api-segment normalization, the query/fragment-preserving path append, and the wording of the refusal.api_key/key/token/access_tokenquery parameters (case-insensitive) and theuser:password@userinfo component. All twelve log sites in the two clients that name the endpoint route throughmaskCredentialInUrl; nine of those are pre-existingOllamaLlmClientDEBUG/WARN lines that had no masking at all.apiUrl + "/api/tags"), which is silently wrong whenever the endpoint carries a query:http://gateway/?api_key=...+/api/tagsproducedhttp://gateway/?api_key=.../api/tags, whose request target is the path/with the API path glued onto the credential's value. Nothing rejects that URL — it is well-formed — so the request simply went somewhere else with a mangled secret.appendPathsplices the API path in front of the query or fragment instead, andnormalizeBaseUrlis query-aware too:http://gateway/ollama/api?api_key=...was previously left untouched, because the literal string does not end in/api. Verified against a live server, which now logsGET "/api/tags?api_key=..." 200where it previously saw the corrupted path.createHttpGet/createHttpPost, which keep only the parser's reason and failure index, attach no cause, and name the configuration key to inspect instead of failing generically.http.proxy.host/port/username/password) instead of failing opaquely at execute time. The availability path reports unavailable and explains once at ERROR rather than throwing, because both clients reach it synchronously from an eager container init method. No part of the configured value reaches the log, the exception, or its cause chain.Configuration channel
Every
content_chunker.embedding.ollama.*key resolves through the inheritedgetConfigString/getConfigInt/getConfigLong, i.e. theconf/system.propertieschannel that every othercontent_chunker.*setting in Fess core uses and that the admin UI displays. Six of these keys initially read fromfess_config.properties, so an operator following the surrounding convention — or the admin UI — would have found them silently ignored. Theapi.urlkey named in diagnostics is derived fromgetConfigPrefix()rather than spelled out a second time, so it cannot drift from the key actually read.The README is updated accordingly, including
rag.llm.name, which Fess core reads fromconf/system.propertiesbut which was documented here as afess_config.propertieskey.Dependencies
Requires a Fess core
15.8.0-SNAPSHOTcarrying both:Chunker/EmbeddingClientSPI (merged).AbstractEmbeddingClient's shared members andCredentialUrlUtil(merged).CI needs a snapshot published after codelibs/fess#3202 in order to compile.
Test plan
mvn test, run locally against a Fess core15.8.0-SNAPSHOTcarrying both dependencies above — 177/177 passing:OllamaEmbeddingClientTest(new): 60/60OllamaUrlUtilTest(new): 15/15 — pins the normalization, path-append and URL-factory semantics once for both clientsOllamaLlmClientTest: 100/100 — extended with the masking, malformed-endpoint and assembled-URL casesOllamaLlmClientRetryTest(pre-existing, regression check): 2/2Shared test scaffolding lives in
org.codelibs.fess.unit.LogCapturingAppenderrenders an event's throwable as well as its message, because a message-only assertion cannot see a value that reaches the log file through a stack trace, and such an assertion would pass while the rendered log still leaked. ATestSystemPropertiescomponent lets the tests drive the production config getters against the realsystemPropertiescomponent rather than only a hand-written override, so a config-channel regression is detectable in either direction.No public API change; the only DI addition is the
ollamaEmbeddingClientcomponent.