Skip to content

Conversation

aaronsteers
Copy link
Contributor

@aaronsteers aaronsteers commented Sep 19, 2025

Workaround for issue logged here:

Note:

With this change, the input args are no longer optional from the perspective of Python itself. The caller would need to send some input for optional args if the functions were to be called from Python. However, since we aren't calling these directly, and they are only called via MCP, this should be fine for our purposes.

Summary by CodeRabbit

  • Documentation

    • Clarified and standardized parameter defaults across cloud operations, local operations, and connector listing. Generated docs now show explicit defaults and improved descriptions.
  • Refactor

    • Consolidated defaults into metadata for consistent behavior: optional configs/filters default to None; wait defaults to true with a 300s timeout; max_records defaults to 1000; preview limit defaults to 10; streams default to “suggested”; execution mode defaults to “auto”. No changes to function signatures or core behavior.

Important

Auto-merge enabled.

This PR is set to merge automatically when all requirements are met.

Copy link

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

Testing This PyAirbyte Version

You can test this version of PyAirbyte using the following:

# Run PyAirbyte CLI from this branch:
uvx --from 'git+https://github.com/airbytehq/PyAirbyte.git@aj/fix/resolve-nested-anyof-in-mcp-tool-args' pyairbyte --help

# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@aj/fix/resolve-nested-anyof-in-mcp-tool-args'

Helpful Resources

PR Slash Commands

Airbyte Maintainers can execute the following slash commands on your PR:

  • /fix-pr - Fixes most formatting and linting issues
  • /poetry-lock - Updates poetry.lock file
  • /test-pr - Runs tests with the updated PyAirbyte

Community Support

Questions? Join the #pyairbyte channel in our Slack workspace.

📝 Edit this welcome message.

@aaronsteers aaronsteers changed the title Aj/fix/resolve-nested-anyof-in-mcp-tool-args fix(mcp): resolve nested anyOf in MCP tool args Sep 19, 2025
Copy link

github-actions bot commented Sep 19, 2025

PyTest Results (Fast Tests Only, No Creds)

301 tests  ±0   300 ✅ ±0   4m 31s ⏱️ +5s
  1 suites ±0     1 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit eae6fad. ± Comparison against base commit 8ef7237.

♻️ This comment has been updated with latest results.

@aaronsteers aaronsteers requested a review from Copilot September 19, 2025 19:11
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes an issue with nested anyOf handling in MCP tool arguments by restructuring function parameter definitions. The change refactors optional function parameters to use explicit Field objects with default values instead of Python default values, addressing a compatibility issue with the fastmcp library.

  • Restructures function parameter definitions to use Field(default=...) instead of Python default parameters
  • Maintains the same functional behavior while fixing MCP tool argument handling
  • Affects multiple MCP operation functions across connector registry, local operations, and cloud operations

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.

File Description
airbyte/mcp/_local_ops.py Refactors optional parameters in local connector operations functions
airbyte/mcp/_connector_registry.py Updates parameter definitions in connector listing function
airbyte/mcp/_cloud_ops.py Modifies cloud operations functions to use Field defaults

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

coderabbitai bot commented Sep 19, 2025

📝 Walkthrough

Walkthrough

Adjusted Field(...) metadata defaults for parameters across MCP modules, moving default values into Field annotations without changing runtime logic. Updates span cloud operations, connector registry filtering parameters, and local ops functions, including explicit defaults for optional values and toggles.

Changes

Cohort / File(s) Summary of changes
Cloud ops Field defaults
airbyte/mcp/_cloud_ops.py
Added explicit defaults inside Field(...) for: deploy_source_to_cloud (config/config_secret_name=None, unique=True), deploy_destination_to_cloud (config/config_secret_name=None, unique=True), create_connection_on_cloud (table_prefix=None), run_cloud_sync (wait=True, wait_timeout=300), get_cloud_sync_status (job_id=None).
Connector registry parameter metadata
airbyte/mcp/_connector_registry.py
list_connectors: keyword_filter and connector_type_filter now Field(..., default=None); install_types description reformatted into Field(description=(...), default=None) and = None removed from the parameter signature while preserving behavior.
Local ops Field defaults
airbyte/mcp/_local_ops.py
Added explicit defaults inside Field(...): validate_connector_config (config/config_file/config_secret_name=None, override_execution_mode="auto"); get_source_stream_json_schema (same defaults); read_source_stream_records (config/config_file/config_secret_name=None, max_records=1000, override_execution_mode="auto"); get_stream_previews (config/config_file/config_secret_name=None, streams=None, limit=10, override_execution_mode="auto"); sync_source_to_cache (config/config_file/config_secret_name=None, streams="suggested", override_execution_mode="auto"); run_sql_query (max_records=1000).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • fix: misc MCP fixes (from Davin feedback) #791 — Overlaps on MCP modules and similar parameter/Field metadata adjustments; likely closely related. Would you like me to cross-check for any conflicting docstring or signature formatting across these PRs, wdyt?

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title is concise and accurately describes the primary change — resolving nested anyOf in MCP tool arguments — and aligns with the PR objectives and the Field-metadata edits in the diff, so it communicates the main fix clearly to reviewers.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch aj/fix/resolve-nested-anyof-in-mcp-tool-args

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c66205c and eae6fad.

⛔ Files ignored due to path filters (1)
  • poetry.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • airbyte/mcp/_local_ops.py (6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • airbyte/mcp/_local_ops.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Pytest (All, Python 3.10, Windows)
  • GitHub Check: Pytest (No Creds)
  • GitHub Check: Pytest (All, Python 3.11, Windows)
  • GitHub Check: Pytest (All, Python 3.11, Ubuntu)
  • GitHub Check: Pytest (Fast)
  • GitHub Check: Pytest (All, Python 3.10, Ubuntu)

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 12

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8ef7237 and c66205c.

⛔ Files ignored due to path filters (1)
  • poetry.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • airbyte/mcp/_cloud_ops.py (5 hunks)
  • airbyte/mcp/_connector_registry.py (1 hunks)
  • airbyte/mcp/_local_ops.py (6 hunks)
🧰 Additional context used
🪛 GitHub Actions: Run Linters
airbyte/mcp/_local_ops.py

[error] 390-390: Ruff: Line too long (E501) in airbyte/mcp/_local_ops.py:390.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Pytest (All, Python 3.11, Ubuntu)
  • GitHub Check: Pytest (All, Python 3.10, Windows)
  • GitHub Check: Pytest (All, Python 3.11, Windows)
  • GitHub Check: Pytest (All, Python 3.10, Ubuntu)

@aaronsteers
Copy link
Contributor Author

Moving these into a comment so I can resolve the conversations...

image

@aaronsteers aaronsteers enabled auto-merge (squash) September 19, 2025 19:41
Copy link

PyTest Results (Full)

364 tests  ±0   348 ✅ ±0   23m 0s ⏱️ +44s
  1 suites ±0    16 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit eae6fad. ± Comparison against base commit 8ef7237.

@aaronsteers aaronsteers merged commit 503b124 into main Sep 19, 2025
23 checks passed
@aaronsteers aaronsteers deleted the aj/fix/resolve-nested-anyof-in-mcp-tool-args branch September 19, 2025 19:46
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