From dd14f65d935e74c5c1dc6afb38402e32f7cbfac6 Mon Sep 17 00:00:00 2001 From: Shawn Hurley Date: Tue, 4 Feb 2025 12:43:28 -0500 Subject: [PATCH 1/4] :sparkles: package/symbol not found errors are now collapsable for a file * This also adds the clean option to maven, so that maven is recompiled everytime, this is needed when the pom.xml does not yet have the maven compiler plugin. * The lines numbers are not used in the dependency agent, because of this we can assume we know that we don't need to pass it through. Signed-off-by: Shawn Hurley --- .../task_runner/compiler/maven_validator.py | 96 ++++++++++++++++++- 1 file changed, 91 insertions(+), 5 deletions(-) diff --git a/kai/reactive_codeplanner/task_runner/compiler/maven_validator.py b/kai/reactive_codeplanner/task_runner/compiler/maven_validator.py index 3b469aa3..8aa40f70 100755 --- a/kai/reactive_codeplanner/task_runner/compiler/maven_validator.py +++ b/kai/reactive_codeplanner/task_runner/compiler/maven_validator.py @@ -3,6 +3,7 @@ import re import subprocess # trunk-ignore(bandit/B404) from dataclasses import dataclass, field +from functools import cached_property from pathlib import Path from shutil import which from typing import Optional, Type @@ -10,9 +11,10 @@ from opentelemetry import trace from kai.constants import ENV -from kai.logging.logging import get_logger +from kai.logging.logging import TRACE, get_logger from kai.reactive_codeplanner.task_manager.api import ( RpcClientConfig, + Task, ValidationError, ValidationResult, ValidationStep, @@ -86,6 +88,57 @@ def from_match( ) +@dataclass +class CollapsedMavenCompilerError(MavenCompilerError): + lines: list[int] | None = None + + def __eq__(self, other: object) -> bool: + if not isinstance(other, self.__class__): + return False + if not super().__eq__(other): + return False + if self.error_lines and other.error_lines: + return self.error_lines == other.error_lines + return False + + def __hash__(self) -> int: + if self.error_lines: + return hash((super().__hash__(), tuple(self.error_lines))) + return super().__hash__() + + def fuzzy_equals(self, error2: Task, offset: int = 1) -> bool: + if not isinstance(error2, self.__class__): + return False + if not super().__eq__(error2): + return False + if not self.error_lines or not error2.error_lines: + return False + matching_lines: list[int] = [] + for line in self.error_lines: + for error2_line in error2.error_lines: + if line == error2_line: + matching_lines.append(line) + + # All the errors from this are still in the error, this makes them equal + if len(matching_lines) == len(self.error_lines): + return True + + # we didn't find any matches, so error2 is a wholly new set of issues + if len(matching_lines) == 0: + return False + + # Now, we have some errors that have not be resolved and some that have, We will continue to process this task + # in the future we should have a way of creating a new task, with the same priority as this one + # that makes it so we take all the problems + return True + + @cached_property + def error_lines(self) -> list[int] | None: + if self.lines: + self.lines.sort() + return self.lines + + # Subclasses for specific error categories @dataclass(eq=False) class BuildError(MavenCompilerError): @@ -100,14 +153,14 @@ class DependencyResolutionError(MavenCompilerError): @dataclass(eq=False) -class SymbolNotFoundError(MavenCompilerError): +class SymbolNotFoundError(CollapsedMavenCompilerError): missing_symbol: Optional[str] = None symbol_location: Optional[str] = None priority: int = 2 @dataclass(eq=False) -class PackageDoesNotExistError(MavenCompilerError): +class PackageDoesNotExistError(CollapsedMavenCompilerError): priority: int = 1 missing_package: Optional[str] = None @@ -144,7 +197,7 @@ def run_maven(source_directory: Path = Path(".")) -> tuple[int, str, Optional[Pa Also returns the path to the pom.xml file. """ mavenPath = which("mvn") or "mvn" - cmd = [mavenPath, "compile"] + cmd = [mavenPath, "clean", "compile"] # trunk-ignore-begin(bandit/B603) try: process = subprocess.run( @@ -157,6 +210,7 @@ def run_maven(source_directory: Path = Path(".")) -> tuple[int, str, Optional[Pa env=ENV, ) pom_file_path = source_directory / "pom.xml" + logger.log(TRACE, "running maven process: %s", process) return (process.returncode, process.stdout, pom_file_path) # trunk-ignore-end(bandit/B603) except FileNotFoundError: @@ -487,13 +541,45 @@ def deduplicate_errors(errors: list[MavenCompilerError]) -> list[MavenCompilerEr """ Deduplicates errors based on file, line, column, and message. """ + file_type_to_collapsable_error: dict[str, CollapsedMavenCompilerError] = {} unique_errors = [] seen_errors = set() for error in errors: error_id = (error.file, error.line, error.column, error.message) if error_id not in seen_errors: seen_errors.add(error_id) - unique_errors.append(error) + if isinstance(error, CollapsedMavenCompilerError): + dict_key = f"{error.file}-{error.__class__}" + if dict_key in file_type_to_collapsable_error.keys(): + new_error = file_type_to_collapsable_error.get(dict_key) + if not new_error: + continue + if not new_error.lines: + new_error.lines = [new_error.line] + ## Setting this, because we don't want these collapsed errors + ## to every skip fuzzy matching. + new_error.line = 0 + new_error.lines.append(error.line) + else: + file_type_to_collapsable_error[dict_key] = error + else: + unique_errors.append(error) + + for file_path_type, collapsed_error in file_type_to_collapsable_error.items(): + logger.debug( + "adding collapsed_error: %s-lines:%s -- for key: %s", + collapsed_error, + collapsed_error.error_lines, + file_path_type, + ) + if not isinstance(collapsed_error, MavenCompilerError): + logger.debug( + "something went wrong, invalid error in collapsable: %s", + collapsed_error, + ) + continue + unique_errors.append(collapsed_error) + return unique_errors From 1deffbfdfc0a7f2e22010f6ec027c0e2dc2a8ad6 Mon Sep 17 00:00:00 2001 From: Shawn Hurley Date: Tue, 4 Feb 2025 14:13:22 -0500 Subject: [PATCH 2/4] Fixing up review comments from previous PR Signed-off-by: Shawn Hurley --- kai/reactive_codeplanner/task_runner/analyzer_lsp/api.py | 4 ++-- kai/rpc_server/server.py | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/kai/reactive_codeplanner/task_runner/analyzer_lsp/api.py b/kai/reactive_codeplanner/task_runner/analyzer_lsp/api.py index 856e1713..b702cf80 100644 --- a/kai/reactive_codeplanner/task_runner/analyzer_lsp/api.py +++ b/kai/reactive_codeplanner/task_runner/analyzer_lsp/api.py @@ -1,6 +1,6 @@ from dataclasses import dataclass from functools import cached_property -from typing import Any, List +from typing import Any from kai.analyzer_types import Incident, RuleSet, Violation from kai.logging.logging import TRACE, get_logger @@ -11,7 +11,7 @@ @dataclass(eq=False, kw_only=True) class AnalyzerRuleViolation(ValidationError): - incidents: List[Incident] + incidents: list[Incident] # NOTE(JonahSussman): Violation contains a list of Incidents, and RuleSet # contains a list of Violations. We have another class, ExtendedIncident, diff --git a/kai/rpc_server/server.py b/kai/rpc_server/server.py index 64d9ff1b..23bf42a2 100644 --- a/kai/rpc_server/server.py +++ b/kai/rpc_server/server.py @@ -509,14 +509,13 @@ def simple_chat_message(msg: str) -> None: params.incidents.sort() grouped_incidents_by_files = [ - list((g)) for _, g in groupby(params.incidents, key=attrgetter("uri")) + list(g) for _, g in groupby(params.incidents, key=attrgetter("uri")) ] for incidents in grouped_incidents_by_files: # group incidents by violation grouped_violations = [ - list((g)) - for _, g in groupby(incidents, key=attrgetter("violation_name")) + list(g) for _, g in groupby(incidents, key=attrgetter("violation_name")) ] for violation_incidents in grouped_violations: From 987dee32164b6c590183a1ebb5e2dc5e49f5b002 Mon Sep 17 00:00:00 2001 From: Shawn Hurley Date: Tue, 4 Feb 2025 14:37:35 -0500 Subject: [PATCH 3/4] set opentelemetry to pass mypy Signed-off-by: Shawn Hurley --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index c6cd154b..3936b45e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -43,7 +43,7 @@ dependencies = [ "lxml==5.3.0", "boto3==1.36.9", # Allows Amazon Bedrock to work "pylspclient==0.1.2", # used for talking to RPC clients over stdin/stdout - "opentelemetry-sdk", + "opentelemetry-sdk==1.29.0", "opentelemetry-api", "opentelemetry-exporter-otlp", "opentelemetry-instrumentation-threading", From 5f40880d6d6a421b39b95a7514f27fc8cad4f203 Mon Sep 17 00:00:00 2001 From: Shawn Hurley Date: Tue, 4 Feb 2025 17:40:10 -0500 Subject: [PATCH 4/4] Fixing issues with files and compiler messages * Remove file from collapsable key for packages because they work across the project * Add the ability to get the compiler message from the task for maven compiler errors, so that a individual error can define it's own message * fixed up the prompt from dependnecy agent to handle new message * added try around spliting lines of code in maven compiler agent becuase sometimes the files are rewritten very poorly Signed-off-by: Shawn Hurley --- .../dependency_agent/dependency_agent.py | 6 ++- .../agent/maven_compiler_fix/agent.py | 11 ++++- .../compiler/compiler_task_runner.py | 2 +- .../task_runner/compiler/maven_validator.py | 44 +++++++++++++++++-- 4 files changed, 55 insertions(+), 8 deletions(-) diff --git a/kai/reactive_codeplanner/agent/dependency_agent/dependency_agent.py b/kai/reactive_codeplanner/agent/dependency_agent/dependency_agent.py index 473ce0c9..28f39c6b 100644 --- a/kai/reactive_codeplanner/agent/dependency_agent/dependency_agent.py +++ b/kai/reactive_codeplanner/agent/dependency_agent/dependency_agent.py @@ -150,8 +150,10 @@ class MavenDependencyAgent(Agent): If you are at the point of editing, add the final answer of what you did -Message:{message} - """ +Message: + + {message} +""" ) AgentMethods = TypedDict( diff --git a/kai/reactive_codeplanner/agent/maven_compiler_fix/agent.py b/kai/reactive_codeplanner/agent/maven_compiler_fix/agent.py index 94dd04b4..e6aa195d 100644 --- a/kai/reactive_codeplanner/agent/maven_compiler_fix/agent.py +++ b/kai/reactive_codeplanner/agent/maven_compiler_fix/agent.py @@ -2,12 +2,15 @@ from langchain_core.messages import BaseMessage, HumanMessage, SystemMessage from kai.llm_interfacing.model_provider import ModelProvider +from kai.logging.logging import get_logger from kai.reactive_codeplanner.agent.api import Agent, AgentRequest, AgentResult from kai.reactive_codeplanner.agent.maven_compiler_fix.api import ( MavenCompilerAgentRequest, MavenCompilerAgentResult, ) +logger = get_logger(__name__) + class MavenCompilerAgent(Agent): system_message = SystemMessage( @@ -54,7 +57,13 @@ def execute(self, ask: AgentRequest) -> AgentResult: if not isinstance(ask, MavenCompilerAgentRequest): return AgentResult() - line_of_code = ask.file_contents.split("\n")[ask.line_number] + try: + line_of_code = ask.file_contents.split("\n")[ask.line_number] + except Exception: + logger.exception( + "unable to split file contents and get line from linenumber" + ) + return MavenCompilerAgentResult() compile_errors = f"Line of code: {line_of_code};\n{ask.message}" content = self.chat_message_template.render( diff --git a/kai/reactive_codeplanner/task_runner/compiler/compiler_task_runner.py b/kai/reactive_codeplanner/task_runner/compiler/compiler_task_runner.py index 47dceced..2f7cdbcb 100644 --- a/kai/reactive_codeplanner/task_runner/compiler/compiler_task_runner.py +++ b/kai/reactive_codeplanner/task_runner/compiler/compiler_task_runner.py @@ -86,7 +86,7 @@ def execute_task(self, rcm: RepoContextManager, task: Task) -> TaskResult: task, src_file_contents, task.line, - task.message, + task.compiler_error_message(), ) ) diff --git a/kai/reactive_codeplanner/task_runner/compiler/maven_validator.py b/kai/reactive_codeplanner/task_runner/compiler/maven_validator.py index 8aa40f70..f4793c19 100755 --- a/kai/reactive_codeplanner/task_runner/compiler/maven_validator.py +++ b/kai/reactive_codeplanner/task_runner/compiler/maven_validator.py @@ -2,6 +2,7 @@ import re import subprocess # trunk-ignore(bandit/B404) +from abc import ABC, abstractmethod from dataclasses import dataclass, field from functools import cached_property from pathlib import Path @@ -87,9 +88,11 @@ def from_match( details=details.copy(), ) + def compiler_error_message(self) -> str: + return self.message -@dataclass -class CollapsedMavenCompilerError(MavenCompilerError): + +class CollapsedMavenCompilerError(ABC, MavenCompilerError): lines: list[int] | None = None def __eq__(self, other: object) -> bool: @@ -138,6 +141,10 @@ def error_lines(self) -> list[int] | None: self.lines.sort() return self.lines + @abstractmethod + def get_collapsable_key(self) -> str: + pass + # Subclasses for specific error categories @dataclass(eq=False) @@ -151,6 +158,9 @@ class DependencyResolutionError(MavenCompilerError): project: str = "" goal: str = "" + def compiler_error_message(self) -> str: + return self.message + @dataclass(eq=False) class SymbolNotFoundError(CollapsedMavenCompilerError): @@ -158,12 +168,32 @@ class SymbolNotFoundError(CollapsedMavenCompilerError): symbol_location: Optional[str] = None priority: int = 2 + def compiler_error_message(self) -> str: + if self.missing_symbol and self.symbol_location: + return f"{self.message}: {self.missing_symbol} at {self.symbol_location}" + if self.missing_symbol: + return f"{self.message}: {self.missing_symbol}" + return self.message + + def get_collapsable_key(self) -> str: + return f"{self.file}-{self.__class__}-{self.missing_symbol}" + @dataclass(eq=False) class PackageDoesNotExistError(CollapsedMavenCompilerError): priority: int = 1 missing_package: Optional[str] = None + def compiler_error_message(self) -> str: + if self.missing_package: + return f"{self.message}: {self.missing_package}" + else: + return self.message + + # This does not have to be for file, because the package not existing is at the project level + def get_collapsable_key(self) -> str: + return f"{self.__class__}-{self.missing_package}" + @dataclass(eq=False) class SyntaxError(MavenCompilerError): @@ -175,6 +205,12 @@ class TypeMismatchError(MavenCompilerError): expected_type: Optional[str] = None found_type: Optional[str] = None + def compiler_error_message(self) -> str: + if self.expected_type and self.found_type: + return f"{self.message}\nexpected type: {self.expected_type}\nfound type: {self.found_type}" + else: + return self.message + @dataclass(eq=False) class AnnotationError(MavenCompilerError): @@ -549,7 +585,7 @@ def deduplicate_errors(errors: list[MavenCompilerError]) -> list[MavenCompilerEr if error_id not in seen_errors: seen_errors.add(error_id) if isinstance(error, CollapsedMavenCompilerError): - dict_key = f"{error.file}-{error.__class__}" + dict_key = error.get_collapsable_key() if dict_key in file_type_to_collapsable_error.keys(): new_error = file_type_to_collapsable_error.get(dict_key) if not new_error: @@ -557,7 +593,7 @@ def deduplicate_errors(errors: list[MavenCompilerError]) -> list[MavenCompilerEr if not new_error.lines: new_error.lines = [new_error.line] ## Setting this, because we don't want these collapsed errors - ## to every skip fuzzy matching. + ## to ever skip fuzzy matching, which not matching line numbers will do new_error.line = 0 new_error.lines.append(error.line) else: