Skip to content

Fix: snake_case the OpenAPI tool name regardless of preservePropertyNames (adk-python parity) - #801

Open
AmaadMartin wants to merge 2 commits into
fix/openapi-tool-name-snake-case-parityfrom
fix/openapi-tool-name-ignores-preserve-property-names
Open

Fix: snake_case the OpenAPI tool name regardless of preservePropertyNames (adk-python parity)#801
AmaadMartin wants to merge 2 commits into
fix/openapi-tool-name-snake-case-parityfrom
fix/openapi-tool-name-ignores-preserve-property-names

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: OperationParser.getFunctionName() derived the tool name through the
private getParamName() helper, which short-circuits when
preservePropertyNames is set. The flag therefore leaked out of parameter
naming and into the tool name. adk-python returns
_to_snake_case(operation_id)[:60] there and never reads the flag, so the two
SDKs exposed different tool names for the same document.

Solution: getFunctionName() now calls snakeCase() directly, so the tool
name is always snake_case and then truncated to 60 characters. getParamName()
is untouched and still honours the flag for parameter and request-body property
names. Two doc comments record that split, because the missing contract is what
allowed the bug.

Behaviour change: a caller that passes preservePropertyNames: true gets a
different tool name. createUser now yields create_user, not createUser. A
literal toolFilter allowlist or an agent instruction that names such a tool
must be respelled. The default (false) path is byte-for-byte unchanged, the
HTTP request is unchanged, and the affected classes are @experimental.

Stacked PR: this targets fix/openapi-tool-name-snake-case-parity (#690),
which adds the snakeCase() helper this fix calls. snakeCase does not exist
on main. Only my two commits are under review; see
git diff fix/openapi-tool-name-snake-case-parity.

Collision check: gh pr list --repo AmaadMartin/adk-js --state open plus
gh pr diff --name-only on every adjacent OpenAPI PR (#690, #774, #748, #645,
#605, #436). Only #690 touches operation_parser.ts, and it deliberately
leaves this bug alone — it fixes which snake_case algorithm runs, not
whether the flag suppresses it. No duplicate.

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.

Four cases in operation_parser_test.ts and one in openapi_toolset_test.ts.
Each asserts the snake_case tool name and the preserved property names
together, because that pairing is what the fix is about. No existing test was
edited.

npx vitest run --project unit:core \
  core/test/tools/openapi_tool/operation_parser_test.ts \
  core/test/tools/openapi_tool/openapi_toolset_test.ts
  -> Test Files 2 passed (2) | Tests 34 passed (34)

npx vitest run --project unit:core \
  core/test/tools/openapi_tool/openapi_spec_parser_test.ts \
  core/test/tools/openapi_tool/rest_api_tool_test.ts \
  core/test/utils/case_utils_test.ts
  -> Test Files 3 passed (3) | Tests 80 passed (80)

npx vitest run --project unit:core \
  core/test/tools/openapi_tool/openapi_toolset_integration_test.ts
  -> Test Files 1 passed (1) | Tests 5 passed (5)

Proof the tests can fail. I reverted the one-expression change (put
this.getParamName(operationId) back), rebuilt, and re-ran. All five new
assertions failed; the 29 pre-existing tests passed unchanged, which is the
evidence the default path does not move.

× OperationParser > should snake_case the tool name with preservePropertyNames true
  -> expected 'listIssues' to be 'list_issues'
× OperationParser > should convert the tool name while preserving body property names
  -> expected 'createUser' to be 'create_user'
× OperationParser > should convert the tool name while preserving operation parameter names
  -> expected 'jira_list_Issues' to be 'jira_list_issues'
× OperationParser > should truncate the preserved-name tool name after conversion
  -> expected 'listIssuesForTheRepositoryWithAVeryLo…' to be 'list_issues_for_the_repository_with_a…'
× OpenAPIToolset > should snake_case the tool name while preserving property names
  -> expected [ 'createUser' ] to deeply equal [ 'create_user' ]
Test Files 2 failed (2) | Tests 5 failed | 29 passed (34)

The preservePropertyNames: false row of the first case passes either way by
construction. It is the paired control, not a signal.

Coverage. getFunctionName() is fully covered by these suites: 35 calls,
the throw path 1 and the snakeCase return path 34. The changed expression is
the only new production line.

Manual End-to-End (E2E) Tests:

I ran the same spec through both SDKs and compared the output.

adk-js, new OpenAPIToolset({specDict, preservePropertyNames: true}), printing
[tool.name, Object.keys(tool._getDeclaration().parameters.properties)]:

create_user ["firstName","lastName","emailAddress"]
jira_list_issues ["X-API-Key"]

adk-python, OpenAPIToolset(spec_dict=spec, preserve_property_names=True),
printing [tool.name, [p.py_name for p in tool._operation_parser.get_parameters()]]:

create_user ['firstName', 'lastName', 'emailAddress']
jira_list_issues ['X-API-Key']

The tool names now match exactly. To reproduce: build an OpenAPIToolset from
a spec with operationId: createUser and camelCase body properties
firstName / lastName / emailAddress, set preservePropertyNames: true,
and read the names back from getTools().

CI is absent, so I validated locally. The validation workflow triggers on
pull_request: branches: [main], and this PR targets a stacked base, so it
never runs. On the pushed commit eef0fb94:

  • npm run build — passed.
  • npm run lint — passed, no output.
  • npm run format:check — "All matched files use Prettier code style!".
  • npm run ts:check — 281 errors in 41 files, identical with and without my
    change (measured by stashing it). The three errors in
    openapi_toolset_test.ts are at lines 115, 425 and 470, all pre-existing and
    outside the block I added. Several open PRs are dedicated to clearing this
    backlog.
  • The targeted suites above.

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.

Amaad Martin added 2 commits August 8, 2026 03:45
…Names

getFunctionName() derived the tool name through getParamName(), so
preservePropertyNames leaked out of parameter naming and returned the raw
operationId. adk-python returns _to_snake_case(operation_id)[:60] there and
never consults the flag, so the two SDKs exposed different tool names for the
same document.

Call snakeCase() directly, and document that the flag covers parameter and
request-body property names only.
Cover the parser and the toolset. Each case asserts the snake_case tool name
and the preserved property names together, which is the pairing the fix is
about. One case pins that truncation runs after conversion.

Ports test_openapi_toolset_preserve_property_names_body_params from adk-python.
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