Data entry flow previews#3081
Open
iluvdata wants to merge 6 commits intohome-assistant:masterfrom
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds an "Entity previews" section to data entry flow docs describing a websocket preview command, runtime preview entity pattern, registration hook, and how to enable previews from ChangesEntity Previews Documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
docs/data_entry_flow_index.md (1)
603-619: ⚡ Quick winAdd missing imports/definitions or make them explicit placeholders.
Several identifiers used in the Step 1 snippet are not defined/imported within the snippet:
HomeAssistant,Any,MappingcallbackdecoratorConfigFlow,FlowType,SubentryFlowResult,ConfigEntryimports shown partially, but types used in function signature still rely on other symbolsPreviewSensorEntitydepends onSensorEntity,CONF_DEVICE_CLASS, etc. (covered in Step 2, but Step 1 calls it)If the intention is “illustrative pseudocode”, it should say so explicitly; otherwise readers will copy-paste and fail at runtime.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/data_entry_flow_index.md` around lines 603 - 619, The snippet is missing several type and decorator definitions used in function signatures and classes; either add explicit imports (e.g., import HomeAssistant from homeassistant.core, Any/Mapping from typing, callback from homeassistant.core or homeassistant.helpers.event, SensorEntity and CONF_DEVICE_CLASS from their respective Home Assistant modules) and ensure ConfigFlow/FlowType/SubentryFlowResult/ConfigEntry are fully imported where referenced, or mark the whole block as illustrative pseudocode with a comment; update references to PreviewSensorEntity to show it subclasses SensorEntity and uses CONF_DEVICE_CLASS so readers know those dependencies must be imported or stubbed before copying.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/data_entry_flow_index.md`:
- Around line 637-648: The docstring in async_preview_callback incorrectly
references _calculate_state; update the comment text to reference the async
method name _async_calculate_state (the method implemented/called by the Step 2
class) so readers implement the correct method; locate the doc comment inside
async_preview_callback and replace any mention of _calculate_state with
_async_calculate_state and ensure any explanatory text reflects that it is the
async method used for preview/state calculation.
- Around line 676-683: The constructor call for PreviewSensorEntity is
inconsistent with its definition and uses undefined variables: align
PreviewSensorEntity.__init__ and its instantiation by either updating the class
signature to def __init__(self, hass, value: str, config: dict[str, str]) ->
None or change the instantiation to pass the parameters the current __init__
expects; additionally, ensure value and config are defined before use (e.g.,
introduce explicit placeholder variables like value = "<preview_value>" and
config = {"key": "value"} in Step 1) so the call to PreviewSensorEntity(...) and
the async_show_preview subscription use valid, named variables.
- Around line 705-734: The async_show_preview callback has an off-by-one in
error truncation and an inconsistent callback parameter comment: in the except
block for async_show_preview (which calls _async_calculate_state and
preview_callback with CalculatedState values and self.domain), change the
truncation from error[:254] to error[:255] so errors capped at 255 chars are
preserved correctly, and update the preview_callback type comment from "errors"
to "error" to match the variable name used throughout; keep the existing
condition (if len(error) > 255) and ensure the final preview_callback(None,
None, error, None) remains unchanged.
- Around line 628-675: The snippet uses undefined variables cur_step and errors;
make the example self-contained by explicitly defining placeholders and showing
intent: add a cur_step placeholder (e.g., cur_step = flow_status["step"] or
cur_step = {"handler": ("<entry_id>", "<subflow>")} before using cur_step so
entry_id, _ = cur_step["handler"] resolves, and add an errors variable (e.g.,
errors: Mapping[str,str] | None = None or set errors from your validation logic)
before the if errors is not None check; include short comments indicating these
are placeholders to be replaced with real flow step lookup and validation
results (refer to symbols cur_step, flow_status, entry_id, errors,
async_preview_callback).
---
Nitpick comments:
In `@docs/data_entry_flow_index.md`:
- Around line 603-619: The snippet is missing several type and decorator
definitions used in function signatures and classes; either add explicit imports
(e.g., import HomeAssistant from homeassistant.core, Any/Mapping from typing,
callback from homeassistant.core or homeassistant.helpers.event, SensorEntity
and CONF_DEVICE_CLASS from their respective Home Assistant modules) and ensure
ConfigFlow/FlowType/SubentryFlowResult/ConfigEntry are fully imported where
referenced, or mark the whole block as illustrative pseudocode with a comment;
update references to PreviewSensorEntity to show it subclasses SensorEntity and
uses CONF_DEVICE_CLASS so readers know those dependencies must be imported or
stubbed before copying.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 24e149e9-0548-42ab-93d4-1a5dc25660c2
📒 Files selected for processing (1)
docs/data_entry_flow_index.md
12 tasks
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
docs/data_entry_flow_index.md (2)
698-698:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlign
PreviewSensorEntityconstructor usage with class definition.Line 698 passes
hassintoPreviewSensorEntity(...), but Line 720 defines__init__withoutselfand withouthass; Line 722-723 also useconfig.get[...]instead ofconfig.get(...).Suggested patch
- preview = PreviewSensorEntity(hass, preview_value=preview_value, config=config) + preview = PreviewSensorEntity(preview_value=preview_value, config=config) @@ - def __init__(preview_value: str, config: dict[str, str]) -> None: + def __init__(self, preview_value: str, config: dict[str, str]) -> None: self._attr_native_value = preview_value - self._attr_icon = config.get[ATTR_ICON] - self._attr_name = config.get[ATTR_NAME] + self._attr_icon = config.get(ATTR_ICON) + self._attr_name = config.get(ATTR_NAME) self._attr_device_class = config.get(CONF_DEVICE_CLASS) self._attr_native_unit_of_measurement = config.get(CONF_UNIT_OF_MEASUREMENT) self._attr_state_class = config.get(CONF_STATE_CLASS)#!/bin/bash # Verify constructor call/signature and config.get usage mismatch. sed -n '697,725p' docs/data_entry_flow_index.md | nl -baAlso applies to: 720-724
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/data_entry_flow_index.md` at line 698, The constructor call passes hass (preview = PreviewSensorEntity(hass, preview_value=preview_value, config=config)) but the class __init__ signature is defined incorrectly (missing self and hass) and uses bracket indexing for config (config.get[...] instead of config.get(...)); fix by updating PreviewSensorEntity.__init__ to include the proper signature (def __init__(self, hass, preview_value=None, config=None):) or remove hass from the constructor call to match the desired design, and replace all occurrences of config.get[...] with config.get(...) so the config lookups use method calls; ensure the symbol PreviewSensorEntity.__init__ and the constructor call preview = PreviewSensorEntity(...) are consistent.
639-645:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDefine
cur_stepbefore using it.Line 644 reads
cur_step["handler"]without definingcur_step, so the example is still not self-contained.Suggested patch
if msg["flow_type"] is FlowType.CONFIG_SUBENTRIES_FLOW: flow_status: SubentryFlowResult = hass.config_entries.subentries.async_get(msg["flow_id"]) # TODO: get the config entry (if applicable or needed) # handler for subentry flow is a tuple of str of form # (entry_id, subentry_flow_type) + cur_step = flow_status["cur_step"] # replace with the actual key/shape used entry_id, _ = cur_step["handler"] config_entry: ConfigEntry = hass.config_entries.async_get(entry_id)#!/bin/bash # Verify that cur_step is referenced before declaration in the snippet. sed -n '636,646p' docs/data_entry_flow_index.md | nl -ba🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/data_entry_flow_index.md` around lines 639 - 645, The snippet uses cur_step["handler"] before cur_step is defined; update the FlowType.CONFIG_SUBENTRIES_FLOW branch to first obtain the current step (e.g., derive cur_step from flow_status or msg such as using flow_status.cur_step or flow_status["steps"][...]) so cur_step is defined before accessing cur_step["handler"]; ensure you reference the same flow_status from hass.config_entries.subentries.async_get(msg["flow_id"]) and then extract entry_id, _ = cur_step["handler"] and call hass.config_entries.async_get(entry_id) to populate config_entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/data_entry_flow_index.md`:
- Around line 606-616: The snippet has invalid imports and dict syntax: change
the module import from homeassistant.config_entry to
homeassistant.config_entries and ensure commas separate imported names (e.g.,
after FlowType); also fix the constant imports to include commas between
ATTR_ICON and ATTR_NAME. In any example dict initialization shown (where keys
were written with =), replace assignment operators with proper dict key: value
pairs (use colons) and add any missing commas between entries; update the same
fixes for the repeated snippet around the later occurrence of the example (the
block referencing websocket_api,
ConfigEntry/ConfigFlow/FlowType/SubentryFlowResult, and ATTR_ICON/ATTR_NAME).
---
Duplicate comments:
In `@docs/data_entry_flow_index.md`:
- Line 698: The constructor call passes hass (preview =
PreviewSensorEntity(hass, preview_value=preview_value, config=config)) but the
class __init__ signature is defined incorrectly (missing self and hass) and uses
bracket indexing for config (config.get[...] instead of config.get(...)); fix by
updating PreviewSensorEntity.__init__ to include the proper signature (def
__init__(self, hass, preview_value=None, config=None):) or remove hass from the
constructor call to match the desired design, and replace all occurrences of
config.get[...] with config.get(...) so the config lookups use method calls;
ensure the symbol PreviewSensorEntity.__init__ and the constructor call preview
= PreviewSensorEntity(...) are consistent.
- Around line 639-645: The snippet uses cur_step["handler"] before cur_step is
defined; update the FlowType.CONFIG_SUBENTRIES_FLOW branch to first obtain the
current step (e.g., derive cur_step from flow_status or msg such as using
flow_status.cur_step or flow_status["steps"][...]) so cur_step is defined before
accessing cur_step["handler"]; ensure you reference the same flow_status from
hass.config_entries.subentries.async_get(msg["flow_id"]) and then extract
entry_id, _ = cur_step["handler"] and call
hass.config_entries.async_get(entry_id) to populate config_entry.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cae6d8df-753b-472e-b417-ada8964a2d9d
📒 Files selected for processing (1)
docs/data_entry_flow_index.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed change
Add documentation for using data entry flow previews.
Type of change
Checklist
Additional information
Creating an integration config flow with previews took hours of reverse engineering existing code and detailed review of existing components that use previews (for example):
homeassistant.components.templateshomeassistant.components.groupThis additional documentation seeks to help others to avoid this deep dive into the inner workings of the core and frontend to get previews working in their code. Please note the use of
domainin the code is dependent on the frontend PR referenced below. We should discuss if we should use the key "domain" or something like "entity_domain" which might be a bit more clear.Summary by CodeRabbit