fix: validate time range support for explicit engines - #244
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 28 |
| Duplication | 14 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull request overview
This PR prevents misleading “empty result” responses when callers explicitly select engines together with time_range by validating each selected engine’s time-range capability via the SearXNG /config endpoint and failing closed when support can’t be confirmed.
Changes:
- Adds a pre-search validation step that checks
time_range_supportfor explicitly selected engines and rejects incompatible/unknown capability combinations. - Extends instance capability discovery with
getEngineTimeRangeSupport()to classify requested engines as supported/unsupported/unknown. - Updates tool schema documentation and adds unit/regression coverage for supported, unsupported, and
/config-unavailable paths.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/types.ts | Documents the new fail-closed behavior in the tool and time_range parameter descriptions. |
| src/search.ts | Adds validateTimeRangeSupport() and enforces it before executing searches when engines + time_range are provided. |
| src/instance-info.ts | Introduces engine-specific time_range_support capability resolution from aggregated /config data. |
| tests/unit/types.test.ts | Verifies schema/description text reflects the new validation behavior. |
| tests/unit/search.test.ts | Adds regression tests for supported/unsupported engines and /config unavailable behavior. |
| tests/unit/instance-info.test.ts | Adds unit test for getEngineTimeRangeSupport() classification behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if (values.every((value) => value === true)) { | ||
| supported.push(name); | ||
| } else if (values.some((value) => value === false)) { | ||
| unsupported.push(name); | ||
| } else { | ||
| unknown.push(name); | ||
| } |
|
Thanks for the contribution. I reviewed the exact current head ( Before this can be considered ready for another review, please:
All checks on the current head are green, but they do not exercise the partial- |
|
@sawork1987 Thanks again for contributing this. The changes requested in the existing review are still needed before the PR can move forward, particularly the fail-closed multi-instance handling, regression coverage, and README update. |
|
Pushed consolidated update in commit Implemented the requested fail-closed multi-instance handling: when any configured instance fails Added regression coverage for:
Updated README with the multi-instance capability requirement and failure behavior. Verification:
The full suite has one unrelated pre-existing failure in the proxy test |
4c896e7 to
3d13af5
Compare
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Treat any unreachable SearXNG /config instance as unknown when validating explicit time ranges, with regression coverage and README guidance. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
3d13af5 to
6d9448b
Compare
ihor-sokoliuk
left a comment
There was a problem hiding this comment.
Awesome PR! Thanks for your contribution!
Summary
time_rangeagainst each explicitly selected engine's live/configcapabilityenginesunchangedProblem
SearXNG intentionally skips an engine when a request supplies
time_rangebut that engine reportstime_range_support=false. With one explicitly selected engine, for example:SearXNG does not run the GitHub engine and returns an ordinary empty result without an unresponsive-engine error. The adapter currently forwards this parameter combination unchanged, so callers cannot distinguish "no matching repositories" from "the selected engine was skipped".
Behavior
time_range_support=true: search runs unchangedfalseor omits the capability: request fails before search with an actionable explanation/configunavailable: request fails rather than presenting an unchecked empty resultengines, or notime_range: existing behavior remains unchangedThe adapter does not silently remove
time_rangeand does not rewrite the query into engine-specific syntax.Verification
708/708passedgit diff --checkpassed🤖 Generated with Claude Code