feat(plugin-api): export the interactive prompt content models - #2145
feat(plugin-api): export the interactive prompt content models#2145DABH wants to merge 3 commits into
Conversation
The plugin-api facade exports the interactive response models and InteractionPrompt, but not the concrete prompt content models behind the HumanPrompt union. Plugin authors building interaction flows construct prompts, not just responses, and today must import them from nat.data_models.interactive in violation of the import-only-from-plugin_api contract. Export HumanPromptText, HumanPromptNotification, HumanPromptBinary, HumanPromptRadio, HumanPromptCheckbox, and HumanPromptDropdown, together with the BinaryHumanPromptOption and MultipleChoiceOption models that binary and multiple-choice prompts and responses embed. Pin the new symbols in the expected-exports test, extend the denied-import patterns, add a consumer-style test constructing every prompt variant through facade-only imports, and document the new surface in docs/source/extend/plugin-api.md. Signed-off-by: David Hyde <DABH@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe public plugin API now exports concrete HITL prompt and option models. Documentation lists the new models. Tests verify imports, documentation references, prompt construction, and serialization preservation. ChangesInteractive prompt API
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant PluginAPITest
participant nat.plugin_api
participant InteractionPrompt
PluginAPITest->>nat.plugin_api: import concrete prompt and option models
PluginAPITest->>InteractionPrompt: construct prompt with concrete content
InteractionPrompt-->>PluginAPITest: serialize and reparse prompt
PluginAPITest->>PluginAPITest: verify concrete content types and model data
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/nvidia_nat_core/tests/nat/test_plugin_api.py (1)
593-601: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert all prompt fields after the round trip.
The loop checks only the concrete type and
text. A serialization regression can dropplaceholder,required, or embedded option values and still pass.Compare the complete serialized content.
Proposed fix
parsed = InteractionPrompt.model_validate(prompt.model_dump()) assert type(parsed.content) is type(content) assert parsed.content.text == content.text + assert parsed.content.model_dump() == content.model_dump()Confirm this assertion with the repository's Pydantic 2.11.0 test configuration.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nvidia_nat_core/tests/nat/test_plugin_api.py` around lines 593 - 601, Update the round-trip assertions in the InteractionPrompt loop to compare the complete serialized content rather than only its type and text. Serialize both parsed.content and the original content with the repository’s Pydantic 2.11.0 configuration, preserving validation of all fields including placeholder, required, and embedded option values.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/nvidia_nat_core/tests/nat/test_plugin_api.py`:
- Around line 387-392: Extend the denied_patterns list in the plugin API test to
include all six concrete HumanPrompt model symbols newly added to
EXPECTED_PLUGIN_API_EXPORTS, including HumanPromptText, so direct imports from
nat.data_models.interactive are rejected consistently with the existing
option-model entries.
---
Nitpick comments:
In `@packages/nvidia_nat_core/tests/nat/test_plugin_api.py`:
- Around line 593-601: Update the round-trip assertions in the InteractionPrompt
loop to compare the complete serialized content rather than only its type and
text. Serialize both parsed.content and the original content with the
repository’s Pydantic 2.11.0 configuration, preserving validation of all fields
including placeholder, required, and embedded option values.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c1cfeb1e-1eaa-4470-ad5b-c33029a5e3de
📒 Files selected for processing (3)
docs/source/extend/plugin-api.mdpackages/nvidia_nat_core/src/nat/plugin_api/__init__.pypackages/nvidia_nat_core/tests/nat/test_plugin_api.py
Address review feedback on the prompt content model exports: - List all six concrete HumanPrompt models explicitly in the denied-import patterns so coverage no longer depends on the HumanPrompt entry being a substring prefix of each model name. - Compare the full serialized prompt content after the InteractionPrompt round trip so a regression that drops placeholder, required, or embedded option values fails the test instead of passing on type and text alone. Signed-off-by: David Hyde <DABH@users.noreply.github.com>
Description
Third-party plugin packages are asked to import plugin-authoring symbols only from the stable
nat.plugin_apifacade (perdocs/source/extend/third-party-plugins.md). PR #2113 exported the interactive response side of that surface:InteractionPrompt,InteractionResponse, and theHumanResponseunion with its concrete response models. The prompt side is still missing: the concrete prompt content models behind theHumanPromptunion are only importable from the implementation modulenat.data_models.interactive.Plugin authors do not only consume responses. HITL middleware hooks, user-input callbacks, and front-end integrations construct prompts: any plugin that calls
Context.get().user_interaction_manager.prompt_user_input(...)first has to build aHumanPromptText(or a notification, binary, radio, checkbox, or dropdown variant). The in-repo HITL examples importHumanPromptTextfromnat.data_models.interactive, and the interactive workflows guide constructs it while pointing readers at that same internal module path — exactly the dependency an external package following the facade contract is told not to take. In addition, the binary and multiple-choice prompt models embed required option models, and those same option models are the field types of the already-exportedHumanResponseBinary,HumanResponseRadio,HumanResponseCheckbox, andHumanResponseDropdown(selected_option), so typed construction of the exported response models also needs them.This change is purely additive and completes the prompt-and-response symmetry of the interactive surface:
HumanPromptText,HumanPromptNotification,HumanPromptBinary,HumanPromptRadio,HumanPromptCheckbox, andHumanPromptDropdown, together with the embedded option modelsBinaryHumanPromptOptionandMultipleChoiceOption, throughnat.plugin_api, and extend__all__.EXPECTED_PLUGIN_API_EXPORTSand extend the denied-import patterns in the plugin-authoring docs test (the existingHumanPromptpattern already covers theHumanPrompt*model imports as substrings, so only the two option-model patterns are new).nat.plugin_apiimports alone and round-trips each one throughInteractionPromptto check the discriminated union resolves the concrete types.docs/source/extend/plugin-api.md, extending the interactive-models bullet and the "HITL middleware" surface-review row (the models follow that row's existing provisional public, trusted plugin status).Deliberately not exported: the OAuth consent prompt variant stays private (it is underscore-prefixed by design), and the abstract bases (
HumanPromptBase,HumanPromptMultipleChoiceBase) stay internal, mirroring howInteractionBaseis not exported; the already-exportedHumanPromptunion covers the "any prompt" typing need.Migrating the in-repo consumers (the two HITL examples and the interactive workflows guide) to the facade imports is deliberately left as a follow-up so this change stays purely additive. No tracking issue exists for this yet; happy to file one — also covering that migration — if the team prefers.
Testing
uv run pytest packages/nvidia_nat_core/tests/nat/test_plugin_api.py— 8 passed (includes the new consumer-style prompt-construction test).uv run pytest packages/nvidia_nat_core/tests/nat/builder packages/nvidia_nat_core/tests/nat/middleware packages/nvidia_nat_core/tests/nat/runtime packages/nvidia_nat_core/tests/nat/data_models— 852 passed.uv run pre-commit run yapf --files <touched files>anduv run pre-commit run ruff-check --files <touched files>— passed.uv run python ci/scripts/copyright.py --verify-apache-v2— passed.uv run python ci/scripts/path_checks.py— passed.uv run vale docs/source/extend/plugin-api.md— 0 errors, 0 warnings, 0 suggestions.By Submitting this PR I confirm:
Summary by CodeRabbit
New Features
Documentation
Tests