Migrate high-volume-document-analyzer recipe to contrib/python and upgrade model references - #2623
happyhuman wants to merge 4 commits into
Conversation
…nalyzer recipe (b/507490815)
| load_dotenv(override=True) | ||
| PROJECT_ID = os.getenv("GOOGLE_CLOUD_PROJECT") | ||
| LOCATION = os.getenv("GOOGLE_CLOUD_LOCATION", "us-central1") | ||
| LOCATION = os.getenv("GOOGLE_CLOUD_LOCATION", "us-east1") |
There was a problem hiding this comment.
Avoid hardcoding fallback default values on environment variable reads. Default values belong in .env.example where they can be easily discovered.
There was a problem hiding this comment.
Removed the fallback default value from os.getenv('GOOGLE_CLOUD_LOCATION') in deploy.py so the value is solely sourced from the environment and .env.example.
| CHUNK_SIZE = int(os.getenv("BATCH_SIZE", "10")) | ||
| MODEL_NAME = os.getenv("MODEL_NAME_DOC_PROCESSING", "gemini-2.5-flash") | ||
| LOCATION = os.getenv("GOOGLE_CLOUD_LOCATION", "us-central1") | ||
| LOCATION = os.getenv("GOOGLE_CLOUD_LOCATION", "us-east1") |
There was a problem hiding this comment.
Avoid hardcoding fallback default values on environment variable reads. Default values belong in .env.example where they can be easily discovered.
There was a problem hiding this comment.
Removed the fallback default value from os.getenv('GOOGLE_CLOUD_LOCATION') in document_toolset.py so the value is solely sourced from the environment and .env.example.
| load_dotenv(override=True) | ||
| PROJECT_ID = os.getenv("GOOGLE_CLOUD_PROJECT") | ||
| LOCATION = os.getenv("GOOGLE_CLOUD_LOCATION", "us-central1") | ||
| LOCATION = os.getenv("GOOGLE_CLOUD_LOCATION", "us-east1") |
There was a problem hiding this comment.
Avoid hardcoding default values on environment variable reads. The fallback value should instead be placed in .env.example so it remains easily discoverable.
There was a problem hiding this comment.
Removed the fallback default value from os.getenv('GOOGLE_CLOUD_LOCATION') in deploy.py so the value is solely sourced from the environment and .env.example.
| CHUNK_SIZE = int(os.getenv("BATCH_SIZE", "10")) | ||
| MODEL_NAME = os.getenv("MODEL_NAME_DOC_PROCESSING", "gemini-2.5-flash") | ||
| LOCATION = os.getenv("GOOGLE_CLOUD_LOCATION", "us-central1") | ||
| LOCATION = os.getenv("GOOGLE_CLOUD_LOCATION", "us-east1") |
There was a problem hiding this comment.
Avoid hardcoding default values on environment variable reads. The fallback value should instead be placed in .env.example so it remains easily discoverable.
There was a problem hiding this comment.
Removed the fallback default value from os.getenv('GOOGLE_CLOUD_LOCATION') in document_toolset.py so the value is solely sourced from the environment and .env.example.
… reads (b/559874122)
…grade model references (b/559874122)
There was a problem hiding this comment.
Automated House Rules review — 1 finding(s).
Also, on lines this PR does not change:
contrib/python/high-volume-document-analyzer/.env.example:6— [MINOR] "your-project-id" is a stub committed as if it were a real value. Someone copying this file has no way to tell it needs replacing; use <TODO: update-this-value> (.agents/skills/extract-python-environment-variables/)
| """Verify agent.py imports and defines the expected globals.""" | ||
| # provide a dummy GCP project and patch google.auth.default() so import-time | ||
| # credential lookups don't need ADC — the setup must happen before the import. | ||
| os.environ.setdefault("GOOGLE_CLOUD_PROJECT", "test-project") |
There was a problem hiding this comment.
[MINOR] env read carries a hardcoded default (second argument is a hardcoded default); 20 occurrence(s) across 6 file(s). Defaults belong in .env.example, not in the code (contrib/python/high-volume-document-analyzer/tests/test_runnability.py:24; and 5 more)
There was a problem hiding this comment.
Standardized placeholder strings in .env.example using <TODO: update-this-value> and removed hardcoded fallback defaults on runtime environment reads.
| # Load variables from .env if present. In production the environment is | ||
| # already populated by the platform (Cloud Run, GKE, etc.), so a missing | ||
| # .env is expected and not an error. | ||
| load_dotenv() |
There was a problem hiding this comment.
[MAJOR] Can we please call load_dotenv() before importing agent? Since agent.py and the toolset modules read environment variables at the module level during import, importing them first means they won't use the values from .env.
There was a problem hiding this comment.
Moved load_dotenv() in __init__.py before importing agent (and added fallback loading for .env.example), and removed redundant load_dotenv() calls from submodules.
| name="document_analyzer_agent", | ||
| description="Agent that analyzes document collections in chunks to answer user questions.", | ||
| model=os.getenv("MODEL_NAME_AGENT", "gemini-2.5-flash"), | ||
| model=os.getenv("MODEL_NAME_AGENT", "gemini-3.5-flash"), |
There was a problem hiding this comment.
[MAJOR] Is there a reason we are hardcoding defaults on environment reads here? Perhaps we can read MODEL_NAME_AGENT and MODEL_NAME_DOC_PROCESSING directly without fallbacks to avoid hardcoded model literals in non-test Python, as the defaults belong in .env.example (there are four instances of this, see .github/workflows/python-validate-recipe.yml:324).
There was a problem hiding this comment.
Removed hardcoded fallback model defaults from os.getenv('MODEL_NAME_AGENT') and os.getenv('MODEL_NAME_DOC_PROCESSING') across agent.py, document_toolset.py, and deploy.py, delegating defaults to .env.example.
…efaults (b/559874122)
What
Migrates the
high-volume-document-analyzerrecipe from the retiredpython/agents/location tocontrib/python/high-volume-document-analyzer. Updates default generative model references togemini-3.5-flash, bootstraps environment variables in__init__.pyprior to agent imports, standardizes.env.exampleplaceholders with<TODO: update-this-value>, addsmanifest.yamlandtests/test_runnability.py, alignspyproject.toml, removes hardcoded fallback model defaults on environment reads, and updatesuv.lock.Why
Recipes located in root
<language>/agents/directories are retired under repository policy, and changes made inside those frozen paths fail CI validation. Moving tocontrib/python/high-volume-document-analyzerbrings the recipe into compliance with active contribution paths. Upgrading deprecatedgemini-2.5-flashreferences togemini-3.5-flashaligns with supported model tiers. Loading environment variables in__init__.pybefore importing submodules ensures module-level configurations resolve correctly from.envand.env.example, while removing hardcoded model fallbacks in Python code ensures all defaults remain centrally discoverable.How it works
python/agents/high-volume-document-analyzertocontrib/python/high-volume-document-analyzer.manifest.yamlspecifying standalone application metadata and active status.tests/test_runnability.pyto verify root agent import and initialization.__init__.pyusingload_dotenvfor.envand.env.examplebefore importingagent.load_dotenv()calls fromagent.pyandprocess_toolset.py.agent.py,document_toolset.py, anddeployment_utils/deploy.py..env.exampleusing<TODO: update-this-value>.GOOGLE_CLOUD_PROJECTis not configured.pyproject.tomldependencies and lockfile.Testing
Ran repo validators from repository root:
Ran formatters, linters, env var checker, and pyproject check:
Ran test suite via pytest:
Boundary: Local testing verifies unit tests, tool mocking, runnability tests, linting, manifest validation, environment variable mapping, and structure rules. Live Agent Engine deployments and external Secret Manager credentials require configured GCP resources and are skipped in local unit verification.
Notes for the reviewer
GOOGLE_CLOUD_PROJECTis not configured or set to a placeholder.pyproject.tomlreflects the original sample authors.