Resolve clip tool ordering in subscription requests

[asonnenschein]

Resolve clip tool ordering in subscription requests

GH issue #1206

Problem

When using clip_to_source=True in build_request(), the clip tool was appended to the end of the tool list, resulting in incorrect processing order. See the documentation on creating toolchains for more information about toolchain processing order.

Solution

• Defined tool processing weights in planet/constants.py as _SUBSCRIPTION_TOOL_WEIGHT
• Added _validate_tool_order() to validate input tool lists are in correct order
• Added _insert_clip_tool() to insert clip at the correct position based on tool weights
• Updated build_request() to use these helper methods

Example

Before:

tools = [{"type": "harmonize"}, {"type": "file_format"}]
build_request(..., tools=tools, clip_to_source=True)
# Result: ["harmonize", "file_format", "clip"]  ❌

After:

tools = [{"type": "harmonize"}, {"type": "file_format"}]
build_request(..., tools=tools, clip_to_source=True)
# Result: ["harmonize", "clip", "file_format"]  ✅

Testing

• Added 17 new parametrized tests covering tool ordering validation and clip insertion
• All 66 tests pass
• Coverage for planet/subscription_request.py increased to 86%

[Copilot]

Pull request overview

This PR resolves an issue where the clip tool was incorrectly placed at the end of the tool list when using clip_to_source=True in subscription requests, causing API validation errors due to incorrect tool sequencing.

Changes:

• Introduced tool processing weights to define correct ordering constraints for subscription tools
• Added validation to ensure user-provided tools are in correct order before processing
• Implemented smart insertion logic to place the clip tool at the appropriate position based on tool weights

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
planet/constants.py	Defines _SUBSCRIPTION_TOOL_WEIGHT dictionary mapping tool names to processing order weights (1-4)
planet/subscription_request.py	Adds _validate_tool_order() and _insert_clip_tool() helper functions; updates build_request() to validate and correctly insert clip tool
tests/unit/test_subscription_request.py	Adds 17 comprehensive parametrized tests covering tool order validation and clip insertion scenarios

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The insertion logic uses dictionary enumeration indices instead of weight values, which creates an inconsistency with the validation logic. While this works correctly for tools defined in _SUBSCRIPTION_TOOL_WEIGHT, it conflates positional ordering with weight-based ordering.

Consider refactoring to use weight values directly for consistency with _validate_tool_order. For example, the logic could find the first tool with weight greater than 3, or if no such tool exists, find the last tool with weight less than or equal to 3 and insert after it.

Additionally, unknown tools (not in _SUBSCRIPTION_TOOL_WEIGHT) are silently ignored during position calculation, which could result in clip being inserted after them. This behavior should be documented or handled explicitly - consider whether unknown tools should cause an error or be treated as having a specific default weight.

Suggested change

	The clip tool is inserted based on its position in the _SUBSCRIPTION_TOOL_WEIGHT
	dictionary, ensuring it comes before any tool that appears after it in the
	processing order.
	Args:
	tool_list: List of tool configurations (modified in place).
	"""
	# Create a position mapping from the _SUBSCRIPTION_TOOL_WEIGHT dictionary
	tool_order = {name: idx for idx, name in enumerate(_SUBSCRIPTION_TOOL_WEIGHT.keys())}
	clip_position = tool_order['clip']
	insert_index = len(tool_list) # default to end
	for i, tool in enumerate(tool_list):
	tool_type = tool.get('type')
	if tool_type in tool_order:
	if tool_order[tool_type] > clip_position:
	insert_index = i
	break
	The clip tool is inserted based on its weight in the
	_SUBSCRIPTION_TOOL_WEIGHT mapping, ensuring it comes before any tool
	with a greater processing weight.
	Unknown tools (those whose type is not present in
	_SUBSCRIPTION_TOOL_WEIGHT) are ignored when computing the insertion
	position and therefore do not affect where the clip tool is placed.
	Args:
	tool_list: List of tool configurations (modified in place).
	"""
	clip_weight = _SUBSCRIPTION_TOOL_WEIGHT.get('clip')
	# If 'clip' does not have a defined weight, append it to the end to
	# avoid raising errors due to missing configuration.
	if clip_weight is None:
	tool_list.insert(len(tool_list), {'type': 'clip', 'parameters': {}})
	return
	insert_index = len(tool_list) # default to end
	for i, tool in enumerate(tool_list):
	tool_type = tool.get('type')
	curr_weight = _SUBSCRIPTION_TOOL_WEIGHT.get(tool_type)
	# Skip unknown tools; they do not participate in ordering
	if curr_weight is None:
	continue
	if curr_weight > clip_weight:
	insert_index = i
	break

[asonnenschein]

This is by design. _SUBSCRIPTION_TOOL_WEIGHT is added as the SDK source of truth for toolchain processing order, and the weight/key order of tools in the dictionary is based on processing order as defined in the documentation page on creating toolchains.

Subscriptions API will validate toolchain processing order according to the documented processing order. However, the SDK also includes the clip_to_source option as a value add, which appends a clip tool pre-flight to the processing sequence. This means that the SDK also needs to validate any user defined toolchain processing sequence, so that clip is inserted into a position in the processing sequence that is valid.

Per the documentation on creating toolchains, bandmath has the same weight as clip, with the caveat that bandmath must come after clip. Which means that when assuming a valid position in the toolchain processing sequence via clip_to_source, we can guarantee validity if clip comes first in it's weight class.

Finally, I took the feedback about unknown tools and added validation logic to raise an error if the user specifies any tool that isn't defined in the tools documentation page, and mirrored in the _SUBSCRIPTION_TOOL_WEIGHT.

[HenryW95]

I wonder if tool sequencing should be validated in the SDK at all, since it's already validated in the API? I worry about this getting out of sync when changes are made to tools in the API. On the other hand, I see how that info is necessary to store in order to insert clip in the correct place, so maybe there's no avoiding it.

[asonnenschein]

FWIW I didn't embark on this task with the intention of validating in the SDK, but as you pointed out, clip needs to be inserted in the correct position in the toolchain. That position is relative to other items in the toolchain, and I couldn't figure out a way to find that correct position without comparing the toolchain against the reference data.

[HenryW95]

That makes sense. We could add a comment on the precedence file in Subscriptions API to make future devs aware that it needs to be updated here as well.