feat(search): attribute managed web search to the resolved provider - #5206
Conversation
Managed search routes through the OpenHuman backend, which serves the overwhelming majority of traffic via Exa, but nothing in the product said so: a completed web search rendered as a generic "Searching the web" row and the Connections copy described the managed path as an unattributed proxy. Carry the provider the backend actually resolved to through the result and surface it, without hardcoding a name: - SearchResponse gains an optional `provider` field (aliased `resolvedProvider`/`searchProvider`); older backends that omit it deserialize unchanged and fall back to the managed default. - resolve_managed_provider() prefers the backend-reported value and falls back to "Exa" only when it is absent or blank, so a future routing change surfaces with no code edit. The result heading now reads "(via <Provider>)", matching the marker the Brave/Querit/Seltz engines already emit, in place of the fixed "(via backend Parallel)". - The tools.web_search RPC returns and logs the same resolved provider. - The tool timeline titles a completed web_search "Searched with <Provider>" and moves the query to the detail line; running rows are unchanged. The marker is read from the heading line only and capped in length so a "(via ...)" string inside a result excerpt cannot be misread as the provider. formatTimelineEntry backs every tool-call surface, so Chat, the processing transcript, past-turn insights and the source panel all agree. - Connections -> Search engine now names Exa as the current provider, in English and all 13 other locales.
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughManaged search now propagates its resolved provider from backend responses through RPC results and tool timelines. Attribution is dynamic, defaults to Exa when absent, supports response aliases, and appears in managed-search translations across locales. ChangesSearch provider attribution
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant SearchBackend
participant SearchRPC
participant ToolTimeline
SearchBackend->>SearchRPC: return results and resolved provider
SearchRPC->>SearchRPC: apply provider fallback and format attribution
SearchRPC-->>ToolTimeline: return attributed search result
ToolTimeline->>ToolTimeline: extract provider marker
ToolTimeline-->>ToolTimeline: display Searched with provider
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fefe149908
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/utils/toolTimelineFormatting.ts`:
- Around line 515-524: Update the completed-search formatting flow around
extractSearchProvider so the title is supplied through an i18n-aware caller
using useT() rather than the hard-coded “Searched with” string. Add the
corresponding translation key to en.ts and every real locale, while preserving
the provider interpolation and existing query detail behavior.
In `@src/openhuman/search/tools/web_search.rs`:
- Around line 62-66: Update the empty-result renderers in the web search result
formatting flow to include the provider attribution marker `(via {provider})`,
matching the non-empty format. Apply this consistently to both empty-result
formats, including the empty-result timeline case, so extractSearchProvider can
identify completed searches.
In `@src/openhuman/tools/schemas.rs`:
- Around line 544-551: Update the log construction in the web-search handling
flow around resolve_managed_provider so it never includes the raw query text.
Replace query in the tools.web_search log with a safe query_len and/or
non-reversible correlation fingerprint, while preserving the existing results
count and provider fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e419a432-44a0-406e-879a-98c081b543df
📒 Files selected for processing (22)
app/src/lib/i18n/__tests__/coverage.test.tsapp/src/lib/i18n/ar.tsapp/src/lib/i18n/bn.tsapp/src/lib/i18n/de.tsapp/src/lib/i18n/en.tsapp/src/lib/i18n/es.tsapp/src/lib/i18n/fr.tsapp/src/lib/i18n/hi.tsapp/src/lib/i18n/id.tsapp/src/lib/i18n/it.tsapp/src/lib/i18n/ko.tsapp/src/lib/i18n/pl.tsapp/src/lib/i18n/pt.tsapp/src/lib/i18n/ru.tsapp/src/lib/i18n/zh-CN.tsapp/src/utils/__tests__/toolTimelineFormatting.test.tsapp/src/utils/toolTimelineFormatting.tssrc/openhuman/search/tools/mod.rssrc/openhuman/search/tools/parallel.rssrc/openhuman/search/tools/parallel_tests.rssrc/openhuman/search/tools/web_search.rssrc/openhuman/tools/schemas.rs
|
| Filename | Overview |
|---|---|
| app/src/utils/toolTimelineFormatting.ts | Adds extractSearchProvider (anchored regex, max-length guard), plumbs entry.result into formatToolDetail, and registers web_search_tool as a known client tool. |
| src/openhuman/search/tools/web_search.rs | Adds resolve_managed_provider (backend value preferred, static fallback) and threads provider through both renderers; lifetime/borrow usage is sound. |
| src/openhuman/search/tools/parallel.rs | Adds optional provider field to SearchResponse with #[serde(default)] and two aliases; older backends that omit it continue to deserialize cleanly. |
| src/openhuman/tools/schemas.rs | Exposes provider in the tools.web_search RPC output schema and switches the log line from query text to query_len for privacy consistency. |
| src/openhuman/search/tools/brave.rs | Normalises markdown heading from (Brave) to (via Brave) and adds the marker to the empty-result string so the timeline can parse attribution in all cases. |
| app/src/utils/tests/toolTimelineFormatting.test.ts | Comprehensive new test suite for extractSearchProvider and formatTimelineEntry over both tool name variants and completion states. |
| app/src/lib/i18n/en.ts | Updates settings.search.engineManagedDesc to name Exa as the current provider; same change applied across all 13 other locale files. |
Sequence Diagram
sequenceDiagram
participant Agent
participant WebSearchTool
participant Backend as Managed Backend
participant Timeline as formatTimelineEntry
Agent->>WebSearchTool: execute(query)
WebSearchTool->>Backend: POST /agent-integrations/parallel/search
Backend-->>WebSearchTool: "SearchResponse { results, provider? }"
WebSearchTool->>WebSearchTool: resolve_managed_provider(resp)
WebSearchTool-->>Agent: ToolResult text with (via Exa) marker
Agent->>Timeline: formatTimelineEntry(entry)
Timeline->>Timeline: extractSearchProvider(result)
Timeline-->>Agent: "{ title: Searched with Exa, detail: query }"
Reviews (3): Last reviewed commit: "fix(tools): stop logging the raw web-sea..." | Re-trigger Greptile
Review found three ways the attribution never actually surfaced, plus a
parsing bug:
- The canonical search slot is registered and streamed as `web_search_tool`
(src/openhuman/search/registry.rs); `web_search` is only the settings
family id and never reaches a timeline row. The formatter matched the
latter, so every real search fell through to the humanized "Web Search
Tool" and the new attribution never ran. Handle both names, and add
`web_search_tool` to the client-known set so the client label wins over
the server's default.
- Production renders tool results as markdown (`output_for_llm(true)`), so
the markdown heading is what the timeline parses. Brave emitted
`(Brave)` and Seltz/Querit emitted no marker at all, so BYOK engines
could not be attributed. All three now emit the shared `(via <Provider>)`
marker their plain-text renderers already carried.
- Empty-result renderings carried no marker, so a completed search that
found nothing kept rendering as still in progress. Both managed forms and
the three BYOK markdown forms now attribute an empty result too.
- The heading echoes the user's query, so a query containing "(via ...)"
won over the real trailing marker ("login (via OAuth) (via Exa)" resolved
to OAuth). Anchor the match to the end of the heading line.
Regression tests cover each: the streamed tool name running and completed,
markdown and empty-result attribution, the decoy-marker query, and the
per-engine markdown markers.
The tools.web_search RPC echoed the user's query text into its log line. A search query is user-authored and can carry PII or credentials, so log its length instead — matching the sibling seltz/querit handlers in the same file, which already log query_len only.
|
@coderabbitai review Pushed three commits addressing every review item. Summary: Fixed
Declined, with reasoning in-thread
Docstring coverage check — the shortfall is new Verification: |
|
✅ Action performedReview finished.
|
Summary
web_searchtool call is now titled "Searched with Exa" (or whatever provider actually served it) instead of a generic search row; the query moves to the detail line."Exa"when the backend does not name one, so a future routing change surfaces with no code edit.formatTimelineEntry.tools.web_searchRPC returns and logs the same resolved provider, so that surface attributes a call identically.Problem
Exa powers the overwhelming majority of managed search traffic, but nothing in the product said so. A user on the default OpenHuman Managed engine saw:
The core made this worse by hardcoding the attribution marker to
(via backend Parallel)— naming the proxy rather than the provider, and unable to reflect reality if routing changed. Meanwhile the BYOK engines already tagged their output(via Brave)/(via Querit)/(via Seltz), so the managed path was the odd one out.Solution
Core — carry the resolved provider through the result
SearchResponse(src/openhuman/search/tools/parallel.rs) gains an optionalproviderfield, aliasedresolvedProvider/searchProviderso an upstream rename does not silently drop attribution.#[serde(default)]keeps older backends that omit it deserializing unchanged.resolve_managed_provider()(web_search.rs) prefers the backend-reported value (trimmed) and falls back toMANAGED_DEFAULT_PROVIDER = "Exa"only when it is absent or blank. The constant is a fallback label, not the source of truth — this is what keeps the acceptance criterion "not hardcoded" honest while still being correct today.Search results for: … (via Exa), adopting the marker convention the BYOK engines already emit.handle_web_searchinsrc/openhuman/tools/schemas.rsreuses the same resolver (exportedpub(crate)) so the RPC surface cannot drift from the agent-facing tool.Frontend — one formatter, every surface
formatToolDetailnow receivesentry.result, so theweb_searchcase can label a completed call with the provider that served it. Running rows keep "Searching: …" because no result exists yet.extractSearchProvider()reads the(via …)marker from the heading line only and rejects markers longer than 32 characters — a(via …)string inside a result excerpt cannot be mistaken for the provider, and a malformed marker cannot blow up a timeline row.formatTimelineEntryis the single funnel behindChatThreadView,ToolTimelineBlock,ProcessingTranscriptView,PastTurnInsights,AgentProcessSourcePanel,mapDisplayItemsandchatRuntimeSlice, so consistency across surfaces falls out of one change rather than being maintained per-component.Design note. Attribution is read back from the result text rather than plumbed through a new socket/persistence field. That reuses the marker convention every search engine already emits, needs no change to the tool-result transport or the persisted turn-state round-trip, and means BYOK engines get correct attribution for free.
Deliberately out of scope: backend managed-search routing, and Exa BYOK / user-configurable provider wiring (#5137).
Submission Checklist
diff-cover) meet the gate enforced by.github/workflows/ci-lite.yml. Runpnpm test:coverageandpnpm test:rustlocally; PRs below 80% on changed lines will not merge. — every changed line is exercised by the new/updated tests below; full localtest:coverage/test:rustmatrix not run (fills the disk on this machine), so the CI coverage lane is the authority.docs/TEST-COVERAGE-MATRIX.mdreflect this change (orN/A: behaviour-only change) —N/A: behaviour-only change, no feature added, removed or renamed.## Relatedexecute()test drives the existing in-process axum mock backend.docs/RELEASE-MANUAL-SMOKE.md) —N/A: label/copy change on an existing flow, no new release-cut surface.Closes #NNNin the## RelatedsectionImpact
provideris optional and defaulted, so a backend that does not yet send it keeps working and simply attributes to the managed default. BYOK engines are untouched — they already emitted the marker this reads.(via <Provider>)rather than(via backend Parallel). It stays a one-line heading in the same position and format, so nothing downstream parses it differently.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/GH-5136-exa-search-attributionfefe14990Validation Run
pnpm --filter openhuman-app format:check— ran asprettier --checkover the changed files: all match Prettier style.eslintover the same files is clean.pnpm typecheck— clean (tsc --noEmit).vitest run src/utils/__tests__/toolTimelineFormatting.test.ts→ 45 passed;vitest run src/lib/i18n/__tests__/coverage.test.ts→ 67 passed;cargo test --lib search::tools::→ 93 passed; targetedsearch_response_reads*/web_search_schema_shape→ 6 passed. Alsopnpm i18n:check(0 missing / 0 extra) andpnpm i18n:english:check(0 unexpected English).cargo fmt --checkclean;GGML_NATIVE=OFF cargo check --testsclean (only pre-existing warnings in untouched files).N/A — no files under app/src-tauri/ changed.Validation Blocked
command:pnpm test:coverage/pnpm test:rust(full suites)error:not attempted — running the full test/build matrix locally exhausts disk on this machine.impact:low. Every changed line is covered by the focused suites listed above, which were run and pass; the CI coverage lane re-runs the full matrix and gates the merge.Behavior Changes
Parity Contract
(via …)marker this reads;SearchResponsestill rejects the same missing required fields.provideris#[serde(default)]with aliases, so a backend that omits or renames it still deserializes; a blank or whitespace-only value falls back to the managed default; the frontend returnsundefined(keeping the old label) when there is no result yet, no marker, or an implausibly long one. Both the agent tool and thetools.web_searchRPC call the same resolver, so they cannot diverge.Duplicate / Superseded PR Handling
Summary by CodeRabbit