feat(py/genkit): complete DAP integration with registry#4459
Conversation
github-actions
bot
added
docs
Summary of ChangesHello @yesudeep, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new Python sample for Dynamic Action Providers (DAPs), a feature that allows tools to be provided dynamically at runtime rather than being registered at startup. The sample serves as a practical reference for users to understand and implement DAPs, demonstrating key concepts such as dynamic tool creation, caching with Time-To-Live (TTL), and cache invalidation. It also updates the project roadmap to reflect the current parity status of the Python DAP implementation. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a comprehensive and well-documented sample for Dynamic Action Providers (DAPs) in Python, which is a great addition. The sample effectively demonstrates core DAP concepts like dynamic tool creation, caching with TTL, and cache invalidation. My review includes suggestions to make the sample even more robust by demonstrating how a model can use tools from DAPs, improving efficiency with concurrent fetching, and adhering to standard Python packaging conventions.
yesudeep
force-pushed
the
yesudeep/feat/agonizing-knot
branch
3 times, most recently
from
1335313 to
f251fe8
Compare
yesudeep
force-pushed
the
yesudeep/feat/agonizing-knot
branch
from
f251fe8 to
f91e4ed
Compare
yesudeep
force-pushed
the
yesudeep/feat/agonizing-knot
branch
3 times, most recently
from
088fb0b to
0bd2149
Compare
yesudeep
changed the title
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a significant feature implementation, bringing the Python SDK's Dynamic Action Provider (DAP) functionality to full parity with the JavaScript implementation. The changes are extensive, touching the core registry, the DAP block, and adding comprehensive tests and a new sample. The code quality is high, with clear logic and good documentation. The new DAP sample is particularly well-done, providing clear explanations and examples. I have one minor comment regarding an import placement in the new sample that violates the project's coding standards.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a significant and well-executed effort to bring the Python SDK's Dynamic Action Provider (DAP) implementation to full parity with the JavaScript version. The changes are extensive, touching core registry logic, the DAP implementation itself, and adding comprehensive tests and documentation. The new dap-demo sample is an excellent addition that clearly demonstrates the new functionality. I've identified one potential bug in the registry's de-duplication logic for DAP-provided actions that could lead to missing actions in the DevUI, and I've suggested a fix along with a necessary update to a new test case. Overall, this is a high-quality contribution that greatly enhances the Python SDK.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request successfully completes the Dynamic Action Provider (DAP) integration for the Python SDK, bringing it to full feature parity with the JavaScript implementation. The changes are comprehensive, including significant enhancements to registry.py for DAP key parsing and action resolution, and a major refactoring of dap.py to align with the JS version's caching and action signature patterns. The addition of the dap-demo sample is excellent, providing a clear and well-documented example of the new capabilities. The test suite has also been appropriately expanded. Overall, this is a solid contribution. I have one minor suggestion to improve the logic in the new sample.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a significant feature contribution that completes the Dynamic Action Provider (DAP) integration for the Python SDK, bringing it to parity with the JavaScript implementation. The changes are extensive and well-executed, including enhancements to the registry for DAP key parsing and resolution, updates to the core DAP logic for caching and action definition, and comprehensive documentation updates. A new dap-demo sample provides an excellent demonstration of the new functionality, and the test suite has been expanded to ensure correctness. My review identified a minor issue in how action metadata is constructed, where action.metadata could unintentionally overwrite the action's name and description. I've provided suggestions to correct this behavior in the three places this pattern occurs.
This sample demonstrates Dynamic Action Provider (DAP) usage including: - Multiple DAPs with different caching strategies (weather and finance) - Dynamic tool creation via ai.dynamic_tool() - Cache invalidation - Tool listing from DAP cache The Python DAP implementation was already complete and at parity with the JS canonical implementation. This sample provides a reference for users learning to use DAPs. Flows available in DevUI: - weather_assistant: Get weather using dynamic tools - finance_assistant: Answer finance queries - multi_assistant: Combine tools from multiple DAPs - refresh_tools_demo: Cache invalidation demo - list_dap_tools: List available tools Includes ELI5 documentation, data flow diagrams, and testing instructions per GEMINI.md guidelines.
Updates the Python DAP implementation to match the latest JavaScript changes from PR #4050: - DAP action takes no input and returns list[ActionMetadata] - Cache is created before action with set_dap/set_value pattern - transform_dap_value returns flat list instead of grouped dict - Metadata now includes name/description fields explicitly Also updates tests to verify the new API behavior.
- Restructure package: move main.py to src/dap_demo/__init__.py - Update pyproject.toml packages to use proper package directory - Update run.sh to use module execution (-m dap_demo) - Use asyncio.gather for concurrent DAP cache fetches - Improve flows to demonstrate direct tool invocation - Add heuristic-based query routing in finance and multi assistants Note: Dynamic tools from DAPs are not in the global registry, so they must be invoked directly rather than passed to ai.generate() by name.
Fixes potential name collision bug where two different DAPs providing actions with the same short name would cause one to be omitted.
Adds test_runs_action_with_transformed_metadata and test_skips_trace_when_requested to match JS test parity. Now at 100% test coverage parity with JS.
Reverse word order to correctly extract 'AAPL' instead of 'THE' from queries like 'What's the current price of AAPL stock?'
Properly fixes circular import by defining DynamicActionProviderProtocol that describes the interface needed by Registry. This is cleaner than TYPE_CHECKING as it provides runtime type safety through duck typing.
…tion Apply metadata dict first, then set name and description to ensure they cannot be accidentally overwritten by metadata fields.
- Add action.kind to metadata in all three places for DevUI consistency
- Use split(':', 1) to handle colons in action names
- Use ActionKind.DYNAMIC_ACTION_PROVIDER enum instead of string literal
DRY refactor - _create_action_metadata() helper now used in all three places that build action metadata dicts.
Preserves inputSchema, outputSchema, and other metadata fields from DAP-provided actions instead of only name, kind, description.
- registry.py: Include inputSchema/outputSchema in static action metadata for DevUI compatibility using model_validate - dap-demo: Use isalnum() for stock ticker extraction to support tickers with numbers like '7CD'
- dap.py: Include inputSchema and outputSchema in _create_action_metadata so dynamic actions display schemas in DevUI - registry.py: Clarify DAP key format in resolve_action_names docstring with full path prefix example
Use regex to clean punctuation from words before checking for stock tickers. Handles cases like 'GOOG?' correctly and adds minimum length check to avoid matching common words like 'OF'.
yesudeep
force-pushed
the
yesudeep/feat/agonizing-knot
branch
from
f0ed167 to
3e9711c
Compare
…base#4459)" This reverts commit c541f48.
Cross-checked all markdown files in py/ against the codebase and open PRs. Fixed outdated content across 9 files. engdoc/index.md: - Fix Python version: 3.12+ → 3.10+ - Update feature parity table (6 of 7 features now ✅, Agents still ❌) - Replace 8-plugin table with full 23-plugin parity table - Rewrite all 6 Python code examples (generation, structured output, tool calling, chat, agents, data retrieval) with correct imports, Genkit() class API, and @ai.tool() decorator pattern engdoc/extending/api.md: - Replace stale Sync/Async design section (GenkitExperimental/SyncGenkit/ AsyncGenkit never implemented) with actual async-first architecture documenting GenkitRegistry → GenkitBase → Genkit hierarchy engdoc/extending/index.md: - Update d2 diagram plugin list from 7 to 22 plugins engdoc/extending/servers.md: - Fill Python TODO links with actual file paths (flows.py, reflection.py) engdoc/user_guide/python/publishing_pypi.md: - Add ReleaseKit as primary publishing mechanism - Demote manual workflow to "Legacy" section GEMINI.md: - Remove 7 dangling references to deleted files (engdoc/planning/, blog-genkit-python-*.md, release-publishing-guide.md) - Update blog article guidelines from mandatory to optional - Remove stale validation script checking deleted paths .github/PR_RELEASE.md: - Remove dangling reference to deleted blog-genkit-python-0.5.0.md PARITY_AUDIT.md: - G7: ✅ Done → ⬜ Reverted (#4459 reverted by #4469, needs re-land) - §8c.3/§8c.4: Update stale text — X-Genkit-Span-Id IS now sent (#4511) - §1d: genkitx-cohere ❌ → ✅ (in-tree cohere plugin exists) - §6c: Community coverage 3/6 → 4/6 - G17: 🔄 draft → ⬜ (#4521 closed, needs new PR) - G3/G12-G16/G4: Note #4510 is closed, needs new PR after G38 - G2→G1: Mark as superseded (#4516 titled [SUPERSEDED])
Cross-checked all markdown files in py/ against the codebase and open PRs. Fixed outdated content across 9 files. engdoc/index.md: - Fix Python version: 3.12+ → 3.10+ - Update feature parity table (6 of 7 features now ✅, Agents still ❌) - Replace 8-plugin table with full 23-plugin parity table - Rewrite all 6 Python code examples (generation, structured output, tool calling, chat, agents, data retrieval) with correct imports, Genkit() class API, and @ai.tool() decorator pattern engdoc/extending/api.md: - Replace stale Sync/Async design section (GenkitExperimental/SyncGenkit/ AsyncGenkit never implemented) with actual async-first architecture documenting GenkitRegistry → GenkitBase → Genkit hierarchy engdoc/extending/index.md: - Update d2 diagram plugin list from 7 to 22 plugins engdoc/extending/servers.md: - Fill Python TODO links with actual file paths (flows.py, reflection.py) engdoc/user_guide/python/publishing_pypi.md: - Add ReleaseKit as primary publishing mechanism - Demote manual workflow to "Legacy" section GEMINI.md: - Remove 7 dangling references to deleted files (engdoc/planning/, blog-genkit-python-*.md, release-publishing-guide.md) - Update blog article guidelines from mandatory to optional - Remove stale validation script checking deleted paths .github/PR_RELEASE.md: - Remove dangling reference to deleted blog-genkit-python-0.5.0.md PARITY_AUDIT.md: - G7: ✅ Done → ⬜ Reverted (#4459 reverted by #4469, needs re-land) - §8c.3/§8c.4: Update stale text — X-Genkit-Span-Id IS now sent (#4511) - §1d: genkitx-cohere ❌ → ✅ (in-tree cohere plugin exists) - §6c: Community coverage 3/6 → 4/6 - G17: 🔄 draft → ⬜ (#4521 closed, needs new PR) - G3/G12-G16/G4: Note #4510 is closed, needs new PR after G38 - G2→G1: Mark as superseded (#4516 titled [SUPERSEDED])
Cross-checked all markdown files in py/ against the codebase and open PRs. Fixed outdated content across 9 files. engdoc/index.md: - Fix Python version: 3.12+ → 3.10+ - Update feature parity table (6 of 7 features now ✅, Agents still ❌) - Replace 8-plugin table with full 23-plugin parity table - Rewrite all 6 Python code examples (generation, structured output, tool calling, chat, agents, data retrieval) with correct imports, Genkit() class API, and @ai.tool() decorator pattern engdoc/extending/api.md: - Replace stale Sync/Async design section (GenkitExperimental/SyncGenkit/ AsyncGenkit never implemented) with actual async-first architecture documenting GenkitRegistry → GenkitBase → Genkit hierarchy engdoc/extending/index.md: - Update d2 diagram plugin list from 7 to 22 plugins engdoc/extending/servers.md: - Fill Python TODO links with actual file paths (flows.py, reflection.py) engdoc/user_guide/python/publishing_pypi.md: - Add ReleaseKit as primary publishing mechanism - Demote manual workflow to "Legacy" section GEMINI.md: - Remove 7 dangling references to deleted files (engdoc/planning/, blog-genkit-python-*.md, release-publishing-guide.md) - Update blog article guidelines from mandatory to optional - Remove stale validation script checking deleted paths .github/PR_RELEASE.md: - Remove dangling reference to deleted blog-genkit-python-0.5.0.md PARITY_AUDIT.md: - G7: ✅ Done → ⬜ Reverted (#4459 reverted by #4469, needs re-land) - §8c.3/§8c.4: Update stale text — X-Genkit-Span-Id IS now sent (#4511) - §1d: genkitx-cohere ❌ → ✅ (in-tree cohere plugin exists) - §6c: Community coverage 3/6 → 4/6 - G17: 🔄 draft → ⬜ (#4521 closed, needs new PR) - G3/G12-G16/G4: Note #4510 is closed, needs new PR after G38 - G2→G1: Mark as superseded (#4516 titled [SUPERSEDED])
3 participants
Summary
Complete Dynamic Action Provider (DAP) integration for the Python SDK, achieving 100% parity with the JavaScript implementation.
Changes
Registry Enhancements
parse_registry_key()- Parses DAP-style keys like/dynamic-action-provider/mcp-host:tool/mytoolresolve_action_names()- Expands wildcard keys via DAPs (e.g.,tool/*)list_resolvable_actions()- Lists all actions including DAP-provided ones for DevUIis_action_type()- Helper function to validate action type stringsDynamicActionProviderProtocol- Protocol for DAP interface (fixes circular import properly)DevUI Integration
reflection.pyto uselist_resolvable_actions()so DevUI displays DAP actionsCode Quality
TYPE_CHECKINGwith Protocol-based type hints (proper circular import fix)Documentation
Feature Parity
parse_registry_key()resolve_action_names()list_resolvable_actions()is_action_type()Testing
Review Comments Addressed