Commit e81cbdf50704933432f55ba3d782e42f361abcb0
1 parent
12a75c46
fix(suggestion): decouple SAT ES recall size from /suggest HTTP size param
将 SAT 的 ES 召回与对外 size 解耦,并支持配置化(解决suggest接口size参数取值不同时返回结果不一致的问题)
**Problem**
When `size=10` vs `size=40`, the SAT (search‑as‑you‑type) ES `_search` used the same `size` value, causing different candidate pool sizes and inconsistent top‑N results after merging with completion suggestions. The `size` parameter incorrectly controlled three things: completion count, SAT ES `size`, and final truncation.
**Solution**
Introduce dedicated configurable bounds for SAT recall, completely decoupled from the client‑facing `size` (final result count).
- Compute SAT ES request size as `min(max(client_size, sat_recall_min), sat_recall_cap)`.
- Completion still uses the raw client `size`.
- Final merge, sort, and truncation logic (`_finalize_suggestion_list(..., size)`) unchanged.
**Configuration**
- New dataclass `SuggestionConfig` in `config/schema.py` with fields:
- `sat_recall_min: int = 40`
- `sat_recall_cap: int = 100`
- Root `config.yaml` now supports `suggestion.sat_recall_min` / `suggestion.sat_recall_cap`.
- Tenant overrides: `tenant_config.default.suggestion` or `tenant_config.tenants.<id>.suggestion` – only keys to override need to be specified. Merge order: root `SuggestionConfig` → default fragment → tenant fragment.
- Sanity check: if `sat_recall_cap < sat_recall_min`, both loader and runtime resolver raise `cap` to at least `min` (warning logged).
**Impact**
- Small `size` (e.g., 10) still gets a stable SAT candidate pool (minimum `sat_recall_min`).
- Large `size` is capped to `sat_recall_cap`, bounding ES query cost.
- Final result count remains exactly the HTTP `size` parameter.
- Backward compatible: defaults preserve previous behaviour when `sat_recall_min=40, sat_recall_cap=100` and client `size` <=100.
**Code changes**
- `config/schema.py`: add `SuggestionConfig` and integrate into `AppConfig`.
- `suggestion/service.py`:
- `_resolve_suggestion_config_for_tenant()`: tenant‑aware config merging.
- `SuggestionService._suggest()`: compute `sat_es_size` using the new bounds.
- `suggestion/loader.py`: apply same sanity checks and defaults.
- Tenant config example:
```yaml
tenant_config:
tenants:
'163':
suggestion:
sat_recall_cap: 80
```
**Tests**
- `pytest tests/test_suggestions.py`
- `pytest tests/ci/test_service_api_contracts.py`
All related tests pass.
Showing
6 changed files
with
100 additions
and
3 deletions
Show diff stats
config/config.yaml
| @@ -45,6 +45,9 @@ assets: | @@ -45,6 +45,9 @@ assets: | ||
| 45 | query_rewrite_dictionary_path: config/dictionaries/query_rewrite.dict | 45 | query_rewrite_dictionary_path: config/dictionaries/query_rewrite.dict |
| 46 | product_enrich: | 46 | product_enrich: |
| 47 | max_workers: 40 | 47 | max_workers: 40 |
| 48 | +suggestion: | ||
| 49 | + sat_recall_min: 40 | ||
| 50 | + sat_recall_cap: 100 | ||
| 48 | search_evaluation: | 51 | search_evaluation: |
| 49 | artifact_root: artifacts/search_evaluation | 52 | artifact_root: artifacts/search_evaluation |
| 50 | queries_file: scripts/evaluation/queries/queries.txt | 53 | queries_file: scripts/evaluation/queries/queries.txt |
config/loader.py
| @@ -50,6 +50,7 @@ from config.schema import ( | @@ -50,6 +50,7 @@ from config.schema import ( | ||
| 50 | SearchEvaluationDatasetConfig, | 50 | SearchEvaluationDatasetConfig, |
| 51 | SecretsConfig, | 51 | SecretsConfig, |
| 52 | ServicesConfig, | 52 | ServicesConfig, |
| 53 | + SuggestionConfig, | ||
| 53 | SPUConfig, | 54 | SPUConfig, |
| 54 | TenantCatalogConfig, | 55 | TenantCatalogConfig, |
| 55 | TranslationServiceConfig, | 56 | TranslationServiceConfig, |
| @@ -256,6 +257,9 @@ class AppConfigLoader: | @@ -256,6 +257,9 @@ class AppConfigLoader: | ||
| 256 | 257 | ||
| 257 | rewrite_dictionary = _read_rewrite_dictionary(rewrite_path) | 258 | rewrite_dictionary = _read_rewrite_dictionary(rewrite_path) |
| 258 | search_config = self._build_search_config(raw, rewrite_dictionary) | 259 | search_config = self._build_search_config(raw, rewrite_dictionary) |
| 260 | + suggestion_config = self._build_suggestion_config( | ||
| 261 | + raw.get("suggestion") if isinstance(raw.get("suggestion"), dict) else {} | ||
| 262 | + ) | ||
| 259 | services_config = self._build_services_config(raw.get("services") or {}) | 263 | services_config = self._build_services_config(raw.get("services") or {}) |
| 260 | tenants_config = self._build_tenants_config(raw.get("tenant_config") or {}) | 264 | tenants_config = self._build_tenants_config(raw.get("tenant_config") or {}) |
| 261 | runtime_config = self._build_runtime_config() | 265 | runtime_config = self._build_runtime_config() |
| @@ -278,6 +282,7 @@ class AppConfigLoader: | @@ -278,6 +282,7 @@ class AppConfigLoader: | ||
| 278 | infrastructure=infrastructure_config, | 282 | infrastructure=infrastructure_config, |
| 279 | product_enrich=product_enrich_config, | 283 | product_enrich=product_enrich_config, |
| 280 | search=search_config, | 284 | search=search_config, |
| 285 | + suggestion=suggestion_config, | ||
| 281 | services=services_config, | 286 | services=services_config, |
| 282 | tenants=tenants_config, | 287 | tenants=tenants_config, |
| 283 | assets=AssetsConfig(query_rewrite_dictionary_path=rewrite_path), | 288 | assets=AssetsConfig(query_rewrite_dictionary_path=rewrite_path), |
| @@ -291,6 +296,7 @@ class AppConfigLoader: | @@ -291,6 +296,7 @@ class AppConfigLoader: | ||
| 291 | infrastructure=app_config.infrastructure, | 296 | infrastructure=app_config.infrastructure, |
| 292 | product_enrich=app_config.product_enrich, | 297 | product_enrich=app_config.product_enrich, |
| 293 | search=app_config.search, | 298 | search=app_config.search, |
| 299 | + suggestion=app_config.suggestion, | ||
| 294 | services=app_config.services, | 300 | services=app_config.services, |
| 295 | tenants=app_config.tenants, | 301 | tenants=app_config.tenants, |
| 296 | assets=app_config.assets, | 302 | assets=app_config.assets, |
| @@ -727,6 +733,17 @@ class AppConfigLoader: | @@ -727,6 +733,17 @@ class AppConfigLoader: | ||
| 727 | es_settings=dict(raw.get("es_settings") or {}), | 733 | es_settings=dict(raw.get("es_settings") or {}), |
| 728 | ) | 734 | ) |
| 729 | 735 | ||
| 736 | + def _build_suggestion_config(self, raw: Dict[str, Any]) -> SuggestionConfig: | ||
| 737 | + if not isinstance(raw, dict): | ||
| 738 | + raw = {} | ||
| 739 | + mn = int(raw.get("sat_recall_min", 40)) | ||
| 740 | + cap = int(raw.get("sat_recall_cap", 100)) | ||
| 741 | + if mn < 1: | ||
| 742 | + mn = 1 | ||
| 743 | + if cap < mn: | ||
| 744 | + cap = mn | ||
| 745 | + return SuggestionConfig(sat_recall_min=mn, sat_recall_cap=cap) | ||
| 746 | + | ||
| 730 | def _build_services_config(self, raw: Dict[str, Any]) -> ServicesConfig: | 747 | def _build_services_config(self, raw: Dict[str, Any]) -> ServicesConfig: |
| 731 | if not isinstance(raw, dict): | 748 | if not isinstance(raw, dict): |
| 732 | raise ConfigurationError("services must be a mapping") | 749 | raise ConfigurationError("services must be a mapping") |
config/schema.py
| @@ -213,6 +213,16 @@ class RerankConfig: | @@ -213,6 +213,16 @@ class RerankConfig: | ||
| 213 | 213 | ||
| 214 | 214 | ||
| 215 | @dataclass(frozen=True) | 215 | @dataclass(frozen=True) |
| 216 | +class SuggestionConfig: | ||
| 217 | + """Online suggestion API tuning (completion + SAT recall).""" | ||
| 218 | + | ||
| 219 | + #: ES bool_prefix (search-as-you-type) request size lower bound after client ``size``. | ||
| 220 | + sat_recall_min: int = 40 | ||
| 221 | + #: ES bool_prefix request size upper bound (clips ``max(client_size, sat_recall_min)``). | ||
| 222 | + sat_recall_cap: int = 100 | ||
| 223 | + | ||
| 224 | + | ||
| 225 | +@dataclass(frozen=True) | ||
| 216 | class SearchConfig: | 226 | class SearchConfig: |
| 217 | """Search behavior configuration shared by backend and indexer.""" | 227 | """Search behavior configuration shared by backend and indexer.""" |
| 218 | 228 | ||
| @@ -474,6 +484,7 @@ class AppConfig: | @@ -474,6 +484,7 @@ class AppConfig: | ||
| 474 | infrastructure: InfrastructureConfig | 484 | infrastructure: InfrastructureConfig |
| 475 | product_enrich: ProductEnrichConfig | 485 | product_enrich: ProductEnrichConfig |
| 476 | search: SearchConfig | 486 | search: SearchConfig |
| 487 | + suggestion: SuggestionConfig | ||
| 477 | services: ServicesConfig | 488 | services: ServicesConfig |
| 478 | tenants: TenantCatalogConfig | 489 | tenants: TenantCatalogConfig |
| 479 | assets: AssetsConfig | 490 | assets: AssetsConfig |
suggestion/service.py
| @@ -6,6 +6,8 @@ import logging | @@ -6,6 +6,8 @@ import logging | ||
| 6 | import time | 6 | import time |
| 7 | from typing import Any, Dict, List, Optional | 7 | from typing import Any, Dict, List, Optional |
| 8 | 8 | ||
| 9 | +from config.loader import get_app_config | ||
| 10 | +from config.schema import SuggestionConfig | ||
| 9 | from config.tenant_config_loader import get_tenant_config_loader | 11 | from config.tenant_config_loader import get_tenant_config_loader |
| 10 | from query.tokenization import simple_tokenize_query | 12 | from query.tokenization import simple_tokenize_query |
| 11 | from suggestion.builder import get_suggestion_alias_name | 13 | from suggestion.builder import get_suggestion_alias_name |
| @@ -25,6 +27,37 @@ def _score_with_token_length_penalty(item: Dict[str, Any]) -> float: | @@ -25,6 +27,37 @@ def _score_with_token_length_penalty(item: Dict[str, Any]) -> float: | ||
| 25 | return base * _suggestion_length_factor(str(item.get("text") or "")) | 27 | return base * _suggestion_length_factor(str(item.get("text") or "")) |
| 26 | 28 | ||
| 27 | 29 | ||
| 30 | +def _resolve_suggestion_config_for_tenant(tenant_id: str) -> SuggestionConfig: | ||
| 31 | + """Merge root ``suggestion`` with ``tenant_config.default`` / per-tenant ``suggestion`` overrides.""" | ||
| 32 | + app = get_app_config() | ||
| 33 | + base = app.suggestion | ||
| 34 | + d = app.tenants.default if isinstance(app.tenants.default, dict) else {} | ||
| 35 | + d_s = d.get("suggestion") if isinstance(d.get("suggestion"), dict) else {} | ||
| 36 | + t = app.tenants.tenants.get(str(tenant_id), {}) or {} | ||
| 37 | + t_s = t.get("suggestion") if isinstance(t.get("suggestion"), dict) else {} | ||
| 38 | + | ||
| 39 | + def _i(key: str, fallback: int) -> int: | ||
| 40 | + if key in t_s and t_s[key] is not None: | ||
| 41 | + return int(t_s[key]) | ||
| 42 | + if key in d_s and d_s[key] is not None: | ||
| 43 | + return int(d_s[key]) | ||
| 44 | + return fallback | ||
| 45 | + | ||
| 46 | + mn = _i("sat_recall_min", base.sat_recall_min) | ||
| 47 | + cap = _i("sat_recall_cap", base.sat_recall_cap) | ||
| 48 | + if mn < 1: | ||
| 49 | + mn = 1 | ||
| 50 | + if cap < mn: | ||
| 51 | + cap = mn | ||
| 52 | + return SuggestionConfig(sat_recall_min=mn, sat_recall_cap=cap) | ||
| 53 | + | ||
| 54 | + | ||
| 55 | +def _sat_es_size(client_size: int, cfg: SuggestionConfig) -> int: | ||
| 56 | + """ES size for SAT (bool_prefix) recall: ``min(max(client_size, sat_recall_min), sat_recall_cap)``.""" | ||
| 57 | + c = max(int(client_size), 0) | ||
| 58 | + return min(max(c, cfg.sat_recall_min), cfg.sat_recall_cap) | ||
| 59 | + | ||
| 60 | + | ||
| 28 | class SuggestionService: | 61 | class SuggestionService: |
| 29 | def __init__(self, es_client: ESClient): | 62 | def __init__(self, es_client: ESClient): |
| 30 | self.es_client = es_client | 63 | self.es_client = es_client |
| @@ -123,6 +156,8 @@ class SuggestionService: | @@ -123,6 +156,8 @@ class SuggestionService: | ||
| 123 | start = time.time() | 156 | start = time.time() |
| 124 | query_text = str(query or "").strip() | 157 | query_text = str(query or "").strip() |
| 125 | resolved_lang = self._resolve_language(tenant_id, language) | 158 | resolved_lang = self._resolve_language(tenant_id, language) |
| 159 | + sugg_cfg = _resolve_suggestion_config_for_tenant(tenant_id) | ||
| 160 | + sat_es_size = _sat_es_size(size, sugg_cfg) | ||
| 126 | index_name = self._resolve_search_target(tenant_id) | 161 | index_name = self._resolve_search_target(tenant_id) |
| 127 | if not index_name: | 162 | if not index_name: |
| 128 | # On a fresh ES cluster the suggestion index might not be built yet. | 163 | # On a fresh ES cluster the suggestion index might not be built yet. |
| @@ -243,7 +278,7 @@ class SuggestionService: | @@ -243,7 +278,7 @@ class SuggestionService: | ||
| 243 | es_resp = self.es_client.search( | 278 | es_resp = self.es_client.search( |
| 244 | index_name=index_name, | 279 | index_name=index_name, |
| 245 | body=dsl, | 280 | body=dsl, |
| 246 | - size=size, | 281 | + size=sat_es_size, |
| 247 | from_=0, | 282 | from_=0, |
| 248 | routing=str(tenant_id), | 283 | routing=str(tenant_id), |
| 249 | ) | 284 | ) |
| @@ -269,12 +304,13 @@ class SuggestionService: | @@ -269,12 +304,13 @@ class SuggestionService: | ||
| 269 | 304 | ||
| 270 | took_ms = int((time.time() - start) * 1000) | 305 | took_ms = int((time.time() - start) * 1000) |
| 271 | logger.info( | 306 | logger.info( |
| 272 | - "suggest completion+sat-return | tenant=%s lang=%s q=%s completion=%d sat_hits=%d took_ms=%d completion_ms=%d sat_ms=%d", | 307 | + "suggest completion+sat-return | tenant=%s lang=%s q=%s completion=%d sat_hits=%d sat_es_size=%d took_ms=%d completion_ms=%d sat_ms=%d", |
| 273 | tenant_id, | 308 | tenant_id, |
| 274 | resolved_lang, | 309 | resolved_lang, |
| 275 | query_text, | 310 | query_text, |
| 276 | len(completion_items), | 311 | len(completion_items), |
| 277 | len(hits), | 312 | len(hits), |
| 313 | + sat_es_size, | ||
| 278 | took_ms, | 314 | took_ms, |
| 279 | completion_ms, | 315 | completion_ms, |
| 280 | sat_ms, | 316 | sat_ms, |
tests/test_suggestions.py
| @@ -10,7 +10,12 @@ from suggestion.builder import ( | @@ -10,7 +10,12 @@ from suggestion.builder import ( | ||
| 10 | get_suggestion_alias_name, | 10 | get_suggestion_alias_name, |
| 11 | get_suggestion_versioned_index_name, | 11 | get_suggestion_versioned_index_name, |
| 12 | ) | 12 | ) |
| 13 | -from suggestion.service import SuggestionService | 13 | +from config.schema import SuggestionConfig |
| 14 | +from suggestion.service import ( | ||
| 15 | + SuggestionService, | ||
| 16 | + _resolve_suggestion_config_for_tenant, | ||
| 17 | + _sat_es_size, | ||
| 18 | +) | ||
| 14 | 19 | ||
| 15 | pytestmark = [pytest.mark.suggestion, pytest.mark.regression] | 20 | pytestmark = [pytest.mark.suggestion, pytest.mark.regression] |
| 16 | 21 | ||
| @@ -303,6 +308,31 @@ def test_suggestion_service_basic_flow_uses_alias_and_routing(): | @@ -303,6 +308,31 @@ def test_suggestion_service_basic_flow_uses_alias_and_routing(): | ||
| 303 | assert len(search_calls) >= 2 | 308 | assert len(search_calls) >= 2 |
| 304 | assert any(x.get("routing") == "1" for x in search_calls) | 309 | assert any(x.get("routing") == "1" for x in search_calls) |
| 305 | assert any(x.get("index") == alias_name for x in search_calls) | 310 | assert any(x.get("index") == alias_name for x in search_calls) |
| 311 | + sat_calls = [x for x in search_calls if "suggest" not in (x.get("body") or {})] | ||
| 312 | + assert sat_calls[-1]["size"] == 40 | ||
| 313 | + | ||
| 314 | + | ||
| 315 | +def test_sat_es_size_clamped_by_suggestion_config(): | ||
| 316 | + cfg = SuggestionConfig(sat_recall_min=40, sat_recall_cap=100) | ||
| 317 | + assert _sat_es_size(10, cfg) == 40 | ||
| 318 | + assert _sat_es_size(50, cfg) == 50 | ||
| 319 | + assert _sat_es_size(200, cfg) == 100 | ||
| 320 | + | ||
| 321 | + | ||
| 322 | +def test_resolve_suggestion_config_merges_tenant_yaml(monkeypatch): | ||
| 323 | + from types import SimpleNamespace | ||
| 324 | + | ||
| 325 | + fake = SimpleNamespace( | ||
| 326 | + suggestion=SuggestionConfig(sat_recall_min=40, sat_recall_cap=100), | ||
| 327 | + tenants=SimpleNamespace( | ||
| 328 | + default={"suggestion": {"sat_recall_min": 30}}, | ||
| 329 | + tenants={"99": {"suggestion": {"sat_recall_cap": 80}}}, | ||
| 330 | + ), | ||
| 331 | + ) | ||
| 332 | + monkeypatch.setattr("suggestion.service.get_app_config", lambda: fake) | ||
| 333 | + cfg = _resolve_suggestion_config_for_tenant("99") | ||
| 334 | + assert cfg.sat_recall_min == 30 | ||
| 335 | + assert cfg.sat_recall_cap == 80 | ||
| 306 | 336 | ||
| 307 | 337 | ||
| 308 | def test_publish_alias_and_cleanup_old_versions(monkeypatch): | 338 | def test_publish_alias_and_cleanup_old_versions(monkeypatch): |