Support VertexAI

[CTY-git]

PR Checklist

• The commit message follows our guidelines: Code of conduct
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)
• Does this PR introduce a breaking change?
• Include PR in release notes?

PR Type

• Bugfix
• Feature
• Refactoring
• Build /CI
• Documentation
• Others

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Other information

[patched-admin]

File Changed: patchwork/common/client/llm/aio.py

Rule 1: Do not ignore potential bugs in the code

Details: A potential bug has been introduced in the client initialization process. The code previously validated model support using get_models() but now only calls test(). However, test() in the AioLlmClient is implemented as an empty pass-through method, which could lead to silent failures in model validation.

Affected Code Snippet:

try:
    client.test()
    self.__clients.append(client)
except Exception as e:
    logger.error(f"{client.__class__.__name__} Failed with exception: {e}")

Start Line: 34
End Line: 37

---

Details: Another potential bug exists in the handling of Google client initialization. The new code uses boolean operations that could lead to unexpected behavior with falsy values.

Affected Code Snippet:

is_gcp = bool(client_args.get("is_gcp") or os.environ.get("GOOGLE_GENAI_USE_VERTEXAI") or False)
if google_key is not None or is_gcp:
    client = GoogleLlmClient(api_key=google_key, is_gcp=is_gcp)

Start Line: 219
End Line: 221

Rule 2: Do not overlook possible security vulnerabilities

Details: The code introduces a potential security vulnerability by logging exception details which could contain sensitive information from the LLM client initialization process.

Affected Code Snippet:

except Exception as e:
    logger.error(f"{client.__class__.__name__} Failed with exception: {e}")

Start Line: 36
End Line: 37

File Changed: patchwork/common/client/llm/anthropic.py

Rule 1: Do not ignore potential bugs in the code

Details: Potential bug introduced by removing the cached model list functionality. The get_models() method was replaced with an empty test() method, which could break functionality that depends on retrieving the list of supported models.

Affected Code Snippet:

-    @lru_cache(maxsize=None)
-    def get_models(self) -> set[str]:
-        return self.__definitely_allowed_models.union(set(f"{self.__allowed_model_prefix}*"))
+    def test(self):
+        return

Start Line: 245
End Line: 249

---

Rule 3: Do not deviate from original coding standards

Details: Code standards violation observed. The new test() method lacks type hints and docstring, which were present in the original codebase. Additionally, the empty method with just return statement doesn't follow the established coding patterns.

Affected Code Snippet:

+    def test(self):
+        return

Start Line: 248
End Line: 249

---

Summary of Findings:

1. The removal of get_models() method and its replacement with an empty test() method introduces a potential functional regression.
2. The new code deviates from established coding standards by lacking type hints and proper documentation.
3. The changes appear to be incomplete or placeholder code that shouldn't be merged in its current state.

Recommendations:

1. Restore the get_models() functionality or provide a proper replacement
2. If test() method is needed, add appropriate type hints and documentation
3. Remove the empty test() method if it's not serving any purpose

File Changed: patchwork/common/client/llm/google_.py

Rule 1: Do not ignore potential bugs in the code

Details: The error handling in the is_prompt_supported method could mask important errors by returning -1, potentially causing silent failures.

Affected Code Snippet:

        except Exception as e:
            logger.debug(f"Error during token count at GoogleLlmClient: {e}")
            return -1

Start Line: 230
End Line: 232

---

Rule 2: Do not overlook possible security vulnerabilities

Details: The code disables all safety settings for the AI model by setting all thresholds to BLOCK_NONE/OFF, which could lead to generation of harmful content.

Affected Code Snippet:

    __SAFETY_SETTINGS = [
        dict(category="HARM_CATEGORY_HATE_SPEECH", threshold="BLOCK_NONE"),
        dict(category="HARM_CATEGORY_SEXUALLY_EXPLICIT", threshold="BLOCK_NONE"),
        dict(category="HARM_CATEGORY_DANGEROUS_CONTENT", threshold="BLOCK_NONE"),
        dict(category="HARM_CATEGORY_HARASSMENT", threshold="BLOCK_NONE"),
        dict(category="HARM_CATEGORY_CIVIC_INTEGRITY", threshold="BLOCK_NONE"),
    ]

Start Line: 56
End Line: 62

File Changed: patchwork/common/client/llm/openai_.py

Rule 1: Do not ignore potential bugs in the code

Details: Significant functionality change and potential bug introduction. The get_models() method has been replaced with a test() method that doesn't return any value, and the previous functionality is broken. This affects the is_model_supported() method which now directly calls _cached_list_models_from_openai() instead of using the cached result from get_models().

Affected Code Snippet:

def test(self):
    if self.__is_not_openai_url():
        return

    _cached_list_models_from_openai(self.__api_key)
    return

Start Line: 99
End Line: 103

---

Rule 3: Do not deviate from original coding standards

Details: The changes deviate from the original coding standards in several ways:

1. Replacing a typed return method get_models() -> set[str] with an untyped test() method
2. Not following the established pattern of caching model information
3. Introducing redundant return statement
4. Breaking the method naming convention (test is too generic)

Affected Code Snippet:

def test(self):
    if self.__is_not_openai_url():
        return

    _cached_list_models_from_openai(self.__api_key)
    return

Start Line: 99
End Line: 103

File Changed: patchwork/common/client/llm/protocol.py

Rule 1: Do not ignore potential bugs in the code

Details: Critical bug potential detected. The change modifies an abstract method get_models() to test(), which could break the interface contract and cause runtime errors in derived classes. This change appears to remove important functionality (model listing) without proper replacement.

Affected Code Snippet:

@abstractmethod
-    def get_models(self) -> set[str]:
+    def test(self) -> None:

Start Line: 35
End Line: 36

Rule 3: Do not deviate from original coding standards

Details: The change appears to deviate from established coding standards. The original method had a clear, descriptive name (get_models) and return type (set[str]), while the new method has a vague name (test) and removes the return type information, making the code less maintainable and violating typical Python typing conventions.

Affected Code Snippet:

@abstractmethod
-    def get_models(self) -> set[str]:
+    def test(self) -> None:

Start Line: 35
End Line: 36

File Changed: patchwork/common/server.py

Rule 1: Do not ignore potential bugs in the code

Details: The code diff shows a change in the import path from google to google_. This modification could potentially introduce bugs if the new module path doesn't exist or if there are dependencies elsewhere in the codebase still referencing the old path. The underscore suffix naming pattern matches the existing openai_ module, suggesting this might be an intentional standardization, but proper verification of the new module's existence is crucial.

Affected Code Snippet:

-from patchwork.common.client.llm.google import GoogleLlmClient
+from patchwork.common.client.llm.google_ import GoogleLlmClient

Start Line: 8
End Line: 8

File Changed: patchwork/steps/AgenticLLM/typed.py

Rule 1: Do not ignore potential bugs in the code

Details: The introduction of client_is_gcp parameter as a string type could lead to potential bugs since boolean values would be more appropriate for flag/state indicators. The parameter name suggests it's a boolean flag but it's typed as a string.

Affected Code Snippet:

client_is_gcp: Annotated[
    str, StepTypeConfig(is_config=True, or_op=["patched_api_key", "openai_api_key", "anthropic_api_key", "google_api_key"])
]

Start Line: 36
End Line: 38

---

Rule 2: Do not overlook possible security vulnerabilities introduced by code modifications

Details: The addition of client_is_gcp to the or_op lists in all API key configurations could potentially create a security loophole where authentication might succeed without proper API key validation if the GCP client flag is improperly set. This introduces a new authentication bypass vector that needs to be carefully validated.

Affected Code Snippet:

openai_api_key: Annotated[
    str, StepTypeConfig(is_config=True, or_op=["patched_api_key", "google_api_key", "client_is_gcp", "anthropic_api_key"])
]
anthropic_api_key: Annotated[
    str, StepTypeConfig(is_config=True, or_op=["patched_api_key", "google_api_key", "client_is_gcp", "openai_api_key"])
]
patched_api_key: Annotated[
    str,
    StepTypeConfig(
        is_config=True,
        or_op=["openai_api_key", "google_api_key", "client_is_gcp", "anthropic_api_key"],

Start Line: 13
End Line: 23

File Changed: patchwork/steps/CallLLM/typed.py

Rule 1: Do not ignore potential bugs in the code

Details: The modification introduces a potential bug through circular dependencies in the or_op configurations. The new client_is_gcp parameter is added to all API key configurations, creating a situation where each parameter references all others including itself (through transitive dependencies), which could lead to infinite validation loops or unexpected behavior.

Affected Code Snippet:

client_is_gcp: Annotated[
    str, StepTypeConfig(is_config=True, or_op=["patched_api_key", "openai_api_key", "anthropic_api_key", "google_api_key"])
]

Start Line: 38
End Line: 40

---

Rule 2: Do not overlook possible security vulnerabilities

Details: The introduction of client_is_gcp parameter as a string type instead of a boolean could lead to security vulnerabilities. String values can contain arbitrary data and require explicit validation, whereas a boolean would provide strict true/false values. This could potentially be exploited if the validation logic doesn't properly sanitize the input.

Affected Code Snippet:

client_is_gcp: Annotated[
    str, StepTypeConfig(is_config=True, or_op=["patched_api_key", "openai_api_key", "anthropic_api_key", "google_api_key"])
]

Start Line: 38
End Line: 40

File Changed: patchwork/steps/FileAgent/typed.py

Rule 1: Do not ignore potential bugs in the code
Details: The code modification introduces a potential bug by removing the or_op configuration for API keys. The original code allowed fallback options between different API keys (openai, google, anthropic), but the new code removes this flexibility, which could cause runtime failures if the system expects alternative API keys to be available.

Affected Code Snippet:

    anthropic_api_key: Annotated[
        str, StepTypeConfig(is_config=True, or_op=["patched_api_key", "google_api_key", "openai_api_key"])
    ]

Start Line: 14
End Line: 14

---

Rule 2: Do not overlook possible security vulnerabilities
Details: The modification may introduce a security concern by removing the API key validation logic embedded in the or_op configuration. The original code enforced a structured approach to API key validation through TypedDict and Annotated types, ensuring that at least one valid API key was present. The simplified version might allow the system to proceed with insufficient API key validation.

Affected Code Snippet:

    openai_api_key: Annotated[
        str, StepTypeConfig(is_config=True, or_op=["patched_api_key", "google_api_key", "anthropic_api_key"])
    ]
    anthropic_api_key: Annotated[
        str, StepTypeConfig(is_config=True, or_op=["patched_api_key", "google_api_key", "openai_api_key"])
    ]
    google_api_key: Annotated[
        str, StepTypeConfig(is_config=True, or_op=["patched_api_key", "openai_api_key", "anthropic_api_key"])
    ]

Start Line: 11
End Line: 15

File Changed: patchwork/steps/FixIssue/typed.py

Rule 1: Do not ignore potential bugs in the code

Details: Potential circular dependency in API key configuration. The new client_is_gcp parameter is added to the or_op lists of all API keys, but it's also defined with its own or_op list containing all other API keys, which could lead to unexpected behavior in the API key validation logic.

Affected Code Snippet:

client_is_gcp: Annotated[
    str, StepTypeConfig(is_config=True, or_op=["patched_api_key", "openai_api_key", "anthropic_api_key", "google_api_key"])
]

Start Line: 36
End Line: 38

---

Rule 2: Do not overlook possible security vulnerabilities

Details: Using string type for client_is_gcp parameter instead of boolean could lead to security vulnerabilities. String values are more permissive and could allow injection of unexpected values, whereas a boolean would strictly limit the possible values to True/False.

Affected Code Snippet:

client_is_gcp: Annotated[
    str, StepTypeConfig(is_config=True, or_op=["patched_api_key", "openai_api_key", "anthropic_api_key", "google_api_key"])
]

Start Line: 36
End Line: 38

File Changed: patchwork/steps/GitHubAgent/typed.py

Rule 1: Do not ignore potential bugs in the code
Details: No direct bugs identified, but there's a potential for runtime inconsistency in type checking. The client_is_gcp parameter is defined as a string type but appears to represent a boolean flag based on its name. This could lead to type-related bugs if the code assumes boolean values.

Affected Code Snippet:

client_is_gcp: Annotated[
    str, StepTypeConfig(is_config=True, or_op=["patched_api_key", "openai_api_key", "anthropic_api_key", "google_api_key"])
]

Start Line: 15
End Line: 16

---

Rule 2: Do not overlook possible security vulnerabilities
Details: The modification introduces a potential security concern by adding client_is_gcp as an alternative configuration option in the or_op list for API keys. This could potentially allow bypassing API key validation if the GCP client status is not properly validated, leading to unauthorized access.

Affected Code Snippet:

openai_api_key: Annotated[
    str, StepTypeConfig(is_config=True, or_op=["patched_api_key", "google_api_key", "client_is_gcp", "anthropic_api_key"])
]
anthropic_api_key: Annotated[
    str, StepTypeConfig(is_config=True, or_op=["patched_api_key", "google_api_key", "client_is_gcp", "openai_api_key"])
]
google_api_key: Annotated[
    str, StepTypeConfig(is_config=True, or_op=["patched_api_key", "openai_api_key", "client_is_gcp", "anthropic_api_key"])
]

Start Line: 13
End Line: 17

File Changed: patchwork/steps/LLM/typed.py

Rule 1: Do not ignore potential bugs in the code

Details: The addition of client_is_gcp parameter introduces a potential bug in the or_op circular dependency. The new parameter is added to the or_op list of all other API key parameters, and those parameters are added to its own or_op list, creating a circular dependency that could lead to validation issues.

Affected Code Snippet:

client_is_gcp: Annotated[
    str, StepTypeConfig(is_config=True, or_op=["patched_api_key", "openai_api_key", "anthropic_api_key", "google_api_key"])
]

Start Line: 46
End Line: 48

---

Rule 2: Do not overlook possible security vulnerabilities introduced by code modifications

Details: The addition of client_is_gcp parameter as a string type could be a security concern. Since this appears to be a boolean flag indicating GCP client status, using a string type instead of a boolean could allow injection of malicious string values. The type should be boolean instead of string for better security and type safety.

Affected Code Snippet:

client_is_gcp: Annotated[
    str, StepTypeConfig(is_config=True, or_op=["patched_api_key", "openai_api_key", "anthropic_api_key", "google_api_key"])
]

Start Line: 46
End Line: 48

File Changed: patchwork/steps/SimplifiedLLM/typed.py

Rule 1: Do not ignore potential bugs in the code

Details: There is a potential bug in the circular dependency configuration of the or_op parameters. The addition of client_is_gcp to the or_op lists creates a circular reference where each key refers to all others including itself (through transitive dependencies), which could lead to unexpected behavior in the validation logic.

Affected Code Snippet:

openai_api_key: Annotated[
    str, StepTypeConfig(is_config=True, or_op=["patched_api_key", "google_api_key", "client_is_gcp", "anthropic_api_key"])
]
anthropic_api_key: Annotated[
    str, StepTypeConfig(is_config=True, or_op=["patched_api_key", "google_api_key", "client_is_gcp", "openai_api_key"])
]

Start Line: 3
End Line: 8

---

Rule 2: Do not overlook possible security vulnerabilities

Details: The addition of client_is_gcp as a string type instead of a boolean type could potentially lead to security issues. If this field is meant to be a boolean flag for GCP authentication, using a string type allows for arbitrary string values which could be exploited for authentication bypass.

Affected Code Snippet:

client_is_gcp: Annotated[
    str, StepTypeConfig(is_config=True, or_op=["patched_api_key", "openai_api_key", "anthropic_api_key", "google_api_key"])
]

Start Line: 41
End Line: 43

File Changed: patchwork/steps/SimplifiedLLMOnce/typed.py

Rule 1: Do not ignore potential bugs in the code

Details: Potential bug detected in the circular dependency configuration of API keys and GCP client flag. The current setup could lead to a deadlock situation where none of the authentication methods are available despite having valid credentials.

Affected Code Snippet:

client_is_gcp: Annotated[
    str, StepTypeConfig(is_config=True, or_op=["patched_api_key", "openai_api_key", "anthropic_api_key", "google_api_key"])
]

Start Line: 41
End Line: 43

---

Rule 2: Do not overlook possible security vulnerabilities

Details: Security concern identified in the type annotation of client_is_gcp. It's defined as a string type when it should likely be a boolean, which could lead to unexpected behavior and potential security bypass if string values are not properly validated.

Affected Code Snippet:

client_is_gcp: Annotated[
    str, StepTypeConfig(is_config=True, or_op=["patched_api_key", "openai_api_key", "anthropic_api_key", "google_api_key"])
]

Start Line: 41
End Line: 43

File Changed: pyproject.toml

Rule 1: Do not ignore potential bugs in the code

Details: Potential bug risk identified due to the removal of conditional dependency versioning for numpy, pandas, and scipy. The change from Python version-specific dependencies to fixed versions might cause compatibility issues across different Python versions.

Affected Code Snippet:

-numpy = [
-    { version = "^1.26", python = "^3.12", optional = true },
-    { version = "^1.24", python = "^3.9, <=3.11", optional = true }
-]
+numpy = "1.26.4"

Start Line: 56
End Line: 61

---

Rule 2: Do not overlook possible security vulnerabilities

Details: The dependency updates for multiple AI-related packages (pydantic-ai, google-cloud-aiplatform, google-genai) and the addition of boto3 should be reviewed for security implications. Additionally, while semgrep is updated to a newer version (1.52.0), removing the caret (^) prefix makes it a fixed version which might miss security updates.

Affected Code Snippet:

-pydantic-ai = "^0.0.32"
+pydantic-ai = "^0.0.37"
+google-cloud-aiplatform = "^1.84.0"
+google-genai = "^1.2.0"

Start Line: 36
End Line: 39