fix(web-search): handled empty searxng result fallback
Treated SearXNG HTTP 200 responses with no usable sources and upstream engine failures as transient provider errors. Added a generic renderable-content guard so provider fallback continues instead of returning an invisible success.\n\nFixes #2571
This commit is contained in:
@@ -39,6 +39,7 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed `web_search` SearXNG fallback when HTTP 200 responses contain no usable results plus `unresponsive_engines`; SearXNG now raises a transient provider error, and the fallback loop rejects any provider response with no renderable content before formatting an invisible success ([#2571](https://github.com/can1357/oh-my-pi/issues/2571)).
|
||||
- Fixed `read` on a GitHub commit URL (`github.com/<owner>/<repo>/commit/<sha>`) returning the raw commit HTML page instead of structured content. `parseGitHubUrl` had no `commit` case, so commit URLs fell through to generic HTML rendering; they now resolve via the commits API and render as markdown (subject, author, stats, parents, full commit message, and a per-file unified diff), matching the existing blob/tree/issue/PR handling.
|
||||
- Fixed release runs being silently cancelled by a later `main` push, which left tagged versions (`v15.12.6` in the wild) without a GitHub Release or npm publish. The CI workflow's `concurrency` group was `${{ github.workflow }}-${{ github.ref }}`, and since the release-script commit + `v*` tag are pushed atomically to `refs/heads/main`, the release run shared the `CI-refs/heads/main` group with every subsequent push; `cancel-in-progress: true` then killed it before `release_binary` / `release_github` / `release_npm` could run, and no future run carried the release tag at HEAD. The group now resolves to a per-sha `release-<sha>` slot with `cancel-in-progress: false` whenever the push subject matches `chore: bump version to ` (the release-script convention) or `github.ref` is a `v*` tag (`workflow_dispatch` recovery), so release runs are isolated from PR/main churn ([#2564](https://github.com/can1357/oh-my-pi/issues/2564)).
|
||||
- Allowed `compat.streamIdleTimeoutMs: 0` in `models.yml`. The schema was `.positive()`, so the documented "set to 0 to disable" escape hatch was only reachable via the global env var ([#2422](https://github.com/can1357/oh-my-pi/issues/2422))
|
||||
|
||||
@@ -115,6 +115,15 @@ function formatForLLM(response: SearchResponse): string {
|
||||
return parts.join("\n");
|
||||
}
|
||||
|
||||
function hasRenderableSearchContent(response: SearchResponse): boolean {
|
||||
if (response.answer?.trim()) return true;
|
||||
if (response.sources.length > 0) return true;
|
||||
if (response.citations?.length) return true;
|
||||
if (response.relatedQuestions?.some(question => question.trim())) return true;
|
||||
if (response.searchQueries?.some(query => query.trim())) return true;
|
||||
return false;
|
||||
}
|
||||
|
||||
interface ExecuteSearchOptions {
|
||||
authStorage: AuthStorage;
|
||||
sessionId?: string;
|
||||
@@ -162,6 +171,10 @@ async function executeSearch(
|
||||
sessionId,
|
||||
});
|
||||
|
||||
if (!hasRenderableSearchContent(response)) {
|
||||
throw new SearchProviderError(provider.id, `${provider.label} returned no renderable search content.`, 204);
|
||||
}
|
||||
|
||||
const text = formatForLLM(response);
|
||||
|
||||
return {
|
||||
|
||||
@@ -281,9 +281,21 @@ export async function searchSearXNG(params: {
|
||||
});
|
||||
}
|
||||
|
||||
const limitedSources = sources.slice(0, numResults);
|
||||
if (limitedSources.length === 0 && response.unresponsive_engines?.length) {
|
||||
const upstreamFailures = response.unresponsive_engines
|
||||
.map(([engine, reason]) => `${engine}: ${reason}`)
|
||||
.join("; ");
|
||||
throw new SearchProviderError(
|
||||
"searxng",
|
||||
`SearXNG returned no usable results; upstream engines failed: ${upstreamFailures}`,
|
||||
503,
|
||||
);
|
||||
}
|
||||
|
||||
return {
|
||||
provider: "searxng",
|
||||
sources: sources.slice(0, numResults),
|
||||
sources: limitedSources,
|
||||
relatedQuestions: response.suggestions?.length ? response.suggestions : undefined,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -5,6 +5,7 @@ import * as path from "node:path";
|
||||
import type { FetchImpl } from "@oh-my-pi/pi-ai/types";
|
||||
import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import { searchSearXNG } from "@oh-my-pi/pi-coding-agent/web/search/providers/searxng";
|
||||
import { SearchProviderError } from "@oh-my-pi/pi-coding-agent/web/search/types";
|
||||
|
||||
describe("SearXNG web search provider", () => {
|
||||
afterEach(() => {
|
||||
@@ -228,4 +229,27 @@ describe("SearXNG web search provider", () => {
|
||||
|
||||
expect(captured.headers?.get("Authorization")).toBe("Bearer bearer-token");
|
||||
});
|
||||
|
||||
it("treats empty SearXNG results with upstream failures as a provider error", async () => {
|
||||
process.env.SEARXNG_ENDPOINT = "https://searx.example.org";
|
||||
|
||||
const fetchMock: FetchImpl = () =>
|
||||
Promise.resolve(
|
||||
new Response(
|
||||
JSON.stringify({
|
||||
results: [],
|
||||
unresponsive_engines: [
|
||||
["brave", "Suspended: too many requests"],
|
||||
["duckduckgo", "CAPTCHA"],
|
||||
],
|
||||
}),
|
||||
{ status: 200, headers: { "Content-Type": "application/json" } },
|
||||
),
|
||||
);
|
||||
|
||||
await expect(searchSearXNG({ query: "throttled search", fetch: fetchMock })).rejects.toThrow(SearchProviderError);
|
||||
await expect(searchSearXNG({ query: "throttled search", fetch: fetchMock })).rejects.toThrow(
|
||||
"SearXNG returned no usable results; upstream engines failed: brave: Suspended: too many requests; duckduckgo: CAPTCHA",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -160,11 +160,13 @@ describe("Brave provider hard-timeout wiring", () => {
|
||||
describe("executeSearch abort propagation", () => {
|
||||
afterEach(() => vi.restoreAllMocks());
|
||||
|
||||
function fakeProvider(behaviour: (params: SearchParams) => Promise<SearchResponse>): provider.SearchProvider {
|
||||
const id: SearchProviderId = "anthropic";
|
||||
function fakeProvider(
|
||||
id: SearchProviderId,
|
||||
behaviour: (params: SearchParams) => Promise<SearchResponse>,
|
||||
): provider.SearchProvider {
|
||||
return {
|
||||
id,
|
||||
label: "Anthropic",
|
||||
label: id,
|
||||
isAvailable: () => true,
|
||||
isExplicitlyAvailable: () => true,
|
||||
search: behaviour,
|
||||
@@ -178,10 +180,10 @@ describe("executeSearch abort propagation", () => {
|
||||
// re-throw stops the loop immediately.
|
||||
const secondProviderSearch = vi.fn();
|
||||
vi.spyOn(provider, "resolveProviderChain").mockResolvedValue([
|
||||
fakeProvider(async () => {
|
||||
fakeProvider("anthropic", async () => {
|
||||
throw new DOMException("aborted", "AbortError");
|
||||
}),
|
||||
fakeProvider(secondProviderSearch),
|
||||
fakeProvider("brave", secondProviderSearch),
|
||||
]);
|
||||
|
||||
const tool = new WebSearchTool(FAKE_SESSION);
|
||||
@@ -197,7 +199,7 @@ describe("executeSearch abort propagation", () => {
|
||||
// flow. A genuine provider error should still produce an error result
|
||||
// rather than throwing.
|
||||
vi.spyOn(provider, "resolveProviderChain").mockResolvedValue([
|
||||
fakeProvider(async () => {
|
||||
fakeProvider("anthropic", async () => {
|
||||
throw new Error("upstream 500");
|
||||
}),
|
||||
]);
|
||||
@@ -209,4 +211,33 @@ describe("executeSearch abort propagation", () => {
|
||||
expect(block && "text" in block ? block.text : "").toContain("upstream 500");
|
||||
expect(result.details?.error).toContain("upstream 500");
|
||||
});
|
||||
|
||||
it("falls through when a provider returns no renderable search content", async () => {
|
||||
const emptyProviderSearch = vi.fn(
|
||||
async (): Promise<SearchResponse> => ({
|
||||
provider: "searxng",
|
||||
sources: [],
|
||||
}),
|
||||
);
|
||||
const sourceProviderSearch = vi.fn(
|
||||
async (): Promise<SearchResponse> => ({
|
||||
provider: "brave",
|
||||
sources: [{ title: "Fallback result", url: "https://example.com/fallback", snippet: "fallback body" }],
|
||||
}),
|
||||
);
|
||||
vi.spyOn(provider, "resolveProviderChain").mockResolvedValue([
|
||||
fakeProvider("searxng", emptyProviderSearch),
|
||||
fakeProvider("brave", sourceProviderSearch),
|
||||
]);
|
||||
|
||||
const tool = new WebSearchTool(FAKE_SESSION);
|
||||
const result = await tool.execute("test-id", { query: "anything" });
|
||||
|
||||
expect(emptyProviderSearch).toHaveBeenCalledTimes(1);
|
||||
expect(sourceProviderSearch).toHaveBeenCalledTimes(1);
|
||||
const block = result.content[0];
|
||||
expect(block?.type).toBe("text");
|
||||
expect(block && "text" in block ? block.text : "").toContain("Fallback result");
|
||||
expect(result.details?.response.provider).toBe("brave");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user