working on #7 by iamtekson · Pull Request #21 · iamtekson/GeoAgent

iamtekson · 2025-12-29T04:36:13Z

No description provided.

Copilot

Pull request overview

This PR implements a comprehensive geoprocessing workflow system for QGIS integration, introducing algorithm discovery, selection, parameter extraction, and execution capabilities. The changes add substantial functionality for automated geospatial processing tasks.

Key Changes:

Added new processing agent graph that orchestrates multi-step geoprocessing workflows (routing → algorithm discovery → selection → parameter gathering → execution)
Implemented three new tools for algorithm introspection: list_processing_algorithms, get_algorithm_parameters, and find_processing_algorithm
Enhanced execute_processing to automatically load results into QGIS and handle both vector and raster outputs

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 26 comments.

Show a summary per file

File	Description
tools/geoprocessing.py	Added 4 new tools for algorithm discovery and execution; enhanced execute_processing with automatic layer loading
tools/init.py	Imported new processing tools but commented them out in TOOLS dict (needs clarification)
prompts/system.py	Added 3 new prompt templates for processing routing, algorithm selection, and parameter gathering
agents/states.py	Defined 5 new state classes for processing workflow (QGISLayerState, layerListState, RouteState, etc.)
agents/processing.py	New 794-line file implementing complete processing graph with 7 nodes and comprehensive logging
agents/graph.py	Integrated processing graph into unified graph builder; minor formatting fixes
agents/init.py	Updated exports to include ProcessingState and processing-related functions

Comments suppressed due to low confidence (4)

agents/graph.py:87

Inconsistent whitespace handling. Line 87 removes trailing whitespace, but other similar lines (e.g., 72, 80) don't have this cleanup. This inconsistency suggests the whitespace fix on line 87 is incidental to other changes and may not have been intentional.
agents/graph.py:17
The function signature uses any as the return type hint. This should be more specific or use Any from typing (which is already imported). Using the lowercase any is not a valid type hint.
agents/graph.py:62
The function signature uses any as the return type hint. This should be more specific or use Any from typing. Using the lowercase any is not a valid type hint.
agents/graph.py:10
Import of 'HumanMessage' is not used.

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

Copilot · 2025-12-29T04:42:23Z

+                lyr = iface.addVectorLayer(
+                    output_layer_obj, f"Result - {algorithm.split(':')[-1]}", "ogr"
+                )
+                if lyr and lyr.isValid():
+                    layer_added = True
+                else:
+                    # Try raster
+                    lyr = iface.addRasterLayer(
+                        output_layer_obj, f"Result - {algorithm.split(':')[-1]}"
+                    )
+                    if lyr and lyr.isValid():
+                        layer_added = True


The function attempts to add vector layers first, then falls back to raster layers. However, invalid layers may still be added to the project. Consider checking layer validity before adding to the project, and potentially removing invalid layers that were added. Also, the function doesn't handle other layer types (e.g., mesh layers) that QGIS processing might produce.

Copilot · 2025-12-29T04:42:24Z

+    try:
+        layers_result = list_qgis_layers.invoke({})
+        # Extract layer names from the result
+        layer_names = re.findall(r"^(\d+)\.\s+(\w+)", layers_result, re.MULTILINE)


The regex pattern r"^(\d+)\.\s+(\w+)" is too restrictive. It only matches layer names containing word characters (letters, digits, underscore). Layer names in QGIS can contain spaces, hyphens, and other characters. This will fail to extract layer names like "My Layer-2023" or "data file.shp". Consider using a more permissive pattern like r"^(\d+)\.\s+(.+)$" to capture the full layer name.

Suggested change

layer_names = re.findall(r"^(\d+)\.\s+(\w+)", layers_result, re.MULTILINE)

layer_names = re.findall(r"^(\d+)\.\s+(.+)$", layers_result, re.MULTILINE)

Copilot · 2025-12-29T04:42:24Z

+        try:
+            # Use find_processing_algorithm to get relevant candidates
+            # LLM will select the best match in the next node
+            result = find_processing_algorithm.invoke({"query": query, "limit": 500})


The discover_algorithms_node function uses a limit of 500 algorithms (line 312), but this could be excessive and impact performance. The default limit for find_processing_algorithm is 20, which suggests 500 may be unnecessarily high. Consider using a more reasonable limit or making it configurable based on the use case.

Copilot · 2025-12-29T04:42:24Z

+                error_msg = f"Missing required parameters: {', '.join(missing_required)}. LLM extracted: {parameters}"
+                _logger.warning(error_msg)


The error message includes implementation details ("LLM extracted: {parameters}") that may expose internal state and confuse users. Consider simplifying this to focus on what the user needs to provide, or move the extracted parameters detail to debug logging only.

Suggested change

error_msg = f"Missing required parameters: {', '.join(missing_required)}. LLM extracted: {parameters}"

_logger.warning(error_msg)

error_msg = f"Missing required parameters: {', '.join(missing_required)}."

_logger.warning(error_msg)

_logger.debug(

"LLM-extracted parameters when checking for missing required values: %s",

parameters,

)

Copilot · 2025-12-29T04:42:24Z

+    return inferred
+
+
+def build_processing_graph(llm) -> any:


The function signature uses any as the return type hint. This should be more specific. Consider using the actual type returned by graph.compile(), such as CompiledGraph from langgraph, or at minimum use Any from typing instead of the lowercase any.

Copilot · 2025-12-29T04:42:28Z

+        # candidates_text = "\n".join(
+        #     [f"  - {c['id']}: {c['name']}" for c in candidates[:500]]  # Limit to 500
+        # )


The comment formatting is inconsistent. Line 362 has a regular Python comment while line 363 has a similar comment. Line 363's comment should use consistent formatting with the surrounding code, or both commented lines should be removed if not needed.

Suggested change

# candidates_text = "\n".join(

# [f" - {c['id']}: {c['name']}" for c in candidates[:500]] # Limit to 500

# )

Copilot · 2025-12-29T04:42:29Z

+            _logger.warning("Using first candidate as fallback")
+            selected_alg = candidates[0]["id"] if candidates else None
+            confidence = 0.5
+            reasoning = "Error in selection, using first candidate"


Variable confidence is not used.

Suggested change

reasoning = "Error in selection, using first candidate"

reasoning = "Error in selection, using first candidate"

_logger.info(

f"Selected algorithm (fallback): {selected_alg} (confidence: {confidence}, reasoning: {reasoning})"

)

Copilot · 2025-12-29T04:42:29Z

+            confidence = 0.5
+            reasoning = "Error in selection, using first candidate"


Variable reasoning is not used.

Suggested change

confidence = 0.5

reasoning = "Error in selection, using first candidate"

Copilot · 2025-12-29T04:42:29Z

+from ..tools import (
+    list_processing_algorithms,
+    find_processing_algorithm,
+    get_algorithm_parameters,
+    execute_processing,
+    list_qgis_layers,
+)


Import of 'list_processing_algorithms' is not used.

Copilot · 2025-12-29T04:42:29Z

+                    _logger.debug(
+                        f"Inferred {param_name} = {value} (extracted from query)"
+                    )
+                except ValueError:


'except' clause does nothing but pass and there is no explanatory comment.

Suggested change

except ValueError:

except ValueError:

# If parsing fails, just skip inferring this numeric parameter

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 19 comments.

Comments suppressed due to low confidence (3)

agents/graph.py:17

The return type annotation uses any which is not a valid Python type. It should be Any from the typing module (which needs to be imported) or a more specific type like CompiledGraph if available from LangGraph.
agents/graph.py:62
The return type annotation uses any which is not a valid Python type. It should be Any from the typing module (which needs to be imported) or a more specific type like CompiledGraph if available from LangGraph.
agents/graph.py:10
Import of 'HumanMessage' is not used.

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

Copilot · 2025-12-30T00:20:48Z

+        # execute_processing,
+        # list_processing_algorithms,
+        # get_algorithm_parameters,
+        # find_processing_algorithm,


The new geoprocessing tools are imported but commented out in the TOOLS dictionary. This means they are not actually available for use by the agent system. If these tools should not be enabled yet, consider removing the imports and adding a comment explaining why they're disabled.

Copilot · 2025-12-30T00:20:48Z

+# Create global logger instance
+_logger = _setup_logger()


The global _logger variable is created at module import time, which means a log file is created even if the processing functionality is never used. Consider lazy initialization of the logger when build_processing_graph is first called.

Suggested change

# Create global logger instance

_logger = _setup_logger()

# Logger instance (initialized lazily to avoid creating log files on import)

_logger: Optional[logging.Logger] = None

Copilot · 2025-12-30T00:20:49Z

+            tool_inst = TOOLS[call["name"]]
+            result = tool_inst.invoke(call["args"])
+            tool_messages.append(ToolMessage(content=result, tool_call_id=call["id"]))


The tool_node function doesn't handle potential KeyError when accessing TOOLS[call["name"]]. If a tool name is not found in the TOOLS dictionary, this will raise an unhandled exception. Consider adding error handling or validation.

Suggested change

tool_inst = TOOLS[call["name"]]

result = tool_inst.invoke(call["args"])

tool_messages.append(ToolMessage(content=result, tool_call_id=call["id"]))

# Safely resolve the tool instance to avoid KeyError if the name is unknown

tool_name = call.get("name") if isinstance(call, dict) else None

if not tool_name:

_logger.warning("Skipping tool call without a valid 'name': %r", call)

continue

tool_inst = TOOLS.get(tool_name)

if tool_inst is None:

_logger.error("Requested tool '%s' not found in TOOLS registry; skipping.", tool_name)

continue

result = tool_inst.invoke(call["args"])

tool_call_id = call.get("id") if isinstance(call, dict) else None

tool_messages.append(ToolMessage(content=result, tool_call_id=tool_call_id))

Copilot · 2025-12-30T00:20:49Z

+    "_setup_logger",
+]


The _setup_logger function is exported in __all__ with a leading underscore, which conventionally indicates it's a private/internal function. Private functions should not be included in the public API via __all__.

Suggested change

"_setup_logger",

]

]

Copilot · 2025-12-30T00:20:49Z

+class DiscoveryState(RouteState, total=False):
+    """State after algorithm discovery."""
+
+    algorithm_candidates: Optional[list]


The algorithm_candidates field uses the generic list type instead of a more specific type hint. Consider using List[Dict[str, Any]] to better document the structure of algorithm candidates (which contain 'id', 'name', and 'provider' fields).

Copilot · 2025-12-30T00:20:52Z

+            # Fallback: pick first candidate
+            _logger.warning("Using first candidate as fallback")
+            selected_alg = candidates[0]["id"] if candidates else None
+            confidence = 0.1


Variable confidence is not used.

Copilot · 2025-12-30T00:20:52Z

+            _logger.warning("Using first candidate as fallback")
+            selected_alg = candidates[0]["id"] if candidates else None
+            confidence = 0.1
+            reasoning = "Error in selection, using first candidate"


Variable reasoning is not used.

Copilot · 2025-12-30T00:20:52Z

+            # Default value if available
+            try:
+                item["default"] = p.defaultValue()
+            except Exception:


'except' clause does nothing but pass and there is no explanatory comment.

Copilot · 2025-12-30T00:20:52Z

+                        item["allowMultiple"] = bool(
+                            getattr(p, "allowMultiple", lambda: False)()
+                        )
+                    except Exception:


'except' clause does nothing but pass and there is no explanatory comment.

Copilot · 2025-12-30T00:20:52Z

+                        )
+                    except Exception:
+                        pass
+            except Exception:


'except' clause does nothing but pass and there is no explanatory comment.

Copilot · 2025-12-30T00:41:40Z

@iamtekson I've opened a new pull request, #22, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: iamtekson <39838116+iamtekson@users.noreply.github.com>

…ion limit configurable Co-authored-by: iamtekson <39838116+iamtekson@users.noreply.github.com>

…ng conversion Co-authored-by: iamtekson <39838116+iamtekson@users.noreply.github.com>

Fix infinite loop in processing graph conditional routing

iamtekson · 2025-12-30T01:47:24Z

Referenced to #7

iamtekson · 2025-12-31T20:36:44Z

#7

working on #7

cf05d48

iamtekson requested a review from Copilot December 29, 2025 04:36

Copilot started reviewing on behalf of iamtekson December 29, 2025 04:36 View session

Copilot AI reviewed Dec 29, 2025

View reviewed changes

iamtekson added 2 commits December 29, 2025 12:34

updated the processing graph #7

d946ae4

remove unnecessary code and select only native algs for now

ec9258f

iamtekson requested a review from Copilot December 30, 2025 00:15

Copilot started reviewing on behalf of iamtekson December 30, 2025 00:16 View session

Copilot AI reviewed Dec 30, 2025

View reviewed changes

Initial plan

431c781

Copilot AI mentioned this pull request Dec 30, 2025

Fix infinite loop in processing graph conditional routing #22

Merged

Copilot AI and others added 7 commits December 30, 2025 00:45

Fix infinite loop issue in processing graph

8aa2278

Co-authored-by: iamtekson <39838116+iamtekson@users.noreply.github.com>

Address code review feedback - improve error handling and make recurs…

19b1821

…ion limit configurable Co-authored-by: iamtekson <39838116+iamtekson@users.noreply.github.com>

Polish code based on final review - clarify comment and simplify stri…

e2166a6

…ng conversion Co-authored-by: iamtekson <39838116+iamtekson@users.noreply.github.com>

fix minor things

b2ca9c4

Merge pull request #22 from iamtekson/copilot/sub-pr-21

1ccf6a9

Fix infinite loop in processing graph conditional routing

moved max iteration to settings

bd11444

remove recursive limit as it is not available

2f4d86b

iamtekson merged commit 0c0fa24 into main Dec 30, 2025
1 check passed

iamtekson deleted the dev-geo branch December 30, 2025 01:12

iamtekson mentioned this pull request Dec 30, 2025

Run geoprocessing tool via llm #7

Closed

	layer_names = re.findall(r"^(\d+)\.\s+(\w+)", layers_result, re.MULTILINE)
	layer_names = re.findall(r"^(\d+)\.\s+(.+)$", layers_result, re.MULTILINE)

		error_msg = f"Missing required parameters: {', '.join(missing_required)}. LLM extracted: {parameters}"
		_logger.warning(error_msg)

	# candidates_text = "\n".join(
	# [f" - {c['id']}: {c['name']}" for c in candidates[:500]] # Limit to 500
	# )

		confidence = 0.5
		reasoning = "Error in selection, using first candidate"

	except ValueError:
	except ValueError:
	# If parsing fails, just skip inferring this numeric parameter

-            tool_inst = TOOLS[call["name"]]
-            result = tool_inst.invoke(call["args"])
-            tool_messages.append(ToolMessage(content=result, tool_call_id=call["id"]))
+            # Safely resolve the tool instance to avoid KeyError if the name is unknown
+            tool_name = call.get("name") if isinstance(call, dict) else None
+            if not tool_name:
+                _logger.warning("Skipping tool call without a valid 'name': %r", call)
+                continue
+            tool_inst = TOOLS.get(tool_name)
+            if tool_inst is None:
+                _logger.error("Requested tool '%s' not found in TOOLS registry; skipping.", tool_name)
+                continue
+            result = tool_inst.invoke(call["args"])
+            tool_call_id = call.get("id") if isinstance(call, dict) else None
+            tool_messages.append(ToolMessage(content=result, tool_call_id=tool_call_id))

Conversation

iamtekson commented Dec 29, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Dec 30, 2025

Uh oh!

Uh oh!

iamtekson commented Dec 30, 2025

Uh oh!

iamtekson commented Dec 31, 2025

Uh oh!

Reviewers