feat: Add 3 new build strategy submodules (OpenAPI/Sonar, Kotlin Source, Kotlin Destination) #187
Conversation
…ce, Kotlin Destination) - Add OpenAPI/Sonar build strategy (openapi_sonar_v1) for building connectors from OpenAPI specifications - Add Kotlin Source build strategy (kotlin_source_v1) for Kotlin-based source connectors - Add Kotlin Destination build strategy (kotlin_destination_v1) for Kotlin-based destination connectors - Update server.py to register all 4 build strategies (including existing DeclarativeYamlV1) - Each strategy includes: checklist.yaml, guidance tools, validation tools, test tools, and prompts Note: kotlin_destination_v1/guidance.py has a syntax error that will be fixed in follow-up commit Co-Authored-By: AJ Steers <[email protected]>
Original prompt from AJ Steers |
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
📝 WalkthroughWalkthroughThis PR adds support for three new build strategies (Declarative OpenAPI v3, Kotlin Destination, and Kotlin Source) to the MCP server. The server.py is refactored to use a collection-driven approach that iterates over available strategies, registers their tools, and logs unavailable ones, replacing the prior single-strategy registration model. Changes
Sequence DiagramsequenceDiagram
participant Server as Server.py
participant StrategyMgr as Strategy Manager
participant Strategy as BuildStrategy
participant ToolRegistry as Tool Registries
rect rgb(220, 240, 250)
note over Server,ToolRegistry: Old Flow (Single Strategy)
Server->>Strategy: register_*_tools(app)
Strategy->>ToolRegistry: register tools
end
rect rgb(240, 255, 240)
note over Server,ToolRegistry: New Flow (Multi-Strategy)
Server->>StrategyMgr: iterate [DeclarativeOpenApiV3,<br/>KotlinDestination, KotlinSource]
loop for each strategy
StrategyMgr->>Strategy: is_available()
alt available
StrategyMgr->>Strategy: register_guidance_tools(app)
StrategyMgr->>Strategy: register_manifest_check_tools(app)
StrategyMgr->>Strategy: register_manifest_test_tools(app)
StrategyMgr->>Strategy: register_prompts(app)
Strategy->>ToolRegistry: register tools (domain-specific)
StrategyMgr->>Server: log: strategy registered
else unavailable
StrategyMgr->>Server: log: strategy skipped (unavailable)
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes
Possibly related PRs
Suggested reviewers
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
👋 Welcome to the Airbyte Connector Builder MCP!Thank you for your contribution! Here are some helpful tips and reminders for your convenience. Testing This Branch via MCPTo test the changes in this specific branch with an MCP client like Claude Desktop, use the following configuration: {
"mcpServers": {
"connector-builder-mcp-dev": {
"command": "uvx",
"args": ["--from", "git+https://github.com/airbytehq/connector-builder-mcp.git@devin/1763589811-add-build-strategy-submodules", "connector-builder-mcp"]
}
}
}Testing This Branch via CLIYou can test this version of the MCP Server using the following CLI snippet: # Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/connector-builder-mcp.git@devin/1763589811-add-build-strategy-submodules#egg=airbyte-connector-builder-mcp' --helpPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
AI Builder EvaluationsAI builder evaluations run automatically under the following conditions:
A set of standardized evaluations also run on a schedule (Mon/Wed/Fri at midnight UTC) and can be manually triggered via workflow dispatch. Helpful ResourcesIf you have any questions, feel free to ask in the PR comments or join our Slack community. |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
connector_builder_mcp/build_strategies/openapi_sonar_v1/build_strategy.py
Fixed
Show fixed
Hide fixed
connector_builder_mcp/build_strategies/kotlin_source_v1/build_strategy.py
Fixed
Show fixed
Hide fixed
connector_builder_mcp/build_strategies/kotlin_destination_v1/build_strategy.py
Fixed
Show fixed
Hide fixed
…conflict with Kotlin raw string literals Co-Authored-By: AJ Steers <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (8)
connector_builder_mcp/build_strategies/openapi_sonar_v1/manifest_checks.py (1)
19-26: Optional: make list fields explicit future-proof defaults
errors,warnings, andresources_foundare always set explicitly in your function, so the=[]defaults on the model aren’t harmful. If you later constructOpenApiValidationResultwithout passing these fields, consider switching toField(default_factory=list)for each to align with common Pydantic patterns around mutable defaults.connector_builder_mcp/build_strategies/kotlin_source_v1/manifest_checks.py (1)
19-26: Optional: useField(default_factory=list)for list fieldsAs with the OpenAPI result model, you may eventually want to construct
KotlinSourceValidationResultwithout explicitly passingerrors,warnings, orstreams_found. UsingField(default_factory=list)for these fields would follow common Pydantic patterns for list defaults.connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py (1)
431-435: Add return type annotation for consistency.The function is missing a return type annotation. Other similar functions in the codebase include
-> None.def register_guidance_tools( app: FastMCP, -): +) -> None: """Register guidance tools in the MCP server.""" register_mcp_tools(app, domain=ToolDomain.GUIDANCE)connector_builder_mcp/build_strategies/openapi_sonar_v1/guidance.py (1)
201-205: Add return type annotation for consistency.The function is missing a return type annotation.
def register_guidance_tools( app: FastMCP, -): +) -> None: """Register guidance tools in the MCP server.""" register_mcp_tools(app, domain=ToolDomain.GUIDANCE)connector_builder_mcp/build_strategies/kotlin_destination_v1/manifest_checks.py (1)
19-25: Avoid mutable list defaults on Pydantic model fields
errors,warnings, andstreams_founduse=[]as defaults. Even with Pydantic, it’s safer/clearer to usedefault_factoryso instances don’t share list objects.class KotlinDestinationValidationResult(BaseModel): """Result of Kotlin destination connector validation.""" is_valid: bool - errors: list[str] = [] - warnings: list[str] = [] - streams_found: list[str] = [] + errors: list[str] = Field(default_factory=list) + warnings: list[str] = Field(default_factory=list) + streams_found: list[str] = Field(default_factory=list)connector_builder_mcp/build_strategies/kotlin_source_v1/manifest_tests.py (1)
19-27: Usedefault_factoryforerrorslist on result model
errorsis defined with a mutable default=[], which is better expressed viaField(default_factory=list)to avoid shared list instances.class KotlinStreamTestResult(BaseModel): """Result of Kotlin stream testing operation.""" success: bool message: str records_read: int = 0 - errors: list[str] = [] + errors: list[str] = Field(default_factory=list) records: list[dict[str, Any]] | None = Field( default=None, description="Actual record data from the stream" )connector_builder_mcp/build_strategies/openapi_sonar_v1/manifest_tests.py (2)
52-55: Unused parameter: max_records.The
max_recordsparameter is defined but never used in the function body, even in the placeholder implementation. While this is expected given the placeholder status, consider acknowledging it in the success message or adding a TODO comment.Optional improvement:
return ResourceTestResult( success=True, - message=f"Successfully tested resource '{resource_name}' (placeholder implementation)", + message=f"Successfully tested resource '{resource_name}' (placeholder implementation - max_records={max_records} not yet implemented)", records_read=0, errors=[], records=[], )
85-91: Placeholder implementation needs future work.The function currently returns a hardcoded success result without actually testing the OpenAPI resource. This is acknowledged in the PR description as placeholder implementation.
Consider adding a TODO comment or tracking issue to implement the actual testing logic:
+ # TODO: Implement actual OpenAPI resource testing + # - Parse and validate OpenAPI spec + # - Initialize connector with config + # - Execute resource read with max_records limit + # - Collect and return actual records return ResourceTestResult( success=True, message=f"Successfully tested resource '{resource_name}' (placeholder implementation)", records_read=0, errors=[], records=[], )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
connector_builder_mcp/build_strategies/kotlin_destination_v1/__init__.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_destination_v1/build_strategy.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_destination_v1/checklist.yaml(1 hunks)connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_destination_v1/manifest_checks.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_destination_v1/manifest_tests.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_destination_v1/prompts.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_source_v1/__init__.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_source_v1/build_strategy.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_source_v1/checklist.yaml(1 hunks)connector_builder_mcp/build_strategies/kotlin_source_v1/guidance.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_source_v1/manifest_checks.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_source_v1/manifest_tests.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_source_v1/prompts.py(1 hunks)connector_builder_mcp/build_strategies/openapi_sonar_v1/__init__.py(1 hunks)connector_builder_mcp/build_strategies/openapi_sonar_v1/build_strategy.py(1 hunks)connector_builder_mcp/build_strategies/openapi_sonar_v1/checklist.yaml(1 hunks)connector_builder_mcp/build_strategies/openapi_sonar_v1/guidance.py(1 hunks)connector_builder_mcp/build_strategies/openapi_sonar_v1/manifest_checks.py(1 hunks)connector_builder_mcp/build_strategies/openapi_sonar_v1/manifest_tests.py(1 hunks)connector_builder_mcp/build_strategies/openapi_sonar_v1/prompts.py(1 hunks)connector_builder_mcp/server.py(4 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: aaronsteers
Repo: airbytehq/connector-builder-mcp PR: 116
File: .github/workflows/ci-tests.yml:110-113
Timestamp: 2025-10-10T17:09:02.151Z
Learning: The connector-builder-mcp repository (airbytehq/connector-builder-mcp) is an internal/private repository that doesn't accept external contributions, so there are no forked PRs to consider in workflow configurations.
Learnt from: aaronsteers
Repo: airbytehq/connector-builder-mcp PR: 116
File: .github/workflows/ci-tests.yml:110-113
Timestamp: 2025-10-10T17:09:05.986Z
Learning: The connector-builder-mcp repository at airbytehq/connector-builder-mcp is a private/internal repository that does not accept external contributions or forks.
Learnt from: aaronsteers
Repo: airbytehq/connector-builder-mcp PR: 116
File: connector_builder_agents/src/evals/evaluators.py:78-79
Timestamp: 2025-10-08T17:51:38.833Z
Learning: In the connector-builder-mcp repository, internal evaluation code (e.g., in connector_builder_agents/src/evals/) doesn't need to be bulletproof with extensive error handling, as the simpler approach is preferred for internal tooling.
📚 Learning: 2025-10-11T02:19:05.231Z
Learnt from: aaronsteers
Repo: airbytehq/connector-builder-mcp PR: 136
File: connector_builder_agents/src/evals/evaluators.py:233-245
Timestamp: 2025-10-11T02:19:05.231Z
Learning: In the connector-builder-mcp repository Python files, all imports must be placed at the module level (top of the file), never inside functions or methods. This is a recurring issue that should be flagged during code reviews.
Applied to files:
connector_builder_mcp/build_strategies/openapi_sonar_v1/prompts.py
📚 Learning: 2025-11-08T06:53:26.977Z
Learnt from: aaronsteers
Repo: airbytehq/connector-builder-mcp PR: 176
File: connector_builder_mcp/_manifest_history_utils.py:38-49
Timestamp: 2025-11-08T06:53:26.977Z
Learning: In the connector-builder-mcp repository, all imports must be at the module level (top of the file). Inline imports inside functions or methods are not allowed unless explicit permission is granted. This rule must never be violated.
Applied to files:
connector_builder_mcp/server.py
📚 Learning: 2025-10-10T17:09:05.986Z
Learnt from: aaronsteers
Repo: airbytehq/connector-builder-mcp PR: 116
File: .github/workflows/ci-tests.yml:110-113
Timestamp: 2025-10-10T17:09:05.986Z
Learning: The connector-builder-mcp repository at airbytehq/connector-builder-mcp is a private/internal repository that does not accept external contributions or forks.
Applied to files:
connector_builder_mcp/server.py
🧬 Code graph analysis (16)
connector_builder_mcp/build_strategies/openapi_sonar_v1/prompts.py (2)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_prompt(152-182)register_mcp_prompts(282-308)connector_builder_mcp/build_strategies/openapi_sonar_v1/build_strategy.py (1)
register_prompts(57-59)
connector_builder_mcp/build_strategies/openapi_sonar_v1/guidance.py (6)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/kotlin_destination_v1/build_strategy.py (1)
register_guidance_tools(41-43)connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py (1)
register_guidance_tools(431-435)connector_builder_mcp/build_strategies/kotlin_source_v1/build_strategy.py (1)
register_guidance_tools(41-43)connector_builder_mcp/build_strategies/kotlin_source_v1/guidance.py (1)
register_guidance_tools(307-311)connector_builder_mcp/build_strategies/openapi_sonar_v1/build_strategy.py (1)
register_guidance_tools(42-44)
connector_builder_mcp/build_strategies/kotlin_source_v1/guidance.py (2)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/kotlin_source_v1/build_strategy.py (1)
register_guidance_tools(41-43)
connector_builder_mcp/build_strategies/openapi_sonar_v1/build_strategy.py (5)
connector_builder_mcp/build_strategies/base/build_strategy.py (1)
BuildStrategy(18-185)connector_builder_mcp/build_strategies/openapi_sonar_v1/guidance.py (1)
register_guidance_tools(201-205)connector_builder_mcp/build_strategies/openapi_sonar_v1/manifest_checks.py (1)
register_manifest_check_tools(91-97)connector_builder_mcp/build_strategies/openapi_sonar_v1/manifest_tests.py (1)
register_manifest_test_tools(94-100)connector_builder_mcp/build_strategies/openapi_sonar_v1/prompts.py (1)
register_prompts(95-101)
connector_builder_mcp/build_strategies/kotlin_destination_v1/manifest_tests.py (4)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/kotlin_destination_v1/build_strategy.py (1)
register_manifest_test_tools(51-53)connector_builder_mcp/build_strategies/kotlin_source_v1/build_strategy.py (1)
register_manifest_test_tools(51-53)connector_builder_mcp/build_strategies/kotlin_source_v1/manifest_tests.py (1)
register_manifest_test_tools(92-98)
connector_builder_mcp/server.py (5)
connector_builder_mcp/build_strategies/kotlin_destination_v1/build_strategy.py (2)
KotlinDestinationV1Strategy(21-58)is_available(33-38)connector_builder_mcp/build_strategies/kotlin_source_v1/build_strategy.py (2)
KotlinSourceV1Strategy(21-58)is_available(33-38)connector_builder_mcp/build_strategies/openapi_sonar_v1/build_strategy.py (2)
OpenApiSonarV1Strategy(22-59)is_available(34-39)connector_builder_mcp/build_strategies/declarative_yaml_v1/build_strategy.py (2)
DeclarativeYamlV1Strategy(22-61)is_available(35-41)connector_builder_mcp/build_strategies/base/build_strategy.py (2)
is_available(52-61)register_all_mcp_callables(107-120)
connector_builder_mcp/build_strategies/kotlin_destination_v1/build_strategy.py (5)
connector_builder_mcp/build_strategies/base/build_strategy.py (1)
BuildStrategy(18-185)connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py (1)
register_guidance_tools(431-435)connector_builder_mcp/build_strategies/kotlin_destination_v1/manifest_checks.py (1)
register_manifest_check_tools(76-82)connector_builder_mcp/build_strategies/kotlin_destination_v1/manifest_tests.py (1)
register_manifest_test_tools(95-101)connector_builder_mcp/build_strategies/kotlin_destination_v1/prompts.py (1)
register_prompts(93-99)
connector_builder_mcp/build_strategies/kotlin_source_v1/prompts.py (6)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_prompt(152-182)register_mcp_prompts(282-308)connector_builder_mcp/build_strategies/kotlin_destination_v1/build_strategy.py (1)
register_prompts(56-58)connector_builder_mcp/build_strategies/kotlin_destination_v1/prompts.py (1)
register_prompts(93-99)connector_builder_mcp/build_strategies/kotlin_source_v1/build_strategy.py (1)
register_prompts(56-58)connector_builder_mcp/build_strategies/openapi_sonar_v1/build_strategy.py (1)
register_prompts(57-59)connector_builder_mcp/build_strategies/openapi_sonar_v1/prompts.py (1)
register_prompts(95-101)
connector_builder_mcp/build_strategies/kotlin_destination_v1/manifest_checks.py (3)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/kotlin_destination_v1/build_strategy.py (1)
register_manifest_check_tools(46-48)connector_builder_mcp/build_strategies/kotlin_source_v1/manifest_checks.py (1)
register_manifest_check_tools(76-82)
connector_builder_mcp/build_strategies/kotlin_source_v1/manifest_checks.py (4)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/kotlin_destination_v1/build_strategy.py (1)
register_manifest_check_tools(46-48)connector_builder_mcp/build_strategies/kotlin_source_v1/build_strategy.py (1)
register_manifest_check_tools(46-48)connector_builder_mcp/build_strategies/openapi_sonar_v1/build_strategy.py (1)
register_manifest_check_tools(47-49)
connector_builder_mcp/build_strategies/kotlin_source_v1/manifest_tests.py (5)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/kotlin_destination_v1/build_strategy.py (1)
register_manifest_test_tools(51-53)connector_builder_mcp/build_strategies/kotlin_destination_v1/manifest_tests.py (1)
register_manifest_test_tools(95-101)connector_builder_mcp/build_strategies/kotlin_source_v1/build_strategy.py (1)
register_manifest_test_tools(51-53)connector_builder_mcp/build_strategies/openapi_sonar_v1/build_strategy.py (1)
register_manifest_test_tools(52-54)
connector_builder_mcp/build_strategies/kotlin_source_v1/build_strategy.py (5)
connector_builder_mcp/build_strategies/base/build_strategy.py (1)
BuildStrategy(18-185)connector_builder_mcp/build_strategies/kotlin_source_v1/guidance.py (1)
register_guidance_tools(307-311)connector_builder_mcp/build_strategies/kotlin_source_v1/manifest_checks.py (1)
register_manifest_check_tools(76-82)connector_builder_mcp/build_strategies/kotlin_source_v1/manifest_tests.py (1)
register_manifest_test_tools(92-98)connector_builder_mcp/build_strategies/kotlin_source_v1/prompts.py (1)
register_prompts(92-98)
connector_builder_mcp/build_strategies/openapi_sonar_v1/manifest_checks.py (2)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/openapi_sonar_v1/build_strategy.py (1)
register_manifest_check_tools(47-49)
connector_builder_mcp/build_strategies/kotlin_destination_v1/prompts.py (3)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_prompt(152-182)register_mcp_prompts(282-308)connector_builder_mcp/build_strategies/kotlin_destination_v1/build_strategy.py (1)
register_prompts(56-58)connector_builder_mcp/build_strategies/kotlin_source_v1/prompts.py (1)
register_prompts(92-98)
connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py (2)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/kotlin_destination_v1/build_strategy.py (1)
register_guidance_tools(41-43)
connector_builder_mcp/build_strategies/openapi_sonar_v1/manifest_tests.py (2)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/openapi_sonar_v1/build_strategy.py (1)
register_manifest_test_tools(52-54)
🪛 GitHub Actions: CI Tests
connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py
[error] 186-186: SyntaxError: invalid syntax. Perhaps you forgot a comma? (likely inside the "write_modes" block in guidance.py)
🪛 GitHub Actions: Generate Docs
connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py
[error] 186-186: SyntaxError: invalid syntax. Perhaps you forgot a comma?
🪛 GitHub Actions: Linters
connector_builder_mcp/build_strategies/kotlin_source_v1/guidance.py
[error] 118-122: W293: Blank line contains whitespace. Ruff reported trailing/whitespace formatting issues in this file. Review and clean up blank lines.
connector_builder_mcp/build_strategies/openapi_sonar_v1/build_strategy.py
[error] 9-9: F401: 'sys' imported but unused. Remove unused import.
connector_builder_mcp/build_strategies/kotlin_destination_v1/build_strategy.py
[error] 8-8: F401: 'sys' imported but unused. Remove unused import.
connector_builder_mcp/build_strategies/kotlin_source_v1/build_strategy.py
[error] 8-8: F401: 'sys' imported but unused. Remove unused import.
connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py
[error] 122-160: W293, W291, E211: Multiple formatting issues detected (blank lines with whitespace, trailing whitespace, and related formatting hints). Ruff reported numerous whitespace-related lint issues in this file. Review and fix blank lines, trailing spaces, and line formatting to resolve.
🪛 GitHub Actions: Pytest (Fast)
connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py
[error] 186-186: SyntaxError: invalid syntax. Perhaps you forgot a comma?
🔇 Additional comments (18)
connector_builder_mcp/build_strategies/kotlin_source_v1/__init__.py (1)
1-1: Docstring-only module looks fineStraightforward, descriptive module docstring; no issues from a behavior or style perspective.
connector_builder_mcp/build_strategies/kotlin_destination_v1/__init__.py (1)
1-1: Kotlin destination init is fine; remember to fix guidance.py before mergeThe minimal docstring here is fine. As per the PR description, just ensure the known syntax error in
kotlin_destination_v1/guidance.pyis resolved before taking this out of draft.connector_builder_mcp/build_strategies/kotlin_destination_v1/checklist.yaml (1)
1-181: Checklist structure and content look coherentTop-level keys and task lists are well-structured and read logically for a Kotlin destination build workflow; indentation and YAML structure also look valid.
connector_builder_mcp/build_strategies/openapi_sonar_v1/manifest_checks.py (1)
91-97: Registration helper correctly wires tools into MANIFEST_CHECKS domain
register_manifest_check_toolsdelegating toregister_mcp_tools(app, domain=ToolDomain.MANIFEST_CHECKS)matches how the@mcp_tooldecorator tagsvalidate_openapi_spec, so this should integrate cleanly with the shared registration mechanism.connector_builder_mcp/build_strategies/kotlin_source_v1/manifest_checks.py (1)
76-82: Manifest-check tool registration is consistent with shared MCP utilitiesThe registration helper correctly reuses
register_mcp_toolswith the MANIFEST_CHECKS domain, which should make this tool discoverable wherever other manifest-check tools are registered.connector_builder_mcp/build_strategies/kotlin_destination_v1/manifest_tests.py (1)
87-92: Placeholder implementation acknowledged.The placeholder implementation is noted in the PR description. The success return with
records_written=0clearly indicates this is not yet functional.connector_builder_mcp/build_strategies/kotlin_source_v1/prompts.py (1)
20-89: LGTM!The prompt template follows the established MCP pattern and provides comprehensive guidance for building Kotlin source connectors.
connector_builder_mcp/build_strategies/openapi_sonar_v1/__init__.py (1)
1-1: LGTM!Minimal
__init__.pywith just a docstring is appropriate for this module.connector_builder_mcp/server.py (1)
65-78: LGTM!The multi-strategy registration flow correctly checks availability and registers tools for each strategy. The approach allows graceful handling of missing dependencies.
connector_builder_mcp/build_strategies/openapi_sonar_v1/checklist.yaml (1)
1-153: LGTM!The checklist provides comprehensive workflow guidance for OpenAPI/Sonar connector development, following the established pattern from other strategy checklists.
connector_builder_mcp/build_strategies/openapi_sonar_v1/guidance.py (1)
19-198: LGTM!The guidance documentation tool follows the established pattern and provides comprehensive OpenAPI/Sonar connector development guidance.
connector_builder_mcp/build_strategies/kotlin_source_v1/checklist.yaml (1)
1-186: LGTM!The checklist provides comprehensive guidance for Kotlin source connector development, following the established checklist pattern.
connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py (1)
122-160: Fix formatting issues.The linter reports multiple whitespace-related issues in this range: blank lines with whitespace (W293), trailing whitespace (W291), and whitespace before brackets (E211).
Run the formatters to fix these issues:
#!/bin/bash # Fix formatting issues ruff check --fix connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py ruff format connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py⛔ Skipped due to learnings
Learnt from: aaronsteers Repo: airbytehq/connector-builder-mcp PR: 136 File: connector_builder_agents/src/evals/evaluators.py:233-245 Timestamp: 2025-10-11T02:19:05.231Z Learning: In the connector-builder-mcp repository Python files, all imports must be placed at the module level (top of the file), never inside functions or methods. This is a recurring issue that should be flagged during code reviews.connector_builder_mcp/build_strategies/openapi_sonar_v1/prompts.py (1)
1-101: Prompt definition and registration look consistentThe OpenAPI/Sonar prompt and
register_promptshook follow the existing MCP patterns cleanly; parameters, defaults, and registration domain all look good.connector_builder_mcp/build_strategies/kotlin_destination_v1/prompts.py (1)
1-99: Kotlin destination prompt wiring looks goodThe prompt definition, parameter handling, and
register_promptshook are consistent with the existing prompt modules and MCP utilities.connector_builder_mcp/build_strategies/openapi_sonar_v1/manifest_tests.py (3)
1-16: LGTM! Clean module structure.The imports are appropriate and well-organized. The module docstring clearly describes the purpose of testing OpenAPI-based connectors.
19-28: LGTM! Well-designed result model.The
ResourceTestResultmodel provides a clear structure for test results with sensible defaults and helpful field descriptions.
94-100: LGTM! Clean registration helper.The registration function follows the established pattern for registering MCP tools by domain. Implementation is correct and matches the pattern used in other strategy modules.
| from __future__ import annotations | ||
|
|
||
| import sys | ||
|
|
There was a problem hiding this comment.
Drop unused sys import and ensure destination guidance module compiles
Ruff flags sys as unused (F401). You can safely remove it here:
-from __future__ import annotations
-
-import sys
-
-from fastmcp import FastMCP
+from __future__ import annotations
+
+from fastmcp import FastMCPAlso, per the PR description, don’t forget to fix the syntax error in kotlin_destination_v1/guidance.py before enabling this strategy end‑to‑end.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: Linters
[error] 8-8: F401: 'sys' imported but unused. Remove unused import.
🤖 Prompt for AI Agents
In
connector_builder_mcp/build_strategies/kotlin_destination_v1/build_strategy.py
around lines 6 to 9, remove the unused import "sys" (Ruff F401) so the file only
imports needed symbols; additionally open kotlin_destination_v1/guidance.py and
fix the reported syntax error mentioned in the PR (correct the offending
token/structure so the module compiles) before enabling this strategy
end-to-end.
connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py
Outdated
Show resolved
Hide resolved
connector_builder_mcp/build_strategies/kotlin_source_v1/build_strategy.py
Show resolved
Hide resolved
connector_builder_mcp/build_strategies/kotlin_source_v1/guidance.py
Outdated
Show resolved
Hide resolved
| class KotlinSourceValidationResult(BaseModel): | ||
| """Result of Kotlin source connector validation.""" | ||
|
|
||
| is_valid: bool | ||
| errors: list[str] = [] | ||
| warnings: list[str] = [] | ||
| streams_found: list[str] = [] | ||
|
|
||
|
|
||
| @mcp_tool( | ||
| ToolDomain.MANIFEST_CHECKS, | ||
| read_only=True, | ||
| idempotent=True, | ||
| open_world=False, | ||
| ) | ||
| def validate_kotlin_source_connector( | ||
| ctx: Context, | ||
| *, | ||
| project_path: Annotated[ | ||
| str | None, | ||
| Field(description="Path to the Kotlin source connector project directory"), | ||
| ] = None, | ||
| ) -> KotlinSourceValidationResult: | ||
| """Validate a Kotlin source connector project structure and code. | ||
|
|
||
| Args: | ||
| ctx: FastMCP context (automatically injected) | ||
| project_path: Path to the connector project | ||
|
|
||
| Returns: | ||
| Validation result with success status and any errors/warnings | ||
| """ | ||
| logger.info("Validating Kotlin source connector") | ||
|
|
||
| if project_path is None: | ||
| return KotlinSourceValidationResult( | ||
| is_valid=False, | ||
| errors=["No project path provided"], | ||
| warnings=[], | ||
| ) | ||
|
|
||
| errors = [] | ||
| warnings = [] | ||
| streams_found = [] | ||
|
|
||
| warnings.append("Kotlin source validation is a placeholder implementation") | ||
|
|
||
| is_valid = len(errors) == 0 | ||
|
|
||
| return KotlinSourceValidationResult( | ||
| is_valid=is_valid, | ||
| errors=errors, | ||
| warnings=warnings, | ||
| streams_found=streams_found, | ||
| ) |
There was a problem hiding this comment.
Align docstring and help text with current placeholder behavior
Right now the tool only checks that project_path is provided and then returns is_valid=True with a generic “placeholder implementation” warning; it doesn’t yet inspect project structure, code, or streams.
To avoid overpromising to tool users until the real checks exist, consider:
- Updating the function docstring and/or
Fielddescription to explicitly call this a lightweight/placeholder validation, or - Implementing at least a minimal structural check (e.g., presence of expected Gradle/Kotlin files) before advertising full “project structure and code” validation.
This keeps expectations clear while you iterate on the real validator.
🤖 Prompt for AI Agents
In connector_builder_mcp/build_strategies/kotlin_source_v1/manifest_checks.py
around lines 19-73, the function docstring and Field description promise a full
“project structure and code” validation but the implementation is a placeholder
that only checks for project_path and returns a generic placeholder warning;
either update the docstring and the Field(description=...) text to state this is
a lightweight/placeholder validation, or implement a minimal structural check:
verify the project_path exists and contains expected files/directories (e.g.,
build.gradle(.kts) or settings.gradle(.kts), and src/main/kotlin or
src/main/java), append specific errors when missing, set is_valid = len(errors)
== 0 accordingly, and provide concrete warnings/errors in the returned
KotlinSourceValidationResult.
connector_builder_mcp/build_strategies/openapi_sonar_v1/build_strategy.py
Outdated
Show resolved
Hide resolved
| class OpenApiValidationResult(BaseModel): | ||
| """Result of OpenAPI specification validation.""" | ||
|
|
||
| is_valid: bool | ||
| errors: list[str] = [] | ||
| warnings: list[str] = [] | ||
| resources_found: list[str] = [] | ||
|
|
||
|
|
||
| @mcp_tool( | ||
| ToolDomain.MANIFEST_CHECKS, | ||
| read_only=True, | ||
| idempotent=True, | ||
| open_world=False, | ||
| ) | ||
| def validate_openapi_spec( | ||
| ctx: Context, | ||
| *, | ||
| spec_content: Annotated[ | ||
| str | None, | ||
| Field( | ||
| description="The OpenAPI specification content (YAML or JSON). " | ||
| "Can be raw content or path to spec file." | ||
| ), | ||
| ] = None, | ||
| ) -> OpenApiValidationResult: | ||
| """Validate an OpenAPI specification for connector building. | ||
|
|
||
| Args: | ||
| ctx: FastMCP context (automatically injected) | ||
| spec_content: The OpenAPI specification to validate | ||
|
|
||
| Returns: | ||
| Validation result with success status and any errors/warnings | ||
| """ | ||
| logger.info("Validating OpenAPI specification") | ||
|
|
||
| if spec_content is None: | ||
| return OpenApiValidationResult( | ||
| is_valid=False, | ||
| errors=["No OpenAPI specification provided"], | ||
| warnings=[], | ||
| ) | ||
|
|
||
| errors = [] | ||
| warnings = [] | ||
| resources_found = [] | ||
|
|
||
| if len(spec_content) < 10: | ||
| errors.append("OpenAPI specification appears to be empty or too short") | ||
|
|
||
| if "openapi" not in spec_content.lower(): | ||
| errors.append("Missing 'openapi' version field") | ||
|
|
||
| if "paths" not in spec_content.lower(): | ||
| warnings.append("No 'paths' section found in specification") | ||
|
|
||
| if "x-airbyte" not in spec_content: | ||
| warnings.append( | ||
| "No x-airbyte-* extensions found. You may need to add these to define resources." | ||
| ) | ||
|
|
||
| is_valid = len(errors) == 0 | ||
|
|
||
| return OpenApiValidationResult( | ||
| is_valid=is_valid, | ||
| errors=errors, | ||
| warnings=warnings, | ||
| resources_found=resources_found, | ||
| ) |
There was a problem hiding this comment.
Clarify spec_content contract (raw spec vs path) or implement path loading
The implementation treats spec_content purely as a text blob (length checks, substring searches) and never attempts to resolve or read from a filesystem path, but the parameter/Field description says it “Can be raw content or path to spec file.” Passing a path today will just run these string checks on the path itself and likely produce misleading errors/warnings.
Either:
- Narrow the description/docstring to say this expects raw OpenAPI spec content only, or
- Extend the function to detect a likely file path and load its contents before validation (and then revisit
open_worldif you consider filesystem access “external”).
The current behavior is otherwise consistent and safe; this is mainly to avoid confusing tool consumers.
| spec_content: Annotated[ | ||
| str | None, | ||
| Field( | ||
| description="The OpenAPI specification content. Can be raw content or path to spec file." | ||
| ), | ||
| ] = None, | ||
| config: Annotated[ | ||
| dict[str, Any] | str | None, | ||
| Field(description="Connector configuration dictionary (including auth credentials)."), | ||
| ] = None, |
There was a problem hiding this comment.
Consider making required parameters non-optional.
The spec_content and config parameters have default values of None, which suggests they're optional. However, the function body treats them as required by returning errors when they're None (lines 71-83). This creates a misleading API.
Consider one of these approaches:
Option 1 (Preferred): Make them truly required
- spec_content: Annotated[
- str | None,
- Field(
- description="The OpenAPI specification content. Can be raw content or path to spec file."
- ),
- ] = None,
+ spec_content: Annotated[
+ str,
+ Field(
+ description="The OpenAPI specification content. Can be raw content or path to spec file."
+ ),
+ ],
config: Annotated[
- dict[str, Any] | str | None,
+ dict[str, Any] | str,
Field(description="Connector configuration dictionary (including auth credentials)."),
- ] = None,
+ ],Option 2: Accept them as optional and handle gracefully in a real implementation
If there's a valid use case for calling this without credentials (e.g., dry-run validation), keep them optional but document this behavior clearly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| spec_content: Annotated[ | |
| str | None, | |
| Field( | |
| description="The OpenAPI specification content. Can be raw content or path to spec file." | |
| ), | |
| ] = None, | |
| config: Annotated[ | |
| dict[str, Any] | str | None, | |
| Field(description="Connector configuration dictionary (including auth credentials)."), | |
| ] = None, | |
| spec_content: Annotated[ | |
| str, | |
| Field( | |
| description="The OpenAPI specification content. Can be raw content or path to spec file." | |
| ), | |
| ], | |
| config: Annotated[ | |
| dict[str, Any] | str, | |
| Field(description="Connector configuration dictionary (including auth credentials)."), | |
| ], |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: aaronsteers
Repo: airbytehq/connector-builder-mcp PR: 116
File: .github/workflows/ci-tests.yml:110-113
Timestamp: 2025-10-10T17:09:02.151Z
Learning: The connector-builder-mcp repository (airbytehq/connector-builder-mcp) is an internal/private repository that doesn't accept external contributions, so there are no forked PRs to consider in workflow configurations.
📚 Learning: 2025-10-11T02:19:05.231Z
Learnt from: aaronsteers
Repo: airbytehq/connector-builder-mcp PR: 136
File: connector_builder_agents/src/evals/evaluators.py:233-245
Timestamp: 2025-10-11T02:19:05.231Z
Learning: In the connector-builder-mcp repository Python files, all imports must be placed at the module level (top of the file), never inside functions or methods. This is a recurring issue that should be flagged during code reviews.
Applied to files:
connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py
🧬 Code graph analysis (1)
connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py (6)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/kotlin_destination_v1/build_strategy.py (1)
register_guidance_tools(41-43)connector_builder_mcp/build_strategies/kotlin_source_v1/build_strategy.py (1)
register_guidance_tools(41-43)connector_builder_mcp/build_strategies/kotlin_source_v1/guidance.py (1)
register_guidance_tools(307-311)connector_builder_mcp/build_strategies/openapi_sonar_v1/guidance.py (1)
register_guidance_tools(201-205)connector_builder_mcp/build_strategies/openapi_sonar_v1/build_strategy.py (1)
register_guidance_tools(42-44)
🪛 GitHub Actions: Linters
connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py
[warning] 122-122: W293 Blank line contains whitespace
[warning] 124-124: W293 Blank line contains whitespace
[warning] 131-131: W293 Blank line contains whitespace
[warning] 144-144: W293 Blank line contains whitespace
[warning] 158-158: W293 Blank line contains whitespace
[warning] 160-160: W293 Blank line contains whitespace
[warning] 167-167: W293 Blank line contains whitespace
[warning] 177-177: W293 Blank line contains whitespace
[warning] 194-194: W293 Blank line contains whitespace
[warning] 211-211: W293 Blank line contains whitespace
[warning] 213-213: W293 Blank line contains whitespace
[warning] 236-236: W293 Blank line contains whitespace
[warning] 244-244: W293 Blank line contains whitespace
[warning] 265-265: W293 Blank line contains whitespace
[warning] 272-272: W293 Blank line contains whitespace
[warning] 275-275: W293 Blank line contains whitespace
[warning] 297-297: W293 Blank line contains whitespace
[warning] 300-300: W293 Blank line contains whitespace
[warning] 326-326: W293 Blank line contains whitespace
[warning] 342-342: W293 Blank line contains whitespace
[warning] 367-367: W293 Blank line contains whitespace
[warning] 390-390: W293 Blank line contains whitespace
🔇 Additional comments (4)
connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py (4)
1-16: LGTM!Module structure and imports are correct. All imports are at the module level as required.
19-38: LGTM!Function decorator and signature follow the established pattern from other build strategies and correctly use ToolDomain.GUIDANCE.
40-76: LGTM!High-level overview is clear and appropriately references the checklist tool for step-by-step guidance.
431-435: LGTM!Registration function follows the established pattern from other build strategies.
|
|
||
| private val writers = mutableMapOf<String, StreamWriter>() | ||
|
|
||
| override fun start() { | ||
| // Initialize connections and writers | ||
| catalog.streams.forEach { stream -> | ||
| writers[stream.stream.name] = createWriter(stream) | ||
| } | ||
| } | ||
|
|
||
| override fun accept(message: AirbyteMessage) { | ||
| when (message.type) { | ||
| Type.RECORD -> { | ||
| val streamName = message.record.stream | ||
| writers[streamName]?.write(message.record.data) | ||
| } | ||
| Type.STATE -> { | ||
| // Flush buffers and commit | ||
| writers.values.forEach { it.flush() } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| override fun close() { | ||
| writers.values.forEach { it.close() } | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
|
|
||
| ```kotlin | ||
| class BufferedConsumer( | ||
| private val config: JsonNode, | ||
| private val catalog: ConfiguredAirbyteCatalog, | ||
| private val bufferSize: Int = 1000 | ||
| ) : AirbyteMessageConsumer { | ||
|
|
||
| private val buffers = mutableMapOf<String, MutableList<JsonNode>>() | ||
|
|
||
| override fun accept(message: AirbyteMessage) { | ||
| when (message.type) { | ||
| Type.RECORD -> { | ||
| val streamName = message.record.stream | ||
| val buffer = buffers.getOrPut(streamName) { mutableListOf() } | ||
| buffer.add(message.record.data) | ||
|
|
||
| if (buffer.size >= bufferSize) { | ||
| flushBuffer(streamName) | ||
| } | ||
| } | ||
| Type.STATE -> { | ||
| flushAllBuffers() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private fun flushBuffer(streamName: String) { | ||
| val buffer = buffers[streamName] ?: return | ||
| writers[streamName]?.writeBatch(buffer) | ||
| buffer.clear() | ||
| } | ||
| } | ||
| ``` | ||
| """, | ||
| "write_modes": '''# Write Modes Implementation | ||
|
|
||
|
|
||
| ```kotlin | ||
| class AppendWriter( | ||
| private val tableName: String, | ||
| private val connection: Connection | ||
| ) : StreamWriter { | ||
|
|
||
| override fun write(record: JsonNode) { | ||
| val sql = "INSERT INTO $tableName (${getColumns()}) VALUES (${getPlaceholders()})" | ||
| connection.prepareStatement(sql).use { stmt -> | ||
| setParameters(stmt, record) | ||
| stmt.executeUpdate() | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
|
|
||
| ```kotlin | ||
| class OverwriteWriter( | ||
| private val tableName: String, | ||
| private val connection: Connection | ||
| ) : StreamWriter { | ||
|
|
||
| private var isFirstBatch = true | ||
|
|
||
| override fun writeBatch(records: List<JsonNode>) { | ||
| if (isFirstBatch) { | ||
| // Truncate or drop/recreate table | ||
| connection.createStatement().use { | ||
| it.execute("TRUNCATE TABLE $tableName") | ||
| } | ||
| isFirstBatch = false | ||
| } | ||
|
|
||
| // Insert records | ||
| records.forEach { write(it) } | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
|
|
||
| ```kotlin | ||
| class UpsertWriter( | ||
| private val tableName: String, | ||
| private val primaryKeys: List<String>, | ||
| private val connection: Connection | ||
| ) : StreamWriter { | ||
|
|
||
| override fun write(record: JsonNode) { | ||
| val sql = buildUpsertSql() | ||
| connection.prepareStatement(sql).use { stmt -> | ||
| setParameters(stmt, record) | ||
| stmt.executeUpdate() | ||
| } | ||
| } | ||
|
|
||
| private fun buildUpsertSql(): String { | ||
| // PostgreSQL example | ||
| return """ | ||
| INSERT INTO $tableName (${getColumns()}) | ||
| VALUES (${getPlaceholders()}) | ||
| ON CONFLICT (${primaryKeys.joinToString()}) | ||
| DO UPDATE SET ${getUpdateClause()} | ||
| """.trimIndent() | ||
| } | ||
| } | ||
| ``` | ||
| ''', | ||
| "batching": """# Batching Strategies | ||
|
|
||
|
|
||
| ```kotlin | ||
| class BatchWriter( | ||
| private val batchSize: Int = 1000 | ||
| ) { | ||
| private val buffer = mutableListOf<JsonNode>() | ||
|
|
||
| fun addRecord(record: JsonNode) { | ||
| buffer.add(record) | ||
| if (buffer.size >= batchSize) { | ||
| flush() | ||
| } | ||
| } | ||
|
|
||
| fun flush() { | ||
| if (buffer.isEmpty()) return | ||
|
|
||
| val sql = buildBatchInsertSql(buffer.size) | ||
| connection.prepareStatement(sql).use { stmt -> | ||
| var paramIndex = 1 | ||
| buffer.forEach { record -> | ||
| setParameters(stmt, record, paramIndex) | ||
| paramIndex += getColumnCount() | ||
| } | ||
| stmt.executeUpdate() | ||
| } | ||
| buffer.clear() | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
|
|
||
| ```kotlin | ||
| class TimeBasedBatchWriter( | ||
| private val flushIntervalMs: Long = 5000 | ||
| ) { | ||
| private val buffer = mutableListOf<JsonNode>() | ||
| private var lastFlushTime = System.currentTimeMillis() | ||
|
|
||
| fun addRecord(record: JsonNode) { | ||
| buffer.add(record) | ||
|
|
||
| val now = System.currentTimeMillis() | ||
| if (now - lastFlushTime >= flushIntervalMs) { | ||
| flush() | ||
| lastFlushTime = now | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
| """, | ||
| "schema_mapping": '''# Schema Mapping and Type Conversion | ||
|
|
||
|
|
||
| ```kotlin | ||
| object TypeMapper { | ||
| fun airbyteTypeToSqlType(airbyteType: JsonNode): String { | ||
| return when (airbyteType.get("type").asText()) { | ||
| "string" -> "VARCHAR" | ||
| "integer" -> "BIGINT" | ||
| "number" -> "DOUBLE" | ||
| "boolean" -> "BOOLEAN" | ||
| "array" -> "JSON" | ||
| "object" -> "JSON" | ||
| else -> "VARCHAR" | ||
| } | ||
| } | ||
|
|
||
| fun convertValue(value: JsonNode, targetType: String): Any? { | ||
| return when (targetType) { | ||
| "BIGINT" -> value.asLong() | ||
| "DOUBLE" -> value.asDouble() | ||
| "BOOLEAN" -> value.asBoolean() | ||
| "JSON" -> value.toString() | ||
| else -> value.asText() | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
|
|
||
| ```kotlin | ||
| class SchemaManager(private val connection: Connection) { | ||
|
|
||
| fun createTableFromAirbyteSchema( | ||
| tableName: String, | ||
| schema: JsonNode | ||
| ) { | ||
| val columns = schema.get("properties").fields().asSequence() | ||
| .map { (name, type) -> | ||
| "$name ${TypeMapper.airbyteTypeToSqlType(type)}" | ||
| } | ||
| .joinToString(", ") | ||
|
|
||
| val sql = "CREATE TABLE IF NOT EXISTS $tableName ($columns)" | ||
| connection.createStatement().use { it.execute(sql) } | ||
| } | ||
| } | ||
| ``` | ||
| """, | ||
| "error_handling": """# Error Handling and Retry Logic | ||
|
|
||
|
|
||
| ```kotlin | ||
| class RetryableWriter( | ||
| private val delegate: StreamWriter, | ||
| private val maxRetries: Int = 3 | ||
| ) : StreamWriter { | ||
|
|
||
| override fun write(record: JsonNode) { | ||
| var attempt = 0 | ||
| var lastException: Exception? = null | ||
|
|
||
| while (attempt < maxRetries) { | ||
| try { | ||
| delegate.write(record) | ||
| return | ||
| } catch (e: SQLException) { | ||
| lastException = e | ||
| attempt++ | ||
|
|
||
| if (isRetryable(e)) { | ||
| Thread.sleep(getBackoffDelay(attempt)) | ||
| } else { | ||
| throw e | ||
| } | ||
| } | ||
| } | ||
|
|
||
| throw RuntimeException("Failed after $maxRetries attempts", lastException) | ||
| } | ||
|
|
There was a problem hiding this comment.
Address trailing whitespace in code examples.
The linter reports W293 warnings (blank line contains whitespace) at multiple lines within the embedded Kotlin code examples. While these don't affect functionality, removing trailing whitespace will clean up the code and satisfy the linter.
Run the following command to automatically fix trailing whitespace:
#!/bin/bash
# Remove trailing whitespace from the file
sed -i 's/[[:space:]]*$//' connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py🧰 Tools
🪛 GitHub Actions: Linters
[warning] 122-122: W293 Blank line contains whitespace
[warning] 124-124: W293 Blank line contains whitespace
[warning] 131-131: W293 Blank line contains whitespace
[warning] 144-144: W293 Blank line contains whitespace
[warning] 158-158: W293 Blank line contains whitespace
[warning] 160-160: W293 Blank line contains whitespace
[warning] 167-167: W293 Blank line contains whitespace
[warning] 177-177: W293 Blank line contains whitespace
[warning] 194-194: W293 Blank line contains whitespace
[warning] 211-211: W293 Blank line contains whitespace
[warning] 213-213: W293 Blank line contains whitespace
[warning] 236-236: W293 Blank line contains whitespace
[warning] 244-244: W293 Blank line contains whitespace
[warning] 265-265: W293 Blank line contains whitespace
[warning] 272-272: W293 Blank line contains whitespace
[warning] 275-275: W293 Blank line contains whitespace
[warning] 297-297: W293 Blank line contains whitespace
[warning] 300-300: W293 Blank line contains whitespace
[warning] 326-326: W293 Blank line contains whitespace
[warning] 342-342: W293 Blank line contains whitespace
[warning] 367-367: W293 Blank line contains whitespace
[warning] 390-390: W293 Blank line contains whitespace
🤖 Prompt for AI Agents
In connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py
around lines 122-390 there are multiple embedded Kotlin code examples containing
trailing whitespace (linter W293). Remove trailing spaces from those example
blocks — e.g., run a file-wide trailing-whitespace strip (sed -i
's/[[:space:]]*$//'
connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py) or use
your editor/IDE to trim trailing whitespace and reformat the code blocks, then
re-run the linter to confirm W293 warnings are gone.
| "schema_mapping": '''# Schema Mapping and Type Conversion | ||
|
|
||
|
|
||
| ```kotlin | ||
| object TypeMapper { | ||
| fun airbyteTypeToSqlType(airbyteType: JsonNode): String { | ||
| return when (airbyteType.get("type").asText()) { | ||
| "string" -> "VARCHAR" | ||
| "integer" -> "BIGINT" | ||
| "number" -> "DOUBLE" | ||
| "boolean" -> "BOOLEAN" | ||
| "array" -> "JSON" | ||
| "object" -> "JSON" | ||
| else -> "VARCHAR" | ||
| } | ||
| } | ||
|
|
||
| fun convertValue(value: JsonNode, targetType: String): Any? { | ||
| return when (targetType) { | ||
| "BIGINT" -> value.asLong() | ||
| "DOUBLE" -> value.asDouble() | ||
| "BOOLEAN" -> value.asBoolean() | ||
| "JSON" -> value.toString() | ||
| else -> value.asText() | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
|
|
||
| ```kotlin | ||
| class SchemaManager(private val connection: Connection) { | ||
|
|
||
| fun createTableFromAirbyteSchema( | ||
| tableName: String, | ||
| schema: JsonNode | ||
| ) { | ||
| val columns = schema.get("properties").fields().asSequence() | ||
| .map { (name, type) -> | ||
| "$name ${TypeMapper.airbyteTypeToSqlType(type)}" | ||
| } | ||
| .joinToString(", ") | ||
|
|
||
| val sql = "CREATE TABLE IF NOT EXISTS $tableName ($columns)" | ||
| connection.createStatement().use { it.execute(sql) } | ||
| } | ||
| } | ||
| ``` | ||
| """, |
There was a problem hiding this comment.
Critical: Fix mismatched string quotes blocking module load.
The "schema_mapping" topic starts with triple single quotes (''') on line 310 but ends with triple double quotes (""") on line 358. This mismatch causes a syntax error that prevents the module from loading.
Apply this diff to fix the mismatch:
-""",
+''',🧰 Tools
🪛 GitHub Actions: Linters
[warning] 326-326: W293 Blank line contains whitespace
[warning] 342-342: W293 Blank line contains whitespace
🤖 Prompt for AI Agents
In connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py
around lines 310 to 358, the multi-line string for "schema_mapping" begins with
triple single quotes (''') but is closed with triple double quotes ("""),
causing a syntax error; fix it by making the opening and closing delimiters
match (change the closing triple double quotes to triple single quotes or vice
versa) so the string is properly terminated and the module can load.
| "error_handling": """# Error Handling and Retry Logic | ||
|
|
||
|
|
||
| ```kotlin | ||
| class RetryableWriter( | ||
| private val delegate: StreamWriter, | ||
| private val maxRetries: Int = 3 | ||
| ) : StreamWriter { | ||
|
|
||
| override fun write(record: JsonNode) { | ||
| var attempt = 0 | ||
| var lastException: Exception? = null | ||
|
|
||
| while (attempt < maxRetries) { | ||
| try { | ||
| delegate.write(record) | ||
| return | ||
| } catch (e: SQLException) { | ||
| lastException = e | ||
| attempt++ | ||
|
|
||
| if (isRetryable(e)) { | ||
| Thread.sleep(getBackoffDelay(attempt)) | ||
| } else { | ||
| throw e | ||
| } | ||
| } | ||
| } | ||
|
|
||
| throw RuntimeException("Failed after $maxRetries attempts", lastException) | ||
| } | ||
|
|
||
| private fun isRetryable(e: SQLException): Boolean { | ||
| // Check for transient errors | ||
| return e.sqlState in listOf("08001", "08006", "40001") | ||
| } | ||
|
|
||
| private fun getBackoffDelay(attempt: Int): Long { | ||
| return (1000L * Math.pow(2.0, attempt.toDouble())).toLong() | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
|
|
||
| ```kotlin | ||
| class TransactionalWriter( | ||
| private val connection: Connection | ||
| ) : StreamWriter { | ||
|
|
||
| override fun writeBatch(records: List<JsonNode>) { | ||
| connection.autoCommit = false | ||
| try { | ||
| records.forEach { write(it) } | ||
| connection.commit() | ||
| } catch (e: Exception) { | ||
| connection.rollback() | ||
| throw e | ||
| } finally { | ||
| connection.autoCommit = true | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
| ''', |
There was a problem hiding this comment.
Critical: Fix mismatched string quotes blocking module load.
The "error_handling" topic starts with triple double quotes (""") on line 359 but ends with triple single quotes (''') on line 422. This mismatch causes a syntax error that prevents the module from loading.
Apply this diff to fix the mismatch:
-''',
+""",🧰 Tools
🪛 GitHub Actions: Linters
[warning] 367-367: W293 Blank line contains whitespace
[warning] 390-390: W293 Blank line contains whitespace
🤖 Prompt for AI Agents
In connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py
around lines 359 to 422, the multi-line string for "error_handling" opens with
triple double-quotes (""") but is closed with triple single-quotes ('''),
causing a syntax error; fix it by making the opening and closing quotes match
(change the closing quotes to triple double-quotes) so the string is properly
terminated and the module can load.
aaronsteers
changed the title
…d strategies Co-Authored-By: AJ Steers <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
connector_builder_mcp/build_strategies/kotlin_source_v1/manifest_checks.py (1)
34-73: Align docstring with current placeholder behavior.The function docstring and Field description promise "project structure and code" validation, but the implementation is a placeholder that only checks for
project_pathand returns a generic placeholder warning. To avoid overpromising to tool users until the real checks exist, consider updating the docstring to explicitly call this a lightweight/placeholder validation.connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py (3)
310-358: Critical: Fix mismatched string quotes blocking module load.The "schema_mapping" topic starts with triple single quotes (
''') on line 310 but ends with triple double quotes (""") on line 358. This mismatch causes a syntax error that prevents the module from loading.Apply this diff to fix the mismatch:
-""", +''',
359-422: Critical: Fix mismatched string quotes blocking module load.The "error_handling" topic starts with triple double quotes (
""") on line 359 but ends with triple single quotes (''') on line 422. This mismatch causes a syntax error that prevents the module from loading.Apply this diff to fix the mismatch:
-''', +""",
122-390: Address trailing whitespace in code examples.The linter reports W293 warnings (blank line contains whitespace) at multiple lines within the embedded Kotlin code examples. While these don't affect functionality, removing trailing whitespace will clean up the code and satisfy the linter.
Run the following command to automatically fix trailing whitespace:
#!/bin/bash # Remove trailing whitespace from the file sed -i 's/[[:space:]]*$//' connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.pyconnector_builder_mcp/build_strategies/kotlin_source_v1/guidance.py (1)
118-122: Clean up trailing whitespace on blank lines (W293).Ruff reports W293 for blank lines with spaces in this Kotlin code block. Strip whitespace from the empty lines so they are truly blank, or run the project formatter to normalize them.
Run the following command to fix trailing whitespace:
#!/bin/bash # Remove trailing whitespace from the file sed -i 's/[[:space:]]*$//' connector_builder_mcp/build_strategies/kotlin_source_v1/guidance.pyconnector_builder_mcp/build_strategies/openapi_sonar_v1/manifest_checks.py (1)
37-42: Documentation still doesn't match implementation.This is the same issue flagged in the previous review: the Field description claims
spec_content"Can be raw content or path to spec file," but the implementation only performs string checks (length, substring searches) and never attempts to load from a path. Passing a file path will produce misleading validation results.As suggested in the previous review, either:
- Update the description to clarify this expects raw OpenAPI spec content only, or
- Implement path detection and file loading before validation
🧹 Nitpick comments (2)
connector_builder_mcp/build_strategies/kotlin_destination_v1/manifest_checks.py (1)
23-25: UseField(default_factory=list)for mutable defaults in Pydantic models.While Pydantic v2 handles mutable defaults better than v1, the recommended best practice is to use
Field(default_factory=list)for list fields with defaults to avoid potential issues and follow Pydantic conventions.Apply this diff:
- errors: list[str] = [] - warnings: list[str] = [] - streams_found: list[str] = [] + errors: list[str] = Field(default_factory=list) + warnings: list[str] = Field(default_factory=list) + streams_found: list[str] = Field(default_factory=list)connector_builder_mcp/build_strategies/openapi_sonar_v1/manifest_checks.py (1)
19-25: PreferField(default_factory=list)for mutable defaults.While Pydantic v2 handles bare
= []correctly, usingField(default_factory=list)is more explicit and aligns with best practices for mutable defaults.Apply this diff:
class OpenApiValidationResult(BaseModel): """Result of OpenAPI specification validation.""" is_valid: bool - errors: list[str] = [] - warnings: list[str] = [] - resources_found: list[str] = [] + errors: list[str] = Field(default_factory=list) + warnings: list[str] = Field(default_factory=list) + resources_found: list[str] = Field(default_factory=list)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
connector_builder_mcp/build_strategies/kotlin_destination_v1/build_strategy.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_destination_v1/manifest_checks.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_source_v1/build_strategy.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_source_v1/guidance.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_source_v1/manifest_checks.py(1 hunks)connector_builder_mcp/build_strategies/openapi_sonar_v1/build_strategy.py(1 hunks)connector_builder_mcp/build_strategies/openapi_sonar_v1/manifest_checks.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- connector_builder_mcp/build_strategies/openapi_sonar_v1/build_strategy.py
- connector_builder_mcp/build_strategies/kotlin_destination_v1/build_strategy.py
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: aaronsteers
Repo: airbytehq/connector-builder-mcp PR: 116
File: .github/workflows/ci-tests.yml:110-113
Timestamp: 2025-10-10T17:09:02.151Z
Learning: The connector-builder-mcp repository (airbytehq/connector-builder-mcp) is an internal/private repository that doesn't accept external contributions, so there are no forked PRs to consider in workflow configurations.
📚 Learning: 2025-10-11T02:19:05.231Z
Learnt from: aaronsteers
Repo: airbytehq/connector-builder-mcp PR: 136
File: connector_builder_agents/src/evals/evaluators.py:233-245
Timestamp: 2025-10-11T02:19:05.231Z
Learning: In the connector-builder-mcp repository Python files, all imports must be placed at the module level (top of the file), never inside functions or methods. This is a recurring issue that should be flagged during code reviews.
Applied to files:
connector_builder_mcp/build_strategies/kotlin_source_v1/build_strategy.pyconnector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py
📚 Learning: 2025-11-08T06:53:26.977Z
Learnt from: aaronsteers
Repo: airbytehq/connector-builder-mcp PR: 176
File: connector_builder_mcp/_manifest_history_utils.py:38-49
Timestamp: 2025-11-08T06:53:26.977Z
Learning: In the connector-builder-mcp repository, all imports must be at the module level (top of the file). Inline imports inside functions or methods are not allowed unless explicit permission is granted. This rule must never be violated.
Applied to files:
connector_builder_mcp/build_strategies/kotlin_source_v1/build_strategy.py
📚 Learning: 2025-10-10T17:57:12.175Z
Learnt from: aaronsteers
Repo: airbytehq/connector-builder-mcp PR: 113
File: connector_builder_agents/src/agents.py:113-124
Timestamp: 2025-10-10T17:57:12.175Z
Learning: In `connector_builder_agents/src/agents.py`, the spaces after `\n` in log messages (e.g., `\n Task Name:`) are intentional design choices to create indentation in logs for better readability.
Applied to files:
connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py
🧬 Code graph analysis (6)
connector_builder_mcp/build_strategies/openapi_sonar_v1/manifest_checks.py (2)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/openapi_sonar_v1/build_strategy.py (1)
register_manifest_check_tools(45-47)
connector_builder_mcp/build_strategies/kotlin_source_v1/manifest_checks.py (5)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/kotlin_destination_v1/build_strategy.py (1)
register_manifest_check_tools(44-46)connector_builder_mcp/build_strategies/kotlin_source_v1/build_strategy.py (1)
register_manifest_check_tools(44-46)connector_builder_mcp/build_strategies/openapi_sonar_v1/build_strategy.py (1)
register_manifest_check_tools(45-47)connector_builder_mcp/build_strategies/openapi_sonar_v1/manifest_checks.py (1)
register_manifest_check_tools(91-97)
connector_builder_mcp/build_strategies/kotlin_source_v1/guidance.py (5)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/kotlin_destination_v1/build_strategy.py (1)
register_guidance_tools(39-41)connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py (1)
register_guidance_tools(431-435)connector_builder_mcp/build_strategies/kotlin_source_v1/build_strategy.py (1)
register_guidance_tools(39-41)connector_builder_mcp/build_strategies/openapi_sonar_v1/guidance.py (1)
register_guidance_tools(201-205)
connector_builder_mcp/build_strategies/kotlin_destination_v1/manifest_checks.py (2)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/kotlin_destination_v1/build_strategy.py (1)
register_manifest_check_tools(44-46)
connector_builder_mcp/build_strategies/kotlin_source_v1/build_strategy.py (5)
connector_builder_mcp/build_strategies/base/build_strategy.py (1)
BuildStrategy(18-185)connector_builder_mcp/build_strategies/kotlin_source_v1/guidance.py (1)
register_guidance_tools(307-311)connector_builder_mcp/build_strategies/kotlin_source_v1/manifest_checks.py (1)
register_manifest_check_tools(76-82)connector_builder_mcp/build_strategies/kotlin_source_v1/manifest_tests.py (1)
register_manifest_test_tools(92-98)connector_builder_mcp/build_strategies/kotlin_source_v1/prompts.py (1)
register_prompts(92-98)
connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.py (6)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/kotlin_destination_v1/build_strategy.py (1)
register_guidance_tools(39-41)connector_builder_mcp/build_strategies/kotlin_source_v1/build_strategy.py (1)
register_guidance_tools(39-41)connector_builder_mcp/build_strategies/kotlin_source_v1/guidance.py (1)
register_guidance_tools(307-311)connector_builder_mcp/build_strategies/openapi_sonar_v1/build_strategy.py (1)
register_guidance_tools(40-42)connector_builder_mcp/build_strategies/openapi_sonar_v1/guidance.py (1)
register_guidance_tools(201-205)
⏰ 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). (3)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (Fast)
🔇 Additional comments (6)
connector_builder_mcp/build_strategies/kotlin_destination_v1/manifest_checks.py (3)
1-16: LGTM!The module structure, docstring, and imports are well-organized and follow the established patterns in the codebase.
28-73: LGTM!The tool function follows the established MCP tool pattern correctly:
- Proper decorator usage with appropriate flags for a validation tool
- Clear parameter annotations with Field descriptions
- Transparent placeholder implementation (warning added at line 64)
- Appropriate parameter validation
The placeholder implementation is expected per the PR objectives.
76-82: LGTM!The registration function correctly follows the established pattern for registering MCP tools by domain, consistent with the
_mcp_utils.pyimplementation and usage inbuild_strategy.py.connector_builder_mcp/build_strategies/kotlin_source_v1/build_strategy.py (1)
19-56: LGTM!The strategy class properly extends
BuildStrategyand follows the established pattern. All imports are at module level, registration methods correctly delegate to module-level functions, and the class structure aligns with the multi-strategy architecture.connector_builder_mcp/build_strategies/kotlin_source_v1/guidance.py (1)
1-311: Well-structured guidance module.The module follows the established pattern from other guidance modules, with consistent string delimiters throughout (no quote mismatches like in the destination variant). The topic-based documentation structure is clear and the registration function properly delegates to
register_mcp_tools.connector_builder_mcp/build_strategies/openapi_sonar_v1/manifest_checks.py (1)
91-97: LGTM!The registration function correctly delegates to the centralized
register_mcp_toolsutility with the appropriate domain. This follows the established pattern from other build strategies.
| if "x-airbyte" not in spec_content: | ||
| warnings.append( | ||
| "No x-airbyte-* extensions found. You may need to add these to define resources." | ||
| ) |
There was a problem hiding this comment.
Case-sensitive check may miss extension variants.
The check for "x-airbyte" is case-sensitive, which could miss valid variants like "X-AIRBYTE" or "X-Airbyte" in the specification.
Apply this diff to make the check case-insensitive:
- if "x-airbyte" not in spec_content:
+ if "x-airbyte" not in spec_content.lower():
warnings.append(
"No x-airbyte-* extensions found. You may need to add these to define resources."
)🤖 Prompt for AI Agents
In connector_builder_mcp/build_strategies/openapi_sonar_v1/manifest_checks.py
around lines 76 to 79, the code checks for the literal key "x-airbyte" which is
case-sensitive and will miss variants like "X-AIRBYTE" or "X-Airbyte"; replace
the single-key membership test with a case-insensitive scan over the
spec_content keys (e.g., check if any key.lower().startswith("x-airbyte") or
any("x-airbyte" in key.lower()) for key in spec_content) and append the same
warning only if no matching key is found.
- Rename openapi_sonar_v1 → declarative_openapi_v3 - Remove _v1 suffix from kotlin_source and kotlin_destination - Update class names to match new module names - Add Java 21 availability checks for Kotlin connectors - Empty stream_tasks lists for DB connectors (no stream-level tasks) - Update all imports and references in server.py Co-Authored-By: AJ Steers <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
connector_builder_mcp/build_strategies/declarative_openapi_v3/manifest_checks.py (1)
19-88: Tighten Pydantic defaults and makex-airbytecheck case‑insensitiveTwo small but worthwhile tweaks here:
- Pydantic list defaults (Lines 22‑25)
OpenApiValidationResultuseserrors: list[str] = [],warnings: list[str] = [], andresources_found: list[str] = []. In Pydantic it’s safer to useField(default_factory=list)for mutable types to avoid any shared‑mutable‑default surprises and to follow common style:-class OpenApiValidationResult(BaseModel): - """Result of OpenAPI specification validation.""" - - is_valid: bool - errors: list[str] = [] - warnings: list[str] = [] - resources_found: list[str] = [] +class OpenApiValidationResult(BaseModel): + """Result of OpenAPI specification validation.""" + + is_valid: bool + errors: list[str] = Field(default_factory=list) + warnings: list[str] = Field(default_factory=list) + resources_found: list[str] = Field(default_factory=list)
- Case‑insensitive
x-airbytedetection (Lines 76‑79)
As noted in a prior review on a sibling OpenAPI manifest‑checks module, checking for"x-airbyte"in a case‑sensitive way can miss variants like"X-AIRBYTE"or"X-Airbyte". Sincespec_contentis treated as text here, you can cheaply make this check case‑insensitive:- if "x-airbyte" not in spec_content: + if "x-airbyte" not in spec_content.lower(): warnings.append( "No x-airbyte-* extensions found. You may need to add these to define resources." )connector_builder_mcp/build_strategies/kotlin_destination/guidance.py (2)
310-358: Critical: Fix mismatched string quotes blocking module load.The "schema_mapping" topic starts with triple single quotes (
''') on line 310 but ends with triple double quotes (""") on line 358, causing a syntax error that prevents the module from loading.Apply this diff to fix:
-""", +''',
359-422: Critical: Fix mismatched string quotes blocking module load.The "error_handling" topic starts with triple double quotes (
""") on line 359 but ends with triple single quotes (''') on line 422, causing a syntax error that prevents the module from loading.Apply this diff to fix:
-''', +""",
🧹 Nitpick comments (5)
connector_builder_mcp/build_strategies/kotlin_destination/prompts.py (1)
20-101: Prompt definition is clear; verify prompt registration isn’t duplicated across strategiesThe
new_kotlin_destination_connector_promptsignature and content look good: parameters are well-documented viaField, defaults are handled safely, and the returned structure matches the MCP prompt expectations.One thing to double‑check:
register_prompts()callsregister_mcp_prompts(app, domain=ToolDomain.PROMPTS), which iterates over all globally registered PROMPTS‑domain prompts. If other strategy modules (e.g., the OpenAPI prompts module) also callregister_mcp_promptsfor the same domain, the same prompts may be registered multiple times against theFastMCPapp. Please confirm that either:
- each domain is registered only once at the server level, or
register_mcp_prompts/app.promptare idempotent with respect to prompt names.If not, you may want to centralize PROMPTS registration or adjust these
register_promptsmethods to avoid duplicate registrations.connector_builder_mcp/build_strategies/declarative_openapi_v3/prompts.py (1)
20-101: OpenAPI prompt wiring is good; ensure PROMPTS registration pattern is intentionalThe
new_openapi_connector_promptAPI and content look consistent with the rest of the OpenAPI strategy (checklist + validation tooling), and the return shape matches the MCP prompt expectations.As with the Kotlin destination prompts,
register_prompts()delegates toregister_mcp_prompts(app, domain=ToolDomain.PROMPTS), which operates over all globally registered PROMPTS‑domain prompts. If multiple modules/strategies each call this helper for the same domain, the same prompt names may be registered more than once. It would be good to confirm that:
- PROMPTS registration is only invoked once per app, or
- the underlying registration is idempotent with respect to prompt names.
If not, consider centralizing PROMPTS registration (e.g., in
server.py) rather than per‑strategy, or introducing a guard to prevent duplicate registration.connector_builder_mcp/build_strategies/declarative_openapi_v3/build_strategy.py (1)
31-37: Track the placeholder is_available() implementation.The docstring mentions checking for connector-sdk availability, but the implementation always returns
True. Consider adding a TODO or tracking issue for proper dependency detection.Example implementation checking for connector-sdk modules:
@classmethod def is_available(cls) -> bool: """Check if connector-sdk is available. Returns True if connector-sdk related modules are in sys.modules. """ - return True # For now, always available + import sys + # Check for connector-sdk or related modules + return any( + module_name.startswith("airbyte_cdk") + for module_name in sys.modules.keys() + )connector_builder_mcp/build_strategies/kotlin_source/build_strategy.py (1)
33-53: Improve Java version detection and extract duplicated code.Two concerns:
False positives: The check
"21" in version_outputcould match unintended versions like "121" or version strings containing "21" in other contexts.Code duplication: This exact implementation appears in both
KotlinSourceStrategyandKotlinDestinationStrategy.Consider using a more precise regex pattern and extracting to a shared utility:
Create a shared utility in
connector_builder_mcp/build_strategies/base/java_utils.py:import re import shutil import subprocess def is_java_21_available() -> bool: """Check if Java 21 is available. Returns True if Java 21 is installed and available. """ java_path = shutil.which("java") if not java_path: return False try: result = subprocess.run( ["java", "-version"], capture_output=True, text=True, timeout=5, ) version_output = result.stderr + result.stdout # Match version patterns like "21.0.x", "openjdk version \"21", etc. pattern = r'(openjdk\s+)?version\s+"?21\.' return bool(re.search(pattern, version_output, re.IGNORECASE)) except (subprocess.TimeoutExpired, subprocess.SubprocessError): return FalseThen update both strategies:
+from connector_builder_mcp.build_strategies.base.java_utils import is_java_21_available @classmethod def is_available(cls) -> bool: """Check if Java 21 is available. Returns True if Java 21 is installed and available. """ - java_path = shutil.which("java") - if not java_path: - return False - - try: - result = subprocess.run( - ["java", "-version"], - capture_output=True, - text=True, - timeout=5, - ) - version_output = result.stderr + result.stdout - return "21" in version_output or "openjdk 21" in version_output.lower() - except (subprocess.TimeoutExpired, subprocess.SubprocessError): - return False + return is_java_21_available()connector_builder_mcp/build_strategies/kotlin_source/manifest_tests.py (1)
31-89: Track placeholder implementation.The function correctly validates inputs but returns a placeholder success result without actually testing the connector. This is noted in the PR description but should be tracked for future implementation.
Do you want me to open a tracking issue for implementing the actual Kotlin source stream testing logic?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
connector_builder_mcp/build_strategies/declarative_openapi_v3/__init__.py(1 hunks)connector_builder_mcp/build_strategies/declarative_openapi_v3/build_strategy.py(1 hunks)connector_builder_mcp/build_strategies/declarative_openapi_v3/checklist.yaml(1 hunks)connector_builder_mcp/build_strategies/declarative_openapi_v3/guidance.py(1 hunks)connector_builder_mcp/build_strategies/declarative_openapi_v3/manifest_checks.py(1 hunks)connector_builder_mcp/build_strategies/declarative_openapi_v3/manifest_tests.py(1 hunks)connector_builder_mcp/build_strategies/declarative_openapi_v3/prompts.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_destination/__init__.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_destination/build_strategy.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_destination/checklist.yaml(1 hunks)connector_builder_mcp/build_strategies/kotlin_destination/guidance.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_destination/manifest_checks.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_destination/manifest_tests.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_destination/prompts.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_source/__init__.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_source/build_strategy.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_source/checklist.yaml(1 hunks)connector_builder_mcp/build_strategies/kotlin_source/guidance.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_source/manifest_checks.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_source/manifest_tests.py(1 hunks)connector_builder_mcp/build_strategies/kotlin_source/prompts.py(1 hunks)connector_builder_mcp/server.py(4 hunks)
✅ Files skipped from review due to trivial changes (3)
- connector_builder_mcp/build_strategies/kotlin_destination/init.py
- connector_builder_mcp/build_strategies/declarative_openapi_v3/init.py
- connector_builder_mcp/build_strategies/kotlin_source/init.py
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: aaronsteers
Repo: airbytehq/connector-builder-mcp PR: 116
File: .github/workflows/ci-tests.yml:110-113
Timestamp: 2025-10-10T17:09:02.151Z
Learning: The connector-builder-mcp repository (airbytehq/connector-builder-mcp) is an internal/private repository that doesn't accept external contributions, so there are no forked PRs to consider in workflow configurations.
📚 Learning: 2025-10-11T02:19:05.231Z
Learnt from: aaronsteers
Repo: airbytehq/connector-builder-mcp PR: 136
File: connector_builder_agents/src/evals/evaluators.py:233-245
Timestamp: 2025-10-11T02:19:05.231Z
Learning: In the connector-builder-mcp repository Python files, all imports must be placed at the module level (top of the file), never inside functions or methods. This is a recurring issue that should be flagged during code reviews.
Applied to files:
connector_builder_mcp/build_strategies/declarative_openapi_v3/prompts.py
📚 Learning: 2025-11-08T06:53:26.977Z
Learnt from: aaronsteers
Repo: airbytehq/connector-builder-mcp PR: 176
File: connector_builder_mcp/_manifest_history_utils.py:38-49
Timestamp: 2025-11-08T06:53:26.977Z
Learning: In the connector-builder-mcp repository, all imports must be at the module level (top of the file). Inline imports inside functions or methods are not allowed unless explicit permission is granted. This rule must never be violated.
Applied to files:
connector_builder_mcp/build_strategies/declarative_openapi_v3/prompts.pyconnector_builder_mcp/server.py
🧬 Code graph analysis (16)
connector_builder_mcp/build_strategies/kotlin_source/manifest_checks.py (2)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/kotlin_source/build_strategy.py (1)
register_manifest_check_tools(61-63)
connector_builder_mcp/build_strategies/kotlin_destination/guidance.py (6)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/declarative_openapi_v3/build_strategy.py (1)
register_guidance_tools(40-42)connector_builder_mcp/build_strategies/declarative_openapi_v3/guidance.py (1)
register_guidance_tools(201-205)connector_builder_mcp/build_strategies/kotlin_destination/build_strategy.py (1)
register_guidance_tools(56-58)connector_builder_mcp/build_strategies/kotlin_source/build_strategy.py (1)
register_guidance_tools(56-58)connector_builder_mcp/build_strategies/kotlin_source/guidance.py (1)
register_guidance_tools(307-311)
connector_builder_mcp/build_strategies/declarative_openapi_v3/build_strategy.py (5)
connector_builder_mcp/build_strategies/base/build_strategy.py (1)
BuildStrategy(18-185)connector_builder_mcp/build_strategies/declarative_openapi_v3/guidance.py (1)
register_guidance_tools(201-205)connector_builder_mcp/build_strategies/declarative_openapi_v3/manifest_checks.py (1)
register_manifest_check_tools(91-97)connector_builder_mcp/build_strategies/declarative_openapi_v3/manifest_tests.py (1)
register_manifest_test_tools(94-100)connector_builder_mcp/build_strategies/declarative_openapi_v3/prompts.py (1)
register_prompts(95-101)
connector_builder_mcp/build_strategies/kotlin_destination/manifest_checks.py (5)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/declarative_openapi_v3/build_strategy.py (1)
register_manifest_check_tools(45-47)connector_builder_mcp/build_strategies/kotlin_destination/build_strategy.py (1)
register_manifest_check_tools(61-63)connector_builder_mcp/build_strategies/kotlin_source/build_strategy.py (1)
register_manifest_check_tools(61-63)connector_builder_mcp/build_strategies/kotlin_source/manifest_checks.py (1)
register_manifest_check_tools(76-82)
connector_builder_mcp/build_strategies/kotlin_source/prompts.py (2)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_prompt(152-182)register_mcp_prompts(282-308)connector_builder_mcp/build_strategies/kotlin_source/build_strategy.py (1)
register_prompts(71-73)
connector_builder_mcp/build_strategies/declarative_openapi_v3/prompts.py (2)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_prompt(152-182)register_mcp_prompts(282-308)connector_builder_mcp/build_strategies/declarative_openapi_v3/build_strategy.py (1)
register_prompts(55-57)
connector_builder_mcp/build_strategies/declarative_openapi_v3/manifest_checks.py (1)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)
connector_builder_mcp/build_strategies/kotlin_source/build_strategy.py (3)
connector_builder_mcp/build_strategies/base/build_strategy.py (1)
BuildStrategy(18-185)connector_builder_mcp/build_strategies/kotlin_destination/build_strategy.py (5)
is_available(34-53)register_guidance_tools(56-58)register_manifest_check_tools(61-63)register_manifest_test_tools(66-68)register_prompts(71-73)connector_builder_mcp/build_strategies/kotlin_source/guidance.py (1)
register_guidance_tools(307-311)
connector_builder_mcp/build_strategies/kotlin_source/guidance.py (3)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/declarative_openapi_v3/build_strategy.py (1)
register_guidance_tools(40-42)connector_builder_mcp/build_strategies/kotlin_source/build_strategy.py (1)
register_guidance_tools(56-58)
connector_builder_mcp/build_strategies/declarative_openapi_v3/guidance.py (6)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/declarative_openapi_v3/build_strategy.py (1)
register_guidance_tools(40-42)connector_builder_mcp/build_strategies/kotlin_destination/build_strategy.py (1)
register_guidance_tools(56-58)connector_builder_mcp/build_strategies/kotlin_destination/guidance.py (1)
register_guidance_tools(431-435)connector_builder_mcp/build_strategies/kotlin_source/build_strategy.py (1)
register_guidance_tools(56-58)connector_builder_mcp/build_strategies/kotlin_source/guidance.py (1)
register_guidance_tools(307-311)
connector_builder_mcp/build_strategies/kotlin_destination/prompts.py (2)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_prompt(152-182)register_mcp_prompts(282-308)connector_builder_mcp/build_strategies/kotlin_destination/build_strategy.py (1)
register_prompts(71-73)
connector_builder_mcp/server.py (5)
connector_builder_mcp/build_strategies/declarative_openapi_v3/build_strategy.py (2)
DeclarativeOpenApiV3Strategy(20-57)is_available(32-37)connector_builder_mcp/build_strategies/declarative_yaml_v1/build_strategy.py (2)
DeclarativeYamlV1Strategy(22-61)is_available(35-41)connector_builder_mcp/build_strategies/kotlin_destination/build_strategy.py (2)
KotlinDestinationStrategy(22-73)is_available(34-53)connector_builder_mcp/build_strategies/kotlin_source/build_strategy.py (2)
KotlinSourceStrategy(22-73)is_available(34-53)connector_builder_mcp/build_strategies/base/build_strategy.py (2)
is_available(52-61)register_all_mcp_callables(107-120)
connector_builder_mcp/build_strategies/kotlin_destination/build_strategy.py (6)
connector_builder_mcp/build_strategies/base/build_strategy.py (1)
BuildStrategy(18-185)connector_builder_mcp/build_strategies/kotlin_source/build_strategy.py (5)
is_available(34-53)register_guidance_tools(56-58)register_manifest_check_tools(61-63)register_manifest_test_tools(66-68)register_prompts(71-73)connector_builder_mcp/build_strategies/kotlin_destination/guidance.py (1)
register_guidance_tools(431-435)connector_builder_mcp/build_strategies/kotlin_destination/manifest_checks.py (1)
register_manifest_check_tools(76-82)connector_builder_mcp/build_strategies/kotlin_destination/manifest_tests.py (1)
register_manifest_test_tools(95-101)connector_builder_mcp/build_strategies/kotlin_destination/prompts.py (1)
register_prompts(93-99)
connector_builder_mcp/build_strategies/kotlin_destination/manifest_tests.py (2)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/kotlin_destination/build_strategy.py (1)
register_manifest_test_tools(66-68)
connector_builder_mcp/build_strategies/declarative_openapi_v3/manifest_tests.py (2)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/declarative_openapi_v3/build_strategy.py (1)
register_manifest_test_tools(50-52)
connector_builder_mcp/build_strategies/kotlin_source/manifest_tests.py (2)
connector_builder_mcp/mcp/_mcp_utils.py (3)
ToolDomain(49-80)mcp_tool(102-149)register_mcp_tools(246-279)connector_builder_mcp/build_strategies/declarative_openapi_v3/manifest_tests.py (1)
register_manifest_test_tools(94-100)
⏰ 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). (3)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (Fast)
🔇 Additional comments (16)
connector_builder_mcp/build_strategies/kotlin_destination/checklist.yaml (1)
1-138: Kotlin destination checklist content and structure look solidThe checklist is well-structured (phased tasks, explicit readiness and finalization steps) and consistent with the broader MCP workflows (checklists, readiness checks, secrets guidance). I don’t see structural or logical issues in this YAML as written.
connector_builder_mcp/build_strategies/declarative_openapi_v3/checklist.yaml (1)
1-153: Declarative OpenAPI v3 checklist is consistent and actionableThe task breakdown (spec discovery/validation, resource enumeration, auth, special requirements, readiness/finalization) aligns well with an OpenAPI-based workflow and mirrors patterns used in other strategies. No structural or semantic issues stand out in this YAML.
connector_builder_mcp/build_strategies/declarative_openapi_v3/manifest_tests.py (1)
85-91: LGTM: Placeholder implementation noted.The placeholder implementation is acknowledged in the PR objectives and appropriately returns a success result with empty data. This provides a working tool signature that can be implemented fully in future PRs.
connector_builder_mcp/build_strategies/kotlin_destination/manifest_tests.py (1)
87-92: LGTM: Placeholder implementation with appropriate validation.The tool correctly validates all required inputs before returning a placeholder success result. The implementation structure is ready for the full testing logic to be added in future PRs.
connector_builder_mcp/server.py (1)
65-78: LGTM: Clean multi-strategy registration.The collection-driven approach cleanly handles multiple strategies with availability checking. The logging of unavailable strategies (line 78) is helpful for debugging environment issues (e.g., missing Java 21 for Kotlin strategies).
connector_builder_mcp/build_strategies/kotlin_source/checklist.yaml (1)
1-131: LGTM: Comprehensive checklist structure.The checklist provides clear, phased guidance for Kotlin source connector development. The 📝 markers effectively indicate outputs to share with users, and the task flow logically progresses through the development lifecycle.
connector_builder_mcp/build_strategies/kotlin_destination/manifest_checks.py (1)
60-73: LGTM: Validation tool structure ready for implementation.The placeholder warning (line 64) appropriately indicates the current state. The validation result model and tool signature provide a solid foundation for adding actual validation logic in future PRs.
connector_builder_mcp/build_strategies/kotlin_destination/guidance.py (1)
1-435: Approve structure once syntax errors are fixed.The guidance tool structure and documentation content are well-organized and comprehensive. However, the two critical syntax errors (lines 310-358 and 359-422) must be fixed before this file can load successfully.
connector_builder_mcp/build_strategies/declarative_openapi_v3/guidance.py (1)
1-205: LGTM: Clean guidance tool implementation.The documentation structure is clear and comprehensive. All triple-quote delimiters match correctly, avoiding the syntax issues seen in the Kotlin destination guidance module.
connector_builder_mcp/build_strategies/kotlin_source/manifest_checks.py (1)
1-82: LGTM: Consistent validation tool pattern.The structure mirrors the Kotlin destination manifest checks module, maintaining consistency across strategies. The placeholder implementation is appropriately flagged with a warning message.
connector_builder_mcp/build_strategies/declarative_openapi_v3/build_strategy.py (1)
39-57: LGTM!Registration methods correctly delegate to their respective modules and follow the BuildStrategy pattern.
connector_builder_mcp/build_strategies/kotlin_source/guidance.py (1)
19-304: LGTM!The guidance tool is well-structured with comprehensive documentation covering all key aspects of Kotlin source connector development. The topic-based navigation and fallback for unknown topics are well-implemented.
connector_builder_mcp/build_strategies/kotlin_source/build_strategy.py (1)
55-73: LGTM!Registration methods correctly delegate to their respective modules and follow the BuildStrategy pattern.
connector_builder_mcp/build_strategies/kotlin_source/prompts.py (1)
1-98: LGTM!The prompts module is well-implemented with comprehensive guidance for building Kotlin source connectors. The parameter handling and registration are correct.
connector_builder_mcp/build_strategies/kotlin_source/manifest_tests.py (1)
19-28: LGTM!The result model is well-structured with appropriate field types and descriptions.
connector_builder_mcp/build_strategies/kotlin_destination/build_strategy.py (1)
55-73: LGTM!Registration methods correctly delegate to their respective kotlin_destination modules and follow the BuildStrategy pattern.
| @classmethod | ||
| def is_available(cls) -> bool: | ||
| """Check if Java 21 is available. | ||
|
|
||
| Returns True if Java 21 is installed and available. | ||
| """ | ||
| java_path = shutil.which("java") | ||
| if not java_path: | ||
| return False | ||
|
|
||
| try: | ||
| result = subprocess.run( | ||
| ["java", "-version"], | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=5, | ||
| ) | ||
| version_output = result.stderr + result.stdout | ||
| return "21" in version_output or "openjdk 21" in version_output.lower() | ||
| except (subprocess.TimeoutExpired, subprocess.SubprocessError): | ||
| return False |
There was a problem hiding this comment.
Duplicate implementation - see KotlinSourceStrategy.
This is_available() method is identical to the one in kotlin_source/build_strategy.py (lines 33-53). Both the false positive risk in Java version detection and the code duplication issue were flagged in that file's review. Please apply the same refactoring to extract this into a shared utility.
aaronsteers
deleted the
devin/1763589811-add-build-strategy-submodules
branch
Summary
This PR adds three new build strategy submodules to the connector-builder-mcp server, expanding support beyond the existing declarative YAML strategy:
Each strategy follows the established
BuildStrategypattern and includes:The server.py has been updated to register all 4 strategies (including the existing DeclarativeYamlV1Strategy) with availability checking.
connector_builder_mcp/build_strategies/kotlin_destination_v1/guidance.pyat line 186 that needs to be fixed before this PR can be merged. The error appears to be related to the markdown documentation structure but the root cause is unclear.Review & Testing Checklist for Human
Risk Level: 🔴 RED - Contains known syntax error that breaks the build
CRITICAL: Fix syntax error in kotlin_destination_v1/guidance.py - The file has a Python syntax error at line 186. The dictionary structure appears correct but Python reports "Perhaps you forgot a comma?". This needs to be debugged and fixed before merge.
Verify server starts correctly - Run
poe serverorpoe mcp-serve-localto ensure the server starts and all 4 strategies register without errors. Check stderr output for the strategy registration messages.Review placeholder implementations - The validation and testing tools in all 3 new strategies are placeholder implementations that return success but don't actually validate or test anything. Confirm this is acceptable for initial release or if real implementations are needed.
Test checklist YAML loading - Verify that all 3 new checklist.yaml files load correctly using the
get_connector_builder_checklist()tool with each strategy.Review documentation accuracy - The guidance.py files contain extensive Kotlin code examples and OpenAPI documentation. Review for technical accuracy, especially the Kotlin code snippets which use
$for string interpolation.Recommended Test Plan
poe format && poe lint-fixto ensure code qualitypoe testto verify existing tests still passget_connector_builder_docs()for each new strategy to ensure documentation loadsget_connector_builder_checklist()to ensure checklists load correctlyNotes
The syntax error needs to be resolved before this can be merged. Once fixed, the PR adds significant new functionality to support multiple connector development workflows.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.