Re: [PATCH v2] devtools: add Vertex AI to review scripts

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue,  2 Jun 2026 08:44:06 +0200
David Marchand <david.marchand@redhat.com> wrote:

> OpenAI, xAI) can now use Vertex AI with Application Default Credentials.
> 
> This requires a python dependency google-auth but it is left as
> optional.
> 
> Key features:
> - Auto-detection of authentication method based on environment
> - Manual override via --auth flag (auto, direct, vertex)
> - Automatic model name translation for Vertex format
> - Support for both global and regional Vertex endpoints
> - Proper error handling for Vertex API responses
> 
> Provider-specific implementations:
> - Anthropic: Uses /publishers/anthropic/models/{model}:rawPredict
>   with model name format claude-sonnet-4-5@20250929
> - Google: Uses /publishers/google/models/{model}:generateContent
> - OpenAI/xAI: Use /endpoints/openapi/chat/completions
>   with publisher prefix (e.g., openai/gpt-oss-120b-maas)
> 
> Authentication detection logic:
> - Vertex: Requires google-auth library and ADC configured
> - Direct: Falls back to API key from environment variables
> 
> Available models on Vertex AI:
> - Anthropic: All Claude models
> - Google: All Gemini models
> - OpenAI: gpt-oss-120b-maas, gpt-oss-20b-maas (open-weight only)
> - xAI: grok-4.20-*, grok-4.1-fast-* variants
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

I have no direct knowledge of Vertex pro/con.
AI review with better model had some insights.

On the whole the refactor is sound: moving "model" insertion into
_build_request_meta and threading an auth string instead of api_key is
consistent across _common.py, review-patch.py and review-doc.py, and the
build_*_request signature changes match their only callers. Two issues and
a couple of minor notes.

Warning: project-id resolution is split and the early error is unreachable-
proof in the wrong direction.

  get_vertex_credentials() does:

      credentials, project = google_auth_default()
      ...
      if not project:
          error("Could not detect GCP project. Set GOOGLE_CLOUD_PROJECT ...")
      return credentials.token, project

  but the caller then applies its own fallback:

      access_token, project_id = get_vertex_credentials()
      project_id = os.environ.get("GOOGLE_CLOUD_PROJECT") \
                   or os.environ.get("GCP_PROJECT") or project_id

  If ADC resolves no project, get_vertex_credentials() calls error() and
  exits before the GCP_PROJECT fallback can run. google.auth.default()
  honors GOOGLE_CLOUD_PROJECT itself, but not GCP_PROJECT, so the
  GCP_PROJECT branch here is dead in exactly the case it exists for, and
  the error message tells the user to set a variable that wouldn't have
  helped. Resolve the project in one place: drop the "if not project"
  error from get_vertex_credentials() and check after the env fallback,
  or move the env lookups into get_vertex_credentials().

Warning: several added lines exceed the 100-column limit.

      _common.py: error("Could not detect GCP project...")        137 cols
      _common.py: error("Vertex AI support requires...")          108 cols
      _common.py: project_id = os.environ.get(...) or ...         102 cols
      _common.py: vertex_base = f"https://aiplatform..."          104 cols
      _common.py: vertex_base = f"https://{location}-..."         115 cols

  pycodestyle/flake8 will flag all five; the project_id line is also
  black-splittable so "black --check" fails on it. Wrap the f-strings and
  split the message strings.

Info: detect_auth_method() catches bare Exception around
google_auth_default(). Acceptable for best-effort autodetection, but
narrowing it (or a "# noqa"/pylint disable with a reason) documents intent.

Info: the HTTPError path now does error_data[0].get('error', ...) after
unwrapping a list. If a Vertex error body is a list of non-dicts this
raises AttributeError outside the JSONDecodeError guard. Low risk, but a
type check would harden it.

Info: --auth is wired into review-patch.py and review-doc.py but
compare-patch-reviews.sh has no passthrough, so Vertex can't be selected
when comparing providers. May be intentional for this patch.