remove arg-type exclusion from mypy rules on `schema_branch.py` by ajtmccarty · Pull Request #9343 · opsmill/infrahub

ajtmccarty · 2026-05-22T18:39:54Z

Summary

Removes arg-type from the mypy override for infrahub.core.schema.schema_branch and fixes all 20 hidden type errors

Changes

`SchemaBranch.name` is now required

__init__ signature changed from name: str | None = None to name: str — every production call site already passed a name; only test fixtures were relying on
the default.
to_dict_schema_object() now carries "name" in the payload; from_dict_schema_object() reads it. This keeps round-tripping honest across the Pydantic
validator/migration pipelines (validators/model.py, validators/models/validate_migration.py, migrations/schema/models.py).
duplicate(name=None) now falls back to self.name instead of producing a nameless copy.
34 test instantiation sites across 16 files updated to pass name="test".

Type narrowing in `SchemaBranch` methods

Replaced overly broad self.get() calls with the existing typed helpers get_node() / get_generic() at 6 sites where the iteration source guarantees the kind.
Extracted _diff_node_or_generic() helper in SchemaBranch.diff() to dispatch by kind so each subclass's diff(other: Self) contract is satisfied without cast.
Added assert isinstance(node, NodeSchema | GenericSchema) at two loops that iterate node_names + generic_names_without_templates — the union return type was
strictly wider than the iteration source.
Widened generate_profile_from_node(node: NodeSchema) to NodeSchema | GenericSchema. The body only touches BaseNodeSchema attributes, and the calling loop
legitimately invokes it for both kinds (both have generate_profile).

Guard against `None` in `add_hierarchy_node`

Wrapped the parent- and children-relationship blocks in if parent_peer is not None: / if children_peer is not None: guards. Schema procession would make reaching this point impossible, but the typing does not make that clear.

Summary by cubic

Make SchemaBranch.name required and include it in schema serialization. Tightens typing across schema_branch.py, adds safe dispatch in diff(), and removes the arg-type mypy override.

Refactors
- SchemaBranch.__init__ now requires name: str; tests updated to pass name="test".
- to_dict_schema_object() returns SchemaBranchDict and writes "name"; from_dict_schema_object() now requires it.
- Introduced SchemaBranchDict TypedDict.
- Added get_node_or_generic_schema(); replaced broad get() with typed getters (get_node/get_generic/get_profile/get_template) and _diff_node_or_generic() for type-safe diff().
- generate_profile_from_node() accepts NodeSchema | GenericSchema; replaced assertions with conditional checks.
- duplicate(name=None) falls back to self.name; removed arg-type from mypy overrides in pyproject.toml.
Bug Fixes
- Guarded add_hierarchy_node to skip creating parent/children relationships when peers are None.

^{Written for commit 71b0f01. Summary will update on new commits. Review in cubic}

ajtmccarty · 2026-05-22T18:41:13Z

        dependency_map: dict[str, set[str]] = defaultdict(set)
        for name in self.generic_names_without_templates + self.node_names:
            node_schema = self.get(name=name, duplicate=False)
+            assert isinstance(node_schema, NodeSchema | GenericSchema)


not sure if assert is better or a if not isinstance(...): continue

I'd avoid assert in production code. We have it in a few places, but it can be excluded/ignored at runtime with environment variables.

cubic-dev-ai

No issues found across 18 files

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

_{Shadow auto-approve: would auto-approve. This PR primarily tightens types in schema_branch.py and updates test instantiations to pass a required name, with no changes to business logic or runtime behavior, making it a low-risk improvement to type safety.

Re-trigger cubic}

codspeed-hq · 2026-05-22T18:46:37Z

Merging this PR will not alter performance

✅ 12 untouched benchmarks

_{Comparing ajtm-05222026-schema-branch-arg-type (71b0f01) with stable (edaeba7)¹}

No successful run was found on stable (969810f) during the generation of this report, so edaeba7 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

Replace assertions with conditional checks for node schema types.

cubic-dev-ai

1 issue found across 1 file (changes from recent commits).

_{Shadow auto-approve: would not auto-approve because issues were found.

Re-trigger cubic}

cubic-dev-ai

0 issues found across 1 file (changes from recent commits).

_{Shadow auto-approve: would auto-approve. These type-safety improvements enforce existing invariants and eliminate potential runtime errors, without changing behavior or relying on risky assumptions.

Re-trigger cubic}

ogenstad

Nice!

ogenstad · 2026-05-25T08:08:18Z

        return {"nodes": self.nodes, "generics": self.generics, "profiles": self.profiles, "templates": self.templates}

-    def to_dict_schema_object(self, duplicate: bool = False) -> dict[str, dict[str, MainSchemaTypes]]:
+    def to_dict_schema_object(self, duplicate: bool = False) -> dict[str, Any]:


I think this is fine as a way to correct things. But Looking at this method I also think that we could do a bit better. It looks like this should return a TypedDict where "name" is just a string and "nodes" is a dict[str, NodeSchema] etc. Would have to change the method so that we call something other than "self.get" for that to work.

oh yeah. way better. added it.

ogenstad · 2026-05-25T08:27:06Z

                cache[node_hash] = node

-        return cls(cache=cache, data=nodes)
+        return cls(cache=cache, data=nodes, name=data.get("name", "Unknown"))


While I think this should be fine in general there's also the fact that "Unknown" is a perfectly fine name for a branch. I'm assuming that the data dict from here would match the one that's coming from .to_dict_schema_object() if that method returned a typeddict instead we should be fine to just access "name" directly.

ogenstad · 2026-05-25T08:30:02Z

        for name in self.generic_names_without_templates + self.node_names:
            node_schema = self.get(name=name, duplicate=False)
+            if not isinstance(node_schema, NodeSchema | GenericSchema):
+                continue


Was this only for typing? Wondering if we should have some other method aside from self.get() which would return the expected types instead.

yes. I added a get_node_or_generic_schema() method to cover this case and replaced the existing isinstance and asserts for those types with the new method

ogenstad · 2026-05-25T08:30:43Z

        for node_name in self.node_names + self.generic_names_without_templates:
            node = self.get(name=node_name, duplicate=False)
+            if not isinstance(node, NodeSchema | GenericSchema):
+                continue


Same comment as above.

cubic-dev-ai

0 issues found across 1 file (changes from recent commits).

_{Shadow auto-approve: would require human review. This PR refactors critical schema business logic in schema_branch.py, making the name field required, altering to_dict_schema_object output, and adding new type dispatch—these changes affect core data structures and could introduce subtle bugs in schema validation, migration, and...

Re-trigger cubic}

ajtmccarty added 5 commits May 22, 2026 10:18

make name required on SchemaBranch

40ec737

guard for None in add_hierarchy_node

f2c7409

clean up Node/Generic typing

b4b678b

clean up typing in SchemaBranch.diff

4bda2f4

remove arg-type exception from mypy on schema_branch.py

2eda1d6

github-actions Bot added the group/backend Issue related to the backend (API Server, Git Agent) label May 22, 2026

ajtmccarty commented May 22, 2026

View reviewed changes

cubic-dev-ai Bot reviewed May 22, 2026

View reviewed changes

Change assertions to conditionals for node schema validation

359f1b2

Replace assertions with conditional checks for node schema types.

cubic-dev-ai Bot reviewed May 22, 2026

View reviewed changes

Comment thread backend/infrahub/core/schema/schema_branch.py Outdated

ajtmccarty commented May 22, 2026

View reviewed changes

Comment thread backend/infrahub/core/schema/schema_branch.py Outdated

Apply suggestion from @ajtmccarty

01b7aa6

cubic-dev-ai Bot reviewed May 22, 2026

View reviewed changes

ajtmccarty marked this pull request as ready for review May 22, 2026 22:39

ajtmccarty requested a review from a team as a code owner May 22, 2026 22:39

ogenstad approved these changes May 25, 2026

View reviewed changes

ajtmccarty added 4 commits May 26, 2026 10:59

add SchemaBranchDict TypedDict

17122b5

remove .get() now that TypedDict is in place

34504b8

add and use get_node_or_generic_schema()

fd544bd

fix leftover conflict

71b0f01

cubic-dev-ai Bot reviewed May 26, 2026

View reviewed changes

ajtmccarty merged commit e74ce7c into stable May 27, 2026
65 checks passed

ajtmccarty deleted the ajtm-05222026-schema-branch-arg-type branch May 27, 2026 13:26

Conversation

ajtmccarty commented May 22, 2026 • edited by cubic-dev-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

SchemaBranch.name is now required

Type narrowing in SchemaBranch methods

Guard against None in add_hierarchy_node

Summary by cubic

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

codspeed-hq Bot commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ogenstad left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ajtmccarty commented May 22, 2026 •

edited by cubic-dev-ai Bot

Loading

`SchemaBranch.name` is now required

Type narrowing in `SchemaBranch` methods

Guard against `None` in `add_hierarchy_node`

codspeed-hq Bot commented May 22, 2026 •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading