more tools related to project by iamtekson · Pull Request #25 · iamtekson/GeoAgent

iamtekson · 2025-12-30T20:03:16Z

No description provided.

Copilot

Pull request overview

This PR adds project management tools to the GeoAgent QGIS plugin, enabling users to create, save, and load QGIS projects through the AI agent interface.

Introduces three new project management functions: create_new_qgis_project, save_qgis_project, and load_qgis_project
Implements thread-safe project loading with a new ProjectLoadDispatcher that handles project operations on the main Qt thread
Modifies the agent graph to change tool result handling from routing back to the LLM to ending directly after tool execution

Reviewed changes

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

Show a summary per file

File	Description
utils/project_loader.py	New module providing thread-safe QGIS project loading with ProjectLoadDispatcher and callback infrastructure
utils/canvas_refresh.py	Minor grammar fix in comment ("it will" → "will")
tools/io.py	Renames `new_qgis_project` to `create_new_qgis_project` and adds `save_qgis_project` and `load_qgis_project` functions with validation
tools/init.py	Updates exports to include renamed and new project management functions
llm/worker.py	Changes result signal type from AIMessage to generic object to support both AIMessage and ToolMessage
geo_agent.py	Adds ProjectLoadDispatcher initialization and `_project_load_callback` implementation with event loop synchronization
agents/graph.py	Enhances tool node error handling and changes graph edge from "tools" → "llm" to "tools" → END

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

Copilot · 2025-12-30T20:07:56Z

+            self._project_load_dispatcher.result_ready.connect(on_result_ready)
+
+            # queue the load operation on the main thread
+            QMetaObject.invokeMethod(
+                self._project_load_dispatcher,
+                "doLoadProject",
+                Qt.QueuedConnection,
+                Q_ARG(str, path),
+            )
+
+            # wait for the dispatcher to signal completion
+            loop = QEventLoop()
+
+            # max wait 30 seconds timeout
+            QTimer.singleShot(30000, loop.quit)
+            loop.exec_()
+
+            # disconnect signal to avoid memory leaks
+            self._project_load_dispatcher.result_ready.disconnect(on_result_ready)
+
+            # return the result that was set by the dispatcher
+            return self._project_load_dispatcher.result


There's a potential race condition: if an exception occurs during QMetaObject.invokeMethod or the timeout, the signal will remain connected, causing a memory leak. The disconnect should be in a finally block to ensure it always executes, or use a try-finally pattern around the entire signal handling logic.

Copilot · 2025-12-30T20:07:56Z

+        - save_qgis_project('/geoagent/my_project.qgs') 
+        - save_qgis_project('D:/geoagent/project_backup.qgz')  


There are two trailing spaces at the end of these comment lines, which is inconsistent with the rest of the codebase and can cause issues with some linters and version control systems.

Suggested change

- save_qgis_project('/geoagent/my_project.qgs')

- save_qgis_project('D:/geoagent/project_backup.qgz')

- save_qgis_project('/geoagent/my_project.qgs')

- save_qgis_project('D:/geoagent/project_backup.qgz')

Copilot · 2025-12-30T20:07:56Z

+            )
+
+            # wait for the dispatcher to signal completion
+            loop = QEventLoop()


The QEventLoop is created after it's used in the on_result_ready function closure. This will cause a NameError when on_result_ready is called because 'loop' is not yet defined at the time the function is created. The loop variable should be created before defining the on_result_ready function.

Copilot · 2025-12-30T20:07:57Z

+            except:
+                pass


Bare except clause silently catches all exceptions including system-exiting exceptions like KeyboardInterrupt and SystemExit. This can mask serious errors and make debugging difficult. Consider catching specific exception types or at least logging the exception before suppressing it.

Copilot · 2025-12-30T20:07:57Z

+            except:
+                pass


Bare except clause silently catches all exceptions including system-exiting exceptions like KeyboardInterrupt and SystemExit. This can mask serious errors and make debugging difficult. Consider catching specific exception types or at least logging the exception before suppressing it.

Copilot · 2025-12-30T20:07:57Z

+            loop = QEventLoop()
+
+            # max wait 30 seconds timeout
+            QTimer.singleShot(30000, loop.quit)


The magic number 30000 (30 seconds) is used directly without explanation. Consider defining it as a named constant like PROJECT_LOAD_TIMEOUT_MS = 30000 to improve code readability and maintainability.

Copilot · 2025-12-30T20:07:57Z

    graph.set_entry_point("llm")
    graph.add_conditional_edges("llm", should_use_tools, {"tools": "tools", END: END})
-    graph.add_edge("tools", "llm")
+    graph.add_edge("tools", END)


Changing the graph edge from "tools" → "llm" to "tools" → END fundamentally alters the agent's behavior. Previously, tool results would be sent back to the LLM for interpretation and natural language response generation. Now, tool execution results go directly to END, meaning the last message will be a ToolMessage with raw tool output instead of an AIMessage with a natural language response. This breaks the conversational flow as users will see raw tool outputs like "Success: Layer added..." instead of the AI interpreting and presenting results naturally.

Suggested change

graph.add_edge("tools", END)

graph.add_edge("tools", "llm")

Copilot · 2025-12-30T20:07:57Z

+            # max wait 30 seconds timeout
+            QTimer.singleShot(30000, loop.quit)
+            loop.exec_()


The timeout timer is started but there's no mechanism to stop it if the result_ready signal fires first. This means the timer will continue to run and could potentially call loop.quit() even after the loop has already quit from the signal, which could cause issues. Consider storing the timer and calling stop() on it after the loop exits.

Copilot · 2025-12-30T20:07:58Z

 Graph builder for GeoAgent: routes between general conversational mode and geoprocessing workflows.
 """
 from typing import List, Any
+from langchain_core.messages import SystemMessage, HumanMessage


Import of 'SystemMessage' is not used.
Import of 'HumanMessage' is not used.

Suggested change

from langchain_core.messages import SystemMessage, HumanMessage

iamtekson · 2025-12-30T20:13:12Z

@rabenojha can you work on Copilot suggestions? Let's try to implement the good tips and ignore unnecessary suggestions.

Logger and structured output

gemini default model switched from pro to flash

Copilot

Pull request overview

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

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

Copilot · 2025-12-30T23:52:04Z

+            _logger.info(f"Successfully loaded project from '{path}'")
+            return f"Success: Project loaded from '{path}'"
+        else:
+            _logger.error(f"Unknown error loading project from '{path}'")


The function returns None at the end when it should return an error message. After the except block catches an exception and logs it at line 527, control falls through without a return statement. This means the function will return None instead of an error message string, which is inconsistent with the function's return type annotation and other error paths.

Suggested change

_logger.error(f"Unknown error loading project from '{path}'")

_logger.error(f"Unknown error loading project from '{path}'")

return f"Error: Unknown error loading project from '{path}'"

Copilot · 2025-12-30T23:52:04Z

+            self.result["error"] = str(e)
+            try:
+                QgsMessageLog.logMessage(str(e), "GeoAgent", level=Qgis.Warning)
+            except:


Bare except clause will catch all exceptions including system exits and keyboard interrupts, which can make the code difficult to debug and interrupt. Consider catching specific exception types or at least Exception instead of using a bare except.

Suggested change

except:

except Exception:

Copilot · 2025-12-30T23:52:05Z

+	get_processing_logger,
+	set_processing_ui_log_handler,
+)
+
+__all__ = [
+	"UILogHandler",
+	"get_logger",
+	"UILogSignal",
+	"get_processing_logger",
+	"set_processing_ui_log_handler",


Using inconsistent indentation - tabs instead of spaces. The rest of the codebase uses spaces for indentation (as seen in other files), but this file uses tabs on lines 6 and 7. Python PEP 8 recommends using 4 spaces per indentation level, and mixing tabs and spaces can cause issues.

Suggested change

get_processing_logger,

set_processing_ui_log_handler,

)

__all__ = [

"UILogHandler",

"get_logger",

"UILogSignal",

"get_processing_logger",

"set_processing_ui_log_handler",

get_processing_logger,

set_processing_ui_log_handler,

)

__all__ = [

"UILogHandler",

"get_logger",

"UILogSignal",

"get_processing_logger",

"set_processing_ui_log_handler",

Copilot · 2025-12-30T23:52:05Z

+
+
+


Grammar fix looks good but there are two trailing blank lines (78-80) which is inconsistent with the project style. Most Python files should end with a single blank line.

Suggested change

Copilot · 2025-12-30T23:52:05Z

        - new_qgis_project('/geoagent/my_project.qgs')
        - new_qgis_project('/geoagent/test_project.qgz', 'QGIS Test Project')


The docstring examples still reference the old function name "new_qgis_project" instead of the new function name "create_new_qgis_project". This will confuse users reading the documentation.

Suggested change

- new_qgis_project('/geoagent/my_project.qgs')

- new_qgis_project('/geoagent/test_project.qgz', 'QGIS Test Project')

- create_new_qgis_project('/geoagent/my_project.qgs')

- create_new_qgis_project('/geoagent/test_project.qgz', 'QGIS Test Project')

Copilot · 2025-12-30T23:52:09Z

+            self.result["error"] = str(e)
+            try:
+                QgsMessageLog.logMessage(str(e), "GeoAgent", level=Qgis.Warning)
+            except:


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

Suggested change

except:

except Exception:

# Ignore logging failures to avoid masking the original exception.

Copilot · 2025-12-30T23:52:10Z

+            self.text_browser.verticalScrollBar().setValue(
+                self.text_browser.verticalScrollBar().maximum()
+            )
+        except Exception:


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

Copilot · 2025-12-30T23:52:10Z

+
+            # Update line count
+            self.line_count = self.text_browser.document().lineCount()
+        except Exception:


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

Copilot · 2025-12-30T23:52:10Z

+            # trigger UI updates
+            try:
+                project.readProject.emit()
+            except:


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

Copilot · 2025-12-30T23:52:10Z

+            self.result["error"] = str(e)
+            try:
+                QgsMessageLog.logMessage(str(e), "GeoAgent", level=Qgis.Warning)
+            except:


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

rabenojha and others added 6 commits December 30, 2025 19:43

add method for loading project

3109b31

fix bug, close #17

7b488bc

Merge branch 'dev' of github.com:iamtekson/GeoAgent into dev

6b4afd5

making tool node more predictable

18c2821

add method for saving projects

4449b73

Merge branch 'dev' of https://github.com/iamtekson/GeoAgent into dev

9b65b98

iamtekson requested a review from Copilot December 30, 2025 20:03

Copilot started reviewing on behalf of iamtekson December 30, 2025 20:04 View session

geoagent logging implemented

2a3883e

Copilot AI reviewed Dec 30, 2025

View reviewed changes

iamtekson and others added 6 commits December 30, 2025 13:19

debug message for io

23fa31f

structured output from invoke

ad03c01

Merge pull request #27 from iamtekson/logger

d6ffa37

Logger and structured output

gemini default model switched from pro to flash

89d5e66

Merge pull request #28 from iamtekson/logger

2f87c60

gemini default model switched from pro to flash

fix conflict issue between remove_layer and load_qgis_project functions

4de2b32

iamtekson assigned iamtekson and Copilot and unassigned iamtekson and Copilot Dec 30, 2025

iamtekson requested a review from Copilot December 30, 2025 23:46

Copilot started reviewing on behalf of iamtekson December 30, 2025 23:46 View session

Copilot AI reviewed Dec 30, 2025

View reviewed changes

minor fixes for Copilot review

c4cff89

rabenojha merged commit c6e0571 into main Dec 31, 2025
1 check passed

iamtekson deleted the dev branch December 31, 2025 01:10

		- save_qgis_project('/geoagent/my_project.qgs')
		- save_qgis_project('D:/geoagent/project_backup.qgz')

	_logger.error(f"Unknown error loading project from '{path}'")
	_logger.error(f"Unknown error loading project from '{path}'")
	return f"Error: Unknown error loading project from '{path}'"

		- new_qgis_project('/geoagent/my_project.qgs')
		- new_qgis_project('/geoagent/test_project.qgz', 'QGIS Test Project')

	except:
	except Exception:
	# Ignore logging failures to avoid masking the original exception.

Conversation

iamtekson commented Dec 30, 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 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!

iamtekson commented Dec 30, 2025

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants