Repository navigation
feat: turn on DuckDuckGo web search for the chat demo deployment - #1116
Conversation
The compose passthrough for the four web-search env keys has existed since #414, but every default resolves to off and the box's own .env is untracked, so the running demo deployment has no web search. The audit lists it as a missing Claude-defining surface. Enablement moves into deploy-demo-box.yml's workflow env block, which is the versioned place where this specific deployment turns the feature on: shell environment overrides --env-file during compose interpolation (validated with docker compose config against both states), so the four values land in the open-webui container without flipping the shared defaults that the enterprise profile relies on staying off. duckduckgo is the engine because the pinned v0.10.2 backend supports it with zero API key or account signup. Both BYPASS flags keep results snippet-only, which decouples the feature from the RAG embedding path entirely. Metering note: web search itself is not model spend. DuckDuckGo returns free unmetered snippets; only the chat completion that consumes them is metered exactly as any other completion. This change adds nothing to billing.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reachedNext included review available in 54 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
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 |
|
Review streams CodeRabbit CLI ( Plain adversarial pass (this builder, hunting for failure modes rather than style):
No blocking finding from either stream. |
Visual proofWeb search in chat, live on a throwaway stack built from this branch: composer Web Search toggle active (globe pill), model invokes the injected search_web builtin, live DuckDuckGo results return, reply renders with a search_web citation row and a Sources chip.
|
sakibsadmanshajib
left a comment
sakibsadmanshajib
left a comment
There was a problem hiding this comment.
Verdict: NOT-READY
Independent adversarial pass over the full two-file diff. What checks out: all four config keys are real against the pinned image's config.py (ENABLE_WEB_SEARCH, WEB_SEARCH_ENGINE, both BYPASS_WEB_SEARCH_* flags); the duckduckgo engine value is a supported dispatch target (retrieval.py line 2335, retrieval/web/duckduckgo.py present); ddgs==9.14.4 is pinned in the backend requirements; the compose passthrough exists at deploy/docker/docker-compose.yml lines 866 to 869; the workflow-env-over---env-file precedence claim is correct; the enterprise profile never sees these four values because they live only in this workflow.
Findings:
- MEDIUM, silent no-op risk.
web.search.*keys are persistent-config backed: runtime reads go throughConfig.get('web.search.enable'),seed_defaultsinserts everyDEFAULT_CONFIGkey into the config table on first boot, and existing DB rows take precedence over env forever (vendor/open-webui/backend/open_webui/models/config.py). On any box whose config table predates this change,ENABLE_WEB_SEARCH=truechanges the default only while the storedfalsekeeps search off, and the deploy reports green anyway. Nothing in the diff detects this. Before merge: verify against the running box that web search actually serves (a real search-backed answer, which also satisfies the visual-proof rule for a chat-surface change), or assert the effective values fromGET /api/v1/retrieval/configin the post-deploy verification job. - LOW.
RatelimitExceptioncollapses to an empty result list behind alog.error; the user sees a search-less answer with zero surfaced failure. Upstream-inherent, acceptable for demo, worth knowing about when someone reports "search did nothing". - INFO. With both bypass flags set, raw third-party snippets enter model context unfiltered; enabling search widens the prompt-injection surface. Accepted feature risk, stated here so it is on record.
No trivial defect in the diff itself, nothing pushed.
Review on rev 1 caught the first-boot-wins trap: the pinned backend's Config.seed_defaults only fills absent keys, so the demo box's chat database has carried web.search.enable=false, an empty engine and both bypass flags false since its first boot, and the workflow-env flip alone would have been a silent no-op in production. Confirmed read-only against the box's config table (61 web.search rows) before writing code. The four web.search keys now ride the #722 reconcile (owui-patches/hive_rag_env_config.py): they follow the container environment when it names them, so the demo deploy writes true/duckduckgo/true/true over the stale rows on the next boot of the reconcile-carrying image, and the enterprise profile resolves them off so its opt-in ruling stands. Proven against a hand-seeded stale database: the patched image flipped all four rows at boot and logged the reconcile summary. Also fixes the silent rate limit: the pinned engine wrapper swallowed RatelimitException into an empty list, so a rate-limited DuckDuckGo was indistinguishable from an empty web. The new build-time patch re-raises it, landing the error where both paths surface it: the native search_web builtin returns an error payload the model can see and relay, and the legacy path emits its visible error status event. Asserts exact pinned literals so a digest bump breaks the build loudly instead of silently reverting. Demo box enabled by hand today (DB backup first, four rows updated, container recreated with the four env vars), the same effect the PR produces; the next merged deploy re-asserts it from workflow env.
Visual proofLive on the demo box through chat-hive.scubed.co after the reconcile enablement: Hive-branded chat, Web Search globe pill active, model deepseek-v4-flash, turn shows Retrieved 3 sources from live DuckDuckGo. The completion text below it errored only because the verification used a throwaway locally-created chat account with no stored OAuth session for the gateway forward; every real SSO user carries that token.
|
|
Closing the rev 2 review findings MEDIUM (silent no-op on persisted config): CLOSED, with the mechanism shipped rather than a manual step alone. Ground truth first: the pinned image's LOW (rate limit silently empty): CLOSED with code. The pinned engine wrapper swallowed INFO (bypass flags widen injection surface): acknowledged in the PR body under its own heading; accepted for the demo posture (five users, snippets truncated to the configured result count of 3), enterprise stays off by default. No threads remain to resolve (zero inline comments on the PR); this comment is the reply to the review verdict. |
## Summary Reconciles the backlog created by the branch-append restriction in issue #873: every fixed bug, error, failed test, or failed build must be logged in `.wolf/buglog.jsonl`, but never appended directly on a feature branch, since GitHub's server-side merge ignores the `merge=union` driver and two branches that both appended land in hard conflict. The route is to carry the entry in the fix PR's body and append it here afterward, in a dedicated buglog-only PR. This PR is that reconciliation, swept properly rather than trusting a short known list: - Searched all merged PRs whose body contains a "Buglog entry" heading (287 PRs matched via GitHub code search). - Extracted the JSON line following each heading (multiple headings per PR body handled correctly, e.g. PR #814 and PR #1203 each carry two matches, one a prose mention and one the real entry). - Deduplicated against the 314 entries already on `main`, both by `id` and by exact `error_message` text, plus deduplicated within this batch itself. - Result: **197 new entries from 167 source PRs**, spanning PR #787 through PR #1734. - Validated every extracted line has the four required fields (`error_message`, `root_cause`, `fix`, `tags`). All 297 raw extractions had them; zero were rejected as incomplete. - Five entries carried `tags` as a comma-separated string instead of an array (inconsistent with the rest of the file's schema). Normalized to an array by splitting on comma, content unchanged, nothing invented. - The five false-positive "Buglog entry" mentions that were prose references rather than real headings (PRs #1116, #1303 first match, #1438 first match, #814 first match, #1203 first match) were correctly skipped, either because no JSON followed or because the real entry was found at a later heading in the same body. ## Diff scope `.wolf/buglog.jsonl` only, 197 insertions, 0 deletions. No existing line touched (verified byte-identical against the first 314 lines pre-append). ## Test plan - [x] Every one of the 511 resulting lines parses as valid single-line JSON. - [x] `git show --stat` on the pushed commit shows exactly one file changed. - [x] First 314 lines diffed identical to `origin/main`'s current file. - [x] This is on the inert-path allowlist in `.github/workflows/ci.yml`, so the six required checks should report green without running their heavy steps. Refs #873
1 participant
feat: turn on DuckDuckGo web search for the chat demo deployment
Summary
The parity audit lists web search as a missing Claude-defining surface: Hive Chat could not search the web. The wiring for it has actually existed since #414, which added the compose passthrough for four environment keys on the open-webui service, but every default resolves to off and the demo box's own .env is untracked and hand-maintained, so the running demo deployment has had the feature sitting disabled behind defaults nobody sets. This change turns it on for the demo deployment only, through the versioned deploy definition instead of an untracked file.
What changed
.github/workflows/deploy-demo-box.yml: the workflow-level env block that already carries HIVE_COMPOSE_FLAGS now also carries ENABLE_WEB_SEARCH=true, WEB_SEARCH_ENGINE=duckduckgo, BYPASS_WEB_SEARCH_WEB_LOADER=true, BYPASS_WEB_SEARCH_EMBEDDING_AND_RETRIEVAL=true..env.example: the comment above the four keys now points at where demo enablement lives, so the documented story matches reality.No application code changed. No frontend change was needed: the fork's composer keeps upstream's native web-search entry (IntegrationsMenu toggle, active pill,
features.web_searchin the completion payload), which renders as soon as/api/configreportsfeatures.enable_web_search, gated on admin role or the default-onUSER_PERMISSIONS_FEATURES_WEB_SEARCH.Why this mechanism
Shell environment overrides --env-file during compose interpolation, so workflow env wins over anything the untracked box .env carries for these keys, while the shared compose defaults stay off. That preserves the opt-in ruling from #414's review round: docker-compose.yml is shared with the enterprise profile, which must not silently inherit live outbound search calls. Enterprise deployments do not read this workflow and stay off unless their operator opts in. Local dev stacks are equally unaffected.
Validated both resolution states locally with the exact demo flag set:
Engine choice
DuckDuckGo. The pinned v0.10.2 backend ships
retrieval/web/duckduckgo.py; it needs no API key and no account signup, so it works tonight with zero new infrastructure. SearXNG (the old Phase 26 plan) would need a container plus an engine URL and buys nothing for five users on one box. If DDG ever rate limits or breaks, every other engine in the same menu is reachable by changing one env value, no code.Metering note
Web search itself is not model spend and this change does not pretend to meter it. DuckDuckGo snippets are free and unmetered; only the chat completion that consumes the injected results is metered, exactly like any other completion through the gateway. Nothing new flows into billing from this feature.
Data flow honesty
Search runs inside the open-webui container and sends the derived query text to duckduckgo.com over outbound HTTPS. For the demo deployment (five users, controlled box) that is the accepted posture already verified live back in #414. The enterprise profile stays off precisely because a customer-hosted data-sovereign deployment should not gain outbound calls by default.
Live verification (this branch's image)
Throwaway stack: stub OpenAI-compatible LLM + the chat image built from this branch (
hive-open-webui:websearch-proof), env enabled exactly as the workflow sets it./api/configreports features.enable_web_search true.POST /api/v1/retrieval/process/web/searchreturns live DuckDuckGo results for a real query.Follow-up (not this PR): post-deploy-verify.yml could assert the four values resolve true in the resolved open-webui config so a future compose invocation without the workflow env cannot silently drop the feature. Left out of this PR deliberately: that file is heavily contended this week.
Buglog entry: none. No bug fixed here, this is enablement of existing capability.
Rev 2: the persisted-config trap, closed
Review verdict on rev 1 was right: web.search.* are DB-backed config in the pinned image. Config.seed_defaults "inserts keys that don't yet exist in the DB. Existing DB values take precedence over defaults" (models/config.py), so the demo box's chat database has carried web.search.enable = false, an empty engine, and both bypass flags false since its first boot. Read-only inspection of the box's config table (61 web.search rows) confirmed it before any code was written. The rev 1 workflow-env flip alone would have been a silent no-op in production.
The reconcile
The four keys now ride the existing first-boot-wins reconcile that #722 built for exactly this trap (owui-patches/hive_rag_env_config.py, spliced into seed_registered_defaults at build time):
The demo's workflow env sets all four, so the next deploy writes true/duckduckgo/true/true over the stale false rows. The enterprise profile resolves them off, so its opt-in ruling stands. An operator who leaves a variable unset keeps the persisted value (blank never clobbers for the string key; the boolean keys always follow compose resolution, same as the existing product-surface flags).
Manual step taken on the demo box (2026-08-24)
Because the reconcile ships only in this branch's image and the box runs main's image until merge, the feature was enabled on the box by hand today, the same effect the PR produces: the four rows were updated in the box's webui.db config table (backup taken first), and the open-webui container was recreated with the four env vars exported. The next merged deploy rebuilds the image with the reconcile and re-asserts the same values from workflow env, so the manual state and the PR's end state agree.
LOW: rate limits are no longer silent
The pinned engine wrapper swallowed RatelimitException into an empty list, making a rate-limited DuckDuckGo indistinguishable from "the web has nothing": the native search_web builtin returned [] and the model answered "no results" with no signal that search itself was down. A build-time patch (owui-patches/apply_web_search_ratelimit_patch.py) re-raises it instead, which lands the error where both paths surface it: the builtin returns {'error': ...} to the model, and the legacy path emits its visible "An error occurred while searching the web" status event. Asserts the exact pinned literals so a digest bump that shifts the engine wrapper breaks the build loudly.
INFO acknowledged: bypass flags widen the injection surface
Acknowledged. BYPASS_WEB_SEARCH_WEB_LOADER=true means result snippets are injected as context without a full-page fetch and without the RAG embedding/retrieval pipeline, so third-party snippet text reaches the model as untrusted context. That surface exists either way (the snippets would be injected after retrieval instead), and the demo posture accepts it: five users, controlled box, snippets truncated to the configured result count (3 on the box). The enterprise profile stays off by default, which is the posture that matters for the data-sovereign buyers. No code change taken from this one.
Rev 2 live evidence
After the manual box step, verified against the real deployment through chat-hive.scubed.co:
Honest limitation: the completion TEXT on the box errored with the fork's "chat session is not carrying a signed-in user token" message because the verification used a throwaway locally-created chat account, which has no stored OAuth session for hive_jwt_forward to forward to the gateway. Every real user signs in through Hive SSO and carries that token. The search half (the feature this change enables) is fully exercised on the box; the completion-text half is blocked by the throwaway account's missing OAuth session, a pre-existing deployment auth property unrelated to web search. The full cited-reply loop is proven end to end on the throwaway stack running this branch's image.
Reconcile proven against a seeded stale DB
A throwaway boot of the patched image against a hand-seeded webui.db carrying the box's exact stale rows flipped all four at boot (log line "hive: reconciled Open WebUI config from env: ... web.search.enable=True, web.search.engine=duckduckgo ..." plus a read-back showing true / "duckduckgo" / true / true). The rate-limit re-raise is present in the built image.