Engine: local mode + all-in-one Docker image + BYOK + UI + source endpoint + ingest race fix#45
Merged
hallelx2 merged 10 commits intoJun 19, 2026
Conversation
…7654 Adds a local mode that moves the base config to zero-setup defaults BEFORE the file/env layers (which still override): listen on :7654, a localhost Postgres URL matching the bundled/dev database, local file storage, and the Postgres-backed river queue (no Redis needed). The engine then boots with no required configuration — closing the 'database.url is required' gap that otherwise blocks a bare run. - --local flag sets VLE_LOCAL_MODE=true so CLI + Docker (env) share one path. - Adds VLE_STORAGE_LOCAL_ROOT binding for the image's data volume. - cmd/engine is already unauthenticated (single tenant), so local-mode auth needs no extra wiring; documented as dev/local only. - Documented in config.example.yaml; tests cover defaults, truthy forms, env-override precedence, and the non-local missing-DB-URL failure. Foundation for the all-in-one image (HAL-185) + local dashboard (HAL-188). Closes HAL-186.
…mode=disable) Per CodeRabbit review on #42 — the example referenced the DSN without the sslmode param that applyLocalDefaults actually injects.
…319) The ingest worker could race the just-written source object (Local.Put did no fsync; River picks the job up within microseconds), failing with 'parse: fetch source: storage: object not found' and marking the doc failed. Local.Put now fsyncs before returning and the source fetch retries on ErrNotFound with short backoff.
…i gateways (HAL-318)
…HAL-188, HAL-185)
New GET /v1/documents/{id}/source streams the original bytes (for PDF page
previews in clients). Dockerfile.allinone bundles the engine + Postgres + the
local web UI into one image; docker-allinone.yml publishes it to Docker Hub +
GHCR.
… settings (HAL-188) The engine now boots without a provider key in local mode and accepts per-request credentials (X-LLM-Api-Key / X-LLM-Provider / X-LLM-Base-Url / X-LLM-Model), building a per-request client that drives both the treewalk loop and citation span extraction. The bundled UI gains a settings modal that stores the key in the browser and sends it as headers — so a docker-run user configures their key from the dashboard, not only via env.
…-186-engine-zero-config-local-mode
Reviewer's GuideAdds BYOK support for local mode, a new /v1/documents/{id}/source endpoint, an ingest source-read race fix, GLM base_url doc clarification, and a bundled local UI plus all-in-one Docker image (engine + Postgres + viewer) with CI publishing workflow. Sequence diagram for BYOK treewalk answer flowsequenceDiagram
actor User
participant LocalUI as Local_UI
participant Viewer as Viewer_serve_py
participant Engine as Engine_HTTP
participant Deps as Deps_handleAnswerTreeWalk
participant LLMFactory as Deps.BuildLLM
User->>LocalUI: Submit question
LocalUI->>Viewer: POST /engine/v1/answer/treewalk
Note right of LocalUI: Adds X-LLM-Api-Key<br/>X-LLM-Provider<br/>X-LLM-Base-Url<br/>X-LLM-Model
Viewer->>Engine: POST /v1/answer/treewalk
Engine->>Deps: handleAnswerTreeWalk
Deps->>Deps: resolveLLM(request)
alt X-LLM-Api-Key present and BuildLLM set
Deps->>LLMFactory: BuildLLM(provider, apiKey, baseURL, model)
LLMFactory-->>Deps: llmgate.Client (per-request)
else no per-request key
Deps-->>Deps: use shared d.LLM
end
Deps->>Deps: set perReq.LLM and dReq.LLM
Deps->>Engine: serveAnswerTreeWalkStream or answerTreeWalk
Engine-->>Viewer: treewalk answer + citations
Viewer-->>LocalUI: answer, citations, pages_read
LocalUI-->>User: Render cited answer + preview
Sequence diagram for ingest source-read race retrysequenceDiagram
participant Pipeline as Pipeline.parse
participant Storage as storage.Storage
Pipeline->>Pipeline: getSourceWithRetry(ctx, storage, key)
loop up to 6 attempts
Pipeline->>Storage: Get(ctx, key)
alt Storage.Get succeeds
Storage-->>Pipeline: ReadCloser, Metadata
Pipeline-->>Pipeline: return rc, meta
else Storage.Get returns ErrNotFound
Storage-->>Pipeline: ErrNotFound
Pipeline->>Pipeline: backoff 150ms * (attempt+1)
else other error
Storage-->>Pipeline: error
Pipeline-->>Pipeline: return error
end
end
Pipeline->>Pipeline: parsers.Parse(ctx, contentType, filename, rc)
Sequence diagram for GET /v1/documents/{id}/sourcesequenceDiagram
actor User
participant LocalUI as Local_UI
participant Viewer as Viewer_serve_py
participant Engine as Engine_HTTP
participant Deps as Deps_handleGetSource
participant DB as DB
participant Store as Storage
User->>LocalUI: Open PDF preview
LocalUI->>Viewer: GET /engine/v1/documents/{id}/source
Viewer->>Engine: GET /v1/documents/{id}/source
Engine->>Deps: handleGetSource
Deps->>DB: GetDocument(ctx, id, standaloneOrgID, "")
alt document not found
DB-->>Deps: db.ErrNotFound
Deps-->>Engine: 404 document not found
else document found
DB-->>Deps: Document{SourceRef, ContentType}
alt SourceRef is empty
Deps-->>Engine: 404 document has no stored source
else SourceRef set
Deps->>Store: Get(ctx, doc.SourceRef)
alt object missing
Store-->>Deps: storage.ErrNotFound
Deps-->>Engine: 404 source object not found
else success
Store-->>Deps: ReadCloser, Metadata
Deps-->>Engine: 200 stream bytes<br/>Content-Type, Content-Length, inline
end
end
end
Engine-->>Viewer: HTTP response
Viewer-->>LocalUI: HTTP response
LocalUI-->>User: Render PDF page canvas
Flow diagram for engine LLM initialization with local BYOK modeflowchart LR
A[Start engine] --> B["buildLLM(cfg.LLM)"]
B -->|ok| C[llmClient set]
B -->|error| D{LocalModeEnabled<br/>and llmKeyMissing}
D -->|yes| E[Log warning<br/>no provider key]
E --> F[llmClient = nil<br/>TreeWalkStrategy built<br/>with nil client]
D -->|no| G[return init llm error]
F --> H[Deps.BuildLLM = buildLLMFrom]
C --> H
H --> I[Server accepts<br/>X-LLM-* headers<br/>for per-request BYOK]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.OpenGrep is compatible with Semgrep configurations. Add an |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In buildLLMFrom, the baseURL argument is only honored for the anthropic driver and silently ignored for openai/gemini, which makes the X-LLM-Base-Url header misleading for those providers; either wire baseURL through to those clients (if supported) or document/guard that only anthropic supports a custom base URL.
- buildLLM and buildLLMFrom now contain very similar provider/model wiring logic; consider factoring out a shared helper so new providers or config fields can be added in one place without the two paths diverging over time.
- The local viewer’s health status string is hardcoded to show
:7654even though ENGINE_URL in serve.py can point elsewhere; using ENGINE_URL (or the proxied origin) to populate this text would avoid confusing users when the engine listens on a non-default host/port.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In buildLLMFrom, the baseURL argument is only honored for the anthropic driver and silently ignored for openai/gemini, which makes the X-LLM-Base-Url header misleading for those providers; either wire baseURL through to those clients (if supported) or document/guard that only anthropic supports a custom base URL.
- buildLLM and buildLLMFrom now contain very similar provider/model wiring logic; consider factoring out a shared helper so new providers or config fields can be added in one place without the two paths diverging over time.
- The local viewer’s health status string is hardcoded to show `:7654` even though ENGINE_URL in serve.py can point elsewhere; using ENGINE_URL (or the proxied origin) to populate this text would avoid confusing users when the engine listens on a non-default host/port.
## Individual Comments
### Comment 1
<location path="cmd/engine/main.go" line_range="437-429" />
<code_context>
+ return false
+}
+
+func buildLLMFrom(c config.LLMConfig, provider, apiKey, baseURL, model string) (llmgate.Client, error) {
+ if provider == "" {
+ provider = c.Driver
+ }
+ switch provider {
+ case "anthropic":
+ if model == "" {
+ model = c.Anthropic.Model
+ }
+ if baseURL == "" {
+ baseURL = c.Anthropic.BaseURL
+ }
+ return anthropic.New(anthropic.Config{
+ APIKey: apiKey,
+ Model: model,
+ ReasoningModel: c.Anthropic.ReasoningModel,
+ BaseURL: baseURL,
+ })
+ case "openai":
+ if model == "" {
+ model = c.OpenAI.Model
</code_context>
<issue_to_address>
**suggestion (bug_risk):** OpenAI and Gemini branches ignore the `baseURL` argument, which may surprise callers of `BuildLLM`.
`BuildLLM` passes `baseURL` to `buildLLMFrom`, but the `openai` and `gemini` branches ignore it. Calls that set `X-LLM-Base-Url` for these providers will have no effect. Please either plumb `baseURL` into the OpenAI/Gemini client configs (as with Anthropic) or make it explicit in the API/docs that this parameter only applies to Anthropic.
Suggested implementation:
```golang
case "openai":
if model == "" {
model = c.OpenAI.Model
}
if baseURL == "" {
baseURL = c.OpenAI.BaseURL
}
return openai.New(openai.Config{
APIKey: apiKey,
Model: model,
ReasoningModel: c.OpenAI.ReasoningModel,
BaseURL: baseURL,
})
```
```golang
case "gemini":
if model == "" {
model = c.Gemini.Model
}
if baseURL == "" {
baseURL = c.Gemini.BaseURL
}
```
To fully wire `baseURL` for the Gemini provider, ensure the `gemini` case returns a client configured with the `BaseURL` field, mirroring the Anthropic/OpenAI patterns. For example, the body should look like:
```go
case "gemini":
if model == "" {
model = c.Gemini.Model
}
if baseURL == "" {
baseURL = c.Gemini.BaseURL
}
return gemini.New(gemini.Config{
APIKey: apiKey,
Model: model,
ReasoningModel: c.Gemini.ReasoningModel,
BaseURL: baseURL,
})
```
This assumes `openai.Config` and `gemini.Config` both support a `BaseURL` field; if they use a different field name, adjust accordingly.
</issue_to_address>
### Comment 2
<location path="localapp/index.html" line_range="464-469" />
<code_context>
+function cleanSnip(t){ if(!t) return ""; return t.replace(/\s+/g," ").trim().slice(0,200)+(t.length>200?"…":""); }
+
+// pdf preview
+async function loadPdf(docId, page){
+ const wrap=document.getElementById("pvWrap");
+ const srcUrl=E(`/v1/documents/${docId}/source`);
+ const openLink=document.getElementById("pvOpen"); if(openLink) openLink.href=srcUrl;
+ const fallback=()=>{ wrap.innerHTML=`<div class="pvmsg">Inline preview didn’t load — <a href="${srcUrl}" target="_blank" rel="noopener">open the source ↗</a>.</div>`; };
+ if(!window.pdfjsLib){ fallback(); return; }
+ try{
+ // disableRange/Stream: the local proxy serves the whole body with a 200
</code_context>
<issue_to_address>
**issue:** The PDF preview is attempted for any document type, which can cause confusing errors for non-PDF uploads.
Since `loadPdf`/`pdfjsLib.getDocument` is always called, non-PDF docs (DOCX/HTML/MD/TXT) will hit `/source` with a non-PDF body and cause pdf.js failures or generic error messages. You already have `content_type` on the document, so consider only invoking the inline PDF viewer when `content_type === 'application/pdf'` (or similar), and otherwise skip pdf.js and just show the “open source” link to avoid these spurious errors.
</issue_to_address>
### Comment 3
<location path="localapp/index.html" line_range="266" />
<code_context>
+const LS="vl_llm_settings";
+const DEFAULTS={provider:"anthropic",baseUrl:"https://api.z.ai/api/anthropic/v1",model:"glm-4.6",apiKey:""};
+function getSettings(){ try{ return {...DEFAULTS, ...JSON.parse(localStorage.getItem(LS)||"{}")}; }catch{ return {...DEFAULTS}; } }
+function saveSettings(s){ localStorage.setItem(LS, JSON.stringify(s)); refreshKeyStatus(); }
+function llmHeaders(){ const s=getSettings(); const h={}; if(s.apiKey){ h["X-LLM-Api-Key"]=s.apiKey;
+ if(s.provider) h["X-LLM-Provider"]=s.provider; if(s.baseUrl) h["X-LLM-Base-Url"]=s.baseUrl; if(s.model) h["X-LLM-Model"]=s.model; } return h; }
</code_context>
<issue_to_address>
**🚨 issue (security):** Storing raw API keys in `localStorage` increases the blast radius of any XSS in the viewer.
Because `localStorage` is readable by any script on this origin, any present or future XSS in this viewer could leak the BYOK key. If this UI is intended to be used beyond a tightly controlled local/dev context, consider keeping the key only in memory for the session, or otherwise constraining where this viewer can run and documenting the risk explicitly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| switch c.Driver { | ||
| case "anthropic": | ||
| return c.Anthropic.APIKey == "" | ||
| case "openai": |
There was a problem hiding this comment.
suggestion (bug_risk): OpenAI and Gemini branches ignore the baseURL argument, which may surprise callers of BuildLLM.
BuildLLM passes baseURL to buildLLMFrom, but the openai and gemini branches ignore it. Calls that set X-LLM-Base-Url for these providers will have no effect. Please either plumb baseURL into the OpenAI/Gemini client configs (as with Anthropic) or make it explicit in the API/docs that this parameter only applies to Anthropic.
Suggested implementation:
case "openai":
if model == "" {
model = c.OpenAI.Model
}
if baseURL == "" {
baseURL = c.OpenAI.BaseURL
}
return openai.New(openai.Config{
APIKey: apiKey,
Model: model,
ReasoningModel: c.OpenAI.ReasoningModel,
BaseURL: baseURL,
}) case "gemini":
if model == "" {
model = c.Gemini.Model
}
if baseURL == "" {
baseURL = c.Gemini.BaseURL
}To fully wire baseURL for the Gemini provider, ensure the gemini case returns a client configured with the BaseURL field, mirroring the Anthropic/OpenAI patterns. For example, the body should look like:
case "gemini":
if model == "" {
model = c.Gemini.Model
}
if baseURL == "" {
baseURL = c.Gemini.BaseURL
}
return gemini.New(gemini.Config{
APIKey: apiKey,
Model: model,
ReasoningModel: c.Gemini.ReasoningModel,
BaseURL: baseURL,
})This assumes openai.Config and gemini.Config both support a BaseURL field; if they use a different field name, adjust accordingly.
Comment on lines
+464
to
+469
| async function loadPdf(docId, page){ | ||
| const wrap=document.getElementById("pvWrap"); | ||
| const srcUrl=E(`/v1/documents/${docId}/source`); | ||
| const openLink=document.getElementById("pvOpen"); if(openLink) openLink.href=srcUrl; | ||
| const fallback=()=>{ wrap.innerHTML=`<div class="pvmsg">Inline preview didn’t load — <a href="${srcUrl}" target="_blank" rel="noopener">open the source ↗</a>.</div>`; }; | ||
| if(!window.pdfjsLib){ fallback(); return; } |
There was a problem hiding this comment.
issue: The PDF preview is attempted for any document type, which can cause confusing errors for non-PDF uploads.
Since loadPdf/pdfjsLib.getDocument is always called, non-PDF docs (DOCX/HTML/MD/TXT) will hit /source with a non-PDF body and cause pdf.js failures or generic error messages. You already have content_type on the document, so consider only invoking the inline PDF viewer when content_type === 'application/pdf' (or similar), and otherwise skip pdf.js and just show the “open source” link to avoid these spurious errors.
| const LS="vl_llm_settings"; | ||
| const DEFAULTS={provider:"anthropic",baseUrl:"https://api.z.ai/api/anthropic/v1",model:"glm-4.6",apiKey:""}; | ||
| function getSettings(){ try{ return {...DEFAULTS, ...JSON.parse(localStorage.getItem(LS)||"{}")}; }catch{ return {...DEFAULTS}; } } | ||
| function saveSettings(s){ localStorage.setItem(LS, JSON.stringify(s)); refreshKeyStatus(); } |
There was a problem hiding this comment.
🚨 issue (security): Storing raw API keys in localStorage increases the blast radius of any XSS in the viewer.
Because localStorage is readable by any script on this origin, any present or future XSS in this viewer could leak the BYOK key. If this UI is intended to be used beyond a tightly controlled local/dev context, consider keeping the key only in memory for the session, or otherwise constraining where this viewer can run and documenting the risk explicitly.
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
hallelx2
deleted the
halleluyaholudele/hal-186-engine-zero-config-local-mode
branch
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
1 participant
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.
Consolidates the local/self-hosted slice onto main.
What's in here
Dockerfile.allinone+ entrypoint): engine + bundled Postgres + the local web UI in one container; published to Docker Hub + GHCR bydocker-allinone.yml. (HAL-185)X-LLM-*headers; the bundled UI has a settings panel to configure the key from the dashboard. (HAL-188)Local.Putfsyncs + source fetch retries on ErrNotFound — kills intermittentparse: fetch source: storage: object not found. (Closes HAL-319)/v1suffix for Anthropic-compatible gateways. (Closes HAL-318)localapp/): upload, structure map, treewalk Q&A with citations, sections-read panel, PDF page preview, favicon.Verification
go build ./...,go vet ./...clean; all 12 test packages pass.Closes HAL-185
Closes HAL-318
Closes HAL-319
Summary by Sourcery
Add local BYOK support, a bundled local web UI, and an all-in-one Docker image while hardening ingest and improving Anthropic gateway configuration.
New Features:
Bug Fixes:
Enhancements:
Build:
Deployment:
Documentation: