Skip to content

fix(tools): resolve JSON Pointer $ref in _dereference_schema - #6148

Closed
foivos-all wants to merge 3 commits into
google:mainfrom
foivos-all:fix/json-pointer-refs
Closed

foivos-all wants to merge 3 commits into
google:mainfrom
foivos-all:fix/json-pointer-refs

Conversation

@foivos-all

Copy link
Copy Markdown
Contributor

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):

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

Problem:
_dereference_schema currently resolves $ref by looking up the last path segment in $defs/definitions. However, JSON Schema $ref can also use JSON Pointer syntax pointing to arbitrary paths within the document (e.g., "$ref": "#/properties/address" refers to a sibling property). These references are silently left unresolved (returned as-is), which can cause a downstream KeyError or schema conversion failures when the unreferenced $ref keyword leaks into the Gemini schema validator.

Solution:
Added a _resolve_json_pointer helper inside _dereference_schema that navigates the schema tree following the JSON Pointer path. When the existing $defs/definitions lookup fails, the code now attempts JSON Pointer resolution before falling back to returning the $ref as-is. The helper handles:

  • Path traversal through nested dicts (e.g., #/properties/foo/properties/bar)
  • Array index access (e.g., #/properties/items/0)
  • JSON Pointer escape sequences (~1 → /, ~0 → ~)
  • Circular reference detection (reuses the existing path_refs tracking)

Testing Plan

Unit Tests:

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

  • All unit tests pass locally.

  • test_json_pointer_ref: resolves a sibling JSON Pointer #/properties/b

  • test_circular_json_pointer_ref: mutual circular JSON Pointer refs

  • test_json_pointer_to_array_element: JSON Pointer to an array element via _dereference_schema

All 72 tests in test_gemini_schema_util.py pass.

Manual End-to-End (E2E) Tests:

A JSON Pointer $ref like #/properties/b refers to a sibling property within the same schema. On main, this is left unresolved. Run the following:

  • BEFORE (main) - $ref is NOT resolved:

    from google.adk.tools._gemini_schema_util import _dereference_schema
    
    schema = {
        "type": "object",
        "properties": {
            "a": {"$ref": "#/properties/b"},
            "b": {"type": "string"},
        },
    }
    result = _dereference_schema(schema)
    print(result)
    # → {"a": {"$ref": "#/properties/b"}, "b": {"type": "string"}}
    #   ^^^^^^^^ $ref leaked through, unresolved
    
  • AFTER (fix/json-pointer-refs) - $ref is resolved

    from google.adk.tools._gemini_schema_util import _dereference_schema
    
    schema = {
        "type": "object",
        "properties": {
            "a": {"$ref": "#/properties/b"},
            "b": {"type": "string"},
        },
    }
    result = _dereference_schema(schema)
    #   After the fix, JSON Pointer references are traversed and resolved
    print(result)
    # → {"a": {"type": "string"}, "b": {"type": "string"}}
    #   ^^^^^^^^^^^^^^^^^^^^^^^^^^ $ref resolved correctly
    

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.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

This comment on #3880 (comment) is from a reviewer:
image

@rohityan rohityan self-assigned this Jun 17, 2026
@rohityan rohityan added tools [Component] This issue is related to tools request clarification [Status] The maintainer need clarification or more information from the author labels Jun 17, 2026
@rohityan

Copy link
Copy Markdown
Collaborator

Hi @foivos-all , Thank you for your contribution! We appreciate you taking the time to submit this pull request. Can you please fix the failing unit test and fix formatting errors before we can proceed with a review.

@foivos-all
foivos-all force-pushed the fix/json-pointer-refs branch from 83039fc to 1351700 Compare June 17, 2026 19:11
@foivos-all

Copy link
Copy Markdown
Contributor Author

Hi @rohityan, I fixed the failing pre-commit checks. Let me know if you can proceed with a review

@wyf7107 wyf7107 self-assigned this Jun 17, 2026
@GWeale GWeale added the import label Jun 17, 2026
@GWeale GWeale assigned GWeale and unassigned rohityan Jun 18, 2026
@GWeale GWeale removed the import label Jun 18, 2026
@foivos-all foivos-all changed the title fix(tools): fix(tools): resolve JSON Pointer $ref in _dereference_schema fix(tools): resolve JSON Pointer $ref in _dereference_schema Jun 18, 2026
@GWeale GWeale added import and removed import labels Jun 18, 2026
@foivos-all

Copy link
Copy Markdown
Contributor Author

@rohityan I believe the unit test that is failing is broken. It's failing because in tests/unittests/cli/utils/test_cli_deploy_ignore.py:136, cli_deploy.to_agent_engine() is called without project or region, so it falls through to the interactive login/onboarding flow at cli_deploy.py:1128 which fails in
non-interactive environments. It's not related to my changes in any way

copybara-service Bot pushed a commit that referenced this pull request Oct 15, 2026
@adk-bot

adk-bot commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator

Thank you @foivos-all for your contribution! 🎉

Your changes have been successfully imported and merged via Copybara in commit 59fe93b.

Closing this PR as the changes are now in the main branch.

@adk-bot adk-bot added the merged [Status] This PR is merged label Oct 15, 2026
@adk-bot adk-bot closed this Oct 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged [Status] This PR is merged request clarification [Status] The maintainer need clarification or more information from the author tools [Component] This issue is related to tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants