-
Notifications
You must be signed in to change notification settings - Fork 1
fix: preserve source indentation evidence for adjudication #394
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5602096
5219ed8
2aee6ac
93de9cd
6d24618
88a5e88
ecb7374
49d1bd8
cf9505b
9f5c5e9
41d75d0
1f1667f
ec74eed
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,15 +9,18 @@ | |
|
|
||
| import asyncio | ||
| import hashlib | ||
| import json | ||
| import logging | ||
| import math | ||
| from typing import Any, TypeVar | ||
|
|
||
| from .chunking import Chunk, chunk_by_source_body | ||
| from .embedding_client import EmbeddingClient | ||
| from .image_content import ImageContentClient, ImageDescription | ||
| from .http_client import HttpClientError, json_request_body | ||
| from .post_content_normalization import ImageContentResult, normalize_post_body | ||
| from .post_structure import ( | ||
| ContextualOrchestratorPostStructureClient, | ||
| NullPostStructureClient, | ||
| PostStructureClient, | ||
| StructureDecision, | ||
|
|
@@ -31,14 +34,19 @@ | |
|
|
||
|
|
||
| def _bounded_unit_batches( # noqa: UP047 - retain Python 3.10 compatibility. | ||
| units: list[tuple[_BatchKey, str]], | ||
| ) -> list[list[tuple[_BatchKey, str]]]: | ||
| units: list[tuple[_BatchKey, str | dict[str, object]]], | ||
| ) -> list[list[tuple[_BatchKey, str | dict[str, object]]]]: | ||
| """Keep provider requests bounded without changing persisted source units.""" | ||
| batches: list[list[tuple[_BatchKey, str]]] = [] | ||
| batch: list[tuple[_BatchKey, str]] = [] | ||
| batches: list[list[tuple[_BatchKey, str | dict[str, object]]]] = [] | ||
| batch: list[tuple[_BatchKey, str | dict[str, object]]] = [] | ||
| batch_chars = 0 | ||
| for unit in units: | ||
| unit_chars = len(unit[1]) | ||
| payload = unit[1] | ||
| unit_chars = ( | ||
| len(payload) | ||
| if isinstance(payload, str) | ||
| else len(json.dumps(payload, ensure_ascii=False, separators=(",", ":"))) | ||
| ) | ||
| if batch and ( | ||
| len(batch) >= _LLM_BATCH_MAX_UNITS | ||
| or batch_chars + unit_chars > _LLM_BATCH_MAX_CHARS | ||
|
|
@@ -53,6 +61,32 @@ def _bounded_unit_batches( # noqa: UP047 - retain Python 3.10 compatibility. | |
| return batches | ||
|
|
||
|
|
||
| def _bounded_structure_batches( | ||
| units: list[tuple[int, dict[str, object]]], post_title: str | ||
| ) -> list[list[tuple[int, dict[str, object]]]]: | ||
| """Bound structure batches by their exact serialized HTTP request body.""" | ||
| batches: list[list[tuple[int, dict[str, object]]]] = [] | ||
| batch: list[tuple[int, dict[str, object]]] = [] | ||
| for unit in units: | ||
| candidate = [*batch, unit] | ||
| candidate_body = json_request_body( | ||
| ContextualOrchestratorPostStructureClient.request_payload( | ||
| post_title, [payload for _index, payload in candidate] | ||
| ) | ||
| ) | ||
| if batch and ( | ||
| len(batch) >= _LLM_BATCH_MAX_UNITS | ||
| or len(candidate_body) > _LLM_BATCH_MAX_CHARS | ||
| ): | ||
| batches.append(batch) | ||
| batch = [unit] | ||
| else: | ||
| batch = candidate | ||
| if batch: | ||
| batches.append(batch) | ||
| return batches | ||
|
seonghobae marked this conversation as resolved.
|
||
|
|
||
|
|
||
| def _render_description(description: ImageDescription | None) -> str: | ||
| """Render one image or visual-region description as searchable text.""" | ||
| if description is None: | ||
|
|
@@ -151,21 +185,38 @@ async def persist_post_content( | |
| structure_units = [ | ||
| ( | ||
| chunk.index, | ||
| chunk.text[:_STRUCTURE_UNIT_MAX_CHARS] | ||
| + ( | ||
| "\n[truncated for structure adjudication]" | ||
| if len(chunk.text) > _STRUCTURE_UNIT_MAX_CHARS | ||
| else "" | ||
| ), | ||
| { | ||
| "unit_index": chunk.index, | ||
| "text": chunk.text[:_STRUCTURE_UNIT_MAX_CHARS] | ||
| + ( | ||
| "\n[truncated for structure adjudication]" | ||
| if len(chunk.text) > _STRUCTURE_UNIT_MAX_CHARS | ||
| else "" | ||
| ), | ||
| "label": chunk.label, | ||
| "style": formatting.get(chunk.index), | ||
| "source_indent_width": max( | ||
| 0, | ||
| int(chunk.indent_width) - int(chunk.declared_indent_width), | ||
| ), | ||
|
seonghobae marked this conversation as resolved.
|
||
| "declared_indent_width": int(chunk.declared_indent_width), | ||
| }, | ||
| ) | ||
| for chunk in unresolved | ||
| ] | ||
| for batch in _bounded_unit_batches(structure_units): | ||
| for batch in _bounded_structure_batches(structure_units, post_title): | ||
| try: | ||
| request_body = json_request_body( | ||
| ContextualOrchestratorPostStructureClient.request_payload( | ||
| post_title, [payload for _index, payload in batch] | ||
| ) | ||
| ) | ||
| if len(request_body) > _LLM_BATCH_MAX_CHARS: | ||
| raise HttpClientError("structure adjudication request exceeds size limit") | ||
|
seonghobae marked this conversation as resolved.
Comment on lines
+214
to
+215
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Large CJK paragraphs never adjudicated for structure Structure requests are rejected when their ASCII-escaped JSON body exceeds 24000 bytes ( Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. |
||
| decisions = await asyncio.to_thread( | ||
| client.infer, | ||
| post_title, | ||
| [{"unit_index": index, "text": text} for index, text in batch], | ||
| [payload for _index, payload in batch], | ||
| ) | ||
| for decision in decisions: | ||
| if decision.unit_index in unresolved_indexes: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 Info: Size bound assumes the orchestrator client's payload
Both the batcher and the recompute measure request size via
ContextualOrchestratorPostStructureClient.request_payloadregardless of the injectedPostStructureClient. Harmless while that is the only real client, but an alternate client with a different payload shape would be bounded against the wrong body.Was this helpful? React with 👍 or 👎 to provide feedback.