Skip to content

Fix: make -v/--verbose an honest --log_level debug shortcut on every command - #700

Open
AmaadMartin wants to merge 2 commits into
fix/cli-log-level-choices-validationfrom
fix/cli-verbose-log-level-precedence
Open

Fix: make -v/--verbose an honest --log_level debug shortcut on every command#700
AmaadMartin wants to merge 2 commits into
fix/cli-log-level-choices-validationfrom
fix/cli-verbose-log-level-precedence

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Stacked on #592 (fix/cli-log-level-choices-validation), which supplies the ?? fallback this fix needs. Review that one first; this PR targets its branch.

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

  1. Link to an existing issue (if applicable):
    N/A

  2. Or, if no issue exists, describe the change:
    Problem: The CLI advertises -v/--verbose on seven commands and honours it on none of them. On web, api_server and run the flag outranks an explicitly supplied --log_level, so adk web --log_level error --verbose runs at DEBUG. On deploy cloud_run, deploy agent_engine, deploy reasoning_engine and integration conformance the flag is parsed and dropped, so adk deploy cloud_run --verbose still bakes --log_level='info' into the image and sends --verbosity info to gcloud.

Solution: VERBOSE_OPTION now declares .implies({log_level: 'debug'}). Commander applies an implied value only while the target option sits at its default source, so --verbose fills in debug and an explicitly supplied level always wins, in either flag order. That is the rule adk-python gets from ParameterSource.DEFAULT. Every command that registers the shared option inherits it, so the deploy handlers keep reading options['log_level'] and now receive debug. integration conformance calls setLogLevel for the first time, and the help text states the shortcut and the precedence rule.

Notes for the reviewer:

Testing Plan

Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.
Unit Tests:
[x] I have added or updated unit tests for my change.
[x] All unit tests pass locally.

npx vitest run --project unit:dev dev/test/cli/cli_test.ts — 89 passed, 22 of them new. The new block covers every surface: precedence on web in both flag orders, an explicit level that equals the default, api_server, run, both deploy paths, integration conformance, and the help text of all seven commands.

Proof the tests can fail. I ran the new tests against seven mutations.

Mutation New tests that fail
.implies({log_level: 'debug'}) deleted 4, e.g. expected { logLevel: 'info' } to match { logLevel: 'debug' } for deploy cloud_run --verbose
getLogLevelFromOptions short-circuits on verbose again 6, e.g. expected "spy" to be called with [3], received [0] for web --log_level error --verbose
?? changed back to || in the level lookup 2, e.g. web --log_level debug --verbose reports INFO instead of DEBUG
The setLogLevel call removed from integration conformance 3, all with Number of calls: 0
A naive options['verbose'] ? 'debug' : ... in the deploy handlers 2, e.g. deploy agent_engine --log_level=warn --verbose reports debug
The --log_level default flipped to debug 2, e.g. deploy cloud_run with no flags reports debug
The old --verbose help text restored 7, one per command surface

Every new test dies under at least one mutation.

Coverage. cli.ts measures 97.13% lines and 77.27% branches. Every line of the new code runs. Two branch arms do not: the ?? 'info' and the pre-existing ?? LogLevel.INFO in getLogLevelFromOptions. --log_level carries a .default('info') on every command that registers it, so argv cannot reach either fallback. The other uncovered lines are the catch blocks in the action handlers, which this change does not touch. I added no new throw sites.

Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any necessary setup or configuration.

Run npm run build first. I put a stub gcloud on PATH that records its arguments and copies the generated Dockerfile, because the real deploy deletes its temp folder in a finally block.

  1. node dev/dist/esm/cli_entrypoint.js deploy cloud_run --verbose --temp_folder <dir> --project <p> --region <r> ./my-agent
    gcloud receives --verbosity debug, and the Dockerfile ends with CMD npx adk api_server /app/agents/my-agent --port=8000 --host=0.0.0.0 --log_level=debug. Before this change both read info.
  2. The same command with --log_level warn --verbose gives --verbosity warn and --log_level=warn. With neither flag it gives info.
  3. node dev/dist/esm/cli_entrypoint.js integration conformance --help prints Optional. Enable verbose (DEBUG) logging. Shortcut for --log_level debug; an explicitly passed --log_level wins.

The web, api_server and run surfaces pass the resolved level to setLogLevel, which the unit tests assert. I could not observe the level from the terminal, because ADK writes its DEBUG lines during a model-backed agent run.

CI does not run on this PR: the workflow triggers on pull_request: branches: [main] and this PR targets the stacked base. Local validation on the pushed commit: npm run build, npx eslint "dev/**/*.ts" and npx prettier "dev/**/*.ts" --check all pass. npm run ts:check reports 296 errors in core/test and tests/, identical with and without this change and none in dev/; CI does not run it.

Checklist

[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.

Amaad Martin added 2 commits August 5, 2026 22:30
The CLI advertises -v/--verbose on seven commands and honours it on none
of them. web, api_server and run let --verbose outrank an explicitly
supplied --log_level, and the two deploy actions plus integration
conformance never read the flag at all.

resolveLogLevelName() is now the single place that decides the level. It
upgrades to debug only while --log_level sits at its default, which is
the rule adk-python applies through ParameterSource.DEFAULT. The five
action handlers that resolve a level take the leaf Command that
commander already passes them, and integration conformance now calls
setLogLevel at all.

The extra handler parameter makes prettier re-wrap five action
signatures, so most of the cli.ts diff is re-indentation: `git diff -w`
shows the real change.
Commander applies an implied value only while the target option sits at
its default source, which is the same rule the hand-rolled
resolveLogLevelName() applied through getOptionValueSource(). Declaring
`.implies({log_level: 'debug'})` on the shared VERBOSE_OPTION drops the
helper, the `command: Command` parameter that every action handler had
to take, and the re-indentation that parameter forced on five handler
bodies.

The tests are unchanged and still pass: they assert the resolved level,
not how it is resolved.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant