Skip to content

Conversation

@ogenstad
Copy link
Contributor

@ogenstad ogenstad commented Dec 30, 2025

Fixed conflict in uv.lock, also added 51d4feb to fix a new violation in ruff.

Summary by CodeRabbit

Release Notes

  • Documentation

    • Enhanced Python typing guide with expanded step-by-step workflow examples, detailed code samples for GraphQL code generation, and guidance on query naming conventions.
  • Improvements

    • GraphQL code generation now automatically strips unnecessary type metadata fields, producing cleaner and more focused generated Python types.
  • Chores

    • Updated development dependencies for improved code quality checks.

✏️ Tip: You can customize this high-level summary in your review settings.

BaptisteGi and others added 6 commits December 30, 2025 09:10
* Fix issue #713

* Adjust documentation

* Fix issue #712

* Raise error if we can't type for selection node

* Add tests for ctl
Merge stable into develop
Upgrade ruff=0.14.10 and fix new violation
Merge stable into develop
@coderabbitai
Copy link

coderabbitai bot commented Dec 30, 2025

Walkthrough

This pull request adds GraphQL typename-stripping functionality to the SDK's code generation pipeline. Three utility functions are introduced to recursively remove \__typename fields from GraphQL operations and fragments while preserving other query structure. The graphql.py CLI module integrates this stripping before code generation. Documentation is updated with expanded usage examples and warnings about unique query names. Supporting test fixtures and comprehensive unit tests validate the typename-stripping behavior across simple and nested query structures. A minor dependency version bump and code style improvement are also included.

Pre-merge checks

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 64.71% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive Title describes a merge operation between branches but doesn't convey the specific functionality changes (GraphQL typename stripping, typing guide updates, test additions) that form the actual substance of this PR. Consider a more descriptive title reflecting the main changes, such as 'Strip __typename from GraphQL queries and fragments' or 'Add GraphQL typename stripping and expand typing documentation'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

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

@github-actions github-actions bot added the type/documentation Improvements or additions to documentation label Dec 30, 2025
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Dec 30, 2025

Deploying infrahub-sdk-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: 51d4feb
Status: ✅  Deploy successful!
Preview URL: https://3d6b13c0.infrahub-sdk-python.pages.dev
Branch Preview URL: https://pog-develop-to-infrahub-deve-5qnk.infrahub-sdk-python.pages.dev

View logs

@codecov
Copy link

codecov bot commented Dec 30, 2025

Codecov Report

❌ Patch coverage is 92.59259% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
infrahub_sdk/graphql/utils.py 90.90% 1 Missing and 1 partial ⚠️
@@                 Coverage Diff                  @@
##           infrahub-develop     #728      +/-   ##
====================================================
+ Coverage             76.29%   77.08%   +0.78%     
====================================================
  Files                   114      114              
  Lines                  9831     9854      +23     
  Branches               1509     1515       +6     
====================================================
+ Hits                   7501     7596      +95     
+ Misses                 1834     1754      -80     
- Partials                496      504       +8     
Flag Coverage Δ
integration-tests 34.40% <22.22%> (-0.07%) ⬇️
python-3.10 51.01% <92.59%> (+0.85%) ⬆️
python-3.11 51.01% <92.59%> (+0.85%) ⬆️
python-3.12 50.99% <92.59%> (+0.85%) ⬆️
python-3.13 50.99% <92.59%> (+0.85%) ⬆️
python-3.14 52.64% <92.59%> (+0.85%) ⬆️
python-filler-3.12 23.95% <0.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
infrahub_sdk/ctl/graphql.py 88.54% <100.00%> (+53.43%) ⬆️
infrahub_sdk/schema/repository.py 87.33% <100.00%> (-0.09%) ⬇️
infrahub_sdk/graphql/utils.py 73.80% <90.90%> (+53.80%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ogenstad ogenstad marked this pull request as ready for review December 30, 2025 13:34
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
tests/unit/ctl/test_graphql_utils.py (1)

1-242: LGTM! Comprehensive test coverage for typename-stripping utilities.

The test suite thoroughly validates the typename-stripping functionality across various GraphQL structures including nested queries, inline fragments, fragment definitions, and preservation of aliases, arguments, and directives. All tests follow the coding guidelines with proper type hints and no external dependencies.

💡 Optional: Consider adding a test for the TypeError case

The implementation in infrahub_sdk/graphql/utils.py (line 54) raises a TypeError when encountering an unexpected selection node type. While this is defensive programming for a case that shouldn't occur with valid GraphQL, consider adding a test to verify this behavior:

def test_strip_typename_raises_on_invalid_node_type(self) -> None:
    """Verify that invalid selection node types raise TypeError."""
    from graphql import SelectionSetNode
    from unittest.mock import Mock
    
    # Create a mock selection with an invalid type
    invalid_selection = Mock()
    invalid_selection.__class__.__name__ = "InvalidNode"
    
    selection_set = SelectionSetNode(selections=(invalid_selection,))
    
    with pytest.raises(TypeError, match="Unexpected GraphQL selection node type 'InvalidNode'"):
        strip_typename_from_selection_set(selection_set)

This would provide explicit coverage of the error path, though it's optional since the graphql-core library should never produce invalid node types.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 854412b and 51d4feb.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (11)
  • docs/docs/python-sdk/guides/python-typing.mdx
  • infrahub_sdk/ctl/graphql.py
  • infrahub_sdk/graphql/utils.py
  • infrahub_sdk/schema/repository.py
  • pyproject.toml
  • tests/fixtures/unit/test_infrahubctl/graphql/invalid_query.gql
  • tests/fixtures/unit/test_infrahubctl/graphql/query_with_typename.gql
  • tests/fixtures/unit/test_infrahubctl/graphql/test_schema.graphql
  • tests/fixtures/unit/test_infrahubctl/graphql/valid_query.gql
  • tests/unit/ctl/test_graphql_app.py
  • tests/unit/ctl/test_graphql_utils.py
🧰 Additional context used
📓 Path-based instructions (8)
**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Use type hints on all function signatures
Never mix async/sync inappropriately
Never bypass type checking without justification

Files:

  • infrahub_sdk/ctl/graphql.py
  • infrahub_sdk/graphql/utils.py
  • infrahub_sdk/schema/repository.py
  • tests/unit/ctl/test_graphql_app.py
  • tests/unit/ctl/test_graphql_utils.py
infrahub_sdk/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Follow async/sync dual pattern for new features in the Python SDK

Files:

  • infrahub_sdk/ctl/graphql.py
  • infrahub_sdk/graphql/utils.py
  • infrahub_sdk/schema/repository.py
infrahub_sdk/ctl/**/*.py

📄 CodeRabbit inference engine (infrahub_sdk/ctl/AGENTS.md)

infrahub_sdk/ctl/**/*.py: Use @catch_exception(console=console) decorator on all async CLI commands
Include CONFIG_PARAM in all CLI command function parameters, even if unused
Use initialize_client() or initialize_client_sync() from ctl.client for client creation in CLI commands
Use Rich library for output formatting in CLI commands (tables, panels, console.print) instead of plain print() statements
Do not instantiate InfrahubClient directly; always use initialize_client() or initialize_client_sync() helper functions
Do not use plain print() statements in CLI commands; use Rich console.print() instead

Files:

  • infrahub_sdk/ctl/graphql.py
**/*.{md,mdx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{md,mdx}: Use text language for directory structure code blocks in markdown documentation
Add blank lines before and after lists in markdown documentation
Always specify language in fenced code blocks in markdown documentation

Files:

  • docs/docs/python-sdk/guides/python-typing.mdx
docs/docs/**/*.mdx

📄 CodeRabbit inference engine (docs/AGENTS.md)

docs/docs/**/*.mdx: Create MDX files in appropriate directory within docs structure (guides, topics, or reference)
Add frontmatter with title field to MDX documentation files
Use callouts (:::warning, :::note, etc.) for important notes and information in documentation

Files:

  • docs/docs/python-sdk/guides/python-typing.mdx
docs/docs/python-sdk/{guides,topics}/**/*.mdx

📄 CodeRabbit inference engine (docs/AGENTS.md)

docs/docs/python-sdk/{guides,topics}/**/*.mdx: Use Tabs component from '@theme/Tabs' for async/sync examples in documentation
Include both async/sync examples using Tabs in documentation

Files:

  • docs/docs/python-sdk/guides/python-typing.mdx
tests/**/*.py

📄 CodeRabbit inference engine (tests/AGENTS.md)

tests/**/*.py: Use httpx_mock fixture for HTTP mocking in tests instead of making real HTTP requests
Do not add @pytest.mark.asyncio decorator to async test functions (async auto-mode is globally enabled)

Files:

  • tests/unit/ctl/test_graphql_app.py
  • tests/unit/ctl/test_graphql_utils.py
tests/unit/**/*.py

📄 CodeRabbit inference engine (tests/AGENTS.md)

Unit tests must be fast, mocked, and have no external dependencies

Files:

  • tests/unit/ctl/test_graphql_app.py
  • tests/unit/ctl/test_graphql_utils.py
🧠 Learnings (2)
📚 Learning: 2025-12-10T17:13:21.977Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: infrahub_sdk/ctl/AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:21.977Z
Learning: Applies to infrahub_sdk/ctl/**/*.py : Use `initialize_client()` or `initialize_client_sync()` from ctl.client for client creation in CLI commands

Applied to files:

  • infrahub_sdk/ctl/graphql.py
📚 Learning: 2025-12-10T17:13:14.678Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: docs/AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:14.678Z
Learning: Applies to docs/python-sdk/reference/config.mdx : Do not edit `docs/python-sdk/reference/config.mdx` directly; regenerate with `uv run invoke generate-sdk` instead

Applied to files:

  • docs/docs/python-sdk/guides/python-typing.mdx
🧬 Code graph analysis (3)
infrahub_sdk/ctl/graphql.py (1)
infrahub_sdk/graphql/utils.py (4)
  • insert_fragments_inline (98-116)
  • remove_fragment_import (119-125)
  • strip_typename_from_fragment (74-86)
  • strip_typename_from_operation (59-71)
tests/unit/ctl/test_graphql_app.py (2)
infrahub_sdk/ctl/graphql.py (2)
  • find_gql_files (43-59)
  • get_graphql_query (62-79)
tests/helpers/cli.py (1)
  • remove_ansi_color (4-6)
tests/unit/ctl/test_graphql_utils.py (1)
infrahub_sdk/graphql/utils.py (3)
  • strip_typename_from_fragment (74-86)
  • strip_typename_from_operation (59-71)
  • strip_typename_from_selection_set (14-56)
⏰ 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 (17)
infrahub_sdk/schema/repository.py (1)

154-154: LGTM!

The refactor to use read_text(encoding="UTF-8") is more concise and idiomatic for pathlib usage, with explicit encoding specification.

tests/fixtures/unit/test_infrahubctl/graphql/invalid_query.gql (1)

1-9: LGTM!

This test fixture appropriately provides an invalid query for testing error handling in the GraphQL CLI tooling.

tests/fixtures/unit/test_infrahubctl/graphql/query_with_typename.gql (1)

1-16: LGTM!

This fixture appropriately tests the typename-stripping functionality with __typename fields at multiple levels of the query structure.

docs/docs/python-sdk/guides/python-typing.mdx (1)

114-177: LGTM!

The documentation improvements are excellent:

  • Clearer usage examples showing both directory and file usage
  • Well-structured numbered workflow steps
  • Important warning about unique query names to prevent file overrides
  • Consistent example using GetAllTags that aligns with test fixtures
tests/fixtures/unit/test_infrahubctl/graphql/valid_query.gql (1)

1-15: LGTM!

This test fixture provides a valid GraphQL query for testing successful query parsing and code generation.

infrahub_sdk/ctl/graphql.py (5)

25-30: LGTM!

The imports for typename-stripping utilities are correctly added to support the new functionality.


160-164: LGTM!

The typename stripping logic is correctly implemented with excellent inline documentation explaining why this preprocessing is necessary (to prevent ariadne-codegen failures with the __typename introspection field).


168-168: LGTM!

Correctly passes stripped_fragments to the package generator, consistent with the typename-stripping approach.


171-171: LGTM!

The improved default naming logic for target_package_name using directory.name or falling back to "graphql_client" provides better naming conventions for generated packages.


180-180: LGTM!

Correctly iterates over stripped_queries instead of the original queries, ensuring typename fields are not present in the generated code.

tests/unit/ctl/test_graphql_app.py (1)

1-266: LGTM!

Excellent comprehensive test coverage for the GraphQL CLI tooling:

  • Thorough testing of find_gql_files with various scenarios (single file, directory, nested, empty)
  • Complete testing of get_graphql_query with valid/invalid cases
  • End-to-end CLI command tests covering success and error paths
  • Proper verification of typename stripping behavior
  • Well-structured with clear docstrings and proper use of fixtures
pyproject.toml (1)

83-83: ruff 0.14.10 is available and secure. Version exists on PyPI with no known security vulnerabilities or reported regressions. Released Dec 18, 2025.

infrahub_sdk/graphql/utils.py (4)

3-11: LGTM! Necessary imports for typename-stripping functionality.

The imports include all required GraphQL AST node types for manipulating selection sets.


59-71: LGTM! Clean wrapper for operation-level typename stripping.

The function correctly preserves all operation attributes while delegating the selection set processing to strip_typename_from_selection_set.


74-86: LGTM! Clean wrapper for fragment-level typename stripping.

The function correctly preserves all fragment attributes while delegating the selection set processing to strip_typename_from_selection_set.


14-56: graphql-core API compatibility confirmed across version range 3.1.x to 3.2.x.

The implementation correctly handles all GraphQL selection node types and recursively strips __typename fields while preserving query structure (aliases, arguments, directives). The defensive TypeError on line 54 is good practice.

The node constructor signatures (FieldNode, InlineFragmentNode, SelectionSetNode) are stable across graphql-core versions 3.1 and 3.2. The code uses tuple-based selection lists (selections=tuple(...)) which is compatible with both versions, and creates new node instances rather than mutating existing ones—a safe approach that avoids issues from 3.2's immutable tuple transition.

tests/fixtures/unit/test_infrahubctl/graphql/test_schema.graphql (1)

1-47: LGTM! Well-structured GraphQL schema fixture for testing.

The schema provides appropriate test data with sufficient complexity (interfaces, nested objects, relay-style pagination) to validate the typename-stripping utilities. The structure aligns well with the test cases in test_graphql_utils.py.

@ogenstad ogenstad requested a review from a team December 30, 2025 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants