Skip to content

Refactor: rename code_execution_request_processor to code_execution_processor - #675

Open
AmaadMartin wants to merge 1 commit into
mainfrom
feat/rename-code-execution-processor
Open

Refactor: rename code_execution_request_processor to code_execution_processor#675
AmaadMartin wants to merge 1 commit into
mainfrom
feat/rename-code-execution-processor

Conversation

@AmaadMartin

Copy link
Copy Markdown
Owner

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

  1. Or, if no issue exists, describe the change:

Problem: core/src/agents/processors/code_execution_request_processor.ts declares a request processor and a response processor. It is the only file in that directory that holds both. Every other *_request_processor.ts there holds a request processor only, so the _request_ infix hides CodeExecutionResponseProcessor from a contributor who scans the directory.

Solution: I renamed the module and its mirrored test to code_execution_processor.ts, so the name describes the whole file. The rename follows adk-python, which names the equivalent module _code_execution.py for its subject instead of its direction. This is a pure rename: no symbol, behavior, or public API change. I used git mv, so git records both moves and can re-target hunks from other branches.

Scope notes:

  • CodeExecutionRequestProcessor, CodeExecutionResponseProcessor, CODE_EXECUTION_REQUEST_PROCESSOR and the responseProcessor singleton keep their names.
  • I added no export to core/src/index.ts or core/src/common.ts. A relocation is not a promotion to public API.
  • I left the deep specifier shape @google/adk/agents/processors/... in the integration test alone and only updated the filename in it.

Collision check (open PRs on this fork, --limit 1000): no open PR renames this file. Six PRs touch adjacent files and will need a rebase, which the recorded rename should absorb: #639 and #460 edit the moved module, #486, #561 and #73 edit llm_agent.ts, and #632, #486, #561 and #460 edit the sandbox executor test. I branched from main rather than stacking, because the overlap spans four independent branches and this change reads nothing they add.

Sequencing: #486 wires the response processor into the LlmAgent defaults. This PR does not depend on it and touches only the import line in llm_agent.ts, not the processor array literals. Merge the two in either order.

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:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

I added no test. The moved test file is byte-identical to its predecessor except for the import on line 19, so a reviewer can confirm the move preserved content. Test count is unchanged: 8 before, 8 after.

npx vitest run --project unit:core core/test/agents/processors/code_execution_processor_test.ts
  -> 1 file passed, 8 tests passed
npx vitest run --project integration tests/integration/agents/agent_with_sandbox_executor_test.ts
  -> 1 file passed, 1 test passed
npm run lint          -> clean
npm run format:check  -> "All matched files use Prettier code style!"
npm run build         -> success
git grep -n code_execution_request_processor   -> zero matches

Proof the checks can fail. A rename has no behavior to assert, so I mutated each import site and confirmed the toolchain catches the stale path:

  1. Reverted core/src/agents/llm_agent.ts:63 to the old specifier. npm run ts:check went from 281 errors to 282, and the new one is:
    core/src/agents/llm_agent.ts:63:48 - error TS2307: Cannot find module './processors/code_execution_request_processor.js' or its corresponding type declarations.
  2. Reverted the import in the moved test. Vitest failed to collect:
    Error: Cannot find module '../../../src/agents/processors/code_execution_request_processor.js'
  3. Restored both, re-ran, green.

npm run ts:check reports 281 errors on this branch. That count is identical on main with my change stashed, and the errors are unrelated to this PR (mostly Cannot find module '@google/adk' in test files). The one error in a file I touched, tests/integration/agents/agent_with_sandbox_executor_test.ts:9, exists on main too with the old path: that deep specifier resolves only through the vitest alias, never through core/package.json exports. Fixing it is out of scope here.

Manual End-to-End (E2E) Tests:

npm ci
npm run build
npx vitest run --project unit:core core/test/agents/processors/code_execution_processor_test.ts
npx vitest run --project integration tests/integration/agents/agent_with_sandbox_executor_test.ts
git grep -n code_execution_request_processor   # must print nothing

Checklist

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

…tion_processor

The module declares both CodeExecutionRequestProcessor and
CodeExecutionResponseProcessor, so the _request_ infix in the filename
hides the response processor from anyone scanning the directory. It is
the only file in core/src/agents/processors/ that holds both directions.

Pure rename: git mv of the source and its mirrored test, plus the three
import specifiers that point at them. No symbol, behavior, or public API
change.
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