Fix array based conversion of display labels for obsolete format

[ogenstad]

For display_labels we supported both short and verbose attribute values i.e. both ["name"] and ["name__value"], this change ensures that we also support the same when migrating display_labels to display_label within the SchemaBranch.

Summary by CodeRabbit

• Refactor

  • Improved consistency of display label formatting logic through standardized component handling.

• Tests

  • Added comprehensive unit test coverage for display label computation, validating multiple formatting scenarios including single and multi-component labels with various configurations.

[coderabbitai]

Walkthrough

This change refactors display label formatting in the schema module by introducing a centralized helper function to standardize component formatting. The helper function _format_display_label_component ensures display label components have appropriate __value suffixes. The refactoring applies this function to both single-component and multi-component display label constructions within Jinja2 templates. A new comprehensive unit test file validates the display label computation across multiple scenarios, including single and dual-attribute labels with various configurations and filter transformations.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The PR title "Fix array based conversion of display labels for obsolete format" is directly related to the changeset. According to the PR objectives, this PR addresses migration logic for display label fields by ensuring support for both short and verbose attribute formats (e.g., ["name"] and ["name__value"]) when converting from the old display_labels field to the newer display_label field. The changes implement a helper function _format_display_label_component to standardize this formatting conversion and add comprehensive test coverage for various display label scenarios. The title is specific and clear—it identifies the array-based conversion process, the subject (display labels), and the context (obsolete format), making it easy for teammates reviewing history to understand the primary change.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch pog-array-based-conversion-display-labels

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codspeed-hq]

CodSpeed Performance Report

Merging #7434 will not alter performance

_{Comparing pog-array-based-conversion-display-labels (076f2ef) with release-1.5 (e1bb469)}

Summary

✅ 10 untouched

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

backend/infrahub/core/schema/schema_branch.py (1)

2597-2607: Update docstring to follow Google style format.

The docstring is missing required sections per the coding guidelines. Add Args and Returns sections.

Apply this diff:

 def _format_display_label_component(component: str) -> str:
-    """Return correct format for display_label.
+    """Return correct format for display_label component.
 
     Previously both the format of 'name' and 'name__value' was
     supported this function ensures that the proper 'name__value'
     format is used
+
+    Args:
+        component: The display label component to format
+
+    Returns:
+        The formatted component with __value suffix when needed
     """
     if "__" in component:
         return component
 
     return f"{component}__value"

backend/tests/unit/core/schema/schema_branch/test_schema_convert_display_labels.py (1)

95-103: Remove unnecessary async keyword.

The test function is marked as async but contains no await calls. Since the test performs only synchronous operations, the async keyword should be removed.

Apply this diff:

-async def test_expected_final_display_label(
+def test_expected_final_display_label(
     test_case: DisplayLabelTestCase,
 ) -> None:
     """Test that the final computed display label matches the expected value."""

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e1bb469 and 076f2ef.

📒 Files selected for processing (2)

• backend/infrahub/core/schema/schema_branch.py (2 hunks)
• backend/tests/unit/core/schema/schema_branch/test_schema_convert_display_labels.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

backend/tests/**/*

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Place backend tests in backend/tests/

Files:

• backend/tests/unit/core/schema/schema_branch/test_schema_convert_display_labels.py

backend/**/*

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Run backend tests with pytest or via invoke tasks

Files:

• backend/tests/unit/core/schema/schema_branch/test_schema_convert_display_labels.py
• backend/infrahub/core/schema/schema_branch.py

**/*.py

📄 CodeRabbit inference engine (.github/instructions/python-docstring.instructions.md)

**/*.py: Use triple double quotes (""") for all Python docstrings
Write docstrings in Google-style format
Include a brief one-line description at the top of each docstring
Add a detailed description section when additional context is needed
Document function/method parameters under an Args/Parameters section without typing information
Include a Returns section describing the return value
Include a Raises section listing possible exceptions
Provide an Examples section demonstrating usage when helpful

Use ruff and mypy to validate and lint Python files

**/*.py: Use type hints for all Python function parameters and return values
Prefer asynchronous code in Python when feasible
Define asynchronous Python functions with async def
Use await for asynchronous calls in Python
Use Pydantic models instead of standard dataclasses
Use ruff and mypy for linting and type checking

**/*.py: Use type hints for all Python function parameters and return values
Use async/await whenever possible in Python code
Define asynchronous functions with async def
Await asynchronous calls with await
Use Pydantic models instead of standard dataclasses for data modeling
Use triple quotes (""") for all Python docstrings
Write docstrings in Google-style format
Include docstring sections when applicable: one-line summary, optional details, Args (without types), Returns, Raises, Examples
Validate and lint Python with ruff and mypy

Files:

• backend/tests/unit/core/schema/schema_branch/test_schema_convert_display_labels.py
• backend/infrahub/core/schema/schema_branch.py

🧬 Code graph analysis (1)

backend/tests/unit/core/schema/schema_branch/test_schema_convert_display_labels.py (1)

backend/infrahub/core/schema/schema_branch.py (4)

• load_schema (494-523)
• process (525-529)
• get_node (349-354)
• duplicate (280-289)

⏰ 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). (1)

• GitHub Check: Cloudflare Pages

🔇 Additional comments (2)

backend/infrahub/core/schema/schema_branch.py (1)

796-808: LGTM: Display label conversion logic correctly handles both formats.

The implementation properly converts legacy display_labels to the new display_label field, supporting both short (e.g., "name") and verbose (e.g., "name__value") formats as described in the PR objectives. The helper function ensures consistent formatting across single and multi-attribute cases.

backend/tests/unit/core/schema/schema_branch/test_schema_convert_display_labels.py (1)

9-88: Excellent test coverage for display label conversion scenarios.

The test cases comprehensively cover all the conversion scenarios mentioned in the PR: single attribute (with/without __value), dual attributes (mixed formats), and explicitly defined display labels. The parametrized approach makes it easy to understand each scenario.

[ajtmccarty]

good pattern. easy to read

[ogenstad]

Yeah, I agree we have it in a few places and I think we should do more of it. Especially for pytest.params with multiple variables.