Skip to content

Commit 5fc9968

Browse files
committed
fix(search): read truncation from SMW's query-continue-offset
- replace the count-versus-limit check, blind to $smwgQMaxLimit - stop reporting a complete set of exactly 'limit' results as truncated - add SearchParam.return_meta yielding SemanticSearchResult per query - report query, titles, count, truncated and next_offset to callers - leave return_json untouched, its raw dict already holds the offset
1 parent ece8ccd commit 5fc9968

3 files changed

Lines changed: 252 additions & 28 deletions

File tree

‎src/osw/wiki_tools.py‎

Lines changed: 58 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,26 @@ def create_site_object(
131131
return site
132132

133133

134+
class SemanticSearchResult(OswBaseModel):
135+
"""Result of a single semantic query, including whether the wiki truncated it"""
136+
137+
query: str
138+
"""the query as sent to the wiki, including any limit appended by
139+
semantic_search"""
140+
titles: List[str]
141+
"""the page-title fulltext strings of the results that exist"""
142+
count: int
143+
"""the number of results this response carried, before dropping
144+
non-existing pages, so it can be larger than len(titles). It is not the
145+
total number of matching pages on the wiki, which SMW does not report"""
146+
truncated: bool
147+
"""True if the wiki reported further results beyond those returned"""
148+
next_offset: Optional[int] = None
149+
"""the absolute offset at which the remaining results start, to be passed
150+
back as '|offset='. None for a complete result set. It says where to
151+
continue, not how many results remain"""
152+
153+
134154
class SearchParam(OswBaseModel):
135155
"""Search parameters for semantic and prefix search"""
136156

@@ -142,6 +162,11 @@ class SearchParam(OswBaseModel):
142162
Ignored by semantic_search for a query that sets 'limit=' itself, since SMW
143163
honours the last limit in the query string"""
144164
return_json: Optional[bool] = False
165+
return_meta: Optional[bool] = False
166+
"""If True, semantic_search returns one SemanticSearchResult per query instead
167+
of a flat list of titles, so that a caller can report a truncated result set.
168+
Ignored when return_json is True, since the raw wiki response already carries
169+
the truncation signal"""
145170

146171
def __init__(self, **data):
147172
super().__init__(**data)
@@ -280,7 +305,7 @@ def get_query_limit(query: str) -> Optional[int]:
280305

281306
def semantic_search(
282307
site: mwclient.client.Site, query: Union[str, List[str], SearchParam]
283-
) -> Union[List[str], List[dict]]:
308+
) -> Union[List[str], List[dict], List[SemanticSearchResult]]:
284309
"""Semantic query
285310
286311
Parameters
@@ -295,9 +320,12 @@ def semantic_search(
295320
Returns
296321
-------
297322
result:
298-
With ``return_json=False`` (default): a flat list of page-title fulltext
299-
strings. With ``return_json=True``: a list of raw SMW ``ask`` result dicts,
300-
one per query (always a list, even for a single query).
323+
With ``return_json=False`` and ``return_meta=False`` (default): a flat
324+
list of page-title fulltext strings. With ``return_json=True``: a list of
325+
raw SMW ``ask`` result dicts, one per query (always a list, even for a
326+
single query). With ``return_meta=True``: a list of SemanticSearchResult,
327+
one per query, which reports whether the wiki truncated the result set.
328+
``return_json`` takes precedence if both are set.
301329
"""
302330
if not isinstance(query, SearchParam):
303331
query = SearchParam(query=query)
@@ -320,15 +348,21 @@ def semantic_search_(single_query):
320348
print(f"Query '{single_query}' returned no results")
321349
else:
322350
print(f"Query '{single_query}' returned {n} results")
323-
# No limit in force, or 'limit=0' asking for no results at all as a
324-
# count format does, means the result count says nothing about
325-
# truncation
326-
if limit and n >= limit:
351+
# SMW reports an incomplete result set with a top-level
352+
# 'query-continue-offset' holding the offset the remainder starts at,
353+
# and omits the key for a complete one. That replaces the earlier
354+
# comparison of the result count against the limit, which was wrong in
355+
# both directions: it could not see the wiki's own '$smwgQMaxLimit'
356+
# cap, and it reported a complete set of exactly 'limit' results as
357+
# truncated
358+
next_offset = result.get("query-continue-offset")
359+
truncated = next_offset is not None
360+
if truncated:
327361
warnings.warn(
328-
f"Query '{single_query}' returned {n} results, which meets the "
329-
f"requested limit of {limit}. Results are truncated - raise "
330-
f"the limit or page through with '|offset=' to retrieve the "
331-
f"remainder."
362+
f"Query '{single_query}' returned {n} results and the wiki "
363+
f"reports further ones. Results are truncated - raise the "
364+
f"limit or page through with '|offset={next_offset}' to "
365+
f"retrieve the remainder."
332366
)
333367
if query.return_json:
334368
return result
@@ -349,6 +383,14 @@ def semantic_search_(single_query):
349383
f"Query '{single_query}': {dropped} of {n} results were dropped "
350384
f"because the wiki reported them as non-existing pages."
351385
)
386+
if query.return_meta:
387+
return SemanticSearchResult(
388+
query=single_query,
389+
titles=page_list,
390+
count=n,
391+
truncated=truncated,
392+
next_offset=next_offset,
393+
)
352394
return page_list
353395

354396
if query.parallel:
@@ -358,10 +400,10 @@ def semantic_search_(single_query):
358400
else:
359401
query_results = [semantic_search_(single_query=sq) for sq in query.query]
360402

361-
if query.return_json:
362-
# Each entry of query_results is the raw SMW result dict for one query.
363-
# Do not flatten dicts; always return the list of result dicts (one per
364-
# query), even when only a single query was passed.
403+
if query.return_json or query.return_meta:
404+
# Each entry of query_results is the raw SMW result dict, or the
405+
# SemanticSearchResult, for one query. Do not flatten those; always
406+
# return one entry per query, even when only a single query was passed.
365407
return query_results
366408

367409
return [item for sublist in query_results for item in sublist]

‎src/osw/wtsite.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -568,7 +568,9 @@ def semantic_search(self, query: Union[str, SearchParam]):
568568
569569
Returns
570570
-------
571-
A list of page titles
571+
A list of page titles, or, if the SearchParam sets return_json or
572+
return_meta, one raw result dict or SemanticSearchResult per query.
573+
See wiki_tools.semantic_search for details.
572574
"""
573575
return wt.semantic_search(self._site, query)
574576

‎tests/test_wiki_tools.py‎

Lines changed: 191 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,12 @@ def test_create_flat_content_structure_from_wikitext():
1313
assert result == expected
1414

1515

16-
def _ask_result(*titles):
17-
"""Build a minimal SMW ``ask`` API result dict for the given page titles."""
18-
return {
16+
def _ask_result(*titles, continue_offset=None):
17+
"""Build a minimal SMW ``ask`` API result dict for the given page titles.
18+
19+
``continue_offset`` adds the top-level ``query-continue-offset`` key that SMW
20+
sends when further results exist beyond the ones returned."""
21+
result = {
1922
"query": {
2023
"results": {
2124
title: {
@@ -30,6 +33,9 @@ def _ask_result(*titles):
3033
}
3134
}
3235
}
36+
if continue_offset is not None:
37+
result["query-continue-offset"] = continue_offset
38+
return result
3339

3440

3541
def _ask_result_empty():
@@ -167,7 +173,7 @@ def test_semantic_search_parallel_batch_with_one_zero_result_query():
167173

168174
def test_semantic_search_truncation_warning():
169175
titles = [f"Item:OSW{i}" for i in range(5)]
170-
result = _ask_result(*titles)
176+
result = _ask_result(*titles, continue_offset=5)
171177
site = MagicMock()
172178
site.api.return_value = result
173179

@@ -179,6 +185,23 @@ def test_semantic_search_truncation_warning():
179185
assert sorted(out) == sorted(titles)
180186

181187

188+
def test_semantic_search_no_truncation_warning_for_a_complete_set_at_the_limit():
189+
"""A wiki that has exactly as many results as the limit asked for sends no
190+
offset, so meeting the limit is not on its own a sign of truncation."""
191+
titles = [f"Item:OSW{i}" for i in range(5)]
192+
site = MagicMock()
193+
site.api.return_value = _ask_result(*titles)
194+
195+
with warnings.catch_warnings(record=True) as caught:
196+
warnings.simplefilter("always")
197+
out = wt.semantic_search(
198+
site, wt.SearchParam(query="[[HasType::Category:Item]]", limit=5)
199+
)
200+
201+
assert not any("truncated" in str(w.message) for w in caught)
202+
assert sorted(out) == sorted(titles)
203+
204+
182205
def test_semantic_search_no_truncation_warning_below_limit():
183206
titles = [f"Item:OSW{i}" for i in range(5)]
184207
result = _ask_result(*titles)
@@ -207,6 +230,160 @@ def test_semantic_search_exists_drop_warning():
207230
assert out == ["Item:OSW1"]
208231

209232

233+
def test_semantic_search_truncation_warning_from_the_continue_offset():
234+
# Fewer results than the limit, so only the wiki's own signal can report
235+
# that the result set was cut short
236+
result = _ask_result("Item:OSW1", "Item:OSW2", continue_offset=2)
237+
site = MagicMock()
238+
site.api.return_value = result
239+
240+
with pytest.warns(UserWarning, match="truncated"):
241+
out = wt.semantic_search(
242+
site, wt.SearchParam(query="[[HasType::Category:Item]]", limit=1000)
243+
)
244+
245+
assert sorted(out) == ["Item:OSW1", "Item:OSW2"]
246+
247+
248+
def test_semantic_search_limit_none_warns_from_the_continue_offset():
249+
# With no limit in force the wiki applies its own '$smwgQMaxLimit' cap,
250+
# which the result count cannot detect
251+
result = _ask_result("Item:OSW1", "Item:OSW2", continue_offset=2)
252+
site = MagicMock()
253+
site.api.return_value = result
254+
255+
with pytest.warns(UserWarning, match="truncated"):
256+
wt.semantic_search(
257+
site, wt.SearchParam(query="[[HasType::Category:Item]]", limit=None)
258+
)
259+
260+
261+
def test_semantic_search_return_json_keeps_the_continue_offset():
262+
result = _ask_result("Item:OSW1", continue_offset=1)
263+
site = MagicMock()
264+
site.api.return_value = result
265+
266+
with pytest.warns(UserWarning, match="truncated"):
267+
out = wt.semantic_search(
268+
site, wt.SearchParam(query="[[HasType::Category:Item]]", return_json=True)
269+
)
270+
271+
# The raw response is returned unchanged, so it already carries the
272+
# truncation signal
273+
assert out[0]["query-continue-offset"] == 1
274+
275+
276+
def test_semantic_search_return_meta_reports_truncation():
277+
result = _ask_result("Item:OSW1", "Item:OSW2", continue_offset=2)
278+
site = MagicMock()
279+
site.api.return_value = result
280+
281+
with pytest.warns(UserWarning, match="truncated"):
282+
out = wt.semantic_search(
283+
site, wt.SearchParam(query="[[HasType::Category:Item]]", return_meta=True)
284+
)
285+
286+
assert len(out) == 1
287+
assert isinstance(out[0], wt.SemanticSearchResult)
288+
assert out[0].truncated is True
289+
assert out[0].next_offset == 2
290+
assert out[0].count == 2
291+
assert sorted(out[0].titles) == ["Item:OSW1", "Item:OSW2"]
292+
# the limit appended by semantic_search is part of the reported query
293+
assert out[0].query == "[[HasType::Category:Item]]|limit=1000"
294+
295+
296+
def test_semantic_search_return_meta_reports_a_complete_result():
297+
result = _ask_result("Item:OSW1", "Item:OSW2")
298+
site = MagicMock()
299+
site.api.return_value = result
300+
301+
with warnings.catch_warnings(record=True) as caught:
302+
warnings.simplefilter("always")
303+
out = wt.semantic_search(
304+
site, wt.SearchParam(query="[[HasType::Category:Item]]", return_meta=True)
305+
)
306+
307+
assert not any("truncated" in str(w.message) for w in caught)
308+
assert out[0].truncated is False
309+
assert out[0].next_offset is None
310+
assert out[0].count == 2
311+
312+
313+
def test_semantic_search_return_meta_returns_one_result_per_query():
314+
site = MagicMock()
315+
site.api.side_effect = [
316+
_ask_result("Item:OSW1", continue_offset=1),
317+
_ask_result("Item:OSW2"),
318+
]
319+
320+
with pytest.warns(UserWarning, match="truncated"):
321+
out = wt.semantic_search(
322+
site,
323+
wt.SearchParam(
324+
query=["[[HasType::Category:A]]", "[[HasType::Category:B]]"],
325+
return_meta=True,
326+
),
327+
)
328+
329+
# One entry per query, not flattened into a single list of titles
330+
assert len(out) == 2
331+
assert [r.truncated for r in out] == [True, False]
332+
assert [r.titles for r in out] == [["Item:OSW1"], ["Item:OSW2"]]
333+
334+
335+
def test_semantic_search_return_meta_count_includes_dropped_pages():
336+
result = _ask_result("Item:OSW1", "Item:OSW2")
337+
result["query"]["results"]["Item:OSW2"]["exists"] = ""
338+
site = MagicMock()
339+
site.api.return_value = result
340+
341+
with pytest.warns(UserWarning, match="non-existing"):
342+
out = wt.semantic_search(
343+
site, wt.SearchParam(query="[[HasType::Category:Item]]", return_meta=True)
344+
)
345+
346+
assert out[0].count == 2
347+
assert out[0].titles == ["Item:OSW1"]
348+
349+
350+
def test_semantic_search_return_meta_keeps_the_query_order_when_parallel():
351+
# More than five queries makes SearchParam switch to the parallel path, where
352+
# the results must still line up with the queries they came from
353+
queries = [f"[[HasType::Category:C{i}]]" for i in range(6)]
354+
355+
def api(action, query=None, format=None):
356+
index = queries.index(query.split("|limit=")[0])
357+
return _ask_result(f"Item:OSW{index}", continue_offset=index or None)
358+
359+
site = MagicMock()
360+
site.api.side_effect = api
361+
param = wt.SearchParam(query=queries, return_meta=True)
362+
assert param.parallel is True
363+
364+
with pytest.warns(UserWarning, match="truncated"):
365+
out = wt.semantic_search(site, param)
366+
367+
assert [r.query.split("|limit=")[0] for r in out] == queries
368+
assert [r.titles for r in out] == [[f"Item:OSW{i}"] for i in range(6)]
369+
assert [r.truncated for r in out] == [False] + [True] * 5
370+
371+
372+
def test_semantic_search_return_json_takes_precedence_over_return_meta():
373+
result = _ask_result("Item:OSW1")
374+
site = MagicMock()
375+
site.api.return_value = result
376+
377+
out = wt.semantic_search(
378+
site,
379+
wt.SearchParam(
380+
query="[[HasType::Category:Item]]", return_json=True, return_meta=True
381+
),
382+
)
383+
384+
assert out == [result]
385+
386+
210387
@pytest.mark.parametrize(
211388
"query, expected",
212389
[
@@ -264,13 +441,14 @@ def test_semantic_search_query_limit_beats_the_search_param_limit():
264441
assert site.api.call_args.kwargs["query"] == "[[HasType::Category:Item]]|limit=2"
265442

266443

267-
def test_semantic_search_truncation_warning_uses_the_query_limit():
268-
"""The caller's limit is the one the results were truncated at."""
444+
def test_semantic_search_truncation_warning_names_the_query_as_sent():
445+
"""The caller's limit is the one the results were truncated at, so the
446+
warning has to quote the query carrying it."""
269447
titles = [f"Item:OSW{i}" for i in range(2)]
270448
site = MagicMock()
271-
site.api.return_value = _ask_result(*titles)
449+
site.api.return_value = _ask_result(*titles, continue_offset=2)
272450

273-
with pytest.warns(UserWarning, match="requested limit of 2"):
451+
with pytest.warns(UserWarning, match=r"\|limit=2'"):
274452
out = wt.semantic_search(site, "[[HasType::Category:Item]]|limit=2")
275453

276454
assert sorted(out) == sorted(titles)
@@ -299,8 +477,9 @@ def test_semantic_search_limit_none_keeps_a_limit_the_caller_wrote():
299477
assert site.api.call_args.kwargs["query"] == "[[HasType::Category:Item]]|limit=2"
300478

301479

302-
def test_semantic_search_limit_none_does_not_warn_about_truncation():
303-
"""With no limit in force the result count says nothing about truncation."""
480+
def test_semantic_search_limit_none_does_not_warn_without_an_offset():
481+
"""A wiki reporting no further results never triggers the warning, whatever
482+
limit was in force."""
304483
titles = [f"Item:OSW{i}" for i in range(5)]
305484
site = MagicMock()
306485
site.api.return_value = _ask_result(*titles)
@@ -316,7 +495,8 @@ def test_semantic_search_limit_none_does_not_warn_about_truncation():
316495

317496

318497
def test_semantic_search_no_truncation_warning_for_a_zero_limit():
319-
"""'limit=0' asks for no results, so meeting it is not truncation."""
498+
"""'limit=0' asks for a count rather than results, and SMW sends no offset
499+
for it even when the query does match pages."""
320500
site = MagicMock()
321501
site.api.return_value = _ask_result_empty()
322502

0 commit comments

Comments
 (0)