feat(deploy): human-readable deploy drift (--diff human)#2984
Conversation
Add --diff human to print a ToolkitPanel summary and a side-by-side comparison of CDF API dumps vs build YAML for changed resources. Verbose unified diff is skipped when human diff is selected.
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 the 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 counterproductive. 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. |
☂️ Python Coverage
Overall Coverage
New Files
Modified Files
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2984 +/- ##
========================================
Coverage 85.85% 85.85%
========================================
Files 462 463 +1
Lines 42543 42658 +115
========================================
+ Hits 36524 36623 +99
- Misses 6019 6035 +16
🚀 New features to boost your workflow:
|
|
/gemini review |
![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 introduces a human-readable side-by-side diff view for the deploy command, enabling comparison between local build configurations and the CDF API state. It also implements sorting for deployment results to highlight significant changes and enhances UI components with better documentation and padding control. Feedback was provided regarding a bug in the dictionary alignment logic where type mismatches could cause data loss in the diff display.
| elif isinstance(cv, dict): | ||
| out_cdf[k], out_build[k] = _align_nested_dict_pair_for_yaml(cv, {}) | ||
| elif isinstance(bv, dict): | ||
| out_cdf[k], out_build[k] = _align_nested_dict_pair_for_yaml({}, bv) |
There was a problem hiding this comment.
The current implementation of _align_nested_dict_pair_for_yaml causes data loss when there is a type mismatch between the CDF and local build configurations. If one side is a dictionary and the other is a scalar value (e.g., a string or integer), the scalar value is incorrectly replaced by an empty dictionary {} in the aligned output. This leads to misleading diff results where actual values appear to be replaced by empty objects.
The logic should only align with an empty dictionary if the key is entirely missing from the other side.
| elif isinstance(cv, dict): | |
| out_cdf[k], out_build[k] = _align_nested_dict_pair_for_yaml(cv, {}) | |
| elif isinstance(bv, dict): | |
| out_cdf[k], out_build[k] = _align_nested_dict_pair_for_yaml({}, bv) | |
| elif isinstance(cv, dict) and k not in build: | |
| out_cdf[k], out_build[k] = _align_nested_dict_pair_for_yaml(cv, {}) | |
| elif isinstance(bv, dict) and k not in cdf: | |
| out_cdf[k], out_build[k] = _align_nested_dict_pair_for_yaml({}, bv) |
Gemini review: only recurse with an empty dict when the key is missing on the other side, not when types disagree (dict vs scalar). Adds regression tests for dict/scalar mismatches in both directions.
Flatten read-dir summary into the panel section list so bullets line up with the Plan block. Extract hosted extractor source/destination messages as constants and explain id-only CDF comparison. Drop unused future annotations from diff_display per project conventions.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a human-readable diff view for the deploy v2 command, allowing users to see side-by-side comparisons between local build YAMLs and CDF API data. It also includes UI enhancements to ToolkitPanel and ToolkitPanelSection to support better content padding and layout. Feedback was provided regarding missing or non-standard docstrings for the new diff display functions to ensure compliance with the repository style guide.
| def _align_nested_dict_pair_for_yaml(cdf: Any, build: Any) -> tuple[Any, Any]: | ||
| """Return copies of ``cdf`` / ``build`` with identical dict key order for paired dumps. | ||
|
|
||
| Order at each dict level: keys from **build** (insertion order), then keys only in **CDF** | ||
| (insertion order). Nested dict values are aligned the same way so ``yaml_safe_dump(..., | ||
| sort_keys=False)`` produces line-aligned text without global alphabetical sorting. | ||
|
|
||
| Non-dict values and mismatched types are left unchanged; lists are not reordered. | ||
| Empty ``{}`` is used only when a dict exists on one side and the **key** is absent on the other, | ||
| so scalar-vs-dict mismatches are not coerced to ``{}``. | ||
| """ |
There was a problem hiding this comment.
The docstring for _align_nested_dict_pair_for_yaml does not follow the required Args: and Returns: format specified in the repository style guide. Please update it to adhere to the standard format.
def _align_nested_dict_pair_for_yaml(cdf: Any, build: Any) -> tuple[Any, Any]:
"""
Return copies of ``cdf`` / ``build`` with identical dict key order for paired dumps.
Order at each dict level: keys from **build** (insertion order), then keys only in **CDF**
(insertion order). Nested dict values are aligned the same way so ``yaml_safe_dump(...,
sort_keys=False)`` produces line-aligned text without global alphabetical sorting.
Non-dict values and mismatched types are left unchanged; lists are not reordered.
Empty ``{}`` is used only when a dict exists on one side and the **key** is absent on the other,
so scalar-vs-dict mismatches are not coerced to ``{}``.
Args:
cdf: The CDF resource data.
build: The local build resource data.
Returns:
tuple[Any, Any]: A tuple containing the aligned (cdf, build) data.
"""References
- Use concise docstrings with Args/Returns format. (link)
| def render_deploy_human_diff( | ||
| *, | ||
| resource_name: str, | ||
| identifier: Any, | ||
| source_file: Path, | ||
| cdf_dict: dict[str, Any], | ||
| yaml_dict: dict[str, Any], | ||
| sensitive_strings: Iterable[str], | ||
| cdf_project: str, | ||
| ) -> ToolkitPanel: |
There was a problem hiding this comment.
The render_deploy_human_diff function is missing a docstring. According to the repository style guide, all functions must have concise docstrings using the Args: and Returns: format.
def render_deploy_human_diff(
*,
resource_name: str,
identifier: Any,
source_file: Path,
cdf_dict: dict[str, Any],
yaml_dict: dict[str, Any],
sensitive_strings: Iterable[str],
cdf_project: str,
) -> ToolkitPanel:
"""
Renders a human-readable diff panel for a resource.
Args:
resource_name: The display name of the resource type.
identifier: The unique identifier of the resource.
source_file: The path to the local source file.
cdf_dict: The resource data as retrieved from CDF.
yaml_dict: The resource data as defined in the local build.
sensitive_strings: Strings to be masked in the output.
cdf_project: The name of the CDF project.
Returns:
ToolkitPanel: A panel containing the diff view.
"""References
- All functions, methods, and class attributes must have type hints. Use concise docstrings with Args/Returns format. (link)
| "--diff", | ||
| help="When set to 'human', show a ToolkitPanel summary and a side-by-side line comparison of " | ||
| "CDF (API) dump vs build YAML for each resource that would change.", | ||
| ), |
| left = cdf_lines[i1:i2] | ||
| right = yaml_lines[j1:j2] | ||
| for k in range(max(len(left), len(right))): | ||
| l_txt = left[k] if k < len(left) else "" |
There was a problem hiding this comment.
| l_txt = left[k] if k < len(left) else "" | |
| left_text = left[k] if k < len(left) else "" |
| right = yaml_lines[j1:j2] | ||
| for k in range(max(len(left), len(right))): | ||
| l_txt = left[k] if k < len(left) else "" | ||
| r_txt = right[k] if k < len(right) else "" |
There was a problem hiding this comment.
| r_txt = right[k] if k < len(right) else "" | |
| right_text = right[k] if k < len(right) else "" |
| for k in range(max(len(left), len(right))): | ||
| l_txt = left[k] if k < len(left) else "" | ||
| r_txt = right[k] if k < len(right) else "" | ||
| yield (Text(l_txt, style="yellow"), Text(r_txt, style="cyan")) |
There was a problem hiding this comment.
| yield (Text(l_txt, style="yellow"), Text(r_txt, style="cyan")) | |
| yield (Text(left_text, style="yellow"), Text(right_text, style="cyan")) |
3 participants
Description
What you get: With
cdf deploy … --diff human, each resource that differs from CDF is shown in one Rich panel: identifier and file path, a short diff summary, and two columns of YAML (Local build vs CDF, with the project name on the CDF side). Sensitive values are masked the same way as in--verbose; for those resources the long unified verbose diff is not printed, so output stays scannable.Also in this PR: Deploy summary rows are ordered by impact (creates and updates before unchanged). Human diff YAML keeps build key order and pairs keys sensibly (including a fix when one side is a dict and the other a scalar). Hosted extractor source/destination warnings explain that CDF is compared by external id only. The processed build-directory block uses the same nesting as the plan section so bullets line up.
ToolkitPanelSectiongains optionalcontent_padding, with padding tuple behavior documented on the panel helpers.Demo with and without human diff:
Screen.Recording.2026-05-06.at.16.12.16.mov
Bump
Changelog
Added
cdf deploy --diff=humananddeploy_v2/diff_display.pyfor structured drift output.Changed
Improved
Fixed
{}when the other side is a dict for the same key.