fix(docs): update stale env var and script refs from PR #67 feedback#68
fix(docs): update stale env var and script refs from PR #67 feedback#68jeremylongshore merged 1 commit intomainfrom
Conversation
- .env.example: BOB_AGENT_ENGINE_NAME_DEV → AGENT_ENGINE_BOB_ID_DEV - 000-DR-STND-inline-source-deployment: update all references from BOB_AGENT_ENGINE_NAME_DEV → AGENT_ENGINE_BOB_ID_DEV and smoke_test_bob_agent_engine_dev.py → run_agent_engine_dev_smoke.py - Makefile: add explicit --agent bob to smoke target (per Gemini review) Historical AARs (130, 131, 159) left unchanged as they document the state at time of writing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Review Summary by QodoUpdate stale environment variable and script references
WalkthroughsDescription• Updated stale environment variable references from BOB_AGENT_ENGINE_NAME_DEV to AGENT_ENGINE_BOB_ID_DEV • Renamed smoke test script reference from smoke_test_bob_agent_engine_dev.py to run_agent_engine_dev_smoke.py • Added explicit --agent bob flag to Makefile smoke test target • Simplified environment variable format from full resource path to agent ID only Diagramflowchart LR
A["Legacy env var<br/>BOB_AGENT_ENGINE_NAME_DEV"] -->|"Update to"| B["New env var<br/>AGENT_ENGINE_BOB_ID_DEV"]
C["Old script name<br/>smoke_test_bob_agent_engine_dev.py"] -->|"Rename to"| D["New script name<br/>run_agent_engine_dev_smoke.py"]
E["Makefile target"] -->|"Add flag"| F["--agent bob parameter"]
File Changes1. .env.example
|
Summary of ChangesHello @jeremylongshore, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the project's configuration and documentation by addressing outdated references and enhancing a critical development workflow. It ensures consistency in environment variable naming, updates script paths to reflect current practices, and fortifies a Makefile command for agent-specific smoke testing, thereby improving the developer experience and reducing potential confusion. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughEnvironment variable Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
| Filename | Overview |
|---|---|
| .env.example | Updated deprecated BOB_AGENT_ENGINE_NAME_DEV to canonical AGENT_ENGINE_BOB_ID_DEV pattern |
| 000-docs/000-DR-STND-inline-source-deployment-for-vertex-agent-engine.md | Updated 10 references: deprecated env var and archived script path to current conventions |
| Makefile | Added explicit --agent bob flag for resilience against default changes |
Last reviewed commit: 49d57ef
Code Review by Qodo
1. Env example contradicts ID format
|
| # Bob (Global Orchestrator) - Dev instance | ||
| # Required for: make smoke-bob-agent-engine-dev | ||
| # BOB_AGENT_ENGINE_NAME_DEV=projects/205354194989/locations/us-central1/reasoningEngines/1234567890123456789 | ||
| # AGENT_ENGINE_BOB_ID_DEV=1234567890123456789 |
There was a problem hiding this comment.
1. Env example contradicts id format 🐞 Bug ✓ Correctness
.env.example now references AGENT_ENGINE_BOB_ID_DEV, but the surrounding comments still instruct users to paste a full Agent Engine resource name (projects/.../reasoningEngines/...). The runtime config treats this value as an ID and will blindly concatenate it into a full path/URL, producing an invalid resource name if a full path is supplied.
Agent Prompt
### Issue description
`.env.example` now points users to `AGENT_ENGINE_BOB_ID_DEV`, but the surrounding comments still describe pasting a *full* resource name (`projects/.../reasoningEngines/...`). The code treats the env var as an ID and concatenates it into a full resource path, so using a full resource name will create an invalid/doubled resource name.
### Issue Context
- `agents/config/agent_engine.py` documents `reasoning_engine_id` as **ID-only** and constructs the full name via string formatting.
- `.env.example` still claims the values are “full resource identifiers”.
### Fix Focus Areas
- .env.example[28-44]
- agents/config/agent_engine.py[37-64]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| **Location**: `scripts/run_agent_engine_dev_smoke.py` | ||
|
|
||
| **Purpose**: Validates that the inline-deployed Bob agent on Vertex AI Agent Engine is responding correctly to basic health check queries. | ||
|
|
||
| **Requirements**: | ||
| - Run AFTER a successful dev deployment via inline source | ||
| - Requires `BOB_AGENT_ENGINE_NAME_DEV` environment variable set to full Agent Engine resource name | ||
| - Requires `AGENT_ENGINE_BOB_ID_DEV` environment variable set to the Agent Engine ID | ||
| - Uses Vertex AI Agent Engine SDK for connectivity |
There was a problem hiding this comment.
2. Smoke-test standard doc is stale 🐞 Bug ✓ Correctness
The standards doc was updated to reference scripts/run_agent_engine_dev_smoke.py, but the surrounding content still describes the archived smoke script behavior (Vertex SDK usage, [SMOKE] expected output format, different exit codes) and lists env vars (GCP_LOCATION) that the current config code does not read. This can lead to incorrect setup and mis-troubleshooting.
Agent Prompt
### Issue description
The standards doc now points to `scripts/run_agent_engine_dev_smoke.py`, but the rest of the section still describes the old archived smoke script (different output, different flow, different exit codes) and recommends env vars (`GCP_LOCATION`) that the current config module doesn’t read.
### Issue Context
- `scripts/run_agent_engine_dev_smoke.py` is an AE3 smoke test that routes via `service/a2a_gateway/agent_engine_client.call_agent_engine` and requires `DEPLOYMENT_ENV=dev`.
- `agents/config/agent_engine.py` reads `LOCATION`/`VERTEX_LOCATION` (not `GCP_LOCATION`).
### Fix Focus Areas
- 000-docs/000-DR-STND-inline-source-deployment-for-vertex-agent-engine.md[508-561]
- scripts/run_agent_engine_dev_smoke.py[16-31]
- scripts/run_agent_engine_dev_smoke.py[113-120]
- agents/config/agent_engine.py[104-113]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
000-docs/000-DR-STND-inline-source-deployment-for-vertex-agent-engine.md (1)
579-585:⚠️ Potential issue | 🟡 MinorClarify “resource name” vs “ID” to avoid misconfiguration.
Step 6 says to copy the full resource name, while Step 7 uses the
*_ID_*variable. Please clarify that the ID is the final segment of the resource name.✅ Proposed wording fix
-6. Copy Agent Engine resource name from workflow output -7. Set `AGENT_ENGINE_BOB_ID_DEV` environment variable +6. Copy Agent Engine ID from workflow output (last segment of the resource name) +7. Set `AGENT_ENGINE_BOB_ID_DEV` to that ID🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@000-docs/000-DR-STND-inline-source-deployment-for-vertex-agent-engine.md` around lines 579 - 585, Update the docs to clarify that Step 6 requires copying the full Agent Engine resource name but Step 7's AGENT_ENGINE_BOB_ID_DEV expects only the resource ID (the final segment of the full resource name); explicitly show an example full resource name and highlight extracting the last path segment as the ID (e.g., show full: projects/.../agentEngines/AGENT_ENGINE_ID and then AGENT_ENGINE_ID used for AGENT_ENGINE_BOB_ID_DEV) and update the Step 6/7 wording to state this extraction clearly..env.example (1)
31-43:⚠️ Potential issue | 🟡 MinorClarify that
AGENT_ENGINE_BOB_ID_DEVexpects the ID, not the full resource name.The comments still describe full resource identifiers, which conflicts with the new “ID” variable naming and example. This can cause users to paste the wrong value.
✅ Proposed doc fix
-# These are the full Agent Engine resource identifiers from Vertex AI -# Format: projects/PROJECT_ID/locations/LOCATION/reasoningEngines/AGENT_ID +# These are the numeric Agent Engine IDs (final segment of the resource name) +# Resource name format: projects/PROJECT_ID/locations/LOCATION/reasoningEngines/AGENT_ID +# Use only the trailing AGENT_ID value here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.example around lines 31 - 43, Update the .env example comments to make it explicit that AGENT_ENGINE_BOB_ID_DEV expects just the agent ID (e.g., the numeric AGENT_ID) and not the full Vertex AI resource name; change or add text near the AGENT_ENGINE_BOB_ID_DEV example to say "Enter only the AGENT_ID (numeric ID), not the full resource name like projects/.../reasoningEngines/..." and remove or separate the earlier "full resource identifier" instructions so they don't conflict with the AGENT_ENGINE_BOB_ID_DEV variable naming.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.env.example:
- Around line 31-43: Update the .env example comments to make it explicit that
AGENT_ENGINE_BOB_ID_DEV expects just the agent ID (e.g., the numeric AGENT_ID)
and not the full Vertex AI resource name; change or add text near the
AGENT_ENGINE_BOB_ID_DEV example to say "Enter only the AGENT_ID (numeric ID),
not the full resource name like projects/.../reasoningEngines/..." and remove or
separate the earlier "full resource identifier" instructions so they don't
conflict with the AGENT_ENGINE_BOB_ID_DEV variable naming.
In `@000-docs/000-DR-STND-inline-source-deployment-for-vertex-agent-engine.md`:
- Around line 579-585: Update the docs to clarify that Step 6 requires copying
the full Agent Engine resource name but Step 7's AGENT_ENGINE_BOB_ID_DEV expects
only the resource ID (the final segment of the full resource name); explicitly
show an example full resource name and highlight extracting the last path
segment as the ID (e.g., show full: projects/.../agentEngines/AGENT_ENGINE_ID
and then AGENT_ENGINE_ID used for AGENT_ENGINE_BOB_ID_DEV) and update the Step
6/7 wording to state this extraction clearly.
There was a problem hiding this comment.
Code Review
The pull request successfully addresses stale references to environment variables and script names in the documentation and .env.example, following feedback from PR #67. The transition from full resource names to simple IDs for Agent Engine configuration is correctly reflected across the modified files. I have one minor suggestion regarding the accuracy of an error message in the troubleshooting documentation to ensure it matches the actual script output.
| 1. **"Missing required environment variable: BOB_AGENT_ENGINE_NAME_DEV"** | ||
| - Cause: Environment variable not set | ||
| - Fix: Export variable with full Agent Engine resource name | ||
| 1. **"Agent not configured"** |
There was a problem hiding this comment.
The error message string here doesn't exactly match the output from scripts/run_agent_engine_dev_smoke.py. For better searchability in the troubleshooting guide, it's best to use the actual string printed by the script.
| 1. **"Agent not configured"** | |
| 1. **"Agent 'bob' is NOT configured for dev"** |
Summary
Addresses Qodo and Gemini feedback from PR #67:
.env.exampleand standards doc still referenced legacyBOB_AGENT_ENGINE_NAME_DEV— updated to canonicalAGENT_ENGINE_BOB_ID_DEVsmoke_test_bob_agent_engine_dev.py— updated torun_agent_engine_dev_smoke.py--agent bobto Makefile target for resilience against default changesHistorical AARs (130, 131, 159) left unchanged as they document the state at time of writing.
Test plan
grep -r BOB_AGENT_ENGINE_NAME_DEV .env.example 000-docs/000-DR-STND-inline-*returns zerogrep -r smoke_test_bob_agent_engine_dev 000-docs/000-DR-STND-inline-*returns zero🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Chores