Skip to content

Commit c92f342

Browse files
nickwinderclaude
andcommitted
Fix rotate and delete_pages with negative page indices
- rotate(): change `start > 0` to `start != 0` so prefix pages are included when a negative start index is used (e.g. start=-3) - delete_pages(): rewrite keep-range algorithm to handle negative delete indices; previously all-negative inputs raised a spurious ValidationError instead of keeping the correct page range Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 211377e commit c92f342

File tree

2 files changed

+144
-17
lines changed

2 files changed

+144
-17
lines changed

src/nutrient_dws/client.py

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1423,7 +1423,7 @@ async def rotate(
14231423
end = pages.get("end", -1)
14241424

14251425
# Add pages before the rotation range
1426-
if start > 0:
1426+
if start != 0:
14271427
part_options = cast(
14281428
"FilePartOptions",
14291429
{"pages": {"start": 0, "end": start - 1}},
@@ -1661,28 +1661,47 @@ async def delete_pages(
16611661

16621662
builder = self.workflow()
16631663

1664-
# Build "keep ranges" by finding gaps between deleted indices
1665-
# This algorithm works with negative indices - server handles them
1666-
current_page = 0
1664+
positive_deletes = [i for i in delete_indices if i >= 0]
1665+
negative_deletes = [i for i in delete_indices if i < 0]
1666+
16671667
page_ranges = []
1668+
current_page = 0
1669+
1670+
# Build keep ranges for positive delete indices
1671+
for delete_index in positive_deletes:
1672+
if current_page < delete_index:
1673+
page_ranges.append(
1674+
normalize_page_params(
1675+
{"start": current_page, "end": delete_index - 1}
1676+
)
1677+
)
1678+
current_page = delete_index + 1
16681679

1669-
for delete_index in delete_indices:
1670-
# Skip negative indices in this calculation - they represent "from end"
1671-
if delete_index >= 0:
1672-
if current_page < delete_index:
1680+
if negative_deletes:
1681+
# Add keep range from current position up to just before the first negative delete
1682+
trailing_end = negative_deletes[0] - 1 # e.g. -1 -> -2, -2 -> -3
1683+
page_ranges.append(
1684+
normalize_page_params({"start": current_page, "end": trailing_end})
1685+
)
1686+
# Add keep ranges between consecutive negative delete indices
1687+
for i in range(len(negative_deletes) - 1):
1688+
between_start = negative_deletes[i] + 1
1689+
between_end = negative_deletes[i + 1] - 1
1690+
if between_start <= between_end:
16731691
page_ranges.append(
16741692
normalize_page_params(
1675-
{"start": current_page, "end": delete_index - 1}
1693+
{"start": between_start, "end": between_end}
16761694
)
16771695
)
1678-
current_page = delete_index + 1
1679-
1680-
# Add remaining pages to end (use -1 for last page) add if we have positive indices processed
1681-
if (
1682-
current_page >= 0
1683-
and len(delete_indices) > 0
1684-
and any(i >= 0 for i in delete_indices)
1685-
):
1696+
# Add keep range after the last negative delete, if it isn't the last page
1697+
if negative_deletes[-1] != -1:
1698+
page_ranges.append(
1699+
normalize_page_params(
1700+
{"start": negative_deletes[-1] + 1, "end": -1}
1701+
)
1702+
)
1703+
else:
1704+
# All deletes are positive: keep remaining pages to end of document
16861705
page_ranges.append(
16871706
normalize_page_params({"start": current_page, "end": -1})
16881707
)

tests/unit/test_client.py

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1202,3 +1202,111 @@ async def test_ocr_with_iso_language_codes(
12021202
mock_workflow_instance.add_file_part.assert_called_once_with(
12031203
file, None, [{"type": "ocr", "language": ["eng", "deu", "fra"]}]
12041204
)
1205+
1206+
1207+
class TestNutrientClientRotate:
1208+
"""Tests for NutrientClient rotate functionality."""
1209+
1210+
def _make_mock_workflow(self, mock_staged_workflow_builder):
1211+
mock_workflow_instance = MagicMock()
1212+
mock_output_stage = MagicMock()
1213+
mock_output_stage.execute = AsyncMock(
1214+
return_value={
1215+
"success": True,
1216+
"output": {
1217+
"buffer": b"test-buffer",
1218+
"mimeType": "application/pdf",
1219+
"filename": "output.pdf",
1220+
},
1221+
}
1222+
)
1223+
mock_workflow_instance.add_file_part.return_value = mock_workflow_instance
1224+
mock_workflow_instance.output_pdf.return_value = mock_output_stage
1225+
mock_staged_workflow_builder.return_value = mock_workflow_instance
1226+
return mock_workflow_instance
1227+
1228+
@patch("nutrient_dws.client.StagedWorkflowBuilder")
1229+
@pytest.mark.asyncio
1230+
async def test_rotate_with_negative_start_includes_prefix_pages(
1231+
self, mock_staged_workflow_builder, unit_client
1232+
):
1233+
"""Rotating last N pages with negative start must keep all preceding pages."""
1234+
mock_workflow = self._make_mock_workflow(mock_staged_workflow_builder)
1235+
1236+
await unit_client.rotate("document.pdf", 90, {"start": -3, "end": -1})
1237+
1238+
# Expect 2 add_file_part calls: prefix pages then rotated pages
1239+
assert mock_workflow.add_file_part.call_count == 2
1240+
# First call: prefix pages before the rotation range (pages 0 to -4)
1241+
first_call_pages = mock_workflow.add_file_part.call_args_list[0][0][1]
1242+
assert first_call_pages == {"pages": {"start": 0, "end": -4}}
1243+
# Second call: the rotated range with the rotate action
1244+
second_call_pages = mock_workflow.add_file_part.call_args_list[1][0][1]
1245+
assert second_call_pages == {"pages": {"start": -3, "end": -1}}
1246+
1247+
1248+
class TestNutrientClientDeletePages:
1249+
"""Tests for NutrientClient delete_pages functionality."""
1250+
1251+
def _make_mock_workflow(self, mock_staged_workflow_builder):
1252+
mock_workflow_instance = MagicMock()
1253+
mock_output_stage = MagicMock()
1254+
mock_output_stage.execute = AsyncMock(
1255+
return_value={
1256+
"success": True,
1257+
"output": {
1258+
"buffer": b"test-buffer",
1259+
"mimeType": "application/pdf",
1260+
"filename": "output.pdf",
1261+
},
1262+
}
1263+
)
1264+
mock_workflow_instance.add_file_part.return_value = mock_workflow_instance
1265+
mock_workflow_instance.output_pdf.return_value = mock_output_stage
1266+
mock_staged_workflow_builder.return_value = mock_workflow_instance
1267+
return mock_workflow_instance
1268+
1269+
@patch("nutrient_dws.client.StagedWorkflowBuilder")
1270+
@pytest.mark.asyncio
1271+
async def test_delete_last_page_using_negative_index(
1272+
self, mock_staged_workflow_builder, unit_client
1273+
):
1274+
"""delete_pages([-1]) must keep all pages except the last, not raise an error."""
1275+
mock_workflow = self._make_mock_workflow(mock_staged_workflow_builder)
1276+
1277+
result = await unit_client.delete_pages("document.pdf", [-1])
1278+
1279+
assert result["buffer"] == b"test-buffer"
1280+
assert mock_workflow.add_file_part.call_count == 1
1281+
kept_pages = mock_workflow.add_file_part.call_args_list[0][0][1]
1282+
assert kept_pages == {"pages": {"start": 0, "end": -2}}
1283+
1284+
@patch("nutrient_dws.client.StagedWorkflowBuilder")
1285+
@pytest.mark.asyncio
1286+
async def test_delete_multiple_pages_using_only_negative_indices(
1287+
self, mock_staged_workflow_builder, unit_client
1288+
):
1289+
"""delete_pages([-2, -1]) must keep all pages except the last two."""
1290+
mock_workflow = self._make_mock_workflow(mock_staged_workflow_builder)
1291+
1292+
result = await unit_client.delete_pages("document.pdf", [-2, -1])
1293+
1294+
assert result["buffer"] == b"test-buffer"
1295+
assert mock_workflow.add_file_part.call_count == 1
1296+
kept_pages = mock_workflow.add_file_part.call_args_list[0][0][1]
1297+
assert kept_pages == {"pages": {"start": 0, "end": -3}}
1298+
1299+
@patch("nutrient_dws.client.StagedWorkflowBuilder")
1300+
@pytest.mark.asyncio
1301+
async def test_delete_pages_with_mixed_positive_and_negative_indices(
1302+
self, mock_staged_workflow_builder, unit_client
1303+
):
1304+
"""delete_pages([0, -1]) must keep middle pages, not include the deleted last page."""
1305+
mock_workflow = self._make_mock_workflow(mock_staged_workflow_builder)
1306+
1307+
result = await unit_client.delete_pages("document.pdf", [0, -1])
1308+
1309+
assert result["buffer"] == b"test-buffer"
1310+
assert mock_workflow.add_file_part.call_count == 1
1311+
kept_pages = mock_workflow.add_file_part.call_args_list[0][0][1]
1312+
assert kept_pages == {"pages": {"start": 1, "end": -2}}

0 commit comments

Comments
 (0)