Skip to content

Commit ca6947d

Browse files
author
pegah
committed
test: add coverage for Instructor-based structured output
Adds tests/test_instructor_migration.py (23 tests) covering the reliability properties of the Instructor/Pydantic migration: - TOCIndexItem.structure regression test (Optional[str] without default=None is not omittable in Pydantic v2) - add_page_offset_to_toc_json guard against offset=None - llm_structured/llm_astructured: finish_reason truncation detection, max_tokens threading, max_retries capped at 1, litellm/ prefix stripping - toc_transformer end-to-end behavior against the migrated implementation - AliasChoices synonym handling for reasoning fields Also includes the llm_structured/llm_astructured implementation changes these tests cover: switched from create() to create_with_completion() to expose finish_reason, added max_tokens parameter, capped max_retries at 1 (previously 3 — found to compound failures on smaller models by feeding accumulated failed completions back into retry context). tests/test_issue_163.py is left unmodified. Running it against this branch shows 10 failed / 4 passed: the 4 passes (extract_toc_content) are unaffected since that function wasn't migrated; the 10 failures mock llm_completion for functions now using llm_structured, so the mock no longer intercepts the real call path. Not a regression — a consequence of those tests asserting on manual-parsing implementation details this PR removes. Left as-is pending maintainer input on how to handle it.
1 parent 3e92917 commit ca6947d

3 files changed

Lines changed: 335 additions & 71 deletions

File tree

pageindex/page_index.py

Lines changed: 22 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -88,15 +88,15 @@ def _validate_physical_indices(toc: list, total_pages: int, start_index: int = 1
8888
return toc
8989

9090
################### check title in page #########################################################
91-
async def check_title_appearance(item, page_list, start_index=1, model=None):
91+
async def check_title_appearance(item, page_list, start_index=1, model=None):
9292
title=item['title']
9393
if 'physical_index' not in item or item['physical_index'] is None:
9494
return {'list_index': item.get('list_index'), 'answer': 'no', 'title': title, 'page_number': None}
9595

9696
page_number = item['physical_index']
9797
page_text = page_list[page_number-start_index][0]
9898

99-
99+
100100
prompt = _SYSTEM_HARDENING + f"""
101101
Your job is to check if the given section appears or starts in the given page_text.
102102
@@ -108,7 +108,7 @@ async def check_title_appearance(item, page_list, start_index=1, model=None):
108108
return {'list_index': item['list_index'], 'answer': result.answer, 'title': title, 'page_number': page_number}
109109

110110

111-
async def check_title_appearance_in_start(title, page_text, model=None, logger=None):
111+
async def check_title_appearance_in_start(title, page_text, model=None, logger=None):
112112
prompt = _SYSTEM_HARDENING + f"""
113113
You will be given the current section title and the current page_text.
114114
Your job is to check if the current section starts in the beginning of the given page_text.
@@ -210,7 +210,7 @@ def extract_toc_content(content, model=None):
210210
{"role": "assistant", "content": response},
211211
]
212212
continue_prompt = "please continue the generation of table of contents, directly output the remaining part of the structure"
213-
213+
214214
max_attempts = 5
215215
for attempt in range(max_attempts):
216216
new_response, finish_reason = llm_completion(model=model, prompt=continue_prompt, chat_history=chat_history, return_finish_reason=True)
@@ -222,7 +222,7 @@ def extract_toc_content(content, model=None):
222222
break
223223
else:
224224
raise Exception('Failed to complete table of contents extraction after maximum retries')
225-
225+
226226
return response
227227

228228
def detect_page_index(toc_content, model=None):
@@ -298,73 +298,31 @@ def toc_index_extractor(toc, content, model=None):
298298
items = [item.model_dump() for item in result.items]
299299
return _validate_chunk_physical_indices(toc=items, content=content)
300300

301-
302-
303301
def toc_transformer(toc_content, model=None):
302+
# TODO after rebase completes — two known gaps vs. main's version, to
303+
# resolve deliberately (see PR discussion), not silently:
304+
# 1. No continuation/retry logic for truncated completions (main uses
305+
# a chat-history "please continue" loop, capped at 5 attempts).
306+
# llm_structured already fails fast on finish_reason == "length",
307+
# which is a deliberate trade-off, not an oversight.
308+
# 2. No equivalent of check_if_toc_transformation_is_complete — main
309+
# catches "complete-looking but actually missing sections" output
310+
# (finish_reason == "stop" but content silently incomplete), which
311+
# Pydantic validation alone cannot catch (a shorter-than-expected
312+
# list still validates fine). This gap needs an explicit decision:
313+
# port the check, or document and accept it.
304314
print('start toc_transformer')
305315
init_prompt = """
306316
You are given a table of contents, You job is to transform the whole table of content into a JSON format included table_of_contents.
307317
308318
structure is the numeric system which represents the index of the hierarchy section in the table of contents. For example, the first section has structure index 1, the first subsection has structure index 1.1, the second subsection has structure index 1.2, etc.
309319
310-
The response should be in the following JSON format:
311-
{
312-
table_of_contents: [
313-
{
314-
"structure": <structure index, "x.x.x" or None> (string),
315-
"title": <title of the section>,
316-
"page": <page number or None>,
317-
},
318-
...
319-
],
320-
}
321320
You should transform the full table of contents in one go.
322321
Directly return the final JSON structure, do not output anything else. """
323322

324323
prompt = init_prompt + '\n Given table of contents\n:' + _secure_doc_text(toc_content)
325-
last_complete, finish_reason = llm_completion(model=model, prompt=prompt, return_finish_reason=True)
326-
if_complete = check_if_toc_transformation_is_complete(toc_content, last_complete, model)
327-
if if_complete == "yes" and finish_reason == "finished":
328-
last_complete = extract_json(last_complete)
329-
cleaned_response = convert_page_to_int(last_complete.get('table_of_contents', []))
330-
return cleaned_response
331-
332-
last_complete = get_json_content(last_complete)
333-
chat_history = [
334-
{"role": "user", "content": prompt},
335-
{"role": "assistant", "content": last_complete},
336-
]
337-
continue_prompt = "Please continue the table of contents JSON structure from where you left off. Directly output only the remaining part."
338-
339-
position = last_complete.rfind('}')
340-
if position != -1:
341-
last_complete = last_complete[:position+2]
342-
343-
max_attempts = 5
344-
for attempt in range(max_attempts):
345-
346-
new_complete, finish_reason = llm_completion(model=model, prompt=continue_prompt, chat_history=chat_history, return_finish_reason=True)
347-
348-
if new_complete.startswith('```json'):
349-
new_complete = get_json_content(new_complete)
350-
last_complete = last_complete + new_complete
351-
352-
chat_history.append({"role": "user", "content": continue_prompt})
353-
chat_history.append({"role": "assistant", "content": new_complete})
354-
355-
if_complete = check_if_toc_transformation_is_complete(toc_content, last_complete, model)
356-
if if_complete == "yes" and finish_reason == "finished":
357-
break
358-
else:
359-
raise Exception('Failed to complete TOC transformation after maximum retries')
360-
361-
last_complete = extract_json(last_complete)
362-
363-
cleaned_response = convert_page_to_int(last_complete.get('table_of_contents', []))
364-
return cleaned_response
365-
366-
367-
324+
result = llm_structured(model=model, prompt=prompt, response_model=TOCTransformation, max_tokens=8000)
325+
return convert_page_to_int([item.model_dump() for item in result.table_of_contents])
368326

369327
def find_toc_pages(start_page_index, page_list, opt, logger=None):
370328
print('start find_toc_pages')
@@ -572,16 +530,15 @@ def generate_toc_init(part, model=None):
572530
return output
573531

574532
def process_no_toc(page_list, start_index=1, model=None, logger=None):
575-
page_contents=[]
576-
token_lengths=[]
577-
for page_index in range(start_index, start_index+len(page_list)):
533+
page_contents = []
534+
token_lengths = []
535+
for page_index in range(start_index, start_index + len(page_list)):
578536
page_text = f"<physical_index_{page_index}>\n{page_list[page_index-start_index][0]}\n<physical_index_{page_index}>\n\n"
579537
page_contents.append(page_text)
580538
token_lengths.append(count_tokens(page_text, model))
581539
group_texts = page_list_to_group_text(page_contents, token_lengths)
582540
logger.info(f'len(group_texts): {len(group_texts)}')
583541

584-
toc_with_page_number = generate_toc_init(group_texts[0], model)
585542
toc_with_page_number = generate_toc_init(group_texts[0], model)
586543
toc_with_page_number = _validate_chunk_physical_indices(
587544
toc=toc_with_page_number,

pageindex/utils.py

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,30 +35,54 @@ def count_tokens(text, model=None):
3535
return 0
3636
return litellm.token_counter(model=model, text=text)
3737

38-
def llm_structured(model, prompt, response_model, chat_history=None):
38+
def llm_structured(model, prompt, response_model, chat_history=None, max_tokens=4000):
3939
if model:
4040
model = model.removeprefix("litellm/")
4141
messages = list(chat_history) + [{"role": "user", "content": prompt}] if chat_history else [{"role": "user", "content": prompt}]
42-
return sync_instructor_client.chat.completions.create(
42+
43+
result, completion = sync_instructor_client.chat.completions.create_with_completion(
4344
model=model,
4445
messages=messages,
4546
response_model=response_model,
4647
temperature=0,
47-
max_retries=3,
48+
max_retries=1,
49+
max_tokens=max_tokens,
4850
)
4951

50-
async def llm_astructured(model, prompt, response_model):
52+
finish_reason = completion.choices[0].finish_reason
53+
if finish_reason == "length":
54+
raise ValueError(
55+
f"Response was truncated (finish_reason='length') before completing "
56+
f"the {response_model.__name__} structure. Increase max_tokens (currently "
57+
f"{max_tokens}) for this call."
58+
)
59+
60+
return result
61+
62+
async def llm_astructured(model, prompt, response_model, max_tokens=4000):
5163
if model:
5264
model = model.removeprefix("litellm/")
5365
messages = [{"role": "user", "content": prompt}]
54-
return await async_instructor_client.chat.completions.create(
66+
67+
result, completion = await async_instructor_client.chat.completions.create_with_completion(
5568
model=model,
5669
messages=messages,
5770
response_model=response_model,
5871
temperature=0,
59-
max_retries=3,
72+
max_retries=1,
73+
max_tokens=max_tokens,
6074
)
6175

76+
finish_reason = completion.choices[0].finish_reason
77+
if finish_reason == "length":
78+
raise ValueError(
79+
f"Response was truncated (finish_reason='length') before completing "
80+
f"the {response_model.__name__} structure. Increase max_tokens (currently "
81+
f"{max_tokens}) for this call."
82+
)
83+
84+
return result
85+
6286

6387
def llm_completion(model, prompt, chat_history=None, return_finish_reason=False):
6488
if model:

0 commit comments

Comments
 (0)